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 <jcrouse@xxxxxxxxxxxxxx> > Signed-off-by: Jeykumar Sankaran <jsanka@xxxxxxxxxxxxxx> > --- > 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, &info); > if (rc) -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
