00:56Wally: https://paste.centos.org/view/3455c947 Why wont this diff work for adding HMMA.884 to ir, is everything required not being emitted etc?
00:58imirkin: putting stuff after OP_LAST is unlikely to end well
00:58imirkin: also you're only emitting the instruction but none of its args
00:59Wally: How would I emit args?
00:59imirkin: emitGPR and whatnot? look at the other ops
02:13Wally: Sorry that went way over my head, main thing I dont understand is what does emitField[blah] do in the context of emitting args?
02:19imirkin: well, it just sets bit
02:19imirkin: bits*
02:19imirkin: you have an instruction
02:19imirkin: it takes arguments
02:19imirkin: right?
02:19Wally: yes
02:19Wally: 4 registers afaik
02:19imirkin: so ... how are those passed in?
02:20Wally: nvdisasm gave me this https://paste.centos.org/view/4ab025ae
02:20imirkin: right, but ...
02:21imirkin: think about what that emitBLA() is doing
02:21imirkin: it's putting the instruction together
02:21Wally: oh...
02:21imirkin: so ... all the bits need to come from _somewhere_
02:36Wally: STEP0 is 0x00000EEEEE087236, STEP1 is 0x00000EEEEE0a7236, STEP2 is 0x00000EEEEE04723 STEP 3 is 0x00000EEEEE067236
02:37Wally: Where EEEEE is a dependency of what registers are being used
02:40Wally: I have a goodish idea on what set of registers map to what
02:41imirkin: well, it's just bits...
02:41imirkin: some set of bits is the dest reg
02:41imirkin: some set of bits is the src reg
02:41imirkin: etc
02:41imirkin: er, the first src reg
02:42imirkin: second src reg
02:42imirkin: etc
02:42imirkin: various flag bits
02:43Wally: So how do I assemble it?
02:44Wally: An emmitInsn with made by adding onto 0x0000000000007236?
02:44Wally: ;)
02:47imirkin: maybe look at every single other op
02:47imirkin: or any other op
02:47imirkin: and see how it works
02:47imirkin: and then try to apply your learnings here?
12:46karolherbst: okay.. let's fix threading this week for real, shall we
12:47RSpliet:grabs his pom-poms and cheers on karolherbst
12:48karolherbst: I already have something that works, but it's regressing nv50 for very very stupid reasons
12:51karolherbst: the biggest issue is though that nouveau_pushbuf_space can push :(
12:51karolherbst: ehh, flush
13:14mupuf: karolherbst: you are on fire this year!
13:14karolherbst: apparently
13:14karolherbst: tomorrow/today will be hard
13:15karolherbst: mhh.. I think tomorrow, but I won't spoiler anything
16:31karolherbst: imirkin: do you remember the reason for vbo using its own kick_notify handler which doesn't call nouveau_fence_next? It might make sense to move the flushing out of kick_notify alltogether and handle that explicitly on context flushes?
19:50Wally: Is this good when inputting 4 registers? https://paste.centos.org/view/2282000e
19:52TimurTabi: Can someone explain what function nvbios_image() does? What is "struct nvbios_image" supposed to represent?
19:52karolherbst: TimurTabi: it parses the parts of the vbios
19:54TimurTabi: Parses it for what? I guess if I were more familiar with the VBIOS layout I would understand it already.
19:54karolherbst: a vbios contains of mulitple parts (that's the pcir handling) and we put that all together
19:54karolherbst: your life gets easier if you assume it's doing the right thing and you simply start using the vbios
19:55Wally: https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/nouveau/nvkm/subdev/bios/image.c#L69
19:56TimurTabi: I'm looking right at that code. Is image->last a flag that indicates this is the last image in VBIOS?
19:56karolherbst: possibly
19:57Wally: TimurTabi: If your looking for well defined answers to things look at the freedreno project
19:57karolherbst: Wally: freedreno also needs to parse vbios like that?
19:57TimurTabi: Well, what I'm really looking for is a memcpy version of nvbios_rd
19:58Wally: idk, but they seem to document more stuff and its codebase seems way easier to hack onto
19:58karolherbst: TimurTabi: that's nvbios_memcmp, no?
19:58TimurTabi: cpy, not cmp
19:58karolherbst: ohhhh.. right...
19:59karolherbst: write a memcpy similiar to nvbios_memcmp?
19:59karolherbst: shouldn't be too hard
19:59TimurTabi: I guess I will
19:59karolherbst: it just loops over nvbios_rd08 anyway
20:00karolherbst: I guess we could be more efficient, but... stuff gets hairy if you load across part
20:00karolherbst: s
20:00TimurTabi: Well, it should be more efficient than that. :-)
20:00karolherbst: point is, it doesn't matter
20:54TimurTabi: It looks like a given BIOS only supports two images, is that correct?
20:55TimurTabi: bios->image0_size is the size of the first image. And any address higher than that is ... the second image?
20:55karolherbst: TimurTabi: it appears that this might be true, yes
20:55karolherbst: although I think we only encountered vbios with two relevant parts (all of those have a part between the first and the last one, we need to skip over)
20:56karolherbst: so if you hit a vbios with multiple ones, we might have a problem :)
21:44TimurTabi: Would anyone care to briefly review my implementation of nvbios_memcpy? https://codeshare.io/Jb8L9r
21:45karolherbst: TimurTabi: why not split the copy and support it across image boundaries?
21:45karolherbst: copy up to the end of the base, and then from begining of the second part
21:45TimurTabi: To avoid calling nvbios_rd08 repeatedly?
21:45karolherbst: sure, but you can do that with two memcpies instead
21:45TimurTabi: And I'm not sure what the point of copying across an image boundary is, anyway.
21:46karolherbst: I don't exactly know either, but the boundary seems arbitrarily placed
21:46karolherbst: so one might have to.. dunno
21:46karolherbst: depends on what you end up doing I guess
21:46TimurTabi: Hmm... I was under the impression that the second image is semantically separate.
21:46karolherbst: I don't know, but I think it's always 0xf00 sized
21:47karolherbst: so I wouldn't be surprised if tables could be inside both images
21:47TimurTabi: Are you saying that the two images are actually the same blob with some completely pointless split between them at some unrelated address?
21:47karolherbst: but that's just what I'd assume to be sure
21:47karolherbst: TimurTabi: yes
21:47TimurTabi: Woah
21:47karolherbst: as I said, you jsut use the offsets from the vbios
21:47karolherbst: so an offset can be in part one (pointing to BIT tables)
21:47karolherbst: and they point into the second image, but you have to adjust the offset
21:48TimurTabi: Why doesn't Nouveau just copy the two halves into one buffer?
21:48karolherbst: if tables can be inside the image so they are split? potentially?
21:48karolherbst: TimurTabi: good question, I guess we could do that instead
21:48karolherbst: would make vbios parsing a lot cheaper
21:49karolherbst: but I am sure there is a reason for the split, we just don't know what that is :)
21:49TimurTabi: nvbios_addr already requires the caller of the rdxx and wrxx functions to assume that they two halves are merged.
21:49karolherbst: _but_
21:49karolherbst: the gap is more or less equally sized to the second part
21:49karolherbst: and it contains random bytes
21:49karolherbst: so it might be some crypto stuff going on
21:50TimurTabi: Yeah, but the current accessors make it impossible to access those bytes, and it's not like the offsets are preserved.
21:50karolherbst: yeah.. I think it makes sense to copy the vbios into a CPU side buffer, so accesses are cheaper
21:50TimurTabi: For example, I could imagine Nouveau merging the two images into one buffer, and then using nvbios_addr to translate the offset into the merged offset.
21:50karolherbst: there might be a logcally split though, but it's still seen as one big image from within at least
21:51TimurTabi: But we're doing the opposite, and just wasting memory.
21:51karolherbst: TimurTabi: you don't have to translate offsets once you merge it
21:51karolherbst: which I think might be one reason rm does copy it
21:51TimurTabi: Well, I would suspect that there is an offset stored in image0 that references a byte in image1 with it's unmerged address
21:51TimurTabi: but I guess not.
21:52karolherbst: no, the offset assumes the vbios without gaps
21:52TimurTabi: Just so weird
21:52karolherbst: that's why we have to adjust offsets in rdXX
21:52karolherbst: yeah...
21:52karolherbst: I never understood why it is like that
21:52karolherbst: but that's how it is
21:52karolherbst: I am sure there is a good reason somewhere
21:52TimurTabi: Do you know offhand which GPUs do this?
21:53karolherbst: it started somewhere with maxwell2/pascal
21:54karolherbst: hence me thinking there is crypto involved :) maybe some checksuming to make sure nothing messes with the vbios
21:54karolherbst: no idea
21:54karolherbst: but that would be plausible
21:55karolherbst: TimurTabi: there is also this feature in old RM where you could upload an arbitrary vbios to VRAM and make the nvidia driver load this instead of the device one, it stopped working around that as well :)
21:55karolherbst: we used it to mess with the tables to figure out what each table is responsible for
21:55TimurTabi: I'm asking one of our VBIOS experts why the gap exists.
21:55karolherbst: yeah, would be cool to understand why that exist at all :)
22:30TimurTabi: I think I found a bug in nvbios_addr
22:30TimurTabi: static bool
22:30TimurTabi: nvbios_addr(struct nvkm_bios *bios, u32 *addr, u8 size)
22:30TimurTabi: {
22:30TimurTabi: u32 p = *addr;
22:30TimurTabi: if (*addr > bios->image0_size && bios->imaged_addr) {
22:30TimurTabi: Should that be "*addr >= bios->image0_size" ?
22:31karolherbst: mhhh
22:32karolherbst: yeah, I think you are right
22:33TimurTabi: Seems like Nouveau never actually tried to copy a buffer that crosses an image boundary.
22:33karolherbst: yeah, the code doesn't really handle it all that well as it seems
22:33TimurTabi: Ok, I'll post a patch
22:34karolherbst: or it does matter and that would explain a few bugs
22:34karolherbst: dunno
22:34karolherbst: TimurTabi: there is another issue with that, as we don't deal with the size argument
22:34TimurTabi: I was wondering about that
22:34karolherbst: what if you load at bios->image0_size - 1, but you load 4 bytes
22:34TimurTabi: it seems that size of the bios includes the gap
22:35TimurTabi: hmmm
22:35TimurTabi: This is why we should merge the two images when copying from VBIOS
22:36karolherbst: yeah
22:36karolherbst: I am not disagreeing with that idea, I'd discuss this with skeggsb_ though :)
22:36karolherbst: anyway
22:36karolherbst: I think it would be a good idea