Re: [PATCH 6/6] gpu: ipu-v3: Use videomode in struct ipu_di_signal_cfg

[Philipp Zabel <p.zabel@pengutronix.de>]

Hi Steve,

I think this is a good idea. Two small comments below.

Am Montag, den 15.12.2014, 16:29 -0800 schrieb slongerbeam@gmail.com:
> From: Steve Longerbeam <steve_longerbeam@mentor.com>
> 
> This patch changes struct ipu_di_signal_cfg to use struct videomode
> to define video timings and flags.
> 
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
>  drivers/gpu/drm/imx/ipuv3-crtc.c |   27 +++---------
>  drivers/gpu/ipu-v3/ipu-di.c      |   89 ++++++++++++++++++++------------------
>  include/video/imx-ipu-v3.h       |   19 ++------
>  3 files changed, 57 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c
> index fb16026..0a50129 100644
> --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> @@ -158,35 +158,20 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	out_pixel_fmt = ipu_crtc->interface_pix_fmt;
>  
> -	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> -		sig_cfg.interlaced = 1;
> -	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> -		sig_cfg.Hsync_pol = 1;
> -	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> -		sig_cfg.Vsync_pol = 1;
> -
>  	sig_cfg.enable_pol = 1;
>  	sig_cfg.clk_pol = 0;
> -	sig_cfg.width = mode->hdisplay;
> -	sig_cfg.height = mode->vdisplay;
>  	sig_cfg.pixel_fmt = out_pixel_fmt;
> -	sig_cfg.h_start_width = mode->htotal - mode->hsync_end;
> -	sig_cfg.h_sync_width = mode->hsync_end - mode->hsync_start;
> -	sig_cfg.h_end_width = mode->hsync_start - mode->hdisplay;
> -
> -	sig_cfg.v_start_width = mode->vtotal - mode->vsync_end;
> -	sig_cfg.v_sync_width = mode->vsync_end - mode->vsync_start;
> -	sig_cfg.v_end_width = mode->vsync_start - mode->vdisplay;
> -	sig_cfg.pixelclock = mode->clock * 1000;
>  	sig_cfg.clkflags = ipu_crtc->di_clkflags;
> -
>  	sig_cfg.v_to_h_sync = 0;
> -
>  	sig_cfg.hsync_pin = ipu_crtc->di_hsync_pin;
>  	sig_cfg.vsync_pin = ipu_crtc->di_vsync_pin;
>  
> -	ret = ipu_dc_init_sync(ipu_crtc->dc, ipu_crtc->di, sig_cfg.interlaced,
> -			out_pixel_fmt, mode->hdisplay);
> +	videomode_from_drm_display_mode(mode, &sig_cfg.mode);
> +
> +	ret = ipu_dc_init_sync(ipu_crtc->dc, ipu_crtc->di,
> +			       (mode->flags & DRM_MODE_FLAG_INTERLACE) ?
> +			       true : false,

The interlaced parameter to ipu_dc_init_sync is of type bool, so the
()?true:false is superfluous.

[...]
> @@ -433,10 +437,11 @@ static void ipu_di_config_clock(struct ipu_di *di,
>  			unsigned long in_rate;
>  			unsigned div;
>  
> -			clk_set_rate(clk, sig->pixelclock);
> +			clk_set_rate(clk, sig->mode.pixelclock);
>  
>  			in_rate = clk_get_rate(clk);
> -			div = (in_rate + sig->pixelclock / 2) / sig->pixelclock;
> +			div = (in_rate + sig->mode.pixelclock / 2) /
> +				sig->mode.pixelclock;

Let's use this opportunity to switch to DIV_ROUND_CLOSEST here ...

> @@ -473,10 +479,11 @@ static void ipu_di_config_clock(struct ipu_di *di,
>  
>  			clk = di->clk_di;
>  
> -			clk_set_rate(clk, sig->pixelclock);
> +			clk_set_rate(clk, sig->mode.pixelclock);
>  
>  			in_rate = clk_get_rate(clk);
> -			div = (in_rate + sig->pixelclock / 2) / sig->pixelclock;
> +			div = (in_rate + sig->mode.pixelclock / 2) /
> +				sig->mode.pixelclock;

... and here.

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel