diff --git a/sound/soc/sof/intel/hda-pcm.c b/sound/soc/sof/intel/hda-pcm.c index 974383cd0440740c4b904b7994bf3976f6df1df4..d78aa5d8552d58a24d51b358666d7d3258613875 100644 --- a/sound/soc/sof/intel/hda-pcm.c +++ b/sound/soc/sof/intel/hda-pcm.c @@ -202,38 +202,74 @@ snd_pcm_uframes_t hda_dsp_pcm_pointer(struct snd_sof_dev *sdev, goto found; } - /* - * DPIB/posbuf position mode: - * For Playback, Use DPIB register from HDA space which - * reflects the actual data transferred. - * For Capture, Use the position buffer for pointer, as DPIB - * is not accurate enough, its update may be completed - * earlier than the data written to DDR. - */ - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + switch (sof_hda_position_quirk) { + case SOF_HDA_POSITION_QUIRK_USE_SKYLAKE_LEGACY: + /* + * This legacy code, inherited from the Skylake driver, + * mixes DPIB registers and DPIB DDR updates and + * does not seem to follow any known hardware recommendations. + * It's not clear e.g. why there is a different flow + * for capture and playback, the only information that matters is + * what traffic class is used, and on all SOF-enabled platforms + * only VC0 is supported so the work-around was likely not necessary + * and quite possibly wrong. + */ + + /* DPIB/posbuf position mode: + * For Playback, Use DPIB register from HDA space which + * reflects the actual data transferred. + * For Capture, Use the position buffer for pointer, as DPIB + * is not accurate enough, its update may be completed + * earlier than the data written to DDR. + */ + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + pos = snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR, + AZX_REG_VS_SDXDPIB_XBASE + + (AZX_REG_VS_SDXDPIB_XINTERVAL * + hstream->index)); + } else { + /* + * For capture stream, we need more workaround to fix the + * position incorrect issue: + * + * 1. Wait at least 20us before reading position buffer after + * the interrupt generated(IOC), to make sure position update + * happens on frame boundary i.e. 20.833uSec for 48KHz. + * 2. Perform a dummy Read to DPIB register to flush DMA + * position value. + * 3. Read the DMA Position from posbuf. Now the readback + * value should be >= period boundary. + */ + usleep_range(20, 21); + snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR, + AZX_REG_VS_SDXDPIB_XBASE + + (AZX_REG_VS_SDXDPIB_XINTERVAL * + hstream->index)); + pos = snd_hdac_stream_get_pos_posbuf(hstream); + } + break; + case SOF_HDA_POSITION_QUIRK_USE_DPIB_REGISTERS: + /* + * In case VC1 traffic is disabled this is the recommended option + */ pos = snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR, AZX_REG_VS_SDXDPIB_XBASE + (AZX_REG_VS_SDXDPIB_XINTERVAL * hstream->index)); - } else { + break; + case SOF_HDA_POSITION_QUIRK_USE_DPIB_DDR_UPDATE: /* - * For capture stream, we need more workaround to fix the - * position incorrect issue: - * - * 1. Wait at least 20us before reading position buffer after - * the interrupt generated(IOC), to make sure position update - * happens on frame boundary i.e. 20.833uSec for 48KHz. - * 2. Perform a dummy Read to DPIB register to flush DMA - * position value. - * 3. Read the DMA Position from posbuf. Now the readback - * value should be >= period boundary. + * This is the recommended option when VC1 is enabled. + * While this isn't needed for SOF platforms it's added for + * consistency and debug. */ - usleep_range(20, 21); - snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR, - AZX_REG_VS_SDXDPIB_XBASE + - (AZX_REG_VS_SDXDPIB_XINTERVAL * - hstream->index)); pos = snd_hdac_stream_get_pos_posbuf(hstream); + break; + default: + dev_err_once(sdev->dev, "hda_position_quirk value %d not supported\n", + sof_hda_position_quirk); + pos = 0; + break; } if (pos >= hstream->bufsize) diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index cfe026dbf1242c7277dbce01998a3d203d18f5fd..dabbd5d908f6467db093151b97e599da9c5c9946 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -432,6 +432,10 @@ MODULE_PARM_DESC(use_msi, "SOF HDA use PCI MSI mode"); #define hda_use_msi (1) #endif +int sof_hda_position_quirk = SOF_HDA_POSITION_QUIRK_USE_DPIB_REGISTERS; +module_param_named(position_quirk, sof_hda_position_quirk, int, 0444); +MODULE_PARM_DESC(position_quirk, "SOF HDaudio position quirk"); + static char *hda_model; module_param(hda_model, charp, 0444); MODULE_PARM_DESC(hda_model, "Use the given HDA board model."); @@ -610,7 +614,10 @@ static int hda_init(struct snd_sof_dev *sdev) /* HDA bus init */ sof_hda_bus_init(bus, &pci->dev); - bus->use_posbuf = 1; + if (sof_hda_position_quirk == SOF_HDA_POSITION_QUIRK_USE_DPIB_REGISTERS) + bus->use_posbuf = 0; + else + bus->use_posbuf = 1; bus->bdl_pos_adj = 0; bus->sync_write = 1; diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index e2055b6c81398d0eb15b1a77dab62d06edcd0b4b..cb71d9d5cf6c493c18344c497df44cab633dff13 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -740,4 +740,10 @@ struct sof_ipc_dai_config; int hda_ctrl_dai_widget_setup(struct snd_soc_dapm_widget *w, unsigned int quirk_flags); int hda_ctrl_dai_widget_free(struct snd_soc_dapm_widget *w, unsigned int quirk_flags); +#define SOF_HDA_POSITION_QUIRK_USE_SKYLAKE_LEGACY (0) /* previous implementation */ +#define SOF_HDA_POSITION_QUIRK_USE_DPIB_REGISTERS (1) /* recommended if VC0 only */ +#define SOF_HDA_POSITION_QUIRK_USE_DPIB_DDR_UPDATE (2) /* recommended with VC0 or VC1 */ + +extern int sof_hda_position_quirk; + #endif