Re: [PATCH v2 2/4] drm/msm: dsi: Use OPP API to set clk/perf state

From: Matthias Kaehlcke <mka@chromium.org>
To: Rajendra Nayak <rnayak@codeaurora.org>
Cc: robdclark@gmail.com, sean@poorly.run, agross@kernel.org,
	bjorn.andersson@linaro.org, dri-devel@lists.freedesktop.org,
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/4] drm/msm: dsi: Use OPP API to set clk/perf state
Date: Mon, 6 Jul 2020 09:10:57 -0700	[thread overview]
Message-ID: <20200706161057.GG3191083@google.com> (raw)
In-Reply-To: <1593688151-22616-3-git-send-email-rnayak@codeaurora.org>

On Thu, Jul 02, 2020 at 04:39:09PM +0530, Rajendra Nayak wrote:
> On SDM845 and SC7180 DSI needs to express a performance state
> requirement on a power domain depending on the clock rates.
> Use OPP table from DT to register with OPP framework and use
> dev_pm_opp_set_rate() to set the clk/perf state.
> 
> dev_pm_opp_set_rate() is designed to be equivalent to clk_set_rate()
> for devices without an OPP table, hence the change works fine
> on devices/platforms which only need to set a clock rate.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 11ae5b8..09e16b8 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -14,6 +14,7 @@
>  #include <linux/of_graph.h>
>  #include <linux/of_irq.h>
>  #include <linux/pinctrl/consumer.h>
> +#include <linux/pm_opp.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/spinlock.h>
> @@ -111,6 +112,9 @@ struct msm_dsi_host {
>  	struct clk *pixel_clk_src;
>  	struct clk *byte_intf_clk;
>  
> +	struct opp_table *opp_table;
> +	bool has_opp_table;
> +
>  	u32 byte_clk_rate;
>  	u32 pixel_clk_rate;
>  	u32 esc_clk_rate;
> @@ -512,9 +516,10 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host)
>  	DBG("Set clk rates: pclk=%d, byteclk=%d",
>  		msm_host->mode->clock, msm_host->byte_clk_rate);
>  
> -	ret = clk_set_rate(msm_host->byte_clk, msm_host->byte_clk_rate);
> +	ret = dev_pm_opp_set_rate(&msm_host->pdev->dev,
> +				  msm_host->byte_clk_rate);
>  	if (ret) {
> -		pr_err("%s: Failed to set rate byte clk, %d\n", __func__, ret);
> +		pr_err("%s: dev_pm_opp_set_rate failed %d\n", __func__, ret);
>  		return ret;
>  	}
>  
> @@ -658,6 +663,8 @@ int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host)
>  
>  void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host)
>  {
> +	/* Drop the performance state vote */
> +	dev_pm_opp_set_rate(&msm_host->pdev->dev, 0);
>  	clk_disable_unprepare(msm_host->esc_clk);
>  	clk_disable_unprepare(msm_host->pixel_clk);
>  	if (msm_host->byte_intf_clk)
> @@ -1879,6 +1886,18 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
>  		goto fail;
>  	}
>  
> +	msm_host->opp_table = dev_pm_opp_set_clkname(&pdev->dev, "byte");
> +	if (IS_ERR(msm_host->opp_table))
> +		return PTR_ERR(msm_host->opp_table);
> +	/* OPP table is optional */
> +	ret = dev_pm_opp_of_add_table(&pdev->dev);
> +	if (!ret) {
> +		msm_host->has_opp_table = true;
> +	} else if (ret != -ENODEV) {
> +		dev_err(&pdev->dev, "invalid OPP table in device tree\n");

  		dev_pm_opp_put_clkname(msm_host->opp_table);

> +		return ret;
> +	}

With the missing _put_clkname() fixed:

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>