On Thu, Nov 3, 2016 at 8:47 AM Sean Paul <seanpaul at chromium.org> wrote:
> On Tue, Nov 1, 2016 at 3:41 PM, Kristian H. Kristensen
> <hoegsberg at gmail.com> 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 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.
> >
> > BUG=chrome-os-partner:56407
> > TEST=verify 'modetest -M rockchip -s 32 at 28:2400x1600' looks right
> > Signed-off-by: Kristian H. Kristensen <hoegsberg at chromium.org>
> > 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.
>
Yup, I sent a v2 with the headers stripped and fixed the compile errors in
the
encoder modules I didn't initially compile.
Kristian
>
>
> > ---
> > 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 at chromium.org>
>
>
> > 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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161103/093a54b4/attachment-0001.html>
