* Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support
From: Damian @ 2011-03-01 3:13 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1298456210-26519-2-git-send-email-dhobsong@igel.co.jp>
Hi Geert,
On 2011/02/24 15:05, Geert Uytterhoeven wrote:
>>>
>>> My thinking behind separating this out was that I wanted this
>>> define to be accessible from user space. The reason is so that
>>> an application can test the value of .nonstd against the flag to
>>> know for sure if it is dealing with a YUV framebuffer or not.
>>
>> Hm, but ideally we want something standard. How do you know the nonstd
>> flag is working as you expect from user space? All of a sudden you
>> have code that depends on what type of fbdev driver you are using.
>> This is ugly, but abstracting the nonstd interface does not make it
>> better IMO.
>>
>> The nonstd thing is a hack, but for now it is close enough. V4L2 has
>> standard NV12/NV16 support already, but I don't think there is any
>> such thing for fbdev. So i see your patches as a stop-gap, but I
>> really don't want to make it more complicated than it has to be. So
>> exporting nonstd values in a header file to user space seems too
>> complicated IMO.
>>
>> Please just live with the fact that nonstd is special for now. We need
>> to rework the entire LCDC/HDMI/DSI area to support multiple planes and
>> better PM anyway. Perhaps KMS is the way forward, or perhaps Media
>> Controller? Maybe both?
>>
>>> I was under the impression that only headers under include/linux/ should be
>>> accessed from user space, but to be honest, I'm not sure about that.
>>> If that is in fact not the case, then I totally agree that it can go
>>> into include/video/sh_mobile_lcdc.h.
>>
>> Please ditch the SH_FB_YUV constant all together. No need to build
>> some abstraction on top of a hackish interface. Just check if nonstd
>> is non-zero in the driver and assume that means YUV for now. That's
>> good enough.
>
> For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new FB_VISUAL_*
> type instead, which indicates the fb_var_screeninfo.{red,green,blue} fields are
> YCbCr instead of RGB.
> Depending on the frame buffer organization, you also need new FB_TYPE_*/FB_AUX_*
> types.
I just wanted to clarify here. Is your comment about these new flags in
specific reference to this patch or to Magnus' "going forward" comment?
It seems like the beginnings of a method to standardize YCbCr support
in fbdev across all platforms.
Also, do I understand correctly that FB_VISUAL_ would specify the
colorspace (RGB, YCbCr), FB_TYPE_* would be a format specifier (i.e.
planar, semiplanar, interleaved, etc)? I'm not really sure what you are
referring to with the FB_AUX_* however.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks very much,
Damian
^ permalink raw reply
* Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support
From: Geert Uytterhoeven @ 2011-03-01 8:07 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1298456210-26519-2-git-send-email-dhobsong@igel.co.jp>
On Tue, Mar 1, 2011 at 04:13, Damian <dhobsong@igel.co.jp> wrote:
> On 2011/02/24 15:05, Geert Uytterhoeven wrote:
>>>> My thinking behind separating this out was that I wanted this
>>>> define to be accessible from user space. The reason is so that
>>>> an application can test the value of .nonstd against the flag to
>>>> know for sure if it is dealing with a YUV framebuffer or not.
>>>
>>> Hm, but ideally we want something standard. How do you know the nonstd
>>> flag is working as you expect from user space? All of a sudden you
>>> have code that depends on what type of fbdev driver you are using.
>>> This is ugly, but abstracting the nonstd interface does not make it
>>> better IMO.
>>>
>>> The nonstd thing is a hack, but for now it is close enough. V4L2 has
>>> standard NV12/NV16 support already, but I don't think there is any
>>> such thing for fbdev. So i see your patches as a stop-gap, but I
>>> really don't want to make it more complicated than it has to be. So
>>> exporting nonstd values in a header file to user space seems too
>>> complicated IMO.
>>>
>>> Please just live with the fact that nonstd is special for now. We need
>>> to rework the entire LCDC/HDMI/DSI area to support multiple planes and
>>> better PM anyway. Perhaps KMS is the way forward, or perhaps Media
>>> Controller? Maybe both?
>>>
>>>> I was under the impression that only headers under include/linux/ should
>>>> be
>>>> accessed from user space, but to be honest, I'm not sure about that.
>>>> If that is in fact not the case, then I totally agree that it can go
>>>> into include/video/sh_mobile_lcdc.h.
>>>
>>> Please ditch the SH_FB_YUV constant all together. No need to build
>>> some abstraction on top of a hackish interface. Just check if nonstd
>>> is non-zero in the driver and assume that means YUV for now. That's
>>> good enough.
>>
>> For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new
>> FB_VISUAL_*
>> type instead, which indicates the fb_var_screeninfo.{red,green,blue}
>> fields are
>> YCbCr instead of RGB.
>> Depending on the frame buffer organization, you also need new
>> FB_TYPE_*/FB_AUX_*
>> types.
>
> I just wanted to clarify here. Is your comment about these new flags in
> specific reference to this patch or to Magnus' "going forward" comment? It
About new flags.
> seems like the beginnings of a method to standardize YCbCr support in fbdev
> across all platforms.
> Also, do I understand correctly that FB_VISUAL_ would specify the colorspace
FB_VISUAL_* specifies how pixel values (which may be tuples) are mapped to
colors on the screen, so to me it looks like the sensible way to set up YCbCr.
> (RGB, YCbCr), FB_TYPE_* would be a format specifier (i.e. planar,
> semiplanar, interleaved, etc)? I'm not really sure what you are referring
> to with the FB_AUX_* however.
Yep, FB_TYPE_* specifies how pixel values/tuples are laid out in frame buffer
memory.
FB_AUX_* is only used if a specific value of FB_TYPE_* needs an additional
parameter (e.g. the interleave value for interleaved bitplanes).
Another possible use is the offset between the even and odd fields of
the screen,
for hardware that supports interlacing by storing the even and odd
fields separately.
I think I even posted patches for that several years ago... Indeed
http://lkml.indiana.edu/hypermail/linux/kernel/0304.0/0816.html
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support
From: Magnus Damm @ 2011-03-01 8:25 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1298456210-26519-2-git-send-email-dhobsong@igel.co.jp>
Hi Geert,
On Thu, Feb 24, 2011 at 3:05 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, Feb 24, 2011 at 00:28, Magnus Damm <magnus.damm@gmail.com> wrote:
>> Please ditch the SH_FB_YUV constant all together. No need to build
>> some abstraction on top of a hackish interface. Just check if nonstd
>> is non-zero in the driver and assume that means YUV for now. That's
>> good enough.
>
> For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new FB_VISUAL_*
> type instead, which indicates the fb_var_screeninfo.{red,green,blue} fields are
> YCbCr instead of RGB.
> Depending on the frame buffer organization, you also need new FB_TYPE_*/FB_AUX_*
> types.
I'm all for extending the common code instead of hiding code in
drivers. But I wonder how much overlap there is with V4L2 for
instance. I remember adding support for some NVxx formats for V4L2
some years ago. It's mainly used for Video input on Renesas SoCs
though:
http://kerneltrap.org/mailarchive/git-commits-head/2008/12/31/4560474
So I was hoping that something like the above could be added to fbdev
too, but it looks like more code is needed.
Do you have any idea on how to tie in the valid range of each color
channel? The LCDC hardware block can select between 0->255 range or
16->235/240 for the YUV channels. In V4L2 this is handled by
v4l2_colorspace, the 0->255 maps directly to V4L2_COLORSPACE_JPEG.
And how does all this relate to KMS? I'd prefer to keep this code in
one place if possible.
Thanks,
/ magnus
^ permalink raw reply
* [PATCH] omapfb: Fix linker error in drivers/video/omap/lcd_2430sdp.c
From: Jarkko Nikula @ 2011-03-01 8:40 UTC (permalink / raw)
To: linux-fbdev; +Cc: linux-omap, Tomi Valkeinen, Jarkko Nikula
In-Reply-To: <1298905757.9809.135.camel@deskari>
There is a linker error from lcd_2430sdp.c if CONFIG_TWL4030_CORE is not
set. This can be triggered on OMAP2 builds when OMAP3 or OMAP4 are not set.
drivers/built-in.o: In function `sdp2430_panel_disable':
drivers/video/omap/lcd_2430sdp.c:123: undefined reference to `twl_i2c_write_u8'
drivers/video/omap/lcd_2430sdp.c:124: undefined reference to `twl_i2c_write_u8'
drivers/built-in.o: In function `sdp2430_panel_enable':
drivers/video/omap/lcd_2430sdp.c:110: undefined reference to `twl_i2c_write_u8'
drivers/video/omap/lcd_2430sdp.c:112: undefined reference to `twl_i2c_write_u8'
Fix this by selecting the TWL4030_CORE for MACH_OMAP_2430SDP when building
with CONFIG_FB_OMAP as there is no own Kconfig entry for lcd_2430 and it is
compiled always when both MACH_OMAP_2430SDP and FB_OMAP are set.
Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
---
Quite old issue probably. I don't think any fancier fix is required as this
is legacy omapfb code?
---
drivers/video/omap/Kconfig | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/video/omap/Kconfig b/drivers/video/omap/Kconfig
index 083c8fe..c981249 100644
--- a/drivers/video/omap/Kconfig
+++ b/drivers/video/omap/Kconfig
@@ -5,6 +5,7 @@ config FB_OMAP
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
+ select TWL4030_CORE if MACH_OMAP_2430SDP
help
Frame buffer driver for OMAP based boards.
--
1.7.2.3
^ permalink raw reply related
* Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support
From: Magnus Damm @ 2011-03-01 8:59 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1298456210-26519-2-git-send-email-dhobsong@igel.co.jp>
On Thu, Feb 24, 2011 at 12:38 PM, Damian <dhobsong@igel.co.jp> wrote:
> Ok, I see what you're saying. But if the SH_FB_YUV flag is disappearing, I
> guess it makes sense to ditch the second patch in this series as well, since
> that's just further abstraction (albeit locally).
Yep, I think so. Thanks for your help!
/ magnus
^ permalink raw reply
* Re: [PATCH 3/8] Add a mfd IPUv3 driver
From: Sascha Hauer @ 2011-03-01 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201102281811.45277.arnd@arndb.de>
Hi Arnd,
On Mon, Feb 28, 2011 at 06:11:45PM +0100, Arnd Bergmann wrote:
> Hi Sascha,
>
> I've had a brief look around the driver. It looks reasonable in general,
> but the division into subdrivers feels a bit ad-hoc. If all the components
> are built into a single module, I don't see the need for separating the
> data by functional units by file. It seems simple enough to turn this
> into a layered driver with multiple sub-devices each derived from a
> platform_device on its own.
Ok, will do.
>
> On Monday 28 February 2011, Sascha Hauer wrote:
> > arch/arm/plat-mxc/include/mach/ipu-v3.h | 49 +++
> > drivers/video/Kconfig | 2 +
> > drivers/video/Makefile | 1 +
> > drivers/video/imx-ipu-v3/Kconfig | 10 +
> > drivers/video/imx-ipu-v3/Makefile | 3 +
> > drivers/video/imx-ipu-v3/ipu-common.c | 666 +++++++++++++++++++++++++++++++
> > drivers/video/imx-ipu-v3/ipu-cpmem.c | 612 ++++++++++++++++++++++++++++
> > drivers/video/imx-ipu-v3/ipu-dc.c | 364 +++++++++++++++++
> > drivers/video/imx-ipu-v3/ipu-di.c | 550 +++++++++++++++++++++++++
> > drivers/video/imx-ipu-v3/ipu-dmfc.c | 355 ++++++++++++++++
> > drivers/video/imx-ipu-v3/ipu-dp.c | 476 ++++++++++++++++++++++
> > drivers/video/imx-ipu-v3/ipu-prv.h | 216 ++++++++++
> > include/video/imx-ipu-v3.h | 219 ++++++++++
>
> I wonder if this is something that would fit better in drivers/gpu instead
> of drivers/video. We recently discussed the benefits of KMS vs fb drivers,
> and I think it would be good to be prepared for making this a KMS driver
> in the long run, even if the immediate target has to be a simple frame buffer
> driver.
When turning this into a kms driver moving the source code will be the
smallest problem I guess.
I have no preference where to put this code. First it was in
drivers/mfd/ and it felt wrong because there seems to be too much code
in it a mfd maintainer shouldn't be bothered with. drivers/video/ seems
to be wrong because this code will probably support cameras which belong
to drivers/media/video/. So if there's consensus on drivers/gpu/ I will
happily put it there.
What directory do you suggest? drivers/gpu/ or some subdirectory
(drm/vga)?
>
> > +#include "ipu-prv.h"
> > +
> > +static struct clk *ipu_clk;
> > +static struct device *ipu_dev;
> > +
> > +static DEFINE_SPINLOCK(ipu_lock);
> > +static DEFINE_MUTEX(ipu_channel_lock);
> > +void __iomem *ipu_cm_reg;
> > +void __iomem *ipu_idmac_reg;
> > +
> > +static int ipu_use_count;
> > +
> > +static struct ipu_channel channels[64];
>
> Keeping these global limits you to just one ipu, and makes
> understanding the code a little harder. How about moving
> these variables into a struct ipu_data (or similar) that
> is referenced from the platform_device?
>
> > +EXPORT_SYMBOL(ipu_idmac_put);
>
> Why not EXPORT_SYMBOL_GPL?
>
> > +static LIST_HEAD(ipu_irq_handlers);
> > +
> > +static void ipu_irq_update_irq_mask(void)
> > +{
> > + struct ipu_irq_handler *handler;
> > + int i;
> > +
> > + DECLARE_IPU_IRQ_BITMAP(irqs);
> > +
> > + bitmap_zero(irqs, IPU_IRQ_COUNT);
> > +
> > + list_for_each_entry(handler, &ipu_irq_handlers, list)
> > + bitmap_or(irqs, irqs, handler->ipu_irqs, IPU_IRQ_COUNT);
> > +
> > + for (i = 0; i < BITS_TO_LONGS(IPU_IRQ_COUNT); i++)
> > + ipu_cm_write(irqs[i], IPU_INT_CTRL(i + 1));
> > +}
> > +
> > +int ipu_irq_add_handler(struct ipu_irq_handler *ipuirq)
> > +{
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&ipu_lock, flags);
> > +
> > + list_add_tail(&ipuirq->list, &ipu_irq_handlers);
> > + ipu_irq_update_irq_mask();
> > +
> > + spin_unlock_irqrestore(&ipu_lock, flags);
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(ipu_irq_add_handler);
>
> The interrupt logic needs some comments. What are you trying to achieve here?
The IPU has 463 status bits which can trigger an interrupt. Most
of them are associated to channels, some are general interrupts. My
problem is that I don't know how the interrupts will be used by drivers
later. Most drivers will probably use only one interrupt but others
will be interested in a larger group of interrupts. So I decided to
use a bitmap allowing each driver to register for groups of event
it is interested in.
>
> > +int ipu_wait_for_interrupt(int interrupt, int timeout_ms)
> > +{
> > + struct ipu_irq_handler handler;
> > + DECLARE_COMPLETION_ONSTACK(completion);
> > + int ret;
> > +
> > + bitmap_zero(handler.ipu_irqs, IPU_IRQ_COUNT);
> > + bitmap_set(handler.ipu_irqs, interrupt, 1);
> > +
> > + ipu_cm_write(1 << (interrupt % 32), IPU_INT_STAT(interrupt / 32 + 1));
> > +
> > + handler.handler = ipu_completion_handler;
> > + handler.context = &completion;
> > + ipu_irq_add_handler(&handler);
> > +
> > + ret = wait_for_completion_timeout(&completion,
> > + msecs_to_jiffies(timeout_ms));
> > +
> > + ipu_irq_remove_handler(&handler);
> > +
> > + if (ret > 0)
> > + ret = 0;
> > + else
> > + ret = -ETIMEDOUT;
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(ipu_wait_for_interrupt);
>
> If I understand this correctly, this is a very complicated way to implement
> something that could be done with a single wait_event_timeout() and
> wake_up_interruptible_all() from the irq handler.
I added this as a convenience function for a common usecase: wait until
a frame is done or wait until some FIFOs are drained.
I think I can switch this into wait_event_timeout(), but lets first see
if my bitmap-irq implementation survives the review.
>
> > +static irqreturn_t ipu_irq_handler(int irq, void *desc)
> > +{
> > + DECLARE_IPU_IRQ_BITMAP(status);
> > + struct ipu_irq_handler *handler;
> > + int i;
> > +
> > + for (i = 0; i < BITS_TO_LONGS(IPU_IRQ_COUNT); i++) {
> > + status[i] = ipu_cm_read(IPU_INT_STAT(i + 1));
> > + ipu_cm_write(status[i], IPU_INT_STAT(i + 1));
> > + }
> > +
> > + list_for_each_entry(handler, &ipu_irq_handlers, list) {
> > + DECLARE_IPU_IRQ_BITMAP(tmp);
> > + if (bitmap_and(tmp, status, handler->ipu_irqs, IPU_IRQ_COUNT))
> > + handler->handler(tmp, handler->context);
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
>
> This needs to take spin_lock_irqsave before walking the ipu_irq_handlers
> list, in order to prevent another CPU from modifying the list
> while you are in the handler.
ok.
>
> > +static int ipu_reset(void)
> > +{
> > + int timeout = 10000;
> > + u32 val;
> > +
> > + /* hard reset the IPU */
> > + val = readl(MX51_IO_ADDRESS(MX51_SRC_BASE_ADDR));
> > + val |= 1 << 3;
> > + writel(val, MX51_IO_ADDRESS(MX51_SRC_BASE_ADDR));
> > +
> > + ipu_cm_write(0x807FFFFF, IPU_MEM_RST);
> > +
> > + while (ipu_cm_read(IPU_MEM_RST) & 0x80000000) {
> > + if (!timeout--)
> > + return -ETIME;
> > + udelay(100);
> > + }
>
> You have a timeout of over one second with udelay, which
> is not acceptable on many systems. AFAICT, the function
> can sleep, so you can replace udelay with msleep(1), and
> you should use a better logic to determine the end of the
> loop:
>
> unsigned long timeout = jiffies + msecs_to_jiffies(1000);
>
> while (ipu_cm_read(IPU_MEM_RST) & 0x80000000) {
> if (time_after(jiffies, timeout))
> return -ETIME;
> msleep(1);
> }
ok.
>
> > +static u32 *ipu_cpmem_base;
> > +static struct device *ipu_dev;
> > +
> > +struct ipu_ch_param_word {
> > + u32 data[5];
> > + u32 res[3];
> > +};
> > +
> > +struct ipu_ch_param {
> > + struct ipu_ch_param_word word[2];
> > +};
>
> Same comment as for the previous file
> > +
> > +static void __iomem *ipu_dc_reg;
> > +static void __iomem *ipu_dc_tmpl_reg;
> > +static struct device *ipu_dev;
> > +
> > +struct ipu_dc {
> > + unsigned int di; /* The display interface number assigned to this dc channel */
> > + unsigned int channel_offset;
> > +};
> > +
> > +static struct ipu_dc dc_channels[10];
>
> And here again.
>
> > +static void ipu_dc_link_event(int chan, int event, int addr, int priority)
> > +{
> > + u32 reg;
> > +
> > + reg = __raw_readl(DC_RL_CH(chan, event));
> > + reg &= ~(0xFFFF << (16 * (event & 0x1)));
> > + reg |= ((addr << 8) | priority) << (16 * (event & 0x1));
> > + __raw_writel(reg, DC_RL_CH(chan, event));
> > +}
>
> Better use readl/writel instead of __raw_readl/__raw_writel, throughout the
> code.
>
> > +int ipu_di_init(struct platform_device *pdev, int id, unsigned long base,
> > + u32 module, struct clk *ipu_clk);
> > +void ipu_di_exit(struct platform_device *pdev, int id);
> > +
> > +int ipu_dmfc_init(struct platform_device *pdev, unsigned long base,
> > + struct clk *ipu_clk);
> > +void ipu_dmfc_exit(struct platform_device *pdev);
> > +
> > +int ipu_dp_init(struct platform_device *pdev, unsigned long base);
> > +void ipu_dp_exit(struct platform_device *pdev);
> > +
> > +int ipu_dc_init(struct platform_device *pdev, unsigned long base,
> > + unsigned long template_base);
> > +void ipu_dc_exit(struct platform_device *pdev);
> > +
> > +int ipu_cpmem_init(struct platform_device *pdev, unsigned long base);
> > +void ipu_cpmem_exit(struct platform_device *pdev);
>
> If you make the main driver an mfd device, the sub-drivers could become
> real platform drivers, which can structure the layering in a more modular
> way.
> E.g. instead of a single module init function, each subdriver can be
> a module by itself and look at the resources associated with the
> platform device it matches.
ok.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 3/8] Add a mfd IPUv3 driver
From: Sascha Hauer @ 2011-03-01 9:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.00.1102281858240.2701@localhost6.localdomain6>
On Mon, Feb 28, 2011 at 07:33:05PM +0100, Thomas Gleixner wrote:
> On Mon, 28 Feb 2011, Sascha Hauer wrote:
> > +
> > +static int ipu_use_count;
> > +
> > +static struct ipu_channel channels[64];
> > +
> > +struct ipu_channel *ipu_idmac_get(unsigned num)
> > +{
> > + struct ipu_channel *channel;
> > +
> > + dev_dbg(ipu_dev, "%s %d\n", __func__, num);
> > +
> > + if (num > 63)
>
> >= ARRAY_SIZE(channels) or a sensible define please
>
> > + return ERR_PTR(-ENODEV);
> > +
> > + mutex_lock(&ipu_channel_lock);
> > +
> > + channel = &channels[num];
> > +
> > + if (channel->busy) {
> > + channel = ERR_PTR(-EBUSY);
> > + goto out;
> > + }
> > +
> > + channel->busy = 1;
> > + channel->num = num;
> > +
> > +out:
> > + mutex_unlock(&ipu_channel_lock);
> > +
> > + return channel;
> > +}
> > +EXPORT_SYMBOL(ipu_idmac_get);
>
> EXPORT_SYMBOL_GPL all over the place
>
> > +void ipu_idmac_put(struct ipu_channel *channel)
> > +{
> > + dev_dbg(ipu_dev, "%s %d\n", __func__, channel->num);
>
> Do we really need this debug stuff in all these functions ?
Reading this comment I expected tons of dev_dbg in the driver. The one
you mentioned above (plus the corresponding one in ipu_idmac_get) are
indeed not particularly useful, but do you think there is still too much
debug code left?
>
> > + mutex_lock(&ipu_channel_lock);
> > +
> > + channel->busy = 0;
> > +
> > + mutex_unlock(&ipu_channel_lock);
> > +}
> > +EXPORT_SYMBOL(ipu_idmac_put);
> > +
>
> Also exported functions want a proper kerneldoc comment.
>
> > +void ipu_idmac_set_double_buffer(struct ipu_channel *channel, bool doublebuffer)
>
> > +static LIST_HEAD(ipu_irq_handlers);
> > +
> > +static void ipu_irq_update_irq_mask(void)
> > +{
> > + struct ipu_irq_handler *handler;
> > + int i;
> > +
> > + DECLARE_IPU_IRQ_BITMAP(irqs);
>
> Why the hell do we need this? It's a bog standard bitmap, right ?
It's defined as:
#define DECLARE_IPU_IRQ_BITMAP(name) DECLARE_BITMAP(name, IPU_IRQ_COUNT)
So yes, it's a standard bitmask. It can be used in client drivers
aswell. Where's the problem of adding a define for this so that client
drivers do not have to care about the size of the bitmap?
>
> > + bitmap_zero(irqs, IPU_IRQ_COUNT);
> > +
> > + list_for_each_entry(handler, &ipu_irq_handlers, list)
> > + bitmap_or(irqs, irqs, handler->ipu_irqs, IPU_IRQ_COUNT);
> > +
> > + for (i = 0; i < BITS_TO_LONGS(IPU_IRQ_COUNT); i++)
> > + ipu_cm_write(irqs[i], IPU_INT_CTRL(i + 1));
> > +}
>
> > +static void ipu_completion_handler(unsigned long *bitmask, void *context)
> > +{
> > + struct completion *completion = context;
> > +
> > + complete(completion);
> > +}
> > +
> > +int ipu_wait_for_interrupt(int interrupt, int timeout_ms)
> > +{
> > + struct ipu_irq_handler handler;
> > + DECLARE_COMPLETION_ONSTACK(completion);
> > + int ret;
> > +
> > + bitmap_zero(handler.ipu_irqs, IPU_IRQ_COUNT);
> > + bitmap_set(handler.ipu_irqs, interrupt, 1);
> > +
> > + ipu_cm_write(1 << (interrupt % 32), IPU_INT_STAT(interrupt / 32 + 1));
> > +
> > + handler.handler = ipu_completion_handler;
> > + handler.context = &completion;
> > + ipu_irq_add_handler(&handler);
> > +
> > + ret = wait_for_completion_timeout(&completion,
> > + msecs_to_jiffies(timeout_ms));
> > +
> > + ipu_irq_remove_handler(&handler);
> > +
> > + if (ret > 0)
> > + ret = 0;
> > + else
> > + ret = -ETIMEDOUT;
> > +
> > + return ret;
>
> return ret > 0 ? 0 : -ETIMEDOUT;
>
> perhaps ?
ok.
>
>
> > +}
> > +EXPORT_SYMBOL(ipu_wait_for_interrupt);
> > +
> > +static irqreturn_t ipu_irq_handler(int irq, void *desc)
> > +{
> > + DECLARE_IPU_IRQ_BITMAP(status);
> > + struct ipu_irq_handler *handler;
> > + int i;
> > +
> > + for (i = 0; i < BITS_TO_LONGS(IPU_IRQ_COUNT); i++) {
> > + status[i] = ipu_cm_read(IPU_INT_STAT(i + 1));
> > + ipu_cm_write(status[i], IPU_INT_STAT(i + 1));
> > + }
> > +
> > + list_for_each_entry(handler, &ipu_irq_handlers, list) {
> > + DECLARE_IPU_IRQ_BITMAP(tmp);
> > + if (bitmap_and(tmp, status, handler->ipu_irqs, IPU_IRQ_COUNT))
> > + handler->handler(tmp, handler->context);
> > + }
>
> And what protects the list walk? Just the fact that this is a UP
> machine?
Will fix.
>
> > +void ipu_put(void)
> > +{
> > + mutex_lock(&ipu_channel_lock);
> > +
> > + ipu_use_count--;
> > +
> > + if (ipu_use_count = 0)
> > + clk_disable(ipu_clk);
> > +
> > + if (ipu_use_count < 0) {
> > + dev_err(ipu_dev, "ipu use count < 0\n");
>
> This wants to be a WARN_ON(ipu_use_count < 0) so you get some
> information which code is calling this.
yes
>
> > + ipu_use_count = 0;
> > + }
> > +
> > + mutex_unlock(&ipu_channel_lock);
> > +}
>
> > +static int __devinit ipu_probe(struct platform_device *pdev)
> > +{
> > + struct resource *res;
> > + unsigned long ipu_base;
> > + int ret, irq1, irq2;
> > +
> > + /* There can be only one */
> > + if (ipu_dev)
> > + return -EBUSY;
> > +
> > + spin_lock_init(&ipu_lock);
> > +
> > + ipu_dev = &pdev->dev;
> > +
> > + irq1 = platform_get_irq(pdev, 0);
> > + irq2 = platform_get_irq(pdev, 1);
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +
> > + if (!res || irq1 < 0 || irq2 < 0)
> > + return -ENODEV;
> > +
> > + ipu_base = res->start;
> > +
> > + ipu_cm_reg = ioremap(ipu_base + IPU_CM_REG_BASE, PAGE_SIZE);
> > + if (!ipu_cm_reg) {
> > + ret = -ENOMEM;
> > + goto failed_ioremap1;
> > + }
> > +
> > + ipu_idmac_reg = ioremap(ipu_base + IPU_IDMAC_REG_BASE, PAGE_SIZE);
> > + if (!ipu_idmac_reg) {
> > + ret = -ENOMEM;
> > + goto failed_ioremap2;
> > + }
> > +
> > + ipu_clk = clk_get(&pdev->dev, "ipu");
> > + if (IS_ERR(ipu_clk)) {
> > + ret = PTR_ERR(ipu_clk);
> > + dev_err(&pdev->dev, "clk_get failed with %d", ret);
> > + goto failed_clk_get;
> > + }
> > +
> > + ipu_get();
> > +
> > + ret = request_irq(irq1, ipu_irq_handler, IRQF_DISABLED, pdev->name,
> > + &pdev->dev);
>
> s/IRQF_DISABLED/0/ We run all handlers with interrupts disabled
> nowadays.
ok.
>
> > + ret = ipu_submodules_init(pdev, ipu_base, ipu_clk);
> > + if (ret)
> > + goto failed_submodules_init;
> > +
> > + /* Set sync refresh channels as high priority */
> > + ipu_idmac_write(0x18800000, IDMAC_CHA_PRI(0));
>
> Hmm, this random prio setting here is odd.
This is 1:1 from the Freescale Kernel and I never thought about it. We
can remove it and see what happens. Maybe then some day we'll learn
*why* this is done.
>
> > + ret = ipu_add_client_devices(pdev);
> > + if (ret) {
> > + dev_err(&pdev->dev, "adding client devices failed with %d\n", ret);
> > + goto failed_add_clients;
> > + }
>
> White space damage.
>
> > + ret = ipu_wait_for_interrupt(irq, 50);
> > + if (ret)
> > + return;
> > +
> > + /* Wait for DC triple buffer to empty */
> > + if (dc_channels[dc_chan].di = 0)
> > + while ((__raw_readl(DC_STAT) & 0x00000002)
> > + != 0x00000002) {
> > + msleep(2);
> > + timeout -= 2;
> > + if (timeout <= 0)
> > + break;
>
> So we poll stuff which is updated from some other function ?
We poll the DC_STAT register here which is updated from the hardware.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 3/8] Add a mfd IPUv3 driver
From: Thomas Gleixner @ 2011-03-01 10:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110301093956.GL29521@pengutronix.de>
On Tue, 1 Mar 2011, Sascha Hauer wrote:
> On Mon, Feb 28, 2011 at 07:33:05PM +0100, Thomas Gleixner wrote:
> > > +void ipu_idmac_put(struct ipu_channel *channel)
> > > +{
> > > + dev_dbg(ipu_dev, "%s %d\n", __func__, channel->num);
> >
> > Do we really need this debug stuff in all these functions ?
>
> Reading this comment I expected tons of dev_dbg in the driver. The one
> you mentioned above (plus the corresponding one in ipu_idmac_get) are
> indeed not particularly useful, but do you think there is still too much
> debug code left?
Well, I don't see a point in having useless debug stuff around.
> > > + DECLARE_IPU_IRQ_BITMAP(irqs);
> >
> > Why the hell do we need this? It's a bog standard bitmap, right ?
>
> It's defined as:
>
> #define DECLARE_IPU_IRQ_BITMAP(name) DECLARE_BITMAP(name, IPU_IRQ_COUNT)
>
> So yes, it's a standard bitmask. It can be used in client drivers
> aswell. Where's the problem of adding a define for this so that client
> drivers do not have to care about the size of the bitmap?
That's nonsense. You have to know about the size of the bitmap for any
operation on it. Or are you going to provide wrappers around
bitmap_zero() and all other possible bitmap_* functions as well?
> >
> > > + bitmap_zero(irqs, IPU_IRQ_COUNT);
> > > + ret = ipu_submodules_init(pdev, ipu_base, ipu_clk);
> > > + if (ret)
> > > + goto failed_submodules_init;
> > > +
> > > + /* Set sync refresh channels as high priority */
> > > + ipu_idmac_write(0x18800000, IDMAC_CHA_PRI(0));
> >
> > Hmm, this random prio setting here is odd.
>
> This is 1:1 from the Freescale Kernel and I never thought about it. We
> can remove it and see what happens. Maybe then some day we'll learn
> *why* this is done.
Well, the point is to move that to the init function which deals with
IDMAC and not have it at some random place in the code.
> > > + /* Wait for DC triple buffer to empty */
> > > + if (dc_channels[dc_chan].di = 0)
> > > + while ((__raw_readl(DC_STAT) & 0x00000002)
> > > + != 0x00000002) {
> > > + msleep(2);
> > > + timeout -= 2;
> > > + if (timeout <= 0)
> > > + break;
> >
> > So we poll stuff which is updated from some other function ?
>
> We poll the DC_STAT register here which is updated from the hardware.
And there is no interrupt for this ?
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH 3/8] Add a mfd IPUv3 driver
From: Arnd Bergmann @ 2011-03-01 10:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110301091544.GK29521@pengutronix.de>
On Tuesday 01 March 2011, Sascha Hauer wrote:
> When turning this into a kms driver moving the source code will be the
> smallest problem I guess.
> I have no preference where to put this code. First it was in
> drivers/mfd/ and it felt wrong because there seems to be too much code
> in it a mfd maintainer shouldn't be bothered with. drivers/video/ seems
> to be wrong because this code will probably support cameras which belong
> to drivers/media/video/. So if there's consensus on drivers/gpu/ I will
> happily put it there.
> What directory do you suggest? drivers/gpu/ or some subdirectory
> (drm/vga)?
I'd suggest a subdirectory of drivers/gpu/, e.g.
drivers/gpu/embedded/imx-ipu/. Alan is currently adding a driver
for the Intel GMA500, and there are others (TI, ST-Ericsson, ...)
that fit in a similar category of complex graphics subsystems
without an actual GPU core. I think they should all go to the same
place.
> > The interrupt logic needs some comments. What are you trying to achieve here?
>
> The IPU has 463 status bits which can trigger an interrupt. Most
> of them are associated to channels, some are general interrupts. My
> problem is that I don't know how the interrupts will be used by drivers
> later. Most drivers will probably use only one interrupt but others
> will be interested in a larger group of interrupts. So I decided to
> use a bitmap allowing each driver to register for groups of event
> it is interested in.
Ok, I see. So you essentially have a huge nested interrupt controller
and try to be clever about the possible use cases, which may be the
right choice, but apparently you don't know that yet because not
all the drivers have been written at this point.
Taking one step back from this, have you considered making this
a regular interrupt controller? That would make the client drivers
more standard -- you could define the interrupt numbers as resources
of a platform device or in the device tree, for instance.
The cost might be more complex code, e.g. when a device requires
many interrupts, but I think it will be at least as efficient
at run-time, and less surprising for readers and authors of
client drivers.
Arnd
^ permalink raw reply
* Re: [PATCH 3/8] Add a mfd IPUv3 driver
From: Sascha Hauer @ 2011-03-01 11:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201103011127.51605.arnd@arndb.de>
On Tue, Mar 01, 2011 at 11:27:49AM +0100, Arnd Bergmann wrote:
> On Tuesday 01 March 2011, Sascha Hauer wrote:
>
> > When turning this into a kms driver moving the source code will be the
> > smallest problem I guess.
> > I have no preference where to put this code. First it was in
> > drivers/mfd/ and it felt wrong because there seems to be too much code
> > in it a mfd maintainer shouldn't be bothered with. drivers/video/ seems
> > to be wrong because this code will probably support cameras which belong
> > to drivers/media/video/. So if there's consensus on drivers/gpu/ I will
> > happily put it there.
> > What directory do you suggest? drivers/gpu/ or some subdirectory
> > (drm/vga)?
>
> I'd suggest a subdirectory of drivers/gpu/, e.g.
> drivers/gpu/embedded/imx-ipu/. Alan is currently adding a driver
> for the Intel GMA500, and there are others (TI, ST-Ericsson, ...)
> that fit in a similar category of complex graphics subsystems
> without an actual GPU core. I think they should all go to the same
> place.
>
> > > The interrupt logic needs some comments. What are you trying to achieve here?
> >
> > The IPU has 463 status bits which can trigger an interrupt. Most
> > of them are associated to channels, some are general interrupts. My
> > problem is that I don't know how the interrupts will be used by drivers
> > later. Most drivers will probably use only one interrupt but others
> > will be interested in a larger group of interrupts. So I decided to
> > use a bitmap allowing each driver to register for groups of event
> > it is interested in.
>
> Ok, I see. So you essentially have a huge nested interrupt controller
> and try to be clever about the possible use cases, which may be the
> right choice, but apparently you don't know that yet because not
> all the drivers have been written at this point.
>
> Taking one step back from this, have you considered making this
> a regular interrupt controller? That would make the client drivers
> more standard -- you could define the interrupt numbers as resources
> of a platform device or in the device tree, for instance.
> The cost might be more complex code, e.g. when a device requires
> many interrupts, but I think it will be at least as efficient
> at run-time, and less surprising for readers and authors of
> client drivers.
I thought about this, but hesitated to increase NR_IRQS by 463. Do you
think we should do this instead?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 3/8] Add a mfd IPUv3 driver
From: Sascha Hauer @ 2011-03-01 11:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.00.1103011054440.2701@localhost6.localdomain6>
On Tue, Mar 01, 2011 at 11:00:09AM +0100, Thomas Gleixner wrote:
> On Tue, 1 Mar 2011, Sascha Hauer wrote:
> > On Mon, Feb 28, 2011 at 07:33:05PM +0100, Thomas Gleixner wrote:
> > > > +void ipu_idmac_put(struct ipu_channel *channel)
> > > > +{
> > > > + dev_dbg(ipu_dev, "%s %d\n", __func__, channel->num);
> > >
> > > Do we really need this debug stuff in all these functions ?
> >
> > Reading this comment I expected tons of dev_dbg in the driver. The one
> > you mentioned above (plus the corresponding one in ipu_idmac_get) are
> > indeed not particularly useful, but do you think there is still too much
> > debug code left?
>
> Well, I don't see a point in having useless debug stuff around.
> > > > + DECLARE_IPU_IRQ_BITMAP(irqs);
> > >
> > > Why the hell do we need this? It's a bog standard bitmap, right ?
> >
> > It's defined as:
> >
> > #define DECLARE_IPU_IRQ_BITMAP(name) DECLARE_BITMAP(name, IPU_IRQ_COUNT)
> >
> > So yes, it's a standard bitmask. It can be used in client drivers
> > aswell. Where's the problem of adding a define for this so that client
> > drivers do not have to care about the size of the bitmap?
>
> That's nonsense. You have to know about the size of the bitmap for any
> operation on it. Or are you going to provide wrappers around
> bitmap_zero() and all other possible bitmap_* functions as well?
Ok, you're right.
>
> > >
> > > > + bitmap_zero(irqs, IPU_IRQ_COUNT);
> > > > + ret = ipu_submodules_init(pdev, ipu_base, ipu_clk);
> > > > + if (ret)
> > > > + goto failed_submodules_init;
> > > > +
> > > > + /* Set sync refresh channels as high priority */
> > > > + ipu_idmac_write(0x18800000, IDMAC_CHA_PRI(0));
> > >
> > > Hmm, this random prio setting here is odd.
> >
> > This is 1:1 from the Freescale Kernel and I never thought about it. We
> > can remove it and see what happens. Maybe then some day we'll learn
> > *why* this is done.
>
> Well, the point is to move that to the init function which deals with
> IDMAC and not have it at some random place in the code.
Ok, will move it there.
>
> > > > + /* Wait for DC triple buffer to empty */
> > > > + if (dc_channels[dc_chan].di = 0)
> > > > + while ((__raw_readl(DC_STAT) & 0x00000002)
> > > > + != 0x00000002) {
> > > > + msleep(2);
> > > > + timeout -= 2;
> > > > + if (timeout <= 0)
> > > > + break;
> > >
> > > So we poll stuff which is updated from some other function ?
> >
> > We poll the DC_STAT register here which is updated from the hardware.
>
> And there is no interrupt for this ?
Given the sheer amount of interrupt bits it's a bit surprising, but no,
I haven't found an interrupt for this (This of course doesn't mean it
doesn't exist...)
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 3/8] Add a mfd IPUv3 driver
From: Arnd Bergmann @ 2011-03-01 11:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110301111220.GN29521@pengutronix.de>
On Tuesday 01 March 2011, Sascha Hauer wrote:
> > Taking one step back from this, have you considered making this
> > a regular interrupt controller? That would make the client drivers
> > more standard -- you could define the interrupt numbers as resources
> > of a platform device or in the device tree, for instance.
> > The cost might be more complex code, e.g. when a device requires
> > many interrupts, but I think it will be at least as efficient
> > at run-time, and less surprising for readers and authors of
> > client drivers.
>
> I thought about this, but hesitated to increase NR_IRQS by 463. Do you
> think we should do this instead?
I think there is a plan to virtualize the interrupt numbers on ARM,
and in that case NR_IRQS becomes rather meaningless. I don't know
exactly how far that effort has come.
Arnd
^ permalink raw reply
* [RFC/PATCH 0/6] JZ4740: Add SLCD support
From: Maurus Cuelenaere @ 2011-03-01 12:05 UTC (permalink / raw)
To: linux-fbdev
This patch series adds SLCD support to the Jz4740 framebuffer driver and adds a
LCD panel driver for the Truly G240400RTSW using the newly-exported SLCD data
transmission functions.
I'm not sure about the way I've handled this (jz4740_fb_slcd_*), so I marked
the relevant patches as RFC. This could be done by emulating SPI, but as this
seemed too much work for little benefit I picked the lazy way out.
Maurus Cuelenaere (6):
FBDEV: JZ4740: Refactor GPIO pin operations
FBDEV: JZ4740: Initialize jzfb->fb earlier
FBDEV: JZ4740: Initialize framebuffer properly
FBDEV: JZ4740: Add Smart LCD controller support
FBDEV: JZ4740: Get rid of enumeration value not handled in switch
warnings
backlight: Add Truly G240400RTSW LCD panel driver
arch/mips/include/asm/mach-jz4740/jz4740_fb.h | 61 +++-
drivers/video/backlight/Kconfig | 7 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/truly_g240400rtsw.c | 464 ++++++++++++++++++++++
drivers/video/jz4740_fb.c | 524 +++++++++++++++++++++----
include/video/truly_g240400rtsw.h | 35 ++
6 files changed, 990 insertions(+), 102 deletions(-)
create mode 100644 drivers/video/backlight/truly_g240400rtsw.c
create mode 100644 include/video/truly_g240400rtsw.h
--
1.7.4
^ permalink raw reply
* [PATCH 1/6] FBDEV: JZ4740: Refactor GPIO pin operations
From: Maurus Cuelenaere @ 2011-03-01 12:05 UTC (permalink / raw)
To: linux-fbdev
Encapsulate all GPIO pins related operations into 1 function which'll make it
easier to add on SLCD support.
Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
---
drivers/video/jz4740_fb.c | 82 +++++++++++++++++++++-----------------------
1 files changed, 39 insertions(+), 43 deletions(-)
diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
index 7e61e68..65741d0 100644
--- a/drivers/video/jz4740_fb.c
+++ b/drivers/video/jz4740_fb.c
@@ -178,59 +178,60 @@ static const struct jz_gpio_bulk_request jz_lcd_data_pins[] = {
JZ_GPIO_BULK_PIN(LCD_DATA17),
};
-static unsigned int jzfb_num_ctrl_pins(struct jzfb *jzfb)
+enum jzfb_pin_operation {
+ REQUEST_PINS,
+ FREE_PINS,
+ RESUME_PINS,
+ SUSPEND_PINS,
+};
+
+static void jzfb_pins_operation(struct jzfb *jzfb,
+ enum jzfb_pin_operation operation)
{
- unsigned int num;
+ unsigned int ctrl_num = 0, data_num = 0, data_start = 0;
switch (jzfb->pdata->lcd_type) {
case JZ_LCD_TYPE_GENERIC_16_BIT:
- num = 4;
+ ctrl_num = 4;
+ data_num = 16;
break;
case JZ_LCD_TYPE_GENERIC_18_BIT:
- num = 4;
+ ctrl_num = 4;
+ data_num = 18;
break;
case JZ_LCD_TYPE_8BIT_SERIAL:
- num = 3;
+ ctrl_num = 3;
+ data_num = 8;
break;
case JZ_LCD_TYPE_SPECIAL_TFT_1:
case JZ_LCD_TYPE_SPECIAL_TFT_2:
case JZ_LCD_TYPE_SPECIAL_TFT_3:
- num = 8;
- break;
- default:
- num = 0;
+ ctrl_num = 8;
+ if (jzfb->pdata->bpp = 18)
+ data_num = 18;
+ else
+ data_num = 16;
break;
}
- return num;
-}
-static unsigned int jzfb_num_data_pins(struct jzfb *jzfb)
-{
- unsigned int num;
-
- switch (jzfb->pdata->lcd_type) {
- case JZ_LCD_TYPE_GENERIC_16_BIT:
- num = 16;
+ switch (operation) {
+ case REQUEST_PINS:
+ jz_gpio_bulk_request(jz_lcd_ctrl_pins, ctrl_num);
+ jz_gpio_bulk_request(&jz_lcd_data_pins[data_start], data_num);
break;
- case JZ_LCD_TYPE_GENERIC_18_BIT:
- num = 18;
+ case FREE_PINS:
+ jz_gpio_bulk_free(jz_lcd_ctrl_pins, ctrl_num);
+ jz_gpio_bulk_free(&jz_lcd_data_pins[data_start], data_num);
break;
- case JZ_LCD_TYPE_8BIT_SERIAL:
- num = 8;
- break;
- case JZ_LCD_TYPE_SPECIAL_TFT_1:
- case JZ_LCD_TYPE_SPECIAL_TFT_2:
- case JZ_LCD_TYPE_SPECIAL_TFT_3:
- if (jzfb->pdata->bpp = 18)
- num = 18;
- else
- num = 16;
+ case RESUME_PINS:
+ jz_gpio_bulk_resume(jz_lcd_ctrl_pins, ctrl_num);
+ jz_gpio_bulk_resume(&jz_lcd_data_pins[data_start], data_num);
break;
- default:
- num = 0;
+ case SUSPEND_PINS:
+ jz_gpio_bulk_suspend(jz_lcd_ctrl_pins, ctrl_num);
+ jz_gpio_bulk_suspend(&jz_lcd_data_pins[data_start], data_num);
break;
}
- return num;
}
/* Based on CNVT_TOHW macro from skeletonfb.c */
@@ -487,8 +488,7 @@ static void jzfb_enable(struct jzfb *jzfb)
clk_enable(jzfb->ldclk);
- jz_gpio_bulk_resume(jz_lcd_ctrl_pins, jzfb_num_ctrl_pins(jzfb));
- jz_gpio_bulk_resume(jz_lcd_data_pins, jzfb_num_data_pins(jzfb));
+ jzfb_pins_operation(jzfb, RESUME_PINS);
writel(0, jzfb->base + JZ_REG_LCD_STATE);
@@ -511,8 +511,7 @@ static void jzfb_disable(struct jzfb *jzfb)
ctrl = readl(jzfb->base + JZ_REG_LCD_STATE);
} while (!(ctrl & JZ_LCD_STATE_DISABLED));
- jz_gpio_bulk_suspend(jz_lcd_ctrl_pins, jzfb_num_ctrl_pins(jzfb));
- jz_gpio_bulk_suspend(jz_lcd_data_pins, jzfb_num_data_pins(jzfb));
+ jzfb_pins_operation(jzfb, SUSPEND_PINS);
clk_disable(jzfb->ldclk);
}
@@ -715,8 +714,7 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
fb->mode = NULL;
jzfb_set_par(fb);
- jz_gpio_bulk_request(jz_lcd_ctrl_pins, jzfb_num_ctrl_pins(jzfb));
- jz_gpio_bulk_request(jz_lcd_data_pins, jzfb_num_data_pins(jzfb));
+ jzfb_pins_operation(jzfb, REQUEST_PINS);
ret = register_framebuffer(fb);
if (ret) {
@@ -729,8 +727,7 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
return 0;
err_free_devmem:
- jz_gpio_bulk_free(jz_lcd_ctrl_pins, jzfb_num_ctrl_pins(jzfb));
- jz_gpio_bulk_free(jz_lcd_data_pins, jzfb_num_data_pins(jzfb));
+ jzfb_pins_operation(jzfb, FREE_PINS);
fb_dealloc_cmap(&fb->cmap);
jzfb_free_devmem(jzfb);
@@ -753,8 +750,7 @@ static int __devexit jzfb_remove(struct platform_device *pdev)
jzfb_blank(FB_BLANK_POWERDOWN, jzfb->fb);
- jz_gpio_bulk_free(jz_lcd_ctrl_pins, jzfb_num_ctrl_pins(jzfb));
- jz_gpio_bulk_free(jz_lcd_data_pins, jzfb_num_data_pins(jzfb));
+ jzfb_pins_operation(jzfb, FREE_PINS);
iounmap(jzfb->base);
release_mem_region(jzfb->mem->start, resource_size(jzfb->mem));
--
1.7.4
^ permalink raw reply related
* [PATCH 2/6] FBDEV: JZ4740: Initialize jzfb->fb earlier
From: Maurus Cuelenaere @ 2011-03-01 12:05 UTC (permalink / raw)
To: linux-fbdev
There's no reason to not do this, and this is needed in the upcoming SLCD
support patches.
Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
---
drivers/video/jz4740_fb.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
index 65741d0..cea7b7e 100644
--- a/drivers/video/jz4740_fb.c
+++ b/drivers/video/jz4740_fb.c
@@ -654,6 +654,7 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
fb->flags = FBINFO_DEFAULT;
jzfb = fb->par;
+ jzfb->fb = fb;
jzfb->pdev = pdev;
jzfb->pdata = pdata;
jzfb->mem = mem;
@@ -722,8 +723,6 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
goto err_free_devmem;
}
- jzfb->fb = fb;
-
return 0;
err_free_devmem:
--
1.7.4
^ permalink raw reply related
* [PATCH 3/6] FBDEV: JZ4740: Initialize framebuffer properly
From: Maurus Cuelenaere @ 2011-03-01 12:06 UTC (permalink / raw)
To: linux-fbdev
Use the jzfb_enable() function instead of manually faking it in jzfb_probe().
Also add the reverse operation when the framebuffer couldn't be registered.
Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
---
drivers/video/jz4740_fb.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
index cea7b7e..2f3ea57 100644
--- a/drivers/video/jz4740_fb.c
+++ b/drivers/video/jz4740_fb.c
@@ -707,16 +707,13 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
fb_alloc_cmap(&fb->cmap, 256, 0);
- clk_enable(jzfb->ldclk);
- jzfb->is_enabled = 1;
-
- writel(jzfb->framedesc->next, jzfb->base + JZ_REG_LCD_DA0);
-
fb->mode = NULL;
jzfb_set_par(fb);
jzfb_pins_operation(jzfb, REQUEST_PINS);
+ jzfb_blank(FB_BLANK_UNBLANK, fb);
+
ret = register_framebuffer(fb);
if (ret) {
dev_err(&pdev->dev, "Failed to register framebuffer: %d\n", ret);
@@ -726,6 +723,7 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
return 0;
err_free_devmem:
+ jzfb_blank(FB_BLANK_POWERDOWN, fb);
jzfb_pins_operation(jzfb, FREE_PINS);
fb_dealloc_cmap(&fb->cmap);
--
1.7.4
^ permalink raw reply related
* [RFC/PATCH 4/6] FBDEV: JZ4740: Add Smart LCD controller support
From: Maurus Cuelenaere @ 2011-03-01 12:06 UTC (permalink / raw)
To: linux-fbdev
This patch adds SLCD support to the Ingenic Jz4740 framebuffer driver.
Besides adding the new LCD types to the platform data, this adds chip select
and register select active low support (SLCD only). Also, this exports some
functions related to starting and stopping the SLCD image transfer and sending
commands.
A lot of this code was based on work by Maarten ter Huurne:
https://github.com/mthuurne/opendingux-kernel/commit/3dba5b5e7a868b1067acae8d1b5a19121b77bc6a
Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
---
arch/mips/include/asm/mach-jz4740/jz4740_fb.h | 33 ++
drivers/video/jz4740_fb.c | 435 ++++++++++++++++++++++---
2 files changed, 429 insertions(+), 39 deletions(-)
diff --git a/arch/mips/include/asm/mach-jz4740/jz4740_fb.h b/arch/mips/include/asm/mach-jz4740/jz4740_fb.h
index 6a50e6f..eaaac43 100644
--- a/arch/mips/include/asm/mach-jz4740/jz4740_fb.h
+++ b/arch/mips/include/asm/mach-jz4740/jz4740_fb.h
@@ -30,6 +30,13 @@ enum jz4740_fb_lcd_type {
JZ_LCD_TYPE_DUAL_COLOR_STN = 10,
JZ_LCD_TYPE_DUAL_MONOCHROME_STN = 11,
JZ_LCD_TYPE_8BIT_SERIAL = 12,
+
+ JZ_SLCD_TYPE_PARALLEL_8_BIT = 1 | (1 << 5),
+ JZ_SLCD_TYPE_PARALLEL_16_BIT = 0 | (1 << 5),
+ JZ_SLCD_TYPE_PARALLEL_18_BIT = 2 | (1 << 5),
+ JZ_SLCD_TYPE_SERIAL_8_BIT = 1 | (3 << 5),
+ JZ_SLCD_TYPE_SERIAL_16_BIT = 0 | (3 << 5),
+ JZ_SLCD_TYPE_SERIAL_18_BIT = 2 | (3 << 5),
};
#define JZ4740_FB_SPECIAL_TFT_CONFIG(start, stop) (((start) << 16) | (stop))
@@ -62,6 +69,32 @@ struct jz4740_fb_platform_data {
unsigned pixclk_falling_edge:1;
unsigned date_enable_active_low:1;
+ unsigned chip_select_active_low:1;
+ unsigned register_select_active_low:1;
};
+struct platform_device;
+
+/*
+ * JzFB SLCD related functions
+ *
+ * jz4740_fb_slcd_disable_transfer: disables the current image transfer going
+ * on between memory and the LCD controller
+ * jz4740_fb_slcd_enable_transfer: the reverse operation of the above
+ * jz4740_fb_slcd_send_cmd: sends a command without any data to the LCD
+ * controller
+ * jz4740_fb_slcd_send_cmd_data: sends a command with a data argument to the LCD
+ * controller
+ *
+ * These functions can sleep and thus should not be called from an atomic
+ * context. Also, make sure you disable the SLCD image transfer *before* sending
+ * any commands and do not forget to re-enable it.
+ */
+extern void jz4740_fb_slcd_disable_transfer(struct platform_device *pdev);
+extern void jz4740_fb_slcd_enable_transfer(struct platform_device *pdev);
+extern void jz4740_fb_slcd_send_cmd_data(struct platform_device *pdev,
+ unsigned int cmd, unsigned int data);
+extern void jz4740_fb_slcd_send_cmd(struct platform_device *pdev,
+ unsigned int cmd);
+
#endif
diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
index 2f3ea57..4064812 100644
--- a/drivers/video/jz4740_fb.c
+++ b/drivers/video/jz4740_fb.c
@@ -26,6 +26,7 @@
#include <linux/dma-mapping.h>
+#include <asm/mach-jz4740/dma.h>
#include <asm/mach-jz4740/jz4740_fb.h>
#include <asm/mach-jz4740/gpio.h>
@@ -107,6 +108,40 @@
#define JZ_LCD_STATE_DISABLED BIT(0)
+#define JZ_REG_SLCD_CFG 0xA0
+#define JZ_REG_SLCD_CTRL 0xA4
+#define JZ_REG_SLCD_STATE 0xA8
+#define JZ_REG_SLCD_DATA 0xAC
+#define JZ_REG_SLCD_FIFO 0xB0
+
+#define JZ_SLCD_CFG_BURST_8_WORD BIT(14)
+#define JZ_SLCD_CFG_DWIDTH_MASK (7 << 10)
+#define JZ_SLCD_CFG_DWIDTH_18 (0 << 10)
+#define JZ_SLCD_CFG_DWIDTH_16 (1 << 10)
+#define JZ_SLCD_CFG_DWIDTH_8_x3 (2 << 10)
+#define JZ_SLCD_CFG_DWIDTH_8_x2 (3 << 10)
+#define JZ_SLCD_CFG_DWIDTH_8_x1 (4 << 10)
+#define JZ_SLCD_CFG_DWIDTH_9_x2 (7 << 10)
+#define JZ_SLCD_CFG_CWIDTH_MASK (3 << 8)
+#define JZ_SLCD_CFG_CWIDTH(n) ((n) << 8)
+#define JZ_SLCD_CFG_CWIDTH_16BIT (0 << 8)
+#define JZ_SLCD_CFG_CWIDTH_8BIT (1 << 8)
+#define JZ_SLCD_CFG_CWIDTH_18BIT (2 << 8)
+#define JZ_SLCD_CFG_CS_ACTIVE_HIGH BIT(4)
+#define JZ_SLCD_CFG_RS_CMD_HIGH BIT(3)
+#define JZ_SLCD_CFG_CLK_ACTIVE_RISING BIT(1)
+#define JZ_SLCD_CFG_TYPE_SERIAL BIT(0)
+
+#define JZ_SLCD_CTRL_DMA_EN (1 << 0)
+
+#define JZ_SLCD_STATE_BUSY (1 << 0)
+
+#define JZ_SLCD_DATA_RS_DATA (0 << 31)
+#define JZ_SLCD_DATA_RS_COMMAND (1 << 31)
+
+#define JZ_SLCD_FIFO_RS_DATA (0 << 31)
+#define JZ_SLCD_FIFO_RS_COMMAND (1 << 31)
+
struct jzfb_framedesc {
uint32_t next;
uint32_t addr;
@@ -118,6 +153,7 @@ struct jzfb {
struct fb_info *fb;
struct platform_device *pdev;
void __iomem *base;
+ phys_t phys_base;
struct resource *mem;
struct jz4740_fb_platform_data *pdata;
@@ -126,6 +162,7 @@ struct jzfb {
dma_addr_t vidmem_phys;
struct jzfb_framedesc *framedesc;
dma_addr_t framedesc_phys;
+ struct jz4740_dma_chan *slcd_dma;
struct clk *ldclk;
struct clk *lpclk;
@@ -136,6 +173,9 @@ struct jzfb {
uint32_t pseudo_palette[16];
};
+#define JZFB_IS_SLCD(jzfb) ((jzfb)->pdata->lcd_type & (1 << 5))
+#define JZFB_IS_SLCD_SERIAL_TYPE(jzfb) ((jzfb)->pdata->lcd_type & (2 << 5))
+
static const struct fb_fix_screeninfo jzfb_fix __devinitdata = {
.id = "JZ4740 FB",
.type = FB_TYPE_PACKED_PIXELS,
@@ -192,14 +232,17 @@ static void jzfb_pins_operation(struct jzfb *jzfb,
switch (jzfb->pdata->lcd_type) {
case JZ_LCD_TYPE_GENERIC_16_BIT:
+ case JZ_SLCD_TYPE_PARALLEL_16_BIT:
ctrl_num = 4;
data_num = 16;
break;
case JZ_LCD_TYPE_GENERIC_18_BIT:
+ case JZ_SLCD_TYPE_PARALLEL_18_BIT:
ctrl_num = 4;
data_num = 18;
break;
case JZ_LCD_TYPE_8BIT_SERIAL:
+ case JZ_SLCD_TYPE_PARALLEL_8_BIT:
ctrl_num = 3;
data_num = 8;
break;
@@ -212,8 +255,17 @@ static void jzfb_pins_operation(struct jzfb *jzfb,
else
data_num = 16;
break;
+ case JZ_SLCD_TYPE_SERIAL_8_BIT:
+ case JZ_SLCD_TYPE_SERIAL_16_BIT:
+ case JZ_SLCD_TYPE_SERIAL_18_BIT:
+ data_start = 15;
+ data_num = 1;
+ break;
}
+ if (JZFB_IS_SLCD(jzfb))
+ ctrl_num = 3;
+
switch (operation) {
case REQUEST_PINS:
jz_gpio_bulk_request(jz_lcd_ctrl_pins, ctrl_num);
@@ -348,12 +400,9 @@ static int jzfb_check_var(struct fb_var_screeninfo *var, struct fb_info *fb)
return 0;
}
-static int jzfb_set_par(struct fb_info *info)
+static void jzfb_lcd_set_par(struct jzfb *jzfb, struct fb_videomode *mode)
{
- struct jzfb *jzfb = info->par;
struct jz4740_fb_platform_data *pdata = jzfb->pdata;
- struct fb_var_screeninfo *var = &info->var;
- struct fb_videomode *mode;
uint16_t hds, vds;
uint16_t hde, vde;
uint16_t ht, vt;
@@ -361,15 +410,6 @@ static int jzfb_set_par(struct fb_info *info)
uint32_t cfg;
unsigned long rate;
- mode = jzfb_get_mode(jzfb, var);
- if (mode = NULL)
- return -EINVAL;
-
- if (mode = info->mode)
- return 0;
-
- info->mode = mode;
-
hds = mode->hsync_len + mode->left_margin;
hde = hds + mode->xres;
ht = hde + mode->right_margin;
@@ -478,10 +518,154 @@ static int jzfb_set_par(struct fb_info *info)
clk_set_rate(jzfb->lpclk, rate);
clk_set_rate(jzfb->ldclk, rate * 3);
+}
+
+static void jzfb_slcd_set_par(struct jzfb *jzfb, struct fb_videomode *mode)
+{
+ struct jz4740_fb_platform_data *pdata = jzfb->pdata;
+ uint32_t cfg;
+ unsigned long rate;
+
+ cfg = JZ_SLCD_CFG_BURST_8_WORD;
+ cfg |= JZ_SLCD_CFG_CWIDTH(jzfb->pdata->lcd_type & 3);
+
+ if (JZFB_IS_SLCD_SERIAL_TYPE(jzfb)) {
+ switch (jzfb->pdata->lcd_type) {
+ case JZ_SLCD_TYPE_SERIAL_8_BIT:
+ cfg |= JZ_SLCD_CFG_DWIDTH_8_x1;
+ break;
+ case JZ_SLCD_TYPE_SERIAL_16_BIT:
+ cfg |= JZ_SLCD_CFG_DWIDTH_16;
+ break;
+ case JZ_SLCD_TYPE_SERIAL_18_BIT:
+ cfg |= JZ_SLCD_CFG_DWIDTH_18;
+ break;
+ }
+ cfg |= JZ_SLCD_CFG_TYPE_SERIAL;
+ } else {
+ switch (jzfb->pdata->bpp) {
+ case 8:
+ cfg |= JZ_SLCD_CFG_DWIDTH_8_x1;
+ break;
+ case 15:
+ case 16:
+ switch (jzfb->pdata->lcd_type) {
+ case JZ_SLCD_TYPE_PARALLEL_8_BIT:
+ cfg |= JZ_SLCD_CFG_DWIDTH_8_x2;
+ break;
+ case JZ_SLCD_TYPE_PARALLEL_16_BIT:
+ case JZ_SLCD_TYPE_PARALLEL_18_BIT:
+ cfg |= JZ_SLCD_CFG_DWIDTH_16;
+ break;
+ }
+ break;
+ case 18:
+ switch (jzfb->pdata->lcd_type) {
+ case JZ_SLCD_TYPE_PARALLEL_8_BIT:
+ cfg |= JZ_SLCD_CFG_DWIDTH_8_x3;
+ break;
+ case JZ_SLCD_TYPE_PARALLEL_16_BIT:
+ cfg |= JZ_SLCD_CFG_DWIDTH_9_x2;
+ break;
+ case JZ_SLCD_TYPE_PARALLEL_18_BIT:
+ cfg |= JZ_SLCD_CFG_DWIDTH_18;
+ break;
+ }
+ break;
+ }
+ }
+
+ if (!pdata->pixclk_falling_edge)
+ cfg |= JZ_SLCD_CFG_CLK_ACTIVE_RISING;
+
+ if (!pdata->chip_select_active_low)
+ cfg |= JZ_SLCD_CFG_CS_ACTIVE_HIGH;
+
+ if (!pdata->register_select_active_low)
+ cfg |= JZ_SLCD_CFG_RS_CMD_HIGH;
+
+ if (mode->pixclock) {
+ rate = PICOS2KHZ(mode->pixclock) * 1000;
+ mode->refresh = rate;
+ } else {
+ rate = mode->refresh;
+ mode->pixclock = KHZ2PICOS(rate / 1000);
+ }
+
+ mutex_lock(&jzfb->lock);
+ if (!jzfb->is_enabled)
+ clk_enable(jzfb->ldclk);
+
+ writel(JZ_LCD_CFG_SLCD, jzfb->base + JZ_REG_LCD_CFG);
+ writel(cfg, jzfb->base + JZ_REG_SLCD_CFG);
+ writel(0, jzfb->base + JZ_REG_SLCD_CTRL);
+
+ if (!jzfb->is_enabled)
+ clk_disable(jzfb->ldclk);
+ mutex_unlock(&jzfb->lock);
+
+ clk_set_rate(jzfb->lpclk, rate);
+ clk_set_rate(jzfb->ldclk, rate * 3);
+}
+
+static int jzfb_set_par(struct fb_info *info)
+{
+ struct jzfb *jzfb = info->par;
+ struct fb_var_screeninfo *var = &info->var;
+ struct fb_videomode *mode;
+
+ mode = jzfb_get_mode(jzfb, var);
+ if (mode = NULL)
+ return -EINVAL;
+
+ if (mode = info->mode)
+ return 0;
+
+ info->mode = mode;
+
+ if (JZFB_IS_SLCD(jzfb))
+ jzfb_slcd_set_par(jzfb, mode);
+ else
+ jzfb_lcd_set_par(jzfb, mode);
return 0;
}
+static void jzfb_slcd_wait(struct jzfb *jzfb)
+{
+ int timeout = 1000;
+ while (readb(jzfb->base + JZ_REG_SLCD_STATE) & JZ_SLCD_STATE_BUSY
+ && timeout--)
+ cpu_relax();
+
+ if (timeout <= 0)
+ dev_warn(&jzfb->pdev->dev, "waiting for SLCD busy timed out!");
+}
+
+static void jzfb_slcd_start_dma(struct jzfb *jzfb)
+{
+ struct fb_info *fb = jzfb->fb;
+ unsigned int length = fb->fix.line_length * fb->mode->yres;
+
+ jzfb_slcd_wait(jzfb);
+
+ jz4740_dma_set_src_addr(jzfb->slcd_dma, jzfb->vidmem_phys);
+ jz4740_dma_set_dst_addr(jzfb->slcd_dma,
+ jzfb->phys_base + JZ_REG_SLCD_FIFO);
+ jz4740_dma_set_transfer_count(jzfb->slcd_dma, length);
+
+ jz4740_dma_enable(jzfb->slcd_dma);
+}
+
+static void jzfb_slcd_dma_callback(struct jz4740_dma_chan *chan, int unk,
+ void *dev)
+{
+ struct jzfb *jzfb = dev;
+
+ /* TODO: use DMA descriptors! */
+ jzfb_slcd_start_dma(jzfb);
+}
+
static void jzfb_enable(struct jzfb *jzfb)
{
uint32_t ctrl;
@@ -489,28 +673,40 @@ static void jzfb_enable(struct jzfb *jzfb)
clk_enable(jzfb->ldclk);
jzfb_pins_operation(jzfb, RESUME_PINS);
+ if (JZFB_IS_SLCD(jzfb)) {
+ jzfb_slcd_wait(jzfb);
+ writeb(JZ_SLCD_CTRL_DMA_EN, jzfb->base + JZ_REG_SLCD_CTRL);
- writel(0, jzfb->base + JZ_REG_LCD_STATE);
+ jzfb_slcd_start_dma(jzfb);
+ } else {
+ writel(0, jzfb->base + JZ_REG_LCD_STATE);
- writel(jzfb->framedesc->next, jzfb->base + JZ_REG_LCD_DA0);
+ writel(jzfb->framedesc->next, jzfb->base + JZ_REG_LCD_DA0);
- ctrl = readl(jzfb->base + JZ_REG_LCD_CTRL);
- ctrl |= JZ_LCD_CTRL_ENABLE;
- ctrl &= ~JZ_LCD_CTRL_DISABLE;
- writel(ctrl, jzfb->base + JZ_REG_LCD_CTRL);
+ ctrl = readl(jzfb->base + JZ_REG_LCD_CTRL);
+ ctrl |= JZ_LCD_CTRL_ENABLE;
+ ctrl &= ~JZ_LCD_CTRL_DISABLE;
+ writel(ctrl, jzfb->base + JZ_REG_LCD_CTRL);
+ }
}
static void jzfb_disable(struct jzfb *jzfb)
{
uint32_t ctrl;
- ctrl = readl(jzfb->base + JZ_REG_LCD_CTRL);
- ctrl |= JZ_LCD_CTRL_DISABLE;
- writel(ctrl, jzfb->base + JZ_REG_LCD_CTRL);
- do {
- ctrl = readl(jzfb->base + JZ_REG_LCD_STATE);
- } while (!(ctrl & JZ_LCD_STATE_DISABLED));
+ if (JZFB_IS_SLCD(jzfb)) {
+ jz4740_dma_disable(jzfb->slcd_dma);
+ jzfb_slcd_wait(jzfb);
+ writeb(0, jzfb->base + JZ_REG_SLCD_CTRL);
+ } else {
+ ctrl = readl(jzfb->base + JZ_REG_LCD_CTRL);
+ ctrl |= JZ_LCD_CTRL_DISABLE;
+ writel(ctrl, jzfb->base + JZ_REG_LCD_CTRL);
+ do {
+ ctrl = readl(jzfb->base + JZ_REG_LCD_STATE);
+ } while (!(ctrl & JZ_LCD_STATE_DISABLED));
+ }
jzfb_pins_operation(jzfb, SUSPEND_PINS);
clk_disable(jzfb->ldclk);
@@ -564,12 +760,19 @@ static int jzfb_alloc_devmem(struct jzfb *jzfb)
max_videosize *= jzfb_get_controller_bpp(jzfb) >> 3;
- jzfb->framedesc = dma_alloc_coherent(&jzfb->pdev->dev,
- sizeof(*jzfb->framedesc),
- &jzfb->framedesc_phys, GFP_KERNEL);
+ if (!JZFB_IS_SLCD(jzfb)) {
+ jzfb->framedesc = dma_alloc_coherent(&jzfb->pdev->dev,
+ sizeof(*jzfb->framedesc),
+ &jzfb->framedesc_phys,
+ GFP_KERNEL);
- if (!jzfb->framedesc)
- return -ENOMEM;
+ if (!jzfb->framedesc)
+ return -ENOMEM;
+ } else {
+ jzfb->slcd_dma = jz4740_dma_request(jzfb, "SLCD");
+ if (!jzfb->slcd_dma)
+ return -ENXIO;
+ }
jzfb->vidmem_size = PAGE_ALIGN(max_videosize);
jzfb->vidmem = dma_alloc_coherent(&jzfb->pdev->dev,
@@ -585,17 +788,48 @@ static int jzfb_alloc_devmem(struct jzfb *jzfb)
SetPageReserved(virt_to_page(page));
}
- jzfb->framedesc->next = jzfb->framedesc_phys;
- jzfb->framedesc->addr = jzfb->vidmem_phys;
- jzfb->framedesc->id = 0xdeafbead;
- jzfb->framedesc->cmd = 0;
- jzfb->framedesc->cmd |= max_videosize / 4;
+ if (jzfb->framedesc) {
+ jzfb->framedesc->next = jzfb->framedesc_phys;
+ jzfb->framedesc->addr = jzfb->vidmem_phys;
+ jzfb->framedesc->id = 0xdeafbead;
+ jzfb->framedesc->cmd = 0;
+ jzfb->framedesc->cmd |= max_videosize / 4;
+ } else {
+ struct jz4740_dma_config config = {
+ .src_width = JZ4740_DMA_WIDTH_32BIT,
+ .request_type = JZ4740_DMA_TYPE_SLCD,
+ .flags = JZ4740_DMA_SRC_AUTOINC,
+ .mode = JZ4740_DMA_MODE_SINGLE,
+ };
+
+ switch (jzfb->pdata->bpp) {
+ case 1 ... 8:
+ config.dst_width = JZ4740_DMA_WIDTH_8BIT;
+ config.transfer_size = JZ4740_DMA_TRANSFER_SIZE_1BYTE;
+ break;
+ case 9 ... 16:
+ config.dst_width = JZ4740_DMA_WIDTH_16BIT;
+ config.transfer_size = JZ4740_DMA_TRANSFER_SIZE_2BYTE;
+ break;
+ case 17 ... 32:
+ config.dst_width = JZ4740_DMA_WIDTH_32BIT;
+ config.transfer_size = JZ4740_DMA_TRANSFER_SIZE_4BYTE;
+ break;
+ }
+
+ jz4740_dma_configure(jzfb->slcd_dma, &config);
+ jz4740_dma_set_complete_cb(jzfb->slcd_dma,
+ jzfb_slcd_dma_callback);
+ }
return 0;
err_free_framedesc:
- dma_free_coherent(&jzfb->pdev->dev, sizeof(*jzfb->framedesc),
- jzfb->framedesc, jzfb->framedesc_phys);
+ if (jzfb->framedesc)
+ dma_free_coherent(&jzfb->pdev->dev, sizeof(*jzfb->framedesc),
+ jzfb->framedesc, jzfb->framedesc_phys);
+ if (jzfb->slcd_dma)
+ jz4740_dma_free(jzfb->slcd_dma);
return -ENOMEM;
}
@@ -603,8 +837,11 @@ static void jzfb_free_devmem(struct jzfb *jzfb)
{
dma_free_coherent(&jzfb->pdev->dev, jzfb->vidmem_size,
jzfb->vidmem, jzfb->vidmem_phys);
- dma_free_coherent(&jzfb->pdev->dev, sizeof(*jzfb->framedesc),
- jzfb->framedesc, jzfb->framedesc_phys);
+ if (jzfb->framedesc)
+ dma_free_coherent(&jzfb->pdev->dev, sizeof(*jzfb->framedesc),
+ jzfb->framedesc, jzfb->framedesc_phys);
+ if (jzfb->slcd_dma)
+ jz4740_dma_free(jzfb->slcd_dma);
}
static struct fb_ops jzfb_ops = {
@@ -618,6 +855,125 @@ static struct fb_ops jzfb_ops = {
.fb_setcolreg = jzfb_setcolreg,
};
+static void send_panel_command(struct jzfb *jzfb, u32 cmd)
+{
+ u16 slcd_cfg = readw(jzfb->base + JZ_REG_SLCD_CFG);
+
+ jzfb_slcd_wait(jzfb);
+
+ switch (slcd_cfg & JZ_SLCD_CFG_CWIDTH_MASK) {
+ case JZ_SLCD_CFG_CWIDTH_8BIT:
+ writel(JZ_SLCD_DATA_RS_COMMAND | ((cmd & 0xff00) >> 8),
+ jzfb->base + JZ_REG_SLCD_DATA);
+ jzfb_slcd_wait(jzfb);
+ writel(JZ_SLCD_DATA_RS_COMMAND | (cmd & 0xff),
+ jzfb->base + JZ_REG_SLCD_DATA);
+ break;
+
+ case JZ_SLCD_CFG_CWIDTH_16BIT:
+ writel(JZ_SLCD_DATA_RS_COMMAND | (cmd & 0xffff),
+ jzfb->base + JZ_REG_SLCD_DATA);
+ break;
+
+ case JZ_SLCD_CFG_CWIDTH_18BIT:
+ writel(JZ_SLCD_DATA_RS_COMMAND | ((cmd & 0xff00) << 2) |
+ ((cmd & 0xff) << 1), jzfb->base + JZ_REG_SLCD_DATA);
+ break;
+ }
+}
+
+static void send_panel_data(struct jzfb *jzfb, u32 data)
+{
+ u16 slcd_cfg = readw(jzfb->base + JZ_REG_SLCD_CFG);
+
+ switch (slcd_cfg & JZ_SLCD_CFG_DWIDTH_MASK) {
+ case JZ_SLCD_CFG_DWIDTH_18:
+ case JZ_SLCD_CFG_DWIDTH_9_x2:
+ data = ((data & 0xff) << 1) | ((data & 0xff00) << 2);
+ data = ((data << 6) & 0xfc0000) | ((data << 4) & 0xfc00) |
+ ((data << 2) & 0x0000fc);
+ break;
+
+ case JZ_SLCD_CFG_DWIDTH_16:
+ default:
+ data &= 0xffff;
+ break;
+ }
+
+ jzfb_slcd_wait(jzfb);
+ writel(JZ_SLCD_DATA_RS_DATA | data, jzfb->base + JZ_REG_SLCD_DATA);
+}
+
+void jz4740_fb_slcd_disable_transfer(struct platform_device *pdev)
+{
+ struct jzfb *jzfb = platform_get_drvdata(pdev);
+
+ mutex_lock(&jzfb->lock);
+
+ if (jzfb->is_enabled) {
+ jz4740_dma_disable(jzfb->slcd_dma);
+ jzfb_slcd_wait(jzfb);
+ }
+
+ mutex_unlock(&jzfb->lock);
+}
+EXPORT_SYMBOL_GPL(jz4740_fb_slcd_disable_transfer);
+
+void jz4740_fb_slcd_enable_transfer(struct platform_device *pdev)
+{
+ struct jzfb *jzfb = platform_get_drvdata(pdev);
+
+ mutex_lock(&jzfb->lock);
+
+ if (jzfb->is_enabled)
+ jzfb_slcd_start_dma(jzfb);
+
+ mutex_unlock(&jzfb->lock);
+}
+EXPORT_SYMBOL_GPL(jz4740_fb_slcd_enable_transfer);
+
+void jz4740_fb_slcd_send_cmd_data(struct platform_device *pdev,
+ unsigned int cmd, unsigned int data)
+{
+ struct jzfb *jzfb = platform_get_drvdata(pdev);
+
+ mutex_lock(&jzfb->lock);
+
+ if (!jzfb->is_enabled)
+ clk_enable(jzfb->ldclk);
+
+ send_panel_command(jzfb, cmd);
+ send_panel_data(jzfb, data);
+
+ if (!jzfb->is_enabled) {
+ jzfb_slcd_wait(jzfb);
+ clk_disable(jzfb->ldclk);
+ }
+
+ mutex_unlock(&jzfb->lock);
+}
+EXPORT_SYMBOL_GPL(jz4740_fb_slcd_send_cmd_data);
+
+void jz4740_fb_slcd_send_cmd(struct platform_device *pdev, unsigned int cmd)
+{
+ struct jzfb *jzfb = platform_get_drvdata(pdev);
+
+ mutex_lock(&jzfb->lock);
+
+ if (!jzfb->is_enabled)
+ clk_enable(jzfb->ldclk);
+
+ send_panel_command(jzfb, cmd);
+
+ if (!jzfb->is_enabled) {
+ jzfb_slcd_wait(jzfb);
+ clk_disable(jzfb->ldclk);
+ }
+
+ mutex_unlock(&jzfb->lock);
+}
+EXPORT_SYMBOL_GPL(jz4740_fb_slcd_send_cmd);
+
static int __devinit jzfb_probe(struct platform_device *pdev)
{
int ret;
@@ -673,6 +1029,7 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
goto err_put_ldclk;
}
+ jzfb->phys_base = mem->start;
jzfb->base = ioremap(mem->start, resource_size(mem));
if (!jzfb->base) {
dev_err(&pdev->dev, "Failed to ioremap register memory region\n");
--
1.7.4
^ permalink raw reply related
* [PATCH 5/6] FBDEV: JZ4740: Get rid of enumeration value not handled in switch warnings
From: Maurus Cuelenaere @ 2011-03-01 12:06 UTC (permalink / raw)
To: linux-fbdev
Convert enum jz4740_fb_lcd_type to #define's in order to get rid of these
warnings.
Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
---
arch/mips/include/asm/mach-jz4740/jz4740_fb.h | 40 ++++++++++++-------------
1 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/arch/mips/include/asm/mach-jz4740/jz4740_fb.h b/arch/mips/include/asm/mach-jz4740/jz4740_fb.h
index eaaac43..d6c11f2 100644
--- a/arch/mips/include/asm/mach-jz4740/jz4740_fb.h
+++ b/arch/mips/include/asm/mach-jz4740/jz4740_fb.h
@@ -17,27 +17,25 @@
#include <linux/fb.h>
-enum jz4740_fb_lcd_type {
- JZ_LCD_TYPE_GENERIC_16_BIT = 0,
- JZ_LCD_TYPE_GENERIC_18_BIT = 0 | (1 << 4),
- JZ_LCD_TYPE_SPECIAL_TFT_1 = 1,
- JZ_LCD_TYPE_SPECIAL_TFT_2 = 2,
- JZ_LCD_TYPE_SPECIAL_TFT_3 = 3,
- JZ_LCD_TYPE_NON_INTERLACED_CCIR656 = 5,
- JZ_LCD_TYPE_INTERLACED_CCIR656 = 7,
- JZ_LCD_TYPE_SINGLE_COLOR_STN = 8,
- JZ_LCD_TYPE_SINGLE_MONOCHROME_STN = 9,
- JZ_LCD_TYPE_DUAL_COLOR_STN = 10,
- JZ_LCD_TYPE_DUAL_MONOCHROME_STN = 11,
- JZ_LCD_TYPE_8BIT_SERIAL = 12,
+#define JZ_LCD_TYPE_GENERIC_16_BIT (0)
+#define JZ_LCD_TYPE_GENERIC_18_BIT (0 | (1 << 4))
+#define JZ_LCD_TYPE_SPECIAL_TFT_1 (1)
+#define JZ_LCD_TYPE_SPECIAL_TFT_2 (2)
+#define JZ_LCD_TYPE_SPECIAL_TFT_3 (3)
+#define JZ_LCD_TYPE_NON_INTERLACED_CCIR656 (5)
+#define JZ_LCD_TYPE_INTERLACED_CCIR656 (7)
+#define JZ_LCD_TYPE_SINGLE_COLOR_STN (8)
+#define JZ_LCD_TYPE_SINGLE_MONOCHROME_STN (9)
+#define JZ_LCD_TYPE_DUAL_COLOR_STN (10)
+#define JZ_LCD_TYPE_DUAL_MONOCHROME_STN (11)
+#define JZ_LCD_TYPE_8BIT_SERIAL (12)
- JZ_SLCD_TYPE_PARALLEL_8_BIT = 1 | (1 << 5),
- JZ_SLCD_TYPE_PARALLEL_16_BIT = 0 | (1 << 5),
- JZ_SLCD_TYPE_PARALLEL_18_BIT = 2 | (1 << 5),
- JZ_SLCD_TYPE_SERIAL_8_BIT = 1 | (3 << 5),
- JZ_SLCD_TYPE_SERIAL_16_BIT = 0 | (3 << 5),
- JZ_SLCD_TYPE_SERIAL_18_BIT = 2 | (3 << 5),
-};
+#define JZ_SLCD_TYPE_PARALLEL_8_BIT (1 | (1 << 5))
+#define JZ_SLCD_TYPE_PARALLEL_16_BIT (0 | (1 << 5))
+#define JZ_SLCD_TYPE_PARALLEL_18_BIT (2 | (1 << 5))
+#define JZ_SLCD_TYPE_SERIAL_8_BIT (1 | (3 << 5))
+#define JZ_SLCD_TYPE_SERIAL_16_BIT (0 | (3 << 5))
+#define JZ_SLCD_TYPE_SERIAL_18_BIT (2 | (3 << 5))
#define JZ4740_FB_SPECIAL_TFT_CONFIG(start, stop) (((start) << 16) | (stop))
@@ -58,7 +56,7 @@ struct jz4740_fb_platform_data {
struct fb_videomode *modes;
unsigned int bpp;
- enum jz4740_fb_lcd_type lcd_type;
+ unsigned int lcd_type;
struct {
uint32_t spl;
--
1.7.4
^ permalink raw reply related
* [RFC/PATCH 6/6] backlight: Add Truly G240400RTSW LCD panel driver
From: Maurus Cuelenaere @ 2011-03-01 12:06 UTC (permalink / raw)
To: linux-fbdev
This is a driver for the Truly G240400RTSW LCD panel using the Jz4740
SLCD framebuffer functions to communicate with the driver chip (Renesas R61509).
It currently supports initializing at boot and suspending, but it fails to come
back up after resume and it doesn't support any gamma or image control.
Tested on an Onda VX747 (Ingenic Jz4740 device).
Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
---
drivers/video/backlight/Kconfig | 7 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/truly_g240400rtsw.c | 464 +++++++++++++++++++++++++++
include/video/truly_g240400rtsw.h | 35 ++
4 files changed, 507 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/backlight/truly_g240400rtsw.c
create mode 100644 include/video/truly_g240400rtsw.h
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index e54a337..afa96c2 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -109,6 +109,13 @@ config LCD_S6E63M0
If you have an S6E63M0 LCD Panel, say Y to enable its
LCD control driver.
+config LCD_G240400
+ tristate "Truly G240400RTSW LCD Driver"
+ depends on MACH_JZ4740
+ help
+ If you have a Truly G2400400RTSW LCD panel, say Y to enable its LCD
+ control driver.
+
endif # LCD_CLASS_DEVICE
#
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 44c0f81..1e338ce 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_LCD_VGG2432A4) += vgg2432a4.o
obj-$(CONFIG_LCD_TDO24M) += tdo24m.o
obj-$(CONFIG_LCD_TOSA) += tosa_lcd.o
obj-$(CONFIG_LCD_S6E63M0) += s6e63m0.o
+obj-$(CONFIG_LCD_G240400) += truly_g240400rtsw.o
obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE) += backlight.o
obj-$(CONFIG_BACKLIGHT_ATMEL_PWM) += atmel-pwm-bl.o
diff --git a/drivers/video/backlight/truly_g240400rtsw.c b/drivers/video/backlight/truly_g240400rtsw.c
new file mode 100644
index 0000000..437980b
--- /dev/null
+++ b/drivers/video/backlight/truly_g240400rtsw.c
@@ -0,0 +1,464 @@
+/*
+ * drivers/video/backlight/truly_g240400rtsw.c
+ *
+ * Truly G240400RTSW LCD panel driver
+ *
+ * Copyright (c) 2011, Maurus Cuelenaere <mcuelenaere@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+#include <linux/backlight.h>
+#include <linux/device.h>
+#include <linux/fb.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/lcd.h>
+
+#include <asm/mach-jz4740/jz4740_fb.h>
+
+#include <video/truly_g240400rtsw.h>
+
+/* This LCD panel contains a Renesas R61509 driver chip */
+/* Register list */
+#define REG_DEVICE_CODE 0x000
+#define REG_DRIVER_OUTPUT 0x001
+#define REG_LCD_DR_WAVE_CTRL 0x002
+#define REG_ENTRY_MODE 0x003
+#define REG_OUTL_SHARP_CTRL 0x006
+#define REG_DISP_CTRL1 0x007
+#define REG_DISP_CTRL2 0x008
+#define REG_DISP_CTRL3 0x009
+#define REG_LPCTRL 0x00B
+#define REG_EXT_DISP_CTRL1 0x00C
+#define REG_EXT_DISP_CTRL2 0x00F
+#define REG_PAN_INTF_CTRL1 0x010
+#define REG_PAN_INTF_CTRL2 0x011
+#define REG_PAN_INTF_CTRL3 0x012
+#define REG_PAN_INTF_CTRL4 0x020
+#define REG_PAN_INTF_CTRL5 0x021
+#define REG_PAN_INTF_CTRL6 0x022
+#define REG_FRM_MRKR_CTRL 0x090
+
+#define REG_PWR_CTRL1 0x100
+#define REG_PWR_CTRL2 0x101
+#define REG_PWR_CTRL3 0x102
+#define REG_PWR_CTRL4 0x103
+#define REG_PWR_CTRL5 0x107
+#define REG_PWR_CTRL6 0x110
+#define REG_PWR_CTRL7 0x112
+
+#define REG_RAM_HADDR_SET 0x200
+#define REG_RAM_VADDR_SET 0x201
+#define REG_RW_GRAM 0x202
+#define REG_RAM_HADDR_START 0x210
+#define REG_RAM_HADDR_END 0x211
+#define REG_RAM_VADDR_START 0x212
+#define REG_RAM_VADDR_END 0x213
+#define REG_RW_NVM 0x280
+#define REG_VCOM_HVOLTAGE1 0x281
+#define REG_VCOM_HVOLTAGE2 0x282
+
+#define REG_GAMMA_CTRL1 0x300
+#define REG_GAMMA_CTRL2 0x301
+#define REG_GAMMA_CTRL3 0x302
+#define REG_GAMMA_CTRL4 0x303
+#define REG_GAMMA_CTRL5 0x304
+#define REG_GAMMA_CTRL6 0x305
+#define REG_GAMMA_CTRL7 0x306
+#define REG_GAMMA_CTRL8 0x307
+#define REG_GAMMA_CTRL9 0x308
+#define REG_GAMMA_CTRL10 0x309
+#define REG_GAMMA_CTRL11 0x30A
+#define REG_GAMMA_CTRL12 0x30B
+#define REG_GAMMA_CTRL13 0x30C
+#define REG_GAMMA_CTRL14 0x30D
+
+#define REG_BIMG_NR_LINE 0x400
+#define REG_BIMG_DISP_CTRL 0x401
+#define REG_BIMG_VSCROLL_CTRL 0x404
+
+#define REG_PARTIMG1_POS 0x500
+#define REG_PARTIMG1_RAM_START 0x501
+#define REG_PARTIMG1_RAM_END 0x502
+#define REG_PARTIMG2_POS 0x503
+#define REG_PARTIMG2_RAM_START 0x504
+#define REG_PARTIMG2_RAM_END 0x505
+
+#define REG_SOFT_RESET 0x600
+#define REG_ENDIAN_CTRL 0x606
+#define REG_NVM_ACCESS_CTRL 0x6F0
+
+/* Bits */
+#define DRIVER_OUTPUT_SS_BIT (1 << 8)
+#define DRIVER_OUTPUT_SM_BIT (1 << 10)
+
+#define ENTRY_MODE_TRI (1 << 15)
+#define ENTRY_MODE_DFM (1 << 14)
+#define ENTRY_MODE_BGR (1 << 12)
+#define ENTRY_MODE_HWM (1 << 9)
+#define ENTRY_MODE_ORG (1 << 7)
+#define ENTRY_MODE_VID (1 << 5)
+#define ENTRY_MODE_HID (1 << 4)
+#define ENTRY_MODE_AM (1 << 3)
+#define ENTRY_MODE_EPF(n) (n & 3)
+
+#define OUTL_SHARP_CTRL_EGMODE (1 << 15)
+#define OUTL_SHARP_CTRL_AVST(n) ((n & 7) << 7)
+#define OUTL_SHARP_CTRL_ADST(n) ((n & 7) << 4)
+#define OUTL_SHARP_CTRL_DTHU(n) ((n & 3) << 2)
+#define OUTL_SHARP_CTRL_DTHL(n) (n & 3)
+
+#define DISP_CTRL1_PTDE(n) ((n & 4) << 12)
+#define DISP_CTRL1_BASEE (1 << 8)
+#define DISP_CTRL1_VON (1 << 6)
+#define DISP_CTRL1_GON (1 << 5)
+#define DISP_CTRL1_DTE (1 << 4)
+#define DISP_CTRL1_D(n) (n & 3)
+
+#define EXT_DISP_CTRL1_ENC(n) ((n & 7) << 12)
+#define EXT_DISP_CTRL1_RM(n) ((n & 1) << 8)
+#define EXT_DISP_CTRL1_DM(n) ((n & 3) << 4)
+#define EXT_DISP_CTRL1_RIM(n) (n & 3)
+
+#define PWR_CTRL1_SAP(n) ((n & 3) << 13)
+#define PWR_CTRL1_SAPE (1 << 12)
+#define PWR_CTRL1_BT(n) ((n & 7) << 8)
+#define PWR_CTRL1_APE (1 << 7)
+#define PWR_CTRL1_AP(n) ((n & 7) << 4)
+#define PWR_CTRL1_DSTB (1 << 2)
+#define PWR_CTRL1_SLP (1 << 1)
+
+#define SOFT_RESET(n) (n << 0)
+
+struct g240400 {
+ struct device *dev;
+ struct lcd_device *lcd;
+
+ struct g240400_pdata pdata;
+
+ int power;
+};
+
+#define POWER_IS_ON(pwr) ((pwr) <= FB_BLANK_NORMAL)
+
+static void g240400_reset(struct g240400 *lcm)
+{
+ struct platform_device *jzfb = lcm->pdata.jz4740_fb;
+
+ if (gpio_is_valid(lcm->pdata.gpio_reset)) {
+ gpio_direction_output(lcm->pdata.gpio_reset, 0);
+ mdelay(1);
+ gpio_direction_output(lcm->pdata.gpio_reset, 1);
+ }
+
+ jz4740_fb_slcd_disable_transfer(jzfb);
+
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_SOFT_RESET, SOFT_RESET(1));
+ mdelay(1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_SOFT_RESET, SOFT_RESET(0));
+ mdelay(1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_ENDIAN_CTRL, 0);
+
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_DRIVER_OUTPUT, 0x100);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_LCD_DR_WAVE_CTRL, 0x100);
+
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_ENTRY_MODE, (ENTRY_MODE_BGR | ENTRY_MODE_HWM | ENTRY_MODE_DFM | ENTRY_MODE_VID | ENTRY_MODE_HID));
+
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_DISP_CTRL2, 0x503);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_DISP_CTRL3, 1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_LPCTRL, 0x10);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_EXT_DISP_CTRL1, EXT_DISP_CTRL1_RIM(1)); /* 16-bit RGB interface */
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_EXT_DISP_CTRL2, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_DISP_CTRL1, DISP_CTRL1_D(1));
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PAN_INTF_CTRL1, 0x12);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PAN_INTF_CTRL2, 0x202);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PAN_INTF_CTRL3, 0x300);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PAN_INTF_CTRL4, 0x21e);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PAN_INTF_CTRL5, 0x202);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PAN_INTF_CTRL6, 0x100);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_FRM_MRKR_CTRL, 0x8000);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PWR_CTRL1, (PWR_CTRL1_SAPE | PWR_CTRL1_BT(6) | PWR_CTRL1_APE | PWR_CTRL1_AP(3)));
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PWR_CTRL2, 0x147);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PWR_CTRL3, 0x1bd);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PWR_CTRL4, 0x2f00);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PWR_CTRL5, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PWR_CTRL6, 1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_RW_NVM, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_VCOM_HVOLTAGE1, 6);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_VCOM_HVOLTAGE2, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL1, 0x101);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL2, 0xb27);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL3, 0x132a);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL4, 0x2a13);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL5, 0x270b);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL6, 0x101);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL7, 0x1205);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL8, 0x512);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL9, 5);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL10, 3);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL11, 0xf04);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL12, 0xf00);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL13, 0xf);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_GAMMA_CTRL14, 0x40f);
+ jz4740_fb_slcd_send_cmd_data(jzfb, 0x30e, 0x300);
+ jz4740_fb_slcd_send_cmd_data(jzfb, 0x30f, 0x500);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_BIMG_NR_LINE, 0x3100);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_BIMG_DISP_CTRL, 1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_BIMG_VSCROLL_CTRL, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PARTIMG1_POS, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PARTIMG1_RAM_START, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PARTIMG1_RAM_END, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PARTIMG2_POS, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PARTIMG2_RAM_START, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PARTIMG2_RAM_END, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_ENDIAN_CTRL, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_NVM_ACCESS_CTRL, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, 0x7f0, 0x5420);
+ jz4740_fb_slcd_send_cmd_data(jzfb, 0x7f3, 0x288a);
+ jz4740_fb_slcd_send_cmd_data(jzfb, 0x7f4, 0x22);
+ jz4740_fb_slcd_send_cmd_data(jzfb, 0x7f5, 1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, 0x7f0, 0);
+
+ /* LCD ON */
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_DISP_CTRL1, (DISP_CTRL1_GON |
+ DISP_CTRL1_D(1)));
+ mdelay(1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_DISP_CTRL1, (DISP_CTRL1_VON |
+ DISP_CTRL1_GON | DISP_CTRL1_D(1)));
+ mdelay(1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_DISP_CTRL1, (DISP_CTRL1_BASEE |
+ DISP_CTRL1_VON | DISP_CTRL1_GON |
+ DISP_CTRL1_DTE | DISP_CTRL1_D(3)));
+
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_RAM_HADDR_START, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_RAM_HADDR_END, lcm->pdata.default_mode->xres - 1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_RAM_VADDR_START, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_RAM_VADDR_END, lcm->pdata.default_mode->yres - 1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_RAM_HADDR_SET, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_RAM_VADDR_SET, 0);
+ jz4740_fb_slcd_send_cmd(jzfb, REG_RW_GRAM); /* write data to GRAM */
+
+ jz4740_fb_slcd_enable_transfer(jzfb);
+}
+
+static void g240400_powerdown(struct g240400 *lcm)
+{
+ struct platform_device *jzfb = lcm->pdata.jz4740_fb;
+
+ jz4740_fb_slcd_disable_transfer(jzfb);
+
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_DISP_CTRL1, (DISP_CTRL1_VON |
+ DISP_CTRL1_GON | DISP_CTRL1_DTE |
+ DISP_CTRL1_D(2)));
+ mdelay(1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_DISP_CTRL1, DISP_CTRL1_D(1));
+ mdelay(1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_DISP_CTRL1, DISP_CTRL1_D(0));
+
+ jz4740_fb_slcd_enable_transfer(jzfb);
+}
+
+static void g240400_standby(struct g240400 *lcm)
+{
+ struct platform_device *jzfb = lcm->pdata.jz4740_fb;
+
+ jz4740_fb_slcd_disable_transfer(jzfb);
+
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PWR_CTRL1, PWR_CTRL1_SLP);
+
+ jz4740_fb_slcd_enable_transfer(jzfb);
+}
+
+static void g240400_poweron(struct g240400 *lcm)
+{
+ struct platform_device *jzfb = lcm->pdata.jz4740_fb;
+
+ jz4740_fb_slcd_disable_transfer(jzfb);
+
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_PWR_CTRL1, (PWR_CTRL1_SAPE | PWR_CTRL1_BT(6) | PWR_CTRL1_APE | PWR_CTRL1_AP(3)));
+
+ jz4740_fb_slcd_enable_transfer(jzfb);
+}
+
+static int g240400_get_power(struct lcd_device *lcd)
+{
+ struct g240400 *lcm = lcd_get_data(lcd);
+
+ return lcm->power;
+}
+
+static int g240400_set_power(struct lcd_device *lcd, int power)
+{
+ struct g240400 *lcm = lcd_get_data(lcd);
+
+ if (power = lcm->power)
+ return 0;
+
+ if (POWER_IS_ON(power))
+ g240400_poweron(lcm);
+ else
+ g240400_standby(lcm);
+
+ lcm->power = power;
+
+ return 0;
+}
+
+static int g240400_set_mode(struct lcd_device *lcd, struct fb_videomode *mode)
+{
+ struct g240400 *lcm = lcd_get_data(lcd);
+ struct platform_device *jzfb = lcm->pdata.jz4740_fb;
+
+ jz4740_fb_slcd_disable_transfer(jzfb);
+
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_RAM_HADDR_START, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_RAM_HADDR_END, mode->xres - 1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_RAM_VADDR_START, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_RAM_VADDR_END, mode->yres - 1);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_RAM_HADDR_SET, 0);
+ jz4740_fb_slcd_send_cmd_data(jzfb, REG_RAM_VADDR_SET, 0);
+ jz4740_fb_slcd_send_cmd(jzfb, REG_RW_GRAM); /* write data to GRAM */
+
+ jz4740_fb_slcd_enable_transfer(jzfb);
+
+ lcm->pdata.default_mode = mode;
+
+ return 0;
+}
+
+static struct lcd_ops g240400_lcd_ops = {
+ .get_power = g240400_get_power,
+ .set_power = g240400_set_power,
+ .set_mode = g240400_set_mode,
+};
+
+static int __devinit g240400_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct g240400 *lcm;
+ struct lcd_device *lcd;
+ struct g240400_pdata *pdata = pdev->dev.platform_data;
+
+ if (!pdata) {
+ dev_err(&pdev->dev, "Platform data wasn't provided!");
+ return -EINVAL;
+ }
+
+ lcm = kzalloc(sizeof(struct g240400), GFP_KERNEL);
+ if (!lcm)
+ return -ENOMEM;
+
+ lcm->dev = &pdev->dev;
+ memcpy(&lcm->pdata, pdata, sizeof(struct g240400_pdata));
+
+ lcd = lcd_device_register("truly_g240400", &pdev->dev, lcm,
+ &g240400_lcd_ops);
+ if (IS_ERR(lcd)) {
+ ret = PTR_ERR(lcd);
+ goto out_free_device;
+ }
+
+ if (gpio_is_valid(lcm->pdata.gpio_reset)) {
+ ret = gpio_request(lcm->pdata.gpio_reset, "lcd reset");
+ if (ret)
+ goto out_unregister_lcd;
+ }
+
+ if (gpio_is_valid(lcm->pdata.gpio_cs)) {
+ ret = gpio_request(lcm->pdata.gpio_cs, "lcd chip select");
+ if (ret)
+ goto out_free_pin_cs;
+ }
+
+ dev_set_drvdata(&pdev->dev, lcm);
+
+ /* Always select LCD chip */
+ if (gpio_is_valid(lcm->pdata.gpio_cs))
+ gpio_direction_output(lcm->pdata.gpio_cs, 0);
+
+ g240400_reset(lcm);
+
+ return 0;
+
+out_free_pin_cs:
+ if (gpio_is_valid(lcm->pdata.gpio_cs))
+ gpio_free(lcm->pdata.gpio_cs);
+out_unregister_lcd:
+ lcd_device_unregister(lcd);
+out_free_device:
+ kfree(lcm);
+
+ return ret;
+}
+
+static int __devexit g240400_remove(struct platform_device *pdev)
+{
+ struct g240400 *lcm = dev_get_drvdata(&pdev->dev);
+
+ if (gpio_is_valid(lcm->pdata.gpio_reset))
+ gpio_free(lcm->pdata.gpio_reset);
+ if (gpio_is_valid(lcm->pdata.gpio_cs))
+ gpio_free(lcm->pdata.gpio_cs);
+ lcd_device_unregister(lcm->lcd);
+ kfree(lcm);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM
+static int g240400_suspend(struct device *dev)
+{
+ struct g240400 *lcm = dev_get_drvdata(dev);
+
+ g240400_powerdown(lcm);
+ gpio_direction_output(lcm->pdata.gpio_cs, 1);
+
+ return 0;
+}
+
+static int g240400_resume(struct device *dev)
+{
+ struct g240400 *lcm = dev_get_drvdata(dev);
+
+ gpio_direction_output(lcm->pdata.gpio_cs, 0);
+ g240400_reset(lcm);
+
+ return 0;
+}
+
+static const struct dev_pm_ops g240400_ops = {
+ .suspend = g240400_suspend,
+ .resume = g240400_resume,
+};
+#endif
+
+static struct platform_driver g240400_driver = {
+ .driver = {
+ .name = "truly_g240400rtsw",
+ .owner = THIS_MODULE,
+#ifdef CONFIG_PM
+ .pm = &g240400_ops,
+#endif
+ },
+ .probe = g240400_probe,
+ .remove = __devexit_p(g240400_remove),
+};
+
+static int __init g240400_init(void)
+{
+ return platform_driver_register(&g240400_driver);
+}
+late_initcall(g240400_init); /* Ensure this gets loaded *after* the FB */
+
+static void __exit g240400_exit(void)
+{
+ platform_driver_unregister(&g240400_driver);
+}
+module_exit(g240400_exit);
diff --git a/include/video/truly_g240400rtsw.h b/include/video/truly_g240400rtsw.h
new file mode 100644
index 0000000..e4147b2
--- /dev/null
+++ b/include/video/truly_g240400rtsw.h
@@ -0,0 +1,35 @@
+/*
+ * include/video/truly_g240400rtsw.h
+ *
+ * Truly G240400RTSW LCD panel driver
+ *
+ * Copyright (c) 2011, Maurus Cuelenaere <mcuelenaere@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+
+#ifndef __VIDEO_TRULY_G240400RTSW_H__
+#define __VIDEO_TRULY_G240400RTSW_H__
+
+struct platform_device;
+struct fb_videomode;
+
+/**
+ * @jz4740_fb: pointer to the platform_device corresponding with the Ingenic
+ * Jz4740 framebuffer driver, required
+ * @default_mode: initial video mode when LCD panel gets powered on, required
+ * @gpio_reset: GPIO reset pin, set to -1 for unused
+ * @gpio_cs: GPIO chip select pin, set to -1 for unused
+ */
+struct g240400_pdata {
+ struct platform_device *jz4740_fb;
+ struct fb_videomode *default_mode;
+ int gpio_reset;
+ int gpio_cs;
+};
+
+#endif /* __VIDEO_TRULY_G240400RTSW_H__ */
--
1.7.4
^ permalink raw reply related
* Re: [PATCH 1/6] FBDEV: JZ4740: Refactor GPIO pin operations
From: Lars-Peter Clausen @ 2011-03-01 13:56 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <7f854ef86a16dd03f415965b3535c607c401941e.1298980528.git.mcuelenaere@gmail.com>
On 03/01/2011 01:05 PM, Maurus Cuelenaere wrote:
> Encapsulate all GPIO pins related operations into 1 function which'll make it
> easier to add on SLCD support.
>
> Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
> ---
> drivers/video/jz4740_fb.c | 82 +++++++++++++++++++++-----------------------
> 1 files changed, 39 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
> index 7e61e68..65741d0 100644
> --- a/drivers/video/jz4740_fb.c
> +++ b/drivers/video/jz4740_fb.c
> @@ -178,59 +178,60 @@ static const struct jz_gpio_bulk_request jz_lcd_data_pins[] = {
> JZ_GPIO_BULK_PIN(LCD_DATA17),
> };
>
> -static unsigned int jzfb_num_ctrl_pins(struct jzfb *jzfb)
> +enum jzfb_pin_operation {
> + REQUEST_PINS,
> + FREE_PINS,
> + RESUME_PINS,
> + SUSPEND_PINS,
> +};
Please add a JZFB_ prefix.
^ permalink raw reply
* Re: [PATCH 2/6] FBDEV: JZ4740: Initialize jzfb->fb earlier
From: Lars-Peter Clausen @ 2011-03-01 13:58 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <80cfeb81c3b4bae96f8ade317a08566c87493578.1298980528.git.mcuelenaere@gmail.com>
On 03/01/2011 01:05 PM, Maurus Cuelenaere wrote:
> There's no reason to not do this, and this is needed in the upcoming SLCD
> support patches.
>
Is there any specific reason, why this change can't be done in the patch adding
SLCD support?
- Lars
> Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
> ---
> drivers/video/jz4740_fb.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
> index 65741d0..cea7b7e 100644
> --- a/drivers/video/jz4740_fb.c
> +++ b/drivers/video/jz4740_fb.c
> @@ -654,6 +654,7 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
> fb->flags = FBINFO_DEFAULT;
>
> jzfb = fb->par;
> + jzfb->fb = fb;
> jzfb->pdev = pdev;
> jzfb->pdata = pdata;
> jzfb->mem = mem;
> @@ -722,8 +723,6 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
> goto err_free_devmem;
> }
>
> - jzfb->fb = fb;
> -
> return 0;
>
> err_free_devmem:
^ permalink raw reply
* Re: [PATCH 3/6] FBDEV: JZ4740: Initialize framebuffer properly
From: Lars-Peter Clausen @ 2011-03-01 13:58 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1d49e2b9aa2fa143c14a9782fd4e67cb50610ca8.1298980528.git.mcuelenaere@gmail.com>
On 03/01/2011 01:06 PM, Maurus Cuelenaere wrote:
> Use the jzfb_enable() function instead of manually faking it in jzfb_probe().
> Also add the reverse operation when the framebuffer couldn't be registered.
>
> Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
> ---
> drivers/video/jz4740_fb.c | 8 +++-----
> 1 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
> index cea7b7e..2f3ea57 100644
> --- a/drivers/video/jz4740_fb.c
> +++ b/drivers/video/jz4740_fb.c
> @@ -707,16 +707,13 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
>
> fb_alloc_cmap(&fb->cmap, 256, 0);
>
> - clk_enable(jzfb->ldclk);
> - jzfb->is_enabled = 1;
> -
> - writel(jzfb->framedesc->next, jzfb->base + JZ_REG_LCD_DA0);
> -
> fb->mode = NULL;
> jzfb_set_par(fb);
>
> jzfb_pins_operation(jzfb, REQUEST_PINS);
>
> + jzfb_blank(FB_BLANK_UNBLANK, fb);
> +
I'd prefer:
jzfb_enable(jzfb);
jzfb->is_enabled = 1;
since there is no need for locking here.
> ret = register_framebuffer(fb);
> if (ret) {
> dev_err(&pdev->dev, "Failed to register framebuffer: %d\n", ret);
> @@ -726,6 +723,7 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
> return 0;
>
> err_free_devmem:
> + jzfb_blank(FB_BLANK_POWERDOWN, fb);
> jzfb_pins_operation(jzfb, FREE_PINS);
>
> fb_dealloc_cmap(&fb->cmap);
^ permalink raw reply
* Re: [PATCH 3/8] Add a mfd IPUv3 driver
From: Thomas Gleixner @ 2011-03-01 14:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201103011243.33765.arnd@arndb.de>
On Tue, 1 Mar 2011, Arnd Bergmann wrote:
> On Tuesday 01 March 2011, Sascha Hauer wrote:
> > > Taking one step back from this, have you considered making this
> > > a regular interrupt controller? That would make the client drivers
> > > more standard -- you could define the interrupt numbers as resources
> > > of a platform device or in the device tree, for instance.
> > > The cost might be more complex code, e.g. when a device requires
> > > many interrupts, but I think it will be at least as efficient
> > > at run-time, and less surprising for readers and authors of
> > > client drivers.
> >
> > I thought about this, but hesitated to increase NR_IRQS by 463. Do you
> > think we should do this instead?
>
> I think there is a plan to virtualize the interrupt numbers on ARM,
> and in that case NR_IRQS becomes rather meaningless. I don't know
> exactly how far that effort has come.
Also sparse irqs allows us now to allocate beyond NR_IRQS. With sparse
irqs NR_IRQS is pretty meaningless and just gives us an indicator how
large the irq space might become, but we allow up to 8k dynamically
allocated irqs beyond NR_IRQS, so this should be sufficient for your
problem.
Thanks,
tglx
^ permalink raw reply
* Re: [RFC/PATCH 4/6] FBDEV: JZ4740: Add Smart LCD controller support
From: Lars-Peter Clausen @ 2011-03-01 14:20 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <56c410783be268e00d09e2c9450e56925bd815a8.1298980528.git.mcuelenaere@gmail.com>
On 03/01/2011 01:06 PM, Maurus Cuelenaere wrote:
> This patch adds SLCD support to the Ingenic Jz4740 framebuffer driver.
> Besides adding the new LCD types to the platform data, this adds chip select
> and register select active low support (SLCD only). Also, this exports some
> functions related to starting and stopping the SLCD image transfer and sending
> commands.
>
> A lot of this code was based on work by Maarten ter Huurne:
> https://github.com/mthuurne/opendingux-kernel/commit/3dba5b5e7a868b1067acae8d1b5a19121b77bc6a
>
> Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
Looks mostly good :)
> ---
> arch/mips/include/asm/mach-jz4740/jz4740_fb.h | 33 ++
> drivers/video/jz4740_fb.c | 435 ++++++++++++++++++++++---
> 2 files changed, 429 insertions(+), 39 deletions(-)
>
> diff --git a/arch/mips/include/asm/mach-jz4740/jz4740_fb.h b/arch/mips/include/asm/mach-jz4740/jz4740_fb.h
> index 6a50e6f..eaaac43 100644
> --- a/arch/mips/include/asm/mach-jz4740/jz4740_fb.h
> +++ b/arch/mips/include/asm/mach-jz4740/jz4740_fb.h
> @@ -30,6 +30,13 @@ enum jz4740_fb_lcd_type {
> JZ_LCD_TYPE_DUAL_COLOR_STN = 10,
> JZ_LCD_TYPE_DUAL_MONOCHROME_STN = 11,
> JZ_LCD_TYPE_8BIT_SERIAL = 12,
> +
> + JZ_SLCD_TYPE_PARALLEL_8_BIT = 1 | (1 << 5),
> + JZ_SLCD_TYPE_PARALLEL_16_BIT = 0 | (1 << 5),
> + JZ_SLCD_TYPE_PARALLEL_18_BIT = 2 | (1 << 5),
> + JZ_SLCD_TYPE_SERIAL_8_BIT = 1 | (3 << 5),
> + JZ_SLCD_TYPE_SERIAL_16_BIT = 0 | (3 << 5),
> + JZ_SLCD_TYPE_SERIAL_18_BIT = 2 | (3 << 5),
I would prefer
JZ_LCD_TYPE_SLCD_...
> };
Maybe add
#define JZ_LCD_TYPE_SLCD (1 << 5)
#define JZ_LCD_TYPE_SLCD_SERIAL (JZ_LCD_TYPE_SLCD | (1 << 6))
>
> #define JZ4740_FB_SPECIAL_TFT_CONFIG(start, stop) (((start) << 16) | (stop))
> @@ -62,6 +69,32 @@ struct jz4740_fb_platform_data {
>
> unsigned pixclk_falling_edge:1;
> unsigned date_enable_active_low:1;
> + unsigned chip_select_active_low:1;
> + unsigned register_select_active_low:1;
> };
>
> +struct platform_device;
> +
> +/*
> + * JzFB SLCD related functions
> + *
> + * jz4740_fb_slcd_disable_transfer: disables the current image transfer going
> + * on between memory and the LCD controller
> + * jz4740_fb_slcd_enable_transfer: the reverse operation of the above
> + * jz4740_fb_slcd_send_cmd: sends a command without any data to the LCD
> + * controller
> + * jz4740_fb_slcd_send_cmd_data: sends a command with a data argument to the LCD
> + * controller
> + *
> + * These functions can sleep and thus should not be called from an atomic
> + * context. Also, make sure you disable the SLCD image transfer *before* sending
> + * any commands and do not forget to re-enable it.
> + */
> +extern void jz4740_fb_slcd_disable_transfer(struct platform_device *pdev);
> +extern void jz4740_fb_slcd_enable_transfer(struct platform_device *pdev);
> +extern void jz4740_fb_slcd_send_cmd_data(struct platform_device *pdev,
> + unsigned int cmd, unsigned int data);
> +extern void jz4740_fb_slcd_send_cmd(struct platform_device *pdev,
> + unsigned int cmd);
> +
> #endif
> diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
> index 2f3ea57..4064812 100644
> --- a/drivers/video/jz4740_fb.c
> +++ b/drivers/video/jz4740_fb.c
> @@ -26,6 +26,7 @@
>
> #include <linux/dma-mapping.h>
>
> +#include <asm/mach-jz4740/dma.h>
> #include <asm/mach-jz4740/jz4740_fb.h>
> #include <asm/mach-jz4740/gpio.h>
>
> @@ -107,6 +108,40 @@
>
> #define JZ_LCD_STATE_DISABLED BIT(0)
>
> +#define JZ_REG_SLCD_CFG 0xA0
> +#define JZ_REG_SLCD_CTRL 0xA4
> +#define JZ_REG_SLCD_STATE 0xA8
> +#define JZ_REG_SLCD_DATA 0xAC
> +#define JZ_REG_SLCD_FIFO 0xB0
> +
> +#define JZ_SLCD_CFG_BURST_8_WORD BIT(14)
> +#define JZ_SLCD_CFG_DWIDTH_MASK (7 << 10)
> +#define JZ_SLCD_CFG_DWIDTH_18 (0 << 10)
> +#define JZ_SLCD_CFG_DWIDTH_16 (1 << 10)
> +#define JZ_SLCD_CFG_DWIDTH_8_x3 (2 << 10)
> +#define JZ_SLCD_CFG_DWIDTH_8_x2 (3 << 10)
> +#define JZ_SLCD_CFG_DWIDTH_8_x1 (4 << 10)
> +#define JZ_SLCD_CFG_DWIDTH_9_x2 (7 << 10)
> +#define JZ_SLCD_CFG_CWIDTH_MASK (3 << 8)
> +#define JZ_SLCD_CFG_CWIDTH(n) ((n) << 8)
> +#define JZ_SLCD_CFG_CWIDTH_16BIT (0 << 8)
> +#define JZ_SLCD_CFG_CWIDTH_8BIT (1 << 8)
> +#define JZ_SLCD_CFG_CWIDTH_18BIT (2 << 8)
> +#define JZ_SLCD_CFG_CS_ACTIVE_HIGH BIT(4)
> +#define JZ_SLCD_CFG_RS_CMD_HIGH BIT(3)
> +#define JZ_SLCD_CFG_CLK_ACTIVE_RISING BIT(1)
> +#define JZ_SLCD_CFG_TYPE_SERIAL BIT(0)
> +
> +#define JZ_SLCD_CTRL_DMA_EN (1 << 0)
> +
> +#define JZ_SLCD_STATE_BUSY (1 << 0)
> +
> +#define JZ_SLCD_DATA_RS_DATA (0 << 31)
> +#define JZ_SLCD_DATA_RS_COMMAND (1 << 31)
> +
> +#define JZ_SLCD_FIFO_RS_DATA (0 << 31)
> +#define JZ_SLCD_FIFO_RS_COMMAND (1 << 31)
> +
> struct jzfb_framedesc {
> uint32_t next;
> uint32_t addr;
> @@ -118,6 +153,7 @@ struct jzfb {
> struct fb_info *fb;
> struct platform_device *pdev;
> void __iomem *base;
> + phys_t phys_base;
> struct resource *mem;
> struct jz4740_fb_platform_data *pdata;
>
> @@ -126,6 +162,7 @@ struct jzfb {
> dma_addr_t vidmem_phys;
> struct jzfb_framedesc *framedesc;
> dma_addr_t framedesc_phys;
> + struct jz4740_dma_chan *slcd_dma;
>
> struct clk *ldclk;
> struct clk *lpclk;
> @@ -136,6 +173,9 @@ struct jzfb {
> uint32_t pseudo_palette[16];
> };
>
> +#define JZFB_IS_SLCD(jzfb) ((jzfb)->pdata->lcd_type & (1 << 5))
> +#define JZFB_IS_SLCD_SERIAL_TYPE(jzfb) ((jzfb)->pdata->lcd_type & (2 << 5))
These two should be static inline bool.
> +static void send_panel_command(struct jzfb *jzfb, u32 cmd)
You've been using u{16,32} throughout the whole patch, since the existing code
uses uint_{16,32}t you should stick to them.
> +{
> + u16 slcd_cfg = readw(jzfb->base + JZ_REG_SLCD_CFG);
> +
> + jzfb_slcd_wait(jzfb);
> +
> + switch (slcd_cfg & JZ_SLCD_CFG_CWIDTH_MASK) {
> + case JZ_SLCD_CFG_CWIDTH_8BIT:
> + writel(JZ_SLCD_DATA_RS_COMMAND | ((cmd & 0xff00) >> 8),
> + jzfb->base + JZ_REG_SLCD_DATA);
> + jzfb_slcd_wait(jzfb);
> + writel(JZ_SLCD_DATA_RS_COMMAND | (cmd & 0xff),
> + jzfb->base + JZ_REG_SLCD_DATA);
> + break;
> +
> + case JZ_SLCD_CFG_CWIDTH_16BIT:
> + writel(JZ_SLCD_DATA_RS_COMMAND | (cmd & 0xffff),
> + jzfb->base + JZ_REG_SLCD_DATA);
> + break;
> +
> + case JZ_SLCD_CFG_CWIDTH_18BIT:
> + writel(JZ_SLCD_DATA_RS_COMMAND | ((cmd & 0xff00) << 2) |
> + ((cmd & 0xff) << 1), jzfb->base + JZ_REG_SLCD_DATA);
> + break;
> + }
> +}
> +
> +static void send_panel_data(struct jzfb *jzfb, u32 data)
> +{
> + u16 slcd_cfg = readw(jzfb->base + JZ_REG_SLCD_CFG);
> +
> + switch (slcd_cfg & JZ_SLCD_CFG_DWIDTH_MASK) {
> + case JZ_SLCD_CFG_DWIDTH_18:
> + case JZ_SLCD_CFG_DWIDTH_9_x2:
> + data = ((data & 0xff) << 1) | ((data & 0xff00) << 2);
> + data = ((data << 6) & 0xfc0000) | ((data << 4) & 0xfc00) |
> + ((data << 2) & 0x0000fc);
> + break;
> +
> + case JZ_SLCD_CFG_DWIDTH_16:
> + default:
> + data &= 0xffff;
> + break;
> + }
> +
> + jzfb_slcd_wait(jzfb);
> + writel(JZ_SLCD_DATA_RS_DATA | data, jzfb->base + JZ_REG_SLCD_DATA);
> +}
> +
> +void jz4740_fb_slcd_disable_transfer(struct platform_device *pdev)
> +{
> + struct jzfb *jzfb = platform_get_drvdata(pdev);
> +
> + mutex_lock(&jzfb->lock);
> +
> + if (jzfb->is_enabled) {
> + jz4740_dma_disable(jzfb->slcd_dma);
> + jzfb_slcd_wait(jzfb);
> + }
> +
> + mutex_unlock(&jzfb->lock);
> +}
> +EXPORT_SYMBOL_GPL(jz4740_fb_slcd_disable_transfer);
> +
> +void jz4740_fb_slcd_enable_transfer(struct platform_device *pdev)
> +{
> + struct jzfb *jzfb = platform_get_drvdata(pdev);
> +
> + mutex_lock(&jzfb->lock);
> +
> + if (jzfb->is_enabled)
> + jzfb_slcd_start_dma(jzfb);
> +
> + mutex_unlock(&jzfb->lock);
> +}
> +EXPORT_SYMBOL_GPL(jz4740_fb_slcd_enable_transfer);
> +
> +void jz4740_fb_slcd_send_cmd_data(struct platform_device *pdev,
> + unsigned int cmd, unsigned int data)
> +{
> + struct jzfb *jzfb = platform_get_drvdata(pdev);
> +
> + mutex_lock(&jzfb->lock);
> +
> + if (!jzfb->is_enabled)
> + clk_enable(jzfb->ldclk);
> +
> + send_panel_command(jzfb, cmd);
> + send_panel_data(jzfb, data);
> +
> + if (!jzfb->is_enabled) {
> + jzfb_slcd_wait(jzfb);
> + clk_disable(jzfb->ldclk);
> + }
> +
> + mutex_unlock(&jzfb->lock);
> +}
> +EXPORT_SYMBOL_GPL(jz4740_fb_slcd_send_cmd_data);
> +
> +void jz4740_fb_slcd_send_cmd(struct platform_device *pdev, unsigned int cmd)
> +{
> + struct jzfb *jzfb = platform_get_drvdata(pdev);
> +
> + mutex_lock(&jzfb->lock);
> +
> + if (!jzfb->is_enabled)
> + clk_enable(jzfb->ldclk);
> +
> + send_panel_command(jzfb, cmd);
> +
> + if (!jzfb->is_enabled) {
> + jzfb_slcd_wait(jzfb);
> + clk_disable(jzfb->ldclk);
> + }
> +
> + mutex_unlock(&jzfb->lock);
> +}
> +EXPORT_SYMBOL_GPL(jz4740_fb_slcd_send_cmd);
jz4740_fb_slcd_send_cmd_data and jz4740_fb_slcd_send_cmd are almost identical
it might makes sense to merge them.
I don't like the idea of adding a jzfb specific interface for talking to the
lcd panels, because it will tightly couple the panel driver to the framebuffer
driver and it won't be possible to reuse for some board using the same panel
but a different SoC.
- Lars
^ permalink raw reply
* Re: [PATCH 5/6] FBDEV: JZ4740: Get rid of enumeration value not handled
From: Lars-Peter Clausen @ 2011-03-01 14:23 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1202f26b58f8dea782600827a158f08b1775953b.1298980528.git.mcuelenaere@gmail.com>
On 03/01/2011 01:06 PM, Maurus Cuelenaere wrote:
> Convert enum jz4740_fb_lcd_type to #define's in order to get rid of these
> warnings.
>
Nack. If it is not handled the code is wrong.
If this is because you only handle SLCD types in some SLCD specific functions add a
default:
BUG();
> Signed-off-by: Maurus Cuelenaere <mcuelenaere@gmail.com>
> ---
> arch/mips/include/asm/mach-jz4740/jz4740_fb.h | 40 ++++++++++++-------------
> 1 files changed, 19 insertions(+), 21 deletions(-)
>
> diff --git a/arch/mips/include/asm/mach-jz4740/jz4740_fb.h b/arch/mips/include/asm/mach-jz4740/jz4740_fb.h
> index eaaac43..d6c11f2 100644
> --- a/arch/mips/include/asm/mach-jz4740/jz4740_fb.h
> +++ b/arch/mips/include/asm/mach-jz4740/jz4740_fb.h
> @@ -17,27 +17,25 @@
>
> #include <linux/fb.h>
>
> -enum jz4740_fb_lcd_type {
> - JZ_LCD_TYPE_GENERIC_16_BIT = 0,
> - JZ_LCD_TYPE_GENERIC_18_BIT = 0 | (1 << 4),
> - JZ_LCD_TYPE_SPECIAL_TFT_1 = 1,
> - JZ_LCD_TYPE_SPECIAL_TFT_2 = 2,
> - JZ_LCD_TYPE_SPECIAL_TFT_3 = 3,
> - JZ_LCD_TYPE_NON_INTERLACED_CCIR656 = 5,
> - JZ_LCD_TYPE_INTERLACED_CCIR656 = 7,
> - JZ_LCD_TYPE_SINGLE_COLOR_STN = 8,
> - JZ_LCD_TYPE_SINGLE_MONOCHROME_STN = 9,
> - JZ_LCD_TYPE_DUAL_COLOR_STN = 10,
> - JZ_LCD_TYPE_DUAL_MONOCHROME_STN = 11,
> - JZ_LCD_TYPE_8BIT_SERIAL = 12,
> +#define JZ_LCD_TYPE_GENERIC_16_BIT (0)
> +#define JZ_LCD_TYPE_GENERIC_18_BIT (0 | (1 << 4))
> +#define JZ_LCD_TYPE_SPECIAL_TFT_1 (1)
> +#define JZ_LCD_TYPE_SPECIAL_TFT_2 (2)
> +#define JZ_LCD_TYPE_SPECIAL_TFT_3 (3)
> +#define JZ_LCD_TYPE_NON_INTERLACED_CCIR656 (5)
> +#define JZ_LCD_TYPE_INTERLACED_CCIR656 (7)
> +#define JZ_LCD_TYPE_SINGLE_COLOR_STN (8)
> +#define JZ_LCD_TYPE_SINGLE_MONOCHROME_STN (9)
> +#define JZ_LCD_TYPE_DUAL_COLOR_STN (10)
> +#define JZ_LCD_TYPE_DUAL_MONOCHROME_STN (11)
> +#define JZ_LCD_TYPE_8BIT_SERIAL (12)
>
> - JZ_SLCD_TYPE_PARALLEL_8_BIT = 1 | (1 << 5),
> - JZ_SLCD_TYPE_PARALLEL_16_BIT = 0 | (1 << 5),
> - JZ_SLCD_TYPE_PARALLEL_18_BIT = 2 | (1 << 5),
> - JZ_SLCD_TYPE_SERIAL_8_BIT = 1 | (3 << 5),
> - JZ_SLCD_TYPE_SERIAL_16_BIT = 0 | (3 << 5),
> - JZ_SLCD_TYPE_SERIAL_18_BIT = 2 | (3 << 5),
> -};
> +#define JZ_SLCD_TYPE_PARALLEL_8_BIT (1 | (1 << 5))
> +#define JZ_SLCD_TYPE_PARALLEL_16_BIT (0 | (1 << 5))
> +#define JZ_SLCD_TYPE_PARALLEL_18_BIT (2 | (1 << 5))
> +#define JZ_SLCD_TYPE_SERIAL_8_BIT (1 | (3 << 5))
> +#define JZ_SLCD_TYPE_SERIAL_16_BIT (0 | (3 << 5))
> +#define JZ_SLCD_TYPE_SERIAL_18_BIT (2 | (3 << 5))
>
> #define JZ4740_FB_SPECIAL_TFT_CONFIG(start, stop) (((start) << 16) | (stop))
>
> @@ -58,7 +56,7 @@ struct jz4740_fb_platform_data {
> struct fb_videomode *modes;
>
> unsigned int bpp;
> - enum jz4740_fb_lcd_type lcd_type;
> + unsigned int lcd_type;
>
> struct {
> uint32_t spl;
Nack
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox