drm/msm/dpu: fix documentation for intf_type
PATCH v3 1/3 drm/msm/dpu: fix documentation for intf_type
Fix intf_type description in msm_disp_info to show that it represents drm encoder mode of the display.
changes in v3:
- introduced in the series
--- drivers/gpu/drm/msm/msm_drv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/msm/msm_drv.h +1 -1
@@ -119,7 +119,7 @@ struct msm_display_topology {/** * struct msm_display_info - defines display properties -* @intf_type: DRM_MODE_CONNECTOR_ display type +* @intf_type: DRM_MODE_ENCODER_ type * @capabilities: Bitmask of display flags * @num_of_h_tiles: Number of horizontal tiles in case of split interface * @h_tile_instance: Controller instance used per tile. Number of elements is
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
A message that is not in the archive (sent off-list or not yet received): 1544727064-26763-1-git-send-email-jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org
PATCH v3 2/3 drm/msm/dpu: handle failures while initializing displays
Bail out KMS hw init on display initialization failures with proper error logging.
changes in v3:
- introduced in the series
--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 39 +++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 12 deletions(-)
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +27 -12
@@ -405,33 +405,36 @@ static void dpu_kms_wait_for_commit_done(struct msm_kms *kms, } } -static void _dpu_kms_initialize_dsi (struct drm_device *dev, +static int _dpu_kms_initialize_dsi (struct drm_device *dev,struct msm_drm_private *priv,struct dpu_kms *dpu_kms) {struct drm_encoder *encoder =NULL ; -int i, rc; +int i, rc =0 ; + +if (!(priv->dsi[0 ] || priv->dsi[1 ])) +return rc;/*TODO: Support two independent DSI connectors */ encoder =dpu_encoder_init (dev,DRM_MODE_ENCODER_DSI ); -if (IS_ERR_OR_NULL (encoder)) { +if (IS_ERR (encoder)) {DPU_ERROR ("encoder init failed for dsi display\n" ); -return ; +return PTR_ERR (encoder); }for (i =0 ; i <ARRAY_SIZE (priv->dsi); i++) { -if (!priv->dsi[i]) { -DPU_DEBUG ("invalid msm_dsi for ctrl %d\n" , i); -return ; - } +if (!priv->dsi[i]) +continue ; rc =msm_dsi_modeset_init (priv->dsi[i], dev, encoder);if (rc) {DPU_ERROR ("modeset_init failed for dsi[%d], rc = %d\n" , i, rc); -continue ; +return rc; } } + +return rc; }/**
@@ -442,16 +445,24 @@ static void _dpu_kms_initialize_dsi(struct drm_device *dev,* @dpu_kms: Pointer to dpu kms structure * Returns: Zero on success */ -static void _dpu_kms_setup_displays (struct drm_device *dev, +static int _dpu_kms_setup_displays (struct drm_device *dev,struct msm_drm_private *priv,struct dpu_kms *dpu_kms) { -_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +int rc =0 ; + + rc =_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +if (rc) { +DPU_ERROR ("failed to initialize dsi, rc = %d\n" , rc); +return rc; + }/** * Extend this function to initialize other * types of displays */ + +return rc; }static void _dpu_kms_drm_obj_destroy (struct dpu_kms *dpu_kms)
@@ -517,7 +528,11 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)* Create encoder and query display drivers to create * bridges and connectors */ -_dpu_kms_setup_displays (dev, priv, dpu_kms); + ret =_dpu_kms_setup_displays (dev, priv, dpu_kms); +if (ret) { +DPU_ERROR ("failed to setup display, rc = %d\n" , ret); + goto fail; + } max_crtc_count =min (catalog->mixer_count, (u32 )dev->mode_config.num_encoder);
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
A message that is not in the archive (sent off-list or not yet received): 1544727064-26763-2-git-send-email-jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org
Re: [PATCH v3 2/3] drm/msm/dpu: handle failures while initializing displays
Quoted 19 lines · Jeykumar Sankaran
Bail out KMS hw init on display initialization failures with proper error logging.
changes in v3:
- introduced in the seriesSigned-off-by Jeykumar Sankaran <[email protected]>--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 39 +++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 12 deletions(-)drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +0
-0@@ -405,33 +405,36 @@ static void dpu_kms_wait_for_commit_done(struct msm_kms *kms, } }
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct msm_kms *kms, · line 415
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +4
-2-static void _dpu_kms_initialize_dsi (struct drm_device *dev, +static int _dpu_kms_initialize_dsi (struct drm_device *dev,struct msm_drm_private *priv,struct dpu_kms *dpu_kms) {struct drm_encoder *encoder =NULL ; -int i, rc; +int i, rc =0 ; + +if (!(priv->dsi[0 ] || priv->dsi[1 ]))
It'd be nice to not have to support both blocks if only one is hooked up. Perhaps move the TODO below above this check.
Quoted 7 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-1+return rc;/*TODO: Support two independent DSI connectors */ encoder =dpu_encoder_init (dev,DRM_MODE_ENCODER_DSI ); -if (IS_ERR_OR_NULL (encoder)) { +if (IS_ERR (encoder)) {DPU_ERROR ("encoder init failed for dsi display\n" );
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct msm_kms *kms, · line 426
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-5-return ; +return PTR_ERR (encoder); }for (i =0 ; i <ARRAY_SIZE (priv->dsi); i++) { -if (!priv->dsi[i]) { -DPU_DEBUG ("invalid msm_dsi for ctrl %d\n" , i); -return ; - } +if (!priv->dsi[i])
I don't think this is possible given the check at the beginning of the function
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct msm_kms *kms, · line 437
Quoted 12 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +4
-1+continue ; rc =msm_dsi_modeset_init (priv->dsi[i], dev, encoder);if (rc) {DPU_ERROR ("modeset_init failed for dsi[%d], rc = %d\n" , i, rc); -continue ; +return rc; } } + +return rc;
nit: You can keep rc uninitialized above and just return 0 here.
Quoted 4 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +0
-0}/**
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_initialize_dsi(struct drm_device *dev, · line 452
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-2@@ -442,16 +445,24 @@ static void _dpu_kms_initialize_dsi(struct drm_device *dev,* @dpu_kms: Pointer to dpu kms structure * Returns: Zero on success */ -static void _dpu_kms_setup_displays (struct drm_device *dev, +static int _dpu_kms_setup_displays (struct drm_device *dev,struct msm_drm_private *priv,struct dpu_kms *dpu_kms) { -_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +int rc =0 ;
nit: No need to initialize to 0
Quoted 37 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +13
-1+ + rc =_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +if (rc) { +DPU_ERROR ("failed to initialize dsi, rc = %d\n" , rc); +return rc; + }/** * Extend this function to initialize other * types of displays */ + +return rc; }static void _dpu_kms_drm_obj_destroy (struct dpu_kms *dpu_kms)@@ -517,7 +528,11 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)* Create encoder and query display drivers to create * bridges and connectors */ -_dpu_kms_setup_displays (dev, priv, dpu_kms); + ret =_dpu_kms_setup_displays (dev, priv, dpu_kms); +if (ret) { +DPU_ERROR ("failed to setup display, rc = %d\n" , ret); + goto fail; + } max_crtc_count =min (catalog->mixer_count, (u32 )dev->mode_config.num_encoder);Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
Signature · 5 lines
Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [PATCH v3 2/3] drm/msm/dpu: handle failures while initializing displays
Quoted 23 lines · Sean Paul
Quoted 9 lines · Jeykumar Sankaran
Bail out KMS hw init on display initialization failures with proper error logging.
changes in v3:
- introduced in the seriesSigned-off-by Jeykumar Sankaran <[email protected]>--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 39+++++++++++++++++++++++----------
Review
Quoted 3 lines
1 file changed, 27 insertions(+), 12 deletions(-)(quoted diff) +0
-0
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.cdrivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct
Quoted 4 lines
index 685686e..39c8549 100644
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +0
-0@@ -405,33 +405,36 @@ static void dpu_kms_wait_for_commit_done(struct
msm_kms*kms,Quoted 3 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +0
-0} }
Quoted 14 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct msm_kms *kms, · line 415
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +4
-2-static void _dpu_kms_initialize_dsi (struct drm_device *dev, +static int _dpu_kms_initialize_dsi (struct drm_device *dev,struct msm_drm_private *priv,struct dpu_kms *dpu_kms) {struct drm_encoder *encoder =NULL ; -int i, rc; +int i, rc =0 ; + +if (!(priv->dsi[0 ] || priv->dsi[1 ]))It'd be nice to not have to support both blocks if only one is hooked up. Perhaps move the TODO below above this check.
For dual DSI panels (e.g: MTP panels), both of these blocks will be populated. TODO comment below is to convey that the support for two independent single DSI panels is not yet available in DPU.
Thanks and Regards,
Jeykumar S.
Quoted 94 lines
Quoted 7 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-1+return rc;/*TODO: Support two independent DSI connectors */ encoder =dpu_encoder_init (dev,DRM_MODE_ENCODER_DSI ); -if (IS_ERR_OR_NULL (encoder)) { +if (IS_ERR (encoder)) {DPU_ERROR ("encoder init failed for dsi display\n" );drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct msm_kms *kms, · line 426
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-5-return ; +return PTR_ERR (encoder); }for (i =0 ; i <ARRAY_SIZE (priv->dsi); i++) { -if (!priv->dsi[i]) { -DPU_DEBUG ("invalid msm_dsi for ctrl %d\n" , i); -return ; - } +if (!priv->dsi[i])I don't think this is possible given the check at the beginning of the function
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct msm_kms *kms,
Quoted 5 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +1
-0+continue ; rc =msm_dsi_modeset_init (priv->dsi[i], dev, encoder);if (rc) { DPU_ERROR("modeset_init failed for dsi[%d], rc =%d\n",
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct msm_kms *kms, · line 437
Quoted 7 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +3
-1i, rc); -continue ; +return rc; } } + +return rc;nit: You can keep rc uninitialized above and just return 0 here.
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_initialize_dsi(struct
Quoted 4 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +0
-0}/** @@ -442,16 +445,24 @@ static void _dpu_kms_initialize_dsi(struct
drm_device*dev,drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_initialize_dsi(struct drm_device *dev, · line 452
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-2* @dpu_kms: Pointer to dpu kms structure * Returns: Zero on success */ -static void _dpu_kms_setup_displays (struct drm_device *dev, +static int _dpu_kms_setup_displays (struct drm_device *dev,struct msm_drm_private *priv,struct dpu_kms *dpu_kms) { -_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +int rc =0 ;nit: No need to initialize to 0
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static int _dpu_kms_drm_obj_init(struct dpu_kms
Quoted 17 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +8
-0+ + rc =_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +if (rc) { +DPU_ERROR ("failed to initialize dsi, rc = %d\n" , rc); +return rc; + }/** * Extend this function to initialize other * types of displays */ + +return rc; }static void _dpu_kms_drm_obj_destroy (struct dpu_kms *dpu_kms)@@ -517,7 +528,11 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms
*dpu_kms)Quoted 13 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +5
-2* Create encoder and query display drivers to create * bridges and connectors */ -_dpu_kms_setup_displays (dev, priv, dpu_kms); + ret =_dpu_kms_setup_displays (dev, priv, dpu_kms); +if (ret) { +DPU_ERROR ("failed to setup display, rc = %d\n" , ret); + goto fail; + } max_crtc_count =min (catalog->mixer_count, (u32 )dev->mode_config.num_encoder); --Quoted 1 line
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
Quoted 6 lines
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno
Signature · 5 lines
Jeykumar S _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [PATCH v3 2/3] drm/msm/dpu: handle failures while initializing displays
In reply to Sean PaulQuoted 48 lines · Sean Paul
Quoted 9 lines · Jeykumar Sankaran
Bail out KMS hw init on display initialization failures with proper error logging.
changes in v3:
- introduced in the seriesSigned-off-by Jeykumar Sankaran <[email protected]>--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 39+++++++++++++++++++++++----------
Review
Quoted 3 lines
1 file changed, 27 insertions(+), 12 deletions(-)(quoted diff) +0
-0
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.cdrivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct
Quoted 4 lines
index 685686e..39c8549 100644
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +0
-0@@ -405,33 +405,36 @@ static void dpu_kms_wait_for_commit_done(struct
msm_kms*kms,drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct msm_kms *kms, · line 415
Quoted 13 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +4
-2} } -static void _dpu_kms_initialize_dsi (struct drm_device *dev, +static int _dpu_kms_initialize_dsi (struct drm_device *dev,struct msm_drm_private *priv,struct dpu_kms *dpu_kms) {struct drm_encoder *encoder =NULL ; -int i, rc; +int i, rc =0 ; + +if (!(priv->dsi[0 ] || priv->dsi[1 ]))It'd be nice to not have to support both blocks if only one is hooked up. Perhaps move the TODO below above this check.
Quoted 11 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +3
-2+return rc;/*TODO: Support two independent DSI connectors */ encoder =dpu_encoder_init (dev,DRM_MODE_ENCODER_DSI ); -if (IS_ERR_OR_NULL (encoder)) { +if (IS_ERR (encoder)) {DPU_ERROR ("encoder init failed for dsi display\n" ); -return ; +return PTR_ERR (encoder); }
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct msm_kms *kms, · line 426
Quoted 6 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +1
-4for (i =0 ; i <ARRAY_SIZE (priv->dsi); i++) { -if (!priv->dsi[i]) { -DPU_DEBUG ("invalid msm_dsi for ctrl %d\n" , i); -return ; - } +if (!priv->dsi[i])I don't think this is possible given the check at the beginning of the function
The check at the beginning returns only if BOTH the dsi blocks are NULL. In case of full hd panel, only one of the dsi block will be active. The above check is needed to return on the inactive dsi.
Thanks,
Jeykumar S.
Quoted 73 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct msm_kms *kms,
Quoted 5 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +1
-0+continue ; rc =msm_dsi_modeset_init (priv->dsi[i], dev, encoder);if (rc) { DPU_ERROR("modeset_init failed for dsi[%d], rc =%d\n",
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct msm_kms *kms, · line 437
Quoted 7 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +3
-1i, rc); -continue ; +return rc; } } + +return rc;nit: You can keep rc uninitialized above and just return 0 here.
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_initialize_dsi(struct
Quoted 4 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +0
-0}/** @@ -442,16 +445,24 @@ static void _dpu_kms_initialize_dsi(struct
drm_device*dev,drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_initialize_dsi(struct drm_device *dev, · line 452
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-2* @dpu_kms: Pointer to dpu kms structure * Returns: Zero on success */ -static void _dpu_kms_setup_displays (struct drm_device *dev, +static int _dpu_kms_setup_displays (struct drm_device *dev,struct msm_drm_private *priv,struct dpu_kms *dpu_kms) { -_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +int rc =0 ;nit: No need to initialize to 0
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static int _dpu_kms_drm_obj_init(struct dpu_kms
Quoted 17 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +8
-0+ + rc =_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +if (rc) { +DPU_ERROR ("failed to initialize dsi, rc = %d\n" , rc); +return rc; + }/** * Extend this function to initialize other * types of displays */ + +return rc; }static void _dpu_kms_drm_obj_destroy (struct dpu_kms *dpu_kms)@@ -517,7 +528,11 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms
*dpu_kms)Quoted 13 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +5
-2* Create encoder and query display drivers to create * bridges and connectors */ -_dpu_kms_setup_displays (dev, priv, dpu_kms); + ret =_dpu_kms_setup_displays (dev, priv, dpu_kms); +if (ret) { +DPU_ERROR ("failed to setup display, rc = %d\n" , ret); + goto fail; + } max_crtc_count =min (catalog->mixer_count, (u32 )dev->mode_config.num_encoder); --Quoted 1 line
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
Quoted 6 lines
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno
Signature · 5 lines
Jeykumar S _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [PATCH v3 2/3] drm/msm/dpu: handle failures while initializing displays
In reply to a message not in the archiveQuoted 48 lines · Jeykumar Sankaran
Bail out KMS hw init on display initialization failures with proper error logging.
changes in v3:
- introduced in the seriesSigned-off-by Jeykumar Sankaran <[email protected]>--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 39 +++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 12 deletions(-)drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +9
-8@@ -405,33 +405,36 @@ static void dpu_kms_wait_for_commit_done(struct msm_kms *kms, } } -static void _dpu_kms_initialize_dsi (struct drm_device *dev, +static int _dpu_kms_initialize_dsi (struct drm_device *dev,struct msm_drm_private *priv,struct dpu_kms *dpu_kms) {struct drm_encoder *encoder =NULL ; -int i, rc; +int i, rc =0 ; + +if (!(priv->dsi[0 ] || priv->dsi[1 ])) +return rc;/*TODO: Support two independent DSI connectors */ encoder =dpu_encoder_init (dev,DRM_MODE_ENCODER_DSI ); -if (IS_ERR_OR_NULL (encoder)) { +if (IS_ERR (encoder)) {DPU_ERROR ("encoder init failed for dsi display\n" ); -return ; +return PTR_ERR (encoder); }for (i =0 ; i <ARRAY_SIZE (priv->dsi); i++) { -if (!priv->dsi[i]) { -DPU_DEBUG ("invalid msm_dsi for ctrl %d\n" , i); -return ; - } +if (!priv->dsi[i]) +continue ;
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void dpu_kms_wait_for_commit_done(struct msm_kms *kms, · line 437
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +3
-1rc =msm_dsi_modeset_init (priv->dsi[i], dev, encoder);if (rc) {DPU_ERROR ("modeset_init failed for dsi[%d], rc = %d\n" , i, rc); -continue ; +return rc; } } + +return rc;
This also looks like one of those cases where you can drop out of the if statement to return rc.
Quoted 17 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +5
-2}/** @@ -442,16 +445,24 @@ static void _dpu_kms_initialize_dsi(struct drm_device *dev,* @dpu_kms: Pointer to dpu kms structure * Returns: Zero on success */ -static void _dpu_kms_setup_displays (struct drm_device *dev, +static int _dpu_kms_setup_displays (struct drm_device *dev,struct msm_drm_private *priv,struct dpu_kms *dpu_kms) { -_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +int rc =0 ; + + rc =_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +if (rc) {
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_initialize_dsi(struct drm_device *dev, · line 465
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +5
-0+DPU_ERROR ("failed to initialize dsi, rc = %d\n" , rc); +return rc; + }/** * Extend this function to initialize other * types of displays */ + +return rc;
Here too.
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms) · line 533
Quoted 11 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +3
-1}static void _dpu_kms_drm_obj_destroy (struct dpu_kms *dpu_kms)@@ -517,7 +528,11 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)* Create encoder and query display drivers to create * bridges and connectors */ -_dpu_kms_setup_displays (dev, priv, dpu_kms); + ret =_dpu_kms_setup_displays (dev, priv, dpu_kms); +if (ret) { +DPU_ERROR ("failed to setup display, rc = %d\n" , ret);
You don't need to pile on with another error message.
Quoted 5 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-0+ goto fail; + } max_crtc_count =min (catalog->mixer_count, (u32 )dev->mode_config.num_encoder);
Signature · 6 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
PATCH v3 3/3 drm/msm/dpu: add display port support in DPU
In reply to a message not in the archiveAdd 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)
[1] https://lwn.net/Articles/768265/
[2] https://lkml.org/lkml/2018/11/17/87
--- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 51 +++++++++++++++++++++++++---- 2 files changed, 49 insertions(+), 10 deletions(-)
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +4 -4
@@ -2029,7 +2029,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) {
@@ -2052,9 +2052,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 );
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +45 -6
70 lines not on this page · raw message
Signature · 7 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
A message that is not in the archive (sent off-list or not yet received): 1544727064-26763-3-git-send-email-jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org
Sean Paul
Reviewed-byRe: [PATCH v3 3/3] drm/msm/dpu: add display port support in DPU
Quoted 15 lines · 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)
Quoted 4 lines
[1] https://lwn.net/Articles/768265/
[2] https://lkml.org/lkml/2018/11/17/87Signed-off-by Jeykumar Sankaran <[email protected]>
LGTM, thanks for revising.
I'll land the first 2 patches (after the 2nd one is fixed up), and Chandan can roll this patch into his set when he sends the new version.
Quoted 112 lines
--- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 51 +++++++++++++++++++++++++---- 2 files changed, 49 insertions(+), 10 deletions(-)drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +4
-4@@ -2029,7 +2029,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) {@@ -2052,9 +2052,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 );drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +45
-6@@ -437,6 +437,32 @@ 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 =0 ; + +if (!priv->dp) +return rc; + + 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; + } + +return rc; +} +/** * _dpu_kms_setup_displays - create encoders, bridges and connectors * for underlying displays @@ -457,6 +483,12 @@ static int _dpu_kms_setup_displays(struct drm_device *dev,return rc; } + rc =_dpu_kms_initialize_displayport (dev, priv, dpu_kms); +if (rc) { +DPU_ERROR ("failed to initialize display port, rc = %d\n" , rc); +return rc; + } +/** * Extend this function to initialize other * types of displays @@ -675,13 +707,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, &info);if (rc)Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Signature · 5 lines
Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [PATCH v3 3/3] drm/msm/dpu: add display port support in DPU
In reply to a message not in the archiveQuoted 15 lines · 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)
Quoted 2 lines
[1] https://lwn.net/Articles/768265/
[2] https://lkml.org/lkml/2018/11/17/87
Two more nits - with these: Reviewed-by: Jordan Crouse <[email protected]>
Quoted 51 lines
Signed-off-by Jeykumar Sankaran <[email protected]>--- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 ++--- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 51 +++++++++++++++++++++++++---- 2 files changed, 49 insertions(+), 10 deletions(-)drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +4
-4@@ -2029,7 +2029,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) {@@ -2052,9 +2052,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 );drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +11
-0@@ -437,6 +437,32 @@ 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 =0 ; + +if (!priv->dp) +return rc; + + encoder =dpu_encoder_init (dev,DRM_MODE_ENCODER_TMDS );
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static int _dpu_kms_initialize_dsi(struct drm_device *dev, · line 460
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +10
-0+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;
Return rc isn't needed here because you also return rc immediately after the block.
Quoted 9 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +5
-0+ } + +return rc; +} +/** * _dpu_kms_setup_displays - create encoders, bridges and connectors * for underlying displays
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static int _dpu_kms_setup_displays(struct drm_device *dev, · line 488
Quoted 6 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +3
-0@@ -457,6 +483,12 @@ static int _dpu_kms_setup_displays(struct drm_device *dev,return rc; } + rc =_dpu_kms_initialize_displayport (dev, priv, dpu_kms); +if (rc) { +DPU_ERROR ("failed to initialize display port, rc = %d\n" , rc);
_dpu_kms_initialize_displayport() already prints error messages in every path so you don't need to pile on.
Quoted 33 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +16
-6+return rc; + } +/** * Extend this function to initialize other * types of displays @@ -675,13 +707,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, &info);if (rc)
Signature · 6 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno