Re: OMAP4 DSS clock setup
On Mon, 2011-04-11 at 12:20 -0600, Paul Walmsley wrote: Hi On Mon, 11 Apr 2011, Tomi Valkeinen wrote: However, I think there is one difference between the clock used just to enable the DSS registers, and the one used to output pixels: we need to be able to adjust the rate of the clock. Thus we need to have a common (omap2/3/4) clock name for it to be able to clk_get() it. Should that clock name be just the main clock provided automatically, or something else? Are you referring here to the system DPLL and its output dividers, or are you referring to the DSS module's internal dividers? The system DPLL and its divider. If we are using the dss_dss_clk as fclk, we need to adjust it depending on the required pixel clock and use cases (e.g. some scaling factors may need higher fclk). Tomi -- 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
Re: OMAP4 DSS clock setup
On Mon, 2011-04-11 at 10:05 -0600, Paul Walmsley wrote: Hi On Mon, 11 Apr 2011, Tomi Valkeinen wrote: On Fri, 2011-04-08 at 10:28 -0600, Paul Walmsley wrote: On Fri, 8 Apr 2011, Tomi Valkeinen wrote: Also, I hope you and the other DSS hackers can finish the PM runtime conversion of the DSS driver soon. Ideally before any new DSS features are added. We really need to be able to separate the OMAP integration details from the drivers, and right now, hwmod and PM runtime are the best way we have to do that. I also started to look at the PM runtime, but it doesn't fix the issue with the inconsistent clock name I described above. After the hwmod/PM runtime conversion, I don't think any of the clock aliases currently in clock*_data.c should be used by the DSS driver (or by anything else on the system, for that matter). That's because the omap_device code should set up the main alias for the DSS main When should main clock be used by the driver? Or never, and pm_runtime handles that? If the DSS needs to change the parent or the clock rate of the main clock, then it will need to clk_get(dev, main) and use the clock API. My only point here was that the main alias should be automatically added by the omap_device code, not via the clock aliases in clock*_data.c. Ok. How about OMAP3? It also has an optional functional clock, but that doesn't seem to be set in dss_opt_clocks in omap_hwmod_3xxx_data.c. Should it be there? Probably so. It seems dss1_alwon_fclk is currently the main clock for OMAP3 dss hwmods. Does that mean it can never be turned off if DSS is in use? If the DSS driver were using PM runtime, then yes, that would be true. Ok. Then there's also room for improvement, as the dss1_alwon_fclk is an optional clock on OMAP3. Ok. Then we need omap_hwmod_opt_clk for at least for VENC and HDMI, I believe. In terms of the VENC, I'd agree with that -- based on OMAP4 Public TRM Rev O Figure 10-177 Video Encoder Overview, it looks like VENC uses DSS_TV_FCLK. On OMAP3 DSS_96M_FCLK goes to VENC's video DACs. I don't quite understand how it goes in OMAP4, the clock tree figure shows something going into VDAC, but it's unclear what. Well, anyway, the point was not to dig out the clocks now, but just to point out that some extra clocks are needed =). In terms of the HDMI IP block, based on OMAP4 Public TRM Rev O Figure 10-159 HDMI Overview, it looks like sys_clk should be one of the optional clocks for HDMI? Hard to tell from that diagram. Yes, I think sys_clk is the input clock for the HDMI PLL. The output from the PLL can then be used for DISPC fckl. I believe HDMI_PHY_48M_FCLK (dss_48mhz_clk) is also needed. Tomi -- 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
Re: OMAP4 DSS clock setup
On Fri, 2011-04-08 at 10:28 -0600, Paul Walmsley wrote:
Hi Tomi,
On Fri, 8 Apr 2011, Tomi Valkeinen wrote:
Also, I hope you and the other DSS hackers can finish the PM runtime
conversion of the DSS driver soon. Ideally before any new DSS
features are added. We really need to be able to separate the OMAP
integration details from the drivers, and right now, hwmod and PM
runtime are the best way we have to do that.
I also started to look at the PM runtime, but it doesn't fix the issue
with the inconsistent clock name I described above.
After the hwmod/PM runtime conversion, I don't think any of the clock
aliases currently in clock*_data.c should be used by the DSS driver (or by
anything else on the system, for that matter). That's because the
omap_device code should set up the main alias for the DSS main
When should main clock be used by the driver? Or never, and pm_runtime
handles that?
functional clock[*], as well as the aliases in the optional clock data in
the OMAP hwmod data files:
static struct omap_hwmod_opt_clk dss_opt_clks[] = {
{ .role = sys_clk, .clk = dss_sys_clk },
{ .role = tv_clk, .clk = dss_tv_clk },
{ .role = dss_clk, .clk = dss_dss_clk },
{ .role = video_clk, .clk = dss_48mhz_clk },
};
It might be that some of these role names aren't quite accurate and need
to be changed. Those are intended to be meaningful to the driver, so
comments there are definitely welcome.
How about OMAP3? It also has an optional functional clock, but that
doesn't seem to be set in dss_opt_clocks in omap_hwmod_3xxx_data.c.
Should it be there?
It seems dss1_alwon_fclk is currently the main clock for OMAP3 dss
hwmods. Does that mean it can never be turned off if DSS is in use?
[*]. The main alias should be defined by the omap_device code
automatically, similarly to how _add_optional_clock_clkdev() does it. It
does not do so currently. This needs to be fixed in the omap_device.c
code.
I also have some questions regarding PM runtime, perhaps I'll just put
them here as they are slightly related:
- Should every DSS module handle their clocks independently, i.e. should
VENC get its own clocks and DSI should get its own? If so, we need a
bunch of new clock aliases so the devices can get their clocks.
If all that driver code needs to do is to enable its main functional clock
when it is active and disable that clock when the driver is inactive,
then, no, the drivers shouldn't need their own clock aliases. Same if the
driver only needs to get the rate or set the rate on that main functional
clock, since that alias should be set up automatically.
But if the driver for that submodule needs to control PRCM-provided
optional clocks, then it will need to have struct omap_hwmod_opt_clk
entries defined in the hwmod data.
Ok. Then we need omap_hwmod_opt_clk for at least for VENC and HDMI, I
believe.
- Should every DSS module have their own PM runtime handling? (actually
related to the question above)
Yes, I think so. From the integration perspective, we are trying to get
to the point where each omap_device maps to only one hwmod.
- If the modules are handled separately, how should the dependencies be
handled? For example, dss_core's reset will reset all the other modules
also, and most of the submodules need functions from dss_core and
dss_dispc. So should, say, dss_dsi then call functions in core and dispc
to get them, i.e. increase their pm runtime use count?
Probably not.
Here is how I would suggest structuring the code. This is only a naïve
suggestion; you and your team almost certainly know better than I.
I'd suggest that you separate low-level register access into .c files
that are targeted specifically for the IP block. So in the DSS case,
you'd have dss.c, dispc.c, dsi.c, hdmi.c, rfbi.c, venc.c. Each one should
be a separate platform_device and would export symbols. Hopefully, there
should be no cross-dependencies between these low-level files.
Then you'd have higher-level code that might need to read/write from
multiple DSS submodules to complete some higher-level operation. That
would be at least one separate driver -- say, dss2 or something -- with
dependencies on the low-level drivers. So, for example, when that
higher-level driver is modprobe'd, Linux would also load the drivers for
the underlying hardware blocks that it uses.
But this still leaves my question open: If this dss2 needs to use,
say, dispc.c and dsi.c, those low level drivers need to be enabled. So I
guess we would have something like dispc_get() or dispc_enable(), which
dss2 would call first. Those functions would then use pm_runtime to
enable the HW module. And the higher level driver would keep the low
level devices enabled while its doing something.
I have been thinking something like this also earlier. It would separate
things cleanly. One thing I don't like
Re: OMAP4 DSS clock setup
On Fri, 2011-04-08 at 08:55 -0600, Paul Walmsley wrote: Hi On Fri, 8 Apr 2011, Tomi Valkeinen wrote: and not the clock used for the pixel clock? If so, I'm fine with having fck to be what it is currently, but then we need a new name for the clock used for pixel clock, which is consistent on all platforms. If there is a separate PRCM-provided clock used only for the pixel clock, then that clock should have an alias name of system_pixel_ck or something similar that is meaningful to the DSS driver. I think the problem in this case is that dss_dss_clk is (optionally) used for two purposes: optionally as a main PRCM-provided functional clock and optionally as a system-provided pixel clock. Not only for pixel clock, but in the end it'll come out as pixel clock. I'm not sure what exactly is a functional clock here. I mean, one could think it as a basic functionality of DSS to read the pixels, manipulate them, and output them (with the rate of the pixel clock). However, I think there is one difference between the clock used just to enable the DSS registers, and the one used to output pixels: we need to be able to adjust the rate of the clock. Thus we need to have a common (omap2/3/4) clock name for it to be able to clk_get() it. Should that clock name be just the main clock provided automatically, or something else? Tomi -- 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
Re: OMAP4 DSS clock setup
On Fri, 2011-04-08 at 10:50 -0600, Paul Walmsley wrote: Hi Tomi, On Mon, 4 Apr 2011, Tomi Valkeinen wrote: Not directly related, but something I've been wondering about is how to abstract the DSI/HDMI PLLs in DSS. What do you think, would it be possible/worth it to create struct clks for the clocks coming out of those PLLs? These would, of course, be DSS internal clks. I'm not very familiar with the clock framework, so I don't really have any idea what this would require and what would be the pros and cons. Yes, I think it would be good to try to implement the entire DSS clock tree in the clock framework. One change to the clock code that we know we'll need is to put a hwmod pointer in the struct clk which tells the clock code that the hwmod needs to be enabled in order to access the clock's registers. Right now, the clock code assumes that all of the clock registers are accessible, all of the time. It's not quite that simple, as DSI PLL also needs the vdds_dsi regulator to be enabled... And to use DSI PLL, not only do you need to access DSI PLL registers, you also need to use DSI registers. Is it possible to have the driver create its own clock structs when it's loaded, and have the code for that clock inside the driver, or are clocks something that has to be handled in the core platform code? Tomi -- 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
Re: OMAP4 DSS clock setup
Hi On Mon, 11 Apr 2011, Tomi Valkeinen wrote: On Fri, 2011-04-08 at 10:28 -0600, Paul Walmsley wrote: On Fri, 8 Apr 2011, Tomi Valkeinen wrote: Also, I hope you and the other DSS hackers can finish the PM runtime conversion of the DSS driver soon. Ideally before any new DSS features are added. We really need to be able to separate the OMAP integration details from the drivers, and right now, hwmod and PM runtime are the best way we have to do that. I also started to look at the PM runtime, but it doesn't fix the issue with the inconsistent clock name I described above. After the hwmod/PM runtime conversion, I don't think any of the clock aliases currently in clock*_data.c should be used by the DSS driver (or by anything else on the system, for that matter). That's because the omap_device code should set up the main alias for the DSS main When should main clock be used by the driver? Or never, and pm_runtime handles that? If the DSS needs to change the parent or the clock rate of the main clock, then it will need to clk_get(dev, main) and use the clock API. My only point here was that the main alias should be automatically added by the omap_device code, not via the clock aliases in clock*_data.c. How about OMAP3? It also has an optional functional clock, but that doesn't seem to be set in dss_opt_clocks in omap_hwmod_3xxx_data.c. Should it be there? Probably so. It seems dss1_alwon_fclk is currently the main clock for OMAP3 dss hwmods. Does that mean it can never be turned off if DSS is in use? If the DSS driver were using PM runtime, then yes, that would be true. Ok. Then we need omap_hwmod_opt_clk for at least for VENC and HDMI, I believe. In terms of the VENC, I'd agree with that -- based on OMAP4 Public TRM Rev O Figure 10-177 Video Encoder Overview, it looks like VENC uses DSS_TV_FCLK. In terms of the HDMI IP block, based on OMAP4 Public TRM Rev O Figure 10-159 HDMI Overview, it looks like sys_clk should be one of the optional clocks for HDMI? Hard to tell from that diagram. - Should every DSS module have their own PM runtime handling? (actually related to the question above) Yes, I think so. From the integration perspective, we are trying to get to the point where each omap_device maps to only one hwmod. - If the modules are handled separately, how should the dependencies be handled? For example, dss_core's reset will reset all the other modules also, and most of the submodules need functions from dss_core and dss_dispc. So should, say, dss_dsi then call functions in core and dispc to get them, i.e. increase their pm runtime use count? Probably not. Here is how I would suggest structuring the code. This is only a naïve suggestion; you and your team almost certainly know better than I. I'd suggest that you separate low-level register access into .c files that are targeted specifically for the IP block. So in the DSS case, you'd have dss.c, dispc.c, dsi.c, hdmi.c, rfbi.c, venc.c. Each one should be a separate platform_device and would export symbols. Hopefully, there should be no cross-dependencies between these low-level files. Then you'd have higher-level code that might need to read/write from multiple DSS submodules to complete some higher-level operation. That would be at least one separate driver -- say, dss2 or something -- with dependencies on the low-level drivers. So, for example, when that higher-level driver is modprobe'd, Linux would also load the drivers for the underlying hardware blocks that it uses. But this still leaves my question open: If this dss2 needs to use, say, dispc.c and dsi.c, those low level drivers need to be enabled. So I guess we would have something like dispc_get() or dispc_enable(), which dss2 would call first. Those functions would then use pm_runtime to enable the HW module. And the higher level driver would keep the low level devices enabled while its doing something. That makes sense to me. I have been thinking something like this also earlier. It would separate things cleanly. One thing I don't like about it is the big amount of low level DSS internal functions that need to be exported. Yeah, that's one of the downsides of that approach. - There seems to be some child/parent relationships in PM runtime. Should dss_core be the parent for the rest of the DSS modules? This would at least solve the reset issue easily, I guess. Yes, I think that's more accurate, anyway. Something isn't right with the How are the child/parent relationships done for omap_devices? Does it come somehow from the hwmod data? Right now, there's no explicit support in omap_device for parent/child relationships. Probably the right place for that is in the omap_hwmod layer, since omap_device code just maps the hwmod data into the Linux device/driver model. There is an
Re: OMAP4 DSS clock setup
Hi On Mon, 11 Apr 2011, Tomi Valkeinen wrote: On Fri, 2011-04-08 at 08:55 -0600, Paul Walmsley wrote: On Fri, 8 Apr 2011, Tomi Valkeinen wrote: and not the clock used for the pixel clock? If so, I'm fine with having fck to be what it is currently, but then we need a new name for the clock used for pixel clock, which is consistent on all platforms. If there is a separate PRCM-provided clock used only for the pixel clock, then that clock should have an alias name of system_pixel_ck or something similar that is meaningful to the DSS driver. I think the problem in this case is that dss_dss_clk is (optionally) used for two purposes: optionally as a main PRCM-provided functional clock and optionally as a system-provided pixel clock. Not only for pixel clock, but in the end it'll come out as pixel clock. I'm not sure what exactly is a functional clock here. I mean, one could think it as a basic functionality of DSS to read the pixels, manipulate them, and output them (with the rate of the pixel clock). Broadly speaking, a functional clock is defined negatively - 'anything that's not an interface clock.' That's why the term 'functional clock' is not so useful by itself. But when we talk about a module's 'main functional clock,' that means the functional clock that is needed for register accesses to work. However, I think there is one difference between the clock used just to enable the DSS registers, and the one used to output pixels: we need to be able to adjust the rate of the clock. Thus we need to have a common (omap2/3/4) clock name for it to be able to clk_get() it. Should that clock name be just the main clock provided automatically, or something else? Are you referring here to the system DPLL and its output dividers, or are you referring to the DSS module's internal dividers? - Paul -- 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
Re: OMAP4 DSS clock setup
On 4/11/2011 6:05 PM, Paul Walmsley wrote: Hi On Mon, 11 Apr 2011, Tomi Valkeinen wrote: On Fri, 2011-04-08 at 10:28 -0600, Paul Walmsley wrote: On Fri, 8 Apr 2011, Tomi Valkeinen wrote: Also, I hope you and the other DSS hackers can finish the PM runtime conversion of the DSS driver soon. Ideally before any new DSS features are added. We really need to be able to separate the OMAP integration details from the drivers, and right now, hwmod and PM runtime are the best way we have to do that. I also started to look at the PM runtime, but it doesn't fix the issue with the inconsistent clock name I described above. After the hwmod/PM runtime conversion, I don't think any of the clock aliases currently in clock*_data.c should be used by the DSS driver (or by anything else on the system, for that matter). That's because the omap_device code should set up the main alias for the DSS main When should main clock be used by the driver? Or never, and pm_runtime handles that? If the DSS needs to change the parent or the clock rate of the main clock, then it will need to clk_get(dev, main) and use the clock API. My only point here was that the main alias should be automatically added by the omap_device code, not via the clock aliases in clock*_data.c. How about OMAP3? It also has an optional functional clock, but that doesn't seem to be set in dss_opt_clocks in omap_hwmod_3xxx_data.c. Should it be there? Probably so. It seems dss1_alwon_fclk is currently the main clock for OMAP3 dss hwmods. Does that mean it can never be turned off if DSS is in use? If the DSS driver were using PM runtime, then yes, that would be true. Ok. Then we need omap_hwmod_opt_clk for at least for VENC and HDMI, I believe. In terms of the VENC, I'd agree with that -- based on OMAP4 Public TRM Rev O Figure 10-177 Video Encoder Overview, it looks like VENC uses DSS_TV_FCLK. In terms of the HDMI IP block, based on OMAP4 Public TRM Rev O Figure 10-159 HDMI Overview, it looks like sys_clk should be one of the optional clocks for HDMI? Hard to tell from that diagram. - Should every DSS module have their own PM runtime handling? (actually related to the question above) Yes, I think so. From the integration perspective, we are trying to get to the point where each omap_device maps to only one hwmod. - If the modules are handled separately, how should the dependencies be handled? For example, dss_core's reset will reset all the other modules also, and most of the submodules need functions from dss_core and dss_dispc. So should, say, dss_dsi then call functions in core and dispc to get them, i.e. increase their pm runtime use count? Probably not. Here is how I would suggest structuring the code. This is only a naïve suggestion; you and your team almost certainly know better than I. I'd suggest that you separate low-level register access into .c files that are targeted specifically for the IP block. So in the DSS case, you'd have dss.c, dispc.c, dsi.c, hdmi.c, rfbi.c, venc.c. Each one should be a separate platform_device and would export symbols. Hopefully, there should be no cross-dependencies between these low-level files. Then you'd have higher-level code that might need to read/write from multiple DSS submodules to complete some higher-level operation. That would be at least one separate driver -- say, dss2 or something -- with dependencies on the low-level drivers. So, for example, when that higher-level driver is modprobe'd, Linux would also load the drivers for the underlying hardware blocks that it uses. But this still leaves my question open: If this dss2 needs to use, say, dispc.c and dsi.c, those low level drivers need to be enabled. So I guess we would have something like dispc_get() or dispc_enable(), which dss2 would call first. Those functions would then use pm_runtime to enable the HW module. And the higher level driver would keep the low level devices enabled while its doing something. That makes sense to me. I have been thinking something like this also earlier. It would separate things cleanly. One thing I don't like about it is the big amount of low level DSS internal functions that need to be exported. Yeah, that's one of the downsides of that approach. - There seems to be some child/parent relationships in PM runtime. Should dss_core be the parent for the rest of the DSS modules? This would at least solve the reset issue easily, I guess. Yes, I think that's more accurate, anyway. Something isn't right with the How are the child/parent relationships done for omap_devices? Does it come somehow from the hwmod data? Right now, there's no explicit support in omap_device for parent/child relationships. Probably the right place for that is in the omap_hwmod layer, since omap_device code just maps the hwmod data into the Linux device/driver model. There is an interconnect hierarchy that's encoded in the omap_hwmod data, but currently there's no explicit handling for a case
Re: OMAP4 DSS clock setup
On 4/11/2011 6:05 PM, Paul Walmsley wrote: Hi On Mon, 11 Apr 2011, Tomi Valkeinen wrote: On Fri, 2011-04-08 at 10:28 -0600, Paul Walmsley wrote: On Fri, 8 Apr 2011, Tomi Valkeinen wrote: Also, I hope you and the other DSS hackers can finish the PM runtime conversion of the DSS driver soon. Ideally before any new DSS features are added. We really need to be able to separate the OMAP integration details from the drivers, and right now, hwmod and PM runtime are the best way we have to do that. I also started to look at the PM runtime, but it doesn't fix the issue with the inconsistent clock name I described above. After the hwmod/PM runtime conversion, I don't think any of the clock aliases currently in clock*_data.c should be used by the DSS driver (or by anything else on the system, for that matter). That's because the omap_device code should set up the main alias for the DSS main When should main clock be used by the driver? Or never, and pm_runtime handles that? If the DSS needs to change the parent or the clock rate of the main clock, then it will need to clk_get(dev, main) and use the clock API. My only point here was that the main alias should be automatically added by the omap_device code, not via the clock aliases in clock*_data.c. That will be doable only when main_clk will not be mapped to modulemode. You cannot change the clock rate of the modulemode since it is a fake clock :-( How about OMAP3? It also has an optional functional clock, but that doesn't seem to be set in dss_opt_clocks in omap_hwmod_3xxx_data.c. Should it be there? Probably so. It seems dss1_alwon_fclk is currently the main clock for OMAP3 dss hwmods. Does that mean it can never be turned off if DSS is in use? If the DSS driver were using PM runtime, then yes, that would be true. And then even if we switch to dss2 as the functional clock, dss1 cannot be gated. Ok. Then we need omap_hwmod_opt_clk for at least for VENC and HDMI, I believe. In terms of the VENC, I'd agree with that -- based on OMAP4 Public TRM Rev O Figure 10-177 Video Encoder Overview, it looks like VENC uses DSS_TV_FCLK. If we do have an hwmod for dss_venc, we might consider the tv_clk as the main clock, which is the case anyway. The only tricky part is the dependency with the dss modulemode / fclk that have to be enabled first. In terms of the HDMI IP block, based on OMAP4 Public TRM Rev O Figure 10-159 HDMI Overview, it looks like sys_clk should be one of the optional clocks for HDMI? Hard to tell from that diagram. - Should every DSS module have their own PM runtime handling? (actually related to the question above) Yes, I think so. From the integration perspective, we are trying to get to the point where each omap_device maps to only one hwmod. - If the modules are handled separately, how should the dependencies be handled? For example, dss_core's reset will reset all the other modules also, and most of the submodules need functions from dss_core and dss_dispc. So should, say, dss_dsi then call functions in core and dispc to get them, i.e. increase their pm runtime use count? Probably not. Here is how I would suggest structuring the code. This is only a naïve suggestion; you and your team almost certainly know better than I. I'd suggest that you separate low-level register access into .c files that are targeted specifically for the IP block. So in the DSS case, you'd have dss.c, dispc.c, dsi.c, hdmi.c, rfbi.c, venc.c. Each one should be a separate platform_device and would export symbols. Hopefully, there should be no cross-dependencies between these low-level files. Then you'd have higher-level code that might need to read/write from multiple DSS submodules to complete some higher-level operation. That would be at least one separate driver -- say, dss2 or something -- with dependencies on the low-level drivers. So, for example, when that higher-level driver is modprobe'd, Linux would also load the drivers for the underlying hardware blocks that it uses. But this still leaves my question open: If this dss2 needs to use, say, dispc.c and dsi.c, those low level drivers need to be enabled. So I guess we would have something like dispc_get() or dispc_enable(), which dss2 would call first. Those functions would then use pm_runtime to enable the HW module. And the higher level driver would keep the low level devices enabled while its doing something. That makes sense to me. I have been thinking something like this also earlier. It would separate things cleanly. One thing I don't like about it is the big amount of low level DSS internal functions that need to be exported. Yeah, that's one of the downsides of that approach. - There seems to be some child/parent relationships in PM runtime. Should dss_core be the parent for the rest of the DSS modules? This would at least solve the reset issue easily, I guess. Yes, I think that's more accurate, anyway. Something isn't right with the How
Re: OMAP4 DSS clock setup
Hi Paul, On 4/7/2011 9:27 PM, Paul Walmsley wrote: Hello Tomi, Benoît, On Mon, 4 Apr 2011, Tomi Valkeinen wrote: On Fri, 2011-04-01 at 20:12 -0600, Paul Walmsley wrote: Based on the E-mail thread so far, I'd say that changing the clock aliases is the way to go for right now. The clock aliases are not hardware data; they just control how the clock hardware is mapped to the drivers. I'd very much prefer this option. Below is a patch for this. If Benoit doesn't complain too much about this, I'd like to get this merged as soon as possible, as OMAP4 DSS is currently crashing in the mainline kernel. I will queue your clock aliases patch and attempt to merge it upstream for the -rc series. I am not happy to be disagreeing with Benoît here, but this is the rationale that I am using. (Benoît, Tomi, please feel free to correct if there is something blatantly false below.) That's quite good that you were disagreeing with me, because I think that you are indeed right:-) 1. The integration of the DSS module is an unusual case. The MODULEMODE bits for the DSS bits do not control the DSS main functional clock (the clock that is needed to access device registers); instead, they only control the DSS interface clock. (This is because the DSS can use a functional clock that is not provided by the OMAP core.) So calling the DSS MODULEMODE struct clk dss_fck is not really correct, since it is really controlling the interface clock. I've just got the confirmation from a PRCM designer that this is indeed what is happening. In the case of the DSS the optional functional clocks are provided by the PRCM and thus managed by the PRCM. The only difference is that since two different functional clocks are available, the PRCM cannot chose automatically the proper one. Hence the term optional fck, meaning that the SW has to explicitly enable them. The PRCM will not do it when modulemode will be enabled. So in that case enabling the modulemode will effectively enable the module for the PRCM point of view and just request the iclk if not already available. The only point that we need to take care of is the sequence. The PRCM spec clearly specify that one of the optional clock must be active before the modulemode is enabled. That does not seems to be the case in the current DSS code. 2. This patch does not create a precedent for hacking around general driver clocking problems in the clock or clock data. This is only needed because the MODULEMODE bits don't control the module functional clock in this case. As I understand it, the only other device that this problem could occur with is McBSP, due to mcbsp_clks. In that case this is slightly different because even the PRCM does not control these external clocks. Whereas in the case of the dss_fck, if the DPLL is not locked when you request it, it will be enabled automatically. Assuming that auto mode are enabled. 3. The existing DSS2 driver clock design apparently works fine when this change is implemented[1], so no driver changes will be needed. Yeah, but my point was that driver changes will be needed anyway, hence my preference to hack the driver instead of hacking the clock data. 4. The proposed DSS driver patch to work around this uses a nasty hack that really should be NACK'ed[2]. All this said, we may not be able to merge this change upstream, due to the recent unhappiness about the clock data changes. If Linus rejects it, you'll need a Plan B. That's was my #2 concern as well. See you soon at ELC. 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
Re: OMAP4 DSS clock setup
Hi On Fri, 8 Apr 2011, Tomi Valkeinen wrote: On Thu, 2011-04-07 at 13:27 -0600, Paul Walmsley wrote: On Mon, 4 Apr 2011, Tomi Valkeinen wrote: On Fri, 2011-04-01 at 20:12 -0600, Paul Walmsley wrote: Based on the E-mail thread so far, I'd say that changing the clock aliases is the way to go for right now. The clock aliases are not hardware data; they just control how the clock hardware is mapped to the drivers. I'd very much prefer this option. Below is a patch for this. If Benoit doesn't complain too much about this, I'd like to get this merged as soon as possible, as OMAP4 DSS is currently crashing in the mainline kernel. I will queue your clock aliases patch and attempt to merge it upstream for the -rc series. I am not happy to be disagreeing with Benoît here, but this is the rationale that I am using. (Benoît, Tomi, please feel free to correct if there is something blatantly false below.) 1. The integration of the DSS module is an unusual case. The MODULEMODE bits for the DSS bits do not control the DSS main functional clock (the clock that is needed to access device registers); instead, they only control the DSS interface clock. (This is because the DSS can use a functional clock that is not provided by the OMAP core.) So calling the DSS MODULEMODE struct clk dss_fck is not really correct, since it is really controlling the interface clock. If we don't apply the patch, I think the clock aliases are still broken. Let's forget the ick/fck thing for a second, and just think the clock from PRCM which is used as the source clock for pixel clock. That is currently called fck on OMAP2/3, but dss_dss_clk on OMAP4. This cannot be right, can it? From DSS's point of view that is the same clock, it should clearly have the same alias on all platforms. I don't care what the name is as long as it's consistent on all platforms. Yes, I agree. That is another good point. And I have to say I still don't quite get it what is the problem with fck name. After the hwmod/PM runtime conversion, there shouldn't be any clock aliases left that are simply called fck, because it is almost a meaningless name. Or is that meant to be a clock which enables the registers on the module, After the hwmod/PM runtime conversion, that should have an alias name of main that is automatically added by the omap_device code. (Note that it does not appear to do so now; that is a bug that needs be fixed.) and not the clock used for the pixel clock? If so, I'm fine with having fck to be what it is currently, but then we need a new name for the clock used for pixel clock, which is consistent on all platforms. If there is a separate PRCM-provided clock used only for the pixel clock, then that clock should have an alias name of system_pixel_ck or something similar that is meaningful to the DSS driver. I think the problem in this case is that dss_dss_clk is (optionally) used for two purposes: optionally as a main PRCM-provided functional clock and optionally as a system-provided pixel clock. I'll reply to the rest of your mail in a separate E-mail... - Paul
Re: OMAP4 DSS clock setup
Hi Tomi, On 4/8/2011 7:51 AM, Valkeinen, Tomi wrote: Hi Paul, On Thu, 2011-04-07 at 13:27 -0600, Paul Walmsley wrote: Hello Tomi, Benoît, On Mon, 4 Apr 2011, Tomi Valkeinen wrote: On Fri, 2011-04-01 at 20:12 -0600, Paul Walmsley wrote: Based on the E-mail thread so far, I'd say that changing the clock aliases is the way to go for right now. The clock aliases are not hardware data; they just control how the clock hardware is mapped to the drivers. I'd very much prefer this option. Below is a patch for this. If Benoit doesn't complain too much about this, I'd like to get this merged as soon as possible, as OMAP4 DSS is currently crashing in the mainline kernel. I will queue your clock aliases patch and attempt to merge it upstream for the -rc series. I am not happy to be disagreeing with Benoît here, but this is the rationale that I am using. (Benoît, Tomi, please feel free to correct if there is something blatantly false below.) 1. The integration of the DSS module is an unusual case. The MODULEMODE bits for the DSS bits do not control the DSS main functional clock (the clock that is needed to access device registers); instead, they only control the DSS interface clock. (This is because the DSS can use a functional clock that is not provided by the OMAP core.) So calling the DSS MODULEMODE struct clk dss_fck is not really correct, since it is really controlling the interface clock. If we don't apply the patch, I think the clock aliases are still broken. Let's forget the ick/fck thing for a second, and just think the clock from PRCM which is used as the source clock for pixel clock. That is currently called fck on OMAP2/3, but dss_dss_clk on OMAP4. This cannot be right, can it? From DSS's point of view that is the same clock, it should clearly have the same alias on all platforms. I don't care what the name is as long as it's consistent on all platforms. And I have to say I still don't quite get it what is the problem with fck name. Or is that meant to be a clock which enables the registers on the module, and not the clock used for the pixel clock? If so, I'm fine with having fck to be what it is currently, but then we need a new name for the clock used for pixel clock, which is consistent on all platforms. Both can be used for the system clock (sys_clk - DSI DPLL or dss_dss_clk). In fact after multiple discussions with DSS and PRCM folks, I realized that OMAP3 and 4 are using exactly the same clock scheme :-( The confusion in my mind came from change in naming convention in both the PRCM and the DSS at the same time between OMAP3 and 4. OMAP3 - OMAP4 dss1_alwon_fck - dss_dss_clk (from dpll per output in both cases) dss2_alwon_fck - dss_sys_clk dss_ick- dss_fck (- modulemode) that name really needs to be changed to avoid further confusion. So in fact the OMAP4 aliased should be: CLK(omapdss_dss, fck, dss_dss_clk, CK_443X), CLK(omapdss_dss, sys_clk, dss_sys_clk, CK_443X), CLK(omapdss_dss, ick, dss_fck, CK_443X), That will map perfectly what we have on OMAP3: CLK(omapdss_dss, fck, dss1_alwon_fck_3430es2, ...), CLK(omapdss_dss, sys_clk, dss2_alwon_fck, ...), CLK(omapdss_dss, ick, dss_ick_3430es2, ...), 2. This patch does not create a precedent for hacking around general driver clocking problems in the clock or clock data. This is only needed because the MODULEMODE bits don't control the module functional clock in this case. As I understand it, the only other device that this problem could occur with is McBSP, due to mcbsp_clks. 3. The existing DSS2 driver clock design apparently works fine when this change is implemented[1], so no driver changes will be needed. 4. The proposed DSS driver patch to work around this uses a nasty hack that really should be NACK'ed[2]. All this said, we may not be able to merge this change upstream, due to the recent unhappiness about the clock data changes. If Linus rejects it, you'll need a Plan B. ... Also, I hope you and the other DSS hackers can finish the PM runtime conversion of the DSS driver soon. Ideally before any new DSS features are added. We really need to be able to separate the OMAP integration details from the drivers, and right now, hwmod and PM runtime are the best way we have to do that. I also started to look at the PM runtime, but it doesn't fix the issue with the inconsistent clock name I described above. No indeed. I also have some questions regarding PM runtime, perhaps I'll just put them here as they are slightly related: - Should every DSS module handle their clocks independently, i.e. should VENC get its own clocks and DSI should get its own? If so, we need a bunch of new clock aliases so the devices can get their clocks. For dedicated clock like tv_clk, it probably makes sense, because
Re: OMAP4 DSS clock setup
Hi Tomi,
On Fri, 8 Apr 2011, Tomi Valkeinen wrote:
Also, I hope you and the other DSS hackers can finish the PM runtime
conversion of the DSS driver soon. Ideally before any new DSS
features are added. We really need to be able to separate the OMAP
integration details from the drivers, and right now, hwmod and PM
runtime are the best way we have to do that.
I also started to look at the PM runtime, but it doesn't fix the issue
with the inconsistent clock name I described above.
After the hwmod/PM runtime conversion, I don't think any of the clock
aliases currently in clock*_data.c should be used by the DSS driver (or by
anything else on the system, for that matter). That's because the
omap_device code should set up the main alias for the DSS main
functional clock[*], as well as the aliases in the optional clock data in
the OMAP hwmod data files:
static struct omap_hwmod_opt_clk dss_opt_clks[] = {
{ .role = sys_clk, .clk = dss_sys_clk },
{ .role = tv_clk, .clk = dss_tv_clk },
{ .role = dss_clk, .clk = dss_dss_clk },
{ .role = video_clk, .clk = dss_48mhz_clk },
};
It might be that some of these role names aren't quite accurate and need
to be changed. Those are intended to be meaningful to the driver, so
comments there are definitely welcome.
[*]. The main alias should be defined by the omap_device code
automatically, similarly to how _add_optional_clock_clkdev() does it. It
does not do so currently. This needs to be fixed in the omap_device.c
code.
I also have some questions regarding PM runtime, perhaps I'll just put
them here as they are slightly related:
- Should every DSS module handle their clocks independently, i.e. should
VENC get its own clocks and DSI should get its own? If so, we need a
bunch of new clock aliases so the devices can get their clocks.
If all that driver code needs to do is to enable its main functional clock
when it is active and disable that clock when the driver is inactive,
then, no, the drivers shouldn't need their own clock aliases. Same if the
driver only needs to get the rate or set the rate on that main functional
clock, since that alias should be set up automatically.
But if the driver for that submodule needs to control PRCM-provided
optional clocks, then it will need to have struct omap_hwmod_opt_clk
entries defined in the hwmod data.
- Should every DSS module have their own PM runtime handling? (actually
related to the question above)
Yes, I think so. From the integration perspective, we are trying to get
to the point where each omap_device maps to only one hwmod.
- If the modules are handled separately, how should the dependencies be
handled? For example, dss_core's reset will reset all the other modules
also, and most of the submodules need functions from dss_core and
dss_dispc. So should, say, dss_dsi then call functions in core and dispc
to get them, i.e. increase their pm runtime use count?
Probably not.
Here is how I would suggest structuring the code. This is only a naïve
suggestion; you and your team almost certainly know better than I.
I'd suggest that you separate low-level register access into .c files
that are targeted specifically for the IP block. So in the DSS case,
you'd have dss.c, dispc.c, dsi.c, hdmi.c, rfbi.c, venc.c. Each one should
be a separate platform_device and would export symbols. Hopefully, there
should be no cross-dependencies between these low-level files.
Then you'd have higher-level code that might need to read/write from
multiple DSS submodules to complete some higher-level operation. That
would be at least one separate driver -- say, dss2 or something -- with
dependencies on the low-level drivers. So, for example, when that
higher-level driver is modprobe'd, Linux would also load the drivers for
the underlying hardware blocks that it uses.
- There seems to be some child/parent relationships in PM runtime.
Should dss_core be the parent for the rest of the DSS modules? This
would at least solve the reset issue easily, I guess.
Yes, I think that's more accurate, anyway. Something isn't right with the
DSS hwmod data. According to the OMAP4 Public TRM Rev O Table 10-3 DSS
Integration, there's a Sonics LX bus lurking in there. All of the DSS
submodules should have slave sockets with that Sonics LX bus as their
master. And the hwmod associated with the SLX should have an address
range that covers all of the DSS submodules. I assume that the logical
hwmod to associate the SLX with is dss_core, as you write.
Also, I notice the CAUTION boxes in Section 10.1.3 DSS Register
Manual, 10.2.7 Display Controller Register Manual, etc. etc., that say
that the DSS and submodules should be accessed through the L3 address
space. But all of the DSS hwmod register targets are listed as the L4_PER
variants. So the hwmod data also doesn't appear to be correct there. The
correct approach would be to have both address
Re: OMAP4 DSS clock setup
On Fri, 2011-04-08 at 17:36 +0200, Cousson, Benoit wrote: Hi Tomi, On 4/8/2011 7:51 AM, Valkeinen, Tomi wrote: - If the modules are handled separately, how should the dependencies be handled? For example, dss_core's reset will reset all the other modules also, and most of the submodules need functions from dss_core and dss_dispc. So should, say, dss_dsi then call functions in core and dispc to get them, i.e. increase their pm runtime use count? Are you sure about that? The dss_core does not have any reset in the sysonfig (only a status), but the dispc does have one and the dsi as well. Ah, I might be speaking only of OMAP2/3. I remember somebody saying that the DSS reset bit on OMAP4 is marked as reserved. But for OMAP3 (and I think for OMAP2 also) the DSS_SYSCONFIG reset bit will reset also the rest of the DSS. Tomi -- 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
Re: OMAP4 DSS clock setup
Hi Tomi,
On Mon, 4 Apr 2011, Tomi Valkeinen wrote:
On Fri, 2011-04-01 at 20:12 -0600, Paul Walmsley wrote:
Of course, at some point soon, those clock aliases are going to go away.
But hopefully you all will have converted the driver over to PM runtime at
that point.
Once that happens, there's another problem: omap_hwmod_enable() is defined
that the hardware registers should be accessible by the MPU after it
completes. So by that definition, the hwmod code should be
enabling/disabling that dss_clk clock too when it enables/idles/shuts down
the hwmod. Probably we'd need to mark that struct omap_hwmod_opt_clk with
some flag. Then we'd need some kind of way for the DSS to tell the hwmod
code whether it is or isn't reliant on the PRCM-provided functional clock
for its internal functional clock. Maybe something like
omap_hwmod_{release,require}_system_fclk()?
Hmm, right. I guess no other HW module has clock setups like this?
I think McBSP can have an optional main functional clock. So something
like that should be usable for that case too. (In the McBSP case, the
optional clock isn't controlled by the PRCM, it's controlled by the SCM,
but that doesn't really matter to the hwmod code.)
Currently DSS can use clk_enable/disable() for the system fclk when its
using DSI PLL for the fclk. So omap_hwmod_{release,require}_system_fclk
sounds like a simple solution to this.
OK. The only problem with those functions (actually
omap_device_{release,require}_system_fclk()) is that they will need to be
called through platform_data function pointers for now. Maybe it is
possible to handle something like this simply with the clock framework
also.
Not directly related, but something I've been wondering about is how to
abstract the DSI/HDMI PLLs in DSS. What do you think, would it be
possible/worth it to create struct clks for the clocks coming out of
those PLLs? These would, of course, be DSS internal clks. I'm not very
familiar with the clock framework, so I don't really have any idea what
this would require and what would be the pros and cons.
Yes, I think it would be good to try to implement the entire DSS clock
tree in the clock framework. One change to the clock code that we know
we'll need is to put a hwmod pointer in the struct clk which tells the
clock code that the hwmod needs to be enabled in order to access the
clock's registers. Right now, the clock code assumes that all of the
clock registers are accessible, all of the time.
- Paul
--
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
Re: OMAP4 DSS clock setup
Hello Tomi, Benoît, On Mon, 4 Apr 2011, Tomi Valkeinen wrote: On Fri, 2011-04-01 at 20:12 -0600, Paul Walmsley wrote: Based on the E-mail thread so far, I'd say that changing the clock aliases is the way to go for right now. The clock aliases are not hardware data; they just control how the clock hardware is mapped to the drivers. I'd very much prefer this option. Below is a patch for this. If Benoit doesn't complain too much about this, I'd like to get this merged as soon as possible, as OMAP4 DSS is currently crashing in the mainline kernel. I will queue your clock aliases patch and attempt to merge it upstream for the -rc series. I am not happy to be disagreeing with Benoît here, but this is the rationale that I am using. (Benoît, Tomi, please feel free to correct if there is something blatantly false below.) 1. The integration of the DSS module is an unusual case. The MODULEMODE bits for the DSS bits do not control the DSS main functional clock (the clock that is needed to access device registers); instead, they only control the DSS interface clock. (This is because the DSS can use a functional clock that is not provided by the OMAP core.) So calling the DSS MODULEMODE struct clk dss_fck is not really correct, since it is really controlling the interface clock. 2. This patch does not create a precedent for hacking around general driver clocking problems in the clock or clock data. This is only needed because the MODULEMODE bits don't control the module functional clock in this case. As I understand it, the only other device that this problem could occur with is McBSP, due to mcbsp_clks. 3. The existing DSS2 driver clock design apparently works fine when this change is implemented[1], so no driver changes will be needed. 4. The proposed DSS driver patch to work around this uses a nasty hack that really should be NACK'ed[2]. All this said, we may not be able to merge this change upstream, due to the recent unhappiness about the clock data changes. If Linus rejects it, you'll need a Plan B. ... Also, I hope you and the other DSS hackers can finish the PM runtime conversion of the DSS driver soon. Ideally before any new DSS features are added. We really need to be able to separate the OMAP integration details from the drivers, and right now, hwmod and PM runtime are the best way we have to do that. Comments welcome. - Paul 1. Valkeinen, Tomi. _Re: OMAP4 DSS clock setup_. E-mail to linux-omap@vger.kernel.org mailing list dated Wed, 30 Mar 2011 05:59:06 -0700. http://www.mail-archive.com/linux-omap@vger.kernel.org/msg47665.html 2. ibid.
Re: OMAP4 DSS clock setup
Hi Paul, On Thu, 2011-04-07 at 13:27 -0600, Paul Walmsley wrote: Hello Tomi, Benoît, On Mon, 4 Apr 2011, Tomi Valkeinen wrote: On Fri, 2011-04-01 at 20:12 -0600, Paul Walmsley wrote: Based on the E-mail thread so far, I'd say that changing the clock aliases is the way to go for right now. The clock aliases are not hardware data; they just control how the clock hardware is mapped to the drivers. I'd very much prefer this option. Below is a patch for this. If Benoit doesn't complain too much about this, I'd like to get this merged as soon as possible, as OMAP4 DSS is currently crashing in the mainline kernel. I will queue your clock aliases patch and attempt to merge it upstream for the -rc series. I am not happy to be disagreeing with Benoît here, but this is the rationale that I am using. (Benoît, Tomi, please feel free to correct if there is something blatantly false below.) 1. The integration of the DSS module is an unusual case. The MODULEMODE bits for the DSS bits do not control the DSS main functional clock (the clock that is needed to access device registers); instead, they only control the DSS interface clock. (This is because the DSS can use a functional clock that is not provided by the OMAP core.) So calling the DSS MODULEMODE struct clk dss_fck is not really correct, since it is really controlling the interface clock. If we don't apply the patch, I think the clock aliases are still broken. Let's forget the ick/fck thing for a second, and just think the clock from PRCM which is used as the source clock for pixel clock. That is currently called fck on OMAP2/3, but dss_dss_clk on OMAP4. This cannot be right, can it? From DSS's point of view that is the same clock, it should clearly have the same alias on all platforms. I don't care what the name is as long as it's consistent on all platforms. And I have to say I still don't quite get it what is the problem with fck name. Or is that meant to be a clock which enables the registers on the module, and not the clock used for the pixel clock? If so, I'm fine with having fck to be what it is currently, but then we need a new name for the clock used for pixel clock, which is consistent on all platforms. 2. This patch does not create a precedent for hacking around general driver clocking problems in the clock or clock data. This is only needed because the MODULEMODE bits don't control the module functional clock in this case. As I understand it, the only other device that this problem could occur with is McBSP, due to mcbsp_clks. 3. The existing DSS2 driver clock design apparently works fine when this change is implemented[1], so no driver changes will be needed. 4. The proposed DSS driver patch to work around this uses a nasty hack that really should be NACK'ed[2]. All this said, we may not be able to merge this change upstream, due to the recent unhappiness about the clock data changes. If Linus rejects it, you'll need a Plan B. ... Also, I hope you and the other DSS hackers can finish the PM runtime conversion of the DSS driver soon. Ideally before any new DSS features are added. We really need to be able to separate the OMAP integration details from the drivers, and right now, hwmod and PM runtime are the best way we have to do that. I also started to look at the PM runtime, but it doesn't fix the issue with the inconsistent clock name I described above. I also have some questions regarding PM runtime, perhaps I'll just put them here as they are slightly related: - Should every DSS module handle their clocks independently, i.e. should VENC get its own clocks and DSI should get its own? If so, we need a bunch of new clock aliases so the devices can get their clocks. - Should every DSS module have their own PM runtime handling? (actually related to the question above) - If the modules are handled separately, how should the dependencies be handled? For example, dss_core's reset will reset all the other modules also, and most of the submodules need functions from dss_core and dss_dispc. So should, say, dss_dsi then call functions in core and dispc to get them, i.e. increase their pm runtime use count? - There seems to be some child/parent relationships in PM runtime. Should dss_core be the parent for the rest of the DSS modules? This would at least solve the reset issue easily, I guess. - How does saving/restoring the registers for OFF mode go with PM runtime? Should the registers be saved in runtime_suspend(), and restored in runtime_resume()? Can/should omap_pm_get_dev_context_loss_count() still be used to optimize the restoring? Tomi -- 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
Re: OMAP4 DSS clock setup
Paul, Benoit,
On Mon, 2011-04-04 at 09:53 +0300, Tomi Valkeinen wrote:
Hi Paul, Benoit,
On Fri, 2011-04-01 at 20:12 -0600, Paul Walmsley wrote:
Based on the E-mail thread so far, I'd say that changing the clock aliases
is the way to go for right now. The clock aliases are not hardware data;
they just control how the clock hardware is mapped to the drivers.
I'd very much prefer this option. Below is a patch for this.
If Benoit doesn't complain too much about this, I'd like to get this
merged as soon as possible, as OMAP4 DSS is currently crashing in the
mainline kernel.
I can either handle it myself if I get your acks, or you can send a pull
request for this if you have some other patches going in also.
Ping. Can I get an ack/nack from you for the patch below?
Tomi
Of course, at some point soon, those clock aliases are going to go away.
But hopefully you all will have converted the driver over to PM runtime at
that point.
Once that happens, there's another problem: omap_hwmod_enable() is defined
that the hardware registers should be accessible by the MPU after it
completes. So by that definition, the hwmod code should be
enabling/disabling that dss_clk clock too when it enables/idles/shuts down
the hwmod. Probably we'd need to mark that struct omap_hwmod_opt_clk with
some flag. Then we'd need some kind of way for the DSS to tell the hwmod
code whether it is or isn't reliant on the PRCM-provided functional clock
for its internal functional clock. Maybe something like
omap_hwmod_{release,require}_system_fclk()?
Hmm, right. I guess no other HW module has clock setups like this?
Currently DSS can use clk_enable/disable() for the system fclk when its
using DSI PLL for the fclk. So omap_hwmod_{release,require}_system_fclk
sounds like a simple solution to this.
Not directly related, but something I've been wondering about is how to
abstract the DSI/HDMI PLLs in DSS. What do you think, would it be
possible/worth it to create struct clks for the clocks coming out of
those PLLs? These would, of course, be DSS internal clks. I'm not very
familiar with the clock framework, so I don't really have any idea what
this would require and what would be the pros and cons.
---
From fceb48b2e22217dccc85b33362b6a17e5a00 Mon Sep 17 00:00:00 2001
From: Tomi Valkeinen tomi.valkei...@ti.com
Date: Mon, 4 Apr 2011 09:26:19 +0300
Subject: [PATCH] OMAP4: clock data: Change DSS clock aliases
DSS driver has used fck and ick clocks on OMAP2/3 to get DSS HW up and
running, and also to get the pixel clock's source clock rate from the
fck.
On OMAP4 the clock data is set up in a different way, as there's no ick,
dss_fck points to a fake clock which just affects DSS's MODULEMODE, and
dss_dss_clk if the DSS_FCK.
From DSS driver's point of view the dss_fck sounds like an ick, and
dss_dss_clk is the fck. While this is not entirely correct from HW point
of view, especially for the ick, configuring the clock aliases that way
makes DSS just work with OMAP4's clock setup.
In the (hopefully near) future DSS driver will be reworked to use
pm_runtime support which should clean up the clock code.
Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
---
arch/arm/mach-omap2/clock44xx_data.c |9 ++---
1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-omap2/clock44xx_data.c
b/arch/arm/mach-omap2/clock44xx_data.c
index 276992d..8c96567 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -3116,14 +3116,9 @@ static struct omap_clk omap44xx_clks[] = {
CLK(NULL, dsp_fck, dsp_fck,
CK_443X),
CLK(omapdss_dss, sys_clk, dss_sys_clk,
CK_443X),
CLK(omapdss_dss, tv_clk, dss_tv_clk,
CK_443X),
- CLK(omapdss_dss, dss_clk, dss_dss_clk,
CK_443X),
CLK(omapdss_dss, video_clk,dss_48mhz_clk,
CK_443X),
- CLK(omapdss_dss, fck, dss_fck,
CK_443X),
- /*
- * On OMAP4, DSS ick is a dummy clock; this is needed for compatibility
- * with OMAP2/3.
- */
- CLK(omapdss_dss, ick, dummy_ck,
CK_443X),
+ CLK(omapdss_dss, fck, dss_dss_clk,
CK_443X),
+ CLK(omapdss_dss, ick, dss_fck,
CK_443X),
CLK(NULL, efuse_ctrl_cust_fck, efuse_ctrl_cust_fck,
CK_443X),
CLK(NULL, emif1_fck,emif1_fck,
CK_443X),
CLK(NULL, emif2_fck,emif2_fck,
CK_443X),
--
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
Re: OMAP4 DSS clock setup
Hi Paul, Benoit,
On Fri, 2011-04-01 at 20:12 -0600, Paul Walmsley wrote:
Based on the E-mail thread so far, I'd say that changing the clock aliases
is the way to go for right now. The clock aliases are not hardware data;
they just control how the clock hardware is mapped to the drivers.
I'd very much prefer this option. Below is a patch for this.
If Benoit doesn't complain too much about this, I'd like to get this
merged as soon as possible, as OMAP4 DSS is currently crashing in the
mainline kernel.
I can either handle it myself if I get your acks, or you can send a pull
request for this if you have some other patches going in also.
Of course, at some point soon, those clock aliases are going to go away.
But hopefully you all will have converted the driver over to PM runtime at
that point.
Once that happens, there's another problem: omap_hwmod_enable() is defined
that the hardware registers should be accessible by the MPU after it
completes. So by that definition, the hwmod code should be
enabling/disabling that dss_clk clock too when it enables/idles/shuts down
the hwmod. Probably we'd need to mark that struct omap_hwmod_opt_clk with
some flag. Then we'd need some kind of way for the DSS to tell the hwmod
code whether it is or isn't reliant on the PRCM-provided functional clock
for its internal functional clock. Maybe something like
omap_hwmod_{release,require}_system_fclk()?
Hmm, right. I guess no other HW module has clock setups like this?
Currently DSS can use clk_enable/disable() for the system fclk when its
using DSI PLL for the fclk. So omap_hwmod_{release,require}_system_fclk
sounds like a simple solution to this.
Not directly related, but something I've been wondering about is how to
abstract the DSI/HDMI PLLs in DSS. What do you think, would it be
possible/worth it to create struct clks for the clocks coming out of
those PLLs? These would, of course, be DSS internal clks. I'm not very
familiar with the clock framework, so I don't really have any idea what
this would require and what would be the pros and cons.
---
From fceb48b2e22217dccc85b33362b6a17e5a00 Mon Sep 17 00:00:00 2001
From: Tomi Valkeinen tomi.valkei...@ti.com
Date: Mon, 4 Apr 2011 09:26:19 +0300
Subject: [PATCH] OMAP4: clock data: Change DSS clock aliases
DSS driver has used fck and ick clocks on OMAP2/3 to get DSS HW up and
running, and also to get the pixel clock's source clock rate from the
fck.
On OMAP4 the clock data is set up in a different way, as there's no ick,
dss_fck points to a fake clock which just affects DSS's MODULEMODE, and
dss_dss_clk if the DSS_FCK.
From DSS driver's point of view the dss_fck sounds like an ick, and
dss_dss_clk is the fck. While this is not entirely correct from HW point
of view, especially for the ick, configuring the clock aliases that way
makes DSS just work with OMAP4's clock setup.
In the (hopefully near) future DSS driver will be reworked to use
pm_runtime support which should clean up the clock code.
Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
---
arch/arm/mach-omap2/clock44xx_data.c |9 ++---
1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-omap2/clock44xx_data.c
b/arch/arm/mach-omap2/clock44xx_data.c
index 276992d..8c96567 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -3116,14 +3116,9 @@ static struct omap_clk omap44xx_clks[] = {
CLK(NULL, dsp_fck, dsp_fck,
CK_443X),
CLK(omapdss_dss, sys_clk, dss_sys_clk,
CK_443X),
CLK(omapdss_dss, tv_clk, dss_tv_clk,
CK_443X),
- CLK(omapdss_dss, dss_clk, dss_dss_clk,
CK_443X),
CLK(omapdss_dss, video_clk,dss_48mhz_clk,
CK_443X),
- CLK(omapdss_dss, fck, dss_fck,
CK_443X),
- /*
-* On OMAP4, DSS ick is a dummy clock; this is needed for compatibility
-* with OMAP2/3.
-*/
- CLK(omapdss_dss, ick, dummy_ck,
CK_443X),
+ CLK(omapdss_dss, fck, dss_dss_clk,
CK_443X),
+ CLK(omapdss_dss, ick, dss_fck,
CK_443X),
CLK(NULL, efuse_ctrl_cust_fck, efuse_ctrl_cust_fck,
CK_443X),
CLK(NULL, emif1_fck,emif1_fck,
CK_443X),
CLK(NULL, emif2_fck,emif2_fck,
CK_443X),
--
1.7.1
--
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
Re: OMAP4 DSS clock setup
Hi Tomi, Benoît, et al,
On Wed, 30 Mar 2011, Tomi Valkeinen wrote:
Currently we have these aliases for OMAP4:
dss_clk - dss_dss_clk
fck - dss_fck
ick - dummy_ck
If that would be changed to:
fck - dss_dss_clk
ick - dss_fck
The driver would work the same way for all OMAPs.
This looks reasonable to me, and seems to match the TRM's Figure 10-4 DSS
Clock Tree.
The current OMAP4 clock data name dss_fck is just kind of a fake name
for that clock in the OMAP4 context.
Anyway. To get things working for rc2 (DSS currently crashes due to not
enabling dss_clk) we need to either:
1) Change the clock aliases as above
2) Add something like Sumit's patch (attached) to handle dss_clk. While
the patch is not that complex, it feels rather hacky.
Yeah, that patch looks like a hack to me, especially something like
this:
+ if (pdata-opt_clock_available(dss_clk)) {
Based on the E-mail thread so far, I'd say that changing the clock aliases
is the way to go for right now. The clock aliases are not hardware data;
they just control how the clock hardware is mapped to the drivers.
Of course, at some point soon, those clock aliases are going to go away.
But hopefully you all will have converted the driver over to PM runtime at
that point.
Once that happens, there's another problem: omap_hwmod_enable() is defined
that the hardware registers should be accessible by the MPU after it
completes. So by that definition, the hwmod code should be
enabling/disabling that dss_clk clock too when it enables/idles/shuts down
the hwmod. Probably we'd need to mark that struct omap_hwmod_opt_clk with
some flag. Then we'd need some kind of way for the DSS to tell the hwmod
code whether it is or isn't reliant on the PRCM-provided functional clock
for its internal functional clock. Maybe something like
omap_hwmod_{release,require}_system_fclk()?
- Paul
Re: OMAP4 DSS clock setup
On Wednesday 30 March 2011 06:51 PM, Cousson, Benoit wrote: On 3/30/2011 2:58 PM, Valkeinen, Tomi wrote: On Wed, 2011-03-30 at 14:12 +0200, Cousson, Benoit wrote: On 3/30/2011 1:03 PM, Valkeinen, Tomi wrote: On Wed, 2011-03-30 at 11:32 +0200, Cousson, Benoit wrote: Hi Tomi, On 3/30/2011 8:48 AM, Valkeinen, Tomi wrote: Hi Benoit, Paul, I've been discussing with Sumit and Archit to understand how the DSS clocks are set up on OMAP4. I think I now have some idea how things work, but I'm still at loss why things are the way they are. So, if I look at OMAP4 TRM, Figure 10-4 DSS Clock Tree, there are two clocks in PRCM block that are relevant to this discussion: DSS_L3_ICLK and DSS_FCLK. To my understanding DSS_L3_ICLK is not really controllable, but it is affected by MODULEMODE bit. Then we have two relevant clocks defined in clock44xx_data.c: dss_fck and dss_dss_clk. dss_fck controls the MODULEMODE bit, and dss_dss_clk is the TRM's DSS_FCLK. Was that correct? Yes. For the moment, but this is not the final state. If so, from DSS driver's perspective, the dss_fck sounds very much like an interface clock (it's always needed when DSS is used) and dss_dss_clk sounds very much like functional clock (it's always needed, except if DSI PLL is used for DSS functional clock). dss_dss_fck is one of the possible functional clocks, hence the optional attribute. You can run the DSS only for sys_clk is you want. We can? I remember this was possible on OMAP2, but I can't seem to find it on OMAP4 TRM... Ah right, you mean sys_clk goes to DSI PLL, and the output from DSI PLL can be used as functional clock? Yes, you're right, maybe we can still use the sys_clk directly with a parallel QVGA LCD like on OMAP2:-) We _always_ need DSS_FCLK to get DSS up and running, and to configure DSI PLL if we want to get the clocks from there. So it's not quite that optional. But it's true that we can turn it off later. Just wanted to clarify the issue for myself and summarise: hwmod and pm runtime ensures that the MODULEMODE bits are set, and technically, that should be enough for a module to get up and running. But because of the strange design of DSS HW, we need an additional clock (DSS_CLK at bootup, could be something else later on) to access DSS registers. So we need to enable dss_dss_fclk in our driver in the beginning itself to access a register, hence Sumit's patch is needed. The strange thing is that if dss_dss_fclk is off(OMAP4430_OPTFCLKEN_DSSCLK bit is 0) it doesn't mean that the clock is surely OFF. Its only OFF when the DPLL per m5x2 divider allows HW auto-gating of the clock. So OPTCLKEN_DSSCLK is in a way a dummy bit which when set, ensures that the M5 divider doesn't auto gate. So it isn't exactly a enable/disable bit. In our tree(2.6.38-rc series), HW auto gating bit was disabled for m5x2 divider, and hence, it never mattered to us if OPTCLKEN_DSSCLK was enabled or disabled. We went on happily without bothering about this opt clock. When things got merged in mainline, the HW auto gating for m5x2 came into picture(HSDIVIDER_CLKOUT2_GATE_CTRL in CM_DIV_M5_DPLL_PER), and then suddenly dss_dss_fclk became super crucial, and a register access depended on it. This was the main reason of the confusion I guess. Archit Yes, that was confirmed by HW folks, as soon as you have one functional clock active, you can disable the other one. That's why the hwmod fmwk cannot choose for you which one you will need depending of your panel. It is up to the driver to select the correct one. But isn't that DSS internal thing? (I'm looking again at the DSS clock tree figure). DSS_FCLK from the PRCM should always be the same from DSS's point of view, it doesn't change. If we use the clock from DSI PLL, we'll get clock for LCD2_CLK, LCD1_CLK and DISPC_FCLK. But DSS_FCLK/DSS_CLK is always the same. Yes, but there is no other need for the DSS_CLK beside these internal nodes and the DSI engines. So you can shut it down as soon as an alternate clock is available (PLL1_CLK1 or PLL2_CLK1 in your case). If dss_fck would control DSS_FCLK and dss_ick would control MODULEMODE, they would be about the same as the clocks in OMAP2 and 3, and we wouldn't need any omap4 spesific trickery in the DSS driver. (dss_dss_clk wouldn't be needed). You cannot play with iclk, because this clock is supposed to be handled automatically by the HW. This was the case on OMAP23 as well BTW. Right. But now we have 1) dss_fck, which isn't the DSS_FCLK in TRM, and in fact not fck at all, or even clock at all 2) dss_dss_clk, a new clock whose name doesn't match to anything in TRM, and is in fact the DSS_FCLK. So they both sound like they are confusingly named. The naming convention is simple: opt clock are using the module name + the original clock name: dss_XXX, whereas the modulemode is using the module name + fck, because it is for the moment similar to the fclk we have on simpler module. iclk_en is not
Re: OMAP4 DSS clock setup
On Wed, 2011-03-30 at 15:21 +0200, Cousson, Benoit wrote: On 3/30/2011 2:58 PM, Valkeinen, Tomi wrote: Anyway. To get things working for rc2 (DSS currently crashes due to not enabling dss_clk) we need to either: 1) Change the clock aliases as above Changing the clock alias for my point of view is like hacking the HW definition to fit your need. Even if this is temporary, I do not like that approach. 2) Add something like Sumit's patch (attached) to handle dss_clk. While the patch is not that complex, it feels rather hacky. Yes, I'd rather hack that issue internally, because this is something you should fix as soon as you switch to PM runtime. What's your opinion? Definitively #2. Ok, let's go with that. I don't like it but hopefully pm_runtime will clear the mess from dss driver. Thanks for your time explaining the clock setup to me =) Tomi -- 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
Re: OMAP4 DSS clock setup
Hi Archit, On 3/31/2011 8:42 AM, Taneja, Archit wrote: On Wednesday 30 March 2011 06:51 PM, Cousson, Benoit wrote: On 3/30/2011 2:58 PM, Valkeinen, Tomi wrote: On Wed, 2011-03-30 at 14:12 +0200, Cousson, Benoit wrote: On 3/30/2011 1:03 PM, Valkeinen, Tomi wrote: On Wed, 2011-03-30 at 11:32 +0200, Cousson, Benoit wrote: Hi Tomi, On 3/30/2011 8:48 AM, Valkeinen, Tomi wrote: Hi Benoit, Paul, I've been discussing with Sumit and Archit to understand how the DSS clocks are set up on OMAP4. I think I now have some idea how things work, but I'm still at loss why things are the way they are. So, if I look at OMAP4 TRM, Figure 10-4 DSS Clock Tree, there are two clocks in PRCM block that are relevant to this discussion: DSS_L3_ICLK and DSS_FCLK. To my understanding DSS_L3_ICLK is not really controllable, but it is affected by MODULEMODE bit. Then we have two relevant clocks defined in clock44xx_data.c: dss_fck and dss_dss_clk. dss_fck controls the MODULEMODE bit, and dss_dss_clk is the TRM's DSS_FCLK. Was that correct? Yes. For the moment, but this is not the final state. If so, from DSS driver's perspective, the dss_fck sounds very much like an interface clock (it's always needed when DSS is used) and dss_dss_clk sounds very much like functional clock (it's always needed, except if DSI PLL is used for DSS functional clock). dss_dss_fck is one of the possible functional clocks, hence the optional attribute. You can run the DSS only for sys_clk is you want. We can? I remember this was possible on OMAP2, but I can't seem to find it on OMAP4 TRM... Ah right, you mean sys_clk goes to DSI PLL, and the output from DSI PLL can be used as functional clock? Yes, you're right, maybe we can still use the sys_clk directly with a parallel QVGA LCD like on OMAP2:-) We _always_ need DSS_FCLK to get DSS up and running, and to configure DSI PLL if we want to get the clocks from there. So it's not quite that optional. But it's true that we can turn it off later. Just wanted to clarify the issue for myself and summarise: hwmod and pm runtime ensures that the MODULEMODE bits are set, and technically, that should be enough for a module to get up and running. But because of the strange design of DSS HW, we need an additional clock (DSS_CLK at bootup, could be something else later on) to access DSS registers. So we need to enable dss_dss_fclk in our driver in the beginning itself to access a register, hence Sumit's patch is needed. The strange thing is that if dss_dss_fclk is off(OMAP4430_OPTFCLKEN_DSSCLK bit is 0) it doesn't mean that the clock is surely OFF. Its only OFF when the DPLL per m5x2 divider allows HW auto-gating of the clock. Hehe, welcome to the PRCM world :-) So OPTCLKEN_DSSCLK is in a way a dummy bit which when set, ensures that the M5 divider doesn't auto gate. So it isn't exactly a enable/disable bit. It is the case for most clocks in the system. You know when it is enabled, but it will be disabled only when the clock domain or in that case the parent clock will do HW auto gating. In this case there is no intermediate gating because the DSS_CLK is the only user of the M5 HW divider output, so gating only the parent is enough. In our tree(2.6.38-rc series), HW auto gating bit was disabled for m5x2 divider, and hence, it never mattered to us if OPTCLKEN_DSSCLK was enabled or disabled. We went on happily without bothering about this opt clock. When things got merged in mainline, the HW auto gating for m5x2 came into picture(HSDIVIDER_CLKOUT2_GATE_CTRL in CM_DIV_M5_DPLL_PER), and then suddenly dss_dss_fclk became super crucial, and a register access depended on it. This was the main reason of the confusion I guess. That make sense now, in theory, all these HS divider should be autogated by default, it was not the case previously because we were still in a non-PM optimize mode. That's why you have to control the clock at your level to ensure that the HW will not gate the parent clock. 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
OMAP4 DSS clock setup
Hi Benoit, Paul, I've been discussing with Sumit and Archit to understand how the DSS clocks are set up on OMAP4. I think I now have some idea how things work, but I'm still at loss why things are the way they are. So, if I look at OMAP4 TRM, Figure 10-4 DSS Clock Tree, there are two clocks in PRCM block that are relevant to this discussion: DSS_L3_ICLK and DSS_FCLK. To my understanding DSS_L3_ICLK is not really controllable, but it is affected by MODULEMODE bit. Then we have two relevant clocks defined in clock44xx_data.c: dss_fck and dss_dss_clk. dss_fck controls the MODULEMODE bit, and dss_dss_clk is the TRM's DSS_FCLK. Was that correct? If so, from DSS driver's perspective, the dss_fck sounds very much like an interface clock (it's always needed when DSS is used) and dss_dss_clk sounds very much like functional clock (it's always needed, except if DSI PLL is used for DSS functional clock). If dss_fck would control DSS_FCLK and dss_ick would control MODULEMODE, they would be about the same as the clocks in OMAP2 and 3, and we wouldn't need any omap4 spesific trickery in the DSS driver. (dss_dss_clk wouldn't be needed). Why are the clocks set up in this strange fashion? Tomi -- 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
Re: OMAP4 DSS clock setup
Hi Tomi, On 3/30/2011 8:48 AM, Valkeinen, Tomi wrote: Hi Benoit, Paul, I've been discussing with Sumit and Archit to understand how the DSS clocks are set up on OMAP4. I think I now have some idea how things work, but I'm still at loss why things are the way they are. So, if I look at OMAP4 TRM, Figure 10-4 DSS Clock Tree, there are two clocks in PRCM block that are relevant to this discussion: DSS_L3_ICLK and DSS_FCLK. To my understanding DSS_L3_ICLK is not really controllable, but it is affected by MODULEMODE bit. Then we have two relevant clocks defined in clock44xx_data.c: dss_fck and dss_dss_clk. dss_fck controls the MODULEMODE bit, and dss_dss_clk is the TRM's DSS_FCLK. Was that correct? Yes. For the moment, but this is not the final state. If so, from DSS driver's perspective, the dss_fck sounds very much like an interface clock (it's always needed when DSS is used) and dss_dss_clk sounds very much like functional clock (it's always needed, except if DSI PLL is used for DSS functional clock). If dss_fck would control DSS_FCLK and dss_ick would control MODULEMODE, they would be about the same as the clocks in OMAP2 and 3, and we wouldn't need any omap4 spesific trickery in the DSS driver. (dss_dss_clk wouldn't be needed). You cannot play with iclk, because this clock is supposed to be handled automatically by the HW. This was the case on OMAP2 3 as well BTW. Why are the clocks set up in this strange fashion? There are a lot of historical reasons and some changes from OMAP3 to OMAP4 that are not well handle by the current clock fmwk / hwmod fmwk. We are still trying to mimic OMAP23 clock management for OMAP4, and in fact we should do the opposite. The issue is due to the confusion with the MODULEMODE and the real input clock state. The MODULEMODE is just enabling the module (using iclk, fclk, opt clock) or whatever clk is used as input clock. In the case of the DSS, several optional clocks can be used as fclk. SYS_CLK, DSSCLK or the output of the DSI. In theory the MODULEMODE should be directly handled by hwmod since it is similar to enable / disable the module. Using a clock to manage the MODULEMODE is a temporary hack we are still using for the moment but that should be removed at some point. The clock fmwk should be used only to select / enable the proper optional clocks. In theory the current OMAP2 and OMAP3 iclken/fclken at PRCM level should be handled as a OMAP4 MODULEMODE and not as 2 clock nodes like it is the case today. Then we will have a consistent module / clock management. Hope this will help and clarify a little bit the current mess :-) Bottom-line is that you cannot do much at your level, the whole clock + hwmod fmwk should be improved. 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
Re: OMAP4 DSS clock setup
On Wed, 2011-03-30 at 11:32 +0200, Cousson, Benoit wrote: Hi Tomi, On 3/30/2011 8:48 AM, Valkeinen, Tomi wrote: Hi Benoit, Paul, I've been discussing with Sumit and Archit to understand how the DSS clocks are set up on OMAP4. I think I now have some idea how things work, but I'm still at loss why things are the way they are. So, if I look at OMAP4 TRM, Figure 10-4 DSS Clock Tree, there are two clocks in PRCM block that are relevant to this discussion: DSS_L3_ICLK and DSS_FCLK. To my understanding DSS_L3_ICLK is not really controllable, but it is affected by MODULEMODE bit. Then we have two relevant clocks defined in clock44xx_data.c: dss_fck and dss_dss_clk. dss_fck controls the MODULEMODE bit, and dss_dss_clk is the TRM's DSS_FCLK. Was that correct? Yes. For the moment, but this is not the final state. If so, from DSS driver's perspective, the dss_fck sounds very much like an interface clock (it's always needed when DSS is used) and dss_dss_clk sounds very much like functional clock (it's always needed, except if DSI PLL is used for DSS functional clock). If dss_fck would control DSS_FCLK and dss_ick would control MODULEMODE, they would be about the same as the clocks in OMAP2 and 3, and we wouldn't need any omap4 spesific trickery in the DSS driver. (dss_dss_clk wouldn't be needed). You cannot play with iclk, because this clock is supposed to be handled automatically by the HW. This was the case on OMAP2 3 as well BTW. Right. But now we have 1) dss_fck, which isn't the DSS_FCLK in TRM, and in fact not fck at all, or even clock at all 2) dss_dss_clk, a new clock whose name doesn't match to anything in TRM, and is in fact the DSS_FCLK. So they both sound like they are confusingly named. If we had dss_fck matching to DSS_FCLK and dss_ick for MODULEMODE, they would, in my opinion, be much more understandable and in many ways relate to the way things are in the HW. Additionally this would match the way clocks are on OMAP2/3 and things would just work. So while I understand that neither the current way and my suggestion exactly match the HW, and also that things are not ready yet and the clocks will change, I don't understand why such a strange method to name the clocks was used, when there's a simpler and backward compatible way. And if this current way to handle OMAP4 DSS clocks is for some reason better and can't be changed, could the OMAP2/3 clocks also be changed to keep things consistent between OMAPs? Because the inconsistency between platforms is the biggest problem for me. Tomi -- 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
Re: OMAP4 DSS clock setup
On 3/30/2011 1:03 PM, Valkeinen, Tomi wrote:
On Wed, 2011-03-30 at 11:32 +0200, Cousson, Benoit wrote:
Hi Tomi,
On 3/30/2011 8:48 AM, Valkeinen, Tomi wrote:
Hi Benoit, Paul,
I've been discussing with Sumit and Archit to understand how the DSS
clocks are set up on OMAP4. I think I now have some idea how things
work, but I'm still at loss why things are the way they are.
So, if I look at OMAP4 TRM, Figure 10-4 DSS Clock Tree, there are two
clocks in PRCM block that are relevant to this discussion: DSS_L3_ICLK
and DSS_FCLK. To my understanding DSS_L3_ICLK is not really
controllable, but it is affected by MODULEMODE bit.
Then we have two relevant clocks defined in clock44xx_data.c: dss_fck
and dss_dss_clk. dss_fck controls the MODULEMODE bit, and dss_dss_clk is
the TRM's DSS_FCLK.
Was that correct?
Yes. For the moment, but this is not the final state.
If so, from DSS driver's perspective, the dss_fck sounds very much like
an interface clock (it's always needed when DSS is used) and dss_dss_clk
sounds very much like functional clock (it's always needed, except if
DSI PLL is used for DSS functional clock).
dss_dss_fck is one of the possible functional clocks, hence the optional
attribute. You can run the DSS only for sys_clk is you want.
That's why the hwmod fmwk cannot choose for you which one you will need
depending of your panel. It is up to the driver to select the correct one.
If dss_fck would control DSS_FCLK and dss_ick would control
MODULEMODE, they would be about the same as the clocks in OMAP2 and 3,
and we wouldn't need any omap4 spesific trickery in the DSS driver.
(dss_dss_clk wouldn't be needed).
You cannot play with iclk, because this clock is supposed to be handled
automatically by the HW. This was the case on OMAP2 3 as well BTW.
Right.
But now we have 1) dss_fck, which isn't the DSS_FCLK in TRM, and in fact
not fck at all, or even clock at all 2) dss_dss_clk, a new clock whose
name doesn't match to anything in TRM, and is in fact the DSS_FCLK.
So they both sound like they are confusingly named.
The naming convention is simple: opt clock are using the module name +
the original clock name: dss_XXX, whereas the modulemode is using the
module name + fck, because it is for the moment similar to the fclk we
have on simpler module. iclk_en is not exposed at all on OMAP4.
optional clocks:
static struct clk dss_sys_clk = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_OPTFCLKEN_SYS_CLK_SHIFT,
static struct clk dss_tv_clk = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_OPTFCLKEN_TV_CLK_SHIFT,
static struct clk dss_dss_clk = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_OPTFCLKEN_DSSCLK_SHIFT,
static struct clk dss_48mhz_clk = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_OPTFCLKEN_48MHZ_CLK_SHIFT,
MODULEMODE:
static struct clk dss_fck = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_MODULEMODE_SWCTRL,
If we had dss_fck matching to DSS_FCLK and dss_ick for MODULEMODE, they
would, in my opinion, be much more understandable and in many ways
relate to the way things are in the HW. Additionally this would match
the way clocks are on OMAP2/3 and things would just work.
No, because you should not have to play with iclk at all.
BTW, for my point of view you have such issue because your are still not
using the pm_runtime API.
As soon as you will switch to pm_runtime, iclk_en and fclk_en (OMAP23
name) or MODULEMODE will be handle by the fmwk. The driver will just
have to handle the optional clock.
So while I understand that neither the current way and my suggestion
exactly match the HW, and also that things are not ready yet and the
clocks will change, I don't understand why such a strange method to name
the clocks was used, when there's a simpler and backward compatible way.
The point is that this automated method works for every IPs in the OMAP
except the DSS that is not managing its clocks like other modules.
Moreover, since the clock fmwk is creating aliases for these clock
nodes, you should never see at all that dss_dss_clk node that seems to
bother you.
And if this current way to handle OMAP4 DSS clocks is for some reason
better and can't be changed, could the OMAP2/3 clocks also be changed to
keep things consistent between OMAPs? Because the inconsistency between
platforms is the biggest problem for me.
As I said previsously, pm_runtime should fix these issues.
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
Re: OMAP4 DSS clock setup
On Wed, 2011-03-30 at 14:12 +0200, Cousson, Benoit wrote:
On 3/30/2011 1:03 PM, Valkeinen, Tomi wrote:
On Wed, 2011-03-30 at 11:32 +0200, Cousson, Benoit wrote:
Hi Tomi,
On 3/30/2011 8:48 AM, Valkeinen, Tomi wrote:
Hi Benoit, Paul,
I've been discussing with Sumit and Archit to understand how the DSS
clocks are set up on OMAP4. I think I now have some idea how things
work, but I'm still at loss why things are the way they are.
So, if I look at OMAP4 TRM, Figure 10-4 DSS Clock Tree, there are two
clocks in PRCM block that are relevant to this discussion: DSS_L3_ICLK
and DSS_FCLK. To my understanding DSS_L3_ICLK is not really
controllable, but it is affected by MODULEMODE bit.
Then we have two relevant clocks defined in clock44xx_data.c: dss_fck
and dss_dss_clk. dss_fck controls the MODULEMODE bit, and dss_dss_clk is
the TRM's DSS_FCLK.
Was that correct?
Yes. For the moment, but this is not the final state.
If so, from DSS driver's perspective, the dss_fck sounds very much like
an interface clock (it's always needed when DSS is used) and dss_dss_clk
sounds very much like functional clock (it's always needed, except if
DSI PLL is used for DSS functional clock).
dss_dss_fck is one of the possible functional clocks, hence the optional
attribute. You can run the DSS only for sys_clk is you want.
We can? I remember this was possible on OMAP2, but I can't seem to find
it on OMAP4 TRM...
Ah right, you mean sys_clk goes to DSI PLL, and the output from DSI PLL
can be used as functional clock?
We _always_ need DSS_FCLK to get DSS up and running, and to configure
DSI PLL if we want to get the clocks from there. So it's not quite that
optional. But it's true that we can turn it off later.
That's why the hwmod fmwk cannot choose for you which one you will need
depending of your panel. It is up to the driver to select the correct one.
But isn't that DSS internal thing? (I'm looking again at the DSS clock
tree figure). DSS_FCLK from the PRCM should always be the same from
DSS's point of view, it doesn't change. If we use the clock from DSI
PLL, we'll get clock for LCD2_CLK, LCD1_CLK and DISPC_FCLK. But
DSS_FCLK/DSS_CLK is always the same.
If dss_fck would control DSS_FCLK and dss_ick would control
MODULEMODE, they would be about the same as the clocks in OMAP2 and 3,
and we wouldn't need any omap4 spesific trickery in the DSS driver.
(dss_dss_clk wouldn't be needed).
You cannot play with iclk, because this clock is supposed to be handled
automatically by the HW. This was the case on OMAP2 3 as well BTW.
Right.
But now we have 1) dss_fck, which isn't the DSS_FCLK in TRM, and in fact
not fck at all, or even clock at all 2) dss_dss_clk, a new clock whose
name doesn't match to anything in TRM, and is in fact the DSS_FCLK.
So they both sound like they are confusingly named.
The naming convention is simple: opt clock are using the module name +
the original clock name: dss_XXX, whereas the modulemode is using the
module name + fck, because it is for the moment similar to the fclk we
have on simpler module. iclk_en is not exposed at all on OMAP4.
optional clocks:
static struct clk dss_sys_clk = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_OPTFCLKEN_SYS_CLK_SHIFT,
static struct clk dss_tv_clk = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_OPTFCLKEN_TV_CLK_SHIFT,
static struct clk dss_dss_clk = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_OPTFCLKEN_DSSCLK_SHIFT,
static struct clk dss_48mhz_clk = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_OPTFCLKEN_48MHZ_CLK_SHIFT,
MODULEMODE:
static struct clk dss_fck = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_MODULEMODE_SWCTRL,
Okay, the naming makes sense now that you explained the convention.
I think I'm finally starting to get this =).
If we had dss_fck matching to DSS_FCLK and dss_ick for MODULEMODE, they
would, in my opinion, be much more understandable and in many ways
relate to the way things are in the HW. Additionally this would match
the way clocks are on OMAP2/3 and things would just work.
No, because you should not have to play with iclk at all.
BTW, for my point of view you have such issue because your are still not
using the pm_runtime API.
As soon as you will switch to pm_runtime, iclk_en and fclk_en (OMAP23
name) or MODULEMODE will be handle by the fmwk. The driver will just
have to handle the optional clock.
So while I understand that neither the current way and my suggestion
exactly match the HW, and also that things are not ready yet and the
clocks will change, I don't understand why such a strange method to name
the clocks was used, when there's a simpler and backward compatible way.
The
Re: OMAP4 DSS clock setup
On 3/30/2011 2:58 PM, Valkeinen, Tomi wrote:
On Wed, 2011-03-30 at 14:12 +0200, Cousson, Benoit wrote:
On 3/30/2011 1:03 PM, Valkeinen, Tomi wrote:
On Wed, 2011-03-30 at 11:32 +0200, Cousson, Benoit wrote:
Hi Tomi,
On 3/30/2011 8:48 AM, Valkeinen, Tomi wrote:
Hi Benoit, Paul,
I've been discussing with Sumit and Archit to understand how the DSS
clocks are set up on OMAP4. I think I now have some idea how things
work, but I'm still at loss why things are the way they are.
So, if I look at OMAP4 TRM, Figure 10-4 DSS Clock Tree, there are two
clocks in PRCM block that are relevant to this discussion: DSS_L3_ICLK
and DSS_FCLK. To my understanding DSS_L3_ICLK is not really
controllable, but it is affected by MODULEMODE bit.
Then we have two relevant clocks defined in clock44xx_data.c: dss_fck
and dss_dss_clk. dss_fck controls the MODULEMODE bit, and dss_dss_clk is
the TRM's DSS_FCLK.
Was that correct?
Yes. For the moment, but this is not the final state.
If so, from DSS driver's perspective, the dss_fck sounds very much like
an interface clock (it's always needed when DSS is used) and dss_dss_clk
sounds very much like functional clock (it's always needed, except if
DSI PLL is used for DSS functional clock).
dss_dss_fck is one of the possible functional clocks, hence the optional
attribute. You can run the DSS only for sys_clk is you want.
We can? I remember this was possible on OMAP2, but I can't seem to find
it on OMAP4 TRM...
Ah right, you mean sys_clk goes to DSI PLL, and the output from DSI PLL
can be used as functional clock?
Yes, you're right, maybe we can still use the sys_clk directly with a
parallel QVGA LCD like on OMAP2:-)
We _always_ need DSS_FCLK to get DSS up and running, and to configure
DSI PLL if we want to get the clocks from there. So it's not quite that
optional. But it's true that we can turn it off later.
Yes, that was confirmed by HW folks, as soon as you have one functional
clock active, you can disable the other one.
That's why the hwmod fmwk cannot choose for you which one you will need
depending of your panel. It is up to the driver to select the correct one.
But isn't that DSS internal thing? (I'm looking again at the DSS clock
tree figure). DSS_FCLK from the PRCM should always be the same from
DSS's point of view, it doesn't change. If we use the clock from DSI
PLL, we'll get clock for LCD2_CLK, LCD1_CLK and DISPC_FCLK. But
DSS_FCLK/DSS_CLK is always the same.
Yes, but there is no other need for the DSS_CLK beside these internal
nodes and the DSI engines. So you can shut it down as soon as an
alternate clock is available (PLL1_CLK1 or PLL2_CLK1 in your case).
If dss_fck would control DSS_FCLK and dss_ick would control
MODULEMODE, they would be about the same as the clocks in OMAP2 and 3,
and we wouldn't need any omap4 spesific trickery in the DSS driver.
(dss_dss_clk wouldn't be needed).
You cannot play with iclk, because this clock is supposed to be handled
automatically by the HW. This was the case on OMAP2 3 as well BTW.
Right.
But now we have 1) dss_fck, which isn't the DSS_FCLK in TRM, and in fact
not fck at all, or even clock at all 2) dss_dss_clk, a new clock whose
name doesn't match to anything in TRM, and is in fact the DSS_FCLK.
So they both sound like they are confusingly named.
The naming convention is simple: opt clock are using the module name +
the original clock name: dss_XXX, whereas the modulemode is using the
module name + fck, because it is for the moment similar to the fclk we
have on simpler module. iclk_en is not exposed at all on OMAP4.
optional clocks:
static struct clk dss_sys_clk = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_OPTFCLKEN_SYS_CLK_SHIFT,
static struct clk dss_tv_clk = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_OPTFCLKEN_TV_CLK_SHIFT,
static struct clk dss_dss_clk = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_OPTFCLKEN_DSSCLK_SHIFT,
static struct clk dss_48mhz_clk = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_OPTFCLKEN_48MHZ_CLK_SHIFT,
MODULEMODE:
static struct clk dss_fck = {
.enable_reg = OMAP4430_CM_DSS_DSS_CLKCTRL,
.enable_bit = OMAP4430_MODULEMODE_SWCTRL,
Okay, the naming makes sense now that you explained the convention.
I think I'm finally starting to get this =).
If we had dss_fck matching to DSS_FCLK and dss_ick for MODULEMODE, they
would, in my opinion, be much more understandable and in many ways
relate to the way things are in the HW. Additionally this would match
the way clocks are on OMAP2/3 and things would just work.
No, because you should not have to play with iclk at all.
BTW, for my point of view you have such issue because your are still not
using the pm_runtime API.
As soon as you will switch to pm_runtime, iclk_en and fclk_en (OMAP23
name) or
