On Mon, Dec 16, 2024 at 11:12:17AM +0800, Damon Ding wrote:
> RK3588 integrates the analogix eDP 1.3 TX controller IP and the HDMI/eDP
> TX Combo PHY based on a Samsung IP block, and there are also two
> independent eDP display interface on RK3588 Soc.
>
> Add just the basic support for now, i.e. RGB output up to 4K@60Hz, without
> the tests of audio, PSR and other eDP 1.3 specific features.
>
> Signed-off-by: Damon Ding <[email protected]>
>
> ---
>
> Changes in v2:
> - Add support for the other eDP output edp1
> ---
> .../gpu/drm/rockchip/analogix_dp-rockchip.c | 82 ++++++++++++++++---
> include/drm/bridge/analogix_dp.h | 3 +-
> 2 files changed, 74 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 871606a31ef1..4c9a55776ada 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -51,10 +51,12 @@ struct rockchip_grf_reg_field {
> /**
> * struct rockchip_dp_chip_data - splite the grf setting of kind of chips
> * @lcdc_sel: grf register field of lcdc_sel
> + * @edp_mode: grf register field of edp_mode
> * @chip_type: specific chip type
> */
> struct rockchip_dp_chip_data {
> const struct rockchip_grf_reg_field lcdc_sel;
> + const struct rockchip_grf_reg_field edp_mode;
> u32 chip_type;
> };
>
> @@ -134,12 +136,21 @@ static int rockchip_dp_poweron(struct
> analogix_dp_plat_data *plat_data)
> return ret;
> }
>
> + ret = rockchip_grf_field_write(dp->grf, &dp->data->edp_mode, 1);
> + if (ret != 0)
> + DRM_DEV_ERROR(dp->dev, "failed to set edp mode %d\n", ret);
Is it to be called for non-eDP hosts too? Or for older hosts?
> +
> return ret;
> }
>
> static int rockchip_dp_powerdown(struct analogix_dp_plat_data *plat_data)
> {
> struct rockchip_dp_device *dp = pdata_encoder_to_dp(plat_data);
> + int ret;
> +
> + ret = rockchip_grf_field_write(dp->grf, &dp->data->edp_mode, 0);
> + if (ret != 0)
> + DRM_DEV_ERROR(dp->dev, "failed to set edp mode %d\n", ret);
Same question.
>
> clk_disable_unprepare(dp->pclk);
>
> @@ -203,6 +214,10 @@ static void rockchip_dp_drm_encoder_enable(struct
> drm_encoder *encoder,
> struct rockchip_dp_device *dp = encoder_to_dp(encoder);
> struct drm_crtc *crtc;
> struct drm_crtc_state *old_crtc_state;
> + struct of_endpoint endpoint;
> + struct device_node *remote_port, *remote_port_parent;
> + char name[32];
> + u32 port_id;
> int ret;
>
> crtc = rockchip_dp_drm_get_new_crtc(encoder, state);
> @@ -220,13 +235,27 @@ static void rockchip_dp_drm_encoder_enable(struct
> drm_encoder *encoder,
> return;
> }
>
> - ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
> + ret = drm_of_encoder_active_endpoint(dp->dev->of_node, encoder,
> &endpoint);
> if (ret < 0)
> return;
>
> - DRM_DEV_DEBUG(dp->dev, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
> + remote_port_parent =
> of_graph_get_remote_port_parent(endpoint.local_node);
> + if (remote_port_parent) {
> + if (of_get_child_by_name(remote_port_parent, "ports")) {
> + remote_port =
> of_graph_get_remote_port(endpoint.local_node);
> + of_property_read_u32(remote_port, "reg", &port_id);
> + of_node_put(remote_port);
> + sprintf(name, "%s vp%d", remote_port_parent->full_name,
> port_id);
> + } else {
> + sprintf(name, "%s %s",
> + remote_port_parent->full_name, endpoint.id ?
> "vopl" : "vopb");
> + }
> + of_node_put(remote_port_parent);
> +
> + DRM_DEV_DEBUG(dp->dev, "%s output to edp\n", name);
> + }
The "output to dp" message got dropped. Is it expected?
>
> - ret = rockchip_grf_field_write(dp->grf, &dp->data->lcdc_sel, ret);
> + ret = rockchip_grf_field_write(dp->grf, &dp->data->lcdc_sel,
> endpoint.id);
> if (ret != 0)
> DRM_DEV_ERROR(dp->dev, "Could not write to GRF: %d\n", ret);
>
> @@ -396,6 +425,7 @@ static int rockchip_dp_probe(struct platform_device *pdev)
> const struct rockchip_dp_chip_data *dp_data;
> struct drm_panel *panel = NULL;
> struct rockchip_dp_device *dp;
> + int id, i;
> int ret;
>
> dp_data = of_device_get_match_data(dev);
> @@ -410,9 +440,22 @@ static int rockchip_dp_probe(struct platform_device
> *pdev)
> if (!dp)
> return -ENOMEM;
>
> + id = of_alias_get_id(dev->of_node, "edp");
> + if (id < 0)
> + id = 0;
Alias is not documented.
Please check how other platforms handle device -> ID conversion and
consider following it (yes, the best option currently known is to
hardcode bus addresses into the driver data).
> +
> + i = 0;
> + while (is_rockchip(dp_data[i].chip_type))
> + i++;
> +
> + if (id >= i) {
> + DRM_DEV_ERROR(dev, "invalid edp id: %d\n", id);
> + return -ENODEV;
> + }
Is it applicable to non-eDP case?
> +
> dp->dev = dev;
> dp->adp = ERR_PTR(-ENODEV);
> - dp->data = dp_data;
> + dp->data = &dp_data[id];
> dp->plat_data.panel = panel;
> dp->plat_data.dev_type = dp->data->chip_type;
> dp->plat_data.power_on = rockchip_dp_poweron;
> @@ -464,19 +507,38 @@ static int rockchip_dp_resume(struct device *dev)
> static DEFINE_RUNTIME_DEV_PM_OPS(rockchip_dp_pm_ops, rockchip_dp_suspend,
> rockchip_dp_resume, NULL);
>
> -static const struct rockchip_dp_chip_data rk3399_edp = {
> - .lcdc_sel = GRF_REG_FIELD(0x6250, 5, 5),
> - .chip_type = RK3399_EDP,
> +static const struct rockchip_dp_chip_data rk3399_edp[] = {
> + {
> + .lcdc_sel = GRF_REG_FIELD(0x6250, 5, 5),
> + .chip_type = RK3399_EDP,
> + },
> + { /* sentinel */ }
> +};
> +
> +static const struct rockchip_dp_chip_data rk3288_dp[] = {
> + {
> + .lcdc_sel = GRF_REG_FIELD(0x025c, 5, 5),
> + .chip_type = RK3288_DP,
> + },
> + { /* sentinel */ }
> };
>
> -static const struct rockchip_dp_chip_data rk3288_dp = {
> - .lcdc_sel = GRF_REG_FIELD(0x025c, 5, 5),
> - .chip_type = RK3288_DP,
> +static const struct rockchip_dp_chip_data rk3588_edp[] = {
> + {
> + .edp_mode = GRF_REG_FIELD(0x0000, 0, 0),
> + .chip_type = RK3588_EDP,
> + },
> + {
> + .edp_mode = GRF_REG_FIELD(0x0004, 0, 0),
> + .chip_type = RK3588_EDP,
> + },
> + { /* sentinel */ }
> };
>
> static const struct of_device_id rockchip_dp_dt_ids[] = {
> {.compatible = "rockchip,rk3288-dp", .data = &rk3288_dp },
> {.compatible = "rockchip,rk3399-edp", .data = &rk3399_edp },
> + {.compatible = "rockchip,rk3588-edp", .data = &rk3588_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 6002c5666031..54086cb2d97d 100644
> --- a/include/drm/bridge/analogix_dp.h
> +++ b/include/drm/bridge/analogix_dp.h
> @@ -15,11 +15,12 @@ enum analogix_dp_devtype {
> EXYNOS_DP,
> RK3288_DP,
> RK3399_EDP,
> + RK3588_EDP,
> };
>
> static inline bool is_rockchip(enum analogix_dp_devtype type)
> {
> - return type == RK3288_DP || type == RK3399_EDP;
> + return type == RK3288_DP || type == RK3399_EDP || type == RK3588_EDP;
> }
>
> struct analogix_dp_plat_data {
> --
> 2.34.1
>
--
With best wishes
Dmitry
