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

Since there were no comments I've gone ahead with the strategy I
outlined in my last comment: Have drm-kmod create a global variable
during LKPI_DRIVER_MODULE and point THIS_MODULE at it.

Maybe an interesting solution would be to do something a bit more DRM specific using LKPI_DRIVER_MODULE. Afaict only the DRM drivers will run into the issue that motivated this change. There's no reason we couldn't do something like this in base or just move the bits I mentioned into base.

Yes sorry, bumped PORTREVISION

This has been tested by @grahamperrin

Moved BUILD_DEPENDS to before as suggested.

Updated to handle 13.2 by patching conftest.sh

Updated to add back in RUN_DEPENDS

In D44305#1010541, @arrowd wrote:

On FreeBSD 13 this results in
../common/inc/nv-mm.h:110:31: error: too many arguments to function call, expected 5, have 8

That's for struct class though, right? I'm talking about struct module, and struct file_operations. Here's an example usage from nvidia-drm: https://github.com/amshafer/nvidia-driver/blob/535.98/nvidia/src/nvidia-drm/nvidia-drm-gem.c#L156. Iirc there's a couple similar ones in other drivers in base.

I don't think so? The 550 build has been broken because of this so no actual version has been "released" for nvidia-drm-510-kmod with 550, so it doesn't seem like bumping the revision would do anything? I can if needed though

I'm open to other suggestions about how we can do this. We definitely want THIS_MODULE to be non-zero, but having it be an opaque pointer that isn't a struct module isn't that great. Barring fully implementing module in linuxkpi I think something like this could make sense to allow emulated code to check if devices/dmabufs/etc come from the same module?

I think you need to update to a more recent CURRENT, that was added at the end of december I think.

Odd I hadn't seen that warning. I removed the PORTVERSIONS and also reset PORTREVISION.

Update to add a pkg-message warning about the new modesetting sysctl

Rebase on latest main

Note that this shouldn't be merged until after PR 277028

Motivated by reports such as: https://github.com/amshafer/nvidia-driver/issues/21

It doesn't build, I got notified of this by a failing build: https://pkg-status.freebsd.org/beefy18/data/main-amd64-default/p384f6f077a9c_s1ff41cad71/logs/nvidia-drm-515-kmod-535.104.05_2.log

I know there's a lot of updates to the DRM infrastructure (terrific btw!) happening so feel free to ping me if there's anything you think might break nvidia-drm and you want me to test. I've verified 6.1-lts builds with nvidia-drm fine and I plan on testing that on my machines at some point soon.

This should fix recent package build failures. Tested building locally.

Off the top of my head I thought I tested with GENERIC but because that KASSERT didn't fire I'm assuming I had GENERIC-NODEBUG installed, which I normally do on half my machines.

haha that certainly looks a bit more complicated :)

Using PKGNAME lets me include the PORTREVISION in addition to the previous fields. That way it will look for tags such as nvidia-drm-515-kmod-535.98_1 instead of nvidia-drm-515-kmod-535.98. This seemed like the easiest way to do this.

Fixed styling

This is part of a fix for https://github.com/amshafer/nvidia-driver/issues/20

The code is still NVIDIA code, I've just modified it for compatability. The nvidia-drm-kmod port's Makefile is mine though, is that what you're referring to? I think the license of nvidia-drm-kmod should match the child ports to avoid anyone getting confused and thinking nvidia-drm.ko is BSD licensed.

Setting LICENSE_FILE= ${WRKSRC}/nvidia/doc/license.txt in Makefile.common should be fine for the child ports, but for the metaport I'm not sure the best way to include that? For nvidia-drm-kmod should I just copy that license file in the directory and reference that in LICENSE_FILE?

It's documented here: https://github.com/amshafer/nvidia-driver/blob/535.86.05/UPDATING.md

One thing of interest to reviewers here: how should we (meaning me) best coordinate matching updates of nvidia-drm-kmod with updates to nvidia-driver? nvidia-drm-kmod isn't as simple as bumping a version number since I have to populate a driver branch in github for a particular version.

Fixed 515 to actually be 515, reworked RUN_DEPENDS, I think I got the pre/post.mk stuff you recommended correct but let me know if not.

I originally added NVIDIA_DISTVERSION since we will need to have multiple nvidia-drm-kmod ports which match the different versions of drm-kmod. For now basically nvidia-drm-510-kmod and nvidia-drm-515-kmod. It seemed easiest to have one Makefile.version in nvidia-driver which defines the version number to avoid having to update it by hand in all the ports that depend on nvidia-drm, instead you'd only have to update the distinfos. I added something similar to drm-kmod a bit ago. My goal was make one common nvidia-drm-kmod Makefile.common which uses NVIDIA_DISTVERSION, NVIDIA_GH_TAGNAME, and DRM_KMOD_GH_TAGNAME to select the versions of everything, and each nvidia-drm-*-kmod port imports the Makefile.versions it wants to reference.

Bump version to 535.86.05