Re: [PATCH] drm/i915: Don't del_timer_sync uninitialized timer

From: Daniel Vetter <daniel@ffwll.ch>
To: Ben Widawsky <ben@bwidawsk.net>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>,
	Intel GFX <intel-gfx@lists.freedesktop.org>,
	Ben Widawsky <benjamin.widawsky@intel.com>
Subject: Re: [PATCH] drm/i915: Don't del_timer_sync uninitialized timer
Date: Sat, 15 Mar 2014 20:13:44 +0100	[thread overview]
Message-ID: <CAKMK7uEis=QZ5rYDneezD091p1J2JvV7icBRZDxQDGxiMUzK+Q@mail.gmail.com> (raw)
In-Reply-To: <20140315183018.GA9156@bwidawsk.net>

On Sat, Mar 15, 2014 at 7:30 PM, Ben Widawsky <ben@bwidawsk.net> wrote:
>> > > + setup_timer(&dev_priv->uncore.force_wake_timer,
>> > > +             gen6_force_wake_timer, (unsigned long)dev_priv);
>> >
>> > We call early_sanitize also from our resume code, so this will now
>> > re-setup the timer again. We generally don't do that since if we ever leak
>> > the timer to here in an enabled state it causes havoc.
>>
>> Gah, really? intel_uncore_early_init()! There must be a clean way to
>> break this up.
>> -Chris
>
> At least in the code base I was looking at, we currently do this also,
> so I didn't think this was any worse.

Hm, where? Imo such a thing would be a bug.

> With lockdep turned on, the module will not even load, so please either
> revert the original, or merge this.

I think we can just move the call to early_sanitize into uncore_init,
right after the timer setup.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch