On Fri, Jul 12, 2013 at 02:19:42PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>
> Most of the hardware needs to be disabled before LCPLL is disabled, so
> let's add a function to assert some of items listed in the "Display
> Sequences for LCPLL disabling" documentation.
>
> The idea is that hsw_disable_lcpll should not disable the hardware,
> the callers need to take care of calling hsw_disable_lcpll only once
> everything is already disabled.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
I don't see a reason to separate this patch from the previous one. It
makes review easier, but it's weird bisect wise. The correct order, if
you wanted to do them as separate patches would be to introduce the
assertions, and then add the functionality (I think).
> ---
> drivers/gpu/drm/i915/i915_reg.h | 8 ++++++++
> drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8e5a5ec..9556dff 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2195,6 +2195,8 @@
> #define BLC_PWM_CPU_CTL2 0x48250
> #define BLC_PWM_CPU_CTL 0x48254
>
> +#define HSW_BLC_PWM2_CTL 0x48350
> +
> /* PCH CTL1 is totally different, all but the below bits are reserved. CTL2 is
> * like the normal CTL from gen4 and earlier. Hooray for confusing naming. */
> #define BLC_PWM_PCH_CTL1 0xc8250
> @@ -2203,6 +2205,12 @@
> #define BLM_PCH_POLARITY (1 << 29)
> #define BLC_PWM_PCH_CTL2 0xc8254
>
> +#define UTIL_PIN_CTL 0x48400
> +#define UTIL_PIN_ENABLE (1 << 31)
> +
> +#define PCH_GTC_CTL 0xe7000
> +#define PCH_GTC_ENABLE (1 << 31)
> +
> /* TV port control */
> #define TV_CTL 0x68000
> /** Enables the TV encoder */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ffb08bf..9055f60 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5922,6 +5922,32 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> return true;
> }
>
> +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)
> + WARN(crtc->base.enabled, "CRTC for pipe %c enabled\n",
> + 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(PCH_PP_STATUS) & PP_ON, "Panel power on\n");
> + WARN(I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE,
> + "CPU PWM1 enabled\n");
> + WARN(I915_READ(HSW_BLC_PWM2_CTL) & BLM_PWM_ENABLE,
> + "CPU PWM2 enabled\n");
> + WARN(I915_READ(BLC_PWM_PCH_CTL1) & BLM_PCH_PWM_ENABLE,
> + "PCH PWM1 enabled\n");
> + WARN(I915_READ(UTIL_PIN_CTL) & UTIL_PIN_ENABLE,
> + "Utility pin enabled\n");
> + WARN(I915_READ(PCH_GTC_CTL) & PCH_GTC_ENABLE, "PCH GTC enabled\n");
Looking at probably the same list you've looked at, I don't see:
Audio controller
eDP HPD
Other CPU/PCH interrupts
You've worked with this code longer than I have, so maybe you can
explain why you've skipped them.
> +}
> +
> /*
> * This function implements pieces of two sequences from BSpec:
> * - Sequence for display software to disable LCPLL
> @@ -5935,6 +5961,8 @@ void hsw_disable_lcpll(struct drm_i915_private *dev_priv,
> {
> uint32_t val;
>
> + assert_can_disable_lcpll(dev_priv);
> +
Do we want to proceed if the above fails? I was sort of under the
impression that hard hangs can occur as a result of some functions still
being enabled when we change clocks. I'm fine with continuing on since
we have the WARNS, just wondering if you've thought about it.
> val = I915_READ(LCPLL_CTL);
>
> if (switch_to_fclk) {
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
