<sima>
tzimmermann, thanks for completing the list, somehow I ignored the driver patches
vliaskov__ has joined #dri-devel
lsntvt_ has joined #dri-devel
vliaskov_ has quit [Ping timeout: 480 seconds]
<tzimmermann>
sima, thanks for the detailed replies
phasta has joined #dri-devel
<sima>
tzimmermann, wrt the hangs, I'm honestly not sure what's happening there, could be gpu hang
<sima>
but also wondering whether something with the import/export goes out of sync and that then leads to rendering to the wrong buffer or something like that
<tzimmermann>
sima, i've not seen that. i don't think it's related, but i'm not going to take a chance here
<sima>
yeah same
<sima>
it could be related to the ->dma_buf change in prime maybe, and your patch to hold a handle_count reference moved the race around enough for things to blow up
<tzimmermann>
sima, if we cannot unify dma_buf and import_attach->dmabuf in the gem object; we should document that one is only for export and one is only for import
<sima>
yeah
<sima>
was also thinking that some kunit to repro all the corner cases we've hit would be good
<sima>
since pretty clearly we (you, christian, me) don't really understand what's all going on there
<tzimmermann>
i'm preparing a patchset that reverts all the commits
<sima>
I was also completely taken by surprise how christians patch in drm-misc-next a month ago blew up
<sima>
thanks a lot!
<tzimmermann>
yeah, i'm somewhat conerned that we missed all the corner cases
<tzimmermann>
can i see the result of that intel-gfx-ci run somewhere?
<sima>
should show up on patchwork, but maybe I'm stuck in a moderation queue?
<tzimmermann>
sima, i've seen this. i'll check it later
<sima>
hwentlan_, no amd fixes pull this week?
<sima>
agd5f seems not around
sguddati has joined #dri-devel
Karyon has joined #dri-devel
<tzimmermann>
sima, sent out the reverts. smoke testing on various drivers looks good. the week's PR of drm-misc-fixes is already out. coud you merge it, and i'll send out a second PR with the reverts later?
<sima>
tzimmermann, was thinking I'll just apply them to drm-fixes directly
<tzimmermann>
ok, sure
<sima>
drm-misc-fixes is stuck on -rc1 anyway, backmerge can't hurt :-P
<tzimmermann>
i've cc'ed the intel ci bot
<sima>
thx
<sima>
somehow mine didn't make it on patchwork, no idea
<sima>
but the one I cc'ed to intel-xe did work earlier this week
<sima>
I guess reply on-list with what's busted (I don't see it immediately) and maybe more reverts :-/
<sima>
and I guess we need to re-baseline in this entire area with improved docs with all we learned, and kunit to hit the corners
<tzimmermann>
it breaks dma_buf_vmap when sharing between amdgpu and udl
<sima>
and then restart :-(
<sima>
same NULL check as the others?
<tzimmermann>
yeah, it's really not going well recently
<sima>
yeah
<sima>
so yeah, I guess more reverts because clearly we collective forgot how this all works :-(
<tzimmermann>
i didn't debug closely, but the vmap'ed pointer in udl appears to be NULL
<tzimmermann>
the buffer comes from amdgpu
<sima>
uh
<tzimmermann>
it's another easy revert, but still sucks to hit the error
<sima>
yeah I think we need to figure out how to kunit the common cross-driver and sharing paths
<sima>
since igt also didn't hit these really, with the exception of the issue from Christians re-export cleanup (which turned out to just break that)
<sima>
since trying to cover it all with hw testing seems unrealistic, and we have a lot of tricky corner cases in this entire area
<sima>
tzimmermann, replied to one patch with a bit more text that I want to include when merging, I think for the others I'll just add Fixes: lines
phasta has quit [Ping timeout: 480 seconds]
rsalvaterra_ has joined #dri-devel
rsalvaterra_ is now known as rsalvaterra
sguddati has quit [Ping timeout: 480 seconds]
pcercuei has joined #dri-devel
vliaskov__ has quit [Remote host closed the connection]
vliaskov__ has joined #dri-devel
vliaskov_ has joined #dri-devel
sguddati has joined #dri-devel
alarumbe has joined #dri-devel
vliaskov__ has quit [Ping timeout: 480 seconds]
fab has quit [Quit: fab]
fab has joined #dri-devel
guludo has joined #dri-devel
vliaskov_ has quit [Remote host closed the connection]
fab_ has joined #dri-devel
fab_ is now known as Guest21738
sguddati has quit [Ping timeout: 480 seconds]
fab has quit [Ping timeout: 480 seconds]
frankbinns has quit [Remote host closed the connection]
frankbinns has joined #dri-devel
frankbinns has quit [Remote host closed the connection]
frankbinns has joined #dri-devel
<tzimmermann>
sima, thanks a lot
<sima>
thanks to you too!
<sima>
tzimmermann, also I guess the getfb/getfb2 issues christian pointed out again needs an igt and then either a quick hack or maybe redo the handle_count stuff as a fix for that (but only after we're a lot more confident we have good test kunit coverage imo for this all)
<tzimmermann>
sima, i've not seen them with the original code
<tzimmermann>
they seem rare
kts has joined #dri-devel
<javierm>
sima, tzimmermann: probably worth adding an entry to Documentation/gpu/todo.rst ? About the need for kunits on this area
minecrell has quit [Quit: Ping timeout (120 seconds)]
sgerhold has quit [Quit: Ping timeout (120 seconds)]
<tzimmermann>
javierm, could do
sgerhold has joined #dri-devel
<tzimmermann>
it's non-trival though. the number of possible candidates is small :(
minecrell has joined #dri-devel
<javierm>
tzimmermann: that's OK I think, there are other tasks already with 'Level: Expert'
<sima>
tzimmermann, it needs a specific squence of ioctl that doesn't make sense for real userspace
<sima>
but you can hit a WARN_ON in the kernel with it, so it's kinda a very mild CVE issue
Guest21738 has quit []
<sima>
especially if you have oops-on-warn and reboot-on-oops enabled :-)
fab has joined #dri-devel
<sima>
javierm, might need a "Level: Too hard for maintainers" for this one :-/
<tzimmermann>
well, oops
<javierm>
sima: I see...
<tzimmermann>
javierm +1 :D
<sima>
tzimmermann, but it's also really old one, so imo not a good reasons to somehow try to squeeze the handle_count change in still
<sima>
tbh surprised syzkaller hasn't found it yet, it should be able to
FireBurn has quit [Remote host closed the connection]
<tzimmermann>
sima, i guess it could be solved by flagging the handle as gone, and later break the cycle during the framebuffer cleanup if flagged
<tzimmermann>
but that need to handle the situation without framebuffer as well
<sima>
yeah it all looks complicated and a bunch of cases
<sima>
I'm kinda leaning towards just detecting the case and bailing out with EALREADY or something
<sima>
since it's clearly userspace doing something funny, or we'd have gotten bug reports
jkrzyszt_ has quit [Remote host closed the connection]
nerdopolis has joined #dri-devel
<javierm>
tzimmermann: while reviewing your vesadrm 8-bit palettes patches, I rememberd that stumbled across the following while working on the st7567 support:
<javierm>
I wonder if that driver could use your new helpers too with a custom drm_crtc_set_lut_func callback
<tzimmermann>
that's a funny thing. i've seen it before
<tzimmermann>
but it encodes 3 gray-scale pixels in one byte. that's something else
<tzimmermann>
we need a more flexible way of writing these pixels to output buffers. right now it's just system or i/o memory. that leaves out all the devices on serial busses. i've been thinking about that though
<tzimmermann>
i first want to fix the 'read side' where we fetch the pixel data fro msource buffers.
<tzimmermann>
that funny format would likely be DRM_FORMAT_R3R3R2
<javierm>
tzimmermann: yeah, but it's still a C8 right? And it has this custom st7586_xrgb8888_to_gray332() too that might be worth to move to drivers/gpu/drm/drm_format_helper.c
<javierm>
or maybe not, since is too driver specific?
<tzimmermann>
javierm, it's a grayscale display AFAICT. and the first and second 3 bits index into a 8-entry grayscale lut
<tzimmermann>
the final 2 bits index into a 4-entry lut
<javierm>
tzimmermann: ahh, I see
<javierm>
such a weird format...
<javierm>
tzimmermann: anyways, your patches looked good to me. Even when I'm not an expert on these ancient displays and LUTs :) Sorry for taking me so long to review them
<tzimmermann>
thanks for reviewing. if it wasn't for you, i'd not make much progress with this
<tzimmermann>
those convert two drivers to use shadow-plane helpers.
phasta has joined #dri-devel
<tzimmermann>
with gem-dma, it's possible to get the buffer's vaddr in kernel space directly. but it's not recomnended. the clean way is to call vmap and use the returned pointers
<tzimmermann>
we have a maintainer for those two drivers, but i haven't heard of him yet
<javierm>
tzimmermann: sure, let me take a look to those now
<tzimmermann>
thank you so much
YuGiOhJCJ has quit []
<javierm>
tzimmermann: reviewing these patches reminded me that we talked at some point to get rid of drm_atomic_helper_damage_merged() callback (which both drivers use it)
vliaskov has joined #dri-devel
<javierm>
jfalempe found that is much better to iterate over the damage clips instead of using a merged rect, that could be quite big in some cases
<javierm>
tzimmermann: that might be a good candidate for an entry in Documentation/gpu/todo.rst since the changes are trivial
<tzimmermann>
i think so. but you also mentioned that i2c is really slow. there might be corner cases where merging updates makes sense
<javierm>
tzimmermann: hmm, right
<javierm>
you are correct, better to keep in these drivers then since we don't know what the trade offs are for these particular display controllers
<tzimmermann>
javierm, IIRC the original quake software renderer had to avoid redraws of overlapping updates at all cost (because of pentium performance). it used a dedicated algorithm to do that. it draw the scenery and the monsters on top, but each pixel was only written once. maybe that is something to look into here as well. both problems seem related.
agd5f has joined #dri-devel
yshui_ has quit [Remote host closed the connection]
yshui has joined #dri-devel
sguddati has joined #dri-devel
nerdopolis has quit [Read error: Connection reset by peer]