Re: [Freedreno] [PATCH v4 3/3] drm/msm/dpu: add display port support in DPU

2019-01-23 Thread Sean Paul
On Mon, Dec 17, 2018 at 02:35:05PM -0800, Jeykumar Sankaran wrote:
> Add display port support in DPU by creating hooks
> for DP encoder enumeration and encoder mode
> initialization.
> 
> This change is based on the SDM845 Display port
> driver changes[1].
> 
> changes in v2:
> - rebase on [2] (Sean Paul)
> - remove unwanted error checks and
>   switch cases (Jordan Crouse)
> changes in v3:
> - add dp support after fixing
>   the current code base for error logging (Sean Paul)
> changes in v4:
> -  avoid duplicate returns (Jordan Crouse)
> -  get rid of duplicate error logs (Jordan Crouse)
> 
> [1] https://lwn.net/Articles/768265/
> [2] https://lkml.org/lkml/2018/11/17/87
> 
> Signed-off-by: Jeykumar Sankaran 
> Reviewed-by: Jordan Crouse 
> Reviewed-by: Sean Paul 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  8 ++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 58 
> +
>  2 files changed, 54 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 0dda4a6..371d17d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -2031,7 +2031,7 @@ static int dpu_encoder_setup_display(struct 
> dpu_encoder_virt *dpu_enc,
>  {
>   int ret = 0;
>   int i = 0;
> - enum dpu_intf_type intf_type;
> + enum dpu_intf_type intf_type = INTF_NONE;
>   struct dpu_enc_phys_init_params phys_params;
>  
>   if (!dpu_enc || !dpu_kms) {
> @@ -2054,9 +2054,9 @@ static int dpu_encoder_setup_display(struct 
> dpu_encoder_virt *dpu_enc,
>   case DRM_MODE_ENCODER_DSI:
>   intf_type = INTF_DSI;
>   break;
> - default:
> - DPU_ERROR_ENC(dpu_enc, "unsupported display interface type\n");
> - return -EINVAL;
> + case DRM_MODE_ENCODER_TMDS:
> + intf_type = INTF_DP;
> + break;
>   }
>  
>   WARN_ON(disp_info->num_of_h_tiles < 1);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 885bf88..62b400c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -439,6 +439,31 @@ static int _dpu_kms_initialize_dsi(struct drm_device 
> *dev,
>   return rc;
>  }
>  
> +static int _dpu_kms_initialize_displayport(struct drm_device *dev,
> + struct msm_drm_private *priv,
> + struct dpu_kms *dpu_kms)
> +{
> + struct drm_encoder *encoder = NULL;
> + int rc;
> +
> + if (!priv->dp)
> + return 0;
> +
> + encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
> + if (IS_ERR(encoder)) {
> + DPU_ERROR("encoder init failed for dsi display\n");
> + return PTR_ERR(encoder);
> + }
> +
> + rc = msm_dp_modeset_init(priv->dp, dev, encoder);
> + if (rc) {
> + DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
> + drm_encoder_cleanup(encoder);
> + }
> +
> + return rc;
> +}
> +
>  /**
>   * _dpu_kms_setup_displays - create encoders, bridges and connectors
>   *   for underlying displays
> @@ -451,12 +476,22 @@ static int _dpu_kms_setup_displays(struct drm_device 
> *dev,
>   struct msm_drm_private *priv,
>   struct dpu_kms *dpu_kms)
>  {
> + int rc;
> +
> + rc = _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
> + if (rc)
> + goto fail;
> +
> + rc = _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
> + if (rc)
> + goto fail;
> +
>   /**
>* Extend this function to initialize other
>* types of displays
>*/
> -
> - return _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
> +fail:

One more thing, remove this label and just return directly above.

> + return rc;

Then this becomes return 0

>  }
>  
>  static void _dpu_kms_drm_obj_destroy(struct dpu_kms *dpu_kms)
> @@ -669,13 +704,20 @@ static void _dpu_kms_set_encoder_mode(struct msm_kms 
> *kms,
>   info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
>   MSM_DISPLAY_CAP_VID_MODE;
>  
> - /* TODO: No support for DSI swap */
> - for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> - if (priv->dsi[i]) {
> - info.h_tile_instance[info.num_of_h_tiles] = i;
> - info.num_of_h_tiles++;
> + switch (info.intf_type) {
> + case DRM_MODE_ENCODER_DSI:
> + /* TODO: No support for DSI swap */
> + for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> + if (priv->dsi[i]) {
> + info.h_tile_instance[info.num_of_h_tiles] = i;
> + info.num_of_h_tiles++;
> + }
>  

Re: [PATCH v4 3/3] drm/msm/dpu: add display port support in DPU

2018-12-18 Thread kbuild test robot
Hi Jeykumar,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robclark/msm-next]
[also build test ERROR on v4.20-rc7 next-20181218]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Jeykumar-Sankaran/drm-msm-dpu-fix-documentation-for-intf_type/20181218-070519
base:   git://people.freedesktop.org/~robclark/linux msm-next
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c: In function 
'_dpu_kms_initialize_displayport':
>> drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c:449:13: error: 'struct 
>> msm_drm_private' has no member named 'dp'; did you mean 'edp'?
 if (!priv->dp)
^~
edp
>> drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c:458:7: error: implicit declaration 
>> of function 'msm_dp_modeset_init'; did you mean 'msm_edp_modeset_init'? 
>> [-Werror=implicit-function-declaration]
 rc = msm_dp_modeset_init(priv->dp, dev, encoder);
  ^~~
  msm_edp_modeset_init
   drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c:458:33: error: 'struct 
msm_drm_private' has no member named 'dp'; did you mean 'edp'?
 rc = msm_dp_modeset_init(priv->dp, dev, encoder);
^~
edp
   cc1: some warnings being treated as errors

vim +449 drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c

   441  
   442  static int _dpu_kms_initialize_displayport(struct drm_device *dev,
   443  struct msm_drm_private 
*priv,
   444  struct dpu_kms *dpu_kms)
   445  {
   446  struct drm_encoder *encoder = NULL;
   447  int rc;
   448  
 > 449  if (!priv->dp)
   450  return 0;
   451  
   452  encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
   453  if (IS_ERR(encoder)) {
   454  DPU_ERROR("encoder init failed for dsi display\n");
   455  return PTR_ERR(encoder);
   456  }
   457  
 > 458  rc = msm_dp_modeset_init(priv->dp, dev, encoder);
   459  if (rc) {
   460  DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
   461  drm_encoder_cleanup(encoder);
   462  }
   463  
   464  return rc;
   465  }
   466  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v4 3/3] drm/msm/dpu: add display port support in DPU

2018-12-17 Thread Jeykumar Sankaran
Add display port support in DPU by creating hooks
for DP encoder enumeration and encoder mode
initialization.

This change is based on the SDM845 Display port
driver changes[1].

changes in v2:
- rebase on [2] (Sean Paul)
- remove unwanted error checks and
  switch cases (Jordan Crouse)
changes in v3:
- add dp support after fixing
  the current code base for error logging (Sean Paul)
changes in v4:
-  avoid duplicate returns (Jordan Crouse)
-  get rid of duplicate error logs (Jordan Crouse)

[1] https://lwn.net/Articles/768265/
[2] https://lkml.org/lkml/2018/11/17/87

Signed-off-by: Jeykumar Sankaran 
Reviewed-by: Jordan Crouse 
Reviewed-by: Sean Paul 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  8 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 58 +
 2 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0dda4a6..371d17d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2031,7 +2031,7 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,
 {
int ret = 0;
int i = 0;
-   enum dpu_intf_type intf_type;
+   enum dpu_intf_type intf_type = INTF_NONE;
struct dpu_enc_phys_init_params phys_params;
 
if (!dpu_enc || !dpu_kms) {
@@ -2054,9 +2054,9 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,
case DRM_MODE_ENCODER_DSI:
intf_type = INTF_DSI;
break;
-   default:
-   DPU_ERROR_ENC(dpu_enc, "unsupported display interface type\n");
-   return -EINVAL;
+   case DRM_MODE_ENCODER_TMDS:
+   intf_type = INTF_DP;
+   break;
}
 
WARN_ON(disp_info->num_of_h_tiles < 1);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 885bf88..62b400c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -439,6 +439,31 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
return rc;
 }
 
+static int _dpu_kms_initialize_displayport(struct drm_device *dev,
+   struct msm_drm_private *priv,
+   struct dpu_kms *dpu_kms)
+{
+   struct drm_encoder *encoder = NULL;
+   int rc;
+
+   if (!priv->dp)
+   return 0;
+
+   encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
+   if (IS_ERR(encoder)) {
+   DPU_ERROR("encoder init failed for dsi display\n");
+   return PTR_ERR(encoder);
+   }
+
+   rc = msm_dp_modeset_init(priv->dp, dev, encoder);
+   if (rc) {
+   DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
+   drm_encoder_cleanup(encoder);
+   }
+
+   return rc;
+}
+
 /**
  * _dpu_kms_setup_displays - create encoders, bridges and connectors
  *   for underlying displays
@@ -451,12 +476,22 @@ static int _dpu_kms_setup_displays(struct drm_device *dev,
struct msm_drm_private *priv,
struct dpu_kms *dpu_kms)
 {
+   int rc;
+
+   rc = _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
+   if (rc)
+   goto fail;
+
+   rc = _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
+   if (rc)
+   goto fail;
+
/**
 * Extend this function to initialize other
 * types of displays
 */
-
-   return _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
+fail:
+   return rc;
 }
 
 static void _dpu_kms_drm_obj_destroy(struct dpu_kms *dpu_kms)
@@ -669,13 +704,20 @@ static void _dpu_kms_set_encoder_mode(struct msm_kms *kms,
info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
MSM_DISPLAY_CAP_VID_MODE;
 
-   /* TODO: No support for DSI swap */
-   for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
-   if (priv->dsi[i]) {
-   info.h_tile_instance[info.num_of_h_tiles] = i;
-   info.num_of_h_tiles++;
+   switch (info.intf_type) {
+   case DRM_MODE_ENCODER_DSI:
+   /* TODO: No support for DSI swap */
+   for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
+   if (priv->dsi[i]) {
+   info.h_tile_instance[info.num_of_h_tiles] = i;
+   info.num_of_h_tiles++;
+   }
}
-   }
+   break;
+   case DRM_MODE_ENCODER_TMDS:
+   info.num_of_h_tiles = 1;
+   break;
+   };
 
rc = dpu_encoder_setup(encoder->dev, encoder, );
if (rc)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux