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

Kristian Høgsberg hoegsberg at gmail.com
Thu Oct 19 17:46:43 UTC 2017


On Wed, Oct 18, 2017 at 5:49 PM, Mark yao <mark.yao at rock-chips.com> wrote:
> 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.

What would you suggest then? We need this to be able to drive eDP from
the big VOP. We've been carrying this patch for quite a while and it
hasn't regressed neither rk3288 nor rk3399 boards.

Kristian

> 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
>>
>>
>>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


More information about the dri-devel mailing list