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

[Petr Vandrovec <vandrove@vc.cvut.cz>]

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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/