Re: [PATCH 1/3] drm/i915: Program VSYNCSHIFT in a more consistent manner

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Fri, 28 Mar 2014 23:29:30 +0200
ville.syrjala@linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> When interlaced sdvo output is used, vsyncshift should supposedly
> be (htotal-1)/2. In reality PIPECONF/TRANSCONF will override it by
> using the legacy vsyncshift interlace mode which causes the hardware
> to ignore the VSYNCSHIFT register.
> 
> The only odd thing here is that on PCH platforms we program the
> VSYNCSHIFT on both CPU and PCH, and it's not entirely clear if both
> sides have to agree on the value or not. On the CPU side there's no
> way to override the value via PIPECONF anymore, so if we want to make
> the CPU side agree with the PCH side, we should probably program the
> approriate value into VSYNCSHIFT manually. So let's do that, but for
> now leave the PCH side to still use the legacy interlace mode in
> TRANSCONF.
> 
> We can also drop the gen2 check since gen2 doesn't support interlaced
> modes at all.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  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 d633139..a9a4f6a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5377,21 +5377,23 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>  	enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
>  	struct drm_display_mode *adjusted_mode =
>  		&intel_crtc->config.adjusted_mode;
> -	uint32_t vsyncshift, crtc_vtotal, crtc_vblank_end;
> +	uint32_t vsyncshift = 0, crtc_vtotal, crtc_vblank_end;
>  
>  	/* We need to be careful not to changed the adjusted mode, for otherwise
>  	 * the hw state checker will get angry at the mismatch. */
>  	crtc_vtotal = adjusted_mode->crtc_vtotal;
>  	crtc_vblank_end = adjusted_mode->crtc_vblank_end;
>  
> -	if (!IS_GEN2(dev) && adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
>  		/* the chip adds 2 halflines automatically */
>  		crtc_vtotal -= 1;
>  		crtc_vblank_end -= 1;
> -		vsyncshift = adjusted_mode->crtc_hsync_start
> -			     - adjusted_mode->crtc_htotal / 2;
> -	} else {
> -		vsyncshift = 0;
> +
> +		if (intel_pipe_has_type(&intel_crtc->base, INTEL_OUTPUT_SDVO))
> +			vsyncshift = (adjusted_mode->crtc_htotal - 1) / 2;
> +		else
> +			vsyncshift = adjusted_mode->crtc_hsync_start -
> +				adjusted_mode->crtc_htotal / 2;
>  	}
>  
>  	if (INTEL_INFO(dev)->gen > 3)

My only concern here is that some chip might try to use a nonzero
vsyncshift for a non-interlaced mode.  But that should be easy to
bisect to if so, so

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx