On Fri, 16 Jun 2017 17:00:13 +0200, Takashi Sakamoto wrote:
On Jun 16 2017 04:06, Takashi Iwai wrote:
Some devices/drivers request applications to perform this, due to their design of hardware for data transmission.
Yes, and it's the default behavior of all non-x86 platforms, too.
Here, you have mixture of two items; architecture difference for cache coherent functionality, and device feature for data transmission. These two items are apparently different things.
In other words, devices can be supported independently of platform architectures. Why should I apply the solution prepared for cache coherency issue to add better support for the device with new feature for its data transmission? On x86 platform (although including some exceptions such as old ATOM processors), status/control data of PCM substream can successfully be mapped to process' VMA of applications. Why should it be disable it even if works well?
Because it doesn't work well for the new feature :)
Here, I'll show a table with chang history of relevant codes in kernel land, and information resources.
year | release | pcm | relevant changes | version | iface | ================================================================================== 2000 | v0.5.0 | 1.0.0 | status/control/fragments data can be mapped into VMA[1] 2003 | v0.9.0 | 2.0.5 | status/control data can be mapped independently[2] 2004 | v1.0.5 | 2.0.7 | SYNC_PTR command was introduced for PCM ioctl operation[3] 2004 | v1.0.7 | 2.0.7 | mmap for status/control data is allowed for x86/ppc/alpha[4]
In note for v1.0.5 release, Jaroslav described[5]:
- PCM midlevel - added SYNC_PTR ioctl (for problematic cache coherency archs)The SYNC_PTR and architecture-dependent enabling of the mmap was originally introduced to solve cache coherent issue.
It's becoming again like a typical situation you falling often into while discussing with others; your new term is too ambiguous and unique to parse without a clarification. PLEASE, clarify your idea in a more understandable way, at best with a (pseudo-) code snippet.
The above.
So... what's the "new"? That is what I don't understand... It's the already existing model deployed without mmap. Nothing new.
This subsystem has no drivers with the similar feature, thus it's new design for data transmission. The design supports below things:
- Hardware features:
1.1. Data transmission is done by direct media access (DMA). 1.2. Hardware cares of two points for its data transmission; one is hwptr and another is appl_ptr on PCM buffer dedicated for the data transmission. 1.3. (perhaps)The granurarity of data transmission can be differed, not fixed to the size of 'period'.
- Drivers are designed with below items:
2.1. Return SNDRV_PCM_ACCESS_MMAP_XXX flag 2.2. Implement for 'struct snd_pcm_ops.ack' to tell appl_ptr to hardware.
- Applications should perform according to below items:
3.1. Operate with SNDRV_PCM_IOCTL_[READ|WRITE][N|I] to drive kernel land stuffs with changed appl_ptr. 3.2. Or operate with SYNC_PTR to driver kernel land stuffs when handling any PCM frames.
As of v4.12-rc5, there's no stuffs to satisfy all of these items.
It works on non-x86 systems as is with 4.12.
It's clear that we're working to support devices with the new design.
Yeah, I sort of understand you're sticking with "new design". So, no need to argue about it. It doesn't matter whether it's really so shiny new or not. It's something we need to fix, after all.
In other words, such a driver already works fine on non-x86 platforms. It's "broken" only on x86 due to the forced mmap usage.
It's better to distinguish two issues; architecture's support for cache coherency and support for new type of device.
Well, that's not really true. Most of drivers worked so far with a luck. Fortunately, the hardware that doesn't have the mappable hardware buffer can be woken up well enough via period setup, thus it could synchronize appl_ptr that user-space already changed in the past. This, however, doesn't mean that the current x86 mmap is perfectly working.
Imagine that you stop the stream at the middle of period chunk. For the hardware with the mapped h/w buffer, it can play up to the aborted position. OTOH, for the hardware without mapped buffer, it can't reach at that position because the sync of appl_ptr was missing before the period elapsed. If the appl_ptr could have been notified via sync_ptr, the driver could copy the data, and it could reach to the aborted point.
So, currently it effectively enforces the BATCH style buffer although it doesn't have to be so. It's a known bug by the status / control mmap, but we've ignored this just because such hardware are minor and the problem itself is trivial. But you see that the current code is even not perfect for the existing hardware.
And, I think you miss a few points, thus the argument was twisted.
- The primary goal is to achieve the notification of appl_ptr change to kernel.
It's for the purpose to support devices which have the new design for data transmission. This is the reason that I agree with upgrading version of PCM interface. I suggest adding new info flags for the specific purpose.
Disabling mmap for status/control data of PCM substream is just to support architectures to which ALSA PCM core judge non cache coherency. It's not good to utilize it as a solution of this issue because of abuse of interfaces.
No, it's no abuse. The sync_ptr is the correct and designed API to notify appl_ptr update from the user space to kernel. For most of device drivers on x86, this isn't requested strictly, thus it's not done when the PCM control is mmapped. We've been just lucky, so far.
I described that it was originally designed to solve architecture's support for cache coherency. Don't depend on extra bonus from it.
Oh well... Please stop such a fundamentalism argument. The original purpose doesn't mean to limit its usage. We aren't arguing history or religion.
That is, letting user-space to use the sync_ptr is the right strategy from the API POV, and it can be achieved by the disablement of mmap. Unfortunately, we can't currently disable only PCM control mmap, but we have to disable both control and status PCM mmaps.
Usage of SYNC_PTR ioctl command and disabling mmap are not tight-coupled. Both of them can be used in the same time if user land stuffs is writte so.
Sure, it's a semantic difference. I don't pursue this way if we are allowed to extend ABI, either. This will be the "phase 2".
Remember that my suggestion is for the "phase 1", to fix the stuff for the existing applications without changing the ABI.
It's a trade-off, but still good enough for the driver requesting it (the Intel one, which can achieve the deep sleep by that).
What's the 'deep sleep'? Please explain about it when you introduce new words into this discussion.
I thought Pierre (or other Intel people) already mentioned that. Or maybe it's in a different patchset. In anyway...
The chip (DSP) can prefetch the data on the buffer and go to a deep sleep mode. It's the reason why appl_ptr update is needed. Then we can know how much data can be prefetched. This should be the great reduction of power, thus a slight increase of instructions would be like a peanut.
- The kernel needs to work with the existing user-space. It's a MUST.
I described in a previous message.
Let me repeat: the support of old user-space is a must. This is the fact. Not faked.
On Jun 14 2017 23:34, Takashi Sakamoto wrote:
When working with old userspace stuffs, drivers run with old mode (namely, run for current interface). The difference of interface version is absorbed in alsa-lib as much as possible, then application runs without large change
... and how the kernel knows that a newer alsa-lib is running, BTW?
There is no such information right now, and it's why I suggested an extension of ABI.
Enough information is not provided yet. Furthermore, I have few interests in it at present.
Right, not yet. That's the point. If we want to achieve the fix without changing the user-space, you can't rely on this not-yet-present information.
- appl_ptr or whatever a status *is* maintained in kernel -- or better to say, it's kept in both kernel and user-space. (Otherwise why it becomes a problem now?)
This is not achieved in current implementation of ALSA PCM core yet.
Sigh, again the misunderstanding due to the ambiguous term usage.
The appl_ptr value is tracked in user-space and kernel sides -- so they are "maintained" in both sides. "Maintain a value" doesn't define who triggers the value change.
The important thing for this issue is 'how to deliver the position of appl_ptr from applications to hardware'. In current implementation, when applications operate for mapped PCM buffer, kernel land stuffs don't have correct appl_ptr. Hardware is given ways to get appl_ptr and drivers don't assist it because appl_ptr is in process's VMA of applications.
Yes. That's what I called synchronization in the below.
Even if hardware generates any event to get current application-side position on PCM buffer, it can't be achieved. There's no way for hardwares. Therefore, it's unavoided to request userspace to have care of the above pseudo code, when supporting devices with the new design.
Yes, the missing piece is the synchronization between the appl_ptr values maintained in both user-space and kernel sides. And it's only on x86, and the simplest way to fix it is to disable the PCM control mmap.
It's not better to depend on bonus from architecture differences. Provide enough information to userspace via interface.
So, if you have a better idea for achieving the goal without changing the current ABI, please tell us. It'd be really appreciated!
For the future development with the ABI extension, we may do implement in a different level, yes. Basically we can change everything. But this is not the thing we need to fix right now.
I'm open for any changes with the ABI extension. It's certainly exciting. But, don't mix up this with the solution for the current implementation.
Your idea to extend current ABI includes some jumps from current position of discussion. At least, the reason of it is somehow based on your idea to disable mmap for control/status data of PCM substream for some devices/drivers. As already stated, I'm against it with several reasons. On cache coherent architecture, there's no matter to consider about it. Thus they're not matters to me at present.
It's fine that you say against it. But then please state your idea how to implement the appl_ptr notification for the existing application ABI? Without it, the argument is simply useless.
I already prepared patches for my ideas several days ago. However, I have hesitation to post it because you have mixtures of two issues, which I described. As long as you adhere to the mixtures, my patches are not evaluated in a appropriate logic, thus I'm unwilling to post it. I'd not like to waste of my private time.
Sakamoto-san, if you have the idea to fix the behavior, PLEASE PLEASE show it at first!!! This is greatly appreciated.
I meant here a solution that works with the currently existing application as is -- i.e. change only in the kernel side. If this can be achieved without disabling mmap, it's very interesting. Please let me know.
Once we fix this for the existing applications, then we can go further, a better implementation with the extension of ABI.
BTW, mentioning about waste of your private time doesn't sound so impressive, honestly speaking. I noticed it often appearing in your patch changelogs. Most of people (including me) spend private time for developing Linux kernel stuff, too. My own main job is not maintaining ALSA, either.
thanks,
Takashi
Also, think again which disadvantages would disabling the control mmap (not the status mmap) have, when sync_ptr is mandatory. Or better in another form: suppose you need to use sync_ptr for appl_ptr update, what merit would you have by keeping the PCM control mmap...?
Take a look at the content of snd_pcm_mmap_control. It's only appl_ptr and avail_min where the latter can be ignored mostly. That is, disabling the PCM control mmap is effectively equivalent with requesting the appl_ptr update via sync_ptr.
So, if we can disable only the PCM control mmap, the efficiency won't be lost for kernel -> user-space PCM status update by keeping the PCM status mmap. It's the plan I mentioned as the ABI extension.
To recap:
- The disablement of control/status mmap is allowed by request from a driver to enforce the sync_ptr usage for appl_ptr update.
Not fair. The feature to disable mmap is just for issues from architecture feature. It's not good to utilize it for the other kind of issues. It surely puzzle developers in user space. When working for x86 architecture, developers just consider about content on mapped control/status data. We soulnd not change it.
- The extension of ABI for further improvements; then we can keep mmap of PCM status while disabling the PCM control mmap.
I'm not interested in issues directly relevant to current issue. I'd like to just focus on the current one.
[1] ftp://ftp.alsa-project.org/pub/driver/alsa-driver-0.5.0.tar.gz [2] ftp://ftp.alsa-project.org/pub/driver/alsa-driver-0.9.0rc8.tar.bz2 [3] http://git.alsa-project.org/?p=alsa-driver.git;a=commitdiff;h=60300f540726;h... [4] http://git.alsa-project.org/?p=alsa-driver.git;a=commitdiff;h=6cd0240aa173;h... [5] https://lwn.net/Articles/87517/
Regards
Takashi Sakamoto