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

[Heikki Lindholm]

27.8.2010 7.46, Alex Deucher kirjoitti:
> On Mon, Aug 23, 2010 at 2:52 AM, Heikki Lindholm<holin at iki.fi>  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) {
>
> don't we want to check the bpp rather then the scrn depth?

I would have to guess no, because it just didn't seem to work that way. 
The cases I listed as tested behaved (with the patch applied) identical 
to what I saw on x86 with M7, whereas using bpp would quite definitely 
look different, for example, in the bpp = 4 (RGBA), depth = 16 case. 
Maybe there's byteswapping going on elsewhere as well?

Btw. at least the YUV cases used to work with the 1.6.x driver versions. 
I wouldn't be surprised if the RGB overlays were never really tested on 
big endian at all.

> Wouldn't something like this work and be cleaner?
>
> diff --git a/src/radeon_video.c b/src/radeon_video.c
> index dc75279..df0faf8 100644
> --- a/src/radeon_video.c
> +++ b/src/radeon_video.c
> @@ -2207,20 +2207,13 @@ RADEONCopyData(
>          int swap = RADEON_HOST_DATA_SWAP_NONE;
>
>   #if X_BYTE_ORDER == X_BIG_ENDIAN
> -       if (info->kms_enabled) {
> -           switch(bpp) {
> -           case 2:
> -               swap = RADEON_HOST_DATA_SWAP_16BIT;
> -               break;
> -           case 4:
> -               swap = RADEON_HOST_DATA_SWAP_32BIT;
> -               break;
> -           }
> -       } else if (bpp != pScrn->bitsPerPixel) {
> -           if (bpp == 8)
> -               swap = RADEON_HOST_DATA_SWAP_32BIT;
> -           else
> -               swap = RADEON_HOST_DATA_SWAP_HDW;
> +       switch(bpp) {
> +       case 2:
> +         swap = RADEON_HOST_DATA_SWAP_16BIT;
> +         break;
> +       case 4:
> +         swap = RADEON_HOST_DATA_SWAP_32BIT;
> +         break;
>          }
>   #endif

See above, and the bpp == 2 case is never hit, if you look at the very 
top of the function. Can't test the kms path myself, because kms enabled 
just hangs the gpu here.

-- Heikki Lindholm