[PATCH 01/11] drm/i915/dp: Fix DSC line buffer depth programming

[Imre Deak]

On Tue, Mar 26, 2024 at 12:50:17PM -0700, Manasi Navare wrote:
Hi,

> Hi Imre,
> 
> Thanks for the DSC fixes.
>
> Would the line buf depth calculation that was getting set to 0 impact
> DSC on all platforms or was this issue only specific to MTL and was
> getting set correctly with older platforms?

Only those configs are affected where both the source and the sink supports
DSC1.2, so ADL is not affected.

> We didnt notice any DSC issues/corruptions with ADL based systems.

Yes, that makes sense.

> The actual change makes sense, just want to confirm if this applies to
> all platforms or any particular?

The change will make a difference only on MTL+.

> With that clarification:
> 
> Reviewed-by: Manasi Navare <navaremanasi at chromium.org>

Thanks.

> Regards
> Manasi
> 
> On Tue, Mar 26, 2024 at 3:01 AM Nautiyal, Ankit K
> <ankit.k.nautiyal at intel.com> wrote:
> >
> >
> > On 3/21/2024 1:41 AM, Imre Deak wrote:
> > > Fix the calculation of the DSC line buffer depth. This is limited both
> > > by the source's and sink's maximum line buffer depth, but the former one
> > > was not taken into account. On all Intel platform's the source's maximum
> > > buffer depth is 13, so the overall limit is simply the minimum of the
> > > source/sink's limit, regardless of the DSC version.
> > >
> > > This leaves the DSI DSC line buffer depth calculation as-is, trusting
> > > VBT.
> > >
> > > On DSC version 1.2 for sinks reporting a maximum line buffer depth of 16
> > > the line buffer depth was incorrectly programmed as 0, leading to a
> > > corruption in color gradients / lines on the decompressed screen image.
> > >
> > > Cc: dri-devel at lists.freedesktop.org
> > > Signed-off-by: Imre Deak <imre.deak at intel.com>
> >
> > LGTM.
> >
> > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> >
> > > ---
> > >   drivers/gpu/drm/i915/display/intel_dp.c | 16 ++++++----------
> > >   include/drm/display/drm_dsc.h           |  3 ---
> > >   2 files changed, 6 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index af7ca00e9bc0a..dbe65651bf277 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -89,6 +89,9 @@
> > >   #define DP_DSC_MAX_ENC_THROUGHPUT_0         340000
> > >   #define DP_DSC_MAX_ENC_THROUGHPUT_1         400000
> > >
> > > +/* Max DSC line buffer depth supported by HW. */
> > > +#define INTEL_DP_DSC_MAX_LINE_BUF_DEPTH              13
> > > +
> > >   /* DP DSC FEC Overhead factor in ppm = 1/(0.972261) = 1.028530 */
> > >   #define DP_DSC_FEC_OVERHEAD_FACTOR          1028530
> > >
> > > @@ -1703,7 +1706,6 @@ static int intel_dp_dsc_compute_params(const struct intel_connector *connector,
> > >   {
> > >       struct drm_i915_private *i915 = to_i915(connector->base.dev);
> > >       struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config;
> > > -     u8 line_buf_depth;
> > >       int ret;
> > >
> > >       /*
> > > @@ -1732,20 +1734,14 @@ static int intel_dp_dsc_compute_params(const struct intel_connector *connector,
> > >                       connector->dp.dsc_dpcd[DP_DSC_DEC_COLOR_FORMAT_CAP - DP_DSC_SUPPORT] &
> > >                       DP_DSC_RGB;
> > >
> > > -     line_buf_depth = drm_dp_dsc_sink_line_buf_depth(connector->dp.dsc_dpcd);
> > > -     if (!line_buf_depth) {
> > > +     vdsc_cfg->line_buf_depth = min(INTEL_DP_DSC_MAX_LINE_BUF_DEPTH,
> > > +                                    drm_dp_dsc_sink_line_buf_depth(connector->dp.dsc_dpcd));
> > > +     if (!vdsc_cfg->line_buf_depth) {
> > >               drm_dbg_kms(&i915->drm,
> > >                           "DSC Sink Line Buffer Depth invalid\n");
> > >               return -EINVAL;
> > >       }
> > >
> > > -     if (vdsc_cfg->dsc_version_minor == 2)
> > > -             vdsc_cfg->line_buf_depth = (line_buf_depth == DSC_1_2_MAX_LINEBUF_DEPTH_BITS) ?
> > > -                     DSC_1_2_MAX_LINEBUF_DEPTH_VAL : line_buf_depth;
> > > -     else
> > > -             vdsc_cfg->line_buf_depth = (line_buf_depth > DSC_1_1_MAX_LINEBUF_DEPTH_BITS) ?
> > > -                     DSC_1_1_MAX_LINEBUF_DEPTH_BITS : line_buf_depth;
> > > -
> > >       vdsc_cfg->block_pred_enable =
> > >               connector->dp.dsc_dpcd[DP_DSC_BLK_PREDICTION_SUPPORT - DP_DSC_SUPPORT] &
> > >               DP_DSC_BLK_PREDICTION_IS_SUPPORTED;
> > > diff --git a/include/drm/display/drm_dsc.h b/include/drm/display/drm_dsc.h
> > > index bc90273d06a62..bbbe7438473d3 100644
> > > --- a/include/drm/display/drm_dsc.h
> > > +++ b/include/drm/display/drm_dsc.h
> > > @@ -40,9 +40,6 @@
> > >   #define DSC_PPS_RC_RANGE_MINQP_SHIFT                11
> > >   #define DSC_PPS_RC_RANGE_MAXQP_SHIFT                6
> > >   #define DSC_PPS_NATIVE_420_SHIFT            1
> > > -#define DSC_1_2_MAX_LINEBUF_DEPTH_BITS               16
> > > -#define DSC_1_2_MAX_LINEBUF_DEPTH_VAL                0
> > > -#define DSC_1_1_MAX_LINEBUF_DEPTH_BITS               13
> > >
> > >   /**
> > >    * struct drm_dsc_rc_range_parameters - DSC Rate Control range parameters