On Tue, 2015-02-03 at 11:30 +0100, Daniel Vetter wrote:
> With Ville's rework to use drm_crtc_vblank_on/off the core will take
> care of rejecting drm_vblank_get calls when the pipe is off. Also the
> core won't call the get_vblank_counter hooks in that case either. And
> since we've dropped ums support recently we can now remove these
> hacks, yay!
> 
> Noticed while trying to answer questions Laurent had about how the new
> atomic helpers work.
> 
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 52 
> -----------------------------------------
>  1 file changed, 52 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 23bfe2232b6a..80f35dcffea4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -492,31 +492,6 @@ static void i915_enable_asle_pipestat(struct drm_device 
> *dev)
>       spin_unlock_irq(&dev_priv->irq_lock);
>  }
>  
> -/**
> - * i915_pipe_enabled - check if a pipe is enabled
> - * @dev: DRM device
> - * @pipe: pipe to check
> - *
> - * Reading certain registers when the pipe is disabled can hang the chip.
> - * Use this routine to make sure the PLL is running and the pipe is active
> - * before reading such registers if unsure.
> - */
> -static int
> -i915_pipe_enabled(struct drm_device *dev, int pipe)
> -{
> -     struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -     if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> -             /* Locking is horribly broken here, but whatever. */
> -             struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> -             struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -
> -             return intel_crtc->active;
> -     } else {
> -             return I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE;
> -     }
> -}
> -
>  /*
>   * This timing diagram depicts the video signal in and
>   * around the vertical blanking period.
> @@ -583,12 +558,6 @@ static u32 i915_get_vblank_counter(struct drm_device 
> *dev, int pipe)
>       unsigned long low_frame;
>       u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
>  
> -     if (!i915_pipe_enabled(dev, pipe)) {
> -             DRM_DEBUG_DRIVER("trying to get vblank count for disabled "
> -                             "pipe %c\n", pipe_name(pipe));
> -             return 0;
> -     }

Wouldn't it make sense to still keep these as an assert (by throwing
away the !MODESET part in i915_pipe_enabled). To me it seems we could
still get here with the pipe disabled at least via drm_irq_uninstall().

> -
>       if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>               struct intel_crtc *intel_crtc =
>                       to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> @@ -648,12 +617,6 @@ static u32 gm45_get_vblank_counter(struct drm_device 
> *dev, int pipe)
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       int reg = PIPE_FRMCOUNT_GM45(pipe);
>  
> -     if (!i915_pipe_enabled(dev, pipe)) {
> -             DRM_DEBUG_DRIVER("trying to get vblank count for disabled "
> -                              "pipe %c\n", pipe_name(pipe));
> -             return 0;
> -     }
> -
>       return I915_READ(reg);
>  }
>  
> @@ -2660,9 +2623,6 @@ static int i915_enable_vblank(struct drm_device *dev, 
> int pipe)
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       unsigned long irqflags;
>  
> -     if (!i915_pipe_enabled(dev, pipe))
> -             return -EINVAL;
> -
>       spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>       if (INTEL_INFO(dev)->gen >= 4)
>               i915_enable_pipestat(dev_priv, pipe,
> @@ -2682,9 +2642,6 @@ static int ironlake_enable_vblank(struct drm_device 
> *dev, int pipe)
>       uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
>                                                    DE_PIPE_VBLANK(pipe);
>  
> -     if (!i915_pipe_enabled(dev, pipe))
> -             return -EINVAL;
> -
>       spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>       ironlake_enable_display_irq(dev_priv, bit);
>       spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> @@ -2697,9 +2654,6 @@ static int valleyview_enable_vblank(struct drm_device 
> *dev, int pipe)
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       unsigned long irqflags;
>  
> -     if (!i915_pipe_enabled(dev, pipe))
> -             return -EINVAL;
> -
>       spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>       i915_enable_pipestat(dev_priv, pipe,
>                            PIPE_START_VBLANK_INTERRUPT_STATUS);
> @@ -2713,9 +2667,6 @@ static int gen8_enable_vblank(struct drm_device *dev, 
> int pipe)
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       unsigned long irqflags;
>  
> -     if (!i915_pipe_enabled(dev, pipe))
> -             return -EINVAL;
> -
>       spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>       dev_priv->de_irq_mask[pipe] &= ~GEN8_PIPE_VBLANK;
>       I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
> @@ -2767,9 +2718,6 @@ static void gen8_disable_vblank(struct drm_device *dev, 
> int pipe)
>       struct drm_i915_private *dev_priv = dev->dev_private;
>       unsigned long irqflags;
>  
> -     if (!i915_pipe_enabled(dev, pipe))
> -             return;
> -
>       spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>       dev_priv->de_irq_mask[pipe] |= GEN8_PIPE_VBLANK;
>       I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);


Reply via email to