Re: y4T�TCH 4/4] drm/amdgpu: Add audio component support

[jimqu]

On 2018年07月25日 16:20, Takashi Iwai wrote:
> On Wed, 25 Jul 2018 10:02:34 +0200,
> jimqu wrote:
>>
>>
>> On 2018年07月25日 13:46, Takashi Iwai wrote:
>>> On Wed, 25 Jul 2018 07:38:37 +0200,
>>> Qu, Jim wrote:
>>>> Jim: Just like Alex said, we want driver can get eld info when hotplug in new device. amdgpu driver is a bit difference from radeon driver, it is not a suitable place to call notify() function in *_audio_enable() , since they are not in the hotplug process context like them in radeon driver, but the mode setting.
>>>>
>>>> IMO, the right place to call notify() function is also in the amdgpu_connector_xxxx_detect() in amdgpu_connector.c.
>>> Hm, but by the modesetting, it actually enables / disables the audio
>>> as well, no?  If so, the notifier is exactly for that purpose.  The
>>> audio driver needs to know not only about the physical connection but
>>> whether the audio can be actually driven.
>>>
>>> That is, even if the monitor is connected, the audio won't come out if
>>> the mode is off.  That is equivalent with the unplugged state for the
>>> audio driver.
>>>
>>> The i915 driver deals with the notifier just like the above, so the
>>> behavior is intentional.
>>>
>>>
>>> thanks,
>>>
>>> Takashi
>> I am afraid if device hotplug out, how is audio state if it follow up
>> eld info? Since the modesetting is never performed for the display
>> which is plugged out, so there is no notify() call on it.
> In principle, the HDMI audio just needs to follows the video state,
> and it doesn't need to care actual physical connections.  As long as
> video can go out, it's fine, audio can, too.  When video is disabled
> (even if connected), audio can't be used as well, so it must follow to
> off, too.  The notifier is used to follow this video state change.
>
> Practically seen, the user-space shall switch off the video
> accordingly upon hot unplug, then the audio notifier is sent, and the
> audio gets off, too.
>
>
> thanks,
>
> Takashi

Ok, that means there are other code pathes to update audio state. 
Anyway, for patch 3/4 Acked-by: Jim Qu <Jim.Qu at amd.com>
There may be other concerns from Alex/Christian.

Thanks
JimQu