Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

[Dmitry Baryshkov]

On Thu, Sep 25, 2025 at 12:05:09PM +0800, Jianfeng Liu wrote:
> After reusing drm_hdmi_audio_* helpers and drm_bridge_connector
> integration in drm/msm/dp, we have dropped msm_dp_audio_hw_params and
> use msm_dp_audio_prepare instead. While userspace is still calling
> hw_params to do audio initialization, and we get the following errors:
> 
> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> hdmi-audio-codec hdmi-audio-codec.0.auto: hdmi_codec_hw_params() started
> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
> qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: Failed to start APM port 104
> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC error (-22): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0
> MultiMedia2 Playback: ASoC error (-22): at dpcm_run_update_startup() on MultiMedia2 Playback

And a call to hdmi_codec_prepare() comes only at this place.

Srini, Mark, when selecting to only implement .prepare for codec ops I
was following the commit 2fef64eec23a ("ASoC: hdmi-codec: Add a prepare
hook"), which documents that IEC958 status bit is set after
.hw_params(), so it's only visible during .prepare(). Is it okay to
implement both callbacks? Or should the audioreach DAI driver be fixed
somehow instead (I suppose it assumes that the port is available after
.hw_params(), not sure if that assumption is correct)?

> 
> msm_dp_audio_prepare is not called because hdmi-codec driver only checks
> and runs hw_params before q6apm_lpass_dai_prepare(). This commit will
> add hw_params callback same as drm_connector_hdmi_audio_prepare, so that
> hdmi-codec driver can work with userspace alsa.
> 
> Tested with Radxa Dragon Q6A.
> 
> Fixes: 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec implementation")
> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>

The patch LGTM, but I would wait for response from audio maintainers.

> ---
> 
> Changes in v2:
> - Link to v1: https://lore.kernel.org/linux-arm-msm/20250924085804.34183-1-liujianfeng1994@gmail.com/
> - Use more detailed trace log in commit message.
> - Drop the empty line between Fixex and SoB.
> 
>  drivers/gpu/drm/display/drm_hdmi_audio_helper.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
> index 7d78b02c1446..6ca1c7ad0632 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
> @@ -130,6 +130,7 @@ EXPORT_SYMBOL(drm_connector_hdmi_audio_plugged_notify);
>  
>  static const struct hdmi_codec_ops drm_connector_hdmi_audio_ops = {
>  	.audio_startup = drm_connector_hdmi_audio_startup,
> +	.hw_params = drm_connector_hdmi_audio_prepare,
>  	.prepare = drm_connector_hdmi_audio_prepare,
>  	.audio_shutdown = drm_connector_hdmi_audio_shutdown,
>  	.mute_stream = drm_connector_hdmi_audio_mute_stream,
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

[Srinivas Kandagatla]

On 9/25/25 5:28 AM, Dmitry Baryshkov wrote:
> On Thu, Sep 25, 2025 at 12:05:09PM +0800, Jianfeng Liu wrote:
>> After reusing drm_hdmi_audio_* helpers and drm_bridge_connector
>> integration in drm/msm/dp, we have dropped msm_dp_audio_hw_params and
>> use msm_dp_audio_prepare instead. While userspace is still calling
>> hw_params to do audio initialization, and we get the following errors:
>>
>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>> hdmi-audio-codec hdmi-audio-codec.0.auto: hdmi_codec_hw_params() started
>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>> qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
>> qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: Failed to start APM port 104
>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC error (-22): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0
>> MultiMedia2 Playback: ASoC error (-22): at dpcm_run_update_startup() on MultiMedia2 Playback
> 
> And a call to hdmi_codec_prepare() comes only at this place.
> 
> Srini, Mark, when selecting to only implement .prepare for codec ops I
> was following the commit 2fef64eec23a ("ASoC: hdmi-codec: Add a prepare
> hook"), which documents that IEC958 status bit is set after
> .hw_params(), so it's only visible during .prepare(). Is it okay to
> implement both callbacks? Or should the audioreach DAI driver be fixed
> somehow instead (I suppose it assumes that the port is available after
> .hw_params(), not sure if that assumption is correct)?
> 
>>
>> msm_dp_audio_prepare is not called because hdmi-codec driver only checks
>> and runs hw_params before q6apm_lpass_dai_prepare(). This commit will
>> add hw_params callback same as drm_connector_hdmi_audio_prepare, so that
>> hdmi-codec driver can work with userspace alsa.
>>
>> Tested with Radxa Dragon Q6A.
>>
>> Fixes: 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec implementation")
>> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
> 
> The patch LGTM, but I would wait for response from audio maintainers.
> 

The ordering matters in this case as we need clocks and audio
configuration on DP codec side to be setup before we start configuring
the dsp pipeline. Looks like that DSP is trying to setup DP endpoint
even before it is ready.

q6apm prepare loads the dsp pipeline and starts configuring the
endpoints, if the DP endpoint is not ready dsp would throw an error.

We might be able to pull in some dsp logs to confirm this, but I dont
have a setup that I can reproduce this issue.


--srini
>> ---
>>
>> Changes in v2:
>> - Link to v1: https://lore.kernel.org/linux-arm-msm/20250924085804.34183-1-liujianfeng1994@gmail.com/
>> - Use more detailed trace log in commit message.
>> - Drop the empty line between Fixex and SoB.
>>
>>  drivers/gpu/drm/display/drm_hdmi_audio_helper.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
>> index 7d78b02c1446..6ca1c7ad0632 100644
>> --- a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
>> +++ b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
>> @@ -130,6 +130,7 @@ EXPORT_SYMBOL(drm_connector_hdmi_audio_plugged_notify);
>>  
>>  static const struct hdmi_codec_ops drm_connector_hdmi_audio_ops = {
>>  	.audio_startup = drm_connector_hdmi_audio_startup,
>> +	.hw_params = drm_connector_hdmi_audio_prepare,
>>  	.prepare = drm_connector_hdmi_audio_prepare,
>>  	.audio_shutdown = drm_connector_hdmi_audio_shutdown,
>>  	.mute_stream = drm_connector_hdmi_audio_mute_stream,
>> -- 
>> 2.43.0
>>
> 

Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

[Dmitry Baryshkov]

On Fri, Sep 26, 2025 at 11:30:26AM +0100, Srinivas Kandagatla wrote:
> 
> 
> On 9/25/25 5:28 AM, Dmitry Baryshkov wrote:
> > On Thu, Sep 25, 2025 at 12:05:09PM +0800, Jianfeng Liu wrote:
> >> After reusing drm_hdmi_audio_* helpers and drm_bridge_connector
> >> integration in drm/msm/dp, we have dropped msm_dp_audio_hw_params and
> >> use msm_dp_audio_prepare instead. While userspace is still calling
> >> hw_params to do audio initialization, and we get the following errors:
> >>
> >> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >> hdmi-audio-codec hdmi-audio-codec.0.auto: hdmi_codec_hw_params() started
> >> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >> qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
> >> qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
> >> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: Failed to start APM port 104
> >> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC error (-22): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0
> >> MultiMedia2 Playback: ASoC error (-22): at dpcm_run_update_startup() on MultiMedia2 Playback
> > 
> > And a call to hdmi_codec_prepare() comes only at this place.
> > 
> > Srini, Mark, when selecting to only implement .prepare for codec ops I
> > was following the commit 2fef64eec23a ("ASoC: hdmi-codec: Add a prepare
> > hook"), which documents that IEC958 status bit is set after
> > .hw_params(), so it's only visible during .prepare(). Is it okay to
> > implement both callbacks? Or should the audioreach DAI driver be fixed
> > somehow instead (I suppose it assumes that the port is available after
> > .hw_params(), not sure if that assumption is correct)?
> > 
> >>
> >> msm_dp_audio_prepare is not called because hdmi-codec driver only checks
> >> and runs hw_params before q6apm_lpass_dai_prepare(). This commit will
> >> add hw_params callback same as drm_connector_hdmi_audio_prepare, so that
> >> hdmi-codec driver can work with userspace alsa.
> >>
> >> Tested with Radxa Dragon Q6A.
> >>
> >> Fixes: 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec implementation")
> >> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
> > 
> > The patch LGTM, but I would wait for response from audio maintainers.
> > 
> 
> The ordering matters in this case as we need clocks and audio
> configuration on DP codec side to be setup before we start configuring
> the dsp pipeline. Looks like that DSP is trying to setup DP endpoint
> even before it is ready.
> 
> q6apm prepare loads the dsp pipeline and starts configuring the
> endpoints, if the DP endpoint is not ready dsp would throw an error.
> 
> We might be able to pull in some dsp logs to confirm this, but I dont
> have a setup that I can reproduce this issue.

What would be your recommendation to proceed? Is it okay for the DAI
driver to depend on the .hw_params enabling the clock? Also I see that
the error regarding the clocks comes from .prepare callback too. What is
the order of .prepare callbacks()? Can we influence it?

> 
> 
> --srini
> >> ---
> >>
> >> Changes in v2:
> >> - Link to v1: https://lore.kernel.org/linux-arm-msm/20250924085804.34183-1-liujianfeng1994@gmail.com/
> >> - Use more detailed trace log in commit message.
> >> - Drop the empty line between Fixex and SoB.
> >>
> >>  drivers/gpu/drm/display/drm_hdmi_audio_helper.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
> >> index 7d78b02c1446..6ca1c7ad0632 100644
> >> --- a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
> >> +++ b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
> >> @@ -130,6 +130,7 @@ EXPORT_SYMBOL(drm_connector_hdmi_audio_plugged_notify);
> >>  
> >>  static const struct hdmi_codec_ops drm_connector_hdmi_audio_ops = {
> >>  	.audio_startup = drm_connector_hdmi_audio_startup,
> >> +	.hw_params = drm_connector_hdmi_audio_prepare,
> >>  	.prepare = drm_connector_hdmi_audio_prepare,
> >>  	.audio_shutdown = drm_connector_hdmi_audio_shutdown,
> >>  	.mute_stream = drm_connector_hdmi_audio_mute_stream,
> >> -- 
> >> 2.43.0
> >>
> > 
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

[Dmitry Baryshkov]

On 26/09/2025 18:09, Dmitry Baryshkov wrote:
> On Fri, Sep 26, 2025 at 11:30:26AM +0100, Srinivas Kandagatla wrote:
>>
>>
>> On 9/25/25 5:28 AM, Dmitry Baryshkov wrote:
>>> On Thu, Sep 25, 2025 at 12:05:09PM +0800, Jianfeng Liu wrote:
>>>> After reusing drm_hdmi_audio_* helpers and drm_bridge_connector
>>>> integration in drm/msm/dp, we have dropped msm_dp_audio_hw_params and
>>>> use msm_dp_audio_prepare instead. While userspace is still calling
>>>> hw_params to do audio initialization, and we get the following errors:
>>>>
>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>> hdmi-audio-codec hdmi-audio-codec.0.auto: hdmi_codec_hw_params() started
>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>> qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
>>>> qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: Failed to start APM port 104
>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC error (-22): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0
>>>> MultiMedia2 Playback: ASoC error (-22): at dpcm_run_update_startup() on MultiMedia2 Playback
>>>
>>> And a call to hdmi_codec_prepare() comes only at this place.
>>>
>>> Srini, Mark, when selecting to only implement .prepare for codec ops I
>>> was following the commit 2fef64eec23a ("ASoC: hdmi-codec: Add a prepare
>>> hook"), which documents that IEC958 status bit is set after
>>> .hw_params(), so it's only visible during .prepare(). Is it okay to
>>> implement both callbacks? Or should the audioreach DAI driver be fixed
>>> somehow instead (I suppose it assumes that the port is available after
>>> .hw_params(), not sure if that assumption is correct)?
>>>
>>>>
>>>> msm_dp_audio_prepare is not called because hdmi-codec driver only checks
>>>> and runs hw_params before q6apm_lpass_dai_prepare(). This commit will
>>>> add hw_params callback same as drm_connector_hdmi_audio_prepare, so that
>>>> hdmi-codec driver can work with userspace alsa.
>>>>
>>>> Tested with Radxa Dragon Q6A.
>>>>
>>>> Fixes: 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec implementation")
>>>> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
>>>
>>> The patch LGTM, but I would wait for response from audio maintainers.
>>>
>>
>> The ordering matters in this case as we need clocks and audio
>> configuration on DP codec side to be setup before we start configuring
>> the dsp pipeline. Looks like that DSP is trying to setup DP endpoint
>> even before it is ready.
>>
>> q6apm prepare loads the dsp pipeline and starts configuring the
>> endpoints, if the DP endpoint is not ready dsp would throw an error.
>>
>> We might be able to pull in some dsp logs to confirm this, but I dont
>> have a setup that I can reproduce this issue.
> 
> What would be your recommendation to proceed? Is it okay for the DAI
> driver to depend on the .hw_params enabling the clock? Also I see that
> the error regarding the clocks comes from .prepare callback too. What is
> the order of .prepare callbacks()? Can we influence it?

Srinivas, any response from your side?


-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

[Srinivas Kandagatla]

On 9/26/25 4:09 PM, Dmitry Baryshkov wrote:
> On Fri, Sep 26, 2025 at 11:30:26AM +0100, Srinivas Kandagatla wrote:
>>
>>
>> On 9/25/25 5:28 AM, Dmitry Baryshkov wrote:
>>> On Thu, Sep 25, 2025 at 12:05:09PM +0800, Jianfeng Liu wrote:
>>>> After reusing drm_hdmi_audio_* helpers and drm_bridge_connector
>>>> integration in drm/msm/dp, we have dropped msm_dp_audio_hw_params and
>>>> use msm_dp_audio_prepare instead. While userspace is still calling
>>>> hw_params to do audio initialization, and we get the following errors:
>>>>
>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>> hdmi-audio-codec hdmi-audio-codec.0.auto: hdmi_codec_hw_params() started
>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>> qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
>>>> qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: Failed to start APM port 104
>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC error (-22): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0
>>>> MultiMedia2 Playback: ASoC error (-22): at dpcm_run_update_startup() on MultiMedia2 Playback
>>>
>>> And a call to hdmi_codec_prepare() comes only at this place.
>>>
>>> Srini, Mark, when selecting to only implement .prepare for codec ops I
>>> was following the commit 2fef64eec23a ("ASoC: hdmi-codec: Add a prepare
>>> hook"), which documents that IEC958 status bit is set after
>>> .hw_params(), so it's only visible during .prepare(). Is it okay to
>>> implement both callbacks? Or should the audioreach DAI driver be fixed
>>> somehow instead (I suppose it assumes that the port is available after
>>> .hw_params(), not sure if that assumption is correct)?
>>>
>>>>
>>>> msm_dp_audio_prepare is not called because hdmi-codec driver only checks
>>>> and runs hw_params before q6apm_lpass_dai_prepare(). This commit will
>>>> add hw_params callback same as drm_connector_hdmi_audio_prepare, so that
>>>> hdmi-codec driver can work with userspace alsa.
>>>>
>>>> Tested with Radxa Dragon Q6A.
>>>>
>>>> Fixes: 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec implementation")
>>>> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
>>>
>>> The patch LGTM, but I would wait for response from audio maintainers.
>>>
>>
>> The ordering matters in this case as we need clocks and audio
>> configuration on DP codec side to be setup before we start configuring
>> the dsp pipeline. Looks like that DSP is trying to setup DP endpoint
>> even before it is ready.
>>
>> q6apm prepare loads the dsp pipeline and starts configuring the
>> endpoints, if the DP endpoint is not ready dsp would throw an error.
>>
>> We might be able to pull in some dsp logs to confirm this, but I dont
>> have a setup that I can reproduce this issue.
> 
> What would be your recommendation to proceed? Is it okay for the DAI
> driver to depend on the .hw_params enabling the clock? Also I see that
> the error regarding the clocks comes from .prepare callback too. What is
> the order of .prepare callbacks()? Can we influence it?

prepare follows hw-params, and prepare can be called multiple times

When you mean order of prepare callbacks, you mean w.r.t codec and dsp
backend dia link drivers ?

TBH, Am not sure, I did not find anything that was obvious from the code.



--srini



> 
>>
>>
>> --srini
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - Link to v1: https://lore.kernel.org/linux-arm-msm/20250924085804.34183-1-liujianfeng1994@gmail.com/
>>>> - Use more detailed trace log in commit message.
>>>> - Drop the empty line between Fixex and SoB.
>>>>
>>>>  drivers/gpu/drm/display/drm_hdmi_audio_helper.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
>>>> index 7d78b02c1446..6ca1c7ad0632 100644
>>>> --- a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
>>>> +++ b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
>>>> @@ -130,6 +130,7 @@ EXPORT_SYMBOL(drm_connector_hdmi_audio_plugged_notify);
>>>>  
>>>>  static const struct hdmi_codec_ops drm_connector_hdmi_audio_ops = {
>>>>  	.audio_startup = drm_connector_hdmi_audio_startup,
>>>> +	.hw_params = drm_connector_hdmi_audio_prepare,
>>>>  	.prepare = drm_connector_hdmi_audio_prepare,
>>>>  	.audio_shutdown = drm_connector_hdmi_audio_shutdown,
>>>>  	.mute_stream = drm_connector_hdmi_audio_mute_stream,
>>>> -- 
>>>> 2.43.0
>>>>
>>>
>>
> 

Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

[Dmitry Baryshkov]

On Fri, Oct 03, 2025 at 05:35:16PM +0100, Srinivas Kandagatla wrote:
> 
> 
> On 9/26/25 4:09 PM, Dmitry Baryshkov wrote:
> > On Fri, Sep 26, 2025 at 11:30:26AM +0100, Srinivas Kandagatla wrote:
> >>
> >>
> >> On 9/25/25 5:28 AM, Dmitry Baryshkov wrote:
> >>> On Thu, Sep 25, 2025 at 12:05:09PM +0800, Jianfeng Liu wrote:
> >>>> After reusing drm_hdmi_audio_* helpers and drm_bridge_connector
> >>>> integration in drm/msm/dp, we have dropped msm_dp_audio_hw_params and
> >>>> use msm_dp_audio_prepare instead. While userspace is still calling
> >>>> hw_params to do audio initialization, and we get the following errors:
> >>>>
> >>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>> hdmi-audio-codec hdmi-audio-codec.0.auto: hdmi_codec_hw_params() started
> >>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>> qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
> >>>> qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
> >>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: Failed to start APM port 104
> >>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC error (-22): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0
> >>>> MultiMedia2 Playback: ASoC error (-22): at dpcm_run_update_startup() on MultiMedia2 Playback
> >>>
> >>> And a call to hdmi_codec_prepare() comes only at this place.
> >>>
> >>> Srini, Mark, when selecting to only implement .prepare for codec ops I
> >>> was following the commit 2fef64eec23a ("ASoC: hdmi-codec: Add a prepare
> >>> hook"), which documents that IEC958 status bit is set after
> >>> .hw_params(), so it's only visible during .prepare(). Is it okay to
> >>> implement both callbacks? Or should the audioreach DAI driver be fixed
> >>> somehow instead (I suppose it assumes that the port is available after
> >>> .hw_params(), not sure if that assumption is correct)?
> >>>
> >>>>
> >>>> msm_dp_audio_prepare is not called because hdmi-codec driver only checks
> >>>> and runs hw_params before q6apm_lpass_dai_prepare(). This commit will
> >>>> add hw_params callback same as drm_connector_hdmi_audio_prepare, so that
> >>>> hdmi-codec driver can work with userspace alsa.
> >>>>
> >>>> Tested with Radxa Dragon Q6A.
> >>>>
> >>>> Fixes: 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec implementation")
> >>>> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
> >>>
> >>> The patch LGTM, but I would wait for response from audio maintainers.
> >>>
> >>
> >> The ordering matters in this case as we need clocks and audio
> >> configuration on DP codec side to be setup before we start configuring
> >> the dsp pipeline. Looks like that DSP is trying to setup DP endpoint
> >> even before it is ready.
> >>
> >> q6apm prepare loads the dsp pipeline and starts configuring the
> >> endpoints, if the DP endpoint is not ready dsp would throw an error.
> >>
> >> We might be able to pull in some dsp logs to confirm this, but I dont
> >> have a setup that I can reproduce this issue.
> > 
> > What would be your recommendation to proceed? Is it okay for the DAI
> > driver to depend on the .hw_params enabling the clock? Also I see that
> > the error regarding the clocks comes from .prepare callback too. What is
> > the order of .prepare callbacks()? Can we influence it?
> 
> prepare follows hw-params, and prepare can be called multiple times
> 
> When you mean order of prepare callbacks, you mean w.r.t codec and dsp
> backend dia link drivers ?

Yes. Here we got a dependency from the cpu dai to be prepare()'d after
the DP driver performs some actions, which were a part of hw_params()
callback but were moved to be executed during prepare() callback.

This leads me to two sets of questions:
- In which order are those callbacks executed? Can we make the ASoC
  enforce some order of DAI's prepare() callbacks?

- More improtantly, isn't it a sympthom of DP driver (incorrectly)
  performing too much in the .hw_params() / .prepare() callback? Should
  we move some of the setup to the .audio_startup() instead? What is the
  expected split between those callbacks?

> 
> TBH, Am not sure, I did not find anything that was obvious from the code.
> 
> 
> 
> --srini
> 
> 
> 
> > 
> >>
> >>
> >> --srini
> >>>> ---
> >>>>
> >>>> Changes in v2:
> >>>> - Link to v1: https://lore.kernel.org/linux-arm-msm/20250924085804.34183-1-liujianfeng1994@gmail.com/
> >>>> - Use more detailed trace log in commit message.
> >>>> - Drop the empty line between Fixex and SoB.
> >>>>
> >>>>  drivers/gpu/drm/display/drm_hdmi_audio_helper.c | 1 +
> >>>>  1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
> >>>> index 7d78b02c1446..6ca1c7ad0632 100644
> >>>> --- a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
> >>>> +++ b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
> >>>> @@ -130,6 +130,7 @@ EXPORT_SYMBOL(drm_connector_hdmi_audio_plugged_notify);
> >>>>  
> >>>>  static const struct hdmi_codec_ops drm_connector_hdmi_audio_ops = {
> >>>>  	.audio_startup = drm_connector_hdmi_audio_startup,
> >>>> +	.hw_params = drm_connector_hdmi_audio_prepare,
> >>>>  	.prepare = drm_connector_hdmi_audio_prepare,
> >>>>  	.audio_shutdown = drm_connector_hdmi_audio_shutdown,
> >>>>  	.mute_stream = drm_connector_hdmi_audio_mute_stream,
> >>>> -- 
> >>>> 2.43.0
> >>>>
> >>>
> >>
> > 
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

[Srinivas Kandagatla]

On 10/3/25 6:02 PM, Dmitry Baryshkov wrote:
> On Fri, Oct 03, 2025 at 05:35:16PM +0100, Srinivas Kandagatla wrote:
>>
>>
>> On 9/26/25 4:09 PM, Dmitry Baryshkov wrote:
>>> On Fri, Sep 26, 2025 at 11:30:26AM +0100, Srinivas Kandagatla wrote:
>>>>
>>>>
>>>> On 9/25/25 5:28 AM, Dmitry Baryshkov wrote:
>>>>> On Thu, Sep 25, 2025 at 12:05:09PM +0800, Jianfeng Liu wrote:
>>>>>> After reusing drm_hdmi_audio_* helpers and drm_bridge_connector
>>>>>> integration in drm/msm/dp, we have dropped msm_dp_audio_hw_params and
>>>>>> use msm_dp_audio_prepare instead. While userspace is still calling
>>>>>> hw_params to do audio initialization, and we get the following errors:
>>>>>>
>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>>>> hdmi-audio-codec hdmi-audio-codec.0.auto: hdmi_codec_hw_params() started
>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>>>> qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
>>>>>> qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: Failed to start APM port 104
>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC error (-22): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0
>>>>>> MultiMedia2 Playback: ASoC error (-22): at dpcm_run_update_startup() on MultiMedia2 Playback
>>>>>
>>>>> And a call to hdmi_codec_prepare() comes only at this place.
>>>>>
>>>>> Srini, Mark, when selecting to only implement .prepare for codec ops I
>>>>> was following the commit 2fef64eec23a ("ASoC: hdmi-codec: Add a prepare
>>>>> hook"), which documents that IEC958 status bit is set after
>>>>> .hw_params(), so it's only visible during .prepare(). Is it okay to
>>>>> implement both callbacks? Or should the audioreach DAI driver be fixed
>>>>> somehow instead (I suppose it assumes that the port is available after
>>>>> .hw_params(), not sure if that assumption is correct)?
>>>>>
>>>>>>
>>>>>> msm_dp_audio_prepare is not called because hdmi-codec driver only checks
>>>>>> and runs hw_params before q6apm_lpass_dai_prepare(). This commit will
>>>>>> add hw_params callback same as drm_connector_hdmi_audio_prepare, so that
>>>>>> hdmi-codec driver can work with userspace alsa.
>>>>>>
>>>>>> Tested with Radxa Dragon Q6A.
>>>>>>
>>>>>> Fixes: 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec implementation")
>>>>>> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
>>>>>
>>>>> The patch LGTM, but I would wait for response from audio maintainers.
>>>>>
>>>>
>>>> The ordering matters in this case as we need clocks and audio
>>>> configuration on DP codec side to be setup before we start configuring
>>>> the dsp pipeline. Looks like that DSP is trying to setup DP endpoint
>>>> even before it is ready.
>>>>
>>>> q6apm prepare loads the dsp pipeline and starts configuring the
>>>> endpoints, if the DP endpoint is not ready dsp would throw an error.
>>>>
>>>> We might be able to pull in some dsp logs to confirm this, but I dont
>>>> have a setup that I can reproduce this issue.
>>>
>>> What would be your recommendation to proceed? Is it okay for the DAI
>>> driver to depend on the .hw_params enabling the clock? Also I see that
>>> the error regarding the clocks comes from .prepare callback too. What is
>>> the order of .prepare callbacks()? Can we influence it?
>>
>> prepare follows hw-params, and prepare can be called multiple times
>>
>> When you mean order of prepare callbacks, you mean w.r.t codec and dsp
>> backend dia link drivers ?
> 
> Yes. Here we got a dependency from the cpu dai to be prepare()'d after
> the DP driver performs some actions, which were a part of hw_params()
> callback but were moved to be executed during prepare() callback.
> 
> This leads me to two sets of questions:
> - In which order are those callbacks executed? Can we make the ASoC
>   enforce some order of DAI's prepare() callbacks?
> 
> - More improtantly, isn't it a sympthom of DP driver (incorrectly)
>   performing too much in the .hw_params() / .prepare() callback? Should
>   we move some of the setup to the .audio_startup() instead? What is the
>   expected split between those callbacks?

I have not looked at full dp sequences but, if prepare is the only place
when it enables the required clocks for audio block, then it is a
problem, we should do it early so that DSP can enable the required
configuration in prepare.

Its also doable to move out the clock related settings from  prepare to
startup which should work aswell.

--srini

> 
>>
>> TBH, Am not sure, I did not find anything that was obvious from the code.
>>
>>
>>
>> --srini
>>
>>
>>
>>>
>>>>
>>>>
>>>> --srini
>>>>>> ---
>>>>>>
>>>>>> Changes in v2:
>>>>>> - Link to v1: https://lore.kernel.org/linux-arm-msm/20250924085804.34183-1-liujianfeng1994@gmail.com/
>>>>>> - Use more detailed trace log in commit message.
>>>>>> - Drop the empty line between Fixex and SoB.
>>>>>>
>>>>>>  drivers/gpu/drm/display/drm_hdmi_audio_helper.c | 1 +
>>>>>>  1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
>>>>>> index 7d78b02c1446..6ca1c7ad0632 100644
>>>>>> --- a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
>>>>>> +++ b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
>>>>>> @@ -130,6 +130,7 @@ EXPORT_SYMBOL(drm_connector_hdmi_audio_plugged_notify);
>>>>>>  
>>>>>>  static const struct hdmi_codec_ops drm_connector_hdmi_audio_ops = {
>>>>>>  	.audio_startup = drm_connector_hdmi_audio_startup,
>>>>>> +	.hw_params = drm_connector_hdmi_audio_prepare,
>>>>>>  	.prepare = drm_connector_hdmi_audio_prepare,
>>>>>>  	.audio_shutdown = drm_connector_hdmi_audio_shutdown,
>>>>>>  	.mute_stream = drm_connector_hdmi_audio_mute_stream,
>>>>>> -- 
>>>>>> 2.43.0
>>>>>>
>>>>>
>>>>
>>>
>>
> 

Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

[Dmitry Baryshkov]

On Sat, Oct 04, 2025 at 02:16:08PM +0100, Srinivas Kandagatla wrote:
> 
> 
> On 10/3/25 6:02 PM, Dmitry Baryshkov wrote:
> > On Fri, Oct 03, 2025 at 05:35:16PM +0100, Srinivas Kandagatla wrote:
> >>
> >>
> >> On 9/26/25 4:09 PM, Dmitry Baryshkov wrote:
> >>> On Fri, Sep 26, 2025 at 11:30:26AM +0100, Srinivas Kandagatla wrote:
> >>>>
> >>>>
> >>>> On 9/25/25 5:28 AM, Dmitry Baryshkov wrote:
> >>>>> On Thu, Sep 25, 2025 at 12:05:09PM +0800, Jianfeng Liu wrote:
> >>>>>> After reusing drm_hdmi_audio_* helpers and drm_bridge_connector
> >>>>>> integration in drm/msm/dp, we have dropped msm_dp_audio_hw_params and
> >>>>>> use msm_dp_audio_prepare instead. While userspace is still calling
> >>>>>> hw_params to do audio initialization, and we get the following errors:
> >>>>>>
> >>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>>>> hdmi-audio-codec hdmi-audio-codec.0.auto: hdmi_codec_hw_params() started
> >>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>>>> qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
> >>>>>> qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
> >>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: Failed to start APM port 104
> >>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC error (-22): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0
> >>>>>> MultiMedia2 Playback: ASoC error (-22): at dpcm_run_update_startup() on MultiMedia2 Playback
> >>>>>
> >>>>> And a call to hdmi_codec_prepare() comes only at this place.
> >>>>>
> >>>>> Srini, Mark, when selecting to only implement .prepare for codec ops I
> >>>>> was following the commit 2fef64eec23a ("ASoC: hdmi-codec: Add a prepare
> >>>>> hook"), which documents that IEC958 status bit is set after
> >>>>> .hw_params(), so it's only visible during .prepare(). Is it okay to
> >>>>> implement both callbacks? Or should the audioreach DAI driver be fixed
> >>>>> somehow instead (I suppose it assumes that the port is available after
> >>>>> .hw_params(), not sure if that assumption is correct)?
> >>>>>
> >>>>>>
> >>>>>> msm_dp_audio_prepare is not called because hdmi-codec driver only checks
> >>>>>> and runs hw_params before q6apm_lpass_dai_prepare(). This commit will
> >>>>>> add hw_params callback same as drm_connector_hdmi_audio_prepare, so that
> >>>>>> hdmi-codec driver can work with userspace alsa.
> >>>>>>
> >>>>>> Tested with Radxa Dragon Q6A.
> >>>>>>
> >>>>>> Fixes: 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec implementation")
> >>>>>> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
> >>>>>
> >>>>> The patch LGTM, but I would wait for response from audio maintainers.
> >>>>>
> >>>>
> >>>> The ordering matters in this case as we need clocks and audio
> >>>> configuration on DP codec side to be setup before we start configuring
> >>>> the dsp pipeline. Looks like that DSP is trying to setup DP endpoint
> >>>> even before it is ready.
> >>>>
> >>>> q6apm prepare loads the dsp pipeline and starts configuring the
> >>>> endpoints, if the DP endpoint is not ready dsp would throw an error.
> >>>>
> >>>> We might be able to pull in some dsp logs to confirm this, but I dont
> >>>> have a setup that I can reproduce this issue.
> >>>
> >>> What would be your recommendation to proceed? Is it okay for the DAI
> >>> driver to depend on the .hw_params enabling the clock? Also I see that
> >>> the error regarding the clocks comes from .prepare callback too. What is
> >>> the order of .prepare callbacks()? Can we influence it?
> >>
> >> prepare follows hw-params, and prepare can be called multiple times
> >>
> >> When you mean order of prepare callbacks, you mean w.r.t codec and dsp
> >> backend dia link drivers ?
> > 
> > Yes. Here we got a dependency from the cpu dai to be prepare()'d after
> > the DP driver performs some actions, which were a part of hw_params()
> > callback but were moved to be executed during prepare() callback.
> > 
> > This leads me to two sets of questions:
> > - In which order are those callbacks executed? Can we make the ASoC
> >   enforce some order of DAI's prepare() callbacks?
> > 
> > - More improtantly, isn't it a sympthom of DP driver (incorrectly)
> >   performing too much in the .hw_params() / .prepare() callback? Should
> >   we move some of the setup to the .audio_startup() instead? What is the
> >   expected split between those callbacks?
> 
> I have not looked at full dp sequences but, if prepare is the only place
> when it enables the required clocks for audio block, then it is a
> problem, we should do it early so that DSP can enable the required
> configuration in prepare.

As I wrote, previously it was done as a part of hw_params, which is
also not optimal as hdmi-codec clearly defines both callbacks as
optional, requiring only one of them. Do you know, what is actually
required on the DP side for the DSP to be able to preapre()?

> 
> Its also doable to move out the clock related settings from  prepare to
> startup which should work aswell.
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

[Dmitry Baryshkov]

On Sun, Oct 05, 2025 at 06:29:20PM +0300, Dmitry Baryshkov wrote:
> On Sat, Oct 04, 2025 at 02:16:08PM +0100, Srinivas Kandagatla wrote:
> > 
> > 
> > On 10/3/25 6:02 PM, Dmitry Baryshkov wrote:
> > > On Fri, Oct 03, 2025 at 05:35:16PM +0100, Srinivas Kandagatla wrote:
> > >>
> > >>
> > >> On 9/26/25 4:09 PM, Dmitry Baryshkov wrote:
> > >>> On Fri, Sep 26, 2025 at 11:30:26AM +0100, Srinivas Kandagatla wrote:
> > >>>>
> > >>>>
> > >>>> On 9/25/25 5:28 AM, Dmitry Baryshkov wrote:
> > >>>>> On Thu, Sep 25, 2025 at 12:05:09PM +0800, Jianfeng Liu wrote:
> > >>>>>> After reusing drm_hdmi_audio_* helpers and drm_bridge_connector
> > >>>>>> integration in drm/msm/dp, we have dropped msm_dp_audio_hw_params and
> > >>>>>> use msm_dp_audio_prepare instead. While userspace is still calling
> > >>>>>> hw_params to do audio initialization, and we get the following errors:
> > >>>>>>
> > >>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> > >>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> > >>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> > >>>>>> hdmi-audio-codec hdmi-audio-codec.0.auto: hdmi_codec_hw_params() started
> > >>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> > >>>>>> qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
> > >>>>>> qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
> > >>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: Failed to start APM port 104
> > >>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC error (-22): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0
> > >>>>>> MultiMedia2 Playback: ASoC error (-22): at dpcm_run_update_startup() on MultiMedia2 Playback
> > >>>>>
> > >>>>> And a call to hdmi_codec_prepare() comes only at this place.
> > >>>>>
> > >>>>> Srini, Mark, when selecting to only implement .prepare for codec ops I
> > >>>>> was following the commit 2fef64eec23a ("ASoC: hdmi-codec: Add a prepare
> > >>>>> hook"), which documents that IEC958 status bit is set after
> > >>>>> .hw_params(), so it's only visible during .prepare(). Is it okay to
> > >>>>> implement both callbacks? Or should the audioreach DAI driver be fixed
> > >>>>> somehow instead (I suppose it assumes that the port is available after
> > >>>>> .hw_params(), not sure if that assumption is correct)?
> > >>>>>
> > >>>>>>
> > >>>>>> msm_dp_audio_prepare is not called because hdmi-codec driver only checks
> > >>>>>> and runs hw_params before q6apm_lpass_dai_prepare(). This commit will
> > >>>>>> add hw_params callback same as drm_connector_hdmi_audio_prepare, so that
> > >>>>>> hdmi-codec driver can work with userspace alsa.
> > >>>>>>
> > >>>>>> Tested with Radxa Dragon Q6A.
> > >>>>>>
> > >>>>>> Fixes: 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec implementation")
> > >>>>>> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
> > >>>>>
> > >>>>> The patch LGTM, but I would wait for response from audio maintainers.
> > >>>>>
> > >>>>
> > >>>> The ordering matters in this case as we need clocks and audio
> > >>>> configuration on DP codec side to be setup before we start configuring
> > >>>> the dsp pipeline. Looks like that DSP is trying to setup DP endpoint
> > >>>> even before it is ready.
> > >>>>
> > >>>> q6apm prepare loads the dsp pipeline and starts configuring the
> > >>>> endpoints, if the DP endpoint is not ready dsp would throw an error.
> > >>>>
> > >>>> We might be able to pull in some dsp logs to confirm this, but I dont
> > >>>> have a setup that I can reproduce this issue.
> > >>>
> > >>> What would be your recommendation to proceed? Is it okay for the DAI
> > >>> driver to depend on the .hw_params enabling the clock? Also I see that
> > >>> the error regarding the clocks comes from .prepare callback too. What is
> > >>> the order of .prepare callbacks()? Can we influence it?
> > >>
> > >> prepare follows hw-params, and prepare can be called multiple times
> > >>
> > >> When you mean order of prepare callbacks, you mean w.r.t codec and dsp
> > >> backend dia link drivers ?
> > > 
> > > Yes. Here we got a dependency from the cpu dai to be prepare()'d after
> > > the DP driver performs some actions, which were a part of hw_params()
> > > callback but were moved to be executed during prepare() callback.
> > > 
> > > This leads me to two sets of questions:
> > > - In which order are those callbacks executed? Can we make the ASoC
> > >   enforce some order of DAI's prepare() callbacks?
> > > 
> > > - More improtantly, isn't it a sympthom of DP driver (incorrectly)
> > >   performing too much in the .hw_params() / .prepare() callback? Should
> > >   we move some of the setup to the .audio_startup() instead? What is the
> > >   expected split between those callbacks?
> > 
> > I have not looked at full dp sequences but, if prepare is the only place
> > when it enables the required clocks for audio block, then it is a
> > problem, we should do it early so that DSP can enable the required
> > configuration in prepare.
> 
> As I wrote, previously it was done as a part of hw_params, which is
> also not optimal as hdmi-codec clearly defines both callbacks as
> optional, requiring only one of them. Do you know, what is actually
> required on the DP side for the DSP to be able to preapre()?

Srinivas, did you have a chance to look at this? Or the .hw_prepare
ordering? I really don't want to land this fix only to face the same
issue because on some of the platforms .hw_prepare() callbacks will be
called in a different order.

> 
> > 
> > Its also doable to move out the clock related settings from  prepare to
> > startup which should work aswell.
> > 
> 
> -- 
> With best wishes
> Dmitry

-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

[Srinivas Kandagatla]

On 10/23/25 12:43 PM, Dmitry Baryshkov wrote:
> On Sun, Oct 05, 2025 at 06:29:20PM +0300, Dmitry Baryshkov wrote:
>> On Sat, Oct 04, 2025 at 02:16:08PM +0100, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 10/3/25 6:02 PM, Dmitry Baryshkov wrote:
>>>> On Fri, Oct 03, 2025 at 05:35:16PM +0100, Srinivas Kandagatla wrote:
>>>>>
>>>>>
>>>>> On 9/26/25 4:09 PM, Dmitry Baryshkov wrote:
>>>>>> On Fri, Sep 26, 2025 at 11:30:26AM +0100, Srinivas Kandagatla wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 9/25/25 5:28 AM, Dmitry Baryshkov wrote:
>>>>>>>> On Thu, Sep 25, 2025 at 12:05:09PM +0800, Jianfeng Liu wrote:
>>>>>>>>> After reusing drm_hdmi_audio_* helpers and drm_bridge_connector
>>>>>>>>> integration in drm/msm/dp, we have dropped msm_dp_audio_hw_params and
>>>>>>>>> use msm_dp_audio_prepare instead. While userspace is still calling
>>>>>>>>> hw_params to do audio initialization, and we get the following errors:
>>>>>>>>>
>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>>>>>>> hdmi-audio-codec hdmi-audio-codec.0.auto: hdmi_codec_hw_params() started
>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>>>>>>> qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
>>>>>>>>> qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: Failed to start APM port 104
>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC error (-22): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0
>>>>>>>>> MultiMedia2 Playback: ASoC error (-22): at dpcm_run_update_startup() on MultiMedia2 Playback
>>>>>>>>
>>>>>>>> And a call to hdmi_codec_prepare() comes only at this place.
>>>>>>>>
>>>>>>>> Srini, Mark, when selecting to only implement .prepare for codec ops I
>>>>>>>> was following the commit 2fef64eec23a ("ASoC: hdmi-codec: Add a prepare
>>>>>>>> hook"), which documents that IEC958 status bit is set after
>>>>>>>> .hw_params(), so it's only visible during .prepare(). Is it okay to
>>>>>>>> implement both callbacks? Or should the audioreach DAI driver be fixed
>>>>>>>> somehow instead (I suppose it assumes that the port is available after
>>>>>>>> .hw_params(), not sure if that assumption is correct)?
>>>>>>>>
>>>>>>>>>
>>>>>>>>> msm_dp_audio_prepare is not called because hdmi-codec driver only checks
>>>>>>>>> and runs hw_params before q6apm_lpass_dai_prepare(). This commit will
>>>>>>>>> add hw_params callback same as drm_connector_hdmi_audio_prepare, so that
>>>>>>>>> hdmi-codec driver can work with userspace alsa.
>>>>>>>>>
>>>>>>>>> Tested with Radxa Dragon Q6A.
>>>>>>>>>
>>>>>>>>> Fixes: 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec implementation")
>>>>>>>>> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
>>>>>>>>
>>>>>>>> The patch LGTM, but I would wait for response from audio maintainers.
>>>>>>>>
>>>>>>>
>>>>>>> The ordering matters in this case as we need clocks and audio
>>>>>>> configuration on DP codec side to be setup before we start configuring
>>>>>>> the dsp pipeline. Looks like that DSP is trying to setup DP endpoint
>>>>>>> even before it is ready.
>>>>>>>
>>>>>>> q6apm prepare loads the dsp pipeline and starts configuring the
>>>>>>> endpoints, if the DP endpoint is not ready dsp would throw an error.
>>>>>>>
>>>>>>> We might be able to pull in some dsp logs to confirm this, but I dont
>>>>>>> have a setup that I can reproduce this issue.
>>>>>>
>>>>>> What would be your recommendation to proceed? Is it okay for the DAI
>>>>>> driver to depend on the .hw_params enabling the clock? Also I see that
>>>>>> the error regarding the clocks comes from .prepare callback too. What is
>>>>>> the order of .prepare callbacks()? Can we influence it?
>>>>>
>>>>> prepare follows hw-params, and prepare can be called multiple times
>>>>>
>>>>> When you mean order of prepare callbacks, you mean w.r.t codec and dsp
>>>>> backend dia link drivers ?
>>>>
>>>> Yes. Here we got a dependency from the cpu dai to be prepare()'d after
>>>> the DP driver performs some actions, which were a part of hw_params()
>>>> callback but were moved to be executed during prepare() callback.
>>>>
>>>> This leads me to two sets of questions:
>>>> - In which order are those callbacks executed? Can we make the ASoC
>>>>   enforce some order of DAI's prepare() callbacks?
>>>>
>>>> - More improtantly, isn't it a sympthom of DP driver (incorrectly)
>>>>   performing too much in the .hw_params() / .prepare() callback? Should
>>>>   we move some of the setup to the .audio_startup() instead? What is the
>>>>   expected split between those callbacks?
>>>
>>> I have not looked at full dp sequences but, if prepare is the only place
>>> when it enables the required clocks for audio block, then it is a
>>> problem, we should do it early so that DSP can enable the required
>>> configuration in prepare.
>>
>> As I wrote, previously it was done as a part of hw_params, which is
>> also not optimal as hdmi-codec clearly defines both callbacks as
>> optional, requiring only one of them. Do you know, what is actually
>> required on the DP side for the DSP to be able to preapre()?
> 
> Srinivas, did you have a chance to look at this? Or the .hw_prepare
> ordering? I really don't want to land this fix only to face the same
> issue because on some of the platforms .hw_prepare() callbacks will be
> called in a different order.
> 
Dmitry, few days back Krzysztof was enabling DP audio on T14s, and we
have seen that the DSP is not happy and returning error 1 when we try to
prepare the dp port. The exact reason is codec is not ready at that point.


By adding this patch, Krzystof managed to get DP working.
XElite platforms were working before 2fef64eec23a ("ASoC: hdmi-codec:
Add a prepare) patch was merged, basically 2fef64eec23a broke what was
working before.

Am happy with this patch, better move to hw_params instead of prepare.


--srini

>>
>>>
>>> Its also doable to move out the clock related settings from  prepare to
>>> startup which should work aswell.
>>>
>>
>> -- 
>> With best wishes
>> Dmitry
> 

Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

[Dmitry Baryshkov]

On Thu, Oct 23, 2025 at 12:48:59PM +0100, Srinivas Kandagatla wrote:
> 
> 
> On 10/23/25 12:43 PM, Dmitry Baryshkov wrote:
> > On Sun, Oct 05, 2025 at 06:29:20PM +0300, Dmitry Baryshkov wrote:
> >> On Sat, Oct 04, 2025 at 02:16:08PM +0100, Srinivas Kandagatla wrote:
> >>>
> >>>
> >>> On 10/3/25 6:02 PM, Dmitry Baryshkov wrote:
> >>>> On Fri, Oct 03, 2025 at 05:35:16PM +0100, Srinivas Kandagatla wrote:
> >>>>>
> >>>>>
> >>>>> On 9/26/25 4:09 PM, Dmitry Baryshkov wrote:
> >>>>>> On Fri, Sep 26, 2025 at 11:30:26AM +0100, Srinivas Kandagatla wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On 9/25/25 5:28 AM, Dmitry Baryshkov wrote:
> >>>>>>>> On Thu, Sep 25, 2025 at 12:05:09PM +0800, Jianfeng Liu wrote:
> >>>>>>>>> After reusing drm_hdmi_audio_* helpers and drm_bridge_connector
> >>>>>>>>> integration in drm/msm/dp, we have dropped msm_dp_audio_hw_params and
> >>>>>>>>> use msm_dp_audio_prepare instead. While userspace is still calling
> >>>>>>>>> hw_params to do audio initialization, and we get the following errors:
> >>>>>>>>>
> >>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>>>>>>> hdmi-audio-codec hdmi-audio-codec.0.auto: hdmi_codec_hw_params() started
> >>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>>>>>>> qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
> >>>>>>>>> qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
> >>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: Failed to start APM port 104
> >>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC error (-22): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0
> >>>>>>>>> MultiMedia2 Playback: ASoC error (-22): at dpcm_run_update_startup() on MultiMedia2 Playback
> >>>>>>>>
> >>>>>>>> And a call to hdmi_codec_prepare() comes only at this place.
> >>>>>>>>
> >>>>>>>> Srini, Mark, when selecting to only implement .prepare for codec ops I
> >>>>>>>> was following the commit 2fef64eec23a ("ASoC: hdmi-codec: Add a prepare
> >>>>>>>> hook"), which documents that IEC958 status bit is set after
> >>>>>>>> .hw_params(), so it's only visible during .prepare(). Is it okay to
> >>>>>>>> implement both callbacks? Or should the audioreach DAI driver be fixed
> >>>>>>>> somehow instead (I suppose it assumes that the port is available after
> >>>>>>>> .hw_params(), not sure if that assumption is correct)?
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>> msm_dp_audio_prepare is not called because hdmi-codec driver only checks
> >>>>>>>>> and runs hw_params before q6apm_lpass_dai_prepare(). This commit will
> >>>>>>>>> add hw_params callback same as drm_connector_hdmi_audio_prepare, so that
> >>>>>>>>> hdmi-codec driver can work with userspace alsa.
> >>>>>>>>>
> >>>>>>>>> Tested with Radxa Dragon Q6A.
> >>>>>>>>>
> >>>>>>>>> Fixes: 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec implementation")
> >>>>>>>>> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
> >>>>>>>>
> >>>>>>>> The patch LGTM, but I would wait for response from audio maintainers.
> >>>>>>>>
> >>>>>>>
> >>>>>>> The ordering matters in this case as we need clocks and audio
> >>>>>>> configuration on DP codec side to be setup before we start configuring
> >>>>>>> the dsp pipeline. Looks like that DSP is trying to setup DP endpoint
> >>>>>>> even before it is ready.
> >>>>>>>
> >>>>>>> q6apm prepare loads the dsp pipeline and starts configuring the
> >>>>>>> endpoints, if the DP endpoint is not ready dsp would throw an error.
> >>>>>>>
> >>>>>>> We might be able to pull in some dsp logs to confirm this, but I dont
> >>>>>>> have a setup that I can reproduce this issue.
> >>>>>>
> >>>>>> What would be your recommendation to proceed? Is it okay for the DAI
> >>>>>> driver to depend on the .hw_params enabling the clock? Also I see that
> >>>>>> the error regarding the clocks comes from .prepare callback too. What is
> >>>>>> the order of .prepare callbacks()? Can we influence it?
> >>>>>
> >>>>> prepare follows hw-params, and prepare can be called multiple times
> >>>>>
> >>>>> When you mean order of prepare callbacks, you mean w.r.t codec and dsp
> >>>>> backend dia link drivers ?
> >>>>
> >>>> Yes. Here we got a dependency from the cpu dai to be prepare()'d after
> >>>> the DP driver performs some actions, which were a part of hw_params()
> >>>> callback but were moved to be executed during prepare() callback.
> >>>>
> >>>> This leads me to two sets of questions:
> >>>> - In which order are those callbacks executed? Can we make the ASoC
> >>>>   enforce some order of DAI's prepare() callbacks?
> >>>>
> >>>> - More improtantly, isn't it a sympthom of DP driver (incorrectly)
> >>>>   performing too much in the .hw_params() / .prepare() callback? Should
> >>>>   we move some of the setup to the .audio_startup() instead? What is the
> >>>>   expected split between those callbacks?
> >>>
> >>> I have not looked at full dp sequences but, if prepare is the only place
> >>> when it enables the required clocks for audio block, then it is a
> >>> problem, we should do it early so that DSP can enable the required
> >>> configuration in prepare.
> >>
> >> As I wrote, previously it was done as a part of hw_params, which is
> >> also not optimal as hdmi-codec clearly defines both callbacks as
> >> optional, requiring only one of them. Do you know, what is actually
> >> required on the DP side for the DSP to be able to preapre()?
> > 
> > Srinivas, did you have a chance to look at this? Or the .hw_prepare
> > ordering? I really don't want to land this fix only to face the same
> > issue because on some of the platforms .hw_prepare() callbacks will be
> > called in a different order.
> > 
> Dmitry, few days back Krzysztof was enabling DP audio on T14s, and we
> have seen that the DSP is not happy and returning error 1 when we try to
> prepare the dp port. The exact reason is codec is not ready at that point.
> 
> 
> By adding this patch, Krzystof managed to get DP working.
> XElite platforms were working before 2fef64eec23a ("ASoC: hdmi-codec:
> Add a prepare) patch was merged, basically 2fef64eec23a broke what was
> working before.
> 
> Am happy with this patch, better move to hw_params instead of prepare.

And I'm not happy with it :-)

hdmi-codec defines that one of the callbacks can be optional. We know
that at .hw_params time we don't have IEC 958 status bits, so we will be
programming incorrect audioframe.

Should some bits of code be moved to .audio_enable isntead? If so, which
bits?

I agree that there is a regression and that this patch fixes it. I argue
that the fix is incorrect.

-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

[Srinivas Kandagatla]

On 10/23/25 1:24 PM, Dmitry Baryshkov wrote:
> On Thu, Oct 23, 2025 at 12:48:59PM +0100, Srinivas Kandagatla wrote:
>>
>>
>> On 10/23/25 12:43 PM, Dmitry Baryshkov wrote:
>>> On Sun, Oct 05, 2025 at 06:29:20PM +0300, Dmitry Baryshkov wrote:
>>>> On Sat, Oct 04, 2025 at 02:16:08PM +0100, Srinivas Kandagatla wrote:
>>>>>
>>>>>
>>>>> On 10/3/25 6:02 PM, Dmitry Baryshkov wrote:
>>>>>> On Fri, Oct 03, 2025 at 05:35:16PM +0100, Srinivas Kandagatla wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 9/26/25 4:09 PM, Dmitry Baryshkov wrote:
>>>>>>>> On Fri, Sep 26, 2025 at 11:30:26AM +0100, Srinivas Kandagatla wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 9/25/25 5:28 AM, Dmitry Baryshkov wrote:
>>>>>>>>>> On Thu, Sep 25, 2025 at 12:05:09PM +0800, Jianfeng Liu wrote:
>>>>>>>>>>> After reusing drm_hdmi_audio_* helpers and drm_bridge_connector
>>>>>>>>>>> integration in drm/msm/dp, we have dropped msm_dp_audio_hw_params and
>>>>>>>>>>> use msm_dp_audio_prepare instead. While userspace is still calling
>>>>>>>>>>> hw_params to do audio initialization, and we get the following errors:
>>>>>>>>>>>
>>>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>>>>>>>>> hdmi-audio-codec hdmi-audio-codec.0.auto: hdmi_codec_hw_params() started
>>>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
>>>>>>>>>>> qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
>>>>>>>>>>> qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
>>>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: Failed to start APM port 104
>>>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC error (-22): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0
>>>>>>>>>>> MultiMedia2 Playback: ASoC error (-22): at dpcm_run_update_startup() on MultiMedia2 Playback
>>>>>>>>>>
>>>>>>>>>> And a call to hdmi_codec_prepare() comes only at this place.
>>>>>>>>>>
>>>>>>>>>> Srini, Mark, when selecting to only implement .prepare for codec ops I
>>>>>>>>>> was following the commit 2fef64eec23a ("ASoC: hdmi-codec: Add a prepare
>>>>>>>>>> hook"), which documents that IEC958 status bit is set after
>>>>>>>>>> .hw_params(), so it's only visible during .prepare(). Is it okay to
>>>>>>>>>> implement both callbacks? Or should the audioreach DAI driver be fixed
>>>>>>>>>> somehow instead (I suppose it assumes that the port is available after
>>>>>>>>>> .hw_params(), not sure if that assumption is correct)?
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> msm_dp_audio_prepare is not called because hdmi-codec driver only checks
>>>>>>>>>>> and runs hw_params before q6apm_lpass_dai_prepare(). This commit will
>>>>>>>>>>> add hw_params callback same as drm_connector_hdmi_audio_prepare, so that
>>>>>>>>>>> hdmi-codec driver can work with userspace alsa.
>>>>>>>>>>>
>>>>>>>>>>> Tested with Radxa Dragon Q6A.
>>>>>>>>>>>
>>>>>>>>>>> Fixes: 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec implementation")
>>>>>>>>>>> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
>>>>>>>>>>
>>>>>>>>>> The patch LGTM, but I would wait for response from audio maintainers.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The ordering matters in this case as we need clocks and audio
>>>>>>>>> configuration on DP codec side to be setup before we start configuring
>>>>>>>>> the dsp pipeline. Looks like that DSP is trying to setup DP endpoint
>>>>>>>>> even before it is ready.
>>>>>>>>>
>>>>>>>>> q6apm prepare loads the dsp pipeline and starts configuring the
>>>>>>>>> endpoints, if the DP endpoint is not ready dsp would throw an error.
>>>>>>>>>
>>>>>>>>> We might be able to pull in some dsp logs to confirm this, but I dont
>>>>>>>>> have a setup that I can reproduce this issue.
>>>>>>>>
>>>>>>>> What would be your recommendation to proceed? Is it okay for the DAI
>>>>>>>> driver to depend on the .hw_params enabling the clock? Also I see that
>>>>>>>> the error regarding the clocks comes from .prepare callback too. What is
>>>>>>>> the order of .prepare callbacks()? Can we influence it?
>>>>>>>
>>>>>>> prepare follows hw-params, and prepare can be called multiple times
>>>>>>>
>>>>>>> When you mean order of prepare callbacks, you mean w.r.t codec and dsp
>>>>>>> backend dia link drivers ?
>>>>>>
>>>>>> Yes. Here we got a dependency from the cpu dai to be prepare()'d after
>>>>>> the DP driver performs some actions, which were a part of hw_params()
>>>>>> callback but were moved to be executed during prepare() callback.
>>>>>>
>>>>>> This leads me to two sets of questions:
>>>>>> - In which order are those callbacks executed? Can we make the ASoC
>>>>>>   enforce some order of DAI's prepare() callbacks?
>>>>>>
>>>>>> - More improtantly, isn't it a sympthom of DP driver (incorrectly)
>>>>>>   performing too much in the .hw_params() / .prepare() callback? Should
>>>>>>   we move some of the setup to the .audio_startup() instead? What is the
>>>>>>   expected split between those callbacks?
>>>>>
>>>>> I have not looked at full dp sequences but, if prepare is the only place
>>>>> when it enables the required clocks for audio block, then it is a
>>>>> problem, we should do it early so that DSP can enable the required
>>>>> configuration in prepare.
>>>>
>>>> As I wrote, previously it was done as a part of hw_params, which is
>>>> also not optimal as hdmi-codec clearly defines both callbacks as
>>>> optional, requiring only one of them. Do you know, what is actually
>>>> required on the DP side for the DSP to be able to preapre()?
>>>
>>> Srinivas, did you have a chance to look at this? Or the .hw_prepare
>>> ordering? I really don't want to land this fix only to face the same
>>> issue because on some of the platforms .hw_prepare() callbacks will be
>>> called in a different order.
>>>
>> Dmitry, few days back Krzysztof was enabling DP audio on T14s, and we
>> have seen that the DSP is not happy and returning error 1 when we try to
>> prepare the dp port. The exact reason is codec is not ready at that point.
>>
>>
>> By adding this patch, Krzystof managed to get DP working.
>> XElite platforms were working before 2fef64eec23a ("ASoC: hdmi-codec:
>> Add a prepare) patch was merged, basically 2fef64eec23a broke what was
>> working before.
>>
>> Am happy with this patch, better move to hw_params instead of prepare.
> 
> And I'm not happy with it :-)
> 
> hdmi-codec defines that one of the callbacks can be optional. We know
> that at .hw_params time we don't have IEC 958 status bits, so we will be
> programming incorrect audioframe.
Your statement is contradicting with callbacks that are supported by
hdmi-codec.

hdmi-codec uses IEC 958 status bits both in hw_params and prepare, if
hw_params is not the right place, then that should be clarified and not
correct for any of the codec drivers to use this callback.>
> Should some bits of code be moved to .audio_enable isntead? If so, which
> bits?

I have not read the data sheet of msm dp block yet, but what it looks to
me is the audio engine in the DP is powered off while dsp is trying to
set it up.

--srini>
> I agree that there is a regression and that this patch fixes it. I argue
> that the fix is incorrect.

> 

Re: [PATCH v2] drm/display: add hw_params callback function to drm_connector_hdmi_audio_ops

[Dmitry Baryshkov]

On Thu, Oct 23, 2025 at 01:47:33PM +0100, Srinivas Kandagatla wrote:
> 
> 
> On 10/23/25 1:24 PM, Dmitry Baryshkov wrote:
> > On Thu, Oct 23, 2025 at 12:48:59PM +0100, Srinivas Kandagatla wrote:
> >>
> >>
> >> On 10/23/25 12:43 PM, Dmitry Baryshkov wrote:
> >>> On Sun, Oct 05, 2025 at 06:29:20PM +0300, Dmitry Baryshkov wrote:
> >>>> On Sat, Oct 04, 2025 at 02:16:08PM +0100, Srinivas Kandagatla wrote:
> >>>>>
> >>>>>
> >>>>> On 10/3/25 6:02 PM, Dmitry Baryshkov wrote:
> >>>>>> On Fri, Oct 03, 2025 at 05:35:16PM +0100, Srinivas Kandagatla wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On 9/26/25 4:09 PM, Dmitry Baryshkov wrote:
> >>>>>>>> On Fri, Sep 26, 2025 at 11:30:26AM +0100, Srinivas Kandagatla wrote:
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> On 9/25/25 5:28 AM, Dmitry Baryshkov wrote:
> >>>>>>>>>> On Thu, Sep 25, 2025 at 12:05:09PM +0800, Jianfeng Liu wrote:
> >>>>>>>>>>> After reusing drm_hdmi_audio_* helpers and drm_bridge_connector
> >>>>>>>>>>> integration in drm/msm/dp, we have dropped msm_dp_audio_hw_params and
> >>>>>>>>>>> use msm_dp_audio_prepare instead. While userspace is still calling
> >>>>>>>>>>> hw_params to do audio initialization, and we get the following errors:
> >>>>>>>>>>>
> >>>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>>>>>>>>> hdmi-audio-codec hdmi-audio-codec.0.auto: hdmi_codec_hw_params() started
> >>>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: q6apm_lpass_dai_prepare() started
> >>>>>>>>>>> qcom-apm gprsvc:service:2:1: Error (1) Processing 0x01001002 cmd
> >>>>>>>>>>> qcom-apm gprsvc:service:2:1: DSP returned error[1001002] 1
> >>>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: Failed to start APM port 104
> >>>>>>>>>>> q6apm-lpass-dais 3700000.remoteproc:glink-edge:gpr:service@1:bedais: ASoC error (-22): at snd_soc_dai_prepare() on DISPLAY_PORT_RX_0
> >>>>>>>>>>> MultiMedia2 Playback: ASoC error (-22): at dpcm_run_update_startup() on MultiMedia2 Playback
> >>>>>>>>>>
> >>>>>>>>>> And a call to hdmi_codec_prepare() comes only at this place.
> >>>>>>>>>>
> >>>>>>>>>> Srini, Mark, when selecting to only implement .prepare for codec ops I
> >>>>>>>>>> was following the commit 2fef64eec23a ("ASoC: hdmi-codec: Add a prepare
> >>>>>>>>>> hook"), which documents that IEC958 status bit is set after
> >>>>>>>>>> .hw_params(), so it's only visible during .prepare(). Is it okay to
> >>>>>>>>>> implement both callbacks? Or should the audioreach DAI driver be fixed
> >>>>>>>>>> somehow instead (I suppose it assumes that the port is available after
> >>>>>>>>>> .hw_params(), not sure if that assumption is correct)?
> >>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> msm_dp_audio_prepare is not called because hdmi-codec driver only checks
> >>>>>>>>>>> and runs hw_params before q6apm_lpass_dai_prepare(). This commit will
> >>>>>>>>>>> add hw_params callback same as drm_connector_hdmi_audio_prepare, so that
> >>>>>>>>>>> hdmi-codec driver can work with userspace alsa.
> >>>>>>>>>>>
> >>>>>>>>>>> Tested with Radxa Dragon Q6A.
> >>>>>>>>>>>
> >>>>>>>>>>> Fixes: 98a8920e7b07 ("drm/msm/dp: reuse generic HDMI codec implementation")
> >>>>>>>>>>> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
> >>>>>>>>>>
> >>>>>>>>>> The patch LGTM, but I would wait for response from audio maintainers.
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> The ordering matters in this case as we need clocks and audio
> >>>>>>>>> configuration on DP codec side to be setup before we start configuring
> >>>>>>>>> the dsp pipeline. Looks like that DSP is trying to setup DP endpoint
> >>>>>>>>> even before it is ready.
> >>>>>>>>>
> >>>>>>>>> q6apm prepare loads the dsp pipeline and starts configuring the
> >>>>>>>>> endpoints, if the DP endpoint is not ready dsp would throw an error.
> >>>>>>>>>
> >>>>>>>>> We might be able to pull in some dsp logs to confirm this, but I dont
> >>>>>>>>> have a setup that I can reproduce this issue.
> >>>>>>>>
> >>>>>>>> What would be your recommendation to proceed? Is it okay for the DAI
> >>>>>>>> driver to depend on the .hw_params enabling the clock? Also I see that
> >>>>>>>> the error regarding the clocks comes from .prepare callback too. What is
> >>>>>>>> the order of .prepare callbacks()? Can we influence it?
> >>>>>>>
> >>>>>>> prepare follows hw-params, and prepare can be called multiple times
> >>>>>>>
> >>>>>>> When you mean order of prepare callbacks, you mean w.r.t codec and dsp
> >>>>>>> backend dia link drivers ?
> >>>>>>
> >>>>>> Yes. Here we got a dependency from the cpu dai to be prepare()'d after
> >>>>>> the DP driver performs some actions, which were a part of hw_params()
> >>>>>> callback but were moved to be executed during prepare() callback.
> >>>>>>
> >>>>>> This leads me to two sets of questions:
> >>>>>> - In which order are those callbacks executed? Can we make the ASoC
> >>>>>>   enforce some order of DAI's prepare() callbacks?
> >>>>>>
> >>>>>> - More improtantly, isn't it a sympthom of DP driver (incorrectly)
> >>>>>>   performing too much in the .hw_params() / .prepare() callback? Should
> >>>>>>   we move some of the setup to the .audio_startup() instead? What is the
> >>>>>>   expected split between those callbacks?
> >>>>>
> >>>>> I have not looked at full dp sequences but, if prepare is the only place
> >>>>> when it enables the required clocks for audio block, then it is a
> >>>>> problem, we should do it early so that DSP can enable the required
> >>>>> configuration in prepare.
> >>>>
> >>>> As I wrote, previously it was done as a part of hw_params, which is
> >>>> also not optimal as hdmi-codec clearly defines both callbacks as
> >>>> optional, requiring only one of them. Do you know, what is actually
> >>>> required on the DP side for the DSP to be able to preapre()?
> >>>
> >>> Srinivas, did you have a chance to look at this? Or the .hw_prepare
> >>> ordering? I really don't want to land this fix only to face the same
> >>> issue because on some of the platforms .hw_prepare() callbacks will be
> >>> called in a different order.
> >>>
> >> Dmitry, few days back Krzysztof was enabling DP audio on T14s, and we
> >> have seen that the DSP is not happy and returning error 1 when we try to
> >> prepare the dp port. The exact reason is codec is not ready at that point.
> >>
> >>
> >> By adding this patch, Krzystof managed to get DP working.
> >> XElite platforms were working before 2fef64eec23a ("ASoC: hdmi-codec:
> >> Add a prepare) patch was merged, basically 2fef64eec23a broke what was
> >> working before.
> >>
> >> Am happy with this patch, better move to hw_params instead of prepare.
> > 
> > And I'm not happy with it :-)
> > 
> > hdmi-codec defines that one of the callbacks can be optional. We know
> > that at .hw_params time we don't have IEC 958 status bits, so we will be
> > programming incorrect audioframe.
> Your statement is contradicting with callbacks that are supported by
> hdmi-codec.
> 
> hdmi-codec uses IEC 958 status bits both in hw_params and prepare, if
> hw_params is not the right place, then that should be clarified and not
> correct for any of the codec drivers to use this callback.>

See https://lore.kernel.org/all/20210525132354.297468-6-maxime@cerno.tech/

> > Should some bits of code be moved to .audio_enable isntead? If so, which
> > bits?
> 
> I have not read the data sheet of msm dp block yet, but what it looks to
> me is the audio engine in the DP is powered off while dsp is trying to
> set it up.

Okay. What exactly is expected by the DSP? Which part, signal or clock
should be on?

-- 
With best wishes
Dmitry