[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6176714.lOV4Wx5bFT@earth>
Date: Wed, 23 Jul 2025 11:22:05 -0400
From: Detlev Casanova <detlev.casanova@...labora.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: linux-kernel@...r.kernel.org, Andrzej Hajda <andrzej.hajda@...el.com>,
Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
Douglas Anderson <dianders@...omium.org>, Heiko Stuebner <heiko@...ech.de>,
Sugar Zhang <sugar.zhang@...k-chips.com>,
Cristian Ciocaltea <cristian.ciocaltea@...labora.com>,
Charles Keepax <ckeepax@...nsource.cirrus.com>,
Raag Jadav <raag.jadav@...el.com>, dri-devel@...ts.freedesktop.org,
linux-sound@...r.kernel.org, kernel@...labora.com
Subject:
Re: [PATCH v2 1/3] drm/bridge: dw-hdmi-qp: Return 0 in audio prepare when
disconnected
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@...labora.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.
Powered by blists - more mailing lists