| Message ID | 20210119015415.2511028-2-lyude@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drivers/nouveau/kms/nv50-: Reject format modifiers for cursor planes | expand |
On Tuesday, January 19th, 2021 at 2:54 AM, Lyude Paul <lyude@redhat.com> wrote: > Cc: Martin Peres <martin.peres@free.fr> > Cc: Jeremy Cline <jcline@redhat.com> > Cc: Simon Ser <contact@emersion.fr> > Signed-off-by: Lyude Paul <lyude@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) { I can confirm this works fine on GK208B. Tested with wlroots. I don't have older cards to test, though. Tested-by: Simon Ser <contact@emersion.fr> > + 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, Tue, Jan 19, 2021 02:54:13 +0100: > 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; This change broke X cursor in my setup, and reverting the commit restores it. Dell Precision M4800, issue ~2014 with GK106GLM [Quadro K2100M] (rev a1). libdrm 2.4.91-1 (Debian 10.8 stable). There are no errors or warnings in Xorg logs nor in the kernel log. Regards, Alex
On Tue, Feb 23, 2021 at 9:26 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > Lyude Paul, Tue, Jan 19, 2021 02:54:13 +0100: > > 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; > > This change broke X cursor in my setup, and reverting the commit restores it. > > Dell Precision M4800, issue ~2014 with GK106GLM [Quadro K2100M] (rev a1). > libdrm 2.4.91-1 (Debian 10.8 stable). > There are no errors or warnings in Xorg logs nor in the kernel log. Hi Alex, Could you confirm which ddx is driving the nvidia hw? You can find this out by running "xrandr --listproviders", or also in the xorg log. Thanks, -ilia
Ilia Mirkin, Tue, Feb 23, 2021 15:56:21 +0100: > On Tue, Feb 23, 2021 at 9:26 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > Lyude Paul, Tue, Jan 19, 2021 02:54:13 +0100: > > > 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; > > > > This change broke X cursor in my setup, and reverting the commit restores it. > > > > Dell Precision M4800, issue ~2014 with GK106GLM [Quadro K2100M] (rev a1). > > libdrm 2.4.91-1 (Debian 10.8 stable). > > There are no errors or warnings in Xorg logs nor in the kernel log. > > Could you confirm which ddx is driving the nvidia hw? You can find > this out by running "xrandr --listproviders", or also in the xorg log. xrandr(1) does not seem to list much: $ xrandr --listproviders Providers: number : 1 Provider 0: id: 0x48 cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 4 outputs: 5 associated providers: 0 name:modesetting I failed to find a DDX in Xorg.0.log. Both Xorg.0.log and dmesg can be seen here: https://gist.github.com/ar-cetitec/68c27551d9a59b89dc73bffe0456bbef
On Tue, Feb 23, 2021 at 10:36 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > Ilia Mirkin, Tue, Feb 23, 2021 15:56:21 +0100: > > On Tue, Feb 23, 2021 at 9:26 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > > Lyude Paul, Tue, Jan 19, 2021 02:54:13 +0100: > > > > 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; > > > > > > This change broke X cursor in my setup, and reverting the commit restores it. > > > > > > Dell Precision M4800, issue ~2014 with GK106GLM [Quadro K2100M] (rev a1). > > > libdrm 2.4.91-1 (Debian 10.8 stable). > > > There are no errors or warnings in Xorg logs nor in the kernel log. > > > > Could you confirm which ddx is driving the nvidia hw? You can find > > this out by running "xrandr --listproviders", or also in the xorg log. > > xrandr(1) does not seem to list much: > > $ xrandr --listproviders > Providers: number : 1 > Provider 0: id: 0x48 cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 4 outputs: 5 associated providers: 0 name:modesetting Thanks - this is what I was looking for. name:modesetting, i.e. the modesetting ddx driver. I checked nouveau source, and it seems like it uses a 64x64 cursor no matter what. Not sure what the modesetting ddx does. I'd recommend using xf86-video-nouveau in any case, but some distros have decided to explicitly force modesetting in preference of nouveau. Oh well. (And regardless, the regression should be addressed somehow, but it's also good to understand what the problem is.) Can you confirm what the problem with the cursor is? -ilia
Ilia Mirkin, Tue, Feb 23, 2021 16:46:52 +0100: > On Tue, Feb 23, 2021 at 10:36 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > Ilia Mirkin, Tue, Feb 23, 2021 15:56:21 +0100: > > > On Tue, Feb 23, 2021 at 9:26 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > > > > > > > This change broke X cursor in my setup, and reverting the commit restores it. > > > > > > > > Dell Precision M4800, issue ~2014 with GK106GLM [Quadro K2100M] (rev a1). > > > > libdrm 2.4.91-1 (Debian 10.8 stable). > > > > There are no errors or warnings in Xorg logs nor in the kernel log. > > > > > > Could you confirm which ddx is driving the nvidia hw? You can find > > > this out by running "xrandr --listproviders", or also in the xorg log. > > > > xrandr(1) does not seem to list much: > > > > $ xrandr --listproviders > > Providers: number : 1 > > Provider 0: id: 0x48 cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 4 outputs: 5 associated providers: 0 name:modesetting > > Thanks - this is what I was looking for. name:modesetting, i.e. the > modesetting ddx driver. > > I checked nouveau source, and it seems like it uses a 64x64 cursor no > matter what. Not sure what the modesetting ddx does. > > I'd recommend using xf86-video-nouveau in any case, but some distros I would like try this out. Do you know how to force the xorg server to choose this driver instead of modesetting? > have decided to explicitly force modesetting in preference of nouveau. > Oh well. (And regardless, the regression should be addressed somehow, > but it's also good to understand what the problem is.) > > Can you confirm what the problem with the cursor is? The cursor looks stretched vertically over a bigger matrix, while missing some lines and being wrapped over the bottom on top of that matrix.
Alex Riesen, Tue, Feb 23, 2021 16:51:26 +0100: > Ilia Mirkin, Tue, Feb 23, 2021 16:46:52 +0100: > > I'd recommend using xf86-video-nouveau in any case, but some distros > > I would like try this out. Do you know how to force the xorg server to > choose this driver instead of modesetting? Found that myself (a Device section with Driver set to "nouveau"): $ xrandr --listproviders Providers: number : 1 Provider 0: id: 0x68 cap: 0x7, Source Output, Sink Output, Source Offload crtcs: 4 outputs: 5 associated providers: 0 name:nouveau And yes, the cursor looks good in v5.11 even without reverting the commit.
On Tue, Feb 23, 2021 at 11:23 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > Alex Riesen, Tue, Feb 23, 2021 16:51:26 +0100: > > Ilia Mirkin, Tue, Feb 23, 2021 16:46:52 +0100: > > > I'd recommend using xf86-video-nouveau in any case, but some distros > > > > I would like try this out. Do you know how to force the xorg server to > > choose this driver instead of modesetting? > > Found that myself (a Device section with Driver set to "nouveau"): > > $ xrandr --listproviders > Providers: number : 1 > Provider 0: id: 0x68 cap: 0x7, Source Output, Sink Output, Source Offload crtcs: 4 outputs: 5 associated providers: 0 name:nouveau > > And yes, the cursor looks good in v5.11 even without reverting the commit. FWIW it's not immediately apparent to me what grave error modesetting is committing in setting the cursor. The logic looks perfectly reasonable. It's not trying to be fancy with rendering the cursor/etc. The one thing is that it's using drmModeSetCursor2 which sets the hotspot at the same time. But internally inside nouveau I think it should work out to the same thing. Perhaps setting the hotspot, or something in that path, doesn't quite work for 256x256? [Again, no clue what that might be.] It might also be worthwhile just testing if the 256x256 cursor works quite the way one would want. If you're interested, grab libdrm, there's a test called 'modetest', which has an option to enable a moving cursor (-c iirc). It's hard-coded to 64x64, so you'll have to modify it there too (and probably change the pattern from plain gray to any one of the other ones). Cheers, -ilia
Ilia Mirkin, Tue, Feb 23, 2021 19:13:59 +0100: > On Tue, Feb 23, 2021 at 11:23 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > > > $ xrandr --listproviders > > Providers: number : 1 > > Provider 0: id: 0x68 cap: 0x7, Source Output, Sink Output, Source Offload crtcs: 4 outputs: 5 associated providers: 0 name:nouveau > > > > And yes, the cursor looks good in v5.11 even without reverting the commit. > > FWIW it's not immediately apparent to me what grave error modesetting > is committing in setting the cursor. The logic looks perfectly > reasonable. It's not trying to be fancy with rendering the cursor/etc. > > The one thing is that it's using drmModeSetCursor2 which sets the > hotspot at the same time. But internally inside nouveau I think it > should work out to the same thing. Perhaps setting the hotspot, or > something in that path, doesn't quite work for 256x256? [Again, no > clue what that might be.] > > It might also be worthwhile just testing if the 256x256 cursor works > quite the way one would want. If you're interested, grab libdrm, > there's a test called 'modetest', which has an option to enable a > moving cursor (-c iirc). It's hard-coded to 64x64, so you'll have to > modify it there too (and probably change the pattern from plain gray > to any one of the other ones). I am interested, so I did. If I start the test without X running, the sprite of 256x256 cursor always contained horizontal lines across it, both with commit reverted and vanilla v5.11. Similarly, the 64x64 cursor has no lines across it in both kernels. The test does not seem to work at all if there is an X server running (using modesetting driver): modetest complained about permission denied to set the mode, and just sits there, drawing nothing on the displays. So I could not run the test in the environment of original problem. Am I starting it correctly? Is the change in modetest.c correct? $ ./modetest -c |grep '^[0-9]\|preferred' 85 86 connected LVDS-1 340x190 13 86 #0 1920x1080 60.01 1920 2010 2070 2226 1080 1086 1095 1142 152540 flags: phsync, nvsync; type: preferred, driver 87 89 connected DP-1 470x300 18 88, 89 #0 1680x1050 59.88 1680 1728 1760 1840 1050 1053 1059 1080 119000 flags: phsync, nvsync; type: preferred, driver 90 0 disconnected DP-2 0x0 0 91, 92 93 95 connected DP-3 520x320 10 94, 95 #0 1920x1200 59.95 1920 1968 2000 2080 1200 1203 1209 1235 154000 flags: phsync, nvsync; type: preferred, driver 96 0 disconnected VGA-1 0x0 0 97 $ ./modetest -s 85:1920x1080 -s 93:1920x1200 -s 87:1680x1050 -C trying to open device 'i915'...failed trying to open device 'amdgpu'...failed trying to open device 'radeon'...failed trying to open device 'nouveau'...done setting mode 1920x1080-60.01Hz on connectors 85, crtc 50 starting cursor cursor stopped This is the change on top of 1225171b (master): diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index fc75383a..cdba7b4e 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -1730,14 +1730,14 @@ static void set_cursors(struct device *dev, struct pipe_arg *pipes, unsigned int int ret; /* maybe make cursor width/height configurable some day */ - uint32_t cw = 64; - uint32_t ch = 64; + uint32_t cw = 256; + uint32_t ch = 256; /* create cursor bo.. just using PATTERN_PLAIN as it has * translucent alpha */ bo = bo_create(dev->fd, DRM_FORMAT_ARGB8888, cw, ch, handles, pitches, - offsets, UTIL_PATTERN_PLAIN); + offsets, UTIL_PATTERN_SMPTE); if (bo == NULL) return;
[+emersion, -various people and lists who definitely don't care] On Wed, Feb 24, 2021 at 4:09 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > Ilia Mirkin, Tue, Feb 23, 2021 19:13:59 +0100: > > On Tue, Feb 23, 2021 at 11:23 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > > > > > $ xrandr --listproviders > > > Providers: number : 1 > > > Provider 0: id: 0x68 cap: 0x7, Source Output, Sink Output, Source Offload crtcs: 4 outputs: 5 associated providers: 0 name:nouveau > > > > > > And yes, the cursor looks good in v5.11 even without reverting the commit. > > > > FWIW it's not immediately apparent to me what grave error modesetting > > is committing in setting the cursor. The logic looks perfectly > > reasonable. It's not trying to be fancy with rendering the cursor/etc. > > > > The one thing is that it's using drmModeSetCursor2 which sets the > > hotspot at the same time. But internally inside nouveau I think it > > should work out to the same thing. Perhaps setting the hotspot, or > > something in that path, doesn't quite work for 256x256? [Again, no > > clue what that might be.] > > > > It might also be worthwhile just testing if the 256x256 cursor works > > quite the way one would want. If you're interested, grab libdrm, > > there's a test called 'modetest', which has an option to enable a > > moving cursor (-c iirc). It's hard-coded to 64x64, so you'll have to > > modify it there too (and probably change the pattern from plain gray > > to any one of the other ones). > > I am interested, so I did. > > If I start the test without X running, the sprite of 256x256 cursor always > contained horizontal lines across it, both with commit reverted and vanilla > v5.11. Similarly, the 64x64 cursor has no lines across it in both kernels. > > The test does not seem to work at all if there is an X server running (using > modesetting driver): modetest complained about permission denied to set the > mode, and just sits there, drawing nothing on the displays. > So I could not run the test in the environment of original problem. > Am I starting it correctly? Is the change in modetest.c correct? Looks right. Although TBH I'd just start it on a single display (I forgot you could even do multiple displays). You should be able to start it with the X server running if you switch to a vt (and run it as root). If you can't, that means the modesetting driver is forgetting to do something in the LeaveVT function. The fact that you're getting lines with modetest means there's something wrong with 256x256. What if you do 128x128 -- does that work OK? Simon, you tested on a GK208, that has a slightly later display controller than the GK104 -- can you try to reproduce Alex's results? Perhaps there was a problem with the GK10x's and it starts working OK with the GK110 disp. I don't have any GK10x's in my posession (I have nearly every other iteration of the display controller), but hopefully someone on the nouveau team will be able to dig one up and reproduce. Thanks for testing, Alex! > > $ ./modetest -c |grep '^[0-9]\|preferred' > 85 86 connected LVDS-1 340x190 13 86 > #0 1920x1080 60.01 1920 2010 2070 2226 1080 1086 1095 1142 152540 flags: phsync, nvsync; type: preferred, driver > 87 89 connected DP-1 470x300 18 88, 89 > #0 1680x1050 59.88 1680 1728 1760 1840 1050 1053 1059 1080 119000 flags: phsync, nvsync; type: preferred, driver > 90 0 disconnected DP-2 0x0 0 91, 92 > 93 95 connected DP-3 520x320 10 94, 95 > #0 1920x1200 59.95 1920 1968 2000 2080 1200 1203 1209 1235 154000 flags: phsync, nvsync; type: preferred, driver > 96 0 disconnected VGA-1 0x0 0 97 > > $ ./modetest -s 85:1920x1080 -s 93:1920x1200 -s 87:1680x1050 -C > trying to open device 'i915'...failed > trying to open device 'amdgpu'...failed > trying to open device 'radeon'...failed > trying to open device 'nouveau'...done > setting mode 1920x1080-60.01Hz on connectors 85, crtc 50 > starting cursor > > cursor stopped > > This is the change on top of 1225171b (master): > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c > index fc75383a..cdba7b4e 100644 > --- a/tests/modetest/modetest.c > +++ b/tests/modetest/modetest.c > @@ -1730,14 +1730,14 @@ static void set_cursors(struct device *dev, struct pipe_arg *pipes, unsigned int > int ret; > > /* maybe make cursor width/height configurable some day */ > - uint32_t cw = 64; > - uint32_t ch = 64; > + uint32_t cw = 256; > + uint32_t ch = 256; > > /* create cursor bo.. just using PATTERN_PLAIN as it has > * translucent alpha > */ > bo = bo_create(dev->fd, DRM_FORMAT_ARGB8888, cw, ch, handles, pitches, > - offsets, UTIL_PATTERN_PLAIN); > + offsets, UTIL_PATTERN_SMPTE); > if (bo == NULL) > return; >
Ilia Mirkin, Wed, Feb 24, 2021 16:10:57 +0100: > On Wed, Feb 24, 2021 at 4:09 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > Ilia Mirkin, Tue, Feb 23, 2021 19:13:59 +0100: > > > It might also be worthwhile just testing if the 256x256 cursor works > > > quite the way one would want. If you're interested, grab libdrm, > > > there's a test called 'modetest', which has an option to enable a > > > moving cursor (-c iirc). It's hard-coded to 64x64, so you'll have to > > > modify it there too (and probably change the pattern from plain gray > > > to any one of the other ones). > > > > I am interested, so I did. > > > > If I start the test without X running, the sprite of 256x256 cursor always > > contained horizontal lines across it, both with commit reverted and vanilla > > v5.11. Similarly, the 64x64 cursor has no lines across it in both kernels. > > > > The test does not seem to work at all if there is an X server running (using > > modesetting driver): modetest complained about permission denied to set the > > mode, and just sits there, drawing nothing on the displays. > > So I could not run the test in the environment of original problem. > > Am I starting it correctly? Is the change in modetest.c correct? > > Looks right. Although TBH I'd just start it on a single display (I > forgot you could even do multiple displays). You should be able to > start it with the X server running if you switch to a vt (and run it > as root). If you can't, that means the modesetting driver is > forgetting to do something in the LeaveVT function. Tried that and yes, modetest works. Even without running it as root. > The fact that you're getting lines with modetest means there's > something wrong with 256x256. What if you do 128x128 -- does that work > OK? Yes. Unfortunately in both cases. > Simon, you tested on a GK208, that has a slightly later display > controller than the GK104 -- can you try to reproduce Alex's results? > Perhaps there was a problem with the GK10x's and it starts working OK > with the GK110 disp. > > I don't have any GK10x's in my posession (I have nearly every other > iteration of the display controller), but hopefully someone on the > nouveau team will be able to dig one up and reproduce. > > Thanks for testing, Alex! You're welcome! As I'm stuck with this particular machine for foreseeable future, you can count on mine GK10x as well.
On Wed, Feb 24, 2021 at 11:35 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > Ilia Mirkin, Wed, Feb 24, 2021 16:10:57 +0100: > > The fact that you're getting lines with modetest means there's > > something wrong with 256x256. What if you do 128x128 -- does that work > > OK? > > Yes. Unfortunately in both cases. Just to be crystal clear -- are you saying that 128x128 works or does not work? (You said "yes", which would imply it works OK, but then you said both cases, which would imply doesn't work since 256x256 doesn't work?) Thanks, -ilia
Ilia Mirkin, Wed, Feb 24, 2021 17:48:39 +0100: > On Wed, Feb 24, 2021 at 11:35 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > Ilia Mirkin, Wed, Feb 24, 2021 16:10:57 +0100: > > > The fact that you're getting lines with modetest means there's > > > something wrong with 256x256. What if you do 128x128 -- does that work > > > OK? > > > > Yes. Unfortunately in both cases. > > Just to be crystal clear -- are you saying that 128x128 works or does > not work? (You said "yes", which would imply it works OK, but then you > said both cases, which would imply doesn't work since 256x256 doesn't > work?) Modetest with 128x128 cursor works. Without damage to the cursor: modetest shows normal cursor in vanilla v5.11. Modetest also shows normal cursor in vanilla v5.11 with the commit reverted. Which is unfortunate, as the only way to see the damaged cursor is to start the X environment with modesetting DDX driver.
On Wed, Feb 24, 2021 at 11:53 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > Ilia Mirkin, Wed, Feb 24, 2021 17:48:39 +0100: > > On Wed, Feb 24, 2021 at 11:35 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > > Ilia Mirkin, Wed, Feb 24, 2021 16:10:57 +0100: > > > > The fact that you're getting lines with modetest means there's > > > > something wrong with 256x256. What if you do 128x128 -- does that work > > > > OK? > > > > > > Yes. Unfortunately in both cases. > > > > Just to be crystal clear -- are you saying that 128x128 works or does > > not work? (You said "yes", which would imply it works OK, but then you > > said both cases, which would imply doesn't work since 256x256 doesn't > > work?) > > Modetest with 128x128 cursor works. Without damage to the cursor: modetest > shows normal cursor in vanilla v5.11. Modetest also shows normal cursor in > vanilla v5.11 with the commit reverted. But modetest with 256x256 doesn't work (correctly) right? Or did I misunderstand? All the patch does is allow those large cursors to be used, which gets reported via drm APIs and modesetting picks the largest cursor available. (And actually I think it's even not required to use the large cursors, it just controls what's reported in the defaults to userspace.) Thanks, -ilia
Ilia Mirkin, Wed, Feb 24, 2021 17:57:41 +0100: > On Wed, Feb 24, 2021 at 11:53 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > Ilia Mirkin, Wed, Feb 24, 2021 17:48:39 +0100: > > > Just to be crystal clear -- are you saying that 128x128 works or does > > > not work? (You said "yes", which would imply it works OK, but then you > > > said both cases, which would imply doesn't work since 256x256 doesn't > > > work?) > > > > Modetest with 128x128 cursor works. Without damage to the cursor: modetest > > shows normal cursor in vanilla v5.11. Modetest also shows normal cursor in > > vanilla v5.11 with the commit reverted. > > But modetest with 256x256 doesn't work (correctly) right? Or did I > misunderstand? Right. That's why I was asking if I did everything right: it was just corrupted in both kernels. > All the patch does is allow those large cursors to be used, which gets > reported via drm APIs and modesetting picks the largest cursor > available. (And actually I think it's even not required to use the > large cursors, it just controls what's reported in the defaults to > userspace.) Maybe something in X code is not prepared to handle the kernel reporting large cursor support? Even though 128x128 is pretty large, and I don't think I even use that large cursors in X configuration. How can I check?
On Wed, Feb 24, 2021 at 12:03 PM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > Ilia Mirkin, Wed, Feb 24, 2021 17:57:41 +0100: > > On Wed, Feb 24, 2021 at 11:53 AM Alex Riesen <alexander.riesen@cetitec.com> wrote: > > > Ilia Mirkin, Wed, Feb 24, 2021 17:48:39 +0100: > > > > Just to be crystal clear -- are you saying that 128x128 works or does > > > > not work? (You said "yes", which would imply it works OK, but then you > > > > said both cases, which would imply doesn't work since 256x256 doesn't > > > > work?) > > > > > > Modetest with 128x128 cursor works. Without damage to the cursor: modetest > > > shows normal cursor in vanilla v5.11. Modetest also shows normal cursor in > > > vanilla v5.11 with the commit reverted. > > > > But modetest with 256x256 doesn't work (correctly) right? Or did I > > misunderstand? > > Right. That's why I was asking if I did everything right: it was just corrupted > in both kernels. OK. So 128x128 works, 256x256 does not. Interesting. > > > All the patch does is allow those large cursors to be used, which gets > > reported via drm APIs and modesetting picks the largest cursor > > available. (And actually I think it's even not required to use the > > large cursors, it just controls what's reported in the defaults to > > userspace.) > > Maybe something in X code is not prepared to handle the kernel reporting > large cursor support? Even though 128x128 is pretty large, and I don't think > I even use that large cursors in X configuration. How can I check? Yes, 64x64 is enough for anyone (or was it 640kb?) But it's unlikely to be an issue. I believe that AMD also exposes 256x256 cursors depending on the gen: display/dc/dce100/dce100_resource.c: dc->caps.max_cursor_size = 128; display/dc/dce110/dce110_resource.c: dc->caps.max_cursor_size = 128; display/dc/dce112/dce112_resource.c: dc->caps.max_cursor_size = 128; display/dc/dce120/dce120_resource.c: dc->caps.max_cursor_size = 128; display/dc/dce60/dce60_resource.c: dc->caps.max_cursor_size = 64; display/dc/dce60/dce60_resource.c: dc->caps.max_cursor_size = 64; display/dc/dce60/dce60_resource.c: dc->caps.max_cursor_size = 64; display/dc/dce80/dce80_resource.c: dc->caps.max_cursor_size = 128; display/dc/dce80/dce80_resource.c: dc->caps.max_cursor_size = 128; display/dc/dce80/dce80_resource.c: dc->caps.max_cursor_size = 128; display/dc/dcn10/dcn10_resource.c: dc->caps.max_cursor_size = 256; display/dc/dcn20/dcn20_resource.c: dc->caps.max_cursor_size = 256; display/dc/dcn21/dcn21_resource.c: dc->caps.max_cursor_size = 256; display/dc/dcn30/dcn30_resource.c: dc->caps.max_cursor_size = 256; which should have the equivalent effect. But since you're seeing issues with modetest as well (which uses the ioctl's pretty directly), presumably Xorg is not to blame. It's easy enough to adjust the kernel to report a lower size (and reject the larger cursors), I just want to understand which gens are affected by this. Cheers, -ilia
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;
Cc: Martin Peres <martin.peres@free.fr> Cc: Jeremy Cline <jcline@redhat.com> Cc: Simon Ser <contact@emersion.fr> Signed-off-by: Lyude Paul <lyude@redhat.com> --- drivers/gpu/drm/nouveau/dispnv50/disp.c | 8 ++++++++ 1 file changed, 8 insertions(+)