| Message ID | 20230102113937.9263-1-stanislav.lisovskiy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Implement workaround for DP2 UHBR bandwidth check | expand |
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. 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? > + return -EINVAL; > + } > + } > } > /* > * VDSC engine operates at 1 Pixel per clock, so if peak pixel rate > -- > 2.37.3 >
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 > >
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 */ + 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"); + return -EINVAL; + } + } } /* * VDSC engine operates at 1 Pixel per clock, so if peak pixel rate
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(+)