[PATCH v2 0/3] Clean some logs on rockchip hdmi audio

[PATCH v2 0/3] Clean some logs on rockchip hdmi audio

[Detlev Casanova]

HDMI audio is showing lots of logs, particularly when HDMI is
disconnected.

It doubled on RK3588 with the addition of the second HDMI TX port, so
clean it up to avoid missing important log lines.

Changes since v1:
 - Add comment about clock requirements
 - Add Fixes tags to the drm/bridge commits
 - Link to v1: https://lore.kernel.org/all/20250717215620.288651-1-detlev.casanova@collabora.com/

Detlev Casanova (3):
  drm/bridge: dw-hdmi-qp: Return 0 in audio prepare when disconnected
  ASoC: hdac_hdmi: Use dev_info on invalid ELD version
  drm/bridge: synopsys: Do not warn about audio params computation

 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 13 ++++++++++---
 sound/soc/codecs/hdac_hdmi.c                 |  2 +-
 2 files changed, 11 insertions(+), 4 deletions(-)

-- 
2.50.1

[PATCH v2 1/3] drm/bridge: dw-hdmi-qp: Return 0 in audio prepare when disconnected

[Detlev Casanova]

To configure audio registers, the clock of the video port in use must be
enabled.
As those clocks are managed by the VOP driver, they can't be enabled here
to write the registers even when the HDMI cable is disconnected.

Furthermore, the registers values are computed from the TMDS char rate,
which is not available when disconnected.

Returning -ENODEV seemed reasonable at first, but ASoC will log an error
multiple times if dw_hdmi_qp_audio_prepare() return an error.
Userspace might also retry multiple times, filling the kernel log with:

hdmi-audio-codec hdmi-audio-codec.0.auto: ASoC error (-19): at snd_soc_dai_prepare() on i2s-hifi

This has become even worse with the support of the second HDMI TX port.

Activating the clocks to write fake data (fake because the TMDS char
rate is unavailable) would require API changes to communicate between
VOP and HDMI, which doesn't really make sense.

Using a cached regmap to be dumped when a cable is connected won't work
because writing order is important and some data needs to be retrieved
from registers to write others.

Returning 0 to silently fail sounds like the best and simplest solution.

Fixes: fd0141d1a8a2 ("drm/bridge: synopsys: Add audio support for dw-hdmi-qp")
Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index 5e5f8c2f95be1..9b9d43c02e3a5 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -458,8 +458,16 @@ static int dw_hdmi_qp_audio_prepare(struct drm_connector *connector,
 	struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
 	bool ref2stream = false;
 
+	/*
+	 * Silently return if tmds_char_rate is not set.
+	 *
+	 * Writing audio registers requires that the clock of the Video Port currently in
+	 * use by the VOP (dclk_vp<id>) is enabled.
+	 * That clock is guaranteed to be enabled when hdmi->tmds_char_rate is set, so we
+	 * only configure audio when it is set.
+	 */
 	if (!hdmi->tmds_char_rate)
-		return -ENODEV;
+		return 0;
 
 	if (fmt->bit_clk_provider | fmt->frame_clk_provider) {
 		dev_err(hdmi->dev, "unsupported clock settings\n");
-- 
2.50.1

Re: [PATCH v2 1/3] drm/bridge: dw-hdmi-qp: Return 0 in audio prepare when disconnected

[Dmitry Baryshkov]

On Tue, Jul 22, 2025 at 03:54:35PM -0400, Detlev Casanova wrote:
> To configure audio registers, the clock of the video port in use must be
> enabled.
> As those clocks are managed by the VOP driver, they can't be enabled here
> to write the registers even when the HDMI cable is disconnected.
> 
> Furthermore, the registers values are computed from the TMDS char rate,
> which is not available when disconnected.
> 
> Returning -ENODEV seemed reasonable at first, but ASoC will log an error
> multiple times if dw_hdmi_qp_audio_prepare() return an error.
> Userspace might also retry multiple times, filling the kernel log with:
> 
> hdmi-audio-codec hdmi-audio-codec.0.auto: ASoC error (-19): at snd_soc_dai_prepare() on i2s-hifi
> 
> This has become even worse with the support of the second HDMI TX port.
> 
> Activating the clocks to write fake data (fake because the TMDS char
> rate is unavailable) would require API changes to communicate between
> VOP and HDMI, which doesn't really make sense.
> 
> Using a cached regmap to be dumped when a cable is connected won't work
> because writing order is important and some data needs to be retrieved
> from registers to write others.
> 
> Returning 0 to silently fail sounds like the best and simplest solution.
> 
> Fixes: fd0141d1a8a2 ("drm/bridge: synopsys: Add audio support for dw-hdmi-qp")
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> index 5e5f8c2f95be1..9b9d43c02e3a5 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> @@ -458,8 +458,16 @@ static int dw_hdmi_qp_audio_prepare(struct drm_connector *connector,
>  	struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
>  	bool ref2stream = false;
>  
> +	/*
> +	 * Silently return if tmds_char_rate is not set.
> +	 *
> +	 * Writing audio registers requires that the clock of the Video Port currently in
> +	 * use by the VOP (dclk_vp<id>) is enabled.
> +	 * That clock is guaranteed to be enabled when hdmi->tmds_char_rate is set, so we
> +	 * only configure audio when it is set.
> +	 */
>  	if (!hdmi->tmds_char_rate)
> -		return -ENODEV;
> +		return 0;

What if the cable gets diconnected _while_ this function is running?

>  
>  	if (fmt->bit_clk_provider | fmt->frame_clk_provider) {
>  		dev_err(hdmi->dev, "unsupported clock settings\n");
> -- 
> 2.50.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 1/3] drm/bridge: dw-hdmi-qp: Return 0 in audio prepare when disconnected

[Detlev Casanova]

Hi Dmitry
On Wednesday, 23 July 2025 07:41:43 EDT Dmitry Baryshkov wrote:
> On Tue, Jul 22, 2025 at 03:54:35PM -0400, Detlev Casanova wrote:
> > To configure audio registers, the clock of the video port in use must be
> > enabled.
> > As those clocks are managed by the VOP driver, they can't be enabled here
> > to write the registers even when the HDMI cable is disconnected.
> > 
> > Furthermore, the registers values are computed from the TMDS char rate,
> > which is not available when disconnected.
> > 
> > Returning -ENODEV seemed reasonable at first, but ASoC will log an error
> > multiple times if dw_hdmi_qp_audio_prepare() return an error.
> > Userspace might also retry multiple times, filling the kernel log with:
> > 
> > hdmi-audio-codec hdmi-audio-codec.0.auto: ASoC error (-19): at
> > snd_soc_dai_prepare() on i2s-hifi
> > 
> > This has become even worse with the support of the second HDMI TX port.
> > 
> > Activating the clocks to write fake data (fake because the TMDS char
> > rate is unavailable) would require API changes to communicate between
> > VOP and HDMI, which doesn't really make sense.
> > 
> > Using a cached regmap to be dumped when a cable is connected won't work
> > because writing order is important and some data needs to be retrieved
> > from registers to write others.
> > 
> > Returning 0 to silently fail sounds like the best and simplest solution.
> > 
> > Fixes: fd0141d1a8a2 ("drm/bridge: synopsys: Add audio support for
> > dw-hdmi-qp") Signed-off-by: Detlev Casanova
> > <detlev.casanova@collabora.com>
> > ---
> > 
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c index
> > 5e5f8c2f95be1..9b9d43c02e3a5 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> > @@ -458,8 +458,16 @@ static int dw_hdmi_qp_audio_prepare(struct
> > drm_connector *connector,> 
> >  	struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
> >  	bool ref2stream = false;
> > 
> > +	/*
> > +	 * Silently return if tmds_char_rate is not set.
> > +	 *
> > +	 * Writing audio registers requires that the clock of the Video Port
> > currently in +	 * use by the VOP (dclk_vp<id>) is enabled.
> > +	 * That clock is guaranteed to be enabled when hdmi->tmds_char_rate 
is
> > set, so we +	 * only configure audio when it is set.
> > +	 */
> > 
> >  	if (!hdmi->tmds_char_rate)
> > 
> > -		return -ENODEV;
> > +		return 0;
> 
> What if the cable gets diconnected _while_ this function is running?

Unplugging the cable will not deactivate the clocks, that's the job of the 
VOP's vop2_crtc_atomic_disable() function. My understanding is that that 
function will not be called until this one has returned, but I could be wrong.

> >  	if (fmt->bit_clk_provider | fmt->frame_clk_provider) {
> >  	
> >  		dev_err(hdmi->dev, "unsupported clock settings\n");

Regards,
Detlev.

[PATCH v2 2/3] ASoC: hdac_hdmi: Use dev_info on invalid ELD version

[Detlev Casanova]

When disconnected, the ELD data cannot be read by the display driver, so
it just sets the data to 0.

That makes the ELD parsing code read an ELD version of 0, which is
invalid. In hdac_hdmi, that is logged with dev_err(), but should be
logged with dev_info() instead as it is done in sound/core/pcm_drm_eld.c

This avoids printing multiple messages like:

    HDMI: Unknown ELD version 0

in the kernel log when userspace tries to open the sound device.

Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 sound/soc/codecs/hdac_hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 1139a2754ca33..4cc3b7a1062bd 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -1232,7 +1232,7 @@ static int hdac_hdmi_parse_eld(struct hdac_device *hdev,
 						>> DRM_ELD_VER_SHIFT;
 
 	if (ver != ELD_VER_CEA_861D && ver != ELD_VER_PARTIAL) {
-		dev_err(&hdev->dev, "HDMI: Unknown ELD version %d\n", ver);
+		dev_info(&hdev->dev, "HDMI: Unknown ELD version %d\n", ver);
 		return -EINVAL;
 	}
 
-- 
2.50.1

Re: [PATCH v2 2/3] ASoC: hdac_hdmi: Use dev_info on invalid ELD version

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 901 bytes --]

On Tue, Jul 22, 2025 at 03:54:36PM -0400, Detlev Casanova wrote:
> When disconnected, the ELD data cannot be read by the display driver, so
> it just sets the data to 0.

Please don't put patches for different subsystems into the same series
if there's no dependencies, it just makes dependencies less obvious and
creates hassle merging things.

> That makes the ELD parsing code read an ELD version of 0, which is
> invalid. In hdac_hdmi, that is logged with dev_err(), but should be
> logged with dev_info() instead as it is done in sound/core/pcm_drm_eld.c
> 
> This avoids printing multiple messages like:
> 
>     HDMI: Unknown ELD version 0
> 
> in the kernel log when userspace tries to open the sound device.

It doesn't, it just lowers the severity of the logs that are printed.
If the goal is to lower the number of messages printed you need to use
a ratelimited print.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/3] ASoC: hdac_hdmi: Use dev_info on invalid ELD version

[Detlev Casanova]

Hi Mark,
On Wednesday, 23 July 2025 06:40:13 EDT Mark Brown wrote:
> On Tue, Jul 22, 2025 at 03:54:36PM -0400, Detlev Casanova wrote:
> > When disconnected, the ELD data cannot be read by the display driver, so
> > it just sets the data to 0.
> 
> Please don't put patches for different subsystems into the same series
> if there's no dependencies, it just makes dependencies less obvious and
> creates hassle merging things.

Yes, sorry, I'll send v3 of this patch separately.

> > That makes the ELD parsing code read an ELD version of 0, which is
> > invalid. In hdac_hdmi, that is logged with dev_err(), but should be
> > logged with dev_info() instead as it is done in sound/core/pcm_drm_eld.c
> > 
> > This avoids printing multiple messages like:
> >     HDMI: Unknown ELD version 0
> > 
> > in the kernel log when userspace tries to open the sound device.
> 
> It doesn't, it just lowers the severity of the logs that are printed.
> If the goal is to lower the number of messages printed you need to use
> a ratelimited print.

I see, ratelimited would be good, but it still prints a message about 
something that is normal behaviour. Maybe this should go further to a 
dev_dbg(), or is there a specific reason to show this message ?

This could also be a special case:
 - version == 0 -> dev_dbg()
 - version !=0 && != known_versions -> dev_err()

Detlev.

[PATCH v2 3/3] drm/bridge: synopsys: Do not warn about audio params computation

[Detlev Casanova]

There is no need to warn about non pre-computed values, just change it to
dbg.

Fixes: fd0141d1a8a2 ("drm/bridge: synopsys: Add audio support for dw-hdmi-qp")
Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index 9b9d43c02e3a5..d974bcad8f94a 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -276,8 +276,7 @@ static unsigned int dw_hdmi_qp_find_n(struct dw_hdmi_qp *hdmi, unsigned long pix
 	if (n > 0)
 		return n;
 
-	dev_warn(hdmi->dev, "Rate %lu missing; compute N dynamically\n",
-		 pixel_clk);
+	dev_dbg(hdmi->dev, "Rate %lu missing; compute N dynamically\n", pixel_clk);
 
 	return dw_hdmi_qp_compute_n(hdmi, pixel_clk, sample_rate);
 }
-- 
2.50.1