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

[Dmitry Osipenko <dmitry.osipenko@collabora.com>]

On 7/21/26 08:31, Igor Paunovic wrote:
> 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.

Feel free to send the follow ups.

-- 
Best regards,
Dmitry