Re: [PATCH 4/5] drm/i915/psr: DSC configuration for Early Transport

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

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: <stable@vger.kernel.org> # 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 <jouni.hogander@intel.com>
> ---
>   .../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()


>   
>   	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.


> +	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.

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.


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;
>   }
>