| Message ID | 1464073043-4931-1-git-send-email-ykk@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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).
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". > >
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 --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 {
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(-)