Bug 51658

Created attachment 63712 [details] [review]
Essential patch to allow 2048 pixel blits.

Without this patch, gnome shell crashes. 2048 pixel blits are used.

Created attachment 63713 [details] [review]
Essential patch to reapply dirtied texenv registers.

Without this patch, colour corruption happens with xv.  xf86-video-ati textured xv dirties texenv registers and r200 DRI does not reapply them.

Created attachment 63714 [details] [review]
Unessential fixes/enhancements patch.

Minor spelling fixes & enhancements.

Created attachment 63715 [details] [review]
Proposed blit register dirtying patch.

It appears bliting dirties some registers without notifying. This proposed patch notifies of register dirtying.

Created attachment 63716 [details] [review]
Optional patch to warn (once) when a blit with 2048 pixel dimension occurs on r200.

If 2048 pixel blits cause graphical glitches/problems on r200, this patch can be used to provide a single warning.

Created attachment 63717 [details] [review]
Untested but probably essential patch to allow 2048 pixel blits on r100.

Without this patch, gnome shell is expected to crash. This patch is untested.

Created attachment 63718 [details] [review]
Optional untested patch to warn (once) when a blit with 2048 pixel dimension occurs on r100.

If 2048 pixel blits cause graphical glitches/problems on r100, this patch can
be used to provide a single warning. This patch is untested.

A summary of all of the patches follows.

r200 essential patches (1st 3 patches) for gnome shell:
"Essential patch to disable texture formats that are reported unrenderable elsewhere in driver."
"Essential patch to allow 2048 pixel blits."
"Essential patch to reapply dirtied texenv registers."

suggested minor fixes patch (4th patch):
"Unessential fixes/enhancements patch."

proposed r200 blit patch (5th patch) - unknown importance:
"Proposed blit register dirtying patch."

untested but probably essential r100/radeon patch (7th patch) for gnome shell:
"Untested but probably essential patch to allow 2048 pixel blits on r100."

optional unrecommended patches to supplement blit patches already mentioned above (6th & 8th patches):
"Optional patch to warn (once) when a blit with 2048 pixel dimension occurs on r200."
"Optional untested patch to warn (once) when a blit with 2048 pixel dimension occurs on r100."

(In reply to comment #8)
> A summary of all of the patches follows.
> 
> r200 essential patches (1st 3 patches) for gnome shell:
> "Essential patch to disable texture formats that are reported unrenderable
> elsewhere in driver."
I'm not really happy with that. I guess the problem is if we try to attach a texture to a fbo it is too late to notice we cannot render to that format. But this sort of sucks for ordinary textures. Ideally we'd probably be able to really determine our format only at the first upload of data to the texture (which presumably won't happen) so we could change it if we attach it to a fbo. Though if there is indeed already data uploaded we're screwed.

> "Essential patch to allow 2048 pixel blits."
This one looks good as far as I can tell. The programmed values are width/height -1 so this clearly should work.

> "Essential patch to reapply dirtied texenv registers."
I don't understand this. If the state isn't used anyway why need to reuppload it, dirty or not? I think this is just hiding the root cause of another bug.

> 
> suggested minor fixes patch (4th patch):
> "Unessential fixes/enhancements patch."
Look good.

> 
> proposed r200 blit patch (5th patch) - unknown importance:
> "Proposed blit register dirtying patch."
Makes sense to me.

> 
> untested but probably essential r100/radeon patch (7th patch) for gnome shell:
> "Untested but probably essential patch to allow 2048 pixel blits on r100."
Yes that should be same as for r200.

> 
> optional unrecommended patches to supplement blit patches already mentioned
> above (6th & 8th patches):
> "Optional patch to warn (once) when a blit with 2048 pixel dimension occurs on
> r200."
> "Optional untested patch to warn (once) when a blit with 2048 pixel dimension
> occurs on r100."
I don't think this is necessary - at least not there. texture src width/height of 2048 should clearly work. I am however not so sure about destination. width/height are fed into RADEON_RE_WIDTH_HEIGHT, and those have just 11 bit, which isn't enough for 2048. Furthermore, docs say the values are inclusive. Not sure what's up with that, the ddx also just uses width/height and not width/height -1 but clamps them to 2047 in some places. dri driver though otherwise seems to use x2/y2 (i.e. width -1, height -1). So I think this should be fixed everywhere to really use width/height -1.

With 5b88a2a22daae4d09596804d8edc6b8796d05150 attachment 63712 [details] [review], 63716, 63717, 63718 are obsolete. Still unsure what to do with the others.