On 2023-11-08 06:40, Sebastian Wick wrote: > On Wed, Nov 8, 2023 at 11:16 AM Pekka Paalanen <ppaalanen@xxxxxxxxx> wrote: >> >> On Tue, 7 Nov 2023 11:58:26 -0500 >> Harry Wentland <harry.wentland@xxxxxxx> wrote: >> >>> On 2023-11-07 04:55, Pekka Paalanen wrote: >>>> On Mon, 6 Nov 2023 11:19:27 -0500 >>>> Harry Wentland <harry.wentland@xxxxxxx> wrote: >>>> >>>>> On 2023-10-20 06:36, Pekka Paalanen wrote: >>>>>> On Thu, 19 Oct 2023 10:56:40 -0400 >>>>>> Harry Wentland <harry.wentland@xxxxxxx> wrote: >>>>>> >>>>>>> On 2023-10-10 12:13, Melissa Wen wrote: >>>>>>>> O 09/08, Harry Wentland wrote: >>>>>>>>> Signed-off-by: Harry Wentland <harry.wentland@xxxxxxx> >>>>>> >>>>>> ... >>>>>> >>>>>>>> Also, with this new plane API in place, I understand that we will >>>>>>>> already need think on how to deal with the mixing between old drm color >>>>>>>> properties (color encoding and color range) and these new way of setting >>>>>>>> plane color properties. IIUC, Pekka asked a related question about it >>>>>>>> when talking about CRTC automatic RGB->YUV (?) >>>>>>>> >>>>>>> >>>>>>> We'll still need to confirm whether we'll want to deprecate these >>>>>>> existing properties. If we do that we'd want a client prop. Things >>>>>>> should still work without deprecating them, if drivers just pick up >>>>>>> after the initial encoding and range CSC. >>>>>>> >>>>>>> But realistically it might be better to deprecate them and turn them >>>>>>> into explicit colorops. >>>>>> >>>>>> The existing properties would need to be explicitly reflected in the >>>>>> new pipelines anyway, otherwise there would always be doubt at which >>>>>> point of a pipeline the old properties apply, and they might even >>>>>> need to change positions between pipelines. >>>>>> >>>>>> I think it is simply easier to just hide all old color related >>>>>> properties when userspace sets the client-cap to enable pipelines. The >>>>>> problem is to make sure to hide all old properties on all drivers that >>>>>> support the client-cap. >>>>>> >>>>>> As a pipeline must be complete (describe everything that happens to >>>>>> pixel values), it's going to be a flag day per driver. >>>>>> >>>>>> Btw. the plane FB YUV->RGB conversion needs a colorop in every pipeline >>>>>> as well. Maybe it's purely informative and non-configurable, keyed by >>>>>> FB pixel format, but still. >>>>>> >>>>>> We also need a colorop to represent sample filtering, e.g. bilinear, >>>>>> like I think Sebastian may have mentioned in the past. Everything >>>>>> before the sample filter happens "per tap" as Joshua Ashton put it, and >>>>>> everything after it happens on the sample that was computed as a >>>>>> weighted average of the filter tap inputs (texels). >>>>>> >>>>>> There could be colorops other than sample filtering that operate on >>>>>> more than one sample at a time, like blur or sharpness. There could >>>>>> even be colorops that change the image size like adding padding that >>>>>> the following colorop hardware requires, and then yet another colorop >>>>>> that clips that padding away. For an example, see >>>>>> https://lists.freedesktop.org/archives/dri-devel/2023-October/427015.html >>>>>> >>>>>> If that padding and its color can affect the pipeline results of the >>>>>> pixels near the padding (e.g. some convolution is applied with them, >>>>>> which may be the reason why padding is necessary to begin with), then >>>>>> it would be best to model it. >>>>>> >>>>> >>>>> I hear you but I'm also somewhat shying away from defining this at this point. >>>> >>>> Would you define them before the new UAPI is released though? >>>> >>>> I agree there is no need to have them in this patch series, but I think >>>> we'd hit the below problems if the UAPI is released without them. >>>> >>>>> There are already too many things that need to happen and I will focus on the >>>>> actual color blocks (LUTs, matrices) first. We'll always be able to add a new >>>>> (read-only) colorop type to define scaling and tap behavior at any point and >>>>> a client is free to ignore a color pipeline if it doesn't find any tap/scale >>>>> info in it. >>>> >>>> How would userspace know to look for tap/scale info, if there is no >>>> upstream definition even on paper? >>>> >>> >>> So far OSes did not care about this. Whether that's good or bad is >>> something everyone can answer for themselves. >>> >>> If you write a compositor and really need this you can just ignore >>> color pipelines that don't have this, i.e., you'll probably want >>> to wait with implementing color pipeline support until you have what >>> you need from DRM/KMS. >>> >>> This is not to say I don't want to have support for Weston. But I'm >>> wondering if we place too much importance on getting every little >>> thing figured out whereas we could be making forward progress and >>> address more aspects of a color pipeline in the future. There is a >>> reason gamescope has made a huge difference in driving the color >>> management work forward. >>> >>>> And the opposite case, if someone writes userspace without tap/scale >>>> colorops, and then drivers add those, and there is no pipeline without >>>> them, because they always exist. Would that userspace disregard all >>>> those pipelines because it does not understand tap/scale colorops, >>>> leaving no usable pipelines? Would that not be kernel regressing >>>> userspace? >>>> >>> >>> The simple solution is to leave previously advertised pipelines >>> untouched and add a new version that does include scaling information. >>> >>>> If the kernel keeps on exposing pipelines without the colorops, it >>>> fails the basic promise of the whole design: that all pixel value >>>> affecting operations are at least listed if not controllable. >>>> >>>> How will we avoid painting ourselves in a corner? >>>> >>>> Maybe we need a colorop for "here be dragons" documented as having >>>> unknown and unreliable effects, until driver authors are sure that >>>> everything has been modelled in the pipeline and there are no unknowns? >>>> Or a flag on the pipelines, if we can have that. Then we can at least >>>> tell when the pipeline does not fulfil the basic promise. >>>> >>> >>> The will always be dragons at some level. >> >> Do I understand right that the goal of fully understood color pipelines >> is a lost cause? >> >> That every pipeline might always have something unknown and there is no >> way for userspace to know if it does? Maybe because driver developers >> don't know either? >> >> By something unknown I refer to anything outside of basic precision >> issues. Doing interpolation or mixing of inputs on the wrong side of a >> known non-linear colorop, for example. > > I don't think that's the case. Hardware vendors should understand the > hardware and exposing everything that affects the values is the goal > here. There will be a transitional period where the pipelines might > not expose every detail yet but that's fine. It's better than what we > have now and should become even better with time. It would maybe be > helpful in the future to have a cap, or property, or whatever, to > indicate that the pipelines are "complete" descriptions of what > happens to the values but we can discuss it when it becomes relevant. > I agree, for the most part. But how do you then define "complete" if you exclude precision issues? >> An incremental UAPI development approach is fine by me, meaning that >> pipelines might not be complete at first, but I believe that requires >> telling userspace whether the driver developers consider the pipeline >> complete (no undescribed operations that would significantly change >> results from the expected results given the UAPI exposed pipeline). >> >> The prime example of what I would like to know is that if a FB >> contains PQ-encoded image and I use a color pipeline to scale that >> image up, will the interpolation happen before or after the non-linear >> colorop that decodes PQ. That is a significant difference as pointed >> out by Joshua. >> That's fair and I want to give that to you. My concern stems from the sentiment that I hear that any pipeline that doesn't explicitly advertise this is useless. I don't agree there. Let's not let perfect be the enemy of good. Harry >> >> Thanks, >> pq >