04:09adhami3310: hey sorry i'm new here, i was looking into https://gitlab.freedesktop.org/xdg/xdg-sound-theme and it seems it's no longer maintained, if i make a MR, is there anyone to approve of it/merge it or mark a release?
07:19narmstrong: lumag: I’m ok if the meson change goes into msm next
07:38lumag: narmstrong: would a-b it?
07:39lumag: Also... do we need somebody's ack for the first patch?
07:41lumag: or for https://patchwork.freedesktop.org/patch/712670/?series=163431&rev=2 ?
07:42mripard: lumag: works for me too
07:46lumag: mripard: which one? :-)
07:49mripard: the MIPI RGB 101010 one :)
07:50lumag: thanks!
07:51lumag: What about the https://patchwork.freedesktop.org/patch/712670/?series=163431&rev=2 ? I'd like an ack from somebody else, also looking at DSI and DSC.
07:52lumag: But... I'm not sure if anybody else has DSI DSC support.
09:28bbrezillon: robclark: I recently copied your can_block() helper to panthor for my shrinker implementation, and Claude rightfully pointed out that some tests are redundant there (last bullet in https://lore.gitlab.freedesktop.org/drm-ai-reviews/review-patch9-20260309151119.290217-10-boris.brezillon@collabora.com/) 09:30bbrezillon: it looks like `return sc->gfp_mask & __GFP_DIRECT_RECLAIM;` would do exactly the same the current code does, question is, is this what we want :D
14:03emersion: tomba: will look again at the formats patches tomorrow!
14:04tomba: emersion: alright, thanks!
14:05emersion: (i generally have time on Thursdays - other days i have $daywork)
14:08tomba: (note to self: post patches on wed)
14:13emersion: lol
14:21robclark: bbrezillon: __GFP_RECLAIM is a superset of __GFP_DIRECT_RECLAIM
14:23bbrezillon: robclark: yep
14:23bbrezillon: so, if __GFP_DIRECT_RECLAIM
14:23bbrezillon: is set, the following test is always true
14:25robclark: true, but the current_is_kswapd() might not be.. still need caffeine but I think dropping that 2nd check would change the result in thing ___GFP_KSWAPD_RECLAIM case
14:26bbrezillon: so, at the moment, __GFP_DIRECT_RECLAIM is the only situation where can_block() can be true
14:26bbrezillon: I wonder if what we want is
14:26bbrezillon: static bool can_block(struct shrink_control *sc)
14:27bbrezillon: {
14:27bbrezillon: return (sc->gfp_mask & __GFP_DIRECT_RECLAIM) ||
14:27bbrezillon: (current_is_kswapd() && (sc->gfp_mask & __GFP_KSWAPD_RECLAIM));
14:27bbrezillon: }
14:27robclark: yeah, that should be equiv
14:28bbrezillon: equiv to what?
14:28robclark: equiv to the current can_block()
14:28bbrezillon: not quite
14:28bbrezillon: we never get passed the first check, at the moment
14:28robclark: oh, right
14:28bbrezillon: if DIRECT_RECLAIM is not set, we just bail out
14:31robclark: right.. I think that is what we want.. with the disclaimer that it's been a while since I'd dug thru that
14:32robclark: I wonder of claude could learn to linewrap?
14:33bbrezillon: :D
14:37bbrezillon: robclark: since we're here, talking about the shrinker, any chance I could bribe you or anyone in the MSM team to look at the panthor shrinker?
14:38robclark: ok
14:39robclark: bbrezillon: btw, maybe this would be clearer:
14:39robclark: https://www.irccloud.com/pastebin/7DEM47dB/ 14:39bbrezillon: happy to review other stuff in exchange, if that's of any help
14:40bbrezillon: robclark: hm, but we still never get passed the first check, do we?
14:42robclark: well, depends on the sc flags
14:42robclark: if shrinker is telling us not to block, we shouldn't
14:44bbrezillon: to me, DIRECT_RECLAIM == start the shrinker from my execution context, while KSWAP_RECLAIM == kick kswapd to reclaim in the background
14:46bbrezillon: is kswapd adjusts the sc->gfp_flags to add DIRECT_RECLAIM, that's okay, but then the is-this-kswapd check is irrelevant, isn't it?
14:47bbrezillon: if, on the other hand, kswapd doesn't touch the sc->gfp_flags, we need to consider DIRECT_RECLAIM and in_kswapd+KSWAPD_RECLAIM separately
14:47bbrezillon: which won't happen if we bail out when DIRECT_RECLAIM is not set
14:47bbrezillon: am I missing something?
14:47robclark: so originally the first check was bailing if __GFP_ATOMIC.. 7860d720a84c
14:48bbrezillon: yep, I've noticed that too
14:48robclark: so I think !DIRECT_RECLAIM should be equiv.. or at least that is my recollection
14:53bbrezillon: then the following line is still confusing to me
14:53robclark: yeah, ig the first check should be __GFP_RECLAIM
14:57bbrezillon: let's sum up, if it's DIRECT_RECLAIM, we can block in the caller context and in the kswapd context, if it's KSWAPD_RECLAIM only, we can only block in a kswap context
14:58robclark: right
15:00bbrezillon: so this
15:00bbrezillon: static bool can_block(struct shrink_control *sc)
15:00bbrezillon: {
15:00bbrezillon: if (!(sc->gfp_mask & __GFP_RECLAIM))
15:00bbrezillon: return false;
15:00bbrezillon: return current_is_kswapd() || (sc->gfp_mask & __GFP_DIRECT_RECLAIM);
15:00bbrezillon: }
15:00bbrezillon: or
15:00bbrezillon: static bool can_block(struct shrink_control *sc)
15:00bbrezillon: {
15:01bbrezillon: return (sc->gfp_mask & __GFP_DIRECT_RECLAIM) ||
15:01bbrezillon: (current_is_kswapd() && (sc->gfp_mask & __GFP_KSWAPD_RECLAIM));
15:01bbrezillon: }
15:01robclark: 2nd looks more clear to me.. and gets rid of the double-negative :-)
15:01bbrezillon: yep, I agree :D
15:02bbrezillon: I'll go with that in my v6
15:02robclark: sg
15:03bbrezillon: I'll let you take care of MSM, unless you want me to add a patch for that
15:05robclark: either way is fine
16:20openglfreak: Hi, on AMD with non-default-priority user queues (either high or low) it seems that once a queue is scheduled out once, it will never get scheduled back in automaticelly by the MES and make progress even if more work is submitted and the doorbell is rung. Any ideas why this might happen?
16:21openglfreak: On GFX11 / 7900 XTX I should add
16:45agd5f: openglfreak, are you updating the wptr location as well? If a queue is swapped out, you need to make sure the wptr memory is updated as well so firmware knows that the queue has work.
16:48openglfreak: Yes, I'm doing the same thing radeonsi does, which is to write the wptr value in GTT and then write the wptr value to the doorbell page, if I understand correctly
16:49openglfreak: `_mm_mfence(); *(volatile uint64_t *)userq->wptr_bo_map = userq->next_wptr; _mm_mfence(); *(volatile uint64_t *)&userq->doorbell_bo_map[AMDGPU_USERQ_DOORBELL_INDEX] = userq->next_wptr; _mm_mfence();`
16:53openglfreak: This can be reproduced on radeonsi btw by hacking mesa code such that the queue is created with a priority other than normal, and then running an opengl compute workload
16:55openglfreak: I assume it doesn't happen on normal priority solely because there's enough random submissions of work that the MES is constantly kicked anyway
16:57openglfreak: Oh, another thing, a scheduled out queue does make progress if another application submits work at the same priority level, but then of course stops again once it gets scheduled out the next time
17:56openglfreak: What's the difference between addresses of the form 0xffff800100001000 vs 0x800100001000?
20:50mlankhorst: airlied, sima: https://lore.kernel.org/all/a89da668-578f-4ae0-809b-f0e68e3550e1@roeck-us.net/ -- any insights?
20:53sima: mlankhorst, yeah your patch doesn't look sound
20:53sima: the drm_dev_enter/exit should be in driver code, not core code
20:53sima: and also looks like it's really an independent fix
20:54sima: mlankhorst, the second hunk with the fb cleanup is just plain wrong imo
20:55sima: the way this is supposed to work is that you're supposed to only call drm_mode_config_cleanup when the drm_device gets cleaned up
20:55sima: meaning a drmm cleanup action
20:55sima: not a devres cleanup action
20:55sima: almost all drivers get this wrong
20:55mlankhorst: sima: https://patchwork.freedesktop.org/patch/706994/?series=162133&rev=2 20:55sima: but the fix is to use drmm_mode_config_init, not add hacks to drm_mode_config_cleanup
20:56sima: mlankhorst, so yeah, your drm patch imo needs to be reverted, and then you need xe/intel-display fixes
20:56sima: do you need me to type this up on the m-l or is this here good enough to cook up a revert commit message?
20:58mlankhorst: Feel free to revert for now, but trying to figure out a proper fix. The problem is the IGT testcase I linked.
20:58sima: yeah, intel driver bugs 2x
20:59sima: mlankhorst, I don't think the issue is severe enough for me to push a revert directly, normal process and all that seems sufficient
20:59sima: I'll type a reply on-list so guenther is in the loop too
21:05mlankhorst: is a drm_device reference held when opening a drm_file?
21:08mlankhorst: ah hidden in drm_minor_release
21:11sima: yup
21:12sima: mlankhorst, thellstrom_ pointed it out, but I guess you got confused because there's an entire pile of bugs in the average driver of destroying way too much at remove/hotunplug time and not correctly delaying it all to drm_device teardown
21:12sima: which is also why we've added a pile of drmm_ functions for crtc/plane/connector/encoder setup, so drivers get this less wrong
21:12sima: anyway, typed up a longer reply on-list