[PATCH 0/3] OMAPDSS: Miscellaneous DISPC Fixes

[PATCH 0/3] OMAPDSS: Miscellaneous DISPC Fixes

[Archit Taneja]

These are some minor Display Controller fixes.

Tested on 4430sdp and 3430sdp on the tree:

git://gitorious.org/linux-omap-dss2/linux.git dev

Lajos Molnar (3):
  OMAPDSS: DISPC: Fix OMAP4 supported color formats
  OMAPDSS: DISPC: Fix FIR coefficients
  OMAPDSS: DSS: Add runtime_pm protection around wait_for_vsync.

 drivers/video/omap2/dss/dispc.c        |   24 +++++++++++++++---------
 drivers/video/omap2/dss/dispc_coefs.c  |    9 ++++-----
 drivers/video/omap2/dss/dss_features.c |    3 ++-
 3 files changed, 21 insertions(+), 15 deletions(-)

-- 
1.7.5.4

[PATCH 1/3] OMAPDSS: DISPC: Fix OMAP4 supported color formats

[Archit Taneja]

From: Lajos Molnar <lajos@ti.com>

Add missing but supported color formats for GFX pipeline in dss features:
RGBX16-4444, RGBA16-4444 and XRGB16-1555.

In dispc_ovl_set_color_mode():

- Remove unsupported modes on GFX pipeline: YUV2 and UYVY. Replace these by
  missing modes supported by GFX pipelines: RGBX16-4444 and RGBA16-4444.

- Fix swapped modes on VID pipelines: RGBX16-4444 and XRGB16-4444.

Signed-off-by: Lajos Molnar <lajos@ti.com>
Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dispc.c        |    8 ++++----
 drivers/video/omap2/dss/dss_features.c |    3 ++-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 14b1f54..df6259e 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -736,11 +736,11 @@ static void dispc_ovl_set_color_mode(enum omap_plane plane,
 		switch (color_mode) {
 		case OMAP_DSS_COLOR_NV12:
 			m = 0x0; break;
-		case OMAP_DSS_COLOR_RGB12U:
+		case OMAP_DSS_COLOR_RGBX16:
 			m = 0x1; break;
 		case OMAP_DSS_COLOR_RGBA16:
 			m = 0x2; break;
-		case OMAP_DSS_COLOR_RGBX16:
+		case OMAP_DSS_COLOR_RGB12U:
 			m = 0x4; break;
 		case OMAP_DSS_COLOR_ARGB16:
 			m = 0x5; break;
@@ -789,9 +789,9 @@ static void dispc_ovl_set_color_mode(enum omap_plane plane,
 			m = 0x8; break;
 		case OMAP_DSS_COLOR_RGB24P:
 			m = 0x9; break;
-		case OMAP_DSS_COLOR_YUV2:
+		case OMAP_DSS_COLOR_RGBX16:
 			m = 0xa; break;
-		case OMAP_DSS_COLOR_UYVY:
+		case OMAP_DSS_COLOR_RGBA16:
 			m = 0xb; break;
 		case OMAP_DSS_COLOR_ARGB32:
 			m = 0xc; break;
diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index 0a92643..ce14aa6 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -190,7 +190,8 @@ static const enum omap_color_mode omap4_dss_supported_color_modes[] = {
 	OMAP_DSS_COLOR_RGB16 | OMAP_DSS_COLOR_RGB24U |
 	OMAP_DSS_COLOR_RGB24P | OMAP_DSS_COLOR_ARGB32 |
 	OMAP_DSS_COLOR_RGBA32 | OMAP_DSS_COLOR_RGBX32 |
-	OMAP_DSS_COLOR_ARGB16_1555,
+	OMAP_DSS_COLOR_ARGB16_1555 | OMAP_DSS_COLOR_RGBX16 |
+	OMAP_DSS_COLOR_RGBA16 | OMAP_DSS_COLOR_XRGB16_1555,
 
 	/* OMAP_DSS_VIDEO1 */
 	OMAP_DSS_COLOR_RGB16 | OMAP_DSS_COLOR_RGB12U |
-- 
1.7.5.4

[PATCH 2/3] OMAPDSS: DISPC: Fix FIR coefficients

[Archit Taneja]

From: Lajos Molnar <lajos@ti.com>

coef3_M8 had an incorrect phase with 50% more intensity. This resulted in
banding on slightly down/upscaled images. Fixed a rounding error in coef5_M9.

Also removed ARRAY_LEN macro as ARRAY_SIZE is the standard linux one.

Signed-off-by: Lajos Molnar <lajos@ti.com>
Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dispc_coefs.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc_coefs.c b/drivers/video/omap2/dss/dispc_coefs.c
index 069bccb..038c15b 100644
--- a/drivers/video/omap2/dss/dispc_coefs.c
+++ b/drivers/video/omap2/dss/dispc_coefs.c
@@ -19,14 +19,13 @@
 
 #include <linux/kernel.h>
 #include <video/omapdss.h>
-#include "dispc.h"
 
-#define ARRAY_LEN(array) (sizeof(array) / sizeof(array[0]))
+#include "dispc.h"
 
 static const struct dispc_coef coef3_M8[8] = {
 	{ 0,  0, 128,  0, 0 },
 	{ 0, -4, 123,  9, 0 },
-	{ 0, -4, 108, 87, 0 },
+	{ 0, -4, 108, 24, 0 },
 	{ 0, -2,  87, 43, 0 },
 	{ 0, 64,  64,  0, 0 },
 	{ 0, 43,  87, -2, 0 },
@@ -168,7 +167,7 @@ static const struct dispc_coef coef5_M8[8] = {
 
 static const struct dispc_coef coef5_M9[8] = {
 	{  -3,  10, 114,  10,  -3 },
-	{  -6,  24, 110,   0,  -1 },
+	{  -6,  24, 111,   0,  -1 },
 	{  -8,  40, 103,  -7,   0 },
 	{ -11,  58,  91, -11,   1 },
 	{   0, -12,  76,  76, -12 },
@@ -319,7 +318,7 @@ const struct dispc_coef *dispc_ovl_get_scale_coef(int inc, int five_taps)
 	};
 
 	inc /= 128;
-	for (i = 0; i < ARRAY_LEN(coefs); ++i)
+	for (i = 0; i < ARRAY_SIZE(coefs); ++i)
 		if (inc >= coefs[i].Mmin && inc <= coefs[i].Mmax)
 			return five_taps ? coefs[i].coef_5 : coefs[i].coef_3;
 	return NULL;
-- 
1.7.5.4

[PATCH 3/3] OMAPDSS: DSS: Add runtime_pm protection around wait_for_vsync.

[Archit Taneja]

From: Lajos Molnar <lajos@ti.com>

If DSS is suspended during a wait_for_vsync operation, it may loose its clock.
Request runtime_pm around wait_for_vsync.

Signed-off-by: Lajos Molnar <lajos@ti.com>
Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index df6259e..dd08356 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -3228,11 +3228,15 @@ int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
 	int r;
 	DECLARE_COMPLETION_ONSTACK(completion);
 
+	r = dispc_runtime_get();
+	if (r)
+		return r;
+
 	r = omap_dispc_register_isr(dispc_irq_wait_handler, &completion,
 			irqmask);
 
 	if (r)
-		return r;
+		goto done;
 
 	timeout = wait_for_completion_interruptible_timeout(&completion,
 			timeout);
@@ -3240,12 +3244,14 @@ int omap_dispc_wait_for_irq_interruptible_timeout(u32 irqmask,
 	omap_dispc_unregister_isr(dispc_irq_wait_handler, &completion, irqmask);
 
 	if (timeout == 0)
-		return -ETIMEDOUT;
+		r = -ETIMEDOUT;
+	else if (timeout == -ERESTARTSYS)
+		r = timeout;
 
-	if (timeout == -ERESTARTSYS)
-		return -ERESTARTSYS;
+done:
+	dispc_runtime_put();
 
-	return 0;
+	return r;
 }
 
 #ifdef CONFIG_OMAP2_DSS_FAKE_VSYNC
-- 
1.7.5.4

Re: [PATCH 3/3] OMAPDSS: DSS: Add runtime_pm protection around wait_for_vsync.

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 894 bytes --]

On Tue, 2012-02-21 at 19:36 +0530, Archit Taneja wrote:
> From: Lajos Molnar <lajos@ti.com>
> 
> If DSS is suspended during a wait_for_vsync operation, it may loose its clock.
> Request runtime_pm around wait_for_vsync.
> 
> Signed-off-by: Lajos Molnar <lajos@ti.com>
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
>  drivers/video/omap2/dss/dispc.c |   16 +++++++++++-----
>  1 files changed, 11 insertions(+), 5 deletions(-)

This only handles omap_dispc_wait_for_irq_interruptible_timeout(),
there's also omap_dispc_wait_for_irq_timeout().

However, I think it'd be better to do the runtime_get/put in the caller,
instead of in these dispc's wait funcs. While it doesn't really matter
with dss_mgr_wait_for_vsync(), for dss_mgr/ovl_wait_for_go() it makes
much more sense to get/put there just once, instead of every time the
omap_dispc_wait_* is called.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] OMAPDSS: DSS: Add runtime_pm protection around wait_for_vsync.

[Archit Taneja]

On Tuesday 21 February 2012 09:38 PM, Tomi Valkeinen wrote:
> On Tue, 2012-02-21 at 19:36 +0530, Archit Taneja wrote:
>> From: Lajos Molnar<lajos@ti.com>
>>
>> If DSS is suspended during a wait_for_vsync operation, it may loose its clock.
>> Request runtime_pm around wait_for_vsync.
>>
>> Signed-off-by: Lajos Molnar<lajos@ti.com>
>> Signed-off-by: Archit Taneja<archit@ti.com>
>> ---
>>   drivers/video/omap2/dss/dispc.c |   16 +++++++++++-----
>>   1 files changed, 11 insertions(+), 5 deletions(-)
>
> This only handles omap_dispc_wait_for_irq_interruptible_timeout(),
> there's also omap_dispc_wait_for_irq_timeout().
>
> However, I think it'd be better to do the runtime_get/put in the caller,
> instead of in these dispc's wait funcs. While it doesn't really matter
> with dss_mgr_wait_for_vsync(), for dss_mgr/ovl_wait_for_go() it makes
> much more sense to get/put there just once, instead of every time the
> omap_dispc_wait_* is called.

Right, that makes sense. Btw, in the current code, how do we ensure that 
clocks are enabled when someone calls omap_dss_mgr_apply().

Archit

>
>   Tomi
>

Re: [PATCH 3/3] OMAPDSS: DSS: Add runtime_pm protection around wait_for_vsync.

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

On Wed, 2012-02-22 at 11:15 +0530, Archit Taneja wrote:
> On Tuesday 21 February 2012 09:38 PM, Tomi Valkeinen wrote:
> > On Tue, 2012-02-21 at 19:36 +0530, Archit Taneja wrote:
> >> From: Lajos Molnar<lajos@ti.com>
> >>
> >> If DSS is suspended during a wait_for_vsync operation, it may loose its clock.
> >> Request runtime_pm around wait_for_vsync.
> >>
> >> Signed-off-by: Lajos Molnar<lajos@ti.com>
> >> Signed-off-by: Archit Taneja<archit@ti.com>
> >> ---
> >>   drivers/video/omap2/dss/dispc.c |   16 +++++++++++-----
> >>   1 files changed, 11 insertions(+), 5 deletions(-)
> >
> > This only handles omap_dispc_wait_for_irq_interruptible_timeout(),
> > there's also omap_dispc_wait_for_irq_timeout().
> >
> > However, I think it'd be better to do the runtime_get/put in the caller,
> > instead of in these dispc's wait funcs. While it doesn't really matter
> > with dss_mgr_wait_for_vsync(), for dss_mgr/ovl_wait_for_go() it makes
> > much more sense to get/put there just once, instead of every time the
> > omap_dispc_wait_* is called.
> 
> Right, that makes sense. Btw, in the current code, how do we ensure that 
> clocks are enabled when someone calls omap_dss_mgr_apply().

We don't. Apply does not touch any of the registers if the corresponding
manager is not enabled, so there's no need to enable clocks.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 0/3] OMAPDSS: Miscellaneous DISPC Fixes

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 696 bytes --]

On Tue, 2012-02-21 at 19:36 +0530, Archit Taneja wrote:
> These are some minor Display Controller fixes.
> 
> Tested on 4430sdp and 3430sdp on the tree:
> 
> git://gitorious.org/linux-omap-dss2/linux.git dev
> 
> Lajos Molnar (3):
>   OMAPDSS: DISPC: Fix OMAP4 supported color formats
>   OMAPDSS: DISPC: Fix FIR coefficients
>   OMAPDSS: DSS: Add runtime_pm protection around wait_for_vsync.
> 
>  drivers/video/omap2/dss/dispc.c        |   24 +++++++++++++++---------
>  drivers/video/omap2/dss/dispc_coefs.c  |    9 ++++-----
>  drivers/video/omap2/dss/dss_features.c |    3 ++-
>  3 files changed, 21 insertions(+), 15 deletions(-)

Thanks, I've applied these.

 Tomi



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] OMAPDSS: DSS: Add runtime_pm protection around wait_for_vsync.

[Archit Taneja]

On Wednesday 22 February 2012 12:19 PM, Tomi Valkeinen wrote:
> On Wed, 2012-02-22 at 11:15 +0530, Archit Taneja wrote:
>> On Tuesday 21 February 2012 09:38 PM, Tomi Valkeinen wrote:
>>> On Tue, 2012-02-21 at 19:36 +0530, Archit Taneja wrote:
>>>> From: Lajos Molnar<lajos@ti.com>
>>>>
>>>> If DSS is suspended during a wait_for_vsync operation, it may loose its clock.
>>>> Request runtime_pm around wait_for_vsync.
>>>>
>>>> Signed-off-by: Lajos Molnar<lajos@ti.com>
>>>> Signed-off-by: Archit Taneja<archit@ti.com>
>>>> ---
>>>>    drivers/video/omap2/dss/dispc.c |   16 +++++++++++-----
>>>>    1 files changed, 11 insertions(+), 5 deletions(-)
>>>
>>> This only handles omap_dispc_wait_for_irq_interruptible_timeout(),
>>> there's also omap_dispc_wait_for_irq_timeout().
>>>
>>> However, I think it'd be better to do the runtime_get/put in the caller,
>>> instead of in these dispc's wait funcs. While it doesn't really matter
>>> with dss_mgr_wait_for_vsync(), for dss_mgr/ovl_wait_for_go() it makes
>>> much more sense to get/put there just once, instead of every time the
>>> omap_dispc_wait_* is called.
>>
>> Right, that makes sense. Btw, in the current code, how do we ensure that
>> clocks are enabled when someone calls omap_dss_mgr_apply().
>
> We don't. Apply does not touch any of the registers if the corresponding
> manager is not enabled, so there's no need to enable clocks.

Okay, so if a manager is disabled, we won't write to the registers, but 
still update the private data of the manager and connected overlays, and 
these will be later written to the registers once the manager gets 
enabled. Makes sense.

In the older apply, we used to always enable clocks, even if we wrote to 
registers or not. So I got mixed up with that.

Thanks,
Archit

>
>   Tomi
>