* [PATCH] pwm-backlight: Use devm_pwm_get in pwm_bl.c
From: Sachin Kamat @ 2012-09-11 8:54 UTC (permalink / raw)
To: linux-fbdev
This file already makes use of device managed functions.
Convert pwm_get() too to use it.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
drivers/video/backlight/pwm_bl.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 4965408..0c91023 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -218,7 +218,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
pb->exit = data->exit;
pb->dev = &pdev->dev;
- pb->pwm = pwm_get(&pdev->dev, NULL);
+ pb->pwm = devm_pwm_get(&pdev->dev, NULL);
if (IS_ERR(pb->pwm)) {
dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
@@ -251,7 +251,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
if (IS_ERR(bl)) {
dev_err(&pdev->dev, "failed to register backlight\n");
ret = PTR_ERR(bl);
- goto err_bl;
+ goto err_alloc;
}
bl->props.brightness = data->dft_brightness;
@@ -260,8 +260,6 @@ static int pwm_backlight_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, bl);
return 0;
-err_bl:
- pwm_put(pb->pwm);
err_alloc:
if (data->exit)
data->exit(&pdev->dev);
@@ -276,7 +274,6 @@ static int pwm_backlight_remove(struct platform_device *pdev)
backlight_device_unregister(bl);
pwm_config(pb->pwm, 0, pb->period);
pwm_disable(pb->pwm);
- pwm_put(pb->pwm);
if (pb->exit)
pb->exit(&pdev->dev);
return 0;
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH v2] fbdev: Add Renesas vdc4 framebuffer driver
From: Jingoo Han @ 2012-09-11 2:31 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1347267372-22949-1-git-send-email-phil.edworthy@renesas.com>
On Monday, September 10, 2012 5:56 PM Phil Edworthy wrote
>
> The vdc4 display hardware is found on the sh7269 device.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Reviewed-by: Jingoo Han <jg1.han@samsung.com>
Best regards,
Jingoo Han
> ---
> v2:
> * Use devm_ variants of clk_get, ioremap_nocache, request_irq.
> * Replace spaces with tabs.
> * Check ren_vdc4_start return value.
> * Fix headers used.
>
> drivers/video/Kconfig | 10 +
> drivers/video/Makefile | 1 +
> drivers/video/ren_vdc4fb.c | 641 ++++++++++++++++++++++++++++++++++++++++++++
> include/video/ren_vdc4fb.h | 19 ++
> 4 files changed, 671 insertions(+), 0 deletions(-)
> create mode 100644 drivers/video/ren_vdc4fb.c
> create mode 100644 include/video/ren_vdc4fb.h
^ permalink raw reply
* [PATCH v3] OMAPDSS: Fix IRQ unregister race
From: Dimitar Dimitrov @ 2012-09-10 19:34 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Dimitar Dimitrov, stable
In-Reply-To: <201209102219.29105.dinuxbg@gmail.com>
Very rare kernel crashes are reported on a custom OMAP4 board. Kernel
panics due to corrupted completion structure while executing
dispc_irq_wait_handler(). Excerpt from kernel log:
Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP
Unable to handle kernel paging request at virtual address 00400130
...
PC is at 0xebf205bc
LR is at __wake_up_common+0x54/0x94
...
(__wake_up_common+0x0/0x94)
(complete+0x0/0x60)
(dispc_irq_wait_handler.36902+0x0/0x14)
(omap_dispc_irq_handler+0x0/0x354)
(handle_irq_event_percpu+0x0/0x188)
(handle_irq_event+0x0/0x64)
(handle_fasteoi_irq+0x0/0x10c)
(generic_handle_irq+0x0/0x48)
(asm_do_IRQ+0x0/0xc0)
DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
unregister_isr() and DISPC IRQ might be running in parallel on different
CPUs. So there is a chance that a callback is executed even though it
has been unregistered. As omap_dispc_wait_for_irq_timeout() declares a
completion on stack, the dispc_irq_wait_handler() callback might try to
access a completion structure that is invalid. This leads to crashes and
hangs.
Solution is to divide unregister calls into two sets:
1. Non-strict unregistering of callbacks. Callbacks could safely be
executed after unregistering them. This is the case with unregister
calls from the IRQ handler itself.
2. Strict (synchronized) unregistering. Callbacks are not allowed
after unregistering. This is the case with completion waiting.
The above solution should satisfy one of the original intentions of the
driver: callbacks should be able to unregister themselves.
Also fix DSI IRQ unregister which has similar logic to DISPC IRQ handling.
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Dimitar Dimitrov <dinuxbg@gmail.com>
---
Changes since v2:
- Fix formatting per Tomi Valkeinen's comments.
- Restored accidental removal of EXPORT_SYMBOL for omap_dispc_register_isr.
drivers/staging/omapdrm/omap_plane.c | 2 +-
drivers/video/omap2/dss/apply.c | 2 +-
drivers/video/omap2/dss/dispc.c | 34 +++++++++++++++++++++++++++++++++-
drivers/video/omap2/dss/dsi.c | 15 +++++++++++++++
include/video/omapdss.h | 1 +
5 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/omapdrm/omap_plane.c b/drivers/staging/omapdrm/omap_plane.c
index 7997be7..8d8aa5b 100644
--- a/drivers/staging/omapdrm/omap_plane.c
+++ b/drivers/staging/omapdrm/omap_plane.c
@@ -82,7 +82,7 @@ static void dispc_isr(void *arg, uint32_t mask)
struct omap_plane *omap_plane = to_omap_plane(plane);
struct omap_drm_private *priv = plane->dev->dev_private;
- omap_dispc_unregister_isr(dispc_isr, plane,
+ omap_dispc_unregister_isr_nosync(dispc_isr, plane,
id2irq[omap_plane->ovl->id]);
queue_work(priv->wq, &omap_plane->work);
diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 0fefc68..9386834 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -847,7 +847,7 @@ static void dss_unregister_vsync_isr(void)
for (i = 0; i < num_mgrs; ++i)
mask |= dispc_mgr_get_framedone_irq(i);
- r = omap_dispc_unregister_isr(dss_apply_irq_handler, NULL, mask);
+ r = omap_dispc_unregister_isr_nosync(dss_apply_irq_handler, NULL, mask);
WARN_ON(r);
dss_data.irq_enabled = false;
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index ee9e296..6032252 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -3320,7 +3320,8 @@ err:
}
EXPORT_SYMBOL(omap_dispc_register_isr);
-int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
+/* WARNING: callback might be executed even after this function returns! */
+int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void *arg, u32 mask)
{
int i;
unsigned long flags;
@@ -3352,6 +3353,37 @@ int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
return ret;
}
+EXPORT_SYMBOL(omap_dispc_unregister_isr_nosync);
+
+/*
+ * Ensure that callback <isr> will NOT be executed after this function
+ * returns. Must be called from sleepable context, though!
+ */
+int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
+{
+ int ret;
+
+ ret = omap_dispc_unregister_isr_nosync(isr, arg, mask);
+
+ /*
+ * Task context is not really needed. But if we're called from atomic
+ * context, it is probably from DISPC IRQ, where we will deadlock.
+ * So use might_sleep() to catch potential deadlocks.
+ */
+ might_sleep();
+
+ /*
+ * DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
+ * unregister_isr() and DISPC IRQ might be running in parallel on
+ * different CPUs. So there is a chance that a callback is executed
+ * even though it has been unregistered. Add a barrier, in order to
+ * ensure that after returning from this function, the new DISPC IRQ
+ * will use an updated callback array, and NOT its cached copy.
+ */
+ synchronize_irq(dispc.irq);
+
+ return ret;
+}
EXPORT_SYMBOL(omap_dispc_unregister_isr);
#ifdef DEBUG
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index b07e886..c0fcb68 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -960,6 +960,11 @@ static int dsi_unregister_isr(struct platform_device *dsidev,
spin_unlock_irqrestore(&dsi->irq_lock, flags);
+ might_sleep();
+
+ /* See notes for dispc.c:omap_dispc_unregister_isr() . */
+ synchronize_irq(dsi->irq);
+
return r;
}
@@ -1002,6 +1007,11 @@ static int dsi_unregister_isr_vc(struct platform_device *dsidev, int channel,
spin_unlock_irqrestore(&dsi->irq_lock, flags);
+ might_sleep();
+
+ /* See notes for dispc.c:omap_dispc_unregister_isr() . */
+ synchronize_irq(dsi->irq);
+
return r;
}
@@ -1042,6 +1052,11 @@ static int dsi_unregister_isr_cio(struct platform_device *dsidev,
spin_unlock_irqrestore(&dsi->irq_lock, flags);
+ might_sleep();
+
+ /* See notes for dispc.c:omap_dispc_unregister_isr() . */
+ synchronize_irq(dsi->irq);
+
return r;
}
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index a6267a2..769f981 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -708,6 +708,7 @@ void omapdss_default_get_timings(struct omap_dss_device *dssdev,
typedef void (*omap_dispc_isr_t) (void *arg, u32 mask);
int omap_dispc_register_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
+int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void *arg, u32 mask);
int omap_dispc_wait_for_irq_timeout(u32 irqmask, unsigned long timeout);
int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
--
1.7.10.4
^ permalink raw reply related
* Re: [RFC PATCH v2] OMAPDSS: Fix IRQ unregister race
From: Dimitar Dimitrov @ 2012-09-10 19:19 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1347263345.8127.22.camel@deskari>
On Monday 10 September 2012 10:49:05 Tomi Valkeinen wrote:
> On Sat, 2012-09-08 at 18:05 +0300, Dimitar Dimitrov wrote:
> > Very rare kernel crashes are reported on a custom OMAP4 board. Kernel
> > panics due to corrupted completion structure while executing
> >
> > dispc_irq_wait_handler(). Excerpt from kernel log:
> > Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP
> > Unable to handle kernel paging request at virtual address 00400130
> > ...
> > PC is at 0xebf205bc
> > LR is at __wake_up_common+0x54/0x94
> > ...
> > (__wake_up_common+0x0/0x94)
> > (complete+0x0/0x60)
> > (dispc_irq_wait_handler.36902+0x0/0x14)
> > (omap_dispc_irq_handler+0x0/0x354)
> > (handle_irq_event_percpu+0x0/0x188)
> > (handle_irq_event+0x0/0x64)
> > (handle_fasteoi_irq+0x0/0x10c)
> > (generic_handle_irq+0x0/0x48)
> > (asm_do_IRQ+0x0/0xc0)
> >
> > DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
> > unregister_isr() and DISPC IRQ might be running in parallel on different
> > CPUs. So there is a chance that a callback is executed even though it
> > has been unregistered. As omap_dispc_wait_for_irq_timeout() declares a
> > completion on stack, the dispc_irq_wait_handler() callback might try to
> > access a completion structure that is invalid. This leads to crashes and
> > hangs.
> >
> > Solution is to divide unregister calls into two sets:
> > 1. Non-strict unregistering of callbacks. Callbacks could safely be
> >
> > executed after unregistering them. This is the case with unregister
> > calls from the IRQ handler itself.
> >
> > 2. Strict (synchronized) unregistering. Callbacks are not allowed
> >
> > after unregistering. This is the case with completion waiting.
> >
> > The above solution should satisfy one of the original intentions of the
> > driver: callbacks should be able to unregister themselves.
> >
> > Also fix DSI IRQ unregister which has similar logic to DISPC IRQ
> > handling.
> >
> > Signed-off-by: Dimitar Dimitrov <dinuxbg@gmail.com>
> > ---
> >
> > WARNING: This bug is quite old. The patch has been tested on v3.0. No
> > testing has been done after rebasing to v3.6. Hence the RFC tag.
> > Hopefully someone will beat me in testing with latest linux-omap/master.
> >
> > Changes since v1 per Tomi Valkeinen's comments:
> > - Don't rename omap_dispc_unregister_isr, just introduce nosync
> > variant. - Apply the same fix for DSI IRQ which suffers from the same
> > race condition.
>
> I made a quick test and works for me, although I haven't encountered the
> problem itself.
This issue is very rarely seen during normal operation. You may not hit it
even after days of stress testing. To speed up reproducing an artificial delay
in the IRQ routine could be used while stress-testing DSS, e.g.:
@@ -3462,6 +3462,8 @@ static irqreturn_t omap_dispc_irq_handler(int irq, void
*a
spin_unlock(&dispc.irq_lock);
+ { static int iii; if ((iii++ % 10) = 0) mdelay(100); }
+
for (i = 0; i < DISPC_MAX_NR_ISRS; i++) {
isr_data = ®istered_isr[i];
>
> Some mostly cosmetic comments below.
>
> This seems to apply cleanly to v3.4+ kernels, but not earlier ones. Do
> you want to make the needed modifications and mail this and the modified
> patches for stable kernels also? I can do that also if you don't want
> to.
I can do the rebase and cleanup. I'll send the modified patch per your
comments in the next hour.
Currently I cannot test, though. I hope to have a working setup by end of week
in order to send sanity-tested patches for stable kernels.
>
> > drivers/staging/omapdrm/omap_plane.c | 2 +-
> > drivers/video/omap2/dss/apply.c | 2 +-
> > drivers/video/omap2/dss/dispc.c | 45
> > +++++++++++++++++++++++++++++----- drivers/video/omap2/dss/dsi.c
> > | 19 ++++++++++++++
> > include/video/omapdss.h | 1 +
> > 5 files changed, 61 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/omapdrm/omap_plane.c
> > b/drivers/staging/omapdrm/omap_plane.c index 7997be7..8d8aa5b 100644
> > --- a/drivers/staging/omapdrm/omap_plane.c
> > +++ b/drivers/staging/omapdrm/omap_plane.c
> > @@ -82,7 +82,7 @@ static void dispc_isr(void *arg, uint32_t mask)
> >
> > struct omap_plane *omap_plane = to_omap_plane(plane);
> > struct omap_drm_private *priv = plane->dev->dev_private;
> >
> > - omap_dispc_unregister_isr(dispc_isr, plane,
> > + omap_dispc_unregister_isr_nosync(dispc_isr, plane,
> >
> > id2irq[omap_plane->ovl->id]);
> >
> > queue_work(priv->wq, &omap_plane->work);
> >
> > diff --git a/drivers/video/omap2/dss/apply.c
> > b/drivers/video/omap2/dss/apply.c index 0fefc68..9386834 100644
> > --- a/drivers/video/omap2/dss/apply.c
> > +++ b/drivers/video/omap2/dss/apply.c
> > @@ -847,7 +847,7 @@ static void dss_unregister_vsync_isr(void)
> >
> > for (i = 0; i < num_mgrs; ++i)
> >
> > mask |= dispc_mgr_get_framedone_irq(i);
> >
> > - r = omap_dispc_unregister_isr(dss_apply_irq_handler, NULL, mask);
> > + r = omap_dispc_unregister_isr_nosync(dss_apply_irq_handler, NULL,
> > mask);
> >
> > WARN_ON(r);
> >
> > dss_data.irq_enabled = false;
> >
> > diff --git a/drivers/video/omap2/dss/dispc.c
> > b/drivers/video/omap2/dss/dispc.c index ee9e296..a67d92c 100644
> > --- a/drivers/video/omap2/dss/dispc.c
> > +++ b/drivers/video/omap2/dss/dispc.c
> > @@ -2421,8 +2421,8 @@ static void dispc_mgr_enable_digit_out(bool enable)
> >
> > enable ? "start" : "stop");
> >
> > }
> >
> > - r = omap_dispc_unregister_isr(dispc_disable_isr,
> > &frame_done_completion, - irq_mask);
> > + r = omap_dispc_unregister_isr(dispc_disable_isr,
> > + &frame_done_completion, irq_mask);
>
> This change is not needed.
Sorry, I missed those. Will remove and resend.
>
> > if (r)
> >
> > DSSERR("failed to unregister %x isr\n", irq_mask);
> >
> > @@ -3320,7 +3320,8 @@ err:
> > }
> > EXPORT_SYMBOL(omap_dispc_register_isr);
> >
> > -int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
> > +/* WARNING: callback might be executed even after this function returns!
> > */ +int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void
> > *arg, u32 mask)
> >
> > {
> >
> > int i;
> > unsigned long flags;
> >
> > @@ -3352,7 +3353,37 @@ int omap_dispc_unregister_isr(omap_dispc_isr_t
> > isr, void *arg, u32 mask)
> >
> > return ret;
> >
> > }
> >
> > -EXPORT_SYMBOL(omap_dispc_unregister_isr);
> > +EXPORT_SYMBOL(omap_dispc_unregister_isr_nosync);
> > +
> > +/*
> > + * Ensure that callback <isr> will NOT be executed after this function
> > + * returns. Must be called from sleepable context, though!
> > + */
> > +int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
> > +{
> > + int ret;
> > +
> > + ret = omap_dispc_unregister_isr_nosync(isr, arg, mask);
> > +
> > + /* Task context is not really needed. But if we're called from atomic
> > + * context, it is probably from DISPC IRQ, where we will deadlock.
> > + * So use might_sleep() to catch potential deadlocks.
> > + */
>
> Use the kernel multi-line comment style, i.e.:
>
> /*
> * foobar
> */
Will fix.
>
> > + might_sleep();
> > +
> > +#if defined(CONFIG_SMP)
> > + /* DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
> > + * unregister_isr() and DISPC IRQ might be running in parallel on
> > + * different CPUs. So there is a chance that a callback is executed
> > + * even though it has been unregistered. Add a barrier, in order to
> > + * ensure that after returning from this function, the new DISPC IRQ
> > + * will use an updated callback array, and NOT its cached copy.
> > + */
>
> Comment style here also.
Will fix.
>
> > + synchronize_irq(dispc.irq);
> > +#endif
>
> Why do you use #if defined(CONFIG_SMP)? synchronize_irq is defined
> with !CONFIG_SMP also. In that case it becomes just barrier().
>
You're correct. I'll remove "defined(CONFIG_SMP)".
> > +
> > + return ret;
> > +}
I forgot to keep EXPORT_SYMBOL for omap_dispc_unregister_isr. Will add it.
> >
> > #ifdef DEBUG
> > static void print_irq_status(u32 status)
> >
> > @@ -3567,7 +3598,8 @@ int omap_dispc_wait_for_irq_timeout(u32 irqmask,
> > unsigned long timeout)
> >
> > timeout = wait_for_completion_timeout(&completion, timeout);
> >
> > - omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion,
> > irqmask); + omap_dispc_unregister_isr(dispc_irq_wait_handler,
> > &completion, + irqmask);
>
> Change not needed.
Will remove.
>
> > if (timeout = 0)
> >
> > return -ETIMEDOUT;
> >
> > @@ -3598,7 +3630,8 @@ int
> > omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
> >
> > timeout = wait_for_completion_interruptible_timeout(&completion,
> >
> > timeout);
> >
> > - omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion,
> > irqmask); + omap_dispc_unregister_isr(dispc_irq_wait_handler,
> > &completion, + irqmask);
>
> Change not needed.
Will remove.
>
> > if (timeout = 0)
> >
> > return -ETIMEDOUT;
> >
> > diff --git a/drivers/video/omap2/dss/dsi.c
> > b/drivers/video/omap2/dss/dsi.c index b07e886..24b4a3e 100644
> > --- a/drivers/video/omap2/dss/dsi.c
> > +++ b/drivers/video/omap2/dss/dsi.c
> > @@ -960,6 +960,13 @@ static int dsi_unregister_isr(struct platform_device
> > *dsidev,
> >
> > spin_unlock_irqrestore(&dsi->irq_lock, flags);
> >
> > + might_sleep();
> > +
> > +#if defined(CONFIG_SMP)
> > + /* See notes for dispc.c:omap_dispc_unregister_isr() . */
> > + synchronize_irq(dsi->irq);
> > +#endif
> > +
>
> Same SMP comment as with dispc.
>
> Tomi
Thanks,
Dimitar
^ permalink raw reply
* [PATCH 2/2] video: s3c2410: fix checkpatch warnings
From: Jingoo Han @ 2012-09-10 10:55 UTC (permalink / raw)
To: linux-fbdev
This patch fixes the checkpatch warnings listed below:
WARNING: usleep_range should not use min = max args; see Documentation/timers/timers-howto.txt
WARNING: quoted string split across lines
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/s3c2410fb.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/video/s3c2410fb.c b/drivers/video/s3c2410fb.c
index 1aa37ea..1083bb9 100644
--- a/drivers/video/s3c2410fb.c
+++ b/drivers/video/s3c2410fb.c
@@ -935,7 +935,7 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
clk_enable(info->clk);
dprintk("got and enabled clock\n");
- usleep_range(1000, 1000);
+ usleep_range(1000, 1100);
info->clk_rate = clk_get_rate(info->clk);
@@ -1034,7 +1034,7 @@ static int __devexit s3c2410fb_remove(struct platform_device *pdev)
s3c2410fb_cpufreq_deregister(info);
s3c2410fb_lcd_enable(info, 0);
- usleep_range(1000, 1000);
+ usleep_range(1000, 1100);
s3c2410fb_unmap_video_memory(fbinfo);
@@ -1071,7 +1071,7 @@ static int s3c2410fb_suspend(struct platform_device *dev, pm_message_t state)
* the LCD DMA engine is not going to get back on the bus
* before the clock goes off again (bjd) */
- usleep_range(1000, 1000);
+ usleep_range(1000, 1100);
clk_disable(info->clk);
return 0;
@@ -1083,7 +1083,7 @@ static int s3c2410fb_resume(struct platform_device *dev)
struct s3c2410fb_info *info = fbinfo->par;
clk_enable(info->clk);
- usleep_range(1000, 1000);
+ usleep_range(1000, 1100);
s3c2410fb_init_registers(fbinfo);
@@ -1140,8 +1140,8 @@ static void __exit s3c2410fb_cleanup(void)
module_init(s3c2410fb_init);
module_exit(s3c2410fb_cleanup);
-MODULE_AUTHOR("Arnaud Patard <arnaud.patard@rtp-net.org>, "
- "Ben Dooks <ben-linux@fluff.org>");
+MODULE_AUTHOR("Arnaud Patard <arnaud.patard@rtp-net.org>");
+MODULE_AUTHOR("Ben Dooks <ben-linux@fluff.org>");
MODULE_DESCRIPTION("Framebuffer driver for the s3c2410");
MODULE_LICENSE("GPL");
MODULE_ALIAS("platform:s3c2410-lcd");
--
1.7.1
^ permalink raw reply related
* [PATCH 1/2] video: s3c2410: Use pr_* and dev_* instead of printk
From: Jingoo Han @ 2012-09-10 10:54 UTC (permalink / raw)
To: linux-fbdev
From: Sachin Kamat <sachin.kamat@linaro.org>
printk calls are replaced by pr_* and dev_* calls to silence
checkpatch warnings.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/s3c2410fb.c | 22 ++++++++++++++--------
1 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/video/s3c2410fb.c b/drivers/video/s3c2410fb.c
index 77f34c6..1aa37ea 100644
--- a/drivers/video/s3c2410fb.c
+++ b/drivers/video/s3c2410fb.c
@@ -11,6 +11,8 @@
* Driver based on skeletonfb.c, sa1100fb.c and others.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/err.h>
@@ -48,7 +50,11 @@ static int debug = 1;
static int debug;
#endif
-#define dprintk(msg...) if (debug) printk(KERN_DEBUG "s3c2410fb: " msg);
+#define dprintk(msg...) \
+do { \
+ if (debug) \
+ pr_debug(msg); \
+} while (0)
/* useful functions */
@@ -598,11 +604,11 @@ static int s3c2410fb_debug_store(struct device *dev,
if (strnicmp(buf, "on", 2) = 0 ||
strnicmp(buf, "1", 1) = 0) {
debug = 1;
- printk(KERN_DEBUG "s3c2410fb: Debug On");
+ dev_dbg(dev, "s3c2410fb: Debug On");
} else if (strnicmp(buf, "off", 3) = 0 ||
strnicmp(buf, "0", 1) = 0) {
debug = 0;
- printk(KERN_DEBUG "s3c2410fb: Debug Off");
+ dev_dbg(dev, "s3c2410fb: Debug Off");
} else {
return -EINVAL;
}
@@ -921,7 +927,7 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
info->clk = clk_get(NULL, "lcd");
if (IS_ERR(info->clk)) {
- printk(KERN_ERR "failed to get lcd clock source\n");
+ dev_err(&pdev->dev, "failed to get lcd clock source\n");
ret = PTR_ERR(info->clk);
goto release_irq;
}
@@ -947,7 +953,7 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
/* Initialize video memory */
ret = s3c2410fb_map_video_memory(fbinfo);
if (ret) {
- printk(KERN_ERR "Failed to allocate video RAM: %d\n", ret);
+ dev_err(&pdev->dev, "Failed to allocate video RAM: %d\n", ret);
ret = -ENOMEM;
goto release_clock;
}
@@ -970,7 +976,7 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
ret = register_framebuffer(fbinfo);
if (ret < 0) {
- printk(KERN_ERR "Failed to register framebuffer device: %d\n",
+ dev_err(&pdev->dev, "Failed to register framebuffer device: %d\n",
ret);
goto free_cpufreq;
}
@@ -978,9 +984,9 @@ static int __devinit s3c24xxfb_probe(struct platform_device *pdev,
/* create device files */
ret = device_create_file(&pdev->dev, &dev_attr_debug);
if (ret)
- printk(KERN_ERR "failed to add debug attribute\n");
+ dev_err(&pdev->dev, "failed to add debug attribute\n");
- printk(KERN_INFO "fb%d: %s frame buffer device\n",
+ dev_info(&pdev->dev, "fb%d: %s frame buffer device\n",
fbinfo->node, fbinfo->fix.id);
return 0;
--
1.7.1
^ permalink raw reply related
* [PATCH v2] fbdev: Add Renesas vdc4 framebuffer driver
From: Phil Edworthy @ 2012-09-10 8:56 UTC (permalink / raw)
To: linux-fbdev
The vdc4 display hardware is found on the sh7269 device.
Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v2:
* Use devm_ variants of clk_get, ioremap_nocache, request_irq.
* Replace spaces with tabs.
* Check ren_vdc4_start return value.
* Fix headers used.
drivers/video/Kconfig | 10 +
drivers/video/Makefile | 1 +
drivers/video/ren_vdc4fb.c | 641 ++++++++++++++++++++++++++++++++++++++++++++
include/video/ren_vdc4fb.h | 19 ++
4 files changed, 671 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/ren_vdc4fb.c
create mode 100644 include/video/ren_vdc4fb.h
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 0217f74..89c9250 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1990,6 +1990,16 @@ config FB_W100
If unsure, say N.
+config FB_REN_VDC4FB
+ tristate "Renesas VDC4 framebuffer support"
+ depends on FB && CPU_SUBTYPE_SH7269
+ select FB_SYS_FILLRECT
+ select FB_SYS_COPYAREA
+ select FB_SYS_IMAGEBLIT
+ select FB_SYS_FOPS
+ ---help---
+ Frame buffer driver for the Renesas VDC4.
+
config FB_SH_MOBILE_LCDC
tristate "SuperH Mobile LCDC framebuffer support"
depends on FB && (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index ee8dafb..ba69fcb 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -140,6 +140,7 @@ obj-$(CONFIG_SH_MIPI_DSI) += sh_mipi_dsi.o
obj-$(CONFIG_FB_SH_MOBILE_HDMI) += sh_mobile_hdmi.o
obj-$(CONFIG_FB_SH_MOBILE_MERAM) += sh_mobile_meram.o
obj-$(CONFIG_FB_SH_MOBILE_LCDC) += sh_mobile_lcdcfb.o
+obj-$(CONFIG_FB_REN_VDC4FB) += ren_vdc4fb.o
obj-$(CONFIG_FB_OMAP) += omap/
obj-y += omap2/
obj-$(CONFIG_XEN_FBDEV_FRONTEND) += xen-fbfront.o
diff --git a/drivers/video/ren_vdc4fb.c b/drivers/video/ren_vdc4fb.c
new file mode 100644
index 0000000..73daee3
--- /dev/null
+++ b/drivers/video/ren_vdc4fb.c
@@ -0,0 +1,641 @@
+/*
+ * Renesas VDC4 Framebuffer
+ *
+ * Based on sh_mobile_lcdcfb.c
+ * Copyright (c) 2012 Renesas Electronics Europe Ltd
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/vmalloc.h>
+#include <linux/module.h>
+#include <video/ren_vdc4fb.h>
+
+#define PALETTE_NR 16
+
+struct ren_vdc4_priv {
+ void __iomem *base;
+ int irq;
+ struct clk *dot_clk;
+ struct clk *clk;
+ struct fb_info *info;
+ dma_addr_t dma_handle;
+ struct ren_vdc4_info *cfg;
+ u32 pseudo_palette[PALETTE_NR];
+};
+
+/* Register offsets/reading and writing functions */
+enum {
+ SCL0_UPDATE, SCL0_FRC1, SCL0_FRC2, SCL0_FRC3,
+ SCL0_FRC4, SCL0_FRC5, SCL0_FRC6, SCL0_FRC7,
+ SCL0_DS1, SCL0_US1,
+
+ GR1_UPDATE, GR1_AB1,
+
+ GR2_UPDATE, GR2_AB1,
+
+ GR3_UPDATE, GR3_FLM_RD, GR3_FLM1, GR3_FLM2,
+ GR3_FLM3, GR3_FLM4, GR3_FLM5, GR3_FLM6, GR3_AB1,
+ GR3_AB2, GR3_AB3, GR3_AB4, GR3_AB5, GR3_AB6,
+ GR3_AB7, GR3_AB8, GR3_AB9, GR3_AB10, GR3_AB11,
+ GR3_BASE, GR3_CLUT_INT, GR3_MON,
+
+ TCON_UPDATE, TCON_TIM, TCON_TIM_STVA1, TCON_TIM_STVA2,
+ TCON_TIM_STVB1, TCON_TIM_STVB2, TCON_TIM_STH1,
+ TCON_TIM_STH2, TCON_TIM_STB1, TCON_TIM_STB2,
+ TCON_TIM_CPV1, TCON_TIM_CPV2, TCON_TIM_POLA1,
+ TCON_TIM_POLA2, TCON_TIM_POLB1, TCON_TIM_POLB2,
+ TCON_TIM_DE,
+
+ OUT_UPDATE, OUT_SET, OUT_BRIGHT1,
+ OUT_BRIGHT2, OUT_CONTRAST, OUT_PDTHA, OUT_CLK_PHASE,
+
+ SYSCNT_INT1, SYSCNT_INT2, SYSCNT_INT3, SYSCNT_INT4,
+ SYSCNT_PANEL_CLK, SYSCNT_CLUT
+};
+
+static unsigned long vdc4_offsets[] = {
+ [SCL0_UPDATE] = 0x0100,
+ [SCL0_FRC1] = 0x0104,
+ [SCL0_FRC2] = 0x0108,
+ [SCL0_FRC3] = 0x010C,
+ [SCL0_FRC4] = 0x0110,
+ [SCL0_FRC5] = 0x0114,
+ [SCL0_FRC6] = 0x0118,
+ [SCL0_FRC7] = 0x011C,
+ [SCL0_DS1] = 0x012C,
+ [SCL0_US1] = 0x0148,
+ [GR1_UPDATE] = 0x0200,
+ [GR1_AB1] = 0x0220,
+ [GR2_UPDATE] = 0x0300,
+ [GR2_AB1] = 0x0320,
+ [GR3_UPDATE] = 0x0380,
+ [GR3_FLM_RD] = 0x0384,
+ [GR3_FLM1] = 0x0388,
+ [GR3_FLM2] = 0x038C,
+ [GR3_FLM3] = 0x0390,
+ [GR3_FLM4] = 0x0394,
+ [GR3_FLM5] = 0x0398,
+ [GR3_FLM6] = 0x039C,
+ [GR3_AB1] = 0x03A0,
+ [GR3_AB2] = 0x03A4,
+ [GR3_AB3] = 0x03A8,
+ [GR3_AB4] = 0x03AC,
+ [GR3_AB5] = 0x03B0,
+ [GR3_AB6] = 0x03B4,
+ [GR3_AB7] = 0x03B8,
+ [GR3_AB8] = 0x03BC,
+ [GR3_AB9] = 0x03C0,
+ [GR3_AB10] = 0x03C4,
+ [GR3_AB11] = 0x03C8,
+ [GR3_BASE] = 0x03CC,
+ [GR3_CLUT_INT] = 0x03D0,
+ [GR3_MON] = 0x03D4,
+ [TCON_UPDATE] = 0x0580,
+ [TCON_TIM] = 0x0584,
+ [TCON_TIM_STVA1] = 0x0588,
+ [TCON_TIM_STVA2] = 0x058C,
+ [TCON_TIM_STVB1] = 0x0590,
+ [TCON_TIM_STVB2] = 0x0594,
+ [TCON_TIM_STH1] = 0x0598,
+ [TCON_TIM_STH2] = 0x059C,
+ [TCON_TIM_STB1] = 0x05A0,
+ [TCON_TIM_STB2] = 0x05A4,
+ [TCON_TIM_CPV1] = 0x05A8,
+ [TCON_TIM_CPV2] = 0x05AC,
+ [TCON_TIM_POLA1] = 0x05B0,
+ [TCON_TIM_POLA2] = 0x05B4,
+ [TCON_TIM_POLB1] = 0x05B8,
+ [TCON_TIM_POLB2] = 0x05BC,
+ [TCON_TIM_DE] = 0x05C0,
+ [OUT_UPDATE] = 0x0600,
+ [OUT_SET] = 0x0604,
+ [OUT_BRIGHT1] = 0x0608,
+ [OUT_BRIGHT2] = 0x060C,
+ [OUT_CONTRAST] = 0x0610,
+ [OUT_PDTHA] = 0x0614,
+ [OUT_CLK_PHASE] = 0x0624,
+ [SYSCNT_INT1] = 0x0680,
+ [SYSCNT_INT2] = 0x0684,
+ [SYSCNT_INT3] = 0x0688,
+ [SYSCNT_INT4] = 0x068C,
+ [SYSCNT_PANEL_CLK] = 0x0690, /* 16-bit */
+ [SYSCNT_CLUT] = 0x0692, /* 16-bit */
+};
+
+/* SYSCNT */
+#define ICKEN (1 << 8)
+
+/* SCL Syncs */
+#define FREE_RUN_VSYNC 0x0001
+
+/* OUTPUT */
+#define OUT_FMT_RGB666 (1 << 12)
+
+/* TCON Timings */
+#define STVB_SEL_BITS 0x0007
+#define STVB_HS_SEL 2
+
+#define STH2_SEL_BITS 0x0007
+#define STH2_DE_SEL 7
+
+/* OUTCLK */
+#define LCD_DATA_EDGE 0x0100
+#define STVB_EDGE 0x0020
+#define STH_EDGE 0x0010
+
+/* SCL_UPDATE */
+#define SCL0_UPDATE_BIT 0x0100
+#define SCL0_VEN_BIT 0x0010
+
+/* TCON_UPDATE */
+#define TCON_VEN_BIT 0x0001
+
+/* OUT_UPDATE */
+#define OUTCNT_VEN_BIT 0x0001
+
+/* GR_UPDATE */
+#define P_VEN_UPDATE 0x0010
+#define IBUS_VEN_UPDATE 0x0001
+
+/* GR_AB1 */
+#define DISPSEL_BCKGND 0x0000
+#define DISPSEL_LOWER 0x0001
+#define DISPSEL_CUR 0x0002
+
+/* GR_FLM_RD */
+#define FB_R_ENB 0x01
+
+static void vdc4_write(struct ren_vdc4_priv *priv,
+ unsigned long reg_offs, unsigned long data)
+{
+ if ((SYSCNT_PANEL_CLK = reg_offs) || (SYSCNT_CLUT = reg_offs))
+ iowrite16(data, priv->base + vdc4_offsets[reg_offs]);
+ else
+ iowrite32(data, priv->base + vdc4_offsets[reg_offs]);
+}
+
+static unsigned long vdc4_read(struct ren_vdc4_priv *priv,
+ unsigned long reg_offs)
+{
+ if ((SYSCNT_PANEL_CLK = reg_offs) || (SYSCNT_CLUT = reg_offs))
+ return ioread16(priv->base + vdc4_offsets[reg_offs]);
+ else
+ return ioread32(priv->base + vdc4_offsets[reg_offs]);
+}
+
+static irqreturn_t ren_vdc4_irq(int irq, void *data)
+{
+ /* Not currently implemented/used */
+ return IRQ_HANDLED;
+}
+
+static void lcd_clear_display(struct ren_vdc4_priv *priv)
+{
+ unsigned char *pdest;
+ unsigned long size;
+
+ pdest = (unsigned char *)priv->dma_handle;
+ size = priv->cfg->lcd_cfg.xres * priv->cfg->lcd_cfg.yres * 2;
+
+ memset(pdest, 0, size);
+}
+
+static void restart_tft_display(struct ren_vdc4_priv *priv,
+ int clock_source)
+{
+ struct fb_videomode *lcd;
+ unsigned long h;
+ unsigned long v;
+ unsigned long tmp;
+
+ /* FB setup */
+ lcd = &priv->cfg->lcd_cfg;
+ lcd_clear_display(priv);
+
+ /* VDC clock Setup */
+ tmp = priv->cfg->clock_divider;
+ tmp |= clock_source << 12;
+ tmp |= ICKEN;
+ vdc4_write(priv, SYSCNT_PANEL_CLK, tmp);
+
+ /* Clear and Disable all interrupts */
+ vdc4_write(priv, SYSCNT_INT1, 0);
+ vdc4_write(priv, SYSCNT_INT2, 0);
+ vdc4_write(priv, SYSCNT_INT3, 0);
+ vdc4_write(priv, SYSCNT_INT4, 0);
+
+ /* Setup free-running syncs */
+ vdc4_write(priv, SCL0_FRC3, FREE_RUN_VSYNC);
+
+ /* Disable scale up/down */
+ vdc4_write(priv, SCL0_DS1, 0);
+ vdc4_write(priv, SCL0_US1, 0);
+
+ /* Timing registers */
+ h = lcd->hsync_len + lcd->left_margin + lcd->xres + lcd->right_margin;
+ v = lcd->vsync_len + lcd->upper_margin + lcd->yres + lcd->lower_margin;
+ tmp = (v - 1) << 16;
+ tmp |= h - 1;
+ vdc4_write(priv, SCL0_FRC4, tmp);
+
+ vdc4_write(priv, TCON_TIM, (((h - 1) / 2) << 16));
+
+ tmp = (lcd->vsync_len + lcd->upper_margin) << 16;
+ tmp |= lcd->yres;
+ vdc4_write(priv, SCL0_FRC6, tmp);
+ vdc4_write(priv, TCON_TIM_STVB1, tmp);
+ vdc4_write(priv, GR3_AB2, tmp);
+
+ tmp = lcd->left_margin << 16;
+ tmp |= lcd->xres;
+ vdc4_write(priv, SCL0_FRC7, tmp);
+ vdc4_write(priv, TCON_TIM_STB1, tmp);
+ vdc4_write(priv, GR3_AB3, tmp);
+
+ vdc4_write(priv, SCL0_FRC1, 0);
+ vdc4_write(priv, SCL0_FRC2, 0);
+ vdc4_write(priv, SCL0_FRC5, 0);
+
+ /* Set output format */
+ vdc4_write(priv, OUT_SET, OUT_FMT_RGB666);
+
+ /* STH TCON Timing */
+ tmp = priv->cfg->hs_pulse_width;
+ tmp |= priv->cfg->hs_start_pos << 16;
+ vdc4_write(priv, TCON_TIM_STH1, tmp);
+
+ /* Setup STVB as HSYNC */
+ tmp = vdc4_read(priv, TCON_TIM_STVB2);
+ tmp &= ~STVB_SEL_BITS;
+ tmp |= STVB_HS_SEL;
+ vdc4_write(priv, TCON_TIM_STVB2, tmp);
+
+ tmp = vdc4_read(priv, OUT_CLK_PHASE);
+ tmp &= ~STVB_EDGE;
+ vdc4_write(priv, OUT_CLK_PHASE, tmp);
+
+ /* Setup STH as DE */
+ tmp = vdc4_read(priv, TCON_TIM_STH2);
+ tmp &= ~STH2_SEL_BITS;
+ tmp |= STH2_DE_SEL;
+ vdc4_write(priv, TCON_TIM_STH2, tmp);
+
+ tmp = vdc4_read(priv, OUT_CLK_PHASE);
+ tmp &= ~STH_EDGE;
+ vdc4_write(priv, OUT_CLK_PHASE, tmp);
+
+ /* Output clock rising edge */
+ tmp = vdc4_read(priv, OUT_CLK_PHASE);
+ tmp &= ~LCD_DATA_EDGE;
+ vdc4_write(priv, OUT_CLK_PHASE, tmp);
+
+ /* Setup graphics buffers and update all registers */
+ vdc4_write(priv, GR1_AB1, DISPSEL_BCKGND);
+ vdc4_write(priv, GR2_AB1, DISPSEL_LOWER);
+ vdc4_write(priv, GR3_AB1, DISPSEL_CUR);
+
+ /* Setup framebuffer base/output */
+ vdc4_write(priv, GR3_FLM_RD, FB_R_ENB);
+
+ vdc4_write(priv, GR3_FLM2, (unsigned long)priv->info->screen_base);
+
+ vdc4_write(priv, GR3_FLM3, (lcd->xres * 2) << 16);
+
+ tmp = vdc4_read(priv, GR3_FLM5);
+ tmp |= lcd->yres << 16;
+ vdc4_write(priv, GR3_FLM5, tmp);
+
+ tmp = lcd->xres << 16;
+ vdc4_write(priv, GR3_FLM6, tmp);
+
+ /* Apply all register settings */
+ vdc4_write(priv, SCL0_UPDATE, SCL0_VEN_BIT | SCL0_UPDATE_BIT);
+ vdc4_write(priv, GR1_UPDATE, P_VEN_UPDATE);
+ vdc4_write(priv, GR2_UPDATE, P_VEN_UPDATE);
+ vdc4_write(priv, GR3_UPDATE, P_VEN_UPDATE | IBUS_VEN_UPDATE);
+ vdc4_write(priv, OUT_UPDATE, OUTCNT_VEN_BIT);
+ vdc4_write(priv, TCON_UPDATE, TCON_VEN_BIT);
+}
+
+static int ren_vdc4_setup_clocks(struct platform_device *pdev,
+ int clock_source,
+ struct ren_vdc4_priv *priv)
+{
+ priv->clk = devm_clk_get(&pdev->dev, "vdc4");
+ if (IS_ERR(priv->clk)) {
+ dev_err(&pdev->dev, "cannot get clock \"vdc4\"\n");
+ return PTR_ERR(priv->clk);
+ }
+
+ if (clock_source = VDC4_PERI_CLK) {
+ priv->dot_clk = devm_clk_get(&pdev->dev, "peripheral_clk");
+ if (IS_ERR(priv->dot_clk)) {
+ dev_err(&pdev->dev, "cannot get peripheral clock\n");
+ return PTR_ERR(priv->dot_clk);
+ }
+ }
+
+ return 0;
+}
+
+static int ren_vdc4_setcolreg(u_int regno,
+ u_int red, u_int green, u_int blue,
+ u_int transp, struct fb_info *info)
+{
+ u32 *palette = info->pseudo_palette;
+
+ if (regno >= PALETTE_NR)
+ return -EINVAL;
+
+ /* only FB_VISUAL_TRUECOLOR supported */
+
+ red >>= 16 - info->var.red.length;
+ green >>= 16 - info->var.green.length;
+ blue >>= 16 - info->var.blue.length;
+ transp >>= 16 - info->var.transp.length;
+
+ palette[regno] = (red << info->var.red.offset) |
+ (green << info->var.green.offset) |
+ (blue << info->var.blue.offset) |
+ (transp << info->var.transp.offset);
+
+ return 0;
+}
+
+static struct fb_fix_screeninfo ren_vdc4_fix = {
+ .id = "Renesas VDC4FB",
+ .type = FB_TYPE_PACKED_PIXELS,
+ .visual = FB_VISUAL_TRUECOLOR,
+ .accel = FB_ACCEL_NONE,
+};
+
+static struct fb_ops ren_vdc4_ops = {
+ .owner = THIS_MODULE,
+ .fb_setcolreg = ren_vdc4_setcolreg,
+ .fb_read = fb_sys_read,
+ .fb_write = fb_sys_write,
+ .fb_fillrect = sys_fillrect,
+ .fb_copyarea = sys_copyarea,
+ .fb_imageblit = sys_imageblit,
+};
+
+static int ren_vdc4_set_bpp(struct fb_var_screeninfo *var, int bpp)
+{
+ switch (bpp) {
+ case 16: /* RGB 565 */
+ var->red.offset = 11;
+ var->red.length = 5;
+ var->green.offset = 5;
+ var->green.length = 6;
+ var->blue.offset = 0;
+ var->blue.length = 5;
+ var->transp.offset = 0;
+ var->transp.length = 0;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ var->bits_per_pixel = bpp;
+ var->red.msb_right = 0;
+ var->green.msb_right = 0;
+ var->blue.msb_right = 0;
+ var->transp.msb_right = 0;
+ return 0;
+}
+
+/* PM Functions */
+static int ren_vdc4_start(struct ren_vdc4_priv *priv,
+ int clock_source)
+{
+ int ret;
+
+ ret = clk_enable(priv->clk);
+ if (ret < 0)
+ return ret;
+
+ if (priv->dot_clk) {
+ ret = clk_enable(priv->dot_clk);
+ if (ret < 0)
+ return ret;
+ }
+
+ restart_tft_display(priv, clock_source);
+
+ return ret;
+}
+
+static void ren_vdc4_stop(struct ren_vdc4_priv *priv)
+{
+ if (priv->dot_clk)
+ clk_disable(priv->dot_clk);
+ clk_disable(priv->clk);
+}
+
+static int ren_vdc4_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+
+ ren_vdc4_stop(platform_get_drvdata(pdev));
+ return 0;
+}
+
+static int ren_vdc4_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct ren_vdc4_info *pdata = pdev->dev.platform_data;
+
+ return ren_vdc4_start(platform_get_drvdata(pdev), pdata->clock_source);
+}
+
+static const struct dev_pm_ops ren_vdc4_dev_pm_ops = {
+ .suspend = ren_vdc4_suspend,
+ .resume = ren_vdc4_resume,
+};
+
+static int ren_vdc4_remove(struct platform_device *pdev);
+
+static int __devinit ren_vdc4_probe(struct platform_device *pdev)
+{
+ struct fb_info *info;
+ struct ren_vdc4_priv *priv;
+ struct ren_vdc4_info *pdata = pdev->dev.platform_data;
+ struct resource *res;
+ void *buf;
+ int irq, error;
+
+ if (!pdata) {
+ dev_err(&pdev->dev, "no platform data defined\n");
+ return -EINVAL;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ irq = platform_get_irq(pdev, 0);
+ if (!res || irq < 0) {
+ dev_err(&pdev->dev, "cannot get platform resources\n");
+ return -ENOENT;
+ }
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ dev_err(&pdev->dev, "cannot allocate device data\n");
+ return -ENOMEM;
+ }
+
+ platform_set_drvdata(pdev, priv);
+
+ error = devm_request_irq(&pdev->dev, irq, ren_vdc4_irq, 0,
+ dev_name(&pdev->dev), priv);
+ if (error) {
+ dev_err(&pdev->dev, "unable to request irq\n");
+ goto err1;
+ }
+
+ priv->irq = irq;
+ pdata = pdev->dev.platform_data;
+
+ priv->cfg = pdata;
+
+ error = ren_vdc4_setup_clocks(pdev, pdata->clock_source, priv);
+ if (error) {
+ dev_err(&pdev->dev, "unable to setup clocks\n");
+ goto err1;
+ }
+
+ priv->base = devm_ioremap_nocache(&pdev->dev, res->start,
+ resource_size(res));
+ if (!priv->base) {
+ dev_err(&pdev->dev, "unable to ioremap\n");
+ goto err1;
+ }
+
+ priv->info = framebuffer_alloc(0, &pdev->dev);
+ if (!priv->info) {
+ dev_err(&pdev->dev, "unable to allocate fb_info\n");
+ goto err1;
+ }
+
+ info = priv->info;
+ info->fbops = &ren_vdc4_ops;
+ info->var.xres = info->var.xres_virtual = pdata->lcd_cfg.xres;
+ info->var.yres = info->var.yres_virtual = pdata->lcd_cfg.yres;
+ info->var.width = pdata->panel_width;
+ info->var.height = pdata->panel_height;
+ info->var.activate = FB_ACTIVATE_NOW;
+ info->pseudo_palette = priv->pseudo_palette;
+ error = ren_vdc4_set_bpp(&info->var, pdata->bpp);
+ if (error)
+ goto err1;
+
+ info->fix = ren_vdc4_fix;
+ info->fix.line_length = pdata->lcd_cfg.xres * (pdata->bpp / 8);
+ info->fix.smem_len = info->fix.line_length * pdata->lcd_cfg.yres;
+
+ buf = dma_alloc_coherent(&pdev->dev, info->fix.smem_len,
+ &priv->dma_handle, GFP_KERNEL);
+ if (!buf) {
+ dev_err(&pdev->dev, "unable to allocate buffer\n");
+ goto err1;
+ }
+
+ info->flags = FBINFO_FLAG_DEFAULT;
+
+ error = fb_alloc_cmap(&info->cmap, PALETTE_NR, 0);
+ if (error < 0) {
+ dev_err(&pdev->dev, "unable to allocate cmap\n");
+ goto err1;
+ }
+
+ memset(buf, 0, info->fix.smem_len);
+ info->fix.smem_start = priv->dma_handle;
+ info->screen_base = buf;
+ info->device = &pdev->dev;
+ info->par = priv;
+
+ if (error)
+ goto err1;
+
+ error = ren_vdc4_start(priv, pdata->clock_source);
+ if (error) {
+ dev_err(&pdev->dev, "unable to start hardware\n");
+ goto err1;
+ }
+
+ info = priv->info;
+
+ error = register_framebuffer(info);
+ if (error < 0)
+ goto err1;
+
+ dev_info(info->dev,
+ "registered %s as %udx%ud %dbpp.\n",
+ pdev->name,
+ (int) pdata->lcd_cfg.xres,
+ (int) pdata->lcd_cfg.yres,
+ pdata->bpp);
+
+ return 0;
+
+err1:
+ ren_vdc4_remove(pdev);
+ return error;
+}
+
+static int ren_vdc4_remove(struct platform_device *pdev)
+{
+ struct ren_vdc4_priv *priv = platform_get_drvdata(pdev);
+ struct fb_info *info;
+
+ if (priv->info->dev)
+ unregister_framebuffer(priv->info);
+
+ ren_vdc4_stop(priv);
+
+ info = priv->info;
+
+ if (!info || !info->device) {
+ dev_err(&pdev->dev, "Failed to dealloc/release fb_info\n");
+ } else {
+ fb_dealloc_cmap(&info->cmap);
+ framebuffer_release(info);
+ }
+
+ return 0;
+}
+
+static struct platform_driver ren_vdc4_driver = {
+ .driver = {
+ .name = "ren_vdc4fb",
+ .owner = THIS_MODULE,
+ .pm = &ren_vdc4_dev_pm_ops,
+ },
+ .probe = ren_vdc4_probe,
+ .remove = ren_vdc4_remove,
+};
+
+static int __init ren_vdc4_init(void)
+{
+ return platform_driver_register(&ren_vdc4_driver);
+}
+
+static void __exit ren_vdc4_exit(void)
+{
+ platform_driver_unregister(&ren_vdc4_driver);
+}
+
+module_init(ren_vdc4_init);
+module_exit(ren_vdc4_exit);
+
+MODULE_DESCRIPTION("Renesas VDC4 Framebuffer driver");
+MODULE_AUTHOR("Phil Edworthy <phil.edworthy@renesas.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/video/ren_vdc4fb.h b/include/video/ren_vdc4fb.h
new file mode 100644
index 0000000..e91a515
--- /dev/null
+++ b/include/video/ren_vdc4fb.h
@@ -0,0 +1,19 @@
+#ifndef __REN_VDC4_H__
+#define __REN_VDC4_H__
+
+#include <linux/fb.h>
+
+enum { VDC4_EXTCLK = 1, VDC4_PERI_CLK };
+
+struct ren_vdc4_info {
+ int bpp;
+ int clock_source;
+ int clock_divider;
+ int hs_pulse_width;
+ int hs_start_pos;
+ struct fb_videomode lcd_cfg;
+ unsigned long panel_width;
+ unsigned long panel_height;
+};
+
+#endif
--
1.7.5.4
^ permalink raw reply related
* [PATCHv3 4/4] video: mmp: add tpo hvga panel supported
From: Zhou Zhu @ 2012-09-10 8:27 UTC (permalink / raw)
To: linux-fbdev
From: Lisa Du <cldu@marvell.com>
Add tpo hvga panel support in marvell display framework.
This panel driver implements modes query and power on/off.
This panel driver gets panel config/ plat power on/off/ connected
path name from machine-info and registered as a spi device.
This panel driver uses mmp_disp supplied register_panel function to
register panel to path as machine-info defined.
Signed-off-by: Lisa Du <cldu@marvell.com>
Signed-off-by: Zhou Zhu <zzhu3@marvell.com>
---
drivers/video/mmp/Kconfig | 1 +
drivers/video/mmp/Makefile | 2 +-
drivers/video/mmp/panel/Kconfig | 6 +
drivers/video/mmp/panel/Makefile | 1 +
drivers/video/mmp/panel/tpo_tj032md01bw.c | 188 +++++++++++++++++++++++++++++
5 files changed, 197 insertions(+), 1 deletions(-)
create mode 100644 drivers/video/mmp/panel/Kconfig
create mode 100644 drivers/video/mmp/panel/Makefile
create mode 100644 drivers/video/mmp/panel/tpo_tj032md01bw.c
diff --git a/drivers/video/mmp/Kconfig b/drivers/video/mmp/Kconfig
index ed51d15..e9ea39e 100644
--- a/drivers/video/mmp/Kconfig
+++ b/drivers/video/mmp/Kconfig
@@ -6,5 +6,6 @@ menuconfig MMP_DISP
if MMP_DISP
source "drivers/video/mmp/hw/Kconfig"
+source "drivers/video/mmp/panel/Kconfig"
source "drivers/video/mmp/fb/Kconfig"
endif
diff --git a/drivers/video/mmp/Makefile b/drivers/video/mmp/Makefile
index 6999a09..a014cb3 100644
--- a/drivers/video/mmp/Makefile
+++ b/drivers/video/mmp/Makefile
@@ -1 +1 @@
-obj-y += core.o hw/ fb/
+obj-y += core.o hw/ panel/ fb/
diff --git a/drivers/video/mmp/panel/Kconfig b/drivers/video/mmp/panel/Kconfig
new file mode 100644
index 0000000..4b2c4f4
--- /dev/null
+++ b/drivers/video/mmp/panel/Kconfig
@@ -0,0 +1,6 @@
+config MMP_PANEL_TPOHVGA
+ bool "tpohvga panel TJ032MD01BW support"
+ depends on SPI_MASTER
+ default n
+ help
+ tpohvga panel support
diff --git a/drivers/video/mmp/panel/Makefile b/drivers/video/mmp/panel/Makefile
new file mode 100644
index 0000000..2f91611
--- /dev/null
+++ b/drivers/video/mmp/panel/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_MMP_PANEL_TPOHVGA) += tpo_tj032md01bw.o
diff --git a/drivers/video/mmp/panel/tpo_tj032md01bw.c b/drivers/video/mmp/panel/tpo_tj032md01bw.c
new file mode 100644
index 0000000..f54b8a7
--- /dev/null
+++ b/drivers/video/mmp/panel/tpo_tj032md01bw.c
@@ -0,0 +1,188 @@
+/*
+ * linux/drivers/video/mmp/panel/tpo_tj032md01bw.c
+ * active panel using spi interface to do init
+ *
+ * Copyright (C) 2012 Marvell Technology Group Ltd.
+ * Authors: Guoqing Li <ligq@marvell.com>
+ * Lisa Du <cldu@marvell.com>
+ * Zhou Zhu <zzhu3@marvell.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.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/spi/spi.h>
+#include <video/mmp_disp.h>
+
+static u16 init[] = {
+ 0x0801,
+ 0x0800,
+ 0x0200,
+ 0x0304,
+ 0x040e,
+ 0x0903,
+ 0x0b18,
+ 0x0c53,
+ 0x0d01,
+ 0x0ee0,
+ 0x0f01,
+ 0x1058,
+ 0x201e,
+ 0x210a,
+ 0x220a,
+ 0x231e,
+ 0x2400,
+ 0x2532,
+ 0x2600,
+ 0x27ac,
+ 0x2904,
+ 0x2aa2,
+ 0x2b45,
+ 0x2c45,
+ 0x2d15,
+ 0x2e5a,
+ 0x2fff,
+ 0x306b,
+ 0x310d,
+ 0x3248,
+ 0x3382,
+ 0x34bd,
+ 0x35e7,
+ 0x3618,
+ 0x3794,
+ 0x3801,
+ 0x395d,
+ 0x3aae,
+ 0x3bff,
+ 0x07c9,
+};
+
+static u16 poweroff[] = {
+ 0x07d9,
+};
+
+struct tpohvga_plat_data {
+ void (*plat_onoff)(int status);
+ struct spi_device *spi;
+};
+
+static void tpohvga_onoff(struct mmp_panel *panel, int status)
+{
+ struct tpohvga_plat_data *plat = panel->plat_data;
+ int ret;
+
+ if (status) {
+ plat->plat_onoff(1);
+
+ ret = spi_write(plat->spi, init, sizeof(init));
+ if (ret < 0)
+ dev_warn(panel->dev, "init cmd failed(%d)\n", ret);
+ } else {
+ ret = spi_write(plat->spi, poweroff, sizeof(poweroff));
+ if (ret < 0)
+ dev_warn(panel->dev, "poweroff cmd failed(%d)\n", ret);
+
+ plat->plat_onoff(0);
+ }
+}
+
+static struct mmp_mode mmp_modes_tpohvga[] = {
+ [0] = {
+ .pixclock_freq = 10394400,
+ .refresh = 60,
+ .xres = 320,
+ .yres = 480,
+ .hsync_len = 10,
+ .left_margin = 15,
+ .right_margin = 10,
+ .vsync_len = 2,
+ .upper_margin = 4,
+ .lower_margin = 2,
+ .invert_pixclock = 1,
+ .pix_fmt_out = PIXFMT_RGB565,
+ },
+};
+
+static int tpohvga_get_modelist(struct mmp_panel *panel,
+ struct mmp_mode **modelist)
+{
+ *modelist = mmp_modes_tpohvga;
+ return 1;
+}
+
+static struct mmp_panel panel_tpohvga = {
+ .name = "tpohvga",
+ .panel_type = PANELTYPE_Active,
+ .get_modelist = tpohvga_get_modelist,
+ .set_onoff = tpohvga_onoff,
+};
+
+static int __devinit tpohvga_probe(struct spi_device *spi)
+{
+ struct mmp_mach_panel_info *mi;
+ int ret;
+ struct tpohvga_plat_data *plat_data;
+
+ /* get configs from platform data */
+ mi = spi->dev.platform_data;
+ if (mi = NULL) {
+ dev_err(&spi->dev, "%s: no platform data defined\n", __func__);
+ return -EINVAL;
+ }
+
+ /* setup spi related info */
+ spi->bits_per_word = 16;
+ ret = spi_setup(spi);
+ if (ret < 0) {
+ dev_err(&spi->dev, "spi setup failed %d", ret);
+ return ret;
+ }
+
+ plat_data = kzalloc(sizeof(*plat_data), GFP_KERNEL);
+ if (plat_data = NULL)
+ return -ENOMEM;
+
+ plat_data->spi = spi;
+ plat_data->plat_onoff = mi->plat_set_onoff;
+ panel_tpohvga.plat_data = plat_data;
+ panel_tpohvga.plat_path_name = mi->plat_path_name;
+ panel_tpohvga.dev = &spi->dev;
+
+ if (!mmp_register_panel(&panel_tpohvga)) {
+ dev_err(&spi->dev, "%s: register failed\n", __func__);
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static struct spi_driver panel_tpohvga_driver = {
+ .driver = {
+ .name = "tpo-hvga",
+ .owner = THIS_MODULE,
+ },
+ .probe = tpohvga_probe,
+};
+module_spi_driver(panel_tpohvga_driver);
+
+MODULE_AUTHOR("Lisa Du<cldu@marvell.com>");
+MODULE_DESCRIPTION("Panel driver for tpohvga");
+MODULE_LICENSE("GPL");
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] fbdev: Add Renesas vdc4 framebuffer driver
From: phil.edworthy @ 2012-09-10 8:14 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344428095-12203-1-git-send-email-phil.edworthy@renesas.com>
Hi Jingoo Han,
> > I reviewed your patch.
> > Please refer to my comments.
> > Good luck.
Thanks for the review, I'll have a look at the items you have highlighted.
Phil
^ permalink raw reply
* Re: [RFC PATCH v2] OMAPDSS: Fix IRQ unregister race
From: Tomi Valkeinen @ 2012-09-10 7:49 UTC (permalink / raw)
To: Dimitar Dimitrov; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1347116753-17064-1-git-send-email-dinuxbg@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 8260 bytes --]
On Sat, 2012-09-08 at 18:05 +0300, Dimitar Dimitrov wrote:
> Very rare kernel crashes are reported on a custom OMAP4 board. Kernel
> panics due to corrupted completion structure while executing
> dispc_irq_wait_handler(). Excerpt from kernel log:
>
> Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP
> Unable to handle kernel paging request at virtual address 00400130
> ...
> PC is at 0xebf205bc
> LR is at __wake_up_common+0x54/0x94
> ...
> (__wake_up_common+0x0/0x94)
> (complete+0x0/0x60)
> (dispc_irq_wait_handler.36902+0x0/0x14)
> (omap_dispc_irq_handler+0x0/0x354)
> (handle_irq_event_percpu+0x0/0x188)
> (handle_irq_event+0x0/0x64)
> (handle_fasteoi_irq+0x0/0x10c)
> (generic_handle_irq+0x0/0x48)
> (asm_do_IRQ+0x0/0xc0)
>
> DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
> unregister_isr() and DISPC IRQ might be running in parallel on different
> CPUs. So there is a chance that a callback is executed even though it
> has been unregistered. As omap_dispc_wait_for_irq_timeout() declares a
> completion on stack, the dispc_irq_wait_handler() callback might try to
> access a completion structure that is invalid. This leads to crashes and
> hangs.
>
> Solution is to divide unregister calls into two sets:
> 1. Non-strict unregistering of callbacks. Callbacks could safely be
> executed after unregistering them. This is the case with unregister
> calls from the IRQ handler itself.
> 2. Strict (synchronized) unregistering. Callbacks are not allowed
> after unregistering. This is the case with completion waiting.
>
> The above solution should satisfy one of the original intentions of the
> driver: callbacks should be able to unregister themselves.
>
> Also fix DSI IRQ unregister which has similar logic to DISPC IRQ handling.
>
> Signed-off-by: Dimitar Dimitrov <dinuxbg@gmail.com>
> ---
>
> WARNING: This bug is quite old. The patch has been tested on v3.0. No testing
> has been done after rebasing to v3.6. Hence the RFC tag. Hopefully someone
> will beat me in testing with latest linux-omap/master.
>
> Changes since v1 per Tomi Valkeinen's comments:
> - Don't rename omap_dispc_unregister_isr, just introduce nosync variant.
> - Apply the same fix for DSI IRQ which suffers from the same race condition.
I made a quick test and works for me, although I haven't encountered the
problem itself.
Some mostly cosmetic comments below.
This seems to apply cleanly to v3.4+ kernels, but not earlier ones. Do
you want to make the needed modifications and mail this and the modified
patches for stable kernels also? I can do that also if you don't want
to.
> drivers/staging/omapdrm/omap_plane.c | 2 +-
> drivers/video/omap2/dss/apply.c | 2 +-
> drivers/video/omap2/dss/dispc.c | 45 +++++++++++++++++++++++++++++-----
> drivers/video/omap2/dss/dsi.c | 19 ++++++++++++++
> include/video/omapdss.h | 1 +
> 5 files changed, 61 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/omapdrm/omap_plane.c b/drivers/staging/omapdrm/omap_plane.c
> index 7997be7..8d8aa5b 100644
> --- a/drivers/staging/omapdrm/omap_plane.c
> +++ b/drivers/staging/omapdrm/omap_plane.c
> @@ -82,7 +82,7 @@ static void dispc_isr(void *arg, uint32_t mask)
> struct omap_plane *omap_plane = to_omap_plane(plane);
> struct omap_drm_private *priv = plane->dev->dev_private;
>
> - omap_dispc_unregister_isr(dispc_isr, plane,
> + omap_dispc_unregister_isr_nosync(dispc_isr, plane,
> id2irq[omap_plane->ovl->id]);
>
> queue_work(priv->wq, &omap_plane->work);
> diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
> index 0fefc68..9386834 100644
> --- a/drivers/video/omap2/dss/apply.c
> +++ b/drivers/video/omap2/dss/apply.c
> @@ -847,7 +847,7 @@ static void dss_unregister_vsync_isr(void)
> for (i = 0; i < num_mgrs; ++i)
> mask |= dispc_mgr_get_framedone_irq(i);
>
> - r = omap_dispc_unregister_isr(dss_apply_irq_handler, NULL, mask);
> + r = omap_dispc_unregister_isr_nosync(dss_apply_irq_handler, NULL, mask);
> WARN_ON(r);
>
> dss_data.irq_enabled = false;
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index ee9e296..a67d92c 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -2421,8 +2421,8 @@ static void dispc_mgr_enable_digit_out(bool enable)
> enable ? "start" : "stop");
> }
>
> - r = omap_dispc_unregister_isr(dispc_disable_isr, &frame_done_completion,
> - irq_mask);
> + r = omap_dispc_unregister_isr(dispc_disable_isr,
> + &frame_done_completion, irq_mask);
This change is not needed.
> if (r)
> DSSERR("failed to unregister %x isr\n", irq_mask);
>
> @@ -3320,7 +3320,8 @@ err:
> }
> EXPORT_SYMBOL(omap_dispc_register_isr);
>
> -int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
> +/* WARNING: callback might be executed even after this function returns! */
> +int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void *arg, u32 mask)
> {
> int i;
> unsigned long flags;
> @@ -3352,7 +3353,37 @@ int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
>
> return ret;
> }
> -EXPORT_SYMBOL(omap_dispc_unregister_isr);
> +EXPORT_SYMBOL(omap_dispc_unregister_isr_nosync);
> +
> +/*
> + * Ensure that callback <isr> will NOT be executed after this function
> + * returns. Must be called from sleepable context, though!
> + */
> +int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
> +{
> + int ret;
> +
> + ret = omap_dispc_unregister_isr_nosync(isr, arg, mask);
> +
> + /* Task context is not really needed. But if we're called from atomic
> + * context, it is probably from DISPC IRQ, where we will deadlock.
> + * So use might_sleep() to catch potential deadlocks.
> + */
Use the kernel multi-line comment style, i.e.:
/*
* foobar
*/
> + might_sleep();
> +
> +#if defined(CONFIG_SMP)
> + /* DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
> + * unregister_isr() and DISPC IRQ might be running in parallel on
> + * different CPUs. So there is a chance that a callback is executed
> + * even though it has been unregistered. Add a barrier, in order to
> + * ensure that after returning from this function, the new DISPC IRQ
> + * will use an updated callback array, and NOT its cached copy.
> + */
Comment style here also.
> + synchronize_irq(dispc.irq);
> +#endif
Why do you use #if defined(CONFIG_SMP)? synchronize_irq is defined
with !CONFIG_SMP also. In that case it becomes just barrier().
> +
> + return ret;
> +}
>
> #ifdef DEBUG
> static void print_irq_status(u32 status)
> @@ -3567,7 +3598,8 @@ int omap_dispc_wait_for_irq_timeout(u32 irqmask, unsigned long timeout)
>
> timeout = wait_for_completion_timeout(&completion, timeout);
>
> - omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion, irqmask);
> + omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion,
> + irqmask);
Change not needed.
>
> if (timeout == 0)
> return -ETIMEDOUT;
> @@ -3598,7 +3630,8 @@ int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
> timeout = wait_for_completion_interruptible_timeout(&completion,
> timeout);
>
> - omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion, irqmask);
> + omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion,
> + irqmask);
Change not needed.
>
> if (timeout == 0)
> return -ETIMEDOUT;
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index b07e886..24b4a3e 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -960,6 +960,13 @@ static int dsi_unregister_isr(struct platform_device *dsidev,
>
> spin_unlock_irqrestore(&dsi->irq_lock, flags);
>
> + might_sleep();
> +
> +#if defined(CONFIG_SMP)
> + /* See notes for dispc.c:omap_dispc_unregister_isr() . */
> + synchronize_irq(dsi->irq);
> +#endif
> +
Same SMP comment as with dispc.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] fbdev: Add Renesas vdc4 framebuffer driver
From: Jingoo Han @ 2012-09-10 4:37 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344428095-12203-1-git-send-email-phil.edworthy@renesas.com>
On Monday, September 10, 2012 1:31 PM Jingoo Han wrote
>
> On Wednesday, August 08, 2012 9:15 PM Phil Edworthy wrote
>
> Hi Phil Edworthy,
>
> I reviewed your patch.
> Please refer to my comments.
> Good luck.
>
> Best regards,
> Jingoo Han
>
> >
> > The vdc4 display hardware is found on the sh7269 device.
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
>
> Please insert one line between the commit message and Signed-off-by.
>
> > ---
> > drivers/video/Kconfig | 10 +
> > drivers/video/Makefile | 1 +
> > drivers/video/ren_vdc4fb.c | 653 ++++++++++++++++++++++++++++++++++++++++++++
> > include/video/ren_vdc4fb.h | 19 ++
> > 4 files changed, 683 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/video/ren_vdc4fb.c
> > create mode 100644 include/video/ren_vdc4fb.h
> >
.....
> > +
> > +static int ren_vdc4_setup_clocks(struct platform_device *pdev,
> > + int clock_source,
> > + struct ren_vdc4_priv *priv)
> > +{
> > + priv->clk = clk_get(&pdev->dev, "vdc4");
>
> How about using devm_clk_get() instead of kzalloc()?
> It makes the code simpler.
Sorry, it's a typo.
It is not kzalloc(), but clk_get().
>
> > + if (IS_ERR(priv->clk)) {
> > + dev_err(&pdev->dev, "cannot get clock \"vdc4\"\n");
> > + return PTR_ERR(priv->clk);
> > + }
> > +
> > + if (clock_source = VDC4_PERI_CLK) {
> > + priv->dot_clk = clk_get(&pdev->dev, "peripheral_clk");
>
> How about using devm_clk_get() instead of kzalloc()?
> It makes the code simpler.
Same as above.
>
>
^ permalink raw reply
* Re: [PATCH] fbdev: Add Renesas vdc4 framebuffer driver
From: Jingoo Han @ 2012-09-10 4:31 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344428095-12203-1-git-send-email-phil.edworthy@renesas.com>
On Wednesday, August 08, 2012 9:15 PM Phil Edworthy wrote
Hi Phil Edworthy,
I reviewed your patch.
Please refer to my comments.
Good luck.
Best regards,
Jingoo Han
>
> The vdc4 display hardware is found on the sh7269 device.
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Please insert one line between the commit message and Signed-off-by.
> ---
> drivers/video/Kconfig | 10 +
> drivers/video/Makefile | 1 +
> drivers/video/ren_vdc4fb.c | 653 ++++++++++++++++++++++++++++++++++++++++++++
> include/video/ren_vdc4fb.h | 19 ++
> 4 files changed, 683 insertions(+), 0 deletions(-)
> create mode 100644 drivers/video/ren_vdc4fb.c
> create mode 100644 include/video/ren_vdc4fb.h
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 0217f74..89c9250 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -1990,6 +1990,16 @@ config FB_W100
>
> If unsure, say N.
>
> +config FB_REN_VDC4FB
> + tristate "Renesas VDC4 framebuffer support"
> + depends on FB && CPU_SUBTYPE_SH7269
> + select FB_SYS_FILLRECT
> + select FB_SYS_COPYAREA
> + select FB_SYS_IMAGEBLIT
> + select FB_SYS_FOPS
> + ---help---
> + Frame buffer driver for the Renesas VDC4.
> +
> config FB_SH_MOBILE_LCDC
> tristate "SuperH Mobile LCDC framebuffer support"
> depends on FB && (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index ee8dafb..ba69fcb 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -140,6 +140,7 @@ obj-$(CONFIG_SH_MIPI_DSI) += sh_mipi_dsi.o
> obj-$(CONFIG_FB_SH_MOBILE_HDMI) += sh_mobile_hdmi.o
> obj-$(CONFIG_FB_SH_MOBILE_MERAM) += sh_mobile_meram.o
> obj-$(CONFIG_FB_SH_MOBILE_LCDC) += sh_mobile_lcdcfb.o
> +obj-$(CONFIG_FB_REN_VDC4FB) += ren_vdc4fb.o
> obj-$(CONFIG_FB_OMAP) += omap/
> obj-y += omap2/
> obj-$(CONFIG_XEN_FBDEV_FRONTEND) += xen-fbfront.o
> diff --git a/drivers/video/ren_vdc4fb.c b/drivers/video/ren_vdc4fb.c
> new file mode 100644
> index 0000000..1a31e85
> --- /dev/null
> +++ b/drivers/video/ren_vdc4fb.c
> @@ -0,0 +1,653 @@
> +/*
> + * Renesas VDC4 Framebuffer
> + *
> + * Based on sh_mobile_lcdcfb.c
> + * Copyright (c) 2012 Renesas Electronics Europe Ltd
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License. See the file "COPYING" in the main directory of this archive
> + * for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/mm.h>
> +#include <linux/clk.h>
> +#include <linux/sh_clk.h>
> +#include <linux/platform_device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/interrupt.h>
> +#include <linux/vmalloc.h>
> +#include <linux/module.h>
> +#include <video/ren_vdc4fb.h>
> +
> +#define PALETTE_NR 16
> +
> +struct ren_vdc4_priv {
> + void __iomem *base;
> + int irq;
> + struct clk *dot_clk;
> + struct clk *clk;
> + struct fb_info *info;
> + dma_addr_t dma_handle;
> + struct ren_vdc4_info *cfg;
> + u32 pseudo_palette[PALETTE_NR];
> +};
> +
> +/* Register offsets/reading and writing functions */
> +enum {
> + SCL0_UPDATE, SCL0_FRC1, SCL0_FRC2, SCL0_FRC3,
> + SCL0_FRC4, SCL0_FRC5, SCL0_FRC6, SCL0_FRC7,
> + SCL0_DS1, SCL0_US1,
> +
> + GR1_UPDATE, GR1_AB1,
> +
> + GR2_UPDATE, GR2_AB1,
> +
> + GR3_UPDATE, GR3_FLM_RD, GR3_FLM1, GR3_FLM2,
> + GR3_FLM3, GR3_FLM4, GR3_FLM5, GR3_FLM6, GR3_AB1,
> + GR3_AB2, GR3_AB3, GR3_AB4, GR3_AB5, GR3_AB6,
> + GR3_AB7, GR3_AB8, GR3_AB9, GR3_AB10, GR3_AB11,
> + GR3_BASE, GR3_CLUT_INT, GR3_MON,
> +
> + TCON_UPDATE, TCON_TIM, TCON_TIM_STVA1, TCON_TIM_STVA2,
> + TCON_TIM_STVB1, TCON_TIM_STVB2, TCON_TIM_STH1,
> + TCON_TIM_STH2, TCON_TIM_STB1, TCON_TIM_STB2,
> + TCON_TIM_CPV1, TCON_TIM_CPV2, TCON_TIM_POLA1,
> + TCON_TIM_POLA2, TCON_TIM_POLB1, TCON_TIM_POLB2,
> + TCON_TIM_DE,
> +
> + OUT_UPDATE, OUT_SET, OUT_BRIGHT1,
> + OUT_BRIGHT2, OUT_CONTRAST, OUT_PDTHA, OUT_CLK_PHASE,
> +
> + SYSCNT_INT1, SYSCNT_INT2, SYSCNT_INT3, SYSCNT_INT4,
> + SYSCNT_PANEL_CLK, SYSCNT_CLUT
> +};
> +
> +static unsigned long vdc4_offsets[] = {
> + [SCL0_UPDATE] = 0x0100,
> + [SCL0_FRC1] = 0x0104,
> + [SCL0_FRC2] = 0x0108,
> + [SCL0_FRC3] = 0x010C,
> + [SCL0_FRC4] = 0x0110,
> + [SCL0_FRC5] = 0x0114,
> + [SCL0_FRC6] = 0x0118,
> + [SCL0_FRC7] = 0x011C,
> + [SCL0_DS1] = 0x012C,
> + [SCL0_US1] = 0x0148,
> + [GR1_UPDATE] = 0x0200,
> + [GR1_AB1] = 0x0220,
> + [GR2_UPDATE] = 0x0300,
> + [GR2_AB1] = 0x0320,
> + [GR3_UPDATE] = 0x0380,
> + [GR3_FLM_RD] = 0x0384,
> + [GR3_FLM1] = 0x0388,
> + [GR3_FLM2] = 0x038C,
> + [GR3_FLM3] = 0x0390,
> + [GR3_FLM4] = 0x0394,
> + [GR3_FLM5] = 0x0398,
> + [GR3_FLM6] = 0x039C,
> + [GR3_AB1] = 0x03A0,
> + [GR3_AB2] = 0x03A4,
> + [GR3_AB3] = 0x03A8,
> + [GR3_AB4] = 0x03AC,
> + [GR3_AB5] = 0x03B0,
> + [GR3_AB6] = 0x03B4,
> + [GR3_AB7] = 0x03B8,
> + [GR3_AB8] = 0x03BC,
> + [GR3_AB9] = 0x03C0,
> + [GR3_AB10] = 0x03C4,
> + [GR3_AB11] = 0x03C8,
> + [GR3_BASE] = 0x03CC,
> + [GR3_CLUT_INT] = 0x03D0,
> + [GR3_MON] = 0x03D4,
> + [TCON_UPDATE] = 0x0580,
> + [TCON_TIM] = 0x0584,
> + [TCON_TIM_STVA1] = 0x0588,
> + [TCON_TIM_STVA2] = 0x058C,
> + [TCON_TIM_STVB1] = 0x0590,
> + [TCON_TIM_STVB2] = 0x0594,
> + [TCON_TIM_STH1] = 0x0598,
> + [TCON_TIM_STH2] = 0x059C,
> + [TCON_TIM_STB1] = 0x05A0,
> + [TCON_TIM_STB2] = 0x05A4,
> + [TCON_TIM_CPV1] = 0x05A8,
> + [TCON_TIM_CPV2] = 0x05AC,
> + [TCON_TIM_POLA1] = 0x05B0,
> + [TCON_TIM_POLA2] = 0x05B4,
> + [TCON_TIM_POLB1] = 0x05B8,
> + [TCON_TIM_POLB2] = 0x05BC,
> + [TCON_TIM_DE] = 0x05C0,
> + [OUT_UPDATE] = 0x0600,
> + [OUT_SET] = 0x0604,
> + [OUT_BRIGHT1] = 0x0608,
> + [OUT_BRIGHT2] = 0x060C,
> + [OUT_CONTRAST] = 0x0610,
> + [OUT_PDTHA] = 0x0614,
> + [OUT_CLK_PHASE] = 0x0624,
> + [SYSCNT_INT1] = 0x0680,
> + [SYSCNT_INT2] = 0x0684,
> + [SYSCNT_INT3] = 0x0688,
> + [SYSCNT_INT4] = 0x068C,
> + [SYSCNT_PANEL_CLK] = 0x0690, /* 16-bit */
> + [SYSCNT_CLUT] = 0x0692, /* 16-bit */
> +};
> +
> +/* SYSCNT */
> +#define ICKEN (1 << 8)
> +
> +/* SCL Syncs */
> +#define FREE_RUN_VSYNC 0x0001
> +
> +/* OUTPUT */
> +#define OUT_FMT_RGB666 (1 << 12)
> +
> +/* TCON Timings */
> +#define STVB_SEL_BITS 0x0007
> +#define STVB_HS_SEL 2
> +
> +#define STH2_SEL_BITS 0x0007
> +#define STH2_DE_SEL 7
> +
> +/* OUTCLK */
> +#define LCD_DATA_EDGE 0x0100
> +#define STVB_EDGE 0x0020
> +#define STH_EDGE 0x0010
> +
> +/* SCL_UPDATE */
> +#define SCL0_UPDATE_BIT 0x0100
> +#define SCL0_VEN_BIT 0x0010
> +
> +/* TCON_UPDATE */
> +#define TCON_VEN_BIT 0x0001
> +
> +/* OUT_UPDATE */
> +#define OUTCNT_VEN_BIT 0x0001
> +
> +/* GR_UPDATE */
> +#define P_VEN_UPDATE 0x0010
> +#define IBUS_VEN_UPDATE 0x0001
> +
> +/* GR_AB1 */
> +#define DISPSEL_BCKGND 0x0000
> +#define DISPSEL_LOWER 0x0001
> +#define DISPSEL_CUR 0x0002
> +
> +/* GR_FLM_RD */
> +#define FB_R_ENB 0x01
> +
> +
Please remove unnecessary line.
> +static void vdc4_write(struct ren_vdc4_priv *priv,
> + unsigned long reg_offs, unsigned long data)
> +{
> + if ((SYSCNT_PANEL_CLK = reg_offs) || (SYSCNT_CLUT = reg_offs))
> + iowrite16(data, priv->base + vdc4_offsets[reg_offs]);
> + else
> + iowrite32(data, priv->base + vdc4_offsets[reg_offs]);
> +}
> +
> +static unsigned long vdc4_read(struct ren_vdc4_priv *priv,
> + unsigned long reg_offs)
> +{
> + if ((SYSCNT_PANEL_CLK = reg_offs) || (SYSCNT_CLUT = reg_offs))
> + return ioread16(priv->base + vdc4_offsets[reg_offs]);
> + else
> + return ioread32(priv->base + vdc4_offsets[reg_offs]);
> +}
> +
> +static irqreturn_t ren_vdc4_irq(int irq, void *data)
> +{
> + /* Not currently implemented/used */
> + return IRQ_HANDLED;
> +}
> +
> +static void lcd_clear_display(struct ren_vdc4_priv *priv)
> +{
> + unsigned char *pdest;
> + unsigned long size;
> +
> + pdest = (unsigned char *)priv->dma_handle;
> + size = priv->cfg->lcd_cfg.xres * priv->cfg->lcd_cfg.yres * 2;
> +
> + memset(pdest, 0, size);
> +}
> +
> +static void restart_tft_display(struct ren_vdc4_priv *priv,
> + int clock_source)
> +{
> + struct fb_videomode *lcd;
> + unsigned long h;
> + unsigned long v;
> + unsigned long tmp;
> +
> + /* FB setup */
> + lcd = &priv->cfg->lcd_cfg;
> + lcd_clear_display(priv);
> +
> + /* VDC clock Setup */
> + tmp = priv->cfg->clock_divider;
> + tmp |= clock_source << 12;
> + tmp |= ICKEN;
> + vdc4_write(priv, SYSCNT_PANEL_CLK, tmp);
> +
> + /* Clear and Disable all interrupts */
> + vdc4_write(priv, SYSCNT_INT1, 0);
> + vdc4_write(priv, SYSCNT_INT2, 0);
> + vdc4_write(priv, SYSCNT_INT3, 0);
> + vdc4_write(priv, SYSCNT_INT4, 0);
> +
> + /* Setup free-running syncs */
> + vdc4_write(priv, SCL0_FRC3, FREE_RUN_VSYNC);
> +
> + /* Disable scale up/down */
> + vdc4_write(priv, SCL0_DS1, 0);
> + vdc4_write(priv, SCL0_US1, 0);
> +
> + /* Timing registers */
> + h = lcd->hsync_len + lcd->left_margin + lcd->xres + lcd->right_margin;
> + v = lcd->vsync_len + lcd->upper_margin + lcd->yres + lcd->lower_margin;
> + tmp = (v - 1) << 16;
> + tmp |= h - 1;
> + vdc4_write(priv, SCL0_FRC4, tmp);
> +
> + vdc4_write(priv, TCON_TIM, (((h - 1) / 2) << 16));
> +
> + tmp = (lcd->vsync_len + lcd->upper_margin) << 16;
> + tmp |= lcd->yres;
> + vdc4_write(priv, SCL0_FRC6, tmp);
> + vdc4_write(priv, TCON_TIM_STVB1, tmp);
> + vdc4_write(priv, GR3_AB2, tmp);
> +
> + tmp = lcd->left_margin << 16;
> + tmp |= lcd->xres;
> + vdc4_write(priv, SCL0_FRC7, tmp);
> + vdc4_write(priv, TCON_TIM_STB1, tmp);
> + vdc4_write(priv, GR3_AB3, tmp);
> +
> + vdc4_write(priv, SCL0_FRC1, 0);
> + vdc4_write(priv, SCL0_FRC2, 0);
> + vdc4_write(priv, SCL0_FRC5, 0);
> +
> + /* Set output format */
> + vdc4_write(priv, OUT_SET, OUT_FMT_RGB666);
> +
> + /* STH TCON Timing */
> + tmp = priv->cfg->hs_pulse_width;
> + tmp |= priv->cfg->hs_start_pos << 16;
> + vdc4_write(priv, TCON_TIM_STH1, tmp);
> +
> + /* Setup STVB as HSYNC */
> + tmp = vdc4_read(priv, TCON_TIM_STVB2);
> + tmp &= ~STVB_SEL_BITS;
> + tmp |= STVB_HS_SEL;
> + vdc4_write(priv, TCON_TIM_STVB2, tmp);
> +
> + tmp = vdc4_read(priv, OUT_CLK_PHASE);
> + tmp &= ~STVB_EDGE;
> + vdc4_write(priv, OUT_CLK_PHASE, tmp);
> +
> + /* Setup STH as DE */
> + tmp = vdc4_read(priv, TCON_TIM_STH2);
> + tmp &= ~STH2_SEL_BITS;
> + tmp |= STH2_DE_SEL;
> + vdc4_write(priv, TCON_TIM_STH2, tmp);
> +
> + tmp = vdc4_read(priv, OUT_CLK_PHASE);
> + tmp &= ~STH_EDGE;
> + vdc4_write(priv, OUT_CLK_PHASE, tmp);
> +
> + /* Output clock rising edge */
> + tmp = vdc4_read(priv, OUT_CLK_PHASE);
> + tmp &= ~LCD_DATA_EDGE;
> + vdc4_write(priv, OUT_CLK_PHASE, tmp);
> +
> + /* Setup graphics buffers and update all registers */
> + vdc4_write(priv, GR1_AB1, DISPSEL_BCKGND);
> + vdc4_write(priv, GR2_AB1, DISPSEL_LOWER);
> + vdc4_write(priv, GR3_AB1, DISPSEL_CUR);
> +
> + /* Setup framebuffer base/output */
> + vdc4_write(priv, GR3_FLM_RD, FB_R_ENB);
> +
> + vdc4_write(priv, GR3_FLM2, (unsigned long)priv->info->screen_base);
> +
> + vdc4_write(priv, GR3_FLM3, (lcd->xres * 2) << 16);
> +
> + tmp = vdc4_read(priv, GR3_FLM5);
> + tmp |= lcd->yres << 16;
> + vdc4_write(priv, GR3_FLM5, tmp);
> +
> + tmp = lcd->xres << 16;
> + vdc4_write(priv, GR3_FLM6, tmp);
> +
> + /* Apply all register settings */
> + vdc4_write(priv, SCL0_UPDATE, SCL0_VEN_BIT | SCL0_UPDATE_BIT);
> + vdc4_write(priv, GR1_UPDATE, P_VEN_UPDATE);
> + vdc4_write(priv, GR2_UPDATE, P_VEN_UPDATE);
> + vdc4_write(priv, GR3_UPDATE, P_VEN_UPDATE | IBUS_VEN_UPDATE);
> + vdc4_write(priv, OUT_UPDATE, OUTCNT_VEN_BIT);
> + vdc4_write(priv, TCON_UPDATE, TCON_VEN_BIT);
> +}
> +
> +static int ren_vdc4_setup_clocks(struct platform_device *pdev,
> + int clock_source,
> + struct ren_vdc4_priv *priv)
> +{
> + priv->clk = clk_get(&pdev->dev, "vdc4");
How about using devm_clk_get() instead of kzalloc()?
It makes the code simpler.
> + if (IS_ERR(priv->clk)) {
> + dev_err(&pdev->dev, "cannot get clock \"vdc4\"\n");
> + return PTR_ERR(priv->clk);
> + }
> +
> + if (clock_source = VDC4_PERI_CLK) {
> + priv->dot_clk = clk_get(&pdev->dev, "peripheral_clk");
How about using devm_clk_get() instead of kzalloc()?
It makes the code simpler.
> + if (IS_ERR(priv->dot_clk)) {
> + dev_err(&pdev->dev, "cannot get peripheral clock\n");
> + clk_put(priv->clk);
> + return PTR_ERR(priv->dot_clk);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int ren_vdc4_setcolreg(u_int regno,
> + u_int red, u_int green, u_int blue,
> + u_int transp, struct fb_info *info)
> +{
> + u32 *palette = info->pseudo_palette;
> +
> + if (regno >= PALETTE_NR)
> + return -EINVAL;
> +
> + /* only FB_VISUAL_TRUECOLOR supported */
> +
> + red >>= 16 - info->var.red.length;
> + green >>= 16 - info->var.green.length;
> + blue >>= 16 - info->var.blue.length;
> + transp >>= 16 - info->var.transp.length;
> +
> + palette[regno] = (red << info->var.red.offset) |
> + (green << info->var.green.offset) |
> + (blue << info->var.blue.offset) |
> + (transp << info->var.transp.offset);
> +
> + return 0;
> +}
> +
> +static struct fb_fix_screeninfo ren_vdc4_fix = {
> + .id = "Renesas VDC4FB",
> + .type = FB_TYPE_PACKED_PIXELS,
> + .visual = FB_VISUAL_TRUECOLOR,
> + .accel = FB_ACCEL_NONE,
> +};
> +
> +static struct fb_ops ren_vdc4_ops = {
> + .owner = THIS_MODULE,
^^^^^^^^
Please use tab instead of spaces.
> + .fb_setcolreg = ren_vdc4_setcolreg,
> + .fb_read = fb_sys_read,
> + .fb_write = fb_sys_write,
Same as above.
> + .fb_fillrect = sys_fillrect,
> + .fb_copyarea = sys_copyarea,
> + .fb_imageblit = sys_imageblit,
> +};
> +
> +static int ren_vdc4_set_bpp(struct fb_var_screeninfo *var, int bpp)
> +{
> + switch (bpp) {
> + case 16: /* RGB 565 */
> + var->red.offset = 11;
> + var->red.length = 5;
> + var->green.offset = 5;
> + var->green.length = 6;
> + var->blue.offset = 0;
> + var->blue.length = 5;
> + var->transp.offset = 0;
> + var->transp.length = 0;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + var->bits_per_pixel = bpp;
> + var->red.msb_right = 0;
> + var->green.msb_right = 0;
> + var->blue.msb_right = 0;
> + var->transp.msb_right = 0;
> + return 0;
> +}
> +
> +/* PM Functions */
> +static int ren_vdc4_start(struct ren_vdc4_priv *priv,
> + int clock_source)
> +{
> + int ret;
> +
> + ret = clk_enable(priv->clk);
> + if (ret < 0)
> + return ret;
> +
> + if (priv->dot_clk) {
> + ret = clk_enable(priv->dot_clk);
> + if (ret < 0)
> + return ret;
> + }
> +
> + restart_tft_display(priv, clock_source);
> +
> + return ret;
> +}
> +
> +static void ren_vdc4_stop(struct ren_vdc4_priv *priv)
> +{
> + if (priv->dot_clk)
> + clk_disable(priv->dot_clk);
> + clk_disable(priv->clk);
> +}
> +
> +static int ren_vdc4_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> +
> + ren_vdc4_stop(platform_get_drvdata(pdev));
> + return 0;
> +}
> +
> +static int ren_vdc4_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct ren_vdc4_info *pdata = pdev->dev.platform_data;
> +
> + return ren_vdc4_start(platform_get_drvdata(pdev), pdata->clock_source);
> +}
> +
> +static const struct dev_pm_ops ren_vdc4_dev_pm_ops = {
> + .suspend = ren_vdc4_suspend,
> + .resume = ren_vdc4_resume,
> +};
> +
> +static int ren_vdc4_remove(struct platform_device *pdev);
> +
> +static int __devinit ren_vdc4_probe(struct platform_device *pdev)
> +{
> + struct fb_info *info;
> + struct ren_vdc4_priv *priv;
> + struct ren_vdc4_info *pdata = pdev->dev.platform_data;
> + struct resource *res;
> + void *buf;
> + int irq, error;
> +
> + if (!pdata) {
> + dev_err(&pdev->dev, "no platform data defined\n");
> + return -EINVAL;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + irq = platform_get_irq(pdev, 0);
> + if (!res || irq < 0) {
> + dev_err(&pdev->dev, "cannot get platform resources\n");
> + return -ENOENT;
> + }
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
How about using devm_kzalloc() instead of kzalloc()?
It makes the code simpler.
> + if (!priv) {
> + dev_err(&pdev->dev, "cannot allocate device data\n");
> + return -ENOMEM;
> + }
> +
> + platform_set_drvdata(pdev, priv);
> +
> + error = request_irq(irq, ren_vdc4_irq, 0, dev_name(&pdev->dev), priv);
How about using devm_request_irq()instead of request_irq()?
It makes the code simpler.
> + if (error) {
> + dev_err(&pdev->dev, "unable to request irq\n");
> + goto err1;
> + }
> +
> + priv->irq = irq;
> + pdata = pdev->dev.platform_data;
> +
> + priv->cfg = pdata;
> +
> + error = ren_vdc4_setup_clocks(pdev, pdata->clock_source, priv);
> + if (error) {
> + dev_err(&pdev->dev, "unable to setup clocks\n");
> + goto err1;
> + }
> +
> + priv->base = ioremap_nocache(res->start, resource_size(res));
How about using devm_ioremap_nocache() instead of ioremap_nocache()?
It makes the code simpler.
> + if (!priv->base) {
> + dev_err(&pdev->dev, "unable to ioremap\n");
> + goto err1;
> + }
> +
> + priv->info = framebuffer_alloc(0, &pdev->dev);
> + if (!priv->info) {
> + dev_err(&pdev->dev, "unable to allocate fb_info\n");
> + goto err1;
> + }
> +
> + info = priv->info;
> + info->fbops = &ren_vdc4_ops;
> + info->var.xres = info->var.xres_virtual = pdata->lcd_cfg.xres;
> + info->var.yres = info->var.yres_virtual = pdata->lcd_cfg.yres;
> + info->var.width = pdata->panel_width;
> + info->var.height = pdata->panel_height;
> + info->var.activate = FB_ACTIVATE_NOW;
> + info->pseudo_palette = priv->pseudo_palette;
> + error = ren_vdc4_set_bpp(&info->var, pdata->bpp);
> + if (error)
> + goto err1;
> +
> + info->fix = ren_vdc4_fix;
> + info->fix.line_length = pdata->lcd_cfg.xres * (pdata->bpp / 8);
> + info->fix.smem_len = info->fix.line_length * pdata->lcd_cfg.yres;
> +
> + buf = dma_alloc_coherent(&pdev->dev, info->fix.smem_len,
> + &priv->dma_handle, GFP_KERNEL);
> + if (!buf) {
> + dev_err(&pdev->dev, "unable to allocate buffer\n");
> + goto err1;
> + }
> +
> + info->flags = FBINFO_FLAG_DEFAULT;
> +
> + error = fb_alloc_cmap(&info->cmap, PALETTE_NR, 0);
> + if (error < 0) {
> + dev_err(&pdev->dev, "unable to allocate cmap\n");
> + goto err1;
> + }
> +
> + memset(buf, 0, info->fix.smem_len);
> + info->fix.smem_start = priv->dma_handle;
> + info->screen_base = buf;
> + info->device = &pdev->dev;
> + info->par = priv;
> +
> + if (error)
> + goto err1;
> +
> + ren_vdc4_start(priv, pdata->clock_source);
Return value should be checked as follow:
+ error = ren_vdc4_start(priv, pdata->clock_source);
> + if (error) {
> + dev_err(&pdev->dev, "unable to start hardware\n");
> + goto err1;
> + }
> +
> + info = priv->info;
> +
> + error = register_framebuffer(info);
> + if (error < 0)
> + goto err1;
> +
> + dev_info(info->dev,
> + "registered %s as %udx%ud %dbpp.\n",
> + pdev->name,
> + (int) pdata->lcd_cfg.xres,
> + (int) pdata->lcd_cfg.yres,
> + pdata->bpp);
> +
> + return 0;
> +
> +err1:
> + ren_vdc4_remove(pdev);
> + return error;
> +}
> +
> +static int ren_vdc4_remove(struct platform_device *pdev)
> +{
> + struct ren_vdc4_priv *priv = platform_get_drvdata(pdev);
> + struct fb_info *info;
> +
> + if (priv->info->dev)
> + unregister_framebuffer(priv->info);
> +
> + ren_vdc4_stop(priv);
> +
> + info = priv->info;
> +
> + if (!info || !info->device) {
> + dev_err(&pdev->dev, "Failed to dealloc/release fb_info\n");
> + } else {
> + fb_dealloc_cmap(&info->cmap);
> + framebuffer_release(info);
> + }
> +
> + if (priv->dot_clk)
> + clk_put(priv->dot_clk);
> + clk_put(priv->clk);
If devm_clk_get() is used in probe(), this clk_put() is not needed.
> +
> + if (priv->base)
> + iounmap(priv->base);
If devm_ioremap_nocache()is used in probe(), this iounmap() is not
needed.
> +
> + if (priv->irq)
> + free_irq(priv->irq, priv);
If devm_request_irq()is used in probe(), this free_irq() is not needed.
> +
> + kfree(priv);
If devm_kzalloc() is used in probe(), this kfree() is not needed.
> + return 0;
> +}
> +
> +static struct platform_driver ren_vdc4_driver = {
> + .driver = {
> + .name = "ren_vdc4fb",
> + .owner = THIS_MODULE,
> + .pm = &ren_vdc4_dev_pm_ops,
> + },
> + .probe = ren_vdc4_probe,
> + .remove = ren_vdc4_remove,
> +};
> +
> +static int __init ren_vdc4_init(void)
> +{
> + return platform_driver_register(&ren_vdc4_driver);
> +}
> +
> +static void __exit ren_vdc4_exit(void)
> +{
> + platform_driver_unregister(&ren_vdc4_driver);
> +}
> +
> +module_init(ren_vdc4_init);
> +module_exit(ren_vdc4_exit);
> +
> +MODULE_DESCRIPTION("Renesas VDC4 Framebuffer driver");
> +MODULE_AUTHOR("Phil Edworthy <phil.edworthy@renesas.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/video/ren_vdc4fb.h b/include/video/ren_vdc4fb.h
> new file mode 100644
> index 0000000..e91a515
> --- /dev/null
> +++ b/include/video/ren_vdc4fb.h
> @@ -0,0 +1,19 @@
> +#ifndef __REN_VDC4_H__
> +#define __REN_VDC4_H__
> +
> +#include <linux/fb.h>
> +
> +enum { VDC4_EXTCLK = 1, VDC4_PERI_CLK };
> +
> +struct ren_vdc4_info {
> + int bpp;
> + int clock_source;
> + int clock_divider;
> + int hs_pulse_width;
> + int hs_start_pos;
> + struct fb_videomode lcd_cfg;
> + unsigned long panel_width;
> + unsigned long panel_height;
> +};
> +
> +#endif
> --
> 1.7.5.4
>
> --
^ permalink raw reply
* Re: video: exynos_dp: Add device tree based discovery support
From: Jingoo Han @ 2012-09-10 1:31 UTC (permalink / raw)
To: 'Ajay Kumar'
Cc: linux-samsung-soc, linux-fbdev, FlorianSchandinat,
'Jingoo Han'
In-Reply-To: <1346835251-1276-1-git-send-email-~@samsung.com>
On Wednesday, September 05, 2012 5:54 PM Ajay Kumar wrote
>
> From: Ajay Kumar <ajaykumar.rs@samsung.com>
>
> Add device tree match table for Exynos DP
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
Please insert one line between the commit message and Signed-off-by.
Also, add '[PATCH]' to subject as follow:
[PATCH] video: exynos_dp: Add device tree based discovery support
Last, pay attention of your mail address and name.
Um, now, you are using "~@samsung.com" as mail address and name.
Please configure your mail program in order to set "Ajay Kumar"
and "ajaykumar.rs@samsung.com".
If you want to get reviewed, please keep the basic rules
as I mentioned above.
Best regards,
Jingoo Han
> ---
> drivers/video/exynos/exynos_dp_core.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index c6c016a..3bccd6b 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -18,6 +18,7 @@
> #include <linux/io.h>
> #include <linux/interrupt.h>
> #include <linux/delay.h>
> +#include <linux/of.h>
>
> #include <video/exynos_dp.h>
>
> @@ -1019,6 +1020,14 @@ static const struct dev_pm_ops exynos_dp_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(exynos_dp_suspend, exynos_dp_resume)
> };
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id exynos_dp_match[] = {
> + { .compatible = "samsung,exynos5-dp" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_dp_match);
> +#endif
> +
> static struct platform_driver exynos_dp_driver = {
> .probe = exynos_dp_probe,
> .remove = __devexit_p(exynos_dp_remove),
> @@ -1026,6 +1035,7 @@ static struct platform_driver exynos_dp_driver = {
> .name = "exynos-dp",
> .owner = THIS_MODULE,
> .pm = &exynos_dp_pm_ops,
> + .of_match_table = of_match_ptr(exynos_dp_match),
> },
> };
>
> --
> 1.7.0.4
^ permalink raw reply
* [PATCH] fbdev: jz4740: Use devm_request_and_ioremap
From: Lars-Peter Clausen @ 2012-09-09 15:38 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: Damien Cassou, linux-fbdev, linux-kernel, Lars-Peter Clausen
Use devm_request_and_ioremap instead of request_mem_region + devm_ioremap.
This also fixes the following compile error introduced in commit b2ca7f4d
("drivers/video/jz4740_fb.c: use devm_ functions"):
drivers/video/jz4740_fb.c: In function 'jzfb_probe':
drivers/video/jz4740_fb.c:676:2: error: implicit declaration of function 'devm_ioremap'
drivers/video/jz4740_fb.c:676:13: warning: assignment makes pointer from integer without a cast
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/video/jz4740_fb.c | 24 +++---------------------
1 files changed, 3 insertions(+), 21 deletions(-)
diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
index 7669770..b0df279 100644
--- a/drivers/video/jz4740_fb.c
+++ b/drivers/video/jz4740_fb.c
@@ -632,23 +632,10 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
return -ENXIO;
}
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem) {
- dev_err(&pdev->dev, "Failed to get register memory resource\n");
- return -ENXIO;
- }
-
- mem = request_mem_region(mem->start, resource_size(mem), pdev->name);
- if (!mem) {
- dev_err(&pdev->dev, "Failed to request register memory region\n");
- return -EBUSY;
- }
-
fb = framebuffer_alloc(sizeof(struct jzfb), &pdev->dev);
if (!fb) {
dev_err(&pdev->dev, "Failed to allocate framebuffer device\n");
- ret = -ENOMEM;
- goto err_release_mem_region;
+ return -ENOMEM;
}
fb->fbops = &jzfb_ops;
@@ -657,7 +644,6 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
jzfb = fb->par;
jzfb->pdev = pdev;
jzfb->pdata = pdata;
- jzfb->mem = mem;
jzfb->ldclk = devm_clk_get(&pdev->dev, "lcd");
if (IS_ERR(jzfb->ldclk)) {
@@ -673,9 +659,9 @@ static int __devinit jzfb_probe(struct platform_device *pdev)
goto err_framebuffer_release;
}
- jzfb->base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ jzfb->base = devm_request_and_ioremap(&pdev->dev, mem);
if (!jzfb->base) {
- dev_err(&pdev->dev, "Failed to ioremap register memory region\n");
ret = -EBUSY;
goto err_framebuffer_release;
}
@@ -736,8 +722,6 @@ err_free_devmem:
jzfb_free_devmem(jzfb);
err_framebuffer_release:
framebuffer_release(fb);
-err_release_mem_region:
- release_mem_region(mem->start, resource_size(mem));
return ret;
}
@@ -750,8 +734,6 @@ static int __devexit jzfb_remove(struct platform_device *pdev)
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));
- release_mem_region(jzfb->mem->start, resource_size(jzfb->mem));
-
fb_dealloc_cmap(&jzfb->fb->cmap);
jzfb_free_devmem(jzfb);
--
1.7.2.5
^ permalink raw reply related
* [RFC PATCH v2] OMAPDSS: Fix IRQ unregister race
From: Dimitar Dimitrov @ 2012-09-08 15:05 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Dimitar Dimitrov
In-Reply-To: <201209071742.11735.dinuxbg@gmail.com>
Very rare kernel crashes are reported on a custom OMAP4 board. Kernel
panics due to corrupted completion structure while executing
dispc_irq_wait_handler(). Excerpt from kernel log:
Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP
Unable to handle kernel paging request at virtual address 00400130
...
PC is at 0xebf205bc
LR is at __wake_up_common+0x54/0x94
...
(__wake_up_common+0x0/0x94)
(complete+0x0/0x60)
(dispc_irq_wait_handler.36902+0x0/0x14)
(omap_dispc_irq_handler+0x0/0x354)
(handle_irq_event_percpu+0x0/0x188)
(handle_irq_event+0x0/0x64)
(handle_fasteoi_irq+0x0/0x10c)
(generic_handle_irq+0x0/0x48)
(asm_do_IRQ+0x0/0xc0)
DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
unregister_isr() and DISPC IRQ might be running in parallel on different
CPUs. So there is a chance that a callback is executed even though it
has been unregistered. As omap_dispc_wait_for_irq_timeout() declares a
completion on stack, the dispc_irq_wait_handler() callback might try to
access a completion structure that is invalid. This leads to crashes and
hangs.
Solution is to divide unregister calls into two sets:
1. Non-strict unregistering of callbacks. Callbacks could safely be
executed after unregistering them. This is the case with unregister
calls from the IRQ handler itself.
2. Strict (synchronized) unregistering. Callbacks are not allowed
after unregistering. This is the case with completion waiting.
The above solution should satisfy one of the original intentions of the
driver: callbacks should be able to unregister themselves.
Also fix DSI IRQ unregister which has similar logic to DISPC IRQ handling.
Signed-off-by: Dimitar Dimitrov <dinuxbg@gmail.com>
---
WARNING: This bug is quite old. The patch has been tested on v3.0. No testing
has been done after rebasing to v3.6. Hence the RFC tag. Hopefully someone
will beat me in testing with latest linux-omap/master.
Changes since v1 per Tomi Valkeinen's comments:
- Don't rename omap_dispc_unregister_isr, just introduce nosync variant.
- Apply the same fix for DSI IRQ which suffers from the same race condition.
drivers/staging/omapdrm/omap_plane.c | 2 +-
drivers/video/omap2/dss/apply.c | 2 +-
drivers/video/omap2/dss/dispc.c | 45 +++++++++++++++++++++++++++++-----
drivers/video/omap2/dss/dsi.c | 19 ++++++++++++++
include/video/omapdss.h | 1 +
5 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/omapdrm/omap_plane.c b/drivers/staging/omapdrm/omap_plane.c
index 7997be7..8d8aa5b 100644
--- a/drivers/staging/omapdrm/omap_plane.c
+++ b/drivers/staging/omapdrm/omap_plane.c
@@ -82,7 +82,7 @@ static void dispc_isr(void *arg, uint32_t mask)
struct omap_plane *omap_plane = to_omap_plane(plane);
struct omap_drm_private *priv = plane->dev->dev_private;
- omap_dispc_unregister_isr(dispc_isr, plane,
+ omap_dispc_unregister_isr_nosync(dispc_isr, plane,
id2irq[omap_plane->ovl->id]);
queue_work(priv->wq, &omap_plane->work);
diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 0fefc68..9386834 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -847,7 +847,7 @@ static void dss_unregister_vsync_isr(void)
for (i = 0; i < num_mgrs; ++i)
mask |= dispc_mgr_get_framedone_irq(i);
- r = omap_dispc_unregister_isr(dss_apply_irq_handler, NULL, mask);
+ r = omap_dispc_unregister_isr_nosync(dss_apply_irq_handler, NULL, mask);
WARN_ON(r);
dss_data.irq_enabled = false;
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index ee9e296..a67d92c 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2421,8 +2421,8 @@ static void dispc_mgr_enable_digit_out(bool enable)
enable ? "start" : "stop");
}
- r = omap_dispc_unregister_isr(dispc_disable_isr, &frame_done_completion,
- irq_mask);
+ r = omap_dispc_unregister_isr(dispc_disable_isr,
+ &frame_done_completion, irq_mask);
if (r)
DSSERR("failed to unregister %x isr\n", irq_mask);
@@ -3320,7 +3320,8 @@ err:
}
EXPORT_SYMBOL(omap_dispc_register_isr);
-int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
+/* WARNING: callback might be executed even after this function returns! */
+int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void *arg, u32 mask)
{
int i;
unsigned long flags;
@@ -3352,7 +3353,37 @@ int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
return ret;
}
-EXPORT_SYMBOL(omap_dispc_unregister_isr);
+EXPORT_SYMBOL(omap_dispc_unregister_isr_nosync);
+
+/*
+ * Ensure that callback <isr> will NOT be executed after this function
+ * returns. Must be called from sleepable context, though!
+ */
+int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
+{
+ int ret;
+
+ ret = omap_dispc_unregister_isr_nosync(isr, arg, mask);
+
+ /* Task context is not really needed. But if we're called from atomic
+ * context, it is probably from DISPC IRQ, where we will deadlock.
+ * So use might_sleep() to catch potential deadlocks.
+ */
+ might_sleep();
+
+#if defined(CONFIG_SMP)
+ /* DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
+ * unregister_isr() and DISPC IRQ might be running in parallel on
+ * different CPUs. So there is a chance that a callback is executed
+ * even though it has been unregistered. Add a barrier, in order to
+ * ensure that after returning from this function, the new DISPC IRQ
+ * will use an updated callback array, and NOT its cached copy.
+ */
+ synchronize_irq(dispc.irq);
+#endif
+
+ return ret;
+}
#ifdef DEBUG
static void print_irq_status(u32 status)
@@ -3567,7 +3598,8 @@ int omap_dispc_wait_for_irq_timeout(u32 irqmask, unsigned long timeout)
timeout = wait_for_completion_timeout(&completion, timeout);
- omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion, irqmask);
+ omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion,
+ irqmask);
if (timeout = 0)
return -ETIMEDOUT;
@@ -3598,7 +3630,8 @@ int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
timeout = wait_for_completion_interruptible_timeout(&completion,
timeout);
- omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion, irqmask);
+ omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion,
+ irqmask);
if (timeout = 0)
return -ETIMEDOUT;
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index b07e886..24b4a3e 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -960,6 +960,13 @@ static int dsi_unregister_isr(struct platform_device *dsidev,
spin_unlock_irqrestore(&dsi->irq_lock, flags);
+ might_sleep();
+
+#if defined(CONFIG_SMP)
+ /* See notes for dispc.c:omap_dispc_unregister_isr() . */
+ synchronize_irq(dsi->irq);
+#endif
+
return r;
}
@@ -1002,6 +1009,12 @@ static int dsi_unregister_isr_vc(struct platform_device *dsidev, int channel,
spin_unlock_irqrestore(&dsi->irq_lock, flags);
+ might_sleep();
+
+#if defined(CONFIG_SMP)
+ /* See notes for dispc.c:omap_dispc_unregister_isr() . */
+ synchronize_irq(dsi->irq);
+#endif
return r;
}
@@ -1042,6 +1055,12 @@ static int dsi_unregister_isr_cio(struct platform_device *dsidev,
spin_unlock_irqrestore(&dsi->irq_lock, flags);
+ might_sleep();
+
+#if defined(CONFIG_SMP)
+ /* See notes for dispc.c:omap_dispc_unregister_isr() . */
+ synchronize_irq(dsi->irq);
+#endif
return r;
}
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index a6267a2..769f981 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -708,6 +708,7 @@ void omapdss_default_get_timings(struct omap_dss_device *dssdev,
typedef void (*omap_dispc_isr_t) (void *arg, u32 mask);
int omap_dispc_register_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
+int omap_dispc_unregister_isr_nosync(omap_dispc_isr_t isr, void *arg, u32 mask);
int omap_dispc_wait_for_irq_timeout(u32 irqmask, unsigned long timeout);
int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 0/8] OMAPDSS: Misc improvements
From: Tomi Valkeinen @ 2012-09-07 16:59 UTC (permalink / raw)
To: Tony Lindgren; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <20120907162258.GT1303@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 1315 bytes --]
On Fri, 2012-09-07 at 09:22 -0700, Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [120907 03:16]:
> > On Thu, 2012-09-06 at 13:13 -0700, Tony Lindgren wrote:
> > > * Tomi Valkeinen <tomi.valkeinen@ti.com> [120904 00:23]:
> > > > Hi Tony,
> > > >
> > > > Can you check the arch/arm patches below, and suggest how you'd like to
> > > > go forward with them?
> > >
> > > Acked them, then as soon as we have the initial immutable header
> > > move branch available, you should merge with that to avoid
> > > merge conflicts in upstream.
> >
> > Thanks, but you missed the first patch "[PATCH 1/8] OMAPDSS: HDMI: Move
> > GPIO handling to HDMI driver".
>
> Oops sorry, that's a nice one, acked that too. BTW, do you have any
> platform code callbacks remaining for DSS?
Yes, for quite many boards:
$ git grep platform_enable arch/arm/mach-omap2/|wc -l
17
Some of those are trivial, I just need to move the panel's reset gpio
handling to the panel driver. But some fiddle around with board specific
gpios that do not belong to the panel driver. Also some board files
contain backlight handling.
Those are for panel drivers, then there's also callbacks for the omapdss
driver itself. get_context_loss_count, set_min_bus_tput, and dsi pin
enable/disable.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v5 1/4] Runtime Interpreted Power Sequences
From: Stephen Warren @ 2012-09-07 16:36 UTC (permalink / raw)
To: Heiko Stübner
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stephen Warren, linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Brown,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Anton Vorontsov,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David Woodhouse,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <201209071108.42083.heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
On 09/07/2012 03:08 AM, Heiko Stübner wrote:
> Am Freitag, 7. September 2012, 10:04:24 schrieb Alex Courbot:
>>> For your power_seq_run function you write that it simply returns an error
>>> code on failure and looking through it I also just found the error return
>>> statement. This would leave a device half turned on.
>>>
>>> So I'm wondering, if it shouldn't turn off all the things it turned on
>>> until the step that produced the error. All your possible step types
>>> (execpt the delay) are booleans, so it should be possible to simply
>>> negate them when backtracking through the previous steps.
>>
>> Indeed, I think you raised an important point. Right now all step types are
>> invertible, but we cannot rely on that statement to be true forever. For
>> instance, one short-term improvement will be to allow finer regulator
>> control, like voltage setting. In this case, how can we go back to the
>> initial state without recording it?
>>
>> If e.g. the power on sequence fails at step N (of M steps for that
>> sequence), one could try playing the corresponding power off sequence
>> (either completely of from step M - N), but then again we cannot rely on
>> sequences to be perfectly symetrical. Maybe this is more something for the
>> calling driver to check for and control?
>
> Am Freitag, 7. September 2012, 10:15:03 schrieb Mark Brown:
>> On Fri, Sep 07, 2012 at 05:04:24PM +0900, Alex Courbot wrote:
>>> If e.g. the power on sequence fails at step N (of M steps for that
>>> sequence), one could try playing the corresponding power off sequence
>>> (either completely of from step M - N), but then again we cannot rely on
>>> sequences to be perfectly symetrical. Maybe this is more something for
>>> the calling driver to check for and control?
>>
>> That had been my thought too - depending on what the sequence is for it
>> may be that the corrective action is something very different to
>> reversing the sequence, for example a device reset may be required.
>
> If I understood the description correctly, the power sequence should be
> transparent to the driver, as it implements board specific actions and
> shouldn't bother the driver with it to much.
Well, the contents/implementation of the sequence should be transparent
to the driver. The fact that a sequence exists and needs to be executed
obviously can't be transparent to the driver, since the driver needs to
call an API to execute the sequence.
I'd assert that requiring the driver to get back to a sane state by
executing sequence (b) if sequence (a) fails is fairly reasonable, and
doesn't give the driver any more knowledge of what the sequences are
than what it already has.
But then I start to wonder: What if the "help something went wrong"
sequence gets an error...
> Therefore my thoughts went along
> the lines how gpio_request_array handles this, always producing a sane state
> at the end.
>
> Recording the previous state, could be done by making a copy of the current
> sequence, and just noting the previous values (including voltages etc) in the
> respective entries. And in the error case running this new sequence from the
> error point instead to power down again.
>
>
> As both Alex and Mark wrote, reversing the sequence might be the action of
> choice only for some devices, but others might need to run a completely
> different powerdown sequence and still others would need special handling.
>
> Would it be possible to encode this in the sequence definition, something like
> on-error = "reverse"
>
> on-error = "sequence"
> error-seq = <&other_sequence>
>
> on-error = "driver"
> with better names and types of course.
>
> This would keep the power sequence transparent to most drivers and only the
> real esoteric ones would need to do their special handling on their own.
Yes, something like that sounds reasonable on the surface. I'm not sure
about the on-error="driver" case though; if the driver knows nothing
about the content of the sequences, I'm not sure how the driver could
possibly do anything other than execute some sequence to recover.
^ permalink raw reply
* Re: [PATCH 0/8] OMAPDSS: Misc improvements
From: Tony Lindgren @ 2012-09-07 16:22 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <1347012960.2646.5.camel@deskari>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [120907 03:16]:
> On Thu, 2012-09-06 at 13:13 -0700, Tony Lindgren wrote:
> > * Tomi Valkeinen <tomi.valkeinen@ti.com> [120904 00:23]:
> > > Hi Tony,
> > >
> > > Can you check the arch/arm patches below, and suggest how you'd like to
> > > go forward with them?
> >
> > Acked them, then as soon as we have the initial immutable header
> > move branch available, you should merge with that to avoid
> > merge conflicts in upstream.
>
> Thanks, but you missed the first patch "[PATCH 1/8] OMAPDSS: HDMI: Move
> GPIO handling to HDMI driver".
Oops sorry, that's a nice one, acked that too. BTW, do you have any
platform code callbacks remaining for DSS?
Regards,
Tony
^ permalink raw reply
* Re: [PATCH 1/8] OMAPDSS: HDMI: Move GPIO handling to HDMI driver
From: Tony Lindgren @ 2012-09-07 16:21 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: archit, linux-omap, linux-fbdev
In-Reply-To: <1345729514-2441-2-git-send-email-tomi.valkeinen@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [120823 06:46]:
> We currently manage HDMI GPIOs in the board files via
> platform_enable/disable calls. This won't work with device tree, and in
> any case the correct place to manage the GPIOs is in the HDMI driver.
>
> This patch moves the handling of the GPIOs to the HDMI driver. The GPIO
> handling is moved to the common hdmi.c file, and this probably needs to
> be revisited when adding OMAP5 HDMI support to see if the GPIO handling
> needs to be moved to IP specific files.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
This too looks safe to merge via fb tree:
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply
* Re: [RFC PATCH] OMAPDSS: DISPC: Fix IRQ unregister race
From: Dimitar Dimitrov @ 2012-09-07 14:42 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1346938614.2737.68.camel@deskari>
Hi,
On Thursday 06 September 2012 16:36:54 Tomi Valkeinen wrote:
> Hi,
>
> On Sun, 2012-09-02 at 22:12 +0300, Dimitar Dimitrov wrote:
> > Very rare kernel crashes are reported on a custom OMAP4 board. Kernel
> > panics due to corrupted completion structure while executing
> >
> > dispc_irq_wait_handler(). Excerpt from kernel log:
> > Internal error: Oops - undefined instruction: 0 [#1] PREEMPT SMP
> > Unable to handle kernel paging request at virtual address 00400130
> > ...
> > PC is at 0xebf205bc
> > LR is at __wake_up_common+0x54/0x94
> > ...
> > (__wake_up_common+0x0/0x94)
> > (complete+0x0/0x60)
> > (dispc_irq_wait_handler.36902+0x0/0x14)
> > (omap_dispc_irq_handler+0x0/0x354)
> > (handle_irq_event_percpu+0x0/0x188)
> > (handle_irq_event+0x0/0x64)
> > (handle_fasteoi_irq+0x0/0x10c)
> > (generic_handle_irq+0x0/0x48)
> > (asm_do_IRQ+0x0/0xc0)
> >
> > DISPC IRQ executes callbacks with dispc.irq_lock released. Hence
> > unregister_isr() and DISPC IRQ might be running in parallel on different
> > CPUs. So there is a chance that a callback is executed even though it
> > has been unregistered. As omap_dispc_wait_for_irq_timeout() declares a
> > completion on stack, the dispc_irq_wait_handler() callback might try to
> > access a completion structure that is invalid. This leads to crashes and
> > hangs.
> >
> > Solution is to divide unregister calls into two sets:
> > 1. Non-strict unregistering of callbacks. Callbacks could safely be
> >
> > executed after unregistering them. This is the case with unregister
> > calls from the IRQ handler itself.
> >
> > 2. Strict (synchronized) unregistering. Callbacks are not allowed
> >
> > after unregistering. This is the case with completion waiting.
> >
> > The above solution should satisfy one of the original intentions of the
> > driver: callbacks should be able to unregister themselves.
>
> I think it'd be better to create a new function for the nosync version,
> and keep the old name for the sync version. The reason for this is to
> minimize the amount of changes, as I think this one needs to be applied
> to stable kernel trees also.
My intention was to force all callers to pick sides. In case of rebase issues
we get link errors instead of rare and subtle run-time races. Still, if you
think we should leave the old name untouched then I'll change my
patch.
>
> Also, I think we need similar one for dsi.c, as it has the same kind of
> irq handling. But with a quick glance only sync version is needed there.
Thanks, I missed that. I'll try to fix and send again.
>
> However, I'm not quite sure about this approach. The fix makes sense,
> but it makes me think if the irq handling is designed the wrong way.
>
> While debugging and fixing this, did you think some other irq handling
> approach would be saner?
I tried but could not come up with better approach. The main difficulty is
that there are two contradicting requirements:
1. Some callbacks unregister other callbacks, including themselves, from
DISPC IRQ context.
2. Some functions expect that once a callback is unregistered it is never
executed.
Hence it is natural to split callers into two sets. I'm open for suggestions.
>
> Tomi
Thanks,
Dimitar
^ permalink raw reply
* Re: [PATCH] fbdev: Add Renesas vdc4 framebuffer driver
From: phil.edworthy @ 2012-09-07 14:04 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1344428095-12203-1-git-send-email-phil.edworthy@renesas.com>
Hi,
Anything happening with this patch?
Thanks
Phil
> From: Phil Edworthy <phil.edworthy@renesas.com>
> To: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>, linux-
> fbdev@vger.kernel.org,
> Cc: linux-sh@vger.kernel.org, Phil Edworthy <phil.edworthy@renesas.com>
> Date: 08/08/2012 13:15
> Subject: [PATCH] fbdev: Add Renesas vdc4 framebuffer driver
>
> The vdc4 display hardware is found on the sh7269 device.
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
> drivers/video/Kconfig | 10 +
> drivers/video/Makefile | 1 +
> drivers/video/ren_vdc4fb.c | 653 +++++++++++++++++++++++++++++++++
> +++++++++++
> include/video/ren_vdc4fb.h | 19 ++
> 4 files changed, 683 insertions(+), 0 deletions(-)
> create mode 100644 drivers/video/ren_vdc4fb.c
> create mode 100644 include/video/ren_vdc4fb.h
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 0217f74..89c9250 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -1990,6 +1990,16 @@ config FB_W100
>
> If unsure, say N.
>
> +config FB_REN_VDC4FB
> + tristate "Renesas VDC4 framebuffer support"
> + depends on FB && CPU_SUBTYPE_SH7269
> + select FB_SYS_FILLRECT
> + select FB_SYS_COPYAREA
> + select FB_SYS_IMAGEBLIT
> + select FB_SYS_FOPS
> + ---help---
> + Frame buffer driver for the Renesas VDC4.
> +
> config FB_SH_MOBILE_LCDC
> tristate "SuperH Mobile LCDC framebuffer support"
> depends on FB && (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index ee8dafb..ba69fcb 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -140,6 +140,7 @@ obj-$(CONFIG_SH_MIPI_DSI) += sh_mipi_dsi.o
> obj-$(CONFIG_FB_SH_MOBILE_HDMI) += sh_mobile_hdmi.o
> obj-$(CONFIG_FB_SH_MOBILE_MERAM) += sh_mobile_meram.o
> obj-$(CONFIG_FB_SH_MOBILE_LCDC) += sh_mobile_lcdcfb.o
> +obj-$(CONFIG_FB_REN_VDC4FB) += ren_vdc4fb.o
> obj-$(CONFIG_FB_OMAP) += omap/
> obj-y += omap2/
> obj-$(CONFIG_XEN_FBDEV_FRONTEND) += xen-fbfront.o
> diff --git a/drivers/video/ren_vdc4fb.c b/drivers/video/ren_vdc4fb.c
> new file mode 100644
> index 0000000..1a31e85
> --- /dev/null
> +++ b/drivers/video/ren_vdc4fb.c
> @@ -0,0 +1,653 @@
> +/*
> + * Renesas VDC4 Framebuffer
> + *
> + * Based on sh_mobile_lcdcfb.c
> + * Copyright (c) 2012 Renesas Electronics Europe Ltd
> + *
> + * This file is subject to the terms and conditions of the GNU General
Public
> + * License. See the file "COPYING" in the main directory of this
archive
> + * for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/mm.h>
> +#include <linux/clk.h>
> +#include <linux/sh_clk.h>
> +#include <linux/platform_device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/interrupt.h>
> +#include <linux/vmalloc.h>
> +#include <linux/module.h>
> +#include <video/ren_vdc4fb.h>
> +
> +#define PALETTE_NR 16
> +
> +struct ren_vdc4_priv {
> + void __iomem *base;
> + int irq;
> + struct clk *dot_clk;
> + struct clk *clk;
> + struct fb_info *info;
> + dma_addr_t dma_handle;
> + struct ren_vdc4_info *cfg;
> + u32 pseudo_palette[PALETTE_NR];
> +};
> +
> +/* Register offsets/reading and writing functions */
> +enum {
> + SCL0_UPDATE, SCL0_FRC1, SCL0_FRC2, SCL0_FRC3,
> + SCL0_FRC4, SCL0_FRC5, SCL0_FRC6, SCL0_FRC7,
> + SCL0_DS1, SCL0_US1,
> +
> + GR1_UPDATE, GR1_AB1,
> +
> + GR2_UPDATE, GR2_AB1,
> +
> + GR3_UPDATE, GR3_FLM_RD, GR3_FLM1, GR3_FLM2,
> + GR3_FLM3, GR3_FLM4, GR3_FLM5, GR3_FLM6, GR3_AB1,
> + GR3_AB2, GR3_AB3, GR3_AB4, GR3_AB5, GR3_AB6,
> + GR3_AB7, GR3_AB8, GR3_AB9, GR3_AB10, GR3_AB11,
> + GR3_BASE, GR3_CLUT_INT, GR3_MON,
> +
> + TCON_UPDATE, TCON_TIM, TCON_TIM_STVA1, TCON_TIM_STVA2,
> + TCON_TIM_STVB1, TCON_TIM_STVB2, TCON_TIM_STH1,
> + TCON_TIM_STH2, TCON_TIM_STB1, TCON_TIM_STB2,
> + TCON_TIM_CPV1, TCON_TIM_CPV2, TCON_TIM_POLA1,
> + TCON_TIM_POLA2, TCON_TIM_POLB1, TCON_TIM_POLB2,
> + TCON_TIM_DE,
> +
> + OUT_UPDATE, OUT_SET, OUT_BRIGHT1,
> + OUT_BRIGHT2, OUT_CONTRAST, OUT_PDTHA, OUT_CLK_PHASE,
> +
> + SYSCNT_INT1, SYSCNT_INT2, SYSCNT_INT3, SYSCNT_INT4,
> + SYSCNT_PANEL_CLK, SYSCNT_CLUT
> +};
> +
> +static unsigned long vdc4_offsets[] = {
> + [SCL0_UPDATE] = 0x0100,
> + [SCL0_FRC1] = 0x0104,
> + [SCL0_FRC2] = 0x0108,
> + [SCL0_FRC3] = 0x010C,
> + [SCL0_FRC4] = 0x0110,
> + [SCL0_FRC5] = 0x0114,
> + [SCL0_FRC6] = 0x0118,
> + [SCL0_FRC7] = 0x011C,
> + [SCL0_DS1] = 0x012C,
> + [SCL0_US1] = 0x0148,
> + [GR1_UPDATE] = 0x0200,
> + [GR1_AB1] = 0x0220,
> + [GR2_UPDATE] = 0x0300,
> + [GR2_AB1] = 0x0320,
> + [GR3_UPDATE] = 0x0380,
> + [GR3_FLM_RD] = 0x0384,
> + [GR3_FLM1] = 0x0388,
> + [GR3_FLM2] = 0x038C,
> + [GR3_FLM3] = 0x0390,
> + [GR3_FLM4] = 0x0394,
> + [GR3_FLM5] = 0x0398,
> + [GR3_FLM6] = 0x039C,
> + [GR3_AB1] = 0x03A0,
> + [GR3_AB2] = 0x03A4,
> + [GR3_AB3] = 0x03A8,
> + [GR3_AB4] = 0x03AC,
> + [GR3_AB5] = 0x03B0,
> + [GR3_AB6] = 0x03B4,
> + [GR3_AB7] = 0x03B8,
> + [GR3_AB8] = 0x03BC,
> + [GR3_AB9] = 0x03C0,
> + [GR3_AB10] = 0x03C4,
> + [GR3_AB11] = 0x03C8,
> + [GR3_BASE] = 0x03CC,
> + [GR3_CLUT_INT] = 0x03D0,
> + [GR3_MON] = 0x03D4,
> + [TCON_UPDATE] = 0x0580,
> + [TCON_TIM] = 0x0584,
> + [TCON_TIM_STVA1] = 0x0588,
> + [TCON_TIM_STVA2] = 0x058C,
> + [TCON_TIM_STVB1] = 0x0590,
> + [TCON_TIM_STVB2] = 0x0594,
> + [TCON_TIM_STH1] = 0x0598,
> + [TCON_TIM_STH2] = 0x059C,
> + [TCON_TIM_STB1] = 0x05A0,
> + [TCON_TIM_STB2] = 0x05A4,
> + [TCON_TIM_CPV1] = 0x05A8,
> + [TCON_TIM_CPV2] = 0x05AC,
> + [TCON_TIM_POLA1] = 0x05B0,
> + [TCON_TIM_POLA2] = 0x05B4,
> + [TCON_TIM_POLB1] = 0x05B8,
> + [TCON_TIM_POLB2] = 0x05BC,
> + [TCON_TIM_DE] = 0x05C0,
> + [OUT_UPDATE] = 0x0600,
> + [OUT_SET] = 0x0604,
> + [OUT_BRIGHT1] = 0x0608,
> + [OUT_BRIGHT2] = 0x060C,
> + [OUT_CONTRAST] = 0x0610,
> + [OUT_PDTHA] = 0x0614,
> + [OUT_CLK_PHASE] = 0x0624,
> + [SYSCNT_INT1] = 0x0680,
> + [SYSCNT_INT2] = 0x0684,
> + [SYSCNT_INT3] = 0x0688,
> + [SYSCNT_INT4] = 0x068C,
> + [SYSCNT_PANEL_CLK] = 0x0690, /* 16-bit */
> + [SYSCNT_CLUT] = 0x0692, /* 16-bit */
> +};
> +
> +/* SYSCNT */
> +#define ICKEN (1 << 8)
> +
> +/* SCL Syncs */
> +#define FREE_RUN_VSYNC 0x0001
> +
> +/* OUTPUT */
> +#define OUT_FMT_RGB666 (1 << 12)
> +
> +/* TCON Timings */
> +#define STVB_SEL_BITS 0x0007
> +#define STVB_HS_SEL 2
> +
> +#define STH2_SEL_BITS 0x0007
> +#define STH2_DE_SEL 7
> +
> +/* OUTCLK */
> +#define LCD_DATA_EDGE 0x0100
> +#define STVB_EDGE 0x0020
> +#define STH_EDGE 0x0010
> +
> +/* SCL_UPDATE */
> +#define SCL0_UPDATE_BIT 0x0100
> +#define SCL0_VEN_BIT 0x0010
> +
> +/* TCON_UPDATE */
> +#define TCON_VEN_BIT 0x0001
> +
> +/* OUT_UPDATE */
> +#define OUTCNT_VEN_BIT 0x0001
> +
> +/* GR_UPDATE */
> +#define P_VEN_UPDATE 0x0010
> +#define IBUS_VEN_UPDATE 0x0001
> +
> +/* GR_AB1 */
> +#define DISPSEL_BCKGND 0x0000
> +#define DISPSEL_LOWER 0x0001
> +#define DISPSEL_CUR 0x0002
> +
> +/* GR_FLM_RD */
> +#define FB_R_ENB 0x01
> +
> +
> +static void vdc4_write(struct ren_vdc4_priv *priv,
> + unsigned long reg_offs, unsigned long data)
> +{
> + if ((SYSCNT_PANEL_CLK = reg_offs) || (SYSCNT_CLUT = reg_offs))
> + iowrite16(data, priv->base + vdc4_offsets[reg_offs]);
> + else
> + iowrite32(data, priv->base + vdc4_offsets[reg_offs]);
> +}
> +
> +static unsigned long vdc4_read(struct ren_vdc4_priv *priv,
> + unsigned long reg_offs)
> +{
> + if ((SYSCNT_PANEL_CLK = reg_offs) || (SYSCNT_CLUT = reg_offs))
> + return ioread16(priv->base + vdc4_offsets[reg_offs]);
> + else
> + return ioread32(priv->base + vdc4_offsets[reg_offs]);
> +}
> +
> +static irqreturn_t ren_vdc4_irq(int irq, void *data)
> +{
> + /* Not currently implemented/used */
> + return IRQ_HANDLED;
> +}
> +
> +static void lcd_clear_display(struct ren_vdc4_priv *priv)
> +{
> + unsigned char *pdest;
> + unsigned long size;
> +
> + pdest = (unsigned char *)priv->dma_handle;
> + size = priv->cfg->lcd_cfg.xres * priv->cfg->lcd_cfg.yres * 2;
> +
> + memset(pdest, 0, size);
> +}
> +
> +static void restart_tft_display(struct ren_vdc4_priv *priv,
> + int clock_source)
> +{
> + struct fb_videomode *lcd;
> + unsigned long h;
> + unsigned long v;
> + unsigned long tmp;
> +
> + /* FB setup */
> + lcd = &priv->cfg->lcd_cfg;
> + lcd_clear_display(priv);
> +
> + /* VDC clock Setup */
> + tmp = priv->cfg->clock_divider;
> + tmp |= clock_source << 12;
> + tmp |= ICKEN;
> + vdc4_write(priv, SYSCNT_PANEL_CLK, tmp);
> +
> + /* Clear and Disable all interrupts */
> + vdc4_write(priv, SYSCNT_INT1, 0);
> + vdc4_write(priv, SYSCNT_INT2, 0);
> + vdc4_write(priv, SYSCNT_INT3, 0);
> + vdc4_write(priv, SYSCNT_INT4, 0);
> +
> + /* Setup free-running syncs */
> + vdc4_write(priv, SCL0_FRC3, FREE_RUN_VSYNC);
> +
> + /* Disable scale up/down */
> + vdc4_write(priv, SCL0_DS1, 0);
> + vdc4_write(priv, SCL0_US1, 0);
> +
> + /* Timing registers */
> + h = lcd->hsync_len + lcd->left_margin + lcd->xres +
lcd->right_margin;
> + v = lcd->vsync_len + lcd->upper_margin + lcd->yres +
lcd->lower_margin;
> + tmp = (v - 1) << 16;
> + tmp |= h - 1;
> + vdc4_write(priv, SCL0_FRC4, tmp);
> +
> + vdc4_write(priv, TCON_TIM, (((h - 1) / 2) << 16));
> +
> + tmp = (lcd->vsync_len + lcd->upper_margin) << 16;
> + tmp |= lcd->yres;
> + vdc4_write(priv, SCL0_FRC6, tmp);
> + vdc4_write(priv, TCON_TIM_STVB1, tmp);
> + vdc4_write(priv, GR3_AB2, tmp);
> +
> + tmp = lcd->left_margin << 16;
> + tmp |= lcd->xres;
> + vdc4_write(priv, SCL0_FRC7, tmp);
> + vdc4_write(priv, TCON_TIM_STB1, tmp);
> + vdc4_write(priv, GR3_AB3, tmp);
> +
> + vdc4_write(priv, SCL0_FRC1, 0);
> + vdc4_write(priv, SCL0_FRC2, 0);
> + vdc4_write(priv, SCL0_FRC5, 0);
> +
> + /* Set output format */
> + vdc4_write(priv, OUT_SET, OUT_FMT_RGB666);
> +
> + /* STH TCON Timing */
> + tmp = priv->cfg->hs_pulse_width;
> + tmp |= priv->cfg->hs_start_pos << 16;
> + vdc4_write(priv, TCON_TIM_STH1, tmp);
> +
> + /* Setup STVB as HSYNC */
> + tmp = vdc4_read(priv, TCON_TIM_STVB2);
> + tmp &= ~STVB_SEL_BITS;
> + tmp |= STVB_HS_SEL;
> + vdc4_write(priv, TCON_TIM_STVB2, tmp);
> +
> + tmp = vdc4_read(priv, OUT_CLK_PHASE);
> + tmp &= ~STVB_EDGE;
> + vdc4_write(priv, OUT_CLK_PHASE, tmp);
> +
> + /* Setup STH as DE */
> + tmp = vdc4_read(priv, TCON_TIM_STH2);
> + tmp &= ~STH2_SEL_BITS;
> + tmp |= STH2_DE_SEL;
> + vdc4_write(priv, TCON_TIM_STH2, tmp);
> +
> + tmp = vdc4_read(priv, OUT_CLK_PHASE);
> + tmp &= ~STH_EDGE;
> + vdc4_write(priv, OUT_CLK_PHASE, tmp);
> +
> + /* Output clock rising edge */
> + tmp = vdc4_read(priv, OUT_CLK_PHASE);
> + tmp &= ~LCD_DATA_EDGE;
> + vdc4_write(priv, OUT_CLK_PHASE, tmp);
> +
> + /* Setup graphics buffers and update all registers */
> + vdc4_write(priv, GR1_AB1, DISPSEL_BCKGND);
> + vdc4_write(priv, GR2_AB1, DISPSEL_LOWER);
> + vdc4_write(priv, GR3_AB1, DISPSEL_CUR);
> +
> + /* Setup framebuffer base/output */
> + vdc4_write(priv, GR3_FLM_RD, FB_R_ENB);
> +
> + vdc4_write(priv, GR3_FLM2, (unsigned long)priv->info->screen_base);
> +
> + vdc4_write(priv, GR3_FLM3, (lcd->xres * 2) << 16);
> +
> + tmp = vdc4_read(priv, GR3_FLM5);
> + tmp |= lcd->yres << 16;
> + vdc4_write(priv, GR3_FLM5, tmp);
> +
> + tmp = lcd->xres << 16;
> + vdc4_write(priv, GR3_FLM6, tmp);
> +
> + /* Apply all register settings */
> + vdc4_write(priv, SCL0_UPDATE, SCL0_VEN_BIT | SCL0_UPDATE_BIT);
> + vdc4_write(priv, GR1_UPDATE, P_VEN_UPDATE);
> + vdc4_write(priv, GR2_UPDATE, P_VEN_UPDATE);
> + vdc4_write(priv, GR3_UPDATE, P_VEN_UPDATE | IBUS_VEN_UPDATE);
> + vdc4_write(priv, OUT_UPDATE, OUTCNT_VEN_BIT);
> + vdc4_write(priv, TCON_UPDATE, TCON_VEN_BIT);
> +}
> +
> +static int ren_vdc4_setup_clocks(struct platform_device *pdev,
> + int clock_source,
> + struct ren_vdc4_priv *priv)
> +{
> + priv->clk = clk_get(&pdev->dev, "vdc4");
> + if (IS_ERR(priv->clk)) {
> + dev_err(&pdev->dev, "cannot get clock \"vdc4\"\n");
> + return PTR_ERR(priv->clk);
> + }
> +
> + if (clock_source = VDC4_PERI_CLK) {
> + priv->dot_clk = clk_get(&pdev->dev, "peripheral_clk");
> + if (IS_ERR(priv->dot_clk)) {
> + dev_err(&pdev->dev, "cannot get peripheral clock\n");
> + clk_put(priv->clk);
> + return PTR_ERR(priv->dot_clk);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int ren_vdc4_setcolreg(u_int regno,
> + u_int red, u_int green, u_int blue,
> + u_int transp, struct fb_info *info)
> +{
> + u32 *palette = info->pseudo_palette;
> +
> + if (regno >= PALETTE_NR)
> + return -EINVAL;
> +
> + /* only FB_VISUAL_TRUECOLOR supported */
> +
> + red >>= 16 - info->var.red.length;
> + green >>= 16 - info->var.green.length;
> + blue >>= 16 - info->var.blue.length;
> + transp >>= 16 - info->var.transp.length;
> +
> + palette[regno] = (red << info->var.red.offset) |
> + (green << info->var.green.offset) |
> + (blue << info->var.blue.offset) |
> + (transp << info->var.transp.offset);
> +
> + return 0;
> +}
> +
> +static struct fb_fix_screeninfo ren_vdc4_fix = {
> + .id = "Renesas VDC4FB",
> + .type = FB_TYPE_PACKED_PIXELS,
> + .visual = FB_VISUAL_TRUECOLOR,
> + .accel = FB_ACCEL_NONE,
> +};
> +
> +static struct fb_ops ren_vdc4_ops = {
> + .owner = THIS_MODULE,
> + .fb_setcolreg = ren_vdc4_setcolreg,
> + .fb_read = fb_sys_read,
> + .fb_write = fb_sys_write,
> + .fb_fillrect = sys_fillrect,
> + .fb_copyarea = sys_copyarea,
> + .fb_imageblit = sys_imageblit,
> +};
> +
> +static int ren_vdc4_set_bpp(struct fb_var_screeninfo *var, int bpp)
> +{
> + switch (bpp) {
> + case 16: /* RGB 565 */
> + var->red.offset = 11;
> + var->red.length = 5;
> + var->green.offset = 5;
> + var->green.length = 6;
> + var->blue.offset = 0;
> + var->blue.length = 5;
> + var->transp.offset = 0;
> + var->transp.length = 0;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + var->bits_per_pixel = bpp;
> + var->red.msb_right = 0;
> + var->green.msb_right = 0;
> + var->blue.msb_right = 0;
> + var->transp.msb_right = 0;
> + return 0;
> +}
> +
> +/* PM Functions */
> +static int ren_vdc4_start(struct ren_vdc4_priv *priv,
> + int clock_source)
> +{
> + int ret;
> +
> + ret = clk_enable(priv->clk);
> + if (ret < 0)
> + return ret;
> +
> + if (priv->dot_clk) {
> + ret = clk_enable(priv->dot_clk);
> + if (ret < 0)
> + return ret;
> + }
> +
> + restart_tft_display(priv, clock_source);
> +
> + return ret;
> +}
> +
> +static void ren_vdc4_stop(struct ren_vdc4_priv *priv)
> +{
> + if (priv->dot_clk)
> + clk_disable(priv->dot_clk);
> + clk_disable(priv->clk);
> +}
> +
> +static int ren_vdc4_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> +
> + ren_vdc4_stop(platform_get_drvdata(pdev));
> + return 0;
> +}
> +
> +static int ren_vdc4_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct ren_vdc4_info *pdata = pdev->dev.platform_data;
> +
> + return ren_vdc4_start(platform_get_drvdata(pdev),
pdata->clock_source);
> +}
> +
> +static const struct dev_pm_ops ren_vdc4_dev_pm_ops = {
> + .suspend = ren_vdc4_suspend,
> + .resume = ren_vdc4_resume,
> +};
> +
> +static int ren_vdc4_remove(struct platform_device *pdev);
> +
> +static int __devinit ren_vdc4_probe(struct platform_device *pdev)
> +{
> + struct fb_info *info;
> + struct ren_vdc4_priv *priv;
> + struct ren_vdc4_info *pdata = pdev->dev.platform_data;
> + struct resource *res;
> + void *buf;
> + int irq, error;
> +
> + if (!pdata) {
> + dev_err(&pdev->dev, "no platform data defined\n");
> + return -EINVAL;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + irq = platform_get_irq(pdev, 0);
> + if (!res || irq < 0) {
> + dev_err(&pdev->dev, "cannot get platform resources\n");
> + return -ENOENT;
> + }
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv) {
> + dev_err(&pdev->dev, "cannot allocate device data\n");
> + return -ENOMEM;
> + }
> +
> + platform_set_drvdata(pdev, priv);
> +
> + error = request_irq(irq, ren_vdc4_irq, 0, dev_name(&pdev->dev),
priv);
> + if (error) {
> + dev_err(&pdev->dev, "unable to request irq\n");
> + goto err1;
> + }
> +
> + priv->irq = irq;
> + pdata = pdev->dev.platform_data;
> +
> + priv->cfg = pdata;
> +
> + error = ren_vdc4_setup_clocks(pdev, pdata->clock_source, priv);
> + if (error) {
> + dev_err(&pdev->dev, "unable to setup clocks\n");
> + goto err1;
> + }
> +
> + priv->base = ioremap_nocache(res->start, resource_size(res));
> + if (!priv->base) {
> + dev_err(&pdev->dev, "unable to ioremap\n");
> + goto err1;
> + }
> +
> + priv->info = framebuffer_alloc(0, &pdev->dev);
> + if (!priv->info) {
> + dev_err(&pdev->dev, "unable to allocate fb_info\n");
> + goto err1;
> + }
> +
> + info = priv->info;
> + info->fbops = &ren_vdc4_ops;
> + info->var.xres = info->var.xres_virtual = pdata->lcd_cfg.xres;
> + info->var.yres = info->var.yres_virtual = pdata->lcd_cfg.yres;
> + info->var.width = pdata->panel_width;
> + info->var.height = pdata->panel_height;
> + info->var.activate = FB_ACTIVATE_NOW;
> + info->pseudo_palette = priv->pseudo_palette;
> + error = ren_vdc4_set_bpp(&info->var, pdata->bpp);
> + if (error)
> + goto err1;
> +
> + info->fix = ren_vdc4_fix;
> + info->fix.line_length = pdata->lcd_cfg.xres * (pdata->bpp / 8);
> + info->fix.smem_len = info->fix.line_length * pdata->lcd_cfg.yres;
> +
> + buf = dma_alloc_coherent(&pdev->dev, info->fix.smem_len,
> + &priv->dma_handle, GFP_KERNEL);
> + if (!buf) {
> + dev_err(&pdev->dev, "unable to allocate buffer\n");
> + goto err1;
> + }
> +
> + info->flags = FBINFO_FLAG_DEFAULT;
> +
> + error = fb_alloc_cmap(&info->cmap, PALETTE_NR, 0);
> + if (error < 0) {
> + dev_err(&pdev->dev, "unable to allocate cmap\n");
> + goto err1;
> + }
> +
> + memset(buf, 0, info->fix.smem_len);
> + info->fix.smem_start = priv->dma_handle;
> + info->screen_base = buf;
> + info->device = &pdev->dev;
> + info->par = priv;
> +
> + if (error)
> + goto err1;
> +
> + ren_vdc4_start(priv, pdata->clock_source);
> + if (error) {
> + dev_err(&pdev->dev, "unable to start hardware\n");
> + goto err1;
> + }
> +
> + info = priv->info;
> +
> + error = register_framebuffer(info);
> + if (error < 0)
> + goto err1;
> +
> + dev_info(info->dev,
> + "registered %s as %udx%ud %dbpp.\n",
> + pdev->name,
> + (int) pdata->lcd_cfg.xres,
> + (int) pdata->lcd_cfg.yres,
> + pdata->bpp);
> +
> + return 0;
> +
> +err1:
> + ren_vdc4_remove(pdev);
> + return error;
> +}
> +
> +static int ren_vdc4_remove(struct platform_device *pdev)
> +{
> + struct ren_vdc4_priv *priv = platform_get_drvdata(pdev);
> + struct fb_info *info;
> +
> + if (priv->info->dev)
> + unregister_framebuffer(priv->info);
> +
> + ren_vdc4_stop(priv);
> +
> + info = priv->info;
> +
> + if (!info || !info->device) {
> + dev_err(&pdev->dev, "Failed to dealloc/release fb_info\n");
> + } else {
> + fb_dealloc_cmap(&info->cmap);
> + framebuffer_release(info);
> + }
> +
> + if (priv->dot_clk)
> + clk_put(priv->dot_clk);
> + clk_put(priv->clk);
> +
> + if (priv->base)
> + iounmap(priv->base);
> +
> + if (priv->irq)
> + free_irq(priv->irq, priv);
> +
> + kfree(priv);
> + return 0;
> +}
> +
> +static struct platform_driver ren_vdc4_driver = {
> + .driver = {
> + .name = "ren_vdc4fb",
> + .owner = THIS_MODULE,
> + .pm = &ren_vdc4_dev_pm_ops,
> + },
> + .probe = ren_vdc4_probe,
> + .remove = ren_vdc4_remove,
> +};
> +
> +static int __init ren_vdc4_init(void)
> +{
> + return platform_driver_register(&ren_vdc4_driver);
> +}
> +
> +static void __exit ren_vdc4_exit(void)
> +{
> + platform_driver_unregister(&ren_vdc4_driver);
> +}
> +
> +module_init(ren_vdc4_init);
> +module_exit(ren_vdc4_exit);
> +
> +MODULE_DESCRIPTION("Renesas VDC4 Framebuffer driver");
> +MODULE_AUTHOR("Phil Edworthy <phil.edworthy@renesas.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/video/ren_vdc4fb.h b/include/video/ren_vdc4fb.h
> new file mode 100644
> index 0000000..e91a515
> --- /dev/null
> +++ b/include/video/ren_vdc4fb.h
> @@ -0,0 +1,19 @@
> +#ifndef __REN_VDC4_H__
> +#define __REN_VDC4_H__
> +
> +#include <linux/fb.h>
> +
> +enum { VDC4_EXTCLK = 1, VDC4_PERI_CLK };
> +
> +struct ren_vdc4_info {
> + int bpp;
> + int clock_source;
> + int clock_divider;
> + int hs_pulse_width;
> + int hs_start_pos;
> + struct fb_videomode lcd_cfg;
> + unsigned long panel_width;
> + unsigned long panel_height;
> +};
> +
> +#endif
> --
> 1.7.5.4
>
^ permalink raw reply
* Re: [PATCH 12/17] OMAPDSS: clean up dss_mgr_set_timings
From: Archit Taneja @ 2012-09-07 11:48 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap
In-Reply-To: <1347012673.2646.3.camel@deskari>
On Friday 07 September 2012 03:41 PM, Tomi Valkeinen wrote:
> On Wed, 2012-09-05 at 11:25 +0300, Tomi Valkeinen wrote:
>> dss_mgr_set_timings() can only be called when the output is not active.
>> This means that most of the code in the function is extra, as there's no
>> need to write the values to registers, etc, because that will be handled
>> when the output will be enabled.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>> drivers/video/omap2/dss/apply.c | 18 ++++++++----------
>> 1 file changed, 8 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
>> index 53629dd..1b49019 100644
>> --- a/drivers/video/omap2/dss/apply.c
>> +++ b/drivers/video/omap2/dss/apply.c
>> @@ -1314,21 +1314,19 @@ void dss_mgr_set_timings(struct omap_overlay_manager *mgr,
>> const struct omap_video_timings *timings)
>> {
>> unsigned long flags;
>> -
>> - mutex_lock(&apply_lock);
>> + struct mgr_priv_data *mp = get_mgr_priv(mgr);
>>
>> spin_lock_irqsave(&data_lock, flags);
>>
>> - dss_apply_mgr_timings(mgr, timings);
>> -
>> - dss_write_regs();
>> - dss_set_go_bits();
>> + if (mp->enabled) {
>> + DSSERR("cannot set timings for %s: manager needs to be disabled\n",
>> + mgr->name);
>> + goto out;
>> + }
>
> There was a problem with this one. When using manual update display, we
> call set_timings before each update, and the mgr is enabled at that
> time.
>
> I'll fix this by changing the check from mp->enabled to mp->updating.
> That flag tells if the DISPC channel is actually enabled or not. Enabled
> flag just tells that the channel is being reserved, although for auto
> update displays that also implies "updating".
>
> But do you see any reason to call set_timings before each update? It was
> required when we have partial update support, but now we support only
> full screen updates, so isn't it enough to set the timings just once
> when configuring?
I think we put it there for rotation. We may need to swap manager width
and height before an update.
Archit
^ permalink raw reply
* Re: [PATCH 0/8] OMAPDSS: Misc improvements
From: Tomi Valkeinen @ 2012-09-07 10:16 UTC (permalink / raw)
To: Tony Lindgren; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <20120906201330.GF1303@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 525 bytes --]
On Thu, 2012-09-06 at 13:13 -0700, Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [120904 00:23]:
> > Hi Tony,
> >
> > Can you check the arch/arm patches below, and suggest how you'd like to
> > go forward with them?
>
> Acked them, then as soon as we have the initial immutable header
> move branch available, you should merge with that to avoid
> merge conflicts in upstream.
Thanks, but you missed the first patch "[PATCH 1/8] OMAPDSS: HDMI: Move
GPIO handling to HDMI driver".
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 12/17] OMAPDSS: clean up dss_mgr_set_timings
From: Tomi Valkeinen @ 2012-09-07 10:11 UTC (permalink / raw)
To: archit; +Cc: linux-fbdev, linux-omap
In-Reply-To: <1346833555-31258-13-git-send-email-tomi.valkeinen@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1907 bytes --]
On Wed, 2012-09-05 at 11:25 +0300, Tomi Valkeinen wrote:
> dss_mgr_set_timings() can only be called when the output is not active.
> This means that most of the code in the function is extra, as there's no
> need to write the values to registers, etc, because that will be handled
> when the output will be enabled.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/apply.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
> index 53629dd..1b49019 100644
> --- a/drivers/video/omap2/dss/apply.c
> +++ b/drivers/video/omap2/dss/apply.c
> @@ -1314,21 +1314,19 @@ void dss_mgr_set_timings(struct omap_overlay_manager *mgr,
> const struct omap_video_timings *timings)
> {
> unsigned long flags;
> -
> - mutex_lock(&apply_lock);
> + struct mgr_priv_data *mp = get_mgr_priv(mgr);
>
> spin_lock_irqsave(&data_lock, flags);
>
> - dss_apply_mgr_timings(mgr, timings);
> -
> - dss_write_regs();
> - dss_set_go_bits();
> + if (mp->enabled) {
> + DSSERR("cannot set timings for %s: manager needs to be disabled\n",
> + mgr->name);
> + goto out;
> + }
There was a problem with this one. When using manual update display, we
call set_timings before each update, and the mgr is enabled at that
time.
I'll fix this by changing the check from mp->enabled to mp->updating.
That flag tells if the DISPC channel is actually enabled or not. Enabled
flag just tells that the channel is being reserved, although for auto
update displays that also implies "updating".
But do you see any reason to call set_timings before each update? It was
required when we have partial update support, but now we support only
full screen updates, so isn't it enough to set the timings just once
when configuring?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v5 1/4] Runtime Interpreted Power Sequences
From: Heiko Stübner @ 2012-09-07 9:08 UTC (permalink / raw)
To: Alex Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org,
linux-doc@vger.kernel.org
In-Reply-To: <1887927.1deN8M9siP@percival>
Am Freitag, 7. September 2012, 10:04:24 schrieb Alex Courbot:
> > For your power_seq_run function you write that it simply returns an error
> > code on failure and looking through it I also just found the error return
> > statement. This would leave a device half turned on.
> >
> > So I'm wondering, if it shouldn't turn off all the things it turned on
> > until the step that produced the error. All your possible step types
> > (execpt the delay) are booleans, so it should be possible to simply
> > negate them when backtracking through the previous steps.
>
> Indeed, I think you raised an important point. Right now all step types are
> invertible, but we cannot rely on that statement to be true forever. For
> instance, one short-term improvement will be to allow finer regulator
> control, like voltage setting. In this case, how can we go back to the
> initial state without recording it?
>
> If e.g. the power on sequence fails at step N (of M steps for that
> sequence), one could try playing the corresponding power off sequence
> (either completely of from step M - N), but then again we cannot rely on
> sequences to be perfectly symetrical. Maybe this is more something for the
> calling driver to check for and control?
Am Freitag, 7. September 2012, 10:15:03 schrieb Mark Brown:
> On Fri, Sep 07, 2012 at 05:04:24PM +0900, Alex Courbot wrote:
> > If e.g. the power on sequence fails at step N (of M steps for that
> > sequence), one could try playing the corresponding power off sequence
> > (either completely of from step M - N), but then again we cannot rely on
> > sequences to be perfectly symetrical. Maybe this is more something for
> > the calling driver to check for and control?
>
> That had been my thought too - depending on what the sequence is for it
> may be that the corrective action is something very different to
> reversing the sequence, for example a device reset may be required.
If I understood the description correctly, the power sequence should be
transparent to the driver, as it implements board specific actions and
shouldn't bother the driver with it to much. Therefore my thoughts went along
the lines how gpio_request_array handles this, always producing a sane state
at the end.
Recording the previous state, could be done by making a copy of the current
sequence, and just noting the previous values (including voltages etc) in the
respective entries. And in the error case running this new sequence from the
error point instead to power down again.
As both Alex and Mark wrote, reversing the sequence might be the action of
choice only for some devices, but others might need to run a completely
different powerdown sequence and still others would need special handling.
Would it be possible to encode this in the sequence definition, something like
on-error = "reverse"
on-error = "sequence"
error-seq = <&other_sequence>
on-error = "driver"
with better names and types of course.
This would keep the power sequence transparent to most drivers and only the
real esoteric ones would need to do their special handling on their own.
Heiko
^ 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