| Message ID | de6d0b357f8d5c33826739117f18b58aa87beae2.1479832733.git.jsarha@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
On 22/11/16 18:54, Jyri Sarha wrote: > From: Bartosz Golaszewski <bgolaszewski@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@baylibre.com> > Signed-off-by: Jyri Sarha <jsarha@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
On 11/24/16 11:29, Tomi Valkeinen wrote: >> @@ -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(). > My final version loads it at the end of modeset_nofb(), to avoid this: 1. Load the fb address to dma registers with "ingenious" 64 bit write 2. Load the dma registers to a temporary storrage 3. Put palette address in dma registers and load the palette 4. Restore the dma registers (= fb address) But, sure. I can load the palette every time the mode changes, but not every time the display is enabled. Thanks, Jyri
On 24/11/16 11:39, Jyri Sarha wrote: > On 11/24/16 11:29, Tomi Valkeinen wrote: >>> @@ -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(). >> > > My final version loads it at the end of modeset_nofb(), to avoid this: > > 1. Load the fb address to dma registers with "ingenious" 64 bit write > > 2. Load the dma registers to a temporary storrage > > 3. Put palette address in dma registers and load the palette > > 4. Restore the dma registers (= fb address) > > But, sure. I can load the palette every time the mode changes, but not > every time the display is enabled. What is the difference? If mode changes, you need to disable and enable the crtc, right? What other cases there are to enable the display? After blank? Then the display has been off, and I presume palette has to be loaded. Tomi
On 11/24/16 11:43, Tomi Valkeinen wrote: > What is the difference? If mode changes, you need to disable and enable > the crtc, right? What other cases there are to enable the display? After > blank? Then the display has been off, and I presume palette has to be > loaded. At the moment the palette or register values do not appear to vanish ever. But that is probably due to PM not doing much to optimize the LCDC power consumption. Anyway, if simple enable is enough to turn on the display - all video timings, frame buffer dma addresses etc. are already in the registers - then I think it is safe to assume that the palette is still in there too. Then it is a different issue, that I should probably put the same functionality into PM runtime_suspend() and runtime_resume() callbacks, that is currently in suspend() and resume() callbacks, to be ready if PM ever does anything more for LCDC that it does today. I could of course add a test if the registers are still intact before doing a restore. BR, Jyri
On 24/11/16 12:03, Jyri Sarha wrote: > On 11/24/16 11:43, Tomi Valkeinen wrote: >> What is the difference? If mode changes, you need to disable and enable >> the crtc, right? What other cases there are to enable the display? After >> blank? Then the display has been off, and I presume palette has to be >> loaded. > > At the moment the palette or register values do not appear to vanish > ever. But that is probably due to PM not doing much to optimize the LCDC > power consumption. If runtime PM for the device goes to suspend, you have to presume the IP has lost all context. That may not always happen, but that's what the driver has to presume, unless there's a way for the driver to verify whether the context has been lost or not. > Anyway, if simple enable is enough to turn on the display - all video > timings, frame buffer dma addresses etc. are already in the registers - > then I think it is safe to assume that the palette is still in there too. As long as the driver makes sure the device doesn't go to suspend (by having called pm_runtime_get). > Then it is a different issue, that I should probably put the same > functionality into PM runtime_suspend() and runtime_resume() callbacks, > that is currently in suspend() and resume() callbacks, to be ready if PM > ever does anything more for LCDC that it does today. I could of course > add a test if the registers are still intact before doing a restore. You can do things in resume callback, but I think quite often it's simplest to just do those things when enabling the display. The device never goes to suspend if the display is enabled. And if you disable the display, the device should go to suspend, so usually enable is called after the device has been in suspend. So, I haven't looked at the tilcdc code in detail in this regard, but my guess is that runtime PM resume could be used to set hardcoded things to registers, stuff that you always know how they are set. Everything else can be just programmed at enable. Looking at the registers to find out if they're intact is fine, but it's just an optimization. Tomi
On 11/24/16 12:25, Tomi Valkeinen wrote: > On 24/11/16 12:03, Jyri Sarha wrote: >> On 11/24/16 11:43, Tomi Valkeinen wrote: >>> What is the difference? If mode changes, you need to disable and enable >>> the crtc, right? What other cases there are to enable the display? After >>> blank? Then the display has been off, and I presume palette has to be >>> loaded. >> >> At the moment the palette or register values do not appear to vanish >> ever. But that is probably due to PM not doing much to optimize the LCDC >> power consumption. > > If runtime PM for the device goes to suspend, you have to presume the IP > has lost all context. That may not always happen, but that's what the > driver has to presume, unless there's a way for the driver to verify > whether the context has been lost or not. > There is couple of registers I can verify that from (reset value != value always used by the driver). >> Anyway, if simple enable is enough to turn on the display - all video >> timings, frame buffer dma addresses etc. are already in the registers - >> then I think it is safe to assume that the palette is still in there too. > > As long as the driver makes sure the device doesn't go to suspend (by > having called pm_runtime_get). Runtime get has always been called when modeset_nofb() is called. > >> Then it is a different issue, that I should probably put the same >> functionality into PM runtime_suspend() and runtime_resume() callbacks, >> that is currently in suspend() and resume() callbacks, to be ready if PM >> ever does anything more for LCDC that it does today. I could of course >> add a test if the registers are still intact before doing a restore. > > You can do things in resume callback, but I think quite often it's > simplest to just do those things when enabling the display. The device > never goes to suspend if the display is enabled. And if you disable the > display, the device should go to suspend, so usually enable is called > after the device has been in suspend. > Well, the two places are pretty much the same thing in tilcdc, because enable calls pm_runtime_get(). Also with atomic the suspend/resume implementation is really straight forward. Just get the current atomic state with drm_atomic_helper_suspend() and commit it back in with drm_atomic_helper_resume(), if needed. I don't think I should implement myself something that is so well provided by pm and drm infrastructure. I will implement that as soon as I get this current mess with LCDC rev 1 and bridge support sorted out. > So, I haven't looked at the tilcdc code in detail in this regard, but my > guess is that runtime PM resume could be used to set hardcoded things to > registers, stuff that you always know how they are set. Everything else > can be just programmed at enable. > > Looking at the registers to find out if they're intact is fine, but it's > just an optimization. > Yes, of course. BR, Jyri
On 24/11/16 12:38, Jyri Sarha wrote: >> As long as the driver makes sure the device doesn't go to suspend (by >> having called pm_runtime_get). > > Runtime get has always been called when modeset_nofb() is called. Yes, runtime get is needed to access the HW, but the question here was "has runtime_get been active ever since setting the registers previously". If you do runtime_get(); setup_things(); runtime_put(); runtime_get(); setup_more_things(); runtime_put(); the code is broken as the context is possibly lost between those two blocks. Unless runtime suspend/resume callbacks do a full context save and restore. >>> Then it is a different issue, that I should probably put the same >>> functionality into PM runtime_suspend() and runtime_resume() callbacks, >>> that is currently in suspend() and resume() callbacks, to be ready if PM >>> ever does anything more for LCDC that it does today. I could of course >>> add a test if the registers are still intact before doing a restore. >> >> You can do things in resume callback, but I think quite often it's >> simplest to just do those things when enabling the display. The device >> never goes to suspend if the display is enabled. And if you disable the >> display, the device should go to suspend, so usually enable is called >> after the device has been in suspend. >> > > Well, the two places are pretty much the same thing in tilcdc, because > enable calls pm_runtime_get(). Also with atomic the suspend/resume > implementation is really straight forward. Just get the current atomic > state with drm_atomic_helper_suspend() and commit it back in with > drm_atomic_helper_resume(), if needed. I don't think I should implement > myself something that is so well provided by pm and drm infrastructure. The suspend/resume you're talking there is the system suspend/resume. That's quite different than the runtime suspend/resume, and they should do very different things. The current system suspend/resume in tilcdc looks fine. Tomi
On 11/24/16 13:10, Tomi Valkeinen wrote: > On 24/11/16 12:38, Jyri Sarha wrote: > >>> As long as the driver makes sure the device doesn't go to suspend (by >>> having called pm_runtime_get). >> >> Runtime get has always been called when modeset_nofb() is called. > > Yes, runtime get is needed to access the HW, but the question here was > "has runtime_get been active ever since setting the registers previously". > > If you do > > runtime_get(); > setup_things(); > runtime_put(); > > runtime_get(); > setup_more_things(); > runtime_put(); > > the code is broken as the context is possibly lost between those two > blocks. Unless runtime suspend/resume callbacks do a full context save > and restore. > Yes, I know that. In the atomic driver the setup is always done in the drm_mode_config_funcs atomic_commit. The tilcdc commit takes does runtime_get() there, before the setup starts, and does a runtime_put() after drm_atomic_helper_commit_modeset_enables() has run. The drm_atomic_helper_commit_modeset_enables() has turned the display on, if necessary, an that will keep the HW powered until the display is turned off by another commit. Now, I am not sure if drm atomic core assumes optimizes the mode setting in commit, if there previously has been >>>> Then it is a different issue, that I should probably put the same >>>> functionality into PM runtime_suspend() and runtime_resume() callbacks, >>>> that is currently in suspend() and resume() callbacks, to be ready if PM >>>> ever does anything more for LCDC that it does today. I could of course >>>> add a test if the registers are still intact before doing a restore. >>> >>> You can do things in resume callback, but I think quite often it's >>> simplest to just do those things when enabling the display. The device >>> never goes to suspend if the display is enabled. And if you disable the >>> display, the device should go to suspend, so usually enable is called >>> after the device has been in suspend. >>> >> >> Well, the two places are pretty much the same thing in tilcdc, because >> enable calls pm_runtime_get(). Also with atomic the suspend/resume >> implementation is really straight forward. Just get the current atomic >> state with drm_atomic_helper_suspend() and commit it back in with >> drm_atomic_helper_resume(), if needed. I don't think I should implement >> myself something that is so well provided by pm and drm infrastructure. > > The suspend/resume you're talking there is the system suspend/resume. > That's quite different than the runtime suspend/resume, and they should > do very different things. > > The current system suspend/resume in tilcdc looks fine. > I don't undestand the same mechanism could not be used for runtime resume. Why should it matter if we configure the previous drm atomic state after system suspend or simple LCDC power down suspend? There may be some need for differentiation with more complex display hardware, but I see no such need for tilcdc. > Tomi >
On 24/11/16 14:03, Jyri Sarha wrote: >> The suspend/resume you're talking there is the system suspend/resume. >> That's quite different than the runtime suspend/resume, and they should >> do very different things. >> >> The current system suspend/resume in tilcdc looks fine. >> > > I don't undestand the same mechanism could not be used for runtime > resume. Why should it matter if we configure the previous drm atomic > state after system suspend or simple LCDC power down suspend? They are quite different things. For example, you have display up and running. The user requests for system suspend. System suspend callback is called in tilcdc. That callback should turn off the displays etc. Runtime PM suspend should not do anything like that, because it's called when the driver says the IP is not used, which means the driver has already turned off the displays etc. So, for example, when the system suspend happens, tilcdc's system suspend callback disables the displays by calling some functions. These functions stop the HW, maybe do other cleanups, and then do pm_runtime_put(). This then causes runtime PM suspend callback to be called. And, of course, runtime PM suspend is called anytime the driver is not using the HW, not only when system suspend happens. So, system suspend is a high level thing, comes at any point of time from the userspace. Runtime PM is a driver internal thing, comes from the driver. Tomi
On 11/24/16 14:56, Tomi Valkeinen wrote: > On 24/11/16 14:03, Jyri Sarha wrote: > >>> The suspend/resume you're talking there is the system suspend/resume. >>> That's quite different than the runtime suspend/resume, and they should >>> do very different things. >>> >>> The current system suspend/resume in tilcdc looks fine. >>> >> >> I don't undestand the same mechanism could not be used for runtime >> resume. Why should it matter if we configure the previous drm atomic >> state after system suspend or simple LCDC power down suspend? > > They are quite different things. > > For example, you have display up and running. The user requests for > system suspend. System suspend callback is called in tilcdc. That > callback should turn off the displays etc. > > Runtime PM suspend should not do anything like that, because it's called > when the driver says the IP is not used, which means the driver has > already turned off the displays etc. > > So, for example, when the system suspend happens, tilcdc's system > suspend callback disables the displays by calling some functions. These > functions stop the HW, maybe do other cleanups, and then do > pm_runtime_put(). This then causes runtime PM suspend callback to be called. > > And, of course, runtime PM suspend is called anytime the driver is not > using the HW, not only when system suspend happens. > > So, system suspend is a high level thing, comes at any point of time > from the userspace. Runtime PM is a driver internal thing, comes from > the driver. > I am aware of the difference, but in theory I thought it should work, because the situation is pretty much the same. We need restore the state that was in LCDC when it was shut down in disable and restore the state right before we are turning it back on, all the runtime_get() and puts would serialize nicely. But now after giving it a bit more thought, the drm locking prevents this from working. Who ever is enabling the display, is already holding some modeset locks and prevents drm_atomic_helper_resume() from taking the drm_modeset_lock_all(). Actually following your suggestion appears to be really straight forward. Simply get rid of mode_set_nofb() callback and call the same function in enable just before turning the raster on. I think I'll make a patch right away. Cheers, Jyri
On Thu, Nov 24, 2016 at 10:32:59PM +0200, Jyri Sarha wrote: > On 11/24/16 14:56, Tomi Valkeinen wrote: > > On 24/11/16 14:03, Jyri Sarha wrote: > > > >>> The suspend/resume you're talking there is the system suspend/resume. > >>> That's quite different than the runtime suspend/resume, and they should > >>> do very different things. > >>> > >>> The current system suspend/resume in tilcdc looks fine. > >>> > >> > >> I don't undestand the same mechanism could not be used for runtime > >> resume. Why should it matter if we configure the previous drm atomic > >> state after system suspend or simple LCDC power down suspend? > > > > They are quite different things. > > > > For example, you have display up and running. The user requests for > > system suspend. System suspend callback is called in tilcdc. That > > callback should turn off the displays etc. > > > > Runtime PM suspend should not do anything like that, because it's called > > when the driver says the IP is not used, which means the driver has > > already turned off the displays etc. > > > > So, for example, when the system suspend happens, tilcdc's system > > suspend callback disables the displays by calling some functions. These > > functions stop the HW, maybe do other cleanups, and then do > > pm_runtime_put(). This then causes runtime PM suspend callback to be called. > > > > And, of course, runtime PM suspend is called anytime the driver is not > > using the HW, not only when system suspend happens. > > > > So, system suspend is a high level thing, comes at any point of time > > from the userspace. Runtime PM is a driver internal thing, comes from > > the driver. > > > > I am aware of the difference, but in theory I thought it should work, > because the situation is pretty much the same. We need restore the state > that was in LCDC when it was shut down in disable and restore the state > right before we are turning it back on, all the runtime_get() and puts > would serialize nicely. > > But now after giving it a bit more thought, the drm locking prevents > this from working. Who ever is enabling the display, is already holding > some modeset locks and prevents drm_atomic_helper_resume() from taking > the drm_modeset_lock_all(). > > Actually following your suggestion appears to be really straight > forward. Simply get rid of mode_set_nofb() callback and call the same > function in enable just before turning the raster on. Yup, that's the right way to do this, and the kernel-doc for mode_set_nofb even explains it ;-) -Daniel
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); + drm_crtc_vblank_off(crtc); tilcdc_crtc_disable_irqs(dev); @@ -846,6 +914,14 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc) dev_err_ratelimited(dev->dev, "%s(0x%08x): FIFO underflow", __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); + } + } + if (stat & LCDC_SYNC_LOST) { dev_err_ratelimited(dev->dev, "%s(0x%08x): Sync lost", __func__, stat); @@ -890,6 +966,16 @@ struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev) return NULL; } + if (priv->rev == 1) { + init_completion(&tilcdc_crtc->palette_loaded); + tilcdc_crtc->palette_base = dmam_alloc_coherent(dev->dev, + TILCDC_REV1_PALETTE_SIZE, + &tilcdc_crtc->palette_dma_handle, + GFP_KERNEL | __GFP_ZERO); + if (!tilcdc_crtc->palette_base) + return ERR_PTR(-ENOMEM); + } + crtc = &tilcdc_crtc->base; ret = tilcdc_plane_init(dev, &tilcdc_crtc->primary);