diff mbox series

drm/nouveau/kms/nv50-: Check plane size for cursors, not fb size

Message ID 20210318215545.901756-1-lyude@redhat.com (mailing list archive)
State New, archived
Headers show
Series drm/nouveau/kms/nv50-: Check plane size for cursors, not fb size | expand

Commit Message

Lyude Paul March 18, 2021, 9:55 p.m. UTC
Found this while trying to make some changes to the kms_cursor_crc test.
curs507a_acquire checks that the width and height of the cursor framebuffer
are equal (asyw->image.{w,h}). This is actually wrong though, as we only
want to be concerned that the actual width/height of the plane are the
same. It's fine if we scan out from an fb that's slightly larger than the
cursor plane (in fact, some igt tests actually do this).

Note that I'm not entirely sure why this wasn't previously breaking
kms_cursor_crc tests - they all set up cursors with the height being one
pixel larger than the actual size of the cursor. But this seems to fix
things, and the code before was definitely incorrect - so it's not really
worth looking into further imho.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: Martin Peres <martin.peres@mupuf.org>
Cc: Jeremy Cline <jcline@redhat.com>
---
 drivers/gpu/drm/nouveau/dispnv50/curs507a.c | 2 +-
 drivers/gpu/drm/nouveau/dispnv50/head507d.c | 2 +-
 drivers/gpu/drm/nouveau/dispnv50/head917d.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Comments

Ilia Mirkin March 18, 2021, 10:13 p.m. UTC | #1
On Thu, Mar 18, 2021 at 5:56 PM Lyude Paul <lyude@redhat.com> wrote:
>
> Found this while trying to make some changes to the kms_cursor_crc test.
> curs507a_acquire checks that the width and height of the cursor framebuffer
> are equal (asyw->image.{w,h}). This is actually wrong though, as we only
> want to be concerned that the actual width/height of the plane are the
> same. It's fine if we scan out from an fb that's slightly larger than the
> cursor plane (in fact, some igt tests actually do this).

How so? The scanout engine expects the data to be packed. Height can
be larger, but width has to match.

  -ilia
Lyude Paul March 18, 2021, 10:39 p.m. UTC | #2
On Thu, 2021-03-18 at 18:13 -0400, Ilia Mirkin wrote:
> On Thu, Mar 18, 2021 at 5:56 PM Lyude Paul <lyude@redhat.com> wrote:
> > 
> > Found this while trying to make some changes to the kms_cursor_crc test.
> > curs507a_acquire checks that the width and height of the cursor framebuffer
> > are equal (asyw->image.{w,h}). This is actually wrong though, as we only
> > want to be concerned that the actual width/height of the plane are the
> > same. It's fine if we scan out from an fb that's slightly larger than the
> > cursor plane (in fact, some igt tests actually do this).
> 
> How so? The scanout engine expects the data to be packed. Height can
> be larger, but width has to match.

Huh - wasn't expecting that, nice catch. I'll fix this up in a moment

> 
>   -ilia
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
index 54fbd6fe751d..7a7f80e51ec0 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
@@ -109,7 +109,7 @@  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->state.crtc_w != asyw->state.crtc_h)
 		return -EINVAL;
 
 	ret = head->func->curs_layout(head, asyw, asyh);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head507d.c b/drivers/gpu/drm/nouveau/dispnv50/head507d.c
index 09b89983864b..3d230ca488c9 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head507d.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head507d.c
@@ -176,7 +176,7 @@  int
 head507d_curs_layout(struct nv50_head *head, struct nv50_wndw_atom *asyw,
 		     struct nv50_head_atom *asyh)
 {
-	switch (asyw->image.w) {
+	switch (asyw->state.crtc_w) {
 	case 32: asyh->curs.layout = NV507D_HEAD_SET_CONTROL_CURSOR_SIZE_W32_H32; break;
 	case 64: asyh->curs.layout = NV507D_HEAD_SET_CONTROL_CURSOR_SIZE_W64_H64; break;
 	default:
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head917d.c b/drivers/gpu/drm/nouveau/dispnv50/head917d.c
index 4ce47b55f72c..caa7d633691b 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head917d.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head917d.c
@@ -103,7 +103,7 @@  int
 head917d_curs_layout(struct nv50_head *head, struct nv50_wndw_atom *asyw,
 		     struct nv50_head_atom *asyh)
 {
-	switch (asyw->state.fb->width) {
+	switch (asyw->state.crtc_w) {
 	case  32: asyh->curs.layout = NV917D_HEAD_SET_CONTROL_CURSOR_SIZE_W32_H32; break;
 	case  64: asyh->curs.layout = NV917D_HEAD_SET_CONTROL_CURSOR_SIZE_W64_H64; break;
 	case 128: asyh->curs.layout = NV917D_HEAD_SET_CONTROL_CURSOR_SIZE_W128_H128; break;