drm/msm/dpu: add display port support in DPU
PATCH v2 1/1 drm/msm/dpu: add display port support in DPU
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)
[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 | 47 +++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 10 deletions(-)
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +4 -4
@@ -2015,7 +2015,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) {
@@ -2038,9 +2038,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 +41 -6
66 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 _______________________________________________ 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): 1543357710-3918-1-git-send-email-jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org
kbuild test robot
build botThank you for the patch! Yet something to improve:
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-rc4 next-20181130]
[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-add-display-port-support-in-DPU/20181130-230021 base: git://people.freedesktop.org/~robclark/linux msm-next config: arm-allmodconfig (attached as .config) compiler: arm-linux-gnueabi-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=arm
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:483: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:492: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:492:33: error: 'struct msm_drm_private' has no member named 'dp'; did you mean 'edp'?
rc = msm_dp_modeset_init(priv->dp, dev, encoder);
^~
edpcc1: some warnings being treated as errors
vim +483 drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c
475
476 static void _dpu_kms_initialize_displayport(struct drm_device *dev,
477 struct msm_drm_private *priv,
478 struct dpu_kms *dpu_kms)
479 {
480 struct drm_encoder *encoder = NULL;
481 int rc;
482
> 483 if (!priv->dp)
484 return;
485
486 encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
487 if (IS_ERR(encoder)) {
488 DPU_ERROR("encoder init failed for dsi display\n");
489 return;
490 }
491
> 492 rc = msm_dp_modeset_init(priv->dp, dev, encoder); 493 if (rc) {
494 DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
495 drm_encoder_cleanup(encoder);
496 return;
497 }
498
499 priv->encoders[priv->num_encoders++] = encoder;
500 }
501
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno
Quoted 26 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)[1] https://lwn.net/Articles/768265/
[2] https://lkml.org/lkml/2018/11/17/87Signed-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 | 47 +++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 10 deletions(-)drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +0
-0
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, · line 2018
Quoted 5 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +1
-1@@ -2015,7 +2015,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 ;
dpu_intf_type seems unnecessary, you could just use the DRM_MODE_ENCODER_* value directly?
Quoted 38 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +3
-3struct dpu_enc_phys_init_params phys_params;if (!dpu_enc || !dpu_kms) {@@ -2038,9 +2038,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 +14
-0@@ -473,6 +473,32 @@ static void _dpu_kms_initialize_dsi(struct drm_device *dev, } } +static void _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 ; + + encoder =dpu_encoder_init (dev,DRM_MODE_ENCODER_TMDS ); +if (IS_ERR (encoder)) { +DPU_ERROR ("encoder init failed for dsi display\n" ); +return ;
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_initialize_dsi(struct drm_device *dev, · line 499
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +10
-0+ } + + 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 ; + } + + priv->encoders[priv->num_encoders++] = encoder;
No need to keep track of drm resources at the driver level, the core will do this for you. So can you please add a patch preceding this one to remove the priv->encoders/crtc/planes/connectors arrays?
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_setup_displays(struct drm_device *dev,
Quoted 6 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-0+} +/** * _dpu_kms_setup_displays - create encoders, bridges and connectors * for underlying displays @@ -487,6 +513,8 @@ static void _dpu_kms_setup_displays(struct drm_device *dev,
Why are these functions voids? Seems like there are plenty of places for them to fail :)
Let's add a patch to the beginning of this series to properly handle failures in setup_displays and initialize_dsi
Quoted 43 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +15
-6{_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +_dpu_kms_initialize_displayport (dev, priv, dpu_kms); +/** * Extend this function to initialize other * types of displays @@ -723,13 +751,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 · 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
Quoted 90 lines · Sean Paul
Quoted 12 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)Quoted 7 lines
[1] https://lwn.net/Articles/768265/
[2] https://lkml.org/lkml/2018/11/17/87Signed-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 | 47+++++++++++++++++++++++++----
Review
Quoted 3 lines
2 files changed, 45 insertions(+), 10 deletions(-)(quoted diff) +0
-0
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.cdrivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c static int dpu_encoder_setup_display(struct
Quoted 4 lines
index d3f4501..1f6b4b1 100644
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +0
-0@@ -2015,7 +2015,7 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt*dpu_enc,drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, · line 2018
Quoted 5 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +1
-1{int ret =0 ;int i =0 ; -enum dpu_intf_type intf_type; +enum dpu_intf_type intf_type =INTF_NONE ;
dpu_intf_typeseems unnecessary, you could just use the DRM_MODE_ENCODER_* value
directly?drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c static int dpu_encoder_setup_display(struct
Quoted 4 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +0
-0struct dpu_enc_phys_init_params phys_params;if (!dpu_enc || !dpu_kms) {@@ -2038,9 +2038,9 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt*dpu_enc,drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
Quoted 5 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +0
-2case DRM_MODE_ENCODER_DSI : intf_type =INTF_DSI ;break ; -default : - DPU_ERROR_ENC(dpu_enc, "unsupported display interfacetype\n");
Review
Quoted 8 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +3
-1-return -EINVAL ; +case DRM_MODE_ENCODER_TMDS : + intf_type =INTF_DP ; +break ; }WARN_ON (disp_info->num_of_h_tiles <1 );(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_initialize_dsi(struct
Quoted 4 lines
index 985c855..7d931ae 100644
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +0
-0@@ -473,6 +473,32 @@ static void _dpu_kms_initialize_dsi(struct
drm_device*dev,Quoted 25 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +22
-0} } +static void _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 ; + + encoder =dpu_encoder_init (dev,DRM_MODE_ENCODER_TMDS ); +if (IS_ERR (encoder)) { +DPU_ERROR ("encoder init failed for dsi display\n" ); +return ; + } + + 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 ; + }
Quoted 9 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_initialize_dsi(struct · line 499
Quoted 2 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-0+ + priv->encoders[priv->num_encoders++] = encoder;No need to keep track of drm resources at the driver level, the core will do
this for you. So can you please add a patch preceding this one to remove the
priv->encoders/crtc/planes/connectors arrays?
priv arrays for tracking drm components and priv->num_** counters are introduced by mdp4/5. DPU just adapted the implementation.
De-coupling DPU from these arrays is much easier than fixing them in mdp4/5. I see mdp5 is using it to track the max resources and also in ./msm_fbdev.c.
Thanks and Regards,
Jeykumar S.
Quoted 62 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_setup_displays(struct
Quoted 6 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-0+} +/** * _dpu_kms_setup_displays - create encoders, bridges and connectors * for underlying displays @@ -487,6 +513,8 @@ static void _dpu_kms_setup_displays(struct
drm_device*dev,Why are these functions voids? Seems like there are plenty of places for them to
fail :)Let's add a patch to the beginning of this series to properly handle failures in
setup_displaysandinitialize_dsidrivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_set_encoder_mode(struct
Quoted 9 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-0{_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +_dpu_kms_initialize_displayport (dev, priv, dpu_kms); +/** * Extend this function to initialize other * types of displays @@ -723,13 +751,20 @@ static void _dpu_kms_set_encoder_mode(struct
msm_kms*kms,Quoted 4 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +0
-1info.capabilities = cmd_mode ?MSM_DISPLAY_CAP_CMD_MODE :MSM_DISPLAY_CAP_VID_MODE ; -/* TODO: No support for DSI swap */ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_set_encoder_mode(struct msm_kms *kms,
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +6
-4-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;
Quoted 13 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +7
-2+ 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) --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
A message that is not in the archive (sent off-list or not yet received): 3ee2493cf6b31c903add62e837364056-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org
Quoted 102 lines · Jeykumar Sankaran
Quoted 92 lines · Sean Paul
Quoted 12 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)Quoted 7 lines
[1] https://lwn.net/Articles/768265/
[2] https://lkml.org/lkml/2018/11/17/87Signed-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 | 47+++++++++++++++++++++++++----
Review
Quoted 3 lines
2 files changed, 45 insertions(+), 10 deletions(-)(quoted diff) +0
-0
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.cdrivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c static int dpu_encoder_setup_display(struct
Quoted 4 lines
index d3f4501..1f6b4b1 100644
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +0
-0@@ -2015,7 +2015,7 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt*dpu_enc,drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, · line 2018
Quoted 5 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +1
-1{int ret =0 ;int i =0 ; -enum dpu_intf_type intf_type; +enum dpu_intf_type intf_type =INTF_NONE ;
dpu_intf_typeseems unnecessary, you could just use the DRM_MODE_ENCODER_*
value
directly?drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c static int dpu_encoder_setup_display(struct
Quoted 4 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +0
-0struct dpu_enc_phys_init_params phys_params;if (!dpu_enc || !dpu_kms) {@@ -2038,9 +2038,9 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt*dpu_enc,drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
Quoted 5 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +0
-2case DRM_MODE_ENCODER_DSI : intf_type =INTF_DSI ;break ; -default : - DPU_ERROR_ENC(dpu_enc, "unsupported display interfacetype\n");
Review
Quoted 8 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +3
-1-return -EINVAL ; +case DRM_MODE_ENCODER_TMDS : + intf_type =INTF_DP ; +break ; }WARN_ON (disp_info->num_of_h_tiles <1 );(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_initialize_dsi(struct
Quoted 4 lines
index 985c855..7d931ae 100644
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +0
-0@@ -473,6 +473,32 @@ static void _dpu_kms_initialize_dsi(struct
drm_device*dev,Quoted 26 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +23
-0} } +static void _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 ; + + encoder =dpu_encoder_init (dev,DRM_MODE_ENCODER_TMDS ); +if (IS_ERR (encoder)) { +DPU_ERROR ("encoder init failed for dsi display\n" ); +return ; + } + + 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 ; + } +Quoted 9 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_initialize_dsi(struct drm_device *dev, · line 499
Quoted 1 line
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +1
-0+ priv->encoders[priv->num_encoders++] = encoder;No need to keep track of drm resources at the driver level, the core will
do
this for you. So can you please add a patch preceding this one to remove the
priv->encoders/crtc/planes/connectors arrays?
Quoted 7 lines
priv arrays for tracking drm components and priv->num_** counters are introduced
by mdp4/5. DPU just adapted the implementation.De-coupling DPU from these arrays is much easier than fixing them in mdp4/5. I see
mdp5 is using it to track the max resources and also in ./msm_fbdev.c.
Ok, I'll take a look at it
Sean
Quoted 68 lines
Thanks and Regards,
Jeykumar S.Quoted 62 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_setup_displays(struct
Quoted 6 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-0+} +/** * _dpu_kms_setup_displays - create encoders, bridges and connectors * for underlying displays @@ -487,6 +513,8 @@ static void _dpu_kms_setup_displays(struct
drm_device*dev,Why are these functions voids? Seems like there are plenty of places for them to
fail :)Let's add a patch to the beginning of this series to properly handle failures in
setup_displaysandinitialize_dsidrivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_set_encoder_mode(struct
Quoted 9 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-0{_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +_dpu_kms_initialize_displayport (dev, priv, dpu_kms); +/** * Extend this function to initialize other * types of displays @@ -723,13 +751,20 @@ static void _dpu_kms_set_encoder_mode(struct
msm_kms*kms,Quoted 4 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +0
-1info.capabilities = cmd_mode ?MSM_DISPLAY_CAP_CMD_MODE :MSM_DISPLAY_CAP_VID_MODE ; -/* TODO: No support for DSI swap */ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_set_encoder_mode(struct msm_kms *kms,
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +6
-4-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;
Quoted 13 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +7
-2+ 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) --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/freedrenoSignature · 1 line
Jeykumar S
Signature · 5 lines
Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
Quoted 30 lines · Sean Paul
Quoted 12 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)Quoted 7 lines
[1] https://lwn.net/Articles/768265/
[2] https://lkml.org/lkml/2018/11/17/87Signed-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 | 47+++++++++++++++++++++++++----
Review
Quoted 3 lines
2 files changed, 45 insertions(+), 10 deletions(-)(quoted diff) +0
-0
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.cdrivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c static int dpu_encoder_setup_display(struct
Quoted 4 lines
index d3f4501..1f6b4b1 100644
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +0
-0@@ -2015,7 +2015,7 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt*dpu_enc,
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, · line 2018
Quoted 5 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +1
-1{int ret =0 ;int i =0 ; -enum dpu_intf_type intf_type; +enum dpu_intf_type intf_type =INTF_NONE ;
dpu_intf_typeseems unnecessary, you could just use the DRM_MODE_ENCODER_* value
directly?
enum dpu_intf_type enumerates HW interface types the SOC has. Below switch case maps the DRM_MODE_ENCODER_* to HW dpu_intf_type it should reserve. Note that DRM_MODE_ENCODER_* and dpu_intf_type are not mapped 1-to-1. e.g. DRM_MODE_ENCODER_TMDS can be mapped to HDMI or DisplayPort.
Thanks,
Jeykumar S.
Quoted 121 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c static int dpu_encoder_setup_display(struct
Quoted 4 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +0
-0struct dpu_enc_phys_init_params phys_params;if (!dpu_enc || !dpu_kms) {@@ -2038,9 +2038,9 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt*dpu_enc,drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
Quoted 5 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +0
-2case DRM_MODE_ENCODER_DSI : intf_type =INTF_DSI ;break ; -default : - DPU_ERROR_ENC(dpu_enc, "unsupported display interfacetype\n");
Review
Quoted 8 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +3
-1-return -EINVAL ; +case DRM_MODE_ENCODER_TMDS : + intf_type =INTF_DP ; +break ; }WARN_ON (disp_info->num_of_h_tiles <1 );(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_initialize_dsi(struct
Quoted 4 lines
index 985c855..7d931ae 100644
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +0
-0@@ -473,6 +473,32 @@ static void _dpu_kms_initialize_dsi(struct
drm_device*dev,Quoted 17 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +14
-0} } +static void _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 ; + + encoder =dpu_encoder_init (dev,DRM_MODE_ENCODER_TMDS ); +if (IS_ERR (encoder)) { +DPU_ERROR ("encoder init failed for dsi display\n" ); +return ;drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_initialize_dsi(struct drm_device *dev, · line 499
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +10
-0+ } + + 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 ; + } + + priv->encoders[priv->num_encoders++] = encoder;No need to keep track of drm resources at the driver level, the core will do
this for you. So can you please add a patch preceding this one to remove the
priv->encoders/crtc/planes/connectors arrays?drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_setup_displays(struct
Quoted 6 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-0+} +/** * _dpu_kms_setup_displays - create encoders, bridges and connectors * for underlying displays @@ -487,6 +513,8 @@ static void _dpu_kms_setup_displays(struct
drm_device*dev,Why are these functions voids? Seems like there are plenty of places for them to
fail :)Let's add a patch to the beginning of this series to properly handle failures in
setup_displaysandinitialize_dsidrivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_set_encoder_mode(struct
Quoted 9 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +2
-0{_dpu_kms_initialize_dsi (dev, priv, dpu_kms); +_dpu_kms_initialize_displayport (dev, priv, dpu_kms); +/** * Extend this function to initialize other * types of displays @@ -723,13 +751,20 @@ static void _dpu_kms_set_encoder_mode(struct
msm_kms*kms,Quoted 4 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +0
-1info.capabilities = cmd_mode ?MSM_DISPLAY_CAP_CMD_MODE :MSM_DISPLAY_CAP_VID_MODE ; -/* TODO: No support for DSI swap */ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c static void _dpu_kms_set_encoder_mode(struct msm_kms *kms,
Quoted 10 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +6
-4-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;
Quoted 13 lines
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +7
-2+ 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) --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