Bug 788

<MrCooper> sth_: so, can you try rounding up the depth pitch to a multiple of 
32 pixels? (or maybe using a multiple of 32 horizontal width for a start) 
<MrCooper> sth_: I mean virtual width of course 
<sth_> MrCooper: Setting the XServer virtual resolution to something like 
1696? 
<MrCooper> sth_: exactly 
 
I added Virtual 1696 1050 to my XF86Config-4, restarted X11 and the problem 
disappeared. 
 

Created attachment 402 [details] [review]
Fix

I committed this patch to DRI CVS to fix the problem. Thanks for the report.

Fixed in DRI CVS.

we should probably apply this to xorg cvs as well...

Reopening this, because it says "we should probably apply to xorg
CVS also" but no indication of wether that ever happened or not.

Comment on attachment 402 [details] [review]
Fix

Adding to 6.8.x approval request queue.

Comment on attachment 402 [details] [review]
Fix

DRI CVS was merged into X.org CVS a long time ago... this fix is on the 6.8
branch, as a little investigation would have shown.

However, it looks like this case might have been overlooked when support for
colour tiling was added on HEAD, Roland?

(In reply to comment #7)
> 
> However, it looks like this case might have been overlooked when support for
> colour tiling was added on HEAD, Roland?

Oops, this might be ambiguous... I mean Roland Scheidegger.

I removed that since for color tiling displayWidth must be a multiple of 256
bytes (i.e. 64 pixels minimum at 32bpp, 128 pixels for 16bpp). Then I discovered
even without color tiling the requested pitchInc for mode validation is 64 *
pScrn->bitsPerPixel, i.e. 64 pixels too, so the additional check for z tiling
alignment is unnecessary. At least I think that is what this code does...

(In reply to comment #9)
> Then I discovered even without color tiling the requested pitchInc for mode
> validation is 64 * pScrn->bitsPerPixel, i.e. 64 pixels too, so the additional
> check for z tiling alignment is unnecessary.

That's only true for the cases when xf86ValidateModes() is used AFAICT.

Are not all modes validated? If not at least color tiling would not work in
these cases. Or does mode validation not happen if you use fbdev? In that case
the check needs to be added back.

(In reply to comment #11)
> Are not all modes validated?

They are, but the radeon driver doesn't use xf86ValidateModes() in all cases but
has some of its own validation functions. Even when xf86ValidateModes() is used,
I'm not sure how it interacts with virtual resolutions e.g.? Have you tried
1680x1050 virtual resolution?

(In reply to comment #12)
> (In reply to comment #11)
> > Are not all modes validated?
> 
> They are, but the radeon driver doesn't use xf86ValidateModes() in all cases but
> has some of its own validation functions.
I think they ALL call xf86ValidateModes though.

> Even when xf86ValidateModes() is used,
> I'm not sure how it interacts with virtual resolutions e.g.?
I believe that actually _only_ virtual resolutions are validated.

> Have you tried 1680x1050 virtual resolution?
I have tried virtual resolutions slightly larger than 1280x1024 (e.g. 1288x1024)
 they all result in a pitch of 1344, without problems.
However, I have also tried the same with UseFBDev, the indicated pitch was still
1344, but the results inconclusive to that specific problem, since the screen in
2d was a garbled mess anyway. All virtual resolutions which do not have a pitch
of 64 pixels simply do not work, maybe the modes used by fbdev and the modes
which xorg thinks get used (as per mode validation) are not quite the same or
something like that.

(> > They are, but the radeon driver doesn't use xf86ValidateModes() in all
cases but
> > has some of its own validation functions.
> I think they ALL call xf86ValidateModes though.
Argh, that's apparently not true. This is sick, there are two dozens different
validation routines, and they don't validate all the same things.
So, I'll add that check back.
However, these modes which are not going through the xf86ValidateModes validator
and are thus not tested for a "valid" pitchInc are not going to work with color
tiling. Since the pitchInc is 64 pixels even without color tiling, I'm wondering
why those work at all?

Ignoring tiling, the strictest restriction for the pitch I can find in the specs
is that it has to be a multiple of 64 bytes (I think even multiples of 32 should
work, but the dedicated pitch registers (as opposed to the combined offset/pitch
ones) probably need to be used for that). Where do the 64 pixels come from?

(In reply to comment #15)
> Ignoring tiling, the strictest restriction for the pitch I can find in the specs
> is that it has to be a multiple of 64 bytes (I think even multiples of 32 should
> work, but the dedicated pitch registers (as opposed to the combined offset/pitch
> ones) probably need to be used for that). Where do the 64 pixels come from?
That's a very good question. No idea. Those validatemode functions which do not
call xf86ValidateModes in fact only request a pitch of 64bytes (for 32bit and
16bit) and 128bytes (for 8bit modes). And in fact, I _did_ adjust this
RadeonSetPitch function to work correctly with color tiling, so this ought to
work (phew!). This only leaves the original problem (e.g. without color tiling,
depth pitch must be adjusted). I'll fix that in a minute. Maybe the requested
pitchInc for xf86ValidateModes is indeed more restrictive than necessary, but
I'm certainly not going to touch that...

fixed in xorg cvs head.