Quoting Dmitry Baryshkov (2022-01-31 13:05:13)
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
> index 094b39bfed8c..f16072f33cdb 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -10,7 +10,6 @@
> #include <linux/phy/phy.h>
> #include <linux/phy/phy-dp.h>
>
> -#include "dp_clk_util.h"
> #include "msm_drv.h"
>
> #define DP_LABEL "MDSS DP DISPLAY"
> @@ -106,6 +105,22 @@ struct dp_regulator_cfg {
> struct dp_reg_entry regs[DP_DEV_REGULATOR_MAX];
> };
>
> +enum dss_clk_type {
> + DSS_CLK_AHB, /* no set rate. rate controlled through rpm */
> + DSS_CLK_PCLK,
> +};
> +
> +struct dss_clk {
> + enum dss_clk_type type;
> + unsigned long rate;
> +};
> +
> +struct dss_module_power {
> + unsigned int num_clk;
> + struct clk_bulk_data *clocks;
> + struct dss_clk *clk_config;
> +};
> +
> /**
> * struct dp_parser - DP parser's data exposed to clients
> *
> diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c
> index b48b45e92bfa..318e1f8629cb 100644
> --- a/drivers/gpu/drm/msm/dp/dp_power.c
> +++ b/drivers/gpu/drm/msm/dp/dp_power.c
> @@ -105,59 +105,40 @@ static int dp_power_clk_init(struct dp_power_private *power)
> ctrl = &power->parser->mp[DP_CTRL_PM];
> stream = &power->parser->mp[DP_STREAM_PM];
>
> - rc = msm_dss_get_clk(dev, core->clk_config, core->num_clk);
> + rc = devm_clk_bulk_get(dev, core->num_clk, core->clocks);
Could we go further and devm_clk_bulk_get_all() and then either have
some new clk API that goes through the bulk list and finds the one named
something and hands over a pointer to it, think "clk_get() on top of
clk_bulk_data", or just get the clk again that you want to set a rate on
and have two pointers but otherwise it's a don't care. Then we wouldn't
need to do much at all here to store the rate settable clk and find it
in a loop.
> if (rc) {
> DRM_ERROR("failed to get %s clk. err=%d\n",
> dp_parser_pm_name(DP_CORE_PM), rc);
> return rc;
> }
>
> - rc = msm_dss_get_clk(dev, ctrl->clk_config, ctrl->num_clk);
> + rc = devm_clk_bulk_get(dev, ctrl->num_clk, ctrl->clocks);
> if (rc) {
> DRM_ERROR("failed to get %s clk. err=%d\n",
> dp_parser_pm_name(DP_CTRL_PM), rc);
> - msm_dss_put_clk(core->clk_config, core->num_clk);
> return -ENODEV;
> }
>
> - rc = msm_dss_get_clk(dev, stream->clk_config, stream->num_clk);
> + rc = devm_clk_bulk_get(dev, stream->num_clk, stream->clocks);
> if (rc) {
> DRM_ERROR("failed to get %s clk. err=%d\n",
> dp_parser_pm_name(DP_CTRL_PM), rc);
> - msm_dss_put_clk(core->clk_config, core->num_clk);
> return -ENODEV;
> }
>
> return 0;
> }
>
> -static int dp_power_clk_deinit(struct dp_power_private *power)
> -{
> - struct dss_module_power *core, *ctrl, *stream;
> -
> - core = &power->parser->mp[DP_CORE_PM];
> - ctrl = &power->parser->mp[DP_CTRL_PM];
> - stream = &power->parser->mp[DP_STREAM_PM];
> -
> - if (!core || !ctrl || !stream) {
> - DRM_ERROR("invalid power_data\n");
> - return -EINVAL;
> - }
> -
> - msm_dss_put_clk(ctrl->clk_config, ctrl->num_clk);
> - msm_dss_put_clk(core->clk_config, core->num_clk);
> - msm_dss_put_clk(stream->clk_config, stream->num_clk);
> - 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)
> + struct dss_module_power *mp, int enable)
> {
> + struct dss_clk *clk_arry = mp->clk_config;
> + int num_clk = mp->num_clk;
> u32 rate;
> int i, rc = 0;
>
> for (i = 0; i < num_clk; i++) {
> - if (clk_arry[i].clk) {
> + if (mp->clocks[i].clk) {
> if (clk_arry[i].type == DSS_CLK_PCLK) {
> if (enable)
> rate = clk_arry[i].rate;
> @@ -168,9 +149,49 @@ static int dp_power_clk_set_link_rate(struct dp_power_private *power,
> if (rc)
> break;
> }
> + } else {
> + DRM_ERROR("%pS->%s: '%s' is not available\n",
> + __builtin_return_address(0), __func__,
> + mp->clocks[i].id);
> + rc = -EPERM;
> + break;
> + }
> + }
> + return rc;
> +}
> +
> +static int dp_clk_set_rate(struct dss_module_power *mp)
> +{
> + struct dss_clk *clk_arry = mp->clk_config;
> + int num_clk = mp->num_clk;
> + int i, rc = 0;
>
> + for (i = 0; i < num_clk; i++) {
> + if (mp->clocks[i].clk) {
> + if (clk_arry[i].type != DSS_CLK_AHB) {
This loops is gross.
> + DRM_DEBUG("%pS->%s: '%s' rate %ld\n",
> + __builtin_return_address(0), __func__,
> + mp->clocks[i].id,
> + clk_arry[i].rate);
> + rc = clk_set_rate(mp->clocks[i].clk,
> + clk_arry[i].rate);
> + if (rc) {
> + DRM_ERROR("%pS->%s: %s failed. rc=%d\n",
> + __builtin_return_address(0),
> + __func__,
> + mp->clocks[i].id, rc);
> + break;
> + }
> + }
> + } else {
> + DRM_ERROR("%pS->%s: '%s' is not available\n",
> + __builtin_return_address(0), __func__,
> + mp->clocks[i].id);
> + rc = -EPERM;
> + break;
> }
> }
> +
> return rc;
> }
>
