2021-05-31 13:57:31

by Rajeev Nandan

[permalink] [raw]
Subject: [v1 0/3] drm/msm/dsi: Add display DSI support for SC7280 target

Changes in this series add support for MSM display DSI CTRL & PHY drivers
for the SC7280 SoC, which has DSI controller v2.5.0 and DSI PHY v4.1.

This series also updates the missing bindings (yaml) for the 7nm DSI PHY
driver on "msm-next" branch.

Rajeev Nandan (3):
dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY
drm/msm/dsi: Add PHY configuration for SC7280
drm/msm/dsi: Add DSI support for SC7280

.../bindings/display/msm/dsi-phy-7nm.yaml | 68 ++++++++++++++++++++++
drivers/gpu/drm/msm/Kconfig | 6 +-
drivers/gpu/drm/msm/dsi/dsi_cfg.c | 20 +++++++
drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 +
drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 2 +
drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 1 +
drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 22 +++++++
7 files changed, 117 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml

--
2.7.4


2021-05-31 13:58:29

by Rajeev Nandan

[permalink] [raw]
Subject: [v1 3/3] drm/msm/dsi: Add DSI support for SC7280

Add support for v2.5.0 DSI block in the SC7280 SoC.

Signed-off-by: Rajeev Nandan <[email protected]>
---
drivers/gpu/drm/msm/dsi/dsi_cfg.c | 20 ++++++++++++++++++++
drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 +
2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index f3f1c03..d76a680 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -200,6 +200,24 @@ static const struct msm_dsi_config sc7180_dsi_cfg = {
.num_dsi = 1,
};

+static const char * const dsi_sc7280_bus_clk_names[] = {
+ "iface", "bus",
+};
+
+static const struct msm_dsi_config sc7280_dsi_cfg = {
+ .io_offset = DSI_6G_REG_SHIFT,
+ .reg_cfg = {
+ .num = 1,
+ .regs = {
+ {"vdda", 8350, 0 }, /* 1.2 V */
+ },
+ },
+ .bus_clk_names = dsi_sc7280_bus_clk_names,
+ .num_bus_clks = ARRAY_SIZE(dsi_sc7280_bus_clk_names),
+ .io_start = { 0xae94000 },
+ .num_dsi = 1,
+};
+
static const struct msm_dsi_host_cfg_ops msm_dsi_v2_host_ops = {
.link_clk_set_rate = dsi_link_clk_set_rate_v2,
.link_clk_enable = dsi_link_clk_enable_v2,
@@ -267,6 +285,8 @@ static const struct msm_dsi_cfg_handler dsi_cfg_handlers[] = {
&sdm845_dsi_cfg, &msm_dsi_6g_v2_host_ops},
{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_4_1,
&sc7180_dsi_cfg, &msm_dsi_6g_v2_host_ops},
+ {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_5_0,
+ &sc7280_dsi_cfg, &msm_dsi_6g_v2_host_ops},
};

const struct msm_dsi_cfg_handler *msm_dsi_cfg_get(u32 major, u32 minor)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.h b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
index ade9b60..b2c4d5e 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.h
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
@@ -24,6 +24,7 @@
#define MSM_DSI_6G_VER_MINOR_V2_3_0 0x20030000
#define MSM_DSI_6G_VER_MINOR_V2_4_0 0x20040000
#define MSM_DSI_6G_VER_MINOR_V2_4_1 0x20040001
+#define MSM_DSI_6G_VER_MINOR_V2_5_0 0x20050000

#define MSM_DSI_V2_VER_MINOR_8064 0x0

--
2.7.4

2021-05-31 13:58:56

by Rajeev Nandan

[permalink] [raw]
Subject: [v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY

Add YAML schema for the device tree bindings for MSM 7nm DSI PHY driver.

Cc: Jonathan Marek <[email protected]>
Signed-off-by: Rajeev Nandan <[email protected]>
---
.../bindings/display/msm/dsi-phy-7nm.yaml | 68 ++++++++++++++++++++++
1 file changed, 68 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml

diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
new file mode 100644
index 00000000..f17cfde
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/msm/dsi-phy-7nm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Display DSI 7nm PHY
+
+maintainers:
+ - Rajeev Nandan <[email protected]>
+
+allOf:
+ - $ref: dsi-phy-common.yaml#
+
+properties:
+ compatible:
+ oneOf:
+ - const: qcom,dsi-phy-7nm
+ - const: qcom,dsi-phy-7nm-7280
+ - const: qcom,dsi-phy-7nm-8150
+
+ reg:
+ items:
+ - description: dsi phy register set
+ - description: dsi phy lane register set
+ - description: dsi pll register set
+
+ reg-names:
+ items:
+ - const: dsi_phy
+ - const: dsi_phy_lane
+ - const: dsi_pll
+
+ vdds-supply:
+ description: Phandle to 0.9V power supply regulator device node.
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - vdds-supply
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,dispcc-sc7280.h>
+ #include <dt-bindings/clock/qcom,rpmh.h>
+
+ dsi-phy@ae94400 {
+ compatible = "qcom,dsi-phy-7nm-7280";
+ reg = <0x0ae94400 0x200>,
+ <0x0ae94600 0x280>,
+ <0x0ae94900 0x280>;
+ reg-names = "dsi_phy",
+ "dsi_phy_lane",
+ "dsi_pll";
+
+ #clock-cells = <1>;
+ #phy-cells = <0>;
+
+ clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+ <&rpmhcc RPMH_CXO_CLK>;
+ clock-names = "iface", "ref";
+
+ vdds-supply = <&vreg_l10c_0p8>;
+ };
+...
--
2.7.4

2021-05-31 17:59:48

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [v1 3/3] drm/msm/dsi: Add DSI support for SC7280

On 31/05/2021 16:33, Rajeev Nandan wrote:
> Add support for v2.5.0 DSI block in the SC7280 SoC.
>
> Signed-off-by: Rajeev Nandan <[email protected]>

Reviewed-by: Dmitry Baryshkov <[email protected]>


> ---
> drivers/gpu/drm/msm/dsi/dsi_cfg.c | 20 ++++++++++++++++++++
> drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 +
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> index f3f1c03..d76a680 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> @@ -200,6 +200,24 @@ static const struct msm_dsi_config sc7180_dsi_cfg = {
> .num_dsi = 1,
> };
>
> +static const char * const dsi_sc7280_bus_clk_names[] = {
> + "iface", "bus",
> +};
> +
> +static const struct msm_dsi_config sc7280_dsi_cfg = {
> + .io_offset = DSI_6G_REG_SHIFT,
> + .reg_cfg = {
> + .num = 1,
> + .regs = {
> + {"vdda", 8350, 0 }, /* 1.2 V */
> + },
> + },
> + .bus_clk_names = dsi_sc7280_bus_clk_names,
> + .num_bus_clks = ARRAY_SIZE(dsi_sc7280_bus_clk_names),
> + .io_start = { 0xae94000 },
> + .num_dsi = 1,
> +};
> +
> static const struct msm_dsi_host_cfg_ops msm_dsi_v2_host_ops = {
> .link_clk_set_rate = dsi_link_clk_set_rate_v2,
> .link_clk_enable = dsi_link_clk_enable_v2,
> @@ -267,6 +285,8 @@ static const struct msm_dsi_cfg_handler dsi_cfg_handlers[] = {
> &sdm845_dsi_cfg, &msm_dsi_6g_v2_host_ops},
> {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_4_1,
> &sc7180_dsi_cfg, &msm_dsi_6g_v2_host_ops},
> + {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_5_0,
> + &sc7280_dsi_cfg, &msm_dsi_6g_v2_host_ops},
> };
>
> const struct msm_dsi_cfg_handler *msm_dsi_cfg_get(u32 major, u32 minor)
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.h b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
> index ade9b60..b2c4d5e 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.h
> +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
> @@ -24,6 +24,7 @@
> #define MSM_DSI_6G_VER_MINOR_V2_3_0 0x20030000
> #define MSM_DSI_6G_VER_MINOR_V2_4_0 0x20040000
> #define MSM_DSI_6G_VER_MINOR_V2_4_1 0x20040001
> +#define MSM_DSI_6G_VER_MINOR_V2_5_0 0x20050000
>
> #define MSM_DSI_V2_VER_MINOR_8064 0x0
>
>


--
With best wishes
Dmitry

2021-06-01 13:33:21

by Rob Herring

[permalink] [raw]
Subject: Re: [v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY

On Mon, 31 May 2021 19:03:53 +0530, Rajeev Nandan wrote:
> Add YAML schema for the device tree bindings for MSM 7nm DSI PHY driver.
>
> Cc: Jonathan Marek <[email protected]>
> Signed-off-by: Rajeev Nandan <[email protected]>
> ---
> .../bindings/display/msm/dsi-phy-7nm.yaml | 68 ++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.8/dist-packages/dtschema/schemas/display/msm/dsi-phy-common.yaml'
xargs: dt-doc-validate: exited with status 255; aborting
Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.example.dts:19:18: fatal error: dt-bindings/clock/qcom,dispcc-sc7280.h: No such file or directory
19 | #include <dt-bindings/clock/qcom,dispcc-sc7280.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1416: dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1485686

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

2021-06-01 21:00:48

by Rob Herring

[permalink] [raw]
Subject: Re: [v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY

On Mon, May 31, 2021 at 07:03:53PM +0530, Rajeev Nandan wrote:
> Add YAML schema for the device tree bindings for MSM 7nm DSI PHY driver.
>
> Cc: Jonathan Marek <[email protected]>
> Signed-off-by: Rajeev Nandan <[email protected]>
> ---
> .../bindings/display/msm/dsi-phy-7nm.yaml | 68 ++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
> new file mode 100644
> index 00000000..f17cfde
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/msm/dsi-phy-7nm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Display DSI 7nm PHY
> +
> +maintainers:
> + - Rajeev Nandan <[email protected]>
> +
> +allOf:
> + - $ref: dsi-phy-common.yaml#
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: qcom,dsi-phy-7nm

When would one use this?

> + - const: qcom,dsi-phy-7nm-7280
> + - const: qcom,dsi-phy-7nm-8150

These don't look like full SoC names (sm8150?) and it's
<vendor>,<soc>-<block>.

> +
> + reg:
> + items:
> + - description: dsi phy register set
> + - description: dsi phy lane register set
> + - description: dsi pll register set
> +
> + reg-names:
> + items:
> + - const: dsi_phy
> + - const: dsi_phy_lane
> + - const: dsi_pll
> +
> + vdds-supply:
> + description: Phandle to 0.9V power supply regulator device node.
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - vdds-supply
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,dispcc-sc7280.h>
> + #include <dt-bindings/clock/qcom,rpmh.h>
> +
> + dsi-phy@ae94400 {
> + compatible = "qcom,dsi-phy-7nm-7280";
> + reg = <0x0ae94400 0x200>,
> + <0x0ae94600 0x280>,
> + <0x0ae94900 0x280>;
> + reg-names = "dsi_phy",
> + "dsi_phy_lane",
> + "dsi_pll";
> +
> + #clock-cells = <1>;
> + #phy-cells = <0>;
> +
> + clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> + <&rpmhcc RPMH_CXO_CLK>;
> + clock-names = "iface", "ref";
> +
> + vdds-supply = <&vreg_l10c_0p8>;
> + };
> +...
> --
> 2.7.4

2021-06-02 20:05:47

by Rajeev Nandan

[permalink] [raw]
Subject: Re: [v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY

On 02-06-2021 02:28, Rob Herring wrote:
> On Mon, May 31, 2021 at 07:03:53PM +0530, Rajeev Nandan wrote:

>> +
>> +properties:
>> + compatible:
>> + oneOf:
>> + - const: qcom,dsi-phy-7nm
>
> When would one use this?
This is for SM8250.

>
>> + - const: qcom,dsi-phy-7nm-7280
>> + - const: qcom,dsi-phy-7nm-8150
>
> These don't look like full SoC names (sm8150?) and it's
> <vendor>,<soc>-<block>.

Thanks, Rob, for the review.

I just took the `compatible` property currently used in the DSI PHY
driver
(drivers/gpu/drm/msm/dsi/phy/dsi_phy.c), and added a new entry for
sc7280.
A similar pattern of `compatible` names are used in other variants of
the
DSI PHY driver e.g. qcom,qcom,dsi-phy-10nm-8998, qcom,dsi-phy-14nm-660
etc.

The existing compatible names "qcom,dsi-phy-7nm-8150" (SoC at the end)
make
some sense, if we look at the organization of the dsi phy driver code.
I am new to this and don't know the reason behind the current code
organization and this naming.

Yes, I agree with you, we should use full SoC names. Adding
the SoC name at the end does not feel very convincing, so I will change
this
to the suggested format e.g. "qcom,sm8250-dsi-phy-7nm", and will rename
the
occurrences in the driver and device tree accordingly.
Do I need to make changes for 10nm, 14nm, 20nm, and 28nm DSI PHY too?
Bindings doc for these PHYs recently got merged to msm-next [1]


[1]
https://gitlab.freedesktop.org/drm/msm/-/commit/8fc939e72ff80116c090aaf03952253a124d2a8e


Thanks,
Rajeev

2021-06-16 05:53:41

by Rajeev Nandan

[permalink] [raw]
Subject: Re: [v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY

On 03-06-2021 01:32, [email protected] wrote:
> On 02-06-2021 02:28, Rob Herring wrote:
>> On Mon, May 31, 2021 at 07:03:53PM +0530, Rajeev Nandan wrote:
>
>>> +
>>> +properties:
>>> + compatible:
>>> + oneOf:
>>> + - const: qcom,dsi-phy-7nm
>>
>> When would one use this?
> This is for SM8250.
>
>>
>>> + - const: qcom,dsi-phy-7nm-7280
>>> + - const: qcom,dsi-phy-7nm-8150
>>
>> These don't look like full SoC names (sm8150?) and it's
>> <vendor>,<soc>-<block>.
>
> Thanks, Rob, for the review.
>
> I just took the `compatible` property currently used in the DSI PHY
> driver
> (drivers/gpu/drm/msm/dsi/phy/dsi_phy.c), and added a new entry for
> sc7280.
> A similar pattern of `compatible` names are used in other variants of
> the
> DSI PHY driver e.g. qcom,qcom,dsi-phy-10nm-8998, qcom,dsi-phy-14nm-660
> etc.
>
> The existing compatible names "qcom,dsi-phy-7nm-8150" (SoC at the end)
> make
> some sense, if we look at the organization of the dsi phy driver code.
> I am new to this and don't know the reason behind the current code
> organization and this naming.
>
> Yes, I agree with you, we should use full SoC names. Adding
> the SoC name at the end does not feel very convincing, so I will change
> this
> to the suggested format e.g. "qcom,sm8250-dsi-phy-7nm", and will rename
> the
> occurrences in the driver and device tree accordingly.
> Do I need to make changes for 10nm, 14nm, 20nm, and 28nm DSI PHY too?
> Bindings doc for these PHYs recently got merged to msm-next [1]
>
>
> [1]
> https://gitlab.freedesktop.org/drm/msm/-/commit/8fc939e72ff80116c090aaf03952253a124d2a8e
>

Hi Rob,

I missed adding "[email protected]" earlier in this thread.

Please check my response to your review comments. Regarding your
suggestion to use <vendor>,<soc>-<block> format for compatible property,
should I also upload a new patch to make changes in 10nm, 14nm, 20nm,
and 28nm DSI PHY DT bindings?

Thanks,
Rajeev



2021-06-17 15:10:45

by Jonathan Marek

[permalink] [raw]
Subject: Re: [v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY

On 6/16/21 1:50 AM, [email protected] wrote:
> On 03-06-2021 01:32, [email protected] wrote:
>> On 02-06-2021 02:28, Rob Herring wrote:
>>> On Mon, May 31, 2021 at 07:03:53PM +0530, Rajeev Nandan wrote:
>>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    oneOf:
>>>> +      - const: qcom,dsi-phy-7nm
>>>
>>> When would one use this?
>> This is for SM8250.
>>
>>>
>>>> +      - const: qcom,dsi-phy-7nm-7280
>>>> +      - const: qcom,dsi-phy-7nm-8150
>>>
>>> These don't look like full SoC names (sm8150?) and it's
>>> <vendor>,<soc>-<block>.
>>
>> Thanks, Rob, for the review.
>>
>> I just took the `compatible` property currently used in the DSI PHY
>> driver
>> (drivers/gpu/drm/msm/dsi/phy/dsi_phy.c), and added a new entry for
>> sc7280.
>> A similar pattern of `compatible` names are used in other variants of the
>> DSI PHY driver e.g. qcom,qcom,dsi-phy-10nm-8998, qcom,dsi-phy-14nm-660
>> etc.
>>
>> The existing compatible names "qcom,dsi-phy-7nm-8150" (SoC at the end)
>> make
>> some sense, if we look at the organization of the dsi phy driver code.
>> I am new to this and don't know the reason behind the current code
>> organization and this naming.
>>
>> Yes, I agree with you, we should use full SoC names. Adding
>> the SoC name at the end does not feel very convincing, so I will
>> change this
>> to the suggested format e.g. "qcom,sm8250-dsi-phy-7nm", and will
>> rename the
>> occurrences in the driver and device tree accordingly.
>> Do I need to make changes for 10nm, 14nm, 20nm, and 28nm DSI PHY too?
>> Bindings doc for these PHYs recently got merged to msm-next [1]
>>
>>
>> [1]
>> https://gitlab.freedesktop.org/drm/msm/-/commit/8fc939e72ff80116c090aaf03952253a124d2a8e
>>
>>
>
> Hi Rob,
>
> I missed adding "[email protected]" earlier in this thread.
>
> Please check my response to your review comments. Regarding your
> suggestion to use <vendor>,<soc>-<block> format for compatible property,
> should I also upload a new patch to make changes in 10nm, 14nm, 20nm,
> and 28nm DSI PHY DT bindings?
>
> Thanks,
> Rajeev
>

Hi,

I missed this and ended up sending a similar patch a week later (as part
of my cphy series, because I needed it to add a "phy-type" property).

"qcom,dsi-phy-7nm" and "qcom,dsi-phy-7nm-8150" aren't new compatibles,
they were previously documented in the .txt bindings, which are getting
removed, but the new .yaml bindings didn't include them. Documenting
them is just a fixup to that patch [1] which is already R-B'd by RobH
(and has similar compatibles such as "qcom,dsi-phy-10nm" and
"qcom,dsi-phy-10nm-8998
").

You can use a different/better naming scheme for sc7280, but changing
the others has nothing to do with adding support for sc7280.

[1]
https://gitlab.freedesktop.org/drm/msm/-/commit/8fc939e72ff80116c090aaf03952253a124d2a8e




2021-06-18 12:35:28

by Rajeev Nandan

[permalink] [raw]
Subject: Re: [v1 1/3] dt-bindings: msm/dsi: Add yaml schema for 7nm DSI PHY


On 17-06-2021 20:37, Jonathan Marek wrote:
> On 6/16/21 1:50 AM, [email protected] wrote:
>> On 03-06-2021 01:32, [email protected] wrote:
>>> On 02-06-2021 02:28, Rob Herring wrote:
>>>> On Mon, May 31, 2021 at 07:03:53PM +0530, Rajeev Nandan wrote:
>>>
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    oneOf:
>>>>> +      - const: qcom,dsi-phy-7nm
>>>>
>>>> When would one use this?
>>> This is for SM8250.
>>>
>>>>
>>>>> +      - const: qcom,dsi-phy-7nm-7280
>>>>> +      - const: qcom,dsi-phy-7nm-8150
>>>>
>>>> These don't look like full SoC names (sm8150?) and it's
>>>> <vendor>,<soc>-<block>.
>>>
>>> Thanks, Rob, for the review.
>>>
>>> I just took the `compatible` property currently used in the DSI PHY
>>> driver
>>> (drivers/gpu/drm/msm/dsi/phy/dsi_phy.c), and added a new entry for
>>> sc7280.
>>> A similar pattern of `compatible` names are used in other variants of
>>> the
>>> DSI PHY driver e.g. qcom,qcom,dsi-phy-10nm-8998,
>>> qcom,dsi-phy-14nm-660 etc.
>>>
>>> The existing compatible names "qcom,dsi-phy-7nm-8150" (SoC at the
>>> end) make
>>> some sense, if we look at the organization of the dsi phy driver
>>> code.
>>> I am new to this and don't know the reason behind the current code
>>> organization and this naming.
>>>
>>> Yes, I agree with you, we should use full SoC names. Adding
>>> the SoC name at the end does not feel very convincing, so I will
>>> change this
>>> to the suggested format e.g. "qcom,sm8250-dsi-phy-7nm", and will
>>> rename the
>>> occurrences in the driver and device tree accordingly.
>>> Do I need to make changes for 10nm, 14nm, 20nm, and 28nm DSI PHY too?
>>> Bindings doc for these PHYs recently got merged to msm-next [1]
>>>
>>>
>>> [1]
>>> https://gitlab.freedesktop.org/drm/msm/-/commit/8fc939e72ff80116c090aaf03952253a124d2a8e
>>
>> Hi Rob,
>>
>> I missed adding "[email protected]" earlier in this thread.
>>
>> Please check my response to your review comments. Regarding your
>> suggestion to use <vendor>,<soc>-<block> format for compatible
>> property, should I also upload a new patch to make changes in 10nm,
>> 14nm, 20nm, and 28nm DSI PHY DT bindings?
>>
>> Thanks,
>> Rajeev
>>
>
> Hi,
>
> I missed this and ended up sending a similar patch a week later (as
> part of my cphy series, because I needed it to add a "phy-type"
> property).
>
> "qcom,dsi-phy-7nm" and "qcom,dsi-phy-7nm-8150" aren't new compatibles,
> they were previously documented in the .txt bindings, which are
> getting removed, but the new .yaml bindings didn't include them.
> Documenting them is just a fixup to that patch [1] which is already
> R-B'd by RobH (and has similar compatibles such as "qcom,dsi-phy-10nm"
> and "qcom,dsi-phy-10nm-8998
> ").
>
> You can use a different/better naming scheme for sc7280, but changing
> the others has nothing to do with adding support for sc7280.
>
> [1]
> https://gitlab.freedesktop.org/drm/msm/-/commit/8fc939e72ff80116c090aaf03952253a124d2a8e

Hi Jonathan,

I will discard this patch and will add the bindings for the sc7280 on
top of your patch [1].

[1]
https://lore.kernel.org/linux-arm-msm/[email protected]/


Thanks,
Rajeev