On 17/05/2019 13:10, Andi Shyti wrote:
On Fri, May 17, 2019 at 12:25:25PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Instead of hardcoding the VCS balancing engines, discover, both with the
new engines query, or with the legacy get_param in the fallback case, so
class based addressing always works.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
benchmarks/gem_wsim.c | 180 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 173 insertions(+), 7 deletions(-)
diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
index d43e7c767801..539de243f6e8 100644
--- a/benchmarks/gem_wsim.c
+++ b/benchmarks/gem_wsim.c
@@ -365,34 +365,198 @@ static int str_to_engine(const char *str)
return -1;
}
+static bool __engines_queried;
+static unsigned int __num_engines;
+static struct i915_engine_class_instance *__engines;
+
+static int
+__i915_query(int i915, struct drm_i915_query *q)
+{
+ if (igt_ioctl(i915, DRM_IOCTL_I915_QUERY, q))
+ return -errno;
+ return 0;
+}
+
+static int
+__i915_query_items(int i915, struct drm_i915_query_item *items, uint32_t n_items)
+{
+ struct drm_i915_query q = {
+ .num_items = n_items,
+ .items_ptr = to_user_pointer(items),
+ };
+ return __i915_query(i915, &q);
+}
+
+static void
+i915_query_items(int i915, struct drm_i915_query_item *items, uint32_t n_items)
+{
+ igt_assert_eq(__i915_query_items(i915, items, n_items), 0);
+}
+
+static bool has_query(int i915)
+{
+ struct drm_i915_query query = {};
+
+ return __i915_query(i915, &query) == 0;
+}
+
+static bool has_engine_query(int i915)
+{
+ struct drm_i915_query_item item = {
+ .query_id = DRM_I915_QUERY_ENGINE_INFO,
+ };
+
+ return __i915_query_items(i915, &item, 1) == 0 && item.length > 0;
+}
+
+static void query_engines(void)
+{
+ struct i915_engine_class_instance *engines;
+ unsigned int num;
+
+ if (__engines_queried)
+ return;
+
+ __engines_queried = true;
+
+ if (!has_query(fd) || !has_engine_query(fd)) {
One question, still. What is the real use of this check and
'has_query' that is used only here.
I mean... here you want to check whether the "ioctl is not
implemented" or "ioctl is not implemented and length is 0".
Wouldn't in this case just '!has_engine_query()' be enough? or
have I missed any case?
You haven't missed anything. I have been pointlessly verbose and a bit
lazy by copy-pasting a lot.
has_engine_query is a superset of has_query for the purpose of ioctl
detection.
+ unsigned int num_bsd = gem_has_bsd(fd) + gem_has_bsd2(fd);
+ unsigned int i = 0;
+
+ igt_assert(num);
+
+ num = 1 + num_bsd;
did you mean the above two lines swapped?
No, I want to avoid running on platforms with no vcs engines since no
one ever tested gem_wsim there.
+
+ if (gem_has_blt(fd))
+ num++;
+
+ if (gem_has_vebox(fd))
+ num++;
+
+ engines = calloc(num,
+ sizeof(struct i915_engine_class_instance));
+ igt_assert(engines);
+
+ engines[i].engine_class = I915_ENGINE_CLASS_RENDER;
+ engines[i].engine_instance = 0;
+ i++;
+
+ if (gem_has_blt(fd)) {
+ engines[i].engine_class = I915_ENGINE_CLASS_COPY;
+ engines[i].engine_instance = 0;
+ i++;
+ }
+
+ if (gem_has_bsd(fd)) {
+ engines[i].engine_class = I915_ENGINE_CLASS_VIDEO;
+ engines[i].engine_instance = 0;
+ i++;
+ }
+
+ if (gem_has_bsd2(fd)) {
+ engines[i].engine_class = I915_ENGINE_CLASS_VIDEO;
+ engines[i].engine_instance = 1;
+ i++;
+ }
+
+ if (gem_has_vebox(fd)) {
+ engines[i].engine_class =
+ I915_ENGINE_CLASS_VIDEO_ENHANCE;
+ engines[i].engine_instance = 0;
+ i++;
+ }
mmhhh... isn't this the intel_execution_engine2[]? Yet another
way for having engine list... in the long run, updating here (as
well) won't be easy to remember.
Not here, gem_wsim uses some of the IGT libraries, but should keep it at
minimum. So I think we don't want to pull in the engine array etc.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx