public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
From: Matt Coster <matt.coster@imgtec.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>
Cc: Frank Binns <frank.binns@imgtec.com>,
	Brajesh Gupta <brajesh.gupta@imgtec.com>,
	Alessio Belle <alessio.belle@imgtec.com>,
	Alexandru Dadu <alexandru.dadu@imgtec.com>,
	<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
	"Matt Coster" <matt.coster@imgtec.com>
Subject: [PATCH v2 2/2] drm/imagination: Add PVR_GPU_ID_FMT to format pvr_gpu_id
Date: Fri, 20 Mar 2026 15:22:13 +0000	[thread overview]
Message-ID: <20260320-bvnc-cleanup-v2-2-49c3c1453289@imgtec.com> (raw)
In-Reply-To: <20260320-bvnc-cleanup-v2-0-49c3c1453289@imgtec.com>

There are currently two different combinations of format specifiers used to
print a struct pvr_gpu_id, one using %i and one using %d. Both of these
are technically incorrect since the components are stored as u16.

Introduce macros to simplify and correct the formatting of these values:
 - PVR_GPU_ID_FMT: A pre-constructed format string fragment, in the style
   of PRIu32.
 - PVR_GPU_ID_FMT_ARGS(): Accepts a &struct pvr_gpu_id and expands the
   fields into appropriate format arguments to be used with PVR_GPU_ID_FMT.
 - PVR_GPU_ID_FMT_ARGS_PACKED(): Accepts a packed GPU ID as a u64 and
   extracts the components directly into appropriate format arguments to
   be used with PVR_GPU_ID_FMT.

Signed-off-by: Matt Coster <matt.coster@imgtec.com>

---
Changes in v2:
- Simplify the change made to pvr_fw_validate()
- Link to v1: https://patch.msgid.link/20260206-bvnc-cleanup-v1-2-f3c818541fbe@imgtec.com
---
 drivers/gpu/drm/imagination/pvr_device.c | 4 ++--
 drivers/gpu/drm/imagination/pvr_device.h | 6 ++++++
 drivers/gpu/drm/imagination/pvr_fw.c     | 9 +++------
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
index 9b26585a42bf..263d1badacf3 100644
--- a/drivers/gpu/drm/imagination/pvr_device.c
+++ b/drivers/gpu/drm/imagination/pvr_device.c
@@ -362,8 +362,8 @@ pvr_build_firmware_filename(struct pvr_device *pvr_dev, const char *base,
 {
 	struct pvr_gpu_id *gpu_id = &pvr_dev->gpu_id;
 
-	return kasprintf(GFP_KERNEL, "%s_%d.%d.%d.%d_v%d.fw", base, gpu_id->b,
-			 gpu_id->v, gpu_id->n, gpu_id->c, major);
+	return kasprintf(GFP_KERNEL, "%s_" PVR_GPU_ID_FMT "_v%d.fw", base,
+			 PVR_GPU_ID_FMT_ARGS(gpu_id), major);
 }
 
 static void
diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
index 55d8ff11d507..54e3e947e60e 100644
--- a/drivers/gpu/drm/imagination/pvr_device.h
+++ b/drivers/gpu/drm/imagination/pvr_device.h
@@ -53,6 +53,12 @@ struct pvr_gpu_id {
 	u16 b, v, n, c;
 };
 
+#define PVR_GPU_ID_FMT "%u.%u.%u.%u"
+#define PVR_GPU_ID_FMT_ARGS(gpu_id) (gpu_id)->b, (gpu_id)->v, (gpu_id)->n, (gpu_id)->c
+#define PVR_GPU_ID_FMT_ARGS_PACKED(gpu_id) \
+	(u32)FIELD_GET(DRM_PVR_BVNC_B, gpu_id), (u32)FIELD_GET(DRM_PVR_BVNC_V, gpu_id), \
+	(u32)FIELD_GET(DRM_PVR_BVNC_N, gpu_id), (u32)FIELD_GET(DRM_PVR_BVNC_C, gpu_id)
+
 /**
  * struct pvr_fw_version - Firmware version information
  * @major: Major version number.
diff --git a/drivers/gpu/drm/imagination/pvr_fw.c b/drivers/gpu/drm/imagination/pvr_fw.c
index 288516dc2560..0b1c2a0d950c 100644
--- a/drivers/gpu/drm/imagination/pvr_fw.c
+++ b/drivers/gpu/drm/imagination/pvr_fw.c
@@ -126,12 +126,9 @@ pvr_fw_validate(struct pvr_device *pvr_dev)
 	}
 
 	if (pvr_gpu_id_to_packed_bvnc(&pvr_dev->gpu_id) != header->bvnc) {
-		struct pvr_gpu_id fw_gpu_id;
-
-		packed_bvnc_to_pvr_gpu_id(header->bvnc, &fw_gpu_id);
-		drm_err(drm_dev, "FW built for incorrect GPU ID %i.%i.%i.%i (expected %i.%i.%i.%i)\n",
-			fw_gpu_id.b, fw_gpu_id.v, fw_gpu_id.n, fw_gpu_id.c,
-			pvr_dev->gpu_id.b, pvr_dev->gpu_id.v, pvr_dev->gpu_id.n, pvr_dev->gpu_id.c);
+		drm_err(drm_dev, "FW built for incorrect GPU ID " PVR_GPU_ID_FMT " (expected " PVR_GPU_ID_FMT ")\n",
+			PVR_GPU_ID_FMT_ARGS_PACKED(header->bvnc),
+			PVR_GPU_ID_FMT_ARGS(&pvr_dev->gpu_id));
 		return -EINVAL;
 	}
 

-- 
2.53.0


  parent reply	other threads:[~2026-03-20 15:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-20 15:22 [PATCH v2 0/2] drm/imagination: GPU_ID-related cleanups Matt Coster
2026-03-20 15:22 ` [PATCH v2 1/2] drm/imagination: Define packed BVNCs in the uapi Matt Coster
2026-03-21 17:38   ` Claude review: " Claude Code Review Bot
2026-03-20 15:22 ` Matt Coster [this message]
2026-03-21 17:38   ` Claude review: drm/imagination: Add PVR_GPU_ID_FMT to format pvr_gpu_id Claude Code Review Bot
2026-03-21 17:38 ` Claude review: drm/imagination: GPU_ID-related cleanups Claude Code Review Bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260320-bvnc-cleanup-v2-2-49c3c1453289@imgtec.com \
    --to=matt.coster@imgtec.com \
    --cc=airlied@gmail.com \
    --cc=alessio.belle@imgtec.com \
    --cc=alexandru.dadu@imgtec.com \
    --cc=brajesh.gupta@imgtec.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=frank.binns@imgtec.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox