Re: [patch 06/16] Fix matroxfb on big-endian hardware

Petr Vandrovec
2005-05-24T10:22:01Z

Chris Wright wrote:
> * Al Viro (viro@parcelfarce.linux.theplanet.co.uk) wrote:
>
>>On Mon, May 23, 2005 at 04:22:07PM -0700, Chris Wright wrote:
>>
>>>- mga_writel(mmio, 0, *chardata);
>>>+#if defined(__BIG_ENDIAN)
>>>+ fb_writel((*chardata) << 24, mmio.vaddr);
>>>+#else
>>>+ fb_writel(*chardata, mmio.vaddr);
>>>+#endif
>>
>>So basically you are passing it cpu_to_le32(*chardata)?
>>
>>
>>>+#if defined(__BIG_ENDIAN)
>>>+ fb_writel((*(u_int16_t*)chardata) << 16, mmio.vaddr);
>>>+#else
>>>+ fb_writel(*(u_int16_t*)chardata, mmio.vaddr);
>>>+#endif
>>
>>*yuck*
>>
>>cpu_to_le32(le16_to_cpu(*(__le16 *)chardata)? Is that what you are doing
>>here?

Yes. Hardware wants it this way. For 8bit wide font you must write font data
in low 8 bits (some hardware on the way does swapping on BE archs), and for
16bit wide font you must write font data in low 16 bits. In both cases first
pixel is in bit7 of byte 0, going through to bit0 of byte 0, followed by bit7 of
byte 1 through bit0 of byte 1. And so on for widths > 16. Inner leX_to_cpu
works on data of font size, while outer cpu_to_le32 works on accelerator data
size, which is always 32 bit.

If you want it absolutely correct (as font data are in big endian), you should
write cpu_to_le32(swab<font_width>(be<font_width>_to_cpup(chardata))). Inner
be16_to_cpup retrieves font data into bits 15 -> 0, swab reorders bytes so first
pixel is in bit 7, not bit 15 (or 31 for 32bit wide font), and outer cpu_to_le32
nullifies effect of external swab32() engine.

> Petr, care to comment? Best I can tell this is from you and is already
> upstream. Any reason not to use cpu_to_xx instead of what's done?

I'm not sure about speed effects. Is gcc smart enough to notice that two
different width byteswaps can be combined to simple shift?
Thanks,
Petr Vandrovec