[Intel-gfx] [PATCH v2] drm/i915: Protect DDI port to DPLL map from theoretical race.

Maarten Lankhorst maarten.lankhorst at linux.intel.com
Mon Dec 18 10:40:02 UTC 2017


Op 15-12-17 om 23:43 schreef Rodrigo Vivi:
> In case we have multiple modesets for different connectors
> happening in parallel we could have a race on the RMW on these
> shared registers.
>
> This possibility was initially raised by Paulo when reviewing
> commit '555e38d27317 ("drm/i915/cnl: DDI - PLL mapping")'
> but the original possibility comes from commit '5416d871136d
> ("drm/i915/skl: Set the eDP link rate on DPLL0")'. Or maybe
> later when atomic commits entered into picture.
>
> Apparently the discussion around this topic showed that the
> right solution would be on serializing the atomic commits in
> a way that we don't have the possibility of races here since
> if that parallel modeset happenings apparently many other
> things will be on fire.
>
> Code is there since SKL and there was no report of issue,
> but since we never looked back to that serialization possibility,
> and also we don't have an igt case for that it is better to at
> least protect this corner.
>
> Suggested-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Fixes: 555e38d27317 ("drm/i915/cnl: DDI - PLL mapping")
> Fixes: 5416d871136d ("drm/i915/skl: Set the eDP link rate on DPLL0")
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Maarten Lankhorst maarten.lankhorst at linux.intel.com
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 369f780588fb..f624ba8e23be 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2095,6 +2095,8 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
>  	if (WARN_ON(!pll))
>  		return;
>  
> +	 mutex_lock(&dev_priv->dpll_lock);
> +
>  	if (IS_CANNONLAKE(dev_priv)) {
>  		/* Configure DPCLKA_CFGCR0 to map the DPLL to the DDI. */
>  		val = I915_READ(DPCLKA_CFGCR0);
> @@ -2124,6 +2126,8 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
>  	} else if (INTEL_INFO(dev_priv)->gen < 9) {
>  		I915_WRITE(PORT_CLK_SEL(port), hsw_pll_to_ddi_pll_sel(pll));
>  	}
> +
> +	mutex_unlock(&dev_priv->dpll_lock);
>  }
>  
>  static void intel_ddi_clk_disable(struct intel_encoder *encoder)

What is the difference between v1 and this? Changelog would be nice, or a comment that you resent it. :)

Patch looks sane, so Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>



More information about the Intel-gfx mailing list