On Wed, 2026-02-25 at 17:36 +0530, Nautiyal, Ankit K wrote:
>
> On 2/19/2026 6:37 PM, Jouni Högander wrote:
> > There is Selective Update slice row per frame and picture height
> > configurations needed on DSC when using Selective Update Early
> > Transport. Calculate and configure these when using Early
> > Transport.
> >
> > Bspec: 68927
> > Fixes: 467e4e061c44 ("drm/i915/psr: Enable psr2 early transport as
> > possible")
> > Cc: <[email protected]> # v6.9+
>
>
> This patch needs the other patch where registers are defined. I am
> not
> sure if stable will only pick this patch or will try to find out the
> dependency patch.
>
> We need to check if there is a way to tell the dependency
> patch/commit
> to stable, so that both patches are applied together.
>
> If we want this change to get ported to older kernels, we might need
> to
> squash the register definition patch with this patch.
>
>
> > Signed-off-by: Jouni Högander <[email protected]>
> > ---
> > .../drm/i915/display/intel_display_types.h | 1 +
> > drivers/gpu/drm/i915/display/intel_psr.c | 24
> > +++++++++++++++++++
> > 2 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index e8e4af03a6a6..8903804c04b1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1381,6 +1381,7 @@ struct intel_crtc_state {
> > u32 psr2_man_track_ctl;
> >
> > u32 pipe_srcsz_early_tpt;
> > + u32 dsc_su_parameter_set_0_calc;
>
> I think let's just have a bool parameter something like
> psr_su_update_dsc_pps.
>
> We can set this bool variable during intel_psr2_sel_fetch_update()
You mean calculating value for the register when writing it? I think
for that purpose we can rely on crtc_state->enable_psr2_su_region_et
and crtc_state->dsc.compression_enable. No need to add new boolean.
Let's do it that way.
>
>
> >
> > struct drm_rect psr2_su_area;
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 331645a2c9f6..0a2948ec308d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -2618,6 +2618,11 @@ void
> > intel_psr2_program_trans_man_trk_ctl(struct intel_dsb *dsb,
> >
> > intel_de_write_dsb(display, dsb, PIPE_SRCSZ_ERLY_TPT(crtc-
> > >pipe),
> > crtc_state->pipe_srcsz_early_tpt);
> > + intel_de_write_dsb(display, dsb,
> > DSC_SU_PARAMETER_SET_0_DSC0(crtc->pipe),
> > + crtc_state-
> > >dsc_su_parameter_set_0_calc);
> > + if (intel_dsc_get_vdsc_per_pipe(crtc_state) > 1)
> > + intel_de_write_dsb(display, dsb,
> > DSC_SU_PARAMETER_SET_0_DSC1(crtc->pipe),
> > + crtc_state-
> > >dsc_su_parameter_set_0_calc);
> > }
> >
> > static void psr2_man_trk_ctl_calc(struct intel_crtc_state
> > *crtc_state,
> > @@ -2668,6 +2673,23 @@ static u32
> > psr2_pipe_srcsz_early_tpt_calc(struct intel_crtc_state *crtc_state,
> > return PIPESRC_WIDTH(width - 1) | PIPESRC_HEIGHT(height -
> > 1);
> > }
> >
> > +static u32 psr2_dsc_su_parameter_set_0_calc(struct
> > intel_crtc_state *crtc_state,
> > + bool full_update)
> > +{
> > + const struct drm_dsc_config *vdsc_cfg = &crtc_state-
> > >dsc.config;
> > + int slice_row_per_frame, pic_height;
> > +
> > + if (!crtc_state->enable_psr2_su_region_et || full_update
> > ||
> > + !crtc_state->dsc.compression_enable)
> > + return 0;
> > +
>
> Although we are making sure that height of the psr2_su_area is a
> multiple of the slice_height, perhaps it would be good to have a
> drm_WARN here to flag any misconfiguration i.e. if height is not a
> multiple of slice_height.
I will add that warning and move these to intel_vdsc.c. I will also
move those register definitions you commented in patch 2.
>
>
> > + slice_row_per_frame = drm_rect_height(&crtc_state-
> > >psr2_su_area) / vdsc_cfg->slice_height;
> > + pic_height = slice_row_per_frame * vdsc_cfg->slice_height;
> > +
> > + return
> > DSC_SU_PARAMETER_SET_0_SU_SLICE_ROW_PER_FRAME(slice_row_per_frame)
> > |
> > + DSC_SU_PARAMETER_SET_0_SU_PIC_HEIGHT(pic_height);
> > +}
>
> Since this writes a DSC register belonging to PPS Set 0, this
> function
> should be moved to intel_vdsc.c.
>
> Also, based on the boolean flag (psr_su_update_dsc_pps) discussed
> above,
> this function should simply retrieve the required fields from
> crtc_state
> and program the register.
Now as you pointed this out I see there is no real reason follow what
is done for PSR2_MAN_TRK_CTL.
>
> Such a helper function should then be called from
> intel_psr2_program_trans_man_trk_ctl() in place of the direct
> intel_reg_write() call.
>
> IMO, all register reads/writes, along with the wrappers/helpers
> around
> them, should live in the file corresponding to the block that owns
> those
> registers, based on context.
Ok, you convinced me. I will move these.
BR,
Jouni Högander
>
>
> Regards,
>
> Ankit
>
> > +
> > static void clip_area_update(struct drm_rect
> > *overlap_damage_area,
> > struct drm_rect *damage_area,
> > struct drm_rect *pipe_src)
> > @@ -3026,6 +3048,8 @@ int intel_psr2_sel_fetch_update(struct
> > intel_atomic_state *state,
> > psr2_man_trk_ctl_calc(crtc_state, full_update);
> > crtc_state->pipe_srcsz_early_tpt =
> > psr2_pipe_srcsz_early_tpt_calc(crtc_state,
> > full_update);
> > + crtc_state->dsc_su_parameter_set_0_calc =
> > psr2_dsc_su_parameter_set_0_calc(crtc_state,
> > +
> > full_update);
> > return 0;
> > }
> >
