All of lore.kernel.org
 help / color / mirror / Atom feed
From: Denis Benato <denis.benato@linux.dev>
To: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: busybox11 <busybox11th@gmail.com>,
	Corentin Chary <corentin.chary@gmail.com>,
	"Luke D. Jones" <luke@ljones.dev>,
	Hans de Goede <hansg@kernel.org>,
	platform-driver-x86@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] platform/x86: asus-armoury: add dGPU disable fallback DEVID for ProArt H7606 series
Date: Tue, 15 Sep 2026 23:03:12 +0200	[thread overview]
Message-ID: <246a7bf2-a51b-442f-97aa-c767f8c02f8c@linux.dev> (raw)
In-Reply-To: <c2d620e4-25ba-c31b-0143-4a53ff3edd8b@linux.intel.com>


On 9/15/26 22:56, Ilpo Järvinen wrote:
> On Tue, 15 Sep 2026, Denis Benato wrote:
>
>> On 9/15/26 22:38, Ilpo Järvinen wrote:
>>> On Mon, 10 Aug 2026, busybox11 via B4 Relay wrote:
>>>
>>>> From: busybox11 <busybox11th@gmail.com>
>>>>
>>>> Newer ASUS ProArt laptops (e.g. H7606 series) implement dGPU power
>>>> control on WMI DEVID 0x00090120 instead of the usual 0x00090020.
>>>> The default DEVID returns 0xFFFFFFFE on these models, so the
>>>> dgpu_disable firmware attribute is never created and the dGPU cannot
>>>> be re-enabled from Linux at all.
>>>>
>>>> Add a fallback probe for device ID 0x00090120 following the same
>>>> approach as the gpu_mux_dev_id probe.
>>>>
>>>> Details for 0x00090120 (confirmed on H7606W using direct WMNB calls
>>>> and inspecting the DSDT DEVS handler):
>>>>   - Reading DSTS returns 0x00010001 when the dGPU is off (CUMA=1),
>>>> 	or 0x00010000 when it's on.
>>>>   - Writing DEVS: 0 enables the dGPU (calls PG00._ON() + Notifies
>>>> 	PEGP, Device Check), and 1 disables or ejects it.
>>>>
>>>> This behavior matches the dgpu_disable attribute (1 = disabled).
>>>>
>>>> Signed-off-by: busybox11 <busybox11th@gmail.com>
>>> Unfortunately, we cannot accept anonymous or pseudonym signed 
>>> contributions so please sign off with your real name. For more 
>>> information about signing of patches, please see:
>>>
>>> Documentation/process/submitting-patches.rst
>>>
>>>> ---
>>>>  drivers/platform/x86/asus-armoury.c        | 32 +++++++++++++++++++++++++-----
>>>>  include/linux/platform_data/x86/asus-wmi.h |  3 +++
>>>>  2 files changed, 30 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
>>>> index 495dc1e31..f39b52451 100644
>>>> --- a/drivers/platform/x86/asus-armoury.c
>>>> +++ b/drivers/platform/x86/asus-armoury.c
>>>> @@ -93,6 +93,7 @@ struct asus_armoury_priv {
>>>>  
>>>>  	u32 mini_led_dev_id;
>>>>  	u32 gpu_mux_dev_id;
>>>> +	u32 dgpu_disable_dev_id;
>>>>  };
>>>>  
>>>>  static struct asus_armoury_priv asus_armoury = {
>>>> @@ -452,8 +453,8 @@ static ssize_t gpu_mux_mode_current_value_store(struct kobject *kobj,
>>>>  	if (err)
>>>>  		return err;
>>>>  
>>>> -	if (armoury_has_devstate(ASUS_WMI_DEVID_DGPU)) {
>>>> -		err = armoury_get_devstate(NULL, &result, ASUS_WMI_DEVID_DGPU);
>>>> +	if (asus_armoury.dgpu_disable_dev_id) {
>>>> +		err = armoury_get_devstate(NULL, &result, asus_armoury.dgpu_disable_dev_id);
>>>>  		if (err)
>>>>  			return err;
>>>>  		if (result && !optimus) {
>>>> @@ -507,7 +508,8 @@ static ssize_t dgpu_disable_current_value_store(struct kobject *kobj,
>>>>  	}
>>>>  
>>>>  	scoped_guard(mutex, &asus_armoury.egpu_mutex) {
>>>> -		err = armoury_set_devstate(attr, disable ? 1 : 0, NULL, ASUS_WMI_DEVID_DGPU);
>>>> +		err = armoury_set_devstate(attr, disable ? 1 : 0, NULL,
>>>> +					   asus_armoury.dgpu_disable_dev_id);
>>>>  		if (err)
>>>>  			return err;
>>>>  	}
>>>> @@ -516,7 +518,7 @@ static ssize_t dgpu_disable_current_value_store(struct kobject *kobj,
>>>>  
>>>>  	return count;
>>>>  }
>>>> -ASUS_WMI_SHOW_INT(dgpu_disable_current_value, ASUS_WMI_DEVID_DGPU);
>>>> +ASUS_WMI_SHOW_INT(dgpu_disable_current_value, asus_armoury.dgpu_disable_dev_id);
>>>>  ASUS_ATTR_GROUP_BOOL(dgpu_disable, "dgpu_disable", "Disable the dGPU");
>>>>  
>>>>  /* Values map for eGPU activation requests. */
>>>> @@ -790,7 +792,6 @@ ASUS_ATTR_GROUP_INT_VALUE_ONLY_RO(nv_base_tgp, ATTR_NV_BASE_TGP, ASUS_WMI_DEVID_
>>>>  static const struct asus_attr_group armoury_attr_groups[] = {
>>>>  	{ &egpu_connected_attr_group, ASUS_WMI_DEVID_EGPU_CONNECTED },
>>>>  	{ &egpu_enable_attr_group, ASUS_WMI_DEVID_EGPU },
>>>> -	{ &dgpu_disable_attr_group, ASUS_WMI_DEVID_DGPU },
>>>>  	{ &apu_mem_attr_group, ASUS_WMI_DEVID_APU_MEM },
>>>>  
>>>>  	{ &ppt_pl1_spl_attr_group, ASUS_WMI_DEVID_PPT_PL1_SPL },
>>>> @@ -934,6 +935,21 @@ static int asus_fw_attr_add(void)
>>>>  		}
>>>>  	}
>>>>  
>>>> +	asus_armoury.dgpu_disable_dev_id = 0;
>>>> +	if (armoury_has_devstate(ASUS_WMI_DEVID_DGPU))
>>>> +		asus_armoury.dgpu_disable_dev_id = ASUS_WMI_DEVID_DGPU;
>>>> +	else if (armoury_has_devstate(ASUS_WMI_DEVID_GPU_MODE))
>>>> +		asus_armoury.dgpu_disable_dev_id = ASUS_WMI_DEVID_GPU_MODE;
>>>> +
>>>> +	if (asus_armoury.dgpu_disable_dev_id) {
>>> Now I started to wonder why are we doing it like this and not using 
>>> .is_visible?
>>>
>>> This question applies to the similar, existing cases as well.
>>>
>>> Denis?
>>>
>>>
>> I don't understand the question. There are a bunch of .is_visible in the kernel and
>> I'm not sure if I am looking the right one and therefore if I understood the question correctly.
>>
>> It is written as this to memorize what the correct id is and therefore what
>> to do.... but I doubt this answers you.
> No, it doesn't.
>
> With sysfs, .is_visible callbacks are normally used to hide attributes 
> when feature is not supported or for other reasons, which simplifies the
> setup code. This one checks manually asus_armoury.dgpu_disable_dev_id != 0 
> in the setup code.
>
> (In addition, sysfs could even be asked to change the visibility 
> on-the-fly so it would re-execute the .is_visible callbacks to determine 
> which of the attributes to show.)
>
>
This is a very good idea. I am not aware of any particular reason not to use
it as long as fwupd likes it I'm fine with it and I am also available to change
these to use in_visible.

      reply	other threads:[~2026-09-15 21:03 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-10 10:42 [PATCH] platform/x86: asus-armoury: add dGPU disable fallback DEVID for ProArt H7606 series busybox11
2026-08-10 10:42 ` busybox11 via B4 Relay
2026-09-15 20:38 ` Ilpo Järvinen
2026-09-15 20:48   ` Denis Benato
2026-09-15 20:56     ` Ilpo Järvinen
2026-09-15 21:03       ` Denis Benato [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=246a7bf2-a51b-442f-97aa-c767f8c02f8c@linux.dev \
    --to=denis.benato@linux.dev \
    --cc=busybox11th@gmail.com \
    --cc=corentin.chary@gmail.com \
    --cc=hansg@kernel.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luke@ljones.dev \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.