[PATCH 1/5] ALSA: hda - Force polling mode on CNL for fixing codec communication

[PATCH 1/5] ALSA: hda - Force polling mode on CNL for fixing codec communication

[Bard liao]

From: Bard Liao <yung-chuan.liao@linux.intel.com>

We observed the same issue as reported by commit a8d7bde23e7130686b7662
("ALSA: hda - Force polling mode on CFL for fixing codec communication")
We don't have a better solution. So apply the same workaround to CNL.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/pci/hda/hda_intel.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 2ec91085fa3e..a93468ffb85c 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -375,6 +375,7 @@ enum {
 
 #define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x5a98)
 #define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa348)
+#define IS_CNL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x9dc8)
 
 static char *driver_short_names[] = {
 	[AZX_DRIVER_ICH] = "HDA Intel",
@@ -1700,8 +1701,8 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci,
 	else
 		chip->bdl_pos_adj = bdl_pos_adj[dev];
 
-	/* Workaround for a communication error on CFL (bko#199007) */
-	if (IS_CFL(pci))
+	/* Workaround for a communication error on CFL (bko#199007) and CNL */
+	if (IS_CFL(pci) || IS_CNL(pci))
 		chip->polling_mode = 1;
 
 	err = azx_bus_init(chip, model[dev], &pci_hda_io_ops);
-- 
2.17.1

[PATCH 2/5] ALSA: hda: assign polling_mode after azx_bus_init

[Bard liao]

From: Bard Liao <yung-chuan.liao@linux.intel.com>

We will move the polling_mode flag from struct azx to struct hdac_bus,
and the flag should be assigned after bus init.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/pci/hda/hda_intel.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index a93468ffb85c..a3c190f2e17a 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1701,10 +1701,6 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci,
 	else
 		chip->bdl_pos_adj = bdl_pos_adj[dev];
 
-	/* Workaround for a communication error on CFL (bko#199007) and CNL */
-	if (IS_CFL(pci) || IS_CNL(pci))
-		chip->polling_mode = 1;
-
 	err = azx_bus_init(chip, model[dev], &pci_hda_io_ops);
 	if (err < 0) {
 		kfree(hda);
@@ -1712,6 +1708,10 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci,
 		return err;
 	}
 
+	/* Workaround for a communication error on CFL (bko#199007) and CNL */
+	if (IS_CFL(pci) || IS_CNL(pci))
+		chip->polling_mode = 1;
+
 	if (chip->driver_type == AZX_DRIVER_NVIDIA) {
 		dev_dbg(chip->card->dev, "Enable delay in RIRB handling\n");
 		chip->bus.needs_damn_long_delay = 1;
-- 
2.17.1

[PATCH 3/5] ALSA: hda: move polling_mode flag to struct hdac_bus

[Bard liao]

From: Bard Liao <yung-chuan.liao@linux.intel.com>

polling mode is a useful function in the get_response function. Move
polling_mode flag from struct azx to struct hdac_bus so people can
implement polling mode in their own get_response function without
adding a polling_mode flag in their local chip structure.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 include/sound/hdaudio.h        |  3 +++
 sound/pci/hda/hda_controller.c | 12 ++++++------
 sound/pci/hda/hda_controller.h |  2 --
 sound/pci/hda/hda_intel.c      |  2 +-
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 45f944d57982..a4ab251d10b2 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -357,6 +357,9 @@ struct hdac_bus {
 	bool align_bdle_4k:1;		/* BDLE align 4K boundary */
 	bool reverse_assign:1;		/* assign devices in reverse order */
 	bool corbrp_self_clear:1;	/* CORBRP clears itself after reset */
+	bool polling_mode:1;
+
+	int poll_count;
 
 	int bdl_pos_adj;		/* BDL position adjustment */
 
diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index 532e081f8b8a..53feaeef1553 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -806,11 +806,11 @@ static int azx_rirb_get_response(struct hdac_bus *bus, unsigned int addr,
 
 	for (loopcounter = 0;; loopcounter++) {
 		spin_lock_irq(&bus->reg_lock);
-		if (chip->polling_mode || do_poll)
+		if (bus->polling_mode || do_poll)
 			snd_hdac_bus_update_rirb(bus);
 		if (!bus->rirb.cmds[addr]) {
 			if (!do_poll)
-				chip->poll_count = 0;
+				bus->poll_count = 0;
 			if (res)
 				*res = bus->rirb.res[addr]; /* the last value */
 			spin_unlock_irq(&bus->reg_lock);
@@ -830,21 +830,21 @@ static int azx_rirb_get_response(struct hdac_bus *bus, unsigned int addr,
 	if (hbus->no_response_fallback)
 		return -EIO;
 
-	if (!chip->polling_mode && chip->poll_count < 2) {
+	if (!bus->polling_mode && bus->poll_count < 2) {
 		dev_dbg(chip->card->dev,
 			"azx_get_response timeout, polling the codec once: last cmd=0x%08x\n",
 			bus->last_cmd[addr]);
 		do_poll = 1;
-		chip->poll_count++;
+		bus->poll_count++;
 		goto again;
 	}
 
 
-	if (!chip->polling_mode) {
+	if (!bus->polling_mode) {
 		dev_warn(chip->card->dev,
 			 "azx_get_response timeout, switching to polling mode: last cmd=0x%08x\n",
 			 bus->last_cmd[addr]);
-		chip->polling_mode = 1;
+		bus->polling_mode = 1;
 		goto again;
 	}
 
diff --git a/sound/pci/hda/hda_controller.h b/sound/pci/hda/hda_controller.h
index 7185ed574b41..8d886791cf0f 100644
--- a/sound/pci/hda/hda_controller.h
+++ b/sound/pci/hda/hda_controller.h
@@ -142,11 +142,9 @@ struct azx {
 
 	/* flags */
 	int bdl_pos_adj;
-	int poll_count;
 	unsigned int running:1;
 	unsigned int fallback_to_single_cmd:1;
 	unsigned int single_cmd:1;
-	unsigned int polling_mode:1;
 	unsigned int msi:1;
 	unsigned int probing:1; /* codec probing phase */
 	unsigned int snoop:1;
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index a3c190f2e17a..910dfd7e998c 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1710,7 +1710,7 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci,
 
 	/* Workaround for a communication error on CFL (bko#199007) and CNL */
 	if (IS_CFL(pci) || IS_CNL(pci))
-		chip->polling_mode = 1;
+		azx_bus(chip)->polling_mode = 1;
 
 	if (chip->driver_type == AZX_DRIVER_NVIDIA) {
 		dev_dbg(chip->card->dev, "Enable delay in RIRB handling\n");
-- 
2.17.1

[PATCH 4/5] ALSA: hda: add polling mode in snd_hdac_bus_get_response

[Bard liao]

From: Bard Liao <yung-chuan.liao@linux.intel.com>

Polling mode is useful if a machine somehow missed an expected IRQ.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/hda/hdac_controller.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
index b2e9454f5816..a16ac31bda83 100644
--- a/sound/hda/hdac_controller.c
+++ b/sound/hda/hdac_controller.c
@@ -239,6 +239,8 @@ int snd_hdac_bus_get_response(struct hdac_bus *bus, unsigned int addr,
 	timeout = jiffies + msecs_to_jiffies(1000);
 
 	for (loopcounter = 0;; loopcounter++) {
+		if (bus->polling_mode)
+			snd_hdac_bus_update_rirb(bus);
 		spin_lock_irq(&bus->reg_lock);
 		if (!bus->rirb.cmds[addr]) {
 			if (res)
-- 
2.17.1

[PATCH 5/5] ASoC: SOF: Force polling mode on CFL and CNL

[Bard liao]

From: Bard Liao <yung-chuan.liao@linux.intel.com>

There is a workaround in legacy HDA codec for too long time respone
with CFL machine. We need the same workaround on SOF driver. The same
issue is also seen on CNL machine.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 5c78f4dde6f5..b9f3c802924b 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -32,6 +32,9 @@
 /* platform specific devices */
 #include "shim.h"
 
+#define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa348)
+#define IS_CNL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x9dc8)
+
 /*
  * Debug
  */
@@ -217,6 +220,11 @@ static int hda_init(struct snd_sof_dev *sdev)
 	ext_ops = snd_soc_hdac_hda_get_ops();
 #endif
 	sof_hda_bus_init(bus, &pci->dev, ext_ops);
+
+	/* Workaround for a communication error on CFL (bko#199007) and CNL */
+	if (IS_CFL(pci) || IS_CNL(pci))
+		bus->polling_mode = 1;
+
 	bus->use_posbuf = 1;
 	bus->bdl_pos_adj = 0;
 
-- 
2.17.1

Re: [PATCH 1/5] ALSA: hda - Force polling mode on CNL for fixing codec communication

[Takashi Iwai]

On Sun, 26 May 2019 18:58:32 +0200,
Bard liao wrote:
> 
> From: Bard Liao <yung-chuan.liao@linux.intel.com>
> 
> We observed the same issue as reported by commit a8d7bde23e7130686b7662
> ("ALSA: hda - Force polling mode on CFL for fixing codec communication")
> We don't have a better solution. So apply the same workaround to CNL.
> 
> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>

I think this should go to 5.2-rc, as it's a bug fix, so likely I'm
going to take it soon later.

The question is about the rest patches.  The patches 2-4 are basically
to move the polling handling into HDA-core, so they could be for 5.3.
And patch 5 is for SOF.  This is still an open question.  If this SOF
fix is needed for 5.2, all patches should go as well.


thanks,

Takashi


> ---
>  sound/pci/hda/hda_intel.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index 2ec91085fa3e..a93468ffb85c 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -375,6 +375,7 @@ enum {
>  
>  #define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x5a98)
>  #define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa348)
> +#define IS_CNL(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x9dc8)
>  
>  static char *driver_short_names[] = {
>  	[AZX_DRIVER_ICH] = "HDA Intel",
> @@ -1700,8 +1701,8 @@ static int azx_create(struct snd_card *card, struct pci_dev *pci,
>  	else
>  		chip->bdl_pos_adj = bdl_pos_adj[dev];
>  
> -	/* Workaround for a communication error on CFL (bko#199007) */
> -	if (IS_CFL(pci))
> +	/* Workaround for a communication error on CFL (bko#199007) and CNL */
> +	if (IS_CFL(pci) || IS_CNL(pci))
>  		chip->polling_mode = 1;
>  
>  	err = azx_bus_init(chip, model[dev], &pci_hda_io_ops);
> -- 
> 2.17.1
> 

Re: [PATCH 1/5] ALSA: hda - Force polling mode on CNL for fixing codec communication

[Liao, Bard]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Monday, May 27, 2019 3:37 PM
> To: Bard liao <yung-chuan.liao@linux.intel.com>
> Cc: broonie@kernel.org; alsa-devel@alsa-project.org;
> liam.r.girdwood@linux.intel.com; pierre-louis.bossart@linux.intel.com; Liao,
> Bard <bard.liao@intel.com>; Yang, Libin <libin.yang@intel.com>
> Subject: Re: [PATCH 1/5] ALSA: hda - Force polling mode on CNL for fixing codec
> communication
> 
> On Sun, 26 May 2019 18:58:32 +0200,
> Bard liao wrote:
> >
> > From: Bard Liao <yung-chuan.liao@linux.intel.com>
> >
> > We observed the same issue as reported by commit
> > a8d7bde23e7130686b7662
> > ("ALSA: hda - Force polling mode on CFL for fixing codec
> > communication") We don't have a better solution. So apply the same
> workaround to CNL.
> >
> > Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> 
> I think this should go to 5.2-rc, as it's a bug fix, so likely I'm going to take it soon
> later.
> 
> The question is about the rest patches.  The patches 2-4 are basically to move
> the polling handling into HDA-core, so they could be for 5.3.
> And patch 5 is for SOF.  This is still an open question.  If this SOF fix is needed for
> 5.2, all patches should go as well.

I am fine if patches 2-5 are for 5.3 :)

> 
> 
> thanks,
> 
> Takashi
> 
> 
> > ---
> >  sound/pci/hda/hda_intel.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > index 2ec91085fa3e..a93468ffb85c 100644
> > --- a/sound/pci/hda/hda_intel.c
> > +++ b/sound/pci/hda/hda_intel.c
> > @@ -375,6 +375,7 @@ enum {
> >
> >  #define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device ==
> > 0x5a98)  #define IS_CFL(pci) ((pci)->vendor == 0x8086 && (pci)->device
> > == 0xa348)
> > +#define IS_CNL(pci) ((pci)->vendor == 0x8086 && (pci)->device ==
> > +0x9dc8)
> >
> >  static char *driver_short_names[] = {
> >  	[AZX_DRIVER_ICH] = "HDA Intel",
> > @@ -1700,8 +1701,8 @@ static int azx_create(struct snd_card *card, struct
> pci_dev *pci,
> >  	else
> >  		chip->bdl_pos_adj = bdl_pos_adj[dev];
> >
> > -	/* Workaround for a communication error on CFL (bko#199007) */
> > -	if (IS_CFL(pci))
> > +	/* Workaround for a communication error on CFL (bko#199007) and
> CNL */
> > +	if (IS_CFL(pci) || IS_CNL(pci))
> >  		chip->polling_mode = 1;
> >
> >  	err = azx_bus_init(chip, model[dev], &pci_hda_io_ops);
> > --
> > 2.17.1
> >

Re: [PATCH 1/5] ALSA: hda - Force polling mode on CNL for fixing codec communication

[Takashi Iwai]

On Tue, 28 May 2019 02:10:02 +0200,
Liao, Bard wrote:
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Monday, May 27, 2019 3:37 PM
> > To: Bard liao <yung-chuan.liao@linux.intel.com>
> > Cc: broonie@kernel.org; alsa-devel@alsa-project.org;
> > liam.r.girdwood@linux.intel.com; pierre-louis.bossart@linux.intel.com; Liao,
> > Bard <bard.liao@intel.com>; Yang, Libin <libin.yang@intel.com>
> > Subject: Re: [PATCH 1/5] ALSA: hda - Force polling mode on CNL for fixing codec
> > communication
> > 
> > On Sun, 26 May 2019 18:58:32 +0200,
> > Bard liao wrote:
> > >
> > > From: Bard Liao <yung-chuan.liao@linux.intel.com>
> > >
> > > We observed the same issue as reported by commit
> > > a8d7bde23e7130686b7662
> > > ("ALSA: hda - Force polling mode on CFL for fixing codec
> > > communication") We don't have a better solution. So apply the same
> > workaround to CNL.
> > >
> > > Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> > 
> > I think this should go to 5.2-rc, as it's a bug fix, so likely I'm going to take it soon
> > later.
> > 
> > The question is about the rest patches.  The patches 2-4 are basically to move
> > the polling handling into HDA-core, so they could be for 5.3.
> > And patch 5 is for SOF.  This is still an open question.  If this SOF fix is needed for
> > 5.2, all patches should go as well.
> 
> I am fine if patches 2-5 are for 5.3 :)

OK, now I applied the patch 1 to for-linus branch.
The rest are put into topic/hda-polling-mode branch, and merged to
for-next branch.

Mark, if you need the SOF change into your tree, pull
topic/hda-polling-mode branch.  It's an immutable branch.


thanks,

Takashi