03:25bbhtt: hello, seeing a build failure with 24.2.1
03:25bbhtt: logs only say `FAILED: src/nouveau/headers/nvh_hwref_gp100_mmu.rs ` any idea?
03:26bbhtt: https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/jobs/7692988910/artifacts/external_file/cache/buildstream/logs/freedesktop-sdk/extensions-mesa-mesa/668a5254-build.20240829-025058.log
03:47airlied: bbhtt: might be a bindgen bug
03:47airlied: gfxstrand: ^?
03:49bbhtt: 24.2.0 was working with the same bindgen
06:43dj-death: how come RADV doesn't ever return VK_ERROR_FRAGMENTED_POOL ?
06:43dj-death: for vkAllocateDescriptorSets
06:56tzimmermann: mripard, hi. shall i send another PR for drm-misc-fixes today?
06:56mripard: tzimmermann: I'm back, I can take care of it
06:56tzimmermann: ok, great
06:57tzimmermann: thanks
06:57mripard: thanks for sending them over the last couple of weeks :)
09:24jani: what's the consensus nowadays in kernel, is it fine to declare the loop variable within the for loop, i.e. for (int i = 0; i < N; i++)?
09:56Mary: DavidHeidelberg: yeah I never gave midgard opencl a try and don't really have anything to test it sadly
10:57DavidHeidelberg: Mary: you have my sword... Ehm, you can use the CI :D
11:02mairacanal: jani, I believe so, as we are using C11
11:35jani: mairacanal: but it was possible with gnu c way before, but the direction from Linus was no
11:51ManMower: sima: (or anyone else for that matter...) can I bother you for a quick knee-jerk stink check on https://pastebin.com/t8JjGNiK ?
11:53sima: ManMower, looks like a good idea, assuming we're somewhat consistent with gating magic hdmi connector behaviour on connector->funcs.hdmi ...
11:53sima: but I think this is more for mripard to judge
11:56mripard: On principle, I'm fine with trying to streamline it as much as we can, but it's inconsistent with how other properties are typically handled
11:56mripard: so the discussion (and the solution) feel much larger to me
11:58ManMower: my specific problem is using a bridge, not really wanting to write all my own vfuncs and do "all that connector work", but the bridge code provides its own connector_reset helper which doesn't reset the hdmi properties. so maybe I've presented the wrong X for this X/Y problem. :)
11:59ManMower: wrong Y, even.
12:00mripard: the bridge stuff should deal with hooking everything for you to leverage the HDMI infrastructure code already
12:00mripard: if it's missing a call to that reset, then it's indeed a bug and should be fixed in the bridge code
12:01ManMower: I can do that - I think I still need the code motion from the front end of my pastebin though
12:01ManMower: as simply calling the function where it is now results in a module dependency loop
12:01mripard: I think lumag has fixed that one already?
12:01ManMower: oh that's delightful
12:02mripard: https://lore.kernel.org/dri-devel/20240715-drm-bridge-connector-fix-hdmi-reset-v4-0-61e6417cfd99@linaro.org/
12:03mripard: which also seems to fix your issue
12:03mripard: I think it would be worth sending the last patches as is without the immutable discussion
12:04ManMower: that definitely looks like it fixes my problem
12:04mripard: lumag: ^
12:05lumag: mripard, yep. I was OoO for three weeks. Now I hope to get back to the patches
12:13mripard: lumag: welcome back :)
12:48ManMower: mripard: oh, I have another question... when hdr_output_metadata changes, I need a mode change to cause the new DRM infoframe to be written. is that something I should handle in my encoder atomic check function, or is that generic enough that it could happen in drm_atomic_helper_connector_hdmi_check?
12:55mripard: both would be acceptable, but I'm not sure which one is best
12:55mripard: I'm not familiar enough with HDR metadata to tell
13:10sima: ManMower, so the idea is that if connector props have changed that require a full modeset you set crtc_state->connectors_changed from your connector's atomic_check
13:13sima: drm_atomic_helper_check_modeset should have enough retry trickery to cope with that kind of stuff from the connector's ->atomic_check
13:18sima: ManMower, I think for bridges there's no other way to update connector stuff than full modeset, so yeah I think unconditionally doing that makes sense
13:18sima: if people are unhappy about that we'd need to add more callbacks to adjust that without a full modeset
13:19sima: e.g. i915 can fix up infoframes without a full modeset
13:19ManMower: I think for HDR10 it's probably legit to require a modeset. HDR10+ or DV or something with the dynamic metadata... I have noooo idea
13:20ManMower: well, "legit" is doing a lot of heavy lifting. I don't expect my display not to flash once at the start of an hdr10 movie. but that's subjective and personal.
13:20sima: yeah for dynamic infoframes updates we might need to add them to the right states, make sure they're computed correctly and then that there's functions to update them on the right vblank
13:21sima: it's all a bit a mess and requires a lot of hw specific knowledge, so for bridge chains I'd say we skip this for now and just force a full modeset
13:22ManMower: it appears I'm setting crtc_state->mode_changed and not crtc_state->connectors_changed from my bridge atomic check function, is that ok?
13:23MrCooper: you're saying a modeset would be required for changing HDR metadata, even if HDR was already enabled?
13:23sima: you need ->connectors_changed
13:24sima: they all get reflected in drm_atomic_crtc_needs_modeset, but each bit is owned by different parts
13:24sima: mode_changed is for the crtc
13:24ManMower: MrCooper: me? I'm more asking than saying... and specifically static metadata like HDR10
13:24sima: connectors_changed for connectors
13:25sima: and active_changed is very specific
13:25MrCooper: FWIW, that would seem bad from a Wayland compositor PoV
13:26sima: ManMower, the thing is that drivers can assume that they own mode_changed and some reset it, if the mode change can be done without flickering at the crtc level
13:26sima: so you can't touch that from non-crtc code
13:26sima: docs could maybe be a bit improved
13:28ManMower: sima: oh dear. we're calling drm_atomic_helper_connector_hdmi_check() from drm_bridge_funcs->atomic_check in our drive. the former sets mode_changed for some common state change patterns...
13:29ManMower: I guess we can't call that from a bridge vfunc
13:29sima: yeah that should all be connectors_changed I think
13:29sima: hm wait
13:29ManMower: I suppose my move here is to messily refactor the guts of drm_atomic_helper_connector_hdmi_check(), or open code a copy that sets connectors_changed?
13:29ManMower: or waiting, I can wait
13:30ManMower: MrCooper: that's good to know, but can you tell me why?
13:33sima: ManMower, ok so correction, you can set connectors_changed from encoder and hence also bridge functions
13:33sima: it does fall apart on hw that supports cloning, i.e. more than one connector
13:33sima: so maybe we should encode that with a WARN_ON check, but ... eh
13:34sima: you really shouldn't touch mode_changed, but as long as your bridge chain never runs on a driver that does touch mode_changed in that driver's crtc code, you are fine
13:34ManMower: so I *can't* just set mode_changed, but I *should* set connectors_changed
13:34ManMower: hehe, that's probably a safe assumption, but nasty to make
13:34sima: yeah I think you should just change that code to set connectors_changed instead
13:35sima: since that's officially your own turf as a bridge/connector/encoder thing
13:35sima: so maybe also a doc patch that bridge_funcs->atomic_check can set this to true
13:35sima: and maybe that mode_changed really shouldn't be touched by bridges who can't make assumptions about what the crtc code does
13:36ManMower: if I was writing connector code and not bridge code, would mode_changed be ok? (ie: drm_atomic_helper_connector_hdmi_check is doign the right thing, I just ought not call it from where I am)
13:37sima: also mode_valid should maybe check that drm_atomic_add_affected_connectors wouldn't add anything if connectors_changed changed
13:38sima: it's all a bit a convoluted mess
13:49zamundaaa[m]: ManMower: changing the transfer function and setting / unsetting the metadata should require ALLOW_MODESET from the compositor, because some displays don't handle it very nicely
13:50zamundaaa[m]: Changing the other bits should not require it though
13:52ManMower: zamundaaa[m]: that seems very reasonable to me. the display I'm using falls into the "not very nicely" category. :)
13:53ManMower: zamundaaa[m]: so some changes of the content of the DRM infoframe should be ok without a modeset?
13:53ManMower: this causes me a problem on the bridge side, because I need to disable/enable to get the DRM infoframe to write. :/
13:55zamundaaa[m]: If you can't do it without a modeset, then you just can't do it and compositors will need to handle that
13:55zamundaaa[m]: FWIW KWin never changes the contents without a modeset anyways, dunno about other compositors though
13:59ManMower: I'm most familiar with weston, and I think it currently only sets it once at startup. not the best test case. :)
14:04MrCooper: ManMower: specifically, mutter/gnome-shell generally doesn't want to do any modesets outside of the user explicitly changing display settings (normally in gnome-control-center), presumably it'll want to change HDR metadata on the fly though
14:07ManMower: MrCooper: I have a display in the other room that blacks out for 7 seconds when you change the transfer function. it might be best to limit on the fly changes.
14:09Company: if a display blacks or not depends on which properties you change
14:09MrCooper: then compositors might want a way to detect that ahead of time, so they can pick suitable static metadata instead
14:10Company: however, I don't think it's documented anywhere which properties do or do not black
14:11Company: what I expect to happen is that compositors figure out a set of properties that can be changed at runtime and use those
14:16ManMower: I think wanting to change static metadata dynamically may lead to sadness.
14:25Company: depends - when you want to watch a movie, switching to the movie's tf is kinda useful, especially on a long flight - but for compositing your regular desktop you might want to use a different tf
14:26Company: and now you have the problem where (un)fullscreening your video player is the time when you want to switch
14:26gfxstrand: bbhtt: I'm not seeing the error in that log
14:26Company: or when you want to overlay the "low battery" warning over the movie
14:27gfxstrand: Oh, I see it now
14:27gfxstrand: Yeah, I have no idea
14:28ManMower: Company: yes, but wanting those things doesn't make my display not blank for 7 seconds when you try to do them. :)
14:29Company: yeah, but now what?
14:29ManMower: interact with hardware in a way that your users don't hate, I guess?
14:29Company: you mean run out of battery?
14:29Company: or blank screen for 7s?
14:30ManMower: you could name the buttons in the configuration dialog exactly that :)
14:30Company: users want the "do the right choice automatically" button
14:32bbhtt: gfxstrand: Might be some sort of race condition or something in CI, I just tried mesa 24.2.1 in our master branch and it worked
14:32Company: ManMower: is that really 7s btw? My monitors manage in 1-2s so 7 is substantially worse
14:32ManMower: I think we're a bit off in the weeds now. if displays exist that take a long time (I'll concede 7s is pathological, but someone built it, and it exists), then you'll probably want that to be protected by the mode set flag
14:32bbhtt: gfxstrand: I'll trigger the CI again on that branch, although both branches have same mesa, bindgen and rust
14:32ManMower: it's probably closer to 8, but yeah, it's obnoxious
14:32gfxstrand: Could be? Seems odd, though, because I don't think bindgen should be depending on any other generators
14:46MrCooper: ManMower: what I mean is: at least some compositors will want to change metadata dynamically; when that's not possible though, they might want to use static metadata which differs from the one they'd start with assuming they could change dynamically
14:47MrCooper: hence the need for a way to detect ahead of time, before trying to actually switch on the fly
14:47ManMower: MrCooper: isn't that what the mode set flag is for?
14:48ManMower: it's kind of a blunt instrument, especially if you're going to try every possible hdr_output_metadata blob you can build and find they all fail
14:49ManMower: but I think (and I'm literally just making this up) it'll come down to: someone has a monitor somewhere that can't change *any* HDR10 static metadata settings without a lengthy resync.
14:49MrCooper: good point though, I guess a TEST_ONLY commit which changes only metadata could work
14:50sima: imo we should first do correct, we means more modesets with bridges involved
14:50sima: then make stuff more dynamic, assuming compositors, hw, drivers and displays can cope
14:50ManMower: I love that answer because it's easy for me in the short term. :)
14:51MrCooper: if that happens once at session startup, that's "fine", since I understand something like that might happen when enabling HDR anyway; if it happens in the middle of the session though, not great
14:51ManMower: and I have a limited collection of HDR capable displays to test on
14:51sima: MrCooper, compositors can at least figure that out with TEST_ONLY and then bail
14:51ManMower: the problem with static metadata is you'll almost certainly want to change it if you start playing a video
14:51sima: which maybe not great, but at least correct (I think)
14:52MrCooper: yeah, sorry, didn't think of that obvious solution :/
14:52sima: ofc this might mean that we needless run the color pipeline with maybe a gpu mapping job while playing video, wasting power, but I think it should at least all ok
14:53sima: and it's too hot here, can't english anymore
14:53ManMower: the run out of battery button.
14:53ManMower: but everything looks correct
14:57zamundaaa[m]: ManMower: that "almost certainly" isn't true for KWin
14:57Company: has anybody ever figured out if monitors take more energy in HDR modes?
14:58ManMower: Company: almost certainly has to be true for a monitor with zoned backlights?
14:58Company: dunno - I'm assuming you have equivalent brightness and display only SDR content
14:58ManMower: I don't think I can just crank my brightness settings to the max and have them light up in SDR mode
14:59ManMower: zamundaaa[m]: are you saying KWin users won't want to / be allowed to purchase such a display?
14:59zamundaaa[m]: what? I'm saying that it never changes the hdr metadata
14:59ManMower: ah, I context switched. fwiw, I like your design.
14:59zamundaaa[m]: It just wouldn't be a good user experience if a video or game looked different fullscreen vs windowed
15:30ManMower: Company: anecdotally, the monitor I have on my desk seems to consume a maximum of around 60 watts in SDR mode with the brightness cranked, but will hit almost 90W in "HDR 600" mode, depending on content.
15:31Company: yeah, I expect massively more usage for high brightness situations
15:31Company: what I'm wondering about is how many nuclear plants you need extra when you just switch all monitors into HDR at boot
15:32ManMower: for equivalent brightness, this seems about the same
15:32ManMower: so you could switch this into HDR at boot and render a desktop that doesn't make your eyes bleed, and you probably aren't destroying the planet
15:33Company: if that is true - and so far I haven't seen anything indicating it isn't - it would make a lot of things a lot easier
15:35Company: I mean, it's not there yet because compositors and KMS need to drive those monitors properly in HDR mode without wasting tons of energy doing the color conversions in the wrong place
15:36Company: but it'd make the end goal clear
15:37ManMower: depends on a lot of things. you could still end up with a display that only supports static metadata, and a desire to play a video with a different MaxCLL/MaxFALL than what you set your GUI up for
15:37zamundaaa[m]: KWin doesn't really use any more energy or noticeable amount of GPU power for HDR
15:37zamundaaa[m]: The problem for enabling HDR by default is more that many drivers are still broken, and probably some displays just have really terrible behavior
15:38zamundaaa[m]: Afaik MS is solving the latter problem for Windows by checking the certification of the display
15:39ManMower: is that exposed through EDID? or are they expecting "monitor drivers" from manufacturers?
15:40zamundaaa[m]: I would expect VESA to have a database for which displays have which VESA certification
15:43zamundaaa[m]: Lol, there's a giant table on their website: https://displayhdr.org/certified-products/#tab-400
15:44zamundaaa[m]: Now we just need that in a more machine readable format
15:46zamundaaa[m]: ManMower: afaict from https://learn.microsoft.com/en-us/windows/win32/direct3darticles/high-dynamic-range, automatically enabling "auto color management" depends on the display manufacturer to do... *something*
15:59ManMower: zamundaaa[m]: interesting, thanks. apparently I have a display that's in the vesa certification table, has windows "drivers" for calibration profile etc, but shows "HDR cerification: not found" in the display app. I wonder if it's even a vesa certification, or if they have their own certifaction program. :)