Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5814d779-0635-43fe-3fe8-31c130f05b3a@collabora.com>
Date:   Fri, 31 Mar 2023 10:11:07 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH v1 RESEND 2/2] drm/panfrost: Add basic support for speed
 binning
To:     airlied@gmail.com, boris.brezillon@collabora.com
Cc:     daniel@ffwll.ch, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, wenst@chromium.org,
        steven.price@arm.com, alyssa.rosenzweig@collabora.com,
        robh@kernel.org, dri-devel@lists.freedesktop.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        kernel@collabora.com
References: <20230323090822.61766-1-angelogioacchino.delregno@collabora.com>
 <20230323090822.61766-3-angelogioacchino.delregno@collabora.com>
Content-Language: en-US
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>
In-Reply-To: <20230323090822.61766-3-angelogioacchino.delregno@collabora.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Il 23/03/23 10:08, AngeloGioacchino Del Regno ha scritto:
> Some SoCs implementing ARM Mali GPUs are subject to speed binning:
> this means that some versions of the same SoC model may need to be
> limited to a slower frequency compared to the other:
> this is being addressed by reading nvmem (usually, an eFuse array)
> containing a number that identifies the speed binning of the chip,
> which is usually related to silicon quality.
> 
> To address such situation, add basic support for reading the
> speed-bin through nvmem, as to make it possible to specify the
> supported hardware in the OPP table for GPUs.
> This commit also keeps compatibility with any platform that does
> not specify (and does not even support) speed-binning.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Hello maintainers,
I've seen that this got archived in the dri-devel patchwork; because of that and
only that, I'm sending this ping to get this patch reviewed.

(perhaps we can even get it picked for v6.4?)

Regards,
Angelo

> ---
>   drivers/gpu/drm/panfrost/panfrost_devfreq.c | 30 +++++++++++++++++++++
>   1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> index fe5f12f16a63..58dfb15a8757 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> @@ -4,6 +4,7 @@
>   #include <linux/clk.h>
>   #include <linux/devfreq.h>
>   #include <linux/devfreq_cooling.h>
> +#include <linux/nvmem-consumer.h>
>   #include <linux/platform_device.h>
>   #include <linux/pm_opp.h>
>   
> @@ -82,6 +83,31 @@ static struct devfreq_dev_profile panfrost_devfreq_profile = {
>   	.get_dev_status = panfrost_devfreq_get_dev_status,
>   };
>   
> +static int panfrost_read_speedbin(struct device *dev)
> +{
> +	u32 val;
> +	int ret;
> +
> +	ret = nvmem_cell_read_variable_le_u32(dev, "speed-bin", &val);
> +	if (ret) {
> +		/*
> +		 * -ENOENT means that this platform doesn't support speedbins
> +		 * as it didn't declare any speed-bin nvmem: in this case, we
> +		 * keep going without it; any other error means that we are
> +		 * supposed to read the bin value, but we failed doing so.
> +		 */
> +		if (ret != -ENOENT) {
> +			DRM_DEV_ERROR(dev, "Cannot read speed-bin (%d).", ret);
> +			return ret;
> +		}
> +
> +		return 0;
> +	}
> +	DRM_DEV_DEBUG(dev, "Using speed-bin = 0x%x\n", val);
> +
> +	return devm_pm_opp_set_supported_hw(dev, &val, 1);
> +}
> +
>   int panfrost_devfreq_init(struct panfrost_device *pfdev)
>   {
>   	int ret;
> @@ -101,6 +127,10 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
>   		return 0;
>   	}
>   
> +	ret = panfrost_read_speedbin(dev);
> +	if (ret)
> +		return ret;
> +
>   	ret = devm_pm_opp_set_regulators(dev, pfdev->comp->supply_names);
>   	if (ret) {
>   		/* Continue if the optional regulator is missing */