On Thu, 8 Aug 2024, Stefan Sichler wrote:
> this patch (which is now committed to the kernel as commit
> 520ee4ea1cc60251a6e3c911cf0336278aa52634 since v5.18-rc1) unfortunately
> introduced a regression on my HP EliteBook 2760p Convertible:
>
> Tablet mode is no longer detected.
>
> It worked flawlessly before (when enable_tablet_mode_sw module param was
> set to 1).
>
> Debugging showed that on this device, two problems prevent the table
> mode detection from working:
>
> - Chassis Type is reported as 0x10 (= Lunch Box)
>
> - the query of HPWMI_SYSTEM_DEVICE_MODE does not report tablet state
> at all
>
> Note that the chassis type of this device (switch to tablet mode by
> screen *rotation*) actually differs from the newer HP models (switch to
> tablet mode by screen *flipping*).
>
>
> I suggest fixing this by re-adding the removed module parameter
> "enable_tablet_mode_sw", but change its behavior to work in the
> following way:
>
> - when left at default -1 (auto): no change to current (new)
> implementation
>
> - when set to 0: unconditionally disable table mode reporting at all
>
> - when set to 1: ignore Chassis type and use old-skool
> hp_wmi_hw_state(HPWMI_TABLET_MASK) query method to determine tablet mode
> in addition to new hp_wmi_perform_query(HPWMI_SYSTEM_DEVICE_MODE...) method
>
>
> I prepared a patch based on commit
> 520ee4ea1cc60251a6e3c911cf0336278aa52634, and tested it successfully on
> my device.
> See below.
>
> Regards,
> Stefan
>
> --- hp-wmi.c.orig 2024-03-10 21:38:09.000000000 +0100
> +++ hp-wmi.c 2024-08-08 09:23:29.509113900 +0200
This submission does not follow the normal patch formatting guidelines,
please see Documentation/process/submitting-patches.rst.
> @@ -35,6 +35,10 @@
> MODULE_ALIAS("wmi:95F24279-4D7B-4334-9387-ACCDC67EF61C");
> MODULE_ALIAS("wmi:5FB7F034-2C63-45e9-BE91-3D44E2C707E4");
>
> +static int enable_tablet_mode_sw = -1;
> +module_param(enable_tablet_mode_sw, int, 0444);
> +MODULE_PARM_DESC(enable_tablet_mode_sw, "Enable SW_TABLET_MODE
> reporting (-1=auto, 0=no, 1=yes)");
> +
> #define HPWMI_EVENT_GUID "95F24279-4D7B-4334-9387-ACCDC67EF61C"
> #define HPWMI_BIOS_GUID "5FB7F034-2C63-45e9-BE91-3D44E2C707E4"
> #define HP_OMEN_EC_THERMAL_PROFILE_OFFSET 0x95
> @@ -428,6 +432,9 @@
> bool tablet_found;
> int ret;
>
> + if (!enable_tablet_mode_sw)
> + return -ENODEV;
> +
> chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
> if (!chassis_type)
> return -ENODEV;
> @@ -435,16 +442,24 @@
> tablet_found = match_string(tablet_chassis_types,
> ARRAY_SIZE(tablet_chassis_types),
> chassis_type) >= 0;
> - if (!tablet_found)
> + if (!tablet_found && enable_tablet_mode_sw < 0 /*auto*/)
Having to add a comment like that is a very strong indication you'd want
to have a named define instead, e.g., HPWMI_TABLET_MODE_AUTO.
> return -ENODEV;
>
> ret = hp_wmi_perform_query(HPWMI_SYSTEM_DEVICE_MODE, HPWMI_READ,
> system_device_mode,
> zero_if_sup(system_device_mode),
> sizeof(system_device_mode));
> - if (ret < 0)
> - return ret;
> + if (ret >= 0)
> + ret = (system_device_mode[0] == DEVICE_MODE_TABLET);
> +
> + /* workaround for older convertibles */
> + if (ret <= 0 && enable_tablet_mode_sw > 0)
> + {
> + ret = hp_wmi_read_int(HPWMI_HARDWARE_QUERY);
> + if (!(ret < 0))
> + ret = !!(ret & HPWMI_TABLET_MASK);
> + }
>
> - return system_device_mode[0] == DEVICE_MODE_TABLET;
> + return ret;
The logic is quite hard to follow. It would be better to return
early.
if (ret < 0 && enable_tablet_mode_sw == HPWMI_TABLET_MODE_AUTO)
return ret;
if (ret >= 0 && system_device_mode[0] == DEVICE_MODE_TABLET)
return 1;
ret = hp_wmi_read_int(HPWMI_HARDWARE_QUERY);
if (ret < 0)
return ret;
return !!(ret & HPWMI_TABLET_MASK);
However, automatically detecting this condition over adding the module
parameter would be the preferred solution.
--
i.
> }
>
> static int omen_thermal_profile_set(int mode)
>
>
