[PATCH v2 0/5] drm/panel-simple: Patches for N116BCA-EA1

[PATCH v2 0/5] drm/panel-simple: Patches for N116BCA-EA1

[Douglas Anderson]

This series is to get the N116BCA-EA1 panel working. Most of the
patches are simple, but on hardware I have in front of me the panel
sometimes doesn't come up. I'm still working with the hardware
manufacturer to get to the bottom of it, but I've got it working with
retries. Adding the retries doesn't seem like an insane thing to do
and makes some of the error handling more robust, so I've gone ahead
and included those patches here. Hopefully they look OK.

Changes in v2:
- Set the "unprepared_time" so if we retry we give the proper delay.
- ("drm/panel-simple: Don't wait longer for HPD...") new for v2.
- ("drm/panel-simple: Retry if we timeout waiting for HPD") new for v2.
- ("dt-bindings: dt-bindings: display: simple: Add N116BCA-EA1") new for v2.
- ("drm/panel-simple: Add N116BCA-EA1") new for v2.

Douglas Anderson (5):
  drm/panel-simple: Undo enable if HPD never asserts
  drm/panel-simple: Don't wait longer for HPD than hpd_absent_delay
  drm/panel-simple: Retry if we timeout waiting for HPD
  dt-bindings: dt-bindings: display: simple: Add N116BCA-EA1
  drm/panel-simple: Add N116BCA-EA1

 .../bindings/display/panel/panel-simple.yaml  |  2 +
 drivers/gpu/drm/panel/panel-simple.c          | 84 +++++++++++++++++--
 2 files changed, 80 insertions(+), 6 deletions(-)

-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 1/5] drm/panel-simple: Undo enable if HPD never asserts

[Douglas Anderson]

If the HPD signal never asserts in panel_simple_prepare() and we
return an error, we should unset the enable GPIO and disable the
regulator to make it consistent for the caller.

At the moment I have some hardware where HPD sometimes doesn't assert.
Obviously that needs to be debugged, but this patch makes it so that
if I add a retry that I can make things work.

Fixes: 48834e6084f1 ("drm/panel-simple: Support hpd-gpios for delaying prepare()")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Stephen: I didn't take your Reviewed-by tag since I made a small
change.  If you're OK with this change please re-add.

Changes in v2:
- Set the "unprepared_time" so if we retry we give the proper delay.

 drivers/gpu/drm/panel/panel-simple.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 71ae200ac48a..581ab6810b70 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -406,7 +406,7 @@ static int panel_simple_prepare(struct drm_panel *panel)
 		if (IS_ERR(p->hpd_gpio)) {
 			err = panel_simple_get_hpd_gpio(panel->dev, p, false);
 			if (err)
-				return err;
+				goto error;
 		}
 
 		err = readx_poll_timeout(gpiod_get_value_cansleep, p->hpd_gpio,
@@ -418,13 +418,20 @@ static int panel_simple_prepare(struct drm_panel *panel)
 		if (err) {
 			dev_err(panel->dev,
 				"error waiting for hpd GPIO: %d\n", err);
-			return err;
+			goto error;
 		}
 	}
 
 	p->prepared_time = ktime_get();
 
 	return 0;
+
+error:
+	gpiod_set_value_cansleep(p->enable_gpio, 0);
+	regulator_disable(p->supply);
+	p->unprepared_time = ktime_get();
+
+	return err;
 }
 
 static int panel_simple_enable(struct drm_panel *panel)
-- 
2.30.0.284.gd98b1dd5eaa7-goog

Re: [PATCH v2 1/5] drm/panel-simple: Undo enable if HPD never asserts

[Stephen Boyd]

Quoting Douglas Anderson (2021-01-15 14:44:16)
> If the HPD signal never asserts in panel_simple_prepare() and we
> return an error, we should unset the enable GPIO and disable the
> regulator to make it consistent for the caller.
> 
> At the moment I have some hardware where HPD sometimes doesn't assert.
> Obviously that needs to be debugged, but this patch makes it so that
> if I add a retry that I can make things work.
> 
> Fixes: 48834e6084f1 ("drm/panel-simple: Support hpd-gpios for delaying prepare()")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

Nice catch on the unprepared_time

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

[PATCH v2 2/5] drm/panel-simple: Don't wait longer for HPD than hpd_absent_delay

[Douglas Anderson]

If a panel has an hpd_absent_delay specified then we know exactly how
long the maximum time is before HPD must be asserted.  That means we
can use it as a timeout for polling the HPD pin instead of using an
arbitrary timeout.  This is especially useful for dealing with panels
that periodically fail to power on and need to be retried.  We can
detect the problem sooner.

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

Changes in v2:
- ("drm/panel-simple: Don't wait longer for HPD...") new for v2.

 drivers/gpu/drm/panel/panel-simple.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 581ab6810b70..30842cf6d414 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -382,6 +382,7 @@ static int panel_simple_prepare(struct drm_panel *panel)
 	unsigned int delay;
 	int err;
 	int hpd_asserted;
+	unsigned long hpd_wait_us;
 
 	if (p->prepared_time != 0)
 		return 0;
@@ -409,9 +410,14 @@ static int panel_simple_prepare(struct drm_panel *panel)
 				goto error;
 		}
 
+		if (p->desc->delay.hpd_absent_delay)
+			hpd_wait_us = p->desc->delay.hpd_absent_delay * 1000UL;
+		else
+			hpd_wait_us = 2000000;
+
 		err = readx_poll_timeout(gpiod_get_value_cansleep, p->hpd_gpio,
 					 hpd_asserted, hpd_asserted,
-					 1000, 2000000);
+					 1000, hpd_wait_us);
 		if (hpd_asserted < 0)
 			err = hpd_asserted;
 
-- 
2.30.0.284.gd98b1dd5eaa7-goog

Re: [PATCH v2 2/5] drm/panel-simple: Don't wait longer for HPD than hpd_absent_delay

[Stephen Boyd]

Quoting Douglas Anderson (2021-01-15 14:44:17)
> If a panel has an hpd_absent_delay specified then we know exactly how
> long the maximum time is before HPD must be asserted.  That means we
> can use it as a timeout for polling the HPD pin instead of using an
> arbitrary timeout.  This is especially useful for dealing with panels
> that periodically fail to power on and need to be retried.  We can
> detect the problem sooner.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

[PATCH v2 3/5] drm/panel-simple: Retry if we timeout waiting for HPD

[Douglas Anderson]

On an Innolux N116BCA panel that I have in front of me, sometimes HPD
simply doesn't assert no matter how long you wait for it. As per the
very wise advice of The IT Crowd ("Have you tried turning it off and
on again?") it appears that power cycling is enough to kick this panel
back into a sane state.

From tests on this panel, it appears that leaving it powered off for a
while stimulates the problem. Adding a 6 second sleep at the start of
panel_simple_prepare_once() makes it happen fairly reliably and, with
this delay, I saw up to 3 retries needed sometimes. Without the 6
second sleep, however, the panel came up much more reliably the first
time or after only 1 retry.

While it's unknown what the problems are with this panel (and probably
the hardware should be debugged), adding a few retries to the power on
routine doesn't seem insane. Even if this panel's problems are
attributed to the fact that it's pre-production and/or can be fixed,
retries clearly can help in some cases and really don't hurt.

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

Changes in v2:
- ("drm/panel-simple: Retry if we timeout waiting for HPD") new for v2.

 drivers/gpu/drm/panel/panel-simple.c | 32 +++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 30842cf6d414..823177d89d1b 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -376,7 +376,7 @@ static int panel_simple_get_hpd_gpio(struct device *dev,
 	return 0;
 }
 
-static int panel_simple_prepare(struct drm_panel *panel)
+static int panel_simple_prepare_once(struct drm_panel *panel)
 {
 	struct panel_simple *p = to_panel_simple(panel);
 	unsigned int delay;
@@ -422,8 +422,9 @@ static int panel_simple_prepare(struct drm_panel *panel)
 			err = hpd_asserted;
 
 		if (err) {
-			dev_err(panel->dev,
-				"error waiting for hpd GPIO: %d\n", err);
+			if (err != -ETIMEDOUT)
+				dev_err(panel->dev,
+					"error waiting for hpd GPIO: %d\n", err);
 			goto error;
 		}
 	}
@@ -440,6 +441,31 @@ static int panel_simple_prepare(struct drm_panel *panel)
 	return err;
 }
 
+/*
+ * Some panels simply don't always come up and need to be power cycled to
+ * work properly.  We'll allow for a handful of retries.
+ */
+#define MAX_PANEL_PREPARE_TRIES		5
+
+static int panel_simple_prepare(struct drm_panel *panel)
+{
+	int ret;
+	int try;
+
+	for (try = 0; try < MAX_PANEL_PREPARE_TRIES; try++) {
+		ret = panel_simple_prepare_once(panel);
+		if (ret != -ETIMEDOUT)
+			break;
+	}
+
+	if (ret == -ETIMEDOUT)
+		dev_err(panel->dev, "Prepare timeout after %d tries\n", try);
+	else if (try)
+		dev_warn(panel->dev, "Prepare needed %d retries\n", try);
+
+	return ret;
+}
+
 static int panel_simple_enable(struct drm_panel *panel)
 {
 	struct panel_simple *p = to_panel_simple(panel);
-- 
2.30.0.284.gd98b1dd5eaa7-goog

Re: [PATCH v2 3/5] drm/panel-simple: Retry if we timeout waiting for HPD

[Stephen Boyd]

Quoting Douglas Anderson (2021-01-15 14:44:18)
> On an Innolux N116BCA panel that I have in front of me, sometimes HPD
> simply doesn't assert no matter how long you wait for it. As per the
> very wise advice of The IT Crowd ("Have you tried turning it off and
> on again?") it appears that power cycling is enough to kick this panel
> back into a sane state.
> 
> From tests on this panel, it appears that leaving it powered off for a
> while stimulates the problem. Adding a 6 second sleep at the start of
> panel_simple_prepare_once() makes it happen fairly reliably and, with
> this delay, I saw up to 3 retries needed sometimes. Without the 6
> second sleep, however, the panel came up much more reliably the first
> time or after only 1 retry.
> 
> While it's unknown what the problems are with this panel (and probably
> the hardware should be debugged), adding a few retries to the power on
> routine doesn't seem insane. Even if this panel's problems are
> attributed to the fact that it's pre-production and/or can be fixed,
> retries clearly can help in some cases and really don't hurt.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

> @@ -440,6 +441,31 @@ static int panel_simple_prepare(struct drm_panel *panel)
>         return err;
>  }
>  
> +/*
> + * Some panels simply don't always come up and need to be power cycled to
> + * work properly.  We'll allow for a handful of retries.
> + */
> +#define MAX_PANEL_PREPARE_TRIES                5

Is this define used anywhere else? Feels like it would be better to
inline the constant and move the comment above the loop, but I guess
this is OK too.

> +
> +static int panel_simple_prepare(struct drm_panel *panel)
> +{
> +       int ret;
> +       int try;
> +
> +       for (try = 0; try < MAX_PANEL_PREPARE_TRIES; try++) {
> +               ret = panel_simple_prepare_once(panel);
> +               if (ret != -ETIMEDOUT)
> +                       break;
> +       }
> +
> +       if (ret == -ETIMEDOUT)
> +               dev_err(panel->dev, "Prepare timeout after %d tries\n", try);
> +       else if (try)
> +               dev_warn(panel->dev, "Prepare needed %d retries\n", try);
> +

Re: [PATCH v2 3/5] drm/panel-simple: Retry if we timeout waiting for HPD

[Doug Anderson]

Hi,

On Mon, Jan 25, 2021 at 12:28 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> > +/*
> > + * Some panels simply don't always come up and need to be power cycled to
> > + * work properly.  We'll allow for a handful of retries.
> > + */
> > +#define MAX_PANEL_PREPARE_TRIES                5
>
> Is this define used anywhere else? Feels like it would be better to
> inline the constant and move the comment above the loop, but I guess
> this is OK too.

Hrm, usually I only add a #define like this when I need to use the
same number more than once, but I'm not doing that here.  Maybe I did
in an earlier version of the code and then didn't go back and remove
the #define when I reworked it?

Since this is a bit of a style issue, I will leave it to the
simple-panel maintainers to decide.  I'm happy to spin this and add
the comment before the loop and just hardcode "5" in the loop instead
of using MAX_PANEL_PREPARE_TRIES, so just let me know.

-Doug

[PATCH v2 4/5] dt-bindings: dt-bindings: display: simple: Add N116BCA-EA1

[Douglas Anderson]

Another simple eDP panel.

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

Changes in v2:
- ("dt-bindings: dt-bindings: display: simple: Add N116BCA-EA1") new for v2.

 .../devicetree/bindings/display/panel/panel-simple.yaml         | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index 35b42ee4ed1d..a2db2a8def15 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -160,6 +160,8 @@ properties:
         # Innolux Corporation 12.1" G121X1-L03 XGA (1024x768) TFT LCD panel
       - innolux,g121x1-l03
         # Innolux Corporation 11.6" WXGA (1366x768) TFT LCD panel
+      - innolux,n116bca-ea1
+        # Innolux Corporation 11.6" WXGA (1366x768) TFT LCD panel
       - innolux,n116bge
         # InnoLux 13.3" FHD (1920x1080) eDP TFT LCD panel
       - innolux,n125hce-gn1
-- 
2.30.0.284.gd98b1dd5eaa7-goog

Re: [PATCH v2 4/5] dt-bindings: dt-bindings: display: simple: Add N116BCA-EA1

[Doug Anderson]

Hi,

On Fri, Jan 15, 2021 at 2:44 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> Another simple eDP panel.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v2:
> - ("dt-bindings: dt-bindings: display: simple: Add N116BCA-EA1") new for v2.
>
>  .../devicetree/bindings/display/panel/panel-simple.yaml         | 2 ++
>  1 file changed, 2 insertions(+)

As per always I find my typos right after I hit send.  ${SUBJECT} has
one too many copies of the "dt-bindings:" tag.  I will assume this can
be fixed when it's applied.  Please yell if you'd rather I spam a
repost.  ;-)

-Doug

Re: [PATCH v2 4/5] dt-bindings: dt-bindings: display: simple: Add N116BCA-EA1

[Stephen Boyd]

Quoting Douglas Anderson (2021-01-15 14:44:19)
> Another simple eDP panel.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

With subject fixed

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v2 4/5] dt-bindings: dt-bindings: display: simple: Add N116BCA-EA1

[Rob Herring]

On Fri, 15 Jan 2021 14:44:19 -0800, Douglas Anderson wrote:
> Another simple eDP panel.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
> Changes in v2:
> - ("dt-bindings: dt-bindings: display: simple: Add N116BCA-EA1") new for v2.
> 
>  .../devicetree/bindings/display/panel/panel-simple.yaml         | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v2 5/5] drm/panel-simple: Add N116BCA-EA1

[Douglas Anderson]

This panel is quite similar to the similarly named N116BGE panel (the
nominal timings are, in fact identical).  However, let's add a new
entry because the full range of clocks listed for N116BGE aren't
supported for N116BCA-EA1, at least according to the datasheet.

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

Changes in v2:
- ("drm/panel-simple: Add N116BCA-EA1") new for v2.

 drivers/gpu/drm/panel/panel-simple.c | 33 ++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 823177d89d1b..76a1bebaaaf9 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2407,6 +2407,36 @@ static const struct panel_desc innolux_g121x1_l03 = {
 	},
 };
 
+static const struct drm_display_mode innolux_n116bca_ea1_mode = {
+	.clock = 76420,
+	.hdisplay = 1366,
+	.hsync_start = 1366 + 136,
+	.hsync_end = 1366 + 136 + 30,
+	.htotal = 1366 + 136 + 30 + 60,
+	.vdisplay = 768,
+	.vsync_start = 768 + 8,
+	.vsync_end = 768 + 8 + 12,
+	.vtotal = 768 + 8 + 12 + 12,
+	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+static const struct panel_desc innolux_n116bca_ea1 = {
+	.modes = &innolux_n116bca_ea1_mode,
+	.num_modes = 1,
+	.bpc = 6,
+	.size = {
+		.width = 256,
+		.height = 144,
+	},
+	.delay = {
+		.hpd_absent_delay = 200,
+		.prepare_to_enable = 80,
+		.unprepare = 500,
+	},
+	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
+	.connector_type = DRM_MODE_CONNECTOR_eDP,
+};
+
 /*
  * Datasheet specifies that at 60 Hz refresh rate:
  * - total horizontal time: { 1506, 1592, 1716 }
@@ -4320,6 +4350,9 @@ static const struct of_device_id platform_of_match[] = {
 	}, {
 		.compatible = "innolux,g121x1-l03",
 		.data = &innolux_g121x1_l03,
+	}, {
+		.compatible = "innolux,n116bca-ea1",
+		.data = &innolux_n116bca_ea1,
 	}, {
 		.compatible = "innolux,n116bge",
 		.data = &innolux_n116bge,
-- 
2.30.0.284.gd98b1dd5eaa7-goog

Re: [PATCH v2 5/5] drm/panel-simple: Add N116BCA-EA1

[Stephen Boyd]

Quoting Douglas Anderson (2021-01-15 14:44:20)
> This panel is quite similar to the similarly named N116BGE panel (the
> nominal timings are, in fact identical).  However, let's add a new
> entry because the full range of clocks listed for N116BGE aren't
> supported for N116BCA-EA1, at least according to the datasheet.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v2 0/5] drm/panel-simple: Patches for N116BCA-EA1

[Doug Anderson]

Hi folks,

On Fri, Jan 15, 2021 at 2:44 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> This series is to get the N116BCA-EA1 panel working. Most of the
> patches are simple, but on hardware I have in front of me the panel
> sometimes doesn't come up. I'm still working with the hardware
> manufacturer to get to the bottom of it, but I've got it working with
> retries. Adding the retries doesn't seem like an insane thing to do
> and makes some of the error handling more robust, so I've gone ahead
> and included those patches here. Hopefully they look OK.
>
> Changes in v2:
> - Set the "unprepared_time" so if we retry we give the proper delay.
> - ("drm/panel-simple: Don't wait longer for HPD...") new for v2.
> - ("drm/panel-simple: Retry if we timeout waiting for HPD") new for v2.
> - ("dt-bindings: dt-bindings: display: simple: Add N116BCA-EA1") new for v2.
> - ("drm/panel-simple: Add N116BCA-EA1") new for v2.
>
> Douglas Anderson (5):
>   drm/panel-simple: Undo enable if HPD never asserts
>   drm/panel-simple: Don't wait longer for HPD than hpd_absent_delay
>   drm/panel-simple: Retry if we timeout waiting for HPD
>   dt-bindings: dt-bindings: display: simple: Add N116BCA-EA1
>   drm/panel-simple: Add N116BCA-EA1
>
>  .../bindings/display/panel/panel-simple.yaml  |  2 +
>  drivers/gpu/drm/panel/panel-simple.c          | 84 +++++++++++++++++--
>  2 files changed, 80 insertions(+), 6 deletions(-)

While this isn't massively urgent, I'm hoping to get some confirmation
that it's still in someone's queue to look at.  A quick "it's still in
my queue" would be much appreciated!  :-)  If I don't hear anything
then I guess next week I'll see if I can find other ways to poke folks
or find a different route to land this series.  Thanks!

-Doug

Re: [PATCH v2 0/5] drm/panel-simple: Patches for N116BCA-EA1

[Linus Walleij]

On Fri, Jan 15, 2021 at 11:44 PM Douglas Anderson <dianders@chromium.org> wrote:

> - ("drm/panel-simple: Don't wait longer for HPD...") new for v2.
> - ("drm/panel-simple: Retry if we timeout waiting for HPD") new for v2.

I couldn't find these patches in my inbox but my concern would
be that at some point panel-simple will turn from simple into
panel-rube-goldberg-machine.

Given that the talk with the manufacturer may result
in even more quirks... maybe this should just be a separate
panel driver?

(I expect pushback because I see how handy it is, but
I am the guy writing new panel drivers all the time rather than
using simple.)

Yours,
Linus Walleij

Re: [PATCH v2 0/5] drm/panel-simple: Patches for N116BCA-EA1

[Doug Anderson]

Hi,

On Wed, Mar 10, 2021 at 3:25 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Fri, Jan 15, 2021 at 11:44 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> > - ("drm/panel-simple: Don't wait longer for HPD...") new for v2.
> > - ("drm/panel-simple: Retry if we timeout waiting for HPD") new for v2.
>
> I couldn't find these patches in my inbox

Doh! Sorry about that. I think get_maintainer tagged you only on the
patches that had the explicit "fixes" in them on something you were
involved in. I tend to rely on get_maintainer heavily unless I think
someone is particularly interested in the whole series. I will make
sure to CC you on the whole series if I post it again. In the meantime
the whole series is on lore:

https://lore.kernel.org/lkml/20210115224420.1635017-1-dianders@chromium.org/

> but my concern would
> be that at some point panel-simple will turn from simple into
> panel-rube-goldberg-machine.

Yes, it's definitely something to watch out for. I guess you're
commenting on the general number of changes I've made to simple-panel
in the last year or two? I guess my comment on those:

* Many of the changes I made were around HPD handling and supporting a
GPIO (and also supporting the "hpd absent delay"). The fact that we
use a GPIO for HPD is actually an attribute of our board design,
though, so if I had forked panel drivers for each of the panels that
needed it then it would have required copying the code lots of places
(or implementing some code sharing). I was specifically asked to do
the HPD handling code at the panel layer.

* The other big change I made recently was around allowing for
relative rather than absolute timings (instead of a fixed delay at a
given stage adding a constraint that enough time had passed since a
previous event). When I proposed that the feedback I got back was that
it was a good improvement for all panels and something that had been
on a TODO list for a while.

...so while it feels like I keep piling crap onto simple-panel, I
_think_ they've been good general improvements that many people will
be able to benefit from and I don't think they've uglified things
lots.

> Given that the talk with the manufacturer may result
> in even more quirks... maybe this should just be a separate
> panel driver?

I don't _think_ this will end up with more quirks. At least I sure
hope not. So far it seems like pure luck that I even noticed it
because only one other device in the whole batch produced had similar
problems. That leads me to believe that there could be other panels
with a similar need.

> (I expect pushback because I see how handy it is, but
> I am the guy writing new panel drivers all the time rather than
> using simple.)

I guess what I'd say in summary is:

* If you object to the retries in simple panel, I still hope the rest
of the series can land.

* If somehow this panel gets out into real users hands and we find
that the retries are necessary and people still don't want the retries
in simple panel, I can fork a special panel driver just for it then.

-Doug

Re: [PATCH v2 0/5] drm/panel-simple: Patches for N116BCA-EA1

[Linus Walleij]

On Thu, Mar 11, 2021 at 12:47 AM Doug Anderson <dianders@chromium.org> wrote:

> I guess what I'd say in summary is:
>
> * If you object to the retries in simple panel, I still hope the rest
> of the series can land.
>
> * If somehow this panel gets out into real users hands and we find
> that the retries are necessary and people still don't want the retries
> in simple panel, I can fork a special panel driver just for it then.

I'm fine with the retries, if it is needed outside of the "simple" (hm)
panel driver then we can certainly factor it out as a helper or
library.

I looked at the patches at lore.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
I see also Stephen has reviewed some patches.

Tell me if you need me to also apply them to drm-misc.
(I guess yes?)

Yours,
Linus Walleij

Re: [PATCH v2 0/5] drm/panel-simple: Patches for N116BCA-EA1

[Doug Anderson]

Hi,

On Wed, Mar 10, 2021 at 4:57 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Mar 11, 2021 at 12:47 AM Doug Anderson <dianders@chromium.org> wrote:
>
> > I guess what I'd say in summary is:
> >
> > * If you object to the retries in simple panel, I still hope the rest
> > of the series can land.
> >
> > * If somehow this panel gets out into real users hands and we find
> > that the retries are necessary and people still don't want the retries
> > in simple panel, I can fork a special panel driver just for it then.
>
> I'm fine with the retries, if it is needed outside of the "simple" (hm)
> panel driver then we can certainly factor it out as a helper or
> library.
>
> I looked at the patches at lore.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> I see also Stephen has reviewed some patches.
>
> Tell me if you need me to also apply them to drm-misc.
> (I guess yes?)

Yes please. I was giving Sam time to do it but I haven't heard from
him for a while. Right before you responded I poked Thierry to see if
he was available but if you're willing/able to do it then I'm sure it
would save him the trouble.

If you'd like me to re-post the patches (CCing you) I can. Please let me know.

If you happen to feel in an applying mood one other patch to
simple-panel I think is OK to land is at:

https://lore.kernel.org/r/20210222081716.1.I1a45aece5d2ac6a2e73bbec50da2086e43e0862b@changeid

-Doug

Re: [PATCH v2 0/5] drm/panel-simple: Patches for N116BCA-EA1

[Linus Walleij]

On Thu, Mar 11, 2021 at 2:01 AM Doug Anderson <dianders@chromium.org> wrote:

> If you happen to feel in an applying mood one other patch to
> simple-panel I think is OK to land is at:
>
> https://lore.kernel.org/r/20210222081716.1.I1a45aece5d2ac6a2e73bbec50da2086e43e0862b@changeid

I applied and pushed this as well.

Yours,
Linus Walleij

Re: [PATCH v2 0/5] drm/panel-simple: Patches for N116BCA-EA1

[Linus Walleij]

On Fri, Jan 15, 2021 at 11:44 PM Douglas Anderson <dianders@chromium.org> wrote:

> This series is to get the N116BCA-EA1 panel working. Most of the
> patches are simple, but on hardware I have in front of me the panel
> sometimes doesn't come up. I'm still working with the hardware
> manufacturer to get to the bottom of it, but I've got it working with
> retries. Adding the retries doesn't seem like an insane thing to do
> and makes some of the error handling more robust, so I've gone ahead
> and included those patches here. Hopefully they look OK.
>
> Changes in v2:

This v2 version applied to drm-misc-next.

Yours,
Linus Walleij