[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPM=9txpeYNrGEd=KbHe0mLbrG+vucwdQYRMfmcXcXwWoeCkWA@mail.gmail.com>
Date: Wed, 10 Dec 2025 10:40:28 +1000
From: Dave Airlie <airlied@...il.com>
To: Lyude Paul <lyude@...hat.com>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
stable@...r.kernel.org, nouveau@...ts.freedesktop.org,
Faith Ekstrand <faith.ekstrand@...labora.com>, Dave Airlie <airlied@...hat.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Ben Skeggs <bskeggs@...dia.com>,
Simona Vetter <simona@...ll.ch>, Thomas Zimmermann <tzimmermann@...e.de>,
Maxime Ripard <mripard@...nel.org>, Danilo Krummrich <dakr@...nel.org>, James Jones <jajones@...dia.com>
Subject: Re: [PATCH] drm/nouveau/dispnv50: Don't call drm_atomic_get_crtc_state()
in prepare_fb
On Sat, 6 Dec 2025 at 07:32, Lyude Paul <lyude@...hat.com> wrote:
>
> Since we recently started warning about uses of this function after the
> atomic check phase completes, we've started getting warnings about this in
> nouveau. It appears a misplaced drm_atomic_get_crtc_state() call has been
> hiding in our .prepare_fb callback for a while.
>
> So, fix this by adding a new nv50_head_atom_get_new() function and use that
> in our .prepare_fb callback instead.
>
> Signed-off-by: Lyude Paul <lyude@...hat.com>
>
> Fixes: 1590700d94ac ("drm/nouveau/kms/nv50-: split each resource type into their own source files")
> Cc: <stable@...r.kernel.org> # v4.18+
> Signed-off-by: Lyude Paul <lyude@...hat.com>
> ---
> drivers/gpu/drm/nouveau/dispnv50/atom.h | 13 +++++++++++++
> drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 +-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/atom.h b/drivers/gpu/drm/nouveau/dispnv50/atom.h
> index 93f8f4f645784..85b7cf70d13c4 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/atom.h
> +++ b/drivers/gpu/drm/nouveau/dispnv50/atom.h
> @@ -152,8 +152,21 @@ static inline struct nv50_head_atom *
> nv50_head_atom_get(struct drm_atomic_state *state, struct drm_crtc *crtc)
> {
> struct drm_crtc_state *statec = drm_atomic_get_crtc_state(state, crtc);
> +
> if (IS_ERR(statec))
> return (void *)statec;
> +
> + return nv50_head_atom(statec);
> +}
> +
> +static inline struct nv50_head_atom *
> +nv50_head_atom_get_new(struct drm_atomic_state *state, struct drm_crtc *crtc)
> +{
> + struct drm_crtc_state *statec = drm_atomic_get_new_crtc_state(state, crtc);
> +
> + if (IS_ERR(statec))
> + return (void*)statec;
> +
So I was at kernel summit and someone was talking about AI review
prompts so I threw this patch at it, and it we shouldn't use IS_ERR
here, and I think it is correct.
get_new_crtc_state only returns NULL not an error.
Dave.
Powered by blists - more mailing lists