Received-SPF: pass (google.com: domain of linux-kernel+bounces-107515-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Tue, 19 Mar 2024 14:26:26 +0200 (EET)
To: "Luke D. 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 v2 1/1] platform/x86: asus-wmi: add support for Vivobook
 GPU MUX
In-Reply-To: <20240310055312.11293-2-luke@ljones.dev>
Message-ID: <0f35beb8-5453-63cb-0570-752b2693f6ec@linux.intel.com>
References: <20240310055312.11293-1-luke@ljones.dev> <20240310055312.11293-2-luke@ljones.dev>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323328-1561235282-1710851186=:1267"

  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-1561235282-1710851186=:1267
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Sun, 10 Mar 2024, Luke D. Jones wrote:

> Adjust existing MUX support to select whichever MUX support is available
> so that ASUS Vivobook MUX can also be used if detected.

This description is a bit on the short side. It wouldn't have hurt to=20
first state that Vivobooks come with a GPU MUX WMI that has a different=20
WMI device ID. I can infer that after reading the diff but the description=
=20
should not require reading the patch.

The code change itself looks fine,

Reviewed-by: Ilpo J=E4rvinen <ilpo.jarvinen@linux.intel.com>

--=20
 i.

>=20
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
>  drivers/platform/x86/asus-wmi.c            | 18 +++++++++++++-----
>  include/linux/platform_data/x86/asus-wmi.h |  1 +
>  2 files changed, 14 insertions(+), 5 deletions(-)
>=20
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-=
wmi.c
> index 94cc589607b3..2cf695289655 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -268,6 +268,7 @@ struct asus_wmi {
>  =09bool egpu_connect_available;
>  =09bool dgpu_disable_available;
>  =09bool gpu_mux_mode_available;
> +=09u32 gpu_mux_dev;
> =20
>  =09/* Tunables provided by ASUS for gaming laptops */
>  =09bool ppt_pl2_sppt_available;
> @@ -682,7 +683,7 @@ static ssize_t dgpu_disable_store(struct device *dev,
>  =09=09return -EINVAL;
> =20
>  =09if (asus->gpu_mux_mode_available) {
> -=09=09result =3D asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_GPU_M=
UX);
> +=09=09result =3D asus_wmi_get_devstate_simple(asus, asus->gpu_mux_dev);
>  =09=09if (result < 0)
>  =09=09=09/* An error here may signal greater failure of GPU handling */
>  =09=09=09return result;
> @@ -748,7 +749,7 @@ static ssize_t egpu_enable_store(struct device *dev,
>  =09}
> =20
>  =09if (asus->gpu_mux_mode_available) {
> -=09=09result =3D asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_GPU_M=
UX);
> +=09=09result =3D asus_wmi_get_devstate_simple(asus, asus->gpu_mux_dev);
>  =09=09if (result < 0) {
>  =09=09=09/* An error here may signal greater failure of GPU handling */
>  =09=09=09pr_warn("Failed to get gpu mux status: %d\n", result);
> @@ -801,7 +802,7 @@ static ssize_t gpu_mux_mode_show(struct device *dev,
>  =09struct asus_wmi *asus =3D dev_get_drvdata(dev);
>  =09int result;
> =20
> -=09result =3D asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_GPU_MUX)=
;
> +=09result =3D asus_wmi_get_devstate_simple(asus, asus->gpu_mux_dev);
>  =09if (result < 0)
>  =09=09return result;
> =20
> @@ -847,7 +848,7 @@ static ssize_t gpu_mux_mode_store(struct device *dev,
>  =09=09}
>  =09}
> =20
> -=09err =3D asus_wmi_set_devstate(ASUS_WMI_DEVID_GPU_MUX, optimus, &resul=
t);
> +=09err =3D asus_wmi_set_devstate(asus->gpu_mux_dev, optimus, &result);
>  =09if (err) {
>  =09=09dev_err(dev, "Failed to set GPU MUX mode: %d\n", err);
>  =09=09return err;
> @@ -4507,7 +4508,6 @@ static int asus_wmi_add(struct platform_device *pde=
v)
>  =09asus->egpu_enable_available =3D asus_wmi_dev_is_present(asus, ASUS_WM=
I_DEVID_EGPU);
>  =09asus->egpu_connect_available =3D asus_wmi_dev_is_present(asus, ASUS_W=
MI_DEVID_EGPU_CONNECTED);
>  =09asus->dgpu_disable_available =3D asus_wmi_dev_is_present(asus, ASUS_W=
MI_DEVID_DGPU);
> -=09asus->gpu_mux_mode_available =3D asus_wmi_dev_is_present(asus, ASUS_W=
MI_DEVID_GPU_MUX);
>  =09asus->kbd_rgb_mode_available =3D asus_wmi_dev_is_present(asus, ASUS_W=
MI_DEVID_TUF_RGB_MODE);
>  =09asus->kbd_rgb_state_available =3D asus_wmi_dev_is_present(asus, ASUS_=
WMI_DEVID_TUF_RGB_STATE);
>  =09asus->ppt_pl2_sppt_available =3D asus_wmi_dev_is_present(asus, ASUS_W=
MI_DEVID_PPT_PL2_SPPT);
> @@ -4529,6 +4529,14 @@ static int asus_wmi_add(struct platform_device *pd=
ev)
>  =09=09asus->mini_led_dev_id =3D ASUS_WMI_DEVID_MINI_LED_MODE2;
>  =09}
> =20
> +=09if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_GPU_MUX)) {
> +=09=09asus->gpu_mux_mode_available =3D true;
> +=09=09asus->gpu_mux_dev =3D ASUS_WMI_DEVID_GPU_MUX;
> +=09} else if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_GPU_MUX_VIVO)=
) {
> +=09=09asus->gpu_mux_mode_available =3D true;
> +=09=09asus->gpu_mux_dev =3D ASUS_WMI_DEVID_GPU_MUX_VIVO;
> +=09}
> +
>  =09err =3D fan_boost_mode_check_present(asus);
>  =09if (err)
>  =09=09goto fail_fan_boost_mode;
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/p=
latform_data/x86/asus-wmi.h
> index 9cadce10ad9a..b48b024dd844 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -128,6 +128,7 @@
> =20
>  /* gpu mux switch, 0 =3D dGPU, 1 =3D Optimus */
>  #define ASUS_WMI_DEVID_GPU_MUX=09=090x00090016
> +#define ASUS_WMI_DEVID_GPU_MUX_VIVO=090x00090026
> =20
>  /* TUF laptop RGB modes/colours */
>  #define ASUS_WMI_DEVID_TUF_RGB_MODE=090x00100056
>=20
--8323328-1561235282-1710851186=:1267--