* [PATCH v2 13/13] dt-bindings: msm/hdmi: Add HDMI PHY bindings
[not found] <1455519206-12939-1-git-send-email-architt@codeaurora.org>
@ 2016-02-15 6:53 ` Archit Taneja
2016-02-22 2:54 ` Rob Herring
0 siblings, 1 reply; 7+ messages in thread
From: Archit Taneja @ 2016-02-15 6:53 UTC (permalink / raw
To: robdclark; +Cc: linux-arm-msm, dri-devel, devicetree
Add HDMI PHY bindings. Update the example to use HDMI PHY.
Add a missing power-domains property in the HDMI core bindings.
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
.../devicetree/bindings/display/msm/hdmi.txt | 39 +++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/msm/hdmi.txt b/Documentation/devicetree/bindings/display/msm/hdmi.txt
index 379ee2e..4d71910 100644
--- a/Documentation/devicetree/bindings/display/msm/hdmi.txt
+++ b/Documentation/devicetree/bindings/display/msm/hdmi.txt
@@ -11,6 +11,7 @@ Required properties:
- reg: Physical base address and length of the controller's registers
- reg-names: "core_physical"
- interrupts: The interrupt signal from the hdmi block.
+- power-domains: Should be <&mmcc MDSS_GDSC>.
- clocks: device clocks
See ../clocks/clock-bindings.txt for details.
- qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin
@@ -18,6 +19,7 @@ Required properties:
- qcom,hdmi-tx-hpd-gpio: hpd pin
- core-vdda-supply: phandle to supply regulator
- hdmi-mux-supply: phandle to mux regulator
+- qcom,hdmi-phy: phandle to HDMI PHY device node
Optional properties:
- qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin
@@ -27,6 +29,27 @@ Optional properties:
- pinctrl-0: the default pinctrl state (active)
- pinctrl-1: the "sleep" pinctrl state
+HDMI PHY:
+Required properties:
+- compatible: Could be the following
+ * "qcom,hdmi-phy-8x60"
+ * "qcom,hdmi-phy-8960"
+ * "qcom,hdmi-phy-8x74"
+ * "qcom,hdmi-phy-8996"
+- reg: Physical base address and length of the registers of the PHY sub blocks.
+- reg-names: The names of register regions. The following regions are required:
+ * "hdmi_pll"
+ * "hdmi_phy"
+ For HDMI PHY on msm8996, these additional register regions are required:
+ * "hdmi_tx_l0"
+ * "hdmi_tx_l1"
+ * "hdmi_tx_l3"
+ * "hdmi_tx_l4"
+- power-domains: Should be <&mmcc MDSS_GDSC>.
+- clocks: device clocks
+ See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
+- core-vdda-supply: phandle to vdda regulator device node
+
Example:
/ {
@@ -35,7 +58,7 @@ Example:
hdmi: qcom,hdmi-tx-8960@4a00000 {
compatible = "qcom,hdmi-tx-8960";
reg-names = "core_physical";
- reg = <0x04a00000 0x1000>;
+ reg = <0x04a00000 0x2f0>;
interrupts = <GIC_SPI 79 0>;
power-domains = <&mmcc MDSS_GDSC>;
clock-names =
@@ -54,5 +77,19 @@ Example:
pinctrl-names = "default", "sleep";
pinctrl-0 = <&hpd_active &ddc_active &cec_active>;
pinctrl-1 = <&hpd_suspend &ddc_suspend &cec_suspend>;
+
+ qcom,hdmi-phy = <&hdmi_phy>;
+ };
+
+ hdmi_phy: qcom,hdmi-phy-8960@4a00400 {
+ compatible = "qcom,hdmi-phy-8960";
+ reg-names = "hdmi_phy",
+ "hdmi_pll";
+ reg = <0x4a00400 0x60>,
+ <0x4a00500 0x100>;
+ power-domains = <&mmcc MDSS_GDSC>;
+ clock-names = "slave_iface_clk";
+ clocks = <&mmcc HDMI_S_AHB_CLK>;
+ core-vdda-supply = <&pm8921_hdmi_mvs>;
};
};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 13/13] dt-bindings: msm/hdmi: Add HDMI PHY bindings
2016-02-15 6:53 ` [PATCH v2 13/13] dt-bindings: msm/hdmi: Add HDMI PHY bindings Archit Taneja
@ 2016-02-22 2:54 ` Rob Herring
2016-02-22 11:07 ` Archit Taneja
0 siblings, 1 reply; 7+ messages in thread
From: Rob Herring @ 2016-02-22 2:54 UTC (permalink / raw
To: Archit Taneja; +Cc: linux-arm-msm, dri-devel, devicetree
On Mon, Feb 15, 2016 at 12:23:26PM +0530, Archit Taneja wrote:
> Add HDMI PHY bindings. Update the example to use HDMI PHY.
>
> Add a missing power-domains property in the HDMI core bindings.
>
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
>
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
> .../devicetree/bindings/display/msm/hdmi.txt | 39 +++++++++++++++++++++-
> 1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/hdmi.txt b/Documentation/devicetree/bindings/display/msm/hdmi.txt
> index 379ee2e..4d71910 100644
> --- a/Documentation/devicetree/bindings/display/msm/hdmi.txt
> +++ b/Documentation/devicetree/bindings/display/msm/hdmi.txt
> @@ -11,6 +11,7 @@ Required properties:
> - reg: Physical base address and length of the controller's registers
> - reg-names: "core_physical"
> - interrupts: The interrupt signal from the hdmi block.
> +- power-domains: Should be <&mmcc MDSS_GDSC>.
> - clocks: device clocks
> See ../clocks/clock-bindings.txt for details.
> - qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin
> @@ -18,6 +19,7 @@ Required properties:
> - qcom,hdmi-tx-hpd-gpio: hpd pin
> - core-vdda-supply: phandle to supply regulator
> - hdmi-mux-supply: phandle to mux regulator
> +- qcom,hdmi-phy: phandle to HDMI PHY device node
Why not use the generic phy binding?
>
> Optional properties:
> - qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin
> @@ -27,6 +29,27 @@ Optional properties:
> - pinctrl-0: the default pinctrl state (active)
> - pinctrl-1: the "sleep" pinctrl state
>
> +HDMI PHY:
> +Required properties:
> +- compatible: Could be the following
> + * "qcom,hdmi-phy-8x60"
> + * "qcom,hdmi-phy-8960"
> + * "qcom,hdmi-phy-8x74"
No wildcards please. Where's 8994?
> + * "qcom,hdmi-phy-8996"
> +- reg: Physical base address and length of the registers of the PHY sub blocks.
> +- reg-names: The names of register regions. The following regions are required:
> + * "hdmi_pll"
> + * "hdmi_phy"
> + For HDMI PHY on msm8996, these additional register regions are required:
> + * "hdmi_tx_l0"
> + * "hdmi_tx_l1"
> + * "hdmi_tx_l3"
> + * "hdmi_tx_l4"
> +- power-domains: Should be <&mmcc MDSS_GDSC>.
> +- clocks: device clocks
> + See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
> +- core-vdda-supply: phandle to vdda regulator device node
> +
> Example:
>
> / {
> @@ -35,7 +58,7 @@ Example:
> hdmi: qcom,hdmi-tx-8960@4a00000 {
Node names should be generic, so just "hdmi".
> compatible = "qcom,hdmi-tx-8960";
> reg-names = "core_physical";
> - reg = <0x04a00000 0x1000>;
> + reg = <0x04a00000 0x2f0>;
> interrupts = <GIC_SPI 79 0>;
> power-domains = <&mmcc MDSS_GDSC>;
> clock-names =
> @@ -54,5 +77,19 @@ Example:
> pinctrl-names = "default", "sleep";
> pinctrl-0 = <&hpd_active &ddc_active &cec_active>;
> pinctrl-1 = <&hpd_suspend &ddc_suspend &cec_suspend>;
> +
> + qcom,hdmi-phy = <&hdmi_phy>;
> + };
> +
> + hdmi_phy: qcom,hdmi-phy-8960@4a00400 {
ditto. phy@...
> + compatible = "qcom,hdmi-phy-8960";
> + reg-names = "hdmi_phy",
> + "hdmi_pll";
> + reg = <0x4a00400 0x60>,
> + <0x4a00500 0x100>;
> + power-domains = <&mmcc MDSS_GDSC>;
> + clock-names = "slave_iface_clk";
> + clocks = <&mmcc HDMI_S_AHB_CLK>;
> + core-vdda-supply = <&pm8921_hdmi_mvs>;
> };
> };
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 13/13] dt-bindings: msm/hdmi: Add HDMI PHY bindings
2016-02-22 2:54 ` Rob Herring
@ 2016-02-22 11:07 ` Archit Taneja
2016-02-22 19:27 ` Rob Herring
0 siblings, 1 reply; 7+ messages in thread
From: Archit Taneja @ 2016-02-22 11:07 UTC (permalink / raw
To: Rob Herring; +Cc: linux-arm-msm, dri-devel, devicetree
On 02/22/2016 08:24 AM, Rob Herring wrote:
> On Mon, Feb 15, 2016 at 12:23:26PM +0530, Archit Taneja wrote:
>> Add HDMI PHY bindings. Update the example to use HDMI PHY.
>>
>> Add a missing power-domains property in the HDMI core bindings.
>>
>> Cc: devicetree@vger.kernel.org
>> Cc: Rob Herring <robh@kernel.org>
>>
>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>> ---
>> .../devicetree/bindings/display/msm/hdmi.txt | 39 +++++++++++++++++++++-
>> 1 file changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/msm/hdmi.txt b/Documentation/devicetree/bindings/display/msm/hdmi.txt
>> index 379ee2e..4d71910 100644
>> --- a/Documentation/devicetree/bindings/display/msm/hdmi.txt
>> +++ b/Documentation/devicetree/bindings/display/msm/hdmi.txt
>> @@ -11,6 +11,7 @@ Required properties:
>> - reg: Physical base address and length of the controller's registers
>> - reg-names: "core_physical"
>> - interrupts: The interrupt signal from the hdmi block.
>> +- power-domains: Should be <&mmcc MDSS_GDSC>.
>> - clocks: device clocks
>> See ../clocks/clock-bindings.txt for details.
>> - qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin
>> @@ -18,6 +19,7 @@ Required properties:
>> - qcom,hdmi-tx-hpd-gpio: hpd pin
>> - core-vdda-supply: phandle to supply regulator
>> - hdmi-mux-supply: phandle to mux regulator
>> +- qcom,hdmi-phy: phandle to HDMI PHY device node
>
> Why not use the generic phy binding?
You'd asked about this in the first version of this patch. You
probably missed reading my reply. Partially my fault since I
missed out putting the "In-Reply-to" when posting this set. I've
mentioned the reason again here:
The PHY in the HDMI and DSI blocks can't be implemented using the
common phy framework. The PHY blocks have a PLL sub-block within
them which acts as a pixel clock source for the display processor
block.
This dependency causes the need to split the phy power on sequence
into 2 parts (one to enable resources to enable the PLL, and the
other to enable the phy itself), which the phy framework can't
do. That's the main reason not to use it. There are some more
complex use cases for DSI PHY (drive two PHYs with the same
DSI PLL) which the phy framework can't support.
>
>>
>> Optional properties:
>> - qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin
>> @@ -27,6 +29,27 @@ Optional properties:
>> - pinctrl-0: the default pinctrl state (active)
>> - pinctrl-1: the "sleep" pinctrl state
>>
>> +HDMI PHY:
>> +Required properties:
>> +- compatible: Could be the following
>> + * "qcom,hdmi-phy-8x60"
>> + * "qcom,hdmi-phy-8960"
>> + * "qcom,hdmi-phy-8x74"
>
> No wildcards please. Where's 8994?
I'll remove the wildcards. 8994 PHY isn't supported by the driver at
the moment. I could keep the 8994 compatible string, the driver will
bail out with an error. But that's something we already do for 8x74
since it doesn't have full PHY support either.
>
>> + * "qcom,hdmi-phy-8996"
>> +- reg: Physical base address and length of the registers of the PHY sub blocks.
>> +- reg-names: The names of register regions. The following regions are required:
>> + * "hdmi_pll"
>> + * "hdmi_phy"
>> + For HDMI PHY on msm8996, these additional register regions are required:
>> + * "hdmi_tx_l0"
>> + * "hdmi_tx_l1"
>> + * "hdmi_tx_l3"
>> + * "hdmi_tx_l4"
>> +- power-domains: Should be <&mmcc MDSS_GDSC>.
>> +- clocks: device clocks
>> + See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
>> +- core-vdda-supply: phandle to vdda regulator device node
>> +
>> Example:
>>
>> / {
>> @@ -35,7 +58,7 @@ Example:
>> hdmi: qcom,hdmi-tx-8960@4a00000 {
>
> Node names should be generic, so just "hdmi".
>
>> compatible = "qcom,hdmi-tx-8960";
>> reg-names = "core_physical";
>> - reg = <0x04a00000 0x1000>;
>> + reg = <0x04a00000 0x2f0>;
>> interrupts = <GIC_SPI 79 0>;
>> power-domains = <&mmcc MDSS_GDSC>;
>> clock-names =
>> @@ -54,5 +77,19 @@ Example:
>> pinctrl-names = "default", "sleep";
>> pinctrl-0 = <&hpd_active &ddc_active &cec_active>;
>> pinctrl-1 = <&hpd_suspend &ddc_suspend &cec_suspend>;
>> +
>> + qcom,hdmi-phy = <&hdmi_phy>;
>> + };
>> +
>> + hdmi_phy: qcom,hdmi-phy-8960@4a00400 {
>
> ditto. phy@...
I'll fix these.
Thanks,
Archit
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 13/13] dt-bindings: msm/hdmi: Add HDMI PHY bindings
2016-02-22 11:07 ` Archit Taneja
@ 2016-02-22 19:27 ` Rob Herring
2016-02-23 9:36 ` Archit Taneja
0 siblings, 1 reply; 7+ messages in thread
From: Rob Herring @ 2016-02-22 19:27 UTC (permalink / raw
To: Archit Taneja
Cc: Rob Clark, linux-arm-msm, dri-devel, devicetree@vger.kernel.org
On Mon, Feb 22, 2016 at 5:07 AM, Archit Taneja <architt@codeaurora.org> wrote:
>
>
> On 02/22/2016 08:24 AM, Rob Herring wrote:
>>
>> On Mon, Feb 15, 2016 at 12:23:26PM +0530, Archit Taneja wrote:
>>>
>>> Add HDMI PHY bindings. Update the example to use HDMI PHY.
>>>
>>> Add a missing power-domains property in the HDMI core bindings.
>>>
>>> Cc: devicetree@vger.kernel.org
>>> Cc: Rob Herring <robh@kernel.org>
>>>
>>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>>> ---
>>> .../devicetree/bindings/display/msm/hdmi.txt | 39
>>> +++++++++++++++++++++-
>>> 1 file changed, 38 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>> b/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>> index 379ee2e..4d71910 100644
>>> --- a/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>> +++ b/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>> @@ -11,6 +11,7 @@ Required properties:
>>> - reg: Physical base address and length of the controller's registers
>>> - reg-names: "core_physical"
>>> - interrupts: The interrupt signal from the hdmi block.
>>> +- power-domains: Should be <&mmcc MDSS_GDSC>.
>>> - clocks: device clocks
>>> See ../clocks/clock-bindings.txt for details.
>>> - qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin
>>> @@ -18,6 +19,7 @@ Required properties:
>>> - qcom,hdmi-tx-hpd-gpio: hpd pin
>>> - core-vdda-supply: phandle to supply regulator
>>> - hdmi-mux-supply: phandle to mux regulator
>>> +- qcom,hdmi-phy: phandle to HDMI PHY device node
>>
>>
>> Why not use the generic phy binding?
>
>
> You'd asked about this in the first version of this patch. You
> probably missed reading my reply. Partially my fault since I
> missed out putting the "In-Reply-to" when posting this set. I've
> mentioned the reason again here:
>
> The PHY in the HDMI and DSI blocks can't be implemented using the
> common phy framework. The PHY blocks have a PLL sub-block within
> them which acts as a pixel clock source for the display processor
> block.
That sounds like a problem with the common phy framework, not the DT
binding. Nothing says you have to use the common phy f/w if you use
the phy binding. I say that as the binding maintainer. As a kernel
developer, I would say fix the common phy framework to handle this
case. However, I don't think anything prevents the phy driver from
registering both a phy and a clock.
> This dependency causes the need to split the phy power on sequence
> into 2 parts (one to enable resources to enable the PLL, and the
> other to enable the phy itself), which the phy framework can't
> do. That's the main reason not to use it. There are some more
> complex use cases for DSI PHY (drive two PHYs with the same
> DSI PLL) which the phy framework can't support.
Doesn't the phy framework already support the former? It has power on
and init calls. Personally, I find the split there ill-defined.
>>> Optional properties:
>>> - qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin
>>> @@ -27,6 +29,27 @@ Optional properties:
>>> - pinctrl-0: the default pinctrl state (active)
>>> - pinctrl-1: the "sleep" pinctrl state
>>>
>>> +HDMI PHY:
>>> +Required properties:
>>> +- compatible: Could be the following
>>> + * "qcom,hdmi-phy-8x60"
>>> + * "qcom,hdmi-phy-8960"
>>> + * "qcom,hdmi-phy-8x74"
>>
>>
>> No wildcards please. Where's 8994?
>
>
> I'll remove the wildcards. 8994 PHY isn't supported by the driver at
> the moment. I could keep the 8994 compatible string, the driver will
> bail out with an error. But that's something we already do for 8x74
> since it doesn't have full PHY support either.
You can document it later if you want. I just asked 'cause I knew the
8994 had one.
Rob
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 13/13] dt-bindings: msm/hdmi: Add HDMI PHY bindings
2016-02-22 19:27 ` Rob Herring
@ 2016-02-23 9:36 ` Archit Taneja
2016-02-24 12:00 ` Kishon Vijay Abraham I
0 siblings, 1 reply; 7+ messages in thread
From: Archit Taneja @ 2016-02-23 9:36 UTC (permalink / raw
To: Rob Herring, Kishon Vijay Abraham I
Cc: linux-arm-msm, dri-devel, devicetree@vger.kernel.org
On 02/23/2016 12:57 AM, Rob Herring wrote:
> On Mon, Feb 22, 2016 at 5:07 AM, Archit Taneja <architt@codeaurora.org> wrote:
>>
>>
>> On 02/22/2016 08:24 AM, Rob Herring wrote:
>>>
>>> On Mon, Feb 15, 2016 at 12:23:26PM +0530, Archit Taneja wrote:
>>>>
>>>> Add HDMI PHY bindings. Update the example to use HDMI PHY.
>>>>
>>>> Add a missing power-domains property in the HDMI core bindings.
>>>>
>>>> Cc: devicetree@vger.kernel.org
>>>> Cc: Rob Herring <robh@kernel.org>
>>>>
>>>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>>>> ---
>>>> .../devicetree/bindings/display/msm/hdmi.txt | 39
>>>> +++++++++++++++++++++-
>>>> 1 file changed, 38 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>>> b/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>>> index 379ee2e..4d71910 100644
>>>> --- a/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>>> +++ b/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>>> @@ -11,6 +11,7 @@ Required properties:
>>>> - reg: Physical base address and length of the controller's registers
>>>> - reg-names: "core_physical"
>>>> - interrupts: The interrupt signal from the hdmi block.
>>>> +- power-domains: Should be <&mmcc MDSS_GDSC>.
>>>> - clocks: device clocks
>>>> See ../clocks/clock-bindings.txt for details.
>>>> - qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin
>>>> @@ -18,6 +19,7 @@ Required properties:
>>>> - qcom,hdmi-tx-hpd-gpio: hpd pin
>>>> - core-vdda-supply: phandle to supply regulator
>>>> - hdmi-mux-supply: phandle to mux regulator
>>>> +- qcom,hdmi-phy: phandle to HDMI PHY device node
>>>
>>>
>>> Why not use the generic phy binding?
>>
>>
>> You'd asked about this in the first version of this patch. You
>> probably missed reading my reply. Partially my fault since I
>> missed out putting the "In-Reply-to" when posting this set. I've
>> mentioned the reason again here:
>>
>> The PHY in the HDMI and DSI blocks can't be implemented using the
>> common phy framework. The PHY blocks have a PLL sub-block within
>> them which acts as a pixel clock source for the display processor
>> block.
>
> That sounds like a problem with the common phy framework, not the DT
> binding. Nothing says you have to use the common phy f/w if you use
> the phy binding. I say that as the binding maintainer. As a kernel
> developer, I would say fix the common phy framework to handle this
> case. However, I don't think anything prevents the phy driver from
> registering both a phy and a clock.
Okay. For some reason, I thought it would be wrong to use the same
common phy bindings but use our own phy driver.
I will convert the bindings to the generic phy binding.
>
>> This dependency causes the need to split the phy power on sequence
>> into 2 parts (one to enable resources to enable the PLL, and the
>> other to enable the phy itself), which the phy framework can't
>> do. That's the main reason not to use it. There are some more
>> complex use cases for DSI PHY (drive two PHYs with the same
>> DSI PLL) which the phy framework can't support.
>
> Doesn't the phy framework already support the former? It has power on
> and init calls. Personally, I find the split there ill-defined.
I always assumed that the init/exit ops were something that you would
call just once (during probe/remove) and then forget about it. I only
now noticed that init and power_on are often paired together (as are
power_off and exit). I went through the common phy framework code in
more detail, but I realized I would face the following issue:
I was looking for the sequence:
1. enable PHY resources (enables clocks/regulators/pm_runtime_get_sync)
2. set_rate/enable the PLL clock provided by PHY
3. enable PHY (configure some PHY registers)
I can achieve this using the common phy framework if I tie step 1)
to phy_power_on(), and step 3) to phy_init(). The other way round won't
work because phy_init() seems to disable runtime pm as it exits.
If I use the phy framework in its current state, I'll end up stuffing
the phy ops and use them in a weird way just to make sure my PHY
works.
Copying Kishon if he has any opinions.
>
>>>> Optional properties:
>>>> - qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin
>>>> @@ -27,6 +29,27 @@ Optional properties:
>>>> - pinctrl-0: the default pinctrl state (active)
>>>> - pinctrl-1: the "sleep" pinctrl state
>>>>
>>>> +HDMI PHY:
>>>> +Required properties:
>>>> +- compatible: Could be the following
>>>> + * "qcom,hdmi-phy-8x60"
>>>> + * "qcom,hdmi-phy-8960"
>>>> + * "qcom,hdmi-phy-8x74"
>>>
>>>
>>> No wildcards please. Where's 8994?
>>
>>
>> I'll remove the wildcards. 8994 PHY isn't supported by the driver at
>> the moment. I could keep the 8994 compatible string, the driver will
>> bail out with an error. But that's something we already do for 8x74
>> since it doesn't have full PHY support either.
>
> You can document it later if you want. I just asked 'cause I knew the
> 8994 had one.
Okay.
Thanks,
Archit
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 13/13] dt-bindings: msm/hdmi: Add HDMI PHY bindings
2016-02-23 9:36 ` Archit Taneja
@ 2016-02-24 12:00 ` Kishon Vijay Abraham I
2016-02-25 5:40 ` Archit Taneja
0 siblings, 1 reply; 7+ messages in thread
From: Kishon Vijay Abraham I @ 2016-02-24 12:00 UTC (permalink / raw
To: Archit Taneja, Rob Herring
Cc: Rob Clark, linux-arm-msm, dri-devel, devicetree@vger.kernel.org
Hi Archit,
On Tuesday 23 February 2016 03:06 PM, Archit Taneja wrote:
>
>
> On 02/23/2016 12:57 AM, Rob Herring wrote:
>> On Mon, Feb 22, 2016 at 5:07 AM, Archit Taneja <architt@codeaurora.org> wrote:
>>>
>>>
>>> On 02/22/2016 08:24 AM, Rob Herring wrote:
>>>>
>>>> On Mon, Feb 15, 2016 at 12:23:26PM +0530, Archit Taneja wrote:
>>>>>
>>>>> Add HDMI PHY bindings. Update the example to use HDMI PHY.
>>>>>
>>>>> Add a missing power-domains property in the HDMI core bindings.
>>>>>
>>>>> Cc: devicetree@vger.kernel.org
>>>>> Cc: Rob Herring <robh@kernel.org>
>>>>>
>>>>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>>>>> ---
>>>>> .../devicetree/bindings/display/msm/hdmi.txt | 39
>>>>> +++++++++++++++++++++-
>>>>> 1 file changed, 38 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>>>> b/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>>>> index 379ee2e..4d71910 100644
>>>>> --- a/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>>>> +++ b/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>>>> @@ -11,6 +11,7 @@ Required properties:
>>>>> - reg: Physical base address and length of the controller's registers
>>>>> - reg-names: "core_physical"
>>>>> - interrupts: The interrupt signal from the hdmi block.
>>>>> +- power-domains: Should be <&mmcc MDSS_GDSC>.
>>>>> - clocks: device clocks
>>>>> See ../clocks/clock-bindings.txt for details.
>>>>> - qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin
>>>>> @@ -18,6 +19,7 @@ Required properties:
>>>>> - qcom,hdmi-tx-hpd-gpio: hpd pin
>>>>> - core-vdda-supply: phandle to supply regulator
>>>>> - hdmi-mux-supply: phandle to mux regulator
>>>>> +- qcom,hdmi-phy: phandle to HDMI PHY device node
>>>>
>>>>
>>>> Why not use the generic phy binding?
>>>
>>>
>>> You'd asked about this in the first version of this patch. You
>>> probably missed reading my reply. Partially my fault since I
>>> missed out putting the "In-Reply-to" when posting this set. I've
>>> mentioned the reason again here:
>>>
>>> The PHY in the HDMI and DSI blocks can't be implemented using the
>>> common phy framework. The PHY blocks have a PLL sub-block within
>>> them which acts as a pixel clock source for the display processor
>>> block.
>>
>> That sounds like a problem with the common phy framework, not the DT
>> binding. Nothing says you have to use the common phy f/w if you use
>> the phy binding. I say that as the binding maintainer. As a kernel
>> developer, I would say fix the common phy framework to handle this
>> case. However, I don't think anything prevents the phy driver from
>> registering both a phy and a clock.
>
> Okay. For some reason, I thought it would be wrong to use the same
> common phy bindings but use our own phy driver.
>
> I will convert the bindings to the generic phy binding.
>
>>
>>> This dependency causes the need to split the phy power on sequence
>>> into 2 parts (one to enable resources to enable the PLL, and the
>>> other to enable the phy itself), which the phy framework can't
>>> do. That's the main reason not to use it. There are some more
>>> complex use cases for DSI PHY (drive two PHYs with the same
>>> DSI PLL) which the phy framework can't support.
>>
>> Doesn't the phy framework already support the former? It has power on
>> and init calls. Personally, I find the split there ill-defined.
>
> I always assumed that the init/exit ops were something that you would
> call just once (during probe/remove) and then forget about it. I only
right, the phy ops were intended to be used that way. However because of
different sequence requirements for different controllers, that's not followed
always.
> now noticed that init and power_on are often paired together (as are
> power_off and exit). I went through the common phy framework code in
> more detail, but I realized I would face the following issue:
>
> I was looking for the sequence:
>
> 1. enable PHY resources (enables clocks/regulators/pm_runtime_get_sync)
get_sync is invoked by the phy core during phy_init and phy_power_on (it was
done basically to access registers that have to be configured during init and
power on). regulator is enabled during phy_power_on and clocks (opt func
clocks) should be enable during runtime callbacks in the driver (main clock is
enabled as part of get_sync).
>
> 2. set_rate/enable the PLL clock provided by PHY
If the PHY is the source of clock, then the PHY should also be modeled as clock
provider. (see rockchip-usb PHY)
>
> 3. enable PHY (configure some PHY registers)
the general configuration of the PHY should go in phy_init (e.g any calibration
settings) and registers to power_on the PHY should go in phy_power_on.
Thanks
Kishon
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 13/13] dt-bindings: msm/hdmi: Add HDMI PHY bindings
2016-02-24 12:00 ` Kishon Vijay Abraham I
@ 2016-02-25 5:40 ` Archit Taneja
0 siblings, 0 replies; 7+ messages in thread
From: Archit Taneja @ 2016-02-25 5:40 UTC (permalink / raw
To: Kishon Vijay Abraham I, Rob Herring
Cc: linux-arm-msm, dri-devel, devicetree@vger.kernel.org
Hi Kishon,
On 02/24/2016 05:30 PM, Kishon Vijay Abraham I wrote:
> Hi Archit,
>
> On Tuesday 23 February 2016 03:06 PM, Archit Taneja wrote:
>>
>>
>> On 02/23/2016 12:57 AM, Rob Herring wrote:
>>> On Mon, Feb 22, 2016 at 5:07 AM, Archit Taneja <architt@codeaurora.org> wrote:
>>>>
>>>>
>>>> On 02/22/2016 08:24 AM, Rob Herring wrote:
>>>>>
>>>>> On Mon, Feb 15, 2016 at 12:23:26PM +0530, Archit Taneja wrote:
>>>>>>
>>>>>> Add HDMI PHY bindings. Update the example to use HDMI PHY.
>>>>>>
>>>>>> Add a missing power-domains property in the HDMI core bindings.
>>>>>>
>>>>>> Cc: devicetree@vger.kernel.org
>>>>>> Cc: Rob Herring <robh@kernel.org>
>>>>>>
>>>>>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>>>>>> ---
>>>>>> .../devicetree/bindings/display/msm/hdmi.txt | 39
>>>>>> +++++++++++++++++++++-
>>>>>> 1 file changed, 38 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>>>>> b/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>>>>> index 379ee2e..4d71910 100644
>>>>>> --- a/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>>>>> +++ b/Documentation/devicetree/bindings/display/msm/hdmi.txt
>>>>>> @@ -11,6 +11,7 @@ Required properties:
>>>>>> - reg: Physical base address and length of the controller's registers
>>>>>> - reg-names: "core_physical"
>>>>>> - interrupts: The interrupt signal from the hdmi block.
>>>>>> +- power-domains: Should be <&mmcc MDSS_GDSC>.
>>>>>> - clocks: device clocks
>>>>>> See ../clocks/clock-bindings.txt for details.
>>>>>> - qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin
>>>>>> @@ -18,6 +19,7 @@ Required properties:
>>>>>> - qcom,hdmi-tx-hpd-gpio: hpd pin
>>>>>> - core-vdda-supply: phandle to supply regulator
>>>>>> - hdmi-mux-supply: phandle to mux regulator
>>>>>> +- qcom,hdmi-phy: phandle to HDMI PHY device node
>>>>>
>>>>>
>>>>> Why not use the generic phy binding?
>>>>
>>>>
>>>> You'd asked about this in the first version of this patch. You
>>>> probably missed reading my reply. Partially my fault since I
>>>> missed out putting the "In-Reply-to" when posting this set. I've
>>>> mentioned the reason again here:
>>>>
>>>> The PHY in the HDMI and DSI blocks can't be implemented using the
>>>> common phy framework. The PHY blocks have a PLL sub-block within
>>>> them which acts as a pixel clock source for the display processor
>>>> block.
>>>
>>> That sounds like a problem with the common phy framework, not the DT
>>> binding. Nothing says you have to use the common phy f/w if you use
>>> the phy binding. I say that as the binding maintainer. As a kernel
>>> developer, I would say fix the common phy framework to handle this
>>> case. However, I don't think anything prevents the phy driver from
>>> registering both a phy and a clock.
>>
>> Okay. For some reason, I thought it would be wrong to use the same
>> common phy bindings but use our own phy driver.
>>
>> I will convert the bindings to the generic phy binding.
>>
>>>
>>>> This dependency causes the need to split the phy power on sequence
>>>> into 2 parts (one to enable resources to enable the PLL, and the
>>>> other to enable the phy itself), which the phy framework can't
>>>> do. That's the main reason not to use it. There are some more
>>>> complex use cases for DSI PHY (drive two PHYs with the same
>>>> DSI PLL) which the phy framework can't support.
>>>
>>> Doesn't the phy framework already support the former? It has power on
>>> and init calls. Personally, I find the split there ill-defined.
>>
>> I always assumed that the init/exit ops were something that you would
>> call just once (during probe/remove) and then forget about it. I only
>
> right, the phy ops were intended to be used that way. However because of
> different sequence requirements for different controllers, that's not followed
> always.
>> now noticed that init and power_on are often paired together (as are
>> power_off and exit). I went through the common phy framework code in
>> more detail, but I realized I would face the following issue:
>>
>> I was looking for the sequence:
>>
>> 1. enable PHY resources (enables clocks/regulators/pm_runtime_get_sync)
>
> get_sync is invoked by the phy core during phy_init and phy_power_on (it was
> done basically to access registers that have to be configured during init and
> power on). regulator is enabled during phy_power_on and clocks (opt func
> clocks) should be enable during runtime callbacks in the driver (main clock is
> enabled as part of get_sync).
>>
>> 2. set_rate/enable the PLL clock provided by PHY
>
> If the PHY is the source of clock, then the PHY should also be modeled as clock
> provider. (see rockchip-usb PHY)
>>
>> 3. enable PHY (configure some PHY registers)
>
> the general configuration of the PHY should go in phy_init (e.g any calibration
> settings) and registers to power_on the PHY should go in phy_power_on.
Thanks for the input. I currently have a custom hdmi phy driver, which
is a clock provider too. The PLL clock provided by the PHY can't be
configured (clk_set_rate, enabe etc.) unless we enable some of the PHY
interface clocks and other resources (enabled via pm_runtime_get_sync).
These resources need to be left on for the clock to be configured
successfully. The only way I can think of mapping my code with the
phy ops that satisfies the above requirements will result in the
following sequence:
...
phy_power_on(phy);
clk_set_rate(pll);
clk_prepare_enable(pll);
phy_init(phy);
...
This would be a bit of a misuse. To make things more complicated, some
of the PHY register need to be configured in a sequence which also
contains PLL registers. So, instead of these being in
phy_init/phy_power_on, they have to be in the PLL's clock
ops, which sort of makes things even more vague.
I would go with my custom hdmi phy implementation for now using generic
phy bindings, but I'll try to investigate more on what can be done to
get this running using the common phy f/w.
Thanks,
Archit
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-02-25 5:40 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1455519206-12939-1-git-send-email-architt@codeaurora.org>
2016-02-15 6:53 ` [PATCH v2 13/13] dt-bindings: msm/hdmi: Add HDMI PHY bindings Archit Taneja
2016-02-22 2:54 ` Rob Herring
2016-02-22 11:07 ` Archit Taneja
2016-02-22 19:27 ` Rob Herring
2016-02-23 9:36 ` Archit Taneja
2016-02-24 12:00 ` Kishon Vijay Abraham I
2016-02-25 5:40 ` Archit Taneja
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 read-only IMAP folder(s) and NNTP newsgroup(s).