00:35Kayden: mareko: okay, thanks!
01:10airlied:likes catching a630_vk 2/2 fail just before marge kicks me out
01:15jenatali: I'm assuming someone's working on making that less flaky?
03:31HdkR: just rep movsb amirite?
03:31jekstrand:hates LLVM for sticking so much memcpy in these stuid kernels
03:32jekstrand:should probably just teach nir_opt_copy_prop_vars about memcpy
07:05mareko: airlied: would moving drm kernel development to Gitlab be OK with regard to how it's email-based currently?
08:12MrCooper: mareko: DRM will test the waters there; if it works well, other subsystems might follow
09:10tomeu: argh, more opportunities to benefit from pre-merge CI :/
09:51tzimmermann: danvet, can i merge "drm/arcpgu: Really delete file"
09:51airlied: mareko: yeah nit sure howngood the plans for doing mr workflows are yet, but we do want to investigate, esp for ci opportunites
10:00danvet: tzimmermann, sure, just didn't get around to that yet
10:00danvet: lynxeye, will you apply the 2 etnaviv fixes?
10:00tzimmermann: danvet, ok. what's the current -fixes branch. i have a patch for 5.11
10:01danvet: drm-misc-fixes should be open for business
10:01danvet: mlankhorst rolled it forward yesterday
10:03lynxeye: danvet: yep
10:04danvet: lynxeye, you'll fix the typo in the commit message of patch 2 while applying?
10:14lynxeye: danvet: sure
16:15haasn: as an independent contributor, having to submit to a mailing list is one of the main reasons for failure to contribute
16:16haasn: so I'm always hugely in favor of switching to merge requests for any project I'm *not* a maintainer of
16:16imirkin: haasn: i'd say the inverse is true -- having to sign up in some thing is the main reason to fail to contribute. sending email is easy though!
16:16haasn: I think a good middle ground there is having oauth/sso-type authentication
16:17haasn: like if you can cross-authenticate with your gitlab or github accounts, of which everybody has one
16:17imirkin: haasn: i'd still have to create a repository / tree / manage branch in some "other" system
16:17imirkin: sending email just requires the thing being on my comp where i'm developing it
16:17haasn: it probably comes down to what you're most used to, in the end
16:18LaserEyess: why is both not an option?
16:18haasn: the main benefit of gitlab et al. is that it centralizes metadata
16:18LaserEyess: it's git though, you don't lose anything if a commit comes from a MR vs an email
16:19haasn: so for example on email system there's just no clear analog to do something like search for open or closed MRs
16:19LaserEyess: I guess mailing list discussion?
16:19haasn: the best thing you have is something like an IMAP tag which is only for the maintainer
16:19haasn: i.e. not "centralized"
16:19vsyrjala: one thing i'm wondering about gitlab style mrs is what happens to the old version when you force push a new version?
16:19haasn: my main gripe with mailing lists is that it's always those projects that still use mailing lists that seem to consistently forget about my patches
16:20emersion: vsyrjala: GitLab allows you to navigate between the different versions
16:20LaserEyess: vsyrjala: it's just a separate git branch/repo so it does the same thing as a regular forcepush
16:20LaserEyess: oh, you mean the old revisions
16:20imirkin: haasn: just as easy to forget about merge requests in some UI :)
16:20haasn: vsyrjala: gitlab specifically keeps track of force pushes and lets you show diffs between versions as well
16:21haasn: has buttons like "compare with previous version"
16:21vsyrjala: well, i'd not want to use thatf terrible web ui for anything. i guess there would be a way to see it from real git somehow
16:22imirkin: fyi, you can usually add .patch to random URL's, and it tends to do what you'd expect
16:22LaserEyess: you don't have to use the webUI at all actually, if you subscribe to a MR thread you can get it all emailed to you
16:22LaserEyess: gitlab does that automatically for things you're tagged in
16:22haasn: oh yeah that is actually another advantage of webUIs in my mind, it's _easier_ to go from the URL to a .patch than from an open email, but feel free to blame my email client for that :)
16:22LaserEyess: it's html by default though
16:23imirkin: i gave up with on the gitlab notifications pretty quickly - just have them all disabled completely
16:23imirkin: if you want to reach me, use email ;)
16:24jenatali: I like 'em when they work, but I seem to randomly get unsubscribed from threads for no good reason
16:24LaserEyess: gitlab notifications are email...
16:25imirkin: anyways, i don't think anyone is changing anyone's mind here
16:25imirkin: just be aware that people prefer one or the other, and they have good reasons for that preference
16:25LaserEyess: I'm curious why it has to be "one or the other" though
16:26vsyrjala:still waiting for someone to make a gitlab command line tool that's usable
16:26imirkin: i'm sure there are people who like both
16:26LaserEyess: no I mean, why would a project have to choose
16:26imirkin: but the most outspoken people are in favor of one thing, and opposed to the other thing
16:26LaserEyess: what about gitlab eliminates the possibility of mailing lists and vice versa?
16:26imirkin: because things like gitlab lose a lot of their advantage when not everything goes through them
16:27imirkin: and so the gitlab-liking people bully the mailing-list liking people into using the stupid gitlab tool
16:27LaserEyess: like what
16:27LaserEyess: I guess CI, maybe? but what else
16:28LaserEyess: that image doesn't make sense to be because I still do not see how they are mutually exclusive
16:28imirkin: CI is probably the main listed reason
16:29imirkin: they're not. that's the point. except they are because people tend to prefer one or another. as it happens, i use emacs and vi. but most people are pretty religious about it.
16:30vsyrjala: patch review is the thing that troubles me most about gitlab. no way i'd be able to do that effectively using that crap web ui. and so far i've not seen any alternative ui for it
16:31vsyrjala: dealing with bugs is already hard enough using that ui
16:32imirkin: feels like most patches nowadays don't really get much review anyways, so it's ok
16:32strfllw: "move fast and break things", right?
16:33imirkin: probably a side-effect of being in gitlab and having a lot less visibility than in email
16:34vsyrjala: kernel patches get reviewed
16:34imirkin: they're not in gitlab though, i imagine
16:35vsyrjala: that pretty much agrees with my observation of gitlab review. ok for high level "yeah looks good" vs. "nah looks like crap" but no good for detailed review
16:36LaserEyess: gitlab can issue plaintext emails and you can subscribe to them for any patches that get sent in, and respond in plaintext as well
16:36vsyrjala: the high level thing would only work if a) you don't really care about quality, or b) everyone is super competent and can do good changes based on high level review
16:37LaserEyess: admittedly, I don't know how MRs look formatted as plaintext patches, but I'm sure git can read them at least
16:37imirkin: LaserEyess: can i reply to them with line-by-line feedback?
16:37LaserEyess: I think you guys are purposefully only discussing the specific issues with your workflows, which are vaild, but not the whole context
16:37emersion: no, MR diffs aren't sent you by email
16:37emersion: to you*
16:37emersion: you only get a "there's a new MR" notification, with the description
16:38LaserEyess: really? that sucks then
16:38emersion: also, you get one email per MR comment
16:59MrCooper: vsyrjala: https://gitlab.com/bichon-project/bichon was pointed out to you before
17:00MrCooper: as far as I'm concerned, patch review with GitLab is leaps and bounds ahead of e-mail, and keeps getting better
17:02MrCooper: e.g. the last version added a way to hide changes that one has reviewed by default, so one doesn't have to waste time wading through already reviewed changes next time
17:03MrCooper: the killer feature might be the direct integration and automatic cross references between MRs and issues though
17:04vsyrjala: haven't had time to look at this bichon yet. oh, it does say something about review, so might even be promising
17:06vsyrjala: also i hate things that hide code. just means i'd probably miss the fact the author did 10 random changes in addition to the stuff i requested
17:07imirkin: fwiw, i'm not opposed to online review tools. i've co-developed one, so i know what a good one can be like. gitlab ain't it.
17:07MrCooper: vsyrjala: it only hides changes you've seen (if you click that box), not later changes
17:08LaserEyess: he means in email only, I think
17:08LaserEyess: imirkin: what's something you consider a good online review tool, honest question
17:08LaserEyess: I like gitlab a lot, but I do feel like it has strange limitations sometimes
17:09vsyrjala: i would never click such a box. how else would i remember what all the changes in the patch were if i can't see them?
17:10imirkin: LaserEyess: the one i worked on had a moderately tight integration with the revision control system, making it essentially a no-op to "upload" changes (the system could pull them in itself). it was later open-sourced as "Rietveld", although that has a number of components that were redone to remove the internal dependencies. in later companies, i've used this tool, with some modifications to
17:10imirkin: reinstate some of the removed functionality.
17:18LaserEyess: imirkin: looks to be a dead-ish project that only claims to support a dead language
17:18LaserEyess: I watched some of guido's video at least, looked like a nice compromise between email style review and gitlab style review
17:19imirkin: note the dates of these things, might explain the reason ;)
17:19imirkin: but it was a tool which enabled very quick review, as well as detailed and iterative review
17:20imirkin: with (imho) excellent keyboard navigation
17:21emersion: imirkin: curious, which tool have you co-developer?
17:21imirkin: emersion: rietveld (well, the internal tool it's based on)
17:22LaserEyess: imirkin: have you heard of sourcehut? that's probably osmewhat close to what you'd want
17:22LaserEyess: it has first class email support
17:23imirkin: LaserEyess: i've seen the name before, but beyond it having something to do with source code, no clue what it is
17:23imirkin: thanks, will check it out
17:24LaserEyess: I think there's a longer blog that drew did on all of the major features but I forget the link to that
17:24LaserEyess: drew = the creator
17:24imirkin: ah ok
17:25ddevault: I also idle in this channel, hi
17:25ddevault: lmk if you have any questions or run into any problems
17:25LaserEyess: well there you go
17:54pcercuei: In the ingenic-drm driver, I discover the HDMI chip/driver using drm_of_find_panel_or_bridge(). The problem is that this will always return -EPROBE_DEFER if the HDMI driver is disabled in the config. So I can't create a minimal kernel that only supports the internal screen, since ingenic-drm won't probe...
17:54pcercuei: Is there a way around this?
17:55pcercuei: I can't really use IS_ENABLED(CONFIG_*) since that would be hardcoding board details in the driver...
17:55imirkin: msm had a similar issue with panel -- if you didn't build a panel into the kernel, then it failed
17:56imirkin: robclark: did you solve that issue somehow? --^
17:56lynxeye: pcercuei: Nope, state of the art is to also remove the phandle to the bridge from teh DT if you want a minimal system
17:57robclark: I don't believe you need to build panel into kernel.. although the drm device won't probe until you get the module loaded
17:57imirkin: i didn't mean like "builtin", but just built
17:57imirkin: if there was no panel at all, it failed to load
17:58pcercuei: lynxeye: But DT isn't for configuration...
18:00robclark: imirkin: right, if you don't build the panel at all, then it won't probe.. it will wait forever trying to collect all the jigsaw pieces
18:00imirkin: ok. so it sounds like DT *is* for configuration ;)
18:00robclark: otoh, that isn't really something that doesn't happen all over the place
18:01imirkin: i wasn't suggesting you did something wrong, was just remembering a similar issue and wondering if it had been solved somehow
18:03lynxeye: pcercuei: As you can't hotplug a bridge there is no other way than wait for all the pieces to show up. If you don't want that, you need to tell the kernel your intention of not using that bridge. DT is the way to do that. Maybe not in a static DT file, but your firmware/bootloader can just patch up the DT passed to the kernel to remove the link if you don't want to use it.
18:05pcercuei: I don't think Rob Herring would agree with that
18:05pcercuei: and I don't agree with that either
18:07lynxeye: DT is the interface between firmware and kernel. U-Boot and other are patching up the DT for various reasons.
18:08danvet: pcercuei, essentially your case isn't supported
18:08danvet: it's a bit like building a kernel with your driver outright missing
18:08danvet: and then complaining you don't have the gpu working
18:08danvet: or maybe, your driver with parts of it not built
18:08danvet: and yes the fix is DT
18:08danvet: essentially disable that device
18:09mareko: airlied: if we moved to Gitlab for amdgpu, one of the interesting features is that we can hopefully delete accidentally released confidential information before it propagates out
18:10danvet: mareko, one thing from the wider kernel community is pushing stuff to lore.kernel.org
18:10pcercuei: Again, disagreed. You should be able to disable stuff you don't need (but that are supported by your board) in the kernel config
18:11danvet: I have honestly not looked into that, my idea was to first hack around on CI
18:11danvet: before looking into MR workflow
18:11danvet: since lack of CI for the combinatorial nonsense of options we have is already hurting us right now
18:11danvet: and fixing that would be a great selling point imo for adopting MR
18:12robher: danvet, lynxeye, pcercuei: The problem is the kernel IMO. It's a common problem. There are some partial solutions to giving up deferred probe, but they are opt-in (by subsys typically).
18:13anholt: pcercuei: in the DT world, if your DT describes there being a panel but you don't have a panel connected, then your DT is broken and that needs fixing by whatever precedes linux. This means that autodetection of user-configurable panels is basically not an option. this sucks.
18:13danvet: robher, well I think "there's partial solutions" is a pretty good summary of the DT world :-/
18:13danvet: another eternally unsolved one is that eprobe_defer is slow
18:14danvet: or at least it's still coming up as justification for people hand-rolling hacks and backwards driver architecture
18:14pcercuei: anholt: true, but in my case, there is a panel connected, there is a HDMI chip that you could use, but you only really need to output on the panel, so the HDMI driver should be disabled in the config
18:15robher: danvet: It's partial because there's no way to say "I've loaded all the drivers I know about" unless modules are disabled. That has nothing to do with DT.
18:15pcercuei: so the DT describes what's there, but that doesn't mean the kernel should use everything
18:16anholt: pcercuei: at the point where you're building a custom kernel because you want less features on your existing hardware, I'd say you should just go edit the C code, too.
18:16robher: or even knows about everything. old kernel with newer DT for example
18:17pcercuei: anholt: yes, and commit it into a nice patch, and upstream it ;)
18:18robclark: I think folks who actually want to use their panels might nak that ;-)
18:22robher: danvet: device links for the most part eliminates deferred probes. That's been there for a while now and switched on by default in 5.12 (though might be switched back off).
18:23pcercuei: We need a function that can answer "does remote device at <port> have a driver?"
18:24robher: pcercuei: with modules enabled, the answer is either yes or maybe later.
18:31robher: pcercuei: with modules disabled (or a timeout) that function is driver_deferred_probe_check_state(). Again, it's opt-in by subsystem and DRM does not.
18:37pcercuei: I guess I can use it in my driver directly, no?
18:38robher: pcercuei: If you control returning EPROBE_DEFER, yes.
18:38pcercuei: if (err == -EPROBE_DEFER) err = driver_deferred_probe_check_state(dev);
18:38pcercuei: not the cleanest, but that would work
18:38pcercuei: Thanks for the info, I didn't know about this function
18:49Venemo: jekstrand: what is the proper way to deal with nir_metadata_not_properly_reset?
18:51Venemo: jekstrand: I can see that the NIR_PASS macro sets that flag. I'm trying to use dominance info in a pass, and I bumped into an assertion because of it
19:01Venemo: jekstrand: specifically, I'm trying to use nir_unsigned_upper_bound in nir_opt_constant_folding and that needs the dominance info
19:11karolherbst: curro: ehh.. somehow I messed up the llvm-12 fix: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9372
19:25curro: karolherbst: oops, fix is R-b me
19:33jekstrand: Venemo: You need to call nir_metadata_preserve()
19:41Venemo: jekstrand: it already does, doesn't it
19:41Venemo: jekstrand: or do you mean I should call it at the beginning of the pass?
19:49jekstrand: Venemo: YOu have to call it before hte pass exits
19:49jekstrand: Venemo: which pass is this?
19:52Venemo: jekstrand: specifically, I'm trying to use nir_unsigned_upper_bound in nir_opt_constant_folding and that needs the dominance info
19:55Venemo: as far as I can see, nir_opt_constant folding uses a helper which calls nir_metadata_preserve at the end
19:55jekstrand: it does
19:56Venemo: so you're telling me I'd need to move that call to before it starts iterating over the instructions?
19:56jekstrand: Uh... no.
19:56jekstrand: Venemo: Can you paste your patche?
19:56imirkin: anholt: not sure if you saw, but it looks like one of your changes broke PBO for (some of) nouveau: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4380 -- any ideas? this is pretty far out of my knowledge zone
19:57anholt: I had seen. any chance I could see the NIR just before conversion with NIR_TO_TGSI_DEBUG=1?
19:57imirkin: (gimme a few)
19:58anholt: saw it this morning and started thinking about it, then ended up in piglit runner land
19:58Venemo: jekstrand: I'm on my phone right now, so don't have the exact code, but I'm working on this thing: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9201/diffs?commit_id=323c5430afb9c5fdfee530ce13d3f4e5a9843ee4
19:59Venemo: jekstrand: in my local version, it calls the real unsigned_upper_bound and not the hack that you can see in the link
19:59jekstrand:is very confused
19:59imirkin: anholt: added into the issue. i guess you're missing the wrmask on the store_output?
19:59Venemo: what about? sorry if I'm vague, jekstrand
20:00jekstrand: Venemo: I don't see how adding a call to nir_max_unsigned_upper_bound() would cause a metadata failure
20:01Venemo: jekstrand: that's what I'm saying. I deleted nir_max_unsigned_upper_bound and use the normal nir_unsigned_upper_bound now. That goes after phis, and uses dominance info
20:02Venemo: The nir_max_unsigned_upper_bound was just a hack that I added there
20:02jekstrand: Are you calling nir_metadata_require(impl, nit_metadata_dominance) anywhere?
20:04Venemo: jekstrand: nir_unsigned_upper_bound calls nir_block_dominates here: https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/compiler/nir/nir_range_analysis.c#L1279
20:05Venemo: jekstrand: and that hits this assertion here: https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/compiler/nir/nir_dominance.c#L255
20:05anholt: imirkin: thanks, really helpful. we're clearly losing our writemask, but how?
20:07imirkin: anholt: you should be able to repro on softpipe/etc if you disable VS layer export (but leave GS support in)
20:08jekstrand: Venemo: Right. THen you need to call nir_metadata_require(impl, nit_metadata_dominance) somewhere
20:08jekstrand: Venemo: You should be able to call it anywhere and calling it multiple times is mostly harmless.
20:08imirkin: [since the pbo shader will export directly to the layer from VS if that's allowed]
20:10Venemo: jekstrand: currently it is called here: https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/compiler/nir/nir_builder.h#L116 - are you saying I should call it at the beginning of nir_opt_constant_folding too?
20:10jekstrand: Venemo: "require" not "preserve"
20:10jekstrand: Venemo: nir_metadata_preserve marks everything not preserved as being gone.
20:11jekstrand: require checks if the metadata is available and, if it isn't, goes off and runs the analysis pass.
20:11Venemo: isn't that a bit too heavy handed here?
20:12Venemo: jekstrand: would it be OK to call it only once, before my 1st call to nir_unsigned_upper_bound?
20:13anholt: imirkin: MR up shortly
20:13imirkin: anholt: awesome, thanks!
20:13imirkin: happy to test
20:16anholt: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9376 I think is it
20:18jekstrand: Venemo: You can call it as many times as you want and it will only run once.
20:21imirkin: anholt: thanks for the quick fix
20:21Venemo: jekstrand: I guess that's because nothing in nir_opt_algebraic actually harms this meta data?
20:25Venemo: Okay, thanks jekstrand :)
20:39Venemo: jekstrand: btw how would feel about adding the nir_unsigned_upper_bound_config to the nir_shader?
20:41jekstrand: I don't know what that is. :-/
20:43Venemo: it has some information about the shader's execution, eg. the max number of invocations, upper bounds for vertex attributes and such, which is used by nir_unsigned_upper_bound to give an upper bound to various intrinsics
20:44Venemo: I'm a little surprised to see that we are its only users
20:44jekstrand: There's some of that stuff in shader_info
20:45Venemo: https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/compiler/nir/nir.h#L5163 this is what I'm talking about
20:48Venemo: it doesn't seem redundant with shader_info, but I guess it could be moved to shader_info
20:52jekstrand: Where does the information come from?
20:53Venemo: the backend sets it
20:55Venemo: there could also be some sensible defaults based on common sense and API limits
20:55ajax: the "cb" in st_cb_*.c means... callback? probably?
20:56imirkin: basically the callbacks called by the core mesa/main code
20:56imirkin: st_atom_* are the state object generators (which are fed into a cso cache thing)
21:24fmlatghor: Is Linux Device Drivers Third Edition completely irrelevant nowadays if I want to learn to write device drivers/kernel modules for linux?
21:25pmoreau: jekstrand: As far as I can tell, `vtn_handle_execution_mode()` does not check which entry point it applies to, or am I missing something? It is an issue when there are more than one `OpExecutionMode` in a SPIR-V module (due to applying to different entry points found in that module), as the current code will apply all modes onto all entry points.
21:33jekstrand: pmoreau: I thought I handled that case....
21:34jekstrand: pmoreau: Yeah, it's only run on the one entrypoint.
21:34pmoreau: I could very well be missing something in the code, since I only quickly looked at it.
21:34jekstrand: Look at the two callers
21:38pmoreau: So I see where `vtn_handle_execution_mode` is called from `spirv_to_nir` and it is only for a single entry point, but I don’t see how it avoids applying both `OpExecutionMode %foo LocalSize 1 2 3` and `OpExecutionMode %bar LocalSize 4 5 6` unless some previous pass removes the execution modes that do not affect the current entry point.
21:40jekstrand: Execution modes, when first parsed, are handled as decorations. We just stuff them in a linked list and deal with them later.
21:40jekstrand: Then, when we get to the end, we handle all the ones that are are on the entrypoint you specified.
21:43pmoreau: I must be completely blind, but where is that check performed that you only handle the ones for the entry point that was specified? There is a `vtn_assert(b->entry_point == entry_point)` but that just checks that the entry point given as argument matches the one stored in the builder. I never see a check for the entry point stored in the ExecutionMode decoration.
21:46jekstrand: Each vtn_value has a singly linked list of decorations. When we handle an OpDecorate*, we just stuff an item on the list. We then parse them with vtn_foreach_decoration.
21:46jekstrand: Execution modes work the same way. Exactly the same way.
21:46jekstrand: We treat them as decorations with scope = VTN_DEC_EXECUTION_MODE
21:54pmoreau: Ah right, I had missed that each value had its own list of decorations, so each entry point has its own list of decorations/execution modes.
21:55pmoreau: Thank you for the help!
23:12airlied:gets a headache trying to do shader ballot and llvmpipe fragment shaders where only 1 pixel is being rendererd
23:13airlied: have to figure out if subgroup_invocation numbering should just be 0 for the single pixel
23:30alyssa: jekstrand: now in a shader-db run for Bifrost, we spend only 13% of the time in the backend compiler (this includes the backend NIR opt loop. the other 87% accounts for opts run by st/mesa as well as GLSL)
23:30alyssa: and of that 13%, we spend only 3% in register allocation
23:30alyssa: I think I'm satisfied :~)
23:30alyssa: 17% of the time is nir_opt_algebraic
23:31alyssa: (72% of the backend time is NIR passes)
23:31alyssa: somehow DCE is my most expensive backend pass, so I think I screwed up the dataflow analysis :p
23:33alyssa: an annoying amount of time is spent in GLSL, actually
23:34imirkin: alyssa: just load up shadertoy. RA time should increase nicely.
23:34alyssa: imirkin: heh. yes.
23:55daniels: vsyrjala: ooi, have you tried any of the CLI clients I've pointed you to recently?
23:56vsyrjala: not yet. just had a quick look at bichon readme and apparently it tries to do something with review. so sounds somewhat promising
23:58vsyrjala: also i don't have go installed on my laptop so need to jump through that extra hoop to try it out
23:59vsyrjala: dunno why all of these are written in go