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

Tue Nov 7 21:17:04 UTC 2023

On Tue, Nov 07, 2023 at 11:52:11AM -0500, Harry Wentland wrote:
> 
> 
> On 2023-10-26 13:30, Sebastian Wick wrote:
> > On Thu, Oct 26, 2023 at 11:57:47AM +0300, Pekka Paalanen wrote:
> >> On Wed, 25 Oct 2023 15:16:08 -0500 (CDT)
> >> Alex Goins <agoins at nvidia.com> wrote:
> >>
> >>> Thank you Harry and all other contributors for your work on this. Responses
> >>> inline -
> >>>
> >>> On Mon, 23 Oct 2023, Pekka Paalanen wrote:
> >>>
> >>>> On Fri, 20 Oct 2023 11:23:28 -0400
> >>>> Harry Wentland <harry.wentland at 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 at redhat.com> wrote:
> >>>>>>     
> >>>>>>> Thanks for continuing to work on this!
> >>>>>>>
> >>>>>>> On Thu, Oct 19, 2023 at 05:21:22PM -0400, Harry Wentland wrote:    
> 
> snip
> 
> >>
> >>>>>> 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.  
> >>>
> >>> Would it be reasonable to expose this is a 3x4 matrix with a read-only blob and
> >>> no BYPASS property? I already brought up a similar idea at the XDC HDR Workshop
> >>> based on the principle that read-only blobs could be used to express some static
> >>> pipeline elements without the need to define a new type, but got mixed opinions.
> >>> I think this demonstrates the principle further, as clients could detect this
> >>> programmatically instead of having to special-case the informational element.
> >>
> > 
> > I'm all for exposing fixed color ops but I suspect that most of those
> > follow some standard and in those cases instead of exposing the matrix
> > values one should prefer to expose a named matrix (e.g. BT.601, BT.709,
> > BT.2020).
> > 
> 
> Agreed.
> 
> > As a general rule: always expose the highest level description. Going
> > from a name to exact values is trivial, going from values to a name is
> > much harder.
> > 
> >> If the blob depends on the pixel format (i.e. the driver automatically
> >> chooses a different blob per pixel format), then I think we would need
> >> to expose all the blobs and how they correspond to pixel formats.
> >> Otherwise ok, I guess.
> >>
> >> However, do we want or need to make a color pipeline or colorop
> >> conditional on pixel formats? For example, if you use a YUV 4:2:0 type
> >> of pixel format, then you must use this pipeline and not any other. Or
> >> floating-point type of pixel format. I did not anticipate this before,
> >> I assumed that all color pipelines and colorops are independent of the
> >> framebuffer pixel format. A specific colorop might have a property that
> >> needs to agree with the framebuffer pixel format, but I didn't expect
> >> further limitations.
> > 
> > We could simply fail commits when the pipeline and pixel format don't
> > work together. We'll probably need some kind of ingress no-op node
> > anyway and maybe could list pixel formats there if required to make it
> > easier to find a working configuration.
> > 
> 
> The problem with failing commits is that user-space has no idea why it
> failed. If this means that userspace falls back to SW composition for
> NV12 and P010 it would avoid HW offloading in one of the most important
> use-cases on AMD HW for power-saving purposes.

Exposing which pixel formats work with a pipeline should be
uncontroversial, and so should be an informative scaler op.

Both can be added without a problem at a later time, so let's not make
any of that mandatory for the first version. One step after the other.

> 
> snip
> 
> >>> Despite being programmable, the LUTs are updated in a manner that is less
> >>> efficient as compared to e.g. the non-static "degamma" LUT. Would it be helpful
> >>> if there was some way to tag operations according to their performance,
> >>> for example so that clients can prefer a high performance one when they
> >>> intend to do an animated transition? I recall from the XDC HDR workshop
> >>> that this is also an issue with AMD's 3DLUT, where updates can be too
> >>> slow to animate.
> >>
> >> I can certainly see such information being useful, but then we need to
> >> somehow quantize the performance.
> >>
> >> What I was left puzzled about after the XDC workshop is that is it
> >> possible to pre-load configurations in the background (slow), and then
> >> quickly switch between them? Hardware-wise I mean.
> > 
> > We could define that pipelines with a lower ID are to be preferred over
> > higher IDs.
> > 
> > The issue is that if programming a pipeline becomes too slow to be
> > useful it probably should just not be made available to user space.
> > 
> > The prepare-commit idea for blob properties would help to make the
> > pipelines usable again, but until then it's probably a good idea to just
> > not expose those pipelines.
> > 
> 
> It's a bit of a judgment call what's too slow, though. The value of having
> a HW colorop might outweigh the cost of the programming time for some
> compositors but not for others.
> 
> Harry
> 
> >>
> >>
> >> Thanks,
> >> pq
> > 
> > 
> 