Re: [PATCH v2 6/7] drm/i915: vlv: fix mapping of pipestat enable to status bits

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Feb 05, 2014 at 08:55:07PM +0200, Imre Deak wrote:
> At least on VLV we can't get at the pipestat status bits by simply right
> shifting the corresponding enable bits. The mapping between enable and
> status bits for the sprite0,1 flip done and the PSR events don't follow
> this rule, so we need to map them separately.
> 
> The PSR enable for pipe A is DPFLIPSTAT[22], but I haven't added support
> for this, since there is no user of it atm. Until support is added WARN
> if someone tries to enable PSR interrupts, or tries to enable the same
> (1 << 6) bit on pipe B, which MBZ.
> 
> v2:
> - inline the status->enable mask mapping (Ville)
> - don't check for invalid PSR bit on platforms other than VLV (Ville)
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 34 ++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/i915_reg.h |  3 +++
>  2 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 37fe12d..6166dd9 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -515,13 +515,39 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>  	POSTING_READ(reg);
>  }
>  
> +static u32 vlv_get_pipestat_enable_mask(struct drm_device *dev, u32 status_mask)
> +{
> +	u32 enable_mask = status_mask << 16;
> +
> +	/*
> +	 * On pipe A we don't support the PSR interrupt yet, on pipe B the
> +	 * same bit MBZ.
> +	 */
> +	if (WARN_ON_ONCE(status_mask & PIPE_A_PSR_STATUS_VLV))
> +		return 0;
> +
> +	enable_mask &= ~(PIPE_FIFO_UNDERRUN_STATUS |
> +			 SPRITE0_FLIP_DONE_INT_STATUS_VLV |
> +			 SPRITE1_FLIP_DONE_INT_STATUS_VLV);

Those two should have been ENABLE bits.

The rest looks fine. So fix that, and you can add (for the entire series):
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	if (status_mask & SPRITE0_FLIP_DONE_INT_STATUS_VLV)
> +		enable_mask |= SPRITE0_FLIP_DONE_INT_EN_VLV;
> +	if (status_mask & SPRITE1_FLIP_DONE_INT_STATUS_VLV)
> +		enable_mask |= SPRITE1_FLIP_DONE_INT_EN_VLV;
> +
> +	return enable_mask;
> +}
> +
>  void
>  i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>  		     u32 status_mask)
>  {
>  	u32 enable_mask;
>  
> -	enable_mask = status_mask << 16;
> +	if (IS_VALLEYVIEW(dev_priv->dev))
> +		enable_mask = vlv_get_pipestat_enable_mask(dev_priv->dev,
> +							   status_mask);
> +	else
> +		enable_mask = status_mask << 16;
>  	__i915_enable_pipestat(dev_priv, pipe, enable_mask, status_mask);
>  }
>  
> @@ -531,7 +557,11 @@ i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
>  {
>  	u32 enable_mask;
>  
> -	enable_mask = status_mask << 16;
> +	if (IS_VALLEYVIEW(dev_priv->dev))
> +		enable_mask = vlv_get_pipestat_enable_mask(dev_priv->dev,
> +							   status_mask);
> +	else
> +		enable_mask = status_mask << 16;
>  	__i915_disable_pipestat(dev_priv, pipe, enable_mask, status_mask);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0e69c5c..c5e301e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3240,6 +3240,7 @@
>  #define   PIPE_LEGACY_BLC_EVENT_ENABLE		(1UL<<22)
>  #define   PIPE_ODD_FIELD_INTERRUPT_ENABLE	(1UL<<21)
>  #define   PIPE_EVEN_FIELD_INTERRUPT_ENABLE	(1UL<<20)
> +#define   PIPE_B_PSR_INTERRUPT_ENABLE_VLV	(1UL<<19)
>  #define   PIPE_HOTPLUG_TV_INTERRUPT_ENABLE	(1UL<<18) /* pre-965 */
>  #define   PIPE_START_VBLANK_INTERRUPT_ENABLE	(1UL<<18) /* 965 or later */
>  #define   PIPE_VBLANK_INTERRUPT_ENABLE		(1UL<<17)
> @@ -3256,8 +3257,10 @@
>  #define   PIPE_DISPLAY_LINE_COMPARE_STATUS	(1UL<<8)
>  #define   PIPE_DPST_EVENT_STATUS		(1UL<<7)
>  #define   PIPE_LEGACY_BLC_EVENT_STATUS		(1UL<<6)
> +#define   PIPE_A_PSR_STATUS_VLV			(1UL<<6)
>  #define   PIPE_ODD_FIELD_INTERRUPT_STATUS	(1UL<<5)
>  #define   PIPE_EVEN_FIELD_INTERRUPT_STATUS	(1UL<<4)
> +#define   PIPE_B_PSR_STATUS_VLV			(1UL<<3)
>  #define   PIPE_HOTPLUG_TV_INTERRUPT_STATUS	(1UL<<2) /* pre-965 */
>  #define   PIPE_START_VBLANK_INTERRUPT_STATUS	(1UL<<2) /* 965 or later */
>  #define   PIPE_VBLANK_INTERRUPT_STATUS		(1UL<<1)
> -- 
> 1.8.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC