Re: [PATCH v2] drm/rockchip: Use an enum to identify Rockchip VOPs

Mark yao <mark.yao@xxxxxxxxxxxxxx> · Thu, 19 Oct 2017 08:49:46 +0800

On 2017年10月19日 01:52, Brian Norris wrote:
Hi Kristian,

On Thu, Nov 03, 2016 at 12:46:48PM -0700, Kristian Högsberg wrote:
We used to call drm_of_encoder_active_endpoint_id() from
rockchip_dp_drm_encoder_atomic_check() to determine the endpoint for the
active encoder. However, the encoder isn't necessarily active at this
point or it may be connected to different crtc than what we're switching
to. Instead, look at the crtc from the drm_crtc_state.  This fixes wrong
colors when driving the eDP with the big VOP.  Further, we can identify
the type of VOP we're dealing with by just putting a VOP id enum in the
vop_data.

Unfortunately, vop iomux and vop data are not one-to-one correspondence.

Such as rk3288, vop big and vop little both use rk3288_vop data, if you use vop data id will not be able
to distinguish between them.

Mark

On way to test this is to use the modetest tool from libdrm:

   $ modetest -M rockchip -s 32@28:2400x1600

which displays dark or black colors because we fail to look up the
endpoint and use default 0 (which is ROCKCHIP_OUT_MODE_P888) for big
VOP instead of RGB10 as required.

For reference,

   $ modetest -M rockchip -s 32@25:2400x1600

drives the eDP from little VOP and displays correctly.

Signed-off-by: Kristian H. Kristensen <hoegsberg@xxxxxxxxxxxx>
---
v2: Stripped chromeos annotations, fix compile errors for drivers I didn't
compile when I first wrote the patch.
Do you plan to fix the errors Guenter pointed out and resend? This is
still causing conflicts between our ChromeOS kernel trees and upstream.

Also, please CC linux-rockchip on Rockchip-related patches. It's much
easier for me to find your work that way :)

Brian

drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 45 ++++++++++---------------
  drivers/gpu/drm/rockchip/cdn-dp-core.c          | 19 +++++------
  drivers/gpu/drm/rockchip/dw-mipi-dsi.c          | 12 ++++---
  drivers/gpu/drm/rockchip/rockchip_drm_vop.c     |  7 ++++
  drivers/gpu/drm/rockchip/rockchip_drm_vop.h     | 10 ++++++
  drivers/gpu/drm/rockchip/rockchip_vop_reg.c     |  4 +++
  6 files changed, 56 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 558a3bc..9ae4a9c 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -196,14 +196,16 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder)
  	int ret;
  	u32 val;
  
-	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
-	if (ret < 0)
-		return;
-
-	if (ret)
+	switch (vop_get_crtc_vop_id(encoder->crtc)) {
+	case RK3399_VOP_LIT:
  		val = dp->data->lcdsel_lit;
-	else
+		break;
+	case RK3399_VOP_BIG:
  		val = dp->data->lcdsel_big;
+		break;
+	default:
+		return;
+	}
  
  	dev_dbg(dp->dev, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
  
@@ -231,34 +233,23 @@ 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;
  
-	/*
-	 * The hardware IC designed that VOP must output the RGB10 video
-	 * format to eDP contoller, and if eDP panel only support RGB8,
-	 * then eDP contoller should cut down the video data, not via VOP
-	 * contoller, that's why we need to hardcode the VOP output mode
-	 * to RGA10 here.
-	 */
-
-	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
-	if (ret < 0)
-		return 0;
-
-	switch (dp->data->chip_type) {
-	case RK3399_EDP:
+	switch (vop_get_crtc_vop_id(crtc_state->crtc)) {
+	case RK3399_VOP_LIT:
  		/*
  		 * 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;
+		s->output_mode = ROCKCHIP_OUT_MODE_P888;
  		break;
-
  	default:
+		/*
+		 * The hardware IC designed that VOP must output the RGB10 video
+		 * format to eDP contoller, and if eDP panel only support RGB8,
+		 * then eDP contoller should cut down the video data, not via VOP
+		 * contoller, that's why we need to hardcode the VOP output mode
+		 * to RGA10 here.
+		 */
  		s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
  		break;
  	}
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index f020e2e..b9e6184 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -570,21 +570,20 @@ static void cdn_dp_encoder_mode_set(struct drm_encoder *encoder,
  	video->v_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NVSYNC);
  	video->h_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NHSYNC);
  
-	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
-	if (ret < 0) {
-		DRM_DEV_ERROR(dp->dev, "Could not get vop id, %d", ret);
-		return;
-	}
-
-	DRM_DEV_DEBUG_KMS(dp->dev, "vop %s output to cdn-dp\n",
-			  (ret) ? "LIT" : "BIG");
  	state = to_rockchip_crtc_state(encoder->crtc->state);
-	if (ret) {
+	switch (vop_get_crtc_vop_id(encoder->crtc)) {
+	case RK3399_VOP_LIT:
+		DRM_DEV_DEBUG_KMS(dp->dev, "vop LIT output to cdn-dp\n");
  		val = DP_SEL_VOP_LIT | (DP_SEL_VOP_LIT << 16);
  		state->output_mode = ROCKCHIP_OUT_MODE_P888;
-	} else {
+		break;
+	case RK3399_VOP_BIG:
+		DRM_DEV_DEBUG_KMS(dp->dev, "vop BIG output to cdn-dp\n");
  		val = DP_SEL_VOP_LIT << 16;
  		state->output_mode = ROCKCHIP_OUT_MODE_AAAA;
+		break;
+	default:
+		break;
  	}
  
  	ret = cdn_dp_grf_write(dp, GRF_SOC_CON9, val);
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index dedc65b..b01a82a 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -878,7 +878,6 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
  static void dw_mipi_dsi_encoder_commit(struct drm_encoder *encoder)
  {
  	struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
-	int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
  	u32 val;
  
  	if (clk_prepare_enable(dsi->pclk)) {
@@ -894,13 +893,18 @@ static void dw_mipi_dsi_encoder_commit(struct drm_encoder *encoder)
  
  	clk_disable_unprepare(dsi->pclk);
  
-	if (mux)
+	switch (vop_get_crtc_vop_id(encoder->crtc)) {
+	case RK3399_VOP_LIT:
+		dev_dbg(dsi->dev, "vop LIT output to dsi0\n");
  		val = DSI0_SEL_VOP_LIT | (DSI0_SEL_VOP_LIT << 16);
-	else
+		break;
+	default:
+		dev_dbg(dsi->dev, "vop BIG output to dsi0\n");
  		val = DSI0_SEL_VOP_LIT << 16;
+		break;
+	}
  
  	regmap_write(dsi->grf_regmap, GRF_SOC_CON6, val);
-	dev_dbg(dsi->dev, "vop %s output to dsi0\n", (mux) ? "LIT" : "BIG");
  }
  
  static int
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index d5ea4ab..e849f26 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -141,6 +141,13 @@ struct vop {
  	struct vop_win win[];
  };
  
+enum vop_id vop_get_crtc_vop_id(struct drm_crtc *crtc)
+{
+	struct vop *vop = to_vop(crtc);
+
+	return vop->data->id;
+}
+
  static inline void vop_writel(struct vop *vop, uint32_t offset, uint32_t v)
  {
  	writel(v, vop->regs + offset);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index 5a4faa85..2c54bcc 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -135,8 +135,16 @@ struct vop_win_data {
  	enum drm_plane_type type;
  };
  
+enum vop_id {
+	RK3036_VOP,
+	RK3288_VOP,
+	RK3399_VOP_BIG,
+	RK3399_VOP_LIT,
+};
+
  struct vop_data {
  	const struct vop_reg_data *init_table;
+	uint32_t id;
  	unsigned int table_size;
  	const struct vop_ctrl *ctrl;
  	const struct vop_intr *intr;
@@ -321,5 +329,7 @@ static inline int scl_vop_cal_lb_mode(int width, bool is_yuv)
  	return lb_mode;
  }
  
+enum vop_id vop_get_crtc_vop_id(struct drm_crtc *crtc);
+
  extern const struct component_ops vop_component_ops;
  #endif /* _ROCKCHIP_DRM_VOP_H */
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index aaede6b..a46e2c8 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -131,6 +131,7 @@ static const struct vop_reg_data rk3036_vop_init_reg_table[] = {
  };
  
  static const struct vop_data rk3036_vop = {
+	.id = RK3036_VOP,
  	.init_table = rk3036_vop_init_reg_table,
  	.table_size = ARRAY_SIZE(rk3036_vop_init_reg_table),
  	.ctrl = &rk3036_ctrl_data,
@@ -272,6 +273,7 @@ static const struct vop_intr rk3288_vop_intr = {
  };
  
  static const struct vop_data rk3288_vop = {
+	.id = RK3288_VOP,
  	.init_table = rk3288_init_reg_table,
  	.table_size = ARRAY_SIZE(rk3288_init_reg_table),
  	.intr = &rk3288_vop_intr,
@@ -340,6 +342,7 @@ static const struct vop_reg_data rk3399_init_reg_table[] = {
  };
  
  static const struct vop_data rk3399_vop_big = {
+	.id = RK3399_VOP_BIG,
  	.init_table = rk3399_init_reg_table,
  	.table_size = ARRAY_SIZE(rk3399_init_reg_table),
  	.intr = &rk3399_vop_intr,
@@ -359,6 +362,7 @@ static const struct vop_win_data rk3399_vop_lit_win_data[] = {
  };
  
  static const struct vop_data rk3399_vop_lit = {
+	.id = RK3399_VOP_LIT,
  	.init_table = rk3399_init_reg_table,
  	.table_size = ARRAY_SIZE(rk3399_init_reg_table),
  	.intr = &rk3399_vop_intr,
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-rockchip




_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel







