Re: [PATCH] platform/x86: asus-wmi: store a min default for ppt options

[Ilpo Järvinen]

On Thu, 21 Mar 2024, Luke Jones wrote:

> 
> 
> On Thu, 21 Mar 2024, at 2:13 AM, Ilpo Järvinen wrote:
> > On Mon, 11 Mar 2024, Luke D. Jones wrote:
> > 
> > > Laptops with any of the ppt or nv tunables default to the minimum setting
> > > on boot so we can safely assume a stored value is correct.
> > > 
> > > This patch adds storing of those values in the local struct, and enables
> > > reading of those values back.
> > > 
> > > Secondary to the above it renames some internal variables to be more
> > > consistent (which makes code grepping show all related parts)
> > > 
> > > Signed-off-by: Luke D. Jones <luke@xxxxxxxxxx>
> > > ---
> > >  drivers/platform/x86/asus-wmi.c | 141 +++++++++++++++++++++++++-------
> > >  1 file changed, 111 insertions(+), 30 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > > index e4341abb71e0..482e23b55e1e 100644
> > > --- a/drivers/platform/x86/asus-wmi.c
> > > +++ b/drivers/platform/x86/asus-wmi.c
> > > @@ -272,12 +272,19 @@ struct asus_wmi {
> > >  
> > >  /* Tunables provided by ASUS for gaming laptops */
> > >  bool ppt_pl2_sppt_available;
> > > + u32 ppt_pl2_sppt;
> > >  bool ppt_pl1_spl_available;
> > > + u32 ppt_pl1_spl;
> > >  bool ppt_apu_sppt_available;
> > > - bool ppt_plat_sppt_available;
> > > + u32 ppt_apu_sppt;
> > > + bool ppt_platform_sppt_available;
> > > + u32 ppt_platform_sppt;
> > >  bool ppt_fppt_available;
> > > - bool nv_dyn_boost_available;
> > > - bool nv_temp_tgt_available;
> > > + u32 ppt_fppt;
> > > + bool nv_dynamic_boost_available;
> > > + u32 nv_dynamic_boost;
> > > + bool nv_temp_target_available;
> > > + u32 nv_temp_target;
> > >  
> > >  bool kbd_rgb_mode_available;
> > >  u32 kbd_rgb_dev;
> > 
> > Can you check with pahole if this structure is now full of 31-bit holes?
> > 
> > The benefit of keeping bool & u32 doesn't seem that big to begin with 
> > (in visual sense because of the 1 char variation in column).
> 
> I don't really understand what you mean here. I do most of this stuff 
> when I have time, so because my time is limited I usually don't have 
> opportunity to learn much more than the minimum.

pahole is a tool with which you can look into struct layout. Compile the 
driver with debug info on, and run pahole on the object file, then check 
the layout of the members of that struct (mainly holes in this case).

So enable the debug info in .config if not yet enabled and then run:

$ make drivers/platform/x86/asus-wmi.o
$ pahole -C asus_wmi drivers/platform/x86/asus-wmi.o

-- 
 i.