diff mbox

[PATCHv5,2/8] staging: imx-drm: Add RGB666 support for parallel display.

Message ID 1386268092-21719-2-git-send-email-denis@eukrea.com (mailing list archive)
State New, archived
Headers show

Commit Message

Denis Carikli Dec. 5, 2013, 6:28 p.m. UTC
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: driverdev-devel@linuxdriverproject.org
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-media@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric BĂ©nard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v3->v5:
- Use the correct RGB order.

ChangeLog v2->v3:
- Added some interested people in the Cc list.
- Removed the commit message long desciption that was just a copy of the short
  description.
- Rebased the patch.
- Fixed a copy-paste error in the ipu_dc_map_clear parameter.
---
 .../bindings/staging/imx-drm/fsl-imx-drm.txt       |    2 +-
 drivers/staging/imx-drm/ipu-v3/ipu-dc.c            |    9 +++++++++
 drivers/staging/imx-drm/parallel-display.c         |    2 ++
 3 files changed, 12 insertions(+), 1 deletion(-)

Comments

Thierry Reding Dec. 6, 2013, 1:14 p.m. UTC | #1
On Thu, Dec 05, 2013 at 07:28:06PM +0100, Denis Carikli wrote:
[...]
> diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
[...]
> @@ -155,6 +156,8 @@ static int ipu_pixfmt_to_map(u32 fmt)
>  		return IPU_DC_MAP_BGR666;
>  	case V4L2_PIX_FMT_BGR24:
>  		return IPU_DC_MAP_BGR24;
> +	case V4L2_PIX_FMT_RGB666:
> +		return IPU_DC_MAP_RGB666;

Why is this DRM driver even using V4L2 pixel formats in the first place?

Thierry
Lucas Stach Dec. 6, 2013, 1:29 p.m. UTC | #2
Am Freitag, den 06.12.2013, 14:14 +0100 schrieb Thierry Reding:
> On Thu, Dec 05, 2013 at 07:28:06PM +0100, Denis Carikli wrote:
> [...]
> > diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
> [...]
> > @@ -155,6 +156,8 @@ static int ipu_pixfmt_to_map(u32 fmt)
> >  		return IPU_DC_MAP_BGR666;
> >  	case V4L2_PIX_FMT_BGR24:
> >  		return IPU_DC_MAP_BGR24;
> > +	case V4L2_PIX_FMT_RGB666:
> > +		return IPU_DC_MAP_RGB666;
> 
> Why is this DRM driver even using V4L2 pixel formats in the first place?
> 
Because imx-drm is actually a misnomer. The i.MX IPU is a multifunction
device, which as one part has the display controllers, but also camera
interfaces and mem-to-mem scaler devices, which are hooked up via the
V4L2 interface.

The generic IPU part, which is used for example for programming the DMA
channels is using V4L2 pixel formats as a common base. We have patches
to split this out and make this fact more visible. (The IPU core will be
placed aside the Tegra host1x driver)

Regards,
Lucas
Thierry Reding Dec. 6, 2013, 2:16 p.m. UTC | #3
On Fri, Dec 06, 2013 at 02:29:22PM +0100, Lucas Stach wrote:
> Am Freitag, den 06.12.2013, 14:14 +0100 schrieb Thierry Reding:
> > On Thu, Dec 05, 2013 at 07:28:06PM +0100, Denis Carikli wrote:
> > [...]
> > > diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
> > [...]
> > > @@ -155,6 +156,8 @@ static int ipu_pixfmt_to_map(u32 fmt)
> > >  		return IPU_DC_MAP_BGR666;
> > >  	case V4L2_PIX_FMT_BGR24:
> > >  		return IPU_DC_MAP_BGR24;
> > > +	case V4L2_PIX_FMT_RGB666:
> > > +		return IPU_DC_MAP_RGB666;
> > 
> > Why is this DRM driver even using V4L2 pixel formats in the first place?
> > 
> Because imx-drm is actually a misnomer. The i.MX IPU is a multifunction
> device, which as one part has the display controllers, but also camera
> interfaces and mem-to-mem scaler devices, which are hooked up via the
> V4L2 interface.
> 
> The generic IPU part, which is used for example for programming the DMA
> channels is using V4L2 pixel formats as a common base. We have patches
> to split this out and make this fact more visible. (The IPU core will be
> placed aside the Tegra host1x driver)

Have you considered splitting thing up further and move out the display
controller driver to DRM and the camera driver to V4L2? I mean, if that
is even possible with a reasonable amount of work.

Is the "mem-to-mem" the same as the "DMA channels" you mentioned? If it
only does DMA, why does it even need to worry about pixel formats?

Thierry
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
index b876d49..2d24425 100644
--- a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
+++ b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
@@ -29,7 +29,7 @@  Required properties:
 - crtc: the crtc this display is connected to, see below
 Optional properties:
 - interface_pix_fmt: How this display is connected to the
-  crtc. Currently supported types: "rgb24", "rgb565", "bgr666"
+  crtc. Currently supported types: "rgb24", "rgb565", "bgr666", "rgb666"
 - edid: verbatim EDID data block describing attached display.
 - ddc: phandle describing the i2c bus handling the display data
   channel
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
index d0e3bc3..617e65b 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
@@ -92,6 +92,7 @@  enum ipu_dc_map {
 	IPU_DC_MAP_GBR24, /* TVEv2 */
 	IPU_DC_MAP_BGR666,
 	IPU_DC_MAP_BGR24,
+	IPU_DC_MAP_RGB666,
 };
 
 struct ipu_dc {
@@ -155,6 +156,8 @@  static int ipu_pixfmt_to_map(u32 fmt)
 		return IPU_DC_MAP_BGR666;
 	case V4L2_PIX_FMT_BGR24:
 		return IPU_DC_MAP_BGR24;
+	case V4L2_PIX_FMT_RGB666:
+		return IPU_DC_MAP_RGB666;
 	default:
 		return -EINVAL;
 	}
@@ -404,6 +407,12 @@  int ipu_dc_init(struct ipu_soc *ipu, struct device *dev,
 	ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 1, 15, 0xff); /* green */
 	ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 0, 23, 0xff); /* blue */
 
+	/* rgb666 */
+	ipu_dc_map_clear(priv, IPU_DC_MAP_RGB666);
+	ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 0, 5, 0xfc); /* blue */
+	ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 1, 11, 0xfc); /* green */
+	ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 2, 17, 0xfc); /* red */
+
 	return 0;
 }
 
diff --git a/drivers/staging/imx-drm/parallel-display.c b/drivers/staging/imx-drm/parallel-display.c
index 24aa9be..bb71d6d 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -222,6 +222,8 @@  static int imx_pd_probe(struct platform_device *pdev)
 			imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB565;
 		else if (!strcmp(fmt, "bgr666"))
 			imxpd->interface_pix_fmt = V4L2_PIX_FMT_BGR666;
+		else if (!strcmp(fmt, "rgb666"))
+			imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB666;
 	}
 
 	imxpd->dev = &pdev->dev;