00:12 imirkin: i should check if vieux got killed by the recent updates to drawing...
00:13 imirkin: i just have a nv5 plugged in, so i can only test the purely swtnl paths, hopefully that's enough
00:14 Lyude: .......
00:14 Lyude: i just realized what's probably happening with the cursor surface
00:15 Lyude: the mapping from the gpu to the cpu that we copy the cursor data from is made of 4k pages, but the original bo allocation is not
00:15 Lyude: so when the ce is copying the data back into vram, it's arranging it as if it's 4k pages
00:15 Lyude:face palm
00:55 Lyude: btw - just to confirm, the modesetting ddx uses dumb bos correct?
00:56 airlied: yes
00:56 airlied: or that maybe a lie
00:56 airlied: since it uses glamor/GL so it should havea non-dumb path
00:57 Lyude: mhhh - i'm going to double check that this actually fixes the modesetting issues then
00:58 Lyude: also fwiw - looks like I was right about ovlys actually needing large pages. so I guess that mystery is finally solved
00:59 Lyude: imirkin: can you think of an easy way to make a 256x256 cursor appear on X?
01:02 imirkin: not offhand, sorry
01:22 Lyude: I think I'm just going to limit the cursor size then. I've got a good understanding of the issue now, and I'm a bit worried that if anyone is allocating cursor bos outside of the dumb bo path they'll still see broken cursors
01:23 Lyude: at some point I'll just write up some code so that we migrate the memory to a new bo with small pages for curs and a new bo with large pages for ovly, but not today
01:23 Lyude: glad to know i've finally got the ovly weirdness on kepler figured out though
01:35 Lyude: btw skeggsb - I just noticed that it seems like kepler is suddenly extremely slow on gnome-shell, I'm not sure but this might be a performance regression?
01:53 Lyude: skeggsb, imirkin - cursor fix on the list
02:01 imirkin: Lyude: fwiw, your patch doesn't actually limit the cursor size
02:01 imirkin: it just adjusts the "default" cursor size reported to userspace
02:02 Lyude: do we not just use cursor_width/height in our atomic checks?
02:02 imirkin: i assume not
02:02 imirkin: since 256x256 cursors work fine with a kernel that reports 64x64 as the size
02:03 imirkin: we validate that the layout() function doesn't return false
02:03 imirkin: or error or whatever
02:03 imirkin: curs_layout
02:03 imirkin: and that will only allow the known sizes
02:03 Lyude: ahhhh, it's getting late so I'll amend the patch tomorrow
02:03 skeggsb: Lyude: you don't have to move anything, that's not the proper fix, the proper fix is to punt the cursor plane through nv50_wndw_ctxdma_new() like every other plane, and use that ctxdma
02:04 imirkin: Lyude: nah, i think your patch is fine -- just the description is off
02:04 skeggsb: that won't work on nv50 though (single ctxdma for *every* surface/cursor/lut/etc on the core channel)
02:04 skeggsb: g84 fixes that
02:09 imirkin: damo22: if you feel like testing stuff, you can grab my (updated) branch -- it should pass the tests now. but i'm not sure exactly what fixes it -- the top 4 commits are various fixes. if you could figure out which ones are needed to have the fix, that'd be awesome
02:09 imirkin: (each one is a different theory as to what the problem was with the original approach)
02:13 imirkin: [and if you don't, that's totally fine too ... i think pmoreau was going to find some time to do it]
02:30 Lyude: skeggsb: I wasn't talking about a new ctxdma - I was talking about literally migrating the bo to a different page size
02:30 skeggsb: that seems like a lot more round-about way of fixing the problem though
02:32 Lyude: skeggsb: tbh I also just kind of assumed a different ctxdma probably wouldn't work and would just cause the RM to fail with an actual error (iirc you had mentioned that previously?
02:33 skeggsb: i'm not actually sure it shouldn't work now, it actually might
02:33 Lyude: ahhh - definitely would be worth trying
02:34 Lyude: skeggsb: also we don't need to care about nv50 for that - at least for cursors, we don't even support >64x64 there
02:34 Lyude: or the hw doesn't I mean
02:34 skeggsb: yeah i know, just saying, the code is very much wired for nv50-behaviour as far as that goes for cursor
02:36 Lyude: ah-well I can always just have separate codepaths for nv50 and everything else to determine whether we share a ctxdma with the main vram ctxdma or not
02:39 Lyude: i'll make sure to update the patch description with the ctxdma stuff tomorrow btw
02:47 damo22: imirkin: sorry i have to work now
02:55 damo22: imirkin: looking at the commit history, the last two commits were force pushed, so the fix would have to be in the last two commits
02:56 damo22: * ba744eb9c52 (imirkin/nv50_compute) nv50/ir: don't load value at same time as attempting to lock
02:56 damo22: * cf102d69159 nv50/ir: emit store unlock separately from regular store
02:56 damo22: | * d24ae47f789 (HEAD) nv50/ir: don't load value at same time as attempting to lock
02:56 damo22: | * 492b80777dc nv50/ir: emit store unlock separately from regular store
02:56 damo22: |/
02:57 damo22: ie below that is all the same
03:01 damo22: Passed: 4/4 (100.0%)
03:23 imirkin: damo22: yeah, there was a bug in the one i amended
03:23 imirkin: which would have made the overall thing be broken
03:23 imirkin: one of the top 4 commits is the one that fixes it
08:48 kherbst: imirkin: mhhhh.. seems like removing that loop you suggested caused other issues :/
09:26 kherbst: imirkin, tagr: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9425
09:26 kherbst: master still broken but at least this fixes 21.0
09:28 kherbst: and I guess I should deal with the other resources as well
09:28 kherbst: (will add it to this MR)
09:42 kherbst: ahh, doesn't matter
09:42 kherbst: tegra doesn't expose
09:43 kherbst: oh wait...
09:43 kherbst: tegra just asks nvc0
09:43 kherbst: ...
09:44 kherbst: ehh.. checked the wrong member anyway
14:33 imirkin: kherbst: i'd like to see the patch which removes the loop at the top of tegra's draw_vbo and doesn't work
14:34 imirkin: i'm guessing you did something wrong
14:34 kherbst: probably
14:35 kherbst: you mean just removing the clause isn't correct? damn
14:35 imirkin: i dunno
14:35 imirkin: hm
14:36 imirkin: yeah, the if (...) return; condition seems off
14:36 imirkin: i think you need to get rid of the draws[0].count check
14:36 kherbst: the problem wasn't that it breaks something visually, just that nvc0 crashed and stuff
14:36 kherbst: maybe
14:37 imirkin: hmmmm
14:37 imirkin: then why doesn't nvc0 crash already?w eird
14:37 kherbst: well.. the "crash" was that the pushbuffer misses a bo somewhere or we called validate with a null bo? uhm.. something weird
14:38 imirkin: ok. well don't worry about it. "something weird".
14:38 kherbst: https://gist.github.com/karolherbst/4e51f0fb61e530ec741ef844cfee8bbc
14:38 kherbst: yeah
14:39 imirkin: and yeah, this isn't a weird case. num_draws == 1
14:39 imirkin: so it's doubly weird
14:39 kherbst: yep
14:39 imirkin: since the if () at the top wouldn't trigger
14:39 imirkin: oh
14:39 imirkin: this was inside a weird bisect?
14:39 imirkin: cso_draw_arrays used to have an issue
14:40 imirkin: ah, but this is cso_multi_draw. hm.
14:40 kherbst: ahh, yeah, I picked the first stacktrace matching bo=0 in the console output :D\
14:40 kherbst: currently not at home so I have to use whatever is in the scrollback :D
14:40 imirkin: yea no worries
14:41 kherbst: imirkin: I pushed the indirect draw thing.. I already wrote the commit just didn't push it
14:41 kherbst: anyway.. the modifier patches break tegra on master :/
14:41 imirkin: yay
14:41 kherbst: _but_ at least there is no blocker bug for that _yet_
14:41 kherbst: *sigh*
14:42 kherbst: making a nouveau bug a blocker, the nervs :D
14:42 imirkin: ;)
14:42 kherbst: blocker for fedora34...
16:43 imirkin: hrmph. we should double-check that nv3x draws didn't get messed up like they did on nv50/nvc0
16:44 imirkin: btw - what piece of software is this - gnome-shell? https://gitlab.freedesktop.org/mesa/mesa/-/issues/4397
17:07 RSpliet: Looks like Unity
17:07 RSpliet: Or... a Unity shell on Gnome Shell?
17:07 RSpliet: FrankenUnity?
17:09 imirkin: FrankenShell!
17:10 RSpliet: Either way, I think they're both based on Mutter these days
21:39 Lyude: skeggsb: thinking about what you mentioned w/r/t using separate ctxdmas for the cursor. I just kind of realized though, it seems like kmsVramCtxDma doesn't have an explicit page type specified which would mean it's 0x00 - which means shouldn't it be defaulting to large pages already?
21:39 Lyude: in which case it seems like if cursors worked with large pages, we'd currently only be having issues with smaller cursors
21:42 Lyude: (mostly want to know so I have the correct description for this patch)
21:50 skeggsb: if (size == 0) {
21:50 skeggsb: if (dmaobj->base.target != NV_MEM_TARGET_VM) {
21:50 skeggsb: kind = GF119_DMA_V0_KIND_PITCH;
21:50 skeggsb: page = GF119_DMA_V0_PAGE_SP;
21:50 skeggsb: that's what happens for unspecified
21:51 Lyude: ooooh, i see
22:10 imirkin: pmoreau: hey, did you get a chance to test to see which of the top 4 commits fixes it?