2013/12/10 Daniel Vetter <daniel@xxxxxxxx>:
> On Fri, Nov 29, 2013 at 11:03:38AM -0200, Rodrigo Vivi wrote:
>> On Wed, Nov 27, 2013 at 06:20:34PM -0200, Paulo Zanoni wrote:
>> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> > index 2715600..70c4cef 100644
>> > --- a/drivers/gpu/drm/i915/i915_irq.c
>> > +++ b/drivers/gpu/drm/i915/i915_irq.c
>> > @@ -2477,6 +2477,12 @@ static void i915_hangcheck_elapsed(unsigned long data)
>> > if (!i915_enable_hangcheck)
>> > return;
>> >
>> > + /* Just postpone in case we're completely idle... */
>> > + if (dev_priv->pm.suspended) {
>> > + i915_queue_hangcheck(dev);
>> > + return;
>> > + }
>>
>> I got the idea here, just not sure if it fits on this patch or it is just the commit message.
>
> This hunk here is the wrong thing to do: If we're suspended and the
> hangcheck fires, we'll just delay it. But if we keep on being suspended
> the hangcheck will fire at the normal recheck rate (but never do anything)
> which wreaks utter havoc with deep sleep residency. The simples solution
> would be to synchronously cancel the timer in the idle work (where we
> already put the execbuf runtime ref) I think.
>
The problem is that if I do this, then we won't end up running the
hangcheck (which we always run today), so we won't check for hangs,
which is probably not good. I could add "i915_force_hangcheck" what
would be called at intel_mark_idle if you would like, but I'm not too
sure what are the downsides of doing this.
I also tried getting/putting a refcount on the hangcheck function, but
I get complaints that it's sleeping from an invalid context.
Which approach is the best?
> I've dropped this hunk from the patch when merging.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
