Re: [PATCH 2/2] drm/bridge: ti-sn65dsi86: Allow DT to set "HPD delay"

[spanda]

On 2018-10-20 01:49, Douglas Anderson wrote:

Let's solve the mystery of commit bf1178c98930 ("drm/bridge:
ti-sn65dsi86: Add mystery delay to enable()").  Specifically the
reason we needed that mystery delay is that we weren't paying
attention to HPD.

Looking at the datasheet for the same panel that was tested for the
original commit, I see there's a timing "t3" that times from power on
to the aux channel being operational.  This time is specced as 0 - 200
ms.  The datasheet says that the aux channel is operational at exactly
the same time that HPD is asserted.

Scoping the signals on this board showed that HPD was asserted 84 ms
after power was asserted.  That very closely matches the magic 70 ms
delay that we had.  ...and actually, in my esting the 70 ms wasn't
quite enough of a delay and some percentage of the time the display
didn't come up until I bumped it to 100 ms.

To solve this, we tried to hook up the HPD signal in the bridge.
...but in doing so we found that that the bridge didn't report that
HPD was asserted until ~280 ms after we powered it (!).  This is
explained by looking at the sn65dsi86 datasheet section "8.4.5.1 HPD
(Hot Plug/Unplug Detection)".  Reading there we see that the bridge
isn't even intended to report HPD until 100 ms after it's asserted.
...but that would have left us at 184 ms.  The extra 100 ms
(presumably) comes from this part in the datasheet:


The HPD state machine operates off an internal ring oscillator. The
ring oscillator frequency will vary [ ... ]. The min/max range in
the HPD State Diagram refers to the possible times based off
variation in the ring oscillator frequency.


Given that the 280 ms we'll end up delaying if we hook up HPD is
_slower_ than the 200 ms we could just hardcode, for now we'll solve
the problem by just allowing boards to hardcode a value.  If someone
using this part finds that they can get things to work more quickly by
actually hooking up HPD that can always be a future patch.

One last note is that I tried to solve this through another way: In
ti_sn_bridge_enable() I tried to use various combinations of
dp_dpcd_writeb() and dp_dpcd_readb() to detect when the aux channel
was up.  In theory that would let me detect _exactly_ when I could
continue and do link training.  Unfortunately even if I did an aux
transfer w/out waiting I couldn't see any errors.  Possibly I could
keep looping over link training until it came back with success, but
that seemed a little overly hacky to me.



Thanks for very detailed explanation.


Signed-off-by: Douglas Anderson 
---

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 45 ++-
 1 file changed, 37 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index f8a931cf3665..5deed667480c 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -93,6 +93,7 @@ struct ti_sn_bridge {
struct clk  *refclk;
struct drm_panel*panel;
struct gpio_desc*enable_gpio;
+   int panel_hpd_delay_ms;
struct regulator_bulk_data  supplies[SN_REGULATOR_SUPPLY_NUM];
 };

@@ -459,16 +460,37 @@ static void ti_sn_bridge_enable(struct drm_bridge 
*bridge)

int ret;

/*
-* FIXME:
-* This 70ms was found necessary by experimentation. If it's not
-	 * present, link training fails. It seems like it can go anywhere 
from

-* pre_enable() up to semi-auto link training initiation below.
+* The timing diagram of some eDP panels says that you're supposed to
+* wait for HPD to be asserted before the aux channel is operational.
 *
-	 * Neither the datasheet for the bridge nor the panel tested mention 
a
-	 * delay of this magnitude in the timing requirements. So for now, 
add

-* the mystery delay until someone figures out a better fix.
+* While we could configure the bridge to report the HPD signal to us
+	 * and add a delay here until the HPD is asserted, it turns out 
that's

+* slower than just hardcoding the max delay from the panel in some
+* cases.  Why?
+*
+* The sn65dsi86 datasheet says that it only reports the debounced
+* HPD signal to software.  It will tell software about HPD assertion
+* as quickly as 100 ms after it's asserted, but sometimes it might
+* take 400 ms because it's timed with a very inaccurate ring
+* oscillator.  In practice it was measured at 200 ms on at least
+* one system.
+*
+	 * On a particular panel, HPD was asserted 84 ms after power was 
given.

+* This same panel specified that HPD would always be asserted within
+* 200 ms of applying power.  Thus on this panel with the measured
+	 * 84 ms to assert HPD + the 200 ms measured debounce we'd wait 284 
ms

+* which is 84 ms longer than just 

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

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:

From: Sean Paul 

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

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

Changes in v3:
- Added to the set

Cc: Sandeep Panda 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index d3e27e52ea759..f8a931cf3665e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -458,6 +458,18 @@ static void ti_sn_bridge_enable(struct drm_bridge 
*bridge)

unsigned int val;
int ret;

+   /*
+* FIXME:
+* This 70ms was found necessary by experimentation. If it's not
+	 * present, link training fails. It seems like it can go anywhere 
from

+* pre_enable() up to semi-auto link training initiation below.
+*
+	 * Neither the datasheet for the bridge nor the panel tested mention 
a
+	 * delay of this magnitude in the timing requirements. So for now, 
add

+* the mystery delay until someone figures out a better fix.
+*/
+   msleep(70);
+
/* DSI_A lane config */
val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,


Reviewed-by: Sandeep Panda 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[spanda]

On 2018-08-23 19:55, Sean Paul wrote:

On Tue, Aug 14, 2018 at 10:01:45AM -0400, Sean Paul wrote:

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

Well, I suppose I shouldn't say that since I've not tried the bridge 
on another

board. I should say, it reproduces on cheza.


Ping. Sandeep: any more feedback?

Sean



Sean

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

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


No more comments from my side.

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

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

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:

From: Sean Paul 

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

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

Changes in v3:
- Added to the set

Cc: Sandeep Panda 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index d3e27e52ea759..f8a931cf3665e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -458,6 +458,18 @@ static void ti_sn_bridge_enable(struct drm_bridge 
*bridge)

unsigned int val;
int ret;

+   /*
+* FIXME:
+* This 70ms was found necessary by experimentation. If it's not
+	 * present, link training fails. It seems like it can go anywhere 
from

+* pre_enable() up to semi-auto link training initiation below.
+*
+	 * Neither the datasheet for the bridge nor the panel tested mention 
a
+	 * delay of this magnitude in the timing requirements. So for now, 
add

+* the mystery delay until someone figures out a better fix.
+*/


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


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

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

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

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:

From: Sean Paul 

Instead of just waiting 20ms for training to complete, actually poll 
the

status to ensure training is finished.

Changes in v3:
- Added to the set

Cc: Sandeep Panda 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index f02bdedae1e5e..d3e27e52ea759 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -493,7 +493,17 @@ static void ti_sn_bridge_enable(struct drm_bridge 
*bridge)


/* Semi auto link training mode */
regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
-   msleep(20); /* 20ms delay recommended by spec */
+   ret = regmap_read_poll_timeout(pdata->regmap, SN_ML_TX_MODE_REG, val,
+  val == ML_TX_MAIN_LINK_OFF ||
+  val == ML_TX_NORMAL_MODE, 1000,
+  500 * 1000);
+   if (ret) {
+   DRM_ERROR("Training complete polling failed (%d)\n", ret);
+   return;
+   } else if (val == ML_TX_MAIN_LINK_OFF) {
+   DRM_ERROR("Link training failed, link is off\n");
+   return;
+   }

/* config video parameters */
ti_sn_bridge_set_video_timings(pdata);

Reviewed-by: Sandeep Panda 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:

From: Sean Paul 

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

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

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

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 587d4e4f5674c..501f4a81ea5ab 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -7,12 +7,14 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -46,7 +48,7 @@
 #define SN_DATA_FORMAT_REG 0x5B
 #define SN_HPD_DISABLE_REG 0x5C
 #define  HPD_DISABLE   BIT(0)
-#define SN_AUX_WDATA0_REG  0x64
+#define SN_AUX_WDATA_REG(x)(0x64 + (x))
 #define SN_AUX_ADDR_19_16_REG  0x74
 #define SN_AUX_ADDR_15_8_REG   0x75
 #define SN_AUX_ADDR_7_0_REG0x76
@@ -54,6 +56,7 @@
 #define SN_AUX_CMD_REG 0x78
 #define  AUX_CMD_SEND  BIT(1)
 #define  AUX_CMD_REQ(x)((x) << 4)
+#define SN_AUX_RDATA_REG(x)(0x79 + (x))
 #define SN_SSC_CONFIG_REG  0x93
 #define  DP_NUM_LANES_MASK GENMASK(5, 4)
 #define  DP_NUM_LANES(x)   ((x) << 4)
@@ -63,6 +66,10 @@
 #define SN_ML_TX_MODE_REG  0x96
 #define  ML_TX_MAIN_LINK_OFF   0
 #define  ML_TX_NORMAL_MODE BIT(0)
+#define SN_AUX_CMD_STATUS_REG  0xF4
+#define  AUX_IRQ_STATUS_AUX_RPLY_TOUT  BIT(3)
+#define  AUX_IRQ_STATUS_AUX_SHORT  BIT(5)
+#define  AUX_IRQ_STATUS_NAT_I2C_FAIL   BIT(6)

 #define MIN_DSI_CLK_FREQ_MHZ   40

@@ -70,11 +77,15 @@
 #define DP_CLK_FUDGE_NUM   10
 #define DP_CLK_FUDGE_DEN   8

+/* Matches DP_AUX_MAX_PAYLOAD_BYTES (for now) */
+#define SN_AUX_MAX_PAYLOAD_BYTES   16
+
 #define SN_REGULATOR_SUPPLY_NUM4

 struct ti_sn_bridge {
struct device   *dev;
struct regmap   *regmap;
+   struct drm_dp_aux   aux;
struct drm_bridge   bridge;
struct drm_connectorconnector;
struct device_node  *host_node;
@@ -471,13 +482,8 @@ static void ti_sn_bridge_enable(struct drm_bridge 
*bridge)
 	 * authentication method. We need to enable this method in the eDP 
panel

 * at DisplayPort address 0x0010A prior to link training.
 */
-   regmap_write(pdata->regmap, SN_AUX_WDATA0_REG, 0x01);
-   regmap_write(pdata->regmap, SN_AUX_ADDR_19_16_REG, 0x00);
-   regmap_write(pdata->regmap, SN_AUX_ADDR_15_8_REG, 0x01);
-   regmap_write(pdata->regmap, SN_AUX_ADDR_7_0_REG, 0x0A);
-   regmap_write(pdata->regmap, SN_AUX_LENGTH_REG, 0x01);
-   regmap_write(pdata->regmap, SN_AUX_CMD_REG, 0x81);
-   usleep_range(1, 10500); /* 10ms delay recommended by spec */
+   drm_dp_dpcd_writeb(>aux, DP_EDP_CONFIGURATION_SET,
+  DP_ALTERNATE_SCRAMBLER_RESET_ENABLE);

/* Semi auto link training mode */
regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0x0A);
@@ -525,6 +531,82 @@ static const struct drm_bridge_funcs 
ti_sn_bridge_funcs = {

.post_disable = ti_sn_bridge_post_disable,
 };

+static struct ti_sn_bridge *aux_to_ti_sn_bridge(struct drm_dp_aux 
*aux)

+{
+   return container_of(aux, struct ti_sn_bridge, aux);
+}
+
+static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
+ struct drm_dp_aux_msg *msg)
+{
+   struct ti_sn_bridge *pdata = aux_to_ti_sn_bridge(aux);
+   u32 request = msg->request & ~DP_AUX_I2C_MOT;
+   u32 request_val = AUX_CMD_REQ(msg->request);
+   u8 *buf = (u8 *)msg->buffer;
+   unsigned int val;
+   int ret, i;
+
+   if (msg->size > SN_AUX_MAX_PAYLOAD_BYTES)
+   return -EINVAL;
+
+   switch (request) {
+   case DP_AUX_NATIVE_WRITE:
+   case DP_AUX_I2C_WRITE:
+   case DP_AUX_NATIVE_READ:
+   case DP_AUX_I2C_READ:
+   regmap_write(pdata->regmap, 

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

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:

From: Sean Paul 

Instead of just waiting and hoping, actually poll for the pll lock to 
be

acquired. As a bonus, this should be significantly faster than the
sleep.

Changes in v3:
- Added to the set

Cc: Sandeep Panda 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index d2119ab546147..f02bdedae1e5e 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -456,6 +456,7 @@ static void ti_sn_bridge_enable(struct drm_bridge 
*bridge)

 {
struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
unsigned int val;
+   int ret;

/* DSI_A lane config */
val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
@@ -472,7 +473,14 @@ static void ti_sn_bridge_enable(struct drm_bridge 
*bridge)


/* enable DP PLL */
regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 1);
-   usleep_range(1, 10500); /* 10ms delay recommended by spec */
+
+   ret = regmap_read_poll_timeout(pdata->regmap, SN_DPPLL_SRC_REG, val,
+  val & DPPLL_SRC_DP_PLL_LOCK, 1000,
+  50 * 1000);
+   if (ret) {
+   DRM_ERROR("DP_PLL_LOCK polling failed (%d)\n", ret);
+   return;
+   }

/**
 * The SN65DSI86 only supports ASSR Display Authentication method and

Reviewed-by: Sandeep Panda 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:

From: Sean Paul 

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

Changes in v3:
- Added to the set

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

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 1b6e8b72be584..587d4e4f5674c 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -18,36 +18,51 @@
 #include 
 #include 

-/* Link Training specific registers */
 #define SN_DEVICE_REV_REG  0x08
-#define SN_HPD_DISABLE_REG 0x5C
 #define SN_DPPLL_SRC_REG   0x0A
+#define  DPPLL_CLK_SRC_DSICLK  BIT(0)
+#define  REFCLK_FREQ_MASK  GENMASK(3, 1)
+#define  REFCLK_FREQ(x)((x) << 1)
+#define  DPPLL_SRC_DP_PLL_LOCK BIT(7)
+#define SN_PLL_ENABLE_REG  0x0D
 #define SN_DSI_LANES_REG   0x10
+#define  CHA_DSI_LANES_MASKGENMASK(4, 3)
+#define  CHA_DSI_LANES(x)  ((x) << 3)
 #define SN_DSIA_CLK_FREQ_REG   0x12
-#define SN_ENH_FRAME_REG   0x5A
-#define SN_SSC_CONFIG_REG  0x93
-#define SN_DATARATE_CONFIG_REG 0x94
-#define SN_PLL_ENABLE_REG  0x0D
-#define SN_SCRAMBLE_CONFIG_REG 0x95
-#define SN_AUX_WDATA0_REG  0x64
-#define SN_AUX_ADDR_19_16_REG  0x74
-#define SN_AUX_ADDR_15_8_REG   0x75
-#define SN_AUX_ADDR_7_0_REG0x76
-#define SN_AUX_LENGTH_REG  0x77
-#define SN_AUX_CMD_REG 0x78
-#define SN_ML_TX_MODE_REG  0x96
-/* video config specific registers */
 #define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG  0x20
 #define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG   0x24
 #define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG   0x2C
 #define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG  0x2D
+#define  CHA_HSYNC_POLARITYBIT(7)
 #define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG   0x30
 #define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG  0x31
+#define  CHA_VSYNC_POLARITYBIT(7)
 #define SN_CHA_HORIZONTAL_BACK_PORCH_REG   0x34
 #define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36
 #define SN_CHA_HORIZONTAL_FRONT_PORCH_REG  0x38
 #define SN_CHA_VERTICAL_FRONT_PORCH_REG0x3A
+#define SN_ENH_FRAME_REG   0x5A
+#define  VSTREAM_ENABLEBIT(3)
 #define SN_DATA_FORMAT_REG 0x5B
+#define SN_HPD_DISABLE_REG 0x5C
+#define  HPD_DISABLE   BIT(0)
+#define SN_AUX_WDATA0_REG  0x64
+#define SN_AUX_ADDR_19_16_REG  0x74
+#define SN_AUX_ADDR_15_8_REG   0x75
+#define SN_AUX_ADDR_7_0_REG0x76
+#define SN_AUX_LENGTH_REG  0x77
+#define SN_AUX_CMD_REG 0x78
+#define  AUX_CMD_SEND  BIT(1)
+#define  AUX_CMD_REQ(x)((x) << 4)
+#define SN_SSC_CONFIG_REG  0x93
+#define  DP_NUM_LANES_MASK GENMASK(5, 4)
+#define  DP_NUM_LANES(x)   ((x) << 4)
+#define SN_DATARATE_CONFIG_REG 0x94
+#define  DP_DATARATE_MASK  GENMASK(7, 5)
+#define  DP_DATARATE(x)((x) << 5)
+#define SN_ML_TX_MODE_REG  0x96
+#define  ML_TX_MAIN_LINK_OFF   0
+#define  ML_TX_NORMAL_MODE BIT(0)

 #define MIN_DSI_CLK_FREQ_MHZ   40

@@ -55,22 +70,6 @@
 #define DP_CLK_FUDGE_NUM   10
 #define DP_CLK_FUDGE_DEN   8

-#define DPPLL_CLK_SRC_REFCLK   0
-#define DPPLL_CLK_SRC_DSICLK   1
-
-#define SN_REFCLK_FREQ_OFFSET  1
-#define SN_DSIA_LANE_OFFSET3
-#define SN_DP_LANE_OFFSET  4
-#define SN_DP_DATA_RATE_OFFSET 5
-#define SN_SYNC_POLARITY_OFFSET7
-
-#define SN_ENABLE_VID_STREAM_BIT   BIT(3)
-#define SN_REFCLK_FREQ_BITSGENMASK(3, 1)
-#define SN_DSIA_NUM_LANES_BITS GENMASK(4, 3)
-#define SN_DP_NUM_LANES_BITS   GENMASK(5, 4)
-#define SN_DP_DATA_RATE_BITS   GENMASK(7, 5)
-#define SN_HPD_DISABLE_BIT BIT(0)
-
 #define SN_REGULATOR_SUPPLY_NUM4

 struct ti_sn_bridge {
@@ -299,8 +298,7 @@ static void ti_sn_bridge_disable(struct drm_bridge 
*bridge)

drm_panel_disable(pdata->panel);

/* disable video stream */
-   regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG,
-  SN_ENABLE_VID_STREAM_BIT, 0);

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

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:

From: Sean Paul 

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

Changes in v3:
- Added to the set

Cc: Sandeep Panda 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 501f4a81ea5ab..d2119ab546147 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -457,8 +457,6 @@ static void ti_sn_bridge_enable(struct drm_bridge 
*bridge)

struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge);
unsigned int val;

-   drm_panel_prepare(pdata->panel);
-
/* DSI_A lane config */
val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
@@ -511,6 +509,8 @@ static void ti_sn_bridge_pre_enable(struct
drm_bridge *bridge)
/* in case drm_panel is connected then HPD is not supported */
regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE,
   HPD_DISABLE);
+
+   drm_panel_prepare(pdata->panel);
 }

 static void ti_sn_bridge_post_disable(struct drm_bridge *bridge)

Reviewed-by: Sandeep Panda 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[spanda]

On 2018-08-14 03:00, Sean Paul wrote:

From: Sean Paul 

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

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

Cc: Sandeep Panda 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/panel/panel-simple.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 5b5d0a24e713e..97964f7f2acee 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1385,6 +1385,9 @@ static const struct panel_desc innolux_tv123wam = 
{

.width = 259,
.height = 173,
},
+   .delay = {
+   .unprepare = 500,
+   },
 };

 static const struct drm_display_mode innolux_zj070na_01p_mode = {

Reviewed-by: Sandeep Panda 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/panel: sn65dsi86: Implement AUX channel

[spanda]

Hi Sean,

Thanks for your patch.
I also made this similar change as part of my PSR support changes (yet 
to be posted for review). But since this patch is posted now, i will 
pick this for my PSR changes.


On 2018-08-09 03:23, Sean Paul wrote:

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

Cc: Sandeep Panda 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 107 --
 1 file changed, 100 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 1b6e8b72be58..50aa8c3c39fc 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -7,12 +7,14 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -29,12 +31,15 @@
 #define SN_DATARATE_CONFIG_REG 0x94
 #define SN_PLL_ENABLE_REG  0x0D
 #define SN_SCRAMBLE_CONFIG_REG 0x95
-#define SN_AUX_WDATA0_REG  0x64
+#define SN_AUX_WDATA_REG(x)(0x64 + x)
 #define SN_AUX_ADDR_19_16_REG  0x74
 #define SN_AUX_ADDR_15_8_REG   0x75
 #define SN_AUX_ADDR_7_0_REG0x76
 #define SN_AUX_LENGTH_REG  0x77
 #define SN_AUX_CMD_REG 0x78
+#define  AUX_CMD_SEND  0x01
+#define  AUX_CMD_REQ(x)(x << 4)
+#define SN_AUX_RDATA_REG(x)(0x79 + x)
 #define SN_ML_TX_MODE_REG  0x96
 /* video config specific registers */
 #define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG  0x20
@@ -64,6 +69,9 @@
 #define SN_DP_DATA_RATE_OFFSET 5
 #define SN_SYNC_POLARITY_OFFSET7

+/* Matches DP_AUX_MAX_PAYLOAD_BYTES (for now) */
+#define SN_AUX_MAX_PAYLOAD_BYTES   16
+
 #define SN_ENABLE_VID_STREAM_BIT   BIT(3)
 #define SN_REFCLK_FREQ_BITSGENMASK(3, 1)
 #define SN_DSIA_NUM_LANES_BITS GENMASK(4, 3)
@@ -76,6 +84,7 @@
 struct ti_sn_bridge {
struct device   *dev;
struct regmap   *regmap;
+   struct drm_dp_aux   aux;
struct drm_bridge   bridge;
struct drm_connectorconnector;
struct device_node  *host_node;
@@ -473,12 +482,7 @@ static void ti_sn_bridge_enable(struct drm_bridge 
*bridge)
 	 * authentication method. We need to enable this method in the eDP 
panel

 * at DisplayPort address 0x0010A prior to link training.
 */
-   regmap_write(pdata->regmap, SN_AUX_WDATA0_REG, 0x01);
-   regmap_write(pdata->regmap, SN_AUX_ADDR_19_16_REG, 0x00);
-   regmap_write(pdata->regmap, SN_AUX_ADDR_15_8_REG, 0x01);
-   regmap_write(pdata->regmap, SN_AUX_ADDR_7_0_REG, 0x0A);
-   regmap_write(pdata->regmap, SN_AUX_LENGTH_REG, 0x01);
-   regmap_write(pdata->regmap, SN_AUX_CMD_REG, 0x81);
+   drm_dp_dpcd_writeb(>aux, DP_EDP_CONFIGURATION_SET, 0);


I think the last argument here should be 0x1 instead of 0. or better put 
it as DP_ALTERNATE_SCRAMBLER_RESET_ENABLE.



usleep_range(1, 10500); /* 10ms delay recommended by spec */


 We can remove this usleep now, as you are already ensuring in the 
newly added function that aux transaction is successful.



/* Semi auto link training mode */
@@ -527,6 +531,90 @@ static const struct drm_bridge_funcs 
ti_sn_bridge_funcs = {

.post_disable = ti_sn_bridge_post_disable,
 };

+static struct ti_sn_bridge *aux_to_ti_sn_bridge(struct drm_dp_aux 
*aux)

+{
+   return container_of(aux, struct ti_sn_bridge, aux);
+}
+
+static bool ti_sn_cmd_done(struct ti_sn_bridge *pdata)
+{
+   int ret;
+   unsigned int val;
+
+   ret = regmap_read(pdata->regmap, SN_AUX_CMD_REG, );


Can we read back register offset 0xF4, instead of 0x78, to check if AUX 
transaction was success or any error has occurred.



+   WARN_ON(ret);
+   return ret || !(val & AUX_CMD_SEND);
+}
+
+static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux,
+ struct drm_dp_aux_msg *msg)
+{
+   struct ti_sn_bridge *pdata = aux_to_ti_sn_bridge(aux);
+   u32 request = msg->request & ~DP_AUX_I2C_MOT;
+   u32 request_val = AUX_CMD_REQ(msg->request);
+   u8 *buf = (u8 *)msg->buffer;
+   bool cmd_done;
+   int ret, i;
+
+   if (msg->size > SN_AUX_MAX_PAYLOAD_BYTES)
+   return -EINVAL;
+
+   switch (request) {
+   case DP_AUX_NATIVE_WRITE:
+   case DP_AUX_I2C_WRITE:
+   case DP_AUX_NATIVE_READ:
+   case DP_AUX_I2C_READ:
+   regmap_write(pdata->regmap, SN_AUX_CMD_REG, request_val);


This regmap_write is not needed since you will be writing it again 
towards the end of this function.



+   

Re: [PATCH] drm/bridge/ti-sn65dsi86: Fix implicit declaration to drm_mode_connector_attach_encoder

[spanda]

On 2018-07-30 23:12, Sean Paul wrote:

This function name was changed to drm_connector_attach_encoder().
Unfortunately this driver was posted on the list before that change, 
and

applied after

Fixes: a095f15c00e2 ("drm/bridge: add support for sn65dsi86 bridge
driver")
Cc: Sandeep Panda 
Cc: Andrzej Hajda 
Cc: Archit Taneja 
Cc: Laurent Pinchart 
Signed-off-by: Sean Paul 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 5b93ef518861..1b6e8b72be58 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -233,7 +233,7 @@ static int ti_sn_bridge_attach(struct drm_bridge 
*bridge)


drm_connector_helper_add(>connector,
 _sn_bridge_connector_helper_funcs);
-	drm_mode_connector_attach_encoder(>connector, 
bridge->encoder);

+   drm_connector_attach_encoder(>connector, bridge->encoder);

/*
 * TODO: ideally finding host resource and dsi dev registration needs

Reviewed-by: Sandeep Panda 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v14 1/2] drm/bridge: add support for sn65dsi86 bridge driver

[spanda]

On 2018-07-19 10:57, Andrzej Hajda wrote:

On 16-Jul-18 17:43, Sandeep Panda wrote:

Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

This chip can be controlled via either i2c interface or
dsi interface. Currently in driver all the control registers
are being accessed through i2c interface only.
Also as of now HPD support has not been added to bridge
chip driver.

Changes in v1:
  - Split the dt-bindings and the driver support into separate patches
(Andrzej Hajda).
  - Use of gpiod APIs to parse and configure gpios instead of obsolete 
ones

(Andrzej Hajda).
  - Use macros to define the register offsets (Andrzej Hajda).

Changes in v2:
  - Separate out edp panel specific HW resource handling from bridge
driver and create a separate edp panel drivers to handle panel
specific mode information and HW resources (Sean Paul).
  - Replace pr_* APIs to DRM_* APIs to log error or debug information
(Sean Paul).
  - Remove some of the unnecessary structure/variable from driver 
(Sean

Paul).
  - Rename the function and structure prefix "sn65dsi86" to 
"ti_sn_bridge"

(Sean Paul / Rob Herring).
  - Remove most of the hard-coding and modified the bridge init 
sequence

based on current mode (Sean Paul).
  - Remove the existing function to retrieve the EDID data and
implemented this as an i2c_adapter and use drm_get_edid() (Sean 
Paul).

  - Remove the dummy irq handler implementation, will add back the
proper irq handling later (Sean Paul).
  - Capture the required enable gpios in a single array based on dt 
entry

instead of having individual descriptor for each gpio (Sean Paul).

Changes in v3:
  - Remove usage of irq_gpio and replace it as "interrupts" property 
(Rob

Herring).
  - Remove the unnecessary header file inclusions (Sean Paul).
  - Rearrange the header files in alphabetical order (Sean Paul).
  - Use regmap interface to perform i2c transactions.
  - Update Copyright/License field and address other review comments
(Jordan Crouse).

Changes in v4:
  - Update License/Copyright (Sean Paul).
  - Add Kconfig and Makefile changes (Sean Paul).
  - Drop i2c gpio handling from this bridge driver, since i2c sda/scl 
gpios

will be handled by i2c master.
  - Update required supplies names.
  - Remove unnecessary goto statements (Sean Paul).
  - Add mutex lock to power_ctrl API to avoid race conditions (Sean
Paul).
  - Add support to parse reference clk frequency from dt(optional).
  - Update the bridge chip enable/disable sequence.

Changes in v5:
  - Fixed Kbuild test service reported warnings.

Changes in v6:
  - Use PM runtime based ref-counting instead of local ref_count 
mechanism

(Stephen Boyd).
  - Clean up some debug logs and indentations (Sean Paul).
  - Simplify dp rate calculation (Sean Paul).
  - Add support to configure refclk based on input REFCLK pin or 
DACP/N

pin (Stephen Boyd).

Changes in v7:
  - Use static supply entries instead of dynamic allocation (Andrzej
Hajda).
  - Defer bridge driver probe if panel is not probed (Andrzej Hajda).
  - Update of_graph APIs for correct node reference management. 
(Andrzej

Hajda).
  - Remove local display_mode object (Andrzej Hajda).
  - Remove version id check function from driver.

Changes in v8:
  - Move dsi register/attach function to bridge driver probe (Andrzej
Hajda).
  - Introduce a new helper function to write 16bit words into 
consecutive

registers (Andrzej Hajda).
  - Remove unnecessary macros (Andrzej Hajda).

Changes in v9:
  - Remove dsi register/attach from bridge probe, since dsi dev 
register
completion also waits for any panel or bridge to get added. This 
creates

deadlock situation when bridge driver calls dsi dev register and
attach before bridge add, in its probe function.
  - Fix issues faced during testing of bridge driver on actual HW.
  - Remove unnecessary initializations (Stephen Boyd).
  - Use local refclk lut size instead of global macro (Sean Paul).

Changes in v10:
  - Use refclk to determine if continuous dsi clock is needed or not.

Changes in v11:
  - Read DPPLL_SRC register to determine continuous clock instead of
using refclk handle (Stephen Boyd).

Changes in v12:
  - Explain in comment as in why dsi dev registration is done in
bridge_attach (Andrzej Hajda).
  - Move HPD disable to bridge_pre_enable (Andrzej Hajda).
  - Make panel/DDC exclusive until HPD support is added (Andrzej 
Hajda).


Changes in v13:
  - eDP panels report EDID via DP-AUX channel, so remove support for
dedicated DDC line (Andrzej Hajda).

Signed-off-by: Sandeep Panda 
---
  drivers/gpu/drm/bridge/Kconfig|   9 +
  drivers/gpu/drm/bridge/Makefile   |   1 +
  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 676 
++

  3 files changed, 686 insertions(+)
  create mode 100644 

Re: [PATCH v13 3/3] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[spanda]

On 2018-06-29 17:31, Andrzej Hajda wrote:

On 27.06.2018 11:57, Sandeep Panda wrote:

Document the bindings used for the sn65dsi86 DSI to eDP bridge.

Changes in v1:
 - Rephrase the dt-binding descriptions to be more inline with 
existing

   bindings (Andrzej Hajda).
 - Add missing dt-binding that are parsed by corresponding driver
   (Andrzej Hajda).

Changes in v2:
 - Remove edp panel specific dt-binding entries. Only keep bridge
   specific entries (Sean Paul).
 - Remove custom-modes dt entry since its usage is removed from driver 
also (Sean Paul).
 - Remove is-pluggable dt entry since this will not be needed anymore 
(Sean Paul).


Changes in v3:
 - Remove irq-gpio dt entry and instead populate is an interrupt
   property (Rob Herring).

Changes in v4:
 - Add link to bridge chip datasheet (Stephen Boyd)
 - Add vpll and vcc regulator supply bindings (Stephen Boyd)
 - Add ref clk optional dt binding (Stephen Boyd)
 - Add gpio-controller optional dt binding (Stephen Boyd)

Changes in v5:
 - Use clock property to specify the input refclk (Stephen Boyd).
 - Update gpio cell and pwm cell numbers (Stephen Boyd).

Changes in v6:
 - Add property to mention the lane mapping scheme and polarity 
inversion

   (Stephen Boyd).

Changes in v7:
 - Detail description of lane mapping scheme dt property (Andrzej
   Hajda/ Rob Herring).
 - Removed HDP gpio binding, since the bridge uses IRQ signal to
   determine HPD, and IRQ property is already documented in binding.

Changes in v8:
 - Removed unnecessary explanation of lane mapping and polarity dt
   property, since these are already explained in 
media/video-interface

   dt binidng (Rob Herring).

Changes in v9:
 - Avoid putting re-definition of lane mapping and polarity dt binding
   (Rob Herring).

Changes in v10:
 - Use interrupts-extended property instead of interrupts to specify
   interrupt line (Andrzej Hajda).
 - Move data-lanes and lane-polarity property example to proper place 
(Andrzej Hajda).


Changes in v11:
 - Add a property for suspend gpio function of GPIO1 pin on bridge 
chip

   (Stephen Boyd).

Signed-off-by: Sandeep Panda 
---
 .../bindings/display/bridge/ti,sn65dsi86.txt   | 89 
++

 1 file changed, 89 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt


diff --git 
a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

new file mode 100644
index ..6787f5f2c7cd
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

@@ -0,0 +1,89 @@
+SN65DSI86 DSI to eDP bridge chip
+
+
+This is the binding for Texas Instruments SN65DSI86 bridge.
+http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86=pdf
+
+Required properties:
+- compatible: Must be "ti,sn65dsi86"
+- reg: i2c address of the chip, 0x2d as per datasheet
+- enable-gpios: OF device-tree gpio specification for bridge_en pin 
(active high)

+
+- vccio-supply: A 1.8V supply that powers up the digital IOs.
+- vpll-supply: A 1.8V supply that powers up the displayport PLL.
+- vcca-supply: A 1.2V supply that powers up the analog circuits.
+- vcc-supply: A 1.2V supply that powers up the digital core.
+
+Optional properties:
+- interrupts-extended: Specifier for the SN65DSI86 interrupt line.
+
+- ddc-i2c-bus: phandle of the I2C controller used for DDC EDID 
probing


One more thing, I have overlooked: why do you need this property? eDP
panels passes its EDID via DP-AUX channel, do you have some strange
panel?


OK i will remove this property in next patchset.


 I have looked for the panel you want to add in this patchset -

"Innolux TV123WAM" and found nothing, there is TV123WAM made by BOE (it
is the same panel??) and it reports EDID via DP-AUX, no I2C lines for 
DDC.


Regards
Andrzej


+
+- gpio-controller: Marks the device has a GPIO controller.
+- #gpio-cells: Should be two. The first cell is the pin number 
and

+   the second cell is used to specify flags.
+   See ../../gpio/gpio.txt for more information.
+- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description 
of

+   the cell formats.
+
+- clock-names: should be "refclk"
+- clocks: Specification for input reference clock. The reference
+ clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
+
+- data-lanes: See ../../media/video-interface.txt
+- lane-polarities: See ../../media/video-interface.txt
+
+- suspend-gpios: OF device-tree specification for GPIO1 pin on bridge 
(active low)

+
+Required nodes:
+This device has two video ports. Their connections are modelled using 
the
+OF graph bindings specified in 
Documentation/devicetree/bindings/graph.txt.

+
+- Video port 0 for DSI input
+- Video port 1 for eDP output
+
+Example
+---
+
+edp-bridge@2d {
+   compatible = "ti,sn65dsi86";
+   

Re: [PATCH v12 5/5] dt-bindings: drm/panel: Document Innolux TV123WAM panel bindings

[spanda]

On 2018-07-05 15:48, Andrzej Hajda wrote:

On 05.07.2018 08:38, spa...@codeaurora.org wrote:

On 2018-06-29 17:44, Andrzej Hajda wrote:

On 21.06.2018 14:32, Sandeep Panda wrote:

Innolux TV123WAM is a 12.3" eDP display panel with
2160x1440 resolution, which can be supported by simple
panel driver.

Are you sure this is Innolux? Quick grep on Internet finds only BOE
panel with this TV123WAM[1].

[1]:
https://e2e.ti.com/cfs-file/__key/communityserver-discussions-components-files/138/TV123WAM_2D00_ND0_5F00_Product-spec_5F00_BOE_5F00_20161115_2D00_A00.pdf


The panel used here is innolux, which is a 2k panel. Where the BOE one
is a 1080p panel.


My doubts comes from the fact that Innolux uses different convention 
for

panel naming, see for example [1].

According to this convention your panel should have name starting with:
N123ZDG-

TV123WAM follows BOE's convention.

Do you have datasheet, physical panel or something to ensure the name 
is

correct?

[1]:
http://www.panelook.com/bramodlist.php?st==[]=63_type_category=70

Regards
Andrzej

The data sheet i have does not mention any version info/id of innolux 
panel, so i took the
version name from a down stream patch we had earlier used to bring up 
this panel.
Unfortunately i can not share the data sheet also as it is water marked 
as confidential.


This panel patches are already merged in drm-misc, will upload a new 
patch to remove tv123* name

and put some generic innolux panel name like innolux,2k





Regards
Andrzej

Changes in v1:
 - Make use of simple panel driver instead of creating
   a new driver for this panel (Sean Paul).
 - Combine dt-binding and driver changes into one patch
   as done by other existing panel support changes.

Changes in v2:
 - Separate driver change from dt-binding documentation (Rob 
Herring).
 - Add the properties from simple-panel binding that are applicable 
to

   this panel (Rob Herring).

Signed-off-by: Sandeep Panda 
Reviewed-by: Rob Herring 
---
 .../bindings/display/panel/innolux,tv123wam.txt  | 20

 1 file changed, 20 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt

diff --git
a/Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt
b/Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt
new file mode 100644
index ..a9b35265fa13
--- /dev/null
+++
b/Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt
@@ -0,0 +1,20 @@
+Innolux TV123WAM 12.3 inch eDP 2K display panel
+
+This binding is compatible with the simple-panel binding, which is
specified
+in simple-panel.txt in this directory.
+
+Required properties:
+- compatible: should be "innolux,tv123wam"
+- power-supply: regulator to provide the supply voltage
+
+Optional properties:
+- enable-gpios: GPIO pin to enable or disable the panel
+- backlight: phandle of the backlight device attached to the panel
+
+Example:
+   panel_edp: panel-edp {
+   compatible = "innolux,tv123wam";
+   enable-gpios = < 31 GPIO_ACTIVE_LOW>;
+   power-supply = <_l2>;
+   backlight = <>;
+   };




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

Re: [PATCH v12 5/5] dt-bindings: drm/panel: Document Innolux TV123WAM panel bindings

[spanda]

On 2018-06-29 17:44, Andrzej Hajda wrote:

On 21.06.2018 14:32, Sandeep Panda wrote:

Innolux TV123WAM is a 12.3" eDP display panel with
2160x1440 resolution, which can be supported by simple
panel driver.


Are you sure this is Innolux? Quick grep on Internet finds only BOE
panel with this TV123WAM[1].

[1]:
https://e2e.ti.com/cfs-file/__key/communityserver-discussions-components-files/138/TV123WAM_2D00_ND0_5F00_Product-spec_5F00_BOE_5F00_20161115_2D00_A00.pdf

The panel used here is innolux, which is a 2k panel. Where the BOE one 
is a 1080p panel.



Regards
Andrzej


Changes in v1:
 - Make use of simple panel driver instead of creating
   a new driver for this panel (Sean Paul).
 - Combine dt-binding and driver changes into one patch
   as done by other existing panel support changes.

Changes in v2:
 - Separate driver change from dt-binding documentation (Rob Herring).
 - Add the properties from simple-panel binding that are applicable to
   this panel (Rob Herring).

Signed-off-by: Sandeep Panda 
Reviewed-by: Rob Herring 
---
 .../bindings/display/panel/innolux,tv123wam.txt  | 20 


 1 file changed, 20 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt


diff --git 
a/Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt 
b/Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt

new file mode 100644
index ..a9b35265fa13
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/display/panel/innolux,tv123wam.txt

@@ -0,0 +1,20 @@
+Innolux TV123WAM 12.3 inch eDP 2K display panel
+
+This binding is compatible with the simple-panel binding, which is 
specified

+in simple-panel.txt in this directory.
+
+Required properties:
+- compatible: should be "innolux,tv123wam"
+- power-supply: regulator to provide the supply voltage
+
+Optional properties:
+- enable-gpios: GPIO pin to enable or disable the panel
+- backlight: phandle of the backlight device attached to the panel
+
+Example:
+   panel_edp: panel-edp {
+   compatible = "innolux,tv123wam";
+   enable-gpios = < 31 GPIO_ACTIVE_LOW>;
+   power-supply = <_l2>;
+   backlight = <>;
+   };

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

Re: [PATCH v12 3/5] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[spanda]

On 2018-06-22 06:42, Stephen Boyd wrote:

Quoting Sandeep Panda (2018-06-21 05:32:07)
diff --git 
a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

new file mode 100644
index ..c8b8f018356f
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

@@ -0,0 +1,86 @@
+SN65DSI86 DSI to eDP bridge chip
+
+
+This is the binding for Texas Instruments SN65DSI86 bridge.
+http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86=pdf
+
+Required properties:
+- compatible: Must be "ti,sn65dsi86"
+- reg: i2c address of the chip, 0x2d as per datasheet
+- enable-gpios: OF device-tree gpio specification for bridge_en pin 
(active high)

+
+- vccio-supply: A 1.8V supply that powers up the digital IOs.
+- vpll-supply: A 1.8V supply that powers up the displayport PLL.
+- vcca-supply: A 1.2V supply that powers up the analog circuits.
+- vcc-supply: A 1.2V supply that powers up the digital core.
+
+Optional properties:
+- interrupts-extended: Specifier for the SN65DSI86 interrupt line.
+
+- ddc-i2c-bus: phandle of the I2C controller used for DDC EDID 
probing

+
+- gpio-controller: Marks the device has a GPIO controller.
+- #gpio-cells: Should be two. The first cell is the pin number 
and

+   the second cell is used to specify flags.
+   See ../../gpio/gpio.txt for more information.
+- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description 
of

+   the cell formats.
+
+- clock-names: should be "refclk"
+- clocks: Specification for input reference clock. The reference
+ clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 
MHz.

+
+- data-lanes: See ../../media/video-interface.txt
+- lane-polarities: See ../../media/video-interface.txt


We need another property for suspend-gpios function of GPIO1. I suppose
for the other GPIOs we don't need anything like this because they're
output functions only? GPIO4 can do PWM and I guess if pwm is used in 
DT

from here then the driver can mux that out of gpio4 properly. I have no
idea how the hsync and vsync GPIOs would work though. I see that GPIO3
can do DSIA hsync or vsync and GPIO2 can do DSIA vsync and I would 
guess

those are in output mode only. Maybe that's just another property on
this node to indicate if we should mux out the function or not. Usually
that's done from pinctrl though. I don't have a use case for those
functions but I do care about suspend gpios so at least add that one
please.


I was thinking of adding gpio1 (suspend-gpio) documentation when we add
support for PSR feature. Since for PSR we will be using this 
suspoend-gpio.


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

Re: [PATCH v11 2/5] drm/bridge: add support for sn65dsi86 bridge driver

[spanda]

On 2018-06-15 16:59, Andrzej Hajda wrote:

Hi Sandeep,

Thanks for the patch, I hope it will be merged soon.

On 15.06.2018 08:43, Sandeep Panda wrote:

Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

This chip can be controlled via either i2c interface or
dsi interface. Currently in driver all the control registers
are being accessed through i2c interface only.
Also as of now HPD support has not been added to bridge
chip driver.

Changes in v1:
 - Split the dt-bindings and the driver support into separate patches
   (Andrzej Hajda).
 - Use of gpiod APIs to parse and configure gpios instead of obsolete 
ones

   (Andrzej Hajda).
 - Use macros to define the register offsets (Andrzej Hajda).

Changes in v2:
 - Separate out edp panel specific HW resource handling from bridge
   driver and create a separate edp panel drivers to handle panel
   specific mode information and HW resources (Sean Paul).
 - Replace pr_* APIs to DRM_* APIs to log error or debug information
   (Sean Paul).
 - Remove some of the unnecessary structure/variable from driver (Sean
   Paul).
 - Rename the function and structure prefix "sn65dsi86" to 
"ti_sn_bridge"

   (Sean Paul / Rob Herring).
 - Remove most of the hard-coding and modified the bridge init 
sequence

   based on current mode (Sean Paul).
 - Remove the existing function to retrieve the EDID data and
   implemented this as an i2c_adapter and use drm_get_edid() (Sean 
Paul).

 - Remove the dummy irq handler implementation, will add back the
   proper irq handling later (Sean Paul).
 - Capture the required enable gpios in a single array based on dt 
entry

   instead of having individual descriptor for each gpio (Sean Paul).

Changes in v3:
 - Remove usage of irq_gpio and replace it as "interrupts" property 
(Rob

   Herring).
 - Remove the unnecessary header file inclusions (Sean Paul).
 - Rearrange the header files in alphabetical order (Sean Paul).
 - Use regmap interface to perform i2c transactions.
 - Update Copyright/License field and address other review comments
   (Jordan Crouse).

Changes in v4:
 - Update License/Copyright (Sean Paul).
 - Add Kconfig and Makefile changes (Sean Paul).
 - Drop i2c gpio handling from this bridge driver, since i2c sda/scl 
gpios

   will be handled by i2c master.
 - Update required supplies names.
 - Remove unnecessary goto statements (Sean Paul).
 - Add mutex lock to power_ctrl API to avoid race conditions (Sean
   Paul).
 - Add support to parse reference clk frequency from dt(optional).
 - Update the bridge chip enable/disable sequence.

Changes in v5:
 - Fixed Kbuild test service reported warnings.

Changes in v6:
 - Use PM runtime based ref-counting instead of local ref_count 
mechanism

   (Stephen Boyd).
 - Clean up some debug logs and indentations (Sean Paul).
 - Simplify dp rate calculation (Sean Paul).
 - Add support to configure refclk based on input REFCLK pin or DACP/N
   pin (Stephen Boyd).

Changes in v7:
 - Use static supply entries instead of dynamic allocation (Andrzej
   Hajda).
 - Defer bridge driver probe if panel is not probed (Andrzej Hajda).
 - Update of_graph APIs for correct node reference management. 
(Andrzej

   Hajda).
 - Remove local display_mode object (Andrzej Hajda).
 - Remove version id check function from driver.

Changes in v8:
 - Move dsi register/attach function to bridge driver probe (Andrzej
   Hajda).
 - Introduce a new helper function to write 16bit words into 
consecutive

   registers (Andrzej Hajda).
 - Remove unnecessary macros (Andrzej Hajda).

Changes in v9:
 - Remove dsi register/attach from bridge probe, since dsi dev 
register
   completion also waits for any panel or bridge to get added. This 
creates

   deadlock situation when bridge driver calls dsi dev register and
   attach before bridge add, in its probe function.
 - Fix issues faced during testing of bridge driver on actual HW.
 - Remove unnecessary initializations (Stephen Boyd).
 - Use local refclk lut size instead of global macro (Sean Paul).

Changes in v10:
 - Use refclk to determine if continuous dsi clock is needed or not.

Changes in v11:
 - Read DPPLL_SRC register to determine continuous clock instead of
   using refclk handle (Stephen Boyd).

Signed-off-by: Sandeep Panda 
---
 drivers/gpu/drm/bridge/Kconfig|   9 +
 drivers/gpu/drm/bridge/Makefile   |   1 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 696 
++

 3 files changed, 706 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/Kconfig 
b/drivers/gpu/drm/bridge/Kconfig

index 3b99d5a..8153150 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -108,6 +108,15 @@ config DRM_TI_TFP410
---help---
  Texas Instruments TFP410 DVI/HDMI Transmitter driver

+config DRM_TI_SN65DSI86
+   tristate "TI SN65DSI86 DSI to eDP bridge"

Re: [PATCH v11 3/5] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[spanda]

On 2018-06-15 12:53, Andrzej Hajda wrote:

On 15.06.2018 08:43, Sandeep Panda wrote:

Document the bindings used for the sn65dsi86 DSI to eDP bridge.

Changes in v1:
 - Rephrase the dt-binding descriptions to be more inline with 
existing

   bindings (Andrzej Hajda).
 - Add missing dt-binding that are parsed by corresponding driver
   (Andrzej Hajda).

Changes in v2:
 - Remove edp panel specific dt-binding entries. Only keep bridge
   specific entries (Sean Paul).
 - Remove custom-modes dt entry since its usage is removed from driver 
also (Sean Paul).
 - Remove is-pluggable dt entry since this will not be needed anymore 
(Sean Paul).


Changes in v3:
 - Remove irq-gpio dt entry and instead populate is an interrupt
   property (Rob Herring).

Changes in v4:
 - Add link to bridge chip datasheet (Stephen Boyd)
 - Add vpll and vcc regulator supply bindings (Stephen Boyd)
 - Add ref clk optional dt binding (Stephen Boyd)
 - Add gpio-controller optional dt binding (Stephen Boyd)

Changes in v5:
 - Use clock property to specify the input refclk (Stephen Boyd).
 - Update gpio cell and pwm cell numbers (Stephen Boyd).

Changes in v6:
 - Add property to mention the lane mapping scheme and polarity 
inversion

   (Stephen Boyd).

Changes in v7:
 - Detail description of lane mapping scheme dt property (Andrzej
   Hajda/ Rob Herring).
 - Removed HDP gpio binding, since the bridge uses IRQ signal to
   determine HPD, and IRQ property is already documented in binding.

Changes in v8:
 - Removed unnecessary explanation of lane mapping and polarity dt
   property, since these are already explained in 
media/video-interface

   dt binidng (Rob Herring).

Changes in v9:
 - Avoid putting re-definition of lane mapping and polarity dt binding
   (Rob Herring).

Signed-off-by: Sandeep Panda 
---
 .../bindings/display/bridge/ti,sn65dsi86.txt   | 87 
++

 1 file changed, 87 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt


diff --git 
a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

new file mode 100644
index 000..507efbb
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

@@ -0,0 +1,87 @@
+SN65DSI86 DSI to eDP bridge chip
+
+
+This is the binding for Texas Instruments SN65DSI86 bridge.
+http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86=pdf
+
+Required properties:
+- compatible: Must be "ti,sn65dsi86"
+- reg: i2c address of the chip, 0x2d as per datasheet
+- enable-gpios: OF device-tree gpio specification for bridge_en pin 
(active high)

+
+- vccio-supply: A 1.8V supply that powers up the digital IOs.
+- vpll-supply: A 1.8V supply that powers up the displayport PLL.
+- vcca-supply: A 1.2V supply that powers up the analog circuits.
+- vcc-supply: A 1.2V supply that powers up the digital core.


Since there are only two voltage levels (1.8 and 1.2) and power on/off
sequence does not require specific order I guess you could merge
supplies with the same level, but this is just an idea, up to you.
For example you can drop (or make optional) vpll-supply and 
vcca-supply.

+


Since the bridge datasheet mentions about 4 different power supplies and 
there
was also comment from other reviewers to keep all of them so i have put 
it like this.
Also this will help in scenarios where board design has 4 different 
sources for these 4

power supplies.


+Optional properties:
+- interrupts: Specifier for the SN65DSI86 interrupt line.


or interrupts-extended


+
+- ddc-i2c-bus: phandle of the I2C controller used for DDC EDID 
probing

+
+- gpio-controller: Marks the device has a GPIO controller.
+- #gpio-cells: Should be two. The first cell is the pin number 
and

+   the second cell is used to specify flags.
+   See ../../gpio/gpio.txt for more information.
+- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description 
of

+   the cell formats.
+
+- clock-names: should be "refclk"
+- clocks: Specification for input reference clock. The reference
+ clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
+
+- data-lanes: See ../../media/video-interface.txt
+- lane-polarities: See ../../media/video-interface.txt


Either you should describe which port these properties are related to,
either put them into endpoint node. According to
./../media/video-interface.txt only the latter is correct.



Data lane and lane-polarity is applicable to both DSI and eDP interface. 
I have
updated the same in ../../media/video-interface.txt. Do you want to 
explicitly

mention here that this property is for eDP data lanes mapping?


+
+Required nodes:
+This device has two video ports. Their connections are modelled using 
the
+OF graph bindings specified in 
Documentation/devicetree/bindings/graph.txt.

+
+- Video port 0 for 

Re: [PATCH v8 1/4] drm/bridge: add support for sn65dsi86 bridge driver

[spanda]

On 2018-06-06 04:29, Sean Paul wrote:

On Tue, Jun 05, 2018 at 11:10:15AM +0530, Sandeep Panda wrote:

Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

This chip can be controlled via either i2c interface or
dsi interface. Currently in driver all the control registers
are being accessed through i2c interface only.
Also as of now HPD support has not been added to bridge
chip driver.

Changes in v1:
 - Split the dt-bindings and the driver support into separate patches
   (Andrzej Hajda).
 - Use of gpiod APIs to parse and configure gpios instead of obsolete 
ones

   (Andrzej Hajda).
 - Use macros to define the register offsets (Andrzej Hajda).

Changes in v2:
 - Separate out edp panel specific HW resource handling from bridge
   driver and create a separate edp panel drivers to handle panel
   specific mode information and HW resources (Sean Paul).
 - Replace pr_* APIs to DRM_* APIs to log error or debug information
   (Sean Paul).
 - Remove some of the unnecessary structure/variable from driver (Sean
   Paul).
 - Rename the function and structure prefix "sn65dsi86" to 
"ti_sn_bridge"

   (Sean Paul / Rob Herring).
 - Remove most of the hard-coding and modified the bridge init 
sequence

   based on current mode (Sean Paul).
 - Remove the existing function to retrieve the EDID data and
   implemented this as an i2c_adapter and use drm_get_edid() (Sean 
Paul).

 - Remove the dummy irq handler implementation, will add back the
   proper irq handling later (Sean Paul).
 - Capture the required enable gpios in a single array based on dt 
entry

   instead of having individual descriptor for each gpio (Sean Paul).

Changes in v3:
 - Remove usage of irq_gpio and replace it as "interrupts" property 
(Rob

   Herring).
 - Remove the unnecessary header file inclusions (Sean Paul).
 - Rearrange the header files in alphabetical order (Sean Paul).
 - Use regmap interface to perform i2c transactions.
 - Update Copyright/License field and address other review comments
   (Jordan Crouse).

Changes in v4:
 - Update License/Copyright (Sean Paul).
 - Add Kconfig and Makefile changes (Sean Paul).
 - Drop i2c gpio handling from this bridge driver, since i2c sda/scl 
gpios

   will be handled by i2c master.
 - Update required supplies names.
 - Remove unnecessary goto statements (Sean Paul).
 - Add mutex lock to power_ctrl API to avoid race conditions (Sean
   Paul).
 - Add support to parse reference clk frequency from dt(optional).
 - Update the bridge chip enable/disable sequence.

Changes in v5:
 - Fixed Kbuild test service reported warnings.

Changes in v6:
 - Use PM runtime based ref-counting instead of local ref_count 
mechanism

   (Stephen Boyd).
 - Clean up some debug logs and indentations (Sean Paul).
 - Simplify dp rate calculation (Sean Paul).
 - Add support to configure refclk based on input REFCLK pin or DACP/N
   pin (Stephen Boyd).

Changes in v7:
 - Use static supply entries instead of dynamic allocation (Andrzej
   Hajda).
 - Defer bridge driver probe if panel is not probed (Andrzej Hajda).
 - Update of_graph APIs for correct node reference management. 
(Andrzej

   Hajda).
 - Remove local display_mode object (Andrzej Hajda).
 - Remove version id check function from driver.

Changes in v8:
 - Move dsi register/attach function to bridge driver probe (Andrzej
   Hajda).
 - Introduce a new helper function to write 16bit words into 
consecutive

   registers (Andrzej Hajda).
 - Remove unnecessary macros (Andrzej Hajda).

Signed-off-by: Sandeep Panda 
---
 drivers/gpu/drm/bridge/Kconfig|   9 +
 drivers/gpu/drm/bridge/Makefile   |   1 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 666 
++

 3 files changed, 676 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/Kconfig 
b/drivers/gpu/drm/bridge/Kconfig

index 3b99d5a..8153150 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -108,6 +108,15 @@ config DRM_TI_TFP410
---help---
  Texas Instruments TFP410 DVI/HDMI Transmitter driver

+config DRM_TI_SN65DSI86
+   tristate "TI SN65DSI86 DSI to eDP bridge"
+   depends on OF
+   select DRM_KMS_HELPER
+   select REGMAP_I2C
+   select DRM_PANEL
+   ---help---
+ Texas Instruments SN65DSI86 DSI to eDP Bridge driver
+
 source "drivers/gpu/drm/bridge/analogix/Kconfig"

 source "drivers/gpu/drm/bridge/adv7511/Kconfig"
diff --git a/drivers/gpu/drm/bridge/Makefile 
b/drivers/gpu/drm/bridge/Makefile

index 373eb28..3711be8 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -12,4 +12,5 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
 obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
 obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
+obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o


SN65DSI86 

Re: [PATCH v8 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[spanda]

On 2018-06-05 20:50, Rob Herring wrote:

On Tue, Jun 05, 2018 at 11:10:16AM +0530, Sandeep Panda wrote:

Document the bindings used for the sn65dsi86 DSI to eDP bridge.

Changes in v1:
 - Rephrase the dt-binding descriptions to be more inline with 
existing

   bindings (Andrzej Hajda).
 - Add missing dt-binding that are parsed by corresponding driver
   (Andrzej Hajda).

Changes in v2:
 - Remove edp panel specific dt-binding entries. Only keep bridge
   specific entries (Sean Paul).
 - Remove custom-modes dt entry since its usage is removed from driver 
also (Sean Paul).
 - Remove is-pluggable dt entry since this will not be needed anymore 
(Sean Paul).


Changes in v3:
 - Remove irq-gpio dt entry and instead populate is an interrupt
   property (Rob Herring).

Changes in v4:
 - Add link to bridge chip datasheet (Stephen Boyd)
 - Add vpll and vcc regulator supply bindings (Stephen Boyd)
 - Add ref clk optional dt binding (Stephen Boyd)
 - Add gpio-controller optional dt binding (Stephen Boyd)

Changes in v5:
 - Use clock property to specify the input refclk (Stephen Boyd).
 - Update gpio cell and pwm cell numbers (Stephen Boyd).

Changes in v6:
 - Add property to mention the lane mapping scheme and polarity 
inversion

   (Stephen Boyd).

Changes in v7:
 - Detail description of lane mapping scheme dt property (Andrzej
   Hajda/ Rob Herring).
 - Removed HDP gpio binding, since the bridge uses IRQ signal to
   determine HPD, and IRQ property is already documented in binding.

Signed-off-by: Sandeep Panda 
---
 .../bindings/display/bridge/ti,sn65dsi86.txt   | 109 
+

 1 file changed, 109 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt


diff --git 
a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

new file mode 100644
index 000..33329f9
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

@@ -0,0 +1,109 @@
+SN65DSI86 DSI to eDP bridge chip
+
+
+This is the binding for Texas Instruments SN65DSI86 bridge.
+http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86=pdf
+
+Required properties:
+- compatible: Must be "ti,sn65dsi86"
+- reg: i2c address of the chip, 0x2d as per datasheet
+- enable-gpios: OF device-tree gpio specification for bridge_en pin 
(active high)

+
+- vccio-supply: A 1.8V supply that powers up the digital IOs.
+- vpll-supply: A 1.8V supply that powers up the displayport PLL.
+- vcca-supply: A 1.2V supply that powers up the analog circuits.
+- vcc-supply: A 1.2V supply that powers up the digital core.
+
+Optional properties:
+- interrupts: Specifier for the SN65DSI86 interrupt line.
+
+- ddc-i2c-bus: phandle of the I2C controller used for DDC EDID 
probing

+
+- gpio-controller: Marks the device has a GPIO controller.
+- #gpio-cells: Should be two. The first cell is the pin number 
and

+   the second cell is used to specify flags.
+   See ../../gpio/gpio.txt for more information.
+- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description 
of

+   the cell formats.
+
+- clock-names: should be "refclk"
+- clocks: Specification for input reference clock. The reference
+ clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
+
+- lane-mapping: Specification to describe the logical to physical 
lane


As I mentioned in v7, we already have a property for this. It's called
'data-lanes' and defined in media/video-interfaces.txt. Use that. If 
you

need polarity too, then add a property for that. And add it to
video-interfaces.txt.



Ok. modified in next patchset.


+   mapping scheme and polarity inversion of eDP lanes on PCB.
+   Each pair present at index n (where n lies between 0 and 3)
+   describes the lane mapping of logical lane to physical lane n
+		and the polarity(it should be either 1 or 0) of the physical lane 
n.

+
+   For example:
+   lane-mapping = <2 1>,
+   <1 0>,
+   <3 1>,
+   <0 0>;
+
+   The above mapping describes that logical lane 2 is mapped to
+   physical lane 0 and polarity of physical lane 0 is inverted,
+   logical lane 1 is mapped to physical lane 1 and polarity of
+   physical lane 1 is normal, logical lane 3 is mapped to physical
+   lane 2 and polarity of physical lane 2 is inverted, logical 
lane 0
+		is mapped to physical lane 4 and polarity of physical lane 3 is 
normal.

+
+   If this property is not mentioned then it is assumed that 
physical
+		lanes 0 through 3 are mapped to logical lanes 0 through 3 and 
polarity

+   of all physical lanes is normal.
+
+Required nodes:
+This device has two video ports. 

Re: [PATCH v8 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[spanda]

On 2018-06-12 14:01, Stephen Boyd wrote:

Quoting spa...@codeaurora.org (2018-06-05 21:50:16)

On 2018-06-05 20:50, Rob Herring wrote:
> On Tue, Jun 05, 2018 at 11:10:16AM +0530, Sandeep Panda wrote:
>> Document the bindings used for the sn65dsi86 DSI to eDP bridge.

[...]

>> and
>> +   the second cell is used to specify flags.
>> +   See ../../gpio/gpio.txt for more information.
>> +- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description
>> of
>> +   the cell formats.
>> +
>> +- clock-names: should be "refclk"
>> +- clocks: Specification for input reference clock. The reference
>> +  clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
>> +
>> +- lane-mapping: Specification to describe the logical to physical
>> lane
>
> As I mentioned in v7, we already have a property for this. It's called
> 'data-lanes' and defined in media/video-interfaces.txt. Use that. If
> you
> need polarity too, then add a property for that. And add it to
> video-interfaces.txt.

The data-lanes property mentioned in media/video-interfaces.txt is
referring
to DSI/CSI lanes where assumption is clock lane is fixed at index 0. 
But

here
the we want to mention about eDP lanes which do not have dedicated 
clock

lane.
So can we still use the existing data-lanes property here?


Why is that a problem? It's just a property name.

There are data-lanes and clock-lanes properties in the
video-interfaces.txt file by the way. It would be nice if that document
could be updated for displayport and DSI (e.g.  clock-noncontinuous or
link-frequencies) or even just mention in there that these can apply to
DSI and displayport too.


For this current review, i have modified the property description to 
point to media/video.txt for explanation.

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

Re: [PATCH v8 1/4] drm/bridge: add support for sn65dsi86 bridge driver

[spanda]

On 2018-06-12 05:05, Stephen Boyd wrote:

Quoting Sandeep Panda (2018-06-04 22:40:15)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c

new file mode 100644
index 000..add6e0f
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -0,0 +1,666 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+

[...]

+
+static const struct regmap_config ti_sn_bridge_regmap_config = {
+   .reg_bits = 8,
+   .val_bits = 8,
+   .volatile_table = _sn_bridge_volatile_table,
+   .cache_type = REGCACHE_NONE,
+};
+
+static void ti_sn_bridge_write_u16(struct ti_sn_bridge *pdata,
+  unsigned int reg, u16 val)
+{
+   regmap_write(pdata->regmap, reg, val & 0xFF);
+   regmap_write(pdata->regmap, reg + 1, val >> 8);
+}
+
+static int __maybe_unused ti_sn_bridge_resume(struct device *dev)
+{
+   struct ti_sn_bridge *pdata = dev_get_drvdata(dev);
+   int ret = 0;


Please don't assign variables and then reassign them again immediately
after. It hides use before real initialization bugs.


Ok.


+
+   ret = regulator_bulk_enable(SN_REGULATOR_SUPPLY_NUM, 
pdata->supplies);

[...]

+
+static int ti_sn_bridge_probe(struct i2c_client *client,
+const struct i2c_device_id *id)
+{
+   struct ti_sn_bridge *pdata;
+   struct device_node *ddc_node;
+   struct mipi_dsi_host *host;
+   struct mipi_dsi_device *dsi;
+   int ret = 0;
+   const struct mipi_dsi_device_info info = { .type = 
"ti_sn_bridge",

+  .channel = 0,
+  .node = NULL,
+};
+
+   if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+   DRM_ERROR("device doesn't support I2C\n");
+   return -ENODEV;
+   }
+
+   ret = ti_sn_bridge_parse_dsi_host(pdata);
+   if (ret)
+   return ret;
+
+   host = of_find_mipi_dsi_host_by_node(pdata->host_node);
+   if (!host) {
+   DRM_ERROR("failed to find dsi host\n");


Not sure we want to print an error and then return -EPROBE_DEFER.
Usually EPROBE_DEFER is silent.


+   ret = -EPROBE_DEFER;
+   goto err_dsi_host;
+   }
+

[...]

+
+   /* TODO: setting to 4 lanes always for now */
+   dsi->lanes = 4;
+   dsi->format = MIPI_DSI_FMT_RGB888;
+   dsi->mode_flags = MIPI_DSI_MODE_VIDEO | 
MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
+ MIPI_DSI_MODE_EOT_PACKET | 
MIPI_DSI_MODE_VIDEO_HSE;

+
+   ret = mipi_dsi_attach(dsi);
+   if (ret < 0) {
+   DRM_ERROR("failed to attach dsi to host\n");
+   goto err_dsi_attach;
+   }
+   pdata->dsi = dsi;
+
+   pdata->refclk = devm_clk_get(pdata->dev, "refclk");


We need to check for error

if (IS_ERR(pdata->refclk))

And then if it's EPROBE_DEFER I suppose we would bail out, otherwise
assume it's not present?


Yes found this issue while testing the driver on actual sn65dsi86 HW, i 
will fix this in next patchset.

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

Re: [PATCH v8 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[spanda]

On 2018-06-05 20:50, Rob Herring wrote:

On Tue, Jun 05, 2018 at 11:10:16AM +0530, Sandeep Panda wrote:

Document the bindings used for the sn65dsi86 DSI to eDP bridge.

Changes in v1:
 - Rephrase the dt-binding descriptions to be more inline with 
existing

   bindings (Andrzej Hajda).
 - Add missing dt-binding that are parsed by corresponding driver
   (Andrzej Hajda).

Changes in v2:
 - Remove edp panel specific dt-binding entries. Only keep bridge
   specific entries (Sean Paul).
 - Remove custom-modes dt entry since its usage is removed from driver 
also (Sean Paul).
 - Remove is-pluggable dt entry since this will not be needed anymore 
(Sean Paul).


Changes in v3:
 - Remove irq-gpio dt entry and instead populate is an interrupt
   property (Rob Herring).

Changes in v4:
 - Add link to bridge chip datasheet (Stephen Boyd)
 - Add vpll and vcc regulator supply bindings (Stephen Boyd)
 - Add ref clk optional dt binding (Stephen Boyd)
 - Add gpio-controller optional dt binding (Stephen Boyd)

Changes in v5:
 - Use clock property to specify the input refclk (Stephen Boyd).
 - Update gpio cell and pwm cell numbers (Stephen Boyd).

Changes in v6:
 - Add property to mention the lane mapping scheme and polarity 
inversion

   (Stephen Boyd).

Changes in v7:
 - Detail description of lane mapping scheme dt property (Andrzej
   Hajda/ Rob Herring).
 - Removed HDP gpio binding, since the bridge uses IRQ signal to
   determine HPD, and IRQ property is already documented in binding.

Signed-off-by: Sandeep Panda 
---
 .../bindings/display/bridge/ti,sn65dsi86.txt   | 109 
+

 1 file changed, 109 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt


diff --git 
a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

new file mode 100644
index 000..33329f9
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

@@ -0,0 +1,109 @@
+SN65DSI86 DSI to eDP bridge chip
+
+
+This is the binding for Texas Instruments SN65DSI86 bridge.
+http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86=pdf
+
+Required properties:
+- compatible: Must be "ti,sn65dsi86"
+- reg: i2c address of the chip, 0x2d as per datasheet
+- enable-gpios: OF device-tree gpio specification for bridge_en pin 
(active high)

+
+- vccio-supply: A 1.8V supply that powers up the digital IOs.
+- vpll-supply: A 1.8V supply that powers up the displayport PLL.
+- vcca-supply: A 1.2V supply that powers up the analog circuits.
+- vcc-supply: A 1.2V supply that powers up the digital core.
+
+Optional properties:
+- interrupts: Specifier for the SN65DSI86 interrupt line.
+
+- ddc-i2c-bus: phandle of the I2C controller used for DDC EDID 
probing

+
+- gpio-controller: Marks the device has a GPIO controller.
+- #gpio-cells: Should be two. The first cell is the pin number 
and

+   the second cell is used to specify flags.
+   See ../../gpio/gpio.txt for more information.
+- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description 
of

+   the cell formats.
+
+- clock-names: should be "refclk"
+- clocks: Specification for input reference clock. The reference
+ clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
+
+- lane-mapping: Specification to describe the logical to physical 
lane


As I mentioned in v7, we already have a property for this. It's called
'data-lanes' and defined in media/video-interfaces.txt. Use that. If 
you

need polarity too, then add a property for that. And add it to
video-interfaces.txt.


The data-lanes property mentioned in media/video-interfaces.txt is 
referring
to DSI/CSI lanes where assumption is clock lane is fixed at index 0. But 
here
the we want to mention about eDP lanes which do not have dedicated clock 
lane.

So can we still use the existing data-lanes property here?




+   mapping scheme and polarity inversion of eDP lanes on PCB.
+   Each pair present at index n (where n lies between 0 and 3)
+   describes the lane mapping of logical lane to physical lane n
+		and the polarity(it should be either 1 or 0) of the physical lane 
n.

+
+   For example:
+   lane-mapping = <2 1>,
+   <1 0>,
+   <3 1>,
+   <0 0>;
+
+   The above mapping describes that logical lane 2 is mapped to
+   physical lane 0 and polarity of physical lane 0 is inverted,
+   logical lane 1 is mapped to physical lane 1 and polarity of
+   physical lane 1 is normal, logical lane 3 is mapped to physical
+   lane 2 and polarity of physical lane 2 is inverted, logical 
lane 0
+		is mapped to physical lane 4 and polarity of physical lane 3 is 

Re: [PATCH v6 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[spanda]

On 2018-05-16 21:57, Stephen Boyd wrote:

Quoting Sandeep Panda (2018-05-14 22:52:42)
diff --git 
a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

new file mode 100644
index 000..b82bb56
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

@@ -0,0 +1,81 @@
+Optional properties:
+- interrupts: Specifier for the SN65DSI86 interrupt line.
+- hpd-gpios: OF device-tree gpio specifications for HPD pin.
+
+- gpio-controller: Marks the device has a GPIO controller.
+- #gpio-cells: Should be two. The first cell is the pin number 
and

+   the second cell is used to specify flags.
+   See ../../gpio/gpio.txt for more information.
+- #pwm-cells : Should be one. See ../../pwm/pwm.txt for description 
of

+   the cell formats.
+
+- clock-names: should be "refclk"
+- clocks: OF device-tree clock specification for refclk input. The 
reference


What is "OF device-tree .* specification" providing? This is all an OF
device-tree specification.
Ok. i will remove OF device tree, only keep specification for refclk 
input.



+  clock rate must be 12 MHz, 19.2 MHz, 26 MHz, 27 MHz or 38.4 MHz.
+
+Required nodes:
+
+This device has two video ports. Their connections are modelled using 
the
+OF graph bindings specified in 
Documentation/devicetree/bindings/graph.txt.

+
+- Video port 0 for DSI input
+- Video port 1 for eDP output
+
+Example
+---
+
+edp-bridge@2d {
+   compatible = "ti,sn65dsi86";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0x2d>;
+
+   enable-gpios = < 33 GPIO_ACTIVE_HIGH>;
+   interrupt-parent = <>;
+   interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+
+   vccio-supply = <_l17>;
+   vcca-supply = <_l6>;
+   vpll-supply = <_l17>;
+   vcc-supply = <_l6>;
+
+   clock-names = "refclk";
+   clocks = <_refclk>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+
+   edp_bridge_in: endpoint {
+   remote-endpoint = <_out>;


How do we know the number of lanes that are connected and if there's 
one

channel (A) or two channels (A and B)? Would there be two endpoints in
that case?
Currently we have hard coded the number of lanes to 4 in driver. And 
regarding supporting 2 DSI input channels dt-binding, we are planning to 
add those entries once we upload a patchset support dual dsi 
configuration in bridge driver.



+   };
+   };
+
+   port@1 {
+   reg = <1>;
+
+   edp_bridge_out: endpoint {
+   remote-endpoint = <_panel_in>;


The hardware looks to support some sort of lane renumbering scheme,
where the eDP logical lane 0 can be routed through a different pin than
MLP/N0, same for logical lane 1, etc. I don't have a use case for this
right now, but I hope that it could be added somewhere in the binding 
as

an optional property to describe this lane remapping feature. It also
has some sort of lane polarity inversion feature. Perhaps there needs 
to

be a lane-config property that does this remapping and inversion with
two cells.

Ok. I will add this feature to dt binding.


lane-config = <0 0>, /* Lane 0 logical is lane 0 phys (!inv) */
  <1 0>, /* Lane 1 logical is lane 1 phys (!inv) */
  <2 0>, /* Lane 2 logical is lane 2 phys (!inv) */
  <3 0>; /* Lane 3 logical is lane 3 phys (!inv) */

Or

lane-config = <2 1>, /* Lane 2 logical is lane 0 phys (inv) */
  <1 0>, /* Lane 1 logical is lane 1 phys (!inv) */
  <3 1>, /* Lane 3 logical is lane 2 phys (inv) */
  <0 0>; /* Lane 0 logical is lane 3 phys (!inv) */



+   };
+   };
+   };

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

Re: [PATCH v6 1/4] drm/bridge: add support for sn65dsi86 bridge driver

[spanda]

On 2018-05-16 12:56, Andrzej Hajda wrote:

I suppose you wanted to respond on the list, so I have added back all
recipients.

On 16.05.2018 06:39, spa...@codeaurora.org wrote:

On 2018-05-15 19:23, Andrzej Hajda wrote:

On 15.05.2018 07:52, Sandeep Panda wrote:

Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

This chip can be controlled via either i2c interface or
dsi interface. Currently in driver all the control registers
are being accessed through i2c interface only.
Also as of now HPD support has not been added to bridge
chip driver.

Changes in v1:
 - Split the dt-bindings and the driver support into separate 
patches

   (Andrzej Hajda).
 - Use of gpiod APIs to parse and configure gpios instead of 
obsolete

ones
   (Andrzej Hajda).
 - Use macros to define the register offsets (Andrzej Hajda).

Changes in v2:
 - Separate out edp panel specific HW resource handling from bridge
   driver and create a separate edp panel drivers to handle panel
   specific mode information and HW resources (Sean Paul).
 - Replace pr_* APIs to DRM_* APIs to log error or debug information
   (Sean Paul).
 - Remove some of the unnecessary structure/variable from driver 
(Sean

   Paul).
 - Rename the function and structure prefix "sn65dsi86" to
"ti_sn_bridge"
   (Sean Paul / Rob Herring).
 - Remove most of the hard-coding and modified the bridge init
sequence
   based on current mode (Sean Paul).
 - Remove the existing function to retrieve the EDID data and
   implemented this as an i2c_adapter and use drm_get_edid() (Sean
Paul).
 - Remove the dummy irq handler implementation, will add back the
   proper irq handling later (Sean Paul).
 - Capture the required enable gpios in a single array based on dt
entry
   instead of having individual descriptor for each gpio (Sean 
Paul).


Changes in v3:
 - Remove usage of irq_gpio and replace it as "interrupts" property
(Rob
   Herring).
 - Remove the unnecessary header file inclusions (Sean Paul).
 - Rearrange the header files in alphabetical order (Sean Paul).
 - Use regmap interface to perform i2c transactions.
 - Update Copyright/License field and address other review comments
   (Jordan Crouse).

Changes in v4:
 - Update License/Copyright (Sean Paul).
 - Add Kconfig and Makefile changes (Sean Paul).
 - Drop i2c gpio handling from this bridge driver, since i2c sda/scl
gpios
   will be handled by i2c master.
 - Update required supplies names.
 - Remove unnecessary goto statements (Sean Paul).
 - Add mutex lock to power_ctrl API to avoid race conditions (Sean
   Paul).
 - Add support to parse reference clk frequency from dt(optional).
 - Update the bridge chip enable/disable sequence.

Changes in v5:
 - Fixed Kbuild test service reported warnings.

Changes in v6:
 - Use PM runtime based ref-counting instead of local ref_count
mechanism
   (Stephen Boyd).
 - Clean up some debug logs and indentations (Sean Paul).
 - Simplify dp rate calculation (Sean Paul).
 - Add support to configure refclk based on input REFCLK pin or 
DACP/N

   pin (Stephen Boyd).

Signed-off-by: Sandeep Panda 
---
 drivers/gpu/drm/bridge/Kconfig|   9 +
 drivers/gpu/drm/bridge/Makefile   |   1 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 766
++
 3 files changed, 776 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/Kconfig
b/drivers/gpu/drm/bridge/Kconfig
index 3b99d5a..8153150 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -108,6 +108,15 @@ config DRM_TI_TFP410
---help---
  Texas Instruments TFP410 DVI/HDMI Transmitter driver

+config DRM_TI_SN65DSI86
+   tristate "TI SN65DSI86 DSI to eDP bridge"
+   depends on OF
+   select DRM_KMS_HELPER
+   select REGMAP_I2C
+   select DRM_PANEL
+   ---help---
+ Texas Instruments SN65DSI86 DSI to eDP Bridge driver
+
 source "drivers/gpu/drm/bridge/analogix/Kconfig"

 source "drivers/gpu/drm/bridge/adv7511/Kconfig"
diff --git a/drivers/gpu/drm/bridge/Makefile
b/drivers/gpu/drm/bridge/Makefile
index 373eb28..3711be8 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -12,4 +12,5 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
 obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
 obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
+obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
 obj-y += synopsys/
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
new file mode 100644
index 000..1d3e549
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -0,0 +1,766 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 

Re: [PATCH v5 3/4] drm/panel: add Innolux TV123WAM panel driver support

[spanda]

On 2018-05-03 00:35, Sean Paul wrote:

On Wed, May 02, 2018 at 10:02:01AM +0530, Sandeep Panda wrote:

Add support for Innolux TV123WAM, which is a 12.3" eDP
display panel with 2160x1440 resolution.

Signed-off-by: Sandeep Panda 
---
 drivers/gpu/drm/panel/panel-simple.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c

index 234af81..52bbcfb 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1939,6 +1939,30 @@ static void panel_simple_shutdown(struct device 
*dev)

.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
 };

+static const struct drm_display_mode innolux_tv123wam_mode = {
+   .clock = 206016,
+   .hdisplay = 2160,
+   .hsync_start = 2160 + 48,
+   .hsync_end = 2160 + 48 + 32,
+   .htotal = 2160 + 48 + 32 + 80,
+   .vdisplay = 1440,
+   .vsync_start = 1440 + 3,
+   .vsync_end = 1440 + 3 + 10,
+   .vtotal = 1440 + 3 + 10 + 27,
+   .vrefresh = 60,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
+};
+
+static const struct panel_desc innolux_tv123wam = {
+   .modes = _tv123wam_mode,
+   .num_modes = 1,
+   .bpc = 8,
+   .size = {
+   .width = 259,
+   .height = 173,
+   },
+};
+
 static const struct of_device_id platform_of_match[] = {
{
.compatible = "ampire,am-480272h3tmqw-t01h",
@@ -2142,6 +2166,9 @@ static void panel_simple_shutdown(struct device 
*dev)

.compatible = "winstar,wf35ltiacd",
.data = _wf35ltiacd,
}, {
+   .compatible = "innolux,tv123wam",
+   .data = _tv123wam,
+   }, {


These are alphabetically ordered, as are the 
drm_display_mode/panel_desc

structs.

Sean


Done.

/* sentinel */
}
 };
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,

a Linux Foundation Collaborative Project


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

Re: [PATCH v5 1/4] drm/bridge: add support for sn65dsi86 bridge driver

[spanda]

On 2018-05-03 00:33, Sean Paul wrote:

On Wed, May 02, 2018 at 10:01:59AM +0530, Sandeep Panda wrote:

Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

This chip can be controlled via either i2c interface or
dsi interface. Currently in driver all the control registers
are being accessed through i2c interface only.
Also as of now HPD support has not been added to bridge
chip driver.

Changes in v1:
 - Split the dt-bindings and the driver support into separate patches
   (Andrzej Hajda).
 - Use of gpiod APIs to parse and configure gpios instead of obsolete 
ones

   (Andrzej Hajda).
 - Use macros to define the register offsets (Andrzej Hajda).

Changes in v2:
 - Separate out edp panel specific HW resource handling from bridge
   driver and create a separate edp panel drivers to handle panel
   specific mode information and HW resources (Sean Paul).
 - Replace pr_* APIs to DRM_* APIs to log error or debug information
   (Sean Paul).
 - Remove some of the unnecessary structure/variable from driver (Sean
   Paul).
 - Rename the function and structure prefix "sn65dsi86" to 
"ti_sn_bridge"

   (Sean Paul / Rob Herring).
 - Remove most of the hard-coding and modified the bridge init 
sequence

   based on current mode (Sean Paul).
 - Remove the existing function to retrieve the EDID data and
   implemented this as an i2c_adapter and use drm_get_edid() (Sean 
Paul).

 - Remove the dummy irq handler implementation, will add back the
   proper irq handling later (Sean Paul).
 - Capture the required enable gpios in a single array based on dt 
entry

   instead of having individual descriptor for each gpio (Sean Paul).

Changes in v3:
 - Remove usage of irq_gpio and replace it as "interrupts" property 
(Rob

   Herring).
 - Remove the unnecessary header file inclusions (Sean Paul).
 - Rearrange the header files in alphabetical order (Sean Paul).
 - Use regmap interface to perform i2c transactions.
 - Update Copyright/License field and address other review comments
   (Jordan Crouse).

Changes in v4:
 - Update License/Copyright (Sean Paul).
 - Add Kconfig and Makefile changes (Sean Paul).
 - Drop i2c gpio handling from this bridge driver, since i2c sda/scl 
gpios

   will be handled by i2c master.
 - Remove unnecessary goto statements (Sean Paul).
 - Add mutex lock to power_ctrl API to avoid race conditions (Sean
   Paul).
 - Add support to parse reference clk frequency from dt(optional).
 - Update the bridge chip enable/disable sequence.


It seems like you also added 2 new supply names, as well as remove the
configurable gpios?



Changes in v5:
 - Fixed Kbuild test service reported warnings.

Signed-off-by: Sandeep Panda 
---
 drivers/gpu/drm/bridge/Kconfig|   9 +
 drivers/gpu/drm/bridge/Makefile   |   1 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 725 
++

 3 files changed, 735 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/Kconfig 
b/drivers/gpu/drm/bridge/Kconfig

index 3b99d5a..8153150 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -108,6 +108,15 @@ config DRM_TI_TFP410
---help---
  Texas Instruments TFP410 DVI/HDMI Transmitter driver

+config DRM_TI_SN65DSI86
+   tristate "TI SN65DSI86 DSI to eDP bridge"
+   depends on OF
+   select DRM_KMS_HELPER
+   select REGMAP_I2C
+   select DRM_PANEL
+   ---help---
+ Texas Instruments SN65DSI86 DSI to eDP Bridge driver
+
 source "drivers/gpu/drm/bridge/analogix/Kconfig"

 source "drivers/gpu/drm/bridge/adv7511/Kconfig"
diff --git a/drivers/gpu/drm/bridge/Makefile 
b/drivers/gpu/drm/bridge/Makefile

index 373eb28..3711be8 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -12,4 +12,5 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
 obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
 obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
+obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
 obj-y += synopsys/
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c

new file mode 100644
index 000..019c7cd
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -0,0 +1,725 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SN_BRIDGE_REVISION_ID 0x2
+
+/* Link Training specific registers */
+#define SN_DEVICE_REV_REG  0x08
+#define SN_HPD_DISABLE_REG 0x5C
+#define SN_REFCLK_FREQ_REG 0x0A
+#define SN_DSI_LANES_REG   0x10
+#define SN_DSIA_CLK_FREQ_REG   0x12
+#define 

Re: [PATCH v5 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[spanda]

On 2018-05-08 03:48, Stephen Boyd wrote:

Quoting spa...@codeaurora.org (2018-05-03 02:41:29)

On 2018-05-02 22:31, Stephen Boyd wrote:
> Quoting Sandeep Panda (2018-05-01 21:32:00)
>> diff --git
>> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
>> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
>> new file mode 100644
>> index 000..0d042ce
>
> Please use the clocks property instead. We may need to turn the clk on
> first before this can work so the driver would use the clk framework
> (at
> least in linux). clock-names could have 'refclk' because that's the pin
> name.
>
> Is there a way in DRM to figure out the frequency of the clock
> frequency
> for DACP/N? It looks like if refclk is grounded, then the DACP/N pins
> from the DSI side should be one of a set of frequencies, so I'm just
> curious how that will work and if the binding would need to be updated
> to indicate what the frequency of the DSI clock lane is, or if DRM can
> tell this driver through the port/graph stuff somehow.
>

Can we do something like below?
1. Add a required dt-property to indicate what is the source of
refclk, ti,sn-refclk-src = <0> ---> means refclk is derived from 
refclk

pin.

   ti,sn-refclk-src = <1> ---> means refclk is derived from DACP/N 
pin.
2. Add a clock property to indicate the refclk frequency for 
refclk

pin.
3. In driver, parse the refclk source dt-property. If the source 
is
refclk pin then get the frquency from clock dt-property and program 
the

i2c register accordingly.
   Else if the source is DACP/N pin then calculate the DSIA 
frequency
based on current display mode (by the time we go for configuring 
refclk,

drm_mode_set is already done and in  diver we can calculate the
frequency) and program the i2c register accordingly.


The presence or non-presence of the refclk should still be indicated 
via

the standard clock property instead of some TI specific property. The
driver can try to clk_get() the refclk and if its there it can call
clk_get_rate() to figure out the reference clk frequency. It should 
also

turn it on with clk_prepare_enable() to make sure the clk is clocking
and turn it off when the driver isn't using it.

If the reference clk is recovered from the DACP/N pin then there won't
be a clocks property, and the driver can do what you describe in #3.



>> +
>> +- gpio-controller: Marks the device has a GPIO controller.
>> +- #gpio-cells: Number of GPIO cells. Refer to binding document
>> "gpio/gpio.txt"
>
> What's the number? 2?
number is 4, i will update this in binding


Really? What do you need 4 cells for? The number of cells doesn't
indicate the number of GPIOs on the device.


It should be 2, got confused with number GPIOs.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 1/4] drm/bridge: add support for sn65dsi86 bridge driver

[spanda]

On 2018-05-08 03:50, Stephen Boyd wrote:

Quoting Sean Paul (2018-05-02 12:03:16)

On Wed, May 02, 2018 at 10:01:59AM +0530, Sandeep Panda wrote:

> + struct drm_display_mode curr_mode;
> + struct mutex lock;
> + unsigned int ctrl_ref_count;
> +};
> +
> +static const struct regmap_range ti_sn_bridge_volatile_ranges[] = {
> + { .range_min = 0, .range_max = 0xff },
> +};
> +
> +static const struct regmap_access_table ti_sn_bridge_volatile_table = {
> + .yes_ranges = ti_sn_bridge_volatile_ranges,
> + .n_yes_ranges = ARRAY_SIZE(ti_sn_bridge_volatile_ranges),
> +};
> +
> +static const struct regmap_config ti_sn_bridge_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .volatile_table = _sn_bridge_volatile_table,
> + .cache_type = REGCACHE_NONE,
> +};
> +
> +static int ti_sn_bridge_power_ctrl(struct ti_sn_bridge *pdata, bool enable)
> +{
> + int ret = 0;
> +
> + mutex_lock(>lock);
> + if (enable)
> + pdata->ctrl_ref_count++;
> + else
> + pdata->ctrl_ref_count--;

I think you should use a kref instead of rolling your own ref_count. 
You can
handle release by calling kref_put_mutex(), which will handle the 
reference and
the lock. On the acquire side, you can use kref_get_unless_zero which 
will be

fast if the reference is already active.


Why not use runtime PM?


I think PM runtime will be a better approach since we are trying to 
protect bridge power source related resources here.

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

Re: [PATCH v5 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[spanda]

On 2018-05-02 22:31, Stephen Boyd wrote:

Quoting Sandeep Panda (2018-05-01 21:32:00)
diff --git 
a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

new file mode 100644
index 000..0d042ce
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

@@ -0,0 +1,76 @@
+SN65DSI86 DSI to eDP bridge chip
+
+
+This is the binding for Texas Instruments SN65DSI86 bridge.
+http://www.ti.com/general/docs/lit/getliterature.tsp?genericPartNumber=sn65dsi86=pdf
+
+Required properties:
+- compatible: Must be "ti,sn65dsi86"
+- reg: i2c address of the chip, 0x2d as per datasheet
+- enable-gpios: OF device-tree gpio specification for bridge_en pin
+
+- vccio-supply: A 1.8V supply that powers up the digital IOs.
+- vpll-supply: A 1.8V supply that powers up the displayport PLL.
+- vcca-supply: A 1.2V supply that powers up the analog circuits.
+- vcc-supply: A 1.2V supply that powers up the digital core.
+
+Optional properties:
+- interrupts: Specifier for the SN65DSI86 interrupt line.
+- hpd-gpios: OF device-tree gpio specifications for HPD pin.
+
+- refclk-freq-khz: Value specifying the frequency of reference clock 
in KHz unit.


Please use the clocks property instead. We may need to turn the clk on
first before this can work so the driver would use the clk framework 
(at

least in linux). clock-names could have 'refclk' because that's the pin
name.

Is there a way in DRM to figure out the frequency of the clock 
frequency

for DACP/N? It looks like if refclk is grounded, then the DACP/N pins
from the DSI side should be one of a set of frequencies, so I'm just
curious how that will work and if the binding would need to be updated
to indicate what the frequency of the DSI clock lane is, or if DRM can
tell this driver through the port/graph stuff somehow.



Can we do something like below?
   1. Add a required dt-property to indicate what is the source of 
refclk, ti,sn-refclk-src = <0> ---> means refclk is derived from refclk 
pin.
 
  ti,sn-refclk-src = <1> ---> means refclk is derived from DACP/N pin.
   2. Add a clock property to indicate the refclk frequency for refclk 
pin.
   3. In driver, parse the refclk source dt-property. If the source is 
refclk pin then get the frquency from clock dt-property and program the 
i2c register accordingly.
  Else if the source is DACP/N pin then calculate the DSIA frequency 
based on current display mode (by the time we go for configuring refclk, 
drm_mode_set is already done and in  diver we can calculate the 
frequency) and program the i2c register accordingly.



+
+- gpio-controller: Marks the device has a GPIO controller.
+- #gpio-cells: Number of GPIO cells. Refer to binding document 
"gpio/gpio.txt"


What's the number? 2?

number is 4, i will update this in binding


+- #pwm-cells : Number of cells used to specify a PWM. See pwm.txt in 
this directory

+  for a description of the cell formats.


What's the number? 1? There's a pwm.txt in this directory?

yes number is 1, i will update the binding. it should be pwm/pwm.txt
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [[RFC]DPU PATCH 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[spanda]

On 2018-04-27 08:43, Rob Herring wrote:

On Wed, Apr 25, 2018 at 08:46:13PM -0400, Rob Clark wrote:
On Wed, Apr 25, 2018 at 7:45 PM, Stephen Boyd  
wrote:

> Quoting Sandeep Panda (2018-04-19 10:56:06)
>> Document the bindings used for the sn65dsi86 DSI to eDP bridge.
>>
>> Changes in v1:
>>  - Rephrase the dt-binding descriptions to be more inline with existing
>>bindings (Andrzej Hajda).
>>  - Add missing dt-binding that are parsed by corresponding driver
>>(Andrzej Hajda).
>>
>> Changes in v2:
>>  - Removed edp panel specific dt-binding entries. Only keep bridge
>>specific entries (Sean Paul).
>>  - Remove custom-modes dt entry since its usage is removed from driver also 
(Sean Paul).
>>  - Remove is-pluggable dt entry since this will not be needed anymore (Sean 
Paul).
>>
>> Changes in v3:
>>  - Removed irq-gpio dt entry and instead populate is an interrupt
>>property (Rob Herring).
>
> These changelogs usually go below the triple dash, but maybe drm is
> different and wants them?

yeah, drm generally wants them in the commit msg rather than below the
triple-dash, although I guess for bindings docs it should follow the
rules for that tree.. I usually just fix up these sort of things as I
apply patches, but not sure what other maintainers prefer


Well, these DPU patches aren't targeted for upstream so who cares.


This change is independent of other DPU patches. We are planning to 
upstream these bridge and panel changes.
I will upload the next patchset dropping the DPU tag to avoid any 
confusion.


Many patch revision changelogs I see are crap with statements like
"implement changes requested by ??". But in this case, the changelog is
really good.

Rob

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

Re: [[RFC]DPU PATCH 4/4] dt-bindings: Add Innolux TV123WAM panel bindings

[spanda]

On 2018-04-24 20:13, Rob Herring wrote:

On Wed, Apr 18, 2018 at 05:50:02PM +0530, Sandeep Panda wrote:

The Innolux TV123WAM is a 12.3" eDP display panel
with 2160x1440 resolution.


Why don't you just submit this for upstream?
Sean Paul has suggested to reuse simple panel driver instead of having a 
new driver altogether. So all these dt-bindings wont be needed.




Signed-off-by: Sandeep Panda 
---
 .../bindings/display/panel/innolux,edp-2k-panel.txt | 21 
+

 1 file changed, 21 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/innolux,edp-2k-panel.txt


diff --git 
a/Documentation/devicetree/bindings/display/panel/innolux,edp-2k-panel.txt 
b/Documentation/devicetree/bindings/display/panel/innolux,edp-2k-panel.txt

new file mode 100644
index 000..19f271c
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/display/panel/innolux,edp-2k-panel.txt

@@ -0,0 +1,21 @@
+Innolux TV123WAM 12.3 inch eDP display panel
+
+Required properties:
+- power-supply: regulator to provide the supply voltage
+- enable-gpios: GPIO pin to enable or disable the panel


Need to state the active state.


+
+Optional properties:
+- backlight: phandle of the backlight device attached to the panel
+
+Example:
+
+   edp_panel: edp_panel {


panel {


+   compatible = "innolux, edp_2k_panel";


spurious space^


+   reg = <0>;


Not documented. Should be dropped.


+
+   enable-gpios = < 32 GPIO_ACTIVE_HIGH>;
+
+   power-supply = <_l8>;
+


Remove the extra blank lines.


+   backlight = <>;
+   };
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,

a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe devicetree" 
in

the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [[RFC]DPU PATCH 1/4] drm/bridge: add support for sn65dsi86 bridge driver

[spanda]

On 2018-04-21 01:06, Sean Paul wrote:

On Thu, Apr 19, 2018 at 11:26:05PM +0530, Sandeep Panda wrote:

Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

This chip can be controlled via either i2c interface or
dsi interface. Currently in driver all the control registers
are being accessed through i2c interface only.
Also as of now HPD support has not been added to bridge
chip driver.

Changes in v1:
 - Split the dt-bindings and the driver support into separate patches
   (Andrzej Hajda).
 - Use of gpiod APIs to parse and configure gpios instead of obsolete 
ones

   (Andrzej Hajda).
 - Use macros to define the register offsets (Andrzej Hajda).

Changes in v2:
 - Separate out edp panel specific HW resource handling from bridge
   driver and create a separate edp panel drivers to handle panel
   specific mode information and HW resources (Sean Paul).
 - Replace pr_* APIs to DRM_* APIs to log error or debug information
   (Sean Paul).
 - Remove some of the unnecessary structure/variable from driver (Sean
   Paul).
 - Rename the function and structure prefix "sn65dsi86" to 
"ti_sn_bridge"

   (Sean Paul / Rob Herring).
 - Remove most of the hard-coding and modified the bridge init 
sequence

   based on current mode (Sean Paul).
 - Remove the existing function to retrieve the EDID data and
   implemented this as an i2c_adapter and use drm_get_edid() (Sean 
Paul).

 - Remove the dummy irq handler implementation, will add back the
   proper irq handling later (Sean Paul).
 - Capture the required enable gpios in a single array based on dt 
entry

   instead of having individual descriptor for each gpio (Sean Paul).

Changes in v3:
 - Remove usage of irq_gpio and replace it as "interrupts" property 
(Rob

   Herring).
 - Remove the unnecessary header file inclusions (Sean Paul).
 - Rearrange the header files in alphabetical order (Sean Paul).
 - Use regmap interface to perform i2c transactions.
 - Update Copyright/License field and address other review comments
   (Jordan Crouse).

Signed-off-by: Sandeep Panda 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 690 
++


What about Kconfig/Makefile?


Thought of adding this in another change once review is over. But will 
add to current change in next patchset.





 1 file changed, 690 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c

new file mode 100644
index 000..a2a95f5
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -0,0 +1,690 @@
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or 
modify

+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */


Use SPDX license


Will add back SPDX along with copyright in next patchset.




+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SN_BRIDGE_REVISION_ID 0x2
+
+/* Link Training specific registers */
+#define SN_DEVICE_REV_REG  0x08
+#define SN_REFCLK_FREQ_REG 0x0A
+#define SN_DSI_LANES_REG   0x10
+#define SN_DSIA_CLK_FREQ_REG   0x12
+#define SN_ENH_FRAME_REG   0x5A
+#define SN_SSC_CONFIG_REG  0x93
+#define SN_DATARATE_CONFIG_REG 0x94
+#define SN_PLL_ENABLE_REG  0x0D
+#define SN_SCRAMBLE_CONFIG_REG 0x95
+#define SN_AUX_WDATA0_REG  0x64
+#define SN_AUX_ADDR_19_16_REG  0x74
+#define SN_AUX_ADDR_15_8_REG   0x75
+#define SN_AUX_ADDR_7_0_REG0x76
+#define SN_AUX_LENGTH_REG  0x77
+#define SN_AUX_CMD_REG 0x78
+#define SN_ML_TX_MODE_REG  0x96
+/* video config specific registers */
+#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG  0x20
+#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG 0x21
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG   0x24
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG  0x25
+#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG   0x2C
+#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG  0x2D
+#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG   0x30
+#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG  0x31
+#define SN_CHA_HORIZONTAL_BACK_PORCH_REG   0x34
+#define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36
+#define 

Re: [[RFC]DPU PATCH 3/4] drm/panel: add Innolux TV123WAM eDP panel driver

[spanda]

On 2018-04-21 01:13, Sean Paul wrote:

On Thu, Apr 19, 2018 at 11:26:07PM +0530, Sandeep Panda wrote:

Add support for Innolux TV123WAM 12.3" panel which
is an eDP panel.


It seems like you could just use the panel-simple driver, no? Given 
that this
driver doesn't have any power timing, that will probably work better 
since the

warranty won't be voided :)


While going through the simple panel driver first saw couple of calls to 
mipi_dsi
APIs, and since this panel is an eDP panel so decided to make a new 
driver instead.
But it looks like we can reuse simple-panel for this edp panel also. 
Will use simple-panel driver in next patch.


Sean



Signed-off-by: Sandeep Panda 
---
 drivers/gpu/drm/panel/panel-innolux-tv123wam.c | 293 
+

 1 file changed, 293 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-innolux-tv123wam.c

diff --git a/drivers/gpu/drm/panel/panel-innolux-tv123wam.c 
b/drivers/gpu/drm/panel/panel-innolux-tv123wam.c

new file mode 100644
index 000..5632b02
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-innolux-tv123wam.c
@@ -0,0 +1,293 @@
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or 
modify

+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct innolux_edp_2k_panel_desc {
+   const struct drm_display_mode *modes;
+   unsigned int num_modes;
+   u32 bpc;
+   u32 bus_flags;
+};
+
+struct innolux_edp_2k_panel {
+   struct drm_panel base;
+   bool enabled;
+   bool prepared;
+   const struct innolux_edp_2k_panel_desc *desc;
+   struct regulator *supply;
+   struct gpio_desc *enable_gpio;
+   struct backlight_device *backlight;
+};
+
+static const struct drm_display_mode innolux_edp_2k_mode = {
+   .clock = 206016,
+   .hdisplay = 2160,
+   .hsync_start = 2160 + 48,
+   .hsync_end = 2160 + 48 + 32,
+   .htotal = 2160 + 48 + 32 + 80,
+   .vdisplay = 1440,
+   .vsync_start = 1440 + 3,
+   .vsync_end = 1440 + 3 + 10,
+   .vtotal = 1440 + 3 + 10 + 27,
+   .vrefresh = 60,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
+};
+
+static const struct innolux_edp_2k_panel_desc innolux_edp_2k = {
+   .modes = _edp_2k_mode,
+   .num_modes = 1,
+   .bpc = 8,
+   .bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE,
+};
+
+static const struct of_device_id platform_of_match[] = {
+   {
+   .compatible = "innolux, edp_2k_panel",
+   .data = _edp_2k,
+   },
+};
+
+static inline struct innolux_edp_2k_panel *
+to_innolux_edp_2k_panel(struct drm_panel *panel)
+{
+   return container_of(panel, struct innolux_edp_2k_panel, base);
+}
+
+static int innolux_edp_2k_panel_disable(struct drm_panel *panel)
+{
+   struct innolux_edp_2k_panel *pdata = to_innolux_edp_2k_panel(panel);
+
+   if (!pdata->enabled)
+   return 0;
+
+   if (pdata->backlight) {
+   pdata->backlight->props.power = FB_BLANK_POWERDOWN;
+   pdata->backlight->props.state |= BL_CORE_FBBLANK;
+   backlight_update_status(pdata->backlight);
+   }
+
+   pdata->enabled = false;
+
+   return 0;
+}
+
+static int innolux_edp_2k_panel_unprepare(struct drm_panel *panel)
+{
+   struct innolux_edp_2k_panel *pdata = to_innolux_edp_2k_panel(panel);
+
+   if (!pdata->prepared)
+   return 0;
+
+   if (pdata->enable_gpio)
+   gpiod_set_value_cansleep(pdata->enable_gpio, 0);
+
+   regulator_disable(pdata->supply);
+
+   pdata->prepared = false;
+
+   return 0;
+}
+
+static int innolux_edp_2k_panel_prepare(struct drm_panel *panel)
+{
+   struct innolux_edp_2k_panel *pdata = to_innolux_edp_2k_panel(panel);
+   int ret;
+
+   if (pdata->prepared)
+   return 0;
+
+   ret = regulator_enable(pdata->supply);
+   if (ret < 0) {
+   dev_err(panel->dev, "failed to enable supply: %d\n", ret);
+   return ret;
+   }
+
+   if (pdata->enable_gpio)
+   gpiod_set_value_cansleep(pdata->enable_gpio, 1);
+
+   pdata->prepared = true;
+
+   return 0;
+}
+
+static int innolux_edp_2k_panel_enable(struct drm_panel *panel)
+{
+   struct innolux_edp_2k_panel *pdata = to_innolux_edp_2k_panel(panel);
+
+   if (pdata->enabled)
+   return 0;
+
+   if (pdata->backlight) {
+   pdata->backlight->props.state &= ~BL_CORE_FBBLANK;
+   pdata->backlight->props.power = 

Re: [[RFC]DPU PATCH 1/4] drm/bridge: add support for sn65dsi86 bridge driver

[spanda]

On 2018-04-19 20:44, Jordan Crouse wrote:

On Wed, Apr 18, 2018 at 05:49:59PM +0530, Sandeep Panda wrote:

Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

This chip can be controlled via either i2c interface or
dsi interface. Currently in driver all the control registers
are being accessed through i2c interface only.
Also as of now HPD support has not been added to bridge
chip driver.

Signed-off-by: Sandeep Panda 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 742 
++

 1 file changed, 742 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c

new file mode 100644
index 000..c798f2f
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -0,0 +1,742 @@
+/* SPDX-License-Identifier: GPL-2.0 */


There should be a copyright here.


+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SN_BRIDGE_REVISION_ID 0x2
+
+/* Link Training specific registers */
+#define SN_DEVICE_REV_REG  0x08
+#define SN_REFCLK_FREQ_REG 0x0A
+#define SN_DSI_LANES_REG   0x10
+#define SN_DSIA_CLK_FREQ_REG   0x12
+#define SN_ENH_FRAME_REG   0x5A
+#define SN_SSC_CONFIG_REG  0x93
+#define SN_DATARATE_CONFIG_REG 0x94
+#define SN_PLL_ENABLE_REG  0x0D
+#define SN_SCRAMBLE_CONFIG_REG 0x95
+#define SN_AUX_WDATA0_REG  0x64
+#define SN_AUX_ADDR_19_16_REG  0x74
+#define SN_AUX_ADDR_15_8_REG   0x75
+#define SN_AUX_ADDR_7_0_REG0x76
+#define SN_AUX_LENGTH_REG  0x77
+#define SN_AUX_CMD_REG 0x78
+#define SN_ML_TX_MODE_REG  0x96
+#define SN_PAGE_SELECT_REG 0xFF
+#define SN_AUX_RDATA0_REG  0x79
+/* video config specific registers */
+#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG  0x20
+#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG 0x21
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG   0x24
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG  0x25
+#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG   0x2C
+#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG  0x2D
+#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG   0x30
+#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG  0x31
+#define SN_CHA_HORIZONTAL_BACK_PORCH_REG   0x34
+#define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36
+#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG  0x38
+#define SN_CHA_VERTICAL_FRONT_PORCH_REG0x3A
+#define SN_DATA_FORMAT_REG 0x5B
+#define SN_COLOR_BAR_CONFIG_REG0x3C
+
+#define DPPLL_CLK_SRC_REFCLK   0
+#define DPPLL_CLK_SRC_DSICLK   1
+#define MIN_DSI_CLK_FREQ_MHZ   40
+
+enum ti_sn_bridge_ref_clks {
+   SN_REF_CLK_12_MHZ = 0,
+   SN_REF_CLK_19_2_MHZ,
+   SN_REF_CLK_26_MHZ,
+   SN_REF_CLK_27_MHZ,
+   SN_REF_CLK_38_4_MHZ,
+   SN_REF_CLK_MAX,
+};
+
+struct ti_sn_bridge {
+   struct device *dev;
+   struct drm_bridge bridge;
+   struct drm_connector connector;
+   struct device_node *host_node;
+   struct mipi_dsi_device *dsi;
+   /* handle to the connected eDP panel */
+   struct drm_panel *panel;
+   int irq;
+   struct gpio_desc *irq_gpio;
+   /* list of gpios needed to enable the bridge functionality */
+   struct gpio_descs *enable_gpios;
+   unsigned int num_supplies;
+   struct regulator_bulk_data *supplies;
+   struct i2c_client *i2c_client;
+   struct i2c_adapter *ddc;
+   bool power_on;
+   u32 num_modes;
+   struct drm_display_mode curr_mode;
+};
+
+static int ti_sn_bridge_write(struct ti_sn_bridge *pdata, u8 reg, u8 
val)

+{
+   struct i2c_client *client = pdata->i2c_client;
+   u8 buf[2] = {reg, val};
+   struct i2c_msg msg = {
+   .addr = client->addr,
+   .flags = 0,
+   .len = 2,
+   .buf = buf,
+   };
+
+   if (i2c_transfer(client->adapter, , 1) < 1) {
+   DRM_ERROR("i2c write failed\n");
+   return -EIO;
+   }
+
+   return 0;
+}
+
+static int ti_sn_bridge_read(struct ti_sn_bridge *pdata,
+   u8 reg, char *buf, u32 size)
+{
+   struct i2c_client *client = pdata->i2c_client;
+   struct i2c_msg msg[2] = {
+   {
+   .addr = client->addr,
+   .flags = 0,
+   .len = 1,
+   .buf = ,
+   },
+   {
+   

Re: [[RFC]DPU PATCH 0/4] Add suppport for sn65dsi86 bridge chip and Innolux 2k edp panel driver

[spanda]

On 2018-04-18 19:42, Sean Paul wrote:

On Wed, Apr 18, 2018 at 05:49:58PM +0530, Sandeep Panda wrote:

Changelog:

v3 -> v4:


I didn't really bother to do a thorough review given that there are 
obvious
mistakes and ignored feedback, it's a waste of time, tbh. Please go 
through

the previous reviews and resend a more polished version.


I have replied to your review comments. Please review the edp driver, so 
that

i can address all the comments together while uploading next patchset.


Sean

Current patchset separates out eDP panel specific resources from 
sn65dsi86

bridge driver and creates a separate driver for the innloux edp panel.

Now bridge driver check if any panel is attached or not to get the 
supported
modes. If any panel is attached then query from panel driver to get 
the modes,

or else probe the provided i2c adapter to read the modes from EDID.

Removed hardcoding of bridge init sequence. With this patchset bridge 
driver now
will program the init sequence based on the current mode set by drm 
framework.


Current patchset is not tested on actual bridge chip/panel.

Sandeep Panda (4):
  drm/bridge: add support for sn65dsi86 bridge driver
  dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings
  drm/panel: add Innolux TV123WAM eDP panel driver
  dt-bindings: Add Innolux TV123WAM panel bindings

 .../bindings/display/bridge/ti,sn65dsi86.txt   |  60 ++
 .../display/panel/innolux,edp-2k-panel.txt |  21 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c  | 742 
+

 drivers/gpu/drm/panel/panel-innolux-tv123wam.c | 299 +
 4 files changed, 1122 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
 create mode 100644 
Documentation/devicetree/bindings/display/panel/innolux,edp-2k-panel.txt

 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c
 create mode 100644 drivers/gpu/drm/panel/panel-innolux-tv123wam.c

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,

a Linux Foundation Collaborative Project


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

Re: [[RFC]DPU PATCH 1/4] drm/bridge: add support for sn65dsi86 bridge driver

[spanda]

On 2018-04-18 19:41, Sean Paul wrote:

On Wed, Apr 18, 2018 at 05:49:59PM +0530, Sandeep Panda wrote:

Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

This chip can be controlled via either i2c interface or
dsi interface. Currently in driver all the control registers
are being accessed through i2c interface only.
Also as of now HPD support has not been added to bridge
chip driver.

Signed-off-by: Sandeep Panda 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 742 
++

 1 file changed, 742 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c

new file mode 100644
index 000..c798f2f
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -0,0 +1,742 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 


I have a hard time believing that you need all of these includes. Off 
the top of
my head, you could probably remove types, kernel, init, 
platform_device, delay,
drmP, drm_atomic, drm_crtc_helper. You could probably trim it even 
further with

the help of your compiler. These should also be in alphabetical order.

I will take care of alphabetical order and generic kernel header file 
inclusion. But the header files of

drmP, drm_atomic, drm_crtc_helper are required here.

drmP, drm_atomic, drm_crtc_helper all these are needed because of the 
drm api that we using in this driver
for modes, connectors and bridges. the drm header files i have already 
checked with compiler.




+
+#define SN_BRIDGE_REVISION_ID 0x2
+
+/* Link Training specific registers */
+#define SN_DEVICE_REV_REG  0x08
+#define SN_REFCLK_FREQ_REG 0x0A
+#define SN_DSI_LANES_REG   0x10
+#define SN_DSIA_CLK_FREQ_REG   0x12
+#define SN_ENH_FRAME_REG   0x5A
+#define SN_SSC_CONFIG_REG  0x93
+#define SN_DATARATE_CONFIG_REG 0x94
+#define SN_PLL_ENABLE_REG  0x0D
+#define SN_SCRAMBLE_CONFIG_REG 0x95
+#define SN_AUX_WDATA0_REG  0x64
+#define SN_AUX_ADDR_19_16_REG  0x74
+#define SN_AUX_ADDR_15_8_REG   0x75
+#define SN_AUX_ADDR_7_0_REG0x76
+#define SN_AUX_LENGTH_REG  0x77
+#define SN_AUX_CMD_REG 0x78
+#define SN_ML_TX_MODE_REG  0x96
+#define SN_PAGE_SELECT_REG 0xFF
+#define SN_AUX_RDATA0_REG  0x79
+/* video config specific registers */
+#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG  0x20
+#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG 0x21
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG   0x24
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG  0x25
+#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG   0x2C
+#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG  0x2D
+#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG   0x30
+#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG  0x31
+#define SN_CHA_HORIZONTAL_BACK_PORCH_REG   0x34
+#define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36
+#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG  0x38
+#define SN_CHA_VERTICAL_FRONT_PORCH_REG0x3A
+#define SN_DATA_FORMAT_REG 0x5B
+#define SN_COLOR_BAR_CONFIG_REG0x3C
+
+#define DPPLL_CLK_SRC_REFCLK   0
+#define DPPLL_CLK_SRC_DSICLK   1
+#define MIN_DSI_CLK_FREQ_MHZ   40
+
+enum ti_sn_bridge_ref_clks {
+   SN_REF_CLK_12_MHZ = 0,
+   SN_REF_CLK_19_2_MHZ,
+   SN_REF_CLK_26_MHZ,
+   SN_REF_CLK_27_MHZ,
+   SN_REF_CLK_38_4_MHZ,
+   SN_REF_CLK_MAX,
+};
+
+struct ti_sn_bridge {
+   struct device *dev;
+   struct drm_bridge bridge;
+   struct drm_connector connector;
+   struct device_node *host_node;
+   struct mipi_dsi_device *dsi;
+   /* handle to the connected eDP panel */
+   struct drm_panel *panel;
+   int irq;
+   struct gpio_desc *irq_gpio;
+   /* list of gpios needed to enable the bridge functionality */
+   struct gpio_descs *enable_gpios;


Why are the enable gpios a list?


+   unsigned int num_supplies;
+   struct regulator_bulk_data *supplies;
+   struct i2c_client *i2c_client;
+   struct i2c_adapter *ddc;
+   bool power_on;
+   u32 num_modes;
+   struct drm_display_mode curr_mode;
+};
+
+static int ti_sn_bridge_write(struct ti_sn_bridge *pdata, u8 reg, u8 
val)

+{
+   struct i2c_client *client = pdata->i2c_client;
+   u8 buf[2] = {reg, val};
+   struct i2c_msg msg = {
+   .addr = client->addr,
+   .flags = 0,
+  

Re: [[RFC]DPU PATCH 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

[spanda]

On 2018-04-18 19:28, Sean Paul wrote:

On Wed, Apr 18, 2018 at 05:50:00PM +0530, Sandeep Panda wrote:

Document the bindings used for the sn65dsi86 DSI to eDP bridge.



Please add a changelog to your patches so reviewers know what has 
changed

between patch versions.

I will update the change log in the commit text of the patches.



Signed-off-by: Sandeep Panda 
---
 .../bindings/display/bridge/ti,sn65dsi86.txt   | 60 
++

 1 file changed, 60 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt


diff --git 
a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

new file mode 100644
index 000..9e2612e
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt

@@ -0,0 +1,60 @@
+SN65DSI86 DSI to eDP bridge chip
+
+
+This is the binding for Texas Instruments SN65DSI86 bridge.
+
+Required properties:
+- compatible: Must be "ti,sn65dsi86"
+- reg: i2c address of the chip, 0x2d as per datasheet
+- enable-gpios: OF device-tree gpio specifications for bridge_en pin


The datasheet only has one enable pin, why gpios?

This is the convention followed by other bridge drivers also.
https://www.kernel.org/doc/Documentation/devicetree/bindings/gpio/gpio.txt
please the example mentioned here,
Example of a node using GPIOs:

node {
enable-gpios = <_pio_e 18 GPIO_ACTIVE_HIGH>;
};




+
+- vccio-supply: A 1.8V supply that powers up the digital IOs.
+- vcca-supply: A 1.2V supply that powers up the analog circuits.
+
+Optional properties:
+
+- irq-gpios: OF device-tree gpio specification for interrupt pin


From the last review, Rob said, 'Use "interrupts" property instead.'

You didn't provide responses to prior review comments, so it's unclear 
whether

you've intentionally or unintentionally ignored him :)

Sorry missed it completely while addressing other review comments.


Sean


+
+Required nodes:
+
+This device has two video ports. Their connections are modelled using 
the
+OF graph bindings specified in 
Documentation/devicetree/bindings/graph.txt.

+
+- Video port 0 for DSI input
+- Video port 1 for eDP output
+
+Example
+---
+
+edp-bridge@2d {
+   compatible = "ti,sn65dsi86";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0x2d>;
+
+   enable-gpios = < 33 GPIO_ACTIVE_HIGH>;
+
+   vccio-supply = <_l17>;
+   vcca-supply = <_l6>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+
+   edp_bridge_in: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+
+   edp_bridge_out: endpoint {
+   remote-endpoint = <_panel_in>;
+   };
+   };
+   };
+}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,

a Linux Foundation Collaborative Project


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

Re: [[RFC]DPU PATCH 1/4] drm/bridge: add support for sn65dsi86 bridge driver

[spanda]

On 2018-04-18 19:41, Sean Paul wrote:

On Wed, Apr 18, 2018 at 05:49:59PM +0530, Sandeep Panda wrote:

Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

This chip can be controlled via either i2c interface or
dsi interface. Currently in driver all the control registers
are being accessed through i2c interface only.
Also as of now HPD support has not been added to bridge
chip driver.

Signed-off-by: Sandeep Panda 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 742 
++

 1 file changed, 742 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c

new file mode 100644
index 000..c798f2f
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -0,0 +1,742 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 


I have a hard time believing that you need all of these includes. Off 
the top of
my head, you could probably remove types, kernel, init, 
platform_device, delay,
drmP, drm_atomic, drm_crtc_helper. You could probably trim it even 
further with

the help of your compiler. These should also be in alphabetical order.


+
+#define SN_BRIDGE_REVISION_ID 0x2
+
+/* Link Training specific registers */
+#define SN_DEVICE_REV_REG  0x08
+#define SN_REFCLK_FREQ_REG 0x0A
+#define SN_DSI_LANES_REG   0x10
+#define SN_DSIA_CLK_FREQ_REG   0x12
+#define SN_ENH_FRAME_REG   0x5A
+#define SN_SSC_CONFIG_REG  0x93
+#define SN_DATARATE_CONFIG_REG 0x94
+#define SN_PLL_ENABLE_REG  0x0D
+#define SN_SCRAMBLE_CONFIG_REG 0x95
+#define SN_AUX_WDATA0_REG  0x64
+#define SN_AUX_ADDR_19_16_REG  0x74
+#define SN_AUX_ADDR_15_8_REG   0x75
+#define SN_AUX_ADDR_7_0_REG0x76
+#define SN_AUX_LENGTH_REG  0x77
+#define SN_AUX_CMD_REG 0x78
+#define SN_ML_TX_MODE_REG  0x96
+#define SN_PAGE_SELECT_REG 0xFF
+#define SN_AUX_RDATA0_REG  0x79
+/* video config specific registers */
+#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG  0x20
+#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG 0x21
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG   0x24
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG  0x25
+#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG   0x2C
+#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG  0x2D
+#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG   0x30
+#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG  0x31
+#define SN_CHA_HORIZONTAL_BACK_PORCH_REG   0x34
+#define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36
+#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG  0x38
+#define SN_CHA_VERTICAL_FRONT_PORCH_REG0x3A
+#define SN_DATA_FORMAT_REG 0x5B
+#define SN_COLOR_BAR_CONFIG_REG0x3C
+
+#define DPPLL_CLK_SRC_REFCLK   0
+#define DPPLL_CLK_SRC_DSICLK   1
+#define MIN_DSI_CLK_FREQ_MHZ   40
+
+enum ti_sn_bridge_ref_clks {
+   SN_REF_CLK_12_MHZ = 0,
+   SN_REF_CLK_19_2_MHZ,
+   SN_REF_CLK_26_MHZ,
+   SN_REF_CLK_27_MHZ,
+   SN_REF_CLK_38_4_MHZ,
+   SN_REF_CLK_MAX,
+};
+
+struct ti_sn_bridge {
+   struct device *dev;
+   struct drm_bridge bridge;
+   struct drm_connector connector;
+   struct device_node *host_node;
+   struct mipi_dsi_device *dsi;
+   /* handle to the connected eDP panel */
+   struct drm_panel *panel;
+   int irq;
+   struct gpio_desc *irq_gpio;
+   /* list of gpios needed to enable the bridge functionality */
+   struct gpio_descs *enable_gpios;


Why are the enable gpios a list?


This as per the review comment in previous patchset
"I see IRQ and EN in the datasheet, but not the others. It seems like 
the aux_*
and edp_* gpios are always equal to en. So if you actually need them, 
don't
specify a new struct, just add irq_gpio to the main struct and add an 
array of

en_gpios to handle the rest."

I see in schematic 2 more gpios are needed to enable aux_channel 
communication
through the ddc. So i have put an array of enable gpios. Based on dt it 
will dynamically

parse one or more gpios.




+   unsigned int num_supplies;
+   struct regulator_bulk_data *supplies;
+   struct i2c_client *i2c_client;
+   struct i2c_adapter *ddc;
+   bool power_on;
+   u32 num_modes;
+   struct drm_display_mode curr_mode;
+};
+
+static int ti_sn_bridge_write(struct ti_sn_bridge *pdata, u8 reg, u8 
val)

+{
+   struct i2c_client 

Re: [[RFC]DPU PATCH 1/2] drm/bridge: add support for sn65dsi86 bridge driver

[spanda]

On 2018-04-14 00:59, Sean Paul wrote:

On Fri, Apr 13, 2018 at 10:53:00AM +0530, Sandeep Panda wrote:

Add support for TI's sn65dsi86 dsi2edp bridge chip.
The chip converts DSI transmitted signal to eDP signal,
which is fed to the connected eDP panel.

This chip can be controlled via either i2c interface or
dsi interface. Currently in driver all the control registers
are being accessed through i2c interface only.
Also as of now HPD support has not been added to bridge
chip driver.

Signed-off-by: Sandeep Panda 


Hi Sandeep,
Thank you for your patch!


---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 1019 
+

 1 file changed, 1019 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c

new file mode 100644
index 000..93aa1ad
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -0,0 +1,1019 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#define pr_fmt(fmt) "%s: " fmt, __func__


Instead of using pr_* for logging, please use the DRM_* variants. Since 
there
is unlikely to be more than one of these bridge drivers in a system, 
you won't

need to use the DRM_DEV_* versions.


+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SN_DEVICE_REV_REG 0x08
+
+/* Link Training specific registers */
+#define SN_REFCLK_FREQ_REG 0x0A
+#define SN_DSI_LANES_REG 0x10
+#define SN_DSIA_CLK_FREQ_REG 0x12
+#define SN_ENH_FRAME_REG 0x5A
+#define SN_SSC_CONFIG_REG 0x93
+#define SN_DATARATE_CONFIG_REG 0x94
+#define SN_PLL_ENABLE_REG 0x0D
+#define SN_SCRAMBLE_CONFIG_REG 0x95
+#define SN_AUX_WDATA0_REG 0x64
+#define SN_AUX_ADDR_19_16_REG 0x74
+#define SN_AUX_ADDR_15_8_REG 0x75
+#define SN_AUX_ADDR_7_0_REG 0x76
+#define SN_AUX_LENGTH_REG 0x77
+#define SN_AUX_CMD_REG 0x78
+#define SN_ML_TX_MODE_REG 0x96
+#define SN_PAGE_SELECT_REG 0xFF
+#define SN_AUX_RDATA0_REG 0x79
+
+/* video config specific registers */
+#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG 0x20
+#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG 0x21
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG 0x24
+#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG 0x25
+#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG 0x2C
+#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG 0x2D
+#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG 0x30
+#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG 0x31
+#define SN_CHA_HORIZONTAL_BACK_PORCH_REG 0x34
+#define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36
+#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG 0x38
+#define SN_CHA_VERTICAL_FRONT_PORCH_REG 0x3A
+#define SN_DATA_FORMAT_REG 0x5B
+#define SN_COLOR_BAR_CONFIG_REG 0x3C


It'd be nice if you could tab-align the values.


+
+struct sn65dsi86_reg_cfg {
+   u8 reg;
+   u8 val;
+   int sleep_in_ms;
+};
+
+struct sn65dsi86_video_cfg {
+   u32 h_active;
+   u32 h_front_porch;
+   u32 h_pulse_width;
+   u32 h_back_porch;
+   bool h_polarity;
+   u32 v_active;
+   u32 v_front_porch;
+   u32 v_pulse_width;
+   u32 v_back_porch;
+   bool v_polarity;
+   u32 pclk_khz;
+   u32 num_of_lanes;
+};


All of these can be derived from drm_display_mode except for 
num_of_lanes which

is hardcoded. So let's just use drm_display_mode directly.


+
+struct sn65dsi86_gpios {
+   struct gpio_desc *irq_gpio;
+   struct gpio_desc *enable_gpio;
+   struct gpio_desc *aux_i2c_sda;
+   struct gpio_desc *aux_i2c_scl;
+   struct gpio_desc *edp_bias_en;
+   struct gpio_desc *edp_bklt_en;
+   struct gpio_desc *edp_bklt_ctrl;
+};


I see IRQ and EN in the datasheet, but not the others. It seems like 
the aux_*
and edp_* gpios are always equal to en. So if you actually need them, 
don't
specify a new struct, just add irq_gpio to the main struct and add an 
array of

en_gpios to handle the rest.


+
+struct sn65dsi86 {


I will have fits trying to type this name. Could you please use 
something
simple, like sn_bridge? Same comment applies to all of the function 
names.



+   struct device *dev;
+   struct drm_bridge bridge;
+   struct drm_connector connector;
+
+   struct device_node *host_node;
+   struct mipi_dsi_device *dsi;
+
+   u8 i2c_addr;


Unused


+   int irq;
+
+   struct sn65dsi86_gpios gpios;
+
+   unsigned int num_supplies;
+   struct regulator_bulk_data *supplies;
+
+   struct i2c_client *i2c_client;
+
+   enum drm_connector_status connector_status;


You never use the value of this, you just assign it. So you can remove 
this.



+   bool power_on;
+
+   bool is_pluggable;


As I mentioned in the dt review, you can determine this via panel. You 
should

also take this into account in detect().


+   u32 num_of_modes;
+   struct list_head mode_list;
+   struct edid *edid;
+
+   struct