Re: [DPU PATCH 5/6] drm/msm: hook up DPU with upstream DSI

2018-04-19 Thread Jeykumar Sankaran 

On 2018-04-19 08:38, Sean Paul wrote:

On Mon, Apr 16, 2018 at 11:22:20AM -0700, Jeykumar Sankaran wrote:

Switch DPU from dsi-staging to upstream dsi driver. To make
the switch atomic, this change includes:
- remove dpu connector layers
- clean up dpu connector dependencies in encoder/crtc
- compile out writeback and display port drivers
- compile out dsi-staging driver (separate patch submitted to
  remove the driver)
- adapt upstream device hierarchy

Signed-off-by: Jeykumar Sankaran 
---
 .../config/arm64/chromiumos-arm64.flavour.config   |4 +-
 .../arm64/chromiumos-qualcomm.flavour.config   |4 +-


These files aren't in upstream.




+
+struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
+   int drm_enc_mode)
+{
+   struct dpu_encoder_virt *dpu_enc = NULL;
+
+   dpu_enc = kzalloc(sizeof(*dpu_enc), GFP_KERNEL);


You should probably use devm_kzalloc.


+   if (!dpu_enc)
+   return ERR_PTR(ENOMEM);
+
+   drm_encoder_init(dev, _enc->base, _encoder_funcs,
+   drm_enc_mode, NULL);


Check the return value?



-#ifdef CONFIG_DRM_MSM_DSI_STAGING
 static void _dpu_kms_initialize_dsi(struct drm_device *dev,
struct msm_drm_private *priv,
-   struct dpu_kms *dpu_kms,
-   unsigned max_encoders)
+   struct dpu_kms *dpu_kms)
 {
-   static const struct dpu_connector_ops dsi_ops = {
-   .post_init =  dsi_conn_post_init,
-   .detect = dsi_conn_detect,
-   .get_modes =  dsi_connector_get_modes,
-   .put_modes =  dsi_connector_put_modes,
-   .mode_valid = dsi_conn_mode_valid,
-   .get_info =   dsi_display_get_info,
-   .set_backlight = dsi_display_set_backlight,
-   .soft_reset   = dsi_display_soft_reset,
-   .pre_kickoff  = dsi_conn_pre_kickoff,
-   .clk_ctrl = dsi_display_clk_ctrl,
-   .set_power = dsi_display_set_power,
-   .get_mode_info = dsi_conn_get_mode_info,
-   .get_dst_format = dsi_display_get_dst_format,
-   .post_kickoff = dsi_conn_post_kickoff
-   };
-   struct msm_display_info info;
-   struct drm_encoder *encoder;
-   void *display, *connector;
+   struct drm_encoder *encoder = NULL;
int i, rc;

-   /* dsi */
-   for (i = 0; i < dpu_kms->dsi_display_count &&
-   priv->num_encoders < max_encoders; ++i) {
-   display = dpu_kms->dsi_displays[i];
-   encoder = NULL;
+   /*TODO: Support two independent DSI connectors */
+   encoder = dpu_encoder_init(dev, DRM_MODE_CONNECTOR_DSI);
+   if (IS_ERR_OR_NULL(encoder)) {
+   DPU_ERROR("encoder init failed for dsi %d\n", i);


Is i meaningful here? It seems like it's uninitialized...


+   return;
+   }

-   memset(, 0x0, sizeof(info));
-   rc = dsi_display_get_info(, display);
-   if (rc) {
-   DPU_ERROR("dsi get_info %d failed\n", i);
-   continue;
-   }
+   priv->encoders[priv->num_encoders++] = encoder;

-   encoder = dpu_encoder_init(dev, );
-   if (IS_ERR_OR_NULL(encoder)) {
-   DPU_ERROR("encoder init failed for dsi %d\n", i);
-   continue;
+   for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
+   if (!priv->dsi[i]) {
+   DPU_DEBUG("invalid msm_dsi for ctrl %d\n", i);
+   return;
}

-   rc = dsi_display_drm_bridge_init(display, encoder);
+   rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
if (rc) {
-   DPU_ERROR("dsi bridge %d init failed, %d\n", i,

rc);

-   dpu_encoder_destroy(encoder);
+   DPU_ERROR("modeset_init failed for dsi[%d]\n", i);


Printing rc would be nice here, same for above.


continue;
}
-
-   connector = dpu_connector_init(dev,
-   encoder,
-   0,
-   display,
-   _ops,
-   DRM_CONNECTOR_POLL_HPD,
-   DRM_MODE_CONNECTOR_DSI);
-   if (connector) {
-   priv->encoders[priv->num_encoders++] = encoder;
-   } else {
-   DPU_ERROR("dsi %d connector init failed\n", i);
-   dsi_display_drm_bridge_deinit(display);
-   dpu_encoder_destroy(encoder);
-   }
}
 }
-#endif
+

 #ifdef CONFIG_DRM_MSM_WRITEBACK
 static void

Re: [DPU PATCH 5/6] drm/msm: hook up DPU with upstream DSI

2018-04-19 Thread Sean Paul 
On Mon, Apr 16, 2018 at 11:22:20AM -0700, Jeykumar Sankaran wrote:
> Switch DPU from dsi-staging to upstream dsi driver. To make
> the switch atomic, this change includes:
> - remove dpu connector layers
> - clean up dpu connector dependencies in encoder/crtc
> - compile out writeback and display port drivers
> - compile out dsi-staging driver (separate patch submitted to
>   remove the driver)
> - adapt upstream device hierarchy
> 
> Signed-off-by: Jeykumar Sankaran 
> ---
>  .../config/arm64/chromiumos-arm64.flavour.config   |4 +-
>  .../arm64/chromiumos-qualcomm.flavour.config   |4 +-

These files aren't in upstream.



> +
> +struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
> + int drm_enc_mode)
> +{
> + struct dpu_encoder_virt *dpu_enc = NULL;
> +
> + dpu_enc = kzalloc(sizeof(*dpu_enc), GFP_KERNEL);

You should probably use devm_kzalloc.

> + if (!dpu_enc)
> + return ERR_PTR(ENOMEM);
> +
> + drm_encoder_init(dev, _enc->base, _encoder_funcs,
> + drm_enc_mode, NULL);

Check the return value?


> -#ifdef CONFIG_DRM_MSM_DSI_STAGING
>  static void _dpu_kms_initialize_dsi(struct drm_device *dev,
>   struct msm_drm_private *priv,
> - struct dpu_kms *dpu_kms,
> - unsigned max_encoders)
> + struct dpu_kms *dpu_kms)
>  {
> - static const struct dpu_connector_ops dsi_ops = {
> - .post_init =  dsi_conn_post_init,
> - .detect = dsi_conn_detect,
> - .get_modes =  dsi_connector_get_modes,
> - .put_modes =  dsi_connector_put_modes,
> - .mode_valid = dsi_conn_mode_valid,
> - .get_info =   dsi_display_get_info,
> - .set_backlight = dsi_display_set_backlight,
> - .soft_reset   = dsi_display_soft_reset,
> - .pre_kickoff  = dsi_conn_pre_kickoff,
> - .clk_ctrl = dsi_display_clk_ctrl,
> - .set_power = dsi_display_set_power,
> - .get_mode_info = dsi_conn_get_mode_info,
> - .get_dst_format = dsi_display_get_dst_format,
> - .post_kickoff = dsi_conn_post_kickoff
> - };
> - struct msm_display_info info;
> - struct drm_encoder *encoder;
> - void *display, *connector;
> + struct drm_encoder *encoder = NULL;
>   int i, rc;
>  
> - /* dsi */
> - for (i = 0; i < dpu_kms->dsi_display_count &&
> - priv->num_encoders < max_encoders; ++i) {
> - display = dpu_kms->dsi_displays[i];
> - encoder = NULL;
> + /*TODO: Support two independent DSI connectors */
> + encoder = dpu_encoder_init(dev, DRM_MODE_CONNECTOR_DSI);
> + if (IS_ERR_OR_NULL(encoder)) {
> + DPU_ERROR("encoder init failed for dsi %d\n", i);

Is i meaningful here? It seems like it's uninitialized...

> + return;
> + }
>  
> - memset(, 0x0, sizeof(info));
> - rc = dsi_display_get_info(, display);
> - if (rc) {
> - DPU_ERROR("dsi get_info %d failed\n", i);
> - continue;
> - }
> + priv->encoders[priv->num_encoders++] = encoder;
>  
> - encoder = dpu_encoder_init(dev, );
> - if (IS_ERR_OR_NULL(encoder)) {
> - DPU_ERROR("encoder init failed for dsi %d\n", i);
> - continue;
> + for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> + if (!priv->dsi[i]) {
> + DPU_DEBUG("invalid msm_dsi for ctrl %d\n", i);
> + return;
>   }
>  
> - rc = dsi_display_drm_bridge_init(display, encoder);
> + rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
>   if (rc) {
> - DPU_ERROR("dsi bridge %d init failed, %d\n", i, rc);
> - dpu_encoder_destroy(encoder);
> + DPU_ERROR("modeset_init failed for dsi[%d]\n", i);

Printing rc would be nice here, same for above.

>   continue;
>   }
> -
> - connector = dpu_connector_init(dev,
> - encoder,
> - 0,
> - display,
> - _ops,
> - DRM_CONNECTOR_POLL_HPD,
> - DRM_MODE_CONNECTOR_DSI);
> - if (connector) {
> - priv->encoders[priv->num_encoders++] = encoder;
> - } else {
> - DPU_ERROR("dsi %d connector init failed\n", i);
> - dsi_display_drm_bridge_deinit(display);
> - dpu_encoder_destroy(encoder);
> - }
>   }
>  }
> -#endif
> +
>  
>  #ifdef CONFIG_DRM_MSM_WRITEBACK
>  static

[DPU PATCH 5/6] drm/msm: hook up DPU with upstream DSI

2018-04-16 Thread Jeykumar Sankaran 
Switch DPU from dsi-staging to upstream dsi driver. To make
the switch atomic, this change includes:
- remove dpu connector layers
- clean up dpu connector dependencies in encoder/crtc
- compile out writeback and display port drivers
- compile out dsi-staging driver (separate patch submitted to
  remove the driver)
- adapt upstream device hierarchy

Signed-off-by: Jeykumar Sankaran 
---
 .../config/arm64/chromiumos-arm64.flavour.config   |4 +-
 .../arm64/chromiumos-qualcomm.flavour.config   |4 +-
 drivers/gpu/drm/msm/Makefile   |1 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_connector.c  | 1184 
 drivers/gpu/drm/msm/disp/dpu1/dpu_connector.h  |  555 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c   |9 -
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c|  173 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h|   10 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |8 +-
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |6 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c|  220 +---
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c |   54 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h |   11 +
 drivers/gpu/drm/msm/dpu_dbg.c  |3 -
 drivers/gpu/drm/msm/msm_drv.c  |   41 +-
 drivers/gpu/drm/msm/msm_drv.h  |   39 -
 16 files changed, 158 insertions(+), 2164 deletions(-)
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_connector.c
 delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_connector.h

diff --git a/chromeos/config/arm64/chromiumos-arm64.flavour.config 
b/chromeos/config/arm64/chromiumos-arm64.flavour.config
index c676a69..7b20c8b 100644
--- a/chromeos/config/arm64/chromiumos-arm64.flavour.config
+++ b/chromeos/config/arm64/chromiumos-arm64.flavour.config
@@ -117,14 +117,14 @@ CONFIG_DRM_MIPI_DSI=y
 CONFIG_DRM_MSM=y
 CONFIG_DRM_MSM_DPU=y
 CONFIG_DRM_MSM_DSI=y
-CONFIG_DRM_MSM_DSI_STAGING=y
+# CONFIG_DRM_MSM_DSI_STAGING is not set
 # CONFIG_DRM_MSM_HDCP is not set
 # CONFIG_DRM_MSM_HDMI is not set
 # CONFIG_DRM_MSM_HDMI_HDCP is not set
 # CONFIG_DRM_MSM_MDP4 is not set
 # CONFIG_DRM_MSM_MDP5 is not set
 # CONFIG_DRM_MSM_REGISTER_LOGGING is not set
-CONFIG_DRM_MSM_WRITEBACK=y
+# CONFIG_DRM_MSM_WRITEBACK is not set
 # CONFIG_DRM_NOUVEAU is not set
 CONFIG_DRM_PANEL_INNOLUX_P079ZCA=y
 # CONFIG_DRM_PANEL_JDI_LT070ME05000 is not set
diff --git a/chromeos/config/arm64/chromiumos-qualcomm.flavour.config 
b/chromeos/config/arm64/chromiumos-qualcomm.flavour.config
index e7f45f3..aad22a7 100644
--- a/chromeos/config/arm64/chromiumos-qualcomm.flavour.config
+++ b/chromeos/config/arm64/chromiumos-qualcomm.flavour.config
@@ -30,14 +30,14 @@ CONFIG_DRM_MIPI_DSI=y
 CONFIG_DRM_MSM=y
 CONFIG_DRM_MSM_DPU=y
 CONFIG_DRM_MSM_DSI=y
-CONFIG_DRM_MSM_DSI_STAGING=y
+# CONFIG_DRM_MSM_DSI_STAGING is not set
 # CONFIG_DRM_MSM_HDCP is not set
 # CONFIG_DRM_MSM_HDMI is not set
 # CONFIG_DRM_MSM_HDMI_HDCP is not set
 # CONFIG_DRM_MSM_MDP4 is not set
 # CONFIG_DRM_MSM_MDP5 is not set
 # CONFIG_DRM_MSM_REGISTER_LOGGING is not set
-CONFIG_DRM_MSM_WRITEBACK=y
+# CONFIG_DRM_MSM_WRITEBACK is not set
 CONFIG_DRM_PANEL_TRULY_WQXGA=y
 # CONFIG_DRM_PANEL_INNOLUX_P079ZCA is not set
 # CONFIG_DRM_PANEL_JDI_LT070ME05000 is not set
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index b23a001..a8d8ad9 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -48,7 +48,6 @@ msm-y := \
disp/mdp5/mdp5_plane.o \
disp/mdp5/mdp5_smp.o \
disp/dpu1/dpu_color_processing.o \
-   disp/dpu1/dpu_connector.o \
disp/dpu1/dpu_core_irq.o \
disp/dpu1/dpu_core_perf.o \
disp/dpu1/dpu_crtc.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_connector.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_connector.c
deleted file mode 100644
index dc0978d..000
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_connector.c
+++ /dev/null
@@ -1,1184 +0,0 @@
-/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#define pr_fmt(fmt)"[drm:%s:%d] " fmt, __func__, __LINE__
-#include "msm_drv.h"
-#include "dpu_dbg.h"
-
-#include "dpu_kms.h"
-#include "dpu_connector.h"
-#ifdef CONFIG_DRM_MSM_DSI_STAGING
-#include 
-#include "dsi_drm.h"
-#include "dsi_display.h"
-#endif
-
-#define BL_NODE_NAME_SIZE 32
-
-#define DPU_DEBUG_CONN(c, fmt, ...) DPU_DEBUG("conn%d " fmt,\
-   (c) ? (c)->base.base.id : -1,