Re: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed

From: Pekka Paalanen <ppaalanen@gmail.com>
To: Harry Wentland <harry.wentland@amd.com>
Cc: "Sasha McIntosh" <sashamcintosh@google.com>,
	"Liviu Dudau" <Liviu.Dudau@arm.com>,
	"Victoria Brekenfeld" <victoria@system76.com>,
	dri-devel@lists.freedesktop.org,
	"Michel Dänzer" <mdaenzer@redhat.com>,
	"Arthur Grillo" <arthurgrillo@riseup.net>,
	"Christopher Braga" <quic_cbraga@quicinc.com>,
	"Sebastian Wick" <sebastian.wick@redhat.com>,
	"Shashank Sharma" <shashank.sharma@amd.com>,
	wayland-devel@lists.freedesktop.org,
	"Jonas Ådahl" <jadahl@redhat.com>,
	"Uma Shankar" <uma.shankar@intel.com>,
	"Abhinav Kumar" <quic_abhinavk@quicinc.com>,
	"Naseer Ahmed" <quic_naseer@quicinc.com>,
	"Melissa Wen" <mwen@igalia.com>, "Aleix Pol" <aleixpol@kde.org>,
	"Hector Martin" <marcan@marcan.st>,
	"Xaver Hugl" <xaver.hugl@gmail.com>,
	"Joshua Ashton" <joshua@froggi.es>
Subject: Re: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed
Date: Mon, 23 Oct 2023 11:12:19 +0300	[thread overview]
Message-ID: <20231023111219.6e287958@eldfell> (raw)
In-Reply-To: <20f33898-4c40-4854-a1e8-afaef52f6524@amd.com>

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

On Fri, 20 Oct 2023 11:23:28 -0400
Harry Wentland <harry.wentland@amd.com> wrote:

> On 2023-10-20 10:57, Pekka Paalanen wrote:
> > On Fri, 20 Oct 2023 16:22:56 +0200
> > Sebastian Wick <sebastian.wick@redhat.com> wrote:
> >   
> >> Thanks for continuing to work on this!
> >>
> >> On Thu, Oct 19, 2023 at 05:21:22PM -0400, Harry Wentland wrote:  
> >>> v2:
> >>>  - Update colorop visualizations to match reality (Sebastian, Alex Hung)
> >>>  - Updated wording (Pekka)
> >>>  - Change BYPASS wording to make it non-mandatory (Sebastian)
> >>>  - Drop cover-letter-like paragraph from COLOR_PIPELINE Plane Property
> >>>    section (Pekka)
> >>>  - Use PQ EOTF instead of its inverse in Pipeline Programming example (Melissa)
> >>>  - Add "Driver Implementer's Guide" section (Pekka)
> >>>  - Add "Driver Forward/Backward Compatibility" section (Sebastian, Pekka)
> >>>  
> > 
> > ...
> >   
> >>> +Driver Forward/Backward Compatibility
> >>> +=====================================
> >>> +
> >>> +As this is uAPI drivers can't regress color pipelines that have been
> >>> +introduced for a given HW generation. New HW generations are free to
> >>> +abandon color pipelines advertised for previous generations.
> >>> +Nevertheless, it can be beneficial to carry support for existing color
> >>> +pipelines forward as those will likely already have support in DRM
> >>> +clients.
> >>> +
> >>> +Introducing new colorops to a pipeline is fine, as long as they can be
> >>> +disabled or are purely informational. DRM clients implementing support
> >>> +for the pipeline can always skip unknown properties as long as they can
> >>> +be confident that doing so will not cause unexpected results.
> >>> +
> >>> +If a new colorop doesn't fall into one of the above categories
> >>> +(bypassable or informational) the modified pipeline would be unusable
> >>> +for user space. In this case a new pipeline should be defined.    
> >>
> >> How can user space detect an informational element? Should we just add a
> >> BYPASS property to informational elements, make it read only and set to
> >> true maybe? Or something more descriptive?  
> > 
> > Read-only BYPASS set to true would be fine by me, I guess.
> >   
> 
> Don't you mean set to false? An informational element will always do
> something, so it can't be bypassed.

Yeah, this is why we need a definition. I understand "informational" to
not change pixel values in any way. Previously I had some weird idea
that scaling doesn't alter color, but of course it may.

> > I think we also need a definition of "informational".
> > 
> > Counter-example 1: a colorop that represents a non-configurable  
> 
> Not sure what's "counter" for these examples?
> 
> > YUV<->RGB conversion. Maybe it determines its operation from FB pixel
> > format. It cannot be set to bypass, it cannot be configured, and it
> > will alter color values.
> > 
> > Counter-example 2: image size scaling colorop. It might not be
> > configurable, it is controlled by the plane CRTC_* and SRC_*
> > properties. You still need to understand what it does, so you can
> > arrange the scaling to work correctly. (Do not want to scale an image
> > with PQ-encoded values as Josh demonstrated in XDC.)
> >   
> 
> IMO the position of the scaling operation is the thing that's important
> here as the color pipeline won't define scaling properties.
> 
> > Counter-example 3: image sampling colorop. Averages FB originated color
> > values to produce a color sample. Again do not want to do this with
> > PQ-encoded values.
> >   
> 
> Wouldn't this only happen during a scaling op?

There is certainly some overlap between examples 2 and 3. IIRC SRC_X/Y
coordinates can be fractional, which makes nearest vs. bilinear
sampling have a difference even if there is no scaling.

There is also the question of chroma siting with sub-sampled YUV. I
don't know how that actually works, or how it theoretically should work.

Thanks,
pq

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