[alsa-devel] [PATCH V2] ASoC: hdac_hdmi: add device PM dependency

[Yang, Libin]

Hi Pierre,

Thanks for pointing out my patch's defect. I think we should call 
xxx_del() instread of xxx_free().

Hi all,

I found it's a little confusing when releasing the link. I will send out
a RFC patch based on Pierre's suggestion. Please kindly help review the
logic of releasing the link.

Regards,
Libin


>-----Original Message-----
>From: Yang, Libin
>Sent: Monday, April 1, 2019 9:44 PM
>To: 'Pierre-Louis Bossart' <pierre-louis.bossart at linux.intel.com>; alsa-
>devel at alsa-project.org; tiwai at suse.de; broonie at kernel.org
>Subject: RE: [alsa-devel] [PATCH V2] ASoC: hdac_hdmi: add device PM
>dependency
>
>Hi Pierre,
>
>>-----Original Message-----
>>From: Pierre-Louis Bossart
>>[mailto:pierre-louis.bossart at linux.intel.com]
>>Sent: Monday, April 1, 2019 8:34 PM
>>To: Yang, Libin <libin.yang at intel.com>; alsa-devel at alsa-project.org;
>>tiwai at suse.de; broonie at kernel.org
>>Subject: Re: [alsa-devel] [PATCH V2] ASoC: hdac_hdmi: add device PM
>>dependency
>>
>>On 4/1/19 12:50 AM, libin.yang at intel.com wrote:
>>> From: Libin Yang <libin.yang at intel.com>
>>>
>>> HDMI audio codec register operation depends on the display audio
>>> power domain. The hdmi audio codec dapm event callback must be called
>>> after display audio power is turned on.
>>>
>>> Add hdac_hdmi_device_link_add() in HDMI audio codec driver. The
>>> customer audio driver, such as cAVS/SST and SOF audio driver, can
>>> call the function to set the audio power dependency.
>>>
>>> Signed-off-by: Libin Yang <libin.yang at intel.com>
>>> ---
>>>   sound/soc/codecs/hdac_hdmi.c | 12 ++++++++++++
>>>   sound/soc/codecs/hdac_hdmi.h |  6 ++++++
>>>   2 files changed, 18 insertions(+)
>>>
>>> diff --git a/sound/soc/codecs/hdac_hdmi.c
>>> b/sound/soc/codecs/hdac_hdmi.c index 5eeb0fe..c991407 100644
>>> --- a/sound/soc/codecs/hdac_hdmi.c
>>> +++ b/sound/soc/codecs/hdac_hdmi.c
>>> @@ -1792,6 +1792,18 @@ int hdac_hdmi_jack_init(struct snd_soc_dai
>>> *dai,
>>int device,
>>>   }
>>>   EXPORT_SYMBOL_GPL(hdac_hdmi_jack_init);
>>>
>>> +struct device_link *
>>> +hdac_hdmi_device_link_add(struct device *consumer,
>>> +			  struct snd_soc_component *component,
>>> +			  u32 flags)
>>> +{
>>> +	struct hdac_hdmi_priv *hdmi =
>>snd_soc_component_get_drvdata(component);
>>> +	struct hdac_device *hdev = hdmi->hdev;
>>> +
>>> +	return device_link_add(consumer, &hdev->dev, flags);
>>
>>Don't you need a matching device_link_free() to avoid a memory leak?
>
>Right. Sorry for my poor patch and thanks for pointing it out.
>I will refine it and test tomorrow.
>
>>
>>Also what would be the expectations for the flags and do you really
>>want the consumer to set them?
>
>A consumer setting the flag will be more flexible. For the flag, I'm currently
>thinking to use DL_FLAG_RPM_ACTIVE. Not sure if it is a good flag.
>
>>
>>And last, without a patch of the cAVS driver it's hard to see how this
>>might be used.
>
>I tried on SOF, it worked. I will check if I have a cAVS environment and will
>have a test on cAVS.
>
>Regards,
>Libin
>
>>
>>> +}
>>> +EXPORT_SYMBOL_GPL(hdac_hdmi_device_link_add);
>>> +
>>>   static void hdac_hdmi_present_sense_all_pins(struct hdac_device *hdev,
>>>   			struct hdac_hdmi_priv *hdmi, bool detect_pin_caps)
>>>   {
>>> diff --git a/sound/soc/codecs/hdac_hdmi.h
>>> b/sound/soc/codecs/hdac_hdmi.h index 4fa2fc9..9239c6b 100644
>>> --- a/sound/soc/codecs/hdac_hdmi.h
>>> +++ b/sound/soc/codecs/hdac_hdmi.h
>>> @@ -7,4 +7,10 @@ int hdac_hdmi_jack_init(struct snd_soc_dai *dai, int
>>> pcm,
>>>
>>>   int hdac_hdmi_jack_port_init(struct snd_soc_component *component,
>>>   			struct snd_soc_dapm_context *dapm);
>>> +
>>> +struct device_link *
>>> +hdac_hdmi_device_link_add(struct device *consumer,
>>> +			  struct snd_soc_component *component,
>>> +			  u32 flags);
>>> +
>>>   #endif /* __HDAC_HDMI_H__ */
>>>