| Message ID | 1402662848-24534-20-git-send-email-vinod.koul@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/13/2014 02:34 PM, Vinod Koul wrote: > The is RFC patch for adding the platform mixer controls. This requires the > dapm_set-get to be exported. Or changes required after component series is > merged You are not using dapm_kcontrol_get_value() in this patch and without that calling dapm_kcontrol_set_value() is pretty pointless as the value is never read again. But it is still a good idea to use the generic controls rather than having a custom copy&pasted version. > > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > --- > sound/soc/intel/sst-atom-controls.c | 67 +++++++++++++++++++++++++++++++++++ > 1 files changed, 67 insertions(+), 0 deletions(-) > > diff --git a/sound/soc/intel/sst-atom-controls.c b/sound/soc/intel/sst-atom-controls.c > index 2e733f8..81a24eb 100644 > --- a/sound/soc/intel/sst-atom-controls.c > +++ b/sound/soc/intel/sst-atom-controls.c > @@ -66,6 +66,47 @@ unsigned int sst_reg_write(struct sst_data *drv, unsigned int reg, > return val; > } > > +int sst_mix_put(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + struct snd_soc_dapm_widget_list *wlist;// = dapm_kcontrol_get_wlist(kcontrol); > + struct snd_soc_dapm_widget *widget = wlist->widgets[0]; > + struct soc_mixer_control *mc = > + (struct soc_mixer_control *)kcontrol->private_value; > + struct sst_data *drv = snd_soc_platform_get_drvdata(widget->platform); > + unsigned int mask = (1 << fls(mc->max)) - 1; > + unsigned int val; > + int connect; > + struct snd_soc_dapm_update update; > + > + pr_debug("%s called set %#lx for %s\n", __func__, > + ucontrol->value.integer.value[0], widget->name); > + val = sst_reg_write(drv, mc->reg, mc->shift, mc->max, ucontrol->value.integer.value[0]); > + connect = !!val; > + > + //dapm_kcontrol_set_value(kcontrol, val); > + update.kcontrol = kcontrol; > + update.reg = mc->reg; > + update.mask = mask; > + update.val = val; > + > + snd_soc_dapm_mixer_update_power(widget->dapm, kcontrol, connect, &update); > + return 0; > +} > + > +int sst_mix_get(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + struct snd_soc_dapm_widget_list *wlist;// = dapm_kcontrol_get_wlist(kcontrol); > + struct snd_soc_dapm_widget *w = wlist->widgets[0]; > + struct soc_mixer_control *mc = > + (struct soc_mixer_control *)kcontrol->private_value; > + struct sst_data *drv = snd_soc_platform_get_drvdata(w->platform); > + > + ucontrol->value.integer.value[0] = !!sst_reg_read(drv, mc->reg, mc->shift, mc->max); > + return 0; > +} > + > static inline void sst_fill_byte_control(char *param, > u8 ipc_msg, u8 block, > u8 task_id, u8 pipe_id, > @@ -937,6 +978,32 @@ static const struct snd_soc_dapm_widget sst_dapm_widgets[] = { > SST_PATH_MEDIA_LOOP_OUTPUT("media_loop2_out", SST_TASK_SBA, SST_SWM_OUT_MEDIA_LOOP2, SST_FMT_STEREO, sst_set_media_loop), > > /* Media Mixers */ > + SST_SWM_MIXER("media0_out mix 0", SST_MIX_MEDIA0, SST_TASK_MMX, SST_SWM_OUT_MEDIA0, > + sst_mix_media0_controls, sst_swm_mixer_event), > + SST_SWM_MIXER("media1_out mix 0", SST_MIX_MEDIA1, SST_TASK_MMX, SST_SWM_OUT_MEDIA1, > + sst_mix_media1_controls, sst_swm_mixer_event), > + > + /* SBA PCM mixers */ > + SST_SWM_MIXER("pcm0_out mix 0", SST_MIX_PCM0, SST_TASK_SBA, SST_SWM_OUT_PCM0, > + sst_mix_pcm0_controls, sst_swm_mixer_event), > + SST_SWM_MIXER("pcm1_out mix 0", SST_MIX_PCM1, SST_TASK_SBA, SST_SWM_OUT_PCM1, > + sst_mix_pcm1_controls, sst_swm_mixer_event), > + SST_SWM_MIXER("pcm2_out mix 0", SST_MIX_PCM2, SST_TASK_SBA, SST_SWM_OUT_PCM2, > + sst_mix_pcm2_controls, sst_swm_mixer_event), > + > + /* SBA Loop mixers */ > + SST_SWM_MIXER("sprot_loop_out mix 0", SST_MIX_LOOP0, SST_TASK_SBA, SST_SWM_OUT_SPROT_LOOP, > + sst_mix_sprot_l0_controls, sst_swm_mixer_event), > + SST_SWM_MIXER("media_loop1_out mix 0", SST_MIX_LOOP1, SST_TASK_SBA, SST_SWM_OUT_MEDIA_LOOP1, > + sst_mix_media_l1_controls, sst_swm_mixer_event), > + SST_SWM_MIXER("media_loop2_out mix 0", SST_MIX_LOOP2, SST_TASK_SBA, SST_SWM_OUT_MEDIA_LOOP2, > + sst_mix_media_l2_controls, sst_swm_mixer_event), > + > + /* SBA Backend mixers */ > + SST_SWM_MIXER("codec_out0 mix 0", SST_MIX_CODEC0, SST_TASK_SBA, SST_SWM_OUT_CODEC0, > + sst_mix_codec0_controls, sst_swm_mixer_event), > + SST_SWM_MIXER("codec_out1 mix 0", SST_MIX_CODEC1, SST_TASK_SBA, SST_SWM_OUT_CODEC1, > + sst_mix_codec1_controls, sst_swm_mixer_event), > }; > > static const struct snd_soc_dapm_route intercon[] = { >
On Sat, Jun 14, 2014 at 05:39:09PM +0200, Lars-Peter Clausen wrote: > On 06/13/2014 02:34 PM, Vinod Koul wrote: > >The is RFC patch for adding the platform mixer controls. This requires the > >dapm_set-get to be exported. Or changes required after component series is > >merged > > You are not using dapm_kcontrol_get_value() in this patch and > without that calling dapm_kcontrol_set_value() is pretty pointless > as the value is never read again. But it is still a good idea to use > the generic controls rather than having a custom copy&pasted > version. Hey Lars, Circling back on this one.. We tried implementing based on the feedback. One question though remains. We were able to remove the get/put widget handlers. Now only the callback is present. With this when widget is powered up for a mixer, we need to know the value of the mixer so that we can inform DSP using IPC. Now potentially looks like we should be able to remove the register file as well, but only problem would be how to get the mixer control value (how many and which inputs are on/off and tell DSP). This would need me to export the dapm_get function, is that approach fine, or do we have any other way to get the value of this in driver.
On 07/04/2014 06:46 AM, Vinod Koul wrote: > On Sat, Jun 14, 2014 at 05:39:09PM +0200, Lars-Peter Clausen wrote: >> On 06/13/2014 02:34 PM, Vinod Koul wrote: >>> The is RFC patch for adding the platform mixer controls. This requires the >>> dapm_set-get to be exported. Or changes required after component series is >>> merged >> >> You are not using dapm_kcontrol_get_value() in this patch and >> without that calling dapm_kcontrol_set_value() is pretty pointless >> as the value is never read again. But it is still a good idea to use >> the generic controls rather than having a custom copy&pasted >> version. > > Hey Lars, > > Circling back on this one.. > > We tried implementing based on the feedback. One question though remains. > > We were able to remove the get/put widget handlers. Now only the callback is > present. With this when widget is powered up for a mixer, we need to know the value > of the mixer so that we can inform DSP using IPC. > > Now potentially looks like we should be able to remove the register file as well, > but only problem would be how to get the mixer control value (how many and which > inputs are on/off and tell DSP). This would need me to export the dapm_get > function, is that approach fine, or do we have any other way to get the value of > this in driver. I think it should be fine, if I correctly understand what you want to do. The main issue with all of this is that DAPM internally is very much tailored towards registers IO based systems. And using it for IPC is a bit bumpy. You can do it but the implementation is not always nice. DAPM could probably be adopted to better support non register IO based systems, but that would require a bit of effort. - Lars
On Fri, Jul 04, 2014 at 01:21:17PM +0200, Lars-Peter Clausen wrote: > On 07/04/2014 06:46 AM, Vinod Koul wrote: > >On Sat, Jun 14, 2014 at 05:39:09PM +0200, Lars-Peter Clausen wrote: > >>On 06/13/2014 02:34 PM, Vinod Koul wrote: > >>>The is RFC patch for adding the platform mixer controls. This requires the > >>>dapm_set-get to be exported. Or changes required after component series is > >>>merged > >> > >>You are not using dapm_kcontrol_get_value() in this patch and > >>without that calling dapm_kcontrol_set_value() is pretty pointless > >>as the value is never read again. But it is still a good idea to use > >>the generic controls rather than having a custom copy&pasted > >>version. > > > >Hey Lars, > > > >Circling back on this one.. > > > >We tried implementing based on the feedback. One question though remains. > > > >We were able to remove the get/put widget handlers. Now only the callback is > >present. With this when widget is powered up for a mixer, we need to know the value > >of the mixer so that we can inform DSP using IPC. > > > >Now potentially looks like we should be able to remove the register file as well, > >but only problem would be how to get the mixer control value (how many and which > >inputs are on/off and tell DSP). This would need me to export the dapm_get > >function, is that approach fine, or do we have any other way to get the value of > >this in driver. > > I think it should be fine, if I correctly understand what you want > to do. The main issue with all of this is that DAPM internally is > very much tailored towards registers IO based systems. And using it > for IPC is a bit bumpy. You can do it but the implementation is not > always nice. DAPM could probably be adopted to better support non > register IO based systems, but that would require a bit of effort. For now with this we are removing most of register code. Am optimistic right now that we can get it to work... Stay tuned :)
diff --git a/sound/soc/intel/sst-atom-controls.c b/sound/soc/intel/sst-atom-controls.c index 2e733f8..81a24eb 100644 --- a/sound/soc/intel/sst-atom-controls.c +++ b/sound/soc/intel/sst-atom-controls.c @@ -66,6 +66,47 @@ unsigned int sst_reg_write(struct sst_data *drv, unsigned int reg, return val; } +int sst_mix_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_dapm_widget_list *wlist;// = dapm_kcontrol_get_wlist(kcontrol); + struct snd_soc_dapm_widget *widget = wlist->widgets[0]; + struct soc_mixer_control *mc = + (struct soc_mixer_control *)kcontrol->private_value; + struct sst_data *drv = snd_soc_platform_get_drvdata(widget->platform); + unsigned int mask = (1 << fls(mc->max)) - 1; + unsigned int val; + int connect; + struct snd_soc_dapm_update update; + + pr_debug("%s called set %#lx for %s\n", __func__, + ucontrol->value.integer.value[0], widget->name); + val = sst_reg_write(drv, mc->reg, mc->shift, mc->max, ucontrol->value.integer.value[0]); + connect = !!val; + + //dapm_kcontrol_set_value(kcontrol, val); + update.kcontrol = kcontrol; + update.reg = mc->reg; + update.mask = mask; + update.val = val; + + snd_soc_dapm_mixer_update_power(widget->dapm, kcontrol, connect, &update); + return 0; +} + +int sst_mix_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_soc_dapm_widget_list *wlist;// = dapm_kcontrol_get_wlist(kcontrol); + struct snd_soc_dapm_widget *w = wlist->widgets[0]; + struct soc_mixer_control *mc = + (struct soc_mixer_control *)kcontrol->private_value; + struct sst_data *drv = snd_soc_platform_get_drvdata(w->platform); + + ucontrol->value.integer.value[0] = !!sst_reg_read(drv, mc->reg, mc->shift, mc->max); + return 0; +} + static inline void sst_fill_byte_control(char *param, u8 ipc_msg, u8 block, u8 task_id, u8 pipe_id, @@ -937,6 +978,32 @@ static const struct snd_soc_dapm_widget sst_dapm_widgets[] = { SST_PATH_MEDIA_LOOP_OUTPUT("media_loop2_out", SST_TASK_SBA, SST_SWM_OUT_MEDIA_LOOP2, SST_FMT_STEREO, sst_set_media_loop), /* Media Mixers */ + SST_SWM_MIXER("media0_out mix 0", SST_MIX_MEDIA0, SST_TASK_MMX, SST_SWM_OUT_MEDIA0, + sst_mix_media0_controls, sst_swm_mixer_event), + SST_SWM_MIXER("media1_out mix 0", SST_MIX_MEDIA1, SST_TASK_MMX, SST_SWM_OUT_MEDIA1, + sst_mix_media1_controls, sst_swm_mixer_event), + + /* SBA PCM mixers */ + SST_SWM_MIXER("pcm0_out mix 0", SST_MIX_PCM0, SST_TASK_SBA, SST_SWM_OUT_PCM0, + sst_mix_pcm0_controls, sst_swm_mixer_event), + SST_SWM_MIXER("pcm1_out mix 0", SST_MIX_PCM1, SST_TASK_SBA, SST_SWM_OUT_PCM1, + sst_mix_pcm1_controls, sst_swm_mixer_event), + SST_SWM_MIXER("pcm2_out mix 0", SST_MIX_PCM2, SST_TASK_SBA, SST_SWM_OUT_PCM2, + sst_mix_pcm2_controls, sst_swm_mixer_event), + + /* SBA Loop mixers */ + SST_SWM_MIXER("sprot_loop_out mix 0", SST_MIX_LOOP0, SST_TASK_SBA, SST_SWM_OUT_SPROT_LOOP, + sst_mix_sprot_l0_controls, sst_swm_mixer_event), + SST_SWM_MIXER("media_loop1_out mix 0", SST_MIX_LOOP1, SST_TASK_SBA, SST_SWM_OUT_MEDIA_LOOP1, + sst_mix_media_l1_controls, sst_swm_mixer_event), + SST_SWM_MIXER("media_loop2_out mix 0", SST_MIX_LOOP2, SST_TASK_SBA, SST_SWM_OUT_MEDIA_LOOP2, + sst_mix_media_l2_controls, sst_swm_mixer_event), + + /* SBA Backend mixers */ + SST_SWM_MIXER("codec_out0 mix 0", SST_MIX_CODEC0, SST_TASK_SBA, SST_SWM_OUT_CODEC0, + sst_mix_codec0_controls, sst_swm_mixer_event), + SST_SWM_MIXER("codec_out1 mix 0", SST_MIX_CODEC1, SST_TASK_SBA, SST_SWM_OUT_CODEC1, + sst_mix_codec1_controls, sst_swm_mixer_event), }; static const struct snd_soc_dapm_route intercon[] = {
The is RFC patch for adding the platform mixer controls. This requires the dapm_set-get to be exported. Or changes required after component series is merged Signed-off-by: Vinod Koul <vinod.koul@intel.com> --- sound/soc/intel/sst-atom-controls.c | 67 +++++++++++++++++++++++++++++++++++ 1 files changed, 67 insertions(+), 0 deletions(-)