> -----Original Message-----
> From: Intel-gfx <intel-gfx-boun...@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Monday, June 9, 2025 7:41 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: intel...@lists.freedesktop.org
> Subject: [PATCH v4 05/21] drm/i915/dsb: Move the DSB_PMCTRL* reset out of
> intel_dsb_finish()
>
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
>
> When using the flip queue, due to the DMC vs. DSB register corruption problem,
> we must not issue any register writes from the DSB after unhalting the DMC.
> Currently we are doign just that by trying to restore DSB_PMCTRL* back to a
Nit: Typo in "doing"
> sane state from intel_dsb_finish().
>
> Since the only place left that pokes at DSB_PMCTRL* is intel_dsb_chain() we
> can
> just do DSB_PMCTRL_2/DSB_FORCE_DEWAKE reset in the same place.
>
> The DSB_PMCTRL reset is trickier since we'd have to do it from the chained DSB
> itself. But based on my earlier testing DSB_PMCTRL/DSB_ENABLE_DEWAKE
> doesn't actually do anything if the DSB isn't actually enabled, so we can
> omit the
> reset to keep things a bit simpler. We do need to reset
> DSB_PMCTRL/DSB_ENABLE_DEWAKE before starting the DSB however, in case
> it was left enabled from a previous use.
Looks Good to me.
Reviewed-by: Uma Shankar <uma.shan...@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dsb.c | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
> b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 200555a9e94b..6fdd324615e2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -629,18 +629,6 @@ void intel_dsb_gosub_finish(struct intel_dsb *dsb)
>
> void intel_dsb_finish(struct intel_dsb *dsb) {
> - struct intel_crtc *crtc = dsb->crtc;
> -
> - /*
> - * DSB_FORCE_DEWAKE remains active even after DSB is
> - * disabled, so make sure to clear it (if set during
> - * intel_dsb_commit()). And clear DSB_ENABLE_DEWAKE as
> - * well for good measure.
> - */
> - intel_dsb_reg_write(dsb, DSB_PMCTRL(crtc->pipe, dsb->id), 0);
> - intel_dsb_reg_write_masked(dsb, DSB_PMCTRL_2(crtc->pipe, dsb->id),
> - DSB_FORCE_DEWAKE, 0);
> -
> intel_dsb_align_tail(dsb);
>
> intel_dsb_buffer_flush_map(&dsb->dsb_buf);
> @@ -781,6 +769,8 @@ static void _intel_dsb_chain(struct intel_atomic_state
> *state,
> intel_dsb_reg_write(dsb, DSB_PMCTRL(pipe, chained_dsb->id),
> DSB_ENABLE_DEWAKE |
>
> DSB_SCANLINE_FOR_DEWAKE(hw_dewake_scanline));
> + } else {
> + intel_dsb_reg_write(dsb, DSB_PMCTRL(pipe, chained_dsb->id),
> 0);
> }
>
> intel_dsb_reg_write(dsb, DSB_HEAD(pipe, chained_dsb->id), @@ -802,6
> +792,13 @@ static void _intel_dsb_chain(struct intel_atomic_state *state,
> intel_dsb_wait_scanline_out(state, dsb,
> dsb_dewake_scanline_start(state,
> crtc),
> dsb_dewake_scanline_end(state,
> crtc));
> +
> + /*
> + * DSB_FORCE_DEWAKE remains active even after DSB is
> + * disabled, so make sure to clear it.
> + */
> + intel_dsb_reg_write_masked(dsb, DSB_PMCTRL_2(crtc->pipe,
> dsb->id),
> + DSB_FORCE_DEWAKE, 0);
> }
> }
>
> @@ -857,6 +854,8 @@ void intel_dsb_commit(struct intel_dsb *dsb)
> dsb_error_int_status(display) |
> DSB_PROG_INT_STATUS |
> dsb_error_int_en(display) | DSB_PROG_INT_EN);
>
> + intel_de_write_fw(display, DSB_PMCTRL(pipe, dsb->id), 0);
> +
> intel_de_write_fw(display, DSB_HEAD(pipe, dsb->id),
> intel_dsb_head(dsb));
>
> --
> 2.49.0
