[PATCH v2 0/3] drm/nouveau: Fix WARN_ON() from missing blend modes

[PATCH v2 0/3] drm/nouveau: Fix WARN_ON() from missing blend modes

[Lyude Paul]

Recently a change went into the kernel to ensure that drivers which
expose color formats which have alpha channels. This would cause nouveau
to hit a WARN_ON when setting up cursors, since they do support blending
- but we never bothered trying to hook up the controls for it to
nouveau.

This fixes the issue by ensuring that nouveau always exposes a blend
mode for any planes with alpha channels, regardless of whether or not we
have control of the blend mode hooked up. If someone wants to add
control for this in the future as well, it should be fairly easy to hook
up with this.

Previous version:
	https://patchwork.freedesktop.org/patch/741031/

Lyude Paul (3):
  drm/nouveau/kms/nv50-: Add .blend_modes to nv50_wndw_func
  drm/nouveau/kms/nv50-: Introduce nv50_wndw_default_state()
  drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for
    wndws

 drivers/gpu/drm/nouveau/dispnv50/curs507a.c |  3 ++
 drivers/gpu/drm/nouveau/dispnv50/disp.c     |  4 +++
 drivers/gpu/drm/nouveau/dispnv50/wndw.c     | 40 +++++++++++++++++----
 drivers/gpu/drm/nouveau/dispnv50/wndw.h     | 11 ++++++
 drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  1 +
 drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c |  1 +
 drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c |  1 +
 drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c |  1 +
 8 files changed, 55 insertions(+), 7 deletions(-)


base-commit: b0a652436b892eb9a036a031b33099dca036faaa
-- 
2.55.0

[PATCH v2 0/3] drm/nouveau: Fix WARN_ON() from missing blend modes

[Lyude Paul]

Recently a change went into the kernel to ensure that drivers which
expose color formats which have alpha channels. This would cause nouveau
to hit a WARN_ON when setting up cursors, since they do support blending
- but we never bothered trying to hook up the controls for it to
nouveau.

This fixes the issue by ensuring that nouveau always exposes a blend
mode for any planes with alpha channels, regardless of whether or not we
have control of the blend mode hooked up. If someone wants to add
control for this in the future as well, it should be fairly easy to hook
up with this.

Previous version:
	https://patchwork.freedesktop.org/patch/741031/

Lyude Paul (3):
  drm/nouveau/kms/nv50-: Add .blend_modes to nv50_wndw_func
  drm/nouveau/kms/nv50-: Introduce nv50_wndw_default_state()
  drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for
    wndws

 drivers/gpu/drm/nouveau/dispnv50/curs507a.c |  3 ++
 drivers/gpu/drm/nouveau/dispnv50/disp.c     |  4 +++
 drivers/gpu/drm/nouveau/dispnv50/wndw.c     | 40 +++++++++++++++++----
 drivers/gpu/drm/nouveau/dispnv50/wndw.h     | 11 ++++++
 drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  1 +
 drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c |  1 +
 drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c |  1 +
 drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c |  1 +
 8 files changed, 55 insertions(+), 7 deletions(-)


base-commit: b0a652436b892eb9a036a031b33099dca036faaa
-- 
2.55.0

[PATCH v2 1/3] drm/nouveau/kms/nv50-: Add .blend_modes to nv50_wndw_func

[Lyude Paul]

At the moment, we currently (incorrectly) only expose a blend property on
the plane if we have a nv50_wndw_func->blend_set function for the given
wndw type. But we need to expose a blend property for all types of planes
that can display color formats with an alpha channel.

Prepare for doing this by introducing a way of looking up the supported
blend modes for a type of wndw by introducing .blend_modes, which describes
all possible blending modes nouveau currently supports for a type of wndw.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 860e748bddcc ("drm: ensure blend mode supported if pixel format with alpha exposed")
---
 drivers/gpu/drm/nouveau/dispnv50/curs507a.c |  3 +++
 drivers/gpu/drm/nouveau/dispnv50/wndw.h     | 10 ++++++++++
 drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  1 +
 drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c |  1 +
 drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c |  1 +
 drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c |  1 +
 6 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
index 1a889139cb053..5f2ff40388e7f 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
@@ -29,6 +29,7 @@
 #include <nvhw/class/cl507a.h>
 
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_blend.h>
 #include <drm/drm_fourcc.h>
 
 bool
@@ -164,6 +165,8 @@ curs507a_wndw = {
 	.acquire = curs507a_acquire,
 	.release = curs507a_release,
 	.prepare = curs507a_prepare,
+	// TODO: Cursors also support premulti, but we haven't hooked it up anywhere yet.
+	.blend_modes = BIT(DRM_MODE_BLEND_COVERAGE),
 };
 
 int
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.h b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
index 90d100514bef2..81af5c3369d4c 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
@@ -1,6 +1,8 @@
 #ifndef __NV50_KMS_WNDW_H__
 #define __NV50_KMS_WNDW_H__
 #define nv50_wndw(p) container_of((p), struct nv50_wndw, plane)
+#include <drm/drm_blend.h>
+
 #include "disp.h"
 #include "atom.h"
 #include "lut.h"
@@ -74,6 +76,7 @@ struct nv50_wndw_func {
 	int (*image_clr)(struct nv50_wndw *);
 	int (*scale_set)(struct nv50_wndw *, struct nv50_wndw_atom *);
 	int (*blend_set)(struct nv50_wndw *, struct nv50_wndw_atom *);
+	unsigned int blend_modes;
 
 	int (*update)(struct nv50_wndw *, u32 *interlock);
 };
@@ -139,4 +142,11 @@ int wndwca7e_new(struct nouveau_drm *, enum drm_plane_type, int, s32,
 
 int nv50_wndw_new(struct nouveau_drm *, enum drm_plane_type, int index,
 		  struct nv50_wndw **);
+
+/* A set of blend modes supported by all wndws */
+#define WNDW_BLEND_MODES               \
+	  BIT(DRM_MODE_BLEND_COVERAGE) \
+	| BIT(DRM_MODE_BLEND_PREMULTI) \
+	| BIT(DRM_MODE_BLEND_PIXEL_NONE)
+
 #endif
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c b/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
index 554c4f91f8bec..7ab4d532118ee 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
@@ -343,6 +343,7 @@ wndwc37e = {
 	.image_set = wndwc37e_image_set,
 	.image_clr = wndwc37e_image_clr,
 	.blend_set = wndwc37e_blend_set,
+	.blend_modes = WNDW_BLEND_MODES,
 	.update = wndwc37e_update,
 };
 
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c b/drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c
index d1ca51aae58cd..e220f43b5bca7 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c
@@ -231,6 +231,7 @@ wndwc57e = {
 	.image_set = wndwc57e_image_set,
 	.image_clr = wndwc37e_image_clr,
 	.blend_set = wndwc37e_blend_set,
+	.blend_modes = WNDW_BLEND_MODES,
 	.update = wndwc37e_update,
 };
 
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c b/drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c
index 52af293c98f4a..580cf7abd8193 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c
@@ -95,6 +95,7 @@ wndwc67e = {
 	.image_set = wndwc67e_image_set,
 	.image_clr = wndwc37e_image_clr,
 	.blend_set = wndwc37e_blend_set,
+	.blend_modes = WNDW_BLEND_MODES,
 	.update = wndwc37e_update,
 };
 
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c b/drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c
index 2cec8cfbd5461..d65ec438b434f 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c
@@ -231,6 +231,7 @@ wndwca7e = {
 	.image_set = wndwca7e_image_set,
 	.image_clr = wndwca7e_image_clr,
 	.blend_set = wndwc37e_blend_set,
+	.blend_modes = WNDW_BLEND_MODES,
 	.update = wndwc37e_update,
 };
 
-- 
2.55.0

[PATCH v2 1/3] drm/nouveau/kms/nv50-: Add .blend_modes to nv50_wndw_func

[Lyude Paul]

At the moment, we currently (incorrectly) only expose a blend property on
the plane if we have a nv50_wndw_func->blend_set function for the given
wndw type. But we need to expose a blend property for all types of planes
that can display color formats with an alpha channel.

Prepare for doing this by introducing a way of looking up the supported
blend modes for a type of wndw by introducing .blend_modes, which describes
all possible blending modes nouveau currently supports for a type of wndw.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 860e748bddcc ("drm: ensure blend mode supported if pixel format with alpha exposed")
---
 drivers/gpu/drm/nouveau/dispnv50/curs507a.c |  3 +++
 drivers/gpu/drm/nouveau/dispnv50/wndw.h     | 10 ++++++++++
 drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  1 +
 drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c |  1 +
 drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c |  1 +
 drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c |  1 +
 6 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
index 1a889139cb053..5f2ff40388e7f 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
@@ -29,6 +29,7 @@
 #include <nvhw/class/cl507a.h>
 
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_blend.h>
 #include <drm/drm_fourcc.h>
 
 bool
@@ -164,6 +165,8 @@ curs507a_wndw = {
 	.acquire = curs507a_acquire,
 	.release = curs507a_release,
 	.prepare = curs507a_prepare,
+	// TODO: Cursors also support premulti, but we haven't hooked it up anywhere yet.
+	.blend_modes = BIT(DRM_MODE_BLEND_COVERAGE),
 };
 
 int
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.h b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
index 90d100514bef2..81af5c3369d4c 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
@@ -1,6 +1,8 @@
 #ifndef __NV50_KMS_WNDW_H__
 #define __NV50_KMS_WNDW_H__
 #define nv50_wndw(p) container_of((p), struct nv50_wndw, plane)
+#include <drm/drm_blend.h>
+
 #include "disp.h"
 #include "atom.h"
 #include "lut.h"
@@ -74,6 +76,7 @@ struct nv50_wndw_func {
 	int (*image_clr)(struct nv50_wndw *);
 	int (*scale_set)(struct nv50_wndw *, struct nv50_wndw_atom *);
 	int (*blend_set)(struct nv50_wndw *, struct nv50_wndw_atom *);
+	unsigned int blend_modes;
 
 	int (*update)(struct nv50_wndw *, u32 *interlock);
 };
@@ -139,4 +142,11 @@ int wndwca7e_new(struct nouveau_drm *, enum drm_plane_type, int, s32,
 
 int nv50_wndw_new(struct nouveau_drm *, enum drm_plane_type, int index,
 		  struct nv50_wndw **);
+
+/* A set of blend modes supported by all wndws */
+#define WNDW_BLEND_MODES               \
+	  BIT(DRM_MODE_BLEND_COVERAGE) \
+	| BIT(DRM_MODE_BLEND_PREMULTI) \
+	| BIT(DRM_MODE_BLEND_PIXEL_NONE)
+
 #endif
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c b/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
index 554c4f91f8bec..7ab4d532118ee 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
@@ -343,6 +343,7 @@ wndwc37e = {
 	.image_set = wndwc37e_image_set,
 	.image_clr = wndwc37e_image_clr,
 	.blend_set = wndwc37e_blend_set,
+	.blend_modes = WNDW_BLEND_MODES,
 	.update = wndwc37e_update,
 };
 
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c b/drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c
index d1ca51aae58cd..e220f43b5bca7 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c
@@ -231,6 +231,7 @@ wndwc57e = {
 	.image_set = wndwc57e_image_set,
 	.image_clr = wndwc37e_image_clr,
 	.blend_set = wndwc37e_blend_set,
+	.blend_modes = WNDW_BLEND_MODES,
 	.update = wndwc37e_update,
 };
 
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c b/drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c
index 52af293c98f4a..580cf7abd8193 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c
@@ -95,6 +95,7 @@ wndwc67e = {
 	.image_set = wndwc67e_image_set,
 	.image_clr = wndwc37e_image_clr,
 	.blend_set = wndwc37e_blend_set,
+	.blend_modes = WNDW_BLEND_MODES,
 	.update = wndwc37e_update,
 };
 
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c b/drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c
index 2cec8cfbd5461..d65ec438b434f 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c
@@ -231,6 +231,7 @@ wndwca7e = {
 	.image_set = wndwca7e_image_set,
 	.image_clr = wndwca7e_image_clr,
 	.blend_set = wndwc37e_blend_set,
+	.blend_modes = WNDW_BLEND_MODES,
 	.update = wndwc37e_update,
 };
 
-- 
2.55.0

[PATCH v2 2/3] drm/nouveau/kms/nv50-: Introduce nv50_wndw_default_state()

[Lyude Paul]

While we don't currently read-in the hardware state of planes, now that
we're about to start exposing blend properties for all planes that can
support alpha channels: We need to make sure that the initial atomic state
for a wndw always starts off with a supported value in pixel_blend_mode.

The easiest way to do this is to introduce a nv50_wndw_default_state()
function, and use it in nv50_display_read_hw_state() - and use that
function to enforce a valid value for pixel_blend_mode during driver
startup.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 860e748bddcc ("drm: ensure blend mode supported if pixel format with alpha exposed")
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c |  4 ++++
 drivers/gpu/drm/nouveau/dispnv50/wndw.c | 22 ++++++++++++++++++++++
 drivers/gpu/drm/nouveau/dispnv50/wndw.h |  1 +
 3 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 364227f5456f1..2c66e480b5116 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2768,6 +2768,7 @@ nv50_display_read_hw_state(struct nouveau_drm *drm)
 {
 	struct drm_device *dev = drm->dev;
 	struct drm_encoder *encoder;
+	struct drm_plane *plane;
 	struct drm_modeset_acquire_ctx ctx;
 	struct nv50_disp *disp = nv50_disp(dev);
 	int ret;
@@ -2781,6 +2782,9 @@ nv50_display_read_hw_state(struct nouveau_drm *drm)
 		nv50_display_read_hw_or_state(dev, disp, nouveau_encoder(encoder));
 	}
 
+	drm_for_each_plane(plane, dev)
+		nv50_wndw_default_state(nv50_wndw(plane));
+
 	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 }
 
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 2635458d52acc..0f331f47bded8 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -848,6 +848,28 @@ static const u64 nv50_cursor_format_modifiers[] = {
 	DRM_FORMAT_MOD_INVALID,
 };
 
+/*
+ * Setup defaults for the atomic wndw state
+ */
+void
+nv50_wndw_default_state(struct nv50_wndw *wndw)
+{
+	struct nv50_wndw_atom *armw = nv50_wndw_atom(wndw->plane.state);
+	const unsigned int blend_modes = wndw->func->blend_modes;
+
+	drm_modeset_lock_assert_held(&wndw->plane.mutex);
+
+	/* Ensure the plane's atomic state didn't default to a pixel_blend_mode we don't support */
+	if (blend_modes && (!(armw->state.pixel_blend_mode & blend_modes))) {
+		if (blend_modes & BIT(DRM_MODE_BLEND_COVERAGE))
+			armw->state.pixel_blend_mode = DRM_MODE_BLEND_COVERAGE;
+		else if (blend_modes & BIT(DRM_MODE_BLEND_PREMULTI))
+			armw->state.pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
+		else if (blend_modes & BIT(DRM_MODE_BLEND_PIXEL_NONE))
+			armw->state.pixel_blend_mode = DRM_MODE_BLEND_PIXEL_NONE;
+	}
+}
+
 int
 nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev,
 	       enum drm_plane_type type, const char *name, int index,
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.h b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
index 81af5c3369d4c..13f06b7b67611 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
@@ -46,6 +46,7 @@ void nv50_wndw_flush_clr(struct nv50_wndw *, u32 *interlock, bool flush,
 			 struct nv50_wndw_atom *);
 void nv50_wndw_ntfy_enable(struct nv50_wndw *, struct nv50_wndw_atom *);
 int nv50_wndw_wait_armed(struct nv50_wndw *, struct nv50_wndw_atom *);
+void nv50_wndw_default_state(struct nv50_wndw *wndw);
 
 struct nv50_wndw_func {
 	int (*acquire)(struct nv50_wndw *, struct nv50_wndw_atom *asyw,
-- 
2.55.0

[PATCH v2 2/3] drm/nouveau/kms/nv50-: Introduce nv50_wndw_default_state()

[Lyude Paul]

While we don't currently read-in the hardware state of planes, now that
we're about to start exposing blend properties for all planes that can
support alpha channels: We need to make sure that the initial atomic state
for a wndw always starts off with a supported value in pixel_blend_mode.

The easiest way to do this is to introduce a nv50_wndw_default_state()
function, and use it in nv50_display_read_hw_state() - and use that
function to enforce a valid value for pixel_blend_mode during driver
startup.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 860e748bddcc ("drm: ensure blend mode supported if pixel format with alpha exposed")
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c |  4 ++++
 drivers/gpu/drm/nouveau/dispnv50/wndw.c | 22 ++++++++++++++++++++++
 drivers/gpu/drm/nouveau/dispnv50/wndw.h |  1 +
 3 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 364227f5456f1..2c66e480b5116 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2768,6 +2768,7 @@ nv50_display_read_hw_state(struct nouveau_drm *drm)
 {
 	struct drm_device *dev = drm->dev;
 	struct drm_encoder *encoder;
+	struct drm_plane *plane;
 	struct drm_modeset_acquire_ctx ctx;
 	struct nv50_disp *disp = nv50_disp(dev);
 	int ret;
@@ -2781,6 +2782,9 @@ nv50_display_read_hw_state(struct nouveau_drm *drm)
 		nv50_display_read_hw_or_state(dev, disp, nouveau_encoder(encoder));
 	}
 
+	drm_for_each_plane(plane, dev)
+		nv50_wndw_default_state(nv50_wndw(plane));
+
 	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
 }
 
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 2635458d52acc..0f331f47bded8 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -848,6 +848,28 @@ static const u64 nv50_cursor_format_modifiers[] = {
 	DRM_FORMAT_MOD_INVALID,
 };
 
+/*
+ * Setup defaults for the atomic wndw state
+ */
+void
+nv50_wndw_default_state(struct nv50_wndw *wndw)
+{
+	struct nv50_wndw_atom *armw = nv50_wndw_atom(wndw->plane.state);
+	const unsigned int blend_modes = wndw->func->blend_modes;
+
+	drm_modeset_lock_assert_held(&wndw->plane.mutex);
+
+	/* Ensure the plane's atomic state didn't default to a pixel_blend_mode we don't support */
+	if (blend_modes && (!(armw->state.pixel_blend_mode & blend_modes))) {
+		if (blend_modes & BIT(DRM_MODE_BLEND_COVERAGE))
+			armw->state.pixel_blend_mode = DRM_MODE_BLEND_COVERAGE;
+		else if (blend_modes & BIT(DRM_MODE_BLEND_PREMULTI))
+			armw->state.pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
+		else if (blend_modes & BIT(DRM_MODE_BLEND_PIXEL_NONE))
+			armw->state.pixel_blend_mode = DRM_MODE_BLEND_PIXEL_NONE;
+	}
+}
+
 int
 nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev,
 	       enum drm_plane_type type, const char *name, int index,
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.h b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
index 81af5c3369d4c..13f06b7b67611 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
@@ -46,6 +46,7 @@ void nv50_wndw_flush_clr(struct nv50_wndw *, u32 *interlock, bool flush,
 			 struct nv50_wndw_atom *);
 void nv50_wndw_ntfy_enable(struct nv50_wndw *, struct nv50_wndw_atom *);
 int nv50_wndw_wait_armed(struct nv50_wndw *, struct nv50_wndw_atom *);
+void nv50_wndw_default_state(struct nv50_wndw *wndw);
 
 struct nv50_wndw_func {
 	int (*acquire)(struct nv50_wndw *, struct nv50_wndw_atom *asyw,
-- 
2.55.0

[PATCH v2 3/3] drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for wndws

[Lyude Paul]

Since the commit mentioned down below, it looks like we've been triggering
this WARN_ON():

  [   31.477009] ------------[ cut here ]------------
  [   31.477012] [PLANE:66:curs-0] pixel format with alpha exposed but
  blend mode not setup
  [   31.477014] WARNING: drivers/gpu/drm/drm_mode_config.c:872 at
  drm_mode_config_validate+0x38c/0x4c0, CPU#1: modprobe/1264
  [   31.477125] CPU: 1 UID: 0 PID: 1264 Comm: modprobe Not tainted
  7.2.0-rc2Lyude-Test+ #1 PREEMPT(lazy)
  [   31.477129] Hardware name: Micro-Star International Co., Ltd.
  MS-7A39/A320M GAMING PRO (MS-7A39), BIOS 1.M4 05/10/2023
  [   31.477132] RIP: 0010:drm_mode_config_validate+0x394/0x4c0
  [   31.477136] Code: 41 3b 9e 80 00 00 00 73 bd 49 8b 46 78 8b 3c 98 e8
  c1 3d ff ff 80 78 14 00 74 e1 48 8d 3d 64 e6 e7 01 49 8b 56 18 41 8b 76
  50 <67> 48 0f b9 3a 49 8b 46 08 4c 8d 70 f8 49 39 c5 75 97 8b 95 b8 02
  [   31.477139] RSP: 0018:ffffd48e053bb7f8 EFLAGS: 00010202
  [   31.477143] RAX: ffffffffaf1da9c0 RBX: 0000000000000000 RCX: 0000000000000034
  [   31.477145] RDX: ffff8cd883c1feb0 RSI: 0000000000000042 RDI: ffffffffb069f840
  [   31.477148] RBP: ffff8cd88ea3d800 R08: 0000000034325241 R09: ffff8cd88ea3dac0
  [   31.477150] R10: fffffaf5c4043600 R11: ffff8cd88004f600 R12: 0000000000000001
  [   31.477152] R13: ffff8cd88ea3da90 R14: ffff8cd88b9ec038 R15: ffff8cd88ea3dab0
  [   31.477154] FS:  00007fb934898780(0000) GS:ffff8cd9f3a6a000(0000) knlGS:0000000000000000
  [   31.477157] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  [   31.477160] CR2: 0000555efff79520 CR3: 0000000122cd5000 CR4: 00000000003506f0
  [   31.477162] Call Trace:
  [   31.477165]  <TASK>
  [   31.477168]  ? nouveau_hwmon_init+0x2d6/0x330 [nouveau]
  [   31.477346]  drm_dev_register+0x19d/0x270
  [   31.477352]  nouveau_drm_device_init+0x17a/0x230 [nouveau]
  [   31.477523]  nouveau_drm_probe+0x13d/0x1e0 [nouveau]
  [   31.477690]  local_pci_probe+0x3e/0x90
  [   31.477697]  pci_call_probe+0x59/0x190
  [   31.477702]  ? srso_return_thunk+0x5/0x5f
  [   31.477706]  ? pci_match_device+0x15a/0x180
  [   31.477710]  ? pci_assign_irq+0x2d/0x160
  [   31.477714]  pci_device_probe+0x95/0x160
  [   31.477719]  call_driver_probe+0x26/0x100
  [   31.477724]  ? driver_sysfs_add+0x59/0xd0
  [   31.477738]  really_probe+0xcc/0x2c0
  [   31.477743]  ? srso_return_thunk+0x5/0x5f
  [   31.477749]  __driver_probe_device+0x85/0x170
  [   31.477754]  driver_probe_device+0x1f/0x90
  [   31.477760]  ? __pfx___driver_attach+0x10/0x10
  [   31.477766]  __driver_attach+0xcb/0x210
  [   31.477772]  bus_for_each_dev+0x85/0xd0
  [   31.477779]  bus_add_driver+0x13e/0x220
  [   31.477786]  ? __pfx_init_module+0x10/0x10 [nouveau]
  [   31.477979]  driver_register+0x75/0xe0
  [   31.477984]  ? srso_return_thunk+0x5/0x5f
  [   31.477990]  nouveau_drm_init+0x8b/0xff0 [nouveau]
  [   31.478192]  do_one_initcall+0x5d/0x300
  [   31.478204]  do_init_module+0x86/0x290
  [   31.478212]  init_module_from_file+0xd8/0x140
  [   31.478223]  idempotent_init_module+0x114/0x310
  [   31.478233]  __x64_sys_finit_module+0x71/0xe0
  [   31.478238]  do_syscall_64+0xe2/0x630
  [   31.478252]  ? srso_return_thunk+0x5/0x5f
  [   31.478257]  ? vfs_read+0x165/0x390
  [   31.478267]  ? srso_return_thunk+0x5/0x5f
  [   31.478271]  ? srso_return_thunk+0x5/0x5f
  [   31.478276]  ? switch_fpu_return+0x52/0xe0
  [   31.478282]  ? srso_return_thunk+0x5/0x5f
  [   31.478286]  ? arch_exit_to_user_mode_prepare.isra.0+0x7e/0x90
  [   31.478292]  ? srso_return_thunk+0x5/0x5f
  [   31.478296]  ? do_syscall_64+0x11f/0x630
  [   31.478301]  ? srso_return_thunk+0x5/0x5f
  [   31.478305]  ? arch_exit_to_user_mode_prepare.isra.0+0x7e/0x90
  [   31.478310]  ? srso_return_thunk+0x5/0x5f
  [   31.478314]  ? do_syscall_64+0x11f/0x630
  [   31.478320]  ? srso_return_thunk+0x5/0x5f
  [   31.478324]  ? arch_exit_to_user_mode_prepare.isra.0+0x7e/0x90
  [   31.478329]  ? srso_return_thunk+0x5/0x5f
  [   31.478334]  ? srso_return_thunk+0x5/0x5f
  [   31.478338]  ? do_syscall_64+0x99/0x630
  [   31.478343]  ? exc_page_fault+0x82/0x1c0
  [   31.478350]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
  [   31.478355] RIP: 0033:0x7fb93410030d
  [   31.478372] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa
  48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f
  05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 5a 0f 00 f7 d8 64 89 01 48
  [   31.478375] RSP: 002b:00007ffc587ce738 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
  [   31.478381] RAX: ffffffffffffffda RBX: 0000562f5fb3da60 RCX: 00007fb93410030d
  [   31.478385] RDX: 0000000000000000 RSI: 0000562f5fb40590 RDI: 000000000000000d
  [   31.478388] RBP: 00007ffc587ce7d0 R08: 0000000000000000 R09: 0000000000000000
  [   31.478391] R10: 0000000000000000 R11: 0000000000000246 R12: 0000562f5fb40590
  [   31.478394] R13: 0000000000040000 R14: 0000562f5fb3db80 R15: 0000000000000009
  [   31.478404]  </TASK>
  [   31.478407] ---[ end trace 0000000000000000 ]---

Fix it by making sure we unconditionally call
drm_plane_create_blend_mode_property for all types of wndws, including
those (such as cursors) without a blend_set function.

Fixes: 860e748bddcc ("drm: ensure blend mode supported if pixel format with alpha exposed")
Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V2:
* Address Sashiko issue regarding pixel_blend_mode defaults
* The cursor uses DRM_MODE_BLEND_COVERAGE (I think), not
  DRM_MODE_BLEND_PIXEL_NONE
* Start using the new blend_modes vtable entry

 drivers/gpu/drm/nouveau/dispnv50/wndw.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 0f331f47bded8..84b9a16bdb8ab 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -930,13 +930,6 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev,
 		ret = drm_plane_create_alpha_property(&wndw->plane);
 		if (ret)
 			return ret;
-
-		ret = drm_plane_create_blend_mode_property(&wndw->plane,
-				BIT(DRM_MODE_BLEND_PIXEL_NONE) |
-				BIT(DRM_MODE_BLEND_PREMULTI) |
-				BIT(DRM_MODE_BLEND_COVERAGE));
-		if (ret)
-			return ret;
 	} else {
 		ret = drm_plane_create_zpos_immutable_property(&wndw->plane,
 				nv50_wndw_zpos_default(&wndw->plane));
@@ -944,6 +937,17 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev,
 			return ret;
 	}
 
+	/*
+	 * DRM requires that we have a blend mode property for any type of plane that exposes color
+	 * formats with an alpha channel. So do this, even if we don't actually have control for the
+	 * blend property hooked up with blend_set.
+	 */
+	if (func->blend_modes) {
+		ret = drm_plane_create_blend_mode_property(&wndw->plane, func->blend_modes);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
-- 
2.55.0

[PATCH v2 3/3] drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for wndws

[Lyude Paul]

Since the commit mentioned down below, it looks like we've been triggering
this WARN_ON():

  [   31.477009] ------------[ cut here ]------------
  [   31.477012] [PLANE:66:curs-0] pixel format with alpha exposed but
  blend mode not setup
  [   31.477014] WARNING: drivers/gpu/drm/drm_mode_config.c:872 at
  drm_mode_config_validate+0x38c/0x4c0, CPU#1: modprobe/1264
  [   31.477125] CPU: 1 UID: 0 PID: 1264 Comm: modprobe Not tainted
  7.2.0-rc2Lyude-Test+ #1 PREEMPT(lazy)
  [   31.477129] Hardware name: Micro-Star International Co., Ltd.
  MS-7A39/A320M GAMING PRO (MS-7A39), BIOS 1.M4 05/10/2023
  [   31.477132] RIP: 0010:drm_mode_config_validate+0x394/0x4c0
  [   31.477136] Code: 41 3b 9e 80 00 00 00 73 bd 49 8b 46 78 8b 3c 98 e8
  c1 3d ff ff 80 78 14 00 74 e1 48 8d 3d 64 e6 e7 01 49 8b 56 18 41 8b 76
  50 <67> 48 0f b9 3a 49 8b 46 08 4c 8d 70 f8 49 39 c5 75 97 8b 95 b8 02
  [   31.477139] RSP: 0018:ffffd48e053bb7f8 EFLAGS: 00010202
  [   31.477143] RAX: ffffffffaf1da9c0 RBX: 0000000000000000 RCX: 0000000000000034
  [   31.477145] RDX: ffff8cd883c1feb0 RSI: 0000000000000042 RDI: ffffffffb069f840
  [   31.477148] RBP: ffff8cd88ea3d800 R08: 0000000034325241 R09: ffff8cd88ea3dac0
  [   31.477150] R10: fffffaf5c4043600 R11: ffff8cd88004f600 R12: 0000000000000001
  [   31.477152] R13: ffff8cd88ea3da90 R14: ffff8cd88b9ec038 R15: ffff8cd88ea3dab0
  [   31.477154] FS:  00007fb934898780(0000) GS:ffff8cd9f3a6a000(0000) knlGS:0000000000000000
  [   31.477157] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  [   31.477160] CR2: 0000555efff79520 CR3: 0000000122cd5000 CR4: 00000000003506f0
  [   31.477162] Call Trace:
  [   31.477165]  <TASK>
  [   31.477168]  ? nouveau_hwmon_init+0x2d6/0x330 [nouveau]
  [   31.477346]  drm_dev_register+0x19d/0x270
  [   31.477352]  nouveau_drm_device_init+0x17a/0x230 [nouveau]
  [   31.477523]  nouveau_drm_probe+0x13d/0x1e0 [nouveau]
  [   31.477690]  local_pci_probe+0x3e/0x90
  [   31.477697]  pci_call_probe+0x59/0x190
  [   31.477702]  ? srso_return_thunk+0x5/0x5f
  [   31.477706]  ? pci_match_device+0x15a/0x180
  [   31.477710]  ? pci_assign_irq+0x2d/0x160
  [   31.477714]  pci_device_probe+0x95/0x160
  [   31.477719]  call_driver_probe+0x26/0x100
  [   31.477724]  ? driver_sysfs_add+0x59/0xd0
  [   31.477738]  really_probe+0xcc/0x2c0
  [   31.477743]  ? srso_return_thunk+0x5/0x5f
  [   31.477749]  __driver_probe_device+0x85/0x170
  [   31.477754]  driver_probe_device+0x1f/0x90
  [   31.477760]  ? __pfx___driver_attach+0x10/0x10
  [   31.477766]  __driver_attach+0xcb/0x210
  [   31.477772]  bus_for_each_dev+0x85/0xd0
  [   31.477779]  bus_add_driver+0x13e/0x220
  [   31.477786]  ? __pfx_init_module+0x10/0x10 [nouveau]
  [   31.477979]  driver_register+0x75/0xe0
  [   31.477984]  ? srso_return_thunk+0x5/0x5f
  [   31.477990]  nouveau_drm_init+0x8b/0xff0 [nouveau]
  [   31.478192]  do_one_initcall+0x5d/0x300
  [   31.478204]  do_init_module+0x86/0x290
  [   31.478212]  init_module_from_file+0xd8/0x140
  [   31.478223]  idempotent_init_module+0x114/0x310
  [   31.478233]  __x64_sys_finit_module+0x71/0xe0
  [   31.478238]  do_syscall_64+0xe2/0x630
  [   31.478252]  ? srso_return_thunk+0x5/0x5f
  [   31.478257]  ? vfs_read+0x165/0x390
  [   31.478267]  ? srso_return_thunk+0x5/0x5f
  [   31.478271]  ? srso_return_thunk+0x5/0x5f
  [   31.478276]  ? switch_fpu_return+0x52/0xe0
  [   31.478282]  ? srso_return_thunk+0x5/0x5f
  [   31.478286]  ? arch_exit_to_user_mode_prepare.isra.0+0x7e/0x90
  [   31.478292]  ? srso_return_thunk+0x5/0x5f
  [   31.478296]  ? do_syscall_64+0x11f/0x630
  [   31.478301]  ? srso_return_thunk+0x5/0x5f
  [   31.478305]  ? arch_exit_to_user_mode_prepare.isra.0+0x7e/0x90
  [   31.478310]  ? srso_return_thunk+0x5/0x5f
  [   31.478314]  ? do_syscall_64+0x11f/0x630
  [   31.478320]  ? srso_return_thunk+0x5/0x5f
  [   31.478324]  ? arch_exit_to_user_mode_prepare.isra.0+0x7e/0x90
  [   31.478329]  ? srso_return_thunk+0x5/0x5f
  [   31.478334]  ? srso_return_thunk+0x5/0x5f
  [   31.478338]  ? do_syscall_64+0x99/0x630
  [   31.478343]  ? exc_page_fault+0x82/0x1c0
  [   31.478350]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
  [   31.478355] RIP: 0033:0x7fb93410030d
  [   31.478372] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa
  48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f
  05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c3 5a 0f 00 f7 d8 64 89 01 48
  [   31.478375] RSP: 002b:00007ffc587ce738 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
  [   31.478381] RAX: ffffffffffffffda RBX: 0000562f5fb3da60 RCX: 00007fb93410030d
  [   31.478385] RDX: 0000000000000000 RSI: 0000562f5fb40590 RDI: 000000000000000d
  [   31.478388] RBP: 00007ffc587ce7d0 R08: 0000000000000000 R09: 0000000000000000
  [   31.478391] R10: 0000000000000000 R11: 0000000000000246 R12: 0000562f5fb40590
  [   31.478394] R13: 0000000000040000 R14: 0000562f5fb3db80 R15: 0000000000000009
  [   31.478404]  </TASK>
  [   31.478407] ---[ end trace 0000000000000000 ]---

Fix it by making sure we unconditionally call
drm_plane_create_blend_mode_property for all types of wndws, including
those (such as cursors) without a blend_set function.

Fixes: 860e748bddcc ("drm: ensure blend mode supported if pixel format with alpha exposed")
Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V2:
* Address Sashiko issue regarding pixel_blend_mode defaults
* The cursor uses DRM_MODE_BLEND_COVERAGE (I think), not
  DRM_MODE_BLEND_PIXEL_NONE
* Start using the new blend_modes vtable entry

 drivers/gpu/drm/nouveau/dispnv50/wndw.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 0f331f47bded8..84b9a16bdb8ab 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -930,13 +930,6 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev,
 		ret = drm_plane_create_alpha_property(&wndw->plane);
 		if (ret)
 			return ret;
-
-		ret = drm_plane_create_blend_mode_property(&wndw->plane,
-				BIT(DRM_MODE_BLEND_PIXEL_NONE) |
-				BIT(DRM_MODE_BLEND_PREMULTI) |
-				BIT(DRM_MODE_BLEND_COVERAGE));
-		if (ret)
-			return ret;
 	} else {
 		ret = drm_plane_create_zpos_immutable_property(&wndw->plane,
 				nv50_wndw_zpos_default(&wndw->plane));
@@ -944,6 +937,17 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev,
 			return ret;
 	}
 
+	/*
+	 * DRM requires that we have a blend mode property for any type of plane that exposes color
+	 * formats with an alpha channel. So do this, even if we don't actually have control for the
+	 * blend property hooked up with blend_set.
+	 */
+	if (func->blend_modes) {
+		ret = drm_plane_create_blend_mode_property(&wndw->plane, func->blend_modes);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
-- 
2.55.0

Re: [PATCH v2 1/3] drm/nouveau/kms/nv50-: Add .blend_modes to nv50_wndw_func

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Pre-Volta primary planes (base507c, base827c, base907c) expose alpha formats but are missing `.blend_modes` assignments, leaving the DRM core warning unfixed for these architectures.
--

--- Patch [1]: [PATCH v2 1/3] drm/nouveau/kms/nv50-: Add .blend_modes to nv50_wndw_func ---
commit 742edbbf0331352f33ff9e6b7929be21b191145a
Author: Lyude Paul <lyude@redhat.com>
Subject: drm/nouveau/kms/nv50-: Add .blend_modes to nv50_wndw_func

At the moment, we currently (incorrectly) only expose a blend property on
the plane if we have a nv50_wndw_func->blend_set function for the given
wndw type. But we need to expose a blend property for all types of planes
that can display color formats with an alpha channel.

Prepare for doing this by introducing a way of looking up the supported
blend modes for a type of wndw by introducing .blend_modes, which describes
all possible blending modes nouveau currently supports for a type of wndw.

> diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.h b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
> index 90d100514bef2..81af5c3369d4c 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.h
> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
[ ... ]
> @@ -74,6 +76,7 @@ struct nv50_wndw_func {
>  	int (*image_clr)(struct nv50_wndw *);
>  	int (*scale_set)(struct nv50_wndw *, struct nv50_wndw_atom *);
>  	int (*blend_set)(struct nv50_wndw *, struct nv50_wndw_atom *);
> +	unsigned int blend_modes;

[Severity: Medium]
Does this patch miss setting .blend_modes for pre-Volta primary planes?

Looking at base507c, base827c, and base907c in the driver, they expose alpha
formats but don't seem to have .blend_modes assignments added in this patch.

For example, in drivers/gpu/drm/nouveau/dispnv50/base507c.c:

static const struct nv50_wndw_func
base507c = {
	.acquire = base507c_acquire,
	.release = base507c_release,
	...
	.image_clr = base507c_image_clr,
	.update = base507c_update,
};

Without setting .blend_modes for these architectures, will they still trigger
the DRM core warning "[PLANE:%d:%s] pixel format with alpha exposed but blend
mode not setup" during display initialization on Tesla through Pascal hardware?

>  
>  	int (*update)(struct nv50_wndw *, u32 *interlock);
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720205541.257781-1-lyude@redhat.com?part=1

Re: [PATCH v2 2/3] drm/nouveau/kms/nv50-: Introduce nv50_wndw_default_state()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect comparison between the enum value `pixel_blend_mode` and the bitmask `blend_modes` causes a valid default mode to be silently overwritten.
--

--- Patch [2]: [PATCH v2 2/3] drm/nouveau/kms/nv50-: Introduce nv50_wndw_default_state() ---
commit 43504d2c3aaff4f127ccc271e9439f3f380cd55b
Author: Lyude Paul <lyude@redhat.com>

drm/nouveau/kms/nv50-: Introduce nv50_wndw_default_state()

This commit introduces nv50_wndw_default_state() to enforce a valid
pixel_blend_mode for planes supporting alpha channels during driver startup.

> diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> index 2635458d52acc..0f331f47bded8 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c

[ ... ]

> @@ -848,6 +848,28 @@ static const u64 nv50_cursor_format_modifiers[] = {
>  	DRM_FORMAT_MOD_INVALID,
>  };
>  
> +/*
> + * Setup defaults for the atomic wndw state
> + */
> +void
> +nv50_wndw_default_state(struct nv50_wndw *wndw)
> +{
> +	struct nv50_wndw_atom *armw = nv50_wndw_atom(wndw->plane.state);
> +	const unsigned int blend_modes = wndw->func->blend_modes;
> +
> +	drm_modeset_lock_assert_held(&wndw->plane.mutex);
> +
> +	/* Ensure the plane's atomic state didn't default to a pixel_blend_mode we don't support */
> +	if (blend_modes && (!(armw->state.pixel_blend_mode & blend_modes))) {

[Severity: High]
Does this incorrectly perform a bitwise AND between an enum value and a bitmask?

Since DRM_MODE_BLEND_PREMULTI is defined as 0, when pixel_blend_mode defaults
to PREMULTI, armw->state.pixel_blend_mode & blend_modes will evaluate to 0.

This causes the negation !(0) to evaluate to true, unintentionally entering
this block and overwriting the valid default mode with DRM_MODE_BLEND_COVERAGE.

Should this comparison be using BIT(armw->state.pixel_blend_mode) & blend_modes
to properly check if the enum value is supported by the bitmask?

> +		if (blend_modes & BIT(DRM_MODE_BLEND_COVERAGE))
> +			armw->state.pixel_blend_mode = DRM_MODE_BLEND_COVERAGE;
> +		else if (blend_modes & BIT(DRM_MODE_BLEND_PREMULTI))
> +			armw->state.pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> +		else if (blend_modes & BIT(DRM_MODE_BLEND_PIXEL_NONE))
> +			armw->state.pixel_blend_mode = DRM_MODE_BLEND_PIXEL_NONE;
> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720205541.257781-1-lyude@redhat.com?part=2

Re: [PATCH v2 3/3] drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for wndws

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a conditional check that contradicts its commit message and results in an incomplete fix for a DRM core WARN_ON. While claiming to 'unconditionally create blend_mode prop for wndws', the code limits this to planes where `func->blend_modes` is non-zero. Older primary planes (e.g., `base507c`) expose alpha formats but lack `.blend_modes` initialization, meaning they will still trigger the warning.
--

--- Patch [3]: [PATCH v2 3/3] drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for wndws ---
commit d890a1a203754ff1359f8cdacfa01e033bb7b8f3
Author: Lyude Paul <lyude@redhat.com>
drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for wndws

This patch attempts to resolve a DRM core validation warning by ensuring the
blend_mode property is created for all window types that expose color formats
with an alpha channel, even if they lack a blend_set function.

> diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> index 0f331f47bded8..84b9a16bdb8ab 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c

[ ... ]

> @@ -944,6 +937,17 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev,
>  			return ret;
>  	}
>  
> +	/*
> +	 * DRM requires that we have a blend mode property for any type of plane that exposes color
> +	 * formats with an alpha channel. So do this, even if we don't actually have control for the
> +	 * blend property hooked up with blend_set.
> +	 */
> +	if (func->blend_modes) {

[Severity: Medium]
Does this conditional check contradict the commit message? The commit states
the blend_mode property is created unconditionally for all types of wndws.

However, for older primary planes like base507c, the nv50_wndw_func definition
does not initialize .blend_modes:

static const struct nv50_wndw_func
base507c = {
	.acquire = base507c_acquire,
	.release = base507c_release,
	.sema_set = base507c_sema_set,
	...

Since func->blend_modes evaluates to 0 here, the property creation is skipped.

Because base507c_format exposes formats with an alpha channel (such as
DRM_FORMAT_ARGB8888), won't skipping the property creation here leave the issue
unfixed and cause the DRM core validation WARN_ON to still trigger?

> +		ret = drm_plane_create_blend_mode_property(&wndw->plane, func->blend_modes);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720205541.257781-1-lyude@redhat.com?part=3

Re: [PATCH v2 0/3] drm/nouveau: Fix WARN_ON() from missing blend modes

[lyude]

I had thought that testing this on fbcon would be enough, but there is
still more issues with this series I found when adding these to the
kernel I dog food on my desktop. Will figure them out and send another
respin of this series asap

On Mon, 2026-07-20 at 16:43 -0400, Lyude Paul wrote:
> Recently a change went into the kernel to ensure that drivers which
> expose color formats which have alpha channels. This would cause
> nouveau
> to hit a WARN_ON when setting up cursors, since they do support
> blending
> - but we never bothered trying to hook up the controls for it to
> nouveau.
> 
> This fixes the issue by ensuring that nouveau always exposes a blend
> mode for any planes with alpha channels, regardless of whether or not
> we
> have control of the blend mode hooked up. If someone wants to add
> control for this in the future as well, it should be fairly easy to
> hook
> up with this.
> 
> Previous version:
> 	https://patchwork.freedesktop.org/patch/741031/
> 
> Lyude Paul (3):
>   drm/nouveau/kms/nv50-: Add .blend_modes to nv50_wndw_func
>   drm/nouveau/kms/nv50-: Introduce nv50_wndw_default_state()
>   drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for
>     wndws
> 
>  drivers/gpu/drm/nouveau/dispnv50/curs507a.c |  3 ++
>  drivers/gpu/drm/nouveau/dispnv50/disp.c     |  4 +++
>  drivers/gpu/drm/nouveau/dispnv50/wndw.c     | 40 +++++++++++++++++--
> --
>  drivers/gpu/drm/nouveau/dispnv50/wndw.h     | 11 ++++++
>  drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  1 +
>  drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c |  1 +
>  drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c |  1 +
>  drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c |  1 +
>  8 files changed, 55 insertions(+), 7 deletions(-)
> 
> 
> base-commit: b0a652436b892eb9a036a031b33099dca036faaa

Re: [PATCH v2 0/3] drm/nouveau: Fix WARN_ON() from missing blend modes

[lyude]

I had thought that testing this on fbcon would be enough, but there is
still more issues with this series I found when adding these to the
kernel I dog food on my desktop. Will figure them out and send another
respin of this series asap

On Mon, 2026-07-20 at 16:43 -0400, Lyude Paul wrote:
> Recently a change went into the kernel to ensure that drivers which
> expose color formats which have alpha channels. This would cause
> nouveau
> to hit a WARN_ON when setting up cursors, since they do support
> blending
> - but we never bothered trying to hook up the controls for it to
> nouveau.
> 
> This fixes the issue by ensuring that nouveau always exposes a blend
> mode for any planes with alpha channels, regardless of whether or not
> we
> have control of the blend mode hooked up. If someone wants to add
> control for this in the future as well, it should be fairly easy to
> hook
> up with this.
> 
> Previous version:
> 	https://patchwork.freedesktop.org/patch/741031/
> 
> Lyude Paul (3):
>   drm/nouveau/kms/nv50-: Add .blend_modes to nv50_wndw_func
>   drm/nouveau/kms/nv50-: Introduce nv50_wndw_default_state()
>   drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for
>     wndws
> 
>  drivers/gpu/drm/nouveau/dispnv50/curs507a.c |  3 ++
>  drivers/gpu/drm/nouveau/dispnv50/disp.c     |  4 +++
>  drivers/gpu/drm/nouveau/dispnv50/wndw.c     | 40 +++++++++++++++++--
> --
>  drivers/gpu/drm/nouveau/dispnv50/wndw.h     | 11 ++++++
>  drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  1 +
>  drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c |  1 +
>  drivers/gpu/drm/nouveau/dispnv50/wndwc67e.c |  1 +
>  drivers/gpu/drm/nouveau/dispnv50/wndwca7e.c |  1 +
>  8 files changed, 55 insertions(+), 7 deletions(-)
> 
> 
> base-commit: b0a652436b892eb9a036a031b33099dca036faaa

Re: [PATCH v2 3/3] drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for wndws

[lyude]

On Mon, 2026-07-20 at 21:15 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [Medium] The patch introduces a conditional check that contradicts
> its commit message and results in an incomplete fix for a DRM core
> WARN_ON. While claiming to 'unconditionally create blend_mode prop
> for wndws', the code limits this to planes where `func->blend_modes`
> is non-zero. Older primary planes (e.g., `base507c`) expose alpha
> formats but lack `.blend_modes` initialization, meaning they will
> still trigger the warning.
> --
> 
> --- Patch [3]: [PATCH v2 3/3] drm/nouveau/kms/nv50-: Unconditionally
> create blend_mode prop for wndws ---
> commit d890a1a203754ff1359f8cdacfa01e033bb7b8f3
> Author: Lyude Paul <lyude@redhat.com>
> drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for
> wndws
> 
> This patch attempts to resolve a DRM core validation warning by
> ensuring the
> blend_mode property is created for all window types that expose color
> formats
> with an alpha channel, even if they lack a blend_set function.
> 
> > diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> > b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> > index 0f331f47bded8..84b9a16bdb8ab 100644
> > --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> 
> [ ... ]
> 
> > @@ -944,6 +937,17 @@ nv50_wndw_new_(const struct nv50_wndw_func
> > *func, struct drm_device *dev,
> >  			return ret;
> >  	}
> >  
> > +	/*
> > +	 * DRM requires that we have a blend mode property for any
> > type of plane that exposes color
> > +	 * formats with an alpha channel. So do this, even if we
> > don't actually have control for the
> > +	 * blend property hooked up with blend_set.
> > +	 */
> > +	if (func->blend_modes) {
> 
> [Severity: Medium]
> Does this conditional check contradict the commit message? The commit
> states
> the blend_mode property is created unconditionally for all types of
> wndws.
> 
> However, for older primary planes like base507c, the nv50_wndw_func
> definition
> does not initialize .blend_modes:
> 
> static const struct nv50_wndw_func
> base507c = {
> 	.acquire = base507c_acquire,
> 	.release = base507c_release,
> 	.sema_set = base507c_sema_set,
> 	...
> 
> Since func->blend_modes evaluates to 0 here, the property creation is
> skipped.
> 
> Because base507c_format exposes formats with an alpha channel (such
> as
> DRM_FORMAT_ARGB8888), won't skipping the property creation here leave
> the issue
> unfixed and cause the DRM core validation WARN_ON to still trigger?

this is kind of fascinating, it's referencing an issue that was on the
last version of this patch series but is most certainly fixed on this
patch series.

Weird.
> 
> > +		ret = drm_plane_create_blend_mode_property(&wndw-
> > >plane, func->blend_modes);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> >  	return 0;
> >  }