01:07 thegeeko: Hi .. I'm trying to understand what vmids in amdgpus are .. I understood that they're vm created auto by the driver and u can ask and reserve some .. then I created an amdgpu device a bo and mapped the buffer object to cpu mem and wrote to the buffer using umr to read the mem I can see my writes are there .. the issue is when I read from vmid 1 it works and 2, 3, and 4 .. why so ? shouldn't it be 1 vmid per application ?
01:19 soreau: unless environment and driconf are interchangeable, it's not clear to which this refers because the source seems to disagree with the comment: https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/src/mesa/main/shared.c#L85
01:22 soreau: apparently, the change to GL name reuse is causing some nasty artifacts on gles compositors and want to make sure how to set this before trying latest mesa with the switch to test. related: https://gitlab.freedesktop.org/mesa/mesa/-/issues/12707
03:02 soreau: I guess gstreamer vaapi waylandsink is broken on mesa git too but that's a bisect for another day :P
04:47 jadahl: so with mutter more or less giving up on using atomic KMS for cursor updates for the time being, are there any guarantees that mixing legacy and atomic API is actually expected o work across all drivers?
07:54 tzimmermann: sima, hi. a question about device ref-counting. there's put_device(dev->dev) at https://elixir.bootlin.com/linux/v6.13.4/source/drivers/gpu/drm/drm_drv.c#L588
07:55 tzimmermann: shouldn't we do this in drm_dev_unplug() already ?
09:05 javierm: tzimmermann: isn't drm_dev_unplug() only to make the DRM device not accessible to user-space? Why would the dev->dev kref be decremented for the parent device at this point?
09:08 javierm: or maybe I misunderstood the question
09:13 tzimmermann: javierm, sima, drm_dev_unplug() happens on pci/usb/etc device removal. i was a bit surprosed that we're not immediatelly removing our device references. that put_device() only happens when the drm_device is being cleaned up AFAIU.
09:14 tzimmermann: and drm_dev_unplug() signals drivers to not use the hardware device any longer. so i was under the impressino that we'd also release the hardware device then.
10:25 sima: tzimmermann, it's complicated
10:25 sima: and buggy
10:26 sima: tzimmermann, the short summary is that drm_device is both the hw device
10:27 sima: and the lifetime of that ends after ->remove/unplug/whatever is finishes
10:27 sima: *finished
10:27 sima: and so that's what drm_dev_unplug does essentially, tell all concurrent code that the hw device is gone
10:28 sima: argh, I already got it wrong before I started :-/
10:28 sima: I typed this up somewhere, it's really complicated
10:29 sima: tzimmermann, while I'm trying to find a reference, might be better to start with why you're pondering this?
10:29 sima: like is something broken
10:36 javierm: sima: maybe you are referring to https://lists.freedesktop.org/archives/intel-xe/2024-April/034195.html ?
10:37 for_opel_astra: I decided to also quit talking about those things, as i am not being keenly expected to participate on other matters either it seems, even though mareko asked if i would participate and though it was very kind remark imo as the man himself is also cool guy but we'd just end up splitting our ways, and if that is not going to work out , it's a fight called in, i ain't gonna let myself
10:37 for_opel_astra: consistently humiliated and assaulted at all. I do not possibly see issues you see and that is clear underneath my soul.
10:39 sima: javierm, ah yeah that's the proper write up
10:39 sima: I tried to find it in our docs somewhere
10:39 sima: javierm, one part this is missing is lifetime of the module itself aka try_module_get()
10:40 sima: which is intentionally broken because developers prefer module unload convenience over correctness
10:40 sima: tzimmermann, maybe we should convert the mail javierm dug out into a doc section somewhere?
10:46 tzimmermann: sima, javierm, thanks. i'll read through this. i'm not saying it's broken. i looked through this code and that caught my eye. it is not what i expected
10:50 sima: tzimmermann, it's trapdoors all the way down in this area unfortunately
10:50 sima: hence better docs probably a good idea
10:50 sima: tzimmermann, https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#device-hot-unplug here might be good, linking to the functions/concepts we do have meanwhile
10:50 sima: maybe in a separate section for drivers about how to implement this
10:51 tzimmermann: sima, javierm, on that url: point 1: "devm is for hardware stuff, [...] _even_ when you hold onto a struct device reference." that nicely avoids answering my question. :p when all the HW resources are gone. what's the point of keeping the reference. shouldn't we at least _try_ to put the device reference here?
10:53 sima: tzimmermann, that's an uaf
10:53 sima: at least with current drivers
10:53 sima: if you have a driver that's entirely using drmm and devm, then yeah the very next thing will be the drm_dev_put()
10:54 sima: but we're not there yet, we'd need a devm_drm_dev_register for that
10:54 sima: iirc
10:54 sima:not entirely awake, still a bit a stuffy nose
10:54 sima: so it's essentially 1. drm_dev_unplug 2. legacy cleanup for drivers that aren't fully using drmm or devm 3. drm_dev_put
10:55 sima: tzimmermann, there's also the hilarious confusion that most developers insist on drm_dev_unregister without the hotunplug, so that drm_atomic_helper_shutdown does something useful
10:55 sima: it's ... a complete mess
10:55 tzimmermann: it's ok. my take-away is that not all drivers support an early put_device()
10:56 sima: yeah I think in theory your understanding is the conceptually clean approach
10:56 tzimmermann: hence it's done later
10:56 sima: and what I'm trying to push drivers towards, eventually
10:56 tzimmermann: sima, great. thanks a lot
10:56 sima: like one practical approach is that for production we really want drm_dev_unplug
10:56 sima: but developers really want drm_dev_unregister + hw shutdown
10:57 sima: and you can't have both
10:57 sima: and that's a bit the hold-up for converting the last bits over to drmm/devm for at least simpler drivers
10:57 tzimmermann: i personally do dev_unplug and that's it
10:58 sima: tzimmermann, but yeah might be worth it to review some of the simpler drivers and see what exactly we still have between the drm_dev_unplug and the drm_dev_put
10:58 sima: tzimmermann, it's a pain if you want to use module reload for testing new driver code without a full reboot
10:59 javierm: sima: even module removal I found that is usually not well tested for drivers in embedded plaforms, because people usually just built-in
10:59 sima: dakr, airlied I dropped a reply somewhere in that very big and very confused nova-core hotunplug discussion
10:59 sima: probably need more docs, see also discussion right now here
10:59 sima: javierm, yeah it's all very messy
11:00 sima: javierm, but driver unload should work, because that's usually subsystem levels of broken if it doesn't
11:00 sima: *cough* drm_bridge *cough*
11:01 sima: dakr, if I should reply somewhere else in there pls holler
11:02 sima: tzimmermann, looking at some tiny drivers I think what we'd need is a devm_drm_mode_config_reset (calls drm_atomic_helper_shutdown as devm cleanup action)
11:02 sima: and devm_drm_dev_register() (calls drm_dev_unplug as cleanup action)
11:02 sima: and we'd get to the nirvana world of "look nothing in ->remove callbacks anymore"
11:03 sima: oh also that's kinda the reason for the drm_dev_put not being immediate: drm_atomic_helper_shutdown not only shuts down hw
11:03 sima: but also cleans up a bunch of lingering references so that you don't hit any of the WARN_ON that detect leaks later on
11:03 sima: and you need a still-valid drm_device reference for calling that
11:04 sima: but yeah with that simple drivers would have 3 devm_drm_ calls int total (1. one is devm_drm_dev_alloc)
11:04 sima: and all the other sw stuff is drmm_
11:04 sima: and all the hw stuff devm_ (like gpio)
11:04 sima: and pls don't look at drm_bridge, because that's totally broken still rn :-/
11:07 javierm: sima: and on top of that, the fbdev emulation layer requires the fbdev to outlive a a drm_dev_unplug() since user-space might still had a /dev/fb0 open and mmap()'ed
11:07 javierm: I remember some UAF that fixed somewhere due that and not the driver not waiting for the fd close
11:07 javierm: *and the driver not
11:07 sima: javierm, I thought we refcount that one already?
11:08 sima: like fb_open grabs a drm_device refcount, or at least it really, really should
11:08 sima: maybe with the exception of fb_open for fbcon, but that's a completely different can of worms with imo unfixable locking
11:09 javierm: sima: yes I think is fixed already, but was just mentioning as a reason for drm_dev_put not being immediate
11:09 sima: javierm, we have a few more iirc, I think dma_buf exports also should grab a drm_device reference, just to be safe
11:09 javierm: right
11:09 sima: with dma_fence the plan is different, but not yet implemented, because there doing lots of references for these short-lived things might not be the best
11:11 sima: javierm, well we only take a module refcount right now, so this might not be the right thing
11:11 sima: but fbdev unregister is also fairly synchronous
11:11 sima: so maybe it's enough
11:12 sima: tzimmermann, since you've done these, this might be a confusion between drm_device lifetime and underlying module lifetime, which is a confusing mess
11:12 sima: could be interesting to see whether a sysfs driver unbind can still hit the bugs, in which case we'd also need a drm_device_get/put in the various fb_open implementations
11:13 sima: also a bit funny we have three copies for these, they all look the same between dma/shmem/ttm to mem
11:14 javierm: so we have the DRM dev, the HW dev, the client DRM fbdev and DRM module lifetimes to take into account. My brain hurts right now :)
11:20 jadahl: re-asking the same question again when more people are awake: so with mutter more or less giving up on using atomic KMS for cursor updates for the time being, are there any guarantees that mixing legacy and atomic API is actually supported usage and expected o work across all drivers forever?
11:20 sima: jadahl, not really
11:20 sima: it's piles of hacks and mostly works on the drivers people care about for mutter and cros (since they do the same)
11:20 sima: or at least cros did the same for the longest time
11:21 sima: jadahl, I actually deleted a lot of the hacks a while ago because it was just too broken
11:21 jadahl: MrCooper is arguing that it is in practice, but it seems fragile. the problem is that the atomic uapi just isn't good enough of a replacement in this particular situation
11:21 sima: jadahl, I think drivers which implement async_flip on cursor planes might fare better
11:22 sima: jadahl, the practice is shrinking afaict
11:22 sima: or at least it includes the occasional kernel oops
11:22 jadahl: is it though? what we need is doing a cursor plane movement, that still lets us do a "real" commit in the same refresh cycle targeting the same vblank
11:22 sima: javierm, it's a lot of fun
11:23 sima: jadahl, I meant wrt driver support
11:23 jadahl: ah, I see
11:23 sima: the trouble is that on modern hw there's no cursor plane, so you need to support both commits on that plane and cursor semantics
11:23 sima: without going boom
11:25 sima: back when I designed this all we assumed we'd only need this until Xorg is dead as a legacy hack
11:25 jadahl: I would imagine those wouldn't actually expose a cursor plane to begin with
11:25 sima: so I didn't put much thought into it and got it all wrong
11:25 jadahl: I can imagine that, yes, but seems we can't bend atomic to work well enough here :(
11:25 sima: jadahl, those =?
11:25 jadahl: those drivers which don't have a cursor plane
11:25 jadahl: hardware, I mean
11:26 sima: uh they do
11:26 sima: there's not endless amounts of planes, and some customers want all the planes as real planes
11:26 sima: and mutter wants a cursor plane
11:26 sima: so we make one do for both and suffer
11:26 jadahl: the apple hw one, IIRC, doesn't, because an o plane wasn't bendy enough to be seen as a cursor plane
11:27 jadahl: but maybe that is the special case, I dunno
11:27 sima: jadahl, yeah sometimes you don't have any cursor suitable plane at all
11:28 sima: but this is the crux, it's even harder in the kernel to do this right because the problem has become harder
11:28 sima: since more generic
11:28 jadahl: from a userspace perspective, i'd rather have no cursor plane than a cursor plane that "arbitrarily" doesn't work
11:28 sima: if we do get it right we can sneak in late updates, and async_flip is trying to make that happen for real, properly
11:29 sima: jadahl, lotta people disagree
11:29 sima: hence endless hacks
11:29 sima: defacto on modern-ish hw a cursor plane can have all the same funny plane limitations like a real plane
11:29 sima: sometimes even when it's a dedicated cursor plane, due to hw bugs
11:30 jadahl: anyway, i'm somewhat reading your take on "atomic uapi + drmModeMoveCursor()" as "not really supported, but might work, for now."
11:30 sima: like some intel atom gpu on the 3rd pipe the cursor can't be off-screen partially
11:30 sima: jadahl, I think actual async_flip might be eventually, since there we could even do a test_only mode so you know whether the cursor plane supports your change
11:30 jadahl: can't be off screen IIRC was the apple hw limitation, or something along those lines. that just makes it impossible to use as a cursor plane
11:31 sima: but yeah atomic uapi plus legacy cursor is good luck, you'll need it
11:31 jadahl: because cursors really do go partially off screen
11:31 sima: jadahl, yeah xorg modesetting falls back to sw cursor any time the kernel ioctl fail
11:31 sima: so that's kinda the uapi contract for legacy cursor
11:32 sima: "exactly whatever nonsense xorg did, mostly as implemented by -modesetting"
11:32 sima: it's the "I want Xorg cursor uapi" uapi
11:32 jadahl: some very similar thing that is likely being used in mutter to get low latency cursor without running into atomic KMS problems
11:33 sima: jadahl, so maybe we should start out with documenting this stuff as it's used
11:33 jadahl: sima: that would be good
11:34 sima: we still have a lot of these legacy uapi warts, e.g. the recent patch from ville to no-op out dpms calls because apparently some xorg or whatever loves to do them
11:34 jadahl: fun
11:35 sima: well it's how legacy kms happened
11:35 sima: in theory it was a cross-driver api
11:35 sima: but in practice it started with every driver having it's matching xorg userspace, and so there was some really bad proliferation of messy and sometimes incompatible semantics
11:35 sima: and only very slowly we've nailed these down
11:36 sima: atomic tries to do a lot better
11:37 sima: we're probably still about as bad, since atomic is a lot more complex :-/
11:39 jadahl: it's more complex, but still not complex enough to handle this use case :P
11:42 sima: jadahl, well part is the intertia here of "legacy cursor seems good enough" and so no one put in the work to spec out what that should actually do
11:42 sima: and whether drivers actually implement it
11:43 sima: like you might randomly stall with legacy cursors, there's zero guarantees it wont eat a full vblank/atomic commit
11:46 jadahl: you mean there are zero guarantees legacy cursor movements won't eat a full vblank?
11:55 sima: jadahl, not zero, but there's no way for userspace to find out
11:55 sima: and not zero in the sense of "some drivers try pretty hard"
11:55 sima: with no userspace accessible definition of "some" and "try pretty hard"
11:56 MrCooper: sima: reality called to remind us it's still needed, see https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/4249#note_2357169 and other threads on that MR
11:56 sima: this is why I think we need cursor as async_flip within atomic and clear semantics
11:56 MrCooper: and the async flag alone is not equivalent
11:56 MrCooper: anyway gotta go, bbl
11:57 sima: MrCooper, oh I know it's needed, people wouldn't keep using it otherwise
11:57 sima: it's just, as long as we have that "you get what xorg driver-specific umd originally wanted" uapi, you won't get anything with clearly defined semantics
11:57 sima: it's kinda like atomic except no ALLOW_MODESET flag
11:58 jadahl: sima: well, I'm more or less trying to figure out what response to expect if drmModeMoveCursor() regresses when used with atomic, i.e. if the response will be "will fix" or "that worked by accident, you're on your own"
11:58 sima: MrCooper, also what you explain in that comment is pretty much what async_flip is for
11:59 sima: except not just base address, but x/y coordinates too
11:59 sima: jadahl, the future is hard to predict
11:59 jadahl: indeed
11:59 jadahl: if it's documented expected usage, it's easier
11:59 sima: you'll probably get signed up to define what exact semantics it is you want
11:59 sima: and then implement it
12:00 sima: currently it's a pile of hacks
12:00 sima: MrCooper, afaik on intel display cursor isn't async like on amdgpu btw
12:00 sima: jadahl, ^^ so another fun one
12:04 zamundaaa[m]: sima: TIL "xorg modesetting falls back to dw cursor any time the kernel ioctl fails"
12:05 zamundaaa[m]: That's more than most Wayland compositors do. Afaik KWin and Weston are still the only ones that do this with atomic even...
12:06 sima: with atomic?
12:06 jadahl: sima: how likely would it be to get a "DRM_MODE_CURSOR_MOVE is expected to work together without resulting in the next DRM_IOCTL_MODE_ATOMIC returning EBUSY or getting delayd one refresh cycle" doc patch accepted? :P
12:06 sima: we really should start documenting all these fallback expectations between kernel and compositors
12:06 sima: jadahl, nope
12:06 zamundaaa[m]: sima: most compositors expect being able to enable and move the cursor plane to any place without a test commit
12:06 sima: jadahl, well so the EBUSY would be a bug
12:06 jadahl: zamundaaa[m]: mutter tries to too automagically fall back to software cursors too
12:06 sima: that shouldn't happen
12:07 sima: zamundaaa[m], the other one is really best effort, and I think the only way to fix that is with atomic asyc_flip and some very explicit flags/semantics about how/when it should fail and what should happen
12:07 sima: zamundaaa[m], yeah that's not a thing
12:07 zamundaaa[m]: jadahl: good, so that got fixed at least. Definitely still plenty of compositors out there that don't though :/
12:08 jadahl: zamundaaa[m]: I think it has for a long time. maybe it stopped and got fixed. it was added due to some arm driver long long ago IIRC
12:09 sima: this is why I think vkms with arbitrary atomic_check restrictions implemented in ebpf would be really good for compositor testing
12:10 sima: because the list of things that work mostly, except on some hw is very, very long
12:10 sima: and so unless every compositor dev has a warehouse full of machines, you cannot test it
12:10 jadahl: sima: wouldn't that be nice :P
12:11 javierm: sima, zamundaaa[m] I think they did because cursor hotspots was not supported for atomic
12:12 emersion: sima: completely agree re: cursor API
12:12 sima: javierm, that's fixed now I thought?
12:12 sima: or at least the internal atomic machinery has hotspot support now
12:12 zamundaaa[m]: jadahl: definitely possible. This did regress in KWin a few times too, because it's so seldomly required in practice...
12:13 javierm: sima: yes, that's why I said "did", but I remember that argument was brought up when zackr added the hotspot support to atomic KMS
12:13 sima: another fun one that apple can bring back is connectors with status = unknown
12:13 jadahl: it might have regressed with the KMS cursor thread short cut, will have to check...
12:13 jannau: the apple HW limitation is 32 pixels (width and height) need to remain on screen. If there was a way to guarantee enough horizontal padding for the cursor fb it would work. vertical padding can be added in the driver. there is no cursor plane just overlays
12:13 jadahl: IIRC I added code to when adding the atomic support to bail on hw cursors if the atomic commit with a cursor failed
12:14 sima: afaict a lot of soc hw looks a lot more like apple than amd's display block
12:14 sima: jadahl, the biggest issue with legacy cursor is really that you cannot tell what it will do
12:14 sima: it might be an async x/y position update like in the comment MrCooper linked
12:14 zamundaaa[m]: For hw like that I really prefer to not have a cursor plane but just the universal / overlay planes
12:14 sima: or just async against concurrent atomic and wont eat a vblank
12:14 sima: or it'll eat a full vblank
12:15 sima: or it might "randomly" fail for arbitrary reasons
12:15 sima: there's just no rules really
12:15 jadahl: sima: I guess a deny/allow list is the best we can do then
12:15 sima: jadahl, nope, please nope
12:15 sima: we've had that with hotspot, it's just pain
12:16 sima: real semantic flags please instead of mutual guessing games
12:16 sima: hence async_flip in atomic
12:16 sima: with like actual docs of what it should do
12:16 sima: and a real contract that drivers need to obey or reject the request at least so userspace can fall back
12:16 javierm: jadahl: yeah, the deny list is painful because as sima said, when hotspot was fixed it didn't change anything in mutter until was patched to remove for example virtio-gpu from the list
12:16 jadahl: sima: we already need it. nvidia-drm apparently doesn't like drmModeMoveCursor()
12:17 sima:sighs
12:17 sima: also lunch is ready, I'll be back later
12:17 sima: jadahl, I'd like to avoid it for upstream at least, because of what javierm said
12:17 sima: it just cements the current disaster uapi forever
12:17 sima: and fragments the uapi landscape even more
12:17 sima: because no one will make sure the deny list is consistent across compositors
12:18 jadahl: we'll see. if some upstream driver starts to delay atomic commits that gets a drmModeMoveCursor() early, we simply need to add them to said list
12:18 jadahl: or s/starts //
12:21 jannau: think the correct thing on driver side would be not to expose legacy cursor uapi. the only issue might that drm helpers make that available as soon as a cursor plane exists
12:22 jannau: I can't remember any issues reported due to the lack of support for legacy cursors on apple hw
12:23 tomeu: zmike: hi there, would you have time to clarify what you meant by your last comment in https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/30096 ?
12:28 sima: jadahl, just looked and my patch to nuke it all is still not yet merged
12:28 sima: so essentially it just randomly breaks drivers
12:28 sima: which is really awkward if that assumption is ever more baked in
12:28 sima: breaks drivers = you can oops the kernel if you try hard enough
12:29 jadahl: sima: what does your patch nuke exactly?
12:29 sima: the legacy cursor not stalling other commits for most drivers
12:30 jadahl: so if your patch lands, drmModeMoveCursor() will stall other commits?
12:30 sima: yeah
12:30 sima: but fewer oopses
12:30 jadahl: heh, so exactly the thing that that mutter MR MrCooper linked to relys on *not* stalling other commits
12:31 sima: jadahl, https://lore.kernel.org/dri-devel/4getu2xtlxudcy53emipvtfxjnxg2mrupwfcekdjizjdtbk3k7@nlii76skfuh4/
12:31 sima: most recent discussion
12:31 sima: essentially the legacy cursor hack I've done is fundamentally busted
12:31 sima: and so you'd need an allow-list
12:32 sima: it's pretty much just i915, msm, amdgpu afaik
12:32 sima: everything else is "don't do that"
12:32 sima: otoh that patch is 7 years old by now, I guess people are just ok with being able to oops kms drivers if you try hard enough :-)
12:33 jadahl: MrCooper: sounds like maybe that deny list should be an allow list? ^^^
12:33 sima: jadahl, also for that special thing where the xy position update is async, I think it's really only amdgpu
12:34 jadahl: seems to have worked with other drivers in that mr
12:34 sima: jadahl, I mean worst case we'll just fake update the position and return immediately
12:34 sima: that's pretty easy to do
12:34 sima: well actually no
12:34 sima: jadahl, currently it works with most
12:34 sima: (if you ignore that you can blow up most)
12:35 jadahl: I'm getting mixed signals here it feels like :P
12:35 sima: jadahl, essentially from my pov it was a mistake, and the only easy fix is to just do stalls for drivers that don't have special cursor code
12:35 sima: jadahl, but the more userspace insists on no stalls, the more we just need to keep the oops in the kernel
12:36 sima: so it's a case of very hard rock and hammer
12:36 jadahl: doing kms is hitting rock bottom? :P
12:36 sima: jadahl, eventually we might get a cve and just have to land that, and break the world
12:36 sima: so I'd much prefer if the world insists a bit less on fast cursor updates concurrently with atomic
12:37 pac85: sima async flips on cursor means it could get torn right? Afaik legacy kms does effectively mailbox semantic
12:37 sima: pac85, undefined
12:37 sima: legacy does whatever legacy does essentially
12:37 jadahl: sima: people like their cursor movements smooth though
12:38 pac85: Though I think mailbox is the desidered semantics for cursor
12:38 sima: we have drivers that copy the provided image into the hw cursor, that definitely tears
12:38 sima: jadahl, I know
12:38 pac85: what about changing position?
12:38 sima: but they don't like them enough to be smooth to fix the mess we have
12:38 sima: pac85, who knows, I don't
12:39 pac85: I think that's what people want to optimize for
12:39 pac85: for minimizing cursor latency
12:39 sima: I know
12:39 sima: but as long as the contract is entirely undefined or just in random allow/deny lists in random compositors
12:39 sima: we'll keep spinning wheels
12:39 sima: and I'll keep sitting on an oops fix
12:40 pac85: I see
12:40 sima: and I've tried to fix this fully generically, it's kinda not doable
12:40 sima: it's really that much of a mess between inconsistent legacy semantics and very random hw limitations
12:41 pac85: I see
12:42 jadahl: as with the hotspot and cursor planes, I suspect the only viable way forward is still to have a list of allow/deny, and eventually an atomic replacement where one can predict whether it'll work as expected (not stall)
12:42 sima: jadahl, yeah and atm I think the only allow list is amdgpu and maybe i915/msm
12:42 jadahl: eventually the list would be defunct, since all drivers would support the non-legacy api, be it saying "can't" or "can"
12:43 pac85: what does the allow list do exactly?
12:43 sima: or other option is we go and nuke a lot of the atomic cursor support, but no idea what that could break
12:43 jadahl: allow drmModeMoveCursor() long before drmModeAtomicCommit() of a cycle
12:43 sima: pac85, whether you can use the legacy cursor together with atomic
12:43 pac85: ah I see
12:43 sima: jadahl, so that's the crux, the use-after-free fix is to just stall to the next atomic commit :-/
12:44 sima: or the previous one, wasn't sure anymore which one it is
12:44 jadahl: :(
12:45 sima: but iirc if you do a legacy cursor in between two atomic cursor plane updates, you just go boom
12:47 jadahl: I mean there is a risk that that'd happen. set cursor fb id to A, *commit*, *move*, set cursor fb to B *commit*
12:47 jadahl: why would that go boom?
12:48 zamundaaa[m]:is happy to have dropped atomic and legacy mixing in KWin a long time ago
12:48 jadahl: zamundaaa[m]: I've kept it out until now :|
12:49 sima: jadahl, so the design idea of atomic is that you have a string of updates
12:49 sima: which are ordered through completions
12:49 sima: specifically the hw_done and flip_done completions
12:49 sima: the legacy cursor hack that makes almost all other drivers work just short-circuits that chain in the middle
12:49 sima: which kinda works as long as you don't mix it
12:50 sima: but if you do, and the atomic commits around the cursor update are nonblocking
12:50 sima: then the chain of completions is broken
12:50 sima: and commit B starts cleaning up shit before A has finished
12:50 sima: lolz ensures
12:51 jadahl: commits only happen once per cycle. why would there be a chain of them?
12:51 sima: it might be that it's enough to block on A finishing, but tbh it's so messy it's really not clear to me
12:51 pac85: does atomic still have the limitation that async flips cannot happen together with cursor updates?
12:51 sima: jadahl, they can overlap in their phases
12:51 jadahl: hrm, ok
12:52 sima: pac85, not sure, wasn't designed like that, but maybe is the case if your driver does cursors with the legacy hack
12:52 sima: jadahl, it's really hard to hit, all I have is a bunch of kasan reports from various users
12:53 sima: and CI occasionally blowing up in i915 until it stopped using that stuff
12:54 sima: hm maybe I could split that patch up
12:54 sima: jadahl, how bad is stalling on the previous atomic commit?
12:55 jadahl: the *previous*?
12:55 sima: yeah
12:55 sima: that might be enough to stop the uaf
12:55 jadahl: any stalling would be terrible
12:55 jadahl: every missed frame is terrible
12:55 sima: yeah I can't fix the uaf without some stalling
12:55 sima: or we latch the entire thing into a thread
12:55 sima: at that point you'll never get a failure value though if it goes wrong
12:56 jadahl: compare the cursor missing a frame, and the whole screen animation missing a frame
12:56 jadahl: the latter is more jarring
12:57 pac85: regarding cursor updates with async, I said it based on the text on a kwin MR "Another annoyance with async atomic commits is that only FB_ID can be changed" https://invent.kde.org/plasma/kwin/-/merge_requests/4800
12:57 jadahl: apparently nouveau seems to handle the mixing well as well...
12:59 sima: jadahl, they all do as long as my hack is still there
12:59 sima: but they also almost all have an uaf in the kernel
12:59 sima: so right now your allow list is pretty much everything
12:59 sima: except I have a 7 year old bug on my hands I dunno how to fix
13:00 jadahl: ah, I see
13:01 zamundaaa[m]: pac85: yes, you can't do cursor updates with async right now
13:01 zamundaaa[m]: KWin "solves" that by assuming that it won't work and falling back to a software cursor while trying to do tearing
13:01 jadahl: tricky .. uaf and a smooth system, or no uaf and laggy system
13:02 sima: jadahl, well since userspace seems to insist that legacy cursor is the best we can ever do, I'm pondering whether I can fix this with some absolute horror show of in-kernel threading
13:02 javierm: pac85, zamundaaa[m]: yeah, I think is not limited to only cursor but legacy KMS does not really support async page flip
13:02 pac85: zamundaaa[m] ah I see. A lot of wayland compositors seems to just do tearing in a very unreliable way (like, they don't do it when the cursor is visible but also in some games it will just never work)
13:02 javierm: I remember had this issue when tried to add support in mutter to damage handling on legacy KMS
13:02 sima: it's not going to make the semantics of legacy cursors any better though
13:03 pac85: javierm legacy can do async flips fine all the time at least on amd
13:03 javierm: pac85: hmm, maybe was only related to dirtyfb that damage clips needed
13:05 pac85: javierm not sure what those things are, a bit outside my area of knowledge. I suppose related to compositor's damage tracking?
13:05 javierm: pac85: yes
13:05 pac85: Ah ok I see
13:05 javierm: pac85: https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2979 was the MR in case you are curious
13:06 pac85: thx!
13:06 jadahl: sima: what userspace is predictability. if we can get a cursor plane, make it behave like one, in whatever way you do. if not, give us an overlay plane, and we'll put a cursor there and move it a bit slower (ideally, mutter doesn't but I plan to add it)
13:09 sima: hm
13:11 sima: well I just stumbled over the dumpster fire of DIRTYFB legacy semantics getting worse too
13:11 sima: yay
13:13 sima:cries over 9e4dde28e9cd3
13:13 sima: the docs of dirtyfb are very explicit that yes it stalls, and it does so intentionally
13:13 pac85: uhm so reading a bit, if you scribble in the front buffer there is no guarantee it will show up on the display until dirtyfb is called?
13:13 sima: pac85, yes
13:14 sima: well, you can do atomic updates
13:14 sima: today is friday, and I feel like kms was a mistake
13:15 pac85: dirtyfb stalls on vblank? Then how does Xorg work? It can blit frames to the front buffer as soon as they are presented independently of vblank
13:16 sima: dirtyfb is called from your block handler
13:16 sima: so it batches up
13:16 sima: and there's been bug reports that it has to, or some dump shit application redraws at 100% cpu
13:17 sima: except everyone disagrees for their own little setup, so since we've done that drivers are randomly growing hacks to disable this again
13:17 sima: because having a consistent kms uapi is apparently not something folks want
13:17 sima: at least for legacy
13:17 sima: "eglgears is limited to 120fps"
13:17 sima: hence we must break dirtyfb for other people
13:17 sima:apologizes for the sarcasm
13:18 pac85: I guess dirtyfb should have had a flags arg and accept async in it?
13:19 sima: it's called atomic, we have it
13:19 sima: and can't fix old uapi
13:19 pac85: I mean if you are not doing front buffer rendering async flips are actually supported better on legacy kms since they work with cursor updates
13:20 sima: oh I thought you've meant "nonblocking" when you've said "async"
13:20 sima: they're not the same
13:20 sima: there's no async dirty upload anywhere really
13:21 sima: robclark, 9e4dde28e9cd3 is this really required, and why for msm only?
13:21 pac85: ah ok
13:32 mairacanal: anholt, could you ack (or nack if you want to keep the maintainership) https://lore.kernel.org/dri-devel/20250226-v3d-gpu-reset-fixes-v1-6-83a969fdd9c1@igalia.com/? thanks
13:54 robclark: sima: yes, really required.. and other drivers that support a combination of push and pull (command vs video) type displays likely want something similar.. but depends on how hw latches the changes, I suppose
13:54 sima: robclark, well the commit justifies it with "eglgears is limited to 120fps", which really, do we care that much for a legacy ioctl?
13:55 robclark: vblank_mode=0 should not be limited
13:55 sima: currently userspace has no way to find this out
13:55 sima: robclark, well when I did the atomic version for dirtyfb there was apparently some clear evidence that ratelimiting was needed
13:55 robclark: userspace should unconditionally dirtyfb.. kernel should nop it when it is not needed
13:55 sima: so something is not quite right
13:56 robclark: userspace isn't flipping, because vblank_mode=0
13:56 sima: yeah it's never flipping if it's dirtyfb
13:56 robclark: but dirtyfb should nop when it is not needed.. that is what that patch implements.. kms core doesn't really know, so I did it in driver
13:57 sima: robclark, yeah, but that's kinda not great if dirty ioctl can do stuff atomic userspace cannot
13:57 sima: all that achieves is further fragment semantics of legacy ioctls and proliferation of hacks
13:57 sima: (I started looking into this all again due to the endless legacy cursor saga)
13:58 sima: so really not a big fan if we merge new special semantics for legacy ioctl for one driver in 2022 or so
13:58 sima: we're trying to get away from that since a decade, and it's really hard
13:58 robclark: I'd have to go back and look at how this works w/ atomic, maybe it needs a similar trick.. but for sure _something_ is needed
13:58 sima: we = well maybe that many people perhaps
13:59 sima: robclark, maybe just flips lazily?
13:59 sima: instead of on every update
13:59 sima: or maybe a case of "xorg-modesetting doesn't do atomic, so hacking legacy is enough"
13:59 robclark: some combinations of wsi + compositor do that
14:00 sima: there's also intel's dirty fb, which is full blast nonblocking always
14:00 sima: so maybe that's the rule, but then we should implement that in the helper
14:00 sima: just push to a worker, fbcon already does that, we actually discussed whether we should except it looked like mostly dirtyfb should block
14:00 robclark: if compositor is managing things paced on vbl then all of a sudden we don't need hacks in the driver.. so I guess you could look at it as a workaround for legacy userspace
14:00 sima: yeah but then why just one driver
14:01 robclark: because I guess at the time dirtyfb was no-op for most anyone who didn't support dsi cmd mode panels?
14:02 robclark:bbiab
14:02 sima: there's a lot of drivers that have some ->dirty
14:03 sima: and we have a fairly common one for all atomic drivers, except i915
14:03 sima: and since 2022 msm
14:03 sima: and since 2023 amdgpu
14:03 sima: it's like ... going in the wrong direction here
14:04 sima: robclark, was even you who implemented the generic version ...
14:05 sima: so robclark from 2018 disagrees with the one from 2022
14:05 sima: hwentlan_, agd5f 1c6b6bd0780f2 looks funny for similar reasons of "why" and "are you sure"
14:06 sima: but it's after the msm one landed
14:19 pac85: I had this issue running X on msm of vblank_mode=0 not being unthrottled as it should, is that what that patch fixes?
14:20 pac85: seems to be from the commit message. FWIW Xorg on amdgpu doesn't behave like that
14:24 pac85: Also the fact that you get two frames in quick succession then one at the correct rate is probably why I was seeing the animation break in glxgears
14:28 sima: pac85, yeah amd has a hack too
14:30 pac85: sima: If I can "vote" I'd want the non blocking behavior, it makes X quite broken on msm and I'd imagine X is the biggest user of those APIs
14:30 sima: pac85, then I kinda wonder where we had that "must block" idea from
14:30 pac85: I'd imagine X is the oldest user too
14:39 pq: jadahl, sima, how about using eBPF programs to let the kernel do the input device -> cursor position without roundtripping to userspace at all. ;-p
14:40 pac85: brilliant idea. Get rid of kms, bring back user mode setting but through eBPF. Now the kernel can't be blamed for any bug
14:42 jadahl: lets go HID-BPF directly to KMS-BPF, bypass libinput too
14:43 jadahl: well, that is just a different way of saying what pq meant I guess :P
14:43 agd5f: to add to the fun, the cursor plane on AMD hw is not actually a fully independent plane. It inherits a lot of attributes from the plane it's enabled on
14:52 robclark: sima: I guess if I disagree with myself, it is only because kms core doesn't know about push vs pull outputs? But non-blocking dirtyfb might be nice (although would need a kernel thread since from hw pov it is blocking.. unless you go thru the pain that msm does to make cursor updates non-blocking
15:03 sima: robclark, more wondering why we didn't do it in 2018 and thought blocking is the right semantics?
15:13 robclark: yeah, idk.. blocking fits w/ how the hw works, at least in my case.. blocking for cmd mode panel is less problematic because you only need to block until you've squirted the pixels to the panel (more or less).. so blocking-but-nop-for-video-mode is a practical soln
15:20 MrCooper: no time to read through all scrollback, sorry; if I missed something important, please raise it again
15:20 MrCooper: sima: keep in mind mutter doesn't use overlay planes yet, so I fail to see a qualitative difference between my mutter MR and Xorg using the legacy page flip ioctl
15:21 sima: MrCooper, oh it's the same issue, just fewer people using Xorg
15:21 sima: *ever fewer
15:21 MrCooper: (quantitatively, my MR is easier on the kernel than Xorg, because it calls atomic commits and legacy cursor ioctl from the same thread)
15:21 MrCooper: still a lot of them though, I'd export to see reports of such failures
15:22 jadahl: MrCooper: the tl;dr seems to be, "kind of safe for amdgpu, i915 and msm; not safe for the rest if sima's patch ever lands"
15:23 MrCooper: should I make it an allowlist for those drivers then?
15:24 jadahl: could do. sad part is that it apparently helps with some nouveau issue too. until sima's patch lands, if it does...
15:25 sima: jadahl, MrCooper I guess I accepted the reality that I need to do some horrible thread hacks in the kernel
15:25 sima: just no idea yet how
15:25 sima: so ... eh
15:26 MrCooper: sima: also, the async flip stuff isn't enough to get all the same benefits, see the earlier MR thread about VRR
15:27 Ermine: seems like a topic for xdc talk
15:27 sima: MrCooper, kinda not sure why VRR doesn't ramp up frequency when you do async flips ... that seems like a driver bug
15:28 MrCooper: other way around, it does but shouldn't
15:28 sima: oh
15:28 MrCooper: (for cursor plane moves)
15:28 sima: hm why?
15:28 sima: sluggish cursor sounds kinda bad
15:28 MrCooper: guess it's counter-intuitive
15:29 MrCooper: it actually helps for that
15:29 sima: huh, link for context?
15:30 MrCooper: the fundamental problem is that we don't want to start a refresh cycle for only moving the cursor plane if there's new plane content coming "soon"
15:31 MrCooper: that thread starts at https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/4249#note_2352195
15:32 alyssa: can we get Xinhui.Pan@amd.com removed from MAINTAINERS? emails keep bouncing
15:32 sima: agd5f, ^^
15:32 MrCooper: so without truly asynchronous cursor plane moves, the compositor has to hold back cursor plane moves in case new plane contents show up
15:33 sima: oh
15:33 sima: so more flags needed I guess
15:33 MrCooper: I'm skeptical those flags are useful for other use cases though
15:34 MrCooper: so it's not obvious to me that it's really the way to go
15:34 zamundaaa[m]: MrCooper: cursor-only atomic commits don't actually change the refresh rate with VRR
15:34 sima: well random special-case semantics isn't great either
15:34 zamundaaa[m]: Not with amdgpu at least; I know because I hit that case
15:34 MrCooper: zamundaaa[m]: that would be a bug, we talked about it at XDC
15:35 zamundaaa[m]: Yeah, it is. But that's to say, "it does but it shouldn't" isn't right, we need both behaviors
15:35 MrCooper: atomic KMS API is that every commit for the CRTC starts a cycle
15:35 MrCooper: hence the (for cursor plane moves) clarification
15:36 zamundaaa[m]: While I suppose the compositor could commit the primary plane when it wants the refresh rate to be affected, that would at least technically be an API break
15:41 MrCooper: sima: BTW, by "thread hacks" do you mean kernel-internal threads?
15:43 sima: MrCooper, yeah most likely just the late commit trickery mutter does
15:43 sima: or lazy committery
15:43 sima: not sure what we can do really
15:44 sima: the problem is really that every driver that looked into this grew it's own set of hacks
15:44 sima: so there's really no consistent uapi contract for legacy cursor at all
15:44 sima: even on atomic drivers
15:45 sima: MrCooper, your assumption that legacy cursor move does not trigger vrr uprates is very bold
15:45 sima: for an uapi that has pretty much zero documented rules
15:46 MrCooper: FWIW, if a driver can't program the cursor plane move immediately, I'd expect it to just remember the position and program the latest position next chance it gets
15:46 pac85: I guess with legacy cursor api if you have low framerate compensation going with vrr your cursor actually moves at the "real" refresh rate?
15:46 sima: yeah that's not what's happening for most drivers
15:46 MrCooper: sima: I didn't originally make that assumption, it's really kind of a tangent
15:46 sima: MrCooper, well it's kinda the entire story for legacy cursor
15:47 sima: there's no clear rules at all, it's just both sides piling up hacks as we go
15:47 MrCooper: I'd argue it's the only thing that makes sense though
15:47 sima: and so every round this gets worse
15:47 sima: not sure legacy cursor is still in the "makes sense" territory
15:47 pac85: uh that seems to be the case with legacy api, cursor moves at 130 with 65fps content
15:48 sima: legacy cursor is mostly just random semantics + ever more hacks
15:48 MrCooper: pac85: yep, that's one benefit with amdgpu
15:48 sima: on both kmd (per driver hacks only ofc) and compositor side (also seemingly moving to per-driver hacks)
15:48 pac85: I guess legacy cursor on amd semantics is what they meant
15:48 sima: jadahl, btw out of morbid curiosity, how does legacy cursor blow up on nvidia-drm?
15:49 MrCooper: sima: getting tired of the "Wayland cursor latency sucks" complaints :(
15:49 sima: MrCooper, oh I know the issue
15:49 jadahl: sima: dunno exactly, I'm playing catch up after being somewhat away and still in atimezone where every one but me sleeps for most of the day
15:49 sima: it's just, we're ever more walking away from making kms an actual cross-driver api you can use with all these fixes
15:50 MrCooper: sima: no special handling, so presumably every legacy cursor ioctl results in a full-blown atomic commit
15:50 jadahl: and I've been trying to smack this god damn mosquitoe that is taunting me for 2 hours now
15:50 pac85: btw from the little I know about amd hw doing the mailbox cursor movement semantics legacy kms has is just a matter of changing the coordinate regs, then when something else flips the move takes effect
15:50 sima: MrCooper, I think the only realistic way out is to do legacy cursor with async_flip, fix the semantics of that
15:50 sima: and force a consistent emulation on all other drivers
15:50 sima: in the kernel
15:51 pac85: uhm I think async flip wouldn't be as good
15:51 sima: and then add flags to async_flip as needed to make it to the right thing
15:51 pac85: it needs to be "passive" for vrr and not tear
15:51 pac85: (unless the main plane tears)
15:51 sima: from an sw pov the main thing with async is that it doesn't hold up atomic commits but free-wheels
15:51 sima: everything else is hw features
15:52 sima: which might or might not exist
15:52 pac85: well, for main plane you want async to tear, for cursor you want it to not tear
15:52 MrCooper: sima: I'm not against that, just afraid I'm not gonna be the one pushing that in the near future, and too impatient to wait for it myself :)
15:52 pac85: kms gives you that on amd
15:54 MrCooper: sima: pac85 makes a good point though, async_flip would allow tearing, whereas we don't want cursor plane moves to tear (the legacy cursor ioctl isn't supposed to)
15:55 MrCooper: so that would make yet another flag: "async, no tearing though, no new refresh cycle please, and feel free to program before previous commits"
15:55 pac85: yeah, I think it would be accepatble for it to tear if you are also tearing the main fb but not otherwise (I'd imagine that's how and hw would work).
15:57 MrCooper: I suspect AMD HW doesn't tear the cursor regardless, not sure though
16:00 pac85: MrCooper: IMHO it should be a flag called mailbox, and then something you attach to the whole commit to say "no new refresh cycle for vrr" which I'd call passive update. I say this because having mailbox semantics that trigger a new refresh cycle could be useful to have for plane updates. It would allow to simplify how direct scanout is done (no need for the compositor to guess a deadline, just commit any frame) and achieve lower latency
16:03 sima: MrCooper, I'm mostly worried about how to make the kernel side no longer have uaf, and async_flip infrastructure is about the only way out there
16:04 sima: unless you just require that drivers implement bespoke cursors and get it all wrong in worse ways
16:04 sima: how you exactly smash this into the hw isn't my top concern, I'd just like to retire an 8 years old uaf eventually
16:05 sima: well known since 8 years at least
16:05 sima: meanwhile compositors seem to holler ever louder that they really rely on the semantics that uaf provides
16:05 sima: and mostly test on amd/i915 and msm, which all have bespoke legacy cursor handling anyway
16:06 sima: and then a quick test somewhere else, where "hey it works there too" because I haven't ripped out the uaf yet
16:11 MrCooper: if I make the mutter MR active only for those 3 drivers, would you be comfortable with that for now?
16:11 pac85: sima: I know very little of this area of the kernel but from what you said above, it sounds like having a chain of updates that is tied to flip_done (which I suppose means either blank or an event that happens immediately for async) for completion is fundamentally incompatible with the desidered mailbox semantics for cursors? Would it be possible to lift that restriction? A mailbox is fundamentally not a chain of updates
16:12 sima: MrCooper, it's a bunch more drivers actually, trying to swap in all the context again
16:12 sima: MrCooper, it's more me lamenting that we're still piling up hacks, and that I'm doomed to write one for the kernel to ever get the uaf fix in
16:13 MrCooper: sima: all the ones which reference legacy_cursor_update or atomic_async_check?
16:13 sima: pac85, yeah that's what the new infra does
16:13 sima: it's also not async_flip but async_update because of exactly the tearing vs not-tearing and other questions
16:13 sima: except I'm not sure amd does this right ...
16:14 sima: MrCooper, all those which have an atomic_async_check or a bespoke ->update_plane for their cursor (I think only i915 is in the latter)
16:14 sima: the legacy_cursor_check is mostly trying to handle fallout from the hack and doing slightly fewer uaf
16:14 sima: except amdgpu hand-rolling a bit too much of their atomic_check implementation
16:15 MrCooper: I see amdgpu, loongson, mediatek, msm, rockchip, tegra & vc4 referencing atomic_async_check
16:15 sima: nouveau hand-rolls something funny too
16:16 sima: MrCooper, essentially my problem is that I have no idea what the legacy cursor uapi is, and whether I can emulate it at all without an uaf
16:16 sima: and I'm not sure you're clear on the first part either
16:16 pac85: sima: ok so it sounds to me like it is a matter of making the distinction between async with tearing and without explicit so it becomes possible to request an async flip that doesn't tear on hw that supports tearing. That + lifting all the restrictions that make cursor updates not work with async flip would fix this properly I guess?
16:16 sima: because amd, i915, msm and nouveau all have very custom hacks
16:18 sima: and so that doesn't yet even cover a driver that does the new atomic_async_check in it's pristine form
16:18 sima: so maybe that one is still wrong for what you want
16:21 sima: plus we have a lot of atomic drivers with cursor planes that don't have an atomic_async_check
16:22 sima: and without atomic_async_check best I can do is either uaf or a thread that lazily pushes an update and hopes
16:22 sima: I think, not entirely sure
16:23 sima: MrCooper, I think the uapi you want is one where you get the return of atomic_async_check and not have an in-kmd fallback to something you don't like
16:24 sima: because I pretty much guarantee you that one is wrong for some cases or drivers no matter what
16:24 MrCooper: the kernel offering functionality to user space but hoping for the latter not to use it (while Xorg has been all along) isn't really great either
16:24 sima: oh it's all around bad, I know
16:24 sima: I'm trying to find a way that's not just making it worse
16:25 MrCooper: making anything worse is definitely not my intention
16:26 sima: like I guess a minimal uapi would be that we expose "is this a driver with cursor that has atomic_async_check" except that leaves the hacks in i915 and nouveau out
16:26 sima: plus you still don't know when it fails and falls back to something you don't want, like a full committ
16:27 sima: I think ideally we have an uapi where userspace either gets a real cursor (for a sufficiently clear definition of real) or errno
16:27 sima: and then we just emulate something that's a bit less buggy for legacy ioctl
16:28 sima: but yeah if mutter starts using legacy ioctl the uaf might pop up again a lot more
16:29 sima: and the only fix I can come up with for drivers that don't have atomic_async_update is going to have at least some stalls in some cases
16:29 agd5f: sima, alyssa patch was included in this week's -fixes PR
16:29 sima: stalls or lagging cursor, it's a bit a sliding scale
16:29 sima: agd5f, thx
16:29 alyssa: agd5f: cool thx
16:30 sima: MrCooper, so I guess at least for current kernels you have the following set of legacy cursor uapi
16:30 sima: 1. amdgpu dc
16:30 sima: 2. nouveau
16:30 sima: 3. msm
16:30 sima: 4. i915
16:30 sima: 5. all the others with atomic_async_commit
16:31 sima: 6. atomic drivers with cursors not yet listed above
16:31 sima: 7. whatever all the non-atomic drivers do, but I guess those don't matter since we're talking about drivers with atomic only
16:31 sima: none of these uapi are documented
16:31 sima: 6 is buggy
16:31 sima: 2 maybe too, haven't looked
16:32 sima: if you also include older kernels, it's more messy since the drivers with bespoke hacks all evolved on their own
16:33 sima: oh 6 is atomic drivers with cursor planes
16:33 sima: there's also an msm variant where the cursor exists, but it is not a cursor plane
16:33 sima: not sure how many of those we have in other places
16:34 sima: MrCooper, so the above mess is why I'm not super enthusiastic about you looking at this legacy cursor + atomic uapi combo and going "this is what I want"
16:35 MrCooper: that's not what I'm saying at all
16:35 MrCooper: I want comparable cursor latency as with Xorg, and this is the only way I can get it ATM
16:36 sima: yeah that's the other side, I'd like to give you that, hence why I've been pushing for ->atomic_async_check and things like that
16:36 agd5f: AMD cursors are double buffered and they latch on vblank so you can update them whenever you want for the most part
16:36 MrCooper: agd5f: latching only on vblank can't explain the numbers
16:36 sima: except there's no way for you to know you'll get a good cursor
16:36 sima: and not so much what it exactly means
16:37 sima: MrCooper, and if we go with an allow-list in mutter the incentives to create more drivers with good cursors are essentially just not there
16:37 agd5f: IIRC, there is a lock driver takes and it won't switch unless the lock is not held by driver
16:38 MrCooper: my assumption is that it can latch any time the current scanout line doesn't overlap with the cursor
16:39 agd5f: maybe it's changed on newer chips. My knowledge of this dates back the the pre-DC days :)
16:39 pac85: MrCooper: why not? with atomic you can only commit once per cycle and no matter how close you try to get to the actual limit there would be some time between your commit and actual vblank. With legacy you just keep changing the registers and it gets latched by hw exactly at vblank
16:40 sima: 0b8de7a04f7c1 maybe relevant? agd5f?
16:40 MrCooper: pac85: why not what? :) sounds like what I'm saying
16:40 sima: there's been a bunch of recent-ish changes
16:40 MrCooper: pac85: ah, I'm saying it can latch even after vblank
16:40 pac85: MrCooper: I'm saying that even if it doesn't it still explains lower latency
16:41 MrCooper: otherwise I can't explain the numbers measured with Xorg
16:41 MrCooper: pac85: nope, in that case mutter would be much closer to Xorg
16:41 pac85: uhm
16:41 pac85: can you share the numbers?
16:42 MrCooper: mutter commits ~1 ms before start of vblank and uses the latest cursor position available at that point
16:42 pac85: I thought you measured the latency
16:42 MrCooper: see the post linked from https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/4249#note_2357169
16:43 pac85: uhm I see
16:43 pac85: I suppose what you propose makes sense since amd has dedicated cursor hw
16:44 MrCooper: actually generally less than 1 ms before my MR
16:44 pac85: .9 of a frame yeah
16:45 MrCooper: right
16:45 MrCooper: consistent with the cursor being near the bottom, as on the picture
16:46 MrCooper: (the difference should be smaller near the top)
16:48 MrCooper: and the difference was more noticeable than I'd expected at 60 Hz
16:49 anholt: mairacanal: ack, I meant to be removed from all kernel maintainership long ago.
17:29 sima: tzimmermann, random thing I've noticed: I think you could unify a lot of the drm_fb_helper_funcs->fb_dirty implementations, I think only the ttm one still is special
17:29 sima: or I'm blind
17:29 tzimmermann: sima, i want this to go away entirely. but we're not there yet.
17:29 sima: even the damage_blit is probably fairly generic
17:29 sima: tzimmermann, ah even better :-)
17:30 tzimmermann: but now it's weekend here :)
17:30 sima: oh yeah here too
17:37 karolherbst: airlied: can I have multiple csos of the same nir_shader with llvmpipe? Because uhm.. that seems to be crashing for me inside LLVM
17:41 karolherbst: mhh actually the issue might be something else
17:45 zmike: I don't see why you couldn't
18:06 alyssa: does anybody like XSD? if so can you explain why
18:06 alyssa: i am trying to have an open mind here but
18:41 alyssa: relaxng wins :3
19:26 alyssa: btw, is there policy for python package build-time deps for Mesa?
19:26 alyssa: dcbaker: ^
19:27 alyssa: I'm pulling in a new package, it's in debian oldstable & fedora in addition to pip, but idk what the e.g. android build situation is like
19:27 alyssa: (I have it as an optional dep rn but obviously it's easier to require things instead of falling back)
19:27 dcbaker: alyssa: Not that I'm aware of? I'd guess people would probably frown at adding a dependency that does the same thing as an existing dependency (say adding jinja when we already have mako)
19:27 alyssa: sure
19:28 alyssa: it's rnc2rng & lxml, which together gives us rnc validation
19:28 alyssa: optional in the sense that you don't need validation if you don't change the xml file
19:29 alyssa: (i'm also falling in love with rnc, and discovered a new hatred of XSD, and i would like to start doing formal rnc schemas for more XML in tree.)
19:29 alyssa: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33814/diffs?commit_id=a65f30653a14c4e619524f23a9be79cd61585458
19:29 dcbaker: If you're adding it as a unittest then the requirements probably pretty lax
19:29 alyssa: no, core build script
19:29 dcbaker: as in, as long as it skips/doesn't run gracefully when dependencies are missing I doubt anyone cares.
19:30 dcbaker: I also do not like XSD
19:30 alyssa: since if you have the deps, it should run before the build (not after) so you don't get stupid KeyErrors or whatever later
19:30 alyssa: see the link for what I did
19:31 alyssa: (tangentially it's a bit annoying I can't find a one-shot rnc validator packaged in fedora but oh well)
19:33 dcbaker: Hmmm, yeah. That's probably fine.
19:33 dcbaker:should get around to adding something to meson to allow people to add to the lint target...
20:06 alyssa: dcbaker: also, any thoughts on us growing a python util/ for sharing stuff like safe_name?
20:07 pinchartl: alyssa: are you planning to fix the android buid issue ? :-)
20:07 pinchartl: s/buid/build/
20:07 dcbaker: alyssa: I have wanted to have a shared location for python modules
20:07 dcbaker: I can't remember what stalled that TBH
20:08 alyssa: pinchartl: what issue?
20:09 alyssa: dcbaker: OK
20:10 pinchartl: alyssa: the fact that android doesn't support meson
20:14 alyssa: Oh
20:14 alyssa: sounds like a google problem (:
20:17 mattst88: yeah
20:17 mattst88: FWIW, we're working on https://github.com/rjodinchr/ninja-to-soong
20:18 mattst88: idea is: you configure mesa with meson against the Android NDK to generate build.ninja files
20:18 mattst88: then ninja-to-soong translates that to Android.bp that you check into your mesa repo
20:19 mattst88: code generated during the build is pre-generated using the system tools, and then checked into the mesa repo along with Android.bp
20:20 mattst88: currently working on reducing the amount of stuff that needs to be pre-generated
20:28 dcbaker: mattst88: I’ve also been talking to Google people about a meson -> hermetic build thing that would use meson to generate song and bazel? Is that different effort?
20:30 mattst88: I know there's a "meson2hermetic" tool in AOSP's /external/mesa3d repo -- is that it?
20:30 mattst88: we looked at that and ... were not impressed
20:31 mattst88: IIRC it converted a meson build system into a single massive python program that would generate Android.bp, I think
20:47 Lyude: alyssa: yeah honestly most of what's left is just writing more kms bindings, but I think the hardest parts of that are more or less complete at this pint
20:48 Lyude: the actual abstraction stuff is afaict mostly figured out
20:48 Lyude: though I've only gotten review from sima and a few rust people
20:48 Lyude: (I will be sending out a new version of the patch series next week without the WIP tag btw)
20:48 alyssa: Lyude: yay!
20:48 alyssa: ..what are you replying to
20:49 Lyude: 11:18 <alyssa> also, we already have rust GPU driver that needs to get upstreamed as rust
20:49 Lyude: 11:18 <alyssa> and once that's upstream, together with Lyude's KMS bindings upstreamed
20:49 alyssa: ah right
20:49 alyssa: Lyude: I am like DRAM
20:49 alyssa: I will forget things unless you are constantly refreshing my memory
20:49 Lyude: np :P, I am the same
20:50 Lyude: but yeah - granted, the stuff we have right now is very bare (we don't even really have proper state iterators), but I've been focusing on trying to get dependencies and what we have so far upstream which is why there's not much more yet.
20:50 alyssa: yeah =D
20:50 Lyude: i'm kind of glad I've been keeping it at this state because it's already a lot of jumping around just handling the handful of deps I have for this :p
20:55 alyssa: yeah, for sure