hda: how to implement component master_unbind?

[Lucas De Marchi]

Thanks Kai to pointing me to this thread, trying to revive it.
Also adding dri-devel as it may be relevant there and Maarten who worked
on the xe integration recently

On Tue, Sep 28, 2021 at 01:07:34PM +0200, Takashi Iwai wrote:
>On Wed, 22 Sep 2021 14:40:19 +0200,
>Kai Vehmanen wrote:
>>
>> Hi Takashi et al,
>>
>> I've been having multiple discussions with our i915 team w.r.t. audio
>> driver behaviour when the aggregate component is unbound, triggered by
>> i915 unbind. This came up again in the recent regression with devres
>> allocations and I now dug into the topic again.
>>
>> In short, I'm puzzled how to really implement this. ALSA (or ASoC) don't
>> really have support for individual components of a card disappearing in a
>> hotplug fashion. At card level, we do have such support (USB, firewire and
>> recent work for PCI hotplug). But not individual components of a card
>> getting unplugged.


I think we will need to add some support here and handle the component
going down the same as a pci hotplug.


>>
>> In current code we have this:
>> static void hdac_component_master_unbind(struct device *dev)
>> {
>> »       struct drm_audio_component *acomp = hdac_get_acomp(dev);
>>
>> »       if (acomp->audio_ops && acomp->audio_ops->master_unbind)
>> »       »       acomp->audio_ops->master_unbind(dev, acomp);
>> »       module_put(acomp->ops->owner);
>> »       component_unbind_all(dev, acomp);
>> »       WARN_ON(acomp->ops || acomp->dev);
>> }
>>
>> ... when e.g. i915 driver is unbound (but could be any driver using the
>> component framework, not jus Intel hw), i915 calls component_del() and HDA
>> gets call to the above. After this, acomp ops are null are audio no longer
>> has ability to talk to i915 driver (which makes sense given it's unbound).
>>
>> If audio was runtime suspended, this kind of works (but relies on some
>> good luck). Upon HDA controller resume, we note acomp ops are NULL and
>> HDMI/DP operations (like snd_hdac_display_power()) silently become no-ops.
>> PCM streaming will go to /dev/null, but this is ok'ish since no monitor
>> can be connected anyways.
>>
>> If audio was active, we start to get warnings or worse. Audio expects hw
>> display codec to be powered and programmed, but suddenly it mey lose
>> state. If the audio controller is actually part of the display hardware
>> (e.g. discrete GPUs), then controller registers can stop functioning (they
>> should be still available, but given the main diplay driver is unbound,
>> odds of suprising behaviour are high).
>>
>> So this is less than ideal. I've been looking at options:
>>
>> 1) prevent/block the unbind if audio device is busy
>>
>> The component framework does not allow individual components to return
>> -EBUSY, so there's no way to let others know that unbind is not possible
>> now.

and there's no way to block unbind from the pci level neither, so this
is not really possible. There's nothing blocking someone to unplug the
card if it's on a hotplug-capable bus and/or someone calling

	# echo 0000:00:02.0 > /sys/module/xe/drivers/pci:xe/unbind

to tell the module to unbind from the device. If that involves multiple
components, I think they all should treat that as a device hotplug
rather than handling it differently.

>>
>> This would force anyone testing DRM driver unbind to first unbind
>> the audio driver and stop any active audio use-cases if needed.
>>
>> 2) unbind the ALSA card
>>
>> I've experimented doing a device_unregister() from the above callback, but
>> this has not really worked (onion peeling exercise of new probelms). The
>> code is shared by multiple controllers, so getting a handle to an snd_card
>> object is not straighforward (drvdata is differnet for SOF and
>> snd-hda-intel for instance). But with some new work, maybe we could call
>> snd_card_disconnect() to detach the card and inform user-space.

yeah, since it depends on the i915/xe side to power up the display
engine, I think handling that esssentially the same as a hotplug would
be ideal


>>
>> 3) continue as-is and try to fix bugs
>>
>> If audio is active, maybe we could force a acomp->put_power() to ensure
>> a clean unregister of the display part. But this leaves a big chunk of
>> issues especially with HDA controllers that require the display to
>> be powered on, to function.
>>
>> ..
>>
>> Any ideas, and/or has there been prior work? It seems Takashi it's been
>> mostly you who has been active working on the acomp integration recently.
>> I also included Chris, Daniel and Jani who've had patches to
>> hdac_component.c.
>
>Removing a component from the card is a PITA for now, indeed,
>especially when its influence is over different APIs (PCM, control,
>whatever)...

I'm not yet very familar with the sound side and checking if something changed
from when this thread started: for cards that can't work without the
other component, would it be hard to escalate that event to handle it
the same as a hotplug? Because from this thread it seems usb/pci hotplug
is already available.

thanks
Lucas De Marchi

>
>One thing I can think of is to perform like the vga_switcheroo
>handling in hda_intel.c; it's essentially a forced runtime suspend,
>and disable the whole card.  But in the case of audio-component
>unbind, we need to think about re-binding -- or completely ignore
>re-binding until the whole driver gets unloaded.
>
>
>Takashi