Re: [PATCH 13/13] drm/i915/xe3lpd: Use DMC wakelock by default

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

Quoting Luca Coelho (2024-11-01 11:27:10-03:00)
>On Mon, 2024-10-21 at 19:27 -0300, Gustavo Sousa wrote:
>> Although Bspec doesn't explicitly mentions that, as of Xe3_LPD, using
>> DMC wakelock is the officially recommended way of accessing registers
>> that would be off during DC5/DC6 and the legacy method (where the DMC
>> intercepts MMIO to wake up the hardware) is to be avoided.
>> 
>> As such, update the driver to use the DMC wakelock by default starting
>> with Xe3_LPD. Since the feature is somewhat new to the driver, also
>> allow disabling it via a module parameter for debugging purposes.
>> 
>> For that, make the existing parameter allow values -1 (per-chip
>> default), 0 (disabled) and 1 (enabled), similarly to what is done for
>> other parameters.
>> 
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_display_params.c | 4 ++--
>>  drivers/gpu/drm/i915/display/intel_display_params.h | 2 +-
>>  drivers/gpu/drm/i915/display/intel_dmc_wl.c         | 6 +++++-
>>  3 files changed, 8 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_params.c b/drivers/gpu/drm/i915/display/intel_display_params.c
>> index 024de8abcb1a..bf00e5f1f145 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_params.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_params.c
>> @@ -123,10 +123,10 @@ intel_display_param_named_unsafe(enable_psr2_sel_fetch, bool, 0400,
>>          "(0=disabled, 1=enabled) "
>>          "Default: 1");
>>  
>> -intel_display_param_named_unsafe(enable_dmc_wl, bool, 0400,
>> +intel_display_param_named_unsafe(enable_dmc_wl, int, 0400,
>>          "Enable DMC wakelock "
>>          "(0=disabled, 1=enabled) "
>> -        "Default: 0");
>> +        "Default: -1 (use per-chip default)");
>
>We're already explaining the possible values in the previous
>parentheses, so maybe the -1 should also be explained there?

Yep that makes sense. I was following the trend of what was done for
enable_fbc and enable_psr, but I guess following other examples in this
same file where we tag the default one with "[default]" is better.

Thanks! I'll update this on the next version.

--
Gustavo Sousa