[PATCH 0/3] Add support rockchip RGB output interface

[PATCH 0/3] Add support rockchip RGB output interface

[Sandy Huang]

This patches add support some rockchip Soc like rv1108, the VOP can directly
output parallel and serial RGB data to panel or to conversion chip. So we add
this driver to probe encoder and connector to support this case.

Sandy Huang (3):
  dt-bindings: Add document for rockchip RGB output interface
  drm/rockchip: Add support for Rockchip Soc RGB output interface
  drm/rockchip: vop: Add more RGB output interface type

 .../bindings/display/rockchip/rockchip-rgb.txt     |  80 +++++
 drivers/gpu/drm/rockchip/Kconfig                   |   9 +
 drivers/gpu/drm/rockchip/Makefile                  |   1 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c        |   2 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h        |   1 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h        |   2 +
 drivers/gpu/drm/rockchip/rockchip_rgb.c            | 327 +++++++++++++++++++++
 7 files changed, 422 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_rgb.c

-- 
2.7.4

[PATCH 1/3] dt-bindings: Add document for rockchip RGB output interface

[Sandy Huang]

This path add support rv1108 rgb output interface driver.

Signed-off-by: Sandy Huang <hjc@rock-chips.com>
---
 .../bindings/display/rockchip/rockchip-rgb.txt     | 80 ++++++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
new file mode 100644
index 0000000..4164512
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
@@ -0,0 +1,80 @@
+Rockchip RV1108 RGB interface
+================================
+
+Required properties:
+- compatible: matching the soc type:
+	- "rockchip,rv1108-rgb";
+
+Optional properties:
+- pinctrl-names: must contain a "lcdc" entry.
+- pinctrl-0: pin control group to be used for this interface.
+
+Required nodes:
+- rockchip,rgb-mode: should be "p888", "p666", "p565", "s888", "s888-dummy"
+	- p888: output r8-g8-b8 at each dclk cycle for per-pixel
+	- p666: output r6-g6-b6 at each dclk cycle for per-pixel
+	- p565: output r5-g6-b5 at each dclk cycle for per-pixel
+	- s888: output r8-g8-b8 in three dclk cycle for per-pixel
+	- s888-dmmy: output r8-g8-b8-dummy in four dclk cycle for per-pixel
+
+The rgb has two video ports described by:
+	Documentation/devicetree/bindings/media/video-interfaces.txt
+Their connections are modeled using the OF graph bindings specified in
+	Documentation/devicetree/bindings/graph.txt.
+
+- video port 0 for the VOP input, the remote endpoint maybe vopb/vopl/vop
+- video port 1 for either a panel or subsequent encoder
+
+the panel described by:
+	Documentation/devicetree/bindings/display/panel/simple-panel.txt
+Panel other required properties:
+- ports for remote rgb output.
+
+Example:
+
+panel: panel {
+	compatible = "auo,b101ean01";
+	enable-gpios = <&gpio7 21 GPIO_ACTIVE_HIGH>;
+
+	ports {
+		panel_in_rgb: endpoint {
+			remote-endpoint = <&rgb_out_panel>;
+		};
+	};
+};
+
+For Rockchip RV1108:
+
+	rgb: rgb {
+		compatible = "rockchip,rv1108-rgb";
+		pinctrl-names = "lcdc";
+		pinctrl-0 = <&lcdc_ctl>;
+		rockchip,rgb-mode = "p888";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			rgb_in: port at 0 {
+				reg = <0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				rgb_in_vop: endpoint at 0 {
+					reg = <0>;
+					remote-endpoint = <&vop_out_rgb>;
+				};
+			};
+
+			rgb_out: port at 1 {
+				reg = <1>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				rgb_out_panel: endpoint at 0 {
+					reg = <0>;
+					remote-endpoint = <&panel_in_rgb>;
+				};
+			};
+		};
+	};
-- 
2.7.4

[PATCH 1/3] dt-bindings: Add document for rockchip RGB output interface

[Rob Herring]

On Thu, Sep 14, 2017 at 11:43:18AM +0800, Sandy Huang wrote:
> This path add support rv1108 rgb output interface driver.
> 
> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
> ---
>  .../bindings/display/rockchip/rockchip-rgb.txt     | 80 ++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
> new file mode 100644
> index 0000000..4164512
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
> @@ -0,0 +1,80 @@
> +Rockchip RV1108 RGB interface
> +================================
> +
> +Required properties:
> +- compatible: matching the soc type:
> +	- "rockchip,rv1108-rgb";
> +
> +Optional properties:
> +- pinctrl-names: must contain a "lcdc" entry.
> +- pinctrl-0: pin control group to be used for this interface.
> +
> +Required nodes:
> +- rockchip,rgb-mode: should be "p888", "p666", "p565", "s888", "s888-dummy"

This should be a standard property. Any device with a parallel interface 
is going to need something like this.

> +	- p888: output r8-g8-b8 at each dclk cycle for per-pixel
> +	- p666: output r6-g6-b6 at each dclk cycle for per-pixel
> +	- p565: output r5-g6-b5 at each dclk cycle for per-pixel
> +	- s888: output r8-g8-b8 in three dclk cycle for per-pixel
> +	- s888-dmmy: output r8-g8-b8-dummy in four dclk cycle for per-pixel
> +
> +The rgb has two video ports described by:
> +	Documentation/devicetree/bindings/media/video-interfaces.txt
> +Their connections are modeled using the OF graph bindings specified in
> +	Documentation/devicetree/bindings/graph.txt.
> +
> +- video port 0 for the VOP input, the remote endpoint maybe vopb/vopl/vop
> +- video port 1 for either a panel or subsequent encoder
> +
> +the panel described by:
> +	Documentation/devicetree/bindings/display/panel/simple-panel.txt
> +Panel other required properties:
> +- ports for remote rgb output.
> +
> +Example:
> +
> +panel: panel {
> +	compatible = "auo,b101ean01";
> +	enable-gpios = <&gpio7 21 GPIO_ACTIVE_HIGH>;
> +
> +	ports {
> +		panel_in_rgb: endpoint {
> +			remote-endpoint = <&rgb_out_panel>;
> +		};
> +	};
> +};
> +
> +For Rockchip RV1108:
> +
> +	rgb: rgb {
> +		compatible = "rockchip,rv1108-rgb";
> +		pinctrl-names = "lcdc";
> +		pinctrl-0 = <&lcdc_ctl>;
> +		rockchip,rgb-mode = "p888";
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			rgb_in: port at 0 {
> +				reg = <0>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				rgb_in_vop: endpoint at 0 {
> +					reg = <0>;

Don't need reg for a single endpoint.

> +					remote-endpoint = <&vop_out_rgb>;
> +				};
> +			};
> +
> +			rgb_out: port at 1 {
> +				reg = <1>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				rgb_out_panel: endpoint at 0 {
> +					reg = <0>;

ditto.

> +					remote-endpoint = <&panel_in_rgb>;
> +				};
> +			};
> +		};
> +	};
> -- 
> 2.7.4
> 
> 

[PATCH 1/3] dt-bindings: Add document for rockchip RGB output interface

[Sandy Huang]

Hi rob,
     thanks for you review.

? 2017/9/19 22:46, Rob Herring ??:
> On Thu, Sep 14, 2017 at 11:43:18AM +0800, Sandy Huang wrote:
>> This path add support rv1108 rgb output interface driver.
>>
>> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
>> ---
>>   .../bindings/display/rockchip/rockchip-rgb.txt     | 80 ++++++++++++++++++++++
>>   1 file changed, 80 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>> new file mode 100644
>> index 0000000..4164512
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>> @@ -0,0 +1,80 @@
>> +Rockchip RV1108 RGB interface
>> +================================
>> +
>> +Required properties:
>> +- compatible: matching the soc type:
>> +	- "rockchip,rv1108-rgb";
>> +
>> +Optional properties:
>> +- pinctrl-names: must contain a "lcdc" entry.
>> +- pinctrl-0: pin control group to be used for this interface.
>> +
>> +Required nodes:
>> +- rockchip,rgb-mode: should be "p888", "p666", "p565", "s888", "s888-dummy"
> 
> This should be a standard property. Any device with a parallel interface
> is going to need something like this.
> 
so, i need to move this property to panel? or just rename 
rockchip,rgb-mode to rgb-mode?

>> +	- p888: output r8-g8-b8 at each dclk cycle for per-pixel
>> +	- p666: output r6-g6-b6 at each dclk cycle for per-pixel
>> +	- p565: output r5-g6-b5 at each dclk cycle for per-pixel
>> +	- s888: output r8-g8-b8 in three dclk cycle for per-pixel
>> +	- s888-dmmy: output r8-g8-b8-dummy in four dclk cycle for per-pixel
>> +
>> +The rgb has two video ports described by:
>> +	Documentation/devicetree/bindings/media/video-interfaces.txt
>> +Their connections are modeled using the OF graph bindings specified in
>> +	Documentation/devicetree/bindings/graph.txt.
>> +
>> +- video port 0 for the VOP input, the remote endpoint maybe vopb/vopl/vop
>> +- video port 1 for either a panel or subsequent encoder
>> +
>> +the panel described by:
>> +	Documentation/devicetree/bindings/display/panel/simple-panel.txt
>> +Panel other required properties:
>> +- ports for remote rgb output.
>> +
>> +Example:
>> +
>> +panel: panel {
>> +	compatible = "auo,b101ean01";
>> +	enable-gpios = <&gpio7 21 GPIO_ACTIVE_HIGH>;
>> +
>> +	ports {
>> +		panel_in_rgb: endpoint {
>> +			remote-endpoint = <&rgb_out_panel>;
>> +		};
>> +	};
>> +};
>> +
>> +For Rockchip RV1108:
>> +
>> +	rgb: rgb {
>> +		compatible = "rockchip,rv1108-rgb";
>> +		pinctrl-names = "lcdc";
>> +		pinctrl-0 = <&lcdc_ctl>;
>> +		rockchip,rgb-mode = "p888";
>> +
>> +		ports {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			rgb_in: port at 0 {
>> +				reg = <0>;
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +
>> +				rgb_in_vop: endpoint at 0 {
>> +					reg = <0>;
> 
> Don't need reg for a single endpoint.
> 
ok, this will be deleted at next version.
>> +					remote-endpoint = <&vop_out_rgb>;
>> +				};
>> +			};
>> +
>> +			rgb_out: port at 1 {
>> +				reg = <1>;
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +
>> +				rgb_out_panel: endpoint at 0 {
>> +					reg = <0>;
> 
> ditto.
> 
ok, this will be deleted at next version.
>> +					remote-endpoint = <&panel_in_rgb>;
>> +				};
>> +			};
>> +		};
>> +	};
>> -- 
>> 2.7.4
>>
>>
> 
> 
> 

[PATCH 1/3] dt-bindings: Add document for rockchip RGB output interface

[Sandy Huang]

? 2017/9/20 9:51, Sandy Huang ??:
> Hi rob,
>  ??? thanks for you review.
> 
> ? 2017/9/19 22:46, Rob Herring ??:
>> On Thu, Sep 14, 2017 at 11:43:18AM +0800, Sandy Huang wrote:
>>> This path add support rv1108 rgb output interface driver.
>>>
>>> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
>>> ---
>>> ? .../bindings/display/rockchip/rockchip-rgb.txt???? | 80 
>>> ++++++++++++++++++++++
>>> ? 1 file changed, 80 insertions(+)
>>> ? create mode 100644 
>>> Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt 
>>> b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>>> new file mode 100644
>>> index 0000000..4164512
>>> --- /dev/null
>>> +++ 
>>> b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>>> @@ -0,0 +1,80 @@
>>> +Rockchip RV1108 RGB interface
>>> +================================
>>> +
>>> +Required properties:
>>> +- compatible: matching the soc type:
>>> +??? - "rockchip,rv1108-rgb";
>>> +
>>> +Optional properties:
>>> +- pinctrl-names: must contain a "lcdc" entry.
>>> +- pinctrl-0: pin control group to be used for this interface.
>>> +
>>> +Required nodes:
>>> +- rockchip,rgb-mode: should be "p888", "p666", "p565", "s888", 
>>> "s888-dummy"
>>
>> This should be a standard property. Any device with a parallel interface
>> is going to need something like this.
>>
> so, i need to move this property to panel? or just rename 
> rockchip,rgb-mode to rgb-mode?
> 
>>> +??? - p888: output r8-g8-b8 at each dclk cycle for per-pixel
>>> +??? - p666: output r6-g6-b6 at each dclk cycle for per-pixel
>>> +??? - p565: output r5-g6-b5 at each dclk cycle for per-pixel
>>> +??? - s888: output r8-g8-b8 in three dclk cycle for per-pixel
>>> +??? - s888-dmmy: output r8-g8-b8-dummy in four dclk cycle for per-pixel
>>> +
>>> +The rgb has two video ports described by:
>>> +??? Documentation/devicetree/bindings/media/video-interfaces.txt
>>> +Their connections are modeled using the OF graph bindings specified in
>>> +??? Documentation/devicetree/bindings/graph.txt.
>>> +
>>> +- video port 0 for the VOP input, the remote endpoint maybe 
>>> vopb/vopl/vop
>>> +- video port 1 for either a panel or subsequent encoder
>>> +
>>> +the panel described by:
>>> +??? Documentation/devicetree/bindings/display/panel/simple-panel.txt
>>> +Panel other required properties:
>>> +- ports for remote rgb output.
>>> +
>>> +Example:
>>> +
>>> +panel: panel {
>>> +??? compatible = "auo,b101ean01";
>>> +??? enable-gpios = <&gpio7 21 GPIO_ACTIVE_HIGH>;
>>> +
>>> +??? ports {
>>> +??????? panel_in_rgb: endpoint {
>>> +??????????? remote-endpoint = <&rgb_out_panel>;
>>> +??????? };
>>> +??? };
>>> +};
>>> +
>>> +For Rockchip RV1108:
>>> +
>>> +??? rgb: rgb {
>>> +??????? compatible = "rockchip,rv1108-rgb";
>>> +??????? pinctrl-names = "lcdc";
>>> +??????? pinctrl-0 = <&lcdc_ctl>;
>>> +??????? rockchip,rgb-mode = "p888";
>>> +
>>> +??????? ports {
>>> +??????????? #address-cells = <1>;
>>> +??????????? #size-cells = <0>;
>>> +
>>> +??????????? rgb_in: port at 0 {
>>> +??????????????? reg = <0>;
>>> +??????????????? #address-cells = <1>;
>>> +??????????????? #size-cells = <0>;
>>> +
>>> +??????????????? rgb_in_vop: endpoint at 0 {
>>> +??????????????????? reg = <0>;
>>
>> Don't need reg for a single endpoint.
>>
> ok, this will be deleted at next version.
>>> +??????????????????? remote-endpoint = <&vop_out_rgb>;
>>> +??????????????? };
>>> +??????????? };
>>> +
>>> +??????????? rgb_out: port at 1 {
>>> +??????????????? reg = <1>;
>>> +??????????????? #address-cells = <1>;
>>> +??????????????? #size-cells = <0>;
>>> +
>>> +??????????????? rgb_out_panel: endpoint at 0 {
>>> +??????????????????? reg = <0>;
>>
>> ditto.
>>
> ok, this will be deleted at next version.
sorry,this can't be deleted, because rgb output remote endpoint maybe 
panel or convert chip, the dts node maybe like this:

panel: panel {
	status = "disabled";
	ports {
		panel_in_rgb: endpoint {
			remote-endpoint =  <&rgb_out_panel>;
		};
	};
};

bridge: bridge {
	status = "okay";
	ports {
		bridge_in_rgb: endpoint {
			remote-endpoint =  <&rgb_out_bridge>;
		};
	};
};

rgb_out: port at 1 {
	reg = <1>;
	#address-cells = <1>;
	#size-cells = <0>;
	rgb_out_panel: endpoint at 0 {
		 reg = <0>;
		remote-endpoint = <&panel_in_rgb>;
	};
	rgb_out_bridge: endpoint at 1 {
		 reg = <1>;
		remote-endpoint = <&bridge_in_rgb>;
	};
  };

so rockchip_rgb_bind() @ rockchip_rgb.c use reg id to find the remote 
actived endpoint.

for_each_child_of_node(port, endpoint) {
	of_property_read_u32(endpoint, "reg", &endpoint_id);
	ret = drm_of_find_panel_or_bridge(dev->of_node, 1, endpoint_id, 
&rgb->panel, &rgb->bridge);
	if (!ret)
		break;
}

>>> +??????????????????? remote-endpoint = <&panel_in_rgb>;
>>> +??????????????? };
>>> +??????????? };
>>> +??????? };
>>> +??? };
>>> -- 
>>> 2.7.4
>>>
>>>
>>
>>
>>
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 1/3] dt-bindings: Add document for rockchip RGB output interface

[Rob Herring]

On Tue, Sep 19, 2017 at 9:57 PM, Sandy Huang <hjc@rock-chips.com> wrote:
>
>
> ? 2017/9/20 9:51, Sandy Huang ??:
>>
>> Hi rob,
>>      thanks for you review.
>>
>> ? 2017/9/19 22:46, Rob Herring ??:
>>>
>>> On Thu, Sep 14, 2017 at 11:43:18AM +0800, Sandy Huang wrote:
>>>>
>>>> This path add support rv1108 rgb output interface driver.
>>>>
>>>> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
>>>> ---
>>>>   .../bindings/display/rockchip/rockchip-rgb.txt     | 80
>>>> ++++++++++++++++++++++
>>>>   1 file changed, 80 insertions(+)
>>>>   create mode 100644
>>>> Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>>>> b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>>>> new file mode 100644
>>>> index 0000000..4164512
>>>> --- /dev/null
>>>> +++
>>>> b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>>>> @@ -0,0 +1,80 @@
>>>> +Rockchip RV1108 RGB interface
>>>> +================================
>>>> +
>>>> +Required properties:
>>>> +- compatible: matching the soc type:
>>>> +    - "rockchip,rv1108-rgb";
>>>> +
>>>> +Optional properties:
>>>> +- pinctrl-names: must contain a "lcdc" entry.
>>>> +- pinctrl-0: pin control group to be used for this interface.
>>>> +
>>>> +Required nodes:
>>>> +- rockchip,rgb-mode: should be "p888", "p666", "p565", "s888",
>>>> "s888-dummy"
>>>
>>>
>>> This should be a standard property. Any device with a parallel interface
>>> is going to need something like this.
>>>
>> so, i need to move this property to panel? or just rename
>> rockchip,rgb-mode to rgb-mode?
>>
>>>> +    - p888: output r8-g8-b8 at each dclk cycle for per-pixel
>>>> +    - p666: output r6-g6-b6 at each dclk cycle for per-pixel
>>>> +    - p565: output r5-g6-b5 at each dclk cycle for per-pixel
>>>> +    - s888: output r8-g8-b8 in three dclk cycle for per-pixel
>>>> +    - s888-dmmy: output r8-g8-b8-dummy in four dclk cycle for per-pixel
>>>> +
>>>> +The rgb has two video ports described by:
>>>> +    Documentation/devicetree/bindings/media/video-interfaces.txt
>>>> +Their connections are modeled using the OF graph bindings specified in
>>>> +    Documentation/devicetree/bindings/graph.txt.
>>>> +
>>>> +- video port 0 for the VOP input, the remote endpoint maybe
>>>> vopb/vopl/vop
>>>> +- video port 1 for either a panel or subsequent encoder
>>>> +
>>>> +the panel described by:
>>>> +    Documentation/devicetree/bindings/display/panel/simple-panel.txt
>>>> +Panel other required properties:
>>>> +- ports for remote rgb output.
>>>> +
>>>> +Example:
>>>> +
>>>> +panel: panel {
>>>> +    compatible = "auo,b101ean01";
>>>> +    enable-gpios = <&gpio7 21 GPIO_ACTIVE_HIGH>;
>>>> +
>>>> +    ports {
>>>> +        panel_in_rgb: endpoint {
>>>> +            remote-endpoint = <&rgb_out_panel>;
>>>> +        };
>>>> +    };
>>>> +};
>>>> +
>>>> +For Rockchip RV1108:
>>>> +
>>>> +    rgb: rgb {
>>>> +        compatible = "rockchip,rv1108-rgb";
>>>> +        pinctrl-names = "lcdc";
>>>> +        pinctrl-0 = <&lcdc_ctl>;
>>>> +        rockchip,rgb-mode = "p888";
>>>> +
>>>> +        ports {
>>>> +            #address-cells = <1>;
>>>> +            #size-cells = <0>;
>>>> +
>>>> +            rgb_in: port at 0 {
>>>> +                reg = <0>;
>>>> +                #address-cells = <1>;
>>>> +                #size-cells = <0>;
>>>> +
>>>> +                rgb_in_vop: endpoint at 0 {
>>>> +                    reg = <0>;
>>>
>>>
>>> Don't need reg for a single endpoint.
>>>
>> ok, this will be deleted at next version.
>>>>
>>>> +                    remote-endpoint = <&vop_out_rgb>;
>>>> +                };
>>>> +            };
>>>> +
>>>> +            rgb_out: port at 1 {
>>>> +                reg = <1>;
>>>> +                #address-cells = <1>;
>>>> +                #size-cells = <0>;
>>>> +
>>>> +                rgb_out_panel: endpoint at 0 {
>>>> +                    reg = <0>;
>>>
>>>
>>> ditto.
>>>
>> ok, this will be deleted at next version.
>
> sorry,this can't be deleted, because rgb output remote endpoint maybe panel
> or convert chip, the dts node maybe like this:
>
> panel: panel {
>         status = "disabled";
>         ports {
>                 panel_in_rgb: endpoint {
>                         remote-endpoint =  <&rgb_out_panel>;
>                 };
>         };
> };
>
> bridge: bridge {
>         status = "okay";
>         ports {
>                 bridge_in_rgb: endpoint {
>                         remote-endpoint =  <&rgb_out_bridge>;
>                 };
>         };
> };
>
> rgb_out: port at 1 {
>         reg = <1>;
>         #address-cells = <1>;
>         #size-cells = <0>;
>         rgb_out_panel: endpoint at 0 {
>                  reg = <0>;
>                 remote-endpoint = <&panel_in_rgb>;
>         };
>         rgb_out_bridge: endpoint at 1 {
>                  reg = <1>;
>                 remote-endpoint = <&bridge_in_rgb>;
>         };
>  };
>
> so rockchip_rgb_bind() @ rockchip_rgb.c use reg id to find the remote
> actived endpoint.
>
> for_each_child_of_node(port, endpoint) {
>         of_property_read_u32(endpoint, "reg", &endpoint_id);

Lack of reg property here should imply 0 for endpoint_id.

>         ret = drm_of_find_panel_or_bridge(dev->of_node, 1, endpoint_id,
> &rgb->panel, &rgb->bridge);
>         if (!ret)
>                 break;
> }
>
>>>> +                    remote-endpoint = <&panel_in_rgb>;
>>>> +                };
>>>> +            };
>>>> +        };
>>>> +    };
>>>> --
>>>> 2.7.4
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>

[PATCH 1/3] dt-bindings: Add document for rockchip RGB output interface

[Sandy Huang]

Hi rob,

? 2017/9/22 3:40, Rob Herring ??:
> On Tue, Sep 19, 2017 at 9:57 PM, Sandy Huang <hjc@rock-chips.com> wrote:
>>
>>
>> ? 2017/9/20 9:51, Sandy Huang ??:
>>>
>>> Hi rob,
>>>       thanks for you review.
>>>
>>> ? 2017/9/19 22:46, Rob Herring ??:
>>>>
>>>> On Thu, Sep 14, 2017 at 11:43:18AM +0800, Sandy Huang wrote:
>>>>>
>>>>> This path add support rv1108 rgb output interface driver.
>>>>>
>>>>> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
>>>>> ---
>>>>>    .../bindings/display/rockchip/rockchip-rgb.txt     | 80
>>>>> ++++++++++++++++++++++
>>>>>    1 file changed, 80 insertions(+)
>>>>>    create mode 100644
>>>>> Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>>>>> b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>>>>> new file mode 100644
>>>>> index 0000000..4164512
>>>>> --- /dev/null
>>>>> +++
>>>>> b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
>>>>> @@ -0,0 +1,80 @@
>>>>> +Rockchip RV1108 RGB interface
>>>>> +================================
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible: matching the soc type:
>>>>> +    - "rockchip,rv1108-rgb";
>>>>> +
>>>>> +Optional properties:
>>>>> +- pinctrl-names: must contain a "lcdc" entry.
>>>>> +- pinctrl-0: pin control group to be used for this interface.
>>>>> +
>>>>> +Required nodes:
>>>>> +- rockchip,rgb-mode: should be "p888", "p666", "p565", "s888",
>>>>> "s888-dummy"
>>>>
>>>>
>>>> This should be a standard property. Any device with a parallel interface
>>>> is going to need something like this.
>>>>
>>> so, i need to move this property to panel? or just rename
>>> rockchip,rgb-mode to rgb-mode?
>>>
>>>>> +    - p888: output r8-g8-b8 at each dclk cycle for per-pixel
>>>>> +    - p666: output r6-g6-b6 at each dclk cycle for per-pixel
>>>>> +    - p565: output r5-g6-b5 at each dclk cycle for per-pixel
>>>>> +    - s888: output r8-g8-b8 in three dclk cycle for per-pixel
>>>>> +    - s888-dmmy: output r8-g8-b8-dummy in four dclk cycle for per-pixel
>>>>> +
>>>>> +The rgb has two video ports described by:
>>>>> +    Documentation/devicetree/bindings/media/video-interfaces.txt
>>>>> +Their connections are modeled using the OF graph bindings specified in
>>>>> +    Documentation/devicetree/bindings/graph.txt.
>>>>> +
>>>>> +- video port 0 for the VOP input, the remote endpoint maybe
>>>>> vopb/vopl/vop
>>>>> +- video port 1 for either a panel or subsequent encoder
>>>>> +
>>>>> +the panel described by:
>>>>> +    Documentation/devicetree/bindings/display/panel/simple-panel.txt
>>>>> +Panel other required properties:
>>>>> +- ports for remote rgb output.
>>>>> +
>>>>> +Example:
>>>>> +
>>>>> +panel: panel {
>>>>> +    compatible = "auo,b101ean01";
>>>>> +    enable-gpios = <&gpio7 21 GPIO_ACTIVE_HIGH>;
>>>>> +
>>>>> +    ports {
>>>>> +        panel_in_rgb: endpoint {
>>>>> +            remote-endpoint = <&rgb_out_panel>;
>>>>> +        };
>>>>> +    };
>>>>> +};
>>>>> +
>>>>> +For Rockchip RV1108:
>>>>> +
>>>>> +    rgb: rgb {
>>>>> +        compatible = "rockchip,rv1108-rgb";
>>>>> +        pinctrl-names = "lcdc";
>>>>> +        pinctrl-0 = <&lcdc_ctl>;
>>>>> +        rockchip,rgb-mode = "p888";
>>>>> +
>>>>> +        ports {
>>>>> +            #address-cells = <1>;
>>>>> +            #size-cells = <0>;
>>>>> +
>>>>> +            rgb_in: port at 0 {
>>>>> +                reg = <0>;
>>>>> +                #address-cells = <1>;
>>>>> +                #size-cells = <0>;
>>>>> +
>>>>> +                rgb_in_vop: endpoint at 0 {
>>>>> +                    reg = <0>;
>>>>
>>>>
>>>> Don't need reg for a single endpoint.
>>>>
>>> ok, this will be deleted at next version.
>>>>>
>>>>> +                    remote-endpoint = <&vop_out_rgb>;
>>>>> +                };
>>>>> +            };
>>>>> +
>>>>> +            rgb_out: port at 1 {
>>>>> +                reg = <1>;
>>>>> +                #address-cells = <1>;
>>>>> +                #size-cells = <0>;
>>>>> +
>>>>> +                rgb_out_panel: endpoint at 0 {
>>>>> +                    reg = <0>;
>>>>
>>>>
>>>> ditto.
>>>>
>>> ok, this will be deleted at next version.
>>
>> sorry,this can't be deleted, because rgb output remote endpoint maybe panel
>> or convert chip, the dts node maybe like this:
>>
>> panel: panel {
>>          status = "disabled";
>>          ports {
>>                  panel_in_rgb: endpoint {
>>                          remote-endpoint =  <&rgb_out_panel>;
>>                  };
>>          };
>> };
>>
>> bridge: bridge {
>>          status = "okay";
>>          ports {
>>                  bridge_in_rgb: endpoint {
>>                          remote-endpoint =  <&rgb_out_bridge>;
>>                  };
>>          };
>> };
>>
>> rgb_out: port at 1 {
>>          reg = <1>;
>>          #address-cells = <1>;
>>          #size-cells = <0>;
>>          rgb_out_panel: endpoint at 0 {
>>                   reg = <0>;
>>                  remote-endpoint = <&panel_in_rgb>;
>>          };
>>          rgb_out_bridge: endpoint at 1 {
>>                   reg = <1>;
>>                  remote-endpoint = <&bridge_in_rgb>;
>>          };
>>   };
>>
>> so rockchip_rgb_bind() @ rockchip_rgb.c use reg id to find the remote
>> actived endpoint.
>>
>> for_each_child_of_node(port, endpoint) {
>>          of_property_read_u32(endpoint, "reg", &endpoint_id);
> 
> Lack of reg property here should imply 0 for endpoint_id.
> 
ok, i will delete this reg id and set the endpoint_id default value to 0 
when lack of reg property at next version, thanks.
>>          ret = drm_of_find_panel_or_bridge(dev->of_node, 1, endpoint_id,
>> &rgb->panel, &rgb->bridge);
>>          if (!ret)
>>                  break;
>> }
>>
>>>>> +                    remote-endpoint = <&panel_in_rgb>;
>>>>> +                };
>>>>> +            };
>>>>> +        };
>>>>> +    };
>>>>> --
>>>>> 2.7.4
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>> _______________________________________________
>>> Linux-rockchip mailing list
>>> Linux-rockchip at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>
>>
> 
> 
> 

[PATCH 2/3] drm/rockchip: Add support for Rockchip Soc RGB output interface

[Sandy Huang]

Like rockchip rv1108 crtc can directly output parallel and serial
RGB data to panel or conversion chip, so we add this driver to
probe encoder and connector.

Signed-off-by: Sandy Huang <hjc@rock-chips.com>
---
 drivers/gpu/drm/rockchip/Kconfig            |   9 +
 drivers/gpu/drm/rockchip/Makefile           |   1 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c |   2 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   1 +
 drivers/gpu/drm/rockchip/rockchip_rgb.c     | 327 ++++++++++++++++++++++++++++
 5 files changed, 340 insertions(+)
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_rgb.c

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 0c31f0a..ff1c781 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -8,6 +8,7 @@ config DRM_ROCKCHIP
 	select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
 	select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
 	select DRM_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
+	select DRM_RGB if ROCKCHIP_RGB
 	select SND_SOC_HDMI_CODEC if ROCKCHIP_CDN_DP && SND_SOC
 	help
 	  Choose this option if you have a Rockchip soc chipset.
@@ -65,4 +66,12 @@ config ROCKCHIP_LVDS
 	  Rockchip rk3288 SoC has LVDS TX Controller can be used, and it
 	  support LVDS, rgb, dual LVDS output mode. say Y to enable its
 	  driver.
+
+config ROCKCHIP_RGB
+	bool "Rockchip RGB support"
+	help
+	  Choose this option to enable support for Rockchip RGB output.
+	  Like Rockchip rv1108 SoC CRTC can directly output parallel and
+	  serial RGB format to panel or connect to a conversion chip.
+	  say Y to enable its driver.
 endif
diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
index a881d2c..f32a17f 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -13,5 +13,6 @@ rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
 rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
 rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
 rockchipdrm-$(CONFIG_ROCKCHIP_LVDS) += rockchip_lvds.o
+rockchipdrm-$(CONFIG_ROCKCHIP_RGB) += rockchip_rgb.o
 
 obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 082c251..36e602a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -449,6 +449,8 @@ static int __init rockchip_drm_init(void)
 				CONFIG_ROCKCHIP_LVDS);
 	ADD_ROCKCHIP_SUB_DRIVER(rockchip_dp_driver,
 				CONFIG_ROCKCHIP_ANALOGIX_DP);
+	ADD_ROCKCHIP_SUB_DRIVER(rockchip_rgb_driver,
+				CONFIG_ROCKCHIP_RGB);
 	ADD_ROCKCHIP_SUB_DRIVER(cdn_dp_driver, CONFIG_ROCKCHIP_CDN_DP);
 	ADD_ROCKCHIP_SUB_DRIVER(dw_hdmi_rockchip_pltfm_driver,
 				CONFIG_ROCKCHIP_DW_HDMI);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index 498dfbc..6b0ec7e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -70,5 +70,6 @@ extern struct platform_driver dw_mipi_dsi_driver;
 extern struct platform_driver inno_hdmi_driver;
 extern struct platform_driver rockchip_dp_driver;
 extern struct platform_driver rockchip_lvds_driver;
+extern struct platform_driver rockchip_rgb_driver;
 extern struct platform_driver vop_platform_driver;
 #endif /* _ROCKCHIP_DRM_DRV_H_ */
diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c b/drivers/gpu/drm/rockchip/rockchip_rgb.c
new file mode 100644
index 0000000..0f0e6b464
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
@@ -0,0 +1,327 @@
+/*
+ * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
+ * Author:
+ *      Sandy Huang <hjc@rock-chips.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_dp_helper.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_of.h>
+
+#include <linux/component.h>
+#include <linux/of_graph.h>
+
+#include "rockchip_drm_drv.h"
+#include "rockchip_drm_vop.h"
+
+#define connector_to_rgb(c) container_of(c, struct rockchip_rgb, connector)
+#define encoder_to_rgb(c) container_of(c, struct rockchip_rgb, encoder)
+
+struct rockchip_rgb {
+	struct device *dev;
+	struct drm_device *drm_dev;
+	struct drm_panel *panel;
+	struct drm_bridge *bridge;
+	struct drm_connector connector;
+	struct drm_encoder encoder;
+	struct dev_pin_info *pins;
+	int output_mode;
+};
+
+static inline int name_to_output_mode(const char *s)
+{
+	if (strncmp(s, "p888", 4) == 0)
+		return ROCKCHIP_OUT_MODE_P888;
+	else if (strncmp(s, "p666", 4) == 0)
+		return ROCKCHIP_OUT_MODE_P666;
+	else if (strncmp(s, "p565", 4) == 0)
+		return ROCKCHIP_OUT_MODE_P565;
+	else if (strncmp(s, "s888", 4) == 0)
+		return ROCKCHIP_OUT_MODE_S888;
+	else if (strncmp(s, "s888_dummy", 10) == 0)
+		return ROCKCHIP_OUT_MODE_S888_DUMMY;
+
+	return -EINVAL;
+}
+
+static const struct drm_connector_funcs rockchip_rgb_connector_funcs = {
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int rockchip_rgb_connector_get_modes(struct drm_connector *connector)
+{
+	struct rockchip_rgb *rgb = connector_to_rgb(connector);
+	struct drm_panel *panel = rgb->panel;
+
+	return drm_panel_get_modes(panel);
+}
+
+static const
+struct drm_connector_helper_funcs rockchip_rgb_connector_helper_funcs = {
+	.get_modes = rockchip_rgb_connector_get_modes,
+};
+
+static void rockchip_rgb_encoder_enable(struct drm_encoder *encoder)
+{
+	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
+
+	drm_panel_prepare(rgb->panel);
+	/* iomux to LCD data/sync mode */
+	if (rgb->pins && !IS_ERR(rgb->pins->default_state))
+		pinctrl_select_state(rgb->pins->p, rgb->pins->default_state);
+
+	drm_panel_enable(rgb->panel);
+}
+
+static void rockchip_rgb_encoder_disable(struct drm_encoder *encoder)
+{
+	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
+
+	drm_panel_disable(rgb->panel);
+	drm_panel_unprepare(rgb->panel);
+}
+
+static int
+rockchip_rgb_encoder_atomic_check(struct drm_encoder *encoder,
+				   struct drm_crtc_state *crtc_state,
+				   struct drm_connector_state *conn_state)
+{
+	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
+	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
+
+	s->output_mode = rgb->output_mode;
+	s->output_type = DRM_MODE_CONNECTOR_LVDS;
+
+	return 0;
+}
+
+static const
+struct drm_encoder_helper_funcs rockchip_rgb_encoder_helper_funcs = {
+	.enable = rockchip_rgb_encoder_enable,
+	.disable = rockchip_rgb_encoder_disable,
+	.atomic_check = rockchip_rgb_encoder_atomic_check,
+};
+
+static const struct drm_encoder_funcs rockchip_rgb_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+static const struct of_device_id rockchip_rgb_dt_ids[] = {
+	{
+		.compatible = "rockchip,rv1108-rgb",
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, rockchip_rgb_dt_ids);
+
+static int rockchip_rgb_bind(struct device *dev, struct device *master,
+			     void *data)
+{
+	struct rockchip_rgb *rgb = dev_get_drvdata(dev);
+	struct drm_device *drm_dev = data;
+	struct drm_encoder *encoder;
+	struct drm_connector *connector;
+	struct device_node *remote = NULL;
+	struct device_node  *port, *endpoint;
+	u32 endpoint_id;
+	const char *name;
+	int ret;
+
+	rgb->drm_dev = drm_dev;
+	port = of_graph_get_port_by_id(dev->of_node, 1);
+	if (!port) {
+		DRM_DEV_ERROR(dev,
+			      "can't found port point, please init rgb panel port!\n");
+		return -EINVAL;
+	}
+	for_each_child_of_node(port, endpoint) {
+		of_property_read_u32(endpoint, "reg", &endpoint_id);
+		ret = drm_of_find_panel_or_bridge(dev->of_node, 1, endpoint_id,
+						  &rgb->panel, &rgb->bridge);
+		if (!ret)
+			break;
+	}
+	if (ret) {
+		DRM_DEV_ERROR(dev, "failed to find panel and bridge node\n");
+		ret  = -EPROBE_DEFER;
+		goto err_put_port;
+	}
+	if (rgb->panel)
+		remote = rgb->panel->dev->of_node;
+	else
+		remote = rgb->bridge->of_node;
+	if (of_property_read_string(dev->of_node, "rockchip,rgb-mode", &name))
+		/* default set it as output mode P888 */
+		rgb->output_mode = ROCKCHIP_OUT_MODE_P888;
+	else
+		rgb->output_mode = name_to_output_mode(name);
+	if (rgb->output_mode < 0) {
+		DRM_DEV_ERROR(dev, "invalid rockchip,rgb-mode [%s]\n", name);
+		ret = rgb->output_mode;
+		goto err_put_remote;
+	}
+
+	encoder = &rgb->encoder;
+	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
+							     dev->of_node);
+
+	ret = drm_encoder_init(drm_dev, encoder, &rockchip_rgb_encoder_funcs,
+			       DRM_MODE_ENCODER_NONE, NULL);
+	if (ret < 0) {
+		DRM_DEV_ERROR(drm_dev->dev,
+			      "failed to initialize encoder: %d\n", ret);
+		goto err_put_remote;
+	}
+
+	drm_encoder_helper_add(encoder, &rockchip_rgb_encoder_helper_funcs);
+
+	if (rgb->panel) {
+		connector = &rgb->connector;
+		connector->dpms = DRM_MODE_DPMS_OFF;
+		ret = drm_connector_init(drm_dev, connector,
+					 &rockchip_rgb_connector_funcs,
+					 DRM_MODE_CONNECTOR_Unknown);
+		if (ret < 0) {
+			DRM_DEV_ERROR(drm_dev->dev,
+				      "failed to initialize connector: %d\n",
+				      ret);
+			goto err_free_encoder;
+		}
+
+		drm_connector_helper_add(connector,
+					 &rockchip_rgb_connector_helper_funcs);
+
+		ret = drm_mode_connector_attach_encoder(connector, encoder);
+		if (ret < 0) {
+			DRM_DEV_ERROR(drm_dev->dev,
+				      "failed to attach encoder: %d\n", ret);
+			goto err_free_connector;
+		}
+
+		ret = drm_panel_attach(rgb->panel, connector);
+		if (ret < 0) {
+			DRM_DEV_ERROR(drm_dev->dev,
+				      "failed to attach panel: %d\n", ret);
+			goto err_free_connector;
+		}
+	} else {
+		rgb->bridge->encoder = encoder;
+		ret = drm_bridge_attach(encoder, rgb->bridge, NULL);
+		if (ret) {
+			DRM_DEV_ERROR(drm_dev->dev,
+				      "failed to attach bridge: %d\n", ret);
+			goto err_free_encoder;
+		}
+		encoder->bridge = rgb->bridge;
+	}
+
+	of_node_put(remote);
+	of_node_put(port);
+
+	return 0;
+
+err_free_connector:
+	drm_connector_cleanup(connector);
+err_free_encoder:
+	drm_encoder_cleanup(encoder);
+err_put_remote:
+	of_node_put(remote);
+err_put_port:
+	of_node_put(port);
+
+	return ret;
+}
+
+static void rockchip_rgb_unbind(struct device *dev, struct device *master,
+				void *data)
+{
+	struct rockchip_rgb *rgb = dev_get_drvdata(dev);
+
+	rockchip_rgb_encoder_disable(&rgb->encoder);
+	if (rgb->panel)
+		drm_panel_detach(rgb->panel);
+	drm_connector_cleanup(&rgb->connector);
+	drm_encoder_cleanup(&rgb->encoder);
+}
+
+static const struct component_ops rockchip_rgb_component_ops = {
+	.bind = rockchip_rgb_bind,
+	.unbind = rockchip_rgb_unbind,
+};
+
+static int rockchip_rgb_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rockchip_rgb *rgb;
+	const struct of_device_id *match;
+	int ret;
+
+	if (!dev->of_node)
+		return -ENODEV;
+
+	rgb = devm_kzalloc(&pdev->dev, sizeof(*rgb), GFP_KERNEL);
+	if (!rgb)
+		return -ENOMEM;
+
+	rgb->dev = dev;
+	match = of_match_node(rockchip_rgb_dt_ids, dev->of_node);
+	if (!match)
+		return -ENODEV;
+
+	rgb->pins = devm_kzalloc(rgb->dev, sizeof(*rgb->pins), GFP_KERNEL);
+	if (!rgb->pins)
+		return -ENOMEM;
+	rgb->pins->p = devm_pinctrl_get(rgb->dev);
+	if (IS_ERR(rgb->pins->p)) {
+		DRM_DEV_ERROR(dev, "no pinctrl handle\n");
+		devm_kfree(rgb->dev, rgb->pins);
+		rgb->pins = NULL;
+	} else {
+		rgb->pins->default_state =
+			pinctrl_lookup_state(rgb->pins->p, "lcdc");
+		if (IS_ERR(rgb->pins->default_state)) {
+			DRM_DEV_ERROR(dev, "no default pinctrl state\n");
+			devm_kfree(rgb->dev, rgb->pins);
+			rgb->pins = NULL;
+		}
+	}
+
+	dev_set_drvdata(dev, rgb);
+	ret = component_add(&pdev->dev, &rockchip_rgb_component_ops);
+	if (ret < 0)
+		DRM_DEV_ERROR(dev, "failed to add component\n");
+
+	return ret;
+}
+
+static int rockchip_rgb_remove(struct platform_device *pdev)
+{
+	component_del(&pdev->dev, &rockchip_rgb_component_ops);
+
+	return 0;
+}
+
+struct platform_driver rockchip_rgb_driver = {
+	.probe = rockchip_rgb_probe,
+	.remove = rockchip_rgb_remove,
+	.driver = {
+		   .name = "rockchip-rgb",
+		   .of_match_table = of_match_ptr(rockchip_rgb_dt_ids),
+	},
+};
-- 
2.7.4

[PATCH 2/3] drm/rockchip: Add support for Rockchip Soc RGB output interface

[Sean Paul]

On Thu, Sep 14, 2017 at 11:43:23AM +0800, Sandy Huang wrote:
> Like rockchip rv1108 crtc can directly output parallel and serial
> RGB data to panel or conversion chip, so we add this driver to
> probe encoder and connector.
> 
> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
> ---
>  drivers/gpu/drm/rockchip/Kconfig            |   9 +
>  drivers/gpu/drm/rockchip/Makefile           |   1 +
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c |   2 +
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   1 +
>  drivers/gpu/drm/rockchip/rockchip_rgb.c     | 327 ++++++++++++++++++++++++++++
>  5 files changed, 340 insertions(+)
>  create mode 100644 drivers/gpu/drm/rockchip/rockchip_rgb.c
> 
> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
> index 0c31f0a..ff1c781 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -8,6 +8,7 @@ config DRM_ROCKCHIP
>  	select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
>  	select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
>  	select DRM_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
> +	select DRM_RGB if ROCKCHIP_RGB
>  	select SND_SOC_HDMI_CODEC if ROCKCHIP_CDN_DP && SND_SOC
>  	help
>  	  Choose this option if you have a Rockchip soc chipset.
> @@ -65,4 +66,12 @@ config ROCKCHIP_LVDS
>  	  Rockchip rk3288 SoC has LVDS TX Controller can be used, and it
>  	  support LVDS, rgb, dual LVDS output mode. say Y to enable its
>  	  driver.
> +
> +config ROCKCHIP_RGB
> +	bool "Rockchip RGB support"
> +	help
> +	  Choose this option to enable support for Rockchip RGB output.
> +	  Like Rockchip rv1108 SoC CRTC can directly output parallel and

The wording here is a little awkward. Perhaps change to:

Some Rockchip CRTCs, like rv1108, can directly output parallel and
...

> +	  serial RGB format to panel or connect to a conversion chip.
> +	  say Y to enable its driver.
>  endif
> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
> index a881d2c..f32a17f 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -13,5 +13,6 @@ rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
>  rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
>  rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
>  rockchipdrm-$(CONFIG_ROCKCHIP_LVDS) += rockchip_lvds.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_RGB) += rockchip_rgb.o
>  
>  obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 082c251..36e602a 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -449,6 +449,8 @@ static int __init rockchip_drm_init(void)
>  				CONFIG_ROCKCHIP_LVDS);
>  	ADD_ROCKCHIP_SUB_DRIVER(rockchip_dp_driver,
>  				CONFIG_ROCKCHIP_ANALOGIX_DP);
> +	ADD_ROCKCHIP_SUB_DRIVER(rockchip_rgb_driver,
> +				CONFIG_ROCKCHIP_RGB);
>  	ADD_ROCKCHIP_SUB_DRIVER(cdn_dp_driver, CONFIG_ROCKCHIP_CDN_DP);
>  	ADD_ROCKCHIP_SUB_DRIVER(dw_hdmi_rockchip_pltfm_driver,
>  				CONFIG_ROCKCHIP_DW_HDMI);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index 498dfbc..6b0ec7e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -70,5 +70,6 @@ extern struct platform_driver dw_mipi_dsi_driver;
>  extern struct platform_driver inno_hdmi_driver;
>  extern struct platform_driver rockchip_dp_driver;
>  extern struct platform_driver rockchip_lvds_driver;
> +extern struct platform_driver rockchip_rgb_driver;
>  extern struct platform_driver vop_platform_driver;
>  #endif /* _ROCKCHIP_DRM_DRV_H_ */
> diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c b/drivers/gpu/drm/rockchip/rockchip_rgb.c
> new file mode 100644
> index 0000000..0f0e6b464
> --- /dev/null
> +++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
> @@ -0,0 +1,327 @@
> +/*
> + * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
> + * Author:
> + *      Sandy Huang <hjc@rock-chips.com>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_dp_helper.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_of.h>
> +
> +#include <linux/component.h>
> +#include <linux/of_graph.h>
> +
> +#include "rockchip_drm_drv.h"
> +#include "rockchip_drm_vop.h"
> +
> +#define connector_to_rgb(c) container_of(c, struct rockchip_rgb, connector)
> +#define encoder_to_rgb(c) container_of(c, struct rockchip_rgb, encoder)
> +
> +struct rockchip_rgb {
> +	struct device *dev;
> +	struct drm_device *drm_dev;
> +	struct drm_panel *panel;
> +	struct drm_bridge *bridge;
> +	struct drm_connector connector;
> +	struct drm_encoder encoder;
> +	struct dev_pin_info *pins;
> +	int output_mode;
> +};
> +
> +static inline int name_to_output_mode(const char *s)
> +{
> +	if (strncmp(s, "p888", 4) == 0)
> +		return ROCKCHIP_OUT_MODE_P888;
> +	else if (strncmp(s, "p666", 4) == 0)
> +		return ROCKCHIP_OUT_MODE_P666;
> +	else if (strncmp(s, "p565", 4) == 0)
> +		return ROCKCHIP_OUT_MODE_P565;
> +	else if (strncmp(s, "s888", 4) == 0)
> +		return ROCKCHIP_OUT_MODE_S888;
> +	else if (strncmp(s, "s888_dummy", 10) == 0)
> +		return ROCKCHIP_OUT_MODE_S888_DUMMY;

Instead of hardcoding the string lengths, try:

        static const struct {
                const char *name;
                int format;
        } formats[] = {
                { "p888", ROCKCHIP_OUT_MODE_P888 },
                { "p666", ROCKCHIP_OUT_MODE_P666 },
                { "p565", ROCKCHIP_OUT_MODE_P565 },
                { "s888", ROCKCHIP_OUT_MODE_S888 },
                { "s888_dummy", ROCKCHIP_OUT_MODE_S888_DUMMY }
        };
        int i;

        for (i = 0; i < ARRAY_SIZE(formats); i++)
                if (!strncmp(s, formats[i].name, strlen(formats[i].name)))
                        return formats[i].format;

Yes, strlen isn't as performant, but this code only runs once and it's safer.

> +
> +	return -EINVAL;
> +}
> +
> +static const struct drm_connector_funcs rockchip_rgb_connector_funcs = {
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = drm_connector_cleanup,
> +	.reset = drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int rockchip_rgb_connector_get_modes(struct drm_connector *connector)
> +{
> +	struct rockchip_rgb *rgb = connector_to_rgb(connector);
> +	struct drm_panel *panel = rgb->panel;
> +
> +	return drm_panel_get_modes(panel);
> +}
> +
> +static const
> +struct drm_connector_helper_funcs rockchip_rgb_connector_helper_funcs = {
> +	.get_modes = rockchip_rgb_connector_get_modes,
> +};
> +
> +static void rockchip_rgb_encoder_enable(struct drm_encoder *encoder)
> +{
> +	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
> +
> +	drm_panel_prepare(rgb->panel);
> +	/* iomux to LCD data/sync mode */
> +	if (rgb->pins && !IS_ERR(rgb->pins->default_state))
> +		pinctrl_select_state(rgb->pins->p, rgb->pins->default_state);
> +
> +	drm_panel_enable(rgb->panel);
> +}
> +
> +static void rockchip_rgb_encoder_disable(struct drm_encoder *encoder)
> +{
> +	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
> +
> +	drm_panel_disable(rgb->panel);
> +	drm_panel_unprepare(rgb->panel);
> +}
> +
> +static int
> +rockchip_rgb_encoder_atomic_check(struct drm_encoder *encoder,
> +				   struct drm_crtc_state *crtc_state,
> +				   struct drm_connector_state *conn_state)
> +{
> +	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
> +	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
> +
> +	s->output_mode = rgb->output_mode;
> +	s->output_type = DRM_MODE_CONNECTOR_LVDS;
> +
> +	return 0;
> +}
> +
> +static const
> +struct drm_encoder_helper_funcs rockchip_rgb_encoder_helper_funcs = {
> +	.enable = rockchip_rgb_encoder_enable,
> +	.disable = rockchip_rgb_encoder_disable,
> +	.atomic_check = rockchip_rgb_encoder_atomic_check,
> +};
> +
> +static const struct drm_encoder_funcs rockchip_rgb_encoder_funcs = {
> +	.destroy = drm_encoder_cleanup,
> +};
> +
> +static const struct of_device_id rockchip_rgb_dt_ids[] = {
> +	{
> +		.compatible = "rockchip,rv1108-rgb",
> +	},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, rockchip_rgb_dt_ids);
> +
> +static int rockchip_rgb_bind(struct device *dev, struct device *master,
> +			     void *data)
> +{
> +	struct rockchip_rgb *rgb = dev_get_drvdata(dev);
> +	struct drm_device *drm_dev = data;
> +	struct drm_encoder *encoder;
> +	struct drm_connector *connector;
> +	struct device_node *remote = NULL;
> +	struct device_node  *port, *endpoint;
> +	u32 endpoint_id;
> +	const char *name;
> +	int ret;
> +
> +	rgb->drm_dev = drm_dev;
> +	port = of_graph_get_port_by_id(dev->of_node, 1);
> +	if (!port) {
> +		DRM_DEV_ERROR(dev,
> +			      "can't found port point, please init rgb panel port!\n");
> +		return -EINVAL;
> +	}
> +	for_each_child_of_node(port, endpoint) {
> +		of_property_read_u32(endpoint, "reg", &endpoint_id);
> +		ret = drm_of_find_panel_or_bridge(dev->of_node, 1, endpoint_id,
> +						  &rgb->panel, &rgb->bridge);
> +		if (!ret)
> +			break;
> +	}
> +	if (ret) {
> +		DRM_DEV_ERROR(dev, "failed to find panel and bridge node\n");
> +		ret  = -EPROBE_DEFER;
> +		goto err_put_port;
> +	}
> +	if (rgb->panel)
> +		remote = rgb->panel->dev->of_node;
> +	else
> +		remote = rgb->bridge->of_node;
> +	if (of_property_read_string(dev->of_node, "rockchip,rgb-mode", &name))
> +		/* default set it as output mode P888 */
> +		rgb->output_mode = ROCKCHIP_OUT_MODE_P888;
> +	else
> +		rgb->output_mode = name_to_output_mode(name);
> +	if (rgb->output_mode < 0) {
> +		DRM_DEV_ERROR(dev, "invalid rockchip,rgb-mode [%s]\n", name);
> +		ret = rgb->output_mode;
> +		goto err_put_remote;
> +	}
> +
> +	encoder = &rgb->encoder;
> +	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
> +							     dev->of_node);
> +
> +	ret = drm_encoder_init(drm_dev, encoder, &rockchip_rgb_encoder_funcs,
> +			       DRM_MODE_ENCODER_NONE, NULL);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(drm_dev->dev,
> +			      "failed to initialize encoder: %d\n", ret);
> +		goto err_put_remote;
> +	}
> +
> +	drm_encoder_helper_add(encoder, &rockchip_rgb_encoder_helper_funcs);
> +
> +	if (rgb->panel) {
> +		connector = &rgb->connector;
> +		connector->dpms = DRM_MODE_DPMS_OFF;
> +		ret = drm_connector_init(drm_dev, connector,
> +					 &rockchip_rgb_connector_funcs,
> +					 DRM_MODE_CONNECTOR_Unknown);
> +		if (ret < 0) {
> +			DRM_DEV_ERROR(drm_dev->dev,
> +				      "failed to initialize connector: %d\n",
> +				      ret);
> +			goto err_free_encoder;
> +		}
> +
> +		drm_connector_helper_add(connector,
> +					 &rockchip_rgb_connector_helper_funcs);
> +
> +		ret = drm_mode_connector_attach_encoder(connector, encoder);
> +		if (ret < 0) {
> +			DRM_DEV_ERROR(drm_dev->dev,
> +				      "failed to attach encoder: %d\n", ret);
> +			goto err_free_connector;
> +		}
> +
> +		ret = drm_panel_attach(rgb->panel, connector);
> +		if (ret < 0) {
> +			DRM_DEV_ERROR(drm_dev->dev,
> +				      "failed to attach panel: %d\n", ret);
> +			goto err_free_connector;
> +		}
> +	} else {
> +		rgb->bridge->encoder = encoder;
> +		ret = drm_bridge_attach(encoder, rgb->bridge, NULL);
> +		if (ret) {
> +			DRM_DEV_ERROR(drm_dev->dev,
> +				      "failed to attach bridge: %d\n", ret);
> +			goto err_free_encoder;
> +		}
> +		encoder->bridge = rgb->bridge;
> +	}
> +
> +	of_node_put(remote);
> +	of_node_put(port);
> +
> +	return 0;
> +
> +err_free_connector:
> +	drm_connector_cleanup(connector);
> +err_free_encoder:
> +	drm_encoder_cleanup(encoder);
> +err_put_remote:
> +	of_node_put(remote);
> +err_put_port:
> +	of_node_put(port);
> +
> +	return ret;
> +}
> +
> +static void rockchip_rgb_unbind(struct device *dev, struct device *master,
> +				void *data)
> +{
> +	struct rockchip_rgb *rgb = dev_get_drvdata(dev);
> +
> +	rockchip_rgb_encoder_disable(&rgb->encoder);
> +	if (rgb->panel)
> +		drm_panel_detach(rgb->panel);
> +	drm_connector_cleanup(&rgb->connector);
> +	drm_encoder_cleanup(&rgb->encoder);
> +}
> +
> +static const struct component_ops rockchip_rgb_component_ops = {
> +	.bind = rockchip_rgb_bind,
> +	.unbind = rockchip_rgb_unbind,
> +};
> +
> +static int rockchip_rgb_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rockchip_rgb *rgb;
> +	const struct of_device_id *match;
> +	int ret;
> +
> +	if (!dev->of_node)
> +		return -ENODEV;
> +
> +	rgb = devm_kzalloc(&pdev->dev, sizeof(*rgb), GFP_KERNEL);
> +	if (!rgb)
> +		return -ENOMEM;
> +
> +	rgb->dev = dev;
> +	match = of_match_node(rockchip_rgb_dt_ids, dev->of_node);
> +	if (!match)
> +		return -ENODEV;
> +
> +	rgb->pins = devm_kzalloc(rgb->dev, sizeof(*rgb->pins), GFP_KERNEL);
> +	if (!rgb->pins)
> +		return -ENOMEM;

Can you please log errors for all of these failing conditions?

> +	rgb->pins->p = devm_pinctrl_get(rgb->dev);
> +	if (IS_ERR(rgb->pins->p)) {
> +		DRM_DEV_ERROR(dev, "no pinctrl handle\n");
> +		devm_kfree(rgb->dev, rgb->pins);
> +		rgb->pins = NULL;
> +	} else {
> +		rgb->pins->default_state =
> +			pinctrl_lookup_state(rgb->pins->p, "lcdc");
> +		if (IS_ERR(rgb->pins->default_state)) {
> +			DRM_DEV_ERROR(dev, "no default pinctrl state\n");
> +			devm_kfree(rgb->dev, rgb->pins);
> +			rgb->pins = NULL;
> +		}
> +	}
> +
> +	dev_set_drvdata(dev, rgb);
> +	ret = component_add(&pdev->dev, &rockchip_rgb_component_ops);
> +	if (ret < 0)
> +		DRM_DEV_ERROR(dev, "failed to add component\n");
> +
> +	return ret;
> +}
> +
> +static int rockchip_rgb_remove(struct platform_device *pdev)
> +{
> +	component_del(&pdev->dev, &rockchip_rgb_component_ops);
> +
> +	return 0;
> +}
> +
> +struct platform_driver rockchip_rgb_driver = {
> +	.probe = rockchip_rgb_probe,
> +	.remove = rockchip_rgb_remove,
> +	.driver = {
> +		   .name = "rockchip-rgb",
> +		   .of_match_table = of_match_ptr(rockchip_rgb_dt_ids),
> +	},
> +};
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS

[PATCH 2/3] drm/rockchip: Add support for Rockchip Soc RGB output interface

[Sandy Huang]

Hi sean,
     Thanks for your review.

? 2017/9/20 7:02, Sean Paul ??:
> On Thu, Sep 14, 2017 at 11:43:23AM +0800, Sandy Huang wrote:
>> Like rockchip rv1108 crtc can directly output parallel and serial
>> RGB data to panel or conversion chip, so we add this driver to
>> probe encoder and connector.
>>
>> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
>> ---
>>   drivers/gpu/drm/rockchip/Kconfig            |   9 +
>>   drivers/gpu/drm/rockchip/Makefile           |   1 +
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.c |   2 +
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   1 +
>>   drivers/gpu/drm/rockchip/rockchip_rgb.c     | 327 ++++++++++++++++++++++++++++
>>   5 files changed, 340 insertions(+)
>>   create mode 100644 drivers/gpu/drm/rockchip/rockchip_rgb.c
>>
>> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
>> index 0c31f0a..ff1c781 100644
>> --- a/drivers/gpu/drm/rockchip/Kconfig
>> +++ b/drivers/gpu/drm/rockchip/Kconfig
>> @@ -8,6 +8,7 @@ config DRM_ROCKCHIP
>>   	select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
>>   	select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
>>   	select DRM_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
>> +	select DRM_RGB if ROCKCHIP_RGB
>>   	select SND_SOC_HDMI_CODEC if ROCKCHIP_CDN_DP && SND_SOC
>>   	help
>>   	  Choose this option if you have a Rockchip soc chipset.
>> @@ -65,4 +66,12 @@ config ROCKCHIP_LVDS
>>   	  Rockchip rk3288 SoC has LVDS TX Controller can be used, and it
>>   	  support LVDS, rgb, dual LVDS output mode. say Y to enable its
>>   	  driver.
>> +
>> +config ROCKCHIP_RGB
>> +	bool "Rockchip RGB support"
>> +	help
>> +	  Choose this option to enable support for Rockchip RGB output.
>> +	  Like Rockchip rv1108 SoC CRTC can directly output parallel and
> 
> The wording here is a little awkward. Perhaps change to:
> 
> Some Rockchip CRTCs, like rv1108, can directly output parallel and
> ...
> 
ok, i will update at next version.

>> +	  serial RGB format to panel or connect to a conversion chip.
>> +	  say Y to enable its driver.
>>   endif
>> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
>> index a881d2c..f32a17f 100644
>> --- a/drivers/gpu/drm/rockchip/Makefile
>> +++ b/drivers/gpu/drm/rockchip/Makefile
>> @@ -13,5 +13,6 @@ rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
>>   rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
>>   rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
>>   rockchipdrm-$(CONFIG_ROCKCHIP_LVDS) += rockchip_lvds.o
>> +rockchipdrm-$(CONFIG_ROCKCHIP_RGB) += rockchip_rgb.o
>>   
>>   obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> index 082c251..36e602a 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> @@ -449,6 +449,8 @@ static int __init rockchip_drm_init(void)
>>   				CONFIG_ROCKCHIP_LVDS);
>>   	ADD_ROCKCHIP_SUB_DRIVER(rockchip_dp_driver,
>>   				CONFIG_ROCKCHIP_ANALOGIX_DP);
>> +	ADD_ROCKCHIP_SUB_DRIVER(rockchip_rgb_driver,
>> +				CONFIG_ROCKCHIP_RGB);
>>   	ADD_ROCKCHIP_SUB_DRIVER(cdn_dp_driver, CONFIG_ROCKCHIP_CDN_DP);
>>   	ADD_ROCKCHIP_SUB_DRIVER(dw_hdmi_rockchip_pltfm_driver,
>>   				CONFIG_ROCKCHIP_DW_HDMI);
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> index 498dfbc..6b0ec7e 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> @@ -70,5 +70,6 @@ extern struct platform_driver dw_mipi_dsi_driver;
>>   extern struct platform_driver inno_hdmi_driver;
>>   extern struct platform_driver rockchip_dp_driver;
>>   extern struct platform_driver rockchip_lvds_driver;
>> +extern struct platform_driver rockchip_rgb_driver;
>>   extern struct platform_driver vop_platform_driver;
>>   #endif /* _ROCKCHIP_DRM_DRV_H_ */
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c b/drivers/gpu/drm/rockchip/rockchip_rgb.c
>> new file mode 100644
>> index 0000000..0f0e6b464
>> --- /dev/null
>> +++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
>> @@ -0,0 +1,327 @@
>> +/*
>> + * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
>> + * Author:
>> + *      Sandy Huang <hjc@rock-chips.com>
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_dp_helper.h>
>> +#include <drm/drm_panel.h>
>> +#include <drm/drm_of.h>
>> +
>> +#include <linux/component.h>
>> +#include <linux/of_graph.h>
>> +
>> +#include "rockchip_drm_drv.h"
>> +#include "rockchip_drm_vop.h"
>> +
>> +#define connector_to_rgb(c) container_of(c, struct rockchip_rgb, connector)
>> +#define encoder_to_rgb(c) container_of(c, struct rockchip_rgb, encoder)
>> +
>> +struct rockchip_rgb {
>> +	struct device *dev;
>> +	struct drm_device *drm_dev;
>> +	struct drm_panel *panel;
>> +	struct drm_bridge *bridge;
>> +	struct drm_connector connector;
>> +	struct drm_encoder encoder;
>> +	struct dev_pin_info *pins;
>> +	int output_mode;
>> +};
>> +
>> +static inline int name_to_output_mode(const char *s)
>> +{
>> +	if (strncmp(s, "p888", 4) == 0)
>> +		return ROCKCHIP_OUT_MODE_P888;
>> +	else if (strncmp(s, "p666", 4) == 0)
>> +		return ROCKCHIP_OUT_MODE_P666;
>> +	else if (strncmp(s, "p565", 4) == 0)
>> +		return ROCKCHIP_OUT_MODE_P565;
>> +	else if (strncmp(s, "s888", 4) == 0)
>> +		return ROCKCHIP_OUT_MODE_S888;
>> +	else if (strncmp(s, "s888_dummy", 10) == 0)
>> +		return ROCKCHIP_OUT_MODE_S888_DUMMY;
> 
> Instead of hardcoding the string lengths, try:
> 
>          static const struct {
>                  const char *name;
>                  int format;
>          } formats[] = {
>                  { "p888", ROCKCHIP_OUT_MODE_P888 },
>                  { "p666", ROCKCHIP_OUT_MODE_P666 },
>                  { "p565", ROCKCHIP_OUT_MODE_P565 },
>                  { "s888", ROCKCHIP_OUT_MODE_S888 },
>                  { "s888_dummy", ROCKCHIP_OUT_MODE_S888_DUMMY }
>          };
>          int i;
> 
>          for (i = 0; i < ARRAY_SIZE(formats); i++)
>                  if (!strncmp(s, formats[i].name, strlen(formats[i].name)))
>                          return formats[i].format;
> 
> Yes, strlen isn't as performant, but this code only runs once and it's safer.
> 
ok, i will update at next version.
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static const struct drm_connector_funcs rockchip_rgb_connector_funcs = {
>> +	.fill_modes = drm_helper_probe_single_connector_modes,
>> +	.destroy = drm_connector_cleanup,
>> +	.reset = drm_atomic_helper_connector_reset,
>> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +static int rockchip_rgb_connector_get_modes(struct drm_connector *connector)
>> +{
>> +	struct rockchip_rgb *rgb = connector_to_rgb(connector);
>> +	struct drm_panel *panel = rgb->panel;
>> +
>> +	return drm_panel_get_modes(panel);
>> +}
>> +
>> +static const
>> +struct drm_connector_helper_funcs rockchip_rgb_connector_helper_funcs = {
>> +	.get_modes = rockchip_rgb_connector_get_modes,
>> +};
>> +
>> +static void rockchip_rgb_encoder_enable(struct drm_encoder *encoder)
>> +{
>> +	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
>> +
>> +	drm_panel_prepare(rgb->panel);
>> +	/* iomux to LCD data/sync mode */
>> +	if (rgb->pins && !IS_ERR(rgb->pins->default_state))
>> +		pinctrl_select_state(rgb->pins->p, rgb->pins->default_state);
>> +
>> +	drm_panel_enable(rgb->panel);
>> +}
>> +
>> +static void rockchip_rgb_encoder_disable(struct drm_encoder *encoder)
>> +{
>> +	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
>> +
>> +	drm_panel_disable(rgb->panel);
>> +	drm_panel_unprepare(rgb->panel);
>> +}
>> +
>> +static int
>> +rockchip_rgb_encoder_atomic_check(struct drm_encoder *encoder,
>> +				   struct drm_crtc_state *crtc_state,
>> +				   struct drm_connector_state *conn_state)
>> +{
>> +	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
>> +	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
>> +
>> +	s->output_mode = rgb->output_mode;
>> +	s->output_type = DRM_MODE_CONNECTOR_LVDS;
>> +
>> +	return 0;
>> +}
>> +
>> +static const
>> +struct drm_encoder_helper_funcs rockchip_rgb_encoder_helper_funcs = {
>> +	.enable = rockchip_rgb_encoder_enable,
>> +	.disable = rockchip_rgb_encoder_disable,
>> +	.atomic_check = rockchip_rgb_encoder_atomic_check,
>> +};
>> +
>> +static const struct drm_encoder_funcs rockchip_rgb_encoder_funcs = {
>> +	.destroy = drm_encoder_cleanup,
>> +};
>> +
>> +static const struct of_device_id rockchip_rgb_dt_ids[] = {
>> +	{
>> +		.compatible = "rockchip,rv1108-rgb",
>> +	},
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, rockchip_rgb_dt_ids);
>> +
>> +static int rockchip_rgb_bind(struct device *dev, struct device *master,
>> +			     void *data)
>> +{
>> +	struct rockchip_rgb *rgb = dev_get_drvdata(dev);
>> +	struct drm_device *drm_dev = data;
>> +	struct drm_encoder *encoder;
>> +	struct drm_connector *connector;
>> +	struct device_node *remote = NULL;
>> +	struct device_node  *port, *endpoint;
>> +	u32 endpoint_id;
>> +	const char *name;
>> +	int ret;
>> +
>> +	rgb->drm_dev = drm_dev;
>> +	port = of_graph_get_port_by_id(dev->of_node, 1);
>> +	if (!port) {
>> +		DRM_DEV_ERROR(dev,
>> +			      "can't found port point, please init rgb panel port!\n");
>> +		return -EINVAL;
>> +	}
>> +	for_each_child_of_node(port, endpoint) {
>> +		of_property_read_u32(endpoint, "reg", &endpoint_id);
>> +		ret = drm_of_find_panel_or_bridge(dev->of_node, 1, endpoint_id,
>> +						  &rgb->panel, &rgb->bridge);
>> +		if (!ret)
>> +			break;
>> +	}
>> +	if (ret) {
>> +		DRM_DEV_ERROR(dev, "failed to find panel and bridge node\n");
>> +		ret  = -EPROBE_DEFER;
>> +		goto err_put_port;
>> +	}
>> +	if (rgb->panel)
>> +		remote = rgb->panel->dev->of_node;
>> +	else
>> +		remote = rgb->bridge->of_node;
>> +	if (of_property_read_string(dev->of_node, "rockchip,rgb-mode", &name))
>> +		/* default set it as output mode P888 */
>> +		rgb->output_mode = ROCKCHIP_OUT_MODE_P888;
>> +	else
>> +		rgb->output_mode = name_to_output_mode(name);
>> +	if (rgb->output_mode < 0) {
>> +		DRM_DEV_ERROR(dev, "invalid rockchip,rgb-mode [%s]\n", name);
>> +		ret = rgb->output_mode;
>> +		goto err_put_remote;
>> +	}
>> +
>> +	encoder = &rgb->encoder;
>> +	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
>> +							     dev->of_node);
>> +
>> +	ret = drm_encoder_init(drm_dev, encoder, &rockchip_rgb_encoder_funcs,
>> +			       DRM_MODE_ENCODER_NONE, NULL);
>> +	if (ret < 0) {
>> +		DRM_DEV_ERROR(drm_dev->dev,
>> +			      "failed to initialize encoder: %d\n", ret);
>> +		goto err_put_remote;
>> +	}
>> +
>> +	drm_encoder_helper_add(encoder, &rockchip_rgb_encoder_helper_funcs);
>> +
>> +	if (rgb->panel) {
>> +		connector = &rgb->connector;
>> +		connector->dpms = DRM_MODE_DPMS_OFF;
>> +		ret = drm_connector_init(drm_dev, connector,
>> +					 &rockchip_rgb_connector_funcs,
>> +					 DRM_MODE_CONNECTOR_Unknown);
>> +		if (ret < 0) {
>> +			DRM_DEV_ERROR(drm_dev->dev,
>> +				      "failed to initialize connector: %d\n",
>> +				      ret);
>> +			goto err_free_encoder;
>> +		}
>> +
>> +		drm_connector_helper_add(connector,
>> +					 &rockchip_rgb_connector_helper_funcs);
>> +
>> +		ret = drm_mode_connector_attach_encoder(connector, encoder);
>> +		if (ret < 0) {
>> +			DRM_DEV_ERROR(drm_dev->dev,
>> +				      "failed to attach encoder: %d\n", ret);
>> +			goto err_free_connector;
>> +		}
>> +
>> +		ret = drm_panel_attach(rgb->panel, connector);
>> +		if (ret < 0) {
>> +			DRM_DEV_ERROR(drm_dev->dev,
>> +				      "failed to attach panel: %d\n", ret);
>> +			goto err_free_connector;
>> +		}
>> +	} else {
>> +		rgb->bridge->encoder = encoder;
>> +		ret = drm_bridge_attach(encoder, rgb->bridge, NULL);
>> +		if (ret) {
>> +			DRM_DEV_ERROR(drm_dev->dev,
>> +				      "failed to attach bridge: %d\n", ret);
>> +			goto err_free_encoder;
>> +		}
>> +		encoder->bridge = rgb->bridge;
>> +	}
>> +
>> +	of_node_put(remote);
>> +	of_node_put(port);
>> +
>> +	return 0;
>> +
>> +err_free_connector:
>> +	drm_connector_cleanup(connector);
>> +err_free_encoder:
>> +	drm_encoder_cleanup(encoder);
>> +err_put_remote:
>> +	of_node_put(remote);
>> +err_put_port:
>> +	of_node_put(port);
>> +
>> +	return ret;
>> +}
>> +
>> +static void rockchip_rgb_unbind(struct device *dev, struct device *master,
>> +				void *data)
>> +{
>> +	struct rockchip_rgb *rgb = dev_get_drvdata(dev);
>> +
>> +	rockchip_rgb_encoder_disable(&rgb->encoder);
>> +	if (rgb->panel)
>> +		drm_panel_detach(rgb->panel);
>> +	drm_connector_cleanup(&rgb->connector);
>> +	drm_encoder_cleanup(&rgb->encoder);
>> +}
>> +
>> +static const struct component_ops rockchip_rgb_component_ops = {
>> +	.bind = rockchip_rgb_bind,
>> +	.unbind = rockchip_rgb_unbind,
>> +};
>> +
>> +static int rockchip_rgb_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct rockchip_rgb *rgb;
>> +	const struct of_device_id *match;
>> +	int ret;
>> +
>> +	if (!dev->of_node)
>> +		return -ENODEV;
>> +
>> +	rgb = devm_kzalloc(&pdev->dev, sizeof(*rgb), GFP_KERNEL);
>> +	if (!rgb)
>> +		return -ENOMEM;
>> +
>> +	rgb->dev = dev;
>> +	match = of_match_node(rockchip_rgb_dt_ids, dev->of_node);
>> +	if (!match)
>> +		return -ENODEV;
>> +
>> +	rgb->pins = devm_kzalloc(rgb->dev, sizeof(*rgb->pins), GFP_KERNEL);
>> +	if (!rgb->pins)
>> +		return -ENOMEM;
> 
> Can you please log errors for all of these failing conditions?
> 
ok, i will add error log for this return, except the devm_kzalloc error.

>> +	rgb->pins->p = devm_pinctrl_get(rgb->dev);
>> +	if (IS_ERR(rgb->pins->p)) {
>> +		DRM_DEV_ERROR(dev, "no pinctrl handle\n");
>> +		devm_kfree(rgb->dev, rgb->pins);
>> +		rgb->pins = NULL;
>> +	} else {
>> +		rgb->pins->default_state =
>> +			pinctrl_lookup_state(rgb->pins->p, "lcdc");
>> +		if (IS_ERR(rgb->pins->default_state)) {
>> +			DRM_DEV_ERROR(dev, "no default pinctrl state\n");
>> +			devm_kfree(rgb->dev, rgb->pins);
>> +			rgb->pins = NULL;
>> +		}
>> +	}
>> +
>> +	dev_set_drvdata(dev, rgb);
>> +	ret = component_add(&pdev->dev, &rockchip_rgb_component_ops);
>> +	if (ret < 0)
>> +		DRM_DEV_ERROR(dev, "failed to add component\n");
>> +
>> +	return ret;
>> +}
>> +
>> +static int rockchip_rgb_remove(struct platform_device *pdev)
>> +{
>> +	component_del(&pdev->dev, &rockchip_rgb_component_ops);
>> +
>> +	return 0;
>> +}
>> +
>> +struct platform_driver rockchip_rgb_driver = {
>> +	.probe = rockchip_rgb_probe,
>> +	.remove = rockchip_rgb_remove,
>> +	.driver = {
>> +		   .name = "rockchip-rgb",
>> +		   .of_match_table = of_match_ptr(rockchip_rgb_dt_ids),
>> +	},
>> +};
>> -- 
>> 2.7.4
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

[PATCH 3/3] drm/rockchip: vop: Add more RGB output interface type

[Sandy Huang]

This patch add serial RGB output interface for rockchip vop, the
more info about serial RGB output interface described at the
following file:

Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt

Signed-off-by: Sandy Huang <hjc@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index 56bbd2e..59a01c3 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -209,6 +209,8 @@ struct vop_data {
 #define ROCKCHIP_OUT_MODE_P888	0
 #define ROCKCHIP_OUT_MODE_P666	1
 #define ROCKCHIP_OUT_MODE_P565	2
+#define ROCKCHIP_OUT_MODE_S888	8
+#define ROCKCHIP_OUT_MODE_S888_DUMMY	12
 /* for use special outface */
 #define ROCKCHIP_OUT_MODE_AAAA	15
 
-- 
2.7.4

[PATCH 3/3] drm/rockchip: vop: Add more RGB output interface type

[Sean Paul]

On Thu, Sep 14, 2017 at 11:43:28AM +0800, Sandy Huang wrote:
> This patch add serial RGB output interface for rockchip vop, the
> more info about serial RGB output interface described at the
> following file:
> 
> Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
> 
> Signed-off-by: Sandy Huang <hjc@rock-chips.com>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index 56bbd2e..59a01c3 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -209,6 +209,8 @@ struct vop_data {
>  #define ROCKCHIP_OUT_MODE_P888	0
>  #define ROCKCHIP_OUT_MODE_P666	1
>  #define ROCKCHIP_OUT_MODE_P565	2
> +#define ROCKCHIP_OUT_MODE_S888	8
> +#define ROCKCHIP_OUT_MODE_S888_DUMMY	12
>  /* for use special outface */
>  #define ROCKCHIP_OUT_MODE_AAAA	15
>  
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS