On Mon, 12 Sep 2016 18:03:16 +0200, Charles Keepax wrote:
On Mon, Sep 12, 2016 at 05:28:58PM +0200, Takashi Iwai wrote:
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:
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.
So looking at the Skylake code it does look like certainly the read opertaion returns a TLV header, its a bit less clear with the write operation it looks like it does use one but sometimes it will be passed straight through to the firmware.
So looks more like we really should take the pain of the ABI change and update wm_adsp to be consistent, two implementations is not good. Although I do feel we should add/strip the header in ASoC rather than expecting the driver callbacks to do so, seems odd to push that requirement into end drivers, it has certainly taken me by surpise. Also I guess with the current tinyalsa implementation the end user has to add/strip the headers whereas really I would have thought tinyalsa should be doing that.
If we can align the behavior, it'd be great. And, if we can fix the wm_adsp side, we can have the common handler of TLV inside the ASoC caller side, as one of Sakamoto-san's patches showed.
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?
So I guess the question would be if you couldn't read the controls size from info how would you find out the control size?
Well, I'm not against the idea to expose the size in callback. The behavior without read/write bits is just undefined, and we need a clear definition to avoid further confusion. I guess the introduction of a new flag would be the start.
thanks,
Takashi