Re: [PATCH v3 2/4] media: synopsys: hdmirx: add HDMI audio capture support

[Igor Paunovic]

Hi Dmitry,

> Re-arming will be best option if it works properly.

Went back to it and made it work properly -- with one honest caveat
at the end.

Plain cancel/re-arm could not have been enough as previously
written: hdmirx_resume() fully resets the controller via
reset_control_bulk_assert/deassert in hdmirx_enable(), wiping the
audio configuration (I2S_EN, AUDIO_ENABLE, FIFO thresholds), and
the worker's error path never re-programs those -- it only resyncs
the clock rate and reinits the FIFO. So v4 does:

- suspend: cancel_delayed_work_sync() on the audio worker before
the clocks are gated (audio_streaming stays set);
- resume: if a capture stream is active, re-program the whole
audio path (clock from the last known sample rate, FIFO
thresholds, I2S_EN, AUDIO_ENABLE) and re-arm the worker.

The programming is factored out of hw_params() into a shared
hdmirx_audio_setup() helper so resume and hw_params cannot drift.
I also checked that nothing in the post-resume hotplug re-init path
(plugout/plugin/submodule_init/controller_init) touches the audio
bits, so the restore survives the replug dance resume triggers.

Measured on the Orange Pi 5 Plus (pm_test=devices, i.e. full device
suspend/resume callbacks, active capture stream with live audio
through the whole cycle): on resume the capture simply continues --
audio was flowing again the moment the display came back, no
userspace intervention, clean dmesg, no oops in any of the ~6
cycles exercised tonight.

The caveat: a *second* suspend/resume cycle in the same boot leaves
the audio datapath silent (the I2S slave sees no bit clock, so
capture reads time out) even though the resume restore re-programs
everything, and a fresh stream open does not recover it -- only a
reboot does. Notably this happens with plain no-stream suspend
cycles as well, so it is not caused by (and would not be prevented
by) either the re-arm or the -EBUSY approach -- it looks like more
controller state lost on repeated reset that neither hw_params nor
resume currently covers, likely the same warm-up state machine I
described in the v3 testing note (audio needing a video STREAMON
after a replug). I am instrumenting the register state next to
root-cause it; I'd treat it as a follow-up rather than a v4
blocker, but happy to hear if you see it differently -- and curious
whether the 5B shows the same second-cycle behavior.

v4 follows with the re-arm plus the dts and binding changes already
mentioned.

One more small thing the audio bit exposes, for a possible
follow-up: hdmirx_cec_update_bits() is a non-atomic RMW (the read
and the write each take rst_lock separately), and with audio added
there are now two writers of GLOBAL_SWENABLE, so a CEC adapter
enable racing an audio enable/disable could in theory lose the
other side's bit. Mainline today has a single writer, so it is not
a bug there. Can send a small patch making it atomic if you think
it is worth it.

Best regards,
Igor

On Mon, Jul 20, 2026 at 4:37 PM Dmitry Osipenko
<dmitry.osipenko@xxxxxxxxxxxxx> wrote:
>
> On 7/20/26 15:30, Igor Paunovic wrote:
> > Hi Dmitry,
> >
> > Great to hear it works on the 5B -- thanks for testing!
> >
> > Agreed on the -EBUSY guard, done in my tree exactly as you wrote it.
> > It also supersedes the suspend cancel/re-arm I had queued from the
> > bot review: with suspend refused while audio_streaming is set, the
> > worker can never be armed at suspend time, so those hunks are
> > dropped.
> >
> > v4 follows shortly with this plus Sebastian's dts restructuring
> > (shared disabled-by-default card in rk3588-extra.dtsi, renamed to
> > "RK3588 HDMI-IN") and Krzysztof's binding comments (dai-common.yaml
> > reference + unevaluatedProperties).
>
> Re-arming will be best option if it works properly.
>
> --
> Best regards,
> Dmitry