update sysfs amd collector to pull pretty name from amdgpu.ids (#1569)

agent/gpu_amd_linux.go

@@ -3,6 +3,7 @@
 package agent
 
 import (
+	"bufio"
 	"fmt"
 	"log/slog"
 	"os"
@@ -15,6 +16,15 @@ import (
 	"github.com/henrygd/beszel/internal/entities/system"
 )
 
+var amdgpuNameCache = struct {
+	sync.RWMutex
+	hits   map[string]string
+	misses map[string]struct{}
+}{
+	hits:   make(map[string]string),
+	misses: make(map[string]struct{}),
+}
+
 // hasAmdSysfs returns true if any AMD GPU sysfs nodes are found
 func (gm *GPUManager) hasAmdSysfs() bool {
 	cards, err := filepath.Glob("/sys/class/drm/card*/device/vendor")
@@ -75,6 +85,7 @@ func (gm *GPUManager) collectAmdStats() error {
 	}
 }
 
+// isAmdGpu checks whether a DRM card path belongs to AMD vendor ID 0x1002.
 func isAmdGpu(cardPath string) bool {
 	vendorPath := filepath.Join(cardPath, "device/vendor")
 	vendor, err := os.ReadFile(vendorPath)
@@ -134,6 +145,7 @@ func (gm *GPUManager) updateAmdGpuData(cardPath string) bool {
 	return true
 }
 
+// readSysfsFloat reads and parses a numeric value from a sysfs file.
 func readSysfsFloat(path string) (float64, error) {
 	val, err := os.ReadFile(path)
 	if err != nil {
@@ -142,6 +154,110 @@ func readSysfsFloat(path string) (float64, error) {
 	return strconv.ParseFloat(strings.TrimSpace(string(val)), 64)
 }
 
+// normalizeHexID normalizes hex IDs by trimming spaces, lowercasing, and dropping 0x.
+func normalizeHexID(id string) string {
+	return strings.TrimPrefix(strings.ToLower(strings.TrimSpace(id)), "0x")
+}
+
+// cacheKeyForAmdgpu builds the cache key for a device and optional revision.
+func cacheKeyForAmdgpu(deviceID, revisionID string) string {
+	if revisionID != "" {
+		return deviceID + ":" + revisionID
+	}
+	return deviceID
+}
+
+// lookupAmdgpuNameInFile resolves an AMDGPU name from amdgpu.ids by device/revision.
+func lookupAmdgpuNameInFile(deviceID, revisionID, filePath string) (name string, exact bool, found bool) {
+	file, err := os.Open(filePath)
+	if err != nil {
+		return "", false, false
+	}
+	defer file.Close()
+
+	var byDevice string
+	scanner := bufio.NewScanner(file)
+	for scanner.Scan() {
+		line := strings.TrimSpace(scanner.Text())
+		if line == "" || strings.HasPrefix(line, "#") {
+			continue
+		}
+		parts := strings.SplitN(line, ",", 3)
+		if len(parts) != 3 {
+			continue
+		}
+
+		dev := normalizeHexID(parts[0])
+		rev := normalizeHexID(parts[1])
+		productName := strings.TrimSpace(parts[2])
+		if dev == "" || productName == "" || dev != deviceID {
+			continue
+		}
+		if byDevice == "" {
+			byDevice = productName
+		}
+		if revisionID != "" && rev == revisionID {
+			return productName, true, true
+		}
+	}
+	if byDevice != "" {
+		return byDevice, false, true
+	}
+	return "", false, false
+}
+
+// getCachedAmdgpuName returns cached hit/miss status for the given device/revision.
+func getCachedAmdgpuName(deviceID, revisionID string) (name string, found bool, done bool) {
+	// Build the list of cache keys to check. We always look up the exact device+revision key.
+	// When revisionID is set, we also look up deviceID alone, since the cache may store a
+	// device-only fallback when we couldn't resolve the exact revision.
+	keys := []string{cacheKeyForAmdgpu(deviceID, revisionID)}
+	if revisionID != "" {
+		keys = append(keys, deviceID)
+	}
+
+	knownMisses := 0
+	amdgpuNameCache.RLock()
+	defer amdgpuNameCache.RUnlock()
+	for _, key := range keys {
+		if name, ok := amdgpuNameCache.hits[key]; ok {
+			return name, true, true
+		}
+		if _, ok := amdgpuNameCache.misses[key]; ok {
+			knownMisses++
+		}
+	}
+	// done=true means "don't bother doing slow lookup": we either found a name (above) or
+	// every key we checked was already a known miss, so we've tried before and failed.
+	return "", false, knownMisses == len(keys)
+}
+
+// normalizeAmdgpuName trims standard suffixes from AMDGPU product names.
+func normalizeAmdgpuName(name string) string {
+	return strings.TrimSuffix(strings.TrimSpace(name), " Graphics")
+}
+
+// cacheAmdgpuName stores a resolved AMDGPU name in the lookup cache.
+func cacheAmdgpuName(deviceID, revisionID, name string, exact bool) {
+	name = normalizeAmdgpuName(name)
+	amdgpuNameCache.Lock()
+	defer amdgpuNameCache.Unlock()
+	if exact && revisionID != "" {
+		amdgpuNameCache.hits[cacheKeyForAmdgpu(deviceID, revisionID)] = name
+	}
+	amdgpuNameCache.hits[deviceID] = name
+}
+
+// cacheMissingAmdgpuName records unresolved device/revision lookups.
+func cacheMissingAmdgpuName(deviceID, revisionID string) {
+	amdgpuNameCache.Lock()
+	defer amdgpuNameCache.Unlock()
+	amdgpuNameCache.misses[deviceID] = struct{}{}
+	if revisionID != "" {
+		amdgpuNameCache.misses[cacheKeyForAmdgpu(deviceID, revisionID)] = struct{}{}
+	}
+}
+
 // getAmdGpuName attempts to get a descriptive GPU name.
 // First tries product_name (rarely available), then looks up the PCI device ID.
 // Falls back to showing the raw device ID if not found in the lookup table.
@@ -153,33 +269,24 @@ func getAmdGpuName(devicePath string) string {
 
 	// Read PCI device ID and look it up
 	if deviceID, err := os.ReadFile(filepath.Join(devicePath, "device")); err == nil {
-		id := strings.TrimPrefix(strings.ToLower(strings.TrimSpace(string(deviceID))), "0x")
-		if name, ok := getRadeonNames()[id]; ok {
-			return fmt.Sprintf("Radeon %s", name)
+		id := normalizeHexID(string(deviceID))
+		revision := ""
+		if revBytes, revErr := os.ReadFile(filepath.Join(devicePath, "revision")); revErr == nil {
+			revision = normalizeHexID(string(revBytes))
 		}
+
+		if name, found, done := getCachedAmdgpuName(id, revision); found {
+			return name
+		} else if !done {
+			if name, exact, ok := lookupAmdgpuNameInFile(id, revision, "/usr/share/libdrm/amdgpu.ids"); ok {
+				cacheAmdgpuName(id, revision, name, exact)
+				return normalizeAmdgpuName(name)
+			}
+			cacheMissingAmdgpuName(id, revision)
+		}
+
 		return fmt.Sprintf("AMD GPU (%s)", id)
 	}
 
 	return "AMD GPU"
 }
-
-// getRadeonNames returns the AMD GPU name lookup table
-// Device IDs from https://pci-ids.ucw.cz/read/PC/1002
-var getRadeonNames = sync.OnceValue(func() map[string]string {
-	return map[string]string{
-		"7550": "RX 9070",
-		"7590": "RX 9060 XT",
-		"7551": "AI PRO R9700",
-
-		"744c": "RX 7900",
-
-		"1681": "680M",
-
-		"7448": "PRO W7900",
-		"745e": "PRO W7800",
-		"7470": "PRO W7700",
-		"73e3": "PRO W6600",
-		"7422": "PRO W6400",
-		"7341": "PRO W5500",
-	}
-})