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@suse.de
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:
- 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).
- 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@intel.com
Thanks!
Takashi