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 arc...@ti.com
 ---
  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
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 arc...@ti.com
 ---
  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 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