hda: how to implement component master_unbind?

[Takashi Iwai]

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.
> 
> 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.
> 
> 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.
> 
> 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)...

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