Hi,
On 6/15/22 23:31, Andy Shevchenko wrote:
> On Wed, Jun 15, 2022 at 9:57 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>>
>> Since ACPICA commit f005ee6b90d1 / Linux commit 2d3349de8072
>> ("ACPICA: Namespace: Reorder \_SB._INI to make sure it is evaluated
>> before _REG evaluations") acpi_initialize_objects() calls \_SB._INI
>> before executing _REG OpRegion methods, because the _REG methods may
>> rely on initialization done by this _INI method.
>>
>> In many DSDTs the \_SB.PC00._INI / \_SB.PCI0._INI methods set an OSYS
>> global variable based on _OSI evaluations.
>>
>> In some cases there are _REG methods which depend on the OSYS value and
>> before this change ACPICA would run these _REG methods before running
>> _SB.PC00._INI / \_SB.PCI0._INI causing issues.
>>
>> 2 examples of problems caused by running _REG methods before these
>> _INI methods are:
>>
>> 1. on a "Lenovo IdeaPad 5 15ITL05" \_SB.PC00.LPCB.EC0._REG gets
>> evaluated before \_SB.PC00._INI and that _REG contains:
>>
>> If ((OSYS == 0x07DF))
>> {
>> Local0 = 0x06
>> }
>>
>> If ((Acquire (LFCM, 0xA000) == Zero))
>> {
>> OSTY = Local0
>> ...
>>
>> With OSTY being a SystemMemory OpRegion field, due to the _INI running
>> too late, Local0 stays at 0. Causing OSTY to be set to 0 instead of 6,
>> which causes the brightness up/down keys to not work:
>> https://bugzilla.kernel.org/show_bug.cgi?id=214899
>>
>> 2. On a "Lenovo Thinkbook 14-ILL" \\_SB_.PCI0.I2C0._REG gets
>> evaluated before \_SB.PCI0._INI and that _REG contains:
>>
>> If ((OSYS == 0x07DF))
>> {
>> ...
>> LNUX = Zero
>> TPID = 0x4E
>> }
>> else
>> {
>> LNUX = One
>> TPID = 0xBB
>> }
>>
>> And then later on the TPID value gets used to decide for which of multiple
>> devices describing the touchpad _STA should return 0xF and the one which
>> gets enabled by TPID=0xBB is broken, causing to the touchpad to not work:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1842039
>>
>> Fix these issues by adding \_SB.PC00._INI / \_SB.PCI0._INI to the list of
>> _INI methods to run early (before executing _REG methods).
>
> ...
>
>> - char path[ACPI_PATH_SEGMENT_LENGTH + 2];
>> + char path[ACPI_PATH_SEGMENT_LENGTH * 2 + 2];
>
> Strictly speaking this should be, IIUC,
>
> 1 + ACPI_PATH_SEGMENT_LENGTH + ACPI_NAMESEG_SIZE + 1
>
> \\ + path segment length (with a separator) + name + \0
>
> That said, it seems the original code adds 1 unneeded byte.
Right I already knew I needed the " * 2" when writing the
original code, so I decided to do things this way for KISS
reasons.
> Perhaps a comment in the code?
I've added a comment to my local version now.
After this thread has had some more time to gather feedback
I will turn the ACPICA patches into a acpica github pull-req
with the comment added.
(or if necessary rework the entire series)
Regards,
Hans
