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

Mark yao mark.yao at rock-chips.com
Thu Oct 19 00:49:46 UTC 2017


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 at 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 at 25:2400x1600
>>
>> drives the eDP from little VOP and displays correctly.
>>
>> Signed-off-by: Kristian H. Kristensen <hoegsberg at chromium.org>
>> ---
>> 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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
>



More information about the dri-devel mailing list