Re: [PATCH v2 13/13] dt-bindings: msm/hdmi: Add HDMI PHY bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 02/23/2016 12:57 AM, Rob Herring wrote:
On Mon, Feb 22, 2016 at 5:07 AM, Archit Taneja <architt@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
Cc: Rob Herring <robh@xxxxxxxxxx>

Signed-off-by: Archit Taneja <architt@xxxxxxxxxxxxxx>
---
   .../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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux