[PATCH 1/9] dri2: Fix can_exchange() to allow page-flipping on new servers.

[Mario Kleiner]

On 02/20/2012 11:27 AM, Michel Dänzer wrote:
> On Mon, 2012-02-20 at 05:59 +0100, Mario Kleiner wrote:
>> On 02/16/2012 11:04 AM, Michel Dänzer wrote:
>>> On Don, 2012-02-16 at 00:45 +0100, Mario Kleiner wrote:
>>>> can_exchange() fails on at least Xorg 1.12+. This fixes
>>>> it in the same way it was fixed in the ati&   intel ddx.
>>>>
>>>> Signed-off-by: Mario Kleiner<mario.kleiner at tuebingen.mpg.de>
>>>> ---
>>>>    src/nouveau_dri2.c |    2 +-
>>>>    1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
>>>> index 3aa5ec5..5b62425 100644
>>>> --- a/src/nouveau_dri2.c
>>>> +++ b/src/nouveau_dri2.c
>>>> @@ -160,7 +160,7 @@ can_exchange(DrawablePtr draw, PixmapPtr dst_pix, PixmapPtr src_pix)
>>>>    	return ((DRI2CanFlip(draw)&&   pNv->has_pageflip))&&
>>>>    		dst_pix->drawable.width == src_pix->drawable.width&&
>>>>    		dst_pix->drawable.height == src_pix->drawable.height&&
>>>> -		dst_pix->drawable.depth == src_pix->drawable.depth&&
>>>> +		dst_pix->drawable.bitsPerPixel == src_pix->drawable.bitsPerPixel&&
>>>>    		dst_pix->devKind == src_pix->devKind;
>>>>    }
>>>>
>>>
>>> Actually, it seems like the pixmap depths really should match, otherwise
>>> one could end up with the front pixmap depth not matching the window
>>> depth. Not sure that's a real problem right now, but it seems wonky at
>>> least...
>>>
>>> Have you investigated why the depths don't match?
>>
>> Depends on the meaning of "investigated": One of the pixmaps has depth
>> 24 bits (the pixmap of the root window) the other 32 bits (as requested
>> from the client via DRI2GetBuffersWithFormat for RGBA8 visuals). Both
>> have 32 bpp. I checked what the intel and ati ddx do. The ati ddx always
>> checked for matching drawable.bitsPerPixel since kms pageflip support
>> was implemented. The intel ddx does the same, but the code and comments
>> suggests they tried both and checking for matching depths probably
>> didn't work:
>>
>> cd xorg/drivers/xf86-video-intel/
>> git log -p e2615cdeef078dbd2e834b68c437f098a92b941d
>>
>> So everybody does it like this currently, and it seems to work.
>
> Right. I must have been incorrectly thinking of flipping as changing the
> window pixmap.
>
> I still have some doubts — e.g. why is an RGBA visual chosen for a
> fullscreen window, and does this really have anything to do with changes
> in xserver 1.12, or rather in Mesa — but I think the change itself is
> okay.
>

You are right, it is not the xserver, but more likely Mesa. I tried the 
patch again with some debug output against xserver 1.10.4 (Ubuntu 11.10 
standard x-server) and the same happens. The FRONT_LEFT buffer is taken 
from the windows pixmap, which is depth 24, the BACK_LEFT buffer is 
allocated as a pixmap of 32 bits. So now i'm surprised it ever worked 
before...

Original testing of this nouveau patchset was around August 2011, since 
then i've also updated mesa to 7.11 as part of a distribution upgrade 
from Ubuntu 11.04 -> 11.10.

Mesa requests 32 bit buffers as part of the BACK_LEFT attachment in 
DRI2GetBuffersWithFormat(). As far as i could trace it back, the code in 
src/gallium/state_trackers/dri/drm/dri2.c, dri2_drawable_get_buffers() 
mapped the PIPE_FORMAT_B8G8R8A8_UNORM (as requested by my toolkit) and 
also PIPE_FORMAT_B8G8R8X8_UNORM (from glxgears) to a DRI2 buffer format 
of 32 bits and it seems that that is used as .depth for the pixmaps 
drawable as well.

There's this related commit in mesa:

git log -p 576161289df68eedade591fbca4013329c9e5ded

which changed the logic a bit after Mesa 7.12. Also this one:

git log -p c72d7df16879e3210946ba92a7edc823815b6f16

So to support page flipping on different mesa versions i guess 
can_exchange needs to be lenient and check for the matching bitsPerPixel 
instead.

I will change the commit message and title of this patch to be less 
misleading about the cause of the problem.

I wonder if the depth field is still a good way to describe the buffer 
format, especially with things like depth 30 RGB10 scanout.

-mario