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

sourab gupta
2014-05-16T06:24:10Z

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

On Fri, May 16, 2014 at 11:03 AM, akash goel <akash.goels@gmail.com> wrote:

>
> 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>"
>
>
Don't have enough info about the initial scanline counter values for
HSW+ and gen2. Otherwise, you can add my r-b tag

Reviewed-by: "Sourab Gupta <sourabgupta@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
>>
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>

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

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