Re: [PATCH 04/13] drm/i915/dmc_wl: Get wakelock when disabling dynamic DC states

[Gustavo Sousa <gustavo.sousa@intel.com>]

Quoting Luca Coelho (2024-11-01 09:24:08-03:00)
>On Mon, 2024-10-21 at 19:27 -0300, Gustavo Sousa wrote:
>> Bspec says that disabling dynamic DC states require taking the DMC
>> wakelock to cause an DC exit before writing to DC_STATE_EN. Implement
>> that.
>> 
>> In fact, testing on PTL revealed we end up failing to exit DC5/6 without
>> this step.
>> 
>> Bspec: 71583
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  .../drm/i915/display/intel_display_power_well.c    | 10 +++++++---
>>  drivers/gpu/drm/i915/display/intel_dmc_wl.c        | 14 ++++++++++++--
>>  drivers/gpu/drm/i915/display/intel_dmc_wl.h        |  2 ++
>>  3 files changed, 21 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c
>> index adaf7cf3a33b..e8946ce86aaa 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
>> @@ -987,10 +987,14 @@ void gen9_disable_dc_states(struct intel_display *display)
>>                  return;
>>          }
>>  
>> -        gen9_set_dc_state(display, DC_STATE_DISABLE);
>> -
>> -        if (!HAS_DISPLAY(display))
>> +        if (HAS_DISPLAY(display)) {
>> +                intel_dmc_wl_get_noreg(display);
>> +                gen9_set_dc_state(display, DC_STATE_DISABLE);
>> +                intel_dmc_wl_put_noreg(display);
>> +        } else {
>> +                gen9_set_dc_state(display, DC_STATE_DISABLE);
>>                  return;
>> +        }
>
>I think intel_dmc_get/put() already protect indirectly on
>HAS_DISPLAY(), doesn't it? If that's the case, then the if here is
>unnecessary.

Actually, intel_dmc_wl_init() gets called only when HAS_DISPLAY() is
true, so I think using intel_dmc_wl_{get,put}_noreg() for the
!HAS_DISPLAY() case would not be right, at least not with the current
state of the code.

--
Gustavo Sousa