Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S938704AbcJXJZY (ORCPT ); Mon, 24 Oct 2016 05:25:24 -0400 Received: from comal.ext.ti.com ([198.47.26.152]:35666 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934500AbcJXJZW (ORCPT ); Mon, 24 Oct 2016 05:25:22 -0400 Subject: Re: [PATCH v2] drm: tilcdc: implement palette loading for rev1 To: Bartosz Golaszewski , Tomi Valkeinen , David Airlie , Kevin Hilman , Michael Turquette , Sekhar Nori References: <1477298581-31291-1-git-send-email-bgolaszewski@baylibre.com> CC: LKML , linux-drm , Laurent Pinchart , Peter Ujfalusi From: Jyri Sarha Message-ID: <9cabd4e2-b7a3-6925-e8a6-c399e96b537b@ti.com> Date: Mon, 24 Oct 2016 12:25:12 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <1477298581-31291-1-git-send-email-bgolaszewski@baylibre.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5996 Lines: 177 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 > --- > 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 > #include > #include > +#include > > #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); >