07:36hakzsam: FYI, I'm going to stress the vkcts-navi21-valve job this week because a regression has been introduced last week and it randomly hangs now.
07:38javierm: tzimmermann: hi, when you have some time could you please take a look to https://lists.freedesktop.org/archives/dri-devel/2023-April/399428.html ?
07:38javierm: tzimmermann: I tried to fix with https://lists.freedesktop.org/archives/dri-devel/2023-April/399963.html but the reporter said that didn't fix it
07:39javierm: tzimmermann: so then proposed https://lists.freedesktop.org/archives/dri-devel/2023-April/400088.html and that works for him. But don't know what info from FW can be trusted and what not
07:39tzimmermann: javierm, hi. i'm just back from vacation. could take a bit
07:41javierm: tzimmermann: yeah I figured. I'm summarizing the threads since there were many mails there
07:41eric_engestrom: hakzsam: thanks! fyi I've hit this in 3 out of 3 runs of `vkcts-navi21-valve 3/3` (https://gitlab.freedesktop.org/mesa/mesa/-/jobs/40079004), while the other 2 worked on the first try; perhaps that can help narrow it down
07:42hakzsam: yeah, we are working on
07:50tzimmermann: javierm, looking at it
07:52javierm: tzimmermann: thanks, no rush
08:02tzimmermann: javierm, thanks your looking at this bug report. i think we should try that fixed version you posted
08:07tzimmermann: err, it was posted by pierre
08:10javierm: tzimmermann: the max(max(max... ?
08:10javierm: that's horrible IMO :)
08:11tzimmermann: it is :) but it fixes the problem and is clear once you got it
08:11tzimmermann: let me type a reply
08:11javierm: tzimmermann: but if you think that's the correct version, I won't going to argue since you looked at this issue more than me
08:16tzimmermann: javierm, it's been a while
08:19javierm: tzimmermann: I think the question is what can be trusted and what can't
08:19tzimmermann: the proposed fix is closer to the original code. IIRC i discarded lfb_depth because it didn't work with cases of 15-bit rgb. this cases used bpp=16 and one of the tests failed. adding it back in this max3() statement should still work
08:20javierm: tzimmermann: yeah, you explained why lfb_depth can't be trusted and it seems there are BIOS that don't report some channels (i.e: for xRGB the filler bits)
08:20javierm: but Pierre patch is still assuming that either of those can be trusted
08:21javierm: while my patch assumes that lfb_linelength and lfb_width can be trusted (we are relying on those anyways)
08:22javierm: tzimmermann: if we go with Pierre's patch, then I think that we should also apply my v1 (recalculate lfb_linelength if BPP is calculated)
08:22javierm: in other words, we should either trust lfb_linelength or recalculate it
08:22javierm: if we trust it, then can be used to calculate the BPP or if we don't, we shouldn't use it as provided
08:24tzimmermann: javierm, IDK which information is trustworthy. i'll try to send pierre's patch through a number of systems
08:24tzimmermann: javierm, i think your patch failed at the test at : https://elixir.bootlin.com/linux/v6.3-rc1/source/drivers/firmware/sysfb_simplefb.c#L73
08:24javierm: tzimmermann: can you also do it with my patch? Because otherwise doesn't make sense to me
08:24javierm: with my v1 I mean
08:25tzimmermann: whihc one is v1?
08:25javierm: tzimmermann: https://lists.freedesktop.org/archives/dri-devel/2023-April/399963.html
08:26tzimmermann: javierm, yeah. i assume that your patch still fails at https://elixir.bootlin.com/linux/v6.3-rc1/source/drivers/firmware/sysfb_simplefb.c#L73 .
08:26javierm: the commit message isn't accurate because wasn't the cause after all, but we are calculating BPP and then blindly use lfb_linelength to create the I/O resource
08:26javierm: tzimmermann: yeah, patch v2 worked for Pierre though
08:27javierm: because BPP is calculated from lfb_linelength and lfb_width
08:27tzimmermann: and then it still picks and incorrect format for the sysfb framebuffer. hence, there's a sysfb but with the wrong colroformet
08:28javierm: tzimmermann: correct, but the stride matches the (wrong color format BPP) at least
08:28javierm: that's why I think we need both to prevent the calculated BPP and reported stride to not match
08:29javierm: tzimmermann: let me answer in the thread
08:29tzimmermann: javierm, i cannot follow 100%, but the stride has no effect on the internal pixel format: if you need xrgb8888, but select rgb888, each pixel will still look wrong. stride only affects the overall line length
08:30javierm: tzimmermann: I know but if you pick a wrong color format (i.e: rgb8888) then the line lenght will be bigger than format * resolution
08:31javierm: it should match, regardless if was picked correctly or not (of course ideally correct but the selected pixel format and stride should match)
08:40MrCooper: karolherbst: piglit/bin/cl-api-enqueue-fill-image fails an assertion in radeonsi because depth == 0
08:59javierm: tzimmermann: answered in the thread, maybe I'm missing something silly but don't understand how we can't trust lfb_depth and then happily use lfb_linelength rather than a stride using the calculated BPP
09:02tzimmermann: javierm, stride is an arbitrary value. it is only vaguely connected to the bpp.
09:07javierm: tzimmermann: is used to calculate the size of I/O resource memory though: https://elixir.bootlin.com/linux/latest/source/drivers/firmware/sysfb_simplefb.c#L93
09:08tzimmermann: because we trust it ;)
09:09javierm: tzimmermann: ah, I see. Is not that we don't trust the lfb_depth is just that is wrong to assume that's the BPP ?
09:10javierm: so is only a problem of format selection
09:12javierm: IOW, always lfb_depth == (lfb_linelength * 8 / lfb_width) but the error is assuming that lfb_depth == bpp and that's why you used the color bits to calculate it ?
09:13tzimmermann: javierm, i think format selection is the problem here. lfb_depth can be 15, but bits_per_pixel is 16. consequently the test failed at https://elixir.bootlin.com/linux/v6.2/source/drivers/firmware/sysfb_simplefb.c#L40 that's why i replaced the direct use of lfb_depth
09:17javierm: tzimmermann: perfect, then if you trust lfb_linelength and lfb_width, you can just do: bpp = (lfb_linelength * 8 / lfb_width)
09:17javierm: I don't see why the calculation has to be so complicated with 3 max and using the color bits
09:18javierm: tzimmermann: since the SIMPLEFB_FORMATS array has the bpp that can be used to match the format
09:21javierm: we are trusting those anyways to calculate the I/O resource mem size as mentioned so I don't see why can't be trusted for the BPP too. That was my rationale in v2
09:21javierm: tzimmermann: but up to you, I'm OK with Pierre's patch too
09:24tzimmermann: javierm, in an email reply, i've given the example of allocating 800x600, which gives a stride of 832 pixel (at 4 bpp): 832 × 4 × 8 ÷ 800 = 33,28
09:26tzimmermann: that's 33 bits_per_pixel. that's what I meant with 'bpp and linelength are only vaguely connected'.
09:26tzimmermann: sorry for all the mess and confusion here
09:30javierm: tzimmermann: got it. On the contrary, thanks for the clarifications and sorry for my confusion
11:12karolherbst: MrCooper: ohh interesting, maybe I should run piglit on radeonsi then
11:13karolherbst: but normally that shouldn't happen :)
11:13karolherbst: I'll check what's wrong there
12:55MrCooper: karolherbst: to be clear, that was without your !22506
12:56karolherbst: right, but that one doesn't fix anything really
12:56karolherbst: just adds more validation
12:57karolherbst: the CL spec already requires the unused dimension to be set correctly, so I'm mostly curious what happens with piglit here
12:57karolherbst: could be also a piglit bug
12:58MrCooper: interesting
13:01tzimmermann: javierm, can you review some fbdev patchsets?
13:01karolherbst: could also be that the crash is when piglit checks for the error code to be returned :)
13:02javierm: tzimmermann: sure, I didn't because thought that all where reviewed by the drivers' maintainers
13:03karolherbst: either way, that MR doesn't fix the problems I'm seeing with darktables. Interesting enough, I do see them also on llvmpipe and older intel gens.. so might be some weird general problem. Maybe something annoying like clamping behavior
13:03javierm: tzimmermann: I see that Marek only provided his tested-by but not ack/review for "[PATCH 0/5] drm/exynos: Convert fbdev to DRM client"
13:03tzimmermann: javierm, yeah, those client conversions. i did not get much response for armada: https://patchwork.freedesktop.org/series/115848/
13:04tzimmermann: javierm, exynos is on its way into drm-next
13:04tzimmermann: the maintainer sent the PR today
13:04javierm: tzimmermann: ah, Ok. I'll review armada then after a meeting
13:05tzimmermann: thanks a lot
13:12karolherbst: uhm.. I think I just realized I pushed to the wrong drm-misc branch. If I want to get a fix in existing in 6.3-rc1, it should go through drm-misc-next-fixes, no?
13:12karolherbst: or drm-misc-fixes?
13:13karolherbst: or ist next-fixes for before rc1 was tagged?
14:04eric_engestrom: am I missing something, or is there zero CI coverage of the WSI right now?
14:04eric_engestrom: from what I can tell, not a single vulkan driver HWCI_START_WESTON or HWCI_START_XORG in its test jobs; is there another way to start them that I'm missing?
14:07javierm: tzimmermann: .fb_destroy was the callback that was executed only when the last client closed the fbdev fb right ?
14:07javierm: even after module removel / unregister
14:09tzimmermann: javierm, yes, it's the cleanup after the fbdev device has been removed. it's called from framebuffer_release() IIRC
14:11javierm: tzimmermann: yeah, just checked it. Just wanted to be sure that I reminded it correctly
14:12javierm: tzimmermann: r-b patches #3 and #4, didn't for #1 and #2 because I see that Sui already did for those
14:13tzimmermann: great, thank you so much. I'll leave the patchset around for a bit. maybe the maintainers still want to comment
14:13javierm: tzimmermann: you are welcome
14:13javierm: tzimmermann: any other patchset that you are missing a r-b or that is the last one from your series ?
14:18tzimmermann: javierm, so let me introduce you to that 100+ patches series that i've been working on for a while....
14:19tzimmermann: just kidding
14:20tzimmermann: there's still a similar series for i915. jani didn't have the time to review it. but i got errors from the CI. i'd first have to look through them and see if they are related
14:24javierm: tzimmermann: Ok
15:16jenatali: David Heidelberg: https://github.com/microsoft/DirectX-Headers/pull/93
15:24DavidHeidelberg[m]: jenatali: nice, thank you much ;)
15:25DavidHeidelberg[m]: I'll try ot today
15:31jenatali: Not sure if there's a better way to detect the use of the MinGW headers which is the real problem, 'cause there's also a Clang included in e.g. MSYS2
15:31jenatali: There's other workarounds like changing the include order that I guess we could do instead
16:12karolherbst: I wonder, are fixes like this valid enough to push them through fixes? https://patchwork.freedesktop.org/series/116536/ I don't think any of those actually fix bugs, they just mostly clean up bad style code or resolve undefined behavior
16:15mripard: karolherbst: yeah, if it doesn't actually fix anything there's no real reason to make them go through fixes
16:17mripard: take extra care with Markus Elfring patches though, he has (or used to have) very bad reputation
16:17karolherbst: yeah
16:17karolherbst: the patches are all fine tho
16:18karolherbst: just dealing with &NULL->some_field things
16:18karolherbst: mostly
16:18karolherbst: not sure what's the reason for bad reputation, just if it's because of stuff like that, I wouldn't mind as much. Though I'd still drop Fixes tags unless people don't care either way
16:19karolherbst: or rather, those patches just move such dereferences behind null pointer checks
16:19karolherbst: and the other part is seq_printf -> seq_puts/putc
16:26MrCooper: karolherbst: the bad reputation is because his patches tend to be trivial and some of them incorrect
16:28karolherbst: not sure if that alone justifies bad rep
16:36MrCooper: indeed, he also tends to turn review feedback into pointless arguments
16:38karolherbst: ah yeah, that's more of an issue 🙃
16:57mdnavare: daniels: Were you able to update the ssh keys for me so I can set up my dim?
17:02mdnavare: daniels: My fd.o username is mdnavare, how do I change that to match my login name from my Google linux machine?
17:03mdnavare: and I will upload the ssh keys in the plain text format to the ticket
17:05daniels: mdnavare: https://gitlab.freedesktop.org/freedesktop/freedesktop/-/issues/603#note_1871233
17:05daniels: thanks
17:05daniels: you can put 'User mdnavare' in your ~/.ssh/config
17:05daniels: you don't need to have the same username
17:06mdnavare: Okay so I can update the ~/.ssh/config to add mdnavare username and then regenerate the ssh keys?
17:07daniels: yes
17:11mdnavare: I didnt save my .ssh/config from my old laptop, could you share the example config for reference so I can update mine?
17:16mdnavare: what should be the format to add fd.or host and hostname in ssh/config: Host example
17:16mdnavare: HostName example.net
17:16mdnavare: User buck
17:18daniels: Host *.freedesktop.org
17:18daniels: User mdnavare
17:18daniels: it's also documented through man ssh_config
17:21mdnavare: okay added, attaching the ssh keys to the ticket now
17:21mdnavare: daniels: Also with this I should still be able to retain the commit rights right?
17:22mdnavare: I need to merge something to drm-misc
17:23daniels: yes, your account is still there from Intel and it still has the same permissions
17:24mdnavare: Okay great thanks a lot daniels , I have just uploaded the new keys directly so should be in plain txt
17:25daniels: thanks, I've added your new key now but it'll take ~10min until it's active
17:26mdnavare: Okay great, i will try my dim setup in a few mins
17:39DavidHeidelberg[m]: jenatali: tested; works fine. Thank you again
17:39jenatali: 👍
17:47mdnavare: daniels: Still failing, probably need to wait a bit longer
17:47mdnavare: ./maintainer-tools/dim setup
17:47mdnavare: Setting up drm-rerere ...
17:47mdnavare: mdnavare@git.freedesktop.org: Permission denied (publickey).
17:47mdnavare: fatal: Could not read from remote repository.
17:47mdnavare: Please make sure you have the correct access rights
17:47mdnavare: and the repository exists.
17:47mdnavare: dim: Failed to fetch drm-tip
17:50jenatali: David Heidelberg: Any known issues with one of the lima machines? https://gitlab.freedesktop.org/mesa/mesa/-/jobs/40121453
17:50jenatali: Seems consistently failing but unrelated to payload
17:50DavidHeidelberg[m]: jenatali: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22531
17:50DavidHeidelberg[m]: waiting :/
17:50jenatali: Aha, thanks
17:51DavidHeidelberg[m]: jenatali: sadly bump to kernel from 6.1 to 6.3 worked great for everything. Except disabled lima EGL tests :D
17:51jenatali: Yeah no worries, just wanted to make sure it was known
17:51DavidHeidelberg[m]: so when lima got brought up, it passed, but started flaking. enunes will check it when he gets back
18:06daniels: mdnavare: please attach the output somewhere of ssh -vvv git.freedesktop.org
18:09mdnavare: https://www.irccloud.com/pastebin/82t99x0i/Output_ssh_fdo
18:10mdnavare: daniels: heres the output
18:14daniels: mdnavare: your ssh client isn't trying to send that ssh key to us
18:14daniels: mdnavare: I guess you saved it somewhere that SSH doesn't know how to find it
18:15mdnavare: Hmm I ran ssh-keygen -t rsa and then gave a filename to save, should have kept that as default?
18:16mdnavare: It has saved the private key and publick key in HOME for me
18:16daniels: if you've saved it to a different path, then you need to tell ssh where you saved it - it doesn't find it automatically
18:16mdnavare: is it trying to find in .ssh/id_dsa ?
18:17DavidHeidelberg[m]: mdnavare: use -i path/file
18:17DavidHeidelberg[m]: dsa? these days we usually use rsa or 25519
18:17daniels: mdnavare: look at lines 265 onwards of the output you pasted; it tells you where it searches by default
18:18daniels: IdentityFile (documented in man ssh_config) alters the search path
18:19mdnavare: let me regenerate and save it in default location
18:19mdnavare: ?
18:20karolherbst: MrCooper: yeah.. so it was a negative test so the API validation part of that MR fixes it
18:20daniels: mdnavare: you can just move it to the default location, no need to regenerate it
18:20daniels: it's a file like any other
18:22mdnavare: just move the private key to .ssh/ and save as id_rsa right?
18:22daniels: yep, that should work
18:24mdnavare: cool, yes done! dim setup doesnt error out now so its probably fetching the repos 🤞
18:28mdnavare: Thanks a lot daniels its fetching everything now
18:33daniels: np
22:38kchibisov: What is :cs0 thread created by mesa responsible for?
22:50Kayden: that's the radeonsi command submission thread created by util_queue in src/gallium/winsys/amdgpu
22:50Kayden: radeonsi enqueues jobs, and that thread takes those and actually submits them to the kernel
22:51Kayden: (assuming you're on radeonsi; maybe other drivers have "cs" threads too that I'm not aware of)
22:51kchibisov: Yeah, I only have amdgpu hardware.
22:52Kayden: at one point it was created to hide the latency of command submission in the kernel, but at this point threaded submission is assumed to exist for some fencing stuff, I think.
22:53kchibisov: So it's basically a transmission channel of everything my application is doing to the kernel.
22:54Kayden: not quite everything - radeonsi constructs batches of GPU commands (set state, draw, and so on) and asks the kernel to execute those batches. those batch submissions are what go through the :cs0 thread, IIRC.
22:55Kayden: other things, like "allocate me a buffer" can just go directly to the kernel without going through that queue
22:55Kayden: having trouble with it?
22:56kchibisov: I have frequent page faults with my pure gles2 boring applications.
22:57kchibisov: Which mentions cs0 thread. Though, I have a feeling that GPU firmware is at fault.
22:58kchibisov: At this point, I'm trying to reduce vector or maybe developing some workaround in the application(at least for myself).
23:00Kayden: ah, GPU hangs?
23:01kchibisov: They don't really hang.
23:01kchibisov: It's more like "we try to handle" -> reset or recovered.
23:02Kayden: right
23:03Kayden: #radeon may be able to help too, though sounds like filing a bug against radeonsi at https://gitlab.freedesktop.org/mesa/mesa/-/issues with some way to reproduce it might be the way to go (if you haven't already)
23:03kchibisov: Kayden: oh, there're bugs already for that.
23:03Kayden: ah
23:03kchibisov: But I have a feeling that unless you fix it yourself there's no luck.
23:05Kayden:doesn't actually work on amdgpu...just read through that code a bit trying to understand how other drivers handle certain things
23:06kchibisov: I could easily make my program robust, though, the issue is that it won't help me :p
23:06kchibisov: Because my compositor is not robust yet.
23:07kchibisov: But it's not really a "solution".
23:11airlied: if you are getting faults from a gl or gles program, there is either a bug in your app or the shader compiler
23:11kchibisov: airlied: the app works fine for 2-3 years and crashes only for me on a specific GPU.
23:12kchibisov: And the crashes are recent thing.
23:12airlied: okay so might be a bug in the shader compiler or just some undefined behaviour raising its' head
23:12kchibisov: I could share the shaders, they are sort of simple.
23:13kchibisov: I know that I have crash with gles2 shaders, OpenGL 3.3 shaders, and when using zink as well.
23:14kchibisov: we could continue in #radeon, airlied if it's better.