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: [PATCH v11 10/10] PM / devfreq: tegra30: Separate
 configurations per-SoC generation
To:     Dmitry Osipenko <digetx@gmail.com>,
        Thierry Reding <thierry.reding@gmail.com>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        Georgi Djakov <georgi.djakov@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Peter De Schrijver <pdeschrijver@nvidia.com>,
        MyungJoo Ham <myungjoo.ham@samsung.com>,
        Kyungmin Park <kyungmin.park@samsung.com>,
        Mikko Perttunen <cyndis@kapsi.fi>,
        Viresh Kumar <vireshk@kernel.org>,
        Peter Geis <pgwipeout@gmail.com>,
        Nicolas Chauvet <kwizart@gmail.com>,
        Krzysztof Kozlowski <krzk@kernel.org>
Cc:     linux-tegra@vger.kernel.org, linux-pm@vger.kernel.org,
        linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
        devicetree@vger.kernel.org
From:   Chanwoo Choi <cw00.choi@samsung.com>
Organization: Samsung Electronics
Message-ID: <1c4f7c6f-7fff-60c1-a7b1-a099a88df1c6@samsung.com>
Date:   Mon, 7 Dec 2020 10:33:06 +0900
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101
        Thunderbird/59.0
MIME-Version: 1.0
In-Reply-To: <20201203192439.16177-11-digetx@gmail.com>
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 101P
DLP-Filter: Pass
References: <20201203192439.16177-1-digetx@gmail.com>
        <CGME20201203192730epcas1p1005aa9bcd60748dd9ecc0374349aac8b@epcas1p1.samsung.com>
        <20201203192439.16177-11-digetx@gmail.com>
Precedence: bulk

On 12/4/20 4:24 AM, Dmitry Osipenko wrote:
> Previously we were using count-weight of the T124 for T30 in order to
> get EMC clock rate that was reasonable for T30. In fact the count-weight
> should be x2 times smaller on T30, but then devfreq was producing a bit
> too low EMC clock rate for ISO memory clients, like display controller
> for example.
> 
> Now both Tegra ACTMON and Tegra DRM display drivers support interconnect
> framework and display driver tells to ICC what a minimum memory bandwidth
> is needed, preventing FIFO underflows. Thus, now we can use a proper
> count-weight value for Tegra30 and MC_ALL device config needs a bit more
> aggressive boosting.
> 
> Add a separate ACTMON driver configuration that is specific to Tegra30.
> 
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Nicolas Chauvet <kwizart@gmail.com>
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/devfreq/tegra30-devfreq.c | 68 ++++++++++++++++++++++++-------
>  1 file changed, 54 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
> index 145ef91ae092..117cad7968ab 100644
> --- a/drivers/devfreq/tegra30-devfreq.c
> +++ b/drivers/devfreq/tegra30-devfreq.c
> @@ -57,13 +57,6 @@
>  #define ACTMON_BELOW_WMARK_WINDOW				3
>  #define ACTMON_BOOST_FREQ_STEP					16000
>  
> -/*
> - * Activity counter is incremented every 256 memory transactions, and each
> - * transaction takes 4 EMC clocks for Tegra124; So the COUNT_WEIGHT is
> - * 4 * 256 = 1024.
> - */
> -#define ACTMON_COUNT_WEIGHT					0x400
> -
>  /*
>   * ACTMON_AVERAGE_WINDOW_LOG2: default value for @DEV_CTRL_K_VAL, which
>   * translates to 2 ^ (K_VAL + 1). ex: 2 ^ (6 + 1) = 128
> @@ -111,7 +104,7 @@ enum tegra_actmon_device {
>  	MCCPU,
>  };
>  
> -static const struct tegra_devfreq_device_config actmon_device_configs[] = {
> +static const struct tegra_devfreq_device_config tegra124_device_configs[] = {
>  	{
>  		/* MCALL: All memory accesses (including from the CPUs) */
>  		.offset = 0x1c0,
> @@ -133,6 +126,28 @@ static const struct tegra_devfreq_device_config actmon_device_configs[] = {
>  	},
>  };
>  
> +static const struct tegra_devfreq_device_config tegra30_device_configs[] = {
> +	{
> +		/* MCALL: All memory accesses (including from the CPUs) */
> +		.offset = 0x1c0,
> +		.irq_mask = 1 << 26,
> +		.boost_up_coeff = 200,
> +		.boost_down_coeff = 50,
> +		.boost_up_threshold = 20,
> +		.boost_down_threshold = 10,
> +	},
> +	{
> +		/* MCCPU: memory accesses from the CPUs */
> +		.offset = 0x200,
> +		.irq_mask = 1 << 25,
> +		.boost_up_coeff = 800,
> +		.boost_down_coeff = 40,
> +		.boost_up_threshold = 27,
> +		.boost_down_threshold = 10,
> +		.avg_dependency_threshold = 16000, /* 16MHz in kHz units */
> +	},
> +};
> +
>  /**
>   * struct tegra_devfreq_device - state specific to an ACTMON device
>   *
> @@ -155,6 +170,12 @@ struct tegra_devfreq_device {
>  	unsigned long target_freq;
>  };
>  
> +struct tegra_devfreq_soc_data {
> +	const struct tegra_devfreq_device_config *configs;
> +	/* Weight value for count measurements */
> +	unsigned int count_weight;
> +};
> +
>  struct tegra_devfreq {
>  	struct devfreq		*devfreq;
>  	struct opp_table	*opp_table;
> @@ -171,11 +192,13 @@ struct tegra_devfreq {
>  	struct delayed_work	cpufreq_update_work;
>  	struct notifier_block	cpu_rate_change_nb;
>  
> -	struct tegra_devfreq_device devices[ARRAY_SIZE(actmon_device_configs)];
> +	struct tegra_devfreq_device devices[2];
>  
>  	unsigned int		irq;
>  
>  	bool			started;
> +
> +	const struct tegra_devfreq_soc_data *soc;
>  };
>  
>  struct tegra_actmon_emc_ratio {
> @@ -488,7 +511,7 @@ static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
>  	tegra_devfreq_update_avg_wmark(tegra, dev);
>  	tegra_devfreq_update_wmark(tegra, dev);
>  
> -	device_writel(dev, ACTMON_COUNT_WEIGHT, ACTMON_DEV_COUNT_WEIGHT);
> +	device_writel(dev, tegra->soc->count_weight, ACTMON_DEV_COUNT_WEIGHT);
>  	device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
>  
>  	val |= ACTMON_DEV_CTRL_ENB_PERIODIC;
> @@ -779,6 +802,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>  	if (!tegra)
>  		return -ENOMEM;
>  
> +	tegra->soc = of_device_get_match_data(&pdev->dev);
> +
>  	tegra->regs = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(tegra->regs))
>  		return PTR_ERR(tegra->regs);
> @@ -852,9 +877,9 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>  
>  	tegra->max_freq = rate / KHZ;
>  
> -	for (i = 0; i < ARRAY_SIZE(actmon_device_configs); i++) {
> +	for (i = 0; i < ARRAY_SIZE(tegra->devices); i++) {
>  		dev = tegra->devices + i;
> -		dev->config = actmon_device_configs + i;
> +		dev->config = tegra->soc->configs + i;
>  		dev->regs = tegra->regs + dev->config->offset;
>  	}
>  
> @@ -916,9 +941,24 @@ static int tegra_devfreq_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct tegra_devfreq_soc_data tegra124_soc = {
> +	.configs = tegra124_device_configs,
> +
> +	/*
> +	 * Activity counter is incremented every 256 memory transactions,
> +	 * and each transaction takes 4 EMC clocks.
> +	 */
> +	.count_weight = 4 * 256,
> +};
> +
> +static const struct tegra_devfreq_soc_data tegra30_soc = {
> +	.configs = tegra30_device_configs,
> +	.count_weight = 2 * 256,
> +};
> +
>  static const struct of_device_id tegra_devfreq_of_match[] = {
> -	{ .compatible = "nvidia,tegra30-actmon" },
> -	{ .compatible = "nvidia,tegra124-actmon" },
> +	{ .compatible = "nvidia,tegra30-actmon",  .data = &tegra30_soc, },
> +	{ .compatible = "nvidia,tegra124-actmon", .data = &tegra124_soc, },
>  	{ },
>  };
>  
> 

Applied it. Thanks.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics