Simon Ser
2021-Feb-05  16:45 UTC
[Nouveau] [PATCH] nouveau/dispnv50: add cursor size/pitch checks
The hardware needs a FB which is packed and not too large. Add
checks to make sure this is the case.
While at it, add a debug log for the existing check. This allows
user-space to more easily figure out why a configuration is
rejected.
This commit depends on "drm/nouveau/kms/nv50-: Report max cursor
size to userspace", otherwise mode_config.cursor_{width,height}
is zero.
Signed-off-by: Simon Ser <contact at emersion.fr>
Cc: Lyude Paul <lyude at redhat.com>
Cc: Ben Skeggs <bskeggs at redhat.com>
Cc: Ilia Mirkin <imirkin at alum.mit.edu>
---
 drivers/gpu/drm/nouveau/dispnv50/curs507a.c | 22 ++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
index 54fbd6fe751d..9a401751c56d 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
@@ -99,6 +99,7 @@ curs507a_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom
*asyw,
 		 struct nv50_head_atom *asyh)
 {
 	struct nv50_head *head = nv50_head(asyw->state.crtc);
+	struct drm_device *dev = head->base.base.dev;
 	int ret;
 
 	ret = drm_atomic_helper_check_plane_state(&asyw->state,
&asyh->state,
@@ -109,8 +110,27 @@ curs507a_acquire(struct nv50_wndw *wndw, struct
nv50_wndw_atom *asyw,
 	if (ret || !asyh->curs.visible)
 		return ret;
 
-	if (asyw->image.w != asyw->image.h)
+	if (asyw->image.w != asyw->image.h) {
+		drm_dbg_atomic(dev,
+			       "Invalid cursor image size: width (%d) must match height
(%d)",
+			       asyw->image.w, asyw->image.h);
 		return -EINVAL;
+	}
+	if (asyw->image.w > dev->mode_config.cursor_width ||
+	    asyw->image.h > dev->mode_config.cursor_height) {
+		drm_dbg_atomic(dev,
+			       "Invalid cursor image size: too large (%dx%d > %dx%d)",
+			       asyw->image.w, asyw->image.h,
+			       dev->mode_config.cursor_width,
+			       dev->mode_config.cursor_height);
+		return -EINVAL;
+	}
+	if (asyw->image.pitch[0] != asyw->image.w * 4) {
+		drm_dbg_atomic(dev,
+			       "Invalid cursor image pitch: image must be packed (pitch = %d,
width = %d)",
+			       asyw->image.pitch[0], asyw->image.w);
+		return -EINVAL;
+	}
 
 	ret = head->func->curs_layout(head, asyw, asyh);
 	if (ret)
-- 
2.30.0
Ilia Mirkin
2021-Feb-05  17:34 UTC
[Nouveau] [PATCH] nouveau/dispnv50: add cursor size/pitch checks
On Fri, Feb 5, 2021 at 11:45 AM Simon Ser <contact at emersion.fr> wrote:> > The hardware needs a FB which is packed and not too large. Add > checks to make sure this is the case. > > While at it, add a debug log for the existing check. This allows > user-space to more easily figure out why a configuration is > rejected. > > This commit depends on "drm/nouveau/kms/nv50-: Report max cursor > size to userspace", otherwise mode_config.cursor_{width,height} > is zero. > > Signed-off-by: Simon Ser <contact at emersion.fr> > Cc: Lyude Paul <lyude at redhat.com> > Cc: Ben Skeggs <bskeggs at redhat.com> > Cc: Ilia Mirkin <imirkin at alum.mit.edu> > --- > drivers/gpu/drm/nouveau/dispnv50/curs507a.c | 22 ++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c > index 54fbd6fe751d..9a401751c56d 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c > @@ -99,6 +99,7 @@ curs507a_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw, > struct nv50_head_atom *asyh) > { > struct nv50_head *head = nv50_head(asyw->state.crtc); > + struct drm_device *dev = head->base.base.dev; > int ret; > > ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state, > @@ -109,8 +110,27 @@ curs507a_acquire(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw, > if (ret || !asyh->curs.visible) > return ret; > > - if (asyw->image.w != asyw->image.h) > + if (asyw->image.w != asyw->image.h) { > + drm_dbg_atomic(dev, > + "Invalid cursor image size: width (%d) must match height (%d)", > + asyw->image.w, asyw->image.h); > return -EINVAL; > + } > + if (asyw->image.w > dev->mode_config.cursor_width || > + asyw->image.h > dev->mode_config.cursor_height) { > + drm_dbg_atomic(dev, > + "Invalid cursor image size: too large (%dx%d > %dx%d)", > + asyw->image.w, asyw->image.h, > + dev->mode_config.cursor_width, > + dev->mode_config.cursor_height); > + return -EINVAL; > + } > + if (asyw->image.pitch[0] != asyw->image.w * 4) {Rather than hard-coding to 4, make this look at the format (or cpp, which should be available somewhere too I think). (Yeah, currently we only expose RGBA8, but we should also be doing RGB5A1.)> + drm_dbg_atomic(dev, > + "Invalid cursor image pitch: image must be packed (pitch = %d, width = %d)", > + asyw->image.pitch[0], asyw->image.w); > + return -EINVAL; > + } > > ret = head->func->curs_layout(head, asyw, asyh);And this will fail due to the width/height not being supported, right? -ilia