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

[Steffen Trumtrar <s.trumtrar@pengutronix.de>]

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

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