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

rust: samples: fix init argument #335

Open
wants to merge 741 commits into
base: asahi
Choose a base branch
from

Conversation

Bruno-Eduardo
Copy link

I was trying to compile the rust_minimal for arm64 using asahi rust/kernel and found the following error:

$ make CROSS_COMPILE="aarch64-linux-gnu-" ARCH=arm64 LLVM=1 ./samples/rust/rust_minimal.o
  CALL    scripts/checksyscalls.sh
  RUSTC [M] samples/rust/rust_minimal.o
error[E0050]: method `init` has 1 parameter but the declaration in trait `kernel::Module::init` has 2
  --> /largeFiles/linux_asahi/samples/rust/rust_minimal.rs:20:22
   |
20 |     fn init(_module: &'static ThisModule) -> Result<Self> {
   |                      ^^^^^^^^^^^^^^^^^^^ expected 2 parameters, found 1
   |
   = note: `init` from trait: `fn(&'static kernel::prelude::CStr, &'static kernel::ThisModule) -> core::result::Result<Self, kernel::error::Error>`

error: aborting due to 1 previous error

And an equivalent error for ./samples/rust/rust_print.o

After commit c96b547:
rust: Addname argument to Module::init()
init needs a name argument. Since both rust samples were made
before that, they fail to compile.

After fix:

$ make CROSS_COMPILE="aarch64-linux-gnu-" ARCH=arm64 LLVM=1 ./samples/rust/rust_minimal.o
  CALL    scripts/checksyscalls.sh
  RUSTC [M] samples/rust/rust_minimal.o
$ make CROSS_COMPILE="aarch64-linux-gnu-" ARCH=arm64 LLVM=1 ./samples/rust/rust_print.o
  CALL    scripts/checksyscalls.sh
  RUSTC [M] samples/rust/rust_print.o

Fixes warning:

[    8.502802] ------------[ cut here ]------------
[    8.503445] cpu_latency_qos_remove_request called for unknown object
[    8.504269] WARNING: CPU: 5 PID: 2790 at kernel/power/qos.c:322 cpu_latency_qos_remove_request+0x48/0x98
[    8.505499] CPU: 5 PID: 2790 Comm: wireplumber Tainted: G        W          6.5.0-asahi-00708-gb9b88240f7ae #2291
[    8.506777] Hardware name: Apple MacBook Air (13-inch, M2, 2022) (DT)
<snip regs>
[    8.519099] Call trace:
[    8.519402]  cpu_latency_qos_remove_request+0x48/0x98
[    8.520027]  snd_pcm_ioctl+0x86c/0x182c
[    8.520519]  __arm64_sys_ioctl+0xf8/0xbd0
[    8.521020]  invoke_syscall.constprop.0+0x78/0xc8
[    8.521604]  do_el0_svc+0x58/0x154
[    8.522026]  el0_svc+0x34/0xe4
[    8.522409]  el0t_64_sync_handler+0x120/0x12c
[    8.522951]  el0t_64_sync+0x190/0x194
[    8.523408] ---[ end trace 0000000000000000 ]---

Signed-off-by: Hector Martin <[email protected]>
alsamixer/etc really don't like write-only controls...

Signed-off-by: Hector Martin <[email protected]>
On the speakers PCM, this sequence:

1. Open playback
2. Open sense
3. Close playback
4. Close sense

would result in the sense FE being marked as clocks in use at (2), since
there is a clock provider (playback FE). Then at (4) this would WARN since
there is no driver any more when closing the in use clocks.

If (1) and (2) are reversed this does not happen, since the sense PCM is
not marked as using the clocks when there is no provider yet. So, check
explicitly whether the substream FE is a clock provider in be_prepare,
and skip everything if it isn't.

Signed-off-by: Hector Martin <[email protected]>
For machines where we do not consider things safe yet, require the
commandline argument. Without it, speakers are simply disabled, we don't
refuse probe entirely.

Signed-off-by: Hector Martin <[email protected]>
These are unintentionally aliased. Pending a solution for this, let's
just use the same limit for now.

Signed-off-by: Hector Martin <[email protected]>
Signed-off-by: Hector Martin <[email protected]>
The ASoC convention is that clocks are removed after codec mute, and
power up/down is more about top level power management. For these chips,
the "mute" state still expects a TDM clock, and yanking the clock in
this state will trigger clock errors. So, do the full
shutdown<->mute<->active transition on the mute operation, so the amp is
in software shutdown by the time the clocks are removed.

This fixes TDM clock errors when streams are stopped.

Signed-off-by: Hector Martin <[email protected]>
Multiple amps can be connected to the same SDZ GPIO. Using raw GPIOs for
this breaks, as there is no concept of refcounting/sharing. In order to
model these platforms, introduce support for an SDZ "regulator". This
allows us to represent the SDZ GPIO as a simple regulator-fixed, and
then the regulator core takes care of refcounting so that all codecs are
only powered down once all the driver instances are in the suspend
state.

Signed-off-by: Hector Martin <[email protected]>
Otherwise we can end up recursively locking the controls lock in the
start/stop path, since it can be called from a control change.

Signed-off-by: Hector Martin <[email protected]>
Since the bit is self-clearing.

Signed-off-by: Hector Martin <[email protected]>
Saw this fail once, let's be safer.

Signed-off-by: Hector Martin <[email protected]>
When a PCM is suspended, we pause the DMA. If the PCM is then closed
while in this state, it does not receive the STOP trigger (as it is not
running). In this case, we fail to properly terminate the DMA, calling
dmaengine_synchronize() nonetheless, which is undefined behavior.

Make sure we always call dmaengine_terminate_async() on PCM close,
regardless of whether it has been called previously or not in the
trigger callbacks.

Signed-off-by: Hector Martin <[email protected]>
When we shut down the amp, we need to wait for the built-in ramp-down
before we can remove the TDM clocks. There is no documented status
regiter to poll, so the best we can do is a delay. Datasheet says 5.9ms
for ramp-down and 1.5ms between shutdown and next startup, so let's do
6ms after mute and 2ms after shutdown. That gives us a cumulative 8ms
for ramp-down and guaratees the required minimum shutdown time.

Signed-off-by: Hector Martin <[email protected]>
0xf seems to fix the random overcurrent behavior.

Signed-off-by: Hector Martin <[email protected]>
Instead of open-coding a fixup function for each platform, let's make it
declarative. This is a lot less error-prone.

Signed-off-by: Hector Martin <[email protected]>
Multiple amps can be connected to the same SDZ GPIO. Using raw GPIOs for
this breaks, as there is no concept of refcounting/sharing. In order to
model these platforms, introduce support for an SDZ "regulator". This
allows us to represent the SDZ GPIO as a simple regulator-fixed, and
then the regulator core takes care of refcounting so that all codecs are
only powered down once all the driver instances are in the suspend
state.

This also reworks the sleep/resume logic to copy what tas2764 does,
which makes more sense.

Signed-off-by: Hector Martin <[email protected]>
The ISENSE/VSENSE blocks are only powered up when the amplifier
transitions from shutdown to active. This means that if those controls
are flipped on while the amplifier is already playing back audio, they
will have no effect.

Fix this by forcing a power cycle around transitions in those controls.

Signed-off-by: Hector Martin <[email protected]>
Expose the bits that control the behavior of the SDOUT pin when not
actively transmitting slot data. Zero-fill is useful when there is a
single amp on the SDOUT bus (e.g. Apple machines with mono speakers or a
single stereo pair, where L/R are on separate buses).

Pull-down is useful, though not perfect, when multiple amps share a
bus. It typically takes around 2 bits for the line to transition from
high to low after going Hi-Z, with the pull-down.

Signed-off-by: Hector Martin <[email protected]>
We don't actually support configuring the PDM input right now. Rather,
this is useful as a hack.

On Apple Silicon machines, amps are split between two I2S buses which
are logically ANDed internally at the SoC. Odd and even slot groups are
driven by amps on either bus respectively. Since the signals are ANDed,
unused slot groups must be driven as zero to avoid corrupting the data
from the other side.

On most recent machines (TAS2764-based), this is accomplished using the
"SDOUT zero mask" feature of that chip. Unfortunately, TAS2770 does not
support this. It does support zeroing out *all* unused slots, which
works well for machines with a single amp per I2S bus. That is all,
except one.

The 13" M1 MacBook Pro is the only machine using TAS2764 and two amps
per I2S bus:

L Bus: SPK0I SPK0V Hi-Z  Hi-Z  SPK2I SPK2V Hi-Z  Hi-Z
R Bus: Hi-Z  Hi-Z  SPK1I SPK2V Hi-Z  Hi-Z  SPK3I SPK3V

To ensure uncorrupted data, we need to force all the Hi-Z periods to
zero. We cannot use the "force all zero" feature, as that would cause a
bus conflict between both amps. We can use the pull-down feature, but
that leaves a few bits of garbage on the trailing edge of the speaker
data, since the pull-down is weak.

This is where the PDM transmit feature comes in. With PDM grounded and
disabled (the default state), the PDM slot is transmitted as all zeroes.
We can use that to force a zero 16-bit slot after the voltage data for
each speaker, cleaning it up. Then the pull-down ensures the line stays
low for the subsequent slot:

L Bus: SPK0I SPK0V PDM0  PulDn SPK2I SPK2V PDM0  PulDn
R Bus: PDM0  PulDn SPK1I SPK2V PDM0  PulDn SPK3I SPK3V

Yes, this is a horrible hack, but it beats adding dummy slots that would
be visible to the userspace capture side. There may be some other way to
fix the logical AND behavior on the MCA side... that would make this
unnecessary.

("How does Apple deal with this"? - they don't, macOS does not use
IVSENSE on TAS2764 machines even though it's physically wired up,
but we want to do so on Linux.)

Signed-off-by: Hector Martin <[email protected]>
The scale starts at -100dB, not -128dB.

Signed-off-by: Hector Martin <[email protected]>
This one already uses a gain lower than the others. It doesn't look like
full scale no-DSP output with typical music is particularly dangerous here,
and we probably want the headroom for DSP, so let's not do it for this
one.

Signed-off-by: Hector Martin <[email protected]>
This PCM triggers speakersafetyd, so hide it if it can't work.

Signed-off-by: Hector Martin <[email protected]>
Still hard gated on speakersafetyd for now.

Signed-off-by: Hector Martin <[email protected]>
TX launch polarity needs to be the opposite of RX capture polarity, to
generate the right bit slot alignment.

Signed-off-by: Hector Martin <[email protected]>
asahilina and others added 25 commits September 25, 2024 02:55
Possibly because we don't have support in the helper program, this is
broken and causes channel errors. Hack in high priority for now, which
works around it.

Use debug_flags 0x1000000000000 to re-enable for testing.

Signed-off-by: Asahi Lina <[email protected]>
After commit c96b547:
    rust: Add `name` argument to Module::init()
init needs a name argument. Since both rust samples were made
    before that, they fail to compile.

Signed-off-by: Bruno Oliveira <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.