[Intel-gfx] [PATCH 3/3] drm/i915: cancel the panel VDD work when we do it manually

[Paulo Zanoni]

2013/10/31 Jani Nikula <jani.nikula at linux.intel.com>:
> On Thu, 31 Oct 2013, Daniel Vetter <daniel at ffwll.ch> wrote:
>> On Wed, Oct 30, 2013 at 07:50:28PM -0200, Paulo Zanoni wrote:
>>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>>
>>> After I reorganized the panel VDD debug messages I was able to spot we
>>> were disabling it one extra time. The problem is that we're missing
>>> the call to cancel the delayed work in one of the instances where we
>>> manually call ironlake_panel_vdd_off_sync().
>>>
>>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_dp.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 8db1fda..f2280b4 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -1159,6 +1159,7 @@ void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync)
>>>      intel_dp->want_panel_vdd = false;
>>>
>>>      if (sync) {
>>> +            cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
>>
>> This will deadlock since we hold the mode_config.mutex here, and the
>> panel_vdd_work also needs that. I've merged the other two patches.
>
> Ooof, reviewer facepalm.

Actually it was a "developer facepalm" for messing with locks without
lockdep enabled... I thought I had it enabled, but it seems I didn't
have. Lockdep screams almost 100 lines complaining about my code. My
bad.


>
> So we are calling ironlake_panel_vdd_off_sync() twice in this path, but
> that shouldn't matter, as we have
>
> if (!intel_dp->want_panel_vdd && ironlake_edp_have_panel_vdd(intel_dp))
>
> in there, right? Paulo, is that what you're seeing, or something else?

Yeah, I kept investigating the code and reached the same conclusion,
except for the fact that we were still printing "Turning eDP VDD off"
twice without a "Turn eDP VDD on" in the middle. So I kept
investigating and found we were using the wrong register (as I
reported on IRC). I'll submit another patch soon.

>
>
> BR,
> Jani.
>
>
>
>> -Daniel
>>
>>>              ironlake_panel_vdd_off_sync(intel_dp);
>>>      } else {
>>>              /*
>>> --
>>> 1.8.3.1
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center



-- 
Paulo Zanoni