Re: [PATCH 6/8] ALSA: pcm: conditionally avoid mmap of control data - Pierre-Louis Bossart

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org, broonie@kernel.org,
	Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
	Kai Vehmanen <kai.vehmanen@linux.intel.com>
Subject: Re: [PATCH 6/8] ALSA: pcm: conditionally avoid mmap of control data
Date: Tue, 13 Jul 2021 07:39:24 -0500	[thread overview]
Message-ID: <d7a3b3fd-eddf-000b-f9ed-711d3e3156f4@linux.intel.com> (raw)
In-Reply-To: <s5h8s2a3fgh.wl-tiwai@suse.de>

On 7/13/21 1:17 AM, Takashi Iwai wrote:
> On Mon, 12 Jul 2021 22:56:07 +0200,
> Pierre-Louis Bossart wrote:
>>
>>
>>
>> On 6/13/21 2:28 AM, Takashi Iwai wrote:
>>> On Thu, 10 Jun 2021 22:53:24 +0200,
>>> Pierre-Louis Bossart wrote:
>>>>
>>>> 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.
>>>>
>>>> This patch was originally submitted in 2017, c.f.
>>>> https://lore.kernel.org/alsa-devel/1494896518-23399-4-git-send-email-subhransu.s.prusty@intel.com/
>>>>
>>>> Suggested-by: Takashi Iwai <tiwai@suse.de>
>>>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>>> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
>>>> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
>>>
>>> This kind of flag itself was what I also introduced for another
>>> purpose, too.  There is a WIP patch that allows the use of
>>> non-coherent non-contiguous buffer pages, and this flag would fit for
>>> that.  FWIW, the patch is found at
>>>   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/log/?h=topic/memalloc
>>
>> Sorry Takashi, I missed your feedback on this patch.
>>
>> Are you saying I should use the definition in that patch?
>>
>> +#define SNDRV_DMA_TYPE_NONCONTIG	8	/* non-coherent SG buffer */
>>
>> I am not quite sure if how this is related to the application using mmap or not?
> 
> Not about that, but rather meant that some flag for disabling the mmap
> of PCM control record would be needed for other purposes like the
> above, too.  That is, this patch could be out of series and applied
> beforehand in my side.

Thanks Takashi, I will resubmit this separately. I may also break the series in two, the two parts (pause/L1EN support and rewinds) are not really connected, they help reduce power but at different levels.