Re: 答复: 答复: [alsa-devel] 答复: [PATCH] vgaswitchroo: set audio client id according to bound gpu client id

[Alex Deucher]

On Mon, Jul 16, 2018 at 11:25 AM, Takashi Iwai <tiwai at suse.de> wrote:
> On Mon, 16 Jul 2018 17:10:43 +0200,
> Harry Wentland wrote:
>>
>>
>>
>> On 2018-07-15 10:36 AM, Alex Deucher wrote:
>> > On Sat, Jul 14, 2018 at 12:31 PM, Takashi Iwai <tiwai at suse.de> wrote:
>> >> On Sat, 14 Jul 2018 14:03:26 +0200,
>> >> jimqu wrote:
>> >>>
>> >>>
>> >>>
>> >>> 在 2018/7/13 23:07, Takashi Iwai 写道:
>> >>>> On Wed, 11 Jul 2018 13:12:01 +0200,
>> >>>> Takashi Iwai wrote:
>> >>>>> And the forced runtime PM is still an issue, and this would need the
>> >>>>> other notification mechanism than the HD-audio unsolicited event as
>> >>>>> AMD HDMI controller doesn't honor the HD-audio WAKEEN bit.
>> >>>> Since we had a nice "hack week" in this week at SUSE, I spent some
>> >>>> time to write some patches for the support of the direct hotplug
>> >>>> notification / ELD query between HD-audio and radeon/amdgpu.  It
>> >>>> re-utilizes the audio component framework for i915 but in a slightly
>> >>>> more flexible way.
>> >>>>
>> >>>> The patches are found in topic/hda-acomp branch of my sound.git tree:
>> >>>>    git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
>> >>>>
>> >>>> The following commits are relevant:
>> >>>>    drm/i915: Split audio component to a generic type
>> >>>>    ALSA: hda/i915: Allow delayed i915 audio component binding
>> >>>>    ALSA: hda/i915: Associate audio component with devres
>> >>>>    ALSA: hda: Make audio component support more generic
>> >>>>    ALSA: hda/hdmi: Allow audio component for AMD/ATI HDMI
>> >>>>    ALSA: hda/hdmi: Use single mutex unlock in error paths
>> >>>>    drm/radeon: Add audio component support
>> >>>>    drm/amdgpu: Add audio component support
>> >>>>
>> >>>> The branch should be cleanly pullable onto the latest 4.18-rc.
>> >>>>
>> >>>> I couldn't test amdgpu but the test with a radeon driver on an old
>> >>>> laptop seemed working through a very quick test.
>> >>>>
>> >>>> Please give it a try.
>> >>>
>> >>> That is really wonderful work. I will check it on our AMD
>> >>> platform.
>> >>
>> >> Thanks, it'll be great if you can check whether the current code works
>> >> or not.  I'd love to push the stuff for 4.19.  Hopefully I'll start
>> >> submitting the preparation patches in the next week.
>> >>
>> >> Basically this also opens the door of the similar capability for
>> >> nouveau, and I guess it's also trivial enough.
>> >>
>> >>> BTW, For display, AMD has moved to use DC to support new
>> >>> asics. so there also need a patch for amdgpu in DC code.
>> >>
>> >> Could you give a more hint?  I'll try adapt the code if such a change
>> >> is already in upstream tree.
>> >
>> > The new code is in drivers/gpu/drm/amd/display.
>> >
>>
>> In particular, I imagine all you need should be in display/amdgpu_dm, although there's chance you might have to touch display/dc/dce/dce_audio.c if you have to do anything with the unsolicited event.
>
> Thanks, I'm currently struggling to read down the whole complex DC
> code tree, and it's a very helpful hint.
>
> How is the audio enable/disable call associated with the pipe index?
> IOW, if I add a hook to call a notifier callback to pass the pipe
> index that is enabled/disabled, how can it be deduced?
>
> Similarly, when DC driver gets the query about the ELD on the given
> pipe, where can I convert it to which object?
>
> DC is quite another beast, so I still haven't figured out the
> structures yet...
>
>
>> I see you're using the GPL license, rather than MIT in amdgpu_audio.c. Since the code in display/dc is shared with other OSes and internal test frameworks it has to be MIT. Not entirely sure about display/amdgpu_dm, but if GPL is fine in amd/amdgpu it's probably fine there as well.
>
> Oh I just didn't know that amdgpu code was with MIT.  Indeed the
> driver module is provided via "GPL and additional rights".
>
> I'm fine to change the license to follow other code bits there.  What
> exactly SPDX tag should be put there?

Preferably:
SPDX-License-Identifier: MIT
or I suppose dual licensing (MIT or GPL 2.0) is ok too.

Alex

>
>
> thanks,
>
> Takashi