Re: [PATCH v1 RESEND 2/2] drm/panfrost: Add basic support for speed binning

[Boris Brezillon]

On Fri, 31 Mar 2023 10:11:07 +0200
AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
wrote:

> 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@xxxxxxxxxxxxx>  
> 
> 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.

Looks good to me. If you can get a DT maintainer to review the binding
(Rob?), I'd be happy to queue the series to drm-misc-next.

> 
> (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 */  
> 
>