[PATCH] drm/nouveau/disp: Set drm_mode_config_funcs.atomic_(check|commit) (attempt 2)

[PATCH] drm/nouveau/disp: Set drm_mode_config_funcs.atomic_(check|commit) (attempt 2)

[Dave Airlie via B4 Relay]

From: Dave Airlie <airlied@redhat.com>

commit 604826acb3f5 attempted to add these flags but
commit 6c65db809796 reverted it due to it
failing on hw from pre-atomic times.

So use a second set of functions.

original commit msg from Lyude:
"Apparently we never actually filled these in, despite the fact that we do
in fact technically support atomic modesetting.

Since not having these filled in causes us to potentially forget to disable
fbdev and friends during suspend/resume, let's fix it."

Reported-by: Ruben Wauters <rubenru09@aol.com>
Closes: https://gitlab.freedesktop.org/drm/nouveau/-/work_items/517
Fixes: 6c65db809796 ("Revert "drm/nouveau/disp: Set drm_mode_config_funcs.atomic_(check|commit)")
Cc: stable@vger.kernel.org
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_display.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index d71dcfc6ee66..07882624a7f6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -354,6 +354,11 @@ static const struct drm_mode_config_funcs nouveau_mode_config_funcs = {
 	.fb_create = nouveau_user_framebuffer_create,
 };
 
+static const struct drm_mode_config_funcs nouveau_mode_config_funcs_atomic = {
+	.fb_create = nouveau_user_framebuffer_create,
+	.atomic_commit = drm_atomic_helper_commit,
+	.atomic_check = drm_atomic_helper_check,
+};
 
 struct nouveau_drm_prop_enum_list {
 	u8 gen_mask;
@@ -654,7 +659,11 @@ nouveau_display_create(struct drm_device *dev)
 	drm_mode_create_scaling_mode_property(dev);
 	drm_mode_create_dvi_i_properties(dev);
 
-	dev->mode_config.funcs = &nouveau_mode_config_funcs;
+	/* this only gets set if the driver has the ATOMIC flag */
+	if (drm_drv_uses_atomic_modeset(dev))
+		dev->mode_config.funcs = &nouveau_mode_config_funcs_atomic;
+	else
+		dev->mode_config.funcs = &nouveau_mode_config_funcs;
 
 	dev->mode_config.min_width = 0;
 	dev->mode_config.min_height = 0;

---
base-commit: 067c5c24cef23d913b67eaaf71ae67e7169a6620
change-id: 20261111-nouveau-atomic-fix-7478260ce6a7

Best regards,
-- 
Dave Airlie <airlied@redhat.com>

Re: [PATCH] drm/nouveau/disp: Set drm_mode_config_funcs.atomic_(check|commit) (attempt 2)

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] drm/nouveau/disp: missing closing quote in Fixes: trailer
  subject
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [High] drm/nouveau: wrong dtor order and disp leak in
  nouveau_display_create() [1]
--

commit cf4b32d591c8a8a0033562aad54661c60e5b35bf
Author: Dave Airlie <airlied@redhat.com>

> drm/nouveau/disp: Set drm_mode_config_funcs.atomic_(check|commit) (attempt 2)
> 
> commit 604826acb3f5 attempted to add these flags but
> commit 6c65db809796 reverted it due to it
> failing on hw from pre-atomic times.
> 
> So use a second set of functions.
> 
> original commit msg from Lyude:
> "Apparently we never actually filled these in, despite the fact that we do
> in fact technically support atomic modesetting.
> 
> Since not having these filled in causes us to potentially forget to disable
> fbdev and friends during suspend/resume, let's fix it."
> 
> Reported-by: Ruben Wauters <rubenru09@aol.com>
> Closes: https://gitlab.freedesktop.org/drm/nouveau/-/work_items/517
> Fixes: 6c65db809796 ("Revert "drm/nouveau/disp: Set drm_mode_config_funcs.atomic_(check|commit)")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dave Airlie <airlied@redhat.com>

[Severity: Low]
The Fixes: trailer is missing the closing quote from the reverted commit's
subject:

  Fixes: 6c65db809796 ("Revert "drm/nouveau/disp: Set drm_mode_config_funcs.atomic_(check|commit)"")

> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index d71dcfc6ee66b..07882624a7f6f 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
[ ... ]
> @@ -654,7 +659,11 @@ nouveau_display_create(struct drm_device *dev)
>  	drm_mode_create_scaling_mode_property(dev);
>  	drm_mode_create_dvi_i_properties(dev);
>  
> -	dev->mode_config.funcs = &nouveau_mode_config_funcs;
> +	/* this only gets set if the driver has the ATOMIC flag */
> +	if (drm_drv_uses_atomic_modeset(dev))
> +		dev->mode_config.funcs = &nouveau_mode_config_funcs_atomic;
> +	else
> +		dev->mode_config.funcs = &nouveau_mode_config_funcs;
>  
>  	dev->mode_config.min_width = 0;
>  	dev->mode_config.min_height = 0;

[Severity: High]
This problem wasn't introduced by this patch, but can the error unwind paths
in nouveau_display_create() cause a NULL pointer dereference and leak
disp [1]?

drivers/gpu/drm/nouveau/nouveau_display.c:nouveau_display_create() {
	...
vblank_err:
	disp->dtor(dev);
disp_create_err:
	drm_kms_helper_poll_fini(dev);
	drm_mode_config_cleanup(dev);
	return ret;
}

When drm_vblank_init() fails, vblank_err calls disp->dtor(dev) before
drm_mode_config_cleanup(dev), which is the opposite of the order in
nouveau_display_destroy().  On pre-NV50 hardware, nv04_display_destroy()
clears nouveau_display(dev)->priv to NULL and frees struct nv04_display; when
drm_mode_config_cleanup() then runs nv_crtc_destroy(), nv04_display(crtc->dev)
returns NULL and dereferencing disp->image[nv_crtc->index] crashes.

In addition, neither error path calls nvif_disp_dtor(&disp->disp) or
kfree(disp), and nouveau_drm_device_init() skips nouveau_display_destroy() on
failure, leaking both objects and leaving drm->display dangling.

[1] https://sashiko.dev/#/bug/linux-dd1d9372-c030-413b-9ce8-94b2029fab64

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-nouveau-atomic-fix-v1-1-118e65d3a2f5@redhat.com?part=1

Re: [PATCH] drm/nouveau/disp: Set drm_mode_config_funcs.atomic_(check|commit) (attempt 2)

[lyude]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Wed, 2026-11-11 at 16:00 +1000, Dave Airlie via B4 Relay wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> commit 604826acb3f5 attempted to add these flags but
> commit 6c65db809796 reverted it due to it
> failing on hw from pre-atomic times.
> 
> So use a second set of functions.
> 
> original commit msg from Lyude:
> "Apparently we never actually filled these in, despite the fact that
> we do
> in fact technically support atomic modesetting.
> 
> Since not having these filled in causes us to potentially forget to
> disable
> fbdev and friends during suspend/resume, let's fix it."
> 
> Reported-by: Ruben Wauters <rubenru09@aol.com>
> Closes: https://gitlab.freedesktop.org/drm/nouveau/-/work_items/517
> Fixes: 6c65db809796 ("Revert "drm/nouveau/disp: Set
> drm_mode_config_funcs.atomic_(check|commit)")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_display.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
> b/drivers/gpu/drm/nouveau/nouveau_display.c
> index d71dcfc6ee66..07882624a7f6 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -354,6 +354,11 @@ static const struct drm_mode_config_funcs
> nouveau_mode_config_funcs = {
>  	.fb_create = nouveau_user_framebuffer_create,
>  };
>  
> +static const struct drm_mode_config_funcs
> nouveau_mode_config_funcs_atomic = {
> +	.fb_create = nouveau_user_framebuffer_create,
> +	.atomic_commit = drm_atomic_helper_commit,
> +	.atomic_check = drm_atomic_helper_check,
> +};
>  
>  struct nouveau_drm_prop_enum_list {
>  	u8 gen_mask;
> @@ -654,7 +659,11 @@ nouveau_display_create(struct drm_device *dev)
>  	drm_mode_create_scaling_mode_property(dev);
>  	drm_mode_create_dvi_i_properties(dev);
>  
> -	dev->mode_config.funcs = &nouveau_mode_config_funcs;
> +	/* this only gets set if the driver has the ATOMIC flag */
> +	if (drm_drv_uses_atomic_modeset(dev))
> +		dev->mode_config.funcs =
> &nouveau_mode_config_funcs_atomic;
> +	else
> +		dev->mode_config.funcs = &nouveau_mode_config_funcs;
>  
>  	dev->mode_config.min_width = 0;
>  	dev->mode_config.min_height = 0;
> 
> ---
> base-commit: 067c5c24cef23d913b67eaaf71ae67e7169a6620
> change-id: 20261111-nouveau-atomic-fix-7478260ce6a7
> 
> Best regards,