Hi Steve, I think this is a good idea. Two small comments below. Am Montag, den 15.12.2014, 16:29 -0800 schrieb slongerbeam@xxxxxxxxx: > From: Steve Longerbeam <steve_longerbeam@xxxxxxxxxx> > > 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@xxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel