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
