| Message ID | 57C91FD8.1090205@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 02, 2016 at 02:44:40PM +0800, Mengdong Lin wrote: > On 08/24/2016 01:41 AM, Mark Brown wrote: > > This seems *incredibly* implementation specific. Why wouldn't the > > driver for the thing implementing the topology be able to figure out the > > ordering here? What's the use case? What happens when we change away > > from DPCM? > There is another patch (04/13) to add generic flags and flag mask to PCM > objects. So we'll allow users to set DPCM trigger ordering as flags as > below, to avoid using trigger[] in ABI. The topology kernel driver will > check the flag bits and set the proper trigger ordering to FE DAI links. If > we change away from DPCM in the future, user can stop using these flags. And > the 32-bit flags seems enough for future extension. This doesn't seem much better to be honest, it's just shuffling the problem around. Why do these things need to be triggered in this particular order and why is that invisible to the system?
+ Rakesh Hi Hardik/Vinod/Rakesh, Would you please share more info about DPCM trigger ordering used in current ADSP firmware? And why we hope to configure this by topology? Here is Mark's question "Why do these things need to be triggered in this particular order and why is that invisible to the system?". Please see link http://mailman.alsa-project.org/pipermail/alsa-devel/2016-September/112539.html My mail client lost Mark's mail so I paste the link here. Thanks Mengdong On 09/02/2016 02:44 PM, Mengdong Lin wrote: > > > On 08/24/2016 01:41 AM, Mark Brown wrote: >> On Fri, Aug 19, 2016 at 06:12:55PM +0800, mengdong.lin@linux.intel.com >> wrote: >> >>> Definition of dynamic PCM trigger ordering is exposed to uapi asoc.h, >>> and topology allows user space to define the trigger ordering for PCM >>> (FE links). >> >> This seems *incredibly* implementation specific. Why wouldn't the >> driver for the thing implementing the topology be able to figure out the >> ordering here? What's the use case? What happens when we change away >> from DPCM? >> > > There is another patch (04/13) to add generic flags and flag mask to PCM > objects. So we'll allow users to set DPCM trigger ordering as flags as > below, to avoid using trigger[] in ABI. The topology kernel driver will > check the flag bits and set the proper trigger ordering to FE DAI links. > If we change away from DPCM in the future, user can stop using these > flags. And the 32-bit flags seems enough for future extension. > > diff --git a/include/uapi/sound/asoc.h b/include/uapi/sound/asoc.h > index f734bea..30da32f 100644 > --- a/include/uapi/sound/asoc.h > +++ b/include/uapi/sound/asoc.h > @@ -130,6 +130,16 @@ > #define SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_CHANNELS (1 << 1) > #define SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_SAMPLEBITS (1 << 2) > > +/* DAI link flags */ > +#define SND_SOC_TPLG_LNK_FLGBIT_IGNORE_SUSPEND (1 << 0) > +#define SND_SOC_TPLG_LNK_FLGBIT_IGNORE_POWERDOWN_TIME (1 << 1) > +#define SND_SOC_TPLG_LNK_FLGBIT_PLAYBACK_DPCM_TRIGGER_PRE (1<<2) > +#define SND_SOC_TPLG_LNK_FLGBIT_PLAYBACK_DPCM_TRIGGER_POST (1<<3) > +#define SND_SOC_TPLG_LNK_FLGBIT_PLAYBACK_DPCM_TRIGGER_BESPOKE (1<<4) > +#define SND_SOC_TPLG_LNK_FLGBIT_CAPTURE_DPCM_TRIGGER_PRE (1<<2) > +#define SND_SOC_TPLG_LNK_FLGBIT_CAPTURE_DPCM_TRIGGER_POST (1<<3) > +#define SND_SOC_TPLG_LNK_FLGBIT_CAPTURE_DPCM_TRIGGER_BESPOKE (1<<4) > + > /* > * Block Header. > * This header precedes all object and object arrays below. > @@ -439,6 +449,8 @@ struct snd_soc_tplg_pcm { > struct snd_soc_tplg_stream > stream[SND_SOC_TPLG_STREAM_CONFIG_MAX]; /* for DAI link */ > __le32 num_streams; /* number of streams */ > struct snd_soc_tplg_stream_caps caps[2]; /* playback and > capture for DAI */ > + __le32 flag_mask; /* bitmask of flags to configure */ > + __le32 flags; /* SND_SOC_TPLG_LNK_FLGBIT_* flag value */ > } __attribute__((packed)); > > > Thanks > Mengdong
Hi Mark, We'll drop support for configuring DPCM trigger ordering in topology, and the driver will set this in kernel. I'll submit the v2 series. Please review. Thanks Mengdong On 09/06/2016 02:15 PM, Mengdong Lin wrote: > + Rakesh > > > Hi Hardik/Vinod/Rakesh, > > Would you please share more info about DPCM trigger ordering used in > current ADSP firmware? And why we hope to configure this by topology? > > Here is Mark's question "Why do these things need to be triggered in > this particular order and why is that invisible to the system?". > > Please see link > http://mailman.alsa-project.org/pipermail/alsa-devel/2016-September/112539.html > > > My mail client lost Mark's mail so I paste the link here. > > Thanks > Mengdong > > On 09/02/2016 02:44 PM, Mengdong Lin wrote: >> >> >> On 08/24/2016 01:41 AM, Mark Brown wrote: >>> On Fri, Aug 19, 2016 at 06:12:55PM +0800, mengdong.lin@linux.intel.com >>> wrote: >>> >>>> Definition of dynamic PCM trigger ordering is exposed to uapi asoc.h, >>>> and topology allows user space to define the trigger ordering for PCM >>>> (FE links). >>> >>> This seems *incredibly* implementation specific. Why wouldn't the >>> driver for the thing implementing the topology be able to figure out the >>> ordering here? What's the use case? What happens when we change away >>> from DPCM? >>> >> >> There is another patch (04/13) to add generic flags and flag mask to PCM >> objects. So we'll allow users to set DPCM trigger ordering as flags as >> below, to avoid using trigger[] in ABI. The topology kernel driver will >> check the flag bits and set the proper trigger ordering to FE DAI links. >> If we change away from DPCM in the future, user can stop using these >> flags. And the 32-bit flags seems enough for future extension. >> >> diff --git a/include/uapi/sound/asoc.h b/include/uapi/sound/asoc.h >> index f734bea..30da32f 100644 >> --- a/include/uapi/sound/asoc.h >> +++ b/include/uapi/sound/asoc.h >> @@ -130,6 +130,16 @@ >> #define SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_CHANNELS (1 << 1) >> #define SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_SAMPLEBITS (1 << 2) >> >> +/* DAI link flags */ >> +#define SND_SOC_TPLG_LNK_FLGBIT_IGNORE_SUSPEND (1 << 0) >> +#define SND_SOC_TPLG_LNK_FLGBIT_IGNORE_POWERDOWN_TIME (1 << 1) >> +#define SND_SOC_TPLG_LNK_FLGBIT_PLAYBACK_DPCM_TRIGGER_PRE (1<<2) >> +#define SND_SOC_TPLG_LNK_FLGBIT_PLAYBACK_DPCM_TRIGGER_POST (1<<3) >> +#define SND_SOC_TPLG_LNK_FLGBIT_PLAYBACK_DPCM_TRIGGER_BESPOKE (1<<4) >> +#define SND_SOC_TPLG_LNK_FLGBIT_CAPTURE_DPCM_TRIGGER_PRE (1<<2) >> +#define SND_SOC_TPLG_LNK_FLGBIT_CAPTURE_DPCM_TRIGGER_POST (1<<3) >> +#define SND_SOC_TPLG_LNK_FLGBIT_CAPTURE_DPCM_TRIGGER_BESPOKE (1<<4) >> + >> /* >> * Block Header. >> * This header precedes all object and object arrays below. >> @@ -439,6 +449,8 @@ struct snd_soc_tplg_pcm { >> struct snd_soc_tplg_stream >> stream[SND_SOC_TPLG_STREAM_CONFIG_MAX]; /* for DAI link */ >> __le32 num_streams; /* number of streams */ >> struct snd_soc_tplg_stream_caps caps[2]; /* playback and >> capture for DAI */ >> + __le32 flag_mask; /* bitmask of flags to configure */ >> + __le32 flags; /* SND_SOC_TPLG_LNK_FLGBIT_* flag >> value */ >> } __attribute__((packed)); >> >> >> Thanks >> Mengdong
diff --git a/include/uapi/sound/asoc.h b/include/uapi/sound/asoc.h index f734bea..30da32f 100644 --- a/include/uapi/sound/asoc.h +++ b/include/uapi/sound/asoc.h @@ -130,6 +130,16 @@ #define SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_CHANNELS (1 << 1) #define SND_SOC_TPLG_DAI_FLGBIT_SYMMETRIC_SAMPLEBITS (1 << 2) +/* DAI link flags */ +#define SND_SOC_TPLG_LNK_FLGBIT_IGNORE_SUSPEND (1 << 0) +#define SND_SOC_TPLG_LNK_FLGBIT_IGNORE_POWERDOWN_TIME (1 << 1) +#define SND_SOC_TPLG_LNK_FLGBIT_PLAYBACK_DPCM_TRIGGER_PRE (1<<2) +#define SND_SOC_TPLG_LNK_FLGBIT_PLAYBACK_DPCM_TRIGGER_POST (1<<3) +#define SND_SOC_TPLG_LNK_FLGBIT_PLAYBACK_DPCM_TRIGGER_BESPOKE (1<<4) +#define SND_SOC_TPLG_LNK_FLGBIT_CAPTURE_DPCM_TRIGGER_PRE (1<<2) +#define SND_SOC_TPLG_LNK_FLGBIT_CAPTURE_DPCM_TRIGGER_POST (1<<3) +#define SND_SOC_TPLG_LNK_FLGBIT_CAPTURE_DPCM_TRIGGER_BESPOKE (1<<4) + /* * Block Header. * This header precedes all object and object arrays below. @@ -439,6 +449,8 @@ struct snd_soc_tplg_pcm { struct snd_soc_tplg_stream stream[SND_SOC_TPLG_STREAM_CONFIG_MAX]; /* for DAI link */ __le32 num_streams; /* number of streams */ struct snd_soc_tplg_stream_caps caps[2]; /* playback and capture for DAI */ + __le32 flag_mask; /* bitmask of flags to configure */ + __le32 flags; /* SND_SOC_TPLG_LNK_FLGBIT_* flag value */ } __attribute__((packed));