[Intel-gfx] [PATCH 7/9] drm/i915: allow package C8+ states on Haswell (disabled)

[Paulo Zanoni]

2013/8/10 Chris Wilson <chris at chris-wilson.co.uk>:
> On Sat, Aug 10, 2013 at 09:55:14AM +0200, Daniel Vetter wrote:
>> On Fri, Aug 9, 2013 at 11:34 PM, Paulo Zanoni <przanoni at gmail.com> wrote:
>> > 2013/8/9 Chris Wilson <chris at chris-wilson.co.uk>:
>> >> Quick note...
>> >>
>> >> On Fri, Aug 09, 2013 at 05:10:05PM -0300, Paulo Zanoni wrote:
>> >>> +     WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock));
>> >>
>> >> Preferred form is now lockdep_assert_held(&dev_priv->pc8.lock);
>> >
>> > Should I also convert all our other usages of
>> > WARN_ON(!mutex_is_locked()) and BUG_ON(!mutex_is_locked()) too? On a
>> > separate patch, of course. We have currently no usage of
>> > lockdep_assert_held, and I like consistency, so fully switching to the
>> > preferred form is good IMHO.
>>
>> Tbh I don't understand really why lockdep_assert_held is better ...
>> it's right that it also checks that indeed the current task is holding
>> the lock (and not some random other imposter). But the downside is
>> that it's a noop without CONFIG_PROVE_LOCKING. And due to the massive
>> perf impact of that option not many people actually run with it. At
>> least I tend to only enable it when doing tricky locking work on my
>> dev machines and not in general ...
>
> It doesn't shout so much and people are starting to complain about all
> the sanity checks existing outside of the grand lockdep.
>
> Who doesn't have a few machines running with lockdep all the time? ;-)

Considering we still don't have a consensus, I'll keep the WARNs so
our driver stays consistent. If I see patches converting everything on
our driver to lockdep_assert_held, then I'll update this patch. IMHO
we should either convert everything or nothing.


> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre



-- 
Paulo Zanoni