[PATCH v2] drm/rockchip: Use an enum to identify Rockchip VOPs
Mark yao
mark.yao at rock-chips.com
Fri Oct 20 01:38:57 UTC 2017
On 2017年10月20日 01:46, Kristian Høgsberg wrote:
> 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.
The display issue already be fixed, and your kernel seems old
See the patch: http://patchwork.freedesktop.org/patch/msgid/1495885416-22216-1-git-send-email-mark.yao@rock-chips.com
Now eDP always set its outmode as ROCKCHIP_OUT_MODE_AAAA.
And on vop driver, force RGB10 to RGB888 if vop not support rgb10bit.
drivers/gpu/drm/rockchip/rockchip_drm_vop.c:
/*
* if vop is not support RGB10 output, need force RGB10 to RGB888.
*/
if (s->output_mode == ROCKCHIP_OUT_MODE_AAAA &&
!(vop_data->feature & VOP_FEATURE_OUTPUT_RGB10))
s->output_mode = ROCKCHIP_OUT_MODE_P888;
Thanks.
>>>>
>>>> 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
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
--
Ｍark Yao
More information about the dri-devel
mailing list