On Fri, Jan 22, 2016 at 03:06:28PM -0800, yu.dai@xxxxxxxxx wrote:
> From: Alex Dai <yu.dai@xxxxxxxxx>
>
> Previously GuC uses ring id as engine id because of same definition.
> But this is not true since this commit:
>
> commit de1add360522c876c25ef2bbbbab1c94bdb509ab
> Author: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Date: Fri Jan 15 15:12:50 2016 +0000
>
> drm/i915: Decouple execbuf uAPI from internal implementation
>
> Added GuC engine id into GuC interface to decouple it from ring id used
> by driver.
>
> Signed-off-by: Alex Dai <yu.dai@xxxxxxxxx>
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index c5db235..9a4e01e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2446,7 +2446,6 @@ static void i915_guc_client_info(struct seq_file *m,
> struct drm_i915_private *dev_priv,
> struct i915_guc_client *client)
> {
> - struct intel_engine_cs *ring;
> uint64_t tot = 0;
> uint32_t i;
>
> @@ -2461,10 +2460,9 @@ static void i915_guc_client_info(struct seq_file *m,
> seq_printf(m, "\tFailed doorbell: %u\n", client->b_fail);
> seq_printf(m, "\tLast submission result: %d\n", client->retcode);
>
> - for_each_ring(ring, dev_priv, i) {
> - seq_printf(m, "\tSubmissions: %llu %s\n",
> - client->submissions[i],
> - ring->name);
> + for (i = GUC_RENDER_ENGINE; i < GUC_MAX_ENGINES_NUM; i++) {
> + seq_printf(m, "\tSubmissions: %llu, engine %d\n",
> + client->submissions[i], i);
> tot += client->submissions[i];
> }
> seq_printf(m, "\tTotal: %llu\n", tot);
> @@ -2477,7 +2475,6 @@ static int i915_guc_info(struct seq_file *m, void *data)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_guc guc;
> struct i915_guc_client client = {};
> - struct intel_engine_cs *ring;
> enum intel_ring_id i;
> u64 total = 0;
>
> @@ -2501,9 +2498,9 @@ static int i915_guc_info(struct seq_file *m, void *data)
> seq_printf(m, "GuC last action error code: %d\n", guc.action_err);
>
> seq_printf(m, "\nGuC submissions:\n");
> - for_each_ring(ring, dev_priv, i) {
> - seq_printf(m, "\t%-24s: %10llu, last seqno 0x%08x %9d\n",
> - ring->name, guc.submissions[i],
> + for (i = GUC_RENDER_ENGINE; i < GUC_MAX_ENGINES_NUM; i++) {
> + seq_printf(m, "\tengine %d: %10llu, last seqno 0x%08x %9d\n",
> + i, guc.submissions[i],
> guc.last_seqno[i], guc.last_seqno[i]);
> total += guc.submissions[i];
For debugfs, would it not be more convenient to use the ring->name and
show the corresponding guc engine id?
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 51ae5c1..601e2c8 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -365,6 +365,14 @@ static void guc_init_proc_desc(struct intel_guc *guc,
> kunmap_atomic(base);
> }
>
> +static const enum intel_ring_id guc_engine_map[GUC_MAX_ENGINES_NUM] = {
> + [GUC_RENDER_ENGINE] = RCS,
> + [GUC_VIDEO_ENGINE] = VCS,
> + [GUC_BLITTER_ENGINE] = BCS,
> + [GUC_VIDEOENHANCE_ENGINE] = VECS,
> + [GUC_VIDEO_ENGINE2] = VCS2
> +};
> +
> /*
> * Initialise/clear the context descriptor shared with the GuC firmware.
> *
> @@ -388,9 +396,10 @@ static void guc_init_ctx_desc(struct intel_guc *guc,
> desc.priority = client->priority;
> desc.db_id = client->doorbell_id;
>
> - for (i = 0; i < I915_NUM_RINGS; i++) {
> + for (i = GUC_RENDER_ENGINE; i < GUC_MAX_ENGINES_NUM; i++) {
> struct guc_execlist_context *lrc = &desc.lrc[i];
Again, would it not be more consistent to iterate over engines
(for_each_ring) and use struct guc_execlist_context *lrc =
&desc.lrc[ring->guc_id] ?
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index b12f2aa..b69eadb 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -158,6 +158,7 @@ struct intel_engine_cs {
> #define I915_NUM_RINGS 5
> #define _VCS(n) (VCS + (n))
> unsigned int exec_id;
> + unsigned int guc_engine_id;
Is the tautology useful? It is an engine, so intel_engine_cs->guc_id
would mean the correspondance map between our engines and the guc's.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
