+ Paul
On 19 June 2012 12:48, Cousson, Benoit <b-cousson@xxxxxx> wrote:
> On 6/19/2012 6:39 PM, Omar Ramirez Luna wrote:
>>
>> Hi Benoit,
>>
>> On 19 June 2012 07:36, Cousson, Benoit <b-cousson@xxxxxx> 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?
I was waiting for some comments then I realized Paul wasn't actually
in the list of recipients for this email.
Regards,
Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
