Re: [PATCH v6 2/4] drm: Add CRTC background color property

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, Jan 29, 2026 at 02:58:52AM +0200, Cristian Ciocaltea wrote:
> Some display controllers can be hardware programmed to show non-black
> colors for pixels that are either not covered by any plane or are
> exposed through transparent regions of higher planes.  This feature can
> help reduce memory bandwidth usage, e.g. in compositors managing a UI
> with a solid background color while using smaller planes to render the
> remaining content.
> 
> To support this capability, introduce the BACKGROUND_COLOR standard DRM
> mode property, which can be attached to a CRTC through the
> drm_crtc_attach_background_color_property() helper function.
> 
> Additionally, define a 64-bit ARGB format value to be built with the
> help of a couple of dedicated DRM_ARGB64_PREP*() helpers.  Individual
> color components can be extracted with desired precision using the
> corresponding DRM_ARGB64_GET*() macros.

...

> +/*
> + * Put 16-bit ARGB values into a standard 64-bit representation that can be
> + * used for ioctl parameters, inter-driver communication, etc.
> + *
> + * If the component values being provided contain less than 16 bits of
> + * precision, use a conversion ratio to get a better color approximation.
> + * The ratio is computed as (2^16 - 1) / (2^bpc - 1), where bpc and 16 are
> + * the input and output precision, respectively.

Not sure if you should explicitly mention that "bpc must not be 0"
(it can be derived from the "division by 0" in the given formula,
but still...).

> + */
> +#define __DRM_ARGB64_PREP(c, shift)					\
> +	(((__u64)(c) & __GENMASK(15, 0)) << (shift))
> +
> +#define __DRM_ARGB64_PREP_BPC(c, shift, bpc)({				\

Not sure if this is an accepted style in DRM, by I find it difficult
to follow. I would expect the "({" be on a separate line.

> +	__u16 mask = __GENMASK((bpc) - 1, 0);				\
> +	__u16 conv = __KERNEL_DIV_ROUND_CLOSEST((mask & (c)) *		\
> +						__GENMASK(15, 0), mask);\

The whole point of the first patch is to use it in the divisions by 2^n - 1.
Can we transform this to make it "divisions" by power-of-two?

     ...: def dbm2(c, bpc):
     ...:     m = (1 << bpc) - 1
     ...:     c1 = m & c
     ...:     r = c1 << (16 - bpc)
     ...:     for i in range(1, 16 // bpc):
     ...:         r = r + (c1 << (16 - (i + 1) * bpc))
     ...:     return r

The above is a Python version of PoC of this approximation. Basically
we transform the fraction X / (2^n - 1) to a chained version of
X / 2^n + X / 2^2n + ... X / 2^kn as derived from recurrent formula
of i+1:th iteration as Xi+1 = Xi / 2^n + Xi / (2^n * (2^n - 1)).

So, maybe that one should be used instead? (It may be thought through
on how to collapse the for-loop to maybe some bitops, but even with
a for-loop it might be faster than real division.)

Note, we have some (for sure more than one, I remember the same Q appeared to
me a few years ago) of the examples which may avoid division at all. I would
like to have this macro to be kernel wide (and UAPI seems also okay).

> +	__DRM_ARGB64_PREP(conv, shift);					\
> +})

-- 
With Best Regards,
Andy Shevchenko