* [PATCH 0/3] Fix Tegra194 HDA regression @ 2021-12-07 6:32 Sameer Pujar 2021-12-07 6:32 ` [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices Sameer Pujar ` (3 more replies) 0 siblings, 4 replies; 39+ messages in thread From: Sameer Pujar @ 2021-12-07 6:32 UTC (permalink / raw) To: tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, digetx, alsa-devel, devicetree, linux-tegra, linux-kernel, Sameer Pujar HDA probe failure is observed on Tegra194 based platforms and this happens due to a reset failure. The series fixes this problem by avoiding explicit resets on BPMP based devices. Sameer Pujar (3): ALSA: hda/tegra: Skip reset on BPMP devices dt-bindings: sound: tegra: Update HDA resets arm64: tegra: Remove non existent Tegra194 reset .../bindings/sound/nvidia,tegra30-hda.yaml | 13 ++++++++---- arch/arm64/boot/dts/nvidia/tegra194.dtsi | 5 ++--- sound/pci/hda/hda_tegra.c | 24 +++++++++++++++++----- 3 files changed, 30 insertions(+), 12 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 6:32 [PATCH 0/3] Fix Tegra194 HDA regression Sameer Pujar @ 2021-12-07 6:32 ` Sameer Pujar 2021-12-07 8:16 ` Thierry Reding 2021-12-07 10:22 ` Dmitry Osipenko 2021-12-07 6:32 ` [PATCH 2/3] dt-bindings: sound: tegra: Update HDA resets Sameer Pujar ` (2 subsequent siblings) 3 siblings, 2 replies; 39+ messages in thread From: Sameer Pujar @ 2021-12-07 6:32 UTC (permalink / raw) To: tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, digetx, alsa-devel, devicetree, linux-tegra, linux-kernel, Sameer Pujar, stable HDA regression is recently reported on Tegra194 based platforms. This happens because "hda2codec_2x" reset does not really exist in Tegra194 and it causes probe failure. All the HDA based audio tests fail at the moment. This underlying issue is exposed by commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP response") which now checks return code of BPMP command response. The failure can be fixed by avoiding above reset in the driver, but the explicit reset is not necessary for Tegra devices which depend on BPMP. On such devices, BPMP ensures reset application during unpowergate calls. Hence skip reset on these devices which is applicable for Tegra186 and later. Signed-off-by: Sameer Pujar <spujar@nvidia.com> Cc: stable@vger.kernel.org Depends-on: 87f0e46e7559 ("ALSA: hda/tegra: Reset hardware") --- sound/pci/hda/hda_tegra.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c index ea700395..862141e 100644 --- a/sound/pci/hda/hda_tegra.c +++ b/sound/pci/hda/hda_tegra.c @@ -68,6 +68,10 @@ */ #define TEGRA194_NUM_SDO_LINES 4 +struct hda_data { + unsigned int do_reset:1; +}; + struct hda_tegra { struct azx chip; struct device *dev; @@ -76,6 +80,7 @@ struct hda_tegra { unsigned int nclocks; void __iomem *regs; struct work_struct probe_work; + const struct hda_data *data; }; #ifdef CONFIG_PM @@ -427,8 +432,13 @@ static int hda_tegra_create(struct snd_card *card, return 0; } +static const struct hda_data tegra30_data = { + .do_reset = 1, +}; + static const struct of_device_id hda_tegra_match[] = { - { .compatible = "nvidia,tegra30-hda" }, + { .compatible = "nvidia,tegra30-hda", .data = &tegra30_data }, + { .compatible = "nvidia,tegra186-hda" }, { .compatible = "nvidia,tegra194-hda" }, {}, }; @@ -449,6 +459,8 @@ static int hda_tegra_probe(struct platform_device *pdev) hda->dev = &pdev->dev; chip = &hda->chip; + hda->data = of_device_get_match_data(&pdev->dev); + err = snd_card_new(&pdev->dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1, THIS_MODULE, 0, &card); if (err < 0) { @@ -456,10 +468,12 @@ static int hda_tegra_probe(struct platform_device *pdev) return err; } - hda->reset = devm_reset_control_array_get_exclusive(&pdev->dev); - if (IS_ERR(hda->reset)) { - err = PTR_ERR(hda->reset); - goto out_free; + if (hda->data && hda->data->do_reset) { + hda->reset = devm_reset_control_array_get_exclusive(&pdev->dev); + if (IS_ERR(hda->reset)) { + err = PTR_ERR(hda->reset); + goto out_free; + } } hda->clocks[hda->nclocks++].id = "hda"; -- 2.7.4 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 6:32 ` [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices Sameer Pujar @ 2021-12-07 8:16 ` Thierry Reding 2021-12-07 8:36 ` Takashi Iwai 2021-12-07 10:22 ` Dmitry Osipenko 1 sibling, 1 reply; 39+ messages in thread From: Thierry Reding @ 2021-12-07 8:16 UTC (permalink / raw) To: Sameer Pujar Cc: tiwai, broonie, lgirdwood, robh+dt, perex, jonathanh, digetx, alsa-devel, devicetree, linux-tegra, linux-kernel, stable [-- Attachment #1: Type: text/plain, Size: 3182 bytes --] On Tue, Dec 07, 2021 at 12:02:48PM +0530, Sameer Pujar wrote: > HDA regression is recently reported on Tegra194 based platforms. > This happens because "hda2codec_2x" reset does not really exist > in Tegra194 and it causes probe failure. All the HDA based audio > tests fail at the moment. This underlying issue is exposed by > commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP > response") which now checks return code of BPMP command response. > > The failure can be fixed by avoiding above reset in the driver, > but the explicit reset is not necessary for Tegra devices which > depend on BPMP. On such devices, BPMP ensures reset application > during unpowergate calls. Hence skip reset on these devices > which is applicable for Tegra186 and later. > > Signed-off-by: Sameer Pujar <spujar@nvidia.com> > Cc: stable@vger.kernel.org > Depends-on: 87f0e46e7559 ("ALSA: hda/tegra: Reset hardware") > --- > sound/pci/hda/hda_tegra.c | 24 +++++++++++++++++++----- > 1 file changed, 19 insertions(+), 5 deletions(-) > > diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c > index ea700395..862141e 100644 > --- a/sound/pci/hda/hda_tegra.c > +++ b/sound/pci/hda/hda_tegra.c > @@ -68,6 +68,10 @@ > */ > #define TEGRA194_NUM_SDO_LINES 4 > > +struct hda_data { > + unsigned int do_reset:1; > +}; I suppose this could also be a bool. Not sure if we need to care about packing optimizations at this point. It may also be useful to rename this to something less generic to avoid potential clashes with other data structures in the future. We've often used the _soc suffix in other drivers to mark this kind of SoC-specific data. In this case it would be struct hda_tegra_soc. If Takashi is fine with this as-is, I don't have any strong objections, though. > + > struct hda_tegra { > struct azx chip; > struct device *dev; > @@ -76,6 +80,7 @@ struct hda_tegra { > unsigned int nclocks; > void __iomem *regs; > struct work_struct probe_work; > + const struct hda_data *data; > }; > > #ifdef CONFIG_PM > @@ -427,8 +432,13 @@ static int hda_tegra_create(struct snd_card *card, > return 0; > } > > +static const struct hda_data tegra30_data = { > + .do_reset = 1, > +}; > + > static const struct of_device_id hda_tegra_match[] = { > - { .compatible = "nvidia,tegra30-hda" }, > + { .compatible = "nvidia,tegra30-hda", .data = &tegra30_data }, > + { .compatible = "nvidia,tegra186-hda" }, > { .compatible = "nvidia,tegra194-hda" }, > {}, > }; One other thing we've done in the past is to explicitly pass these structures for each compatible string. That simplifies things a bit because we don't have to keep checking for non-NULL pointers and instead rely on the fact that there's always a valid pointer. To do so, you'd basically add: static const struct hda_data tegra186_data = { .do_reset = 0, }; And reference that for both the Tegra186 and Tegra194 entries. Again, not strictly necessary and since we have only one occurrence where we need to check this, it seems fine as-is, so: Acked-by: Thierry Reding <treding@nvidia.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 8:16 ` Thierry Reding @ 2021-12-07 8:36 ` Takashi Iwai 2021-12-07 9:09 ` Sameer Pujar 0 siblings, 1 reply; 39+ messages in thread From: Takashi Iwai @ 2021-12-07 8:36 UTC (permalink / raw) To: Thierry Reding Cc: Sameer Pujar, tiwai, broonie, lgirdwood, robh+dt, perex, jonathanh, digetx, alsa-devel, devicetree, linux-tegra, linux-kernel, stable On Tue, 07 Dec 2021 09:16:43 +0100, Thierry Reding wrote: > > On Tue, Dec 07, 2021 at 12:02:48PM +0530, Sameer Pujar wrote: > > HDA regression is recently reported on Tegra194 based platforms. > > This happens because "hda2codec_2x" reset does not really exist > > in Tegra194 and it causes probe failure. All the HDA based audio > > tests fail at the moment. This underlying issue is exposed by > > commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP > > response") which now checks return code of BPMP command response. > > > > The failure can be fixed by avoiding above reset in the driver, > > but the explicit reset is not necessary for Tegra devices which > > depend on BPMP. On such devices, BPMP ensures reset application > > during unpowergate calls. Hence skip reset on these devices > > which is applicable for Tegra186 and later. > > > > Signed-off-by: Sameer Pujar <spujar@nvidia.com> > > Cc: stable@vger.kernel.org > > Depends-on: 87f0e46e7559 ("ALSA: hda/tegra: Reset hardware") > > --- > > sound/pci/hda/hda_tegra.c | 24 +++++++++++++++++++----- > > 1 file changed, 19 insertions(+), 5 deletions(-) > > > > diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c > > index ea700395..862141e 100644 > > --- a/sound/pci/hda/hda_tegra.c > > +++ b/sound/pci/hda/hda_tegra.c > > @@ -68,6 +68,10 @@ > > */ > > #define TEGRA194_NUM_SDO_LINES 4 > > > > +struct hda_data { > > + unsigned int do_reset:1; > > +}; > > I suppose this could also be a bool. Not sure if we need to care about > packing optimizations at this point. > > It may also be useful to rename this to something less generic to avoid > potential clashes with other data structures in the future. We've often > used the _soc suffix in other drivers to mark this kind of SoC-specific > data. In this case it would be struct hda_tegra_soc. > > If Takashi is fine with this as-is, I don't have any strong objections, > though. Indeed, a bit more prefix would be better for avoiding the possible conflict in future, but the struct name is local, so I don't mind to use the simple name for now. We can change it later once when needed, too. > > + > > struct hda_tegra { > > struct azx chip; > > struct device *dev; > > @@ -76,6 +80,7 @@ struct hda_tegra { > > unsigned int nclocks; > > void __iomem *regs; > > struct work_struct probe_work; > > + const struct hda_data *data; > > }; > > > > #ifdef CONFIG_PM > > @@ -427,8 +432,13 @@ static int hda_tegra_create(struct snd_card *card, > > return 0; > > } > > > > +static const struct hda_data tegra30_data = { > > + .do_reset = 1, > > +}; > > + > > static const struct of_device_id hda_tegra_match[] = { > > - { .compatible = "nvidia,tegra30-hda" }, > > + { .compatible = "nvidia,tegra30-hda", .data = &tegra30_data }, > > + { .compatible = "nvidia,tegra186-hda" }, > > { .compatible = "nvidia,tegra194-hda" }, > > {}, > > }; > > One other thing we've done in the past is to explicitly pass these > structures for each compatible string. That simplifies things a bit > because we don't have to keep checking for non-NULL pointers and instead > rely on the fact that there's always a valid pointer. > > To do so, you'd basically add: > > static const struct hda_data tegra186_data = { > .do_reset = 0, > }; > > And reference that for both the Tegra186 and Tegra194 entries. Again, > not strictly necessary and since we have only one occurrence where we > need to check this, it seems fine as-is, so: > > Acked-by: Thierry Reding <treding@nvidia.com> That's true, too. OTOH, completely without a NULL check would be also unsafe, so some sanity check would be still required. That said, the current patch is good enough for taking as a regression fix, but I'm fine to wait for a while for v2 to address those, too :) thanks, Takashi ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 8:36 ` Takashi Iwai @ 2021-12-07 9:09 ` Sameer Pujar 0 siblings, 0 replies; 39+ messages in thread From: Sameer Pujar @ 2021-12-07 9:09 UTC (permalink / raw) To: Takashi Iwai, Thierry Reding Cc: tiwai, broonie, lgirdwood, robh+dt, perex, jonathanh, digetx, alsa-devel, devicetree, linux-tegra, linux-kernel, stable On 12/7/2021 2:06 PM, Takashi Iwai wrote: > On Tue, 07 Dec 2021 09:16:43 +0100, > Thierry Reding wrote: >> I suppose this could also be a bool. Not sure if we need to care about >> packing optimizations at this point. >> >> It may also be useful to rename this to something less generic to avoid >> potential clashes with other data structures in the future. We've often >> used the _soc suffix in other drivers to mark this kind of SoC-specific >> data. In this case it would be struct hda_tegra_soc. >> >> If Takashi is fine with this as-is, I don't have any strong objections, >> though. > Indeed, a bit more prefix would be better for avoiding the possible > conflict in future, but the struct name is local, so I don't mind to > use the simple name for now. We can change it later once when needed, > too. [...] >> >> One other thing we've done in the past is to explicitly pass these >> structures for each compatible string. That simplifies things a bit >> because we don't have to keep checking for non-NULL pointers and instead >> rely on the fact that there's always a valid pointer. >> >> To do so, you'd basically add: >> >> static const struct hda_data tegra186_data = { >> .do_reset = 0, >> }; >> >> And reference that for both the Tegra186 and Tegra194 entries. Again, >> not strictly necessary and since we have only one occurrence where we >> need to check this, it seems fine as-is, so: >> >> Acked-by: Thierry Reding <treding@nvidia.com> > That's true, too. OTOH, completely without a NULL check would be also > unsafe, so some sanity check would be still required. > > That said, the current patch is good enough for taking as a regression > fix, but I'm fine to wait for a while for v2 to address those, too :) > Let me send a v2 for above. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 6:32 ` [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices Sameer Pujar 2021-12-07 8:16 ` Thierry Reding @ 2021-12-07 10:22 ` Dmitry Osipenko 2021-12-07 10:44 ` Dmitry Osipenko 2021-12-07 12:00 ` Sameer Pujar 1 sibling, 2 replies; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-07 10:22 UTC (permalink / raw) To: Sameer Pujar, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable 07.12.2021 09:32, Sameer Pujar пишет: > HDA regression is recently reported on Tegra194 based platforms. > This happens because "hda2codec_2x" reset does not really exist > in Tegra194 and it causes probe failure. All the HDA based audio > tests fail at the moment. This underlying issue is exposed by > commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP > response") which now checks return code of BPMP command response. > > The failure can be fixed by avoiding above reset in the driver, > but the explicit reset is not necessary for Tegra devices which > depend on BPMP. On such devices, BPMP ensures reset application > during unpowergate calls. Hence skip reset on these devices > which is applicable for Tegra186 and later. The power domain is shared with the display, AFAICS. The point of reset is to bring h/w into predictable state. It doesn't make sense to me to skip the reset. If T194+ doesn't have hda2codec_2x reset, then don't request that reset for T194+. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 10:22 ` Dmitry Osipenko @ 2021-12-07 10:44 ` Dmitry Osipenko 2021-12-07 10:58 ` Dmitry Osipenko 2021-12-07 12:00 ` Sameer Pujar 1 sibling, 1 reply; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-07 10:44 UTC (permalink / raw) To: Sameer Pujar, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable 07.12.2021 13:22, Dmitry Osipenko пишет: > 07.12.2021 09:32, Sameer Pujar пишет: >> HDA regression is recently reported on Tegra194 based platforms. >> This happens because "hda2codec_2x" reset does not really exist >> in Tegra194 and it causes probe failure. All the HDA based audio >> tests fail at the moment. This underlying issue is exposed by >> commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP >> response") which now checks return code of BPMP command response. >> >> The failure can be fixed by avoiding above reset in the driver, >> but the explicit reset is not necessary for Tegra devices which >> depend on BPMP. On such devices, BPMP ensures reset application >> during unpowergate calls. Hence skip reset on these devices >> which is applicable for Tegra186 and later. > > The power domain is shared with the display, AFAICS. The point of reset > is to bring h/w into predictable state. It doesn't make sense to me to > skip the reset. > > If T194+ doesn't have hda2codec_2x reset, then don't request that reset > for T194+. > I don't see the problem in the driver. It's only the device-tree that is wrong. This hda_tegra.c patch should be unneeded, please fix only the device-tree. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 10:44 ` Dmitry Osipenko @ 2021-12-07 10:58 ` Dmitry Osipenko 2021-12-07 11:02 ` Jon Hunter 0 siblings, 1 reply; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-07 10:58 UTC (permalink / raw) To: Sameer Pujar, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable 07.12.2021 13:44, Dmitry Osipenko пишет: > 07.12.2021 13:22, Dmitry Osipenko пишет: >> 07.12.2021 09:32, Sameer Pujar пишет: >>> HDA regression is recently reported on Tegra194 based platforms. >>> This happens because "hda2codec_2x" reset does not really exist >>> in Tegra194 and it causes probe failure. All the HDA based audio >>> tests fail at the moment. This underlying issue is exposed by >>> commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP >>> response") which now checks return code of BPMP command response. I see that this BPMP commit already has been reverted. There is no problem in this hda_tegra driver at all. >>> The failure can be fixed by avoiding above reset in the driver, >>> but the explicit reset is not necessary for Tegra devices which >>> depend on BPMP. On such devices, BPMP ensures reset application >>> during unpowergate calls. Hence skip reset on these devices >>> which is applicable for Tegra186 and later. >> >> The power domain is shared with the display, AFAICS. The point of reset >> is to bring h/w into predictable state. It doesn't make sense to me to >> skip the reset. >> >> If T194+ doesn't have hda2codec_2x reset, then don't request that reset >> for T194+. >> > > I don't see the problem in the driver. It's only the device-tree that is > wrong. This hda_tegra.c patch should be unneeded, please fix only the > device-tree. > ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 10:58 ` Dmitry Osipenko @ 2021-12-07 11:02 ` Jon Hunter 2021-12-07 11:57 ` Dmitry Osipenko 0 siblings, 1 reply; 39+ messages in thread From: Jon Hunter @ 2021-12-07 11:02 UTC (permalink / raw) To: Dmitry Osipenko, Sameer Pujar, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: alsa-devel, devicetree, linux-tegra, linux-kernel, stable On 07/12/2021 10:58, Dmitry Osipenko wrote: > 07.12.2021 13:44, Dmitry Osipenko пишет: >> 07.12.2021 13:22, Dmitry Osipenko пишет: >>> 07.12.2021 09:32, Sameer Pujar пишет: >>>> HDA regression is recently reported on Tegra194 based platforms. >>>> This happens because "hda2codec_2x" reset does not really exist >>>> in Tegra194 and it causes probe failure. All the HDA based audio >>>> tests fail at the moment. This underlying issue is exposed by >>>> commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP >>>> response") which now checks return code of BPMP command response. > > I see that this BPMP commit already has been reverted. There is no > problem in this hda_tegra driver at all. That is temporary until this fix is merged and then we will revert the revert. Jon -- nvpublic ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 11:02 ` Jon Hunter @ 2021-12-07 11:57 ` Dmitry Osipenko 2021-12-07 15:07 ` Jon Hunter 0 siblings, 1 reply; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-07 11:57 UTC (permalink / raw) To: Jon Hunter, Sameer Pujar, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: alsa-devel, devicetree, linux-tegra, linux-kernel, stable 07.12.2021 14:02, Jon Hunter пишет: > > On 07/12/2021 10:58, Dmitry Osipenko wrote: >> 07.12.2021 13:44, Dmitry Osipenko пишет: >>> 07.12.2021 13:22, Dmitry Osipenko пишет: >>>> 07.12.2021 09:32, Sameer Pujar пишет: >>>>> HDA regression is recently reported on Tegra194 based platforms. >>>>> This happens because "hda2codec_2x" reset does not really exist >>>>> in Tegra194 and it causes probe failure. All the HDA based audio >>>>> tests fail at the moment. This underlying issue is exposed by >>>>> commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP >>>>> response") which now checks return code of BPMP command response. >> >> I see that this BPMP commit already has been reverted. There is no >> problem in this hda_tegra driver at all. > > That is temporary until this fix is merged and then we will revert the > revert. It's the device-tree that is broken, not the driver. If you don't care about broken HDMI audio using outdated dtb, then there is nothing to fix in the code. Otherwise, the fix is to skip the non-existent reset. You could add workaround to the BPMP reset driver by making it always return success for TEGRA194_RESET_HDA2CODEC_2X invocation instead of making the FW call for that reset. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 11:57 ` Dmitry Osipenko @ 2021-12-07 15:07 ` Jon Hunter 0 siblings, 0 replies; 39+ messages in thread From: Jon Hunter @ 2021-12-07 15:07 UTC (permalink / raw) To: Dmitry Osipenko, Sameer Pujar, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: alsa-devel, devicetree, linux-tegra, linux-kernel, stable On 07/12/2021 11:57, Dmitry Osipenko wrote: > 07.12.2021 14:02, Jon Hunter пишет: >> >> On 07/12/2021 10:58, Dmitry Osipenko wrote: >>> 07.12.2021 13:44, Dmitry Osipenko пишет: >>>> 07.12.2021 13:22, Dmitry Osipenko пишет: >>>>> 07.12.2021 09:32, Sameer Pujar пишет: >>>>>> HDA regression is recently reported on Tegra194 based platforms. >>>>>> This happens because "hda2codec_2x" reset does not really exist >>>>>> in Tegra194 and it causes probe failure. All the HDA based audio >>>>>> tests fail at the moment. This underlying issue is exposed by >>>>>> commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP >>>>>> response") which now checks return code of BPMP command response. >>> >>> I see that this BPMP commit already has been reverted. There is no >>> problem in this hda_tegra driver at all. >> >> That is temporary until this fix is merged and then we will revert the >> revert. > > It's the device-tree that is broken, not the driver. If you don't care > about broken HDMI audio using outdated dtb, then there is nothing to fix > in the code. That's correct. However, we do care about HDMI audio being broken with existing DTBs and so we need to make sure they still work. Jon -- nvpublic ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 10:22 ` Dmitry Osipenko 2021-12-07 10:44 ` Dmitry Osipenko @ 2021-12-07 12:00 ` Sameer Pujar 2021-12-07 12:05 ` Dmitry Osipenko 1 sibling, 1 reply; 39+ messages in thread From: Sameer Pujar @ 2021-12-07 12:00 UTC (permalink / raw) To: Dmitry Osipenko, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar On 12/7/2021 3:52 PM, Dmitry Osipenko wrote: > 07.12.2021 09:32, Sameer Pujar пишет: >> HDA regression is recently reported on Tegra194 based platforms. >> This happens because "hda2codec_2x" reset does not really exist >> in Tegra194 and it causes probe failure. All the HDA based audio >> tests fail at the moment. This underlying issue is exposed by >> commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP >> response") which now checks return code of BPMP command response. >> >> The failure can be fixed by avoiding above reset in the driver, >> but the explicit reset is not necessary for Tegra devices which >> depend on BPMP. On such devices, BPMP ensures reset application >> during unpowergate calls. Hence skip reset on these devices >> which is applicable for Tegra186 and later. > The power domain is shared with the display, AFAICS. The point of reset > is to bring h/w into predictable state. It doesn't make sense to me to > skip the reset. Yes the power-domain is shared with display. As mentioned above, explicit reset in driver is not really necessary since BPMP is already doing it during unpowergate stage. So the h/w is already ensured to be in a good state. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 12:00 ` Sameer Pujar @ 2021-12-07 12:05 ` Dmitry Osipenko 2021-12-07 12:40 ` Sameer Pujar 0 siblings, 1 reply; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-07 12:05 UTC (permalink / raw) To: Sameer Pujar, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar 07.12.2021 15:00, Sameer Pujar пишет: > > > On 12/7/2021 3:52 PM, Dmitry Osipenko wrote: >> 07.12.2021 09:32, Sameer Pujar пишет: >>> HDA regression is recently reported on Tegra194 based platforms. >>> This happens because "hda2codec_2x" reset does not really exist >>> in Tegra194 and it causes probe failure. All the HDA based audio >>> tests fail at the moment. This underlying issue is exposed by >>> commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP >>> response") which now checks return code of BPMP command response. >>> >>> The failure can be fixed by avoiding above reset in the driver, >>> but the explicit reset is not necessary for Tegra devices which >>> depend on BPMP. On such devices, BPMP ensures reset application >>> during unpowergate calls. Hence skip reset on these devices >>> which is applicable for Tegra186 and later. >> The power domain is shared with the display, AFAICS. The point of reset >> is to bring h/w into predictable state. It doesn't make sense to me to >> skip the reset. > > Yes the power-domain is shared with display. As mentioned above, > explicit reset in driver is not really necessary since BPMP is already > doing it during unpowergate stage. So the h/w is already ensured to be > in a good state. If you'll reload the driver module, then h/w won't be reset. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 12:05 ` Dmitry Osipenko @ 2021-12-07 12:40 ` Sameer Pujar 2021-12-07 14:07 ` Dmitry Osipenko 0 siblings, 1 reply; 39+ messages in thread From: Sameer Pujar @ 2021-12-07 12:40 UTC (permalink / raw) To: Dmitry Osipenko, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar On 12/7/2021 5:35 PM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > 07.12.2021 15:00, Sameer Pujar пишет: >> >> On 12/7/2021 3:52 PM, Dmitry Osipenko wrote: >>> 07.12.2021 09:32, Sameer Pujar пишет: >>>> HDA regression is recently reported on Tegra194 based platforms. >>>> This happens because "hda2codec_2x" reset does not really exist >>>> in Tegra194 and it causes probe failure. All the HDA based audio >>>> tests fail at the moment. This underlying issue is exposed by >>>> commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP >>>> response") which now checks return code of BPMP command response. >>>> >>>> The failure can be fixed by avoiding above reset in the driver, >>>> but the explicit reset is not necessary for Tegra devices which >>>> depend on BPMP. On such devices, BPMP ensures reset application >>>> during unpowergate calls. Hence skip reset on these devices >>>> which is applicable for Tegra186 and later. >>> The power domain is shared with the display, AFAICS. The point of reset >>> is to bring h/w into predictable state. It doesn't make sense to me to >>> skip the reset. >> Yes the power-domain is shared with display. As mentioned above, >> explicit reset in driver is not really necessary since BPMP is already >> doing it during unpowergate stage. So the h/w is already ensured to be >> in a good state. > If you'll reload the driver module, then h/w won't be reset. How the reload case would be different? Can you please specify more details if you are referring to a particular scenario? ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 12:40 ` Sameer Pujar @ 2021-12-07 14:07 ` Dmitry Osipenko 2021-12-07 14:49 ` Sameer Pujar 0 siblings, 1 reply; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-07 14:07 UTC (permalink / raw) To: Sameer Pujar, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar 07.12.2021 15:40, Sameer Pujar пишет: > > > On 12/7/2021 5:35 PM, Dmitry Osipenko wrote: >> External email: Use caution opening links or attachments >> >> >> 07.12.2021 15:00, Sameer Pujar пишет: >>> >>> On 12/7/2021 3:52 PM, Dmitry Osipenko wrote: >>>> 07.12.2021 09:32, Sameer Pujar пишет: >>>>> HDA regression is recently reported on Tegra194 based platforms. >>>>> This happens because "hda2codec_2x" reset does not really exist >>>>> in Tegra194 and it causes probe failure. All the HDA based audio >>>>> tests fail at the moment. This underlying issue is exposed by >>>>> commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP >>>>> response") which now checks return code of BPMP command response. >>>>> >>>>> The failure can be fixed by avoiding above reset in the driver, >>>>> but the explicit reset is not necessary for Tegra devices which >>>>> depend on BPMP. On such devices, BPMP ensures reset application >>>>> during unpowergate calls. Hence skip reset on these devices >>>>> which is applicable for Tegra186 and later. >>>> The power domain is shared with the display, AFAICS. The point of reset >>>> is to bring h/w into predictable state. It doesn't make sense to me to >>>> skip the reset. >>> Yes the power-domain is shared with display. As mentioned above, >>> explicit reset in driver is not really necessary since BPMP is already >>> doing it during unpowergate stage. So the h/w is already ensured to be >>> in a good state. >> If you'll reload the driver module, then h/w won't be reset. > > How the reload case would be different? Can you please specify more > details if you are referring to a particular scenario? You have a shared power domain. Since power domain can be turned off only when nobody keeps domain turned on, you now making reset of HDA controller dependent on the state of display driver. Do you want to have inconsistent h/w reset behaviour depending on the runtime state of display driver? ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 14:07 ` Dmitry Osipenko @ 2021-12-07 14:49 ` Sameer Pujar 2021-12-07 15:35 ` Dmitry Osipenko 0 siblings, 1 reply; 39+ messages in thread From: Sameer Pujar @ 2021-12-07 14:49 UTC (permalink / raw) To: Dmitry Osipenko, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar On 12/7/2021 7:37 PM, Dmitry Osipenko wrote: > 07.12.2021 15:40, Sameer Pujar пишет: >> >> On 12/7/2021 5:35 PM, Dmitry Osipenko wrote: >>> External email: Use caution opening links or attachments >>> >>> >>> 07.12.2021 15:00, Sameer Pujar пишет: >>>> On 12/7/2021 3:52 PM, Dmitry Osipenko wrote: >>>>> 07.12.2021 09:32, Sameer Pujar пишет: >>>>>> HDA regression is recently reported on Tegra194 based platforms. >>>>>> This happens because "hda2codec_2x" reset does not really exist >>>>>> in Tegra194 and it causes probe failure. All the HDA based audio >>>>>> tests fail at the moment. This underlying issue is exposed by >>>>>> commit c045ceb5a145 ("reset: tegra-bpmp: Handle errors in BPMP >>>>>> response") which now checks return code of BPMP command response. >>>>>> >>>>>> The failure can be fixed by avoiding above reset in the driver, >>>>>> but the explicit reset is not necessary for Tegra devices which >>>>>> depend on BPMP. On such devices, BPMP ensures reset application >>>>>> during unpowergate calls. Hence skip reset on these devices >>>>>> which is applicable for Tegra186 and later. >>>>> The power domain is shared with the display, AFAICS. The point of reset >>>>> is to bring h/w into predictable state. It doesn't make sense to me to >>>>> skip the reset. >>>> Yes the power-domain is shared with display. As mentioned above, >>>> explicit reset in driver is not really necessary since BPMP is already >>>> doing it during unpowergate stage. So the h/w is already ensured to be >>>> in a good state. >>> If you'll reload the driver module, then h/w won't be reset. >> How the reload case would be different? Can you please specify more >> details if you are referring to a particular scenario? > You have a shared power domain. Since power domain can be turned off > only when nobody keeps domain turned on, you now making reset of HDA > controller dependent on the state of display driver. I don't think that the state of display driver would affect. The HDA driver itself can issue unpowergate calls which in turn ensures h/w reset. If display driver is already runtime active, HDA driver runtime resume after this would be still fine since h/w reset is already applied during display runtime resume. Note that both HDA and display resets are connected to this power-domain and BPMP applies these resets during unpowergate. > Do you want to have > inconsistent h/w reset behaviour depending on the runtime state of > display driver? Of course no. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 14:49 ` Sameer Pujar @ 2021-12-07 15:35 ` Dmitry Osipenko 2021-12-07 17:37 ` Sameer Pujar 0 siblings, 1 reply; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-07 15:35 UTC (permalink / raw) To: Sameer Pujar, tiwai, broonie, lgirdwood, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar, robh+dt 07.12.2021 17:49, Sameer Pujar пишет: ... >>> How the reload case would be different? Can you please specify more >>> details if you are referring to a particular scenario? >> You have a shared power domain. Since power domain can be turned off >> only when nobody keeps domain turned on, you now making reset of HDA >> controller dependent on the state of display driver. > > I don't think that the state of display driver would affect. The HDA > driver itself can issue unpowergate calls which in turn ensures h/w > reset. If display driver is already runtime active, HDA driver runtime > resume after this would be still fine since h/w reset is already applied > during display runtime resume. Note that both HDA and display resets are > connected to this power-domain and BPMP applies these resets during > unpowergate. HDA won't be reset while display is active on T186+. HDA will be reset while is display is inactive on T186+. HDA will be reset regardless of display state on pre-T186. This is a pure inconsistency of the reset behaviour. Please don't do it. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 15:35 ` Dmitry Osipenko @ 2021-12-07 17:37 ` Sameer Pujar 2021-12-07 18:02 ` Dmitry Osipenko 0 siblings, 1 reply; 39+ messages in thread From: Sameer Pujar @ 2021-12-07 17:37 UTC (permalink / raw) To: Dmitry Osipenko, tiwai, broonie, lgirdwood, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar, robh+dt On 12/7/2021 9:05 PM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > 07.12.2021 17:49, Sameer Pujar пишет: > ... >>>> How the reload case would be different? Can you please specify more >>>> details if you are referring to a particular scenario? >>> You have a shared power domain. Since power domain can be turned off >>> only when nobody keeps domain turned on, you now making reset of HDA >>> controller dependent on the state of display driver. >> I don't think that the state of display driver would affect. The HDA >> driver itself can issue unpowergate calls which in turn ensures h/w >> reset. If display driver is already runtime active, HDA driver runtime >> resume after this would be still fine since h/w reset is already applied >> during display runtime resume. Note that both HDA and display resets are >> connected to this power-domain and BPMP applies these resets during >> unpowergate. > HDA won't be reset while display is active on T186+. No. HDA reset is applied whenever power-domain is ungated. It can happen when either HDA or display device becomes active. So I don't think that it is inconsistent. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 17:37 ` Sameer Pujar @ 2021-12-07 18:02 ` Dmitry Osipenko 2021-12-08 5:22 ` Sameer Pujar 0 siblings, 1 reply; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-07 18:02 UTC (permalink / raw) To: Sameer Pujar, tiwai, broonie, lgirdwood, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar, robh+dt 07.12.2021 20:37, Sameer Pujar пишет: .. >> HDA won't be reset while display is active on T186+. > > No. HDA reset is applied whenever power-domain is ungated. It can happen > when either HDA or display device becomes active. I said "display is active", where do you see "becomes active"? > So I don't think that > it is inconsistent. If display is already active, then shared power domain is already ungated. It won't be ungated second time in a row, HDA won't get the reset. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-07 18:02 ` Dmitry Osipenko @ 2021-12-08 5:22 ` Sameer Pujar 2021-12-08 12:05 ` Dmitry Osipenko 0 siblings, 1 reply; 39+ messages in thread From: Sameer Pujar @ 2021-12-08 5:22 UTC (permalink / raw) To: Dmitry Osipenko, tiwai, broonie, lgirdwood, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar, robh+dt On 12/7/2021 11:32 PM, Dmitry Osipenko wrote > If display is already active, then shared power domain is already > ungated. If display is already active, then shared power domain is already ungated. HDA reset is already applied during this ungate. In other words, HDA would be reset as well when display ungates power-domain. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-08 5:22 ` Sameer Pujar @ 2021-12-08 12:05 ` Dmitry Osipenko 2021-12-14 6:02 ` Sameer Pujar 0 siblings, 1 reply; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-08 12:05 UTC (permalink / raw) To: Sameer Pujar, tiwai, broonie, lgirdwood, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar, robh+dt 08.12.2021 08:22, Sameer Pujar пишет: > > > On 12/7/2021 11:32 PM, Dmitry Osipenko wrote >> If display is already active, then shared power domain is already >> ungated. > > If display is already active, then shared power domain is already > ungated. HDA reset is already applied during this ungate. In other > words, HDA would be reset as well when display ungates power-domain. Now, if you'll reload the HDA driver module while display is active, you'll get a different reset behaviour. HDA hardware will be reset on pre-T186, on T186+ it won't be reset. Please make v2 using devm_reset_control_bulk_get_exclusive(), skipping the non-existent reset, or move the workaround to the BPMP driver like I suggested in the other reply. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-08 12:05 ` Dmitry Osipenko @ 2021-12-14 6:02 ` Sameer Pujar 2021-12-14 6:09 ` Dmitry Osipenko 0 siblings, 1 reply; 39+ messages in thread From: Sameer Pujar @ 2021-12-14 6:02 UTC (permalink / raw) To: Dmitry Osipenko, tiwai, broonie, lgirdwood, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar, robh+dt On 12/8/2021 5:35 PM, Dmitry Osipenko wrote: > 08.12.2021 08:22, Sameer Pujar пишет: >> >> On 12/7/2021 11:32 PM, Dmitry Osipenko wrote >>> If display is already active, then shared power domain is already >>> ungated. >> If display is already active, then shared power domain is already >> ungated. HDA reset is already applied during this ungate. In other >> words, HDA would be reset as well when display ungates power-domain. > Now, if you'll reload the HDA driver module while display is active, > you'll get a different reset behaviour. HDA hardware will be reset on > pre-T186, on T186+ it won't be reset. How the reset behavior is different? At this point when HDA driver is loaded the HW is already reset during display ungate. What matters, during HDA driver load, is whether the HW is in predictable state or not and the answer is yes. So I am not sure what problem you are referring to. Question is, if BPMP already ensures this, then why driver needs to take care of it. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-14 6:02 ` Sameer Pujar @ 2021-12-14 6:09 ` Dmitry Osipenko 2021-12-14 6:15 ` Dmitry Osipenko 2021-12-14 7:22 ` Sameer Pujar 0 siblings, 2 replies; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-14 6:09 UTC (permalink / raw) To: Sameer Pujar, tiwai, broonie, lgirdwood, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar, robh+dt 14.12.2021 09:02, Sameer Pujar пишет: > > > On 12/8/2021 5:35 PM, Dmitry Osipenko wrote: >> 08.12.2021 08:22, Sameer Pujar пишет: >>> >>> On 12/7/2021 11:32 PM, Dmitry Osipenko wrote >>>> If display is already active, then shared power domain is already >>>> ungated. >>> If display is already active, then shared power domain is already >>> ungated. HDA reset is already applied during this ungate. In other >>> words, HDA would be reset as well when display ungates power-domain. >> Now, if you'll reload the HDA driver module while display is active, >> you'll get a different reset behaviour. HDA hardware will be reset on >> pre-T186, on T186+ it won't be reset. > > How the reset behavior is different? At this point when HDA driver is > loaded the HW is already reset during display ungate. What matters, > during HDA driver load, is whether the HW is in predictable state or not > and the answer is yes. So I am not sure what problem you are referring > to. Question is, if BPMP already ensures this, then why driver needs to > take care of it. 1. Enable display 2. Play audio over HDMI 3. HDA hardware now is in dirty state 4. Reload HDA driver 5. In your case HDA is kept in dirty state, in my no ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-14 6:09 ` Dmitry Osipenko @ 2021-12-14 6:15 ` Dmitry Osipenko 2021-12-14 7:22 ` Sameer Pujar 1 sibling, 0 replies; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-14 6:15 UTC (permalink / raw) To: Sameer Pujar, tiwai, broonie, lgirdwood, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar, robh+dt 14.12.2021 09:09, Dmitry Osipenko пишет: > 14.12.2021 09:02, Sameer Pujar пишет: >> >> >> On 12/8/2021 5:35 PM, Dmitry Osipenko wrote: >>> 08.12.2021 08:22, Sameer Pujar пишет: >>>> >>>> On 12/7/2021 11:32 PM, Dmitry Osipenko wrote >>>>> If display is already active, then shared power domain is already >>>>> ungated. >>>> If display is already active, then shared power domain is already >>>> ungated. HDA reset is already applied during this ungate. In other >>>> words, HDA would be reset as well when display ungates power-domain. >>> Now, if you'll reload the HDA driver module while display is active, >>> you'll get a different reset behaviour. HDA hardware will be reset on >>> pre-T186, on T186+ it won't be reset. >> >> How the reset behavior is different? At this point when HDA driver is >> loaded the HW is already reset during display ungate. What matters, >> during HDA driver load, is whether the HW is in predictable state or not >> and the answer is yes. So I am not sure what problem you are referring >> to. Question is, if BPMP already ensures this, then why driver needs to >> take care of it. > > 1. Enable display > 2. Play audio over HDMI > 3. HDA hardware now is in dirty state > 4. Reload HDA driver > 5. In your case HDA is kept in dirty state, in my no > The power domain is shared by display and HDA, is this correct? If yes, then the shared power domain will be turned off only when all its clients are turned off, i.e. both display and HDA simultaneously. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-14 6:09 ` Dmitry Osipenko 2021-12-14 6:15 ` Dmitry Osipenko @ 2021-12-14 7:22 ` Sameer Pujar 2021-12-14 13:56 ` Dmitry Osipenko 1 sibling, 1 reply; 39+ messages in thread From: Sameer Pujar @ 2021-12-14 7:22 UTC (permalink / raw) To: Dmitry Osipenko, tiwai, broonie, lgirdwood, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar, robh+dt On 12/14/2021 11:39 AM, Dmitry Osipenko wrote: > 14.12.2021 09:02, Sameer Pujar пишет: >> >> On 12/8/2021 5:35 PM, Dmitry Osipenko wrote: >>> 08.12.2021 08:22, Sameer Pujar пишет: >>>> On 12/7/2021 11:32 PM, Dmitry Osipenko wrote >>>>> If display is already active, then shared power domain is already >>>>> ungated. >>>> If display is already active, then shared power domain is already >>>> ungated. HDA reset is already applied during this ungate. In other >>>> words, HDA would be reset as well when display ungates power-domain. >>> Now, if you'll reload the HDA driver module while display is active, >>> you'll get a different reset behaviour. HDA hardware will be reset on >>> pre-T186, on T186+ it won't be reset. >> How the reset behavior is different? At this point when HDA driver is >> loaded the HW is already reset during display ungate. What matters, >> during HDA driver load, is whether the HW is in predictable state or not >> and the answer is yes. So I am not sure what problem you are referring >> to. Question is, if BPMP already ensures this, then why driver needs to >> take care of it. > 1. Enable display > 2. Play audio over HDMI > 3. HDA hardware now is in dirty state Why this would be a dirty state? It is rather a functional state. Isn't it? Power-domain is ON while all this happens. Another point is, with present logic the reset is not applied for every runtime PM resume of HDA device, which is confusing. It depends on the state of 'chip->running' flag and I don't see this getting cleared anywhere. Would you say subsequent HDA playback happen under a dirty state? ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-14 7:22 ` Sameer Pujar @ 2021-12-14 13:56 ` Dmitry Osipenko 2021-12-14 14:29 ` Takashi Iwai 2021-12-20 10:32 ` Sameer Pujar 0 siblings, 2 replies; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-14 13:56 UTC (permalink / raw) To: Sameer Pujar, tiwai, broonie, lgirdwood, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar, robh+dt 14.12.2021 10:22, Sameer Pujar пишет: ... >>> How the reset behavior is different? At this point when HDA driver is >>> loaded the HW is already reset during display ungate. What matters, >>> during HDA driver load, is whether the HW is in predictable state or not >>> and the answer is yes. So I am not sure what problem you are referring >>> to. Question is, if BPMP already ensures this, then why driver needs to >>> take care of it. >> 1. Enable display >> 2. Play audio over HDMI > >> 3. HDA hardware now is in dirty state > > Why this would be a dirty state? It is rather a functional state. Isn't > it? Power-domain is ON while all this happens. In general state should be a functional, but we shouldn't assume that. There is always a possibility for a subtle bug in a driver that may put h/w into a bad state. Full hardware reset is encouraged by users. > Another point is, with present logic the reset is not applied for every > runtime PM resume of HDA device, which is confusing. It depends on the > state of 'chip->running' flag and I don't see this getting cleared > anywhere. Would you say subsequent HDA playback happen under a dirty state? This is a good point. There should be another potential problem in the HDA driver for newer SoCs because apparently we don't re-initialize HDA controller properly after runtime PM resume. See hda_tegra_first_init() that is invoked only during driver probe, it configures FPCI_DBG_CFG_2 register on T194, which isn't done by hda_tegra_init(), and thus, this register may be in reset state after resume from RPM suspend. It should be a bug in the HDA driver that needs to be fixed. On older SoCs: HDA resides in the APB power domain which could be disabled only across system suspend/resume. HDA is only clock-gated during runtime PM suspend. On newer SoCs: HDA power state could be lost after RPM suspend/resume, depending on the state of display. I'm wondering whether HDMI playback works after DPMS on T194+, I assume this case was never tested properly. It looks like it should be safe to reset HDA on runtime PM resume regardless of the chip->running, and thus, we could remove that check and reset HDA unconditionally. Will great if you could check/test and improve this in the driver. I'm also wondering whether snd_power_change_state() should be moved into RPM callbacks and whether this function does anything practically useful on Tegra at all. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-14 13:56 ` Dmitry Osipenko @ 2021-12-14 14:29 ` Takashi Iwai 2021-12-14 15:34 ` Dmitry Osipenko 2021-12-20 10:32 ` Sameer Pujar 1 sibling, 1 reply; 39+ messages in thread From: Takashi Iwai @ 2021-12-14 14:29 UTC (permalink / raw) To: Dmitry Osipenko Cc: Sameer Pujar, tiwai, broonie, lgirdwood, thierry.reding, perex, jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar, robh+dt On Tue, 14 Dec 2021 14:56:12 +0100, Dmitry Osipenko wrote: > > 14.12.2021 10:22, Sameer Pujar пишет: > ... > >>> How the reset behavior is different? At this point when HDA driver is > >>> loaded the HW is already reset during display ungate. What matters, > >>> during HDA driver load, is whether the HW is in predictable state or not > >>> and the answer is yes. So I am not sure what problem you are referring > >>> to. Question is, if BPMP already ensures this, then why driver needs to > >>> take care of it. > >> 1. Enable display > >> 2. Play audio over HDMI > > > >> 3. HDA hardware now is in dirty state > > > > Why this would be a dirty state? It is rather a functional state. Isn't > > it? Power-domain is ON while all this happens. > > In general state should be a functional, but we shouldn't assume that. > There is always a possibility for a subtle bug in a driver that may put > h/w into a bad state. Full hardware reset is encouraged by users. > > > Another point is, with present logic the reset is not applied for every > > runtime PM resume of HDA device, which is confusing. It depends on the > > state of 'chip->running' flag and I don't see this getting cleared > > anywhere. Would you say subsequent HDA playback happen under a dirty state? > > This is a good point. There should be another potential problem in the > HDA driver for newer SoCs because apparently we don't re-initialize HDA > controller properly after runtime PM resume. > > See hda_tegra_first_init() that is invoked only during driver probe, it > configures FPCI_DBG_CFG_2 register on T194, which isn't done by > hda_tegra_init(), and thus, this register may be in reset state after > resume from RPM suspend. It should be a bug in the HDA driver that needs > to be fixed. > > On older SoCs: HDA resides in the APB power domain which could be > disabled only across system suspend/resume. HDA is only clock-gated > during runtime PM suspend. > > On newer SoCs: HDA power state could be lost after RPM suspend/resume, > depending on the state of display. I'm wondering whether HDMI playback > works after DPMS on T194+, I assume this case was never tested properly. > > It looks like it should be safe to reset HDA on runtime PM resume > regardless of the chip->running, and thus, we could remove that check > and reset HDA unconditionally. Will great if you could check/test and > improve this in the driver. > > I'm also wondering whether snd_power_change_state() should be moved into > RPM callbacks and whether this function does anything practically useful > on Tegra at all. This call is mostly for ALSA core stuff, and not necessarily reflecting the exact device power state. The major role is for controlling / blocking the device accesses at the system suspend/resume, so it's correct to set only in the system suspend/resume callbacks, not in runtime PM. Takashi ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-14 14:29 ` Takashi Iwai @ 2021-12-14 15:34 ` Dmitry Osipenko 0 siblings, 0 replies; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-14 15:34 UTC (permalink / raw) To: Takashi Iwai Cc: Sameer Pujar, tiwai, broonie, lgirdwood, thierry.reding, perex, jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar, robh+dt 14.12.2021 17:29, Takashi Iwai пишет: >> I'm also wondering whether snd_power_change_state() should be moved into >> RPM callbacks and whether this function does anything practically useful >> on Tegra at all. > This call is mostly for ALSA core stuff, and not necessarily > reflecting the exact device power state. The major role is for > controlling / blocking the device accesses at the system > suspend/resume, so it's correct to set only in the system > suspend/resume callbacks, not in runtime PM. > Thank you for the clarification. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices 2021-12-14 13:56 ` Dmitry Osipenko 2021-12-14 14:29 ` Takashi Iwai @ 2021-12-20 10:32 ` Sameer Pujar 1 sibling, 0 replies; 39+ messages in thread From: Sameer Pujar @ 2021-12-20 10:32 UTC (permalink / raw) To: Dmitry Osipenko, tiwai, broonie, lgirdwood, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel, stable, Mohan Kumar, robh+dt On 12/14/2021 7:26 PM, Dmitry Osipenko wrote: > 14.12.2021 10:22, Sameer Pujar пишет: > ... >>>> How the reset behavior is different? At this point when HDA driver is >>>> loaded the HW is already reset during display ungate. What matters, >>>> during HDA driver load, is whether the HW is in predictable state or not >>>> and the answer is yes. So I am not sure what problem you are referring >>>> to. Question is, if BPMP already ensures this, then why driver needs to >>>> take care of it. >>> 1. Enable display >>> 2. Play audio over HDMI >>> 3. HDA hardware now is in dirty state >> Why this would be a dirty state? It is rather a functional state. Isn't >> it? Power-domain is ON while all this happens. > In general state should be a functional, but we shouldn't assume that. > There is always a possibility for a subtle bug in a driver that may put > h/w into a bad state. Full hardware reset is encouraged by users. OK. I will prepare a v2 by just skipping the invalid reset for Tegra194. > >> Another point is, with present logic the reset is not applied for every >> runtime PM resume of HDA device, which is confusing. It depends on the >> state of 'chip->running' flag and I don't see this getting cleared >> anywhere. Would you say subsequent HDA playback happen under a dirty state? > This is a good point. There should be another potential problem in the > HDA driver for newer SoCs because apparently we don't re-initialize HDA > controller properly after runtime PM resume. > > See hda_tegra_first_init() that is invoked only during driver probe, it > configures FPCI_DBG_CFG_2 register on T194, which isn't done by > hda_tegra_init(), and thus, this register may be in reset state after > resume from RPM suspend. It should be a bug in the HDA driver that needs > to be fixed. > > On older SoCs: HDA resides in the APB power domain which could be > disabled only across system suspend/resume. HDA is only clock-gated > during runtime PM suspend. > > On newer SoCs: HDA power state could be lost after RPM suspend/resume, > depending on the state of display. I'm wondering whether HDMI playback > works after DPMS on T194+, I assume this case was never tested properly. > > It looks like it should be safe to reset HDA on runtime PM resume > regardless of the chip->running, and thus, we could remove that check > and reset HDA unconditionally. Will great if you could check/test and > improve this in the driver. There seems to be multiple issues. I will work on this separately and send a separate series. Presently basic function is broken on Tegra194 and will first send v2 to fix the regression. Thanks for review. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 2/3] dt-bindings: sound: tegra: Update HDA resets 2021-12-07 6:32 [PATCH 0/3] Fix Tegra194 HDA regression Sameer Pujar 2021-12-07 6:32 ` [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices Sameer Pujar @ 2021-12-07 6:32 ` Sameer Pujar 2021-12-07 8:18 ` Thierry Reding 2021-12-07 10:14 ` Dmitry Osipenko 2021-12-07 6:32 ` [PATCH 3/3] arm64: tegra: Remove non existent Tegra194 reset Sameer Pujar 2021-12-07 8:04 ` [PATCH 0/3] Fix Tegra194 HDA regression Takashi Iwai 3 siblings, 2 replies; 39+ messages in thread From: Sameer Pujar @ 2021-12-07 6:32 UTC (permalink / raw) To: tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, digetx, alsa-devel, devicetree, linux-tegra, linux-kernel, Sameer Pujar Tegra194 HDA has only two resets unlike the previous generations of Tegra SoCs. Hence update the reset list accordingly. Fixes: 2d8f8955fe02 ("dt-bindings: tegra: Convert HDA doc to json-schema") Signed-off-by: Sameer Pujar <spujar@nvidia.com> --- .../devicetree/bindings/sound/nvidia,tegra30-hda.yaml | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml b/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml index b55775e..70dbdff5 100644 --- a/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml +++ b/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml @@ -50,13 +50,18 @@ properties: - const: hda2codec_2x resets: + minItems: 2 maxItems: 3 reset-names: - items: - - const: hda - - const: hda2hdmi - - const: hda2codec_2x + oneOf: + - items: + - const: hda + - const: hda2hdmi + - const: hda2codec_2x + - items: + - const: hda + - const: hda2hdmi power-domains: maxItems: 1 -- 2.7.4 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 2/3] dt-bindings: sound: tegra: Update HDA resets 2021-12-07 6:32 ` [PATCH 2/3] dt-bindings: sound: tegra: Update HDA resets Sameer Pujar @ 2021-12-07 8:18 ` Thierry Reding 2021-12-07 9:27 ` Sameer Pujar 2021-12-07 10:14 ` Dmitry Osipenko 1 sibling, 1 reply; 39+ messages in thread From: Thierry Reding @ 2021-12-07 8:18 UTC (permalink / raw) To: Sameer Pujar Cc: tiwai, broonie, lgirdwood, robh+dt, perex, jonathanh, digetx, alsa-devel, devicetree, linux-tegra, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1523 bytes --] On Tue, Dec 07, 2021 at 12:02:49PM +0530, Sameer Pujar wrote: > Tegra194 HDA has only two resets unlike the previous generations of > Tegra SoCs. Hence update the reset list accordingly. > > Fixes: 2d8f8955fe02 ("dt-bindings: tegra: Convert HDA doc to json-schema") > Signed-off-by: Sameer Pujar <spujar@nvidia.com> > --- > .../devicetree/bindings/sound/nvidia,tegra30-hda.yaml | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml b/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml > index b55775e..70dbdff5 100644 > --- a/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml > +++ b/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.yaml > @@ -50,13 +50,18 @@ properties: > - const: hda2codec_2x > > resets: > + minItems: 2 > maxItems: 3 > > reset-names: > - items: > - - const: hda > - - const: hda2hdmi > - - const: hda2codec_2x > + oneOf: > + - items: > + - const: hda > + - const: hda2hdmi > + - const: hda2codec_2x > + - items: > + - const: hda > + - const: hda2hdmi We could make this more specific using an if: conditional and match on nvidia,tegra194-hda, but this would work as well, especially since I presume that Tegra194's successors will also not have the hda2codec_2x reset. Acked-by: Thierry Reding <treding@nvidia.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/3] dt-bindings: sound: tegra: Update HDA resets 2021-12-07 8:18 ` Thierry Reding @ 2021-12-07 9:27 ` Sameer Pujar 0 siblings, 0 replies; 39+ messages in thread From: Sameer Pujar @ 2021-12-07 9:27 UTC (permalink / raw) To: Thierry Reding Cc: tiwai, broonie, lgirdwood, robh+dt, perex, jonathanh, digetx, alsa-devel, devicetree, linux-tegra, linux-kernel On 12/7/2021 1:48 PM, Thierry Reding wrote: > We could make this more specific using an if: conditional and match on > nvidia,tegra194-hda, but this would work as well, especially since I > presume that Tegra194's successors will also not have the hda2codec_2x > reset. Yes, reset hda2codec_2x is dropped from Tegra194 onward. So I believe this should be good enough. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/3] dt-bindings: sound: tegra: Update HDA resets 2021-12-07 6:32 ` [PATCH 2/3] dt-bindings: sound: tegra: Update HDA resets Sameer Pujar 2021-12-07 8:18 ` Thierry Reding @ 2021-12-07 10:14 ` Dmitry Osipenko 2021-12-07 11:04 ` Sameer Pujar 1 sibling, 1 reply; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-07 10:14 UTC (permalink / raw) To: Sameer Pujar, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel 07.12.2021 09:32, Sameer Pujar пишет: > Tegra194 HDA has only two resets unlike the previous generations of > Tegra SoCs. Hence update the reset list accordingly. > > Fixes: 2d8f8955fe02 ("dt-bindings: tegra: Convert HDA doc to json-schema") The original txt binding was already wrong, this "fixes" tag is wrong. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/3] dt-bindings: sound: tegra: Update HDA resets 2021-12-07 10:14 ` Dmitry Osipenko @ 2021-12-07 11:04 ` Sameer Pujar 2021-12-07 12:02 ` Dmitry Osipenko 0 siblings, 1 reply; 39+ messages in thread From: Sameer Pujar @ 2021-12-07 11:04 UTC (permalink / raw) To: Dmitry Osipenko, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel On 12/7/2021 3:44 PM, Dmitry Osipenko wrote: > 07.12.2021 09:32, Sameer Pujar пишет: >> Tegra194 HDA has only two resets unlike the previous generations of >> Tegra SoCs. Hence update the reset list accordingly. >> >> Fixes: 2d8f8955fe02 ("dt-bindings: tegra: Convert HDA doc to json-schema") > The original txt binding was already wrong, this "fixes" tag is wrong. The text didn't document "nvidia,tegra194-hda" compatibile support until the json-schema conversion happened. Perhaps the text doc was not updated when Tegra194 support was added. So wouldn't this be right to use json-schema commit as a base for this? ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/3] dt-bindings: sound: tegra: Update HDA resets 2021-12-07 11:04 ` Sameer Pujar @ 2021-12-07 12:02 ` Dmitry Osipenko 2021-12-07 12:22 ` Sameer Pujar 0 siblings, 1 reply; 39+ messages in thread From: Dmitry Osipenko @ 2021-12-07 12:02 UTC (permalink / raw) To: Sameer Pujar, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel 07.12.2021 14:04, Sameer Pujar пишет: > > > On 12/7/2021 3:44 PM, Dmitry Osipenko wrote: >> 07.12.2021 09:32, Sameer Pujar пишет: >>> Tegra194 HDA has only two resets unlike the previous generations of >>> Tegra SoCs. Hence update the reset list accordingly. >>> >>> Fixes: 2d8f8955fe02 ("dt-bindings: tegra: Convert HDA doc to >>> json-schema") >> The original txt binding was already wrong, this "fixes" tag is wrong. > > The text didn't document "nvidia,tegra194-hda" compatibile support until > the json-schema conversion happened. Perhaps the text doc was not > updated when Tegra194 support was added. So wouldn't this be right to > use json-schema commit as a base for this? This problem didn't exist when the binding was converted. Should be better to drop this tag since it doesn't add much value and creates confusion, IMO. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/3] dt-bindings: sound: tegra: Update HDA resets 2021-12-07 12:02 ` Dmitry Osipenko @ 2021-12-07 12:22 ` Sameer Pujar 0 siblings, 0 replies; 39+ messages in thread From: Sameer Pujar @ 2021-12-07 12:22 UTC (permalink / raw) To: Dmitry Osipenko, tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, alsa-devel, devicetree, linux-tegra, linux-kernel On 12/7/2021 5:32 PM, Dmitry Osipenko wrote: > 07.12.2021 14:04, Sameer Pujar пишет: >> >> On 12/7/2021 3:44 PM, Dmitry Osipenko wrote: >>> 07.12.2021 09:32, Sameer Pujar пишет: >>>> Tegra194 HDA has only two resets unlike the previous generations of >>>> Tegra SoCs. Hence update the reset list accordingly. >>>> >>>> Fixes: 2d8f8955fe02 ("dt-bindings: tegra: Convert HDA doc to >>>> json-schema") >>> The original txt binding was already wrong, this "fixes" tag is wrong. >> The text didn't document "nvidia,tegra194-hda" compatibile support until >> the json-schema conversion happened. Perhaps the text doc was not >> updated when Tegra194 support was added. So wouldn't this be right to >> use json-schema commit as a base for this? > This problem didn't exist when the binding was converted. Should be > better to drop this tag since it doesn't add much value and creates > confusion, IMO. It is true that the problem was introduced recently, but from the documentation point of view, given the HW never had this reset it should not have been included in the first place. If it is not useful, I can just drop this tag. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 3/3] arm64: tegra: Remove non existent Tegra194 reset 2021-12-07 6:32 [PATCH 0/3] Fix Tegra194 HDA regression Sameer Pujar 2021-12-07 6:32 ` [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices Sameer Pujar 2021-12-07 6:32 ` [PATCH 2/3] dt-bindings: sound: tegra: Update HDA resets Sameer Pujar @ 2021-12-07 6:32 ` Sameer Pujar 2021-12-07 8:04 ` [PATCH 0/3] Fix Tegra194 HDA regression Takashi Iwai 3 siblings, 0 replies; 39+ messages in thread From: Sameer Pujar @ 2021-12-07 6:32 UTC (permalink / raw) To: tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex Cc: jonathanh, digetx, alsa-devel, devicetree, linux-tegra, linux-kernel, Sameer Pujar Tegra194 does not really have "hda2codec_2x" related reset. Hence drop this entry to reflect actual HW. Fixes: 4878cc0c9fab ("arm64: tegra: Add HDA controller on Tegra194") Signed-off-by: Sameer Pujar <spujar@nvidia.com> --- arch/arm64/boot/dts/nvidia/tegra194.dtsi | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi index 8d29b7f..6a1d896 100644 --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi @@ -976,9 +976,8 @@ <&bpmp TEGRA194_CLK_HDA2CODEC_2X>; clock-names = "hda", "hda2hdmi", "hda2codec_2x"; resets = <&bpmp TEGRA194_RESET_HDA>, - <&bpmp TEGRA194_RESET_HDA2HDMICODEC>, - <&bpmp TEGRA194_RESET_HDA2CODEC_2X>; - reset-names = "hda", "hda2hdmi", "hda2codec_2x"; + <&bpmp TEGRA194_RESET_HDA2HDMICODEC>; + reset-names = "hda", "hda2hdmi"; power-domains = <&bpmp TEGRA194_POWER_DOMAIN_DISP>; interconnects = <&mc TEGRA194_MEMORY_CLIENT_HDAR &emc>, <&mc TEGRA194_MEMORY_CLIENT_HDAW &emc>; -- 2.7.4 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 0/3] Fix Tegra194 HDA regression 2021-12-07 6:32 [PATCH 0/3] Fix Tegra194 HDA regression Sameer Pujar ` (2 preceding siblings ...) 2021-12-07 6:32 ` [PATCH 3/3] arm64: tegra: Remove non existent Tegra194 reset Sameer Pujar @ 2021-12-07 8:04 ` Takashi Iwai 2021-12-07 8:21 ` Thierry Reding 3 siblings, 1 reply; 39+ messages in thread From: Takashi Iwai @ 2021-12-07 8:04 UTC (permalink / raw) To: Sameer Pujar Cc: tiwai, broonie, lgirdwood, robh+dt, thierry.reding, perex, jonathanh, digetx, alsa-devel, devicetree, linux-tegra, linux-kernel On Tue, 07 Dec 2021 07:32:47 +0100, Sameer Pujar wrote: > > HDA probe failure is observed on Tegra194 based platforms and this > happens due to a reset failure. The series fixes this problem by > avoiding explicit resets on BPMP based devices. > > Sameer Pujar (3): > ALSA: hda/tegra: Skip reset on BPMP devices > dt-bindings: sound: tegra: Update HDA resets > arm64: tegra: Remove non existent Tegra194 reset The change in HDA drier looks fine, and the question how to take those patches. If other people can give acks, I can take those to sound.git tree destined for 5.16. Or, if anyone else prefers taking those, feel free to do: for the first patch, Reviewed-by: Takashi Iwai <tiwai@suse.de> thanks, Takashi ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/3] Fix Tegra194 HDA regression 2021-12-07 8:04 ` [PATCH 0/3] Fix Tegra194 HDA regression Takashi Iwai @ 2021-12-07 8:21 ` Thierry Reding 0 siblings, 0 replies; 39+ messages in thread From: Thierry Reding @ 2021-12-07 8:21 UTC (permalink / raw) To: Takashi Iwai Cc: Sameer Pujar, tiwai, broonie, lgirdwood, robh+dt, perex, jonathanh, digetx, alsa-devel, devicetree, linux-tegra, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1300 bytes --] On Tue, Dec 07, 2021 at 09:04:54AM +0100, Takashi Iwai wrote: > On Tue, 07 Dec 2021 07:32:47 +0100, > Sameer Pujar wrote: > > > > HDA probe failure is observed on Tegra194 based platforms and this > > happens due to a reset failure. The series fixes this problem by > > avoiding explicit resets on BPMP based devices. > > > > Sameer Pujar (3): > > ALSA: hda/tegra: Skip reset on BPMP devices > > dt-bindings: sound: tegra: Update HDA resets > > arm64: tegra: Remove non existent Tegra194 reset > > The change in HDA drier looks fine, and the question how to take those > patches. If other people can give acks, I can take those to sound.git > tree destined for 5.16. > > Or, if anyone else prefers taking those, feel free to do: for the > first patch, > > Reviewed-by: Takashi Iwai <tiwai@suse.de> My understanding is that patches 1-2 are mostly independent of patch 3, with the latter being mostly a complementary cleanup. Patch 1 will effectively ignore the non-existent reset on Tegra194 anyway. Patch 1 (and potentially 2) could go into v5.16 to fix the regression via sound.git, but the cleanup in patch 3 isn't time-critical, so I could just queue that up for v5.17 via the Tegra tree. Sameer, any objections to that? Did I miss anything? Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2021-12-20 10:32 UTC | newest] Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-07 6:32 [PATCH 0/3] Fix Tegra194 HDA regression Sameer Pujar 2021-12-07 6:32 ` [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices Sameer Pujar 2021-12-07 8:16 ` Thierry Reding 2021-12-07 8:36 ` Takashi Iwai 2021-12-07 9:09 ` Sameer Pujar 2021-12-07 10:22 ` Dmitry Osipenko 2021-12-07 10:44 ` Dmitry Osipenko 2021-12-07 10:58 ` Dmitry Osipenko 2021-12-07 11:02 ` Jon Hunter 2021-12-07 11:57 ` Dmitry Osipenko 2021-12-07 15:07 ` Jon Hunter 2021-12-07 12:00 ` Sameer Pujar 2021-12-07 12:05 ` Dmitry Osipenko 2021-12-07 12:40 ` Sameer Pujar 2021-12-07 14:07 ` Dmitry Osipenko 2021-12-07 14:49 ` Sameer Pujar 2021-12-07 15:35 ` Dmitry Osipenko 2021-12-07 17:37 ` Sameer Pujar 2021-12-07 18:02 ` Dmitry Osipenko 2021-12-08 5:22 ` Sameer Pujar 2021-12-08 12:05 ` Dmitry Osipenko 2021-12-14 6:02 ` Sameer Pujar 2021-12-14 6:09 ` Dmitry Osipenko 2021-12-14 6:15 ` Dmitry Osipenko 2021-12-14 7:22 ` Sameer Pujar 2021-12-14 13:56 ` Dmitry Osipenko 2021-12-14 14:29 ` Takashi Iwai 2021-12-14 15:34 ` Dmitry Osipenko 2021-12-20 10:32 ` Sameer Pujar 2021-12-07 6:32 ` [PATCH 2/3] dt-bindings: sound: tegra: Update HDA resets Sameer Pujar 2021-12-07 8:18 ` Thierry Reding 2021-12-07 9:27 ` Sameer Pujar 2021-12-07 10:14 ` Dmitry Osipenko 2021-12-07 11:04 ` Sameer Pujar 2021-12-07 12:02 ` Dmitry Osipenko 2021-12-07 12:22 ` Sameer Pujar 2021-12-07 6:32 ` [PATCH 3/3] arm64: tegra: Remove non existent Tegra194 reset Sameer Pujar 2021-12-07 8:04 ` [PATCH 0/3] Fix Tegra194 HDA regression Takashi Iwai 2021-12-07 8:21 ` Thierry Reding
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).