On 6/19/2012 6:39 PM, Omar Ramirez Luna wrote:
Hi Benoit,

On 19 June 2012 07:36, Cousson, Benoit <b-cous...@ti.com> wrote:
On 6/16/2012 3:56 AM, Omar Ramirez Luna wrote:
...
+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,
+                     .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@4a066000 {
                compatible = "omap-mmu";
                ti,hwmods = "mmu_ipu";
                reg...;
                irqs...;
        }
}

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.

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.

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?

Regards,
Benoit
--
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

Reply via email to