[PATCH v3 7/7] dt-bindings: drm/bridge: Update bindings for ADV7533

[PATCH v3 7/7] dt-bindings: drm/bridge: Update bindings for ADV7533

[Archit Taneja]

Add description of ADV7533. Add the required and optional properties that
are specific to it.

Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Signed-off-by: Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 .../bindings/display/bridge/adi,adv7511.txt        | 25 +++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 96c25ee..420da5a 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -1,13 +1,19 @@
-Analog Device ADV7511(W)/13 HDMI Encoders
+Analog Device ADV7511(W)/13/33 HDMI Encoders
 -----------------------------------------
 
-The ADV7511, ADV7511W and ADV7513 are HDMI audio and video transmitters
+The ADV7511, ADV7511W, ADV7513 and ADV7533 are HDMI audio and video transmitters
 compatible with HDMI 1.4 and DVI 1.0. They support color space conversion,
-S/PDIF, CEC and HDCP.
+S/PDIF, CEC and HDCP. ADV7533 supports the DSI interface for input pixels, while
+the others support RGB interface.
 
 Required properties:
 
-- compatible: Should be one of "adi,adv7511", "adi,adv7511w" or "adi,adv7513"
+- compatible: Should be one of:
+		"adi,adv7511"
+		"adi,adv7511w"
+		"adi,adv7513"
+		"adi,adv7533"
+
 - reg: I2C slave address
 
 The ADV7511 supports a large number of input data formats that differ by their
@@ -32,6 +38,11 @@ The following input format properties are required except in "rgb 1x" and
 - adi,input-justification: The input bit justification ("left", "evenly",
   "right").
 
+The following properties are required for ADV7533:
+
+- adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It should
+  be one of 1, 2, 3 or 4.
+
 Optional properties:
 
 - interrupts: Specifier for the ADV7511 interrupt
@@ -42,13 +53,17 @@ Optional properties:
 - adi,embedded-sync: The input uses synchronization signals embedded in the
   data stream (similar to BT.656). Defaults to separate H/V synchronization
   signals.
+- adi,disable-timing-generator: Only for ADV7533. Disables the internal timing
+  generator. The chip will rely on the sync signals in the DSI data lanes,
+  rather than generate its own timings for HDMI output.
 
 Required nodes:
 
 The ADV7511 has two video ports. Their connections are modelled using the OF
 graph bindings specified in Documentation/devicetree/bindings/graph.txt.
 
-- Video port 0 for the RGB or YUV input
+- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533, the
+  remote endpoint phandle should refer to a valid mipi_dsi_host device node.
 - Video port 1 for the HDMI output
 
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 7/7] dt-bindings: drm/bridge: Update bindings for ADV7533

[Rob Herring]

On Wed, Mar 09, 2016 at 04:27:18PM +0530, Archit Taneja wrote:
> Add description of ADV7533. Add the required and optional properties that
> are specific to it.
> 
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
> 
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
>  .../bindings/display/bridge/adi,adv7511.txt        | 25 +++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 7/7] dt-bindings: drm/bridge: Update bindings for ADV7533

[Laurent Pinchart]

Hi Archit,

Thank you for the patch.

On Wednesday 09 Mar 2016 16:27:18 Archit Taneja wrote:
> Add description of ADV7533. Add the required and optional properties that
> are specific to it.
> 
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
> 
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
>  .../bindings/display/bridge/adi,adv7511.txt        | 25 ++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index
> 96c25ee..420da5a 100644
> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> @@ -1,13 +1,19 @@
> -Analog Device ADV7511(W)/13 HDMI Encoders
> +Analog Device ADV7511(W)/13/33 HDMI Encoders
>  -----------------------------------------
> 
> -The ADV7511, ADV7511W and ADV7513 are HDMI audio and video transmitters
> +The ADV7511, ADV7511W, ADV7513 and ADV7533 are HDMI audio and video
> transmitters compatible with HDMI 1.4 and DVI 1.0. They support color space
> conversion, -S/PDIF, CEC and HDCP.
> +S/PDIF, CEC and HDCP. ADV7533 supports the DSI interface for input pixels,
> while +the others support RGB interface.
> 
>  Required properties:
> 
> -- compatible: Should be one of "adi,adv7511", "adi,adv7511w" or
> "adi,adv7513" +- compatible: Should be one of:
> +		"adi,adv7511"
> +		"adi,adv7511w"
> +		"adi,adv7513"
> +		"adi,adv7533"
> +
>  - reg: I2C slave address
> 
>  The ADV7511 supports a large number of input data formats that differ by
> their @@ -32,6 +38,11 @@ The following input format properties are required
> except in "rgb 1x" and - adi,input-justification: The input bit
> justification ("left", "evenly", "right").
> 
> +The following properties are required for ADV7533:
> +
> +- adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It
> should +  be one of 1, 2, 3 or 4.

Does the ADV7533 support data lanes remapping ?

>  Optional properties:
> 
>  - interrupts: Specifier for the ADV7511 interrupt
> @@ -42,13 +53,17 @@ Optional properties:
>  - adi,embedded-sync: The input uses synchronization signals embedded in the
> data stream (similar to BT.656). Defaults to separate H/V synchronization
> signals.
> +- adi,disable-timing-generator: Only for ADV7533. Disables the internal
> timing +  generator. The chip will rely on the sync signals in the DSI data
> lanes, +  rather than generate its own timings for HDMI output.

Isn't that something that should be selectable at runtime ?

>  Required nodes:
> 
>  The ADV7511 has two video ports. Their connections are modelled using the
> OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> 
> -- Video port 0 for the RGB or YUV input
> +- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533, the
> +  remote endpoint phandle should refer to a valid mipi_dsi_host device

Nitpicking, I would write "reference" instead of "refer to".

> node. - Video port 1 for the HDMI output

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 7/7] dt-bindings: drm/bridge: Update bindings for ADV7533

[Archit Taneja]

On 04/22/2016 04:02 AM, Laurent Pinchart wrote:
> Hi Archit,
>
> Thank you for the patch.
>
> On Wednesday 09 Mar 2016 16:27:18 Archit Taneja wrote:
>> Add description of ADV7533. Add the required and optional properties that
>> are specific to it.
>>
>> Cc: devicetree@vger.kernel.org
>> Cc: Rob Herring <robh@kernel.org>
>>
>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>> ---
>>   .../bindings/display/bridge/adi,adv7511.txt        | 25 ++++++++++++++-----
>>   1 file changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index
>> 96c25ee..420da5a 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>> @@ -1,13 +1,19 @@
>> -Analog Device ADV7511(W)/13 HDMI Encoders
>> +Analog Device ADV7511(W)/13/33 HDMI Encoders
>>   -----------------------------------------
>>
>> -The ADV7511, ADV7511W and ADV7513 are HDMI audio and video transmitters
>> +The ADV7511, ADV7511W, ADV7513 and ADV7533 are HDMI audio and video
>> transmitters compatible with HDMI 1.4 and DVI 1.0. They support color space
>> conversion, -S/PDIF, CEC and HDCP.
>> +S/PDIF, CEC and HDCP. ADV7533 supports the DSI interface for input pixels,
>> while +the others support RGB interface.
>>
>>   Required properties:
>>
>> -- compatible: Should be one of "adi,adv7511", "adi,adv7511w" or
>> "adi,adv7513" +- compatible: Should be one of:
>> +		"adi,adv7511"
>> +		"adi,adv7511w"
>> +		"adi,adv7513"
>> +		"adi,adv7533"
>> +
>>   - reg: I2C slave address
>>
>>   The ADV7511 supports a large number of input data formats that differ by
>> their @@ -32,6 +38,11 @@ The following input format properties are required
>> except in "rgb 1x" and - adi,input-justification: The input bit
>> justification ("left", "evenly", "right").
>>
>> +The following properties are required for ADV7533:
>> +
>> +- adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It
>> should +  be one of 1, 2, 3 or 4.
>
> Does the ADV7533 support data lanes remapping ?

It doesn't support remapping of lanes. There is only one register field
that allows us to select the number of lanes.

>
>>   Optional properties:
>>
>>   - interrupts: Specifier for the ADV7511 interrupt
>> @@ -42,13 +53,17 @@ Optional properties:
>>   - adi,embedded-sync: The input uses synchronization signals embedded in the
>> data stream (similar to BT.656). Defaults to separate H/V synchronization
>> signals.
>> +- adi,disable-timing-generator: Only for ADV7533. Disables the internal
>> timing +  generator. The chip will rely on the sync signals in the DSI data
>> lanes, +  rather than generate its own timings for HDMI output.
>
> Isn't that something that should be selectable at runtime ?

The timing generator can be enabled/disabled at runtime. Although, we
don't have a way to tell the driver whether we want to keep it enabled
or not.

It's a hardware feature that works well on most platforms, but not on
all. In particular, it works well on DB410c, but causes issues with
the Hikey 96 board. The DSI host on Hikey has different clock sources 
that generate the display controller's pixel clock and DSI byte clock,
whereas the Qualcomm SoC uses the same source. My guess is that the
ADV7533's timing generator doesn't like it when the pixel data and
clock are out of phase or something.

Since it is a hardware feature which needs tweaking, I thought it
qualified as a DT property.

>
>>   Required nodes:
>>
>>   The ADV7511 has two video ports. Their connections are modelled using the
>> OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>
>> -- Video port 0 for the RGB or YUV input
>> +- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533, the
>> +  remote endpoint phandle should refer to a valid mipi_dsi_host device
>
> Nitpicking, I would write "reference" instead of "refer to".

I'll fix this in the next revision.

Thanks,
Archit

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum, hosted by The Linux Foundation

[PATCH v4 7/7] dt-bindings: drm/bridge: Update bindings for ADV7533

[Archit Taneja]

Add description of ADV7533. Add the required and optional properties that
are specific to it.

Cc: devicetree@vger.kernel.org

Acked-by: Rob Herring <robh@kernel.org>

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 .../bindings/display/bridge/adi,adv7511.txt        | 26 +++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 96c25ee..6532a59 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -1,13 +1,19 @@
-Analog Device ADV7511(W)/13 HDMI Encoders
+Analog Device ADV7511(W)/13/33 HDMI Encoders
 -----------------------------------------
 
-The ADV7511, ADV7511W and ADV7513 are HDMI audio and video transmitters
+The ADV7511, ADV7511W, ADV7513 and ADV7533 are HDMI audio and video transmitters
 compatible with HDMI 1.4 and DVI 1.0. They support color space conversion,
-S/PDIF, CEC and HDCP.
+S/PDIF, CEC and HDCP. ADV7533 supports the DSI interface for input pixels, while
+the others support RGB interface.
 
 Required properties:
 
-- compatible: Should be one of "adi,adv7511", "adi,adv7511w" or "adi,adv7513"
+- compatible: Should be one of:
+		"adi,adv7511"
+		"adi,adv7511w"
+		"adi,adv7513"
+		"adi,adv7533"
+
 - reg: I2C slave address
 
 The ADV7511 supports a large number of input data formats that differ by their
@@ -32,6 +38,11 @@ The following input format properties are required except in "rgb 1x" and
 - adi,input-justification: The input bit justification ("left", "evenly",
   "right").
 
+The following properties are required for ADV7533:
+
+- adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It should
+  be one of 1, 2, 3 or 4.
+
 Optional properties:
 
 - interrupts: Specifier for the ADV7511 interrupt
@@ -42,13 +53,18 @@ Optional properties:
 - adi,embedded-sync: The input uses synchronization signals embedded in the
   data stream (similar to BT.656). Defaults to separate H/V synchronization
   signals.
+- adi,disable-timing-generator: Only for ADV7533. Disables the internal timing
+  generator. The chip will rely on the sync signals in the DSI data lanes,
+  rather than generate its own timings for HDMI output.
 
 Required nodes:
 
 The ADV7511 has two video ports. Their connections are modelled using the OF
 graph bindings specified in Documentation/devicetree/bindings/graph.txt.
 
-- Video port 0 for the RGB or YUV input
+- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533, the
+  remote endpoint phandle should be a reference to a valid mipi_dsi_host device
+  node.
 - Video port 1 for the HDMI output
 
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v3 7/7] dt-bindings: drm/bridge: Update bindings for ADV7533

[Laurent Pinchart]

Hi Archit,

On Friday 22 Apr 2016 11:10:18 Archit Taneja wrote:
> On 04/22/2016 04:02 AM, Laurent Pinchart wrote:
> > On Wednesday 09 Mar 2016 16:27:18 Archit Taneja wrote:
> >> Add description of ADV7533. Add the required and optional properties that
> >> are specific to it.
> >> 
> >> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >> 
> >> Signed-off-by: Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> >> ---
> >> 
> >> .../bindings/display/bridge/adi,adv7511.txt        | 25 ++++++++++++-----
> >> 1 file changed, 20 insertions(+), 5 deletions(-)
> >> 
> >> diff --git
> >> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> >> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index
> >> 96c25ee..420da5a 100644
> >> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> >> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt

[snip]

> >> +- adi,disable-timing-generator: Only for ADV7533. Disables the internal
> >> timing
> >> +  generator. The chip will rely on the sync signals in the DSI data
> >> lanes,
> >> +  rather than generate its own timings for HDMI output.
> > 
> > Isn't that something that should be selectable at runtime ?
> 
> The timing generator can be enabled/disabled at runtime. Although, we
> don't have a way to tell the driver whether we want to keep it enabled
> or not.
> 
> It's a hardware feature that works well on most platforms, but not on
> all. In particular, it works well on DB410c, but causes issues with
> the Hikey 96 board. The DSI host on Hikey has different clock sources
> that generate the display controller's pixel clock and DSI byte clock,
> whereas the Qualcomm SoC uses the same source. My guess is that the
> ADV7533's timing generator doesn't like it when the pixel data and
> clock are out of phase or something.
> 
> Since it is a hardware feature which needs tweaking, I thought it
> qualified as a DT property.

The fact that a hardware generator is present is certainly describes the 
hardware, but I'm not sure whether to enable it or not also qualifies as a 
hardware feature.

Are there use cases for using the timing generator conditionally on a given 
board ? As you implement support for disabling it, I assume it's not 
mandatory. What feature(s) do we lose if we keep it disabled ?

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 7/7] dt-bindings: drm/bridge: Update bindings for ADV7533

[Archit Taneja]

On 05/16/2016 05:31 PM, Laurent Pinchart wrote:
> Hi Archit,
>
> On Friday 22 Apr 2016 11:10:18 Archit Taneja wrote:
>> On 04/22/2016 04:02 AM, Laurent Pinchart wrote:
>>> On Wednesday 09 Mar 2016 16:27:18 Archit Taneja wrote:
>>>> Add description of ADV7533. Add the required and optional properties that
>>>> are specific to it.
>>>>
>>>> Cc: devicetree@vger.kernel.org
>>>> Cc: Rob Herring <robh@kernel.org>
>>>>
>>>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>>>> ---
>>>>
>>>> .../bindings/display/bridge/adi,adv7511.txt        | 25 ++++++++++++-----
>>>> 1 file changed, 20 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index
>>>> 96c25ee..420da5a 100644
>>>> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>
> [snip]
>
>>>> +- adi,disable-timing-generator: Only for ADV7533. Disables the internal
>>>> timing
>>>> +  generator. The chip will rely on the sync signals in the DSI data
>>>> lanes,
>>>> +  rather than generate its own timings for HDMI output.
>>>
>>> Isn't that something that should be selectable at runtime ?
>>
>> The timing generator can be enabled/disabled at runtime. Although, we
>> don't have a way to tell the driver whether we want to keep it enabled
>> or not.
>>
>> It's a hardware feature that works well on most platforms, but not on
>> all. In particular, it works well on DB410c, but causes issues with
>> the Hikey 96 board. The DSI host on Hikey has different clock sources
>> that generate the display controller's pixel clock and DSI byte clock,
>> whereas the Qualcomm SoC uses the same source. My guess is that the
>> ADV7533's timing generator doesn't like it when the pixel data and
>> clock are out of phase or something.
>>
>> Since it is a hardware feature which needs tweaking, I thought it
>> qualified as a DT property.
>
> The fact that a hardware generator is present is certainly describes the
> hardware, but I'm not sure whether to enable it or not also qualifies as a
> hardware feature.
>
> Are there use cases for using the timing generator conditionally on a given
> board ? As you implement support for disabling it, I assume it's not
> mandatory. What feature(s) do we lose if we keep it disabled ?
>

The spec says it's recommended to use the internal timing generator. In
the case of db410c, I observe an unstable output/flicker for certain
modes if I don't enable it.

In the case of hikey platform, it's the other way round.

Xinliang, could you describe the problems you face when the timing
generator is enabled?

Thanks,
Archit

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum, hosted by The Linux Foundation

Re: [PATCH v3 7/7] dt-bindings: drm/bridge: Update bindings for ADV7533

[Xinliang Liu]

On 17 May 2016 at 11:43, Archit Taneja <architt@codeaurora.org> wrote:
>
>
> On 05/16/2016 05:31 PM, Laurent Pinchart wrote:
>>
>> Hi Archit,
>>
>> On Friday 22 Apr 2016 11:10:18 Archit Taneja wrote:
>>>
>>> On 04/22/2016 04:02 AM, Laurent Pinchart wrote:
>>>>
>>>> On Wednesday 09 Mar 2016 16:27:18 Archit Taneja wrote:
>>>>>
>>>>> Add description of ADV7533. Add the required and optional properties
>>>>> that
>>>>> are specific to it.
>>>>>
>>>>> Cc: devicetree@vger.kernel.org
>>>>> Cc: Rob Herring <robh@kernel.org>
>>>>>
>>>>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>>>>> ---
>>>>>
>>>>> .../bindings/display/bridge/adi,adv7511.txt        | 25
>>>>> ++++++++++++-----
>>>>> 1 file changed, 20 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>> index
>>>>> 96c25ee..420da5a 100644
>>>>> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>
>>
>> [snip]
>>
>>>>> +- adi,disable-timing-generator: Only for ADV7533. Disables the
>>>>> internal
>>>>> timing
>>>>> +  generator. The chip will rely on the sync signals in the DSI data
>>>>> lanes,
>>>>> +  rather than generate its own timings for HDMI output.
>>>>
>>>>
>>>> Isn't that something that should be selectable at runtime ?
>>>
>>>
>>> The timing generator can be enabled/disabled at runtime. Although, we
>>> don't have a way to tell the driver whether we want to keep it enabled
>>> or not.
>>>
>>> It's a hardware feature that works well on most platforms, but not on
>>> all. In particular, it works well on DB410c, but causes issues with
>>> the Hikey 96 board. The DSI host on Hikey has different clock sources
>>> that generate the display controller's pixel clock and DSI byte clock,
>>> whereas the Qualcomm SoC uses the same source. My guess is that the
>>> ADV7533's timing generator doesn't like it when the pixel data and
>>> clock are out of phase or something.
>>>
>>> Since it is a hardware feature which needs tweaking, I thought it
>>> qualified as a DT property.
>>
>>
>> The fact that a hardware generator is present is certainly describes the
>> hardware, but I'm not sure whether to enable it or not also qualifies as a
>> hardware feature.
>>
>> Are there use cases for using the timing generator conditionally on a
>> given
>> board ? As you implement support for disabling it, I assume it's not
>> mandatory. What feature(s) do we lose if we keep it disabled ?
>>
>
> The spec says it's recommended to use the internal timing generator. In
> the case of db410c, I observe an unstable output/flicker for certain
> modes if I don't enable it.
>
> In the case of hikey platform, it's the other way round.
>
> Xinliang, could you describe the problems you face when the timing
> generator is enabled?

Yes,  opening the timing generator of ADV7533 can benefit the HDMI
output signal.
But, for some circumstances, we need to disable timing generator:
To make modes work, the timing parameters (hfp, hbp, etc.) used by
ADV7533 timing generator should match the ones used in DSI.
If the timing parameters changed in DSI and these changing  timing
parameters can't pass to ADV7533, then it need to disable the timing
generator of ADV7533 to make mode work.
Some modes in HiKey is in this case.

Thanks,
-xinliang

>
>
> 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

Re: [PATCH v3 7/7] dt-bindings: drm/bridge: Update bindings for ADV7533

[Archit Taneja]

On 05/17/2016 09:48 AM, Xinliang Liu wrote:
> On 17 May 2016 at 11:43, Archit Taneja <architt@codeaurora.org> wrote:
>>
>>
>> On 05/16/2016 05:31 PM, Laurent Pinchart wrote:
>>>
>>> Hi Archit,
>>>
>>> On Friday 22 Apr 2016 11:10:18 Archit Taneja wrote:
>>>>
>>>> On 04/22/2016 04:02 AM, Laurent Pinchart wrote:
>>>>>
>>>>> On Wednesday 09 Mar 2016 16:27:18 Archit Taneja wrote:
>>>>>>
>>>>>> Add description of ADV7533. Add the required and optional properties
>>>>>> that
>>>>>> are specific to it.
>>>>>>
>>>>>> Cc: devicetree@vger.kernel.org
>>>>>> Cc: Rob Herring <robh@kernel.org>
>>>>>>
>>>>>> Signed-off-by: Archit Taneja <architt@codeaurora.org>
>>>>>> ---
>>>>>>
>>>>>> .../bindings/display/bridge/adi,adv7511.txt        | 25
>>>>>> ++++++++++++-----
>>>>>> 1 file changed, 20 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>>> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>>> index
>>>>>> 96c25ee..420da5a 100644
>>>>>> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>>> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>
>>>
>>> [snip]
>>>
>>>>>> +- adi,disable-timing-generator: Only for ADV7533. Disables the
>>>>>> internal
>>>>>> timing
>>>>>> +  generator. The chip will rely on the sync signals in the DSI data
>>>>>> lanes,
>>>>>> +  rather than generate its own timings for HDMI output.
>>>>>
>>>>>
>>>>> Isn't that something that should be selectable at runtime ?
>>>>
>>>>
>>>> The timing generator can be enabled/disabled at runtime. Although, we
>>>> don't have a way to tell the driver whether we want to keep it enabled
>>>> or not.
>>>>
>>>> It's a hardware feature that works well on most platforms, but not on
>>>> all. In particular, it works well on DB410c, but causes issues with
>>>> the Hikey 96 board. The DSI host on Hikey has different clock sources
>>>> that generate the display controller's pixel clock and DSI byte clock,
>>>> whereas the Qualcomm SoC uses the same source. My guess is that the
>>>> ADV7533's timing generator doesn't like it when the pixel data and
>>>> clock are out of phase or something.
>>>>
>>>> Since it is a hardware feature which needs tweaking, I thought it
>>>> qualified as a DT property.
>>>
>>>
>>> The fact that a hardware generator is present is certainly describes the
>>> hardware, but I'm not sure whether to enable it or not also qualifies as a
>>> hardware feature.
>>>
>>> Are there use cases for using the timing generator conditionally on a
>>> given
>>> board ? As you implement support for disabling it, I assume it's not
>>> mandatory. What feature(s) do we lose if we keep it disabled ?
>>>
>>
>> The spec says it's recommended to use the internal timing generator. In
>> the case of db410c, I observe an unstable output/flicker for certain
>> modes if I don't enable it.
>>
>> In the case of hikey platform, it's the other way round.
>>
>> Xinliang, could you describe the problems you face when the timing
>> generator is enabled?
>
> Yes,  opening the timing generator of ADV7533 can benefit the HDMI
> output signal.
> But, for some circumstances, we need to disable timing generator:
> To make modes work, the timing parameters (hfp, hbp, etc.) used by
> ADV7533 timing generator should match the ones used in DSI.
> If the timing parameters changed in DSI and these changing  timing
> parameters can't pass to ADV7533, then it need to disable the timing
> generator of ADV7533 to make mode work.
> Some modes in HiKey is in this case.


The ADV7533 chip's DSI receiver supports the DSI Video mode
sub-mode: "Non-burst Mode with sync pulses". Ideally, a DSI
receiver supporting this mode should be able to reconstruct
the timings using packets embedded in the controller.

With DB410c, it seems to struggle without it, and with
Hikey, we need to explicitly disable it because, as I understand,
the DSI controller there tweaks some of the timings parameters
in the mode it wants to set.

It would have been ideal to not expose this knob in DT, but
we need it for ADV7533 to work across multiple platforms.

Laurent, do you have any more thoughts on this? I'd posted
out a v4 of this patch set. Please have a look when you get
the chance.

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

[PATCH v5 7/7] dt-bindings: drm/bridge: Update bindings for ADV7533

[Archit Taneja]

Add description of ADV7533. Add the required and optional properties that
are specific to it.

Cc: devicetree@vger.kernel.org

Acked-by: Rob Herring <robh@kernel.org>

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 .../bindings/display/bridge/adi,adv7511.txt        | 26 +++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 96c25ee..6532a59 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -1,13 +1,19 @@
-Analog Device ADV7511(W)/13 HDMI Encoders
+Analog Device ADV7511(W)/13/33 HDMI Encoders
 -----------------------------------------
 
-The ADV7511, ADV7511W and ADV7513 are HDMI audio and video transmitters
+The ADV7511, ADV7511W, ADV7513 and ADV7533 are HDMI audio and video transmitters
 compatible with HDMI 1.4 and DVI 1.0. They support color space conversion,
-S/PDIF, CEC and HDCP.
+S/PDIF, CEC and HDCP. ADV7533 supports the DSI interface for input pixels, while
+the others support RGB interface.
 
 Required properties:
 
-- compatible: Should be one of "adi,adv7511", "adi,adv7511w" or "adi,adv7513"
+- compatible: Should be one of:
+		"adi,adv7511"
+		"adi,adv7511w"
+		"adi,adv7513"
+		"adi,adv7533"
+
 - reg: I2C slave address
 
 The ADV7511 supports a large number of input data formats that differ by their
@@ -32,6 +38,11 @@ The following input format properties are required except in "rgb 1x" and
 - adi,input-justification: The input bit justification ("left", "evenly",
   "right").
 
+The following properties are required for ADV7533:
+
+- adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It should
+  be one of 1, 2, 3 or 4.
+
 Optional properties:
 
 - interrupts: Specifier for the ADV7511 interrupt
@@ -42,13 +53,18 @@ Optional properties:
 - adi,embedded-sync: The input uses synchronization signals embedded in the
   data stream (similar to BT.656). Defaults to separate H/V synchronization
   signals.
+- adi,disable-timing-generator: Only for ADV7533. Disables the internal timing
+  generator. The chip will rely on the sync signals in the DSI data lanes,
+  rather than generate its own timings for HDMI output.
 
 Required nodes:
 
 The ADV7511 has two video ports. Their connections are modelled using the OF
 graph bindings specified in Documentation/devicetree/bindings/graph.txt.
 
-- Video port 0 for the RGB or YUV input
+- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533, the
+  remote endpoint phandle should be a reference to a valid mipi_dsi_host device
+  node.
 - Video port 1 for the HDMI output
 
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v6 8/8] dt-bindings: drm/bridge: Update bindings for ADV7533

[Archit Taneja]

Add description of ADV7533. Add the required and optional properties that
are specific to it.

Cc: devicetree@vger.kernel.org

Acked-by: Rob Herring <robh@kernel.org>

Signed-off-by: Archit Taneja <architt@codeaurora.org>
---
 .../bindings/display/bridge/adi,adv7511.txt        | 26 +++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 96c25ee..6532a59 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -1,13 +1,19 @@
-Analog Device ADV7511(W)/13 HDMI Encoders
+Analog Device ADV7511(W)/13/33 HDMI Encoders
 -----------------------------------------
 
-The ADV7511, ADV7511W and ADV7513 are HDMI audio and video transmitters
+The ADV7511, ADV7511W, ADV7513 and ADV7533 are HDMI audio and video transmitters
 compatible with HDMI 1.4 and DVI 1.0. They support color space conversion,
-S/PDIF, CEC and HDCP.
+S/PDIF, CEC and HDCP. ADV7533 supports the DSI interface for input pixels, while
+the others support RGB interface.
 
 Required properties:
 
-- compatible: Should be one of "adi,adv7511", "adi,adv7511w" or "adi,adv7513"
+- compatible: Should be one of:
+		"adi,adv7511"
+		"adi,adv7511w"
+		"adi,adv7513"
+		"adi,adv7533"
+
 - reg: I2C slave address
 
 The ADV7511 supports a large number of input data formats that differ by their
@@ -32,6 +38,11 @@ The following input format properties are required except in "rgb 1x" and
 - adi,input-justification: The input bit justification ("left", "evenly",
   "right").
 
+The following properties are required for ADV7533:
+
+- adi,dsi-lanes: Number of DSI data lanes connected to the DSI host. It should
+  be one of 1, 2, 3 or 4.
+
 Optional properties:
 
 - interrupts: Specifier for the ADV7511 interrupt
@@ -42,13 +53,18 @@ Optional properties:
 - adi,embedded-sync: The input uses synchronization signals embedded in the
   data stream (similar to BT.656). Defaults to separate H/V synchronization
   signals.
+- adi,disable-timing-generator: Only for ADV7533. Disables the internal timing
+  generator. The chip will rely on the sync signals in the DSI data lanes,
+  rather than generate its own timings for HDMI output.
 
 Required nodes:
 
 The ADV7511 has two video ports. Their connections are modelled using the OF
 graph bindings specified in Documentation/devicetree/bindings/graph.txt.
 
-- Video port 0 for the RGB or YUV input
+- Video port 0 for the RGB, YUV or DSI input. In the case of ADV7533, the
+  remote endpoint phandle should be a reference to a valid mipi_dsi_host device
+  node.
 - Video port 1 for the HDMI output
 
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation