Re: [PATCH v4 21/34] drm/exynos: Use mode_set to configure fimd

[Andrzej Hajda <a.hajda@samsung.com>]

Hi Sean,

On 01/30/2014 10:19 PM, Sean Paul wrote:
> This patch uses the mode passed into mode_set to configure fimd instead
> of directly using the panel from context. This will allow us to move
> the exynos_drm_display implementation out of fimd, where it doesn't
> belong.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
(...)
> +	vblank = mode->crtc_vblank_end - mode->crtc_vblank_start;
> +	vsync_len = mode->crtc_vsync_end - mode->crtc_vsync_start;
> +	vbpd = (vblank - vsync_len) / 2;
> +	vfpd = vblank - vsync_len - vbpd;

It should be rather:
	vbpd = mode->crtc_vtotal - mode->crtc_vsync_end;
	vfpd = mode->crtc_vsync_start - mode->crtc_vdisplay;

> +
> +	val = VIDTCON0_VBPD(vbpd - 1) |
> +		VIDTCON0_VFPD(vfpd - 1) |
> +		VIDTCON0_VSPW(vsync_len - 1);
>  	writel(val, ctx->regs + driver_data->timing_base + VIDTCON0);
>  
>  	/* setup horizontal timing values.  */
> -	val = VIDTCON1_HBPD(vm->hback_porch - 1) |
> -	       VIDTCON1_HFPD(vm->hfront_porch - 1) |
> -	       VIDTCON1_HSPW(vm->hsync_len - 1);
> +	hblank = mode->crtc_hblank_end - mode->crtc_hblank_start;
> +	hsync_len = mode->crtc_hsync_end - mode->crtc_hsync_start;
> +	hbpd = (hblank - hsync_len) / 2;
> +	hfpd = hblank - hsync_len - hbpd;

	hbpd = mode->crtc_htotal - mode->crtc_hsync_end;
	hfpd = mode->crtc_hsync_start - mode->crtc_hdisplay;

I have pointed it out already in my exynos/dsi patches[1](they were
based on v3 of your patches), but I forgot to cc it to you.

[1] http://permalink.gmane.org/gmane.comp.video.dri.devel/98737

Regards
Andrzej