On Fri, 30 Jul 2021 15:59:54 +0200,
Cezary Rojewski wrote:
>
> On 2021-07-28 4:19 PM, Takashi Iwai wrote:
> > skl_platform_soc_mmap() just calls the standard mmap helper, hence
> > it's superfluous. Let's drop it.
> >
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > ---
> > sound/soc/intel/skylake/skl-pcm.c | 8 --------
> > 1 file changed, 8 deletions(-)
> >
> > diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
> > index b1ca64d2f7ea..c604200de80e 100644
> > --- a/sound/soc/intel/skylake/skl-pcm.c
> > +++ b/sound/soc/intel/skylake/skl-pcm.c
> > @@ -1214,13 +1214,6 @@ static snd_pcm_uframes_t skl_platform_soc_pointer(
> > return bytes_to_frames(substream->runtime, pos);
> > }
> > -static int skl_platform_soc_mmap(struct snd_soc_component
> > *component,
> > - struct snd_pcm_substream *substream,
> > - struct vm_area_struct *area)
> > -{
> > - return snd_pcm_lib_default_mmap(substream, area);
> > -}
> > -
> > static u64 skl_adjust_codec_delay(struct snd_pcm_substream *substream,
> > u64 nsec)
> > {
> > @@ -1460,7 +1453,6 @@ static const struct snd_soc_component_driver skl_component = {
> > .trigger = skl_platform_soc_trigger,
> > .pointer = skl_platform_soc_pointer,
> > .get_time_info = skl_platform_soc_get_time_info,
> > - .mmap = skl_platform_soc_mmap,
> > .pcm_construct = skl_platform_soc_new,
> > .module_get_upon_open = 1, /* increment refcount when a pcm is opened */
> > };
> >
>
> Thanks for the input, Takashi.
> While I welcome the change, have two quick questions:
>
> 1) Does this impact hw_support_mmap() and its user behavior? i.e. is
> there some implicit behavior change that skylake-users may experience
> along the way?
hw_support_mmap() must return true for this case as long as you've set
up the buffer in the right way (either preallocation or managed).
> 2) Since snd_pcm_mmap_data() defaults to snd_pcm_lib_default_mmap()
> why not update ops assignment - snd_pcm_set_ops() - in such a way that
> if/else is no longer needed in the former?
Simply put: when the buffer is allocated via
snd_pcm_set_managed_buffer*(), you can omit the mmap callback.
The only case you need the mmap callback is only when a special buffer
is used or it needs a special way of mmap itself.
> Pretty sure other drivers have been updated in similar fashion and my
> two cents should not be blocking integration:
>
> Reviewed-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
Thanks!
Takashi
