Merge pull request #28 from RanderWang/ipx_fix

Fix IPC timeout issue on APL&CNL

Author: plbossart

sound/soc/sof/intel/bdw.c

@@ -70,6 +70,8 @@ static const struct snd_sof_debugfs_map bdw_debugfs[] = {
 	{"shim", BDW_DSP_BAR, SHIM_OFFSET, SHIM_SIZE},
 };
 
+static int bdw_cmd_done(struct snd_sof_dev *sdev, int dir);
+
 /*
  * Memory copy.
  */
@@ -361,15 +363,16 @@ static irqreturn_t bdw_irq_thread(int irq, void *context)
 		/* Handle Immediate reply from DSP Core */
 		bdw_mailbox_read(sdev, sdev->host_box.offset, &hdr,
 				 sizeof(hdr));
-		snd_sof_ipc_reply(sdev, hdr);
-
-		/* clear DONE bit - tell DSP we have completed */
-		snd_sof_dsp_update_bits_unlocked(sdev, BDW_DSP_BAR, SHIM_IPCX,
-						 SHIM_IPCX_DONE, 0);
 
-		/* unmask Done interrupt */
-		snd_sof_dsp_update_bits_unlocked(sdev, BDW_DSP_BAR, SHIM_IMRX,
-						 SHIM_IMRX_DONE, 0);
+		/*
+		 * handle immediate reply from DSP core. If the msg is
+		 * found, set done bit in cmd_done which is called at the
+		 * end of message processing function, else set it here
+		 * because the done bit can't be set in cmd_done function
+		 * which is triggered by msg
+		 */
+		if (snd_sof_ipc_reply(sdev, hdr))
+			bdw_cmd_done(sdev, SOF_IPC_DSP_REPLY);
 	}
 
 	ipcd = snd_sof_dsp_read(sdev, BDW_DSP_BAR, SHIM_IPCD);
@@ -526,7 +529,7 @@ static int bdw_is_ready(struct snd_sof_dev *sdev)
 	u32 val;
 
 	val = snd_sof_dsp_read(sdev, BDW_DSP_BAR, SHIM_IPCX);
-	if (val & SHIM_IPCX_BUSY)
+	if ((val & SHIM_IPCX_BUSY) || (val & SHIM_IPCX_DONE))
 		return 0;
 
 	return 1;
@@ -573,16 +576,26 @@ static int bdw_get_reply(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
 	return ret;
 }
 
-static int bdw_cmd_done(struct snd_sof_dev *sdev)
+static int bdw_cmd_done(struct snd_sof_dev *sdev, int dir)
 {
-	/* clear BUSY bit and set DONE bit - accept new messages */
-	snd_sof_dsp_update_bits_unlocked(sdev, BDW_DSP_BAR, SHIM_IPCD,
-					 SHIM_IPCD_BUSY | SHIM_IPCD_DONE,
-					 SHIM_IPCD_DONE);
-
-	/* unmask busy interrupt */
-	snd_sof_dsp_update_bits_unlocked(sdev, BDW_DSP_BAR, SHIM_IMRX,
-					 SHIM_IMRX_BUSY, 0);
+	if (dir == SOF_IPC_HOST_REPLY) {
+		/* clear BUSY bit and set DONE bit - accept new messages */
+		snd_sof_dsp_update_bits_unlocked(sdev, BDW_DSP_BAR, SHIM_IPCD,
+						 SHIM_IPCD_BUSY | SHIM_IPCD_DONE,
+						 SHIM_IPCD_DONE);
+
+		/* unmask busy interrupt */
+		snd_sof_dsp_update_bits_unlocked(sdev, BDW_DSP_BAR, SHIM_IMRX,
+						 SHIM_IMRX_BUSY, 0);
+	} else {
+		/* clear DONE bit - tell DSP we have completed */
+		snd_sof_dsp_update_bits_unlocked(sdev, BDW_DSP_BAR, SHIM_IPCX,
+						 SHIM_IPCX_DONE, 0);
+
+		/* unmask Done interrupt */
+		snd_sof_dsp_update_bits_unlocked(sdev, BDW_DSP_BAR, SHIM_IMRX,
+						 SHIM_IMRX_DONE, 0);
+	}
 
 	return 0;
 }

sound/soc/sof/intel/byt.c

@@ -95,6 +95,8 @@ static const struct snd_sof_debugfs_map cht_debugfs[] = {
 	{"shim", BYT_DSP_BAR, SHIM_OFFSET, SHIM_SIZE},
 };
 
+static int byt_cmd_done(struct snd_sof_dev *sdev, int dir);
+
 /*
  * Register IO
  */
@@ -376,16 +378,15 @@ static irqreturn_t byt_irq_thread(int irq, void *context)
 
 	/* reply message from DSP */
 	if (ipcx & SHIM_BYT_IPCX_DONE) {
-		/* Handle Immediate reply from DSP Core */
-		snd_sof_ipc_reply(sdev, ipcx);
-
-		/* clear DONE bit - tell DSP we have completed */
-		snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IPCX,
-						   SHIM_BYT_IPCX_DONE, 0);
-
-		/* unmask Done interrupt */
-		snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IMRX,
-						   SHIM_IMRX_DONE, 0);
+		/*
+		 * handle immediate reply from DSP core. If the msg is
+		 * found, set done bit in cmd_done which is called at the
+		 * end of message processing function, else set it here
+		 * because the done bit can't be set in cmd_done function
+		 * which is triggered by msg
+		 */
+		if (snd_sof_ipc_reply(sdev, ipcx))
+			byt_cmd_done(sdev, SOF_IPC_DSP_REPLY);
 	}
 
 	/* new message from DSP */
@@ -405,10 +406,11 @@ static irqreturn_t byt_irq_thread(int irq, void *context)
 
 static int byt_is_ready(struct snd_sof_dev *sdev)
 {
-	u64 imrx;
+	u64 imrx, ipcx;
 
 	imrx = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IMRX);
-	if (imrx & SHIM_IMRX_DONE)
+	ipcx = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IPCX);
+	if ((imrx & SHIM_IMRX_DONE) || (ipcx & SHIM_BYT_IPCX_DONE))
 		return 0;
 
 	return 1;
@@ -458,17 +460,27 @@ static int byt_get_reply(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
 	return ret;
 }
 
-static int byt_cmd_done(struct snd_sof_dev *sdev)
+static int byt_cmd_done(struct snd_sof_dev *sdev, int dir)
 {
-	/* clear BUSY bit and set DONE bit - accept new messages */
-	snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IPCD,
-					   SHIM_BYT_IPCD_BUSY |
-					   SHIM_BYT_IPCD_DONE,
-					   SHIM_BYT_IPCD_DONE);
+	if (dir == SOF_IPC_HOST_REPLY) {
+		/* clear BUSY bit and set DONE bit - accept new messages */
+		snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IPCD,
+						   SHIM_BYT_IPCD_BUSY |
+						   SHIM_BYT_IPCD_DONE,
+						   SHIM_BYT_IPCD_DONE);
 
-	/* unmask busy interrupt */
-	snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IMRX,
-					   SHIM_IMRX_BUSY, 0);
+		/* unmask busy interrupt */
+		snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IMRX,
+						   SHIM_IMRX_BUSY, 0);
+	} else {
+		/* clear DONE bit - tell DSP we have completed */
+		snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IPCX,
+						   SHIM_BYT_IPCX_DONE, 0);
+
+		/* unmask Done interrupt */
+		snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IMRX,
+						   SHIM_IMRX_DONE, 0);
+	}
 
 	return 0;
 }

sound/soc/sof/intel/cnl.c

@@ -40,6 +40,8 @@ static const struct snd_sof_debugfs_map cnl_dsp_debugfs[] = {
 	{"dsp", HDA_DSP_BAR,  0, 0x10000},
 };
 
+static int cnl_ipc_cmd_done(struct snd_sof_dev *sdev, int dir);
+
 static irqreturn_t cnl_ipc_irq_thread(int irq, void *context)
 {
 	struct snd_sof_dev *sdev = (struct snd_sof_dev *)context;
@@ -69,20 +71,15 @@ static irqreturn_t cnl_ipc_irq_thread(int irq, void *context)
 					CNL_DSP_REG_HIPCCTL,
 					CNL_DSP_REG_HIPCCTL_DONE, 0);
 
-		/* handle immediate reply from DSP core */
-		snd_sof_ipc_reply(sdev, msg);
-
-		/* clear DONE bit - tell DSP we have completed the operation */
-		snd_sof_dsp_update_bits_forced(sdev, HDA_DSP_BAR,
-					       CNL_DSP_REG_HIPCIDA,
-					       CNL_DSP_REG_HIPCIDA_DONE,
-					       CNL_DSP_REG_HIPCIDA_DONE);
-
-		/* unmask Done interrupt */
-		snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR,
-					CNL_DSP_REG_HIPCCTL,
-					CNL_DSP_REG_HIPCCTL_DONE,
-					CNL_DSP_REG_HIPCCTL_DONE);
+		/*
+		 * handle immediate reply from DSP core. If the msg is
+		 * found, set done bit in cmd_done which is called at the
+		 * end of message processing function, else set it here
+		 * because the done bit can't be set in cmd_done function
+		 * which is triggered by msg
+		 */
+		if (snd_sof_ipc_reply(sdev, msg))
+			cnl_ipc_cmd_done(sdev, SOF_IPC_DSP_REPLY);
 
 		ret = IRQ_HANDLED;
 	}
@@ -108,8 +105,10 @@ static irqreturn_t cnl_ipc_irq_thread(int irq, void *context)
 			snd_sof_ipc_msgs_rx(sdev);
 		}
 
-		/* clear busy interrupt to tell dsp controller this */
-		/* interrupt has been accepted, not trigger it again */
+		/*
+		 * clear busy interrupt to tell dsp controller this
+		 * interrupt has been accepted, not trigger it again
+		 */
 		snd_sof_dsp_update_bits_forced(sdev, HDA_DSP_BAR,
 					       CNL_DSP_REG_HIPCTDR,
 					       CNL_DSP_REG_HIPCTDR_BUSY,
@@ -132,23 +131,45 @@ static irqreturn_t cnl_ipc_irq_thread(int irq, void *context)
 	return ret;
 }
 
-static int cnl_ipc_cmd_done(struct snd_sof_dev *sdev)
+static int cnl_ipc_cmd_done(struct snd_sof_dev *sdev, int dir)
 {
-	/* set done bit to ack dsp the msg has been processed */
-	snd_sof_dsp_update_bits_forced(sdev, HDA_DSP_BAR,
-				       CNL_DSP_REG_HIPCTDA,
-				       CNL_DSP_REG_HIPCTDA_DONE,
-				       CNL_DSP_REG_HIPCTDA_DONE);
+	if (dir == SOF_IPC_HOST_REPLY) {
+		/*
+		 * set done bit to ack dsp the msg has been
+		 * processed and send reply msg to dsp
+		 */
+		snd_sof_dsp_update_bits_forced(sdev, HDA_DSP_BAR,
+					       CNL_DSP_REG_HIPCTDA,
+					       CNL_DSP_REG_HIPCTDA_DONE,
+					       CNL_DSP_REG_HIPCTDA_DONE);
+	} else {
+		/*
+		 * set DONE bit - tell DSP we have received the reply msg
+		 * from DSP, and processed it, don't send more reply to host
+		 */
+		snd_sof_dsp_update_bits_forced(sdev, HDA_DSP_BAR,
+					       CNL_DSP_REG_HIPCIDA,
+					       CNL_DSP_REG_HIPCIDA_DONE,
+					       CNL_DSP_REG_HIPCIDA_DONE);
+
+		/* unmask Done interrupt */
+		snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR,
+					CNL_DSP_REG_HIPCCTL,
+					CNL_DSP_REG_HIPCCTL_DONE,
+					CNL_DSP_REG_HIPCCTL_DONE);
+	}
 
 	return 0;
 }
 
 static int cnl_ipc_is_ready(struct snd_sof_dev *sdev)
 {
-	u64 val;
+	u64 busy, done;
 
-	val = snd_sof_dsp_read(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDR);
-	if (val & CNL_DSP_REG_HIPCIDR_BUSY)
+	busy = snd_sof_dsp_read(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDR);
+	done = snd_sof_dsp_read(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDA);
+	if ((busy & CNL_DSP_REG_HIPCIDR_BUSY) ||
+	    (done & CNL_DSP_REG_HIPCIDA_DONE))
 		return 0;
 
 	return 1;

sound/soc/sof/intel/hda-ipc.c

@@ -34,23 +34,46 @@
 #include "../ops.h"
 #include "hda.h"
 
-int hda_dsp_ipc_cmd_done(struct snd_sof_dev *sdev)
+int hda_dsp_ipc_cmd_done(struct snd_sof_dev *sdev, int dir)
 {
-	/* tell DSP cmd is done - clear busy interrupt */
-	snd_sof_dsp_update_bits_forced(sdev, HDA_DSP_BAR,
-				       HDA_DSP_REG_HIPCT,
-				       HDA_DSP_REG_HIPCT_BUSY,
-				       HDA_DSP_REG_HIPCT_BUSY);
+	if (dir == SOF_IPC_HOST_REPLY) {
+		/*
+		 * tell DSP cmd is done - clear busy
+		 * interrupt and send reply msg to dsp
+		 */
+		snd_sof_dsp_update_bits_forced(sdev, HDA_DSP_BAR,
+					       HDA_DSP_REG_HIPCT,
+					       HDA_DSP_REG_HIPCT_BUSY,
+					       HDA_DSP_REG_HIPCT_BUSY);
+	} else {
+		/*
+		 * set DONE bit - tell DSP we have received the reply msg
+		 * from DSP, and processed it, don't send more reply to host
+		 */
+		snd_sof_dsp_update_bits_forced(sdev, HDA_DSP_BAR,
+					       HDA_DSP_REG_HIPCIE,
+					       HDA_DSP_REG_HIPCIE_DONE,
+					       HDA_DSP_REG_HIPCIE_DONE);
+
+		/* unmask Done interrupt */
+		snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR,
+					HDA_DSP_REG_HIPCCTL,
+					HDA_DSP_REG_HIPCCTL_DONE,
+					HDA_DSP_REG_HIPCCTL_DONE);
+	}
+
 	return 0;
 }
 
 int hda_dsp_ipc_is_ready(struct snd_sof_dev *sdev)
 {
-	u64 val;
+	u64 busy, done;
 
 	/* is DSP ready for next IPC command */
-	val = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_HIPCI);
-	if (val & HDA_DSP_REG_HIPCI_BUSY)
+	busy = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_HIPCI);
+	done = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_REG_HIPCIE);
+	if ((busy & HDA_DSP_REG_HIPCI_BUSY) ||
+	    (done & HDA_DSP_REG_HIPCIE_DONE))
 		return 0;
 
 	return 1;
@@ -108,6 +131,7 @@ irqreturn_t hda_dsp_ipc_irq_thread(int irq, void *context)
 	struct snd_sof_dev *sdev = (struct snd_sof_dev *)context;
 	u32 hipci, hipcie, hipct, hipcte, msg = 0, msg_ext = 0;
 	irqreturn_t ret = IRQ_NONE;
+	int reply = -EINVAL;
 
 	/* here we handle IPC interrupts only */
 	if (!(sdev->irq_status & HDA_DSP_ADSPIS_IPC))
@@ -137,19 +161,17 @@ irqreturn_t hda_dsp_ipc_irq_thread(int irq, void *context)
 
 		/* handle immediate reply from DSP core - ignore ROM messages */
 		if (msg != 0x1004000)
-			snd_sof_ipc_reply(sdev, msg);
-
-		/* clear DONE bit - tell DSP we have completed the operation */
-		snd_sof_dsp_update_bits_forced(sdev, HDA_DSP_BAR,
-					       HDA_DSP_REG_HIPCIE,
-					       HDA_DSP_REG_HIPCIE_DONE,
-					       HDA_DSP_REG_HIPCIE_DONE);
-
-		/* unmask Done interrupt */
-		snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR,
-					HDA_DSP_REG_HIPCCTL,
-					HDA_DSP_REG_HIPCCTL_DONE,
-					HDA_DSP_REG_HIPCCTL_DONE);
+			reply = snd_sof_ipc_reply(sdev, msg);
+
+		/*
+		 * handle immediate reply from DSP core. If the msg is
+		 * found, set done bit in cmd_done which is called at the
+		 * end of message processing function, else set it here
+		 * because the done bit can't be set in cmd_done function
+		 * which is triggered by msg
+		 */
+		if (reply)
+			hda_dsp_ipc_cmd_done(sdev, SOF_IPC_DSP_REPLY);
 
 		ret = IRQ_HANDLED;
 	}

sound/soc/sof/intel/hda.h

@@ -473,7 +473,7 @@ int hda_dsp_ipc_get_reply(struct snd_sof_dev *sdev,
 int hda_dsp_ipc_fw_ready(struct snd_sof_dev *sdev, u32 msg_id);
 irqreturn_t hda_dsp_ipc_irq_handler(int irq, void *context);
 irqreturn_t hda_dsp_ipc_irq_thread(int irq, void *context);
-int hda_dsp_ipc_cmd_done(struct snd_sof_dev *sdev);
+int hda_dsp_ipc_cmd_done(struct snd_sof_dev *sdev, int dir);
 
 /*
  * DSP Code loader.