Re: [PATCH v5 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board

[Inki Dae]

2017ë 01ì 05ì 15:55ì Andrzej Hajda ì(ê) ì ê:
> On 04.01.2017 15:44, Rob Herring wrote:
>> On Wed, Jan 04, 2017 at 05:15:10PM +0900, Hoegeun Kwon wrote:
>>> This patch add support for MIPI-DSI based S6E3HA2 AMOLED panel
>>> driver. This panel has 1440x2560 resolution in 5.7-inch physical
>>> panel in the TM2 device.
>>>
>>> Signed-off-by: Donghwa Lee <dh09.lee@xxxxxxxxxxx>
>>> Signed-off-by: Hyungwon Hwang <human.hwang@xxxxxxxxxxx>
>>> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@xxxxxxxxxxx>
>>> ---
>>>  .../bindings/display/panel/samsung,s6e3ha2.txt     |  40 ++
>>>  drivers/gpu/drm/panel/Kconfig                      |   6 +
>>>  drivers/gpu/drm/panel/Makefile                     |   1 +
>>>  drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c      | 741 +++++++++++++++++++++
>>>  4 files changed, 788 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
>>>  create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
>>> new file mode 100644
>>> index 0000000..6879f51
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
>>> @@ -0,0 +1,40 @@
>>> +Samsung S6E3HA2 5.7" 1440x2560 AMOLED panel
>>> +
>>> +Required properties:
>>> +  - compatible: "samsung,s6e3ha2"
>>> +  - reg: the virtual channel number of a DSI peripheral
>>> +  - vdd3-supply: I/O voltage supply
>>> +  - vci-supply: voltage supply for analog circuits
>>> +  - reset-gpios: a GPIO spec for the reset pin (active low)
>>> +  - enable-gpios: a GPIO spec for the panel enable pin (active high)
>>> +  - te-gpios: a GPIO spec for the tearing effect synchronization signal
>>> +    gpio pin (active high)
>>> +
>>> +The device node can contain one 'port' child node with one child
>>> +'endpoint' node, according to the bindings defined in [1]. This
>>> +node should describe panel's video bus.
>>> +
>>> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
>>> +
>>> +Example:
>>> +
>>> +&dsi {
>>> +	...
>>> +
>>> +	panel@0 {
>>> +		compatible = "samsung,s6e3ha2";
>>> +		reg = <0>;
>>> +		vdd3-supply = <&ldo27_reg>;
>>> +		vci-supply = <&ldo28_reg>;
>>> +		reset-gpios = <&gpg0 0 GPIO_ACTIVE_LOW>;
>>> +		enable-gpios = <&gpf1 5 GPIO_ACTIVE_HIGH>;
>>> +		te-gpios = <&gpf1 3 GPIO_ACTIVE_HIGH>;
>>> +
>>> +		port {
>>> +			panel_in: endpoint {
>>> +				remote-endpoint = <&dsi_out>;
>> As I said previously, it makes no sense to have a graph to dsi_out it is 
>> simply the parent node.
> 
> The problem is that exynos_dsi requires presence of endpoint node, when
> it was written the policy was that graphs must be always present.
> DSI reads from this node samsung,burst-clock-frequency and
> samsung,esc-clock-frequency. For example in exynos4412-trats2.dts:
> 
>>         dsi_0: dsi@11C80000 {
>>                 ...
>>                 ports {
>>                         #address-cells = <1>;
>>                         #size-cells = <0>;
>>  
>>                         port@1 {
>>                                 reg = <1>;
>>
>>                                 dsi_out: endpoint {
>>                                         remote-endpoint = <&dsi_in>;
>>                                         samsung,burst-clock-frequency
>> = <500000000>;
>>                                         samsung,esc-clock-frequency =
>> <20000000>;
>>                                 };
>>                         };
>>                 };
>>         
>>                 panel@0 {
>>                         ...
>>                         port {
>>                                 dsi_in: endpoint {
>>                                         remote-endpoint = <&dsi_out>;
>>                                 };
>>                         };
>>                 };
>>         };
> 
> However, DSI driver does not use remote-endpoint property, it is here
> only to fulfill of_graph policy.
> So if something like below is acceptable, we can get rid of port node in
> panel:
> 
>>         dsi_0: dsi@11C80000 {
>>                 ...
>>                 ports {
>>                         #address-cells = <1>;
>>                         #size-cells = <0>;
>>  
>>                         port@1 {
>>                                 reg = <1>;
>>
>>                                 dsi_out: endpoint {
>>                                         samsung,burst-clock-frequency
>> = <500000000>;
>>                                         samsung,esc-clock-frequency =
>> <20000000>;
>>                                 };
>>                         };
>>                 };
>>         
>>                 panel@0 {
>>                         ...
>>                 };
>>         };
> 
> What do you think?
> 
> Other solution is to move problematic properties somewhere else, but
> this require change of bindings.
> Anyway I would be glad to remove port nodes in other samsung panels:
> s6e8aa0, ld9040.

In addition,

Now dsi and mic device nodes of exynos5433.dtsi include remote nodes which define Display pipeline. This is wrong.
Display pipeline is specific to board, not SoC so these things should be moved to exynos5433-tm2.dts file.

Hoegeun, you can do this with other patch also.

Thanks.

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