Re: [PATCH 1/9] tools/include: Sync uapi/drm/i915_drm.h with the kernel sources
Hello,
On Tue, Apr 9, 2024 at 3:14 AM Jani Nikula wrote:
>
> On Tue, 09 Apr 2024, Ingo Molnar wrote:
> > * Jani Nikula wrote:
> >
> >> On Mon, 08 Apr 2024, Namhyung Kim wrote:
> >> > To pick up changes from:
> >> >
> >> >b112364867499 ("drm/i915: Add GuC submission interface version query")
> >> >5cf0fbf763741 ("drm/i915: Add some boring kerneldoc")
> >> >
> >> > This should be used to beautify DRM syscall arguments and it addresses
> >> > these tools/perf build warnings:
> >> >
> >> > Warning: Kernel ABI header differences:
> >> > diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h
> >>
> >> All these years and I never realized there are header copies
> >> there. But... why copies?
> >
> > It's better than all the alternatives we tried so far:
> >
> > - Symbolic links and direct #includes: this was the original approach but
> >was pushed back on from the kernel side, when tooling modified the
> >headers and broke them accidentally for kernel builds.
> >
> > - Duplicate self-defined ABI headers like glibc: double the maintenance
> >burden, double the chance for mistakes, plus there's no tech-driven
> >notification mechanism to look at new kernel side changes.
> >
> > What we are doing now is a third option:
> >
> > - A software-enforced copy-on-write mechanism of kernel headers to
> >tooling, driven by non-fatal warnings on the tooling side build when
> >kernel headers get modified:
> >
> > Warning: Kernel ABI header differences:
> > diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h
> > diff -u tools/include/uapi/linux/fs.h include/uapi/linux/fs.h
> > diff -u tools/include/uapi/linux/kvm.h include/uapi/linux/kvm.h
> > ...
> >
> >The tooling policy is to always pick up the kernel side headers as-is,
> >and integate them into the tooling build. The warnings above serve as a
> >notification to tooling maintainers that there's changes on the kernel
> >side.
> >
> > We've been using this for many years now, and it might seem hacky, but
> > works surprisingly well.
> >
> > Does this make sense to you?
>
> Yes, although there are probably pieces of the puzzle I'm missing.
> Thanks for the explanation! (That might work almost as-is copied to
> tools/include/uapi/README. ;)
>
> It's also kind of funny to find this kind of back alleys of the kernel
> repo I've never wandered to before.
I have some explanation in the cover letter of the series but I forgot
to mention that in each commit message. Probably I can update the
explanation with Ingo's reply.
Thanks,
Namhyung
[PATCH 1/9] tools/include: Sync uapi/drm/i915_drm.h with the kernel sources
To pick up changes from:
b112364867499 ("drm/i915: Add GuC submission interface version query")
5cf0fbf763741 ("drm/i915: Add some boring kerneldoc")
This should be used to beautify DRM syscall arguments and it addresses
these tools/perf build warnings:
Warning: Kernel ABI header differences:
diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: David Airlie
Cc: Daniel Vetter
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Namhyung Kim
---
tools/include/uapi/drm/i915_drm.h | 16
1 file changed, 16 insertions(+)
diff --git a/tools/include/uapi/drm/i915_drm.h
b/tools/include/uapi/drm/i915_drm.h
index fd4f9574d177..2ee338860b7e 100644
--- a/tools/include/uapi/drm/i915_drm.h
+++ b/tools/include/uapi/drm/i915_drm.h
@@ -3013,6 +3013,7 @@ struct drm_i915_query_item {
* - %DRM_I915_QUERY_MEMORY_REGIONS (see struct
drm_i915_query_memory_regions)
* - %DRM_I915_QUERY_HWCONFIG_BLOB (see `GuC HWCONFIG blob uAPI`)
* - %DRM_I915_QUERY_GEOMETRY_SUBSLICES (see struct
drm_i915_query_topology_info)
+* - %DRM_I915_QUERY_GUC_SUBMISSION_VERSION (see struct
drm_i915_query_guc_submission_version)
*/
__u64 query_id;
#define DRM_I915_QUERY_TOPOLOGY_INFO 1
@@ -3021,6 +3022,7 @@ struct drm_i915_query_item {
#define DRM_I915_QUERY_MEMORY_REGIONS 4
#define DRM_I915_QUERY_HWCONFIG_BLOB 5
#define DRM_I915_QUERY_GEOMETRY_SUBSLICES 6
+#define DRM_I915_QUERY_GUC_SUBMISSION_VERSION 7
/* Must be kept compact -- no holes and well documented */
/**
@@ -3566,6 +3568,20 @@ struct drm_i915_query_memory_regions {
struct drm_i915_memory_region_info regions[];
};
+/**
+ * struct drm_i915_query_guc_submission_version - query GuC submission
interface version
+ */
+struct drm_i915_query_guc_submission_version {
+ /** @branch: Firmware branch version. */
+ __u32 branch;
+ /** @major: Firmware major version. */
+ __u32 major;
+ /** @minor: Firmware minor version. */
+ __u32 minor;
+ /** @patch: Firmware patch version. */
+ __u32 patch;
+};
+
/**
* DOC: GuC HWCONFIG blob uAPI
*
--
2.44.0.478.gd926399ef9-goog
[PATCH 01/14] tools headers UAPI: Update tools's copy of drm headers
tldr; Just FYI, I'm carrying this on the perf tools tree.
Full explanation:
There used to be no copies, with tools/ code using kernel headers
directly. From time to time tools/perf/ broke due to legitimate kernel
hacking. At some point Linus complained about such direct usage. Then we
adopted the current model.
The way these headers are used in perf are not restricted to just
including them to compile something.
There are sometimes used in scripts that convert defines into string
tables, etc, so some change may break one of these scripts, or new MSRs
may use some different #define pattern, etc.
E.g.:
$ ls -1 tools/perf/trace/beauty/*.sh | head -5
tools/perf/trace/beauty/arch_errno_names.sh
tools/perf/trace/beauty/drm_ioctl.sh
tools/perf/trace/beauty/fadvise.sh
tools/perf/trace/beauty/fsconfig.sh
tools/perf/trace/beauty/fsmount.sh
$
$ tools/perf/trace/beauty/fadvise.sh
static const char *fadvise_advices[] = {
[0] = "NORMAL",
[1] = "RANDOM",
[2] = "SEQUENTIAL",
[3] = "WILLNEED",
[4] = "DONTNEED",
[5] = "NOREUSE",
};
$
The tools/perf/check-headers.sh script, part of the tools/ build
process, points out changes in the original files.
So its important not to touch the copies in tools/ when doing changes in
the original kernel headers, that will be done later, when
check-headers.sh inform about the change to the perf tools hackers.
Cc: David Airlie
Cc: Daniel Vetter
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Namhyung Kim
---
tools/include/uapi/drm/drm.h | 20
tools/include/uapi/drm/i915_drm.h | 8
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/tools/include/uapi/drm/drm.h b/tools/include/uapi/drm/drm.h
index 794c1d857677..de723566c5ae 100644
--- a/tools/include/uapi/drm/drm.h
+++ b/tools/include/uapi/drm/drm.h
@@ -1134,6 +1134,26 @@ extern "C" {
#define DRM_IOCTL_MODE_PAGE_FLIP DRM_IOWR(0xB0, struct
drm_mode_crtc_page_flip)
#define DRM_IOCTL_MODE_DIRTYFB DRM_IOWR(0xB1, struct
drm_mode_fb_dirty_cmd)
+/**
+ * DRM_IOCTL_MODE_CREATE_DUMB - Create a new dumb buffer object.
+ *
+ * KMS dumb buffers provide a very primitive way to allocate a buffer object
+ * suitable for scanout and map it for software rendering. KMS dumb buffers are
+ * not suitable for hardware-accelerated rendering nor video decoding. KMS dumb
+ * buffers are not suitable to be displayed on any other device than the KMS
+ * device where they were allocated from. Also see
+ * :ref:`kms_dumb_buffer_objects`.
+ *
+ * The IOCTL argument is a struct drm_mode_create_dumb.
+ *
+ * User-space is expected to create a KMS dumb buffer via this IOCTL, then add
+ * it as a KMS framebuffer via _IOCTL_MODE_ADDFB and map it via
+ * _IOCTL_MODE_MAP_DUMB.
+ *
+ * _CAP_DUMB_BUFFER indicates whether this IOCTL is supported.
+ * _CAP_DUMB_PREFERRED_DEPTH and _CAP_DUMB_PREFER_SHADOW indicate
+ * driver preferences for dumb buffers.
+ */
#define DRM_IOCTL_MODE_CREATE_DUMB DRM_IOWR(0xB2, struct drm_mode_create_dumb)
#define DRM_IOCTL_MODE_MAP_DUMBDRM_IOWR(0xB3, struct drm_mode_map_dumb)
#define DRM_IOCTL_MODE_DESTROY_DUMBDRM_IOWR(0xB4, struct
drm_mode_destroy_dumb)
diff --git a/tools/include/uapi/drm/i915_drm.h
b/tools/include/uapi/drm/i915_drm.h
index 7000e5910a1d..218edb0a96f8 100644
--- a/tools/include/uapi/drm/i915_drm.h
+++ b/tools/include/uapi/drm/i915_drm.h
@@ -38,13 +38,13 @@ extern "C" {
*/
/**
- * DOC: uevents generated by i915 on it's device node
+ * DOC: uevents generated by i915 on its device node
*
* I915_L3_PARITY_UEVENT - Generated when the driver receives a parity mismatch
- * event from the gpu l3 cache. Additional information supplied is ROW,
+ * event from the GPU L3 cache. Additional information supplied is ROW,
* BANK, SUBBANK, SLICE of the affected cacheline. Userspace should keep
- * track of these events and if a specific cache-line seems to have a
- * persistent error remap it with the l3 remapping tool supplied in
+ * track of these events, and if a specific cache-line seems to have a
+ * persistent error, remap it with the L3 remapping tool supplied in
* intel-gpu-tools. The value supplied with the event is always 1.
*
* I915_ERROR_UEVENT - Generated upon error detection, currently only via
--
2.43.0.rc1.413.gea7ed67945-goog
