Re: [PATCH] drm/bridge: sii902x: Add audio context to suspend/resume routine
From: Wang, Sen
Date: Wed Mar 18 2026 - 15:35:43 EST
Hi Devarsh,
Thanks for the review.
On 3/18/2026 8:39 AM, Thakkar, Devarsh wrote:
Hi Sen,
Thanks for the patch.
On 12/02/26 02:40, Sen Wang wrote:
The sii902x driver has existing suspend/resume handlers that save and
restore video-related register context (TPI mode and interrupts), but
these handlers were not saving/restoring audio configuration registers.
This caused HDMI audio to stop working after system suspend/resume cycles.
Therefore add audio-related register context to the existing
suspend/resume handlers when audio context needs to be saved/restored. As
well as mclk for the sake of power saving, in the case of sii902x being
the frame producer.
The audio context is only saved/restored when audio.active is true,
avoiding unnecessary register access when audio is not in use.
Tested on TI SK-AM62P-LP board with HDMI audio playback across multiple
suspend/resume cycles.
Signed-off-by: Sen Wang <sen@xxxxxx>
---
drivers/gpu/drm/bridge/sii902x.c | 91 +++++++++++++++++++++++++++++++-
1 file changed, 90 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 134657041799..fd38a6ae86b2 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -192,6 +192,13 @@ struct sii902x {
struct platform_device *pdev;
struct clk *mclk;
u32 i2s_fifo_sequence[4];
+ bool active;
+ /* Audio register context for save/resume */
+ unsigned int ctx_i2s_input_config;
+ unsigned int ctx_audio_config_byte2;
+ unsigned int ctx_audio_config_byte3;
+ u8 ctx_i2s_stream_header[SII902X_TPI_I2S_STRM_HDR_SIZE];
+ u8 ctx_audio_infoframe[SII902X_TPI_MISC_INFOFRAME_SIZE];
} audio;
};
@@ -764,6 +771,8 @@ static int sii902x_audio_hw_params(struct device *dev, void *data,
if (ret)
goto out;
+ sii902x->audio.active = true;
+
dev_dbg(dev, "%s: hdmi audio enabled\n", __func__);
out:
mutex_unlock(&sii902x->mutex);
@@ -786,6 +795,8 @@ static void sii902x_audio_shutdown(struct device *dev, void *data)
regmap_write(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE2_REG,
SII902X_TPI_AUDIO_INTERFACE_DISABLE);
+ sii902x->audio.active = false;
+
mutex_unlock(&sii902x->mutex);
clk_disable_unprepare(sii902x->audio.mclk);
@@ -1081,7 +1092,7 @@ static int __maybe_unused sii902x_resume(struct device *dev)
{
struct sii902x *sii902x = dev_get_drvdata(dev);
unsigned int tpi_reg, status;
- int ret;
+ int ret, i;
ret = regmap_read(sii902x->regmap, SII902X_REG_TPI_RQB, &tpi_reg);
if (ret)
@@ -1109,7 +1120,62 @@ static int __maybe_unused sii902x_resume(struct device *dev)
regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
+ /*
+ * Restore audio context if audio was active before suspend,
+ * in the matching order of sii902x_audio_hw_params()
+ * initialization
+ */
+ if (sii902x->audio.active) {
+ /* Re-enable mclk */
+ ret = clk_prepare_enable(sii902x->audio.mclk);
+ if (ret) {
+ dev_err(dev, "Failed to re-enable mclk: %d\n", ret);
+ return ret;
+ }
+
+ ret = regmap_write(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE2_REG,
+ sii902x->audio.ctx_audio_config_byte2);
+ if (ret)
+ goto err_audio_resume;
+
+ ret = regmap_write(sii902x->regmap, SII902X_TPI_I2S_INPUT_CONFIG_REG,
+ sii902x->audio.ctx_i2s_input_config);
+ if (ret)
+ goto err_audio_resume;
+
+ for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) &&
+ sii902x->audio.i2s_fifo_sequence[i]; i++) {
+ ret = regmap_write(sii902x->regmap,
+ SII902X_TPI_I2S_ENABLE_MAPPING_REG,
+ sii902x->audio.i2s_fifo_sequence[i]);
+ if (ret)
+ goto err_audio_resume;
+ }
+
+ ret = regmap_write(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE3_REG,
+ sii902x->audio.ctx_audio_config_byte3);
+ if (ret)
+ goto err_audio_resume;
+
+ ret = regmap_bulk_write(sii902x->regmap, SII902X_TPI_I2S_STRM_HDR_BASE,
+ sii902x->audio.ctx_i2s_stream_header,
+ SII902X_TPI_I2S_STRM_HDR_SIZE);
+ if (ret)
+ goto err_audio_resume;
+
+ ret = regmap_bulk_write(sii902x->regmap, SII902X_TPI_MISC_INFOFRAME_BASE,
+ sii902x->audio.ctx_audio_infoframe,
+ SII902X_TPI_MISC_INFOFRAME_SIZE);
+ if (ret)
+ goto err_audio_resume;
+ }
+
return 0;
+
+err_audio_resume:
+ clk_disable_unprepare(sii902x->audio.mclk);
+ dev_err(dev, "Failed to restore audio registers: %d\n", ret);
+ return ret;
}
static int __maybe_unused sii902x_suspend(struct device *dev)
@@ -1122,6 +1188,29 @@ static int __maybe_unused sii902x_suspend(struct device *dev)
regmap_read(sii902x->regmap, SII902X_INT_ENABLE,
&sii902x->ctx_interrupt);
+ /*
+ * Save audio context if audio is active, and
+ * in the matching order of sii902x_audio_hw_params()
+ * initialization
+ */
+ if (sii902x->audio.active) {
+ regmap_read(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE2_REG,
+ &sii902x->audio.ctx_audio_config_byte2);
+ regmap_read(sii902x->regmap, SII902X_TPI_I2S_INPUT_CONFIG_REG,
+ &sii902x->audio.ctx_i2s_input_config);
+ regmap_read(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE3_REG,
+ &sii902x->audio.ctx_audio_config_byte3);
+ regmap_bulk_read(sii902x->regmap, SII902X_TPI_I2S_STRM_HDR_BASE,
+ sii902x->audio.ctx_i2s_stream_header,
+ SII902X_TPI_I2S_STRM_HDR_SIZE);
+ regmap_bulk_read(sii902x->regmap, SII902X_TPI_MISC_INFOFRAME_BASE,
+ sii902x->audio.ctx_audio_infoframe,
+ SII902X_TPI_MISC_INFOFRAME_SIZE);
+
In resume path you were checking failure for each regmap_write I guess
similar approach is needed here if going with register reads, so in case
one or more regmap_read/regmap_bulk_read fails you can return error with
dev_err message.
Okay sounds good, I'll add that precaution checks as well.
But I think even better approach would be to cache the corresponding reg
values to corresponding struct members during hw_params callback itself
given all the parameters being used sii902x->audio.ctx do not change
between hw_params callback and suspend callback as all these params are
stream related? , If so that would avoid costly i2c operations for
reading register values which can be skipped in this function.
I tried using regcache on the context save/restore, but there is definitely some state machine write sequence that I'm struggling to isolate. The moment I changed the flow from the init route, context save/restore no longer works.
The driver is definitely too scuffed to go through this kind of major revision if we wanted to incorporate regcache. So unfortunately this is the second best option to consider.
This is a bad wording on my part, audio.active indicates an active stream. Stream status should be preserved and resumes playback on+ /* Disable mclk during suspend */
+ clk_disable_unprepare(sii902x->audio.mclk);
+ }
+
audio.active should be cleared here or do we need to call audio_shutdown
explicitly?
context restore.
Also my apologies on sending an incomplete patch, looks like there's
a prerequisite patch sitting in vendor kernel that was never upstreamed.
Will clean it up and send a new patch altogether.
Best,
Sen Wang
Regards
Devarsh