[Sound-open-firmware] [PATCH] Volume: Remove channels mapping via channel map array from volume control
This patch removes the channel map table lookup from all volume control since it is not used by the SOF 1.0 driver. Instead of enum channel descriptors the channels are addressed with indices in 0..<number of channels -1> range.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com --- src/audio/volume.c | 80 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 30 deletions(-)
diff --git a/src/audio/volume.c b/src/audio/volume.c index 415a853..fb8d9e7 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -53,6 +53,7 @@ #define VOL_RAMP_US 2000 #define VOL_RAMP_STEP (1 << 11) #define VOL_MAX (1 << 16) +#define VOL_MIN 0
/* * Simple volume control @@ -282,14 +283,14 @@ static const struct comp_func_map func_map[] = { /* synchronise host mmap() volume with real value */ static void vol_sync_host(struct comp_data *cd, uint32_t chan) { - int i; - if (cd->hvol == NULL) return;
- for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) { - if (cd->hvol[i].channel == cd->chan[chan]) - cd->hvol[i].value = cd->volume[chan]; + if (chan < SOF_IPC_MAX_CHANNELS) + cd->hvol[chan].value = cd->volume[chan]; + else { + trace_volume_error("veh"); + tracev_value(chan); } }
@@ -414,10 +415,20 @@ static int volume_params(struct comp_dev *dev) static inline void volume_set_chan(struct comp_dev *dev, int chan, uint32_t vol) { struct comp_data *cd = comp_get_drvdata(dev); + uint32_t v = vol; + + /* Limit received volume gain to MIN..MAX range before applying it. + * MAX is needed for now for the generic C gain arithmetics to prevent + * multiplication overflow with the 32 bit value. Non-zero MIN option + * can be useful to prevent totally muted small volume gain. + */ + if (v < VOL_MIN) + v = VOL_MIN; + + if (v > VOL_MAX) + v = VOL_MAX;
- /* TODO: ignore vol of 0 atm - bad IPC */ - if (vol > 0 && vol <= VOL_MAX) - cd->tvolume[chan] = vol; + cd->tvolume[chan] = v; }
static inline void volume_set_chan_mute(struct comp_dev *dev, int chan) @@ -450,12 +461,16 @@ static int volume_ctrl_set_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c switch (cdata->cmd) { case SOF_CTRL_CMD_VOLUME: trace_volume("vst"); - for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) { - for (j = 0; j < cdata->num_elems; j++) { - tracev_value(cdata->chanv[j].channel); - tracev_value(cdata->chanv[j].value); - if (cdata->chanv[j].channel == cd->chan[i]) - volume_set_chan(dev, i, cdata->chanv[j].value); + trace_value(cdata->comp_id); + for (j = 0; j < cdata->num_elems; j++) { + trace_value(cdata->chanv[j].channel); + trace_value(cdata->chanv[j].value); + i = cdata->chanv[j].channel; + if ((i >= 0) && (i < SOF_IPC_MAX_CHANNELS)) + volume_set_chan(dev, i, cdata->chanv[j].value); + else { + trace_volume_error("gs2"); + tracev_value(i); } } work_schedule_default(&cd->volwork, VOL_RAMP_US); @@ -463,16 +478,19 @@ static int volume_ctrl_set_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c
case SOF_CTRL_CMD_SWITCH: trace_volume("mst"); - for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) { - for (j = 0; j < cdata->num_elems; j++) { - tracev_value(cdata->chanv[j].channel); - tracev_value(cdata->chanv[j].value); - if (cdata->chanv[j].channel == cd->chan[i]) { - if (cdata->chanv[j].value) - volume_set_chan_unmute(dev, i); - else - volume_set_chan_mute(dev, i); - } + trace_value(cdata->comp_id); + for (j = 0; j < cdata->num_elems; j++) { + trace_value(cdata->chanv[j].channel); + trace_value(cdata->chanv[j].value); + i = cdata->chanv[j].channel; + if ((i >= 0) && (i < SOF_IPC_MAX_CHANNELS)) { + if (cdata->chanv[j].value) + volume_set_chan_unmute(dev, i); + else + volume_set_chan_mute(dev, i); + } else { + trace_volume_error("gs3"); + tracev_value(i); } } work_schedule_default(&cd->volwork, VOL_RAMP_US); @@ -489,21 +507,23 @@ static int volume_ctrl_set_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c static int volume_ctrl_get_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) { struct comp_data *cd = comp_get_drvdata(dev); - int i; int j;
/* validate */ if (cdata->num_elems == 0 || cdata->num_elems >= SOF_IPC_MAX_CHANNELS) { trace_volume_error("gc0"); + tracev_value(cdata->num_elems); return -EINVAL; }
if (cdata->cmd == SOF_CTRL_CMD_VOLUME) { - for (i = 0; i < cdata->num_elems; i++) { - for (j = 0; j < cdata->num_elems; j++) { - if (cdata->chanv[j].value == cd->chan[i]) - cdata->chanv[j].value = cd->tvolume[i]; - } + trace_volume("vgt"); + trace_value(cdata->comp_id); + for (j = 0; j < cdata->num_elems; j++) { + cdata->chanv[j].channel = j; + cdata->chanv[j].value = cd->tvolume[j]; + trace_value(cdata->chanv[j].channel); + trace_value(cdata->chanv[j].value); } } else { trace_volume_error("ec2");
-----Original Message----- From: sound-open-firmware-bounces@alsa-project.org [mailto:sound-open- firmware-bounces@alsa-project.org] On Behalf Of Seppo Ingalsuo Sent: Monday, November 13, 2017 6:12 PM To: sound-open-firmware@alsa-project.org Cc: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com Subject: [Sound-open-firmware] [PATCH] Volume: Remove channels mapping via channel map array from volume control
This patch removes the channel map table lookup from all volume control since it is not used by the SOF 1.0 driver. Instead of enum channel descriptors the channels are addressed with indices in 0..<number of channels -1> range.
So does this suppose that the array index is identical to the channel index? If so, we can delete member channel in struct sof_ipc_ctrl_value_chan.
But looks this array indices are not equal to channel mapping if my understanding Correctly, e.g. front_left and front right are set to channel index 0 and 1 usually, But their enumerated value are 3 and 4 in the enum sof_ipc_chmap definition: SOF_CHMAP_FL, /* front left */ SOF_CHMAP_FR, /* front right */
Thanks, ~Keyon
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com
src/audio/volume.c | 80 ++++++++++++++++++++++++++++++++++-----------------
1 file changed, 50 insertions(+), 30 deletions(-)
diff --git a/src/audio/volume.c b/src/audio/volume.c index 415a853..fb8d9e7 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -53,6 +53,7 @@ #define VOL_RAMP_US 2000 #define VOL_RAMP_STEP (1 << 11) #define VOL_MAX (1 << 16) +#define VOL_MIN 0
/*
- Simple volume control
@@ -282,14 +283,14 @@ static const struct comp_func_map func_map[] = { /* synchronise host mmap() volume with real value */ static void vol_sync_host(struct comp_data *cd, uint32_t chan) {
int i;
if (cd->hvol == NULL) return;
for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) {
if (cd->hvol[i].channel == cd->chan[chan])cd->hvol[i].value = cd->volume[chan];
- if (chan < SOF_IPC_MAX_CHANNELS)
cd->hvol[chan].value = cd->volume[chan];- else {
trace_volume_error("veh");tracev_value(chan);}
@@ -414,10 +415,20 @@ static int volume_params(struct comp_dev *dev) static inline void volume_set_chan(struct comp_dev *dev, int chan, uint32_t vol) { struct comp_data *cd = comp_get_drvdata(dev);
- uint32_t v = vol;
- /* Limit received volume gain to MIN..MAX range before applying it.
* MAX is needed for now for the generic C gain arithmetics to prevent* multiplication overflow with the 32 bit value. Non-zero MIN option* can be useful to prevent totally muted small volume gain.*/- if (v < VOL_MIN)
v = VOL_MIN;- if (v > VOL_MAX)
v = VOL_MAX;
- /* TODO: ignore vol of 0 atm - bad IPC */
- if (vol > 0 && vol <= VOL_MAX)
cd->tvolume[chan] = vol;
- cd->tvolume[chan] = v;
}
static inline void volume_set_chan_mute(struct comp_dev *dev, int chan) @@ - 450,12 +461,16 @@ static int volume_ctrl_set_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c switch (cdata->cmd) { case SOF_CTRL_CMD_VOLUME: trace_volume("vst");
for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) {for (j = 0; j < cdata->num_elems; j++) {tracev_value(cdata->chanv[j].channel);tracev_value(cdata->chanv[j].value);if (cdata->chanv[j].channel == cd->chan[i])volume_set_chan(dev, i, cdata-chanv[j].value);
trace_value(cdata->comp_id);for (j = 0; j < cdata->num_elems; j++) {trace_value(cdata->chanv[j].channel);trace_value(cdata->chanv[j].value);i = cdata->chanv[j].channel;if ((i >= 0) && (i < SOF_IPC_MAX_CHANNELS))volume_set_chan(dev, i, cdata->chanv[j].value);else {trace_volume_error("gs2");tracev_value(i); }463,16 +478,19 @@ static int volume_ctrl_set_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c
case SOF_CTRL_CMD_SWITCH: trace_volume("mst");
for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) {for (j = 0; j < cdata->num_elems; j++) {tracev_value(cdata->chanv[j].channel);tracev_value(cdata->chanv[j].value);if (cdata->chanv[j].channel == cd->chan[i]) {if (cdata->chanv[j].value)volume_set_chan_unmute(dev,i);
elsevolume_set_chan_mute(dev, i);}
trace_value(cdata->comp_id);for (j = 0; j < cdata->num_elems; j++) {trace_value(cdata->chanv[j].channel);trace_value(cdata->chanv[j].value);i = cdata->chanv[j].channel;if ((i >= 0) && (i < SOF_IPC_MAX_CHANNELS)) {if (cdata->chanv[j].value)volume_set_chan_unmute(dev, i);elsevolume_set_chan_mute(dev, i);} else {trace_volume_error("gs3");tracev_value(i); }489,21 +507,23 @@ static int volume_ctrl_set_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c static int volume_ctrl_get_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) { struct comp_data *cd = comp_get_drvdata(dev);
int i; int j;
/* validate */ if (cdata->num_elems == 0 || cdata->num_elems >=
SOF_IPC_MAX_CHANNELS) { trace_volume_error("gc0");
tracev_value(cdata->num_elems);return -EINVAL; }
if (cdata->cmd == SOF_CTRL_CMD_VOLUME) {
for (i = 0; i < cdata->num_elems; i++) {for (j = 0; j < cdata->num_elems; j++) {if (cdata->chanv[j].value == cd->chan[i])cdata->chanv[j].value = cd->tvolume[i];}
trace_volume("vgt");trace_value(cdata->comp_id);for (j = 0; j < cdata->num_elems; j++) {cdata->chanv[j].channel = j;cdata->chanv[j].value = cd->tvolume[j];trace_value(cdata->chanv[j].channel);trace_value(cdata->chanv[j].value);-- 2.11.0
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On 13.11.2017 15:45, Jie, Yang wrote:
-----Original Message----- From: sound-open-firmware-bounces@alsa-project.org [mailto:sound-open- firmware-bounces@alsa-project.org] On Behalf Of Seppo Ingalsuo Sent: Monday, November 13, 2017 6:12 PM To: sound-open-firmware@alsa-project.org Cc: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com Subject: [Sound-open-firmware] [PATCH] Volume: Remove channels mapping via channel map array from volume control
This patch removes the channel map table lookup from all volume control since it is not used by the SOF 1.0 driver. Instead of enum channel descriptors the channels are addressed with indices in 0..<number of channels -1> range.
So does this suppose that the array index is identical to the channel index? If so, we can delete member channel in struct sof_ipc_ctrl_value_chan.
No, the channel to apply is got from cdata->chanv[j].channel. The channel is not directly the index to array (j).
Without this feature it would not be possible to update just single channel M without need to update the same time all channels 0..M.
But looks this array indices are not equal to channel mapping if my understanding Correctly, e.g. front_left and front right are set to channel index 0 and 1 usually, But their enumerated value are 3 and 4 in the enum sof_ipc_chmap definition: SOF_CHMAP_FL, /* front left */ SOF_CHMAP_FR, /* front right */
Populating the channel map is not planned for this driver version so this feature could not be used. When debugging the firmware it's seen that channel map is all zeros.
The sof_ipc_chmap also is limited to ITU-R speaker locations so they wouldn't work for lowest level speaker channels (digital multi-way speakers or non-std speaker locations) and mic array channels not directly associated with media content representation. So in future ideally there should be both direct channel indexes for low level gain control and channel map based gain for higher level gain controls where content rendering speaker locations are known.
Thanks, ~Keyon
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com
src/audio/volume.c | 80 ++++++++++++++++++++++++++++++++++-----------------
1 file changed, 50 insertions(+), 30 deletions(-)
diff --git a/src/audio/volume.c b/src/audio/volume.c index 415a853..fb8d9e7 100644 --- a/src/audio/volume.c +++ b/src/audio/volume.c @@ -53,6 +53,7 @@ #define VOL_RAMP_US 2000 #define VOL_RAMP_STEP (1 << 11) #define VOL_MAX (1 << 16) +#define VOL_MIN 0
/*
- Simple volume control
@@ -282,14 +283,14 @@ static const struct comp_func_map func_map[] = { /* synchronise host mmap() volume with real value */ static void vol_sync_host(struct comp_data *cd, uint32_t chan) {
int i;
if (cd->hvol == NULL) return;
for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) {
if (cd->hvol[i].channel == cd->chan[chan])cd->hvol[i].value = cd->volume[chan];
- if (chan < SOF_IPC_MAX_CHANNELS)
cd->hvol[chan].value = cd->volume[chan];- else {
trace_volume_error("veh");tracev_value(chan);}
@@ -414,10 +415,20 @@ static int volume_params(struct comp_dev *dev) static inline void volume_set_chan(struct comp_dev *dev, int chan, uint32_t vol) { struct comp_data *cd = comp_get_drvdata(dev);
- uint32_t v = vol;
- /* Limit received volume gain to MIN..MAX range before applying it.
* MAX is needed for now for the generic C gain arithmetics to prevent* multiplication overflow with the 32 bit value. Non-zero MIN option* can be useful to prevent totally muted small volume gain.*/- if (v < VOL_MIN)
v = VOL_MIN;- if (v > VOL_MAX)
v = VOL_MAX;
- /* TODO: ignore vol of 0 atm - bad IPC */
- if (vol > 0 && vol <= VOL_MAX)
cd->tvolume[chan] = vol;
- cd->tvolume[chan] = v;
}
static inline void volume_set_chan_mute(struct comp_dev *dev, int chan) @@ - 450,12 +461,16 @@ static int volume_ctrl_set_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c switch (cdata->cmd) { case SOF_CTRL_CMD_VOLUME: trace_volume("vst");
for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) {for (j = 0; j < cdata->num_elems; j++) {tracev_value(cdata->chanv[j].channel);tracev_value(cdata->chanv[j].value);if (cdata->chanv[j].channel == cd->chan[i])volume_set_chan(dev, i, cdata-chanv[j].value);
trace_value(cdata->comp_id);for (j = 0; j < cdata->num_elems; j++) {trace_value(cdata->chanv[j].channel);trace_value(cdata->chanv[j].value);i = cdata->chanv[j].channel;if ((i >= 0) && (i < SOF_IPC_MAX_CHANNELS))volume_set_chan(dev, i, cdata->chanv[j].value);else {trace_volume_error("gs2");tracev_value(i); }463,16 +478,19 @@ static int volume_ctrl_set_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c
case SOF_CTRL_CMD_SWITCH: trace_volume("mst");
for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) {for (j = 0; j < cdata->num_elems; j++) {tracev_value(cdata->chanv[j].channel);tracev_value(cdata->chanv[j].value);if (cdata->chanv[j].channel == cd->chan[i]) {if (cdata->chanv[j].value)volume_set_chan_unmute(dev,i);
elsevolume_set_chan_mute(dev, i);}
trace_value(cdata->comp_id);for (j = 0; j < cdata->num_elems; j++) {trace_value(cdata->chanv[j].channel);trace_value(cdata->chanv[j].value);i = cdata->chanv[j].channel;if ((i >= 0) && (i < SOF_IPC_MAX_CHANNELS)) {if (cdata->chanv[j].value)volume_set_chan_unmute(dev, i);elsevolume_set_chan_mute(dev, i);} else {trace_volume_error("gs3");tracev_value(i); }489,21 +507,23 @@ static int volume_ctrl_set_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c static int volume_ctrl_get_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata) { struct comp_data *cd = comp_get_drvdata(dev);
int i; int j;
/* validate */ if (cdata->num_elems == 0 || cdata->num_elems >=
SOF_IPC_MAX_CHANNELS) { trace_volume_error("gc0");
tracev_value(cdata->num_elems);return -EINVAL; }
if (cdata->cmd == SOF_CTRL_CMD_VOLUME) {
for (i = 0; i < cdata->num_elems; i++) {for (j = 0; j < cdata->num_elems; j++) {if (cdata->chanv[j].value == cd->chan[i])cdata->chanv[j].value = cd->tvolume[i];}
trace_volume("vgt");trace_value(cdata->comp_id);for (j = 0; j < cdata->num_elems; j++) {cdata->chanv[j].channel = j;cdata->chanv[j].value = cd->tvolume[j];trace_value(cdata->chanv[j].channel);trace_value(cdata->chanv[j].value);-- 2.11.0
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On Mon, 2017-11-13 at 12:11 +0200, Seppo Ingalsuo wrote:
This patch removes the channel map table lookup from all volume control since it is not used by the SOF 1.0 driver. Instead of enum channel descriptors the channels are addressed with indices in 0..<number of channels -1> range.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com
src/audio/volume.c | 80 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 30 deletions(-)
Applied.
Thanks
Liam
participants (3)
-
Jie, Yang -
Liam Girdwood -
Seppo Ingalsuo