[Intel-gfx] [PATCH 39/66] drm/i915: Check hw state in assert_can_disable_lcpll

[Daniel Vetter]

On Thu, May 22, 2014 at 03:10:37PM -0300, Paulo Zanoni wrote:
> 2014-04-24 18:55 GMT-03:00 Daniel Vetter <daniel.vetter at ffwll.ch>:
> > All the other checks also check hw state, so checking our software
> > refcounts for the plls looks a bit odd.
> 
> As I mentioned before, this contradicts your own previous review of
> the patch that added this code. In addition, you said many times that
> we should do SW checks instead of HW checks when possible.
> 
> What should be looking odd here is that we check registers directly
> for the other stuff, instead of looking at some struct :)
> 
> > Also this will simplify the
> > conversion over to the shared dpll framework, which itself has massive
> > amounts of checks to make sure that we never leave a display pll
> > enabled when we shouldn't.
> 
> What you wrote above is a nice reason to check the new shared DPLL
> structs instead of the registers directly: it has tons of WARNs, so
> it's unlikely the structs will be wrong.

If I've done this correctly this should happen a few patches later on. If
not I can throw a new patch on top. I'll check this.
-Daniel
> 
> >
> > So after that conversion we should stil have a good enough coverage of
> > asserts for entering pc8/runtime pm on hsw/bdw.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 1513d9fceebe..22b3d74f9ecc 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6926,7 +6926,6 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> >  static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)
> >  {
> >         struct drm_device *dev = dev_priv->dev;
> > -       struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
> >         struct intel_crtc *crtc;
> >
> >         list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head)
> > @@ -6934,9 +6933,9 @@ static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)
> >                      pipe_name(crtc->pipe));
> >
> >         WARN(I915_READ(HSW_PWR_WELL_DRIVER), "Power well on\n");
> > -       WARN(plls->spll_refcount, "SPLL enabled\n");
> > -       WARN(plls->wrpll1_refcount, "WRPLL1 enabled\n");
> > -       WARN(plls->wrpll2_refcount, "WRPLL2 enabled\n");
> > +       WARN(I915_READ(SPLL_CTL) & SPLL_PLL_ENABLE, "SPLL enabled\n");
> > +       WARN(I915_READ(WRPLL_CTL1) & WRPLL_PLL_ENABLE, "WRPLL1 enabled\n");
> > +       WARN(I915_READ(WRPLL_CTL2) & WRPLL_PLL_ENABLE, "WRPLL2 enabled\n");
> >         WARN(I915_READ(PCH_PP_STATUS) & PP_ON, "Panel power on\n");
> >         WARN(I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE,
> >              "CPU PWM1 enabled\n");
> > --
> > 1.8.1.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch