Re: [Intel-gfx] [v7 3/3] drm/i915/display/dsc: Force dsc BPP

Kulkarni, Vandita
2021-07-08T13:24:54Z

> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Thursday, July 8, 2021 6:44 PM
> To: Kulkarni, Vandita <vandita.kulkarni@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Kulkarni, Vandita <vandita.kulkarni@intel.com>
> Subject: Re: [v7 3/3] drm/i915/display/dsc: Force dsc BPP
>
> On Thu, 08 Jul 2021, Jani Nikula <jani.nikula@intel.com> wrote:
> > On Thu, 08 Jul 2021, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> >> Set DSC BPP to the value forced through debugfs. It can go from bpc
> >> to bpp-1.
> >>
> >> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/display/intel_dp.c | 17 +++++++++++++++++
> >> 1 file changed, 17 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> >> b/drivers/gpu/drm/i915/display/intel_dp.c
> >> index 5b52beaddada..3e50cdd7e448 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> @@ -1240,6 +1240,23 @@ static int intel_dp_dsc_compute_config(struct
> intel_dp *intel_dp,
> >> pipe_config->port_clock = intel_dp->common_rates[limits-
> >max_clock];
> >> pipe_config->lane_count = limits->max_lane_count;
> >>
> >> + if (intel_dp->force_dsc_en) {
>
> Oh, this should check for intel_dp->force_dsc_bpp. We don't want to always
> force the bpp when we force dsc enable.
Okay will fix this.
And I was returning -EINVAL , to fail the test on setting invalid BPP.

>
> >> + /* As of today we support DSC for only RGB */
> >> + if (intel_dp->force_dsc_bpp >= 8 &&
> >> + intel_dp->force_dsc_bpp < pipe_bpp) {
> >> + drm_dbg_kms(&dev_priv->drm,
> >> + "DSC BPP forced to %d",
> >> + intel_dp->force_dsc_bpp);
> >> + pipe_config->dsc.compressed_bpp =
> >> + intel_dp->force_dsc_bpp;
> >> + } else {
> >> + drm_dbg_kms(&dev_priv->drm,
> >> + "Invalid DSC BPP %d",
> >> + intel_dp->force_dsc_bpp);
> >> + return -EINVAL;
> >
> > I'd just let it use the normal compressed_bpp, with the debug message,
> > instead of returning -EINVAL.
> >
> >> + }
> >> + }
> >> +
> >
> > This should be *after* the below blocks, because otherwise
> > compressed_bpp will be overridden by the normal case, not by the force
> > case!
> >
> > BR,
> > Jani.
> >
> >> if (intel_dp_is_edp(intel_dp)) {
> >> pipe_config->dsc.compressed_bpp =
> >> min_t(u16,
> drm_edp_dsc_sink_output_bpp(intel_dp->dsc_dpcd) >> 4,
>
> --
> Jani Nikula, Intel Open Source Graphics Center