[alsa-devel] [PATCH v5 2/9] ASoC: hdac_hda: add support for HDMI/DP as a HDA codec

[Takashi Iwai]

On Wed, 25 Sep 2019 13:38:50 +0200,
Kai Vehmanen wrote:
> 
> Hello,
> 
> Takashi, please check whether this is ok. I'll highlight the changed 
> section below:
> 
> On Wed, 25 Sep 2019, Kai Vehmanen wrote:
> 
> >  static int hdac_hda_codec_probe(struct snd_soc_component *component)
> >  {
> >  	struct hdac_hda_priv *hda_pvt =
> > @@ -322,6 +392,15 @@ static int hdac_hda_codec_probe(struct snd_soc_component *component)
> >  
> >  	snd_hdac_ext_bus_link_get(hdev->bus, hlink);
> >  
> > +	/*
> > +	 * Ensure any HDA display is powered at codec probe.
> > +	 * After snd_hda_codec_device_new(), display power is
> > +	 * managed by runtime PM.
> > +	 */
> > +	if (hda_pvt->need_display_power)
> > +		snd_hdac_display_power(hdev->bus, HDA_CODEC_IDX_CONTROLLER,
> > +				       true);
> > +
> 
> This is the new bit (and matching logic in patch 5).The bug required a 
> very specific timing sequence to trigger, but a clear bug nevertheless. I 
> tried to fix it in spirit of your refactoring patch of to this area 
> 029d92c289bd, "ALSA: hda: Refactor display power management". I.e. just 
> like snd_hda_intel's controller code, display power is enabled before 
> probe and later managed by codec using common code.
> 
> Additional SOF specific twist is that I need to pass the 
> "need_display_power" info from SOF code (where the initial codec probe is 
> done and we detect a HDMI HDA codec)), to soc/codecs/hdac_hda.c where the 
> actual driver probe is run for the codec. In snd_hda_intel this is all in 
> one place, so somewhat more straighforward, but logic is the same.

This change looks OK to me.  The recent display power management is no
longer refcounted, so it's fine to toggle power on at the early stage
like this patch does.


thanks,

Takashi