Re: [Intel-gfx] [PATCH] drm/i915: Implement workaround for DP2 UHBR bandwidth check

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

On Tue, Jan 03, 2023 at 11:32:28AM -0500, Rodrigo Vivi wrote:
> 
> on the subject: This is not a hw workaround. Please remove the workaround from
> the subject and the wrong comment.
> 
> "The HSD given is a 'feature' rather than 'bugeco' so no workaround details are
> present here."
> 
> 
> On Mon, Jan 02, 2023 at 01:39:37PM +0200, Stanislav Lisovskiy wrote:
> > According to spec, we should check if output_bpp * pixel_rate is less
> > than DDI clock * 72, if UHBR is used.
> > 
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index bf80f296a8fdb..13baf3cb5f934 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1582,6 +1582,17 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> >  		drm_dbg_kms(&dev_priv->drm, "DSC: compressed bpp %d slice count %d\n",
> >  			    pipe_config->dsc.compressed_bpp,
> >  			    pipe_config->dsc.slice_count);
> > +
> > +		/* wa1406899791 */
> 
> even if it was a bugeco, the notation doesn't follow the standard.
> 
> But anyway, as I pointed out, this is not a workaround so
> you probably just want a
> 
> HSDES: 1406899791
> BSPEC: 49259
> 
> in your commit msg.

Ok, will add this thanks.

> 
> Also maybe a "Fixes:" tag pointing to the commit that added the sequence
> but didn't added this part of the sequence?
> 
> > +		if (intel_dp_is_uhbr(pipe_config)) {
> > +			int output_bpp = pipe_config->dsc.compressed_bpp;
> > +
> > +			if (output_bpp * adjusted_mode->crtc_clock >=
> > +			    pipe_config->port_clock * 72) {
> > +				drm_dbg_kms(&dev_priv->drm, "DP2 UHBR check failed\n");
> 
> some probably dummy question:
> do we need to add a check for the DP 2.0 above as well?
> or it is unecessary/redundant?

I think this check is more related to hardware limitation, rather than
DP 2.0 standard. I mean if it was even not DP 2.0, but still UHBR I really doubt
that this limitation wouldn't be essential still.

Stan

> 
> > +				return -EINVAL;
> > +			}
> > +		}
> >  	}
> >  	/*
> >  	 * VDSC engine operates at 1 Pixel per clock, so if peak pixel rate
> > -- 
> > 2.37.3
> >