-----Original Message-----
From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Rodrigo Vivi
Sent: Thursday, September 12, 2024 10:26 AM
To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>
Subject: [PATCH 3/3] drm/i915/irq: Rename suspend/resume funcitons
>
s/funcitons/functions
> Although these functions are used in runtime_pm, they are not
> exclusively used there, so remove the misleading prefix.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gt/intel_reset.c | 4 ++--
> drivers/gpu/drm/i915/i915_driver.c | 12 +++++------
> drivers/gpu/drm/i915/i915_irq.c | 30 +++++++++++++--------------
> drivers/gpu/drm/i915/i915_irq.h | 4 ++--
> 4 files changed, 24 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index 8f1ea95471ef..f42f21632306 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -1233,7 +1233,7 @@ void intel_gt_reset(struct intel_gt *gt,
> }
>
> if (INTEL_INFO(gt->i915)->gpu_reset_clobbers_display)
> - intel_runtime_pm_disable_interrupts(gt->i915);
> + intel_irq_suspend(gt->i915);
>
> if (do_reset(gt, stalled_mask)) {
> gt_err(gt, "Failed to reset chip\n");
> @@ -1241,7 +1241,7 @@ void intel_gt_reset(struct intel_gt *gt,
> }
>
> if (INTEL_INFO(gt->i915)->gpu_reset_clobbers_display)
> - intel_runtime_pm_enable_interrupts(gt->i915);
> + intel_irq_resume(gt->i915);
>
> intel_overlay_reset(gt->i915);
>
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 943e938040c0..f82aa313f854 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -950,7 +950,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>
> intel_dp_mst_suspend(i915);
>
> - intel_runtime_pm_disable_interrupts(i915);
> + intel_irq_suspend(i915);
> intel_hpd_cancel_work(i915);
>
> if (HAS_DISPLAY(i915))
> @@ -1035,7 +1035,7 @@ static int i915_drm_suspend(struct drm_device *dev)
>
> intel_dp_mst_suspend(dev_priv);
>
> - intel_runtime_pm_disable_interrupts(dev_priv);
> + intel_irq_suspend(dev_priv);
> intel_hpd_cancel_work(dev_priv);
>
> if (HAS_DISPLAY(dev_priv))
> @@ -1181,7 +1181,7 @@ static int i915_drm_resume(struct drm_device *dev)
> * Modeset enabling in intel_display_driver_init_hw() also needs working
> * interrupts.
> */
> - intel_runtime_pm_enable_interrupts(dev_priv);
> + intel_irq_resume(dev_priv);
>
> if (HAS_DISPLAY(dev_priv))
> drm_mode_config_reset(dev);
> @@ -1481,7 +1481,7 @@ static int intel_runtime_suspend(struct device *kdev)
> for_each_gt(gt, dev_priv, i)
> intel_gt_runtime_suspend(gt);
>
> - intel_runtime_pm_disable_interrupts(dev_priv);
> + intel_irq_suspend(dev_priv);
>
> for_each_gt(gt, dev_priv, i)
> intel_uncore_suspend(gt->uncore);
> @@ -1494,7 +1494,7 @@ static int intel_runtime_suspend(struct device *kdev)
> "Runtime suspend failed, disabling it (%d)\n", ret);
> intel_uncore_runtime_resume(&dev_priv->uncore);
>
> - intel_runtime_pm_enable_interrupts(dev_priv);
> + intel_irq_resume(dev_priv);
>
> for_each_gt(gt, dev_priv, i)
> intel_gt_runtime_resume(gt);
> @@ -1587,7 +1587,7 @@ static int intel_runtime_resume(struct device *kdev)
> for_each_gt(gt, dev_priv, i)
> intel_uncore_runtime_resume(gt->uncore);
>
> - intel_runtime_pm_enable_interrupts(dev_priv);
> + intel_irq_resume(dev_priv);
>
> /*
> * No point of rolling back things in case of an error, as the best
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 2c0fdb5e05a6..d42997fdee65 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1449,31 +1449,29 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
> }
>
> /**
> - * intel_runtime_pm_disable_interrupts - runtime interrupt disabling
> - * @dev_priv: i915 device instance
> + * intel_irq_suspend - Suspend interrupts
> + * @i915: i915 device instance
> *
> - * This function is used to disable interrupts at runtime, both in the runtime
> - * pm and the system suspend/resume code.
> + * This function is used to disable interrupts at runtime.
> */
> -void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
> +void intel_irq_suspend(struct drm_i915_private *i915)
> {
> - intel_irq_reset(dev_priv);
> - dev_priv->irqs_enabled = false;
> - intel_synchronize_irq(dev_priv);
> + intel_irq_reset(i915);
> + i915->irqs_enabled = false;
> + intel_synchronize_irq(i915);
> }
>
> /**
> - * intel_runtime_pm_enable_interrupts - runtime interrupt enabling
> - * @dev_priv: i915 device instance
> + * intel_irq_resume - Resume interrupts
> + * @i915: i915 device instance
> *
> - * This function is used to enable interrupts at runtime, both in the runtime
> - * pm and the system suspend/resume code.
> + * This function is used to enable interrupts at runtime.
> */
> -void intel_runtime_pm_enable_interrupts(struct drm_i915_private *dev_priv)
> +void intel_irq_resume(struct drm_i915_private *i915)
> {
> - dev_priv->irqs_enabled = true;
> - intel_irq_reset(dev_priv);
> - intel_irq_postinstall(dev_priv);
> + i915->irqs_enabled = true;
> + intel_irq_reset(i915);
> + intel_irq_postinstall(i915);
> }
>
> bool intel_irqs_enabled(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_irq.h b/drivers/gpu/drm/i915/i915_irq.h
> index e665a1b007dc..cde4cac5eca2 100644
> --- a/drivers/gpu/drm/i915/i915_irq.h
> +++ b/drivers/gpu/drm/i915/i915_irq.h
> @@ -34,8 +34,8 @@ void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv);
> void gen6_rps_reset_ei(struct drm_i915_private *dev_priv);
> u32 gen6_sanitize_rps_pm_mask(const struct drm_i915_private *i915, u32 mask);
>
> -void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv);
> -void intel_runtime_pm_enable_interrupts(struct drm_i915_private *dev_priv);
> +void intel_irq_suspend(struct drm_i915_private *i915);
> +void intel_irq_resume(struct drm_i915_private *i915);
Aside:
Though part of me would've preferred intel_irq_disable and intel_irq_enable
for these function names, I can see why we would want to use suspend and
resume instead: irrespective of naming policy, intel_irq_enable is just a bit
too close to intel_irqs_enabled in name.
LGTM.
Reviewed-by: Jonathan Cavitt <jonathan.cavitt@xxxxxxxxx>
-Jonathan Cavitt
> bool intel_irqs_enabled(struct drm_i915_private *dev_priv);
> void intel_synchronize_irq(struct drm_i915_private *i915);
> void intel_synchronize_hardirq(struct drm_i915_private *i915);
> --
> 2.46.0
>
>
