Re: [PATCH v2 19/25] drm/i915: reinit GT power save during resume

Ville Syrjälä
2014-04-16T17:46:55Z

On Mon, Apr 14, 2014 at 08:24:40PM +0300, Imre Deak wrote:
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b87109c..1f88917 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -920,6 +920,7 @@ static int intel_runtime_suspend(struct device *device)
> DRM_DEBUG_KMS("Suspending device\n");
>
> intel_runtime_pm_disable_interrupts(dev);
> + cancel_work_sync(&dev_priv->rps.work);

What happens if the rps work still runs after
intel_runtime_pm_disable_interrupts()? To me it looks like it can hit
the WARN in snb_update_pm_irq(), or there's a race and it'll miss the
dev_priv->pm.irqs_disabled update and unmask something in PMIMR.

Hmm, I guess the same issue already exists in intel_disable_gt_powersave().

But since it's just PMIMR I guess it's not really dangerous since IER
isn't touched so we don't actually get any interrupts. But it feels a bit
fragile.

>
> if (IS_GEN6(dev))
> ;
> @@ -968,6 +969,7 @@ static int intel_runtime_resume(struct device *device)
>
> i915_gem_init_swizzling(dev);
> gen6_update_ring_freq(dev);
> + intel_reset_gt_powersave(dev);
>
> intel_runtime_pm_restore_interrupts(dev);
>
> --
> 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