Re: [PATCH v2 9/9] drm/i915: Fix gen2 and hsw+ scanline counter

akash goel
2014-05-16T05:33:33Z

[-- Attachment #1.1: Type: text/plain, Size: 6625 bytes --]

Sorry not aware of this specific difference in the starting value of
scanline counter for HSW+ (& gen 2), but implementation wise, patch looks
fine.

Reviewed-by: "Akash Goel <akash.goels@gmail.com>"

On Thu, May 15, 2014 at 10:53 PM, <ville.syrjala@linux.intel.com> wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On gen2 the scanline counter behaves a bit differently from the
> later generations. Instead of adding one to the raw scanline
> counter value, we must subtract one.
>
> On HSW/BDW the scanline counter requires a +2 adjustment on HDMI
> outputs. DP outputs on the on the other require the typical +1
> adjustment.
>
> As the fixup we must apply to the hardware scanline counter
> depends on several factors, compute the desired offset at modeset
> time and tuck it away for when it's needed.
>
> v2: Clarify HSW+ situation
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 14 ++++-------
> drivers/gpu/drm/i915/intel_display.c | 45
> +++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> 3 files changed, 51 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index bb9b061..80003b5 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -818,9 +818,9 @@ static int __intel_get_crtc_scanline(struct intel_crtc
> *crtc)
> struct drm_i915_private *dev_priv = dev->dev_private;
> const struct drm_display_mode *mode = &crtc->config.adjusted_mode;
> enum pipe pipe = crtc->pipe;
> - int vtotal = mode->crtc_vtotal;
> - int position;
> + int position, vtotal;
>
> + vtotal = mode->crtc_vtotal;
> if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> vtotal /= 2;
>
> @@ -830,14 +830,10 @@ static int __intel_get_crtc_scanline(struct
> intel_crtc *crtc)
> position = __raw_i915_read32(dev_priv, PIPEDSL(pipe)) &
> DSL_LINEMASK_GEN3;
>
> /*
> - * Scanline counter increments at leading edge of hsync, and
> - * it starts counting from vtotal-1 on the first active line.
> - * That means the scanline counter value is always one less
> - * than what we would expect. Ie. just after start of vblank,
> - * which also occurs at start of hsync (on the last active line),
> - * the scanline counter will read vblank_start-1.
> + * See update_scanline_offset() for the details on the
> + * scanline_offset adjustment.
> */
> - return (position + 1) % vtotal;
> + return (position + crtc->scanline_offset) % vtotal;
> }
>
> static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 0f8f9bc..f7222d7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10164,6 +10164,44 @@ void ironlake_check_encoder_dotclock(const struct
> intel_crtc_config *pipe_config
> pipe_config->adjusted_mode.crtc_clock, dotclock);
> }
>
> +static void update_scanline_offset(struct intel_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->base.dev;
> +
> + /*
> + * The scanline counter increments at the leading edge of hsync.
> + *
> + * On most platforms it starts counting from vtotal-1 on the
> + * first active line. That means the scanline counter value is
> + * always one less than what we would expect. Ie. just after
> + * start of vblank, which also occurs at start of hsync (on the
> + * last active line), the scanline counter will read
> vblank_start-1.
> + *
> + * On gen2 the scanline counter starts counting from 1 instead
> + * of vtotal-1, so we have to subtract one (or rather add vtotal-1
> + * to keep the value positive), instead of adding one.
> + *
> + * On HSW+ the behaviour of the scanline counter depends on the
> output
> + * type. For DP ports it behaves like most other platforms, but on
> HDMI
> + * there's an extra 1 line difference. So we need to add two
> instead of
> + * one to the value.
> + */
> + if (IS_GEN2(dev)) {
> + const struct drm_display_mode *mode =
> &crtc->config.adjusted_mode;
> + int vtotal;
> +
> + vtotal = mode->crtc_vtotal;
> + if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> + vtotal /= 2;
> +
> + crtc->scanline_offset = vtotal - 1;
> + } else if (HAS_DDI(dev) &&
> + intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI)) {
> + crtc->scanline_offset = 2;
> + } else
> + crtc->scanline_offset = 1;
> +}
> +
> static int __intel_set_mode(struct drm_crtc *crtc,
> struct drm_display_mode *mode,
> int x, int y, struct drm_framebuffer *fb)
> @@ -10262,8 +10300,11 @@ static int __intel_set_mode(struct drm_crtc *crtc,
> }
>
> /* Now enable the clocks, plane, pipe, and connectors that we set
> up. */
> - for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc)
> + for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) {
> + update_scanline_offset(intel_crtc);
> +
> dev_priv->display.crtc_enable(&intel_crtc->base);
> + }
>
> /* FIXME: add subpixel order */
> done:
> @@ -11789,6 +11830,8 @@ static void intel_sanitize_crtc(struct intel_crtc
> *crtc)
> */
> crtc->cpu_fifo_underrun_disabled = true;
> crtc->pch_fifo_underrun_disabled = true;
> +
> + update_scanline_offset(crtc);
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 32a74e1..dd562b9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -403,6 +403,8 @@ struct intel_crtc {
> } wm;
>
> wait_queue_head_t vbl_wait;
> +
> + int scanline_offset;
> };
>
> struct intel_plane_wm_parameters {
> --
> 1.8.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

[-- Attachment #1.2: Type: text/html, Size: 8156 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]