Re: [PATCH] drm/i915: fix rps.vlv_work initialization

Ville Syrjälä
2013-10-01T15:55:39Z

On Tue, Oct 01, 2013 at 06:11:26PM +0300, Imre Deak wrote:
> During driver loading we are initializing rps.vlv_work in
> valleyview_enable_rps() via the rps.delayed_resume_work delayed work.
> This is too late since we are using vlv_work already via
> i915_driver_load()->intel_uncore_sanitize()->
> intel_disable_gt_powersave(). This at least leads to the following
> kernel warning:
>
> INFO: trying to register non-static key.
> the code is fine but needs lockdep annotation.
> turning off the locking correctness validator.
>
> Fix this by initialzing vlv_work before we call intel_uncore_sanitize().
>
> The regression was introduced in
>
> commit 7dcd2677ea912573d9ed4bcd629b0023b2d11505
> Author: Konstantin Khlebnikov <khlebnikov@openvz.org>
> Date: Wed Jul 17 10:22:58 2013 +0400
>
> drm/i915: fix long-standing SNB regression in power consumption
> after resume
>
> though there was no good reason to initialize the static vlv_work from
> another delayed work to begin with (especially since this will happen
> multiple times).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=69397
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 698257c..2a0a340 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3894,8 +3894,6 @@ static void valleyview_enable_rps(struct drm_device *dev)
> dev_priv->rps.rpe_delay),
> dev_priv->rps.rpe_delay);
>
> - INIT_DELAYED_WORK(&dev_priv->rps.vlv_work, vlv_rps_timer_work);
> -
> valleyview_set_rps(dev_priv->dev, dev_priv->rps.rpe_delay);
>
> gen6_enable_rps_interrupts(dev);
> @@ -5805,5 +5803,7 @@ void intel_pm_init(struct drm_device *dev)
>
> INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
> intel_gen6_powersave_work);
> +
> + INIT_DELAYED_WORK(&dev_priv->rps.vlv_work, vlv_rps_timer_work);

We're initializing rps.work in intel_irq_init(). Maybe we should try to
keep rps related stuff together?

> }
>
> --
> 1.8.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel OTC