[PATCH 0/3] Fix packed pixel modes display on MHL2

[PATCH 0/3] Fix packed pixel modes display on MHL2

[Maciej Purski]

Hi all,

this patchset is a next attempt to fix packed pixel modes display on MHL2. It appeared, that
the current implementation does not work well with every dongle. Thanks to bigger number
of test devices I managed to prepare a patchset, which should finally fix display
of packed pixel modes on MHL2, while not breaking it on MHL3.

Best regards,

Maciej Purski

Maciej Purski (3):
  drm/bridge/sii8620: Send AVI infoframe in all MHL versions
  drm/bridge/sii8620: Fix display of packed pixel modes
  drm/bridge/sii8620: Fix link mode selection

 drivers/gpu/drm/bridge/sil-sii8620.c | 86 +++++++++++++++++++++++-------------
 1 file changed, 55 insertions(+), 31 deletions(-)

-- 
2.7.4

[PATCH 1/3] drm/bridge/sii8620: Send AVI infoframe in all MHL versions

[Maciej Purski]

Currently AVI infoframe is sent only in MHL3. However, some MHL2 dongles
need AVI infoframe to work correctly in either packed pixel mode or
non-packed pixel mode.

Send AVI infoframe in set_infoframes() in every case. Create an
infoframe using drm_hdmi_infoframe_from_display_mode() instead of
manually filling each infoframe structure's field.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/bridge/sil-sii8620.c | 39 ++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index 250effa..bd30ccf 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -14,6 +14,7 @@
 #include <drm/bridge/mhl.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_edid.h>
+#include <drm/drm_encoder.h>
 
 #include <linux/clk.h>
 #include <linux/delay.h>
@@ -72,9 +73,7 @@ struct sii8620 {
 	struct regulator_bulk_data supplies[2];
 	struct mutex lock; /* context lock, protects fields below */
 	int error;
-	int pixel_clock;
 	unsigned int use_packed_pixel:1;
-	int video_code;
 	enum sii8620_mode mode;
 	enum sii8620_sink_type sink_type;
 	u8 cbus_status;
@@ -82,7 +81,6 @@ struct sii8620 {
 	u8 xstat[MHL_XDS_SIZE];
 	u8 devcap[MHL_DCAP_SIZE];
 	u8 xdevcap[MHL_XDC_SIZE];
-	u8 avif[HDMI_INFOFRAME_SIZE(AVI)];
 	bool feature_complete;
 	bool devcap_read;
 	bool sink_detected;
@@ -1082,18 +1080,28 @@ static ssize_t mhl3_infoframe_pack(struct mhl3_infoframe *frame,
 	return frm_len;
 }
 
-static void sii8620_set_infoframes(struct sii8620 *ctx)
+static void sii8620_set_infoframes(struct sii8620 *ctx,
+				   struct drm_display_mode *mode)
 {
 	struct mhl3_infoframe mhl_frm;
 	union hdmi_infoframe frm;
 	u8 buf[31];
 	int ret;
 
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
+						       mode,
+						       true);
+	if (ctx->use_packed_pixel)
+		frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
+
+	if (!ret)
+		ret = hdmi_avi_infoframe_pack(&frm.avi, buf, ARRAY_SIZE(buf));
+	if (ret > 0)
+		sii8620_write_buf(ctx, REG_TPI_AVI_CHSUM, buf + 3, ret - 3);
+
 	if (!sii8620_is_mhl3(ctx) || !ctx->use_packed_pixel) {
 		sii8620_write(ctx, REG_TPI_SC,
 			BIT_TPI_SC_TPI_OUTPUT_MODE_0_HDMI);
-		sii8620_write_buf(ctx, REG_TPI_AVI_CHSUM, ctx->avif + 3,
-			ARRAY_SIZE(ctx->avif) - 3);
 		sii8620_write(ctx, REG_PKT_FILTER_0,
 			BIT_PKT_FILTER_0_DROP_CEA_GAMUT_PKT |
 			BIT_PKT_FILTER_0_DROP_MPEG_PKT |
@@ -1102,16 +1110,6 @@ static void sii8620_set_infoframes(struct sii8620 *ctx)
 		return;
 	}
 
-	ret = hdmi_avi_infoframe_init(&frm.avi);
-	frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
-	frm.avi.active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
-	frm.avi.picture_aspect = HDMI_PICTURE_ASPECT_16_9;
-	frm.avi.colorimetry = HDMI_COLORIMETRY_ITU_709;
-	frm.avi.video_code = ctx->video_code;
-	if (!ret)
-		ret = hdmi_avi_infoframe_pack(&frm.avi, buf, ARRAY_SIZE(buf));
-	if (ret > 0)
-		sii8620_write_buf(ctx, REG_TPI_AVI_CHSUM, buf + 3, ret - 3);
 	sii8620_write(ctx, REG_PKT_FILTER_0,
 		BIT_PKT_FILTER_0_DROP_CEA_GAMUT_PKT |
 		BIT_PKT_FILTER_0_DROP_MPEG_PKT |
@@ -1131,6 +1129,9 @@ static void sii8620_set_infoframes(struct sii8620 *ctx)
 
 static void sii8620_start_video(struct sii8620 *ctx)
 {
+	struct drm_display_mode *mode =
+		&ctx->bridge.encoder->crtc->state->adjusted_mode;
+
 	if (!sii8620_is_mhl3(ctx))
 		sii8620_stop_video(ctx);
 
@@ -1167,7 +1168,7 @@ static void sii8620_start_video(struct sii8620 *ctx)
 			  MHL_XDS_LINK_RATE_6_0_GBPS, 0x40 },
 		};
 		u8 p0_ctrl = BIT_M3_P0CTRL_MHL3_P0_PORT_EN;
-		int clk = ctx->pixel_clock * (ctx->use_packed_pixel ? 2 : 3);
+		int clk = mode->clock * (ctx->use_packed_pixel ? 2 : 3);
 		int i;
 
 		for (i = 0; i < ARRAY_SIZE(clk_spec) - 1; ++i)
@@ -1196,7 +1197,7 @@ static void sii8620_start_video(struct sii8620 *ctx)
 			clk_spec[i].link_rate);
 	}
 
-	sii8620_set_infoframes(ctx);
+	sii8620_set_infoframes(ctx, mode);
 }
 
 static void sii8620_disable_hpd(struct sii8620 *ctx)
@@ -2242,8 +2243,6 @@ static bool sii8620_mode_fixup(struct drm_bridge *bridge,
 	mutex_lock(&ctx->lock);
 
 	ctx->use_packed_pixel = sii8620_is_packing_required(ctx, adjusted_mode);
-	ctx->video_code = drm_match_cea_mode(adjusted_mode);
-	ctx->pixel_clock = adjusted_mode->clock;
 
 	mutex_unlock(&ctx->lock);
 
-- 
2.7.4

[PATCH 2/3] drm/bridge/sii8620: Fix display of packed pixel modes

[Maciej Purski]

Current implementation does not guarantee packed pixel modes working
with every dongle. There are some dongles, which require selecting
the output mode explicitly.

Write proper values to registers in packed_pixel mode, based on how it
is done in vendor's code. Select output color space: RGB
(no packed pixel) or YCBCR422 (packed pixel).

This reverts commit e8b92efa629dac0e70ea4145c5e70616de5f89c8
("drm/bridge/sii8620: fix display of packed pixel modes in MHL2").

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/bridge/sil-sii8620.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index bd30ccf..16fe7ea 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -1015,21 +1015,36 @@ static void sii8620_stop_video(struct sii8620 *ctx)
 
 static void sii8620_set_format(struct sii8620 *ctx)
 {
+	u8 out_fmt;
+
 	if (sii8620_is_mhl3(ctx)) {
 		sii8620_setbits(ctx, REG_M3_P0CTRL,
 				BIT_M3_P0CTRL_MHL3_P0_PIXEL_MODE_PACKED,
 				ctx->use_packed_pixel ? ~0 : 0);
 	} else {
+		if (ctx->use_packed_pixel) {
+			sii8620_write_seq_static(ctx,
+				REG_VID_MODE, BIT_VID_MODE_M1080P,
+				REG_MHL_TOP_CTL, BIT_MHL_TOP_CTL_MHL_PP_SEL | 1,
+				REG_MHLTX_CTL6, 0x60
+			);
+		} else {
 			sii8620_write_seq_static(ctx,
 				REG_VID_MODE, 0,
 				REG_MHL_TOP_CTL, 1,
 				REG_MHLTX_CTL6, 0xa0
 			);
+		}
 	}
 
+	if (ctx->use_packed_pixel)
+		out_fmt = VAL_TPI_FORMAT(YCBCR422, FULL);
+	else
+		out_fmt = VAL_TPI_FORMAT(RGB, FULL);
+
 	sii8620_write_seq(ctx,
 		REG_TPI_INPUT, VAL_TPI_FORMAT(RGB, FULL),
-		REG_TPI_OUTPUT, VAL_TPI_FORMAT(RGB, FULL),
+		REG_TPI_OUTPUT, out_fmt,
 	);
 }
 
-- 
2.7.4

[PATCH 3/3] drm/bridge/sii8620: Fix link mode selection

[Maciej Purski]

Current link mode values do not allow to enable packed pixel modes.

Select packed pixel clock mode, if needed, every time the link mode
register gets updated.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/bridge/sil-sii8620.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index 16fe7ea..a6e8f45 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -1165,8 +1165,14 @@ static void sii8620_start_video(struct sii8620 *ctx)
 	sii8620_set_format(ctx);
 
 	if (!sii8620_is_mhl3(ctx)) {
-		sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE),
-			MHL_DST_LM_CLK_MODE_NORMAL | MHL_DST_LM_PATH_ENABLED);
+		u8 link_mode = MHL_DST_LM_PATH_ENABLED;
+
+		if (ctx->use_packed_pixel)
+			link_mode |= MHL_DST_LM_CLK_MODE_PACKED_PIXEL;
+		else
+			link_mode |= MHL_DST_LM_CLK_MODE_NORMAL;
+
+		sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE), link_mode);
 		sii8620_set_auto_zone(ctx);
 	} else {
 		static const struct {
@@ -1677,14 +1683,18 @@ static void sii8620_status_dcap_ready(struct sii8620 *ctx)
 
 static void sii8620_status_changed_path(struct sii8620 *ctx)
 {
-	if (ctx->stat[MHL_DST_LINK_MODE] & MHL_DST_LM_PATH_ENABLED) {
-		sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE),
-				      MHL_DST_LM_CLK_MODE_NORMAL
-				      | MHL_DST_LM_PATH_ENABLED);
-	} else {
-		sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE),
-				      MHL_DST_LM_CLK_MODE_NORMAL);
-	}
+	u8 link_mode;
+
+	if (ctx->use_packed_pixel)
+		link_mode = MHL_DST_LM_CLK_MODE_PACKED_PIXEL;
+	else
+		link_mode = MHL_DST_LM_CLK_MODE_NORMAL;
+
+	if (ctx->stat[MHL_DST_LINK_MODE] & MHL_DST_LM_PATH_ENABLED)
+		link_mode |= MHL_DST_LM_PATH_ENABLED;
+
+	sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE),
+			      link_mode);
 }
 
 static void sii8620_msc_mr_write_stat(struct sii8620 *ctx)
-- 
2.7.4

Re: [PATCH 3/3] drm/bridge/sii8620: Fix link mode selection

[Andrzej Hajda]

On 28.06.2018 18:44, Maciej Purski wrote:
> Current link mode values do not allow to enable packed pixel modes.
>
> Select packed pixel clock mode, if needed, every time the link mode
> register gets updated.
>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>

Queued all three patches to drm-misc-fixes.

Regards
Andrzej

> ---
>  drivers/gpu/drm/bridge/sil-sii8620.c | 30 ++++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
> index 16fe7ea..a6e8f45 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -1165,8 +1165,14 @@ static void sii8620_start_video(struct sii8620 *ctx)
>  	sii8620_set_format(ctx);
>  
>  	if (!sii8620_is_mhl3(ctx)) {
> -		sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE),
> -			MHL_DST_LM_CLK_MODE_NORMAL | MHL_DST_LM_PATH_ENABLED);
> +		u8 link_mode = MHL_DST_LM_PATH_ENABLED;
> +
> +		if (ctx->use_packed_pixel)
> +			link_mode |= MHL_DST_LM_CLK_MODE_PACKED_PIXEL;
> +		else
> +			link_mode |= MHL_DST_LM_CLK_MODE_NORMAL;
> +
> +		sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE), link_mode);
>  		sii8620_set_auto_zone(ctx);
>  	} else {
>  		static const struct {
> @@ -1677,14 +1683,18 @@ static void sii8620_status_dcap_ready(struct sii8620 *ctx)
>  
>  static void sii8620_status_changed_path(struct sii8620 *ctx)
>  {
> -	if (ctx->stat[MHL_DST_LINK_MODE] & MHL_DST_LM_PATH_ENABLED) {
> -		sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE),
> -				      MHL_DST_LM_CLK_MODE_NORMAL
> -				      | MHL_DST_LM_PATH_ENABLED);
> -	} else {
> -		sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE),
> -				      MHL_DST_LM_CLK_MODE_NORMAL);
> -	}
> +	u8 link_mode;
> +
> +	if (ctx->use_packed_pixel)
> +		link_mode = MHL_DST_LM_CLK_MODE_PACKED_PIXEL;
> +	else
> +		link_mode = MHL_DST_LM_CLK_MODE_NORMAL;
> +
> +	if (ctx->stat[MHL_DST_LINK_MODE] & MHL_DST_LM_PATH_ENABLED)
> +		link_mode |= MHL_DST_LM_PATH_ENABLED;
> +
> +	sii8620_mt_write_stat(ctx, MHL_DST_REG(LINK_MODE),
> +			      link_mode);
>  }
>  
>  static void sii8620_msc_mr_write_stat(struct sii8620 *ctx)