Lyude Paul
2021-Jan-19 01:54 UTC
[Nouveau] [PATCH 1/3] drivers/nouveau/kms/nv50-: Reject format modifiers for cursor planes
Nvidia hardware doesn't actually support using tiling formats with the
cursor plane, only linear is allowed. In the future, we should write a
testcase for this.
Fixes: c586f30bf74c ("drm/nouveau/kms: Add format mod prop to
base/ovly/nvdisp")
Cc: James Jones <jajones at nvidia.com>
Cc: Martin Peres <martin.peres at free.fr>
Cc: Jeremy Cline <jcline at redhat.com>
Cc: Simon Ser <contact at emersion.fr>
Cc: <stable at vger.kernel.org> # v5.8+
Signed-off-by: Lyude Paul <lyude at redhat.com>
---
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index ce451242f79e..271de3a63f21 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -702,6 +702,11 @@ nv50_wndw_init(struct nv50_wndw *wndw)
nvif_notify_get(&wndw->notify);
}
+static const u64 nv50_cursor_format_modifiers[] = {
+ DRM_FORMAT_MOD_LINEAR,
+ DRM_FORMAT_MOD_INVALID,
+};
+
int
nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev,
enum drm_plane_type type, const char *name, int index,
@@ -713,6 +718,7 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct
drm_device *dev,
struct nvif_mmu *mmu = &drm->client.mmu;
struct nv50_disp *disp = nv50_disp(dev);
struct nv50_wndw *wndw;
+ const u64 *format_modifiers;
int nformat;
int ret;
@@ -728,10 +734,13 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct
drm_device *dev,
for (nformat = 0; format[nformat]; nformat++);
- ret = drm_universal_plane_init(dev, &wndw->plane, heads,
&nv50_wndw,
- format, nformat,
- nouveau_display(dev)->format_modifiers,
- type, "%s-%d", name, index);
+ if (type == DRM_PLANE_TYPE_CURSOR)
+ format_modifiers = nv50_cursor_format_modifiers;
+ else
+ format_modifiers = nouveau_display(dev)->format_modifiers;
+
+ ret = drm_universal_plane_init(dev, &wndw->plane, heads,
&nv50_wndw, format, nformat,
+ format_modifiers, type, "%s-%d", name, index);
if (ret) {
kfree(*pwndw);
*pwndw = NULL;
--
2.29.2
Lyude Paul
2021-Jan-19 01:54 UTC
[Nouveau] [PATCH 2/3] drm/nouveau/kms/nv50-: Report max cursor size to userspace
Cc: Martin Peres <martin.peres at free.fr>
Cc: Jeremy Cline <jcline at redhat.com>
Cc: Simon Ser <contact at emersion.fr>
Signed-off-by: Lyude Paul <lyude at redhat.com>
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index c6367035970e..5f4f09a601d4 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2663,6 +2663,14 @@ nv50_display_create(struct drm_device *dev)
else
nouveau_display(dev)->format_modifiers = disp50xx_modifiers;
+ if (disp->disp->object.oclass >= GK104_DISP) {
+ dev->mode_config.cursor_width = 256;
+ dev->mode_config.cursor_height = 256;
+ } else {
+ dev->mode_config.cursor_width = 64;
+ dev->mode_config.cursor_height = 64;
+ }
+
/* create crtc objects to represent the hw heads */
if (disp->disp->object.oclass >= GV100_DISP)
crtcs = nvif_rd32(&device->object, 0x610060) & 0xff;
--
2.29.2
Lyude Paul
2021-Jan-19 01:54 UTC
[Nouveau] [PATCH 3/3] drm/nouveau/kms/nve4-nv138: Fix > 64x64 cursors
While we do handle the additional cursor sizes introduced in NVE4, it looks
like we accidentally broke this when converting over to use Nvidia's
display headers. Since we now use NVVAL in dispnv50/head907d.c in order to
format the value for the cursor layout and NVD9 only had one byte reserved
vs. the 2 bytes reserved in later generations, we end up accidentally
stripping the second bit in the cursor layout format parameter - causing us
to set the wrong cursor size.
This fixes that by adding our own curs_set hook for 917d which uses the
NV917D headers.
Cc: Martin Peres <martin.peres at free.fr>
Cc: Jeremy Cline <jcline at redhat.com>
Cc: Simon Ser <contact at emersion.fr>
Cc: <stable at vger.kernel.org> # v5.9+
Signed-off-by: Lyude Paul <lyude at redhat.com>
Fixes: ed0b86a90bf9 ("drm/nouveau/kms/nv50-: use NVIDIA's headers for
core head_curs_set()")
---
drivers/gpu/drm/nouveau/dispnv50/head917d.c | 28 ++++++++++++++++++-
.../drm/nouveau/include/nvhw/class/cl917d.h | 4 +++
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head917d.c
b/drivers/gpu/drm/nouveau/dispnv50/head917d.c
index a5d827403660..ea9f8667305e 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head917d.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head917d.c
@@ -22,6 +22,7 @@
#include "head.h"
#include "core.h"
+#include "nvif/push.h"
#include <nvif/push507c.h>
#include <nvhw/class/cl917d.h>
@@ -73,6 +74,31 @@ head917d_base(struct nv50_head *head, struct nv50_head_atom
*asyh)
return 0;
}
+static int
+head917d_curs_set(struct nv50_head *head, struct nv50_head_atom *asyh)
+{
+ struct nvif_push *push =
nv50_disp(head->base.base.dev)->core->chan.push;
+ const int i = head->base.index;
+ int ret;
+
+ ret = PUSH_WAIT(push, 5);
+ if (ret)
+ return ret;
+
+ PUSH_MTHD(push, NV917D, HEAD_SET_CONTROL_CURSOR(i),
+ NVDEF(NV917D, HEAD_SET_CONTROL_CURSOR, ENABLE, ENABLE) |
+ NVVAL(NV917D, HEAD_SET_CONTROL_CURSOR, FORMAT, asyh->curs.format) |
+ NVVAL(NV917D, HEAD_SET_CONTROL_CURSOR, SIZE, asyh->curs.layout) |
+ NVVAL(NV917D, HEAD_SET_CONTROL_CURSOR, HOT_SPOT_X, 0) |
+ NVVAL(NV917D, HEAD_SET_CONTROL_CURSOR, HOT_SPOT_Y, 0) |
+ NVDEF(NV917D, HEAD_SET_CONTROL_CURSOR, COMPOSITION, ALPHA_BLEND),
+
+ HEAD_SET_OFFSET_CURSOR(i), asyh->curs.offset >> 8);
+
+ PUSH_MTHD(push, NV917D, HEAD_SET_CONTEXT_DMA_CURSOR(i), asyh->curs.handle);
+ return 0;
+}
+
int
head917d_curs_layout(struct nv50_head *head, struct nv50_wndw_atom *asyw,
struct nv50_head_atom *asyh)
@@ -101,7 +127,7 @@ head917d = {
.core_clr = head907d_core_clr,
.curs_layout = head917d_curs_layout,
.curs_format = head507d_curs_format,
- .curs_set = head907d_curs_set,
+ .curs_set = head917d_curs_set,
.curs_clr = head907d_curs_clr,
.base = head917d_base,
.ovly = head907d_ovly,
diff --git a/drivers/gpu/drm/nouveau/include/nvhw/class/cl917d.h
b/drivers/gpu/drm/nouveau/include/nvhw/class/cl917d.h
index 2a2612d6e1e0..fb223723a38a 100644
--- a/drivers/gpu/drm/nouveau/include/nvhw/class/cl917d.h
+++ b/drivers/gpu/drm/nouveau/include/nvhw/class/cl917d.h
@@ -66,6 +66,10 @@
#define NV917D_HEAD_SET_CONTROL_CURSOR_COMPOSITION_ALPHA_BLEND
(0x00000000)
#define NV917D_HEAD_SET_CONTROL_CURSOR_COMPOSITION_PREMULT_ALPHA_BLEND
(0x00000001)
#define NV917D_HEAD_SET_CONTROL_CURSOR_COMPOSITION_XOR
(0x00000002)
+#define NV917D_HEAD_SET_OFFSET_CURSOR(a)
(0x00000484 + (a)*0x00000300)
+#define NV917D_HEAD_SET_OFFSET_CURSOR_ORIGIN
31:0
+#define NV917D_HEAD_SET_CONTEXT_DMA_CURSOR(a)
(0x0000048C + (a)*0x00000300)
+#define NV917D_HEAD_SET_CONTEXT_DMA_CURSOR_HANDLE
31:0
#define NV917D_HEAD_SET_DITHER_CONTROL(a)
(0x000004A0 + (a)*0x00000300)
#define NV917D_HEAD_SET_DITHER_CONTROL_ENABLE
0:0
#define NV917D_HEAD_SET_DITHER_CONTROL_ENABLE_DISABLE
(0x00000000)
--
2.29.2
Simon Ser
2021-Jan-19 10:18 UTC
[Nouveau] [PATCH 1/3] drivers/nouveau/kms/nv50-: Reject format modifiers for cursor planes
On Tuesday, January 19th, 2021 at 2:54 AM, Lyude Paul <lyude at redhat.com> wrote:> Nvidia hardware doesn't actually support using tiling formats with the > cursor plane, only linear is allowed. In the future, we should write a > testcase for this. > > Fixes: c586f30bf74c ("drm/nouveau/kms: Add format mod prop to base/ovly/nvdisp") > Cc: James Jones <jajones at nvidia.com> > Cc: Martin Peres <martin.peres at free.fr> > Cc: Jeremy Cline <jcline at redhat.com> > Cc: Simon Ser <contact at emersion.fr> > Cc: <stable at vger.kernel.org> # v5.8+ > Signed-off-by: Lyude Paul <lyude at redhat.com>Together with [1], this patch allows me to run unpatched modifier-aware user-space successfully, without a cursor visual glitch. drm_info correctly reports the new modifier list, and wlroots logs confirm that a flavor of NVIDIA_BLOCK_LINEAR_2D is used for the primary buffers and LINEAR is used for cursor buffers. Code looks good to me as well. Reviewed-by: Simon Ser <contact at emersion.fr> [1]: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/3724
Ville Syrjälä
2021-Jan-19 15:50 UTC
[Nouveau] [PATCH 1/3] drivers/nouveau/kms/nv50-: Reject format modifiers for cursor planes
On Mon, Jan 18, 2021 at 08:54:12PM -0500, Lyude Paul wrote:> Nvidia hardware doesn't actually support using tiling formats with the > cursor plane, only linear is allowed. In the future, we should write a > testcase for this.There are a couple of old modifier/format sanity tests here: https://patchwork.freedesktop.org/series/46876/ Couple of things missing that now came to my mind: - test setplane/etc. rejects unsupported formats/modifiers even if addfb allowed them, exactly for the case where only a subset of planes support something - validate the IN_FORMATS blob harder, eg. make sure each modifier listed there supports at least one format. IIRC this was busted on a few drivers last year, dunno if they got fixed or not. Hmm, actually since this is now using the pre-parsed stuff I guess we should just stick an assert into igt_fill_plane_format_mod() where the blob gets parsed> > Fixes: c586f30bf74c ("drm/nouveau/kms: Add format mod prop to base/ovly/nvdisp") > Cc: James Jones <jajones at nvidia.com> > Cc: Martin Peres <martin.peres at free.fr> > Cc: Jeremy Cline <jcline at redhat.com> > Cc: Simon Ser <contact at emersion.fr> > Cc: <stable at vger.kernel.org> # v5.8+ > Signed-off-by: Lyude Paul <lyude at redhat.com> > --- > drivers/gpu/drm/nouveau/dispnv50/wndw.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > index ce451242f79e..271de3a63f21 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > @@ -702,6 +702,11 @@ nv50_wndw_init(struct nv50_wndw *wndw) > nvif_notify_get(&wndw->notify); > } > > +static const u64 nv50_cursor_format_modifiers[] = { > + DRM_FORMAT_MOD_LINEAR, > + DRM_FORMAT_MOD_INVALID, > +}; > + > int > nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev, > enum drm_plane_type type, const char *name, int index, > @@ -713,6 +718,7 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev, > struct nvif_mmu *mmu = &drm->client.mmu; > struct nv50_disp *disp = nv50_disp(dev); > struct nv50_wndw *wndw; > + const u64 *format_modifiers; > int nformat; > int ret; > > @@ -728,10 +734,13 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev, > > for (nformat = 0; format[nformat]; nformat++); > > - ret = drm_universal_plane_init(dev, &wndw->plane, heads, &nv50_wndw, > - format, nformat, > - nouveau_display(dev)->format_modifiers, > - type, "%s-%d", name, index); > + if (type == DRM_PLANE_TYPE_CURSOR) > + format_modifiers = nv50_cursor_format_modifiers; > + else > + format_modifiers = nouveau_display(dev)->format_modifiers; > + > + ret = drm_universal_plane_init(dev, &wndw->plane, heads, &nv50_wndw, format, nformat, > + format_modifiers, type, "%s-%d", name, index); > if (ret) { > kfree(*pwndw); > *pwndw = NULL; > -- > 2.29.2 > > _______________________________________________ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel-- Ville Syrj?l? Intel
James Jones
2021-Jan-19 15:52 UTC
[Nouveau] [PATCH 1/3] drivers/nouveau/kms/nv50-: Reject format modifiers for cursor planes
Gah, yes, good catch. Reviewed-by: James Jones <jajones at nvidia.com> On 1/18/21 5:54 PM, Lyude Paul wrote:> Nvidia hardware doesn't actually support using tiling formats with the > cursor plane, only linear is allowed. In the future, we should write a > testcase for this. > > Fixes: c586f30bf74c ("drm/nouveau/kms: Add format mod prop to base/ovly/nvdisp") > Cc: James Jones <jajones at nvidia.com> > Cc: Martin Peres <martin.peres at free.fr> > Cc: Jeremy Cline <jcline at redhat.com> > Cc: Simon Ser <contact at emersion.fr> > Cc: <stable at vger.kernel.org> # v5.8+ > Signed-off-by: Lyude Paul <lyude at redhat.com> > --- > drivers/gpu/drm/nouveau/dispnv50/wndw.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > index ce451242f79e..271de3a63f21 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > @@ -702,6 +702,11 @@ nv50_wndw_init(struct nv50_wndw *wndw) > nvif_notify_get(&wndw->notify); > } > > +static const u64 nv50_cursor_format_modifiers[] = { > + DRM_FORMAT_MOD_LINEAR, > + DRM_FORMAT_MOD_INVALID, > +}; > + > int > nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev, > enum drm_plane_type type, const char *name, int index, > @@ -713,6 +718,7 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev, > struct nvif_mmu *mmu = &drm->client.mmu; > struct nv50_disp *disp = nv50_disp(dev); > struct nv50_wndw *wndw; > + const u64 *format_modifiers; > int nformat; > int ret; > > @@ -728,10 +734,13 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev, > > for (nformat = 0; format[nformat]; nformat++); > > - ret = drm_universal_plane_init(dev, &wndw->plane, heads, &nv50_wndw, > - format, nformat, > - nouveau_display(dev)->format_modifiers, > - type, "%s-%d", name, index); > + if (type == DRM_PLANE_TYPE_CURSOR) > + format_modifiers = nv50_cursor_format_modifiers; > + else > + format_modifiers = nouveau_display(dev)->format_modifiers; > + > + ret = drm_universal_plane_init(dev, &wndw->plane, heads, &nv50_wndw, format, nformat, > + format_modifiers, type, "%s-%d", name, index); > if (ret) { > kfree(*pwndw); > *pwndw = NULL; >