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;
Content-Type: multipart/mixed; boundary="------------wXV3NyKm5Y3zQIfoH2wA27S0"
Message-ID: <4b7da537-8ec3-95e2-4053-871c56e980ea@redhat.com>
Date:   Fri, 14 Jul 2023 19:00:13 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH] platform/x86/intel/hid: Add HP Dragonfly G2 to DMI quirks
Content-Language: en-US, nl
To:     Maxim Mikityanskiy <maxtram95@gmail.com>,
        Alex Hung <alexhung@gmail.com>,
        Mark Gross <markgross@kernel.org>
Cc:     platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org,
        Richard Hughes <hughsient@gmail.com>,
        Jorge Lopez <jorge.lopez2@hp.com>
References: <20230712175023.31651-1-maxtram95@gmail.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <20230712175023.31651-1-maxtram95@gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------wXV3NyKm5Y3zQIfoH2wA27S0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

Hi Maxim,

On 7/12/23 19:50, Maxim Mikityanskiy wrote:
> SW_TABLET_MODE reports are broken on BIOS versions newer than 1.9.1 on
> HP Elite Dragonfly G2. Analysis of SSDT9 shows that the BTNL method has
> to be called to start getting 0xcc and 0xcd events. Apparently, the
> button_array_present method used to return true on BIOS 1.9.1 and older,
> but it returns false on newer BIOSes due to HEBC returning 0x000033f3
> (bits 0x60000 and 0x20000 are not set).
> 
> Add this laptop to button_array_table to force the BTNL call, and also
> add it to dmi_vgbs_allow_list to read the initial state and sync VBDS
> with VBPS, because this laptop has a reliable VGBS method.
> 
> Tested with BIOS 1.13.1.
> 
> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>

Thank you for figuring this out and thank you for the patch.

WRT the need to call BTNL, I expect that if one laptop needs this likely more models will need it and I expect / hope that this will not cause any issues when called everywhere.

So I think it would be better to instead of adding this model to the button_array_table[] it would be better to just always call BTNL (I suspect this is also what Windows does).

Can you give the attached path to always call BTNL a try ?

As for adding the model to dmi_vgbs_allow_list[] on many models it seems that the EC will send a 0xcc / 0xcd event at boot (now that I think about it I guess this may be in response to the BTNL call) and then on that event the SW_TABLET_STATE evdev gets registered and the first event also syncs the state.

Can you check if just using the always call BTNL patch, without the quirk perhaps already makes the SW_TABLET_STATE evdev show up (including the correct state) ?

If no event is send during boot, the n I'm fine with adding the model to the dmi_vgbs_allow_list[], but perhaps this is not necessary?

Regards,

Hans


> ---
>  drivers/platform/x86/intel/hid.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/hid.c b/drivers/platform/x86/intel/hid.c
> index 5632bd3c534a..5c78b476ed1e 100644
> --- a/drivers/platform/x86/intel/hid.c
> +++ b/drivers/platform/x86/intel/hid.c
> @@ -128,6 +128,13 @@ static const struct dmi_system_id button_array_table[] = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "Surface Go 3"),
>  		},
>  	},
> +	{
> +		.ident = "HP Elite Dragonfly G2",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "HP Elite Dragonfly G2 Notebook PC"),
> +		},
> +	},
>  	{ }
>  };
>  
> @@ -150,6 +157,12 @@ static const struct dmi_system_id dmi_vgbs_allow_list[] = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "Surface Go"),
>  		},
>  	},
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "HP Elite Dragonfly G2 Notebook PC"),
> +		},
> +	},
>  	{ }
>  };
>  

--------------wXV3NyKm5Y3zQIfoH2wA27S0
Content-Type: text/x-patch; charset=UTF-8;
 name="0001-platform-x86-intel-hid-Always-call-BTNL-ACPI-method.patch"
Content-Disposition: attachment;
 filename*0="0001-platform-x86-intel-hid-Always-call-BTNL-ACPI-method.pat";
 filename*1="ch"
Content-Transfer-Encoding: base64

RnJvbSAxZmI0NDNjYjFjYWY3M2U5NDBhNmIwNTUwYjdlNzljODZlZGYzYWVmIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBIYW5zIGRlIEdvZWRlIDxoZGVnb2VkZUByZWRoYXQu
Y29tPgpEYXRlOiBGcmksIDE0IEp1bCAyMDIzIDE3OjUxOjM4ICswMjAwClN1YmplY3Q6IFtQ
QVRDSF0gcGxhdGZvcm0veDg2OiBpbnRlbDogaGlkOiBBbHdheXMgY2FsbCBCVE5MIEFDUEkg
bWV0aG9kCgpPbiBhIEhQIEVsaXRlIERyYWdvbmZseSBHMiB0aGUgMHhjYyBhbmQgMHhjZCBl
dmVudHMgZm9yIFNXX1RBQkxFVF9NT0RFCmFyZSBvbmx5IHNlbmQgYWZ0ZXIgdGhlIEJUTkwg
QUNQSSBtZXRob2QgaGFzIGJlZW4gY2FsbGVkLgoKTGlrZWx5IG1vcmUgZGV2aWNlcyBuZWVk
IHRoaXMsIHNvIG1ha2UgdGhlIEJUTkwgQUNQSSBtZXRob2QgdW5jb25kaXRpb25hbAppbnN0
ZWFkIG9mIG9ubHkgZG9pbmcgaXQgb24gZGV2aWNlcyB3aXRoIGEgNSBidXR0b24gYXJyYXku
CgpOb3RlIHRoaXMgYWxzbyBtYWtlcyB0aGUgaW50ZWxfYnV0dG9uX2FycmF5X2VuYWJsZSgp
IGNhbGwgaW4gcHJvYmUoKQp1bmNvbmRpdGlvbmFsLCB0aGF0IGZ1bmN0aW9uIGRvZXMgaXRz
IG93biBwcml2LT5hcnJheSBjaGVjay4gVGhpcyBtYWtlcwp0aGUgaW50ZWxfYnV0dG9uX2Fy
cmF5X2VuYWJsZSgpIGNhbGwgaW4gcHJvYmUoKSBjb25zaXN0ZW50IHdpdGggdGhlIGNhbGxz
CmRvbmUgb24gc3VzcGVuZC9yZXN1bWUgd2hpY2ggYWxzbyByZWx5IG9uIHRoZSBwcml2LT5h
cnJheSBjaGVjayBpbnNpZGUKdGhlIGZ1bmN0aW9uLgoKUmVwb3J0ZWQtYnk6IE1heGltIE1p
a2l0eWFuc2tpeSA8bWF4dHJhbTk1QGdtYWlsLmNvbT4KQ2xvc2VzOiBodHRwczovL2xvcmUu
a2VybmVsLm9yZy9wbGF0Zm9ybS1kcml2ZXIteDg2LzIwMjMwNzEyMTc1MDIzLjMxNjUxLTEt
bWF4dHJhbTk1QGdtYWlsLmNvbS8KU2lnbmVkLW9mZi1ieTogSGFucyBkZSBHb2VkZSA8aGRl
Z29lZGVAcmVkaGF0LmNvbT4KLS0tCiBkcml2ZXJzL3BsYXRmb3JtL3g4Ni9pbnRlbC9oaWQu
YyB8IDIxICsrKysrKysrKy0tLS0tLS0tLS0tLQogMSBmaWxlIGNoYW5nZWQsIDkgaW5zZXJ0
aW9ucygrKSwgMTIgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvZHJpdmVycy9wbGF0Zm9y
bS94ODYvaW50ZWwvaGlkLmMgYi9kcml2ZXJzL3BsYXRmb3JtL3g4Ni9pbnRlbC9oaWQuYwpp
bmRleCA1NjMyYmQzYzUzNGEuLjY0MWYyNzk3NDA2ZSAxMDA2NDQKLS0tIGEvZHJpdmVycy9w
bGF0Zm9ybS94ODYvaW50ZWwvaGlkLmMKKysrIGIvZHJpdmVycy9wbGF0Zm9ybS94ODYvaW50
ZWwvaGlkLmMKQEAgLTYyMCw3ICs2MjAsNyBAQCBzdGF0aWMgYm9vbCBidXR0b25fYXJyYXlf
cHJlc2VudChzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpkZXZpY2UpCiBzdGF0aWMgaW50IGlu
dGVsX2hpZF9wcm9iZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpkZXZpY2UpCiB7CiAJYWNw
aV9oYW5kbGUgaGFuZGxlID0gQUNQSV9IQU5ETEUoJmRldmljZS0+ZGV2KTsKLQl1bnNpZ25l
ZCBsb25nIGxvbmcgbW9kZTsKKwl1bnNpZ25lZCBsb25nIGxvbmcgbW9kZSwgZHVtbXk7CiAJ
c3RydWN0IGludGVsX2hpZF9wcml2ICpwcml2OwogCWFjcGlfc3RhdHVzIHN0YXR1czsKIAlp
bnQgZXJyOwpAQCAtNjkyLDE4ICs2OTIsMTUgQEAgc3RhdGljIGludCBpbnRlbF9oaWRfcHJv
YmUoc3RydWN0IHBsYXRmb3JtX2RldmljZSAqZGV2aWNlKQogCWlmIChlcnIpCiAJCWdvdG8g
ZXJyX3JlbW92ZV9ub3RpZnk7CiAKLQlpZiAocHJpdi0+YXJyYXkpIHsKLQkJdW5zaWduZWQg
bG9uZyBsb25nIGR1bW15OworCWludGVsX2J1dHRvbl9hcnJheV9lbmFibGUoJmRldmljZS0+
ZGV2LCB0cnVlKTsKIAotCQlpbnRlbF9idXR0b25fYXJyYXlfZW5hYmxlKCZkZXZpY2UtPmRl
diwgdHJ1ZSk7Ci0KLQkJLyogQ2FsbCBidXR0b24gbG9hZCBtZXRob2QgdG8gZW5hYmxlIEhJ
RCBwb3dlciBidXR0b24gKi8KLQkJaWYgKCFpbnRlbF9oaWRfZXZhbHVhdGVfbWV0aG9kKGhh
bmRsZSwgSU5URUxfSElEX0RTTV9CVE5MX0ZOLAotCQkJCQkgICAgICAgJmR1bW15KSkgewot
CQkJZGV2X3dhcm4oJmRldmljZS0+ZGV2LAotCQkJCSAiZmFpbGVkIHRvIGVuYWJsZSBISUQg
cG93ZXIgYnV0dG9uXG4iKTsKLQkJfQotCX0KKwkvKgorCSAqIENhbGwgYnV0dG9uIGxvYWQg
bWV0aG9kIHRvIGVuYWJsZSBISUQgcG93ZXIgYnV0dG9uCisJICogQWx3YXlzIGRvIHRoaXMg
c2luY2UgaXQgYWN0aXZhdGVzIGV2ZW50cyBvbiBzb21lIGRldmljZXMgd2l0aG91dAorCSAq
IGEgYnV0dG9uIGFycmF5IHRvby4KKwkgKi8KKwlpZiAoIWludGVsX2hpZF9ldmFsdWF0ZV9t
ZXRob2QoaGFuZGxlLCBJTlRFTF9ISURfRFNNX0JUTkxfRk4sICZkdW1teSkpCisJCWRldl93
YXJuKCZkZXZpY2UtPmRldiwgImZhaWxlZCB0byBlbmFibGUgSElEIHBvd2VyIGJ1dHRvblxu
Iik7CiAKIAlkZXZpY2VfaW5pdF93YWtldXAoJmRldmljZS0+ZGV2LCB0cnVlKTsKIAkvKgot
LSAKMi40MS4wCgo=

--------------wXV3NyKm5Y3zQIfoH2wA27S0--