[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: dbuf cleanups
== Series Details == Series: drm/i915: dbuf cleanups URL : https://patchwork.freedesktop.org/series/89171/ State : success == Summary == CI Bug Log - changes from CI_DRM_9978_full -> Patchwork_19944_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_19944_full that come from known issues: ### IGT changes ### Issues hit * igt@feature_discovery@display-3x: - shard-glk: NOTRUN -> [SKIP][1] ([fdo#109271]) +13 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-glk7/igt@feature_discov...@display-3x.html * igt@gem_create@create-massive: - shard-snb: NOTRUN -> [DMESG-WARN][2] ([i915#3002]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-snb6/igt@gem_cre...@create-massive.html * igt@gem_ctx_persistence@legacy-engines-mixed: - shard-snb: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#1099]) +5 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-snb6/igt@gem_ctx_persiste...@legacy-engines-mixed.html * igt@gem_eio@unwedge-stress: - shard-snb: NOTRUN -> [FAIL][4] ([i915#3354]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-snb6/igt@gem_...@unwedge-stress.html * igt@gem_exec_fair@basic-none-vip@rcs0: - shard-kbl: [PASS][5] -> [FAIL][6] ([i915#2842]) +3 similar issues [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9978/shard-kbl4/igt@gem_exec_fair@basic-none-...@rcs0.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-kbl7/igt@gem_exec_fair@basic-none-...@rcs0.html * igt@gem_exec_fair@basic-none@vcs1: - shard-iclb: NOTRUN -> [FAIL][7] ([i915#2842]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-iclb4/igt@gem_exec_fair@basic-n...@vcs1.html * igt@gem_exec_fair@basic-pace-share@rcs0: - shard-glk: [PASS][8] -> [FAIL][9] ([i915#2842]) +1 similar issue [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9978/shard-glk9/igt@gem_exec_fair@basic-pace-sh...@rcs0.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-glk4/igt@gem_exec_fair@basic-pace-sh...@rcs0.html * igt@gem_mmap_gtt@big-copy: - shard-skl: [PASS][10] -> [FAIL][11] ([i915#307]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9978/shard-skl3/igt@gem_mmap_...@big-copy.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-skl5/igt@gem_mmap_...@big-copy.html * igt@gem_pread@exhaustion: - shard-snb: NOTRUN -> [WARN][12] ([i915#2658]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-snb6/igt@gem_pr...@exhaustion.html * igt@gem_pwrite@basic-exhaustion: - shard-apl: NOTRUN -> [WARN][13] ([i915#2658]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-apl6/igt@gem_pwr...@basic-exhaustion.html * igt@gem_render_copy@x-tiled-to-vebox-yf-tiled: - shard-kbl: NOTRUN -> [SKIP][14] ([fdo#109271]) +62 similar issues [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-kbl3/igt@gem_render_c...@x-tiled-to-vebox-yf-tiled.html * igt@gem_userptr_blits@vma-merge: - shard-snb: NOTRUN -> [FAIL][15] ([i915#2724]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-snb6/igt@gem_userptr_bl...@vma-merge.html * igt@gem_vm_create@destroy-race: - shard-tglb: [PASS][16] -> [TIMEOUT][17] ([i915#2795]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9978/shard-tglb2/igt@gem_vm_cre...@destroy-race.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-tglb8/igt@gem_vm_cre...@destroy-race.html * igt@kms_ccs@pipe-c-bad-rotation-90: - shard-skl: NOTRUN -> [SKIP][18] ([fdo#109271] / [fdo#111304]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-skl4/igt@kms_...@pipe-c-bad-rotation-90.html * igt@kms_chamelium@hdmi-hpd: - shard-skl: NOTRUN -> [SKIP][19] ([fdo#109271] / [fdo#111827]) +3 similar issues [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-skl8/igt@kms_chamel...@hdmi-hpd.html * igt@kms_chamelium@hdmi-hpd-for-each-pipe: - shard-kbl: NOTRUN -> [SKIP][20] ([fdo#109271] / [fdo#111827]) +5 similar issues [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-kbl3/igt@kms_chamel...@hdmi-hpd-for-each-pipe.html * igt@kms_color@pipe-b-ctm-0-5: - shard-skl: [PASS][21] -> [DMESG-WARN][22] ([i915#1982]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9978/shard-skl3/igt@kms_co...@pipe-b-ctm-0-5.html [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/shard-skl5/igt@kms_co...@pipe-b-ctm-0-5.html * igt@kms_color_chamelium@pipe-a-ctm-limited-range:
Re: [Intel-gfx] [Mesa-dev] [PATCH v3 3/4] drm/i915/uapi: convert i915_query and friend to kernel doc
Daniel Vetter writes: > On Fri, Apr 16, 2021 at 12:25 AM Ian Romanick wrote: >> Since we just had a big discussion about this on mesa-dev w.r.t. Mesa >> code and documentation... does the kernel have a policy about which >> flavor (pun intended) of English should be used? > > I'm not finding it documented in > https://dri.freedesktop.org/docs/drm/doc-guide/sphinx.html but I > thought we've discussed it. Adding linux-doc and Jon Corbet. It's in Documentation/doc-guide/contributing.rst: > Please note that some things are *not* typos and should not be "fixed": > > - Both American and British English spellings are allowed within the >kernel documentation. There is no need to fix one by replacing it with >the other. Thanks, jon ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915/uapi: convert i915_query and friend to kernel doc
Reviewed-by: Jason Ekstrand
On April 16, 2021 05:37:55 Matthew Auld wrote:
Add a note about the two-step process.
v2(Tvrtko):
- Also document the other method of just passing in a buffer which is
large enough, which avoids two ioctl calls. Can make sense for
smaller query items.
Suggested-by: Daniel Vetter
Signed-off-by: Matthew Auld
Cc: Joonas Lahtinen
Cc: Tvrtko Ursulin
Cc: Jordan Justen
Cc: Daniel Vetter
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Dave Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
Reviewed-by: Daniel Vetter
---
include/uapi/drm/i915_drm.h | 61 ++---
1 file changed, 50 insertions(+), 11 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index d79b51c12ff2..12f375c52317 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2218,14 +2218,23 @@ struct drm_i915_perf_oa_config {
__u64 flex_regs_ptr;
};
+/**
+ * struct drm_i915_query_item - An individual query for the kernel to process.
+ *
+ * The behaviour is determined by the @query_id. Note that exactly what
+ * @data_ptr is also depends on the specific @query_id.
+ */
struct drm_i915_query_item {
+ /** @query_id: The id for this query */
__u64 query_id;
#define DRM_I915_QUERY_TOPOLOGY_INFO1
#define DRM_I915_QUERY_ENGINE_INFO 2
#define DRM_I915_QUERY_PERF_CONFIG 3
/* Must be kept compact -- no holes and well documented */
- /*
+ /**
+* @length:
+*
* When set to zero by userspace, this is filled with the size of the
* data to be written at the data_ptr pointer. The kernel sets this
* value to a negative value to signal an error on a particular query
@@ -2233,21 +2242,26 @@ struct drm_i915_query_item {
*/
__s32 length;
- /*
+ /**
+* @flags:
+*
* When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
*
* When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
-* following :
-* - DRM_I915_QUERY_PERF_CONFIG_LIST
-* - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
-* - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
+* following:
+*
+* - DRM_I915_QUERY_PERF_CONFIG_LIST
+* - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
+* - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
*/
__u32 flags;
#define DRM_I915_QUERY_PERF_CONFIG_LIST 1
#define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
#define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID 3
- /*
+ /**
+* @data_ptr:
+*
* Data will be written at the location pointed by data_ptr when the
* value of length matches the length of the data to be written by the
* kernel.
@@ -2255,16 +2269,41 @@ struct drm_i915_query_item {
__u64 data_ptr;
};
+/**
+ * struct drm_i915_query - Supply an array of drm_i915_query_item for the
kernel
+ * to fill out.
+ *
+ * Note that this is generally a two step process for each drm_i915_query_item
+ * in the array:
+ *
+ * 1. Call the DRM_IOCTL_I915_QUERY, giving it our array of
drm_i915_query_item,
+ *with drm_i915_query_item.size set to zero. The kernel will then fill in
+ *the size, in bytes, which tells userspace how memory it needs to
allocate
+ *for the blob(say for an array of properties).
+ *
+ * 2. Next we call DRM_IOCTL_I915_QUERY again, this time with the
+ *drm_i915_query_item.data_ptr equal to our newly allocated blob. Note
that
+ *the i915_query_item.size should still be the same as what the kernel
+ *previously set. At this point the kernel can fill in the blob.
+ *
+ * Note that for some query items it can make sense for userspace to just pass
+ * in a buffer/blob equal to or larger than the required size. In this
case only
+ * a single ioctl call is needed. For some smaller query items this can work
+ * quite well.
+ *
+ */
struct drm_i915_query {
+ /** @num_items: The number of elements in the @items_ptr array */
__u32 num_items;
- /*
-* Unused for now. Must be cleared to zero.
+ /**
+* @flags: Unused for now. Must be cleared to zero.
*/
__u32 flags;
- /*
-* This points to an array of num_items drm_i915_query_item structures.
+ /**
+* @items_ptr: This points to an array of num_items drm_i915_query_item
+* structures.
*/
__u64 items_ptr;
};
--
2.26.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915/uapi: convert i915_user_extension to kernel doc
On April 16, 2021 05:37:52 Matthew Auld wrote:
Add some example usage for the extension chaining also, which is quite
nifty.
v2: (Daniel)
- clarify that the name is just some integer, also document that the
name space is not global
Suggested-by: Daniel Vetter
Signed-off-by: Matthew Auld
Cc: Joonas Lahtinen
Cc: Jordan Justen
Cc: Daniel Vetter
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Dave Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
Reviewed-by: Daniel Vetter
---
include/uapi/drm/i915_drm.h | 54 ++---
1 file changed, 50 insertions(+), 4 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 92da48e935d1..d79b51c12ff2 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -62,8 +62,8 @@ extern "C" {
#define I915_ERROR_UEVENT "ERROR"
#define I915_RESET_UEVENT "RESET"
-/*
- * i915_user_extension: Base class for defining a chain of extensions
+/**
+ * struct i915_user_extension - Base class for defining a chain of extensions
*
* Many interfaces need to grow over time. In most cases we can simply
* extend the struct and have userspace pass in more data. Another option,
@@ -76,12 +76,58 @@ extern "C" {
* increasing complexity, and for large parts of that interface to be
* entirely optional. The downside is more pointer chasing; chasing across
* the __user boundary with pointers encapsulated inside u64.
+ *
+ * Example chaining:
+ *
+ * .. code-block:: C
+ *
+ * struct i915_user_extension ext3 {
+ * .next_extension = 0, // end
+ * .name = ...,
+ * };
+ * struct i915_user_extension ext2 {
+ * .next_extension = (uintptr_t),
+ * .name = ...,
+ * };
+ * struct i915_user_extension ext1 {
+ * .next_extension = (uintptr_t),
+ * .name = ...,
+ * };
+ *
+ * Typically the i915_user_extension would be embedded in some uAPI
struct, and
+ * in this case we would feed it the head of the chain(i.e ext1), which would
+ * then apply all of the above extensions.
+ *
*/
struct i915_user_extension {
+ /**
+ * @next_extension:
+ *
+ * Pointer to the next struct i915_user_extension, or zero if the end.
+ */
__u64 next_extension;
+ /**
+ * @name: Name of the extension.
+ *
+ * Note that the name here is just some integer.
+ *
+ * Also note that the name space for this is not global for the whole
+ * driver, but rather its scope/meaning is limited to the specific piece
+ * of uAPI which has embedded the struct i915_user_extension.
We may want to rethink this decision. In Vulkan, we have several cases
where we use the same extension multiple places. Given that the only
extensible thing currently landed is context creation, we still could make
this global. Then again, forcing us to go through the exercise of
redefining every time has its advantages too.
In any case, this is a correct description of the current state of affairs, so
Reviewed-by: Jason Ekstrand
+ */
__u32 name;
- __u32 flags; /* All undefined bits must be zero. */
- __u32 rsvd[4]; /* Reserved for future use; must be zero. */
+ /**
+ * @flags: MBZ
+ *
+ * All undefined bits must be zero.
+ */
+ __u32 flags;
+ /**
+ * @rsvd: MBZ
+ *
+ * Reserved for future use; must be zero.
+ */
+ __u32 rsvd[4];
};
/*
--
2.26.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: dbuf cleanups
== Series Details ==
Series: drm/i915: dbuf cleanups
URL : https://patchwork.freedesktop.org/series/89171/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_9978 -> Patchwork_19944
Summary
---
**SUCCESS**
No regressions found.
External URL:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/index.html
Known issues
Here are the changes found in Patchwork_19944 that come from known issues:
### IGT changes ###
Possible fixes
* igt@core_hotunplug@unbind-rebind:
- fi-bwr-2160:[FAIL][1] ([i915#3194]) -> [PASS][2]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9978/fi-bwr-2160/igt@core_hotunp...@unbind-rebind.html
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/fi-bwr-2160/igt@core_hotunp...@unbind-rebind.html
* igt@i915_selftest@live@gt_timelines:
- fi-skl-6600u: [DMESG-WARN][3] -> [PASS][4]
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9978/fi-skl-6600u/igt@i915_selftest@live@gt_timelines.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/fi-skl-6600u/igt@i915_selftest@live@gt_timelines.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#3194]: https://gitlab.freedesktop.org/drm/intel/issues/3194
[i915#3277]: https://gitlab.freedesktop.org/drm/intel/issues/3277
[i915#3283]: https://gitlab.freedesktop.org/drm/intel/issues/3283
Participating hosts (44 -> 41)
--
Missing(3): fi-icl-y fi-bsw-cyan fi-bdw-samus
Build changes
-
* Linux: CI_DRM_9978 -> Patchwork_19944
CI-20190529: 20190529
CI_DRM_9978: 732dea807fffac8bed7696a335973c95f810e4b1 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_6067: 14317b92a672d9a20cd04fc3b0c80e2fb12d51d5 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_19944: bb42f353dfe4f00c428649ce0ac736b6975bd237 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
bb42f353dfe4 drm/i915: Say "enable foo" instead of "set foo to enabled"
b571b996abd4 drm/i915: Add enabledisable()
bbfbba08f9aa drm/i915: Polish for_each_dbuf_slice()
407ca0202e11 drm/i915: Use intel_de_rmw() for DBUF_POWER_REQUEST
4bec22613175 drm/i915: Use intel_dbuf_slice_size()
6182af680aea drm/i915: Store dbuf slice mask in device info
3bcf75a562a4 drm/i915: Handle dbuf bypass path allocation earlier
c4e158b6022a drm/i915: Collect dbuf device info into a sub-struct
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19944/index.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 4/4] drm/doc/rfc: i915 DG1 uAPI
On 4/16/2021 10:02 AM, Daniel Vetter wrote:
On Fri, Apr 16, 2021 at 6:38 PM Jason Ekstrand wrote:
On Thu, Apr 15, 2021 at 11:04 AM Matthew Auld wrote:
Add an entry for the new uAPI needed for DG1.
v2(Daniel):
- include the overall upstreaming plan
- add a note for mmap, there are differences here for TTM vs i915
- bunch of other suggestions from Daniel
v3:
(Daniel)
- add a note for set/get caching stuff
- add some more docs for existing query and extensions stuff
- add an actual code example for regions query
- bunch of other stuff
(Jason)
- uAPI change(!):
- try a simpler design with the placements extension
- rather than have a generic setparam which can cover multiple
use cases, have each extension be responsible for one thing
only
Signed-off-by: Matthew Auld
Cc: Joonas Lahtinen
Cc: Jordan Justen
Cc: Daniel Vetter
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Dave Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
---
Documentation/gpu/rfc/i915_gem_lmem.h | 255
Documentation/gpu/rfc/i915_gem_lmem.rst | 139 +
Documentation/gpu/rfc/index.rst | 4 +
3 files changed, 398 insertions(+)
create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h
create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.rst
diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h
b/Documentation/gpu/rfc/i915_gem_lmem.h
new file mode 100644
index ..2a82a452e9f2
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_gem_lmem.h
@@ -0,0 +1,255 @@
+/*
+ * Note that drm_i915_query_item and drm_i915_query are existing bits of uAPI.
+ * For the regions query we are just adding a new query id, so no actual new
+ * ioctl or anything, but including it here for reference.
+ */
+struct drm_i915_query_item {
+#define DRM_I915_QUERY_MEMORY_REGIONS 0xdeadbeaf
+
+__u64 query_id;
+
+/*
+ * When set to zero by userspace, this is filled with the size of the
+ * data to be written at the data_ptr pointer. The kernel sets this
+ * value to a negative value to signal an error on a particular query
+ * item.
+ */
+__s32 length;
+
+__u32 flags;
+/*
+ * Data will be written at the location pointed by data_ptr when the
+ * value of length matches the length of the data to be written by the
+ * kernel.
+ */
+__u64 data_ptr;
+};
+
+struct drm_i915_query {
+__u32 num_items;
+/*
+ * Unused for now. Must be cleared to zero.
+ */
+__u32 flags;
+/*
+ * This points to an array of num_items drm_i915_query_item structures.
+ */
+__u64 items_ptr;
+};
+
+#define DRM_IOCTL_I915_QUERY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_QUERY,
struct drm_i915_query)
+
+/**
+ * enum drm_i915_gem_memory_class
+ */
+enum drm_i915_gem_memory_class {
+ /** @I915_MEMORY_CLASS_SYSTEM: system memory */
+ I915_MEMORY_CLASS_SYSTEM = 0,
+ /** @I915_MEMORY_CLASS_DEVICE: device local-memory */
+ I915_MEMORY_CLASS_DEVICE,
+};
+
+/**
+ * struct drm_i915_gem_memory_class_instance
+ */
+struct drm_i915_gem_memory_class_instance {
+ /** @memory_class: see enum drm_i915_gem_memory_class */
+ __u16 memory_class;
+
+ /** @memory_instance: which instance */
+ __u16 memory_instance;
+};
+
+/**
+ * struct drm_i915_memory_region_info
+ *
+ * Describes one region as known to the driver.
+ *
+ * Note that we reserve quite a lot of stuff here for potential future work. As
+ * an example we might want expose the capabilities(see caps) for a given
+ * region, which could include things like if the region is CPU
+ * mappable/accessible etc.
I get caps but I'm seriously at a loss as to what the rest of this
would be used for. Why are caps and flags both there and separate?
Flags are typically something you set, not query. Also, what's with
rsvd1 at the end? This smells of substantial over-building to me.
I thought to myself, "maybe I'm missing a future use-case" so I looked
at the internal tree and none of this is being used there either.
This indicates to me that either I'm missing something and there's
code somewhere I don't know about or, with three years of building on
internal branches, we still haven't proven that any of this is needed.
If it's the latter, which I strongly suspect, maybe we should drop the
unnecessary bits and only add them back in if and when we have proof
that they're useful.
To be clear, I don't mind the query API as such and the class/instance
stuff seems fine and I really like being able to get the sizes
directly. What concerns me is all this extra future-proofing that we
have zero proof is actually useful. In my experience, when you build
out like this without so much as a use-case, you always end up
building the wrong thing.
+ */
+struct
[Intel-gfx] ✗ Fi.CI.DOCS: warning for drm/i915: dbuf cleanups
== Series Details == Series: drm/i915: dbuf cleanups URL : https://patchwork.freedesktop.org/series/89171/ State : warning == Summary == $ make htmldocs 2>&1 > /dev/null | grep i915 ./drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:102: warning: Function parameter or member 'ww' not described in 'i915_gem_shrink' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function parameter 'trampoline' description in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'jump_whitelist' not described in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'shadow_map' not described in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'batch_map' not described in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function parameter 'trampoline' description in 'intel_engine_cmd_parser' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: dbuf cleanups
== Series Details == Series: drm/i915: dbuf cleanups URL : https://patchwork.freedesktop.org/series/89171/ State : warning == Summary == $ dim sparse --fast origin/drm-tip Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. - +drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31 +drivers/gpu/drm/i915/gt/intel_reset.c:1329:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block +drivers/gpu/drm/i915/gt/intel_ring_submission.c:1203:24: warning: Using plain integer as NULL pointer +drivers/gpu/drm/i915/gvt/mmio.c:295:23: warning: memcpy with byte count of 279040 +drivers/gpu/drm/i915/i915_perf.c:1434:15: warning: memset with byte count of 16777216 +drivers/gpu/drm/i915/i915_perf.c:1488:15: warning: memset with byte count of 16777216 +drivers/gpu/drm/i915/intel_wakeref.c:137:19: warning: context imbalance in 'wakeref_auto_timeout' - unexpected unlock +drivers/gpu/drm/i915/selftests/i915_syncmap.c:80:54: warning: dubious: x | !y +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block +./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: dbuf cleanups
== Series Details == Series: drm/i915: dbuf cleanups URL : https://patchwork.freedesktop.org/series/89171/ State : warning == Summary == $ dim checkpatch origin/drm-tip c4e158b6022a drm/i915: Collect dbuf device info into a sub-struct 3bcf75a562a4 drm/i915: Handle dbuf bypass path allocation earlier -:33: WARNING:LINE_CONTINUATIONS: Avoid unnecessary line continuations #33: FILE: drivers/gpu/drm/i915/i915_pci.c:723: + .dbuf.size = 512 - 4, /* 4 blocks for bypass path allocation */ \ -:41: WARNING:LINE_CONTINUATIONS: Avoid unnecessary line continuations #41: FILE: drivers/gpu/drm/i915/i915_pci.c:730: + .dbuf.size = 1024 - 4, /* 4 blocks for bypass path allocation */ \ total: 0 errors, 2 warnings, 0 checks, 47 lines checked 6182af680aea drm/i915: Store dbuf slice mask in device info 4bec22613175 drm/i915: Use intel_dbuf_slice_size() 407ca0202e11 drm/i915: Use intel_de_rmw() for DBUF_POWER_REQUEST bbfbba08f9aa drm/i915: Polish for_each_dbuf_slice() -:79: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__slice' - possible side-effects? #79: FILE: drivers/gpu/drm/i915/display/intel_display.h:191: +#define for_each_dbuf_slice(__dev_priv, __slice) \ for ((__slice) = DBUF_S1; (__slice) < I915_MAX_DBUF_SLICES; (__slice)++) \ + for_each_if(INTEL_INFO(__dev_priv)->dbuf.slice_mask & BIT(__slice)) -:86: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses #86: FILE: drivers/gpu/drm/i915/display/intel_display.h:195: +#define for_each_dbuf_slice_in_mask(__dev_priv, __slice, __mask) \ + for_each_dbuf_slice((__dev_priv), (__slice)) \ + for_each_if((__mask) & BIT(__slice)) -:86: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__slice' - possible side-effects? #86: FILE: drivers/gpu/drm/i915/display/intel_display.h:195: +#define for_each_dbuf_slice_in_mask(__dev_priv, __slice, __mask) \ + for_each_dbuf_slice((__dev_priv), (__slice)) \ + for_each_if((__mask) & BIT(__slice)) total: 1 errors, 0 warnings, 2 checks, 162 lines checked b571b996abd4 drm/i915: Add enabledisable() bb42f353dfe4 drm/i915: Say "enable foo" instead of "set foo to enabled" ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/8] drm/i915: Use intel_dbuf_slice_size()
From: Ville Syrjälä
Use intel_dbuf_slice_size() instead of hand rolling it.
Also clean up some of the types.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_pm.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 88eb54241b9f..38e2ba45bfd8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4066,12 +4066,9 @@ skl_ddb_entry_for_slices(struct drm_i915_private
*dev_priv, u8 slice_mask,
u32 skl_ddb_dbuf_slice_mask(struct drm_i915_private *dev_priv,
const struct skl_ddb_entry *entry)
{
- u32 slice_mask = 0;
- u16 ddb_size = intel_dbuf_size(dev_priv);
- int num_slices = intel_dbuf_num_slices(dev_priv);
- u16 slice_size = ddb_size / num_slices;
- u16 start_slice;
- u16 end_slice;
+ int slice_size = intel_dbuf_slice_size(dev_priv);
+ enum dbuf_slice start_slice, end_slice;
+ u8 slice_mask = 0;
if (!skl_ddb_entry_size(entry))
return 0;
--
2.26.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/8] drm/i915: Add enabledisable()
From: Ville Syrjälä
'enable ? "enable" : "disable"' is a fairly common pattern in
out debug prints. Let's introduce a helper for it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_ddi.c | 4 ++--
drivers/gpu/drm/i915/display/intel_display_power.c| 2 +-
drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 2 +-
drivers/gpu/drm/i915/i915_utils.h | 5 +
5 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 4ef573883412..f4249f087fa7 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2334,8 +2334,8 @@ static void
intel_dp_sink_set_msa_timing_par_ignore_state(struct intel_dp *intel
if (drm_dp_dpcd_writeb(_dp->aux, DP_DOWNSPREAD_CTRL,
enable ? DP_MSA_TIMING_PAR_IGNORE_EN : 0) <= 0)
drm_dbg_kms(>drm,
- "Failed to set MSA_TIMING_PAR_IGNORE %s in the
sink\n",
- enable ? "enable" : "disable");
+ "Failed to %s MSA_TIMING_PAR_IGNORE in the sink\n",
+ enabledisable(enable));
}
static void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
b/drivers/gpu/drm/i915/display/intel_display_power.c
index 0fb4864a191a..d48dd15a4f6e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -4766,7 +4766,7 @@ static void gen9_dbuf_slice_set(struct drm_i915_private
*dev_priv,
state = intel_de_read(dev_priv, reg) & DBUF_POWER_STATE;
drm_WARN(_priv->drm, enable != state,
"DBuf slice %d power %s timeout!\n",
-slice, enable ? "enable" : "disable");
+slice, enabledisable(enable));
}
void gen9_dbuf_slices_update(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 5ee953aaa00c..44109a4b69aa 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1861,7 +1861,7 @@ void intel_dp_sink_set_decompression_state(struct
intel_dp *intel_dp,
if (ret < 0)
drm_dbg_kms(>drm,
"Failed to %s sink decompression state\n",
- enable ? "enable" : "disable");
+ enabledisable(enable));
}
static void
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index 4f8337c7fd2e..8e9ac9ba1d38 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -291,7 +291,7 @@ static void set_vesa_backlight_enable(struct intel_dp
*intel_dp, bool enable)
if (drm_dp_dpcd_writeb(_dp->aux, DP_EDP_DISPLAY_CONTROL_REGISTER,
reg_val) != 1) {
drm_dbg_kms(>drm, "Failed to %s aux backlight\n",
- enable ? "enable" : "disable");
+ enabledisable(enable));
}
}
diff --git a/drivers/gpu/drm/i915/i915_utils.h
b/drivers/gpu/drm/i915/i915_utils.h
index abd4dcd9f79c..f02f52ab5070 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -418,6 +418,11 @@ static inline const char *onoff(bool v)
return v ? "on" : "off";
}
+static inline const char *enabledisable(bool v)
+{
+ return v ? "enable" : "disable";
+}
+
static inline const char *enableddisabled(bool v)
{
return v ? "enabled" : "disabled";
--
2.26.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/8] drm/i915: Use intel_de_rmw() for DBUF_POWER_REQUEST
From: Ville Syrjälä
Use intel_de_rmw() instead of hand rolling it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_display_power.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
b/drivers/gpu/drm/i915/display/intel_display_power.c
index 0435103082eb..528fbede0ee7 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -4757,14 +4757,9 @@ static void gen9_dbuf_slice_set(struct drm_i915_private
*dev_priv,
{
i915_reg_t reg = DBUF_CTL_S(slice);
bool state;
- u32 val;
- val = intel_de_read(dev_priv, reg);
- if (enable)
- val |= DBUF_POWER_REQUEST;
- else
- val &= ~DBUF_POWER_REQUEST;
- intel_de_write(dev_priv, reg, val);
+ intel_de_rmw(dev_priv, reg, DBUF_POWER_REQUEST,
+enable ? DBUF_POWER_REQUEST : 0);
intel_de_posting_read(dev_priv, reg);
udelay(10);
--
2.26.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/8] drm/i915: Handle dbuf bypass path allocation earlier
From: Ville Syrjälä
We always reserve the same 4 dbuf blocks for the bypass path
allocation, so might as well do that when declaring the dbuf
size.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_pci.c | 8
drivers/gpu/drm/i915/intel_pm.c | 9 +
2 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 484d2633894a..981d12702c49 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -647,7 +647,7 @@ static const struct intel_device_info chv_info = {
.has_gt_uc = 1, \
.display.has_hdcp = 1, \
.display.has_ipc = 1, \
- .dbuf.size = 896, \
+ .dbuf.size = 896 - 4, /* 4 blocks for bypass path allocation */ \
.dbuf.num_slices = 1
#define SKL_PLATFORM \
@@ -720,14 +720,14 @@ static const struct intel_device_info skl_gt4_info = {
static const struct intel_device_info bxt_info = {
GEN9_LP_FEATURES,
PLATFORM(INTEL_BROXTON),
- .dbuf.size = 512,
+ .dbuf.size = 512 - 4, /* 4 blocks for bypass path allocation */ \
};
static const struct intel_device_info glk_info = {
GEN9_LP_FEATURES,
PLATFORM(INTEL_GEMINILAKE),
.display.ver = 10,
- .dbuf.size = 1024,
+ .dbuf.size = 1024 - 4, /* 4 blocks for bypass path allocation */ \
GLK_COLORS,
};
@@ -790,7 +790,7 @@ static const struct intel_device_info cml_gt2_info = {
#define GEN10_FEATURES \
GEN9_FEATURES, \
GEN(10), \
- .dbuf.size = 1024, \
+ .dbuf.size = 1024 - 4, /* 4 blocks for bypass path allocation */ \
.display.has_dsc = 1, \
.has_coherent_ggtt = false, \
GLK_COLORS
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ced1eb32cb78..8d6ee5ad761e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4030,14 +4030,7 @@ static int intel_compute_sagv_mask(struct
intel_atomic_state *state)
static int intel_dbuf_size(struct drm_i915_private *dev_priv)
{
- int ddb_size = INTEL_INFO(dev_priv)->dbuf.size;
-
- drm_WARN_ON(_priv->drm, ddb_size == 0);
-
- if (DISPLAY_VER(dev_priv) < 11)
- return ddb_size - 4; /* 4 blocks for bypass path allocation */
-
- return ddb_size;
+ return INTEL_INFO(dev_priv)->dbuf.size;
}
static int intel_dbuf_slice_size(struct drm_i915_private *dev_priv)
--
2.26.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/8] drm/i915: dbuf cleanups
From: Ville Syrjälä A bunch of drive-by-cleanup While I was reading through the dbuf code. Ville Syrjälä (8): drm/i915: Collect dbuf device info into a sub-struct drm/i915: Handle dbuf bypass path allocation earlier drm/i915: Store dbuf slice mask in device info drm/i915: Use intel_dbuf_slice_size() drm/i915: Use intel_de_rmw() for DBUF_POWER_REQUEST drm/i915: Polish for_each_dbuf_slice() drm/i915: Add enabledisable() drm/i915: Say "enable foo" instead of "set foo to enabled" drivers/gpu/drm/i915/display/intel_bw.c | 11 ++--- drivers/gpu/drm/i915/display/intel_ddi.c | 4 +- drivers/gpu/drm/i915/display/intel_display.h | 9 ++-- .../drm/i915/display/intel_display_power.c| 24 -- drivers/gpu/drm/i915/display/intel_dp.c | 14 +++--- .../drm/i915/display/intel_dp_aux_backlight.c | 2 +- drivers/gpu/drm/i915/display/intel_tc.c | 4 +- drivers/gpu/drm/i915/i915_pci.c | 16 +++ drivers/gpu/drm/i915/i915_utils.h | 5 +++ drivers/gpu/drm/i915/intel_device_info.h | 6 ++- drivers/gpu/drm/i915/intel_pm.c | 45 +++ drivers/gpu/drm/i915/intel_pm.h | 1 + 12 files changed, 65 insertions(+), 76 deletions(-) -- 2.26.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/8] drm/i915: Collect dbuf device info into a sub-struct
From: Ville Syrjälä
Collect the related dbuf information into a struct.
Signed-off-by: Ville Syrjälä
---
.../gpu/drm/i915/display/intel_display_power.c | 4 ++--
drivers/gpu/drm/i915/i915_pci.c | 16
drivers/gpu/drm/i915/intel_device_info.h | 6 --
drivers/gpu/drm/i915/intel_pm.c | 14 +++---
4 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
b/drivers/gpu/drm/i915/display/intel_display_power.c
index 0af1dee1ac95..0e433a0e1fce 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -4777,7 +4777,7 @@ static void gen9_dbuf_slice_set(struct drm_i915_private
*dev_priv,
void gen9_dbuf_slices_update(struct drm_i915_private *dev_priv,
u8 req_slices)
{
- int num_slices = INTEL_INFO(dev_priv)->num_supported_dbuf_slices;
+ int num_slices = INTEL_INFO(dev_priv)->dbuf.num_slices;
struct i915_power_domains *power_domains = _priv->power_domains;
enum dbuf_slice slice;
@@ -4825,7 +4825,7 @@ static void gen9_dbuf_disable(struct drm_i915_private
*dev_priv)
static void gen12_dbuf_slices_config(struct drm_i915_private *dev_priv)
{
- const int num_slices = INTEL_INFO(dev_priv)->num_supported_dbuf_slices;
+ int num_slices = INTEL_INFO(dev_priv)->dbuf.num_slices;
enum dbuf_slice slice;
for (slice = DBUF_S1; slice < (DBUF_S1 + num_slices); slice++)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 44e7b94db63d..484d2633894a 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -647,8 +647,8 @@ static const struct intel_device_info chv_info = {
.has_gt_uc = 1, \
.display.has_hdcp = 1, \
.display.has_ipc = 1, \
- .ddb_size = 896, \
- .num_supported_dbuf_slices = 1
+ .dbuf.size = 896, \
+ .dbuf.num_slices = 1
#define SKL_PLATFORM \
GEN9_FEATURES, \
@@ -683,7 +683,7 @@ static const struct intel_device_info skl_gt4_info = {
#define GEN9_LP_FEATURES \
GEN(9), \
.is_lp = 1, \
- .num_supported_dbuf_slices = 1, \
+ .dbuf.num_slices = 1, \
.display.has_hotplug = 1, \
.platform_engine_mask = BIT(RCS0) | BIT(VCS0) | BIT(BCS0) | BIT(VECS0),
\
.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C), \
@@ -720,14 +720,14 @@ static const struct intel_device_info skl_gt4_info = {
static const struct intel_device_info bxt_info = {
GEN9_LP_FEATURES,
PLATFORM(INTEL_BROXTON),
- .ddb_size = 512,
+ .dbuf.size = 512,
};
static const struct intel_device_info glk_info = {
GEN9_LP_FEATURES,
PLATFORM(INTEL_GEMINILAKE),
.display.ver = 10,
- .ddb_size = 1024,
+ .dbuf.size = 1024,
GLK_COLORS,
};
@@ -790,7 +790,7 @@ static const struct intel_device_info cml_gt2_info = {
#define GEN10_FEATURES \
GEN9_FEATURES, \
GEN(10), \
- .ddb_size = 1024, \
+ .dbuf.size = 1024, \
.display.has_dsc = 1, \
.has_coherent_ggtt = false, \
GLK_COLORS
@@ -830,8 +830,8 @@ static const struct intel_device_info cnl_info = {
[TRANSCODER_DSI_1] = TRANSCODER_DSI1_OFFSET, \
}, \
GEN(11), \
- .ddb_size = 2048, \
- .num_supported_dbuf_slices = 2, \
+ .dbuf.size = 2048, \
+ .dbuf.num_slices = 2, \
.has_logical_ring_elsq = 1, \
.color = { .degamma_lut_size = 33, .gamma_lut_size = 262145 }
diff --git a/drivers/gpu/drm/i915/intel_device_info.h
b/drivers/gpu/drm/i915/intel_device_info.h
index 8ab4fa6c7fdd..74591e4f9c44 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -196,8 +196,10 @@ struct intel_device_info {
#undef DEFINE_FLAG
} display;
- u16 ddb_size; /* in blocks */
- u8 num_supported_dbuf_slices; /* number of DBuf slices */
+ struct {
+ u16 size; /* in blocks */
+ u8 num_slices;
+ } dbuf;
/* Register offsets for the various display pipes and transcoders */
int pipe_offsets[I915_MAX_TRANSCODERS];
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index eaf4c072ade0..ced1eb32cb78 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3637,10 +3637,10 @@ bool ilk_disable_lp_wm(struct drm_i915_private
*dev_priv)
u8 intel_enabled_dbuf_slices_mask(struct drm_i915_private *dev_priv)
{
int i;
- int max_slices = INTEL_INFO(dev_priv)->num_supported_dbuf_slices;
+ int num_slices = INTEL_INFO(dev_priv)->dbuf.num_slices;
u8 enabled_slices_mask = 0;
- for (i = 0; i < max_slices; i++) {
+ for (i = 0; i < num_slices; i++) {
if (intel_uncore_read(_priv->uncore,
[Intel-gfx] [PATCH 8/8] drm/i915: Say "enable foo" instead of "set foo to enabled"
From: Ville Syrjälä
Use simpler sentences. Just say "enable foo" instead
of "set foo to enabled" etc.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_dp.c | 12 ++--
drivers/gpu/drm/i915/display/intel_tc.c | 4 ++--
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 44109a4b69aa..52ea09fc5e70 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2293,8 +2293,8 @@ void intel_dp_configure_protocol_converter(struct
intel_dp *intel_dp,
if (drm_dp_dpcd_writeb(_dp->aux,
DP_PROTOCOL_CONVERTER_CONTROL_0, tmp) != 1)
- drm_dbg_kms(>drm, "Failed to set protocol converter HDMI
mode to %s\n",
- enableddisabled(intel_dp->has_hdmi_sink));
+ drm_dbg_kms(>drm, "Failed to %s protocol converter HDMI
mode\n",
+ enabledisable(intel_dp->has_hdmi_sink));
tmp = crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 &&
intel_dp->dfp.ycbcr_444_to_420 ?
DP_CONVERSION_TO_YCBCR420_ENABLE : 0;
@@ -2302,8 +2302,8 @@ void intel_dp_configure_protocol_converter(struct
intel_dp *intel_dp,
if (drm_dp_dpcd_writeb(_dp->aux,
DP_PROTOCOL_CONVERTER_CONTROL_1, tmp) != 1)
drm_dbg_kms(>drm,
- "Failed to set protocol converter YCbCr 4:2:0
conversion mode to %s\n",
- enableddisabled(intel_dp->dfp.ycbcr_444_to_420));
+ "Failed to %s protocol converter YCbCr 4:2:0
conversion mode\n",
+ enabledisable(intel_dp->dfp.ycbcr_444_to_420));
tmp = 0;
if (intel_dp->dfp.rgb_to_ycbcr) {
@@ -2340,8 +2340,8 @@ void intel_dp_configure_protocol_converter(struct
intel_dp *intel_dp,
if (drm_dp_pcon_convert_rgb_to_ycbcr(_dp->aux, tmp) < 0)
drm_dbg_kms(>drm,
- "Failed to set protocol converter RGB->YCbCr
conversion mode to %s\n",
- enableddisabled(tmp ? true : false));
+ "Failed to %s protocol converter RGB->YCbCr
conversion mode\n",
+ enabledisable(tmp));
}
diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
b/drivers/gpu/drm/i915/display/intel_tc.c
index 88085486ee59..59de6ca436db 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -267,8 +267,8 @@ static bool icl_tc_phy_set_safe_mode(struct
intel_digital_port *dig_port,
PORT_TX_DFLEXDPCSSS(dig_port->tc_phy_fia));
if (val == 0x) {
drm_dbg_kms(>drm,
- "Port %s: PHY in TCCOLD, can't set safe-mode to
%s\n",
- dig_port->tc_port_name, enableddisabled(enable));
+ "Port %s: PHY in TCCOLD, can't %s safe-mode\n",
+ dig_port->tc_port_name, enabledisable(enable));
return false;
}
--
2.26.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/8] drm/i915: Polish for_each_dbuf_slice()
From: Ville Syrjälä
Now that we have the dbuf slice mask stored in the device info
let's use it for for_each_dbuf_slice_in_mask*().
With this we cal also rip out intel_dbuf_size() and
intel_dbuf_num_slices().
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_bw.c | 11 +++---
drivers/gpu/drm/i915/display/intel_display.h | 9 ++---
.../drm/i915/display/intel_display_power.c| 13 ---
drivers/gpu/drm/i915/intel_pm.c | 34 +++
4 files changed, 29 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bw.c
b/drivers/gpu/drm/i915/display/intel_bw.c
index 20dbc3759d27..969169743630 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -390,7 +390,6 @@ int skl_bw_calc_min_cdclk(struct intel_atomic_state *state)
const struct intel_crtc_state *crtc_state;
struct intel_crtc *crtc;
int max_bw = 0;
- int slice_id;
enum pipe pipe;
int i;
@@ -418,6 +417,7 @@ int skl_bw_calc_min_cdclk(struct intel_atomic_state *state)
_state->wm.skl.plane_ddb_uv[plane_id];
unsigned int data_rate =
crtc_state->data_rate[plane_id];
unsigned int dbuf_mask = 0;
+ enum dbuf_slice slice;
dbuf_mask |= skl_ddb_dbuf_slice_mask(dev_priv,
plane_alloc);
dbuf_mask |= skl_ddb_dbuf_slice_mask(dev_priv,
uv_plane_alloc);
@@ -435,8 +435,8 @@ int skl_bw_calc_min_cdclk(struct intel_atomic_state *state)
* pessimistic, which shouldn't pose any significant
* problem anyway.
*/
- for_each_dbuf_slice_in_mask(slice_id, dbuf_mask)
- crtc_bw->used_bw[slice_id] += data_rate;
+ for_each_dbuf_slice_in_mask(dev_priv, slice, dbuf_mask)
+ crtc_bw->used_bw[slice] += data_rate;
}
}
@@ -445,10 +445,11 @@ int skl_bw_calc_min_cdclk(struct intel_atomic_state
*state)
for_each_pipe(dev_priv, pipe) {
struct intel_dbuf_bw *crtc_bw;
+ enum dbuf_slice slice;
crtc_bw = _bw_state->dbuf_bw[pipe];
- for_each_dbuf_slice(slice_id) {
+ for_each_dbuf_slice(dev_priv, slice) {
/*
* Current experimental observations show that contrary
* to BSpec we get underruns once we exceed 64 * CDCLK
@@ -457,7 +458,7 @@ int skl_bw_calc_min_cdclk(struct intel_atomic_state *state)
* bumped up all the time we calculate CDCLK according
* to this formula for overall bw consumed by slices.
*/
- max_bw += crtc_bw->used_bw[slice_id];
+ max_bw += crtc_bw->used_bw[slice];
}
}
diff --git a/drivers/gpu/drm/i915/display/intel_display.h
b/drivers/gpu/drm/i915/display/intel_display.h
index 105294ec2dcc..b68bcd502206 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -188,12 +188,13 @@ enum plane_id {
for ((__p) = PLANE_PRIMARY; (__p) < I915_MAX_PLANES; (__p)++) \
for_each_if((__crtc)->plane_ids_mask & BIT(__p))
-#define for_each_dbuf_slice_in_mask(__slice, __mask) \
+#define for_each_dbuf_slice(__dev_priv, __slice) \
for ((__slice) = DBUF_S1; (__slice) < I915_MAX_DBUF_SLICES;
(__slice)++) \
- for_each_if((BIT(__slice)) & (__mask))
+ for_each_if(INTEL_INFO(__dev_priv)->dbuf.slice_mask &
BIT(__slice))
-#define for_each_dbuf_slice(__slice) \
- for_each_dbuf_slice_in_mask(__slice, BIT(I915_MAX_DBUF_SLICES) - 1)
+#define for_each_dbuf_slice_in_mask(__dev_priv, __slice, __mask) \
+ for_each_dbuf_slice((__dev_priv), (__slice)) \
+ for_each_if((__mask) & BIT(__slice))
enum port {
PORT_NONE = -1,
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
b/drivers/gpu/drm/i915/display/intel_display_power.c
index 528fbede0ee7..0fb4864a191a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -4772,13 +4772,13 @@ static void gen9_dbuf_slice_set(struct drm_i915_private
*dev_priv,
void gen9_dbuf_slices_update(struct drm_i915_private *dev_priv,
u8 req_slices)
{
- int num_slices = intel_dbuf_num_slices(dev_priv);
struct i915_power_domains *power_domains = _priv->power_domains;
+ u8 slice_mask = INTEL_INFO(dev_priv)->dbuf.slice_mask;
enum dbuf_slice slice;
- drm_WARN(_priv->drm, req_slices & ~(BIT(num_slices) - 1),
-"Invalid set of dbuf slices (0x%x) requested (num dbuf
[Intel-gfx] [PATCH 3/8] drm/i915: Store dbuf slice mask in device info
From: Ville Syrjälä
Let's just store the dbuf slice information as a bitmask
in the device info. Makes life a little easier later.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_display_power.c | 4 ++--
drivers/gpu/drm/i915/i915_pci.c| 6 +++---
drivers/gpu/drm/i915/intel_device_info.h | 2 +-
drivers/gpu/drm/i915/intel_pm.c| 13 +
drivers/gpu/drm/i915/intel_pm.h| 1 +
5 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
b/drivers/gpu/drm/i915/display/intel_display_power.c
index 0e433a0e1fce..0435103082eb 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -4777,7 +4777,7 @@ static void gen9_dbuf_slice_set(struct drm_i915_private
*dev_priv,
void gen9_dbuf_slices_update(struct drm_i915_private *dev_priv,
u8 req_slices)
{
- int num_slices = INTEL_INFO(dev_priv)->dbuf.num_slices;
+ int num_slices = intel_dbuf_num_slices(dev_priv);
struct i915_power_domains *power_domains = _priv->power_domains;
enum dbuf_slice slice;
@@ -4825,7 +4825,7 @@ static void gen9_dbuf_disable(struct drm_i915_private
*dev_priv)
static void gen12_dbuf_slices_config(struct drm_i915_private *dev_priv)
{
- int num_slices = INTEL_INFO(dev_priv)->dbuf.num_slices;
+ int num_slices = intel_dbuf_num_slices(dev_priv);
enum dbuf_slice slice;
for (slice = DBUF_S1; slice < (DBUF_S1 + num_slices); slice++)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 981d12702c49..15eb078fe6bb 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -648,7 +648,7 @@ static const struct intel_device_info chv_info = {
.display.has_hdcp = 1, \
.display.has_ipc = 1, \
.dbuf.size = 896 - 4, /* 4 blocks for bypass path allocation */ \
- .dbuf.num_slices = 1
+ .dbuf.slice_mask = BIT(DBUF_S1)
#define SKL_PLATFORM \
GEN9_FEATURES, \
@@ -683,7 +683,7 @@ static const struct intel_device_info skl_gt4_info = {
#define GEN9_LP_FEATURES \
GEN(9), \
.is_lp = 1, \
- .dbuf.num_slices = 1, \
+ .dbuf.slice_mask = BIT(DBUF_S1), \
.display.has_hotplug = 1, \
.platform_engine_mask = BIT(RCS0) | BIT(VCS0) | BIT(BCS0) | BIT(VECS0),
\
.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C), \
@@ -831,7 +831,7 @@ static const struct intel_device_info cnl_info = {
}, \
GEN(11), \
.dbuf.size = 2048, \
- .dbuf.num_slices = 2, \
+ .dbuf.slice_mask = BIT(DBUF_S1) | BIT(DBUF_S2), \
.has_logical_ring_elsq = 1, \
.color = { .degamma_lut_size = 33, .gamma_lut_size = 262145 }
diff --git a/drivers/gpu/drm/i915/intel_device_info.h
b/drivers/gpu/drm/i915/intel_device_info.h
index 74591e4f9c44..6aefe4fde197 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -198,7 +198,7 @@ struct intel_device_info {
struct {
u16 size; /* in blocks */
- u8 num_slices;
+ u8 slice_mask;
} dbuf;
/* Register offsets for the various display pipes and transcoders */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8d6ee5ad761e..88eb54241b9f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3637,7 +3637,7 @@ bool ilk_disable_lp_wm(struct drm_i915_private *dev_priv)
u8 intel_enabled_dbuf_slices_mask(struct drm_i915_private *dev_priv)
{
int i;
- int num_slices = INTEL_INFO(dev_priv)->dbuf.num_slices;
+ int num_slices = intel_dbuf_num_slices(dev_priv);
u8 enabled_slices_mask = 0;
for (i = 0; i < num_slices; i++) {
@@ -4033,10 +4033,15 @@ static int intel_dbuf_size(struct drm_i915_private
*dev_priv)
return INTEL_INFO(dev_priv)->dbuf.size;
}
+int intel_dbuf_num_slices(struct drm_i915_private *dev_priv)
+{
+ return hweight8(INTEL_INFO(dev_priv)->dbuf.slice_mask);
+}
+
static int intel_dbuf_slice_size(struct drm_i915_private *dev_priv)
{
return intel_dbuf_size(dev_priv) /
- INTEL_INFO(dev_priv)->dbuf.num_slices;
+ intel_dbuf_num_slices(dev_priv);
}
static void
@@ -4063,7 +4068,7 @@ u32 skl_ddb_dbuf_slice_mask(struct drm_i915_private
*dev_priv,
{
u32 slice_mask = 0;
u16 ddb_size = intel_dbuf_size(dev_priv);
- int num_slices = INTEL_INFO(dev_priv)->dbuf.num_slices;
+ int num_slices = intel_dbuf_num_slices(dev_priv);
u16 slice_size = ddb_size / num_slices;
u16 start_slice;
u16 end_slice;
@@ -5821,7 +5826,7 @@ skl_compute_ddb(struct intel_atomic_state *state)
"Enabled dbuf slices 0x%x -> 0x%x (out of %d dbuf
Re: [Intel-gfx] [PATCH v3 4/4] drm/doc/rfc: i915 DG1 uAPI
On Fri, Apr 16, 2021 at 6:38 PM Jason Ekstrand wrote:
>
> On Thu, Apr 15, 2021 at 11:04 AM Matthew Auld wrote:
> >
> > Add an entry for the new uAPI needed for DG1.
> >
> > v2(Daniel):
> > - include the overall upstreaming plan
> > - add a note for mmap, there are differences here for TTM vs i915
> > - bunch of other suggestions from Daniel
> > v3:
> > (Daniel)
> > - add a note for set/get caching stuff
> > - add some more docs for existing query and extensions stuff
> > - add an actual code example for regions query
> > - bunch of other stuff
> > (Jason)
> > - uAPI change(!):
> > - try a simpler design with the placements extension
> > - rather than have a generic setparam which can cover multiple
> > use cases, have each extension be responsible for one thing
> > only
> >
> > Signed-off-by: Matthew Auld
> > Cc: Joonas Lahtinen
> > Cc: Jordan Justen
> > Cc: Daniel Vetter
> > Cc: Kenneth Graunke
> > Cc: Jason Ekstrand
> > Cc: Dave Airlie
> > Cc: dri-de...@lists.freedesktop.org
> > Cc: mesa-...@lists.freedesktop.org
> > ---
> > Documentation/gpu/rfc/i915_gem_lmem.h | 255
> > Documentation/gpu/rfc/i915_gem_lmem.rst | 139 +
> > Documentation/gpu/rfc/index.rst | 4 +
> > 3 files changed, 398 insertions(+)
> > create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h
> > create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.rst
> >
> > diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h
> > b/Documentation/gpu/rfc/i915_gem_lmem.h
> > new file mode 100644
> > index ..2a82a452e9f2
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/i915_gem_lmem.h
> > @@ -0,0 +1,255 @@
> > +/*
> > + * Note that drm_i915_query_item and drm_i915_query are existing bits of
> > uAPI.
> > + * For the regions query we are just adding a new query id, so no actual
> > new
> > + * ioctl or anything, but including it here for reference.
> > + */
> > +struct drm_i915_query_item {
> > +#define DRM_I915_QUERY_MEMORY_REGIONS 0xdeadbeaf
> > +
> > +__u64 query_id;
> > +
> > +/*
> > + * When set to zero by userspace, this is filled with the size of
> > the
> > + * data to be written at the data_ptr pointer. The kernel sets this
> > + * value to a negative value to signal an error on a particular
> > query
> > + * item.
> > + */
> > +__s32 length;
> > +
> > +__u32 flags;
> > +/*
> > + * Data will be written at the location pointed by data_ptr when
> > the
> > + * value of length matches the length of the data to be written by
> > the
> > + * kernel.
> > + */
> > +__u64 data_ptr;
> > +};
> > +
> > +struct drm_i915_query {
> > +__u32 num_items;
> > +/*
> > + * Unused for now. Must be cleared to zero.
> > + */
> > +__u32 flags;
> > +/*
> > + * This points to an array of num_items drm_i915_query_item
> > structures.
> > + */
> > +__u64 items_ptr;
> > +};
> > +
> > +#define DRM_IOCTL_I915_QUERY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_QUERY,
> > struct drm_i915_query)
> > +
> > +/**
> > + * enum drm_i915_gem_memory_class
> > + */
> > +enum drm_i915_gem_memory_class {
> > + /** @I915_MEMORY_CLASS_SYSTEM: system memory */
> > + I915_MEMORY_CLASS_SYSTEM = 0,
> > + /** @I915_MEMORY_CLASS_DEVICE: device local-memory */
> > + I915_MEMORY_CLASS_DEVICE,
> > +};
> > +
> > +/**
> > + * struct drm_i915_gem_memory_class_instance
> > + */
> > +struct drm_i915_gem_memory_class_instance {
> > + /** @memory_class: see enum drm_i915_gem_memory_class */
> > + __u16 memory_class;
> > +
> > + /** @memory_instance: which instance */
> > + __u16 memory_instance;
> > +};
> > +
> > +/**
> > + * struct drm_i915_memory_region_info
> > + *
> > + * Describes one region as known to the driver.
> > + *
> > + * Note that we reserve quite a lot of stuff here for potential future
> > work. As
> > + * an example we might want expose the capabilities(see caps) for a given
> > + * region, which could include things like if the region is CPU
> > + * mappable/accessible etc.
>
> I get caps but I'm seriously at a loss as to what the rest of this
> would be used for. Why are caps and flags both there and separate?
> Flags are typically something you set, not query. Also, what's with
> rsvd1 at the end? This smells of substantial over-building to me.
>
> I thought to myself, "maybe I'm missing a future use-case" so I looked
> at the internal tree and none of this is being used there either.
> This indicates to me that either I'm missing something and there's
> code somewhere I don't know about or, with three years of building on
> internal branches, we still haven't proven that any of this is needed.
> If it's the latter, which I strongly suspect, maybe we should drop the
> unnecessary bits and only add them
Re: [Intel-gfx] [PATCH v3 4/4] drm/doc/rfc: i915 DG1 uAPI
On Thu, Apr 15, 2021 at 11:04 AM Matthew Auld wrote:
>
> Add an entry for the new uAPI needed for DG1.
>
> v2(Daniel):
> - include the overall upstreaming plan
> - add a note for mmap, there are differences here for TTM vs i915
> - bunch of other suggestions from Daniel
> v3:
> (Daniel)
> - add a note for set/get caching stuff
> - add some more docs for existing query and extensions stuff
> - add an actual code example for regions query
> - bunch of other stuff
> (Jason)
> - uAPI change(!):
> - try a simpler design with the placements extension
> - rather than have a generic setparam which can cover multiple
> use cases, have each extension be responsible for one thing
> only
>
> Signed-off-by: Matthew Auld
> Cc: Joonas Lahtinen
> Cc: Jordan Justen
> Cc: Daniel Vetter
> Cc: Kenneth Graunke
> Cc: Jason Ekstrand
> Cc: Dave Airlie
> Cc: dri-de...@lists.freedesktop.org
> Cc: mesa-...@lists.freedesktop.org
> ---
> Documentation/gpu/rfc/i915_gem_lmem.h | 255
> Documentation/gpu/rfc/i915_gem_lmem.rst | 139 +
> Documentation/gpu/rfc/index.rst | 4 +
> 3 files changed, 398 insertions(+)
> create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h
> create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.rst
>
> diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h
> b/Documentation/gpu/rfc/i915_gem_lmem.h
> new file mode 100644
> index ..2a82a452e9f2
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_gem_lmem.h
> @@ -0,0 +1,255 @@
> +/*
> + * Note that drm_i915_query_item and drm_i915_query are existing bits of
> uAPI.
> + * For the regions query we are just adding a new query id, so no actual new
> + * ioctl or anything, but including it here for reference.
> + */
> +struct drm_i915_query_item {
> +#define DRM_I915_QUERY_MEMORY_REGIONS 0xdeadbeaf
> +
> +__u64 query_id;
> +
> +/*
> + * When set to zero by userspace, this is filled with the size of the
> + * data to be written at the data_ptr pointer. The kernel sets this
> + * value to a negative value to signal an error on a particular query
> + * item.
> + */
> +__s32 length;
> +
> +__u32 flags;
> +/*
> + * Data will be written at the location pointed by data_ptr when the
> + * value of length matches the length of the data to be written by
> the
> + * kernel.
> + */
> +__u64 data_ptr;
> +};
> +
> +struct drm_i915_query {
> +__u32 num_items;
> +/*
> + * Unused for now. Must be cleared to zero.
> + */
> +__u32 flags;
> +/*
> + * This points to an array of num_items drm_i915_query_item
> structures.
> + */
> +__u64 items_ptr;
> +};
> +
> +#define DRM_IOCTL_I915_QUERY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_QUERY,
> struct drm_i915_query)
> +
> +/**
> + * enum drm_i915_gem_memory_class
> + */
> +enum drm_i915_gem_memory_class {
> + /** @I915_MEMORY_CLASS_SYSTEM: system memory */
> + I915_MEMORY_CLASS_SYSTEM = 0,
> + /** @I915_MEMORY_CLASS_DEVICE: device local-memory */
> + I915_MEMORY_CLASS_DEVICE,
> +};
> +
> +/**
> + * struct drm_i915_gem_memory_class_instance
> + */
> +struct drm_i915_gem_memory_class_instance {
> + /** @memory_class: see enum drm_i915_gem_memory_class */
> + __u16 memory_class;
> +
> + /** @memory_instance: which instance */
> + __u16 memory_instance;
> +};
> +
> +/**
> + * struct drm_i915_memory_region_info
> + *
> + * Describes one region as known to the driver.
> + *
> + * Note that we reserve quite a lot of stuff here for potential future work.
> As
> + * an example we might want expose the capabilities(see caps) for a given
> + * region, which could include things like if the region is CPU
> + * mappable/accessible etc.
I get caps but I'm seriously at a loss as to what the rest of this
would be used for. Why are caps and flags both there and separate?
Flags are typically something you set, not query. Also, what's with
rsvd1 at the end? This smells of substantial over-building to me.
I thought to myself, "maybe I'm missing a future use-case" so I looked
at the internal tree and none of this is being used there either.
This indicates to me that either I'm missing something and there's
code somewhere I don't know about or, with three years of building on
internal branches, we still haven't proven that any of this is needed.
If it's the latter, which I strongly suspect, maybe we should drop the
unnecessary bits and only add them back in if and when we have proof
that they're useful.
To be clear, I don't mind the query API as such and the class/instance
stuff seems fine and I really like being able to get the sizes
directly. What concerns me is all this extra future-proofing that we
have zero proof is actually useful. In my experience, when you build
out like this
Re: [Intel-gfx] [PATCH 03/19] drm/i915: Create stolen memory region from local memory
On 14/04/2021 16:01, Tvrtko Ursulin wrote:
On 12/04/2021 10:05, Matthew Auld wrote:
From: CQ Tang
Add "REGION_STOLEN" device info to dg1, create stolen memory
region from upper portion of local device memory, starting
from DSMBASE.
v2:
- s/drm_info/drm_dbg; userspace likely doesn't care about stolen.
- mem->type is only setup after the region probe, so setting the
name
as stolen-local or stolen-system based on this value won't
work. Split
system vs local stolen setup to fix this.
- kill all the region->devmem/is_devmem stuff. We already
differentiate
the different types of stolen so such things shouldn't be needed
anymore.
Signed-off-by: CQ Tang
Signed-off-by: Matthew Auld
---
drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 99 +++---
drivers/gpu/drm/i915/gem/i915_gem_stolen.h | 3 +
drivers/gpu/drm/i915/i915_pci.c | 2 +-
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_memory_region.c | 6 ++
drivers/gpu/drm/i915/intel_memory_region.h | 5 +-
6 files changed, 102 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index b0597de206de..56dd58bef5ee 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -10,6 +10,7 @@
#include
#include
+#include "gem/i915_gem_lmem.h"
#include "gem/i915_gem_region.h"
#include "i915_drv.h"
#include "i915_gem_stolen.h"
@@ -121,6 +122,14 @@ static int i915_adjust_stolen(struct
drm_i915_private *i915,
}
}
+ /*
+ * With device local memory, we don't need to check the address
range,
+ * this is device memory physical address, could overlap with system
+ * memory.
+ */
+ if (HAS_LMEM(i915))
+ return 0;
+
/*
* Verify that nothing else uses this physical address. Stolen
* memory should be reserved by the BIOS and hidden from the
@@ -374,8 +383,9 @@ static void icl_get_stolen_reserved(struct
drm_i915_private *i915,
}
}
-static int i915_gem_init_stolen(struct drm_i915_private *i915)
+static int i915_gem_init_stolen(struct intel_memory_region *mem)
{
+ struct drm_i915_private *i915 = mem->i915;
struct intel_uncore *uncore = >uncore;
resource_size_t reserved_base, stolen_top;
resource_size_t reserved_total, reserved_size;
@@ -396,10 +406,10 @@ static int i915_gem_init_stolen(struct
drm_i915_private *i915)
return 0;
}
- if (resource_size(_graphics_stolen_res) == 0)
+ if (resource_size(>region) == 0)
return 0;
- i915->dsm = intel_graphics_stolen_res;
+ i915->dsm = mem->region;
if (i915_adjust_stolen(i915, >dsm))
return 0;
@@ -684,23 +694,36 @@ static int _i915_gem_object_stolen_init(struct
intel_memory_region *mem,
return ret;
}
+struct intel_memory_region *i915_stolen_region(struct
drm_i915_private *i915)
+{
+ if (HAS_LMEM(i915))
+ return i915->mm.regions[INTEL_REGION_STOLEN_LMEM];
+
+ return i915->mm.regions[INTEL_REGION_STOLEN_SMEM];
+}
Could be a bikeshedding comment only - especially since I think this
path gets very little used at runtime so it is most likely pointless to
fiddle with it, but it just strikes me a bit not fully elegant to do:
i915_gem_object_create_stolen
-> i915_gem_object_create_region
-> i915_stolen_region
And end up in here, when alternative could be at driver init:
i915->stolen_region_id = HAS_LMEM() ? ... : ...;
i915_gem_object_create_stolen
->
i915_gem_object_create_region(i915->mm.regions[i915->stolen_region_id]);
Or pointer to region. Would avoid having to export i915_stolen_region as
well.
Or is i915->dsm already the right thing? Because..
I guess we could just have an i915->stolen_region short-cut or something?
+
struct drm_i915_gem_object *
i915_gem_object_create_stolen(struct drm_i915_private *i915,
resource_size_t size)
{
- return
i915_gem_object_create_region(i915->mm.regions[INTEL_REGION_STOLEN_SMEM],
+ return i915_gem_object_create_region(i915_stolen_region(i915),
size, I915_BO_ALLOC_CONTIGUOUS);
}
static int init_stolen(struct intel_memory_region *mem)
{
- intel_memory_region_set_name(mem, "stolen");
+ if (HAS_LMEM(mem->i915)) {
+ if (!io_mapping_init_wc(>iomap,
+ mem->io_start,
+ resource_size(>region)))
+ return -EIO;
+ }
/*
* Initialise stolen early so that we may reserve preallocated
* objects for the BIOS to KMS transition.
*/
- return i915_gem_init_stolen(mem->i915);
+ return i915_gem_init_stolen(mem);
... I find the mem region init paths a bit convoluted, stolen
especially, and struggle to figure it out every time.
For instance we have i915_region_stolen_ops shared between system and
local stolen. But
Re: [Intel-gfx] [PATCH v19 6/6] drm/i915/selftests: Rename functions names
On Fri, Apr 16, 2021 at 10:23 PM Jani Nikula
wrote:
>
> On Thu, 15 Apr 2021, Hsin-Yi Wang wrote:
> > pm_resume and pm_suspend might be conflict with the ones defined in
> > include/linux/suspend.h. Rename pm_resume{suspend} to
> > i915_pm_resume{suspend} since they are only used here.
>
> I agree with the rationale here.
>
> Do you need this to be part of your series, or shall we just pick this
> up for i915? (We might consider renaming to something else or prefix the
> functions with _ though, as we also have existing i915_pm_suspend and
> i915_pm_resume elsewhere.)
>
This patch can be separated from the series, thanks.
> BR,
> Jani.
>
> >
> > Signed-off-by: Hsin-Yi Wang
> > Reported-by: kernel test robot
> > ---
> > drivers/gpu/drm/i915/selftests/i915_gem.c | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c
> > b/drivers/gpu/drm/i915/selftests/i915_gem.c
> > index dc394fb7ccfa..525afda9d31f 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
> > @@ -94,7 +94,7 @@ static int pm_prepare(struct drm_i915_private *i915)
> > return 0;
> > }
> >
> > -static void pm_suspend(struct drm_i915_private *i915)
> > +static void i915_pm_suspend(struct drm_i915_private *i915)
> > {
> > intel_wakeref_t wakeref;
> >
> > @@ -116,7 +116,7 @@ static void pm_hibernate(struct drm_i915_private *i915)
> > }
> > }
> >
> > -static void pm_resume(struct drm_i915_private *i915)
> > +static void i915_pm_resume(struct drm_i915_private *i915)
> > {
> > intel_wakeref_t wakeref;
> >
> > @@ -152,12 +152,12 @@ static int igt_gem_suspend(void *arg)
> > if (err)
> > goto out;
> >
> > - pm_suspend(i915);
> > + i915_pm_suspend(i915);
> >
> > /* Here be dragons! Note that with S3RST any S3 may become S4! */
> > simulate_hibernate(i915);
> >
> > - pm_resume(i915);
> > + i915_pm_resume(i915);
> >
> > err = switch_to_context(ctx);
> > out:
> > @@ -192,7 +192,7 @@ static int igt_gem_hibernate(void *arg)
> > /* Here be dragons! */
> > simulate_hibernate(i915);
> >
> > - pm_resume(i915);
> > + i915_pm_resume(i915);
> >
> > err = switch_to_context(ctx);
> > out:
>
> --
> Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/19] drm/i915/lmem: Bypass aperture when lmem is available
On 14/04/2021 16:33, Tvrtko Ursulin wrote:
On 12/04/2021 10:05, Matthew Auld wrote:
From: Anusha Srivatsa
In the scenario where local memory is available, we have
rely on CPU access via lmem directly instead of aperture.
v2:
gmch is only relevant for much older hw, therefore we can drop the
has_aperture check since it should always be present on such platforms.
(Chris)
Cc: Ville Syrjälä
Cc: Dhinakaran Pandiyan
Cc: Maarten Lankhorst
Cc: Chris P Wilson
Cc: Daniel Vetter
Cc: Joonas Lahtinen
Cc: Daniele Ceraolo Spurio
Cc: CQ Tang
Signed-off-by: Anusha Srivatsa
---
drivers/gpu/drm/i915/display/intel_fbdev.c | 22 +++---
drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 15 +++
drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 5 +
drivers/gpu/drm/i915/i915_vma.c | 19 +--
4 files changed, 48 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 2b37959da747..4af40229f5ec 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -139,14 +139,22 @@ static int intelfb_alloc(struct drm_fb_helper
*helper,
size = mode_cmd.pitches[0] * mode_cmd.height;
size = PAGE_ALIGN(size);
- /* If the FB is too big, just don't use it since fbdev is not very
- * important and we should probably use that space with FBC or other
- * features. */
obj = ERR_PTR(-ENODEV);
- if (size * 2 < dev_priv->stolen_usable_size)
- obj = i915_gem_object_create_stolen(dev_priv, size);
- if (IS_ERR(obj))
- obj = i915_gem_object_create_shmem(dev_priv, size);
+ if (HAS_LMEM(dev_priv)) {
+ obj = i915_gem_object_create_lmem(dev_priv, size,
+ I915_BO_ALLOC_CONTIGUOUS);
Has to be contiguous? Question for display experts I guess.
[Comes back later.] Ah for iomap? Put a comment to that effect perhaps?
I don't think it has to be, since we could in theory just use pin_map()
underneath, which can already deal with non-contiguous chunks of lmem,
although that might bring in ww locking. I think for now just add a
comment and mark this as XXX, and potentially revisit as follow up?
+ } else {
+ /*
+ * If the FB is too big, just don't use it since fbdev is not
very
+ * important and we should probably use that space with FBC
or other
+ * features.
+ */
+ if (size * 2 < dev_priv->stolen_usable_size)
+ obj = i915_gem_object_create_stolen(dev_priv, size);
+ if (IS_ERR(obj))
+ obj = i915_gem_object_create_shmem(dev_priv, size);
+ }
Could we keep the IS_ERR ordered allocation order to save having to
re-indent? Bike shed so optional..
+
if (IS_ERR(obj)) {
drm_err(_priv->drm, "failed to allocate framebuffer\n");
return PTR_ERR(obj);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
index 017db8f71130..f44bdd08f7cb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
@@ -17,6 +17,21 @@ const struct drm_i915_gem_object_ops
i915_gem_lmem_obj_ops = {
.release = i915_gem_object_release_memory_region,
};
+void __iomem *
+i915_gem_object_lmem_io_map(struct drm_i915_gem_object *obj,
+ unsigned long n,
+ unsigned long size)
+{
+ resource_size_t offset;
+
+ GEM_BUG_ON(!i915_gem_object_is_contiguous(obj));
+
+ offset = i915_gem_object_get_dma_address(obj, n);
+ offset -= obj->mm.region->region.start;
+
+ return io_mapping_map_wc(>mm.region->iomap, offset, size);
+}
+
bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj)
{
struct intel_memory_region *mr = obj->mm.region;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
index 036d53c01de9..fac6bc5a5ebb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
@@ -14,6 +14,11 @@ struct intel_memory_region;
extern const struct drm_i915_gem_object_ops i915_gem_lmem_obj_ops;
+void __iomem *
+i915_gem_object_lmem_io_map(struct drm_i915_gem_object *obj,
+ unsigned long n,
+ unsigned long size);
+
bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj);
struct drm_i915_gem_object *
diff --git a/drivers/gpu/drm/i915/i915_vma.c
b/drivers/gpu/drm/i915/i915_vma.c
index 07490db51cdc..e24d33aecac4 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -27,6 +27,7 @@
#include "display/intel_frontbuffer.h"
+#include "gem/i915_gem_lmem.h"
#include "gt/intel_engine.h"
#include "gt/intel_engine_heartbeat.h"
#include "gt/intel_gt.h"
@@ -448,9 +449,11 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma
*vma)
void __iomem *ptr;
int err;
- if
Re: [Intel-gfx] [PATCH v19 6/6] drm/i915/selftests: Rename functions names
On Thu, 15 Apr 2021, Hsin-Yi Wang wrote:
> pm_resume and pm_suspend might be conflict with the ones defined in
> include/linux/suspend.h. Rename pm_resume{suspend} to
> i915_pm_resume{suspend} since they are only used here.
I agree with the rationale here.
Do you need this to be part of your series, or shall we just pick this
up for i915? (We might consider renaming to something else or prefix the
functions with _ though, as we also have existing i915_pm_suspend and
i915_pm_resume elsewhere.)
BR,
Jani.
>
> Signed-off-by: Hsin-Yi Wang
> Reported-by: kernel test robot
> ---
> drivers/gpu/drm/i915/selftests/i915_gem.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c
> b/drivers/gpu/drm/i915/selftests/i915_gem.c
> index dc394fb7ccfa..525afda9d31f 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
> @@ -94,7 +94,7 @@ static int pm_prepare(struct drm_i915_private *i915)
> return 0;
> }
>
> -static void pm_suspend(struct drm_i915_private *i915)
> +static void i915_pm_suspend(struct drm_i915_private *i915)
> {
> intel_wakeref_t wakeref;
>
> @@ -116,7 +116,7 @@ static void pm_hibernate(struct drm_i915_private *i915)
> }
> }
>
> -static void pm_resume(struct drm_i915_private *i915)
> +static void i915_pm_resume(struct drm_i915_private *i915)
> {
> intel_wakeref_t wakeref;
>
> @@ -152,12 +152,12 @@ static int igt_gem_suspend(void *arg)
> if (err)
> goto out;
>
> - pm_suspend(i915);
> + i915_pm_suspend(i915);
>
> /* Here be dragons! Note that with S3RST any S3 may become S4! */
> simulate_hibernate(i915);
>
> - pm_resume(i915);
> + i915_pm_resume(i915);
>
> err = switch_to_context(ctx);
> out:
> @@ -192,7 +192,7 @@ static int igt_gem_hibernate(void *arg)
> /* Here be dragons! */
> simulate_hibernate(i915);
>
> - pm_resume(i915);
> + i915_pm_resume(i915);
>
> err = switch_to_context(ctx);
> out:
--
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/doc: add section for driver uAPI
On Fri, Apr 16, 2021 at 12:37 PM Matthew Auld wrote: > > Add section for drm/i915 uAPI and pull in i915_drm.h. > > Suggested-by: Daniel Vetter > Signed-off-by: Matthew Auld > Cc: Joonas Lahtinen > Cc: Jordan Justen > Cc: Daniel Vetter > Cc: Kenneth Graunke > Cc: Jason Ekstrand > Cc: Dave Airlie > Cc: dri-de...@lists.freedesktop.org > Cc: mesa-...@lists.freedesktop.org lgtm. Reviewed-by: Daniel Vetter > --- > Documentation/gpu/driver-uapi.rst | 8 > Documentation/gpu/index.rst | 1 + > 2 files changed, 9 insertions(+) > create mode 100644 Documentation/gpu/driver-uapi.rst > > diff --git a/Documentation/gpu/driver-uapi.rst > b/Documentation/gpu/driver-uapi.rst > new file mode 100644 > index ..4411e6919a3d > --- /dev/null > +++ b/Documentation/gpu/driver-uapi.rst > @@ -0,0 +1,8 @@ > +=== > +DRM Driver uAPI > +=== > + > +drm/i915 uAPI > += > + > +.. kernel-doc:: include/uapi/drm/i915_drm.h > diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst > index ec4bc72438e4..b9c1214d8f23 100644 > --- a/Documentation/gpu/index.rst > +++ b/Documentation/gpu/index.rst > @@ -10,6 +10,7 @@ Linux GPU Driver Developer's Guide > drm-kms > drm-kms-helpers > drm-uapi > + driver-uapi > drm-client > drivers > backlight > -- > 2.26.3 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/19] drm/i915/stolen: pass the allocation flags
On 14/04/2021 16:09, Tvrtko Ursulin wrote:
On 12/04/2021 10:05, Matthew Auld wrote:
From: CQ Tang
Stolen memory is always allocated as physically contiguous pages, mark
the object flags as such.
Signed-off-by: CQ Tang
Signed-off-by: Matthew Auld
---
drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index f713eabb7671..49a2dfcc8ba7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -633,14 +633,15 @@ static const struct drm_i915_gem_object_ops
i915_gem_object_stolen_ops = {
static int __i915_gem_object_create_stolen(struct
intel_memory_region *mem,
struct drm_i915_gem_object *obj,
- struct drm_mm_node *stolen)
+ struct drm_mm_node *stolen,
+ unsigned int flags)
{
static struct lock_class_key lock_class;
unsigned int cache_level;
int err;
drm_gem_private_object_init(>i915->drm, >base,
stolen->size);
- i915_gem_object_init(obj, _gem_object_stolen_ops,
_class, 0);
+ i915_gem_object_init(obj, _gem_object_stolen_ops,
_class, flags);
obj->stolen = stolen;
obj->read_domains = I915_GEM_DOMAIN_CPU | I915_GEM_DOMAIN_GTT;
@@ -682,7 +683,7 @@ static int _i915_gem_object_stolen_init(struct
intel_memory_region *mem,
if (ret)
goto err_free;
- ret = __i915_gem_object_create_stolen(mem, obj, stolen);
+ ret = __i915_gem_object_create_stolen(mem, obj, stolen, flags);
if (ret)
goto err_remove;
@@ -840,7 +841,8 @@
i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private
*i915,
goto err_stolen;
}
- ret = __i915_gem_object_create_stolen(mem, obj, stolen);
+ ret = __i915_gem_object_create_stolen(mem, obj, stolen,
+ I915_BO_ALLOC_CONTIGUOUS);
if (ret)
goto err_object_free;
Are all stolen objects always contiguous or only ones allocated by
i915_gem_object_create_stolen_for_preallocated? If former should
__i915_gem_object_create_stolen just set the flag without the need to
pass it in?
Yes, all stolen object are physically contiguous. Agreed, moving the
I915_BO_ALLOC_CONTIGUOUS into __i915_gem_object_create_stolen() makes
more sense here.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/4] drm/i915/uapi: fix kernel doc warnings
== Series Details == Series: series starting with [1/4] drm/i915/uapi: fix kernel doc warnings URL : https://patchwork.freedesktop.org/series/89155/ State : success == Summary == CI Bug Log - changes from CI_DRM_9976_full -> Patchwork_19943_full Summary --- **SUCCESS** No regressions found. Known issues Here are the changes found in Patchwork_19943_full that come from known issues: ### IGT changes ### Issues hit * igt@feature_discovery@display-3x: - shard-tglb: NOTRUN -> [SKIP][1] ([i915#1839]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-tglb3/igt@feature_discov...@display-3x.html * igt@gem_create@create-massive: - shard-skl: NOTRUN -> [DMESG-WARN][2] ([i915#3002]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-skl3/igt@gem_cre...@create-massive.html * igt@gem_ctx_persistence@engines-queued: - shard-snb: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#1099]) +1 similar issue [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-snb7/igt@gem_ctx_persiste...@engines-queued.html * igt@gem_ctx_persistence@many-contexts: - shard-tglb: [PASS][4] -> [FAIL][5] ([i915#2410]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/shard-tglb2/igt@gem_ctx_persiste...@many-contexts.html [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-tglb2/igt@gem_ctx_persiste...@many-contexts.html * igt@gem_ctx_persistence@smoketest: - shard-iclb: [PASS][6] -> [FAIL][7] ([i915#2896]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/shard-iclb4/igt@gem_ctx_persiste...@smoketest.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-iclb8/igt@gem_ctx_persiste...@smoketest.html * igt@gem_eio@in-flight-contexts-10ms: - shard-tglb: [PASS][8] -> [TIMEOUT][9] ([i915#3063]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/shard-tglb6/igt@gem_...@in-flight-contexts-10ms.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-tglb5/igt@gem_...@in-flight-contexts-10ms.html * igt@gem_exec_capture@pi@rcs0: - shard-skl: NOTRUN -> [INCOMPLETE][10] ([i915#2369]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-skl1/igt@gem_exec_capture@p...@rcs0.html * igt@gem_exec_fair@basic-flow@rcs0: - shard-tglb: [PASS][11] -> [FAIL][12] ([i915#2842]) +1 similar issue [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/shard-tglb7/igt@gem_exec_fair@basic-f...@rcs0.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-tglb1/igt@gem_exec_fair@basic-f...@rcs0.html * igt@gem_exec_fair@basic-none-solo@rcs0: - shard-kbl: NOTRUN -> [FAIL][13] ([i915#2842]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-kbl6/igt@gem_exec_fair@basic-none-s...@rcs0.html * igt@gem_exec_fair@basic-none@vcs1: - shard-iclb: NOTRUN -> [FAIL][14] ([i915#2842]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-iclb4/igt@gem_exec_fair@basic-n...@vcs1.html * igt@gem_exec_fair@basic-pace-solo@rcs0: - shard-glk: [PASS][15] -> [FAIL][16] ([i915#2842]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/shard-glk7/igt@gem_exec_fair@basic-pace-s...@rcs0.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-glk8/igt@gem_exec_fair@basic-pace-s...@rcs0.html * igt@gem_exec_fair@basic-pace@rcs0: - shard-kbl: [PASS][17] -> [FAIL][18] ([i915#2842]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/shard-kbl1/igt@gem_exec_fair@basic-p...@rcs0.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-kbl1/igt@gem_exec_fair@basic-p...@rcs0.html * igt@gem_exec_fair@basic-pace@vecs0: - shard-kbl: [PASS][19] -> [SKIP][20] ([fdo#109271]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/shard-kbl1/igt@gem_exec_fair@basic-p...@vecs0.html [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-kbl1/igt@gem_exec_fair@basic-p...@vecs0.html * igt@gem_exec_fair@basic-throttle@rcs0: - shard-tglb: NOTRUN -> [FAIL][21] ([i915#2842]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-tglb3/igt@gem_exec_fair@basic-throt...@rcs0.html - shard-iclb: [PASS][22] -> [FAIL][23] ([i915#2849]) [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/shard-iclb5/igt@gem_exec_fair@basic-throt...@rcs0.html [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/shard-iclb7/igt@gem_exec_fair@basic-throt...@rcs0.html * igt@gem_exec_suspend@basic-s3: - shard-kbl: [PASS][24] -> [DMESG-WARN][25] ([i915#180]) +3 similar issues [24]:
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/uapi: fix kernel doc warnings
== Series Details ==
Series: series starting with [1/4] drm/i915/uapi: fix kernel doc warnings
URL : https://patchwork.freedesktop.org/series/89155/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_9976 -> Patchwork_19943
Summary
---
**SUCCESS**
No regressions found.
External URL:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/index.html
Possible new issues
---
Here are the unknown changes that may have been introduced in Patchwork_19943:
### IGT changes ###
Suppressed
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@gem_exec_suspend@basic-s0:
- {fi-cml-drallion}: NOTRUN -> [INCOMPLETE][1]
[1]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/fi-cml-drallion/igt@gem_exec_susp...@basic-s0.html
Known issues
Here are the changes found in Patchwork_19943 that come from known issues:
### IGT changes ###
Issues hit
* igt@i915_selftest@live@hangcheck:
- fi-snb-2600:NOTRUN -> [INCOMPLETE][2] ([i915#2782])
[2]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/fi-snb-2600/igt@i915_selftest@l...@hangcheck.html
* igt@kms_chamelium@dp-crc-fast:
- fi-kbl-7500u: [PASS][3] -> [FAIL][4] ([i915#1161])
[3]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/fi-kbl-7500u/igt@kms_chamel...@dp-crc-fast.html
[4]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/fi-kbl-7500u/igt@kms_chamel...@dp-crc-fast.html
* igt@runner@aborted:
- fi-skl-6700k2: NOTRUN -> [FAIL][5] ([i915#1814])
[5]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/fi-skl-6700k2/igt@run...@aborted.html
Possible fixes
* igt@i915_module_load@reload:
- fi-tgl-y: [DMESG-WARN][6] ([i915#1982] / [k.org#205379]) ->
[PASS][7]
[6]:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9976/fi-tgl-y/igt@i915_module_l...@reload.html
[7]:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/fi-tgl-y/igt@i915_module_l...@reload.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#1161]: https://gitlab.freedesktop.org/drm/intel/issues/1161
[i915#1208]: https://gitlab.freedesktop.org/drm/intel/issues/1208
[i915#1814]: https://gitlab.freedesktop.org/drm/intel/issues/1814
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2782]: https://gitlab.freedesktop.org/drm/intel/issues/2782
[i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
[k.org#205379]: https://bugzilla.kernel.org/show_bug.cgi?id=205379
Participating hosts (45 -> 43)
--
Additional (1): fi-cml-drallion
Missing(3): fi-ilk-m540 fi-bsw-cyan fi-bdw-samus
Build changes
-
* Linux: CI_DRM_9976 -> Patchwork_19943
CI-20190529: 20190529
CI_DRM_9976: 73739c865de5c4c504adff6d9873ca5dea2dc704 @
git://anongit.freedesktop.org/gfx-ci/linux
IGT_6067: 14317b92a672d9a20cd04fc3b0c80e2fb12d51d5 @
git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_19943: 80ea253b8276ee1b2fce6e6fdfaf4520aebde3f9 @
git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
80ea253b8276 drm/i915/uapi: convert i915_query and friend to kernel doc
fb68e132c93e drm/i915/uapi: convert i915_user_extension to kernel doc
92879b3dd962 drm/doc: add section for driver uAPI
e98fe0216171 drm/i915/uapi: fix kernel doc warnings
== Logs ==
For more details see:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19943/index.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.DOCS: warning for series starting with [1/4] drm/i915/uapi: fix kernel doc warnings
== Series Details == Series: series starting with [1/4] drm/i915/uapi: fix kernel doc warnings URL : https://patchwork.freedesktop.org/series/89155/ State : warning == Summary == $ make htmldocs 2>&1 > /dev/null | grep i915 ./drivers/gpu/drm/i915/gem/i915_gem_shrinker.c:102: warning: Function parameter or member 'ww' not described in 'i915_gem_shrink' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function parameter 'trampoline' description in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'jump_whitelist' not described in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'shadow_map' not described in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Function parameter or member 'batch_map' not described in 'intel_engine_cmd_parser' ./drivers/gpu/drm/i915/i915_cmd_parser.c:1420: warning: Excess function parameter 'trampoline' description in 'intel_engine_cmd_parser' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/uapi: fix kernel doc warnings
== Series Details == Series: series starting with [1/4] drm/i915/uapi: fix kernel doc warnings URL : https://patchwork.freedesktop.org/series/89155/ State : warning == Summary == $ dim checkpatch origin/drm-tip e98fe0216171 drm/i915/uapi: fix kernel doc warnings 92879b3dd962 drm/doc: add section for driver uAPI -:20: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #20: new file mode 100644 -:25: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1 #25: FILE: Documentation/gpu/driver-uapi.rst:1: +=== total: 0 errors, 2 warnings, 0 checks, 15 lines checked fb68e132c93e drm/i915/uapi: convert i915_user_extension to kernel doc 80ea253b8276 drm/i915/uapi: convert i915_query and friend to kernel doc ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915/uapi: convert i915_query and friend to kernel doc
Add a note about the two-step process.
v2(Tvrtko):
- Also document the other method of just passing in a buffer which is
large enough, which avoids two ioctl calls. Can make sense for
smaller query items.
Suggested-by: Daniel Vetter
Signed-off-by: Matthew Auld
Cc: Joonas Lahtinen
Cc: Tvrtko Ursulin
Cc: Jordan Justen
Cc: Daniel Vetter
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Dave Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
Reviewed-by: Daniel Vetter
---
include/uapi/drm/i915_drm.h | 61 ++---
1 file changed, 50 insertions(+), 11 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index d79b51c12ff2..12f375c52317 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2218,14 +2218,23 @@ struct drm_i915_perf_oa_config {
__u64 flex_regs_ptr;
};
+/**
+ * struct drm_i915_query_item - An individual query for the kernel to process.
+ *
+ * The behaviour is determined by the @query_id. Note that exactly what
+ * @data_ptr is also depends on the specific @query_id.
+ */
struct drm_i915_query_item {
+ /** @query_id: The id for this query */
__u64 query_id;
#define DRM_I915_QUERY_TOPOLOGY_INFO1
#define DRM_I915_QUERY_ENGINE_INFO 2
#define DRM_I915_QUERY_PERF_CONFIG 3
/* Must be kept compact -- no holes and well documented */
- /*
+ /**
+* @length:
+*
* When set to zero by userspace, this is filled with the size of the
* data to be written at the data_ptr pointer. The kernel sets this
* value to a negative value to signal an error on a particular query
@@ -2233,21 +2242,26 @@ struct drm_i915_query_item {
*/
__s32 length;
- /*
+ /**
+* @flags:
+*
* When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
*
* When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
-* following :
-* - DRM_I915_QUERY_PERF_CONFIG_LIST
-* - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
-* - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
+* following:
+*
+* - DRM_I915_QUERY_PERF_CONFIG_LIST
+* - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
+* - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
*/
__u32 flags;
#define DRM_I915_QUERY_PERF_CONFIG_LIST 1
#define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
#define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID 3
- /*
+ /**
+* @data_ptr:
+*
* Data will be written at the location pointed by data_ptr when the
* value of length matches the length of the data to be written by the
* kernel.
@@ -2255,16 +2269,41 @@ struct drm_i915_query_item {
__u64 data_ptr;
};
+/**
+ * struct drm_i915_query - Supply an array of drm_i915_query_item for the
kernel
+ * to fill out.
+ *
+ * Note that this is generally a two step process for each drm_i915_query_item
+ * in the array:
+ *
+ * 1. Call the DRM_IOCTL_I915_QUERY, giving it our array of
drm_i915_query_item,
+ *with drm_i915_query_item.size set to zero. The kernel will then fill in
+ *the size, in bytes, which tells userspace how memory it needs to allocate
+ *for the blob(say for an array of properties).
+ *
+ * 2. Next we call DRM_IOCTL_I915_QUERY again, this time with the
+ *drm_i915_query_item.data_ptr equal to our newly allocated blob. Note that
+ *the i915_query_item.size should still be the same as what the kernel
+ *previously set. At this point the kernel can fill in the blob.
+ *
+ * Note that for some query items it can make sense for userspace to just pass
+ * in a buffer/blob equal to or larger than the required size. In this case
only
+ * a single ioctl call is needed. For some smaller query items this can work
+ * quite well.
+ *
+ */
struct drm_i915_query {
+ /** @num_items: The number of elements in the @items_ptr array */
__u32 num_items;
- /*
-* Unused for now. Must be cleared to zero.
+ /**
+* @flags: Unused for now. Must be cleared to zero.
*/
__u32 flags;
- /*
-* This points to an array of num_items drm_i915_query_item structures.
+ /**
+* @items_ptr: This points to an array of num_items drm_i915_query_item
+* structures.
*/
__u64 items_ptr;
};
--
2.26.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915/uapi: convert i915_user_extension to kernel doc
Add some example usage for the extension chaining also, which is quite
nifty.
v2: (Daniel)
- clarify that the name is just some integer, also document that the
name space is not global
Suggested-by: Daniel Vetter
Signed-off-by: Matthew Auld
Cc: Joonas Lahtinen
Cc: Jordan Justen
Cc: Daniel Vetter
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Dave Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
Reviewed-by: Daniel Vetter
---
include/uapi/drm/i915_drm.h | 54 ++---
1 file changed, 50 insertions(+), 4 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 92da48e935d1..d79b51c12ff2 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -62,8 +62,8 @@ extern "C" {
#define I915_ERROR_UEVENT "ERROR"
#define I915_RESET_UEVENT "RESET"
-/*
- * i915_user_extension: Base class for defining a chain of extensions
+/**
+ * struct i915_user_extension - Base class for defining a chain of extensions
*
* Many interfaces need to grow over time. In most cases we can simply
* extend the struct and have userspace pass in more data. Another option,
@@ -76,12 +76,58 @@ extern "C" {
* increasing complexity, and for large parts of that interface to be
* entirely optional. The downside is more pointer chasing; chasing across
* the __user boundary with pointers encapsulated inside u64.
+ *
+ * Example chaining:
+ *
+ * .. code-block:: C
+ *
+ * struct i915_user_extension ext3 {
+ * .next_extension = 0, // end
+ * .name = ...,
+ * };
+ * struct i915_user_extension ext2 {
+ * .next_extension = (uintptr_t),
+ * .name = ...,
+ * };
+ * struct i915_user_extension ext1 {
+ * .next_extension = (uintptr_t),
+ * .name = ...,
+ * };
+ *
+ * Typically the i915_user_extension would be embedded in some uAPI struct, and
+ * in this case we would feed it the head of the chain(i.e ext1), which would
+ * then apply all of the above extensions.
+ *
*/
struct i915_user_extension {
+ /**
+* @next_extension:
+*
+* Pointer to the next struct i915_user_extension, or zero if the end.
+*/
__u64 next_extension;
+ /**
+* @name: Name of the extension.
+*
+* Note that the name here is just some integer.
+*
+* Also note that the name space for this is not global for the whole
+* driver, but rather its scope/meaning is limited to the specific piece
+* of uAPI which has embedded the struct i915_user_extension.
+*/
__u32 name;
- __u32 flags; /* All undefined bits must be zero. */
- __u32 rsvd[4]; /* Reserved for future use; must be zero. */
+ /**
+* @flags: MBZ
+*
+* All undefined bits must be zero.
+*/
+ __u32 flags;
+ /**
+* @rsvd: MBZ
+*
+* Reserved for future use; must be zero.
+*/
+ __u32 rsvd[4];
};
/*
--
2.26.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/doc: add section for driver uAPI
Add section for drm/i915 uAPI and pull in i915_drm.h. Suggested-by: Daniel Vetter Signed-off-by: Matthew Auld Cc: Joonas Lahtinen Cc: Jordan Justen Cc: Daniel Vetter Cc: Kenneth Graunke Cc: Jason Ekstrand Cc: Dave Airlie Cc: dri-de...@lists.freedesktop.org Cc: mesa-...@lists.freedesktop.org --- Documentation/gpu/driver-uapi.rst | 8 Documentation/gpu/index.rst | 1 + 2 files changed, 9 insertions(+) create mode 100644 Documentation/gpu/driver-uapi.rst diff --git a/Documentation/gpu/driver-uapi.rst b/Documentation/gpu/driver-uapi.rst new file mode 100644 index ..4411e6919a3d --- /dev/null +++ b/Documentation/gpu/driver-uapi.rst @@ -0,0 +1,8 @@ +=== +DRM Driver uAPI +=== + +drm/i915 uAPI += + +.. kernel-doc:: include/uapi/drm/i915_drm.h diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst index ec4bc72438e4..b9c1214d8f23 100644 --- a/Documentation/gpu/index.rst +++ b/Documentation/gpu/index.rst @@ -10,6 +10,7 @@ Linux GPU Driver Developer's Guide drm-kms drm-kms-helpers drm-uapi + driver-uapi drm-client drivers backlight -- 2.26.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915/uapi: fix kernel doc warnings
Fix the cases where it is almost already valid kernel doc, for the
others just nerf the warnings for now.
Signed-off-by: Matthew Auld
Cc: Joonas Lahtinen
Cc: Jordan Justen
Cc: Daniel Vetter
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Dave Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
---
include/uapi/drm/i915_drm.h | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index ddc47bbf48b6..92da48e935d1 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1054,12 +1054,12 @@ struct drm_i915_gem_exec_fence {
__u32 flags;
};
-/**
+/*
* See drm_i915_gem_execbuffer_ext_timeline_fences.
*/
#define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
-/**
+/*
* This structure describes an array of drm_syncobj and associated points for
* timeline variants of drm_syncobj. It is invalid to append this structure to
* the execbuf if I915_EXEC_FENCE_ARRAY is set.
@@ -1700,7 +1700,7 @@ struct drm_i915_gem_context_param {
__u64 value;
};
-/**
+/*
* Context SSEU programming
*
* It may be necessary for either functional or performance reason to configure
@@ -2067,7 +2067,7 @@ struct drm_i915_perf_open_param {
__u64 properties_ptr;
};
-/**
+/*
* Enable data capture for a stream that was either opened in a disabled state
* via I915_PERF_FLAG_DISABLED or was later disabled via
* I915_PERF_IOCTL_DISABLE.
@@ -2081,7 +2081,7 @@ struct drm_i915_perf_open_param {
*/
#define I915_PERF_IOCTL_ENABLE _IO('i', 0x0)
-/**
+/*
* Disable data capture for a stream.
*
* It is an error to try and read a stream that is disabled.
@@ -2090,7 +2090,7 @@ struct drm_i915_perf_open_param {
*/
#define I915_PERF_IOCTL_DISABLE_IO('i', 0x1)
-/**
+/*
* Change metrics_set captured by a stream.
*
* If the stream is bound to a specific context, the configuration change
@@ -2103,7 +2103,7 @@ struct drm_i915_perf_open_param {
*/
#define I915_PERF_IOCTL_CONFIG _IO('i', 0x2)
-/**
+/*
* Common to all i915 perf records
*/
struct drm_i915_perf_record_header {
@@ -2151,7 +2151,7 @@ enum drm_i915_perf_record_type {
DRM_I915_PERF_RECORD_MAX /* non-ABI */
};
-/**
+/*
* Structure to upload perf dynamic configuration into the kernel.
*/
struct drm_i915_perf_oa_config {
@@ -2292,21 +2292,21 @@ struct drm_i915_query_topology_info {
* Describes one engine and it's capabilities as known to the driver.
*/
struct drm_i915_engine_info {
- /** Engine class and instance. */
+ /** @engine: Engine class and instance. */
struct i915_engine_class_instance engine;
- /** Reserved field. */
+ /** @rsvd0: Reserved field. */
__u32 rsvd0;
- /** Engine flags. */
+ /** @flags: Engine flags. */
__u64 flags;
- /** Capabilities of this engine. */
+ /** @capabilities: Capabilities of this engine. */
__u64 capabilities;
#define I915_VIDEO_CLASS_CAPABILITY_HEVC (1 << 0)
#define I915_VIDEO_AND_ENHANCE_CLASS_CAPABILITY_SFC(1 << 1)
- /** Reserved fields. */
+ /** @rsvd1: Reserved fields. */
__u64 rsvd1[4];
};
@@ -2317,13 +2317,13 @@ struct drm_i915_engine_info {
* an array of struct drm_i915_engine_info structures.
*/
struct drm_i915_query_engine_info {
- /** Number of struct drm_i915_engine_info structs following. */
+ /** @num_engines: Number of struct drm_i915_engine_info structs
following. */
__u32 num_engines;
- /** MBZ */
+ /** @rsvd: MBZ */
__u32 rsvd[3];
- /** Marker for drm_i915_engine_info structures. */
+ /** @engines: Marker for drm_i915_engine_info structures. */
struct drm_i915_engine_info engines[];
};
--
2.26.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 4/4] drm/doc/rfc: i915 DG1 uAPI
On Thu, Apr 15, 2021 at 04:59:58PM +0100, Matthew Auld wrote:
> Add an entry for the new uAPI needed for DG1.
>
> v2(Daniel):
> - include the overall upstreaming plan
> - add a note for mmap, there are differences here for TTM vs i915
> - bunch of other suggestions from Daniel
> v3:
> (Daniel)
> - add a note for set/get caching stuff
> - add some more docs for existing query and extensions stuff
> - add an actual code example for regions query
> - bunch of other stuff
> (Jason)
> - uAPI change(!):
> - try a simpler design with the placements extension
> - rather than have a generic setparam which can cover multiple
> use cases, have each extension be responsible for one thing
> only
>
> Signed-off-by: Matthew Auld
> Cc: Joonas Lahtinen
> Cc: Jordan Justen
> Cc: Daniel Vetter
> Cc: Kenneth Graunke
> Cc: Jason Ekstrand
> Cc: Dave Airlie
> Cc: dri-de...@lists.freedesktop.org
> Cc: mesa-...@lists.freedesktop.org
> ---
> Documentation/gpu/rfc/i915_gem_lmem.h | 255
> Documentation/gpu/rfc/i915_gem_lmem.rst | 139 +
> Documentation/gpu/rfc/index.rst | 4 +
> 3 files changed, 398 insertions(+)
> create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.h
> create mode 100644 Documentation/gpu/rfc/i915_gem_lmem.rst
>
> diff --git a/Documentation/gpu/rfc/i915_gem_lmem.h
> b/Documentation/gpu/rfc/i915_gem_lmem.h
> new file mode 100644
> index ..2a82a452e9f2
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_gem_lmem.h
> @@ -0,0 +1,255 @@
> +/*
> + * Note that drm_i915_query_item and drm_i915_query are existing bits of
> uAPI.
> + * For the regions query we are just adding a new query id, so no actual new
> + * ioctl or anything, but including it here for reference.
> + */
Oops I didn't realize this. I think the better/prettier way is to just
mention how it's built on top of the query ioctl and structs, and use
kerneldoc hyperlinks to point there. That way it's still easy to find, and
also serves as better documentation for the uapi when it's all merged.
See
https://dri.freedesktop.org/docs/drm/doc-guide/kernel-doc.html#highlights-and-cross-references
That's also why it matters that we pull the kerneldoc into our overall
documentation, otherwise the hyperlinks aren't working.
> +struct drm_i915_query_item {
> +#define DRM_I915_QUERY_MEMORY_REGIONS 0xdeadbeaf
> +
> +__u64 query_id;
> +
> +/*
> + * When set to zero by userspace, this is filled with the size of the
> + * data to be written at the data_ptr pointer. The kernel sets this
> + * value to a negative value to signal an error on a particular query
> + * item.
> + */
> +__s32 length;
> +
> +__u32 flags;
> +/*
> + * Data will be written at the location pointed by data_ptr when the
> + * value of length matches the length of the data to be written by
> the
> + * kernel.
> + */
> +__u64 data_ptr;
> +};
> +
> +struct drm_i915_query {
> +__u32 num_items;
> +/*
> + * Unused for now. Must be cleared to zero.
> + */
> +__u32 flags;
> +/*
> + * This points to an array of num_items drm_i915_query_item
> structures.
> + */
> +__u64 items_ptr;
> +};
> +
> +#define DRM_IOCTL_I915_QUERY DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_QUERY,
> struct drm_i915_query)
> +
> +/**
> + * enum drm_i915_gem_memory_class
> + */
> +enum drm_i915_gem_memory_class {
> + /** @I915_MEMORY_CLASS_SYSTEM: system memory */
> + I915_MEMORY_CLASS_SYSTEM = 0,
> + /** @I915_MEMORY_CLASS_DEVICE: device local-memory */
> + I915_MEMORY_CLASS_DEVICE,
> +};
> +
> +/**
> + * struct drm_i915_gem_memory_class_instance
> + */
> +struct drm_i915_gem_memory_class_instance {
> + /** @memory_class: see enum drm_i915_gem_memory_class */
> + __u16 memory_class;
> +
> + /** @memory_instance: which instance */
> + __u16 memory_instance;
> +};
> +
> +/**
> + * struct drm_i915_memory_region_info
> + *
> + * Describes one region as known to the driver.
> + *
> + * Note that we reserve quite a lot of stuff here for potential future work.
> As
> + * an example we might want expose the capabilities(see caps) for a given
> + * region, which could include things like if the region is CPU
> + * mappable/accessible etc.
> + */
> +struct drm_i915_memory_region_info {
> + /** @region: class:instance pair encoding */
> + struct drm_i915_gem_memory_class_instance region;
> +
> + /** @rsvd0: MBZ */
> + __u32 rsvd0;
> +
> + /** @caps: MBZ */
> + __u64 caps;
> +
> + /** @flags: MBZ */
> + __u64 flags;
> +
> + /** @probed_size: Memory probed by the driver (-1 = unknown) */
> + __u64 probed_size;
> +
> + /** @unallocated_size: Estimate of memory remaining (-1 = unknown) */
> + __u64 unallocated_size;
> +
> + /** @rsvd1: MBZ */
> +
Re: [Intel-gfx] [Mesa-dev] [PATCH v3 3/4] drm/i915/uapi: convert i915_query and friend to kernel doc
On Fri, Apr 16, 2021 at 12:25 AM Ian Romanick wrote:
> On 4/15/21 8:59 AM, Matthew Auld wrote:
> > Add a note about the two-step process.
> >
> > Suggested-by: Daniel Vetter
> > Signed-off-by: Matthew Auld
> > Cc: Joonas Lahtinen
> > Cc: Jordan Justen
> > Cc: Daniel Vetter
> > Cc: Kenneth Graunke
> > Cc: Jason Ekstrand
> > Cc: Dave Airlie
> > Cc: dri-de...@lists.freedesktop.org
> > Cc: mesa-...@lists.freedesktop.org
> > ---
> > include/uapi/drm/i915_drm.h | 57 ++---
> > 1 file changed, 46 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > index d9c954a5a456..ef36f1a0adde 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -2210,14 +2210,23 @@ struct drm_i915_perf_oa_config {
> > __u64 flex_regs_ptr;
> > };
> >
> > +/**
> > + * struct drm_i915_query_item - An individual query for the kernel to
> > process.
> > + *
> > + * The behaviour is determined by the @query_id. Note that exactly what
>
> Since we just had a big discussion about this on mesa-dev w.r.t. Mesa
> code and documentation... does the kernel have a policy about which
> flavor (pun intended) of English should be used?
I'm not finding it documented in
https://dri.freedesktop.org/docs/drm/doc-guide/sphinx.html but I
thought we've discussed it. Adding linux-doc and Jon Corbet.
-Daniel
>
> > + * @data_ptr is also depends on the specific @query_id.
> > + */
> > struct drm_i915_query_item {
> > + /** @query_id: The id for this query */
> > __u64 query_id;
> > #define DRM_I915_QUERY_TOPOLOGY_INFO1
> > #define DRM_I915_QUERY_ENGINE_INFO 2
> > #define DRM_I915_QUERY_PERF_CONFIG 3
> > /* Must be kept compact -- no holes and well documented */
> >
> > - /*
> > + /**
> > + * @length:
> > + *
> >* When set to zero by userspace, this is filled with the size of the
> >* data to be written at the data_ptr pointer. The kernel sets this
> >* value to a negative value to signal an error on a particular query
> > @@ -2225,21 +2234,26 @@ struct drm_i915_query_item {
> >*/
> > __s32 length;
> >
> > - /*
> > + /**
> > + * @flags:
> > + *
> >* When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
> >*
> >* When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
> > - * following :
> > - * - DRM_I915_QUERY_PERF_CONFIG_LIST
> > - * - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> > - * - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
> > + * following:
> > + *
> > + * - DRM_I915_QUERY_PERF_CONFIG_LIST
> > + * - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> > + * - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
> >*/
> > __u32 flags;
> > #define DRM_I915_QUERY_PERF_CONFIG_LIST 1
> > #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
> > #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID 3
> >
> > - /*
> > + /**
> > + * @data_ptr:
> > + *
> >* Data will be written at the location pointed by data_ptr when the
> >* value of length matches the length of the data to be written by the
> >* kernel.
> > @@ -2247,16 +2261,37 @@ struct drm_i915_query_item {
> > __u64 data_ptr;
> > };
> >
> > +/**
> > + * struct drm_i915_query - Supply an array of drm_i915_query_item for the
> > kernel
> > + * to fill out.
> > + *
> > + * Note that this is generally a two step process for each
> > drm_i915_query_item
> > + * in the array:
> > + *
> > + * 1.) Call the DRM_IOCTL_I915_QUERY, giving it our array of
> > + * drm_i915_query_item, with drm_i915_query_item.size set to zero. The
> > + * kernel will then fill in the size, in bytes, which tells userspace how
> > + * memory it needs to allocate for the blob(say for an array of
> > + * properties).
> > + *
> > + * 2.) Next we call DRM_IOCTL_I915_QUERY again, this time with the
> > + * drm_i915_query_item.data_ptr equal to our newly allocated blob. Note
> > + * that the i915_query_item.size should still be the same as what the
> > + * kernel previously set. At this point the kernel can fill in the blob.
> > + *
> > + */
> > struct drm_i915_query {
> > + /** @num_items: The number of elements in the @items_ptr array */
> > __u32 num_items;
> >
> > - /*
> > - * Unused for now. Must be cleared to zero.
> > + /**
> > + * @flags: Unused for now. Must be cleared to zero.
> >*/
> > __u32 flags;
> >
> > - /*
> > - * This points to an array of num_items drm_i915_query_item
> > structures.
> > + /**
> > + * @items_ptr: This points to an array of num_items
> > drm_i915_query_item
> > + * structures.
> >*/
> > __u64 items_ptr;
> > };
> >
>
> ___
> mesa-dev mailing list
> mesa-...@lists.freedesktop.org
>
Re: [Intel-gfx] [PATCH v3 1/4] drm/i915/uapi: hide kernel doc warnings
On Fri, Apr 16, 2021 at 10:44:28AM +0200, Daniel Vetter wrote:
> On Thu, Apr 15, 2021 at 04:59:55PM +0100, Matthew Auld wrote:
> > It's not properly formatted kernel doc, just nerf the warnings for now.
> >
> > Signed-off-by: Matthew Auld
> > Cc: Joonas Lahtinen
> > Cc: Jordan Justen
> > Cc: Daniel Vetter
> > Cc: Kenneth Graunke
> > Cc: Jason Ekstrand
> > Cc: Dave Airlie
> > Cc: dri-de...@lists.freedesktop.org
> > Cc: mesa-...@lists.freedesktop.org
>
> Reviewed-by: Daniel Vetter
Ok I need to revise, we need to pull this into Documentation/gpu/. I think
best would be to create a new driver-uapi.rst file, put it right after
drm-uapi.rst, and then add a section for drm/i915 uapi or similar.
Also since pxp patches, Jason's ctx cleanup and lmem all need this prep
work in patches 1-3 here, can you pls just resend those with the review
feedback so we can fast-track merging?
Thanks, Daniel
>
> > ---
> > include/uapi/drm/i915_drm.h | 16
> > 1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > index ddc47bbf48b6..a50257cde9ff 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -1054,12 +1054,12 @@ struct drm_i915_gem_exec_fence {
> > __u32 flags;
> > };
> >
> > -/**
> > +/*
> > * See drm_i915_gem_execbuffer_ext_timeline_fences.
> > */
> > #define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
> >
> > -/**
> > +/*
> > * This structure describes an array of drm_syncobj and associated points
> > for
> > * timeline variants of drm_syncobj. It is invalid to append this
> > structure to
> > * the execbuf if I915_EXEC_FENCE_ARRAY is set.
> > @@ -1700,7 +1700,7 @@ struct drm_i915_gem_context_param {
> > __u64 value;
> > };
> >
> > -/**
> > +/*
> > * Context SSEU programming
> > *
> > * It may be necessary for either functional or performance reason to
> > configure
> > @@ -2067,7 +2067,7 @@ struct drm_i915_perf_open_param {
> > __u64 properties_ptr;
> > };
> >
> > -/**
> > +/*
> > * Enable data capture for a stream that was either opened in a disabled
> > state
> > * via I915_PERF_FLAG_DISABLED or was later disabled via
> > * I915_PERF_IOCTL_DISABLE.
> > @@ -2081,7 +2081,7 @@ struct drm_i915_perf_open_param {
> > */
> > #define I915_PERF_IOCTL_ENABLE _IO('i', 0x0)
> >
> > -/**
> > +/*
> > * Disable data capture for a stream.
> > *
> > * It is an error to try and read a stream that is disabled.
> > @@ -2090,7 +2090,7 @@ struct drm_i915_perf_open_param {
> > */
> > #define I915_PERF_IOCTL_DISABLE_IO('i', 0x1)
> >
> > -/**
> > +/*
> > * Change metrics_set captured by a stream.
> > *
> > * If the stream is bound to a specific context, the configuration change
> > @@ -2103,7 +2103,7 @@ struct drm_i915_perf_open_param {
> > */
> > #define I915_PERF_IOCTL_CONFIG _IO('i', 0x2)
> >
> > -/**
> > +/*
> > * Common to all i915 perf records
> > */
> > struct drm_i915_perf_record_header {
> > @@ -2151,7 +2151,7 @@ enum drm_i915_perf_record_type {
> > DRM_I915_PERF_RECORD_MAX /* non-ABI */
> > };
> >
> > -/**
> > +/*
> > * Structure to upload perf dynamic configuration into the kernel.
> > */
> > struct drm_i915_perf_oa_config {
> > --
> > 2.26.3
> >
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/4] drm/i915/uapi: convert i915_query and friend to kernel doc
On Thu, Apr 15, 2021 at 04:59:57PM +0100, Matthew Auld wrote:
> Add a note about the two-step process.
>
> Suggested-by: Daniel Vetter
> Signed-off-by: Matthew Auld
> Cc: Joonas Lahtinen
> Cc: Jordan Justen
> Cc: Daniel Vetter
> Cc: Kenneth Graunke
> Cc: Jason Ekstrand
> Cc: Dave Airlie
> Cc: dri-de...@lists.freedesktop.org
> Cc: mesa-...@lists.freedesktop.org
> ---
> include/uapi/drm/i915_drm.h | 57 ++---
> 1 file changed, 46 insertions(+), 11 deletions(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index d9c954a5a456..ef36f1a0adde 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -2210,14 +2210,23 @@ struct drm_i915_perf_oa_config {
> __u64 flex_regs_ptr;
> };
>
> +/**
> + * struct drm_i915_query_item - An individual query for the kernel to
> process.
> + *
> + * The behaviour is determined by the @query_id. Note that exactly what
> + * @data_ptr is also depends on the specific @query_id.
> + */
> struct drm_i915_query_item {
> + /** @query_id: The id for this query */
> __u64 query_id;
> #define DRM_I915_QUERY_TOPOLOGY_INFO1
> #define DRM_I915_QUERY_ENGINE_INFO 2
> #define DRM_I915_QUERY_PERF_CONFIG 3
> /* Must be kept compact -- no holes and well documented */
>
> - /*
> + /**
> + * @length:
> + *
>* When set to zero by userspace, this is filled with the size of the
>* data to be written at the data_ptr pointer. The kernel sets this
>* value to a negative value to signal an error on a particular query
> @@ -2225,21 +2234,26 @@ struct drm_i915_query_item {
>*/
> __s32 length;
>
> - /*
> + /**
> + * @flags:
> + *
>* When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
>*
>* When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
> - * following :
> - * - DRM_I915_QUERY_PERF_CONFIG_LIST
> - * - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> - * - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
> + * following:
> + *
> + * - DRM_I915_QUERY_PERF_CONFIG_LIST
> + * - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> + * - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
>*/
> __u32 flags;
> #define DRM_I915_QUERY_PERF_CONFIG_LIST 1
> #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
> #define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID 3
>
> - /*
> + /**
> + * @data_ptr:
> + *
>* Data will be written at the location pointed by data_ptr when the
>* value of length matches the length of the data to be written by the
>* kernel.
> @@ -2247,16 +2261,37 @@ struct drm_i915_query_item {
> __u64 data_ptr;
> };
>
> +/**
> + * struct drm_i915_query - Supply an array of drm_i915_query_item for the
> kernel
> + * to fill out.
> + *
> + * Note that this is generally a two step process for each
> drm_i915_query_item
> + * in the array:
> + *
> + * 1.) Call the DRM_IOCTL_I915_QUERY, giving it our array of
I'm not sure this results in pretty rendering in htmldocs output. Please
check this.
This also made me realize that we're not pulling any of this into the drm
documents at all. I'll revise my review on patch 1.
Docs here look good:
Reviewed-by: Daniel Vetter
> + * drm_i915_query_item, with drm_i915_query_item.size set to zero. The
> + * kernel will then fill in the size, in bytes, which tells userspace how
> + * memory it needs to allocate for the blob(say for an array of
> + * properties).
> + *
> + * 2.) Next we call DRM_IOCTL_I915_QUERY again, this time with the
> + * drm_i915_query_item.data_ptr equal to our newly allocated blob. Note
> + * that the i915_query_item.size should still be the same as what the
> + * kernel previously set. At this point the kernel can fill in the blob.
> + *
> + */
> struct drm_i915_query {
> + /** @num_items: The number of elements in the @items_ptr array */
> __u32 num_items;
>
> - /*
> - * Unused for now. Must be cleared to zero.
> + /**
> + * @flags: Unused for now. Must be cleared to zero.
>*/
> __u32 flags;
>
> - /*
> - * This points to an array of num_items drm_i915_query_item structures.
> + /**
> + * @items_ptr: This points to an array of num_items drm_i915_query_item
> + * structures.
>*/
> __u64 items_ptr;
> };
> --
> 2.26.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/4] drm/i915/uapi: convert i915_user_extension to kernel doc
On Thu, Apr 15, 2021 at 04:59:56PM +0100, Matthew Auld wrote:
> Add some example usage for the extension chaining also, which is quite
> nifty.
>
> Suggested-by: Daniel Vetter
> Signed-off-by: Matthew Auld
> Cc: Joonas Lahtinen
> Cc: Jordan Justen
> Cc: Daniel Vetter
> Cc: Kenneth Graunke
> Cc: Jason Ekstrand
> Cc: Dave Airlie
> Cc: dri-de...@lists.freedesktop.org
> Cc: mesa-...@lists.freedesktop.org
> ---
> include/uapi/drm/i915_drm.h | 46 +
> 1 file changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index a50257cde9ff..d9c954a5a456 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -62,8 +62,8 @@ extern "C" {
> #define I915_ERROR_UEVENT"ERROR"
> #define I915_RESET_UEVENT"RESET"
>
> -/*
> - * i915_user_extension: Base class for defining a chain of extensions
> +/**
> + * struct i915_user_extension - Base class for defining a chain of extensions
> *
> * Many interfaces need to grow over time. In most cases we can simply
> * extend the struct and have userspace pass in more data. Another option,
> @@ -76,12 +76,50 @@ extern "C" {
> * increasing complexity, and for large parts of that interface to be
> * entirely optional. The downside is more pointer chasing; chasing across
> * the __user boundary with pointers encapsulated inside u64.
> + *
> + * Example chaining:
> + *
> + * .. code-block:: C
> + *
> + * struct i915_user_extension ext3 {
> + * .next_extension = 0, // end
> + * .name = ...,
> + * };
> + * struct i915_user_extension ext2 {
> + * .next_extension = (uintptr_t),
> + * .name = ...,
> + * };
> + * struct i915_user_extension ext1 {
> + * .next_extension = (uintptr_t),
> + * .name = ...,
> + * };
> + *
> + * Typically the i915_user_extension would be embedded in some uAPI struct,
> and
> + * in this case we would feed it the head of the chain(i.e ext1), which would
> + * then apply all of the above extensions.
> + *
> */
> struct i915_user_extension {
> + /**
> + * @next_extension:
> + *
> + * Pointer to the next i915_user_extension, or zero if the end.
> + */
> __u64 next_extension;
> + /** @name: Name of the extension */
Maybe clarify that the namespace here is per ioctl, not global. And maybe
also that the name is just an integer #define or something like that.
Either this is solid documentation, either way:
Reviewed-by: Daniel Vetter
> __u32 name;
> - __u32 flags; /* All undefined bits must be zero. */
> - __u32 rsvd[4]; /* Reserved for future use; must be zero. */
> + /**
> + * @flags: MBZ
> + *
> + * All undefined bits must be zero.
> + */
> + __u32 flags;
> + /**
> + * @rsvd: MBZ
> + *
> + * Reserved for future use; must be zero.
> + */
> + __u32 rsvd[4];
> };
>
> /*
> --
> 2.26.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/4] drm/i915/uapi: hide kernel doc warnings
On Thu, Apr 15, 2021 at 04:59:55PM +0100, Matthew Auld wrote:
> It's not properly formatted kernel doc, just nerf the warnings for now.
>
> Signed-off-by: Matthew Auld
> Cc: Joonas Lahtinen
> Cc: Jordan Justen
> Cc: Daniel Vetter
> Cc: Kenneth Graunke
> Cc: Jason Ekstrand
> Cc: Dave Airlie
> Cc: dri-de...@lists.freedesktop.org
> Cc: mesa-...@lists.freedesktop.org
Reviewed-by: Daniel Vetter
> ---
> include/uapi/drm/i915_drm.h | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index ddc47bbf48b6..a50257cde9ff 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1054,12 +1054,12 @@ struct drm_i915_gem_exec_fence {
> __u32 flags;
> };
>
> -/**
> +/*
> * See drm_i915_gem_execbuffer_ext_timeline_fences.
> */
> #define DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES 0
>
> -/**
> +/*
> * This structure describes an array of drm_syncobj and associated points for
> * timeline variants of drm_syncobj. It is invalid to append this structure
> to
> * the execbuf if I915_EXEC_FENCE_ARRAY is set.
> @@ -1700,7 +1700,7 @@ struct drm_i915_gem_context_param {
> __u64 value;
> };
>
> -/**
> +/*
> * Context SSEU programming
> *
> * It may be necessary for either functional or performance reason to
> configure
> @@ -2067,7 +2067,7 @@ struct drm_i915_perf_open_param {
> __u64 properties_ptr;
> };
>
> -/**
> +/*
> * Enable data capture for a stream that was either opened in a disabled
> state
> * via I915_PERF_FLAG_DISABLED or was later disabled via
> * I915_PERF_IOCTL_DISABLE.
> @@ -2081,7 +2081,7 @@ struct drm_i915_perf_open_param {
> */
> #define I915_PERF_IOCTL_ENABLE _IO('i', 0x0)
>
> -/**
> +/*
> * Disable data capture for a stream.
> *
> * It is an error to try and read a stream that is disabled.
> @@ -2090,7 +2090,7 @@ struct drm_i915_perf_open_param {
> */
> #define I915_PERF_IOCTL_DISABLE _IO('i', 0x1)
>
> -/**
> +/*
> * Change metrics_set captured by a stream.
> *
> * If the stream is bound to a specific context, the configuration change
> @@ -2103,7 +2103,7 @@ struct drm_i915_perf_open_param {
> */
> #define I915_PERF_IOCTL_CONFIG _IO('i', 0x2)
>
> -/**
> +/*
> * Common to all i915 perf records
> */
> struct drm_i915_perf_record_header {
> @@ -2151,7 +2151,7 @@ enum drm_i915_perf_record_type {
> DRM_I915_PERF_RECORD_MAX /* non-ABI */
> };
>
> -/**
> +/*
> * Structure to upload perf dynamic configuration into the kernel.
> */
> struct drm_i915_perf_oa_config {
> --
> 2.26.3
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/4] drm/i915/uapi: convert i915_query and friend to kernel doc
On 15/04/2021 16:59, Matthew Auld wrote:
Add a note about the two-step process.
Suggested-by: Daniel Vetter
Signed-off-by: Matthew Auld
Cc: Joonas Lahtinen
Cc: Jordan Justen
Cc: Daniel Vetter
Cc: Kenneth Graunke
Cc: Jason Ekstrand
Cc: Dave Airlie
Cc: dri-de...@lists.freedesktop.org
Cc: mesa-...@lists.freedesktop.org
---
include/uapi/drm/i915_drm.h | 57 ++---
1 file changed, 46 insertions(+), 11 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index d9c954a5a456..ef36f1a0adde 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -2210,14 +2210,23 @@ struct drm_i915_perf_oa_config {
__u64 flex_regs_ptr;
};
+/**
+ * struct drm_i915_query_item - An individual query for the kernel to process.
+ *
+ * The behaviour is determined by the @query_id. Note that exactly what
+ * @data_ptr is also depends on the specific @query_id.
+ */
struct drm_i915_query_item {
+ /** @query_id: The id for this query */
__u64 query_id;
#define DRM_I915_QUERY_TOPOLOGY_INFO1
#define DRM_I915_QUERY_ENGINE_INFO2
#define DRM_I915_QUERY_PERF_CONFIG 3
/* Must be kept compact -- no holes and well documented */
- /*
+ /**
+* @length:
+*
* When set to zero by userspace, this is filled with the size of the
* data to be written at the data_ptr pointer. The kernel sets this
* value to a negative value to signal an error on a particular query
@@ -2225,21 +2234,26 @@ struct drm_i915_query_item {
*/
__s32 length;
- /*
+ /**
+* @flags:
+*
* When query_id == DRM_I915_QUERY_TOPOLOGY_INFO, must be 0.
*
* When query_id == DRM_I915_QUERY_PERF_CONFIG, must be one of the
-* following :
-* - DRM_I915_QUERY_PERF_CONFIG_LIST
-* - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
-* - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
+* following:
+*
+* - DRM_I915_QUERY_PERF_CONFIG_LIST
+* - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
+* - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
*/
__u32 flags;
#define DRM_I915_QUERY_PERF_CONFIG_LIST 1
#define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID 2
#define DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_ID 3
- /*
+ /**
+* @data_ptr:
+*
* Data will be written at the location pointed by data_ptr when the
* value of length matches the length of the data to be written by the
* kernel.
@@ -2247,16 +2261,37 @@ struct drm_i915_query_item {
__u64 data_ptr;
};
+/**
+ * struct drm_i915_query - Supply an array of drm_i915_query_item for the
kernel
+ * to fill out.
+ *
+ * Note that this is generally a two step process for each drm_i915_query_item
+ * in the array:
+ *
+ * 1.) Call the DRM_IOCTL_I915_QUERY, giving it our array of
+ * drm_i915_query_item, with drm_i915_query_item.size set to zero. The
+ * kernel will then fill in the size, in bytes, which tells userspace how
+ * memory it needs to allocate for the blob(say for an array of
+ * properties).
+ *
+ * 2.) Next we call DRM_IOCTL_I915_QUERY again, this time with the
+ * drm_i915_query_item.data_ptr equal to our newly allocated blob. Note
+ * that the i915_query_item.size should still be the same as what the
+ * kernel previously set. At this point the kernel can fill in the blob.
I'd also document the one step alternative where userspace passes in a
buffer equal or larger than the required size. For many small queries
(most?) this actually works just as well and with one ioctl only.
Regards,
Tvrtko
+ *
+ */
struct drm_i915_query {
+ /** @num_items: The number of elements in the @items_ptr array */
__u32 num_items;
- /*
-* Unused for now. Must be cleared to zero.
+ /**
+* @flags: Unused for now. Must be cleared to zero.
*/
__u32 flags;
- /*
-* This points to an array of num_items drm_i915_query_item structures.
+ /**
+* @items_ptr: This points to an array of num_items drm_i915_query_item
+* structures.
*/
__u64 items_ptr;
};
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/modifiers: Enforce consistency between the cap an IN_FORMATS
Reviewed-by: Simon Ser ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
