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@xxxxxxxx>:
> > 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@xxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxxxx
> > 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
