| Message ID | 20240912172539.418957-1-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] drm/i915/irq: Remove duplicated irq_enabled variable | expand |
-----Original Message----- From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Rodrigo Vivi Sent: Thursday, September 12, 2024 10:26 AM To: intel-gfx@lists.freedesktop.org Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Nikula, Jani <jani.nikula@intel.com> Subject: [PATCH 1/3] drm/i915/irq: Remove duplicated irq_enabled variable > > Let's kill this legacy iand almost unused rq_enabled version > in favor of the real one that is checked at > intel_irqs_enabled(). > > The commit 'ac1723c16b66 ("drm/i915: Track IRQ state > in local device state")' shows that this was a legacy > DRM level irq_enabled information that got removed. > > But the driver one already existed under a different > name. > > Cc: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> LGTM. Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> -Jonathan Cavitt > --- > drivers/gpu/drm/i915/i915_drv.h | 2 -- > drivers/gpu/drm/i915/i915_irq.c | 8 ++------ > 2 files changed, 2 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 39f6614a0a99..aa3000349116 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -343,8 +343,6 @@ struct drm_i915_private { > > struct intel_pxp *pxp; > > - bool irq_enabled; > - > struct i915_pmu pmu; > > /* The TTM device structure. */ > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 2321de48d169..9f1a6f692dd1 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1406,14 +1406,12 @@ int intel_irq_install(struct drm_i915_private *dev_priv) > */ > dev_priv->runtime_pm.irqs_enabled = true; > > - dev_priv->irq_enabled = true; > - > intel_irq_reset(dev_priv); > > ret = request_irq(irq, intel_irq_handler(dev_priv), > IRQF_SHARED, DRIVER_NAME, dev_priv); > if (ret < 0) { > - dev_priv->irq_enabled = false; > + dev_priv->runtime_pm.irqs_enabled = false; > return ret; > } > > @@ -1439,11 +1437,9 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) > * intel_display_driver_remove() calling us out of sequence. > * Would be nice if it didn't do that... > */ > - if (!dev_priv->irq_enabled) > + if (!dev_priv->runtime_pm.irqs_enabled) > return; > > - dev_priv->irq_enabled = false; > - > intel_irq_reset(dev_priv); > > free_irq(irq, dev_priv); > -- > 2.46.0 > >
Hi Rodrigo, On Thu, Sep 12, 2024 at 01:25:37PM GMT, Rodrigo Vivi wrote: > Let's kill this legacy iand almost unused rq_enabled version > in favor of the real one that is checked at > intel_irqs_enabled(). > > The commit 'ac1723c16b66 ("drm/i915: Track IRQ state > in local device state")' shows that this was a legacy > DRM level irq_enabled information that got removed. > > But the driver one already existed under a different But the driver has already one under a different name (and perhaps specify which one and which location :-)) I don't think you need to send a v2 for commit log changes. Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Andi
On Thu, 12 Sep 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > Let's kill this legacy iand almost unused rq_enabled version > in favor of the real one that is checked at > intel_irqs_enabled(). > > The commit 'ac1723c16b66 ("drm/i915: Track IRQ state > in local device state")' shows that this was a legacy > DRM level irq_enabled information that got removed. > > But the driver one already existed under a different > name. > > Cc: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 -- > drivers/gpu/drm/i915/i915_irq.c | 8 ++------ > 2 files changed, 2 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 39f6614a0a99..aa3000349116 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -343,8 +343,6 @@ struct drm_i915_private { > > struct intel_pxp *pxp; > > - bool irq_enabled; > - > struct i915_pmu pmu; > > /* The TTM device structure. */ > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 2321de48d169..9f1a6f692dd1 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1406,14 +1406,12 @@ int intel_irq_install(struct drm_i915_private *dev_priv) > */ > dev_priv->runtime_pm.irqs_enabled = true; > > - dev_priv->irq_enabled = true; > - > intel_irq_reset(dev_priv); > > ret = request_irq(irq, intel_irq_handler(dev_priv), > IRQF_SHARED, DRIVER_NAME, dev_priv); > if (ret < 0) { > - dev_priv->irq_enabled = false; > + dev_priv->runtime_pm.irqs_enabled = false; > return ret; > } > > @@ -1439,11 +1437,9 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) > * intel_display_driver_remove() calling us out of sequence. > * Would be nice if it didn't do that... > */ > - if (!dev_priv->irq_enabled) > + if (!dev_priv->runtime_pm.irqs_enabled) I think we should just stick a if (drm_WARN_ON(&dev_priv->drm, !dev_priv->runtime_pm.irqs_enabled)) here and remove the FIXME. I don't see a path like that anywhere, anymore. BR, Jani. > return; > > - dev_priv->irq_enabled = false; > - > intel_irq_reset(dev_priv); > > free_irq(irq, dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 39f6614a0a99..aa3000349116 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -343,8 +343,6 @@ struct drm_i915_private { struct intel_pxp *pxp; - bool irq_enabled; - struct i915_pmu pmu; /* The TTM device structure. */ diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 2321de48d169..9f1a6f692dd1 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1406,14 +1406,12 @@ int intel_irq_install(struct drm_i915_private *dev_priv) */ dev_priv->runtime_pm.irqs_enabled = true; - dev_priv->irq_enabled = true; - intel_irq_reset(dev_priv); ret = request_irq(irq, intel_irq_handler(dev_priv), IRQF_SHARED, DRIVER_NAME, dev_priv); if (ret < 0) { - dev_priv->irq_enabled = false; + dev_priv->runtime_pm.irqs_enabled = false; return ret; } @@ -1439,11 +1437,9 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv) * intel_display_driver_remove() calling us out of sequence. * Would be nice if it didn't do that... */ - if (!dev_priv->irq_enabled) + if (!dev_priv->runtime_pm.irqs_enabled) return; - dev_priv->irq_enabled = false; - intel_irq_reset(dev_priv); free_irq(irq, dev_priv);
Let's kill this legacy iand almost unused rq_enabled version in favor of the real one that is checked at intel_irqs_enabled(). The commit 'ac1723c16b66 ("drm/i915: Track IRQ state in local device state")' shows that this was a legacy DRM level irq_enabled information that got removed. But the driver one already existed under a different name. Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 2 -- drivers/gpu/drm/i915/i915_irq.c | 8 ++------ 2 files changed, 2 insertions(+), 8 deletions(-)