Received-SPF: pass (google.com: domain of linux-kernel+bounces-192008-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Tue, 28 May 2024 12:18:35 +0300 (EEST)
To: "Luke D. Jones" <luke@ljones.dev>
cc: Hans de Goede <hdegoede@redhat.com>, corentin.chary@gmail.com, 
    platform-driver-x86@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 6/9] platform/x86: asus-wmi: add panel-fhd
 functionality
In-Reply-To: <20240528013626.14066-7-luke@ljones.dev>
Message-ID: <53da23e6-deb5-dc4f-057f-afafaa9455aa@linux.intel.com>
References: <20240528013626.14066-1-luke@ljones.dev> <20240528013626.14066-7-luke@ljones.dev>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII

On Tue, 28 May 2024, Luke D. Jones wrote:

> Exposes the FHD panel toggle avavilable on new ASUS Duo laptops.

available

> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> ---
>  .../ABI/testing/sysfs-platform-asus-wmi       |  9 +++++++++
>  drivers/platform/x86/asus-wmi.c               | 20 +++++++++++++++++++
>  include/linux/platform_data/x86/asus-wmi.h    |  1 +
>  3 files changed, 30 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-asus-wmi b/Documentation/ABI/testing/sysfs-platform-asus-wmi
> index 984a04f32fd0..3b4eeea75b7b 100644
> --- a/Documentation/ABI/testing/sysfs-platform-asus-wmi
> +++ b/Documentation/ABI/testing/sysfs-platform-asus-wmi
> @@ -217,3 +217,12 @@ Description:
>  		Set if the MCU can go in to low-power mode on system sleep
>  			* 0 - False,
>  			* 1 - True
> +
> +What:		/sys/devices/platform/<platform>/panel_fhd
> +Date:		Apr 2024
> +KernelVersion:	6.11
> +Contact:	"Luke Jones" <luke@ljones.dev>
> +Description:
> +		Set panel to UHD or FHD mode
> +			* 0 - UHD,
> +			* 1 - FHD

I'd prefer this to be more forward-looking, the filename is based on fhd, 
which is one of the options and the values seem "wrong way" around too 
(FHD < UHD but here you have FHD=1 after UHD=0).

> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 260548aa6a42..4b045f1828f1 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -798,6 +798,23 @@ WMI_ATTR_SIMPLE_RW(panel_od, 0, 1, ASUS_WMI_DEVID_PANEL_OD);
>  WMI_ATTR_SIMPLE_RW(boot_sound, 0, 1, ASUS_WMI_DEVID_BOOT_SOUND);
>  WMI_ATTR_SIMPLE_RO(charge_mode, ASUS_WMI_DEVID_CHARGE_MODE);
>  
> +static ssize_t panel_fhd_store(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	struct asus_wmi *asus = dev_get_drvdata(dev);
> +	int err;
> +
> +	err = rog_tunable_store(asus, &attr->attr, buf, count,
> +				0, 1, -1, NULL, ASUS_WMI_DEVID_PANEL_FHD);
> +	if (err < 0)
> +		return err;
> +
> +	pr_info("Panel UHD/FHD display mode changed, reboot required\n");
> +	return count;
> +}
> +WMI_SIMPLE_SHOW(panel_fhd, "%d\n", ASUS_WMI_DEVID_PANEL_FHD);
> +static DEVICE_ATTR_RW(panel_fhd);
> +
>  /* Tablet mode ****************************************************************/
>  
>  static void asus_wmi_tablet_mode_get_state(struct asus_wmi *asus)
> @@ -4040,6 +4057,7 @@ static struct attribute *platform_attributes[] = {
>  	&dev_attr_mcu_powersave.attr,
>  	&dev_attr_boot_sound.attr,
>  	&dev_attr_panel_od.attr,
> +	&dev_attr_panel_fhd.attr,
>  	&dev_attr_mini_led_mode.attr,
>  	&dev_attr_available_mini_led_mode.attr,
>  	NULL
> @@ -4111,6 +4129,8 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
>  		devid = ASUS_WMI_DEVID_BOOT_SOUND;
>  	else if (attr == &dev_attr_panel_od.attr)
>  		devid = ASUS_WMI_DEVID_PANEL_OD;
> +	else if (attr == &dev_attr_panel_fhd.attr)
> +		devid = ASUS_WMI_DEVID_PANEL_FHD;
>  	else if (attr == &dev_attr_mini_led_mode.attr)
>  		ok = asus->mini_led_dev_id != 0;
>  	else if (attr == &dev_attr_available_mini_led_mode.attr)
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 79a50102440d..6c51d41ffc20 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -72,6 +72,7 @@
>  #define ASUS_WMI_DEVID_LID_FLIP_ROG	0x00060077
>  #define ASUS_WMI_DEVID_MINI_LED_MODE	0x0005001E
>  #define ASUS_WMI_DEVID_MINI_LED_MODE2	0x0005002E
> +#define ASUS_WMI_DEVID_PANEL_FHD	0x0005001C
>  
>  /* Storage */
>  #define ASUS_WMI_DEVID_CARDREADER	0x00080013
> 

-- 
 i.