[PATCH v2] drm/msm/dp: add opp_table corner voting support base on dp_ink_clk rate

[PATCH v2] drm/msm/dp: add opp_table corner voting support base on dp_ink_clk rate

[Kuogee Hsieh]

Set link rate by using OPP set rate api so that CX level will be set
accordingly based on the link rate.

Changes in v2:
-- remove dev from dp_ctrl_put() parameters
-- Add more information to commit message

Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c    | 27 ++++++++++++++++++
 drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
 drivers/gpu/drm/msm/dp/dp_power.c   | 44 ++++++++++++++++++++++++++---
 drivers/gpu/drm/msm/dp/dp_power.h   |  2 +-
 4 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 2e3e1917351f..bbd6e63f0c3f 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -10,6 +10,7 @@
 #include <linux/delay.h>
 #include <linux/phy/phy.h>
 #include <linux/phy/phy-dp.h>
+#include <linux/pm_opp.h>
 #include <drm/drm_fixed.h>
 #include <drm/drm_dp_helper.h>
 #include <drm/drm_print.h>
@@ -76,6 +77,8 @@ struct dp_ctrl_private {
 	struct dp_parser *parser;
 	struct dp_catalog *catalog;
 
+	struct opp_table *opp_table;
+
 	struct completion idle_comp;
 	struct completion video_comp;
 };
@@ -1836,6 +1839,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
 			struct dp_parser *parser)
 {
 	struct dp_ctrl_private *ctrl;
+	int ret;
 
 	if (!dev || !panel || !aux ||
 	    !link || !catalog) {
@@ -1849,6 +1853,20 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
 		return ERR_PTR(-ENOMEM);
 	}
 
+	ctrl->opp_table = dev_pm_opp_set_clkname(dev, "ctrl_link");
+	if (IS_ERR(ctrl->opp_table)) {
+		dev_err(dev, "invalid DP OPP table in device tree\n");
+		ctrl->opp_table = NULL;
+	} else {
+		/* OPP table is optional */
+		ret = dev_pm_opp_of_add_table(dev);
+		if (ret) {
+			dev_err(dev, "failed to add DP OPP table\n");
+			dev_pm_opp_put_clkname(ctrl->opp_table);
+			ctrl->opp_table = NULL;
+		}
+	}
+
 	init_completion(&ctrl->idle_comp);
 	init_completion(&ctrl->video_comp);
 
@@ -1866,4 +1884,13 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
 
 void dp_ctrl_put(struct dp_ctrl *dp_ctrl)
 {
+	struct dp_ctrl_private *ctrl;
+
+	ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
+
+	if (ctrl->opp_table) {
+		dev_pm_opp_of_remove_table(ctrl->dev);
+		dev_pm_opp_put_clkname(ctrl->opp_table);
+		ctrl->opp_table = NULL;
+	}
 }
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 2372de2865e6..518778247464 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -688,7 +688,7 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
 		goto error;
 	}
 
-	dp->power = dp_power_get(dp->parser);
+	dp->power = dp_power_get(dev, dp->parser);
 	if (IS_ERR(dp->power)) {
 		rc = PTR_ERR(dp->power);
 		DRM_ERROR("failed to initialize power, rc = %d\n", rc);
diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c
index 17c1fc6a2d44..9c4ea00a5f2a 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.c
+++ b/drivers/gpu/drm/msm/dp/dp_power.c
@@ -8,12 +8,14 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/regulator/consumer.h>
+#include <linux/pm_opp.h>
 #include "dp_power.h"
 #include "msm_drv.h"
 
 struct dp_power_private {
 	struct dp_parser *parser;
 	struct platform_device *pdev;
+	struct device *dev;
 	struct clk *link_clk_src;
 	struct clk *pixel_provider;
 	struct clk *link_provider;
@@ -148,18 +150,51 @@ static int dp_power_clk_deinit(struct dp_power_private *power)
 	return 0;
 }
 
+static int dp_power_clk_set_link_rate(struct dp_power_private *power,
+			struct dss_clk *clk_arry, int num_clk, int enable)
+{
+	u32 rate;
+	int i, rc = 0;
+
+	for (i = 0; i < num_clk; i++) {
+		if (clk_arry[i].clk) {
+			if (clk_arry[i].type == DSS_CLK_PCLK) {
+				if (enable)
+					rate = clk_arry[i].rate;
+				else
+					rate = 0;
+
+				rc = dev_pm_opp_set_rate(power->dev, rate);
+				if (rc)
+					break;
+			}
+
+		}
+	}
+	return rc;
+}
+
 static int dp_power_clk_set_rate(struct dp_power_private *power,
 		enum dp_pm_type module, bool enable)
 {
 	int rc = 0;
 	struct dss_module_power *mp = &power->parser->mp[module];
 
-	if (enable) {
-		rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
+	if (module == DP_CTRL_PM) {
+		rc = dp_power_clk_set_link_rate(power, mp->clk_config, mp->num_clk, enable);
 		if (rc) {
-			DRM_ERROR("failed to set clks rate.\n");
+			DRM_ERROR("failed to set link clks rate\n");
 			return rc;
 		}
+	} else {
+
+		if (enable) {
+			rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
+			if (rc) {
+				DRM_ERROR("failed to set clks rate\n");
+				return rc;
+			}
+		}
 	}
 
 	rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, enable);
@@ -349,7 +384,7 @@ int dp_power_deinit(struct dp_power *dp_power)
 	return 0;
 }
 
-struct dp_power *dp_power_get(struct dp_parser *parser)
+struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser)
 {
 	struct dp_power_private *power;
 	struct dp_power *dp_power;
@@ -365,6 +400,7 @@ struct dp_power *dp_power_get(struct dp_parser *parser)
 
 	power->parser = parser;
 	power->pdev = parser->pdev;
+	power->dev = dev;
 
 	dp_power = &power->dp_power;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_power.h b/drivers/gpu/drm/msm/dp/dp_power.h
index 76743d755833..7d0327bbc0d5 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.h
+++ b/drivers/gpu/drm/msm/dp/dp_power.h
@@ -102,6 +102,6 @@ void dp_power_client_deinit(struct dp_power *power);
  * methods to be called by the client to configure the power related
  * modueles.
  */
-struct dp_power *dp_power_get(struct dp_parser *parser);
+struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser);
 
 #endif /* _DP_POWER_H_ */

base-commit: febaca2607b310168e5e6a284bdad488210c522f
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2] drm/msm/dp: add opp_table corner voting support base on dp_ink_clk rate

[Kuogee Hsieh]

Set link rate by using OPP set rate api so that CX level will be set
accordingly based on the link rate.

Changes in v2:
-- remove dev from dp_ctrl_put() parameters
-- Add more information to commit message

Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c    | 27 ++++++++++++++++++
 drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
 drivers/gpu/drm/msm/dp/dp_power.c   | 44 ++++++++++++++++++++++++++---
 drivers/gpu/drm/msm/dp/dp_power.h   |  2 +-
 4 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 2e3e1917351f..bbd6e63f0c3f 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -10,6 +10,7 @@
 #include <linux/delay.h>
 #include <linux/phy/phy.h>
 #include <linux/phy/phy-dp.h>
+#include <linux/pm_opp.h>
 #include <drm/drm_fixed.h>
 #include <drm/drm_dp_helper.h>
 #include <drm/drm_print.h>
@@ -76,6 +77,8 @@ struct dp_ctrl_private {
 	struct dp_parser *parser;
 	struct dp_catalog *catalog;
 
+	struct opp_table *opp_table;
+
 	struct completion idle_comp;
 	struct completion video_comp;
 };
@@ -1836,6 +1839,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
 			struct dp_parser *parser)
 {
 	struct dp_ctrl_private *ctrl;
+	int ret;
 
 	if (!dev || !panel || !aux ||
 	    !link || !catalog) {
@@ -1849,6 +1853,20 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
 		return ERR_PTR(-ENOMEM);
 	}
 
+	ctrl->opp_table = dev_pm_opp_set_clkname(dev, "ctrl_link");
+	if (IS_ERR(ctrl->opp_table)) {
+		dev_err(dev, "invalid DP OPP table in device tree\n");
+		ctrl->opp_table = NULL;
+	} else {
+		/* OPP table is optional */
+		ret = dev_pm_opp_of_add_table(dev);
+		if (ret) {
+			dev_err(dev, "failed to add DP OPP table\n");
+			dev_pm_opp_put_clkname(ctrl->opp_table);
+			ctrl->opp_table = NULL;
+		}
+	}
+
 	init_completion(&ctrl->idle_comp);
 	init_completion(&ctrl->video_comp);
 
@@ -1866,4 +1884,13 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
 
 void dp_ctrl_put(struct dp_ctrl *dp_ctrl)
 {
+	struct dp_ctrl_private *ctrl;
+
+	ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
+
+	if (ctrl->opp_table) {
+		dev_pm_opp_of_remove_table(ctrl->dev);
+		dev_pm_opp_put_clkname(ctrl->opp_table);
+		ctrl->opp_table = NULL;
+	}
 }
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 2372de2865e6..518778247464 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -688,7 +688,7 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
 		goto error;
 	}
 
-	dp->power = dp_power_get(dp->parser);
+	dp->power = dp_power_get(dev, dp->parser);
 	if (IS_ERR(dp->power)) {
 		rc = PTR_ERR(dp->power);
 		DRM_ERROR("failed to initialize power, rc = %d\n", rc);
diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c
index 17c1fc6a2d44..9c4ea00a5f2a 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.c
+++ b/drivers/gpu/drm/msm/dp/dp_power.c
@@ -8,12 +8,14 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/regulator/consumer.h>
+#include <linux/pm_opp.h>
 #include "dp_power.h"
 #include "msm_drv.h"
 
 struct dp_power_private {
 	struct dp_parser *parser;
 	struct platform_device *pdev;
+	struct device *dev;
 	struct clk *link_clk_src;
 	struct clk *pixel_provider;
 	struct clk *link_provider;
@@ -148,18 +150,51 @@ static int dp_power_clk_deinit(struct dp_power_private *power)
 	return 0;
 }
 
+static int dp_power_clk_set_link_rate(struct dp_power_private *power,
+			struct dss_clk *clk_arry, int num_clk, int enable)
+{
+	u32 rate;
+	int i, rc = 0;
+
+	for (i = 0; i < num_clk; i++) {
+		if (clk_arry[i].clk) {
+			if (clk_arry[i].type == DSS_CLK_PCLK) {
+				if (enable)
+					rate = clk_arry[i].rate;
+				else
+					rate = 0;
+
+				rc = dev_pm_opp_set_rate(power->dev, rate);
+				if (rc)
+					break;
+			}
+
+		}
+	}
+	return rc;
+}
+
 static int dp_power_clk_set_rate(struct dp_power_private *power,
 		enum dp_pm_type module, bool enable)
 {
 	int rc = 0;
 	struct dss_module_power *mp = &power->parser->mp[module];
 
-	if (enable) {
-		rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
+	if (module == DP_CTRL_PM) {
+		rc = dp_power_clk_set_link_rate(power, mp->clk_config, mp->num_clk, enable);
 		if (rc) {
-			DRM_ERROR("failed to set clks rate.\n");
+			DRM_ERROR("failed to set link clks rate\n");
 			return rc;
 		}
+	} else {
+
+		if (enable) {
+			rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
+			if (rc) {
+				DRM_ERROR("failed to set clks rate\n");
+				return rc;
+			}
+		}
 	}
 
 	rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, enable);
@@ -349,7 +384,7 @@ int dp_power_deinit(struct dp_power *dp_power)
 	return 0;
 }
 
-struct dp_power *dp_power_get(struct dp_parser *parser)
+struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser)
 {
 	struct dp_power_private *power;
 	struct dp_power *dp_power;
@@ -365,6 +400,7 @@ struct dp_power *dp_power_get(struct dp_parser *parser)
 
 	power->parser = parser;
 	power->pdev = parser->pdev;
+	power->dev = dev;
 
 	dp_power = &power->dp_power;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_power.h b/drivers/gpu/drm/msm/dp/dp_power.h
index 76743d755833..7d0327bbc0d5 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.h
+++ b/drivers/gpu/drm/msm/dp/dp_power.h
@@ -102,6 +102,6 @@ void dp_power_client_deinit(struct dp_power *power);
  * methods to be called by the client to configure the power related
  * modueles.
  */
-struct dp_power *dp_power_get(struct dp_parser *parser);
+struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser);
 
 #endif /* _DP_POWER_H_ */

base-commit: febaca2607b310168e5e6a284bdad488210c522f
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/msm/dp: add opp_table corner voting support base on dp_ink_clk rate

[khsieh]

On 2020-10-06 00:31, Rajendra Nayak wrote:
> On 10/4/2020 3:56 AM, Kuogee Hsieh wrote:
>> Set link rate by using OPP set rate api so that CX level will be set
>> accordingly based on the link rate.
>> 
>> Changes in v2:
>> -- remove dev from dp_ctrl_put() parameters
>> -- address review comments
> 
> This needs to go below '---' and should not be part of the
> change log.
> 
>> 
>> Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_ctrl.c    | 26 +++++++++++++++++
>>   drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
>>   drivers/gpu/drm/msm/dp/dp_power.c   | 44 
>> ++++++++++++++++++++++++++---
>>   drivers/gpu/drm/msm/dp/dp_power.h   |  2 +-
>>   4 files changed, 68 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c 
>> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> index 2e3e1917351f..6eb9cdad1421 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> @@ -10,6 +10,7 @@
>>   #include <linux/delay.h>
>>   #include <linux/phy/phy.h>
>>   #include <linux/phy/phy-dp.h>
>> +#include <linux/pm_opp.h>
>>   #include <drm/drm_fixed.h>
>>   #include <drm/drm_dp_helper.h>
>>   #include <drm/drm_print.h>
>> @@ -76,6 +77,8 @@ struct dp_ctrl_private {
>>   	struct dp_parser *parser;
>>   	struct dp_catalog *catalog;
>>   +	struct opp_table *opp_table;
>> +
>>   	struct completion idle_comp;
>>   	struct completion video_comp;
>>   };
>> @@ -1836,6 +1839,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, 
>> struct dp_link *link,
>>   			struct dp_parser *parser)
>>   {
>>   	struct dp_ctrl_private *ctrl;
>> +	int ret;
>>     	if (!dev || !panel || !aux ||
>>   	    !link || !catalog) {
>> @@ -1849,6 +1853,19 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, 
>> struct dp_link *link,
>>   		return ERR_PTR(-ENOMEM);
>>   	}
>>   +	ctrl->opp_table = dev_pm_opp_set_clkname(dev, "ctrl_link");
>> +	if (IS_ERR(ctrl->opp_table)) {
>> +		dev_err(dev, "invalid DP OPP table in device tree\n");
> 
> You do this regardless of an OPP table in DT, so for starters the error
> message is wrong. Secondly this can return you a -EPROBE_DEFER if the
> clock driver isn't ready yet.
> So the ideal thing to do here, is return a PTR_ERR(ctrl->opp_table)
> 
>> +		ctrl->opp_table = NULL;
>> +	} else {
>> +		/* OPP table is optional */
>> +		ret = dev_pm_opp_of_add_table(dev);
>> +		if (ret && ret != -ENODEV) {
>> +			dev_pm_opp_put_clkname(ctrl->opp_table);
>> +			ctrl->opp_table = NULL;
>> +		}
>> +	}
>> +
>>   	init_completion(&ctrl->idle_comp);
>>   	init_completion(&ctrl->video_comp);
>>   @@ -1866,4 +1883,13 @@ struct dp_ctrl *dp_ctrl_get(struct device 
>> *dev, struct dp_link *link,
>>     void dp_ctrl_put(struct dp_ctrl *dp_ctrl)
>>   {
>> +	struct dp_ctrl_private *ctrl;
>> +
>> +	ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
>> +
>> +	if (ctrl->opp_table) {
>> +		dev_pm_opp_of_remove_table(ctrl->dev);
>> +		dev_pm_opp_put_clkname(ctrl->opp_table);
>> +		ctrl->opp_table = NULL;
>> +	}
>>   }
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
>> b/drivers/gpu/drm/msm/dp/dp_display.c
>> index e175aa3fd3a9..269f83550b46 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -698,7 +698,7 @@ static int dp_init_sub_modules(struct 
>> dp_display_private *dp)
>>   		goto error;
>>   	}
>>   -	dp->power = dp_power_get(dp->parser);
>> +	dp->power = dp_power_get(dev, dp->parser);
>>   	if (IS_ERR(dp->power)) {
>>   		rc = PTR_ERR(dp->power);
>>   		DRM_ERROR("failed to initialize power, rc = %d\n", rc);
>> diff --git a/drivers/gpu/drm/msm/dp/dp_power.c 
>> b/drivers/gpu/drm/msm/dp/dp_power.c
>> index 17c1fc6a2d44..9c4ea00a5f2a 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_power.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_power.c
>> @@ -8,12 +8,14 @@
>>   #include <linux/clk.h>
>>   #include <linux/clk-provider.h>
>>   #include <linux/regulator/consumer.h>
>> +#include <linux/pm_opp.h>
>>   #include "dp_power.h"
>>   #include "msm_drv.h"
>>     struct dp_power_private {
>>   	struct dp_parser *parser;
>>   	struct platform_device *pdev;
>> +	struct device *dev;
>>   	struct clk *link_clk_src;
>>   	struct clk *pixel_provider;
>>   	struct clk *link_provider;
>> @@ -148,18 +150,51 @@ static int dp_power_clk_deinit(struct 
>> dp_power_private *power)
>>   	return 0;
>>   }
>>   +static int dp_power_clk_set_link_rate(struct dp_power_private 
>> *power,
>> +			struct dss_clk *clk_arry, int num_clk, int enable)
>> +{
>> +	u32 rate;
>> +	int i, rc = 0;
>> +
>> +	for (i = 0; i < num_clk; i++) {
>> +		if (clk_arry[i].clk) {
>> +			if (clk_arry[i].type == DSS_CLK_PCLK) {
>> +				if (enable)
>> +					rate = clk_arry[i].rate;
>> +				else
>> +					rate = 0;
>> +
>> +				rc = dev_pm_opp_set_rate(power->dev, rate);
> 
> I am not sure how this is expected to work when you have multiple link 
> clocks,
> since you can only associate one of them with the OPP table which ends 
> up
> getting scaled when you do a dev_pm_opp_set_rate()
> Do you really have platforms which will have multiple link clocks?
this clk_arry[] contains two entries, dp_link_clk and dp_link_intf_clk.
only dp_link_clk with DSS_CLK_PCLK type, hence only dp_link_clk use 
dev_pm_opp_set_rate()
to set link rate.

> 
>> +				if (rc)
>> +					break;
>> +			}
>> +
>> +		}
>> +	}
>> +	return rc;
>> +}
>> +
>>   static int dp_power_clk_set_rate(struct dp_power_private *power,
>>   		enum dp_pm_type module, bool enable)
>>   {
>>   	int rc = 0;
>>   	struct dss_module_power *mp = &power->parser->mp[module];
>>   -	if (enable) {
>> -		rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
>> +	if (module == DP_CTRL_PM) {
>> +		rc = dp_power_clk_set_link_rate(power, mp->clk_config, mp->num_clk, 
>> enable);
>>   		if (rc) {
>> -			DRM_ERROR("failed to set clks rate.\n");
>> +			DRM_ERROR("failed to set link clks rate\n");
>>   			return rc;
>>   		}
>> +	} else {
>> +
> 
> extra blank line
> 
>> +		if (enable) {
>> +			rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
>> +			if (rc) {
>> +				DRM_ERROR("failed to set clks rate\n");
>> +				return rc;
>> +			}
>> +		}
>>   	}
>>     	rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, enable);
>> @@ -349,7 +384,7 @@ int dp_power_deinit(struct dp_power *dp_power)
>>   	return 0;
>>   }
>>   -struct dp_power *dp_power_get(struct dp_parser *parser)
>> +struct dp_power *dp_power_get(struct device *dev, struct dp_parser 
>> *parser)
>>   {
>>   	struct dp_power_private *power;
>>   	struct dp_power *dp_power;
>> @@ -365,6 +400,7 @@ struct dp_power *dp_power_get(struct dp_parser 
>> *parser)
>>     	power->parser = parser;
>>   	power->pdev = parser->pdev;
>> +	power->dev = dev;
>>     	dp_power = &power->dp_power;
>>   diff --git a/drivers/gpu/drm/msm/dp/dp_power.h 
>> b/drivers/gpu/drm/msm/dp/dp_power.h
>> index 76743d755833..7d0327bbc0d5 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_power.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_power.h
>> @@ -102,6 +102,6 @@ void dp_power_client_deinit(struct dp_power 
>> *power);
>>    * methods to be called by the client to configure the power related
>>    * modueles.
>>    */
>> -struct dp_power *dp_power_get(struct dp_parser *parser);
>> +struct dp_power *dp_power_get(struct device *dev, struct dp_parser 
>> *parser);
>>     #endif /* _DP_POWER_H_ */
>> 
>> base-commit: d1ea914925856d397b0b3241428f20b945e31434
> 
> ??

Re: [PATCH v2] drm/msm/dp: add opp_table corner voting support base on dp_ink_clk rate

[khsieh]

On 2020-10-06 00:31, Rajendra Nayak wrote:
> On 10/4/2020 3:56 AM, Kuogee Hsieh wrote:
>> Set link rate by using OPP set rate api so that CX level will be set
>> accordingly based on the link rate.
>> 
>> Changes in v2:
>> -- remove dev from dp_ctrl_put() parameters
>> -- address review comments
> 
> This needs to go below '---' and should not be part of the
> change log.
> 
>> 
>> Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_ctrl.c    | 26 +++++++++++++++++
>>   drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
>>   drivers/gpu/drm/msm/dp/dp_power.c   | 44 
>> ++++++++++++++++++++++++++---
>>   drivers/gpu/drm/msm/dp/dp_power.h   |  2 +-
>>   4 files changed, 68 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c 
>> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> index 2e3e1917351f..6eb9cdad1421 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> @@ -10,6 +10,7 @@
>>   #include <linux/delay.h>
>>   #include <linux/phy/phy.h>
>>   #include <linux/phy/phy-dp.h>
>> +#include <linux/pm_opp.h>
>>   #include <drm/drm_fixed.h>
>>   #include <drm/drm_dp_helper.h>
>>   #include <drm/drm_print.h>
>> @@ -76,6 +77,8 @@ struct dp_ctrl_private {
>>   	struct dp_parser *parser;
>>   	struct dp_catalog *catalog;
>>   +	struct opp_table *opp_table;
>> +
>>   	struct completion idle_comp;
>>   	struct completion video_comp;
>>   };
>> @@ -1836,6 +1839,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, 
>> struct dp_link *link,
>>   			struct dp_parser *parser)
>>   {
>>   	struct dp_ctrl_private *ctrl;
>> +	int ret;
>>     	if (!dev || !panel || !aux ||
>>   	    !link || !catalog) {
>> @@ -1849,6 +1853,19 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, 
>> struct dp_link *link,
>>   		return ERR_PTR(-ENOMEM);
>>   	}
>>   +	ctrl->opp_table = dev_pm_opp_set_clkname(dev, "ctrl_link");
>> +	if (IS_ERR(ctrl->opp_table)) {
>> +		dev_err(dev, "invalid DP OPP table in device tree\n");
> 
> You do this regardless of an OPP table in DT, so for starters the error
> message is wrong. Secondly this can return you a -EPROBE_DEFER if the
> clock driver isn't ready yet.
> So the ideal thing to do here, is return a PTR_ERR(ctrl->opp_table)
> 
>> +		ctrl->opp_table = NULL;
>> +	} else {
>> +		/* OPP table is optional */
>> +		ret = dev_pm_opp_of_add_table(dev);
>> +		if (ret && ret != -ENODEV) {
>> +			dev_pm_opp_put_clkname(ctrl->opp_table);
>> +			ctrl->opp_table = NULL;
>> +		}
>> +	}
>> +
>>   	init_completion(&ctrl->idle_comp);
>>   	init_completion(&ctrl->video_comp);
>>   @@ -1866,4 +1883,13 @@ struct dp_ctrl *dp_ctrl_get(struct device 
>> *dev, struct dp_link *link,
>>     void dp_ctrl_put(struct dp_ctrl *dp_ctrl)
>>   {
>> +	struct dp_ctrl_private *ctrl;
>> +
>> +	ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
>> +
>> +	if (ctrl->opp_table) {
>> +		dev_pm_opp_of_remove_table(ctrl->dev);
>> +		dev_pm_opp_put_clkname(ctrl->opp_table);
>> +		ctrl->opp_table = NULL;
>> +	}
>>   }
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
>> b/drivers/gpu/drm/msm/dp/dp_display.c
>> index e175aa3fd3a9..269f83550b46 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -698,7 +698,7 @@ static int dp_init_sub_modules(struct 
>> dp_display_private *dp)
>>   		goto error;
>>   	}
>>   -	dp->power = dp_power_get(dp->parser);
>> +	dp->power = dp_power_get(dev, dp->parser);
>>   	if (IS_ERR(dp->power)) {
>>   		rc = PTR_ERR(dp->power);
>>   		DRM_ERROR("failed to initialize power, rc = %d\n", rc);
>> diff --git a/drivers/gpu/drm/msm/dp/dp_power.c 
>> b/drivers/gpu/drm/msm/dp/dp_power.c
>> index 17c1fc6a2d44..9c4ea00a5f2a 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_power.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_power.c
>> @@ -8,12 +8,14 @@
>>   #include <linux/clk.h>
>>   #include <linux/clk-provider.h>
>>   #include <linux/regulator/consumer.h>
>> +#include <linux/pm_opp.h>
>>   #include "dp_power.h"
>>   #include "msm_drv.h"
>>     struct dp_power_private {
>>   	struct dp_parser *parser;
>>   	struct platform_device *pdev;
>> +	struct device *dev;
>>   	struct clk *link_clk_src;
>>   	struct clk *pixel_provider;
>>   	struct clk *link_provider;
>> @@ -148,18 +150,51 @@ static int dp_power_clk_deinit(struct 
>> dp_power_private *power)
>>   	return 0;
>>   }
>>   +static int dp_power_clk_set_link_rate(struct dp_power_private 
>> *power,
>> +			struct dss_clk *clk_arry, int num_clk, int enable)
>> +{
>> +	u32 rate;
>> +	int i, rc = 0;
>> +
>> +	for (i = 0; i < num_clk; i++) {
>> +		if (clk_arry[i].clk) {
>> +			if (clk_arry[i].type == DSS_CLK_PCLK) {
>> +				if (enable)
>> +					rate = clk_arry[i].rate;
>> +				else
>> +					rate = 0;
>> +
>> +				rc = dev_pm_opp_set_rate(power->dev, rate);
> 
> I am not sure how this is expected to work when you have multiple link 
> clocks,
> since you can only associate one of them with the OPP table which ends 
> up
> getting scaled when you do a dev_pm_opp_set_rate()
> Do you really have platforms which will have multiple link clocks?
this clk_arry[] contains two entries, dp_link_clk and dp_link_intf_clk.
only dp_link_clk with DSS_CLK_PCLK type, hence only dp_link_clk use 
dev_pm_opp_set_rate()
to set link rate.

> 
>> +				if (rc)
>> +					break;
>> +			}
>> +
>> +		}
>> +	}
>> +	return rc;
>> +}
>> +
>>   static int dp_power_clk_set_rate(struct dp_power_private *power,
>>   		enum dp_pm_type module, bool enable)
>>   {
>>   	int rc = 0;
>>   	struct dss_module_power *mp = &power->parser->mp[module];
>>   -	if (enable) {
>> -		rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
>> +	if (module == DP_CTRL_PM) {
>> +		rc = dp_power_clk_set_link_rate(power, mp->clk_config, mp->num_clk, 
>> enable);
>>   		if (rc) {
>> -			DRM_ERROR("failed to set clks rate.\n");
>> +			DRM_ERROR("failed to set link clks rate\n");
>>   			return rc;
>>   		}
>> +	} else {
>> +
> 
> extra blank line
> 
>> +		if (enable) {
>> +			rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
>> +			if (rc) {
>> +				DRM_ERROR("failed to set clks rate\n");
>> +				return rc;
>> +			}
>> +		}
>>   	}
>>     	rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, enable);
>> @@ -349,7 +384,7 @@ int dp_power_deinit(struct dp_power *dp_power)
>>   	return 0;
>>   }
>>   -struct dp_power *dp_power_get(struct dp_parser *parser)
>> +struct dp_power *dp_power_get(struct device *dev, struct dp_parser 
>> *parser)
>>   {
>>   	struct dp_power_private *power;
>>   	struct dp_power *dp_power;
>> @@ -365,6 +400,7 @@ struct dp_power *dp_power_get(struct dp_parser 
>> *parser)
>>     	power->parser = parser;
>>   	power->pdev = parser->pdev;
>> +	power->dev = dev;
>>     	dp_power = &power->dp_power;
>>   diff --git a/drivers/gpu/drm/msm/dp/dp_power.h 
>> b/drivers/gpu/drm/msm/dp/dp_power.h
>> index 76743d755833..7d0327bbc0d5 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_power.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_power.h
>> @@ -102,6 +102,6 @@ void dp_power_client_deinit(struct dp_power 
>> *power);
>>    * methods to be called by the client to configure the power related
>>    * modueles.
>>    */
>> -struct dp_power *dp_power_get(struct dp_parser *parser);
>> +struct dp_power *dp_power_get(struct device *dev, struct dp_parser 
>> *parser);
>>     #endif /* _DP_POWER_H_ */
>> 
>> base-commit: d1ea914925856d397b0b3241428f20b945e31434
> 
> ??
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/msm/dp: add opp_table corner voting support base on dp_ink_clk rate

[Stephen Boyd]

Quoting Rajendra Nayak (2020-10-06 00:31:41)
> 
> On 10/4/2020 3:56 AM, Kuogee Hsieh wrote:
> > Set link rate by using OPP set rate api so that CX level will be set
> > accordingly based on the link rate.
> > 
> > Changes in v2:
> > -- remove dev from dp_ctrl_put() parameters
> > -- address review comments
> 
> This needs to go below '---' and should not be part of the
> change log.

In drm tree they put this above the triple dash.

> > diff --git a/drivers/gpu/drm/msm/dp/dp_power.h b/drivers/gpu/drm/msm/dp/dp_power.h
> > index 76743d755833..7d0327bbc0d5 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_power.h
> > +++ b/drivers/gpu/drm/msm/dp/dp_power.h
> > @@ -102,6 +102,6 @@ void dp_power_client_deinit(struct dp_power *power);
> >    * methods to be called by the client to configure the power related
> >    * modueles.
> >    */
> > -struct dp_power *dp_power_get(struct dp_parser *parser);
> > +struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser);
> >   
> >   #endif /* _DP_POWER_H_ */
> > 
> > base-commit: d1ea914925856d397b0b3241428f20b945e31434
> 
> ??
> 

This commit is in linux-next as d1ea91492585 ("drm/msm/dp: fix incorrect
function prototype of dp_debug_get()"). Seems fine.

Re: [PATCH v2] drm/msm/dp: add opp_table corner voting support base on dp_ink_clk rate

[Stephen Boyd]

Quoting Rajendra Nayak (2020-10-06 00:31:41)
> 
> On 10/4/2020 3:56 AM, Kuogee Hsieh wrote:
> > Set link rate by using OPP set rate api so that CX level will be set
> > accordingly based on the link rate.
> > 
> > Changes in v2:
> > -- remove dev from dp_ctrl_put() parameters
> > -- address review comments
> 
> This needs to go below '---' and should not be part of the
> change log.

In drm tree they put this above the triple dash.

> > diff --git a/drivers/gpu/drm/msm/dp/dp_power.h b/drivers/gpu/drm/msm/dp/dp_power.h
> > index 76743d755833..7d0327bbc0d5 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_power.h
> > +++ b/drivers/gpu/drm/msm/dp/dp_power.h
> > @@ -102,6 +102,6 @@ void dp_power_client_deinit(struct dp_power *power);
> >    * methods to be called by the client to configure the power related
> >    * modueles.
> >    */
> > -struct dp_power *dp_power_get(struct dp_parser *parser);
> > +struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser);
> >   
> >   #endif /* _DP_POWER_H_ */
> > 
> > base-commit: d1ea914925856d397b0b3241428f20b945e31434
> 
> ??
> 

This commit is in linux-next as d1ea91492585 ("drm/msm/dp: fix incorrect
function prototype of dp_debug_get()"). Seems fine.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/msm/dp: add opp_table corner voting support base on dp_ink_clk rate

[Rajendra Nayak]

On 10/4/2020 3:56 AM, Kuogee Hsieh wrote:
> Set link rate by using OPP set rate api so that CX level will be set
> accordingly based on the link rate.
> 
> Changes in v2:
> -- remove dev from dp_ctrl_put() parameters
> -- address review comments

This needs to go below '---' and should not be part of the
change log.

> 
> Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>
> ---
>   drivers/gpu/drm/msm/dp/dp_ctrl.c    | 26 +++++++++++++++++
>   drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
>   drivers/gpu/drm/msm/dp/dp_power.c   | 44 ++++++++++++++++++++++++++---
>   drivers/gpu/drm/msm/dp/dp_power.h   |  2 +-
>   4 files changed, 68 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 2e3e1917351f..6eb9cdad1421 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -10,6 +10,7 @@
>   #include <linux/delay.h>
>   #include <linux/phy/phy.h>
>   #include <linux/phy/phy-dp.h>
> +#include <linux/pm_opp.h>
>   #include <drm/drm_fixed.h>
>   #include <drm/drm_dp_helper.h>
>   #include <drm/drm_print.h>
> @@ -76,6 +77,8 @@ struct dp_ctrl_private {
>   	struct dp_parser *parser;
>   	struct dp_catalog *catalog;
>   
> +	struct opp_table *opp_table;
> +
>   	struct completion idle_comp;
>   	struct completion video_comp;
>   };
> @@ -1836,6 +1839,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
>   			struct dp_parser *parser)
>   {
>   	struct dp_ctrl_private *ctrl;
> +	int ret;
>   
>   	if (!dev || !panel || !aux ||
>   	    !link || !catalog) {
> @@ -1849,6 +1853,19 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
>   		return ERR_PTR(-ENOMEM);
>   	}
>   
> +	ctrl->opp_table = dev_pm_opp_set_clkname(dev, "ctrl_link");
> +	if (IS_ERR(ctrl->opp_table)) {
> +		dev_err(dev, "invalid DP OPP table in device tree\n");

You do this regardless of an OPP table in DT, so for starters the error
message is wrong. Secondly this can return you a -EPROBE_DEFER if the
clock driver isn't ready yet.
So the ideal thing to do here, is return a PTR_ERR(ctrl->opp_table)

> +		ctrl->opp_table = NULL;
> +	} else {
> +		/* OPP table is optional */
> +		ret = dev_pm_opp_of_add_table(dev);
> +		if (ret && ret != -ENODEV) {
> +			dev_pm_opp_put_clkname(ctrl->opp_table);
> +			ctrl->opp_table = NULL;
> +		}
> +	}
> +
>   	init_completion(&ctrl->idle_comp);
>   	init_completion(&ctrl->video_comp);
>   
> @@ -1866,4 +1883,13 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
>   
>   void dp_ctrl_put(struct dp_ctrl *dp_ctrl)
>   {
> +	struct dp_ctrl_private *ctrl;
> +
> +	ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
> +
> +	if (ctrl->opp_table) {
> +		dev_pm_opp_of_remove_table(ctrl->dev);
> +		dev_pm_opp_put_clkname(ctrl->opp_table);
> +		ctrl->opp_table = NULL;
> +	}
>   }
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index e175aa3fd3a9..269f83550b46 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -698,7 +698,7 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
>   		goto error;
>   	}
>   
> -	dp->power = dp_power_get(dp->parser);
> +	dp->power = dp_power_get(dev, dp->parser);
>   	if (IS_ERR(dp->power)) {
>   		rc = PTR_ERR(dp->power);
>   		DRM_ERROR("failed to initialize power, rc = %d\n", rc);
> diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c
> index 17c1fc6a2d44..9c4ea00a5f2a 100644
> --- a/drivers/gpu/drm/msm/dp/dp_power.c
> +++ b/drivers/gpu/drm/msm/dp/dp_power.c
> @@ -8,12 +8,14 @@
>   #include <linux/clk.h>
>   #include <linux/clk-provider.h>
>   #include <linux/regulator/consumer.h>
> +#include <linux/pm_opp.h>
>   #include "dp_power.h"
>   #include "msm_drv.h"
>   
>   struct dp_power_private {
>   	struct dp_parser *parser;
>   	struct platform_device *pdev;
> +	struct device *dev;
>   	struct clk *link_clk_src;
>   	struct clk *pixel_provider;
>   	struct clk *link_provider;
> @@ -148,18 +150,51 @@ static int dp_power_clk_deinit(struct dp_power_private *power)
>   	return 0;
>   }
>   
> +static int dp_power_clk_set_link_rate(struct dp_power_private *power,
> +			struct dss_clk *clk_arry, int num_clk, int enable)
> +{
> +	u32 rate;
> +	int i, rc = 0;
> +
> +	for (i = 0; i < num_clk; i++) {
> +		if (clk_arry[i].clk) {
> +			if (clk_arry[i].type == DSS_CLK_PCLK) {
> +				if (enable)
> +					rate = clk_arry[i].rate;
> +				else
> +					rate = 0;
> +
> +				rc = dev_pm_opp_set_rate(power->dev, rate);

I am not sure how this is expected to work when you have multiple link clocks,
since you can only associate one of them with the OPP table which ends up
getting scaled when you do a dev_pm_opp_set_rate()
Do you really have platforms which will have multiple link clocks?

> +				if (rc)
> +					break;
> +			}
> +
> +		}
> +	}
> +	return rc;
> +}
> +
>   static int dp_power_clk_set_rate(struct dp_power_private *power,
>   		enum dp_pm_type module, bool enable)
>   {
>   	int rc = 0;
>   	struct dss_module_power *mp = &power->parser->mp[module];
>   
> -	if (enable) {
> -		rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
> +	if (module == DP_CTRL_PM) {
> +		rc = dp_power_clk_set_link_rate(power, mp->clk_config, mp->num_clk, enable);
>   		if (rc) {
> -			DRM_ERROR("failed to set clks rate.\n");
> +			DRM_ERROR("failed to set link clks rate\n");
>   			return rc;
>   		}
> +	} else {
> +

extra blank line

> +		if (enable) {
> +			rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
> +			if (rc) {
> +				DRM_ERROR("failed to set clks rate\n");
> +				return rc;
> +			}
> +		}
>   	}
>   
>   	rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, enable);
> @@ -349,7 +384,7 @@ int dp_power_deinit(struct dp_power *dp_power)
>   	return 0;
>   }
>   
> -struct dp_power *dp_power_get(struct dp_parser *parser)
> +struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser)
>   {
>   	struct dp_power_private *power;
>   	struct dp_power *dp_power;
> @@ -365,6 +400,7 @@ struct dp_power *dp_power_get(struct dp_parser *parser)
>   
>   	power->parser = parser;
>   	power->pdev = parser->pdev;
> +	power->dev = dev;
>   
>   	dp_power = &power->dp_power;
>   
> diff --git a/drivers/gpu/drm/msm/dp/dp_power.h b/drivers/gpu/drm/msm/dp/dp_power.h
> index 76743d755833..7d0327bbc0d5 100644
> --- a/drivers/gpu/drm/msm/dp/dp_power.h
> +++ b/drivers/gpu/drm/msm/dp/dp_power.h
> @@ -102,6 +102,6 @@ void dp_power_client_deinit(struct dp_power *power);
>    * methods to be called by the client to configure the power related
>    * modueles.
>    */
> -struct dp_power *dp_power_get(struct dp_parser *parser);
> +struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser);
>   
>   #endif /* _DP_POWER_H_ */
> 
> base-commit: d1ea914925856d397b0b3241428f20b945e31434

??

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v2] drm/msm/dp: add opp_table corner voting support base on dp_ink_clk rate

[Rajendra Nayak]

On 10/4/2020 3:56 AM, Kuogee Hsieh wrote:
> Set link rate by using OPP set rate api so that CX level will be set
> accordingly based on the link rate.
> 
> Changes in v2:
> -- remove dev from dp_ctrl_put() parameters
> -- address review comments

This needs to go below '---' and should not be part of the
change log.

> 
> Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>
> ---
>   drivers/gpu/drm/msm/dp/dp_ctrl.c    | 26 +++++++++++++++++
>   drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
>   drivers/gpu/drm/msm/dp/dp_power.c   | 44 ++++++++++++++++++++++++++---
>   drivers/gpu/drm/msm/dp/dp_power.h   |  2 +-
>   4 files changed, 68 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 2e3e1917351f..6eb9cdad1421 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -10,6 +10,7 @@
>   #include <linux/delay.h>
>   #include <linux/phy/phy.h>
>   #include <linux/phy/phy-dp.h>
> +#include <linux/pm_opp.h>
>   #include <drm/drm_fixed.h>
>   #include <drm/drm_dp_helper.h>
>   #include <drm/drm_print.h>
> @@ -76,6 +77,8 @@ struct dp_ctrl_private {
>   	struct dp_parser *parser;
>   	struct dp_catalog *catalog;
>   
> +	struct opp_table *opp_table;
> +
>   	struct completion idle_comp;
>   	struct completion video_comp;
>   };
> @@ -1836,6 +1839,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
>   			struct dp_parser *parser)
>   {
>   	struct dp_ctrl_private *ctrl;
> +	int ret;
>   
>   	if (!dev || !panel || !aux ||
>   	    !link || !catalog) {
> @@ -1849,6 +1853,19 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
>   		return ERR_PTR(-ENOMEM);
>   	}
>   
> +	ctrl->opp_table = dev_pm_opp_set_clkname(dev, "ctrl_link");
> +	if (IS_ERR(ctrl->opp_table)) {
> +		dev_err(dev, "invalid DP OPP table in device tree\n");

You do this regardless of an OPP table in DT, so for starters the error
message is wrong. Secondly this can return you a -EPROBE_DEFER if the
clock driver isn't ready yet.
So the ideal thing to do here, is return a PTR_ERR(ctrl->opp_table)

> +		ctrl->opp_table = NULL;
> +	} else {
> +		/* OPP table is optional */
> +		ret = dev_pm_opp_of_add_table(dev);
> +		if (ret && ret != -ENODEV) {
> +			dev_pm_opp_put_clkname(ctrl->opp_table);
> +			ctrl->opp_table = NULL;
> +		}
> +	}
> +
>   	init_completion(&ctrl->idle_comp);
>   	init_completion(&ctrl->video_comp);
>   
> @@ -1866,4 +1883,13 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
>   
>   void dp_ctrl_put(struct dp_ctrl *dp_ctrl)
>   {
> +	struct dp_ctrl_private *ctrl;
> +
> +	ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
> +
> +	if (ctrl->opp_table) {
> +		dev_pm_opp_of_remove_table(ctrl->dev);
> +		dev_pm_opp_put_clkname(ctrl->opp_table);
> +		ctrl->opp_table = NULL;
> +	}
>   }
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index e175aa3fd3a9..269f83550b46 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -698,7 +698,7 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
>   		goto error;
>   	}
>   
> -	dp->power = dp_power_get(dp->parser);
> +	dp->power = dp_power_get(dev, dp->parser);
>   	if (IS_ERR(dp->power)) {
>   		rc = PTR_ERR(dp->power);
>   		DRM_ERROR("failed to initialize power, rc = %d\n", rc);
> diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c
> index 17c1fc6a2d44..9c4ea00a5f2a 100644
> --- a/drivers/gpu/drm/msm/dp/dp_power.c
> +++ b/drivers/gpu/drm/msm/dp/dp_power.c
> @@ -8,12 +8,14 @@
>   #include <linux/clk.h>
>   #include <linux/clk-provider.h>
>   #include <linux/regulator/consumer.h>
> +#include <linux/pm_opp.h>
>   #include "dp_power.h"
>   #include "msm_drv.h"
>   
>   struct dp_power_private {
>   	struct dp_parser *parser;
>   	struct platform_device *pdev;
> +	struct device *dev;
>   	struct clk *link_clk_src;
>   	struct clk *pixel_provider;
>   	struct clk *link_provider;
> @@ -148,18 +150,51 @@ static int dp_power_clk_deinit(struct dp_power_private *power)
>   	return 0;
>   }
>   
> +static int dp_power_clk_set_link_rate(struct dp_power_private *power,
> +			struct dss_clk *clk_arry, int num_clk, int enable)
> +{
> +	u32 rate;
> +	int i, rc = 0;
> +
> +	for (i = 0; i < num_clk; i++) {
> +		if (clk_arry[i].clk) {
> +			if (clk_arry[i].type == DSS_CLK_PCLK) {
> +				if (enable)
> +					rate = clk_arry[i].rate;
> +				else
> +					rate = 0;
> +
> +				rc = dev_pm_opp_set_rate(power->dev, rate);

I am not sure how this is expected to work when you have multiple link clocks,
since you can only associate one of them with the OPP table which ends up
getting scaled when you do a dev_pm_opp_set_rate()
Do you really have platforms which will have multiple link clocks?

> +				if (rc)
> +					break;
> +			}
> +
> +		}
> +	}
> +	return rc;
> +}
> +
>   static int dp_power_clk_set_rate(struct dp_power_private *power,
>   		enum dp_pm_type module, bool enable)
>   {
>   	int rc = 0;
>   	struct dss_module_power *mp = &power->parser->mp[module];
>   
> -	if (enable) {
> -		rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
> +	if (module == DP_CTRL_PM) {
> +		rc = dp_power_clk_set_link_rate(power, mp->clk_config, mp->num_clk, enable);
>   		if (rc) {
> -			DRM_ERROR("failed to set clks rate.\n");
> +			DRM_ERROR("failed to set link clks rate\n");
>   			return rc;
>   		}
> +	} else {
> +

extra blank line

> +		if (enable) {
> +			rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
> +			if (rc) {
> +				DRM_ERROR("failed to set clks rate\n");
> +				return rc;
> +			}
> +		}
>   	}
>   
>   	rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, enable);
> @@ -349,7 +384,7 @@ int dp_power_deinit(struct dp_power *dp_power)
>   	return 0;
>   }
>   
> -struct dp_power *dp_power_get(struct dp_parser *parser)
> +struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser)
>   {
>   	struct dp_power_private *power;
>   	struct dp_power *dp_power;
> @@ -365,6 +400,7 @@ struct dp_power *dp_power_get(struct dp_parser *parser)
>   
>   	power->parser = parser;
>   	power->pdev = parser->pdev;
> +	power->dev = dev;
>   
>   	dp_power = &power->dp_power;
>   
> diff --git a/drivers/gpu/drm/msm/dp/dp_power.h b/drivers/gpu/drm/msm/dp/dp_power.h
> index 76743d755833..7d0327bbc0d5 100644
> --- a/drivers/gpu/drm/msm/dp/dp_power.h
> +++ b/drivers/gpu/drm/msm/dp/dp_power.h
> @@ -102,6 +102,6 @@ void dp_power_client_deinit(struct dp_power *power);
>    * methods to be called by the client to configure the power related
>    * modueles.
>    */
> -struct dp_power *dp_power_get(struct dp_parser *parser);
> +struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser);
>   
>   #endif /* _DP_POWER_H_ */
> 
> base-commit: d1ea914925856d397b0b3241428f20b945e31434

??

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] drm/msm/dp: add opp_table corner voting support base on dp_ink_clk rate

[Kuogee Hsieh]

Set link rate by using OPP set rate api so that CX level will be set
accordingly based on the link rate.

Changes in v2:
-- remove dev from dp_ctrl_put() parameters
-- address review comments

Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c    | 26 +++++++++++++++++
 drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
 drivers/gpu/drm/msm/dp/dp_power.c   | 44 ++++++++++++++++++++++++++---
 drivers/gpu/drm/msm/dp/dp_power.h   |  2 +-
 4 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 2e3e1917351f..6eb9cdad1421 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -10,6 +10,7 @@
 #include <linux/delay.h>
 #include <linux/phy/phy.h>
 #include <linux/phy/phy-dp.h>
+#include <linux/pm_opp.h>
 #include <drm/drm_fixed.h>
 #include <drm/drm_dp_helper.h>
 #include <drm/drm_print.h>
@@ -76,6 +77,8 @@ struct dp_ctrl_private {
 	struct dp_parser *parser;
 	struct dp_catalog *catalog;
 
+	struct opp_table *opp_table;
+
 	struct completion idle_comp;
 	struct completion video_comp;
 };
@@ -1836,6 +1839,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
 			struct dp_parser *parser)
 {
 	struct dp_ctrl_private *ctrl;
+	int ret;
 
 	if (!dev || !panel || !aux ||
 	    !link || !catalog) {
@@ -1849,6 +1853,19 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
 		return ERR_PTR(-ENOMEM);
 	}
 
+	ctrl->opp_table = dev_pm_opp_set_clkname(dev, "ctrl_link");
+	if (IS_ERR(ctrl->opp_table)) {
+		dev_err(dev, "invalid DP OPP table in device tree\n");
+		ctrl->opp_table = NULL;
+	} else {
+		/* OPP table is optional */
+		ret = dev_pm_opp_of_add_table(dev);
+		if (ret && ret != -ENODEV) {
+			dev_pm_opp_put_clkname(ctrl->opp_table);
+			ctrl->opp_table = NULL;
+		}
+	}
+
 	init_completion(&ctrl->idle_comp);
 	init_completion(&ctrl->video_comp);
 
@@ -1866,4 +1883,13 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
 
 void dp_ctrl_put(struct dp_ctrl *dp_ctrl)
 {
+	struct dp_ctrl_private *ctrl;
+
+	ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
+
+	if (ctrl->opp_table) {
+		dev_pm_opp_of_remove_table(ctrl->dev);
+		dev_pm_opp_put_clkname(ctrl->opp_table);
+		ctrl->opp_table = NULL;
+	}
 }
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index e175aa3fd3a9..269f83550b46 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -698,7 +698,7 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
 		goto error;
 	}
 
-	dp->power = dp_power_get(dp->parser);
+	dp->power = dp_power_get(dev, dp->parser);
 	if (IS_ERR(dp->power)) {
 		rc = PTR_ERR(dp->power);
 		DRM_ERROR("failed to initialize power, rc = %d\n", rc);
diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c
index 17c1fc6a2d44..9c4ea00a5f2a 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.c
+++ b/drivers/gpu/drm/msm/dp/dp_power.c
@@ -8,12 +8,14 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/regulator/consumer.h>
+#include <linux/pm_opp.h>
 #include "dp_power.h"
 #include "msm_drv.h"
 
 struct dp_power_private {
 	struct dp_parser *parser;
 	struct platform_device *pdev;
+	struct device *dev;
 	struct clk *link_clk_src;
 	struct clk *pixel_provider;
 	struct clk *link_provider;
@@ -148,18 +150,51 @@ static int dp_power_clk_deinit(struct dp_power_private *power)
 	return 0;
 }
 
+static int dp_power_clk_set_link_rate(struct dp_power_private *power,
+			struct dss_clk *clk_arry, int num_clk, int enable)
+{
+	u32 rate;
+	int i, rc = 0;
+
+	for (i = 0; i < num_clk; i++) {
+		if (clk_arry[i].clk) {
+			if (clk_arry[i].type == DSS_CLK_PCLK) {
+				if (enable)
+					rate = clk_arry[i].rate;
+				else
+					rate = 0;
+
+				rc = dev_pm_opp_set_rate(power->dev, rate);
+				if (rc)
+					break;
+			}
+
+		}
+	}
+	return rc;
+}
+
 static int dp_power_clk_set_rate(struct dp_power_private *power,
 		enum dp_pm_type module, bool enable)
 {
 	int rc = 0;
 	struct dss_module_power *mp = &power->parser->mp[module];
 
-	if (enable) {
-		rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
+	if (module == DP_CTRL_PM) {
+		rc = dp_power_clk_set_link_rate(power, mp->clk_config, mp->num_clk, enable);
 		if (rc) {
-			DRM_ERROR("failed to set clks rate.\n");
+			DRM_ERROR("failed to set link clks rate\n");
 			return rc;
 		}
+	} else {
+
+		if (enable) {
+			rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
+			if (rc) {
+				DRM_ERROR("failed to set clks rate\n");
+				return rc;
+			}
+		}
 	}
 
 	rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, enable);
@@ -349,7 +384,7 @@ int dp_power_deinit(struct dp_power *dp_power)
 	return 0;
 }
 
-struct dp_power *dp_power_get(struct dp_parser *parser)
+struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser)
 {
 	struct dp_power_private *power;
 	struct dp_power *dp_power;
@@ -365,6 +400,7 @@ struct dp_power *dp_power_get(struct dp_parser *parser)
 
 	power->parser = parser;
 	power->pdev = parser->pdev;
+	power->dev = dev;
 
 	dp_power = &power->dp_power;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_power.h b/drivers/gpu/drm/msm/dp/dp_power.h
index 76743d755833..7d0327bbc0d5 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.h
+++ b/drivers/gpu/drm/msm/dp/dp_power.h
@@ -102,6 +102,6 @@ void dp_power_client_deinit(struct dp_power *power);
  * methods to be called by the client to configure the power related
  * modueles.
  */
-struct dp_power *dp_power_get(struct dp_parser *parser);
+struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser);
 
 #endif /* _DP_POWER_H_ */

base-commit: d1ea914925856d397b0b3241428f20b945e31434
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2] drm/msm/dp: add opp_table corner voting support base on dp_ink_clk rate

[Kuogee Hsieh]

Set link rate by using OPP set rate api so that CX level will be set
accordingly based on the link rate.

Changes in v2:
-- remove dev from dp_ctrl_put() parameters
-- address review comments

Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c    | 26 +++++++++++++++++
 drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
 drivers/gpu/drm/msm/dp/dp_power.c   | 44 ++++++++++++++++++++++++++---
 drivers/gpu/drm/msm/dp/dp_power.h   |  2 +-
 4 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 2e3e1917351f..6eb9cdad1421 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -10,6 +10,7 @@
 #include <linux/delay.h>
 #include <linux/phy/phy.h>
 #include <linux/phy/phy-dp.h>
+#include <linux/pm_opp.h>
 #include <drm/drm_fixed.h>
 #include <drm/drm_dp_helper.h>
 #include <drm/drm_print.h>
@@ -76,6 +77,8 @@ struct dp_ctrl_private {
 	struct dp_parser *parser;
 	struct dp_catalog *catalog;
 
+	struct opp_table *opp_table;
+
 	struct completion idle_comp;
 	struct completion video_comp;
 };
@@ -1836,6 +1839,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
 			struct dp_parser *parser)
 {
 	struct dp_ctrl_private *ctrl;
+	int ret;
 
 	if (!dev || !panel || !aux ||
 	    !link || !catalog) {
@@ -1849,6 +1853,19 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
 		return ERR_PTR(-ENOMEM);
 	}
 
+	ctrl->opp_table = dev_pm_opp_set_clkname(dev, "ctrl_link");
+	if (IS_ERR(ctrl->opp_table)) {
+		dev_err(dev, "invalid DP OPP table in device tree\n");
+		ctrl->opp_table = NULL;
+	} else {
+		/* OPP table is optional */
+		ret = dev_pm_opp_of_add_table(dev);
+		if (ret && ret != -ENODEV) {
+			dev_pm_opp_put_clkname(ctrl->opp_table);
+			ctrl->opp_table = NULL;
+		}
+	}
+
 	init_completion(&ctrl->idle_comp);
 	init_completion(&ctrl->video_comp);
 
@@ -1866,4 +1883,13 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link,
 
 void dp_ctrl_put(struct dp_ctrl *dp_ctrl)
 {
+	struct dp_ctrl_private *ctrl;
+
+	ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl);
+
+	if (ctrl->opp_table) {
+		dev_pm_opp_of_remove_table(ctrl->dev);
+		dev_pm_opp_put_clkname(ctrl->opp_table);
+		ctrl->opp_table = NULL;
+	}
 }
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index e175aa3fd3a9..269f83550b46 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -698,7 +698,7 @@ static int dp_init_sub_modules(struct dp_display_private *dp)
 		goto error;
 	}
 
-	dp->power = dp_power_get(dp->parser);
+	dp->power = dp_power_get(dev, dp->parser);
 	if (IS_ERR(dp->power)) {
 		rc = PTR_ERR(dp->power);
 		DRM_ERROR("failed to initialize power, rc = %d\n", rc);
diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c
index 17c1fc6a2d44..9c4ea00a5f2a 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.c
+++ b/drivers/gpu/drm/msm/dp/dp_power.c
@@ -8,12 +8,14 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/regulator/consumer.h>
+#include <linux/pm_opp.h>
 #include "dp_power.h"
 #include "msm_drv.h"
 
 struct dp_power_private {
 	struct dp_parser *parser;
 	struct platform_device *pdev;
+	struct device *dev;
 	struct clk *link_clk_src;
 	struct clk *pixel_provider;
 	struct clk *link_provider;
@@ -148,18 +150,51 @@ static int dp_power_clk_deinit(struct dp_power_private *power)
 	return 0;
 }
 
+static int dp_power_clk_set_link_rate(struct dp_power_private *power,
+			struct dss_clk *clk_arry, int num_clk, int enable)
+{
+	u32 rate;
+	int i, rc = 0;
+
+	for (i = 0; i < num_clk; i++) {
+		if (clk_arry[i].clk) {
+			if (clk_arry[i].type == DSS_CLK_PCLK) {
+				if (enable)
+					rate = clk_arry[i].rate;
+				else
+					rate = 0;
+
+				rc = dev_pm_opp_set_rate(power->dev, rate);
+				if (rc)
+					break;
+			}
+
+		}
+	}
+	return rc;
+}
+
 static int dp_power_clk_set_rate(struct dp_power_private *power,
 		enum dp_pm_type module, bool enable)
 {
 	int rc = 0;
 	struct dss_module_power *mp = &power->parser->mp[module];
 
-	if (enable) {
-		rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
+	if (module == DP_CTRL_PM) {
+		rc = dp_power_clk_set_link_rate(power, mp->clk_config, mp->num_clk, enable);
 		if (rc) {
-			DRM_ERROR("failed to set clks rate.\n");
+			DRM_ERROR("failed to set link clks rate\n");
 			return rc;
 		}
+	} else {
+
+		if (enable) {
+			rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk);
+			if (rc) {
+				DRM_ERROR("failed to set clks rate\n");
+				return rc;
+			}
+		}
 	}
 
 	rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, enable);
@@ -349,7 +384,7 @@ int dp_power_deinit(struct dp_power *dp_power)
 	return 0;
 }
 
-struct dp_power *dp_power_get(struct dp_parser *parser)
+struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser)
 {
 	struct dp_power_private *power;
 	struct dp_power *dp_power;
@@ -365,6 +400,7 @@ struct dp_power *dp_power_get(struct dp_parser *parser)
 
 	power->parser = parser;
 	power->pdev = parser->pdev;
+	power->dev = dev;
 
 	dp_power = &power->dp_power;
 
diff --git a/drivers/gpu/drm/msm/dp/dp_power.h b/drivers/gpu/drm/msm/dp/dp_power.h
index 76743d755833..7d0327bbc0d5 100644
--- a/drivers/gpu/drm/msm/dp/dp_power.h
+++ b/drivers/gpu/drm/msm/dp/dp_power.h
@@ -102,6 +102,6 @@ void dp_power_client_deinit(struct dp_power *power);
  * methods to be called by the client to configure the power related
  * modueles.
  */
-struct dp_power *dp_power_get(struct dp_parser *parser);
+struct dp_power *dp_power_get(struct device *dev, struct dp_parser *parser);
 
 #endif /* _DP_POWER_H_ */

base-commit: d1ea914925856d397b0b3241428f20b945e31434
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel