Quoting Gustavo Sousa (2024-11-05 10:46:52-03:00)
>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@xxxxxxxxx>
>>> ---
>>> 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.
Ended up simply doing this:
diff --git a/drivers/gpu/drm/i915/display/intel_display_params.c b/drivers/gpu/drm/i915/display/intel_display_params.c
index bf00e5f1f145..dc666aefa362 100644
--- a/drivers/gpu/drm/i915/display/intel_display_params.c
+++ b/drivers/gpu/drm/i915/display/intel_display_params.c
@@ -125,8 +125,8 @@ intel_display_param_named_unsafe(enable_psr2_sel_fetch, bool, 0400,
intel_display_param_named_unsafe(enable_dmc_wl, int, 0400,
"Enable DMC wakelock "
- "(0=disabled, 1=enabled) "
- "Default: -1 (use per-chip default)");
+ "(-1=use per-chip default, 0=disabled, 1=enabled) "
+ "Default: -1");
__maybe_unused
static void _param_print_bool(struct drm_printer *p, const char *driver_name,
, because repeating the word "default" in "(-1=use per-chip default
[default], 0=disabled, 1=enabled)" looked weird.
--
Gustavo Sousa
>
>Thanks! I'll update this on the next version.
>
>--
>Gustavo Sousa
