Received-SPF: pass (google.com: domain of linux-kernel+bounces-109957-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Thu, 21 Mar 2024 13:23:59 +0200 (EET)
To: Luke Jones <luke@ljones.dev>
cc: platform-driver-x86@vger.kernel.org, Hans de Goede <hdegoede@redhat.com>, 
    LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] platform/x86: asus-wmi: store a min default for ppt
 options
In-Reply-To: <1006a1d3-86f2-4770-9cf2-4f7e3e12a787@app.fastmail.com>
Message-ID: <954e6e5c-dfeb-68b7-6e02-af26ef92c572@linux.intel.com>
References: <20240310233722.30884-1-luke@ljones.dev> <ebe48668-dfca-775c-880e-dfa333b7e562@linux.intel.com> <1006a1d3-86f2-4770-9cf2-4f7e3e12a787@app.fastmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323328-648248580-1711020239=:1077"

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-648248580-1711020239=:1077
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Thu, 21 Mar 2024, Luke Jones wrote:

>=20
>=20
> On Thu, 21 Mar 2024, at 2:13 AM, Ilpo J=C3=A4rvinen wrote:
> > On Mon, 11 Mar 2024, Luke D. Jones wrote:
> >=20
> > > Laptops with any of the ppt or nv tunables default to the minimum set=
ting
> > > on boot so we can safely assume a stored value is correct.
> > >=20
> > > This patch adds storing of those values in the local struct, and enab=
les
> > > reading of those values back.
> > >=20
> > > Secondary to the above it renames some internal variables to be more
> > > consistent (which makes code grepping show all related parts)
> > >=20
> > > Signed-off-by: Luke D. Jones <luke@ljones.dev>
> > > ---
> > >  drivers/platform/x86/asus-wmi.c | 141 +++++++++++++++++++++++++-----=
--
> > >  1 file changed, 111 insertions(+), 30 deletions(-)
> > >=20
> > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/a=
sus-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 {
> > > =20
> > >  /* 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;
> > > =20
> > >  bool kbd_rgb_mode_available;
> > >  u32 kbd_rgb_dev;
> >=20
> > Can you check with pahole if this structure is now full of 31-bit holes=
?
> >=20
> > The benefit of keeping bool & u32 doesn't seem that big to begin with=
=20
> > (in visual sense because of the 1 char variation in column).
>=20
> I don't really understand what you mean here. I do most of this stuff=20
> when I have time, so because my time is limited I usually don't have=20
> opportunity to learn much more than the minimum.

pahole is a tool with which you can look into struct layout. Compile the=20
driver with debug info on, and run pahole on the object file, then check=20
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

--=20
 i.

--8323328-648248580-1711020239=:1077--