[Sound-open-firmware] [PATCH] apl-ssp: fix capture double speed issue

[Keyon Jie]

On 2018年03月15日 02:08, Pierre-Louis Bossart wrote:
> 
>>>>>    /* save SSP context prior to entering D3 */
>>>>>    static int ssp_context_store(struct dai *dai)
>>>>>    {
>>>>> @@ -96,6 +109,9 @@ static inline int ssp_set_config(struct dai *dai,
>>>>>        uint32_t dummy_stop;
>>>>>        uint32_t frame_len = 0;
>>>>>        uint32_t bdiv_min;
>>>>> +    uint32_t active_tx_slots = 2;
>>>>> +    uint32_t active_rx_slots = 2;
>>>>
>>>> why do we use 2 as default value, for stereo. ?
>>>>
>>>> This value should always be set based on channel mask so I would set 
>>>> to 0
>>>> here
>>>> and check that it's not 0 later on (return error and produce trace 
>>>> if 0).
>>>
>>> this was an 'optimization' based on the fact that for I2S/LEFT_J we can
>>> keep this as stereo.
>>> If we don't set those then we have to manually set this to two in the
>>> switch case.
>>>
>>
>> Some of the data used comes from topology so we would need to make 
>> sure that
>> topology tx/rx slot mask would be non zero as they are used to 
>> multiply this
>> value;.

yes, that's true, as Pierre said, we copy this from ssp.c, shall we 
optimize that later, together with ssp.c?

> 
> For I2S and LeftJ I don't think we can have a mono solution or more than 
> 2 channels, we might as well ignore the channel mask to use 2 slots, 
> always.
> 
>>
>>>>
>>>>> +
>>>>>        bool inverted_frame = false;
>>>>>        int ret = 0;
>>>>> @@ -353,6 +369,9 @@ static inline int ssp_set_config(struct dai *dai,
>>>>>            sspsp |= SSPSP_SFRMP(!inverted_frame);
>>>>>            sspsp |= SSPSP_FSRT;
>>>>> +        active_tx_slots = hweight_32(config->tx_slot_mask);
>>>>> +        active_rx_slots = hweight_32(config->rx_slot_mask);
>>>>> +
>>>>>            break;
>>>>>        case SOF_DAI_FMT_DSP_B:
>>>>> @@ -371,6 +390,9 @@ static inline int ssp_set_config(struct dai *dai,
>>>>>             */
>>>>>            sspsp |= SSPSP_SFRMP(!inverted_frame);
>>>>> +        active_tx_slots = hweight_32(config->tx_slot_mask);
>>>>> +        active_rx_slots = hweight_32(config->rx_slot_mask);
>>>>> +
>>>>>            break;
>>>>>        default:
>>>>>            trace_ssp_error("eca");
>>>>> @@ -408,6 +430,9 @@ static inline int ssp_set_config(struct dai *dai,
>>>>>        /* bypass divider for MCLK */
>>>>>        mdivr = 0x00000fff;
>>>>> +    sscr3 |= SSCR3_TX(active_tx_slots * config->sample_valid_bits /
>>>>> 8) |
>>>>> +         SSCR3_RX(active_rx_slots * config->sample_valid_bits /
>>>>> 8);
>>>>> +
>>>
>>> For Baytrail, we had:
>>>
>>>       /* FIXME:
>>>        * watermarks - (RFT + 1) should equal DMA SRC_MSIZE
>>>        */
>>>       sfifott = (SFIFOTT_TX(2*active_tx_slots) |
>>>              SFIFOTT_RX(2*active_rx_slots));
>>>
>>> Don't we need to align RFT/TFT to the DMA_SRC_MSIZE as well? e.g. for 24
>>> bits can we actually have a threshold that would be a multiple of 3??
>>
>> Not sure, iirc one of the watermark registers is deprecated.
> 
> Let's ask a different question: does this work with 24 bits with a DMA 
> handling powers of two?

that's actually a concern from me also after sent this out, maybe I need 
rework to treat 24bits as 32 bits for this case?

Thanks,
~Keyon


> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
>