00:44 karolherbst: plugging in those super old GPUs makes me always feel like that even though stuff is still not perfect, it's a lot better today :D
00:57 karolherbst: lol
00:57 karolherbst: "LTS" but ethernet doesn't work unless I restart the services
01:06 karolherbst: imirkin: yeah sooo.. https://gitlab.freedesktop.org/drm/nouveau/-/issues/123 doesn't trigger on my g73 :/
01:15 imirkin: not extremely surprising
01:16 karolherbst: nv68 being that "special" or something?
01:16 karolherbst: I mean.. despite it not having dedicated VRAM
01:18 imirkin: could be the video thing
01:19 karolherbst: mhh
01:19 karolherbst: but if it happens in this simple game as well?
01:20 imirkin: yea dunno
01:26 karolherbst: I wouldn't be surprised if we somehow manage to trash sysmem there
01:27 karolherbst: imirkin: btw.. if you are willing to try out a newer kernel on fermi, skeggsb has a fix for the fermi regression :D
01:28 imirkin: well, i'm sure if he tested it, it works
01:29 karolherbst: that's kind of the problem, nobody tested it yet
01:29 karolherbst: I would, but...
01:30 imirkin: :)
01:31 imirkin: did he send it to the ML?
01:31 karolherbst: not yet, but I have the link to it... https://paste.centos.org/view/4af46f59
01:32 imirkin: delightful, so gt215 through fermi is broken?
01:32 karolherbst: nope
01:32 karolherbst: not even all fermi is
01:32 karolherbst: it took me like 15 minutes to understand this patch.. it's.... complicated
01:32 imirkin: oh, it's the stupid second CE engine
01:33 karolherbst: yeah..
01:33 imirkin: which is actually a decompression engine?>
01:33 imirkin: annoying.
01:33 karolherbst: yes
01:33 karolherbst: but it got -1 passed in as inst in the gf108_ ctor
01:33 karolherbst: not 0 or 1
01:33 karolherbst: so gf108+ used 103000 as the base address _and_ the ce1 func pointer struct
01:33 imirkin: so as it happens
01:33 karolherbst: the gt215 change is just to print "ce" instead of "ce0" :D
01:34 imirkin: i do have a GT215 and GF108 plugged in
01:34 karolherbst: it's just an implication of the second part of the patch
01:34 karolherbst: so without it nouveau would start printing ce0
01:35 karolherbst: testing on gf108 is I think good enough
01:35 karolherbst: just making sure that the patch actually works
01:35 karolherbst: I think it does.. it's just hard to see
01:36 karolherbst: soo.. the problem is, that gf108+ has only one ce engine, sooo.. chip->ptr.inst is 1
01:36 karolherbst: gt215 was fine, as inst didn't had any implication in the ce ctor
01:37 imirkin: ah, that inst trives the xyz_ctor value, i see
01:37 karolherbst: even though 1 was passed in, it got fixed up to -1 later in the process
01:37 karolherbst: well, or will now anyway
01:37 karolherbst: well..
01:37 karolherbst: no
01:37 karolherbst: inst just is the count of engines in an array
01:37 karolherbst: I mean..
01:37 imirkin: i figured it was the engine index
01:38 karolherbst: device->chip->ptr.inst is
01:38 imirkin: i.e. is it the 0'th, 1st, etc CE engine
01:38 karolherbst: the count
01:38 karolherbst: and then (j) gets passed in as inst in the ctor
01:38 karolherbst: we just special treat devices which only have one engine of those "array engines"
01:38 imirkin: right
01:38 karolherbst: and either gt215 or gf108 could work with the old code
01:38 imirkin: we have to pay special attention to the ones that have a second CE which is not a CE
01:39 karolherbst: that's actually fine :D
01:39 karolherbst: gf100 is not broken
01:39 karolherbst: as inst is 2
01:39 karolherbst: and you don't pass in -1
01:39 imirkin: ah.
01:39 imirkin: i see.
01:39 imirkin: it was getting an unexpected inst.
01:39 karolherbst: well... kind of yes
01:39 karolherbst: the problem is just that the gf108 ctor does something with inst
01:40 imirkin: right
01:40 karolherbst: ehh gf100
01:40 imirkin: which makes sense.
01:40 karolherbst: yes
01:40 imirkin: since the diff CE's are at diff addresses
01:40 imirkin: otherwise how could you address them :)
01:40 karolherbst: it was just fine for gt215 as it didn't care about the inst value
01:40 imirkin: makes sense.
01:40 karolherbst: yeah
01:42 karolherbst: gf100 actually has 3 of them.. no idea why
01:42 karolherbst: heh
01:42 karolherbst: g104 as well
01:42 karolherbst: soo
01:42 imirkin: 2 copy engines
01:42 imirkin: 1 decompress
01:43 imirkin: the decompress one never panned out afaik
01:43 imirkin: er, it might be both compress/decompress
01:43 karolherbst: gf108, gf106, gf116, gf117, gf119are broken as it seems
01:43 imirkin: ok. well only GF108 here.
01:43 karolherbst: the other fermis are fine
01:43 karolherbst: so I guess high perf GPUs get more stuff on them
01:43 karolherbst: :D
01:44 imirkin: well ... the theory was that the compress/decompress was to improve PCIe bandwidth
01:44 karolherbst: ahh
01:44 imirkin: i.e. you use cpu to compress, write to the board, and the board decompresses
01:44 imirkin: at least that was my undersatnding
01:44 karolherbst: ohh so texture compression stuff?
01:44 imirkin: no
01:44 imirkin: this is gzip
01:44 karolherbst: :O
01:44 imirkin: so just data, generically
01:44 karolherbst: oh wow
01:44 imirkin: but i guess it didn't pan out in practice
01:45 imirkin: i don't think blob drivers ever made use
01:45 imirkin: and it's gone from later GPUs
01:45 imirkin: which is a pretty good sign of "didn't pan out" :)
01:45 karolherbst: :D
01:45 karolherbst: I guess there were better alternatives
01:45 imirkin: yeah, like "don't do that"
01:45 imirkin: was a pretty good alternative
01:46 imirkin: PCIe 3.0 came out with Kepler
01:46 karolherbst: well.. there are some compression techniques used on modern GPUs, which I don't think we even make use of, but...
01:46 imirkin: probably had all the bandwidth they wanted
01:46 karolherbst: yeah...
01:46 karolherbst: although PCIe 3.0 isn't extremely fast
01:46 karolherbst: I think that PCIe isn't a real limitation in most cases though anyway
01:47 karolherbst: are you sure this was limited for GPU<->CPU copies?
01:47 karolherbst: *to
01:47 imirkin: that was my understanding.
01:48 imirkin: well, it could do copies whereever
01:48 imirkin: but the idea was to use it for GPU <-> CPU
01:48 karolherbst: mhh, I see
01:51 imirkin: if PCIe 3.0 isn't extremely fast, i can only imagine how you felt about PCIe 2.0
01:53 karolherbst: I mean... PCIe 3.0 is like 16 GB/s? :D
03:12 karolherbst: Lyude, skeggsb: do we want to translate an approval on an MR as an acked-by?
03:12 karolherbst: or reviewed-by?
03:15 imirkin: karolherbst: you should get people in that gitlab issue to test the patch :)
03:15 karolherbst: I guess, but I wanted to wait until skeggsb post it
03:15 karolherbst: *posts
03:15 imirkin: fair enough
03:17 karolherbst: I am fairly convinced that the patch fixes at least _some_ issue, just wanted quick feedback on it :D
03:24 imirkin: ok
15:42 Lyude: karolherbst: I'm fine with it being reviewed-by
16:34 karolherbst: Lyude: and I guess it's fine to use your public gitlab email address for it? (It's the RH one)
16:35 Lyude: yep, fine by me
16:35 karolherbst: but I am still wondering a little about this from a process perspective and what people/upstream think it's okay here :/ Can't just use assume people are fine with it or maybe we can just assume it or something... dunno..., but I also don't know who is allowed to even approve things :D
16:36 karolherbst: anyway.. my idea was to script this with dim, so one doesn't have to do it manually
16:37 karolherbst: "GitLab allows all users with Developer or greater permissions to approve merge requests. Approvals in GitLab Free are optional, and don’t prevent a merge request from merging without approval." okay....
16:37 karolherbst: so we control the list of people allowed to do this, nice
17:52 Lyude: karolherbst: I mean - to me "Approve" on gitlab means reviewed-by, otherwise I don't see how we would reasonably use approvals for patch review since it'd be not great if all of our patches lack R-Bs and only have A-Bs
17:53 Lyude: For things where people actually want to provide acks, it would make sense to also parse Acked-by: tags, and also parse Reviewed-by: tags in gitlab comments as well just for completeness
17:57 karolherbst: Lyude: yeah.. but doing that stuff from comments is what I don't think we want to automate
17:57 karolherbst: or script
17:57 karolherbst: because of those details like comments only for specific commits and stuff
17:58 karolherbst: or if we start to allow certain patterns, then some are missed or whatever
17:58 karolherbst: but yeah.. I think making an MR approval to be a r-by makes sense
17:58 karolherbst: just the gitlab API sucks and requires public email addresses :)
17:59 Lyude: we do work with gitlab, if that's an issue it's definitely something I'd consider mentioning to them
17:59 karolherbst: it's not an issue, it's just that random users are not supposed to get your email address unless you made one public :p
18:00 karolherbst: the only feature I miss atm is commenting on a commit itself not just lines
18:00 karolherbst: but.. oh well
18:01 karolherbst: fetching the email is really no issue, just...
18:01 karolherbst: $ gitlab -o json user get --id 1147 | jq .public_email => "kherbst@redhat.com"
18:01 karolherbst: $ gitlab -o json user get --id 1146 | jq .public_email => ""
18:01 karolherbst: :D
18:01 karolherbst: but this isn't a technical issue
18:02 karolherbst: I just need to poke skeggsb as Ben doesn't have a public email set yet
18:22 karolherbst: ....
18:22 karolherbst: this API...
19:11 karolherbst: sooo.. done :D
19:38 juri_: yay? :)
20:17 karolherbst: Lyude: ohh there is one thing.. gitlab could add the public_email address when fetching approvers :D
20:17 karolherbst: that would safe me a http call per approver
20:17 karolherbst: *save