Re: [PATCH v2] OMAPDSS: HACK: Ensure DSS clock domain gets out of idle when HDMI is enabled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux