Re: [PATCH 24/25] OMAPDSS: DSI: improve DSI module id handling

2012-05-04 Thread Archit Taneja

On Thursday 03 May 2012 07:28 PM, Tomi Valkeinen wrote:

We currently use the id of the dsi platform device (dsidev-id) as the
DSI hardware module ID. This works because we assign the ID manually in
arch/arm/mach-omap2/display.c at boot time.

However, with device tree the platform device IDs are automatically
assigned to an arbitrary number, and we can't use it.


If this number is arbitrary we would need to change the dsi_pdev_map 
approach of mapping a dsi module and it's corresponding platform device.

Currently dsi_pdev_map is:

static struct platform_device *dsi_pdev_map[MAX_NUM_DSI];

So we either need to increase the array size to take larger arbitrary 
numbers, or do something else.


We would also need to fix the usage of dsi_get_dsidev_from_id(), as 
right now we manually pass 0 and 1 to it only, for example:


static void dsi1_dump_irqs(struct seq_file *s)
{
struct platform_device *dsidev = dsi_get_dsidev_from_id(0);

dsi_dump_dsidev_irqs(dsidev, s);
}

The immediate solution that comes to mind is to maintain 2 id's, one 
which is sequential, and the other which the DT has created, and keep an 
array to map these. But this seems messy!


Archit



Instead of using dsidev-id during operation, this patch stores the
value of dsidev-id to a private field of the dsi driver at probe(). The
future device tree code can thus set the private field with some other
way.

Signed-off-by: Tomi Valkeinentomi.valkei...@ti.com
---
  drivers/video/omap2/dss/dsi.c |   46 +++--
  1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 6cc92a8..ce964dd 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -256,6 +256,8 @@ struct dsi_data {
struct platform_device *pdev;
void __iomem*base;

+   int module_id;
+
int irq;

struct clk *dss_clk;
@@ -358,11 +360,6 @@ struct platform_device *dsi_get_dsidev_from_id(int module)
return dsi_pdev_map[module];
  }

-static inline int dsi_get_dsidev_id(struct platform_device *dsidev)
-{
-   return dsidev-id;
-}
-
  static inline void dsi_write_reg(struct platform_device *dsidev,
const struct dsi_reg idx, u32 val)
  {
@@ -1181,10 +1178,9 @@ static unsigned long dsi_get_txbyteclkhs(struct 
platform_device *dsidev)
  static unsigned long dsi_fclk_rate(struct platform_device *dsidev)
  {
unsigned long r;
-   int dsi_module = dsi_get_dsidev_id(dsidev);
struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);

-   if (dss_get_dsi_clk_source(dsi_module) == OMAP_DSS_CLK_SRC_FCK) {
+   if (dss_get_dsi_clk_source(dsi-module_id) == OMAP_DSS_CLK_SRC_FCK) {
/* DSI FCLK source is DSS_CLK_FCK */
r = clk_get_rate(dsi-dss_clk);
} else {
@@ -1683,7 +1679,7 @@ static void dsi_dump_dsidev_clocks(struct platform_device 
*dsidev,
struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
struct dsi_clock_info *cinfo =dsi-current_cinfo;
enum omap_dss_clk_source dispc_clk_src, dsi_clk_src;
-   int dsi_module = dsi_get_dsidev_id(dsidev);
+   int dsi_module = dsi-module_id;

dispc_clk_src = dss_get_dispc_clk_source();
dsi_clk_src = dss_get_dsi_clk_source(dsi_module);
@@ -1755,7 +1751,6 @@ static void dsi_dump_dsidev_irqs(struct platform_device 
*dsidev,
struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
unsigned long flags;
struct dsi_irq_stats stats;
-   int dsi_module = dsi_get_dsidev_id(dsidev);

spin_lock_irqsave(dsi-irq_stats_lock, flags);

@@ -1772,7 +1767,7 @@ static void dsi_dump_dsidev_irqs(struct platform_device 
*dsidev,
  #define PIS(x) \
seq_printf(s, %-20s %10d\n, #x, stats.dsi_irqs[ffs(DSI_IRQ_##x)-1]);

-   seq_printf(s, -- DSI%d interrupts --\n, dsi_module + 1);
+   seq_printf(s, -- DSI%d interrupts --\n, dsi-module_id + 1);
PIS(VC0);
PIS(VC1);
PIS(VC2);
@@ -2272,7 +2267,7 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)

DSSDBGF();

-   r = dss_dsi_enable_pads(dsi_get_dsidev_id(dsidev), 
dsi_get_lane_mask(dssdev));
+   r = dss_dsi_enable_pads(dsi-module_id, dsi_get_lane_mask(dssdev));
if (r)
return r;

@@ -2382,20 +2377,21 @@ err_cio_pwr:
dsi_cio_disable_lane_override(dsidev);
  err_scp_clk_dom:
dsi_disable_scp_clk(dsidev);
-   dss_dsi_disable_pads(dsi_get_dsidev_id(dsidev), 
dsi_get_lane_mask(dssdev));
+   dss_dsi_disable_pads(dsi-module_id, dsi_get_lane_mask(dssdev));
return r;
  }

  static void dsi_cio_uninit(struct omap_dss_device *dssdev)
  {
struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+   struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);

/* DDR_CLK_ALWAYS_ON */
REG_FLD_MOD(dsidev, DSI_CLK_CTRL, 0, 13, 13);

dsi_cio_power(dsidev, 

Re: [PATCH 24/25] OMAPDSS: DSI: improve DSI module id handling

2012-05-04 Thread Tomi Valkeinen
On Fri, 2012-05-04 at 14:39 +0530, Archit Taneja wrote:
 On Thursday 03 May 2012 07:28 PM, Tomi Valkeinen wrote:
  We currently use the id of the dsi platform device (dsidev-id) as the
  DSI hardware module ID. This works because we assign the ID manually in
  arch/arm/mach-omap2/display.c at boot time.
 
  However, with device tree the platform device IDs are automatically
  assigned to an arbitrary number, and we can't use it.
 
 If this number is arbitrary we would need to change the dsi_pdev_map 
 approach of mapping a dsi module and it's corresponding platform device.
 Currently dsi_pdev_map is:
 
 static struct platform_device *dsi_pdev_map[MAX_NUM_DSI];
 
 So we either need to increase the array size to take larger arbitrary 
 numbers, or do something else.
 
 We would also need to fix the usage of dsi_get_dsidev_from_id(), as 
 right now we manually pass 0 and 1 to it only, for example:
 
 static void dsi1_dump_irqs(struct seq_file *s)
 {
  struct platform_device *dsidev = dsi_get_dsidev_from_id(0);
 
  dsi_dump_dsidev_irqs(dsidev, s);
 }
 
 The immediate solution that comes to mind is to maintain 2 id's, one 
 which is sequential, and the other which the DT has created, and keep an 
 array to map these. But this seems messy!

This is only a problem with device tree, and I solved it so that I pass
a DSI module ID in the device tree data. So, with old pdata way I
initialize dsi-module_id from the pdev-id, but with DT I initialize
dsi-module_id from the DT data.

So basically we remove the use of pdev-id in this patch, and add
dsi-module_id field, which needs to be initialized to 0 or 1, depending
on the corresponding HW module. We just happen to use the pdev-id to
initialize it when using the old pdata method, as we know it tells the
right id. But we could initialize it from any other source.

This allows us to keep the 0 and 1 DSI IDs, and I think we need those
anyway. Some parts of the code could work fine with arbitrary ID, as
long as a pdev can be linked to/from this ID. However, there are things
where we must have the ID, like configuring the clock source settings in
dss_core, where we set a certain bit for DSI module 0, and certain bit
for module 1.

Perhaps even those could be handled without explicit ID of 0 or 1, but
that doesn't sound trivial and I didn't want to start tackling that in
this series.

I wish there was a way to get the module ID from the HW registers
somehow. Then we wouldn't need to pass the ID via SW, which doesn't feel
very correct. At least with DT it's a bit wrong, in my opinion, but best
I could come up with.

 Tomi



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


Re: [PATCH 24/25] OMAPDSS: DSI: improve DSI module id handling

2012-05-04 Thread Archit Taneja

On Friday 04 May 2012 03:23 PM, Tomi Valkeinen wrote:

On Fri, 2012-05-04 at 14:39 +0530, Archit Taneja wrote:

On Thursday 03 May 2012 07:28 PM, Tomi Valkeinen wrote:

We currently use the id of the dsi platform device (dsidev-id) as the
DSI hardware module ID. This works because we assign the ID manually in
arch/arm/mach-omap2/display.c at boot time.

However, with device tree the platform device IDs are automatically
assigned to an arbitrary number, and we can't use it.


If this number is arbitrary we would need to change the dsi_pdev_map
approach of mapping a dsi module and it's corresponding platform device.
Currently dsi_pdev_map is:

static struct platform_device *dsi_pdev_map[MAX_NUM_DSI];

So we either need to increase the array size to take larger arbitrary
numbers, or do something else.

We would also need to fix the usage of dsi_get_dsidev_from_id(), as
right now we manually pass 0 and 1 to it only, for example:

static void dsi1_dump_irqs(struct seq_file *s)
{
  struct platform_device *dsidev = dsi_get_dsidev_from_id(0);

  dsi_dump_dsidev_irqs(dsidev, s);
}

The immediate solution that comes to mind is to maintain 2 id's, one
which is sequential, and the other which the DT has created, and keep an
array to map these. But this seems messy!


This is only a problem with device tree, and I solved it so that I pass
a DSI module ID in the device tree data. So, with old pdata way I
initialize dsi-module_id from the pdev-id, but with DT I initialize
dsi-module_id from the DT data.


Oh ok, so the code which decides how dsi-module_id is initialised(from 
DT or pdata) is not in this series right? And it would come later? Right 
now it's just set to dsidev-id in probe.




So basically we remove the use of pdev-id in this patch, and add
dsi-module_id field, which needs to be initialized to 0 or 1, depending
on the corresponding HW module. We just happen to use the pdev-id to
initialize it when using the old pdata method, as we know it tells the
right id. But we could initialize it from any other source.


Right, I get it now.



This allows us to keep the 0 and 1 DSI IDs, and I think we need those
anyway. Some parts of the code could work fine with arbitrary ID, as
long as a pdev can be linked to/from this ID. However, there are things
where we must have the ID, like configuring the clock source settings in
dss_core, where we set a certain bit for DSI module 0, and certain bit
for module 1.

Perhaps even those could be handled without explicit ID of 0 or 1, but
that doesn't sound trivial and I didn't want to start tackling that in
this series.

I wish there was a way to get the module ID from the HW registers
somehow. Then we wouldn't need to pass the ID via SW, which doesn't feel
very correct. At least with DT it's a bit wrong, in my opinion, but best
I could come up with.


We could derive it via a parameter like number of lanes or something 
similar through DSI_GNQ, but that doesn't seem very nice, and may not be 
usable on future OMAPs.


Archit



  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 24/25] OMAPDSS: DSI: improve DSI module id handling

2012-05-04 Thread Tomi Valkeinen
On Fri, 2012-05-04 at 15:35 +0530, Archit Taneja wrote:
  This is only a problem with device tree, and I solved it so that I
 pass
  a DSI module ID in the device tree data. So, with old pdata way I
  initialize dsi-module_id from the pdev-id, but with DT I
 initialize
  dsi-module_id from the DT data.
 
 Oh ok, so the code which decides how dsi-module_id is
 initialised(from 
 DT or pdata) is not in this series right? And it would come later?
 Right 
 now it's just set to dsidev-id in probe.
 
Yes, there's no DT code in this series, only cleanups to make adding DT
support easier.

 Tomi



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


[PATCH 24/25] OMAPDSS: DSI: improve DSI module id handling

2012-05-03 Thread Tomi Valkeinen
We currently use the id of the dsi platform device (dsidev-id) as the
DSI hardware module ID. This works because we assign the ID manually in
arch/arm/mach-omap2/display.c at boot time.

However, with device tree the platform device IDs are automatically
assigned to an arbitrary number, and we can't use it.

Instead of using dsidev-id during operation, this patch stores the
value of dsidev-id to a private field of the dsi driver at probe(). The
future device tree code can thus set the private field with some other
way.

Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
---
 drivers/video/omap2/dss/dsi.c |   46 +++--
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 6cc92a8..ce964dd 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -256,6 +256,8 @@ struct dsi_data {
struct platform_device *pdev;
void __iomem*base;
 
+   int module_id;
+
int irq;
 
struct clk *dss_clk;
@@ -358,11 +360,6 @@ struct platform_device *dsi_get_dsidev_from_id(int module)
return dsi_pdev_map[module];
 }
 
-static inline int dsi_get_dsidev_id(struct platform_device *dsidev)
-{
-   return dsidev-id;
-}
-
 static inline void dsi_write_reg(struct platform_device *dsidev,
const struct dsi_reg idx, u32 val)
 {
@@ -1181,10 +1178,9 @@ static unsigned long dsi_get_txbyteclkhs(struct 
platform_device *dsidev)
 static unsigned long dsi_fclk_rate(struct platform_device *dsidev)
 {
unsigned long r;
-   int dsi_module = dsi_get_dsidev_id(dsidev);
struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 
-   if (dss_get_dsi_clk_source(dsi_module) == OMAP_DSS_CLK_SRC_FCK) {
+   if (dss_get_dsi_clk_source(dsi-module_id) == OMAP_DSS_CLK_SRC_FCK) {
/* DSI FCLK source is DSS_CLK_FCK */
r = clk_get_rate(dsi-dss_clk);
} else {
@@ -1683,7 +1679,7 @@ static void dsi_dump_dsidev_clocks(struct platform_device 
*dsidev,
struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
struct dsi_clock_info *cinfo = dsi-current_cinfo;
enum omap_dss_clk_source dispc_clk_src, dsi_clk_src;
-   int dsi_module = dsi_get_dsidev_id(dsidev);
+   int dsi_module = dsi-module_id;
 
dispc_clk_src = dss_get_dispc_clk_source();
dsi_clk_src = dss_get_dsi_clk_source(dsi_module);
@@ -1755,7 +1751,6 @@ static void dsi_dump_dsidev_irqs(struct platform_device 
*dsidev,
struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
unsigned long flags;
struct dsi_irq_stats stats;
-   int dsi_module = dsi_get_dsidev_id(dsidev);
 
spin_lock_irqsave(dsi-irq_stats_lock, flags);
 
@@ -1772,7 +1767,7 @@ static void dsi_dump_dsidev_irqs(struct platform_device 
*dsidev,
 #define PIS(x) \
seq_printf(s, %-20s %10d\n, #x, stats.dsi_irqs[ffs(DSI_IRQ_##x)-1]);
 
-   seq_printf(s, -- DSI%d interrupts --\n, dsi_module + 1);
+   seq_printf(s, -- DSI%d interrupts --\n, dsi-module_id + 1);
PIS(VC0);
PIS(VC1);
PIS(VC2);
@@ -2272,7 +2267,7 @@ static int dsi_cio_init(struct omap_dss_device *dssdev)
 
DSSDBGF();
 
-   r = dss_dsi_enable_pads(dsi_get_dsidev_id(dsidev), 
dsi_get_lane_mask(dssdev));
+   r = dss_dsi_enable_pads(dsi-module_id, dsi_get_lane_mask(dssdev));
if (r)
return r;
 
@@ -2382,20 +2377,21 @@ err_cio_pwr:
dsi_cio_disable_lane_override(dsidev);
 err_scp_clk_dom:
dsi_disable_scp_clk(dsidev);
-   dss_dsi_disable_pads(dsi_get_dsidev_id(dsidev), 
dsi_get_lane_mask(dssdev));
+   dss_dsi_disable_pads(dsi-module_id, dsi_get_lane_mask(dssdev));
return r;
 }
 
 static void dsi_cio_uninit(struct omap_dss_device *dssdev)
 {
struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
+   struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
 
/* DDR_CLK_ALWAYS_ON */
REG_FLD_MOD(dsidev, DSI_CLK_CTRL, 0, 13, 13);
 
dsi_cio_power(dsidev, DSI_COMPLEXIO_POWER_OFF);
dsi_disable_scp_clk(dsidev);
-   dss_dsi_disable_pads(dsi_get_dsidev_id(dsidev), 
dsi_get_lane_mask(dssdev));
+   dss_dsi_disable_pads(dsi-module_id, dsi_get_lane_mask(dssdev));
 }
 
 static void dsi_config_tx_fifo(struct platform_device *dsidev,
@@ -4277,7 +4273,7 @@ static int dsi_configure_dispc_clocks(struct 
omap_dss_device *dssdev)
 static int dsi_display_init_dsi(struct omap_dss_device *dssdev)
 {
struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
-   int dsi_module = dsi_get_dsidev_id(dsidev);
+   struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev);
int r;
 
r = dsi_pll_init(dsidev, true, true);
@@ -4289,7 +4285,7 @@ static int dsi_display_init_dsi(struct omap_dss_device 
*dssdev)
goto err1;