Re: [PATCH 01/10] drm/i915/guc: Precompute GuC shared data offset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 05 Oct 2017 11:13:40 +0200, Michał Winiarski <michal.winiarski@xxxxxxxxx> wrote:

We're using first page of kernel context state to share data with GuC,
let's precompute the ggtt offset at GuC initialization time rather than
everytime we're using GuC actions.

Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Jeff Mcgee <jeff.mcgee@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++--
 drivers/gpu/drm/i915/intel_uc.c            | 4 ++++
 drivers/gpu/drm/i915/intel_uc.h            | 3 +++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 04f1281d81a5..2c0aeee3143d 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1234,7 +1234,7 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
 	/* any value greater than GUC_POWER_D0 */
 	data[1] = GUC_POWER_D1;
 	/* first page is shared data with GuC */

Please drop this now redundant comment

- data[2] = guc_ggtt_offset(ctx->engine[RCS].state) + LRC_GUCSHR_PN * PAGE_SIZE;
+	data[2] = guc->shared_data_offset;
	return intel_guc_send(guc, data, ARRAY_SIZE(data));
 }
@@ -1260,7 +1260,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
 	data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
 	data[1] = GUC_POWER_D0;
 	/* first page is shared data with GuC */

Please drop this now redundant comment

- data[2] = guc_ggtt_offset(ctx->engine[RCS].state) + LRC_GUCSHR_PN * PAGE_SIZE;
+	data[2] = guc->shared_data_offset;
	return intel_guc_send(guc, data, ARRAY_SIZE(data));
 }
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index e7875277ba97..f4893c85e54a 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -173,6 +173,10 @@ static void guc_free_load_err_log(struct intel_guc *guc)
 static int guc_enable_communication(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct i915_gem_context *ctx = dev_priv->kernel_context;
+
+	guc->shared_data_offset = guc_ggtt_offset(ctx->engine[RCS].state) +
+		LRC_GUCSHR_PN * PAGE_SIZE;

Hmm, I'm not sure that this is the best place for such initialization.
What about doing it directly in intel_uc_init_hw() or in dedicated inline ?

	if (HAS_GUC_CT(dev_priv))
 		return intel_guc_enable_ct(guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 4fa091e90b5f..10e8f0ed02e4 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -121,6 +121,9 @@ struct intel_guc {
 	/* To serialize the intel_guc_send actions */
 	struct mutex send_mutex;
+	/* Kernel context state ggtt offset, first page is shared with GuC */
+	u32 shared_data_offset;

We want to keep all 'send' related members together.
Maybe better place for this new member will be after 'execbuf_client' ?

+
 	/* GuC's FW specific send function */
 	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux