diff mbox series

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

Message ID 20220215141643.3444941-9-dmitry.baryshkov@linaro.org
State New
Headers show
Series drm/msm/dpu: cleanup dpu encoder code | expand

Commit Message

Dmitry Baryshkov Feb. 15, 2022, 2:16 p.m. UTC
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(-)

Comments

Dmitry Baryshkov Feb. 17, 2022, 1:18 a.m. UTC | #1
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 mbox series

Patch

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",