00:02 alyssa: airlied: I'm not trying to win the argument. I'm explaining why I can't do this anymore.
00:03 alyssa: just, physically I mean
00:05 alyssa: instead of just quietly moving downstream & onto smaller projects
00:05 alyssa: my body can't deal with this anymore
00:06 alyssa: i'm not built for high stress environments, which everything in mesa/mesa is for better or worse
00:06 airlied: do you have some deadlines external to this, adding pressure or something?
00:07 airlied: like I get the I've got to ship something tomorrow, oh CI screwed me over stressors, but I wasn't aware too many of the main mesa devs were in that sort of work environment
00:07 gfxstrand: airlied: She's the sole developer of a highly anticipated driver. That's a hell of a lot of pressure.
00:07 gfxstrand:ought to know
00:07 alyssa: real
00:08 airlied: gfxstrand: but it's not time dependant, a day, or a week doesn't matter in any way really
00:09 gfxstrand: Technically, no. But there's a lot of emotional weight there.
00:09 gfxstrand: How many times has someone asked me to fix vkcube with NAK in the last 6 months?
00:09 gfxstrand: Like, of all of the things in this world, that one literally doesn't matter but people seem to care and they ask.
00:09 gfxstrand: It results in a lot of pressure, regardless of whether or not there are deadlines.
00:10 gfxstrand: Because the deadline becomes ASAP, always.
00:10 gfxstrand: And, yes, there's no particular date by which stuff has to get done but when you feel like you started off 5 years behind, that doesn't much matter.
00:11 jenatali: alyssa: That tool should be buildable on Linux. But let me send that issue to my team and see if anyone can look
00:11 gfxstrand: And the world is full of critics but not very full of people offering real help.
00:11 gfxstrand: Staying sane in the midst of that requires a certain amount of mental/emotional discipline and self-care.
00:12 alyssa: jenatali: thanks
00:12 Company: ... and we're probably all perfectionist enough to expect too much from ourselves
00:12 jenatali: If not, yeah disabling the job would be okay until I'm back at the end of the month (assuming the offending MR can't wait)
00:12 gfxstrand: (Well, there are lots of people who try to offer but trying to offer and able to provide are very different things when it comes to help building a driver stack.)
00:13 gfxstrand: This isn't me complaining. Just saying that "under a lot of pressure" can be true, even without horrible managers breathing down your back. Sometimes a bunch of impatient users is just as bad if not worse.
00:15 gfxstrand: And then you add on self-expectations and.... Yeah, it gets to be a lot.
00:15 alyssa: people are expecting dx12 games to run, like, yesterday
00:15 airlied: yeah maybe we need some positive affirmations for driver developers
00:15 airlied: "3 months doesn't matter, 6 months matters less"
00:16 airlied: "your driver will exist for hopefully 10 years, sprinting now isn't going to make it better"
00:17 airlied: "ignore all users, don't read the comments"
00:17 gfxstrand: Easier said than done...
00:17 gfxstrand: But, yes, it's good to remember those things.
00:21 airlied: gfxstrand: also after the first couple, I think vkcube was more of a troll, I'm pretty sure I had patches for vkcube working on NAK months ago :-)
00:21 airlied: so in that case I did help, but you also said you didn't want that help at that exact time
00:25 airlied: maybe the XDC CI talks could become a workshop, where people can decide to remove some of the stuff that got added, or isn't adding that much value anymore
00:26 gfxstrand: I do think we need to take a very hard look at cost vs. benefit of CI jobs.
00:27 gfxstrand: Where cost isn't just runtime or $$$ spent on runners
00:27 airlied: yeah the 10min goal for runs is not use if it takes 45mins to start the job
00:28 airlied: like I think it's possible at the moment to actually merge a core change inside the 60m timeout, which is way better than it was 3-4 weeks ago
00:30 jenatali: Please do ping me if any of our jobs are (non-infrastructure) being problematic. I haven't been paying attention but since I haven't heard until today I've been assuming we're not causing any issues
00:31 zmike: jenatali: there's https://gitlab.freedesktop.org/mesa/mesa/-/issues/9773 which I hit again today
00:31 zmike: but I'm not sure how to repro
00:32 zmike: or if it's just that runner being way overloaded somehow 🤔
00:33 airlied: yeah seems like runner overload
00:33 jenatali: That's just the runner being overloaded, yeah
00:34 jenatali: If it's consistent we can turn down the fraction but I'd hate to miss out on coverage all the time for the infrequent overload
00:35 zmike: I don't think the fraction matters since it takes 10-15 mins on a "good" run
00:36 zmike: but I've never had any other job hit this
00:36 zmike: so it seems to me something is going on here
00:37 airlied: there's the same runner used in a good and bad run there as well
00:37 airlied: so it's not specific to some runner configuration, so just sounds like other jobs are scheduled on the same machine
00:39 zmike: they're not the same
00:39 zmike: one is 5, the other is 6
00:39 zmike: but yes, scheduling may be a factor
00:39 zmike: we had issues with this before where certain jobs weren't scheduling properly
00:40 airlied: zmike: the first overrun you posted is 6
00:40 jenatali: zmike: The Windows runners are shared among all freedesktop projects that use them. Gstreamer often hammers them
00:41 zmike: 🤕
00:41 zmike: maybe we need a dedicated mesa runner then if we're going to keep adding ci for windows?
00:41 jenatali: Agreed
00:42 jenatali: I don't think we have plans to add more CI at this point
05:00 airlied: gfxstrand: still awaiting an answer on copy prop vs cast deref following in 25536
05:07 kode54: I'm planning to test out 25576
07:58 daniels: well it sure is motivational waking up to a screenful of ‘you’re literally destroying Mesa out of malice’ text. guess I’ll go back to holiday.
08:03 daniels: gfxstrand: too true - ‘And the world is full of critics but not very full of people offering real help.’
08:05 daniels: airlied: I don’t think a workshop would be of any value. the people who want to contribute and discuss already do. the rest is just bullshit from people who never look at the stats, engage with the people working on it, or try anything - just demand to permanently remove a single job because they saw a failure once
08:07 daniels: there’s a discussion to be had about runtime for sure, but that’s mostly about how much testing can really be decimated, and how much more difficult it should be to merge more driver features or new CTS
08:09 daniels: both expand runtime, but without a bunch of either manual effort or labour gone into tooling, there’s no ‘why doesn’t the CI team make my driver testing faster’ answer to ‘why are jobs so slow’ - it’s just (continually) trying to fix it after the fact
08:09 daniels: but this is just as much speaking into the void, so back to enjoying Scotland o/
09:03 pq: "ignore all users, don't read the comments" is what I do with Wayland and HDR specifically. They haven't really come to my home turf yet either, and I sure won't be looking for comments about my work.
09:03 pq: Having practically zero presence in social media is good for you.
09:05 ccr: one can always go and read Phoronix forums for a dose of sane, well articulated and insightful views!
09:07 pq: yeah... I sometimes go there to check that yep, it's still the same - and I *never* read anything that might touch my work.
09:07 ccr: :)
09:07 emersion: mandatory https://xkcd.com/386/
09:07 pq: reddit and whatever I've never even looked at to begin with
09:07 daniels: well yeah, feedback is good and helpful, but ideally it’s at least one of actionable, insightful, or at least friendly and well-intentioned
09:09 emersion: the fact that my users are power-users does mitigate somewhat these issues, i must admit
09:13 soreau: daniels: I hope you're enjoying your vacation . Scotland sounds great!
10:21 MrCooper: mripard: I don't see how the DRM panic handler could "prepare" atomic state; every atomic commit defines the full state for all CRTCs/planes/connectors, so going from the arbitrary state that happens to be active when a panic occurs to the "prepared" state would require going through the full atomic commit machinery, which is likely to run into locking issues if nothing else, in particular if the panic occurs while an atomic commit is being
10:21 MrCooper: processed
10:32 kusma: Since there's been a discussion on CI going on, I just want to bring attention to this: https://gitlab.freedesktop.org/mesa/mesa/-/pipelines?page=1&scope=all&source=schedule
10:32 kusma: The nightly CI runs haven't been passing all jobs in like forever...
10:32 kusma: That's a pretty bad indicator for the health of the CI IMO...
10:36 kusma: Seems the last three months has been particularly rough. July 16th was the last time it seemed reasonably healthy... Anything notable happened around then?
10:36 MrCooper: the only important job in scheduled pipelines is the git-archive one, other than that MR pre-merge pipelines are far more important
10:37 kusma: The nightly runs all jobs, so it's a good indicator for the jobs
10:37 kusma: Good indicator for the health of the jobs, I mean
10:38 daniels: kusma: nightly jobs aren’t the same as pre-merge; they’re the only ones that run both full suites that take too long for pre-merge, and also jobs which are just way too unstable for pre-merge
10:38 kusma: daniels: Sure, but it tells us a lot about the health of things.
10:38 daniels: so given they have much wider coverage, the expectation is that they fail and we fix it up post-hoc
10:39 kusma: Sure. But the same jobs have been failing for weeks on end
10:39 daniels: well yeah, people keep writing buggy code … and also some of those jobs are nightly-only for a reason, because the platform is fundamentally unstable
10:39 daniels: really? I’ve seen
10:39 kusma: And we notice some of the rarer of them when we touch on common code.
10:39 sima: MrCooper, mripard yeah for panic the only realistic thing is that the panic handler just writes into the buffer that's currently being displayed
10:39 sima: and praying it's cpu visible
10:40 sima: everything else needs to take so many locks and touch so much hw state that it's not realistic
10:40 kusma: I'm not saying the nightlies must pass all the time or bust. But it failing for every single nightly for three months, that can't be the goal here.
10:41 MrCooper: as jfalempe pointed out, at least amdgpu can write to the FB even if it's not directly CPU accessible, doesn't help for tiling though
10:41 sima: there was infra for just writing the base address, but unless you exactly match position and size and format it's already too much for most hw/drivers
10:42 kusma: I've been away from upstream CI for a while, but last week when I merged a branch touching common code for the first time in a while... well, that was pretty rough.
10:42 MrCooper: that can happen, isn't directly related to the scheduled pipeline though
10:42 sima: MrCooper, I think we need a "write x/y pixel in this fb" helper and then just add enough helpers for the common formats/tiling layouts
10:42 sima: until it's good enough
11:09 shadeslayer: DavidHeidelberg: re Barcelona hack weekend, when is that? ( I live here, so happy to be a fly on the wall )
11:16 DavidHeidelberg: shadeslayer: just after the XDC, in BCN, near to place where sergi1 lives
11:16 DavidHeidelberg: we booked bigger airbnb, so we should have big living room for hacking space
11:18 shadeslayer: DavidHeidelberg: ah ok, so 21/22/23rd, alright, I can just grab the details over XDC :)
11:21 DavidHeidelberg: shadeslayer: one important thing, if you want to have accomodation for good price, we still have one room free
11:21 shadeslayer: DavidHeidelberg: I don't, I live in Barcelona :)
11:21 DavidHeidelberg: right :D
11:21 shadeslayer: but thanks for the offer!
11:21 DavidHeidelberg: I wasn't sure if here meant Spain or BCN :D
11:22 shadeslayer: ahh, right :)
11:26 HdkR: 13
12:02 Company: https://developer.arm.com/documentation/ka004859/latest/ claims I can use glReadPixels() to get at the contents of a dmabuf
12:02 Company: does that always work or just sometimes?
12:02 Company: a dmabuf successfully imported into an EGLImage of course, not any random dmabuf
12:13 pq: it seems weird to me that TEXTURE_EXTERNAL would be usable with an FBO attachment.
12:17 pq: s/with/as/
12:22 pq: https://registry.khronos.org/OpenGL/extensions/OES/OES_EGL_image_external.txt issue 4 implies that external textures cannot be modified from GL ES, so it seems strange that an FBO could be complete with an external attachment.
12:23 pq: maybe that aadvice is Mali specific
12:23 pq: or maybe there is another extension
12:42 sima: pq, glReadPixels is just reading, so as long as the driver has a fallback path to use the gpu to sample the pixel (since cpu mmap isn't guaranteed) it can be made to work
12:45 sima: but yeah not seeing anything in the spec that would guarantee it does work
12:45 sima: Company, ^^
12:46 emersion: pq, i believe EXTERNAL is read-only
12:47 emersion: pq, EXTERNAL is e.g. used with YUV shader lowering, and that's read-only
12:48 Company: yeah, this is about reading only
12:48 kusma: The GLES 3.2 spec for glReadPixels says "An INVALID_OPERATION error is generated if the read framebuffer is not framebuffer complete."
12:49 emersion: i don't think EXTERNAL works with FBOs
12:50 kusma: You can always opt out of supporting it by reporting GL_FRAMEBUFFER_UNSUPPORTED
12:51 kusma: I mean, maybe technically speaking not (that's *really* just about the formats)...
12:51 Company: emersion: https://registry.khronos.org/OpenGL/extensions/OES/OES_EGL_image_external.txt says it can be used with 2D if https://registry.khronos.org/OpenGL/extensions/OES/OES_EGL_image.txt is supported
12:52 pq: Company, for texturing from. FBOs are about writing into.
12:52 emersion: TEXTURE_2D != FBO
12:52 Company: but no idea, it could be that it only applies to formats where eglQueryDmaBufModifiersEXT() returns FALSE for external_only
12:52 kusma: @pq: No, not only.
12:53 pq: kusma, yes, you can use an FBO to read from a texture, but doesn't FBO completeness require the attachments to be renderable?
12:53 kusma: You can bind an FBO to the read-buffer only, for instance
12:53 emersion: "This extension provides a mechanism for creating EGLImage texture targets from EGLImages."
12:53 Company: rght, this is important in the ARM link above, too
12:53 emersion: this extension does not mention FBOs
12:53 Company: they only bind the read framebuffer
12:54 pq: where an FBO is bound to should not make any difference to whether the FBO is complete, should it?
12:54 kusma: @pq: It requires the texture (and all attachments) to be complete... And that requires it to be color-renderable if it's a color format (and similar for depth / stencil)
12:54 pq: don't you query FBO completeness before binding to anything?
12:55 Company: https://www.khronos.org/opengl/wiki/Framebuffer_Object#Framebuffer_Completeness says different rules apply for read and draw framebuffers
12:55 pq: kusma, right, and EXTERNAL is not renderable. That's the whole point of EXTERNAL that it does not need to be modifiable at all.
12:56 pq: Company, interesting
12:56 kusma: @pq: Renderable is determined by the format, not by the source the image comes from
12:56 Company: lots of usual formats aren't renderable
12:56 kusma: There might be some differences between GL and GLES here, BTW
12:56 pq: kusma, yeah, and we're talking about YUV here, because why else would you ever use EXTERNAL.
12:56 Company: like, the FLOAT32 or INT16 formats often aren't
12:57 kusma: @pq: Yeah, those would probably not be renderable, unless the implementation supports an extension that marks them as such.
12:57 pq: TL;DR: no guarantees at all that what the ARM doc said will work.
12:58 kusma: The ARM docs doesn't seem to mention YUV?
12:59 Company: I'm just trying to find the best way to read a dmabuf into an rgb buffer - ideally without havning to support shaders
13:00 kusma: Company: Could you blit from it?
13:00 emersion: the safest choice is to render the EXTERNAL texture into a FBO
13:00 emersion: then glReadPixels() from that FBO
13:00 kusma: Yeah, that sounds like the easiest option
13:00 Company: kusma: blitting is from the read framebuffer, so I would assume glReaadPixels() works at that point?
13:01 kusma: Company: yeah, fair point
13:02 pq: A rendering pass will also let you do any pixel format (I mean bit depth specifically) conversion you might want. That may even be crucial for glReadPixels performance.
13:02 Company: so far I'm less concerned with performance and more with "will this work?"
13:02 pq: but again, I don't think any EGL spec yet guarantees detailed YUV->RGB conversion, so if it converts automatically, you'll get just "something".
13:03 pq: does "too slow" count as "works"?
13:03 kusma: Company: considering that external images are meant primarily for texturing, texturing with it would probably be the most robust option, no?
13:04 pq: there are many ways to do things that would end up too slow, like import to GBM and gbm_bo_map() (did that even map YUV at all? at least it will not convert to RGB)
13:04 kusma: I think Companysaid that they want RGB output anyway?
13:04 pq: exactly
13:05 Company: yeah, I want to end up with RGB output
13:05 pq: so they want YUV converted to RGB, and doing that on the CPU afterwards is too slow, but still works
13:05 Company: this is about the GTK fallback path, which is used for corner cases, debugging and such
13:05 Company: by default this is going to go into a shader just fine
13:06 pq: just do what emersion said then
13:06 pq: but if it's for debugging, are you sure you want an automatic YUV->RGB conversion?
13:06 Company: but someone might want to print a webcam screenshot into a PDF or whatever
13:07 Company: for now I do, because our infrastructure doesn't handle YUV
13:07 pq: ok
13:07 Company: I seem to be dealing with dmabufs instead of implementing color management support
13:08 pq: I'm not surprised. The things I've have to do with Weston and still need to do...
13:14 MrCooper: pq: "why else would you ever use EXTERNAL", e.g. because some modifiers are supported only with EXTERNAL (e.g. the LINEAR one with the nvidia driver)
13:14 pq: oh, right, I never consider proprietary drivers.
13:16 pq: is that due to not supporting mipmaps?
13:16 pq: or clamp modes
13:16 MrCooper: it's just an example, might even be the same with nouveau, since it's due to the HW not supporting linear colorbuffers together with depth/stencil
13:17 pq: TEXTURE_2D can be read-only too, right?
13:17 emersion: can it?
13:17 emersion: i'd always assume write ops to work with TEXTURE_2D
13:17 pq: yeah, most formats are not renderable, traditionally
13:18 emersion: by "writable", i mean "upload pixel data"
13:18 emersion: not "render to"
13:18 pq: well, not read-only, because you can write to it with TexImage2D
13:19 pq: so they need to forbid TexImage2D into a LINEAR...
13:21 pq: hmm, IOW, if one can import a dmabuf as TEXTURE_2D, you can write into it with TexImage2D? Interesting.
13:22 pq: I mean, TexSubImage2D
13:22 pq: TexImage2D would just drop the dmabuf
13:27 alyssa: rpi3 seems especially unstble today
13:28 alyssa: seen them flake for infra several times today cross idffernet mrs
13:28 alyssa: eric_engestrom: ^
14:01 karolherbst: can we get https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22580 reviewed and merged? I kinda need it. It's a MR to deal with array_derefs on vecs, which are legal spir-v. Not sure if also some Vulkan applications are running into this or not, but probably not, as nothing is crazy enough to do indirects on vecs besides LLVM
14:01 mripard: sima: so we should just drop the ball for some workloads and they'll never get to see any panic message?
14:01 sima: mripard, I think it's a balance between the amount of work and likelihood of having such a case
14:02 mripard: framebuffer compression is common on ARM
14:02 sima: and yeah if your display is in a mode where we can't print without a modeset, that's just it
14:02 sima: mripard, yeah so I guess needs a helper to clear the compression bit and write the raw pixel values
14:02 sima: same for all the other compressed formats
14:03 karolherbst: alyssa: maybe in case you are bored enough? Some CL applications are running into this bug (e.g. darktables) and if we use more CL for shaders, it might also happenr randomly
14:03 sima: but ime trying to flip from compressed to uncompressed in panic is no-go, way too much code you need to run and hw to reprogram
14:03 mripard: is the buffer/mapping going to be big enough to do so?
14:03 sima: mripard, if the buffer isn't mapped already there's nothing you can do
14:03 sima: i.e. if it's not in the kernel linear map
14:04 sima: or we'd need some really funny panic vmap slot which special tlb flush because you really cannot do an ipi for the normal pte flush flow
14:04 sima: maybe the kmap_local would work, dunno
14:05 mripard: I agree, which is why I was saying we should have a separate buffer for that
14:05 sima: I guess could ask ogness and measure the loudness of the resulting scream
14:05 sima: mripard, what does that help?
14:05 mripard: you always have a premapped buffer?
14:05 sima: yeah
14:05 mripard: so you know for certain that you have that one you can use
14:05 sima: but how do you get that thing to be scanned out?
14:06 sima: you can guarantee that you'll be able to write into a preallocated buffer
14:06 sima: you can't guarantee the user will see it
14:06 sima: with the approach of using the buffer the user currently sees, you maximize the latter
14:06 sima: (not all cases, manual upload that's cpu driven is not going to happen)
14:06 mripard: you just change the base address of that plane to that buffer's mapping?
14:07 sima: mripard, also with my proposal in theory the driver could use the premapped approach and do a minimal flip to that, if the hw can do that
14:07 sima: mripard, yeah, if that's all you do then many drivers/hw can help
14:07 sima: but the moment you change format it's already much, much worse
14:08 sima: mripard, it's also not something that we need to do in generic code, since for many gpu the mapping issue isn't one
14:08 sima: like x86 doesn't need to punch out pages from the kernel mapping, worst you need some clflush
14:08 sima: or amd with their peek/poke registers
14:08 sima: (clflush is ok in panic)
14:09 mripard: with dma-buf and dma-heaps, you just can't control if the buffer is accessible by the CPU, at all
14:09 mripard: I guess it's something that x86 can ignore, but that's not the case for the rest of the world
14:10 MrCooper: mripard: "no panic output under some circumstances" is strictly better than "no panic output ever"
14:10 sima: depends upon the gpu, if you have mmio peek/poke it'll work
14:10 sima: yeah that too
14:10 javierm: mripard, sima: but isn't the case anyways with the current VT/fbcon panic messages that those could be lost after a KD_GRAPHICS ?
14:11 sima: javierm, yeah it's all shunted off because it caused way more trouble than where it helped
14:11 sima: I think all the panic you currently get is if a) you're on fbcon b) your kms driver is direct scanout
14:11 sima: there's not a lot of b) left since the helpers switched to shadow buffers for many cases
14:11 javierm: what I'm trying to say that in practice, the panic handler will be useful for very early panic before the initrd even can start a user-space KMS based console
14:12 javierm: because anything after that, is just best effort if the fbcon/VT can recover from a switch from VT
14:12 sima: userspace based console wont print panics either
14:12 sima: that kills the kernel
14:12 sima: oopses can get through, if you're lucky
14:12 mripard: MrCooper: guess I'm just tired to be on the receiving end of "well it works for i915 but we never really bothered with ARM so you'll have to do it all again"
14:12 jfalempe: javierm, yes that's my primary focus, when you messed up the initrd. so the panic with simpledrm already works for this case.
14:13 sima: mripard, the thing is I think you can extend it to the arm cases and compression cases
14:13 sima: but it all gets very hw/driver specific fast
14:13 sima: and some combos just wont work ever
14:13 sima: like I see no way to make cpu driven manual upload over usb or spi work, ever
14:14 sima: so you could also say that no one ever thinks about usb&spi, therefore we need to nuke this entire effort to add a panic handler
14:15 pq: sima, is it not the normal case to have the current VRAM framebuffer likely not even CPU accessible at all, let alone already mapped? As GPU rendering and KMS display do not normally need it to be. So magic peek/poke it is.
14:15 mripard: we really can't access USB and SPI atomically
14:15 sima: pq, there's a lot of cases where it happens to be accessible
14:15 mripard: for a lot of devices we can switch planes atomically
14:16 mripard: so that's not really the same situation?
14:16 sima: well I'm also not understanding why we can't make a panic framework work for that case with a bunch more helpers and stuff?
14:17 sima: inflicting the atomic switch on everyone is what we had, and it was a good fireworks show
14:17 sima: it's really hw dependent whether the atomic flip is a good idea or not
14:17 sima: also you need nmi, not just atomic
14:18 alyssa: karolherbst: will look
14:18 karolherbst: thanks!
14:18 sima: and nothing except panic code in your driver ever runs in nmi context
14:18 alyssa: i like excuses to procrsatinate slides
14:18 karolherbst: ohh right..s lides
14:20 mripard: sima: I repeatedly said in my mails that I was fine not going for the atomic commit road but that the get_scanout_buffer interface was too restrictive to expand it to other drivers later on
14:21 mripard: if we figure out another interface that allows to expand later with helpers, I'm all for it
14:22 jfalempe: mripard, what do you need that can't be done with the get_scanout_buffer ?
14:23 sima: jfalempe, by far not everything can be done with an iosys_map
14:23 sima: but I think the "I need a preallocated buffer" case should actually fit, it's amd's peek/poke that doesnt
14:24 sima: jfalempe, essentially for more general case it's the ->panic_draw_xy that noralf linked somewhere in that discussion
14:24 jfalempe: Yes, I think the driver can pre-allocate a buffer, and use that in the get_scanout_buffer() then it probably wants a scanout_flush() callback to send it the hw ?
14:25 jfalempe: sima, Yes, I already planned to add this too, but I don't think it's what mripard needs.
14:25 sima: jfalempe, if it's just a base address flip then that can be done in get_scanout_buffer
14:25 jfalempe: sima, but if it needs a flush, or dma start, then it should be done after the buffer is written.
14:25 sima: jfalempe, I guess there's a bit an argument whether you want a raw data write function or a pixel write function
14:26 sima: looks like the current idea is to put format handling into shared code, not sure that'll scale that well for all the driver specific tiling/compression
14:26 sima: jfalempe, flush after every write
14:26 sima: speed doesn't matter in panic, you're competing with a serial console
14:27 jfalempe: yes, we're not looking for high FPS in the panic code ;)
14:27 sima: at least until we have a driver where a flush is a full screen upload
14:27 sima: then "flush every pixel" is probably too slow
14:29 mripard: sima: what hardware can't turn off compression or change tiling/compression atomically? All the ones I've seen have the registers vblank synchronized so it doesn't really matter
14:29 sima: mripard, reallocating fifo space tends to kill you
14:29 sima: or require at least so much code to be run that it's a big no-no for panic context
14:30 mripard: yeah, ok. so same thing than vc4
14:30 sima: but again it's highly hw dependent, if you have hw that can always & easily flip to full screen xrgb8888 scanout and nothing will ever break
14:30 sima: then that's probably a good thing to code up in the ->panic_prep callback (or whatever we paint that bikeshed)
14:31 sima: so that might need some extensions of the framework
14:31 mripard: which is kind of why I was suggesting the "prepared commit" approach, even though it's flawed
14:31 sima: and a driver where this really is better than just painting into the current buffer
14:31 jfalempe: sima, that can be done in the get_scanout_buffer() before returning the buffer.
14:31 sima: mripard, the problem ime is that the moment you do this, you tempt people to just call into their modeset code
14:31 sima: "it worked in testing"
14:32 sima: been there, done that, ripped it all out
14:32 sima: so that's why I'm heavily leaning towards a panic framework that's very heavily biased towards "we're sure this wont make anything worse"
14:33 sima: jfalempe, yeah I guess you could shoehorn that in
14:33 sima: the trylock should already guarantee that the modeset code isn't touching the hw at least
14:34 sima: uh
14:35 sima: jfalempe, the locking in your patch seems entirely absent?
14:35 sima: that looks not good at all
14:35 mripard: jfalempe: I guess what I'd really like to see is how that works with a "real", helpers based driver, not a firmware based one.
14:36 jfalempe: sima, yes, I need to look into that too
14:36 sima: I thought noralf had coded up something in some really old patches?
14:36 sima: or at least I typed up some design
14:36 mripard: (firmware or relying on shadow buffer and conversions)
14:36 sima: mripard, I'm on board with that
14:37 mripard: like get_scanout_buffer() operating at the drm_mode_config_funcs level probably doesn't work very well when you have multiple CRTCs and outputs
14:37 sima: jfalempe, I'd expect you'll end up with higher level functions like the ones I've proposed, and more bits pushed into helpers that drivers call as needed
14:38 sima: mripard, yeah the discussion I had with noralf years ago covered all that
14:38 jfalempe: sima, currently I know mostly simpledrm, mgag200 and ast drivers, which are very simple. So that will be hard for me to make it work on more complex drivers.
14:40 sima: the real fun is probably a compressed yuv planar buffer on an arm-soc display
14:40 mripard: jfalempe: I can help with that
14:40 sima: aside from peek/poke I think that's the most complex one where we can realistically push out pixels
14:40 mripard: and probably give you a board if you can't get one
14:41 jfalempe: mripard, thanks
14:42 jfalempe: yeah I only have a few very outdated arm board here.
14:48 robclark: mripard: re: fbc, there is probably a straightforward way to have no compression.. ie. fill the metadata plane with a value that indicates "no compression" (which might just mean memset it to zero)
14:49 robclark: mripard, sima: hmm, but crash during protected content scanout might be fun
14:50 pq: that's just reboot, right? ;-)
14:50 sima: robclark, yeah just clearing the metadata plane should work for most I think
14:51 sima: there's some where the metadata is in some special gpu buffer though :-/
14:51 sima: robclark, and yeah compression might be fun, but iirc on intel you can clear it with a single mmio write and nothing else
14:52 sima: *encryption I meant
14:52 sima: and then write unencrypted panic stuff into the same buffer
14:56 mripard: sima: the way it's typically done on ARM is that Linux gets a buffer that has been decoded by the firmware and the content can't be accessed by the access level Linux runs with
14:56 sima: ah yeah that wont work
14:57 sima: so I guess another case where you need to flip to something else
14:57 robclark: you might still be able to flip to a linear
14:57 mripard: sure, but you can't write into the same buffer
15:45 mdnavare: airlied: vsyrjala : For the drm_atomic_check_only() warning of "Adding extra CRTC not allowed without modeset" we see that getting trigged when we move the overlay plane playing a youtube video across to the second screen on MST, this needs higher cdclk now and hence all crtcs get added to the state so that they can all be turned off. But we just throw this warning and continue causing a EBUSY err later and visual artifacts like
15:45 mdnavare: glitches. Can we reject this here instead of just a warning?
15:46 mdnavare: or perhaps reject in the vendor hook when this cdclk condition arises?
15:52 sima: mdnavare, you need to flag that this requires a modeset in both crtc
15:53 sima: at least as an interim fix
15:54 sima: crtc_state->mode_changed = true; when you pull in crtc for cdclk changes
16:07 mdnavare: Thanks sima , I think we do that already but in drm_atomic_check_only it still throws that warning instead of just rejecting it altogether
16:11 sima: mdnavare, oh right the logic is different, but maybe we need to adjust
16:11 sima: the driver is supposed to check for drm_atomic_state->allow_modeset and fail the commit outright
16:11 sima: the idea is that on some hw you can fall back to less optimal configurations that don't require a full commit
16:11 sima: if the driver only sets ->mode_changed then it cannot differentiate
16:14 sima: robclark, 82836692d5d74 is very entertaining, silently upgrading ALLOW_MODESET when userspace didn't set it is really not how the uapi is supposed to work
16:15 sima: I think the allow_modeset = true needs to go
16:15 mdnavare: ok so you are saying that in this case say in i915 where it needs this cdclk change, and when it adds crtc, it can check the drm_atomic_state->allow_modeset and if it is not set then fail it, sima
16:18 sima: robclark, and probably some shuffling to run the ctm pm stuff in the right place ...
16:19 vsyrjala: for i915 what i'd like to have is proper barriers in the commit timeline. that's the reason we add the extra crtcs to the state currently, they don't actually need a commit. but -ENOTIME so haven't tried to cook up anything real
16:22 mdnavare: so as an interim solution, can we fail this in i915 intel_atomic_check after checking drm_Atomic_state if allow modeset is not set?
16:22 mdnavare: vsyrjala: Like sima suggested
16:25 robclark: sima: but we need to allocate the dspp block.. which we can't do statically (potentially fewer dspp blocks than lm's (aka crtc))
16:28 sima: robclark, yeah, but drivers aren't allowed to touch allow_modeset
16:30 sima: robclark, https://paste.debian.net/hidden/eac3c865/ something like this plus whatever it takes to fix msm
16:31 robclark: I can ask the compositor folks about whether they can set ALLOW_MODESET when there is a ctm change.. but that isn't how it has worked before and not sure if it will cause problems on other drivers
16:33 sima: robclark, yeah this is a pretty bad situation
16:33 sima: robclark, so why can't you fix msm code?
16:33 sima: assuming you can actually reassign this stuff without full modesets
16:33 sima: if you need flickering, then this is flat out breaking uapi
16:34 robclark: I think the allocating dynamically allocated resources is tied to allow_modeset.. but does not mean flicker in this case
16:34 robclark: abhinav__ or lumag probably more famliar
16:45 mdnavare: robclark: Have you sent a patch for this already to clarify the drm rules around allow mdoeset?
16:46 zamundaaa[m]: robclark: KWin at least doesn't... but it may if there's a modeset pending because of other stuff already. So the prop changing only with modesets would be pretty annoying
16:46 mdnavare: robclark: And when you say msm fix or i915 fix for this would be to then reject this in the driver?
16:47 robclark: yeah, I kinda prefer the current soln that sima doesn't like so it isn't a modeset from uapi PoV
16:48 zamundaaa[m]: Can you un-set the CTM / set it to identity without a modeset?
16:53 sima: robclark, the trouble is that there's nothing guaranteeing that this it the only reason you need a modeset
16:53 sima: so you could end up with flicker that's not expected by userspace
16:54 sima: so yeah probably needs some rework where you run the right additional functions for the case where it's not a modeset, but just a ctm enable change
16:58 mdnavare: robclark: Currently does msm just set the crtc_state->mode_changed= true w/o letting the userspace know about it? I should check what i915 does when it sees that cdclk needs to be changed and adds all crtcs to the state
17:00 mdnavare: I guess my question would be should such atomi_check calls be rejected in the vendor specific drivers for these conditions when perhaps its needing to set mode_changed or should this be rejected higher up in drm_atomic_check_only and return an -EINVAL or some error
17:00 mdnavare: robclark: sima vsyrjala airlied : Any suggestions?
17:01 sima: robclark, also I don't really buy your "no flickering" claim, setting crtc_state->mode_changed forces a full on->off->on transition of a crtc, and I don't see any special handling of that case in msm code
17:01 sima: so pretty sure this is just broken
17:03 sima: at least a few greps for the usual suspects didn't turn up anything interesting
17:06 robclark: sima: idk but people would be yelling if ctm triggered a flicker so I guess it works somehow
17:07 sima: robclark, it's only when you enable/disable it, not when you change it
17:07 sima: and yeah I'm pretty sure it will go on/off/on
17:07 sima: also, maybe manual upload panels paper over this for the internal output?
17:07 robclark: hmm, I can't rule out that userspace does modeset initially with identity matrix?
17:07 sima: yeah
17:08 sima: if you do a modeset to do the ctm_on/off transition, it's fine
17:08 sima: which would also mean we could silently fix this by just dropping the allow_modeset = true; case
17:08 sima: anyway I sent out the doc patch with the 2 reasons this is bad in the commit message
17:09 sima: once we have consensus on that one way or the other it should also be clearer what msm needs to do
17:11 sima: robclark, I would be not surprised at all that if you dig into compositor history people do the identity matrix "because it avoids flickering"
17:11 robclark: actually I don't see where we obviously need allow_modeset.. but I'll defer to abhinav__
17:13 sima: well you set crtc_state->mode_changed
17:13 sima: allow_modeset only breaks the uapi
17:13 sima: it has no impact on what actually happens in the commit or atomic check
17:17 robclark: hmm, exiting self refresh also sets allow_modeset
17:19 sima: yeah we cheat in some places ...
17:20 sima: probably should do better, and iirc the idea was to just pull these helpers into drivers since it didn't really seem to worth the trouble
17:21 sima: for self refresh the trouble is that there's not really any other way to make it happen
17:21 sima: unless you push the responsibility for going into self refresh to userspace
17:22 sima: robclark, I guess what we could do is make it a per-crtc flag to override the allow_modeset check or something
17:22 sima: or well add a new one like crtc_state->uapi_silent_modeset
17:22 sima: and then use that one in self refresh helpers
17:23 robclark: maybe .. but don't have time right now and wasn't the one that implemented that so not sure offhand what the issue they were hitting was
17:24 robclark: ask me after xdc ;-)
17:24 robclark: or abhinav__ when he shows up
17:24 sima: hm forgot to cc abhinav__ on the patch
17:24 sima: but I guess irc backlog
17:25 lumag: sima, robclark: yes, the issue is that we have to reallocate resources if CTM usage changes.
17:26 sima: lumag, yeah that's what the code comment says
17:26 sima: the question is whether that results in flicker or not, and whether it has to result in flickering or not
17:27 lumag: Only a part of LM blocks (=crtc) support CTM, other blocks do not. So if we are dual-headed, turn CTM on the first head, then disable it, turn it on the second head, the second head might not get it.
17:32 lumag: sima, I think flickering might depend on the rest of the display pipeline. E.g. DSI panel might be quite permissive. DSI->HDMI bridge? Not so sure.
17:32 robclark: I don't believe re-assigning dspp to crtc in and of itself would flicker, any more than sspp (plane) reassignement would
17:32 lumag: robclark, we are reassigning LMs, not just DSPP.
17:33 lumag: And the issue might be in disable_outputs / enable_outputs if I understand sima's concern
17:33 robclark: oh, well.. I think still the same, as long as there is one avail to reassign
17:33 lumag: robclark, if there is none, we just fail the atomic_check
17:34 sima: yeah just reassigning limited shared resources shouldn't cause issues
17:35 sima: you might have sync issues if you push them through as nonblocking commits, maybe it was that?
17:35 lumag: sima, so what's your concern? disable/enable_outpus?
17:35 sima: setting ->allow_modeset
17:35 lumag: Yep, I mean why do you expect flicker?
17:35 sima: https://lore.kernel.org/dri-devel/20231010170746.617366-1-daniel.vetter@ffwll.ch/T/#u
17:35 lumag: no, nonblocking commits will not work. We have to wait for vsync
17:35 sima: you also set ->mode_changed
17:36 sima: lumag, I meant not blocking the ioctl
17:36 lumag: sima, what would be the point?
17:37 sima: and setting ->mode_changed forces a full on->off->on cycle for the crtc
17:37 sima: at least if I'm not entirely out of the loop how this code works
17:38 sima: so that's definitely flickering, and then you hide that all by force-setting allow_modeset, which really only userspace is allowed to set
17:41 sima: lumag, also no idea what you meant with you question
17:42 lumag: sima, I asked, what would be the point in not blocking the ioctl.
17:42 lumag: and anyway, if I'm reading drm_atomic_get_crtc_state() comment, we are in a bigger trouble.
17:42 sima: lumag, it's like half the reason why the atomic ioctl exists
17:43 sima: lumag, yeah you're probably in big trouble all around
17:43 lumag: :D
17:43 sima: DRM_MODE_ATOMIC_NONBLOCK from the uapi header is what I meant with nonblocking
17:44 sima: so yeah you cannot add random other crtc
17:44 sima: only if allow_modeset is set
17:44 lumag: I mean, we might have to add new CRTCs to the state, if we change backing hardware blocks.
17:44 sima: and you're not allowed to just set that
17:44 lumag: ye
17:44 lumag: yes
17:44 sima: so you cannot do that
17:44 sima: instead if you have to manage shared resources, you have to put these into separate driver private state structures
17:44 sima: which you can then add without breaking anything
17:45 sima: but then you get the fun of having to manage synchronization yourself for nonblocking atomic commits
17:45 sima: so fixing this will likely require a pile of work
17:45 sima: also the current "fix" causes flickering, 90% sure on that
17:45 lumag: we already have a private obj with allocated resources state, but then this state was propagated to drm_crtc_state.
17:46 sima: yeah propagating is fine
17:46 sima: adding crtc without ->allow_modeset being set is not fine
17:46 sima: setting ->allow_modeset is very much not fine
17:46 lumag: Yes, I got that
17:46 sima: (I need to fix self refresh helpers I guess)
17:46 lumag: propagating is not fine, as we then are adding new crtc states to the state
17:47 sima: oh I meant just propagating the allocation to each crtc
17:47 sima: so that you know which one is your resource for a specific crtc without having to always get the global state (which isn't great either because it might result in oversync issues)
17:47 sima: if you propagate more, then that might be not fine
17:49 lumag: ok, we are propagating in the atomic_mode_set, which is probably fine, as we are touching the flipped state rather than adding stuff to new_state
17:50 lumag: I think I'd like to catch robclark and abhinav__ during XDC (and you if you have time, ofc) and discuss a part of that. I wanted to rework parts of that resource state machine.
17:50 j`ey: do you need to be subscribed to dri-devel to send emails?
17:53 sima: j`ey, shouldn't be needed, just means you need to be manually approved which can cause delays
17:53 abhinav__: sima sorry i am just catching up on this thread. we need a full modeset when ctm state changes because we reallocate the hw resources of the pipeline feeding one display to another. so lets say we had hardware blocks to just feed one display and then someone turned off the CTM using the display settings on that display, we can re-assign it to the
17:53 abhinav__: other one which also needs it but didnt get it as we had limited resources. Is your concern why we set mode_changed or more about allow_modeset
17:53 sima: allow_modeset
17:53 sima: but you probably need that because you set mode_changed and add crtc
17:53 j`ey: sima: ok, thanks!
17:54 sima: you hit the EBUSY uapi check if allow_modeset isn't set by userspace for this case
17:55 abhinav__: regarding allow_modeset, I have to check about that exactly. but yes most likely that is the reason that usermode did not set allow_modeset for this case
17:56 abhinav__: so are you suggesting that we keep mode_changed but remove allow_modeset setting and let usermode handle that?
17:58 abhinav__: for CTM on/off transition cases
17:58 lumag: abhinav__, I fear that wouldn't work. We might have to crack our atomic_check to force resource reallocation in this case w/o setting crtc->mode_changed.
17:58 sima: abhinav__, you'll probably have more bugs, see the things that lumag mentioned
17:59 sima: lumag, ideally yes, but the bigger issue is setting crtc_state->mode_changed on other crtc
17:59 sima: that's the thing which gets you into trouble with the EBUSY check
17:59 sima: but yeah ideally you can allocate shared resource for ctm without ->mode_changed
18:00 sima: note I'm not sure you set ->mode_changed on other crtc, but it sounded like you do from what you described
18:00 abhinav__: lumag without mode_changed would not be possible. there are display blocks which cannot be re-assigned without a full teardown
18:01 abhinav__: and we cannot expect usermode to know all those hw blocks
18:01 sima: abhinav__, https://lore.kernel.org/dri-devel/20231010170746.617366-1-daniel.vetter@ffwll.ch/T/#u for a bit more context
18:01 sima: abhinav__, yeah but you can't do a full teardown if userspace didn't allow that
18:02 sima: if allow_modeset is not set by userspace, and you need a modeset that results in flickering and stuff like that, you must fail the atomic ioctl
18:02 sima: or well, atomic_check callback really from a driver pov
18:04 sima: probably most fundamental rule of the atomic ioctl is that if ALLOW_MODESET is not set, there's zero flickering
18:05 sima: and that the driver will try hard to hit the next vblank (but that one is a bit more a best effort thing)
18:26 mdnavare: sima: so looking at Danvet's rules , looks like anytime driver needs to override this if allow modeset not set we need to reject that with a -EINVAL and same ind rm_atomic_Check_only. I am gonna send a patch for that
18:48 abhinav__: sima i see your point but I dont know if this is a generic rule observed by all drivers. I do see multiple drivers forcing mode_changed based on their hw specific conditions. Are you suggesting each of those are accompanied by a corresponding allow_modeset? Regarding flickering, I do get it that its a unexpected behavior for usermode. but it
18:48 abhinav__: depends on the use-case. if lets say a user toggled a setting , they might actually be fine with one blink or lets say user was using a writeback connector then there wont be any physical display for the flicker. so i am wondering if this is a really a hard expectation for all use-cases
18:48 sima: abhinav__, setting mode_changed is fine
18:48 sima: setting allow_modeset is where you break uapi
18:49 sima: and the thing is, yes sometimes flickering is required by the hw, but the atomic ioctl contract is that userspace, and userspace alone gets to decide whether flickering is ok
18:49 sima: because like you point out, this dependes upon the use-case, and the kernel has no idea what use-case userspace is aiming for
18:50 sima: that's why we have the ALLOW_MODESET flag in the uapi
18:51 sima: wrt writeback, the uapi allows you to have a writeback connector in parallel to a real one
18:51 sima: so even that one is kinda complicated
18:51 sima: also ALLOW_MODESET also allows you to impact _other_ crtc
18:51 sima: so there's cases where you could support a writeback request, but it would impact other crtc
18:52 sima: again, userspace gets to decide
18:52 sima: drivers might set mode_changed a few too many times for the writeback case, that might indeed be the case
18:53 sima: but also, no one then overrules userspace by changing ->allow_modeset
18:55 lumag: sima, so the expectation is that if we need to perform a modeset, we set the crtc_state->mode_changed, then drm_atomic_check_only() would return -EINVAL if userspace didn't allow full modesetting?
18:55 sima: yeah
18:55 sima: there's the hack in self refresh helpers, but I think I know how to fix that one
18:57 abhinav__: sima ok, i think we need to evaluate the allow_modeset removal in the msm ctm code and if it breaks, we would need compositor to set allow_modeset for ctm transitions. I guess for hardware which doesnt need a full modeset, they wont set mode_changed so no impact
18:58 sima: abhinav__, yeah if we are breaking current userspace that's going to be tricky
18:58 sima: I guess worst case we need a Kconfig/modparam to enable the old hack
18:58 lumag: abhinav__, I wonder how does that work for mali, which just sets crtc_state->mode_changed
18:58 sima: and ideally the driver does the least amount of ->mode_changed that you can do with a given hw
18:58 zamundaaa[m]: Setting the CTM requiring a modeset might be acceptable. If unsetting it also requires a modeset, that's not gonna work
18:59 sima: zamundaaa[m], msm needs both, at least looking at the code
18:59 abhinav__: yeah we need both
18:59 sima: I guess what would work is if the driver internally sets an identity matrix if allow_modeset isn't set
18:59 sima: and only nukes the ctm for real when allow_modeset is set
19:00 sima: that would at least prevent the mode_changed when disabling the ctm
19:00 lumag: and if allow_modset is set, this would allow us to play dirty tricks with all CRTCs, correct?
19:00 sima: zamundaaa[m], but in general there really is a pretty endless list of hw limitations if you look across all drm drivers
19:01 sima: lumag, yup, that's what I'm trying to clarify in my doc patch
19:01 sima: a driver can look (as in read, not write) at allow_modeset in atomic_check
19:01 abhinav__: lumag i guess it all depends on which usermode it was tried with
19:01 sima: if you have a more "optimal" (whatever that means) path which requires a modeset
19:01 lumag: abhinav__, yep
19:02 sima: and a fallback for the case without modeset (could be things like just keeping an identity matrix, or could mean you fail with EINVAL)
19:03 zamundaaa[m]: sima: sure, but userspace can't work if you require modesets for disabling color blocks
19:04 sima: zamundaaa[m], if this is a hard requirement, we need it documented and agreed with drivers and have testcases and all that
19:04 sima: pretty sure there's plenty that break this
19:04 zamundaaa[m]: We have it as a hard requirement for the new color pipeline system. Idk if it's documented anywhere for the existing stuff
19:06 sima: malidp is pretty harsh with setting ->mode_changed
19:06 lumag: abhinav__, One of the issues was that atomic_check(encoder) was not called unless we set mode_changed early. I think we can follow mali's trick more or less.
19:07 lumag: sima, is mali also going to be frowned upon? Then we can't use it as a role model.
19:07 sima: lumag, as long as your callbacks allow, the atomic helpers are intentionally idempotent
19:07 sima: so just call them again (like mali does)
19:07 sima: lumag, no idea, I don't make the uapi rules myself, that's a contract between what drivers can do and what compositors need
19:07 sima: emersion, ^^
19:09 lumag: Interesting. IPU also forces mode_changed in significant amount of conditions.
19:09 sima: yeah plane changes have lots of these conditions
19:10 sima: like nothing guarantees even that you can go back to a full screen xrgb fb
19:12 zamundaaa[m]: for generic compositors at least, that definitely sounds very broken
19:12 sima: zamundaaa[m], I guess for robustness across the board (since this is by for not just a ctm issue) you need a fallback to full screen fb and gpu compositioning and then just mark that driver as "can't use fancy features"
19:12 sima: zamundaaa[m], display hw is very very funny
19:12 sima: if you include all the drivers that require a modeset for fb format changes, the list gets very long
19:14 zamundaaa[m]: Well, I don't think the rules need to be (or can be) that strict for every case. With color management stuff it's just a bit more important because it's so inherently dynamic
19:15 sima: zamundaaa[m], but this is already the case for planes
19:16 sima: and I thought for many cases you practically need planes to make use of hw ctm
19:20 sima: zamundaaa[m], also if we require rules like this, it would mean that on some especially funky hw we'd not be able to expose hw features
19:20 sima: and that would make people who just want to have a static setup rather unhappy
19:20 lumag: ... and then comes gnome-shell/mutter, telling: ``Oh, so you have universal universal planes? We don't like you, let's segfault!''
19:21 zamundaaa[m]: sima: For planes and formats my expectations from drivers are:
19:21 zamundaaa[m]: - if enabling a plane works without a modeset, disabling it again also has to work without a modeset. Enabling it in the first place may fail
19:21 zamundaaa[m]: - if switching to a format works without a modeset, switching back to the previous format also needs to work without a modeset. If the first switch fails without a modeset, that would be fine
19:22 lumag: zamundaaa[m], it might be not that simple, unless you disallow other modesets inbetween.
19:24 sima: zamundaaa[m], yeah I think you're already asking a lot more than drivers/hw can
19:24 lumag: I mean, the 'enabling' part might have freed hardware resource that got taken by the other output, so 'disabling' = 'switching to previous' might not work anymore.
19:24 sima: especially when you chain up changes
19:24 sima: and I think we flat out had asymmetric conditions too
19:27 sima: zamundaaa[m], like lumag points out, general the real fun only kicks in with multiple outputs though
19:28 zamundaaa[m]: how would enabling a plane on one output free up resources on another, when you just have non-blocking commits without allow_modeset?
19:30 sima: zamundaaa[m], if you cover the underlying plane entirely it doesn't really need anything anymore
19:30 sima: not sure that's the example lumag is thinking of
19:34 karolherbst: gfxstrand: quick review pls (it's your code, but had a memory leak): https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25649
19:35 zamundaaa[m]: sima: well, that sucks... I don't think desktop compositors can work with that. From the moment that a mode is set, until the user manually and intentionally triggers a modeset in some way, there must not be any flicker, ever
19:35 lumag: zamundaaa[m], I was thinking about switching the format. If we switch from YUV to RGB, this might free CSC blocks, which might be taken by another user. In case of msm it is slightly different path, but the idea is the same.
19:35 karolherbst: or maybe it should use disk_cache_put_nocopy instead?
19:36 sima: zamundaaa[m], yeah I fully understand, but I also think the pragmatic reality is that you probably need a fallback
19:36 sima: so that users can at least file a bug report against their drm display driver that it sucks too much
19:36 sima: if you just keel over, that's not good
19:37 sima: like we have a really hard time just making the current atomic check/commit stuff right, guaranteeing certain things around always being able to go back means the testing combinatorial cases go really badly through the roof
19:39 sima: zamundaaa[m], and if you insist too much that a driver must not require a modeset you just get hacks like the one in msm that sparked this entire discussion, where the driver flat out lies to userspace
19:40 robclark: it might be possible to guarantee that we can at least always unwind to single fullscreen layer, to cut the testing space.. since that would be good enough for compositor to fallback to gpu
19:41 zamundaaa[m]: robclark: yeah, that would be enough
19:42 lumag: robclark, well, with multirect in place we can do that now.
19:42 sima: robclark, once you reallocate fw and scanout bw that's out
19:42 sima: like 1. full screen rgbx 2. go to full screen yuv 3. enable 2nd output that needs the bw you freed up with the switch to yuv 4. no going back to full screen rgbx on first output
19:43 sima: and if we just preemptively reserve everything we possible need for full screen rgbx people will get pissed about missing power/feature targets
19:44 sima: all we can guarantee that if userspace does a TEST_ONLY | ALLOW_MODESET check with full fb rgbx
19:44 lumag: sima, then it should be limited to: go fullscreen rgbx on a single output.
19:44 sima: that we can get to that with a full modeset
19:44 robclark: sure, but compositor could still unwind to _single_ fullscreen layer on _one_ display.. shutting down non-primary outputs is less bad than getting into a point where it can't pageflipe
19:44 lumag: quite usefull... useless.
19:45 sima: lumag, that requires a modeset to kill the other output?
19:45 zamundaaa[m]: robclark: to do that you'd still need a modeset
19:45 lumag: robclark, then we are also back to the question, what is the primary display
19:45 lumag: or which one is primary
19:45 sima: yeah as soon as we allow modeset as an escape hatch there's all kinds of things userspace can check and the kernel will guarantee
19:45 sima: like with ALLOW_MODESET if a configuration A works, you can _always_ switch to it
19:46 sima: it's only for !ALLOW_MODESET where a configuration might be possible, you can't switch to it from the current one without flickering
19:47 sima: and if we'd want to allow userspace to check for the !ALLOW_MODESET case, we'd need to be able to validate entire flip changes
19:47 sima: so you could check that A->B and B->A works
19:47 sima: or if you then do B->C you can check that C->A still works
19:47 sima: but the current uapi doesn't allow you to do check whether two subsequent flips would work
19:48 sima: *consecutive
19:49 sima: and frankly doing TEST_ONLY for A->B->C scares me a bit ...
19:50 sima: (it would mean a rewrite of everything since drivers assume the current/preceeding state is attached to the object)
19:54 lumag: sima, might I ask an unrelated question? We have been trying to get https://patchwork.freedesktop.org/series/120393/ for quite a while. Patches are acked to go via drm-something, but several last revisions went w/o any attention and/or comment.
19:55 lumag: Is it fine to push them through drm-misc? Should it be taken through drm-intel? Just drop them and drop USB-C DP on msm? :D
19:57 sima: lumag, if you have an ack from i915 display maintainers (jani or rodrigovivi) and the ack for the usb side then just push into drm-misc
19:58 sima: assuming no one is screaming about the concept in general, I have not much useful clue for both bridges and usb-c oob signalling
19:58 lumag: sima, ack, thank you
19:59 lumag: sima, you don't want to know that :D
19:59 lumag: It was more of a question of policy / permission. You answer is fine, we have all the acks.
20:00 sima: lumag, I guess some acks from bridge folks would be good too, but *shrugs*
20:00 sima: lumag, but yeah in general if you have a patch set that has multiple different trees because it goes across, just get acks from everyone else and then land in drm-misc
20:01 sima: or whichever tree makes sense as the main one
20:01 zamundaaa[m]: sima: guess I'll add "broken drm driver" detection to KWin :/
20:02 sima: well there's a lot more reasons for a broken driver, but even for a good driver you just might have really suck hw
20:03 sima: the one thing that we really should be good at guaranteeing (because the entire atomic framework was built to make it happen reliably) is if a full config passed TEST_ONLY | ALLOW_MODESET
20:03 sima: we can always commit it
20:04 sima: only exception could be "ran out of memory meanwhile, sorry"
20:04 sima: but that's just linux' overcommit
20:11 lumag: sima, another question: drm-misc-next is still on the v6.5-rc2. Is it fine to backmerge drm/drm-next to update it to 6.6-rc? Otherwise I'm getting a conflict when applying patches.
20:13 sima: lumag, ask drm-misc maintainers to do the backmerge for you, and explain the reason they should record in the commit message
20:14 sima: since merges are generally rare and need some extra care that's not for committers as a rule
20:15 sima: but yeah generally drm-misc-next shouldn't be stuck on an old -rc for that long ...
20:17 sima: time to sleep here now, ttyt
20:24 tleydxdy: what is DRM_V3D_PERFMON_CREATE, seems v3d is some broadcom gpu? I'm running a vulkan application on amdgpu and I see this pop up in strace
20:28 robclark: tleydxdy: in cases where same ioctl nr are used by multiple drivers, strace should list all the possible ones (or at least all the ones it knows about)
21:10 tleydxdy: hmm, yeah it just have V3D_PERFMON, how do I find out what ioctl numbers these things have?
21:12 tleydxdy: okay, I guess the ioctl is actually DRM_AMDGPU_GEM_VA
21:16 tleydxdy: hmm, and in mesa it's from a file called v3d_simulator.c?
21:17 anholt: tleydxdy: strace can't really know what an ioctl magic number maps to, becuase it's driver dependent.
21:18 anholt: since you know you have AMD, you can ignore the V3D ones.