Re: [PATCH v5 3/5] drm/msm/dp: set stream_pixel rate directly

[Stephen Boyd <swboyd@chromium.org>]

Quoting Dmitry Baryshkov (2022-02-16 21:55:27)
> The only clock for which we set the rate is the "stream_pixel". Rather
> than storing the rate and then setting it by looping over all the
> clocks, set the clock rate directly.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
[...]
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 07f6bf7e1acb..8e6361dedd77 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -1315,7 +1315,7 @@ static void dp_ctrl_set_clock_rate(struct dp_ctrl_private *ctrl,
>         DRM_DEBUG_DP("setting rate=%lu on clk=%s\n", rate, name);
>
>         if (num)
> -               cfg->rate = rate;
> +               clk_set_rate(cfg->clk, rate);

This looks bad. From what I can tell we set the rate of the pixel clk
after enabling the phy and configuring it. See the order of operations
in dp_ctrl_enable_mainlink_clocks() and note how dp_power_clk_enable()
is the one that eventually sets a rate through dp_power_clk_set_rate()

        dp_ctrl_set_clock_rate(ctrl, DP_CTRL_PM, "ctrl_link",
                                        ctrl->link->link_params.rate * 1000);

        phy_configure(phy, &dp_io->phy_opts);
        phy_power_on(phy);

        ret = dp_power_clk_enable(ctrl->power, DP_CTRL_PM, true);	

and I vaguely recall that the DP phy needs to be configured for some
frequency so that the pixel clk can use it when determining the rate to
set.

>         else
>                 DRM_ERROR("%s clock doesn't exit to set rate %lu\n",
>                                 name, rate);