| Message ID | 20250207012531.621369-1-airlied@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/nouveau: select FW caching | expand |
(+ Luis, Russ) On Fri, Feb 07, 2025 at 11:25:31AM +1000, Dave Airlie wrote: > From: Dave Airlie <airlied@redhat.com> > > nouveau tries to load some firmware during suspend that it loaded earlier, but with > fw caching disabled it hangs suspend, so just rely on FW cache enabling instead of > working around it in the driver. > > Fixes: 176fdcbddfd2 ("drm/nouveau/gsp/r535: add support for booting GSP-RM") > Signed-off-by: Dave Airlie <airlied@redhat.com> > --- > drivers/gpu/drm/nouveau/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig > index ce840300578d8..1050a4617fc15 100644 > --- a/drivers/gpu/drm/nouveau/Kconfig > +++ b/drivers/gpu/drm/nouveau/Kconfig > @@ -4,6 +4,7 @@ config DRM_NOUVEAU > depends on DRM && PCI && MMU > select IOMMU_API > select FW_LOADER > + select FW_CACHE if PM_SLEEP CONFIG_FW_CACHE was added, as the Kconfig says, it "can prevent suspend on many platforms". @Luis, Russ: I assume this mostly means embedded platforms? I wonder if we should not insist on FW_CACHE if NOUVEAU_PLATFORM_DRIVER, or even only force FW_CACHE if DRM_NOUVEAU_GSP_DEFAULT?
On Fri, Feb 14, 2025 at 02:05:36PM +0100, Danilo Krummrich wrote:
> only force FW_CACHE if DRM_NOUVEAU_GSP_DEFAULT?
Please scratch that, it was a horrible idea.
On Fri, Feb 14, 2025 at 02:10:27PM +0100, Danilo Krummrich wrote: > On Fri, Feb 14, 2025 at 02:05:36PM +0100, Danilo Krummrich wrote: > > only force FW_CACHE if DRM_NOUVEAU_GSP_DEFAULT? > > Please scratch that, it was a horrible idea. What I recommend is to look into why we disable it by default, I think its sold old obscure reasoning but now suspect it was udev being dumb, in line with why we also try to defensively try to ignore duplicate udev requests for module requests causing a flood. Refer to commit 9b9879fc03275ff ("modules: catch concurrent module loads, treat them as idempotent") for details. Luis
On Tue, Feb 18, 2025 at 06:28:53AM -0800, Luis Chamberlain wrote: > > What I recommend is to look into why we disable it by default, I think I think FW_CACHE is enabled by default, no? > its sold old obscure reasoning but now suspect it was udev being dumb, > in line with why we also try to defensively try to ignore duplicate > udev requests for module requests causing a flood. Refer to commit > 9b9879fc03275ff ("modules: catch concurrent module loads, treat them as > idempotent") for details. Thanks, that helps a lot. Given that this commit landed in v6.5 I guess there's not really a concern to force enable FW_CACHE for Nouveau as a fix for commit 176fdcbddfd2 ("drm/nouveau/gsp/r535: add support for booting GSP-RM"), which landed in v6.7.
On Tue, Feb 18, 2025 at 06:24:43PM +0100, Danilo Krummrich wrote: > On Tue, Feb 18, 2025 at 06:28:53AM -0800, Luis Chamberlain wrote: > > > > What I recommend is to look into why we disable it by default, I think > > I think FW_CACHE is enabled by default, no? > > > its sold old obscure reasoning but now suspect it was udev being dumb, > > in line with why we also try to defensively try to ignore duplicate > > udev requests for module requests causing a flood. Refer to commit > > 9b9879fc03275ff ("modules: catch concurrent module loads, treat them as > > idempotent") for details. > > Thanks, that helps a lot. > > Given that this commit landed in v6.5 That commit lets us ignore the udev floods for superfluous module requests only. > I guess there's not really a concern to > force enable FW_CACHE for Nouveau as a fix for commit 176fdcbddfd2 > ("drm/nouveau/gsp/r535: add support for booting GSP-RM"), which landed in v6.7. I'd recommmend to look into *why* folks didn't like fw cache to look at disabling it in the first place, and look and see if udev was acting up and doing stupid things preventing suspend. Luis
On Wed, 19 Feb 2025 at 07:39, Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Tue, Feb 18, 2025 at 06:24:43PM +0100, Danilo Krummrich wrote: > > On Tue, Feb 18, 2025 at 06:28:53AM -0800, Luis Chamberlain wrote: > > > > > > What I recommend is to look into why we disable it by default, I think > > > > I think FW_CACHE is enabled by default, no? > > > > > its sold old obscure reasoning but now suspect it was udev being dumb, > > > in line with why we also try to defensively try to ignore duplicate > > > udev requests for module requests causing a flood. Refer to commit > > > 9b9879fc03275ff ("modules: catch concurrent module loads, treat them as > > > idempotent") for details. > > > > Thanks, that helps a lot. > > > > Given that this commit landed in v6.5 > > That commit lets us ignore the udev floods for superfluous module > requests only. > > > I guess there's not really a concern to > > force enable FW_CACHE for Nouveau as a fix for commit 176fdcbddfd2 > > ("drm/nouveau/gsp/r535: add support for booting GSP-RM"), which landed in v6.7. > > I'd recommmend to look into *why* folks didn't like fw cache to look at > disabling it in the first place, and look and see if udev was acting up > and doing stupid things preventing suspend. This has been on in Fedora for ever, Linus said it was originally on by default and Greg thinks Android folks had a regression, I don't think the intersections of nouveau and platforms which had problems a long time ago is interesting enough to care, All distros should enable this, it fixes problems, if Android finds a regression with nouveau it'll bisect to here. Dave. > > Luis
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig index ce840300578d8..1050a4617fc15 100644 --- a/drivers/gpu/drm/nouveau/Kconfig +++ b/drivers/gpu/drm/nouveau/Kconfig @@ -4,6 +4,7 @@ config DRM_NOUVEAU depends on DRM && PCI && MMU select IOMMU_API select FW_LOADER + select FW_CACHE if PM_SLEEP select DRM_CLIENT_SELECTION select DRM_DISPLAY_DP_HELPER select DRM_DISPLAY_HDMI_HELPER