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

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

On 2/25/2026 6:59 PM, Jani Nikula wrote:
> On Wed, 25 Feb 2026, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> 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: <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.
> Nope. Neither we nor stable want dependencies squashed. They'll happily
> pick up extra dependency commits if needed, though.
>
> Someone(tm) just needs to let them know about the dependencies when they
> send the mail about (the presumably failed) backport attempt.

Got it :) thanks Jani.

Regards,

Ankit

>
> BR,
> Jani.
>
>
>>
>>> 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;
>>>    }
>>>