[RFC][PATCH 0/3] MERAM support for LCDC

[RFC][PATCH 0/3] MERAM support for LCDC

[Damian Hobson-Garcia]

This patch series is based on the fbdev tree.  

This series adds support for read-ahead cache access through the ICBs and
MERAM available on SH devices.  The exact ICBs to use are chip specific
and are specified in the platform data. 

Damian Hobson-Garcia (3):
  sh_mobile_meram: MERAM framework for LCDC
  sh_mobile_meram: Add support for NV24 framebuffers
  sh_mobile_meram: MERAM platform data for LCDC

 arch/arm/mach-shmobile/board-ap4evb.c |   69 ++++
 drivers/video/Kconfig                 |   12 +
 drivers/video/Makefile                |    1 +
 drivers/video/sh_mobile_lcdcfb.c      |  104 ++++++-
 drivers/video/sh_mobile_lcdcfb.h      |    1 +
 drivers/video/sh_mobile_meram.c       |  567 +++++++++++++++++++++++++++++++++
 drivers/video/sh_mobile_meram.h       |   41 +++
 include/video/sh_mobile_lcdc.h        |    2 +
 include/video/sh_mobile_meram.h       |   69 ++++
 9 files changed, 857 insertions(+), 9 deletions(-)
 create mode 100644 drivers/video/sh_mobile_meram.c
 create mode 100644 drivers/video/sh_mobile_meram.h
 create mode 100644 include/video/sh_mobile_meram.h

Re: [RFC][PATCH 0/3] MERAM support for LCDC

[Magnus Damm]

Hi Damian,

Thanks for your work on this!

On Tue, Mar 29, 2011 at 12:35 PM, Damian Hobson-Garcia
<dhobsong@igel.co.jp> wrote:
> This patch series is based on the fbdev tree.
>
> This series adds support for read-ahead cache access through the ICBs and
> MERAM available on SH devices.  The exact ICBs to use are chip specific
> and are specified in the platform data.
>
> Damian Hobson-Garcia (3):
>  sh_mobile_meram: MERAM framework for LCDC
>  sh_mobile_meram: Add support for NV24 framebuffers
>  sh_mobile_meram: MERAM platform data for LCDC

I think these patches look very good in general. I have one request
and some general comments:

Please add Runtime PM support to the MERAM driver. The MSTP113 bit of
SMSTPCR1 should be dynamically controlled using pm_runtime_get_sync()
and pm_runtime_put_sync().

I also wonder if we can chose to use MERAM dynamically somehow. I know
the sh7372 MERAM can be used to back the entire frame buffer memory
(controlled by the LRCTRL register, see the MERAM section in the
sh7372 data sheet). This fully-backed mode can perhaps be used to save
power for regular low-performance operation and/or semi-standby.

For low-performance operation I imagine that by using MERAM for frame
buffer memory we can put the SDRAM in self-refresh mode and that way
enter deep idle modes even though the screen is still operating.

The semi-standby would be that the "real" framebuffer is in say 32-bit
RGB in SDRAM, but we keep a 8-bit RGB shadow buffer in MERAM and
switch to that at the same instant we move from full backlight to low
backlight. This is most likely useful only when combined with
switching the LCD panel itself to 8-bit mode as well to save
bandwidth.

Then you have your full-performance mode that I guess is what these
patches implement. =)

Any ideas/suggestions about the low power LCDC modes?

Thanks,

/ magnus

Re: [RFC][PATCH 0/3] MERAM support for LCDC

[Damian Hobson-Garcia]

Hi Magnus,

Thanks for your comments.

On 2011/04/26 14:46, Magnus Damm wrote:
> Hi Damian,
> 
> Thanks for your work on this!
> 
> I think these patches look very good in general. I have one request
> and some general comments:
> 
> Please add Runtime PM support to the MERAM driver. The MSTP113 bit of
> SMSTPCR1 should be dynamically controlled using pm_runtime_get_sync()
> and pm_runtime_put_sync().

Ok, I'll look into this, but I think that it will take some thinking.
The MERAM is accessed not only from the LCDC, but can also be accessed
from user space via the uio_pdrv_genirq driver.  I think we need to take
care of proper reference counting across these drivers.
> 
> I also wonder if we can chose to use MERAM dynamically somehow. I know
> the sh7372 MERAM can be used to back the entire frame buffer memory
> (controlled by the LRCTRL register, see the MERAM section in the
> sh7372 data sheet). This fully-backed mode can perhaps be used to save
> power for regular low-performance operation and/or semi-standby.
> 
> For low-performance operation I imagine that by using MERAM for frame
> buffer memory we can put the SDRAM in self-refresh mode and that way
> enter deep idle modes even though the screen is still operating.

I'm not sure that I follow your criteria for "low-performance
operation".  What do you define as low-performance?  Is this assuming
that we don't have (or disable) high-resolution (i.e. HDMI) secondary
output?
If so,it may be possible to dynamically reassign the area of the MERAM
that was allocated to HDMI for LCD output.  Otherwise the whole
framebuffer will not fit in MERAM and we'd have to revert to SDRAM.

> 
> The semi-standby would be that the "real" framebuffer is in say 32-bit
> RGB in SDRAM, but we keep a 8-bit RGB shadow buffer in MERAM and
> switch to that at the same instant we move from full backlight to low
> backlight. This is most likely useful only when combined with
> switching the LCD panel itself to 8-bit mode as well to save
> bandwidth.

By "keep an 8-bit RGB shadow buffer" are you talking about generating a
quantized version (RGB 332) of the "real" framebuffer in the MERAM when
we switch modes? It would be nice to be able to do such a conversion in
the hardware, but there currently isn't any support for VEU or the like
in the kernel.

> 
> Then you have your full-performance mode that I guess is what these
> patches implement. =)
> 
> Any ideas/suggestions about the low power LCDC modes?
> 
> Thanks,
> 
> / magnus

Thanks again for your feedback,
Damian

Re: [RFC][PATCH 0/3] MERAM support for LCDC

[Damian Hobson-Garcia]

Hi Magnus,

> Please add Runtime PM support to the MERAM driver. The MSTP113 bit of
> SMSTPCR1 should be dynamically controlled using pm_runtime_get_sync()
> and pm_runtime_put_sync().

So one thing that I noticed while adding in the runtime PM support is
that the MERAM bit in RMSTPCR1 seems to be enabled as a power on default
on the chip. (I determined this by dumping the register from u-boot and
poking around in the u-boot code to verify that u-boot doesn't seem to
be enabling this).

Even if pm_runtime_get_sync() and pm_runtime_put_sync() correctly enable
and disable the SMSTPCR1, with the RMSTPCR1 enabled, the clock doesn't
actually turn off.

While I still think that its a good idea to add the
pm_runtime_get_sync() and pm_runtime_put_sync() calls to do their thing
on the SMSTPCR1 side, with the current configuration we won't actually
be saving any power.

Damian

Re: [RFC][PATCH 0/3] MERAM support for LCDC

[Magnus Damm]

Hi Damian,

On Mon, May 9, 2011 at 4:03 PM, Damian Hobson-Garcia
<dhobsong@igel.co.jp> wrote:
> Hi Magnus,
>
>> Please add Runtime PM support to the MERAM driver. The MSTP113 bit of
>> SMSTPCR1 should be dynamically controlled using pm_runtime_get_sync()
>> and pm_runtime_put_sync().
>
> So one thing that I noticed while adding in the runtime PM support is
> that the MERAM bit in RMSTPCR1 seems to be enabled as a power on default
> on the chip. (I determined this by dumping the register from u-boot and
> poking around in the u-boot code to verify that u-boot doesn't seem to
> be enabling this).
>
> Even if pm_runtime_get_sync() and pm_runtime_put_sync() correctly enable
> and disable the SMSTPCR1, with the RMSTPCR1 enabled, the clock doesn't
> actually turn off.
>
> While I still think that its a good idea to add the
> pm_runtime_get_sync() and pm_runtime_put_sync() calls to do their thing
> on the SMSTPCR1 side, with the current configuration we won't actually
> be saving any power.

Good catch, yes, the MSTP bits need to be disabled on both sides for
the clocks to be stopped. I recall the default state of each MSTP bit
to vary somehow. I think we simply should add code to initialize all
SH-side MSTP bits as disabled, that's the easiest.

Thanks,

/ magnus