* [PATCH v2 1/2] drm/drm_blend: allow blend mode property without PREMULTI
2026-05-04 14:06 [PATCH v2 0/2] drm/drm_blend: allow blend mode property without PREMULTI Leandro Ribeiro
@ 2026-05-04 14:06 ` Leandro Ribeiro
2026-05-04 22:17 ` Claude review: " Claude Code Review Bot
2026-05-04 14:06 ` [PATCH v2 2/2] drm: ensure blend mode supported if alpha exposed Leandro Ribeiro
2026-05-04 22:17 ` Claude review: drm/drm_blend: allow blend mode property without PREMULTI Claude Code Review Bot
2 siblings, 1 reply; 6+ messages in thread
From: Leandro Ribeiro @ 2026-05-04 14:06 UTC (permalink / raw)
To: dri-devel
Cc: airlied, daniels, maarten.lankhorst, mripard, pekka.paalanen,
simona, tzimmermann, ville.syrjala, linux-kernel
Some hardware only supports the COVERAGE blend mode and lacks PREMULTI
support entirely. DRM currently requires that PREMULTI is present when
creating a blend mode property, which prevents such drivers from being
properly upstreamed.
Remove this restriction and allow drivers to create a blend mode
property without PREMULTI, enabling support for hardware that
implements only COVERAGE blend mode.
This does not introduce a regression, as no existing upstream drivers
expose only COVERAGE. However, userspace that wants to support such kind
of hardware in the future will have to check the supported blend modes
instead of assuming PREMULTI is always supported.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
---
drivers/gpu/drm/drm_blend.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 1f3af27d2418..5ea0767f1cf5 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -563,10 +563,10 @@ EXPORT_SYMBOL(drm_atomic_normalize_zpos);
/**
* drm_plane_create_blend_mode_property - create a new blend mode property
* @plane: drm plane
- * @supported_modes: bitmask of supported modes, must include
- * BIT(DRM_MODE_BLEND_PREMULTI). Current DRM assumption is
- * that alpha is premultiplied, and old userspace can break if
- * the property defaults to anything else.
+ * @supported_modes: bitmask of supported modes. When
+ * BIT(DRM_MODE_BLEND_PREMULTI) is included, it will be used
+ * as the default. Otherwise, the default will fallback to one
+ * of the supported modes.
*
* This creates a new property describing the blend mode.
*
@@ -599,13 +599,13 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane,
{ DRM_MODE_BLEND_PREMULTI, "Pre-multiplied" },
{ DRM_MODE_BLEND_COVERAGE, "Coverage" },
};
+ unsigned int default_mode;
unsigned int valid_mode_mask = BIT(DRM_MODE_BLEND_PIXEL_NONE) |
BIT(DRM_MODE_BLEND_PREMULTI) |
BIT(DRM_MODE_BLEND_COVERAGE);
int i;
- if (WARN_ON((supported_modes & ~valid_mode_mask) ||
- ((supported_modes & BIT(DRM_MODE_BLEND_PREMULTI)) == 0)))
+ if (WARN_ON((supported_modes & ~valid_mode_mask)))
return -EINVAL;
prop = drm_property_create(dev, DRM_MODE_PROP_ENUM,
@@ -630,7 +630,14 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane,
}
}
- drm_object_attach_property(&plane->base, prop, DRM_MODE_BLEND_PREMULTI);
+ if (supported_modes & BIT(DRM_MODE_BLEND_PREMULTI))
+ default_mode = DRM_MODE_BLEND_PREMULTI;
+ else if (supported_modes & BIT(DRM_MODE_BLEND_COVERAGE))
+ default_mode = DRM_MODE_BLEND_COVERAGE;
+ else
+ default_mode = DRM_MODE_BLEND_PIXEL_NONE;
+
+ drm_object_attach_property(&plane->base, prop, default_mode);
plane->blend_mode_property = prop;
return 0;
--
2.54.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Claude review: drm/drm_blend: allow blend mode property without PREMULTI
2026-05-04 14:06 ` [PATCH v2 1/2] " Leandro Ribeiro
@ 2026-05-04 22:17 ` Claude Code Review Bot
0 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-04 22:17 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Verdict: Looks good, minor nit.**
The core change is clean and well-motivated. The WARN_ON condition is simplified to only reject invalid bits, and a fallback default selection is added:
```c
if (supported_modes & BIT(DRM_MODE_BLEND_PREMULTI))
default_mode = DRM_MODE_BLEND_PREMULTI;
else if (supported_modes & BIT(DRM_MODE_BLEND_COVERAGE))
default_mode = DRM_MODE_BLEND_COVERAGE;
else
default_mode = DRM_MODE_BLEND_PIXEL_NONE;
```
This fallback order is reasonable — PREMULTI first (backward compat), then COVERAGE, then PIXEL_NONE.
**Minor nit:** There is no check that `supported_modes != 0`. Passing zero would create a property with zero enum values and set `default_mode = DRM_MODE_BLEND_PIXEL_NONE` (which isn't even in the enum list). The old WARN_ON would have caught this since `BIT(DRM_MODE_BLEND_PREMULTI) == 0` evaluates true. Consider adding `!supported_modes` to the WARN_ON:
```c
if (WARN_ON(!supported_modes || (supported_modes & ~valid_mode_mask)))
return -EINVAL;
```
**Documentation update is good** — the `@supported_modes` doc now accurately reflects the new behavior.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] drm: ensure blend mode supported if alpha exposed
2026-05-04 14:06 [PATCH v2 0/2] drm/drm_blend: allow blend mode property without PREMULTI Leandro Ribeiro
2026-05-04 14:06 ` [PATCH v2 1/2] " Leandro Ribeiro
@ 2026-05-04 14:06 ` Leandro Ribeiro
2026-05-04 22:17 ` Claude review: " Claude Code Review Bot
2026-05-04 22:17 ` Claude review: drm/drm_blend: allow blend mode property without PREMULTI Claude Code Review Bot
2 siblings, 1 reply; 6+ messages in thread
From: Leandro Ribeiro @ 2026-05-04 14:06 UTC (permalink / raw)
To: dri-devel
Cc: airlied, daniels, maarten.lankhorst, mripard, pekka.paalanen,
simona, tzimmermann, ville.syrjala, linux-kernel
Before "drm/drm_blend: allow blend mode property without PREMULTI",
userspace would have to assume that only PREMULTI was supported by
drivers that didn't expose the blend mode property. But now userspace
shouldn't relly on that, as they can't count with drivers always
supporting PREMULTI.
Error out if a driver expose alpha property or pixel formats with alpha
and does not expose the blend mode property. This way userspace don't
have to guess. Drivers that hit such error must be fixed.
Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>
---
drivers/gpu/drm/drm_crtc_internal.h | 1 +
drivers/gpu/drm/drm_drv.c | 7 ++++-
drivers/gpu/drm/drm_mode_config.c | 42 +++++++++++++++++++++++++++++
3 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index c09409229644..bdbb6b9b94ea 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -96,6 +96,7 @@ int drm_mode_setcrtc(struct drm_device *dev,
int drm_modeset_register_all(struct drm_device *dev);
void drm_modeset_unregister_all(struct drm_device *dev);
void drm_mode_config_validate(struct drm_device *dev);
+int drm_mode_config_enforce(struct drm_device *dev);
/* drm_modes.c */
const char *drm_get_mode_status_name(enum drm_mode_status status);
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 985c283cf59f..76766a370650 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -1059,9 +1059,14 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
const struct drm_driver *driver = dev->driver;
int ret;
- if (!driver->load)
+ if (!driver->load) {
drm_mode_config_validate(dev);
+ ret = drm_mode_config_enforce(dev);
+ if (ret)
+ return ret;
+ }
+
WARN_ON(!dev->managed.final_kfree);
if (drm_dev_needs_global_mutex(dev))
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 66f7dc37b597..20fd26ecb957 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -738,3 +738,45 @@ void drm_mode_config_validate(struct drm_device *dev)
"Must have as many primary planes as there are CRTCs, but have %u primary planes and %u CRTCs",
num_primary, dev->mode_config.num_crtc);
}
+
+static int plane_require_blend_mode_for_alpha(struct drm_plane *plane)
+{
+ struct drm_device *dev = plane->dev;
+ const struct drm_format_info *fmt;
+ u32 i;
+
+ /* blend mode property supported, no need to check anything */
+ if (plane->blend_mode_property)
+ return 0;
+
+ if (plane->alpha_property) {
+ drm_err(dev, "[PLANE:%d:%s] alpha property exposed but blend mode not setup",
+ plane->base.id, plane->name);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < plane->format_count; i++) {
+ fmt = drm_format_info(plane->format_types[i]);
+ if (fmt->has_alpha) {
+ drm_err(dev, "[PLANE:%d:%s] pixel format with alpha exposed but blend mode not setup",
+ plane->base.id, plane->name);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+int drm_mode_config_enforce(struct drm_device *dev)
+{
+ struct drm_plane *plane;
+ int ret;
+
+ drm_for_each_plane(plane, dev) {
+ ret = plane_require_blend_mode_for_alpha(plane);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
--
2.54.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Claude review: drm: ensure blend mode supported if alpha exposed
2026-05-04 14:06 ` [PATCH v2 2/2] drm: ensure blend mode supported if alpha exposed Leandro Ribeiro
@ 2026-05-04 22:17 ` Claude Code Review Bot
0 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-04 22:17 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Verdict: Needs rework — will break existing drivers.**
The design intent is correct: if a plane exposes alpha (either through the alpha property or alpha-containing pixel formats), userspace needs to know the blend mode. Enforcing this at driver registration time is the right place.
**Problem 1: Breaks existing in-tree drivers.** As noted above, at least 6 drivers set `alpha_property` without `blend_mode_property`, and many more support ARGB formats without blend mode. These drivers currently work fine. After this patch, `drm_dev_register()` returns `-EINVAL` and the driver fails to load entirely. The commit message acknowledges this ("Drivers that hit such error must be fixed") but does not include the fixes.
This violates the kernel's regression policy — you cannot merge a change that breaks existing working drivers and expect the fixes to come later. The fixes for all affected drivers must be part of this series (or merged first).
**Problem 2: The pixel format alpha check is extremely broad.** Nearly every DRM driver supports ARGB8888 or similar formats. The `has_alpha` check in `plane_require_blend_mode_for_alpha()` will catch a very large number of drivers that don't set up blend mode. This needs careful auditing of every DRM driver in the tree.
**Problem 3: Naming convention.** The split between `drm_mode_config_validate()` (WARN-based, non-fatal) and `drm_mode_config_enforce()` (error-return, fatal) introduces a new pattern. It would be worth documenting why some checks are hard errors and others are soft warnings, or alternatively, consider making this a WARN for now and hardening it later once all drivers are fixed.
**Minor issues:**
- Typo in commit message: "relly" should be "rely", "don't" should be "doesn't", "count with" should be "count on".
- Alignment uses spaces instead of tab at line 373 of the diff:
```c
drm_err(dev, "[PLANE:%d:%s] pixel format with alpha exposed but blend mode not setup",
plane->base.id, plane->name);
```
The `plane->base.id` line appears to use spaces for alignment — should use tabs per kernel coding style (compare with the `alpha_property` error message a few lines above which uses the correct tab alignment).
- The function `drm_mode_config_enforce` is not exported (`EXPORT_SYMBOL`) which is fine since it's only called from `drm_drv.c`, but it also lacks a kerneldoc comment. Since `drm_mode_config_validate()` also lacks one, this is consistent, but worth noting.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread
* Claude review: drm/drm_blend: allow blend mode property without PREMULTI
2026-05-04 14:06 [PATCH v2 0/2] drm/drm_blend: allow blend mode property without PREMULTI Leandro Ribeiro
2026-05-04 14:06 ` [PATCH v2 1/2] " Leandro Ribeiro
2026-05-04 14:06 ` [PATCH v2 2/2] drm: ensure blend mode supported if alpha exposed Leandro Ribeiro
@ 2026-05-04 22:17 ` Claude Code Review Bot
2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-04 22:17 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/drm_blend: allow blend mode property without PREMULTI
Author: Leandro Ribeiro <leandro.ribeiro@collabora.com>
Patches: 3
Reviewed: 2026-05-05T08:17:26.131070
---
This is a 2-patch series to relax the DRM blend mode property creation API so that drivers whose hardware only supports COVERAGE (no PREMULTI) can expose blend mode. Patch 1 removes the PREMULTI requirement; patch 2 adds a hard enforcement that any driver exposing alpha (via alpha property or pixel formats with alpha channel) **must** also expose the blend mode property, or `drm_dev_register()` will fail.
The motivation is sound — hardware exists that only does COVERAGE blending, and the current API artificially prevents those drivers from being upstreamed. However, **patch 2 has a serious problem**: the enforcement will break multiple existing in-tree drivers that are currently working and do not call `drm_plane_create_blend_mode_property()`.
**Affected drivers found** (alpha_property set but no blend_mode_property):
- `sun4i/sun8i_ui_layer.c`
- `sun4i/sun8i_vi_layer.c`
- `atmel-hlcdc/atmel_hlcdc_plane.c`
- `stm/ltdc.c`
- `xlnx/zynqmp_kms.c`
- `logicvc/logicvc_layer.c`
None of these use the deprecated `.load` callback, so the enforcement is not bypassed. Additionally, the pixel-format alpha check (`fmt->has_alpha`) will catch even more drivers — virtually any driver that lists ARGB8888 among its supported formats but doesn't set up blend mode. This is an extremely broad net that could break many more drivers beyond the alpha_property ones listed above.
The series needs either: (a) companion patches to fix all affected in-tree drivers before the enforcement is turned on, or (b) a softer enforcement (WARN instead of error-return) with a plan to harden it later.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 6+ messages in thread