mirror of https://lore.kernel.org/lkml/
 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: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®