RE: [PATCH v3 2/2] OMAP: DSS2: Use dss_features framework on DSS2 code

2010-09-15 Thread Tomi Valkeinen 
On Wed, 2010-09-15 at 13:34 +0200, ext Taneja, Archit wrote:
> Hi,
> 
> I was reworking some patches, I saw 2 functions in manager.c:
> 
> dss_mgr_wait_for_go() and dss_mgr_wait_for_go_ovl() both
> define the var "enum omap_channel channel;" but don't use it.
> 
> Is there any reason this is done, or is it just stray code?

Looks like leftovers from some earlier version...

 Tomi


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v3 2/2] OMAP: DSS2: Use dss_features framework on DSS2 code

2010-09-15 Thread Taneja, Archit 
Hi,

I was reworking some patches, I saw 2 functions in manager.c:

dss_mgr_wait_for_go() and dss_mgr_wait_for_go_ovl() both
define the var "enum omap_channel channel;" but don't use it.

Is there any reason this is done, or is it just stray code?

Tomi Valkeinen wrote:
> On Mon, 2010-09-13 at 07:30 +0200, ext Archit Taneja wrote:
>> Calls init functions of dss_features during dss_probe, and the
>> following features are made omap independent:
>>   - number of managers, overlays
>>   - supported color modes for each overlay
>>   - supported displays for each manager
>>   - global aplha, and restriction of global alpha for video1 pipeline
>>   - The register field ranges : FIRHINC, FIRVINC, FIFOHIGHTHRESHOLD
>> FIFOLOWTHRESHOLD and FIFOSIZE
>> 
>> Signed-off-by: Archit Taneja 
>> ---
>>  arch/arm/plat-omap/include/plat/display.h |   31 
>>  drivers/video/omap2/dss/core.c|3 ++
>>  drivers/video/omap2/dss/dispc.c   |   56
>>  - drivers/video/omap2/dss/manager.c |  
>>  33 - drivers/video/omap2/dss/overlay.c |   24
>>  +--- 5 files changed, 60 insertions(+), 87 deletions(-)
> 
> snip
> 
>>  static void _dispc_setup_global_alpha(enum omap_plane plane, u8
>> global_alpha)  { -
>> -   BUG_ON(plane == OMAP_DSS_VIDEO1);
>> -
>> -   if (cpu_is_omap24xx())
>> +   if (!dss_has_feature(FEAT_GLOBAL_ALPHA))
>> return;
>> 
>> +   BUG_ON(!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1) &&
>> +   plane == OMAP_DSS_VIDEO1);
>> +
>> if (plane == OMAP_DSS_GFX)
>> REG_FLD_MOD(DISPC_GLOBAL_ALPHA, global_alpha, 7, 0);
>> else if (plane == OMAP_DSS_VIDEO2) @@ -949,17 +950,14 @@
>> static void dispc_read_plane_fifo_sizes(void)
>> 
> DISPC_VID_FIFO_SIZE_STATUS(1) };
>> u32 size;
>> int plane;
>> +   u8 size1, size2;
> 
> You have a bit inconsistent naming with these. The function
> is defined as accepting start and end, here you use size1 and size2, below
> you have foo1 and foo2.
> 
> Size is not a good name, as it's not size at all. Perhaps
> foo_start and foo_end (or just start and end if there's only
> one feat being used)? Or foo_msb and foo_lsb (as in most/least significant
> bit)? 
> 
> Otherwise looks good.
> 
>  Tomi--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v3 2/2] OMAP: DSS2: Use dss_features framework on DSS2 code

2010-09-15 Thread Taneja, Archit 
Tomi Valkeinen wrote:
> On Mon, 2010-09-13 at 07:30 +0200, ext Archit Taneja wrote:
>> Calls init functions of dss_features during dss_probe, and the
>> following features are made omap independent:
>>   - number of managers, overlays
>>   - supported color modes for each overlay
>>   - supported displays for each manager
>>   - global aplha, and restriction of global alpha for video1 pipeline
>>   - The register field ranges : FIRHINC, FIRVINC, FIFOHIGHTHRESHOLD
>> FIFOLOWTHRESHOLD and FIFOSIZE
>> 
>> Signed-off-by: Archit Taneja 
>> ---
>>  arch/arm/plat-omap/include/plat/display.h |   31 
>>  drivers/video/omap2/dss/core.c|3 ++
>>  drivers/video/omap2/dss/dispc.c   |   56
>>  - drivers/video/omap2/dss/manager.c |  
>>  33 - drivers/video/omap2/dss/overlay.c |   24
>>  +--- 5 files changed, 60 insertions(+), 87 deletions(-)
> 
> snip
> 
>>  static void _dispc_setup_global_alpha(enum omap_plane plane, u8
>> global_alpha)  { -
>> -   BUG_ON(plane == OMAP_DSS_VIDEO1);
>> -
>> -   if (cpu_is_omap24xx())
>> +   if (!dss_has_feature(FEAT_GLOBAL_ALPHA))
>> return;
>> 
>> +   BUG_ON(!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1) &&
>> +   plane == OMAP_DSS_VIDEO1);
>> +
>> if (plane == OMAP_DSS_GFX)
>> REG_FLD_MOD(DISPC_GLOBAL_ALPHA, global_alpha, 7, 0);
>> else if (plane == OMAP_DSS_VIDEO2) @@ -949,17 +950,14 @@
>> static void dispc_read_plane_fifo_sizes(void)
>> 
> DISPC_VID_FIFO_SIZE_STATUS(1) };
>> u32 size;
>> int plane;
>> +   u8 size1, size2;
> 
> You have a bit inconsistent naming with these. The function
> is defined as accepting start and end, here you use size1 and size2, below
> you have foo1 and foo2.
> 

I think I gave them names based on the name of the reg_field (size for FIFOSIZE,
hinc for FIRHINC etc), 1 and 2 denote the start and end respectively, but yes, I
guess it doesn't explain things that clearly, I'll give more appropriate names.

> Size is not a good name, as it's not size at all. Perhaps
> foo_start and foo_end (or just start and end if there's only
> one feat being used)? Or foo_msb and foo_lsb (as in most/least significant
> bit)? 
> 
> Otherwise looks good.
> 
>  Tomi--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/2] OMAP: DSS2: Use dss_features framework on DSS2 code

2010-09-15 Thread Tomi Valkeinen 
On Mon, 2010-09-13 at 07:30 +0200, ext Archit Taneja wrote:
> Calls init functions of dss_features during dss_probe, and the following
> features are made omap independent:
>   - number of managers, overlays
>   - supported color modes for each overlay
>   - supported displays for each manager
>   - global aplha, and restriction of global alpha for video1 pipeline
>   - The register field ranges : FIRHINC, FIRVINC, FIFOHIGHTHRESHOLD
> FIFOLOWTHRESHOLD and FIFOSIZE
> 
> Signed-off-by: Archit Taneja 
> ---
>  arch/arm/plat-omap/include/plat/display.h |   31 
>  drivers/video/omap2/dss/core.c|3 ++
>  drivers/video/omap2/dss/dispc.c   |   56 
> -
>  drivers/video/omap2/dss/manager.c |   33 -
>  drivers/video/omap2/dss/overlay.c |   24 +---
>  5 files changed, 60 insertions(+), 87 deletions(-)

snip

>  static void _dispc_setup_global_alpha(enum omap_plane plane, u8 global_alpha)
>  {
> -
> -   BUG_ON(plane == OMAP_DSS_VIDEO1);
> -
> -   if (cpu_is_omap24xx())
> +   if (!dss_has_feature(FEAT_GLOBAL_ALPHA))
> return;
> 
> +   BUG_ON(!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1) &&
> +   plane == OMAP_DSS_VIDEO1);
> +
> if (plane == OMAP_DSS_GFX)
> REG_FLD_MOD(DISPC_GLOBAL_ALPHA, global_alpha, 7, 0);
> else if (plane == OMAP_DSS_VIDEO2)
> @@ -949,17 +950,14 @@ static void dispc_read_plane_fifo_sizes(void)
>   DISPC_VID_FIFO_SIZE_STATUS(1) };
> u32 size;
> int plane;
> +   u8 size1, size2;

You have a bit inconsistent naming with these. The function is defined
as accepting start and end, here you use size1 and size2, below you have
foo1 and foo2.

Size is not a good name, as it's not size at all. Perhaps foo_start and
foo_end (or just start and end if there's only one feat being used)? Or
foo_msb and foo_lsb (as in most/least significant bit)?

Otherwise looks good.

 Tomi


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/2] OMAP: DSS2: Use dss_features framework on DSS2 code

2010-09-12 Thread Archit Taneja 
Calls init functions of dss_features during dss_probe, and the following
features are made omap independent:
  - number of managers, overlays
  - supported color modes for each overlay
  - supported displays for each manager
  - global aplha, and restriction of global alpha for video1 pipeline
  - The register field ranges : FIRHINC, FIRVINC, FIFOHIGHTHRESHOLD
FIFOLOWTHRESHOLD and FIFOSIZE

Signed-off-by: Archit Taneja 
---
 arch/arm/plat-omap/include/plat/display.h |   31 
 drivers/video/omap2/dss/core.c|3 ++
 drivers/video/omap2/dss/dispc.c   |   56 -
 drivers/video/omap2/dss/manager.c |   33 -
 drivers/video/omap2/dss/overlay.c |   24 +---
 5 files changed, 60 insertions(+), 87 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/display.h 
b/arch/arm/plat-omap/include/plat/display.h
index 8bd15bd..c915a66 100644
--- a/arch/arm/plat-omap/include/plat/display.h
+++ b/arch/arm/plat-omap/include/plat/display.h
@@ -81,37 +81,6 @@ enum omap_color_mode {
OMAP_DSS_COLOR_ARGB32   = 1 << 11, /* ARGB32 */
OMAP_DSS_COLOR_RGBA32   = 1 << 12, /* RGBA32 */
OMAP_DSS_COLOR_RGBX32   = 1 << 13, /* RGBx32 */
-
-   OMAP_DSS_COLOR_GFX_OMAP2 =
-   OMAP_DSS_COLOR_CLUT1 | OMAP_DSS_COLOR_CLUT2 |
-   OMAP_DSS_COLOR_CLUT4 | OMAP_DSS_COLOR_CLUT8 |
-   OMAP_DSS_COLOR_RGB12U | OMAP_DSS_COLOR_RGB16 |
-   OMAP_DSS_COLOR_RGB24U | OMAP_DSS_COLOR_RGB24P,
-
-   OMAP_DSS_COLOR_VID_OMAP2 =
-   OMAP_DSS_COLOR_RGB16 | OMAP_DSS_COLOR_RGB24U |
-   OMAP_DSS_COLOR_RGB24P | OMAP_DSS_COLOR_YUV2 |
-   OMAP_DSS_COLOR_UYVY,
-
-   OMAP_DSS_COLOR_GFX_OMAP3 =
-   OMAP_DSS_COLOR_CLUT1 | OMAP_DSS_COLOR_CLUT2 |
-   OMAP_DSS_COLOR_CLUT4 | OMAP_DSS_COLOR_CLUT8 |
-   OMAP_DSS_COLOR_RGB12U | OMAP_DSS_COLOR_ARGB16 |
-   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_VID1_OMAP3 =
-   OMAP_DSS_COLOR_RGB12U | OMAP_DSS_COLOR_RGB16 |
-   OMAP_DSS_COLOR_RGB24U | OMAP_DSS_COLOR_RGB24P |
-   OMAP_DSS_COLOR_YUV2 | OMAP_DSS_COLOR_UYVY,
-
-   OMAP_DSS_COLOR_VID2_OMAP3 =
-   OMAP_DSS_COLOR_RGB12U | OMAP_DSS_COLOR_ARGB16 |
-   OMAP_DSS_COLOR_RGB16 | OMAP_DSS_COLOR_RGB24U |
-   OMAP_DSS_COLOR_RGB24P | OMAP_DSS_COLOR_YUV2 |
-   OMAP_DSS_COLOR_UYVY | OMAP_DSS_COLOR_ARGB32 |
-   OMAP_DSS_COLOR_RGBA32 | OMAP_DSS_COLOR_RGBX32,
 };
 
 enum omap_lcd_display_type {
diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index b3a498f..8e89f60 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -37,6 +37,7 @@
 #include 
 
 #include "dss.h"
+#include "dss_features.h"
 
 static struct {
struct platform_device *pdev;
@@ -502,6 +503,8 @@ static int omap_dss_probe(struct platform_device *pdev)
 
core.pdev = pdev;
 
+   dss_features_init();
+
dss_init_overlay_managers(pdev);
dss_init_overlays(pdev);
 
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 90eb110..68f86dc
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -39,6 +39,7 @@
 #include 
 
 #include "dss.h"
+#include "dss_features.h"
 
 /* DISPC */
 #define DISPC_BASE 0x48050400
@@ -774,12 +775,12 @@ static void _dispc_set_vid_size(enum omap_plane plane, 
int width, int height)
 
 static void _dispc_setup_global_alpha(enum omap_plane plane, u8 global_alpha)
 {
-
-   BUG_ON(plane == OMAP_DSS_VIDEO1);
-
-   if (cpu_is_omap24xx())
+   if (!dss_has_feature(FEAT_GLOBAL_ALPHA))
return;
 
+   BUG_ON(!dss_has_feature(FEAT_GLOBAL_ALPHA_VID1) &&
+   plane == OMAP_DSS_VIDEO1);
+
if (plane == OMAP_DSS_GFX)
REG_FLD_MOD(DISPC_GLOBAL_ALPHA, global_alpha, 7, 0);
else if (plane == OMAP_DSS_VIDEO2)
@@ -949,17 +950,14 @@ static void dispc_read_plane_fifo_sizes(void)
  DISPC_VID_FIFO_SIZE_STATUS(1) };
u32 size;
int plane;
+   u8 size1, size2;
 
enable_clocks(1);
 
-   for (plane = 0; plane < ARRAY_SIZE(dispc.fifo_size); ++plane) {
-   if (cpu_is_omap24xx())
-   size = FLD_GET(dispc_read_reg(fsz_reg[plane]), 8, 0);
-   else if (cpu_is_omap34xx())
-   size = FLD_GET(dispc_read_reg(fsz_reg[plane]), 10, 0);
-   else
-   BUG();
+   dss_feat_get_reg_field(FEAT_REG_FIFOSIZE, &size1, &size2);
 
+   for (plane = 0; plane < ARRAY_SIZE(dispc.fifo_size); ++plane) {
+   size = FLD_GET(dispc_read_reg(fsz_r