[Sound-open-firmware] [PATCH] [RFC]Tone: use volume control command instead of switch

[Ranjani Sridharan]

On Thu, 2018-05-31 at 16:07 +0800, Pan, Xiuli wrote:
> 
> On 5/31/2018 15:46, Liam Girdwood wrote:
> > On Tue, 2018-05-29 at 12:11 -0700, Ranjani Sridharan wrote:
> > > This patch makes the following changes to the tone comp:
> > > 
> > > 1. Use SOF_CTRL_CMD_VOLUME instead of the SOF_CTRL_CMD_SWITCH
> > >   command as the trigger. Tone uses a mixer control with a max
> > > value of 1 as the triggering control so change the command
> > > accordingly.
> > > 
> > 
> > What's the benefit of this change, since the value looks like a 0
> > or 1 a switch
> > type is perfect (and more meaningfull from userspace).
> 
> Ranjani must be held by the volume mute switch propose she once had.
> I agree we need a switch type. I am trying to enable a switch
> generic 
> put/get IO handler for the loopback kcontrol.
> 
Xiuli, yes, thanks!

I made this change just to highlight this issue again. A generic switch
IO handler in the driver will make things look much better. 

> Thanks
> Xiuli
> > Thanks
> > 
> > Liam
> > 
> > > 2. Add tone_cmd_get_value() function to send the tone state
> > > back to the driver
> > > 
> > > 3. Include the current tone state in the ipc reply data
> > > when setting the tone state.
> > > 
> > > 4. Compute the sample rate from the pipeline frames and deadline
> > > parameters.
> > > 
> > > Signed-off-by: Ranjani Sridharan <ranjani.sridharan at linux.intel.c
> > > om>
> > > ---
> > >   src/audio/tone.c | 31 +++++++++++++++++++++++++++++--
> > >   1 file changed, 29 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/audio/tone.c b/src/audio/tone.c
> > > index eee6091..53cb486 100644
> > > --- a/src/audio/tone.c
> > > +++ b/src/audio/tone.c
> > > @@ -462,6 +462,25 @@ static int tone_params(struct comp_dev *dev)
> > >   	return 0;
> > >   }
> > >   
> > > +static int tone_cmd_get_value(struct comp_dev *dev,
> > > +			      struct sof_ipc_ctrl_data *cdata)
> > > +{
> > > +	struct comp_data *cd = comp_get_drvdata(dev);
> > > +	int j;
> > > +
> > > +	trace_tone("mgt");
> > > +
> > > +	if (cdata->cmd == SOF_CTRL_CMD_VOLUME) {
> > > +		for (j = 0; j < cdata->num_elems; j++) {
> > > +			cdata->chanv[j].channel = j;
> > > +			cdata->chanv[j].value = !cd->sg[j].mute;
> > > +			trace_value(j);
> > > +			trace_value(cd->sg[j].mute);
> > > +		}
> > > +	}
> > > +	return 0;
> > > +}
> > > +
> > >   static int tone_cmd_set_value(struct comp_dev *dev, struct
> > > sof_ipc_ctrl_data
> > > *cdata)
> > >   {
> > >   	struct comp_data *cd = comp_get_drvdata(dev);
> > > @@ -469,7 +488,8 @@ static int tone_cmd_set_value(struct comp_dev
> > > *dev, struct
> > > sof_ipc_ctrl_data *cd
> > >   	uint32_t ch;
> > >   	bool val;
> > >   
> > > -	if (cdata->cmd == SOF_CTRL_CMD_SWITCH) {
> > > +	/* tone uses a mixer control with max of 1 to trigger
> > > the pipeline */
> > > +	if (cdata->cmd == SOF_CTRL_CMD_VOLUME) {
> > >   		trace_tone("mst");
> > >   		for (j = 0; j < cdata->num_elems; j++) {
> > >   			ch = cdata->chanv[j].channel;
> > > @@ -480,6 +500,10 @@ static int tone_cmd_set_value(struct
> > > comp_dev *dev,
> > > struct sof_ipc_ctrl_data *cd
> > >   				trace_tone_error("che");
> > >   				return -EINVAL;
> > >   			}
> > > +
> > > +			/* send current state back to the driver
> > > */
> > > +			cdata->chanv[j].value = !cd-
> > > >sg[ch].mute;
> > > +
> > >   			if (val)
> > >   				tonegen_unmute(&cd->sg[ch]);
> > >   			else
> > > @@ -583,6 +607,9 @@ static int tone_cmd(struct comp_dev *dev, int
> > > cmd, void
> > > *data)
> > >   	case COMP_CMD_SET_VALUE:
> > >   		ret = tone_cmd_set_value(dev, cdata);
> > >   		break;
> > > +	case COMP_CMD_GET_VALUE:
> > > +		ret = tone_cmd_get_value(dev, cdata);
> > > +		break;
> > >   	}
> > >   
> > >   	return ret;
> > > @@ -641,7 +668,7 @@ static int tone_prepare(struct comp_dev *
> > > dev)
> > >   		return ret;
> > >   
> > >   	cd->channels = dev->params.channels;
> > > -	cd->rate = dev->params.rate;
> > > +	cd->rate = dev->frames * dev->pipeline-
> > > >ipc_pipe.deadline;
> > >   	tracev_value(cd->channels);
> > >   	tracev_value(cd->rate);
> > >   
> > 
> > _______________________________________________
> > Sound-open-firmware mailing list
> > Sound-open-firmware at alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmwar
> > e
> 
>