diff mbox

[v12,1/6] drm/i915/guc: Rename guc_ggtt_offset to intel_guc_ggtt_offset

Message ID 1519949806-1138-1-git-send-email-yaodong.li@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jackie Li March 2, 2018, 12:16 a.m. UTC
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 and this is a
problem for future patches that needs to access intel_guc data to verify
the GGTT offset against the GuC WOPCM top.

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 can have a unified coding style for GuC code and also enable the future
patches to get GuC related data from intel_guc to do the offset
verification. Meanwhile, this patch also moves the GUC_GGTT_TOP from
intel_guc_regs.h to intel_guc.h since it is not GuC register related
definition.

v8:
 - Fixed coding style issues and moved GUC_GGTT_TOP to intel_guc.h (Sagar)
 - Updated commit message to explain to reason and motivation to add
   intel_guc as the first parameter of intel_guc_ggtt_offset (Chris)

v9:
 - Fixed code alignment issue due to line break (Chris)

v10:
 - Removed unnecessary comments, redundant code and avoided reuse variable
   to avoid potential issues (Joonas)

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> (v8)
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v9)
Signed-off-by: Jackie Li <yaodong.li@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> (v11)
---
 drivers/gpu/drm/i915/intel_guc.c            | 11 ++++++-----
 drivers/gpu/drm/i915/intel_guc.h            | 14 ++++++++++++--
 drivers/gpu/drm/i915/intel_guc_ads.c        |  8 ++++----
 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_reg.h        |  3 ---
 drivers/gpu/drm/i915/intel_guc_submission.c | 10 +++++-----
 drivers/gpu/drm/i915/intel_huc.c            |  6 ++++--
 9 files changed, 36 insertions(+), 25 deletions(-)

Comments

sagar.a.kamble@intel.com March 2, 2018, 8:04 a.m. UTC | #1
On 3/2/2018 5:46 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 and this is a
> problem for future patches that needs to access intel_guc data to verify
> the GGTT offset against the GuC WOPCM top.
>
> 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 can have a unified coding style for GuC code and also enable the future
> patches to get GuC related data from intel_guc to do the offset
> verification. Meanwhile, this patch also moves the GUC_GGTT_TOP from
> intel_guc_regs.h to intel_guc.h since it is not GuC register related
> definition.
>
> v8:
>   - Fixed coding style issues and moved GUC_GGTT_TOP to intel_guc.h (Sagar)
>   - Updated commit message to explain to reason and motivation to add
>     intel_guc as the first parameter of intel_guc_ggtt_offset (Chris)
>
> v9:
>   - Fixed code alignment issue due to line break (Chris)
>
> v10:
>   - Removed unnecessary comments, redundant code and avoided reuse variable
>     to avoid potential issues (Joonas)
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> (v8)
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v9)
> Signed-off-by: Jackie Li <yaodong.li@intel.com>
I think maintainers will prefer as:
either

Cc:
Cc:
S-o-b:
R-b:

or

S-o-b:
Cc:
Cc:
R-b:

Similar for other patches.
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> (v11)
> ---
>   drivers/gpu/drm/i915/intel_guc.c            | 11 ++++++-----
>   drivers/gpu/drm/i915/intel_guc.h            | 14 ++++++++++++--
>   drivers/gpu/drm/i915/intel_guc_ads.c        |  8 ++++----
>   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_reg.h        |  3 ---
>   drivers/gpu/drm/i915/intel_guc_submission.c | 10 +++++-----
>   drivers/gpu/drm/i915/intel_huc.c            |  6 ++++--
>   9 files changed, 36 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index e6512cc..a788e15 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -269,8 +269,9 @@ 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, guc->stage_desc_pool);
>   		u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
>   
>   		params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
> @@ -418,7 +419,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));
>   }
> @@ -441,7 +442,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));
>   }
> @@ -463,7 +464,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 52856a9..0c8b10a 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -100,13 +100,23 @@ static inline void intel_guc_notify(struct intel_guc *guc)
>   	guc->notify(guc);
>   }
>   
> -/*
> +/* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
> +#define GUC_GGTT_TOP	0xFEE00000
> +
> +/**
> + * intel_guc_ggtt_offset() - Get and validate the GGTT offset of @vma
> + * @guc: intel_guc structure.
> + * @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);
>   
> diff --git a/drivers/gpu/drm/i915/intel_guc_ads.c b/drivers/gpu/drm/i915/intel_guc_ads.c
> index ac62753..334cb52 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ads.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ads.c
> @@ -75,7 +75,7 @@ static void guc_policies_init(struct guc_policies *policies)
>   int intel_guc_ads_create(struct intel_guc *guc)
>   {
>   	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> -	struct i915_vma *vma;
> +	struct i915_vma *vma, *kernel_ctx_vma;
>   	struct page *page;
>   	/* The ads obj includes the struct itself and buffers passed to GuC */
>   	struct {
> @@ -121,9 +121,9 @@ int intel_guc_ads_create(struct intel_guc *guc)
>   	 * to find it. Note that we have to skip our header (1 page),
>   	 * because our GuC shared data is there.
>   	 */
> +	kernel_ctx_vma = dev_priv->kernel_context->engine[RCS].state;
>   	blob->ads.golden_context_lrca =
> -		guc_ggtt_offset(dev_priv->kernel_context->engine[RCS].state) +
> -		skipped_offset;
> +		intel_guc_ggtt_offset(guc, kernel_ctx_vma) + skipped_offset;
>   
>   	/*
>   	 * The GuC expects us to exclude the portion of the context image that
> @@ -135,7 +135,7 @@ 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);
> 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 7b5074e..33636de 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -638,7 +638,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;
>   	guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags;
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
> index 19a9247..711e9e9 100644
> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
> @@ -80,9 +80,6 @@
>   #define   GUC_WOPCM_TOP			  (0x80 << 12)	/* 512KB */
>   #define   BXT_GUC_WOPCM_RC6_RESERVED	  (0x10 << 12)	/* 64KB  */
>   
> -/* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
> -#define GUC_GGTT_TOP			0xFEE00000
> -
>   #define GEN8_GT_PM_CONFIG		_MMIO(0x138140)
>   #define GEN9LP_GT_PM_CONFIG		_MMIO(0x138140)
>   #define GEN9_GT_PM_CONFIG		_MMIO(0x13816c)
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 8a8ad2f..33af293 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -386,8 +386,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
> @@ -395,7 +395,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;
> @@ -411,7 +411,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));
> @@ -584,7 +584,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 ef9a05d..70a5282 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -138,7 +138,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);
>   
> @@ -214,7 +215,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;
Jackie Li March 5, 2018, 9:49 p.m. UTC | #2
On 03/02/2018 12:04 AM, Sagar Arun Kamble wrote:
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> (v8)
>> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v9)
>> Signed-off-by: Jackie Li <yaodong.li@intel.com>
> I think maintainers will prefer as:
> either
>
> Cc:
> Cc:
> S-o-b:
> R-b:
>
> or
>
> S-o-b:
> Cc:
> Cc:
> R-b:
Thanks Sagar! I will include these changes along with other 
comments/suggestion
from Joonas (if any:-))

Regards,
-Jackie
Joonas Lahtinen March 13, 2018, 10:21 a.m. UTC | #3
Quoting Jackie Li (2018-03-02 02:16:41)
> 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 and this is a
> problem for future patches that needs to access intel_guc data to verify
> the GGTT offset against the GuC WOPCM top.
> 
> 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 can have a unified coding style for GuC code and also enable the future
> patches to get GuC related data from intel_guc to do the offset
> verification. Meanwhile, this patch also moves the GUC_GGTT_TOP from
> intel_guc_regs.h to intel_guc.h since it is not GuC register related
> definition.
> 
> v8:
>  - Fixed coding style issues and moved GUC_GGTT_TOP to intel_guc.h (Sagar)
>  - Updated commit message to explain to reason and motivation to add
>    intel_guc as the first parameter of intel_guc_ggtt_offset (Chris)
> 
> v9:
>  - Fixed code alignment issue due to line break (Chris)
> 
> v10:
>  - Removed unnecessary comments, redundant code and avoided reuse variable
>    to avoid potential issues (Joonas)
> 
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> (v8)
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v9)
> Signed-off-by: Jackie Li <yaodong.li@intel.com>
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> (v11)

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index e6512cc..a788e15 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -269,8 +269,9 @@  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, guc->stage_desc_pool);
 		u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
 
 		params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
@@ -418,7 +419,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));
 }
@@ -441,7 +442,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));
 }
@@ -463,7 +464,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 52856a9..0c8b10a 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -100,13 +100,23 @@  static inline void intel_guc_notify(struct intel_guc *guc)
 	guc->notify(guc);
 }
 
-/*
+/* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
+#define GUC_GGTT_TOP	0xFEE00000
+
+/**
+ * intel_guc_ggtt_offset() - Get and validate the GGTT offset of @vma
+ * @guc: intel_guc structure.
+ * @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);
 
diff --git a/drivers/gpu/drm/i915/intel_guc_ads.c b/drivers/gpu/drm/i915/intel_guc_ads.c
index ac62753..334cb52 100644
--- a/drivers/gpu/drm/i915/intel_guc_ads.c
+++ b/drivers/gpu/drm/i915/intel_guc_ads.c
@@ -75,7 +75,7 @@  static void guc_policies_init(struct guc_policies *policies)
 int intel_guc_ads_create(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	struct i915_vma *vma;
+	struct i915_vma *vma, *kernel_ctx_vma;
 	struct page *page;
 	/* The ads obj includes the struct itself and buffers passed to GuC */
 	struct {
@@ -121,9 +121,9 @@  int intel_guc_ads_create(struct intel_guc *guc)
 	 * to find it. Note that we have to skip our header (1 page),
 	 * because our GuC shared data is there.
 	 */
+	kernel_ctx_vma = dev_priv->kernel_context->engine[RCS].state;
 	blob->ads.golden_context_lrca =
-		guc_ggtt_offset(dev_priv->kernel_context->engine[RCS].state) +
-		skipped_offset;
+		intel_guc_ggtt_offset(guc, kernel_ctx_vma) + skipped_offset;
 
 	/*
 	 * The GuC expects us to exclude the portion of the context image that
@@ -135,7 +135,7 @@  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);
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 7b5074e..33636de 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -638,7 +638,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;
 	guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags;
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h b/drivers/gpu/drm/i915/intel_guc_reg.h
index 19a9247..711e9e9 100644
--- a/drivers/gpu/drm/i915/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/intel_guc_reg.h
@@ -80,9 +80,6 @@ 
 #define   GUC_WOPCM_TOP			  (0x80 << 12)	/* 512KB */
 #define   BXT_GUC_WOPCM_RC6_RESERVED	  (0x10 << 12)	/* 64KB  */
 
-/* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
-#define GUC_GGTT_TOP			0xFEE00000
-
 #define GEN8_GT_PM_CONFIG		_MMIO(0x138140)
 #define GEN9LP_GT_PM_CONFIG		_MMIO(0x138140)
 #define GEN9_GT_PM_CONFIG		_MMIO(0x13816c)
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 8a8ad2f..33af293 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -386,8 +386,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
@@ -395,7 +395,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;
@@ -411,7 +411,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));
@@ -584,7 +584,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 ef9a05d..70a5282 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -138,7 +138,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);
 
@@ -214,7 +215,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;