Re: [PATCH v7 2/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset

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

 





On 1/19/2018 6:59 AM, Jackie Li wrote:
GuC related exported functions should start with "intel_guc_"
prefix and pass intel_guc as the first parameter since its guc
related. Current guc_ggtt_offset() failed to follow this code
convention.

This patch renames the guc_ggtt_offset to intel_guc_ggtt_offset
and updates the related code to pass intel_guc pointer to
this function call. so that we have a unified coding style for
GuC code.

Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Signed-off-by: Jackie Li <yaodong.li@xxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_guc.c            | 12 +++++++-----
  drivers/gpu/drm/i915/intel_guc.h            | 10 ++++++++--
  drivers/gpu/drm/i915/intel_guc_ads.c        | 25 +++++++++++++------------
  drivers/gpu/drm/i915/intel_guc_ct.c         |  5 +++--
  drivers/gpu/drm/i915/intel_guc_fw.c         |  2 +-
  drivers/gpu/drm/i915/intel_guc_log.c        |  2 +-
  drivers/gpu/drm/i915/intel_guc_submission.c | 10 +++++-----
  drivers/gpu/drm/i915/intel_huc.c            |  6 ++++--
  8 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index b8b6d4a..e70885b 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -264,8 +264,10 @@ void intel_guc_init_params(struct intel_guc *guc)
/* If GuC submission is enabled, set up additional parameters here */
  	if (USES_GUC_SUBMISSION(dev_priv)) {
-		u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
-		u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
+		u32 ads = intel_guc_ggtt_offset(guc,
+						guc->ads_vma) >> PAGE_SHIFT;
+		u32 pgs = intel_guc_ggtt_offset(guc,
+						dev_priv->guc.stage_desc_pool);
  		u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
@@ -413,7 +415,7 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
  	data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
  	/* any value greater than GUC_POWER_D0 */
  	data[1] = GUC_POWER_D1;
-	data[2] = guc_ggtt_offset(guc->shared_data);
+	data[2] = intel_guc_ggtt_offset(guc, guc->shared_data);
return intel_guc_send(guc, data, ARRAY_SIZE(data));
  }
@@ -436,7 +438,7 @@ int intel_guc_reset_engine(struct intel_guc *guc,
  	data[3] = 0;
  	data[4] = 0;
  	data[5] = guc->execbuf_client->stage_id;
-	data[6] = guc_ggtt_offset(guc->shared_data);
+	data[6] = intel_guc_ggtt_offset(guc, guc->shared_data);
return intel_guc_send(guc, data, ARRAY_SIZE(data));
  }
@@ -458,7 +460,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
  	data[1] = GUC_POWER_D0;
-	data[2] = guc_ggtt_offset(guc->shared_data);
+	data[2] = intel_guc_ggtt_offset(guc, guc->shared_data);
return intel_guc_send(guc, data, ARRAY_SIZE(data));
  }
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 9e0a97e..b7e2a18 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -101,13 +101,19 @@ static inline void intel_guc_notify(struct intel_guc *guc)
  	guc->notify(guc);
  }
-/*
Should add "/**" instead of removing "/*"
+/* intel_guc_ggtt_offset() - Get the GGTT offset of @vma.
I feel this function is more validating the offset so "Validate and get the ggtt offset of @vma" ?
+ * @guc: intel guc.
+ * @vma: i915 graphics virtual memory area.
+ *
   * GuC does not allow any gfx GGTT address that falls into range [0, WOPCM_TOP),
   * which is reserved for Boot ROM, SRAM and WOPCM. Currently this top address is
   * 512K. In order to exclude 0-512K address space from GGTT, all gfx objects
   * used by GuC is pinned with PIN_OFFSET_BIAS along with size of WOPCM.
+ *
+ * Return: GGTT offset that meets the GuC gfx address requirement.
   */
-static inline u32 guc_ggtt_offset(struct i915_vma *vma)
+static inline u32 intel_guc_ggtt_offset(struct intel_guc *guc,
+					struct i915_vma *vma)
  {
  	u32 offset = i915_ggtt_offset(vma);
This function uses GUC_GGTT_TOP that is defined in guc_reg.h and I think we can move it to guc.h similar
to other WOPCM related moves in earlier patch.
diff --git a/drivers/gpu/drm/i915/intel_guc_ads.c b/drivers/gpu/drm/i915/intel_guc_ads.c
index ac62753..7215594 100644
--- a/drivers/gpu/drm/i915/intel_guc_ads.c
+++ b/drivers/gpu/drm/i915/intel_guc_ads.c
@@ -113,17 +113,6 @@ int intel_guc_ads_create(struct intel_guc *guc)
  		blob->reg_state.white_list[engine->guc_id].count = 0;
  	}
- /*
-	 * The GuC requires a "Golden Context" when it reinitialises
-	 * engines after a reset. Here we use the Render ring default
-	 * context, which must already exist and be pinned in the GGTT,
-	 * so its address won't change after we've told the GuC where
-	 * to find it. Note that we have to skip our header (1 page),
-	 * because our GuC shared data is there.
-	 */
-	blob->ads.golden_context_lrca =
-		guc_ggtt_offset(dev_priv->kernel_context->engine[RCS].state) +
-		skipped_offset;
This move seems unnecessary
  	/*
  	 * The GuC expects us to exclude the portion of the context image that
@@ -135,11 +124,23 @@ int intel_guc_ads_create(struct intel_guc *guc)
  		blob->ads.eng_state_size[engine->guc_id] =
  			engine->context_size - skipped_size;
- base = guc_ggtt_offset(vma);
+	base = intel_guc_ggtt_offset(guc, vma);
  	blob->ads.scheduler_policies = base + ptr_offset(blob, policies);
  	blob->ads.reg_state_buffer = base + ptr_offset(blob, reg_state_buffer);
  	blob->ads.reg_state_addr = base + ptr_offset(blob, reg_state);
+ /*
+	 * The GuC requires a "Golden Context" when it reinitialises
+	 * engines after a reset. Here we use the Render ring default
+	 * context, which must already exist and be pinned in the GGTT,
+	 * so its address won't change after we've told the GuC where
+	 * to find it. Note that we have to skip our header (1 page),
+	 * because our GuC shared data is there.
+	 */
+	vma = dev_priv->kernel_context->engine[RCS].state;
+	blob->ads.golden_context_lrca = intel_guc_ggtt_offset(guc, vma) +
+		skipped_offset;
+
  	kunmap(page);
return 0;
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 24ad557..0a0d3d5 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -156,7 +156,8 @@ static int ctch_init(struct intel_guc *guc,
  		err = PTR_ERR(blob);
  		goto err_vma;
  	}
-	DRM_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
+	DRM_DEBUG_DRIVER("CT: vma base=%#x\n",
+			 intel_guc_ggtt_offset(guc, ctch->vma));
/* store pointers to desc and cmds */
  	for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
@@ -202,7 +203,7 @@ static int ctch_open(struct intel_guc *guc,
  	}
/* vma should be already allocated and map'ed */
-	base = guc_ggtt_offset(ctch->vma);
+	base = intel_guc_ggtt_offset(guc, ctch->vma);
/* (re)initialize descriptors
  	 * cmds buffers are in the second half of the blob page
diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index 3b09329..178d339 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -165,7 +165,7 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
  	I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);
/* Set the source address for the new blob */
-	offset = guc_ggtt_offset(vma) + guc_fw->header_offset;
+	offset = intel_guc_ggtt_offset(guc, vma) + guc_fw->header_offset;
  	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
  	I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 2ffc966..4efe564 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -549,7 +549,7 @@ int intel_guc_log_create(struct intel_guc *guc)
  		(GUC_LOG_ISR_PAGES << GUC_LOG_ISR_SHIFT) |
  		(GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT);
- offset = guc_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */
+	offset = intel_guc_ggtt_offset(guc, vma) >> PAGE_SHIFT; /* in pages */
  	guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags;
return 0;
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 1f3a878..c56e4f4 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -375,8 +375,8 @@ static void guc_stage_desc_init(struct intel_guc *guc,
  		lrc->context_desc = lower_32_bits(ce->lrc_desc);
/* The state page is after PPHWSP */
-		lrc->ring_lrca =
-			guc_ggtt_offset(ce->state) + LRC_STATE_PN * PAGE_SIZE;
+		lrc->ring_lrca = intel_guc_ggtt_offset(guc, ce->state) +
+				 LRC_STATE_PN * PAGE_SIZE;
/* XXX: In direct submission, the GuC wants the HW context id
  		 * here. In proxy submission, it wants the stage id
@@ -384,7 +384,7 @@ static void guc_stage_desc_init(struct intel_guc *guc,
  		lrc->context_id = (client->stage_id << GUC_ELC_CTXID_OFFSET) |
  				(guc_engine_id << GUC_ELC_ENGINE_OFFSET);
- lrc->ring_begin = guc_ggtt_offset(ce->ring->vma);
+		lrc->ring_begin = intel_guc_ggtt_offset(guc, ce->ring->vma);
  		lrc->ring_end = lrc->ring_begin + ce->ring->size - 1;
  		lrc->ring_next_free_location = lrc->ring_begin;
  		lrc->ring_current_tail_pointer_value = 0;
@@ -400,7 +400,7 @@ static void guc_stage_desc_init(struct intel_guc *guc,
  	 * The doorbell, process descriptor, and workqueue are all parts
  	 * of the client object, which the GuC will reference via the GGTT
  	 */
-	gfx_addr = guc_ggtt_offset(client->vma);
+	gfx_addr = intel_guc_ggtt_offset(guc, client->vma);
  	desc->db_trigger_phy = sg_dma_address(client->vma->pages->sgl) +
  				client->doorbell_offset;
  	desc->db_trigger_cpu = ptr_to_u64(__get_doorbell(client));
@@ -596,7 +596,7 @@ static void inject_preempt_context(struct work_struct *work)
  	data[3] = engine->guc_id;
  	data[4] = guc->execbuf_client->priority;
  	data[5] = guc->execbuf_client->stage_id;
-	data[6] = guc_ggtt_offset(guc->shared_data);
+	data[6] = intel_guc_ggtt_offset(guc, guc->shared_data);
if (WARN_ON(intel_guc_send(guc, data, ARRAY_SIZE(data)))) {
  		execlists_clear_active(&engine->execlists,
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 8ed0518..aed9c1c 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -137,7 +137,8 @@ static int huc_ucode_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
  	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
/* Set the source address for the uCode */
-	offset = guc_ggtt_offset(vma) + huc_fw->header_offset;
+	offset = intel_guc_ggtt_offset(&dev_priv->guc, vma) +
+		 huc_fw->header_offset;
  	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
  	I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
@@ -213,7 +214,8 @@ int intel_huc_auth(struct intel_huc *huc)
  	}
ret = intel_guc_auth_huc(guc,
-				 guc_ggtt_offset(vma) + huc->fw.rsa_offset);
+				 intel_guc_ggtt_offset(guc, vma) +
+				 huc->fw.rsa_offset);
  	if (ret) {
  		DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
  		goto out;

--
Thanks,
Sagar

_______________________________________________
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