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 <[email protected]>
---
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,
--
1.7.9.5
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 <[email protected]>
> ---
> 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.
--
Ｍark Yao
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.
--
Ｍark Yao
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.
--
Ｍark Yao