On Thu, 19 Dec 2013, Paulo Zanoni <przanoni@xxxxxxxxx> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>
> Depending on the HW gen and the connector type, the pipe won't start
> running right after we call intel_enable_pipe, so that
> intel_wait_for_vblank call we currently have will just sit there for
> the full 50ms timeout. So this patch adds an argument that will allow
> us to avoid the vblank wait in case we want. Currently all the callers
> still request for the vblank wait, so the behavior should still be the
> same.
>
> We also added a POSTING_READ on the register: previously
> intel_wait_for_vblank was acting as a POSTING_READ, but now if
> wait_for_vblank is false we'll stkip it, so we need an explicit
> POSTING_READ.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 869be78..6865fa2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1753,7 +1753,7 @@ static void lpt_disable_pch_transcoder(struct drm_i915_private *dev_priv)
> * returning.
> */
> static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
> - bool pch_port, bool dsi)
> + bool pch_port, bool dsi, bool wait_for_vblank)
> {
> enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
> pipe);
> @@ -1796,7 +1796,9 @@ static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
> return;
>
> I915_WRITE(reg, val | PIPECONF_ENABLE);
> - intel_wait_for_vblank(dev_priv->dev, pipe);
> + POSTING_READ(reg);
> + if (wait_for_vblank)
> + intel_wait_for_vblank(dev_priv->dev, pipe);
> }
>
> /**
> @@ -3558,7 +3560,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>
> intel_update_watermarks(crtc);
> intel_enable_pipe(dev_priv, pipe,
> - intel_crtc->config.has_pch_encoder, false);
> + intel_crtc->config.has_pch_encoder, false, true);
> intel_enable_primary_plane(dev_priv, plane, pipe);
> intel_enable_planes(crtc);
> intel_crtc_update_cursor(crtc, true);
> @@ -3704,7 +3706,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>
> intel_update_watermarks(crtc);
> intel_enable_pipe(dev_priv, pipe,
> - intel_crtc->config.has_pch_encoder, false);
> + intel_crtc->config.has_pch_encoder, false, true);
>
> if (intel_crtc->config.has_pch_encoder)
> lpt_pch_enable(crtc);
> @@ -4145,7 +4147,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
> intel_crtc_load_lut(crtc);
>
> intel_update_watermarks(crtc);
> - intel_enable_pipe(dev_priv, pipe, false, is_dsi);
> + intel_enable_pipe(dev_priv, pipe, false, is_dsi, true);
> intel_enable_primary_plane(dev_priv, plane, pipe);
> intel_enable_planes(crtc);
> intel_crtc_update_cursor(crtc, true);
> @@ -4183,7 +4185,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
> intel_crtc_load_lut(crtc);
>
> intel_update_watermarks(crtc);
> - intel_enable_pipe(dev_priv, pipe, false, false);
> + intel_enable_pipe(dev_priv, pipe, false, false, true);
A general bikeshed: adding plenty of boolean parameters makes the code
annoying to read. A bit mask with named constants makes it nicer. I'm
not saying you need to do this now; just something to keep in mind.
Cheers,
Jani.
> intel_enable_primary_plane(dev_priv, plane, pipe);
> intel_enable_planes(crtc);
> /* The fixup needs to happen before cursor is enabled */
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
