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

2013-01-15 Thread Mohammed, Afzal 
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

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

2013-01-07 Thread Steffen Trumtrar 
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 af...@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 = 3000;
 + 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- |
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-01-07 Thread Mohammed, Afzal 
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