Re: [PATCH 2/2] drm/i915: Control Vblank through IER instead of IMR

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: "Runyan, Arthur J" <arthur.j.runyan@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
	Vidya Srinivas <vidya.srinivas@intel.com>
Subject: Re: [PATCH 2/2] drm/i915: Control Vblank through IER instead of IMR
Date: Thu, 14 Sep 2017 15:23:52 +0300	[thread overview]
Message-ID: <20170914122352.GW4914@intel.com> (raw)
In-Reply-To: <1505391122-9437-2-git-send-email-vidya.srinivas@intel.com>

On Thu, Sep 14, 2017 at 05:42:02PM +0530, Vidya Srinivas wrote:
> From: Uma Shankar <uma.shankar@intel.com>
> 
> Frame timestamp register is not updated if the vblank
> inteerupts are not unmasked.

Art,

Do you know if there's some other way we could coax the hardware to
update the frame timestamp? I see that on non-DSI the timestamp seems to
update even with vblank irq masked in IMR, but DSI seems to be special.

Or even better if we could get the scanline counter to tick on DSI, but
I presume that simply can't be done.

And if we just have to do the IMR unmasking trick, any idea what the cost
of keeping vblank irqs always unmasked would be? I assume it has some
very slight power cost, but not sure if it's small enough to be meaningless.

> This is needed to calculate
> scanlines for DSI encoders. This patch changes the
> vblank enable/disable logic by controlling it through
> Pipe IER register instead of IMR. Pipe IMR will be unmasked
> permanently and IER will be toggled based on need.
> 
> Credits-to: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 47668dd..a417cea 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -498,15 +498,14 @@ void bdw_update_pipe_irq(struct drm_i915_private *dev_priv,
>  	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
>  		return;
>  
> -	new_val = dev_priv->de_irq_mask[pipe];
> -	new_val &= ~interrupt_mask;
> -	new_val |= (~enabled_irq_mask & interrupt_mask);
> +	new_val = I915_READ(GEN8_DE_PIPE_IER(pipe));
> +	if (enabled_irq_mask)
> +		new_val |= enabled_irq_mask;
> +	else
> +		new_val &= ~interrupt_mask;
>  
> -	if (new_val != dev_priv->de_irq_mask[pipe]) {
> -		dev_priv->de_irq_mask[pipe] = new_val;
> -		I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
> -		POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
> -	}
> +	I915_WRITE(GEN8_DE_PIPE_IER(pipe), new_val);
> +	POSTING_READ(GEN8_DE_PIPE_IER(pipe));
>  }
>  
>  /**
> @@ -3450,8 +3449,9 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>  				  GEN8_DE_PIPE_IRQ_FAULT_ERRORS;
>  	}
>  
> -	de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
> -					   GEN8_PIPE_FIFO_UNDERRUN;
> +	de_pipe_enables = de_pipe_masked | GEN8_PIPE_FIFO_UNDERRUN;
> +
> +	de_pipe_masked |= GEN8_PIPE_VBLANK;
>  
>  	de_port_enables = de_port_masked;
>  	if (IS_GEN9_LP(dev_priv))
> -- 
> 1.9.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx