On Fri, 2025-01-17 at 19:06 -0300, Gustavo Sousa wrote:
> We use a spinlock to protect DMC wakelock debugfs data, since it is also
> accessed by the core DMC wakelock logic. Taking the spinlock when the
> debugfs is not in use introduces a small but unnecessary penalty.
>
> Since the debugfs functionality is only expected to be used for, uh,
> debugging sessions, let's protect it behind a module parameter
> enable_dmc_wl_debugfs. That way, we only take the lock if the feature
> was enabled in the first place.
>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@xxxxxxxxx>
> ---
Looks good. With a small optional nitpick below.
Reviewed-by: Luca Coelho <luciano.coelho@xxxxxxxxx>
[...]
> diff --git a/drivers/gpu/drm/i915/display/intel_display_params.c b/drivers/gpu/drm/i915/display/intel_display_params.c
> index c4f1ab43fc0c..bc36d1b0ef87 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_params.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_params.c
> @@ -479,9 +488,14 @@ void intel_dmc_wl_debugfs_log_untracked(struct intel_display *display, u32 offse
> bool intel_dmc_wl_debugfs_offset_in_extra_ranges(struct intel_display *display, u32 offset)
> {
> struct intel_dmc_wl_dbg *dbg = &display->wl.dbg;
> - bool ret = false;
> + bool ret;
Why not keep this as it was...
> unsigned long flags;
>
> + if (!display->params.enable_dmc_wl_debugfs)
> + return false;
> +
> + ret = false;
> +
...then you don't need to set it here, and can return ret in the if
above for consistency.
--
Cheers,
Luca.
