[PATCH 1/2] video: imxfb: Fix retrieve values from DT

[PATCH 1/2] video: imxfb: Fix retrieve values from DT

[Alexander Shiyan]

Display settings should be retrieved from "display" node, not from
root fb node. This patch fix this bug.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/video/imxfb.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 38733ac..8e104c4 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -753,12 +753,12 @@ static int imxfb_resume(struct platform_device *dev)
 #define imxfb_resume	NULL
 #endif
 
-static int imxfb_init_fbinfo(struct platform_device *pdev)
+static int imxfb_init_fbinfo(struct platform_device *pdev,
+			     struct device_node *np)
 {
 	struct imx_fb_platform_data *pdata = pdev->dev.platform_data;
 	struct fb_info *info = dev_get_drvdata(&pdev->dev);
 	struct imxfb_info *fbi = info->par;
-	struct device_node *np;
 
 	pr_debug("%s\n",__func__);
 
@@ -799,7 +799,6 @@ static int imxfb_init_fbinfo(struct platform_device *pdev)
 		fbi->lcd_power			= pdata->lcd_power;
 		fbi->backlight_power		= pdata->backlight_power;
 	} else {
-		np = pdev->dev.of_node;
 		info->var.grayscale = of_property_read_bool(np,
 						"cmap-greyscale");
 		fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse");
@@ -858,6 +857,7 @@ static int imxfb_of_read_mode(struct device *dev, struct device_node *np,
 
 static int imxfb_probe(struct platform_device *pdev)
 {
+	struct device_node *display_np = NULL;
 	struct imxfb_info *fbi;
 	struct fb_info *info;
 	struct imx_fb_platform_data *pdata;
@@ -887,7 +887,17 @@ static int imxfb_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, info);
 
-	ret = imxfb_init_fbinfo(pdev);
+	if (pdev->dev.of_node) {
+		display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
+		if (!display_np) {
+			dev_err(&pdev->dev,
+				"No display defined in devicetree\n");
+			ret = -EINVAL;
+			goto failed_init;
+		}
+	}
+
+	ret = imxfb_init_fbinfo(pdev, display_np);
 	if (ret < 0)
 		goto failed_init;
 
@@ -898,16 +908,8 @@ static int imxfb_probe(struct platform_device *pdev)
 		fbi->mode = pdata->mode;
 		fbi->num_modes = pdata->num_modes;
 	} else {
-		struct device_node *display_np;
 		fb_mode = NULL;
 
-		display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
-		if (!display_np) {
-			dev_err(&pdev->dev, "No display defined in devicetree\n");
-			ret = -EINVAL;
-			goto failed_of_parse;
-		}
-
 		/*
 		 * imxfb does not support more modes, we choose only the native
 		 * mode.
-- 
1.8.1.5

[PATCH 2/2] video: imxfb: Add feature to setup PWM Contrast Control Register

[Alexander Shiyan]

This patch adds feature to setup PWM Contrast Control Register.
This register is used to control the signal output at the contrast pin,
which controls contrast of the LCD panel.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 Documentation/devicetree/bindings/video/fsl,imx-fb.txt | 1 +
 drivers/video/imxfb.c                                  | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
index 46da08d..be10c65 100644
--- a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
+++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
@@ -18,6 +18,7 @@ Optional properties:
 - fsl,dmacr: DMA Control Register value. This is optional. By default, the
 	register is not modified as recommended by the datasheet.
 - fsl,lscr1: LCDC Sharp Configuration Register value.
+- fsl,pwmr: LCDC PWM Contrast Control Register value.
 
 Example:
 
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 8e104c4..d98299a 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -806,8 +806,8 @@ static int imxfb_init_fbinfo(struct platform_device *pdev,
 
 		fbi->lscr1 = IMXFB_LSCR1_DEFAULT;
 		of_property_read_u32(np, "fsl,lscr1", &fbi->lscr1);
-
 		of_property_read_u32(np, "fsl,dmacr", &fbi->dmacr);
+		of_property_read_u32(np, "fsl,pwmr", &fbi->pwmr);
 
 		/* These two function pointers could be used by some specific
 		 * platforms. */
-- 
1.8.1.5

[PATCH 2/2] video: imxfb: Add feature to setup PWM Contrast Control Register

[Markus Pargmann]

Hi,

On Sun, Jul 21, 2013 at 12:35:09PM +0400, Alexander Shiyan wrote:
> This patch adds feature to setup PWM Contrast Control Register.
> This register is used to control the signal output at the contrast pin,
> which controls contrast of the LCD panel.

http://www.spinics.net/lists/linux-fbdev/msg10002.html

Regards,

Markus

> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  Documentation/devicetree/bindings/video/fsl,imx-fb.txt | 1 +
>  drivers/video/imxfb.c                                  | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> index 46da08d..be10c65 100644
> --- a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> @@ -18,6 +18,7 @@ Optional properties:
>  - fsl,dmacr: DMA Control Register value. This is optional. By default, the
>  	register is not modified as recommended by the datasheet.
>  - fsl,lscr1: LCDC Sharp Configuration Register value.
> +- fsl,pwmr: LCDC PWM Contrast Control Register value.
>  
>  Example:
>  
> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index 8e104c4..d98299a 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -806,8 +806,8 @@ static int imxfb_init_fbinfo(struct platform_device *pdev,
>  
>  		fbi->lscr1 = IMXFB_LSCR1_DEFAULT;
>  		of_property_read_u32(np, "fsl,lscr1", &fbi->lscr1);
> -
>  		of_property_read_u32(np, "fsl,dmacr", &fbi->dmacr);
> +		of_property_read_u32(np, "fsl,pwmr", &fbi->pwmr);
>  
>  		/* These two function pointers could be used by some specific
>  		 * platforms. */
> -- 
> 1.8.1.5
> 
> 

-- 
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 |

[PATCH 2/2] video: imxfb: Add feature to setup PWM Contrast Control Register

[Alexander Shiyan]

On Thu, 25 Jul 2013 15:38:56 +0200
Markus Pargmann <mpa@pengutronix.de> wrote:

> Hi,
> 
> On Sun, Jul 21, 2013 at 12:35:09PM +0400, Alexander Shiyan wrote:
> > This patch adds feature to setup PWM Contrast Control Register.
> > This register is used to control the signal output at the contrast pin,
> > which controls contrast of the LCD panel.
> 
> http://www.spinics.net/lists/linux-fbdev/msg10002.html

PWM? I can not understand how the PWM driver will be connected to the
framebuffer driver. It is not backlight, this is contrast.
Even if we imagine that the driver will be connected through phandle,
then we will need to have at least 4! additional parameters in the framebuffer
driver: freq source, frequency and active pulse period + phandle to PWM.
Is it worth it, given that these parameters should not be adjustable?
On my opinion, the only one additional parameter in framebuffer is enough.
Thanks.

[...]
> > ---
> >  Documentation/devicetree/bindings/video/fsl,imx-fb.txt | 1 +
> >  drivers/video/imxfb.c                                  | 2 +-
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > index 46da08d..be10c65 100644
> > --- a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > @@ -18,6 +18,7 @@ Optional properties:
> >  - fsl,dmacr: DMA Control Register value. This is optional. By default, the
> >  	register is not modified as recommended by the datasheet.
> >  - fsl,lscr1: LCDC Sharp Configuration Register value.
> > +- fsl,pwmr: LCDC PWM Contrast Control Register value.
> >  
> >  Example:
> >  
> > diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> > index 8e104c4..d98299a 100644
> > --- a/drivers/video/imxfb.c
> > +++ b/drivers/video/imxfb.c
> > @@ -806,8 +806,8 @@ static int imxfb_init_fbinfo(struct platform_device *pdev,
> >  
> >  		fbi->lscr1 = IMXFB_LSCR1_DEFAULT;
> >  		of_property_read_u32(np, "fsl,lscr1", &fbi->lscr1);
> > -
> >  		of_property_read_u32(np, "fsl,dmacr", &fbi->dmacr);
> > +		of_property_read_u32(np, "fsl,pwmr", &fbi->pwmr);
> >  
> >  		/* These two function pointers could be used by some specific
> >  		 * platforms. */
> > -- 

-- 
Alexander Shiyan <shc_work@mail.ru>

[PATCH 2/2] video: imxfb: Add feature to setup PWM Contrast Control Register

[Sascha Hauer]

On Thu, Jul 25, 2013 at 09:20:53PM +0400, Alexander Shiyan wrote:
> On Thu, 25 Jul 2013 15:38:56 +0200
> Markus Pargmann <mpa@pengutronix.de> wrote:
> 
> > Hi,
> > 
> > On Sun, Jul 21, 2013 at 12:35:09PM +0400, Alexander Shiyan wrote:
> > > This patch adds feature to setup PWM Contrast Control Register.
> > > This register is used to control the signal output at the contrast pin,
> > > which controls contrast of the LCD panel.
> > 
> > http://www.spinics.net/lists/linux-fbdev/msg10002.html
> 
> PWM? I can not understand how the PWM driver will be connected to the
> framebuffer driver. It is not backlight, this is contrast.
> Even if we imagine that the driver will be connected through phandle,
> then we will need to have at least 4! additional parameters in the framebuffer
> driver: freq source, frequency and active pulse period + phandle to PWM.
> Is it worth it, given that these parameters should not be adjustable?
> On my opinion, the only one additional parameter in framebuffer is enough.
> Thanks.

Why should the contrast of a display not be adjustable?

Sascha

-- 
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 2/2] video: imxfb: Add feature to setup PWM Contrast Control Register

[Alexander Shiyan]

> On Thu, Jul 25, 2013 at 09:20:53PM +0400, Alexander Shiyan wrote:
> > On Thu, 25 Jul 2013 15:38:56 +0200
> > Markus Pargmann <mpa@pengutronix.de> wrote:
> > 
> > > Hi,
> > > 
> > > On Sun, Jul 21, 2013 at 12:35:09PM +0400, Alexander Shiyan wrote:
> > > > This patch adds feature to setup PWM Contrast Control Register.
> > > > This register is used to control the signal output at the contrast pin,
> > > > which controls contrast of the LCD panel.
> > > 
> > > http://www.spinics.net/lists/linux-fbdev/msg10002.html
> > 
> > PWM? I can not understand how the PWM driver will be connected to the
> > framebuffer driver. It is not backlight, this is contrast.
> > Even if we imagine that the driver will be connected through phandle,
> > then we will need to have at least 4! additional parameters in the framebuffer
> > driver: freq source, frequency and active pulse period + phandle to PWM.
> > Is it worth it, given that these parameters should not be adjustable?
> > On my opinion, the only one additional parameter in framebuffer is enough.
> > Thanks.
> 
> Why should the contrast of a display not be adjustable?

I still can not understand the interface through which it can be changed.
Same as how the default settings can be passed to the framebuffer driver.

---