On 16/01/2018 17:40, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-01-16 16:22:52)
On 16/01/2018 16:02, Lionel Landwerlin wrote:
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
index 6468ca613d27..81367c8224ee 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -25,8 +25,100 @@
#include "i915_drv.h"
#include <uapi/drm/i915_drm.h>
+static int copy_query_data(struct drm_i915_query_item *query_item,
+ const void *item_ptr, u32 item_length,
+ const void *data_ptr, u32 data_length)
+{
+ u32 total_length = item_length + data_length;
+
+ if (query_item->length == 0)
+ return total_length;
+
+ if (query_item->length != total_length)
Pretty please
if (query_item->length < total_length)
+ return -EINVAL;
+
+ if (copy_to_user(u64_to_user_ptr(query_item->data_ptr),
+ item_ptr, item_length))
+ return -EFAULT;
+
+ if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + item_length),
+ data_ptr, data_length))
+ return -EFAULT;
+
+ return total_length;
+}
for (i = 0; i < args->num_items; i++, user_item_ptr++) {
struct drm_i915_query_item item;
+ int ret;
if (copy_from_user(&item, user_item_ptr, sizeof(item)))
return -EFAULT;
@@ -45,11 +138,24 @@ int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
return -EINVAL;
switch (item.query_id) {
+ case DRM_I915_QUERY_SLICE_INFO:
+ ret = query_slice_info(dev_priv, &item);
+ break;
+ case DRM_I915_QUERY_SUBSLICE_INFO:
+ ret = query_subslice_info(dev_priv, &item);
+ break;
+ case DRM_I915_QUERY_EU_INFO:
+ ret = query_eu_info(dev_priv, &item);
+ break;
I still can't believe Tvrtko hasn't asked this to become an function
pointer array. Tvrtko, I presume you've looked at how engine-info may
fit into this? And engine-stats? And have suggestions for making adding
new queries easier. Or are you happy as is?
I was happy with the switch for now. Was expecting someone would
complain on the whole idea anyway. Maybe I was too pessimistic. :)
As for the engine-info, I couldn't think of any reasons why this
framework wouldn't work for it. I'd put input parameters into the query
blobs.
Engine-stats I had no plans to expose here. You think it would be
useful? Main issue would be that we'd need commands to enable/disable
and not just queries. Unless some magic to auto-enable on first query
and drop after some timeout.. thinking to far ahead.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
