Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH v2 0/10] Add RK3399 eDP support and fix some bugs to analogix_dp driver.

64 views
Skip to first unread message

Yakir Yang

unread,
May 24, 2016, 1:10:06 AM5/24/16
to

Hi all,

This series have been posted about one month, still no comments, help here :(

RK3399 and RK3288 shared the same eDP IP controller, only some light
difference with VOP configure and GRF configure.

Also same misc fix to analogix_dp driver:
- Hotplug invalid which report by Dan Carpenter
- Make panel detect to an optional action
- correct the register bit define error in ANALOGIX_DP_PLL_REG_1


Changes in v2:
- new patch in v2
- rebase with drm-next, fix some conflicts
- new patch in v2

Yakir Yang (10):
drm/bridge: analogix_dp: rename RK3288_DP to ROCKCHIP_DP
drm/rockchip: analogix_dp: split the lcdc select setting into device
data
drm/bridge: analogix_dp: correct the register bit define error in
ANALOGIX_DP_PLL_REG_1
drm/bridge: analogix_dp: some rockchip chips need to flip REF_CLK bit
setting
drm/rockchip: analogix_dp: add rk3399 eDP support
drm/rockchip: analogix_dp: make panel detect to an optional action
drm/bridge: analogix_dp: introduce connector mode_valid callback to
plat driver
drm/rockchip: analogix_dp: correct the connector display color format
and bpc
drm/rockchip: analogix_dp: update the comments about why need to
hardcode VOP output mode
drm/bridge: analogix_dp: fix no drm hpd event when panel plug in

.../bindings/display/bridge/analogix_dp.txt | 1 +
.../display/rockchip/analogix_dp-rockchip.txt | 2 +-
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 19 ++-
drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 8 +-
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 12 +-
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h | 5 +-
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 157 ++++++++++++++-------
include/drm/bridge/analogix_dp.h | 10 ++
8 files changed, 152 insertions(+), 62 deletions(-)

--
1.9.1

Yakir Yang

unread,
May 24, 2016, 1:10:06 AM5/24/16
to
The enum value of DP_IRQ_TYPE_HP_CABLE_IN is zero, but driver only
send drm hp event when the irq_type and the enum value is true.

if (irq_type & DP_IRQ_TYPE_HP_CABLE_IN || ...)
drm_helper_hpd_irq_event(dp->drm_dev);

So there would no drm hpd event when cable plug in, to fix that
just need to assign all hotplug enum with no-zero values.

Reported-by: Dan Carpenter <dan.ca...@oracle.com>
Signed-off-by: Yakir Yang <y...@rock-chips.com>
---
Changes in v2: None

drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index f09275d..b456380 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -127,10 +127,10 @@ enum analog_power_block {
};

enum dp_irq_type {
- DP_IRQ_TYPE_HP_CABLE_IN,
- DP_IRQ_TYPE_HP_CABLE_OUT,
- DP_IRQ_TYPE_HP_CHANGE,
- DP_IRQ_TYPE_UNKNOWN,
+ DP_IRQ_TYPE_HP_CABLE_IN = BIT(0),
+ DP_IRQ_TYPE_HP_CABLE_OUT = BIT(1),
+ DP_IRQ_TYPE_HP_CHANGE = BIT(2),
+ DP_IRQ_TYPE_UNKNOWN = BIT(3),
};

struct video_info {
--
1.9.1

Yakir Yang

unread,
May 24, 2016, 1:10:06 AM5/24/16
to
As vendor document indicate, when REF_CLK bit set 0, then DP
phy's REF_CLK should switch to 24M source clock.

But due to IC PHY layout mistaken, some chips need to flip this
bit(like RK3288), and unfortunately they didn't indicate in the
DP version register. That's why we have to make this little hack.

Signed-off-by: Yakir Yang <y...@rock-chips.com>
---
Changes in v2:
- new patch in v2

drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 6 +++++-
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h | 1 +
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 3 +++
include/drm/bridge/analogix_dp.h | 5 +++++
4 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
index 931a76c..31366bf 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -75,7 +75,11 @@ void analogix_dp_init_analog_param(struct analogix_dp_device *dp)
writel(reg, dp->reg_base + ANALOGIX_DP_ANALOG_CTL_2);

if (dp->plat_data && (dp->plat_data->dev_type == ROCKCHIP_DP)) {
- writel(REF_CLK_24M, dp->reg_base + ANALOGIX_DP_PLL_REG_1);
+ reg = REF_CLK_24M;
+ if (dp->plat_data->subdev_type == RK3288_DP)
+ reg = ~reg & REF_CLK_MASK;
+
+ writel(reg, dp->reg_base + ANALOGIX_DP_PLL_REG_1);
writel(0x95, dp->reg_base + ANALOGIX_DP_PLL_REG_2);
writel(0x40, dp->reg_base + ANALOGIX_DP_PLL_REG_3);
writel(0x58, dp->reg_base + ANALOGIX_DP_PLL_REG_4);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
index 88d56ad..cdcc6c5 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
@@ -165,6 +165,7 @@
/* ANALOGIX_DP_PLL_REG_1 */
#define REF_CLK_24M (0x1 << 0)
#define REF_CLK_27M (0x0 << 0)
+#define REF_CLK_MASK (0x1 << 0)

/* ANALOGIX_DP_LANE_MAP */
#define LANE3_MAP_LOGIC_LANE_0 (0x0 << 6)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 260c43f..29c4105 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -41,6 +41,7 @@ struct rockchip_dp_chip_data {
u32 lcdsel_big;
u32 lcdsel_lit;
u32 lcdsel_mask;
+ u32 chip_type;
};

struct rockchip_dp_device {
@@ -281,6 +282,7 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
dp->plat_data.encoder = &dp->encoder;

dp->plat_data.dev_type = ROCKCHIP_DP;
+ dp->plat_data.subdev_type = dp_data->chip_type;
dp->plat_data.power_on = rockchip_dp_poweron;
dp->plat_data.power_off = rockchip_dp_powerdown;

@@ -380,6 +382,7 @@ static const struct rockchip_dp_chip_data rk3288_dp = {
.lcdsel_big = 0,
.lcdsel_lit = BIT(5),
.lcdsel_mask = BIT(21),
+ .chip_type = RK3288_DP,
};

static const struct of_device_id rockchip_dp_dt_ids[] = {
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 9e5d013..06c0250 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -18,8 +18,13 @@ enum analogix_dp_devtype {
ROCKCHIP_DP,
};

+enum analogix_dp_sub_devtype {
+ RK3288_DP,
+};
+
struct analogix_dp_plat_data {
enum analogix_dp_devtype dev_type;
+ enum analogix_dp_sub_devtype subdev_type;
struct drm_panel *panel;
struct drm_encoder *encoder;
struct drm_connector *connector;
--
1.9.1

Yakir Yang

unread,
May 24, 2016, 1:10:06 AM5/24/16
to
It's helpful to expand the mode_valid callback to platform driver,
so they could valid the display mode or information.

Signed-off-by: Yakir Yang <y...@rock-chips.com>
---
Changes in v2: None

drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 15 +++++++++++++++
include/drm/bridge/analogix_dp.h | 4 ++++
2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 4a1b3b8..5af9ce4 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -943,6 +943,20 @@ int analogix_dp_get_modes(struct drm_connector *connector)
return num_modes;
}

+static enum drm_mode_status
+analogix_dp_mode_valid(struct drm_connector *connector,
+ struct drm_display_mode *mode)
+{
+ struct analogix_dp_device *dp = to_dp(connector);
+ enum drm_mode_status status = MODE_OK;
+
+ if (dp->plat_data->mode_valid)
+ status = dp->plat_data->mode_valid(dp->plat_data, connector,
+ mode);
+
+ return status;
+}
+
static struct drm_encoder *
analogix_dp_best_encoder(struct drm_connector *connector)
{
@@ -954,6 +968,7 @@ analogix_dp_best_encoder(struct drm_connector *connector)
static const struct drm_connector_helper_funcs analogix_dp_connector_helper_funcs = {
.get_modes = analogix_dp_get_modes,
.best_encoder = analogix_dp_best_encoder,
+ .mode_valid = analogix_dp_mode_valid,
};

enum drm_connector_status
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 82b8135..9ef89de 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -35,6 +35,10 @@ struct analogix_dp_plat_data {
int (*attach)(struct analogix_dp_plat_data *, struct drm_bridge *,
struct drm_connector *);
int (*get_modes)(struct analogix_dp_plat_data *);
+
+ enum drm_mode_status (*mode_valid)(struct analogix_dp_plat_data *,
+ struct drm_connector *,
+ struct drm_display_mode *);
};

int analogix_dp_resume(struct device *dev);
--
1.9.1

Yakir Yang

unread,
May 24, 2016, 1:10:07 AM5/24/16
to
Rockchip VOP couldn't output YUV video format for eDP controller, so
when driver detect connector support YUV video format, we need to hack
it down to RGB888.

Signed-off-by: Yakir Yang <y...@rock-chips.com>
---
Changes in v2: None

drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index f29ca3d..910cceb 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -97,6 +97,24 @@ static int rockchip_dp_powerdown(struct analogix_dp_plat_data *plat_data)
return 0;
}

+static enum drm_mode_status
+rockchip_dp_mode_valid(struct analogix_dp_plat_data *plat_data,
+ struct drm_connector *connector,
+ struct drm_display_mode *mode)
+{
+ struct drm_display_info *di = &connector->display_info;
+
+ if (di->color_formats & DRM_COLOR_FORMAT_YCRCB444 ||
+ di->color_formats & DRM_COLOR_FORMAT_YCRCB422) {
+ di->color_formats &= ~(DRM_COLOR_FORMAT_YCRCB422 |
+ DRM_COLOR_FORMAT_YCRCB444);
+ di->color_formats |= DRM_COLOR_FORMAT_RGB444;
+ di->bpc = 8;
+ }
+
+ return MODE_OK;
+}
+
static bool
rockchip_dp_drm_encoder_mode_fixup(struct drm_encoder *encoder,
const struct drm_display_mode *mode,
@@ -306,6 +324,7 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
dp->plat_data.subdev_type = dp_data->chip_type;
dp->plat_data.power_on = rockchip_dp_poweron;
dp->plat_data.power_off = rockchip_dp_powerdown;
+ dp->plat_data.mode_valid = rockchip_dp_mode_valid;

return analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
}
--
1.9.1

Yakir Yang

unread,
May 24, 2016, 1:10:08 AM5/24/16
to
RK3399 and RK3288 shared the same eDP IP controller, only some light
difference with VOP configure and GRF configure.

Signed-off-by: Yakir Yang <y...@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 | 34 ++++++++++++++++++++--
include/drm/bridge/analogix_dp.h | 1 +
4 files changed, 35 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"
-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..d684c97 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -149,6 +149,8 @@ rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder,
{
struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);

+ s->output_type = DRM_MODE_CONNECTOR_eDP;
+
/*
* FIXME(Yakir): driver should configure the CRTC output video
* mode with the display information which indicated the monitor
@@ -162,8 +164,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 +398,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 +416,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 {
--
1.9.1

Yakir Yang

unread,
May 24, 2016, 3:00:05 AM5/24/16
to
RK3399 and RK3288 shared the same eDP IP controller, only some light
difference with VOP configure and GRF configure.

Signed-off-by: Yakir Yang <y...@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"
-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,
@@ -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 = {

Heiko Stuebner

unread,
May 24, 2016, 6:20:06 AM5/24/16
to
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 <y...@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

unread,
May 24, 2016, 2:20:07 PM5/24/16
to
Hi,

On Tue, May 24, 2016 at 3:17 AM, Heiko Stuebner <he...@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 Stuebner

unread,
May 24, 2016, 2:30:07 PM5/24/16
to
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

unread,
May 25, 2016, 2:00:06 AM5/25/16
to


On 05/24/2016 06:17 PM, Heiko Stuebner wrote:
> Am Dienstag, 24. Mai 2016, 14:57:23 schrieb Yakir Yang:
> [........]
Done,

Thanks,
- Yakir

Yakir Yang

unread,
May 25, 2016, 2:00:07 AM5/25/16
to
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

unread,
May 26, 2016, 5:40:06 AM5/26/16
to
Hi Javier,

On 05/24/2016 01:01 PM, Yakir Yang wrote:
> Hi all,
>
> This series have been posted about one month, still no comments, help here :(
This series works rightly on Rockchip platform, and most of them haven't
touch the
common analogix_dp driver (except for the hotplug fixed). So i guess
Exynos platform
should also happy with this changes.

But not sure about that. So, is it possible that you could help to check
this on Exynos
Chromebook, if so i would be very grateful about that.

Thanks,
- Yakir

Javier Martinez Canillas

unread,
May 26, 2016, 8:50:06 AM5/26/16
to
Hello Yakir,

On 05/26/2016 05:34 AM, Yakir Yang wrote:
> Hi Javier,
>
> On 05/24/2016 01:01 PM, Yakir Yang wrote:
>> Hi all,
>>
>> This series have been posted about one month, still no comments, help here :(
> This series works rightly on Rockchip platform, and most of them haven't touch the
> common analogix_dp driver (except for the hotplug fixed). So i guess Exynos platform
> should also happy with this changes.
>
> But not sure about that. So, is it possible that you could help to check this on Exynos
> Chromebook, if so i would be very grateful about that.
>

Of course, I' ll test. Could you please provide me a branch that I can
pull directly to avoid cherry-picking all the patches from the list?

> Thanks,
> - Yakir
>>

Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America

Yakir Yang

unread,
May 27, 2016, 2:20:05 AM5/27/16
to
Hi Javier,

On 05/26/2016 08:48 PM, Javier Martinez Canillas wrote:
> Hello Yakir,
>
> On 05/26/2016 05:34 AM, Yakir Yang wrote:
>> Hi Javier,
>>
>> On 05/24/2016 01:01 PM, Yakir Yang wrote:
>>> Hi all,
>>>
>>> This series have been posted about one month, still no comments, help here :(
>> This series works rightly on Rockchip platform, and most of them haven't touch the
>> common analogix_dp driver (except for the hotplug fixed). So i guess Exynos platform
>> should also happy with this changes.
>>
>> But not sure about that. So, is it possible that you could help to check this on Exynos
>> Chromebook, if so i would be very grateful about that.
>>
> Of course, I' ll test. Could you please provide me a branch that I can
> pull directly to avoid cherry-picking all the patches from the list?
>
Ah, thanks a lot, I do have a tree
https://github.com/yakir-Yang/linux/tree/fromlist/3399-edp

- Yakir

Javier Martinez Canillas

unread,
May 31, 2016, 4:10:08 PM5/31/16
to
Hello Yakir,

On 05/27/2016 02:16 AM, Yakir Yang wrote:
> Hi Javier,
>
> On 05/26/2016 08:48 PM, Javier Martinez Canillas wrote:
>> Hello Yakir,
>>
>> On 05/26/2016 05:34 AM, Yakir Yang wrote:
>>> Hi Javier,
>>>
>>> On 05/24/2016 01:01 PM, Yakir Yang wrote:
>>>> Hi all,
>>>>
>>>> This series have been posted about one month, still no comments, help here :(
>>> This series works rightly on Rockchip platform, and most of them haven't touch the
>>> common analogix_dp driver (except for the hotplug fixed). So i guess Exynos platform
>>> should also happy with this changes.
>>>
>>> But not sure about that. So, is it possible that you could help to check this on Exynos
>>> Chromebook, if so i would be very grateful about that.
>>>
>> Of course, I' ll test. Could you please provide me a branch that I can
>> pull directly to avoid cherry-picking all the patches from the list?
>>
> Ah, thanks a lot, I do have a tree https://github.com/yakir-Yang/linux/tree/fromlist/3399-edp
>

I tested your branch on an Exynos5800 Peach Pi Chromebook and display
is working correctly. So feel free to add for the whole series:

Tested-by: Javier Martinez Canillas <jav...@osg.samsung.com>

Mark yao

unread,
May 31, 2016, 9:50:06 PM5/31/16
to
On 2016年05月24日 13:02, Yakir Yang wrote:
> RK3399 and RK3288 shared the same eDP IP controller, only some light
> difference with VOP configure and GRF configure.
>
> Signed-off-by: Yakir Yang <y...@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 | 34 ++++++++++++++++++++--
> include/drm/bridge/analogix_dp.h | 1 +
> 4 files changed, 35 insertions(+), 3 deletions(-)
>
>
Looks for me, So for the drm/rockchip side:

Acked-by: Mark Yao <mark...@rock-chips.com>

Thanks.

--
Mark Yao

Yakir Yang

unread,
May 31, 2016, 9:50:06 PM5/31/16
to
Javier, Mark, Inki, Jingoo

On 06/01/2016 04:01 AM, Javier Martinez Canillas wrote:
> Hello Yakir,
>
> On 05/27/2016 02:16 AM, Yakir Yang wrote:
>> Hi Javier,
>>
>> On 05/26/2016 08:48 PM, Javier Martinez Canillas wrote:
>>> Hello Yakir,
>>>
>>> On 05/26/2016 05:34 AM, Yakir Yang wrote:
>>>> Hi Javier,
>>>>
>>>> On 05/24/2016 01:01 PM, Yakir Yang wrote:
>>>>> Hi all,
>>>>>
>>>>> This series have been posted about one month, still no comments, help here :(
>>>> This series works rightly on Rockchip platform, and most of them haven't touch the
>>>> common analogix_dp driver (except for the hotplug fixed). So i guess Exynos platform
>>>> should also happy with this changes.
>>>>
>>>> But not sure about that. So, is it possible that you could help to check this on Exynos
>>>> Chromebook, if so i would be very grateful about that.
>>>>
>>> Of course, I' ll test. Could you please provide me a branch that I can
>>> pull directly to avoid cherry-picking all the patches from the list?
>>>
>> Ah, thanks a lot, I do have a tree https://github.com/yakir-Yang/linux/tree/fromlist/3399-edp
>>
> I tested your branch on an Exynos5800 Peach Pi Chromebook and display
> is working correctly. So feel free to add for the whole series:
>
> Tested-by: Javier Martinez Canillas <jav...@osg.samsung.com>
Hi Javier,

Thank you very much, it's great to hear that, and make a big step
forward. :-D


Hi Mark, Inki, Jingoo

Those patches have been tested on Exynos and Rockchip platform rightly, and

This patches have been touched some rockchip_drm and analogix_dp core
code, it would be great to get some reviewed/acked from you. And for now
this
patch-set have been tested on Rockchip and Exynos platform rightly, so
could you
help to share some further comments here ;)

Mark yao

unread,
Jun 1, 2016, 12:00:07 AM6/1/16
to
On 2016年05月24日 13:02, Yakir Yang wrote:
Looks for me, So:

Acked-by: Mark Yao <mark...@rock-chips.com>

--
Mark Yao
0 new messages