OMAP: iommu: hwmod, reset handling and runtime PM

Add mmu hwmod data for ipu and dsp.

Signed-off-by Omar Ramirez Luna <[email protected]>

---
  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  136 +++++++++++++++++++++++++++-
  1 files changed, 134 insertions(+), 2 deletions(-)

arch/arm/mach-omap2/omap_hwmod_44xx_data.c +65 -2

index ebf9657..3879e9c 100644
@@ -30,6 +30,7 @@
  #include <plat/mmc.h>
  #include <plat/dmtimer.h>
  #include <plat/common.h>
+#include <plat/iommu.h>
 
  #include "omap_hwmod_common_data.h"
 

@@ -614,7 +615,6 @@ static struct omap_hwmod_irq_info omap44xx_dsp_irqs[] = {
 
  static struct omap_hwmod_rst_info omap44xx_dsp_resets[] = {
  	{ .name = "dsp", .rst_shift = 0 },
-	{ .name = "mmu_cache", .rst_shift = 1 },
  };
 
  static struct omap_hwmod omap44xx_dsp_hwmod = {

@@ -1629,7 +1629,6 @@ static struct omap_hwmod_irq_info omap44xx_ipu_irqs[] = {
  static struct omap_hwmod_rst_info omap44xx_ipu_resets[] = {
  	{ .name = "cpu0", .rst_shift = 0 },
  	{ .name = "cpu1", .rst_shift = 1 },
-	{ .name = "mmu_cache", .rst_shift = 2 },
  };
 
  static struct omap_hwmod omap44xx_ipu_hwmod = {

@@ -2436,6 +2435,137 @@ static struct omap_hwmod omap44xx_mmc5_hwmod = {
  };
 
  /*
+ * 'mmu' class
+ * The memory management unit performs virtual to physical address translation
+ * for its requestors.
+ */
+
+static struct omap_hwmod_class_sysconfig mmu_sysc = {
+	.rev_offs	= 0x000,
+	.sysc_offs	= 0x010,
+	.syss_offs	= 0x014,
+	.sysc_flags	= (SYSC_HAS_CLOCKACTIVITY | SYSC_HAS_SIDLEMODE |
+			   SYSC_HAS_SOFTRESET | SYSC_HAS_AUTOIDLE),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap44xx_mmu_hwmod_class = {
+	.name = "mmu",
+	.sysc = &mmu_sysc,
+};
+
+/* ipu mmu */
+
+static struct omap_mmu_dev_attr ipu_mmu_dev_attr = {
+	.da_start = 0x0,
+	.da_end = 0xfffff000,
+	.nr_tlb_entries = 32,
+};
+
+static struct omap_hwmod omap44xx_ipu_mmu_hwmod;
+static struct omap_hwmod_irq_info omap44xx_ipu_mmu_irqs[] = {
+	{ .irq = 100 + OMAP44XX_IRQ_GIC_START, },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_rst_info omap44xx_ipu_mmu_resets[] = {
+	{ .name = "mmu_cache", .rst_shift = 2 },
+};
+
+static struct omap_hwmod_addr_space omap44xx_ipu_mmu_addrs[] = {
+	{
+		.pa_start	= 0x55082000,
+		.pa_end		= 0x550820ff,
+		.flags		= ADDR_TYPE_RT,
+	},
+	{ }
+};
+
+/* l3_main_2 -> ipu_mmu */
+static struct omap_hwmod_ocp_if omap44xx_l3_main_2__ipu_mmu = {
+	.master		= &omap44xx_l3_main_2_hwmod,
+	.slave		= &omap44xx_ipu_mmu_hwmod,
+	.clk		= "l3_div_ck",
+	.addr		= omap44xx_ipu_mmu_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+static struct omap_hwmod omap44xx_ipu_mmu_hwmod = {
+	.name		= "ipu_mmu",
+	.class		= &omap44xx_mmu_hwmod_class,
+	.clkdm_name	= "ducati_clkdm",
+	.mpu_irqs	= omap44xx_ipu_mmu_irqs,
+	.rst_lines	= omap44xx_ipu_mmu_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_ipu_mmu_resets),
+	.main_clk	= "ipu_fck",

On 6/16/2012 3:56 AM, Omar Ramirez Luna wrote:

Quoted 8 lines

(quoted diff) +8 -0

+static struct omap_hwmod omap44xx_ipu_mmu_hwmod = {
+ ? ? .name ? ? ? ? ? = "ipu_mmu",
+ ? ? .class ? ? ? ? ?= &omap44xx_mmu_hwmod_class,
+ ? ? .clkdm_name ? ? = "ducati_clkdm",
+ ? ? .mpu_irqs ? ? ? = omap44xx_ipu_mmu_irqs,
+ ? ? .rst_lines ? ? ?= omap44xx_ipu_mmu_resets,
+ ? ? .rst_lines_cnt ?= ARRAY_SIZE(omap44xx_ipu_mmu_resets),
+ ? ? .main_clk ? ? ? = "ipu_fck",

Review

Quoted 10 lines

(quoted diff) +10 -0

+ ? ? .prcm = {
+ ? ? ? ? ? ? .omap4 = {
+ ? ? ? ? ? ? ? ? ? ? .clkctrl_offs = OMAP4_CM_DUCATI_DUCATI_CLKCTRL_OFFSET,
+ ? ? ? ? ? ? ? ? ? ? .rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
+ ? ? ? ? ? ? ? ? ? ? .context_offs = OMAP4_RM_DUCATI_DUCATI_CONTEXT_OFFSET,
+ ? ? ? ? ? ? ? ? ? ? .modulemode ? = MODULEMODE_HWCTRL,
+ ? ? ? ? ? ? },
+ ? ? },
+ ? ? .dev_attr ? ? ? = &ipu_mmu_dev_attr,
+};

In fact, the MMU_IPU hwmod is now almost the same one than the previous IPU one... If we do that we should maybe just rename the IPU -> MMU_IPU and DSP -> MMU_DSP.

But by doing that we will assume that the MMU does represent the subsystem, which is not necessarily super nice.

I guess that a much better representation will be to keep the subsystem (IPU) to handle the PRCM part:

+ ? ? ? .main_clk ? ? ? = "ipu_fck",
+ ? ? ? .prcm = {
+ ? ? ? ? ? ? ? .clkctrl_offs = OMAP4_CM_DUCATI_DUCATI_CLKCTRL_OFFSET,
+ ? ? ? ? ? ? ? .rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
+ ? ? ? ? ? ? ? .context_offs = OMAP4_RM_DUCATI_DUCATI_CONTEXT_OFFSET,
+ ? ? ? ? ? ? ? .modulemode ? = MODULEMODE_HWCTRL,

And then the MMU_IPU will handle the configuration registers part and the reset + irq.

But then, you will have to create a parent child dependency between your devices to ensure that the IPU subsystem device will be enabled before trying to access the MMU_IPU.

This is what the DSS is about to do to handle the same kind of power dependency. The DSS device is the parent of all the DSS IPs (DISPC, HDMI...) and thus pm_runtime will ensure that the parent is enabled before trying to enable the children.

In term of DT, just to illustrate the situation, it will be something like that:

ipu {
? ? ? ?compatible = "simple-bus";
? ? ? ?ti,hwmods = "ipu";

? ? ? ?ipu_mmu: mmu at 4a066000 {
? ? ? ? ? ? ? ?compatible = "omap-mmu";
? ? ? ? ? ? ? ?ti,hwmods = "mmu_ipu";
? ? ? ? ? ? ? ?reg...;
? ? ? ? ? ? ? ?irqs...;
? ? ? ?}
}

Is it something you can handle with your current framework?

Hi Benoit,

Quoted 1 line · Benoit

On 6/16/2012 3:56 AM, Omar Ramirez Luna wrote:

...

Quoted 53 lines

Quoted 8 lines

arch/arm/mach-omap2/omap_hwmod_44xx_data.c +8 -0

+static struct omap_hwmod omap44xx_ipu_mmu_hwmod = {
+     .name           = "ipu_mmu",
+     .class          = &omap44xx_mmu_hwmod_class,
+     .clkdm_name     = "ducati_clkdm",
+     .mpu_irqs       = omap44xx_ipu_mmu_irqs,
+     .rst_lines      = omap44xx_ipu_mmu_resets,
+     .rst_lines_cnt  = ARRAY_SIZE(omap44xx_ipu_mmu_resets),
+     .main_clk       = "ipu_fck",

arch/arm/mach-omap2/omap_hwmod_44xx_data.c static struct omap_hwmod omap44xx_mmc5_hwmod = { · line 2511

Quoted 10 lines

arch/arm/mach-omap2/omap_hwmod_44xx_data.c +10 -0

+     .prcm = {
+             .omap4 = {
+                     .clkctrl_offs = OMAP4_CM_DUCATI_DUCATI_CLKCTRL_OFFSET,
+                     .rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
+                     .context_offs = OMAP4_RM_DUCATI_DUCATI_CONTEXT_OFFSET,
+                     .modulemode   = MODULEMODE_HWCTRL,
+             },
+     },
+     .dev_attr       = &ipu_mmu_dev_attr,
+};

In fact, the MMU_IPU hwmod is now almost the same one than the previous IPU one... If we do that we should maybe just rename the IPU -> MMU_IPU and DSP -> MMU_DSP.

But by doing that we will assume that the MMU does represent the subsystem, which is not necessarily super nice.

I guess that a much better representation will be to keep the subsystem (IPU) to handle the PRCM part:

arch/arm/mach-omap2/omap_hwmod_44xx_data.c +6 -0

+       .main_clk       = "ipu_fck",
+       .prcm = {
+               .clkctrl_offs = OMAP4_CM_DUCATI_DUCATI_CLKCTRL_OFFSET,
+               .rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
+               .context_offs = OMAP4_RM_DUCATI_DUCATI_CONTEXT_OFFSET,
+               .modulemode   = MODULEMODE_HWCTRL,

And then the MMU_IPU will handle the configuration registers part and the reset + irq.

But then, you will have to create a parent child dependency between your devices to ensure that the IPU subsystem device will be enabled before trying to access the MMU_IPU.

This is what the DSS is about to do to handle the same kind of power dependency. The DSS device is the parent of all the DSS IPs (DISPC, HDMI...) and thus pm_runtime will ensure that the parent is enabled before trying to enable the children.

In term of DT, just to illustrate the situation, it will be something like that:

ipu {
        compatible = "simple-bus";
        ti,hwmods = "ipu";

        ipu_mmu: mmu at 4a066000 {
                compatible = "omap-mmu";
                ti,hwmods = "mmu_ipu";
                reg...;
                irqs...;
        }
}

Quoted 1 line

Is it something you can handle with your current framework?

I agree it would be nice only IPU managed the prcm, however we can't do that right now because hwmod expects the IP block to be out of reset to continue the _enable sequence. On IPU both reset lines are asserted at that point and hence _are_any_hardreset_lines_asserted check will bail out, and ipu resets can't be lifted without a configured iommu otherwise it might execute random garbage.

So, for IPU and DSP the mmu must be deasserted and configured before the processor reset line (which is more like a parking brake) is deasserted, and the latter should be made before _enable is called so it can fully execute the enable sequence.

Quoted 79 lines · Omar Ramirez Luna

Hi Benoit,

Quoted 2 lines · Benoit

On 6/16/2012 3:56 AM, Omar Ramirez Luna wrote:

...

Quoted 68 lines

Quoted 13 lines

+static struct omap_hwmod omap44xx_ipu_mmu_hwmod = {
+ ? ? .name ? ? ? ? ? = "ipu_mmu",
+ ? ? .class ? ? ? ? ?= &omap44xx_mmu_hwmod_class,
+ ? ? .clkdm_name ? ? = "ducati_clkdm",
+ ? ? .mpu_irqs ? ? ? = omap44xx_ipu_mmu_irqs,
+ ? ? .rst_lines ? ? ?= omap44xx_ipu_mmu_resets,
+ ? ? .rst_lines_cnt ?= ARRAY_SIZE(omap44xx_ipu_mmu_resets),
+ ? ? .main_clk ? ? ? = "ipu_fck",
+ ? ? .prcm = {
+ ? ? ? ? ? ? .omap4 = {
+ ? ? ? ? ? ? ? ? ? ? .clkctrl_offs = OMAP4_CM_DUCATI_DUCATI_CLKCTRL_OFFSET,

Quoted 8 lines

+ ? ? ? ? ? ? ? ? ? ? .rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
+ ? ? ? ? ? ? ? ? ? ? .context_offs = OMAP4_RM_DUCATI_DUCATI_CONTEXT_OFFSET,
+ ? ? ? ? ? ? ? ? ? ? .modulemode ? = MODULEMODE_HWCTRL,
+ ? ? ? ? ? ? },
+ ? ? },
+ ? ? .dev_attr ? ? ? = &ipu_mmu_dev_attr,
+};

In fact, the MMU_IPU hwmod is now almost the same one than the previous IPU one...
If we do that we should maybe just rename the IPU -> MMU_IPU and DSP -> MMU_DSP.

But by doing that we will assume that the MMU does represent the subsystem, which is not necessarily super nice.

I guess that a much better representation will be to keep the subsystem (IPU) to handle the PRCM part:

+ ? ? ? .main_clk ? ? ? = "ipu_fck",
+ ? ? ? .prcm = {
+ ? ? ? ? ? ? ? .clkctrl_offs = OMAP4_CM_DUCATI_DUCATI_CLKCTRL_OFFSET,
+ ? ? ? ? ? ? ? .rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
+ ? ? ? ? ? ? ? .context_offs = OMAP4_RM_DUCATI_DUCATI_CONTEXT_OFFSET,
+ ? ? ? ? ? ? ? .modulemode ? = MODULEMODE_HWCTRL,

And then the MMU_IPU will handle the configuration registers part and the reset + irq.

But then, you will have to create a parent child dependency between your devices to ensure that the IPU subsystem device will be enabled before trying to access the MMU_IPU.

This is what the DSS is about to do to handle the same kind of power dependency. The DSS device is the parent of all the DSS IPs (DISPC, HDMI...) and thus pm_runtime will ensure that the parent is enabled before trying to enable the children.

In term of DT, just to illustrate the situation, it will be something like that:

ipu {
? ? ? ?compatible = "simple-bus";
? ? ? ?ti,hwmods = "ipu";

? ? ? ?ipu_mmu: mmu at 4a066000 {
? ? ? ? ? ? ? ?compatible = "omap-mmu";
? ? ? ? ? ? ? ?ti,hwmods = "mmu_ipu";
? ? ? ? ? ? ? ?reg...;
? ? ? ? ? ? ? ?irqs...;
? ? ? ?}
}

Quoted 1 line

Is it something you can handle with your current framework?

Quoted 6 lines

I agree it would be nice only IPU managed the prcm, however we can't do that right now because hwmod expects the IP block to be out of reset to continue the _enable sequence. On IPU both reset lines are asserted at that point and hence _are_any_hardreset_lines_asserted check will bail out, and ipu resets can't be lifted without a configured iommu otherwise it might execute random garbage.

That's a good point, but like Paul said, the hard reset was removed outside of the fmwk due to the lack of understanding about the real usage / need for it.

If we do have a better understanding, we might add some more support in the fmwk or at least improve it.

I'm now realizing that aborting the init if some reset lines are asserted is not what we want to do for the IPU SS hwmod that will contain the IPU (processor) reset control.
In fact the previous approach with a fake hwmod for the ipu_c0 processor would have been avoided that issue.

If we do not want to go back with that, we should clearly revise the _are_any_hardreset_lines_asserted approach and not prevent the init in such case since it will prevent all the subsystem to start properly.

Quoted 4 lines

So, for IPU and DSP the mmu must be deasserted and configured before the processor reset line (which is more like a parking brake) is deasserted, and the latter should be made before _enable is called so it can fully execute the enable sequence.

Yep, so we have to change the way it is handled today.

Paul,

If we consider that the reset lines are stored in the subsystem hwmod (IPU, DSP or IVA), we cannot prevent the init phase because some line are asserted. Otherwise we will never allow the MMU or processor to be enabled later.
We might have to remove that check, maybe based on flag if we want to keep that functionality or do an explicit reset control like we use to do.

What do you think?

Quoted 124 lines · Benoit

Quoted 79 lines · Omar Ramirez Luna

Hi Benoit,

Quoted 2 lines · Benoit

On 6/16/2012 3:56 AM, Omar Ramirez Luna wrote:

...

Quoted 68 lines

Quoted 16 lines

arch/arm/mach-omap2/omap_hwmod_44xx_data.c +11 -0

+static struct omap_hwmod omap44xx_ipu_mmu_hwmod = {
+     .name           = "ipu_mmu",
+     .class          = &omap44xx_mmu_hwmod_class,
+     .clkdm_name     = "ducati_clkdm",
+     .mpu_irqs       = omap44xx_ipu_mmu_irqs,
+     .rst_lines      = omap44xx_ipu_mmu_resets,
+     .rst_lines_cnt  = ARRAY_SIZE(omap44xx_ipu_mmu_resets),
+     .main_clk       = "ipu_fck",
+     .prcm = {
+             .omap4 = {
+                     .clkctrl_offs =

OMAP4_CM_DUCATI_DUCATI_CLKCTRL_OFFSET,

(quoted diff) +2 -0

+                     .rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
+                     .context_offs =

OMAP4_RM_DUCATI_DUCATI_CONTEXT_OFFSET,

arch/arm/mach-omap2/omap_hwmod_44xx_data.c static struct omap_hwmod omap44xx_mmc5_hwmod = { · line 2511

Quoted 5 lines

arch/arm/mach-omap2/omap_hwmod_44xx_data.c +5 -0

+                     .modulemode   = MODULEMODE_HWCTRL,
+             },
+     },
+     .dev_attr       = &ipu_mmu_dev_attr,
+};

In fact, the MMU_IPU hwmod is now almost the same one than the previous IPU one...
If we do that we should maybe just rename the IPU -> MMU_IPU and DSP -> MMU_DSP.

But by doing that we will assume that the MMU does represent the subsystem, which is not necessarily super nice.

I guess that a much better representation will be to keep the subsystem (IPU) to handle the PRCM part:

arch/arm/mach-omap2/omap_hwmod_44xx_data.c +6 -0

+       .main_clk       = "ipu_fck",
+       .prcm = {
+               .clkctrl_offs = OMAP4_CM_DUCATI_DUCATI_CLKCTRL_OFFSET,
+               .rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
+               .context_offs = OMAP4_RM_DUCATI_DUCATI_CONTEXT_OFFSET,
+               .modulemode   = MODULEMODE_HWCTRL,

And then the MMU_IPU will handle the configuration registers part and the reset + irq.

But then, you will have to create a parent child dependency between your devices to ensure that the IPU subsystem device will be enabled before trying to access the MMU_IPU.

This is what the DSS is about to do to handle the same kind of power dependency. The DSS device is the parent of all the DSS IPs (DISPC, HDMI...) and thus pm_runtime will ensure that the parent is enabled before trying to enable the children.

In term of DT, just to illustrate the situation, it will be something like that:

ipu {
       compatible = "simple-bus";
       ti,hwmods = "ipu";

       ipu_mmu: mmu at 4a066000 {
               compatible = "omap-mmu";
               ti,hwmods = "mmu_ipu";
               reg...;
               irqs...;
       }
}

Quoted 1 line

Is it something you can handle with your current framework?

Quoted 6 lines

I agree it would be nice only IPU managed the prcm, however we can't do that right now because hwmod expects the IP block to be out of reset to continue the _enable sequence. On IPU both reset lines are asserted at that point and hence _are_any_hardreset_lines_asserted check will bail out, and ipu resets can't be lifted without a configured iommu otherwise it might execute random garbage.

That's a good point, but like Paul said, the hard reset was removed outside of the fmwk due to the lack of understanding about the real usage / need for it.

If we do have a better understanding, we might add some more support in the fmwk or at least improve it.

I'm now realizing that aborting the init if some reset lines are asserted is not what we want to do for the IPU SS hwmod that will contain the IPU (processor) reset control.
In fact the previous approach with a fake hwmod for the ipu_c0 processor would have been avoided that issue.

If we do not want to go back with that, we should clearly revise the _are_any_hardreset_lines_asserted approach and not prevent the init in such case since it will prevent all the subsystem to start properly.

Quoted 4 lines

So, for IPU and DSP the mmu must be deasserted and configured before the processor reset line (which is more like a parking brake) is deasserted, and the latter should be made before _enable is called so it can fully execute the enable sequence.

Yep, so we have to change the way it is handled today.

Paul,

If we consider that the reset lines are stored in the subsystem hwmod (IPU, DSP or IVA), we cannot prevent the init phase because some line are asserted. Otherwise we will never allow the MMU or processor to be enabled later.
We might have to remove that check, maybe based on flag if we want to keep that functionality or do an explicit reset control like we use to do.

Quoted 1 line

What do you think?

I was waiting for some comments then I realized Paul wasn't actually in the list of recipients for this email.

Omar Ramirez Luna (6):
ARM: OMAP: iommu: fix including iommu.h without IOMMU_API selected
ARM: OMAP3: hwmod data: add mmu data for iva and isp
ARM: OMAP4: hwmod data: add mmu hwmod for ipu and dsp
ARM: OMAP3/4: iommu: migrate to hwmod framework
ARM: OMAP2+: iommu: add reset handling
ARM: OMAP3/4: iommu: adapt to runtime pm

Quoted 7 lines · Omar Ramirez Luna

Omar Ramirez Luna (6):
ARM: OMAP: iommu: fix including iommu.h without IOMMU_API selected
ARM: OMAP3: hwmod data: add mmu data for iva and isp
ARM: OMAP4: hwmod data: add mmu hwmod for ipu and dsp
ARM: OMAP3/4: iommu: migrate to hwmod framework
ARM: OMAP2+: iommu: add reset handling
ARM: OMAP3/4: iommu: adapt to runtime pm

What happens with this patch-set. Comments anyone? Ohad?

+ Paul

Quoted 9 lines · Joerg Roedel

Quoted 7 lines · Omar Ramirez Luna

Omar Ramirez Luna (6):
ARM: OMAP: iommu: fix including iommu.h without IOMMU_API selected
ARM: OMAP3: hwmod data: add mmu data for iva and isp
ARM: OMAP4: hwmod data: add mmu hwmod for ipu and dsp
ARM: OMAP3/4: iommu: migrate to hwmod framework
ARM: OMAP2+: iommu: add reset handling
ARM: OMAP3/4: iommu: adapt to runtime pm

Quoted 1 line

What happens with this patch-set. Comments anyone? Ohad?

Looks like it depends on a few OMAP hwmod changes (reset handling) which require Paul's ACK.