Re: [PATCH 1/2] drm: add crtc background color property

Harry Wentland
2021-07-13T13:56:56Z

On 2021-07-13 3:52 a.m., Pekka Paalanen wrote:
> On Mon, 12 Jul 2021 12:15:59 -0400
> Harry Wentland <harry.wentland@amd.com> 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@foss.st.com> 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@foss.st.com> 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@foss.st.com>
>>>>>> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>>>>>> ---
>>>>>> 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?
>
> 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].

Harry

>
> Thanks,
> pq