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

[Jani Nikula <jani.nikula@intel.com>]

On Thu, 08 Jul 2021, "Kulkarni, Vandita" <vandita.kulkarni@intel.com> wrote:
>> -----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.

Okay, if it makes the test easier, I guess it's fine. Up to you.

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx