[PATCH] ASoC: bdw-rt5677: enable runtime channel merge

[PATCH] ASoC: bdw-rt5677: enable runtime channel merge

[Brent Lu]

In the DAI link "Capture PCM", the FE DAI "Capture Pin" supports 4-channel
capture but the BE DAI supports only 2-channel capture. To fix the channel
mismatch, we need to enable the runtime channel merge for this DAI link.

Signed-off-by: Brent Lu <brent.lu@intel.com>
---
 sound/soc/intel/boards/bdw-rt5677.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c
index 4a4d335..8d07038 100644
--- a/sound/soc/intel/boards/bdw-rt5677.c
+++ b/sound/soc/intel/boards/bdw-rt5677.c
@@ -273,6 +273,7 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = {
 		},
 		.dpcm_capture = 1,
 		.dpcm_playback = 1,
+		.dpcm_merged_chan = 1,
 		SND_SOC_DAILINK_REG(fe, dummy, platform),
 	},
 
-- 
2.7.4

RE: [PATCH] ASoC: bdw-rt5677: enable runtime channel merge

[Lu, Brent]

> Subject: [PATCH] ASoC: bdw-rt5677: enable runtime channel merge
> 
> In the DAI link "Capture PCM", the FE DAI "Capture Pin" supports 4-channel
> capture but the BE DAI supports only 2-channel capture. To fix the channel
> mismatch, we need to enable the runtime channel merge for this DAI link.
> 

Hi Pierre,

This patch is for the same issue discussed in the following thread:
https://patchwork.kernel.org/patch/11134167/

We enable the runtime channel merge for the DMIC DAI instead of adding a
machine driver constraint. It's working good on chrome's 3.14 branch (which
requires some backport for the runtime channel merge feature). Please let
me know if this implementation is correct for the FE/BE mismatch problem.
Thanks.


Regards,
Brent

Re: [alsa-devel] [PATCH] ASoC: bdw-rt5677: enable runtime channel merge

[Pierre-Louis Bossart]

>> In the DAI link "Capture PCM", the FE DAI "Capture Pin" supports 4-channel
>> capture but the BE DAI supports only 2-channel capture. To fix the channel
>> mismatch, we need to enable the runtime channel merge for this DAI link.
>>
> 
> Hi Pierre,
> 
> This patch is for the same issue discussed in the following thread:
> https://patchwork.kernel.org/patch/11134167/
> 
> We enable the runtime channel merge for the DMIC DAI instead of adding a
> machine driver constraint. It's working good on chrome's 3.14 branch (which
> requires some backport for the runtime channel merge feature). Please let
> me know if this implementation is correct for the FE/BE mismatch problem.

Sorry, I don't fully understand your points, and it's the first time I 
see anyone use this .dpcm_merged_chan field for an Intel platform.

If I look at the code I see that the core would limit the number of 
channels to two. But that code needs the CPU_DAI to use 2 channels, 
which I don't see. So is this patch self-contained or do we need an 
additional constraint on the FE?

Thanks
-Pierre

RE: [alsa-devel] [PATCH] ASoC: bdw-rt5677: enable runtime channel merge

[Lu, Brent]

> >> In the DAI link "Capture PCM", the FE DAI "Capture Pin" supports
> >> 4-channel capture but the BE DAI supports only 2-channel capture. To
> >> fix the channel mismatch, we need to enable the runtime channel merge
> for this DAI link.
> >>
> >
> > Hi Pierre,
> >
> > This patch is for the same issue discussed in the following thread:
> > https://patchwork.kernel.org/patch/11134167/
> >
> > We enable the runtime channel merge for the DMIC DAI instead of adding
> > a machine driver constraint. It's working good on chrome's 3.14 branch
> > (which requires some backport for the runtime channel merge feature).
> > Please let me know if this implementation is correct for the FE/BE mismatch
> problem.
> 
> Sorry, I don't fully understand your points, and it's the first time I see anyone
> use this .dpcm_merged_chan field for an Intel platform.
> 
> If I look at the code I see that the core would limit the number of channels to
> two. But that code needs the CPU_DAI to use 2 channels, which I don't see.
> So is this patch self-contained or do we need an additional constraint on the
> FE?
> 
> Thanks
> -Pierre

Hi Pierre,

We don't need constraint on FE because dpcm_runtime_merge_chan() modifies
the channel number of snd_pcm_hardware structure directly. The structure will
be used to initialize the snd_pcm_hw_constraints structure later in the
snd_pcm_hw_constraints_complete() function. Since the channel number is already
modified, we don't need a constraint to install an extra rule for it.

The result of using dpcm_merged_chan flag and machine driver constraint should
be the same when user space programs calling HW_REFINE ioctl call but I think the
flag is more elegant for machine driver code.


Regards,
Brent

Re: [alsa-devel] [PATCH] ASoC: bdw-rt5677: enable runtime channel merge

[Pierre-Louis Bossart]

On 11/5/19 12:03 AM, Lu, Brent wrote:
>>>> In the DAI link "Capture PCM", the FE DAI "Capture Pin" supports
>>>> 4-channel capture but the BE DAI supports only 2-channel capture. To
>>>> fix the channel mismatch, we need to enable the runtime channel merge
>> for this DAI link.
>>>>
>>>
>>> Hi Pierre,
>>>
>>> This patch is for the same issue discussed in the following thread:
>>> https://patchwork.kernel.org/patch/11134167/
>>>
>>> We enable the runtime channel merge for the DMIC DAI instead of adding
>>> a machine driver constraint. It's working good on chrome's 3.14 branch
>>> (which requires some backport for the runtime channel merge feature).
>>> Please let me know if this implementation is correct for the FE/BE mismatch
>> problem.
>>
>> Sorry, I don't fully understand your points, and it's the first time I see anyone
>> use this .dpcm_merged_chan field for an Intel platform.
>>
>> If I look at the code I see that the core would limit the number of channels to
>> two. But that code needs the CPU_DAI to use 2 channels, which I don't see.
>> So is this patch self-contained or do we need an additional constraint on the
>> FE?
>>
>> Thanks
>> -Pierre
> 
> Hi Pierre,
> 
> We don't need constraint on FE because dpcm_runtime_merge_chan() modifies
> the channel number of snd_pcm_hardware structure directly. The structure will
> be used to initialize the snd_pcm_hw_constraints structure later in the
> snd_pcm_hw_constraints_complete() function. Since the channel number is already
> modified, we don't need a constraint to install an extra rule for it.
> 
> The result of using dpcm_merged_chan flag and machine driver constraint should
> be the same when user space programs calling HW_REFINE ioctl call but I think the
> flag is more elegant for machine driver code.

I could use the opposite argument: by using a capability that no one 
else uses, you make the solution more obscure and less intuitive. All 
other machine drivers use constraints, so if the two solutions are 
equivalent let's follow the more common solution.