linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm: bridge: icn6211: Adjust clock phase using SYS_CTRL_1
@ 2022-05-23  2:38 Jonathan Liu
  2022-05-23 11:10 ` Marek Vasut
  0 siblings, 1 reply; 2+ messages in thread
From: Jonathan Liu @ 2022-05-23  2:38 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, Maxime Ripard, Jonathan Liu, Jagan Teki,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	linux-kernel

The code from [1] sets SYS_CTRL_1 to different values depending on the
desired clock phase (0, 1/4, 1/2 or 3/4). A clock phase of 0 aligns the
positive edge of the clock with the pixel data while other values delay
the clock by a fraction of the clock period. A clock phase of 1/2 aligns
the negative edge of the clock with the pixel data.

The driver currently hard codes SYS_CTRL_1 to 0x88 which corresponds to
aligning the positive edge of the clock with the pixel data. This won't
work correctly for panels that require aligning the negative edge of the
clock with the pixel data.

Adjust the clock phase to 0 if DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE is
present in bus_flags, otherwise adjust the clock phase to 1/2 as
appropriate for DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE.

[1] https://github.com/tdjastrzebski/ICN6211-Configurator

Signed-off-by: Jonathan Liu <net147@gmail.com>
---
 drivers/gpu/drm/bridge/chipone-icn6211.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c
index 47dea657a752..df0290059aa3 100644
--- a/drivers/gpu/drm/bridge/chipone-icn6211.c
+++ b/drivers/gpu/drm/bridge/chipone-icn6211.c
@@ -57,6 +57,10 @@
 #define BIST_CHESS_XY_H		0x30
 #define BIST_FRAME_TIME_L	0x31
 #define BIST_FRAME_TIME_H	0x32
+#define CLK_PHASE_0		0x88
+#define CLK_PHASE_1_4		0x98
+#define CLK_PHASE_1_2		0xa8
+#define CLK_PHASE_3_4		0xb8
 #define FIFO_MAX_ADDR_LOW	0x33
 #define SYNC_EVENT_DLY		0x34
 #define HSW_MIN			0x35
@@ -414,7 +418,11 @@ static void chipone_atomic_enable(struct drm_bridge *bridge,
 	chipone_configure_pll(icn, mode);
 
 	chipone_writeb(icn, SYS_CTRL(0), 0x40);
-	chipone_writeb(icn, SYS_CTRL(1), 0x88);
+
+	if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
+		chipone_writeb(icn, SYS_CTRL(1), CLK_PHASE_0);
+	else
+		chipone_writeb(icn, SYS_CTRL(1), CLK_PHASE_1_2);
 
 	/* icn6211 specific sequence */
 	chipone_writeb(icn, MIPI_FORCE_0, 0x20);
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] drm: bridge: icn6211: Adjust clock phase using SYS_CTRL_1
  2022-05-23  2:38 [PATCH] drm: bridge: icn6211: Adjust clock phase using SYS_CTRL_1 Jonathan Liu
@ 2022-05-23 11:10 ` Marek Vasut
  0 siblings, 0 replies; 2+ messages in thread
From: Marek Vasut @ 2022-05-23 11:10 UTC (permalink / raw)
  To: Jonathan Liu, dri-devel
  Cc: Maxime Ripard, Jagan Teki, Andrzej Hajda, Neil Armstrong,
	Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	David Airlie, Daniel Vetter, linux-kernel

On 5/23/22 04:38, Jonathan Liu wrote:
> The code from [1] sets SYS_CTRL_1 to different values depending on the
> desired clock phase (0, 1/4, 1/2 or 3/4). A clock phase of 0 aligns the
> positive edge of the clock with the pixel data while other values delay
> the clock by a fraction of the clock period. A clock phase of 1/2 aligns
> the negative edge of the clock with the pixel data.
> 
> The driver currently hard codes SYS_CTRL_1 to 0x88 which corresponds to
> aligning the positive edge of the clock with the pixel data. This won't
> work correctly for panels that require aligning the negative edge of the
> clock with the pixel data.
> 
> Adjust the clock phase to 0 if DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE is
> present in bus_flags, otherwise adjust the clock phase to 1/2 as
> appropriate for DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE.
> 
> [1] https://github.com/tdjastrzebski/ICN6211-Configurator
> 
> Signed-off-by: Jonathan Liu <net147@gmail.com>
> ---
>   drivers/gpu/drm/bridge/chipone-icn6211.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c
> index 47dea657a752..df0290059aa3 100644
> --- a/drivers/gpu/drm/bridge/chipone-icn6211.c
> +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c
> @@ -57,6 +57,10 @@
>   #define BIST_CHESS_XY_H		0x30
>   #define BIST_FRAME_TIME_L	0x31
>   #define BIST_FRAME_TIME_H	0x32
> +#define CLK_PHASE_0		0x88
> +#define CLK_PHASE_1_4		0x98
> +#define CLK_PHASE_1_2		0xa8
> +#define CLK_PHASE_3_4		0xb8
>   #define FIFO_MAX_ADDR_LOW	0x33
>   #define SYNC_EVENT_DLY		0x34
>   #define HSW_MIN			0x35
> @@ -414,7 +418,11 @@ static void chipone_atomic_enable(struct drm_bridge *bridge,
>   	chipone_configure_pll(icn, mode);
>   
>   	chipone_writeb(icn, SYS_CTRL(0), 0x40);
> -	chipone_writeb(icn, SYS_CTRL(1), 0x88);
> +
> +	if (bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE)
> +		chipone_writeb(icn, SYS_CTRL(1), CLK_PHASE_0);
> +	else
> +		chipone_writeb(icn, SYS_CTRL(1), CLK_PHASE_1_2);

Shouldn't both be "chipone_writeb(icn, SYS_CTRL(1), CLK_PHASE_0 | 0x8); 
and chipone_writeb(icn, SYS_CTRL(1), CLK_PHASE_1_2 | 0x8); respectively 
? I recall you mentioned that only the top two bits indicate the clock 
polarity , so the bottom 6 bits are something else ?

With that fixed:

Reviewed-by: Marek Vasut <marex@denx.de>

Thanks

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-05-23 11:11 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-23  2:38 [PATCH] drm: bridge: icn6211: Adjust clock phase using SYS_CTRL_1 Jonathan Liu
2022-05-23 11:10 ` Marek Vasut

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).