[PATCH] staging: imx_drm: imx_ldb: fix 18 bit format

[PATCH] staging: imx_drm: imx_ldb: fix 18 bit format

[Steffen Trumtrar]

From: Markus Niebel <Markus.Niebel-77yLcl8UjJQ@public.gmane.org>

Since 18 bit is supported as datawidth in device tree it should be
supported in driver. Beside the LDB channel the IPU channel has also
to be configured to use BGR666.

Signed-off-by: Markus Niebel <Markus.Niebel-77yLcl8UjJQ@public.gmane.org>
Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
This is based on the series: "staging: drm/imx: Add LDB support"
by Philipp.


 drivers/staging/imx-drm/imx-ldb.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c
index d8fc93c..84bf7d3 100644
--- a/drivers/staging/imx-drm/imx-ldb.c
+++ b/drivers/staging/imx-drm/imx-ldb.c
@@ -179,6 +179,7 @@ static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
 	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
 	struct imx_ldb *ldb = imx_ldb_ch->ldb;
 	struct drm_display_mode *mode = &encoder->crtc->mode;
+	u32 pixel_fmt;
 	unsigned long serial_clk;
 	unsigned long di_clk = mode->clock * 1000;
 	int mux = imx_drm_encoder_get_mux_id(imx_ldb_ch->imx_drm_encoder,
@@ -194,8 +195,23 @@ static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
 		imx_ldb_set_clock(ldb, mux, imx_ldb_ch->chno, serial_clk, di_clk);
 	}
 
+	switch (imx_ldb_ch->chno) {
+	case 0:
+		pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
+			V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
+		break;
+	case 1:
+		pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH1_24) ?
+			V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
+		break;
+	default:
+		dev_err(ldb->dev, "unable to config di%d panel format\n",
+			imx_ldb_ch->chno);
+		pixel_fmt = V4L2_PIX_FMT_RGB24;
+	}
+
 	imx_drm_crtc_panel_format_pins(encoder->crtc, DRM_MODE_ENCODER_LVDS,
-			V4L2_PIX_FMT_RGB24, 2, 3);
+			pixel_fmt, 2, 3);
 }
 
 static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
-- 
1.8.2.rc2

Re: [PATCH] staging: imx_drm: imx_ldb: fix 18 bit format

[Shawn Guo]

On Fri, Jun 07, 2013 at 10:37:08AM +0200, Steffen Trumtrar wrote:
> From: Markus Niebel <Markus.Niebel-77yLcl8UjJQ@public.gmane.org>
> 
> Since 18 bit is supported as datawidth in device tree it should be
> supported in driver. Beside the LDB channel the IPU channel has also
> to be configured to use BGR666.
> 
> Signed-off-by: Markus Niebel <Markus.Niebel-77yLcl8UjJQ@public.gmane.org>
> Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> This is based on the series: "staging: drm/imx: Add LDB support"
> by Philipp.
> 
> 
>  drivers/staging/imx-drm/imx-ldb.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c
> index d8fc93c..84bf7d3 100644
> --- a/drivers/staging/imx-drm/imx-ldb.c
> +++ b/drivers/staging/imx-drm/imx-ldb.c
> @@ -179,6 +179,7 @@ static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
>  	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
>  	struct imx_ldb *ldb = imx_ldb_ch->ldb;
>  	struct drm_display_mode *mode = &encoder->crtc->mode;
> +	u32 pixel_fmt;
>  	unsigned long serial_clk;
>  	unsigned long di_clk = mode->clock * 1000;
>  	int mux = imx_drm_encoder_get_mux_id(imx_ldb_ch->imx_drm_encoder,
> @@ -194,8 +195,23 @@ static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
>  		imx_ldb_set_clock(ldb, mux, imx_ldb_ch->chno, serial_clk, di_clk);
>  	}
>  
> +	switch (imx_ldb_ch->chno) {
> +	case 0:
> +		pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
> +			V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
> +		break;
> +	case 1:
> +		pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH1_24) ?
> +			V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
> +		break;
> +	default:
> +		dev_err(ldb->dev, "unable to config di%d panel format\n",
> +			imx_ldb_ch->chno);
> +		pixel_fmt = V4L2_PIX_FMT_RGB24;
> +	}
> +
>  	imx_drm_crtc_panel_format_pins(encoder->crtc, DRM_MODE_ENCODER_LVDS,
> -			V4L2_PIX_FMT_RGB24, 2, 3);
> +			pixel_fmt, 2, 3);

It seems the patch is based on an updated version of "staging: drm/imx:
Add LDB support"?  The last one from Philipp [1] calls function
imx_drm_crtc_panel_format() rather than imx_drm_crtc_panel_format_pins()
there.

Shawn

[1] http://thread.gmane.org/gmane.linux.drivers.driver-project.devel/36836/focus=36834

>  }
>  
>  static void imx_ldb_encoder_commit(struct drm_encoder *encoder)
> -- 
> 1.8.2.rc2
> 

Re: [PATCH] staging: imx_drm: imx_ldb: fix 18 bit format

[Steffen Trumtrar]

Hi Shawn!

On Sat, Jun 08, 2013 at 12:49:47PM +0800, Shawn Guo wrote:
> On Fri, Jun 07, 2013 at 10:37:08AM +0200, Steffen Trumtrar wrote:
> > From: Markus Niebel <Markus.Niebel@tqs.de>
> > 
> > Since 18 bit is supported as datawidth in device tree it should be
> > supported in driver. Beside the LDB channel the IPU channel has also
> > to be configured to use BGR666.
> > 
> > Signed-off-by: Markus Niebel <Markus.Niebel@tqs.de>
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > ---
> > This is based on the series: "staging: drm/imx: Add LDB support"
> > by Philipp.
> > 
> > 
> >  drivers/staging/imx-drm/imx-ldb.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c
> > index d8fc93c..84bf7d3 100644
> > --- a/drivers/staging/imx-drm/imx-ldb.c
> > +++ b/drivers/staging/imx-drm/imx-ldb.c
> > @@ -179,6 +179,7 @@ static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
> >  	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
> >  	struct imx_ldb *ldb = imx_ldb_ch->ldb;
> >  	struct drm_display_mode *mode = &encoder->crtc->mode;
> > +	u32 pixel_fmt;
> >  	unsigned long serial_clk;
> >  	unsigned long di_clk = mode->clock * 1000;
> >  	int mux = imx_drm_encoder_get_mux_id(imx_ldb_ch->imx_drm_encoder,
> > @@ -194,8 +195,23 @@ static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
> >  		imx_ldb_set_clock(ldb, mux, imx_ldb_ch->chno, serial_clk, di_clk);
> >  	}
> >  
> > +	switch (imx_ldb_ch->chno) {
> > +	case 0:
> > +		pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
> > +			V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
> > +		break;
> > +	case 1:
> > +		pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH1_24) ?
> > +			V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
> > +		break;
> > +	default:
> > +		dev_err(ldb->dev, "unable to config di%d panel format\n",
> > +			imx_ldb_ch->chno);
> > +		pixel_fmt = V4L2_PIX_FMT_RGB24;
> > +	}
> > +
> >  	imx_drm_crtc_panel_format_pins(encoder->crtc, DRM_MODE_ENCODER_LVDS,
> > -			V4L2_PIX_FMT_RGB24, 2, 3);
> > +			pixel_fmt, 2, 3);
> 
> It seems the patch is based on an updated version of "staging: drm/imx:
> Add LDB support"?  The last one from Philipp [1] calls function
> imx_drm_crtc_panel_format() rather than imx_drm_crtc_panel_format_pins()
> there.
> 

Grmbl! Yes. The last call should be

imx_drm_crtc_panel_format(encoder->crtc, DRM_MODE_ENCODER_LVDS,
			V4L2_PIX_FMT_RGB24);

like it already is in Philipps series.
This patch sits on a topic branch, that (I think) Philipp didn't yet sent out.
I will resent.

Thanks,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] staging: imx_drm: imx_ldb: fix 18 bit format

[Markus Niebel]

Am 08.06.2013 09:25, wrote Steffen Trumtrar:
> Hi Shawn!
> 
> On Sat, Jun 08, 2013 at 12:49:47PM +0800, Shawn Guo wrote:
>> On Fri, Jun 07, 2013 at 10:37:08AM +0200, Steffen Trumtrar wrote:
>>> From: Markus Niebel <Markus.Niebel@tqs.de>
>>>
>>> Since 18 bit is supported as datawidth in device tree it should be
>>> supported in driver. Beside the LDB channel the IPU channel has also
>>> to be configured to use BGR666.
>>>
>>> Signed-off-by: Markus Niebel <Markus.Niebel@tqs.de>
>>> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>>> ---
>>> This is based on the series: "staging: drm/imx: Add LDB support"
>>> by Philipp.
>>>
>>>
>>>  drivers/staging/imx-drm/imx-ldb.c | 18 +++++++++++++++++-
>>>  1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c
>>> index d8fc93c..84bf7d3 100644
>>> --- a/drivers/staging/imx-drm/imx-ldb.c
>>> +++ b/drivers/staging/imx-drm/imx-ldb.c
>>> @@ -179,6 +179,7 @@ static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
>>>  	struct imx_ldb_channel *imx_ldb_ch = enc_to_imx_ldb_ch(encoder);
>>>  	struct imx_ldb *ldb = imx_ldb_ch->ldb;
>>>  	struct drm_display_mode *mode = &encoder->crtc->mode;
>>> +	u32 pixel_fmt;
>>>  	unsigned long serial_clk;
>>>  	unsigned long di_clk = mode->clock * 1000;
>>>  	int mux = imx_drm_encoder_get_mux_id(imx_ldb_ch->imx_drm_encoder,
>>> @@ -194,8 +195,23 @@ static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
>>>  		imx_ldb_set_clock(ldb, mux, imx_ldb_ch->chno, serial_clk, di_clk);
>>>  	}
>>>  
>>> +	switch (imx_ldb_ch->chno) {
>>> +	case 0:
>>> +		pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
>>> +			V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
>>> +		break;
>>> +	case 1:
>>> +		pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH1_24) ?
>>> +			V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
>>> +		break;
>>> +	default:
>>> +		dev_err(ldb->dev, "unable to config di%d panel format\n",
>>> +			imx_ldb_ch->chno);
>>> +		pixel_fmt = V4L2_PIX_FMT_RGB24;
>>> +	}
>>> +
>>>  	imx_drm_crtc_panel_format_pins(encoder->crtc, DRM_MODE_ENCODER_LVDS,
>>> -			V4L2_PIX_FMT_RGB24, 2, 3);
>>> +			pixel_fmt, 2, 3);
>>
>> It seems the patch is based on an updated version of "staging: drm/imx:
>> Add LDB support"?  The last one from Philipp [1] calls function
>> imx_drm_crtc_panel_format() rather than imx_drm_crtc_panel_format_pins()
>> there.
>>
> 
> Grmbl! Yes. The last call should be
> 
> imx_drm_crtc_panel_format(encoder->crtc, DRM_MODE_ENCODER_LVDS,
> 			V4L2_PIX_FMT_RGB24);
> 
> like it already is in Philipps series.
> This patch sits on a topic branch, that (I think) Philipp didn't yet sent out.
> I will resent.

We should call 

imx_drm_crtc_panel_format(encoder->crtc, DRM_MODE_ENCODER_LVDS, pixel_fmt);

Sorry, original patch was created before fixing imx_drm_crtc_panel_format.

> 
> Thanks,
> Steffen
>