[PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Stephen Boyd]

This patch series cleans up the DDC code a little bit so that
it is more efficient time wise and supports grabbing the EDID
of the eDP panel over the aux channel. I timed this on a board
I have on my desk and it takes about 20ms to grab the EDID out
of the panel and make sure it is valid.

The first two patches seem less controversial so I stuck them at
the beginning. The third patch does the EDID reading and caches
it so we don't have to keep grabbing it over and over again. And
finally the last patch updates the reply field so that short
reads and nacks over the channel are reflected properly instead of
treating them as some sort of error that can't be discerned.

Stephen Boyd (4):
  drm/bridge: ti-sn65dsi86: Combine register accesses in
    ti_sn_aux_transfer()
  drm/bridge: ti-sn65dsi86: Make polling a busy loop
  drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
  drm/bridge: ti-sn65dsi86: Update reply on aux failures

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 108 ++++++++++++++++++--------
 1 file changed, 75 insertions(+), 33 deletions(-)

Cc: Douglas Anderson <dianders@chromium.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Cc: Sean Paul <seanpaul@chromium.org>

base-commit: 3650b228f83adda7e5ee532e2b90429c03f7b9ec
-- 
Sent by a computer, using git, on the internet

[PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Stephen Boyd]

This patch series cleans up the DDC code a little bit so that
it is more efficient time wise and supports grabbing the EDID
of the eDP panel over the aux channel. I timed this on a board
I have on my desk and it takes about 20ms to grab the EDID out
of the panel and make sure it is valid.

The first two patches seem less controversial so I stuck them at
the beginning. The third patch does the EDID reading and caches
it so we don't have to keep grabbing it over and over again. And
finally the last patch updates the reply field so that short
reads and nacks over the channel are reflected properly instead of
treating them as some sort of error that can't be discerned.

Stephen Boyd (4):
  drm/bridge: ti-sn65dsi86: Combine register accesses in
    ti_sn_aux_transfer()
  drm/bridge: ti-sn65dsi86: Make polling a busy loop
  drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
  drm/bridge: ti-sn65dsi86: Update reply on aux failures

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 108 ++++++++++++++++++--------
 1 file changed, 75 insertions(+), 33 deletions(-)

Cc: Douglas Anderson <dianders@chromium.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Cc: Sean Paul <seanpaul@chromium.org>

base-commit: 3650b228f83adda7e5ee532e2b90429c03f7b9ec
-- 
Sent by a computer, using git, on the internet

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

[PATCH v2 1/4] drm/bridge: ti-sn65dsi86: Combine register accesses in ti_sn_aux_transfer()

[Stephen Boyd]

These register reads and writes are sometimes directly next to each
other in the register address space. Let's use regmap bulk read/write
APIs to get the data with one transfer instead of multiple i2c
transfers. This helps cut down on the number of transfers in the case of
something like reading an EDID where we read in blocks of 16 bytes at a
time and the last for loop here is sending an i2c transfer for each of
those 16 bytes, one at a time. Ouch!

Changes in v2:
 - Combined AUX_CMD register write

Cc: Douglas Anderson <dianders@chromium.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 52 ++++++++++++---------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index ecdf9b01340f..a1ebfa95088c 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -17,6 +17,8 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
+#include <asm/unaligned.h>
+
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
@@ -72,6 +74,7 @@
 #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_ADDR_MASK			GENMASK(19, 0)
 #define SN_AUX_LENGTH_REG			0x77
 #define SN_AUX_CMD_REG				0x78
 #define  AUX_CMD_SEND				BIT(0)
@@ -841,11 +844,13 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
 	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;
+	u8 *buf = msg->buffer;
+	unsigned int len = msg->size;
 	unsigned int val;
-	int ret, i;
+	int ret;
+	u8 reg_buf[SN_AUX_CMD_REG + 1 - SN_AUX_ADDR_19_16_REG];
 
-	if (msg->size > SN_AUX_MAX_PAYLOAD_BYTES)
+	if (len > SN_AUX_MAX_PAYLOAD_BYTES)
 		return -EINVAL;
 
 	switch (request) {
@@ -853,25 +858,20 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
 	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);
+	BUILD_BUG_ON(sizeof(reg_buf) < sizeof(__be32));
+	put_unaligned_be32((msg->address & SN_AUX_ADDR_MASK) << 8 | len,
+			   reg_buf);
+	reg_buf[SN_AUX_CMD_REG - SN_AUX_ADDR_19_16_REG] = request_val;
+	regmap_bulk_write(pdata->regmap, SN_AUX_ADDR_19_16_REG, reg_buf,
+			  ARRAY_SIZE(reg_buf));
 
-	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]);
-	}
+	if (request == DP_AUX_NATIVE_WRITE || request == DP_AUX_I2C_WRITE)
+		regmap_bulk_write(pdata->regmap, SN_AUX_WDATA_REG(0), buf, len);
 
 	/* Clear old status bits before start so we don't get confused */
 	regmap_write(pdata->regmap, SN_AUX_CMD_STATUS_REG,
@@ -895,21 +895,15 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
 		 || (val & AUX_IRQ_STATUS_AUX_SHORT))
 		return -ENXIO;
 
-	if (request == DP_AUX_NATIVE_WRITE || request == DP_AUX_I2C_WRITE)
-		return msg->size;
+	if (request == DP_AUX_NATIVE_WRITE || request == DP_AUX_I2C_WRITE ||
+	    len == 0)
+		return len;
 
-	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);
-	}
+	ret = regmap_bulk_read(pdata->regmap, SN_AUX_RDATA_REG(0), buf, len);
+	if (ret)
+		return ret;
 
-	return msg->size;
+	return len;
 }
 
 static int ti_sn_bridge_parse_dsi_host(struct ti_sn_bridge *pdata)
-- 
Sent by a computer, using git, on the internet

[PATCH v2 1/4] drm/bridge: ti-sn65dsi86: Combine register accesses in ti_sn_aux_transfer()

[Stephen Boyd]

These register reads and writes are sometimes directly next to each
other in the register address space. Let's use regmap bulk read/write
APIs to get the data with one transfer instead of multiple i2c
transfers. This helps cut down on the number of transfers in the case of
something like reading an EDID where we read in blocks of 16 bytes at a
time and the last for loop here is sending an i2c transfer for each of
those 16 bytes, one at a time. Ouch!

Changes in v2:
 - Combined AUX_CMD register write

Cc: Douglas Anderson <dianders@chromium.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 52 ++++++++++++---------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index ecdf9b01340f..a1ebfa95088c 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -17,6 +17,8 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
+#include <asm/unaligned.h>
+
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
@@ -72,6 +74,7 @@
 #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_ADDR_MASK			GENMASK(19, 0)
 #define SN_AUX_LENGTH_REG			0x77
 #define SN_AUX_CMD_REG				0x78
 #define  AUX_CMD_SEND				BIT(0)
@@ -841,11 +844,13 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
 	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;
+	u8 *buf = msg->buffer;
+	unsigned int len = msg->size;
 	unsigned int val;
-	int ret, i;
+	int ret;
+	u8 reg_buf[SN_AUX_CMD_REG + 1 - SN_AUX_ADDR_19_16_REG];
 
-	if (msg->size > SN_AUX_MAX_PAYLOAD_BYTES)
+	if (len > SN_AUX_MAX_PAYLOAD_BYTES)
 		return -EINVAL;
 
 	switch (request) {
@@ -853,25 +858,20 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
 	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);
+	BUILD_BUG_ON(sizeof(reg_buf) < sizeof(__be32));
+	put_unaligned_be32((msg->address & SN_AUX_ADDR_MASK) << 8 | len,
+			   reg_buf);
+	reg_buf[SN_AUX_CMD_REG - SN_AUX_ADDR_19_16_REG] = request_val;
+	regmap_bulk_write(pdata->regmap, SN_AUX_ADDR_19_16_REG, reg_buf,
+			  ARRAY_SIZE(reg_buf));
 
-	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]);
-	}
+	if (request == DP_AUX_NATIVE_WRITE || request == DP_AUX_I2C_WRITE)
+		regmap_bulk_write(pdata->regmap, SN_AUX_WDATA_REG(0), buf, len);
 
 	/* Clear old status bits before start so we don't get confused */
 	regmap_write(pdata->regmap, SN_AUX_CMD_STATUS_REG,
@@ -895,21 +895,15 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
 		 || (val & AUX_IRQ_STATUS_AUX_SHORT))
 		return -ENXIO;
 
-	if (request == DP_AUX_NATIVE_WRITE || request == DP_AUX_I2C_WRITE)
-		return msg->size;
+	if (request == DP_AUX_NATIVE_WRITE || request == DP_AUX_I2C_WRITE ||
+	    len == 0)
+		return len;
 
-	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);
-	}
+	ret = regmap_bulk_read(pdata->regmap, SN_AUX_RDATA_REG(0), buf, len);
+	if (ret)
+		return ret;
 
-	return msg->size;
+	return len;
 }
 
 static int ti_sn_bridge_parse_dsi_host(struct ti_sn_bridge *pdata)
-- 
Sent by a computer, using git, on the internet

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

Re: [PATCH v2 1/4] drm/bridge: ti-sn65dsi86: Combine register accesses in ti_sn_aux_transfer()

[Doug Anderson]

Hi,

On Thu, Oct 29, 2020 at 6:17 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> These register reads and writes are sometimes directly next to each
> other in the register address space. Let's use regmap bulk read/write
> APIs to get the data with one transfer instead of multiple i2c
> transfers. This helps cut down on the number of transfers in the case of
> something like reading an EDID where we read in blocks of 16 bytes at a
> time and the last for loop here is sending an i2c transfer for each of
> those 16 bytes, one at a time. Ouch!
>
> Changes in v2:
>  - Combined AUX_CMD register write

The change from v1 to v2 makes me slightly nervous, though I guess
it's fine.  Specifically, all the examples in the datasheet show
programming the CMD before the ADDR and LEN.  This change will make it
programmed after.  Since there's a separate START bit I guess it's OK,
though.  Nothing in the datasheet explicitly says that the order in
the examples is the only order that will work...

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v2 1/4] drm/bridge: ti-sn65dsi86: Combine register accesses in ti_sn_aux_transfer()

[Doug Anderson]

Hi,

On Thu, Oct 29, 2020 at 6:17 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> These register reads and writes are sometimes directly next to each
> other in the register address space. Let's use regmap bulk read/write
> APIs to get the data with one transfer instead of multiple i2c
> transfers. This helps cut down on the number of transfers in the case of
> something like reading an EDID where we read in blocks of 16 bytes at a
> time and the last for loop here is sending an i2c transfer for each of
> those 16 bytes, one at a time. Ouch!
>
> Changes in v2:
>  - Combined AUX_CMD register write

The change from v1 to v2 makes me slightly nervous, though I guess
it's fine.  Specifically, all the examples in the datasheet show
programming the CMD before the ADDR and LEN.  This change will make it
programmed after.  Since there's a separate START bit I guess it's OK,
though.  Nothing in the datasheet explicitly says that the order in
the examples is the only order that will work...

Reviewed-by: Douglas Anderson <dianders@chromium.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/4] drm/bridge: ti-sn65dsi86: Combine register accesses in ti_sn_aux_transfer()

[Stephen Boyd]

Quoting Doug Anderson (2020-11-02 08:18:47)
> Hi,
> 
> On Thu, Oct 29, 2020 at 6:17 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > These register reads and writes are sometimes directly next to each
> > other in the register address space. Let's use regmap bulk read/write
> > APIs to get the data with one transfer instead of multiple i2c
> > transfers. This helps cut down on the number of transfers in the case of
> > something like reading an EDID where we read in blocks of 16 bytes at a
> > time and the last for loop here is sending an i2c transfer for each of
> > those 16 bytes, one at a time. Ouch!
> >
> > Changes in v2:
> >  - Combined AUX_CMD register write
> 
> The change from v1 to v2 makes me slightly nervous, though I guess
> it's fine.  Specifically, all the examples in the datasheet show
> programming the CMD before the ADDR and LEN.  This change will make it
> programmed after.  Since there's a separate START bit I guess it's OK,
> though.  Nothing in the datasheet explicitly says that the order in
> the examples is the only order that will work...

Hmmm now that you mention it the SEND bit is explicitly being cleared in
the programming sequence by being there at the start. If I want to
combine that with the adjacent register writes then I should make sure
that the SEND bit is cleared at the beginning. Otherwise the hardware
may be in the middle of a transaction if the previous transaction is
still running, i.e. a timeout where the SEND bit never cleared.

I think we should go back to the previous patch I had here. Combining
this register write is wrong. If anything, we should clear the SEND bit
on a timeout and make sure during probe that this bit is clear and then
drop the programming of this register from this function entirely. That
would reduce the sequence by one register, but is more complicated vs.
just making sure it has the clear bit here to begin with.

> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Thanks, but I'll send another round picking up acks and such and your
previous review tag on the v1 of this patch.

Re: [PATCH v2 1/4] drm/bridge: ti-sn65dsi86: Combine register accesses in ti_sn_aux_transfer()

[Stephen Boyd]

Quoting Doug Anderson (2020-11-02 08:18:47)
> Hi,
> 
> On Thu, Oct 29, 2020 at 6:17 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > These register reads and writes are sometimes directly next to each
> > other in the register address space. Let's use regmap bulk read/write
> > APIs to get the data with one transfer instead of multiple i2c
> > transfers. This helps cut down on the number of transfers in the case of
> > something like reading an EDID where we read in blocks of 16 bytes at a
> > time and the last for loop here is sending an i2c transfer for each of
> > those 16 bytes, one at a time. Ouch!
> >
> > Changes in v2:
> >  - Combined AUX_CMD register write
> 
> The change from v1 to v2 makes me slightly nervous, though I guess
> it's fine.  Specifically, all the examples in the datasheet show
> programming the CMD before the ADDR and LEN.  This change will make it
> programmed after.  Since there's a separate START bit I guess it's OK,
> though.  Nothing in the datasheet explicitly says that the order in
> the examples is the only order that will work...

Hmmm now that you mention it the SEND bit is explicitly being cleared in
the programming sequence by being there at the start. If I want to
combine that with the adjacent register writes then I should make sure
that the SEND bit is cleared at the beginning. Otherwise the hardware
may be in the middle of a transaction if the previous transaction is
still running, i.e. a timeout where the SEND bit never cleared.

I think we should go back to the previous patch I had here. Combining
this register write is wrong. If anything, we should clear the SEND bit
on a timeout and make sure during probe that this bit is clear and then
drop the programming of this register from this function entirely. That
would reduce the sequence by one register, but is more complicated vs.
just making sure it has the clear bit here to begin with.

> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Thanks, but I'll send another round picking up acks and such and your
previous review tag on the v1 of this patch.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/4] drm/bridge: ti-sn65dsi86: Make polling a busy loop

[Stephen Boyd]

There's no reason we need to wait here to poll a register over i2c. The
i2c bus is inherently slow and delays are practically part of the
protocol because we have to wait for the device to respond to any
request for a register. Let's rely on the sleeping of the i2c controller
instead of adding any sort of delay here in the bridge driver.

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@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 a1ebfa95088c..c77f46a21aae 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -881,9 +881,9 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
 
 	regmap_write(pdata->regmap, SN_AUX_CMD_REG, request_val | AUX_CMD_SEND);
 
+	/* Zero delay loop because i2c transactions are slow already */
 	ret = regmap_read_poll_timeout(pdata->regmap, SN_AUX_CMD_REG, val,
-				       !(val & AUX_CMD_SEND), 200,
-				       50 * 1000);
+				       !(val & AUX_CMD_SEND), 0, 50 * 1000);
 	if (ret)
 		return ret;
 
-- 
Sent by a computer, using git, on the internet

[PATCH v2 2/4] drm/bridge: ti-sn65dsi86: Make polling a busy loop

[Stephen Boyd]

There's no reason we need to wait here to poll a register over i2c. The
i2c bus is inherently slow and delays are practically part of the
protocol because we have to wait for the device to respond to any
request for a register. Let's rely on the sleeping of the i2c controller
instead of adding any sort of delay here in the bridge driver.

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@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 a1ebfa95088c..c77f46a21aae 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -881,9 +881,9 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
 
 	regmap_write(pdata->regmap, SN_AUX_CMD_REG, request_val | AUX_CMD_SEND);
 
+	/* Zero delay loop because i2c transactions are slow already */
 	ret = regmap_read_poll_timeout(pdata->regmap, SN_AUX_CMD_REG, val,
-				       !(val & AUX_CMD_SEND), 200,
-				       50 * 1000);
+				       !(val & AUX_CMD_SEND), 0, 50 * 1000);
 	if (ret)
 		return ret;
 
-- 
Sent by a computer, using git, on the internet

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

[PATCH v2 3/4] drm/bridge: ti-sn65dsi86: Read EDID blob over DDC

[Stephen Boyd]

Use the DDC connection to read the EDID from the eDP panel instead of
relying on the panel to tell us the modes.

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index c77f46a21aae..f86934fd6cc8 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -119,6 +119,7 @@
  * @debugfs:      Used for managing our debugfs.
  * @host_node:    Remote DSI node.
  * @dsi:          Our MIPI DSI source.
+ * @edid:         Detected EDID of eDP panel.
  * @refclk:       Our reference clock.
  * @panel:        Our panel.
  * @enable_gpio:  The GPIO we toggle to enable the bridge.
@@ -144,6 +145,7 @@ struct ti_sn_bridge {
 	struct drm_bridge		bridge;
 	struct drm_connector		connector;
 	struct dentry			*debugfs;
+	struct edid			*edid;
 	struct device_node		*host_node;
 	struct mipi_dsi_device		*dsi;
 	struct clk			*refclk;
@@ -265,6 +267,23 @@ connector_to_ti_sn_bridge(struct drm_connector *connector)
 static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
 {
 	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
+	struct edid *edid = pdata->edid;
+	int num, ret;
+
+	if (!edid) {
+		pm_runtime_get_sync(pdata->dev);
+		edid = pdata->edid = drm_get_edid(connector, &pdata->aux.ddc);
+		pm_runtime_put(pdata->dev);
+	}
+
+	if (edid && drm_edid_is_valid(edid)) {
+		ret = drm_connector_update_edid_property(connector, edid);
+		if (!ret) {
+			num = drm_add_edid_modes(connector, edid);
+			if (num)
+				return num;
+		}
+	}
 
 	return drm_panel_get_modes(pdata->panel, connector);
 }
@@ -1245,6 +1264,7 @@ static int ti_sn_bridge_remove(struct i2c_client *client)
 	if (!pdata)
 		return -EINVAL;
 
+	kfree(pdata->edid);
 	ti_sn_debugfs_remove(pdata);
 
 	of_node_put(pdata->host_node);
-- 
Sent by a computer, using git, on the internet

[PATCH v2 3/4] drm/bridge: ti-sn65dsi86: Read EDID blob over DDC

[Stephen Boyd]

Use the DDC connection to read the EDID from the eDP panel instead of
relying on the panel to tell us the modes.

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index c77f46a21aae..f86934fd6cc8 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -119,6 +119,7 @@
  * @debugfs:      Used for managing our debugfs.
  * @host_node:    Remote DSI node.
  * @dsi:          Our MIPI DSI source.
+ * @edid:         Detected EDID of eDP panel.
  * @refclk:       Our reference clock.
  * @panel:        Our panel.
  * @enable_gpio:  The GPIO we toggle to enable the bridge.
@@ -144,6 +145,7 @@ struct ti_sn_bridge {
 	struct drm_bridge		bridge;
 	struct drm_connector		connector;
 	struct dentry			*debugfs;
+	struct edid			*edid;
 	struct device_node		*host_node;
 	struct mipi_dsi_device		*dsi;
 	struct clk			*refclk;
@@ -265,6 +267,23 @@ connector_to_ti_sn_bridge(struct drm_connector *connector)
 static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
 {
 	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
+	struct edid *edid = pdata->edid;
+	int num, ret;
+
+	if (!edid) {
+		pm_runtime_get_sync(pdata->dev);
+		edid = pdata->edid = drm_get_edid(connector, &pdata->aux.ddc);
+		pm_runtime_put(pdata->dev);
+	}
+
+	if (edid && drm_edid_is_valid(edid)) {
+		ret = drm_connector_update_edid_property(connector, edid);
+		if (!ret) {
+			num = drm_add_edid_modes(connector, edid);
+			if (num)
+				return num;
+		}
+	}
 
 	return drm_panel_get_modes(pdata->panel, connector);
 }
@@ -1245,6 +1264,7 @@ static int ti_sn_bridge_remove(struct i2c_client *client)
 	if (!pdata)
 		return -EINVAL;
 
+	kfree(pdata->edid);
 	ti_sn_debugfs_remove(pdata);
 
 	of_node_put(pdata->host_node);
-- 
Sent by a computer, using git, on the internet

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

Re: [PATCH v2 3/4] drm/bridge: ti-sn65dsi86: Read EDID blob over DDC

[Laurent Pinchart]

Hi Stephen,

Thank you for the patch.

On Thu, Oct 29, 2020 at 06:17:37PM -0700, Stephen Boyd wrote:
> Use the DDC connection to read the EDID from the eDP panel instead of
> relying on the panel to tell us the modes.
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec@siol.net>
> Cc: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index c77f46a21aae..f86934fd6cc8 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -119,6 +119,7 @@
>   * @debugfs:      Used for managing our debugfs.
>   * @host_node:    Remote DSI node.
>   * @dsi:          Our MIPI DSI source.
> + * @edid:         Detected EDID of eDP panel.
>   * @refclk:       Our reference clock.
>   * @panel:        Our panel.
>   * @enable_gpio:  The GPIO we toggle to enable the bridge.
> @@ -144,6 +145,7 @@ struct ti_sn_bridge {
>  	struct drm_bridge		bridge;
>  	struct drm_connector		connector;
>  	struct dentry			*debugfs;
> +	struct edid			*edid;
>  	struct device_node		*host_node;
>  	struct mipi_dsi_device		*dsi;
>  	struct clk			*refclk;
> @@ -265,6 +267,23 @@ connector_to_ti_sn_bridge(struct drm_connector *connector)
>  static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
>  {
>  	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
> +	struct edid *edid = pdata->edid;
> +	int num, ret;
> +
> +	if (!edid) {
> +		pm_runtime_get_sync(pdata->dev);
> +		edid = pdata->edid = drm_get_edid(connector, &pdata->aux.ddc);
> +		pm_runtime_put(pdata->dev);
> +	}

Do we need to cache the EDID ? It seems like something that should be
done by the DRM core (well, caching modes in that case), not by
individual bridge drivers.

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +	if (edid && drm_edid_is_valid(edid)) {
> +		ret = drm_connector_update_edid_property(connector, edid);
> +		if (!ret) {
> +			num = drm_add_edid_modes(connector, edid);
> +			if (num)
> +				return num;
> +		}
> +	}
>  
>  	return drm_panel_get_modes(pdata->panel, connector);
>  }
> @@ -1245,6 +1264,7 @@ static int ti_sn_bridge_remove(struct i2c_client *client)
>  	if (!pdata)
>  		return -EINVAL;
>  
> +	kfree(pdata->edid);
>  	ti_sn_debugfs_remove(pdata);
>  
>  	of_node_put(pdata->host_node);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/4] drm/bridge: ti-sn65dsi86: Read EDID blob over DDC

[Laurent Pinchart]

Hi Stephen,

Thank you for the patch.

On Thu, Oct 29, 2020 at 06:17:37PM -0700, Stephen Boyd wrote:
> Use the DDC connection to read the EDID from the eDP panel instead of
> relying on the panel to tell us the modes.
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec@siol.net>
> Cc: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index c77f46a21aae..f86934fd6cc8 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -119,6 +119,7 @@
>   * @debugfs:      Used for managing our debugfs.
>   * @host_node:    Remote DSI node.
>   * @dsi:          Our MIPI DSI source.
> + * @edid:         Detected EDID of eDP panel.
>   * @refclk:       Our reference clock.
>   * @panel:        Our panel.
>   * @enable_gpio:  The GPIO we toggle to enable the bridge.
> @@ -144,6 +145,7 @@ struct ti_sn_bridge {
>  	struct drm_bridge		bridge;
>  	struct drm_connector		connector;
>  	struct dentry			*debugfs;
> +	struct edid			*edid;
>  	struct device_node		*host_node;
>  	struct mipi_dsi_device		*dsi;
>  	struct clk			*refclk;
> @@ -265,6 +267,23 @@ connector_to_ti_sn_bridge(struct drm_connector *connector)
>  static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
>  {
>  	struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
> +	struct edid *edid = pdata->edid;
> +	int num, ret;
> +
> +	if (!edid) {
> +		pm_runtime_get_sync(pdata->dev);
> +		edid = pdata->edid = drm_get_edid(connector, &pdata->aux.ddc);
> +		pm_runtime_put(pdata->dev);
> +	}

Do we need to cache the EDID ? It seems like something that should be
done by the DRM core (well, caching modes in that case), not by
individual bridge drivers.

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +	if (edid && drm_edid_is_valid(edid)) {
> +		ret = drm_connector_update_edid_property(connector, edid);
> +		if (!ret) {
> +			num = drm_add_edid_modes(connector, edid);
> +			if (num)
> +				return num;
> +		}
> +	}
>  
>  	return drm_panel_get_modes(pdata->panel, connector);
>  }
> @@ -1245,6 +1264,7 @@ static int ti_sn_bridge_remove(struct i2c_client *client)
>  	if (!pdata)
>  		return -EINVAL;
>  
> +	kfree(pdata->edid);
>  	ti_sn_debugfs_remove(pdata);
>  
>  	of_node_put(pdata->host_node);

-- 
Regards,

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

Re: [PATCH v2 3/4] drm/bridge: ti-sn65dsi86: Read EDID blob over DDC

[Doug Anderson]

Hi,

On Sun, Nov 1, 2020 at 11:21 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Stephen,
>
> Thank you for the patch.
>
> On Thu, Oct 29, 2020 at 06:17:37PM -0700, Stephen Boyd wrote:
> > Use the DDC connection to read the EDID from the eDP panel instead of
> > relying on the panel to tell us the modes.
> >
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> > Cc: Jonas Karlman <jonas@kwiboo.se>
> > Cc: Jernej Skrabec <jernej.skrabec@siol.net>
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index c77f46a21aae..f86934fd6cc8 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -119,6 +119,7 @@
> >   * @debugfs:      Used for managing our debugfs.
> >   * @host_node:    Remote DSI node.
> >   * @dsi:          Our MIPI DSI source.
> > + * @edid:         Detected EDID of eDP panel.
> >   * @refclk:       Our reference clock.
> >   * @panel:        Our panel.
> >   * @enable_gpio:  The GPIO we toggle to enable the bridge.
> > @@ -144,6 +145,7 @@ struct ti_sn_bridge {
> >       struct drm_bridge               bridge;
> >       struct drm_connector            connector;
> >       struct dentry                   *debugfs;
> > +     struct edid                     *edid;
> >       struct device_node              *host_node;
> >       struct mipi_dsi_device          *dsi;
> >       struct clk                      *refclk;
> > @@ -265,6 +267,23 @@ connector_to_ti_sn_bridge(struct drm_connector *connector)
> >  static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
> >  {
> >       struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
> > +     struct edid *edid = pdata->edid;
> > +     int num, ret;
> > +
> > +     if (!edid) {
> > +             pm_runtime_get_sync(pdata->dev);
> > +             edid = pdata->edid = drm_get_edid(connector, &pdata->aux.ddc);
> > +             pm_runtime_put(pdata->dev);
> > +     }
>
> Do we need to cache the EDID ? It seems like something that should be
> done by the DRM core (well, caching modes in that case), not by
> individual bridge drivers.

I can take the blame for the fact that it does caching, since I
requested it in early reviews.  In general boot speed is pretty
important to me and each read of the EDID take 20 ms.  There are
definitely several calls to get the EDID during a normal bootup.
Stephen did a little more digging into exactly what was causing all
these calls and can chime in, but in general until we can eliminate
the extra calls it seems like it'd be nice to keep the caching?  This
bridge chip is intended for use for eDP for internal panels, so there
should be no downside to caching.  If we can later optimize the DRM
core, we can fix this and a pre-existing driver that does the same
type of caching (analogix-anx6345.c) at the same time?

-Doug

Re: [PATCH v2 3/4] drm/bridge: ti-sn65dsi86: Read EDID blob over DDC

[Doug Anderson]

Hi,

On Sun, Nov 1, 2020 at 11:21 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Stephen,
>
> Thank you for the patch.
>
> On Thu, Oct 29, 2020 at 06:17:37PM -0700, Stephen Boyd wrote:
> > Use the DDC connection to read the EDID from the eDP panel instead of
> > relying on the panel to tell us the modes.
> >
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> > Cc: Jonas Karlman <jonas@kwiboo.se>
> > Cc: Jernej Skrabec <jernej.skrabec@siol.net>
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index c77f46a21aae..f86934fd6cc8 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -119,6 +119,7 @@
> >   * @debugfs:      Used for managing our debugfs.
> >   * @host_node:    Remote DSI node.
> >   * @dsi:          Our MIPI DSI source.
> > + * @edid:         Detected EDID of eDP panel.
> >   * @refclk:       Our reference clock.
> >   * @panel:        Our panel.
> >   * @enable_gpio:  The GPIO we toggle to enable the bridge.
> > @@ -144,6 +145,7 @@ struct ti_sn_bridge {
> >       struct drm_bridge               bridge;
> >       struct drm_connector            connector;
> >       struct dentry                   *debugfs;
> > +     struct edid                     *edid;
> >       struct device_node              *host_node;
> >       struct mipi_dsi_device          *dsi;
> >       struct clk                      *refclk;
> > @@ -265,6 +267,23 @@ connector_to_ti_sn_bridge(struct drm_connector *connector)
> >  static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
> >  {
> >       struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
> > +     struct edid *edid = pdata->edid;
> > +     int num, ret;
> > +
> > +     if (!edid) {
> > +             pm_runtime_get_sync(pdata->dev);
> > +             edid = pdata->edid = drm_get_edid(connector, &pdata->aux.ddc);
> > +             pm_runtime_put(pdata->dev);
> > +     }
>
> Do we need to cache the EDID ? It seems like something that should be
> done by the DRM core (well, caching modes in that case), not by
> individual bridge drivers.

I can take the blame for the fact that it does caching, since I
requested it in early reviews.  In general boot speed is pretty
important to me and each read of the EDID take 20 ms.  There are
definitely several calls to get the EDID during a normal bootup.
Stephen did a little more digging into exactly what was causing all
these calls and can chime in, but in general until we can eliminate
the extra calls it seems like it'd be nice to keep the caching?  This
bridge chip is intended for use for eDP for internal panels, so there
should be no downside to caching.  If we can later optimize the DRM
core, we can fix this and a pre-existing driver that does the same
type of caching (analogix-anx6345.c) at the same time?

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

Re: [PATCH v2 3/4] drm/bridge: ti-sn65dsi86: Read EDID blob over DDC

[Stephen Boyd]

Quoting Doug Anderson (2020-11-02 08:06:14)
> On Sun, Nov 1, 2020 at 11:21 AM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Thu, Oct 29, 2020 at 06:17:37PM -0700, Stephen Boyd wrote:
> > > @@ -265,6 +267,23 @@ connector_to_ti_sn_bridge(struct drm_connector *connector)
> > >  static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
> > >  {
> > >       struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
> > > +     struct edid *edid = pdata->edid;
> > > +     int num, ret;
> > > +
> > > +     if (!edid) {
> > > +             pm_runtime_get_sync(pdata->dev);
> > > +             edid = pdata->edid = drm_get_edid(connector, &pdata->aux.ddc);
> > > +             pm_runtime_put(pdata->dev);
> > > +     }
> >
> > Do we need to cache the EDID ? It seems like something that should be
> > done by the DRM core (well, caching modes in that case), not by
> > individual bridge drivers.
> 
> I can take the blame for the fact that it does caching, since I
> requested it in early reviews.  In general boot speed is pretty
> important to me and each read of the EDID take 20 ms.  There are
> definitely several calls to get the EDID during a normal bootup.
> Stephen did a little more digging into exactly what was causing all
> these calls and can chime in, 

In ChromeOS we get modes a couple times and then whenever we connect or
disconnect a DP cable for external display we also get modes. It seems
that we also run modetest at boot but I'm not sure why we do that. I
think it is to gather diagnostic data for all the EDIDs on the device at
boot so we know what all is connected.

> but in general until we can eliminate
> the extra calls it seems like it'd be nice to keep the caching?  This
> bridge chip is intended for use for eDP for internal panels, so there
> should be no downside to caching.  If we can later optimize the DRM
> core, we can fix this and a pre-existing driver that does the same
> type of caching (analogix-anx6345.c) at the same time?

I'd like to add the caching somewhere in the core (maybe the bridge
connector code?) but I don't know what the logic should be. Is it eDP
and if not hpd notify then cache all the time and if it is eDP and hpd
notify then cache once hpd notify says detected and drop cache when no
longer detected?

	if (eDP) {
		if (!hpd)
			cache();
		else if (hpd_detected()) {
			cache();
		else if (!hpd_detected()) {
			drop_cache();
		}
	}

I thought that EDID could change and HPD can be pulsed to notify that it
should be read again.

Re: [PATCH v2 3/4] drm/bridge: ti-sn65dsi86: Read EDID blob over DDC

[Stephen Boyd]

Quoting Doug Anderson (2020-11-02 08:06:14)
> On Sun, Nov 1, 2020 at 11:21 AM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Thu, Oct 29, 2020 at 06:17:37PM -0700, Stephen Boyd wrote:
> > > @@ -265,6 +267,23 @@ connector_to_ti_sn_bridge(struct drm_connector *connector)
> > >  static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
> > >  {
> > >       struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
> > > +     struct edid *edid = pdata->edid;
> > > +     int num, ret;
> > > +
> > > +     if (!edid) {
> > > +             pm_runtime_get_sync(pdata->dev);
> > > +             edid = pdata->edid = drm_get_edid(connector, &pdata->aux.ddc);
> > > +             pm_runtime_put(pdata->dev);
> > > +     }
> >
> > Do we need to cache the EDID ? It seems like something that should be
> > done by the DRM core (well, caching modes in that case), not by
> > individual bridge drivers.
> 
> I can take the blame for the fact that it does caching, since I
> requested it in early reviews.  In general boot speed is pretty
> important to me and each read of the EDID take 20 ms.  There are
> definitely several calls to get the EDID during a normal bootup.
> Stephen did a little more digging into exactly what was causing all
> these calls and can chime in, 

In ChromeOS we get modes a couple times and then whenever we connect or
disconnect a DP cable for external display we also get modes. It seems
that we also run modetest at boot but I'm not sure why we do that. I
think it is to gather diagnostic data for all the EDIDs on the device at
boot so we know what all is connected.

> but in general until we can eliminate
> the extra calls it seems like it'd be nice to keep the caching?  This
> bridge chip is intended for use for eDP for internal panels, so there
> should be no downside to caching.  If we can later optimize the DRM
> core, we can fix this and a pre-existing driver that does the same
> type of caching (analogix-anx6345.c) at the same time?

I'd like to add the caching somewhere in the core (maybe the bridge
connector code?) but I don't know what the logic should be. Is it eDP
and if not hpd notify then cache all the time and if it is eDP and hpd
notify then cache once hpd notify says detected and drop cache when no
longer detected?

	if (eDP) {
		if (!hpd)
			cache();
		else if (hpd_detected()) {
			cache();
		else if (!hpd_detected()) {
			drop_cache();
		}
	}

I thought that EDID could change and HPD can be pulsed to notify that it
should be read again.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/4] drm/bridge: ti-sn65dsi86: Read EDID blob over DDC

[Laurent Pinchart]

Hi Stephen,

On Mon, Nov 02, 2020 at 09:38:12AM -0800, Stephen Boyd wrote:
> Quoting Doug Anderson (2020-11-02 08:06:14)
> > On Sun, Nov 1, 2020 at 11:21 AM Laurent Pinchart wrote:
> > > On Thu, Oct 29, 2020 at 06:17:37PM -0700, Stephen Boyd wrote:
> > > > @@ -265,6 +267,23 @@ connector_to_ti_sn_bridge(struct drm_connector *connector)
> > > >  static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
> > > >  {
> > > >       struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
> > > > +     struct edid *edid = pdata->edid;
> > > > +     int num, ret;
> > > > +
> > > > +     if (!edid) {
> > > > +             pm_runtime_get_sync(pdata->dev);
> > > > +             edid = pdata->edid = drm_get_edid(connector, &pdata->aux.ddc);
> > > > +             pm_runtime_put(pdata->dev);
> > > > +     }
> > >
> > > Do we need to cache the EDID ? It seems like something that should be
> > > done by the DRM core (well, caching modes in that case), not by
> > > individual bridge drivers.
> > 
> > I can take the blame for the fact that it does caching, since I
> > requested it in early reviews.  In general boot speed is pretty
> > important to me and each read of the EDID take 20 ms.  There are
> > definitely several calls to get the EDID during a normal bootup.
> > Stephen did a little more digging into exactly what was causing all
> > these calls and can chime in, 
> 
> In ChromeOS we get modes a couple times and then whenever we connect or
> disconnect a DP cable for external display we also get modes. It seems
> that we also run modetest at boot but I'm not sure why we do that. I
> think it is to gather diagnostic data for all the EDIDs on the device at
> boot so we know what all is connected.
> 
> > but in general until we can eliminate
> > the extra calls it seems like it'd be nice to keep the caching?  This
> > bridge chip is intended for use for eDP for internal panels, so there
> > should be no downside to caching.  If we can later optimize the DRM
> > core, we can fix this and a pre-existing driver that does the same
> > type of caching (analogix-anx6345.c) at the same time?
> 
> I'd like to add the caching somewhere in the core (maybe the bridge
> connector code?) but I don't know what the logic should be. Is it eDP
> and if not hpd notify then cache all the time and if it is eDP and hpd
> notify then cache once hpd notify says detected and drop cache when no
> longer detected?
> 
> 	if (eDP) {
> 		if (!hpd)
> 			cache();
> 		else if (hpd_detected()) {
> 			cache();
> 		else if (!hpd_detected()) {
> 			drop_cache();
> 		}
> 	}
> 
> I thought that EDID could change and HPD can be pulsed to notify that it
> should be read again.

I think we should expose a flag tells the panel is fixed instead of
making it a special case of eDP, as other panel types could benefit from
the same mechanism. Otherwise, yes, I think it's really about caching
the EDID the first time we read it, and then reusing it. The question is
who should convert EDID to modes. At the moment bridge drivers do so,
and we're migrating to drm_bridge_connector for most cases. That would
be a candidate location to cache EDID. The DRM core would be another
one, but in that case we may need to also cache the modes.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/4] drm/bridge: ti-sn65dsi86: Read EDID blob over DDC

[Laurent Pinchart]

Hi Stephen,

On Mon, Nov 02, 2020 at 09:38:12AM -0800, Stephen Boyd wrote:
> Quoting Doug Anderson (2020-11-02 08:06:14)
> > On Sun, Nov 1, 2020 at 11:21 AM Laurent Pinchart wrote:
> > > On Thu, Oct 29, 2020 at 06:17:37PM -0700, Stephen Boyd wrote:
> > > > @@ -265,6 +267,23 @@ connector_to_ti_sn_bridge(struct drm_connector *connector)
> > > >  static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
> > > >  {
> > > >       struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector);
> > > > +     struct edid *edid = pdata->edid;
> > > > +     int num, ret;
> > > > +
> > > > +     if (!edid) {
> > > > +             pm_runtime_get_sync(pdata->dev);
> > > > +             edid = pdata->edid = drm_get_edid(connector, &pdata->aux.ddc);
> > > > +             pm_runtime_put(pdata->dev);
> > > > +     }
> > >
> > > Do we need to cache the EDID ? It seems like something that should be
> > > done by the DRM core (well, caching modes in that case), not by
> > > individual bridge drivers.
> > 
> > I can take the blame for the fact that it does caching, since I
> > requested it in early reviews.  In general boot speed is pretty
> > important to me and each read of the EDID take 20 ms.  There are
> > definitely several calls to get the EDID during a normal bootup.
> > Stephen did a little more digging into exactly what was causing all
> > these calls and can chime in, 
> 
> In ChromeOS we get modes a couple times and then whenever we connect or
> disconnect a DP cable for external display we also get modes. It seems
> that we also run modetest at boot but I'm not sure why we do that. I
> think it is to gather diagnostic data for all the EDIDs on the device at
> boot so we know what all is connected.
> 
> > but in general until we can eliminate
> > the extra calls it seems like it'd be nice to keep the caching?  This
> > bridge chip is intended for use for eDP for internal panels, so there
> > should be no downside to caching.  If we can later optimize the DRM
> > core, we can fix this and a pre-existing driver that does the same
> > type of caching (analogix-anx6345.c) at the same time?
> 
> I'd like to add the caching somewhere in the core (maybe the bridge
> connector code?) but I don't know what the logic should be. Is it eDP
> and if not hpd notify then cache all the time and if it is eDP and hpd
> notify then cache once hpd notify says detected and drop cache when no
> longer detected?
> 
> 	if (eDP) {
> 		if (!hpd)
> 			cache();
> 		else if (hpd_detected()) {
> 			cache();
> 		else if (!hpd_detected()) {
> 			drop_cache();
> 		}
> 	}
> 
> I thought that EDID could change and HPD can be pulsed to notify that it
> should be read again.

I think we should expose a flag tells the panel is fixed instead of
making it a special case of eDP, as other panel types could benefit from
the same mechanism. Otherwise, yes, I think it's really about caching
the EDID the first time we read it, and then reusing it. The question is
who should convert EDID to modes. At the moment bridge drivers do so,
and we're migrating to drm_bridge_connector for most cases. That would
be a candidate location to cache EDID. The DRM core would be another
one, but in that case we may need to also cache the modes.

-- 
Regards,

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

[PATCH v2 4/4] drm/bridge: ti-sn65dsi86: Update reply on aux failures

[Stephen Boyd]

We should be setting the drm_dp_aux_msg::reply field if a NACK or a
SHORT reply happens. Update the error bit handling logic in
ti_sn_aux_transfer() to handle these cases and notify upper layers that
such errors have happened. This helps the retry logic understand that a
timeout has happened, or to shorten the read length if the panel isn't
able to handle the longest read possible.

Note: I don't have any hardware that exhibits these code paths so this
is written based on reading the datasheet for this bridge and inspecting
the code and how this is called.

Changes in v2:
 - Handle WRITE_STATUS_UPDATE properly

Cc: Douglas Anderson <dianders@chromium.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 36 ++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index f86934fd6cc8..984ea41deca8 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -873,10 +873,16 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
 		return -EINVAL;
 
 	switch (request) {
+	case DP_AUX_I2C_WRITE_STATUS_UPDATE:
+		/* WRITE_STATUS_UPDATE only matters for request_val */
+		request &= ~DP_AUX_I2C_WRITE_STATUS_UPDATE;
+		fallthrough;
 	case DP_AUX_NATIVE_WRITE:
 	case DP_AUX_I2C_WRITE:
 	case DP_AUX_NATIVE_READ:
 	case DP_AUX_I2C_READ:
+		/* Assume it's good */
+		msg->reply = 0;
 		break;
 	default:
 		return -EINVAL;
@@ -909,10 +915,32 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
 	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 (val & AUX_IRQ_STATUS_AUX_RPLY_TOUT) {
+		/*
+		 * The hardware tried the message seven times per the DP spec
+		 * but it hit a timeout. We ignore defers here because they're
+		 * handled in hardware.
+		 */
+		return -ETIMEDOUT;
+	}
+	if (val & AUX_IRQ_STATUS_AUX_SHORT) {
+		ret = regmap_read(pdata->regmap, SN_AUX_LENGTH_REG, &len);
+		if (ret)
+			return ret;
+	} else if (val & AUX_IRQ_STATUS_NAT_I2C_FAIL) {
+		switch (request) {
+		case DP_AUX_I2C_WRITE:
+		case DP_AUX_I2C_READ:
+			msg->reply |= DP_AUX_I2C_REPLY_NACK;
+			break;
+		case DP_AUX_NATIVE_READ:
+		case DP_AUX_NATIVE_WRITE:
+			msg->reply |= DP_AUX_NATIVE_REPLY_NACK;
+			break;
+		}
+		return 0;
+	}
 
 	if (request == DP_AUX_NATIVE_WRITE || request == DP_AUX_I2C_WRITE ||
 	    len == 0)
-- 
Sent by a computer, using git, on the internet

[PATCH v2 4/4] drm/bridge: ti-sn65dsi86: Update reply on aux failures

[Stephen Boyd]

We should be setting the drm_dp_aux_msg::reply field if a NACK or a
SHORT reply happens. Update the error bit handling logic in
ti_sn_aux_transfer() to handle these cases and notify upper layers that
such errors have happened. This helps the retry logic understand that a
timeout has happened, or to shorten the read length if the panel isn't
able to handle the longest read possible.

Note: I don't have any hardware that exhibits these code paths so this
is written based on reading the datasheet for this bridge and inspecting
the code and how this is called.

Changes in v2:
 - Handle WRITE_STATUS_UPDATE properly

Cc: Douglas Anderson <dianders@chromium.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 36 ++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index f86934fd6cc8..984ea41deca8 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -873,10 +873,16 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
 		return -EINVAL;
 
 	switch (request) {
+	case DP_AUX_I2C_WRITE_STATUS_UPDATE:
+		/* WRITE_STATUS_UPDATE only matters for request_val */
+		request &= ~DP_AUX_I2C_WRITE_STATUS_UPDATE;
+		fallthrough;
 	case DP_AUX_NATIVE_WRITE:
 	case DP_AUX_I2C_WRITE:
 	case DP_AUX_NATIVE_READ:
 	case DP_AUX_I2C_READ:
+		/* Assume it's good */
+		msg->reply = 0;
 		break;
 	default:
 		return -EINVAL;
@@ -909,10 +915,32 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
 	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 (val & AUX_IRQ_STATUS_AUX_RPLY_TOUT) {
+		/*
+		 * The hardware tried the message seven times per the DP spec
+		 * but it hit a timeout. We ignore defers here because they're
+		 * handled in hardware.
+		 */
+		return -ETIMEDOUT;
+	}
+	if (val & AUX_IRQ_STATUS_AUX_SHORT) {
+		ret = regmap_read(pdata->regmap, SN_AUX_LENGTH_REG, &len);
+		if (ret)
+			return ret;
+	} else if (val & AUX_IRQ_STATUS_NAT_I2C_FAIL) {
+		switch (request) {
+		case DP_AUX_I2C_WRITE:
+		case DP_AUX_I2C_READ:
+			msg->reply |= DP_AUX_I2C_REPLY_NACK;
+			break;
+		case DP_AUX_NATIVE_READ:
+		case DP_AUX_NATIVE_WRITE:
+			msg->reply |= DP_AUX_NATIVE_REPLY_NACK;
+			break;
+		}
+		return 0;
+	}
 
 	if (request == DP_AUX_NATIVE_WRITE || request == DP_AUX_I2C_WRITE ||
 	    len == 0)
-- 
Sent by a computer, using git, on the internet

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

Re: [PATCH v2 4/4] drm/bridge: ti-sn65dsi86: Update reply on aux failures

[Doug Anderson]

Hi,

On Thu, Oct 29, 2020 at 6:17 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> We should be setting the drm_dp_aux_msg::reply field if a NACK or a
> SHORT reply happens. Update the error bit handling logic in
> ti_sn_aux_transfer() to handle these cases and notify upper layers that
> such errors have happened. This helps the retry logic understand that a
> timeout has happened, or to shorten the read length if the panel isn't
> able to handle the longest read possible.
>
> Note: I don't have any hardware that exhibits these code paths so this
> is written based on reading the datasheet for this bridge and inspecting
> the code and how this is called.
>
> Changes in v2:
>  - Handle WRITE_STATUS_UPDATE properly
>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec@siol.net>
> Cc: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 36 ++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 4 deletions(-)

This looks right to me, now.  Hopefully if/when someone ends up with
hardware that exercises these codepaths they'll at least be in a
better state and maybe they will all just work!  :-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v2 4/4] drm/bridge: ti-sn65dsi86: Update reply on aux failures

[Doug Anderson]

Hi,

On Thu, Oct 29, 2020 at 6:17 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> We should be setting the drm_dp_aux_msg::reply field if a NACK or a
> SHORT reply happens. Update the error bit handling logic in
> ti_sn_aux_transfer() to handle these cases and notify upper layers that
> such errors have happened. This helps the retry logic understand that a
> timeout has happened, or to shorten the read length if the panel isn't
> able to handle the longest read possible.
>
> Note: I don't have any hardware that exhibits these code paths so this
> is written based on reading the datasheet for this bridge and inspecting
> the code and how this is called.
>
> Changes in v2:
>  - Handle WRITE_STATUS_UPDATE properly
>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec@siol.net>
> Cc: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 36 ++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 4 deletions(-)

This looks right to me, now.  Hopefully if/when someone ends up with
hardware that exercises these codepaths they'll at least be in a
better state and maybe they will all just work!  :-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Sam Ravnborg]

Hi Stephen.

On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> This patch series cleans up the DDC code a little bit so that
> it is more efficient time wise and supports grabbing the EDID
> of the eDP panel over the aux channel. I timed this on a board
> I have on my desk and it takes about 20ms to grab the EDID out
> of the panel and make sure it is valid.
> 
> The first two patches seem less controversial so I stuck them at
> the beginning. The third patch does the EDID reading and caches
> it so we don't have to keep grabbing it over and over again. And
> finally the last patch updates the reply field so that short
> reads and nacks over the channel are reflected properly instead of
> treating them as some sort of error that can't be discerned.
> 
> Stephen Boyd (4):
>   drm/bridge: ti-sn65dsi86: Combine register accesses in
>     ti_sn_aux_transfer()
>   drm/bridge: ti-sn65dsi86: Make polling a busy loop
>   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
>   drm/bridge: ti-sn65dsi86: Update reply on aux failures

Series looks good. You can add my a-b on the full series.
Acked-by: Sam Ravnborg <sam@ravnborg.org>

I can apply after Douglas have had a look at the patches he did not r-b
yet.

Any chance we can convince you to prepare this bridge driver for use in
a chained bridge setup where the connector is created by the display
driver and uses drm_bridge_funcs?

First step wuld be to introduce the use of a panel_bridge.
Then add get_edid to drm_bridge_funcs and maybe more helpers.

Then natural final step would be to move connector creation to the
display driver - see how other uses drm_bridge_connector_init() to do so
- it is relatively simple.

Should be doable - and reach out if you need some help.

	Sam


> 
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 108 ++++++++++++++++++--------
>  1 file changed, 75 insertions(+), 33 deletions(-)
> 
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec@siol.net>
> Cc: Sean Paul <seanpaul@chromium.org>
> 
> base-commit: 3650b228f83adda7e5ee532e2b90429c03f7b9ec
> -- 
> Sent by a computer, using git, on the internet
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Sam Ravnborg]

Hi Stephen.

On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> This patch series cleans up the DDC code a little bit so that
> it is more efficient time wise and supports grabbing the EDID
> of the eDP panel over the aux channel. I timed this on a board
> I have on my desk and it takes about 20ms to grab the EDID out
> of the panel and make sure it is valid.
> 
> The first two patches seem less controversial so I stuck them at
> the beginning. The third patch does the EDID reading and caches
> it so we don't have to keep grabbing it over and over again. And
> finally the last patch updates the reply field so that short
> reads and nacks over the channel are reflected properly instead of
> treating them as some sort of error that can't be discerned.
> 
> Stephen Boyd (4):
>   drm/bridge: ti-sn65dsi86: Combine register accesses in
>     ti_sn_aux_transfer()
>   drm/bridge: ti-sn65dsi86: Make polling a busy loop
>   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
>   drm/bridge: ti-sn65dsi86: Update reply on aux failures

Series looks good. You can add my a-b on the full series.
Acked-by: Sam Ravnborg <sam@ravnborg.org>

I can apply after Douglas have had a look at the patches he did not r-b
yet.

Any chance we can convince you to prepare this bridge driver for use in
a chained bridge setup where the connector is created by the display
driver and uses drm_bridge_funcs?

First step wuld be to introduce the use of a panel_bridge.
Then add get_edid to drm_bridge_funcs and maybe more helpers.

Then natural final step would be to move connector creation to the
display driver - see how other uses drm_bridge_connector_init() to do so
- it is relatively simple.

Should be doable - and reach out if you need some help.

	Sam


> 
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 108 ++++++++++++++++++--------
>  1 file changed, 75 insertions(+), 33 deletions(-)
> 
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Jernej Skrabec <jernej.skrabec@siol.net>
> Cc: Sean Paul <seanpaul@chromium.org>
> 
> base-commit: 3650b228f83adda7e5ee532e2b90429c03f7b9ec
> -- 
> Sent by a computer, using git, on the internet
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Doug Anderson]

Hi,

On Sun, Nov 1, 2020 at 9:37 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Stephen.
>
> On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> > This patch series cleans up the DDC code a little bit so that
> > it is more efficient time wise and supports grabbing the EDID
> > of the eDP panel over the aux channel. I timed this on a board
> > I have on my desk and it takes about 20ms to grab the EDID out
> > of the panel and make sure it is valid.
> >
> > The first two patches seem less controversial so I stuck them at
> > the beginning. The third patch does the EDID reading and caches
> > it so we don't have to keep grabbing it over and over again. And
> > finally the last patch updates the reply field so that short
> > reads and nacks over the channel are reflected properly instead of
> > treating them as some sort of error that can't be discerned.
> >
> > Stephen Boyd (4):
> >   drm/bridge: ti-sn65dsi86: Combine register accesses in
> >     ti_sn_aux_transfer()
> >   drm/bridge: ti-sn65dsi86: Make polling a busy loop
> >   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
> >   drm/bridge: ti-sn65dsi86: Update reply on aux failures
>
> Series looks good. You can add my a-b on the full series.
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
> I can apply after Douglas have had a look at the patches he did not r-b
> yet.

They look fine to me now assuming that Stepehn has tested patch #1
enough that we're confident that the slight change in ordering isn't
going to mess anything up.  Laurent also had comments about caching
the EDID on patch #3.  If he feels strongly about getting rid of that,
it'll need another spin and we'll just have to suck up the small boot
time penalty until we can find a solution in the core.


> Any chance we can convince you to prepare this bridge driver for use in
> a chained bridge setup where the connector is created by the display
> driver and uses drm_bridge_funcs?
>
> First step wuld be to introduce the use of a panel_bridge.
> Then add get_edid to drm_bridge_funcs and maybe more helpers.
>
> Then natural final step would be to move connector creation to the
> display driver - see how other uses drm_bridge_connector_init() to do so
> - it is relatively simple.
>
> Should be doable - and reach out if you need some help.

At some point I think Vinod tried to prepare a patch for this and I
tried it, but it didn't just work.  I spent an hour or so poking at it
and I couldn't quite figure out why and I couldn't find enough other
examples to compare against to see what was wrong...  That was a few
months ago, though.  Maybe things are in a better shape now?

-Doug

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Doug Anderson]

Hi,

On Sun, Nov 1, 2020 at 9:37 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Stephen.
>
> On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> > This patch series cleans up the DDC code a little bit so that
> > it is more efficient time wise and supports grabbing the EDID
> > of the eDP panel over the aux channel. I timed this on a board
> > I have on my desk and it takes about 20ms to grab the EDID out
> > of the panel and make sure it is valid.
> >
> > The first two patches seem less controversial so I stuck them at
> > the beginning. The third patch does the EDID reading and caches
> > it so we don't have to keep grabbing it over and over again. And
> > finally the last patch updates the reply field so that short
> > reads and nacks over the channel are reflected properly instead of
> > treating them as some sort of error that can't be discerned.
> >
> > Stephen Boyd (4):
> >   drm/bridge: ti-sn65dsi86: Combine register accesses in
> >     ti_sn_aux_transfer()
> >   drm/bridge: ti-sn65dsi86: Make polling a busy loop
> >   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
> >   drm/bridge: ti-sn65dsi86: Update reply on aux failures
>
> Series looks good. You can add my a-b on the full series.
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
> I can apply after Douglas have had a look at the patches he did not r-b
> yet.

They look fine to me now assuming that Stepehn has tested patch #1
enough that we're confident that the slight change in ordering isn't
going to mess anything up.  Laurent also had comments about caching
the EDID on patch #3.  If he feels strongly about getting rid of that,
it'll need another spin and we'll just have to suck up the small boot
time penalty until we can find a solution in the core.


> Any chance we can convince you to prepare this bridge driver for use in
> a chained bridge setup where the connector is created by the display
> driver and uses drm_bridge_funcs?
>
> First step wuld be to introduce the use of a panel_bridge.
> Then add get_edid to drm_bridge_funcs and maybe more helpers.
>
> Then natural final step would be to move connector creation to the
> display driver - see how other uses drm_bridge_connector_init() to do so
> - it is relatively simple.
>
> Should be doable - and reach out if you need some help.

At some point I think Vinod tried to prepare a patch for this and I
tried it, but it didn't just work.  I spent an hour or so poking at it
and I couldn't quite figure out why and I couldn't find enough other
examples to compare against to see what was wrong...  That was a few
months ago, though.  Maybe things are in a better shape now?

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

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Stephen Boyd]

Quoting Doug Anderson (2020-11-02 08:37:21)
> Hi,
> 
> On Sun, Nov 1, 2020 at 9:37 AM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > Hi Stephen.
> >
> > On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> > > This patch series cleans up the DDC code a little bit so that
> > > it is more efficient time wise and supports grabbing the EDID
> > > of the eDP panel over the aux channel. I timed this on a board
> > > I have on my desk and it takes about 20ms to grab the EDID out
> > > of the panel and make sure it is valid.
> > >
> > > The first two patches seem less controversial so I stuck them at
> > > the beginning. The third patch does the EDID reading and caches
> > > it so we don't have to keep grabbing it over and over again. And
> > > finally the last patch updates the reply field so that short
> > > reads and nacks over the channel are reflected properly instead of
> > > treating them as some sort of error that can't be discerned.
> > >
> > > Stephen Boyd (4):
> > >   drm/bridge: ti-sn65dsi86: Combine register accesses in
> > >     ti_sn_aux_transfer()
> > >   drm/bridge: ti-sn65dsi86: Make polling a busy loop
> > >   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
> > >   drm/bridge: ti-sn65dsi86: Update reply on aux failures
> >
> > Series looks good. You can add my a-b on the full series.
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> >
> > I can apply after Douglas have had a look at the patches he did not r-b
> > yet.
> 
> They look fine to me now assuming that Stepehn has tested patch #1
> enough that we're confident that the slight change in ordering isn't
> going to mess anything up.

I did test it but the test isn't thorough enough to cover the timeout
case. I'll resend with v1 of this patch and pick up acks and include
Sam on To line.

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Stephen Boyd]

Quoting Doug Anderson (2020-11-02 08:37:21)
> Hi,
> 
> On Sun, Nov 1, 2020 at 9:37 AM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > Hi Stephen.
> >
> > On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> > > This patch series cleans up the DDC code a little bit so that
> > > it is more efficient time wise and supports grabbing the EDID
> > > of the eDP panel over the aux channel. I timed this on a board
> > > I have on my desk and it takes about 20ms to grab the EDID out
> > > of the panel and make sure it is valid.
> > >
> > > The first two patches seem less controversial so I stuck them at
> > > the beginning. The third patch does the EDID reading and caches
> > > it so we don't have to keep grabbing it over and over again. And
> > > finally the last patch updates the reply field so that short
> > > reads and nacks over the channel are reflected properly instead of
> > > treating them as some sort of error that can't be discerned.
> > >
> > > Stephen Boyd (4):
> > >   drm/bridge: ti-sn65dsi86: Combine register accesses in
> > >     ti_sn_aux_transfer()
> > >   drm/bridge: ti-sn65dsi86: Make polling a busy loop
> > >   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
> > >   drm/bridge: ti-sn65dsi86: Update reply on aux failures
> >
> > Series looks good. You can add my a-b on the full series.
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> >
> > I can apply after Douglas have had a look at the patches he did not r-b
> > yet.
> 
> They look fine to me now assuming that Stepehn has tested patch #1
> enough that we're confident that the slight change in ordering isn't
> going to mess anything up.

I did test it but the test isn't thorough enough to cover the timeout
case. I'll resend with v1 of this patch and pick up acks and include
Sam on To line.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Vinod Koul]

Hi,

Thanks Doug for adding me

On 02-11-20, 08:37, Doug Anderson wrote:
> > On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:

> > Any chance we can convince you to prepare this bridge driver for use in
> > a chained bridge setup where the connector is created by the display
> > driver and uses drm_bridge_funcs?
> >
> > First step wuld be to introduce the use of a panel_bridge.
> > Then add get_edid to drm_bridge_funcs and maybe more helpers.
> >
> > Then natural final step would be to move connector creation to the
> > display driver - see how other uses drm_bridge_connector_init() to do so
> > - it is relatively simple.
> >
> > Should be doable - and reach out if you need some help.

Yes it is and doable and you find this at [1], would need a rebase
though.

> At some point I think Vinod tried to prepare a patch for this and I
> tried it, but it didn't just work.  I spent an hour or so poking at it
> and I couldn't quite figure out why and I couldn't find enough other
> examples to compare against to see what was wrong...  That was a few
> months ago, though.  Maybe things are in a better shape now?

It worked fine for me on Rb3 and db410c where we had HDMI connector. I
don't have a panel device to test and Bjorn tried to help out with a bit
of testing. This didn't work on the laptop, that is why I haven't posted
it yet.

This has conversion of msm driver and bridge drivers lt9611, adv7511 and
ti-sn65dsi86.

[1]: https://git.linaro.org/people/vinod.koul/kernel.git/log/?h=wip/msm_bridges_no_conn

Thanks
-- 
~Vinod

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Vinod Koul]

Hi,

Thanks Doug for adding me

On 02-11-20, 08:37, Doug Anderson wrote:
> > On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:

> > Any chance we can convince you to prepare this bridge driver for use in
> > a chained bridge setup where the connector is created by the display
> > driver and uses drm_bridge_funcs?
> >
> > First step wuld be to introduce the use of a panel_bridge.
> > Then add get_edid to drm_bridge_funcs and maybe more helpers.
> >
> > Then natural final step would be to move connector creation to the
> > display driver - see how other uses drm_bridge_connector_init() to do so
> > - it is relatively simple.
> >
> > Should be doable - and reach out if you need some help.

Yes it is and doable and you find this at [1], would need a rebase
though.

> At some point I think Vinod tried to prepare a patch for this and I
> tried it, but it didn't just work.  I spent an hour or so poking at it
> and I couldn't quite figure out why and I couldn't find enough other
> examples to compare against to see what was wrong...  That was a few
> months ago, though.  Maybe things are in a better shape now?

It worked fine for me on Rb3 and db410c where we had HDMI connector. I
don't have a panel device to test and Bjorn tried to help out with a bit
of testing. This didn't work on the laptop, that is why I haven't posted
it yet.

This has conversion of msm driver and bridge drivers lt9611, adv7511 and
ti-sn65dsi86.

[1]: https://git.linaro.org/people/vinod.koul/kernel.git/log/?h=wip/msm_bridges_no_conn

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

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Stephen Boyd]

Quoting Sam Ravnborg (2020-11-01 09:37:41)
> Hi Stephen.
> 
> On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> > This patch series cleans up the DDC code a little bit so that
> > it is more efficient time wise and supports grabbing the EDID
> > of the eDP panel over the aux channel. I timed this on a board
> > I have on my desk and it takes about 20ms to grab the EDID out
> > of the panel and make sure it is valid.
> > 
> > The first two patches seem less controversial so I stuck them at
> > the beginning. The third patch does the EDID reading and caches
> > it so we don't have to keep grabbing it over and over again. And
> > finally the last patch updates the reply field so that short
> > reads and nacks over the channel are reflected properly instead of
> > treating them as some sort of error that can't be discerned.
> > 
> > Stephen Boyd (4):
> >   drm/bridge: ti-sn65dsi86: Combine register accesses in
> >     ti_sn_aux_transfer()
> >   drm/bridge: ti-sn65dsi86: Make polling a busy loop
> >   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
> >   drm/bridge: ti-sn65dsi86: Update reply on aux failures
> 
> Series looks good. You can add my a-b on the full series.
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> 
> I can apply after Douglas have had a look at the patches he did not r-b
> yet.
> 
> Any chance we can convince you to prepare this bridge driver for use in
> a chained bridge setup where the connector is created by the display
> driver and uses drm_bridge_funcs?
> 
> First step wuld be to introduce the use of a panel_bridge.
> Then add get_edid to drm_bridge_funcs and maybe more helpers.
> 
> Then natural final step would be to move connector creation to the
> display driver - see how other uses drm_bridge_connector_init() to do so
> - it is relatively simple.
> 
> Should be doable - and reach out if you need some help.
> 

I started to look at this and got stuck at ti_sn_bridge_get_bpp(). Where
can I get the details of the bpc for the downstream bridge or panel? Is
there some function that can tell this bridge what the bpc is for the
attached connector? I see that td_mode_valid() in
drivers/gpu/drm/bridge/tc358775.c stores away the bpc from the incoming
drm_display_info pointer but I'm not sure that is correct because can't
that be called for various and not necessarily the one we're using?

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Stephen Boyd]

Quoting Sam Ravnborg (2020-11-01 09:37:41)
> Hi Stephen.
> 
> On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> > This patch series cleans up the DDC code a little bit so that
> > it is more efficient time wise and supports grabbing the EDID
> > of the eDP panel over the aux channel. I timed this on a board
> > I have on my desk and it takes about 20ms to grab the EDID out
> > of the panel and make sure it is valid.
> > 
> > The first two patches seem less controversial so I stuck them at
> > the beginning. The third patch does the EDID reading and caches
> > it so we don't have to keep grabbing it over and over again. And
> > finally the last patch updates the reply field so that short
> > reads and nacks over the channel are reflected properly instead of
> > treating them as some sort of error that can't be discerned.
> > 
> > Stephen Boyd (4):
> >   drm/bridge: ti-sn65dsi86: Combine register accesses in
> >     ti_sn_aux_transfer()
> >   drm/bridge: ti-sn65dsi86: Make polling a busy loop
> >   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
> >   drm/bridge: ti-sn65dsi86: Update reply on aux failures
> 
> Series looks good. You can add my a-b on the full series.
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> 
> I can apply after Douglas have had a look at the patches he did not r-b
> yet.
> 
> Any chance we can convince you to prepare this bridge driver for use in
> a chained bridge setup where the connector is created by the display
> driver and uses drm_bridge_funcs?
> 
> First step wuld be to introduce the use of a panel_bridge.
> Then add get_edid to drm_bridge_funcs and maybe more helpers.
> 
> Then natural final step would be to move connector creation to the
> display driver - see how other uses drm_bridge_connector_init() to do so
> - it is relatively simple.
> 
> Should be doable - and reach out if you need some help.
> 

I started to look at this and got stuck at ti_sn_bridge_get_bpp(). Where
can I get the details of the bpc for the downstream bridge or panel? Is
there some function that can tell this bridge what the bpc is for the
attached connector? I see that td_mode_valid() in
drivers/gpu/drm/bridge/tc358775.c stores away the bpc from the incoming
drm_display_info pointer but I'm not sure that is correct because can't
that be called for various and not necessarily the one we're using?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Stephen Boyd]

Quoting Stephen Boyd (2020-11-02 17:15:24)
> Quoting Sam Ravnborg (2020-11-01 09:37:41)
> > Hi Stephen.
> > 
> > On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> > > This patch series cleans up the DDC code a little bit so that
> > > it is more efficient time wise and supports grabbing the EDID
> > > of the eDP panel over the aux channel. I timed this on a board
> > > I have on my desk and it takes about 20ms to grab the EDID out
> > > of the panel and make sure it is valid.
> > > 
> > > The first two patches seem less controversial so I stuck them at
> > > the beginning. The third patch does the EDID reading and caches
> > > it so we don't have to keep grabbing it over and over again. And
> > > finally the last patch updates the reply field so that short
> > > reads and nacks over the channel are reflected properly instead of
> > > treating them as some sort of error that can't be discerned.
> > > 
> > > Stephen Boyd (4):
> > >   drm/bridge: ti-sn65dsi86: Combine register accesses in
> > >     ti_sn_aux_transfer()
> > >   drm/bridge: ti-sn65dsi86: Make polling a busy loop
> > >   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
> > >   drm/bridge: ti-sn65dsi86: Update reply on aux failures
> > 
> > Series looks good. You can add my a-b on the full series.
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > 
> > I can apply after Douglas have had a look at the patches he did not r-b
> > yet.
> > 
> > Any chance we can convince you to prepare this bridge driver for use in
> > a chained bridge setup where the connector is created by the display
> > driver and uses drm_bridge_funcs?
> > 
> > First step wuld be to introduce the use of a panel_bridge.
> > Then add get_edid to drm_bridge_funcs and maybe more helpers.
> > 
> > Then natural final step would be to move connector creation to the
> > display driver - see how other uses drm_bridge_connector_init() to do so
> > - it is relatively simple.
> > 
> > Should be doable - and reach out if you need some help.
> > 
> 
> I started to look at this and got stuck at ti_sn_bridge_get_bpp(). Where
> can I get the details of the bpc for the downstream bridge or panel? Is
> there some function that can tell this bridge what the bpc is for the
> attached connector? I see that td_mode_valid() in
> drivers/gpu/drm/bridge/tc358775.c stores away the bpc from the incoming
> drm_display_info pointer but I'm not sure that is correct because can't
> that be called for various and not necessarily the one we're using?

Looking closer I see that tc358775 doesn't store away the incoming
value, it derives a bpc from what comes in. I guess it isn't really a
good example for this problem.

I tried to hack around this and convert this driver to use bridge funcs
with Vinod's msm patch underneath but now the encoder is invalid. I
wonder if something is wrong with how the msm display driver drives the
bridge and connector? That TODO in ti_sn_bridge_attach() makes me think
something else needs fixing. I'll send another patch series tomorrow on
top of this one that kicks off the conversion to bridge connector so we
can discuss there.

[    4.577814] [drm:dpu_core_perf_crtc_update:397] [dpu error]crtc-49: failed to update bus bw vote
[    4.603697] panel-simple panel: error waiting for hpd GPIO: -6
[    4.613106] dsi_calc_pclk: forcing mdss_dsi lanes to 1
[    4.619070] dsi_link_clk_set_rate_6g: Failed to set rate pixel clk, -22
[    4.625883] msm_dsi_host_power_on: failed to enable link clocks. ret=-22
[    4.632784] dsi_mgr_bridge_pre_enable: power on host 0 failed, -22
[    4.646235] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000370
[    4.655279] Mem abort info:
[    4.658249]   ESR = 0x96000006
[    4.661456]   EC = 0x25: DABT (current EL), IL = 32 bits
[    4.667047]   SET = 0, FnV = 0
[    4.670195]   EA = 0, S1PTW = 0
[    4.673430] Data abort info:
[    4.676401]   ISV = 0, ISS = 0x00000006
[    4.680347]   CM = 0, WnR = 0
[    4.683413] user pgtable: 4k pages, 39-bit VAs, pgdp=0000000104f18000
[    4.690033] [0000000000000370] pgd=0000000104f21003, p4d=0000000104f21003, pud=0000000104f21003, pmd=0000000000000000
[    4.700941] Internal error: Oops: 96000006 [#1] PREEMPT SMP
[    4.706672] Modules linked in: cdc_ether usbnet r8152 joydev mii
[    4.712866] CPU: 6 PID: 376 Comm: frecon Not tainted 5.10.0-rc2+ #5
[    4.719302] Hardware name: Google Lazor (rev1+) with LTE (DT)
[    4.725201] pstate: 60400009 (nZCv daif +PAN -UAO -TCO BTYPE=--)
[    4.731390] pc : ti_sn_bridge_enable+0xa4/0x7ec
[    4.736047] lr : ti_sn_bridge_enable+0x54/0x7ec
[    4.740704] sp : ffffffc0109db960
[    4.744119] x29: ffffffc0109db990 x28: ffffffd216acb000 
[    4.749586] x27: ffffffd216854000 x26: ffffffd216acb000 
[    4.755046] x25: ffffffd2164e89d0 x24: 0000000000000000 
[    4.760508] x23: 0000000000000000 x22: ffffff8281de5080 
[    4.765970] x21: ffffff8281de56f0 x20: ffffff8281de5670 
[    4.771431] x19: ffffff8281de5090 x18: 0000000000000484 
[    4.776893] x17: 0000000000000000 x16: 0000000000000001 
[    4.782355] x15: 0000000000000010 x14: 0000000000000000 
[    4.787816] x13: 00000000000013a8 x12: ffffff828208f950 
[    4.793277] x11: ffffff82818e1d80 x10: 0000000000000000 
[    4.798738] x9 : 0000000000000002 x8 : 0000000000000002 
[    4.804200] x7 : ffffffc0109db978 x6 : 0000000000000001 
[    4.809660] x5 : 0000000000000001 x4 : ffffffd2166b6388 
[    4.815122] x3 : 0000000000000001 x2 : ffffffd216680dab 
[    4.820584] x1 : 0000000000000010 x0 : ffffff8280ceac00 
[    4.826046] Call trace:
[    4.828568]  ti_sn_bridge_enable+0xa4/0x7ec
[    4.832886]  drm_atomic_bridge_chain_enable+0x7c/0xa4
[    4.838095]  drm_atomic_helper_commit_modeset_enables+0x19c/0x23c
[    4.844360]  msm_atomic_commit_tail+0x308/0x6e8
[    4.849017]  commit_tail+0xa8/0x140
[    4.852608]  drm_atomic_helper_commit+0xf8/0x100
[    4.857354]  drm_atomic_commit+0x50/0x5c
[    4.861385]  drm_atomic_helper_set_config+0xcc/0xd4
[    4.866398]  drm_mode_setcrtc+0x280/0x5c0
[    4.870519]  drm_ioctl_kernel+0xa0/0x118
[    4.874549]  drm_ioctl+0x240/0x3dc

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Stephen Boyd]

Quoting Stephen Boyd (2020-11-02 17:15:24)
> Quoting Sam Ravnborg (2020-11-01 09:37:41)
> > Hi Stephen.
> > 
> > On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> > > This patch series cleans up the DDC code a little bit so that
> > > it is more efficient time wise and supports grabbing the EDID
> > > of the eDP panel over the aux channel. I timed this on a board
> > > I have on my desk and it takes about 20ms to grab the EDID out
> > > of the panel and make sure it is valid.
> > > 
> > > The first two patches seem less controversial so I stuck them at
> > > the beginning. The third patch does the EDID reading and caches
> > > it so we don't have to keep grabbing it over and over again. And
> > > finally the last patch updates the reply field so that short
> > > reads and nacks over the channel are reflected properly instead of
> > > treating them as some sort of error that can't be discerned.
> > > 
> > > Stephen Boyd (4):
> > >   drm/bridge: ti-sn65dsi86: Combine register accesses in
> > >     ti_sn_aux_transfer()
> > >   drm/bridge: ti-sn65dsi86: Make polling a busy loop
> > >   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
> > >   drm/bridge: ti-sn65dsi86: Update reply on aux failures
> > 
> > Series looks good. You can add my a-b on the full series.
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > 
> > I can apply after Douglas have had a look at the patches he did not r-b
> > yet.
> > 
> > Any chance we can convince you to prepare this bridge driver for use in
> > a chained bridge setup where the connector is created by the display
> > driver and uses drm_bridge_funcs?
> > 
> > First step wuld be to introduce the use of a panel_bridge.
> > Then add get_edid to drm_bridge_funcs and maybe more helpers.
> > 
> > Then natural final step would be to move connector creation to the
> > display driver - see how other uses drm_bridge_connector_init() to do so
> > - it is relatively simple.
> > 
> > Should be doable - and reach out if you need some help.
> > 
> 
> I started to look at this and got stuck at ti_sn_bridge_get_bpp(). Where
> can I get the details of the bpc for the downstream bridge or panel? Is
> there some function that can tell this bridge what the bpc is for the
> attached connector? I see that td_mode_valid() in
> drivers/gpu/drm/bridge/tc358775.c stores away the bpc from the incoming
> drm_display_info pointer but I'm not sure that is correct because can't
> that be called for various and not necessarily the one we're using?

Looking closer I see that tc358775 doesn't store away the incoming
value, it derives a bpc from what comes in. I guess it isn't really a
good example for this problem.

I tried to hack around this and convert this driver to use bridge funcs
with Vinod's msm patch underneath but now the encoder is invalid. I
wonder if something is wrong with how the msm display driver drives the
bridge and connector? That TODO in ti_sn_bridge_attach() makes me think
something else needs fixing. I'll send another patch series tomorrow on
top of this one that kicks off the conversion to bridge connector so we
can discuss there.

[    4.577814] [drm:dpu_core_perf_crtc_update:397] [dpu error]crtc-49: failed to update bus bw vote
[    4.603697] panel-simple panel: error waiting for hpd GPIO: -6
[    4.613106] dsi_calc_pclk: forcing mdss_dsi lanes to 1
[    4.619070] dsi_link_clk_set_rate_6g: Failed to set rate pixel clk, -22
[    4.625883] msm_dsi_host_power_on: failed to enable link clocks. ret=-22
[    4.632784] dsi_mgr_bridge_pre_enable: power on host 0 failed, -22
[    4.646235] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000370
[    4.655279] Mem abort info:
[    4.658249]   ESR = 0x96000006
[    4.661456]   EC = 0x25: DABT (current EL), IL = 32 bits
[    4.667047]   SET = 0, FnV = 0
[    4.670195]   EA = 0, S1PTW = 0
[    4.673430] Data abort info:
[    4.676401]   ISV = 0, ISS = 0x00000006
[    4.680347]   CM = 0, WnR = 0
[    4.683413] user pgtable: 4k pages, 39-bit VAs, pgdp=0000000104f18000
[    4.690033] [0000000000000370] pgd=0000000104f21003, p4d=0000000104f21003, pud=0000000104f21003, pmd=0000000000000000
[    4.700941] Internal error: Oops: 96000006 [#1] PREEMPT SMP
[    4.706672] Modules linked in: cdc_ether usbnet r8152 joydev mii
[    4.712866] CPU: 6 PID: 376 Comm: frecon Not tainted 5.10.0-rc2+ #5
[    4.719302] Hardware name: Google Lazor (rev1+) with LTE (DT)
[    4.725201] pstate: 60400009 (nZCv daif +PAN -UAO -TCO BTYPE=--)
[    4.731390] pc : ti_sn_bridge_enable+0xa4/0x7ec
[    4.736047] lr : ti_sn_bridge_enable+0x54/0x7ec
[    4.740704] sp : ffffffc0109db960
[    4.744119] x29: ffffffc0109db990 x28: ffffffd216acb000 
[    4.749586] x27: ffffffd216854000 x26: ffffffd216acb000 
[    4.755046] x25: ffffffd2164e89d0 x24: 0000000000000000 
[    4.760508] x23: 0000000000000000 x22: ffffff8281de5080 
[    4.765970] x21: ffffff8281de56f0 x20: ffffff8281de5670 
[    4.771431] x19: ffffff8281de5090 x18: 0000000000000484 
[    4.776893] x17: 0000000000000000 x16: 0000000000000001 
[    4.782355] x15: 0000000000000010 x14: 0000000000000000 
[    4.787816] x13: 00000000000013a8 x12: ffffff828208f950 
[    4.793277] x11: ffffff82818e1d80 x10: 0000000000000000 
[    4.798738] x9 : 0000000000000002 x8 : 0000000000000002 
[    4.804200] x7 : ffffffc0109db978 x6 : 0000000000000001 
[    4.809660] x5 : 0000000000000001 x4 : ffffffd2166b6388 
[    4.815122] x3 : 0000000000000001 x2 : ffffffd216680dab 
[    4.820584] x1 : 0000000000000010 x0 : ffffff8280ceac00 
[    4.826046] Call trace:
[    4.828568]  ti_sn_bridge_enable+0xa4/0x7ec
[    4.832886]  drm_atomic_bridge_chain_enable+0x7c/0xa4
[    4.838095]  drm_atomic_helper_commit_modeset_enables+0x19c/0x23c
[    4.844360]  msm_atomic_commit_tail+0x308/0x6e8
[    4.849017]  commit_tail+0xa8/0x140
[    4.852608]  drm_atomic_helper_commit+0xf8/0x100
[    4.857354]  drm_atomic_commit+0x50/0x5c
[    4.861385]  drm_atomic_helper_set_config+0xcc/0xd4
[    4.866398]  drm_mode_setcrtc+0x280/0x5c0
[    4.870519]  drm_ioctl_kernel+0xa0/0x118
[    4.874549]  drm_ioctl+0x240/0x3dc
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Laurent Pinchart]

Hi Stephen,

On Mon, Nov 02, 2020 at 05:15:24PM -0800, Stephen Boyd wrote:
> Quoting Sam Ravnborg (2020-11-01 09:37:41)
> > Hi Stephen.
> > 
> > On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> > > This patch series cleans up the DDC code a little bit so that
> > > it is more efficient time wise and supports grabbing the EDID
> > > of the eDP panel over the aux channel. I timed this on a board
> > > I have on my desk and it takes about 20ms to grab the EDID out
> > > of the panel and make sure it is valid.
> > > 
> > > The first two patches seem less controversial so I stuck them at
> > > the beginning. The third patch does the EDID reading and caches
> > > it so we don't have to keep grabbing it over and over again. And
> > > finally the last patch updates the reply field so that short
> > > reads and nacks over the channel are reflected properly instead of
> > > treating them as some sort of error that can't be discerned.
> > > 
> > > Stephen Boyd (4):
> > >   drm/bridge: ti-sn65dsi86: Combine register accesses in
> > >     ti_sn_aux_transfer()
> > >   drm/bridge: ti-sn65dsi86: Make polling a busy loop
> > >   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
> > >   drm/bridge: ti-sn65dsi86: Update reply on aux failures
> > 
> > Series looks good. You can add my a-b on the full series.
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > 
> > I can apply after Douglas have had a look at the patches he did not r-b
> > yet.
> > 
> > Any chance we can convince you to prepare this bridge driver for use in
> > a chained bridge setup where the connector is created by the display
> > driver and uses drm_bridge_funcs?
> > 
> > First step wuld be to introduce the use of a panel_bridge.
> > Then add get_edid to drm_bridge_funcs and maybe more helpers.
> > 
> > Then natural final step would be to move connector creation to the
> > display driver - see how other uses drm_bridge_connector_init() to do so
> > - it is relatively simple.
> > 
> > Should be doable - and reach out if you need some help.
> 
> I started to look at this and got stuck at ti_sn_bridge_get_bpp(). Where
> can I get the details of the bpc for the downstream bridge or panel? Is
> there some function that can tell this bridge what the bpc is for the
> attached connector?

I've posted a patch series to convert to DRM_BRIDGE_ATTACH_NO_CONNECTOR
yesterday (and have CC'ed you), but I've overlooked this particular
problem :-S

You can't get the connector in the .enable() operation, but you can get
it in .atomic_enable(), with
drm_atomic_get_new_connector_for_encoder(). This being said, it's
probably not the right option.

What matters here isn't the bpc for the connector, but the format
expected by the next bridge in the chain. drm_bridge_funcs has two
operations, .atomic_get_output_bus_fmts() and
.atomic_get_input_bus_fmts(), to negotiate that format along a chain of
bridges. The panel bridge driver (drivers/gpu/drm/bridge/panel.c)
doesn't implement those operations, and neither does
display-connector.c, so that may be what we should start with.

> I see that td_mode_valid() in
> drivers/gpu/drm/bridge/tc358775.c stores away the bpc from the incoming
> drm_display_info pointer but I'm not sure that is correct because can't
> that be called for various and not necessarily the one we're using?

You're right, .mode_valid() shouldn't do that.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Laurent Pinchart]

Hi Stephen,

On Mon, Nov 02, 2020 at 05:15:24PM -0800, Stephen Boyd wrote:
> Quoting Sam Ravnborg (2020-11-01 09:37:41)
> > Hi Stephen.
> > 
> > On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> > > This patch series cleans up the DDC code a little bit so that
> > > it is more efficient time wise and supports grabbing the EDID
> > > of the eDP panel over the aux channel. I timed this on a board
> > > I have on my desk and it takes about 20ms to grab the EDID out
> > > of the panel and make sure it is valid.
> > > 
> > > The first two patches seem less controversial so I stuck them at
> > > the beginning. The third patch does the EDID reading and caches
> > > it so we don't have to keep grabbing it over and over again. And
> > > finally the last patch updates the reply field so that short
> > > reads and nacks over the channel are reflected properly instead of
> > > treating them as some sort of error that can't be discerned.
> > > 
> > > Stephen Boyd (4):
> > >   drm/bridge: ti-sn65dsi86: Combine register accesses in
> > >     ti_sn_aux_transfer()
> > >   drm/bridge: ti-sn65dsi86: Make polling a busy loop
> > >   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
> > >   drm/bridge: ti-sn65dsi86: Update reply on aux failures
> > 
> > Series looks good. You can add my a-b on the full series.
> > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > 
> > I can apply after Douglas have had a look at the patches he did not r-b
> > yet.
> > 
> > Any chance we can convince you to prepare this bridge driver for use in
> > a chained bridge setup where the connector is created by the display
> > driver and uses drm_bridge_funcs?
> > 
> > First step wuld be to introduce the use of a panel_bridge.
> > Then add get_edid to drm_bridge_funcs and maybe more helpers.
> > 
> > Then natural final step would be to move connector creation to the
> > display driver - see how other uses drm_bridge_connector_init() to do so
> > - it is relatively simple.
> > 
> > Should be doable - and reach out if you need some help.
> 
> I started to look at this and got stuck at ti_sn_bridge_get_bpp(). Where
> can I get the details of the bpc for the downstream bridge or panel? Is
> there some function that can tell this bridge what the bpc is for the
> attached connector?

I've posted a patch series to convert to DRM_BRIDGE_ATTACH_NO_CONNECTOR
yesterday (and have CC'ed you), but I've overlooked this particular
problem :-S

You can't get the connector in the .enable() operation, but you can get
it in .atomic_enable(), with
drm_atomic_get_new_connector_for_encoder(). This being said, it's
probably not the right option.

What matters here isn't the bpc for the connector, but the format
expected by the next bridge in the chain. drm_bridge_funcs has two
operations, .atomic_get_output_bus_fmts() and
.atomic_get_input_bus_fmts(), to negotiate that format along a chain of
bridges. The panel bridge driver (drivers/gpu/drm/bridge/panel.c)
doesn't implement those operations, and neither does
display-connector.c, so that may be what we should start with.

> I see that td_mode_valid() in
> drivers/gpu/drm/bridge/tc358775.c stores away the bpc from the incoming
> drm_display_info pointer but I'm not sure that is correct because can't
> that be called for various and not necessarily the one we're using?

You're right, .mode_valid() shouldn't do that.

-- 
Regards,

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

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Stephen Boyd]

Quoting Laurent Pinchart (2021-03-22 17:00:23)
> Hi Stephen,
> 
> On Mon, Nov 02, 2020 at 05:15:24PM -0800, Stephen Boyd wrote:
> > Quoting Sam Ravnborg (2020-11-01 09:37:41)
> > > Hi Stephen.
> > > 
> > > On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> > > > This patch series cleans up the DDC code a little bit so that
> > > > it is more efficient time wise and supports grabbing the EDID
> > > > of the eDP panel over the aux channel. I timed this on a board
> > > > I have on my desk and it takes about 20ms to grab the EDID out
> > > > of the panel and make sure it is valid.
> > > > 
> > > > The first two patches seem less controversial so I stuck them at
> > > > the beginning. The third patch does the EDID reading and caches
> > > > it so we don't have to keep grabbing it over and over again. And
> > > > finally the last patch updates the reply field so that short
> > > > reads and nacks over the channel are reflected properly instead of
> > > > treating them as some sort of error that can't be discerned.
> > > > 
> > > > Stephen Boyd (4):
> > > >   drm/bridge: ti-sn65dsi86: Combine register accesses in
> > > >     ti_sn_aux_transfer()
> > > >   drm/bridge: ti-sn65dsi86: Make polling a busy loop
> > > >   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
> > > >   drm/bridge: ti-sn65dsi86: Update reply on aux failures
> > > 
> > > Series looks good. You can add my a-b on the full series.
> > > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > > 
> > > I can apply after Douglas have had a look at the patches he did not r-b
> > > yet.
> > > 
> > > Any chance we can convince you to prepare this bridge driver for use in
> > > a chained bridge setup where the connector is created by the display
> > > driver and uses drm_bridge_funcs?
> > > 
> > > First step wuld be to introduce the use of a panel_bridge.
> > > Then add get_edid to drm_bridge_funcs and maybe more helpers.
> > > 
> > > Then natural final step would be to move connector creation to the
> > > display driver - see how other uses drm_bridge_connector_init() to do so
> > > - it is relatively simple.
> > > 
> > > Should be doable - and reach out if you need some help.
> > 
> > I started to look at this and got stuck at ti_sn_bridge_get_bpp(). Where
> > can I get the details of the bpc for the downstream bridge or panel? Is
> > there some function that can tell this bridge what the bpc is for the
> > attached connector?
> 
> I've posted a patch series to convert to DRM_BRIDGE_ATTACH_NO_CONNECTOR
> yesterday (and have CC'ed you), but I've overlooked this particular
> problem :-S

!

> 
> You can't get the connector in the .enable() operation, but you can get
> it in .atomic_enable(), with
> drm_atomic_get_new_connector_for_encoder(). This being said, it's
> probably not the right option.
> 
> What matters here isn't the bpc for the connector, but the format
> expected by the next bridge in the chain. drm_bridge_funcs has two
> operations, .atomic_get_output_bus_fmts() and
> .atomic_get_input_bus_fmts(), to negotiate that format along a chain of
> bridges. The panel bridge driver (drivers/gpu/drm/bridge/panel.c)
> doesn't implement those operations, and neither does
> display-connector.c, so that may be what we should start with.

Ok, makes sense. I'd gladly test things out if you come up with some
solution here.

Re: [PATCH v2 0/4] drm/bridge: ti-sn65dsi86: Support EDID reading

[Stephen Boyd]

Quoting Laurent Pinchart (2021-03-22 17:00:23)
> Hi Stephen,
> 
> On Mon, Nov 02, 2020 at 05:15:24PM -0800, Stephen Boyd wrote:
> > Quoting Sam Ravnborg (2020-11-01 09:37:41)
> > > Hi Stephen.
> > > 
> > > On Thu, Oct 29, 2020 at 06:17:34PM -0700, Stephen Boyd wrote:
> > > > This patch series cleans up the DDC code a little bit so that
> > > > it is more efficient time wise and supports grabbing the EDID
> > > > of the eDP panel over the aux channel. I timed this on a board
> > > > I have on my desk and it takes about 20ms to grab the EDID out
> > > > of the panel and make sure it is valid.
> > > > 
> > > > The first two patches seem less controversial so I stuck them at
> > > > the beginning. The third patch does the EDID reading and caches
> > > > it so we don't have to keep grabbing it over and over again. And
> > > > finally the last patch updates the reply field so that short
> > > > reads and nacks over the channel are reflected properly instead of
> > > > treating them as some sort of error that can't be discerned.
> > > > 
> > > > Stephen Boyd (4):
> > > >   drm/bridge: ti-sn65dsi86: Combine register accesses in
> > > >     ti_sn_aux_transfer()
> > > >   drm/bridge: ti-sn65dsi86: Make polling a busy loop
> > > >   drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
> > > >   drm/bridge: ti-sn65dsi86: Update reply on aux failures
> > > 
> > > Series looks good. You can add my a-b on the full series.
> > > Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > > 
> > > I can apply after Douglas have had a look at the patches he did not r-b
> > > yet.
> > > 
> > > Any chance we can convince you to prepare this bridge driver for use in
> > > a chained bridge setup where the connector is created by the display
> > > driver and uses drm_bridge_funcs?
> > > 
> > > First step wuld be to introduce the use of a panel_bridge.
> > > Then add get_edid to drm_bridge_funcs and maybe more helpers.
> > > 
> > > Then natural final step would be to move connector creation to the
> > > display driver - see how other uses drm_bridge_connector_init() to do so
> > > - it is relatively simple.
> > > 
> > > Should be doable - and reach out if you need some help.
> > 
> > I started to look at this and got stuck at ti_sn_bridge_get_bpp(). Where
> > can I get the details of the bpc for the downstream bridge or panel? Is
> > there some function that can tell this bridge what the bpc is for the
> > attached connector?
> 
> I've posted a patch series to convert to DRM_BRIDGE_ATTACH_NO_CONNECTOR
> yesterday (and have CC'ed you), but I've overlooked this particular
> problem :-S

!

> 
> You can't get the connector in the .enable() operation, but you can get
> it in .atomic_enable(), with
> drm_atomic_get_new_connector_for_encoder(). This being said, it's
> probably not the right option.
> 
> What matters here isn't the bpc for the connector, but the format
> expected by the next bridge in the chain. drm_bridge_funcs has two
> operations, .atomic_get_output_bus_fmts() and
> .atomic_get_input_bus_fmts(), to negotiate that format along a chain of
> bridges. The panel bridge driver (drivers/gpu/drm/bridge/panel.c)
> doesn't implement those operations, and neither does
> display-connector.c, so that may be what we should start with.

Ok, makes sense. I'd gladly test things out if you come up with some
solution here.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel