[PATCH 0/4] drm/imx: fix TVE / HDMI coexistence

[PATCH 0/4] drm/imx: fix TVE / HDMI coexistence

[Dmitry Baryshkov]

Testing MCIMXHDMICARD on IMX53 QSRB platform revealed that the HDMI
output is broken. The TVE output (even unconnected) forces the PLL4
frequency, breaking the frequency selected for the HDMI output. Fix
those issue, making TVE stop grabbing the PLL4 frequency (by correctly
reporting if it is unconnected).

While we are at it, fix two plane-related issues, which are now reported
by DRM.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
Dmitry Baryshkov (4):
      drm/imx: ipuv3-plane: do not advertise separate alpha formats on DC planes
      drm/imx: ipuv3-plane: add the pixel blend mode property
      drm/imx: imx-tve: probe DDC before claiming the connector is connected
      gpu: ipu-v3: di: report an unattainable pixel clock

 drivers/gpu/drm/imx/ipuv3/imx-tve.c     |  1 +
 drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c | 49 +++++++++++++++++++++++++--------
 drivers/gpu/ipu-v3/ipu-di.c             | 16 +++++++++++
 3 files changed, 55 insertions(+), 11 deletions(-)
---
base-commit: 6b8c8af514d739d0335f5579b585e02babe8a727
change-id: 20260810-imx53-fix-hdmi-03129d798678

Best regards,
--  
With best wishes
Dmitry

[PATCH 1/4] drm/imx: ipuv3-plane: do not advertise separate alpha formats on DC planes

[Dmitry Baryshkov]

Separate alpha formats need a companion IDMAC channel, which only the
graphics (DP) channels have. ipu_channel_alpha_channel() has no case for
IPUV3_CHANNEL_MEM_DC_SYNC, so ipu_plane_get_resources() leaves alpha_ch
at NULL for planes on the DC path. Yet ipu_plane_rgb_formats[], used for
exactly those planes, still advertises the six *_A8 fourccs.

Userspace picking one of them gets past ipu_plane_atomic_check(), which
never looks at alpha_ch, and ipu_plane_atomic_update() then dereferences
the NULL channel in ipu_cpmem_zero(). On i.MX53 this is reachable from
any client scanning out on DI1.

Drop the separate alpha formats from the RGB list.

Fixes: f6b50ef14ea8 ("drm/imx: ipuv3-plane: add support for separate alpha planes")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
index c7ec09e557c1..a0fd39eebbbc 100644
--- a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
@@ -96,12 +96,6 @@ static const uint32_t ipu_plane_rgb_formats[] = {
 	DRM_FORMAT_BGRA8888,
 	DRM_FORMAT_BGRX8888,
 	DRM_FORMAT_RGB565,
-	DRM_FORMAT_RGB565_A8,
-	DRM_FORMAT_BGR565_A8,
-	DRM_FORMAT_RGB888_A8,
-	DRM_FORMAT_BGR888_A8,
-	DRM_FORMAT_RGBX8888_A8,
-	DRM_FORMAT_BGRX8888_A8,
 };
 
 static const uint64_t ipu_format_modifiers[] = {

-- 
2.47.3

[PATCH 2/4] drm/imx: ipuv3-plane: add the pixel blend mode property

[Dmitry Baryshkov]

All three IPUv3 planes advertise formats with an alpha channel, but none
of them ever created the "pixel blend mode" property, so userspace has no
way to learn how that alpha is interpreted. drm_mode_config_validate()
points this out once per plane at every boot:

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

The DP blends the foreground over the background as
fg * alpha + bg * (1 - alpha), i.e. it consumes coverage alpha and has no
premultiplied mode. Planes that are not part of a DP flow have nothing to
blend against and ignore alpha entirely.

Create the property accordingly: PIXEL_NONE and COVERAGE for the DP
planes, PIXEL_NONE alone for the rest, and honour the resulting blend
mode when deciding between global and per-pixel alpha.
__drm_atomic_helper_plane_reset() defaults pixel_blend_mode to
PREMULTI unconditionally, so override it in the driver reset.

Fixes: e6245fc78b65 ("imx-drm: ipuv3-plane: allow local alpha in ipu_plane_mode_set()")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c | 43 +++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
index a0fd39eebbbc..bde5892264f4 100644
--- a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
@@ -291,8 +291,32 @@ void ipu_plane_disable_deferred(struct drm_plane *plane)
 	}
 }
 
+static unsigned int ipu_plane_blend_modes(int dp_flow)
+{
+	if (dp_flow == IPU_DP_FLOW_SYNC_BG || dp_flow == IPU_DP_FLOW_SYNC_FG)
+		return BIT(DRM_MODE_BLEND_PIXEL_NONE) |
+		       BIT(DRM_MODE_BLEND_COVERAGE);
+
+	return BIT(DRM_MODE_BLEND_PIXEL_NONE);
+}
+
+static unsigned int ipu_plane_default_blend_mode(int dp_flow)
+{
+	if (dp_flow == IPU_DP_FLOW_SYNC_BG || dp_flow == IPU_DP_FLOW_SYNC_FG)
+		return DRM_MODE_BLEND_COVERAGE;
+
+	return DRM_MODE_BLEND_PIXEL_NONE;
+}
+
+static bool ipu_plane_use_pixel_alpha(struct drm_plane_state *state)
+{
+	return state->fb->format->has_alpha &&
+	       state->pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE;
+}
+
 static void ipu_plane_state_reset(struct drm_plane *plane)
 {
+	struct ipu_plane *ipu_plane = to_ipu_plane(plane);
 	struct ipu_plane_state *ipu_state;
 
 	if (plane->state) {
@@ -304,8 +328,12 @@ static void ipu_plane_state_reset(struct drm_plane *plane)
 
 	ipu_state = kzalloc_obj(*ipu_state);
 
-	if (ipu_state)
+	if (ipu_state) {
 		__drm_atomic_helper_plane_reset(plane, &ipu_state->base);
+		/* the helper defaults to premultiplied, which the DP lacks */
+		ipu_state->base.pixel_blend_mode =
+			ipu_plane_default_blend_mode(ipu_plane->dp_flow);
+	}
 }
 
 static struct drm_plane_state *
@@ -593,8 +621,8 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
 	case IPU_DP_FLOW_SYNC_BG:
 		if (new_state->normalized_zpos == 1) {
 			ipu_dp_set_global_alpha(ipu_plane->dp,
-						!fb->format->has_alpha, 0xff,
-						true);
+						!ipu_plane_use_pixel_alpha(new_state),
+						0xff, true);
 		} else {
 			ipu_dp_set_global_alpha(ipu_plane->dp, true, 0, true);
 		}
@@ -602,8 +630,8 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
 	case IPU_DP_FLOW_SYNC_FG:
 		if (new_state->normalized_zpos == 1) {
 			ipu_dp_set_global_alpha(ipu_plane->dp,
-						!fb->format->has_alpha, 0xff,
-						false);
+						!ipu_plane_use_pixel_alpha(new_state),
+						0xff, false);
 		}
 		break;
 	}
@@ -930,6 +958,11 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
 	if (ret)
 		return ERR_PTR(ret);
 
+	ret = drm_plane_create_blend_mode_property(&ipu_plane->base,
+						   ipu_plane_blend_modes(dp));
+	if (ret)
+		return ERR_PTR(ret);
+
 	ret = drm_plane_create_color_properties(&ipu_plane->base,
 			BIT(DRM_COLOR_YCBCR_BT601) |
 			BIT(DRM_COLOR_YCBCR_BT709),

-- 
2.47.3

Re: [PATCH 2/4] drm/imx: ipuv3-plane: add the pixel blend mode property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Plane atomic update callback calls functions that acquire a sleeping lock (mutex).
--

--- Patch [2]: [PATCH 2/4] drm/imx: ipuv3-plane: add the pixel blend mode property ---
commit 166d0e9ef492a9f6b268953776bb8c6c929a2acc
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm/imx: ipuv3-plane: add the pixel blend mode property

This commit adds a pixel blend mode property to IPUv3 planes. It configures
the planes to support coverage or none based on whether they are part of a
DP flow, and uses this blend mode to decide between global and per-pixel
alpha during plane updates.

> diff --git a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
> index a0fd39eebbbc2..bde5892264f4a 100644
> --- a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c

[ ... ]

> @@ -593,8 +621,8 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
>  	case IPU_DP_FLOW_SYNC_BG:
>  		if (new_state->normalized_zpos == 1) {
>  			ipu_dp_set_global_alpha(ipu_plane->dp,
> -						!fb->format->has_alpha, 0xff,
> -						true);
> +						!ipu_plane_use_pixel_alpha(new_state),
> +						0xff, true);

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to call
ipu_dp_set_global_alpha() from the atomic update callback?

The DRM core invokes ipu_plane_atomic_update() in an atomic context
where sleeping is forbidden. However, ipu_dp_set_global_alpha()
unconditionally acquires a sleeping lock:

drivers/gpu/ipu-v3/ipu-dp.c:ipu_dp_set_global_alpha() {
	...
	mutex_lock(&priv->mutex);
	...
}

Could this trigger kernel warnings or deadlocks if the thread is scheduled
out while executing in a non-blocking DRM atomic commit phase?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-imx53-fix-hdmi-v1-0-fff9b1b17f25@oss.qualcomm.com?part=2

[PATCH 3/4] drm/imx: imx-tve: probe DDC before claiming the connector is connected

[Dmitry Baryshkov]

imx-tve provides no detect callback, so its connector is always reported
as connected and always gets the drm_add_modes_noedid() fallback list,
whether or not anything is attached to the VGA output.

On an i.MX53 QSRB with the MCIMXHDMICARD fitted and nothing on VGA, the
fbdev client therefore lights up both CRTCs: HDMI on DI0 at its preferred
1920x1080 (148.5 MHz) and the phantom VGA output on DI1 at the 1024x768
fallback (65 MHz). Both DIs are ultimately clocked from PLL4 - DI0 via
di_pll4_podf, the TVE via tve_pred/tve_ext_sel - and every link
propagates rate changes, so the TVE reprograms PLL4 underneath the
already running DI0. DI0 stops generating valid timings, its IDMAC EOF
interrupt (which ipuv3-crtc uses as the vblank source) never arrives, and
every commit waits out its full 10 s:

  imx-drm display-subsystem: [drm] *ERROR* [CRTC:40:crtc-0] flip_done timed out

The TVE outputs have no hotplug detection, but a DDC response is decent
evidence that a display is attached. The connector is already registered
with its DDC adapter, so wire up drm_connector_helper_detect_from_ddc().
It reports the connector as disconnected when the DDC bus is there but
nothing answers on it, and falls back to unknown only for boards that
describe no ddc-i2c-bus at all, which keeps those working as before.

Fixes: fcbc51e54d2a ("staging: drm/imx: Add support for Television Encoder (TVEv2)")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/imx/ipuv3/imx-tve.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/imx/ipuv3/imx-tve.c b/drivers/gpu/drm/imx/ipuv3/imx-tve.c
index e861b8b9d8fa..11ac69561eb8 100644
--- a/drivers/gpu/drm/imx/ipuv3/imx-tve.c
+++ b/drivers/gpu/drm/imx/ipuv3/imx-tve.c
@@ -322,6 +322,7 @@ static const struct drm_connector_funcs imx_tve_connector_funcs = {
 static const struct drm_connector_helper_funcs imx_tve_connector_helper_funcs = {
 	.get_modes = imx_tve_connector_get_modes,
 	.mode_valid = imx_tve_connector_mode_valid,
+	.detect_ctx = drm_connector_helper_detect_from_ddc,
 };
 
 static const struct drm_encoder_helper_funcs imx_tve_encoder_helper_funcs = {

-- 
2.47.3

Re: [PATCH 3/4] drm/imx: imx-tve: probe DDC before claiming the connector is connected

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Missing polling configuration for dynamic hotplug detection.

Pre-existing issues:
- [High] Missing -EPROBE_DEFER handling undermines the intended fix under normal probe ordering.
--

--- Patch [3]: [PATCH 3/4] drm/imx: imx-tve: probe DDC before claiming the connector is connected ---
commit 496cfa19c398d86d2f77e8a07cbe96a51f14ca0a
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm/imx: imx-tve: probe DDC before claiming the connector is connected

This commit adds a detect_ctx hook to the TVE connector using the DDC bus to
detect whether a monitor is attached. It aims to fix a CRTC timeout issue
caused by unconditionally lighting up the VGA output when nothing is
connected.

> diff --git a/drivers/gpu/drm/imx/ipuv3/imx-tve.c b/drivers/gpu/drm/imx/ipuv3/imx-tve.c
> index e861b8b9d8fa8..11ac69561eb81 100644
> --- a/drivers/gpu/drm/imx/ipuv3/imx-tve.c
> +++ b/drivers/gpu/drm/imx/ipuv3/imx-tve.c
> @@ -322,6 +322,7 @@ static const struct drm_connector_funcs imx_tve_connector_funcs = {
>  static const struct drm_connector_helper_funcs imx_tve_connector_helper_funcs = {
>  	.get_modes = imx_tve_connector_get_modes,
>  	.mode_valid = imx_tve_connector_mode_valid,
> +	.detect_ctx = drm_connector_helper_detect_from_ddc,

[Severity: Medium]
Since this adds dynamic detection through the detect_ctx callback, shouldn't
we also configure the connector polling flags in imx_tve_bind()?

It looks like the connector is initialized here:

imx_tve_bind() {
    ...
    ret = drm_connector_init_with_ddc(drm, connector,
                                      &imx_tve_connector_funcs,
                                      DRM_MODE_CONNECTOR_VGA, tve->ddc);
    ...
}

Without setting connector->polled = DRM_CONNECTOR_POLL_CONNECT |
DRM_CONNECTOR_POLL_DISCONNECT, the DRM core's periodic poll worker will
ignore this connector. Will this prevent dynamic detection if a user plugs
in a monitor after boot?

[Severity: High]
This is a pre-existing issue, but could probe ordering undermine this fix?

The patch relies on tve->ddc being initialized to accurately report the
connector status. However, imx_tve_probe() does not currently handle probe
deferral:

imx_tve_probe() {
    ...
    tve->ddc = of_find_i2c_adapter_by_node(ddc_node);
    of_node_put(ddc_node);
    if (tve->ddc) {
    ...
}

If the I2C driver hasn't probed yet, tve->ddc will be left as NULL instead
of returning -EPROBE_DEFER.

If detect_ctx is later called with a NULL DDC adapter,
drm_connector_helper_detect_from_ddc() will return connector_status_unknown.
Because the DRM core treats unknown as connected when fallback modes are
involved, does this mean the CRTC timeout bug will reoccur whenever the I2C
driver probes after imx-tve?

>  };
>  
>  static const struct drm_encoder_helper_funcs imx_tve_encoder_helper_funcs = {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-imx53-fix-hdmi-v1-0-fff9b1b17f25@oss.qualcomm.com?part=3

[PATCH 4/4] gpu: ipu-v3: di: report an unattainable pixel clock

[Dmitry Baryshkov]

When the DI cannot be programmed to the requested pixel clock the only
symptom is a "flip_done timed out" ten seconds later, because the display
never starts and the IDMAC EOF interrupt that ipuv3-crtc uses as its
vblank source never arrives. Working out that the clock is at fault
currently requires rebuilding with dynamic debug enabled.

Print an error when the rate the DI ends up with differs from the
requested one by more than 1%. The CLKMODE_SYNC case is skipped, as
there the encoder programs the pixel clock after this runs.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/ipu-v3/ipu-di.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
index 0a34e0ab4fe6..c971c37356f4 100644
--- a/drivers/gpu/ipu-v3/ipu-di.c
+++ b/drivers/gpu/ipu-v3/ipu-di.c
@@ -497,6 +497,22 @@ static void ipu_di_config_clock(struct ipu_di *di,
 		clk_get_rate(di->clk_di),
 		clk == di->clk_di ? "DI" : "IPU",
 		clk_get_rate(di->clk_di_pixel) / (clkgen0 >> 4));
+
+	/*
+	 * With CLKMODE_SYNC the encoder programs the pixel clock after us, so
+	 * there is nothing to check yet. Otherwise the rate is final, and a
+	 * wrong one only shows up later as a "flip_done timed out".
+	 */
+	if (!(sig->clkflags & IPU_DI_CLKMODE_SYNC)) {
+		unsigned long rate = clk_get_rate(di->clk_di_pixel) /
+				     (clkgen0 >> 4);
+
+		if (abs((long)(rate - sig->mode.pixelclock)) >
+		    sig->mode.pixelclock / 100)
+			dev_err(di->ipu->dev,
+				"DI%d: cannot generate %luHz pixel clock, got %luHz\n",
+				di->id, sig->mode.pixelclock, rate);
+	}
 }
 
 /*

-- 
2.47.3