From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Marijn Suijten <marijn.suijten@somainline.org>,
phone-devel@vger.kernel.org
Cc: ~postmarketos/upstreaming@lists.sr.ht,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@somainline.org>,
Konrad Dybcio <konrad.dybcio@somainline.org>,
Martin Botka <martin.botka@somainline.org>,
Jami Kettunen <jami.kettunen@somainline.org>,
Andy Gross <agross@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>, Rob Clark <robdclark@gmail.com>,
Sean Paul <sean@poorly.run>, David Airlie <airlied@linux.ie>,
Daniel Vetter <daniel@ffwll.ch>,
Abhinav Kumar <abhinavk@codeaurora.org>,
Jonathan Marek <jonathan@marek.ca>,
Matthias Kaehlcke <mka@chromium.org>,
Douglas Anderson <dianders@chromium.org>,
linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
freedreno@lists.freedesktop.org,
Stephen Boyd <swboyd@chromium.org>
Subject: Re: [PATCH v3 1/2] drm/msm/dsi: Use "ref" fw clock instead of global name for VCO parent
Date: Wed, 15 Dec 2021 22:57:48 +0300 [thread overview]
Message-ID: <e1e7f91c-fcdb-7789-0094-80e2b5b09d4f@linaro.org> (raw)
In-Reply-To: <20210911131922.387964-2-marijn.suijten@somainline.org>
On 11/09/2021 16:19, Marijn Suijten wrote:
> All DSI PHY/PLL drivers were referencing their VCO parent clock by a
> global name, most of which don't exist or have been renamed. These
> clock drivers seem to function fine without that except the 14nm driver
> for sdm6xx [1].
>
> At the same time all DTs provide a "ref" clock as per the requirements
> of dsi-phy-common.yaml, but the clock is never used. This patchset puts
> that clock to use without relying on a global clock name, so that all
> dependencies are explicitly defined in DT (the firmware) in the end.
>
> Note that this patch intentionally breaks older firmware (DT) that
> relies on the clock to be found globally instead. The only affected
> platform is msm8974 [2] for whose dsi_phy_28nm a .name="xo" fallback is
> left in place to accommodate a more graceful transition period. All
> other platforms had the "ref" clock added to their phy node since its
> inception, or in a followup patch some time after. These patches
> wrongly assumed that the "ref" clock was actively used and have hence
> been listed as "Fixes:" below.
> Furthermore apq8064 was providing the wrong 19.2MHz cxo instead of
> 27MHz pxo clock, which has been addressed in [3].
>
> It is expected that both [2] and [3] are applied to the tree well in
> advance of this patch such that any actual breakage is extremely
> unlikely, but might still occur if kernel upgrades are performed without
> the DT to match. After some time the fallback for msm8974 can be
> removed again as well.
>
> [1]: https://lore.kernel.org/linux-arm-msm/386db1a6-a1cd-3c7d-a88e-dc83f8a1be96@somainline.org/
> [2]: https://lore.kernel.org/linux-arm-msm/20210830175739.143401-1-marijn.suijten@somainline.org/
> [3]: https://lore.kernel.org/linux-arm-msm/20210829203027.276143-2-marijn.suijten@somainline.org/
>
> Fixes: 79e51645a1dd ("arm64: dts: qcom: msm8916: Set 'xo_board' as ref clock of the DSI PHY")
> Fixes: 6969d1d9c615 ("ARM: dts: qcom-apq8064: Set 'cxo_board' as ref clock of the DSI PHY")
> Fixes: 0c0e72705a33 ("arm64: dts: sdm845: Set 'bi_tcxo' as ref clock of the DSI PHYs")
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 4 +++-
> drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 4 +++-
> drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 4 +++-
> drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c | 4 +++-
> drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 4 +++-
> 5 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
> index e46b10fc793a..3cbb1f1475e8 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
> @@ -562,7 +562,9 @@ static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm, struct clk_hw **prov
> char clk_name[32], parent[32], vco_name[32];
> char parent2[32], parent3[32], parent4[32];
> struct clk_init_data vco_init = {
> - .parent_names = (const char *[]){ "xo" },
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "ref",
> + },
> .num_parents = 1,
> .name = vco_name,
> .flags = CLK_IGNORE_UNUSED,
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> index ebedbb6c8961..789b08c24d25 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> @@ -802,7 +802,9 @@ static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **prov
> {
> char clk_name[32], parent[32], vco_name[32];
> struct clk_init_data vco_init = {
> - .parent_names = (const char *[]){ "xo" },
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "ref",
> + },
> .num_parents = 1,
> .name = vco_name,
> .flags = CLK_IGNORE_UNUSED,
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
> index eb1b8ff61da1..531c4b65aede 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
> @@ -521,7 +521,9 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
> {
> char clk_name[32], parent1[32], parent2[32], vco_name[32];
> struct clk_init_data vco_init = {
> - .parent_names = (const char *[]){ "xo" },
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "ref", .name = "xo",
> + },
> .num_parents = 1,
> .name = vco_name,
> .flags = CLK_IGNORE_UNUSED,
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> index aaa37456f4ee..9662cb236468 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> @@ -385,7 +385,9 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
> {
> char *clk_name, *parent_name, *vco_name;
> struct clk_init_data vco_init = {
> - .parent_names = (const char *[]){ "pxo" },
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "ref",
> + },
> .num_parents = 1,
> .flags = CLK_IGNORE_UNUSED,
> .ops = &clk_ops_dsi_pll_28nm_vco,
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
> index 9eade6d81a54..1a5abbd9fb76 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
> @@ -588,7 +588,9 @@ static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provide
> char clk_name[32], parent[32], vco_name[32];
> char parent2[32], parent3[32], parent4[32];
> struct clk_init_data vco_init = {
> - .parent_names = (const char *[]){ "bi_tcxo" },
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "ref",
> + },
> .num_parents = 1,
> .name = vco_name,
> .flags = CLK_IGNORE_UNUSED,
> --
> 2.33.0
>
--
With best wishes
Dmitry
WARNING: multiple messages have this Message-ID (diff)
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Marijn Suijten <marijn.suijten@somainline.org>,
phone-devel@vger.kernel.org
Cc: David Airlie <airlied@linux.ie>,
Michael Turquette <mturquette@baylibre.com>,
Konrad Dybcio <konrad.dybcio@somainline.org>,
dri-devel@lists.freedesktop.org,
Bjorn Andersson <bjorn.andersson@linaro.org>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@somainline.org>,
linux-clk@vger.kernel.org, Jonathan Marek <jonathan@marek.ca>,
Andy Gross <agross@kernel.org>,
linux-arm-msm@vger.kernel.org,
Abhinav Kumar <abhinavk@codeaurora.org>,
Stephen Boyd <swboyd@chromium.org>,
Martin Botka <martin.botka@somainline.org>,
~postmarketos/upstreaming@lists.sr.ht,
Sean Paul <sean@poorly.run>,
Jami Kettunen <jami.kettunen@somainline.org>,
Matthias Kaehlcke <mka@chromium.org>,
Douglas Anderson <dianders@chromium.org>,
linux-kernel@vger.kernel.org, Stephen Boyd <sboyd@kernel.org>,
freedreno@lists.freedesktop.org
Subject: Re: [PATCH v3 1/2] drm/msm/dsi: Use "ref" fw clock instead of global name for VCO parent
Date: Wed, 15 Dec 2021 22:57:48 +0300 [thread overview]
Message-ID: <e1e7f91c-fcdb-7789-0094-80e2b5b09d4f@linaro.org> (raw)
In-Reply-To: <20210911131922.387964-2-marijn.suijten@somainline.org>
On 11/09/2021 16:19, Marijn Suijten wrote:
> All DSI PHY/PLL drivers were referencing their VCO parent clock by a
> global name, most of which don't exist or have been renamed. These
> clock drivers seem to function fine without that except the 14nm driver
> for sdm6xx [1].
>
> At the same time all DTs provide a "ref" clock as per the requirements
> of dsi-phy-common.yaml, but the clock is never used. This patchset puts
> that clock to use without relying on a global clock name, so that all
> dependencies are explicitly defined in DT (the firmware) in the end.
>
> Note that this patch intentionally breaks older firmware (DT) that
> relies on the clock to be found globally instead. The only affected
> platform is msm8974 [2] for whose dsi_phy_28nm a .name="xo" fallback is
> left in place to accommodate a more graceful transition period. All
> other platforms had the "ref" clock added to their phy node since its
> inception, or in a followup patch some time after. These patches
> wrongly assumed that the "ref" clock was actively used and have hence
> been listed as "Fixes:" below.
> Furthermore apq8064 was providing the wrong 19.2MHz cxo instead of
> 27MHz pxo clock, which has been addressed in [3].
>
> It is expected that both [2] and [3] are applied to the tree well in
> advance of this patch such that any actual breakage is extremely
> unlikely, but might still occur if kernel upgrades are performed without
> the DT to match. After some time the fallback for msm8974 can be
> removed again as well.
>
> [1]: https://lore.kernel.org/linux-arm-msm/386db1a6-a1cd-3c7d-a88e-dc83f8a1be96@somainline.org/
> [2]: https://lore.kernel.org/linux-arm-msm/20210830175739.143401-1-marijn.suijten@somainline.org/
> [3]: https://lore.kernel.org/linux-arm-msm/20210829203027.276143-2-marijn.suijten@somainline.org/
>
> Fixes: 79e51645a1dd ("arm64: dts: qcom: msm8916: Set 'xo_board' as ref clock of the DSI PHY")
> Fixes: 6969d1d9c615 ("ARM: dts: qcom-apq8064: Set 'cxo_board' as ref clock of the DSI PHY")
> Fixes: 0c0e72705a33 ("arm64: dts: sdm845: Set 'bi_tcxo' as ref clock of the DSI PHYs")
> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 4 +++-
> drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 4 +++-
> drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 4 +++-
> drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c | 4 +++-
> drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 4 +++-
> 5 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
> index e46b10fc793a..3cbb1f1475e8 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
> @@ -562,7 +562,9 @@ static int pll_10nm_register(struct dsi_pll_10nm *pll_10nm, struct clk_hw **prov
> char clk_name[32], parent[32], vco_name[32];
> char parent2[32], parent3[32], parent4[32];
> struct clk_init_data vco_init = {
> - .parent_names = (const char *[]){ "xo" },
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "ref",
> + },
> .num_parents = 1,
> .name = vco_name,
> .flags = CLK_IGNORE_UNUSED,
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> index ebedbb6c8961..789b08c24d25 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> @@ -802,7 +802,9 @@ static int pll_14nm_register(struct dsi_pll_14nm *pll_14nm, struct clk_hw **prov
> {
> char clk_name[32], parent[32], vco_name[32];
> struct clk_init_data vco_init = {
> - .parent_names = (const char *[]){ "xo" },
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "ref",
> + },
> .num_parents = 1,
> .name = vco_name,
> .flags = CLK_IGNORE_UNUSED,
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
> index eb1b8ff61da1..531c4b65aede 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
> @@ -521,7 +521,9 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
> {
> char clk_name[32], parent1[32], parent2[32], vco_name[32];
> struct clk_init_data vco_init = {
> - .parent_names = (const char *[]){ "xo" },
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "ref", .name = "xo",
> + },
> .num_parents = 1,
> .name = vco_name,
> .flags = CLK_IGNORE_UNUSED,
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> index aaa37456f4ee..9662cb236468 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
> @@ -385,7 +385,9 @@ static int pll_28nm_register(struct dsi_pll_28nm *pll_28nm, struct clk_hw **prov
> {
> char *clk_name, *parent_name, *vco_name;
> struct clk_init_data vco_init = {
> - .parent_names = (const char *[]){ "pxo" },
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "ref",
> + },
> .num_parents = 1,
> .flags = CLK_IGNORE_UNUSED,
> .ops = &clk_ops_dsi_pll_28nm_vco,
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
> index 9eade6d81a54..1a5abbd9fb76 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
> @@ -588,7 +588,9 @@ static int pll_7nm_register(struct dsi_pll_7nm *pll_7nm, struct clk_hw **provide
> char clk_name[32], parent[32], vco_name[32];
> char parent2[32], parent3[32], parent4[32];
> struct clk_init_data vco_init = {
> - .parent_names = (const char *[]){ "bi_tcxo" },
> + .parent_data = &(const struct clk_parent_data) {
> + .fw_name = "ref",
> + },
> .num_parents = 1,
> .name = vco_name,
> .flags = CLK_IGNORE_UNUSED,
> --
> 2.33.0
>
--
With best wishes
Dmitry
next prev parent reply other threads:[~2021-12-15 19:57 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-11 13:19 [PATCH v3 0/2] Use "ref" clocks from firmware for DSI PLL VCO parent Marijn Suijten
2021-09-11 13:19 ` Marijn Suijten
2021-09-11 13:19 ` [PATCH v3 1/2] drm/msm/dsi: Use "ref" fw clock instead of global name for " Marijn Suijten
2021-09-11 13:19 ` Marijn Suijten
2021-12-15 19:57 ` Dmitry Baryshkov [this message]
2021-12-15 19:57 ` Dmitry Baryshkov
2021-09-11 13:19 ` [PATCH v3 2/2] clk: qcom: gcc-sdm660: Remove transient global "xo" clock Marijn Suijten
2021-09-11 13:19 ` Marijn Suijten
2021-09-14 21:44 ` [PATCH v3 0/2] Use "ref" clocks from firmware for DSI PLL VCO parent Stephen Boyd
2021-09-14 21:44 ` Stephen Boyd
2021-09-18 14:40 ` Marijn Suijten
2021-09-18 14:40 ` Marijn Suijten
2021-12-14 19:46 ` Marijn Suijten
2021-12-14 19:46 ` Marijn Suijten
2021-12-15 20:02 ` Dmitry Baryshkov
2021-12-16 0:43 ` Stephen Boyd
2021-12-21 16:22 ` Marijn Suijten
2021-12-21 16:22 ` Marijn Suijten
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=e1e7f91c-fcdb-7789-0094-80e2b5b09d4f@linaro.org \
--to=dmitry.baryshkov@linaro.org \
--cc=abhinavk@codeaurora.org \
--cc=agross@kernel.org \
--cc=airlied@linux.ie \
--cc=angelogioacchino.delregno@somainline.org \
--cc=bjorn.andersson@linaro.org \
--cc=daniel@ffwll.ch \
--cc=dianders@chromium.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=jami.kettunen@somainline.org \
--cc=jonathan@marek.ca \
--cc=konrad.dybcio@somainline.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marijn.suijten@somainline.org \
--cc=martin.botka@somainline.org \
--cc=mka@chromium.org \
--cc=mturquette@baylibre.com \
--cc=phone-devel@vger.kernel.org \
--cc=robdclark@gmail.com \
--cc=sboyd@kernel.org \
--cc=sean@poorly.run \
--cc=swboyd@chromium.org \
--cc=~postmarketos/upstreaming@lists.sr.ht \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.