Bug 51658 - r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
Summary: r200 (& possibly radeon) DRI fixes for gnome shell on Mesa 8.0.3
Status: NEW
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/r200 (show other bugs)
Version: 8.0
Hardware: All All
: medium major
Assignee: Default DRI bug account
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-02 10:25 UTC by Eugene St Leger
Modified: 2012-08-01 13:07 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Essential patch to disable texture formats that are reported unrenderable elsewhere in driver. (3.01 KB, patch)
2012-07-02 10:25 UTC, Eugene St Leger
Details | Splinter Review
Essential patch to allow 2048 pixel blits. (535 bytes, patch)
2012-07-02 10:29 UTC, Eugene St Leger
Details | Splinter Review
Essential patch to reapply dirtied texenv registers. (918 bytes, patch)
2012-07-02 10:34 UTC, Eugene St Leger
Details | Splinter Review
Unessential fixes/enhancements patch. (7.22 KB, patch)
2012-07-02 10:37 UTC, Eugene St Leger
Details | Splinter Review
Proposed blit register dirtying patch. (1.98 KB, patch)
2012-07-02 10:40 UTC, Eugene St Leger
Details | Splinter Review
Optional patch to warn (once) when a blit with 2048 pixel dimension occurs on r200. (623 bytes, patch)
2012-07-02 10:45 UTC, Eugene St Leger
Details | Splinter Review
Untested but probably essential patch to allow 2048 pixel blits on r100. (553 bytes, patch)
2012-07-02 10:49 UTC, Eugene St Leger
Details | Splinter Review
Optional untested patch to warn (once) when a blit with 2048 pixel dimension occurs on r100. (639 bytes, patch)
2012-07-02 10:51 UTC, Eugene St Leger
Details | Splinter Review

Description Eugene St Leger 2012-07-02 10:25:38 UTC
Created attachment 63711 [details] [review]
Essential patch to disable texture formats that are reported unrenderable elsewhere in driver.

Without r200-tex-format-fix.patch, gnome shell crashes. Texture formats are selected by radeonChoose8888TexFormat but reported as unrenderable by radeonIsFormatRenderable.
Comment 1 Eugene St Leger 2012-07-02 10:29:44 UTC
Created attachment 63712 [details] [review]
Essential patch to allow 2048 pixel blits.

Without this patch, gnome shell crashes. 2048 pixel blits are used.
Comment 2 Eugene St Leger 2012-07-02 10:34:41 UTC
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.
Comment 3 Eugene St Leger 2012-07-02 10:37:47 UTC
Created attachment 63714 [details] [review]
Unessential fixes/enhancements patch.

Minor spelling fixes & enhancements.
Comment 4 Eugene St Leger 2012-07-02 10:40:51 UTC
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.
Comment 5 Eugene St Leger 2012-07-02 10:45:01 UTC
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.
Comment 6 Eugene St Leger 2012-07-02 10:49:07 UTC
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.
Comment 7 Eugene St Leger 2012-07-02 10:51:10 UTC
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.
Comment 8 Eugene St Leger 2012-07-02 11:04:54 UTC
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."
Comment 9 Roland Scheidegger 2012-07-03 05:20:47 UTC
(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.
Comment 10 Roland Scheidegger 2012-08-01 13:07:09 UTC
With 5b88a2a22daae4d09596804d8edc6b8796d05150 attachment 63712 [details] [review], 63716, 63717, 63718 are obsolete. Still unsure what to do with the others.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.