All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/i915/guc: Use formalized struct definition for ads object
@ 2017-03-07 11:09 Michal Wajdeczko
  2017-03-07 12:17 ` ✓ Fi.CI.BAT: success for drm/i915/guc: Use formalized struct definition for ads object (rev2) Patchwork
  2017-03-07 12:36 ` [PATCH v2] drm/i915/guc: Use formalized struct definition for ads object Chris Wilson
  0 siblings, 2 replies; 3+ messages in thread
From: Michal Wajdeczko @ 2017-03-07 11:09 UTC (permalink / raw)
  To: intel-gfx

Manual pointer manipulation is error prone. Let compiler calculate
right offsets for us in case we need to change ads layout.

v2: don't call it object (Chris)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 50 ++++++++++++++----------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index beb38e3..c95616e 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -810,22 +810,21 @@ static void guc_addon_create(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	struct i915_vma *vma;
-	struct guc_ads *ads;
-	struct guc_policies *policies;
-	struct guc_mmio_reg_state *reg_state;
-	struct intel_engine_cs *engine;
-	enum intel_engine_id id;
 	struct page *page;
-	u32 size;
-
 	/* The ads obj includes the struct itself and buffers passed to GuC */
-	size = sizeof(struct guc_ads) + sizeof(struct guc_policies) +
-			sizeof(struct guc_mmio_reg_state) +
-			GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE;
+	struct __guc_ads_blob {
+		struct guc_ads ads;
+		struct guc_policies policies;
+		struct guc_mmio_reg_state reg_state;
+		u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE];
+	} __packed *blob;
+	u32 offset;
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
 
 	vma = guc->ads_vma;
 	if (!vma) {
-		vma = intel_guc_allocate_vma(guc, PAGE_ALIGN(size));
+		vma = intel_guc_allocate_vma(guc, PAGE_ALIGN(sizeof(*blob)));
 		if (IS_ERR(vma))
 			return;
 
@@ -833,7 +832,8 @@ static void guc_addon_create(struct intel_guc *guc)
 	}
 
 	page = i915_vma_first_page(vma);
-	ads = kmap(page);
+	blob = kmap(page);
+	offset = i915_ggtt_offset(vma);
 
 	/*
 	 * The GuC requires a "Golden Context" when it reinitialises
@@ -843,34 +843,32 @@ static void guc_addon_create(struct intel_guc *guc)
 	 * to find it.
 	 */
 	engine = dev_priv->engine[RCS];
-	ads->golden_context_lrca = engine->status_page.ggtt_offset;
+	blob->ads.golden_context_lrca = engine->status_page.ggtt_offset;
 
 	for_each_engine(engine, dev_priv, id)
-		ads->eng_state_size[engine->guc_id] = intel_lr_context_size(engine);
+		blob->ads.eng_state_size[engine->guc_id] =
+			intel_lr_context_size(engine);
 
 	/* GuC scheduling policies */
-	policies = (void *)ads + sizeof(struct guc_ads);
-	guc_policies_init(policies);
+	guc_policies_init(&blob->policies);
 
-	ads->scheduler_policies =
-		guc_ggtt_offset(vma) + sizeof(struct guc_ads);
+	blob->ads.scheduler_policies = offset +
+		offsetof(struct __guc_ads_blob, policies);
 
 	/* MMIO reg state */
-	reg_state = (void *)policies + sizeof(struct guc_policies);
-
 	for_each_engine(engine, dev_priv, id) {
-		reg_state->mmio_white_list[engine->guc_id].mmio_start =
+		blob->reg_state.mmio_white_list[engine->guc_id].mmio_start =
 			engine->mmio_base + GUC_MMIO_WHITE_LIST_START;
 
 		/* Nothing to be saved or restored for now. */
-		reg_state->mmio_white_list[engine->guc_id].count = 0;
+		blob->reg_state.mmio_white_list[engine->guc_id].count = 0;
 	}
 
-	ads->reg_state_addr = ads->scheduler_policies +
-			sizeof(struct guc_policies);
+	blob->ads.reg_state_addr = offset +
+		offsetof(struct __guc_ads_blob, reg_state);
 
-	ads->reg_state_buffer = ads->reg_state_addr +
-			sizeof(struct guc_mmio_reg_state);
+	blob->ads.reg_state_buffer = offset +
+		offsetof(struct __guc_ads_blob, reg_state_buffer);
 
 	kunmap(page);
 }
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/guc: Use formalized struct definition for ads object (rev2)
  2017-03-07 11:09 [PATCH v2] drm/i915/guc: Use formalized struct definition for ads object Michal Wajdeczko
@ 2017-03-07 12:17 ` Patchwork
  2017-03-07 12:36 ` [PATCH v2] drm/i915/guc: Use formalized struct definition for ads object Chris Wilson
  1 sibling, 0 replies; 3+ messages in thread
From: Patchwork @ 2017-03-07 12:17 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Use formalized struct definition for ads object (rev2)
URL   : https://patchwork.freedesktop.org/series/20826/
State : success

== Summary ==

Series 20826v2 drm/i915/guc: Use formalized struct definition for ads object
https://patchwork.freedesktop.org/api/1.0/series/20826/revisions/2/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test gem_exec_suspend:
        Subgroup basic-s3:
                dmesg-warn -> PASS       (fi-snb-2600)

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 461s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 614s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 548s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 620s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 507s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 500s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 440s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 435s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 437s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 509s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 478s
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time: 485s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 511s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 595s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 506s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 552s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 552s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 423s

c07cf6d486149608afb2d37a33a00775c3ac312b drm-tip: 2017y-03m-07d-11h-07m-45s UTC integration manifest
ed1bc95 drm/i915/guc: Use formalized struct definition for ads object

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4082/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] drm/i915/guc: Use formalized struct definition for ads object
  2017-03-07 11:09 [PATCH v2] drm/i915/guc: Use formalized struct definition for ads object Michal Wajdeczko
  2017-03-07 12:17 ` ✓ Fi.CI.BAT: success for drm/i915/guc: Use formalized struct definition for ads object (rev2) Patchwork
@ 2017-03-07 12:36 ` Chris Wilson
  1 sibling, 0 replies; 3+ messages in thread
From: Chris Wilson @ 2017-03-07 12:36 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

On Tue, Mar 07, 2017 at 11:09:36AM +0000, Michal Wajdeczko wrote:
> Manual pointer manipulation is error prone. Let compiler calculate
> right offsets for us in case we need to change ads layout.
> 
> v2: don't call it object (Chris)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

Looks good. I would have done mine line splitting slightly differently.
For

> -	ads->reg_state_buffer = ads->reg_state_addr +
> -			sizeof(struct guc_mmio_reg_state);
> +	blob->ads.reg_state_buffer = offset +
> +		offsetof(struct __guc_ads_blob, reg_state_buffer);

I would have used

	blob->ads.reg_state_buffer =
		offset + offsetof(struct __guc_ads_blob, reg_state_buffer);

I didn't see any mistakes in the conversion, and the packing is
certainly more obvious now.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-03-07 12:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-07 11:09 [PATCH v2] drm/i915/guc: Use formalized struct definition for ads object Michal Wajdeczko
2017-03-07 12:17 ` ✓ Fi.CI.BAT: success for drm/i915/guc: Use formalized struct definition for ads object (rev2) Patchwork
2017-03-07 12:36 ` [PATCH v2] drm/i915/guc: Use formalized struct definition for ads object Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.