[PATCH 0/2] drm/msm/mdp4: fix intermittent blue screen on Galaxy S4

[PATCH 0/2] drm/msm/mdp4: fix intermittent blue screen on Galaxy S4

[Alexandre MINETTE via B4 Relay]

This series fixes an intermittent blue screen seen on the Samsung
Galaxy S4 with the MDP4 display controller. The issue happens on roughly
half of boots.

The first patch keeps the AXI/bus clock enabled while a CRTC is active,
so scanout does not lose bus access while the display pipeline is still
running.

The second patch handles the case where firmware left the display
pipeline enabled before Linux takes over. In that case, the required
display clocks are kept enabled until the first DRM modeset, where the
driver takes ownership of the pipeline.

Tested on a Samsung Galaxy S4 GT-I9505, codename jflte.

Signed-off-by: Alexandre MINETTE <contact@alex-min.fr>
---
Alexandre MINETTE (2):
      drm/msm/mdp4: keep the bus clock enabled while a CRTC is active
      drm/msm/mdp4: keep inherited display clocks enabled until modeset

 drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c |   3 +
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c  | 108 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h  |   8 +++
 3 files changed, 118 insertions(+), 1 deletion(-)
---
base-commit: acb7500801e98639f6d8c2d796ed9f64cba83d3a
change-id: 20260610-mainline-fix-mdp4-blue-screen-sending-v1-0637d7a6c355

Best regards,
--  
Alexandre MINETTE <contact@alex-min.fr>

[PATCH 0/2] drm/msm/mdp4: fix intermittent blue screen on Galaxy S4

[Alexandre MINETTE]

This series fixes an intermittent blue screen seen on the Samsung
Galaxy S4 with the MDP4 display controller. The issue happens on roughly
half of boots.

The first patch keeps the AXI/bus clock enabled while a CRTC is active,
so scanout does not lose bus access while the display pipeline is still
running.

The second patch handles the case where firmware left the display
pipeline enabled before Linux takes over. In that case, the required
display clocks are kept enabled until the first DRM modeset, where the
driver takes ownership of the pipeline.

Tested on a Samsung Galaxy S4 GT-I9505, codename jflte.

Signed-off-by: Alexandre MINETTE <contact@alex-min.fr>
---
Alexandre MINETTE (2):
      drm/msm/mdp4: keep the bus clock enabled while a CRTC is active
      drm/msm/mdp4: keep inherited display clocks enabled until modeset

 drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c |   3 +
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c  | 108 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h  |   8 +++
 3 files changed, 118 insertions(+), 1 deletion(-)
---
base-commit: acb7500801e98639f6d8c2d796ed9f64cba83d3a
change-id: 20260610-mainline-fix-mdp4-blue-screen-sending-v1-0637d7a6c355

Best regards,
--  
Alexandre MINETTE <contact@alex-min.fr>

[PATCH 1/2] drm/msm/mdp4: keep the bus clock enabled while a CRTC is active

[Alexandre MINETTE via B4 Relay]

From: Alexandre MINETTE <contact@alex-min.fr>

MDP4 scanout continues to fetch framebuffer data while the CRTC is
active. Keep the AXI/bus clock enabled for the lifetime of each active
CRTC so that temporary MDP4 disable paths do not gate the bus clock
while scanout is still running.

This avoids display corruption seen on the Samsung Galaxy S4 when the
display pipeline is active.

Signed-off-by: Alexandre MINETTE <contact@alex-min.fr>
---
 drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c |  2 ++
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c  | 42 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h  |  6 +++++
 3 files changed, 50 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
index e8066f9fd534..9e53c9d956ca 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
@@ -271,6 +271,7 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc *crtc,
 
 	mdp_irq_unregister(&mdp4_kms->base, &mdp4_crtc->err);
 	mdp4_disable(mdp4_kms);
+	mdp4_crtc_bus_put(mdp4_kms);
 
 	if (crtc->state->event && !crtc->state->active) {
 		WARN_ON(mdp4_crtc->event);
@@ -295,6 +296,7 @@ static void mdp4_crtc_atomic_enable(struct drm_crtc *crtc,
 		return;
 
 	mdp4_enable(mdp4_kms);
+	mdp4_crtc_bus_get(mdp4_kms);
 
 	/* Restore vblank irq handling after power is enabled */
 	drm_crtc_vblank_on(crtc);
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index 809ca191e9de..4c60b49fb784 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -118,6 +118,39 @@ static long mdp4_round_pixclk(struct msm_kms *kms, unsigned long rate,
 	}
 }
 
+void mdp4_crtc_bus_get(struct mdp4_kms *mdp4_kms)
+{
+	int ret = 0;
+
+	mutex_lock(&mdp4_kms->clock_lock);
+
+	if (!mdp4_kms->crtc_bus_count)
+		ret = clk_prepare_enable(mdp4_kms->axi_clk);
+
+	if (!ret)
+		mdp4_kms->crtc_bus_count++;
+
+	mutex_unlock(&mdp4_kms->clock_lock);
+
+	WARN_ON(ret);
+}
+
+void mdp4_crtc_bus_put(struct mdp4_kms *mdp4_kms)
+{
+	mutex_lock(&mdp4_kms->clock_lock);
+	if (!mdp4_kms->crtc_bus_count)
+		goto unlock;
+
+	mdp4_kms->crtc_bus_count--;
+	if (mdp4_kms->crtc_bus_count)
+		goto unlock;
+
+	clk_disable_unprepare(mdp4_kms->axi_clk);
+
+unlock:
+	mutex_unlock(&mdp4_kms->clock_lock);
+}
+
 static void mdp4_destroy(struct msm_kms *kms)
 {
 	struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
@@ -137,6 +170,13 @@ static void mdp4_destroy(struct msm_kms *kms)
 	if (mdp4_kms->rpm_enabled)
 		pm_runtime_disable(dev);
 
+	mutex_lock(&mdp4_kms->clock_lock);
+	if (mdp4_kms->crtc_bus_count) {
+		clk_disable_unprepare(mdp4_kms->axi_clk);
+		mdp4_kms->crtc_bus_count = 0;
+	}
+	mutex_unlock(&mdp4_kms->clock_lock);
+
 	mdp_kms_destroy(&mdp4_kms->base);
 }
 
@@ -514,6 +554,8 @@ static int mdp4_probe(struct platform_device *pdev)
 	if (!mdp4_kms)
 		return -ENOMEM;
 
+	mutex_init(&mdp4_kms->clock_lock);
+
 	mdp4_kms->mmio = msm_ioremap(pdev, NULL);
 	if (IS_ERR(mdp4_kms->mmio))
 		return PTR_ERR(mdp4_kms->mmio);
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
index 06458d4ee48c..86391d9bc55f 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
@@ -7,6 +7,8 @@
 #ifndef __MDP4_KMS_H__
 #define __MDP4_KMS_H__
 
+#include <linux/mutex.h>
+
 #include <drm/drm_panel.h>
 
 #include "msm_drv.h"
@@ -31,6 +33,8 @@ struct mdp4_kms {
 	struct clk *pclk;
 	struct clk *lut_clk;
 	struct clk *axi_clk;
+	struct mutex clock_lock;
+	unsigned int crtc_bus_count;
 
 	struct mdp_irq error_handler;
 
@@ -148,6 +152,8 @@ static inline uint32_t mixercfg(uint32_t mixer_cfg, int mixer,
 
 int mdp4_disable(struct mdp4_kms *mdp4_kms);
 int mdp4_enable(struct mdp4_kms *mdp4_kms);
+void mdp4_crtc_bus_get(struct mdp4_kms *mdp4_kms);
+void mdp4_crtc_bus_put(struct mdp4_kms *mdp4_kms);
 
 void mdp4_set_irqmask(struct mdp_kms *mdp_kms, uint32_t irqmask,
 		uint32_t old_irqmask);

-- 
2.43.0

[PATCH 1/2] drm/msm/mdp4: keep the bus clock enabled while a CRTC is active

[Alexandre MINETTE]

MDP4 scanout continues to fetch framebuffer data while the CRTC is
active. Keep the AXI/bus clock enabled for the lifetime of each active
CRTC so that temporary MDP4 disable paths do not gate the bus clock
while scanout is still running.

This avoids display corruption seen on the Samsung Galaxy S4 when the
display pipeline is active.

Signed-off-by: Alexandre MINETTE <contact@alex-min.fr>
---
 drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c |  2 ++
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c  | 42 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h  |  6 +++++
 3 files changed, 50 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
index e8066f9fd534..9e53c9d956ca 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
@@ -271,6 +271,7 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc *crtc,
 
 	mdp_irq_unregister(&mdp4_kms->base, &mdp4_crtc->err);
 	mdp4_disable(mdp4_kms);
+	mdp4_crtc_bus_put(mdp4_kms);
 
 	if (crtc->state->event && !crtc->state->active) {
 		WARN_ON(mdp4_crtc->event);
@@ -295,6 +296,7 @@ static void mdp4_crtc_atomic_enable(struct drm_crtc *crtc,
 		return;
 
 	mdp4_enable(mdp4_kms);
+	mdp4_crtc_bus_get(mdp4_kms);
 
 	/* Restore vblank irq handling after power is enabled */
 	drm_crtc_vblank_on(crtc);
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index 809ca191e9de..4c60b49fb784 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -118,6 +118,39 @@ static long mdp4_round_pixclk(struct msm_kms *kms, unsigned long rate,
 	}
 }
 
+void mdp4_crtc_bus_get(struct mdp4_kms *mdp4_kms)
+{
+	int ret = 0;
+
+	mutex_lock(&mdp4_kms->clock_lock);
+
+	if (!mdp4_kms->crtc_bus_count)
+		ret = clk_prepare_enable(mdp4_kms->axi_clk);
+
+	if (!ret)
+		mdp4_kms->crtc_bus_count++;
+
+	mutex_unlock(&mdp4_kms->clock_lock);
+
+	WARN_ON(ret);
+}
+
+void mdp4_crtc_bus_put(struct mdp4_kms *mdp4_kms)
+{
+	mutex_lock(&mdp4_kms->clock_lock);
+	if (!mdp4_kms->crtc_bus_count)
+		goto unlock;
+
+	mdp4_kms->crtc_bus_count--;
+	if (mdp4_kms->crtc_bus_count)
+		goto unlock;
+
+	clk_disable_unprepare(mdp4_kms->axi_clk);
+
+unlock:
+	mutex_unlock(&mdp4_kms->clock_lock);
+}
+
 static void mdp4_destroy(struct msm_kms *kms)
 {
 	struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
@@ -137,6 +170,13 @@ static void mdp4_destroy(struct msm_kms *kms)
 	if (mdp4_kms->rpm_enabled)
 		pm_runtime_disable(dev);
 
+	mutex_lock(&mdp4_kms->clock_lock);
+	if (mdp4_kms->crtc_bus_count) {
+		clk_disable_unprepare(mdp4_kms->axi_clk);
+		mdp4_kms->crtc_bus_count = 0;
+	}
+	mutex_unlock(&mdp4_kms->clock_lock);
+
 	mdp_kms_destroy(&mdp4_kms->base);
 }
 
@@ -514,6 +554,8 @@ static int mdp4_probe(struct platform_device *pdev)
 	if (!mdp4_kms)
 		return -ENOMEM;
 
+	mutex_init(&mdp4_kms->clock_lock);
+
 	mdp4_kms->mmio = msm_ioremap(pdev, NULL);
 	if (IS_ERR(mdp4_kms->mmio))
 		return PTR_ERR(mdp4_kms->mmio);
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
index 06458d4ee48c..86391d9bc55f 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
@@ -7,6 +7,8 @@
 #ifndef __MDP4_KMS_H__
 #define __MDP4_KMS_H__
 
+#include <linux/mutex.h>
+
 #include <drm/drm_panel.h>
 
 #include "msm_drv.h"
@@ -31,6 +33,8 @@ struct mdp4_kms {
 	struct clk *pclk;
 	struct clk *lut_clk;
 	struct clk *axi_clk;
+	struct mutex clock_lock;
+	unsigned int crtc_bus_count;
 
 	struct mdp_irq error_handler;
 
@@ -148,6 +152,8 @@ static inline uint32_t mixercfg(uint32_t mixer_cfg, int mixer,
 
 int mdp4_disable(struct mdp4_kms *mdp4_kms);
 int mdp4_enable(struct mdp4_kms *mdp4_kms);
+void mdp4_crtc_bus_get(struct mdp4_kms *mdp4_kms);
+void mdp4_crtc_bus_put(struct mdp4_kms *mdp4_kms);
 
 void mdp4_set_irqmask(struct mdp_kms *mdp_kms, uint32_t irqmask,
 		uint32_t old_irqmask);

-- 
2.43.0

Re: [PATCH 1/2] drm/msm/mdp4: keep the bus clock enabled while a CRTC is active

[sashiko-bot]

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

New issues:
- [High] Sleeping functions called from atomic context.
- [High] Unbalanced reference counting on AXI clock enable failure leads to premature clock disable for active CRTCs.

Pre-existing issues:
- [High] Failure to enforce IOMMU requirement by allowing a NULL VM fallback in drm_gpuvm conversion.
--

commit f1ffff35bea0debf56db480bb3da6443ad42c41a
Author: Alexandre MINETTE <contact@alex-min.fr>
Subject: drm/msm/mdp4: keep the bus clock enabled while a CRTC is active

This patch keeps the AXI/bus clock enabled for the lifetime of each active
CRTC to avoid temporary MDP4 disable paths gating the bus clock while
scanout is still running.

> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> index e8066f9fd5343..9e53c9d956ca8 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> @@ -271,6 +271,7 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc *crtc,
>  
>  	mdp_irq_unregister(&mdp4_kms->base, &mdp4_crtc->err);
>  	mdp4_disable(mdp4_kms);
> +	mdp4_crtc_bus_put(mdp4_kms);

[Severity: High]
Does calling mdp4_crtc_bus_put() here introduce a sleeping lock into an atomic
context?

CRTC atomic disable callbacks execute in an atomic context where sleeping is
forbidden, but mdp4_crtc_bus_put() acquires a mutex lock.

[ ... ]

> @@ -295,6 +296,7 @@ static void mdp4_crtc_atomic_enable(struct drm_crtc *crtc,
>  		return;
>  
>  	mdp4_enable(mdp4_kms);
> +	mdp4_crtc_bus_get(mdp4_kms);

[Severity: High]
Similarly, does this introduce sleeping calls into the atomic enable path?

mdp4_crtc_bus_get() uses both mutex_lock() and clk_prepare_enable(), which
may sleep.

[ ... ]

> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> index 809ca191e9de7..4c60b49fb7842 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> @@ -118,6 +118,39 @@ static long mdp4_round_pixclk(struct msm_kms *kms, unsigned long rate,
[ ... ]
> +void mdp4_crtc_bus_get(struct mdp4_kms *mdp4_kms)
> +{
> +	int ret = 0;
> +
> +	mutex_lock(&mdp4_kms->clock_lock);
> +
> +	if (!mdp4_kms->crtc_bus_count)
> +		ret = clk_prepare_enable(mdp4_kms->axi_clk);
> +
> +	if (!ret)
> +		mdp4_kms->crtc_bus_count++;
> +
> +	mutex_unlock(&mdp4_kms->clock_lock);
> +
> +	WARN_ON(ret);
> +}
> +
> +void mdp4_crtc_bus_put(struct mdp4_kms *mdp4_kms)
> +{
> +	mutex_lock(&mdp4_kms->clock_lock);
> +	if (!mdp4_kms->crtc_bus_count)
> +		goto unlock;
> +
> +	mdp4_kms->crtc_bus_count--;

[Severity: High]
Could this result in unbalanced reference counting and premature clock disable?

If clk_prepare_enable() fails in mdp4_crtc_bus_get(), crtc_bus_count is not
incremented, but the error isn't propagated since the function returns void.
If a subsequent CRTC successfully enables the clock, the counter becomes 1.

Later, when the first (failed) CRTC is disabled, mdp4_crtc_bus_put() will
unconditionally decrement crtc_bus_count to 0 and disable the clock. This
steals the reference belonging to the second active CRTC, leaving it scanning
out with its bus clock disabled.

[ ... ]

While reviewing this file, I noticed a pre-existing issue in mdp4_kms_init():

drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c:mdp4_kms_init() {
    ...
	vm = msm_kms_init_vm(mdp4_kms->dev, NULL);
	if (IS_ERR(vm)) {
		ret = PTR_ERR(vm);
		goto fail;
	}

	kms->vm = vm;
    ...
}

[Severity: High]
This is a pre-existing issue, but does this bypass the IOMMU requirement if
msm_kms_init_vm() returns NULL?

The DRM subsystem requires IOMMU support for drm_gpuvm and does not support
fallback modes. Since msm_kms_init_vm() might return NULL when IOMMU is
unavailable, checking only IS_ERR(vm) allows driver initialization to proceed
with a NULL VM instead of failing with an error.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611-mainline-fix-mdp4-blue-screen-sending-v1-v1-0-22925600e4f2@alex-min.fr?part=1

Re: [PATCH 1/2] drm/msm/mdp4: keep the bus clock enabled while a CRTC is active

[Konrad Dybcio]

On 6/11/26 8:07 AM, Alexandre MINETTE via B4 Relay wrote:
> From: Alexandre MINETTE <contact@alex-min.fr>
> 
> MDP4 scanout continues to fetch framebuffer data while the CRTC is
> active. Keep the AXI/bus clock enabled for the lifetime of each active
> CRTC so that temporary MDP4 disable paths do not gate the bus clock
> while scanout is still running.
> 
> This avoids display corruption seen on the Samsung Galaxy S4 when the
> display pipeline is active.
> 
> Signed-off-by: Alexandre MINETTE <contact@alex-min.fr>
> ---

[...]

> +void mdp4_crtc_bus_put(struct mdp4_kms *mdp4_kms)
> +{
> +	mutex_lock(&mdp4_kms->clock_lock);

If you use guard(mutex)(&mdp4_kms->lock_lock), you can drop
the manual _unlock() calls (the lock will be dropped once you
exit the scope

Konrad

[PATCH 2/2] drm/msm/mdp4: keep inherited display clocks enabled until modeset

[Alexandre MINETTE via B4 Relay]

From: Alexandre MINETTE <contact@alex-min.fr>

Some devices boot with the MDP4 display pipeline already enabled by
firmware. On the Samsung Galaxy S4, disabling clocks used by that
inherited pipeline before DRM takes over can leave the panel showing a
blue screen.

Keep the required display clocks prepared when an inherited
DSI/LCDC/DTV enable state is detected, and drop the extra clock
references on the first DRM CRTC enable. At that point the driver owns
the display pipeline.

Signed-off-by: Alexandre MINETTE <contact@alex-min.fr>
---
 drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c |  1 +
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c  | 66 ++++++++++++++++++++++++++++++-
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h  |  2 +
 3 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
index 9e53c9d956ca..0aeefd48be1f 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
@@ -297,6 +297,7 @@ static void mdp4_crtc_atomic_enable(struct drm_crtc *crtc,
 
 	mdp4_enable(mdp4_kms);
 	mdp4_crtc_bus_get(mdp4_kms);
+	mdp4_inherited_display_clocks_put(mdp4_kms);
 
 	/* Restore vblank irq handling after power is enabled */
 	drm_crtc_vblank_on(crtc);
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index 4c60b49fb784..1264ce7e8459 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -151,6 +151,58 @@ void mdp4_crtc_bus_put(struct mdp4_kms *mdp4_kms)
 	mutex_unlock(&mdp4_kms->clock_lock);
 }
 
+static bool mdp4_has_inherited_display_state(struct mdp4_kms *mdp4_kms)
+{
+	return mdp4_read(mdp4_kms, REG_MDP4_DSI_ENABLE) ||
+	       mdp4_read(mdp4_kms, REG_MDP4_LCDC_ENABLE) ||
+	       mdp4_read(mdp4_kms, REG_MDP4_DTV_ENABLE);
+}
+
+/*
+ * If firmware left MDP4 scanout enabled, preserve the core/LUT clocks until
+ * the first DRM CRTC enable takes ownership of the display pipeline.
+ */
+static void mdp4_inherited_display_clocks_get(struct mdp4_kms *mdp4_kms)
+{
+	bool inherited_display;
+
+	mutex_lock(&mdp4_kms->clock_lock);
+
+	clk_prepare_enable(mdp4_kms->clk);
+	clk_prepare_enable(mdp4_kms->pclk);
+	clk_prepare_enable(mdp4_kms->lut_clk);
+	clk_prepare_enable(mdp4_kms->axi_clk);
+
+	inherited_display = mdp4_has_inherited_display_state(mdp4_kms);
+
+	if (inherited_display) {
+		clk_prepare_enable(mdp4_kms->clk);
+		clk_prepare_enable(mdp4_kms->lut_clk);
+		mdp4_kms->inherited_display_clocks_held = true;
+	}
+
+	clk_disable_unprepare(mdp4_kms->clk);
+	clk_disable_unprepare(mdp4_kms->pclk);
+	clk_disable_unprepare(mdp4_kms->lut_clk);
+	clk_disable_unprepare(mdp4_kms->axi_clk);
+
+	mutex_unlock(&mdp4_kms->clock_lock);
+}
+
+void mdp4_inherited_display_clocks_put(struct mdp4_kms *mdp4_kms)
+{
+	mutex_lock(&mdp4_kms->clock_lock);
+	if (!mdp4_kms->inherited_display_clocks_held)
+		goto unlock;
+
+	clk_disable_unprepare(mdp4_kms->clk);
+	clk_disable_unprepare(mdp4_kms->lut_clk);
+	mdp4_kms->inherited_display_clocks_held = false;
+
+unlock:
+	mutex_unlock(&mdp4_kms->clock_lock);
+}
+
 static void mdp4_destroy(struct msm_kms *kms)
 {
 	struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
@@ -171,6 +223,11 @@ static void mdp4_destroy(struct msm_kms *kms)
 		pm_runtime_disable(dev);
 
 	mutex_lock(&mdp4_kms->clock_lock);
+	if (mdp4_kms->inherited_display_clocks_held) {
+		clk_disable_unprepare(mdp4_kms->clk);
+		clk_disable_unprepare(mdp4_kms->lut_clk);
+		mdp4_kms->inherited_display_clocks_held = false;
+	}
 	if (mdp4_kms->crtc_bus_count) {
 		clk_disable_unprepare(mdp4_kms->axi_clk);
 		mdp4_kms->crtc_bus_count = 0;
@@ -549,6 +606,7 @@ static int mdp4_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct mdp4_kms *mdp4_kms;
 	int irq;
+	int ret;
 
 	mdp4_kms = devm_kzalloc(dev, sizeof(*mdp4_kms), GFP_KERNEL);
 	if (!mdp4_kms)
@@ -594,7 +652,13 @@ static int mdp4_probe(struct platform_device *pdev)
 	if (IS_ERR(mdp4_kms->lut_clk))
 		return dev_err_probe(dev, PTR_ERR(mdp4_kms->lut_clk), "failed to get lut_clk\n");
 
-	return msm_drv_probe(&pdev->dev, mdp4_kms_init, &mdp4_kms->base.base);
+	mdp4_inherited_display_clocks_get(mdp4_kms);
+
+	ret = msm_drv_probe(&pdev->dev, mdp4_kms_init, &mdp4_kms->base.base);
+	if (ret)
+		mdp4_inherited_display_clocks_put(mdp4_kms);
+
+	return ret;
 }
 
 static void mdp4_remove(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
index 86391d9bc55f..289b5b476d99 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
@@ -34,6 +34,7 @@ struct mdp4_kms {
 	struct clk *lut_clk;
 	struct clk *axi_clk;
 	struct mutex clock_lock;
+	bool inherited_display_clocks_held;
 	unsigned int crtc_bus_count;
 
 	struct mdp_irq error_handler;
@@ -152,6 +153,7 @@ static inline uint32_t mixercfg(uint32_t mixer_cfg, int mixer,
 
 int mdp4_disable(struct mdp4_kms *mdp4_kms);
 int mdp4_enable(struct mdp4_kms *mdp4_kms);
+void mdp4_inherited_display_clocks_put(struct mdp4_kms *mdp4_kms);
 void mdp4_crtc_bus_get(struct mdp4_kms *mdp4_kms);
 void mdp4_crtc_bus_put(struct mdp4_kms *mdp4_kms);
 

-- 
2.43.0

[PATCH 2/2] drm/msm/mdp4: keep inherited display clocks enabled until modeset

[Alexandre MINETTE]

Some devices boot with the MDP4 display pipeline already enabled by
firmware. On the Samsung Galaxy S4, disabling clocks used by that
inherited pipeline before DRM takes over can leave the panel showing a
blue screen.

Keep the required display clocks prepared when an inherited
DSI/LCDC/DTV enable state is detected, and drop the extra clock
references on the first DRM CRTC enable. At that point the driver owns
the display pipeline.

Signed-off-by: Alexandre MINETTE <contact@alex-min.fr>
---
 drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c |  1 +
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c  | 66 ++++++++++++++++++++++++++++++-
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h  |  2 +
 3 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
index 9e53c9d956ca..0aeefd48be1f 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
@@ -297,6 +297,7 @@ static void mdp4_crtc_atomic_enable(struct drm_crtc *crtc,
 
 	mdp4_enable(mdp4_kms);
 	mdp4_crtc_bus_get(mdp4_kms);
+	mdp4_inherited_display_clocks_put(mdp4_kms);
 
 	/* Restore vblank irq handling after power is enabled */
 	drm_crtc_vblank_on(crtc);
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index 4c60b49fb784..1264ce7e8459 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -151,6 +151,58 @@ void mdp4_crtc_bus_put(struct mdp4_kms *mdp4_kms)
 	mutex_unlock(&mdp4_kms->clock_lock);
 }
 
+static bool mdp4_has_inherited_display_state(struct mdp4_kms *mdp4_kms)
+{
+	return mdp4_read(mdp4_kms, REG_MDP4_DSI_ENABLE) ||
+	       mdp4_read(mdp4_kms, REG_MDP4_LCDC_ENABLE) ||
+	       mdp4_read(mdp4_kms, REG_MDP4_DTV_ENABLE);
+}
+
+/*
+ * If firmware left MDP4 scanout enabled, preserve the core/LUT clocks until
+ * the first DRM CRTC enable takes ownership of the display pipeline.
+ */
+static void mdp4_inherited_display_clocks_get(struct mdp4_kms *mdp4_kms)
+{
+	bool inherited_display;
+
+	mutex_lock(&mdp4_kms->clock_lock);
+
+	clk_prepare_enable(mdp4_kms->clk);
+	clk_prepare_enable(mdp4_kms->pclk);
+	clk_prepare_enable(mdp4_kms->lut_clk);
+	clk_prepare_enable(mdp4_kms->axi_clk);
+
+	inherited_display = mdp4_has_inherited_display_state(mdp4_kms);
+
+	if (inherited_display) {
+		clk_prepare_enable(mdp4_kms->clk);
+		clk_prepare_enable(mdp4_kms->lut_clk);
+		mdp4_kms->inherited_display_clocks_held = true;
+	}
+
+	clk_disable_unprepare(mdp4_kms->clk);
+	clk_disable_unprepare(mdp4_kms->pclk);
+	clk_disable_unprepare(mdp4_kms->lut_clk);
+	clk_disable_unprepare(mdp4_kms->axi_clk);
+
+	mutex_unlock(&mdp4_kms->clock_lock);
+}
+
+void mdp4_inherited_display_clocks_put(struct mdp4_kms *mdp4_kms)
+{
+	mutex_lock(&mdp4_kms->clock_lock);
+	if (!mdp4_kms->inherited_display_clocks_held)
+		goto unlock;
+
+	clk_disable_unprepare(mdp4_kms->clk);
+	clk_disable_unprepare(mdp4_kms->lut_clk);
+	mdp4_kms->inherited_display_clocks_held = false;
+
+unlock:
+	mutex_unlock(&mdp4_kms->clock_lock);
+}
+
 static void mdp4_destroy(struct msm_kms *kms)
 {
 	struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
@@ -171,6 +223,11 @@ static void mdp4_destroy(struct msm_kms *kms)
 		pm_runtime_disable(dev);
 
 	mutex_lock(&mdp4_kms->clock_lock);
+	if (mdp4_kms->inherited_display_clocks_held) {
+		clk_disable_unprepare(mdp4_kms->clk);
+		clk_disable_unprepare(mdp4_kms->lut_clk);
+		mdp4_kms->inherited_display_clocks_held = false;
+	}
 	if (mdp4_kms->crtc_bus_count) {
 		clk_disable_unprepare(mdp4_kms->axi_clk);
 		mdp4_kms->crtc_bus_count = 0;
@@ -549,6 +606,7 @@ static int mdp4_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct mdp4_kms *mdp4_kms;
 	int irq;
+	int ret;
 
 	mdp4_kms = devm_kzalloc(dev, sizeof(*mdp4_kms), GFP_KERNEL);
 	if (!mdp4_kms)
@@ -594,7 +652,13 @@ static int mdp4_probe(struct platform_device *pdev)
 	if (IS_ERR(mdp4_kms->lut_clk))
 		return dev_err_probe(dev, PTR_ERR(mdp4_kms->lut_clk), "failed to get lut_clk\n");
 
-	return msm_drv_probe(&pdev->dev, mdp4_kms_init, &mdp4_kms->base.base);
+	mdp4_inherited_display_clocks_get(mdp4_kms);
+
+	ret = msm_drv_probe(&pdev->dev, mdp4_kms_init, &mdp4_kms->base.base);
+	if (ret)
+		mdp4_inherited_display_clocks_put(mdp4_kms);
+
+	return ret;
 }
 
 static void mdp4_remove(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
index 86391d9bc55f..289b5b476d99 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
@@ -34,6 +34,7 @@ struct mdp4_kms {
 	struct clk *lut_clk;
 	struct clk *axi_clk;
 	struct mutex clock_lock;
+	bool inherited_display_clocks_held;
 	unsigned int crtc_bus_count;
 
 	struct mdp_irq error_handler;
@@ -152,6 +153,7 @@ static inline uint32_t mixercfg(uint32_t mixer_cfg, int mixer,
 
 int mdp4_disable(struct mdp4_kms *mdp4_kms);
 int mdp4_enable(struct mdp4_kms *mdp4_kms);
+void mdp4_inherited_display_clocks_put(struct mdp4_kms *mdp4_kms);
 void mdp4_crtc_bus_get(struct mdp4_kms *mdp4_kms);
 void mdp4_crtc_bus_put(struct mdp4_kms *mdp4_kms);
 

-- 
2.43.0

Re: [PATCH 2/2] drm/msm/mdp4: keep inherited display clocks enabled until modeset

[sashiko-bot]

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

New issues:
- [High] Unchecked clk_prepare_enable() return values can lead to synchronous external aborts during MMIO access or unbalanced clock disables.
- [High] The patch explicitly turns off `pclk` and `axi_clk` during probe, which can break the inherited display pipeline.
- [High] Inherited display clocks are leaked indefinitely if DRM components fail to bind, if the CRTC is disabled without being enabled first, or if userspace never initiates a modeset.

Pre-existing issues:
- [High] Calling sleeping functions from atomic context in CRTC atomic enable callback.
--

commit deec425702ee01b881e4c0a6e40e6ec4f167024f
Author: Alexandre MINETTE <contact@alex-min.fr>

drm/msm/mdp4: keep inherited display clocks enabled until modeset

This patch detects if the MDP4 display pipeline was already enabled by
firmware and keeps the required display clocks prepared to prevent panel
glitches. It drops the extra clock references on the first DRM CRTC
enable when the driver takes ownership.

> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> @@ -297,6 +297,7 @@ static void mdp4_crtc_atomic_enable(struct drm_crtc *crtc,
>  
>  	mdp4_enable(mdp4_kms);
>  	mdp4_crtc_bus_get(mdp4_kms);
> +	mdp4_inherited_display_clocks_put(mdp4_kms);

[Severity: High]
Will the inherited clock references be leaked if the display components
never bind or if userspace never initiates a modeset?

If msm_drv_probe() successfully adds the master component but the required
display components fail to bind, the mdp4_destroy() callback is never
executed. If userspace never initiates a modeset or explicitly disables
the display first, atomic_enable is never reached. In these cases, it
appears the driver holds explicit clk_prepare_enable references that will
bypass the clock framework's late initcalls, leaving the hardware fully
powered on indefinitely.

[Severity: High]
This is a pre-existing issue, but does adding this call here worsen a
violation of atomic context rules?

CRTC atomic enable callbacks run in atomic context during non-blocking
commits, where sleeping is strictly forbidden. The mdp4_crtc_atomic_enable()
function already contained sleeping calls like mdp4_enable(), and this
patch adds mdp4_inherited_display_clocks_put() which acquires a mutex and
calls clk_disable_unprepare(). Both of these operations can sleep. Could
this lead to kernel might_sleep() warnings or deadlocks during non-blocking
commits?

>  
>  	/* Restore vblank irq handling after power is enabled */
>  	drm_crtc_vblank_on(crtc);

[ ... ]

> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
[ ... ]
> +static void mdp4_inherited_display_clocks_get(struct mdp4_kms *mdp4_kms)
> +{
> +	bool inherited_display;
> +
> +	mutex_lock(&mdp4_kms->clock_lock);
> +
> +	clk_prepare_enable(mdp4_kms->clk);
> +	clk_prepare_enable(mdp4_kms->pclk);
> +	clk_prepare_enable(mdp4_kms->lut_clk);
> +	clk_prepare_enable(mdp4_kms->axi_clk);

[Severity: High]
Is it safe to proceed with hardware MMIO reads if any of these clocks fail
to enable?

The return values from clk_prepare_enable() are completely ignored here.
If enabling the core or bus clock fails, the driver will proceed to perform
MMIO reads in mdp4_has_inherited_display_state() on unclocked hardware. On
MSM platforms, accessing unclocked registers typically triggers an unhandled
synchronous external abort, resulting in a kernel panic. Should these
return values be checked and handled?

> +
> +	inherited_display = mdp4_has_inherited_display_state(mdp4_kms);
> +
> +	if (inherited_display) {
> +		clk_prepare_enable(mdp4_kms->clk);
> +		clk_prepare_enable(mdp4_kms->lut_clk);
> +		mdp4_kms->inherited_display_clocks_held = true;
> +	}
> +
> +	clk_disable_unprepare(mdp4_kms->clk);
> +	clk_disable_unprepare(mdp4_kms->pclk);
> +	clk_disable_unprepare(mdp4_kms->lut_clk);
> +	clk_disable_unprepare(mdp4_kms->axi_clk);

[Severity: High]
Will unconditionally disabling pclk and axi_clk here break the inherited
display pipeline?

The Linux Common Clock Framework initializes clocks left on by the
bootloader with a software usage count of 0. By temporarily enabling and
then disabling them, the usage count goes from 0 to 1 and back to 0. This
triggers the framework to physically turn off the hardware clocks
immediately. Disabling the pixel clock (pclk) and memory bus clock
(axi_clk) while the hardware is actively scanning out a frame will
typically starve the pipeline, causing bus hangs, underflows, or the exact
blank screen this patch aims to prevent. Should the active state of these
clocks be preserved if an inherited display is detected?

> +
> +	mutex_unlock(&mdp4_kms->clock_lock);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611-mainline-fix-mdp4-blue-screen-sending-v1-v1-0-22925600e4f2@alex-min.fr?part=2