Re: [PATCH 1/2] drm/i915: Merge duplicate gen4 and vlv/chv enable vblank callbacks

Daniel Vetter
2016-10-13T14:19:53Z

On Fri, Oct 07, 2016 at 08:49:52PM +0100, Chris Wilson wrote:
> gen4/vlv/chv all use the same bits in pipestat to enable the vblank
> interrupt, so they can share the same callbacks to enable/disable.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

Irrespective of patch 2 this seems nice. git made a mess of the diff, but
I think it all checks out.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
> drivers/gpu/drm/i915/i915_irq.c | 59 +++++++++++++++++++----------------------
> 1 file changed, 28 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index bd6c8b0eeaef..1e43fe30da11 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2718,45 +2718,40 @@ void i915_handle_error(struct drm_i915_private *dev_priv,
> /* Called from drm generic code, passed 'crtc' which
> * we use as a pipe index
> */
> -static int i915_enable_vblank(struct drm_device *dev, unsigned int pipe)
> +static int i8xx_enable_vblank(struct drm_device *dev, unsigned int pipe)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> unsigned long irqflags;
>
> spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> - if (INTEL_INFO(dev)->gen >= 4)
> - i915_enable_pipestat(dev_priv, pipe,
> - PIPE_START_VBLANK_INTERRUPT_STATUS);
> - else
> - i915_enable_pipestat(dev_priv, pipe,
> - PIPE_VBLANK_INTERRUPT_STATUS);
> + i915_enable_pipestat(dev_priv, pipe, PIPE_VBLANK_INTERRUPT_STATUS);
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>
> return 0;
> }
>
> -static int ironlake_enable_vblank(struct drm_device *dev, unsigned int pipe)
> +static int i965_enable_vblank(struct drm_device *dev, unsigned int pipe)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> unsigned long irqflags;
> - uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
> - DE_PIPE_VBLANK(pipe);
>
> spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> - ilk_enable_display_irq(dev_priv, bit);
> + i915_enable_pipestat(dev_priv, pipe,
> + PIPE_START_VBLANK_INTERRUPT_STATUS);
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>
> return 0;
> }
>
> -static int valleyview_enable_vblank(struct drm_device *dev, unsigned int pipe)
> +static int ironlake_enable_vblank(struct drm_device *dev, unsigned int pipe)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> unsigned long irqflags;
> + uint32_t bit = INTEL_GEN(dev) >= 7 ?
> + DE_PIPE_VBLANK_IVB(pipe) : DE_PIPE_VBLANK(pipe);
>
> spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> - i915_enable_pipestat(dev_priv, pipe,
> - PIPE_START_VBLANK_INTERRUPT_STATUS);
> + ilk_enable_display_irq(dev_priv, bit);
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>
> return 0;
> @@ -2777,38 +2772,36 @@ static int gen8_enable_vblank(struct drm_device *dev, unsigned int pipe)
> /* Called from drm generic code, passed 'crtc' which
> * we use as a pipe index
> */
> -static void i915_disable_vblank(struct drm_device *dev, unsigned int pipe)
> +static void i8xx_disable_vblank(struct drm_device *dev, unsigned int pipe)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> unsigned long irqflags;
>
> spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> - i915_disable_pipestat(dev_priv, pipe,
> - PIPE_VBLANK_INTERRUPT_STATUS |
> - PIPE_START_VBLANK_INTERRUPT_STATUS);
> + i915_disable_pipestat(dev_priv, pipe, PIPE_VBLANK_INTERRUPT_STATUS);
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> }
>
> -static void ironlake_disable_vblank(struct drm_device *dev, unsigned int pipe)
> +static void i965_disable_vblank(struct drm_device *dev, unsigned int pipe)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> unsigned long irqflags;
> - uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
> - DE_PIPE_VBLANK(pipe);
>
> spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> - ilk_disable_display_irq(dev_priv, bit);
> + i915_disable_pipestat(dev_priv, pipe,
> + PIPE_START_VBLANK_INTERRUPT_STATUS);
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> }
>
> -static void valleyview_disable_vblank(struct drm_device *dev, unsigned int pipe)
> +static void ironlake_disable_vblank(struct drm_device *dev, unsigned int pipe)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> unsigned long irqflags;
> + uint32_t bit = INTEL_GEN(dev) >= 7 ?
> + DE_PIPE_VBLANK_IVB(pipe) : DE_PIPE_VBLANK(pipe);
>
> spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> - i915_disable_pipestat(dev_priv, pipe,
> - PIPE_START_VBLANK_INTERRUPT_STATUS);
> + ilk_disable_display_irq(dev_priv, bit);
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> }
>
> @@ -4579,16 +4572,16 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> dev->driver->irq_preinstall = cherryview_irq_preinstall;
> dev->driver->irq_postinstall = cherryview_irq_postinstall;
> dev->driver->irq_uninstall = cherryview_irq_uninstall;
> - dev->driver->enable_vblank = valleyview_enable_vblank;
> - dev->driver->disable_vblank = valleyview_disable_vblank;
> + dev->driver->enable_vblank = i965_enable_vblank;
> + dev->driver->disable_vblank = i965_disable_vblank;
> dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
> } else if (IS_VALLEYVIEW(dev_priv)) {
> dev->driver->irq_handler = valleyview_irq_handler;
> dev->driver->irq_preinstall = valleyview_irq_preinstall;
> dev->driver->irq_postinstall = valleyview_irq_postinstall;
> dev->driver->irq_uninstall = valleyview_irq_uninstall;
> - dev->driver->enable_vblank = valleyview_enable_vblank;
> - dev->driver->disable_vblank = valleyview_disable_vblank;
> + dev->driver->enable_vblank = i965_enable_vblank;
> + dev->driver->disable_vblank = i965_disable_vblank;
> dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
> } else if (INTEL_INFO(dev_priv)->gen >= 8) {
> dev->driver->irq_handler = gen8_irq_handler;
> @@ -4617,21 +4610,25 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> dev->driver->irq_postinstall = i8xx_irq_postinstall;
> dev->driver->irq_handler = i8xx_irq_handler;
> dev->driver->irq_uninstall = i8xx_irq_uninstall;
> + dev->driver->enable_vblank = i8xx_enable_vblank;
> + dev->driver->disable_vblank = i8xx_disable_vblank;
> } else if (IS_GEN3(dev_priv)) {
> dev->driver->irq_preinstall = i915_irq_preinstall;
> dev->driver->irq_postinstall = i915_irq_postinstall;
> dev->driver->irq_uninstall = i915_irq_uninstall;
> dev->driver->irq_handler = i915_irq_handler;
> + dev->driver->enable_vblank = i8xx_enable_vblank;
> + dev->driver->disable_vblank = i8xx_disable_vblank;
> } else {
> dev->driver->irq_preinstall = i965_irq_preinstall;
> dev->driver->irq_postinstall = i965_irq_postinstall;
> dev->driver->irq_uninstall = i965_irq_uninstall;
> dev->driver->irq_handler = i965_irq_handler;
> + dev->driver->enable_vblank = i965_enable_vblank;
> + dev->driver->disable_vblank = i965_disable_vblank;
> }
> if (I915_HAS_HOTPLUG(dev_priv))
> dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
> - dev->driver->enable_vblank = i915_enable_vblank;
> - dev->driver->disable_vblank = i915_disable_vblank;
> }
> }
>
> --
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation