> On Thu, 13 Jul 2023, Suraj Kandpal <suraj.kandpal@xxxxxxxxx> wrote:
> > In intel_vdsc_get_config we only read the primary dsc engine register
> > and not take into account if the other dsc engine is in use and if
> > both registers have the same value or not this patche fixes that by
> > adding a check.
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_vdsc.c | 15 ++++++++++++++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > index 530f3c08a172..d48b8306bfc3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > @@ -939,7 +939,7 @@ void intel_dsc_get_config(struct intel_crtc_state
> *crtc_state)
> > enum pipe pipe = crtc->pipe;
> > enum intel_display_power_domain power_domain;
> > intel_wakeref_t wakeref;
> > - u32 dss_ctl1, dss_ctl2, pps0 = 0, pps1 = 0;
> > + u32 dss_ctl1, dss_ctl2, pps0 = 0, pps1 = 0, pps_temp0 = 0, pps_temp1
> > += 1;
> >
> > if (!intel_dsc_source_support(crtc_state))
> > return;
> > @@ -965,11 +965,24 @@ void intel_dsc_get_config(struct intel_crtc_state
> *crtc_state)
> > /* PPS0 & PPS1 */
> > if (!is_pipe_dsc(crtc, cpu_transcoder)) {
> > pps1 = intel_de_read(dev_priv,
> DSCA_PICTURE_PARAMETER_SET_1);
> > + if (crtc_state->dsc.dsc_split) {
> > + pps_temp1 = intel_de_read(dev_priv,
> DSCC_PICTURE_PARAMETER_SET_1);
> > + drm_WARN_ON(&dev_priv->drm, pps1 !=
> pps_temp1);
> > + }
> > +
>
> Superfluous newline.
>
Thanks for the review will fix that
> > } else {
> > pps0 = intel_de_read(dev_priv,
> >
> ICL_DSC0_PICTURE_PARAMETER_SET_0(pipe));
> > pps1 = intel_de_read(dev_priv,
> >
> ICL_DSC0_PICTURE_PARAMETER_SET_1(pipe));
> > + if (crtc_state->dsc.dsc_split) {
> > + pps_temp0 = intel_de_read(dev_priv,
> > +
> ICL_DSC0_PICTURE_PARAMETER_SET_0(pipe));
> > + pps_temp1 = intel_de_read(dev_priv,
> > +
> ICL_DSC0_PICTURE_PARAMETER_SET_1(pipe));
>
> Those are the same two registers as above?
>
Yes they are should have been _DSC1_instead
Regards,
Suraj Kandpal
> BR,
> Jani.
>
> > + drm_WARN_ON(&dev_priv->drm, pps0 !=
> pps_temp0);
> > + drm_WARN_ON(&dev_priv->drm, pps1 !=
> pps_temp1);
> > + }
> > }
> >
> > vdsc_cfg->bits_per_pixel = pps1;
>
> --
> Jani Nikula, Intel Open Source Graphics Center
