[PATCH v3 0/7] drm/bridge: ti-sn65dsi86: Fix bridge for non always-on regulators

[PATCH v3 0/7] drm/bridge: ti-sn65dsi86: Fix bridge for non always-on regulators

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

We noticed the bridge/panel on our devices stopped working if the
regulator wasn't always-on/boot-on. In an attempt to fix that, I came up
with the following patches. I've sent some of them up to the list
already, but none have landed, so I'll concatonate them in this series
going forward.

Sean

PS- chromium.org is now enforcing DMARC, which will conveniently deposit
my patches into gmail spam upon delivery. I'll be using this email
address to send mail/patches for the foreseeable future.

Sean Paul (7):
  drm/panel: simple: tv123wam: Add unprepare delay
  drm/bridge: ti-sn65dsi86: Fixup register names
  drm/bridge: ti-sn65dsi86: Implement AUX channel
  drm/bridge: ti-sn65dsi86: Move panel_prepare() to pre_enable()
  drm/bridge: ti-sn65dsi86: Poll for DP PLL Lock
  drm/bridge: ti-sn65dsi86: Poll for training complete
  drm/bridge: ti-sn65dsi86: Add mystery delay to enable()

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 227 +++++++++++++++++++-------
 drivers/gpu/drm/panel/panel-simple.c  |   3 +
 2 files changed, 174 insertions(+), 56 deletions(-)

-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 1/7] drm/panel: simple: tv123wam: Add unprepare delay

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

The panel datasheet specifies a 500ms delay after power-down before
re-enabling.

Changes in v2:
- None
Changes in v3:
- Added to the set

Cc: Sandeep Panda <spanda@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/panel/panel-simple.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 5b5d0a24e713e..97964f7f2acee 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1385,6 +1385,9 @@ static const struct panel_desc innolux_tv123wam = {
 		.width = 259,
 		.height = 173,
 	},
+	.delay = {
+		.unprepare = 500,
+	},
 };
 
 static const struct drm_display_mode innolux_zj070na_01p_mode = {
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/7] drm/panel: simple: tv123wam: Add unprepare delay

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> The panel datasheet specifies a 500ms delay after power-down before
> re-enabling.
> 
> Changes in v2:
> - None
> Changes in v3:
> - Added to the set
> 
> Cc: Sandeep Panda <spanda@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 5b5d0a24e713e..97964f7f2acee 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1385,6 +1385,9 @@ static const struct panel_desc innolux_tv123wam = 
> {
>  		.width = 259,
>  		.height = 173,
>  	},
> +	.delay = {
> +		.unprepare = 500,
> +	},
>  };
> 
>  static const struct drm_display_mode innolux_zj070na_01p_mode = {
Reviewed-by: Sandeep Panda <spanda@codeaurora.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/7] drm/panel: simple: tv123wam: Add unprepare delay

[Sean Paul]

On Tue, Aug 14, 2018 at 04:10:02PM +0530, spanda@codeaurora.org wrote:
> On 2018-08-14 03:00, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > The panel datasheet specifies a 500ms delay after power-down before
> > re-enabling.
> > 
> > Changes in v2:
> > - None
> > Changes in v3:
> > - Added to the set
> > 
> > Cc: Sandeep Panda <spanda@codeaurora.org>
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/gpu/drm/panel/panel-simple.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c
> > b/drivers/gpu/drm/panel/panel-simple.c
> > index 5b5d0a24e713e..97964f7f2acee 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -1385,6 +1385,9 @@ static const struct panel_desc innolux_tv123wam =
> > {
> >  		.width = 259,
> >  		.height = 173,
> >  	},
> > +	.delay = {
> > +		.unprepare = 500,
> > +	},
> >  };
> > 
> >  static const struct drm_display_mode innolux_zj070na_01p_mode = {
> Reviewed-by: Sandeep Panda <spanda@codeaurora.org>

Applied to -misc-next-fixes, thanks for your review.

Sean

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 2/7] drm/bridge: ti-sn65dsi86: Fixup register names

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Order registers by offset and rename bits & masks to match the
datasheet. This makes the driver a bit easier to grok and
cross-reference with the datasheet.

Changes in v3:
- Added to the set

Cc: Sandeep Panda <spanda@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 92 +++++++++++++--------------
 1 file changed, 45 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 1b6e8b72be584..587d4e4f5674c 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -18,36 +18,51 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
-/* Link Training specific registers */
 #define SN_DEVICE_REV_REG			0x08
-#define SN_HPD_DISABLE_REG			0x5C
 #define SN_DPPLL_SRC_REG			0x0A
+#define  DPPLL_CLK_SRC_DSICLK			BIT(0)
+#define  REFCLK_FREQ_MASK			GENMASK(3, 1)
+#define  REFCLK_FREQ(x)				((x) << 1)
+#define  DPPLL_SRC_DP_PLL_LOCK			BIT(7)
+#define SN_PLL_ENABLE_REG			0x0D
 #define SN_DSI_LANES_REG			0x10
+#define  CHA_DSI_LANES_MASK			GENMASK(4, 3)
+#define  CHA_DSI_LANES(x)			((x) << 3)
 #define SN_DSIA_CLK_FREQ_REG			0x12
-#define SN_ENH_FRAME_REG			0x5A
-#define SN_SSC_CONFIG_REG			0x93
-#define SN_DATARATE_CONFIG_REG			0x94
-#define SN_PLL_ENABLE_REG			0x0D
-#define SN_SCRAMBLE_CONFIG_REG			0x95
-#define SN_AUX_WDATA0_REG			0x64
-#define SN_AUX_ADDR_19_16_REG			0x74
-#define SN_AUX_ADDR_15_8_REG			0x75
-#define SN_AUX_ADDR_7_0_REG			0x76
-#define SN_AUX_LENGTH_REG			0x77
-#define SN_AUX_CMD_REG				0x78
-#define SN_ML_TX_MODE_REG			0x96
-/* video config specific registers */
 #define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG	0x20
 #define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG	0x24
 #define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG	0x2C
 #define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG	0x2D
+#define  CHA_HSYNC_POLARITY			BIT(7)
 #define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG	0x30
 #define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG	0x31
+#define  CHA_VSYNC_POLARITY			BIT(7)
 #define SN_CHA_HORIZONTAL_BACK_PORCH_REG	0x34
 #define SN_CHA_VERTICAL_BACK_PORCH_REG		0x36
 #define SN_CHA_HORIZONTAL_FRONT_PORCH_REG	0x38
 #define SN_CHA_VERTICAL_FRONT_PORCH_REG		0x3A
+#define SN_ENH_FRAME_REG			0x5A
+#define  VSTREAM_ENABLE				BIT(3)
 #define SN_DATA_FORMAT_REG			0x5B
+#define SN_HPD_DISABLE_REG			0x5C
+#define  HPD_DISABLE				BIT(0)
+#define SN_AUX_WDATA0_REG			0x64
+#define SN_AUX_ADDR_19_16_REG			0x74
+#define SN_AUX_ADDR_15_8_REG			0x75
+#define SN_AUX_ADDR_7_0_REG			0x76
+#define SN_AUX_LENGTH_REG			0x77
+#define SN_AUX_CMD_REG				0x78
+#define  AUX_CMD_SEND				BIT(1)
+#define  AUX_CMD_REQ(x)				((x) << 4)
+#define SN_SSC_CONFIG_REG			0x93
+#define  DP_NUM_LANES_MASK			GENMASK(5, 4)
+#define  DP_NUM_LANES(x)			((x) << 4)
+#define SN_DATARATE_CONFIG_REG			0x94
+#define  DP_DATARATE_MASK			GENMASK(7, 5)
+#define  DP_DATARATE(x)				((x) << 5)
+#define SN_ML_TX_MODE_REG			0x96
+#define  ML_TX_MAIN_LINK_OFF			0
+#define  ML_TX_NORMAL_MODE			BIT(0)
 
 #define MIN_DSI_CLK_FREQ_MHZ	40
 
@@ -55,22 +70,6 @@
 #define DP_CLK_FUDGE_NUM	10
 #define DP_CLK_FUDGE_DEN	8
 
-#define DPPLL_CLK_SRC_REFCLK	0
-#define DPPLL_CLK_SRC_DSICLK	1
-
-#define SN_REFCLK_FREQ_OFFSET	1
-#define SN_DSIA_LANE_OFFSET	3
-#define SN_DP_LANE_OFFSET	4
-#define SN_DP_DATA_RATE_OFFSET	5
-#define SN_SYNC_POLARITY_OFFSET	7
-
-#define SN_ENABLE_VID_STREAM_BIT	BIT(3)
-#define SN_REFCLK_FREQ_BITS		GENMASK(3, 1)
-#define SN_DSIA_NUM_LANES_BITS		GENMASK(4, 3)
-#define SN_DP_NUM_LANES_BITS		GENMASK(5, 4)
-#define SN_DP_DATA_RATE_BITS		GENMASK(7, 5)
-#define SN_HPD_DISABLE_BIT		BIT(0)
-
 #define SN_REGULATOR_SUPPLY_NUM		4
 
 struct ti_sn_bridge {
@@ -299,8 +298,7 @@ static void ti_sn_bridge_disable(struct drm_bridge *bridge)
 	drm_panel_disable(pdata->panel);
 
 	/* disable video stream */
-	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
-			   SN_ENABLE_VID_STREAM_BIT, 0);
+	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, VSTREAM_ENABLE, 0);
 	/* semi auto link training mode OFF */
 	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0);
 	/* disable DP PLL */
@@ -363,8 +361,8 @@ static void ti_sn_bridge_set_refclk_freq(struct ti_sn_bridge *pdata)
 		if (refclk_lut[i] == refclk_rate)
 			break;
 
-	regmap_update_bits(pdata->regmap, SN_DPPLL_SRC_REG,
-			   SN_REFCLK_FREQ_BITS, i << SN_REFCLK_FREQ_OFFSET);
+	regmap_update_bits(pdata->regmap, SN_DPPLL_SRC_REG, REFCLK_FREQ_MASK,
+			   REFCLK_FREQ(i));
 }
 
 /**
@@ -401,7 +399,7 @@ static void ti_sn_bridge_set_dsi_dp_rate(struct ti_sn_bridge *pdata)
 			break;
 
 	regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG,
-			   SN_DP_DATA_RATE_BITS, i << SN_DP_DATA_RATE_OFFSET);
+			   DP_DATARATE_MASK, DP_DATARATE(i));
 }
 
 static void ti_sn_bridge_set_video_timings(struct ti_sn_bridge *pdata)
@@ -411,9 +409,9 @@ static void ti_sn_bridge_set_video_timings(struct ti_sn_bridge *pdata)
 	u8 hsync_polarity = 0, vsync_polarity = 0;
 
 	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
-		hsync_polarity = BIT(SN_SYNC_POLARITY_OFFSET);
+		hsync_polarity = CHA_HSYNC_POLARITY;
 	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
-		vsync_polarity = BIT(SN_SYNC_POLARITY_OFFSET);
+		vsync_polarity = CHA_VSYNC_POLARITY;
 
 	ti_sn_bridge_write_u16(pdata, SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG,
 			       mode->hdisplay);
@@ -451,14 +449,14 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge)
 	drm_panel_prepare(pdata->panel);
 
 	/* DSI_A lane config */
-	val = (4 - pdata->dsi->lanes) << SN_DSIA_LANE_OFFSET;
+	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
 	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
-			   SN_DSIA_NUM_LANES_BITS, val);
+			   CHA_DSI_LANES_MASK, val);
 
 	/* DP lane config */
-	val = (pdata->dsi->lanes - 1) << SN_DP_LANE_OFFSET;
-	regmap_update_bits(pdata->regmap, SN_SSC_CONFIG_REG,
-			   SN_DP_NUM_LANES_BITS, val);
+	val = DP_NUM_LANES(pdata->dsi->lanes - 1);
+	regmap_update_bits(pdata->regmap, SN_SSC_CONFIG_REG, DP_NUM_LANES_MASK,
+			   val);
 
 	/* set dsi/dp clk frequency value */
 	ti_sn_bridge_set_dsi_dp_rate(pdata);
@@ -489,8 +487,8 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge)
 	ti_sn_bridge_set_video_timings(pdata);
 
 	/* enable video stream */
-	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
-			   SN_ENABLE_VID_STREAM_BIT, SN_ENABLE_VID_STREAM_BIT);
+	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, VSTREAM_ENABLE,
+			   VSTREAM_ENABLE);
 
 	drm_panel_enable(pdata->panel);
 }
@@ -505,8 +503,8 @@ static void ti_sn_bridge_pre_enable(struct drm_bridge *bridge)
 	ti_sn_bridge_set_refclk_freq(pdata);
 
 	/* in case drm_panel is connected then HPD is not supported */
-	regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG,
-			   SN_HPD_DISABLE_BIT, SN_HPD_DISABLE_BIT);
+	regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE,
+			   HPD_DISABLE);
 }
 
 static void ti_sn_bridge_post_disable(struct drm_bridge *bridge)
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/7] drm/bridge: ti-sn65dsi86: Fixup register names

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Order registers by offset and rename bits & masks to match the
> datasheet. This makes the driver a bit easier to grok and
> cross-reference with the datasheet.
> 
> Changes in v3:
> - Added to the set
> 
> Cc: Sandeep Panda <spanda@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 92 +++++++++++++--------------
>  1 file changed, 45 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 1b6e8b72be584..587d4e4f5674c 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -18,36 +18,51 @@
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
> 
> -/* Link Training specific registers */
>  #define SN_DEVICE_REV_REG			0x08
> -#define SN_HPD_DISABLE_REG			0x5C
>  #define SN_DPPLL_SRC_REG			0x0A
> +#define  DPPLL_CLK_SRC_DSICLK			BIT(0)
> +#define  REFCLK_FREQ_MASK			GENMASK(3, 1)
> +#define  REFCLK_FREQ(x)				((x) << 1)
> +#define  DPPLL_SRC_DP_PLL_LOCK			BIT(7)
> +#define SN_PLL_ENABLE_REG			0x0D
>  #define SN_DSI_LANES_REG			0x10
> +#define  CHA_DSI_LANES_MASK			GENMASK(4, 3)
> +#define  CHA_DSI_LANES(x)			((x) << 3)
>  #define SN_DSIA_CLK_FREQ_REG			0x12
> -#define SN_ENH_FRAME_REG			0x5A
> -#define SN_SSC_CONFIG_REG			0x93
> -#define SN_DATARATE_CONFIG_REG			0x94
> -#define SN_PLL_ENABLE_REG			0x0D
> -#define SN_SCRAMBLE_CONFIG_REG			0x95
> -#define SN_AUX_WDATA0_REG			0x64
> -#define SN_AUX_ADDR_19_16_REG			0x74
> -#define SN_AUX_ADDR_15_8_REG			0x75
> -#define SN_AUX_ADDR_7_0_REG			0x76
> -#define SN_AUX_LENGTH_REG			0x77
> -#define SN_AUX_CMD_REG				0x78
> -#define SN_ML_TX_MODE_REG			0x96
> -/* video config specific registers */
>  #define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG	0x20
>  #define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG	0x24
>  #define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG	0x2C
>  #define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG	0x2D
> +#define  CHA_HSYNC_POLARITY			BIT(7)
>  #define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG	0x30
>  #define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG	0x31
> +#define  CHA_VSYNC_POLARITY			BIT(7)
>  #define SN_CHA_HORIZONTAL_BACK_PORCH_REG	0x34
>  #define SN_CHA_VERTICAL_BACK_PORCH_REG		0x36
>  #define SN_CHA_HORIZONTAL_FRONT_PORCH_REG	0x38
>  #define SN_CHA_VERTICAL_FRONT_PORCH_REG		0x3A
> +#define SN_ENH_FRAME_REG			0x5A
> +#define  VSTREAM_ENABLE				BIT(3)
>  #define SN_DATA_FORMAT_REG			0x5B
> +#define SN_HPD_DISABLE_REG			0x5C
> +#define  HPD_DISABLE				BIT(0)
> +#define SN_AUX_WDATA0_REG			0x64
> +#define SN_AUX_ADDR_19_16_REG			0x74
> +#define SN_AUX_ADDR_15_8_REG			0x75
> +#define SN_AUX_ADDR_7_0_REG			0x76
> +#define SN_AUX_LENGTH_REG			0x77
> +#define SN_AUX_CMD_REG				0x78
> +#define  AUX_CMD_SEND				BIT(1)
> +#define  AUX_CMD_REQ(x)				((x) << 4)
> +#define SN_SSC_CONFIG_REG			0x93
> +#define  DP_NUM_LANES_MASK			GENMASK(5, 4)
> +#define  DP_NUM_LANES(x)			((x) << 4)
> +#define SN_DATARATE_CONFIG_REG			0x94
> +#define  DP_DATARATE_MASK			GENMASK(7, 5)
> +#define  DP_DATARATE(x)				((x) << 5)
> +#define SN_ML_TX_MODE_REG			0x96
> +#define  ML_TX_MAIN_LINK_OFF			0
> +#define  ML_TX_NORMAL_MODE			BIT(0)
> 
>  #define MIN_DSI_CLK_FREQ_MHZ	40
> 
> @@ -55,22 +70,6 @@
>  #define DP_CLK_FUDGE_NUM	10
>  #define DP_CLK_FUDGE_DEN	8
> 
> -#define DPPLL_CLK_SRC_REFCLK	0
> -#define DPPLL_CLK_SRC_DSICLK	1
> -
> -#define SN_REFCLK_FREQ_OFFSET	1
> -#define SN_DSIA_LANE_OFFSET	3
> -#define SN_DP_LANE_OFFSET	4
> -#define SN_DP_DATA_RATE_OFFSET	5
> -#define SN_SYNC_POLARITY_OFFSET	7
> -
> -#define SN_ENABLE_VID_STREAM_BIT	BIT(3)
> -#define SN_REFCLK_FREQ_BITS		GENMASK(3, 1)
> -#define SN_DSIA_NUM_LANES_BITS		GENMASK(4, 3)
> -#define SN_DP_NUM_LANES_BITS		GENMASK(5, 4)
> -#define SN_DP_DATA_RATE_BITS		GENMASK(7, 5)
> -#define SN_HPD_DISABLE_BIT		BIT(0)
> -
>  #define SN_REGULATOR_SUPPLY_NUM		4
> 
>  struct ti_sn_bridge {
> @@ -299,8 +298,7 @@ static void ti_sn_bridge_disable(struct drm_bridge 
> *bridge)
>  	drm_panel_disable(pdata->panel);
> 
>  	/* disable video stream */
> -	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
> -			   SN_ENABLE_VID_STREAM_BIT, 0);
> +	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, VSTREAM_ENABLE, 
> 0);
>  	/* semi auto link training mode OFF */
>  	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0);
>  	/* disable DP PLL */
> @@ -363,8 +361,8 @@ static void ti_sn_bridge_set_refclk_freq(struct
> ti_sn_bridge *pdata)
>  		if (refclk_lut[i] == refclk_rate)
>  			break;
> 
> -	regmap_update_bits(pdata->regmap, SN_DPPLL_SRC_REG,
> -			   SN_REFCLK_FREQ_BITS, i << SN_REFCLK_FREQ_OFFSET);
> +	regmap_update_bits(pdata->regmap, SN_DPPLL_SRC_REG, REFCLK_FREQ_MASK,
> +			   REFCLK_FREQ(i));
>  }
> 
>  /**
> @@ -401,7 +399,7 @@ static void ti_sn_bridge_set_dsi_dp_rate(struct
> ti_sn_bridge *pdata)
>  			break;
> 
>  	regmap_update_bits(pdata->regmap, SN_DATARATE_CONFIG_REG,
> -			   SN_DP_DATA_RATE_BITS, i << SN_DP_DATA_RATE_OFFSET);
> +			   DP_DATARATE_MASK, DP_DATARATE(i));
>  }
> 
>  static void ti_sn_bridge_set_video_timings(struct ti_sn_bridge *pdata)
> @@ -411,9 +409,9 @@ static void ti_sn_bridge_set_video_timings(struct
> ti_sn_bridge *pdata)
>  	u8 hsync_polarity = 0, vsync_polarity = 0;
> 
>  	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> -		hsync_polarity = BIT(SN_SYNC_POLARITY_OFFSET);
> +		hsync_polarity = CHA_HSYNC_POLARITY;
>  	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> -		vsync_polarity = BIT(SN_SYNC_POLARITY_OFFSET);
> +		vsync_polarity = CHA_VSYNC_POLARITY;
> 
>  	ti_sn_bridge_write_u16(pdata, SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG,
>  			       mode->hdisplay);
> @@ -451,14 +449,14 @@ static void ti_sn_bridge_enable(struct drm_bridge 
> *bridge)
>  	drm_panel_prepare(pdata->panel);
> 
>  	/* DSI_A lane config */
> -	val = (4 - pdata->dsi->lanes) << SN_DSIA_LANE_OFFSET;
> +	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
>  	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
> -			   SN_DSIA_NUM_LANES_BITS, val);
> +			   CHA_DSI_LANES_MASK, val);
> 
>  	/* DP lane config */
> -	val = (pdata->dsi->lanes - 1) << SN_DP_LANE_OFFSET;
> -	regmap_update_bits(pdata->regmap, SN_SSC_CONFIG_REG,
> -			   SN_DP_NUM_LANES_BITS, val);
> +	val = DP_NUM_LANES(pdata->dsi->lanes - 1);
> +	regmap_update_bits(pdata->regmap, SN_SSC_CONFIG_REG, 
> DP_NUM_LANES_MASK,
> +			   val);
> 
>  	/* set dsi/dp clk frequency value */
>  	ti_sn_bridge_set_dsi_dp_rate(pdata);
> @@ -489,8 +487,8 @@ static void ti_sn_bridge_enable(struct drm_bridge 
> *bridge)
>  	ti_sn_bridge_set_video_timings(pdata);
> 
>  	/* enable video stream */
> -	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
> -			   SN_ENABLE_VID_STREAM_BIT, SN_ENABLE_VID_STREAM_BIT);
> +	regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, VSTREAM_ENABLE,
> +			   VSTREAM_ENABLE);
> 
>  	drm_panel_enable(pdata->panel);
>  }
> @@ -505,8 +503,8 @@ static void ti_sn_bridge_pre_enable(struct
> drm_bridge *bridge)
>  	ti_sn_bridge_set_refclk_freq(pdata);
> 
>  	/* in case drm_panel is connected then HPD is not supported */
> -	regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG,
> -			   SN_HPD_DISABLE_BIT, SN_HPD_DISABLE_BIT);
> +	regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE,
> +			   HPD_DISABLE);
>  }
> 
>  static void ti_sn_bridge_post_disable(struct drm_bridge *bridge)
Reviewed-by: Sandeep Panda <spanda@codeaurora.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 3/7] drm/bridge: ti-sn65dsi86: Implement AUX channel

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

This was hand-rolled in the first version, and will surely be useful as
we expand the driver to support more varied use cases.

Changes in v2:
- Change subject prefix s/panel/bridge/
- Downgrade warning in poll function to error message
- Fix DP_EDP_CONFIGURATION_SET write value (Sandeep)
- Mask upper 8 bits of msg->address (Sandeep)
- Check aux cmd status for errors after completing the send (Sandeep)
- Remove length check since it's covered in the aux status
- Flip the READ check in transfer to WRITE check + early exit
Changes in v3:
- Added to the set
- Wrapped (x) in WDATA/RDATA #defines
- Replace readx_poll* with regmap_read_poll*

Cc: Sandeep Panda <spanda@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 103 ++++++++++++++++++++++++--
 1 file changed, 95 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 587d4e4f5674c..501f4a81ea5ab 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -7,12 +7,14 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_dp_helper.h>
 #include <drm/drm_mipi_dsi.h>
 #include <drm/drm_of.h>
 #include <drm/drm_panel.h>
 #include <linux/clk.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
+#include <linux/iopoll.h>
 #include <linux/of_graph.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
@@ -46,7 +48,7 @@
 #define SN_DATA_FORMAT_REG			0x5B
 #define SN_HPD_DISABLE_REG			0x5C
 #define  HPD_DISABLE				BIT(0)
-#define SN_AUX_WDATA0_REG			0x64
+#define SN_AUX_WDATA_REG(x)			(0x64 + (x))
 #define SN_AUX_ADDR_19_16_REG			0x74
 #define SN_AUX_ADDR_15_8_REG			0x75
 #define SN_AUX_ADDR_7_0_REG			0x76
@@ -54,6 +56,7 @@
 #define SN_AUX_CMD_REG				0x78
 #define  AUX_CMD_SEND				BIT(1)
 #define  AUX_CMD_REQ(x)				((x) << 4)
+#define SN_AUX_RDATA_REG(x)			(0x79 + (x))
 #define SN_SSC_CONFIG_REG			0x93
 #define  DP_NUM_LANES_MASK			GENMASK(5, 4)
 #define  DP_NUM_LANES(x)			((x) << 4)
@@ -63,6 +66,10 @@
 #define SN_ML_TX_MODE_REG			0x96
 #define  ML_TX_MAIN_LINK_OFF			0
 #define  ML_TX_NORMAL_MODE			BIT(0)
+#define SN_AUX_CMD_STATUS_REG			0xF4
+#define  AUX_IRQ_STATUS_AUX_RPLY_TOUT		BIT(3)
+#define  AUX_IRQ_STATUS_AUX_SHORT		BIT(5)
+#define  AUX_IRQ_STATUS_NAT_I2C_FAIL		BIT(6)
 
 #define MIN_DSI_CLK_FREQ_MHZ	40
 
@@ -70,11 +77,15 @@
 #define DP_CLK_FUDGE_NUM	10
 #define DP_CLK_FUDGE_DEN	8
 
+/* Matches DP_AUX_MAX_PAYLOAD_BYTES (for now) */
+#define SN_AUX_MAX_PAYLOAD_BYTES	16
+
 #define SN_REGULATOR_SUPPLY_NUM		4
 
 struct ti_sn_bridge {
 	struct device			*dev;
 	struct regmap			*regmap;
+	struct drm_dp_aux		aux;
 	struct drm_bridge		bridge;
 	struct drm_connector		connector;
 	struct device_node		*host_node;
@@ -471,13 +482,8 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge)
 	 * authentication method. We need to enable this method in the eDP panel
 	 * at DisplayPort address 0x0010A prior to link training.
 	 */
-	regmap_write(pdata->regmap, SN_AUX_WDATA0_REG, 0x01);
-	regmap_write(pdata->regmap, SN_AUX_ADDR_19_16_REG, 0x00);
-	regmap_write(pdata->regmap, SN_AUX_ADDR_15_8_REG, 0x01);
-	regmap_write(pdata->regmap, SN_AUX_ADDR_7_0_REG, 0x0A);
-	regmap_write(pdata->regmap, SN_AUX_LENGTH_REG, 0x01);
-	regmap_write(pdata->regmap, SN_AUX_CMD_REG, 0x81);
-	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
+	drm_dp_dpcd_writeb(&pdata->aux, DP_EDP_CONFIGURATION_SET,
+			   DP_ALTERNATE_SCRAMBLER_RESET_ENABLE);
 
 	/* Semi auto link training mode */
 	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
@@ -525,6 +531,82 @@ static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
 	.post_disable = ti_sn_bridge_post_disable,
 };
 
+static struct ti_sn_bridge *aux_to_ti_sn_bridge(struct drm_dp_aux *aux)
+{
+	return container_of(aux, struct ti_sn_bridge, aux);
+}
+
+static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
+				  struct drm_dp_aux_msg *msg)
+{
+	struct ti_sn_bridge *pdata = aux_to_ti_sn_bridge(aux);
+	u32 request = msg->request & ~DP_AUX_I2C_MOT;
+	u32 request_val = AUX_CMD_REQ(msg->request);
+	u8 *buf = (u8 *)msg->buffer;
+	unsigned int val;
+	int ret, i;
+
+	if (msg->size > SN_AUX_MAX_PAYLOAD_BYTES)
+		return -EINVAL;
+
+	switch (request) {
+	case DP_AUX_NATIVE_WRITE:
+	case DP_AUX_I2C_WRITE:
+	case DP_AUX_NATIVE_READ:
+	case DP_AUX_I2C_READ:
+		regmap_write(pdata->regmap, SN_AUX_CMD_REG, request_val);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	regmap_write(pdata->regmap, SN_AUX_ADDR_19_16_REG,
+		     (msg->address >> 16) & 0xF);
+	regmap_write(pdata->regmap, SN_AUX_ADDR_15_8_REG,
+		     (msg->address >> 8) & 0xFF);
+	regmap_write(pdata->regmap, SN_AUX_ADDR_7_0_REG, msg->address & 0xFF);
+
+	regmap_write(pdata->regmap, SN_AUX_LENGTH_REG, msg->size);
+
+	if (request == DP_AUX_NATIVE_WRITE || request == DP_AUX_I2C_WRITE) {
+		for (i = 0; i < msg->size; i++)
+			regmap_write(pdata->regmap, SN_AUX_WDATA_REG(i),
+				     buf[i]);
+	}
+
+	regmap_write(pdata->regmap, SN_AUX_CMD_REG, request_val | AUX_CMD_SEND);
+
+	ret = regmap_read_poll_timeout(pdata->regmap, SN_AUX_CMD_REG, val,
+				       !(val & AUX_CMD_SEND), 200,
+				       50 * 1000);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(pdata->regmap, SN_AUX_CMD_STATUS_REG, &val);
+	if (ret)
+		return ret;
+	else if ((val & AUX_IRQ_STATUS_NAT_I2C_FAIL)
+		 || (val & AUX_IRQ_STATUS_AUX_RPLY_TOUT)
+		 || (val & AUX_IRQ_STATUS_AUX_SHORT))
+		return -ENXIO;
+
+	if (request == DP_AUX_NATIVE_WRITE || request == DP_AUX_I2C_WRITE)
+		return msg->size;
+
+	for (i = 0; i < msg->size; i++) {
+		unsigned int val;
+		ret = regmap_read(pdata->regmap, SN_AUX_RDATA_REG(i),
+				  &val);
+		if (ret)
+			return ret;
+
+		WARN_ON(val & ~0xFF);
+		buf[i] = (u8)(val & 0xFF);
+	}
+
+	return msg->size;
+}
+
 static int ti_sn_bridge_parse_dsi_host(struct ti_sn_bridge *pdata)
 {
 	struct device_node *np = pdata->dev->of_node;
@@ -604,6 +686,11 @@ static int ti_sn_bridge_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, pdata);
 
+	pdata->aux.name = "ti-sn65dsi86-aux";
+	pdata->aux.dev = pdata->dev;
+	pdata->aux.transfer = ti_sn_aux_transfer;
+	drm_dp_aux_register(&pdata->aux);
+
 	pdata->bridge.funcs = &ti_sn_bridge_funcs;
 	pdata->bridge.of_node = client->dev.of_node;
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 3/7] drm/bridge: ti-sn65dsi86: Implement AUX channel

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> This was hand-rolled in the first version, and will surely be useful as
> we expand the driver to support more varied use cases.
> 
> Changes in v2:
> - Change subject prefix s/panel/bridge/
> - Downgrade warning in poll function to error message
> - Fix DP_EDP_CONFIGURATION_SET write value (Sandeep)
> - Mask upper 8 bits of msg->address (Sandeep)
> - Check aux cmd status for errors after completing the send (Sandeep)
> - Remove length check since it's covered in the aux status
> - Flip the READ check in transfer to WRITE check + early exit
> Changes in v3:
> - Added to the set
> - Wrapped (x) in WDATA/RDATA #defines
> - Replace readx_poll* with regmap_read_poll*
> 
> Cc: Sandeep Panda <spanda@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 103 ++++++++++++++++++++++++--
>  1 file changed, 95 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 587d4e4f5674c..501f4a81ea5ab 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -7,12 +7,14 @@
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_dp_helper.h>
>  #include <drm/drm_mipi_dsi.h>
>  #include <drm/drm_of.h>
>  #include <drm/drm_panel.h>
>  #include <linux/clk.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/i2c.h>
> +#include <linux/iopoll.h>
>  #include <linux/of_graph.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
> @@ -46,7 +48,7 @@
>  #define SN_DATA_FORMAT_REG			0x5B
>  #define SN_HPD_DISABLE_REG			0x5C
>  #define  HPD_DISABLE				BIT(0)
> -#define SN_AUX_WDATA0_REG			0x64
> +#define SN_AUX_WDATA_REG(x)			(0x64 + (x))
>  #define SN_AUX_ADDR_19_16_REG			0x74
>  #define SN_AUX_ADDR_15_8_REG			0x75
>  #define SN_AUX_ADDR_7_0_REG			0x76
> @@ -54,6 +56,7 @@
>  #define SN_AUX_CMD_REG				0x78
>  #define  AUX_CMD_SEND				BIT(1)
>  #define  AUX_CMD_REQ(x)				((x) << 4)
> +#define SN_AUX_RDATA_REG(x)			(0x79 + (x))
>  #define SN_SSC_CONFIG_REG			0x93
>  #define  DP_NUM_LANES_MASK			GENMASK(5, 4)
>  #define  DP_NUM_LANES(x)			((x) << 4)
> @@ -63,6 +66,10 @@
>  #define SN_ML_TX_MODE_REG			0x96
>  #define  ML_TX_MAIN_LINK_OFF			0
>  #define  ML_TX_NORMAL_MODE			BIT(0)
> +#define SN_AUX_CMD_STATUS_REG			0xF4
> +#define  AUX_IRQ_STATUS_AUX_RPLY_TOUT		BIT(3)
> +#define  AUX_IRQ_STATUS_AUX_SHORT		BIT(5)
> +#define  AUX_IRQ_STATUS_NAT_I2C_FAIL		BIT(6)
> 
>  #define MIN_DSI_CLK_FREQ_MHZ	40
> 
> @@ -70,11 +77,15 @@
>  #define DP_CLK_FUDGE_NUM	10
>  #define DP_CLK_FUDGE_DEN	8
> 
> +/* Matches DP_AUX_MAX_PAYLOAD_BYTES (for now) */
> +#define SN_AUX_MAX_PAYLOAD_BYTES	16
> +
>  #define SN_REGULATOR_SUPPLY_NUM		4
> 
>  struct ti_sn_bridge {
>  	struct device			*dev;
>  	struct regmap			*regmap;
> +	struct drm_dp_aux		aux;
>  	struct drm_bridge		bridge;
>  	struct drm_connector		connector;
>  	struct device_node		*host_node;
> @@ -471,13 +482,8 @@ static void ti_sn_bridge_enable(struct drm_bridge 
> *bridge)
>  	 * authentication method. We need to enable this method in the eDP 
> panel
>  	 * at DisplayPort address 0x0010A prior to link training.
>  	 */
> -	regmap_write(pdata->regmap, SN_AUX_WDATA0_REG, 0x01);
> -	regmap_write(pdata->regmap, SN_AUX_ADDR_19_16_REG, 0x00);
> -	regmap_write(pdata->regmap, SN_AUX_ADDR_15_8_REG, 0x01);
> -	regmap_write(pdata->regmap, SN_AUX_ADDR_7_0_REG, 0x0A);
> -	regmap_write(pdata->regmap, SN_AUX_LENGTH_REG, 0x01);
> -	regmap_write(pdata->regmap, SN_AUX_CMD_REG, 0x81);
> -	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
> +	drm_dp_dpcd_writeb(&pdata->aux, DP_EDP_CONFIGURATION_SET,
> +			   DP_ALTERNATE_SCRAMBLER_RESET_ENABLE);
> 
>  	/* Semi auto link training mode */
>  	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> @@ -525,6 +531,82 @@ static const struct drm_bridge_funcs 
> ti_sn_bridge_funcs = {
>  	.post_disable = ti_sn_bridge_post_disable,
>  };
> 
> +static struct ti_sn_bridge *aux_to_ti_sn_bridge(struct drm_dp_aux 
> *aux)
> +{
> +	return container_of(aux, struct ti_sn_bridge, aux);
> +}
> +
> +static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
> +				  struct drm_dp_aux_msg *msg)
> +{
> +	struct ti_sn_bridge *pdata = aux_to_ti_sn_bridge(aux);
> +	u32 request = msg->request & ~DP_AUX_I2C_MOT;
> +	u32 request_val = AUX_CMD_REQ(msg->request);
> +	u8 *buf = (u8 *)msg->buffer;
> +	unsigned int val;
> +	int ret, i;
> +
> +	if (msg->size > SN_AUX_MAX_PAYLOAD_BYTES)
> +		return -EINVAL;
> +
> +	switch (request) {
> +	case DP_AUX_NATIVE_WRITE:
> +	case DP_AUX_I2C_WRITE:
> +	case DP_AUX_NATIVE_READ:
> +	case DP_AUX_I2C_READ:
> +		regmap_write(pdata->regmap, SN_AUX_CMD_REG, request_val);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	regmap_write(pdata->regmap, SN_AUX_ADDR_19_16_REG,
> +		     (msg->address >> 16) & 0xF);
> +	regmap_write(pdata->regmap, SN_AUX_ADDR_15_8_REG,
> +		     (msg->address >> 8) & 0xFF);
> +	regmap_write(pdata->regmap, SN_AUX_ADDR_7_0_REG, msg->address & 
> 0xFF);
> +
> +	regmap_write(pdata->regmap, SN_AUX_LENGTH_REG, msg->size);
> +
> +	if (request == DP_AUX_NATIVE_WRITE || request == DP_AUX_I2C_WRITE) {
> +		for (i = 0; i < msg->size; i++)
> +			regmap_write(pdata->regmap, SN_AUX_WDATA_REG(i),
> +				     buf[i]);
> +	}
> +
> +	regmap_write(pdata->regmap, SN_AUX_CMD_REG, request_val | 
> AUX_CMD_SEND);
> +
> +	ret = regmap_read_poll_timeout(pdata->regmap, SN_AUX_CMD_REG, val,
> +				       !(val & AUX_CMD_SEND), 200,
> +				       50 * 1000);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(pdata->regmap, SN_AUX_CMD_STATUS_REG, &val);
> +	if (ret)
> +		return ret;
> +	else if ((val & AUX_IRQ_STATUS_NAT_I2C_FAIL)
> +		 || (val & AUX_IRQ_STATUS_AUX_RPLY_TOUT)
> +		 || (val & AUX_IRQ_STATUS_AUX_SHORT))
> +		return -ENXIO;
> +
> +	if (request == DP_AUX_NATIVE_WRITE || request == DP_AUX_I2C_WRITE)
> +		return msg->size;
> +
> +	for (i = 0; i < msg->size; i++) {
> +		unsigned int val;
> +		ret = regmap_read(pdata->regmap, SN_AUX_RDATA_REG(i),
> +				  &val);
> +		if (ret)
> +			return ret;
> +
> +		WARN_ON(val & ~0xFF);
> +		buf[i] = (u8)(val & 0xFF);
> +	}
> +
> +	return msg->size;
> +}
> +
>  static int ti_sn_bridge_parse_dsi_host(struct ti_sn_bridge *pdata)
>  {
>  	struct device_node *np = pdata->dev->of_node;
> @@ -604,6 +686,11 @@ static int ti_sn_bridge_probe(struct i2c_client 
> *client,
> 
>  	i2c_set_clientdata(client, pdata);
> 
> +	pdata->aux.name = "ti-sn65dsi86-aux";
> +	pdata->aux.dev = pdata->dev;
> +	pdata->aux.transfer = ti_sn_aux_transfer;
> +	drm_dp_aux_register(&pdata->aux);
> +
>  	pdata->bridge.funcs = &ti_sn_bridge_funcs;
>  	pdata->bridge.of_node = client->dev.of_node;
Reviewed-by: Sandeep Panda <spanda@codeaurora.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 4/7] drm/bridge: ti-sn65dsi86: Move panel_prepare() to pre_enable()

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

prepare() is the old-timey way to say pre_enable(). It should be called
before modeset. This fixes an issue where the panel on cheza must have
the regulator always-on/boot-on for it to work.

Changes in v3:
- Added to the set

Cc: Sandeep Panda <spanda@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 501f4a81ea5ab..d2119ab546147 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -457,8 +457,6 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge)
 	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
 	unsigned int val;
 
-	drm_panel_prepare(pdata->panel);
-
 	/* DSI_A lane config */
 	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
 	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
@@ -511,6 +509,8 @@ static void ti_sn_bridge_pre_enable(struct drm_bridge *bridge)
 	/* in case drm_panel is connected then HPD is not supported */
 	regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE,
 			   HPD_DISABLE);
+
+	drm_panel_prepare(pdata->panel);
 }
 
 static void ti_sn_bridge_post_disable(struct drm_bridge *bridge)
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 4/7] drm/bridge: ti-sn65dsi86: Move panel_prepare() to pre_enable()

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> prepare() is the old-timey way to say pre_enable(). It should be called
> before modeset. This fixes an issue where the panel on cheza must have
> the regulator always-on/boot-on for it to work.
> 
> Changes in v3:
> - Added to the set
> 
> Cc: Sandeep Panda <spanda@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 501f4a81ea5ab..d2119ab546147 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -457,8 +457,6 @@ static void ti_sn_bridge_enable(struct drm_bridge 
> *bridge)
>  	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>  	unsigned int val;
> 
> -	drm_panel_prepare(pdata->panel);
> -
>  	/* DSI_A lane config */
>  	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
>  	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
> @@ -511,6 +509,8 @@ static void ti_sn_bridge_pre_enable(struct
> drm_bridge *bridge)
>  	/* in case drm_panel is connected then HPD is not supported */
>  	regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE,
>  			   HPD_DISABLE);
> +
> +	drm_panel_prepare(pdata->panel);
>  }
> 
>  static void ti_sn_bridge_post_disable(struct drm_bridge *bridge)
Reviewed-by: Sandeep Panda <spanda@codeaurora.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 5/7] drm/bridge: ti-sn65dsi86: Poll for DP PLL Lock

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Instead of just waiting and hoping, actually poll for the pll lock to be
acquired. As a bonus, this should be significantly faster than the
sleep.

Changes in v3:
- Added to the set

Cc: Sandeep Panda <spanda@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index d2119ab546147..f02bdedae1e5e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -456,6 +456,7 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge)
 {
 	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
 	unsigned int val;
+	int ret;
 
 	/* DSI_A lane config */
 	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
@@ -472,7 +473,14 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge)
 
 	/* enable DP PLL */
 	regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 1);
-	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
+
+	ret = regmap_read_poll_timeout(pdata->regmap, SN_DPPLL_SRC_REG, val,
+				       val & DPPLL_SRC_DP_PLL_LOCK, 1000,
+				       50 * 1000);
+	if (ret) {
+		DRM_ERROR("DP_PLL_LOCK polling failed (%d)\n", ret);
+		return;
+	}
 
 	/**
 	 * The SN65DSI86 only supports ASSR Display Authentication method and
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 5/7] drm/bridge: ti-sn65dsi86: Poll for DP PLL Lock

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Instead of just waiting and hoping, actually poll for the pll lock to 
> be
> acquired. As a bonus, this should be significantly faster than the
> sleep.
> 
> Changes in v3:
> - Added to the set
> 
> Cc: Sandeep Panda <spanda@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index d2119ab546147..f02bdedae1e5e 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -456,6 +456,7 @@ static void ti_sn_bridge_enable(struct drm_bridge 
> *bridge)
>  {
>  	struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
>  	unsigned int val;
> +	int ret;
> 
>  	/* DSI_A lane config */
>  	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
> @@ -472,7 +473,14 @@ static void ti_sn_bridge_enable(struct drm_bridge 
> *bridge)
> 
>  	/* enable DP PLL */
>  	regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 1);
> -	usleep_range(10000, 10500); /* 10ms delay recommended by spec */
> +
> +	ret = regmap_read_poll_timeout(pdata->regmap, SN_DPPLL_SRC_REG, val,
> +				       val & DPPLL_SRC_DP_PLL_LOCK, 1000,
> +				       50 * 1000);
> +	if (ret) {
> +		DRM_ERROR("DP_PLL_LOCK polling failed (%d)\n", ret);
> +		return;
> +	}
> 
>  	/**
>  	 * The SN65DSI86 only supports ASSR Display Authentication method and
Reviewed-by: Sandeep Panda <spanda@codeaurora.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 6/7] drm/bridge: ti-sn65dsi86: Poll for training complete

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Instead of just waiting 20ms for training to complete, actually poll the
status to ensure training is finished.

Changes in v3:
- Added to the set

Cc: Sandeep Panda <spanda@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index f02bdedae1e5e..d3e27e52ea759 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -493,7 +493,17 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge)
 
 	/* Semi auto link training mode */
 	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
-	msleep(20); /* 20ms delay recommended by spec */
+	ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
+				       val == ML_TX_MAIN_LINK_OFF ||
+				       val == ML_TX_NORMAL_MODE, 1000,
+				       500 * 1000);
+	if (ret) {
+		DRM_ERROR("Training complete polling failed (%d)\n", ret);
+		return;
+	} else if (val == ML_TX_MAIN_LINK_OFF) {
+		DRM_ERROR("Link training failed, link is off\n");
+		return;
+	}
 
 	/* config video parameters */
 	ti_sn_bridge_set_video_timings(pdata);
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 6/7] drm/bridge: ti-sn65dsi86: Poll for training complete

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Instead of just waiting 20ms for training to complete, actually poll 
> the
> status to ensure training is finished.
> 
> Changes in v3:
> - Added to the set
> 
> Cc: Sandeep Panda <spanda@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index f02bdedae1e5e..d3e27e52ea759 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -493,7 +493,17 @@ static void ti_sn_bridge_enable(struct drm_bridge 
> *bridge)
> 
>  	/* Semi auto link training mode */
>  	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> -	msleep(20); /* 20ms delay recommended by spec */
> +	ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
> +				       val == ML_TX_MAIN_LINK_OFF ||
> +				       val == ML_TX_NORMAL_MODE, 1000,
> +				       500 * 1000);
> +	if (ret) {
> +		DRM_ERROR("Training complete polling failed (%d)\n", ret);
> +		return;
> +	} else if (val == ML_TX_MAIN_LINK_OFF) {
> +		DRM_ERROR("Link training failed, link is off\n");
> +		return;
> +	}
> 
>  	/* config video parameters */
>  	ti_sn_bridge_set_video_timings(pdata);
Reviewed-by: Sandeep Panda <spanda@codeaurora.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 6/7] drm/bridge: ti-sn65dsi86: Poll for training complete

[Sean Paul]

On Tue, Aug 14, 2018 at 04:56:27PM +0530, spanda@codeaurora.org wrote:
> On 2018-08-14 03:00, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > Instead of just waiting 20ms for training to complete, actually poll the
> > status to ensure training is finished.
> > 
> > Changes in v3:
> > - Added to the set
> > 
> > Cc: Sandeep Panda <spanda@codeaurora.org>
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index f02bdedae1e5e..d3e27e52ea759 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -493,7 +493,17 @@ static void ti_sn_bridge_enable(struct drm_bridge
> > *bridge)
> > 
> >  	/* Semi auto link training mode */
> >  	regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
> > -	msleep(20); /* 20ms delay recommended by spec */
> > +	ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
> > +				       val == ML_TX_MAIN_LINK_OFF ||
> > +				       val == ML_TX_NORMAL_MODE, 1000,
> > +				       500 * 1000);
> > +	if (ret) {
> > +		DRM_ERROR("Training complete polling failed (%d)\n", ret);
> > +		return;
> > +	} else if (val == ML_TX_MAIN_LINK_OFF) {
> > +		DRM_ERROR("Link training failed, link is off\n");
> > +		return;
> > +	}
> > 
> >  	/* config video parameters */
> >  	ti_sn_bridge_set_video_timings(pdata);
> Reviewed-by: Sandeep Panda <spanda@codeaurora.org>

I've pushed 2-6 to drm-misc-next with your review.

Thanks,

Sean

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

This patch adds a 70ms mystery delay to the bridge driver in enable.
By experimentation, it seems like it can go anywhere up until we
initiate semi-auto link training. If we don't have the delay, link
training fails.

I tried to root cause this as best I could, but neither the datasheet
for the panel nor the bridge mention a delay of this magnitude in their
timing requirements. So for now, add the mystery delay until someone
figures out a better fix.

Changes in v3:
- Added to the set

Cc: Sandeep Panda <spanda@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index d3e27e52ea759..f8a931cf3665e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -458,6 +458,18 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge)
 	unsigned int val;
 	int ret;
 
+	/*
+	 * FIXME:
+	 * This 70ms was found necessary by experimentation. If it's not
+	 * present, link training fails. It seems like it can go anywhere from
+	 * pre_enable() up to semi-auto link training initiation below.
+	 *
+	 * Neither the datasheet for the bridge nor the panel tested mention a
+	 * delay of this magnitude in the timing requirements. So for now, add
+	 * the mystery delay until someone figures out a better fix.
+	 */
+	msleep(70);
+
 	/* DSI_A lane config */
 	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
 	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> This patch adds a 70ms mystery delay to the bridge driver in enable.
> By experimentation, it seems like it can go anywhere up until we
> initiate semi-auto link training. If we don't have the delay, link
> training fails.
> 
> I tried to root cause this as best I could, but neither the datasheet
> for the panel nor the bridge mention a delay of this magnitude in their
> timing requirements. So for now, add the mystery delay until someone
> figures out a better fix.
> 
> Changes in v3:
> - Added to the set
> 
> Cc: Sandeep Panda <spanda@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index d3e27e52ea759..f8a931cf3665e 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -458,6 +458,18 @@ static void ti_sn_bridge_enable(struct drm_bridge 
> *bridge)
>  	unsigned int val;
>  	int ret;
> 
> +	/*
> +	 * FIXME:
> +	 * This 70ms was found necessary by experimentation. If it's not
> +	 * present, link training fails. It seems like it can go anywhere 
> from
> +	 * pre_enable() up to semi-auto link training initiation below.
> +	 *
> +	 * Neither the datasheet for the bridge nor the panel tested mention 
> a
> +	 * delay of this magnitude in the timing requirements. So for now, 
> add
> +	 * the mystery delay until someone figures out a better fix.
> +	 */

Is this newly found issue? Specific to any board config?

> +	msleep(70);
> +
>  	/* DSI_A lane config */
>  	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
>  	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()

[Sean Paul]

On Tue, Aug 14, 2018 at 04:59:31PM +0530, spanda@codeaurora.org wrote:
> On 2018-08-14 03:00, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > This patch adds a 70ms mystery delay to the bridge driver in enable.
> > By experimentation, it seems like it can go anywhere up until we
> > initiate semi-auto link training. If we don't have the delay, link
> > training fails.
> > 
> > I tried to root cause this as best I could, but neither the datasheet
> > for the panel nor the bridge mention a delay of this magnitude in their
> > timing requirements. So for now, add the mystery delay until someone
> > figures out a better fix.
> > 
> > Changes in v3:
> > - Added to the set
> > 
> > Cc: Sandeep Panda <spanda@codeaurora.org>
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index d3e27e52ea759..f8a931cf3665e 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -458,6 +458,18 @@ static void ti_sn_bridge_enable(struct drm_bridge
> > *bridge)
> >  	unsigned int val;
> >  	int ret;
> > 
> > +	/*
> > +	 * FIXME:
> > +	 * This 70ms was found necessary by experimentation. If it's not
> > +	 * present, link training fails. It seems like it can go anywhere from
> > +	 * pre_enable() up to semi-auto link training initiation below.
> > +	 *
> > +	 * Neither the datasheet for the bridge nor the panel tested mention a
> > +	 * delay of this magnitude in the timing requirements. So for now, add
> > +	 * the mystery delay until someone figures out a better fix.
> > +	 */
> 
> Is this newly found issue? Specific to any board config?

It's specific to cheza. This was found with swboyd changed the panel regulator
from always-on/boot-on to toggle.

I've pushed the tag "mtp-testing-0813" to dpu-staging so you can play around
with it. Without this delay, link training fails.

Sean

> 
> > +	msleep(70);
> > +
> >  	/* DSI_A lane config */
> >  	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
> >  	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()

[Sean Paul]

On Tue, Aug 14, 2018 at 10:00:33AM -0400, Sean Paul wrote:
> On Tue, Aug 14, 2018 at 04:59:31PM +0530, spanda@codeaurora.org wrote:
> > On 2018-08-14 03:00, Sean Paul wrote:
> > > From: Sean Paul <seanpaul@chromium.org>
> > > 
> > > This patch adds a 70ms mystery delay to the bridge driver in enable.
> > > By experimentation, it seems like it can go anywhere up until we
> > > initiate semi-auto link training. If we don't have the delay, link
> > > training fails.
> > > 
> > > I tried to root cause this as best I could, but neither the datasheet
> > > for the panel nor the bridge mention a delay of this magnitude in their
> > > timing requirements. So for now, add the mystery delay until someone
> > > figures out a better fix.
> > > 
> > > Changes in v3:
> > > - Added to the set
> > > 
> > > Cc: Sandeep Panda <spanda@codeaurora.org>
> > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > > ---
> > >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > index d3e27e52ea759..f8a931cf3665e 100644
> > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > @@ -458,6 +458,18 @@ static void ti_sn_bridge_enable(struct drm_bridge
> > > *bridge)
> > >  	unsigned int val;
> > >  	int ret;
> > > 
> > > +	/*
> > > +	 * FIXME:
> > > +	 * This 70ms was found necessary by experimentation. If it's not
> > > +	 * present, link training fails. It seems like it can go anywhere from
> > > +	 * pre_enable() up to semi-auto link training initiation below.
> > > +	 *
> > > +	 * Neither the datasheet for the bridge nor the panel tested mention a
> > > +	 * delay of this magnitude in the timing requirements. So for now, add
> > > +	 * the mystery delay until someone figures out a better fix.
> > > +	 */
> > 
> > Is this newly found issue? Specific to any board config?
> 
> It's specific to cheza. 

Well, I suppose I shouldn't say that since I've not tried the bridge on another
board. I should say, it reproduces on cheza.

Sean

> This was found with swboyd changed the panel regulator
> from always-on/boot-on to toggle.
> 
> I've pushed the tag "mtp-testing-0813" to dpu-staging so you can play around
> with it. Without this delay, link training fails.
> 
> Sean
> 
> > 
> > > +	msleep(70);
> > > +
> > >  	/* DSI_A lane config */
> > >  	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
> > >  	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()

[Sean Paul]

On Tue, Aug 14, 2018 at 10:01:45AM -0400, Sean Paul wrote:
> On Tue, Aug 14, 2018 at 10:00:33AM -0400, Sean Paul wrote:
> > On Tue, Aug 14, 2018 at 04:59:31PM +0530, spanda@codeaurora.org wrote:
> > > On 2018-08-14 03:00, Sean Paul wrote:
> > > > From: Sean Paul <seanpaul@chromium.org>
> > > > 
> > > > This patch adds a 70ms mystery delay to the bridge driver in enable.
> > > > By experimentation, it seems like it can go anywhere up until we
> > > > initiate semi-auto link training. If we don't have the delay, link
> > > > training fails.
> > > > 
> > > > I tried to root cause this as best I could, but neither the datasheet
> > > > for the panel nor the bridge mention a delay of this magnitude in their
> > > > timing requirements. So for now, add the mystery delay until someone
> > > > figures out a better fix.
> > > > 
> > > > Changes in v3:
> > > > - Added to the set
> > > > 
> > > > Cc: Sandeep Panda <spanda@codeaurora.org>
> > > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > > > ---
> > > >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 ++++++++++++
> > > >  1 file changed, 12 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > index d3e27e52ea759..f8a931cf3665e 100644
> > > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > > > @@ -458,6 +458,18 @@ static void ti_sn_bridge_enable(struct drm_bridge
> > > > *bridge)
> > > >  	unsigned int val;
> > > >  	int ret;
> > > > 
> > > > +	/*
> > > > +	 * FIXME:
> > > > +	 * This 70ms was found necessary by experimentation. If it's not
> > > > +	 * present, link training fails. It seems like it can go anywhere from
> > > > +	 * pre_enable() up to semi-auto link training initiation below.
> > > > +	 *
> > > > +	 * Neither the datasheet for the bridge nor the panel tested mention a
> > > > +	 * delay of this magnitude in the timing requirements. So for now, add
> > > > +	 * the mystery delay until someone figures out a better fix.
> > > > +	 */
> > > 
> > > Is this newly found issue? Specific to any board config?
> > 
> > It's specific to cheza. 
> 
> Well, I suppose I shouldn't say that since I've not tried the bridge on another
> board. I should say, it reproduces on cheza.

Ping. Sandeep: any more feedback?

Sean

> 
> Sean
> 
> > This was found with swboyd changed the panel regulator
> > from always-on/boot-on to toggle.
> > 
> > I've pushed the tag "mtp-testing-0813" to dpu-staging so you can play around
> > with it. Without this delay, link training fails.
> > 
> > Sean
> > 
> > > 
> > > > +	msleep(70);
> > > > +
> > > >  	/* DSI_A lane config */
> > > >  	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
> > > >  	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
> > 
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()

[spanda]

On 2018-08-23 19:55, Sean Paul wrote:
> On Tue, Aug 14, 2018 at 10:01:45AM -0400, Sean Paul wrote:
>> On Tue, Aug 14, 2018 at 10:00:33AM -0400, Sean Paul wrote:
>> > On Tue, Aug 14, 2018 at 04:59:31PM +0530, spanda@codeaurora.org wrote:
>> > > On 2018-08-14 03:00, Sean Paul wrote:
>> > > > From: Sean Paul <seanpaul@chromium.org>
>> > > >
>> > > > This patch adds a 70ms mystery delay to the bridge driver in enable.
>> > > > By experimentation, it seems like it can go anywhere up until we
>> > > > initiate semi-auto link training. If we don't have the delay, link
>> > > > training fails.
>> > > >
>> > > > I tried to root cause this as best I could, but neither the datasheet
>> > > > for the panel nor the bridge mention a delay of this magnitude in their
>> > > > timing requirements. So for now, add the mystery delay until someone
>> > > > figures out a better fix.
>> > > >
>> > > > Changes in v3:
>> > > > - Added to the set
>> > > >
>> > > > Cc: Sandeep Panda <spanda@codeaurora.org>
>> > > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> > > > ---
>> > > >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 ++++++++++++
>> > > >  1 file changed, 12 insertions(+)
>> > > >
>> > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> > > > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> > > > index d3e27e52ea759..f8a931cf3665e 100644
>> > > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> > > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> > > > @@ -458,6 +458,18 @@ static void ti_sn_bridge_enable(struct drm_bridge
>> > > > *bridge)
>> > > >  	unsigned int val;
>> > > >  	int ret;
>> > > >
>> > > > +	/*
>> > > > +	 * FIXME:
>> > > > +	 * This 70ms was found necessary by experimentation. If it's not
>> > > > +	 * present, link training fails. It seems like it can go anywhere from
>> > > > +	 * pre_enable() up to semi-auto link training initiation below.
>> > > > +	 *
>> > > > +	 * Neither the datasheet for the bridge nor the panel tested mention a
>> > > > +	 * delay of this magnitude in the timing requirements. So for now, add
>> > > > +	 * the mystery delay until someone figures out a better fix.
>> > > > +	 */
>> > >
>> > > Is this newly found issue? Specific to any board config?
>> >
>> > It's specific to cheza.
>> 
>> Well, I suppose I shouldn't say that since I've not tried the bridge 
>> on another
>> board. I should say, it reproduces on cheza.
> 
> Ping. Sandeep: any more feedback?
> 
> Sean
> 
>> 
>> Sean
>> 
>> > This was found with swboyd changed the panel regulator
>> > from always-on/boot-on to toggle.
>> >
>> > I've pushed the tag "mtp-testing-0813" to dpu-staging so you can play around
>> > with it. Without this delay, link training fails.
>> >
>> > Sean
>> >
>> > >
>> > > > +	msleep(70);
>> > > > +
>> > > >  	/* DSI_A lane config */
>> > > >  	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
>> > > >  	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
>> >
>> > --
>> > Sean Paul, Software Engineer, Google / Chromium OS
>> 
>> --
>> Sean Paul, Software Engineer, Google / Chromium OS

No more comments from my side.

Sandeep
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()

[Sean Paul]

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

On Sun, Aug 26, 2018, 1:44 AM <spanda@codeaurora.org> wrote:

> On 2018-08-23 19:55, Sean Paul wrote:
> > On Tue, Aug 14, 2018 at 10:01:45AM -0400, Sean Paul wrote:
> >> On Tue, Aug 14, 2018 at 10:00:33AM -0400, Sean Paul wrote:
> >> > On Tue, Aug 14, 2018 at 04:59:31PM +0530, spanda@codeaurora.org
> wrote:
> >> > > On 2018-08-14 03:00, Sean Paul wrote:
> >> > > > From: Sean Paul <seanpaul@chromium.org>
> >> > > >
> >> > > > This patch adds a 70ms mystery delay to the bridge driver in
> enable.
> >> > > > By experimentation, it seems like it can go anywhere up until we
> >> > > > initiate semi-auto link training. If we don't have the delay, link
> >> > > > training fails.
> >> > > >
> >> > > > I tried to root cause this as best I could, but neither the
> datasheet
> >> > > > for the panel nor the bridge mention a delay of this magnitude in
> their
> >> > > > timing requirements. So for now, add the mystery delay until
> someone
> >> > > > figures out a better fix.
> >> > > >
> >> > > > Changes in v3:
> >> > > > - Added to the set
> >> > > >
> >> > > > Cc: Sandeep Panda <spanda@codeaurora.org>
> >> > > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> >> > > > ---
> >> > > >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 ++++++++++++
> >> > > >  1 file changed, 12 insertions(+)
> >> > > >
> >> > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >> > > > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >> > > > index d3e27e52ea759..f8a931cf3665e 100644
> >> > > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >> > > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >> > > > @@ -458,6 +458,18 @@ static void ti_sn_bridge_enable(struct
> drm_bridge
> >> > > > *bridge)
> >> > > >        unsigned int val;
> >> > > >        int ret;
> >> > > >
> >> > > > +      /*
> >> > > > +       * FIXME:
> >> > > > +       * This 70ms was found necessary by experimentation. If
> it's not
> >> > > > +       * present, link training fails. It seems like it can go
> anywhere from
> >> > > > +       * pre_enable() up to semi-auto link training initiation
> below.
> >> > > > +       *
> >> > > > +       * Neither the datasheet for the bridge nor the panel
> tested mention a
> >> > > > +       * delay of this magnitude in the timing requirements. So
> for now, add
> >> > > > +       * the mystery delay until someone figures out a better
> fix.
> >> > > > +       */
> >> > >
> >> > > Is this newly found issue? Specific to any board config?
> >> >
> >> > It's specific to cheza.
> >>
> >> Well, I suppose I shouldn't say that since I've not tried the bridge
> >> on another
> >> board. I should say, it reproduces on cheza.
> >
> > Ping. Sandeep: any more feedback?
> >
> > Sean
> >
> >>
> >> Sean
> >>
> >> > This was found with swboyd changed the panel regulator
> >> > from always-on/boot-on to toggle.
> >> >
> >> > I've pushed the tag "mtp-testing-0813" to dpu-staging so you can play
> around
> >> > with it. Without this delay, link training fails.
> >> >
> >> > Sean
> >> >
> >> > >
> >> > > > +      msleep(70);
> >> > > > +
> >> > > >        /* DSI_A lane config */
> >> > > >        val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
> >> > > >        regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
> >> >
> >> > --
> >> > Sean Paul, Software Engineer, Google / Chromium OS
> >>
> >> --
> >> Sean Paul, Software Engineer, Google / Chromium OS
>
> No more comments from my side.
>

So, should I find another reviewer?

Sean


> Sandeep
>

[-- Attachment #1.2: Type: text/html, Size: 5700 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> This patch adds a 70ms mystery delay to the bridge driver in enable.
> By experimentation, it seems like it can go anywhere up until we
> initiate semi-auto link training. If we don't have the delay, link
> training fails.
> 
> I tried to root cause this as best I could, but neither the datasheet
> for the panel nor the bridge mention a delay of this magnitude in their
> timing requirements. So for now, add the mystery delay until someone
> figures out a better fix.
> 
> Changes in v3:
> - Added to the set
> 
> Cc: Sandeep Panda <spanda@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index d3e27e52ea759..f8a931cf3665e 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -458,6 +458,18 @@ static void ti_sn_bridge_enable(struct drm_bridge 
> *bridge)
>  	unsigned int val;
>  	int ret;
> 
> +	/*
> +	 * FIXME:
> +	 * This 70ms was found necessary by experimentation. If it's not
> +	 * present, link training fails. It seems like it can go anywhere 
> from
> +	 * pre_enable() up to semi-auto link training initiation below.
> +	 *
> +	 * Neither the datasheet for the bridge nor the panel tested mention 
> a
> +	 * delay of this magnitude in the timing requirements. So for now, 
> add
> +	 * the mystery delay until someone figures out a better fix.
> +	 */
> +	msleep(70);
> +
>  	/* DSI_A lane config */
>  	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
>  	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,

Reviewed-by: Sandeep Panda <spanda@codeaurora.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()

[Sean Paul]

On Mon, Aug 27, 2018 at 07:40:05AM +0530, spanda@codeaurora.org wrote:
> On 2018-08-14 03:00, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > This patch adds a 70ms mystery delay to the bridge driver in enable.
> > By experimentation, it seems like it can go anywhere up until we
> > initiate semi-auto link training. If we don't have the delay, link
> > training fails.
> > 
> > I tried to root cause this as best I could, but neither the datasheet
> > for the panel nor the bridge mention a delay of this magnitude in their
> > timing requirements. So for now, add the mystery delay until someone
> > figures out a better fix.
> > 
> > Changes in v3:
> > - Added to the set
> > 
> > Cc: Sandeep Panda <spanda@codeaurora.org>
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index d3e27e52ea759..f8a931cf3665e 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -458,6 +458,18 @@ static void ti_sn_bridge_enable(struct drm_bridge
> > *bridge)
> >  	unsigned int val;
> >  	int ret;
> > 
> > +	/*
> > +	 * FIXME:
> > +	 * This 70ms was found necessary by experimentation. If it's not
> > +	 * present, link training fails. It seems like it can go anywhere from
> > +	 * pre_enable() up to semi-auto link training initiation below.
> > +	 *
> > +	 * Neither the datasheet for the bridge nor the panel tested mention a
> > +	 * delay of this magnitude in the timing requirements. So for now, add
> > +	 * the mystery delay until someone figures out a better fix.
> > +	 */
> > +	msleep(70);
> > +
> >  	/* DSI_A lane config */
> >  	val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
> >  	regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
> 
> Reviewed-by: Sandeep Panda <spanda@codeaurora.org>

Pushed to -misc-next, thanks for your reviews.

Sean

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel