Re: [PATCH v3 3/3] drm/msm/dpu: add display port support in DPU
On Thu, Dec 13, 2018 at 10:51:04AM -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)
>
> [1] https://lwn.net/Articles/768265/
> [2] https://lkml.org/lkml/2018/11/17/87
Two more nits - with these: Reviewed-by: Jordan Crouse
> Signed-off-by: Jeykumar Sankaran
> ---
> 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(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 36158b7..b79fd9d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -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);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 39c8549..ba3e75c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -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 isn't needed here because you also return rc immediately after the
block.
> + }
> +
> + 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);
_dpu_kms_initialize_displayport() already prints error messages in every path
so you don't need to pile on.
> + 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, );
> if (rc)
--
The Qualcomm Innovation Center, Inc. is a member
Re: [PATCH v3 3/3] drm/msm/dpu: add display port support in DPU
On Thu, Dec 13, 2018 at 10:51:04AM -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)
>
> [1] https://lwn.net/Articles/768265/
> [2] https://lkml.org/lkml/2018/11/17/87
>
> Signed-off-by: Jeykumar Sankaran
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.
Reviewed-by: Sean Paul
> ---
> 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(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 36158b7..b79fd9d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -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);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 39c8549..ba3e75c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -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, );
> if (rc)
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation
[PATCH v3 3/3] 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)
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
Signed-off-by: Jeykumar Sankaran
---
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(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 36158b7..b79fd9d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -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);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 39c8549..ba3e75c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -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, );
if (rc)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
