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 1/2] drm/imagination: Define packed BVNCs in the uapi
Date: Fri, 20 Mar 2026 15:22:12 +0000	[thread overview]
Message-ID: <20260320-bvnc-cleanup-v2-1-49c3c1453289@imgtec.com> (raw)
In-Reply-To: <20260320-bvnc-cleanup-v2-0-49c3c1453289@imgtec.com>

Using the uapi-safe __GENMASK_ULL(), we can stably define the layout of
64-bit packed BVNCs. These defs replace the replicated doc comment that
appears all over the place.

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

---
Changes in v2:
- Add _CONST variant of PVR_PACKED_BVNC() for use in case statements.
  This fixes the build issue reported by the test bot[1][2]
- Link to v1: https://patch.msgid.link/20260206-bvnc-cleanup-v1-1-f3c818541fbe@imgtec.com

[1]: https://lore.kernel.org/r/202602070204.PjaKNSpZ-lkp@intel.com/
[2]: https://lore.kernel.org/r/202602070500.zo0DrH6U-lkp@intel.com/
---
 drivers/gpu/drm/imagination/pvr_device.c |  6 ++---
 drivers/gpu/drm/imagination/pvr_device.h | 43 +++++++++++++-------------------
 include/uapi/drm/pvr_drm.h               | 14 +++++------
 3 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c
index f58bb66a6327..9b26585a42bf 100644
--- a/drivers/gpu/drm/imagination/pvr_device.c
+++ b/drivers/gpu/drm/imagination/pvr_device.c
@@ -547,11 +547,11 @@ 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):
+	case PVR_PACKED_BVNC_CONST(33, 15, 11, 3):
+	case PVR_PACKED_BVNC_CONST(36, 53, 104, 796):
 		return PVR_GPU_SUPPORTED;
 
-	case PVR_PACKED_BVNC(36, 52, 104, 182):
+	case PVR_PACKED_BVNC_CONST(36, 52, 104, 182):
 		return PVR_GPU_EXPERIMENTAL;
 
 	default:
diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
index d51c57cf9332..55d8ff11d507 100644
--- a/drivers/gpu/drm/imagination/pvr_device.h
+++ b/drivers/gpu/drm/imagination/pvr_device.h
@@ -14,7 +14,7 @@
 #include <drm/drm_file.h>
 #include <drm/drm_mm.h>
 
-#include <linux/bits.h>
+#include <linux/bitfield.h>
 #include <linux/compiler_attributes.h>
 #include <linux/compiler_types.h>
 #include <linux/device.h>
@@ -470,6 +470,12 @@ struct pvr_file {
 
 #define to_pvr_file(file) ((file)->driver_priv)
 
+#define __PVR_PACKED_BVNC(prep, b, v, n, c) \
+	(prep(DRM_PVR_BVNC_B, b) | \
+	 prep(DRM_PVR_BVNC_V, v) | \
+	 prep(DRM_PVR_BVNC_N, n) | \
+	 prep(DRM_PVR_BVNC_C, c))
+
 /**
  * PVR_PACKED_BVNC() - Packs B, V, N and C values into a 64-bit unsigned integer
  * @b: Branch ID.
@@ -477,39 +483,24 @@ struct pvr_file {
  * @n: Number of scalable units.
  * @c: Config ID.
  *
- * The packed layout is as follows:
- *
- *    +--------+--------+--------+-------+
- *    | 63..48 | 47..32 | 31..16 | 15..0 |
- *    +========+========+========+=======+
- *    | B      | V      | N      | C     |
- *    +--------+--------+--------+-------+
+ * The packed layout follows the bitfield defined by the DRM_PVR_BVNC_* macros.
  *
  * pvr_gpu_id_to_packed_bvnc() should be used instead of this macro when a
  * &struct pvr_gpu_id is available in order to ensure proper type checking.
  *
  * Return: Packed BVNC.
  */
-/* clang-format off */
-#define PVR_PACKED_BVNC(b, v, n, c) \
-	((((u64)(b) & GENMASK_ULL(15, 0)) << 48) | \
-	 (((u64)(v) & GENMASK_ULL(15, 0)) << 32) | \
-	 (((u64)(n) & GENMASK_ULL(15, 0)) << 16) | \
-	 (((u64)(c) & GENMASK_ULL(15, 0)) <<  0))
-/* clang-format on */
+#define PVR_PACKED_BVNC(b, v, n, c) __PVR_PACKED_BVNC(FIELD_PREP, b, v, n, c)
+
+/** PVR_PACKED_BVNC_CONST() - Compile-time equivalent of PVR_PACKED_BVNC(). */
+#define PVR_PACKED_BVNC_CONST(b, v, n, c) __PVR_PACKED_BVNC(FIELD_PREP_CONST, b, v, n, c)
 
 /**
  * pvr_gpu_id_to_packed_bvnc() - Packs B, V, N and C values into a 64-bit
  * unsigned integer
  * @gpu_id: GPU ID.
  *
- * The packed layout is as follows:
- *
- *    +--------+--------+--------+-------+
- *    | 63..48 | 47..32 | 31..16 | 15..0 |
- *    +========+========+========+=======+
- *    | B      | V      | N      | C     |
- *    +--------+--------+--------+-------+
+ * The packed layout follows the bitfield defined by the DRM_PVR_BVNC_* macros.
  *
  * This should be used in preference to PVR_PACKED_BVNC() when a &struct
  * pvr_gpu_id is available in order to ensure proper type checking.
@@ -525,10 +516,10 @@ pvr_gpu_id_to_packed_bvnc(const struct pvr_gpu_id *gpu_id)
 static __always_inline void
 packed_bvnc_to_pvr_gpu_id(u64 bvnc, struct pvr_gpu_id *gpu_id)
 {
-	gpu_id->b = (bvnc & GENMASK_ULL(63, 48)) >> 48;
-	gpu_id->v = (bvnc & GENMASK_ULL(47, 32)) >> 32;
-	gpu_id->n = (bvnc & GENMASK_ULL(31, 16)) >> 16;
-	gpu_id->c = bvnc & GENMASK_ULL(15, 0);
+	gpu_id->b = FIELD_GET(DRM_PVR_BVNC_B, bvnc);
+	gpu_id->v = FIELD_GET(DRM_PVR_BVNC_V, bvnc);
+	gpu_id->n = FIELD_GET(DRM_PVR_BVNC_N, bvnc);
+	gpu_id->c = FIELD_GET(DRM_PVR_BVNC_C, bvnc);
 }
 
 int pvr_device_init(struct pvr_device *pvr_dev);
diff --git a/include/uapi/drm/pvr_drm.h b/include/uapi/drm/pvr_drm.h
index ccf6c2112468..72f3f90560cf 100644
--- a/include/uapi/drm/pvr_drm.h
+++ b/include/uapi/drm/pvr_drm.h
@@ -6,6 +6,7 @@
 
 #include "drm.h"
 
+#include <linux/bits.h>
 #include <linux/const.h>
 #include <linux/types.h>
 
@@ -113,6 +114,11 @@ struct drm_pvr_obj_array {
  * DOC: PowerVR IOCTL DEV_QUERY interface
  */
 
+#define DRM_PVR_BVNC_B __GENMASK_ULL(63, 48)
+#define DRM_PVR_BVNC_V __GENMASK_ULL(47, 32)
+#define DRM_PVR_BVNC_N __GENMASK_ULL(31, 16)
+#define DRM_PVR_BVNC_C __GENMASK_ULL(15, 0)
+
 /**
  * struct drm_pvr_dev_query_gpu_info - Container used to fetch information about
  * the graphics processor.
@@ -125,13 +131,7 @@ struct drm_pvr_dev_query_gpu_info {
 	 * @gpu_id: GPU identifier.
 	 *
 	 * For all currently supported GPUs this is the BVNC encoded as a 64-bit
-	 * value as follows:
-	 *
-	 *    +--------+--------+--------+-------+
-	 *    | 63..48 | 47..32 | 31..16 | 15..0 |
-	 *    +========+========+========+=======+
-	 *    | B      | V      | N      | C     |
-	 *    +--------+--------+--------+-------+
+	 * value using the DRM_PVR_BVNC_* bitmasks.
 	 */
 	__u64 gpu_id;
 

-- 
2.53.0


  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 ` Matt Coster [this message]
2026-03-21 17:38   ` Claude review: drm/imagination: Define packed BVNCs in the uapi Claude Code Review Bot
2026-03-20 15:22 ` [PATCH v2 2/2] drm/imagination: Add PVR_GPU_ID_FMT to format pvr_gpu_id Matt Coster
2026-03-21 17:38   ` Claude review: " 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-1-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