| Message ID | 1517875362-12755-2-git-send-email-yaodong.li@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2/6/2018 5:32 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) > > 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> > Signed-off-by: Jackie Li <yaodong.li@intel.com> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > --- > drivers/gpu/drm/i915/intel_guc.c | 12 +++++++----- > drivers/gpu/drm/i915/intel_guc.h | 14 ++++++++++++-- > 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_reg.h | 3 --- > drivers/gpu/drm/i915/intel_guc_submission.c | 10 +++++----- > drivers/gpu/drm/i915/intel_huc.c | 6 ++++-- > 9 files changed, 46 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c > index 9f45e6d..d9bc2a9 100644 > --- a/drivers/gpu/drm/i915/intel_guc.c > +++ b/drivers/gpu/drm/i915/intel_guc.c > @@ -269,8 +269,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; > @@ -418,7 +420,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 +443,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 +465,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..50be6de 100644 > --- a/drivers/gpu/drm/i915/intel_guc.h > +++ b/drivers/gpu/drm/i915/intel_guc.h > @@ -101,13 +101,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. > + * @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..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; > > /* > * 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 7b5074e..eca8725 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; /* in pages */ > 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 1f52fb8..de4f78b 100644 > --- a/drivers/gpu/drm/i915/intel_guc_reg.h > +++ b/drivers/gpu/drm/i915/intel_guc_reg.h > @@ -76,9 +76,6 @@ > /* Defines WOPCM space available to GuC firmware */ > #define GUC_WOPCM_SIZE _MMIO(0xc050) > > -/* 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 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;
Quoting Jackie Li (2018-02-06 00:02:38) > 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; > + vma = dev_priv->kernel_context->engine[RCS].state; > + blob->ads.golden_context_lrca = intel_guc_ggtt_offset(guc, vma) + > + skipped_offset; > + Please consider the visual grouping in line breaks carefully. The alignment in the first stanza couples the ggtt_offset with the addition of the skipped_offset. The lack of alignment in the second, makes the skipped_offset appear an unrelated after thought. -Chris
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c index 9f45e6d..d9bc2a9 100644 --- a/drivers/gpu/drm/i915/intel_guc.c +++ b/drivers/gpu/drm/i915/intel_guc.c @@ -269,8 +269,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; @@ -418,7 +420,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 +443,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 +465,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..50be6de 100644 --- a/drivers/gpu/drm/i915/intel_guc.h +++ b/drivers/gpu/drm/i915/intel_guc.h @@ -101,13 +101,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. + * @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..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; /* * 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 7b5074e..eca8725 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; /* in pages */ 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 1f52fb8..de4f78b 100644 --- a/drivers/gpu/drm/i915/intel_guc_reg.h +++ b/drivers/gpu/drm/i915/intel_guc_reg.h @@ -76,9 +76,6 @@ /* Defines WOPCM space available to GuC firmware */ #define GUC_WOPCM_SIZE _MMIO(0xc050) -/* 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 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;
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) 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> Signed-off-by: Jackie Li <yaodong.li@intel.com> --- drivers/gpu/drm/i915/intel_guc.c | 12 +++++++----- drivers/gpu/drm/i915/intel_guc.h | 14 ++++++++++++-- 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_reg.h | 3 --- drivers/gpu/drm/i915/intel_guc_submission.c | 10 +++++----- drivers/gpu/drm/i915/intel_huc.c | 6 ++++-- 9 files changed, 46 insertions(+), 33 deletions(-)