* [PATCH v3 0/7] drm/bridge: ti-sn65dsi86: Fix bridge for non always-on regulators
@ 2018-08-13 21:30 Sean Paul
2018-08-13 21:30 ` [PATCH v3 1/7] drm/panel: simple: tv123wam: Add unprepare delay Sean Paul
` (6 more replies)
0 siblings, 7 replies; 24+ messages in thread
From: Sean Paul @ 2018-08-13 21:30 UTC (permalink / raw)
To: dri-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 1/7] drm/panel: simple: tv123wam: Add unprepare delay
2018-08-13 21:30 [PATCH v3 0/7] drm/bridge: ti-sn65dsi86: Fix bridge for non always-on regulators Sean Paul
@ 2018-08-13 21:30 ` Sean Paul
2018-08-14 10:40 ` spanda
2018-08-13 21:30 ` [PATCH v3 2/7] drm/bridge: ti-sn65dsi86: Fixup register names Sean Paul
` (5 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: Sean Paul @ 2018-08-13 21:30 UTC (permalink / raw)
To: dri-devel; +Cc: Thierry Reding, Sandeep Panda
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 2/7] drm/bridge: ti-sn65dsi86: Fixup register names
2018-08-13 21:30 [PATCH v3 0/7] drm/bridge: ti-sn65dsi86: Fix bridge for non always-on regulators Sean Paul
2018-08-13 21:30 ` [PATCH v3 1/7] drm/panel: simple: tv123wam: Add unprepare delay Sean Paul
@ 2018-08-13 21:30 ` Sean Paul
2018-08-14 10:50 ` spanda
2018-08-13 21:30 ` [PATCH v3 3/7] drm/bridge: ti-sn65dsi86: Implement AUX channel Sean Paul
` (4 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: Sean Paul @ 2018-08-13 21:30 UTC (permalink / raw)
To: dri-devel; +Cc: Sandeep Panda, Laurent Pinchart
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 3/7] drm/bridge: ti-sn65dsi86: Implement AUX channel
2018-08-13 21:30 [PATCH v3 0/7] drm/bridge: ti-sn65dsi86: Fix bridge for non always-on regulators Sean Paul
2018-08-13 21:30 ` [PATCH v3 1/7] drm/panel: simple: tv123wam: Add unprepare delay Sean Paul
2018-08-13 21:30 ` [PATCH v3 2/7] drm/bridge: ti-sn65dsi86: Fixup register names Sean Paul
@ 2018-08-13 21:30 ` Sean Paul
2018-08-14 11:06 ` spanda
2018-08-13 21:30 ` [PATCH v3 4/7] drm/bridge: ti-sn65dsi86: Move panel_prepare() to pre_enable() Sean Paul
` (3 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: Sean Paul @ 2018-08-13 21:30 UTC (permalink / raw)
To: dri-devel; +Cc: Sandeep Panda, Laurent Pinchart
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 4/7] drm/bridge: ti-sn65dsi86: Move panel_prepare() to pre_enable()
2018-08-13 21:30 [PATCH v3 0/7] drm/bridge: ti-sn65dsi86: Fix bridge for non always-on regulators Sean Paul
` (2 preceding siblings ...)
2018-08-13 21:30 ` [PATCH v3 3/7] drm/bridge: ti-sn65dsi86: Implement AUX channel Sean Paul
@ 2018-08-13 21:30 ` Sean Paul
2018-08-14 11:09 ` spanda
2018-08-13 21:30 ` [PATCH v3 5/7] drm/bridge: ti-sn65dsi86: Poll for DP PLL Lock Sean Paul
` (2 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: Sean Paul @ 2018-08-13 21:30 UTC (permalink / raw)
To: dri-devel; +Cc: Sandeep Panda, Laurent Pinchart
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 5/7] drm/bridge: ti-sn65dsi86: Poll for DP PLL Lock
2018-08-13 21:30 [PATCH v3 0/7] drm/bridge: ti-sn65dsi86: Fix bridge for non always-on regulators Sean Paul
` (3 preceding siblings ...)
2018-08-13 21:30 ` [PATCH v3 4/7] drm/bridge: ti-sn65dsi86: Move panel_prepare() to pre_enable() Sean Paul
@ 2018-08-13 21:30 ` Sean Paul
2018-08-14 11:10 ` spanda
2018-08-13 21:30 ` [PATCH v3 6/7] drm/bridge: ti-sn65dsi86: Poll for training complete Sean Paul
2018-08-13 21:30 ` [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable() Sean Paul
6 siblings, 1 reply; 24+ messages in thread
From: Sean Paul @ 2018-08-13 21:30 UTC (permalink / raw)
To: dri-devel; +Cc: Sandeep Panda, Laurent Pinchart
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 6/7] drm/bridge: ti-sn65dsi86: Poll for training complete
2018-08-13 21:30 [PATCH v3 0/7] drm/bridge: ti-sn65dsi86: Fix bridge for non always-on regulators Sean Paul
` (4 preceding siblings ...)
2018-08-13 21:30 ` [PATCH v3 5/7] drm/bridge: ti-sn65dsi86: Poll for DP PLL Lock Sean Paul
@ 2018-08-13 21:30 ` Sean Paul
2018-08-14 11:26 ` spanda
2018-08-13 21:30 ` [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable() Sean Paul
6 siblings, 1 reply; 24+ messages in thread
From: Sean Paul @ 2018-08-13 21:30 UTC (permalink / raw)
To: dri-devel; +Cc: Sandeep Panda, Laurent Pinchart
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()
2018-08-13 21:30 [PATCH v3 0/7] drm/bridge: ti-sn65dsi86: Fix bridge for non always-on regulators Sean Paul
` (5 preceding siblings ...)
2018-08-13 21:30 ` [PATCH v3 6/7] drm/bridge: ti-sn65dsi86: Poll for training complete Sean Paul
@ 2018-08-13 21:30 ` Sean Paul
2018-08-14 11:29 ` spanda
2018-08-27 2:10 ` spanda
6 siblings, 2 replies; 24+ messages in thread
From: Sean Paul @ 2018-08-13 21:30 UTC (permalink / raw)
To: dri-devel; +Cc: Sandeep Panda, Laurent Pinchart
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/7] drm/panel: simple: tv123wam: Add unprepare delay
2018-08-13 21:30 ` [PATCH v3 1/7] drm/panel: simple: tv123wam: Add unprepare delay Sean Paul
@ 2018-08-14 10:40 ` spanda
2018-08-14 17:55 ` Sean Paul
0 siblings, 1 reply; 24+ messages in thread
From: spanda @ 2018-08-14 10:40 UTC (permalink / raw)
To: Sean Paul; +Cc: Thierry Reding, dri-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 2/7] drm/bridge: ti-sn65dsi86: Fixup register names
2018-08-13 21:30 ` [PATCH v3 2/7] drm/bridge: ti-sn65dsi86: Fixup register names Sean Paul
@ 2018-08-14 10:50 ` spanda
0 siblings, 0 replies; 24+ messages in thread
From: spanda @ 2018-08-14 10:50 UTC (permalink / raw)
To: Sean Paul; +Cc: Laurent Pinchart, dri-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/7] drm/bridge: ti-sn65dsi86: Implement AUX channel
2018-08-13 21:30 ` [PATCH v3 3/7] drm/bridge: ti-sn65dsi86: Implement AUX channel Sean Paul
@ 2018-08-14 11:06 ` spanda
0 siblings, 0 replies; 24+ messages in thread
From: spanda @ 2018-08-14 11:06 UTC (permalink / raw)
To: Sean Paul; +Cc: Laurent Pinchart, dri-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 4/7] drm/bridge: ti-sn65dsi86: Move panel_prepare() to pre_enable()
2018-08-13 21:30 ` [PATCH v3 4/7] drm/bridge: ti-sn65dsi86: Move panel_prepare() to pre_enable() Sean Paul
@ 2018-08-14 11:09 ` spanda
0 siblings, 0 replies; 24+ messages in thread
From: spanda @ 2018-08-14 11:09 UTC (permalink / raw)
To: Sean Paul; +Cc: Laurent Pinchart, dri-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 5/7] drm/bridge: ti-sn65dsi86: Poll for DP PLL Lock
2018-08-13 21:30 ` [PATCH v3 5/7] drm/bridge: ti-sn65dsi86: Poll for DP PLL Lock Sean Paul
@ 2018-08-14 11:10 ` spanda
0 siblings, 0 replies; 24+ messages in thread
From: spanda @ 2018-08-14 11:10 UTC (permalink / raw)
To: Sean Paul; +Cc: Laurent Pinchart, dri-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 6/7] drm/bridge: ti-sn65dsi86: Poll for training complete
2018-08-13 21:30 ` [PATCH v3 6/7] drm/bridge: ti-sn65dsi86: Poll for training complete Sean Paul
@ 2018-08-14 11:26 ` spanda
2018-08-14 18:15 ` Sean Paul
0 siblings, 1 reply; 24+ messages in thread
From: spanda @ 2018-08-14 11:26 UTC (permalink / raw)
To: Sean Paul; +Cc: Laurent Pinchart, dri-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()
2018-08-13 21:30 ` [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable() Sean Paul
@ 2018-08-14 11:29 ` spanda
2018-08-14 14:00 ` Sean Paul
2018-08-27 2:10 ` spanda
1 sibling, 1 reply; 24+ messages in thread
From: spanda @ 2018-08-14 11:29 UTC (permalink / raw)
To: Sean Paul; +Cc: Laurent Pinchart, dri-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()
2018-08-14 11:29 ` spanda
@ 2018-08-14 14:00 ` Sean Paul
2018-08-14 14:01 ` Sean Paul
0 siblings, 1 reply; 24+ messages in thread
From: Sean Paul @ 2018-08-14 14:00 UTC (permalink / raw)
To: spanda; +Cc: dri-devel, Sean Paul, Laurent Pinchart, 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()
2018-08-14 14:00 ` Sean Paul
@ 2018-08-14 14:01 ` Sean Paul
2018-08-23 14:25 ` Sean Paul
0 siblings, 1 reply; 24+ messages in thread
From: Sean Paul @ 2018-08-14 14:01 UTC (permalink / raw)
To: spanda; +Cc: dri-devel, Sean Paul, Laurent Pinchart, 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/7] drm/panel: simple: tv123wam: Add unprepare delay
2018-08-14 10:40 ` spanda
@ 2018-08-14 17:55 ` Sean Paul
0 siblings, 0 replies; 24+ messages in thread
From: Sean Paul @ 2018-08-14 17:55 UTC (permalink / raw)
To: spanda; +Cc: Sean Paul, Thierry Reding, Sean Paul, dri-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 6/7] drm/bridge: ti-sn65dsi86: Poll for training complete
2018-08-14 11:26 ` spanda
@ 2018-08-14 18:15 ` Sean Paul
0 siblings, 0 replies; 24+ messages in thread
From: Sean Paul @ 2018-08-14 18:15 UTC (permalink / raw)
To: spanda; +Cc: dri-devel, Sean Paul, Laurent Pinchart, 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()
2018-08-14 14:01 ` Sean Paul
@ 2018-08-23 14:25 ` Sean Paul
2018-08-26 5:44 ` spanda
0 siblings, 1 reply; 24+ messages in thread
From: Sean Paul @ 2018-08-23 14:25 UTC (permalink / raw)
To: spanda; +Cc: dri-devel, Sean Paul, Laurent Pinchart, 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()
2018-08-23 14:25 ` Sean Paul
@ 2018-08-26 5:44 ` spanda
2018-08-26 11:37 ` Sean Paul
0 siblings, 1 reply; 24+ messages in thread
From: spanda @ 2018-08-26 5:44 UTC (permalink / raw)
To: Sean Paul; +Cc: Sean Paul, Laurent Pinchart, dri-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()
2018-08-26 5:44 ` spanda
@ 2018-08-26 11:37 ` Sean Paul
0 siblings, 0 replies; 24+ messages in thread
From: Sean Paul @ 2018-08-26 11:37 UTC (permalink / raw)
To: Sandeep Panda; +Cc: Sean Paul, Laurent Pinchart, Maling list - DRI developers
[-- 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()
2018-08-13 21:30 ` [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable() Sean Paul
2018-08-14 11:29 ` spanda
@ 2018-08-27 2:10 ` spanda
2018-08-27 15:28 ` Sean Paul
1 sibling, 1 reply; 24+ messages in thread
From: spanda @ 2018-08-27 2:10 UTC (permalink / raw)
To: Sean Paul; +Cc: Sean Paul, Laurent Pinchart, dri-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable()
2018-08-27 2:10 ` spanda
@ 2018-08-27 15:28 ` Sean Paul
0 siblings, 0 replies; 24+ messages in thread
From: Sean Paul @ 2018-08-27 15:28 UTC (permalink / raw)
To: spanda; +Cc: dri-devel, Sean Paul, Laurent Pinchart, 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
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2018-08-27 15:28 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-13 21:30 [PATCH v3 0/7] drm/bridge: ti-sn65dsi86: Fix bridge for non always-on regulators Sean Paul
2018-08-13 21:30 ` [PATCH v3 1/7] drm/panel: simple: tv123wam: Add unprepare delay Sean Paul
2018-08-14 10:40 ` spanda
2018-08-14 17:55 ` Sean Paul
2018-08-13 21:30 ` [PATCH v3 2/7] drm/bridge: ti-sn65dsi86: Fixup register names Sean Paul
2018-08-14 10:50 ` spanda
2018-08-13 21:30 ` [PATCH v3 3/7] drm/bridge: ti-sn65dsi86: Implement AUX channel Sean Paul
2018-08-14 11:06 ` spanda
2018-08-13 21:30 ` [PATCH v3 4/7] drm/bridge: ti-sn65dsi86: Move panel_prepare() to pre_enable() Sean Paul
2018-08-14 11:09 ` spanda
2018-08-13 21:30 ` [PATCH v3 5/7] drm/bridge: ti-sn65dsi86: Poll for DP PLL Lock Sean Paul
2018-08-14 11:10 ` spanda
2018-08-13 21:30 ` [PATCH v3 6/7] drm/bridge: ti-sn65dsi86: Poll for training complete Sean Paul
2018-08-14 11:26 ` spanda
2018-08-14 18:15 ` Sean Paul
2018-08-13 21:30 ` [PATCH v3 7/7] drm/bridge: ti-sn65dsi86: Add mystery delay to enable() Sean Paul
2018-08-14 11:29 ` spanda
2018-08-14 14:00 ` Sean Paul
2018-08-14 14:01 ` Sean Paul
2018-08-23 14:25 ` Sean Paul
2018-08-26 5:44 ` spanda
2018-08-26 11:37 ` Sean Paul
2018-08-27 2:10 ` spanda
2018-08-27 15:28 ` Sean Paul
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.