| Message ID | 1505729491-26148-1-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 18 Sep 2017 12:11:24 +0200, Sagar Arun Kamble <sagar.a.kamble@intel.com> wrote: > s/guc_init_send_regs/intel_guc_init_send_regs. > Added declaration in intel_uc.h. Calling it from intel_uc_init_hw as it > is one time setup. > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Michał Winiarski <michal.winiarski@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > --- > drivers/gpu/drm/i915/intel_uc.c | 6 +++--- > drivers/gpu/drm/i915/intel_uc.h | 1 + > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_uc.c > b/drivers/gpu/drm/i915/intel_uc.c > index 0178ba4..499ecf3 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -271,7 +271,7 @@ static inline i915_reg_t guc_send_reg(struct > intel_guc *guc, u32 i) > return _MMIO(guc->send_regs.base + 4 * i); > } > -static void guc_init_send_regs(struct intel_guc *guc) > +void intel_guc_init_send_regs(struct intel_guc *guc) Hmm, there is no reason to export this function now, as it called by the function defined below. > { > struct drm_i915_private *dev_priv = guc_to_i915(guc); > enum forcewake_domains fw_domains = 0; > @@ -309,8 +309,6 @@ static int guc_enable_communication(struct intel_guc > *guc) > { > struct drm_i915_private *dev_priv = guc_to_i915(guc); > - guc_init_send_regs(guc); > - > if (HAS_GUC_CT(dev_priv)) > return intel_guc_enable_ct(guc); > @@ -386,6 +384,8 @@ int intel_uc_init_hw(struct drm_i915_private > *dev_priv) > if (ret) > goto err_log_capture; > + intel_guc_init_send_regs(guc); > + Hmm, if you want to make it 'one-time-setup' then this is still wrong place as intel_uc_init_hw() can be called several times during driver life cycle. Maybe all we need is new function intel_uc_init(dev_priv) as existing intel_uc_init_early() may be too early ;) Michal > ret = guc_enable_communication(guc); > if (ret) > goto err_log_capture; > diff --git a/drivers/gpu/drm/i915/intel_uc.h > b/drivers/gpu/drm/i915/intel_uc.h > index 7703c9a..77e6d83 100644 > --- a/drivers/gpu/drm/i915/intel_uc.h > +++ b/drivers/gpu/drm/i915/intel_uc.h > @@ -211,6 +211,7 @@ struct intel_huc { > int intel_guc_sample_forcewake(struct intel_guc *guc); > int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 > len); > int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 > len); > +void intel_guc_init_send_regs(struct intel_guc *guc); > static inline int intel_guc_send(struct intel_guc *guc, const u32 > *action, u32 len) > {
On 9/18/2017 3:49 PM, Michal Wajdeczko wrote: > On Mon, 18 Sep 2017 12:11:24 +0200, Sagar Arun Kamble > <sagar.a.kamble@intel.com> wrote: > >> s/guc_init_send_regs/intel_guc_init_send_regs. >> Added declaration in intel_uc.h. Calling it from intel_uc_init_hw as it >> is one time setup. >> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Michał Winiarski <michal.winiarski@intel.com> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> --- >> drivers/gpu/drm/i915/intel_uc.c | 6 +++--- >> drivers/gpu/drm/i915/intel_uc.h | 1 + >> 2 files changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_uc.c >> b/drivers/gpu/drm/i915/intel_uc.c >> index 0178ba4..499ecf3 100644 >> --- a/drivers/gpu/drm/i915/intel_uc.c >> +++ b/drivers/gpu/drm/i915/intel_uc.c >> @@ -271,7 +271,7 @@ static inline i915_reg_t guc_send_reg(struct >> intel_guc *guc, u32 i) >> return _MMIO(guc->send_regs.base + 4 * i); >> } >> -static void guc_init_send_regs(struct intel_guc *guc) >> +void intel_guc_init_send_regs(struct intel_guc *guc) > > Hmm, there is no reason to export this function now, as it called > by the function defined below. Yeah :) ... I was probably thinking this to be defined in intel_guc.c which is changed with this series. > >> { >> struct drm_i915_private *dev_priv = guc_to_i915(guc); >> enum forcewake_domains fw_domains = 0; >> @@ -309,8 +309,6 @@ static int guc_enable_communication(struct >> intel_guc *guc) >> { >> struct drm_i915_private *dev_priv = guc_to_i915(guc); >> - guc_init_send_regs(guc); >> - >> if (HAS_GUC_CT(dev_priv)) >> return intel_guc_enable_ct(guc); >> @@ -386,6 +384,8 @@ int intel_uc_init_hw(struct drm_i915_private >> *dev_priv) >> if (ret) >> goto err_log_capture; >> + intel_guc_init_send_regs(guc); >> + > > Hmm, if you want to make it 'one-time-setup' then this is still > wrong place as intel_uc_init_hw() can be called several times > during driver life cycle. Maybe all we need is new function > intel_uc_init(dev_priv) as existing intel_uc_init_early() may > be too early ;) > > Michal Right. Will move this in i915_driver_init_mmio after intel_uncore_init. > >> ret = guc_enable_communication(guc); >> if (ret) >> goto err_log_capture; >> diff --git a/drivers/gpu/drm/i915/intel_uc.h >> b/drivers/gpu/drm/i915/intel_uc.h >> index 7703c9a..77e6d83 100644 >> --- a/drivers/gpu/drm/i915/intel_uc.h >> +++ b/drivers/gpu/drm/i915/intel_uc.h >> @@ -211,6 +211,7 @@ struct intel_huc { >> int intel_guc_sample_forcewake(struct intel_guc *guc); >> int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 >> len); >> int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, >> u32 len); >> +void intel_guc_init_send_regs(struct intel_guc *guc); >> static inline int intel_guc_send(struct intel_guc *guc, const u32 >> *action, u32 len) >> {
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 0178ba4..499ecf3 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -271,7 +271,7 @@ static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i) return _MMIO(guc->send_regs.base + 4 * i); } -static void guc_init_send_regs(struct intel_guc *guc) +void intel_guc_init_send_regs(struct intel_guc *guc) { struct drm_i915_private *dev_priv = guc_to_i915(guc); enum forcewake_domains fw_domains = 0; @@ -309,8 +309,6 @@ static int guc_enable_communication(struct intel_guc *guc) { struct drm_i915_private *dev_priv = guc_to_i915(guc); - guc_init_send_regs(guc); - if (HAS_GUC_CT(dev_priv)) return intel_guc_enable_ct(guc); @@ -386,6 +384,8 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv) if (ret) goto err_log_capture; + intel_guc_init_send_regs(guc); + ret = guc_enable_communication(guc); if (ret) goto err_log_capture; diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index 7703c9a..77e6d83 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -211,6 +211,7 @@ struct intel_huc { int intel_guc_sample_forcewake(struct intel_guc *guc); int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len); int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len); +void intel_guc_init_send_regs(struct intel_guc *guc); static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len) {
s/guc_init_send_regs/intel_guc_init_send_regs. Added declaration in intel_uc.h. Calling it from intel_uc_init_hw as it is one time setup. Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Michał Winiarski <michal.winiarski@intel.com> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> --- drivers/gpu/drm/i915/intel_uc.c | 6 +++--- drivers/gpu/drm/i915/intel_uc.h | 1 + 2 files changed, 4 insertions(+), 3 deletions(-)