* [RFC 0/1] Adding DT support to video/da8xx-fb.c @ 2013-08-08 20:15 Darren Etheridge 2013-08-08 20:15 ` [RFC 1/1] video: da8xx-fb: adding dt support Darren Etheridge 2013-08-09 17:55 ` [RFC 0/1] Adding DT support to video/da8xx-fb.c Tomi Valkeinen 0 siblings, 2 replies; 6+ messages in thread From: Darren Etheridge @ 2013-08-08 20:15 UTC (permalink / raw) To: devicetree, tomi.valkeinen, plagnioj, linux-fbdev, detheridge; +Cc: afzal This is part of a larger series of patches to upgrade the da8xx-fb.c driver to support the Texas Instruments AM335x device. As part of this upgrade we also want to add devicetree support for both the original da8xx and the am335x. Tomi Valkeinen has reviewed the fbdev changes but he suggested that it was prudent to extract the dt pieces and run it through the devicetree mailing list for review. Thanks, Darren Darren Etheridge (1): video: da8xx-fb: adding dt support .../devicetree/bindings/video/fb-da8xx.txt | 37 +++++++++++ drivers/video/da8xx-fb.c | 67 +++++++++++++++++++- 2 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC 1/1] video: da8xx-fb: adding dt support 2013-08-08 20:15 [RFC 0/1] Adding DT support to video/da8xx-fb.c Darren Etheridge @ 2013-08-08 20:15 ` Darren Etheridge 2013-08-09 4:07 ` Prabhakar Lad 2013-08-09 17:55 ` [RFC 0/1] Adding DT support to video/da8xx-fb.c Tomi Valkeinen 1 sibling, 1 reply; 6+ messages in thread From: Darren Etheridge @ 2013-08-08 20:15 UTC (permalink / raw) To: devicetree, tomi.valkeinen, plagnioj, linux-fbdev, detheridge; +Cc: afzal Enhancing driver to enable probe triggered by a corresponding dt entry. Add da8xx-fb.txt documentation to devicetree section. Obtain fb_videomode details for the connected lcd panel using the display timing details present in DT. Ensure that platform data is present before checking whether platform callback is present (the one used to control backlight). So far this was not an issue as driver was purely non-DT triggered, but now DT support has been added this check must be performed. v2: squashing multiple commits from Afzal Mohammed (afzal@ti.com) v3: remove superfluous cast v4: expose both ti,am3352-lcdc and ti,da830-lcdc for .compatible as driver can use enhanced features of all version of the silicon block. Signed-off-by: Darren Etheridge <detheridge@ti.com> --- .../devicetree/bindings/video/fb-da8xx.txt | 37 +++++++++++ drivers/video/da8xx-fb.c | 67 +++++++++++++++++++- 2 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt diff --git a/Documentation/devicetree/bindings/video/fb-da8xx.txt b/Documentation/devicetree/bindings/video/fb-da8xx.txt new file mode 100644 index 0000000..a6cfe3c --- /dev/null +++ b/Documentation/devicetree/bindings/video/fb-da8xx.txt @@ -0,0 +1,37 @@ +TI LCD Controller on DA830/DA850/AM335x SoC's + +Required properties: +- compatible: + DA830 - "ti,da830-lcdc" + AM335x SoC's - "ti,am3352-lcdc" +- reg: Address range of lcdc register set +- interrupts: lcdc interrupt +- display-timings: typical videomode of lcd panel, represented as child. + Refer Documentation/devicetree/bindings/video/display-timing.txt for + display timing binding details. If multiple videomodes are mentioned + in display timings node, typical videomode has to be mentioned as the + native mode or it has to be first child (driver cares only for native + videomode). + +Example: + +lcdc@4830e000 { + compatible = "ti,am3352-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 ea9771f..64fd8af 100644 --- a/drivers/video/da8xx-fb.c +++ b/drivers/video/da8xx-fb.c @@ -36,6 +36,7 @@ #include <linux/slab.h> #include <linux/delay.h> #include <linux/lcm.h> +#include <video/of_display_timing.h> #include <video/da8xx-fb.h> #include <asm/div64.h> @@ -1297,12 +1298,57 @@ static struct fb_ops da8xx_fb_ops = { .fb_blank = cfb_blank, }; +static struct lcd_ctrl_config *da8xx_fb_create_cfg(struct platform_device *dev) +{ + struct lcd_ctrl_config *cfg; + + cfg = devm_kzalloc(&dev->dev, sizeof(struct fb_videomode), GFP_KERNEL); + if (!cfg) { + dev_err(&dev->dev, "memory allocation failed\n"); + return NULL; + } + + /* default values */ + + if (lcd_revision == LCD_VERSION_1) + cfg->bpp = 16; + else + cfg->bpp = 32; + + /* + * For panels so far used with this LCDC, below statement is sufficient. + * For new panels, if required, struct lcd_ctrl_cfg fields to be updated + * with additional/modified values. Those values would have to be then + * obtained from dt(requiring new dt bindings). + */ + + cfg->panel_shade = COLOR_ACTIVE; + + return cfg; +} + 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, OF_USE_NATIVE_MODE)) { + 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) @@ -1331,7 +1377,7 @@ static int fb_probe(struct platform_device *device) int ret; unsigned long ulcm; - if (fb_pdata == NULL) { + if (fb_pdata == NULL && !device->dev.of_node) { dev_err(&device->dev, "Can not get platform data\n"); return -ENOENT; } @@ -1371,7 +1417,10 @@ static int fb_probe(struct platform_device *device) break; } - lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data; + if (device->dev.of_node) + lcd_cfg = da8xx_fb_create_cfg(device); + else + lcd_cfg = fb_pdata->controller_data; if (!lcd_cfg) { ret = -EINVAL; @@ -1390,7 +1439,7 @@ static int fb_probe(struct platform_device *device) par->dev = &device->dev; par->lcdc_clk = tmp_lcdc_clk; par->lcdc_clk_rate = clk_get_rate(par->lcdc_clk); - if (fb_pdata->panel_power_ctrl) { + if (fb_pdata && fb_pdata->panel_power_ctrl) { par->panel_power_ctrl = fb_pdata->panel_power_ctrl; par->panel_power_ctrl(1); } @@ -1638,6 +1687,17 @@ static int fb_resume(struct platform_device *dev) #define fb_resume NULL #endif +static const struct of_device_id da8xx_fb_of_match[] = { + /* + * this driver supports version 1 and version 2 of the + * Texas Instruments lcd controller (lcdc) hardware block + */ + {.compatible = "ti,da830-lcdc", }, + {.compatible = "ti,am3352-lcdc", }, + {}, +}; +MODULE_DEVICE_TABLE(of, da8xx_fb_of_match); + static struct platform_driver da8xx_fb_driver = { .probe = fb_probe, .remove = fb_remove, @@ -1646,6 +1706,7 @@ static struct platform_driver da8xx_fb_driver = { .driver = { .name = DRIVER_NAME, .owner = THIS_MODULE, + .of_match_table = of_match_ptr(da8xx_fb_of_match), }, }; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC 1/1] video: da8xx-fb: adding dt support 2013-08-08 20:15 ` [RFC 1/1] video: da8xx-fb: adding dt support Darren Etheridge @ 2013-08-09 4:07 ` Prabhakar Lad 2013-08-16 20:11 ` Etheridge, Darren 0 siblings, 1 reply; 6+ messages in thread From: Prabhakar Lad @ 2013-08-09 4:07 UTC (permalink / raw) To: Darren Etheridge; +Cc: devicetree, tomi.valkeinen, plagnioj, linux-fbdev, afzal Hi Darren, Thanks for the patch, below are few nits! On Fri, Aug 9, 2013 at 1:45 AM, Darren Etheridge <detheridge@ti.com> wrote: > Enhancing driver to enable probe triggered by a corresponding dt entry. > > Add da8xx-fb.txt documentation to devicetree section. > > Obtain fb_videomode details for the connected lcd panel using the > display timing details present in DT. > > Ensure that platform data is present before checking whether platform > callback is present (the one used to control backlight). So far this > was not an issue as driver was purely non-DT triggered, but now DT > support has been added this check must be performed. > > v2: squashing multiple commits from Afzal Mohammed (afzal@ti.com) > v3: remove superfluous cast > v4: expose both ti,am3352-lcdc and ti,da830-lcdc for .compatible > as driver can use enhanced features of all version of the > silicon block. > > Signed-off-by: Darren Etheridge <detheridge@ti.com> > --- > .../devicetree/bindings/video/fb-da8xx.txt | 37 +++++++++++ > drivers/video/da8xx-fb.c | 67 +++++++++++++++++++- > 2 files changed, 101 insertions(+), 3 deletions(-) > create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt > > diff --git a/Documentation/devicetree/bindings/video/fb-da8xx.txt b/Documentation/devicetree/bindings/video/fb-da8xx.txt > new file mode 100644 > index 0000000..a6cfe3c > --- /dev/null > +++ b/Documentation/devicetree/bindings/video/fb-da8xx.txt > @@ -0,0 +1,37 @@ > +TI LCD Controller on DA830/DA850/AM335x SoC's > + > +Required properties: > +- compatible: > + DA830 - "ti,da830-lcdc" > + AM335x SoC's - "ti,am3352-lcdc" > +- reg: Address range of lcdc register set > +- interrupts: lcdc interrupt > +- display-timings: typical videomode of lcd panel, represented as child. > + Refer Documentation/devicetree/bindings/video/display-timing.txt for > + display timing binding details. If multiple videomodes are mentioned > + in display timings node, typical videomode has to be mentioned as the > + native mode or it has to be first child (driver cares only for native > + videomode). > + > +Example: > + > +lcdc@4830e000 { > + compatible = "ti,am3352-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 ea9771f..64fd8af 100644 > --- a/drivers/video/da8xx-fb.c > +++ b/drivers/video/da8xx-fb.c > @@ -36,6 +36,7 @@ > #include <linux/slab.h> > #include <linux/delay.h> > #include <linux/lcm.h> > +#include <video/of_display_timing.h> > #include <video/da8xx-fb.h> > #include <asm/div64.h> > > @@ -1297,12 +1298,57 @@ static struct fb_ops da8xx_fb_ops = { > .fb_blank = cfb_blank, > }; > > +static struct lcd_ctrl_config *da8xx_fb_create_cfg(struct platform_device *dev) > +{ > + struct lcd_ctrl_config *cfg; > + > + cfg = devm_kzalloc(&dev->dev, sizeof(struct fb_videomode), GFP_KERNEL); > + if (!cfg) { > + dev_err(&dev->dev, "memory allocation failed\n"); devm_* api will print the message if allocation has failed, please remove the above err statement and also similarly below. > + return NULL; > + } > + > + /* default values */ > + > + if (lcd_revision == LCD_VERSION_1) > + cfg->bpp = 16; > + else > + cfg->bpp = 32; > + > + /* > + * For panels so far used with this LCDC, below statement is sufficient. > + * For new panels, if required, struct lcd_ctrl_cfg fields to be updated > + * with additional/modified values. Those values would have to be then > + * obtained from dt(requiring new dt bindings). > + */ > + > + cfg->panel_shade = COLOR_ACTIVE; > + > + return cfg; > +} > + > 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"); ditto > + return NULL; > + } > + if (of_get_fb_videomode(np, lcdc_info, OF_USE_NATIVE_MODE)) { > + 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) > @@ -1331,7 +1377,7 @@ static int fb_probe(struct platform_device *device) > int ret; > unsigned long ulcm; > > - if (fb_pdata == NULL) { > + if (fb_pdata == NULL && !device->dev.of_node) { > dev_err(&device->dev, "Can not get platform data\n"); > return -ENOENT; > } > @@ -1371,7 +1417,10 @@ static int fb_probe(struct platform_device *device) > break; > } > > - lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data; > + if (device->dev.of_node) > + lcd_cfg = da8xx_fb_create_cfg(device); > + else > + lcd_cfg = fb_pdata->controller_data; > > if (!lcd_cfg) { > ret = -EINVAL; > @@ -1390,7 +1439,7 @@ static int fb_probe(struct platform_device *device) > par->dev = &device->dev; > par->lcdc_clk = tmp_lcdc_clk; > par->lcdc_clk_rate = clk_get_rate(par->lcdc_clk); > - if (fb_pdata->panel_power_ctrl) { > + if (fb_pdata && fb_pdata->panel_power_ctrl) { > par->panel_power_ctrl = fb_pdata->panel_power_ctrl; > par->panel_power_ctrl(1); What happens to this data in DT case ? > } > @@ -1638,6 +1687,17 @@ static int fb_resume(struct platform_device *dev) > #define fb_resume NULL > #endif > > +static const struct of_device_id da8xx_fb_of_match[] = { > + /* > + * this driver supports version 1 and version 2 of the > + * Texas Instruments lcd controller (lcdc) hardware block > + */ > + {.compatible = "ti,da830-lcdc", }, > + {.compatible = "ti,am3352-lcdc", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, da8xx_fb_of_match); > + you can bound this structure and the macro in IS_ENABLED(CONFIG_OF) Regards, --Prabhakar Lad ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [RFC 1/1] video: da8xx-fb: adding dt support 2013-08-09 4:07 ` Prabhakar Lad @ 2013-08-16 20:11 ` Etheridge, Darren 0 siblings, 0 replies; 6+ messages in thread From: Etheridge, Darren @ 2013-08-16 20:11 UTC (permalink / raw) To: Prabhakar Lad Cc: devicetree@vger.kernel.org, Valkeinen, Tomi, plagnioj@jcrosoft.com, linux-fbdev@vger.kernel.org, Mohammed, Afzal From: Prabhakar Lad [mailto:prabhakar.csengg@gmail.com] > Sent: Thursday, August 08, 2013 11:07 PM > Subject: Re: [RFC 1/1] video: da8xx-fb: adding dt support > > Hi Darren, > > Thanks for the patch, below are few nits! > Prabhakar, Thanks for reviewing, somehow I missed your reply so sorry about late response. > On Fri, Aug 9, 2013 at 1:45 AM, Darren Etheridge <detheridge@ti.com> > wrote: > > Enhancing driver to enable probe triggered by a corresponding dt entry. > > <snip> > > + > > + cfg = devm_kzalloc(&dev->dev, sizeof(struct fb_videomode), > GFP_KERNEL); > > + if (!cfg) { > > + dev_err(&dev->dev, "memory allocation failed\n"); > > devm_* api will print the message if allocation has failed, please remove the > above err statement and also similarly below. Will fix. <snip> > > + lcdc_info = devm_kzalloc(&dev->dev, > > + sizeof(struct fb_videomode), > > + GFP_KERNEL); > > + if (!lcdc_info) { > > + dev_err(&dev->dev, "memory allocation > > + failed\n"); > ditto Will fix <snip> > > @@ -1390,7 +1439,7 @@ static int fb_probe(struct platform_device > *device) > > par->dev = &device->dev; > > par->lcdc_clk = tmp_lcdc_clk; > > par->lcdc_clk_rate = clk_get_rate(par->lcdc_clk); > > - if (fb_pdata->panel_power_ctrl) { > > + if (fb_pdata && fb_pdata->panel_power_ctrl) { > > par->panel_power_ctrl = fb_pdata->panel_power_ctrl; > > par->panel_power_ctrl(1); > > What happens to this data in DT case ? > Good find, right now we don't do any power control in the DT case - this is something that needs to be addressed. > > > } > > @@ -1638,6 +1687,17 @@ static int fb_resume(struct platform_device > > *dev) #define fb_resume NULL #endif > > > > +static const struct of_device_id da8xx_fb_of_match[] = { > > + /* > > + * this driver supports version 1 and version 2 of the > > + * Texas Instruments lcd controller (lcdc) hardware block > > + */ > > + {.compatible = "ti,da830-lcdc", }, > > + {.compatible = "ti,am3352-lcdc", }, > > + {}, > > +}; > > +MODULE_DEVICE_TABLE(of, da8xx_fb_of_match); > > + > you can bound this structure and the macro in IS_ENABLED(CONFIG_OF) Ok will look at that. Thanks, Darren ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC 0/1] Adding DT support to video/da8xx-fb.c 2013-08-08 20:15 [RFC 0/1] Adding DT support to video/da8xx-fb.c Darren Etheridge 2013-08-08 20:15 ` [RFC 1/1] video: da8xx-fb: adding dt support Darren Etheridge @ 2013-08-09 17:55 ` Tomi Valkeinen 2013-08-12 19:11 ` Darren Etheridge 1 sibling, 1 reply; 6+ messages in thread From: Tomi Valkeinen @ 2013-08-09 17:55 UTC (permalink / raw) To: Darren Etheridge; +Cc: devicetree, plagnioj, linux-fbdev, afzal [-- Attachment #1: Type: text/plain, Size: 1144 bytes --] Hi, On 08/08/13 23:15, Darren Etheridge wrote: > This is part of a larger series of patches to upgrade the da8xx-fb.c driver > to support the Texas Instruments AM335x device. As part of this upgrade > we also want to add devicetree support for both the original da8xx and the > am335x. Tomi Valkeinen has reviewed the fbdev changes but he suggested > that it was prudent to extract the dt pieces and run it through the > devicetree mailing list for review. > > Thanks, > Darren > > Darren Etheridge (1): > video: da8xx-fb: adding dt support > > .../devicetree/bindings/video/fb-da8xx.txt | 37 +++++++++++ > drivers/video/da8xx-fb.c | 67 +++++++++++++++++++- > 2 files changed, 101 insertions(+), 3 deletions(-) > create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt I just realized something. We have both drm and fb drivers for the LCDC IP. The hardware is the same, thus, there should only be one set of DT bindings, used by both drivers. I don't know why this didn't occur to me earlier, as it's the same situation with OMAP DSS. Tomi [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 901 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC 0/1] Adding DT support to video/da8xx-fb.c 2013-08-09 17:55 ` [RFC 0/1] Adding DT support to video/da8xx-fb.c Tomi Valkeinen @ 2013-08-12 19:11 ` Darren Etheridge 0 siblings, 0 replies; 6+ messages in thread From: Darren Etheridge @ 2013-08-12 19:11 UTC (permalink / raw) To: Tomi Valkeinen; +Cc: devicetree, plagnioj, linux-fbdev, afzal Tomi Valkeinen <tomi.valkeinen@ti.com> wrote on Fri [2013-Aug-09 20:55:28 +0300]: > Hi, > > On 08/08/13 23:15, Darren Etheridge wrote: > > This is part of a larger series of patches to upgrade the da8xx-fb.c driver > > to support the Texas Instruments AM335x device. As part of this upgrade > > we also want to add devicetree support for both the original da8xx and the > > am335x. Tomi Valkeinen has reviewed the fbdev changes but he suggested > > that it was prudent to extract the dt pieces and run it through the > > devicetree mailing list for review. > > > > Thanks, > > Darren > > > > Darren Etheridge (1): > > video: da8xx-fb: adding dt support > > > > .../devicetree/bindings/video/fb-da8xx.txt | 37 +++++++++++ > > drivers/video/da8xx-fb.c | 67 +++++++++++++++++++- > > 2 files changed, 101 insertions(+), 3 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt > > I just realized something. We have both drm and fb drivers for the LCDC > IP. The hardware is the same, thus, there should only be one set of DT > bindings, used by both drivers. > > I don't know why this didn't occur to me earlier, as it's the same > situation with OMAP DSS. > OK that makes sense. So in this case what I need to do is change the .compatible to match the gpu/drm/driver/tilcdc driver which has the existing binding so it would become ti,am33xx-tilcdc instead of ti,am3352-lcdc. This fbdev driver also supports da8xx class devices so I would just rename this second .compatible to be ti,da8xx-tilcdc to be consistent with the other. The fbdev driver doesn't support some of the options that the tilcdc/drm driver supports, but that shouldn't be a problem for the fbdev driver. I will also change the name of the documentation file to be da8xx-fb.txt to match the driver name. Darren ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-08-16 22:09 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-08-08 20:15 [RFC 0/1] Adding DT support to video/da8xx-fb.c Darren Etheridge 2013-08-08 20:15 ` [RFC 1/1] video: da8xx-fb: adding dt support Darren Etheridge 2013-08-09 4:07 ` Prabhakar Lad 2013-08-16 20:11 ` Etheridge, Darren 2013-08-09 17:55 ` [RFC 0/1] Adding DT support to video/da8xx-fb.c Tomi Valkeinen 2013-08-12 19:11 ` Darren Etheridge
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).