linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
To: Maxime Ripard <maxime.ripard@bootlin.com>,
	Daniel Vetter <daniel@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	David Airlie <airlied@linux.ie>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Sean Paul <seanpaul@chromium.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	"open list:DMA BUFFER SHARING FRAMEWORK" 
	<linux-media@vger.kernel.org>,
	Boris Brezillon <boris.brezillon@collabora.com>
Subject: Re: [PATCH 00/20] drm: Split out the formats API and move it to a common place
Date: Thu, 18 Apr 2019 13:49:54 +0200	[thread overview]
Message-ID: <6578c22ddf5420d4dead69d29f451bc6a91f6c4a.camel@bootlin.com> (raw)
In-Reply-To: <20190418090221.e57dogn4yx5nwdni@flea>

Hi,

On Thu, 2019-04-18 at 11:02 +0200, Maxime Ripard wrote:
> On Thu, Apr 18, 2019 at 09:52:10AM +0200, Daniel Vetter wrote:
> > And a lot of people pushed for the "fourcc is a standard", when
> > really it's totally not.
> 
> Even if it's not a standard, having consistency would be a good thing.
> 
> And you said yourself that DRM fourcc is now pretty much an authority
> for the fourcc, so it definitely looks like a standard to me.

I think trying to make the V4L2 and DRM fourccs converge is a lost
cause, as it has already significantly diverged. Even if we coordinate
an effort to introduce new formats with the same fourcc on both sides,
I don't see what good that would be since the formats we have now are
still plagued by the inconsistency.

I think we always need an explicit translation step from either v4l2 or
drm to the internal representation and back, without ever assuming that
formats might be compatible because they share the same fourcc.

It looks like so far, V4L2 pixel formats describe a DRM pixel format +
modifier. I think Boris (CCed) is working to change that by allowing to
pass a DRM modifier through V4L2. With that, we'd be in a situation
where some formats are described by the v4l2 pixfmt alone and some
formats are also described a modifier (but I looked at it from a
distance so might have misunderstod). That feels better since it avoids
the combinatory explosion from describing each format + modifier
individually.

What do you think?

Cheers,

Paul

> > v4l tends to conflate pixel format with stuff that we tend to encode
> > in modifiers a lot more.
> 
> Boris is working on adding the modifiers concept to v4l2, so we're
> converging here, and we can totally have a layer in v4l2 to convert
> between old v4l2 "format+modifiers" formats, and DRM style formats.
> 
> > There's a bunch of reasons we can't just use v4l, and they're as
> > valid as ever:
> > 
> > - We overlap badly in some areas, so even if fourcc codes match, we
> >   can't use them and need a duplicated DRM_FOURCC code.
> 
> Do yo have an example of one of those areas?
> 
> > - v4l encodes some metadata into the fourcc that we encode elsewhere,
> >   e.g. offset for planar yuv formats, or tiling mode
> 
> As I was saying, this changes on the v4l2 side, and converging to
> what DRM is doing.
> 
> > - drm fourcc code doesn't actually define the drm_format_info
> >   uniquely, drivers can override that (that's an explicit design
> >   intent of modifiers, to allow drivers to add another plane for
> >   e.g. compression information). You'd need to pull that driver
> >   knowledge into your format library.
> 
> I'm not sure how my patches are changing anything here. This is
> litterally the same code, with the functions renamed.
> 
> If drivers want to override that, then yeah, fine, we can let them do
> that. Just like any helper this just provides a default that covers
> most of the cases.
> 
> > Iow there's no way we can easily adopt v4l fourcc, except if we do
> > something like a new addfb flag.
> 
> For the formats that would be described as a modifier, sure. For all
> the others (that are not yet supported by DRM), then I don't really
> see why not.
> 
> > > And given how the current state is a mess in this regard, I'm not too
> > > optimistic about keeping the formats in their relevant frameworks.
> > > 
> > > Having a shared library, governed by both, will make this far easier,
> > > since it will be easy to discover the formats that are already
> > > supported by the other subsystem.
> > 
> > I think a compat library that (tries to, best effort) convert between
> > v4l and drm fourcc would make sense. Somewhere in drivers/video, next
> > to the conversion functions for videomode <-> drm_display_mode
> > perhaps. That should be useful for drivers.
> 
> That's not really what this series is about though. That series is
> about sharing the (image|pixels) formats database across everyone so
> that everyone can benefit from it.
> 
> > Unifying the formats themselves, and all the associated metadata is
> > imo a no-go, and was a pretty conscious decision when we implemented
> > drm_fourcc a few years back.
> > 
> > > If we want to keep the current library in DRM, we have two options
> > > then:
> > > 
> > >   - Support all the v4l2 formats in the DRM library, which is
> > >     essentially what I'm doing in the last patches. However, that
> > >     would require to have the v4l2 developpers also reviewing stuff
> > >     there. And given how busy they are, I cannot really see how that
> > >     would work.
> > 
> > Well, if we end up with a common library then yes we need cross
> > review. There's no way around that. Doesn't matter where exactly that
> > library is in the filesystem tree, and adding a special MAINTAINERS
> > entry for anything related to fourcc (both drm and v4l) to make sure
> > they get cross-posted is easy. No file renaming needed.
> 
> That would create some governing issues as well. For example, if you
> ever have a patch from one fourcc addition (that might or might not be
> covered by v4l2), will you wait for any v4l2 developper to review it?
> 
> If it's shared code, then it should be shared, and every client
> framework put on an equal footing.
> 
> > >   - Develop the same library from within v4l2. That is really a poor
> > >     solution, since the information would be greatly duplicated
> > >     between the two, and in terms of maintainance, code, and binary
> > >     size that would be duplicated too.
> > 
> > It's essentially what we decided to do for drm years back.
> 
> And it was probably the right solution back then, but I'm really not
> convinced it's still the right thing to do today.
> 
> > > Having it shared allows to easily share, and discover formats from the
> > > other subsystem, and to have a single, unique place where this is
> > > centralized.
> > 
> > What I think could work as middle ground:
> > - Put drm_format stuff into a separate .ko
> > - Add a MAINTAINERS entry to make sure all things fourcc are cross
> > posted to both drm and v4l lists. Easy on the drm side, since it's all
> > separate files. Not sure it's so convenient for v4l uapi.
> > - Add a conversion library that tries to best-effort map between drm
> > and v4l formats. On the drm side that most likely means you need
> > offsets for planes, and modifiers too (since those are implied in some
> > v4l fourcc). Emphasis on "best effort" i.e. only support as much as
> > the drivers that use this library need.
> > - Add drm_fourcc as-needed by these drivers that want to use a unified
> > format space.
> > 
> > Forcing this unification on everyone otoh is imo way too much.
> 
> v4l2 is starting to converge with DRM, and we're using the DRM API
> pretty much untouched for that library, so I'm not really sure how
> anyone is hurt by that unification.
> 
> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


  parent reply	other threads:[~2019-04-18 11:50 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-17  7:54 [PATCH 00/20] drm: Split out the formats API and move it to a common place Maxime Ripard
2019-04-17  7:54 ` [PATCH 01/20] drm: Remove users of drm_format_num_planes Maxime Ripard
2019-04-17  7:54 ` [PATCH 02/20] drm: Remove users of drm_format_(horz|vert)_chroma_subsampling Maxime Ripard
2019-04-17 13:32   ` Philipp Zabel
2019-04-17  7:54 ` [PATCH 03/20] drm/fourcc: Pass the format_info pointer to drm_format_plane_cpp Maxime Ripard
2019-04-17  7:54 ` [PATCH 04/20] drm/fourcc: Pass the format_info pointer to drm_format_plane_width/height Maxime Ripard
     [not found]   ` <776131c6-b8be-4302-ea9a-f7d84203f28c@linux.intel.com>
2019-04-17 11:01     ` Maxime Ripard
2019-04-17 11:10       ` Maarten Lankhorst
2019-04-17 13:12         ` Maxime Ripard
2019-04-17  7:54 ` [PATCH 05/20] drm: Replace instances of drm_format_info by drm_get_format_info Maxime Ripard
2019-04-17  7:54 ` [PATCH 06/20] lib: Add video format information library Maxime Ripard
2019-04-17 12:34   ` Paul Kocialkowski
2019-04-17 12:48     ` Maxime Ripard
2019-04-17 14:03       ` Paul Kocialkowski
2019-04-23 11:22   ` Thomas Zimmermann
2019-04-23 16:56     ` Paul Kocialkowski
2019-04-17  7:54 ` [PATCH 07/20] drm/fb: Move from drm_format_info to image_format_info Maxime Ripard
2019-04-17  7:54 ` [PATCH 08/20] drm/malidp: Convert to generic image format library Maxime Ripard
2019-04-17  7:54 ` [PATCH 09/20] drm/client: " Maxime Ripard
2019-04-17  7:54 ` [PATCH 10/20] drm/exynos: " Maxime Ripard
2019-04-17  7:54 ` [PATCH 11/20] drm/i915: " Maxime Ripard
2019-04-17  7:54 ` [PATCH 12/20] drm/ipuv3: " Maxime Ripard
2019-04-17  7:54 ` [PATCH 13/20] drm/msm: " Maxime Ripard
2019-04-17  7:54 ` [PATCH 14/20] drm/omap: " Maxime Ripard
2019-04-17  7:54 ` [PATCH 15/20] drm/rockchip: " Maxime Ripard
2019-04-17  7:54 ` [PATCH 16/20] drm/tegra: " Maxime Ripard
2019-04-17  7:54 ` [PATCH 17/20] drm/fourcc: Remove old DRM format API Maxime Ripard
2019-04-17  7:54 ` [PATCH 18/20] lib: image-formats: Add v4l2 formats support Maxime Ripard
2019-05-02  8:24   ` Hans Verkuil
2019-05-06 13:22     ` Maxime Ripard
2019-04-17  7:54 ` [PATCH 19/20] lib: image-formats: Add more functions Maxime Ripard
2019-04-17 12:39   ` Paul Kocialkowski
2019-04-17 12:41   ` Sakari Ailus
2019-04-17  7:54 ` [PATCH 20/20] media: sun6i: Convert to the image format API Maxime Ripard
2019-04-17 12:23 ` [PATCH 00/20] drm: Split out the formats API and move it to a common place Paul Kocialkowski
2019-04-17 12:38 ` Paul Kocialkowski
2019-04-17 15:41 ` Daniel Vetter
2019-04-18  6:22   ` Maxime Ripard
2019-04-18  7:52     ` Daniel Vetter
2019-04-18  9:02       ` Maxime Ripard
2019-04-18 10:07         ` Daniel Vetter
2019-04-18 12:01           ` Maxime Ripard
2019-04-18 12:32             ` Daniel Vetter
2019-04-18 20:56               ` Maxime Ripard
2019-04-20 23:05                 ` Laurent Pinchart
2019-05-02  8:25                 ` Hans Verkuil
2019-04-20 22:59           ` Laurent Pinchart
2019-04-23  7:25             ` Daniel Vetter
2019-04-23  8:59               ` Daniel Stone
2019-04-23 15:54                 ` Laurent Pinchart
2019-04-23 16:02                   ` Daniel Stone
2019-04-23 16:38                     ` Paul Kocialkowski
2019-04-23 15:45               ` Laurent Pinchart
2019-04-23 16:46                 ` Paul Kocialkowski
2019-04-23 19:18                 ` Daniel Vetter
2019-05-11 19:26                   ` Laurent Pinchart
2019-05-13 14:57                     ` Daniel Vetter
2019-05-13 15:23                       ` Mauro Carvalho Chehab
2019-04-18 11:49         ` Paul Kocialkowski [this message]
2019-04-20 22:40           ` Laurent Pinchart
2019-04-23  7:30             ` Daniel Vetter
2019-04-23 12:33               ` Paul Kocialkowski
2019-04-23 14:28                 ` Nicolas Dufresne
2019-04-23 14:55                   ` Paul Kocialkowski
2019-04-23 15:09                   ` Daniel Vetter
2019-04-23 17:16                     ` Nicolas Dufresne
2019-04-23 19:06                       ` Daniel Vetter
2019-04-23 16:54             ` Paul Kocialkowski
2019-05-11 19:19               ` Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6578c22ddf5420d4dead69d29f451bc6a91f6c4a.camel@bootlin.com \
    --to=paul.kocialkowski@bootlin.com \
    --cc=airlied@linux.ie \
    --cc=boris.brezillon@collabora.com \
    --cc=daniel.vetter@intel.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hans.verkuil@cisco.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=maxime.ripard@bootlin.com \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=seanpaul@chromium.org \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).