mbox series

[v2,0/4] ASoC: qcom: display port changes

Message ID 20240422134354.89291-1-srinivas.kandagatla@linaro.org (mailing list archive)
Headers show
Series ASoC: qcom: display port changes | expand

Message

Srinivas Kandagatla April 22, 2024, 1:43 p.m. UTC
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patchset adds support for.
	1. parse Display Port module tokens from ASoC topology
	2. add support to DP/HDMI Jack events.
	3. fixes a typo in function name in sm8250

Verified these patches on X13s along with changes to tplg in 
https://git.codelinaro.org/linaro/qcomlt/audioreach-topology/-/tree/topic/x13s-dp?ref_type=heads
and ucm changes from https://github.com/Srinivas-Kandagatla/alsa-ucm-conf/tree/topic/x13s-dp

Thanks,
Srini

Changes since v1:
	- Fixed unused variable warning.
	- fixed warning 'break;' to avoid fall-through

Srinivas Kandagatla (4):
  ASoC: qcom: q6dsp: parse Display port tokens
  ASoC: qcom: common: add Display port Jack function
  ASoC: qcom: sc8280xp: add Display port Jack
  ASoC: qcom: sm8250: fix a typo in function name

 sound/soc/qcom/common.c         | 29 +++++++++++++++++++++++++++++
 sound/soc/qcom/common.h         |  3 +++
 sound/soc/qcom/qdsp6/topology.c | 26 ++++++++++++++++++++++++++
 sound/soc/qcom/sc8280xp.c       | 14 ++++++++++++++
 sound/soc/qcom/sm8250.c         |  4 ++--
 5 files changed, 74 insertions(+), 2 deletions(-)

Comments

Johan Hovold April 23, 2024, 11:59 a.m. UTC | #1
On Mon, Apr 22, 2024 at 02:43:50PM +0100, Srinivas Kandagatla wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> This patchset adds support for.
> 	1. parse Display Port module tokens from ASoC topology
> 	2. add support to DP/HDMI Jack events.
> 	3. fixes a typo in function name in sm8250
> 
> Verified these patches on X13s along with changes to tplg in 
> https://git.codelinaro.org/linaro/qcomlt/audioreach-topology/-/tree/topic/x13s-dp?ref_type=heads
> and ucm changes from https://github.com/Srinivas-Kandagatla/alsa-ucm-conf/tree/topic/x13s-dp

It looks like your UCM changes are still muxing the speaker and *each*
displayport output so that you can only use one device at a time (i.e.
only Speaker or DP1 or DP2 can be used).

As we discussed off list last week, this seems unnecessarily limited and
as far as I understood is mostly needed to work around some
implementation details (not sure why DP1 and DP2 can't be used in
parallel either).

Can you please describe the problem here so that we can discuss this
before merging an unnecessarily restricted solution which may later be
harder to change (e.g. as kernel, topology and ucm may again need to be
updated in lock step).

>From what I could tell after a quick look, this series does not
necessarily depend on muxing things this way, but please confirm that
too.

Johan
Srinivas Kandagatla April 23, 2024, 12:38 p.m. UTC | #2
On 23/04/2024 12:59, Johan Hovold wrote:
> On Mon, Apr 22, 2024 at 02:43:50PM +0100, Srinivas Kandagatla wrote:
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> This patchset adds support for.
>> 	1. parse Display Port module tokens from ASoC topology
>> 	2. add support to DP/HDMI Jack events.
>> 	3. fixes a typo in function name in sm8250
>>
>> Verified these patches on X13s along with changes to tplg in
>> https://git.codelinaro.org/linaro/qcomlt/audioreach-topology/-/tree/topic/x13s-dp?ref_type=heads
>> and ucm changes from https://github.com/Srinivas-Kandagatla/alsa-ucm-conf/tree/topic/x13s-dp
> 
> It looks like your UCM changes are still muxing the speaker and *each*
> displayport output so that you can only use one device at a time (i.e.
> only Speaker or DP1 or DP2 can be used).
that is true.

What is the use-case to use more than one audio sink devices at the same 
time for a laptops?

How do you test it? I never tested anything like that on a full desktop 
setup.

May be some manual setup in Wireplumber, but not 100% sure about 
multiple stream handling.

> 
> As we discussed off list last week, this seems unnecessarily limited and
> as far as I understood is mostly needed to work around some
> implementation details (not sure why DP1 and DP2 can't be used in
> parallel either).

It is absolutely possible to run all the streams in parallel from the 
Audio hardware and DSP point of view.

One thing to note is, On Qualcomm DP IP, we can not read/write registers 
if the DP port is not connected, which means that we can not send data 
in such cases.

This makes it challenging to work with sound-servers like pipewire or 
pulseaudio as they tend to send silence data at very early stages in the 
full system boot up, ignoring state of the Jack events.

> 
> Can you please describe the problem here so that we can discuss this
> before merging an unnecessarily restricted solution which may later be
> harder to change (e.g. as kernel, topology and ucm may again need to be
> updated in lock step).
> 
>  From what I could tell after a quick look, this series does not
> necessarily depend on muxing things this way, but please confirm that
> too.

These patches have nothing to do with how we model the muxing in UCM or 
in tplg.

so these can go as it is irrespective of how we want to model the DP 
sinks in the UCM or tplg.


--srini
> 
> Johan
Johan Hovold April 23, 2024, 2:58 p.m. UTC | #3
On Tue, Apr 23, 2024 at 01:38:18PM +0100, Srinivas Kandagatla wrote:
> On 23/04/2024 12:59, Johan Hovold wrote:

> > It looks like your UCM changes are still muxing the speaker and *each*
> > displayport output so that you can only use one device at a time (i.e.
> > only Speaker or DP1 or DP2 can be used).

> that is true.
> 
> What is the use-case to use more than one audio sink devices at the same 
> time for a laptops?

I can imagine streaming audio and video to a TV (or audio to a soundbar)
over DP while playing systems sounds and doing video conferencing using
the internal speakers (or the other DP port).

> How do you test it? I never tested anything like that on a full desktop 
> setup.

You can select the sink per application in pavucontrol. Just verified
that playing audio over the 3.5 mm jack while playing system sounds
using the internal speakers works just fine.

> > As we discussed off list last week, this seems unnecessarily limited and
> > as far as I understood is mostly needed to work around some
> > implementation details (not sure why DP1 and DP2 can't be used in
> > parallel either).
> 
> It is absolutely possible to run all the streams in parallel from the 
> Audio hardware and DSP point of view.
> 
> One thing to note is, On Qualcomm DP IP, we can not read/write registers 
> if the DP port is not connected, which means that we can not send data 
> in such cases.
> 
> This makes it challenging to work with sound-servers like pipewire or 
> pulseaudio as they tend to send silence data at very early stages in the 
> full system boot up, ignoring state of the Jack events.

This bit sounds like it can and should be worked around by the driver to
avoid hard-coding policy which would prevent use cases such as the ones
mentioned above.

> > Can you please describe the problem here so that we can discuss this
> > before merging an unnecessarily restricted solution which may later be
> > harder to change (e.g. as kernel, topology and ucm may again need to be
> > updated in lock step).
> > 
> >  From what I could tell after a quick look, this series does not
> > necessarily depend on muxing things this way, but please confirm that
> > too.
> 
> These patches have nothing to do with how we model the muxing in UCM or 
> in tplg.
> 
> so these can go as it is irrespective of how we want to model the DP 
> sinks in the UCM or tplg.

Thanks for confirming.

Johan
Srinivas Kandagatla April 23, 2024, 3:59 p.m. UTC | #4
On 23/04/2024 15:58, Johan Hovold wrote:
>> It is absolutely possible to run all the streams in parallel from the
>> Audio hardware and DSP point of view.
>>
>> One thing to note is, On Qualcomm DP IP, we can not read/write registers
>> if the DP port is not connected, which means that we can not send data
>> in such cases.
>>
>> This makes it challenging to work with sound-servers like pipewire or
>> pulseaudio as they tend to send silence data at very early stages in the
>> full system boot up, ignoring state of the Jack events.
> This bit sounds like it can and should be worked around by the driver to
> avoid hard-coding policy which would prevent use cases such as the ones
> mentioned above.
This is not simple as you say. We have to fit these into a proper DPCM.
Either we have a dummy Backend connected for each of these pcm 
sub-devices when DP port is not connected and then switch back to DP 
when its connected.

Or somehow find a way to not let the pipewire talk to devices which are 
not connected.


thanks,
srini
Johan Hovold April 29, 2024, 3 p.m. UTC | #5
On Tue, Apr 23, 2024 at 04:59:56PM +0100, Srinivas Kandagatla wrote:
> On 23/04/2024 15:58, Johan Hovold wrote:

> >> It is absolutely possible to run all the streams in parallel from the
> >> Audio hardware and DSP point of view.
> >>
> >> One thing to note is, On Qualcomm DP IP, we can not read/write registers
> >> if the DP port is not connected, which means that we can not send data
> >> in such cases.
> >>
> >> This makes it challenging to work with sound-servers like pipewire or
> >> pulseaudio as they tend to send silence data at very early stages in the
> >> full system boot up, ignoring state of the Jack events.

> > This bit sounds like it can and should be worked around by the driver to
> > avoid hard-coding policy which would prevent use cases such as the ones
> > mentioned above.

> This is not simple as you say. We have to fit these into a proper DPCM.
> Either we have a dummy Backend connected for each of these pcm 
> sub-devices when DP port is not connected and then switch back to DP 
> when its connected.

I don't know how best to implement it, but we shouldn't necessarily let
that determine the user experience.

> Or somehow find a way to not let the pipewire talk to devices which are 
> not connected.

Yes, perhaps it requires a change in user space.

But it seems the kernel should be able to fake whatever probing user
space currently does to determine if the there is a DP jack (even when
there is nothing connected).

Johan