linux-kernel - Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <s5hv8zr9s5a.wl-tiwai@suse.de>
Date:   Tue, 14 Dec 2021 15:29:21 +0100
From:   Takashi Iwai <tiwai@...e.de>
To:     Dmitry Osipenko <digetx@...il.com>
Cc:     Sameer Pujar <spujar@...dia.com>, tiwai@...e.com,
        broonie@...nel.org, lgirdwood@...il.com, thierry.reding@...il.com,
        perex@...ex.cz, jonathanh@...dia.com, alsa-devel@...a-project.org,
        devicetree@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Mohan Kumar <mkumard@...dia.com>, robh+dt@...nel.org
Subject: Re: [PATCH 1/3] ALSA: hda/tegra: Skip reset on BPMP devices

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