Re: [PATCH v6 5/5] drm/nouveau/kms/nv50-: Enable atomic modesetting by default

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch completely fails to enable atomic modesetting by default on nv50+ hardware and introduces a spurious warning on every boot for legacy hardware.
--

--- Patch [5]: [PATCH v6 5/5] drm/nouveau/kms/nv50-: Enable atomic modesetting by default ---
commit 72a6dbf310da7445e59ffec7fdf8ec40a1396882
Author: Lyude Paul <[email protected]>
drm/nouveau/kms/nv50-: Enable atomic modesetting by default

This commit aims to enable atomic modesetting by default on nv50+ hardware
while retaining a module parameter to turn it off if regressions occur.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 82a8061e6d3e2..8ee36c5d79fdb 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -745,7 +745,7 @@ nouveau_drm_device_new(struct device *parent, struct nvkm_device *device)
>  		else
>  			driver = &driver_legacy_kms;
>  	} else {
> -		if (nouveau_atomic)
> +		if (nouveau_atomic != -1)

[Severity: Medium]
Does this inadvertently leave atomic modesetting disabled by default on
nv50+ hardware, and introduce a regression where spurious warnings are
printed on older hardware?

Looking at nouveau_drm_device_new(), the check above this else block for nv50+
hardware remains unmodified:

	if (device->card_type >= NV_50) {
		if (nouveau_atomic)
			driver = &driver_atomic_kms;
		else
			driver = &driver_legacy_kms;
	}

Since the module parameter nouveau_atomic remains initialized to 0, atomic
modesetting remains disabled by default on nv50+, which seems to defeat the
purpose described in the commit message.

Additionally, for pre-nv50 hardware, the modified condition 
(nouveau_atomic != -1) now evaluates to true by default (0 != -1), which will
cause this dev_warn() to print spuriously on every boot.

>  			dev_warn(parent, "Atomic modesetting not supported (needs nv50+)\n");
>  
>  		driver = &driver_legacy_kms;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5