worproject / Rockchip-Windows-Drivers

Windows on Arm drivers for RK35xx platforms.
123 stars 15 forks source link

Add I2S Audio Driver (WDM) #15

Closed coolstar closed 10 months ago

coolstar commented 10 months ago

Add I2S Audio driver. Reads ACPI properties from the bus driver to enumerate audio endpoints.

Supports:

  1. I2S Output (Orange Pi 5 jack)
  2. I2S Input (Orange Pi 5 jack)

Supported, but untested:

  1. HDMI Audio (Requires VOP driver for this to work -- though the endpoint is enumerated provided ACPI is setup for it)

Notes:

  1. I2S Input is buggy and only works correctly 1 in 3 times. 2 out of the 3 times, the microphone input just gets static. But once it's working, it stays working reliably for at least 12+ hours
coolstar commented 10 months ago

Will do a followup PR to switch all the drivers over to READ_REGISTER_ULONG / WRITE_REGISTER_ULONG

mariobalanica commented 10 months ago

Looks good to me otherwise. Great work!

idigdoug commented 10 months ago

Will do a followup PR to switch all the drivers over to READ_REGISTER_ULONG / WRITE_REGISTER_ULONG

If you're concerned about performance, you can consider READ_REGISTER_NOFENCE_ULONG.

op_REGISTER_ULONG:

op_REGISTER_ULONG_NOFENCE:

My understanding is that in most cases, op_REGISTER_ULONG_NOFENCE is sufficient. Though honestly, I'm a bit unclear about when you actually need the dsb, as it doesn't seem to do a lot of good in the cases I've considered (i.e. by itself it doesn't flush cache so it doesn't fix DMA).

coolstar commented 10 months ago
  • The driver binary is not part of the artifact. Does it build to the same location as the other drivers?

Might have been missing as the package was removed (fixed this), as the output location was correct

  • Source\Utilities\pl330dma.h could be moved to drivers\include since it may be used by other drivers. It's also missing include guards.

Done

  • Can you rename the device desc to "Rockchip I2S Controller" for consistency?

Done

  • Not a fan of the multiple vcxprojs TBH, the files could be all included in the base project without changing the directory structure (which you said you care about).

Could look into this for a follow-up PR

Looks good to me otherwise. Great work!