All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/msm/dpu: clear pending flush state before physical cleanup
@ 2026-08-26 18:24 Saim Shujah
  2026-08-26 18:38 ` sashiko-bot
  2026-09-13 21:59 ` Dmitry Baryshkov
  0 siblings, 2 replies; 4+ messages in thread
From: Saim Shujah @ 2026-08-26 18:24 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov
  Cc: Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
	linux-arm-msm, dri-devel, freedreno, linux-kernel, stable,
	Saim Shujah

Hardware can consume an active-frame CTL flush while the cached software
pending-flush mask retains the submitted bits. Video-mode disable stops the
timing engine before physical cleanup, which then adds teardown bits to the
stale cached mask.

The teardown trigger can replay source-pipe bits after timing
stops, leaving one pending and preventing the first commit after re-enable.

Clear the cached pending-flush bookkeeping before constructing the teardown
state. clear_pending_flush() updates software bookkeeping only; it does not
write the hardware CTL_FLUSH register.

Fixes: 22cb02bc96ff ("drm/msm/disp/dpu: reset the datapath after timing engine disable")
Cc: stable@vger.kernel.org
Signed-off-by: Saim Shujah <saimzst@gmail.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 778e231..f3b1fdc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2264,6 +2264,7 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
 
 	dpu_enc = to_dpu_encoder_virt(phys_enc->parent);
 
+	ctl->ops.clear_pending_flush(ctl);
 	ctl->ops.reset(ctl);
 
 	dpu_encoder_helper_reset_mixers(phys_enc);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/msm/dpu: clear pending flush state before physical cleanup
  2026-08-26 18:24 [PATCH] drm/msm/dpu: clear pending flush state before physical cleanup Saim Shujah
@ 2026-08-26 18:38 ` sashiko-bot
  2026-09-13 21:59 ` Dmitry Baryshkov
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-26 18:38 UTC (permalink / raw)
  To: Saim Shujah; +Cc: dri-devel

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

Pre-existing issues:
- [High] dpu_hw_ctl_clear_pending_flush() fails to clear pending_periph_flush_mask, leaving stale peripheral flush state across encoder teardowns.
- [High] Sleeping function usleep_range() is called from display driver atomic contexts during encoder cleanup.
--

commit 8f92b7c700308d037e1db8696d21fde498d38560
Author: Saim Shujah <saimzst@gmail.com>

drm/msm/dpu: clear pending flush state before physical cleanup

Hardware can consume an active-frame CTL flush while the cached software
pending-flush mask retains the submitted bits. The patch clears the cached
pending-flush bookkeeping before constructing the teardown state.

> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 1f20695f81e35..45189b6076bc4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -2263,6 +2263,7 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
>  
>  	dpu_enc = to_dpu_encoder_virt(phys_enc->parent);
>  
> +	ctl->ops.clear_pending_flush(ctl);

[Severity: High]
This isn't a bug introduced by this patch, but does dpu_hw_ctl_clear_pending_flush()
leave stale peripheral flush state across encoder teardowns?

In drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c:dpu_hw_ctl_clear_pending_flush():

	ctx->pending_flush_mask = 0x0;
	ctx->pending_intf_flush_mask = 0;
	ctx->pending_wb_flush_mask = 0;
	ctx->pending_cwb_flush_mask = 0;
	ctx->pending_merge_3d_flush_mask = 0;
	ctx->pending_dsc_flush_mask = 0;
	ctx->pending_cdm_flush_mask = 0;

	memset(ctx->pending_dspp_flush_mask, 0,
		sizeof(ctx->pending_dspp_flush_mask));

The function clears most software state caches, but ctx->pending_periph_flush_mask
is missed.

Because pending_periph_flush_mask is omitted from the clear operation, any
peripheral flush bits set from the previous active frame remain in the software
cache. When the encoder is later re-enabled or flushed, this stale mask could
be inadvertently flushed to the CTL_PERIPH_FLUSH hardware register, potentially
causing unexpected hardware states or hangs.

>  	ctl->ops.reset(ctl);

[Severity: High]
This is a pre-existing issue, but does this hardware cleanup execute a sleeping
polling path while in an atomic context?

dpu_encoder_helper_phys_cleanup() is called from the encoder atomic disable
callback. ctl->ops.reset(ctl) resolves to dpu_hw_ctl_reset_control(), which
polls the hardware and calls usleep_range():

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c:dpu_hw_ctl_poll_reset_status():

		if (status)
			usleep_range(20, 50);

Calling sleeping functions like usleep_range() from atomic context paths in
display drivers can cause kernel warnings, system instability, and potential
deadlocks.

>  
>  	dpu_encoder_helper_reset_mixers(phys_enc);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826182459.1506522-1-saimzst@gmail.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/msm/dpu: clear pending flush state before physical cleanup
  2026-08-26 18:24 [PATCH] drm/msm/dpu: clear pending flush state before physical cleanup Saim Shujah
  2026-08-26 18:38 ` sashiko-bot
@ 2026-09-13 21:59 ` Dmitry Baryshkov
  2026-09-14 23:08   ` Dmitry Baryshkov
  1 sibling, 1 reply; 4+ messages in thread
From: Dmitry Baryshkov @ 2026-09-13 21:59 UTC (permalink / raw)
  To: Saim Shujah
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, linux-arm-msm, dri-devel, freedreno,
	linux-kernel, stable

On Wed, Aug 26, 2026 at 11:24:59PM +0500, Saim Shujah wrote:
> Hardware can consume an active-frame CTL flush while the cached software
> pending-flush mask retains the submitted bits. Video-mode disable stops the
> timing engine before physical cleanup, which then adds teardown bits to the
> stale cached mask.
> 
> The teardown trigger can replay source-pipe bits after timing
> stops, leaving one pending and preventing the first commit after re-enable.
> 
> Clear the cached pending-flush bookkeeping before constructing the teardown
> state. clear_pending_flush() updates software bookkeeping only; it does not
> write the hardware CTL_FLUSH register.
> 
> Fixes: 22cb02bc96ff ("drm/msm/disp/dpu: reset the datapath after timing engine disable")
> Cc: stable@vger.kernel.org
> Signed-off-by: Saim Shujah <saimzst@gmail.com>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

> 
-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/msm/dpu: clear pending flush state before physical cleanup
  2026-09-13 21:59 ` Dmitry Baryshkov
@ 2026-09-14 23:08   ` Dmitry Baryshkov
  0 siblings, 0 replies; 4+ messages in thread
From: Dmitry Baryshkov @ 2026-09-14 23:08 UTC (permalink / raw)
  To: Saim Shujah
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, linux-arm-msm, dri-devel, freedreno,
	linux-kernel, stable

On Mon, Sep 14, 2026 at 12:59:30AM +0300, Dmitry Baryshkov wrote:
> On Wed, Aug 26, 2026 at 11:24:59PM +0500, Saim Shujah wrote:
> > Hardware can consume an active-frame CTL flush while the cached software
> > pending-flush mask retains the submitted bits. Video-mode disable stops the
> > timing engine before physical cleanup, which then adds teardown bits to the
> > stale cached mask.
> > 
> > The teardown trigger can replay source-pipe bits after timing
> > stops, leaving one pending and preventing the first commit after re-enable.
> > 
> > Clear the cached pending-flush bookkeeping before constructing the teardown
> > state. clear_pending_flush() updates software bookkeeping only; it does not
> > write the hardware CTL_FLUSH register.
> > 
> > Fixes: 22cb02bc96ff ("drm/msm/disp/dpu: reset the datapath after timing engine disable")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Saim Shujah <saimzst@gmail.com>
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

Unfortunately this patch breaks several IGT tests on sc7180:

kms_plane_alpha_blend@constant-alpha-max
kms_plane_alpha_blend@constant-alpha-mid
kms_rotation_crc@cursor-rotation-180
kms_rotation_crc@primary-rotation-180

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-14 23:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-26 18:24 [PATCH] drm/msm/dpu: clear pending flush state before physical cleanup Saim Shujah
2026-08-26 18:38 ` sashiko-bot
2026-09-13 21:59 ` Dmitry Baryshkov
2026-09-14 23:08   ` Dmitry Baryshkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.