On Tue, 2012-02-14 at 19:00 +0530, Archit Taneja wrote:
> Hi,
>
> On Tuesday 14 February 2012 06:45 PM, Tomi Valkeinen wrote:
> > On Tue, 2012-02-14 at 13:58 +0100, Cousson, Benoit wrote:
> >> Hi Tomi,
> >
> >>> Benoit, do you think we'll get the MODULEMODE mess cleaned up in the
> >>> hwmod/clk framework at some point, and the drivers could do without
> >>> these kinds of hacks? =)
> >>
> >> The best way to fix that for my point of view is to go to device tree
> >> or/and to consider the DSS as the parent of all the DSS modules.
> >> pm_runtime will then always ensure that the parent is enabled before any
> >> of the child are used.
> >
> > Ah, right. Sounds fine to me.
> >
> > But is that a proper "fix"? Are we sure the MODULEMODE will then always
> > be handled correctly? Isn't the core problem still there, it just
> > doesn't happen with the setup anymore?
> >
> > I mean, if we have these special requirements regarding MODULEMODE, and
> > the code doesn't really know about it, would it get broken easily with
> > restructuring/changes?
> >
> > And no, I don't have any clear idea why/how it would break, but I have
> > just gotten the impression that the MODULEMODE is not handled quite
> > properly (and so we have these current problems), and having dss_core as
> > the parent of other dss modules doesn't really fix that in any way.
>
> I agree with that.
>
> In the current approach, we have multiple platform devices for DSS, and
> all of them belong to the same clock domain, and the clock domain has
> just one MODULEMODE bit field.
>
> When shutting off a platform device(by calling pm_runtime_put()), hwmod
> enables/disables MODULEMODE without taking into mind that other active
> platform devices may still need it. So, for example, if we have 2
> platform devices, say dss and dispc, and we have code like:
>
> dispc_foo()
> {
> pm_runtime_get(dispc_pdev);
> ...
> ...
> pm_runtime_put(dispc_pdev);
> }
>
> dss_foo()
> {
> pm_runtime_get(dss_pdev);
> ...
> ...
> dispc_foo(); /* MODULEMODE off after this */
> ...
> ...
> pm_runtime_put(dss_pdev);
> }
>
> This will lead to the situation of one platform device disabling
> MODULEMODE even though other platform devices need it.
>
> This may not be resolved in device tree either. We would need to have
> some use count mechanism for these bits, or attach MODULEMODE only to
> one platform device, and don't give others control to enable/disable it.
Hmm, are you sure? Not that I checked the code, but isn't MODULEMODE
mapped to a dss clock (was it "dss_clk")?. And so, the clock's refcount
should keep the MODULEMODE enabled/disabled?
Tomi
Attachment:
signature.asc
Description: This is a digitally signed message part
