diff mbox

[08/10] video: da8xx-fb: obtain fb_videomode info from dt

Message ID 78e026555731e0bbe8475610279d3e5265d124b9.1357304090.git.afzal@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Afzal Mohammed Jan. 7, 2013, 5:11 a.m. UTC
Obtain fb_videomode details for the connected lcd panel using the
display timing details present in DT.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 .../devicetree/bindings/video/fb-da8xx.txt         |   20 ++++++++++++++++++++
 drivers/video/da8xx-fb.c                           |   16 ++++++++++++++++
 2 files changed, 36 insertions(+)

Comments

Steffen Trumtrar Jan. 7, 2013, 9:11 a.m. UTC | #1
Hi!

On Mon, Jan 07, 2013 at 10:41:30AM +0530, Afzal Mohammed wrote:
> Obtain fb_videomode details for the connected lcd panel using the
> display timing details present in DT.
> 
> Signed-off-by: Afzal Mohammed <afzal@ti.com>
> ---
>  .../devicetree/bindings/video/fb-da8xx.txt         |   20 ++++++++++++++++++++
>  drivers/video/da8xx-fb.c                           |   16 ++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/video/fb-da8xx.txt b/Documentation/devicetree/bindings/video/fb-da8xx.txt
> index 581e014..eeb935b 100644
> --- a/Documentation/devicetree/bindings/video/fb-da8xx.txt
> +++ b/Documentation/devicetree/bindings/video/fb-da8xx.txt
> @@ -6,6 +6,11 @@ Required properties:
>  	AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc"
>  - reg: Address range of lcdc register set
>  - interrupts: lcdc interrupt
> +- display-timings: list of different videomodes supported by the lcd
> +  panel, represented as childs, can have multiple modes supported, if
> +  only one, then it is considered native mode, if multiple modes are
> +  provided, native mode can be set explicitly, more details available
> +  @Documentation/devicetree/bindings/video/display-timing.txt
>  

Keep in mind that the text combined with...

>  Example:
>  
> @@ -13,4 +18,19 @@ lcdc@4830e000 {
>  	compatible = "ti,am3352-lcdc", "ti,da830-lcdc";
>  	reg =  <0x4830e000 0x1000>;
>  	interrupts = <36>;
> +	display-timings {
> +		800x480p62 {
> +			clock-frequency = <30000000>;
> +			hactive = <800>;
> +			vactive = <480>;
> +			hfront-porch = <39>;
> +			hback-porch = <39>;
> +			hsync-len = <47>;
> +			vback-porch = <29>;
> +			vfront-porch = <13>;
> +			vsync-len = <2>;
> +			hsync-active = <1>;
> +			vsync-active = <1>;
> +		};
> +	};
>  };
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 68ae925..94add01 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -1261,8 +1261,24 @@ static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
>  {
>  	struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
>  	struct fb_videomode *lcdc_info;
> +	struct device_node *np = dev->dev.of_node;
>  	int i;
>  
> +	if (np) {
> +		lcdc_info = devm_kzalloc(&dev->dev,
> +					 sizeof(struct fb_videomode),
> +					 GFP_KERNEL);
> +		if (!lcdc_info) {
> +			dev_err(&dev->dev, "memory allocation failed\n");
> +			return NULL;
> +		}
> +		if (of_get_fb_videomode(np, lcdc_info, 0)) {
> +			dev_err(&dev->dev, "timings not available in DT\n");
> +			return NULL;
> +		}
> +		return lcdc_info;
> +	}

... this is not correct. You are just supporting the first display-timings
subnode (of_get_fb_videomode(..., 0)).

Regards,
Steffen
Afzal Mohammed Jan. 7, 2013, 9:21 a.m. UTC | #2
Hi Steffen,

On Mon, Jan 07, 2013 at 14:41:31, Steffen Trumtrar wrote:
> On Mon, Jan 07, 2013 at 10:41:30AM +0530, Afzal Mohammed wrote:


> > Obtain fb_videomode details for the connected lcd panel using the

> > display timing details present in DT.


> > +- display-timings: list of different videomodes supported by the lcd

> > +  panel, represented as childs, can have multiple modes supported, if

> > +  only one, then it is considered native mode, if multiple modes are

> > +  provided, native mode can be set explicitly, more details available

> > +  @Documentation/devicetree/bindings/video/display-timing.txt


> Keep in mind that the text combined with...


> > +		if (of_get_fb_videomode(np, lcdc_info, 0)) {

> > +			dev_err(&dev->dev, "timings not available in DT\n");

> > +			return NULL;

> > +		}

> > +		return lcdc_info;

> > +	}

> 

> ... this is not correct. You are just supporting the first display-timings

> subnode (of_get_fb_videomode(..., 0)).



Yes right, I will modify the text to reflect what the driver does.

Regards
Afzal
Afzal Mohammed Jan. 15, 2013, 1:59 p.m. UTC | #3
Hi Steffen,

On Mon, Jan 07, 2013 at 14:51:15, Mohammed, Afzal wrote:
> On Mon, Jan 07, 2013 at 14:41:31, Steffen Trumtrar wrote:

> > On Mon, Jan 07, 2013 at 10:41:30AM +0530, Afzal Mohammed wrote:


> > > +- display-timings: list of different videomodes supported by the lcd

> > > +  panel, represented as childs, can have multiple modes supported, if

> > > +  only one, then it is considered native mode, if multiple modes are

> > > +  provided, native mode can be set explicitly, more details available

> > > +  @Documentation/devicetree/bindings/video/display-timing.txt

> 

> > Keep in mind that the text combined with...

> 

> > > +		if (of_get_fb_videomode(np, lcdc_info, 0)) {

> > > +			dev_err(&dev->dev, "timings not available in DT\n");

> > > +			return NULL;

> > > +		}

> > > +		return lcdc_info;

> > > +	}

> > 

> > ... this is not correct. You are just supporting the first display-timings

> > subnode (of_get_fb_videomode(..., 0)).

> 

> 

> Yes right, I will modify the text to reflect what the driver does.


Thinking about it further, it seems the right thing to do
in this case would be to invoke as,

"of_get_fb_videomode(np, lcd_info, OF_USE_NATIVE_MODE)".

Updated version has been posted to the lists (forgot to cc you)

Regards
Afzal
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/video/fb-da8xx.txt b/Documentation/devicetree/bindings/video/fb-da8xx.txt
index 581e014..eeb935b 100644
--- a/Documentation/devicetree/bindings/video/fb-da8xx.txt
+++ b/Documentation/devicetree/bindings/video/fb-da8xx.txt
@@ -6,6 +6,11 @@  Required properties:
 	AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc"
 - reg: Address range of lcdc register set
 - interrupts: lcdc interrupt
+- display-timings: list of different videomodes supported by the lcd
+  panel, represented as childs, can have multiple modes supported, if
+  only one, then it is considered native mode, if multiple modes are
+  provided, native mode can be set explicitly, more details available
+  @Documentation/devicetree/bindings/video/display-timing.txt
 
 Example:
 
@@ -13,4 +18,19 @@  lcdc@4830e000 {
 	compatible = "ti,am3352-lcdc", "ti,da830-lcdc";
 	reg =  <0x4830e000 0x1000>;
 	interrupts = <36>;
+	display-timings {
+		800x480p62 {
+			clock-frequency = <30000000>;
+			hactive = <800>;
+			vactive = <480>;
+			hfront-porch = <39>;
+			hback-porch = <39>;
+			hsync-len = <47>;
+			vback-porch = <29>;
+			vfront-porch = <13>;
+			vsync-len = <2>;
+			hsync-active = <1>;
+			vsync-active = <1>;
+		};
+	};
 };
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 68ae925..94add01 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1261,8 +1261,24 @@  static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
 {
 	struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
 	struct fb_videomode *lcdc_info;
+	struct device_node *np = dev->dev.of_node;
 	int i;
 
+	if (np) {
+		lcdc_info = devm_kzalloc(&dev->dev,
+					 sizeof(struct fb_videomode),
+					 GFP_KERNEL);
+		if (!lcdc_info) {
+			dev_err(&dev->dev, "memory allocation failed\n");
+			return NULL;
+		}
+		if (of_get_fb_videomode(np, lcdc_info, 0)) {
+			dev_err(&dev->dev, "timings not available in DT\n");
+			return NULL;
+		}
+		return lcdc_info;
+	}
+
 	for (i = 0, lcdc_info = known_lcd_panels;
 		i < ARRAY_SIZE(known_lcd_panels); i++, lcdc_info++) {
 		if (strcmp(fb_pdata->type, lcdc_info->name) == 0)