On Thu, Mar 10, 2022 at 12:26:12PM +0000, Tvrtko Ursulin wrote:
>
> On 10/03/2022 05:18, Matt Atwood wrote:
> > Newer platforms have DSS that aren't necessarily available for both
> > geometry and compute, two queries will need to exist. This introduces
> > the first, when passing a valid engine class and engine instance in the
> > flags returns a topology describing geometry.
> >
> > v2: fix white space errors
> >
> > Cc: Ashutosh Dixit <ashutosh.di...@intel.com>
> > Cc: Matt Roper <matthew.d.ro...@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> > UMD (mesa): https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14143
> > Signed-off-by: Matt Atwood <matthew.s.atw...@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_query.c | 68 ++++++++++++++++++++++---------
> > include/uapi/drm/i915_drm.h | 24 +++++++----
> > 2 files changed, 65 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_query.c
> > b/drivers/gpu/drm/i915/i915_query.c
> > index 2dfbc22857a3..e4f35da28642 100644
> > --- a/drivers/gpu/drm/i915/i915_query.c
> > +++ b/drivers/gpu/drm/i915/i915_query.c
> > @@ -9,6 +9,7 @@
> > #include "i915_drv.h"
> > #include "i915_perf.h"
> > #include "i915_query.h"
> > +#include "gt/intel_engine_user.h"
> > #include <uapi/drm/i915_drm.h>
> > static int copy_query_item(void *query_hdr, size_t query_sz,
> > @@ -28,36 +29,30 @@ static int copy_query_item(void *query_hdr, size_t
> > query_sz,
> > return 0;
> > }
> > -static int query_topology_info(struct drm_i915_private *dev_priv,
> > - struct drm_i915_query_item *query_item)
> > +static int fill_topology_info(const struct sseu_dev_info *sseu,
> > + struct drm_i915_query_item *query_item,
> > + const u8 *subslice_mask)
> > {
> > - const struct sseu_dev_info *sseu = &to_gt(dev_priv)->info.sseu;
> > struct drm_i915_query_topology_info topo;
> > u32 slice_length, subslice_length, eu_length, total_length;
> > int ret;
> > - if (query_item->flags != 0)
> > - return -EINVAL;
> > + BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask));
> > if (sseu->max_slices == 0)
> > return -ENODEV;
> > - BUILD_BUG_ON(sizeof(u8) != sizeof(sseu->slice_mask));
> > -
> > slice_length = sizeof(sseu->slice_mask);
> > subslice_length = sseu->max_slices * sseu->ss_stride;
> > eu_length = sseu->max_slices * sseu->max_subslices * sseu->eu_stride;
> > total_length = sizeof(topo) + slice_length + subslice_length +
> > eu_length;
> > - ret = copy_query_item(&topo, sizeof(topo), total_length,
> > - query_item);
> > + ret = copy_query_item(&topo, sizeof(topo), total_length, query_item);
> > +
> > if (ret != 0)
> > return ret;
> > - if (topo.flags != 0)
> > - return -EINVAL;
> > -
> > memset(&topo, 0, sizeof(topo));
> > topo.max_slices = sseu->max_slices;
> > topo.max_subslices = sseu->max_subslices;
> > @@ -69,27 +64,61 @@ static int query_topology_info(struct drm_i915_private
> > *dev_priv,
> > topo.eu_stride = sseu->eu_stride;
> > if (copy_to_user(u64_to_user_ptr(query_item->data_ptr),
> > - &topo, sizeof(topo)))
> > + &topo, sizeof(topo)))
> > return -EFAULT;
> > if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + sizeof(topo)),
> > - &sseu->slice_mask, slice_length))
> > + &sseu->slice_mask, slice_length))
> > return -EFAULT;
> > if (copy_to_user(u64_to_user_ptr(query_item->data_ptr +
> > - sizeof(topo) + slice_length),
> > - sseu->subslice_mask, subslice_length))
> > + sizeof(topo) + slice_length),
> > + subslice_mask, subslice_length))
> > return -EFAULT;
> > if (copy_to_user(u64_to_user_ptr(query_item->data_ptr +
> > - sizeof(topo) +
> > - slice_length + subslice_length),
> > - sseu->eu_mask, eu_length))
> > + sizeof(topo) +
> > + slice_length + subslice_length),
> > + sseu->eu_mask, eu_length))
> > return -EFAULT;
> > return total_length;
> > }
> > +static int query_topology_info(struct drm_i915_private *dev_priv,
> > + struct drm_i915_query_item *query_item)
> > +{
> > + const struct sseu_dev_info *sseu = &to_gt(dev_priv)->info.sseu;
> > +
> > + if (query_item->flags != 0)
> > + return -EINVAL;
> > +
> > + return fill_topology_info(sseu, query_item, sseu->subslice_mask);
> > +}
> > +
> > +static int query_geometry_subslices(struct drm_i915_private *i915,
> > + struct drm_i915_query_item *query_item)
> > +{
> > + const struct sseu_dev_info *sseu;
> > + struct intel_engine_cs *engine;
> > + u8 engine_class, engine_instance;
> > +
> > + if (GRAPHICS_VER_FULL(i915) < IP_VER(12, 50))
> > + return -ENODEV;
> > +
> > + engine_class = query_item->flags & 0xFF;
> > + engine_instance = (query_item->flags >> 8) & 0xFF;
> > +
> > + engine = intel_engine_lookup_user(i915, engine_class, engine_instance);
> > +
> > + if (!engine)
> > + return -EINVAL;
> > +
> > + sseu = &engine->gt->info.sseu;
> > +
> > + return fill_topology_info(sseu, query_item,
> > sseu->geometry_subslice_mask);
> > +}
> > +
> > static int
> > query_engine_info(struct drm_i915_private *i915,
> > struct drm_i915_query_item *query_item)
> > @@ -485,6 +514,7 @@ static int (* const i915_query_funcs[])(struct
> > drm_i915_private *dev_priv,
> > query_engine_info,
> > query_perf_config,
> > query_memregion_info,
> > + query_geometry_subslices,
> > };
> > 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 05c3642aaece..1fa6022e1558 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -2687,10 +2687,11 @@ struct drm_i915_perf_oa_config {
> > struct drm_i915_query_item {
> > /** @query_id: The id for this query */
> > __u64 query_id;
> > -#define DRM_I915_QUERY_TOPOLOGY_INFO 1
> > -#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_TOPOLOGY_INFO 1
> > +#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_GEOMETRY_SUBSLICES 5
> > /* Must be kept compact -- no holes and well documented */
> > /**
> > @@ -2714,6 +2715,9 @@ struct drm_i915_query_item {
> > * - DRM_I915_QUERY_PERF_CONFIG_LIST
> > * - DRM_I915_QUERY_PERF_CONFIG_DATA_FOR_UUID
> > * - DRM_I915_QUERY_PERF_CONFIG_FOR_UUID
> > + *
> > + * When query_id == DRM_I915_QUERY_GEOMETRY_SUBSLICES must have bits
> > 0:7 set
> > + * as a valid engine class, and bits 8:15 must have a valid engine
> > instance.
>
> Alternatively, all other uapi uses struct i915_engine_class_instance to
> address engines which uses u16:u16.
>
> How ugly it is to stuff a struct into u32 flags is the question... But you
> could at least use u16:u16 for consistency. Unless you wanted to leave some
> bits free for the future?
Originally when I wrote this I was wanting to leave space in case it was
ever needed. I'm not particularly for or against keeping the space now.
MattA
>
> Regards,
>
> Tvrtko
>
> > */
> > __u32 flags;
> > #define DRM_I915_QUERY_PERF_CONFIG_LIST 1
> > @@ -2772,16 +2776,20 @@ struct drm_i915_query {
> > };
> > /*
> > - * Data written by the kernel with query DRM_I915_QUERY_TOPOLOGY_INFO :
> > + * Data written by the kernel with query DRM_I915_QUERY_TOPOLOGY_INFO,
> > + * DRM_I915_QUERY_GEOMETRY_SUBSLICE:
> > *
> > * data: contains the 3 pieces of information :
> > *
> > - * - the slice mask with one bit per slice telling whether a slice is
> > - * available. The availability of slice X can be queried with the
> > following
> > - * formula :
> > + * - For DRM_I915_QUERY_TOPOLOGY_INFO the slice mask with one bit per slice
> > + * telling whether a slice is available. The availability of slice X can
> > be
> > + * queried with the following formula :
> > *
> > * (data[X / 8] >> (X % 8)) & 1
> > *
> > + * - For DRM_I915_QUERY_GEOMETRY_SUBSLICES Slices are equal to 1 and this
> > field
> > + * is not used.
> > + *
> > * - the subslice mask for each slice with one bit per subslice telling
> > * whether a subslice is available. Gen12 has dual-subslices, which are
> > * similar to two gen11 subslices. For gen12, this array represents
> > dual-
