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
>
>
>
