[alsa-devel] [PATCH 2/3] ALSA: core: modify .ack callback to take arguments for updating appl ptr

[Takashi Iwai]

On Mon, 22 May 2017 07:22:19 +0200,
Vinod Koul wrote:
> 
> On Fri, May 19, 2017 at 10:01:04AM -0500, Pierre-Louis Bossart wrote:
> > On 5/19/17 1:27 AM, Takashi Iwai wrote:
> > >On Fri, 19 May 2017 05:57:05 +0200,
> > >Takashi Sakamoto wrote:
> > >>
> > >>Hi,
> > >>
> > >>On May 16 2017 10:01, Subhransu S. Prusty wrote:
> > >>>From: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> > >>>
> > >>>When appl_ptr is updated let low-level driver know, e.g.  to let the
> > >>>low-level driver/hardware pre-fetch data opportunistically.
> > >>>
> > >>>The existing .ack callback is extended with new attribute argument, to
> > >>>support this capability. Legacy driver subscribe to SND_PCM_ACK_LEGACY and
> > >>>doesn't process ack if it is not set. SND_PCM_ACK_APP_PTR can be used to
> > >>>process the application ptr update in the driver like in the skylake
> > >>>driver which can use this to inform position of appl pointer to host DMA
> > >>>controller. The skylake driver to process the SND_PCM_ACK_APP_PTR will be
> > >>>submitted with a separate patch set.
> > >>>
> > >>>In the ALSA core, this capability is independent from the NO_REWIND
> > >>>hardware flag. The low-level driver may however tie both options and only
> > >>>use the updated appl_ptr when rewinds are disabled due to hardware
> > >>>limitations.
> > >>>
> > >>>Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> > >>>Signed-off-by: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi at intel.com>
> > >>>Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty at intel.com>
> > >>>---
> > >>>  include/sound/pcm-indirect.h  |  4 ++--
> > >>>  include/sound/pcm.h           |  8 +++++++-
> > >>>  sound/core/pcm_lib.c          |  6 ++++--
> > >>>  sound/core/pcm_native.c       | 24 +++++++++++++++++++++++-
> > >>>  sound/mips/hal2.c             | 14 +++++++++++---
> > >>>  sound/pci/cs46xx/cs46xx_lib.c | 18 ++++++++++++++----
> > >>>  sound/pci/emu10k1/emupcm.c    |  8 ++++++--
> > >>>  sound/pci/rme32.c             | 15 ++++++++++++---
> > >>>  8 files changed, 79 insertions(+), 18 deletions(-)
> > >>
> > >>I think it better to take care of
> > >>'drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c' as
> > >>well. This is a driver for sound functionality of Raspberry PI 1/zero
> > >>and some people may still get influences from this work.
> > >>
> > >>$ git grep -A20 'struct snd_pcm_ops' v4.12-rc1 | grep \\.ack
> > >>v4.12-rc1:drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c-...
> > >>...
> > >
> > >Yep, we need to cover all codes if we really change the PCM ops.
> > 
> > The reason precisely why I preferred to avoid mucking with the
> > existing .ack(). we have a risk of introducing problems for legacy
> > and staging, not to mention out-of-tree.
> 
> Patching them should be fine, and I wont mind breaking out-of tree ones,
> isn't that a good thing :)

Heh, that's a fun as always :)

Actually it's a difficult judgment when to change the API and when
not.  This case is also in the gray zone.

If it were only additions of some new features, I'd think of avoiding
to change the existing API.  But, when it eventually fixes the bugs in
the existing drivers, we should not be too afraid of changing it.


Takashi