[PATCH 0/3] Tegra194 HW Fixes

[PATCH 0/3] Tegra194 HW Fixes

[Sameer Pujar]

This series proposes SW workarounds for Tegra194 HDA HW bugs.
Following are the two issues seen:
 1. GCAP register does not reflect true capability.
    The actual number of SDO lines is "4", where as it reflects "2".
 2. With 4 SDO line configuration playback fails for,
    44.1K/48K, 2-channel, 16-bps audio stream.

After fixing [1], issue [2] is uncovered.
As per recommendation by Tegra HW team the workarounds are pushed.

Testing done
============
 * Verify GCAP register after registering HDA sound card
 * Verify audio playback for 44.1K/48K, 2-channel, 16-bps.

Sameer Pujar (3):
  ALSA: hda/tegra: correct number of SDO lines for Tegra194
  ALSA: hda: add member to store ratio for stripe control
  ALSA: hda/tegra: workaround playback failure on Tegra194

 include/sound/hdaudio.h     |  3 +++
 sound/hda/hdac_controller.c | 11 ++++++++++
 sound/hda/hdac_stream.c     |  2 +-
 sound/pci/hda/hda_tegra.c   | 50 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH 1/3] ALSA: hda/tegra: correct number of SDO lines for Tegra194

[Sameer Pujar]

Tegra194 supports 4 SDO lines but GCAP register indicates 2 lines. Thus it
does not reflect the true capability of the HW. This patch presents a
workaround by updating NSDO value accordingly in T_AZA_DBG_CFG_2 register.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 sound/pci/hda/hda_tegra.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index 773992a..45dc544 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -52,10 +52,21 @@
 #define HDA_IPFS_INTR_MASK        0x188
 #define HDA_IPFS_EN_INTR          (1 << 16)
 
+/* FPCI */
+#define FPCI_DBG_CFG_2		  0x10F4
+#define FPCI_GCAP_NSDO_SHIFT	  18
+#define FPCI_GCAP_NSDO_MASK	  (0x3 << FPCI_GCAP_NSDO_SHIFT)
+
 /* max number of SDs */
 #define NUM_CAPTURE_SD 1
 #define NUM_PLAYBACK_SD 1
 
+/*
+ * Tegra194 does not reflect correct number of SDO lines. Below macro
+ * is used to update the GCAP register to workaround the issue.
+ */
+#define TEGRA194_NUM_SDO_LINES	  4
+
 struct hda_tegra {
 	struct azx chip;
 	struct device *dev;
@@ -275,6 +286,7 @@ static int hda_tegra_init_clk(struct hda_tegra *hda)
 
 static int hda_tegra_first_init(struct azx *chip, struct platform_device *pdev)
 {
+	struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
 	struct hdac_bus *bus = azx_bus(chip);
 	struct snd_card *card = chip->card;
 	int err;
@@ -298,6 +310,26 @@ static int hda_tegra_first_init(struct azx *chip, struct platform_device *pdev)
 	bus->irq = irq_id;
 	card->sync_irq = bus->irq;
 
+	/*
+	 * Tegra194 has 4 SDO lines and the STRIPE can be used to
+	 * indicate how many of the SDO lines the stream should be
+	 * striped. But GCAP register does not reflect the true
+	 * capability of HW. Below workaround helps to fix this.
+	 *
+	 * GCAP_NSDO is bits 19:18 in T_AZA_DBG_CFG_2,
+	 * 0 for 1 SDO, 1 for 2 SDO, 2 for 4 SDO lines.
+	 */
+	if (of_device_is_compatible(np, "nvidia,tegra194-hda")) {
+		u32 val;
+
+		dev_info(card->dev, "Override SDO lines to %u\n",
+			 TEGRA194_NUM_SDO_LINES);
+
+		val = readl(hda->regs + FPCI_DBG_CFG_2) & ~FPCI_GCAP_NSDO_MASK;
+		val |= (TEGRA194_NUM_SDO_LINES >> 1) << FPCI_GCAP_NSDO_SHIFT;
+		writel(val, hda->regs + FPCI_DBG_CFG_2);
+	}
+
 	gcap = azx_readw(chip, GCAP);
 	dev_dbg(card->dev, "chipset global capabilities = 0x%x\n", gcap);
 
@@ -408,6 +440,7 @@ static int hda_tegra_create(struct snd_card *card,
 
 static const struct of_device_id hda_tegra_match[] = {
 	{ .compatible = "nvidia,tegra30-hda" },
+	{ .compatible = "nvidia,tegra194-hda" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, hda_tegra_match);
-- 
2.7.4

[PATCH 2/3] ALSA: hda: add member to store ratio for stripe control

[Sameer Pujar]

Stripe control programming is governed by following formula, which is
referenced from the HD Audio specification(Revision 1.0a).
  { ((num_channels * bits_per_sample) / number of SDOs) >= 8 }

Currently above is implemented in snd_hdac_get_stream_stripe_ctl().
This patch introduces a structure member to store the default factor
of '8'. If any HW wants to use a different value, this member can be
easily updated.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 include/sound/hdaudio.h     |  3 +++
 sound/hda/hdac_controller.c | 12 ++++++++++++
 sound/hda/hdac_stream.c     |  2 +-
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index affedc2..d365297 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -364,6 +364,9 @@ struct hdac_bus {
 	/* link management */
 	struct list_head hlink_list;
 	bool cmd_dma_state;
+
+	/* factor used to derive STRIPE control value */
+	unsigned int sdo_limit;
 };
 
 int snd_hdac_bus_init(struct hdac_bus *bus, struct device *dev,
diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
index bc4a8b6..ac15aa3 100644
--- a/sound/hda/hdac_controller.c
+++ b/sound/hda/hdac_controller.c
@@ -527,6 +527,18 @@ bool snd_hdac_bus_init_chip(struct hdac_bus *bus, bool full_reset)
 	}
 
 	bus->chip_init = true;
+
+	/*
+	 * Default value of '8' is as per the HD audio specification (Rev 1.0a).
+	 * Following relation is used to derive STRIPE control value.
+	 *  For sample rate <= 48K:
+	 *   { ((num_channels * bits_per_sample) / number of SDOs) >= 8 }
+	 *  For sample rate > 48K:
+	 *   { ((num_channels * bits_per_sample * rate/48000) /
+	 *	number of SDOs) >= 8 }
+	 */
+	bus->sdo_limit = 8;
+
 	return true;
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_init_chip);
diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index a314b03..a38a2af 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -38,7 +38,7 @@ int snd_hdac_get_stream_stripe_ctl(struct hdac_bus *bus,
 		else
 			value = (channels * bits_per_sample) / sdo_line;
 
-		if (value >= 8)
+		if (value >= bus->sdo_limit)
 			break;
 	}
 
-- 
2.7.4

[PATCH 3/3] ALSA: hda/tegra: workaround playback failure on Tegra194

[Sameer Pujar]

Tegra194 has 4 SDO lines and with this configuration playback fails
for 44.1K/48K, 2-channel and 16-bps. It results in below print,
  "aplay: pcm_write:2011: write error: Input/output error"

Below relation is used to derive stripe control and is referenced
from HD Audio Specification: Revision 1.0a.
  { ((num_channels * bits_per_sample) / number of SDOs) >= 8 }

Due to a legacy HW design problem, playback issue is hit while using
a stripe value resulting from above formula when ratio is '8'. Thus
it is recommended that the ratio must be greater than '8'. Since the
number of SDO lines is in powers of 2, next available ratio '16' is
used as a limiting factor on Tegra194 to workaround the problem.

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 sound/pci/hda/hda_tegra.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
index 45dc544..0cc5fad 100644
--- a/sound/pci/hda/hda_tegra.c
+++ b/sound/pci/hda/hda_tegra.c
@@ -364,6 +364,23 @@ static int hda_tegra_first_init(struct azx *chip, struct platform_device *pdev)
 	/* initialize chip */
 	azx_init_chip(chip, 1);
 
+	/*
+	 * Playback (for 44.1K/48K, 2-channel, 16-bps) fails with
+	 * 4 SDO lines due to legacy design limitation. Following
+	 * is, from HD Audio Specification (Revision 1.0a), used to
+	 * control striping of the stream across multiple SDO lines
+	 * for sample rates <= 48K.
+	 *
+	 * { ((num_channels * bits_per_sample) / number of SDOs) >= 8 }
+	 *
+	 * Due to legacy design issue it is recommended that above
+	 * ratio must be greater than 8. Since number of SDO lines is
+	 * in powers of 2, next available ratio is 16 which can be
+	 * used as a limiting factor here.
+	 */
+	if (of_device_is_compatible(np, "nvidia,tegra194-hda"))
+		chip->bus.core.sdo_limit = 16;
+
 	/* codec detection */
 	if (!bus->codec_mask) {
 		dev_err(card->dev, "no codecs found!\n");
-- 
2.7.4

Re: [PATCH 0/3] Tegra194 HW Fixes

[Takashi Iwai]

On Mon, 04 May 2020 10:16:13 +0200,
Sameer Pujar wrote:
> 
> This series proposes SW workarounds for Tegra194 HDA HW bugs.
> Following are the two issues seen:
>  1. GCAP register does not reflect true capability.
>     The actual number of SDO lines is "4", where as it reflects "2".
>  2. With 4 SDO line configuration playback fails for,
>     44.1K/48K, 2-channel, 16-bps audio stream.
> 
> After fixing [1], issue [2] is uncovered.
> As per recommendation by Tegra HW team the workarounds are pushed.
> 
> Testing done
> ============
>  * Verify GCAP register after registering HDA sound card
>  * Verify audio playback for 44.1K/48K, 2-channel, 16-bps.
> 
> Sameer Pujar (3):
>   ALSA: hda/tegra: correct number of SDO lines for Tegra194
>   ALSA: hda: add member to store ratio for stripe control
>   ALSA: hda/tegra: workaround playback failure on Tegra194

Through a quick glance, all changes look good.

Is the device already in market, i.e. it had hit users?  If yes, I'm
going to merge for 5.7, otherwise for 5.8.


thanks,

Takashi

Re: [PATCH 0/3] Tegra194 HW Fixes

[Sameer Pujar]

On 5/4/2020 1:59 PM, Takashi Iwai wrote:
> External email: Use caution opening links or attachments
>
>
> On Mon, 04 May 2020 10:16:13 +0200,
> Sameer Pujar wrote:
>> This series proposes SW workarounds for Tegra194 HDA HW bugs.
>> Following are the two issues seen:
>>   1. GCAP register does not reflect true capability.
>>      The actual number of SDO lines is "4", where as it reflects "2".
>>   2. With 4 SDO line configuration playback fails for,
>>      44.1K/48K, 2-channel, 16-bps audio stream.
>>
>> After fixing [1], issue [2] is uncovered.
>> As per recommendation by Tegra HW team the workarounds are pushed.
>>
>> Testing done
>> ============
>>   * Verify GCAP register after registering HDA sound card
>>   * Verify audio playback for 44.1K/48K, 2-channel, 16-bps.
>>
>> Sameer Pujar (3):
>>    ALSA: hda/tegra: correct number of SDO lines for Tegra194
>>    ALSA: hda: add member to store ratio for stripe control
>>    ALSA: hda/tegra: workaround playback failure on Tegra194
> Through a quick glance, all changes look good.
>
> Is the device already in market, i.e. it had hit users?  If yes, I'm
> going to merge for 5.7, otherwise for 5.8.

Yes the device is in the market. But has not been reported by end users.
During internal resting this has been discovered. I am fine with merging 
this to 5.8.

Thanks,
Sameer.
>
>
> thanks,
>
> Takashi

Re: [PATCH 0/3] Tegra194 HW Fixes

[Sameer Pujar]

On 5/4/2020 2:05 PM, Sameer Pujar wrote:
>
>
> On 5/4/2020 1:59 PM, Takashi Iwai wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On Mon, 04 May 2020 10:16:13 +0200,
>> Sameer Pujar wrote:
>>> This series proposes SW workarounds for Tegra194 HDA HW bugs.
>>> Following are the two issues seen:
>>>   1. GCAP register does not reflect true capability.
>>>      The actual number of SDO lines is "4", where as it reflects "2".
>>>   2. With 4 SDO line configuration playback fails for,
>>>      44.1K/48K, 2-channel, 16-bps audio stream.
>>>
>>> After fixing [1], issue [2] is uncovered.
>>> As per recommendation by Tegra HW team the workarounds are pushed.
>>>
>>> Testing done
>>> ============
>>>   * Verify GCAP register after registering HDA sound card
>>>   * Verify audio playback for 44.1K/48K, 2-channel, 16-bps.
>>>
>>> Sameer Pujar (3):
>>>    ALSA: hda/tegra: correct number of SDO lines for Tegra194
>>>    ALSA: hda: add member to store ratio for stripe control
>>>    ALSA: hda/tegra: workaround playback failure on Tegra194
>> Through a quick glance, all changes look good.
>>
>> Is the device already in market, i.e. it had hit users?  If yes, I'm
>> going to merge for 5.7, otherwise for 5.8.
>
> Yes the device is in the market. But has not been reported by end users.
> During internal resting this has been discovered. I am fine with 
> merging this to 5.8.

To add, end users currently won't see this problem because things work 
fine with 2-SDO lines. The issue is seen when Tegra194 is allowed to 
utilize its actual capability of 4-SDO lines.
>
> Thanks,
> Sameer.
>>
>>
>> thanks,
>>
>> Takashi
>

Re: [PATCH 0/3] Tegra194 HW Fixes

[Takashi Iwai]

On Mon, 04 May 2020 10:48:46 +0200,
Sameer Pujar wrote:
> 
> 
> 
> On 5/4/2020 2:05 PM, Sameer Pujar wrote:
> >
> >
> > On 5/4/2020 1:59 PM, Takashi Iwai wrote:
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> On Mon, 04 May 2020 10:16:13 +0200,
> >> Sameer Pujar wrote:
> >>> This series proposes SW workarounds for Tegra194 HDA HW bugs.
> >>> Following are the two issues seen:
> >>>   1. GCAP register does not reflect true capability.
> >>>      The actual number of SDO lines is "4", where as it reflects "2".
> >>>   2. With 4 SDO line configuration playback fails for,
> >>>      44.1K/48K, 2-channel, 16-bps audio stream.
> >>>
> >>> After fixing [1], issue [2] is uncovered.
> >>> As per recommendation by Tegra HW team the workarounds are pushed.
> >>>
> >>> Testing done
> >>> ============
> >>>   * Verify GCAP register after registering HDA sound card
> >>>   * Verify audio playback for 44.1K/48K, 2-channel, 16-bps.
> >>>
> >>> Sameer Pujar (3):
> >>>    ALSA: hda/tegra: correct number of SDO lines for Tegra194
> >>>    ALSA: hda: add member to store ratio for stripe control
> >>>    ALSA: hda/tegra: workaround playback failure on Tegra194
> >> Through a quick glance, all changes look good.
> >>
> >> Is the device already in market, i.e. it had hit users?  If yes, I'm
> >> going to merge for 5.7, otherwise for 5.8.
> >
> > Yes the device is in the market. But has not been reported by end users.
> > During internal resting this has been discovered. I am fine with
> > merging this to 5.8.
> 
> To add, end users currently won't see this problem because things work
> fine with 2-SDO lines. The issue is seen when Tegra194 is allowed to
> utilize its actual capability of 4-SDO lines.

Thanks for the information.  Then I queued all three patches now to
for-next branch destined for 5.8.


Takashi