05:54 igorkovalenko: airlied: I do not see 20.2.2 milestone in gitlab tracker, do you use milestones to mark issues at all? just wanted to be sure bug #3613 will be fixed in 20.2.2
06:14 igorkovalenko: asked to include the fix in cherry-picks for 20.2 https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7282
06:14 gitbot: Mesa issue (Merge request) 7282 in mesa "20.2 cherry picks" [Opened]
06:26 igorkovalenko: fwif I was able to reproduce the same firefox webrender text issue with r600 driver on current mesa master, issue is not reproduced with llvmpipe, first screenshot in the bug report here https://bugzilla.mozilla.org/show_bug.cgi?id=1673939
06:27 igorkovalenko: it is very annoing since it does not allow to read useful long text pages in other bug reports and kernel changelogs
06:28 igorkovalenko: what else can I try to pin this issue?
08:20 pq: airlied, HDR knowledge? I'm still designing protocol, I've never even tried to enable a HDR mode on hardware yet. No idea what drivers support what, all I see is the existing UAPI in headers.
10:16 mlankhorst_: danvet: drivers/gpu/drm/nouveau/nouveau_ttm.c:320:19: error: implicit declaration of function ‘swiotlb_nr_tbl’ [-Werror=implicit-function-declaration]
10:17 mlankhorst_: missing include/swiotlb.h ?
10:18 danvet: mlankhorst_, is that me or which tree?
10:18 mlankhorst_: not sure, drm-tip at least
10:19 mlankhorst_: oh seems to be ckoenig who broke it. drm/ttm: wire up the new pool as default one v2
10:24 danvet: merge problem?
10:24 danvet: drm-misc-next seems to compile fine here
10:26 mlankhorst_: ah
13:57 vsyrjala: integer overflow looking for review https://patchwork.freedesktop.org/patch/396379/?series=82962&rev=1
15:52 jenatali: jekstrand: I think you pushed the wrong branch to !6332 :)
15:54 jekstrand: jenatali: Thanks. Fixed. :)
15:55 jekstrand:
15:55 jekstrand:
15:55 jekstrand:
15:55 jekstrand: 21:09 <cmarcelo> just an initial thought as I start to look at the patch: /me slightly "nervous" about multiple modes... worried about things like "we
15:55 jekstrand: need to lower mode X (uses ==)" -> "this is mode X|Y, so ignored" -> "but we need to circle back once we discover is X"...
15:55 jekstrand: Day changed to 30 Oct 2020
15:55 jekstrand: 08:39 <jekstrand> Yeah, that's nervewracking
15:55 jekstrand: 08:39 <jekstrand> But ultimately necessary, I think.
15:55 jekstrand: 08:39 <jekstrand> Fortunately, for uniforms, system values, and shader I/O, we assert in nir_validate that they only have one mode.
15:55 jekstrand: 08:48 <cmarcelo> I'm pondering if well named functions would alleviate the problem by making things more explicit. bit ops are explicit BUT I feel is
15:55 jekstrand: one of the things easy to overlook.
15:55 jekstrand: 08:49 <jekstrand> Yeah, maybe. I'm open to suggestions.
15:55 jekstrand: 08:51 <jekstrand> deref_may_have_mode(deref, mode)?
15:56 jekstrand: 08:51 <jekstrand> deref_has_mode(deref, mode)?
15:56 jekstrand: 08:51 <jekstrand> deref_mode_contains(deref, modes)?
15:56 jekstrand: 08:54 <cmarcelo> deref_mode_is_only, deref_mode_is_not, deref_mode_contains, deref_mode_not_contains
15:56 cmarcelo: maybe instead of "is" deref_mode_equals, deref_mode_not_equals (to be more symmetric with contains)
15:56 jekstrand: I don't think we need not_contains. That's redundent with !contains
15:56 cmarcelo: right
15:57 cmarcelo: same could be said for is_not I guess
15:59 jekstrand: In my patch, I called out three cases: 1. equal/not_equal 2. may_be, and 3. is
16:04 cmarcelo: 3. deref->modes & ~nir_var_foo if we want to skip everything that isn't
16:04 cmarcelo: guaranteed to be a nir_var_foo.
16:05 jekstrand: !(deref->modes & ~nir_var_foo) is deref_mode_is
16:05 jekstrand: As in "it's guaranteed to be one of these modes."
16:05 jekstrand: whereas deref->modes & nir_var_foo is deref_mode_may_be
16:06 cmarcelo: right
16:06 jekstrand: I think my inclination is to have nir_deref_mode_equals do a == check and also assert(deref_mode_is(deref, foo))
16:07 jekstrand: That way it blows up if you ever use it in a scenario where multiple modes may leak in.
16:07 cmarcelo: need a zero check for deref_mode_is also?
16:07 jekstrand: Modes aren't allowed to be zero
16:07 jekstrand: But, yeah, everything can assert != 0
16:07 cmarcelo: zero assert then
16:08 jenatali: What's to stop people from just adding new potentially confusing/incorrect equality/bitfield checks without using the helpers? Just code review?
16:08 jekstrand: jenatali: Yeah
16:08 cmarcelo: jenatali: kind of. if I'm basing my pass on another pass that uses those, I'll just use them.
16:08 jekstrand: jenatali: But if I go through and rewrite everything to the helpers (which I'll do) then people will copy+paste the right thing. :-)
16:09 cmarcelo: (I think this scenario of "let's look elsewhere to see how it is done" is quite common)
16:09 jenatali: Heh, fair
16:09 jekstrand: So I think my new plan is to replace my "make a bitfield" patch with two patches: 1. Add and use the helpers everywhere. 2. switch to a bitfield.
16:09 jenatali: I guess I'm more thinking of someone who's already familiar with nir, but hopefully the rename of "mode" to "modes" is enough to make them go look for examples elsewhere
16:10 jekstrand: That way it's easy to review that we didn't miss anything because it won't build if there's a missing helper.
16:10 jekstrand: cmarcelo: Are you good with those helper names? _equals, _may_be, and _is?
16:11 jekstrand: cmarcelo: I'd like to make sure we're ok before I start typing like mad. :-)
16:11 jenatali: jekstrand: I don't quite follow the difference between _equals and _is
16:11 cmarcelo: jekstrand: trying to understand if we really need equals and _is
16:11 cmarcelo: besides "more efficient in release mode"
16:11 jekstrand: jenatali: Equals is the lazy one. If you don't want to think about "what happens if this is combined with other modes?" then you use _equals.
16:11 jekstrand: It'll assert on you if you got it wrong.
16:12 cmarcelo: jekstrand: ok. but I think the names could reflect that better
16:12 jenatali: Yeah, "equals" and "is" are just synonyms to me
16:12 jekstrand: Yeah.....
16:12 jekstrand: Maybe _is, _must_be, and _may_be?
16:13 jekstrand: I don't like "must"
16:13 jekstrand: Though it does have the right connotation, I guess.
16:13 jekstrand: Naming is hard. :-(
16:13 jekstrand: I do like the idea of _is being the lazy one.
16:13 jenatali: _all_of, _one_of, _none_of ?
16:14 jenatali: No, that's not right, I'm missing a concept
16:14 jekstrand: The concepts we want for the complex cases are, I think, "may be" and "known to be"
16:15 cmarcelo: _is and _is_subset?
16:15 cmarcelo: _is and _is_in_mask?
16:16 jekstrand: How is _is_in_mask better than bitops?
16:16 cmarcelo: it is "positive"... the bitops need you to do !(... & ~...) right?
16:17 jekstrand: I guess, yeah.
16:18 cmarcelo: jekstrand: should _is assert on having a single mode as argument? or we have exact match for multiples?
16:18 jekstrand: cmarcelo: Yeah, it'd assert that it's only one mode
16:19 jenatali: Hm... what about _is, _is_one_of, _may_be, _may_be_one_of ?
16:21 cmarcelo: I think having the lazy being _is is good (that discards _is_one_of)... then _may_be, ok. _may_be_one_of seems OK, but what about _may_be_only?
16:21 cmarcelo: _is _may_be _may_be_only
16:21 jekstrand: may_only_be
16:21 jekstrand: ?
16:21 jekstrand: That's the same as must_be
16:21 jekstrand: I guess must is ok
16:23 cmarcelo: sounds ok to me. we could always live with just _may_be and _must_be, but _is will be more common so shorter, and perform extra protection for the "just one mode".
16:23 jekstrand: Yeah
16:23 jekstrand: I'm going to document them like crazy too. :-)
16:24 cmarcelo: jekstrand: the whole helper thing breaks down a bit with switches
16:24 jenatali: My thinking was that _is* indicates the deref is single-mode (and should be asserted), where _may_be* means the deref could potentially be multiple
16:24 cmarcelo: but it might be ok.
16:25 jenatali: Not sure if it matters, just seemed like it might make code easier to read or reason about
16:27 cmarcelo: jenatali: I see your point now.
16:30 cmarcelo: _is (just single) _is_one_of (must be a single that match one of the args, short hand for _is || _is || _is ...) _may_be (does the mode overlap with the arguments?) _must_be (is the mode a subset of the arguments?)
16:31 jenatali: That looks good to me
16:33 cmarcelo: I'd also expect most passes to stick with _is, _is_one_of or switch statements on the mode. (but I havent seen all the generic ptrs patches yet)
16:34 jenatali: Agreed, it's mainly just lower_explicit_io and a few optimization passes that need to deal with generic derefs
16:36 jekstrand: Sadly, a lot more than nir_lower_explicit_io has to deal with them. :-/
16:36 jekstrand: Any optimization on function_temp, for instance.
16:37 jenatali: Ah... yeah, true
16:37 cmarcelo: perhaps _is could do what _is_one_of does... _is (the mode must be single and *one of* the args).
16:38 cmarcelo: but that might be confusing
16:38 jenatali: I'd expect _is to be an equality check from the name
16:38 cmarcelo: _is_one_of reads better when looking at random code
16:38 cmarcelo: jenatali: yeah
16:42 jekstrand: cmarcelo, jenatali: https://paste.centos.org/view/21ee3dd2
16:45 jenatali: jekstrand: Yeah, looks good to me
16:51 cmarcelo: jekstrand: I'm a bit confused about line 13 asserts
16:51 cmarcelo: jekstrand: in line 30, I think you also want to check (not assert) util_bitcount() == 1
17:06 jekstrand: cmarcelo: The line 13 assert could be written as "if (deref->modes & mode) assert(util_bitcount(deref->modes) == 1);"
17:06 jekstrand: except I messed up the bitcount. :)
17:06 jekstrand: I'll write it as an if. That's more clear and the compiler should delete it in release builds.
17:06 cmarcelo: do we want that assert though?
17:06 jekstrand: Yes.
17:07 jekstrand: If you're using one of the lazy helpers, we want to blow up if they ever see a generic pointer they might interact with.
17:07 jekstrand: If they silently fail, someone's going to get it wrong and not notice.
17:07 cmarcelo: OK. Perhaps add a note to that fact near the assert.
17:08 cmarcelo: so if there's a chance a pass interacts with generic pointers, the guidance is to use may/must helpers then? (perhaps make this guidance even more explicit)
17:09 jekstrand: Yes
17:09 jekstrand: If it interacts with generic pointers, you have to use may/must and actually think about what you're doing.
17:09 jekstrand: If it's a driver-internal pass for a non-CL driver or if it's a pass that works on system values or I/O, then use the simple ones.
17:35 igorkovalenko: hi I have apitrace which shows issue with r600 but not with llvm pipe, should I file a bug report and attach apitrace there? it's 300kb bzip2 compressed or 530kb uncompressed
17:58 bnieuwenhuizen: dcbaker: I think there was a stable release for this wednesday but I didn't see any email for it yet. Any issues I can help with?
18:00 EdB: airlied: nice that you worked on nir printf
18:27 jekstrand: cmarcelo: I'm really liking the new helpers. :D
18:40 jekstrand: Way more expressive
18:43 anholt: igorkovalenko: that's plenty small, go for it.
18:45 igorkovalenko: filed bug #3720
18:47 igorkovalenko: this one https://gitlab.freedesktop.org/mesa/mesa/-/issues/3720
18:47 gitbot: Mesa issue 3720 in mesa "Firefox with WebRender enabled produces garbled text display towards end of long page with r600 driver" [Opened]
18:50 igorkovalenko: as this is ancient Radeon HD 4290 IGP how long it is usually triaged if at all? I'm willing to dig deeper but not sure where to start as usual
19:06 cmarcelo: jekstrand: :-)
19:29 DPA: Could someone move xorg/xserver issue 1083 to mesa for me please? I seam to lack permissions to do so.
19:29 iive: igorkovalenko, i just tried to replay your apitrace, it seems to render correctly on my Radeon HD5670. It's distro provided mesa 20.2.1
19:30 ajax: DPA: done, mesa/mesa#3721
19:30 iive: igorkovalenko, could we talk in #radeon, this is more developer channel. I'm "advanced" user
19:30 DPA: ajax: Thanks!
19:30 igorkovalenko: iive: thanks, I'm suspecting an issue with HD4xxx and maybe HD3xxx similar to the one reported on mozilla bugzilla 8 years back
19:33 dcbaker: bnieuwenhuizen: I just need to make it. Just been really busy working on cargo + meson integration and losing track of time :/
19:33 dcbaker: working on it now
19:37 dcbaker: and I forgot to push the staging branch on Tuesday...
19:37 dcbaker: sigh
20:12 jekstrand: jenatali, cmarcelo: Updated with the new helpers. It's so much better now. Thanks!
20:12 jenatali: \o/
20:13 jekstrand: There are still a few places that & and | manually but it's very few.
20:13 jekstrand: Most stuff calls a nice descriptive function.
20:13 jekstrand: And I have a lot more confidence in all the places where I put deref_mode_is because the asserts will tell us if we got it wrong.
20:16 LiquidAcid: question for the amdgpu people, i have an usb-c port on this device, which also works as DP output. shouldn't this port show up in /sys/class/typec?
20:25 jenatali: jekstrand: Yeah, those helpers look great, much easier to read/understand
20:25 jenatali: Even before adding in generic pointers
20:31 jekstrand: Yeah
20:31 jekstrand: I really like is_one_of
20:38 jekstrand: Why did we enable -Wimplicit-fallthrough in CI? xxhash fills the entire log with warnings and I can't see why the compile is failing.
20:40 FLHerne: jekstrand: tarceri fixed them all when he added it :p https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5705/commits
20:40 gitbot: Mesa issue (Merge request) 5705 in mesa "Turn on Wimplicit-fallthrough project wide" [Anv, Glx, Nir, Spir-V, Mesa, Nouveau, R100, R300, Radeonsi, Svga, Merged]
20:40 anholt: jekstrand: do we have a bad compiler in ci that's not processing the fallthrough comments?
20:40 jekstrand: FLHerne: Aparently not. :P
20:40 jekstrand: anholt: It looks like that may be the case.
20:40 jekstrand: anholt: Where that bad compiler is called "clang" :-P
20:44 FLHerne: https://bugs.llvm.org/show_bug.cgi?id=43465#c37
20:44 FLHerne: (it was implemented, then reverted because they decided it was a bad idea)
20:46 jekstrand: :-/
20:49 FLHerne: The kernel devs have a script to automatically replace such comments with the attribute: https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe@perches.com/
20:50 FLHerne: ...but I bet MSVC blows up
20:51 jekstrand: We'd want to make it a #define likely so we could handle MSVC and older compilers.
20:51 cmarcelo: could we use the same solution as the kernel.
20:51 cmarcelo: ?
20:52 jekstrand: probably
20:52 cmarcelo: a macro "fallthrough" that does the attribute thing (which seems GCC also recognizes, not sure since what version though)
20:52 cmarcelo: a small upside is no more looking up the right spelling of fallthrough since code will fall to compile if you get wrong (-:
20:53 jekstrand: I'd love it if "fallthrough" was a statement like break or continue.
21:01 imirkin: jekstrand: i thought i saw something in the kernel that made it like that
21:01 imirkin: probably a clever define, not sure
21:02 imirkin: but it's something that plays with the static analysis tools
21:13 swick: imirkin: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html -Wimplicit-fallthrough
21:13 imirkin: ah right. so probably a #define fallthrough __attribute__ ((fallthrough));
21:13 imirkin: and if you name your variable "fallthrough" ... don't do that.
21:14 swick: or just comment /* fallthrough */
21:14 imirkin: jekstrand was asking about a "break" style thing.
21:14 imirkin: i.e. an actual statement
21:14 FLHerne: swick: Clang doesn't understand such comments, that's where this discussion started
21:14 swick: oh
21:15 jekstrand: Yeah, we could define it to "__attribute__ ((fallthrough))" if you have that and "do { } while (0)" otherwise.
21:15 jekstrand: And then I have to figure out how to get vim to syntax highlight it. :D
21:28 ajax: airlied: have we tried gles3.2 for llvmpipe?
21:29 imirkin: should be minor, if there's even any (backend-visible) difference...
21:30 ajax: i don't imagine it's a lot of work if you have 4.6, no. just playing pokemon here
21:33 imirkin: optimizations could be possible for llvmpipe with OES_primitive_bounding_box but it's completely optional
21:41 jekstrand: ajax: Point/line clipping are different
21:41 jekstrand: ajax: Also, the GLES CTS is a *lot* more thorough. I wouldn't be surprised it it doesn't find a bunch more bugs.
21:41 jekstrand: Though, likely, those are found by the Vulkan CTS too.
21:43 imirkin: oh right yeah -- point clipping is different. but iirc llvmpipe implements both. just have to be sure to select the right one.
21:45 jekstrand: Yup
21:46 imirkin: also agreed on the thoroughness. but the GL CTS also finds bugs that the GLES CTS doesn't. so they're different.
21:46 imirkin: GLES CTS is better at testing little things. GL CTS is better at finding issues which are entirely unrelated to the features being tested.
21:47 jekstrand: lol
21:47 imirkin: like some point rasterization test will be broken because there's something weird about how it uploads uniforms. that sort of thing.
22:08 airlied: ajax: gles3.2 needs a bunch of gles2.0 fixes
22:09 airlied: so I think whatever fails are in CI is pretty much it (lines, points)
22:10 airlied: I've done some complete gles3.2 cts runs and I think it was mostly just that, should probably get it back on my radr
22:10 airlied: vulkan lines are enough different to make it all a mess