| Message ID | a35154278816426fee7045795aa4894ff615efdf.1657302034.git.geert@linux-m68k.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for low-color frame buffer formats | expand |
Hi Geert, On Fri, Jul 08, 2022 at 08:21:31PM +0200, Geert Uytterhoeven wrote: > Fill in the LSB when converting color components from 8-bit to 16-bit. > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > --- > v2: > - New. > --- > tests/util/pattern.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tests/util/pattern.c b/tests/util/pattern.c > index 178aee8341a38920..3753ebc1eeae6c9a 100644 > --- a/tests/util/pattern.c > +++ b/tests/util/pattern.c > @@ -646,9 +646,9 @@ void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut) > memset(lut, 0, size * sizeof(struct drm_color_lut)); > > #define FILL_COLOR(idx, r, g, b) \ > - lut[idx].red = (r) << 8; \ > - lut[idx].green = (g) << 8; \ > - lut[idx].blue = (b) << 8 > + lut[idx].red = (r) * 0x101; \ (lut[idx].red = (r) << 8) | 1; had IMO been easier to read. Patch is: Acked-by: Sam Ravnborg <sam@ravnborg.org> for both ways to do it. > + lut[idx].green = (g) * 0x101; \ > + lut[idx].blue = (b) * 0x101 > > FILL_COLOR( 0, 192, 192, 192); /* grey */ > FILL_COLOR( 1, 192, 192, 0 ); /* yellow */ > -- > 2.25.1
Hi Sam, On Sun, Jul 10, 2022 at 12:31 PM Sam Ravnborg <sam@ravnborg.org> wrote: > On Fri, Jul 08, 2022 at 08:21:31PM +0200, Geert Uytterhoeven wrote: > > Fill in the LSB when converting color components from 8-bit to 16-bit. > > > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > > --- > > v2: > > - New. > > --- > > tests/util/pattern.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/tests/util/pattern.c b/tests/util/pattern.c > > index 178aee8341a38920..3753ebc1eeae6c9a 100644 > > --- a/tests/util/pattern.c > > +++ b/tests/util/pattern.c > > @@ -646,9 +646,9 @@ void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut) > > memset(lut, 0, size * sizeof(struct drm_color_lut)); > > > > #define FILL_COLOR(idx, r, g, b) \ > > - lut[idx].red = (r) << 8; \ > > - lut[idx].green = (g) << 8; \ > > - lut[idx].blue = (b) << 8 > > + lut[idx].red = (r) * 0x101; \ > > (lut[idx].red = (r) << 8) | 1; > > had IMO been easier to read. I guess you mean "| (r)" instead of "| 1"? > > Patch is: > Acked-by: Sam Ravnborg <sam@ravnborg.org> > > for both ways to do it. Thanks! 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
Hi Geert, On Sun, Jul 10, 2022 at 01:04:23PM +0200, Geert Uytterhoeven wrote: > Hi Sam, > > On Sun, Jul 10, 2022 at 12:31 PM Sam Ravnborg <sam@ravnborg.org> wrote: > > On Fri, Jul 08, 2022 at 08:21:31PM +0200, Geert Uytterhoeven wrote: > > > Fill in the LSB when converting color components from 8-bit to 16-bit. > > > > > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > > > --- > > > v2: > > > - New. > > > --- > > > tests/util/pattern.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/tests/util/pattern.c b/tests/util/pattern.c > > > index 178aee8341a38920..3753ebc1eeae6c9a 100644 > > > --- a/tests/util/pattern.c > > > +++ b/tests/util/pattern.c > > > @@ -646,9 +646,9 @@ void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut) > > > memset(lut, 0, size * sizeof(struct drm_color_lut)); > > > > > > #define FILL_COLOR(idx, r, g, b) \ > > > - lut[idx].red = (r) << 8; \ > > > - lut[idx].green = (g) << 8; \ > > > - lut[idx].blue = (b) << 8 > > > + lut[idx].red = (r) * 0x101; \ > > > > (lut[idx].red = (r) << 8) | 1; > > > > had IMO been easier to read. > > I guess you mean "| (r)" instead of "| 1"? Well, I meant what I wrote but it is obviously wrong. So yes lut[idx].red = (r) << 8 | (r); is the equivalent of multiplying with 0x101. Whatever works, but if you update this patch, then please update the later patch where the multiply with 0x101 is also used. Sam
Hi Sam, > On Sun, Jul 10, 2022 at 01:04:23PM +0200, Geert Uytterhoeven wrote: > > On Sun, Jul 10, 2022 at 12:31 PM Sam Ravnborg <sam@ravnborg.org> wrote: > > > On Fri, Jul 08, 2022 at 08:21:31PM +0200, Geert Uytterhoeven wrote: > > > > Fill in the LSB when converting color components from 8-bit to 16-bit. > > > > > > > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > > > > --- a/tests/util/pattern.c > > > > +++ b/tests/util/pattern.c > > > > @@ -646,9 +646,9 @@ void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut) > > > > memset(lut, 0, size * sizeof(struct drm_color_lut)); > > > > > > > > #define FILL_COLOR(idx, r, g, b) \ > > > > - lut[idx].red = (r) << 8; \ > > > > - lut[idx].green = (g) << 8; \ > > > > - lut[idx].blue = (b) << 8 > > > > + lut[idx].red = (r) * 0x101; \ > > > > > > (lut[idx].red = (r) << 8) | 1; > > > > > > had IMO been easier to read. > > > > I guess you mean "| (r)" instead of "| 1"? > > Well, I meant what I wrote but it is obviously wrong. ;-) > So yes > lut[idx].red = (r) << 8 | (r); > > is the equivalent of multiplying with 0x101. > > Whatever works, but if you update this patch, then please update the > later patch where the multiply with 0x101 is also used. The advantage of the multiplication is that the macro's parameters are used only once, hence no temporaries are needed to keep them safe. 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
diff --git a/tests/util/pattern.c b/tests/util/pattern.c index 178aee8341a38920..3753ebc1eeae6c9a 100644 --- a/tests/util/pattern.c +++ b/tests/util/pattern.c @@ -646,9 +646,9 @@ void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut) memset(lut, 0, size * sizeof(struct drm_color_lut)); #define FILL_COLOR(idx, r, g, b) \ - lut[idx].red = (r) << 8; \ - lut[idx].green = (g) << 8; \ - lut[idx].blue = (b) << 8 + lut[idx].red = (r) * 0x101; \ + lut[idx].green = (g) * 0x101; \ + lut[idx].blue = (b) * 0x101 FILL_COLOR( 0, 192, 192, 192); /* grey */ FILL_COLOR( 1, 192, 192, 0 ); /* yellow */
Fill in the LSB when converting color components from 8-bit to 16-bit. Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> --- v2: - New. --- tests/util/pattern.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)