Re: [PATCH] drm/i915: Protect DDI port to DPLL map from theoretical race.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 15, 2017 at 2:35 PM, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote:
> 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@xxxxxxxxx>
> 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@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Maarten Lankhorst maarten.lankhorst@xxxxxxxxxxxxxxx
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
>  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 8467a797a70b..eb816ff2fa77 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,

Shouldn't we also do the same in intel_ddi_clk_disable?


>         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_LOG(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)
> --
> 2.13.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 


James Ausmus
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux