[PATCH 0/5] sunxi: fix H6 HDMI related issues

[PATCH 0/5] sunxi: fix H6 HDMI related issues

[Jernej Skrabec]

Over the year I got plenty of reports of troubles with H6 HDMI signal.
Sometimes monitor flickers, sometimes there was no image at all and
sometimes it didn't play well with AVR.

It turns out there are multiple issues. Patch 1 fixes clock issue,
which didn't adjust parent rate, even if it is allowed to do so. Patch 2
adds polarity config in tcon1. This is seemingly not needed for pre-HDMI2
controllers, although BSP drivers set it accordingly every time. It
turns out that HDMI2 controllers often don't work with monitors if
polarity is not set correctly. Patch 3 always set clock rate for HDMI
controller. Patch 4 fixes cpce PHY setting for 594 MHz. Patch 5 fixes
comment and clock rate limit (wrong reasoning).

Please take a look.

Best regards,
Jernej

Jernej Skrabec (5):
  clk: sunxi-ng: mp: fix parent rate change flag check
  drm/sun4i: tcon: set sync polarity for tcon1 channel
  drm/sun4i: dw-hdmi: always set clock rate
  drm/sun4i: Fix H6 HDMI PHY configuration
  drm/sun4i: dw-hdmi: Fix max. frequency for H6

 drivers/clk/sunxi-ng/ccu_mp.c          |  2 +-
 drivers/gpu/drm/sun4i/sun4i_tcon.c     | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun4i_tcon.h     |  5 +++++
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c  | 10 +++-------
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h  |  1 -
 drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c |  2 +-
 6 files changed, 34 insertions(+), 10 deletions(-)

-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/5] clk: sunxi-ng: mp: fix parent rate change flag check

[Jernej Skrabec]

CLK_SET_RATE_PARENT flag is checked on parent clock instead of current
one. Fix that.

Fixes: 3f790433c3cb ("clk: sunxi-ng: Adjust MP clock parent rate when allowed")
Tested-by: Andre Heider <a.heider@gmail.com>
Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/clk/sunxi-ng/ccu_mp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c
index fa4ecb915590..5f40be6d2dfd 100644
--- a/drivers/clk/sunxi-ng/ccu_mp.c
+++ b/drivers/clk/sunxi-ng/ccu_mp.c
@@ -108,7 +108,7 @@ static unsigned long ccu_mp_round_rate(struct ccu_mux_internal *mux,
 	max_m = cmp->m.max ?: 1 << cmp->m.width;
 	max_p = cmp->p.max ?: 1 << ((1 << cmp->p.width) - 1);
 
-	if (!(clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT)) {
+	if (!(clk_hw_get_flags(&cmp->common.hw) & CLK_SET_RATE_PARENT)) {
 		ccu_mp_find_best(*parent_rate, rate, max_m, max_p, &m, &p);
 		rate = *parent_rate / p / m;
 	} else {
-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/5] drm/sun4i: tcon: set sync polarity for tcon1 channel

[Jernej Skrabec]

Channel 1 has polarity bits for vsync and hsync signals but driver never
sets them. It turns out that with pre-HDMI2 controllers seemingly there
is no issue if polarity is not set. However, with HDMI2 controllers
(H6) there often comes to de-synchronization due to phase shift. This
causes flickering screen. It's safe to assume that similar issues might
happen also with pre-HDMI2 controllers.

Solve issue with setting vsync and hsync polarity. Note that display
stacks with tcon top have polarity bits actually in tcon0 polarity
register.

Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
Tested-by: Andre Heider <a.heider@gmail.com>
Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun4i_tcon.h |  5 +++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 6b9af4c08cd6..0d132dae58c0 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon,
 		     SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
 		     SUN4I_TCON1_BASIC5_H_SYNC(hsync));
 
+	/* Setup the polarity of sync signals */
+	if (tcon->quirks->polarity_in_ch0) {
+		val = 0;
+
+		if (mode->flags & DRM_MODE_FLAG_PHSYNC)
+			val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
+
+		if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+			val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
+
+		regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, val);
+	} else {
+		val = SUN4I_TCON1_IO_POL_UNKNOWN;
+
+		if (mode->flags & DRM_MODE_FLAG_PHSYNC)
+			val |= SUN4I_TCON1_IO_POL_HSYNC_POSITIVE;
+
+		if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+			val |= SUN4I_TCON1_IO_POL_VSYNC_POSITIVE;
+
+		regmap_write(tcon->regs, SUN4I_TCON1_IO_POL_REG, val);
+	}
+
 	/* Map output pins to channel 1 */
 	regmap_update_bits(tcon->regs, SUN4I_TCON_GCTL_REG,
 			   SUN4I_TCON_GCTL_IOMAP_MASK,
@@ -1500,6 +1523,7 @@ static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = {
 
 static const struct sun4i_tcon_quirks sun8i_r40_tv_quirks = {
 	.has_channel_1		= true,
+	.polarity_in_ch0	= true,
 	.set_mux		= sun8i_r40_tcon_tv_set_mux,
 };
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
index c5ac1b02482c..b504fb2d3de5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
@@ -154,6 +154,10 @@
 #define SUN4I_TCON1_BASIC5_V_SYNC(height)		(((height) - 1) & 0x3ff)
 
 #define SUN4I_TCON1_IO_POL_REG			0xf0
+#define SUN4I_TCON1_IO_POL_UNKNOWN			BIT(26)
+#define SUN4I_TCON1_IO_POL_HSYNC_POSITIVE		BIT(25)
+#define SUN4I_TCON1_IO_POL_VSYNC_POSITIVE		BIT(24)
+
 #define SUN4I_TCON1_IO_TRI_REG			0xf4
 
 #define SUN4I_TCON_ECC_FIFO_REG			0xf8
@@ -236,6 +240,7 @@ struct sun4i_tcon_quirks {
 	bool	needs_de_be_mux; /* sun6i needs mux to select backend */
 	bool    needs_edp_reset; /* a80 edp reset needed for tcon0 access */
 	bool	supports_lvds;   /* Does the TCON support an LVDS output? */
+	bool	polarity_in_ch0; /* some tcon1 channels have polarity bits in tcon0 pol register */
 	u8	dclk_min_div;	/* minimum divider for TCON0 DCLK */
 
 	/* callback to handle tcon muxing options */
-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/5] drm/sun4i: dw-hdmi: always set clock rate

[Jernej Skrabec]

As expected, HDMI controller clock should always match pixel clock. In
the past, changing HDMI controller rate would seemingly worsen
situation. However, that was the result of other bugs which are now
fixed.

Fix that by removing set_rate quirk and always set clock rate.

Fixes: 40bb9d3147b2 ("drm/sun4i: Add support for H6 DW HDMI controller")
Tested-by: Andre Heider <a.heider@gmail.com>
Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 4 +---
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 -
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
index 92add2cef2e7..23773a5e0650 100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
@@ -21,8 +21,7 @@ static void sun8i_dw_hdmi_encoder_mode_set(struct drm_encoder *encoder,
 {
 	struct sun8i_dw_hdmi *hdmi = encoder_to_sun8i_dw_hdmi(encoder);
 
-	if (hdmi->quirks->set_rate)
-		clk_set_rate(hdmi->clk_tmds, mode->crtc_clock * 1000);
+	clk_set_rate(hdmi->clk_tmds, mode->crtc_clock * 1000);
 }
 
 static const struct drm_encoder_helper_funcs
@@ -295,7 +294,6 @@ static int sun8i_dw_hdmi_remove(struct platform_device *pdev)
 
 static const struct sun8i_dw_hdmi_quirks sun8i_a83t_quirks = {
 	.mode_valid = sun8i_dw_hdmi_mode_valid_a83t,
-	.set_rate = true,
 };
 
 static const struct sun8i_dw_hdmi_quirks sun50i_h6_quirks = {
diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
index d983746fa194..d4b55af0592f 100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
@@ -179,7 +179,6 @@ struct sun8i_dw_hdmi_quirks {
 	enum drm_mode_status (*mode_valid)(struct dw_hdmi *hdmi, void *data,
 					   const struct drm_display_info *info,
 					   const struct drm_display_mode *mode);
-	unsigned int set_rate : 1;
 	unsigned int use_drm_infoframe : 1;
 };
 
-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/5] drm/sun4i: Fix H6 HDMI PHY configuration

[Jernej Skrabec]

cpce value for 594 MHz is set differently in BSP driver. Fix that.

Fixes: c71c9b2fee17 ("drm/sun4i: Add support for Synopsys HDMI PHY")
Tested-by: Andre Heider <a.heider@gmail.com>
Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
index 35c2133724e2..89aff19ddeb4 100644
--- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
+++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
@@ -89,7 +89,7 @@ static const struct dw_hdmi_mpll_config sun50i_h6_mpll_cfg[] = {
 		},
 	},  {
 		594000000, {
-			{ 0x1a40, 0x0003 },
+			{ 0x1a7c, 0x0003 },
 			{ 0x3b4c, 0x0003 },
 			{ 0x5a64, 0x0003 },
 		},
-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/5] drm/sun4i: dw-hdmi: Fix max. frequency for H6

[Jernej Skrabec]

It turns out that reasoning for lowering max. supported frequency is
wrong. Scrambling works just fine. Several now fixed bugs prevented
proper functioning, even with rates lower than 340 MHz. Issues were just
more pronounced with higher frequencies.

Fix that by allowing max. supported frequency in HW and fix the comment.

Fixes: cd9063757a22 ("drm/sun4i: DW HDMI: Lower max. supported rate for H6")
Tested-by: Andre Heider <a.heider@gmail.com>
Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
index 23773a5e0650..bbdfd5e26ec8 100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
@@ -47,11 +47,9 @@ sun8i_dw_hdmi_mode_valid_h6(struct dw_hdmi *hdmi, void *data,
 {
 	/*
 	 * Controller support maximum of 594 MHz, which correlates to
-	 * 4K@60Hz 4:4:4 or RGB. However, for frequencies greater than
-	 * 340 MHz scrambling has to be enabled. Because scrambling is
-	 * not yet implemented, just limit to 340 MHz for now.
+	 * 4K@60Hz 4:4:4 or RGB.
 	 */
-	if (mode->clock > 340000)
+	if (mode->clock > 594000)
 		return MODE_CLOCK_HIGH;
 
 	return MODE_OK;
-- 
2.30.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/5] drm/sun4i: tcon: set sync polarity for tcon1 channel

[Chen-Yu Tsai]

On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <jernej.skrabec@siol.net> wrote:
>
> Channel 1 has polarity bits for vsync and hsync signals but driver never
> sets them. It turns out that with pre-HDMI2 controllers seemingly there
> is no issue if polarity is not set. However, with HDMI2 controllers
> (H6) there often comes to de-synchronization due to phase shift. This
> causes flickering screen. It's safe to assume that similar issues might
> happen also with pre-HDMI2 controllers.
>
> Solve issue with setting vsync and hsync polarity. Note that display
> stacks with tcon top have polarity bits actually in tcon0 polarity
> register.
>
> Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
> Tested-by: Andre Heider <a.heider@gmail.com>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
>  drivers/gpu/drm/sun4i/sun4i_tcon.h |  5 +++++
>  2 files changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 6b9af4c08cd6..0d132dae58c0 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon,
>                      SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
>                      SUN4I_TCON1_BASIC5_H_SYNC(hsync));
>
> +       /* Setup the polarity of sync signals */
> +       if (tcon->quirks->polarity_in_ch0) {
> +               val = 0;
> +
> +               if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> +                       val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
> +
> +               if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> +                       val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
> +
> +               regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, val);
> +       } else {
> +               val = SUN4I_TCON1_IO_POL_UNKNOWN;

I think a comment for the origin of this is warranted.

Otherwise,

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

> +
> +               if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> +                       val |= SUN4I_TCON1_IO_POL_HSYNC_POSITIVE;
> +
> +               if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> +                       val |= SUN4I_TCON1_IO_POL_VSYNC_POSITIVE;
> +
> +               regmap_write(tcon->regs, SUN4I_TCON1_IO_POL_REG, val);
> +       }
> +
>         /* Map output pins to channel 1 */
>         regmap_update_bits(tcon->regs, SUN4I_TCON_GCTL_REG,
>                            SUN4I_TCON_GCTL_IOMAP_MASK,
> @@ -1500,6 +1523,7 @@ static const struct sun4i_tcon_quirks sun8i_a83t_tv_quirks = {
>
>  static const struct sun4i_tcon_quirks sun8i_r40_tv_quirks = {
>         .has_channel_1          = true,
> +       .polarity_in_ch0        = true,
>         .set_mux                = sun8i_r40_tcon_tv_set_mux,
>  };
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> index c5ac1b02482c..b504fb2d3de5 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h
> @@ -154,6 +154,10 @@
>  #define SUN4I_TCON1_BASIC5_V_SYNC(height)              (((height) - 1) & 0x3ff)
>
>  #define SUN4I_TCON1_IO_POL_REG                 0xf0
> +#define SUN4I_TCON1_IO_POL_UNKNOWN                     BIT(26)
> +#define SUN4I_TCON1_IO_POL_HSYNC_POSITIVE              BIT(25)
> +#define SUN4I_TCON1_IO_POL_VSYNC_POSITIVE              BIT(24)
> +
>  #define SUN4I_TCON1_IO_TRI_REG                 0xf4
>
>  #define SUN4I_TCON_ECC_FIFO_REG                        0xf8
> @@ -236,6 +240,7 @@ struct sun4i_tcon_quirks {
>         bool    needs_de_be_mux; /* sun6i needs mux to select backend */
>         bool    needs_edp_reset; /* a80 edp reset needed for tcon0 access */
>         bool    supports_lvds;   /* Does the TCON support an LVDS output? */
> +       bool    polarity_in_ch0; /* some tcon1 channels have polarity bits in tcon0 pol register */
>         u8      dclk_min_div;   /* minimum divider for TCON0 DCLK */
>
>         /* callback to handle tcon muxing options */
> --
> 2.30.0
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [linux-sunxi] [PATCH 1/5] clk: sunxi-ng: mp: fix parent rate change flag check

[Chen-Yu Tsai]

On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <jernej.skrabec@siol.net> wrote:
>
> CLK_SET_RATE_PARENT flag is checked on parent clock instead of current
> one. Fix that.
>
> Fixes: 3f790433c3cb ("clk: sunxi-ng: Adjust MP clock parent rate when allowed")
> Tested-by: Andre Heider <a.heider@gmail.com>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] drm/sun4i: dw-hdmi: always set clock rate

[Chen-Yu Tsai]

On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <jernej.skrabec@siol.net> wrote:
>
> As expected, HDMI controller clock should always match pixel clock. In
> the past, changing HDMI controller rate would seemingly worsen
> situation. However, that was the result of other bugs which are now
> fixed.
>
> Fix that by removing set_rate quirk and always set clock rate.
>
> Fixes: 40bb9d3147b2 ("drm/sun4i: Add support for H6 DW HDMI controller")
> Tested-by: Andre Heider <a.heider@gmail.com>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [linux-sunxi] [PATCH 4/5] drm/sun4i: Fix H6 HDMI PHY configuration

[Chen-Yu Tsai]

On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <jernej.skrabec@siol.net> wrote:
>
> cpce value for 594 MHz is set differently in BSP driver. Fix that.
>
> Fixes: c71c9b2fee17 ("drm/sun4i: Add support for Synopsys HDMI PHY")
> Tested-by: Andre Heider <a.heider@gmail.com>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [linux-sunxi] [PATCH 5/5] drm/sun4i: dw-hdmi: Fix max. frequency for H6

[Chen-Yu Tsai]

On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <jernej.skrabec@siol.net> wrote:
>
> It turns out that reasoning for lowering max. supported frequency is
> wrong. Scrambling works just fine. Several now fixed bugs prevented
> proper functioning, even with rates lower than 340 MHz. Issues were just
> more pronounced with higher frequencies.
>
> Fix that by allowing max. supported frequency in HW and fix the comment.
>
> Fixes: cd9063757a22 ("drm/sun4i: DW HDMI: Lower max. supported rate for H6")
> Tested-by: Andre Heider <a.heider@gmail.com>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/5] drm/sun4i: tcon: set sync polarity for tcon1 channel

[Maxime Ripard]

On Fri, Feb 05, 2021 at 11:21:22AM +0800, Chen-Yu Tsai wrote:
> On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <jernej.skrabec@siol.net> wrote:
> >
> > Channel 1 has polarity bits for vsync and hsync signals but driver never
> > sets them. It turns out that with pre-HDMI2 controllers seemingly there
> > is no issue if polarity is not set. However, with HDMI2 controllers
> > (H6) there often comes to de-synchronization due to phase shift. This
> > causes flickering screen. It's safe to assume that similar issues might
> > happen also with pre-HDMI2 controllers.
> >
> > Solve issue with setting vsync and hsync polarity. Note that display
> > stacks with tcon top have polarity bits actually in tcon0 polarity
> > register.
> >
> > Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
> > Tested-by: Andre Heider <a.heider@gmail.com>
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
> >  drivers/gpu/drm/sun4i/sun4i_tcon.h |  5 +++++
> >  2 files changed, 29 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > index 6b9af4c08cd6..0d132dae58c0 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > @@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon,
> >                      SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
> >                      SUN4I_TCON1_BASIC5_H_SYNC(hsync));
> >
> > +       /* Setup the polarity of sync signals */
> > +       if (tcon->quirks->polarity_in_ch0) {
> > +               val = 0;
> > +
> > +               if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > +                       val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
> > +
> > +               if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > +                       val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
> > +
> > +               regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, val);
> > +       } else {
> > +               val = SUN4I_TCON1_IO_POL_UNKNOWN;
> 
> I think a comment for the origin of this is warranted.

If it's anything like TCON0, it's the pixel clock polarity

Maxime

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Re: [PATCH 2/5] drm/sun4i: tcon: set sync polarity for tcon1 channel

[Jernej Škrabec]

Dne petek, 05. februar 2021 ob 17:01:30 CET je Maxime Ripard napisal(a):
> On Fri, Feb 05, 2021 at 11:21:22AM +0800, Chen-Yu Tsai wrote:
> > On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <jernej.skrabec@siol.net> 
wrote:
> > >
> > > Channel 1 has polarity bits for vsync and hsync signals but driver never
> > > sets them. It turns out that with pre-HDMI2 controllers seemingly there
> > > is no issue if polarity is not set. However, with HDMI2 controllers
> > > (H6) there often comes to de-synchronization due to phase shift. This
> > > causes flickering screen. It's safe to assume that similar issues might
> > > happen also with pre-HDMI2 controllers.
> > >
> > > Solve issue with setting vsync and hsync polarity. Note that display
> > > stacks with tcon top have polarity bits actually in tcon0 polarity
> > > register.
> > >
> > > Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
> > > Tested-by: Andre Heider <a.heider@gmail.com>
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > ---
> > >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
> > >  drivers/gpu/drm/sun4i/sun4i_tcon.h |  5 +++++
> > >  2 files changed, 29 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/
sun4i_tcon.c
> > > index 6b9af4c08cd6..0d132dae58c0 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > @@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct sun4i_tcon 
*tcon,
> > >                      SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
> > >                      SUN4I_TCON1_BASIC5_H_SYNC(hsync));
> > >
> > > +       /* Setup the polarity of sync signals */
> > > +       if (tcon->quirks->polarity_in_ch0) {
> > > +               val = 0;
> > > +
> > > +               if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > > +                       val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
> > > +
> > > +               if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > > +                       val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
> > > +
> > > +               regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, val);
> > > +       } else {
> > > +               val = SUN4I_TCON1_IO_POL_UNKNOWN;
> > 
> > I think a comment for the origin of this is warranted.
> 
> If it's anything like TCON0, it's the pixel clock polarity

Hard to say, DW HDMI controller has "data enable" polarity along hsync and 
vsync. It could be either or none of those.

What should I write in comment? BSP drivers and documentation use only generic 
names like io2_inv.

Best regards,
Jernej



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Re: [PATCH 2/5] drm/sun4i: tcon: set sync polarity for tcon1 channel

[Chen-Yu Tsai]

On Sat, Feb 6, 2021 at 12:21 AM Jernej Škrabec <jernej.skrabec@siol.net> wrote:
>
> Dne petek, 05. februar 2021 ob 17:01:30 CET je Maxime Ripard napisal(a):
> > On Fri, Feb 05, 2021 at 11:21:22AM +0800, Chen-Yu Tsai wrote:
> > > On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <jernej.skrabec@siol.net>
> wrote:
> > > >
> > > > Channel 1 has polarity bits for vsync and hsync signals but driver never
> > > > sets them. It turns out that with pre-HDMI2 controllers seemingly there
> > > > is no issue if polarity is not set. However, with HDMI2 controllers
> > > > (H6) there often comes to de-synchronization due to phase shift. This
> > > > causes flickering screen. It's safe to assume that similar issues might
> > > > happen also with pre-HDMI2 controllers.
> > > >
> > > > Solve issue with setting vsync and hsync polarity. Note that display
> > > > stacks with tcon top have polarity bits actually in tcon0 polarity
> > > > register.
> > > >
> > > > Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
> > > > Tested-by: Andre Heider <a.heider@gmail.com>
> > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > > ---
> > > >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
> > > >  drivers/gpu/drm/sun4i/sun4i_tcon.h |  5 +++++
> > > >  2 files changed, 29 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/
> sun4i_tcon.c
> > > > index 6b9af4c08cd6..0d132dae58c0 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > @@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct sun4i_tcon
> *tcon,
> > > >                      SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
> > > >                      SUN4I_TCON1_BASIC5_H_SYNC(hsync));
> > > >
> > > > +       /* Setup the polarity of sync signals */
> > > > +       if (tcon->quirks->polarity_in_ch0) {
> > > > +               val = 0;
> > > > +
> > > > +               if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > > > +                       val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
> > > > +
> > > > +               if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > > > +                       val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
> > > > +
> > > > +               regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, val);
> > > > +       } else {
> > > > +               val = SUN4I_TCON1_IO_POL_UNKNOWN;
> > >
> > > I think a comment for the origin of this is warranted.
> >
> > If it's anything like TCON0, it's the pixel clock polarity
>
> Hard to say, DW HDMI controller has "data enable" polarity along hsync and
> vsync. It could be either or none of those.
>
> What should I write in comment? BSP drivers and documentation use only generic
> names like io2_inv.

Just say that we don't know exactly what it is, but it is required for things
to work properly? Would be interesting to know what happens if you don't set
this bit, but do set VSYNC/HSYNC polarity properly.

ChenYu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Re: Re: [PATCH 2/5] drm/sun4i: tcon: set sync polarity for tcon1 channel

[Jernej Škrabec]

Dne petek, 05. februar 2021 ob 17:28:23 CET je Chen-Yu Tsai napisal(a):
> On Sat, Feb 6, 2021 at 12:21 AM Jernej Škrabec <jernej.skrabec@siol.net> 
wrote:
> >
> > Dne petek, 05. februar 2021 ob 17:01:30 CET je Maxime Ripard napisal(a):
> > > On Fri, Feb 05, 2021 at 11:21:22AM +0800, Chen-Yu Tsai wrote:
> > > > On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec 
<jernej.skrabec@siol.net>
> > wrote:
> > > > >
> > > > > Channel 1 has polarity bits for vsync and hsync signals but driver 
never
> > > > > sets them. It turns out that with pre-HDMI2 controllers seemingly 
there
> > > > > is no issue if polarity is not set. However, with HDMI2 controllers
> > > > > (H6) there often comes to de-synchronization due to phase shift. 
This
> > > > > causes flickering screen. It's safe to assume that similar issues 
might
> > > > > happen also with pre-HDMI2 controllers.
> > > > >
> > > > > Solve issue with setting vsync and hsync polarity. Note that display
> > > > > stacks with tcon top have polarity bits actually in tcon0 polarity
> > > > > register.
> > > > >
> > > > > Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine 
support")
> > > > > Tested-by: Andre Heider <a.heider@gmail.com>
> > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > > > ---
> > > > >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
> > > > >  drivers/gpu/drm/sun4i/sun4i_tcon.h |  5 +++++
> > > > >  2 files changed, 29 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/
sun4i/
> > sun4i_tcon.c
> > > > > index 6b9af4c08cd6..0d132dae58c0 100644
> > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > @@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct 
sun4i_tcon
> > *tcon,
> > > > >                      SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
> > > > >                      SUN4I_TCON1_BASIC5_H_SYNC(hsync));
> > > > >
> > > > > +       /* Setup the polarity of sync signals */
> > > > > +       if (tcon->quirks->polarity_in_ch0) {
> > > > > +               val = 0;
> > > > > +
> > > > > +               if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > > > > +                       val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
> > > > > +
> > > > > +               if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > > > > +                       val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
> > > > > +
> > > > > +               regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, 
val);
> > > > > +       } else {
> > > > > +               val = SUN4I_TCON1_IO_POL_UNKNOWN;
> > > >
> > > > I think a comment for the origin of this is warranted.
> > >
> > > If it's anything like TCON0, it's the pixel clock polarity
> >
> > Hard to say, DW HDMI controller has "data enable" polarity along hsync and
> > vsync. It could be either or none of those.
> >
> > What should I write in comment? BSP drivers and documentation use only 
generic
> > names like io2_inv.
> 
> Just say that we don't know exactly what it is, but it is required for 
things
> to work properly? Would be interesting to know what happens if you don't set
> this bit, but do set VSYNC/HSYNC polarity properly.

Nothing seems to happen - tested on H3 with HDMI (4k@30) and CVBS. At least I 
didn't notice anything.

Best regards,
Jernej



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Re: [linux-sunxi] [PATCH 4/5] drm/sun4i: Fix H6 HDMI PHY configuration

[Jernej Škrabec]

Dne petek, 05. februar 2021 ob 04:22:56 CET je Chen-Yu Tsai napisal(a):
> On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec <jernej.skrabec@siol.net> 
wrote:
> >
> > cpce value for 594 MHz is set differently in BSP driver. Fix that.
> >
> > Fixes: c71c9b2fee17 ("drm/sun4i: Add support for Synopsys HDMI PHY")
> > Tested-by: Andre Heider <a.heider@gmail.com>
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> 
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>

Thanks, but I figured that this change is not the proper one. It still gives me 
issues with my TV. Proper change is to fix current and voltage settings below. 
I'll replace this patch in v2.

Best regards,
Jernej

> 
> -- 
> You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
email to linux-sunxi+unsubscribe@googlegroups.com.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/
linux-sunxi/
CAGb2v64BpizczmSJdompGosFwWWayNscWvW-7oARLgwNNo%3DteQ%40mail.gmail.com.
> 



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/5] drm/sun4i: tcon: set sync polarity for tcon1 channel

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3905 bytes --]

On Fri, Feb 05, 2021 at 07:47:17PM +0100, Jernej Škrabec wrote:
> Dne petek, 05. februar 2021 ob 17:28:23 CET je Chen-Yu Tsai napisal(a):
> > On Sat, Feb 6, 2021 at 12:21 AM Jernej Škrabec <jernej.skrabec@siol.net> 
> wrote:
> > >
> > > Dne petek, 05. februar 2021 ob 17:01:30 CET je Maxime Ripard napisal(a):
> > > > On Fri, Feb 05, 2021 at 11:21:22AM +0800, Chen-Yu Tsai wrote:
> > > > > On Fri, Feb 5, 2021 at 2:48 AM Jernej Skrabec 
> <jernej.skrabec@siol.net>
> > > wrote:
> > > > > >
> > > > > > Channel 1 has polarity bits for vsync and hsync signals but driver 
> never
> > > > > > sets them. It turns out that with pre-HDMI2 controllers seemingly 
> there
> > > > > > is no issue if polarity is not set. However, with HDMI2 controllers
> > > > > > (H6) there often comes to de-synchronization due to phase shift. 
> This
> > > > > > causes flickering screen. It's safe to assume that similar issues 
> might
> > > > > > happen also with pre-HDMI2 controllers.
> > > > > >
> > > > > > Solve issue with setting vsync and hsync polarity. Note that display
> > > > > > stacks with tcon top have polarity bits actually in tcon0 polarity
> > > > > > register.
> > > > > >
> > > > > > Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine 
> support")
> > > > > > Tested-by: Andre Heider <a.heider@gmail.com>
> > > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > > > > ---
> > > > > >  drivers/gpu/drm/sun4i/sun4i_tcon.c | 24 ++++++++++++++++++++++++
> > > > > >  drivers/gpu/drm/sun4i/sun4i_tcon.h |  5 +++++
> > > > > >  2 files changed, 29 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/
> sun4i/
> > > sun4i_tcon.c
> > > > > > index 6b9af4c08cd6..0d132dae58c0 100644
> > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > @@ -672,6 +672,29 @@ static void sun4i_tcon1_mode_set(struct 
> sun4i_tcon
> > > *tcon,
> > > > > >                      SUN4I_TCON1_BASIC5_V_SYNC(vsync) |
> > > > > >                      SUN4I_TCON1_BASIC5_H_SYNC(hsync));
> > > > > >
> > > > > > +       /* Setup the polarity of sync signals */
> > > > > > +       if (tcon->quirks->polarity_in_ch0) {
> > > > > > +               val = 0;
> > > > > > +
> > > > > > +               if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > > > > > +                       val |= SUN4I_TCON0_IO_POL_HSYNC_POSITIVE;
> > > > > > +
> > > > > > +               if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > > > > > +                       val |= SUN4I_TCON0_IO_POL_VSYNC_POSITIVE;
> > > > > > +
> > > > > > +               regmap_write(tcon->regs, SUN4I_TCON0_IO_POL_REG, 
> val);
> > > > > > +       } else {
> > > > > > +               val = SUN4I_TCON1_IO_POL_UNKNOWN;
> > > > >
> > > > > I think a comment for the origin of this is warranted.
> > > >
> > > > If it's anything like TCON0, it's the pixel clock polarity
> > >
> > > Hard to say, DW HDMI controller has "data enable" polarity along hsync and
> > > vsync. It could be either or none of those.
> > >
> > > What should I write in comment? BSP drivers and documentation use only 
> generic
> > > names like io2_inv.
> > 
> > Just say that we don't know exactly what it is, but it is required for 
> things
> > to work properly? Would be interesting to know what happens if you don't set
> > this bit, but do set VSYNC/HSYNC polarity properly.
> 
> Nothing seems to happen - tested on H3 with HDMI (4k@30) and CVBS. At least I 
> didn't notice anything.

That's pretty normal, an inverted pixel clock would at worst give you
some weird artifacts and / or pixels being of the wrong color. Data
enable on the other hand would very likely stall the HDMI controller
since you would have only the blanking periods that would be considered
valid.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel