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

[Takashi Iwai <tiwai@suse.de>]

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