From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 31F76C3F2C6 for ; Tue, 3 Mar 2020 15:13:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0EFA020842 for ; Tue, 3 Mar 2020 15:13:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727862AbgCCPNy (ORCPT ); Tue, 3 Mar 2020 10:13:54 -0500 Received: from muru.com ([72.249.23.125]:58542 "EHLO muru.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727079AbgCCPNy (ORCPT ); Tue, 3 Mar 2020 10:13:54 -0500 Received: from atomide.com (localhost [127.0.0.1]) by muru.com (Postfix) with ESMTPS id 9BD6180EE; Tue, 3 Mar 2020 15:14:37 +0000 (UTC) Date: Tue, 3 Mar 2020 07:13:49 -0800 From: Tony Lindgren To: Tomi Valkeinen Cc: linux-omap@vger.kernel.org, "Andrew F . Davis" , Dave Gerlach , Faiz Abbas , Greg Kroah-Hartman , Keerthy , Nishanth Menon , Peter Ujfalusi , Roger Quadros , Suman Anna , Tero Kristo , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Jyri Sarha , Laurent Pinchart , dri-devel@lists.freedesktop.org Subject: Re: [PATCH 3/3] bus: ti-sysc: Implement display subsystem reset quirk Message-ID: <20200303151349.GQ37466@atomide.com> References: <20200224191230.30972-1-tony@atomide.com> <20200224191230.30972-4-tony@atomide.com> <7d4af3b5-5dd7-76b3-4d3f-4698bfde288c@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7d4af3b5-5dd7-76b3-4d3f-4698bfde288c@ti.com> Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org Hi, * Tomi Valkeinen [200303 06:03]: > On 24/02/2020 21:12, Tony Lindgren wrote: > > + /* Remap the whole module range to be able to reset dispc outputs */ > > + devm_iounmap(ddata->dev, ddata->module_va); > > + ddata->module_va = devm_ioremap(ddata->dev, > > + ddata->module_pa, > > + ddata->module_size); > > Why is this needed? The range is not mapped when sysc_pre_reset_quirk_dss() > is called? This will unmap and remap twice, as this function is called > twice. And then left mapped. That's because by default we only ioremap the module revision, sysconfig and sysstatus register are and provide the rest as a range for the child nodes. In the dss quirk case we need to tinker with registers also in the dispc range, and at the parent dss probe time dispc has not probed yet. We may be able to eventually move the reset quirk to dispc, but then it won't happen in the current setup until after dss top level driver has loaded. We leave the module range ioremapped as we still need to access sysconfig related registers for PM runtime. > > + if (!ddata->module_va) > > + return -EIO; > > + > > + /* DISP_CONTROL */ > > + val = sysc_read(ddata, dispc_offset + 0x40); > > Defines for dss/dispc register offsets could have been copied from the > platform display.c and used in this file. Yeah I though about that, but decided to keep everything local to the quirk handling. We could have them defined in some dss header though. > > + /* Clear IRQSTATUS */ > > + sysc_write(ddata, 0x1000 + 0x18, irq_mask); > > dispc_offset instead of 0x1000. OK > > + > > + /* Disable outputs */ > > + val = sysc_quirk_dispc(ddata, dispc_offset, true); > > + > > + /* Poll IRQSTATUS */ > > + error = readl_poll_timeout(ddata->module_va + dispc_offset + 0x18, > > + val, val != irq_mask, 100, 50); > > + if (error) > > + dev_warn(ddata->dev, "%s: timed out %08x !+ %08x\n", > > + __func__, val, irq_mask); > > + > > + if (sysc_soc->soc == SOC_3430) { > > + /* Clear DSS_SDI_CONTROL */ > > + sysc_write(ddata, dispc_offset + 0x44, 0); > > + > > + /* Clear DSS_PLL_CONTROL */ > > + sysc_write(ddata, dispc_offset + 0x48, 0); > > These are not dispc registers, but dss registers. Ouch. Thanks for catching this, will include in the fix. > > + } > > + > > + /* Clear DSS_CONTROL to switch DSS clock sources to PRCM if not */ > > + sysc_write(ddata, dispc_offset + 0x40, 0); > > Same here. OK Regards, Tony From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 15436C3F2D1 for ; Tue, 3 Mar 2020 15:14:01 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DB15120842 for ; Tue, 3 Mar 2020 15:14:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="UofWOQGt" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DB15120842 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=atomide.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=vf1aXWpjUZbfFLoI6aU3qPZcs/KamhcTt9ibQhCJpJ4=; b=UofWOQGtSlGtG5 sR3+kYa7VCVRKRvItuj22etgEiKJ32xT2cBA+vDCU8Bs+n1Gh2P2sqzrWBMHAT1cbNSIftmwi1xCe iP5PUbghKvLD/+bYynayByLmoPRpRrMrQWTwv/fV7knPuLzhHmjP1mmMtlnKh5x/PB0nuxIw2854y wWhvOTj4va66j5+Bz10llx/KiViipB+NJiOhVINzgdl0GPUIRdmMg3Us6r2s4fpCtF/kc+JAFDakN qkPo50ryKArqqE4MLCBgulf7OZvNYZ4Q4hjMJQHqpeeRJW8ea/gd/+ia5cy+47VbP3v6Qof+AbXwh 4TdF/1ij7R5C0RU3W+kg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1j99FA-0000Gt-DN; Tue, 03 Mar 2020 15:14:00 +0000 Received: from muru.com ([72.249.23.125]) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1j99F8-0000Ft-A8 for linux-arm-kernel@lists.infradead.org; Tue, 03 Mar 2020 15:13:59 +0000 Received: from atomide.com (localhost [127.0.0.1]) by muru.com (Postfix) with ESMTPS id 9BD6180EE; Tue, 3 Mar 2020 15:14:37 +0000 (UTC) Date: Tue, 3 Mar 2020 07:13:49 -0800 From: Tony Lindgren To: Tomi Valkeinen Subject: Re: [PATCH 3/3] bus: ti-sysc: Implement display subsystem reset quirk Message-ID: <20200303151349.GQ37466@atomide.com> References: <20200224191230.30972-1-tony@atomide.com> <20200224191230.30972-4-tony@atomide.com> <7d4af3b5-5dd7-76b3-4d3f-4698bfde288c@ti.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <7d4af3b5-5dd7-76b3-4d3f-4698bfde288c@ti.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200303_071358_390660_87BCE216 X-CRM114-Status: GOOD ( 18.00 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Nishanth Menon , Tero Kristo , Suman Anna , Dave Gerlach , Keerthy , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Jyri Sarha , "Andrew F . Davis" , Peter Ujfalusi , Faiz Abbas , Laurent Pinchart , Greg Kroah-Hartman , linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Roger Quadros Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, * Tomi Valkeinen [200303 06:03]: > On 24/02/2020 21:12, Tony Lindgren wrote: > > + /* Remap the whole module range to be able to reset dispc outputs */ > > + devm_iounmap(ddata->dev, ddata->module_va); > > + ddata->module_va = devm_ioremap(ddata->dev, > > + ddata->module_pa, > > + ddata->module_size); > > Why is this needed? The range is not mapped when sysc_pre_reset_quirk_dss() > is called? This will unmap and remap twice, as this function is called > twice. And then left mapped. That's because by default we only ioremap the module revision, sysconfig and sysstatus register are and provide the rest as a range for the child nodes. In the dss quirk case we need to tinker with registers also in the dispc range, and at the parent dss probe time dispc has not probed yet. We may be able to eventually move the reset quirk to dispc, but then it won't happen in the current setup until after dss top level driver has loaded. We leave the module range ioremapped as we still need to access sysconfig related registers for PM runtime. > > + if (!ddata->module_va) > > + return -EIO; > > + > > + /* DISP_CONTROL */ > > + val = sysc_read(ddata, dispc_offset + 0x40); > > Defines for dss/dispc register offsets could have been copied from the > platform display.c and used in this file. Yeah I though about that, but decided to keep everything local to the quirk handling. We could have them defined in some dss header though. > > + /* Clear IRQSTATUS */ > > + sysc_write(ddata, 0x1000 + 0x18, irq_mask); > > dispc_offset instead of 0x1000. OK > > + > > + /* Disable outputs */ > > + val = sysc_quirk_dispc(ddata, dispc_offset, true); > > + > > + /* Poll IRQSTATUS */ > > + error = readl_poll_timeout(ddata->module_va + dispc_offset + 0x18, > > + val, val != irq_mask, 100, 50); > > + if (error) > > + dev_warn(ddata->dev, "%s: timed out %08x !+ %08x\n", > > + __func__, val, irq_mask); > > + > > + if (sysc_soc->soc == SOC_3430) { > > + /* Clear DSS_SDI_CONTROL */ > > + sysc_write(ddata, dispc_offset + 0x44, 0); > > + > > + /* Clear DSS_PLL_CONTROL */ > > + sysc_write(ddata, dispc_offset + 0x48, 0); > > These are not dispc registers, but dss registers. Ouch. Thanks for catching this, will include in the fix. > > + } > > + > > + /* Clear DSS_CONTROL to switch DSS clock sources to PRCM if not */ > > + sysc_write(ddata, dispc_offset + 0x40, 0); > > Same here. OK Regards, Tony _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.7 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 82323C3F2D7 for ; Wed, 4 Mar 2020 07:47:59 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3F40C207FD for ; Wed, 4 Mar 2020 07:47:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3F40C207FD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=atomide.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C85946EAC7; Wed, 4 Mar 2020 07:47:30 +0000 (UTC) Received: from muru.com (muru.com [72.249.23.125]) by gabe.freedesktop.org (Postfix) with ESMTP id D90A56E44F for ; Tue, 3 Mar 2020 15:13:54 +0000 (UTC) Received: from atomide.com (localhost [127.0.0.1]) by muru.com (Postfix) with ESMTPS id 9BD6180EE; Tue, 3 Mar 2020 15:14:37 +0000 (UTC) Date: Tue, 3 Mar 2020 07:13:49 -0800 From: Tony Lindgren To: Tomi Valkeinen Subject: Re: [PATCH 3/3] bus: ti-sysc: Implement display subsystem reset quirk Message-ID: <20200303151349.GQ37466@atomide.com> References: <20200224191230.30972-1-tony@atomide.com> <20200224191230.30972-4-tony@atomide.com> <7d4af3b5-5dd7-76b3-4d3f-4698bfde288c@ti.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <7d4af3b5-5dd7-76b3-4d3f-4698bfde288c@ti.com> X-Mailman-Approved-At: Wed, 04 Mar 2020 07:47:28 +0000 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Nishanth Menon , Tero Kristo , Suman Anna , Dave Gerlach , Keerthy , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Jyri Sarha , "Andrew F . Davis" , Peter Ujfalusi , Faiz Abbas , Laurent Pinchart , Greg Kroah-Hartman , linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Roger Quadros Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi, * Tomi Valkeinen [200303 06:03]: > On 24/02/2020 21:12, Tony Lindgren wrote: > > + /* Remap the whole module range to be able to reset dispc outputs */ > > + devm_iounmap(ddata->dev, ddata->module_va); > > + ddata->module_va = devm_ioremap(ddata->dev, > > + ddata->module_pa, > > + ddata->module_size); > > Why is this needed? The range is not mapped when sysc_pre_reset_quirk_dss() > is called? This will unmap and remap twice, as this function is called > twice. And then left mapped. That's because by default we only ioremap the module revision, sysconfig and sysstatus register are and provide the rest as a range for the child nodes. In the dss quirk case we need to tinker with registers also in the dispc range, and at the parent dss probe time dispc has not probed yet. We may be able to eventually move the reset quirk to dispc, but then it won't happen in the current setup until after dss top level driver has loaded. We leave the module range ioremapped as we still need to access sysconfig related registers for PM runtime. > > + if (!ddata->module_va) > > + return -EIO; > > + > > + /* DISP_CONTROL */ > > + val = sysc_read(ddata, dispc_offset + 0x40); > > Defines for dss/dispc register offsets could have been copied from the > platform display.c and used in this file. Yeah I though about that, but decided to keep everything local to the quirk handling. We could have them defined in some dss header though. > > + /* Clear IRQSTATUS */ > > + sysc_write(ddata, 0x1000 + 0x18, irq_mask); > > dispc_offset instead of 0x1000. OK > > + > > + /* Disable outputs */ > > + val = sysc_quirk_dispc(ddata, dispc_offset, true); > > + > > + /* Poll IRQSTATUS */ > > + error = readl_poll_timeout(ddata->module_va + dispc_offset + 0x18, > > + val, val != irq_mask, 100, 50); > > + if (error) > > + dev_warn(ddata->dev, "%s: timed out %08x !+ %08x\n", > > + __func__, val, irq_mask); > > + > > + if (sysc_soc->soc == SOC_3430) { > > + /* Clear DSS_SDI_CONTROL */ > > + sysc_write(ddata, dispc_offset + 0x44, 0); > > + > > + /* Clear DSS_PLL_CONTROL */ > > + sysc_write(ddata, dispc_offset + 0x48, 0); > > These are not dispc registers, but dss registers. Ouch. Thanks for catching this, will include in the fix. > > + } > > + > > + /* Clear DSS_CONTROL to switch DSS clock sources to PRCM if not */ > > + sysc_write(ddata, dispc_offset + 0x40, 0); > > Same here. OK Regards, Tony _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel