[PATCH] xv: fix non-kms/non-dri Xv column ordering on big endian systems

[Michel Dänzer]

On Fre, 2010-08-27 at 09:58 +0300, Heikki Lindholm wrote: 
> 27.8.2010 9.38, Michel Dänzer kirjoitti:
> > On Mon, 2010-08-23 at 09:52 +0300, Heikki Lindholm wrote:
> >> Column order is wrong on big endian systems, primarly because of a
> >> bits / bytes mix up with the bpp variable. Fix tested with r100 and
> >> r300, screen depth 16 and 32 with YV12 and YUY2 (overlay, textured video),
> >> RGBA and RGBT (overlay).
> >>
> >> Should fix: https://bugs.freedesktop.org/show_bug.cgi?id=29041
> >>
> >> Signed-off-by: Heikki Lindholm<holin at iki.fi>
> >> ---
> >>   src/radeon_video.c |   12 ++++++++----
> >>   1 files changed, 8 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/src/radeon_video.c b/src/radeon_video.c
> >> index dc75279..1a42951 100644
> >> --- a/src/radeon_video.c
> >> +++ b/src/radeon_video.c
> >> @@ -2216,11 +2216,15 @@ RADEONCopyData(
> >>   		swap = RADEON_HOST_DATA_SWAP_32BIT;
> >>   		break;
> >>   	    }
> >> -	} else if (bpp != pScrn->bitsPerPixel) {
> >> -	    if (bpp == 8)
> >> +	} else {
> >> +	    switch (pScrn->bitsPerPixel) {
> >> +	    case 16:
> >> +		swap = RADEON_HOST_DATA_SWAP_16BIT;
> >> +		break;
> >> +	    case 32:
> >>   		swap = RADEON_HOST_DATA_SWAP_32BIT;
> >> -	    else
> >> -		swap = RADEON_HOST_DATA_SWAP_HDW;
> >> +		break;
> >> +	    }
> >>   	}
> >>   #endif
> >
> > I'm also not sure why this path shouldn't need to take bpp into account,
> > at least in addition to pScrn->bitsPerPixel. Did you make sure you
> > tested this function being called with bpp=1,2,4 in both depth 24 and
> > 16?

Once again, have you tested in depth 16 as well as 24?


> It seemed a bit odd to me as well, but yes, all those cases were tested: 
> YV12: bpp=1; YUY2/RGBT: bpp=2; RGBA: bpp=4. However, bpp=2 gets 
> transformed into bpp=1 at the top of the function (and doesn't the fact 
> that that works (in the dri case), kind of indicate that it's 
> unnecessary to look at bpp?).

No, at least the bpp=4 case is still different.

> I know of no real application that uses the RGB modes though, [...]

How did you test it? I think some emulators (can) use it.


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer