From: Cezary Rojewski cezary.rojewski@intel.com

HW recommends to set DUM bit on device power up, so that DPIB write request occurs every frame regardless of whether DPIB has changed or not. This addresses incorrect position reporting for capture streams.

Signed-off-by: Leoni Prodduvaka leoni.prodduvaka@intel.com Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/skylake/skl.c | 21 +++++++++++++++++++++ sound/soc/intel/skylake/skl.h | 1 + 2 files changed, 22 insertions(+)

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index f864f7b3df3a..7174b5da4099 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -192,6 +192,25 @@ void skl_update_d0i3c(struct device *dev, bool enable) snd_hdac_chip_readb(bus, VS_D0I3C)); }

+/** + * skl_dum_set - set DUM bit in EM2 register + * @bus: HD-audio core bus + * + * Addresses incorrect position reporting for capture streams. + * Used on device power up. + */ +static void skl_dum_set(struct hdac_bus *bus) +{ + /* For the DUM bit to be set, CRST needs to be out of reset state */ + if (!(snd_hdac_chip_readb(bus, GCTL) & AZX_GCTL_RESET)) { + skl_enable_miscbdcge(bus->dev, false); + snd_hdac_bus_exit_link_reset(bus); + skl_enable_miscbdcge(bus->dev, true); + } + + snd_hdac_chip_updatel(bus, VS_EM2, AZX_VS_EM2_DUM, AZX_VS_EM2_DUM); +} + /* called from IRQ */ static void skl_stream_update(struct hdac_bus *bus, struct hdac_stream *hstr) { @@ -299,6 +318,7 @@ static int _skl_resume(struct hdac_bus *bus) struct skl *skl = bus_to_skl(bus);

skl_init_pci(skl); + skl_dum_set(bus); skl_init_chip(bus, true);

return skl_resume_dsp(skl); @@ -956,6 +976,7 @@ static int skl_first_init(struct hdac_bus *bus)

/* initialize chip */ skl_init_pci(skl); + skl_dum_set(bus);

return skl_init_chip(bus, true); } diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h index 85f8bb6687dc..b92a7f8fe675 100644 --- a/sound/soc/intel/skylake/skl.h +++ b/sound/soc/intel/skylake/skl.h @@ -46,6 +46,7 @@ #define DMA_TRANSMITION_START 2 #define DMA_TRANSMITION_STOP 3

+#define AZX_VS_EM2_DUM BIT(23) #define AZX_REG_VS_EM2_L1SEN BIT(13)

struct skl_dsp_resource {

From: Cezary Rojewski cezary.rojewski@intel.com

Host clears DONE bit to signal IPC target it has completed the operation. Once this is done, IPC target i.e. DSP may proceed with the next reply, filling registers with new portion of data.

Because of this, host should always read all registers prior to clearing DONE and BUSY bits to ensure no desynchronization happens the time in between clearing bits and reading message data (here, extension).

Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/skylake/cnl-sst.c | 2 +- sound/soc/intel/skylake/skl-sst-ipc.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/skylake/cnl-sst.c b/sound/soc/intel/skylake/cnl-sst.c index 245df1067ba8..759ea59615ca 100644 --- a/sound/soc/intel/skylake/cnl-sst.c +++ b/sound/soc/intel/skylake/cnl-sst.c @@ -313,6 +313,7 @@ static irqreturn_t cnl_dsp_irq_thread_handler(int irq, void *context)

hipcida = sst_dsp_shim_read_unlocked(dsp, CNL_ADSP_REG_HIPCIDA); hipctdr = sst_dsp_shim_read_unlocked(dsp, CNL_ADSP_REG_HIPCTDR); + hipctdd = sst_dsp_shim_read_unlocked(dsp, CNL_ADSP_REG_HIPCTDD);

/* reply message from dsp */ if (hipcida & CNL_ADSP_REG_HIPCIDA_DONE) { @@ -332,7 +333,6 @@ static irqreturn_t cnl_dsp_irq_thread_handler(int irq, void *context)

/* new message from dsp */ if (hipctdr & CNL_ADSP_REG_HIPCTDR_BUSY) { - hipctdd = sst_dsp_shim_read_unlocked(dsp, CNL_ADSP_REG_HIPCTDD); header.primary = hipctdr; header.extension = hipctdd; dev_dbg(dsp->dev, "IPC irq: Firmware respond primary:%x", diff --git a/sound/soc/intel/skylake/skl-sst-ipc.c b/sound/soc/intel/skylake/skl-sst-ipc.c index 9f3ce73593ae..5c9206dc7932 100644 --- a/sound/soc/intel/skylake/skl-sst-ipc.c +++ b/sound/soc/intel/skylake/skl-sst-ipc.c @@ -511,6 +511,7 @@ irqreturn_t skl_dsp_irq_thread_handler(int irq, void *context)

hipcie = sst_dsp_shim_read_unlocked(dsp, SKL_ADSP_REG_HIPCIE); hipct = sst_dsp_shim_read_unlocked(dsp, SKL_ADSP_REG_HIPCT); + hipcte = sst_dsp_shim_read_unlocked(dsp, SKL_ADSP_REG_HIPCTE);

/* reply message from DSP */ if (hipcie & SKL_ADSP_REG_HIPCIE_DONE) { @@ -530,7 +531,6 @@ irqreturn_t skl_dsp_irq_thread_handler(int irq, void *context)

/* New message from DSP */ if (hipct & SKL_ADSP_REG_HIPCT_BUSY) { - hipcte = sst_dsp_shim_read_unlocked(dsp, SKL_ADSP_REG_HIPCTE); header.primary = hipct; header.extension = hipcte; dev_dbg(dsp->dev, "IPC irq: Firmware respond primary:%x\n",

From: Gustaw Lewandowski gustaw.lewandowski@intel.com

Since there are multiple IPCs being sent in a short span of time, there is a possibility of more than one message being on the Rx list after receiving response from firmware. In such cases, when the first notification of interrupt from firmware is received, driver retrieves the message from the Rx list but does not delete it from the list till the next lock. In the meantime, when another interrupt is received from the firmware, driver is reading the previous message again since the previous message has not been removed from the list.

Signed-off-by: Gustaw Lewandowski gustaw.lewandowski@intel.com --- sound/soc/intel/skylake/skl-sst-ipc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/skylake/skl-sst-ipc.c b/sound/soc/intel/skylake/skl-sst-ipc.c index 5c9206dc7932..5094205a243f 100644 --- a/sound/soc/intel/skylake/skl-sst-ipc.c +++ b/sound/soc/intel/skylake/skl-sst-ipc.c @@ -344,6 +344,7 @@ static struct ipc_message *skl_ipc_reply_get_msg(struct sst_generic_ipc *ipc,

msg = list_first_entry(&ipc->rx_list, struct ipc_message, list);

+ list_del(&msg->list); out: return msg;

@@ -488,7 +489,6 @@ void skl_ipc_process_reply(struct sst_generic_ipc *ipc, }

spin_lock_irqsave(&ipc->dsp->spinlock, flags); - list_del(&msg->list); sst_ipc_tx_msg_reply_complete(ipc, msg); spin_unlock_irqrestore(&ipc->dsp->spinlock, flags); }

From: Cezary Rojewski cezary.rojewski@intel.com

rx_data and rx_bytes present for tx_wait_done are optional parameters. If not provided, function should not attempt to copy received data. This change fixes memcpy NULL pointer dereference issue occurring when optional rx_data is NULL while received message size is non-zero.

Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/common/sst-ipc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/common/sst-ipc.c b/sound/soc/intel/common/sst-ipc.c index dcff13802c00..fc3340f1da3a 100644 --- a/sound/soc/intel/common/sst-ipc.c +++ b/sound/soc/intel/common/sst-ipc.c @@ -71,7 +71,7 @@ static int tx_wait_done(struct sst_generic_ipc *ipc, } else {

/* copy the data returned from DSP */ - if (msg->rx_size) + if (rx_data) memcpy(rx_data, msg->rx_data, msg->rx_size); ret = msg->errno; }

From: Amadeusz Sławiński amadeuszx.slawinski@intel.com

Before actual deletion, pipeline should enter RESET state. Currently, pipe skips this checkpoint and goes straight to the finish line. This is not the expected path by the FW, so correct it.

Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@intel.com Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/skylake/skl-messages.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c index df01dc952521..79baf90e6116 100644 --- a/sound/soc/intel/skylake/skl-messages.c +++ b/sound/soc/intel/skylake/skl-messages.c @@ -1265,10 +1265,10 @@ int skl_create_pipeline(struct skl_sst *ctx, struct skl_pipe *pipe) }

/* - * A pipeline needs to be deleted on cleanup. If a pipeline is running, then - * pause the pipeline first and then delete it - * The pipe delete is done by sending delete pipeline IPC. DSP will stop the - * DMA engines and releases resources + * A pipeline needs to be deleted on cleanup. If a pipeline is running, + * then pause it first. Before actual deletion, pipeline should enter + * reset state. Finish the procedure by sending delete pipeline IPC. + * DSP will stop the DMA engines and release resources */ int skl_delete_pipe(struct skl_sst *ctx, struct skl_pipe *pipe) { @@ -1276,6 +1276,10 @@ int skl_delete_pipe(struct skl_sst *ctx, struct skl_pipe *pipe)

dev_dbg(ctx->dev, "%s: pipe = %d\n", __func__, pipe->ppl_id);

+ /* If pipe was not created in FW, do not try to delete it */ + if (pipe->state < SKL_PIPE_CREATED) + return 0; + /* If pipe is started, do stop the pipe in FW. */ if (pipe->state >= SKL_PIPE_STARTED) { ret = skl_set_pipe_state(ctx, pipe, PPL_PAUSED); @@ -1287,9 +1291,14 @@ int skl_delete_pipe(struct skl_sst *ctx, struct skl_pipe *pipe) pipe->state = SKL_PIPE_PAUSED; }

- /* If pipe was not created in FW, do not try to delete it */ - if (pipe->state < SKL_PIPE_CREATED) - return 0; + /* reset pipe state before deletion */ + ret = skl_set_pipe_state(ctx, pipe, PPL_RESET); + if (ret < 0) { + dev_err(ctx->dev, "Failed to reset pipe ret=%d\n", ret); + return ret; + } + + pipe->state = SKL_PIPE_RESET;

ret = skl_ipc_delete_pipeline(&ctx->ipc, pipe->ppl_id); if (ret < 0) {