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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 1, 2016 at 3:41 PM, Kristian H. Kristensen
<hoegsberg@xxxxxxxxx> 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.
>
> 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.
>
> BUG=chrome-os-partner:56407
> TEST=verify 'modetest -M rockchip -s 32@28:2400x1600' looks right
> Signed-off-by: Kristian H. Kristensen <hoegsberg@xxxxxxxxxxxx>
> Change-Id: If5c5f36bcee09113008ee5155a13337f0e69371f

Thanks for the patch, Kristian. In the future, can you please strip
out BUG/TEST/Change-Id when you upstream? I think checkpatch will
gently remind you of this as well.


> ---
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 45 ++++++++++---------------
>  drivers/gpu/drm/rockchip/cdn-dp-core.c          | 19 +++++------

cdn-dp isn't upstream yet (I need to get that posted), I'll work on
this next week when I'm back from Plumbers. If that goes smoothly, I
can push this patch on top.

Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx>


>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c          |  9 +++--
>  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, 54 insertions(+), 40 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..c2b0f5d 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,10 +893,14 @@ static void dw_mipi_dsi_encoder_commit(struct drm_encoder *encoder)
>
>         clk_disable_unprepare(dsi->pclk);
>
> -       if (mux)
> +       switch (vop_get_crtc_vop_id(crtc_state->crtc)) {
> +       case RK3399_VOP_LIT:
>                 val = DSI0_SEL_VOP_LIT | (DSI0_SEL_VOP_LIT << 16);
> -       else
> +               break;
> +       default:
>                 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");
> 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,
> --
> 2.8.0.rc3.226.g39d4020
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux