[Intel-gfx] [PATCH v2 24/24] drm/i915/display: Use same permissions for enable_sagv as for rest

[Luca Coelho]

On Tue, 2023-10-24 at 15:15 +0300, Jani Nikula wrote:
> On Tue, 24 Oct 2023, Luca Coelho <luca at coelho.fi> wrote:
> > On Tue, 2023-10-24 at 08:51 +0000, Hogander, Jouni wrote:
> > > On Mon, 2023-10-23 at 17:06 +0300, Luca Coelho wrote:
> > > > On Mon, 2023-10-16 at 14:16 +0300, Jouni Högander wrote:
> > > > > Generally we have writable device parameters in debugfs. No need
> > > > > to allow writing module parameters.
> > > > > 
> > > > > Signed-off-by: Jouni Högander <jouni.hogander at intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_display_params.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_params.c
> > > > > b/drivers/gpu/drm/i915/display/intel_display_params.c
> > > > > index 8e6353c1c25e..077f2dee2975 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display_params.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_params.c
> > > > > @@ -50,7 +50,7 @@ intel_display_param_named_unsafe(enable_dc, int,
> > > > > 0400,
> > > > >  intel_display_param_named_unsafe(enable_dpt, bool, 0400,
> > > > >         "Enable display page table (DPT) (default: true)");
> > > > >  
> > > > > -intel_display_param_named_unsafe(enable_sagv, bool, 0600,
> > > > > +intel_display_param_named_unsafe(enable_sagv, bool, 0400,
> > > > >         "Enable system agent voltage/frequency scaling (SAGV)
> > > > > (default: true)");
> > > > >  
> > > > >  intel_display_param_named_unsafe(disable_power_well, int, 0400,
> > > > 
> > > > This, as well as other similar changes throughout this series, could
> > > > be
> > > > controversial, since it's a userspace API change of sorts.  It used
> > > > to
> > > > be possible to write but it won't be anymore.  But, as we discussed
> > > > offline, it shouldn't be problem, because probably nobody is writing
> > > > to
> > > > them, and most likely doing so wouldn't have the expected result,
> > > > since
> > > > the device copies were not getting updated.
> > > > 
> > > > Reviewed-by: Luca Coelho <luciano.coelho at intel.com>
> > > 
> > > Thank you Luca. I actually moved this change to patch 11 due to your
> > > comment there and added your rb tag there. I was planning to drop this
> > > patch. Are you fine with this?
> > 
> > Yes, this is fine.  I'll review your cahnges in v3 and give the missing
> > r-b tags there, if applicable.
> 
> I think this change is good and frankly needed. It's confusing to be
> able to modify the module param without it having any effect.
> 
> These are for debug, the param is designated "unsafe", and for these I
> don't really care if someone complains they can't write to the file
> anymore.

Right, this was my conclusion as well, and thus, got my r-b. :)

--
Cheers,
Luca.