[Intel-gfx] [PATCH v4 7/9] drm/i915/guc: Remove i915_guc_log_unregister
Michal Wajdeczko
michal.wajdeczko at intel.com
Thu Sep 21 20:39:39 UTC 2017
On Thu, 21 Sep 2017 19:40:04 +0200, Sagar Arun Kamble
<sagar.a.kamble at intel.com> wrote:
>
>
> On 9/21/2017 11:01 PM, Sagar Arun Kamble wrote:
>>
>>
>> On 9/21/2017 2:28 AM, Michal Wajdeczko wrote:
>>> On Wed, 20 Sep 2017 19:38:22 +0200, Sagar Arun Kamble
>>> <sagar.a.kamble at intel.com> wrote:
>>>
>>>> Functionality needed to disable GuC interrupts and cleanup the
>>>> runtime/relay data structures is already covered in the unload path
>>>> via intel_guc_fini_hw and intel_guc_cleanup hence remove
>>>> i915_guc_log_unregister
>>>>
>>>> v2: Removed the function i915_guc_log_unregister.
>>>>
>>>> v3: Rebase as intel_guc.h is removed.
>>>>
>>>> v4: Rebase as intel_guc.h is created again. :)
>>>>
>>>> v5: Rebase as intel_guc.h is removed.
>>>>
>>>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>>> Reviewed-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>>
>>> I'm afraid I've to revoke my r-b as with removal of the
>>> log_unregister()
>>> we will loose symmetry with log_register() where
>>> relay_late_setup_files()
>>> was hidden, and we should still clean it up in i915_driver_unregister()
>>>
>>> Michal
>> Agree with you. We will need to maintain symmetry. Will revisit with
>> separate patches.
>> Better to integrate in the CT series I guess as there are multiple
>> logging related
>> interrupt enable/disable occurrences.
> Without this patch we are getting RPM WARN which I had floated patch
> initally for.
> https://patchwork.freedesktop.org/patch/176538/
> Kindly let me know if we should get that patch in till we fix the
> logging/CT interrupt interaction.
>
Can we only call relay_close() here (without any irq/runtime cleanup,
as that will be covered in unload path) - that way we will keep
symmetry and avoid RPM issue ...
Michal
> [173707.659868] RPM wakelock ref not held during HW access
> [173707.659897] ------------[ cut here ]------------
> [173707.659969] WARNING: CPU: 1 PID: 20769 at
> drivers/gpu/drm/i915/intel_drv.h:1800 fwtable_write32+0x1ad/0x1d0 [i915]
> [173707.660228] gen6_disable_pm_irq+0x47/0x50 [i915]
> [173707.660269] gen9_disable_guc_interrupts+0x33/0x60 [i915]
> [173707.660332] i915_guc_log_unregister+0x2e/0x50 [i915]
> [173707.660373] i915_driver_unload+0x4d/0x180 [i915]
> [173707.660415] i915_pci_remove+0x19/0x30 [i915]
> [173707.660423] pci_device_remove+0x39/0xc0
>
>>
>>
>>>
>>>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/i915_drv.c | 1 -
>>>> drivers/gpu/drm/i915/intel_guc_log.c | 12 ------------
>>>> drivers/gpu/drm/i915/intel_uc.h | 1 -
>>>> 3 files changed, 14 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>>>> b/drivers/gpu/drm/i915/i915_drv.c
>>>> index 6f36ced..c69a30a 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.c
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>>>> @@ -1252,7 +1252,6 @@ static void i915_driver_unregister(struct
>>>> drm_i915_private *dev_priv)
>>>> i915_perf_unregister(dev_priv);
>>>> i915_teardown_sysfs(dev_priv);
>>>> - i915_guc_log_unregister(dev_priv);
>>>> drm_dev_unregister(&dev_priv->drm);
>>>> i915_gem_shrinker_cleanup(dev_priv);
>>>> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c
>>>> b/drivers/gpu/drm/i915/intel_guc_log.c
>>>> index 16d3b87..3c45681 100644
>>>> --- a/drivers/gpu/drm/i915/intel_guc_log.c
>>>> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
>>>> @@ -648,15 +648,3 @@ void i915_guc_log_register(struct
>>>> drm_i915_private *dev_priv)
>>>> guc_log_late_setup(&dev_priv->guc);
>>>> mutex_unlock(&dev_priv->drm.struct_mutex);
>>>> }
>>>> -
>>>> -void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
>>>> -{
>>>> - if (!i915.enable_guc_submission)
>>>> - return;
>>>> -
>>>> - mutex_lock(&dev_priv->drm.struct_mutex);
>>>> - /* GuC logging is currently the only user of Guc2Host interrupts
>>>> */
>>>> - gen9_disable_guc_interrupts(dev_priv);
>>>> - guc_log_runtime_destroy(&dev_priv->guc);
>>>> - mutex_unlock(&dev_priv->drm.struct_mutex);
>>>> -}
>>>> diff --git a/drivers/gpu/drm/i915/intel_uc.h
>>>> b/drivers/gpu/drm/i915/intel_uc.h
>>>> index 8557e33..c2c104a 100644
>>>> --- a/drivers/gpu/drm/i915/intel_uc.h
>>>> +++ b/drivers/gpu/drm/i915/intel_uc.h
>>>> @@ -244,7 +244,6 @@ static inline void intel_guc_notify(struct
>>>> intel_guc *guc)
>>>> void intel_guc_log_destroy(struct intel_guc *guc);
>>>> int i915_guc_log_control(struct drm_i915_private *dev_priv, u64
>>>> control_val);
>>>> void i915_guc_log_register(struct drm_i915_private *dev_priv);
>>>> -void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
>>>> static inline u32 guc_ggtt_offset(struct i915_vma *vma)
>>>> {
>>
More information about the Intel-gfx
mailing list