Re: [PATCH v3 3/4] ASoC: qcom: qdsp6: Set channel mapping instead of fixed defaults

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



Thanks Krzysztof for the patch.


On 09/05/2024 07:51, Krzysztof Kozlowski wrote:
When constructing packets to DSP, the Audioreach code uses 'struct
audioreach_module_config' to configure parameters like number of
channels, bitrate, sample rate etc, but uses defaults for the channel
mapping.

Rework this code to copy the channel mapping from 'struct
audioreach_module_config', instead of using the default.  This requires
all callers to fill that structure: add missing initialization of
channel mapping.
Adding this new function call is logically fine but its going to introducing some sequencing issues.

set_channel_map might be overwritten by this if not done correctly.

One such instance is in this patch..


Entire patch makes code more logical and easier to follow:
1. q6apm-dai and q6apm-lpass-dais code which allocates 'struct
    audioreach_module_config' initializes it fully, so fills both
    the number of channels and the channel mapping.
2. Audioreach code, which uses 'struct audioreach_module_config' when
    constructing packets, copies entire contents of passed config, not
    only pieces of it.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>

---

Changes in v2:
1. Extend commit msg, rationale.
2. Rework to new approach, but most of the code stays.
3. Export audioreach_set_channel_mapping() (needed by Q6APM DAIS and
    LPASS DAIS)
4. Correct channel mapping also in audioreach_mfc_set_media_format(),
    because MFC DAI is now part of backend.
5. Do not adjust dynamic DAIs (drop audioreach_dai_load()).
---
  sound/soc/qcom/qdsp6/audioreach.c       | 30 +++++++-----------------------
  sound/soc/qcom/qdsp6/audioreach.h       |  1 +
  sound/soc/qcom/qdsp6/q6apm-dai.c        |  2 ++
  sound/soc/qcom/qdsp6/q6apm-lpass-dais.c |  5 ++++-
  4 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
index 5291deac0a0b..750b8ba64211 100644
--- a/sound/soc/qcom/qdsp6/audioreach.c
+++ b/sound/soc/qcom/qdsp6/audioreach.c
@@ -267,7 +267,7 @@ void *audioreach_alloc_apm_cmd_pkt(int pkt_size, uint32_t opcode, uint32_t token
  }
  EXPORT_SYMBOL_GPL(audioreach_alloc_apm_cmd_pkt);
-static void audioreach_set_channel_mapping(u8 *ch_map, int num_channels)
+void audioreach_set_channel_mapping(u8 *ch_map, int num_channels)
  {
  	if (num_channels == 1) {
  		ch_map[0] =  PCM_CHANNEL_FL;
@@ -281,6 +281,7 @@ static void audioreach_set_channel_mapping(u8 *ch_map, int num_channels)
  		ch_map[3] =  PCM_CHANNEL_RS;
  	}
  }
+EXPORT_SYMBOL_GPL(audioreach_set_channel_mapping);
static void apm_populate_container_config(struct apm_container_obj *cfg,
  					  struct audioreach_container *cont)
@@ -819,7 +820,7 @@ static int audioreach_mfc_set_media_format(struct q6apm_graph *graph,
  	uint32_t num_channels = cfg->num_channels;
  	int payload_size;
  	struct gpr_pkt *pkt;
-	int rc;
+	int rc, i;
  	void *p;
payload_size = APM_MFC_CFG_PSIZE(media_format, num_channels) +
@@ -842,18 +843,8 @@ static int audioreach_mfc_set_media_format(struct q6apm_graph *graph,
  	media_format->sample_rate = cfg->sample_rate;
  	media_format->bit_width = cfg->bit_width;
  	media_format->num_channels = cfg->num_channels;
-
-	if (num_channels == 1) {
-		media_format->channel_mapping[0] = PCM_CHANNEL_FL;
-	} else if (num_channels == 2) {
-		media_format->channel_mapping[0] = PCM_CHANNEL_FL;
-		media_format->channel_mapping[1] = PCM_CHANNEL_FR;
-	} else if (num_channels == 4) {
-		media_format->channel_mapping[0] = PCM_CHANNEL_FL;
-		media_format->channel_mapping[1] = PCM_CHANNEL_FR;
-		media_format->channel_mapping[2] = PCM_CHANNEL_LS;
-		media_format->channel_mapping[3] = PCM_CHANNEL_RS;
-	}
+	for (i = 0; i < num_channels; i++)
+		media_format->channel_mapping[i] = cfg->channel_map[i];
rc = q6apm_send_cmd_sync(graph->apm, pkt, 0); @@ -883,9 +874,6 @@ static int audioreach_set_compr_media_format(struct media_format *media_fmt_hdr,
  		mp3_cfg->q_factor = mcfg->bit_width - 1;
  		mp3_cfg->endianness = PCM_LITTLE_ENDIAN;
  		mp3_cfg->num_channels = mcfg->num_channels;
-
-		audioreach_set_channel_mapping(mp3_cfg->channel_mapping,
-					       mcfg->num_channels);
  		break;
  	case SND_AUDIOCODEC_AAC:
  		media_fmt_hdr->data_format = DATA_FORMAT_RAW_COMPRESSED;
@@ -1104,9 +1092,7 @@ static int audioreach_pcm_set_media_format(struct q6apm_graph *graph,
  	media_cfg->num_channels = mcfg->num_channels;
  	media_cfg->q_factor = mcfg->bit_width - 1;
  	media_cfg->bits_per_sample = mcfg->bit_width;
-
-	audioreach_set_channel_mapping(media_cfg->channel_mapping,
-				       num_channels);
+	memcpy(media_cfg->channel_mapping, mcfg->channel_map, mcfg->num_channels);
rc = q6apm_send_cmd_sync(graph->apm, pkt, 0); @@ -1163,9 +1149,7 @@ static int audioreach_shmem_set_media_format(struct q6apm_graph *graph,
  		cfg->q_factor = mcfg->bit_width - 1;
  		cfg->endianness = PCM_LITTLE_ENDIAN;
  		cfg->num_channels = mcfg->num_channels;
-
-		audioreach_set_channel_mapping(cfg->channel_mapping,
-					       num_channels);
+		memcpy(cfg->channel_mapping, mcfg->channel_map, mcfg->num_channels);
  	} else {
  		rc = audioreach_set_compr_media_format(header, p, mcfg);
  		if (rc) {
diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
index eb9306280988..208b74e50445 100644
--- a/sound/soc/qcom/qdsp6/audioreach.h
+++ b/sound/soc/qcom/qdsp6/audioreach.h
@@ -766,6 +766,7 @@ struct audioreach_module_config {
  /* Packet Allocation routines */
  void *audioreach_alloc_apm_cmd_pkt(int pkt_size, uint32_t opcode, uint32_t
  				    token);
+void audioreach_set_channel_mapping(u8 *ch_map, int num_channels);
  void *audioreach_alloc_cmd_pkt(int payload_size, uint32_t opcode,
  			       uint32_t token, uint32_t src_port,
  			       uint32_t dest_port);
diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
index 00bbd291be5c..8ab55869e8a2 100644
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -243,6 +243,7 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
  	cfg.num_channels = runtime->channels;
  	cfg.bit_width = prtd->bits_per_sample;
  	cfg.fmt = SND_AUDIOCODEC_PCM;
+	audioreach_set_channel_mapping(cfg.channel_map, runtime->channels);

Prepare can be called multiple times.. so we have channels overwritten here.

--srini
  	if (prtd->state) {
  		/* clear the previous setup if any  */
@@ -669,6 +670,7 @@ static int q6apm_dai_compr_set_params(struct snd_soc_component *component,
  		cfg.num_channels = 2;
  		cfg.bit_width = prtd->bits_per_sample;
  		cfg.fmt = codec->id;
+		audioreach_set_channel_mapping(cfg.channel_map, cfg.num_channels);
  		memcpy(&cfg.codec, codec, sizeof(*codec));
ret = q6apm_graph_media_format_shmem(prtd->graph, &cfg);
diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
index a4ad1d0e6abd..8340e4fb78f4 100644
--- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
+++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
@@ -106,6 +106,7 @@ static int q6hdmi_hw_params(struct snd_pcm_substream *substream,
  	cfg->bit_width = params_width(params);
  	cfg->sample_rate = params_rate(params);
  	cfg->num_channels = channels;
+	audioreach_set_channel_mapping(cfg->channel_map, channels);
switch (dai->id) {
  	case DISPLAY_PORT_RX_0:
@@ -130,10 +131,12 @@ static int q6dma_hw_params(struct snd_pcm_substream *substream,
  {
  	struct q6apm_lpass_dai_data *dai_data = dev_get_drvdata(dai->dev);
  	struct audioreach_module_config *cfg = &dai_data->module_config[dai->id];
+	int channels = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS)->max;
cfg->bit_width = params_width(params);
  	cfg->sample_rate = params_rate(params);
-	cfg->num_channels = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS)->max;
+	cfg->num_channels = channels;
+	audioreach_set_channel_mapping(cfg->channel_map, channels);
return 0;
  }





[Index of Archives]     [Pulseaudio]     [Linux Audio Users]     [ALSA Devel]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux