ASoC: SOF: topology: pass volume min/max linear value to FW #1007
Conversation
zhuyingjiang
requested review from
bardliao,
juimonen,
keyonjie,
lyakh,
ranj063 and
singalsu
as code owners
singalsu
left a comment
singalsu
left a comment
There was a problem hiding this comment.
I've tested this to work OK, thanks @zhuyingjiang!
I'll publish next the FW side PR to utilize this information. The code snippet to shorten the ramp will be needed for backwards compatibility after this only with old kernels those do not provide this information.
plbossart
left a comment
plbossart
left a comment
There was a problem hiding this comment.
The units are completely unclear, "min" and "max" could be interpreted in various ways. I strongly suggest you add a suffix so that reviewers understand what physical values are being handled. From what I understand, you pass the min/max on a linear scale after a conversion from values stored in the topology file (and I am not sure about the units there).
sound/soc/sof/topology.c
Outdated
| list_for_each_entry(scontrol, &sdev->kcontrol_list, list) { | ||
| if (scontrol->comp_id == swidget->comp_id) { | ||
| volume->min_value = | ||
| scontrol->volume_table[scontrol->min]; |
There was a problem hiding this comment.
so scontrol->min and scontrol->max are in what units? dB, steps?
There was a problem hiding this comment.
@plbossart it is in step, and from the struct:
struct snd_soc_tplg_mixer_control {
struct snd_soc_tplg_ctl_hdr hdr;
__le32 size; /* in bytes of this structure */
__le32 min;
__le32 max;
__le32 platform_max;
__le32 invert;
__le32 num_channels;
struct snd_soc_tplg_channel channel[SND_SOC_TPLG_MAX_CHAN];
struct snd_soc_tplg_private priv;
} __attribute__((packed));
OK, I will change to min_volume_step, and max_volume_step.
zhuyingjiang
force-pushed
the
topic/pass-volume-min-max-linear-value-to-FW
branch
from
84a4b9c to
218fa07
Compare
zhuyingjiang
changed the title
zhuyingjiang
requested review from
plbossart and
singalsu
and removed request for
plbossart
singalsu
left a comment
singalsu
left a comment
There was a problem hiding this comment.
Looks good to me. The previous version works OK for me, I didn't retest because this was just rename of two fields.
|
The FW side PR is in thesofproject/sof#1523 . |
plbossart
left a comment
plbossart
left a comment
There was a problem hiding this comment.
The code looks good but please try and improve the code indentation to make it readable.
Also fix the commit message
The driver currently passes the volume ramp type and length
topology tokens to firmware, but the min and max volume are not set. This patch provides a correction to convert the information from the topology file and pass the linear volume min/max value to the firmware to improve transitions.
add two units min_volume_step and max_volume_step to the snd_sof_control struct, for the min and max step of the volume_table. Signed-off-by: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>
The driver currently passes the volume ramp type and length topology tokens to firmware, but the min and max volume are not set. This patch provides a correction to convert the information from the topology file and pass the linear volume min/max value to the firmware to improve transitions. Signed-off-by: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>
zhuyingjiang
force-pushed
the
topic/pass-volume-min-max-linear-value-to-FW
branch
from
218fa07 to
2f94c43
Compare
|
@plbossart Updated the indention and the commit message. |
3 participants
The driver currently passes the volume ramp type and length
topology tokens to firmware. The min/max fields are left zeros
(would be derived from TLV volume scale). This PR send the
linear volume min/max value to the firmware. So firmware can
use these values and current volume for any transition.
fix #922
Signed-off-by: Zhu Yingjiang yingjiang.zhu@linux.intel.com