* [PATCH v8 0/4] Introduce BACKGROUND_COLOR DRM CRTC property
@ 2026-03-03 19:24 Cristian Ciocaltea
2026-03-03 19:24 ` [PATCH v8 1/4] uapi: Provide DIV_ROUND_CLOSEST() Cristian Ciocaltea
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Cristian Ciocaltea @ 2026-03-03 19:24 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Sandy Huang, Heiko Stübner, Andy Yan,
Louis Chauvet, Haneen Mohammed, Melissa Wen, Jani Nikula,
Andy Shevchenko
Cc: Robert Mader, kernel, dri-devel, linux-kernel, linux-arm-kernel,
linux-rockchip, Nícolas F. R. A. Prado, Diederik de Haas,
AngeloGioacchino Del Regno, Matt Roper, Andy Yan
Some display controllers can be hardware-configured to present non-black
colors for pixels which are not covered by any plane (or are exposed
through transparent regions of higher planes).
The 1st patch of the series provides DIV_ROUND_CLOSEST() to uapi, as a
prerequisite to the 2nd patch introducing the BACKGROUND_COLOR DRM
property that can be attached to a CRTC via a dedicated helper function.
A 64-bit ARGB color value format is also defined and can be manipulated
with the help of a few utility macros.
Note this is a reworked version of the patch [1] submitted (many) years
ago by Matt Roper. The main changes are:
* Renamed DRM_ARGB_<COMP>() to DRM_ARGB64_GET<C>_BPC() while providing
convenience wrappers to extract all 16 bits of a specific color via
DRM_ARGB64_GET<C>()
* Replaced drm_argb() function with DRM_ARGB64_PREP_BPC() macro, to
improve uAPI consistency and readability; additionally fixed a bug in
case of using bpc < 16: the unused least-significant bits of a given
component in the output value would contain the unused
most-significant bits of the following component in the input value,
instead of being set to 0
* Replaced GENMASK_ULL(63, 0) with U64_MAX when calling
drm_property_create_range() to create the BACKGROUND_COLOR property
* Moved crtc_state->bgcolor initialization from
__drm_atomic_helper_crtc_reset() to
__drm_atomic_helper_crtc_state_reset()
* Replaced '*bgcolor*' occurrences to '*background_color*' for
consistency with the actual property name in both storage field and
helper functions names
The subsequent patches add background color support to VKMS and the VOP2
display controller used in the RK3568, RK3576, and RK3588 Rockchip SoC
families.
The validation has been done using a dedicated IGT test [2] - see the
reported results below.
On the userland side, a Weston merge request [3] is available, providing
support for the BACKGROUND_COLOR CRTC property to the DRM backend. It
relies on the already existing background-color setting in weston.ini:
[shell]
background-color=0xAARRGGBB
[1] https://lore.kernel.org/all/20190930224707.14904-2-matthew.d.roper@intel.com/
[2] https://lore.kernel.org/all/20251219-crtc-bgcolor-v3-1-31b589911588@collabora.com/
[3] https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1845
IGT kms_crtc_background_color test results
==========================================
* VKMS
virtme-ng$ IGT_FORCE_DRIVER=vkms build/tests/kms_crtc_background_color
IGT-Version: 2.3-g6b07138e8 (x86_64) (Linux: 6.19.0-rc1-virtme x86_64)
Using IGT_SRANDOM=1766149634 for randomisation
Opened device: /dev/dri/card0
Starting subtest: background-color-red
Starting dynamic subtest: pipe-A-Virtual-1
Dynamic subtest pipe-A-Virtual-1: SUCCESS (0.071s)
Subtest background-color-red: SUCCESS (0.073s)
Starting subtest: background-color-green
Starting dynamic subtest: pipe-A-Virtual-1
Dynamic subtest pipe-A-Virtual-1: SUCCESS (0.074s)
Subtest background-color-green: SUCCESS (0.074s)
Starting subtest: background-color-blue
Starting dynamic subtest: pipe-A-Virtual-1
Dynamic subtest pipe-A-Virtual-1: SUCCESS (0.074s)
Subtest background-color-blue: SUCCESS (0.074s)
Starting subtest: background-color-yellow
Starting dynamic subtest: pipe-A-Virtual-1
Dynamic subtest pipe-A-Virtual-1: SUCCESS (0.072s)
Subtest background-color-yellow: SUCCESS (0.073s)
Starting subtest: background-color-purple
Starting dynamic subtest: pipe-A-Virtual-1
Dynamic subtest pipe-A-Virtual-1: SUCCESS (0.072s)
Subtest background-color-purple: SUCCESS (0.074s)
Starting subtest: background-color-cyan
Starting dynamic subtest: pipe-A-Virtual-1
Dynamic subtest pipe-A-Virtual-1: SUCCESS (0.074s)
Subtest background-color-cyan: SUCCESS (0.074s)
Starting subtest: background-color-black
Starting dynamic subtest: pipe-A-Virtual-1
Dynamic subtest pipe-A-Virtual-1: SUCCESS (0.072s)
Subtest background-color-black: SUCCESS (0.072s)
Starting subtest: background-color-white
Starting dynamic subtest: pipe-A-Virtual-1
Dynamic subtest pipe-A-Virtual-1: SUCCESS (0.073s)
Subtest background-color-white: SUCCESS (0.074s)
* Radxa ROCK 5B (RK3588)
rock5b$ build/tests/kms_crtc_background_color --device drm:/dev/dri/card1
IGT-Version: 2.2-g3e4ec308e (aarch64) (Linux: 6.18.0-rc1 aarch64)
Using IGT_SRANDOM=1762774806 for randomisation
Opened device: /dev/dri/card1
Starting subtest: background-color-red
Starting dynamic subtest: pipe-C-DP-1
Dynamic subtest pipe-C-DP-1: SUCCESS (0.491s)
Subtest background-color-red: SUCCESS (0.493s)
Starting subtest: background-color-green
Starting dynamic subtest: pipe-C-DP-1
Dynamic subtest pipe-C-DP-1: SUCCESS (0.533s)
Subtest background-color-green: SUCCESS (0.535s)
Starting subtest: background-color-blue
Starting dynamic subtest: pipe-C-DP-1
Dynamic subtest pipe-C-DP-1: SUCCESS (0.541s)
Subtest background-color-blue: SUCCESS (0.544s)
Starting subtest: background-color-yellow
Starting dynamic subtest: pipe-C-DP-1
Dynamic subtest pipe-C-DP-1: SUCCESS (0.535s)
Subtest background-color-yellow: SUCCESS (0.537s)
Starting subtest: background-color-purple
Starting dynamic subtest: pipe-C-DP-1
Dynamic subtest pipe-C-DP-1: SUCCESS (0.536s)
Subtest background-color-purple: SUCCESS (0.538s)
Starting subtest: background-color-cyan
Starting dynamic subtest: pipe-C-DP-1
Dynamic subtest pipe-C-DP-1: SUCCESS (0.539s)
Subtest background-color-cyan: SUCCESS (0.541s)
Starting subtest: background-color-black
Starting dynamic subtest: pipe-C-DP-1
(kms_crtc_background_color:744) igt_pipe_crc-WARNING: Warning on condition all_zero in function crc_sanity_checks, file ../lib/igt_pipe_crc.c:475
(kms_crtc_background_color:744) igt_pipe_crc-WARNING: Suspicious CRC: All values are 0.
(kms_crtc_background_color:744) igt_pipe_crc-WARNING: Warning on condition all_zero in function crc_sanity_checks, file ../lib/igt_pipe_crc.c:475
(kms_crtc_background_color:744) igt_pipe_crc-WARNING: Suspicious CRC: All values are 0.
Dynamic subtest pipe-C-DP-1: SUCCESS (0.535s)
Subtest background-color-black: SUCCESS (0.537s)
Starting subtest: background-color-white
Starting dynamic subtest: pipe-C-DP-1
Dynamic subtest pipe-C-DP-1: SUCCESS (0.540s)
Subtest background-color-white: SUCCESS (0.542s)
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Changes in v8:
- Collected R-b tags from Louis Chauvet and Andy Yan
- Handled unsupported alpha-blending with background color in
vop2_plane_atomic_check()
- Rebased series onto the very recent drm-misc-next
- Link to v7: https://lore.kernel.org/r/20260204-rk3588-bgcolor-v7-0-78d1d01c5ca1@collabora.com
Changes in v7:
- Consistently put "({" on a separate line for all macro definitions
(Andy Shevchenko)
- Rebased series onto latest drm-misc-next
- Link to v6: https://lore.kernel.org/r/20260129-rk3588-bgcolor-v6-0-c15f755a4055@collabora.com
Changes in v6:
- Collected Acked-by & Reviewed-by tags from Andy S & Angelo
- Handled feedback from Andy Shevchenko
* Fixed up styling for __KERNEL_DIV_ROUND_CLOSEST() macro
* Made use of __GENMASK() helper in __DRM_ARGB64_PREP*() and
__DRM_ARGB64_GET*() definitions
* Introduced DRM_ARGB64_GET*_BPCS() as an alternative for
DRM_ARGB64_GET*_BPC() to help when performance is more important
than accuracy, e.g. used it along with FIELD_MODIFY() in the vop2
related patch to simplify a bit the bgcolor operations
- Link to v5: https://lore.kernel.org/r/20260127-rk3588-bgcolor-v5-0-b25aa8613211@collabora.com
Changes in v5:
- Collected Reviewed-by & Tested-by tags from Nícolas & Diederik
- Dumped background_color prop value in drm_atomic_crtc_print_state()
and updated comment in drm_crtc_state (Nícolas)
- Documented the reasons of not using the DRM_ARGB64_GET*_BPC() helpers
in vop2 related patch (Nícolas)
- Rebased series onto latest drm-misc-next
- Link to v4: https://lore.kernel.org/r/20251219-rk3588-bgcolor-v4-0-2ff1127ea757@collabora.com
Changes in v4:
- Switched to simple bit-shifting approach when performing the bpc
conversion in the vop2 driver, to avoid the expensive division since
we shouldn't be concerned anymore about the precision (Chaoyi)
- Rebased series onto latest drm-misc-next
- Link to v3: https://lore.kernel.org/r/20251118-rk3588-bgcolor-v3-0-a2cc909428ea@collabora.com
Changes in v3:
- Added new patches:
* uapi: Provide DIV_ROUND_CLOSEST()
* drm/vkms: Support setting custom background color
- Improved DRM_ARGB64_{PREP|GET}*() helpers by using a conversion ratio
for better color approximation when dealing with less than 16 bits of
precision
- Mentioned the IGT test in the cover letter while documenting the
validation results; also dropped references to the now useless
modetest wrapper script and its generated report
- Rebased series onto latest drm-misc-next
- Link to v2: https://lore.kernel.org/r/20251013-rk3588-bgcolor-v2-0-25cc3810ba8c@collabora.com
Changes in v2:
- Improved uAPI consistency and readability by introducing
DRM_ARGB64_PREP*() and DRM_ARGB64_GET*() helper macros
- Updated several code comment sections
- Referenced the counterpart Weston support in the cover letter
- Rebased series onto v6.18-rc1
- Link to v1: https://lore.kernel.org/r/20250902-rk3588-bgcolor-v1-0-fd97df91d89f@collabora.com
---
Cristian Ciocaltea (4):
uapi: Provide DIV_ROUND_CLOSEST()
drm: Add CRTC background color property
drm/vkms: Support setting custom background color
drm/rockchip: vop2: Support setting custom background color
drivers/gpu/drm/drm_atomic.c | 1 +
drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
drivers/gpu/drm/drm_blend.c | 39 ++++++++++++--
drivers/gpu/drm/drm_mode_config.c | 6 +++
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 24 ++++++++-
drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 4 ++
drivers/gpu/drm/vkms/vkms_composer.c | 10 +++-
drivers/gpu/drm/vkms/vkms_crtc.c | 3 ++
include/drm/drm_blend.h | 4 +-
include/drm/drm_crtc.h | 12 +++++
include/drm/drm_mode_config.h | 5 ++
include/linux/math.h | 18 +------
include/uapi/drm/drm_mode.h | 80 ++++++++++++++++++++++++++++
include/uapi/linux/const.h | 18 +++++++
15 files changed, 204 insertions(+), 25 deletions(-)
---
base-commit: fca11428425e92bf21d4a7f5865708c5e64430e4
change-id: 20250829-rk3588-bgcolor-c1a7b9a507bc
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v8 1/4] uapi: Provide DIV_ROUND_CLOSEST()
2026-03-03 19:24 [PATCH v8 0/4] Introduce BACKGROUND_COLOR DRM CRTC property Cristian Ciocaltea
@ 2026-03-03 19:24 ` Cristian Ciocaltea
2026-03-03 20:54 ` Claude review: " Claude Code Review Bot
2026-03-03 19:24 ` [PATCH v8 2/4] drm: Add CRTC background color property Cristian Ciocaltea
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Cristian Ciocaltea @ 2026-03-03 19:24 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Sandy Huang, Heiko Stübner, Andy Yan,
Louis Chauvet, Haneen Mohammed, Melissa Wen, Jani Nikula,
Andy Shevchenko
Cc: Robert Mader, kernel, dri-devel, linux-kernel, linux-arm-kernel,
linux-rockchip, Nícolas F. R. A. Prado, Diederik de Haas,
AngeloGioacchino Del Regno
Currently DIV_ROUND_CLOSEST() is only available for the kernel via
include/linux/math.h.
Expose it to userland as well by adding __KERNEL_DIV_ROUND_CLOSEST() as
a common definition in uapi.
Additionally, ensure it allows building ISO C applications by switching
from the 'typeof' GNU extension to the ISO-friendly __typeof__.
Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Tested-by: Diederik de Haas <diederik@cknow-tech.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
include/linux/math.h | 18 +-----------------
include/uapi/linux/const.h | 18 ++++++++++++++++++
2 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/include/linux/math.h b/include/linux/math.h
index 6dc1d1d32fbc..1e8fb3efbc8c 100644
--- a/include/linux/math.h
+++ b/include/linux/math.h
@@ -89,23 +89,7 @@
} \
)
-/*
- * Divide positive or negative dividend by positive or negative divisor
- * and round to closest integer. Result is undefined for negative
- * divisors if the dividend variable type is unsigned and for negative
- * dividends if the divisor variable type is unsigned.
- */
-#define DIV_ROUND_CLOSEST(x, divisor)( \
-{ \
- typeof(x) __x = x; \
- typeof(divisor) __d = divisor; \
- (((typeof(x))-1) > 0 || \
- ((typeof(divisor))-1) > 0 || \
- (((__x) > 0) == ((__d) > 0))) ? \
- (((__x) + ((__d) / 2)) / (__d)) : \
- (((__x) - ((__d) / 2)) / (__d)); \
-} \
-)
+#define DIV_ROUND_CLOSEST __KERNEL_DIV_ROUND_CLOSEST
/*
* Same as above but for u64 dividends. divisor must be a 32-bit
* number.
diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h
index b8f629ef135f..565f309b9df8 100644
--- a/include/uapi/linux/const.h
+++ b/include/uapi/linux/const.h
@@ -50,4 +50,22 @@
#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
+/*
+ * Divide positive or negative dividend by positive or negative divisor
+ * and round to closest integer. Result is undefined for negative
+ * divisors if the dividend variable type is unsigned and for negative
+ * dividends if the divisor variable type is unsigned.
+ */
+#define __KERNEL_DIV_ROUND_CLOSEST(x, divisor) \
+({ \
+ __typeof__(x) __x = x; \
+ __typeof__(divisor) __d = divisor; \
+ \
+ (((__typeof__(x))-1) > 0 || \
+ ((__typeof__(divisor))-1) > 0 || \
+ (((__x) > 0) == ((__d) > 0))) ? \
+ (((__x) + ((__d) / 2)) / (__d)) : \
+ (((__x) - ((__d) / 2)) / (__d)); \
+})
+
#endif /* _UAPI_LINUX_CONST_H */
--
2.52.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v8 2/4] drm: Add CRTC background color property
2026-03-03 19:24 [PATCH v8 0/4] Introduce BACKGROUND_COLOR DRM CRTC property Cristian Ciocaltea
2026-03-03 19:24 ` [PATCH v8 1/4] uapi: Provide DIV_ROUND_CLOSEST() Cristian Ciocaltea
@ 2026-03-03 19:24 ` Cristian Ciocaltea
2026-03-03 20:54 ` Claude review: " Claude Code Review Bot
2026-03-03 19:24 ` [PATCH v8 3/4] drm/vkms: Support setting custom background color Cristian Ciocaltea
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Cristian Ciocaltea @ 2026-03-03 19:24 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Sandy Huang, Heiko Stübner, Andy Yan,
Louis Chauvet, Haneen Mohammed, Melissa Wen, Jani Nikula,
Andy Shevchenko
Cc: Robert Mader, kernel, dri-devel, linux-kernel, linux-arm-kernel,
linux-rockchip, Matt Roper, Nícolas F. R. A. Prado,
Diederik de Haas
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.
Co-developed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Tested-by: Diederik de Haas <diederik@cknow-tech.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/gpu/drm/drm_atomic.c | 1 +
drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
drivers/gpu/drm/drm_atomic_uapi.c | 4 ++
drivers/gpu/drm/drm_blend.c | 39 +++++++++++++--
drivers/gpu/drm/drm_mode_config.c | 6 +++
include/drm/drm_blend.h | 4 +-
include/drm/drm_crtc.h | 12 +++++
include/drm/drm_mode_config.h | 5 ++
include/uapi/drm/drm_mode.h | 80 +++++++++++++++++++++++++++++++
9 files changed, 147 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 04925166df98..4a1e77773f80 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -475,6 +475,7 @@ static void drm_atomic_crtc_print_state(struct drm_printer *p,
drm_printf(p, "\tconnector_mask=%x\n", state->connector_mask);
drm_printf(p, "\tencoder_mask=%x\n", state->encoder_mask);
drm_printf(p, "\tmode: " DRM_MODE_FMT "\n", DRM_MODE_ARG(&state->mode));
+ drm_printf(p, "\tbackground_color=%llx\n", state->background_color);
if (crtc->funcs->atomic_print_state)
crtc->funcs->atomic_print_state(p, state);
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index bd6faa09f83b..76746ad4a1bb 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -75,6 +75,7 @@ __drm_atomic_helper_crtc_state_reset(struct drm_crtc_state *crtc_state,
struct drm_crtc *crtc)
{
crtc_state->crtc = crtc;
+ crtc_state->background_color = DRM_ARGB64_PREP(0xffff, 0, 0, 0);
}
EXPORT_SYMBOL(__drm_atomic_helper_crtc_state_reset);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 87de41fb4459..5bd5bf6661df 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -454,6 +454,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
&replaced);
state->color_mgmt_changed |= replaced;
return ret;
+ } else if (property == config->background_color_property) {
+ state->background_color = val;
} else if (property == config->prop_out_fence_ptr) {
s32 __user *fence_ptr = u64_to_user_ptr(val);
@@ -501,6 +503,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = (state->ctm) ? state->ctm->base.id : 0;
else if (property == config->gamma_lut_property)
*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
+ else if (property == config->background_color_property)
+ *val = state->background_color;
else if (property == config->prop_out_fence_ptr)
*val = 0;
else if (property == crtc->scaling_filter_property)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 3b1f5f72885e..1f3af27d2418 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -191,10 +191,6 @@
* plane does not expose the "alpha" property, then this is
* assumed to be 1.0
*
- * Note that all the property extensions described here apply either to the
- * plane or the CRTC (e.g. for the background color, which currently is not
- * exposed and assumed to be black).
- *
* SCALING_FILTER:
* Indicates scaling filter to be used for plane scaler
*
@@ -207,6 +203,25 @@
*
* Drivers can set up this property for a plane by calling
* drm_plane_create_scaling_filter_property
+ *
+ * The property extensions described above all apply to the plane. Drivers
+ * may also expose the following crtc property extension:
+ *
+ * BACKGROUND_COLOR:
+ * Background color is set up with drm_crtc_attach_background_color_property(),
+ * and expects a 64-bit ARGB value following DRM_FORMAT_ARGB16161616, as
+ * generated by the DRM_ARGB64_PREP*() helpers. It controls the color of a
+ * full-screen layer that exists below all planes. This color will be used
+ * for pixels not covered by any plane and may also be blended with plane
+ * contents as allowed by a plane's alpha values.
+ * The background color defaults to black, and is assumed to be black for
+ * drivers that do not expose this property. Although background color
+ * isn't a plane, it is assumed that the color provided here undergoes the
+ * CRTC degamma/CSC/gamma transformations applied after the planes blending.
+ * Note that the color value includes an alpha channel, hence non-opaque
+ * background color values are allowed, but since physically transparent
+ * monitors do not (yet) exists, the final alpha value may not reach the
+ * video sink or it may simply ignore it.
*/
/**
@@ -621,3 +636,19 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane,
return 0;
}
EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
+
+/**
+ * drm_crtc_attach_background_color_property - attach background color property
+ * @crtc: drm crtc
+ *
+ * Attaches the background color property to @crtc. The property defaults to
+ * solid black and will accept 64-bit ARGB values in the format generated by
+ * DRM_ARGB64_PREP*() helpers.
+ */
+void drm_crtc_attach_background_color_property(struct drm_crtc *crtc)
+{
+ drm_object_attach_property(&crtc->base,
+ crtc->dev->mode_config.background_color_property,
+ DRM_ARGB64_PREP(0xffff, 0, 0, 0));
+}
+EXPORT_SYMBOL(drm_crtc_attach_background_color_property);
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 84ae8a23a367..66f7dc37b597 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -380,6 +380,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
return -ENOMEM;
dev->mode_config.gamma_lut_size_property = prop;
+ prop = drm_property_create_range(dev, 0,
+ "BACKGROUND_COLOR", 0, U64_MAX);
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.background_color_property = prop;
+
prop = drm_property_create(dev,
DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB,
"IN_FORMATS", 0);
diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
index 88bdfec3bd88..c7e888767c81 100644
--- a/include/drm/drm_blend.h
+++ b/include/drm/drm_blend.h
@@ -31,8 +31,9 @@
#define DRM_MODE_BLEND_COVERAGE 1
#define DRM_MODE_BLEND_PIXEL_NONE 2
-struct drm_device;
struct drm_atomic_state;
+struct drm_crtc;
+struct drm_device;
struct drm_plane;
static inline bool drm_rotation_90_or_270(unsigned int rotation)
@@ -58,4 +59,5 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
struct drm_atomic_state *state);
int drm_plane_create_blend_mode_property(struct drm_plane *plane,
unsigned int supported_modes);
+void drm_crtc_attach_background_color_property(struct drm_crtc *crtc);
#endif
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 66278ffeebd6..312fc1e745d2 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -274,6 +274,18 @@ struct drm_crtc_state {
*/
struct drm_property_blob *gamma_lut;
+ /**
+ * @background_color:
+ *
+ * RGB value representing the CRTC's background color. The background
+ * color (aka "canvas color") of a CRTC is the color that will be used
+ * for pixels not covered by a plane, or covered by transparent pixels
+ * of a plane. The value here should be built using DRM_ARGB64_PREP*()
+ * helpers, while the individual color components can be extracted with
+ * desired precision via the DRM_ARGB64_GET*() macros.
+ */
+ u64 background_color;
+
/**
* @target_vblank:
*
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 5e1dd0cfccde..687c0ee163d2 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -836,6 +836,11 @@ struct drm_mode_config {
* gamma LUT as supported by the driver (read-only).
*/
struct drm_property *gamma_lut_size_property;
+ /**
+ * @background_color_property: Optional CRTC property to set the
+ * background color.
+ */
+ struct drm_property *background_color_property;
/**
* @suggested_x_property: Optional connector property with a hint for
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 3693d82b5279..a4bdc4bd11bc 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -27,6 +27,9 @@
#ifndef _DRM_MODE_H
#define _DRM_MODE_H
+#include <linux/bits.h>
+#include <linux/const.h>
+
#include "drm.h"
#if defined(__cplusplus)
@@ -1549,6 +1552,83 @@ struct drm_mode_closefb {
__u32 pad;
};
+/*
+ * 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.
+ * Also note bpc must be greater than 0.
+ */
+#define __DRM_ARGB64_PREP(c, shift) \
+ (((__u64)(c) & __GENMASK(15, 0)) << (shift))
+
+#define __DRM_ARGB64_PREP_BPC(c, shift, bpc) \
+({ \
+ __u16 mask = __GENMASK((bpc) - 1, 0); \
+ __u16 conv = __KERNEL_DIV_ROUND_CLOSEST((mask & (c)) * \
+ __GENMASK(15, 0), mask);\
+ __DRM_ARGB64_PREP(conv, shift); \
+})
+
+#define DRM_ARGB64_PREP(alpha, red, green, blue) \
+( \
+ __DRM_ARGB64_PREP(alpha, 48) | \
+ __DRM_ARGB64_PREP(red, 32) | \
+ __DRM_ARGB64_PREP(green, 16) | \
+ __DRM_ARGB64_PREP(blue, 0) \
+)
+
+#define DRM_ARGB64_PREP_BPC(alpha, red, green, blue, bpc) \
+({ \
+ __typeof__(bpc) __bpc = bpc; \
+ __DRM_ARGB64_PREP_BPC(alpha, 48, __bpc) | \
+ __DRM_ARGB64_PREP_BPC(red, 32, __bpc) | \
+ __DRM_ARGB64_PREP_BPC(green, 16, __bpc) | \
+ __DRM_ARGB64_PREP_BPC(blue, 0, __bpc); \
+})
+
+/*
+ * Extract the specified color component from a standard 64-bit ARGB value.
+ *
+ * If the requested precision is less than 16 bits, make use of a conversion
+ * ratio calculated as (2^bpc - 1) / (2^16 - 1), where bpc and 16 are the
+ * output and input precision, respectively.
+ *
+ * If speed is more important than accuracy, use DRM_ARGB64_GET*_BPCS()
+ * instead of DRM_ARGB64_GET*_BPC() in order to replace the expensive
+ * division with a simple bit right-shift operation.
+ */
+#define __DRM_ARGB64_GET(c, shift) \
+ ((__u16)(((__u64)(c) >> (shift)) & __GENMASK(15, 0)))
+
+#define __DRM_ARGB64_GET_BPC(c, shift, bpc) \
+({ \
+ __u16 comp = __DRM_ARGB64_GET(c, shift); \
+ __KERNEL_DIV_ROUND_CLOSEST(comp * __GENMASK((bpc) - 1, 0), \
+ __GENMASK(15, 0)); \
+})
+
+#define __DRM_ARGB64_GET_BPCS(c, shift, bpc) \
+ (__DRM_ARGB64_GET(c, shift) >> (16 - (bpc)))
+
+#define DRM_ARGB64_GETA(c) __DRM_ARGB64_GET(c, 48)
+#define DRM_ARGB64_GETR(c) __DRM_ARGB64_GET(c, 32)
+#define DRM_ARGB64_GETG(c) __DRM_ARGB64_GET(c, 16)
+#define DRM_ARGB64_GETB(c) __DRM_ARGB64_GET(c, 0)
+
+#define DRM_ARGB64_GETA_BPC(c, bpc) __DRM_ARGB64_GET_BPC(c, 48, bpc)
+#define DRM_ARGB64_GETR_BPC(c, bpc) __DRM_ARGB64_GET_BPC(c, 32, bpc)
+#define DRM_ARGB64_GETG_BPC(c, bpc) __DRM_ARGB64_GET_BPC(c, 16, bpc)
+#define DRM_ARGB64_GETB_BPC(c, bpc) __DRM_ARGB64_GET_BPC(c, 0, bpc)
+
+#define DRM_ARGB64_GETA_BPCS(c, bpc) __DRM_ARGB64_GET_BPCS(c, 48, bpc)
+#define DRM_ARGB64_GETR_BPCS(c, bpc) __DRM_ARGB64_GET_BPCS(c, 32, bpc)
+#define DRM_ARGB64_GETG_BPCS(c, bpc) __DRM_ARGB64_GET_BPCS(c, 16, bpc)
+#define DRM_ARGB64_GETB_BPCS(c, bpc) __DRM_ARGB64_GET_BPCS(c, 0, bpc)
+
#if defined(__cplusplus)
}
#endif
--
2.52.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v8 3/4] drm/vkms: Support setting custom background color
2026-03-03 19:24 [PATCH v8 0/4] Introduce BACKGROUND_COLOR DRM CRTC property Cristian Ciocaltea
2026-03-03 19:24 ` [PATCH v8 1/4] uapi: Provide DIV_ROUND_CLOSEST() Cristian Ciocaltea
2026-03-03 19:24 ` [PATCH v8 2/4] drm: Add CRTC background color property Cristian Ciocaltea
@ 2026-03-03 19:24 ` Cristian Ciocaltea
2026-03-03 20:54 ` Claude review: " Claude Code Review Bot
2026-03-03 19:24 ` [PATCH v8 4/4] drm/rockchip: vop2: " Cristian Ciocaltea
2026-03-03 20:54 ` Claude review: Introduce BACKGROUND_COLOR DRM CRTC property Claude Code Review Bot
4 siblings, 1 reply; 10+ messages in thread
From: Cristian Ciocaltea @ 2026-03-03 19:24 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Sandy Huang, Heiko Stübner, Andy Yan,
Louis Chauvet, Haneen Mohammed, Melissa Wen, Jani Nikula,
Andy Shevchenko
Cc: Robert Mader, kernel, dri-devel, linux-kernel, linux-arm-kernel,
linux-rockchip, Nícolas F. R. A. Prado, Diederik de Haas
Make use of the BACKGROUND_COLOR CRTC property when filling the
background during blending. It already defaults to solid black.
Since the internal representation of the pixel color in VKMS relies on
16 bits of precision, use the newly introduced DRM_ARGB64_GET{R|G|B}()
helpers to access the individual components of the background color
property, which is compliant with DRM_FORMAT_ARGB16161616.
It's worth noting the alpha component is ignored, hence non-opaque
background colors are not supported.
Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Tested-by: Diederik de Haas <diederik@cknow-tech.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/gpu/drm/vkms/vkms_composer.c | 10 ++++++++--
drivers/gpu/drm/vkms/vkms_crtc.c | 3 +++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index cd85de4ffd03..83d217085ad0 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -475,8 +475,14 @@ static void blend(struct vkms_writeback_job *wb,
{
struct vkms_plane_state **plane = crtc_state->active_planes;
u32 n_active_planes = crtc_state->num_active_planes;
-
- const struct pixel_argb_u16 background_color = { .a = 0xffff };
+ u64 bgcolor = crtc_state->base.background_color;
+
+ const struct pixel_argb_u16 background_color = {
+ .a = 0xffff,
+ .r = DRM_ARGB64_GETR(bgcolor),
+ .g = DRM_ARGB64_GETG(bgcolor),
+ .b = DRM_ARGB64_GETB(bgcolor),
+ };
int crtc_y_limit = crtc_state->base.mode.vdisplay;
int crtc_x_limit = crtc_state->base.mode.hdisplay;
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index ba2ff353e1a9..35ddc553a5e6 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -4,6 +4,7 @@
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
+#include <drm/drm_blend.h>
#include <drm/drm_managed.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
@@ -227,6 +228,8 @@ struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *pri
drm_crtc_enable_color_mgmt(crtc, 0, false, VKMS_LUT_SIZE);
+ drm_crtc_attach_background_color_property(crtc);
+
spin_lock_init(&vkms_out->lock);
spin_lock_init(&vkms_out->composer_lock);
--
2.52.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v8 4/4] drm/rockchip: vop2: Support setting custom background color
2026-03-03 19:24 [PATCH v8 0/4] Introduce BACKGROUND_COLOR DRM CRTC property Cristian Ciocaltea
` (2 preceding siblings ...)
2026-03-03 19:24 ` [PATCH v8 3/4] drm/vkms: Support setting custom background color Cristian Ciocaltea
@ 2026-03-03 19:24 ` Cristian Ciocaltea
2026-03-03 20:54 ` Claude review: " Claude Code Review Bot
2026-03-03 20:54 ` Claude review: Introduce BACKGROUND_COLOR DRM CRTC property Claude Code Review Bot
4 siblings, 1 reply; 10+ messages in thread
From: Cristian Ciocaltea @ 2026-03-03 19:24 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Sandy Huang, Heiko Stübner, Andy Yan,
Louis Chauvet, Haneen Mohammed, Melissa Wen, Jani Nikula,
Andy Shevchenko
Cc: Robert Mader, kernel, dri-devel, linux-kernel, linux-arm-kernel,
linux-rockchip, Diederik de Haas, Andy Yan
The Rockchip VOP2 display controller allows configuring the background
color of each video output port.
Since a previous patch introduced the BACKGROUND_COLOR CRTC property,
which defaults to solid black, make use of it when programming the
hardware.
Note the maximum precision allowed by the display controller is 10bpc,
while the alpha component is not supported, hence ignored.
Tested-by: Diederik de Haas <diederik@cknow-tech.com>
Reviewed-by: Andy Yan <andyshrk@163.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 24 +++++++++++++++++++++++-
drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 4 ++++
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index a195f5c819a2..843c7ef979b2 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -1080,6 +1080,13 @@ static int vop2_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
}
+ if ((cstate->background_color << 16) &&
+ (fb->format->has_alpha || pstate->alpha != 0xffff)) {
+ drm_dbg_kms(vop2->drm,
+ "Alpha-blending with background color is unsupported\n");
+ return -EINVAL;
+ }
+
return 0;
}
@@ -1552,6 +1559,7 @@ static void vop2_post_config(struct drm_crtc *crtc)
struct vop2_video_port *vp = to_vop2_video_port(crtc);
struct vop2 *vop2 = vp->vop2;
struct drm_display_mode *mode = &crtc->state->adjusted_mode;
+ u64 bgcolor = crtc->state->background_color;
u16 vtotal = mode->crtc_vtotal;
u16 hdisplay = mode->crtc_hdisplay;
u16 hact_st = mode->crtc_htotal - mode->crtc_hsync_start;
@@ -1597,7 +1605,15 @@ static void vop2_post_config(struct drm_crtc *crtc)
vop2_vp_write(vp, RK3568_VP_POST_DSP_VACT_INFO_F1, val);
}
- vop2_vp_write(vp, RK3568_VP_DSP_BG, 0);
+ /*
+ * Background color is programmed with 10 bits of precision.
+ * Since performance is more important than accuracy here,
+ * make use of the DRM_ARGB64_GET*_BPCS() helpers.
+ */
+ val = FIELD_PREP(RK3568_VP_DSP_BG__DSP_BG_RED, DRM_ARGB64_GETR_BPCS(bgcolor, 10));
+ FIELD_MODIFY(RK3568_VP_DSP_BG__DSP_BG_GREEN, &val, DRM_ARGB64_GETG_BPCS(bgcolor, 10));
+ FIELD_MODIFY(RK3568_VP_DSP_BG__DSP_BG_BLUE, &val, DRM_ARGB64_GETB_BPCS(bgcolor, 10));
+ vop2_vp_write(vp, RK3568_VP_DSP_BG, val);
}
static int us_to_vertical_line(struct drm_display_mode *mode, int us)
@@ -1983,6 +1999,10 @@ static int vop2_crtc_state_dump(struct drm_crtc *crtc, struct seq_file *s)
drm_get_bus_format_name(vcstate->bus_format));
seq_printf(s, "\toutput_mode[%x]", vcstate->output_mode);
seq_printf(s, " color_space[%d]\n", vcstate->color_space);
+ seq_printf(s, "\tbackground color (10bpc): r=0x%x g=0x%x b=0x%x\n",
+ DRM_ARGB64_GETR_BPCS(cstate->background_color, 10),
+ DRM_ARGB64_GETG_BPCS(cstate->background_color, 10),
+ DRM_ARGB64_GETB_BPCS(cstate->background_color, 10));
seq_printf(s, " Display mode: %dx%d%s%d\n",
mode->hdisplay, mode->vdisplay, interlaced ? "i" : "p",
drm_mode_vrefresh(mode));
@@ -2471,6 +2491,8 @@ static int vop2_create_crtcs(struct vop2 *vop2)
return dev_err_probe(drm->dev, ret,
"crtc init for video_port%d failed\n", i);
+ drm_crtc_attach_background_color_property(&vp->crtc);
+
drm_crtc_helper_add(&vp->crtc, &vop2_crtc_helper_funcs);
if (vop2->lut_regs) {
const struct vop2_video_port_data *vp_data = &vop2_data->vp[vp->id];
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
index 9124191899ba..37722652844a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
@@ -658,6 +658,10 @@ enum dst_factor_mode {
#define RK3588_VP_CLK_CTRL__DCLK_OUT_DIV GENMASK(3, 2)
#define RK3588_VP_CLK_CTRL__DCLK_CORE_DIV GENMASK(1, 0)
+#define RK3568_VP_DSP_BG__DSP_BG_RED GENMASK(29, 20)
+#define RK3568_VP_DSP_BG__DSP_BG_GREEN GENMASK(19, 10)
+#define RK3568_VP_DSP_BG__DSP_BG_BLUE GENMASK(9, 0)
+
#define RK3568_VP_POST_SCL_CTRL__VSCALEDOWN BIT(1)
#define RK3568_VP_POST_SCL_CTRL__HSCALEDOWN BIT(0)
--
2.52.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Claude review: Introduce BACKGROUND_COLOR DRM CRTC property
2026-03-03 19:24 [PATCH v8 0/4] Introduce BACKGROUND_COLOR DRM CRTC property Cristian Ciocaltea
` (3 preceding siblings ...)
2026-03-03 19:24 ` [PATCH v8 4/4] drm/rockchip: vop2: " Cristian Ciocaltea
@ 2026-03-03 20:54 ` Claude Code Review Bot
4 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-03-03 20:54 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: Introduce BACKGROUND_COLOR DRM CRTC property
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Patches: 5
Reviewed: 2026-03-04T06:54:46.778075
---
This is a well-structured v8 series introducing BACKGROUND_COLOR as a standard DRM CRTC property, with driver implementations for VKMS (virtual/testing) and Rockchip VOP2 (real hardware). The series is now at v8 and has accumulated substantial review and testing (R-b from Louis Chauvet, Andy Yan, Nicolas Prado, Angelo Del Regno; Acked-by from Andy Shevchenko; Tested-by from Diederik de Haas). The cover letter is excellent, documenting test results from both VKMS and RK3588 hardware, and referencing the IGT test and Weston userspace support.
The architecture is sound: patch 1 provides a uapi math primitive, patch 2 introduces the core property and ARGB64 helpers, patches 3-4 add driver implementations. The property creation follows existing DRM patterns (standard property, opt-in attachment). The ARGB64 helper macros are well-designed with both accurate (division-based) and fast (shift-based) variants.
I have one concern with patch 4 regarding the alpha-blending rejection check that warrants discussion, plus a few minor style observations. Overall, this series looks close to ready.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 10+ messages in thread
* Claude review: uapi: Provide DIV_ROUND_CLOSEST()
2026-03-03 19:24 ` [PATCH v8 1/4] uapi: Provide DIV_ROUND_CLOSEST() Cristian Ciocaltea
@ 2026-03-03 20:54 ` Claude Code Review Bot
0 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-03-03 20:54 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Looks good**
Clean mechanical refactoring moving `DIV_ROUND_CLOSEST()` to a uapi-accessible location as `__KERNEL_DIV_ROUND_CLOSEST()`.
The switch from `typeof` to `__typeof__` is good practice for uapi headers. The commit message's claim about "building ISO C applications" is slightly loose since the statement expression `({...})` syntax is itself a GCC extension, but `__typeof__` is the correct choice for uapi.
Minor style nit: the replacement alias has no blank line separating it from the next comment block:
```c
+#define DIV_ROUND_CLOSEST __KERNEL_DIV_ROUND_CLOSEST
/*
* Same as above but for u64 dividends. divisor must be a 32-bit
```
A blank line between the `#define` and the `/*` comment would improve readability, though this is existing context so may not be worth a respin.
The uapi placement in `include/uapi/linux/const.h` next to the existing `__KERNEL_DIV_ROUND_UP` is appropriate. Tags look solid (Acked-by Andy Shevchenko, Reviewed-by Angelo).
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 10+ messages in thread
* Claude review: drm: Add CRTC background color property
2026-03-03 19:24 ` [PATCH v8 2/4] drm: Add CRTC background color property Cristian Ciocaltea
@ 2026-03-03 20:54 ` Claude Code Review Bot
0 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-03-03 20:54 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Looks good**
This is the core patch and it's well done.
**Property creation and plumbing**: The property is created as a standard property in `drm_mode_create_standard_properties()` with range `[0, U64_MAX]`, and attached to individual CRTCs via `drm_crtc_attach_background_color_property()`. This follows the established DRM pattern (similar to color management properties). The get/set in `drm_atomic_uapi.c` and state dump in `drm_atomic.c` are correct.
**Default initialization**: Setting the default to opaque black in `__drm_atomic_helper_crtc_state_reset()` is correct:
```c
+ crtc_state->background_color = DRM_ARGB64_PREP(0xffff, 0, 0, 0);
```
**ARGB64 macros**: The helper design is well thought out:
- `DRM_ARGB64_PREP()` / `DRM_ARGB64_PREP_BPC()` for packing
- `DRM_ARGB64_GET*()` / `DRM_ARGB64_GET*_BPC()` / `DRM_ARGB64_GET*_BPCS()` for extraction
- The BPC variants use proper rounding (`DIV_ROUND_CLOSEST`) for accuracy
- The BPCS variants use simple right-shift for performance
The scaling math is correct: `DIV_ROUND_CLOSEST(c * 0xFFFF, (2^bpc - 1))` properly maps the input range to 16-bit. I verified the intermediate multiplication won't overflow since the `__u16` operands promote to at least `unsigned long` when multiplied with `__GENMASK(15, 0)`.
**Documentation**: The updated kerneldoc in `drm_blend.c` clearly describes the property semantics, including the CRTC degamma/CSC/gamma pipeline placement and the note about alpha behavior. Good.
Minor: the state dump format `%llx` works but `0x%016llx` would produce more consistently readable output for a 64-bit ARGB value.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 10+ messages in thread
* Claude review: drm/vkms: Support setting custom background color
2026-03-03 19:24 ` [PATCH v8 3/4] drm/vkms: Support setting custom background color Cristian Ciocaltea
@ 2026-03-03 20:54 ` Claude Code Review Bot
0 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-03-03 20:54 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Looks good**
Clean, minimal implementation. Since VKMS uses 16-bit internal pixel representation, using the full-precision `DRM_ARGB64_GET{R,G,B}()` macros is the right choice:
```c
+ const struct pixel_argb_u16 background_color = {
+ .a = 0xffff,
+ .r = DRM_ARGB64_GETR(bgcolor),
+ .g = DRM_ARGB64_GETG(bgcolor),
+ .b = DRM_ARGB64_GETB(bgcolor),
+ };
```
Alpha is hardcoded to `0xffff` (opaque), which is documented in the commit message. The attachment in `vkms_crtc_init()` is properly placed after CRTC initialization. Having VKMS support this property is valuable for testing without requiring specific hardware.
R-b from Louis Chauvet (VKMS maintainer) is appropriate.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 10+ messages in thread
* Claude review: drm/rockchip: vop2: Support setting custom background color
2026-03-03 19:24 ` [PATCH v8 4/4] drm/rockchip: vop2: " Cristian Ciocaltea
@ 2026-03-03 20:54 ` Claude Code Review Bot
0 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-03-03 20:54 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Mostly good, one concern**
The register programming using `FIELD_PREP`/`FIELD_MODIFY` with the BPCS (shift-based) helpers is clean:
```c
+ val = FIELD_PREP(RK3568_VP_DSP_BG__DSP_BG_RED, DRM_ARGB64_GETR_BPCS(bgcolor, 10));
+ FIELD_MODIFY(RK3568_VP_DSP_BG__DSP_BG_GREEN, &val, DRM_ARGB64_GETG_BPCS(bgcolor, 10));
+ FIELD_MODIFY(RK3568_VP_DSP_BG__DSP_BG_BLUE, &val, DRM_ARGB64_GETB_BPCS(bgcolor, 10));
```
The register field definitions (three 10-bit fields in bits 29-0) look correct for VOP2 documentation. Using BPCS (shift instead of divide) is justified for a hot path like `vop2_post_config()`.
**Concern — alpha-blending rejection check:**
```c
+ if ((cstate->background_color << 16) &&
+ (fb->format->has_alpha || pstate->alpha != 0xffff)) {
+ drm_dbg_kms(vop2->drm,
+ "Alpha-blending with background color is unsupported\n");
+ return -EINVAL;
+ }
```
The `<< 16` trick to strip the alpha and check if RGB is non-zero is clever but somewhat obscure — a named helper or a mask like `bgcolor & 0x0000FFFFFFFFFFFF` would be clearer.
More substantively, this check may be **overly conservative**: it rejects any plane that has alpha (format or property) whenever the background is non-black, regardless of whether that plane's pixels actually interact with the background. For example, if plane A is a fully opaque bottom layer covering the entire screen, and plane B above it has `has_alpha`, the check would reject plane B even though plane B blends with plane A, not the background. I understand this is a hardware limitation being enforced conservatively, but it could break valid compositing scenarios with non-black backgrounds. Is the hardware truly incapable of handling this, or is it just that the background-to-first-plane blending is problematic? If the latter, this could be refined to only reject when the lowest z-order plane has alpha.
The state dump addition is a nice touch for debugging:
```c
+ seq_printf(s, "\tbackground color (10bpc): r=0x%x g=0x%x b=0x%x\n",
```
R-b from Andy Yan (Rockchip maintainer) is appropriate.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-03-03 20:54 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-03 19:24 [PATCH v8 0/4] Introduce BACKGROUND_COLOR DRM CRTC property Cristian Ciocaltea
2026-03-03 19:24 ` [PATCH v8 1/4] uapi: Provide DIV_ROUND_CLOSEST() Cristian Ciocaltea
2026-03-03 20:54 ` Claude review: " Claude Code Review Bot
2026-03-03 19:24 ` [PATCH v8 2/4] drm: Add CRTC background color property Cristian Ciocaltea
2026-03-03 20:54 ` Claude review: " Claude Code Review Bot
2026-03-03 19:24 ` [PATCH v8 3/4] drm/vkms: Support setting custom background color Cristian Ciocaltea
2026-03-03 20:54 ` Claude review: " Claude Code Review Bot
2026-03-03 19:24 ` [PATCH v8 4/4] drm/rockchip: vop2: " Cristian Ciocaltea
2026-03-03 20:54 ` Claude review: " Claude Code Review Bot
2026-03-03 20:54 ` Claude review: Introduce BACKGROUND_COLOR DRM CRTC property Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox