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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id D3FD8C433EF for ; Wed, 25 May 2022 22:07:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References :In-Reply-To:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=X1JSYptEOqsTu9H1R9yuIKIYvOPvAxmIIKO17+fzOtM=; b=27mqI2lxXFC9VD iZ99MGvuFV7QaGFp2qOBs/W5s6/vCGfNO0Tcpe3HRZwjGJ2bKnU4JqUAYXYHH6ee+/gNRQl8THaIh O/gOzYhOkr2dkSp8Y1rCEzTXLoeHjjD5+saqSgHlm0YNmzgobdqUjBaAZm88VNTRSaRConDneOIYR 4dDX3rzuY6Dzb3PvkoHZeSTW82Rv7W6W5XJ7u/76YeHI9+i1etF4VWb6UEJ3QxXFb4x3bX7RJSCZe OEc6dJrpeq2oK4n+4lQLF0MjnkEiW/Y0cAGN+G5wBAV+/Qpp9tDCDdvAFeU++Bbo/keYZt6KlkIn/ HyeLXdiiwaJgnkNrt4iQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ntzAA-00CpBX-RA; Wed, 25 May 2022 22:07:30 +0000 Received: from mail-pf1-f171.google.com ([209.85.210.171]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ntz9z-00Cp8G-CG for linux-mediatek@lists.infradead.org; Wed, 25 May 2022 22:07:21 +0000 Received: by mail-pf1-f171.google.com with SMTP id x143so113974pfc.11 for ; Wed, 25 May 2022 15:07:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=IE2Ntb5oMK4K7amafeNetTiCxxwL4oFF63eZ4S/Cu40=; b=J9pZRVyEUYkkxuBGwk7x3QRggkqS7f9JJZR9bdMGDars6m3TbXV30qI0MIqmB+XlTB 0oRZ1J1gJCOsVpf2h2W0Eoz7pB1EOci8AY6CgJS3XNzNsnx1clZEzuyZ4JzH9Hgggl78 Qro7D1QLjzx6AqXOf1X0YRVD6GogDkwEwgZyLDAo1VZlnbnNqFn0PHPp3q02IV2IvUFf huku/s+M28zMhdBJu17E8oQq5pWbGDl6GqeeiKNedYMtu9gukIEORgqY20GGn5tVW3Fg 3ZhtOjNVRCqsCa6lnFhrp2bpi5em2r24bTDBo55v1i3rrBY+CcviCZVKK9h/8QXMI12P mZYw== X-Gm-Message-State: AOAM530cX6V4wZBmMfX2G/nQeGLLy71Mz085po1Ehm0qEP158YTY3NHf M7M17oF6H/fZdB2NZE7e95RI4Q== X-Google-Smtp-Source: ABdhPJwdqFMCLJa7Y7vyj6CS5A+5uCRs6z3mJZo81jSCPzBgpMhVjT7B8zM+9XXAV6oNybU+EL4RYw== X-Received: by 2002:a63:c04a:0:b0:3fa:7277:964c with SMTP id z10-20020a63c04a000000b003fa7277964cmr13087451pgi.499.1653516436926; Wed, 25 May 2022 15:07:16 -0700 (PDT) Received: from localhost (c-71-197-186-152.hsd1.wa.comcast.net. [71.197.186.152]) by smtp.gmail.com with ESMTPSA id o1-20020a170902d4c100b0015edb22aba1sm10030189plg.270.2022.05.25.15.07.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 May 2022 15:07:16 -0700 (PDT) From: Kevin Hilman To: Chen-Yu Tsai , Chanwoo Choi Cc: Roger Lu , Matthias Brugger , Enric Balletbo Serra , Rob Herring , Nicolas Boichat , Stephen Boyd , Philipp Zabel , Fan Chen , Charles Yang , Angus Lin , Mark Rutland , Nishanth Menon , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, Project_Global_Chrome_Upstream_Group@mediatek.com, Guenter Roeck , Jia-wei Chang , =?utf-8?B?UmV4LUJDIENoZW4gKOmZs+afj+i+sCk=?= Subject: Re: [PATCH v25 0/7] soc: mediatek: SVS: introduce MTK SVS In-Reply-To: References: <20220516004311.18358-1-roger.lu@mediatek.com> <7h4k1ndaui.fsf@baylibre.com> <7hy1yzbtb7.fsf@baylibre.com> <7hmtfdbcsc.fsf@baylibre.com> <5a1767dc-ba2d-4de5-d8fe-2f308d3318a9@samsung.com> Date: Wed, 25 May 2022 15:07:15 -0700 Message-ID: <7hleup9sh8.fsf@baylibre.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220525_150719_461906_C33AD109 X-CRM114-Status: GOOD ( 40.51 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org Chen-Yu Tsai writes: > On Fri, May 20, 2022 at 5:53 PM Chanwoo Choi wrote: >> >> Hi Kevin, >> >> On 5/20/22 11:42 AM, Chen-Yu Tsai wrote: >> > On Fri, May 20, 2022 at 9:28 AM Chanwoo Choi wrote: >> >> >> >> Hi Kevin, Chen-Yu, >> >> >> >> On 5/20/22 3:25 AM, Kevin Hilman wrote: >> >>> Chen-Yu Tsai writes: >> >>> >> >>>> n Wed, May 18, 2022 at 8:03 AM Kevin Hilman wrote: >> >>>>> >> >>>>> Kevin Hilman writes: >> >>>>> >> >>>>>> Chen-Yu Tsai writes: >> >>>>>> >> >>>>>>> On Mon, May 16, 2022 at 8:43 AM Roger Lu wrote: >> >>>>>>>> >> >>>>>>>> The Smart Voltage Scaling(SVS) engine is a piece of hardware >> >>>>>>>> which calculates suitable SVS bank voltages to OPP voltage table. >> >>>>>>>> Then, DVFS driver could apply those SVS bank voltages to PMIC/Buck >> >>>>>>>> when receiving OPP_EVENT_ADJUST_VOLTAGE. >> >>>>>>>> >> >>>>>>>> 1. SVS driver uses OPP adjust event in [1] to update OPP table voltage part. >> >>>>>>>> 2. SVS driver gets thermal/GPU device by node [2][3] and CPU device by get_cpu_device(). >> >>>>>>>> After retrieving subsys device, SVS driver calls device_link_add() to make sure probe/suspend callback priority. >> >>>>>>>> >> >>>>>>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/commit/?h=opp/linux-next&id=25cb20a212a1f989385dfe23230817e69c62bee5 >> >>>>>>>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/commit/?h=opp/linux-next&id=b325ce39785b1408040d90365a6ab1aa36e94f87 >> >>>>>>>> [3] https://git.kernel.org/pub/scm/linux/kernel/git/matthias.bgg/linux.git/commit/?h=v5.16-next/dts64&id=a8168cebf1bca1b5269e8a7eb2626fb76814d6e2 >> >>>>>>>> >> >>>>>>>> Change since v24: >> >>>>>>>> - Rebase to Linux 5.18-rc6 >> >>>>>>>> - Show specific fail log in svs_platform_probe() to help catch which step fails quickly >> >>>>>>>> - Remove struct svs_bank member "pd_dev" because all subsys device's power domain has been merged into one node like above [3] >> >>>>>>>> >> >>>>>>>> Test in below environment: >> >>>>>>>> SW: Integration Tree [4] + Thermal patch [5] + SVS v25 (this patchset) >> >>>>>>>> HW: mt8183-Krane >> >>>>>>>> >> >>>>>>>> [4] https://protect2.fireeye.com/v1/url?k=847bae75-e5f0bb43-847a253a-000babff9b5d-0b6f42041b9dea1d&q=1&e=37a26c43-8564-4808-9701-dc76d1ebbb27&u=https%3A%2F%2Fgithub.com%2Fwens%2Flinux%2Fcommits%2Fmt8183-cpufreq-cci-svs-test >> >>>>>>> >> >>>>>>> I've updated my branch to include all the latest versions of the relevant >> >>>>>>> patch series: >> >>>>>>> >> >>>>>>> - anx7625 DPI bus type series v2 (so the display works) >> >>>>>>> - MT8183 thermal series v9 (this seems to have been overlooked by the >> >>>>>>> maintainer) >> >>>>>>> - MTK SVS driver series v25 >> >>>>>>> - devfreq: cpu based scaling support to passive governor series v5 >> >>>>>>> - MTK CCI devfreq series v4 >> >>>>>>> - MT8183 cpufreq series v7 >> >>>>>>> - Additional WIP patches for panfrost MTK devfreq >> >>>>>> >> >>>>>> Thanks for preparing an integration branch Chen-Yu. >> >>>>>> >> >>>>>> I'm testing this on mt8183-pumpkin with one patch to add the CCI >> >>>>>> regulator[1], and the defconfig you posted in a previous rev of this >> >>>>>> series, but the CCI driver still causes a fault on boot[2] on my >> >>>>>> platform. >> >>>>>> >> >>>>>> I mentioned in earlier reviews that I think there's potentially a race >> >>>>>> between CCI and SVS loading since they are co-dependent. My hunch is >> >>>>>> that this is still not being handled properly. >> >>>>> >> >>>>> Ah, actually it's crashing when I try to boot the platform with >> >>>>> `maxcpus=4` on the cmdline (which I have to do because mt8183-pumpkin is >> >>>>> unstable upstream with the 2nd cluster enabled.) >> >> >> >> This warning message is printed by 'WARN_ON(cpufreq_passive_unregister_notifier(devfreq))' >> >> on devfreq passive governor. >> >> >> >> If the cpufreq drivers are not probed before of probing cci devfreq driver >> >> with passive governor, passive governor shows this warning message. >> >> Because passive governor with CPUFREQ_PARENT_DEV depends on the cpufreq driver >> >> in order to get 'struct cpufreq_policy'[2]. >> >> >> >> [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/tree/drivers/devfreq/governor_passive.c?h=devfreq-testing#n339 >> >> [2] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/tree/drivers/devfreq/governor_passive.c?h=devfreq-testing#n282 >> >> >> >> But, as I knew, this message might not stop the kernel. Just show the warning >> >> message and then return -EPROBE_DEFER error. It means that maybe try to >> >> probe the cci devfreq driver on late time of kernel booting >> >> and then will be working. But, I need the full kernel booting log >> >> and the booting sequence of between cpufreq and cci devfreq driver. >> > >> > Maybe just use a standard dev_warn() instead? WARN_ON causes all sorts >> > of panicking in developers' minds. :p >> > >> >> In order to fix your issue, could you share the full booting log? >> >> And if possible, please explain the more detailed something about this. >> > >> > The shortened version is that on an 8 core system, with maxcpus=4, >> > only the first four cores are booted and have cpufreq associated. >> > I've not actually used this mechanism, so I don't really know what >> > happens if the other cores are brought up later with hotplug. Is >> > cpufreq expected to attach to them? >> > >> > Maybe Kevin can add some more details. >> > >> > >> > ChenYu >> > >> > >> >>>>> >> >>>>> The CCI driver should be a bit more robust about detecting >> >>>>> available/online CPUs >> >>>> >> >>>> This all seems to be handled in the devfreq passive governor. >> >>> >> >>> Well, that's the initial crash. But the SVS driver will also go through >> >>> its svs_mt8183_banks[] array (including both big & little clusters) and >> >>> try to init SVS, so presumably that will have some problems also if only >> >>> one cluster is enabled. >> >>> >> >>>> And presumably we'd like to have CCI devfreq running even if just one >> >>>> core was booted. >> >>> >> >>> Yes, I assume so also. >> >>> >> >>>> Added Chanwoo for more ideas. >> >>> >> >>> OK, thanks. >> >>> >> >>> Kevin >> >> >> I tested the passive governor with my temporary test code >> on odroid-xu3 which contains the big.LITTLE cluster (Octa-core). >> >> >> [Sequence of cpufreq/devfreq driver] >> 1. Turn on all cpus >> 2. Probed cpufreq driver >> 3. Probed devfreq driver using passive governor with CPUFREQ_PARENT_DEV >> >> In my test case, there are no warning message during kernel booting. >> Also when scaling the cpu frequency of cpus of big.LITTLE clusters, >> temporary devfreq driver receives the notfication and then >> calculate the target frequency of devfreq device by iterating online cpu. >> >> If there are any h/w constraints on your case, please let me know. > > Could you run your system with maxcpus=4 added to your cmdline? > This is what Kevin was running. > > The current result is that the latter four cores aren't booted, so no > cpufreq tied to them, and the passive governor will fail to get their > cpufreq_policy. As mentioned before, the code path used to have a > WARN_ON(). Now it's a dev_warn(). It will still fail initialization > though. > > We're wondering if devfreq passive governor should be made to work > even if not all cpu cores are available when it probes. For info, here is a boot log[1] from mt8183-pumpkin board where I'm testing Chen-Yu's lastest integration branch. As Chen-Yu said, the part that makes it trigger the warn is disabling some of the CPUs *at boot time*. In this case, I'm passing `maxcpus=4` on the kernel command line. Kevin [1] https://termbin.com/zidi _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek