Re: [Intel-gfx] [PATCH 2/2] drm/i915/dsc: use crtc index for bigjoiner primary/secondary crtc

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

On Wed, 09 Jun 2021, Jani Nikula <jani.nikula@intel.com> wrote:
> On Thu, 03 Jun 2021, Jani Nikula <jani.nikula@intel.com> wrote:
>> Pipe numbering isn't guaranteed to be contiguous; there may be fused off
>> pipes in the middle. The current bigjoiner primary/secondary crtc lookup
>> with pipe +/- 1 does not take this into account, and may fail
>> unexpectedly. Fixing this while using pipe numbering gets complicated.
>
> This is broken; pipes need to be contiguous for big joiner regardless of
> whether pipes have been fused off.

Really meant "adjacent", but you get the point.

>
> BR,
> Jani.
>
>>
>> Switch to using crtc index +/- 1 instead. They are contiguous, and the
>> crtc lookup neatly handles overflows and underflows. The performance
>> penalty from the crtc list lookup is neglible.
>>
>> Fixes: 8a029c113b17 ("drm/i915/dp: Modify VDSC helpers to configure DSC for Bigjoiner slave")
>> Fixes: d961eb20adb6 ("drm/i915/bigjoiner: atomic commit changes for uncompressed joiner")
>> Cc: Animesh Manna <animesh.manna@intel.com>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_vdsc.c | 31 +++++++++++------------
>>  1 file changed, 15 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
>> index 1fd81bd3ea09..1d757b040bce 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
>> @@ -1106,30 +1106,29 @@ static i915_reg_t dss_ctl2_reg(const struct intel_crtc_state *crtc_state)
>>  	return is_pipe_dsc(crtc_state) ? ICL_PIPE_DSS_CTL2(pipe) : DSS_CTL2;
>>  }
>>  
>> -struct intel_crtc *
>> -intel_dsc_get_bigjoiner_secondary(const struct intel_crtc *primary_crtc)
>> +static struct intel_crtc *
>> +get_linked_crtc(const struct intel_crtc *crtc, int index)
>>  {
>> -	struct drm_i915_private *i915 = to_i915(primary_crtc->base.dev);
>> -	enum pipe pipe = primary_crtc->pipe + 1;
>> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>> +	struct intel_crtc *linked_crtc;
>>  
>> -	if (drm_WARN_ON(&i915->drm, pipe >= I915_MAX_PIPES ||
>> -			!(INTEL_INFO(i915)->pipe_mask & BIT(pipe))))
>> -		return NULL;
>> +	linked_crtc = to_intel_crtc(drm_crtc_from_index(&i915->drm, index));
>>  
>> -	return intel_get_crtc_for_pipe(i915, pipe);
>> +	drm_WARN_ON(&i915->drm, !linked_crtc);
>> +
>> +	return linked_crtc;
>>  }
>>  
>>  struct intel_crtc *
>> -intel_dsc_get_bigjoiner_primary(const struct intel_crtc *secondary_crtc)
>> +intel_dsc_get_bigjoiner_secondary(const struct intel_crtc *primary_crtc)
>>  {
>> -	struct drm_i915_private *i915 = to_i915(secondary_crtc->base.dev);
>> -	enum pipe pipe = secondary_crtc->pipe - 1;
>> -
>> -	if (drm_WARN_ON(&i915->drm, pipe <= INVALID_PIPE ||
>> -			!(INTEL_INFO(i915)->pipe_mask & BIT(pipe))))
>> -		return NULL;
>> +	return get_linked_crtc(primary_crtc, primary_crtc->base.index + 1);
>> +}
>>  
>> -	return intel_get_crtc_for_pipe(i915, pipe);
>> +struct intel_crtc *
>> +intel_dsc_get_bigjoiner_primary(const struct intel_crtc *secondary_crtc)
>> +{
>> +	return get_linked_crtc(secondary_crtc, secondary_crtc->base.index - 1);
>>  }
>>  
>>  void intel_uncompressed_joiner_enable(const struct intel_crtc_state *crtc_state)

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx