[Sound-open-firmware] [PATCH] cnl: ssp: fix DSP_B setting in ssp

[Pierre-Louis Bossart]

On 3/7/18 7:43 PM, rander.wang wrote:
> Hi Pierre,
> 
> On 3/7/2018 10:01 PM, Pierre-Louis Bossart wrote:
>> On 3/6/18 10:31 PM, Rander Wang wrote:
>>> set ssp according to DSP_B spec
>>>
>>> Signed-off-by: Rander Wang <rander.wang at linux.intel.com>
>>> ---
>>>   src/drivers/apl-ssp.c | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/drivers/apl-ssp.c b/src/drivers/apl-ssp.c
>>> index a726f3c..c32bd35 100644
>>> --- a/src/drivers/apl-ssp.c
>>> +++ b/src/drivers/apl-ssp.c
>>> @@ -333,10 +333,11 @@ static inline int ssp_set_config(struct dai *dai,
>>>           sscr0 |= SSCR0_MOD | SSCR0_FRDC(config->num_slots);
>>>             /* set asserted frame length */
>>> -        frame_len = config->sample_container_bits;
>>> +        frame_len = 1;
>>
>> this needs to be done for DSP_B as well.
> 
>      you mean DSP_A ? yes, this patch is for DSP_B,  next DSP_A will be 
> validated

yes

> 
>>
>>>             /* handle frame polarity, DSP_A default is rising/active 
>>> high */
>>> -        sspsp |= SSPSP_SFRMP(inverted_frame);
>>> +        sspsp |= SSPSP_SFRMP(!inverted_frame);
>>
>> I don't think this is correct.  the documentation says with this bit 
>> asserted (value=1) the frame is active high which is exactly what we 
>> want.
>>
>      No, the frame active low is what we want according to the ASOC 
> spec.  Data is sent when active low, frame sync is at active high.

Look at the diagrams I shared offline, this is not what you are 
describing. We want the frame sync to be indicated by a low->high 
transition (rising edge) immediately following a falling edge of the bit 
clock.

> 
>      And without this modification, the frame sync clock is rotated 180 
> compared to spec. And it is validated by Zhigang on APL and
> 
>     also it is set by COE
> 
>>
>>> +        sspsp |= SSPSP_FSRT;
>>
>> why is this necessary? this disables a fix for dummy stop/padding.
>      it is get from COE setting.

that's not an explanation, sorry.

>>
>>>             break;
>>>       default:
>>>
>>