On Tue, 2023-10-24 at 15:15 +0300, Jani Nikula wrote: > On Tue, 24 Oct 2023, Luca Coelho <luca@xxxxxxxxx> 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@xxxxxxxxx> > > > > > --- > > > > > 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@xxxxxxxxx> > > > > > > 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.
