On 10/24/16 11:43, Bartosz Golaszewski wrote:
> Revision 1 of the IP doesn't work if we don't load the palette (even
> if it's not used, which is the case for the RGB565 format).
>
> Add a function called from tilcdc_crtc_enable() which performs all
> required actions if we're dealing with a rev1 chip.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski at baylibre.com>
> ---
> v1 -> v2:
> - only allocate dma memory for revision 1
>
> drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 86
> +++++++++++++++++++++++++++++++++++-
> 1 file changed, 85 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 87cfde9..92771c6 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -21,11 +21,14 @@
> #include <drm/drm_flip_work.h>
> #include <drm/drm_plane_helper.h>
> #include <linux/workqueue.h>
> +#include <linux/completion.h>
>
> #include "tilcdc_drv.h"
> #include "tilcdc_regs.h"
>
> -#define TILCDC_VBLANK_SAFETY_THRESHOLD_US 1000
> +#define TILCDC_VBLANK_SAFETY_THRESHOLD_US 1000
> +#define TILCDC_REV1_PALETTE_SIZE 32
> +#define TILCDC_REV1_PALETTE_FIRST_ENTRY 0x4000
>
> struct tilcdc_crtc {
> struct drm_crtc base;
> @@ -53,6 +56,10 @@ struct tilcdc_crtc {
>
> int sync_lost_count;
> bool frame_intact;
> +
> + dma_addr_t palette_dma_handle;
> + void *palette_base;
> + struct completion palette_loaded;
> };
> #define to_tilcdc_crtc(x) container_of(x, struct tilcdc_crtc, base)
>
> @@ -100,6 +107,55 @@ static void set_scanout(struct drm_crtc *crtc, struct
> drm_framebuffer *fb)
> tilcdc_crtc->curr_fb = fb;
> }
>
> +/*
> + * The driver currently only supports the RGB565 format for revision 1. For
> + * 16 bits-per-pixel the palette block is bypassed, but the first 32 bytes of
> + * the framebuffer are still considered palette. The first 16-bit entry must
> + * be 0x4000 while all other entries must be zeroed.
> + */
> +static void tilcdc_crtc_load_palette(struct drm_crtc *crtc)
> +{
> + u32 dma_fb_base, dma_fb_ceiling, raster_ctl;
> + struct tilcdc_crtc *tilcdc_crtc;
> + struct drm_device *dev;
> + u16 *first_entry;
> +
> + dev = crtc->dev;
> + tilcdc_crtc = to_tilcdc_crtc(crtc);
> + first_entry = tilcdc_crtc->palette_base;
> +
> + *first_entry = TILCDC_REV1_PALETTE_FIRST_ENTRY;
> +
> + dma_fb_base = tilcdc_read(dev, LCDC_DMA_FB_BASE_ADDR_0_REG);
> + dma_fb_ceiling = tilcdc_read(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG);
> + raster_ctl = tilcdc_read(dev, LCDC_RASTER_CTRL_REG);
> +
> + /* Tell the LCDC where the palette is located. */
> + tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG,
> + tilcdc_crtc->palette_dma_handle);
> + tilcdc_write(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG,
> + (u32)tilcdc_crtc->palette_dma_handle
> + + TILCDC_REV1_PALETTE_SIZE - 1);
> +
> + /* Load it. */
> + tilcdc_clear(dev, LCDC_RASTER_CTRL_REG,
> + LCDC_PALETTE_LOAD_MODE(DATA_ONLY));
> + tilcdc_set(dev, LCDC_RASTER_CTRL_REG,
> + LCDC_PALETTE_LOAD_MODE(PALETTE_ONLY));
> +
> + /* Enable the LCDC and wait for palette to be loaded. */
> + tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
> + tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
> +
> + wait_for_completion(&tilcdc_crtc->palette_loaded);
> +
> + /* Restore the registers. */
> + tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
> + tilcdc_write(dev, LCDC_DMA_FB_BASE_ADDR_0_REG, dma_fb_base);
> + tilcdc_write(dev, LCDC_DMA_FB_CEILING_ADDR_0_REG, dma_fb_ceiling);
> + tilcdc_write(dev, LCDC_RASTER_CTRL_REG, raster_ctl);
> +}
> +
> static void tilcdc_crtc_enable_irqs(struct drm_device *dev)
> {
> struct tilcdc_drm_private *priv = dev->dev_private;
> @@ -154,6 +210,7 @@ static void tilcdc_crtc_enable(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
> + struct tilcdc_drm_private *priv = dev->dev_private;
>
> WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
>
> @@ -164,6 +221,9 @@ static void tilcdc_crtc_enable(struct drm_crtc *crtc)
>
> reset(crtc);
>
> + if (priv->rev == 1 && !completion_done(&tilcdc_crtc->palette_loaded))
> + tilcdc_crtc_load_palette(crtc);
> +
> tilcdc_crtc_enable_irqs(dev);
>
> tilcdc_clear(dev, LCDC_DMA_CTRL_REG, LCDC_DUAL_FRAME_BUFFER_ENABLE);
> @@ -245,6 +305,12 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
> of_node_put(crtc->port);
> drm_crtc_cleanup(crtc);
> drm_flip_work_cleanup(&tilcdc_crtc->unref_work);
> +
> + if (priv->rev == 1) {
> + dma_free_coherent(crtc->dev->dev, TILCDC_REV1_PALETTE_SIZE,
> + tilcdc_crtc->palette_base,
> + tilcdc_crtc->palette_dma_handle);
> + }
> }
>
> int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
> @@ -804,6 +870,14 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
> dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underfow",
> __func__, stat);
>
> + if (priv->rev == 1) {
> + if (stat & LCDC_PL_LOAD_DONE) {
> + complete(&tilcdc_crtc->palette_loaded);
> + tilcdc_clear(dev,
> + LCDC_RASTER_CTRL_REG, LCDC_V1_PL_INT_ENA);
> + }
> + }
> +
> /* For revision 2 only */
> if (priv->rev == 2) {
> if (stat & LCDC_FRAME_DONE) {
> @@ -845,6 +919,16 @@ struct drm_crtc *tilcdc_crtc_create(struct drm_device
> *dev)
> return NULL;
> }
>
> + if (priv->rev == 1) {
> + init_completion(&tilcdc_crtc->palette_loaded);
Is it enough to load the palette only once? What if lcdc is powered down
by power management in the middle?
I think you should reinit the completion struct at least in
tilcdc_pm_resume(), if not in tilcdc_crtc_disable().
Cheers,
Jyri
> + tilcdc_crtc->palette_base = dma_zalloc_coherent(dev->dev,
> + TILCDC_REV1_PALETTE_SIZE,
> + &tilcdc_crtc->palette_dma_handle,
> + GFP_KERNEL);
> + if (!tilcdc_crtc->palette_base)
> + return ERR_PTR(-ENOMEM);
> + }
> +
> crtc = &tilcdc_crtc->base;
>
> ret = tilcdc_plane_init(dev, &tilcdc_crtc->primary);
>
