This was hand-rolled in the first version, and will surely be useful as we expand the driver to support more varied use cases.
Cc: Sandeep Panda spanda@codeaurora.org Signed-off-by: Sean Paul seanpaul@chromium.org --- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 107 ++++++++++++++++++++++++-- 1 file changed, 100 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index 1b6e8b72be58..50aa8c3c39fc 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> @@ -29,12 +31,15 @@ #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_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 #define SN_AUX_LENGTH_REG 0x77 #define SN_AUX_CMD_REG 0x78 +#define AUX_CMD_SEND 0x01 +#define AUX_CMD_REQ(x) (x << 4) +#define SN_AUX_RDATA_REG(x) (0x79 + x) #define SN_ML_TX_MODE_REG 0x96 /* video config specific registers */ #define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG 0x20 @@ -64,6 +69,9 @@ #define SN_DP_DATA_RATE_OFFSET 5 #define SN_SYNC_POLARITY_OFFSET 7
+/* Matches DP_AUX_MAX_PAYLOAD_BYTES (for now) */ +#define SN_AUX_MAX_PAYLOAD_BYTES 16 + #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) @@ -76,6 +84,7 @@ 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; @@ -473,12 +482,7 @@ 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); + drm_dp_dpcd_writeb(&pdata->aux, DP_EDP_CONFIGURATION_SET, 0); usleep_range(10000, 10500); /* 10ms delay recommended by spec */
/* Semi auto link training mode */ @@ -527,6 +531,90 @@ 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 bool ti_sn_cmd_done(struct ti_sn_bridge *pdata) +{ + int ret; + unsigned int val; + + ret = regmap_read(pdata->regmap, SN_AUX_CMD_REG, &val); + WARN_ON(ret); + return ret || !(val & AUX_CMD_SEND); +} + +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; + bool cmd_done; + 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) & 0xFF); + 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 = readx_poll_timeout(ti_sn_cmd_done, pdata, cmd_done, cmd_done, + 200, 50 * 1000); + if (ret) + return ret; + + if (request == DP_AUX_NATIVE_READ || request == DP_AUX_I2C_READ) { + unsigned int val; + + ret = regmap_read(pdata->regmap, SN_AUX_LENGTH_REG, &val); + if (ret) + return ret; + else if (val != msg->size) + return -ENXIO; + + 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; @@ -606,6 +694,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;
On Wed, Aug 8, 2018, 5:53 PM Sean Paul seanpaul@chromium.org wrote:
Just realized I put drm/panel in the subject, should be drm/bridge.
Sean
This was hand-rolled in the first version, and will surely be useful as
we expand the driver to support more varied use cases.
Cc: Sandeep Panda spanda@codeaurora.org Signed-off-by: Sean Paul seanpaul@chromium.org
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 107 ++++++++++++++++++++++++-- 1 file changed, 100 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index 1b6e8b72be58..50aa8c3c39fc 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> @@ -29,12 +31,15 @@ #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_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 #define SN_AUX_LENGTH_REG 0x77 #define SN_AUX_CMD_REG 0x78 +#define AUX_CMD_SEND 0x01 +#define AUX_CMD_REQ(x) (x << 4) +#define SN_AUX_RDATA_REG(x) (0x79 + x) #define SN_ML_TX_MODE_REG 0x96 /* video config specific registers */ #define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG 0x20 @@ -64,6 +69,9 @@ #define SN_DP_DATA_RATE_OFFSET 5 #define SN_SYNC_POLARITY_OFFSET 7
+/* Matches DP_AUX_MAX_PAYLOAD_BYTES (for now) */ +#define SN_AUX_MAX_PAYLOAD_BYTES 16
#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) @@ -76,6 +84,7 @@ 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;@@ -473,12 +482,7 @@ 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);
drm_dp_dpcd_writeb(&pdata->aux, DP_EDP_CONFIGURATION_SET, 0); usleep_range(10000, 10500); /* 10ms delay recommended by spec */ /* Semi auto link training mode */@@ -527,6 +531,90 @@ 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 bool ti_sn_cmd_done(struct ti_sn_bridge *pdata) +{
int ret;unsigned int val;ret = regmap_read(pdata->regmap, SN_AUX_CMD_REG, &val);WARN_ON(ret);return ret || !(val & AUX_CMD_SEND);+}
+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;bool cmd_done;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) & 0xFF);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 = readx_poll_timeout(ti_sn_cmd_done, pdata, cmd_done, cmd_done,200, 50 * 1000);if (ret)return ret;if (request == DP_AUX_NATIVE_READ || request == DP_AUX_I2C_READ) {unsigned int val;ret = regmap_read(pdata->regmap, SN_AUX_LENGTH_REG, &val);if (ret)return ret;else if (val != msg->size)return -ENXIO;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; @@ -606,6 +694,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
Hi Sean,
Thanks for your patch. I also made this similar change as part of my PSR support changes (yet to be posted for review). But since this patch is posted now, i will pick this for my PSR changes.
On 2018-08-09 03:23, Sean Paul wrote:
This was hand-rolled in the first version, and will surely be useful as we expand the driver to support more varied use cases.
Cc: Sandeep Panda spanda@codeaurora.org Signed-off-by: Sean Paul seanpaul@chromium.org
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 107 ++++++++++++++++++++++++-- 1 file changed, 100 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index 1b6e8b72be58..50aa8c3c39fc 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> @@ -29,12 +31,15 @@ #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_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 #define SN_AUX_LENGTH_REG 0x77 #define SN_AUX_CMD_REG 0x78 +#define AUX_CMD_SEND 0x01 +#define AUX_CMD_REQ(x) (x << 4) +#define SN_AUX_RDATA_REG(x) (0x79 + x) #define SN_ML_TX_MODE_REG 0x96 /* video config specific registers */ #define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG 0x20 @@ -64,6 +69,9 @@ #define SN_DP_DATA_RATE_OFFSET 5 #define SN_SYNC_POLARITY_OFFSET 7
+/* Matches DP_AUX_MAX_PAYLOAD_BYTES (for now) */ +#define SN_AUX_MAX_PAYLOAD_BYTES 16
#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) @@ -76,6 +84,7 @@ 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;
@@ -473,12 +482,7 @@ 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);
- drm_dp_dpcd_writeb(&pdata->aux, DP_EDP_CONFIGURATION_SET, 0);
I think the last argument here should be 0x1 instead of 0. or better put it as DP_ALTERNATE_SCRAMBLER_RESET_ENABLE.
usleep_range(10000, 10500); /* 10ms delay recommended by spec */
We can remove this usleep now, as you are already ensuring in the newly added function that aux transaction is successful.
/* Semi auto link training mode */ @@ -527,6 +531,90 @@ 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 bool ti_sn_cmd_done(struct ti_sn_bridge *pdata) +{
- int ret;
- unsigned int val;
- ret = regmap_read(pdata->regmap, SN_AUX_CMD_REG, &val);
Can we read back register offset 0xF4, instead of 0x78, to check if AUX transaction was success or any error has occurred.
- WARN_ON(ret);
- return ret || !(val & AUX_CMD_SEND);
+}
+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;
- bool cmd_done;
- 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);
This regmap_write is not needed since you will be writing it again towards the end of this function.
break;- default:
return -EINVAL;- }
- regmap_write(pdata->regmap, SN_AUX_ADDR_19_16_REG,
(msg->address >> 16) & 0xFF);
Since we are only concerned about 4 bits (bit 16-19) better to & with 0xF instead of 0xFF
- 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 = readx_poll_timeout(ti_sn_cmd_done, pdata, cmd_done, cmd_done,
200, 50 * 1000);- if (ret)
return ret;- if (request == DP_AUX_NATIVE_READ || request == DP_AUX_I2C_READ) {
unsigned int val;ret = regmap_read(pdata->regmap, SN_AUX_LENGTH_REG, &val);if (ret)return ret;else if (val != msg->size)return -ENXIO;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; @@ -606,6 +694,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;
On Fri, Aug 10, 2018 at 10:22:58AM +0530, spanda@codeaurora.org wrote:
Hi Sean,
Thanks for your patch. I also made this similar change as part of my PSR support changes (yet to be posted for review). But since this patch is posted now, i will pick this for my PSR changes.
Hi Sandeep, Thanks so much for your review. I didn't realize you were working on PSR, that's cool.
I've added a few comments below, and I'll upload a v2 shortly.
On 2018-08-09 03:23, Sean Paul wrote:
This was hand-rolled in the first version, and will surely be useful as we expand the driver to support more varied use cases.
Cc: Sandeep Panda spanda@codeaurora.org Signed-off-by: Sean Paul seanpaul@chromium.org
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 107 ++++++++++++++++++++++++-- 1 file changed, 100 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index 1b6e8b72be58..50aa8c3c39fc 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> @@ -29,12 +31,15 @@ #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_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 #define SN_AUX_LENGTH_REG 0x77 #define SN_AUX_CMD_REG 0x78 +#define AUX_CMD_SEND 0x01 +#define AUX_CMD_REQ(x) (x << 4) +#define SN_AUX_RDATA_REG(x) (0x79 + x) #define SN_ML_TX_MODE_REG 0x96 /* video config specific registers */ #define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG 0x20 @@ -64,6 +69,9 @@ #define SN_DP_DATA_RATE_OFFSET 5 #define SN_SYNC_POLARITY_OFFSET 7
+/* Matches DP_AUX_MAX_PAYLOAD_BYTES (for now) */ +#define SN_AUX_MAX_PAYLOAD_BYTES 16
#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) @@ -76,6 +84,7 @@ 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;
@@ -473,12 +482,7 @@ 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);
- drm_dp_dpcd_writeb(&pdata->aux, DP_EDP_CONFIGURATION_SET, 0);
I think the last argument here should be 0x1 instead of 0. or better put it as DP_ALTERNATE_SCRAMBLER_RESET_ENABLE.
Ah shoot, good catch, not sure what I was thinking.
usleep_range(10000, 10500); /* 10ms delay recommended by spec */
We can remove this usleep now, as you are already ensuring in the newly added function that aux transaction is successful.
/* Semi auto link training mode */ @@ -527,6 +531,90 @@ 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 bool ti_sn_cmd_done(struct ti_sn_bridge *pdata) +{
- int ret;
- unsigned int val;
- ret = regmap_read(pdata->regmap, SN_AUX_CMD_REG, &val);
Can we read back register offset 0xF4, instead of 0x78, to check if AUX transaction was success or any error has occurred.
I think we should continue to read the SEND bit here. The interrupt in F4 is really just tracking SEND, so best to check at the source.
I will add a check for some of the error bits in 0xF4 in transfer after the poll to ensure everything succeeded.
- WARN_ON(ret);
- return ret || !(val & AUX_CMD_SEND);
+}
+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;
- bool cmd_done;
- 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);This regmap_write is not needed since you will be writing it again towards the end of this function.
Yeah, I noticed you were doing this earlier. The programming guide in 8.4.5.2.1 (gosh those sub-sections are atrocious) suggests updating CMD_REG before programming ADDR and LENGTH, so I've done that out of an abundance of caution. We need this switch anyways, so it's not hurting us.
break;- default:
return -EINVAL;- }
- regmap_write(pdata->regmap, SN_AUX_ADDR_19_16_REG,
(msg->address >> 16) & 0xFF);Since we are only concerned about 4 bits (bit 16-19) better to & with 0xF instead of 0xFF
Makes sense, will do.
- 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 = readx_poll_timeout(ti_sn_cmd_done, pdata, cmd_done, cmd_done,
200, 50 * 1000);- if (ret)
return ret;- if (request == DP_AUX_NATIVE_READ || request == DP_AUX_I2C_READ) {
unsigned int val;ret = regmap_read(pdata->regmap, SN_AUX_LENGTH_REG, &val);if (ret)return ret;else if (val != msg->size)return -ENXIO;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; @@ -606,6 +694,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;
dri-devel@lists.freedesktop.org
-
Sean Paul -
spanda＠codeaurora.org