On 03/01/2017 10:25 AM, Songjun Wu wrote:
> Add driver-model support to this driver.
> 
> Signed-off-by: Songjun Wu <songjun...@microchip.com>
> ---
> 
> Changes in v2:
> - Due to the peripheral clock driver improvement, remove
>   the unneccessary clock calling.
> 
>  board/atmel/at91sam9n12ek/at91sam9n12ek.c       |   2 +
>  board/atmel/at91sam9x5ek/at91sam9x5ek.c         |   2 +
>  board/atmel/sama5d2_xplained/sama5d2_xplained.c |   2 +
>  board/atmel/sama5d3xek/sama5d3xek.c             |   2 +
>  board/atmel/sama5d4_xplained/sama5d4_xplained.c |   2 +
>  board/atmel/sama5d4ek/sama5d4ek.c               |   2 +
>  drivers/video/Kconfig                           |   9 +
>  drivers/video/atmel_hlcdfb.c                    | 314 
> +++++++++++++++++++++++-

Split the driver from board changes.

>  include/configs/at91sam9n12ek.h                 |   4 +
>  include/configs/at91sam9x5ek.h                  |   4 +
>  include/configs/ma5d4evk.h                      |   4 +
>  include/configs/sama5d2_xplained.h              |   4 +
>  include/configs/sama5d3xek.h                    |   4 +
>  include/configs/sama5d4_xplained.h              |   4 +
>  include/configs/sama5d4ek.h                     |   4 +
>  15 files changed, 360 insertions(+), 3 deletions(-)

[...]

> diff --git a/drivers/video/atmel_hlcdfb.c b/drivers/video/atmel_hlcdfb.c
> index 960b474b76..0bcb92b2cf 100644
> --- a/drivers/video/atmel_hlcdfb.c
> +++ b/drivers/video/atmel_hlcdfb.c
> @@ -10,13 +10,24 @@
>  #include <asm/io.h>
>  #include <asm/arch/gpio.h>
>  #include <asm/arch/clk.h>
> +#include <clk.h>
> +#include <dm.h>
> +#include <fdtdec.h>
>  #include <lcd.h>
> +#include <video.h>
>  #include <atmel_hlcdc.h>
>  
>  #if defined(CONFIG_LCD_LOGO)
>  #include <bmp_logo.h>
>  #endif
>  
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define lcdc_readl(reg)              __raw_readl((reg))
> +#define lcdc_writel(reg, val)        __raw_writel((val), (reg))

Really ? Do we REALLY need more new accessors ? Just use readl/writel ...

> +#ifndef CONFIG_DM_VIDEO
> +
>  /* configurable parameters */
>  #define ATMEL_LCDC_CVAL_DEFAULT              0xc8
>  #define ATMEL_LCDC_DMA_BURST_LEN     8
> @@ -26,9 +37,6 @@
>  
>  #define ATMEL_LCDC_FIFO_SIZE         512
>  
> -#define lcdc_readl(reg)              __raw_readl((reg))
> -#define lcdc_writel(reg, val)        __raw_writel((val), (reg))
> -
>  /*
>   * the CLUT register map as following
>   * RCLUT(24 ~ 16), GCLUT(15 ~ 8), BCLUT(7 ~ 0)
> @@ -218,3 +226,303 @@ void lcd_ctrl_init(void *lcdbase)
>       /* Enable flushing if we enabled dcache */
>       lcd_set_flush_dcache(1);
>  }
> +
> +#else
> +
> +enum {
> +     LCD_MAX_WIDTH           = 1024,
> +     LCD_MAX_HEIGHT          = 768,
> +     LCD_MAX_LOG2_BPP        = VIDEO_BPP16,
> +};
> +
> +struct atmel_hlcdc_priv {
> +     struct atmel_hlcd_regs *regs;
> +     struct display_timing timing;
> +     unsigned int vl_bpix;
> +     unsigned int guard_time;
> +     ulong clk_rate;
> +};
> +
> +static int at91_hlcdc_enable_clk(struct udevice *dev)
> +{
> +     struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
> +     struct clk clk;
> +     ulong clk_rate;
> +     int ret;
> +
> +     ret = clk_get_by_index(dev, 0, &clk);
> +     if (ret)
> +             return -EINVAL;
> +
> +     ret = clk_enable(&clk);
> +     if (ret)
> +             return ret;
> +
> +     clk_rate = clk_get_rate(&clk);
> +     if (!clk_rate)
> +             return -ENODEV;

Clock are still enabled if you fail here ...

> +     priv->clk_rate = clk_rate;
> +
> +     clk_free(&clk);
> +
> +     return 0;
> +}
> +
> +static void atmel_hlcdc_init(struct udevice *dev)
> +{
> +     struct video_uc_platdata *uc_plat = dev_get_uclass_platdata(dev);
> +     struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
> +     struct atmel_hlcd_regs *regs = priv->regs;
> +     struct display_timing *timing = &priv->timing;
> +     struct lcd_dma_desc *desc;
> +     unsigned long value, vl_clk_pol;
> +
> +     /* Disable DISP signal */
> +     lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_DISPDIS);
> +     while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_DISPSTS))
> +             udelay(1);

wait_for_bit(), fix globally ... and don't use unbounded loops ...

> +     /* Disable synchronization */
> +     lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_SYNCDIS);
> +     while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_LCDSTS))
> +             udelay(1);
> +     /* Disable pixel clock */
> +     lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_CLKDIS);
> +     while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_CLKSTS))
> +             udelay(1);
> +     /* Disable PWM */
> +     lcdc_writel(&regs->lcdc_lcddis, LCDC_LCDDIS_PWMDIS);
> +     while ((lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_PWMSTS))
> +             udelay(1);
> +
> +     /* Set pixel clock */
> +     value = priv->clk_rate / timing->pixelclock.typ;
> +     if (priv->clk_rate % timing->pixelclock.typ)
> +             value++;
> +
> +     vl_clk_pol = 0;
> +     if (timing->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> +             vl_clk_pol = LCDC_LCDCFG0_CLKPOL;
> +
> +     if (value < 1) {

I guess I'd just introduce a variable and ORR it with either CLKSEL or
whatever bits based on the value to reduce this lengthy condition here.
ie u32 foo = LCDC_BAR | LCDC_BAZ;
if (value < 1)
  foo |= LCDC_QUUX;
else
  ...

writel(...);

> +             /* Using system clock as pixel clock */
> +             lcdc_writel(&regs->lcdc_lcdcfg0,
> +                         LCDC_LCDCFG0_CLKDIV(0)
> +                         | LCDC_LCDCFG0_CGDISHCR
> +                         | LCDC_LCDCFG0_CGDISHEO
> +                         | LCDC_LCDCFG0_CGDISOVR1
> +                         | LCDC_LCDCFG0_CGDISBASE
> +                         | vl_clk_pol
> +                         | LCDC_LCDCFG0_CLKSEL);
> +
> +     } else {
> +             lcdc_writel(&regs->lcdc_lcdcfg0,
> +                         LCDC_LCDCFG0_CLKDIV(value - 2)
> +                         | LCDC_LCDCFG0_CGDISHCR
> +                         | LCDC_LCDCFG0_CGDISHEO
> +                         | LCDC_LCDCFG0_CGDISOVR1
> +                         | LCDC_LCDCFG0_CGDISBASE
> +                         | vl_clk_pol);
> +     }
> +
> +     /* Initialize control register 5 */
> +     value = 0;
> +
> +     if (!(timing->flags & DISPLAY_FLAGS_HSYNC_HIGH))
> +             value |= LCDC_LCDCFG5_HSPOL;
> +     if (!(timing->flags & DISPLAY_FLAGS_VSYNC_HIGH))
> +             value |= LCDC_LCDCFG5_VSPOL;
> +
> +#ifndef LCD_OUTPUT_BPP

Just make sure that's always defined . If possible, get this from DT.

> +     /* Output is 24bpp */
> +     value |= LCDC_LCDCFG5_MODE_OUTPUT_24BPP;
> +#else
> +     switch (LCD_OUTPUT_BPP) {
> +     case 12:
> +             value |= LCDC_LCDCFG5_MODE_OUTPUT_12BPP;
> +             break;
> +     case 16:
> +             value |= LCDC_LCDCFG5_MODE_OUTPUT_16BPP;
> +             break;
> +     case 18:
> +             value |= LCDC_LCDCFG5_MODE_OUTPUT_18BPP;
> +             break;
> +     case 24:
> +             value |= LCDC_LCDCFG5_MODE_OUTPUT_24BPP;
> +             break;
> +     default:
> +             BUG();
> +             break;
> +     }
> +#endif
> +
> +     value |= LCDC_LCDCFG5_GUARDTIME(priv->guard_time);
> +     value |= (LCDC_LCDCFG5_DISPDLY | LCDC_LCDCFG5_VSPDLYS);
> +     lcdc_writel(&regs->lcdc_lcdcfg5, value);
> +
> +     /* Vertical & Horizontal Timing */
> +     value = LCDC_LCDCFG1_VSPW(timing->vsync_len.typ - 1);
> +     value |= LCDC_LCDCFG1_HSPW(timing->hsync_len.typ - 1);
> +     lcdc_writel(&regs->lcdc_lcdcfg1, value);
> +
> +     value = LCDC_LCDCFG2_VBPW(timing->vback_porch.typ);
> +     value |= LCDC_LCDCFG2_VFPW(timing->vfront_porch.typ - 1);
> +     lcdc_writel(&regs->lcdc_lcdcfg2, value);
> +
> +     value = LCDC_LCDCFG3_HBPW(timing->hback_porch.typ - 1);
> +     value |= LCDC_LCDCFG3_HFPW(timing->hfront_porch.typ - 1);
> +     lcdc_writel(&regs->lcdc_lcdcfg3, value);
> +
> +     /* Display size */
> +     value = LCDC_LCDCFG4_RPF(timing->vactive.typ - 1);
> +     value |= LCDC_LCDCFG4_PPL(timing->hactive.typ - 1);
> +     lcdc_writel(&regs->lcdc_lcdcfg4, value);
> +
> +     lcdc_writel(&regs->lcdc_basecfg0,
> +                 LCDC_BASECFG0_BLEN_AHB_INCR4 | LCDC_BASECFG0_DLBO);
> +
> +     switch (VNBITS(priv->vl_bpix)) {
> +     case 16:
> +             lcdc_writel(&regs->lcdc_basecfg1,
> +                         LCDC_BASECFG1_RGBMODE_16BPP_RGB_565);
> +             break;
> +     case 32:
> +             lcdc_writel(&regs->lcdc_basecfg1,
> +                         LCDC_BASECFG1_RGBMODE_24BPP_RGB_888);
> +             break;
> +     default:
> +             BUG();
> +             break;
> +     }
> +
> +     lcdc_writel(&regs->lcdc_basecfg2, LCDC_BASECFG2_XSTRIDE(0));
> +     lcdc_writel(&regs->lcdc_basecfg3, 0);
> +     lcdc_writel(&regs->lcdc_basecfg4, LCDC_BASECFG4_DMA);
> +
> +     /* Disable all interrupts */
> +     lcdc_writel(&regs->lcdc_lcdidr, ~0UL);
> +     lcdc_writel(&regs->lcdc_baseidr, ~0UL);
> +
> +     /* Setup the DMA descriptor, this descriptor will loop to itself */
> +     desc = (struct lcd_dma_desc *)(uc_plat->base - 16);
> +
> +     desc->address = (u32)uc_plat->base;
> +
> +     /* Disable DMA transfer interrupt & descriptor loaded interrupt. */
> +     desc->control = LCDC_BASECTRL_ADDIEN | LCDC_BASECTRL_DSCRIEN
> +                     | LCDC_BASECTRL_DMAIEN | LCDC_BASECTRL_DFETCH;
> +     desc->next = (u32)desc;
> +
> +     /* Flush the DMA descriptor if we enabled dcache */
> +     flush_dcache_range((u32)desc, (u32)desc + sizeof(*desc));
> +
> +     lcdc_writel(&regs->lcdc_baseaddr, desc->address);
> +     lcdc_writel(&regs->lcdc_basectrl, desc->control);
> +     lcdc_writel(&regs->lcdc_basenext, desc->next);
> +     lcdc_writel(&regs->lcdc_basecher,
> +                 LCDC_BASECHER_CHEN | LCDC_BASECHER_UPDATEEN);
> +
> +     /* Enable LCD */
> +     value = lcdc_readl(&regs->lcdc_lcden);
> +     lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_CLKEN);
> +     while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_CLKSTS))
> +             udelay(1);

wait_for_bit() ...

> +     value = lcdc_readl(&regs->lcdc_lcden);
> +     lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_SYNCEN);
> +     while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_LCDSTS))
> +             udelay(1);
> +     value = lcdc_readl(&regs->lcdc_lcden);
> +     lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_DISPEN);
> +     while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_DISPSTS))
> +             udelay(1);
> +     value = lcdc_readl(&regs->lcdc_lcden);
> +     lcdc_writel(&regs->lcdc_lcden, value | LCDC_LCDEN_PWMEN);
> +     while (!(lcdc_readl(&regs->lcdc_lcdsr) & LCDC_LCDSR_PWMSTS))
> +             udelay(1);
> +}
> +
> +static int atmel_hlcdc_probe(struct udevice *dev)
> +{
> +     struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> +     struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
> +     int ret;
> +
> +     ret = at91_hlcdc_enable_clk(dev);
> +     if (ret)
> +             return ret;
> +
> +     atmel_hlcdc_init(dev);
> +
> +     uc_priv->xsize = priv->timing.hactive.typ;
> +     uc_priv->ysize = priv->timing.vactive.typ;
> +     uc_priv->bpix = priv->vl_bpix;
> +
> +     /* Enable flushing if we enabled dcache */
> +     video_set_flush_dcache(dev, true);
> +
> +     return 0;
> +}
> +
> +static int atmel_hlcdc_ofdata_to_platdata(struct udevice *dev)
> +{
> +     struct atmel_hlcdc_priv *priv = dev_get_priv(dev);
> +     const void *blob = gd->fdt_blob;
> +     int node = dev->of_offset;
> +
> +     priv->regs = (struct atmel_hlcd_regs *)dev_get_addr(dev);
> +     if (!priv->regs) {
> +             debug("%s: No display controller address\n", __func__);
> +             return -EINVAL;
> +     }
> +
> +     if (fdtdec_decode_display_timing(blob, dev->of_offset,
> +                                      0, &priv->timing)) {
> +             debug("%s: Failed to decode display timing\n", __func__);
> +             return -EINVAL;
> +     }
> +
> +     if (priv->timing.hactive.typ > LCD_MAX_WIDTH)
> +             priv->timing.hactive.typ = LCD_MAX_WIDTH;
> +
> +     if (priv->timing.vactive.typ > LCD_MAX_HEIGHT)
> +             priv->timing.vactive.typ = LCD_MAX_HEIGHT;
> +
> +     priv->vl_bpix = fdtdec_get_int(blob, node, "atmel,vl-bpix", 0);
> +     if (!priv->vl_bpix) {
> +             debug("%s: Failed to get bits per pixel\n", __func__);
> +             return -EINVAL;
> +     }
> +
> +     priv->guard_time = fdtdec_get_int(blob, node, "atmel,guard-time", 1);
> +
> +     return 0;
> +}
> +
> +static int atmel_hlcdc_bind(struct udevice *dev)
> +{
> +     struct video_uc_platdata *uc_plat = dev_get_uclass_platdata(dev);
> +
> +     uc_plat->size = LCD_MAX_WIDTH * LCD_MAX_HEIGHT *
> +                             (1 << LCD_MAX_LOG2_BPP) / 8;
> +
> +     debug("%s: Frame buffer size %x\n", __func__, uc_plat->size);
> +
> +     return 0;
> +}
> +
> +static const struct udevice_id atmel_hlcdc_ids[] = {
> +     { .compatible = "atmel,sama5d2-hlcdc" },
> +     { }
> +};
> +
> +U_BOOT_DRIVER(atmel_hlcdfb) = {
> +     .name   = "atmel_hlcdfb",
> +     .id     = UCLASS_VIDEO,
> +     .of_match = atmel_hlcdc_ids,
> +     .bind   = atmel_hlcdc_bind,
> +     .probe  = atmel_hlcdc_probe,
> +     .ofdata_to_platdata = atmel_hlcdc_ofdata_to_platdata,
> +     .priv_auto_alloc_size = sizeof(struct atmel_hlcdc_priv),
> +};
> +
> +#endif

[...]

> diff --git a/include/configs/sama5d4ek.h b/include/configs/sama5d4ek.h
> index 680d5918d7..4d537b22aa 100644
> --- a/include/configs/sama5d4ek.h
> +++ b/include/configs/sama5d4ek.h
> @@ -91,7 +91,11 @@
>  #define CONFIG_LCD_INFO
>  #define CONFIG_LCD_INFO_BELOW_LOGO
>  #define CONFIG_SYS_WHITE_ON_BLACK
> +
> +#ifndef CONFIG_DM_VIDEO
>  #define CONFIG_ATMEL_HLCD

This should use Kconfig ...

> +#endif
> +
>  #define CONFIG_ATMEL_LCD_RGB565
>  
>  #ifdef CONFIG_SYS_USE_SERIALFLASH
> 


-- 
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to