On Thu, 03 Jun 2021, Jani Nikula <[email protected]> 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 <[email protected]>
> Cc: Manasi Navare <[email protected]>
> Cc: Vandita Kulkarni <[email protected]>
> Signed-off-by: Jani Nikula <[email protected]>
> ---
> 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
