On Tue, 16 May 2017 07:38:40 +0200,
Takashi Sakamoto wrote:
>
> Hi,
>
> On May 16 2017 10:01, Subhransu S. Prusty wrote:
> > From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> >
> > In case of mmap, by default alsa-lib mmaps both control and status data.
> >
> > If driver subscribes for application pointer update, driver needs to get
> > notification whenever appl ptr changes. With the above case driver won't
> > get appl ptr notifications.
> >
> > This patch check on a hw info flag and returns error when user land asks
> > for mmaping control & status data, thus forcing user to issue
> > IOCTL_SYNC_PTR.
> >
> > Suggested-by: Takashi Iwai <tiwai@xxxxxxx>
> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> > Signed-off-by: Ramesh Babu <ramesh.babu@xxxxxxxxx>
> > Signed-off-by: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@xxxxxxxxx>
> > Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@xxxxxxxxx>
> > ---
> > include/uapi/sound/asound.h | 1 +
> > sound/core/pcm_native.c | 17 +++++++++++++++++
> > 2 files changed, 18 insertions(+)
>
> Would you please explain about the case that drivers can't support
> page frame mapping, please?
>
> As long as I know, it depends on kernel and platform architecture
> whether mapping is available or not, because the data of 'struct
> snd_pcm_mmap_status' and 'struct snd_pcm_mmap_control' is not directly
> related to data transmission and independent of target device design.
It's only a part of the condition.
In this case, the problem is that the mmap control allows the appl_ptr
being changed silently without interaction with the driver. If the
driver requires some explicit action for changing the appl_ptr, it
won't work.
> Of course, I can understand your intension for previous patches. But
> the code comment is quite misleading and worthless.
Worthless is a too strong word, but I agree that more clarification
would be more helpful.
thanks,
Takashi
>
> > diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> > index c697ff90450d..dea7d89b41ca 100644
> > --- a/include/uapi/sound/asound.h
> > +++ b/include/uapi/sound/asound.h
> > @@ -284,6 +284,7 @@ enum {
> > #define SNDRV_PCM_INFO_HAS_LINK_ABSOLUTE_ATIME 0x02000000 /* report absolute hardware link audio time, not reset on startup */
> > #define SNDRV_PCM_INFO_HAS_LINK_ESTIMATED_ATIME 0x04000000 /* report estimated link audio time */
> > #define SNDRV_PCM_INFO_HAS_LINK_SYNCHRONIZED_ATIME 0x08000000 /* report synchronized audio/system time */
> > +#define SNDRV_PCM_INFO_NO_STATUS_MMAP 0x10000000 /* status and control mmap not supported */
> >
> > #define SNDRV_PCM_INFO_DRAIN_TRIGGER 0x40000000 /* internal kernel flag - trigger in drain */
> > #define SNDRV_PCM_INFO_FIFO_IN_FRAMES 0x80000000 /* internal kernel flag - FIFO size is in frames */
> > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> > index c14cdbd9ff86..2635a7d4d1fa 100644
> > --- a/sound/core/pcm_native.c
> > +++ b/sound/core/pcm_native.c
> > @@ -3524,21 +3524,38 @@ static int snd_pcm_mmap(struct file *file, struct vm_area_struct *area)
> > struct snd_pcm_file * pcm_file;
> > struct snd_pcm_substream *substream;
> > unsigned long offset;
> > + unsigned int info;
> >
> > pcm_file = file->private_data;
> > substream = pcm_file->substream;
> > if (PCM_RUNTIME_CHECK(substream))
> > return -ENXIO;
> > + info = substream->runtime->hw.info;
> >
> > offset = area->vm_pgoff << PAGE_SHIFT;
> > switch (offset) {
> > case SNDRV_PCM_MMAP_OFFSET_STATUS:
> > if (pcm_file->no_compat_mmap)
> > return -ENXIO;
> > + /*
> > + * force fallback to ioctl if driver doesn't support status
> > + * and control mmap.
> > + */
> > + if (info & SNDRV_PCM_INFO_NO_STATUS_MMAP)
> > + return -ENXIO;
> > +
> > return snd_pcm_mmap_status(substream, file, area);
> > case SNDRV_PCM_MMAP_OFFSET_CONTROL:
> > if (pcm_file->no_compat_mmap)
> > return -ENXIO;
> > +
> > + /*
> > + * force fallback to ioctl if driver doesn't support status
> > + * and control mmap.
> > + */
> > + if (info & SNDRV_PCM_INFO_NO_STATUS_MMAP)
> > + return -ENXIO;
> > +
> > return snd_pcm_mmap_control(substream, file, area);
> > default:
> > return snd_pcm_mmap_data(substream, file, area);
>
> Regards
>
> Takashi Sakamoto
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
