From patchwork Mon Nov 26 12:14:06 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: archit taneja X-Patchwork-Id: 1801941 Return-Path: X-Original-To: patchwork-linux-omap@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 56B373FC54 for ; Mon, 26 Nov 2012 12:14:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754473Ab2KZMOp (ORCPT ); Mon, 26 Nov 2012 07:14:45 -0500 Received: from comal.ext.ti.com ([198.47.26.152]:58364 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753752Ab2KZMOp (ORCPT ); Mon, 26 Nov 2012 07:14:45 -0500 Received: from dbdp20.itg.ti.com ([172.24.170.38]) by comal.ext.ti.com (8.13.7/8.13.7) with ESMTP id qAQCEYAk023738; Mon, 26 Nov 2012 06:14:35 -0600 Received: from DBDE71.ent.ti.com (localhost [127.0.0.1]) by dbdp20.itg.ti.com (8.13.8/8.13.8) with ESMTP id qAQCEVCA020412; Mon, 26 Nov 2012 17:44:32 +0530 (IST) Received: from [172.24.137.91] (172.24.137.91) by DBDE71.ent.ti.com (172.24.170.149) with Microsoft SMTP Server id 14.1.323.3; Mon, 26 Nov 2012 17:44:31 +0530 Message-ID: <50B35D0E.6000509@ti.com> Date: Mon, 26 Nov 2012 17:44:06 +0530 From: Archit Taneja User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: , Tomi Valkeinen CC: Rajendra Nayak , Russell King - ARM Linux , , , Tony Lindgren , "Shilimkar, Santosh" Subject: Re: OMAP4430 produces boot warnings References: <20121121230337.GR3332@n2100.arm.linux.org.uk> <50AE1DC0.3030002@ti.com> <50AE2BB6.8080406@ti.com> <1353594842.786.45.camel@sokoban> <50AE3A5B.5040603@ti.com> <1353663278.25248.4.camel@sokoban> <50B310D3.9050202@ti.com> In-Reply-To: <50B310D3.9050202@ti.com> X-Originating-IP: [172.24.137.91] Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org Hi, On Monday 26 November 2012 12:18 PM, Archit Taneja wrote: > On Friday 23 November 2012 03:04 PM, Tero Kristo wrote: >> On Thu, 2012-11-22 at 16:44 +0200, Tomi Valkeinen wrote: >>> On 2012-11-22 16:34, Tero Kristo wrote: >>> >>>> I guess you checked that DSS pwrdm is switching between RET and ON in >>>> your setup? >>> >>> Yes: >>> >>> # cat /debug/pm_debug/count |grep dss >>> [ 35.356567] pwrdm state mismatch(l3init_pwrdm) 3 != 1 >>> [ 35.361938] pwrdm state mismatch(cam_pwrdm) 3 != 0 >>> [ 35.366973] pwrdm state mismatch(ivahd_pwrdm) 3 != 1 >>> [ 35.372253] pwrdm state mismatch(tesla_pwrdm) 3 != 1 >>> [ 35.377532] pwrdm state mismatch(abe_pwrdm) 3 != 1 >>> dss_pwrdm >>> (RET),OFF:1,RET:11,INA:0,ON:11,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 >>> l3_dss_clkdm->dss_pwrdm (0) >>> >>> then I load and unload the dss modules, and then: >>> >>> # cat /debug/pm_debug/count |grep dss >>> [ 60.813629] pwrdm state mismatch(l3init_pwrdm) 3 != 1 >>> [ 60.819000] pwrdm state mismatch(cam_pwrdm) 3 != 0 >>> [ 60.824127] pwrdm state mismatch(ivahd_pwrdm) 3 != 1 >>> [ 60.829376] pwrdm state mismatch(tesla_pwrdm) 3 != 1 >>> [ 60.834625] pwrdm state mismatch(abe_pwrdm) 3 != 1 >>> dss_pwrdm >>> (ON),OFF:1,RET:21,INA:0,ON:22,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 >>> l3_dss_clkdm->dss_pwrdm (0) >>> >>>>> Does the pwrdm mistakenly think that in RET state the DSS still keeps >>>>> the register contents? >>>> >>>> This might be the case, however the pwrdm code should be generic and >>>> handle all domains properly. What is the tree / branch / commit you are >>>> using for testing this stuff? I can take a look at this also. >>> >>> arm-soc/for-next >> >> I guess this is caused because some of the patches are still not in the >> for-next branch, it looks like at least this is missing: >> >> https://patchwork.kernel.org/patch/1608901/ >> >> ...or the latest update done by Paul to that one. >> >> The patch I posted appears to have a small merge induced bug, it is >> registering the context loss soc_ops for am33xx when it should actually >> register those for omap4. This might explain another bug I've been >> looking at in a different branch recently... The update Paul posted does >> not seem to have this problem, but I haven't tested it myself. > > > Applying the patch above, and registering the soc_ops for omap4 instead > of am33xx doesn't seem to help. The context lost count now always > returns 0. > > And to verify Tomi's observation, if we don't rely on the context lost > count, and restore the registers always, we don't see the problem. 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 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 --- 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;