Stuart Henderson <stu@spacehopper.org> writes:

> On 2024/12/28 20:34, Rane Hebden wrote:
>> Jonathan Gray <jsg@jsg.id.au> writes:
>> 
>> > Removing pci ids is the wrong approach.  Having a match function
>> > return higher would be enough.
>> 
>> I removed them as the #define in question also populates the patterns
>> used by fw_update. With the PCI IDs being retained, both the radeon and
>> amdgpu firmware would be installed automatically by fw_update for cards
>> supported by both drivers. Additionally, I wasn't sure there was benefit to
>> retaining the PCI IDs if the driver would not be used for these PCI IDs
>> with the patch in place, so erred on the side of removing redundant code
>> (redundant in the context of the patch).
>
> installing additional firmware isn't really a problem. and if both are
> present (if this change is made), you could switch between the two
> drivers by disabling amdgpu via ukc or bsd.re-config.

Thank you for this guidance, it's very helpful. Thanks to both of your
feedback, I think we have a good approach worked out.

I've attached an updated (and smaller!) diff, which will:
 - Enable building the bits for amdgpu to support CIK/SI chipsets
 - Retain radeon as the default for these chips
 - Allow installation of amdgpu and radeon firmware only for the chips
   supported by both drivers
 - Enable users to switch to amdgpu if they have a supported chipset
   by adding 'disable radeon' to /etc/bsd.re-config or via other
   configuration editing approaches

This way we provide a sensible/safe default (radeon, which has been
tested more thoroughly by sheer time as the default) but allow users
to use amdgpu instead if they (like me) find it more stable or
performent on their card. This also enables a really easy switch to
amdgpu by default for these cards in the future when/if there is enough
feedback and testing from users, as we can just make amdgpu a higher
priority driver match.

I'll also submit a xenocara patch to update the documentation for
amdgpu, detailing how to enable this support. The current manpage
suggests the driver only supports cards newer than the SI family of
chips, so I think an additional section to detail enabling this support
for CIK/SI makes sense.

Index: dev/pci/drm/files.drm
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/files.drm,v
diff -u -p -u -p -r1.63 files.drm
--- dev/pci/drm/files.drm	11 Apr 2024 03:40:05 -0000	1.63
+++ dev/pci/drm/files.drm	28 Dec 2024 20:50:13 -0000
@@ -632,7 +632,7 @@ file	dev/pci/drm/amd/amdgpu/amdgpu_vce_v
 file	dev/pci/drm/amd/pm/legacy-dpm/amdgpu_kv_dpm.c	amdgpu_cik
 file	dev/pci/drm/amd/pm/legacy-dpm/amdgpu_kv_smc.c	amdgpu_cik
 
-device	amdgpu: drmbase, ttm, wsemuldisplaydev, rasops8, rasops32, firmload, i2cbus, i2c_bitbang, drm_sched
+device	amdgpu: drmbase, ttm, wsemuldisplaydev, rasops8, rasops32, firmload, i2cbus, i2c_bitbang, drm_sched, amdgpu_si, amd_dc_si, amdgpu_cik
 attach	amdgpu at pci
 #file	dev/pci/drm/amd/acp/acp_hw.c				amdgpu
 file	dev/pci/drm/amd/amdgpu/aldebaran.c			amdgpu
Index: dev/pci/drm/amd/amdgpu/amdgpu_devlist.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/amd/amdgpu/amdgpu_devlist.h,v
diff -u -p -u -p -r1.27 amdgpu_devlist.h
--- dev/pci/drm/amd/amdgpu/amdgpu_devlist.h	15 Nov 2024 04:26:33 -0000	1.27
+++ dev/pci/drm/amd/amdgpu/amdgpu_devlist.h	28 Dec 2024 20:50:13 -0000
@@ -1,4 +1,164 @@
 static const struct pci_matchid amdgpu_devices[] = {
+        /* kaveri */
+	{0x1002, 0x1304 },
+	{0x1002, 0x1305 },
+	{0x1002, 0x1306 },
+	{0x1002, 0x1307 },
+	{0x1002, 0x1309 },
+	{0x1002, 0x130A },
+	{0x1002, 0x130B },
+	{0x1002, 0x130C },
+	{0x1002, 0x130D },
+	{0x1002, 0x130E },
+	{0x1002, 0x130F },
+	{0x1002, 0x1310 },
+	{0x1002, 0x1311 },
+	{0x1002, 0x1312 },
+	{0x1002, 0x1313 },
+	{0x1002, 0x1315 },
+	{0x1002, 0x1316 },
+	{0x1002, 0x1317 },
+	{0x1002, 0x1318 },
+	{0x1002, 0x131B },
+	{0x1002, 0x131C },
+	{0x1002, 0x131D },
+	/* oland */
+	{0x1002, 0x6600 },
+	{0x1002, 0x6601 },
+	{0x1002, 0x6602 },
+	{0x1002, 0x6603 },
+	{0x1002, 0x6604 },
+	{0x1002, 0x6605 },
+	{0x1002, 0x6606 },
+	{0x1002, 0x6607 },
+	{0x1002, 0x6608 },
+	{0x1002, 0x6610 },
+	{0x1002, 0x6611 },
+	{0x1002, 0x6613 },
+	{0x1002, 0x6617 },
+	{0x1002, 0x6620 },
+	{0x1002, 0x6621 },
+	{0x1002, 0x6623 },
+	{0x1002, 0x6631 },
+	/* bonaire */
+	{0x1002, 0x6640 },
+	{0x1002, 0x6641 },
+	{0x1002, 0x6646 },
+	{0x1002, 0x6647 },
+	{0x1002, 0x6649 },
+	{0x1002, 0x6650 },
+	{0x1002, 0x6651 },
+	{0x1002, 0x6658 },
+	{0x1002, 0x665c },
+	{0x1002, 0x665d },
+	{0x1002, 0x665f },
+	/* hainan */
+	{0x1002, 0x6660 },
+	{0x1002, 0x6663 },
+	{0x1002, 0x6664 },
+	{0x1002, 0x6665 },
+	{0x1002, 0x6667 },
+	{0x1002, 0x666F },
+	/* tahiti */
+	{0x1002, 0x6780 },
+	{0x1002, 0x6784 },
+	{0x1002, 0x6788 },
+	{0x1002, 0x678A },
+	{0x1002, 0x6790 },
+	{0x1002, 0x6791 },
+	{0x1002, 0x6792 },
+	{0x1002, 0x6798 },
+	{0x1002, 0x6799 },
+	{0x1002, 0x679A },
+	{0x1002, 0x679B },
+	{0x1002, 0x679E },
+	{0x1002, 0x679F },
+	/* hawaii */
+	{0x1002, 0x67A0 },
+	{0x1002, 0x67A1 },
+	{0x1002, 0x67A2 },
+	{0x1002, 0x67A8 },
+	{0x1002, 0x67A9 },
+	{0x1002, 0x67AA },
+	{0x1002, 0x67B0 },
+	{0x1002, 0x67B1 },
+	{0x1002, 0x67B8 },
+	{0x1002, 0x67B9 },
+	{0x1002, 0x67BA },
+	{0x1002, 0x67BE },
+	/* pitcairn */
+	{0x1002, 0x6800 },
+	{0x1002, 0x6801 },
+	{0x1002, 0x6802 },
+	{0x1002, 0x6806 },
+	{0x1002, 0x6808 },
+	{0x1002, 0x6809 },
+	{0x1002, 0x6810 },
+	{0x1002, 0x6811 },
+	{0x1002, 0x6816 },
+	{0x1002, 0x6817 },
+	{0x1002, 0x6818 },
+	{0x1002, 0x6819 },
+	{0x1002, 0x684C },
+	/* verde */
+	{0x1002, 0x6820 },
+	{0x1002, 0x6821 },
+	{0x1002, 0x6822 },
+	{0x1002, 0x6823 },
+	{0x1002, 0x6824 },
+	{0x1002, 0x6825 },
+	{0x1002, 0x6826 },
+	{0x1002, 0x6827 },
+	{0x1002, 0x6828 },
+	{0x1002, 0x6829 },
+	{0x1002, 0x682A },
+	{0x1002, 0x682B },
+	{0x1002, 0x682C },
+	{0x1002, 0x682D },
+	{0x1002, 0x682F },
+	{0x1002, 0x6830 },
+	{0x1002, 0x6831 },
+	{0x1002, 0x6835 },
+	{0x1002, 0x6837 },
+	{0x1002, 0x6838 },
+	{0x1002, 0x6839 },
+	{0x1002, 0x683B },
+	{0x1002, 0x683D },
+	{0x1002, 0x683F },
+	/* kabini */
+	{0x1002, 0x9830 },
+	{0x1002, 0x9831 },
+	{0x1002, 0x9832 },
+	{0x1002, 0x9833 },
+	{0x1002, 0x9834 },
+	{0x1002, 0x9835 },
+	{0x1002, 0x9836 },
+	{0x1002, 0x9837 },
+	{0x1002, 0x9838 },
+	{0x1002, 0x9839 },
+	{0x1002, 0x983a },
+	{0x1002, 0x983b },
+	{0x1002, 0x983c },
+	{0x1002, 0x983d },
+	{0x1002, 0x983e },
+	{0x1002, 0x983f },
+	/* mullins */
+	{0x1002, 0x9850 },
+	{0x1002, 0x9851 },
+	{0x1002, 0x9852 },
+	{0x1002, 0x9853 },
+	{0x1002, 0x9854 },
+	{0x1002, 0x9855 },
+	{0x1002, 0x9856 },
+	{0x1002, 0x9857 },
+	{0x1002, 0x9858 },
+	{0x1002, 0x9859 },
+	{0x1002, 0x985A },
+	{0x1002, 0x985B },
+	{0x1002, 0x985C },
+	{0x1002, 0x985D },
+	{0x1002, 0x985E },
+	{0x1002, 0x985F },
 	/* topaz */
 	{0x1002, 0x6900 },
 	{0x1002, 0x6901 },
Index: dev/pci/drm/include/generated/autoconf.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/include/generated/autoconf.h,v
diff -u -p -u -p -r1.16 autoconf.h
--- dev/pci/drm/include/generated/autoconf.h	16 Aug 2024 10:49:57 -0000	1.16
+++ dev/pci/drm/include/generated/autoconf.h	28 Dec 2024 20:50:15 -0000
@@ -11,8 +11,6 @@
 #if defined(__amd64__) || defined(__i386__)
 #define CONFIG_DRM_AMD_DC_DCN			1
 #define CONFIG_DRM_AMD_DC_FP			1
-#endif
-#if 0
 #define CONFIG_DRM_AMDGPU_SI			1
 #define CONFIG_DRM_AMD_DC_SI			1
 #define CONFIG_DRM_AMDGPU_CIK			1