Re: [PATCH] ASoC: intel: skylake: Drop superfluous mmap callback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux