From: "Cousson, Benoit" <b-cousson@ti.com>
To: Archit Taneja <a0393947@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>,
linux-omap@vger.kernel.org, linux@arm.linux.org.uk,
linux-fbdev@vger.kernel.org
Subject: Re: [PATCH v2] OMAPDSS: HACK: Ensure DSS clock domain gets out of idle when HDMI is enabled
Date: Wed, 15 Feb 2012 12:35:46 +0000 [thread overview]
Message-ID: <4F3BA6A2.60604@ti.com> (raw)
In-Reply-To: <4F3B9E8F.1020002@ti.com>
On 2/15/2012 1:01 PM, Archit Taneja wrote:
> On Tuesday 14 February 2012 09:11 PM, Cousson, Benoit wrote:
>> On 2/14/2012 2:30 PM, 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.
>>
>> And this is exactly what the pm_runtime will provide. The fmwk already
>> handles reference counting.
>> Moreover the dev->parent will increment the power.child_count and thus
>> ensure that the parent is always enabled if at least one child is active.
>>
>> By initializing the dev->parent of each DSS modules to the dss_core, it
>> will ensure that the power dependency is managed properly.
>
> Yes, a parent/child relation will ensure the required dependencies. It
> sounds good!
>
> How do we take care of things in the meanwhile? With the current way of
> representing modulemode as a slave clock, the disabling sequence gets
> messed up. In arch/arm/mach-omap2/omap_hwmod.c:
>
> static int _disable_clocks(struct omap_hwmod *oh)
> {
> ...
> disable mainclk;
> disable slave clocks;
> ...
> }
>
> For DSS, mainclk is the DSS_FCLK opt clock, and slave clock is
> modulemode bits. We need the opposite sequence to happen here to cleanly
> disable DSS.
Yes, but as you pointed out the current clocks mapping in the DSS is a
hack that should not be done like that in theory.
As soon as the devices can handle the dependency, we will be able to fix
the clock mapping in the hwmod data and handle properly the module mode
only from the dss_core hwmod only.
But I think you have to change the driver first, otherwise it will break
the DSS. We did that hack previously because it was the only way to
enable the DSS properly, knowing that disabling it with that method will
be impossible.
I think that changing the device creation to change the dev->parent
should be pretty straightforward.
Regards,
Benoit
next prev parent reply other threads:[~2012-02-15 12:35 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-10 6:27 [PATCH v2] OMAPDSS: HACK: Ensure DSS clock domain gets out of idle when HDMI is enabled Archit Taneja
2012-02-14 11:57 ` Tomi Valkeinen
2012-02-14 12:58 ` Cousson, Benoit
2012-02-14 13:15 ` Tomi Valkeinen
2012-02-14 13:42 ` Archit Taneja
2012-02-14 13:33 ` Tomi Valkeinen
2012-02-14 13:57 ` Archit Taneja
2012-02-14 16:02 ` Cousson, Benoit
2012-02-14 16:59 ` Shilimkar, Santosh
2012-02-14 19:54 ` Archit Taneja
2012-02-14 15:41 ` Cousson, Benoit
2012-02-15 12:13 ` Archit Taneja
2012-02-15 12:35 ` Cousson, Benoit [this message]
2012-02-15 12:51 ` Tomi Valkeinen
2012-02-15 13:04 ` Cousson, Benoit
2012-02-15 19:59 ` Kevin Hilman
2012-02-16 8:22 ` Tomi Valkeinen
2012-02-16 10:16 ` Cousson, Benoit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F3BA6A2.60604@ti.com \
--to=b-cousson@ti.com \
--cc=a0393947@ti.com \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=tomi.valkeinen@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).