Re: [PATCH 2/6] drm/i915/guc: Make guc_enable/disable_communication functions public

["Kamble, Sagar A" <sagar.a.kamble@intel.com>]

On 9/14/2017 9:01 PM, Michal Wajdeczko wrote:
> On Thu, 14 Sep 2017 11:55:04 +0200, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
>
>> From: "Kamble, Sagar A" <sagar.a.kamble@intel.com>
>>
>> This patch is moving guc_enable_communication and
>> guc_disable_communication to intel_guc.c and making it available for
>> use through intel_guc.h.
>> Intent is to reuse this function for calling from intel_uc_init_hw
>> and also as part of intel_uc_fini_hw where it will be coupled with
>> other teardown related to GuC in the upcoming patch.
>
> Hmm, but both intel_uc_init_hw() and intel_uc_fini_hw() shall stay in
> intel_uc.c and the best place for guc_[enable|disable]_communication()
> functions is also in intel_uc.c as they provide generic switch between
> MMIO/CT send mechanism, so I'm not so sure that we need this patch.
>
> Michal
Hi Michal,

Plan was to disable communication while entering rpm/system suspend, 
reset, unload
and enable back when resuming. If we don't want to update these 
mechanisms across rpm/system suspend or reset will
remove this patch then.
>
>>
>> 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_guc.c | 21 +++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_guc.h |  2 ++
>>  drivers/gpu/drm/i915/intel_uc.c  | 29 ++++-------------------------
>>  3 files changed, 27 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc.c 
>> b/drivers/gpu/drm/i915/intel_guc.c
>> index 5559e00..75bb830 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>> @@ -181,3 +181,24 @@ void intel_guc_auth_huc(struct intel_guc *guc, 
>> struct intel_huc *huc)
>>  out:
>>      i915_vma_unpin(vma);
>>  }
>> +
>> +int intel_guc_enable_communication(struct intel_guc *guc)
>> +{
>> +    struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> +
>> +    if (HAS_GUC_CT(dev_priv))
>> +        return intel_guc_enable_ct(guc);
>> +
>> +    guc->send = intel_guc_send_mmio;
>> +    return 0;
>> +}
>> +
>> +void intel_guc_disable_communication(struct intel_guc *guc)
>> +{
>> +    struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> +
>> +    if (HAS_GUC_CT(dev_priv))
>> +        intel_guc_disable_ct(guc);
>> +
>> +    guc->send = intel_guc_send_nop;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_guc.h 
>> b/drivers/gpu/drm/i915/intel_guc.h
>> index 9a282aa..8ed0e81 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>> @@ -149,6 +149,8 @@ static inline u32 guc_ggtt_offset(struct i915_vma 
>> *vma)
>>  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_auth_huc(struct intel_guc *guc, struct intel_huc *huc);
>> +int intel_guc_enable_communication(struct intel_guc *guc);
>> +void intel_guc_disable_communication(struct intel_guc *guc);
>> /* intel_guc_loader.c */
>>  int intel_guc_select_fw(struct intel_guc *guc);
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index a3fc4c8..30c004c 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -265,27 +265,6 @@ static void guc_free_load_err_log(struct 
>> intel_guc *guc)
>>          i915_gem_object_put(guc->load_err_log);
>>  }
>> -static int guc_enable_communication(struct intel_guc *guc)
>> -{
>> -    struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> -
>> -    if (HAS_GUC_CT(dev_priv))
>> -        return intel_guc_enable_ct(guc);
>> -
>> -    guc->send = intel_guc_send_mmio;
>> -    return 0;
>> -}
>> -
>> -static void guc_disable_communication(struct intel_guc *guc)
>> -{
>> -    struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> -
>> -    if (HAS_GUC_CT(dev_priv))
>> -        intel_guc_disable_ct(guc);
>> -
>> -    guc->send = intel_guc_send_nop;
>> -}
>> -
>>  int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>>  {
>>      struct intel_guc *guc = &dev_priv->guc;
>> @@ -295,7 +274,7 @@ int intel_uc_init_hw(struct drm_i915_private 
>> *dev_priv)
>>      if (!i915.enable_guc_loading)
>>          return 0;
>> -    guc_disable_communication(guc);
>> +    intel_guc_disable_communication(guc);
>>      gen9_reset_guc_interrupts(dev_priv);
>>     /* We need to notify the guc whenever we change the GGTT */
>> @@ -347,7 +326,7 @@ int intel_uc_init_hw(struct drm_i915_private 
>> *dev_priv)
>>     intel_guc_init_send_regs(guc);
>> -    ret = guc_enable_communication(guc);
>> +    ret = intel_guc_enable_communication(guc);
>>      if (ret)
>>          goto err_log_capture;
>> @@ -373,7 +352,7 @@ int intel_uc_init_hw(struct drm_i915_private 
>> *dev_priv)
>>       * marks the GPU as wedged until reset).
>>       */
>>  err_interrupts:
>> -    guc_disable_communication(guc);
>> +    intel_guc_disable_communication(guc);
>>      gen9_disable_guc_interrupts(dev_priv);
>>  err_log_capture:
>>      guc_capture_load_err_log(guc);
>> @@ -410,7 +389,7 @@ void intel_uc_fini_hw(struct drm_i915_private 
>> *dev_priv)
>>      if (i915.enable_guc_submission)
>>          i915_guc_submission_disable(dev_priv);
>> -    guc_disable_communication(&dev_priv->guc);
>> +    intel_guc_disable_communication(&dev_priv->guc);
>>     if (i915.enable_guc_submission) {
>>          gen9_disable_guc_interrupts(dev_priv);

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx