* [PATCH] drm/bridge: sii902x: Add Power Management hooks with audio context @ 2026-04-01 1:37 Sen Wang 2026-04-01 14:42 ` Devarsh Thakkar 0 siblings, 1 reply; 8+ messages in thread From: Sen Wang @ 2026-04-01 1:37 UTC (permalink / raw) To: Andrzej Hajda, Neil Armstrong, Robert Foss Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel, linux-kernel, Devarsh Thakkar, Rahul Donadkar, Siddharth Jain, Sen Wang Add suspend and resume hooks. On suspend, save TPI mode, interrupt enable, and audio register context (when active). On resume, detect power loss by comparing the saved TPI value; if changed, reset the device and restore TPI mode and interrupts. Restore audio registers and re-enable mclk if audio was active before suspend. Audio register values are read back during suspend rather than cached during hw_params, as the sii902x requires a specific register write sequence during initialization that must be preserved on resume. Based on initial PM hooks implementation by: Aradhya Bhatia <a-bhatia1@ti.com> Jayesh Choudhary <j-choudhary@ti.com> Signed-off-by: Sen Wang <sen@ti.com> --- Tested on TI SK-AM62P-LP board with HDMI audio playback across multiple suspend/resume cycles. drivers/gpu/drm/bridge/sii902x.c | 165 +++++++++++++++++++++++++++++++ 1 file changed, 165 insertions(+) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 12497f5ce4ff..8da8ca22ac99 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -179,6 +179,8 @@ struct sii902x { struct gpio_desc *reset_gpio; struct i2c_mux_core *i2cmux; u32 bus_width; + unsigned int ctx_tpi; + unsigned int ctx_interrupt; /* * Mutex protects audio and video functions from interfering @@ -189,6 +191,13 @@ struct sii902x { struct platform_device *pdev; struct clk *mclk; u32 i2s_fifo_sequence[4]; + bool active; + /* Audio register context for suspend/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; }; @@ -755,6 +764,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); @@ -777,6 +788,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); @@ -1069,6 +1082,157 @@ static const struct drm_bridge_timings default_sii902x_timings = { | DRM_BUS_FLAG_DE_HIGH, }; +static int sii902x_resume(struct device *dev) +{ + struct sii902x *sii902x = dev_get_drvdata(dev); + unsigned int tpi_reg, status; + int ret, i; + + ret = regmap_read(sii902x->regmap, SII902X_REG_TPI_RQB, &tpi_reg); + if (ret) + return ret; + + if (tpi_reg != sii902x->ctx_tpi) { + /* + * TPI register context has changed. SII902X power supply + * device has been turned off and on. + */ + sii902x_reset(sii902x); + + /* Configure the device to enter TPI mode. */ + ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0); + if (ret) + return ret; + + /* Re-enable the interrupts */ + regmap_write(sii902x->regmap, SII902X_INT_ENABLE, + sii902x->ctx_interrupt); + } + + /* Clear all pending interrupts */ + 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) { + 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 sii902x_suspend(struct device *dev) +{ + struct sii902x *sii902x = dev_get_drvdata(dev); + int ret; + + ret = regmap_read(sii902x->regmap, SII902X_REG_TPI_RQB, + &sii902x->ctx_tpi); + if (ret) + return ret; + + ret = regmap_read(sii902x->regmap, SII902X_INT_ENABLE, + &sii902x->ctx_interrupt); + if (ret) + return ret; + + /* + * Save audio context if audio is active, in the matching order + * of sii902x_audio_hw_params() initialization. + */ + if (sii902x->audio.active) { + ret = regmap_read(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE2_REG, + &sii902x->audio.ctx_audio_config_byte2); + if (ret) + goto err_audio_suspend; + + ret = regmap_read(sii902x->regmap, SII902X_TPI_I2S_INPUT_CONFIG_REG, + &sii902x->audio.ctx_i2s_input_config); + if (ret) + goto err_audio_suspend; + + ret = regmap_read(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE3_REG, + &sii902x->audio.ctx_audio_config_byte3); + if (ret) + goto err_audio_suspend; + + ret = regmap_bulk_read(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_suspend; + + ret = regmap_bulk_read(sii902x->regmap, SII902X_TPI_MISC_INFOFRAME_BASE, + sii902x->audio.ctx_audio_infoframe, + SII902X_TPI_MISC_INFOFRAME_SIZE); + if (ret) + goto err_audio_suspend; + + /* + * audio.active is kept true so that resume restores the audio + * context. sii902x_audio_shutdown() clears it when the stream + * is explicitly closed. + */ + clk_disable_unprepare(sii902x->audio.mclk); + } + + return 0; + +err_audio_suspend: + dev_err(dev, "Failed to save audio context: %d\n", ret); + return ret; +} + +static DEFINE_SIMPLE_DEV_PM_OPS(sii902x_pm_ops, sii902x_suspend, sii902x_resume); + static int sii902x_init(struct sii902x *sii902x) { struct device *dev = &sii902x->i2c->dev; @@ -1247,6 +1411,7 @@ static struct i2c_driver sii902x_driver = { .remove = sii902x_remove, .driver = { .name = "sii902x", + .pm = pm_sleep_ptr(&sii902x_pm_ops), .of_match_table = sii902x_dt_ids, }, .id_table = sii902x_i2c_ids, -- 2.43.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: Add Power Management hooks with audio context 2026-04-01 1:37 [PATCH] drm/bridge: sii902x: Add Power Management hooks with audio context Sen Wang @ 2026-04-01 14:42 ` Devarsh Thakkar 2026-04-01 23:33 ` Sen Wang 0 siblings, 1 reply; 8+ messages in thread From: Devarsh Thakkar @ 2026-04-01 14:42 UTC (permalink / raw) To: Sen Wang, Andrzej Hajda, Neil Armstrong, Robert Foss Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel, linux-kernel, Rahul Donadkar, Siddharth Jain Hi Sen, On 01/04/26 07:07, Sen Wang wrote: Sorry but this is not very clear, is this a V2 to https://lore.kernel.org/all/e6497541-f3e7-4533-a188-9d422cb34d74@ti.com/ ? In that case, the subject should mention it as PATCH v2 along with changelog as documented in kernel patch guidelines : https://docs.kernel.org/process/submitting-patches.html > Add suspend and resume hooks. On suspend, save TPI mode, interrupt > enable, and audio register context (when active). On resume, detect > power loss by comparing the saved TPI value; if changed, reset the > device and restore TPI mode and interrupts. Restore audio registers > and re-enable mclk if audio was active before suspend. > > Audio register values are read back during suspend rather than cached > during hw_params, as the sii902x requires a specific register write > sequence during initialization that must be preserved on resume. > > Based on initial PM hooks implementation by: > Aradhya Bhatia <a-bhatia1@ti.com> > Jayesh Choudhary <j-choudhary@ti.com> > > Signed-off-by: Sen Wang <sen@ti.com> > --- > Tested on TI SK-AM62P-LP board with HDMI audio playback across multiple > suspend/resume cycles. Could you please also share the test logs ? Did you verify that both audio/video context resume back seamlessly afer resuming from system suspend ? I guess this still does not support runtime suspend/resume ? And assuming this is v2, here you should mention changelog and v1 link: V1: https://lore.kernel.org/all/e6497541-f3e7-4533-a188-9d422cb34d74@ti.com/ > > drivers/gpu/drm/bridge/sii902x.c | 165 +++++++++++++++++++++++++++++++ > 1 file changed, 165 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c > index 12497f5ce4ff..8da8ca22ac99 100644 > --- a/drivers/gpu/drm/bridge/sii902x.c > +++ b/drivers/gpu/drm/bridge/sii902x.c > @@ -179,6 +179,8 @@ struct sii902x { > struct gpio_desc *reset_gpio; > struct i2c_mux_core *i2cmux; > u32 bus_width; > + unsigned int ctx_tpi; > + unsigned int ctx_interrupt; > > /* > * Mutex protects audio and video functions from interfering > @@ -189,6 +191,13 @@ struct sii902x { > struct platform_device *pdev; > struct clk *mclk; > u32 i2s_fifo_sequence[4]; > + bool active; > + /* Audio register context for suspend/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; > }; > > @@ -755,6 +764,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); > @@ -777,6 +788,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); > @@ -1069,6 +1082,157 @@ static const struct drm_bridge_timings default_sii902x_timings = { > | DRM_BUS_FLAG_DE_HIGH, > }; > > +static int sii902x_resume(struct device *dev) > +{ > + struct sii902x *sii902x = dev_get_drvdata(dev); > + unsigned int tpi_reg, status; > + int ret, i; > + > + ret = regmap_read(sii902x->regmap, SII902X_REG_TPI_RQB, &tpi_reg); > + if (ret) > + return ret; > + > + if (tpi_reg != sii902x->ctx_tpi) { > + /* > + * TPI register context has changed. SII902X power supply > + * device has been turned off and on. > + */ > + sii902x_reset(sii902x); > + > + /* Configure the device to enter TPI mode. */ > + ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0); > + if (ret) > + return ret; > + > + /* Re-enable the interrupts */ > + regmap_write(sii902x->regmap, SII902X_INT_ENABLE, > + sii902x->ctx_interrupt); > + } > + > + /* Clear all pending interrupts */ > + 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) { audio.active should be protected with mutex lock as done elsewhere in the driver? > + 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; > + } > + Do we need to restore the indirect registers status too with the constants that were used ? /* Decode Level 0 Packets */ regmap_write(regmap, SII902X_IND_SET_PAGE, 0x02); /* 0xBC */ regmap_write(regmap, SII902X_IND_OFFSET, 0x24); /* 0xBD */ regmap_write(regmap, SII902X_IND_VALUE, 0x02); /* 0xBE */ ``` > + 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 sii902x_suspend(struct device *dev) > +{ > + struct sii902x *sii902x = dev_get_drvdata(dev); > + int ret; > + Have you reviewed Table 3.8 of the datasheet ? I think we should probably be utilizing different operating modes during suspend/resume cycles. For e.g. with system suspend go to D3 cold state which is absolute minimum power. For runtime suspend thought, have to be little careful as the D state should be chosen such that it does not have a too high resume latency. If D3 doesn't have too high then probably use the same else use D2. > + ret = regmap_read(sii902x->regmap, SII902X_REG_TPI_RQB, > + &sii902x->ctx_tpi); > + if (ret) > + return ret; > + > + ret = regmap_read(sii902x->regmap, SII902X_INT_ENABLE, > + &sii902x->ctx_interrupt); > + if (ret) > + return ret; > + > + /* > + * Save audio context if audio is active, in the matching order > + * of sii902x_audio_hw_params() initialization. > + */ > + if (sii902x->audio.active) { Here too, I think mutex protection required while accessing this flag. > + ret = regmap_read(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE2_REG, > + &sii902x->audio.ctx_audio_config_byte2); > + if (ret) > + goto err_audio_suspend; > + > + ret = regmap_read(sii902x->regmap, SII902X_TPI_I2S_INPUT_CONFIG_REG, > + &sii902x->audio.ctx_i2s_input_config); > + if (ret) > + goto err_audio_suspend; > + > + ret = regmap_read(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE3_REG, > + &sii902x->audio.ctx_audio_config_byte3); > + if (ret) > + goto err_audio_suspend; > + > + ret = regmap_bulk_read(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_suspend; > + > + ret = regmap_bulk_read(sii902x->regmap, SII902X_TPI_MISC_INFOFRAME_BASE, > + sii902x->audio.ctx_audio_infoframe, > + SII902X_TPI_MISC_INFOFRAME_SIZE); > + if (ret) > + goto err_audio_suspend; > + In the previous revision, my comment was to skip register reads for restoring the context and instead populate the context in hw_params itself : something as below : @@ -710,18 +717,36 @@ static int sii902x_audio_hw_params(struct device *dev, void *data, ret = regmap_write(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE2_REG, config_byte2_reg); - if (ret < 0) + if (ret < 0) goto out; + sii902x->audio.ctx_audio_config_byte2 = config_byte2_reg; ret = regmap_write(sii902x->regmap, SII902X_TPI_I2S_INPUT_CONFIG_REG, i2s_config_reg); if (ret) goto out; + sii902x->audio.ctx_i2s_input_config = i2s_config_reg; for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) && sii902x->audio.i2s_fifo_sequence[i]; i++) regmap_write(sii902x->regmap, SII902X_TPI_I2S_ENABLE_MAPPING_REG, and likewise and then you don't have to do reg reads in resume back > + /* > + * audio.active is kept true so that resume restores the audio > + * context. sii902x_audio_shutdown() clears it when the stream > + * is explicitly closed. > + */ > + clk_disable_unprepare(sii902x->audio.mclk); > + } > + > + return 0; > + > +err_audio_suspend: > + dev_err(dev, "Failed to save audio context: %d\n", ret); > + return ret; > +} > + > +static DEFINE_SIMPLE_DEV_PM_OPS(sii902x_pm_ops, sii902x_suspend, sii902x_resume); > + Can we add runtime suspend/resume hooks too ? > static int sii902x_init(struct sii902x *sii902x) > { > struct device *dev = &sii902x->i2c->dev; > @@ -1247,6 +1411,7 @@ static struct i2c_driver sii902x_driver = { > .remove = sii902x_remove, > .driver = { > .name = "sii902x", > + .pm = pm_sleep_ptr(&sii902x_pm_ops), > .of_match_table = sii902x_dt_ids, > }, > .id_table = sii902x_i2c_ids, Regards Devarsh ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: Add Power Management hooks with audio context 2026-04-01 14:42 ` Devarsh Thakkar @ 2026-04-01 23:33 ` Sen Wang 2026-04-02 7:07 ` Devarsh Thakkar 0 siblings, 1 reply; 8+ messages in thread From: Sen Wang @ 2026-04-01 23:33 UTC (permalink / raw) To: Thakkar, Devarsh, Andrzej Hajda, Neil Armstrong, Robert Foss Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel, linux-kernel, Donadkar, Rishikesh, Jain, Swamil On 4/1/26 09:42, Thakkar, Devarsh wrote: > Hi Sen, > > On 01/04/26 07:07, Sen Wang wrote: > > Sorry but this is not very clear, is this a V2 to > https://lore.kernel.org/all/e6497541-f3e7-4533-a188-9d422cb34d74@ti.com/ ? > > In that case, the subject should mention it as PATCH v2 along with > changelog as documented in kernel patch guidelines : > https://docs.kernel.org/process/submitting-patches.html > Hi Devarsh, Thanks for your review. This new patch encompasses more features than the previous patch to warrant it being a separate patch. But nonetheless my apologies for not stating the indirection in my commit message. >> Add suspend and resume hooks. On suspend, save TPI mode, interrupt >> enable, and audio register context (when active). On resume, detect >> power loss by comparing the saved TPI value; if changed, reset the >> device and restore TPI mode and interrupts. Restore audio registers >> and re-enable mclk if audio was active before suspend. >> >> Audio register values are read back during suspend rather than cached >> during hw_params, as the sii902x requires a specific register write >> sequence during initialization that must be preserved on resume. >> >> Based on initial PM hooks implementation by: >> Aradhya Bhatia <a-bhatia1@ti.com> >> Jayesh Choudhary <j-choudhary@ti.com> >> >> Signed-off-by: Sen Wang <sen@ti.com> >> --- >> Tested on TI SK-AM62P-LP board with HDMI audio playback across multiple >> suspend/resume cycles. > > Could you please also share the test logs ? Did you verify that both > audio/video context resume back seamlessly afer resuming from system > suspend ? > Okay I'll attach it to the commit message for v2 patch. > I guess this still does not support runtime suspend/resume ? > No it doesn't, I don't know if full suspend/resume is necessarily the correct solution for runtime as well, besides there's also ways to do it in mode_set/atomic callbacks. Not well-versed in DRM framework to draw the conclusion here. Runtime PM should ideally decouple sound and video to maximize the power saving. Let me take a deeper look and see if this chip has features that benefits from runtime PM. > And assuming this is v2, here you should mention changelog and v1 link: > V1: https://lore.kernel.org/all/e6497541-f3e7-4533-a188-9d422cb34d74@ti.com/ >> >> drivers/gpu/drm/bridge/sii902x.c | 165 +++++++++++++++++++++++++++++++ >> 1 file changed, 165 insertions(+) >> >> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c >> index 12497f5ce4ff..8da8ca22ac99 100644 >> --- a/drivers/gpu/drm/bridge/sii902x.c >> +++ b/drivers/gpu/drm/bridge/sii902x.c >> @@ -179,6 +179,8 @@ struct sii902x { >> struct gpio_desc *reset_gpio; >> struct i2c_mux_core *i2cmux; >> u32 bus_width; >> + unsigned int ctx_tpi; >> + unsigned int ctx_interrupt; >> >> /* >> * Mutex protects audio and video functions from interfering >> @@ -189,6 +191,13 @@ struct sii902x { >> struct platform_device *pdev; >> struct clk *mclk; >> u32 i2s_fifo_sequence[4]; >> + bool active; >> + /* Audio register context for suspend/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; >> }; >> >> @@ -755,6 +764,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); >> @@ -777,6 +788,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); >> @@ -1069,6 +1082,157 @@ static const struct drm_bridge_timings default_sii902x_timings = { >> | DRM_BUS_FLAG_DE_HIGH, >> }; >> >> +static int sii902x_resume(struct device *dev) >> +{ >> + struct sii902x *sii902x = dev_get_drvdata(dev); >> + unsigned int tpi_reg, status; >> + int ret, i; >> + >> + ret = regmap_read(sii902x->regmap, SII902X_REG_TPI_RQB, &tpi_reg); >> + if (ret) >> + return ret; >> + >> + if (tpi_reg != sii902x->ctx_tpi) { >> + /* >> + * TPI register context has changed. SII902X power supply >> + * device has been turned off and on. >> + */ >> + sii902x_reset(sii902x); >> + >> + /* Configure the device to enter TPI mode. */ >> + ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0); >> + if (ret) >> + return ret; >> + >> + /* Re-enable the interrupts */ >> + regmap_write(sii902x->regmap, SII902X_INT_ENABLE, >> + sii902x->ctx_interrupt); >> + } >> + >> + /* Clear all pending interrupts */ >> + 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) { > > audio.active should be protected with mutex lock as done elsewhere in > the driver? > Sounds good, I'm assuming PM resume/suspend are atomic but nonetheless need to safeguard it from existing ops. >> + 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; >> + } >> + > > Do we need to restore the indirect registers status too with the > constants that were used ? > > /* Decode Level 0 Packets */ > > regmap_write(regmap, SII902X_IND_SET_PAGE, 0x02); /* 0xBC */ > > regmap_write(regmap, SII902X_IND_OFFSET, 0x24); /* 0xBD */ > > regmap_write(regmap, SII902X_IND_VALUE, 0x02); /* 0xBE */ > > ``` > I didn't see any difference when I restore these registers but let me double check just to be sure. >> + 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 sii902x_suspend(struct device *dev) >> +{ >> + struct sii902x *sii902x = dev_get_drvdata(dev); >> + int ret; >> + > > Have you reviewed Table 3.8 of the datasheet ? > > I think we should probably be utilizing different operating modes during > suspend/resume cycles. > > For e.g. with system suspend go to D3 cold state which is absolute > minimum power. > > For runtime suspend thought, have to be little careful as the D state > should be chosen such that it does not have a too high resume latency. > If D3 doesn't have too high then probably use the same else use D2. > >> + ret = regmap_read(sii902x->regmap, SII902X_REG_TPI_RQB, >> + &sii902x->ctx_tpi); >> + if (ret) >> + return ret; >> + >> + ret = regmap_read(sii902x->regmap, SII902X_INT_ENABLE, >> + &sii902x->ctx_interrupt); >> + if (ret) >> + return ret; >> + >> + /* >> + * Save audio context if audio is active, in the matching order >> + * of sii902x_audio_hw_params() initialization. >> + */ >> + if (sii902x->audio.active) { > > Here too, I think mutex protection required while accessing this flag. > Understood >> + ret = regmap_read(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE2_REG, >> + &sii902x->audio.ctx_audio_config_byte2); >> + if (ret) >> + goto err_audio_suspend; >> + >> + ret = regmap_read(sii902x->regmap, SII902X_TPI_I2S_INPUT_CONFIG_REG, >> + &sii902x->audio.ctx_i2s_input_config); >> + if (ret) >> + goto err_audio_suspend; >> + >> + ret = regmap_read(sii902x->regmap, SII902X_TPI_AUDIO_CONFIG_BYTE3_REG, >> + &sii902x->audio.ctx_audio_config_byte3); >> + if (ret) >> + goto err_audio_suspend; >> + >> + ret = regmap_bulk_read(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_suspend; >> + >> + ret = regmap_bulk_read(sii902x->regmap, SII902X_TPI_MISC_INFOFRAME_BASE, >> + sii902x->audio.ctx_audio_infoframe, >> + SII902X_TPI_MISC_INFOFRAME_SIZE); >> + if (ret) >> + goto err_audio_suspend; >> + > > In the previous revision, my comment was to skip register reads for > restoring the context and instead populate the context in hw_params itself : > > something as below : > @@ -710,18 +717,36 @@ static int sii902x_audio_hw_params(struct device > *dev, void *data, > ret = regmap_write(sii902x->regmap, > > SII902X_TPI_AUDIO_CONFIG_BYTE2_REG, > > config_byte2_reg); > > - if (ret < 0) > > + if (ret < 0) > > goto out; > > + sii902x->audio.ctx_audio_config_byte2 = config_byte2_reg; > > > > ret = regmap_write(sii902x->regmap, > SII902X_TPI_I2S_INPUT_CONFIG_REG, > i2s_config_reg); > > if (ret) > > goto out; > > + sii902x->audio.ctx_i2s_input_config = i2s_config_reg; > > > > for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) && > > sii902x->audio.i2s_fifo_sequence[i]; i++) > > regmap_write(sii902x->regmap, > > SII902X_TPI_I2S_ENABLE_MAPPING_REG, > > and likewise and then you don't have to do reg reads in resume back > Okay, let me analyze this. >> + /* >> + * audio.active is kept true so that resume restores the audio >> + * context. sii902x_audio_shutdown() clears it when the stream >> + * is explicitly closed. >> + */ >> + clk_disable_unprepare(sii902x->audio.mclk); >> + } >> + >> + return 0; >> + >> +err_audio_suspend: >> + dev_err(dev, "Failed to save audio context: %d\n", ret); >> + return ret; >> +} >> + >> +static DEFINE_SIMPLE_DEV_PM_OPS(sii902x_pm_ops, sii902x_suspend, sii902x_resume); >> + > > Can we add runtime suspend/resume hooks too ? > I can add what we have for runtime, although I need to investigate this is actually makes a difference. >> static int sii902x_init(struct sii902x *sii902x) >> { >> struct device *dev = &sii902x->i2c->dev; >> @@ -1247,6 +1411,7 @@ static struct i2c_driver sii902x_driver = { >> .remove = sii902x_remove, >> .driver = { >> .name = "sii902x", >> + .pm = pm_sleep_ptr(&sii902x_pm_ops), >> .of_match_table = sii902x_dt_ids, >> }, >> .id_table = sii902x_i2c_ids, > > Regards > Devarsh Thanks for your review Devarsh, let me investigate and follow-up with a v2 patch. Best regards, Sen Wang ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: Add Power Management hooks with audio context 2026-04-01 23:33 ` Sen Wang @ 2026-04-02 7:07 ` Devarsh Thakkar 2026-04-04 17:41 ` Sen Wang 0 siblings, 1 reply; 8+ messages in thread From: Devarsh Thakkar @ 2026-04-02 7:07 UTC (permalink / raw) To: Sen Wang, Andrzej Hajda, Neil Armstrong, Robert Foss Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel, linux-kernel, Donadkar, Rishikesh, Jain, Swamil Hi Sen, Thanks for the update. On 02/04/26 05:03, Sen Wang wrote: > On 4/1/26 09:42, Thakkar, Devarsh wrote: >> Hi Sen, >> >> On 01/04/26 07:07, Sen Wang wrote: >> >> Sorry but this is not very clear, is this a V2 to >> https://lore.kernel.org/all/e6497541-f3e7-4533- >> a188-9d422cb34d74@ti.com/ ? >> >> In that case, the subject should mention it as PATCH v2 along with >> changelog as documented in kernel patch guidelines : >> https://docs.kernel.org/process/submitting-patches.html >> > Hi Devarsh, Thanks for your review. > > This new patch encompasses more features than the previous patch to > warrant it being a separate patch. But nonetheless my apologies for > not stating the indirection in my commit message. > I don't think this qualifies to make it an independent patch altogether, as long as goal of the patch is same as the initial one posted, this should be labelled as a V2 with linkage to V1 in changelog. And the follow up patch should be labelled V3. The changelog should capture the changes under ---: V1->V2 : What changed V2->V3 : What changed along with links for V1 and V2. This gives the reviewer necessary context on architecture and reasoning behind new changes/approaches even if it means the new patch contains some extra feature which was not present in previous patch. Regards Devarsh >>> Add suspend and resume hooks. On suspend, save TPI mode, interrupt >>> enable, and audio register context (when active). On resume, detect >>> power loss by comparing the saved TPI value; if changed, reset the >>> device and restore TPI mode and interrupts. Restore audio registers >>> and re-enable mclk if audio was active before suspend. >>> >>> Audio register values are read back during suspend rather than cached >>> during hw_params, as the sii902x requires a specific register write >>> sequence during initialization that must be preserved on resume. >>> >>> Based on initial PM hooks implementation by: >>> Aradhya Bhatia <a-bhatia1@ti.com> >>> Jayesh Choudhary <j-choudhary@ti.com> >>> >>> Signed-off-by: Sen Wang <sen@ti.com> >>> --- >>> Tested on TI SK-AM62P-LP board with HDMI audio playback across multiple >>> suspend/resume cycles. >> >> Could you please also share the test logs ? Did you verify that both >> audio/video context resume back seamlessly afer resuming from system >> suspend ? >> > > Okay I'll attach it to the commit message for v2 patch. > >> I guess this still does not support runtime suspend/resume ? >> > No it doesn't, I don't know if full suspend/resume is necessarily the > correct solution for runtime as well, besides there's also ways to do it > in mode_set/atomic callbacks. Not well-versed in DRM framework to draw > the conclusion here. > > Runtime PM should ideally decouple sound and video to maximize the power > saving. Let me take a deeper look and see if this chip has features that > benefits from runtime PM. > >> And assuming this is v2, here you should mention changelog and v1 link: >> V1: https://lore.kernel.org/all/e6497541-f3e7-4533- >> a188-9d422cb34d74@ti.com/ >>> >>> drivers/gpu/drm/bridge/sii902x.c | 165 +++++++++++++++++++++++++++ >>> ++++ >>> 1 file changed, 165 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/ >>> bridge/sii902x.c >>> index 12497f5ce4ff..8da8ca22ac99 100644 >>> --- a/drivers/gpu/drm/bridge/sii902x.c >>> +++ b/drivers/gpu/drm/bridge/sii902x.c >>> @@ -179,6 +179,8 @@ struct sii902x { >>> struct gpio_desc *reset_gpio; >>> struct i2c_mux_core *i2cmux; >>> u32 bus_width; >>> + unsigned int ctx_tpi; >>> + unsigned int ctx_interrupt; >>> /* >>> * Mutex protects audio and video functions from interfering >>> @@ -189,6 +191,13 @@ struct sii902x { >>> struct platform_device *pdev; >>> struct clk *mclk; >>> u32 i2s_fifo_sequence[4]; >>> + bool active; >>> + /* Audio register context for suspend/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; >>> }; >>> @@ -755,6 +764,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); >>> @@ -777,6 +788,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); >>> @@ -1069,6 +1082,157 @@ static const struct drm_bridge_timings >>> default_sii902x_timings = { >>> | DRM_BUS_FLAG_DE_HIGH, >>> }; >>> +static int sii902x_resume(struct device *dev) >>> +{ >>> + struct sii902x *sii902x = dev_get_drvdata(dev); >>> + unsigned int tpi_reg, status; >>> + int ret, i; >>> + >>> + ret = regmap_read(sii902x->regmap, SII902X_REG_TPI_RQB, &tpi_reg); >>> + if (ret) >>> + return ret; >>> + >>> + if (tpi_reg != sii902x->ctx_tpi) { >>> + /* >>> + * TPI register context has changed. SII902X power supply >>> + * device has been turned off and on. >>> + */ >>> + sii902x_reset(sii902x); >>> + >>> + /* Configure the device to enter TPI mode. */ >>> + ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0); >>> + if (ret) >>> + return ret; >>> + >>> + /* Re-enable the interrupts */ >>> + regmap_write(sii902x->regmap, SII902X_INT_ENABLE, >>> + sii902x->ctx_interrupt); >>> + } >>> + >>> + /* Clear all pending interrupts */ >>> + 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) { >> >> audio.active should be protected with mutex lock as done elsewhere in >> the driver? >> > Sounds good, I'm assuming PM resume/suspend are atomic but nonetheless > need to safeguard it from existing ops. >>> + 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; >>> + } >>> + >> >> Do we need to restore the indirect registers status too with the >> constants that were used ? >> >> /* Decode Level 0 Packets */ >> >> regmap_write(regmap, SII902X_IND_SET_PAGE, 0x02); /* 0xBC */ >> >> regmap_write(regmap, SII902X_IND_OFFSET, 0x24); /* 0xBD */ >> >> regmap_write(regmap, SII902X_IND_VALUE, 0x02); /* 0xBE */ >> >> ``` >> > I didn't see any difference when I restore these registers but let me > double check just to be sure. >>> + 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 sii902x_suspend(struct device *dev) >>> +{ >>> + struct sii902x *sii902x = dev_get_drvdata(dev); >>> + int ret; >>> + >> >> Have you reviewed Table 3.8 of the datasheet ? >> >> I think we should probably be utilizing different operating modes during >> suspend/resume cycles. >> >> For e.g. with system suspend go to D3 cold state which is absolute >> minimum power. >> >> For runtime suspend thought, have to be little careful as the D state >> should be chosen such that it does not have a too high resume latency. >> If D3 doesn't have too high then probably use the same else use D2. >> >>> + ret = regmap_read(sii902x->regmap, SII902X_REG_TPI_RQB, >>> + &sii902x->ctx_tpi); >>> + if (ret) >>> + return ret; >>> + >>> + ret = regmap_read(sii902x->regmap, SII902X_INT_ENABLE, >>> + &sii902x->ctx_interrupt); >>> + if (ret) >>> + return ret; >>> + >>> + /* >>> + * Save audio context if audio is active, in the matching order >>> + * of sii902x_audio_hw_params() initialization. >>> + */ >>> + if (sii902x->audio.active) { >> >> Here too, I think mutex protection required while accessing this flag. >> > > Understood >>> + ret = regmap_read(sii902x->regmap, >>> SII902X_TPI_AUDIO_CONFIG_BYTE2_REG, >>> + &sii902x->audio.ctx_audio_config_byte2); >>> + if (ret) >>> + goto err_audio_suspend; >>> + >>> + ret = regmap_read(sii902x->regmap, >>> SII902X_TPI_I2S_INPUT_CONFIG_REG, >>> + &sii902x->audio.ctx_i2s_input_config); >>> + if (ret) >>> + goto err_audio_suspend; >>> + >>> + ret = regmap_read(sii902x->regmap, >>> SII902X_TPI_AUDIO_CONFIG_BYTE3_REG, >>> + &sii902x->audio.ctx_audio_config_byte3); >>> + if (ret) >>> + goto err_audio_suspend; >>> + >>> + ret = regmap_bulk_read(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_suspend; >>> + >>> + ret = regmap_bulk_read(sii902x->regmap, >>> SII902X_TPI_MISC_INFOFRAME_BASE, >>> + sii902x->audio.ctx_audio_infoframe, >>> + SII902X_TPI_MISC_INFOFRAME_SIZE); >>> + if (ret) >>> + goto err_audio_suspend; >>> + >> >> In the previous revision, my comment was to skip register reads for >> restoring the context and instead populate the context in hw_params >> itself : >> >> something as below : >> @@ -710,18 +717,36 @@ static int sii902x_audio_hw_params(struct device >> *dev, void *data, >> ret = regmap_write(sii902x->regmap, >> >> SII902X_TPI_AUDIO_CONFIG_BYTE2_REG, >> >> config_byte2_reg); >> >> - if (ret < 0) >> >> + if (ret < 0) >> >> goto out; >> >> + sii902x->audio.ctx_audio_config_byte2 = config_byte2_reg; >> >> >> ret = regmap_write(sii902x->regmap, >> SII902X_TPI_I2S_INPUT_CONFIG_REG, >> i2s_config_reg); >> >> if (ret) >> >> goto out; >> >> + sii902x->audio.ctx_i2s_input_config = i2s_config_reg; >> >> >> for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) && >> >> sii902x->audio.i2s_fifo_sequence[i]; i++) >> >> regmap_write(sii902x->regmap, >> >> SII902X_TPI_I2S_ENABLE_MAPPING_REG, >> >> and likewise and then you don't have to do reg reads in resume back >> > Okay, let me analyze this. >>> + /* >>> + * audio.active is kept true so that resume restores the audio >>> + * context. sii902x_audio_shutdown() clears it when the stream >>> + * is explicitly closed. >>> + */ >>> + clk_disable_unprepare(sii902x->audio.mclk); >>> + } >>> + >>> + return 0; >>> + >>> +err_audio_suspend: >>> + dev_err(dev, "Failed to save audio context: %d\n", ret); >>> + return ret; >>> +} >>> + >>> +static DEFINE_SIMPLE_DEV_PM_OPS(sii902x_pm_ops, sii902x_suspend, >>> sii902x_resume); >>> + >> >> Can we add runtime suspend/resume hooks too ? >> > > I can add what we have for runtime, although I need to investigate this > is actually makes a difference. > >>> static int sii902x_init(struct sii902x *sii902x) >>> { >>> struct device *dev = &sii902x->i2c->dev; >>> @@ -1247,6 +1411,7 @@ static struct i2c_driver sii902x_driver = { >>> .remove = sii902x_remove, >>> .driver = { >>> .name = "sii902x", >>> + .pm = pm_sleep_ptr(&sii902x_pm_ops), >>> .of_match_table = sii902x_dt_ids, >>> }, >>> .id_table = sii902x_i2c_ids, >> >> Regards >> Devarsh > > Thanks for your review Devarsh, let me investigate and follow-up with a > v2 patch. > > Best regards, > Sen Wang ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: Add Power Management hooks with audio context 2026-04-02 7:07 ` Devarsh Thakkar @ 2026-04-04 17:41 ` Sen Wang 2026-04-08 17:55 ` Sen Wang 2026-04-09 15:11 ` Devarsh Thakkar 0 siblings, 2 replies; 8+ messages in thread From: Sen Wang @ 2026-04-04 17:41 UTC (permalink / raw) To: Thakkar, Devarsh, Andrzej Hajda, Neil Armstrong, Robert Foss Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel, linux-kernel, Donadkar, Rishikesh, Jain, Swamil On 4/2/26 02:07, Thakkar, Devarsh wrote: > Hi Sen, > > Thanks for the update. > > On 02/04/26 05:03, Sen Wang wrote: >> On 4/1/26 09:42, Thakkar, Devarsh wrote: >>> Hi Sen, >>> >>> On 01/04/26 07:07, Sen Wang wrote: >>> >>> Sorry but this is not very clear, is this a V2 to >>> https://lore.kernel.org/all/e6497541-f3e7-4533- >>> a188-9d422cb34d74@ti.com/ ? >>> >>> In that case, the subject should mention it as PATCH v2 along with >>> changelog as documented in kernel patch guidelines : >>> https://docs.kernel.org/process/submitting-patches.html >>> >> Hi Devarsh, Thanks for your review. >> >> This new patch encompasses more features than the previous patch to >> warrant it being a separate patch. But nonetheless my apologies for >> not stating the indirection in my commit message. >> > > I don't think this qualifies to make it an independent patch altogether, > as long as goal of the patch is same as the initial one posted, this > should be labelled as a V2 with linkage to V1 in changelog. And the > follow up patch should be labelled V3. > > The changelog should capture the changes under ---: > > V1->V2 : What changed > V2->V3 : What changed > > along with links for V1 and V2. > > This gives the reviewer necessary context on architecture and reasoning > behind new changes/approaches even if it means the new patch contains > some extra feature which was not present in previous patch. > > Regards > Devarsh > > Understood Devarsh, thank you for the detailed explanation. Deeply appreciated. I also dug more into the datasheet with your comments in mind and wanted to gather some feedback on the proposal before sending a proper V3 patch. Here's my proposal, TL;DR: - Runtime suspend to use D2 (Sleep) for a quick bringup - System suspend to use D3 Hot for a complete power-off/on scheme For the full summary: (per Datasheet's powerstate, Table 3.8) D0 - Full power D2 - Quiet Power Down (i2c accessible) D3 Hot - Complete Power Down (via HPD or RSEN wkup) D3 Cold - Complete Power Down (HPD) Runtime suspend on sii902x should NOT use the same flow as System suspend - Runtime suspend to use D2 for a quick bringup - System suspend to use D3 Hot for a complete power-off/on scheme + Hot vs Cold: even though Cold use marginally less power, it can only be entered and wkup via HPD event (Hotplug detect), which is not a software-controlled event. In such case, only system suspend needs to cache TPI and audio context, reset & initalise the hardware. Whereas runtime suspend should just be in a D2 state where all reg values are preserved. Thus providing faster response time for runtime resume, especially when we don't have autosuspend (yet), and avoids latency for spontaneous events in the DRM framework. Best, Sen Wang >>>> Add suspend and resume hooks. On suspend, save TPI mode, interrupt >>>> enable, and audio register context (when active). On resume, detect >>>> power loss by comparing the saved TPI value; if changed, reset the >>>> device and restore TPI mode and interrupts. Restore audio registers >>>> and re-enable mclk if audio was active before suspend. >>>> >>>> Audio register values are read back during suspend rather than cached >>>> during hw_params, as the sii902x requires a specific register write >>>> sequence during initialization that must be preserved on resume. >>>> >>>> Based on initial PM hooks implementation by: >>>> Aradhya Bhatia <a-bhatia1@ti.com> >>>> Jayesh Choudhary <j-choudhary@ti.com> >>>> >>>> Signed-off-by: Sen Wang <sen@ti.com> >>>> --- >>>> Tested on TI SK-AM62P-LP board with HDMI audio playback across multiple >>>> suspend/resume cycles. >>> >>> Could you please also share the test logs ? Did you verify that both >>> audio/video context resume back seamlessly afer resuming from system >>> suspend ? >>> >> >> Okay I'll attach it to the commit message for v2 patch. >> >>> I guess this still does not support runtime suspend/resume ? >>> >> No it doesn't, I don't know if full suspend/resume is necessarily the >> correct solution for runtime as well, besides there's also ways to do it >> in mode_set/atomic callbacks. Not well-versed in DRM framework to draw >> the conclusion here. >> >> Runtime PM should ideally decouple sound and video to maximize the power >> saving. Let me take a deeper look and see if this chip has features that >> benefits from runtime PM. >> >>> And assuming this is v2, here you should mention changelog and v1 link: >>> V1: https://lore.kernel.org/all/e6497541-f3e7-4533- >>> a188-9d422cb34d74@ti.com/ >>>> >>>> drivers/gpu/drm/bridge/sii902x.c | 165 +++++++++++++++++++++++++++ >>>> ++++ >>>> 1 file changed, 165 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/ >>>> bridge/sii902x.c >>>> index 12497f5ce4ff..8da8ca22ac99 100644 >>>> --- a/drivers/gpu/drm/bridge/sii902x.c >>>> +++ b/drivers/gpu/drm/bridge/sii902x.c >>>> @@ -179,6 +179,8 @@ struct sii902x { >>>> struct gpio_desc *reset_gpio; >>>> struct i2c_mux_core *i2cmux; >>>> u32 bus_width; >>>> + unsigned int ctx_tpi; >>>> + unsigned int ctx_interrupt; >>>> /* >>>> * Mutex protects audio and video functions from interfering >>>> @@ -189,6 +191,13 @@ struct sii902x { >>>> struct platform_device *pdev; >>>> struct clk *mclk; >>>> u32 i2s_fifo_sequence[4]; >>>> + bool active; >>>> + /* Audio register context for suspend/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; >>>> }; >>>> @@ -755,6 +764,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); >>>> @@ -777,6 +788,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); >>>> @@ -1069,6 +1082,157 @@ static const struct drm_bridge_timings >>>> default_sii902x_timings = { >>>> | DRM_BUS_FLAG_DE_HIGH, >>>> }; >>>> +static int sii902x_resume(struct device *dev) >>>> +{ >>>> + struct sii902x *sii902x = dev_get_drvdata(dev); >>>> + unsigned int tpi_reg, status; >>>> + int ret, i; >>>> + >>>> + ret = regmap_read(sii902x->regmap, SII902X_REG_TPI_RQB, &tpi_reg); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + if (tpi_reg != sii902x->ctx_tpi) { >>>> + /* >>>> + * TPI register context has changed. SII902X power supply >>>> + * device has been turned off and on. >>>> + */ >>>> + sii902x_reset(sii902x); >>>> + >>>> + /* Configure the device to enter TPI mode. */ >>>> + ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + /* Re-enable the interrupts */ >>>> + regmap_write(sii902x->regmap, SII902X_INT_ENABLE, >>>> + sii902x->ctx_interrupt); >>>> + } >>>> + >>>> + /* Clear all pending interrupts */ >>>> + 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) { >>> >>> audio.active should be protected with mutex lock as done elsewhere in >>> the driver? >>> >> Sounds good, I'm assuming PM resume/suspend are atomic but nonetheless >> need to safeguard it from existing ops. >>>> + 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; >>>> + } >>>> + >>> >>> Do we need to restore the indirect registers status too with the >>> constants that were used ? >>> >>> /* Decode Level 0 Packets */ >>> >>> regmap_write(regmap, SII902X_IND_SET_PAGE, 0x02); /* 0xBC */ >>> >>> regmap_write(regmap, SII902X_IND_OFFSET, 0x24); /* 0xBD */ >>> >>> regmap_write(regmap, SII902X_IND_VALUE, 0x02); /* 0xBE */ >>> >>> ``` >>> >> I didn't see any difference when I restore these registers but let me >> double check just to be sure. >>>> + 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 sii902x_suspend(struct device *dev) >>>> +{ >>>> + struct sii902x *sii902x = dev_get_drvdata(dev); >>>> + int ret; >>>> + >>> >>> Have you reviewed Table 3.8 of the datasheet ? >>> >>> I think we should probably be utilizing different operating modes during >>> suspend/resume cycles. >>> >>> For e.g. with system suspend go to D3 cold state which is absolute >>> minimum power. >>> >>> For runtime suspend thought, have to be little careful as the D state >>> should be chosen such that it does not have a too high resume latency. >>> If D3 doesn't have too high then probably use the same else use D2. >>> >>>> + ret = regmap_read(sii902x->regmap, SII902X_REG_TPI_RQB, >>>> + &sii902x->ctx_tpi); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + ret = regmap_read(sii902x->regmap, SII902X_INT_ENABLE, >>>> + &sii902x->ctx_interrupt); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + /* >>>> + * Save audio context if audio is active, in the matching order >>>> + * of sii902x_audio_hw_params() initialization. >>>> + */ >>>> + if (sii902x->audio.active) { >>> >>> Here too, I think mutex protection required while accessing this flag. >>> >> >> Understood >>>> + ret = regmap_read(sii902x->regmap, >>>> SII902X_TPI_AUDIO_CONFIG_BYTE2_REG, >>>> + &sii902x->audio.ctx_audio_config_byte2); >>>> + if (ret) >>>> + goto err_audio_suspend; >>>> + >>>> + ret = regmap_read(sii902x->regmap, >>>> SII902X_TPI_I2S_INPUT_CONFIG_REG, >>>> + &sii902x->audio.ctx_i2s_input_config); >>>> + if (ret) >>>> + goto err_audio_suspend; >>>> + >>>> + ret = regmap_read(sii902x->regmap, >>>> SII902X_TPI_AUDIO_CONFIG_BYTE3_REG, >>>> + &sii902x->audio.ctx_audio_config_byte3); >>>> + if (ret) >>>> + goto err_audio_suspend; >>>> + >>>> + ret = regmap_bulk_read(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_suspend; >>>> + >>>> + ret = regmap_bulk_read(sii902x->regmap, >>>> SII902X_TPI_MISC_INFOFRAME_BASE, >>>> + sii902x->audio.ctx_audio_infoframe, >>>> + SII902X_TPI_MISC_INFOFRAME_SIZE); >>>> + if (ret) >>>> + goto err_audio_suspend; >>>> + >>> >>> In the previous revision, my comment was to skip register reads for >>> restoring the context and instead populate the context in hw_params >>> itself : >>> >>> something as below : >>> @@ -710,18 +717,36 @@ static int sii902x_audio_hw_params(struct device >>> *dev, void *data, >>> ret = regmap_write(sii902x->regmap, >>> >>> SII902X_TPI_AUDIO_CONFIG_BYTE2_REG, >>> >>> config_byte2_reg); >>> >>> - if (ret < 0) >>> >>> + if (ret < 0) >>> >>> goto out; >>> >>> + sii902x->audio.ctx_audio_config_byte2 = config_byte2_reg; >>> >>> >>> ret = regmap_write(sii902x->regmap, >>> SII902X_TPI_I2S_INPUT_CONFIG_REG, >>> i2s_config_reg); >>> >>> if (ret) >>> >>> goto out; >>> >>> + sii902x->audio.ctx_i2s_input_config = i2s_config_reg; >>> >>> >>> for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) && >>> >>> sii902x->audio.i2s_fifo_sequence[i]; i++) >>> >>> regmap_write(sii902x->regmap, >>> >>> SII902X_TPI_I2S_ENABLE_MAPPING_REG, >>> >>> and likewise and then you don't have to do reg reads in resume back >>> >> Okay, let me analyze this. >>>> + /* >>>> + * audio.active is kept true so that resume restores the audio >>>> + * context. sii902x_audio_shutdown() clears it when the stream >>>> + * is explicitly closed. >>>> + */ >>>> + clk_disable_unprepare(sii902x->audio.mclk); >>>> + } >>>> + >>>> + return 0; >>>> + >>>> +err_audio_suspend: >>>> + dev_err(dev, "Failed to save audio context: %d\n", ret); >>>> + return ret; >>>> +} >>>> + >>>> +static DEFINE_SIMPLE_DEV_PM_OPS(sii902x_pm_ops, sii902x_suspend, >>>> sii902x_resume); >>>> + >>> >>> Can we add runtime suspend/resume hooks too ? >>> >> >> I can add what we have for runtime, although I need to investigate this >> is actually makes a difference. >> >>>> static int sii902x_init(struct sii902x *sii902x) >>>> { >>>> struct device *dev = &sii902x->i2c->dev; >>>> @@ -1247,6 +1411,7 @@ static struct i2c_driver sii902x_driver = { >>>> .remove = sii902x_remove, >>>> .driver = { >>>> .name = "sii902x", >>>> + .pm = pm_sleep_ptr(&sii902x_pm_ops), >>>> .of_match_table = sii902x_dt_ids, >>>> }, >>>> .id_table = sii902x_i2c_ids, >>> >>> Regards >>> Devarsh >> >> Thanks for your review Devarsh, let me investigate and follow-up with a >> v2 patch. >> >> Best regards, >> Sen Wang > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: Add Power Management hooks with audio context 2026-04-04 17:41 ` Sen Wang @ 2026-04-08 17:55 ` Sen Wang 2026-04-09 15:11 ` Devarsh Thakkar 1 sibling, 0 replies; 8+ messages in thread From: Sen Wang @ 2026-04-08 17:55 UTC (permalink / raw) To: Thakkar, Devarsh, Andrzej Hajda, Neil Armstrong, Robert Foss Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel, linux-kernel, Donadkar, Rishikesh, Jain, Swamil On 4/4/26 12:41, Sen Wang wrote: > On 4/2/26 02:07, Thakkar, Devarsh wrote: >> Hi Sen, >> >> Thanks for the update. >> >> On 02/04/26 05:03, Sen Wang wrote: >>> On 4/1/26 09:42, Thakkar, Devarsh wrote: >>>> Hi Sen, >>>> >>>> On 01/04/26 07:07, Sen Wang wrote: >>>> >>>> Sorry but this is not very clear, is this a V2 to >>>> https://lore.kernel.org/all/e6497541-f3e7-4533- >>>> a188-9d422cb34d74@ti.com/ ? >>>> >>>> In that case, the subject should mention it as PATCH v2 along with >>>> changelog as documented in kernel patch guidelines : >>>> https://docs.kernel.org/process/submitting-patches.html >>>> >>> Hi Devarsh, Thanks for your review. >>> >>> This new patch encompasses more features than the previous patch to >>> warrant it being a separate patch. But nonetheless my apologies for >>> not stating the indirection in my commit message. >>> >> >> I don't think this qualifies to make it an independent patch altogether, >> as long as goal of the patch is same as the initial one posted, this >> should be labelled as a V2 with linkage to V1 in changelog. And the >> follow up patch should be labelled V3. >> >> The changelog should capture the changes under ---: >> >> V1->V2 : What changed >> V2->V3 : What changed >> >> along with links for V1 and V2. >> >> This gives the reviewer necessary context on architecture and reasoning >> behind new changes/approaches even if it means the new patch contains >> some extra feature which was not present in previous patch. >> >> Regards >> Devarsh >> >> > Understood Devarsh, thank you for the detailed explanation. Deeply > appreciated. > > I also dug more into the datasheet with your comments in mind and wanted > to gather some feedback on the proposal before sending a proper V3 patch. > > Here's my proposal, TL;DR: > > - Runtime suspend to use D2 (Sleep) for a quick bringup > - System suspend to use D3 Hot for a complete power-off/on scheme > > For the full summary: > > (per Datasheet's powerstate, Table 3.8) > D0 - Full power > D2 - Quiet Power Down (i2c accessible) > D3 Hot - Complete Power Down (via HPD or RSEN wkup) > D3 Cold - Complete Power Down (HPD) > > Runtime suspend on sii902x should NOT use the same flow as System suspend > > - Runtime suspend to use D2 for a quick bringup > - System suspend to use D3 Hot for a complete power-off/on scheme > + Hot vs Cold: even though Cold use marginally less power, it can > only be entered and wkup via HPD event (Hotplug detect), which is not a > software-controlled event. > > > In such case, only system suspend needs to cache TPI and audio context, > reset & initalise the hardware. Whereas runtime suspend should just be > in a D2 state where all reg values are preserved. Thus providing faster > response time for runtime resume, especially when we don't have > autosuspend (yet), and avoids latency for spontaneous events in the DRM > framework. > > Best, > Sen Wang > Hi Devarsh, I have the code ready for V3. So it might be better to review my proposal/code implementation in V3. I will post it soon. Best, Sen Wang >>>>> Add suspend and resume hooks. On suspend, save TPI mode, interrupt >>>>> enable, and audio register context (when active). On resume, detect >>>>> power loss by comparing the saved TPI value; if changed, reset the >>>>> device and restore TPI mode and interrupts. Restore audio registers >>>>> and re-enable mclk if audio was active before suspend. >>>>> >>>>> Audio register values are read back during suspend rather than cached >>>>> during hw_params, as the sii902x requires a specific register write >>>>> sequence during initialization that must be preserved on resume. >>>>> >>>>> Based on initial PM hooks implementation by: >>>>> Aradhya Bhatia <a-bhatia1@ti.com> >>>>> Jayesh Choudhary <j-choudhary@ti.com> >>>>> >>>>> Signed-off-by: Sen Wang <sen@ti.com> >>>>> --- >>>>> Tested on TI SK-AM62P-LP board with HDMI audio playback across >>>>> multiple >>>>> suspend/resume cycles. >>>> >>>> Could you please also share the test logs ? Did you verify that both >>>> audio/video context resume back seamlessly afer resuming from system >>>> suspend ? >>>> >>> >>> Okay I'll attach it to the commit message for v2 patch. >>> >>>> I guess this still does not support runtime suspend/resume ? >>>> >>> No it doesn't, I don't know if full suspend/resume is necessarily the >>> correct solution for runtime as well, besides there's also ways to do it >>> in mode_set/atomic callbacks. Not well-versed in DRM framework to draw >>> the conclusion here. >>> >>> Runtime PM should ideally decouple sound and video to maximize the power >>> saving. Let me take a deeper look and see if this chip has features that >>> benefits from runtime PM. >>> >>>> And assuming this is v2, here you should mention changelog and v1 link: >>>> V1: https://lore.kernel.org/all/e6497541-f3e7-4533- >>>> a188-9d422cb34d74@ti.com/ >>>>> >>>>> drivers/gpu/drm/bridge/sii902x.c | 165 +++++++++++++++++++++++++++ >>>>> ++++ >>>>> 1 file changed, 165 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/ >>>>> bridge/sii902x.c >>>>> index 12497f5ce4ff..8da8ca22ac99 100644 >>>>> --- a/drivers/gpu/drm/bridge/sii902x.c >>>>> +++ b/drivers/gpu/drm/bridge/sii902x.c >>>>> @@ -179,6 +179,8 @@ struct sii902x { >>>>> struct gpio_desc *reset_gpio; >>>>> struct i2c_mux_core *i2cmux; >>>>> u32 bus_width; >>>>> + unsigned int ctx_tpi; >>>>> + unsigned int ctx_interrupt; >>>>> /* >>>>> * Mutex protects audio and video functions from interfering >>>>> @@ -189,6 +191,13 @@ struct sii902x { >>>>> struct platform_device *pdev; >>>>> struct clk *mclk; >>>>> u32 i2s_fifo_sequence[4]; >>>>> + bool active; >>>>> + /* Audio register context for suspend/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; >>>>> }; >>>>> @@ -755,6 +764,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); >>>>> @@ -777,6 +788,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); >>>>> @@ -1069,6 +1082,157 @@ static const struct drm_bridge_timings >>>>> default_sii902x_timings = { >>>>> | DRM_BUS_FLAG_DE_HIGH, >>>>> }; >>>>> +static int sii902x_resume(struct device *dev) >>>>> +{ >>>>> + struct sii902x *sii902x = dev_get_drvdata(dev); >>>>> + unsigned int tpi_reg, status; >>>>> + int ret, i; >>>>> + >>>>> + ret = regmap_read(sii902x->regmap, SII902X_REG_TPI_RQB, >>>>> &tpi_reg); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + if (tpi_reg != sii902x->ctx_tpi) { >>>>> + /* >>>>> + * TPI register context has changed. SII902X power supply >>>>> + * device has been turned off and on. >>>>> + */ >>>>> + sii902x_reset(sii902x); >>>>> + >>>>> + /* Configure the device to enter TPI mode. */ >>>>> + ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, >>>>> 0x0); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + /* Re-enable the interrupts */ >>>>> + regmap_write(sii902x->regmap, SII902X_INT_ENABLE, >>>>> + sii902x->ctx_interrupt); >>>>> + } >>>>> + >>>>> + /* Clear all pending interrupts */ >>>>> + 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) { >>>> >>>> audio.active should be protected with mutex lock as done elsewhere in >>>> the driver? >>>> >>> Sounds good, I'm assuming PM resume/suspend are atomic but nonetheless >>> need to safeguard it from existing ops. >>>>> + 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; >>>>> + } >>>>> + >>>> >>>> Do we need to restore the indirect registers status too with the >>>> constants that were used ? >>>> >>>> /* Decode Level 0 Packets */ >>>> >>>> regmap_write(regmap, SII902X_IND_SET_PAGE, 0x02); /* 0xBC */ >>>> >>>> regmap_write(regmap, SII902X_IND_OFFSET, 0x24); /* 0xBD */ >>>> >>>> regmap_write(regmap, SII902X_IND_VALUE, 0x02); /* 0xBE */ >>>> >>>> ``` >>>> >>> I didn't see any difference when I restore these registers but let me >>> double check just to be sure. >>>>> + 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 sii902x_suspend(struct device *dev) >>>>> +{ >>>>> + struct sii902x *sii902x = dev_get_drvdata(dev); >>>>> + int ret; >>>>> + >>>> >>>> Have you reviewed Table 3.8 of the datasheet ? >>>> >>>> I think we should probably be utilizing different operating modes >>>> during >>>> suspend/resume cycles. >>>> >>>> For e.g. with system suspend go to D3 cold state which is absolute >>>> minimum power. >>>> >>>> For runtime suspend thought, have to be little careful as the D state >>>> should be chosen such that it does not have a too high resume latency. >>>> If D3 doesn't have too high then probably use the same else use D2. >>>> >>>>> + ret = regmap_read(sii902x->regmap, SII902X_REG_TPI_RQB, >>>>> + &sii902x->ctx_tpi); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_read(sii902x->regmap, SII902X_INT_ENABLE, >>>>> + &sii902x->ctx_interrupt); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + /* >>>>> + * Save audio context if audio is active, in the matching order >>>>> + * of sii902x_audio_hw_params() initialization. >>>>> + */ >>>>> + if (sii902x->audio.active) { >>>> >>>> Here too, I think mutex protection required while accessing this flag. >>>> >>> >>> Understood >>>>> + ret = regmap_read(sii902x->regmap, >>>>> SII902X_TPI_AUDIO_CONFIG_BYTE2_REG, >>>>> + &sii902x->audio.ctx_audio_config_byte2); >>>>> + if (ret) >>>>> + goto err_audio_suspend; >>>>> + >>>>> + ret = regmap_read(sii902x->regmap, >>>>> SII902X_TPI_I2S_INPUT_CONFIG_REG, >>>>> + &sii902x->audio.ctx_i2s_input_config); >>>>> + if (ret) >>>>> + goto err_audio_suspend; >>>>> + >>>>> + ret = regmap_read(sii902x->regmap, >>>>> SII902X_TPI_AUDIO_CONFIG_BYTE3_REG, >>>>> + &sii902x->audio.ctx_audio_config_byte3); >>>>> + if (ret) >>>>> + goto err_audio_suspend; >>>>> + >>>>> + ret = regmap_bulk_read(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_suspend; >>>>> + >>>>> + ret = regmap_bulk_read(sii902x->regmap, >>>>> SII902X_TPI_MISC_INFOFRAME_BASE, >>>>> + sii902x->audio.ctx_audio_infoframe, >>>>> + SII902X_TPI_MISC_INFOFRAME_SIZE); >>>>> + if (ret) >>>>> + goto err_audio_suspend; >>>>> + >>>> >>>> In the previous revision, my comment was to skip register reads for >>>> restoring the context and instead populate the context in hw_params >>>> itself : >>>> >>>> something as below : >>>> @@ -710,18 +717,36 @@ static int sii902x_audio_hw_params(struct device >>>> *dev, void *data, >>>> ret = regmap_write(sii902x->regmap, >>>> >>>> SII902X_TPI_AUDIO_CONFIG_BYTE2_REG, >>>> >>>> config_byte2_reg); >>>> >>>> - if (ret < 0) >>>> >>>> + if (ret < 0) >>>> >>>> goto out; >>>> >>>> + sii902x->audio.ctx_audio_config_byte2 = config_byte2_reg; >>>> >>>> >>>> ret = regmap_write(sii902x->regmap, >>>> SII902X_TPI_I2S_INPUT_CONFIG_REG, >>>> i2s_config_reg); >>>> >>>> if (ret) >>>> >>>> goto out; >>>> >>>> + sii902x->audio.ctx_i2s_input_config = i2s_config_reg; >>>> >>>> >>>> for (i = 0; i < ARRAY_SIZE(sii902x- >>>> >audio.i2s_fifo_sequence) && >>>> >>>> sii902x->audio.i2s_fifo_sequence[i]; i++) >>>> >>>> regmap_write(sii902x->regmap, >>>> >>>> SII902X_TPI_I2S_ENABLE_MAPPING_REG, >>>> >>>> and likewise and then you don't have to do reg reads in resume back >>>> >>> Okay, let me analyze this. >>>>> + /* >>>>> + * audio.active is kept true so that resume restores the >>>>> audio >>>>> + * context. sii902x_audio_shutdown() clears it when the >>>>> stream >>>>> + * is explicitly closed. >>>>> + */ >>>>> + clk_disable_unprepare(sii902x->audio.mclk); >>>>> + } >>>>> + >>>>> + return 0; >>>>> + >>>>> +err_audio_suspend: >>>>> + dev_err(dev, "Failed to save audio context: %d\n", ret); >>>>> + return ret; >>>>> +} >>>>> + >>>>> +static DEFINE_SIMPLE_DEV_PM_OPS(sii902x_pm_ops, sii902x_suspend, >>>>> sii902x_resume); >>>>> + >>>> >>>> Can we add runtime suspend/resume hooks too ? >>>> >>> >>> I can add what we have for runtime, although I need to investigate this >>> is actually makes a difference. >>> >>>>> static int sii902x_init(struct sii902x *sii902x) >>>>> { >>>>> struct device *dev = &sii902x->i2c->dev; >>>>> @@ -1247,6 +1411,7 @@ static struct i2c_driver sii902x_driver = { >>>>> .remove = sii902x_remove, >>>>> .driver = { >>>>> .name = "sii902x", >>>>> + .pm = pm_sleep_ptr(&sii902x_pm_ops), >>>>> .of_match_table = sii902x_dt_ids, >>>>> }, >>>>> .id_table = sii902x_i2c_ids, >>>> >>>> Regards >>>> Devarsh >>> >>> Thanks for your review Devarsh, let me investigate and follow-up with a >>> v2 patch. >>> >>> Best regards, >>> Sen Wang >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: Add Power Management hooks with audio context 2026-04-04 17:41 ` Sen Wang 2026-04-08 17:55 ` Sen Wang @ 2026-04-09 15:11 ` Devarsh Thakkar 2026-04-09 20:25 ` Wang, Sen 1 sibling, 1 reply; 8+ messages in thread From: Devarsh Thakkar @ 2026-04-09 15:11 UTC (permalink / raw) To: Sen Wang, Andrzej Hajda, Neil Armstrong, Robert Foss Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel, linux-kernel, Donadkar, Rishikesh, Jain, Swamil Hi Sen, Thanks for looking into this. On 04/04/26 23:11, Sen Wang wrote: > On 4/2/26 02:07, Thakkar, Devarsh wrote: >> Hi Sen, >> >> Thanks for the update. >> >> On 02/04/26 05:03, Sen Wang wrote: >>> On 4/1/26 09:42, Thakkar, Devarsh wrote: >>>> Hi Sen, >>>> >>>> On 01/04/26 07:07, Sen Wang wrote: >>>> >>>> Sorry but this is not very clear, is this a V2 to >>>> https://lore.kernel.org/all/e6497541-f3e7-4533- >>>> a188-9d422cb34d74@ti.com/ ? >>>> >>>> In that case, the subject should mention it as PATCH v2 along with >>>> changelog as documented in kernel patch guidelines : >>>> https://docs.kernel.org/process/submitting-patches.html >>>> >>> Hi Devarsh, Thanks for your review. >>> >>> This new patch encompasses more features than the previous patch to >>> warrant it being a separate patch. But nonetheless my apologies for >>> not stating the indirection in my commit message. >>> >> >> I don't think this qualifies to make it an independent patch altogether, >> as long as goal of the patch is same as the initial one posted, this >> should be labelled as a V2 with linkage to V1 in changelog. And the >> follow up patch should be labelled V3. >> >> The changelog should capture the changes under ---: >> >> V1->V2 : What changed >> V2->V3 : What changed >> >> along with links for V1 and V2. >> >> This gives the reviewer necessary context on architecture and reasoning >> behind new changes/approaches even if it means the new patch contains >> some extra feature which was not present in previous patch. >> >> Regards >> Devarsh >> >> > Understood Devarsh, thank you for the detailed explanation. Deeply > appreciated. > > I also dug more into the datasheet with your comments in mind and wanted > to gather some feedback on the proposal before sending a proper V3 patch. > > Here's my proposal, TL;DR: > > - Runtime suspend to use D2 (Sleep) for a quick bringup > - System suspend to use D3 Hot for a complete power-off/on scheme > > For the full summary: > > (per Datasheet's powerstate, Table 3.8) > D0 - Full power > D2 - Quiet Power Down (i2c accessible) > D3 Hot - Complete Power Down (via HPD or RSEN wkup) > D3 Cold - Complete Power Down (HPD) > > Runtime suspend on sii902x should NOT use the same flow as System suspend > > - Runtime suspend to use D2 for a quick bringup If D3 mode resume latency is faster and comparable to D2 and no obvious tradeoffs then maybe we could use D3 for runtime resume too? > - System suspend to use D3 Hot for a complete power-off/on scheme > + Hot vs Cold: even though Cold use marginally less power, it can > only be entered and wkup via HPD event (Hotplug detect), which is not a > software-controlled event. > For system suspend, anyway the system resume will mostly be done with system wakeup sources so I am not sure if we really need the extra wakeup present in D3 Hot as compared to D3 cold, so probably would be better to stick to D3 cold if no obvious tradeoffs. Also could you please measure resume back latencies for each of the D2 and D3 modes and we can then maybe compare that with the power table to get latency vs power tradeoff view and decide ? > > In such case, only system suspend needs to cache TPI and audio context, > reset & initalise the hardware. Whereas runtime suspend should just be > in a D2 state where all reg values are preserved. Thus providing faster > response time for runtime resume, especially when we don't have > autosuspend (yet), and avoids latency for spontaneous events in the DRM > framework. I think we probably should add auto-suspend support too w.r. runtime suspend as don't want to go to suspend too frequently if the initialization latency (along with power rampup sequences) is too high. Regards Devarsh ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: Add Power Management hooks with audio context 2026-04-09 15:11 ` Devarsh Thakkar @ 2026-04-09 20:25 ` Wang, Sen 0 siblings, 0 replies; 8+ messages in thread From: Wang, Sen @ 2026-04-09 20:25 UTC (permalink / raw) To: Devarsh Thakkar, Andrzej Hajda, Neil Armstrong, Robert Foss Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel, linux-kernel, Donadkar, Rishikesh, Jain, Swamil On 4/9/2026 10:11 AM, Devarsh Thakkar wrote: > Hi Sen, > > Thanks for looking into this. > > On 04/04/26 23:11, Sen Wang wrote: >> On 4/2/26 02:07, Thakkar, Devarsh wrote: >>> Hi Sen, >>> >>> Thanks for the update. >>> >>> On 02/04/26 05:03, Sen Wang wrote: >>>> On 4/1/26 09:42, Thakkar, Devarsh wrote: >>>>> Hi Sen, >>>>> >>>>> On 01/04/26 07:07, Sen Wang wrote: >>>>> >>>>> Sorry but this is not very clear, is this a V2 to >>>>> https://lore.kernel.org/all/e6497541-f3e7-4533- >>>>> a188-9d422cb34d74@ti.com/ ? >>>>> >>>>> In that case, the subject should mention it as PATCH v2 along with >>>>> changelog as documented in kernel patch guidelines : >>>>> https://docs.kernel.org/process/submitting-patches.html >>>>> >>>> Hi Devarsh, Thanks for your review. >>>> >>>> This new patch encompasses more features than the previous patch to >>>> warrant it being a separate patch. But nonetheless my apologies for >>>> not stating the indirection in my commit message. >>>> >>> >>> I don't think this qualifies to make it an independent patch altogether, >>> as long as goal of the patch is same as the initial one posted, this >>> should be labelled as a V2 with linkage to V1 in changelog. And the >>> follow up patch should be labelled V3. >>> >>> The changelog should capture the changes under ---: >>> >>> V1->V2 : What changed >>> V2->V3 : What changed >>> >>> along with links for V1 and V2. >>> >>> This gives the reviewer necessary context on architecture and reasoning >>> behind new changes/approaches even if it means the new patch contains >>> some extra feature which was not present in previous patch. >>> >>> Regards >>> Devarsh >>> >>> >> Understood Devarsh, thank you for the detailed explanation. Deeply >> appreciated. >> >> I also dug more into the datasheet with your comments in mind and >> wanted to gather some feedback on the proposal before sending a proper >> V3 patch. >> >> Here's my proposal, TL;DR: >> >> - Runtime suspend to use D2 (Sleep) for a quick bringup >> - System suspend to use D3 Hot for a complete power-off/on scheme >> >> For the full summary: >> >> (per Datasheet's powerstate, Table 3.8) >> D0 - Full power >> D2 - Quiet Power Down (i2c accessible) >> D3 Hot - Complete Power Down (via HPD or RSEN wkup) >> D3 Cold - Complete Power Down (HPD) >> >> Runtime suspend on sii902x should NOT use the same flow as System suspend >> >> - Runtime suspend to use D2 for a quick bringup > > If D3 mode resume latency is faster and comparable to D2 and no obvious > tradeoffs then maybe we could use D3 for runtime resume too? > D2 resume presumably will be a lot faster because it's a shallow sleep. Also for backwards compatibility, D3 requires a full context switch and I can't guarantee runtime resume would preserve all the states that user relies on. Currently we only restore the essential registers needed for video and audio context. >> - System suspend to use D3 Hot for a complete power-off/on scheme >> + Hot vs Cold: even though Cold use marginally less power, it can >> only be entered and wkup via HPD event (Hotplug detect), which is not >> a software-controlled event. >> > > For system suspend, anyway the system resume will mostly be done with > system wakeup sources so I am not sure if we really need the extra > wakeup present in D3 Hot as compared to D3 cold, so probably would be > better to stick to D3 cold if no obvious tradeoffs. > Well we can't use D3 cold in system suspend. D3 cold only triggers via HDMI wakeup, so it's a passive mechanism rather than the PM's active trigger. That does bring up an alternative approach. Rather than using PM framework I'm more familiar with, we could rely on DRM atomic KMS. Since it handles hotplug detection and we're offloading impl to the core DRM helper APIs. This way no system resume/suspend is needed, but we still get the benefits of D2 runtime resume. > Also could you please measure resume back latencies for each of the D2 > and D3 modes and we can then maybe compare that with the power table to > get latency vs power tradeoff view and decide ? > Compatibility is the biggest concern I have, rather be safe than sorry. And I don't think it's worthwhile to focus so much on sporadic events that runtime PM handles, when the narrative is on the overarching power management. >> >> In such case, only system suspend needs to cache TPI and audio >> context, reset & initalise the hardware. Whereas runtime suspend >> should just be in a D2 state where all reg values are preserved. Thus >> providing faster response time for runtime resume, especially when we >> don't have autosuspend (yet), and avoids latency for spontaneous >> events in the DRM framework. > > I think we probably should add auto-suspend support too w.r. runtime > suspend as don't want to go to suspend too frequently if the > initialization latency (along with power rampup sequences) is too high. > Okay I can do that. > Regards > Devarsh > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-04-09 20:25 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-04-01 1:37 [PATCH] drm/bridge: sii902x: Add Power Management hooks with audio context Sen Wang 2026-04-01 14:42 ` Devarsh Thakkar 2026-04-01 23:33 ` Sen Wang 2026-04-02 7:07 ` Devarsh Thakkar 2026-04-04 17:41 ` Sen Wang 2026-04-08 17:55 ` Sen Wang 2026-04-09 15:11 ` Devarsh Thakkar 2026-04-09 20:25 ` Wang, Sen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®