Re: [Intel-gfx] [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
On 9/14/2017 9:41 PM, Michal Wajdeczko wrote: On Thu, 14 Sep 2017 18:04:27 +0200, Kamble, Sagar Awrote: On 9/14/2017 6:07 PM, Michal Wajdeczko wrote: On Thu, 14 Sep 2017 11:55:08 +0200, Sagar Arun Kamble wrote: From: "Kamble, Sagar A" Disabling GuC interrupts involves access to GuC IRQ control registers hence ensure device is RPM awake. Cc: Michal Wajdeczko Signed-off-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/intel_guc_log.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index ba36162..d7557b5 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv) return; mutex_lock(_priv->drm.struct_mutex); +intel_runtime_pm_get(dev_priv); /* GuC logging is currently the only user of Guc2Host interrupts */ gen9_disable_guc_interrupts(dev_priv); +intel_runtime_pm_put(dev_priv); guc_log_runtime_destroy(_priv->guc); Maybe we should just destroy runtime here, and allow irq to be disabled by intel_uc_fini_hw() which will be called right after. This will also fix the upcoming case when log will not be the only user of Guc irqs. See https://patchwork.freedesktop.org/patch/170390/ Michal Destroying runtime here may create issues as enabled GuC interrupts may be causing the use of the guc_log_runtime. Yes, but this should be easy to fix. Should we move the i915_driver_unregister post i915_gem_unload? But then we will have to handle the case when gem was unloaded but driver will still be in registered state. Note that as guc log will not be the only user of the guc irqs, code that disables irq will be removed from this function anyway. Michal I see now that guc_log_runtime_destroy already happen along intel_uc_fini_hw->i915_guc_submission_fini->intel_guc_log_destroy path too. So we can remove guc_log_unregister completely with irq_disable happening as part of intel_guc_unload and guc_log_runtime_destroy too handled. mutex_unlock(_priv->drm.struct_mutex); } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
On Thu, 14 Sep 2017 18:04:27 +0200, Kamble, Sagar Awrote: On 9/14/2017 6:07 PM, Michal Wajdeczko wrote: On Thu, 14 Sep 2017 11:55:08 +0200, Sagar Arun Kamble wrote: From: "Kamble, Sagar A" Disabling GuC interrupts involves access to GuC IRQ control registers hence ensure device is RPM awake. Cc: Michal Wajdeczko Signed-off-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/intel_guc_log.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index ba36162..d7557b5 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv) return; mutex_lock(_priv->drm.struct_mutex); +intel_runtime_pm_get(dev_priv); /* GuC logging is currently the only user of Guc2Host interrupts */ gen9_disable_guc_interrupts(dev_priv); +intel_runtime_pm_put(dev_priv); guc_log_runtime_destroy(_priv->guc); Maybe we should just destroy runtime here, and allow irq to be disabled by intel_uc_fini_hw() which will be called right after. This will also fix the upcoming case when log will not be the only user of Guc irqs. See https://patchwork.freedesktop.org/patch/170390/ Michal Destroying runtime here may create issues as enabled GuC interrupts may be causing the use of the guc_log_runtime. Yes, but this should be easy to fix. Should we move the i915_driver_unregister post i915_gem_unload? But then we will have to handle the case when gem was unloaded but driver will still be in registered state. Note that as guc log will not be the only user of the guc irqs, code that disables irq will be removed from this function anyway. Michal mutex_unlock(_priv->drm.struct_mutex); } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
On 9/14/2017 6:07 PM, Michal Wajdeczko wrote: On Thu, 14 Sep 2017 11:55:08 +0200, Sagar Arun Kamblewrote: From: "Kamble, Sagar A" Disabling GuC interrupts involves access to GuC IRQ control registers hence ensure device is RPM awake. Cc: Michal Wajdeczko Signed-off-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/intel_guc_log.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index ba36162..d7557b5 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv) return; mutex_lock(_priv->drm.struct_mutex); +intel_runtime_pm_get(dev_priv); /* GuC logging is currently the only user of Guc2Host interrupts */ gen9_disable_guc_interrupts(dev_priv); +intel_runtime_pm_put(dev_priv); guc_log_runtime_destroy(_priv->guc); Maybe we should just destroy runtime here, and allow irq to be disabled by intel_uc_fini_hw() which will be called right after. This will also fix the upcoming case when log will not be the only user of Guc irqs. See https://patchwork.freedesktop.org/patch/170390/ Michal Destroying runtime here may create issues as enabled GuC interrupts may be causing the use of the guc_log_runtime. Should we move the i915_driver_unregister post i915_gem_unload? mutex_unlock(_priv->drm.struct_mutex); } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
On Thu, 14 Sep 2017 11:55:08 +0200, Sagar Arun Kamblewrote: From: "Kamble, Sagar A" Disabling GuC interrupts involves access to GuC IRQ control registers hence ensure device is RPM awake. Cc: Michal Wajdeczko Signed-off-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/intel_guc_log.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index ba36162..d7557b5 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv) return; mutex_lock(_priv->drm.struct_mutex); + intel_runtime_pm_get(dev_priv); /* GuC logging is currently the only user of Guc2Host interrupts */ gen9_disable_guc_interrupts(dev_priv); + intel_runtime_pm_put(dev_priv); guc_log_runtime_destroy(_priv->guc); Maybe we should just destroy runtime here, and allow irq to be disabled by intel_uc_fini_hw() which will be called right after. This will also fix the upcoming case when log will not be the only user of Guc irqs. See https://patchwork.freedesktop.org/patch/170390/ Michal mutex_unlock(_priv->drm.struct_mutex); } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
From: "Kamble, Sagar A"Disabling GuC interrupts involves access to GuC IRQ control registers hence ensure device is RPM awake. Cc: Michal Wajdeczko Signed-off-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/intel_guc_log.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index ba36162..d7557b5 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv) return; mutex_lock(_priv->drm.struct_mutex); + intel_runtime_pm_get(dev_priv); /* GuC logging is currently the only user of Guc2Host interrupts */ gen9_disable_guc_interrupts(dev_priv); + intel_runtime_pm_put(dev_priv); guc_log_runtime_destroy(_priv->guc); mutex_unlock(_priv->drm.struct_mutex); } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
From: "Kamble, Sagar A"Disabling GuC interrupts involves access to GuC IRQ control registers hence ensure device is RPM awake. Cc: Michal Wajdeczko Signed-off-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/intel_guc_log.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index ba36162..d7557b5 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv) return; mutex_lock(_priv->drm.struct_mutex); + intel_runtime_pm_get(dev_priv); /* GuC logging is currently the only user of Guc2Host interrupts */ gen9_disable_guc_interrupts(dev_priv); + intel_runtime_pm_put(dev_priv); guc_log_runtime_destroy(_priv->guc); mutex_unlock(_priv->drm.struct_mutex); } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
On 9/11/2017 11:04 PM, Michal Wajdeczko wrote: On Mon, Sep 11, 2017 at 11:32:24AM +0530, Sagar Arun Kamble wrote: Disabling GuC interrupts involves access to GuC IRQ control registers hence ensure device is RPM awake. Cc: Michal WajdeczkoSigned-off-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/intel_guc_log.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index ba36162..d7557b5 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv) return; mutex_lock(_priv->drm.struct_mutex); + intel_runtime_pm_get(dev_priv); /* GuC logging is currently the only user of Guc2Host interrupts */ gen9_disable_guc_interrupts(dev_priv); + intel_runtime_pm_put(dev_priv); There are other places in this file where guc interrupts are enabled/disabled. Shouldn't we do the same there ? Regards, Michal Those are already covered by the RPM locks along the paths i915_gem_init_hw, i915_guc_log_control, guc_unpause, guc_pause caller in system suspend, i915_reset. guc_log_runtime_destroy(_priv->guc); mutex_unlock(_priv->drm.struct_mutex); } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
On Mon, Sep 11, 2017 at 11:32:24AM +0530, Sagar Arun Kamble wrote: > Disabling GuC interrupts involves access to GuC IRQ control registers > hence ensure device is RPM awake. > > Cc: Michal Wajdeczko> Signed-off-by: Sagar Arun Kamble > --- > drivers/gpu/drm/i915/intel_guc_log.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_guc_log.c > b/drivers/gpu/drm/i915/intel_guc_log.c > index ba36162..d7557b5 100644 > --- a/drivers/gpu/drm/i915/intel_guc_log.c > +++ b/drivers/gpu/drm/i915/intel_guc_log.c > @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private > *dev_priv) > return; > > mutex_lock(_priv->drm.struct_mutex); > + intel_runtime_pm_get(dev_priv); > /* GuC logging is currently the only user of Guc2Host interrupts */ > gen9_disable_guc_interrupts(dev_priv); > + intel_runtime_pm_put(dev_priv); There are other places in this file where guc interrupts are enabled/disabled. Shouldn't we do the same there ? Regards, Michal > guc_log_runtime_destroy(_priv->guc); > mutex_unlock(_priv->drm.struct_mutex); > } > -- > 1.9.1 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
Disabling GuC interrupts involves access to GuC IRQ control registers hence ensure device is RPM awake. Cc: Michal WajdeczkoSigned-off-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/intel_guc_log.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index ba36162..d7557b5 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv) return; mutex_lock(_priv->drm.struct_mutex); + intel_runtime_pm_get(dev_priv); /* GuC logging is currently the only user of Guc2Host interrupts */ gen9_disable_guc_interrupts(dev_priv); + intel_runtime_pm_put(dev_priv); guc_log_runtime_destroy(_priv->guc); mutex_unlock(_priv->drm.struct_mutex); } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx