From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>, bjorn.andersson@linaro.org, broonie@kernel.org, robh@kernel.org Cc: plai@codeaurora.org, tiwai@suse.de, devicetree@vger.kernel.org, perex@perex.cz, alsa-devel@alsa-project.org, lgirdwood@gmail.com, bgoswami@codeaurora.org Subject: Re: [PATCH v6 16/22] ASoC: qdsp6: audioreach: add module configuration command helpers Date: Wed, 15 Sep 2021 11:11:41 -0500 [thread overview] Message-ID: <4cd0e63f-107b-d10a-11e9-bced83f487a5@linux.intel.com> (raw) In-Reply-To: <20210915131333.19047-17-srinivas.kandagatla@linaro.org> > +int q6apm_map_memory_regions(struct q6apm_graph *graph, unsigned int dir, phys_addr_t phys, > + size_t period_sz, unsigned int periods) > +{ > + struct audioreach_graph_data *data; > + struct audio_buffer *buf; > + int cnt; > + int rc; > + > + if (dir == SNDRV_PCM_STREAM_PLAYBACK) > + data = &graph->rx_data; > + else > + data = &graph->tx_data; > + > + mutex_lock(&graph->lock); > + > + if (data->buf) { > + dev_err(graph->dev, "Buffer already allocated\n"); > + mutex_unlock(&graph->lock); > + return 0; is this an error worth of dev_err() if you return 0? > + } > + > + buf = kzalloc(((sizeof(struct audio_buffer)) * periods), GFP_KERNEL); > + if (!buf) { > + mutex_unlock(&graph->lock); > + return -ENOMEM; > + } > + > + if (dir == SNDRV_PCM_STREAM_PLAYBACK) > + data = &graph->rx_data; > + else > + data = &graph->tx_data; > + > + data->buf = buf; > + > + buf[0].phys = phys; > + buf[0].size = period_sz; > + > + for (cnt = 1; cnt < periods; cnt++) { > + if (period_sz > 0) { > + buf[cnt].phys = buf[0].phys + (cnt * period_sz); > + buf[cnt].size = period_sz; > + } > + } > + data->num_periods = periods; > + > + mutex_unlock(&graph->lock); > + > + rc = audioreach_map_memory_regions(graph, dir, period_sz, periods, 1); > + if (rc < 0) { > + dev_err(graph->dev, "Memory_map_regions failed\n"); > + audioreach_graph_free_buf(graph); > + } > + > + return rc; > +} > +EXPORT_SYMBOL_GPL(q6apm_map_memory_regions); > +int q6apm_write_async(struct q6apm_graph *graph, uint32_t len, uint32_t msw_ts, > + uint32_t lsw_ts, uint32_t wflags) > +{ > + struct apm_data_cmd_wr_sh_mem_ep_data_buffer_v2 *write; > + int rc, payload_size, iid; > + struct audio_buffer *ab; > + struct gpr_pkt *pkt; > + > + payload_size = sizeof(*write); nit-pick on variable-naming: I get confused between actions and objects. > + > + iid = q6apm_graph_get_rx_shmem_module_iid(graph); > + pkt = audioreach_alloc_pkt(payload_size, DATA_CMD_WR_SH_MEM_EP_DATA_BUFFER_V2, > + graph->rx_data.dsp_buf | (len << APM_WRITE_TOKEN_LEN_SHIFT), > + graph->port->id, iid); > + if (IS_ERR(pkt)) > + return -ENOMEM; > + > + write = (void *)pkt + GPR_HDR_SIZE; > + > + mutex_lock(&graph->lock); > + ab = &graph->rx_data.buf[graph->rx_data.dsp_buf]; > + > + write->buf_addr_lsw = lower_32_bits(ab->phys); > + write->buf_addr_msw = upper_32_bits(ab->phys); > + write->buf_size = len; > + write->timestamp_lsw = lsw_ts; > + write->timestamp_msw = msw_ts; > + write->mem_map_handle = graph->rx_data.mem_map_handle; > + write->flags = wflags; > + > + graph->rx_data.dsp_buf++; > + > + if (graph->rx_data.dsp_buf >= graph->rx_data.num_periods) > + graph->rx_data.dsp_buf = 0; > + > + mutex_unlock(&graph->lock); > + > + rc = gpr_send_port_pkt(graph->port, pkt); > + > + kfree(pkt); > + > + return rc; > +} > +EXPORT_SYMBOL_GPL(q6apm_write_async); > + > +int q6apm_read(struct q6apm_graph *graph) > +{ > + struct data_cmd_rd_sh_mem_ep_data_buffer_v2 *read; > + struct audioreach_graph_data *port; > + struct audio_buffer *ab; > + struct gpr_pkt *pkt; > + int rc, iid; > + > + iid = q6apm_graph_get_tx_shmem_module_iid(graph); > + pkt = audioreach_alloc_pkt(sizeof(*read), DATA_CMD_RD_SH_MEM_EP_DATA_BUFFER_V2, > + graph->tx_data.dsp_buf, graph->port->id, iid); > + if (IS_ERR(pkt)) > + return -ENOMEM; > + > + read = (void *)pkt + GPR_HDR_SIZE; same nit-pick on variable naming, with the additional present/past grammar issue that you don't know if it's a read buffer or a pointer to data read in the past. > + > + mutex_lock(&graph->lock); > + port = &graph->tx_data; > + ab = &port->buf[port->dsp_buf]; > + > + read->buf_addr_lsw = lower_32_bits(ab->phys); > + read->buf_addr_msw = upper_32_bits(ab->phys); > + read->mem_map_handle = port->mem_map_handle; > + read->buf_size = ab->size; > + > + port->dsp_buf++; > + > + if (port->dsp_buf >= port->num_periods) > + port->dsp_buf = 0; > + > + mutex_unlock(&graph->lock); > + > + rc = gpr_send_port_pkt(graph->port, pkt); > + kfree(pkt); > + > + return rc; > +} > +EXPORT_SYMBOL_GPL(q6apm_read); > + > static int graph_callback(struct gpr_resp_pkt *data, void *priv, int op) > { > struct data_cmd_rsp_rd_sh_mem_ep_data_buffer_done_v2 *rd_done; >
WARNING: multiple messages have this Message-ID (diff)
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>, bjorn.andersson@linaro.org, broonie@kernel.org, robh@kernel.org Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org, bgoswami@codeaurora.org, tiwai@suse.de, plai@codeaurora.org, lgirdwood@gmail.com Subject: Re: [PATCH v6 16/22] ASoC: qdsp6: audioreach: add module configuration command helpers Date: Wed, 15 Sep 2021 11:11:41 -0500 [thread overview] Message-ID: <4cd0e63f-107b-d10a-11e9-bced83f487a5@linux.intel.com> (raw) In-Reply-To: <20210915131333.19047-17-srinivas.kandagatla@linaro.org> > +int q6apm_map_memory_regions(struct q6apm_graph *graph, unsigned int dir, phys_addr_t phys, > + size_t period_sz, unsigned int periods) > +{ > + struct audioreach_graph_data *data; > + struct audio_buffer *buf; > + int cnt; > + int rc; > + > + if (dir == SNDRV_PCM_STREAM_PLAYBACK) > + data = &graph->rx_data; > + else > + data = &graph->tx_data; > + > + mutex_lock(&graph->lock); > + > + if (data->buf) { > + dev_err(graph->dev, "Buffer already allocated\n"); > + mutex_unlock(&graph->lock); > + return 0; is this an error worth of dev_err() if you return 0? > + } > + > + buf = kzalloc(((sizeof(struct audio_buffer)) * periods), GFP_KERNEL); > + if (!buf) { > + mutex_unlock(&graph->lock); > + return -ENOMEM; > + } > + > + if (dir == SNDRV_PCM_STREAM_PLAYBACK) > + data = &graph->rx_data; > + else > + data = &graph->tx_data; > + > + data->buf = buf; > + > + buf[0].phys = phys; > + buf[0].size = period_sz; > + > + for (cnt = 1; cnt < periods; cnt++) { > + if (period_sz > 0) { > + buf[cnt].phys = buf[0].phys + (cnt * period_sz); > + buf[cnt].size = period_sz; > + } > + } > + data->num_periods = periods; > + > + mutex_unlock(&graph->lock); > + > + rc = audioreach_map_memory_regions(graph, dir, period_sz, periods, 1); > + if (rc < 0) { > + dev_err(graph->dev, "Memory_map_regions failed\n"); > + audioreach_graph_free_buf(graph); > + } > + > + return rc; > +} > +EXPORT_SYMBOL_GPL(q6apm_map_memory_regions); > +int q6apm_write_async(struct q6apm_graph *graph, uint32_t len, uint32_t msw_ts, > + uint32_t lsw_ts, uint32_t wflags) > +{ > + struct apm_data_cmd_wr_sh_mem_ep_data_buffer_v2 *write; > + int rc, payload_size, iid; > + struct audio_buffer *ab; > + struct gpr_pkt *pkt; > + > + payload_size = sizeof(*write); nit-pick on variable-naming: I get confused between actions and objects. > + > + iid = q6apm_graph_get_rx_shmem_module_iid(graph); > + pkt = audioreach_alloc_pkt(payload_size, DATA_CMD_WR_SH_MEM_EP_DATA_BUFFER_V2, > + graph->rx_data.dsp_buf | (len << APM_WRITE_TOKEN_LEN_SHIFT), > + graph->port->id, iid); > + if (IS_ERR(pkt)) > + return -ENOMEM; > + > + write = (void *)pkt + GPR_HDR_SIZE; > + > + mutex_lock(&graph->lock); > + ab = &graph->rx_data.buf[graph->rx_data.dsp_buf]; > + > + write->buf_addr_lsw = lower_32_bits(ab->phys); > + write->buf_addr_msw = upper_32_bits(ab->phys); > + write->buf_size = len; > + write->timestamp_lsw = lsw_ts; > + write->timestamp_msw = msw_ts; > + write->mem_map_handle = graph->rx_data.mem_map_handle; > + write->flags = wflags; > + > + graph->rx_data.dsp_buf++; > + > + if (graph->rx_data.dsp_buf >= graph->rx_data.num_periods) > + graph->rx_data.dsp_buf = 0; > + > + mutex_unlock(&graph->lock); > + > + rc = gpr_send_port_pkt(graph->port, pkt); > + > + kfree(pkt); > + > + return rc; > +} > +EXPORT_SYMBOL_GPL(q6apm_write_async); > + > +int q6apm_read(struct q6apm_graph *graph) > +{ > + struct data_cmd_rd_sh_mem_ep_data_buffer_v2 *read; > + struct audioreach_graph_data *port; > + struct audio_buffer *ab; > + struct gpr_pkt *pkt; > + int rc, iid; > + > + iid = q6apm_graph_get_tx_shmem_module_iid(graph); > + pkt = audioreach_alloc_pkt(sizeof(*read), DATA_CMD_RD_SH_MEM_EP_DATA_BUFFER_V2, > + graph->tx_data.dsp_buf, graph->port->id, iid); > + if (IS_ERR(pkt)) > + return -ENOMEM; > + > + read = (void *)pkt + GPR_HDR_SIZE; same nit-pick on variable naming, with the additional present/past grammar issue that you don't know if it's a read buffer or a pointer to data read in the past. > + > + mutex_lock(&graph->lock); > + port = &graph->tx_data; > + ab = &port->buf[port->dsp_buf]; > + > + read->buf_addr_lsw = lower_32_bits(ab->phys); > + read->buf_addr_msw = upper_32_bits(ab->phys); > + read->mem_map_handle = port->mem_map_handle; > + read->buf_size = ab->size; > + > + port->dsp_buf++; > + > + if (port->dsp_buf >= port->num_periods) > + port->dsp_buf = 0; > + > + mutex_unlock(&graph->lock); > + > + rc = gpr_send_port_pkt(graph->port, pkt); > + kfree(pkt); > + > + return rc; > +} > +EXPORT_SYMBOL_GPL(q6apm_read); > + > static int graph_callback(struct gpr_resp_pkt *data, void *priv, int op) > { > struct data_cmd_rsp_rd_sh_mem_ep_data_buffer_done_v2 *rd_done; >
next prev parent reply other threads:[~2021-09-15 16:34 UTC|newest] Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-15 13:13 [PATCH v6 00/22] ASoC: qcom: Add AudioReach support Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 01/22] soc: dt-bindings: qcom: apr: convert to yaml Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 02/22] soc: dt-bindings: qcom: apr: deprecate qcom,apr-domain property Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 02/22] soc: dt-bindings: qcom: apr: deprecate qcom, apr-domain property Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 03/22] soc: qcom: apr: make code more reuseable Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 04/22] soc: dt-bindings: qcom: add gpr bindings Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-16 20:03 ` Rob Herring 2021-09-16 20:03 ` Rob Herring 2021-09-15 13:13 ` [PATCH v6 05/22] soc: qcom: apr: Add GPR support Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 15:47 ` Pierre-Louis Bossart 2021-09-15 15:47 ` Pierre-Louis Bossart 2021-09-16 15:02 ` Srinivas Kandagatla 2021-09-16 15:02 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 06/22] ASoC: dt-bindings: move LPASS dai related bindings out of q6afe Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 07/22] ASoC: dt-bindings: move LPASS clocks " Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 08/22] ASoC: dt-bindings: rename q6afe.h to q6dsp-lpass-ports.h Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 09/22] ASoC: qdsp6: q6afe-dai: move lpass audio ports to common file Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 10/22] ASoC: qdsp6: q6afe-clocks: move audio-clocks " Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 11/22] ASoC: dt-bindings: q6dsp: add q6apm-lpass-dai compatible Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 12/22] ASoC: dt-bindings: lpass-clocks: add q6prm clocks compatible Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 13/22] ASoC: dt-bindings: add q6apm digital audio stream bindings Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-16 20:05 ` Rob Herring 2021-09-16 20:05 ` Rob Herring 2021-09-15 13:13 ` [PATCH v6 14/22] ASoC: qdsp6: audioreach: add basic pkt alloc support Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 15:54 ` Pierre-Louis Bossart 2021-09-15 15:54 ` Pierre-Louis Bossart 2021-09-16 15:02 ` Srinivas Kandagatla 2021-09-16 15:02 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 15/22] ASoC: qdsp6: audioreach: add q6apm support Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 16:02 ` Pierre-Louis Bossart 2021-09-15 16:02 ` Pierre-Louis Bossart 2021-09-16 15:02 ` Srinivas Kandagatla 2021-09-16 15:02 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 16/22] ASoC: qdsp6: audioreach: add module configuration command helpers Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 16:11 ` Pierre-Louis Bossart [this message] 2021-09-15 16:11 ` Pierre-Louis Bossart 2021-09-16 15:02 ` Srinivas Kandagatla 2021-09-16 15:02 ` Srinivas Kandagatla 2021-09-16 15:40 ` Pierre-Louis Bossart 2021-09-16 15:40 ` Pierre-Louis Bossart 2021-09-15 13:13 ` [PATCH v6 17/22] ASoC: qdsp6: audioreach: add Kconfig and Makefile Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 18/22] ASoC: qdsp6: audioreach: add topology support Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 16:22 ` Pierre-Louis Bossart 2021-09-15 16:22 ` Pierre-Louis Bossart 2021-09-16 15:02 ` Srinivas Kandagatla 2021-09-16 15:02 ` Srinivas Kandagatla 2021-09-16 16:28 ` Pierre-Louis Bossart 2021-09-16 16:28 ` Pierre-Louis Bossart 2021-09-15 13:13 ` [PATCH v6 19/22] ASoC: qdsp6: audioreach: add q6apm-dai support Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 20/22] ASoC: qdsp6: audioreach: add q6apm lpass dai support Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 21/22] ASoC: qdsp6: audioreach: add q6prm support Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 13:13 ` [PATCH v6 22/22] ASoC: qdsp6: audioreach: add support for q6prm-clocks Srinivas Kandagatla 2021-09-15 13:13 ` Srinivas Kandagatla 2021-09-15 16:32 ` [PATCH v6 00/22] ASoC: qcom: Add AudioReach support Pierre-Louis Bossart 2021-09-15 16:32 ` Pierre-Louis Bossart
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4cd0e63f-107b-d10a-11e9-bced83f487a5@linux.intel.com \ --to=pierre-louis.bossart@linux.intel.com \ --cc=alsa-devel@alsa-project.org \ --cc=bgoswami@codeaurora.org \ --cc=bjorn.andersson@linaro.org \ --cc=broonie@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=lgirdwood@gmail.com \ --cc=perex@perex.cz \ --cc=plai@codeaurora.org \ --cc=robh@kernel.org \ --cc=srinivas.kandagatla@linaro.org \ --cc=tiwai@suse.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.