thesofproject / linux

Linux kernel source tree
Other
89 stars 129 forks source link

[FEATURE] Support ssp early start of MCLK/BCLK in IPC4 #4514

Open Vamshigopal opened 1 year ago

Vamshigopal commented 1 year ago

Description

Some codecs need SSP bit clock to start before data is provided and stop after codec is down.

With IPC4 we are enabling mclk and bclk during SSP blob set (hw_params stage).
https://github.com/zephyrproject-rtos/zephyr/blob/0a2d538c2bc442e64b3b86f3cf05a311223466ac/drivers/dai/intel/ssp/ssp.c#L1729

In IPC3 we are supporting early mclk and bclk with two clck control bits, SOF_DAI_INTEL_SSP_CLKCTRL_MCLK_ES and SOF_DAI_INTEL_SSP_CLKCTRL_BCLK_ES. MCLK and/or BCLK will start during hw_params and stop during hw_free if the coresponding bit is set.

Next steps Identify the gaps for early MCLK/BCLK impementation in IPC4

plbossart commented 1 year ago

The IPC4 protocol already provides the means to transmit information that the clocks and possibly DMA shall be transmitted earlier. see in SOF src/include/ipc4/ssp.h

struct ipc4_ssp_start_control {
    /* delay in msec between enabling interface (moment when
     * Copier instance is being attached to the interface) and actual
     * interface start. Value of 0 means no delay.
     */
    uint32_t clock_warm_up    : 16;

    /* specifies if parameters target MCLK (1) or SCLK (0) */
    uint32_t mclk             : 1;

    /* value of 1 means that clock should be started immediately
     * even if no Copier instance is currently attached to the interface.
     */
    uint32_t warm_up_ovr      : 1;
    uint32_t rsvd0            : 14;
} __attribute__((packed, aligned(4)));

struct ipc4_ssp_stop_control {
    /* delay in msec between stopping the interface
     * (moment when Copier instance is being detached from the interface)
     * and interface clock stop. Value of 0 means no delay.
     */
    uint32_t clock_stop_delay : 16;

    /* value of 1 means that clock should be kept running (infinite
     * stop delay) after Copier instance detaches from the interface.
     */
    uint32_t keep_running     : 1;

    /* value of 1 means that clock should be stopped immediately */
    uint32_t clock_stop_ovr   : 1;
    uint32_t rsvd1            : 14;
} __attribute__((packed, aligned(4)));

These values should be part of the

struct ipc4_ssp_configuration_blob {
    union ipc4_gateway_attributes gw_attr;

    /* TDM time slot mappings */
    uint32_t tdm_ts_group[I2S_TDM_MAX_SLOT_MAP_COUNT];

    /* i2s port configuration */
    struct ipc4_ssp_driver_config i2s_driver_config;

    /* optional configuration parameters */
    union ipc4_ssp_dma_control i2s_dma_control[];
} __attribute__((packed, aligned(4)));

so my take is that the SSP blobs are missing this information in the topology blobs, or somehow the firmware doesn't yet deal with them.

Best to ask @RanderWang who's got a better understanding than me of all those gateway configurations.

RanderWang commented 1 year ago

ipc4_ssp_dma_control is optional configuration parameters, and it is not supported by both kernel & FW (include zephyr part). In FW it will be checked with

i2s_dma_control_size = blob config size  - sizeof(gw_attr + tdm_ts_group + i2s_driver_config);
if (i2s_dma_control_size > 0) {
       // process it
}
plbossart commented 1 year ago

I am not clear on what the kernel is supposed to do, this sort of configuration must come from SSP blobs which are passed as is to the firmware.

isn't this rather a NHLT/topology and firmware issue (possibly both)?

@ranj063 @mwasko what am I missing?

I am not pushing back in the ask, that's a P1 priority in my book, we absolutely need this capability for MTL products - just not clear at which level we need to change the code.

ranj063 commented 1 year ago

@plbossart yes this is a NHLT issue but it is not clear to me if the information about the early start is part of the existing blobs or an extension to it. If it is the latter, then we may have something to do with it in the kernel about it and then there's the question of how to handle this in the FW. Today we only start the SSP during PCM trigger. This would also need to be changed isnt it?

plbossart commented 1 year ago

@ranj063 I would think we have the ability to generate the blobs already, as measured by this in the topology plugin

static int set_ext_config(struct intel_nhlt_params *nhlt, snd_config_t *cfg, snd_config_t *top)
{
    long mclk_policy_override;
    long mclk_always_running;
    long mclk_starts_on_gtw_init;
    long mclk_starts_on_run;
    long mclk_starts_on_pause;
    long mclk_stops_on_pause;
    long mclk_stops_on_reset;

    long bclk_policy_override;
    long bclk_always_running;
    long bclk_starts_on_gtw_init;
    long bclk_starts_on_run;
    long bclk_starts_on_pause;
    long bclk_stops_on_pause;
    long bclk_stops_on_reset;

    long sync_policy_override;
    long sync_always_running;
    long sync_starts_on_gtw_init;
    long sync_starts_on_run;
    long sync_starts_on_pause;
    long sync_stops_on_pause;
    long sync_stops_on_reset;

I must admit I am not sure if there is a 1:1 alignment between those definitions and those of the IPC4 protocol, it's a bit unclear.

On the trigger parts, I am not sure we have any issues on MTL, the SSP is still handled completely by the firmware.

However with LNL+, there could indeed be some interesting issues on when the GEN bit and RUN bits are set. The use of HDaudio for the link part adds a lot of fun that we didn't quite plan for. D'oh!

RanderWang commented 1 year ago

@plbossart Current blob maybe has included the "union ipc4_ssp_dma_control i2s_dma_control[];". In driver log, I found the blob includes more after normal SSP setting, very likely that it is for i2s_dma_control. @juimonen are you still available to answer this question ? Thanks!

plbossart commented 10 months ago

moved to P1 to bring this to everyone's attention @kv2019i @lgirdwood @mwasko

kv2019i commented 9 months ago

@marcinszkudlinski @plbossart I have completely missed this Linux driver side ticket. I now filed a FW side ticket and assigned it to v2.9 milestone (for now at least). See my initial comments in https://github.com/thesofproject/sof/issues/8524#issuecomment-1826056398