<javierm>
tzimmermann: I should probably post a patch to make all DRM drivers to honour the nomodeset param, I'll do that today
rasterman has joined #dri-devel
<tzimmermann>
javierm, better make a patch set with a patch for each driver
<tzimmermann>
and cc the rsp driver maintainers
<tzimmermann>
this kind of change usually takes several weeks until you got all r-bs/a-bs
<danvet>
tzimmermann, do we have a specific excuse?
<tzimmermann>
danvet, i'd like to backmerge into drm-misc-next. for fixes and specifically for 96c5f82ef0a1 ("drm/vc4: fix error code in vc4_create_object()")
<javierm>
tzimmermann: Ok, I was torn on doing it as a patch-set because it felt that would be a lot of churn for trivial changes. But agree with you that's better that way
<tzimmermann>
it conflicted with drm-misc-next and required a fixup in drm-tip. it would be nice to have this fixes
<javierm>
tzimmermann: also each DRM driver maintainer could review and merge it as their convenience
<tzimmermann>
javierm, not all drivers are developed at the same pace. sometimes you have to (repeataedly) ping people about the patch
rgallaispou has joined #dri-devel
<javierm>
tzimmermann: right
<danvet>
tzimmermann, ok will do
<tzimmermann>
i usually tend to merge such patches once i have an ack or review. so the patchset keeps shrinking over time :)
<javierm>
tzimmermann: Ok. It will be a lot of patches since when I did the nomodeset cleanup notice that only a few drivers check for that param
<tzimmermann>
danvet, thank you
<javierm>
*noticed
<tzimmermann>
and for the final 3 patches, you can tell the reviewers to hurry up; they are late. ;)
<javierm>
:)
<danvet>
hm my git has some really screwed up rerere here
<tzimmermann>
maxime, javierm, for that vc4 patch, what exactly is v3d for? 3d acceleration?
<tzimmermann>
mripard ^
<danvet>
yes
<javierm>
tzimmermann: that's my understanding. So I guess GLES should work with simpledrm + v3d driver ?
<danvet>
on anything than og rpi the 3d block is driven by v3d, display is still vc4
<danvet>
tzimmermann, did you do the conflict resolution in vc4 display code?
<tzimmermann>
danvet, javierm, i see
<tzimmermann>
javierm, i think i have the same concern as you do. IMHO nomodeset should disable all of the native driver
mszyprow_ has quit [Ping timeout: 480 seconds]
<javierm>
tzimmermann: yeah, that's my opinion. And also consistent with what all drivers that currently check for nomodeset do AFAICT
<HdkR>
/win 14
<HdkR>
...
<danvet>
tzimmermann, mripard not sure who's done the vc4 conflict resolution in drm-tip, but it was buggy
<danvet>
a hunk in commit_tail got moved around
<danvet>
unfortunately searching for butchered commits in drm-rerere is very hard
<tzimmermann>
danvet, there was no conflict IIRC. it was silently merged by dim scripts. i later added a fixup for the vc4 driver
<danvet>
tzimmermann, yeah I found it, it was mripard
<tzimmermann>
so the conflict happend on the side of drm-misc-fixes
<danvet>
mripard, ^^ might be good to test drive drm-tip occasionally to prevent getting funny resolutions landing anywhere
<danvet>
javierm, yeah I'm way behind on everything since weeks :-/
<danvet>
and I guess jani didn't volunteer either
<tzimmermann>
i use drm-tip and didn't notice anything strange until dan carpenter complains days later
<danvet>
tzimmermann, yeah I think this was a different one
<tzimmermann>
ok
<javierm>
danvet: Ok, sorry for bothering you then
<danvet>
anyway I'm happy with the result, compile checking now, I'll ping you&mripard
<danvet>
tzimmermann, maybe hold off with further backmerging until mripard confirmed I didn't screw it up :-)
<javierm>
tzimmermann: if my grep fu is correct, 62 DRM drivers currently don't check for nomodeset
<javierm>
tzimmermann: I'll leave vc4 and v3d to mripard, so it will be a 60 patch series :)
boistordu_ex has quit [Remote host closed the connection]
<danvet>
javierm, looked and dropped a minor bikeshed
tursulin has joined #dri-devel
boistordu has joined #dri-devel
vivijim has joined #dri-devel
aravind has quit [Ping timeout: 480 seconds]
<tzimmermann>
danvet, i'll do nothing until i hear from you
<javierm>
danvet: thanks! I didn't receive your reply yet
aravind has joined #dri-devel
<javierm>
tzimmermann: agree on the -ENODEV, I just suggested -EINVAL because that's what the current drivers do. But I'll use the former for the drivers that are missing it
<tzimmermann>
javierm, 60 patches? lol
<tzimmermann>
the current drivers get the returned value wrong. i think EINVAL is just the go-to error for anything
<javierm>
tzimmermann: agreed
tightshoes has joined #dri-devel
<tzimmermann>
BTW, do we have a way of un-setting nomodeset at runtime?
<danvet>
javierm, forgot to hit send :-)
<javierm>
danvet: ah, that explains :)
<javierm>
tzimmermann: we don't but some drivers have a module param that overrides it
<danvet>
tzimmermann, module parameters are in sysfs, but not sure about the normal ones ...
<javierm>
which is the reason why we can't have this logic in the core
<javierm>
danvet: this is not a module parameter... because it's global
<danvet>
javierm, I thought you're adding a drm one?
<javierm>
danvet: you said that it wasn't worth it IIRC
<tzimmermann>
javierm, but it's enabled by a config option and tied to the drm module (?)
tightshoes has quit []
<tzimmermann>
i forgot why exactly we needed the config option
<javierm>
tzimmermann: yeah, but still using it uses the __setup() macro instead of a module_param()
<javierm>
if we use the latter, then it will be drm.nomodeset and danvet said that every blog post on the internet mentions nomodeset without a namespace
<danvet>
javierm, yeah we can't drop nomodeset, but if we also have drm.nomodeset that would cover the use case of disabling it later on
<danvet>
if you make drm.nomodeset a tri-state
<javierm>
danvet: can we find a better name for the drm namespaced one :)
<tzimmermann>
if anything, i'd like to have a kernel-wide parameter selects the graphics output (e.g., text, firmware, native)
<danvet>
mripard, I just pushed a backmerge to drm-next, pls double-check vc4_kms.c and then tell tzimmermann to go ahead
<danvet>
airlied, ^^ fyi
<javierm>
tzimmermann: that sounds like a good idea. Let's make all DRM drivers to check for drm_firmware_drivers_only() first and then we can think how to improve that
<tzimmermann>
javierm, agreed
<javierm>
ideally all this should be handled by the core as we discussed and add a flag to simpledrm (and maybe others) to not be disabled when gfx_output=firmware
<tzimmermann>
once we have drm_firmware_only() in all drivers, we can fix the exact semantics behind the scenes. drivers shouldn't care about this
<javierm>
tzimmermann: yeah
pjakobsson_ is now known as pjakobsson
<javierm>
danvet: "who love pain too much" xD
<javierm>
agreed with your point, I'll post a v2 later
pcercuei has joined #dri-devel
sarnex has quit [Remote host closed the connection]
sarnex has joined #dri-devel
gawin has joined #dri-devel
pnowack has joined #dri-devel
OftenTimeConsuming has quit [Remote host closed the connection]
<karolherbst>
but it does make programming stuff like that so nice :D
mszyprow_ has joined #dri-devel
xantoz has joined #dri-devel
anarsoul has quit [Remote host closed the connection]
anarsoul has joined #dri-devel
mlankhorst has joined #dri-devel
mlankhorst has quit [Remote host closed the connection]
mlankhorst has joined #dri-devel
mal has joined #dri-devel
iive has joined #dri-devel
<jekstrand>
alyssa: Yeah, no idea how much it matters. Probably some.
<jekstrand>
Where "some" may be epsilon
rasterman has quit [Remote host closed the connection]
rasterman has joined #dri-devel
OftenTimeConsuming has joined #dri-devel
<alyssa>
jekstrand: Alas
<jekstrand>
IIRC, we have a fixed point size mode too
<jekstrand>
We only use it to force 1.0 if the shdaer doesn't specify.
<jekstrand>
It would be easy enough to have a NIR pass to scrape it out and stuff it in the shader_info
weekend has joined #dri-devel
rgallaispou has quit [Remote host closed the connection]
* alyssa
nods
<alyssa>
Bifrost/IDVS only support the fixed point size mode, and a lot of dEQP tests write gl_PointSize = 1.0 /even for triangles/
<jekstrand>
ouch
<jekstrand>
Yeah, that'll matter
<alyssa>
So we hit non-IDVS fallback for a lot of dEQP on bifrost
<alyssa>
whether that's meaningful idk
<jekstrand>
skia seems to like gl_PointSize = 1.0
<jekstrand>
I'm seeing 4 games in shader-db that do as well, all 1.0
<jekstrand>
I wonder if it's sufficient to just delete gl_PointSize = 1.0?
mattrope has joined #dri-devel
<jekstrand>
Of course, manhattan sets gl_PointSize = 6.0.
<jekstrand>
So it's clearly worthwhile to optimize that case. (-:
<jekstrand>
Naturally, it's the only app in all of our shader-db that sets gl_PointSize to any constant other than 1.0
<daniels>
with advanced use like that, no wonder it's the benchmark of choice
<jekstrand>
Some ARM engineer probably convinced them to do that just so they could claim a perf improvement on their year-end review.
weekend has quit [Killed (dwfreed (No reason))]
<jekstrand>
alyssa: The good news is that, if we decide to do this, it's going from shader to state so it doesn't add variants.
<alyssa>
jekstrand: Yep.
<alyssa>
I am supposed to be studying for an exam right now
<alyssa>
Clearly that means I should write out the nir pass /s
jewins has joined #dri-devel
<ishitatsuyuki>
good luck (for exam? for nir pass? xD)
<alyssa>
ishitatsuyuki: hey
<alyssa>
if my nir passes so can I
* jekstrand
complains to twitter
camus has quit [Read error: Connection reset by peer]
camus has joined #dri-devel
imirkin_ has joined #dri-devel
<alyssa>
jekstrand: about?
Company has quit [Read error: Connection reset by peer]
<imirkin_>
jekstrand: you probably realize this, but in GLES the only way to set point size is via the shader
Company has joined #dri-devel
<imirkin_>
jekstrand: in desktop GL, there is the "is point size specified via shader or glPointSize" switch
<jekstrand>
imirkin_: No, I didn't realize that. Or if I did, I'd forgotton.
<alyssa>
imirkin_: => GLES apps set via shader even when the hardware could do a constant for cheaper
<jekstrand>
Vulkan doesn't let you set it not through the shader either
<imirkin_>
alyssa: correct. but the application doesn't have the option.
<alyssa>
right
<imirkin_>
so it's not the application's fault :)
<alyssa>
jekstrand: which means this is also an interesting opt for big GL on Zink on desktop hw
<jekstrand>
imirkin_: It is manhattan's fault that it wants 6.0 when no one else seems to care. :-P
<imirkin_>
jekstrand: it's worse than that ... iirc you _must_ set it if drawing points
<alyssa>
and can set it even if not drawing points.
<imirkin_>
a VS (or last-vertex-stage) that doesn't set gl_PointSize when drawing points leads to undefined results
<imirkin_>
(in GLES)
<imirkin_>
(or when the switch is flipped in desktop GL)
<imirkin_>
(since the hw is getting the point size from some built-in varying slot, which isn't being filled in by the shader)
<imirkin_>
(i like parentheses)
<alyssa>
(i do too, keep it up!)
<jekstrand>
Does anyone set PIPE_CAP_MIN_POINT_SIZE[_AA] to anything other than 1.0f?
<jekstrand>
Of course Zink does
<imirkin_>
jekstrand: radeonsi
<imirkin_>
radeonsi/si_get.c- return 1.0 / 8.0; /* due to the register field precision */
<jekstrand>
Ok, I think I've done enough reading to determine that we can use 0.0 in the shader_info point_size to indicate "don't know"
<jekstrand>
Or we could use NaN but that seems mean
<imirkin_>
huh?
<alyssa>
jekstrand: Or negative
<imirkin_>
oh. the shader_info struct.
<jekstrand>
Yup
<alyssa>
jekstrand: See lower_point_size_mov.c
<alyssa>
and also lower_point_size.c
<alyssa>
confusingly names
<jekstrand>
alyssa: I like -1.0
<alyssa>
I think lower_point_size uses 0.0
<alyssa>
jekstrand: "anything non-positive" is probably fine tbh
<alyssa>
jekstrand: Mean would be a negative signaling NaN :-p
<jekstrand>
Yeah, 0.0 seems pretty reasonable
<jekstrand>
Yeah, let's not do that. :)
<jekstrand>
I like 0.0 because then you can do if (nir->info.const_point_size)
<jekstrand>
Or we could default to 1.0
<jekstrand>
But then you can't detect it
rgallaispou has joined #dri-devel
<alyssa>
`if (nir->info.const_point_size > 0.0)` seems less fragile
<alyssa>
But I don't really care what colour the bikeshed is
<jekstrand>
Yup
<jekstrand>
In any case, 0 isn't a valid point size and that's what matters
Duke`` has joined #dri-devel
<jekstrand>
So this does seem like a worthwhile thing for panfrost. ANV could use it too though I doubt it's that important for us.
<alyssa>
I doubt it's that important for panfrost either but wow look union-find data structures!
<jekstrand>
If it lets you turn on IDVS....
<alyssa>
🤷
<jekstrand>
Union-find is a really cool data-structure
<alyssa>
It really is lol
<jekstrand>
I've been tempted to implement it and use it in nir_from_ssa
<alyssa>
what's the use case?
* alyssa
doesn't understand nir_from_ssa as she is not yet a black belt
<jekstrand>
I think it'd be ever so slightly faster than what we have now. But, also, I don't care enough.
<jekstrand>
One of the problems in out-of-ssa is figuring out the "phi webs". i.e. the connected components of the graph where the vertices are SSA values and the edges are "used in the same phi"
<alyssa>
Right, okay
<jekstrand>
A union-find (or disjoint set forest, if you prefer) is the perfect data structure for this.
<alyssa>
the alternative being building up the graph completely and then doing dfs?
<jekstrand>
When I first implemented out-of-SSA, I'd never heard of that data structure and couldn't come up with an efficient way of constructing the phi webs as per the original out-of-SSA paper so I found one that described the merge-sort thing we have today.
<jekstrand>
Actually, I sort-of reinvented union-find when thinking about how to create phi webs but I'd never heard of it and didn't know that, if contsructed properly, union operations are amortized constant time.
<jekstrand>
My top-of-head calculations said union operations were going to be worst-case O(n) which seemed worse than the merge sort thing.
<jekstrand>
I should go rewrite it one of these days but it's not like any compiler is out-of-SSA-bound so meh.
<jekstrand>
Anyway, union-find is really nifty and useful, at least for that particular problem. :)
mbrost has joined #dri-devel
eukara has quit []
rgallaispou has quit [Read error: Connection reset by peer]
eukara has joined #dri-devel
tzimmermann has quit [Quit: Leaving]
Bennett has joined #dri-devel
<graphitemaster>
<jekstrand> In any case, 0 isn't a valid point size and that's what matters
<graphitemaster>
I sure hope it's allowed in a shader though as a sort of culling technique :P
<graphitemaster>
Like writing gl_PointSize = 0 should work for that right
<graphitemaster>
> If the value written to gl_PointSize is less than or equal to zero, results are undefined.
<graphitemaster>
Okay so my engine has UB in it, thanks OpenGL
<imirkin_>
graphitemaster: if you want to cull things out, just set gl_Position.w <= 0
<graphitemaster>
Yeah, that works too. I just didn't think of it at the time..
<graphitemaster>
Still better than D3D I guess, which only allows you to have a point size of 1
<graphitemaster>
(Modern D3D I mean, the one that removes PSIZE semantic)
<alyssa>
graphitemaster: ...lol
<graphitemaster>
Look, rendering degenerate geometry is a time-honored tradition of computer graphics. The fact modern APIs are making it undefined really brings a tear to my eye.
<graphitemaster>
What's next. No more degenerate triangle strips?
<graphitemaster>
We all know they got rid of quads because not co-planar, yet having a quad not lie in a single plane was a very powerful feature of older quad renderers.
<graphitemaster>
How's that for a bikeshed discussion XD
aravind has quit [Ping timeout: 480 seconds]
pendingchaos has quit [Ping timeout: 480 seconds]
OftenTimeConsuming has quit [Remote host closed the connection]
<jekstrand>
graphitemaster: It's not. The spec explicitly says it gets clamped to the minimum value which is > 0
<imirkin_>
jekstrand: maybe the vk spec does
<imirkin_>
i don't think the GL spec has any such thing
<jekstrand>
imirkin_: GLES spec says:
<jekstrand>
"If the last vertex processing stage is a vertex shader, the point size is taken
<jekstrand>
from the shader built-in gl_PointSize written by the vertex shader, and is
<jekstrand>
"
<jekstrand>
clamped to the implementation-dependent point size range.
<imirkin_>
oh, huh
<imirkin_>
news to me :)
<imirkin_>
oh and yeah, but default ES only supports this from VS :)
<jekstrand>
In fact, there's even a NIR pass to insert a clamp because v3d doesn't clamp in HW.
<imirkin_>
there's a OES_something_cool to allow gl_PointSize in other stages
<alyssa>
panfrost too
<alyssa>
and maybe also asahi I can't remember all my drivers look the same
<imirkin_>
(that should have read 'by default' ... )
pendingchaos has joined #dri-devel
mszyprow_ has quit [Ping timeout: 480 seconds]
hikiko has quit [Ping timeout: 480 seconds]
imirkin_ has quit [Quit: Leaving]
gouchi has joined #dri-devel
lemonzest has quit [Quit: WeeChat 3.3]
jhli has joined #dri-devel
hikiko has joined #dri-devel
ybogdano has joined #dri-devel
camus1 has joined #dri-devel
ella-0 has joined #dri-devel
ella-0_ has quit [Read error: Connection reset by peer]
camus has quit [Ping timeout: 480 seconds]
mlankhorst has quit [Ping timeout: 480 seconds]
* jekstrand
can't GitLab today
* jekstrand
has now created two MRs from his jenkins_vulkan branch instead of the correct branch
<airlied>
can you retarget them without deletion, I thought you could
<ajax>
the ui for it isn't super obvious but yes you can
<jekstrand>
Can you? I've never figured it out.
<jekstrand>
Oh, well
<jenatali>
You can retarget the dest, not the source though. Or can you do that too?
<ajax>
definitely the destination. i think you can change the source if it's in your own repo? maybe?
<jenatali>
Looking at the "edit" page I only see a dropdown for changing the dest branch
<ajax>
nope. just the destination. makes sense i suppose
<airlied>
ah yeah dest is possible, maybe not sr
ybogdano has quit [Ping timeout: 480 seconds]
<alyssa>
gitlab doesn't support copy propagation? :-p
<jekstrand>
:P
OftenTimeConsuming has joined #dri-devel
OftenTimeConsuming has quit [Remote host closed the connection]
ybogdano has joined #dri-devel
<anarsoul>
gitlab is down?
<jekstrand>
aparently
<alyssa>
womp womp
<graphitemaster>
gitlab must've used log4j somewhere