* [PATCH v2 1/2] drm/i915: Add perf property support for context HW id
@ 2017-07-19 5:39 Zhenyu Wang
2017-07-19 5:39 ` [PATCH v2 2/2] drm/i915/gvt: expose vGPU context hw id Zhenyu Wang
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Zhenyu Wang @ 2017-07-19 5:39 UTC (permalink / raw)
To: intel-gfx; +Cc: intel-gvt-dev, Jiao Pengyuan
In order to support profiling for special context e.g vGPU context,
we can expose vGPU context hw id and enable i915 perf property to
get target context for profiling. This adds new perf property to
assign target context with hw id.
Jiao Pengyuan has helped to fix context reference bug in original code.
v2:
- new function for context lookup with hw id
- always require priviledged op
- fix error return value
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jiao Pengyuan <pengyuan.jiao@intel.com>
Cc: Niu Bing <bing.niu@intel.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
drivers/gpu/drm/i915/i915_perf.c | 83 ++++++++++++++++++++++++++++++++--------
include/uapi/drm/i915_drm.h | 5 +++
2 files changed, 72 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 96682fd86f82..a80a3959bc3b 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -334,7 +334,9 @@ static struct i915_oa_format gen8_plus_oa_formats[I915_OA_FORMAT_MAX] = {
* struct perf_open_properties - for validated properties given to open a stream
* @sample_flags: `DRM_I915_PERF_PROP_SAMPLE_*` properties are tracked as flags
* @single_context: Whether a single or all gpu contexts should be monitored
+ * @single_context_hw: Set for special context with hw id e.g vGPU
* @ctx_handle: A gem ctx handle for use with @single_context
+ * @ctx_hw_id: A ctx hw id for use with @single_context_hw
* @metrics_set: An ID for an OA unit metric set advertised via sysfs
* @oa_format: An OA unit HW report format
* @oa_periodic: Whether to enable periodic OA unit sampling
@@ -348,7 +350,9 @@ struct perf_open_properties {
u32 sample_flags;
u64 single_context:1;
+ u64 single_context_hw:1;
u64 ctx_handle;
+ u64 ctx_hw_id;
/* OA sampling state */
int metrics_set;
@@ -2482,6 +2486,29 @@ static const struct file_operations fops = {
.unlocked_ioctl = i915_perf_ioctl,
};
+static struct i915_gem_context *
+lookup_context_hw_id(struct drm_i915_private *dev_priv, unsigned int hw_id)
+{
+ struct i915_gem_context *ctx;
+ int ret;
+
+ ret = i915_mutex_lock_interruptible(&dev_priv->drm);
+ if (ret)
+ return ERR_PTR(ret);
+
+ list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
+ if (!i915_gem_context_is_default(ctx))
+ continue;
+
+ if (ctx->hw_id == hw_id) {
+ ret = 1;
+ i915_gem_context_get(ctx);
+ break;
+ }
+ }
+ mutex_unlock(&dev_priv->drm.struct_mutex);
+ return ret ? ctx : NULL;
+}
/**
* i915_perf_open_ioctl_locked - DRM ioctl() for userspace to open a stream FD
@@ -2531,24 +2558,35 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv,
ret = -ENOENT;
goto err;
}
+
+ /*
+ * On Haswell the OA unit supports clock gating off for a specific
+ * context and in this mode there's no visibility of metrics for the
+ * rest of the system, which we consider acceptable for a
+ * non-privileged client.
+ *
+ * For Gen8+ the OA unit no longer supports clock gating off for a
+ * specific context and the kernel can't securely stop the counters
+ * from updating as system-wide / global values. Even though we can
+ * filter reports based on the included context ID we can't block
+ * clients from seeing the raw / global counter values via
+ * MI_REPORT_PERF_COUNT commands and so consider it a privileged op to
+ * enable the OA unit by default.
+ */
+ if (IS_HASWELL(dev_priv) && specific_ctx)
+ privileged_op = false;
}
- /*
- * On Haswell the OA unit supports clock gating off for a specific
- * context and in this mode there's no visibility of metrics for the
- * rest of the system, which we consider acceptable for a
- * non-privileged client.
- *
- * For Gen8+ the OA unit no longer supports clock gating off for a
- * specific context and the kernel can't securely stop the counters
- * from updating as system-wide / global values. Even though we can
- * filter reports based on the included context ID we can't block
- * clients from seeing the raw / global counter values via
- * MI_REPORT_PERF_COUNT commands and so consider it a privileged op to
- * enable the OA unit by default.
- */
- if (IS_HASWELL(dev_priv) && specific_ctx)
- privileged_op = false;
+ if (props->single_context_hw) {
+ unsigned int hw_id = props->ctx_hw_id;
+
+ specific_ctx = lookup_context_hw_id(dev_priv, hw_id);
+ if (IS_ERR_OR_NULL(specific_ctx)) {
+ DRM_DEBUG("Failed to look up HW context id.\n");
+ ret = -ENOENT;
+ goto err;
+ }
+ }
/* Similar to perf's kernel.perf_paranoid_cpu sysctl option
* we check a dev.i915.perf_stream_paranoid sysctl option
@@ -2686,6 +2724,14 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
props->single_context = 1;
props->ctx_handle = value;
break;
+ case DRM_I915_PERF_PROP_CTX_HW_ID:
+ if (value >= MAX_CONTEXT_HW_ID) {
+ DRM_DEBUG("Invalid OA HW context ID\n");
+ return -EINVAL;
+ }
+ props->single_context_hw = 1;
+ props->ctx_hw_id = value;
+ break;
case DRM_I915_PERF_PROP_SAMPLE_OA:
props->sample_flags |= SAMPLE_OA_REPORT;
break;
@@ -2757,6 +2803,11 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
uprop += 2;
}
+ if (props->single_context && props->single_context_hw) {
+ DRM_DEBUG("Assign context handler and HW id simultaneously\n");
+ return -EINVAL;
+ }
+
return 0;
}
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 7ccbd6a2bbe0..ddafa556e290 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1381,6 +1381,11 @@ enum drm_i915_perf_property_id {
*/
DRM_I915_PERF_PROP_OA_EXPONENT,
+ /**
+ * The value specifies ctx with hw_id
+ */
+ DRM_I915_PERF_PROP_CTX_HW_ID,
+
DRM_I915_PERF_PROP_MAX /* non-ABI */
};
--
2.13.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] drm/i915/gvt: expose vGPU context hw id
2017-07-19 5:39 [PATCH v2 1/2] drm/i915: Add perf property support for context HW id Zhenyu Wang
@ 2017-07-19 5:39 ` Zhenyu Wang
2017-07-19 6:10 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: Add perf property support for context HW id Patchwork
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Zhenyu Wang @ 2017-07-19 5:39 UTC (permalink / raw)
To: intel-gfx; +Cc: intel-gvt-dev, Jiao Pengyuan
This exposes vGPU context hw id in mdev sysfs which is used to
do vGPU based profiling. Retrieved vGPU context hw id can be set
through i915 perf ioctl to set profiling for target vGPU.
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jiao Pengyuan <pengyuan.jiao@intel.com>
Cc: Niu Bing <bing.niu@intel.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
drivers/gpu/drm/i915/gvt/kvmgt.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index fd0c85f9ef3c..83e88c70272a 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1170,10 +1170,27 @@ vgpu_id_show(struct device *dev, struct device_attribute *attr,
return sprintf(buf, "\n");
}
+static ssize_t
+hw_id_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct mdev_device *mdev = mdev_from_dev(dev);
+
+ if (mdev) {
+ struct intel_vgpu *vgpu = (struct intel_vgpu *)
+ mdev_get_drvdata(mdev);
+ return sprintf(buf, "%u\n",
+ vgpu->shadow_ctx->hw_id);
+ }
+ return sprintf(buf, "\n");
+}
+
static DEVICE_ATTR_RO(vgpu_id);
+static DEVICE_ATTR_RO(hw_id);
static struct attribute *intel_vgpu_attrs[] = {
&dev_attr_vgpu_id.attr,
+ &dev_attr_hw_id.attr,
NULL
};
--
2.13.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: Add perf property support for context HW id
2017-07-19 5:39 [PATCH v2 1/2] drm/i915: Add perf property support for context HW id Zhenyu Wang
2017-07-19 5:39 ` [PATCH v2 2/2] drm/i915/gvt: expose vGPU context hw id Zhenyu Wang
@ 2017-07-19 6:10 ` Patchwork
2017-07-21 10:50 ` [PATCH v2 1/2] " Lionel Landwerlin
2017-07-21 11:04 ` Chris Wilson
3 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-07-19 6:10 UTC (permalink / raw)
To: Zhenyu Wang; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v2,1/2] drm/i915: Add perf property support for context HW id
URL : https://patchwork.freedesktop.org/series/27547/
State : success
== Summary ==
Series 27547v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/27547/revisions/1/mbox/
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass -> FAIL (fi-snb-2600) fdo#100007
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass -> DMESG-WARN (fi-kbl-7560u) k.org#196399
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass -> DMESG-WARN (fi-byt-n2820) fdo#101705
fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
k.org#196399 https://bugzilla.kernel.org/show_bug.cgi?id=196399
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705
fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:436s
fi-bdw-gvtdvm total:279 pass:265 dwarn:0 dfail:0 fail:0 skip:14 time:427s
fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:354s
fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:529s
fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:511s
fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:486s
fi-byt-n2820 total:279 pass:250 dwarn:1 dfail:0 fail:0 skip:28 time:494s
fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:601s
fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:437s
fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:410s
fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:416s
fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:485s
fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:470s
fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:465s
fi-kbl-7560u total:279 pass:268 dwarn:1 dfail:0 fail:0 skip:10 time:572s
fi-kbl-r total:279 pass:260 dwarn:1 dfail:0 fail:0 skip:18 time:575s
fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:558s
fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:455s
fi-skl-6700hq total:279 pass:262 dwarn:0 dfail:0 fail:0 skip:17 time:581s
fi-skl-6700k total:279 pass:257 dwarn:4 dfail:0 fail:0 skip:18 time:464s
fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:483s
fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:435s
fi-skl-x1585l total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:472s
fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:545s
fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:404s
10de1e17faaab452782e5a1baffd1b30a639a261 drm-tip: 2017y-07m-18d-10h-08m-42s UTC integration manifest
3cb4c1c drm/i915/gvt: expose vGPU context hw id
ae89000 drm/i915: Add perf property support for context HW id
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5229/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] drm/i915: Add perf property support for context HW id
2017-07-19 5:39 [PATCH v2 1/2] drm/i915: Add perf property support for context HW id Zhenyu Wang
2017-07-19 5:39 ` [PATCH v2 2/2] drm/i915/gvt: expose vGPU context hw id Zhenyu Wang
2017-07-19 6:10 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: Add perf property support for context HW id Patchwork
@ 2017-07-21 10:50 ` Lionel Landwerlin
2017-07-21 11:04 ` Chris Wilson
3 siblings, 0 replies; 8+ messages in thread
From: Lionel Landwerlin @ 2017-07-21 10:50 UTC (permalink / raw)
To: Zhenyu Wang, intel-gfx; +Cc: intel-gvt-dev, Jiao Pengyuan
Just a small nit down there.
On 19/07/17 06:39, Zhenyu Wang wrote:
> In order to support profiling for special context e.g vGPU context,
> we can expose vGPU context hw id and enable i915 perf property to
> get target context for profiling. This adds new perf property to
> assign target context with hw id.
>
> Jiao Pengyuan has helped to fix context reference bug in original code.
>
> v2:
> - new function for context lookup with hw id
> - always require priviledged op
> - fix error return value
>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jiao Pengyuan <pengyuan.jiao@intel.com>
> Cc: Niu Bing <bing.niu@intel.com>
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_perf.c | 83 ++++++++++++++++++++++++++++++++--------
> include/uapi/drm/i915_drm.h | 5 +++
> 2 files changed, 72 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 96682fd86f82..a80a3959bc3b 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -334,7 +334,9 @@ static struct i915_oa_format gen8_plus_oa_formats[I915_OA_FORMAT_MAX] = {
> * struct perf_open_properties - for validated properties given to open a stream
> * @sample_flags: `DRM_I915_PERF_PROP_SAMPLE_*` properties are tracked as flags
> * @single_context: Whether a single or all gpu contexts should be monitored
> + * @single_context_hw: Set for special context with hw id e.g vGPU
> * @ctx_handle: A gem ctx handle for use with @single_context
> + * @ctx_hw_id: A ctx hw id for use with @single_context_hw
> * @metrics_set: An ID for an OA unit metric set advertised via sysfs
> * @oa_format: An OA unit HW report format
> * @oa_periodic: Whether to enable periodic OA unit sampling
> @@ -348,7 +350,9 @@ struct perf_open_properties {
> u32 sample_flags;
>
> u64 single_context:1;
> + u64 single_context_hw:1;
> u64 ctx_handle;
> + u64 ctx_hw_id;
>
> /* OA sampling state */
> int metrics_set;
> @@ -2482,6 +2486,29 @@ static const struct file_operations fops = {
> .unlocked_ioctl = i915_perf_ioctl,
> };
>
> +static struct i915_gem_context *
> +lookup_context_hw_id(struct drm_i915_private *dev_priv, unsigned int hw_id)
> +{
> + struct i915_gem_context *ctx;
> + int ret;
> +
> + ret = i915_mutex_lock_interruptible(&dev_priv->drm);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
> + if (!i915_gem_context_is_default(ctx))
> + continue;
> +
> + if (ctx->hw_id == hw_id) {
> + ret = 1;
> + i915_gem_context_get(ctx);
> + break;
> + }
> + }
> + mutex_unlock(&dev_priv->drm.struct_mutex);
> + return ret ? ctx : NULL;
I think you should replace NULL by ERR_PTR(-ENOENT) here, that would
simplify the call to lookup_context_hw_id().
> +}
>
> /**
> * i915_perf_open_ioctl_locked - DRM ioctl() for userspace to open a stream FD
> @@ -2531,24 +2558,35 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv,
> ret = -ENOENT;
> goto err;
> }
> +
> + /*
> + * On Haswell the OA unit supports clock gating off for a specific
> + * context and in this mode there's no visibility of metrics for the
> + * rest of the system, which we consider acceptable for a
> + * non-privileged client.
> + *
> + * For Gen8+ the OA unit no longer supports clock gating off for a
> + * specific context and the kernel can't securely stop the counters
> + * from updating as system-wide / global values. Even though we can
> + * filter reports based on the included context ID we can't block
> + * clients from seeing the raw / global counter values via
> + * MI_REPORT_PERF_COUNT commands and so consider it a privileged op to
> + * enable the OA unit by default.
> + */
> + if (IS_HASWELL(dev_priv) && specific_ctx)
> + privileged_op = false;
> }
>
> - /*
> - * On Haswell the OA unit supports clock gating off for a specific
> - * context and in this mode there's no visibility of metrics for the
> - * rest of the system, which we consider acceptable for a
> - * non-privileged client.
> - *
> - * For Gen8+ the OA unit no longer supports clock gating off for a
> - * specific context and the kernel can't securely stop the counters
> - * from updating as system-wide / global values. Even though we can
> - * filter reports based on the included context ID we can't block
> - * clients from seeing the raw / global counter values via
> - * MI_REPORT_PERF_COUNT commands and so consider it a privileged op to
> - * enable the OA unit by default.
> - */
> - if (IS_HASWELL(dev_priv) && specific_ctx)
> - privileged_op = false;
> + if (props->single_context_hw) {
> + unsigned int hw_id = props->ctx_hw_id;
> +
> + specific_ctx = lookup_context_hw_id(dev_priv, hw_id);
> + if (IS_ERR_OR_NULL(specific_ctx)) {
> + DRM_DEBUG("Failed to look up HW context id.\n");
> + ret = -ENOENT;
> + goto err;
> + }
If you change lookup_context_hw_id() as indicated above, this can be
turned into :
if (IS_ERR(specific_ctx)) {
DRM_DEBUG(...);
ret = PTR_ERR(specific_ctx);
goto err;
}
> + }
>
> /* Similar to perf's kernel.perf_paranoid_cpu sysctl option
> * we check a dev.i915.perf_stream_paranoid sysctl option
> @@ -2686,6 +2724,14 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
> props->single_context = 1;
> props->ctx_handle = value;
> break;
> + case DRM_I915_PERF_PROP_CTX_HW_ID:
> + if (value >= MAX_CONTEXT_HW_ID) {
> + DRM_DEBUG("Invalid OA HW context ID\n");
> + return -EINVAL;
> + }
> + props->single_context_hw = 1;
> + props->ctx_hw_id = value;
> + break;
> case DRM_I915_PERF_PROP_SAMPLE_OA:
> props->sample_flags |= SAMPLE_OA_REPORT;
> break;
> @@ -2757,6 +2803,11 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
> uprop += 2;
> }
>
> + if (props->single_context && props->single_context_hw) {
> + DRM_DEBUG("Assign context handler and HW id simultaneously\n");
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 7ccbd6a2bbe0..ddafa556e290 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1381,6 +1381,11 @@ enum drm_i915_perf_property_id {
> */
> DRM_I915_PERF_PROP_OA_EXPONENT,
>
> + /**
> + * The value specifies ctx with hw_id
> + */
> + DRM_I915_PERF_PROP_CTX_HW_ID,
> +
> DRM_I915_PERF_PROP_MAX /* non-ABI */
> };
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] drm/i915: Add perf property support for context HW id
2017-07-19 5:39 [PATCH v2 1/2] drm/i915: Add perf property support for context HW id Zhenyu Wang
` (2 preceding siblings ...)
2017-07-21 10:50 ` [PATCH v2 1/2] " Lionel Landwerlin
@ 2017-07-21 11:04 ` Chris Wilson
2017-07-21 13:01 ` Lionel Landwerlin
3 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2017-07-21 11:04 UTC (permalink / raw)
To: Zhenyu Wang, intel-gfx; +Cc: intel-gvt-dev, Jiao Pengyuan
Quoting Zhenyu Wang (2017-07-19 06:39:48)
> +static struct i915_gem_context *
> +lookup_context_hw_id(struct drm_i915_private *dev_priv, unsigned int hw_id)
> +{
> + struct i915_gem_context *ctx;
> + int ret;
> +
> + ret = i915_mutex_lock_interruptible(&dev_priv->drm);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
> + if (!i915_gem_context_is_default(ctx))
> + continue;
This is still a massive what? Why ban normal contexts? Why are you not
banning the kernel context?
> +
> + if (ctx->hw_id == hw_id) {
> + ret = 1;
> + i915_gem_context_get(ctx);
> + break;
Bad news, your reference counting is still broken. You actually need an
i915_gem_context_get_rcu() variant for pulling from this list.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] drm/i915: Add perf property support for context HW id
2017-07-21 11:04 ` Chris Wilson
@ 2017-07-21 13:01 ` Lionel Landwerlin
2017-07-25 4:26 ` Zhenyu Wang
0 siblings, 1 reply; 8+ messages in thread
From: Lionel Landwerlin @ 2017-07-21 13:01 UTC (permalink / raw)
To: Chris Wilson, Zhenyu Wang, intel-gfx; +Cc: intel-gvt-dev, Jiao Pengyuan
I think Chris' comments show this isn't actually tested.
Can you please write at least one test case for igt ?
We have a pretty long list of patches that need to land (still need
review on some patches).
I would recommend you base you patches on this :
https://github.com/djdeath/intel-gpu-tools/tree/wip/djdeath/oa-next
Once your tests are passing, I'll put them in that branch.
Thanks!
On 21/07/17 12:04, Chris Wilson wrote:
> Quoting Zhenyu Wang (2017-07-19 06:39:48)
>> +static struct i915_gem_context *
>> +lookup_context_hw_id(struct drm_i915_private *dev_priv, unsigned int hw_id)
>> +{
>> + struct i915_gem_context *ctx;
>> + int ret;
>> +
>> + ret = i915_mutex_lock_interruptible(&dev_priv->drm);
>> + if (ret)
>> + return ERR_PTR(ret);
>> +
>> + list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
>> + if (!i915_gem_context_is_default(ctx))
>> + continue;
> This is still a massive what? Why ban normal contexts? Why are you not
> banning the kernel context?
>
>> +
>> + if (ctx->hw_id == hw_id) {
>> + ret = 1;
>> + i915_gem_context_get(ctx);
>> + break;
> Bad news, your reference counting is still broken. You actually need an
> i915_gem_context_get_rcu() variant for pulling from this list.
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] drm/i915: Add perf property support for context HW id
2017-07-21 13:01 ` Lionel Landwerlin
@ 2017-07-25 4:26 ` Zhenyu Wang
2017-07-25 11:04 ` Lionel Landwerlin
0 siblings, 1 reply; 8+ messages in thread
From: Zhenyu Wang @ 2017-07-25 4:26 UTC (permalink / raw)
To: Lionel Landwerlin; +Cc: intel-gfx, intel-gvt-dev, Jiao Pengyuan
[-- Attachment #1.1: Type: text/plain, Size: 425 bytes --]
On 2017.07.21 14:01:01 +0100, Lionel Landwerlin wrote:
> I think Chris' comments show this isn't actually tested.
It turned out that's true...so currently Pengyuan just tried to
filter by exposed vGPU ctx_hw_id with global mode in gputop. Would
that be ok with you? If yes, then we don't need i915 perf change.
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] drm/i915: Add perf property support for context HW id
2017-07-25 4:26 ` Zhenyu Wang
@ 2017-07-25 11:04 ` Lionel Landwerlin
0 siblings, 0 replies; 8+ messages in thread
From: Lionel Landwerlin @ 2017-07-25 11:04 UTC (permalink / raw)
To: Zhenyu Wang; +Cc: intel-gfx, intel-gvt-dev, Jiao Pengyuan
On 25/07/17 05:26, Zhenyu Wang wrote:
> On 2017.07.21 14:01:01 +0100, Lionel Landwerlin wrote:
>> I think Chris' comments show this isn't actually tested.
> It turned out that's true...so currently Pengyuan just tried to
> filter by exposed vGPU ctx_hw_id with global mode in gputop. Would
> that be ok with you? If yes, then we don't need i915 perf change.
>
If that works for you, it's great.
I kind of like to have the hw_id in userspace too, it's pretty handy to
debug IGT tests :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-07-25 11:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-19 5:39 [PATCH v2 1/2] drm/i915: Add perf property support for context HW id Zhenyu Wang
2017-07-19 5:39 ` [PATCH v2 2/2] drm/i915/gvt: expose vGPU context hw id Zhenyu Wang
2017-07-19 6:10 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: Add perf property support for context HW id Patchwork
2017-07-21 10:50 ` [PATCH v2 1/2] " Lionel Landwerlin
2017-07-21 11:04 ` Chris Wilson
2017-07-21 13:01 ` Lionel Landwerlin
2017-07-25 4:26 ` Zhenyu Wang
2017-07-25 11:04 ` Lionel Landwerlin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox