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

[Michel Dänzer]

On Fre, 2010-08-27 at 10:29 +0300, Heikki Lindholm wrote: 
> 27.8.2010 10.07, Michel Dänzer kirjoitti:
> > 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?
> 
> Yes, like I said in my orig. mail. All listed bpps tested with depth 16 
> and 24, and some with depth 15 even.

Sorry I missed / forgot about that, thanks for confirming.


> >> I know of no real application that uses the RGB modes though, [...]
> >
> > How did you test it? I think some emulators (can) use it.
> 
> Modified the testxv (writes its image bytewise, so it shouldn't have 
> endian problems)

Depends how XVideo defines these formats. E.g. snes9x has an option to
byte-swap the XVideo data, so there may have been some uncertainty or at
least confusion about that.


> program from wikipedia Xvideo page a bit and tested results on both
> x86 and ppc. I can mail you the test cases if you're suspicious about
> them ;)

I don't particularly care about that path anymore, just wanted to make
sure your testing covered all cases, but it appears that way.


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