diff mbox

[PATCHv2,2/3] drm/imx: convey the pixelclk-active and de-active flags from DT to the ipu-di driver

Message ID 1464071511-16363-3-git-send-email-LW@KARO-electronics.de (mailing list archive)
State New, archived
Headers show

Commit Message

Lothar Waßmann May 24, 2016, 6:31 a.m. UTC 
Currently these flags are lost in the call
drm_display_mode_from_videomode()

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 drivers/gpu/drm/imx/imx-ldb.c          | 37 ++++++++++++++++++++++++++--------
 drivers/gpu/drm/imx/parallel-display.c | 31 ++++++++++++++++++++++++----
 2 files changed, 56 insertions(+), 12 deletions(-)

Comments

Lothar Waßmann May 25, 2016, 6:11 a.m. UTC | #1 
Hi,

On Tue, 24 May 2016 16:16:40 +0200 Philipp Zabel wrote:
> Hi Lothar,
> 
> thank you for rebasing. I have applied the other two patches.
> With this one, I'd like to avoid the duplicated code. See below:
> 
> Am Dienstag, den 24.05.2016, 08:31 +0200 schrieb Lothar Waßmann:
> > Currently these flags are lost in the call
> > drm_display_mode_from_videomode()
> > 
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > ---
> >  drivers/gpu/drm/imx/imx-ldb.c          | 37 ++++++++++++++++++++++++++--------
> >  drivers/gpu/drm/imx/parallel-display.c | 31 ++++++++++++++++++++++++----
> >  2 files changed, 56 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
> > index b2dc4df..b17a246 100644
> > --- a/drivers/gpu/drm/imx/imx-ldb.c
> > +++ b/drivers/gpu/drm/imx/imx-ldb.c
> [...]
> > @@ -601,10 +606,26 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data)
> >  			channel->edid = kmemdup(edidp, channel->edid_len,
> >  						GFP_KERNEL);
> >  		} else if (!channel->panel) {
> > -			ret = of_get_drm_display_mode(child, &channel->mode,
> > -						OF_USE_NATIVE_MODE);
> > -			if (!ret)
> > -				channel->mode_valid = 1;
> > +			struct videomode vm;
> > +
> > +			ret = of_get_videomode(child, &vm, OF_USE_NATIVE_MODE);
> > +			if (ret)
> > +				return ret;
> > +			drm_display_mode_from_videomode(&vm, &channel->mode);
> > +
> > +			if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > +				channel->bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
> > +			if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > +				channel->bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> > +
> > +			if (vm.flags & DISPLAY_FLAGS_DE_LOW)
> > +				channel->bus_flags |= DRM_BUS_FLAG_DE_LOW;
> > +			if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> > +				channel->bus_flags |= DRM_BUS_FLAG_DE_HIGH;
> > +
> > +			drm_mode_debug_printmodeline(&channel->mode);
> > +			channel->mode_valid = 1;
> [...]
> > @@ -81,10 +87,27 @@ static int imx_pd_connector_get_modes(struct drm_connector *connector)
> >  
> >  	if (np) {
> >  		struct drm_display_mode *mode = drm_mode_create(connector->dev);
> > +		struct videomode vm;
> > +		int ret;
> >  
> >  		if (!mode)
> >  			return -EINVAL;
> > -		of_get_drm_display_mode(np, &imxpd->mode, OF_USE_NATIVE_MODE);
> > +
> > +		ret = of_get_videomode(np, &vm, OF_USE_NATIVE_MODE);
> > +		if (ret)
> > +			return ret;
> > +		drm_display_mode_from_videomode(&vm, &imxpd->mode);
> > +		drm_mode_debug_printmodeline(&imxpd->mode);
> > +
> > +		if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > +			imxpd->bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
> > +		if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > +			imxpd->bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> > +
> > +		if (vm.flags & DISPLAY_FLAGS_DE_LOW)
> > +			imxpd->bus_flags |= DRM_BUS_FLAG_DE_LOW;
> > +		if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> > +			imxpd->bus_flags |= DRM_BUS_FLAG_DE_HIGH;
> 
> This could be shared between ldb and parallel drivers if you extended
> of_get_drm_display_mode to also return bus_flags, for example. The flags
> translation could be put into a drm_bus_flags_from_videomode helper.
> 
I had that in mind also, but hesitated to change the generic
of_get_drm_display_mode() to implement some i.MX specific functionality.


Lothar Waßmann
Philipp Zabel May 25, 2016, 10:26 a.m. UTC | #2 
Am Mittwoch, den 25.05.2016, 08:11 +0200 schrieb Lothar Waßmann:
[...]
> > > @@ -81,10 +87,27 @@ static int imx_pd_connector_get_modes(struct drm_connector *connector)
> > >  
> > >  	if (np) {
> > >  		struct drm_display_mode *mode = drm_mode_create(connector->dev);
> > > +		struct videomode vm;
> > > +		int ret;
> > >  
> > >  		if (!mode)
> > >  			return -EINVAL;
> > > -		of_get_drm_display_mode(np, &imxpd->mode, OF_USE_NATIVE_MODE);
> > > +
> > > +		ret = of_get_videomode(np, &vm, OF_USE_NATIVE_MODE);
> > > +		if (ret)
> > > +			return ret;
> > > +		drm_display_mode_from_videomode(&vm, &imxpd->mode);
> > > +		drm_mode_debug_printmodeline(&imxpd->mode);
> > > +
> > > +		if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > > +			imxpd->bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
> > > +		if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > > +			imxpd->bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
> > > +
> > > +		if (vm.flags & DISPLAY_FLAGS_DE_LOW)
> > > +			imxpd->bus_flags |= DRM_BUS_FLAG_DE_LOW;
> > > +		if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> > > +			imxpd->bus_flags |= DRM_BUS_FLAG_DE_HIGH;
> > 
> > This could be shared between ldb and parallel drivers if you extended
> > of_get_drm_display_mode to also return bus_flags, for example. The flags
> > translation could be put into a drm_bus_flags_from_videomode helper.
> > 
> I had that in mind also, but hesitated to change the generic
> of_get_drm_display_mode() to implement some i.MX specific functionality.

Since the videomode obtained from DT includes both the timing and pin
polarity information, and the drm way to express these are
drm_display_mode and bus_flags, respectively, there's nothing i.MX
specific about this.

regards
Philipp
diff mbox

Patch

diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index b2dc4df..b17a246 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -27,6 +27,7 @@ 
 #include <linux/of_graph.h>
 #include <video/of_display_timing.h>
 #include <video/of_videomode.h>
+#include <video/videomode.h>
 #include <linux/regmap.h>
 #include <linux/videodev2.h>
 
@@ -65,7 +66,8 @@  struct imx_ldb_channel {
 	int edid_len;
 	struct drm_display_mode mode;
 	int mode_valid;
-	int bus_format;
+	u32 bus_format;
+	u32 bus_flags;
 };
 
 struct bus_mux {
@@ -102,8 +104,10 @@  static int imx_ldb_connector_get_modes(struct drm_connector *connector)
 		struct drm_display_info *di = &connector->display_info;
 
 		num_modes = imx_ldb_ch->panel->funcs->get_modes(imx_ldb_ch->panel);
-		if (!imx_ldb_ch->bus_format && di->num_bus_formats)
+		if (!imx_ldb_ch->bus_format && di->num_bus_formats) {
 			imx_ldb_ch->bus_format = di->bus_formats[0];
+			imx_ldb_ch->bus_flags = di->bus_flags;
+		}
 		if (num_modes > 0)
 			return num_modes;
 	}
@@ -202,7 +206,8 @@  static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
 		break;
 	}
 
-	imx_drm_set_bus_format(encoder, bus_format);
+	imx_drm_set_bus_config(encoder, bus_format, 2, 3,
+			imx_ldb_ch->bus_flags);
 }
 
 static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
@@ -558,7 +563,7 @@  static int imx_ldb_bind(struct device *dev, struct device *master, void *data)
 
 		ret = of_property_read_u32(child, "reg", &i);
 		if (ret || i < 0 || i > 1)
-			return -EINVAL;
+			return ret ?: -EINVAL;
 
 		if (dual && i > 0) {
 			dev_warn(dev, "dual-channel mode, ignoring second output\n");
@@ -601,10 +606,26 @@  static int imx_ldb_bind(struct device *dev, struct device *master, void *data)
 			channel->edid = kmemdup(edidp, channel->edid_len,
 						GFP_KERNEL);
 		} else if (!channel->panel) {
-			ret = of_get_drm_display_mode(child, &channel->mode,
-						OF_USE_NATIVE_MODE);
-			if (!ret)
-				channel->mode_valid = 1;
+			struct videomode vm;
+
+			ret = of_get_videomode(child, &vm, OF_USE_NATIVE_MODE);
+			if (ret)
+				return ret;
+
+			drm_display_mode_from_videomode(&vm, &channel->mode);
+
+			if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+				channel->bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
+			if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+				channel->bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
+
+			if (vm.flags & DISPLAY_FLAGS_DE_LOW)
+				channel->bus_flags |= DRM_BUS_FLAG_DE_LOW;
+			if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
+				channel->bus_flags |= DRM_BUS_FLAG_DE_HIGH;
+
+			drm_mode_debug_printmodeline(&channel->mode);
+			channel->mode_valid = 1;
 		}
 
 		channel->bus_format = of_get_bus_format(dev, child);
diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
index 3db634d..238057b 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -19,9 +19,12 @@ 
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_panel.h>
+#include <linux/of_graph.h>
 #include <linux/videodev2.h>
+#include <video/display_timing.h>
 #include <video/of_display_timing.h>
-#include <linux/of_graph.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
 
 #include "imx-drm.h"
 
@@ -35,6 +38,7 @@  struct imx_parallel_display {
 	void *edid;
 	int edid_len;
 	u32 bus_format;
+	u32 bus_flags;
 	int mode_valid;
 	struct drm_display_mode mode;
 	struct drm_panel *panel;
@@ -57,8 +61,10 @@  static int imx_pd_connector_get_modes(struct drm_connector *connector)
 		struct drm_display_info *di = &connector->display_info;
 
 		num_modes = imxpd->panel->funcs->get_modes(imxpd->panel);
-		if (!imxpd->bus_format && di->num_bus_formats)
+		if (!imxpd->bus_format && di->num_bus_formats) {
 			imxpd->bus_format = di->bus_formats[0];
+			imxpd->bus_flags = di->bus_flags;
+		}
 		if (num_modes > 0)
 			return num_modes;
 	}
@@ -81,10 +87,27 @@  static int imx_pd_connector_get_modes(struct drm_connector *connector)
 
 	if (np) {
 		struct drm_display_mode *mode = drm_mode_create(connector->dev);
+		struct videomode vm;
+		int ret;
 
 		if (!mode)
 			return -EINVAL;
-		of_get_drm_display_mode(np, &imxpd->mode, OF_USE_NATIVE_MODE);
+
+		ret = of_get_videomode(np, &vm, OF_USE_NATIVE_MODE);
+		if (ret)
+			return ret;
+		drm_display_mode_from_videomode(&vm, &imxpd->mode);
+		drm_mode_debug_printmodeline(&imxpd->mode);
+
+		if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+			imxpd->bus_flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
+		if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+			imxpd->bus_flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
+
+		if (vm.flags & DISPLAY_FLAGS_DE_LOW)
+			imxpd->bus_flags |= DRM_BUS_FLAG_DE_LOW;
+		if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
+			imxpd->bus_flags |= DRM_BUS_FLAG_DE_HIGH;
 		drm_mode_copy(mode, &imxpd->mode);
 		mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
 		drm_mode_probed_add(connector, mode);
@@ -116,7 +139,7 @@  static void imx_pd_encoder_prepare(struct drm_encoder *encoder)
 {
 	struct imx_parallel_display *imxpd = enc_to_imxpd(encoder);
 	imx_drm_set_bus_config(encoder, imxpd->bus_format, 2, 3,
-			       imxpd->connector.display_info.bus_flags);
+			       imxpd->bus_flags);
 }
 
 static void imx_pd_encoder_commit(struct drm_encoder *encoder)