Re: [Intel-gfx] [RFC v2 01/22] drm: RFC for Plane Color Hardware Pipeline

Pekka Paalanen
2021-10-15T07:42:54Z

[-- Attachment #1: Type: text/plain, Size: 7883 bytes --]

On Thu, 14 Oct 2021 19:44:25 +0000
"Shankar, Uma" <uma.shankar@intel.com> wrote:

> > -----Original Message-----
> > From: Pekka Paalanen <ppaalanen@gmail.com>
> > Sent: Wednesday, October 13, 2021 2:01 PM
> > To: Shankar, Uma <uma.shankar@intel.com>
> > Cc: harry.wentland@amd.com; ville.syrjala@linux.intel.com; intel-
> > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> > brian.starkey@arm.com; sebastian@sebastianwick.net;
> > Shashank.Sharma@amd.com
> > Subject: Re: [RFC v2 01/22] drm: RFC for Plane Color Hardware Pipeline
> >
> > On Tue, 12 Oct 2021 20:58:27 +0000
> > "Shankar, Uma" <uma.shankar@intel.com> wrote:
> >
> > > > -----Original Message-----
> > > > From: Pekka Paalanen <ppaalanen@gmail.com>
> > > > Sent: Tuesday, October 12, 2021 4:01 PM
> > > > To: Shankar, Uma <uma.shankar@intel.com>
> > > > Cc: intel-gfx@lists.freedesktop.org;
> > > > dri-devel@lists.freedesktop.org; harry.wentland@amd.com;
> > > > ville.syrjala@linux.intel.com; brian.starkey@arm.com;
> > > > sebastian@sebastianwick.net; Shashank.Sharma@amd.com
> > > > Subject: Re: [RFC v2 01/22] drm: RFC for Plane Color Hardware
> > > > Pipeline
> > > >
> > > > On Tue, 7 Sep 2021 03:08:43 +0530 Uma Shankar
> > > > <uma.shankar@intel.com> wrote:
> > > >
> > > > > This is a RFC proposal for plane color hardware blocks.
> > > > > It exposes the property interface to userspace and calls out the
> > > > > details or interfaces created and the intended purpose.
> > > > >
> > > > > Credits: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > > > > ---
> > > > > Documentation/gpu/rfc/drm_color_pipeline.rst | 167
> > > > > +++++++++++++++++++
> > > > > 1 file changed, 167 insertions(+) create mode 100644
> > > > > Documentation/gpu/rfc/drm_color_pipeline.rst
> > > > >
> > > > > diff --git a/Documentation/gpu/rfc/drm_color_pipeline.rst
> > > > > b/Documentation/gpu/rfc/drm_color_pipeline.rst
> > > > > new file mode 100644
> > > > > index 000000000000..0d1ca858783b
> > > > > --- /dev/null
> > > > > +++ b/Documentation/gpu/rfc/drm_color_pipeline.rst
> > > > > @@ -0,0 +1,167 @@
> > > > > +==================================================
> > > > > +Display Color Pipeline: Proposed DRM Properties

...

> > cf. BT.2100 Annex 1, "The relationship between the OETF, the EOTF and
> > the OOTF", although I find those diagrams somewhat confusing still. It
> > does not seem to clearly account for transmission non-linear encoding being different from the display EOTF.
> >
> > Different documents use OOTF to refer to different things. Then there
> > is also the fundamental difference between PQ and HLG systems, where
> > OOTF is by definition in different places of the camera-transmission-display pipeline.
>
> Agree this is a bit confusing, I admit I was much more confused than what you were for sure.
> Will do some research to get my understanding in place. Thanks for all the inputs.

I'm sure I was at least equally confused or even more at the start. I
have just had a year of casual reading, discussions, and a W3C workshop
attendance to improve my understanding. :-)

Now I know enough to be dangerous.

...

> > > > > +
> > > > > +UAPI Name: PLANE_DEGAMMA_MODE
> > > > > +Description: Enum property with values as blob_id's which
> > > > > +advertizes
> > > > > the
> > > >
> > > > Is enum with blob id values even a thing?
> > >
> > > Yeah we could have. This is a dynamic enum created with blobs. Each
> > > entry contains the data structure to extract the full color
> > > capabilities of the hardware. It’s a very interesting play with
> > > blobs (@ville.syrjala@linux.intel.com brainchild)
> >
> > Yes, I think I can imagine how that works. The blobs are created
> > immutable, unable to change once the plane has been advertised to
> > userspace, because their IDs are used as enum values. But that is ok,
> > because the blob describes capabilities that cannot change during the
> > device's lifetime... or can they? What if you would want to extend the
> > blob format, do you need a KMS client cap to change the format which
> > would be impossible because you can't change an enum definition after it has been installed so you cannot swap the blob to a new one?
> >
> > This also relies on enums being identified by their string names,
> > because the numerical value is not a constant but a blob ID and gets
> > determined when the immutable blob is created.
> >
> > Did I understand correctly?
>
> Yes that’s right. We are not expecting these structures to change as
> they represent the platforms capabilities.

Until there comes a new platform whose capabilities you cannot quite
describe with the existing structure. What's the plan to deal with that?
A new enum value, like LUT2 instead of LUT? I suppose that works.

>
> > As a userspace developer, I can totally work with that.
> >
> > > > > + possible degamma modes and lut ranges supported by the platform.
> > > > > + This allows userspace to query and get the plane degamma color
> > > > > + caps and choose the appropriate degamma mode and create lut values
> > > > > + accordingly.
> > > >
> > > > I agree that some sort of "mode" switch is necessary, and
> > > > advertisement of capabilities as well.
> > > >
> > >
> > > This enum with blob id's is an interesting way to advertise segmented lut tables.
> > >
> > > > > +
> > > > > +UAPI Name: PLANE_DEGAMMA_LUT
> > > > > +Description: Blob property which allows a userspace to provide LUT values
> > > > > + to apply degamma curve using the h/w plane degamma processing
> > > > > + engine, thereby making the content as linear for further color
> > > > > + processing. Userspace gets the size of LUT and precision etc
> > > > > + from PLANE_DEGAMA_MODE_PROPERTY
> > > >
> > > > So all degamma modes will always be some kind of LUT? That may be
> > > > a bit restrictive, as I understand AMD may have predefined or
> > > > parameterised curves that are not LUTs. So there should be room
> > > > for an arbitrary structure of parameters, which can be passed in
> > > > as a blob id, and the contents defined by the degamma mode.
> > >
> > > For Intel's hardware these are luts but yeah AMD hardware seems to
> > > have these as fixed function units. We should think of a way to have
> > > this option as well in the UAPI. We could extend the DEGAMMA_MODE
> > > property to have all the info, and DEGAMMA_LUT_PROPERTY may not be
> > > needed based on some of the attributes passed via DEGAMMA_MODE. This
> > > can be
> > one way to have room for both.
> > > @harry.wentland@amd.com thoughts ?
> >
> > Yeah, though I don't think you can use DEGAMMA_MODE property to pass
> > parameters to fixed-function curves. The parameters need another
> > property. Would be natural to use the other property for LUT data when mode it set to LUT.
> >
> > A trivial and made-up example of a parameterised fixed-function curve
> > is pow(x, gamma), where gamma is the parameter.
>
> We can maybe have a parallel property as well with proper
> documentation if this helps with flexibility for varying hardware
> vendors. UAPI document will list what to use and when. May be
> platform will not even list the other one which may make things less
> complicated to userspace.

I'm not sure I got what you're thinking. My idea is that the
interpretation of the blob contents depends on the value of the mode
enum. Obviously the two will always need to be set together by
userspace to ensure they match, otherwise DRM needs to reject the
commit.

The rest of your comments I agree with.

Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]