Re: [PATCH v3] drm/rockchip: get rid of rockchip_drm_crtc_mode_config

[John Keeping]

On Wed, 20 Apr 2016 10:41:42 +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 output_type and output_mode into rockchip_crtc_state,
> it's nice to make them into atomic.
> 
> Signed-off-by: Mark Yao 

Tested-by: John Keeping 
(for HDMI output)

Re: [PATCH v3] drm/rockchip: get rid of rockchip_drm_crtc_mode_config

[John Keeping]

On Wed, 20 Apr 2016 10:41:42 +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 output_type and output_mode into rockchip_crtc_state,
> it's nice to make them into atomic.
> 
> Signed-off-by: Mark Yao 

Tested-by: John Keeping 
(for HDMI output)

[PATCH v3] drm/rockchip: get rid of rockchip_drm_crtc_mode_config

[Mark Yao]

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 output_type and output_mode into rockchip_crtc_state,
it's nice to make them into atomic.

Signed-off-by: Mark Yao 
---
Changes in v3:
- foget remove rockchip_drm_crtc_mode_config function in v2, remove it in v3.
 
Changes in v2:
Advised by John Keeping
- use atomic crtc state to transmit output_type and output_mode
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c |   48 ---
 drivers/gpu/drm/rockchip/dw-mipi-dsi.c  |   38 +++-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c |   17 -
 drivers/gpu/drm/rockchip/inno_hdmi.c|   17 -
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   10 ++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |   75 +--
 6 files changed, 128 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index a1d94d8..50e558d 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -114,27 +114,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_);
-   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;
@@ -158,11 +137,38 @@ static void rockchip_dp_drm_encoder_nop(struct 
drm_encoder *encoder)
/* do nothing */
 }
 
+static int
+rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+   struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
+
+   /*
+* 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.
+*/
+   s->output_mode = ROCKCHIP_OUT_MODE_;
+   s->output_type = DRM_MODE_CONNECTOR_HDMIA;
+
+   return 0;
+}
+
 static struct drm_encoder_helper_funcs rockchip_dp_encoder_helper_funcs = {
.mode_fixup = rockchip_dp_drm_encoder_mode_fixup,
.mode_set = rockchip_dp_drm_encoder_mode_set,
.enable = rockchip_dp_drm_encoder_enable,
.disable = rockchip_dp_drm_encoder_nop,
+   .atomic_check = rockchip_dp_drm_encoder_atomic_check,
 };
 
 static void rockchip_dp_drm_encoder_destroy(struct drm_encoder *encoder)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index 7975158..dedc65b 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -879,7 +879,6 @@ 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,31 +894,41 @@ static void dw_mipi_dsi_encoder_commit(struct drm_encoder 
*encoder)
 
clk_disable_unprepare(dsi->pclk);
 
+   if (mux)
+   val = DSI0_SEL_VOP_LIT | (DSI0_SEL_VOP_LIT << 16);
+   else
+

[PATCH v3] drm/rockchip: get rid of rockchip_drm_crtc_mode_config

[Mark Yao]

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 output_type and output_mode into rockchip_crtc_state,
it's nice to make them into atomic.

Signed-off-by: Mark Yao 
---
Changes in v3:
- foget remove rockchip_drm_crtc_mode_config function in v2, remove it in v3.
 
Changes in v2:
Advised by John Keeping
- use atomic crtc state to transmit output_type and output_mode
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c |   48 ---
 drivers/gpu/drm/rockchip/dw-mipi-dsi.c  |   38 +++-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c |   17 -
 drivers/gpu/drm/rockchip/inno_hdmi.c|   17 -
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   10 ++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |   75 +--
 6 files changed, 128 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index a1d94d8..50e558d 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -114,27 +114,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_);
-   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;
@@ -158,11 +137,38 @@ static void rockchip_dp_drm_encoder_nop(struct 
drm_encoder *encoder)
/* do nothing */
 }
 
+static int
+rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+   struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
+
+   /*
+* 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.
+*/
+   s->output_mode = ROCKCHIP_OUT_MODE_;
+   s->output_type = DRM_MODE_CONNECTOR_HDMIA;
+
+   return 0;
+}
+
 static struct drm_encoder_helper_funcs rockchip_dp_encoder_helper_funcs = {
.mode_fixup = rockchip_dp_drm_encoder_mode_fixup,
.mode_set = rockchip_dp_drm_encoder_mode_set,
.enable = rockchip_dp_drm_encoder_enable,
.disable = rockchip_dp_drm_encoder_nop,
+   .atomic_check = rockchip_dp_drm_encoder_atomic_check,
 };
 
 static void rockchip_dp_drm_encoder_destroy(struct drm_encoder *encoder)
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c 
b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index 7975158..dedc65b 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -879,7 +879,6 @@ 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,31 +894,41 @@ static void dw_mipi_dsi_encoder_commit(struct drm_encoder 
*encoder)
 
clk_disable_unprepare(dsi->pclk);
 
+   if (mux)
+   val = DSI0_SEL_VOP_LIT | (DSI0_SEL_VOP_LIT << 16);
+   else
+   val =