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=-11.5 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable 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 78638C433ED for ; Tue, 14 Jul 2020 09:01:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 58EF521D7D for ; Tue, 14 Jul 2020 09:01:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726752AbgGNJBX (ORCPT ); Tue, 14 Jul 2020 05:01:23 -0400 Received: from foss.arm.com ([217.140.110.172]:54276 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725816AbgGNJBX (ORCPT ); Tue, 14 Jul 2020 05:01:23 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3274E31B; Tue, 14 Jul 2020 02:01:22 -0700 (PDT) Received: from [10.37.12.45] (unknown [10.37.12.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 039A83F7BB; Tue, 14 Jul 2020 02:01:18 -0700 (PDT) Subject: Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode To: Bartlomiej Zolnierkiewicz Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, willy.mh.wolff.ml@gmail.com, k.konieczny@samsung.com, cw00.choi@samsung.com, krzk@kernel.org, chanwoo@kernel.org, myungjoo.ham@samsung.com, kyungmin.park@samsung.com, s.nawrocki@samsung.com, kgene@kernel.org References: <20200710191122.11029-1-lukasz.luba@arm.com> <20200710191122.11029-3-lukasz.luba@arm.com> <1a389137-cab5-124a-e198-8be3bc2ca841@samsung.com> From: Lukasz Luba Message-ID: <3154b8d2-1fa8-c69d-8a9d-05832e12fdd1@arm.com> Date: Tue, 14 Jul 2020 10:01:16 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <1a389137-cab5-124a-e198-8be3bc2ca841@samsung.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Bartek, On 7/14/20 8:42 AM, Bartlomiej Zolnierkiewicz wrote: > > Hi, > > On 7/10/20 9:11 PM, Lukasz Luba wrote: >> The driver can operate in two modes relaying on devfreq monitoring >> mechanism which periodically checks the device status or it can use >> interrupts when they are provided by loaded Device Tree. The newly >> introduced module parameter can be used to choose between devfreq >> monitoring and internal interrupts without modifying the Device Tree. >> It also sets devfreq monitoring as default when the parameter is not set >> (also the case for default when the driver is not built as a module). > > Could you please explain why should we leave the IRQ mode > support in the dmc driver? I am still experimenting with the IRQ mode in DMC, but have limited time for it and no TRM. The IRQ mode in memory controller or bus controller has one major advantage: is more interactive. In polling we have fixed period, i.e. 100ms - that's a lot when we have a sudden, latency sensitive workload. There might be no check of the device load for i.e. 99ms, but the tasks with such workload started running. That's a long period of a few frames which are likely to be junked. Should we adjust polling interval to i.e. 10ms, I don't think so. There is no easy way to address all of the scenarios. > > What are the advantages over the polling mode? As described above: more reactive to sudden workload, which might be latency sensitive and cause junk frames. Drawback: not best in benchmarks which are randomly jumping over the data set, causing low traffic on memory. It could be mitigated as Sylwester described with not only one type of interrupt, but another, which could 'observe' also other information type in the counters and fire. > > In what scenarios it should be used? System like Android with GUI, when there is this sudden workload quite often. I think the interconnect could help here and would adjust the DMC freq upfront. Although I don't know if interconnect on Exynos5422 is in your scope in near future. Of course the interconnect will not cover all scenarios either. > > [ If this is only for documentation purposes then it should be > removed as it would stay in (easily accessible) git history > anyway.. ] The current interrupt mode is definitely not perfect and switching to devfreq monitoring mode has more sense. On the other hand, it still has potential, until there is no interconnect for this SoC. I will continue experimenting with irq mode, so I would like to still have the code in the driver. Regards, Lukasz > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics > >> Reported-by: Willy Wolff >> Signed-off-by: Lukasz Luba >> --- >> drivers/memory/samsung/exynos5422-dmc.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c >> index e03ee35f0ab5..53bfe6b7b703 100644 >> --- a/drivers/memory/samsung/exynos5422-dmc.c >> +++ b/drivers/memory/samsung/exynos5422-dmc.c >> @@ -12,6 +12,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -21,6 +22,10 @@ >> #include "../jedec_ddr.h" >> #include "../of_memory.h" >> >> +static int irqmode; >> +module_param(irqmode, int, 0644); >> +MODULE_PARM_DESC(irqmode, "Enable IRQ mode (0=off [default], 1=on)"); >> + >> #define EXYNOS5_DREXI_TIMINGAREF (0x0030) >> #define EXYNOS5_DREXI_TIMINGROW0 (0x0034) >> #define EXYNOS5_DREXI_TIMINGDATA0 (0x0038) >> @@ -1428,7 +1433,7 @@ static int exynos5_dmc_probe(struct platform_device *pdev) >> /* There is two modes in which the driver works: polling or IRQ */ >> irq[0] = platform_get_irq_byname(pdev, "drex_0"); >> irq[1] = platform_get_irq_byname(pdev, "drex_1"); >> - if (irq[0] > 0 && irq[1] > 0) { >> + if (irq[0] > 0 && irq[1] > 0 && irqmode) { >> ret = devm_request_threaded_irq(dev, irq[0], NULL, >> dmc_irq_thread, IRQF_ONESHOT, >> dev_name(dev), dmc); >> @@ -1485,7 +1490,7 @@ static int exynos5_dmc_probe(struct platform_device *pdev) >> if (dmc->in_irq_mode) >> exynos5_dmc_start_perf_events(dmc, PERF_COUNTER_START_VALUE); >> >> - dev_info(dev, "DMC initialized\n"); >> + dev_info(dev, "DMC initialized, in irq mode: %d\n", dmc->in_irq_mode); >> >> return 0; >> >> > 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=-11.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable 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 019E0C433E4 for ; Tue, 14 Jul 2020 09:03:10 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 C222521927 for ; Tue, 14 Jul 2020 09:03:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="aSPPF831" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C222521927 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+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=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=kOgMn85b4brqk+viptMBqWUv+rf5/2DJTRujF9oOlqo=; b=aSPPF831ZZiENc53qz2h9h6pE rmAS4bNBO2axYZcmc2VmNgfPjVa05jKaXKBaxH4j5hZSs1NgLTjQgNPj1bcscmwPYJa6/bqVQFLd6 QtsHflO/HkFUHucFjLA0roYZoxD1i+QQMpR7y7G/KmPOoYzXfvglIa5h0kL6KNNteuNDFJ9gwRjcg 4FPlFvpB2umBOm2m74paLsQJH3YbFzWD+VqDSoN0bLu6QwqSl31puM4/Bm5jE/a5vAf+mY+6reFiV yAuSkfuw4lt+OjW8p+tBP48RRHvwGR4egoMDlo62g1dIAVJo51F+vSm/TL5Ylr2FoHHRXykHH5LK6 r8q4SzlXA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jvGof-0000Vk-7V; Tue, 14 Jul 2020 09:01:33 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jvGoX-0000QG-5C for linux-arm-kernel@lists.infradead.org; Tue, 14 Jul 2020 09:01:26 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3274E31B; Tue, 14 Jul 2020 02:01:22 -0700 (PDT) Received: from [10.37.12.45] (unknown [10.37.12.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 039A83F7BB; Tue, 14 Jul 2020 02:01:18 -0700 (PDT) Subject: Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode To: Bartlomiej Zolnierkiewicz References: <20200710191122.11029-1-lukasz.luba@arm.com> <20200710191122.11029-3-lukasz.luba@arm.com> <1a389137-cab5-124a-e198-8be3bc2ca841@samsung.com> From: Lukasz Luba Message-ID: <3154b8d2-1fa8-c69d-8a9d-05832e12fdd1@arm.com> Date: Tue, 14 Jul 2020 10:01:16 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <1a389137-cab5-124a-e198-8be3bc2ca841@samsung.com> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200714_050125_434519_2AA51F5E X-CRM114-Status: GOOD ( 32.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: linux-samsung-soc@vger.kernel.org, s.nawrocki@samsung.com, linux-pm@vger.kernel.org, k.konieczny@samsung.com, linux-kernel@vger.kernel.org, krzk@kernel.org, cw00.choi@samsung.com, kyungmin.park@samsung.com, willy.mh.wolff.ml@gmail.com, myungjoo.ham@samsung.com, chanwoo@kernel.org, kgene@kernel.org, linux-arm-kernel@lists.infradead.org Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Bartek, On 7/14/20 8:42 AM, Bartlomiej Zolnierkiewicz wrote: > > Hi, > > On 7/10/20 9:11 PM, Lukasz Luba wrote: >> The driver can operate in two modes relaying on devfreq monitoring >> mechanism which periodically checks the device status or it can use >> interrupts when they are provided by loaded Device Tree. The newly >> introduced module parameter can be used to choose between devfreq >> monitoring and internal interrupts without modifying the Device Tree. >> It also sets devfreq monitoring as default when the parameter is not set >> (also the case for default when the driver is not built as a module). > > Could you please explain why should we leave the IRQ mode > support in the dmc driver? I am still experimenting with the IRQ mode in DMC, but have limited time for it and no TRM. The IRQ mode in memory controller or bus controller has one major advantage: is more interactive. In polling we have fixed period, i.e. 100ms - that's a lot when we have a sudden, latency sensitive workload. There might be no check of the device load for i.e. 99ms, but the tasks with such workload started running. That's a long period of a few frames which are likely to be junked. Should we adjust polling interval to i.e. 10ms, I don't think so. There is no easy way to address all of the scenarios. > > What are the advantages over the polling mode? As described above: more reactive to sudden workload, which might be latency sensitive and cause junk frames. Drawback: not best in benchmarks which are randomly jumping over the data set, causing low traffic on memory. It could be mitigated as Sylwester described with not only one type of interrupt, but another, which could 'observe' also other information type in the counters and fire. > > In what scenarios it should be used? System like Android with GUI, when there is this sudden workload quite often. I think the interconnect could help here and would adjust the DMC freq upfront. Although I don't know if interconnect on Exynos5422 is in your scope in near future. Of course the interconnect will not cover all scenarios either. > > [ If this is only for documentation purposes then it should be > removed as it would stay in (easily accessible) git history > anyway.. ] The current interrupt mode is definitely not perfect and switching to devfreq monitoring mode has more sense. On the other hand, it still has potential, until there is no interconnect for this SoC. I will continue experimenting with irq mode, so I would like to still have the code in the driver. Regards, Lukasz > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics > >> Reported-by: Willy Wolff >> Signed-off-by: Lukasz Luba >> --- >> drivers/memory/samsung/exynos5422-dmc.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c >> index e03ee35f0ab5..53bfe6b7b703 100644 >> --- a/drivers/memory/samsung/exynos5422-dmc.c >> +++ b/drivers/memory/samsung/exynos5422-dmc.c >> @@ -12,6 +12,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -21,6 +22,10 @@ >> #include "../jedec_ddr.h" >> #include "../of_memory.h" >> >> +static int irqmode; >> +module_param(irqmode, int, 0644); >> +MODULE_PARM_DESC(irqmode, "Enable IRQ mode (0=off [default], 1=on)"); >> + >> #define EXYNOS5_DREXI_TIMINGAREF (0x0030) >> #define EXYNOS5_DREXI_TIMINGROW0 (0x0034) >> #define EXYNOS5_DREXI_TIMINGDATA0 (0x0038) >> @@ -1428,7 +1433,7 @@ static int exynos5_dmc_probe(struct platform_device *pdev) >> /* There is two modes in which the driver works: polling or IRQ */ >> irq[0] = platform_get_irq_byname(pdev, "drex_0"); >> irq[1] = platform_get_irq_byname(pdev, "drex_1"); >> - if (irq[0] > 0 && irq[1] > 0) { >> + if (irq[0] > 0 && irq[1] > 0 && irqmode) { >> ret = devm_request_threaded_irq(dev, irq[0], NULL, >> dmc_irq_thread, IRQF_ONESHOT, >> dev_name(dev), dmc); >> @@ -1485,7 +1490,7 @@ static int exynos5_dmc_probe(struct platform_device *pdev) >> if (dmc->in_irq_mode) >> exynos5_dmc_start_perf_events(dmc, PERF_COUNTER_START_VALUE); >> >> - dev_info(dev, "DMC initialized\n"); >> + dev_info(dev, "DMC initialized, in irq mode: %d\n", dmc->in_irq_mode); >> >> return 0; >> >> > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel