On Tuesday 27 November 2012 04:53 PM, Tomi Valkeinen wrote:
On 2012-11-26 14:14, Archit Taneja wrote:So Rajendra and I found the problem. The function _omap4_update_context_lost() reads the register RM_DSS_DSS_CONTEXT for all DSS hwmods, and increments the count if we read a non zero value. The issue is that the DSS's parent platform device (tied to dss_core hwmod) is called first when resuming, it correctly reads the register and observes that DSS lost context, and then clears the register. When the children hwmods are enabled, the see that the registers are cleared, and hence never increment their count. One option is to make the DSS driver use the context lost count of the hwmod corresponding to the parent platform device. It sort of makes a bit of sense as all the DSS platform devices belong to the same power domain, so considering only the parent's context lost count is not so bad. The second option would be to have some usecounting mechanism in omap_hwmod where different hwmods belonging to the same power domain don't have their PM_CONTEXT registers cleared until all the hwmods are enabled. The first option is easier to implement, here is a patch for the DISPC driver: From 619276fa0e62b90875475eb345a310f1223e82f6 Mon Sep 17 00:00:00 2001 From: Archit Taneja <archit@xxxxxx> Date: Mon, 26 Nov 2012 17:22:27 +0530 Subject: [PATCH] OMAPDSS: DISPC: Use DISPC's parent device to get context lost count When enabling a hwmod, omap_hwmod refers to the register mentioned in the hwmod struct's member 'prcm.omap4.context_offs' to see whether context was lost or not. It increments the context lost count for the hwmod and then clears the register. All the DSS hwmods have the same register(RM_DSS_DSS_CONTEXT) as context_offs. When DSS is enabled, the first hwmod to be enabled is the "dss_core" hwmod since it's the parent platform device. The dss_core hwmod updates it's context lost count correctly and clears the register. When the hwmods corresponding to the children platform devices are enabled. They see that the register is clear, and don't increment their context lost count. Therefore, all the children platfrom devices never report a change in context. The DISPC driver currently gets the context lost count for DSS from it's corresponsing platform device instance. The DISPC platform device is one of the child devices, and doesn't report the context lost count correctly. Make the DISPC driver get the context lost count from it's parent. The parent platform device's hwmod is the only one which correctly updates the context lost count. Signed-off-by: Archit Taneja <archit@xxxxxx> --- drivers/video/omap2/dss/dispc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c index a5ab354..d9dfc4ad 100644 --- a/drivers/video/omap2/dss/dispc.c +++ b/drivers/video/omap2/dss/dispc.c @@ -374,7 +374,7 @@ static void dispc_save_context(void) if (dss_has_feature(FEAT_CORE_CLK_DIV)) SR(DIVISOR); - dispc.ctx_loss_cnt = dss_get_ctx_loss_count(&dispc.pdev->dev); + dispc.ctx_loss_cnt = dss_get_ctx_loss_count(dispc.pdev->dev.parent); dispc.ctx_valid = true; DSSDBG("context saved, ctx_loss_count %d\n", dispc.ctx_loss_cnt); @@ -389,7 +389,7 @@ static void dispc_restore_context(void) if (!dispc.ctx_valid) return; - ctx = dss_get_ctx_loss_count(&dispc.pdev->dev); + ctx = dss_get_ctx_loss_count(dispc.pdev->dev.parent); if (ctx >= 0 && ctx == dispc.ctx_loss_cnt) return;Hmm, well this feels like a hack. DISPC driver doesn't know how the DSS modules are arranged, which module belongs to which power domain, etc. If it cannot be fixed in the arch code, I guess we could just have dss_get_ctx_loss_count(void) function which always returns the dss_core's ctx loss count, and define that on all the platforms omapdss is used, the dss_core's ctx loss count is the same as ctx loss count for all the dss submodules. I think the above is true for all OMAPs. But it feels like a hack too, but not as bad as the above patch.
Yes, a function taking in no platform device in dss's core.c would be less hacky. I guess we would need this for now, because a solution in omap_hwmod would be more complex and it may not be ready by the merge window.
Archit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
