video: da8xx-fb: DT support
Subsystems (2)
v2 cover video: da8xx-fb: DT support
Hi,
This series adds DT support to da8xx-fb driver (device found on DaVinci and AM335x SoC's). It does certain cleanup's in the process.
This series as compared to previous version handles configuration of the LCDC input clock to required value if clock divider in IP cannot take care of required pixel clock. In the case of AM335x (where there is requirement of configuring input clock), it's clock tree has to be updated with proper flags, relevant patch is,
"ARM: AM33XX: clock: SET_RATE_PARENT in lcd path".
There is an additional patch in this new series to make io operations safe.
This makes use of Steffen Trumtrar's v16 of display timing DT support.
Testing has been done on AM335x SoC based boards like AM335x EVM and AM335x EVM-SK. It has also been verified that display on DA850 EVM (non-DT boot) works as earlier.
This series is based on v3.8-rc3,
and is dependent on,
1. Series v16 "of: add display helper" by,
Steffen Trumtrar <[email protected]>
2. Patch "da8xx: Allow use by am33xx based devices" by, Pantelis Antoniou <[email protected]>
3. Series v3 "video: da8xx-fb: runtime timing configuration" by, me (Afzal Mohammed <[email protected]>)
To test this series on AM335x based boards,
1. Series v2 "ARM: dts: AM33XX: lcdc support" by,
me (Afzal Mohammed <[email protected]>),
2. Patch "ARM: AM33XX: clock: SET_RATE_PARENT in lcd path" by, me (Afzal Mohammed <[email protected]>),
3. Series "HWMOD fixes for AM33xx PWM submodules and device tree nodes" by, Philip, Avinash <[email protected]>
would be needed.
All above dependencies along with those required for testing is available
@git://gitorious.org/x0148406-public/linux-kernel.git tags/da8xx-fb-dt-v3.8-rc3
Regards
Afzal
v2: 2 new patches - one to configure clock rate properly (12/12)and
other to make io operations safe (1/12)
Afzal Mohammed (11):
video: da8xx-fb: make io operations safe
video: da8xx-fb: enable sync lost intr for v2 ip
video: da8xx-fb: use devres
video: da8xx-fb: ensure non-null cfg in pdata
video: da8xx-fb: reorganize panel detection
video: da8xx-fb: minimal dt support
video: da8xx-fb: invoke platform callback safely
video: da8xx-fb: obtain fb_videomode info from dt
video: da8xx-fb: ensure pdata only for non-dt
video: da8xx-fb: setup struct lcd_ctrl_config for dt
video: da8xx-fb: set upstream clock rate (if reqd)
Manjunathappa, Prakash (1):
video: da8xx-fb: fix 24bpp raster configuration
.../devicetree/bindings/video/fb-da8xx.txt | 37 ++++ drivers/video/da8xx-fb.c | 226 ++++++++++++++------ 2 files changed, 194 insertions(+), 69 deletions(-) create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt
Signature · 1 line
1.7.9.5
PATCH v2 1/12 video: da8xx-fb: make io operations safe
Replace __raw_readl/__raw_writel with readl/writel; this driver is reused on ARMv7 (AM335x SoC).
---
v2: new patch
drivers/video/da8xx-fb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/video/da8xx-fb.c +2 -2
@@ -141,12 +141,12 @@ static int frame_done_flag;static inline unsigned int lcdc_read (unsigned int addr) { -return (unsigned int )__raw_readl (da8xx_fb_reg_base + (addr)); +return (unsigned int )readl (da8xx_fb_reg_base + (addr)); }static inline void lcdc_write (unsigned int val,unsigned int addr) { -__raw_writel (val, da8xx_fb_reg_base + (addr)); +writel (val, da8xx_fb_reg_base + (addr)); }struct da8xx_fb_par {
Signature · 1 line
1.7.9.5
PATCH v2 2/12 video: da8xx-fb: fix 24bpp raster configuration
In reply to Afzal MohammedFrom: "Manjunathappa, Prakash" <[email protected]>
Set only LCD_V2_TFT_24BPP_MODE bit for 24bpp and LCD_V2_TFT_24BPP_UNPACK bit along with LCD_V2_TFT_24BPP_MODE for 32bpp configuration.
Patch is tested on am335x-evm for 24bpp and da850-evm for 16bpp configurations.
--- drivers/video/da8xx-fb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/video/da8xx-fb.c +2 -2
@@ -550,10 +550,10 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,case 4 :case 16 :break ; -case 24 : - reg |=LCD_V2_TFT_24BPP_MODE ;case 32 : reg |=LCD_V2_TFT_24BPP_UNPACK ; +case 24 : + reg |=LCD_V2_TFT_24BPP_MODE ;break ;case 8 :
Signature · 1 line
1.7.9.5
PATCH v2 3/12 video: da8xx-fb: enable sync lost intr for v2 ip
In reply to Afzal Mohammedinterrupt handler is checking for sync lost interrupt, but it was not enabled, enable it.
--- drivers/video/da8xx-fb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/video/da8xx-fb.c +1 -1
@@ -318,7 +318,7 @@ static void lcd_blit(int load_mode, struct da8xx_fb_par *par) reg_int =lcdc_read (LCD_INT_ENABLE_SET_REG ) |LCD_V2_END_OF_FRAME0_INT_ENA |LCD_V2_END_OF_FRAME1_INT_ENA | -LCD_FRAME_DONE ; +LCD_FRAME_DONE |LCD_SYNC_LOST ;lcdc_write (reg_int,LCD_INT_ENABLE_SET_REG ); } reg_dma |=LCD_DUAL_FRAME_BUFFER_ENABLE ;
Signature · 1 line
1.7.9.5
PATCH v2 5/12 video: da8xx-fb: ensure non-null cfg in pdata
In reply to Afzal MohammedEnsure that platform data contains pointer for lcd_ctrl_config.
--- drivers/video/da8xx-fb.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/video/da8xx-fb.c +5 -0
@@ -1320,6 +1320,11 @@ static int fb_probe(struct platform_device *device) lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data; +if (!lcd_cfg) { + ret = -EINVAL ; + goto err_pm_runtime_disable; + } + da8xx_fb_info =framebuffer_alloc (sizeof (struct da8xx_fb_par ), &device->dev);if (!da8xx_fb_info) {
Signature · 1 line
1.7.9.5
PATCH v2 7/12 video: da8xx-fb: minimal dt support
In reply to Afzal MohammedDriver is provided a means to have the probe triggered by DT.
--- .../devicetree/bindings/video/fb-da8xx.txt | 16 ++++++++++++++++ drivers/video/da8xx-fb.c | 7 +++++++ 2 files changed, 23 insertions(+) create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt
Documentation/devicetree/bindings/video/fb-da8xx.txt new file +16 -0
@@ -0,0 +1,16 @@ +TI LCD Controller on DA830/DA850/AM335x SoC's + +Required properties: +- compatible: + DA830 - "ti,da830-lcdc" + AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc" +- reg: Address range of lcdc register set +- interrupts: lcdc interrupt + +Example: + +lcdc@4830e000 { + compatible = "ti,am3352-lcdc", "ti,da830-lcdc"; + reg = <0x4830e000 0x1000>; + interrupts = <36>; +};
drivers/video/da8xx-fb.c +7 -0
@@ -1595,6 +1595,12 @@ static int fb_resume(struct platform_device *dev)#define fb_resume NULL#endif +static const struct of_device_id da8xx_fb_of_match[] = { + {.compatible ="ti,da830-lcdc" , }, + {}, +}; +MODULE_DEVICE_TABLE (of, da8xx_fb_of_match); +static struct platform_driver da8xx_fb_driver = { .probe = fb_probe, .remove = fb_remove,
@@ -1603,6 +1609,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), }, };
Signature · 1 line
1.7.9.5
PATCH v2 8/12 video: da8xx-fb: invoke platform callback safely
In reply to Afzal MohammedEnsure 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.
--- drivers/video/da8xx-fb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/video/da8xx-fb.c +1 -1
@@ -1347,7 +1347,7 @@ static int fb_probe(struct platform_device *device) par->dev = &device->dev; par->lcdc_clk = fb_clk; par->lcd_fck_rate =clk_get_rate (fb_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 ); }
Signature · 1 line
1.7.9.5
PATCH v2 9/12 video: da8xx-fb: obtain fb_videomode info from dt
In reply to Afzal MohammedObtain fb_videomode details for the connected lcd panel using the display timing details present in DT.
--- .../devicetree/bindings/video/fb-da8xx.txt | 21 ++++++++++++++++++++ drivers/video/da8xx-fb.c | 17 ++++++++++++++++ 2 files changed, 38 insertions(+)
Documentation/devicetree/bindings/video/fb-da8xx.txt +21 -0
@@ -6,6 +6,12 @@ Required properties: AM335x SoC's - "ti,am3352-lcdc", "ti,da830-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:
@@ -13,4 +19,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>; + }; + }; };
drivers/video/da8xx-fb.c +17 -0
@@ -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>
@@ -1257,8 +1258,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,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 )
Signature · 1 line
1.7.9.5
PATCH v2 10/12 video: da8xx-fb: ensure pdata only for non-dt
In reply to Afzal MohammedThis driver is DT probe-able, hence ensure presence of platform data only for non-DT boot.
--- drivers/video/da8xx-fb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/video/da8xx-fb.c +1 -1
@@ -1303,7 +1303,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 ; }
Signature · 1 line
1.7.9.5
PATCH v2 4/12 video: da8xx-fb: use devres
In reply to Afzal MohammedReplace existing resource handling in the driver with managed device resource.
--- drivers/video/da8xx-fb.c | 35 ++++++----------------------------- 1 file changed, 6 insertions(+), 29 deletions(-)
drivers/video/da8xx-fb.c +6 -29
@@ -1036,12 +1036,9 @@ static int fb_remove(struct platform_device *dev) par->p_palette_base);dma_free_coherent (NULL , par->vram_size, par->vram_virt, par->vram_phys); -free_irq (par->irq, par);pm_runtime_put_sync (&dev->dev);pm_runtime_disable (&dev->dev);framebuffer_release (info); -iounmap (da8xx_fb_reg_base); -release_mem_region (lcdc_regs->start,resource_size (lcdc_regs)); }return 0 ;
@@ -1265,7 +1262,6 @@ static int fb_probe(struct platform_device *device)struct fb_info *da8xx_fb_info;struct clk *fb_clk =NULL ;struct da8xx_fb_par *par; -resource_size_t len;int ret, i;unsigned long ulcm;
@@ -1275,29 +1271,16 @@ static int fb_probe(struct platform_device *device) } lcdc_regs =platform_get_resource (device,IORESOURCE_MEM ,0 ); -if (!lcdc_regs) { -dev_err (&device->dev, -"Can not get memory resource for LCD controller\n" ); -return -ENOENT ; - } - - len =resource_size (lcdc_regs); - - lcdc_regs =request_mem_region (lcdc_regs->start, len, lcdc_regs->name); -if (!lcdc_regs) -return -EBUSY ; - - da8xx_fb_reg_base =ioremap (lcdc_regs->start, len); + da8xx_fb_reg_base =devm_request_and_ioremap (&device->dev, lcdc_regs);if (!da8xx_fb_reg_base) { - ret = -EBUSY ; - goto err_request_mem; +dev_err (&device->dev,"memory resource setup failed\n" ); +return -EADDRNOTAVAIL ; } - fb_clk =clk_get (&device->dev,"fck" ); + fb_clk =devm_clk_get (&device->dev,"fck" );if (IS_ERR (fb_clk)) {dev_err (&device->dev,"Can not get device clock\n" ); - ret = -ENODEV ; - goto err_ioremap; +return -ENODEV ; }pm_runtime_enable (&device->dev);
@@ -1458,7 +1441,7 @@ static int fb_probe(struct platform_device *device) lcdc_irq_handler = lcdc_irq_handler_rev02; } - ret =request_irq (par->irq, lcdc_irq_handler,0 , + ret =devm_request_irq (&device->dev, par->irq, lcdc_irq_handler,0 ,DRIVER_NAME , par);if (ret) goto irq_freq;
@@ -1488,12 +1471,6 @@ err_pm_runtime_disable:pm_runtime_put_sync (&device->dev);pm_runtime_disable (&device->dev); -err_ioremap: -iounmap (da8xx_fb_reg_base); - -err_request_mem: -release_mem_region (lcdc_regs->start, len); -return ret; }
Signature · 1 line
1.7.9.5
PATCH v2 11/12 video: da8xx-fb: setup struct lcd_ctrl_config for dt
In reply to Afzal Mohammedstrcut lcd_ctrl_config information required for driver is currently obtained via platform data. To handle DT probing, create lcd_ctrl_config and populate it with default values, these values are sufficient for the panels so far used with this controller to work.
--- drivers/video/da8xx-fb.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-)
drivers/video/da8xx-fb.c +33 -1
@@ -1254,6 +1254,35 @@ 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;
@@ -1345,7 +1374,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 = (struct lcd_ctrl_config *)fb_pdata->controller_data;if (!lcd_cfg) { ret = -EINVAL ;
Signature · 1 line
1.7.9.5
PATCH v2 6/12 video: da8xx-fb: reorganize panel detection
In reply to Afzal MohammedMove panel detection to a separate function, this helps in readability as well as makes DT support cleaner.
--- drivers/video/da8xx-fb.c | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-)
drivers/video/da8xx-fb.c +26 -16
@@ -1253,6 +1253,27 @@ static struct fb_ops da8xx_fb_ops = { .fb_blank = cfb_blank, }; +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; +int i; + +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 ) +break ; + } + +if (i ==ARRAY_SIZE (known_lcd_panels)) { +dev_err (&dev->dev,"no panel found\n" ); +return NULL ; + } +dev_info (&dev->dev,"found %s panel\n" , lcdc_info->name); + +return lcdc_info; +} +static int fb_probe (struct platform_device *device) { struct da8xx_lcdc_platform_data *fb_pdata =
@@ -1262,7 +1283,7 @@ static int fb_probe(struct platform_device *device)struct fb_info *da8xx_fb_info;struct clk *fb_clk =NULL ;struct da8xx_fb_par *par; -int ret, i; +int ret;unsigned long ulcm;if (fb_pdata ==NULL ) {
@@ -1270,6 +1291,10 @@ static int fb_probe(struct platform_device *device)return -ENOENT ; } + lcdc_info =da8xx_fb_get_videomode (device); +if (lcdc_info ==NULL ) +return -ENODEV ; + lcdc_regs =platform_get_resource (device,IORESOURCE_MEM ,0 ); da8xx_fb_reg_base =devm_request_and_ioremap (&device->dev, lcdc_regs);if (!da8xx_fb_reg_base) {
@@ -1303,21 +1328,6 @@ static int fb_probe(struct platform_device *device)break ; } -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 ) -break ; - } - -if (i ==ARRAY_SIZE (known_lcd_panels)) { -dev_err (&device->dev,"GLCD: No valid panel found\n" ); - ret = -ENODEV ; - goto err_pm_runtime_disable; - }else -dev_info (&device->dev,"GLCD: Found %s panel\n" , - fb_pdata->type); - lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;if (!lcd_cfg) {
Signature · 1 line
1.7.9.5
PATCH v2 12/12 video: da8xx-fb: set upstream clock rate (if reqd)
In reply to Afzal MohammedLCDC IP has a clock divider to adjust pixel clock, this limits pixel clock range to fck/255 - fck/2(fck - rate of input clock to LCDC IP). In the case of AM335x, where this IP is present, default fck is not sufficient to provide normal pixel clock rates, hence rendering this driver unusable on AM335x.
If input clock too is configurable, allowable range of pixel clock would increase. Here initially it is checked whether with present fck, divider in IP could be configured to obtain required rate, if not, fck is adjusted. This makes it usable on AM335x.
Note:
A better (if allowable) solution may be to represent clock divider in LCDC IP as a basic divider clock - the one defined in common clock framework. But for this to happen, all the platform's using this driver should be using common clock framework (DaVinci is yet to be converted to use common clock framework). And it has to be determined whether common clock framework allows this kind of a clock modelling inside a driver and for this to be part of clock tree. Advantage of doing so would be better resolution for pixel clock, even though without this existing use cases are working properly. Or another extreme alternative would be to replicate clk-divider of common clock framework inside the driver, but that probably is not preferred and not worth as it would be duplication and without much advantage to existing users.
---
v2: new patch
drivers/video/da8xx-fb.c | 76 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 18 deletions(-)
drivers/video/da8xx-fb.c +58 -18
@@ -133,6 +133,9 @@#define WSI_TIMEOUT 50#define PALETTE_SIZE 256 +#define CLK_MIN_DIV 2 +#define CLK_MAX_DIV 255 +static void __iomem *da8xx_fb_reg_base;static struct resource *lcdc_regs;static unsigned int lcd_revision;
@@ -683,23 +686,21 @@ static void da8xx_fb_lcd_reset(void) } } -static inline unsigned da8xx_fb_calc_clk_divider (struct da8xx_fb_par *par, -unsigned pixclock) -{ -return par->lcd_fck_rate / (PICOS2KHZ (pixclock) *1000 ); -} - -static inline unsigned da8xx_fb_round_clk (struct da8xx_fb_par *par, -unsigned pixclock) +static int da8xx_fb_config_clk_divider (struct da8xx_fb_par *par, +unsigned div,unsigned rate) { -unsigned div; +int ret; - div =da8xx_fb_calc_clk_divider (par, pixclock); -return KHZ2PICOS (par->lcd_fck_rate / (1000 * div)); -} +if (par->lcd_fck_rate != rate) { + ret =clk_set_rate (par->lcdc_clk, rate); +if (IS_ERR_VALUE (ret)) { +dev_err (par->dev, +"unable to set clock rate at %u\n" , rate); +return ret; + } + par->lcd_fck_rate =clk_get_rate (par->lcdc_clk); + } -static inline void da8xx_fb_config_clk_divider (unsigned div) -{/* Configure the LCD clock divisor. */ lcdc_write (LCD_CLK_DIVISOR (div) | (LCD_RASTER_MODE &0x1 ),LCD_CTRL_REG );
@@ -707,14 +708,49 @@ static inline void da8xx_fb_config_clk_divider(unsigned div)if (lcd_revision ==LCD_VERSION_2 )lcdc_write (LCD_V2_DMA_CLK_EN |LCD_V2_LIDD_CLK_EN |LCD_V2_CORE_CLK_EN ,LCD_CLK_ENABLE_REG ); + +return 0 ; +} + +static unsigned int da8xx_fb_calc_clk_divider (struct da8xx_fb_par *par, +unsigned pixclock, +unsigned *rate) +{ +unsigned div; + + pixclock =PICOS2KHZ (pixclock) *1000 ; + + *rate = par->lcd_fck_rate; + +if (pixclock < (*rate /CLK_MAX_DIV )) { + *rate =clk_round_rate (par->lcdc_clk, pixclock *CLK_MAX_DIV ); + div =CLK_MAX_DIV ; + }else if (pixclock > (*rate /CLK_MIN_DIV )) { + *rate =clk_round_rate (par->lcdc_clk, pixclock *CLK_MIN_DIV ); + div =CLK_MIN_DIV ; + }else { + div = *rate / pixclock; + } + +return div; } -static inline void da8xx_fb_calc_config_clk_divider (struct da8xx_fb_par *par, +static inline int da8xx_fb_calc_config_clk_divider (struct da8xx_fb_par *par,struct fb_videomode *mode) { -unsigned div =da8xx_fb_calc_clk_divider (par, mode->pixclock); +unsigned rate; +unsigned div =da8xx_fb_calc_clk_divider (par, mode->pixclock, &rate); -da8xx_fb_config_clk_divider (div); +return da8xx_fb_config_clk_divider (par, div, rate); +} + +static inline unsigned da8xx_fb_round_clk (struct da8xx_fb_par *par, +unsigned pixclock) +{ +unsigned div, rate; + + div =da8xx_fb_calc_clk_divider (par, pixclock, &rate); +return KHZ2PICOS (rate / (1000 * div)); } static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,
@@ -723,7 +759,11 @@ static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,u32 bpp;int ret =0 ; -da8xx_fb_calc_config_clk_divider (par, panel); + ret =da8xx_fb_calc_config_clk_divider (par, panel); +if (IS_ERR_VALUE (ret)) { +dev_err (par->dev,"unable to configure clock\n" ); +return ret; + }if (panel->sync &FB_SYNC_CLK_INVERT ) lcdc_write((lcdc_read(LCD_RASTER_TIMING_2_REG) |
Signature · 1 line
1.7.9.5
Re: [PATCH v2 12/12] video: da8xx-fb: set upstream clock rate (if reqd)
Quoting Afzal Mohammed (2013-01-15 05:44:36)
Quoted 11 lines
LCDC IP has a clock divider to adjust pixel clock, this limits pixel clock range to fck/255 - fck/2(fck - rate of input clock to LCDC IP). In the case of AM335x, where this IP is present, default fck is not sufficient to provide normal pixel clock rates, hence rendering this driver unusable on AM335x.
If input clock too is configurable, allowable range of pixel clock would increase. Here initially it is checked whether with present fck, divider in IP could be configured to obtain required rate, if not, fck is adjusted. This makes it usable on AM335x.
Quoted 14 lines
Note:
A better (if allowable) solution may be to represent clock divider in LCDC IP as a basic divider clock - the one defined in common clock framework. But for this to happen, all the platform's using this driver should be using common clock framework (DaVinci is yet to be converted to use common clock framework). And it has to be determined whether common clock framework allows this kind of a clock modelling inside a driver and for this to be part of clock tree. Advantage of doing so would be better resolution for pixel clock, even though without this existing use cases are working properly. Or another extreme alternative would be to replicate clk-divider of common clock framework inside the driver, but that probably is not preferred and not worth as it would be duplication and without much advantage to existing users.
Afzal,
Modeling the divider inside your IP block as a clock is supported in the common clock framework. Linking up these sorts of clocks to the clock tree was one of the original design goals of CCF.
Regarding DaVinci: converting that platform over to use CCF would be the best approach. An alternative would be that you could break single-image boot for AM335x and DaVinci, by having AM335x use CCF and DaVinci use the legacy clock framework. From the LCDC driver's perspective this should not matter and is indeed the purpose of the clk.h api and clkdev interfaces, however looking at this driver I can see there would still be a lot ifdef-ery going on... better to just convert everything over to CCF.
Regards,
Mike
Quoted 126 lines
Signed-off-by Afzal Mohammed <[email protected]>---
v2: new patch
drivers/video/da8xx-fb.c | 76 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 18 deletions(-)drivers/video/da8xx-fb.c +58
-18@@ -133,6 +133,9 @@#define WSI_TIMEOUT 50#define PALETTE_SIZE 256 +#define CLK_MIN_DIV 2 +#define CLK_MAX_DIV 255 +static void __iomem *da8xx_fb_reg_base;static struct resource *lcdc_regs;static unsigned int lcd_revision;@@ -683,23 +686,21 @@ static void da8xx_fb_lcd_reset(void) } } -static inline unsigned da8xx_fb_calc_clk_divider (struct da8xx_fb_par *par, -unsigned pixclock) -{ -return par->lcd_fck_rate / (PICOS2KHZ (pixclock) *1000 ); -} - -static inline unsigned da8xx_fb_round_clk (struct da8xx_fb_par *par, -unsigned pixclock) +static int da8xx_fb_config_clk_divider (struct da8xx_fb_par *par, +unsigned div,unsigned rate) { -unsigned div; +int ret; - div =da8xx_fb_calc_clk_divider (par, pixclock); -return KHZ2PICOS (par->lcd_fck_rate / (1000 * div)); -} +if (par->lcd_fck_rate != rate) { + ret =clk_set_rate (par->lcdc_clk, rate); +if (IS_ERR_VALUE (ret)) { +dev_err (par->dev, +"unable to set clock rate at %u\n" , rate); +return ret; + } + par->lcd_fck_rate =clk_get_rate (par->lcdc_clk); + } -static inline void da8xx_fb_config_clk_divider (unsigned div) -{/* Configure the LCD clock divisor. */ lcdc_write (LCD_CLK_DIVISOR (div) | (LCD_RASTER_MODE &0x1 ),LCD_CTRL_REG );@@ -707,14 +708,49 @@ static inline void da8xx_fb_config_clk_divider(unsigned div)if (lcd_revision ==LCD_VERSION_2 )lcdc_write (LCD_V2_DMA_CLK_EN |LCD_V2_LIDD_CLK_EN |LCD_V2_CORE_CLK_EN ,LCD_CLK_ENABLE_REG ); + +return 0 ; +} + +static unsigned int da8xx_fb_calc_clk_divider (struct da8xx_fb_par *par, +unsigned pixclock, +unsigned *rate) +{ +unsigned div; + + pixclock =PICOS2KHZ (pixclock) *1000 ; + + *rate = par->lcd_fck_rate; + +if (pixclock < (*rate /CLK_MAX_DIV )) { + *rate =clk_round_rate (par->lcdc_clk, pixclock *CLK_MAX_DIV ); + div =CLK_MAX_DIV ; + }else if (pixclock > (*rate /CLK_MIN_DIV )) { + *rate =clk_round_rate (par->lcdc_clk, pixclock *CLK_MIN_DIV ); + div =CLK_MIN_DIV ; + }else { + div = *rate / pixclock; + } + +return div; } -static inline void da8xx_fb_calc_config_clk_divider (struct da8xx_fb_par *par, +static inline int da8xx_fb_calc_config_clk_divider (struct da8xx_fb_par *par,struct fb_videomode *mode) { -unsigned div =da8xx_fb_calc_clk_divider (par, mode->pixclock); +unsigned rate; +unsigned div =da8xx_fb_calc_clk_divider (par, mode->pixclock, &rate); -da8xx_fb_config_clk_divider (div); +return da8xx_fb_config_clk_divider (par, div, rate); +} + +static inline unsigned da8xx_fb_round_clk (struct da8xx_fb_par *par, +unsigned pixclock) +{ +unsigned div, rate; + + div =da8xx_fb_calc_clk_divider (par, pixclock, &rate); +return KHZ2PICOS (rate / (1000 * div)); } static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,@@ -723,7 +759,11 @@ static int lcd_init(struct da8xx_fb_par *par, const struct lcd_ctrl_config *cfg,u32 bpp;int ret =0 ; -da8xx_fb_calc_config_clk_divider (par, panel); + ret =da8xx_fb_calc_config_clk_divider (par, panel); +if (IS_ERR_VALUE (ret)) { +dev_err (par->dev,"unable to configure clock\n" ); +return ret; + }if (panel->sync &FB_SYNC_CLK_INVERT ) lcdc_write((lcdc_read(LCD_RASTER_TIMING_2_REG) |Signature · 1 line
1.7.9.5
Re: [PATCH v2 12/12] video: da8xx-fb: set upstream clock rate (if reqd)
Quoted 27 lines · Mike Turquette
Quoting Afzal Mohammed (2013-01-15 05:44:36)
Quoted 11 lines
LCDC IP has a clock divider to adjust pixel clock, this limits pixel clock range to fck/255 - fck/2(fck - rate of input clock to LCDC IP). In the case of AM335x, where this IP is present, default fck is not sufficient to provide normal pixel clock rates, hence rendering this driver unusable on AM335x.
If input clock too is configurable, allowable range of pixel clock would increase. Here initially it is checked whether with present fck, divider in IP could be configured to obtain required rate, if not, fck is adjusted. This makes it usable on AM335x.
Quoted 14 lines
Note:
A better (if allowable) solution may be to represent clock divider in LCDC IP as a basic divider clock - the one defined in common clock framework. But for this to happen, all the platform's using this driver should be using common clock framework (DaVinci is yet to be converted to use common clock framework). And it has to be determined whether common clock framework allows this kind of a clock modelling inside a driver and for this to be part of clock tree. Advantage of doing so would be better resolution for pixel clock, even though without this existing use cases are working properly. Or another extreme alternative would be to replicate clk-divider of common clock framework inside the driver, but that probably is not preferred and not worth as it would be duplication and without much advantage to existing users.
Quoted 8 lines
Afzal,
Modeling the divider inside your IP block as a clock is supported in the common clock framework. Linking up these sorts of clocks to the clock tree was one of the original design goals of CCF.
Regarding DaVinci: converting that platform over to use CCF would be the best approach.
This is work in progress. There are patches that have been posted. Work has been slow on this though due to lack of bandwidth.
Quoted 3 lines
An alternative would be that you could break
single-image boot for AM335x and DaVinci, by having AM335x use CCF and DaVinci use the legacy clock framework. From the LCDC driver's
Single image for DaVinci and AM335x is not possible anyway since ARMv5 and ARMv6+ cannot be supported in a single image.
Quoted 4 lines
perspective this should not matter and is indeed the purpose of the
clk.hapi and clkdev interfaces, however looking at this driver I can see there would still be a lot ifdef-ery going on... better to just convert everything over to CCF.
Waiting for DaVinci CCF to complete will be too long a wait. Probably convert to CCF just for AM335x ATM. There would be some ifdef'ry but hopefully that need not be inside function bodies. Would have to see the implementation, I guess.
Thanks,
Sekhar
RE: [PATCH v2 12/12] video: da8xx-fb: set upstream clock rate (if reqd)
Hi Mike,
Quoted 39 lines · Sekhar
Quoted 16 lines · Mike Turquette
Quoting Afzal Mohammed (2013-01-15 05:44:36)
Quoted 5 lines
Note:
A better (if allowable) solution may be to represent clock divider in LCDC IP as a basic divider clock - the one defined in common clock framework. But for this to happen, all the platform's using this driver should be using common clock framework (DaVinci is yet to be convertedQuoted 8 lines
to use common clock framework). And it has to be determined whether common clock framework allows this kind of a clock modelling inside a driver and for this to be part of clock tree. Advantage of doing so would be better resolution for pixel clock, even though without this existing use cases are working properly. Or another extreme alternative would be to replicate clk-divider of common clock framework inside the driver, but that probably is not preferred and not worth as it would be duplication and without much advantage to existing users.
Quoted 6 lines
Modeling the divider inside your IP block as a clock is supported in the common clock framework. Linking up these sorts of clocks to the clock tree was one of the original design goals of CCF.
Regarding DaVinci: converting that platform over to use CCF would be the best approach.
This is work in progress. There are patches that have been posted. Work has been slow on this though due to lack of bandwidth.
Quoted 3 lines
An alternative would be that you could break
single-image boot for AM335x and DaVinci, by having AM335x use CCF and DaVinci use the legacy clock framework. From the LCDC driver'sSingle image for DaVinci and AM335x is not possible anyway since ARMv5 and ARMv6+ cannot be supported in a single image.
Quoted 4 lines
perspective this should not matter and is indeed the purpose of the
clk.hapi and clkdev interfaces, however looking at this driver I can see there would still be a lot ifdef-ery going on... better to just convert everything over to CCF.
Quoted 4 lines
Waiting for DaVinci CCF to complete will be too long a wait. Probably convert to CCF just for AM335x ATM. There would be some ifdef'ry but hopefully that need not be inside function bodies. Would have to see the implementation, I guess.
v4 posted has the divider in LCDC IP modeled by clock node for CCF, for non-CCF (DaVinci), existing logic is kept as is with the help of ifdef's (as DaVinci maintainer mentioned that DaVinci CCF may take some time)
Regards
Afzal