On Fri, Dec 15, 2023 at 12:40 AM Jerome Brunet <jbrunet@xxxxxxxxxxxx> wrote: > > > On Fri 15 Dec 2023 at 12:51, Zhengqiao Xia <xiazhengqiao@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote: > > > Hi Jerome, > > > > After my testing, I found that this patch will cause the audio on the external display to not work properly after > > restart. > > You move the plugged_cb to run in hdmi_probe, at this time hcp- > jack = NULL, the driver cannot report `SND_JACK_LINEOUT > > ` normally. > > static void hdmi_codec_jack_report(struct hdmi_codec_priv *hcp, > > unsigned int jack_status) > > { > > printk("xzq-866 hdmi_codec_jack_report: jack=%x, jack_status=%d", hcp->jack, jack_status != hcp->jack_status); > > if (hcp->jack && jack_status != hcp->jack_status) { > > snd_soc_jack_report(hcp->jack, jack_status, SND_JACK_LINEOUT); > > hcp->jack_status = jack_status; > > } > > } > > So we must call plugged_cb in hdmi_codec_set_jack, Can you make some changes? > > Hi Zhengqiao, > > That is unfortunate. Sorry. > > This patch has changed when the hpd callback is registered, no when it > comes in effect. This is still dependent on calling .set_jack() and it > is not happening any later than it was before. So, in theory, it should > not have changed anything, if your driver actually relies on the HPD > event. > > Trying to guess what is happening for you, I suppose your HDMI driver is > "faking" an HPD event to report the initial jack status when the > hook_plugged_cb() is called. Could you point me to the hdmi driver you > are using so I can have a look ? > > My reference when testing this was dw-hdmi-i2s-audio and it does not do > that, it just registers the callback. I think this is what it supposed > to do TBH. > > An idea I have been thinking about for a while is have the hdmi-codec > insert the jack in the card itself, instead of the card doing. That > would give the jack "for free" to any user of the HDMI codec and might > also solve your issue. It would require a small rework of the cards doing > the hdmi jack register, but there are not many of these AFAIK. > The driver is it6505. The implementation of hook_plugged_cb(): 1. register plugged_cb 2. call plugged_cb(bool plugged) bridge detect callback it6505_detect would also call plugged_cb, but only on the first time hpd status changed (eg. changed from connect <--> disconnect) it6505_detect() { status = it6505->hpd_state ... ... if (it6505->connector_status != status) { it6505->connector_status = status; it6505_plugged_status_to_codec(it6505); // this will call plugged_cb } } Unfortunately the first time after boot that hpd status changed was detected before set_jack. If we replug hdmi, the plugged_cb() was called by bridge_detect, which is expected. Prior to this patch, the initial plugged_cb() was called by hook_plugged_cb(). After the patch, plugged_cb() should be called by hpd change (by bridge detect), but due to the driver logic only calling it on the first hpd state change, it fails to call plugged_cb() again when jack is set. I checked the dw-hdmi.c's bridge_detect, and it's similar in that it also checks the last_connector_result, so maybe it's due to a timing difference? > > > > On Mon, Nov 6, 2023 at 6:40 PM Jerome Brunet <jbrunet@xxxxxxxxxxxx> wrote: > > > > The HDMI hotplug callback to the hdmi-codec is currently registered when > > jack is set. > > > > The hotplug not only serves to report the ASoC jack state but also to get > > the ELD. It should be registered when the component probes instead, so it > > does not depend on the card driver registering a jack for the HDMI to > > properly report the ELD. > > > > Fixes: 25ce4f2b3593 ("ASoC: hdmi-codec: Get ELD in before reporting plugged event") > > Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx> > > --- > > sound/soc/codecs/hdmi-codec.c | 27 +++++++++++++++++++-------- > > 1 file changed, 19 insertions(+), 8 deletions(-) > > > > diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c > > index 09eef6042aad..20da1eaa4f1c 100644 > > --- a/sound/soc/codecs/hdmi-codec.c > > +++ b/sound/soc/codecs/hdmi-codec.c > > @@ -877,18 +877,13 @@ static int hdmi_codec_set_jack(struct snd_soc_component *component, > > void *data) > > { > > struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component); > > - int ret = -ENOTSUPP; > > > > if (hcp->hcd.ops->hook_plugged_cb) { > > hcp->jack = jack; > > - ret = hcp->hcd.ops->hook_plugged_cb(component->dev->parent, > > - hcp->hcd.data, > > - plugged_cb, > > - component->dev); > > - if (ret) > > - hcp->jack = NULL; > > + return 0; > > } > > - return ret; > > + > > + return -ENOTSUPP; > > } > > > > static int hdmi_dai_spdif_probe(struct snd_soc_dai *dai) > > @@ -982,6 +977,21 @@ static int hdmi_of_xlate_dai_id(struct snd_soc_component *component, > > return ret; > > } > > > > +static int hdmi_probe(struct snd_soc_component *component) > > +{ > > + struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component); > > + int ret = 0; > > + > > + if (hcp->hcd.ops->hook_plugged_cb) { > > + ret = hcp->hcd.ops->hook_plugged_cb(component->dev->parent, > > + hcp->hcd.data, > > + plugged_cb, > > + component->dev); > > + } > > + > > + return ret; > > +} > > + > > static void hdmi_remove(struct snd_soc_component *component) > > { > > struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component); > > @@ -992,6 +1002,7 @@ static void hdmi_remove(struct snd_soc_component *component) > > } > > > > static const struct snd_soc_component_driver hdmi_driver = { > > + .probe = hdmi_probe, > > .remove = hdmi_remove, > > .dapm_widgets = hdmi_widgets, > > .num_dapm_widgets = ARRAY_SIZE(hdmi_widgets), > > > -- > Jerome