From: abhinavk@codeaurora.org
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>,
Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>,
Jonathan Marek <jonathan@marek.ca>,
Stephen Boyd <sboyd@kernel.org>, David Airlie <airlied@linux.ie>,
Daniel Vetter <daniel@ffwll.ch>,
linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
freedreno@lists.freedesktop.org
Subject: Re: [Freedreno] [PATCH 2/2] drm/msm/dsi: stop setting clock parents manually
Date: Tue, 19 Oct 2021 16:35:49 -0700 [thread overview]
Message-ID: <f7e8176759809f41dff1c41d6adb9d4a@codeaurora.org> (raw)
In-Reply-To: <20211006204828.1218225-2-dmitry.baryshkov@linaro.org>
On 2021-10-06 13:48, Dmitry Baryshkov wrote:
> There is no reason to set clock parents manually, use device tree to
> assign DSI/display clock parents to DSI PHY clocks. Dropping this
> manual
> setup allows us to drop repeating code and to move registration of hw
> clock providers to generic place.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
I believe this was reviewed previously on
https://patchwork.freedesktop.org/patch/443470/
Hence,
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
> ---
> drivers/gpu/drm/msm/dsi/dsi.h | 2 -
> drivers/gpu/drm/msm/dsi/dsi_host.c | 53 ---------------------------
> drivers/gpu/drm/msm/dsi/dsi_manager.c | 11 +-----
> drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 11 ------
> 4 files changed, 2 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.h
> b/drivers/gpu/drm/msm/dsi/dsi.h
> index 7dfb6d198ca9..c03a8d09c764 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.h
> +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> @@ -173,8 +173,6 @@ int msm_dsi_phy_enable(struct msm_dsi_phy *phy,
> void msm_dsi_phy_disable(struct msm_dsi_phy *phy);
> void msm_dsi_phy_set_usecase(struct msm_dsi_phy *phy,
> enum msm_dsi_phy_usecase uc);
> -int msm_dsi_phy_get_clk_provider(struct msm_dsi_phy *phy,
> - struct clk **byte_clk_provider, struct clk **pixel_clk_provider);
> void msm_dsi_phy_pll_save_state(struct msm_dsi_phy *phy);
> int msm_dsi_phy_pll_restore_state(struct msm_dsi_phy *phy);
> void msm_dsi_phy_snapshot(struct msm_disp_state *disp_state, struct
> msm_dsi_phy *phy);
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 1ffcd0577e99..9600b4fa27eb 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -2232,59 +2232,6 @@ void msm_dsi_host_set_phy_mode(struct
> mipi_dsi_host *host,
> msm_host->cphy_mode = src_phy->cphy_mode;
> }
>
> -int msm_dsi_host_set_src_pll(struct mipi_dsi_host *host,
> - struct msm_dsi_phy *src_phy)
> -{
> - struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
> - struct clk *byte_clk_provider, *pixel_clk_provider;
> - int ret;
> -
> - msm_host->cphy_mode = src_phy->cphy_mode;
> -
> - ret = msm_dsi_phy_get_clk_provider(src_phy,
> - &byte_clk_provider, &pixel_clk_provider);
> - if (ret) {
> - pr_info("%s: can't get provider from pll, don't set parent\n",
> - __func__);
> - return 0;
> - }
> -
> - ret = clk_set_parent(msm_host->byte_clk_src, byte_clk_provider);
> - if (ret) {
> - pr_err("%s: can't set parent to byte_clk_src. ret=%d\n",
> - __func__, ret);
> - goto exit;
> - }
> -
> - ret = clk_set_parent(msm_host->pixel_clk_src, pixel_clk_provider);
> - if (ret) {
> - pr_err("%s: can't set parent to pixel_clk_src. ret=%d\n",
> - __func__, ret);
> - goto exit;
> - }
> -
> - if (msm_host->dsi_clk_src) {
> - ret = clk_set_parent(msm_host->dsi_clk_src, pixel_clk_provider);
> - if (ret) {
> - pr_err("%s: can't set parent to dsi_clk_src. ret=%d\n",
> - __func__, ret);
> - goto exit;
> - }
> - }
> -
> - if (msm_host->esc_clk_src) {
> - ret = clk_set_parent(msm_host->esc_clk_src, byte_clk_provider);
> - if (ret) {
> - pr_err("%s: can't set parent to esc_clk_src. ret=%d\n",
> - __func__, ret);
> - goto exit;
> - }
> - }
> -
> -exit:
> - return ret;
> -}
> -
> void msm_dsi_host_reset_phy(struct mipi_dsi_host *host)
> {
> struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index 49a0a0841487..9342a822ad20 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -78,10 +78,7 @@ static int dsi_mgr_setup_components(int id)
>
> msm_dsi_phy_set_usecase(msm_dsi->phy, MSM_DSI_PHY_STANDALONE);
> msm_dsi_host_set_phy_mode(msm_dsi->host, msm_dsi->phy);
> - ret = msm_dsi_host_set_src_pll(msm_dsi->host, msm_dsi->phy);
> - } else if (!other_dsi) {
> - ret = 0;
> - } else {
> + } else if (other_dsi) {
> struct msm_dsi *master_link_dsi = IS_MASTER_DSI_LINK(id) ?
> msm_dsi : other_dsi;
> struct msm_dsi *slave_link_dsi = IS_MASTER_DSI_LINK(id) ?
> @@ -107,13 +104,9 @@ static int dsi_mgr_setup_components(int id)
> MSM_DSI_PHY_SLAVE);
> msm_dsi_host_set_phy_mode(msm_dsi->host, msm_dsi->phy);
> msm_dsi_host_set_phy_mode(other_dsi->host, other_dsi->phy);
> - ret = msm_dsi_host_set_src_pll(msm_dsi->host, clk_master_dsi->phy);
> - if (ret)
> - return ret;
> - ret = msm_dsi_host_set_src_pll(other_dsi->host,
> clk_master_dsi->phy);
> }
>
> - return ret;
> + return 0;
> }
>
> static int enable_phy(struct msm_dsi *msm_dsi,
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> index 8c65ef6968ca..8ec331e751a2 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> @@ -890,17 +890,6 @@ bool msm_dsi_phy_set_continuous_clock(struct
> msm_dsi_phy *phy, bool enable)
> return phy->cfg->ops.set_continuous_clock(phy, enable);
> }
>
> -int msm_dsi_phy_get_clk_provider(struct msm_dsi_phy *phy,
> - struct clk **byte_clk_provider, struct clk **pixel_clk_provider)
> -{
> - if (byte_clk_provider)
> - *byte_clk_provider =
> phy->provided_clocks->hws[DSI_BYTE_PLL_CLK]->clk;
> - if (pixel_clk_provider)
> - *pixel_clk_provider =
> phy->provided_clocks->hws[DSI_PIXEL_PLL_CLK]->clk;
> -
> - return 0;
> -}
> -
> void msm_dsi_phy_pll_save_state(struct msm_dsi_phy *phy)
> {
> if (phy->cfg->ops.save_pll_state) {
next prev parent reply other threads:[~2021-10-19 23:36 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-06 20:48 [PATCH 1/2] drm/msm/dsi: untangle cphy setting from the src pll setting Dmitry Baryshkov
2021-10-06 20:48 ` [PATCH 2/2] drm/msm/dsi: stop setting clock parents manually Dmitry Baryshkov
2021-10-19 23:35 ` abhinavk [this message]
2021-10-19 23:34 ` [Freedreno] [PATCH 1/2] drm/msm/dsi: untangle cphy setting from the src pll setting abhinavk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f7e8176759809f41dff1c41d6adb9d4a@codeaurora.org \
--to=abhinavk@codeaurora.org \
--cc=airlied@linux.ie \
--cc=bjorn.andersson@linaro.org \
--cc=daniel@ffwll.ch \
--cc=dmitry.baryshkov@linaro.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=jonathan@marek.ca \
--cc=linux-arm-msm@vger.kernel.org \
--cc=robdclark@gmail.com \
--cc=sboyd@kernel.org \
--cc=sean@poorly.run \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).