[PATCH 0/2] drm/msm/dpu: enable INTF_HDMI support

[PATCH 0/2] drm/msm/dpu: enable INTF_HDMI support

[Dmitry Baryshkov]

Since msm8998 and older platforms (wip) support INTF_HDMI, extend DPU
driver to support HDMI output. First patch was submitted about a year
ago, but received some pushback ([1], [2]). However it becomes
absolutely required to properly support INTF_HDMI as both HDMI and DP
should use DRM_MODE_ENCODER_TMDS for encoder_type. Thus dpu_encoder can
not make a difference between HDMI and DP outputs (however they require
different handling).

[1] https://patchwork.freedesktop.org/patch/473871/
[2] https://patchwork.freedesktop.org/patch/474271/

Dmitry Baryshkov (2):
  drm/msm/dpu: simplify intf allocation code
  drm/msm/dpu: add HDMI output support

 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 39 ++++++----------
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  4 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     | 51 +++++++++++++++++++--
 3 files changed, 63 insertions(+), 31 deletions(-)

-- 
2.30.2

[PATCH 1/2] drm/msm/dpu: simplify intf allocation code

[Dmitry Baryshkov]

Rather than passing DRM_MODE_ENCODER_* and letting dpu_encoder to guess,
which intf type we mean, pass INTF_DSI/INTF_DP directly. This is
required to support HDMI output in DPU, as both DP and HDMI encoders are
DRM_MODE_ENCODER_TMDS. Thus dpu_encoder code can not make a difference
between HDMI and DP outputs.

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 39 +++++++--------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  4 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     |  6 ++--
 3 files changed, 18 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1dc5dbe58572..b34416cbd0f5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -495,7 +495,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;
 
 	/**
@@ -1127,7 +1127,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(
@@ -1135,7 +1135,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++) {
@@ -1977,7 +1977,7 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc)
 			phys->ops.handle_post_kickoff(phys);
 	}
 
-	if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI &&
+	if (dpu_enc->disp_info.intf_type == INTF_DSI &&
 			!dpu_encoder_vsync_time(drm_enc, &wakeup_time)) {
 		trace_dpu_enc_early_kickoff(DRMID(drm_enc),
 					    ktime_to_ms(wakeup_time));
@@ -2182,7 +2182,7 @@ static int dpu_encoder_virt_add_phys_encs(
 	}
 
 
-	if (disp_info->intf_type == DRM_MODE_ENCODER_VIRTUAL) {
+	if (disp_info->intf_type == INTF_WB) {
 		enc = dpu_encoder_phys_wb_init(params);
 
 		if (IS_ERR(enc)) {
@@ -2231,7 +2231,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) {
@@ -2246,23 +2245,11 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
 	phys_params.parent = &dpu_enc->base;
 	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;
-	case DRM_MODE_ENCODER_VIRTUAL:
-		intf_type = INTF_WB;
-		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);
 
-	if (disp_info->intf_type != DRM_MODE_ENCODER_VIRTUAL)
+	if (disp_info->intf_type != INTF_WB)
 		dpu_enc->idle_pc_supported =
 				dpu_kms->catalog->caps->has_idle_pc;
 
@@ -2290,11 +2277,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);
 
 		phys_params.wb_idx = dpu_encoder_get_wb(dpu_kms->catalog,
-				intf_type, controller_id);
+				disp_info->intf_type, controller_id);
 		/*
 		 * The phys_params might represent either an INTF or a WB unit, but not
 		 * both of them at the same time.
@@ -2302,14 +2289,14 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
 		if ((phys_params.intf_idx == INTF_MAX) &&
 				(phys_params.wb_idx == WB_MAX)) {
 			DPU_ERROR_ENC(dpu_enc, "could not get intf or wb: type %d, id %d\n",
-						  intf_type, controller_id);
+						  disp_info->intf_type, controller_id);
 			ret = -EINVAL;
 		}
 
 		if ((phys_params.intf_idx != INTF_MAX) &&
 				(phys_params.wb_idx != WB_MAX)) {
 			DPU_ERROR_ENC(dpu_enc, "both intf and wb present: type %d, id %d\n",
-						  intf_type, controller_id);
+						  disp_info->intf_type, controller_id);
 			ret = -EINVAL;
 		}
 
@@ -2409,11 +2396,11 @@ 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);
-	else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS)
+	else if (disp_info->intf_type == INTF_DP)
 		dpu_enc->wide_bus_en = msm_dp_wide_bus_available(
 				priv->dp[disp_info->h_tile_instance[0]]);
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 2c9ef8d1b877..6d14f84dd43f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -21,7 +21,7 @@
 
 /**
  * struct msm_display_info - defines display properties
- * @intf_type:          DRM_MODE_ENCODER_ type
+ * @intf_type:          INTF_ type
  * @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
  *                      based on num_of_h_tiles
@@ -31,7 +31,7 @@
  * @dsc:		DSC configuration data for DSC-enabled displays
  */
 struct msm_display_info {
-	int intf_type;
+	enum dpu_intf_type intf_type;
 	uint32_t num_of_h_tiles;
 	uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY];
 	bool is_cmd_mode;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 0e7a68714e9e..e85e3721d2c7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -542,7 +542,7 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
 		}
 
 		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) {
@@ -605,7 +605,7 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev,
 
 		info.num_of_h_tiles = 1;
 		info.h_tile_instance[0] = i;
-		info.intf_type = encoder->encoder_type;
+		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",
@@ -644,7 +644,7 @@ static int _dpu_kms_initialize_writeback(struct drm_device *dev,
 	info.num_of_h_tiles = 1;
 	/* use only WB idx 2 instance for DPU */
 	info.h_tile_instance[0] = WB_2;
-	info.intf_type = encoder->encoder_type;
+	info.intf_type = INTF_WB;
 
 	rc = dpu_encoder_setup(dev, encoder, &info);
 	if (rc) {
-- 
2.30.2

Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: simplify intf allocation code

[Arnaud Vrac]

On Apr 15 20:19, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:

>Rather than passing DRM_MODE_ENCODER_* and letting dpu_encoder to guess,
>which intf type we mean, pass INTF_DSI/INTF_DP directly. This is
>required to support HDMI output in DPU, as both DP and HDMI encoders are
>DRM_MODE_ENCODER_TMDS. Thus dpu_encoder code can not make a difference
>between HDMI and DP outputs.
>
>Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Arnaud Vrac <rawoul@gmail.com>
Tested-by: Arnaud Vrac <rawoul@gmail.com>

>---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 39 +++++++--------------
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  4 +--
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     |  6 ++--
> 3 files changed, 18 insertions(+), 31 deletions(-)
>
>diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>index 1dc5dbe58572..b34416cbd0f5 100644
>--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>@@ -495,7 +495,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;
>
> 	/**
>@@ -1127,7 +1127,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(
>@@ -1135,7 +1135,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++) {
>@@ -1977,7 +1977,7 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc)
> 			phys->ops.handle_post_kickoff(phys);
> 	}
>
>-	if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI &&
>+	if (dpu_enc->disp_info.intf_type == INTF_DSI &&
> 			!dpu_encoder_vsync_time(drm_enc, &wakeup_time)) {
> 		trace_dpu_enc_early_kickoff(DRMID(drm_enc),
> 					    ktime_to_ms(wakeup_time));
>@@ -2182,7 +2182,7 @@ static int dpu_encoder_virt_add_phys_encs(
> 	}
>
>
>-	if (disp_info->intf_type == DRM_MODE_ENCODER_VIRTUAL) {
>+	if (disp_info->intf_type == INTF_WB) {
> 		enc = dpu_encoder_phys_wb_init(params);
>
> 		if (IS_ERR(enc)) {
>@@ -2231,7 +2231,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) {
>@@ -2246,23 +2245,11 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
> 	phys_params.parent = &dpu_enc->base;
> 	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;
>-	case DRM_MODE_ENCODER_VIRTUAL:
>-		intf_type = INTF_WB;
>-		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);
>
>-	if (disp_info->intf_type != DRM_MODE_ENCODER_VIRTUAL)
>+	if (disp_info->intf_type != INTF_WB)
> 		dpu_enc->idle_pc_supported =
> 				dpu_kms->catalog->caps->has_idle_pc;
>
>@@ -2290,11 +2277,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);
>
> 		phys_params.wb_idx = dpu_encoder_get_wb(dpu_kms->catalog,
>-				intf_type, controller_id);
>+				disp_info->intf_type, controller_id);
> 		/*
> 		 * The phys_params might represent either an INTF or a WB unit, but not
> 		 * both of them at the same time.
>@@ -2302,14 +2289,14 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
> 		if ((phys_params.intf_idx == INTF_MAX) &&
> 				(phys_params.wb_idx == WB_MAX)) {
> 			DPU_ERROR_ENC(dpu_enc, "could not get intf or wb: type %d, id %d\n",
>-						  intf_type, controller_id);
>+						  disp_info->intf_type, controller_id);
> 			ret = -EINVAL;
> 		}
>
> 		if ((phys_params.intf_idx != INTF_MAX) &&
> 				(phys_params.wb_idx != WB_MAX)) {
> 			DPU_ERROR_ENC(dpu_enc, "both intf and wb present: type %d, id %d\n",
>-						  intf_type, controller_id);
>+						  disp_info->intf_type, controller_id);
> 			ret = -EINVAL;
> 		}
>
>@@ -2409,11 +2396,11 @@ 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);
>-	else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS)
>+	else if (disp_info->intf_type == INTF_DP)
> 		dpu_enc->wide_bus_en = msm_dp_wide_bus_available(
> 				priv->dp[disp_info->h_tile_instance[0]]);
>
>diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
>index 2c9ef8d1b877..6d14f84dd43f 100644
>--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
>+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
>@@ -21,7 +21,7 @@
>
> /**
>  * struct msm_display_info - defines display properties
>- * @intf_type:          DRM_MODE_ENCODER_ type
>+ * @intf_type:          INTF_ type
>  * @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
>  *                      based on num_of_h_tiles
>@@ -31,7 +31,7 @@
>  * @dsc:		DSC configuration data for DSC-enabled displays
>  */
> struct msm_display_info {
>-	int intf_type;
>+	enum dpu_intf_type intf_type;
> 	uint32_t num_of_h_tiles;
> 	uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY];
> 	bool is_cmd_mode;
>diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>index 0e7a68714e9e..e85e3721d2c7 100644
>--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>@@ -542,7 +542,7 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
> 		}
>
> 		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) {
>@@ -605,7 +605,7 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev,
>
> 		info.num_of_h_tiles = 1;
> 		info.h_tile_instance[0] = i;
>-		info.intf_type = encoder->encoder_type;
>+		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",
>@@ -644,7 +644,7 @@ static int _dpu_kms_initialize_writeback(struct drm_device *dev,
> 	info.num_of_h_tiles = 1;
> 	/* use only WB idx 2 instance for DPU */
> 	info.h_tile_instance[0] = WB_2;
>-	info.intf_type = encoder->encoder_type;
>+	info.intf_type = INTF_WB;
>
> 	rc = dpu_encoder_setup(dev, encoder, &info);
> 	if (rc) {
>-- 
>2.30.2
>

Re: [PATCH 1/2] drm/msm/dpu: simplify intf allocation code

[Abhinav Kumar]

On 4/15/2023 10:19 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. This is
> required to support HDMI output in DPU, as both DP and HDMI encoders are
> DRM_MODE_ENCODER_TMDS. Thus dpu_encoder code can not make a difference
> between HDMI and DP outputs.
> 
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Since it was previously agreed that INTF_eDP will be dropped in favor of 
using just INTF_DP for both eDP and DP (the previous cause of debate), I 
am fine with this.

Hence,

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

[PATCH 2/2] drm/msm/dpu: add HDMI output support

[Dmitry Baryshkov]

MSM8998 and the older Qualcomm platforms support HDMI outputs. Now as
DPU encoder is ready, add support for using INTF_HDMI.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 45 +++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index e85e3721d2c7..65cce59163a4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -617,6 +617,45 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev,
 	return 0;
 }
 
+static int _dpu_kms_initialize_hdmi(struct drm_device *dev,
+				    struct msm_drm_private *priv,
+				    struct dpu_kms *dpu_kms)
+{
+	struct drm_encoder *encoder = NULL;
+	struct msm_display_info info;
+	int rc;
+	int i;
+
+	if (!priv->hdmi)
+		return 0;
+
+	encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
+	if (IS_ERR(encoder)) {
+		DPU_ERROR("encoder init failed for HDMI display\n");
+		return PTR_ERR(encoder);
+	}
+
+	memset(&info, 0, sizeof(info));
+	rc = msm_hdmi_modeset_init(priv->hdmi, dev, encoder);
+	if (rc) {
+		DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
+		drm_encoder_cleanup(encoder);
+		return rc;
+	}
+
+	info.num_of_h_tiles = 1;
+	info.h_tile_instance[0] = i;
+	info.intf_type = INTF_HDMI;
+	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;
+	}
+
+	return 0;
+}
+
 static int _dpu_kms_initialize_writeback(struct drm_device *dev,
 		struct msm_drm_private *priv, struct dpu_kms *dpu_kms,
 		const u32 *wb_formats, int n_formats)
@@ -683,6 +722,12 @@ static int _dpu_kms_setup_displays(struct drm_device *dev,
 		return rc;
 	}
 
+	rc = _dpu_kms_initialize_hdmi(dev, priv, dpu_kms);
+	if (rc) {
+		DPU_ERROR("initialize HDMI failed, rc = %d\n", rc);
+		return rc;
+	}
+
 	/* Since WB isn't a driver check the catalog before initializing */
 	if (dpu_kms->catalog->wb_count) {
 		for (i = 0; i < dpu_kms->catalog->wb_count; i++) {
-- 
2.30.2

Re: [Freedreno] [PATCH 2/2] drm/msm/dpu: add HDMI output support

[Arnaud Vrac]

On Apr 15 20:19, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:

>MSM8998 and the older Qualcomm platforms support HDMI outputs. Now as
>DPU encoder is ready, add support for using INTF_HDMI.
>
>Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 45 +++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
>diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>index e85e3721d2c7..65cce59163a4 100644
>--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>@@ -617,6 +617,45 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev,
> 	return 0;
> }
>
>+static int _dpu_kms_initialize_hdmi(struct drm_device *dev,
>+				    struct msm_drm_private *priv,
>+				    struct dpu_kms *dpu_kms)
>+{
>+	struct drm_encoder *encoder = NULL;
>+	struct msm_display_info info;
>+	int rc;
>+	int i;
>+
>+	if (!priv->hdmi)
>+		return 0;
>+
>+	encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
>+	if (IS_ERR(encoder)) {
>+		DPU_ERROR("encoder init failed for HDMI display\n");
>+		return PTR_ERR(encoder);
>+	}
>+
>+	memset(&info, 0, sizeof(info));

Move this where fields are initialized ?

>+	rc = msm_hdmi_modeset_init(priv->hdmi, dev, encoder);
>+	if (rc) {
>+		DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
>+		drm_encoder_cleanup(encoder);
>+		return rc;
>+	}
>+
>+	info.num_of_h_tiles = 1;
>+	info.h_tile_instance[0] = i;

i is uninitialized here, the line can be removed.

With the above changes:

Reviewed-by: Arnaud Vrac <rawoul@gmail.com>
Tested-by: Arnaud Vrac <rawoul@gmail.com> # on msm8998

-Arnaud

>+	info.intf_type = INTF_HDMI;
>+	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;
>+	}
>+
>+	return 0;
>+}
>+
> static int _dpu_kms_initialize_writeback(struct drm_device *dev,
> 		struct msm_drm_private *priv, struct dpu_kms *dpu_kms,
> 		const u32 *wb_formats, int n_formats)
>@@ -683,6 +722,12 @@ static int _dpu_kms_setup_displays(struct drm_device *dev,
> 		return rc;
> 	}
>
>+	rc = _dpu_kms_initialize_hdmi(dev, priv, dpu_kms);
>+	if (rc) {
>+		DPU_ERROR("initialize HDMI failed, rc = %d\n", rc);
>+		return rc;
>+	}
>+
> 	/* Since WB isn't a driver check the catalog before initializing */
> 	if (dpu_kms->catalog->wb_count) {
> 		for (i = 0; i < dpu_kms->catalog->wb_count; i++) {
>-- 
>2.30.2
>

Re: [PATCH 2/2] drm/msm/dpu: add HDMI output support

[Abhinav Kumar]

On 4/15/2023 10:19 AM, Dmitry Baryshkov wrote:
> MSM8998 and the older Qualcomm platforms support HDMI outputs. Now as
> DPU encoder is ready, add support for using INTF_HDMI.
> 

 From what I see, encoder was always ready but just HDMI case was not 
handled? Or are you saying this because of the prev patch which sorts 
out encoder_type and intf_type

> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 45 +++++++++++++++++++++++++
>   1 file changed, 45 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index e85e3721d2c7..65cce59163a4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -617,6 +617,45 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev,
>   	return 0;
>   }
>   
> +static int _dpu_kms_initialize_hdmi(struct drm_device *dev,
> +				    struct msm_drm_private *priv,
> +				    struct dpu_kms *dpu_kms)
> +{
> +	struct drm_encoder *encoder = NULL;
> +	struct msm_display_info info;
> +	int rc;
> +	int i;
> +
> +	if (!priv->hdmi)
> +		return 0;
> +
> +	encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
> +	if (IS_ERR(encoder)) {
> +		DPU_ERROR("encoder init failed for HDMI display\n");
> +		return PTR_ERR(encoder);
> +	}
> +
> +	memset(&info, 0, sizeof(info));
> +	rc = msm_hdmi_modeset_init(priv->hdmi, dev, encoder);
> +	if (rc) {
> +		DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
> +		drm_encoder_cleanup(encoder);
> +		return rc;
> +	}
> +
> +	info.num_of_h_tiles = 1;
> +	info.h_tile_instance[0] = i;

As Arnaud noted, i is uninitialized here.

> +	info.intf_type = INTF_HDMI;
> +	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;
> +	}
> +
> +	return 0;
> +}
> +
>   static int _dpu_kms_initialize_writeback(struct drm_device *dev,
>   		struct msm_drm_private *priv, struct dpu_kms *dpu_kms,
>   		const u32 *wb_formats, int n_formats)
> @@ -683,6 +722,12 @@ static int _dpu_kms_setup_displays(struct drm_device *dev,
>   		return rc;
>   	}
>   
> +	rc = _dpu_kms_initialize_hdmi(dev, priv, dpu_kms);
> +	if (rc) {
> +		DPU_ERROR("initialize HDMI failed, rc = %d\n", rc);
> +		return rc;
> +	}
> +
>   	/* Since WB isn't a driver check the catalog before initializing */
>   	if (dpu_kms->catalog->wb_count) {
>   		for (i = 0; i < dpu_kms->catalog->wb_count; i++) {