We need to take care of the vop status when use rockchip_drm_crtc_mode_config, if vop is disabled, the function would failed, that is terrible.
Save connector type and output mode on drm_display_mode->private_flags at encoder mode_fixup, then we can configure the type and mode safely on crtc mode_set.
Signed-off-by: Mark Yao mark.yao@rock-chips.com --- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 37 +++++++--------- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 44 ++++++++++--------- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 5 +-- drivers/gpu/drm/rockchip/inno_hdmi.c | 5 +-- drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 - drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 51 +++++++++-------------- drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 6 +++ 7 files changed, 69 insertions(+), 81 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index a1d94d8..0344e52 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -97,7 +97,21 @@ rockchip_dp_drm_encoder_mode_fixup(struct drm_encoder *encoder, const struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode) { - /* do nothing */ + /* + * FIXME(Yakir): driver should configure the CRTC output video + * mode with the display information which indicated the monitor + * support colorimetry. + * + * But don't know why the CRTC driver seems could only output the + * RGBaaa rightly. For example, if connect the "innolux,n116bge" + * eDP screen, EDID would indicated that screen only accepted the + * 6bpc mode. But if I configure CRTC to RGB666 output, then eDP + * screen would show a blue picture (RGB888 show a green picture). + * But if I configure CTRC to RGBaaa, and eDP driver still keep + * RGB666 input video mode, then screen would works prefect. + */ + adjusted_mode->private_flags = ROCKCHIP_DSP_MODE(eDP, AAAA); + return true; }
@@ -114,27 +128,6 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder) int ret; u32 val;
- /* - * FIXME(Yakir): driver should configure the CRTC output video - * mode with the display information which indicated the monitor - * support colorimetry. - * - * But don't know why the CRTC driver seems could only output the - * RGBaaa rightly. For example, if connect the "innolux,n116bge" - * eDP screen, EDID would indicated that screen only accepted the - * 6bpc mode. But if I configure CRTC to RGB666 output, then eDP - * screen would show a blue picture (RGB888 show a green picture). - * But if I configure CTRC to RGBaaa, and eDP driver still keep - * RGB666 input video mode, then screen would works prefect. - */ - ret = rockchip_drm_crtc_mode_config(encoder->crtc, - DRM_MODE_CONNECTOR_eDP, - ROCKCHIP_OUT_MODE_AAAA); - if (ret < 0) { - dev_err(dp->dev, "Could not set crtc mode config (%d)\n", ret); - return; - } - ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder); if (ret < 0) return; diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 7975158..49c4c72 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -875,11 +875,34 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) clk_disable_unprepare(dsi->pclk); }
+static bool dw_mipi_dsi_encoder_mode_fixup(struct drm_encoder *encoder, + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder); + + switch (dsi->format) { + case MIPI_DSI_FMT_RGB888: + adjusted_mode->private_flags = ROCKCHIP_DSP_MODE(DSI, P888); + break; + case MIPI_DSI_FMT_RGB666: + adjusted_mode->private_flags = ROCKCHIP_DSP_MODE(DSI, P666); + break; + case MIPI_DSI_FMT_RGB565: + adjusted_mode->private_flags = ROCKCHIP_DSP_MODE(DSI, P565); + break; + default: + WARN_ON(1); + return false; + } + + return true; +} + 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 interface_pix_fmt; u32 val;
if (clk_prepare_enable(dsi->pclk)) { @@ -895,24 +918,6 @@ static void dw_mipi_dsi_encoder_commit(struct drm_encoder *encoder)
clk_disable_unprepare(dsi->pclk);
- switch (dsi->format) { - case MIPI_DSI_FMT_RGB888: - interface_pix_fmt = ROCKCHIP_OUT_MODE_P888; - break; - case MIPI_DSI_FMT_RGB666: - interface_pix_fmt = ROCKCHIP_OUT_MODE_P666; - break; - case MIPI_DSI_FMT_RGB565: - interface_pix_fmt = ROCKCHIP_OUT_MODE_P565; - break; - default: - WARN_ON(1); - return; - } - - rockchip_drm_crtc_mode_config(encoder->crtc, DRM_MODE_CONNECTOR_DSI, - interface_pix_fmt); - if (mux) val = DSI0_SEL_VOP_LIT | (DSI0_SEL_VOP_LIT << 16); else @@ -924,6 +929,7 @@ static void dw_mipi_dsi_encoder_commit(struct drm_encoder *encoder)
static struct drm_encoder_helper_funcs dw_mipi_dsi_encoder_helper_funcs = { + .mode_fixup = dw_mipi_dsi_encoder_mode_fixup, .commit = dw_mipi_dsi_encoder_commit, .mode_set = dw_mipi_dsi_encoder_mode_set, .disable = dw_mipi_dsi_encoder_disable, diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index d5cfef7..a72af30 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -186,6 +186,8 @@ dw_hdmi_rockchip_encoder_mode_fixup(struct drm_encoder *encoder, const struct drm_display_mode *mode, struct drm_display_mode *adj_mode) { + adj_mode->private_flags = ROCKCHIP_DSP_MODE(HDMIA, AAAA); + return true; }
@@ -201,9 +203,6 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder) u32 val; int mux;
- rockchip_drm_crtc_mode_config(encoder->crtc, DRM_MODE_CONNECTOR_HDMIA, - ROCKCHIP_OUT_MODE_AAAA); - mux = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder); if (mux) val = HDMI_SEL_VOP_LIT | (HDMI_SEL_VOP_LIT << 16); diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c index 10d62ff..ef6ad91 100644 --- a/drivers/gpu/drm/rockchip/inno_hdmi.c +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c @@ -500,9 +500,6 @@ static void inno_hdmi_encoder_enable(struct drm_encoder *encoder) { struct inno_hdmi *hdmi = to_inno_hdmi(encoder);
- rockchip_drm_crtc_mode_config(encoder->crtc, DRM_MODE_CONNECTOR_HDMIA, - ROCKCHIP_OUT_MODE_P888); - inno_hdmi_set_pwr_mode(hdmi, NORMAL); }
@@ -517,6 +514,8 @@ static bool inno_hdmi_encoder_mode_fixup(struct drm_encoder *encoder, const struct drm_display_mode *mode, struct drm_display_mode *adj_mode) { + adj_mode->private_flags = ROCKCHIP_DSP_MODE(HDMIA, P888); + return true; }
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h index 00d17d7..683210b 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h @@ -68,8 +68,6 @@ void rockchip_drm_atomic_work(struct work_struct *work); int rockchip_register_crtc_funcs(struct drm_crtc *crtc, const struct rockchip_crtc_funcs *crtc_funcs); void rockchip_unregister_crtc_funcs(struct drm_crtc *crtc); -int rockchip_drm_crtc_mode_config(struct drm_crtc *crtc, int connector_type, - int out_mode); int rockchip_drm_dma_attach_device(struct drm_device *drm_dev, struct device *dev); void rockchip_drm_dma_detach_device(struct drm_device *drm_dev, diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index a619f12..1a611cf 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -818,38 +818,6 @@ static const struct drm_plane_funcs vop_plane_funcs = { .atomic_destroy_state = vop_atomic_plane_destroy_state, };
-int rockchip_drm_crtc_mode_config(struct drm_crtc *crtc, - int connector_type, - int out_mode) -{ - struct vop *vop = to_vop(crtc); - - if (WARN_ON(!vop->is_enabled)) - return -EINVAL; - - switch (connector_type) { - case DRM_MODE_CONNECTOR_LVDS: - VOP_CTRL_SET(vop, rgb_en, 1); - break; - case DRM_MODE_CONNECTOR_eDP: - VOP_CTRL_SET(vop, edp_en, 1); - break; - case DRM_MODE_CONNECTOR_HDMIA: - VOP_CTRL_SET(vop, hdmi_en, 1); - break; - case DRM_MODE_CONNECTOR_DSI: - VOP_CTRL_SET(vop, mipi_en, 1); - break; - default: - DRM_ERROR("unsupport connector_type[%d]\n", connector_type); - return -EINVAL; - }; - VOP_CTRL_SET(vop, out_mode, out_mode); - - return 0; -} -EXPORT_SYMBOL_GPL(rockchip_drm_crtc_mode_config); - static int vop_crtc_enable_vblank(struct drm_crtc *crtc) { struct vop *vop = to_vop(crtc); @@ -943,6 +911,8 @@ static void vop_crtc_enable(struct drm_crtc *crtc) u16 vact_st = adjusted_mode->vtotal - adjusted_mode->vsync_start; u16 vact_end = vact_st + vdisplay; uint32_t val; + int type = ROCKCHIP_OUT_MODE_TYPE(adjusted_mode->private_flags); + int out_mode = ROCKCHIP_OUT_MODE(adjusted_mode->private_flags);
vop_enable(crtc); /* @@ -985,6 +955,23 @@ static void vop_crtc_enable(struct drm_crtc *crtc) val |= (adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC) ? 0 : 1; val |= (adjusted_mode->flags & DRM_MODE_FLAG_NVSYNC) ? 0 : (1 << 1); VOP_CTRL_SET(vop, pin_pol, val); + switch (type) { + case DRM_MODE_CONNECTOR_LVDS: + VOP_CTRL_SET(vop, rgb_en, 1); + break; + case DRM_MODE_CONNECTOR_eDP: + VOP_CTRL_SET(vop, edp_en, 1); + break; + case DRM_MODE_CONNECTOR_HDMIA: + VOP_CTRL_SET(vop, hdmi_en, 1); + break; + case DRM_MODE_CONNECTOR_DSI: + VOP_CTRL_SET(vop, mipi_en, 1); + break; + default: + DRM_ERROR("unsupport connector_type[%d]\n", type); + } + VOP_CTRL_SET(vop, out_mode, out_mode);
VOP_CTRL_SET(vop, htotal_pw, (htotal << 16) | hsync_len); val = hact_st << 16; diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h index 071ff0b..4c63a66 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h @@ -183,6 +183,12 @@ struct vop_data { /* for use special outface */ #define ROCKCHIP_OUT_MODE_AAAA 15
+#define ROCKCHIP_OUT_MODE_TYPE(x) ((x) >> 16) +#define ROCKCHIP_OUT_MODE(x) ((x) & 0xffff) +#define ROCKCHIP_DSP_MODE(type, mode) \ + ((DRM_MODE_CONNECTOR_##type << 16) | \ + (ROCKCHIP_OUT_MODE_##mode & 0xffff)) + enum alpha_mode { ALPHA_STRAIGHT, ALPHA_INVERSE,
Hi Mark,
On Mon, 18 Apr 2016 11:03:31 +0800, Mark Yao wrote:
We need to take care of the vop status when use rockchip_drm_crtc_mode_config, if vop is disabled, the function would failed, that is terrible.
Save connector type and output mode on drm_display_mode->private_flags at encoder mode_fixup, then we can configure the type and mode safely on crtc mode_set.
Since Rockchip is atomic, shouldn't this be using atomic_check hooks and a subclassed crtc_state structure?
Signed-off-by: Mark Yao mark.yao@rock-chips.com
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 37 +++++++--------- drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 44 ++++++++++--------- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 5 +-- drivers/gpu/drm/rockchip/inno_hdmi.c | 5 +-- drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 - drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 51 +++++++++-------------- drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 6 +++ 7 files changed, 69 insertions(+), 81 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index a1d94d8..0344e52 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -97,7 +97,21 @@ rockchip_dp_drm_encoder_mode_fixup(struct drm_encoder *encoder, const struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode) {
- /* do nothing */
- /*
* FIXME(Yakir): driver should configure the CRTC output video
* mode with the display information which indicated the monitor
* support colorimetry.
*
* But don't know why the CRTC driver seems could only output the
* RGBaaa rightly. For example, if connect the "innolux,n116bge"
* eDP screen, EDID would indicated that screen only accepted the
* 6bpc mode. But if I configure CRTC to RGB666 output, then eDP
* screen would show a blue picture (RGB888 show a green picture).
* But if I configure CTRC to RGBaaa, and eDP driver still keep
* RGB666 input video mode, then screen would works prefect.
*/
- adjusted_mode->private_flags = ROCKCHIP_DSP_MODE(eDP, AAAA);
- return true;
}
@@ -114,27 +128,6 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder) int ret; u32 val;
- /*
* FIXME(Yakir): driver should configure the CRTC output video
* mode with the display information which indicated the monitor
* support colorimetry.
*
* But don't know why the CRTC driver seems could only output the
* RGBaaa rightly. For example, if connect the "innolux,n116bge"
* eDP screen, EDID would indicated that screen only accepted the
* 6bpc mode. But if I configure CRTC to RGB666 output, then eDP
* screen would show a blue picture (RGB888 show a green picture).
* But if I configure CTRC to RGBaaa, and eDP driver still keep
* RGB666 input video mode, then screen would works prefect.
*/
- ret = rockchip_drm_crtc_mode_config(encoder->crtc,
DRM_MODE_CONNECTOR_eDP,
ROCKCHIP_OUT_MODE_AAAA);
- if (ret < 0) {
dev_err(dp->dev, "Could not set crtc mode config (%d)\n", ret);
return;
- }
- ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder); if (ret < 0) return;
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c index 7975158..49c4c72 100644 --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c @@ -875,11 +875,34 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) clk_disable_unprepare(dsi->pclk); }
+static bool dw_mipi_dsi_encoder_mode_fixup(struct drm_encoder *encoder,
const struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
+{
- struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
- switch (dsi->format) {
- case MIPI_DSI_FMT_RGB888:
adjusted_mode->private_flags = ROCKCHIP_DSP_MODE(DSI, P888);
break;
- case MIPI_DSI_FMT_RGB666:
adjusted_mode->private_flags = ROCKCHIP_DSP_MODE(DSI, P666);
break;
- case MIPI_DSI_FMT_RGB565:
adjusted_mode->private_flags = ROCKCHIP_DSP_MODE(DSI, P565);
break;
- default:
WARN_ON(1);
return false;
- }
- return true;
+}
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 interface_pix_fmt; u32 val;
if (clk_prepare_enable(dsi->pclk)) {
@@ -895,24 +918,6 @@ static void dw_mipi_dsi_encoder_commit(struct drm_encoder *encoder)
clk_disable_unprepare(dsi->pclk);
- switch (dsi->format) {
- case MIPI_DSI_FMT_RGB888:
interface_pix_fmt = ROCKCHIP_OUT_MODE_P888;
break;
- case MIPI_DSI_FMT_RGB666:
interface_pix_fmt = ROCKCHIP_OUT_MODE_P666;
break;
- case MIPI_DSI_FMT_RGB565:
interface_pix_fmt = ROCKCHIP_OUT_MODE_P565;
break;
- default:
WARN_ON(1);
return;
- }
- rockchip_drm_crtc_mode_config(encoder->crtc, DRM_MODE_CONNECTOR_DSI,
interface_pix_fmt);
- if (mux) val = DSI0_SEL_VOP_LIT | (DSI0_SEL_VOP_LIT << 16); else
@@ -924,6 +929,7 @@ static void dw_mipi_dsi_encoder_commit(struct drm_encoder *encoder)
static struct drm_encoder_helper_funcs dw_mipi_dsi_encoder_helper_funcs = {
- .mode_fixup = dw_mipi_dsi_encoder_mode_fixup, .commit = dw_mipi_dsi_encoder_commit, .mode_set = dw_mipi_dsi_encoder_mode_set, .disable = dw_mipi_dsi_encoder_disable,
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index d5cfef7..a72af30 100644 --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c @@ -186,6 +186,8 @@ dw_hdmi_rockchip_encoder_mode_fixup(struct drm_encoder *encoder, const struct drm_display_mode *mode, struct drm_display_mode *adj_mode) {
- adj_mode->private_flags = ROCKCHIP_DSP_MODE(HDMIA, AAAA);
- return true;
}
@@ -201,9 +203,6 @@ static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder) u32 val; int mux;
- rockchip_drm_crtc_mode_config(encoder->crtc, DRM_MODE_CONNECTOR_HDMIA,
ROCKCHIP_OUT_MODE_AAAA);
- mux = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder); if (mux) val = HDMI_SEL_VOP_LIT | (HDMI_SEL_VOP_LIT << 16);
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c index 10d62ff..ef6ad91 100644 --- a/drivers/gpu/drm/rockchip/inno_hdmi.c +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c @@ -500,9 +500,6 @@ static void inno_hdmi_encoder_enable(struct drm_encoder *encoder) { struct inno_hdmi *hdmi = to_inno_hdmi(encoder);
- rockchip_drm_crtc_mode_config(encoder->crtc, DRM_MODE_CONNECTOR_HDMIA,
ROCKCHIP_OUT_MODE_P888);
- inno_hdmi_set_pwr_mode(hdmi, NORMAL);
}
@@ -517,6 +514,8 @@ static bool inno_hdmi_encoder_mode_fixup(struct drm_encoder *encoder, const struct drm_display_mode *mode, struct drm_display_mode *adj_mode) {
- adj_mode->private_flags = ROCKCHIP_DSP_MODE(HDMIA, P888);
- return true;
}
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h index 00d17d7..683210b 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h @@ -68,8 +68,6 @@ void rockchip_drm_atomic_work(struct work_struct *work); int rockchip_register_crtc_funcs(struct drm_crtc *crtc, const struct rockchip_crtc_funcs *crtc_funcs); void rockchip_unregister_crtc_funcs(struct drm_crtc *crtc); -int rockchip_drm_crtc_mode_config(struct drm_crtc *crtc, int connector_type,
int out_mode);
int rockchip_drm_dma_attach_device(struct drm_device *drm_dev, struct device *dev); void rockchip_drm_dma_detach_device(struct drm_device *drm_dev, diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index a619f12..1a611cf 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -818,38 +818,6 @@ static const struct drm_plane_funcs vop_plane_funcs = { .atomic_destroy_state = vop_atomic_plane_destroy_state, };
-int rockchip_drm_crtc_mode_config(struct drm_crtc *crtc,
int connector_type,
int out_mode)
-{
- struct vop *vop = to_vop(crtc);
- if (WARN_ON(!vop->is_enabled))
return -EINVAL;
- switch (connector_type) {
- case DRM_MODE_CONNECTOR_LVDS:
VOP_CTRL_SET(vop, rgb_en, 1);
break;
- case DRM_MODE_CONNECTOR_eDP:
VOP_CTRL_SET(vop, edp_en, 1);
break;
- case DRM_MODE_CONNECTOR_HDMIA:
VOP_CTRL_SET(vop, hdmi_en, 1);
break;
- case DRM_MODE_CONNECTOR_DSI:
VOP_CTRL_SET(vop, mipi_en, 1);
break;
- default:
DRM_ERROR("unsupport connector_type[%d]\n", connector_type);
return -EINVAL;
- };
- VOP_CTRL_SET(vop, out_mode, out_mode);
- return 0;
-} -EXPORT_SYMBOL_GPL(rockchip_drm_crtc_mode_config);
static int vop_crtc_enable_vblank(struct drm_crtc *crtc) { struct vop *vop = to_vop(crtc); @@ -943,6 +911,8 @@ static void vop_crtc_enable(struct drm_crtc *crtc) u16 vact_st = adjusted_mode->vtotal - adjusted_mode->vsync_start; u16 vact_end = vact_st + vdisplay; uint32_t val;
int type = ROCKCHIP_OUT_MODE_TYPE(adjusted_mode->private_flags);
int out_mode = ROCKCHIP_OUT_MODE(adjusted_mode->private_flags);
vop_enable(crtc); /*
@@ -985,6 +955,23 @@ static void vop_crtc_enable(struct drm_crtc *crtc) val |= (adjusted_mode->flags & DRM_MODE_FLAG_NHSYNC) ? 0 : 1; val |= (adjusted_mode->flags & DRM_MODE_FLAG_NVSYNC) ? 0 : (1 << 1); VOP_CTRL_SET(vop, pin_pol, val);
switch (type) {
case DRM_MODE_CONNECTOR_LVDS:
VOP_CTRL_SET(vop, rgb_en, 1);
break;
case DRM_MODE_CONNECTOR_eDP:
VOP_CTRL_SET(vop, edp_en, 1);
break;
case DRM_MODE_CONNECTOR_HDMIA:
VOP_CTRL_SET(vop, hdmi_en, 1);
break;
case DRM_MODE_CONNECTOR_DSI:
VOP_CTRL_SET(vop, mipi_en, 1);
break;
default:
DRM_ERROR("unsupport connector_type[%d]\n", type);
}
VOP_CTRL_SET(vop, out_mode, out_mode);
VOP_CTRL_SET(vop, htotal_pw, (htotal << 16) | hsync_len); val = hact_st << 16;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h index 071ff0b..4c63a66 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h @@ -183,6 +183,12 @@ struct vop_data { /* for use special outface */ #define ROCKCHIP_OUT_MODE_AAAA 15
+#define ROCKCHIP_OUT_MODE_TYPE(x) ((x) >> 16) +#define ROCKCHIP_OUT_MODE(x) ((x) & 0xffff) +#define ROCKCHIP_DSP_MODE(type, mode) \
((DRM_MODE_CONNECTOR_##type << 16) | \
(ROCKCHIP_OUT_MODE_##mode & 0xffff))
enum alpha_mode { ALPHA_STRAIGHT, ALPHA_INVERSE,
On 2016年04月18日 17:25, John Keeping wrote:
We need to take care of the vop status when use rockchip_drm_crtc_mode_config, if vop is disabled, the function would failed, that is terrible.
Save connector type and output mode on drm_display_mode->private_flags at encoder mode_fixup, then we can configure the type and mode safely on crtc mode_set.
Since Rockchip is atomic, shouldn't this be using atomic_check hooks and a subclassed crtc_state structure?
Right, Good idea.
Thanks.
On 2016年04月18日 17:25, John Keeping wrote:
On Mon, 18 Apr 2016 11:03:31 +0800, Mark Yao wrote:
We need to take care of the vop status when use rockchip_drm_crtc_mode_config, if vop is disabled, the function would failed, that is terrible.
Save connector type and output mode on drm_display_mode->private_flags at encoder mode_fixup, then we can configure the type and mode safely on crtc mode_set.
Since Rockchip is atomic, shouldn't this be using atomic_check hooks and a subclassed crtc_state structure?
Hi John
I try to use atomic_check with crtc_state, but it seems not easy, there are two value need transmit from connector to vop: connector type and out_mode
the connector type I think we can loop the atomic state to find the connector type. but the out_mode is a custom value, I can't find a generic way to transmit it with atomic state.
BTW, I think on atomic side, the drm_display_mode is under control by atomic state, and the mode->private_flags is not use by drm framework, I found i915 and gma500 also use mode->private_flags to transmit custom value.
So I think it's no problem using mode->private_flags.
On Tue, 19 Apr 2016 10:43:03 +0800, Mark yao wrote:
On 2016年04月18日 17:25, John Keeping wrote:
On Mon, 18 Apr 2016 11:03:31 +0800, Mark Yao wrote:
We need to take care of the vop status when use rockchip_drm_crtc_mode_config, if vop is disabled, the function would failed, that is terrible.
Save connector type and output mode on drm_display_mode->private_flags at encoder mode_fixup, then we can configure the type and mode safely on crtc mode_set.
Since Rockchip is atomic, shouldn't this be using atomic_check hooks and a subclassed crtc_state structure?
I try to use atomic_check with crtc_state, but it seems not easy, there are two value need transmit from connector to vop: connector type and out_mode
the connector type I think we can loop the atomic state to find the connector type. but the out_mode is a custom value, I can't find a generic way to transmit it with atomic state.
BTW, I think on atomic side, the drm_display_mode is under control by atomic state, and the mode->private_flags is not use by drm framework, I found i915 and gma500 also use mode->private_flags to transmit custom value.
So I think it's no problem using mode->private_flags.
The documentation for drm_display_mode::private says:
It shouldn't be used by atomic drivers since they can store any additional data by subclassing state structures.
This applies to private_flags as well.
I think this means that we should do something like the patch below (which isn't even compile tested and doesn't cover implementing drm_encoder_helper_funcs::atomic_check on all of the encoders):
-- >8 -- diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h index 00d17d71aa4c..9d65fa9188f4 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h @@ -31,6 +31,14 @@ struct drm_device; struct drm_connector;
+struct rockchip_crtc_state { + struct drm_crtc_state base; + int output_type; + int output_mode; +}; + +#define to_rockchip_crtc_state(s) container_of(x, struct rockchip_crtc_state, base) + /* * Rockchip drm private crtc funcs. * @enable_vblank: enable crtc vblank irq. diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index a619f120f801..5cdf3123e1eb 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -1044,13 +1044,34 @@ static void vop_crtc_destroy(struct drm_crtc *crtc) drm_crtc_cleanup(crtc); }
+static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc) +{ + struct rockchip_crtc_state *rockchip_state; + + rockchip_state = kzalloc(sizeof(*rockchip_state), GFP_KERNEL); + if (!rockchip_state) + return NULL; + + __drm_atomic_helper_crtc_duplicate_state(crtc, &rockchip_state->base); + return &rockchip_state->base; +} + +static void vop_crtc_destroy_state(struct drm_crtc *crtc, + struct drm_crtc_state *state) +{ + struct rockchip_crtc_state *s = to_rockchip_crtc_state(state); + + __drm_atomic_helper_crtc_destroy_state(crtc, &s->base); + kfree(s); +} + static const struct drm_crtc_funcs vop_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .destroy = vop_crtc_destroy, .reset = drm_atomic_helper_crtc_reset, - .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, - .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, + .atomic_duplicate_state = vop_crtc_duplicate_state, + .atomic_destroy_state = vop_crtc_destroy_state, };
static bool vop_win_pending_is_complete(struct vop_win *vop_win)
On 2016年04月19日 17:06, John Keeping wrote:
On Tue, 19 Apr 2016 10:43:03 +0800, Mark yao wrote:
On 2016年04月18日 17:25, John Keeping wrote:
On Mon, 18 Apr 2016 11:03:31 +0800, Mark Yao wrote:
We need to take care of the vop status when use rockchip_drm_crtc_mode_config, if vop is disabled, the function would failed, that is terrible.
Save connector type and output mode on drm_display_mode->private_flags at encoder mode_fixup, then we can configure the type and mode safely on crtc mode_set.
Since Rockchip is atomic, shouldn't this be using atomic_check hooks and a subclassed crtc_state structure?
I try to use atomic_check with crtc_state, but it seems not easy, there are two value need transmit from connector to vop: connector type and out_mode
the connector type I think we can loop the atomic state to find the connector type. but the out_mode is a custom value, I can't find a generic way to transmit it with atomic state.
BTW, I think on atomic side, the drm_display_mode is under control by atomic state, and the mode->private_flags is not use by drm framework, I found i915 and gma500 also use mode->private_flags to transmit custom value.
So I think it's no problem using mode->private_flags.
The documentation for drm_display_mode::private says:
It shouldn't be used by atomic drivers since they can store any additional data by subclassing state structures.
This applies to private_flags as well.
I think this means that we should do something like the patch below (which isn't even compile tested and doesn't cover implementing drm_encoder_helper_funcs::atomic_check on all of the encoders):
-- >8 -- diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h index 00d17d71aa4c..9d65fa9188f4 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h @@ -31,6 +31,14 @@ struct drm_device; struct drm_connector;
+struct rockchip_crtc_state {
- struct drm_crtc_state base;
- int output_type;
- int output_mode;
+};
+#define to_rockchip_crtc_state(s) container_of(x, struct rockchip_crtc_state, base)
- /*
- Rockchip drm private crtc funcs.
- @enable_vblank: enable crtc vblank irq.
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index a619f120f801..5cdf3123e1eb 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -1044,13 +1044,34 @@ static void vop_crtc_destroy(struct drm_crtc *crtc) drm_crtc_cleanup(crtc); }
+static struct drm_crtc_state *vop_crtc_duplicate_state(struct drm_crtc *crtc) +{
- struct rockchip_crtc_state *rockchip_state;
- rockchip_state = kzalloc(sizeof(*rockchip_state), GFP_KERNEL);
- if (!rockchip_state)
return NULL;
- __drm_atomic_helper_crtc_duplicate_state(crtc, &rockchip_state->base);
- return &rockchip_state->base;
+}
+static void vop_crtc_destroy_state(struct drm_crtc *crtc,
struct drm_crtc_state *state)
+{
- struct rockchip_crtc_state *s = to_rockchip_crtc_state(state);
- __drm_atomic_helper_crtc_destroy_state(crtc, &s->base);
- kfree(s);
+}
- static const struct drm_crtc_funcs vop_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, .destroy = vop_crtc_destroy, .reset = drm_atomic_helper_crtc_reset,
- .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
.atomic_duplicate_state = vop_crtc_duplicate_state,
.atomic_destroy_state = vop_crtc_destroy_state, };
static bool vop_win_pending_is_complete(struct vop_win *vop_win)
Hi John
Good, I had implement it according to your advise.
Thanks.
dri-devel@lists.freedesktop.org