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

Heikki Lindholm holindho at saunalahti.fi
Fri Aug 27 07:56:20 PDT 2010


27.8.2010 10.45, Michel Dänzer kirjoitti:
> 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.

D'oh. Some of the RGB modes are actually native endian. I'll have to fix 
those. Shall we have it on top of the already committed patch or revert 
that and do a new one?

-- Heikki Lindholm


More information about the xorg-driver-ati mailing list