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. Harry > > Thanks, > pq >
