ChanServ changed the topic of #dri-devel to: <ajax> nothing involved with X should ever be unable to find a bar
danvet has quit [Ping timeout: 480 seconds]
nchery has joined #dri-devel
ybogdano has joined #dri-devel
ZeZu has quit [Server closed connection]
sh-zam has quit [Server closed connection]
sh_zam has joined #dri-devel
ZeZu has joined #dri-devel
angular_mike_____ has quit [Server closed connection]
ybogdano has quit [Read error: Connection reset by peer]
angular_mike_____ has joined #dri-devel
OftenTimeConsuming has quit [Server closed connection]
mbrost has joined #dri-devel
karolherbst has quit [Server closed connection]
karolherbst has joined #dri-devel
mhenning has joined #dri-devel
ybogdano has joined #dri-devel
OftenTimeConsuming has joined #dri-devel
mbrost_ has joined #dri-devel
nchery has quit [Ping timeout: 480 seconds]
mbrost has quit [Ping timeout: 480 seconds]
nchery has joined #dri-devel
rasterman has quit [Quit: Gettin' stinky!]
camus1 has joined #dri-devel
iive has quit []
mbrost_ has quit [Ping timeout: 480 seconds]
<kisak>
a little bit of future planning, with the drirc conf files splitting apart by driver, it'd be nice for them to all exist in the same location instead of spread across the source tree.
camus has quit [Ping timeout: 480 seconds]
mclasen has quit [Ping timeout: 480 seconds]
ybogdano has quit [Read error: Connection reset by peer]
co1umbarius has joined #dri-devel
mhenning has quit []
columbarius has quit [Ping timeout: 480 seconds]
aura[m] has quit [Server closed connection]
aura[m] has joined #dri-devel
gagallo7[m] has quit [Server closed connection]
gagallo7[m] has joined #dri-devel
tursulin has quit [Ping timeout: 480 seconds]
ybogdano has joined #dri-devel
kts has quit [Ping timeout: 480 seconds]
ybogdano has quit [Ping timeout: 480 seconds]
idr has quit [Quit: Leaving]
maxzor has quit [Remote host closed the connection]
mbrost has joined #dri-devel
sdutt has joined #dri-devel
camus has joined #dri-devel
camus1 has quit [Read error: Connection reset by peer]
ngcortes has quit [Remote host closed the connection]
mrcivvy_ has quit []
mrcivvvy has joined #dri-devel
mlankhorst_ has quit [Ping timeout: 480 seconds]
<imirkin>
anholt: adding some sort of "randomize" would be nice to better detect flakes
<imirkin>
anholt: i think i figured a bunch of the esp weird fails that i saw on nouveau are actually flakes, dependent on run order
<imirkin>
so they fail consistently, given fixed inputs
mclasen has joined #dri-devel
<zmike>
yea that would be a cool option
mrcivvvy has quit []
mrcivvvy has joined #dri-devel
mnadrian has quit [Ping timeout: 480 seconds]
mrcivvy has quit [Quit: Page closed]
lemonzest has joined #dri-devel
shankaru has joined #dri-devel
jewins has quit [Ping timeout: 480 seconds]
nielsdg has quit [Server closed connection]
nielsdg has joined #dri-devel
Andy[m] has quit [Server closed connection]
Andy[m] has joined #dri-devel
blue_penquin has quit [Server closed connection]
blue_penquin has joined #dri-devel
chivay has quit [Server closed connection]
chivay has joined #dri-devel
Anson[m] has quit [Server closed connection]
Anson[m] has joined #dri-devel
doras[m] has quit [Server closed connection]
doras[m] has joined #dri-devel
egalli has quit [Server closed connection]
egalli has joined #dri-devel
gnustomp[m] has quit [Server closed connection]
gnustomp[m] has joined #dri-devel
halfline[m] has quit [Server closed connection]
halfline[m] has joined #dri-devel
zzoon[m] has quit [Server closed connection]
zzoon[m] has joined #dri-devel
jekstrand[m] has quit [Server closed connection]
jekstrand[m] has joined #dri-devel
moben[m] has quit [Server closed connection]
moben[m] has joined #dri-devel
shankaru has quit [Quit: Leaving.]
Strit[m] has quit [Server closed connection]
Strit[m] has joined #dri-devel
T_UNIX has quit [Server closed connection]
T_UNIX has joined #dri-devel
undvasistas[m] has quit [Server closed connection]
undvasistas[m] has joined #dri-devel
shankaru has joined #dri-devel
kts has joined #dri-devel
Venemo has quit [Server closed connection]
Venemo has joined #dri-devel
kts has quit [Quit: Konversation terminated!]
mattrope has quit [Read error: Connection reset by peer]
Duke`` has joined #dri-devel
nchery has quit [Read error: Connection reset by peer]
mlankhorst_ has joined #dri-devel
itoral has joined #dri-devel
mclasen has quit [Ping timeout: 480 seconds]
mszyprow has joined #dri-devel
danvet has joined #dri-devel
pnowack has joined #dri-devel
mszyprow has quit [Ping timeout: 480 seconds]
test has joined #dri-devel
RAOFhehis[m] has left #dri-devel [#dri-devel]
test has quit [Remote host closed the connection]
kts has joined #dri-devel
kts has quit []
RAOF has joined #dri-devel
frieder has joined #dri-devel
alanc has quit [Remote host closed the connection]
alanc has joined #dri-devel
MajorBiscuit has joined #dri-devel
tursulin has joined #dri-devel
mszyprow has joined #dri-devel
<karolherbst>
jenatali, jekstrand, airlied: one thing I am wondering about is if it makes sense to have a spirv per device or if it makes sense to enforce that all devices get the same spirv. I just don't know have enough experience to make a good judgement here ond what makes sense. Any thoughts?
kts has joined #dri-devel
<karolherbst>
mhh, but the API allows binaries to be compiled differently for each device...
lynxeye has joined #dri-devel
ahajda has joined #dri-devel
frankbinns has joined #dri-devel
<pq>
urgh, is there any fbdev image viewer that would just write the image into /dev/fb1 and quit and not care about ttys or anything...
pcercuei has joined #dri-devel
<pinchartl>
pq: `cat ... > /dev/fb1` ? :-)
<pq>
my image is a PNG... but that's what I'm aiming at now
nottin has joined #dri-devel
sdutt has quit [Ping timeout: 480 seconds]
<pinchartl>
can imagemagick come to the rescue to convert the PNG to a binary format suitable for fbdev ?
sneil has quit [Remote host closed the connection]
<pq>
this turned out quite nicely. Adjusted curves in Gimp, and pushed the image to fbdev while in HDR mode. Much better than the SHINING of an uninitialized buffer.
mbrost has quit [Ping timeout: 480 seconds]
<danvet>
mripard, msdos line endings, what happened?
<danvet>
mripard, legacy cursor updates are busted and might be best to just rip them out
<danvet>
mripard, so imo fix this by nuking legacy cursor (at least in vc4)
<danvet>
I've been tempted a bunch of times to just outright do this in atomic helpers, but the fallout might be a bit much
<danvet>
but maybe now we have enough drivers using the async plane stuff to get away with the fallout
<mripard>
I still think this is the right thing to do, disregarding whether we have legacy cursor or not
<danvet>
padovan was iirc working on this
<mripard>
the entry can still be used up until next vblank, so it makes sense to keep it around at least as long anyway
<mripard>
but noted, it's added on my todo list :)
<danvet>
mripard, I need to understand this first, and my brain is crap these days
<danvet>
expect silly questions :-)
<mripard>
danvet: yeah, go ahead :)
<danvet>
mripard, so maybe I'm not understanding anything, but the irq race should be covered by using flip_done instead of vblank wait
<danvet>
to make sure if you've raced somewhere, we're waiting long enough
<mripard>
it's not really a race condition, or at least not in software
<danvet>
and also the next commit (which is the one which will release all the old crap due to swap_states())
<danvet>
is supposed to wait for both flip done and for hw_done (i.e. any programming done synchronously)
<mripard>
the hardware uses hardware descriptors, and those aren't latched at vblank, so if you modify them during the frame rendering, you're screwed
<danvet>
mripard, if your flip_done doesn't exactly match what hvs does, it's a lie and races somewhere
<danvet>
iow I think if you just keep your dlist allocation until crtc state destruction, you should be fine
<danvet>
which I thought is how this works?
<danvet>
if your old dlist entry is still used after crtc_destroy_state
<danvet>
something is very busted
<danvet>
since that's supposed to be after flip_done
<danvet>
and if your flip_done is correct, your hw really should have moved on
<danvet>
plus/minus legacy cursor update totally wreaking this, which is why I think it should be nuked
<mripard>
yeah, indeed, and getting rid of legacy cursor update makes sure we always have a flip_done called
<danvet>
so iirc if you just clear legacy cursor at the top of your atomic_commit it should be all fine
<mripard>
I *think* the userspace running on the RPi depends on the legacy cursor update, so this would be unfortunate
<danvet>
maybe even at the top of atomic_commit_tail
<mripard>
but I'll give it a try
<danvet>
mripard, async plane updates is suppposed to fix this properly
<danvet>
and essentially anytime you can't do async update and fall back to sync update
<danvet>
is when there would be a bug in the legacy cursor code anyway :-)
<danvet>
mripard, one thing we could look into is doing some non-blocking commits behind the scenes
<danvet>
and batching updates in the cursor code when we pile them up
<danvet>
that at least avoids the stall
<danvet>
kinda like dirtyfb handling
kts has quit [Quit: Konversation terminated!]
kts has joined #dri-devel
<danvet>
mripard, so essentially in the cursor code check before we do the atomic_commit that it's still async
<danvet>
and if not, push it all to a cursor worker
<danvet>
and the cursor worker would batch up everything
rasterman has joined #dri-devel
<danvet>
it'll suck a bit since it also stalls everything else, but oh well
<MrCooper>
pq: there's fbi
<pq>
MrCooper, I tried, but I could not convince it that it really does not need to mess with the tty.
<MrCooper>
oh well
<mripard>
danvet: and you were saying your brain is crap these days ? :)
<pq>
so it just failed on permissions error on tty device
<mripard>
I don't know the cursor code that well, I'll have a look and get back to you
Duke`` has quit [Ping timeout: 480 seconds]
<tjaalton>
dcbaker: it's been three weeks since the previous 22.0-rc, when can we expect the next one?
Duke`` has joined #dri-devel
<danvet>
mripard, dirtyfb code and maybe a chat with seanpaul about how that works
<danvet>
I'm a bit worried about semantics of a hand-rolled nonblocking mode as fallback
<danvet>
but then the entire legacy cursor hack is terrible, and I'm not sure we can make it really worse
<danvet>
and doing that nonblocking trick might be enough for us to drop the legacy cursor hack crap from at least helpers
<danvet>
since it's causing way too much surprises and bugs
<danvet>
imo
<danvet>
but it's also a very unfortunate mess and we've definitely not yet reached an optimal solution
<mripard>
thanks
itoral has quit [Remote host closed the connection]
itoral has joined #dri-devel
pcercuei_ has joined #dri-devel
pcercuei has quit [Ping timeout: 480 seconds]
adjtm has joined #dri-devel
flacks has quit [Quit: Quitter]
flacks has joined #dri-devel
itoral has quit [Remote host closed the connection]
itoral has joined #dri-devel
itoral has quit [Remote host closed the connection]
itoral has joined #dri-devel
itoral has quit [Remote host closed the connection]
itoral has joined #dri-devel
itoral has quit [Remote host closed the connection]
itoral has joined #dri-devel
ella-0 has joined #dri-devel
ella-0_ has quit [Read error: Connection reset by peer]
itoral has quit [Remote host closed the connection]
itoral has joined #dri-devel
DPA has quit [Server closed connection]
mattst88 has quit [Read error: Connection reset by peer]
DPA has joined #dri-devel
itoral has quit [Remote host closed the connection]
itoral has joined #dri-devel
mattst88 has joined #dri-devel
itoral has quit [Remote host closed the connection]
itoral has joined #dri-devel
itoral has quit [Remote host closed the connection]
itoral has joined #dri-devel
rkanwal has joined #dri-devel
JohnnyonFlame has joined #dri-devel
adjtm has quit [Quit: Leaving]
mclasen has joined #dri-devel
<swick>
Lyude: so the VESA DPCD registers for HDR stuff is that in the eDP spec?
jannau has quit [Server closed connection]
jannau has joined #dri-devel
kts has quit [Quit: Konversation terminated!]
itoral has quit [Remote host closed the connection]
kts has joined #dri-devel
JohnnyonFlame has quit [Ping timeout: 480 seconds]
gio has quit [Ping timeout: 480 seconds]
sdutt has joined #dri-devel
devilhorns has joined #dri-devel
mattrope has joined #dri-devel
shashanks has joined #dri-devel
<jekstrand>
karolherbst: My initial gut says all the same but I don't grok opencl's per-device mentality
<jekstrand>
karolherbst: Likely, it has to be per-device from an ICD loader POV. Whether or not it has to from the perspective of the Mesa state tracker, I don't know.
<karolherbst>
jekstrand: yeah.. the issue is just, that you can call clCompileProgram with a different device each time and pass in different flags and/or headers :/
<jekstrand>
I'm inclined to say no since it's the same OpenCL C. Unless we think we're going to use a different set of SPIR-V extensions to compile the same bit of C, I don't see a reason to.
f11f12 has joined #dri-devel
<jekstrand>
Oh, in that case, it's per-device. No questions asked.
<marex>
mripard: thanks, I'm looking at the two outstanding patches
<jekstrand>
Maybe we can de-duplicate a bit but that's about it.
<karolherbst>
jekstrand: the thing is just.. what if you have different kernel entrypoints per device
<karolherbst>
clGetProgramInfo can be used with CL_PROGRAM_NUM_KERNELS, so what to return?
<marex>
mripard: it is like I have this very weird dejavu where I wrote DT bindings extension for data-lanes already ... and I dont see the patch, odd
<karolherbst>
so I am wondering if I miss something or if this is a huge spec bug
<karolherbst>
I mean.. you could probably even link devices differently? Not quite sure
<karolherbst>
ehh wait
<karolherbst>
mhhhh
<marex>
mripard: ah yes of course ... different bridge
<karolherbst>
okay.. so after linking you get one program object, so at least after linking it should be "the same" for each device, although I am not quite sure how much one could cheat with messing around with headers
<karolherbst>
jekstrand: I think I will just require that the spir-v info we extract from each linked binary is the same for each device and move on
<karolherbst>
and if something is different we should fail to link
<karolherbst>
that would include kernels, spec constants and ctors/dtors
Haaninjo has joined #dri-devel
<karolherbst>
what I don't understand is that CL apparently allows the link to fail for _some_ devices and then you are still able to do stuff with it
<karolherbst>
*sigh*
<karolherbst>
who designed it this way and though that's a good idea...
fxkamd has joined #dri-devel
f11f12 has quit [Quit: Leaving]
tzimmermann has joined #dri-devel
pendingchaos_ has joined #dri-devel
pendingchaos has quit [Ping timeout: 480 seconds]
pendingchaos_ is now known as pendingchaos
ppascher has quit [Quit: Gateway shutdown]
shankaru has quit [Quit: Leaving.]
<jekstrand>
karolherbst: Yeah... idk. It seems like a total mess.
<jekstrand>
karolherbst: But maybe we can guarantee our links are all or nothing?
<karolherbst>
jekstrand: mhhh....
idr has joined #dri-devel
<karolherbst>
maybe?
<karolherbst>
jekstrand: ... ehhh I think this was intentional
<karolherbst>
"CL_INVALID_KERNEL_DEFINITION if the function definition for __kernel function given by kernel_name such as the number of arguments, the argument types are not the same for all devices for which the program executable has been built."
lemonzest has quit [Quit: WeeChat 3.4]
<karolherbst>
that's for clCreateKernel
<karolherbst>
so my understanding is.. the cl_program thingies can be pretty much whatever, but once you create a kernel it has to be sane enough or something
<karolherbst>
what a mess
ppascher has joined #dri-devel
<jekstrand>
karolherbst: It sounds like they're trying to allow you to have different versions for different devices. Like maybe you have an intel-optimized version and an nvidia-optimized version that nominally do the same thing.
<karolherbst>
yeah, I'd assume that's kind of the idea
<karolherbst>
but if you can't create kernels for it, what's the point
lemonzest has joined #dri-devel
<jekstrand>
What do you mean "can't create kernels for it"?
<karolherbst>
jekstrand: so to create a kernel out of a program you pass in the program and the kernel_name, that's it
<karolherbst>
no device list or something
<karolherbst>
so if the binaries inside the program object is different for each device you can't do anything with it
<karolherbst>
well.. different as in different signature
<jekstrand>
Right
<karolherbst>
I think it's fine if the code differs between devices, because of random optimizations or whatever, but it's a bit strange that program objects could have different kernels for each device :/
<jekstrand>
But you can have very different binaries as long as the signature matches.
<jekstrand>
Why not? Again, I think this is for the HW-specific optimizations case.
<karolherbst>
matching signature is not requierement
<karolherbst>
you just can't create kernels
<karolherbst>
but you can link that stuff
<jekstrand>
Sure
<jekstrand>
So you make hw-specific SPIR-V for your cl_program[s] which are optimized differently, then you link and then you create a cl_kernel which has different code for different devices but the same signature and it works across devices. Makes sense in a weird twisted openCL sort of way.
fxkamd has quit []
<karolherbst>
jekstrand: ohh sure, but why can it be different after linking?
<jekstrand>
Not sure. I don't have enough of the details paged in.
<karolherbst>
why isn't the runtime responsible for checking and just failing to link if stuff doesn't match
<karolherbst>
yeah...
<karolherbst>
maybe there is a weird reason to allow this, but...
<karolherbst>
"Kernel objects are not created for any __kernel functions in program that do not have the same function definition across all devices for which a program executable has been successfully built."
<karolherbst>
so there we just skip the weird ones...
<karolherbst>
ughhhh... whatever, I think this is all very stupid, but whatever
<jekstrand>
Yeah, skip the weird ones seems odd but on well
<karolherbst>
the big question remains though, what to do with CL_PROGRAM_KERNEL_NAMES and CL_PROGRAM_NUM_KERNELS
<karolherbst>
maybe we have to create a merged list?
<karolherbst>
so if one device has foo and bar, the other foo and bar2, we return 3 with foo, bar and bar2?
<karolherbst>
"Returns the number of kernels declared in program that can be created with clCreateKernel"
<karolherbst>
which is... somehow true? or maybe just return the ones where it would succeed?
<karolherbst>
mhhh, I am sure that's how it's supposed to be
<karolherbst>
at least that wouldn't be too painful to implement
<karolherbst>
just return whatever is available and hope clients don't depend on that weird stuff
sdutt has quit []
sdutt has joined #dri-devel
<jekstrand>
I think we're starting to get into the territory of "implement something and see if CTS tests fail"
<jekstrand>
Maybe we need to only return the things where everything matches and creating would succeed.
<jekstrand>
But we can also kick that can down the road, I think.
<karolherbst>
sure
<karolherbst>
I was just thinking about it because a test checks CL_PROGRAM_KERNEL_NAMES and CL_PROGRAM_NUM_KERNELS
<jekstrand>
right
<karolherbst>
but I don't think the CTS is testing it in depth, so just wanted to make sense out of all of this
JohnnyonFlame has joined #dri-devel
sneil has quit [Remote host closed the connection]
sneil has joined #dri-devel
adjtm has joined #dri-devel
mbrost has joined #dri-devel
mbrost_ has joined #dri-devel
mbrost has quit []
mbrost_ has quit []
mbrost has joined #dri-devel
<krh>
alyssa: did you experiment with a rust backend for nir?
<dcbaker>
do you have any ideas, there's nothing obvious to me that would cause those regressions
<alyssa>
why?
<jenatali>
dcbaker: baseline it - those were incorrect passes, the phi change in the compiler fixes the shader code and exposes those failures
<dcbaker>
jenatali: thanks! I'll add your a-b to the patch if that's cool with you?
<jenatali>
Yep!
<jenatali>
My bad for not putting the CI expectations update into the Fixes: patch
<jasuarez>
anholt: in `poe-powered.sh:110` we are updating the lib/modules from the BOOTFS if those directories doesn't already exists
ybogdano has joined #dri-devel
<jasuarez>
it seems now `/lib/ comes in the the rootstrap, so we don't update it from BOOTFS, which contains the v3d drivers
<jasuarez>
I think we want to propagate $BM_BOOTFS/lib/modules/ and $BM_BOOTFS/usr/lib/modules in any case
<anholt>
jasuarez: I saw the rsync happening for lib/modules to usr/lib/modules, but modprobe was looking in /lib/modules? so I just tried making the rsync from lib/modules go to lib/modules and maybe that'll help?
tango_ has quit [Read error: Connection reset by peer]
tango_ has joined #dri-devel
<swick>
Lyude: just want to know what kinds of control exactly they get us over the displays
frieder has quit [Remote host closed the connection]
enunes has joined #dri-devel
ybogdano has quit [Read error: Connection reset by peer]
<airlied>
dcbaker: yes for ab
nchery has joined #dri-devel
rkanwal has quit [Quit: rkanwal]
rkanwal has joined #dri-devel
ybogdano has joined #dri-devel
lynxeye has quit []
devilhorns has quit []
JohnnyonFlame has quit [Ping timeout: 480 seconds]
mszyprow has quit [Ping timeout: 480 seconds]
Duke`` has quit [Ping timeout: 480 seconds]
ybogdano has quit [Read error: Connection reset by peer]
<dcbaker>
thanks!
<tjaalton>
dcbaker: cool, thanks
<dcbaker>
sorry for the long delay
Duke`` has joined #dri-devel
<jekstrand>
karolherbst: Have you pushed recently>{
<jekstrand>
?
<karolherbst>
yep
<karolherbst>
why?
<jekstrand>
It seemed to be all one big squash still so I didn't know if you'd done anything
<karolherbst>
ahh, yeah I've split out the opencl bindgen and added some more things
<karolherbst>
but I am actually wondering about if it makes sense to split up the stuff or not
<jekstrand>
At some point, it makes sense to do commits just for the sake of collaboration.
<jekstrand>
Easier to know what's changed. Easier to deal with rebase problems, etc.
<karolherbst>
yeah.. that's true
mlankhorst_ has quit []
<krh>
yeah, +1 to that
<karolherbst>
I hope you didn't had too much local changed if at all? But if you are starting to write code I can also stop squashing everything into one commit
<jekstrand>
I don't. I was mostly playing around with the API object nonsense. None of my tries have gone anywhere yet, though, so I'm happy to start over.
<karolherbst>
okay :)
<karolherbst>
I was thinking about pulling some code out of the macro, but I wasn't sure what you already did and if you changed anything at all there
<karolherbst>
but at least you won't need this include! hack anymore
<jekstrand>
Yeah, I'm still trying to come up with a better way to do the base objects. I'll keep playing with that today.
<jekstrand>
I'm learning new things about Rust at every step so it's all good.
<karolherbst>
yeah, me as well
<karolherbst>
and I keep remembering things I already forgotten :D
<jekstrand>
hehe
<jekstrand>
karolherbst: Also, at some point, I'd like to run rustfmt on the whole thing. Since this is the first rust code in Mesa and rust is an opinionated language, we may as well strictly adhere to the rust coding conventions.
<karolherbst>
sure
<karolherbst>
let me do it, but I think I made sure I don't do weird things
<jekstrand>
ok
<krh>
maybe even rustfmt as a git commit hook
<karolherbst>
ehhhhhh
<karolherbst>
whitespaces
<krh>
once it's properly rustfmt'ed whitespace shouldn't change
<karolherbst>
yeah.. but we do 3 spaces :D
<jekstrand>
We don't have to do 3 spaces in our rust code. :)
<karolherbst>
rustfmt has a config file we can change that stuff
<karolherbst>
I think
<krh>
rustfmt.toml, but yes, rust would be a good reason to get away from the 3 spaces
<karolherbst>
I can also just run rustfmt once and we just use the default stuff
<jekstrand>
yup
<jekstrand>
That's what I was suggesting
<krh>
sounds good
<karolherbst>
mhh rustfmt also is doing some annoying linebreaks...
<krh>
struct_lit_width = 40 in rustfmt.toml, might help
<karolherbst>
krh: it's more like the { after the function decl
<karolherbst>
so I kind of prefer to put in a new line before {
<karolherbst>
dcbaker: ohh, cool. Although I am not quite sure we'll actually need it, but it's good to have the option. _but_ there are some nice crates being proc macro crates which can be useful
<karolherbst>
like the ones actually allowing to create new idents in macros
ybogdano has joined #dri-devel
<dcbaker>
We're going to freeze for 0.62 on sunday, and I'm trying really hard to the structured_sources work in, so that we can make working with generated rust more pleasant
<karolherbst>
but that's really not a lot of code :O
<dcbaker>
yeah, basically just some error handling and version checking, plus a test case
bluebugs has joined #dri-devel
danvet has quit [Ping timeout: 480 seconds]
mclasen has quit [Ping timeout: 480 seconds]
lemonzest has quit [Quit: WeeChat 3.4]
<idr>
karolherbst: I'll second that we don't have to do 3 spaces in Rust.
<idr>
I wish we hadn't done 3 spaces in Python. :(
<karolherbst>
idr: there are worse things in terms of coding style
<krh>
I'm not sure
<karolherbst>
krh: ever seen GNU coding style in the real world?
<idr>
There's the style where braces are on a separate line and indented somewhere between the line before and the line after.
<idr>
if (...)
<krh>
yeah... gtk uses it iirc
<idr>
{
<idr>
asdf;
<karolherbst>
idr: that's GNU
<karolherbst>
:D
<idr>
It's Guh-awful.
<krh>
it's verbose, lots of vertical space wasted
<karolherbst>
so, do we want to discuss 3 spaces again, or do we settle with "there are worse things"? :P
<karolherbst>
but that's not even the worst thing
<karolherbst>
they put a space between the function name and brackets
<krh>
I don't think it makes sense to deviate from the rust standard
<karolherbst>
so it's memcpy (....
<jekstrand>
I don't care too much about 3 vs. 4 spaces.
<jekstrand>
I kind-of hate it but I'm numb at this point
<karolherbst>
krh: I mean.. I am not strong about 3 vs 4 either, but mesa uses 3 and inconsistency is kind of more annoying
<dcbaker>
karolherbst: most of the python is 4 spaces I thought?
<krh>
karolherbst: it's a different language though
* dcbaker
says do whatever Rust does normally
<karolherbst>
dcbaker: it's not even consistent within python files in mesa
<krh>
a language that comes with a strong coding convention
* dcbaker
also says python should be changed to 4 spaces, since that's the language standard
<karolherbst>
src/compiler/nir/nir_opt_algebraic.py uses 3 and 4
<karolherbst>
e.g.
<krh>
and a tradition for not inventing one's own variation
* dcbaker
-> convert to 4 spaces
<jekstrand>
Ok, I'm convinced. 4.
Duke`` has quit [Ping timeout: 480 seconds]
<dcbaker>
odd, the python .editorconfig is set to 4 spaces
<jekstrand>
dcbaker: That doesn't mean we've gone through and fixed everything
<Sachiel>
should have 3 spaces on even lines and 4 on odd ones
JohnnyonFlame has joined #dri-devel
<ccr>
per-file even pseudorandom distribution of indentation between 1-8 spaces, but every other line using tabs so appease the tab-people
<ccr>
+statistically even
shashanks has quit [Ping timeout: 480 seconds]
<dcbaker>
4 characters, but randmoly mixed spaces and tabs
<alyssa>
My take is "use the language standard if it exists", which it does for Rust and Python
<alyssa>
Undecided what to do for C(++), panfrost is a horrible mix of 3- and 8-spaces.
<alyssa>
(It started as 8 and I wanted to migrate to 3 to be consistent with Mesa but have been too scared of conflicts to pull the clang-format trigger on the old code.)
<alyssa>
(Somehow this is the worst of both worlds.)
rkanwal has quit []
<alyssa>
i cc stable enough that reformatting now seems mean
gouchi has quit [Remote host closed the connection]
<dcbaker>
actually, if you clang-format everything that makes the stable job easier
<dcbaker>
I can just run clang-format on the stable branch, then all of your patches apply nicely
<dschuermann>
alyssa: just do like us: create a clang-format file, reformat everything and then keep forgetting about it, so that misformatted stuff keeps piling up anyways ;)
* dschuermann
needs a mesa-ci job for that
<jekstrand>
karolherbst: I just pushed a bit of stuff to risticl/object-wrappers in my tree
<jekstrand>
karolherbst: I don't know if I actually like it
<karolherbst>
mhhhhh
<karolherbst>
why the phantom?
<karolherbst>
ohh, mhh
<jekstrand>
karolherbst: A few things I don't like in the current thing:
<jekstrand>
1. I don't like auto-conversion in icd.rs and pretending things are OO. Let's just make from_cl() quick and easy and do that in the implementation of each entrypoint.
<jekstrand>
2. The macro is super magic an declares a thing with a weird type name.
<jekstrand>
3. I don't really like having two layers. If we can have it all in one, that's better.
<jekstrand>
Things I DO like with the current thing:
<jekstrand>
1. Using Result<_, i32> for error handling
<jekstrand>
Things I don't like about my thing:
<alyssa>
dcbaker: Tempting
<jekstrand>
1. release() being a method on the ICDObject feels really sketchy. I've gotten around the sketch by making it return an Arc<T> in the case where it drops the last ref. That at least ensures that the underlying object it's baked into survives until after the function call is done
<alyssa>
dcbaker: Also, I have 100+ patches for Valhall pending, and there's a benevolent panfrost fork with another 100 patches or so, and while I know there's a git rebase clang-format magic script, it seems mean
<dcbaker>
do it right before 22.1?
<jekstrand>
Really, though, thanks to dealing with pointers in the API which may point to Rust objects, there is no real "safe" anywhere when it comes to the lifetimes of those objects. :-( Maybe my release() isn't so bad?
<karolherbst>
mhhh
<alyssa>
dcbaker: i do not remember why it's better to do it right before instead of right after the branch point (or whenever)
<karolherbst>
jekstrand: why can't your release be &mut self?
<zmike>
dcbaker: isn't meson --reconfigure supposed to pull in new versions of deps?
<dcbaker>
zmike: meson configure --clearcache Is what you need I think
<zmike>
ahhh
<zmike>
thanks
<dcbaker>
alyssa: If you do the reformat before the branchpoint then any backport patches apply cleanly
<alyssa>
right, ok
<karolherbst>
jekstrand: there is one thing really annoying though, and that's when we really start to use the objects
<karolherbst>
like e.g. in containers
<karolherbst>
and you need to impl PartialEq and stuff
<alyssa>
dcbaker: also I'd have to settle on a style and is 3-spaces *really* better ;-p
<karolherbst>
but mhh
<karolherbst>
this init_ref stuff is also kind of annoying
<dcbaker>
alyssa: that's the really hard question :)
<jekstrand>
karolherbst: release probably could be &mut but that doesn't prevent them from using the reference afterwards
<jekstrand>
Then again, nothing prevents that
<karolherbst>
it's an arc anyway
<jekstrand>
Yeah, but release() may drop the last arc ref
<karolherbst>
ehh, right, as the wrapper is embedded in the actual thing
<karolherbst>
jekstrand: ehm... you don't restore the Box thing, right?
<karolherbst>
ehh wait, you got rid of that
mbrost has quit [Ping timeout: 480 seconds]
<karolherbst>
jekstrand: did you even runtime test it?
<karolherbst>
because I'd assume this just causes memory issues
<karolherbst>
mhhh, maybe it wouldn't
pnowack has quit [Quit: pnowack]
<jekstrand>
karolherbst: Yup. Passes the same test_basic set
<karolherbst>
jekstrand: soo.. there is one thing I am curious about and I think I am sure it's also a problem in my impl
<karolherbst>
so
<karolherbst>
client allocates an object, no matter what
<karolherbst>
does stuff
<karolherbst>
calls release
<karolherbst>
the API refcounter is 0, and we might hold internal references
<karolherbst>
e.g. a cl_program thing holding a list of cl_device_id objects
<karolherbst>
now we have those APIs which _return_ those devices
mbrost has joined #dri-devel
<karolherbst>
what do we do then?
<karolherbst>
we have to return a pointer to the ICD thing again, and it has to contain valid data (as the ICD uses the func table for stuff)
<karolherbst>
I had this issue kind of in the back on my head, but choose to ignore it for now
<karolherbst>
but what happens if we have to return those internal references even though the client already released stuff
<karolherbst>
(we could just add the strong count to refcnt and check the sum in release, but ughhh)
<jekstrand>
karolherbst: For device, there are special rules. You can't call ReleaseDevice on the ones held by the CL implementation.
<karolherbst>
mhh, good point
<karolherbst>
jekstrand: then same thing with a kernel and a program
<karolherbst>
kernel holds ref to the program, but client already released the program
<jekstrand>
karolherbst: Right... That's problematic, I guess.
<jekstrand>
karolherbst: So, mine is built on the principle that you init_ref() and that stashes an Arc in the ICDObject and sets the reference count to 1.
<karolherbst>
yeah.. well.. even the spec is saying we can't release it even if the refcount is 0
<karolherbst>
"The program object is deleted after all kernel objects associated with program have been deleted and the program reference count becomes zero."
<jekstrand>
Right
<jekstrand>
This is truly pathological :-/
<karolherbst>
I think we need one refcounter, but... that's just super annoying
<karolherbst>
jekstrand: although I think if we keep a Arc clone and call dec/increment_strong_count on the C pointer it should be safe to do so...
<jekstrand>
Yeah, but we've got to track the client refcount
<karolherbst>
but how do we do it in a way that we eventually release it
<jekstrand>
yup
<karolherbst>
mhhh...
<karolherbst>
wait...
<karolherbst>
I think I might have an idea
<karolherbst>
I think the doc in rust is contradicting itself
<karolherbst>
yes
<karolherbst>
jekstrand: we don't have to call Arc::from_raw
<karolherbst>
but that's ugly nonetheless, as it would require us to do from_raw/into_raw dances once we have to create internal refs
<karolherbst>
but decrement_strong_count actually calls from_raw internally
<jekstrand>
I'm not following
<karolherbst>
heh.. we can actually do funny things
mbrost_ has joined #dri-devel
<jekstrand>
Hrm... I guess we could keep a second reference count side-band to report out via queries and then inc/dec_strong
<karolherbst>
no, we don't have to with Arc actually
<jekstrand>
?
<karolherbst>
we could use Arc::as_ptr() to return the object
<karolherbst>
and once we get the pointer back, we can just use from_raw
<karolherbst>
so this way the Arc survives, it's just not directly reachable
<karolherbst>
doesn't matter though, as the client will either use release* or do something meaningful
<karolherbst>
when it does something meaningful, we just create a new Arc out of the pointer
<karolherbst>
in release, we can juse use decrement_strong_count on the pointer as it will tidy up the Arc when the last ref is dropped
<karolherbst>
no need for Arc::from_raw
<jekstrand>
Right, but how do we report reference counts?
<jekstrand>
That'll conflate ours and the client's
aswar002_ has joined #dri-devel
<karolherbst>
Arc::from_raw(ptr).strong_count()
* jekstrand
is confused
<karolherbst>
and then into_raw to leak it again
<karolherbst>
jekstrand: we just have a single Arc
<karolherbst>
and put everything in id
<karolherbst>
*it
<jekstrand>
Sure, but then DEVICE_REFERENCE_COUNT will include both our references and the client's
<karolherbst>
no wrapping at all
<karolherbst>
jekstrand: sure, and?
<karolherbst>
that's totally legal
<jekstrand>
Is it?
ramaling has joined #dri-devel
<karolherbst>
yeah
<karolherbst>
the client could just drop more refs than it's allowed to
<jekstrand>
Then why were we ever doing this nonsense? :-P
<alyssa>
jekstrand: how I feel about OpenCL when both GL and VK have compute
<karolherbst>
because I thought the alternative would have been to keep pointers as refs and other idiotic things
<karolherbst>
alyssa: 64 bit pointers
<alyssa>
karolherbst: are they a good thing?
<jekstrand>
alyssa: You don't care. Not now, anyway. You might care when I start submitting panfrost compiler patches, though. :P
<karolherbst>
do you need more than 4GB of data?
<alyssa>
jekstrand: I'm waiting ;'D
pzanoni has quit [Ping timeout: 480 seconds]
mbrost has quit [Ping timeout: 480 seconds]
aswar002 has quit [Ping timeout: 480 seconds]
ramaling_ has quit [Ping timeout: 480 seconds]
<alyssa>
karolherbst: I think every Mali machine I have has 4GB (or less) of RAM
<karolherbst>
jekstrand: the thing is, that CL relies on the dispatch table at a certain place, so if we don't wrap, the layout is always C... and other reasons
<karolherbst>
but maybe all of that would just be totally fine
<airlied>
I don't think thats legal
<karolherbst>
airlied: what?
<airlied>
the reference count thing
pzanoni has joined #dri-devel
<karolherbst>
to report more or the dirty Arc hacking?
<airlied>
the spec says it increments for retain and decrements for release
<airlied>
reporting more
<karolherbst>
airlied: the spec allows it
<airlied>
it really doesn't
<airlied>
"Reference Count: The life span of an OpenCL object is determined by its reference count—an
<airlied>
internal count of the number of references to the object. When you create an object in OpenCL,
<airlied>
its reference count is set to one. Subsequent calls to the appropriate retain API (such as
<airlied>
clRetainContext, clRetainCommandQueue) increment the reference count. Calls to
<airlied>
the appropriate release API (such as clReleaseContext, clReleaseCommandQueue)
<airlied>
decrement the reference count. After the reference count reaches zero, the object’s resources are
<airlied>
deallocated by OpenCL."
<airlied>
it has no indication the reference count can't change outside of calling those APIs
<jekstrand>
Yeah, I'm pretty sure we need to keep both counts
<karolherbst>
"Implementations may also modify the reference count" ...
<airlied>
where's that written, my CL 1.2 spec is failing me, maybe it's new
<karolherbst>
I have the CL 3.0 one
<karolherbst>
Implementations may also modify the reference count, e.g. to track attached objects or to ensure
<karolherbst>
correct operation of in-progress or scheduled activities. The object becomes inaccessible to host
<karolherbst>
code when the number of release operations performed matches the number of retain
<karolherbst>
operations plus the allocation of the object. At this point the reference count may be zero but
<karolherbst>
this is not guaranteed.
<airlied>
so you still have to keep track
<karolherbst>
I guess so
<airlied>
of release/retain counts
<karolherbst>
yeah, but to protect from stupid clients, sure
<airlied>
to make it inaccessible
<airlied>
usually for these things you have an object and a handle
<karolherbst>
mhh, yeah
<airlied>
and the handle holds a reference to the object and is what the API sees
<airlied>
the internal object has it's own reference count then
<karolherbst>
yeah sure, that's how we do it atm
<airlied>
so the handle can go away while the internal object keeps existing
<jekstrand>
Taking a step back.... Are there cases where an object can hit 0 from the API POV and then get exposed to the client again?
<jekstrand>
It sounded like maybe programs can?
<karolherbst>
airlied: the thing is.. the CL api leaks the external object even though it's already dropped by the client
<karolherbst>
jekstrand: yes
<karolherbst>
drop cl_program, and you can get it back via a cl_kernel one
<jekstrand>
karolherbst: Which call?
<karolherbst>
getKernelInfo
<karolherbst>
with CL_KERNEL_PROGRAM
<karolherbst>
all those *Info calls have that for various objects
<jekstrand>
karolherbst: So that's supposed to increment the reference count?
ybogdano has quit [Read error: Connection reset by peer]
<karolherbst>
nope
<karolherbst>
it's not
<karolherbst>
stays 0 if it's 0
<jekstrand>
Then it's not accessible to the host code
<karolherbst>
sure, but it still gets the pointer
<karolherbst>
and the object has to be valid
<karolherbst>
because we have to verify that stuff I guess?
<karolherbst>
how should the client know that this particular object was already released by it?
<karolherbst>
(well it should know, but if they know why is there that *Info API to get those objects in the first place anyway)
* jekstrand
feels a spec bug coming on
<karolherbst>
yeah..... that's why they talk about internal refs
<karolherbst>
so we hold 1 internal ref
<karolherbst>
and the object stays alive
<karolherbst>
until the kernel gets dropped and we drop the ref to the program
<karolherbst>
and everybody is happy
<jekstrand>
I don't think that's why they talk about internal refs...
<airlied>
a cl_kernel should hold a cl_program reference though
<karolherbst>
yeah
<airlied>
internally at least
<karolherbst>
yeah, we might not want to expose the _full_ ref count
<karolherbst>
but the spec allows us to
<karolherbst>
if the spec allows us to allow more releases than creation+retains... that's a different topic
<airlied>
"The program object is deleted after all kernel objects
<airlied>
associated with program have been deleted"
<jekstrand>
Yes
<jekstrand>
SO we're supposed to hold an internal reference
<karolherbst>
yep, so we have to make sure that the client doesn't trash ref counting
<airlied>
but yeah I see the gap there, guess you have to hold onto the handle for that lifetime as well
<jekstrand>
I'm starting to think that we either want to YOLO it and just use an Arc
<airlied>
so that even when it calls Release, if it gets it back via the Get api it call retain on it