[PATCH v3 02/11] drm/tilcdc: implement palette loading for rev1

[Tomi Valkeinen]

On 22/11/16 18:54, Jyri Sarha wrote:
> From: Bartosz Golaszewski <bgolaszewski at baylibre.com>
> 
> 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>
> Signed-off-by: Jyri Sarha <jsarha at ti.com>
> ---
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 88 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 87 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 5260eb2..0bfd7dd 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -21,11 +21,15 @@
>  #include <drm/drm_flip_work.h>
>  #include <drm/drm_plane_helper.h>
>  #include <linux/workqueue.h>
> +#include <linux/completion.h>
> +#include <linux/dma-mapping.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;
> @@ -56,6 +60,10 @@ struct tilcdc_crtc {
>  	int sync_lost_count;
>  	bool frame_intact;
>  	struct work_struct recover_work;
> +
> +	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)
>  
> @@ -105,6 +113,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;
> @@ -160,6 +217,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));
>  	mutex_lock(&tilcdc_crtc->enable_lock);
> @@ -172,6 +230,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);
> @@ -213,6 +274,13 @@ static void tilcdc_crtc_off(struct drm_crtc *crtc, bool shutdown)
>  				__func__);
>  	}
>  
> +	/*
> +	 * LCDC will not retain the palette when reset. Make sure it gets
> +	 * reloaded on tilcdc_crtc_enable().
> +	 */
> +	if (priv->rev == 1)
> +		reinit_completion(&tilcdc_crtc->palette_loaded);
> +

So when the crtc is disabled, you do this, so that on crtc enable the
driver would load palette? When is the crtc enabled if it hasn't been
disabled first? In other words, when will the palette loading in
tilcdc_crtc_enable() _not_ trigger for v1?

This looks a bit messy to me.

Why not just load the palette every time on crtc enable? And reinit the
completion in tilcdc_crtc_load_palette().

 Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161124/692b7df3/attachment.sig>