All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sameer Pujar <spujar@nvidia.com>
To: Dmitry Osipenko <digetx@gmail.com>,
	tiwai@suse.com, broonie@kernel.org, lgirdwood@gmail.com,
	thierry.reding@gmail.com, perex@perex.cz
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	jonathanh@nvidia.com, robh+dt@kernel.org,
	linux-tegra@vger.kernel.org, Mohan Kumar <mkumard@nvidia.com>
Subject: Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices
Date: Mon, 20 Dec 2021 16:02:26 +0530	[thread overview]
Message-ID: <b57bc2c0-7aed-635c-a488-bb50af470a36@nvidia.com> (raw)
In-Reply-To: <0761f6f2-27f8-4e1a-fabc-9d319f465a9e@gmail.com>



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.

WARNING: multiple messages have this Message-ID (diff)
From: Sameer Pujar <spujar@nvidia.com>
To: Dmitry Osipenko <digetx@gmail.com>,
	tiwai@suse.com, broonie@kernel.org, lgirdwood@gmail.com,
	thierry.reding@gmail.com, perex@perex.cz
Cc: jonathanh@nvidia.com, alsa-devel@alsa-project.org,
	devicetree@vger.kernel.org, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Mohan Kumar <mkumard@nvidia.com>,
	robh+dt@kernel.org
Subject: Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices
Date: Mon, 20 Dec 2021 16:02:26 +0530	[thread overview]
Message-ID: <b57bc2c0-7aed-635c-a488-bb50af470a36@nvidia.com> (raw)
In-Reply-To: <0761f6f2-27f8-4e1a-fabc-9d319f465a9e@gmail.com>



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.

  parent reply	other threads:[~2021-12-20 10:33 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-07  6:32 [PATCH 0/3] Fix Tegra194 HDA regression Sameer Pujar
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
2021-12-07  6:32   ` Sameer Pujar
2021-12-07  8:16   ` Thierry Reding
2021-12-07  8:16     ` Thierry Reding
2021-12-07  8:36     ` Takashi Iwai
2021-12-07  8:36       ` Takashi Iwai
2021-12-07  9:09       ` Sameer Pujar
2021-12-07  9:09         ` Sameer Pujar
2021-12-07 10:22   ` Dmitry Osipenko
2021-12-07 10:22     ` Dmitry Osipenko
2021-12-07 10:44     ` Dmitry Osipenko
2021-12-07 10:44       ` Dmitry Osipenko
2021-12-07 10:58       ` Dmitry Osipenko
2021-12-07 10:58         ` Dmitry Osipenko
2021-12-07 11:02         ` Jon Hunter
2021-12-07 11:02           ` Jon Hunter
2021-12-07 11:57           ` Dmitry Osipenko
2021-12-07 11:57             ` Dmitry Osipenko
2021-12-07 15:07             ` Jon Hunter
2021-12-07 15:07               ` Jon Hunter
2021-12-07 12:00     ` Sameer Pujar
2021-12-07 12:00       ` Sameer Pujar
2021-12-07 12:05       ` Dmitry Osipenko
2021-12-07 12:05         ` Dmitry Osipenko
2021-12-07 12:40         ` Sameer Pujar
2021-12-07 12:40           ` Sameer Pujar
2021-12-07 14:07           ` Dmitry Osipenko
2021-12-07 14:07             ` Dmitry Osipenko
2021-12-07 14:49             ` Sameer Pujar
2021-12-07 14:49               ` Sameer Pujar
2021-12-07 15:35               ` Dmitry Osipenko
2021-12-07 15:35                 ` Dmitry Osipenko
2021-12-07 17:37                 ` Sameer Pujar
2021-12-07 17:37                   ` Sameer Pujar
2021-12-07 18:02                   ` Dmitry Osipenko
2021-12-07 18:02                     ` Dmitry Osipenko
2021-12-08  5:22                     ` Sameer Pujar
2021-12-08  5:22                       ` Sameer Pujar
2021-12-08 12:05                       ` Dmitry Osipenko
2021-12-08 12:05                         ` Dmitry Osipenko
2021-12-14  6:02                         ` Sameer Pujar
2021-12-14  6:02                           ` Sameer Pujar
2021-12-14  6:09                           ` Dmitry Osipenko
2021-12-14  6:09                             ` Dmitry Osipenko
2021-12-14  6:15                             ` Dmitry Osipenko
2021-12-14  6:15                               ` Dmitry Osipenko
2021-12-14  7:22                             ` Sameer Pujar
2021-12-14  7:22                               ` Sameer Pujar
2021-12-14 13:56                               ` Dmitry Osipenko
2021-12-14 13:56                                 ` Dmitry Osipenko
2021-12-14 14:29                                 ` Takashi Iwai
2021-12-14 14:29                                   ` Takashi Iwai
2021-12-14 15:34                                   ` Dmitry Osipenko
2021-12-14 15:34                                     ` Dmitry Osipenko
2021-12-20 10:32                                 ` Sameer Pujar [this message]
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  6:32   ` Sameer Pujar
2021-12-07  8:18   ` Thierry Reding
2021-12-07  8:18     ` Thierry Reding
2021-12-07  9:27     ` Sameer Pujar
2021-12-07  9:27       ` Sameer Pujar
2021-12-07 10:14   ` Dmitry Osipenko
2021-12-07 10:14     ` Dmitry Osipenko
2021-12-07 11:04     ` Sameer Pujar
2021-12-07 11:04       ` Sameer Pujar
2021-12-07 12:02       ` Dmitry Osipenko
2021-12-07 12:02         ` Dmitry Osipenko
2021-12-07 12:22         ` Sameer Pujar
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  6:32   ` Sameer Pujar
2021-12-07  8:04 ` [PATCH 0/3] Fix Tegra194 HDA regression Takashi Iwai
2021-12-07  8:04   ` Takashi Iwai
2021-12-07  8:21   ` Thierry Reding
2021-12-07  8:21     ` Thierry Reding
2021-12-07  8:47     ` Sameer Pujar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b57bc2c0-7aed-635c-a488-bb50af470a36@nvidia.com \
    --to=spujar@nvidia.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=digetx@gmail.com \
    --cc=jonathanh@nvidia.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=mkumard@nvidia.com \
    --cc=perex@perex.cz \
    --cc=robh+dt@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=thierry.reding@gmail.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.