Re: [PATCH 0/4] Yet another patchset for LPE audio refactoring

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 2/6/17 1:07 PM, Takashi Iwai wrote:
> On Fri, 03 Feb 2017 17:43:56 +0100,
> Takashi Iwai wrote:
>>
>> Hi,
>>
>> this is the final part of my cleanup / refactoring patchsets for Intel
>> LPE audio.  Now the PCM stream engine has been rewritten.  It supports
>> more flexible configuration, not only fixed 4 periods, for example,
>> yet with a lot of code reduction.
>>
>> As usual, the patches are found in topic/intel-lpe-audio-cleanup
>> branch.
>
> So far, from my side, now most of changes have been submitted /
> merged.  But I still have some unclear things in the code.
>
> - The PCM format: the code contains the handling of S16_LE, but it
>   doesn't seem work when I enabled the info bit.  What's missing?
>   Also U24_LE format is ever supported...?  How?

In the initial version the samples were assumed to be using the 24 lsb 
of a 32-bit word. 16-bit data can be supported as long as it was 
left-shifted before being written to the ring buffer.

For stereo the channels are assumed to be interleaved in Layout 0 (left, 
right). For more than 2ch, all channels for the same sample are inserted 
consecutively (ie. no blocks of channels). In the initial version the 
number of channels is assumed to be even and one bogus channel had to be 
inserted for odd number of channels.

in more recent versions the samples could also be represented as using 
the 24 msb and the requirement for the bogus channel was removed but I 
never worked on this personally and the documentation is far from clear.

At any rate, here's what I see in the description of the 
STREAM_X_LPE_AUD_CONFIG registers for CHT:

Bit 15:
1: DP mode
0: HDMI (default)

Bit14:
1: no bogus sample for odd channels
0: bogus sample for off channels (default)

Bit13:
1: MSB is bit 31 of 32-bit container
0: MSB is bit 23 of 32-bit container (default)

Bit 12:
1: 16-bit container
0: 32-bit container (default)

Bit 11:
1: send silence stream
0: send null packets (default)

I see no evidence that unsigned data is supported.

I would really take all these configurations with caution, it's not 
clear to me if the documentation reflects the actual implementation.

Note that the hardware can swap channels, I don't remember if this is 
currently enabled.

>
> - The behavior in had_process_buffer_underrun() isn't clear enough.
>   Or, more exactly, the behavior of AUD_HDMI_STATUS reg and AUD_CONFIG
>   reg look complex, and need a bit more description for readers
>   (including me).
>
> Can you guys enlighten a bit?

For the LPE_AUDIO_Buffer_config register (65020h), there is one 
important field
10:8: DMA Fifo watermark. The audio unit has 8x64 bytes, this register 
define when to fetch data. Bottom line it'd be wise to avoid rewinding 
below that FIFO size...


There are two possible interrupt sources in the 
STREAM_X_LPE_AUD_HDMI_STATUS (65064h)
Bit 31: sample buffer underrun. HDMI audio unit did not have enough 
samples to insert in video frame (cleared by writing a one)
Bit 29: buffer done status, write 1 to clear
Bit 15: sample buffer underrun interrupt enable

Hope this helps
-Pierre


>
>
> thanks,
>
> Takashi
>