| Message ID | 20171215223545.12305-1-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 15, 2017 at 2:35 PM, Rodrigo Vivi <rodrigo.vivi@intel.com> 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@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@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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, 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)
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@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@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/intel_ddi.c | 4 ++++ 1 file changed, 4 insertions(+)