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

[Yang, Libin]

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__ */
>>