fb_setcolreg(), CNVT_TOHW() and chan_to_field()

fb_setcolreg(), CNVT_TOHW() and chan_to_field()

[Rabin Vincent]

I'm trying to figure out the best way to implement fb_setcolreg() for
FB_VISUAL_TRUECOLOR.

14 or so drivers copy/paste the CNVT_TOHW from skeletonfb.c:

 #define CNVT_TOHW(val,width) ((((val)<<(width))+0x7FFF-(val))>>16)
 ...
 red = CNVT_TOHW(red, info->var.red.length);

Another 18 or so drivers copy/paste a chan_to_field() function which has
a straightforward shift:

 static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
 {
 	chan &= 0xffff;
 	chan >>= 16 - bf->length;
 	return chan << bf->offset;
 }
 ...
 val = chan_to_field(red, &info->var.red);

Both methods return similar values, except that with CNVT_TOHW() the
extreme values seem to have a smaller range than the other values, while
with the chan_to_field() shift all the values have an equal range.

What's the reason behind choosing one over the other?  Could/should a
common function be provided for all the drivers to use?

Re: fb_setcolreg(), CNVT_TOHW() and chan_to_field()

[Geert Uytterhoeven]

On Tue, Oct 25, 2011 at 09:34, Rabin Vincent <rabin@rab.in> wrote:
> I'm trying to figure out the best way to implement fb_setcolreg() for
> FB_VISUAL_TRUECOLOR.

FB_VISUAL_TRUECOLOR implies the colormap is read-only.
Perhaps you meant FB_VISUAL_DIRECTCOLOR?

> 14 or so drivers copy/paste the CNVT_TOHW from skeletonfb.c:
>
>  #define CNVT_TOHW(val,width) ((((val)<<(width))+0x7FFF-(val))>>16)
>  ...
>  red = CNVT_TOHW(red, info->var.red.length);
>
> Another 18 or so drivers copy/paste a chan_to_field() function which has
> a straightforward shift:
>
>  static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
>  {
>        chan &= 0xffff;
>        chan >>= 16 - bf->length;
>        return chan << bf->offset;
>  }
>  ...
>  val = chan_to_field(red, &info->var.red);
>
> Both methods return similar values, except that with CNVT_TOHW() the
> extreme values seem to have a smaller range than the other values, while
> with the chan_to_field() shift all the values have an equal range.
>
> What's the reason behind choosing one over the other?  Could/should a
> common function be provided for all the drivers to use?

The idea is to use the full dynamic range of the 16-bit values. I.e. 'all ones'
in the hardware-specific value should map to 'all ones' in the 16-bit expanded
value. So the macros are preferred.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: fb_setcolreg(), CNVT_TOHW() and chan_to_field()

[Rabin Vincent]

On Tue, Oct 25, 2011 at 01:15:41PM +0200, Geert Uytterhoeven wrote:
> On Tue, Oct 25, 2011 at 09:34, Rabin Vincent <rabin@rab.in> wrote:
> > I'm trying to figure out the best way to implement fb_setcolreg() for
> > FB_VISUAL_TRUECOLOR.
> 
> FB_VISUAL_TRUECOLOR implies the colormap is read-only.
> Perhaps you meant FB_VISUAL_DIRECTCOLOR?

I was referring to the pseudo palette usage for FB_VISUAL_TRUECOLOR, which is
where most of the drivers use chan_to_field(), for example in s3c2410fb.c:

	switch (info->fix.visual) {
	case FB_VISUAL_TRUECOLOR:
		/* true-colour, use pseudo-palette */

		if (regno < 16) {
			u32 *pal = info->pseudo_palette;

			val  = chan_to_field(red,   &info->var.red);

> 
> > 14 or so drivers copy/paste the CNVT_TOHW from skeletonfb.c:
> >
> >  #define CNVT_TOHW(val,width) ((((val)<<(width))+0x7FFF-(val))>>16)
> >  ...
> >  red = CNVT_TOHW(red, info->var.red.length);
> >
> > Another 18 or so drivers copy/paste a chan_to_field() function which has
> > a straightforward shift:
> >
> >  static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
> >  {
> >        chan &= 0xffff;
> >        chan >>= 16 - bf->length;
> >        return chan << bf->offset;
> >  }
> >  ...
> >  val = chan_to_field(red, &info->var.red);
> >
> > Both methods return similar values, except that with CNVT_TOHW() the
> > extreme values seem to have a smaller range than the other values, while
> > with the chan_to_field() shift all the values have an equal range.
> >
> > What's the reason behind choosing one over the other?  Could/should a
> > common function be provided for all the drivers to use?
> 
> The idea is to use the full dynamic range of the 16-bit values. I.e. 'all ones'
> in the hardware-specific value should map to 'all ones' in the 16-bit expanded
> value. So the macros are preferred.

I'm not sure I follow.  In the pseudo palette, where the bf->length is less
than 16, both CNVT_TOHW() and the plain shift return the same values for the
all-ones case.