Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i5ec1447f:Fastmail
Date:   Mon, 22 Aug 2022 11:07:06 +1200
From:   Luke Jones <luke@ljones.dev>
Subject: Re: [PATCH] asus-wmi: Support the hardware GPU MUX on some laptops
To:     Mario Limonciello <mario.limonciello@amd.com>
Cc:     hdegoede@redhat.com, markgross@kernel.org,
        platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org
Message-Id: <UVMZGR.6LXKPONA9OS33@ljones.dev>
In-Reply-To: <e186622e-f3e6-5715-9f6d-bc41fb41560d@amd.com>
References: <20220813092624.6228-1-luke@ljones.dev>
        <e186622e-f3e6-5715-9f6d-bc41fb41560d@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Precedence: bulk

Hi Mario,

On Mon, Aug 15 2022 at 23:16:12 -0500, Mario Limonciello 
<mario.limonciello@amd.com> wrote:
> On 8/13/22 04:26, Luke D. Jones wrote:
>> Support the hardware GPU MUX switch available on some models. This
>> switch can toggle the MUX between:
>> 
>> - 0, Dedicated mode
>> - 1, Optimus mode
>> 
>> Optimus mode is the regular iGPU + dGPU available, while dedicated
>> mode switches the system to have only the dGPU available.
>> 
>> Signed-off-by: Luke D. Jones <luke@ljones.dev>
>> ---
>>   .../ABI/testing/sysfs-platform-asus-wmi       | 11 ++++
>>   drivers/platform/x86/asus-wmi.c               | 62 
>> +++++++++++++++++++
>>   include/linux/platform_data/x86/asus-wmi.h    |  3 +
>>   3 files changed, 76 insertions(+)
>> 
>> diff --git a/Documentation/ABI/testing/sysfs-platform-asus-wmi 
>> b/Documentation/ABI/testing/sysfs-platform-asus-wmi
>> index 574b5170a37d..03124eab7f01 100644
>> --- a/Documentation/ABI/testing/sysfs-platform-asus-wmi
>> +++ b/Documentation/ABI/testing/sysfs-platform-asus-wmi
>> @@ -58,6 +58,17 @@ Description:
>>   			* 1 - overboost,
>>   			* 2 - silent
>>   +What:          /sys/devices/platform/<platform>/gpu_mux_mode
>> +Date:          Aug 2022
>> +KernelVersion: 6.1
>> +Contact:       "Luke Jones" <luke@ljones.dev>
>> +Description:
>> +               Switch the GPU hardware MUX mode. Laptops with this 
>> feature can
>> +			   can be toggled to boot with only the dGPU (discrete mode) or 
>> in
>> +			   standard Optimus/Hybrid mode. On switch a reboot is required:
>> +                       * 0 - Discrete GPU,
>> +                       * 1 - Optimus/Hybrid,
> 
> This feel like it should probably export using 
> /sys/class/firmware-attributes.  That's exactly how those types of 
> attributes work.
> 
> As a bonus, software like fwupd 1.8.4 knows how to manipulate it and 
> you don't need special documentation.
> 
>> +
>>   What:          /sys/devices/platform/<platform>/dgpu_disable
>>   Date:          Aug 2022
>>   KernelVersion: 5.17
>> diff --git a/drivers/platform/x86/asus-wmi.c 
>> b/drivers/platform/x86/asus-wmi.c
>> index e2b51b5550e8..0421ffb81927 100644
>> --- a/drivers/platform/x86/asus-wmi.c
>> +++ b/drivers/platform/x86/asus-wmi.c
>> @@ -230,6 +230,7 @@ struct asus_wmi {
>>     	bool egpu_enable_available;
>>   	bool dgpu_disable_available;
>> +	bool gpu_mux_mode_available;
>>     	bool throttle_thermal_policy_available;
>>   	u8 throttle_thermal_policy_mode;
>> @@ -668,6 +669,59 @@ static ssize_t egpu_enable_store(struct device 
>> *dev,
>>   }
>>   static DEVICE_ATTR_RW(egpu_enable);
>>   +/* gpu mux switch 
>> *************************************************************/
>> +static int gpu_mux_mode_check_present(struct asus_wmi *asus)
>> +{
>> +	asus->gpu_mux_mode_available = asus_wmi_dev_is_present(asus, 
>> ASUS_WMI_DEVID_GPU_MUX);
>> +
>> +   return 0;
>> +}
>> +
>> +static ssize_t gpu_mux_mode_show(struct device *dev,
>> +                  struct device_attribute *attr, char *buf)
>> +{
>> +   struct asus_wmi *asus = dev_get_drvdata(dev);
>> +   int result;
>> +
>> +   result = asus_wmi_get_devstate_simple(asus, 
>> ASUS_WMI_DEVID_GPU_MUX);
>> +   if (result < 0)
>> +       return result;
>> +
>> +   return sysfs_emit(buf, "%d\n", result);
>> +}
>> +
>> +static ssize_t gpu_mux_mode_store(struct device *dev,
>> +                   struct device_attribute *attr,
>> +                   const char *buf, size_t count)
>> +{
>> +   struct asus_wmi *asus = dev_get_drvdata(dev);
>> +   int result, err;
>> +   u32 optimus;
>> +
>> +   err = kstrtou32(buf, 10, &optimus);
>> +   if (err)
>> +       return err;
>> +
>> +   if (optimus > 1)
>> +       return -EINVAL;
>> +
>> +   err = asus_wmi_set_devstate(ASUS_WMI_DEVID_GPU_MUX, optimus, 
>> &result);
>> +   if (err) {
>> +       dev_err(dev, "Failed to set GPU MUX mode: %d\n", err);
>> +       return err;
>> +   }
>> +	/* !1 is considered a fail by ASUS */
>> +	if (result != 1) {
>> +		dev_warn(dev, "Failed to set GPU MUX mode (result): 0x%x\n", 
>> result);
>> +       return -EIO;
>> +   }
>> +
>> +   sysfs_notify(&asus->platform_device->dev.kobj, NULL, 
>> "gpu_mux_mode");
>> +
>> +   return count;
>> +}
>> +static DEVICE_ATTR_RW(gpu_mux_mode);
>> +
>>   /* Battery 
>> ********************************************************************/
>>     /* The battery maximum charging percentage */
>> @@ -3165,6 +3219,7 @@ static struct attribute *platform_attributes[] 
>> = {
>>   	&dev_attr_touchpad.attr,
>>   	&dev_attr_egpu_enable.attr,
>>   	&dev_attr_dgpu_disable.attr,
>> +	&dev_attr_gpu_mux_mode.attr,
>>   	&dev_attr_lid_resume.attr,
>>   	&dev_attr_als_enable.attr,
>>   	&dev_attr_fan_boost_mode.attr,
>> @@ -3195,6 +3250,8 @@ static umode_t asus_sysfs_is_visible(struct 
>> kobject *kobj,
>>   		ok = asus->egpu_enable_available;
>>   	else if (attr == &dev_attr_dgpu_disable.attr)
>>   		ok = asus->dgpu_disable_available;
>> +	else if (attr == &dev_attr_gpu_mux_mode.attr)
>> +		ok = asus->gpu_mux_mode_available;
>>   	else if (attr == &dev_attr_fan_boost_mode.attr)
>>   		ok = asus->fan_boost_mode_available;
>>   	else if (attr == &dev_attr_throttle_thermal_policy.attr)
>> @@ -3464,6 +3521,10 @@ static int asus_wmi_add(struct 
>> platform_device *pdev)
>>   	if (err)
>>   		goto fail_dgpu_disable;
>>   +	err = gpu_mux_mode_check_present(asus);
>> +   if (err)
>> +       goto fail_gpu_mux_mode;
>> +
>>   	err = fan_boost_mode_check_present(asus);
>>   	if (err)
>>   		goto fail_fan_boost_mode;
>> @@ -3578,6 +3639,7 @@ static int asus_wmi_add(struct platform_device 
>> *pdev)
>>   fail_fan_boost_mode:
>>   fail_egpu_enable:
>>   fail_dgpu_disable:
>> +fail_gpu_mux_mode:
>>   fail_platform:
>>   fail_panel_od:
>>   	kfree(asus);
>> diff --git a/include/linux/platform_data/x86/asus-wmi.h 
>> b/include/linux/platform_data/x86/asus-wmi.h
>> index a571b47ff362..c023332842a2 100644
>> --- a/include/linux/platform_data/x86/asus-wmi.h
>> +++ b/include/linux/platform_data/x86/asus-wmi.h
>> @@ -98,6 +98,9 @@
>>   /* dgpu on/off */
>>   #define ASUS_WMI_DEVID_DGPU		0x00090020
>>   +/* gpu mux switch, 0 = dGPU, 1 = Optimus */
>> +#define ASUS_WMI_DEVID_GPU_MUX 0x00090016
>> +
>>   /* DSTS masks */
>>   #define ASUS_WMI_DSTS_STATUS_BIT	0x00000001
>>   #define ASUS_WMI_DSTS_UNKNOWN_BIT	0x00000002
> 

You can see previous discussion here 
https://lore.kernel.org/platform-driver-x86/c3bb0989-78d9-c513-1669-75407b2acbac@redhat.com/

Below is Hans response verbatim:

 > Yes it sounds like a BIOS setting is being toggled from within
 > Linux, which would normally be done through the
 > "firmware-attributes" class, but all existing "firmware-attributes"
 > class drivers allow changing all BIOS setting not just a single
 > setting, so using the  "firmware-attributes" class here is not really
 > appropriate.

Kind regards,
Luke.