Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
qzed opened this issue Jun 21, 2022 · 3 comments
Labels
A: SoC Area: Qualcomm SoC drivers

Comments

@qzed
Copy link
Member

qzed commented Jun 21, 2022

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.

@qzed qzed added the A: SoC Area: Qualcomm SoC drivers label Jun 21, 2022
This was referenced Jun 21, 2022
@gus33000
Copy link

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
Copy link
Member Author

qzed commented Jun 22, 2022

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
Copy link
Member Author

qzed commented Jun 22, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: SoC Area: Qualcomm SoC drivers
Projects
Status: Todo
Development

No branches or pull requests

2 participants