Re: [PATCH] drm/fb-helper: Redo fb format<->fb_bitfield mapping

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Feb 07, 2018 at 05:08:30PM +0000, Eric Engestrom wrote:
> On Wednesday, 2018-02-07 17:24:45 +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Replace the switch statements that try to map between the fb format and
> > the fb_bitfield infromation with a single table.
> > 
> > Note that this changes the behaviour of drm_fb_helper_check_var() to
> > return an error of the requested fb_bitfields don't match the actual
> > pixel format. Previously we just sort of semi-trusted some of the
> > bpp/depth information the user was asking for, and never actually
> > checked if that matches the fb pixel format.
> > 
> > This prepares us to use all different rgb format channel layouts.
> > Basically would just need some decent way for the driver/cmdline
> > to select the one you want.
> > 
> > I didn't think about endianness here at all. Not sure how fbdev is
> > supposed to deal with that stuff anyway, and I don't think we ever
> > reached a real concensus on the drm fourcc endianness either. So
> > I'll just pretend everything is little endian and ignore everything
> > else.
> > 
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Noralf Trønnes <noralf@tronnes.org>
> > Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c | 280 +++++++++++++++++++++++-----------------
> >  1 file changed, 163 insertions(+), 117 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index 035784ddd133..0c906e3a5bb1 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -40,6 +40,7 @@
> >  #include <drm/drm_crtc_helper.h>
> >  #include <drm/drm_atomic.h>
> >  #include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_fourcc.h>
> >  
> >  #include "drm_crtc_internal.h"
> >  #include "drm_crtc_helper_internal.h"
> > @@ -1561,6 +1562,147 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
> >  }
> >  EXPORT_SYMBOL(drm_fb_helper_ioctl);
> >  
> > +#define FB_FORMAT_C(c) { \
> > +	.format = DRM_FORMAT_C##c,    \
> > +	.blue   = { .length = (c), }, \
> > +	.green  = { .length = (c), }, \
> > +	.red    = { .length = (c), }, \
> > +}
> > +#define FB_FORMAT_RGB(r, g, b) { \
> > +	.format = DRM_FORMAT_RGB##r##g##b, \
> > +	.blue   = { .length = (b), .offset = 0, }, \
> > +	.green  = { .length = (g), .offset = (b), }, \
> > +	.red    = { .length = (r), .offset = (b)+(g), }, \
> > +}
> > +#define FB_FORMAT_BGR(b, g, r) { \
> > +	.format = DRM_FORMAT_BGR##b##g##r, \
> > +	.red    = { .length = (r), .offset = 0, }, \
> > +	.green  = { .length = (g), .offset = (r), }, \
> > +	.blue   = { .length = (b), .offset = (r)+(g), }, \
> > +}
> > +#define FB_FORMAT_XRGB(x, r, g, b) { \
> > +	.format = DRM_FORMAT_XRGB##x##r##g##b, \
> > +	.blue   = { .length = (b), .offset = 0, }, \
> > +	.green  = { .length = (g), .offset = (b), }, \
> > +	.red    = { .length = (r), .offset = (b)+(g), }, \
> > +}
> > +#define FB_FORMAT_XBGR(x, b, g, r) { \
> > +	.format = DRM_FORMAT_XBGR##x##b##g##r, \
> > +	.red    = { .length = (r), .offset = 0, }, \
> > +	.green  = { .length = (g), .offset = (r), }, \
> > +	.blue   = { .length = (b), .offset = (r)+(g), }, \
> > +}
> > +#define FB_FORMAT_RGBX(r, g, b, x) { \
> > +	.format = DRM_FORMAT_RGBX##r##g##b##x, \
> > +	.blue   = { .length = (b), .offset = (x), }, \
> > +	.green  = { .length = (g), .offset = (x)+(b), }, \
> > +	.red    = { .length = (r), .offset = (x)+(b)+(g), }, \
> > +}
> > +#define FB_FORMAT_BGRX(b, g, r, x) { \
> > +	.format = DRM_FORMAT_BGRX##b##g##r##x, \
> > +	.red    = { .length = (r), .offset = (x), }, \
> > +	.green  = { .length = (g), .offset = (x)+(r), }, \
> > +	.blue   = { .length = (b), .offset = (x)+(r)+(g), }, \
> > +}
> > +#define FB_FORMAT_ARGB(a, r, g, b) { \
> > +	.format = DRM_FORMAT_ARGB##a##r##g##b, \
> > +	.blue   = { .length = (b), .offset = 0, }, \
> > +	.green  = { .length = (g), .offset = (b), }, \
> > +	.red    = { .length = (r), .offset = (b)+(g), }, \
> > +	.transp = { .length = (a), .offset = (b)+(g)+(r), }, \
> > +}
> > +#define FB_FORMAT_ABGR(a, b, g, r) { \
> > +	.format = DRM_FORMAT_ABGR##a##b##g##r, \
> > +	.red    = { .length = (r), .offset = 0, }, \
> > +	.green  = { .length = (g), .offset = (r), }, \
> > +	.blue   = { .length = (b), .offset = (r)+(g), },\
> > +	.transp = { .length = (a), .offset = (r)+(g)+(b), }, \
> > +}
> > +#define FB_FORMAT_RGBA(r, g, b, a) { \
> > +	.format = DRM_FORMAT_RGBA##r##g##b##a, \
> > +	.transp = { .length = (a), .offset = 0, }, \
> > +	.blue   = { .length = (b), .offset = (a), }, \
> > +	.green  = { .length = (g), .offset = (a)+(b), }, \
> > +	.red    = { .length = (r), .offset = (a)+(b)+(g), }, \
> > +}
> > +#define FB_FORMAT_BGRA(b, g, r, a) { \
> > +	.format = DRM_FORMAT_BGRA##b##g##r##a, \
> > +	.transp = { .length = (a), .offset = 0, }, \
> > +	.red    = { .length = (r), .offset = (a), }, \
> > +	.green  = { .length = (g), .offset = (a)+(r), }, \
> > +	.blue   = { .length = (b), .offset = (a)+(r)+(g), }, \
> > +}
> > +
> > +struct drm_fb_helper_format {
> > +	u32 format;
> > +	struct fb_bitfield red, green, blue, transp;
> > +};
> > +
> > +static const struct drm_fb_helper_format fb_formats[] = {
> > +	FB_FORMAT_C(8),
> > +
> > +	FB_FORMAT_XRGB(1, 5, 5, 5),
> > +	FB_FORMAT_XBGR(1, 5, 5, 5),
> > +	FB_FORMAT_RGBX(5, 5, 5, 1),
> > +	FB_FORMAT_BGRX(5, 5, 5, 1),
> > +
> > +	FB_FORMAT_ARGB(1, 5, 5, 5),
> > +	FB_FORMAT_ABGR(1, 5, 5, 5),
> > +	FB_FORMAT_RGBA(5, 5, 5, 1),
> > +	FB_FORMAT_BGRA(5, 5, 5, 1),
> > +
> > +	FB_FORMAT_RGB(5, 6, 5),
> > +	FB_FORMAT_BGR(5, 6, 5),
> > +
> > +	FB_FORMAT_RGB(8, 8, 8),
> > +	FB_FORMAT_BGR(8, 8, 8),
> > +
> > +	FB_FORMAT_XRGB(8, 8, 8, 8),
> > +	FB_FORMAT_XBGR(8, 8, 8, 8),
> > +	FB_FORMAT_RGBX(8, 8, 8, 8),
> > +	FB_FORMAT_BGRX(8, 8, 8, 8),
> > +
> > +	FB_FORMAT_ARGB(8, 8, 8, 8),
> > +	FB_FORMAT_ABGR(8, 8, 8, 8),
> > +	FB_FORMAT_RGBA(8, 8, 8, 8),
> > +	FB_FORMAT_BGRA(8, 8, 8, 8),
> > +
> > +	FB_FORMAT_XRGB(2, 10, 10, 10),
> > +	FB_FORMAT_XBGR(2, 10, 10, 10),
> > +	FB_FORMAT_RGBX(10, 10, 10, 2),
> > +	FB_FORMAT_BGRX(10, 10, 10, 2),
> > +
> > +	FB_FORMAT_ARGB(2, 10, 10, 10),
> > +	FB_FORMAT_ABGR(2, 10, 10, 10),
> > +	FB_FORMAT_RGBA(10, 10, 10, 2),
> > +	FB_FORMAT_BGRA(10, 10, 10, 2),
> > +};
> > +
> > +static bool fb_format_match(const struct fb_var_screeninfo *var,
> > +			    const struct drm_fb_helper_format *fb_format)
> > +{
> > +	return !memcmp(&var->red, &fb_format->red,
> > +		       sizeof(var->red)) &&
> > +		!memcmp(&var->green, &fb_format->green,
> > +			sizeof(var->green)) &&
> > +		!memcmp(&var->blue, &fb_format->blue,
> > +			sizeof(var->blue)) &&
> > +		!memcmp(&var->transp, &fb_format->transp,
> > +			sizeof(var->transp));
> 
> This relies on the padding being initialised to 0 everywhere, which I'm
> not sure is true even with the static array here, but feels rather
> fragile regardless...

There should be no padding in these. They are part of the ioctl
structure so any padding would mean they're pretty much broken.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel