Precisely, CONFLICTS_INSTALL for x11/nvidia-secondary-driver* can be removed, too, for main branch.

Simplify CONFLICTS_INSTALL lines.
While here, add missing CONFLICT_INSTALL for x11/linux-nvidia-libs.

Should I split out line 7 and 11 of graphics/nvidia-drm-kmod/Makefile.common, which renames drm-*-kmod part of distfiles to match graphics/drm-*-kmod, to be a separate review and make this (D50282) to depend on it?

In D50282#1149188, @manu wrote:

I'm ok for this but keep in mind that we will still need to bump PORTREVISION on nvidia kmod when we update drm-*-kmod so I'm not sure it's worth doing all that tbh

Rebase after commit 940ca091de52.
Keeping concept until manu@ pops in as the de-facto maintainer of graphics/drm-*-kmod.

Thanks for the idea!

Rebase after commit 3adb70282914 and commit 8495963ac01c.
Stopped bumping PORTREVISION as pointed out by kbowling@.

Stop updating PORTREVISION as pointed out by arrowd@.

Why bump PORTREVISIONs? There should be no visible changes in the resulting package.

Rebase after upgrading to 570.144.
Split out auto-distinfo part (for graphics/nvidia-drm*-kmod).

I'm fine with splitting out "auto-merged distinfo" part for graphics/nvidia-drm-*-kmod into another review.
But as it strongly depends on the remaining part of this review (splitting distinfos for legacy versions), I've opened this as a "merged" one.
Putting "Depends on" line pointing to this review in the summary newly splitted out one does the right thing, right?
Or should I wait for the remaining-here part in this review to be approved and landed prior to opening another review?

Some notes for the future:
If new legacy branch is created upstream from pre-570.144, we would need to add back conditionals for it.
And once we add -devel for New Feature Branch of drivers (to support cutting edge GPUs), we would or wouldn't need to add conditionals (depending on whether the additions/upgrades are trivial or not).

This is basically a refactor to ease maintainances.

Rebase after commit 637e9e68f1a2ce587e5107b0fd296abd16f061a0 "x11/nvidia-driver: Fix too aggressive disabling of GSP firmware".

Because egl-wayland can be installed separately I think that's a good case for why keeping intermediate versions around is not worth it.

For x11/nvidia-driver, yes.

Turned out that use-cases requiring intermediate versions exists.

https://forums.freebsd.org/threads/how-to-install-the-nvidia-drm-kmod-for-the-driver-nvidia-525-78-01-on-freebsd-14-2-to-be-able-to-run-comfyui.97741/

Pending rebase after D50048 landed as commit 13636d8b58f662e12d1513333d1e981a59620109 for now, as D50053 should land first. Would rebase after it lands.

Rebase after D50048 landed as commit 13636d8b58f662e12d1513333d1e981a59620109.

If you feel the value 1 for hw.nvidia.registry.EnableGpuFirmware is OK, please commit this.
(I cannot test whether 17 works for anyone affected or not, as I don't have GPUs with GSP.)

Even though I've left inline comment about PORTREVISION, others looks good to me.
So accepting now.
Modifying PORTREVISION to match reality at the time committed shouldn't matter,
but if you want on commit, I'll accept this again ASAP.

Thanks!
Macro is good to ease and clarify codes "for anyone already thoroughy know it", but for anyone others, it can easily become kinda... hell to track. ;-) So I've left my previous comment for records.

For the record:
Some findings on tracking macros handling registry hw.nvidia.registry.EnableGpuFirmware and the reason why I've chosen this way.

Re-title.
Fix to allow overriding hw.nvidia.registry.EnableGpuFirmware via /boot/loader.conf again, which stopped working after D49828 landed.

In D50053#1141815, @kbowling wrote:

In D50053#1141698, @junchoon_dec.sakura.ne.jp wrote:

In D50053#1141632, @kbowling wrote:

In D50053#1141606, @ashafer wrote:

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.

I agree with this recommendation. This should be a tunable kind of thing since we need package users to be able to try both settings.

So OPTIONify is not needed, unless any case which need global disabling of GSP.
If such a case exists, keeping OPTIONifying but enable by default, with flipping the default for the tunable.
3 entries of OPTIONS_SINGLE would be wanted in that case.

Enable GSP firmware support
Disable GSP firmware support via tunable (default)

and

Globally disable GSP firmware support

to avoid confusions.

If not, simply changing patch to flip the tunable and edit pkg-message (actually, files/pkg-message.in) to tell anyone having problem with the ports default to flip it via tunable is sufficient.
In this case, if we can't edit summary of this revier, I need to abandon this and open a new review. Not sure at this moment.

I don't think the option is worth carrying. Instead, editing extra-gsp-patch-src-nvidia_subr.c (or whatever file instead) to provide the correct tunable) is the proposal from @ashafer.

We will hope that the default can become durable and do the right thing almost all the time, but it still seems like tricky business with all the cards and driver right now.

In D50053#1141632, @kbowling wrote:

In D50053#1141606, @ashafer wrote:

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.

I agree with this recommendation. This should be a tunable kind of thing since we need package users to be able to try both settings.

Looking into x11/nvidia-driver/Makefile and x11/linux-nvidia-libs/Makefile thoroughly, there seems to be no more candidates for conditionals to be pruned, if I'm not overlooking something.

On headsup for pruning intermediate versions, we should clarify:

*Keeping conditionals only the top of master port and each slave (legacy, currently) ports
  and initial version of each conditionals as a record, or

Rebase to chase graphics/drm-[61|66]-kmod upgrades.

In D49982#1140140, @jkim wrote:

In D49982#1140088, @junchoon_dec.sakura.ne.jp wrote:

In D49982#1140083, @ziaee wrote:

Can we have an updated version string in the dmesg? On current with nvidia-drm-66-kmod, I get:

Initialized nvidia-drm 0.0.0 20160202 for nvidia0 on minor 1

Cannot find related codes in nvidia side of src in quick glance, but found in drm-*-kmod part.
(I'm working on stable/14, so cannot test graphics/*drm-66-kmod. But string in dmesg looks same.

It is defined in the NVIDIA driver, i.e., src/nvidia-drm/nvidia-drm-drv.c. Just search 20160202.

Related discussions at Bug 285803, starting from comment 7.

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=285803#c7

In D49982#1140083, @ziaee wrote:

Can we have an updated version string in the dmesg? On current with nvidia-drm-66-kmod, I get:

Initialized nvidia-drm 0.0.0 20160202 for nvidia0 on minor 1

Fixed PORTREVISION in graphics/nvidia-drm-kmod/Makefile.common.

Anyway, whichever builds fine. So accepted.

Looks good to me.
Built/packaged both on bare-metal and poudriere, installed and ran fine using pkg built with poudriere.
Tested on stable/14, amd64 at commit f9afcbff02a230af85e646ef3ae166ae61b04ca1.

Looks good to me.
Built/packaged both on bare-metal and poudriere, installed and ran fine using pkg built with poudriere.
Tested on stable/14, amd64 at commit f9afcbff02a230af85e646ef3ae166ae61b04ca1.

Thanks!

Reset PORTREVISION to 0 for x11/nvidia-driver-390.

A bit too late, but shouldn't graphics/nvidia-drm-kmod/Makefile.common also needed to be handled?
This file is included from graphics/nvidia-drm-*-kmod/Makefile and having MAINTAINER= [email protected] line in it.

Forgot to mention in my previous post.
Wouldn't PORTREVISION bump needed for this update to be included in official pkg?

Why I've asked whether this should be MFH'ed or not is just because now is the timing almost just after 2025Q2 is branched. Usually for this kind of non-security upgrade, me, too, prefer not MFH'ing.

This would be worth mentioned in Chapter 5 and Chapter 6 of the Handbook, too, for whom bitten but doesn't notice pkg-messages displayed on install/upgrade.

Confirmed landed. Thanks!
BTW, would this be worth MFC'ed to 2025Q2? It branched before this landed.

In D49245#1131660, @ziaee wrote:

I don't know what to do here sir, I offered my help making it right. As a doc maintainer, being firm about the doc is my responsibility, and if I'm not going to do it, I should step down.

Thanks!

In preparation for review D47742 "kernel linker: Disable local sym
resolution by default" to be landed, commits

f2cc91f4b56eaa5da41a712b3c42be874161c1c9
  (for graphics/[nvidia-]drm-515-kmod)
8b4b66d94b8ff86c1c764094c38273313dd5f2d3
  (for graphics/[nvidia-]drm-61-kmod)
f7f46d1b58279326de0760ce1fee71631af40cae
  (for graphics/drm-66-kmod)

and

e3d1e7c048d0c909e3f67bc4e2dba558f4c17781
  (for graphics/nvidia-drm-66-kmod)

are landed.

Expanded comment about the introduced workaround for X11 in graphics/nvidia-drm-kmod/Makefile.common as per request from ashafer. No changes for other parts.

Uploaded updated patch (rev6) using option e) at Bug 282312.

Found the root cause in nvidia codes. No need to touch upstream graphics/drm-[515|61]-kmod side.

Intermediate results of option b).
Look for the offending lines after make patch for graphics/nvidia-drm-61-kmod without the workaround by

I've never dug into upstream repo of drm-kmod before, so possibly I'm overlooking or mis-understanding, but tag name drm_v5.10.163_7 (specified by graphics/drm-510-kmod/Makefile.version) still has linuxkpi/dummy/include, so it wouldn't be affected.
So, our options would be:

a) Commit rev5 of the patch at Bug 282312 as-is, and once upstream addressed this issue,
   revert graphics/nvidia-drm-kmod/Makefile.common part of the patch and bump portrevision.

A possibility is that all fixes in drm-kmod upstream are anywhere nvidia-drm-*-kmod builds and left some more that nvidia-drm-*-kmod build picks. The fixes in upstream seems to remove "CFLAGS+= -I${.CURDIR:H:H}/linuxkpi/dummy/include" lines from Makefile here and there, according to commit logs.

In D47290#1078612, @kbowling wrote:

In D47290#1078609, @junchoon_dec.sakura.ne.jp wrote:

In D47290#1078596, @kbowling wrote:

@ashafer_badland.io @manu it seems like there might still be some remaining in drm-kmod, this is still failing for me and @junchoon_dec.sakura.ne.jp

@kbowling, does it still fail with rev5 of the patch at Bug 282312?
It includes workaround in graphics/nvidia-drm-kmod/Makefile.common, obtained from (with a change of a variable to fit with this) freebsd-current ML by Benjamin Jacobs. And it doesn't fail on stable/14 (still clang18), so something changed in clang19 on main (any of new default option, maybe) is causing the issue.

https://lists.freebsd.org/archives/freebsd-current/2024-October/006558.html

No your workaround is good but I think the path needs to be dropped in drm-kmod

In D47290#1078596, @kbowling wrote:

@ashafer_badland.io @manu it seems like there might still be some remaining in drm-kmod, this is still failing for me and @junchoon_dec.sakura.ne.jp

Updated my patch at Bug 282312 as rev3, chasing @manu's commit, for further reviews.
And this review (D47290) can now be closed as of:

Looks good to me.
Once this lands, I'll update my patch at Bug 282312 for further reviews.

Thanks! Now things got clear enough for me.

Tested on

stable/14, amd64 at commit 04a191c251e5ef20deb14fcdcf628a589be5216a

and

main, amd64 at commit 40d951bc5932deb87635f5c1780a6706d0c7c012.

For ports, overriding DISTVERSION with 555.42.02.

If I understand correctly, setting BUILD_DEPENDS alone makes the dependency not to be automatically installed by pkg, including packages which are locally built with poudriere or any other clean-room builders.
Legacy builders like portupgrade should not be affected, though, as it needs BUILD_DEPENDS to be installed first to proceed builds.

Is there any reason to disallow nvidia-drm-61-kmod for whole stable/14 other than that graphics/drm-61-kmod disallows them?
If not, stable/14 having ${OSVERSION} >= 1400508 is now stated to be supported by graphics/drm-61-kmod starting from commit e04b01217828bf06d36a02ad8e69dbb54c30b607 [1].

I should note that anyone using default LLVM (except for powerpc) as external toolchain would be bitten. (Not me, though.)
Default LLVM on ports is still stuck on devel/llvm90 (devel/llvm10 for powerpc only) according to Mk/bsd.default-versions.mk.
Maybe now would be time to switch default LLVM to devel/llvm10 or later (devel/llvm13, which is required by mesa?).

In D35552#807231, @hselasky wrote:

Include all chunks.

In D35552#806924, @hselasky wrote:

Keep trailing space on last line.

Looks and works OK for old (0x100 protocol) ThinkPad.
No new regression on my T420. (Known-not-working and dangerous functions like suspend/resume are untested.)
Not marking as "Accepted" as I don't have newer ThinkPads having 0x200 protocol and no knowledges for 0x200 protocol.
So I just reviewed and tested fallback (to 0x100 protocol) codes.

In D7538#273971, @allanjude wrote:

In D7538#273970, @karl_denninger.net wrote:

I'll try to get some time allocated to that - right now the machine that
I use for this effort is busy doing other things but I may be able to
free up some time on it in the next week or two.

Is there any intent to backport/MFC those changes into 11-STABLE?  That
I can probably test and adapt the code to more-quickly.

The needfree change is a good one, it's just that having it confined to
-HEAD causes problems for me right now due to "free computer of
appropriate configuration" constraints.

Thanks in advance.

• smime.p7s4 KBDownload

I would expect the latest ZFS code to be merged from -current to stable/11 in time for 11.2-RELEASE

Just a heads-up.
r325851 broke this (including Karl's updated patch at bug 187594).
"needfree" in arc.c has gone.

Confirmed that the latest patch Karl uploaded on Bugzilla bug 187594 [1] (merged his patch with the one here) is...

In D7538#260193, @karl_denninger.net wrote:

Being new to phabricator.... how do I upload a diff that is off a (probably) different base rev and has my changes in it (I don't really want to "update" the existing diff, or do I?)

Confirmed. Thanks!
Tested on stable/11 at r304717, amd64, ports tree is at r420859.

Build, install and run: 367.35
Build only: 364.19, 361.45.11, 361.42, 361.28, 358.16, 355.11, 352.79,
            352.55, 352.41, 352.30, 352.21, 349.16, 346.72, 346.59

Now it builds fine and running for stable/11 (r304528) and head (r304535), both amd64.
Thanks!