Re: [PATCH v2 10/18] OMAPDSS: DISPC: Configure input and output sizes for writeback

2012-09-26 Thread Archit Taneja

On Tuesday 25 September 2012 08:03 PM, Tomi Valkeinen wrote:

On Tue, 2012-09-25 at 11:49 +0530, Archit Taneja wrote:

Writeback uses the WB_PICTURE_SIZE register to define the size of the content
written to memory, this is the output of the scaler. It uses the WB_SIZE
register to define the size of the content coming from the overlay/manager to
which it is connected, this is the input to the scaler. This naming is different
as compared to overlays.

Add checks for writeback in dispc_ovl_set_input_size() and
dispc_ovl_set_output_size() to write to the correct registers.

Signed-off-by: Archit Taneja arc...@ti.com
---
  drivers/video/omap2/dss/dispc.c |7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index e42e902..04fdd33 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -719,7 +719,7 @@ static void dispc_ovl_set_input_size(enum omap_plane plane, 
int width,
  {
u32 val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);

-   if (plane == OMAP_DSS_GFX)
+   if (plane == OMAP_DSS_GFX || plane == OMAP_DSS_WB)
dispc_write_reg(DISPC_OVL_SIZE(plane), val);
else
dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
@@ -734,7 +734,10 @@ static void dispc_ovl_set_output_size(enum omap_plane 
plane, int width,

val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);

-   dispc_write_reg(DISPC_OVL_SIZE(plane), val);
+   if (plane == OMAP_DSS_WB)
+   dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
+   else
+   dispc_write_reg(DISPC_OVL_SIZE(plane), val);
  }


Should we just rename the dispc registers to DISPC_OVL_IN_SIZE and
DISPC_OVL_OUT_SIZE, and then we could do without the ifs? The registers
have always confused me a bit, I don't know why they are named so in the
TRM.


It'll be hard for someone who's referring to the TRM in that case. It'll 
also be hard to track reg dumps if these names change. I think we should 
stick to the TRM names only.


Archit

--
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 v2 10/18] OMAPDSS: DISPC: Configure input and output sizes for writeback

2012-09-26 Thread Tomi Valkeinen
On Wed, 2012-09-26 at 11:52 +0530, Archit Taneja wrote:
 On Tuesday 25 September 2012 08:03 PM, Tomi Valkeinen wrote:
  On Tue, 2012-09-25 at 11:49 +0530, Archit Taneja wrote:
  Writeback uses the WB_PICTURE_SIZE register to define the size of the 
  content
  written to memory, this is the output of the scaler. It uses the WB_SIZE
  register to define the size of the content coming from the overlay/manager 
  to
  which it is connected, this is the input to the scaler. This naming is 
  different
  as compared to overlays.
 
  Add checks for writeback in dispc_ovl_set_input_size() and
  dispc_ovl_set_output_size() to write to the correct registers.
 
  Signed-off-by: Archit Taneja arc...@ti.com
  ---
drivers/video/omap2/dss/dispc.c |7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
 
  diff --git a/drivers/video/omap2/dss/dispc.c 
  b/drivers/video/omap2/dss/dispc.c
  index e42e902..04fdd33 100644
  --- a/drivers/video/omap2/dss/dispc.c
  +++ b/drivers/video/omap2/dss/dispc.c
  @@ -719,7 +719,7 @@ static void dispc_ovl_set_input_size(enum omap_plane 
  plane, int width,
{
 u32 val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
 
  -  if (plane == OMAP_DSS_GFX)
  +  if (plane == OMAP_DSS_GFX || plane == OMAP_DSS_WB)
 dispc_write_reg(DISPC_OVL_SIZE(plane), val);
 else
 dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
  @@ -734,7 +734,10 @@ static void dispc_ovl_set_output_size(enum omap_plane 
  plane, int width,
 
 val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
 
  -  dispc_write_reg(DISPC_OVL_SIZE(plane), val);
  +  if (plane == OMAP_DSS_WB)
  +  dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
  +  else
  +  dispc_write_reg(DISPC_OVL_SIZE(plane), val);
}
 
  Should we just rename the dispc registers to DISPC_OVL_IN_SIZE and
  DISPC_OVL_OUT_SIZE, and then we could do without the ifs? The registers
  have always confused me a bit, I don't know why they are named so in the
  TRM.
 
 It'll be hard for someone who's referring to the TRM in that case. It'll 
 also be hard to track reg dumps if these names change. I think we should 
 stick to the TRM names only.

Generally speaking I agree.

However, with each new OMAP we get register name changes. We already
have at least a few of those, and I think it'll just get worse. So at
some point we'll have a mishmash of register names from different omap
versions, or we'll rename the registers according to one particular omap
version (probably the latest one), or we'll name the registers as seems
best for the driver.

But yes, perhaps it's still best to keep the reg names as they are.
Perhaps there's even a logic with the names, I just can't see it =).

 Tomi



signature.asc
Description: This is a digitally signed message part


[PATCH v2 10/18] OMAPDSS: DISPC: Configure input and output sizes for writeback

2012-09-25 Thread Archit Taneja
Writeback uses the WB_PICTURE_SIZE register to define the size of the content
written to memory, this is the output of the scaler. It uses the WB_SIZE
register to define the size of the content coming from the overlay/manager to
which it is connected, this is the input to the scaler. This naming is different
as compared to overlays.

Add checks for writeback in dispc_ovl_set_input_size() and
dispc_ovl_set_output_size() to write to the correct registers.

Signed-off-by: Archit Taneja arc...@ti.com
---
 drivers/video/omap2/dss/dispc.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index e42e902..04fdd33 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -719,7 +719,7 @@ static void dispc_ovl_set_input_size(enum omap_plane plane, 
int width,
 {
u32 val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
 
-   if (plane == OMAP_DSS_GFX)
+   if (plane == OMAP_DSS_GFX || plane == OMAP_DSS_WB)
dispc_write_reg(DISPC_OVL_SIZE(plane), val);
else
dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
@@ -734,7 +734,10 @@ static void dispc_ovl_set_output_size(enum omap_plane 
plane, int width,
 
val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
 
-   dispc_write_reg(DISPC_OVL_SIZE(plane), val);
+   if (plane == OMAP_DSS_WB)
+   dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
+   else
+   dispc_write_reg(DISPC_OVL_SIZE(plane), val);
 }
 
 static void dispc_ovl_set_zorder(enum omap_plane plane,
-- 
1.7.9.5

--
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 v2 10/18] OMAPDSS: DISPC: Configure input and output sizes for writeback

2012-09-25 Thread Tomi Valkeinen
On Tue, 2012-09-25 at 11:49 +0530, Archit Taneja wrote:
 Writeback uses the WB_PICTURE_SIZE register to define the size of the content
 written to memory, this is the output of the scaler. It uses the WB_SIZE
 register to define the size of the content coming from the overlay/manager to
 which it is connected, this is the input to the scaler. This naming is 
 different
 as compared to overlays.
 
 Add checks for writeback in dispc_ovl_set_input_size() and
 dispc_ovl_set_output_size() to write to the correct registers.
 
 Signed-off-by: Archit Taneja arc...@ti.com
 ---
  drivers/video/omap2/dss/dispc.c |7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
 index e42e902..04fdd33 100644
 --- a/drivers/video/omap2/dss/dispc.c
 +++ b/drivers/video/omap2/dss/dispc.c
 @@ -719,7 +719,7 @@ static void dispc_ovl_set_input_size(enum omap_plane 
 plane, int width,
  {
   u32 val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
  
 - if (plane == OMAP_DSS_GFX)
 + if (plane == OMAP_DSS_GFX || plane == OMAP_DSS_WB)
   dispc_write_reg(DISPC_OVL_SIZE(plane), val);
   else
   dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
 @@ -734,7 +734,10 @@ static void dispc_ovl_set_output_size(enum omap_plane 
 plane, int width,
  
   val = FLD_VAL(height - 1, 26, 16) | FLD_VAL(width - 1, 10, 0);
  
 - dispc_write_reg(DISPC_OVL_SIZE(plane), val);
 + if (plane == OMAP_DSS_WB)
 + dispc_write_reg(DISPC_OVL_PICTURE_SIZE(plane), val);
 + else
 + dispc_write_reg(DISPC_OVL_SIZE(plane), val);
  }

Should we just rename the dispc registers to DISPC_OVL_IN_SIZE and
DISPC_OVL_OUT_SIZE, and then we could do without the ifs? The registers
have always confused me a bit, I don't know why they are named so in the
TRM.

 Tomi



signature.asc
Description: This is a digitally signed message part