On Mon, 12 Sep 2016 17:25:31 +0200, Vinod Koul wrote: > > On Mon, Sep 12, 2016 at 01:37:37PM +0100, Charles Keepax wrote: > > On Tue, Sep 06, 2016 at 12:30:35PM +0900, Takashi Sakamoto wrote: > > > On Sep 5 2016 05:45, Takashi Iwai wrote: > > > >Yeah, there are obviously some issues in the current implementation of > > > >wm_adsp and ASoC ext ctl. Although I'll unlikely take Sakamoto-san's > > > >patchset as is from a few reasons, these issues still should be > > > >addressed. > > > > > > OK. I welcome to abandon this patchset ;) > > > > > > >First off, passing the binary blob directly via TLV callback is > > > >incorrect from the ABI perspective. When Vinod proposed the idea via > > > >TLV access originally, we thought they the data is encoded in TLV > > > >format. Alas, the resulted code didn't do that and it slipped into > > > >the upstream without consideration. > > > > > > +1 > > > > > > >Besides that, the second problem is the count value returned via > > > >snd_ctl_elem_info, as mentioned in the above. It's beyond the > > > >original control API design, and a kind of illegal usage. > > > > > > +1 > > > > > > >(Well, it's a philosophical argument: what one would expect for an > > > > element that has neither read nor write access...?) > > > > > > It's an element with no sense for applications. A waste of codes in kernel > > > land. > > > > > > >So, at this point, the main question is whether we keep this access > > > >pattern as is, as a sort of official one, and put some exceptional > > > >rule. Charles, how is the situation? Has it been already deployed to > > > >real systems? > > > > > > > >If we may still change the wm_adsp behavior, we may "fix" the first > > > >issue by passing the blob properly encoded in TLV format, at least. > > > >OTOH, if we need to keep the current ABI abuse as is, one idea is to > > > >add a special flag in SNDRV_CTL_ELEM_ACCESS_* indicating this kind of > > > >control, and we define more strictly how the code should be > > > >implemented. Currently we can judge this element as a one that has no > > > >read/write access but with tlv r/w. But it's way too unclear. > > > > > > The 'abuse' is a part of my understanding of ALSA SoC part. I need a bit > > > time to switch my mind for this issue. > > > > Perhaps we should add this as a topic for discussion at the Audio > > mini-conference? If the general feeling is that this feature is > > badly designed we should certainly be looking at what we can do > > to improve it. > > > > I do very much like the idea of the additional access flag as I > > said. Wrapping the data in a TLV structure we would have to think > > about a little more though as the code has shipped in several > > kernel versions at this point. > > > > We are starting to have a few customers use a 4.4 kernel which > > does include these controls, all our previous backports had used > > a system of partitioning the controls up into multiple 512 byte > > controls as these binary TLV controls were not supported. So > > there is a little bit of friction to major changes to the ABI, > > but I don't think its insurmountable as long as the functionality > > remains the same. > > > > But we need to get Intel involved in that discussion too, as they > > have used these controls quite widely as well. > > Sorry havent been able to follow this yet :( > > > But yes current Skylake Chromebooks ship with this code so we cant break it. > > I am not sure what is the issue with API though. (sorry haven't read the > thread yet). The tlv was designed to allow people send bytes larger than 512 > down to kernel. The Type cna be anything (implementation specific, though we > haven't used it yet), length the blob length and then the bytes blob. Yes, and this part is missing in wm_adsp driver. It passes the blob without TLV encoding, i.e. starting from the offset zero without type and length encoding. > We provide a tunnel and pass these to DSP. They maybe module coefficients, > hotwording blobs etc. So, does Intel driver pass the blob in TLV format? Then we have two different implementations. Also, still another point is to be decided: is passing an arbitrary size via info callback for an element without read/write access bits (but with TLV bit) a right behavior? > Sure, we can this at u-conf :) F2F is always much better ... Well, the u-conf is still over a month ahead. I think we should keep discussing on ML before u-conf. thanks, Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
