hda: how to implement component master_unbind?
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.
Br, Kai
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:
- 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.
- 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.
- 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
Hi,
On Tue, 28 Sep 2021, Takashi Iwai wrote:
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.
thanks for the feedback! The switcheroo approach doesn't work too well for integrated HDA controllers that will typically have other codecs connected as well (and may have a DSP -> e.g. SOF/SST used as controller driver which would all need to implement this separately), but for the discrete GPU case this might be a workable approach (and makes sense as this is what switcheroo is meant for). I think gracefully handling the unbind is priority, but re-binding should be possible as well. We could do a switcheroo-enable type of flow in hdac_component_master_bind() and have the controller back up.
Of course it's not perfect still. I'd guess at least attempt to reach the codec#x procfs entry would hit timeouts if the controller is disabled this way.
Br, Kai
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:
- 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.
- 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
- 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
Hi,
On Fri, 13 Dec 2024, Lucas De Marchi wrote:
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
ack, given fresh wave of bugs this year (even if these are all (?) bugs triggered in test configurations), probably warrants another look.
[1) prevent/block the unbind if audio device is busy]
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
Ack, I think we can put this option to rest.
[unbind the ALSA card]
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
[...]
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.
I don't think much has changed. I think this is (still) a doable option, but just requires effort put in (which so far has not happened). I think the problem can be roughly divided in two categories:
1) discrete graphics - controller on discrete PCI device
Here the problem is more acute (as may affect register accesses done from audio driver), but potentially also easier to fix. There is a single driver (snd-hda-intel) to handle this case in ALSA and the sound card created only exposes the HDMI/DP PCMs, so if graphics side unbinds, disconnecting the ALSA card seems ok as well. The open I have is how do we recreate the ALSA card. Audio driver probe is triggered by attach of the PCI device, but here the PCI device is not unplugged, just unbound by one of the drivers. But doesn't sound like a unique problem to audio, so I'm sure there's some example solution to follow.
2) integrated display codecs
The regular laptop case is a bit more iffy as there are more audio drivers using the display codec driver stack (each need support), and in most cases, the ALSA card is a mix of HDMI/DP but also internal codec and speaker PCMs. If we use the card disconnect infra in ALSA (i.e. same infra as USB hotplug), this means display unbind will not just disconnect the HDMI/DP PCM device, but alsl the interal audio codec and speakers. And there's a bigger issue how we reenumerate the full card again.
I think the longterm solution is to move the HDMI/DP PCMs to their own ALSA card. We in fact some work going on to this direction in the SOF driver, but it's far from complete, and we are not sure whether we can change the existing platforms to use this approach (as changing the card topologies will be visible to user-space as well and potentially break stuff).
I did file a bug to track this in SOF https://github.com/thesofproject/linux/issues/5276 .
Br, Kai
On Thu, Dec 19, 2024 at 04:00:08PM +0200, Kai Vehmanen wrote:
Hi,
On Fri, 13 Dec 2024, Lucas De Marchi wrote:
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
ack, given fresh wave of bugs this year (even if these are all (?) bugs triggered in test configurations), probably warrants another look.
[1) prevent/block the unbind if audio device is busy]
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
Ack, I think we can put this option to rest.
[unbind the ALSA card]
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
[...]
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.
I don't think much has changed. I think this is (still) a doable option, but just requires effort put in (which so far has not happened). I think the problem can be roughly divided in two categories:
- discrete graphics - controller on discrete PCI device
Here the problem is more acute (as may affect register accesses done from audio driver), but potentially also easier to fix. There is a single driver (snd-hda-intel) to handle this case in ALSA and the sound card created only exposes the HDMI/DP PCMs, so if graphics side unbinds, disconnecting the ALSA card seems ok as well. The open I have is how do we recreate the ALSA card. Audio driver probe is triggered by attach of the PCI device, but here the PCI device is not unplugged, just unbound by one of the drivers. But doesn't sound like a unique problem to audio, so I'm sure there's some example solution to follow.
since we are using include/linux/component.h, i915-display will call component_del() on unbind, which should trigger all components to be unbound - snd should release all the references it holds to display power.
once we add back the i915/xe side with component_add() it should complete the aggregate driver and bind again all sides.
Lucas De Marchi
- integrated display codecs
The regular laptop case is a bit more iffy as there are more audio drivers using the display codec driver stack (each need support), and in most cases, the ALSA card is a mix of HDMI/DP but also internal codec and speaker PCMs. If we use the card disconnect infra in ALSA (i.e. same infra as USB hotplug), this means display unbind will not just disconnect the HDMI/DP PCM device, but alsl the interal audio codec and speakers. And there's a bigger issue how we reenumerate the full card again.
I think the longterm solution is to move the HDMI/DP PCMs to their own ALSA card. We in fact some work going on to this direction in the SOF driver, but it's far from complete, and we are not sure whether we can change the existing platforms to use this approach (as changing the card topologies will be visible to user-space as well and potentially break stuff).
I did file a bug to track this in SOF https://github.com/thesofproject/linux/issues/5276 .
Br, Kai
participants (3)
-
Kai Vehmanen -
Lucas De Marchi -
Takashi Iwai