Skip to contentSkip to conversation

video, ipu: make ldb clock frequenz overwriteable through board code

PATCH New Heiko Schocher 2 participants 4 messages Apr 12, 2015 – Apr 17, 2015 Framebuffer layer

lore
Share & tools
lore

Heiko Schocher

author
#0 lore raw

PATCH 1/1 video, ipu: make ldb clock frequenz overwriteable through board code

the ldb clock can be setup in board code (for example set through PLL5). Update the ldb_clock rate also through board code.

This should be removed, if a clock framework is availiable.

Signed-off-by Heiko Schocher <[email protected]>

---

 drivers/video/ipu.h        |  1 +
 drivers/video/ipu_common.c | 11 +++++++++++
 2 files changed, 12 insertions(+)
drivers/video/ipu.h +1 -0
index 091b58f..f13934f 100644
@@ -266,4 +266,5 @@ void ipu_dp_uninit(ipu_channel_t channel);
 void ipu_dp_dc_disable(ipu_channel_t channel, unsigned char swap);
 ipu_color_space_t format_to_colorspace(uint32_t fmt);
 
+int ipu_set_ldb_clock(int rate);
 #endif
drivers/video/ipu_common.c +11 -0
index 1a209d4..dc054bc 100644
@@ -1198,3 +1198,14 @@ ipu_color_space_t format_to_colorspace(uint32_t fmt)
 	}
 	return RGB;
 }
+
+/* should removed when clk framework is availiable */
+int ipu_set_ldb_clock(int rate)
+{
+	if (g_ldb_clk == NULL)
+		return -ENOENT;
+
+	g_ldb_clk->rate = rate;
+
+	return 0;
+}
Signature · 1 line
2.1.0
#1 lore raw

PATCH 1/1 video, ipu: make ldb clock frequenz overwriteable through board code

Hi Heiko,

Quoted 5 lines · Heiko Schocher

the ldb clock can be setup in board code (for example set through PLL5). Update the ldb_clock rate also through board code.

This should be removed, if a clock framework is availiable.

Any chance you're up to the task?

Searching for 'clk_get' in the sources shows a proliferation of attempts for various SOCs and drivers.

Quoted 12 lines
Signed-off-by Heiko Schocher <[email protected]>

---

 drivers/video/ipu.h        |  1 +
 drivers/video/ipu_common.c | 11 +++++++++++
 2 files changed, 12 insertions(+)
drivers/video/ipu.h +0 -0
index 091b58f..f13934f 100644

drivers/video/ipu.h void ipu_dp_uninit(ipu_channel_t channel); · line 268

Quoted 3 lines
drivers/video/ipu.h +0 -0
@@ -266,4 +266,5 @@ void ipu_dp_uninit(ipu_channel_t channel);
 void ipu_dp_dc_disable(ipu_channel_t channel, unsigned char swap);
 ipu_color_space_t format_to_colorspace(uint32_t fmt);

I'd rather see this in imx-common/video.h to avoid #includes out of the driver tree.

Quoted 7 lines
drivers/video/ipu.h +1 -0
+int ipu_set_ldb_clock(int rate);
 #endif
drivers/video/ipu_common.c +0 -0
index 1a209d4..dc054bc 100644

drivers/video/ipu_common.c ipu_color_space_t format_to_colorspace(uint32_t fmt) · line 1204

Quoted 7 lines
drivers/video/ipu_common.c +4 -0
@@ -1198,3 +1198,14 @@ ipu_color_space_t format_to_colorspace(uint32_t fmt)
 	}
 	return RGB;
 }
+
+/* should removed when clk framework is availiable */
+int ipu_set_ldb_clock(int rate)
+{

This forces a tight dependency on when this is called that using ldb_clk directly would prevent.

drivers/video/ipu_common.c ipu_color_space_t format_to_colorspace(uint32_t fmt) · line 1211

Quoted 8 lines
drivers/video/ipu_common.c +7 -0
+	if (g_ldb_clk == NULL)
+		return -ENOENT;
+
+	g_ldb_clk->rate = rate;
+
+	return 0;
+}

The use of g_ldb_clk seems pretty dodgy everywhere in the IPU driver.

Otherwise:

Tested-by Eric Nelson <[email protected]>
author
#2 lore raw

PATCH 1/1 video, ipu: make ldb clock frequenz overwriteable through board code

Hello Eric,

Am 16.04.2015 02:27, schrieb Eric Nelson:

Quoted 6 lines

Hi Heiko,

Quoted 3 lines · Heiko Schocher

the ldb clock can be setup in board code (for example set through PLL5). Update the ldb_clock rate also through board code.

Quoted 6 lines
Quoted 2 lines

This should be removed, if a clock framework is availiable.

Any chance you're up to the task?

Searching for 'clk_get' in the sources shows a proliferation of attempts for various SOCs and drivers.

Yes ... but it seems, I get no time for such a job :-(

Quoted 7 lines
Quoted 7 lines
Signed-off-by Heiko Schocher <[email protected]>

---

  drivers/video/ipu.h        |  1 +
  drivers/video/ipu_common.c | 11 +++++++++++
  2 files changed, 12 insertions(+)
Quoted 11 lines

drivers/video/ipu.h void ipu_dp_uninit(ipu_channel_t channel); · line 268

Quoted 8 lines
drivers/video/ipu.h +0 -0
index 091b58f..f13934f 100644
@@ -266,4 +266,5 @@ void ipu_dp_uninit(ipu_channel_t channel);
  void ipu_dp_dc_disable(ipu_channel_t channel, unsigned char swap);
  ipu_color_space_t format_to_colorspace(uint32_t fmt);

I'd rather see this in imx-common/video.h to avoid #includes out of the driver tree.

ok, moved.

Quoted 9 lines
Quoted 9 lines
drivers/video/ipu.h +1 -0
+int ipu_set_ldb_clock(int rate);
  #endif
drivers/video/ipu_common.c +0 -0
index 1a209d4..dc054bc 100644
@@ -1198,3 +1198,14 @@ ipu_color_space_t format_to_colorspace(uint32_t fmt)
  	}
  	return RGB;
Quoted 8 lines

drivers/video/ipu_common.c ipu_color_space_t format_to_colorspace(uint32_t fmt) · line 1204

Quoted 5 lines
drivers/video/ipu_common.c +4 -0
  }
+
+/* should removed when clk framework is availiable */
+int ipu_set_ldb_clock(int rate)
+{

This forces a tight dependency on when this is called that using ldb_clk directly would prevent.

Sorry, did not understand you here ...

Quoted 12 lines

drivers/video/ipu_common.c ipu_color_space_t format_to_colorspace(uint32_t fmt) · line 1211

Quoted 8 lines
drivers/video/ipu_common.c +7 -0
+	if (g_ldb_clk == NULL)
+		return -ENOENT;
+
+	g_ldb_clk->rate = rate;
+
+	return 0;
+}

The use of g_ldb_clk seems pretty dodgy everywhere in the IPU driver.

Quoted 3 lines

Otherwise:

Tested-by Eric Nelson <[email protected]>

Thanks for testing.

bye,
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#3 lore raw

PATCH 1/1 video, ipu: make ldb clock frequenz overwriteable through board code

Hi Heiko,

Quoted 19 lines · Heiko Schocher
Quoted 10 lines
Quoted 10 lines
drivers/video/ipu.h +1 -0
+int ipu_set_ldb_clock(int rate);
  #endif
drivers/video/ipu_common.c +0 -0
index 1a209d4..dc054bc 100644
@@ -1198,3 +1198,14 @@ ipu_color_space_t
 format_to_colorspace(uint32_t fmt)
      }
      return RGB;
Quoted 8 lines

drivers/video/ipu_common.c ipu_color_space_t format_to_colorspace(uint32_t fmt) · line 1204

Quoted 5 lines
drivers/video/ipu_common.c +4 -0
  }
+
+/* should removed when clk framework is availiable */
+int ipu_set_ldb_clock(int rate)
+{

This forces a tight dependency on when this is called that using ldb_clk directly would prevent.

Quoted 2 lines

Sorry, did not understand you here ...

The variable g_ldb is initialized in ipu_probe(), so this routine must be called at a very particular time.

You must have noticed it, since you put in a check for NULL.

Quoted 3 lines
Quoted 3 lines

drivers/video/ipu_common.c ipu_color_space_t format_to_colorspace(uint32_t fmt) · line 1207

Quoted 3 lines
drivers/video/ipu_common.c +3 -0
+    if (g_ldb_clk == NULL)
+        return -ENOENT;
+

If you replace this with ldb_clk.rate, the dependency goes away.

Quoted 5 lines
Quoted 5 lines

drivers/video/ipu_common.c ipu_color_space_t format_to_colorspace(uint32_t fmt) · line 1211

Quoted 5 lines
drivers/video/ipu_common.c +4 -0
+    g_ldb_clk->rate = rate;
+
+    return 0;
+}

Separately, I don't see any reason not to have g_ldb_clk statically initialized... i.e.

struct clk *g_ldb_clk = &ldb_clk;