[PATCH v2 8/8] drm/msm/dpu: simplify intf allocation code

Bjorn Andersson bjorn.andersson at linaro.org
Tue Feb 15 15:42:51 UTC 2022


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 at 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 at 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
> 


More information about the dri-devel mailing list