[PATCH] drm/imx: dcss: Silence blend mode not set boot warning

[PATCH] drm/imx: dcss: Silence blend mode not set boot warning

[Laurentiu Palcu]

This addresses the following warning that appears at boot time:

--snip--
[PLANE:39:plane-0] pixel format with alpha exposed but blend mode not setup
WARNING: drivers/gpu/drm/drm_mode_config.c:872 at drm_mode_config_validate+0x36c/0x520 [drm]
--snip--

The warning started appearing after the commit mentioned below was
introduced. Add the alpha and blending properties to the primary plane.

Fixes: 860e748bddcc ("drm: ensure blend mode supported if pixel format with alpha exposed")
Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/gpu/drm/imx/dcss/dcss-crtc.c  |  6 ++++++
 drivers/gpu/drm/imx/dcss/dcss-dev.h   |  3 ++-
 drivers/gpu/drm/imx/dcss/dcss-dtg.c   | 12 ++++++++----
 drivers/gpu/drm/imx/dcss/dcss-plane.c | 11 ++++++++++-
 4 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/imx/dcss/dcss-crtc.c b/drivers/gpu/drm/imx/dcss/dcss-crtc.c
index ab41759a9f52d..8358219c94c0d 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-crtc.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-crtc.c
@@ -5,6 +5,7 @@
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_blend.h>
 #include <drm/drm_vblank.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -202,6 +203,11 @@ int dcss_crtc_init(struct dcss_crtc *crtc, struct drm_device *drm)
 		return ret;
 	}
 
+	drm_plane_create_alpha_property(&crtc->plane[0]->base);
+	drm_plane_create_blend_mode_property(&crtc->plane[0]->base,
+					     BIT(DRM_MODE_BLEND_PIXEL_NONE) |
+					     BIT(DRM_MODE_BLEND_COVERAGE));
+
 	crtc->irq = platform_get_irq_byname(pdev, "vblank");
 	if (crtc->irq < 0)
 		return crtc->irq;
diff --git a/drivers/gpu/drm/imx/dcss/dcss-dev.h b/drivers/gpu/drm/imx/dcss/dcss-dev.h
index b032e873d227c..9b3bd2f3a7ef1 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-dev.h
+++ b/drivers/gpu/drm/imx/dcss/dcss-dev.h
@@ -147,7 +147,8 @@ bool dcss_dtg_is_enabled(struct dcss_dtg *dtg);
 void dcss_dtg_ctxld_kick_irq_enable(struct dcss_dtg *dtg, bool en);
 bool dcss_dtg_global_alpha_changed(struct dcss_dtg *dtg, int ch_num, int alpha);
 void dcss_dtg_plane_alpha_set(struct dcss_dtg *dtg, int ch_num,
-			      const struct drm_format_info *format, int alpha);
+			      const struct drm_format_info *format, int alpha,
+			      unsigned int blend_mode);
 void dcss_dtg_plane_pos_set(struct dcss_dtg *dtg, int ch_num,
 			    int px, int py, int pw, int ph);
 void dcss_dtg_ch_enable(struct dcss_dtg *dtg, int ch_num, bool en);
diff --git a/drivers/gpu/drm/imx/dcss/dcss-dtg.c b/drivers/gpu/drm/imx/dcss/dcss-dtg.c
index 6bbfd9aa27aca..21da9d3cce4cb 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-dtg.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-dtg.c
@@ -3,6 +3,7 @@
  * Copyright 2019 NXP.
  */
 
+#include <drm/drm_blend.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
@@ -268,17 +269,20 @@ bool dcss_dtg_global_alpha_changed(struct dcss_dtg *dtg, int ch_num, int alpha)
 }
 
 void dcss_dtg_plane_alpha_set(struct dcss_dtg *dtg, int ch_num,
-			      const struct drm_format_info *format, int alpha)
+			      const struct drm_format_info *format, int alpha,
+			      unsigned int blend_mode)
 {
 	/* we care about alpha only when channel 0 is concerned */
 	if (ch_num)
 		return;
 
 	/*
-	 * Use global alpha if pixel format does not have alpha channel or the
-	 * user explicitly chose to use global alpha (i.e. alpha is not OPAQUE).
+	 * DCSS supports either global alpha or per-pixel blending, following
+	 * the coverage blending formula (with global alpha set to opaque).
+	 * When the pixel blend mode is PIXEL_NONE, the per-pixel alpha is
+	 * meant to be ignored, so fall back to global alpha in that case too.
 	 */
-	if (!format->has_alpha || alpha != 255)
+	if (!format->has_alpha || blend_mode == DRM_MODE_BLEND_PIXEL_NONE)
 		dtg->alpha_cfg = (alpha << DEFAULT_FG_ALPHA_POS) & DEFAULT_FG_ALPHA_MASK;
 	else /* use per-pixel alpha otherwise */
 		dtg->alpha_cfg = CH1_ALPHA_SEL;
diff --git a/drivers/gpu/drm/imx/dcss/dcss-plane.c b/drivers/gpu/drm/imx/dcss/dcss-plane.c
index 1746dc5d4b642..06413d0612349 100644
--- a/drivers/gpu/drm/imx/dcss/dcss-plane.c
+++ b/drivers/gpu/drm/imx/dcss/dcss-plane.c
@@ -209,6 +209,14 @@ static int dcss_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 	}
 
+	/* DCSS supports either per-pixel or global alpha. Not both. */
+	if (new_plane_state->fb->format->has_alpha &&
+	    new_plane_state->pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE &&
+	    new_plane_state->alpha != DRM_BLEND_ALPHA_OPAQUE) {
+		DRM_DEBUG_KMS("Both per-pixel and global alpha not supported.\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
@@ -336,7 +344,8 @@ static void dcss_plane_atomic_update(struct drm_plane *plane,
 	dcss_dtg_plane_pos_set(dcss->dtg, dcss_plane->ch_num,
 			       dst.x1, dst.y1, dst_w, dst_h);
 	dcss_dtg_plane_alpha_set(dcss->dtg, dcss_plane->ch_num,
-				 fb->format, new_state->alpha >> 8);
+				 fb->format, new_state->alpha >> 8,
+				 new_state->pixel_blend_mode);
 
 	if (!dcss_plane->ch_num && (new_state->alpha >> 8) == 0)
 		enable = false;
-- 
2.53.0

Re: [PATCH] drm/imx: dcss: Silence blend mode not set boot warning

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Initial pixel_blend_mode property is set to an unsupported value (PREMULTI), causing userspace compositor crashes.
- [High] Fast-path atomic commits silently drop alpha and pixel_blend_mode property updates.
--

--- Patch [1]: [PATCH] drm/imx: dcss: Silence blend mode not set boot warning ---
commit b8d70d6b989c5f19b4322dbd26927d62d307bf28
Author: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>

drm/imx: dcss: Silence blend mode not set boot warning

This addresses a boot warning about missing blend mode when a pixel format
with alpha is exposed. It adds the alpha and blending properties to the
primary plane.

> diff --git a/drivers/gpu/drm/imx/dcss/dcss-crtc.c b/drivers/gpu/drm/imx/dcss/dcss-crtc.c
> index ab41759a9f52d..8358219c94c0d 100644
> --- a/drivers/gpu/drm/imx/dcss/dcss-crtc.c
> +++ b/drivers/gpu/drm/imx/dcss/dcss-crtc.c
[ ... ]
> @@ -202,6 +203,11 @@ int dcss_crtc_init(struct dcss_crtc *crtc, struct drm_device *drm)
>  		return ret;
>  	}
>  
> +	drm_plane_create_alpha_property(&crtc->plane[0]->base);
> +	drm_plane_create_blend_mode_property(&crtc->plane[0]->base,
> +					     BIT(DRM_MODE_BLEND_PIXEL_NONE) |
> +					     BIT(DRM_MODE_BLEND_COVERAGE));

[Severity: High]
Does exposing only PIXEL_NONE and COVERAGE cause issues with the default plane
reset?

Looking at the driver, it uses the default drm_atomic_helper_plane_reset hook:

drivers/gpu/drm/imx/dcss/dcss-plane.c:
struct drm_plane_funcs dcss_plane_funcs = {
    ...
    .reset = drm_atomic_helper_plane_reset,
    ...
};

This core helper unconditionally initializes state->pixel_blend_mode to
DRM_MODE_BLEND_PREMULTI. If the property is created here with only
PIXEL_NONE and COVERAGE supported, will userspace compositors (like Weston)
receive an invalid enum value during startup and crash when querying
properties?

> +
>  	crtc->irq = platform_get_irq_byname(pdev, "vblank");
>  	if (crtc->irq < 0)
>  		return crtc->irq;
[ ... ]
> diff --git a/drivers/gpu/drm/imx/dcss/dcss-plane.c b/drivers/gpu/drm/imx/dcss/dcss-plane.c
> index 1746dc5d4b642..06413d0612349 100644
> --- a/drivers/gpu/drm/imx/dcss/dcss-plane.c
> +++ b/drivers/gpu/drm/imx/dcss/dcss-plane.c
[ ... ]
> @@ -336,7 +344,8 @@ static void dcss_plane_atomic_update(struct drm_plane *plane,
>  	dcss_dtg_plane_pos_set(dcss->dtg, dcss_plane->ch_num,
>  			       dst.x1, dst.y1, dst_w, dst_h);
>  	dcss_dtg_plane_alpha_set(dcss->dtg, dcss_plane->ch_num,
> -				 fb->format, new_state->alpha >> 8);
> +				 fb->format, new_state->alpha >> 8,
> +				 new_state->pixel_blend_mode);

[Severity: High]
Does the fast-path logic earlier in this function silently drop alpha and
pixel_blend_mode updates?

Earlier in dcss_plane_atomic_update(), there is an early return based on
dcss_plane_needs_setup():

    if (old_state->fb && !drm_atomic_crtc_needs_modeset(crtc_state) &&
        !dcss_plane_needs_setup(new_state, old_state)) {
        dcss_plane_atomic_set_base(dcss_plane);
        return;
    }

Since dcss_plane_needs_setup() only checks geometry, formatting, rotation, and
scaling:

    return state->crtc_x != old_state->crtc_x ||
           ...
           state->scaling_filter != old_state->scaling_filter;

Will an atomic commit that only modifies the alpha or pixel_blend_mode
properties cause dcss_plane_needs_setup() to return false?

This would result in returning early and skipping the
dcss_dtg_plane_alpha_set() hardware update entirely.

>  
>  	if (!dcss_plane->ch_num && (new_state->alpha >> 8) == 0)
>  		enable = false;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821122608.2077746-1-laurentiu.palcu@oss.nxp.com?part=1