On 2021-07-15 5:34 a.m., Pekka Paalanen wrote: > On Wed, 14 Jul 2021 12:13:58 -0400 > Harry Wentland <harry.wentland@xxxxxxx> wrote: > >> On 2021-07-14 3:35 a.m., Pekka Paalanen wrote: >>> On Tue, 13 Jul 2021 09:54:35 -0400 >>> Harry Wentland <harry.wentland@xxxxxxx> wrote: >>> >>>> On 2021-07-13 3:52 a.m., Pekka Paalanen wrote: >>>>> On Mon, 12 Jul 2021 12:15:59 -0400 >>>>> Harry Wentland <harry.wentland@xxxxxxx> wrote: >>>>> >>>>>> On 2021-07-12 4:03 a.m., Pekka Paalanen wrote: >>>>>>> On Fri, 9 Jul 2021 18:23:26 +0200 >>>>>>> Raphael Gallais-Pou <raphael.gallais-pou@xxxxxxxxxxx> wrote: >>>>>>> >>>>>>>> On 7/9/21 10:04 AM, Pekka Paalanen wrote: >>>>>>>>> On Wed, 7 Jul 2021 08:48:47 +0000 >>>>>>>>> Raphael GALLAIS-POU - foss <raphael.gallais-pou@xxxxxxxxxxx> wrote: >>>>>>>>> >>>>>>>>>> Some display controllers can be programmed to present non-black colors >>>>>>>>>> for pixels not covered by any plane (or pixels covered by the >>>>>>>>>> transparent regions of higher planes). Compositors that want a UI with >>>>>>>>>> a solid color background can potentially save memory bandwidth by >>>>>>>>>> setting the CRTC background property and using smaller planes to display >>>>>>>>>> the rest of the content. >>>>>>>>>> >>>>>>>>>> To avoid confusion between different ways of encoding RGB data, we >>>>>>>>>> define a standard 64-bit format that should be used for this property's >>>>>>>>>> value. Helper functions and macros are provided to generate and dissect >>>>>>>>>> values in this standard format with varying component precision values. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Raphael Gallais-Pou <raphael.gallais-pou@xxxxxxxxxxx> >>>>>>>>>> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> >>>>>>>>>> --- >>>>>>>>>> drivers/gpu/drm/drm_atomic_state_helper.c | 1 + >>>>>>>>>> drivers/gpu/drm/drm_atomic_uapi.c | 4 +++ >>>>>>>>>> drivers/gpu/drm/drm_blend.c | 34 +++++++++++++++++++++-- >>>>>>>>>> drivers/gpu/drm/drm_mode_config.c | 6 ++++ >>>>>>>>>> include/drm/drm_blend.h | 1 + >>>>>>>>>> include/drm/drm_crtc.h | 12 ++++++++ >>>>>>>>>> include/drm/drm_mode_config.h | 5 ++++ >>>>>>>>>> include/uapi/drm/drm_mode.h | 28 +++++++++++++++++++ >>>>>>>>>> 8 files changed, 89 insertions(+), 2 deletions(-) >>>>> >>>>> ... >>>>> >>>>>>>>> The question about full vs. limited range seems unnecessary to me, as >>>>>>>>> the background color will be used as-is in the blending stage, so >>>>>>>>> userspace can just program the correct value that fits the pipeline it >>>>>>>>> is setting up. >>>>>>>>> >>>>>>>>> One more question is, as HDR exists, could we need background colors >>>>>>>>> with component values greater than 1.0? >>>>>>>> >>>>>>>> AR4H color format should cover that case, isn't it ? >>>>>>> >>>>>>> Yes, but with the inconvenience I mentioned. >>>>>>> >>>>>>> This is a genuine question though, would anyone actually need >>>>>>> background color values > 1.0. I don't know of any case yet where it >>>>>>> would be required. It would imply that plane blending happens in a >>>>>>> color space where >1.0 values are meaningful. I'm not even sure if any >>>>>>> hardware supporting that exists. >>>>>>> >>>>>>> Maybe it would be best to assume that only [0.0, 1.0] pixel value range >>>>>>> is useful, and mention in the commit message that if someone really >>>>>>> needs values outside of that, they should create another background >>>>>>> color property. Then, you can pick a simple unsigned integer pixel >>>>>>> format, too. (I didn't see any 16 bit-per-channel formats like that in >>>>>>> drm_fourcc.h though.) >>>>>>> >>>>>> >>>>>> I don't think we should artificially limit this to [0.0, 1.0]. As you >>>>>> mentioned above when talking about full vs limited, the userspace >>>>>> understands what's the correct value that fits the pipeline. If that >>>>>> pipeline is FP16 with > 1.0 values then it would make sense that the >>>>>> background color can be > 1.0. >>>>> >>>>> Ok. The standard FP32 format then for ease of use and guaranteed enough >>>>> range and precision for far into the future? >>>>> >>>> >>>> I don't have a strong preference for FP16 vs FP32. My understanding is >>>> that FP16 is enough to represent linearly encoded data in a way that >>>> looks smooth to humans. >>>> >>>> scRGB uses FP16 with linear encoding in a range of [-0.5, 7.4999]. >>>> >>>>> Or do you want to keep it in 64 bits total, so the UABI can pack >>>>> everything into a u64 instead of needing to create a blob? >>>>> >>>>> I don't mind as long as it's clearly documented what it is and how it >>>>> works, and it carries enough precision. >>>>> >>>>> But FP16 with its 10 bits of precision might be too little for integer >>>>> 12-16 bpc pipelines and sinks? >>> >>> The 10 bits worries me still. >>> >>> If you have a pipeline that works in [0.0, 1.0] range only, then FP16 >>> limits precision to 10 bits (in the upper half of the range?). >>> >>>>> >>>>> If the values can go beyond [0.0, 1.0] range, then does the blending >>>>> hardware and the degamma/ctm/gamma coming afterwards cope with them, or >>>>> do they get clamped anyway? >>>>> >>>> >>>> That probably depends on the HW and how it's configured. AMD HW can handle >>>> values above and below [0.0, 1.0]. >>> >>> Right, so how would userspace know what will happen? >>> >>> Or do we need to specify that while values outside that unit range are >>> expressable, it is hardware-specific on how they will behave, so >>> generic userspace should not attempt to use values outside of the unit >>> range? >>> >>> I guess this caveat should be documented for everything, not just for >>> background color? LUT inputs and outputs, CTM input and output ranges, >>> FB formats... >>> >> >> I'm not sure we should artificially limit the interface at this point, or >> document hypotheticals. At this point I don't even know whether going beyond >> [0.0, 1.0] would be a challenge for any HW that supports floating point >> formats. > > Exactly, we don't know. Yet we have to document how background color > works. If background color can express values outside of [0.0, 1.0], > the documentation must say something about it. > > If there is no way to know, then documentation must say you don't know > (or that it is hardware-specific, which to generic userspace is the > same thing). > > If userspace does not know what happens, then obviously it will avoid > using values it does not know what happens with. > > For example, let's say that blending can produce values outside of > [0.0, 1.0]. The next step in the pipeline is DEGAMMA, which is a 1D > LUT. How do you sample a 1D LUT with input values beyond [0.0, 1.0]? Do > you clamp them to the unit range? Does the clamping still happen even > when the LUT is in pass-through mode? > > And in general, how big or how negative values will actually go through > the pipeline? > > Of course the background color property should not document everything > above, but it must say something, like "The handling of values outside > of [0.0, 1.0] depends on the capabilities of the hardware blending > engine." That makes the handling unknown to generic userspace, but > userspace drivers could make use of it. > > The important bit is to understand that the background color values may > sometimes (when?) not reach the sink unmodified even if userspace has > configured the KMS pipeline to not modify them. > > I would expect that values in [0.0, 1.0] have no problem passing > through the KMS pipeline unharmed, and there are obvious expectations > about how a LUT or a CTM processes them. But as soon as values outside > of that range are possible, a whole slew of questions arises. The > documentation must not be silent, it must set expectations like "it's > hardware specific" if that's what it is. > Agreed. I think ultimately we don't know because we haven't gotten to use-cases like that. I'm fine with documentation stating "The handling of values outside of [0.0, 1.0] depends on the capabilities of the hardware blending engine." or "Handling of values outside [0.0, 1.0] is currently undefined." Harry > > Thanks, > pq >
