[Intel-gfx] [PATCH 1/8] drm/i915/guc: Export guc_init_send_regs and call only during intel_uc_init_hw

[Kamble, Sagar A]

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 at 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 at intel.com>
>> Cc: Michał Winiarski <michal.winiarski at intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at 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)
>>  {