On Thu, 2024-03-21 at 08:08 +0000, Shankar, Uma wrote:
>
> > -----Original Message-----
> > From: Coelho, Luciano <luciano.coelho@xxxxxxxxx>
> > Sent: Monday, March 18, 2024 7:08 PM
> > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; Shankar, Uma <uma.shankar@xxxxxxxxx>;
> > ville.syrjala@xxxxxxxxxxxxxxx; Nikula, Jani <jani.nikula@xxxxxxxxx>
> > 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@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxx>
Done. I now added the module param check to the supported() helper
function, so it will be checked everywhere.
Thanks for the r-b.
--
Cheers,
Luca.
