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

Heikki Lindholm holindho at saunalahti.fi
Thu Aug 26 23:58:27 PDT 2010


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?

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?). I know of no real application that uses 
the RGB modes though, so, in the code segment in question, bpp = 1 case 
is the only one that matters in the real world.

-- Heikki Lindholm


More information about the xorg-driver-ati mailing list