Simon Ser
2021-Feb-05 22:24 UTC
[Nouveau] [PATCH v3] nouveau/dispnv50: add cursor pitch check
The hardware needs a FB which is packed. Add checks to make sure
this is the case.
While at it, add debug logs for the existing checks. This allows
user-space to more easily figure out why a configuration is
rejected.
v2:
- Use drm_format_info instead of hardcoding bytes-per-pixel (Ilia)
- Remove unnecessary size check (Ilia)
v3:
- Add missing newlines in debug messages (Lyude)
- Use NV_ATOMIC (Lyude)
- Add missing debug log for invalid format (Ilia)
Signed-off-by: Simon Ser <contact at emersion.fr>
Reviewed-by: 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 | 31 +++++++++++++++++++--
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
index 54fbd6fe751d..56459cfd037e 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
@@ -30,6 +30,7 @@
#include <drm/drm_atomic_helper.h>
#include <drm/drm_plane_helper.h>
+#include <drm/drm_fourcc.h>
bool
curs507a_space(struct nv50_wndw *wndw)
@@ -99,6 +100,8 @@ 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 nouveau_drm *drm = nouveau_drm(head->base.base.dev);
+ struct drm_framebuffer *fb = asyw->state.fb;
int ret;
ret = drm_atomic_helper_check_plane_state(&asyw->state,
&asyh->state,
@@ -109,14 +112,36 @@ 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) {
+ NV_ATOMIC(drm,
+ "Invalid cursor image size: width (%d) must match height
(%d)\n",
+ asyw->image.w, asyw->image.h);
return -EINVAL;
+ }
+ if (asyw->image.pitch[0] != asyw->image.w * fb->format->cpp[0]) {
+ NV_ATOMIC(drm,
+ "Invalid cursor image pitch: image must be packed (pitch = %d, width
= %d)\n",
+ asyw->image.pitch[0], asyw->image.w);
+ return -EINVAL;
+ }
ret = head->func->curs_layout(head, asyw, asyh);
- if (ret)
+ if (ret) {
+ NV_ATOMIC(drm,
+ "Invalid cursor image size: unsupported size %dx%d\n",
+ asyw->image.w, asyw->image.h);
+ return ret;
+ }
+
+ ret = head->func->curs_format(head, asyw, asyh);
+ if (ret) {
+ NV_ATOMIC(drm,
+ "Invalid cursor image format 0x%X\n",
+ fb->format->format);
return ret;
+ }
- return head->func->curs_format(head, asyw, asyh);
+ return 0;
}
static const u32
--
2.30.0
Ilia Mirkin
2021-Feb-05 22:36 UTC
[Nouveau] [PATCH v3] nouveau/dispnv50: add cursor pitch check
On Fri, Feb 5, 2021 at 5:24 PM Simon Ser <contact at emersion.fr> wrote:> > The hardware needs a FB which is packed. Add checks to make sure > this is the case. > > While at it, add debug logs for the existing checks. This allows > user-space to more easily figure out why a configuration is > rejected. > > v2: > - Use drm_format_info instead of hardcoding bytes-per-pixel (Ilia) > - Remove unnecessary size check (Ilia) > > v3: > - Add missing newlines in debug messages (Lyude) > - Use NV_ATOMIC (Lyude) > - Add missing debug log for invalid format (Ilia) > > Signed-off-by: Simon Ser <contact at emersion.fr> > Reviewed-by: 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 | 31 +++++++++++++++++++-- > 1 file changed, 28 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c > index 54fbd6fe751d..56459cfd037e 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c > @@ -30,6 +30,7 @@ > > #include <drm/drm_atomic_helper.h> > #include <drm/drm_plane_helper.h> > +#include <drm/drm_fourcc.h>Why is this needed?> > bool > curs507a_space(struct nv50_wndw *wndw) > @@ -99,6 +100,8 @@ 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 nouveau_drm *drm = nouveau_drm(head->base.base.dev); > + struct drm_framebuffer *fb = asyw->state.fb; > int ret; > > ret = drm_atomic_helper_check_plane_state(&asyw->state, &asyh->state, > @@ -109,14 +112,36 @@ 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) { > + NV_ATOMIC(drm, > + "Invalid cursor image size: width (%d) must match height (%d)\n", > + asyw->image.w, asyw->image.h);Maybe keep with the style of the other NV_ATOMIC's, e.g. include %s: at the beginning with wndw->plane.name as the value? Either way, Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>> return -EINVAL; > + } > + if (asyw->image.pitch[0] != asyw->image.w * fb->format->cpp[0]) { > + NV_ATOMIC(drm, > + "Invalid cursor image pitch: image must be packed (pitch = %d, width = %d)\n", > + asyw->image.pitch[0], asyw->image.w); > + return -EINVAL; > + } > > ret = head->func->curs_layout(head, asyw, asyh); > - if (ret) > + if (ret) { > + NV_ATOMIC(drm, > + "Invalid cursor image size: unsupported size %dx%d\n", > + asyw->image.w, asyw->image.h); > + return ret; > + } > + > + ret = head->func->curs_format(head, asyw, asyh); > + if (ret) { > + NV_ATOMIC(drm, > + "Invalid cursor image format 0x%X\n", > + fb->format->format); > return ret; > + } > > - return head->func->curs_format(head, asyw, asyh); > + return 0; > } > > static const u32 > -- > 2.30.0 >