Received: by 2002:a25:e74b:0:0:0:0:0 with SMTP id e72csp1596669ybh; Tue, 14 Jul 2020 02:02:22 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzZf3GvtbmgG4OKCtkIoPudqeH7oCjsryMPYSQj2Zuc1SMbAYuddXaSZkefmX9BOyPJpOxT X-Received: by 2002:a17:906:3e54:: with SMTP id t20mr3471684eji.471.1594717341970; Tue, 14 Jul 2020 02:02:21 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1594717341; cv=none; d=google.com; s=arc-20160816; b=qCC9UNx/4xHEARYC8zdaXxs/hVYpvPYqBf50c6dJ0BZC887cEN+51EXdI1SBCQj1On WspmCHqWXd0sh3dJrkqRaU9Y7dA+PW2Gf6kRk8ySkLX7HoMtMUA9dCGDQ7BwEN/8F6Da +R91zOH5rggoEWIHkdowwShl20+ZuTi0N/eePp62ePcpwj+GAbGWM44znyrhw9tnWhgq f+3yrooI+uCIg62wdYGmXB/uxkzGE70/rDKZG7rDW+r2iFuRZYmfzbUINXhtOJiBmlwn N9NqEKnROBlLZA7Qzp8AiCzUa0lMPWlVSiGBPsMhX2Wl2xIRaVNlPbwo6Wr8U5PxVleQ HlLg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject; bh=mm2giE63qmbqnCbD5mz3aUKHqZxUTgEO66+XZYz+8Q4=; b=pvLkBb3iM2FFwBnopp+/H5LV2lFyLTjiHLyV5sA9hyLnyWKi5DIbYCFJQMqOz8k5UG ZFts3IpqFBo4rkexg1Y9r0Vn8l6/lDXJr960Nkh2xzEQ/5+RjVr/dK9NWU1voqzXw5kV fHhcmw0nScbkbLn/rBSPUaeS5ELgdaTObYi5/PBzrzE6KXzP0CQLlyldO9dibsZ3Lyt2 P9wAqpb/qYXDauMeZ/oPxdaorCQbHdwxoJjV4PD6dDIp++q6O+TeSTqX7Jo3XmfxAKO6 0ETzo4ksAm6QpTd2hmkUveUXO4sJibv9UMb/ytV7U6vv27aLqrWajh+gSq7SVEthHmjU MGHg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id r3si267089ejr.664.2020.07.14.02.01.58; Tue, 14 Jul 2020 02:02:21 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726729AbgGNJBX (ORCPT + 99 others); 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; >> >> >