Re: [PATCH 16/17] [NOT-FOR-MERGE] media: atomsip: pci: add DMI match for Microsoft Surface 3 with broken DMI (OEMB)

Hans de Goede
2021-10-27T15:31:04Z

Hi,

On 10/27/21 16:47, Tsuchiya Yuto wrote:
> On Thu, 2021-10-21 at 20:46 +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 10/21/21 11:46, Tsuchiya Yuto wrote:
>>> On Mon, 2021-10-18 at 09:56 +0200, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 10/17/21 18:19, Tsuchiya Yuto wrote:
>>>>> This commit is added for Surface 3 with broken DMI table. HACK-ish.
>>>>> Not intended for upstreaming. Thus, NOT-FOR-MERGE. But, if someone
>>>>> knows a nicer way to address this, comments are welcome...
>>>>>
>>>>>> 8-----------------------------------------------------------------8<
>>>>>
>>>>> On some Microsoft Surface 3, the DMI table gets corrupted for unknown
>>>>> reasons and breaks existing DMI matching used for device-specific quirks.
>>>>>
>>>>> This commit adds the (broken) DMI data into dmi_system_id tables used
>>>>> for quirks so that the driver can enable quirks even on the affected
>>>>> systems.
>>>>>
>>>>> On affected systems, the DMI data will look like this:
>>>>>
>>>>> $ grep . /sys/devices/virtual/dmi/id/{bios_vendor,board_name,board_vendor,\
>>>>> chassis_vendor,product_name,sys_vendor}
>>>>> /sys/devices/virtual/dmi/id/bios_vendor:American Megatrends Inc.
>>>>> /sys/devices/virtual/dmi/id/board_name:OEMB
>>>>> /sys/devices/virtual/dmi/id/board_vendor:OEMB
>>>>> /sys/devices/virtual/dmi/id/chassis_vendor:OEMB
>>>>> /sys/devices/virtual/dmi/id/product_name:OEMB
>>>>> /sys/devices/virtual/dmi/id/sys_vendor:OEMB
>>>>
>>>> I wonder what the bios_date field contains ? Typically when the DMI strings
>>>> are no good (e.g. often they contain "Default String" or "To be filled by OEM")
>>>> we add a check on the bios-date, which together with the broken strings is
>>>> considered unique enough to still allow a match with broken strings in the
>>>> kernel.
>>>
>>> Thank you so much for the comment :-)
>>>
>>> Here is the full output of "/sys/devices/virtual/dmi/id/*" (not showing
>>> files that need root permission to read):
>>>
>>> $ grep . /sys/devices/virtual/dmi/id/*
>>> /sys/devices/virtual/dmi/id/bios_date:03/09/2015
>>> /sys/devices/virtual/dmi/id/bios_release:5.6
>>> /sys/devices/virtual/dmi/id/bios_vendor:American Megatrends Inc.
>>> /sys/devices/virtual/dmi/id/bios_version:1.51116.238
>>
>> Interesting, this is the latest BIOS from july 2019 according to:
>> https://support.microsoft.com/en-us/surface/surface-3-update-history-5d86a7bc-03f7-2d27-d858-e90ce637fb52
>> yet the date is still set to 03/09/2015.
>
> Yeah, I'm a little bit confused about this.

Yes this is unusual, bit it just seems to be how Microsoft does things
on the Surface 3.

>> I just checked and the BIOS with not corrupted DMI strings also keeps
>> the date at 03/09/2015 in BIOS updates.
>>
>> So the date is correct, and together with matching a coupleof the OEMB-s
>> (which I've never seen anywhere else either) this should be plenty
>> unique.
>>
>> So this not only allows adding this mathc to atomisp, but also to fix
>> sound + wmi on bad DMI data OEMB Surface 3-s, by updating this patch:
>>
>> https://github.com/linux-surface/linux-surface/blob/2fb7e9ae91350098db9a280277f424272816a9ab/patches/5.5/0003-surface3-oemb.patch
>>
>> To include the BIOS-date match and then submitting this upstream
>> (as 2 separate patches please).
>>
>> Tsuchiya, I take it that your Surface 3 has the OEMB issue, so you
>> can actually test this ?
>
> Yes, my surface3 is also affected and I can test this.
>
>> If you can prepare 2 patches for the sound + wmi then; and submit
>> them upstream that would be great. Please Cc me on both patches.
>
> Thank you for the suggestion, but I started having a mixed feeling about
> sending this kind of patches... This "OEMB" issue is not a design by
> manufacturers, but simply just it got broken after something (maybe a
> force power off?). On the other hand, I know there are also indeed some
> people affected by this issue other than me...
>
> If possible, I rather want to fix this broken DMI table, but I couldn't
> find the way until now though.
>
> But again, thank you for the suggestion. I'll consider sending the
> patches when I gave up fixing it...

Since other people are affected too, fixing this is only really useful
if the fix is doable by others too.

OTOH I just checked:

https://github.com/linuxhw/DMI

Which is a database of all dmidecode-s uploaded to:
https://linux-hardware.org/

And _zero_ of the 56358 dmidecode outputs available
there contains OEMB, so this seems to be quite unique to the
Surface 3. Which, esp. together with the BIOS date makes the DMI
match definitely unique enough. Sometimes DMI strings change
on a BIOS update (not break like this, but just change) so
having 2 entries for a single device-model is not unheard of.

So my cents is to just go with adding the extra entry to
dmi_system_id tables and to not spend too much time on this.

> <Below is completely off topic from atomisp>
>
> I think some useful BIOS option might be just hidden. So, I'd like to
> try this way. I already find the string "Restore Defaults" using
> uefitool/ifrextract:
>
> 0x13429 Form: Save & Exit, FormId: 0x2719 {01 86 19 27 4C 00}
> [...]
> 0x134E0 Suppress If {0A 82}
> 0x134E2 QuestionId: 0x1C3 equals value 0x5 {12 06 C3 01 05 00}
> 0x134E8 Ref: Restore Defaults, VarStoreInfo (VarOffset/VarName): 0xFFFF, VarStore: 0x0, QuestionId: 0x1BC, FormId: 0x2719 {0F 0F 5B 00 5C 00 BC 01 00 00 FF FF 00 19 27}
> 0x134F7 End If {29 02}
> [...]
>
> I currently don't know how I can call this. I want to try this way when
> I have some time...

So this is not a normal hidden item which just changes a setting (for which
there are known ways to change the setting without flashing the BIOS).

But this is some sorta action item which can only be enabled by flashing
a custom BIOS, assuming we can somehow replace the Surface 3 custom
setup menu with the standard IFR based menu, which in itself is a problem...

Regards,

Hans