Re: [PATCH v2 1/7] gpu: ipu-di: Add ipu_di_adjust_videomode()
From: Philipp Zabel <p.zabel@pengutronix.de>
Date: 2014-12-19 11:07:13
Also in:
dri-devel, lkml
Hi Steve, Am Donnerstag, den 18.12.2014, 18:00 -0800 schrieb Steve Longerbeam:
quoted hunk ↗ jump to hunk
From: Jiada Wang <redacted> On some monitors, high resolution modes are not working, exhibiting pixel column truncation problems (for example, 1280x1024 displays as 1280x1022). The function ipu_di_adjust_videomode() aims to fix these issues by adjusting a passed videomode to IPU restrictions. The function can be called from the drm_crtc_helper_funcs->mode_fixup() methods. Signed-off-by: Jiada Wang <redacted> Signed-off-by: Deepak Das <redacted> Signed-off-by: Steve Longerbeam <redacted> --- drivers/gpu/ipu-v3/ipu-di.c | 29 +++++++++++++++++++++++++++++ include/video/imx-ipu-v3.h | 2 ++ 2 files changed, 31 insertions(+)diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c index c490ba4..46f9570 100644 --- a/drivers/gpu/ipu-v3/ipu-di.c +++ b/drivers/gpu/ipu-v3/ipu-di.c@@ -511,6 +511,35 @@ static void ipu_di_config_clock(struct ipu_di *di, clk_get_rate(di->clk_di_pixel) / (clkgen0 >> 4)); } +/* + * This function is called to adjust a video mode to IPU restrictions. + * It is meant to be called from drm crtc mode_fixup() methods. + */ +int ipu_di_adjust_videomode(struct ipu_di *di, struct videomode *mode) +{ + u32 diff; + + if (mode->vfront_porch >= 2) + return 0; + + diff = 2 - mode->vfront_porch; + + if (mode->vback_porch >= diff) { + mode->vfront_porch = 2; + mode->vback_porch -= diff;
Should we also add
else if (mode->vback_porch >= 1 && mode->vsync_len > 1) {
mode->vback_porch--;
mode->vsync_len--;
here and maybe move the two "mode->vback_porch = 2" to a single place
below the conditional statement?
+ } else if (mode->vsync_len > diff) {
+ mode->vfront_porch = 2;
+ mode->vsync_len = mode->vsync_len - diff;Why use "vback_porch -= diff" above, but not "vsync_len -= diff" here?
+ } else {
+ dev_warn(di->ipu->dev, "failed to adjust videomode\n");
+ return -EINVAL;
+ }
+
+ dev_warn(di->ipu->dev, "videomode adapted for IPU restrictions\n");Since we can return the adjusted mode to userspace now, I think this should be dev_dbg. regards Philipp