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