linux-kernel - Re: [RFC PATCH 06/20] lib: Add video format information library

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190321094017.62425776@collabora.com>
Date:   Thu, 21 Mar 2019 09:40:17 +0100
From:   Boris Brezillon <boris.brezillon@...labora.com>
To:     Maxime Ripard <maxime.ripard@...tlin.com>
Cc:     Daniel Vetter <daniel.vetter@...el.com>,
        David Airlie <airlied@...ux.ie>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Sean Paul <seanpaul@...omium.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        Paul Kocialkowski <paul.kocialkowski@...tlin.com>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        linux-media@...r.kernel.org
Subject: Re: [RFC PATCH 06/20] lib: Add video format information library

On Thu, 21 Mar 2019 09:20:41 +0100
Maxime Ripard <maxime.ripard@...tlin.com> wrote:

> Hi Boris,
> 
> On Wed, Mar 20, 2019 at 02:39:44PM +0100, Boris Brezillon wrote:
> > On Tue, 19 Mar 2019 22:57:11 +0100
> > Maxime Ripard <maxime.ripard@...tlin.com> wrote:
> >  
> > > Move the DRM formats API to turn this into a more generic image formats API
> > > to be able to leverage it into some other places of the kernel, such as
> > > v4l2 drivers.
> > >
> > > Signed-off-by: Maxime Ripard <maxime.ripard@...tlin.com>
> > > ---
> > >  include/linux/image-formats.h | 240 +++++++++++-
> > >  lib/Kconfig                   |   7 +-
> > >  lib/Makefile                  |   3 +-
> > >  lib/image-formats-selftests.c | 326 +++++++++++++++-
> > >  lib/image-formats.c           | 760 +++++++++++++++++++++++++++++++++++-
> > >  5 files changed, 1336 insertions(+)
> > >  create mode 100644 include/linux/image-formats.h
> > >  create mode 100644 lib/image-formats-selftests.c
> > >  create mode 100644 lib/image-formats.c
> > >  
> >
> > [...]
> >  
> > > --- /dev/null
> > > +++ b/lib/image-formats.c
> > > @@ -0,0 +1,760 @@
> > > +#include <linux/bug.h>
> > > +#include <linux/image-formats.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/math64.h>
> > > +
> > > +#include <uapi/drm/drm_fourcc.h>
> > > +
> > > +static const struct image_format_info formats[] = {
> > > +	{  
> >
> > ...
> >  
> > > +	},
> > > +};
> > > +
> > > +#define __image_format_lookup(_field, _fmt)			\
> > > +	({							\
> > > +		const struct image_format_info *format = NULL;	\
> > > +		unsigned i;					\
> > > +								\
> > > +		for (i = 0; i < ARRAY_SIZE(formats); i++)	\
> > > +			if (formats[i]._field == _fmt)		\
> > > +				format = &formats[i];		\
> > > +								\
> > > +		format;						\
> > > +	})
> > > +
> > > +/**
> > > + * __image_format_drm_lookup - query information for a given format
> > > + * @drm: DRM fourcc pixel format (DRM_FORMAT_*)
> > > + *
> > > + * The caller should only pass a supported pixel format to this function.
> > > + *
> > > + * Returns:
> > > + * The instance of struct image_format_info that describes the pixel format, or
> > > + * NULL if the format is unsupported.
> > > + */
> > > +const struct image_format_info *__image_format_drm_lookup(u32 drm)
> > > +{
> > > +	return __image_format_lookup(drm_fmt, drm);
> > > +}
> > > +EXPORT_SYMBOL(__image_format_drm_lookup);
> > > +
> > > +/**
> > > + * image_format_drm_lookup - query information for a given format
> > > + * @drm: DRM fourcc pixel format (DRM_FORMAT_*)
> > > + *
> > > + * The caller should only pass a supported pixel format to this function.
> > > + * Unsupported pixel formats will generate a warning in the kernel log.
> > > + *
> > > + * Returns:
> > > + * The instance of struct image_format_info that describes the pixel format, or
> > > + * NULL if the format is unsupported.
> > > + */
> > > +const struct image_format_info *image_format_drm_lookup(u32 drm)
> > > +{
> > > +	const struct image_format_info *format;
> > > +
> > > +	format = __image_format_drm_lookup(drm);
> > > +
> > > +	WARN_ON(!format);
> > > +	return format;
> > > +}
> > > +EXPORT_SYMBOL(image_format_drm_lookup);  
> >
> > I think this function and the DRM formats table should be moved in
> > drivers/gpu/drm/drm_image_format.c since they are DRM specific. The
> > remaining functions can IMHO be placed in include/linux/image-formats.h
> > as static inline funcs. This way you can get rid of lib/image-formats.c
> > and the associated Kconfig entry.  
> 
> I'm not quite sure what you mean. The whole point of the series is to
> split out that table out of DRM so that we can use it in other places,
> so surely putting it back into DRM defeats the purpose?

Sorry, I hadn't read the patch series entirely when replying to this
email. I thought you were planning to create one table for DRM formats
and one for V4L ones and then just use the common infra to have a
generic image_format representation, hence my suggestion.