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;
Subject: Re: [RFC PATCH 0/2] PM / devfreq: Add delayed timer for polling
To:     Chanwoo Choi <cw00.choi@samsung.com>
Cc:     lukasz.luba@arm.com, k.konieczny@samsung.com, krzk@kernel.org,
        kgene@kernel.org, s.nawrocki@samsung.com,
        willy.mh.wolff.ml@gmail.com, chanwoo@kernel.org,
        myungjoo.ham@samsung.com, kyungmin.park@samsung.com,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-samsung-soc@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org
From:   Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Message-ID: <bb3a5b29-c90f-5668-e46d-9eeeb5a9b973@samsung.com>
Date:   Wed, 8 Jul 2020 12:52:53 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
        Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <20200703062622.11773-1-cw00.choi@samsung.com>
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <CGME20200703061508epcas1p171aa3c0ab832b77e5837d8bd1e563742@epcas1p1.samsung.com>
        <20200703062622.11773-1-cw00.choi@samsung.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


Hi Chanwoo,

On 7/3/20 8:26 AM, Chanwoo Choi wrote:
> Add the delayed timer to devfreq framework in order to support
> the periodical polling mode without stop caused by CPU idle state.

Thank you, this patchset looks fine to me and is a step in the right
direction:

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

> Some Non-CPU device must need to monitor the device status like
> utilization regardless of CPU state.

This is probably true for all devfreq devices using simple_ondemand
governor by default:

drivers/devfreq/exynos-bus.c
drivers/devfreq/rk3399_dmc.c
drivers/devfreq/tegra20-devfreq.c
drivers/gpu/drm/lima/lima_devfreq.c
drivers/gpu/drm/msm/msm_gpu.c
drivers/gpu/drm/panfrost/panfrost_devfreq.c
drivers/memory/samsung/exynos5422-dmc.c
drivers/scsi/ufs/ufshcd.c

With devfreq device polling being "coupled" to CPU idle state
the devfreq subsystem behavior is completely unpredictable and
unreliable.

It affects both performance (device opp change up happening too
late) and power consumption (device opp change down happening too
late).

It also causes hardware usage counters support to report too high
values (because of CPU idle "coupling" the real polling period
becomes larger than maximum period supported by the counter and
the counter becomes fully "saturated") which negatively affects
power consumption (as has been observed when using Odroid XU3/4).

[ The only upside of using such "coupling" is lowered CPU power
  usage (in some situations) but at the (unacceptable IMHO) cost
  of the correctness of operations of devfreq subsystem. ]

Unfortunately this patchset currently fixes only exynos5422-dmc
devfreq driver. To fix problems for Exynos platforms we need to
also fix exynos-bus devfreq driver.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> - patch1 explains the detailed reason why the delayed timer is required.
> - patch2 initializes that exynos5422-dmc device use delayed timer as default
> instead of deferrable timer.
> 
> Chanwoo Choi (2):
>   PM / devfreq: Add support delayed timer for polling mode
>   memory: samsung: exynos5422-dmc: Use delayed timer as default
> 
>  Documentation/ABI/testing/sysfs-class-devfreq | 12 +++
>  drivers/devfreq/devfreq.c                     | 83 ++++++++++++++++++-
>  drivers/memory/samsung/exynos5422-dmc.c       |  1 +
>  include/linux/devfreq.h                       |  9 ++
>  4 files changed, 104 insertions(+), 1 deletion(-)