Re: [Intel-gfx] [PATCH 08/13] drm/i915/dp: Consider output_format while computing dsc bpp

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 4/24/2023 6:21 PM, Ville Syrjälä wrote:
> On Fri, Mar 31, 2023 at 03:46:08PM +0530, Ankit Nautiyal wrote:
>> While using DSC the compressed bpp is computed assuming RGB output
>> format. Consider the output_format and compute the compressed bpp
>> during mode valid and compute config steps.
>>
>> For DP-MST we currently use RGB output format only, so continue
>> using RGB while computing compressed bpp for MST case.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c     | 14 +++++++++++++-
>>   drivers/gpu/drm/i915/display/intel_dp.h     |  1 +
>>   drivers/gpu/drm/i915/display/intel_dp_mst.c |  1 +
>>   3 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 4d5c74a1bd29..e5903b5e511b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -712,6 +712,7 @@ u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
>>   				u32 link_clock, u32 lane_count,
>>   				u32 mode_clock, u32 mode_hdisplay,
>>   				bool bigjoiner,
>> +				enum intel_output_format output_format,
>>   				u32 pipe_bpp,
>>   				u32 timeslots)
>>   {
>> @@ -736,6 +737,10 @@ u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
>>   	bits_per_pixel = ((link_clock * lane_count) * timeslots) /
>>   			 (intel_dp_mode_to_fec_clock(mode_clock) * 8);
>>   
>> +	/* Bandwidth required for 420 is half, that of 444 format */
>> +	if (output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
>> +		bits_per_pixel *= 2;
> /2 ?

As per my understanding, we are trying to get the max bits_per_pixel 
that is possible with the link config for a given mode:

Bandwidth required with 444 should be <= Available link Bandwidth

So for a given mode with 444 output format:

ModeClock * bits_per_pixel_444 <= Available Link Bandwidth
bits_per_pixel_444 <= Available Link Bandwidth / ModeClock

For 420 output format, bandwidth required is half that of 444. (So 
bigger bits_per_pixel is possible with the same link config)

(ModeClock * bits_per_pixel_420) / 2 <= Available Link Bandwidth

or bits_per_pixel_420 <= 2 * (Available Link Bandwidth / ModeClock)

or bits_per_pixel_420 <= 2 * bits_per_pixel_444.

Perhaps it will be better to make a helper to do this based on 
output_format.

Regards,

Ankit




>
>> +
>>   	drm_dbg_kms(&i915->drm, "Max link bpp is %u for %u timeslots "
>>   				"total bw %u pixel clock %u\n",
>>   				bits_per_pixel, timeslots,
>> @@ -1133,11 +1138,16 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>>   
>>   	if (HAS_DSC(dev_priv) &&
>>   	    drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)) {
>> +		enum intel_output_format sink_format, output_format;
>> +		int pipe_bpp;
>> +
>> +		sink_format = intel_dp_sink_format(connector, mode);
>> +		output_format = intel_dp_output_format(connector, sink_format);
>>   		/*
>>   		 * TBD pass the connector BPC,
>>   		 * for now U8_MAX so that max BPC on that platform would be picked
>>   		 */
>> -		int pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, U8_MAX);
>> +		pipe_bpp = intel_dp_dsc_compute_bpp(intel_dp, U8_MAX);
>>   
>>   		/*
>>   		 * Output bpp is stored in 6.4 format so right shift by 4 to get the
>> @@ -1157,6 +1167,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>>   							    target_clock,
>>   							    mode->hdisplay,
>>   							    bigjoiner,
>> +							    output_format,
>>   							    pipe_bpp, 64) >> 4;
>>   			dsc_slice_count =
>>   				intel_dp_dsc_get_slice_count(intel_dp,
>> @@ -1655,6 +1666,7 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>>   							    adjusted_mode->crtc_clock,
>>   							    adjusted_mode->crtc_hdisplay,
>>   							    pipe_config->bigjoiner_pipes,
>> +							    pipe_config->output_format,
>>   							    pipe_bpp,
>>   							    timeslots);
>>   			if (!dsc_max_output_bpp) {
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
>> index ef39e4f7a329..db86c2b71c1f 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.h
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
>> @@ -107,6 +107,7 @@ u16 intel_dp_dsc_get_output_bpp(struct drm_i915_private *i915,
>>   				u32 link_clock, u32 lane_count,
>>   				u32 mode_clock, u32 mode_hdisplay,
>>   				bool bigjoiner,
>> +				enum intel_output_format output_format,
>>   				u32 pipe_bpp,
>>   				u32 timeslots);
>>   u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> index 2cc4239cfa5b..daa1591a9ae8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> @@ -927,6 +927,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>>   							    target_clock,
>>   							    mode->hdisplay,
>>   							    bigjoiner,
>> +							    INTEL_OUTPUT_FORMAT_RGB,
>>   							    pipe_bpp, 64) >> 4;
>>   			dsc_slice_count =
>>   				intel_dp_dsc_get_slice_count(intel_dp,
>> -- 
>> 2.25.1