[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b2be1f6c-4f2e-cbb1-af52-46a85ecd5edd@linux.intel.com>
Date: Tue, 20 Jan 2026 17:10:00 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Krishna Chomal <krishna.chomal108@...il.com>
cc: Hans de Goede <hansg@...nel.org>, platform-driver-x86@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 2/2] platform/x86: hp-wmi: Add EC offsets to read
Victus S thermal profile
On Fri, 16 Jan 2026, Krishna Chomal wrote:
> On Thu, Jan 15, 2026 at 03:26:45PM +0200, Ilpo Järvinen wrote:
> [snip]
> > > +static int platform_profile_victus_s_get_ec(enum platform_profile_option
> > > *profile)
> > > +{
> > > + int ret, i;
> > > + bool current_ctgp_state, current_ppab_state;
> > > + u8 current_dstate, current_gpu_slowdown_temp, tp;
> > > + static const u8 tp_ec_offsets[2] = {
> > > HP_OMEN_EC_THERMAL_PROFILE_OFFSET,
> > > +
> > > HP_VICTUS_S_EC_THERMAL_PROFILE_OFFSET };
> > > +
> > > + /*
> > > + * Victus S devices have more than 1 EC layouts, hence we cannot
> > > directly
> > > + * call omen_thermal_profile_get() like other
> > > platform_profile_*_get_ec()
> > > + * variants, since it would only resolve to that 1 type of board.
> > > Hence
> > > + * we iteratively query a set of candidates: tp_ec_offsets[] until we
> > > + * find a valid thermal profile.
> > > + */
> > > + for (i = 0 ; i < ARRAY_SIZE(tp_ec_offsets) ; i++) {
> > > + ret = ec_read(tp_ec_offsets[i], &tp);
> >
> > I'm not so sure about this. Reading EC offsets and hoping we find the
> > correct one doesn't sound the best idea. I'd prefer we store the
> > information like we already do for thermal profiles. Unless there's some
> > other way to detect which layout it is?
>
> I explored the Omen Gaming Hub (OGH) behavior on Windows to see if a WMI
> query exists for readback. OGH appears to default to "Balanced" on first
> run and tracks state via a profile.json file on the disk. Deleting this
> file causes the app to lose the current state, suggesting that there is no
> official WMI readback query. By implementing EC reads, the driver can
> actually remain more consistent with the real hardware state than the
> offcial software.
>
> I agree that iterative EC reads are not ideal. However, since these two
> offsets (0x95 and 0x59) cover all (or almost all) known Victus/Omen layouts,
> the risk of "hoping" is low.
>
> Storing them at compile time in the victus_s array as a part of
> .driver_data is indeed the best thing. But since we do not know what EC
> layout is followed by the existing boards in the array, we can take a
> hybrid approach here:
> 1. I (and subsequent additions) will store their EC offset in the
> .driver_data field struct.
> 2. For already existing boards we will perform this iterative probe once
> during init, and store it somewhere common.
> 3. Then platform_profile_victus_s_get_ec() can simply use this "definite"
> offset to perform the EC read.
I guess we'll have to settle to that but it likely means we'll never be
able to source those offsets because things appear "working" and therefore
cannot get rid of the extra code necessary for the EC offset iteration.
Another alternative would be to add pr_warn() if we don't have the EC
offset yet for a board and not read anything (and hope somebody who has
one of those boards will come to us with the information or patch).
--
i.
Powered by blists - more mailing lists