[PATCH 0/6] drm/imagination: Introduce hardware support check

[PATCH 0/6] drm/imagination: Introduce hardware support check

[Matt Coster]

We're seeing an influx of contributions to add support for lots of
different hardware containing Imagination GPUs, and for that we're
incredibly grateful.

Out of an abundance of caution, let's mark anything with intial support
that isn't yet reasonably widely tested as "experimental".

This serves two goals:
 - Don't accidentally declare that hardware with early support is usable
   without sufficient testing.
 - Allow for future breaking changes that would normally not be allowed
   (limited to this experimental hardware).

Here's a quick breakdown of the series:
 - P1-3: General cleanup & other parameter/debugfs-related enhancements.
 - P4: Introduce a module parameter to override the detected gpuid,
       which is useful for testing.
 - P5: Add KUnit infrastructure to the driver for the first time, to
       validate the error-prone task of parsing a gpuid from a string.
 - P6: Introduce the titular check.

Many of the earlier changes could go in on their own, but are not
critical fixes and would all land in the -next tree anyway so it seems
pointless to separate them from the context of the later changes.

Signed-off-by: Matt Coster <matt.coster@imgtec.com>
---
Alexandru Dadu (1):
      drm/imagination: Add gpuid module parameter

Matt Coster (5):
      drm/imagination: Simplify module parameters
      drm/imagination: Validate fw trace group_mask
      drm/imagination: Load FW trace config at init
      drm/imagination: KUnit test for pvr_gpuid_decode_string()
      drm/imagination: Warn or error on unsupported hardware

 drivers/gpu/drm/imagination/Kconfig        |  12 ++
 drivers/gpu/drm/imagination/Makefile       |   3 +-
 drivers/gpu/drm/imagination/pvr_debugfs.c  |   2 -
 drivers/gpu/drm/imagination/pvr_device.c   | 200 ++++++++++++++++++++++++++---
 drivers/gpu/drm/imagination/pvr_device.h   |  24 ++--
 drivers/gpu/drm/imagination/pvr_fw_trace.c | 125 +++++++++++++++---
 drivers/gpu/drm/imagination/pvr_fw_trace.h |   3 -
 drivers/gpu/drm/imagination/pvr_params.c   | 147 ---------------------
 drivers/gpu/drm/imagination/pvr_params.h   |  72 -----------
 drivers/gpu/drm/imagination/pvr_test.c     |  73 +++++++++++
 10 files changed, 389 insertions(+), 272 deletions(-)
---
base-commit: 4a768c544f64eaa2fc7cfa91e46f43aa4aad0c40
change-id: 20260107-device-support-info-f16d81b672d5

[PATCH 1/6] drm/imagination: Simplify module parameters

[Matt Coster]

We had a whole load of bloaty infrastructure to deal with module parameters
in a way that's wholly unnecessary. Strip it all back to basics to make
adding new parameters less of a headache.

Signed-off-by: Matt Coster <matt.coster@imgtec.com>
---
 drivers/gpu/drm/imagination/Makefile       |   1 -
 drivers/gpu/drm/imagination/pvr_debugfs.c  |   2 -
 drivers/gpu/drm/imagination/pvr_device.c   |   9 --
 drivers/gpu/drm/imagination/pvr_device.h   |  10 --
 drivers/gpu/drm/imagination/pvr_fw_trace.c |  46 ++++++++-
 drivers/gpu/drm/imagination/pvr_fw_trace.h |   3 -
 drivers/gpu/drm/imagination/pvr_params.c   | 147 -----------------------------
 drivers/gpu/drm/imagination/pvr_params.h   |  72 --------------
 8 files changed, 42 insertions(+), 248 deletions(-)

diff --git a/drivers/gpu/drm/imagination/Makefile b/drivers/gpu/drm/imagination/Makefile
index 7cca66f00a38a..ab63eac9ba7f7 100644
--- a/drivers/gpu/drm/imagination/Makefile
+++ b/drivers/gpu/drm/imagination/Makefile
@@ -20,7 +20,6 @@ powervr-y := \
 	pvr_hwrt.o \
 	pvr_job.o \
 	pvr_mmu.o \
-	pvr_params.o \
 	pvr_power.o \
 	pvr_queue.o \
 	pvr_stream.o \
diff --git a/drivers/gpu/drm/imagination/pvr_debugfs.c b/drivers/gpu/drm/imagination/pvr_debugfs.c
index c7ce7daaa87a0..ebdb05de40728 100644
--- a/drivers/gpu/drm/imagination/pvr_debugfs.c
+++ b/drivers/gpu/drm/imagination/pvr_debugfs.c
@@ -5,7 +5,6 @@
 
 #include "pvr_device.h"
 #include "pvr_fw_trace.h"
-#include "pvr_params.h"
 
 #include <linux/dcache.h>
 #include <linux/debugfs.h>
@@ -18,7 +17,6 @@
 #include <drm/drm_print.h>
 
 static const struct pvr_debugfs_entry pvr_debugfs_entries[] = {
-	{"pvr_params", pvr_params_debugfs_init},
 	{"pvr_fw", pvr_fw_trace_debugfs_init},
 };
 
diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
index 78d6b8a0a4506..abe8ad1d447ac 100644
--- a/drivers/gpu/drm/imagination/pvr_device.c
+++ b/drivers/gpu/drm/imagination/pvr_device.c
@@ -5,7 +5,6 @@
 #include "pvr_device_info.h"
 
 #include "pvr_fw.h"
-#include "pvr_params.h"
 #include "pvr_power.h"
 #include "pvr_queue.h"
 #include "pvr_rogue_cr_defs.h"
@@ -607,14 +606,6 @@ pvr_device_init(struct pvr_device *pvr_dev)
 	/* Get the platform-specific data based on the compatible string. */
 	pvr_dev->device_data = of_device_get_match_data(dev);
 
-	/*
-	 * Setup device parameters. We do this first in case other steps
-	 * depend on them.
-	 */
-	err = pvr_device_params_init(&pvr_dev->params);
-	if (err)
-		return err;
-
 	/* Enable and initialize clocks required for the device to operate. */
 	err = pvr_device_clk_init(pvr_dev);
 	if (err)
diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
index ec53ff2755418..d0e61923fd9b4 100644
--- a/drivers/gpu/drm/imagination/pvr_device.h
+++ b/drivers/gpu/drm/imagination/pvr_device.h
@@ -7,7 +7,6 @@
 #include "pvr_ccb.h"
 #include "pvr_device_info.h"
 #include "pvr_fw.h"
-#include "pvr_params.h"
 #include "pvr_rogue_fwif_stream.h"
 #include "pvr_stream.h"
 
@@ -192,15 +191,6 @@ struct pvr_device {
 	/** @fw_dev: Firmware related data. */
 	struct pvr_fw_device fw_dev;
 
-	/**
-	 * @params: Device-specific parameters.
-	 *
-	 *          The values of these parameters are initialized from the
-	 *          defaults specified as module parameters. They may be
-	 *          modified at runtime via debugfs (if enabled).
-	 */
-	struct pvr_device_params params;
-
 	/** @stream_musthave_quirks: Bit array of "must-have" quirks for stream commands. */
 	u32 stream_musthave_quirks[PVR_STREAM_TYPE_MAX][PVR_STREAM_EXTHDR_TYPE_MAX];
 
diff --git a/drivers/gpu/drm/imagination/pvr_fw_trace.c b/drivers/gpu/drm/imagination/pvr_fw_trace.c
index 8a56952f6730e..a607e5b108915 100644
--- a/drivers/gpu/drm/imagination/pvr_fw_trace.c
+++ b/drivers/gpu/drm/imagination/pvr_fw_trace.c
@@ -14,9 +14,26 @@
 #include <linux/build_bug.h>
 #include <linux/dcache.h>
 #include <linux/debugfs.h>
+#include <linux/moduleparam.h>
 #include <linux/sysfs.h>
 #include <linux/types.h>
 
+/*
+ * Don't gate this behind CONFIG_DEBUG_FS so that it can be used as an initial
+ * value without further conditional code...
+ */
+static u32 pvr_fw_trace_init_mask;
+
+/*
+ * ...but do only expose the module parameter if debugfs is enabled, since
+ * there's no reason to turn on fw_trace without it.
+ */
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+module_param_named(init_fw_trace_mask, pvr_fw_trace_init_mask, hexint, 0600);
+MODULE_PARM_DESC(init_fw_trace_mask,
+		 "Enable FW trace for the specified groups at device init time");
+#endif
+
 static void
 tracebuf_ctrl_init(void *cpu_ptr, void *priv)
 {
@@ -126,6 +143,8 @@ void pvr_fw_trace_fini(struct pvr_device *pvr_dev)
  * @group_mask: New log group mask.
  *
  * Returns:
+ *  * 0 if the provided @group_mask is the same as the current value (this is a
+ *    short-circuit evaluation),
  *  * 0 on success,
  *  * Any error returned by pvr_kccb_send_cmd(), or
  *  * -%EIO if the device is lost.
@@ -138,6 +157,10 @@ update_logtype(struct pvr_device *pvr_dev, u32 group_mask)
 	int idx;
 	int err;
 
+	/* No change in group_mask => nothing to update. */
+	if (fw_trace->group_mask == group_mask)
+		return 0;
+
 	if (group_mask)
 		fw_trace->tracebuf_ctrl->log_type = ROGUE_FWIF_LOG_TYPE_TRACE | group_mask;
 	else
@@ -437,13 +460,25 @@ static const struct file_operations pvr_fw_trace_fops = {
 	.release = fw_trace_release,
 };
 
-void
-pvr_fw_trace_mask_update(struct pvr_device *pvr_dev, u32 old_mask, u32 new_mask)
+static int pvr_fw_trace_mask_get(void *data, u64 *value)
+{
+	struct pvr_device *pvr_dev = data;
+
+	*value = pvr_dev->fw_dev.fw_trace.group_mask;
+
+	return 0;
+}
+
+static int pvr_fw_trace_mask_set(void *data, u64 value)
 {
-	if (IS_ENABLED(CONFIG_DEBUG_FS) && old_mask != new_mask)
-		update_logtype(pvr_dev, new_mask);
+	struct pvr_device *pvr_dev = data;
+
+	return update_logtype(pvr_dev, (u32)value);
 }
 
+DEFINE_DEBUGFS_ATTRIBUTE(pvr_fw_trace_mask_fops, pvr_fw_trace_mask_get,
+			 pvr_fw_trace_mask_set, "0x%08llx\n");
+
 void
 pvr_fw_trace_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir)
 {
@@ -463,4 +498,7 @@ pvr_fw_trace_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir)
 				    &fw_trace->buffers[thread_nr],
 				    &pvr_fw_trace_fops);
 	}
+
+	debugfs_create_file("trace_mask", 0600, dir, fw_trace,
+			    &pvr_fw_trace_mask_fops);
 }
diff --git a/drivers/gpu/drm/imagination/pvr_fw_trace.h b/drivers/gpu/drm/imagination/pvr_fw_trace.h
index 1d0ef937427a5..0cc57f66675d8 100644
--- a/drivers/gpu/drm/imagination/pvr_fw_trace.h
+++ b/drivers/gpu/drm/imagination/pvr_fw_trace.h
@@ -68,9 +68,6 @@ void pvr_fw_trace_fini(struct pvr_device *pvr_dev);
 /* Forward declaration from <linux/dcache.h>. */
 struct dentry;
 
-void pvr_fw_trace_mask_update(struct pvr_device *pvr_dev, u32 old_mask,
-			      u32 new_mask);
-
 void pvr_fw_trace_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir);
 
 #endif /* PVR_FW_TRACE_H */
diff --git a/drivers/gpu/drm/imagination/pvr_params.c b/drivers/gpu/drm/imagination/pvr_params.c
deleted file mode 100644
index b91759f362c57..0000000000000
--- a/drivers/gpu/drm/imagination/pvr_params.c
+++ /dev/null
@@ -1,147 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only OR MIT
-/* Copyright (c) 2023 Imagination Technologies Ltd. */
-
-#include "pvr_params.h"
-
-#include <linux/cache.h>
-#include <linux/moduleparam.h>
-
-static struct pvr_device_params pvr_device_param_defaults __read_mostly = {
-#define X(type_, name_, value_, desc_, ...) .name_ = (value_),
-	PVR_DEVICE_PARAMS
-#undef X
-};
-
-#define PVR_DEVICE_PARAM_NAMED(name_, type_, desc_) \
-	module_param_named(name_, pvr_device_param_defaults.name_, type_, \
-			   0400);                                         \
-	MODULE_PARM_DESC(name_, desc_);
-
-/*
- * This list of defines must contain every type specified in "pvr_params.h" as
- * ``PVR_PARAM_TYPE_*_C``.
- */
-#define PVR_PARAM_TYPE_X32_MODPARAM uint
-
-#define X(type_, name_, value_, desc_, ...) \
-	PVR_DEVICE_PARAM_NAMED(name_, PVR_PARAM_TYPE_##type_##_MODPARAM, desc_);
-PVR_DEVICE_PARAMS
-#undef X
-
-int
-pvr_device_params_init(struct pvr_device_params *params)
-{
-	/*
-	 * If heap-allocated parameters are added in the future (e.g.
-	 * modparam's charp type), they must be handled specially here (via
-	 * kstrdup() in the case of charp). Since that's not necessary yet,
-	 * a straight copy will do for now. This change will also require a
-	 * pvr_device_params_fini() function to free any heap-allocated copies.
-	 */
-
-	*params = pvr_device_param_defaults;
-
-	return 0;
-}
-
-#if defined(CONFIG_DEBUG_FS)
-#include "pvr_device.h"
-
-#include <linux/dcache.h>
-#include <linux/debugfs.h>
-#include <linux/export.h>
-#include <linux/fs.h>
-#include <linux/stddef.h>
-
-/*
- * This list of defines must contain every type specified in "pvr_params.h" as
- * ``PVR_PARAM_TYPE_*_C``.
- */
-#define PVR_PARAM_TYPE_X32_FMT "0x%08llx"
-
-#define X_SET(name_, mode_) X_SET_##mode_(name_)
-#define X_SET_DEF(name_, update_, mode_) X_SET_DEF_##mode_(name_, update_)
-
-#define X_SET_RO(name_) NULL
-#define X_SET_RW(name_) __pvr_device_param_##name_##set
-
-#define X_SET_DEF_RO(name_, update_)
-#define X_SET_DEF_RW(name_, update_)                                    \
-	static int                                                      \
-	X_SET_RW(name_)(void *data, u64 val)                            \
-	{                                                               \
-		struct pvr_device *pvr_dev = data;                      \
-		/* This is not just (update_) to suppress -Waddress. */ \
-		if ((void *)(update_) != NULL)                          \
-			(update_)(pvr_dev, pvr_dev->params.name_, val); \
-		pvr_dev->params.name_ = val;                            \
-		return 0;                                               \
-	}
-
-#define X(type_, name_, value_, desc_, mode_, update_)                     \
-	static int                                                         \
-	__pvr_device_param_##name_##_get(void *data, u64 *val)             \
-	{                                                                  \
-		struct pvr_device *pvr_dev = data;                         \
-		*val = pvr_dev->params.name_;                              \
-		return 0;                                                  \
-	}                                                                  \
-	X_SET_DEF(name_, update_, mode_)                                   \
-	static int                                                         \
-	__pvr_device_param_##name_##_open(struct inode *inode,             \
-					  struct file *file)               \
-	{                                                                  \
-		__simple_attr_check_format(PVR_PARAM_TYPE_##type_##_FMT,   \
-					   0ull);                          \
-		return simple_attr_open(inode, file,                       \
-					__pvr_device_param_##name_##_get,  \
-					X_SET(name_, mode_),               \
-					PVR_PARAM_TYPE_##type_##_FMT);     \
-	}
-PVR_DEVICE_PARAMS
-#undef X
-
-#undef X_SET
-#undef X_SET_RO
-#undef X_SET_RW
-#undef X_SET_DEF
-#undef X_SET_DEF_RO
-#undef X_SET_DEF_RW
-
-static struct {
-#define X(type_, name_, value_, desc_, mode_, update_) \
-	const struct file_operations name_;
-	PVR_DEVICE_PARAMS
-#undef X
-} pvr_device_param_debugfs_fops = {
-#define X(type_, name_, value_, desc_, mode_, update_)     \
-	.name_ = {                                         \
-		.owner = THIS_MODULE,                      \
-		.open = __pvr_device_param_##name_##_open, \
-		.release = simple_attr_release,            \
-		.read = simple_attr_read,                  \
-		.write = simple_attr_write,                \
-		.llseek = generic_file_llseek,             \
-	},
-	PVR_DEVICE_PARAMS
-#undef X
-};
-
-void
-pvr_params_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir)
-{
-#define X_MODE(mode_) X_MODE_##mode_
-#define X_MODE_RO 0400
-#define X_MODE_RW 0600
-
-#define X(type_, name_, value_, desc_, mode_, update_)             \
-	debugfs_create_file(#name_, X_MODE(mode_), dir, pvr_dev,   \
-			    &pvr_device_param_debugfs_fops.name_);
-	PVR_DEVICE_PARAMS
-#undef X
-
-#undef X_MODE
-#undef X_MODE_RO
-#undef X_MODE_RW
-}
-#endif
diff --git a/drivers/gpu/drm/imagination/pvr_params.h b/drivers/gpu/drm/imagination/pvr_params.h
deleted file mode 100644
index 5807915b456bf..0000000000000
--- a/drivers/gpu/drm/imagination/pvr_params.h
+++ /dev/null
@@ -1,72 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
-/* Copyright (c) 2023 Imagination Technologies Ltd. */
-
-#ifndef PVR_PARAMS_H
-#define PVR_PARAMS_H
-
-#include "pvr_rogue_fwif.h"
-
-#include <linux/cache.h>
-#include <linux/compiler_attributes.h>
-
-/*
- * This is the definitive list of types allowed in the definition of
- * %PVR_DEVICE_PARAMS.
- */
-#define PVR_PARAM_TYPE_X32_C u32
-
-/*
- * This macro defines all device-specific parameters; that is parameters which
- * are set independently per device.
- *
- * The X-macro accepts the following arguments. Arguments marked with [debugfs]
- * are ignored when debugfs is disabled; values used for these arguments may
- * safely be gated behind CONFIG_DEBUG_FS.
- *
- * @type_: The definitive list of allowed values is PVR_PARAM_TYPE_*_C.
- * @name_: Name of the parameter. This is used both as the field name in C and
- *         stringified as the parameter name.
- * @value_: Initial/default value.
- * @desc_: String literal used as help text to describe the usage of this
- *         parameter.
- * @mode_: [debugfs] One of {RO,RW}. The access mode of the debugfs entry for
- *         this parameter.
- * @update_: [debugfs] When debugfs support is enabled, parameters may be
- *           updated at runtime. When this happens, this function will be
- *           called to allow changes to propagate. The signature of this
- *           function is:
- *
- *              void (*)(struct pvr_device *pvr_dev, T old_val, T new_val)
- *
- *           Where T is the C type associated with @type_.
- *
- *           If @mode_ does not allow write access, this function will never be
- *           called. In this case, or if no update callback is required, you
- *           should specify NULL for this argument.
- */
-#define PVR_DEVICE_PARAMS                                                    \
-	X(X32, fw_trace_mask, ROGUE_FWIF_LOG_TYPE_NONE,                      \
-	  "Enable FW trace for the specified groups. Specifying 0 disables " \
-	  "all FW tracing.",                                                 \
-	  RW, pvr_fw_trace_mask_update)
-
-struct pvr_device_params {
-#define X(type_, name_, value_, desc_, ...) \
-	PVR_PARAM_TYPE_##type_##_C name_;
-	PVR_DEVICE_PARAMS
-#undef X
-};
-
-int pvr_device_params_init(struct pvr_device_params *params);
-
-#if defined(CONFIG_DEBUG_FS)
-/* Forward declaration from "pvr_device.h". */
-struct pvr_device;
-
-/* Forward declaration from <linux/dcache.h>. */
-struct dentry;
-
-void pvr_params_debugfs_init(struct pvr_device *pvr_dev, struct dentry *dir);
-#endif /* defined(CONFIG_DEBUG_FS) */
-
-#endif /* PVR_PARAMS_H */

-- 
2.52.0

[PATCH 2/6] drm/imagination: Validate fw trace group_mask

[Matt Coster]

This value can come from two places: a module parameter or a debugfs file.
In both cases, validate it early to provide feedback to userspace at the
time the value is set instead of deferring until the value is used.

Signed-off-by: Matt Coster <matt.coster@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_fw_trace.c | 76 ++++++++++++++++++++++++------
 1 file changed, 62 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_fw_trace.c b/drivers/gpu/drm/imagination/pvr_fw_trace.c
index a607e5b108915..a2aa588cbe5fa 100644
--- a/drivers/gpu/drm/imagination/pvr_fw_trace.c
+++ b/drivers/gpu/drm/imagination/pvr_fw_trace.c
@@ -12,12 +12,35 @@
 #include <drm/drm_print.h>
 
 #include <linux/build_bug.h>
+#include <linux/compiler_attributes.h>
 #include <linux/dcache.h>
 #include <linux/debugfs.h>
 #include <linux/moduleparam.h>
 #include <linux/sysfs.h>
 #include <linux/types.h>
 
+static int
+validate_group_mask(struct pvr_device *pvr_dev, const u32 group_mask)
+{
+	if (group_mask & ~ROGUE_FWIF_LOG_TYPE_GROUP_MASK) {
+		drm_warn(from_pvr_device(pvr_dev),
+			 "Invalid fw_trace group mask 0x%08x (must be a subset of 0x%08x)",
+			 group_mask, ROGUE_FWIF_LOG_TYPE_GROUP_MASK);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static inline u32
+build_log_type(const u32 group_mask)
+{
+	if (!group_mask)
+		return ROGUE_FWIF_LOG_TYPE_NONE;
+
+	return group_mask | ROGUE_FWIF_LOG_TYPE_TRACE;
+}
+
 /*
  * Don't gate this behind CONFIG_DEBUG_FS so that it can be used as an initial
  * value without further conditional code...
@@ -29,7 +52,33 @@ static u32 pvr_fw_trace_init_mask;
  * there's no reason to turn on fw_trace without it.
  */
 #if IS_ENABLED(CONFIG_DEBUG_FS)
-module_param_named(init_fw_trace_mask, pvr_fw_trace_init_mask, hexint, 0600);
+static int
+pvr_fw_trace_init_mask_set(const char *val, const struct kernel_param *kp)
+{
+	u32 mask = 0;
+	int err;
+
+	err = kstrtouint(val, 0, &mask);
+	if (err)
+		return err;
+
+	err = validate_group_mask(NULL, mask);
+	if (err)
+		return err;
+
+	*(unsigned int *)kp->arg = mask;
+
+	return 0;
+}
+
+const struct kernel_param_ops pvr_fw_trace_init_mask_ops = {
+	.set = pvr_fw_trace_init_mask_set,
+	.get = param_get_hexint,
+};
+
+param_check_hexint(init_fw_trace_mask, &pvr_fw_trace_init_mask);
+module_param_cb(init_fw_trace_mask, &pvr_fw_trace_init_mask_ops, &pvr_fw_trace_init_mask, 0600);
+__MODULE_PARM_TYPE(init_fw_trace_mask, "hexint");
 MODULE_PARM_DESC(init_fw_trace_mask,
 		 "Enable FW trace for the specified groups at device init time");
 #endif
@@ -42,11 +91,7 @@ tracebuf_ctrl_init(void *cpu_ptr, void *priv)
 
 	tracebuf_ctrl->tracebuf_size_in_dwords = ROGUE_FW_TRACE_BUF_DEFAULT_SIZE_IN_DWORDS;
 	tracebuf_ctrl->tracebuf_flags = 0;
-
-	if (fw_trace->group_mask)
-		tracebuf_ctrl->log_type = fw_trace->group_mask | ROGUE_FWIF_LOG_TYPE_TRACE;
-	else
-		tracebuf_ctrl->log_type = ROGUE_FWIF_LOG_TYPE_NONE;
+	tracebuf_ctrl->log_type = build_log_type(fw_trace->group_mask);
 
 	for (u32 thread_nr = 0; thread_nr < ARRAY_SIZE(fw_trace->buffers); thread_nr++) {
 		struct rogue_fwif_tracebuf_space *tracebuf_space =
@@ -140,7 +185,7 @@ void pvr_fw_trace_fini(struct pvr_device *pvr_dev)
 /**
  * update_logtype() - Send KCCB command to trigger FW to update logtype
  * @pvr_dev: Target PowerVR device
- * @group_mask: New log group mask.
+ * @group_mask: New log group mask; must pass validate_group_mask().
  *
  * Returns:
  *  * 0 if the provided @group_mask is the same as the current value (this is a
@@ -153,6 +198,7 @@ static int
 update_logtype(struct pvr_device *pvr_dev, u32 group_mask)
 {
 	struct pvr_fw_trace *fw_trace = &pvr_dev->fw_dev.fw_trace;
+	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
 	struct rogue_fwif_kccb_cmd cmd;
 	int idx;
 	int err;
@@ -161,15 +207,11 @@ update_logtype(struct pvr_device *pvr_dev, u32 group_mask)
 	if (fw_trace->group_mask == group_mask)
 		return 0;
 
-	if (group_mask)
-		fw_trace->tracebuf_ctrl->log_type = ROGUE_FWIF_LOG_TYPE_TRACE | group_mask;
-	else
-		fw_trace->tracebuf_ctrl->log_type = ROGUE_FWIF_LOG_TYPE_NONE;
-
 	fw_trace->group_mask = group_mask;
+	fw_trace->tracebuf_ctrl->log_type = build_log_type(group_mask);
 
 	down_read(&pvr_dev->reset_sem);
-	if (!drm_dev_enter(from_pvr_device(pvr_dev), &idx)) {
+	if (!drm_dev_enter(drm_dev, &idx)) {
 		err = -EIO;
 		goto err_up_read;
 	}
@@ -472,8 +514,14 @@ static int pvr_fw_trace_mask_get(void *data, u64 *value)
 static int pvr_fw_trace_mask_set(void *data, u64 value)
 {
 	struct pvr_device *pvr_dev = data;
+	const u32 group_mask = (u32)value;
+	int err;
+
+	err = validate_group_mask(pvr_dev, group_mask);
+	if (err)
+		return err;
 
-	return update_logtype(pvr_dev, (u32)value);
+	return update_logtype(pvr_dev, group_mask);
 }
 
 DEFINE_DEBUGFS_ATTRIBUTE(pvr_fw_trace_mask_fops, pvr_fw_trace_mask_get,

-- 
2.52.0

[PATCH 3/6] drm/imagination: Load FW trace config at init

[Matt Coster]

We have a module parameter to set the initial group mask before debugfs is
available for any specific device, but don't currently use that value when
initialising devices.

Use the module parameter value as the initial value for group_mask.

Signed-off-by: Matt Coster <matt.coster@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_fw_trace.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_fw_trace.c b/drivers/gpu/drm/imagination/pvr_fw_trace.c
index a2aa588cbe5fa..93119f0f23a92 100644
--- a/drivers/gpu/drm/imagination/pvr_fw_trace.c
+++ b/drivers/gpu/drm/imagination/pvr_fw_trace.c
@@ -130,8 +130,13 @@ int pvr_fw_trace_init(struct pvr_device *pvr_dev)
 		}
 	}
 
-	/* TODO: Provide control of group mask. */
-	fw_trace->group_mask = 0;
+	/*
+	 * Load the initial group_mask from the init_fw_trace_mask module
+	 * parameter. This allows early tracing before the user can write to
+	 * debugfs. Unlike update_logtype(), we don't set log_type here as that
+	 * is initialised by tracebuf_ctrl_init().
+	 */
+	fw_trace->group_mask = pvr_fw_trace_init_mask;
 
 	fw_trace->tracebuf_ctrl =
 		pvr_fw_object_create_and_map(pvr_dev,

-- 
2.52.0

[PATCH 4/6] drm/imagination: Add gpuid module parameter

[Matt Coster]

From: Alexandru Dadu <alexandru.dadu@imgtec.com>

The "gpuid" module parameter is used to override the gpuid read from a
hardware register and is useful for testing the loading of different
firmware (including processing of the firmware header) without having
the hardware to hand.

Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
Signed-off-by: Matt Coster <matt.coster@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_device.c | 117 ++++++++++++++++++++++++++++---
 drivers/gpu/drm/imagination/pvr_device.h |   7 +-
 2 files changed, 114 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
index abe8ad1d447ac..db844e4e2e945 100644
--- a/drivers/gpu/drm/imagination/pvr_device.c
+++ b/drivers/gpu/drm/imagination/pvr_device.c
@@ -421,23 +421,21 @@ pvr_request_firmware(struct pvr_device *pvr_dev)
 }
 
 /**
- * pvr_load_gpu_id() - Load a PowerVR device's GPU ID (BVNC) from control registers.
+ * pvr_gpuid_decode_reg() - Decode the GPU ID from GPU register
  *
- * Sets struct pvr_dev.gpu_id.
+ * Sets the b, v, n, c fields of struct pvr_dev.gpu_id.
  *
  * @pvr_dev: Target PowerVR device.
+ * @gpu_id: Output to be updated with the GPU ID.
  */
 static void
-pvr_load_gpu_id(struct pvr_device *pvr_dev)
+pvr_gpuid_decode_reg(const struct pvr_device *pvr_dev, struct pvr_gpu_id *gpu_id)
 {
-	struct pvr_gpu_id *gpu_id = &pvr_dev->gpu_id;
-	u64 bvnc;
-
 	/*
 	 * Try reading the BVNC using the newer (cleaner) method first. If the
 	 * B value is zero, fall back to the older method.
 	 */
-	bvnc = pvr_cr_read64(pvr_dev, ROGUE_CR_CORE_ID__PBVNC);
+	u64 bvnc = pvr_cr_read64(pvr_dev, ROGUE_CR_CORE_ID__PBVNC);
 
 	gpu_id->b = PVR_CR_FIELD_GET(bvnc, CORE_ID__PBVNC__BRANCH_ID);
 	if (gpu_id->b != 0) {
@@ -456,6 +454,107 @@ pvr_load_gpu_id(struct pvr_device *pvr_dev)
 	}
 }
 
+/**
+ * pvr_gpuid_decode_string() - Decode the GPU ID from a module input string
+ *
+ * Sets the b, v, n, c fields of struct pvr_dev.gpu_id.
+ *
+ * @pvr_dev: Target PowerVR device.
+ * @param_bvnc: GPU ID (BVNC) module parameter.
+ * @gpu_id: Output to be updated with the GPU ID.
+ */
+static int
+pvr_gpuid_decode_string(const struct pvr_device *pvr_dev,
+			const char *param_bvnc, struct pvr_gpu_id *gpu_id)
+{
+	const struct drm_device *drm_dev = &pvr_dev->base;
+	char str_cpy[PVR_GPUID_STRING_MAX_LENGTH];
+	char *pos, *tkn;
+	int ret, idx = 0;
+	u16 user_bvnc_u16[4];
+	u8 dot_cnt = 0;
+
+	ret = strscpy(str_cpy, param_bvnc);
+
+	/*
+	 * strscpy() should return at least a size 7 for the input to be valid.
+	 * Returns -E2BIG for the case when the string is empty or too long.
+	 */
+	if (ret < PVR_GPUID_STRING_MIN_LENGTH) {
+		drm_info(drm_dev,
+			 "Invalid size of the input GPU ID (BVNC): %s",
+			 str_cpy);
+		return -EINVAL;
+	}
+
+	while (*param_bvnc) {
+		if (*param_bvnc == '.')
+			dot_cnt++;
+		param_bvnc++;
+	}
+
+	if (dot_cnt != 3) {
+		drm_info(drm_dev,
+			 "Invalid format of the input GPU ID (BVNC): %s",
+			 str_cpy);
+		return -EINVAL;
+	}
+
+	pos = str_cpy;
+
+	while ((tkn = strsep(&pos, ".")) != NULL && idx < 4) {
+		/* kstrtou16() will also handle the case of consecutive dots */
+		ret = kstrtou16(tkn, 10, &user_bvnc_u16[idx]);
+		if (ret) {
+			drm_info(drm_dev,
+				 "Invalid format of the input GPU ID (BVNC): %s",
+				 str_cpy);
+			return -EINVAL;
+		}
+		idx++;
+	}
+
+	gpu_id->b = user_bvnc_u16[0];
+	gpu_id->v = user_bvnc_u16[1];
+	gpu_id->n = user_bvnc_u16[2];
+	gpu_id->c = user_bvnc_u16[3];
+
+	return 0;
+}
+
+static char *pvr_gpuid_override;
+module_param_named(gpuid, pvr_gpuid_override, charp, 0400);
+MODULE_PARM_DESC(gpuid, "GPU ID (BVNC) to be used instead of the value read from hardware.");
+
+/**
+ * pvr_load_gpu_id() - Load a PowerVR device's GPU ID (BVNC) from control
+ * registers or input parameter. The input parameter is processed instead
+ * of the GPU register if provided.
+ *
+ * Sets the arch field of struct pvr_dev.gpu_id.
+ *
+ * @pvr_dev: Target PowerVR device.
+ */
+static int
+pvr_load_gpu_id(struct pvr_device *pvr_dev)
+{
+	struct pvr_gpu_id *gpu_id = &pvr_dev->gpu_id;
+
+	if (!pvr_gpuid_override || !pvr_gpuid_override[0]) {
+		pvr_gpuid_decode_reg(pvr_dev, gpu_id);
+	} else {
+		drm_warn(from_pvr_device(pvr_dev),
+			 "Using custom GPU ID (BVNC) provided by the user!");
+
+		int err = pvr_gpuid_decode_string(pvr_dev, pvr_gpuid_override,
+						  gpu_id);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 /**
  * pvr_set_dma_info() - Set PowerVR device DMA information
  * @pvr_dev: Target PowerVR device.
@@ -516,7 +615,9 @@ pvr_device_gpu_init(struct pvr_device *pvr_dev)
 {
 	int err;
 
-	pvr_load_gpu_id(pvr_dev);
+	err = pvr_load_gpu_id(pvr_dev);
+	if (err)
+		return err;
 
 	err = pvr_request_firmware(pvr_dev);
 	if (err)
diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
index d0e61923fd9b4..5608a977f6d21 100644
--- a/drivers/gpu/drm/imagination/pvr_device.h
+++ b/drivers/gpu/drm/imagination/pvr_device.h
@@ -39,6 +39,9 @@ struct firmware;
 /* Forward declaration from <linux/pwrseq/consumer.h> */
 struct pwrseq_desc;
 
+#define PVR_GPUID_STRING_MIN_LENGTH 7U
+#define PVR_GPUID_STRING_MAX_LENGTH 32U
+
 /**
  * struct pvr_gpu_id - Hardware GPU ID information for a PowerVR device
  * @b: Branch ID.
@@ -558,7 +561,7 @@ pvr_device_has_feature(struct pvr_device *pvr_dev, u32 feature);
  * Return: The value of the requested register.
  */
 static __always_inline u32
-pvr_cr_read32(struct pvr_device *pvr_dev, u32 reg)
+pvr_cr_read32(const struct pvr_device *pvr_dev, u32 reg)
 {
 	return ioread32(pvr_dev->regs + reg);
 }
@@ -571,7 +574,7 @@ pvr_cr_read32(struct pvr_device *pvr_dev, u32 reg)
  * Return: The value of the requested register.
  */
 static __always_inline u64
-pvr_cr_read64(struct pvr_device *pvr_dev, u32 reg)
+pvr_cr_read64(const struct pvr_device *pvr_dev, u32 reg)
 {
 	return ioread64(pvr_dev->regs + reg);
 }

-- 
2.52.0

[PATCH 5/6] drm/imagination: KUnit test for pvr_gpuid_decode_string()

[Matt Coster]

This is a nice self-contained function to serve as the basis of our first
KUnit tests.

Signed-off-by: Matt Coster <matt.coster@imgtec.com>
---
 drivers/gpu/drm/imagination/Kconfig      | 12 ++++++
 drivers/gpu/drm/imagination/Makefile     |  2 +
 drivers/gpu/drm/imagination/pvr_device.c |  5 ++-
 drivers/gpu/drm/imagination/pvr_device.h |  7 ++-
 drivers/gpu/drm/imagination/pvr_test.c   | 73 ++++++++++++++++++++++++++++++++
 5 files changed, 97 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imagination/Kconfig b/drivers/gpu/drm/imagination/Kconfig
index 0482bfcefdde3..1fd4c635c2c96 100644
--- a/drivers/gpu/drm/imagination/Kconfig
+++ b/drivers/gpu/drm/imagination/Kconfig
@@ -18,3 +18,15 @@ config DRM_POWERVR
 	  Technologies PowerVR (Series 6 or later) or IMG GPU.
 
 	  If "M" is selected, the module will be called powervr.
+
+config DRM_POWERVR_KUNIT_TEST
+	tristate "KUnit tests for the drm powervr driver" if !KUNIT_ALL_TESTS
+	depends on DRM_POWERVR && KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  Choose this option to allow the driver to perform selftests under
+	  the kunit framework
+
+	  Recommended for driver developers only.
+
+	  If in doubt, say "N".
diff --git a/drivers/gpu/drm/imagination/Makefile b/drivers/gpu/drm/imagination/Makefile
index ab63eac9ba7f7..f5072f06b4c41 100644
--- a/drivers/gpu/drm/imagination/Makefile
+++ b/drivers/gpu/drm/imagination/Makefile
@@ -32,3 +32,5 @@ powervr-$(CONFIG_DEBUG_FS) += \
 	pvr_debugfs.o
 
 obj-$(CONFIG_DRM_POWERVR) += powervr.o
+
+obj-$(CONFIG_DRM_POWERVR_KUNIT_TEST) += pvr_test.o
diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
index db844e4e2e945..d87557812409a 100644
--- a/drivers/gpu/drm/imagination/pvr_device.c
+++ b/drivers/gpu/drm/imagination/pvr_device.c
@@ -31,6 +31,8 @@
 #include <linux/types.h>
 #include <linux/workqueue.h>
 
+#include <kunit/visibility.h>
+
 /* Major number for the supported version of the firmware. */
 #define PVR_FW_VERSION_MAJOR 1
 
@@ -463,7 +465,7 @@ pvr_gpuid_decode_reg(const struct pvr_device *pvr_dev, struct pvr_gpu_id *gpu_id
  * @param_bvnc: GPU ID (BVNC) module parameter.
  * @gpu_id: Output to be updated with the GPU ID.
  */
-static int
+VISIBLE_IF_KUNIT int
 pvr_gpuid_decode_string(const struct pvr_device *pvr_dev,
 			const char *param_bvnc, struct pvr_gpu_id *gpu_id)
 {
@@ -521,6 +523,7 @@ pvr_gpuid_decode_string(const struct pvr_device *pvr_dev,
 
 	return 0;
 }
+EXPORT_SYMBOL_IF_KUNIT(pvr_gpuid_decode_string);
 
 static char *pvr_gpuid_override;
 module_param_named(gpuid, pvr_gpuid_override, charp, 0400);
diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
index 5608a977f6d21..cfda215e7428e 100644
--- a/drivers/gpu/drm/imagination/pvr_device.h
+++ b/drivers/gpu/drm/imagination/pvr_device.h
@@ -519,7 +519,7 @@ struct pvr_file {
  * Return: Packed BVNC.
  */
 static __always_inline u64
-pvr_gpu_id_to_packed_bvnc(struct pvr_gpu_id *gpu_id)
+pvr_gpu_id_to_packed_bvnc(const struct pvr_gpu_id *gpu_id)
 {
 	return PVR_PACKED_BVNC(gpu_id->b, gpu_id->v, gpu_id->n, gpu_id->c);
 }
@@ -544,6 +544,11 @@ pvr_device_has_uapi_enhancement(struct pvr_device *pvr_dev, u32 enhancement);
 bool
 pvr_device_has_feature(struct pvr_device *pvr_dev, u32 feature);
 
+#if IS_ENABLED(CONFIG_KUNIT)
+int pvr_gpuid_decode_string(const struct pvr_device *pvr_dev,
+			    const char *param_bvnc, struct pvr_gpu_id *gpu_id);
+#endif
+
 /**
  * PVR_CR_FIELD_GET() - Extract a single field from a PowerVR control register
  * @val: Value of the target register.
diff --git a/drivers/gpu/drm/imagination/pvr_test.c b/drivers/gpu/drm/imagination/pvr_test.c
new file mode 100644
index 0000000000000..506cfa5a02f1e
--- /dev/null
+++ b/drivers/gpu/drm/imagination/pvr_test.c
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/* Copyright (c) 2025 Imagination Technologies Ltd. */
+
+#include "pvr_device.h"
+
+#include <linux/errno.h>
+#include <linux/stddef.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#include <kunit/test.h>
+#include <kunit/visibility.h>
+
+static void decode_gpuid_string(struct kunit *test)
+{
+	const struct pvr_gpu_id bad_gpuid = { 0xdead, 0xbeef, 0xcafe, 0xface };
+	const u64 packed_bad_gpuid = pvr_gpu_id_to_packed_bvnc(&bad_gpuid);
+
+#define GPUID_TEST_CASE(str_, err_, value_)					\
+	do {									\
+		struct pvr_gpu_id _gpuid_out = bad_gpuid;			\
+		int _err;							\
+		_err = pvr_gpuid_decode_string(NULL, str_, &_gpuid_out);	\
+		KUNIT_EXPECT_EQ(test, _err, err_);				\
+		KUNIT_EXPECT_EQ(test,						\
+				pvr_gpu_id_to_packed_bvnc(&_gpuid_out),		\
+				value_);					\
+	} while (0)
+
+#define GPUID_TEST_CASE_OK(str_, b_, v_, n_, c_) \
+	GPUID_TEST_CASE(str_, 0, PVR_PACKED_BVNC(b_, v_, n_, c_))
+
+#define GPUID_TEST_CASE_INVAL(str_) \
+	GPUID_TEST_CASE(str_, -EINVAL, packed_bad_gpuid)
+
+	GPUID_TEST_CASE_OK("12.34.56.78", 12, 34, 56, 78);
+	GPUID_TEST_CASE_OK("0.0.0.0", 0, 0, 0, 0);
+
+	GPUID_TEST_CASE_INVAL("");
+	GPUID_TEST_CASE_INVAL("42.foobar-invalid.gpuid.bvnc");
+
+	/* String longer than PVR_GPUID_STRING_MAX_LENGTH. */
+	GPUID_TEST_CASE_INVAL("12.34.56.789012345678901234567890123456");
+
+	/* Single value overflowing u16. */
+	GPUID_TEST_CASE_INVAL("12.34.56.999999");
+
+	/* Wrong number of parts and/or dots. */
+	GPUID_TEST_CASE_INVAL("12.34.56.78.90");
+	GPUID_TEST_CASE_INVAL("12.34.56..78");
+	GPUID_TEST_CASE_INVAL("12.34..56");
+	GPUID_TEST_CASE_INVAL("12.34.56");
+
+#undef GPUID_TEST_CASE_INVAL
+#undef GPUID_TEST_CASE_OK
+#undef GPUID_TEST_CASE
+}
+
+static struct kunit_case pvr_tests_cases[] = {
+	KUNIT_CASE(decode_gpuid_string),
+	{},
+};
+
+static struct kunit_suite pvr_tests_suite = {
+	.name = "pvr_tests",
+	.test_cases = pvr_tests_cases,
+};
+kunit_test_suite(pvr_tests_suite);
+
+MODULE_AUTHOR("Imagination Technologies Ltd.");
+MODULE_LICENSE("Dual MIT/GPL");
+MODULE_DESCRIPTION("pvr kunit tests");
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");

-- 
2.52.0

[PATCH 6/6] drm/imagination: Warn or error on unsupported hardware

[Matt Coster]

Gate the use of unsupported hardware behind a new module parameter
(exp_hw_support).

Signed-off-by: Matt Coster <matt.coster@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_device.c | 73 +++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
index d87557812409a..f58bb66a63275 100644
--- a/drivers/gpu/drm/imagination/pvr_device.c
+++ b/drivers/gpu/drm/imagination/pvr_device.c
@@ -525,6 +525,77 @@ pvr_gpuid_decode_string(const struct pvr_device *pvr_dev,
 }
 EXPORT_SYMBOL_IF_KUNIT(pvr_gpuid_decode_string);
 
+static bool pvr_exp_hw_support;
+module_param_named(exp_hw_support, pvr_exp_hw_support, bool, 0600);
+MODULE_PARM_DESC(exp_hw_support, "Bypass runtime checks for fully supported GPU cores. WARNING: enabling this option may result in a buggy, insecure, or otherwise unusable driver.");
+
+/**
+ * enum pvr_gpu_support_level - The level of support for a gpu_id in the current
+ * version of the driver.
+ *
+ * @PVR_GPU_UNKNOWN: Cores that are unknown to the driver. These may not even exist.
+ * @PVR_GPU_EXPERIMENTAL: Cores that have experimental support.
+ * @PVR_GPU_SUPPORTED: Cores that are supported and maintained.
+ */
+enum pvr_gpu_support_level {
+	PVR_GPU_UNKNOWN,
+	PVR_GPU_EXPERIMENTAL,
+	PVR_GPU_SUPPORTED,
+};
+
+static enum pvr_gpu_support_level
+pvr_gpu_support_level(const struct pvr_gpu_id *gpu_id)
+{
+	switch (pvr_gpu_id_to_packed_bvnc(gpu_id)) {
+	case PVR_PACKED_BVNC(33, 15, 11, 3):
+	case PVR_PACKED_BVNC(36, 53, 104, 796):
+		return PVR_GPU_SUPPORTED;
+
+	case PVR_PACKED_BVNC(36, 52, 104, 182):
+		return PVR_GPU_EXPERIMENTAL;
+
+	default:
+		return PVR_GPU_UNKNOWN;
+	}
+}
+
+static int
+pvr_check_gpu_supported(struct pvr_device *pvr_dev,
+			const struct pvr_gpu_id *gpu_id)
+{
+	struct drm_device *drm_dev = from_pvr_device(pvr_dev);
+
+	switch (pvr_gpu_support_level(gpu_id)) {
+	case PVR_GPU_SUPPORTED:
+		if (pvr_exp_hw_support)
+			drm_info(drm_dev, "Module parameter 'exp_hw_support' was set, but this hardware is fully supported by the current driver.");
+
+		break;
+
+	case PVR_GPU_EXPERIMENTAL:
+		if (!pvr_exp_hw_support) {
+			drm_err(drm_dev, "Unsupported GPU! Set 'exp_hw_support' to bypass this check.");
+			return -ENODEV;
+		}
+
+		drm_warn(drm_dev, "Running on unsupported hardware; you may encounter bugs!");
+		break;
+
+	/* NOTE: This code path may indicate misbehaving hardware. */
+	case PVR_GPU_UNKNOWN:
+	default:
+		if (!pvr_exp_hw_support) {
+			drm_err(drm_dev, "Unknown GPU! Set 'exp_hw_support' to bypass this check.");
+			return -ENODEV;
+		}
+
+		drm_warn(drm_dev, "Running on unknown hardware; expect issues.");
+		break;
+	}
+
+	return 0;
+}
+
 static char *pvr_gpuid_override;
 module_param_named(gpuid, pvr_gpuid_override, charp, 0400);
 MODULE_PARM_DESC(gpuid, "GPU ID (BVNC) to be used instead of the value read from hardware.");
@@ -555,7 +626,7 @@ pvr_load_gpu_id(struct pvr_device *pvr_dev)
 			return err;
 	}
 
-	return 0;
+	return pvr_check_gpu_supported(pvr_dev, gpu_id);
 }
 
 /**

-- 
2.52.0

Re: [PATCH 0/6] drm/imagination: Introduce hardware support check

[Alessio Belle]

On Tue, 2026-01-13 at 10:16 +0000, Matt Coster wrote:
> We're seeing an influx of contributions to add support for lots of
> different hardware containing Imagination GPUs, and for that we're
> incredibly grateful.
> 
> Out of an abundance of caution, let's mark anything with intial support
> that isn't yet reasonably widely tested as "experimental".
> 
> This serves two goals:
>  - Don't accidentally declare that hardware with early support is usable
>    without sufficient testing.
>  - Allow for future breaking changes that would normally not be allowed
>    (limited to this experimental hardware).
> 
> Here's a quick breakdown of the series:
>  - P1-3: General cleanup & other parameter/debugfs-related enhancements.
>  - P4: Introduce a module parameter to override the detected gpuid,
>        which is useful for testing.
>  - P5: Add KUnit infrastructure to the driver for the first time, to
>        validate the error-prone task of parsing a gpuid from a string.
>  - P6: Introduce the titular check.
> 
> Many of the earlier changes could go in on their own, but are not
> critical fixes and would all land in the -next tree anyway so it seems
> pointless to separate them from the context of the later changes.
> 
> Signed-off-by: Matt Coster <matt.coster@imgtec.com>
> ---
> Alexandru Dadu (1):
>       drm/imagination: Add gpuid module parameter
> 
> Matt Coster (5):
>       drm/imagination: Simplify module parameters
>       drm/imagination: Validate fw trace group_mask
>       drm/imagination: Load FW trace config at init
>       drm/imagination: KUnit test for pvr_gpuid_decode_string()
>       drm/imagination: Warn or error on unsupported hardware
> 
>  drivers/gpu/drm/imagination/Kconfig        |  12 ++
>  drivers/gpu/drm/imagination/Makefile       |   3 +-
>  drivers/gpu/drm/imagination/pvr_debugfs.c  |   2 -
>  drivers/gpu/drm/imagination/pvr_device.c   | 200 ++++++++++++++++++++++++++---
>  drivers/gpu/drm/imagination/pvr_device.h   |  24 ++--
>  drivers/gpu/drm/imagination/pvr_fw_trace.c | 125 +++++++++++++++---
>  drivers/gpu/drm/imagination/pvr_fw_trace.h |   3 -
>  drivers/gpu/drm/imagination/pvr_params.c   | 147 ---------------------
>  drivers/gpu/drm/imagination/pvr_params.h   |  72 -----------
>  drivers/gpu/drm/imagination/pvr_test.c     |  73 +++++++++++
>  10 files changed, 389 insertions(+), 272 deletions(-)
> ---
> base-commit: 4a768c544f64eaa2fc7cfa91e46f43aa4aad0c40
> change-id: 20260107-device-support-info-f16d81b672d5
> 

For the whole serie,

Reviewed-by: Alessio Belle <alessio.belle@imgtec.com>

Thanks,
Alessio

Re: [PATCH 0/6] drm/imagination: Introduce hardware support check

[Matt Coster]

On Tue, 13 Jan 2026 10:16:38 +0000, Matt Coster wrote:
> We're seeing an influx of contributions to add support for lots of
> different hardware containing Imagination GPUs, and for that we're
> incredibly grateful.
> 
> Out of an abundance of caution, let's mark anything with intial support
> that isn't yet reasonably widely tested as "experimental".
> 
> [...]

Applied, thanks!

[1/6] drm/imagination: Simplify module parameters
      commit: a331631496a0af9a6f4e7e1860983afd8b1bb013
[2/6] drm/imagination: Validate fw trace group_mask
      commit: c6978643ea1c74c913f925c08ef9bafbdc031a04
[3/6] drm/imagination: Load FW trace config at init
      commit: ee184ab0ffb6cdd20527aa3b3729b824f52d3cd7
[4/6] drm/imagination: Add gpuid module parameter
      commit: 3bf74137340a1ced1566f4f9e9c2f08cba7bdf7c
[5/6] drm/imagination: KUnit test for pvr_gpuid_decode_string()
      commit: 3519e9ea13b49e7b37a20fa3a11a9e1fc5441af5
[6/6] drm/imagination: Warn or error on unsupported hardware
      commit: 1c21f240fbc1e47b94e68abfa2da2c01ed29a74d

Best regards,
-- 
Matt Coster <matt.coster@imgtec.com>

Re: [PATCH 6/6] drm/imagination: Warn or error on unsupported hardware

[Geert Uytterhoeven]

Hi Matt,

On Tue, 13 Jan 2026 at 11:21, Matt Coster <matt.coster@imgtec.com> wrote:
> Gate the use of unsupported hardware behind a new module parameter
> (exp_hw_support).
>
> Signed-off-by: Matt Coster <matt.coster@imgtec.com>

Thanks for your patch, which is now commit 1c21f240fbc1e47b
("drm/imagination: Warn or error on unsupported hardware")
in v7.0-rc1.

> --- a/drivers/gpu/drm/imagination/pvr_device.c
> +++ b/drivers/gpu/drm/imagination/pvr_device.c
> @@ -525,6 +525,77 @@ pvr_gpuid_decode_string(const struct pvr_device *pvr_dev,
>  }
>  EXPORT_SYMBOL_IF_KUNIT(pvr_gpuid_decode_string);
>
> +static bool pvr_exp_hw_support;
> +module_param_named(exp_hw_support, pvr_exp_hw_support, bool, 0600);
> +MODULE_PARM_DESC(exp_hw_support, "Bypass runtime checks for fully supported GPU cores. WARNING: enabling this option may result in a buggy, insecure, or otherwise unusable driver.");
> +
> +/**
> + * enum pvr_gpu_support_level - The level of support for a gpu_id in the current
> + * version of the driver.
> + *
> + * @PVR_GPU_UNKNOWN: Cores that are unknown to the driver. These may not even exist.
> + * @PVR_GPU_EXPERIMENTAL: Cores that have experimental support.
> + * @PVR_GPU_SUPPORTED: Cores that are supported and maintained.
> + */
> +enum pvr_gpu_support_level {
> +       PVR_GPU_UNKNOWN,
> +       PVR_GPU_EXPERIMENTAL,
> +       PVR_GPU_SUPPORTED,
> +};
> +
> +static enum pvr_gpu_support_level
> +pvr_gpu_support_level(const struct pvr_gpu_id *gpu_id)
> +{
> +       switch (pvr_gpu_id_to_packed_bvnc(gpu_id)) {
> +       case PVR_PACKED_BVNC(33, 15, 11, 3):
> +       case PVR_PACKED_BVNC(36, 53, 104, 796):
> +               return PVR_GPU_SUPPORTED;
> +
> +       case PVR_PACKED_BVNC(36, 52, 104, 182):
> +               return PVR_GPU_EXPERIMENTAL;
> +
> +       default:
> +               return PVR_GPU_UNKNOWN;
> +       }
> +}
> +
> +static int
> +pvr_check_gpu_supported(struct pvr_device *pvr_dev,
> +                       const struct pvr_gpu_id *gpu_id)
> +{
> +       struct drm_device *drm_dev = from_pvr_device(pvr_dev);
> +
> +       switch (pvr_gpu_support_level(gpu_id)) {
> +       case PVR_GPU_SUPPORTED:
> +               if (pvr_exp_hw_support)
> +                       drm_info(drm_dev, "Module parameter 'exp_hw_support' was set, but this hardware is fully supported by the current driver.");
> +
> +               break;
> +
> +       case PVR_GPU_EXPERIMENTAL:
> +               if (!pvr_exp_hw_support) {
> +                       drm_err(drm_dev, "Unsupported GPU! Set 'exp_hw_support' to bypass this check.");
> +                       return -ENODEV;
> +               }
> +
> +               drm_warn(drm_dev, "Running on unsupported hardware; you may encounter bugs!");
> +               break;
> +
> +       /* NOTE: This code path may indicate misbehaving hardware. */
> +       case PVR_GPU_UNKNOWN:
> +       default:
> +               if (!pvr_exp_hw_support) {
> +                       drm_err(drm_dev, "Unknown GPU! Set 'exp_hw_support' to bypass this check.");

After marking the gpu device node enabled in DTS for Renesas Salvator-X
with R-Car M3-W (as you are aware, it is still disabled upstream
because the system may crash when the firmware is not found), this
error message is triggered:

    powervr fd000000.gpu: [drm] *ERROR* Unknown GPU! Set
'exp_hw_support' to bypass this check.

and the driver fails to probe. Using the exp_hw_support flag makes
the driver probe (when the firmware is available):

    # echo 1 > /sys/module/powervr/parameters/exp_hw_support
    # echo fd000000.gpu > /sys/bus/platform/drivers/powervr/bind
    powervr fd000000.gpu: [drm] Running on unknown hardware; expect issues.
    powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
    powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
    [drm] Initialized powervr 1.0.0 for fd000000.gpu on minor 1

I am not sure if this counts as a regression.  Given firmware is
available for this device, I'd expect it to fall (at least) in the the
PVR_GPU_EXPERIMENTAL case?

Thanks!

> +                       return -ENODEV;
> +               }
> +
> +               drm_warn(drm_dev, "Running on unknown hardware; expect issues.");
> +               break;
> +       }
> +
> +       return 0;
> +}
> +
>  static char *pvr_gpuid_override;
>  module_param_named(gpuid, pvr_gpuid_override, charp, 0400);
>  MODULE_PARM_DESC(gpuid, "GPU ID (BVNC) to be used instead of the value read from hardware.");
> @@ -555,7 +626,7 @@ pvr_load_gpu_id(struct pvr_device *pvr_dev)
>                         return err;
>         }
>
> -       return 0;
> +       return pvr_check_gpu_supported(pvr_dev, gpu_id);
>  }
>
>  /**

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds