Jordan Justen <jordan.l.justen@xxxxxxxxx> writes: > John, Rodrigo, > > It is now clear to me just how dependent i915 is going to be on the > closed source guc software, and that's just a fact of life for our > graphics stack going forward. > > In that context, it seems kind of pointless for me to make a big deal > out of this peripheral "query item" commit message. I still think > something as simple and to the point as: > > "In this interface i915 is returning a blob of data which it receives > from the guc software. This blob provides some useful data about the > hardware for drivers. The format of this blob will be documented in the > Programmer Reference Manuals when released." As I said on the internal email thread, *if you use my commit message suggestion*, then, begrudgingly, you can add my: Acked-by: Jordan Justen <jordan.l.justen@xxxxxxxxx> to the patch. Regardless of the commit message, I think you can add: Tested-by: Jordan Justen <jordan.l.justen@xxxxxxxxx> In truth, I've only tested this via the "prelim" i915 Linux uapi fork on an internal kernel tree, but I think that probably is close enough. In case you find it helpful, maybe: Ref: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14511 -Jordan > > ... would be better, but obviously this is really just down in the noise > in terms of concerns about the greater issue. So, feel free (to > continue) to ignore my suggestion. > > Sorry for having wasted your time, > > -Jordan > > John.C.Harrison@xxxxxxxxx writes: > >> From: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> >> >> GuC contains a consolidated table with a bunch of information about the >> current device. >> >> Previously, this information was spread and hardcoded to all the components >> including GuC, i915 and various UMDs. The goal here is to consolidate >> the data into GuC in a way that all interested components can grab the >> very latest and synchronized information using a simple query. >> >> As per most of the other queries, this one can be called twice. >> Once with item.length=0 to determine the exact buffer size, then >> allocate the user memory and call it again for to retrieve the >> table data. For example: >> struct drm_i915_query_item item = { >> .query_id = DRM_I915_QUERY_HWCONCFIG_TABLE; >> }; >> query.items_ptr = (int64_t) &item; >> query.num_items = 1; >> >> ioctl(fd, DRM_IOCTL_I915_QUERY, query, sizeof(query)); >> >> if (item.length <= 0) >> return -ENOENT; >> >> data = malloc(item.length); >> item.data_ptr = (int64_t) &data; >> ioctl(fd, DRM_IOCTL_I915_QUERY, query, sizeof(query)); >> >> // Parse the data as appropriate... >> >> The returned array is a simple and flexible KLV (Key/Length/Value) >> formatted table. For example, it could be just: >> enum device_attr { >> ATTR_SOME_VALUE = 0, >> ATTR_SOME_MASK = 1, >> }; >> >> static const u32 hwconfig[] = { >> ATTR_SOME_VALUE, >> 1, // Value Length in DWords >> 8, // Value >> >> ATTR_SOME_MASK, >> 3, >> 0x00FFFFFFFF, 0xFFFFFFFF, 0xFF000000, >> }; >> >> The attribute ids are defined in a hardware spec. >> >> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> >> Cc: Kenneth Graunke <kenneth.w.graunke@xxxxxxxxx> >> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> >> Cc: Slawomir Milczarek <slawomir.milczarek@xxxxxxxxx> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> >> Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx> >> Reviewed-by: Matthew Brost <matthew.brost@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/i915_query.c | 23 +++++++++++++++++++++++ >> include/uapi/drm/i915_drm.h | 1 + >> 2 files changed, 24 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c >> index 2dfbc22857a3..609e64d5f395 100644 >> --- a/drivers/gpu/drm/i915/i915_query.c >> +++ b/drivers/gpu/drm/i915/i915_query.c >> @@ -479,12 +479,35 @@ static int query_memregion_info(struct drm_i915_private *i915, >> return total_length; >> } >> >> +static int query_hwconfig_table(struct drm_i915_private *i915, >> + struct drm_i915_query_item *query_item) >> +{ >> + struct intel_gt *gt = to_gt(i915); >> + struct intel_guc_hwconfig *hwconfig = >->uc.guc.hwconfig; >> + >> + if (!hwconfig->size || !hwconfig->ptr) >> + return -ENODEV; >> + >> + if (query_item->length == 0) >> + return hwconfig->size; >> + >> + if (query_item->length < hwconfig->size) >> + return -EINVAL; >> + >> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), >> + hwconfig->ptr, hwconfig->size)) >> + return -EFAULT; >> + >> + return hwconfig->size; >> +} >> + >> static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv, >> struct drm_i915_query_item *query_item) = { >> query_topology_info, >> query_engine_info, >> query_perf_config, >> query_memregion_info, >> + query_hwconfig_table, >> }; >> >> int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file) >> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h >> index 914ebd9290e5..132515199f27 100644 >> --- a/include/uapi/drm/i915_drm.h >> +++ b/include/uapi/drm/i915_drm.h >> @@ -2685,6 +2685,7 @@ struct drm_i915_query_item { >> #define DRM_I915_QUERY_ENGINE_INFO 2 >> #define DRM_I915_QUERY_PERF_CONFIG 3 >> #define DRM_I915_QUERY_MEMORY_REGIONS 4 >> +#define DRM_I915_QUERY_HWCONFIG_TABLE 5 >> /* Must be kept compact -- no holes and well documented */ >> >> /** >> -- >> 2.25.1
