[PATCH v3 3/4] drm/i915/display: add module parameter to enable DMC wakelock

[Shankar, Uma]

> -----Original Message-----
> From: Coelho, Luciano <luciano.coelho at intel.com>
> Sent: Monday, March 18, 2024 7:08 PM
> To: intel-gfx at lists.freedesktop.org
> Cc: intel-xe at lists.freedesktop.org; Shankar, Uma <uma.shankar at intel.com>;
> ville.syrjala at linux.intel.com; Nikula, Jani <jani.nikula at intel.com>
> Subject: [PATCH v3 3/4] drm/i915/display: add module parameter to enable DMC
> wakelock
> 
> This feature should be disabled by default until properly tested and mature.  Add
> a module parameter to enable the feature for testing, while keeping it disabled by
> default for now.
> 
> Signed-off-by: Luca Coelho <luciano.coelho at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_params.c |  5 +++++
> drivers/gpu/drm/i915/display/intel_display_params.h |  1 +
>  drivers/gpu/drm/i915/display/intel_dmc_wl.c         | 12 ++++++++----
>  3 files changed, 14 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 11e03cfb774d..f40b223cc8a1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_params.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_params.c
> @@ -116,6 +116,11 @@
> 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,
> +	"Enable DMC wakelock "
> +	"(0=disabled, 1=enabled) "
> +	"Default: 0");
> +
>  __maybe_unused
>  static void _param_print_bool(struct drm_printer *p, const char *driver_name,
>  			      const char *name, bool val)
> diff --git a/drivers/gpu/drm/i915/display/intel_display_params.h
> b/drivers/gpu/drm/i915/display/intel_display_params.h
> index 6206cc51df04..bf8dbbdb20a1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_params.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_params.h
> @@ -46,6 +46,7 @@ struct drm_i915_private;
>  	param(int, enable_psr, -1, 0600) \
>  	param(bool, psr_safest_params, false, 0400) \
>  	param(bool, enable_psr2_sel_fetch, true, 0400) \
> +	param(bool, enable_dmc_wl, false, 0400) \
> 
>  #define MEMBER(T, member, ...) T member;  struct intel_display_params { diff --
> git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
> b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
> index 7c991e22c616..84d054bcb2c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
> +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
> @@ -120,7 +120,8 @@ void intel_dmc_wl_enable(struct drm_i915_private
> *i915)
>  	struct intel_dmc_wl *wl = &i915->display.wl;
>  	unsigned long flags;
> 
> -	if (DISPLAY_VER(i915) < 20)
> +	if (!i915->display.params.enable_dmc_wl ||
> +	    DISPLAY_VER(i915) < 20)

Extend this check to init as well. Else it looks ok to protect under a module parameter.
Reviewed-by: Uma Shankar <uma.shankar at intel.com>

>  		return;
> 
>  	spin_lock_irqsave(&wl->lock, flags);
> @@ -146,7 +147,8 @@ void intel_dmc_wl_disable(struct drm_i915_private
> *i915)
>  	struct intel_dmc_wl *wl = &i915->display.wl;
>  	unsigned long flags;
> 
> -	if (DISPLAY_VER(i915) < 20)
> +	if (!i915->display.params.enable_dmc_wl ||
> +	    DISPLAY_VER(i915) < 20)
>  		return;
> 
>  	flush_delayed_work(&wl->work);
> @@ -177,7 +179,8 @@ void intel_dmc_wl_get(struct drm_i915_private *i915,
> i915_reg_t reg)
>  	struct intel_dmc_wl *wl = &i915->display.wl;
>  	unsigned long flags;
> 
> -	if (DISPLAY_VER(i915) < 20)
> +	if (!i915->display.params.enable_dmc_wl ||
> +	    DISPLAY_VER(i915) < 20)
>  		return;
> 
>  	if (!intel_dmc_wl_check_range(reg.reg))
> @@ -212,7 +215,8 @@ void intel_dmc_wl_put(struct drm_i915_private *i915,
> i915_reg_t reg)
>  	struct intel_dmc_wl *wl = &i915->display.wl;
>  	unsigned long flags;
> 
> -	if (DISPLAY_VER(i915) < 20)
> +	if (!i915->display.params.enable_dmc_wl ||
> +	    DISPLAY_VER(i915) < 20)
>  		return;
> 
>  	if (!intel_dmc_wl_check_range(reg.reg))
> --
> 2.39.2