[PATCH v3 3/7] drm/msm/dpu: support setting up two independent DSI connectors
Dmitry Baryshkov
dmitry.baryshkov at linaro.org
Sat Jul 17 12:35:21 UTC 2021
On 13/07/2021 00:15, abhinavk at codeaurora.org wrote:
> On 2021-07-10 15:20, Dmitry Baryshkov wrote:
>> Move setting up encoders from set_encoder_mode to
>> _dpu_kms_initialize_dsi() / _dpu_kms_initialize_displayport(). This
>> allows us to support not only "single DSI" and "bonded DSI" but also "two
>> independent DSI" configurations. In future this would also help adding
>> support for multiple DP connectors.
>>
>
> This looks quite neat now,so i am okay with this version of it:
>
> Reviewed-by: Abhinav Kumar <abhinavk at codeaurora.org>
>
> Just a suggestion, since we are only supporting two dsis so far, do we need
> an extra variable to get the other DSI? Can't we just do priv->dsi[DSI_1]?
> as usually DSI_0 is the master
I still hope to fix "swap links in bonded DSI mode" (in other words DSI
0 being clock master, but driving right half of the screen) one day.
Thus I don't think we should enforce DSI_0/DSI_1 here.
>
>> + int other = (i + 1) % 2;
>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 102 +++++++++++++-----------
>> 1 file changed, 57 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> index 1d3a4f395e74..3cd2011e18d4 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> @@ -471,30 +471,68 @@ static int _dpu_kms_initialize_dsi(struct
>> drm_device *dev,
>> struct dpu_kms *dpu_kms)
>> {
>> struct drm_encoder *encoder = NULL;
>> + struct msm_display_info info;
>> 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(encoder)) {
>> - DPU_ERROR("encoder init failed for dsi display\n");
>> - return PTR_ERR(encoder);
>> - }
>> -
>> - priv->encoders[priv->num_encoders++] = encoder;
>> -
>> + /*
>> + * We support following confiurations:
>> + * - Single DSI host (dsi0 or dsi1)
>> + * - Two independent DSI hosts
>> + * - Bonded DSI0 and DSI1 hosts
>> + *
>> + * TODO: Support swapping DSI0 and DSI1 in the bonded setup.
>> + */
>> for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
>> + int other = (i + 1) % 2;
>> +
>> if (!priv->dsi[i])
>> continue;
>>
>> + if (msm_dsi_is_bonded_dsi(priv->dsi[i]) &&
>> + !msm_dsi_is_master_dsi(priv->dsi[i]))
>> + continue;
>> +
>> + encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
>> + if (IS_ERR(encoder)) {
>> + DPU_ERROR("encoder init failed for dsi display\n");
>> + return PTR_ERR(encoder);
>> + }
>> +
>> + priv->encoders[priv->num_encoders++] = encoder;
>> +
>> + memset(&info, 0, sizeof(info));
>> + info.intf_type = encoder->encoder_type;
>> +
>> 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);
>> break;
>> }
>> +
>> + info.h_tile_instance[info.num_of_h_tiles++] = i;
>> + info.capabilities = msm_dsi_is_cmd_mode(priv->dsi[i]) ?
>> + MSM_DISPLAY_CAP_CMD_MODE :
>> + MSM_DISPLAY_CAP_VID_MODE;
>> +
>> + if (msm_dsi_is_bonded_dsi(priv->dsi[i]) && priv->dsi[other]) {
>> + rc = msm_dsi_modeset_init(priv->dsi[other], dev, encoder);
>> + if (rc) {
>> + DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
>> + other, rc);
>> + break;
>> + }
>> +
>> + info.h_tile_instance[info.num_of_h_tiles++] = other;
>> + }
>> +
>> + rc = dpu_encoder_setup(dev, encoder, &info);
>> + if (rc)
>> + DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
>> + encoder->base.id, rc);
>> }
>>
>> return rc;
>> @@ -505,6 +543,7 @@ static int _dpu_kms_initialize_displayport(struct
>> drm_device *dev,
>> struct dpu_kms *dpu_kms)
>> {
>> struct drm_encoder *encoder = NULL;
>> + struct msm_display_info info;
>> int rc = 0;
>>
>> if (!priv->dp)
>> @@ -516,6 +555,7 @@ static int _dpu_kms_initialize_displayport(struct
>> drm_device *dev,
>> return PTR_ERR(encoder);
>> }
>>
>> + memset(&info, 0, sizeof(info));
>> rc = msm_dp_modeset_init(priv->dp, dev, encoder);
>> if (rc) {
>> DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
>> @@ -524,6 +564,14 @@ static int _dpu_kms_initialize_displayport(struct
>> drm_device *dev,
>> }
>>
>> priv->encoders[priv->num_encoders++] = encoder;
>> +
>> + info.num_of_h_tiles = 1;
>> + info.capabilities = MSM_DISPLAY_CAP_VID_MODE;
>> + info.intf_type = encoder->encoder_type;
>> + rc = dpu_encoder_setup(dev, encoder, &info);
>> + if (rc)
>> + DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
>> + encoder->base.id, rc);
>> return rc;
>> }
>>
>> @@ -726,41 +774,6 @@ static void dpu_kms_destroy(struct msm_kms *kms)
>> msm_kms_destroy(&dpu_kms->base);
>> }
>>
>> -static void _dpu_kms_set_encoder_mode(struct msm_kms *kms,
>> - struct drm_encoder *encoder,
>> - bool cmd_mode)
>> -{
>> - struct msm_display_info info;
>> - struct msm_drm_private *priv = encoder->dev->dev_private;
>> - int i, rc = 0;
>> -
>> - memset(&info, 0, sizeof(info));
>> -
>> - info.intf_type = encoder->encoder_type;
>> - info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
>> - MSM_DISPLAY_CAP_VID_MODE;
>> -
>> - 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)
>> - DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
>> - encoder->base.id, rc);
>> -}
>> -
>> static irqreturn_t dpu_irq(struct msm_kms *kms)
>> {
>> struct dpu_kms *dpu_kms = to_dpu_kms(kms);
>> @@ -863,7 +876,6 @@ static const struct msm_kms_funcs kms_funcs = {
>> .get_format = dpu_get_msm_format,
>> .round_pixclk = dpu_kms_round_pixclk,
>> .destroy = dpu_kms_destroy,
>> - .set_encoder_mode = _dpu_kms_set_encoder_mode,
>> .snapshot = dpu_kms_mdp_snapshot,
>> #ifdef CONFIG_DEBUG_FS
>> .debugfs_init = dpu_kms_debugfs_init,
--
With best wishes
Dmitry
More information about the dri-devel
mailing list