On Wed, 2026-03-04 at 15:56 +0530, Nautiyal, Ankit K wrote:
>
> On 3/4/2026 2:43 PM, Hogander, Jouni wrote:
> > On Wed, 2026-03-04 at 14:06 +0530, Nautiyal, Ankit K wrote:
> > > On 3/3/2026 6:24 PM, Jouni Högander wrote:
> > > > There are slice row per frame and pic height configuration in
> > > > DSC
> > > > Selective
> > > > Update Parameter Set 1 register. Add helper for configuring
> > > > these.
> > > >
> > > > Bspec: 71709
> > > > Signed-off-by: Jouni Högander <[email protected]>
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_vdsc.c | 22
> > > > ++++++++++++++++++++++
> > > > drivers/gpu/drm/i915/display/intel_vdsc.h | 3 +++
> > > > 2 files changed, 25 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > > index 7e53201b3cb1..ae3af3c2e41a 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > > @@ -820,6 +820,28 @@ void intel_dsc_dp_pps_write(struct
> > > > intel_encoder *encoder,
> > > > sizeof(dp_dsc_pps_sdp));
> > > > }
> > > >
> > > > +void intel_dsc_su_et_parameters_configure(struct intel_dsb
> > > > *dsb,
> > > > struct intel_encoder *encoder,
> > > > + const struct
> > > > intel_crtc_state *crtc_state, int su_lines)
> > > > +{
> > > > + struct intel_display *display =
> > > > to_intel_display(crtc_state);
> > > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state-
> > > > > uapi.crtc);
> > > > + const struct drm_dsc_config *vdsc_cfg = &crtc_state-
> > > > > dsc.config;
> > > > + enum pipe pipe = crtc->pipe;
> > > > + int vdsc_instances_per_pipe =
> > > > intel_dsc_get_vdsc_per_pipe(crtc_state);
> > > > + int slice_row_per_frame = su_lines / vdsc_cfg-
> > > > > slice_height;
> > > > + u32 val;
> > > > +
> > > > + drm_WARN_ON_ONCE(display->drm, su_lines % vdsc_cfg-
> > > > > slice_height);
> > > > +
> > > > + val =
> > > > DSC_SUPS0_SU_SLICE_ROW_PER_FRAME(slice_row_per_frame);
> > > > + val |= DSC_SUPS0_SU_PIC_HEIGHT(su_lines);
> > > > +
> > > > + intel_de_write_dsb(display, dsb,
> > > > LNL_DSC0_SU_PARAMETER_SET_0(pipe), val);
> > > > +
> > > > + if (vdsc_instances_per_pipe > 1)
> > > > + intel_de_write_dsb(display, dsb,
> > > > LNL_DSC1_SU_PARAMETER_SET_0(pipe), val);
> > > Currently 3 DSC engines per pipe is only supported on BMG, which
> > > doesn't
> > > have eDP, so vdsc_instances_per_pipe would never be 3 for now.
> > Yes, but it can be two. Thus > 1.
> > > Furthermore we do not support these registers for BMG AFAICS.
> > >
> > > However later some platform may have 3 VDSC engines and who knows
> > > may
> > > need the Selective Update ET configuration for DSC.
> > >
> > > Since we do not have those registers defined, lets make this
> > > condition
> > > specifically check for `vdsc_instances_per_pipe == 1`
> > We can't do that because instances can be > 1. Actually when
> > running
> > kms_psr2_sf on setup where panel is supporting selective update and
> > dsc
> > this is the case.
>
>
> Sorry I meant `vdsc_instances_per_pipe == 2`.
Ok I will change it to this check. Alltought I don't know if it's any
better to configure only DSC0 if we reach here and
vdsc_instances_per_pipe > 2.
I will also add WARN_ON_ONCE if vdsc_instances_per_pipe > 2.
BR,
Jouni Högander
>
>
> >
> > > We can have perhaps have WARN_ON if vdsc_instances_per_pipe > 2,
> > > at
> > > the
> > > start, as we do not expect the SU ET configuration for 3rd VDSC
> > > engine yet.
> > I see everywhere else in intel_vdsc.c same convention is used. I
> > don't
> > understand why this helper for PSR code should be made different
> > and be
> > responsible for identifying possible DSC configuration issue?
>
> For all the PPS registers we are using, intel_dsc_pps_write() and
> intel_dsc_get_pps_reg() which take care of this.
>
> But yes, I can see there is a miss in 2 places:RC_BUF_THRESH
> registers
> and RC_RANGE_PARAMETER registers, for which 3rd DSC engine thing is
> missing. (I'll check if there are any more instances and rectify this
> soon).
>
> So I recommend filling this register only for 2 vdsc slices per pipe.
>
>
> Regards,
>
> Ankit
>
>
> >
> > BR,
> > Jouni Högander
> >
> > >
> > > Regards,
> > >
> > > Ankit
> > >
> > >
> > > > +}
> > > > +
> > > > static i915_reg_t dss_ctl1_reg(struct intel_crtc *crtc, enum
> > > > transcoder cpu_transcoder)
> > > > {
> > > > return is_pipe_dsc(crtc, cpu_transcoder) ?
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.h
> > > > b/drivers/gpu/drm/i915/display/intel_vdsc.h
> > > > index f4d5b37293cf..3372f8694054 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_vdsc.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.h
> > > > @@ -14,6 +14,7 @@ enum transcoder;
> > > > struct intel_crtc;
> > > > struct intel_crtc_state;
> > > > struct intel_display;
> > > > +struct intel_dsb;
> > > > struct intel_dsc_slice_config;
> > > > struct intel_encoder;
> > > >
> > > > @@ -37,6 +38,8 @@ void intel_dsc_dsi_pps_write(struct
> > > > intel_encoder
> > > > *encoder,
> > > > const struct intel_crtc_state
> > > > *crtc_state);
> > > > void intel_dsc_dp_pps_write(struct intel_encoder *encoder,
> > > > const struct intel_crtc_state
> > > > *crtc_state);
> > > > +void intel_dsc_su_et_parameters_configure(struct intel_dsb
> > > > *dsb,
> > > > struct intel_encoder *encoder,
> > > > + const struct
> > > > intel_crtc_state *crtc_state, int su_lines);
> > > > void intel_vdsc_state_dump(struct drm_printer *p, int
> > > > indent,
> > > > const struct intel_crtc_state
> > > > *crtc_state);
> > > > int intel_vdsc_min_cdclk(const struct intel_crtc_state
> > > > *crtc_state);
