diff mbox

[v2,1/7] gpu: ipu-di: Add ipu_di_adjust_videomode()

Message ID 1418954426-21909-2-git-send-email-steve_longerbeam@mentor.com (mailing list archive)
State New, archived
Headers show

Commit Message

Steve Longerbeam Dec. 19, 2014, 2 a.m. UTC
From: Jiada Wang <jiada_wang@mentor.com>

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 <jiada_wang@mentor.com>
Signed-off-by: Deepak Das <deepak_das@mentor.com>
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
 drivers/gpu/ipu-v3/ipu-di.c |   29 +++++++++++++++++++++++++++++
 include/video/imx-ipu-v3.h  |    2 ++
 2 files changed, 31 insertions(+)

Comments

Philipp Zabel Dec. 19, 2014, 11:07 a.m. UTC | #1
Hi Steve,

Am Donnerstag, den 18.12.2014, 18:00 -0800 schrieb Steve Longerbeam:
> From: Jiada Wang <jiada_wang@mentor.com>
> 
> 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 <jiada_wang@mentor.com>
> Signed-off-by: Deepak Das <deepak_das@mentor.com>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
>  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
diff mbox

Patch

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;
+	} else if (mode->vsync_len > diff) {
+		mode->vfront_porch = 2;
+		mode->vsync_len = mode->vsync_len - diff;
+	} else {
+		dev_warn(di->ipu->dev, "failed to adjust videomode\n");
+		return -EINVAL;
+	}
+
+	dev_warn(di->ipu->dev, "videomode adapted for IPU restrictions\n");
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ipu_di_adjust_videomode);
+
 int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
 {
 	u32 reg;
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index c74bf4a..d333d54 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -17,6 +17,7 @@ 
 #include <linux/bitmap.h>
 #include <linux/fb.h>
 #include <media/v4l2-mediabus.h>
+#include <video/videomode.h>
 
 struct ipu_soc;
 
@@ -236,6 +237,7 @@  void ipu_di_put(struct ipu_di *);
 int ipu_di_disable(struct ipu_di *);
 int ipu_di_enable(struct ipu_di *);
 int ipu_di_get_num(struct ipu_di *);
+int ipu_di_adjust_videomode(struct ipu_di *di, struct videomode *mode);
 int ipu_di_init_sync_panel(struct ipu_di *, struct ipu_di_signal_cfg *sig);
 
 /*