Thanks, I think this seems reasonable now.

The Op want to use 525.78.01 for compatibility with a specific Linux application (here, ComfyUI) and it requires Linux version of libnvidia-egl-wayland.so.1.1.10

Thanks for updating. Sorry I had it on my todo list to look up where to do this for you, but seems you've figured it out anyway.

I think a better idea would be to adjust the patch I made for disabling GSP. That patch was pretty heavy handed and turned off a global which enabled GSP. I think instead we could make the patch default only the sysctl value to zero, disabling GSP by default but allowing users to enable it by setting the tunable in loader.conf. That would prevent us from having to have a build option and prevent users from having to compile things themselves.

@kbowling can you please confirm this is good for me to merge?

LGTM but shouldn't the other nvidia-drm-*-kmod ports also have this updated?

The suspend issue comes from nv_alias_pages not being implemented in our FreeBSD layer. Unfortunately when I resolve that there are still a few issues that GSP encounters when resuming, so I think the best course of action is to disable GSP by default until the issues are resolved.

Thanks, good catch on the revision. Bumped it now.

I don't think Fixes is a good fit for that but I did include the commit hash of the 570 bump to document the previous change.

You want a Fixes tag in this commit? Or are you saying in the future commit that removes this? I don't think putting a Fixes tag in this commit makes sense, this commit doesn't contain a fix it only documents a workaround.

The suspend/resume breakage was found after this commit, it wasn't something known before committing or else it would not have landed.

@grahamperrin so this "failed to allocate memory" issue happens when you are using displays connected through the HP display dock? Does it ever happen when you are just using the monitors directly connected to the NVIDIA card? Is this a laptop? Those docks have always given me all kinds of problems, the thinkpad dock I have likes to turn one particular monitor of mine off and on randomly.

Still LGTM, thanks

Looks good, thanks

Aside from the one note about the fbdev comment this looks good to me, thanks.

+mentor and also manu to double check everything looks compatible with the new drm-66-kmod.

So the stock MODULE_PNP_INFOs do handle structures such as that, I guess the problem is we don't use them for ACPI things. from the manpage:

static struct my_pciids {
               uint32_t devid;
               const char *desc;
       } my_ids[] = {
               { 0x12345678, "Foo bar" },
               { 0x9abcdef0, "Baz fizz" },
       };

Curious your opinions on this. Maybe I'm misreading the pnp code but it looks like we have been handing it the wrong format and it thinks it's a list of strings when it isn't.

No worries, thanks for submitting a fix.

I'm not in the wireless group but I suppose I could be. I also lurk in the kernel/desktop/gaming channels in the freebsd discord if you use that, feel free to ping or email me.

Sure, what IMPROVING file are you referring to? I'd be happy to test things, I'm trying to get a modern laptop setup on this lenovo legion laptop I have so it would be good to use rtw89 for that.

Huh I think I had seen the long idle disconnect thing earlier today but didn't realize that was what it was.

linuxkpi_ieee80211 has a function it calls to zero the next/prev fields called TAILQ_ELEM_INIT, so that it could do this check.

Thanks updated, does this look more clear?

What's preventing rtw89 from getting enabled in the build? With this fix I could comfortably test youtube, iperf3, ssh. Speed was ~20mbps but stable. I think it could make sense to open this up to more people.

Pushed here: https://github.com/amshafer/freebsd/commit/214a96bb38bf5534b59eb149367369a233fa6826

Pushed it here: https://github.com/amshafer/freebsd/tree/this_module

Added parentheses

Yes, thanks. I don't have a committer bit.

Thanks, updated to just remove the module_get() bits for now.

Updated with jhb's proposal. Also implemented module_get and module_put by casting to linker_file_t and bumping the refcount. Retested with PRIME offloading to confirm this still fixes the panic. Unlike the previous (similar) version this handles the statically compiled module case.

Any update on getting this merged?

I believe that would work and I agree it would be much simpler. @kib is there something the (b) variant provides that (a) would be missing if we went with @jhb's proposal? It seems like they provide the same functionality?

I guess the last question is what linuxkpi macro should actually create the global? I had previously used LKPI_DRIVER_MODULE, but not everywhere uses that. I guess we could make our own DEFINE_THIS_MODULE() macro and then call it from LKPI_DRIVER_MODULE, and add calls elsewhere as needed?

It seems the way it works is that THIS_MODULE is a NULL pointer during "builtin" (statically building modules into the kernel), and points to the struct module of the dynamically loaded module otherwise. I think we could do something similar where we get the current linuxkpi behavior (THIS_MODULE is null) if we aren't building a dynamically loaded module?

how would any of proposals handle LKPI modules statically compiled into the kernel?

Instead, it should be only resolved locally, causing undefined symbol error if attempt is made to reference it from kld without the module symbol (section ?).

So what is the expected use of THIS_MODULE in Linux? Determine that current module is not that module, or something else?

Updated with KLD_DPF debug statement.

Updated to be generic. I changed the name to __this_linker_file to better represent
what the variable points at. On linux __this_module points at a struct module, so
for us we point at a linker_file_t.

Updated with style, thanks.

There isn't a PR, this has been a longstanding issue with nvidia-drm that I'm
finally getting around to solving.

Thanks for all the suggestions! I've implemented it by doing the special
casing in linker_file_lookup_symbol_internal as suggested. It looks like
this does work, I can see different owner fields populated in nvidia-drm
and drm. Also confirmed multiple usages of THIS_MODULE in drm.ko point to
the same thing.

It sounds like linux has insmod set up the __this_module variable when a module is loaded. If you're asking about doing something equivalent then I don't have a good estimate for how hard that would be. I don't know the details though.

That's a promising idea. I wonder if instead of having an unusual inline function we could instead make it part of the build setup. Something like setting KBUILD_MODNAME_HASH to whatever sha1 -s "$KBUILD_MODNAME" returns as part of the Makefile? That seems like it might be a little cleaner and easier to debug. Only downside is we have to update all of the makefiles that use KBUILD_MODNAME, I can't think of a way to automate it off the top of my head.

Bump __FreeBSD_version for drm-kmod