| Message ID | 20220215141643.3444941-9-dmitry.baryshkov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm/dpu: cleanup dpu encoder code | expand |
On Tue 15 Feb 08:16 CST 2022, Dmitry Baryshkov wrote: > Rather than passing DRM_MODE_ENCODER_* and letting dpu_encoder to guess, > which intf type we mean, pass INTF_DSI/INTF_DP directly. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 26 +++++++-------------- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 ++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 ++-- > 3 files changed, 13 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index fa1dc088a672..597d40f78d38 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -490,7 +490,7 @@ void dpu_encoder_helper_split_config( > hw_mdptop = phys_enc->hw_mdptop; > disp_info = &dpu_enc->disp_info; > > - if (disp_info->intf_type != DRM_MODE_ENCODER_DSI) > + if (disp_info->intf_type != INTF_DSI) > return; > > /** > @@ -552,7 +552,7 @@ static struct msm_display_topology dpu_encoder_get_topology( > else > topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1; > > - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) { > + if (dpu_enc->disp_info.intf_type == INTF_DSI) { > if (dpu_kms->catalog->dspp && > (dpu_kms->catalog->dspp_count >= topology.num_lm)) > topology.num_dspp = topology.num_lm; > @@ -1074,7 +1074,7 @@ static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc) > } > > > - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_TMDS && > + if (dpu_enc->disp_info.intf_type == INTF_DP && > dpu_enc->cur_master->hw_mdptop && > dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select) > dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select( > @@ -1082,7 +1082,7 @@ static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc) > > _dpu_encoder_update_vsync_source(dpu_enc, &dpu_enc->disp_info); > > - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI && > + if (dpu_enc->disp_info.intf_type == INTF_DSI && > !WARN_ON(dpu_enc->num_phys_encs == 0)) { > unsigned bpc = dpu_enc->connector->display_info.bpc; > for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) { > @@ -1949,7 +1949,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, > { > int ret = 0; > int i = 0; > - enum dpu_intf_type intf_type = INTF_NONE; > struct dpu_enc_phys_init_params phys_params; > > if (!dpu_enc) { > @@ -1965,15 +1964,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, > phys_params.parent_ops = &dpu_encoder_parent_ops; > phys_params.enc_spinlock = &dpu_enc->enc_spinlock; > > - switch (disp_info->intf_type) { > - case DRM_MODE_ENCODER_DSI: > - intf_type = INTF_DSI; > - break; > - case DRM_MODE_ENCODER_TMDS: > - intf_type = INTF_DP; > - break; > - } > - > WARN_ON(disp_info->num_of_h_tiles < 1); > > DPU_DEBUG("dsi_info->num_of_h_tiles %d\n", disp_info->num_of_h_tiles); > @@ -2005,11 +1995,11 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, > i, controller_id, phys_params.split_role); > > phys_params.intf_idx = dpu_encoder_get_intf(dpu_kms->catalog, > - intf_type, > - controller_id); > + disp_info->intf_type, > + controller_id); > if (phys_params.intf_idx == INTF_MAX) { > DPU_ERROR_ENC(dpu_enc, "could not get intf: type %d, id %d\n", > - intf_type, controller_id); > + disp_info->intf_type, controller_id); > ret = -EINVAL; > } > > @@ -2092,7 +2082,7 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, > timer_setup(&dpu_enc->frame_done_timer, > dpu_encoder_frame_done_timeout, 0); > > - if (disp_info->intf_type == DRM_MODE_ENCODER_DSI) > + if (disp_info->intf_type == INTF_DSI) > timer_setup(&dpu_enc->vsync_event_timer, > dpu_encoder_vsync_event_handler, > 0); > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > index ebe3944355bb..3891bcbbe5a4 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > @@ -36,7 +36,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder *encoder, > > /** > * struct msm_display_info - defines display properties > - * @intf_type: DRM_MODE_ENCODER_ type > + * @intf_type: INTF_ 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 > @@ -45,7 +45,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder *encoder, > * used instead of panel TE in cmd mode panels > */ > struct msm_display_info { > - int intf_type; > + enum dpu_intf_type intf_type; > uint32_t capabilities; > uint32_t num_of_h_tiles; > uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY]; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index 5f0dc44119c9..bca4f05a5782 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -564,7 +564,7 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev, > priv->encoders[priv->num_encoders++] = encoder; > > memset(&info, 0, sizeof(info)); > - info.intf_type = encoder->encoder_type; > + info.intf_type = INTF_DSI; > > rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder); > if (rc) { > @@ -630,7 +630,8 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev, > info.num_of_h_tiles = 1; > info.h_tile_instance[0] = i; > info.capabilities = MSM_DISPLAY_CAP_VID_MODE; > - info.intf_type = encoder->encoder_type; > + /* FIXME: HW catalog treats both DP and eDP interfaces as INTF_DP */ Is this really a problem? Don't we need to handle the differences between eDP and DP on a case-by-case basis anyways. E.g. not all DP interfaces seems to have audio support etc. Either way, thanks for clearing this up! Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > + info.intf_type = INTF_DP; > rc = dpu_encoder_setup(dev, encoder, &info); > if (rc) { > DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n", > -- > 2.34.1 >
On 2/15/2022 6:16 AM, Dmitry Baryshkov wrote: > Rather than passing DRM_MODE_ENCODER_* and letting dpu_encoder to guess, > which intf type we mean, pass INTF_DSI/INTF_DP directly. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 26 +++++++-------------- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 ++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 ++-- > 3 files changed, 13 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index fa1dc088a672..597d40f78d38 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -490,7 +490,7 @@ void dpu_encoder_helper_split_config( > hw_mdptop = phys_enc->hw_mdptop; > disp_info = &dpu_enc->disp_info; > > - if (disp_info->intf_type != DRM_MODE_ENCODER_DSI) > + if (disp_info->intf_type != INTF_DSI) > return; > > /** > @@ -552,7 +552,7 @@ static struct msm_display_topology dpu_encoder_get_topology( > else > topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1; > > - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) { > + if (dpu_enc->disp_info.intf_type == INTF_DSI) { > if (dpu_kms->catalog->dspp && > (dpu_kms->catalog->dspp_count >= topology.num_lm)) > topology.num_dspp = topology.num_lm; > @@ -1074,7 +1074,7 @@ static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc) > } > > > - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_TMDS && > + if (dpu_enc->disp_info.intf_type == INTF_DP && > dpu_enc->cur_master->hw_mdptop && > dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select) > dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select( > @@ -1082,7 +1082,7 @@ static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc) > > _dpu_encoder_update_vsync_source(dpu_enc, &dpu_enc->disp_info); > > - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI && > + if (dpu_enc->disp_info.intf_type == INTF_DSI && > !WARN_ON(dpu_enc->num_phys_encs == 0)) { > unsigned bpc = dpu_enc->connector->display_info.bpc; > for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) { > @@ -1949,7 +1949,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, > { > int ret = 0; > int i = 0; > - enum dpu_intf_type intf_type = INTF_NONE; > struct dpu_enc_phys_init_params phys_params; > > if (!dpu_enc) { > @@ -1965,15 +1964,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, > phys_params.parent_ops = &dpu_encoder_parent_ops; > phys_params.enc_spinlock = &dpu_enc->enc_spinlock; > > - switch (disp_info->intf_type) { > - case DRM_MODE_ENCODER_DSI: > - intf_type = INTF_DSI; > - break; > - case DRM_MODE_ENCODER_TMDS: > - intf_type = INTF_DP; > - break; > - } > - > WARN_ON(disp_info->num_of_h_tiles < 1); > > DPU_DEBUG("dsi_info->num_of_h_tiles %d\n", disp_info->num_of_h_tiles); > @@ -2005,11 +1995,11 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, > i, controller_id, phys_params.split_role); > > phys_params.intf_idx = dpu_encoder_get_intf(dpu_kms->catalog, > - intf_type, > - controller_id); > + disp_info->intf_type, > + controller_id); > if (phys_params.intf_idx == INTF_MAX) { > DPU_ERROR_ENC(dpu_enc, "could not get intf: type %d, id %d\n", > - intf_type, controller_id); > + disp_info->intf_type, controller_id); > ret = -EINVAL; > } > > @@ -2092,7 +2082,7 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, > timer_setup(&dpu_enc->frame_done_timer, > dpu_encoder_frame_done_timeout, 0); > > - if (disp_info->intf_type == DRM_MODE_ENCODER_DSI) > + if (disp_info->intf_type == INTF_DSI) > timer_setup(&dpu_enc->vsync_event_timer, > dpu_encoder_vsync_event_handler, > 0); > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > index ebe3944355bb..3891bcbbe5a4 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > @@ -36,7 +36,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder *encoder, > > /** > * struct msm_display_info - defines display properties > - * @intf_type: DRM_MODE_ENCODER_ type > + * @intf_type: INTF_ 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 > @@ -45,7 +45,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder *encoder, > * used instead of panel TE in cmd mode panels > */ > struct msm_display_info { > - int intf_type; > + enum dpu_intf_type intf_type; > uint32_t capabilities; > uint32_t num_of_h_tiles; > uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY]; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index 5f0dc44119c9..bca4f05a5782 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -564,7 +564,7 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev, > priv->encoders[priv->num_encoders++] = encoder; > > memset(&info, 0, sizeof(info)); > - info.intf_type = encoder->encoder_type; > + info.intf_type = INTF_DSI; > > rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder); > if (rc) { > @@ -630,7 +630,8 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev, > info.num_of_h_tiles = 1; > info.h_tile_instance[0] = i; > info.capabilities = MSM_DISPLAY_CAP_VID_MODE; > - info.intf_type = encoder->encoder_type; > + /* FIXME: HW catalog treats both DP and eDP interfaces as INTF_DP */ I thought we discussed an approach to address this FIXME. https://patchwork.freedesktop.org/patch/473871/?series=99651&rev=2 Without addressing that, there is really no need for this change because the only confusion between DRM_MODE_ENCODER_*** mapping to intf_type mapping comes for eDP and DP as they use the same DRM_MODE_ENCODER_TMDS. > + info.intf_type = INTF_DP; > rc = dpu_encoder_setup(dev, encoder, &info); > if (rc) { > DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
On 15/02/2022 20:51, Abhinav Kumar wrote: > > > On 2/15/2022 6:16 AM, Dmitry Baryshkov wrote: >> Rather than passing DRM_MODE_ENCODER_* and letting dpu_encoder to guess, >> which intf type we mean, pass INTF_DSI/INTF_DP directly. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 26 +++++++-------------- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 ++-- >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 ++-- >> 3 files changed, 13 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> index fa1dc088a672..597d40f78d38 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> @@ -490,7 +490,7 @@ void dpu_encoder_helper_split_config( >> hw_mdptop = phys_enc->hw_mdptop; >> disp_info = &dpu_enc->disp_info; >> - if (disp_info->intf_type != DRM_MODE_ENCODER_DSI) >> + if (disp_info->intf_type != INTF_DSI) >> return; >> /** >> @@ -552,7 +552,7 @@ static struct msm_display_topology >> dpu_encoder_get_topology( >> else >> topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 >> : 1; >> - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) { >> + if (dpu_enc->disp_info.intf_type == INTF_DSI) { >> if (dpu_kms->catalog->dspp && >> (dpu_kms->catalog->dspp_count >= topology.num_lm)) >> topology.num_dspp = topology.num_lm; >> @@ -1074,7 +1074,7 @@ static void >> _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc) >> } >> - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_TMDS && >> + if (dpu_enc->disp_info.intf_type == INTF_DP && >> dpu_enc->cur_master->hw_mdptop && >> dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select) >> dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select( >> @@ -1082,7 +1082,7 @@ static void >> _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc) >> _dpu_encoder_update_vsync_source(dpu_enc, &dpu_enc->disp_info); >> - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI && >> + if (dpu_enc->disp_info.intf_type == INTF_DSI && >> !WARN_ON(dpu_enc->num_phys_encs == 0)) { >> unsigned bpc = dpu_enc->connector->display_info.bpc; >> for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) { >> @@ -1949,7 +1949,6 @@ static int dpu_encoder_setup_display(struct >> dpu_encoder_virt *dpu_enc, >> { >> int ret = 0; >> int i = 0; >> - enum dpu_intf_type intf_type = INTF_NONE; >> struct dpu_enc_phys_init_params phys_params; >> if (!dpu_enc) { >> @@ -1965,15 +1964,6 @@ static int dpu_encoder_setup_display(struct >> dpu_encoder_virt *dpu_enc, >> phys_params.parent_ops = &dpu_encoder_parent_ops; >> phys_params.enc_spinlock = &dpu_enc->enc_spinlock; >> - switch (disp_info->intf_type) { >> - case DRM_MODE_ENCODER_DSI: >> - intf_type = INTF_DSI; >> - break; >> - case DRM_MODE_ENCODER_TMDS: >> - intf_type = INTF_DP; >> - break; >> - } >> - >> WARN_ON(disp_info->num_of_h_tiles < 1); >> DPU_DEBUG("dsi_info->num_of_h_tiles %d\n", >> disp_info->num_of_h_tiles); >> @@ -2005,11 +1995,11 @@ static int dpu_encoder_setup_display(struct >> dpu_encoder_virt *dpu_enc, >> i, controller_id, phys_params.split_role); >> phys_params.intf_idx = dpu_encoder_get_intf(dpu_kms->catalog, >> - intf_type, >> - controller_id); >> + disp_info->intf_type, >> + controller_id); >> if (phys_params.intf_idx == INTF_MAX) { >> DPU_ERROR_ENC(dpu_enc, "could not get intf: type %d, id >> %d\n", >> - intf_type, controller_id); >> + disp_info->intf_type, controller_id); >> ret = -EINVAL; >> } >> @@ -2092,7 +2082,7 @@ int dpu_encoder_setup(struct drm_device *dev, >> struct drm_encoder *enc, >> timer_setup(&dpu_enc->frame_done_timer, >> dpu_encoder_frame_done_timeout, 0); >> - if (disp_info->intf_type == DRM_MODE_ENCODER_DSI) >> + if (disp_info->intf_type == INTF_DSI) >> timer_setup(&dpu_enc->vsync_event_timer, >> dpu_encoder_vsync_event_handler, >> 0); >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> index ebe3944355bb..3891bcbbe5a4 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> @@ -36,7 +36,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder >> *encoder, >> /** >> * struct msm_display_info - defines display properties >> - * @intf_type: DRM_MODE_ENCODER_ type >> + * @intf_type: INTF_ 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 >> @@ -45,7 +45,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder >> *encoder, >> * used instead of panel TE in cmd mode panels >> */ >> struct msm_display_info { >> - int intf_type; >> + enum dpu_intf_type intf_type; >> uint32_t capabilities; >> uint32_t num_of_h_tiles; >> uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY]; >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> index 5f0dc44119c9..bca4f05a5782 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c >> @@ -564,7 +564,7 @@ static int _dpu_kms_initialize_dsi(struct >> drm_device *dev, >> priv->encoders[priv->num_encoders++] = encoder; >> memset(&info, 0, sizeof(info)); >> - info.intf_type = encoder->encoder_type; >> + info.intf_type = INTF_DSI; >> rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder); >> if (rc) { >> @@ -630,7 +630,8 @@ static int _dpu_kms_initialize_displayport(struct >> drm_device *dev, >> info.num_of_h_tiles = 1; >> info.h_tile_instance[0] = i; >> info.capabilities = MSM_DISPLAY_CAP_VID_MODE; >> - info.intf_type = encoder->encoder_type; >> + /* FIXME: HW catalog treats both DP and eDP interfaces as >> INTF_DP */ > > I thought we discussed an approach to address this FIXME. > https://patchwork.freedesktop.org/patch/473871/?series=99651&rev=2 > > Without addressing that, there is really no need for this change because > the only confusion between DRM_MODE_ENCODER_*** mapping to intf_type > mapping comes for eDP and DP as they use the same DRM_MODE_ENCODER_TMDS. It's not that distinction which triggered me to sketch this patch. It was more about the code. We loop over all INTF_DSI (or INTF_DP) interfaces. Then we deduce that for this intf we should use DRM_MODE_ENCODER_foo. We pass it to dpu_encoder_setup_display() only to map it back to INTF_DSI (DP). This makes completely no sense to me. > >> + info.intf_type = INTF_DP; >> rc = dpu_encoder_setup(dev, encoder, &info); >> if (rc) { >> DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index fa1dc088a672..597d40f78d38 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -490,7 +490,7 @@ void dpu_encoder_helper_split_config( hw_mdptop = phys_enc->hw_mdptop; disp_info = &dpu_enc->disp_info; - if (disp_info->intf_type != DRM_MODE_ENCODER_DSI) + if (disp_info->intf_type != INTF_DSI) return; /** @@ -552,7 +552,7 @@ static struct msm_display_topology dpu_encoder_get_topology( else topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1; - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) { + if (dpu_enc->disp_info.intf_type == INTF_DSI) { if (dpu_kms->catalog->dspp && (dpu_kms->catalog->dspp_count >= topology.num_lm)) topology.num_dspp = topology.num_lm; @@ -1074,7 +1074,7 @@ static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc) } - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_TMDS && + if (dpu_enc->disp_info.intf_type == INTF_DP && dpu_enc->cur_master->hw_mdptop && dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select) dpu_enc->cur_master->hw_mdptop->ops.intf_audio_select( @@ -1082,7 +1082,7 @@ static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc) _dpu_encoder_update_vsync_source(dpu_enc, &dpu_enc->disp_info); - if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI && + if (dpu_enc->disp_info.intf_type == INTF_DSI && !WARN_ON(dpu_enc->num_phys_encs == 0)) { unsigned bpc = dpu_enc->connector->display_info.bpc; for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) { @@ -1949,7 +1949,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, { int ret = 0; int i = 0; - enum dpu_intf_type intf_type = INTF_NONE; struct dpu_enc_phys_init_params phys_params; if (!dpu_enc) { @@ -1965,15 +1964,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, phys_params.parent_ops = &dpu_encoder_parent_ops; phys_params.enc_spinlock = &dpu_enc->enc_spinlock; - switch (disp_info->intf_type) { - case DRM_MODE_ENCODER_DSI: - intf_type = INTF_DSI; - break; - case DRM_MODE_ENCODER_TMDS: - intf_type = INTF_DP; - break; - } - WARN_ON(disp_info->num_of_h_tiles < 1); DPU_DEBUG("dsi_info->num_of_h_tiles %d\n", disp_info->num_of_h_tiles); @@ -2005,11 +1995,11 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc, i, controller_id, phys_params.split_role); phys_params.intf_idx = dpu_encoder_get_intf(dpu_kms->catalog, - intf_type, - controller_id); + disp_info->intf_type, + controller_id); if (phys_params.intf_idx == INTF_MAX) { DPU_ERROR_ENC(dpu_enc, "could not get intf: type %d, id %d\n", - intf_type, controller_id); + disp_info->intf_type, controller_id); ret = -EINVAL; } @@ -2092,7 +2082,7 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc, timer_setup(&dpu_enc->frame_done_timer, dpu_encoder_frame_done_timeout, 0); - if (disp_info->intf_type == DRM_MODE_ENCODER_DSI) + if (disp_info->intf_type == INTF_DSI) timer_setup(&dpu_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, 0); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h index ebe3944355bb..3891bcbbe5a4 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h @@ -36,7 +36,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder *encoder, /** * struct msm_display_info - defines display properties - * @intf_type: DRM_MODE_ENCODER_ type + * @intf_type: INTF_ 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 @@ -45,7 +45,7 @@ void dpu_encoder_get_hw_resources(struct drm_encoder *encoder, * used instead of panel TE in cmd mode panels */ struct msm_display_info { - int intf_type; + enum dpu_intf_type intf_type; uint32_t capabilities; uint32_t num_of_h_tiles; uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY]; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 5f0dc44119c9..bca4f05a5782 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -564,7 +564,7 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev, priv->encoders[priv->num_encoders++] = encoder; memset(&info, 0, sizeof(info)); - info.intf_type = encoder->encoder_type; + info.intf_type = INTF_DSI; rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder); if (rc) { @@ -630,7 +630,8 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev, info.num_of_h_tiles = 1; info.h_tile_instance[0] = i; info.capabilities = MSM_DISPLAY_CAP_VID_MODE; - info.intf_type = encoder->encoder_type; + /* FIXME: HW catalog treats both DP and eDP interfaces as INTF_DP */ + info.intf_type = INTF_DP; rc = dpu_encoder_setup(dev, encoder, &info); if (rc) { DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
Rather than passing DRM_MODE_ENCODER_* and letting dpu_encoder to guess, which intf type we mean, pass INTF_DSI/INTF_DP directly. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 26 +++++++-------------- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 ++-- 3 files changed, 13 insertions(+), 22 deletions(-)