Re: Wrong piix4_smbus address / slow trackpoint on Thinkpad P14s gen 2 (AMD)

[Shyam Sundar S K <Shyam-sundar.S-k@amd.com>]

On 10/14/2024 22:13, Miroslav Bendík wrote:
> On 14. 10. 2024 18:01, Shyam Sundar S K wrote:
>>
>> On 10/13/2024 23:23, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 13-Oct-24 7:20 PM, Miroslav Bendík wrote:
>>>> On 10. 10. 2024 18:25, Mario Limonciello wrote:
>>>>> On 2/12/2022 11:42, Miroslav Bendík wrote:
>>>>>> Hello,
>>>>>> i think, that SMBus works now pretty good and last problem is
>>>>>> screaming interrupt from synaptics (1000 irq/s). I need little
>>>>>> help to solve this problem.
>>>>>>
>>>>>> Little summary first:
>>>>>>
>>>>>> On this thinkpad is synaptics trackpoint/touchpad connected to
>>>>>> PIIX4. To enable RMI4 mode, SMBus driver should support host
>>>>>> notify protocol. I have added support of host notify and
>>>>>> replaced active waiting transaction with completer + interrupt.
>>>>>> Driver is now pretty stable and works way better, than old
>>>>>> implementation. For example i2c-detect shows real devices
>>>>>> (previous transaction code showed all addresses from 0x1c as
>>>>>> active). Patch on following link is still hack, has hardcoded
>>>>>> IRQ and supports host notifications and interrupts only on
>>>>>> auxiliary port. I can implement other ports later.
>>>>>>
>>>>>> Patch: https://lore.kernel.org/all/c9b0b147-2907-
>>>>>> ff41-4f13-464b3b891c50@wisdomtech.sk/
>>>>>> This patch includes PM register access using MMIO: https://
>>>>>> lore.kernel.org/all/20210715221828.244536-1-Terry.Bowman@amd.com/
>>>>>>
>>>>>> Now i can load psmouse synaptics_intertouch=1 and everything
>>>>>> works great, but it uses 5% CPU and interrupt is called 1000/s.
>>>>>> I have changed interrupt from rising edge to active low (it's
>>>>>> PCIE, PCIE has active low) and i have many times checked if all
>>>>>> interrupt bits are cleared in interrupt request. Yes, they are
>>>>>> always cleared. Interrupts are generated only after first touch
>>>>>> if i have compiled only F12. If i compile F03, then interrupts
>>>>>> are generated immediately after load of psmouse. After unload,
>>>>>> interrupts are not generated (i2c-piix4 still loaded).
>>>>>>
>>>>>> On this machine I2C is accessible using GPIO 19(SCL), 20(SDA).
>>>>>> Using kernel thread with RT priority on isolated core i have
>>>>>> tried to record pin values on GPIO pins. Latency is too high to
>>>>>> record all transferred data. Some state changes are lost
>>>>>> (approximately 1/50 bits). Not too low to read reliably all
>>>>>> data, but good enough to see what happens at bus level. Here is
>>>>>> recorded file: https://mireq.linuxos.sk/kernel/
>>>>>> thinkpad_p14s/i2c_scl_sda.xz.
>>>>>>
>>>>>> Every byte is sample, first bit is SCL, second SDA. Sample rate
>>>>>> is cca 500 000 Hz, but often drops under 100 000 (lost bit).
>>>>>>
>>>>>> On this screenshot is typical activity on bus:
>>>>>> https://mireq.linuxos.sk/ kernel/thinkpad_p14s/i2c_1.png
>>>>>> (pulseview with imported raw file)
>>>>>>
>>>>>> Zoom to two packet is here: https://mireq.linuxos.sk/kernel/
>>>>>> thinkpad_p14s/i2c_2.png
>>>>>>
>>>>>> First packet is SMBus host notify. Address 0x08 is SMBus host
>>>>>> address and 0x58 is address of synaptics (0x2c << 1). Second
>>>>>> packet is reading of interrupt status registers. Data 02 is
>>>>>> length of interrupt status register (9 bits) and last 2 bytes
>>>>>> are zero (idle, when moving cursor, then interrupt status
>>>>>> register contains one bit set).
>>>>>>
>>>>>> Zoomed out: https://mireq.linuxos.sk/kernel/thinkpad_p14s/i2c_3.png
>>>>>>
>>>>>> Before transaction SMBus slave state machine is disabled and
>>>>>> after transaction enabled. If notification is received when
>>>>>> state machine is disabled, then device writes only address and
>>>>>> don't get response. If driver runs with always enabled slave
>>>>>> state machine, then output will contain only notify + read
>>>>>> interrupt status pairs and no separate addresses, but with this
>>>>>> mode bus collisions occur more often.
>>>>>>
>>>>>> Here is dmesg output: https://pastebin.com/RdDYHJn0
>>>>>>
>>>>>> Cursor is moved until 2862.8, then i have not touched trackpoint.
>>>>>>
>>>>>> Idle device don't produce bus collisions. Moving cursor produces
>>>>>> collisions, but sample rate is stable 100Hz, which is way
>>>>>> better, than <40 Hz with PS/2 mode. I don't know how to solve
>>>>>> collisions. Maybe they are related to not silenced host
>>>>>> notifications.
>>>>>>
>>>>>> If i were to be optimistic, then i would say that clearing
>>>>>> interrupt vector will solve all problems. According old RMI4
>>>>>> documentation, reading from interrupt status register should
>>>>>> clear interrupts (status register is cleared), but this don't
>>>>>> prevent device form sending host notifications. Maybe exists new
>>>>>> way to disable interrupts. I don't know, i have no access to
>>>>>> current documentation.
>>>>>>
>>>>>> My device has this signature:
>>>>>> Synaptics, product: TM3471-030, fw id: 3418235
>>>>>>
>>>>>> Any help welcome.
>>>>>>
>>>>> Sorry to bump such an old thread, but AFAIK you never came up
>>>>> with a good solution here.  I did want to point out that there
>>>>> was a very recent submission by Shyam (CC'ed) [1] that adds an
>>>>> ASF driver (which is an extension to PIIX4).  By default it's
>>>>> going to bind to an ACPI ID that isn't present on your system
>>>>> (present on newer systems only) but the hardware for ASF /should/
>>>>> be present even on yours.
>>>>>
>>>>> So I was going to suggest if you still are interested in this to
>>>>> play with that series and come up with a way to force using ASF
>>>>> (perhaps by a DMI match for your system) and see how that goes.
>>>>>
>>>>> [1]
>>>>> https://lore.kernel.org/all/20240923080401.2167310-1-Shyam-sundar.S-k@amd.com/
>>>> Hello.
>>>>
>>>> Thanks for the update. It looks good as a separate driver. I had
>>>> intended to split this driver, replace polling with interrupts,
>>>> and convert all I/O calls to MMIO, similar to how a Windows driver
>>>> operates. I paused the work because I needed documentation and
>>>> fixes from other companies, and I resolved my issue using a
>>>> different approach:
>>>>
>>>> - I have not received a response from Synaptics.
>>>> - I have not received a response from Lenovo.
>>>> - I have fixed the original issue -
>>>> https://patchwork.kernel.org/project/linux-input/patch/71d9dc66-9576-c26f-c9d9-129217f50255@gmail.com/#24848525
>>>> - Too many subsystems are affected, some of which are currently
>>>> hardly fixable.
>>>>
>>>> The biggest issue is interrupt support, which cannot be resolved
>>>> with quirks alone.
>> Do you mean the interrupt support from SMBus controller which happens
>> via the piix4 driver?
>>
>> Note that SMBus controller do not support interrupt and the same has
>> been documented in the datasheet:
>>
>> D14F0x03C [Interrupt Line] (FCH::SMBUSPCI::IntLine)
>>
>> 15:8 InterruptPin: Interrupt Pin. Read. Reset: Fixed,00h.
>>
>> ValidValues:
>>
>>
>> Value             Description
>> -------------        -------------
>> 00h             This module does not generate interrupts.
>> FFh-01h            Interrupt pin.
>>
>> Thanks,
>> Shyam
> 
> Hello,
> 
> i am using ASF documentation (48751 Rev 3.03 - February 19, 2015 BKDG
> for AMD Family 16h Models 00h-0Fh Processors -
> https://www.amd.com/en/search/documentation/hub.html#q=BKDG%2048751).

Hi,

You are referring the right document.
> 
> There is a ASFx0A ASFStatus with SlaveIntr bit. Pointing device on my
> machine triggers interrupt 7 and it can be cleared using ASF when i
> move cursor using touchpad or trackpoint. It's possible to implement
> and enable I2C_CLIENT_HOST_NOTIFY flag on this hardware.

if you want I2C_CLIENT_HOST_NOTIFY flag, then that has to come via a
software_node property and I don't think we have near future thoughts
about having this in BIOS.

> 
> Another interrupt bit Intr from ASFx00 HostStatus is 1 after transfer
> (interrupt 7 is triggered). Transfer can be implemented without timers
> and active waiting on HostBusy status (there is a loop in
> piix4_transaction).

Yeah, but this is what is being documented in the ASF databook and we
have to look at the SMBHSTSTS, based on that the further ASF packets
are processed.

Thanks,
Shyam

> 
> With regards
> Miroslav Bendík
> 
>>
>>>> My device has this DSDT ACPI entry:
>>>>
>>>>   Name (_HID, "SMB0001")  // _HID: Hardware ID
>>>>   Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
>>>>   {
>>>>       IO (Decode16,
>>>>           0x0B20,             // Range Minimum
>>>>           0x0B20,             // Range Maximum
>>>>           0x20,               // Alignment
>>>>           0x20,               // Length
>>>>           )
>>>>       IRQ (Level, ActiveLow, Shared, )
>>>>           {7}
>>>>   })
>>>>
>>>> This entry defines the IRQ number, trigger, and polarity. However,
>>>> the kernel ignores this entry and only uses the "Interrupt Source
>>>> Override" from the MADT table.
>>> Note that we already have a quirk table for this because this hits
>>> more
>>> interrupts in the legacy ISA interrupt range, see:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/acpi/resource.c#n659
>>>
>>> note that the MADT table is already alway skipped on AMD systems,
>>> but currently only for IRQs 1/12 which are the PS/2 kbd + mouse
>>> IRQs.
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>>