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

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Dec 18, 2017 at 10:40:02AM +0000, Maarten Lankhorst wrote:
> 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@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 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. :)

no difference at all...  but I forgot to add the mention of the rebase on the right branch.

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

Thanks... Merging this now...

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