On Mon, Feb 20, 2017 at 04:04:43PM +0200, 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> Pushed to dinq with Daniel's irc r-b. Thanks. 19:22 < vsyrjala> anyone care to review https://patchwork.freedesktop.org/patch/139975/ ? would be one less bug to worry about... 19:28 < danvet> vsyrjala, r-b > --- > 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 730aee755c80..0466db16f193 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -15028,12 +15028,11 @@ int intel_modeset_init(struct drm_device *dev) > } > } > > - intel_update_czclk(dev_priv); > - intel_update_cdclk(dev_priv); > - dev_priv->cdclk.logical = dev_priv->cdclk.actual = dev_priv->cdclk.hw; > - > 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); > > @@ -15591,8 +15590,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); > } > > -- > 2.10.2 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
