* hda: how to implement component master_unbind?
@ 2021-09-22 12:40 Kai Vehmanen
2021-09-28 11:07 ` Takashi Iwai
0 siblings, 1 reply; 3+ messages in thread
From: Kai Vehmanen @ 2021-09-22 12:40 UTC (permalink / raw)
To: Takashi Iwai, alsa-devel, Chris Wilson, Daniel Vetter, Jani Nikula
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: hda: how to implement component master_unbind?
2021-09-22 12:40 hda: how to implement component master_unbind? Kai Vehmanen
@ 2021-09-28 11:07 ` Takashi Iwai
2021-10-01 9:46 ` Kai Vehmanen
0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2021-09-28 11:07 UTC (permalink / raw)
To: Kai Vehmanen; +Cc: Daniel Vetter, alsa-devel, Jani Nikula, Chris Wilson
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: hda: how to implement component master_unbind?
2021-09-28 11:07 ` Takashi Iwai
@ 2021-10-01 9:46 ` Kai Vehmanen
0 siblings, 0 replies; 3+ messages in thread
From: Kai Vehmanen @ 2021-10-01 9:46 UTC (permalink / raw)
To: Takashi Iwai
Cc: Daniel Vetter, alsa-devel, Jani Nikula, Kai Vehmanen, Chris Wilson
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-10-01 9:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 12:40 hda: how to implement component master_unbind? Kai Vehmanen
2021-09-28 11:07 ` Takashi Iwai
2021-10-01 9:46 ` Kai Vehmanen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).