On Wed, 15 Mar 2017, ville.syrjala@xxxxxxxxxxxxxxx wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Currently ILK-BDW explicitly disable LP1+ watermarks from their > .init_clock_gating() hooks. Unfortunately that hook gets called way too > late since by that time we've already initialized all the watermark > state tracking which then gets out of sync with the hardware state. > > We may eventually want to consider killing off the explicit LP1+ > disable from .init_clock_gating(). In the meantime however, we can > avoid the problem by reordering the init sequence such that > intel_modeset_init_hw()->intel_init_clock_gating() gets called > prior to the hardware state takeover. > > I suppose prior to the two stage watermark programming we were > magically saved by something that forced the watermarks to be > reprogrammed fully after .init_clock_gating() got called. But > now that no longer happens. > > Note that the diff might look a bit odd as it kills off one > call of intel_update_cdclk(), but that's fine because > intel_modeset_init_hw() does the exact same thing. Previously > we just did it twice. > > Actually even this new init sequence is pretty bogus as > .init_clock_gating() really should be called before any gem > hardware init since it can configure various clock gating > workarounds and whatnot that affect the GT side as well. Also > intel_modeset_init() really should get split up into better > defined init stages. Another "fun" detail is that > intel_modeset_gem_init() is where RPS/RC6 gets configured. > Why that is done from the display code is beyond me. I've > decided to leave all this be for now, and just try to fix > the init sequence enough for watermarks to work. > > Cc: stable@xxxxxxxxxxxxxxx > Cc: Gabriele Mazzotta <gabriele.mzt@xxxxxxxxx> > Cc: David Purton <dcpurton@xxxxxxxxxxxxxxx> > Cc: Matt Roper <matthew.d.roper@xxxxxxxxx> > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > Reported-by: Gabriele Mazzotta <gabriele.mzt@xxxxxxxxx> > Reported-by: David Purton <dcpurton@xxxxxxxxxxxxxxx> > Tested-by: Gabriele Mazzotta <gabriele.mzt@xxxxxxxxx> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96645 > Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark programming (v11)") > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Link: http://patchwork.freedesktop.org/patch/msgid/20170220140443.30891-1-ville.syrjala@xxxxxxxxxxxxxxx > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> Thanks, pushed to drm-intel-fixes. BR, Jani. > --- > drivers/gpu/drm/i915/intel_display.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 01341670738f..70be773b3e70 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -16696,12 +16696,11 @@ int intel_modeset_init(struct drm_device *dev) > } > } > > - intel_update_czclk(dev_priv); > - intel_update_cdclk(dev_priv); > - dev_priv->atomic_cdclk_freq = dev_priv->cdclk_freq; > - > intel_shared_dpll_init(dev); > > + intel_update_czclk(dev_priv); > + intel_modeset_init_hw(dev); > + > if (dev_priv->max_cdclk_freq == 0) > intel_update_max_cdclk(dev_priv); > > @@ -17258,8 +17257,6 @@ void intel_modeset_gem_init(struct drm_device *dev) > > intel_init_gt_powersave(dev_priv); > > - intel_modeset_init_hw(dev); > - > intel_setup_overlay(dev_priv); > } -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
