Hi,
Cc: devicetree-discuss@lists.ozlabs.org
On 09/24/2012 09:36 PM, Ajay Kumar wrote:
quoted hunk
This patch enables device tree based discovery support for DP driver.
The driver is modified to handle platform data in both the cases:
with DT and non-DT.
Documentation is also added for the DT bindings.
DP-PHY should be regarded as a seperate device node while
being passed from device tree list, and device node for
DP should contain DP-PHY as child node with property name "dp-phy"
associated with it.
Signed-off-by: Ajay Kumar<redacted>
---
.../devicetree/bindings/video/exynos_dp.txt | 83 ++++++++++
drivers/video/exynos/exynos_dp_core.c | 168 ++++++++++++++++++--
drivers/video/exynos/exynos_dp_core.h | 2 +
3 files changed, 239 insertions(+), 14 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/exynos_dp.txt
@@ -0,0 +1,83 @@+Exynos Displayport driver should configure the displayport interface
Don't we need a whitespace between 'display' and 'port' ?
quoted hunk
+based on the type of panel connected to it.++We use two nodes:+ -dptx_phy node+ -display-port-controller node++For the dp-phy initialization, we use a dptx_phy node.+Required properties for dptx_phy:+ -compatible:+ Should be "samsung,dp-phy".+ -samsung,dptx_phy_reg:+ Base address of DP PHY register.
Couldn't just 'reg' be used for this one ?
+ -samsung,enable_bit:
+ The bit used to enable/disable DP PHY.
Is this the bit mask or the bit index ? In the code it's used as
a bitmask. But from description it is not clear whether it is
an index or a mask. Is it different across various SoCs ?
Perhaps it's better to name it samsung,enable_mask (in case some
SoC need more than one bit) ?
quoted hunk
++For the Panel initialization, we read data from display-port-controller node.+Required properties for display-port-controller:+ -compatible:+ Should be "samsung,exynos5-dp".+ -reg:+ physical base address of the controller and length+ of memory mapped region.+ -interrupts:+ Internet combiner values.
what? :)
quoted hunk
+ -interrupt-parent:+ Address of Interrupt combiner node.+ -dp_phy:+ Address of dptx_phy node.
"A phandle to dptx_phy node" ?
quoted hunk
+ -samsung,color_space:+ input video data format.+ COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2
Can this be changed at run time ?
quoted hunk
+ -samsung,dynamic_range:+ dynamic range for input video data.+ VESA = 0, CEA = 1
Why is it in the device tree ? Shouldn't it be configurable at runtime ?
My apologies if this an obvious question, I don't have much experience
with DP.
quoted hunk
+ -samsung,ycbcr_coeff:+ YCbCr co-efficients for input video.+ COLOR_YCBCR601 = 0, COLOR_YCBCR709 = 1+ -samsung,color_depth:+ Bit per color component.
"Number of bits per colour component" ? Also same remark as above.
quoted hunk
+ COLOR_6 = 0, COLOR_8 = 1, COLOR_10 = 2, COLOR_12 = 3+ -samsung,link_rate:+ link rates supportd by the panel.
Is this really a property of a panel ? Why it is in the PHY node ?
Also I can see this is just a single property, so "link rates" is a bit
misleading.
quoted hunk
+ -samsung,lane_count:+ number of lanes supported by the panel.+ LANE_COUNT1 = 1, LANE_COUNT2 = 2, LANE_COUNT4 = 4
What do these symbolic names are needed for ? Is lane_count a number or a
mask, is this really a _maximum_ number of lanes ? What are the valid values,
1, 2 and 4 ? Or maybe 0x3 is also valid which would indicate that we can
use 1 or 2 data lanes ?
quoted hunk
+ -samsung,interlaced:+ Interlace scan mode.+ Progressive if defined, Interlaced if not defined
Why do we need this in the device tree ? Is this really a default scan mode ?
Can it be the changed at runtime ?
quoted hunk
+ -samsung,v_sync_polarity:+ VSYNC polarity configuration.+ High if defined, Low if not defined+ -samsung,h_sync_polarity:+ HSYNC polarity configuration.+ High if defined, Low if not defined++Example:++SOC specific portion:+ dptx_phy: dptx_phy@0x10040720 {+ compatible = "samsung,dp-phy";+ samsung,dptx_phy_reg =<0x10040720>;+ samsung,enable_bit =<1>;+ };++ display-port-controller {+ compatible = "samsung,exynos5-dp";+ reg =<0x145B0000 0x10000>;+ interrupts =<10 3>;+ interrupt-parent =<&combiner>;+ dp_phy =<&dptx_phy>;
From: Jingoo Han <hidden> Date: 2012-09-28 00:11:09
On Thursday, September 27, 2012 10:45 PM Sylwester Nawrocki wrote
Hi,
Cc: devicetree-discuss@lists.ozlabs.org
On 09/24/2012 09:36 PM, Ajay Kumar wrote:
quoted
This patch enables device tree based discovery support for DP driver.
The driver is modified to handle platform data in both the cases:
with DT and non-DT.
Documentation is also added for the DT bindings.
DP-PHY should be regarded as a seperate device node while
being passed from device tree list, and device node for
DP should contain DP-PHY as child node with property name "dp-phy"
associated with it.
Signed-off-by: Ajay Kumar<redacted>
---
.../devicetree/bindings/video/exynos_dp.txt | 83 ++++++++++
drivers/video/exynos/exynos_dp_core.c | 168 ++++++++++++++++++--
drivers/video/exynos/exynos_dp_core.h | 2 +
3 files changed, 239 insertions(+), 14 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/exynos_dp.txt
@@ -0,0 +1,83 @@+Exynos Displayport driver should configure the displayport interface
Don't we need a whitespace between 'display' and 'port' ?
Either 'display port' or 'DisplayPort' is commonly used.
For clarity, display port would be better.
quoted
+based on the type of panel connected to it.++We use two nodes:+ -dptx_phy node+ -display-port-controller node++For the dp-phy initialization, we use a dptx_phy node.+Required properties for dptx_phy:+ -compatible:+ Should be "samsung,dp-phy".+ -samsung,dptx_phy_reg:+ Base address of DP PHY register.
Couldn't just 'reg' be used for this one ?
quoted
+ -samsung,enable_bit:
+ The bit used to enable/disable DP PHY.
Is this the bit mask or the bit index ? In the code it's used as
a bitmask. But from description it is not clear whether it is
an index or a mask. Is it different across various SoCs ?
Perhaps it's better to name it samsung,enable_mask (in case some
SoC need more than one bit) ?
It's the bit mask.
It is different across various SoCs.
OK, enable_mask would be better.
quoted
++For the Panel initialization, we read data from display-port-controller node.+Required properties for display-port-controller:+ -compatible:+ Should be "samsung,exynos5-dp".+ -reg:+ physical base address of the controller and length+ of memory mapped region.+ -interrupts:+ Internet combiner values.
what? :)
Ajay: Please, fix it.
quoted
+ -interrupt-parent:+ Address of Interrupt combiner node.+ -dp_phy:+ Address of dptx_phy node.
"A phandle to dptx_phy node" ?
Yes, you're right.
Ajay: Please, replace 'Address of dptx_phy node'
with 'phandle of dptx_phy node'.
quoted
+ -samsung,color_space:+ input video data format.+ COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2
Can this be changed at run time ?
No, it is not changeable.
It's a default setting according to eDP panel.
quoted
+ -samsung,dynamic_range:+ dynamic range for input video data.+ VESA = 0, CEA = 1
Why is it in the device tree ? Shouldn't it be configurable at runtime ?
My apologies if this an obvious question, I don't have much experience
with DP.
Same above.
quoted
+ -samsung,ycbcr_coeff:+ YCbCr co-efficients for input video.+ COLOR_YCBCR601 = 0, COLOR_YCBCR709 = 1+ -samsung,color_depth:+ Bit per color component.
"Number of bits per colour component" ? Also same remark as above.
Same above.
quoted
+ COLOR_6 = 0, COLOR_8 = 1, COLOR_10 = 2, COLOR_12 = 3+ -samsung,link_rate:+ link rates supportd by the panel.
Is this really a property of a panel ? Why it is in the PHY node ?
Also I can see this is just a single property, so "link rates" is a bit
misleading.
Yes, It's a property of LCD panel.
It is not in the 'PHY' node, but 'display-port-controller' node.
Ajay: Please replace 'link rates' with 'link rate'.
quoted
+ -samsung,lane_count:+ number of lanes supported by the panel.+ LANE_COUNT1 = 1, LANE_COUNT2 = 2, LANE_COUNT4 = 4
What do these symbolic names are needed for ? Is lane_count a number or a
mask, is this really a _maximum_ number of lanes ? What are the valid values,
1, 2 and 4 ? Or maybe 0x3 is also valid which would indicate that we can
use 1 or 2 data lanes ?
These symbolic names are defined in './include/video/exynos_dp.h'.
It seems that Ajay used these definitions.
'lane_count' is a number of used lanes of main link which transfer data
streams as video. The number of lanes of Main Link is 1, 2, or 4 lanes.
It is dependent on eDP panel.
quoted
+ -samsung,interlaced:+ Interlace scan mode.+ Progressive if defined, Interlaced if not defined
Why do we need this in the device tree ? Is this really a default scan mode ?
Can it be the changed at runtime ?
No, it is not changeable.
It's a default setting according to eDP panel.
quoted
+ -samsung,v_sync_polarity:+ VSYNC polarity configuration.+ High if defined, Low if not defined+ -samsung,h_sync_polarity:+ HSYNC polarity configuration.+ High if defined, Low if not defined++Example:++SOC specific portion:+ dptx_phy: dptx_phy@0x10040720 {+ compatible = "samsung,dp-phy";+ samsung,dptx_phy_reg =<0x10040720>;+ samsung,enable_bit =<1>;+ };++ display-port-controller {+ compatible = "samsung,exynos5-dp";+ reg =<0x145B0000 0x10000>;+ interrupts =<10 3>;+ interrupt-parent =<&combiner>;+ dp_phy =<&dptx_phy>;
Shouldn't it be "samsung,dp_phy" ?
Do you mean this ? It is not working.
+ dp_phy = "samsung,dp_phy";
Do you mean this ? It is not working.
+ dp_phy = "samsung,dp_phy";
I believe he meant:
-+ dp_phy =<&dptx_phy>;
++ samsung,dp_phy =<&dptx_phy>;
to mark that this is a Samsung-specific property.
Best regards,
Tomasz Figa
On Fri, Sep 28, 2012 at 5:41 AM, Jingoo Han [off-list ref] wrote:
On Thursday, September 27, 2012 10:45 PM Sylwester Nawrocki wrote
quoted
Hi,
Cc: devicetree-discuss@lists.ozlabs.org
On 09/24/2012 09:36 PM, Ajay Kumar wrote:
quoted
This patch enables device tree based discovery support for DP driver.
The driver is modified to handle platform data in both the cases:
with DT and non-DT.
Documentation is also added for the DT bindings.
DP-PHY should be regarded as a seperate device node while
being passed from device tree list, and device node for
DP should contain DP-PHY as child node with property name "dp-phy"
associated with it.
Signed-off-by: Ajay Kumar<redacted>
---
.../devicetree/bindings/video/exynos_dp.txt | 83 ++++++++++
drivers/video/exynos/exynos_dp_core.c | 168 ++++++++++++++++++--
drivers/video/exynos/exynos_dp_core.h | 2 +
3 files changed, 239 insertions(+), 14 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/exynos_dp.txt
@@ -0,0 +1,83 @@+Exynos Displayport driver should configure the displayport interface
Don't we need a whitespace between 'display' and 'port' ?
Either 'display port' or 'DisplayPort' is commonly used.
For clarity, display port would be better.
Ok. I will change it.
quoted
quoted
+based on the type of panel connected to it.++We use two nodes:+ -dptx_phy node+ -display-port-controller node++For the dp-phy initialization, we use a dptx_phy node.+Required properties for dptx_phy:+ -compatible:+ Should be "samsung,dp-phy".+ -samsung,dptx_phy_reg:+ Base address of DP PHY register.
Couldn't just 'reg' be used for this one ?
quoted
+ -samsung,enable_bit:
+ The bit used to enable/disable DP PHY.
Is this the bit mask or the bit index ? In the code it's used as
a bitmask. But from description it is not clear whether it is
an index or a mask. Is it different across various SoCs ?
Perhaps it's better to name it samsung,enable_mask (in case some
SoC need more than one bit) ?
It's the bit mask.
It is different across various SoCs.
OK, enable_mask would be better.
Ok. I will change it.
quoted
quoted
++For the Panel initialization, we read data from display-port-controller node.+Required properties for display-port-controller:+ -compatible:+ Should be "samsung,exynos5-dp".+ -reg:+ physical base address of the controller and length+ of memory mapped region.+ -interrupts:+ Internet combiner values.
what? :)
Ajay: Please, fix it.
My bad :(...I will fix it.
quoted
quoted
+ -interrupt-parent:+ Address of Interrupt combiner node.+ -dp_phy:+ Address of dptx_phy node.
"A phandle to dptx_phy node" ?
Yes, you're right.
Ajay: Please, replace 'Address of dptx_phy node'
with 'phandle of dptx_phy node'.
Ok.
quoted
quoted
+ -samsung,color_space:+ input video data format.+ COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2
Can this be changed at run time ?
No, it is not changeable.
It's a default setting according to eDP panel.
quoted
quoted
+ -samsung,dynamic_range:+ dynamic range for input video data.+ VESA = 0, CEA = 1
Why is it in the device tree ? Shouldn't it be configurable at runtime ?
My apologies if this an obvious question, I don't have much experience
with DP.
Same above.
quoted
quoted
+ -samsung,ycbcr_coeff:+ YCbCr co-efficients for input video.+ COLOR_YCBCR601 = 0, COLOR_YCBCR709 = 1+ -samsung,color_depth:+ Bit per color component.
"Number of bits per colour component" ? Also same remark as above.
Same above.
quoted
quoted
+ COLOR_6 = 0, COLOR_8 = 1, COLOR_10 = 2, COLOR_12 = 3+ -samsung,link_rate:+ link rates supportd by the panel.
Is this really a property of a panel ? Why it is in the PHY node ?
Also I can see this is just a single property, so "link rates" is a bit
misleading.
Yes, It's a property of LCD panel.
It is not in the 'PHY' node, but 'display-port-controller' node.
Ajay: Please replace 'link rates' with 'link rate'.
Ok.
quoted
quoted
+ -samsung,lane_count:+ number of lanes supported by the panel.+ LANE_COUNT1 = 1, LANE_COUNT2 = 2, LANE_COUNT4 = 4
What do these symbolic names are needed for ? Is lane_count a number or a
mask, is this really a _maximum_ number of lanes ? What are the valid values,
1, 2 and 4 ? Or maybe 0x3 is also valid which would indicate that we can
use 1 or 2 data lanes ?
These symbolic names are defined in './include/video/exynos_dp.h'.
It seems that Ajay used these definitions.
Yes. I referred include/video/exynos_dp.h.
'lane_count' is a number of used lanes of main link which transfer data
streams as video. The number of lanes of Main Link is 1, 2, or 4 lanes.
It is dependent on eDP panel.
quoted
quoted
+ -samsung,interlaced:+ Interlace scan mode.+ Progressive if defined, Interlaced if not defined
Why do we need this in the device tree ? Is this really a default scan mode ?
Can it be the changed at runtime ?
No, it is not changeable.
It's a default setting according to eDP panel.
quoted
quoted
+ -samsung,v_sync_polarity:+ VSYNC polarity configuration.+ High if defined, Low if not defined+ -samsung,h_sync_polarity:+ HSYNC polarity configuration.+ High if defined, Low if not defined++Example:++SOC specific portion:+ dptx_phy: dptx_phy@0x10040720 {+ compatible = "samsung,dp-phy";+ samsung,dptx_phy_reg =<0x10040720>;+ samsung,enable_bit =<1>;+ };++ display-port-controller {+ compatible = "samsung,exynos5-dp";+ reg =<0x145B0000 0x10000>;+ interrupts =<10 3>;+ interrupt-parent =<&combiner>;+ dp_phy =<&dptx_phy>;
Shouldn't it be "samsung,dp_phy" ?
Do you mean this ? It is not working.
+ dp_phy = "samsung,dp_phy";
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Do you mean this ? It is not working.
+ dp_phy = "samsung,dp_phy";
I din't get this.
What do I need to change here?
As Tomasz Figa mentioned, it means that
-+ dp_phy =<&dptx_phy>;
++ samsung,dp_phy =<&dptx_phy>;
It is because this is a Samsung-specific property.
Best regards,
Jingoo Han