diff mbox

[3/3] ALSA: hda - Improved position reporting on SKL+

Message ID 20170331084930.813-4-tiwai@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Takashi Iwai March 31, 2017, 8:49 a.m. UTC 
Apply the same methods to obtain the current stream position as ASoC
Intel SKL driver uses.  It reads the position from DPIB for a playback
stream while it still reads from the position buffer for a capture
stream.  For a capture stream, some ugly workaround is needed to
settle down the inconsistent position.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/hda_intel.c | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

Comments

Vinod Koul April 3, 2017, 2:58 a.m. UTC | #1 
On Fri, Mar 31, 2017 at 10:49:30AM +0200, Takashi Iwai wrote:
> Apply the same methods to obtain the current stream position as ASoC
> Intel SKL driver uses.  It reads the position from DPIB for a playback
> stream while it still reads from the position buffer for a capture
> stream.  For a capture stream, some ugly workaround is needed to
> settle down the inconsistent position.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/pci/hda/hda_intel.c | 36 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index a48330f4a1a9..64db6698214c 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -77,6 +77,7 @@ enum {
>  	POS_FIX_POSBUF,
>  	POS_FIX_VIACOMBO,
>  	POS_FIX_COMBO,
> +	POS_FIX_SKL,
>  };
>  
>  /* Defines for ATI HD Audio support in SB450 south bridge */
> @@ -148,7 +149,7 @@ module_param_array(model, charp, NULL, 0444);
>  MODULE_PARM_DESC(model, "Use the given board model.");
>  module_param_array(position_fix, int, NULL, 0444);
>  MODULE_PARM_DESC(position_fix, "DMA pointer read method."
> -		 "(-1 = system default, 0 = auto, 1 = LPIB, 2 = POSBUF, 3 = VIACOMBO, 4 = COMBO).");
> +		 "(-1 = system default, 0 = auto, 1 = LPIB, 2 = POSBUF, 3 = VIACOMBO, 4 = COMBO, 5 = SKL+).");

do we have people use this module param, or is it find what works for
them...
Takashi Iwai April 3, 2017, 6:20 a.m. UTC | #2 
On Mon, 03 Apr 2017 04:58:38 +0200,
Vinod Koul wrote:
> 
> On Fri, Mar 31, 2017 at 10:49:30AM +0200, Takashi Iwai wrote:
> > Apply the same methods to obtain the current stream position as ASoC
> > Intel SKL driver uses.  It reads the position from DPIB for a playback
> > stream while it still reads from the position buffer for a capture
> > stream.  For a capture stream, some ugly workaround is needed to
> > settle down the inconsistent position.
> > 
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  sound/pci/hda/hda_intel.c | 36 ++++++++++++++++++++++++++++++++++--
> >  1 file changed, 34 insertions(+), 2 deletions(-)
> > 
> > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > index a48330f4a1a9..64db6698214c 100644
> > --- a/sound/pci/hda/hda_intel.c
> > +++ b/sound/pci/hda/hda_intel.c
> > @@ -77,6 +77,7 @@ enum {
> >  	POS_FIX_POSBUF,
> >  	POS_FIX_VIACOMBO,
> >  	POS_FIX_COMBO,
> > +	POS_FIX_SKL,
> >  };
> >  
> >  /* Defines for ATI HD Audio support in SB450 south bridge */
> > @@ -148,7 +149,7 @@ module_param_array(model, charp, NULL, 0444);
> >  MODULE_PARM_DESC(model, "Use the given board model.");
> >  module_param_array(position_fix, int, NULL, 0444);
> >  MODULE_PARM_DESC(position_fix, "DMA pointer read method."
> > -		 "(-1 = system default, 0 = auto, 1 = LPIB, 2 = POSBUF, 3 = VIACOMBO, 4 = COMBO).");
> > +		 "(-1 = system default, 0 = auto, 1 = LPIB, 2 = POSBUF, 3 = VIACOMBO, 4 = COMBO, 5 = SKL+).");
> 
> do we have people use this module param, or is it find what works for
> them...

It's for debug purpose, and I know people trying this for various
issues.


Takashi
diff mbox

Patch

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index a48330f4a1a9..64db6698214c 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -77,6 +77,7 @@  enum {
 	POS_FIX_POSBUF,
 	POS_FIX_VIACOMBO,
 	POS_FIX_COMBO,
+	POS_FIX_SKL,
 };
 
 /* Defines for ATI HD Audio support in SB450 south bridge */
@@ -148,7 +149,7 @@  module_param_array(model, charp, NULL, 0444);
 MODULE_PARM_DESC(model, "Use the given board model.");
 module_param_array(position_fix, int, NULL, 0444);
 MODULE_PARM_DESC(position_fix, "DMA pointer read method."
-		 "(-1 = system default, 0 = auto, 1 = LPIB, 2 = POSBUF, 3 = VIACOMBO, 4 = COMBO).");
+		 "(-1 = system default, 0 = auto, 1 = LPIB, 2 = POSBUF, 3 = VIACOMBO, 4 = COMBO, 5 = SKL+).");
 module_param_array(bdl_pos_adj, int, NULL, 0644);
 MODULE_PARM_DESC(bdl_pos_adj, "BDL position adjustment offset.");
 module_param_array(probe_mask, int, NULL, 0444);
@@ -815,6 +816,31 @@  static unsigned int azx_via_get_position(struct azx *chip,
 	return bound_pos + mod_dma_pos;
 }
 
+static unsigned int azx_skl_get_dpib_pos(struct azx *chip,
+					 struct azx_dev *azx_dev)
+{
+	return _snd_hdac_chip_readl(azx_bus(chip),
+				    AZX_REG_VS_SDXDPIB_XBASE +
+				    (AZX_REG_VS_SDXDPIB_XINTERVAL *
+				     azx_dev->core.index));
+}
+
+/* get the current DMA position with correction on SKL+ chips */
+static unsigned int azx_get_pos_skl(struct azx *chip, struct azx_dev *azx_dev)
+{
+	/* DPIB register gives a more accurate position for playback */
+	if (azx_dev->core.substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		return azx_skl_get_dpib_pos(chip, azx_dev);
+
+	/* For capture, we need to read posbuf, but it requires a delay
+	 * for the possible boundary overlap; the read of DPIB fetches the
+	 * actual posbuf
+	 */
+	udelay(20);
+	azx_skl_get_dpib_pos(chip, azx_dev);
+	return azx_get_pos_posbuf(chip, azx_dev);
+}
+
 #ifdef CONFIG_PM
 static DEFINE_MUTEX(card_list_lock);
 static LIST_HEAD(card_list);
@@ -1351,6 +1377,7 @@  static int check_position_fix(struct azx *chip, int fix)
 	case POS_FIX_POSBUF:
 	case POS_FIX_VIACOMBO:
 	case POS_FIX_COMBO:
+	case POS_FIX_SKL:
 		return fix;
 	}
 
@@ -1371,6 +1398,10 @@  static int check_position_fix(struct azx *chip, int fix)
 		dev_dbg(chip->card->dev, "Using LPIB position fix\n");
 		return POS_FIX_LPIB;
 	}
+	if (IS_SKL_PLUS(chip->pci)) {
+		dev_dbg(chip->card->dev, "Using SKL position fix\n");
+		return POS_FIX_SKL;
+	}
 	return POS_FIX_AUTO;
 }
 
@@ -1382,6 +1413,7 @@  static void assign_position_fix(struct azx *chip, int fix)
 		[POS_FIX_POSBUF] = azx_get_pos_posbuf,
 		[POS_FIX_VIACOMBO] = azx_via_get_position,
 		[POS_FIX_COMBO] = azx_get_pos_lpib,
+		[POS_FIX_SKL] = azx_get_pos_skl,
 	};
 
 	chip->get_position[0] = chip->get_position[1] = callbacks[fix];
@@ -1390,7 +1422,7 @@  static void assign_position_fix(struct azx *chip, int fix)
 	if (fix == POS_FIX_COMBO)
 		chip->get_position[1] = NULL;
 
-	if (fix == POS_FIX_POSBUF &&
+	if ((fix == POS_FIX_POSBUF || fix == POS_FIX_SKL) &&
 	    (chip->driver_caps & AZX_DCAPS_COUNT_LPIB_DELAY)) {
 		chip->get_delay[0] = chip->get_delay[1] =
 			azx_get_delay_from_lpib;