linux-surface / surface-pro-x

Tracking and meta repository for Surface Pro X support.
76 stars 6 forks source link

USB: Use Init-Values from ACPI and Windows Driver instead of the SM8150 ones? #33

Open qzed opened 2 years ago

qzed commented 2 years ago

Both ACPI and the driver provide some init value tables, similar to what we currently use in Linux based on the SM8150. It stands to reason that those are probably better suited for the SC8180X. So we should maybe add some SC8180X-specific tables to the Linux driver.

gus33000 commented 2 years ago

The values specified in Surface Pro X DSDT ACPI table are OEM customizations, just like Microsoft has done for the Surface Duo itself.

In fact, you might notice some strong similarities between the two device registers. Here's ACPI vs DT for Surface Duo: (https://github.com/microsoft/surface-duo-oss-kernel.msm-4..14/blob/surfaceduo/11/2021.1027.156/arch/arm64/boot/dts/surface/surface-duo-usb.dtsi#L27-L177 vs https://raw.githubusercontent.com/WOA-Project/SurfaceDuoPkg/main/Platforms/SurfaceDuoPkg/AcpiTables/8150/src/DSDT.dsl (see Method (PHYC, 0, NotSerialized))

I wrote a parser to know exactly what those values really meant, not that hard to write with the header already existing in the kernel. Values being adjusted are almost the same as on surface duo with some differences.

I believe these are changed for Power Delivery purposes, as the PD phy did not start working correctly until I've set these in.

Normally the ACPI methods for specifying custom register addresses/values are left empty. It isn't really a SC8180X value set.

Unlike other SC8180X devices Microsoft does not use the new USB-C PMIC code/BatteryManager code in ADSP, it is not in the Pro X ADSP.mbn image and instead they use SAM for handling Power Delivery. There's a filter driver in Windows sitting on top of qualcomm's UCSI driver that directly sends messages using the ACPI methods from SAM. I don't know much the specifics of the SAM communication itself given my surface device does not use SAM, but thought this could be interesting for you as well.

qzed commented 2 years ago

Thanks! Right, makes sense that they have SoC specific data in the driver and the device/vendor specific one in ACPI. I noticed that Qualcomm did similar things in other DTs.

Also, yes, PD on the SPX is handled via SAM. Most newer Surface devices handle anything battery related via SAM, but I'm not sure of the extent to which we may need to control SAM or to which extent it "just works" automatically. As far as I know, there are redriver ICs and a PD controller controlled by SAM in-between the USB-C ports and the SoC, so I had assumed that everything would work without us having to control much (but I haven't really tested that yet).

Thanks for pointing out the filter driver, that means that there very well might be more communication going on than I initially assumed. I'll need to take a look at that.

qzed commented 2 years ago

There is indeed some SAM stuff going on in the SurfaceUsbCMuxAcpiFilterDriver.sys using the USC subsystem. There seem to be two methods, specifically. A FilterGetPortNumbers and a FilterSendUSBCAck. In addition to that, there's a PdEventInfoUpdateFromSamToSoc function, which might be some part of a PD event handler by its name.

In fact all three functions are called in the same ProcessUsbCResponse function, first getting the port number(s?), then updating the PD event info, then sending the ACK back. I think what's happening is that SAM might send a PD event when a USB-C source is plugged in. This is then handled by that process-response function.

I think we might need to have to log SAM events on Windows to get a better idea on what's going on with that.