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

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

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.

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