Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <cover.8ec406bf8f4f097e9dc909d5aac466556822f592.1555487650.git-series.maxime.ripard@bootlin.com>
 <20190417154121.GJ13337@phenom.ffwll.local> <20190418062229.eyog4i62eg4pr6uf@flea>
 <CAKMK7uHwSjqXwWGt+wQ6oMFWoPqmBxYHL7r+vTOXdYt9KMCYLQ@mail.gmail.com>
 <20190418090221.e57dogn4yx5nwdni@flea> <CAKMK7uHN6QfYyzx4DjRT+7VLRi6N9DorQtw2GoDiNGTgie=DXA@mail.gmail.com>
 <20190420225904.GZ4964@pendragon.ideasonboard.com> <20190423072554.GW13337@phenom.ffwll.local>
 <CAPj87rMWdjEvTJWRvdMLTZyw7pZpo5PpPr0FWcALNRe_D8cYqw@mail.gmail.com> <20190423155416.GI16111@pendragon.ideasonboard.com>
In-Reply-To: <20190423155416.GI16111@pendragon.ideasonboard.com>
From:   Daniel Stone <daniel@fooishbar.org>
Date:   Tue, 23 Apr 2019 17:02:27 +0100
Message-ID: <CAPj87rOyX_6X-UbgPVrU-uGeeyqgTnp57UYD3QUJeiCWDfk9Kg@mail.gmail.com>
Subject: Re: [PATCH 00/20] drm: Split out the formats API and move it to a
 common place
To:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc:     Maxime Ripard <maxime.ripard@bootlin.com>,
        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>,
        Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
        Hans Verkuil <hans.verkuil@cisco.com>,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
        "open list:DMA BUFFER SHARING FRAMEWORK" 
        <linux-media@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Laurent,

On Tue, 23 Apr 2019 at 16:54, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Tue, Apr 23, 2019 at 09:59:37AM +0100, Daniel Stone wrote:
> > On Tue, 23 Apr 2019 at 08:26, Daniel Vetter <daniel@ffwll.ch> wrote:
> > Totally. Let's take DRM_FORMAT_XRGB8888 + I915_FORMAT_MOD_Y_TILED as
> > an example. [... details ...]
>
> Looks like we have different kinds of metadata to consider. On the V4L2
> side metadata usually refers to the context in which a frame was
> captured, it doesn't tell how to interpret the contents of the pixels.
> In the case you just described, the metadata is part of the frame
> contents. I agree that this is a proper use case for storing such
> metadata in a plane. What I wouldn't like to see being stored in a plane
> is for instance gamma tables or similar data that configures the
> processing pipeline in the display engine (I know we have an API for
> gamma tables, this is just an example).
>
> > It would be good to understand what you had in mind when you said that
> > using multiple planes created a mess. I haven't touched media
> > encode/decode units at a low level for quite a while (hooray for
> > gst-v4l2!), but I remember that they often used padding areas around
> > the buffer for scratch space - maybe motion vectors or similar? That
> > case is quite different to something like CCS, since the data is only
> > meaningful to the media engine and must be ignored (but preserved) by
> > everyone else. Using multiple planes in that case isn't appropriate,
> > since it's very specific to how that hardware unit deals with that
> > buffer, instead of something that every consumer needs to understand
> > in order to use it.
>
> With metadata unrelated to the pixel content, using a separate plane in
> the same buffer resulted in an explosion of the number of combinations
> that we would need to support, and ultimately led to a very ill-defined
> API. We decided to convey metadata related to the frame capture context
> (e.g. what exposure time was used for the frame) and processing pipeline
> configuration data in different buffers than the frame itself.

Yeah, that makes sense. It's not really that different from what
happens with GPUs either: the final colour buffer the display
controller gets from a game is the product of a _lot_ of other work
which is invisible to the display controller, including things like
depth and stencil buffers, command buffers, etc etc. Those are closely
related to the frame production, but totally irrelevant for exchanging
the colour buffer with other subsystems.

I think we should look at the metadata buffers you're describing in
the same way. Perhaps each V4L2 buffer could have driver-private
auxiliary buffer storage, or perhaps it's something you need to
separately expose to userspace as auxiliary data which must be
preserved but ignored. But modifiers are really only about what you
need when exchanging image colour buffers between subsystems, not
anything else.

You're pretty close with gamma tables as well; for HDR and other kinds
of complex colour management, we need to carry a fair bit of auxiliary
information in order to display the image correctly. These have quite
different uses though: normally the colour buffer is produced by the
hardware and the primaries/whitepoints/etc are produced by software,
with the colour-management details remaining static across the life of
a swapchain even as you flip between multiple buffers. Given that, it
doesn't really make sense to try to stuff them into the same storage.

Cheers,
Daniel