diff mbox

[v2,05/10] drm/rockchip: analogix_dp: add rk3399 eDP support

Message ID 1464073043-4931-1-git-send-email-ykk@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yakir Yang May 24, 2016, 6:57 a.m. UTC
RK3399 and RK3288 shared the same eDP IP controller, only some light
difference with VOP configure and GRF configure.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v2:
- rebase with drm-next, fix some conflicts

 .../bindings/display/bridge/analogix_dp.txt        |  1 +
 .../display/rockchip/analogix_dp-rockchip.txt      |  2 +-
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c    | 36 ++++++++++++++++++++--
 include/drm/bridge/analogix_dp.h                   |  1 +
 4 files changed, 37 insertions(+), 3 deletions(-)

Comments

Heiko Stübner May 24, 2016, 10:17 a.m. UTC | #1
Am Dienstag, 24. Mai 2016, 14:57:23 schrieb Yakir Yang:
> RK3399 and RK3288 shared the same eDP IP controller, only some light
> difference with VOP configure and GRF configure.
> 
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> ---
> Changes in v2:
> - rebase with drm-next, fix some conflicts
> 
>  .../bindings/display/bridge/analogix_dp.txt        |  1 +
>  .../display/rockchip/analogix_dp-rockchip.txt      |  2 +-
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c    | 36
> ++++++++++++++++++++-- include/drm/bridge/analogix_dp.h                  
> |  1 +
>  4 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
> b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt index
> 4f2ba8c..4a0f4f7 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
> @@ -5,6 +5,7 @@ Required properties for dp-controller:
>  		platform specific such as:
>  		 * "samsung,exynos5-dp"
>  		 * "rockchip,rk3288-dp"
> +		 * "rockchip,rk3399-edp"

the cleanlines-freak in my likes to know if there is a difference between 
the rk3399 being called -edp here and -dp on the rk3288 :-)

[...]

> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 29c4105..d5d4e04
> 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -148,6 +148,10 @@ rockchip_dp_drm_encoder_atomic_check(struct
> drm_encoder *encoder, struct drm_connector_state *conn_state)
>  {
>  	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
> +	struct rockchip_dp_device *dp = to_dp(encoder);
> +	int ret;
> +
> +	s->output_type = DRM_MODE_CONNECTOR_eDP;
> 
>  	/*
>  	 * FIXME(Yakir): driver should configure the CRTC output video
> @@ -162,8 +166,27 @@ rockchip_dp_drm_encoder_atomic_check(struct
> drm_encoder *encoder, * But if I configure CTRC to RGBaaa, and eDP driver
> still keep * RGB666 input video mode, then screen would works prefect.
>  	 */
> -	s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
> -	s->output_type = DRM_MODE_CONNECTOR_eDP;
> +
> +	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
> +	if (ret < 0)
> +		return;

this needs a value returned (probably ret), otherwise you create a 
warning: ‘return’ with no value, in function returning non-void

drm_of_encoder_active_endpoint_id also always returns -EINVAL on rk3288-
veyron-jerry because encoder->crtc is unset in 
drm_of_encoder_active_endpoint and that breaks display output right now.

Looking through drm code it seems only two functions would set encoder->crtc
- drm_atomic_helper_update_legacy_modeset_state
- drm_crtc_helper_set_config

drm_crtc_helper_set_config callback got dropped in the atomic-conversion and 
the other sounds to be somewhat in the legacy area.

After that drm-internals get a bit confusing.


Heiko
Doug Anderson May 24, 2016, 6:12 p.m. UTC | #2
Hi,

On Tue, May 24, 2016 at 3:17 AM, Heiko Stuebner <heiko@sntech.de> wrote:
>> --- a/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
>> @@ -5,6 +5,7 @@ Required properties for dp-controller:
>>               platform specific such as:
>>                * "samsung,exynos5-dp"
>>                * "rockchip,rk3288-dp"
>> +              * "rockchip,rk3399-edp"
>
> the cleanlines-freak in my likes to know if there is a difference between
> the rk3399 being called -edp here and -dp on the rk3288 :-)
>
> [...]

If I was a guessing man (which I'm not, but if I was), I'd guess that
this is because on rk3288 there was only one DP port and it was an EDP
port.  ...but since there was only one people just referred to it as
the "DP" port and that was codified in the bindings.  On rk3399 there
are two ports: one EDP and one DP.  All of a sudden we need to
differentiate.

Any better suggestions for how to deal with that?


-Doug
Heiko Stübner May 24, 2016, 6:23 p.m. UTC | #3
Am Dienstag, 24. Mai 2016, 11:12:20 schrieb Doug Anderson:
> Hi,
> 
> On Tue, May 24, 2016 at 3:17 AM, Heiko Stuebner <heiko@sntech.de> wrote:
> >> --- a/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
> >> +++ b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
> >> 
> >> @@ -5,6 +5,7 @@ Required properties for dp-controller:
> >>               platform specific such as:
> >>                * "samsung,exynos5-dp"
> >>                * "rockchip,rk3288-dp"
> >> 
> >> +              * "rockchip,rk3399-edp"
> > 
> > the cleanlines-freak in my likes to know if there is a difference
> > between
> > the rk3399 being called -edp here and -dp on the rk3288 :-)
> > 
> > [...]
> 
> If I was a guessing man (which I'm not, but if I was), I'd guess that
> this is because on rk3288 there was only one DP port and it was an EDP
> port.  ...but since there was only one people just referred to it as
> the "DP" port and that was codified in the bindings.  On rk3399 there
> are two ports: one EDP and one DP.  All of a sudden we need to
> differentiate.
> 
> Any better suggestions for how to deal with that?

nope - everything as it should be in that regard then - the rk3288-dp is 
permanent now anyway.

But it looks like I'm sort of blind blind and only now have seen the 
separate DP controller chapter in the TRM (probably because I somehow didn't 
expect a second controller).
Yakir Yang May 25, 2016, 5:54 a.m. UTC | #4
On 05/25/2016 02:23 AM, Heiko Stuebner wrote:
> Am Dienstag, 24. Mai 2016, 11:12:20 schrieb Doug Anderson:
>> Hi,
>>
>> On Tue, May 24, 2016 at 3:17 AM, Heiko Stuebner <heiko@sntech.de> wrote:
>>>> --- a/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
>>>>
>>>> @@ -5,6 +5,7 @@ Required properties for dp-controller:
>>>>                platform specific such as:
>>>>                 * "samsung,exynos5-dp"
>>>>                 * "rockchip,rk3288-dp"
>>>>
>>>> +              * "rockchip,rk3399-edp"
>>> the cleanlines-freak in my likes to know if there is a difference
>>> between
>>> the rk3399 being called -edp here and -dp on the rk3288 :-)
>>>
>>> [...]
>> If I was a guessing man (which I'm not, but if I was), I'd guess that
>> this is because on rk3288 there was only one DP port and it was an EDP
>> port.  ...but since there was only one people just referred to it as
>> the "DP" port and that was codified in the bindings.  On rk3399 there
>> are two ports: one EDP and one DP.  All of a sudden we need to
>> differentiate.
>>
>> Any better suggestions for how to deal with that?
> nope - everything as it should be in that regard then - the rk3288-dp is
> permanent now anyway.
>
> But it looks like I'm sort of blind blind and only now have seen the
> separate DP controller chapter in the TRM (probably because I somehow didn't
> expect a second controller).
RK3399 eDP controller have cut off the DP-Audio function and DP-HDCP 
function, so it can't be treated as DP interfaces any more, that's why I 
call it "eDP".
>
>
Yakir Yang May 25, 2016, 5:55 a.m. UTC | #5
On 05/24/2016 06:17 PM, Heiko Stuebner wrote:
> Am Dienstag, 24. Mai 2016, 14:57:23 schrieb Yakir Yang:
> [........]
>> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 29c4105..d5d4e04
>> 100644
>> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> @@ -148,6 +148,10 @@ rockchip_dp_drm_encoder_atomic_check(struct
>> drm_encoder *encoder, struct drm_connector_state *conn_state)
>>   {
>>   	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
>> +	struct rockchip_dp_device *dp = to_dp(encoder);
>> +	int ret;
>> +
>> +	s->output_type = DRM_MODE_CONNECTOR_eDP;
>>
>>   	/*
>>   	 * FIXME(Yakir): driver should configure the CRTC output video
>> @@ -162,8 +166,27 @@ rockchip_dp_drm_encoder_atomic_check(struct
>> drm_encoder *encoder, * But if I configure CTRC to RGBaaa, and eDP driver
>> still keep * RGB666 input video mode, then screen would works prefect.
>>   	 */
>> -	s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
>> -	s->output_type = DRM_MODE_CONNECTOR_eDP;
>> +
>> +	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
>> +	if (ret < 0)
>> +		return;
> this needs a value returned (probably ret), otherwise you create a
> warning: ‘return’ with no value, in function returning non-void
Done,

Thanks,
- Yakir
>
> drm_of_encoder_active_endpoint_id also always returns -EINVAL on rk3288-
> veyron-jerry because encoder->crtc is unset in
> drm_of_encoder_active_endpoint and that breaks display output right now.
>
> Looking through drm code it seems only two functions would set encoder->crtc
> - drm_atomic_helper_update_legacy_modeset_state
> - drm_crtc_helper_set_config
>
> drm_crtc_helper_set_config callback got dropped in the atomic-conversion and
> the other sounds to be somewhat in the legacy area.
>
> After that drm-internals get a bit confusing.
>
>
> Heiko
>
>
>
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
index 4f2ba8c..4a0f4f7 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
+++ b/Documentation/devicetree/bindings/display/bridge/analogix_dp.txt
@@ -5,6 +5,7 @@  Required properties for dp-controller:
 		platform specific such as:
 		 * "samsung,exynos5-dp"
 		 * "rockchip,rk3288-dp"
+		 * "rockchip,rk3399-edp"
 	-reg:
 		physical base address of the controller and length
 		of memory mapped region.
diff --git a/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt b/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
index e832ff9..5ae55ca 100644
--- a/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
@@ -2,7 +2,7 @@  Rockchip RK3288 specific extensions to the Analogix Display Port
 ================================
 
 Required properties:
-- compatible: "rockchip,rk3288-edp";
+- compatible: "rockchip,rk3288-edp" or "rockchip,rk3399-edp";
 
 - reg: physical base address of the controller and length
 
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 29c4105..d5d4e04 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -148,6 +148,10 @@  rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder,
 				      struct drm_connector_state *conn_state)
 {
 	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
+	struct rockchip_dp_device *dp = to_dp(encoder);
+	int ret;
+
+	s->output_type = DRM_MODE_CONNECTOR_eDP;
 
 	/*
 	 * FIXME(Yakir): driver should configure the CRTC output video
@@ -162,8 +166,27 @@  rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder,
 	 * But if I configure CTRC to RGBaaa, and eDP driver still keep
 	 * RGB666 input video mode, then screen would works prefect.
 	 */
-	s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
-	s->output_type = DRM_MODE_CONNECTOR_eDP;
+
+	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
+	if (ret < 0)
+		return;
+
+	switch (dp->data->chip_type) {
+	case RK3399_EDP:
+		/*
+		 * For RK3399, VOP Lit must code the out mode to RGB888,
+		 * VOP Big must code the out mode to RGB10.
+		 */
+		if (ret)
+			s->output_mode = ROCKCHIP_OUT_MODE_P888;
+		else
+			s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
+		break;
+
+	default:
+		s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
+		break;
+	}
 
 	return 0;
 }
@@ -377,6 +400,14 @@  static const struct dev_pm_ops rockchip_dp_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(rockchip_dp_suspend, rockchip_dp_resume)
 };
 
+static const struct rockchip_dp_chip_data rk3399_edp = {
+	.lcdsel_grf_reg = 0x6250,
+	.lcdsel_big = 0,
+	.lcdsel_lit = BIT(5),
+	.lcdsel_mask = BIT(21),
+	.chip_type = RK3399_EDP,
+};
+
 static const struct rockchip_dp_chip_data rk3288_dp = {
 	.lcdsel_grf_reg = 0x025c,
 	.lcdsel_big = 0,
@@ -387,6 +418,7 @@  static const struct rockchip_dp_chip_data rk3288_dp = {
 
 static const struct of_device_id rockchip_dp_dt_ids[] = {
 	{.compatible = "rockchip,rk3288-dp", .data = &rk3288_dp },
+	{.compatible = "rockchip,rk3399-edp", .data = &rk3399_edp },
 	{}
 };
 MODULE_DEVICE_TABLE(of, rockchip_dp_dt_ids);
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 06c0250..82b8135 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -20,6 +20,7 @@  enum analogix_dp_devtype {
 
 enum analogix_dp_sub_devtype {
 	RK3288_DP,
+	RK3399_EDP,
 };
 
 struct analogix_dp_plat_data {