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

[Kulkarni, Vandita]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula at intel.com>
> Sent: Thursday, July 8, 2021 9:53 PM
> To: Kulkarni, Vandita <vandita.kulkarni at intel.com>; intel-
> gfx at lists.freedesktop.org
> Subject: RE: [v7 3/3] drm/i915/display/dsc: Force dsc BPP
> 
> On Thu, 08 Jul 2021, "Kulkarni, Vandita" <vandita.kulkarni at intel.com> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani <jani.nikula at intel.com>
> >> Sent: Thursday, July 8, 2021 6:44 PM
> >> To: Kulkarni, Vandita <vandita.kulkarni at intel.com>; intel-
> >> gfx at lists.freedesktop.org
> >> Cc: Kulkarni, Vandita <vandita.kulkarni at intel.com>
> >> Subject: Re: [v7 3/3] drm/i915/display/dsc: Force dsc BPP
> >>
> >> On Thu, 08 Jul 2021, Jani Nikula <jani.nikula at intel.com> wrote:
> >> > On Thu, 08 Jul 2021, Vandita Kulkarni <vandita.kulkarni at 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 at 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.
> 
> Okay, if it makes the test easier, I guess it's fine. Up to you.

Okay,  for now I have sent a patch like you suggested,  as I see that there are no negative test cases .
Have sent the v2 of this patch. So,  it wouldn't make much difference.

Thanks,
Vandita
> 
> BR,
> Jani.
> 
> >
> >>
> >> >> +		/* 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
> 
> --
> Jani Nikula, Intel Open Source Graphics Center