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