[PATCH 4/4] drm/i915/dmc_wl: Enable the debugfs only with enable_dmc_wl_debugfs=1

[Luca Coelho]

On Thu, 2025-01-23 at 13:10 -0300, Gustavo Sousa wrote:
> Quoting Luca Coelho (2025-01-22 07:24:43-03:00)
> > 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 at intel.com>
> > > ---
> > 
> > Looks good.  With a small optional nitpick below.
> > 
> > Reviewed-by: Luca Coelho <luciano.coelho at intel.com>
> > 
> > [...]
> > > 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...
> 
> Yeah, I suppose that's fine... I think the compiler is going to optimize
> it. I can send a v2 with this change.
> 
> > 
> > >          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.
> 
> In the if above, I guess I prefer the "return false" because it is
> explicit.

Yeah, fair enough.  It's a matter of preference.

--
Cheers,
Luca.