drm_connector_hdmi_audio_ops implements .prepare and not .hw_params, so every bridge using the HDMI audio helper configures its sink's audio path from the codec DAI's prepare callback.
That is too late when the CPU side of the DAI link cannot start until the sink's audio clock is already running. ASoC's snd_soc_pcm_dai_prepare() walks CPU DAIs before codec DAIs and returns on the first error, so such a CPU DAI fails and aborts the loop before the bridge's prepare is reached -- the clock it was waiting for is enabled by a callback that the failure prevents from running. Qualcomm's LPASS is one such CPU side. On an sm8250 board with the DisplayPort controller bound to the DISPLAY_PORT_RX AFE port, starting playback gives: qcom-q6afe: AFE enable for port 0x6020 failed -110 q6afe-dai: ASoC error (-110): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0 and an ftrace of the attempt shows msm_dp_audio_prepare() is never called at all -- only msm_dp_audio_shutdown(), from the teardown path. Add a .hw_params to the helper that runs the same configuration, and a drm_bridge flag to select it. hw_params runs for every DAI in the link before any of them is prepared, so the clock is up by the time the CPU DAI starts its port. Both callbacks receive identical parameters, so the configuration itself is unchanged. The flag defaults to off: a bridge that does not set it keeps being configured from prepare exactly as before, and the new hw_params returns without touching it. Note that providing .hw_params at all means hdmi_codec_hw_params() no longer returns early for these connectors, so it now runs hdmi_codec_fill_codec_params() and the IEC958 channel status fill before the (inert) callback. Both are computed into an on-stack hdmi_codec_params that is then discarded, and the one shared value it writes, daifmt->bit_fmt, is set by hdmi_codec_prepare() to the same value immediately afterwards. One more side effect worth disclosing: hdmi_codec_fill_codec_params() also writes hcp->chmap_idx, which is not on-stack -- it persists on the hdmi-codec device and is reused by the next call to hdmi_codec_get_ch_alloc_table_idx(). hdmi_codec_prepare() recomputes it immediately afterwards from the same channel count, so the value itself does not go stale. But hdmi_codec_hw_params() now reaching this code at all means that a channel count the sink's ELD does not advertise -- the error path in hdmi_codec_get_ch_alloc_table_idx() -- is reported from hw_params() instead of prepare(), for every bridge sharing drm_connector_hdmi_audio_ops, not only the ones that set hdmi_audio_prepare_early. That changes when a bad channel count is reported, not whether it is, but it is a change to seven bridges this patch has no other reason to touch, and worth a maintainer's opinion. Signed-off-by: Jean-Francois Bobier <[email protected]> --- .../gpu/drm/display/drm_bridge_connector.c | 3 +++ .../gpu/drm/display/drm_hdmi_audio_helper.c | 23 +++++++++++++++++++ drivers/gpu/drm/msm/dp/dp_drm.c | 8 +++++++ include/drm/drm_bridge.h | 17 ++++++++++++++ include/drm/drm_connector.h | 8 +++++++ 5 files changed, 59 insertions(+) diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c index 8b54069fa..51293181b 100644 --- a/drivers/gpu/drm/display/drm_bridge_connector.c +++ b/drivers/gpu/drm/display/drm_bridge_connector.c @@ -1048,6 +1048,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm, bridge->hdmi_audio_dai_port); if (ret) return ERR_PTR(ret); + + connector->hdmi_audio.prepare_early = + bridge->hdmi_audio_prepare_early; } if (bridge_connector->bridge_hdmi_cec && diff --git a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c index 7d78b02c1..3ed5d73f1 100644 --- a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c +++ b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c @@ -34,6 +34,28 @@ static int drm_connector_hdmi_audio_prepare(struct device *dev, void *data, const struct drm_connector_hdmi_audio_funcs *funcs = connector->hdmi_audio.funcs; + if (connector->hdmi_audio.prepare_early) + return 0; + + return funcs->prepare(connector, fmt, hparms); +} + +/* + * Only used by connectors that asked for it; everyone else keeps being + * configured from prepare() and this returns without doing anything. See + * &drm_bridge.hdmi_audio_prepare_early for why a connector would want it. + */ +static int drm_connector_hdmi_audio_hw_params(struct device *dev, void *data, + struct hdmi_codec_daifmt *fmt, + struct hdmi_codec_params *hparms) +{ + struct drm_connector *connector = data; + const struct drm_connector_hdmi_audio_funcs *funcs = + connector->hdmi_audio.funcs; + + if (!connector->hdmi_audio.prepare_early) + return 0; + return funcs->prepare(connector, fmt, hparms); } @@ -130,6 +152,7 @@ EXPORT_SYMBOL(drm_connector_hdmi_audio_plugged_notify); static const struct hdmi_codec_ops drm_connector_hdmi_audio_ops = { .audio_startup = drm_connector_hdmi_audio_startup, + .hw_params = drm_connector_hdmi_audio_hw_params, .prepare = drm_connector_hdmi_audio_prepare, .audio_shutdown = drm_connector_hdmi_audio_shutdown, .mute_stream = drm_connector_hdmi_audio_mute_stream, diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c index c1e4147bf..52c90937f 100644 --- a/drivers/gpu/drm/msm/dp/dp_drm.c +++ b/drivers/gpu/drm/msm/dp/dp_drm.c @@ -327,6 +327,14 @@ int msm_dp_bridge_init(struct msm_dp *msm_dp_display, struct drm_device *dev, bridge->hdmi_audio_dev = &msm_dp_display->pdev->dev; bridge->hdmi_audio_max_i2s_playback_channels = 8; bridge->hdmi_audio_dai_port = -1; + /* + * The LPASS/ADSP side of the DAI link will not start its AFE + * port until this controller's audio clock is running, and + * ASoC prepares CPU DAIs before codec DAIs, so configuring + * from prepare() is too late -- the CPU DAI times out and + * aborts the link before we are called at all. + */ + bridge->hdmi_audio_prepare_early = 1; } rc = devm_drm_bridge_add(dev->dev, bridge); diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 58fff047f..e8fb944f3 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -1128,6 +1128,23 @@ struct drm_bridge { */ unsigned int hdmi_audio_spdif_playback : 1; + /** + * @hdmi_audio_prepare_early: configure the sink's audio path from the + * ASoC hw_params callback rather than from prepare. + * + * Both callbacks receive identical parameters, so for most bridges it + * makes no difference and the default (prepare) is right. It matters + * when the CPU side of the DAI link cannot start until the sink's + * audio clock is already running: ASoC walks CPU DAIs before codec + * DAIs and stops at the first error, so a CPU DAI that fails waiting + * for that clock aborts the loop before this bridge's prepare is ever + * reached. Qualcomm's LPASS/ADSP is one such CPU side. + * + * Setting this moves the configuration to hw_params, which runs for + * every DAI in the link before any of them is prepared. + */ + unsigned int hdmi_audio_prepare_early : 1; + /** * @hdmi_audio_dai_port: sound DAI port for either of * @DRM_BRIDGE_OP_HDMI_AUDIO and @DRM_BRIDGE_OP_DP_AUDIO, -1 if it is diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index a0cf0268d..ae3d1382f 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -2027,6 +2027,14 @@ struct drm_connector_hdmi_audio { * The port in DT that is used for the Codec DAI. */ int dai_port; + + /** + * @prepare_early: + * + * Call @funcs->prepare from the ASoC hw_params callback instead of + * from prepare. See &drm_bridge.hdmi_audio_prepare_early. + */ + bool prepare_early; }; /* -- 2.55.0
