[PATCH 0/3] New eDP panels and a bugfix

[PATCH 0/3] New eDP panels and a bugfix

[Nícolas F. R. A. Prado]

This series adds two new eDP panel entries in the first two patches, and
fixes a typo in the third one that prevented usage of the DT properties.

Please see the note in patch 1 for a question on hpd_reliable vs.
hpd_absent.


Nícolas F. R. A. Prado (3):
  drm/panel-edp: Add panel entry for R140NWF5 RH
  drm/panel-edp: Add panel entry for B120XAN01.0
  drm/panel-edp: Fix variable typo when saving hpd absent delay from DT

 drivers/gpu/drm/panel/panel-edp.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

-- 
2.37.0

[PATCH 0/3] New eDP panels and a bugfix

[Nícolas F. R. A. Prado]

This series adds two new eDP panel entries in the first two patches, and
fixes a typo in the third one that prevented usage of the DT properties.

Please see the note in patch 1 for a question on hpd_reliable vs.
hpd_absent.


Nícolas F. R. A. Prado (3):
  drm/panel-edp: Add panel entry for R140NWF5 RH
  drm/panel-edp: Add panel entry for B120XAN01.0
  drm/panel-edp: Fix variable typo when saving hpd absent delay from DT

 drivers/gpu/drm/panel/panel-edp.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

-- 
2.37.0

[PATCH 1/3] drm/panel-edp: Add panel entry for R140NWF5 RH

[Nícolas F. R. A. Prado]

Add panel identification entry for the IVO R140NWF5 RH (product ID:
0x057d) panel.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

---
The comments on the driver indicate that the T3 timing should be set on
hpd_absent, while hpd_reliable would have a shorter time just while the
HPD line stabilizes on low after power is supplied.

But can we really assume that the HPD line will be reliable at all
before the DDIC is done booting up, at which point the HPD line is
brought up? IOW, shouldn't we always delay T3 (by setting hpd_reliable =
T3), since only then we're really sure that the DDIC is done setting up
and the HPD line is reliable?

I've set the T3 delay to hpd_absent in this series, following what's
instructed in the comments, but I'd like to discuss whether we shouldn't
be setting T3 on hpd_reliable instead, for all panels, to be on the
safer side.

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

diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index 3626469c4cc2..675d793d925e 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1854,6 +1854,12 @@ static const struct panel_delay delay_100_500_e200 = {
 	.enable = 200,
 };
 
+static const struct panel_delay delay_200_500_e200 = {
+	.hpd_absent = 200,
+	.unprepare = 500,
+	.enable = 200,
+};
+
 #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
 { \
 	.name = _name, \
@@ -1882,6 +1888,8 @@ static const struct edp_panel_entry edp_panels[] = {
 
 	EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay, "N116BCA-EA1"),
 
+	EDP_PANEL_ENTRY('I', 'V', 'O', 0x057d, &delay_200_500_e200, "R140NWF5 RH"),
+
 	EDP_PANEL_ENTRY('K', 'D', 'B', 0x0624, &kingdisplay_kd116n21_30nv_a010.delay, "116N21-30NV-A010"),
 	EDP_PANEL_ENTRY('K', 'D', 'B', 0x1120, &delay_200_500_e80_d50, "116N29-30NK-C007"),
 
-- 
2.37.0

[PATCH 1/3] drm/panel-edp: Add panel entry for R140NWF5 RH

[Nícolas F. R. A. Prado]

Add panel identification entry for the IVO R140NWF5 RH (product ID:
0x057d) panel.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

---
The comments on the driver indicate that the T3 timing should be set on
hpd_absent, while hpd_reliable would have a shorter time just while the
HPD line stabilizes on low after power is supplied.

But can we really assume that the HPD line will be reliable at all
before the DDIC is done booting up, at which point the HPD line is
brought up? IOW, shouldn't we always delay T3 (by setting hpd_reliable =
T3), since only then we're really sure that the DDIC is done setting up
and the HPD line is reliable?

I've set the T3 delay to hpd_absent in this series, following what's
instructed in the comments, but I'd like to discuss whether we shouldn't
be setting T3 on hpd_reliable instead, for all panels, to be on the
safer side.

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

diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index 3626469c4cc2..675d793d925e 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1854,6 +1854,12 @@ static const struct panel_delay delay_100_500_e200 = {
 	.enable = 200,
 };
 
+static const struct panel_delay delay_200_500_e200 = {
+	.hpd_absent = 200,
+	.unprepare = 500,
+	.enable = 200,
+};
+
 #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
 { \
 	.name = _name, \
@@ -1882,6 +1888,8 @@ static const struct edp_panel_entry edp_panels[] = {
 
 	EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay, "N116BCA-EA1"),
 
+	EDP_PANEL_ENTRY('I', 'V', 'O', 0x057d, &delay_200_500_e200, "R140NWF5 RH"),
+
 	EDP_PANEL_ENTRY('K', 'D', 'B', 0x0624, &kingdisplay_kd116n21_30nv_a010.delay, "116N21-30NV-A010"),
 	EDP_PANEL_ENTRY('K', 'D', 'B', 0x1120, &delay_200_500_e80_d50, "116N29-30NK-C007"),
 
-- 
2.37.0

[PATCH 2/3] drm/panel-edp: Add panel entry for B120XAN01.0

[Nícolas F. R. A. Prado]

Add panel identification entry for the AUO B120XAN01.0 (product ID:
0x1062) panel.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---

 drivers/gpu/drm/panel/panel-edp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index 675d793d925e..152e00eb846f 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1879,6 +1879,7 @@ static const struct edp_panel_entry edp_panels[] = {
 	EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01"),
 	EDP_PANEL_ENTRY('A', 'U', 'O', 0x615c, &delay_200_500_e50, "B116XAN06.1"),
 	EDP_PANEL_ENTRY('A', 'U', 'O', 0x8594, &delay_200_500_e50, "B133UAN01.0"),
+	EDP_PANEL_ENTRY('A', 'U', 'O', 0x1062, &delay_200_500_e50, "B120XAN01.0"),
 
 	EDP_PANEL_ENTRY('B', 'O', 'E', 0x0786, &delay_200_500_p2e80, "NV116WHM-T01"),
 	EDP_PANEL_ENTRY('B', 'O', 'E', 0x07d1, &boe_nv133fhm_n61.delay, "NV133FHM-N61"),
-- 
2.37.0

[PATCH 2/3] drm/panel-edp: Add panel entry for B120XAN01.0

[Nícolas F. R. A. Prado]

Add panel identification entry for the AUO B120XAN01.0 (product ID:
0x1062) panel.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---

 drivers/gpu/drm/panel/panel-edp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index 675d793d925e..152e00eb846f 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1879,6 +1879,7 @@ static const struct edp_panel_entry edp_panels[] = {
 	EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01"),
 	EDP_PANEL_ENTRY('A', 'U', 'O', 0x615c, &delay_200_500_e50, "B116XAN06.1"),
 	EDP_PANEL_ENTRY('A', 'U', 'O', 0x8594, &delay_200_500_e50, "B133UAN01.0"),
+	EDP_PANEL_ENTRY('A', 'U', 'O', 0x1062, &delay_200_500_e50, "B120XAN01.0"),
 
 	EDP_PANEL_ENTRY('B', 'O', 'E', 0x0786, &delay_200_500_p2e80, "NV116WHM-T01"),
 	EDP_PANEL_ENTRY('B', 'O', 'E', 0x07d1, &boe_nv133fhm_n61.delay, "NV133FHM-N61"),
-- 
2.37.0

[PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT

[Nícolas F. R. A. Prado]

The value read from the "hpd-absent-delay-ms" property in DT was being
saved to the wrong variable, overriding the hpd_reliable delay. Fix the
typo.

Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by EDID")
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---

 drivers/gpu/drm/panel/panel-edp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index 152e00eb846f..b3536d8600f4 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -738,7 +738,7 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
 	of_property_read_u32(dev->of_node, "hpd-reliable-delay-ms", &reliable_ms);
 	desc->delay.hpd_reliable = reliable_ms;
 	of_property_read_u32(dev->of_node, "hpd-absent-delay-ms", &absent_ms);
-	desc->delay.hpd_reliable = absent_ms;
+	desc->delay.hpd_absent = absent_ms;
 
 	/* Power the panel on so we can read the EDID */
 	ret = pm_runtime_get_sync(dev);
-- 
2.37.0

[PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT

[Nícolas F. R. A. Prado]

The value read from the "hpd-absent-delay-ms" property in DT was being
saved to the wrong variable, overriding the hpd_reliable delay. Fix the
typo.

Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by EDID")
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---

 drivers/gpu/drm/panel/panel-edp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index 152e00eb846f..b3536d8600f4 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -738,7 +738,7 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
 	of_property_read_u32(dev->of_node, "hpd-reliable-delay-ms", &reliable_ms);
 	desc->delay.hpd_reliable = reliable_ms;
 	of_property_read_u32(dev->of_node, "hpd-absent-delay-ms", &absent_ms);
-	desc->delay.hpd_reliable = absent_ms;
+	desc->delay.hpd_absent = absent_ms;
 
 	/* Power the panel on so we can read the EDID */
 	ret = pm_runtime_get_sync(dev);
-- 
2.37.0

Re: [PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT

[André Almeida]

Às 17:38 de 19/07/22, Nícolas F. R. A. Prado escreveu:
> The value read from the "hpd-absent-delay-ms" property in DT was being
> saved to the wrong variable, overriding the hpd_reliable delay. Fix the
> typo.
> 
> Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by EDID")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Reviewed-by: André Almeida <andrealmeid@igalia.com>

Re: [PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT

[André Almeida]

Às 17:38 de 19/07/22, Nícolas F. R. A. Prado escreveu:
> The value read from the "hpd-absent-delay-ms" property in DT was being
> saved to the wrong variable, overriding the hpd_reliable delay. Fix the
> typo.
> 
> Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by EDID")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Reviewed-by: André Almeida <andrealmeid@igalia.com>

Re: [PATCH 1/3] drm/panel-edp: Add panel entry for R140NWF5 RH

[Doug Anderson]

Hi,

On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
<nfraprado@collabora.com> wrote:
>
> Add panel identification entry for the IVO R140NWF5 RH (product ID:
> 0x057d) panel.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>
> ---
> The comments on the driver indicate that the T3 timing should be set on
> hpd_absent, while hpd_reliable would have a shorter time just while the
> HPD line stabilizes on low after power is supplied.

Right. Ideally hpd_reliable is 0 unless you've got a badly-designed panel.


> But can we really assume that the HPD line will be reliable at all
> before the DDIC is done booting up, at which point the HPD line is
> brought up? IOW, shouldn't we always delay T3 (by setting hpd_reliable =
> T3), since only then we're really sure that the DDIC is done setting up
> and the HPD line is reliable?

If the panel is hooked up properly, then the HPD pin should be pulled
low at the start and then should only go high after the panel is ready
for us to talk to it, right? So it's not like the DDIC has to boot up
and actively init the state. I would assume that the initial state of
the "HPD output" from the panel's IC would be one of the following:
* A floating input.
* A pulled down input.
* An output driven low.

In any of those cases just adding a pull down on the line would be
enough to ensure that the HPD line is reliable until the panel comes
around and actively drives the line high.

Remember, this is eDP and it's not something that's hot-plugged, so
there's no debouncing involved and in a properly designed system there
should be no time needed for the signal to stabilize. I would also
point out that on the oficial eDP docs the eDP timing diagram doesn't
show the initial state of "HPD" as "unknown". It shows it as low.

Now, that all being said, I have seen at least one panel that glitched
itself at bootup. After you powered it on it would blip its HPD line
high before it had actually finished booting. Then the HPD would go
low again before finally going high after the panel finished booting.
This is the reason for "hpd_reliable".

If you've got a board with a well-designed panel but the hookup
between the panel and the board is wrong (maybe the board is missing a
pulldown on the HPD line?) then you can just set the "no-hpd" property
for your board. That will tell the kernel to just always delay the
"hpd-absent" delay.

> I've set the T3 delay to hpd_absent in this series, following what's
> instructed in the comments, but I'd like to discuss whether we shouldn't
> be setting T3 on hpd_reliable instead, for all panels, to be on the
> safer side.

The way it's specified right now is more flexible, though, isn't it?
This way if you're on a board where the HPD truly _isn't_ stable then
you can just set the "no-hpd" and it will automatically use the
"hpd_absent" delay, right?


>  drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> index 3626469c4cc2..675d793d925e 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -1854,6 +1854,12 @@ static const struct panel_delay delay_100_500_e200 = {
>         .enable = 200,
>  };
>
> +static const struct panel_delay delay_200_500_e200 = {
> +       .hpd_absent = 200,
> +       .unprepare = 500,
> +       .enable = 200,
> +};
> +
>  #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
>  { \
>         .name = _name, \
> @@ -1882,6 +1888,8 @@ static const struct edp_panel_entry edp_panels[] = {
>
>         EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay, "N116BCA-EA1"),
>
> +       EDP_PANEL_ENTRY('I', 'V', 'O', 0x057d, &delay_200_500_e200, "R140NWF5 RH"),
> +

This looks fine to me:

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

I'm happy to apply this in a day or two assuming you're OK with my
explanation above.

Re: [PATCH 1/3] drm/panel-edp: Add panel entry for R140NWF5 RH

[Doug Anderson]

Hi,

On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
<nfraprado@collabora.com> wrote:
>
> Add panel identification entry for the IVO R140NWF5 RH (product ID:
> 0x057d) panel.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>
> ---
> The comments on the driver indicate that the T3 timing should be set on
> hpd_absent, while hpd_reliable would have a shorter time just while the
> HPD line stabilizes on low after power is supplied.

Right. Ideally hpd_reliable is 0 unless you've got a badly-designed panel.


> But can we really assume that the HPD line will be reliable at all
> before the DDIC is done booting up, at which point the HPD line is
> brought up? IOW, shouldn't we always delay T3 (by setting hpd_reliable =
> T3), since only then we're really sure that the DDIC is done setting up
> and the HPD line is reliable?

If the panel is hooked up properly, then the HPD pin should be pulled
low at the start and then should only go high after the panel is ready
for us to talk to it, right? So it's not like the DDIC has to boot up
and actively init the state. I would assume that the initial state of
the "HPD output" from the panel's IC would be one of the following:
* A floating input.
* A pulled down input.
* An output driven low.

In any of those cases just adding a pull down on the line would be
enough to ensure that the HPD line is reliable until the panel comes
around and actively drives the line high.

Remember, this is eDP and it's not something that's hot-plugged, so
there's no debouncing involved and in a properly designed system there
should be no time needed for the signal to stabilize. I would also
point out that on the oficial eDP docs the eDP timing diagram doesn't
show the initial state of "HPD" as "unknown". It shows it as low.

Now, that all being said, I have seen at least one panel that glitched
itself at bootup. After you powered it on it would blip its HPD line
high before it had actually finished booting. Then the HPD would go
low again before finally going high after the panel finished booting.
This is the reason for "hpd_reliable".

If you've got a board with a well-designed panel but the hookup
between the panel and the board is wrong (maybe the board is missing a
pulldown on the HPD line?) then you can just set the "no-hpd" property
for your board. That will tell the kernel to just always delay the
"hpd-absent" delay.

> I've set the T3 delay to hpd_absent in this series, following what's
> instructed in the comments, but I'd like to discuss whether we shouldn't
> be setting T3 on hpd_reliable instead, for all panels, to be on the
> safer side.

The way it's specified right now is more flexible, though, isn't it?
This way if you're on a board where the HPD truly _isn't_ stable then
you can just set the "no-hpd" and it will automatically use the
"hpd_absent" delay, right?


>  drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> index 3626469c4cc2..675d793d925e 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -1854,6 +1854,12 @@ static const struct panel_delay delay_100_500_e200 = {
>         .enable = 200,
>  };
>
> +static const struct panel_delay delay_200_500_e200 = {
> +       .hpd_absent = 200,
> +       .unprepare = 500,
> +       .enable = 200,
> +};
> +
>  #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
>  { \
>         .name = _name, \
> @@ -1882,6 +1888,8 @@ static const struct edp_panel_entry edp_panels[] = {
>
>         EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay, "N116BCA-EA1"),
>
> +       EDP_PANEL_ENTRY('I', 'V', 'O', 0x057d, &delay_200_500_e200, "R140NWF5 RH"),
> +

This looks fine to me:

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

I'm happy to apply this in a day or two assuming you're OK with my
explanation above.

Re: [PATCH 2/3] drm/panel-edp: Add panel entry for B120XAN01.0

[Doug Anderson]

Hi,

On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
<nfraprado@collabora.com> wrote:
>
> Add panel identification entry for the AUO B120XAN01.0 (product ID:
> 0x1062) panel.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>
>  drivers/gpu/drm/panel/panel-edp.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> index 675d793d925e..152e00eb846f 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -1879,6 +1879,7 @@ static const struct edp_panel_entry edp_panels[] = {
>         EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01"),
>         EDP_PANEL_ENTRY('A', 'U', 'O', 0x615c, &delay_200_500_e50, "B116XAN06.1"),
>         EDP_PANEL_ENTRY('A', 'U', 'O', 0x8594, &delay_200_500_e50, "B133UAN01.0"),
> +       EDP_PANEL_ENTRY('A', 'U', 'O', 0x1062, &delay_200_500_e50, "B120XAN01.0"),

 * Sort first by vendor, then by product ID.

0x1062 sorts at the top of the AUO panels, not at the bottom.

-Doug

Re: [PATCH 2/3] drm/panel-edp: Add panel entry for B120XAN01.0

[Doug Anderson]

Hi,

On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
<nfraprado@collabora.com> wrote:
>
> Add panel identification entry for the AUO B120XAN01.0 (product ID:
> 0x1062) panel.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>
>  drivers/gpu/drm/panel/panel-edp.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> index 675d793d925e..152e00eb846f 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -1879,6 +1879,7 @@ static const struct edp_panel_entry edp_panels[] = {
>         EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01"),
>         EDP_PANEL_ENTRY('A', 'U', 'O', 0x615c, &delay_200_500_e50, "B116XAN06.1"),
>         EDP_PANEL_ENTRY('A', 'U', 'O', 0x8594, &delay_200_500_e50, "B133UAN01.0"),
> +       EDP_PANEL_ENTRY('A', 'U', 'O', 0x1062, &delay_200_500_e50, "B120XAN01.0"),

 * Sort first by vendor, then by product ID.

0x1062 sorts at the top of the AUO panels, not at the bottom.

-Doug

Re: [PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT

[Doug Anderson]

Hi,

On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
<nfraprado@collabora.com> wrote:
>
> The value read from the "hpd-absent-delay-ms" property in DT was being
> saved to the wrong variable, overriding the hpd_reliable delay. Fix the
> typo.
>
> Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by EDID")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>
>  drivers/gpu/drm/panel/panel-edp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> index 152e00eb846f..b3536d8600f4 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -738,7 +738,7 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
>         of_property_read_u32(dev->of_node, "hpd-reliable-delay-ms", &reliable_ms);
>         desc->delay.hpd_reliable = reliable_ms;
>         of_property_read_u32(dev->of_node, "hpd-absent-delay-ms", &absent_ms);
> -       desc->delay.hpd_reliable = absent_ms;
> +       desc->delay.hpd_absent = absent_ms;

Well that's embarrassing. In the end I never used any of these
properties for anything shipping since HPD was always hooked up on
later boards and the only board that needed "hpd_reliable" never ended
up switching to the generic "edp-panel".

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

I'll apply this right away to drm-misc-fixes.

Re: [PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT

[Doug Anderson]

Hi,

On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
<nfraprado@collabora.com> wrote:
>
> The value read from the "hpd-absent-delay-ms" property in DT was being
> saved to the wrong variable, overriding the hpd_reliable delay. Fix the
> typo.
>
> Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by EDID")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>
>  drivers/gpu/drm/panel/panel-edp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> index 152e00eb846f..b3536d8600f4 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -738,7 +738,7 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
>         of_property_read_u32(dev->of_node, "hpd-reliable-delay-ms", &reliable_ms);
>         desc->delay.hpd_reliable = reliable_ms;
>         of_property_read_u32(dev->of_node, "hpd-absent-delay-ms", &absent_ms);
> -       desc->delay.hpd_reliable = absent_ms;
> +       desc->delay.hpd_absent = absent_ms;

Well that's embarrassing. In the end I never used any of these
properties for anything shipping since HPD was always hooked up on
later boards and the only board that needed "hpd_reliable" never ended
up switching to the generic "edp-panel".

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

I'll apply this right away to drm-misc-fixes.

Re: [PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT

[Doug Anderson]

Hi,

On Tue, Jul 19, 2022 at 3:45 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
> <nfraprado@collabora.com> wrote:
> >
> > The value read from the "hpd-absent-delay-ms" property in DT was being
> > saved to the wrong variable, overriding the hpd_reliable delay. Fix the
> > typo.
> >
> > Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by EDID")
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---
> >
> >  drivers/gpu/drm/panel/panel-edp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > index 152e00eb846f..b3536d8600f4 100644
> > --- a/drivers/gpu/drm/panel/panel-edp.c
> > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > @@ -738,7 +738,7 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
> >         of_property_read_u32(dev->of_node, "hpd-reliable-delay-ms", &reliable_ms);
> >         desc->delay.hpd_reliable = reliable_ms;
> >         of_property_read_u32(dev->of_node, "hpd-absent-delay-ms", &absent_ms);
> > -       desc->delay.hpd_reliable = absent_ms;
> > +       desc->delay.hpd_absent = absent_ms;
>
> Well that's embarrassing. In the end I never used any of these
> properties for anything shipping since HPD was always hooked up on
> later boards and the only board that needed "hpd_reliable" never ended
> up switching to the generic "edp-panel".
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> I'll apply this right away to drm-misc-fixes.

ef2084a8388b drm/panel-edp: Fix variable typo when saving hpd absent
delay from DT

Re: [PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT

[Doug Anderson]

Hi,

On Tue, Jul 19, 2022 at 3:45 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
> <nfraprado@collabora.com> wrote:
> >
> > The value read from the "hpd-absent-delay-ms" property in DT was being
> > saved to the wrong variable, overriding the hpd_reliable delay. Fix the
> > typo.
> >
> > Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by EDID")
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---
> >
> >  drivers/gpu/drm/panel/panel-edp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > index 152e00eb846f..b3536d8600f4 100644
> > --- a/drivers/gpu/drm/panel/panel-edp.c
> > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > @@ -738,7 +738,7 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
> >         of_property_read_u32(dev->of_node, "hpd-reliable-delay-ms", &reliable_ms);
> >         desc->delay.hpd_reliable = reliable_ms;
> >         of_property_read_u32(dev->of_node, "hpd-absent-delay-ms", &absent_ms);
> > -       desc->delay.hpd_reliable = absent_ms;
> > +       desc->delay.hpd_absent = absent_ms;
>
> Well that's embarrassing. In the end I never used any of these
> properties for anything shipping since HPD was always hooked up on
> later boards and the only board that needed "hpd_reliable" never ended
> up switching to the generic "edp-panel".
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> I'll apply this right away to drm-misc-fixes.

ef2084a8388b drm/panel-edp: Fix variable typo when saving hpd absent
delay from DT

Re: [PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT

[AngeloGioacchino Del Regno]

Il 19/07/22 22:38, Nícolas F. R. A. Prado ha scritto:
> The value read from the "hpd-absent-delay-ms" property in DT was being
> saved to the wrong variable, overriding the hpd_reliable delay. Fix the
> typo.
> 
> Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by EDID")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

I think you should've sent this separately from the rest of the series, especially
because of the questions etc.

In any case, for this commit:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 3/3] drm/panel-edp: Fix variable typo when saving hpd absent delay from DT

[AngeloGioacchino Del Regno]

Il 19/07/22 22:38, Nícolas F. R. A. Prado ha scritto:
> The value read from the "hpd-absent-delay-ms" property in DT was being
> saved to the wrong variable, overriding the hpd_reliable delay. Fix the
> typo.
> 
> Fixes: 5540cf8f3e8d ("drm/panel-edp: Implement generic "edp-panel"s probed by EDID")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

I think you should've sent this separately from the rest of the series, especially
because of the questions etc.

In any case, for this commit:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 1/3] drm/panel-edp: Add panel entry for R140NWF5 RH

[AngeloGioacchino Del Regno]

Il 20/07/22 00:40, Doug Anderson ha scritto:
> Hi,
> 
> On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
> <nfraprado@collabora.com> wrote:
>>
>> Add panel identification entry for the IVO R140NWF5 RH (product ID:
>> 0x057d) panel.
>>
>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>>
>> ---
>> The comments on the driver indicate that the T3 timing should be set on
>> hpd_absent, while hpd_reliable would have a shorter time just while the
>> HPD line stabilizes on low after power is supplied.
> 
> Right. Ideally hpd_reliable is 0 unless you've got a badly-designed panel.
> 
> 
>> But can we really assume that the HPD line will be reliable at all
>> before the DDIC is done booting up, at which point the HPD line is
>> brought up? IOW, shouldn't we always delay T3 (by setting hpd_reliable =
>> T3), since only then we're really sure that the DDIC is done setting up
>> and the HPD line is reliable?
> 
> If the panel is hooked up properly, then the HPD pin should be pulled
> low at the start and then should only go high after the panel is ready
> for us to talk to it, right? So it's not like the DDIC has to boot up
> and actively init the state. I would assume that the initial state of
> the "HPD output" from the panel's IC would be one of the following:
> * A floating input.
> * A pulled down input.
> * An output driven low.
> 
> In any of those cases just adding a pull down on the line would be
> enough to ensure that the HPD line is reliable until the panel comes
> around and actively drives the line high.
> 
> Remember, this is eDP and it's not something that's hot-plugged, so
> there's no debouncing involved and in a properly designed system there
> should be no time needed for the signal to stabilize. I would also
> point out that on the oficial eDP docs the eDP timing diagram doesn't
> show the initial state of "HPD" as "unknown". It shows it as low.
> 
> Now, that all being said, I have seen at least one panel that glitched
> itself at bootup. After you powered it on it would blip its HPD line
> high before it had actually finished booting. Then the HPD would go
> low again before finally going high after the panel finished booting.
> This is the reason for "hpd_reliable".
> 
> If you've got a board with a well-designed panel but the hookup
> between the panel and the board is wrong (maybe the board is missing a
> pulldown on the HPD line?) then you can just set the "no-hpd" property
> for your board. That will tell the kernel to just always delay the
> "hpd-absent" delay.
> 

We were concerned exactly about glitchy HPD during DDIC init, as I didn't
want to trust it because the only testing we could do was on just two units...

...but if you're sure that I was too much paranoid about that, that's good,
as it means I will be a bit more "relaxed" on this topic next time :-)

>> I've set the T3 delay to hpd_absent in this series, following what's
>> instructed in the comments, but I'd like to discuss whether we shouldn't
>> be setting T3 on hpd_reliable instead, for all panels, to be on the
>> safer side.
> 
> The way it's specified right now is more flexible, though, isn't it?
> This way if you're on a board where the HPD truly _isn't_ stable then
> you can just set the "no-hpd" and it will automatically use the
> "hpd_absent" delay, right?
> 

For Chromebooks, that's totally doable, thanks to the bootloader seeking for
proper machine compatibles, so yes I agree with that.

> 
>>   drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
>> index 3626469c4cc2..675d793d925e 100644
>> --- a/drivers/gpu/drm/panel/panel-edp.c
>> +++ b/drivers/gpu/drm/panel/panel-edp.c
>> @@ -1854,6 +1854,12 @@ static const struct panel_delay delay_100_500_e200 = {
>>          .enable = 200,
>>   };
>>
>> +static const struct panel_delay delay_200_500_e200 = {
>> +       .hpd_absent = 200,
>> +       .unprepare = 500,
>> +       .enable = 200,
>> +};
>> +
>>   #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
>>   { \
>>          .name = _name, \
>> @@ -1882,6 +1888,8 @@ static const struct edp_panel_entry edp_panels[] = {
>>
>>          EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay, "N116BCA-EA1"),
>>
>> +       EDP_PANEL_ENTRY('I', 'V', 'O', 0x057d, &delay_200_500_e200, "R140NWF5 RH"),
>> +
> 
> This looks fine to me:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> 
> I'm happy to apply this in a day or two assuming you're OK with my
> explanation above.

Thank you for the long mail, your explanation was truly helpful!
(especially for me being paranoid :-P)

So, I agree to go with that one, for which:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Nic, green light?


Cheers, (and thanks again!)
Angelo

Re: [PATCH 1/3] drm/panel-edp: Add panel entry for R140NWF5 RH

[AngeloGioacchino Del Regno]

Il 20/07/22 00:40, Doug Anderson ha scritto:
> Hi,
> 
> On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
> <nfraprado@collabora.com> wrote:
>>
>> Add panel identification entry for the IVO R140NWF5 RH (product ID:
>> 0x057d) panel.
>>
>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>>
>> ---
>> The comments on the driver indicate that the T3 timing should be set on
>> hpd_absent, while hpd_reliable would have a shorter time just while the
>> HPD line stabilizes on low after power is supplied.
> 
> Right. Ideally hpd_reliable is 0 unless you've got a badly-designed panel.
> 
> 
>> But can we really assume that the HPD line will be reliable at all
>> before the DDIC is done booting up, at which point the HPD line is
>> brought up? IOW, shouldn't we always delay T3 (by setting hpd_reliable =
>> T3), since only then we're really sure that the DDIC is done setting up
>> and the HPD line is reliable?
> 
> If the panel is hooked up properly, then the HPD pin should be pulled
> low at the start and then should only go high after the panel is ready
> for us to talk to it, right? So it's not like the DDIC has to boot up
> and actively init the state. I would assume that the initial state of
> the "HPD output" from the panel's IC would be one of the following:
> * A floating input.
> * A pulled down input.
> * An output driven low.
> 
> In any of those cases just adding a pull down on the line would be
> enough to ensure that the HPD line is reliable until the panel comes
> around and actively drives the line high.
> 
> Remember, this is eDP and it's not something that's hot-plugged, so
> there's no debouncing involved and in a properly designed system there
> should be no time needed for the signal to stabilize. I would also
> point out that on the oficial eDP docs the eDP timing diagram doesn't
> show the initial state of "HPD" as "unknown". It shows it as low.
> 
> Now, that all being said, I have seen at least one panel that glitched
> itself at bootup. After you powered it on it would blip its HPD line
> high before it had actually finished booting. Then the HPD would go
> low again before finally going high after the panel finished booting.
> This is the reason for "hpd_reliable".
> 
> If you've got a board with a well-designed panel but the hookup
> between the panel and the board is wrong (maybe the board is missing a
> pulldown on the HPD line?) then you can just set the "no-hpd" property
> for your board. That will tell the kernel to just always delay the
> "hpd-absent" delay.
> 

We were concerned exactly about glitchy HPD during DDIC init, as I didn't
want to trust it because the only testing we could do was on just two units...

...but if you're sure that I was too much paranoid about that, that's good,
as it means I will be a bit more "relaxed" on this topic next time :-)

>> I've set the T3 delay to hpd_absent in this series, following what's
>> instructed in the comments, but I'd like to discuss whether we shouldn't
>> be setting T3 on hpd_reliable instead, for all panels, to be on the
>> safer side.
> 
> The way it's specified right now is more flexible, though, isn't it?
> This way if you're on a board where the HPD truly _isn't_ stable then
> you can just set the "no-hpd" and it will automatically use the
> "hpd_absent" delay, right?
> 

For Chromebooks, that's totally doable, thanks to the bootloader seeking for
proper machine compatibles, so yes I agree with that.

> 
>>   drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
>> index 3626469c4cc2..675d793d925e 100644
>> --- a/drivers/gpu/drm/panel/panel-edp.c
>> +++ b/drivers/gpu/drm/panel/panel-edp.c
>> @@ -1854,6 +1854,12 @@ static const struct panel_delay delay_100_500_e200 = {
>>          .enable = 200,
>>   };
>>
>> +static const struct panel_delay delay_200_500_e200 = {
>> +       .hpd_absent = 200,
>> +       .unprepare = 500,
>> +       .enable = 200,
>> +};
>> +
>>   #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
>>   { \
>>          .name = _name, \
>> @@ -1882,6 +1888,8 @@ static const struct edp_panel_entry edp_panels[] = {
>>
>>          EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay, "N116BCA-EA1"),
>>
>> +       EDP_PANEL_ENTRY('I', 'V', 'O', 0x057d, &delay_200_500_e200, "R140NWF5 RH"),
>> +
> 
> This looks fine to me:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> 
> I'm happy to apply this in a day or two assuming you're OK with my
> explanation above.

Thank you for the long mail, your explanation was truly helpful!
(especially for me being paranoid :-P)

So, I agree to go with that one, for which:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Nic, green light?


Cheers, (and thanks again!)
Angelo

Re: [PATCH 2/3] drm/panel-edp: Add panel entry for B120XAN01.0

[Nícolas F. R. A. Prado]

On Tue, Jul 19, 2022 at 03:41:43PM -0700, Doug Anderson wrote:
> Hi,
> 
> On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
> <nfraprado@collabora.com> wrote:
> >
> > Add panel identification entry for the AUO B120XAN01.0 (product ID:
> > 0x1062) panel.
> >
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---
> >
> >  drivers/gpu/drm/panel/panel-edp.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > index 675d793d925e..152e00eb846f 100644
> > --- a/drivers/gpu/drm/panel/panel-edp.c
> > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > @@ -1879,6 +1879,7 @@ static const struct edp_panel_entry edp_panels[] = {
> >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01"),
> >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x615c, &delay_200_500_e50, "B116XAN06.1"),
> >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x8594, &delay_200_500_e50, "B133UAN01.0"),
> > +       EDP_PANEL_ENTRY('A', 'U', 'O', 0x1062, &delay_200_500_e50, "B120XAN01.0"),
> 
>  * Sort first by vendor, then by product ID.
> 
> 0x1062 sorts at the top of the AUO panels, not at the bottom.

Right. Will fix on v2.

Thanks,
Nícolas

Re: [PATCH 2/3] drm/panel-edp: Add panel entry for B120XAN01.0

[Nícolas F. R. A. Prado]

On Tue, Jul 19, 2022 at 03:41:43PM -0700, Doug Anderson wrote:
> Hi,
> 
> On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
> <nfraprado@collabora.com> wrote:
> >
> > Add panel identification entry for the AUO B120XAN01.0 (product ID:
> > 0x1062) panel.
> >
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---
> >
> >  drivers/gpu/drm/panel/panel-edp.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > index 675d793d925e..152e00eb846f 100644
> > --- a/drivers/gpu/drm/panel/panel-edp.c
> > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > @@ -1879,6 +1879,7 @@ static const struct edp_panel_entry edp_panels[] = {
> >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01"),
> >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x615c, &delay_200_500_e50, "B116XAN06.1"),
> >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x8594, &delay_200_500_e50, "B133UAN01.0"),
> > +       EDP_PANEL_ENTRY('A', 'U', 'O', 0x1062, &delay_200_500_e50, "B120XAN01.0"),
> 
>  * Sort first by vendor, then by product ID.
> 
> 0x1062 sorts at the top of the AUO panels, not at the bottom.

Right. Will fix on v2.

Thanks,
Nícolas

Re: [PATCH 1/3] drm/panel-edp: Add panel entry for R140NWF5 RH

[Nícolas F. R. A. Prado]

On Wed, Jul 20, 2022 at 09:49:35AM +0200, AngeloGioacchino Del Regno wrote:
> Il 20/07/22 00:40, Doug Anderson ha scritto:
> > Hi,
> > 
> > On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
> > <nfraprado@collabora.com> wrote:
> > > 
> > > Add panel identification entry for the IVO R140NWF5 RH (product ID:
> > > 0x057d) panel.
> > > 
> > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > > 
> > > ---
> > > The comments on the driver indicate that the T3 timing should be set on
> > > hpd_absent, while hpd_reliable would have a shorter time just while the
> > > HPD line stabilizes on low after power is supplied.
> > 
> > Right. Ideally hpd_reliable is 0 unless you've got a badly-designed panel.
> > 
> > 
> > > But can we really assume that the HPD line will be reliable at all
> > > before the DDIC is done booting up, at which point the HPD line is
> > > brought up? IOW, shouldn't we always delay T3 (by setting hpd_reliable =
> > > T3), since only then we're really sure that the DDIC is done setting up
> > > and the HPD line is reliable?
> > 
> > If the panel is hooked up properly, then the HPD pin should be pulled
> > low at the start and then should only go high after the panel is ready
> > for us to talk to it, right? So it's not like the DDIC has to boot up
> > and actively init the state. I would assume that the initial state of
> > the "HPD output" from the panel's IC would be one of the following:
> > * A floating input.
> > * A pulled down input.
> > * An output driven low.
> > 
> > In any of those cases just adding a pull down on the line would be
> > enough to ensure that the HPD line is reliable until the panel comes
> > around and actively drives the line high.
> > 
> > Remember, this is eDP and it's not something that's hot-plugged, so
> > there's no debouncing involved and in a properly designed system there
> > should be no time needed for the signal to stabilize. I would also
> > point out that on the oficial eDP docs the eDP timing diagram doesn't
> > show the initial state of "HPD" as "unknown". It shows it as low.
> > 
> > Now, that all being said, I have seen at least one panel that glitched
> > itself at bootup. After you powered it on it would blip its HPD line
> > high before it had actually finished booting. Then the HPD would go
> > low again before finally going high after the panel finished booting.
> > This is the reason for "hpd_reliable".
> > 
> > If you've got a board with a well-designed panel but the hookup
> > between the panel and the board is wrong (maybe the board is missing a
> > pulldown on the HPD line?) then you can just set the "no-hpd" property
> > for your board. That will tell the kernel to just always delay the
> > "hpd-absent" delay.
> > 

Thank you for the detailed explanation, this does clear all doubts from what me
and Angelo were discussing.

> 
> We were concerned exactly about glitchy HPD during DDIC init, as I didn't
> want to trust it because the only testing we could do was on just two units...
> 
> ...but if you're sure that I was too much paranoid about that, that's good,
> as it means I will be a bit more "relaxed" on this topic next time :-)
> 
> > > I've set the T3 delay to hpd_absent in this series, following what's
> > > instructed in the comments, but I'd like to discuss whether we shouldn't
> > > be setting T3 on hpd_reliable instead, for all panels, to be on the
> > > safer side.
> > 
> > The way it's specified right now is more flexible, though, isn't it?
> > This way if you're on a board where the HPD truly _isn't_ stable then
> > you can just set the "no-hpd" and it will automatically use the
> > "hpd_absent" delay, right?
> > 

Yes, indeed. I just wasn't sure that flexibility brought us anything, but after
your explanation above it makes much more sense now, thanks!

> 
> For Chromebooks, that's totally doable, thanks to the bootloader seeking for
> proper machine compatibles, so yes I agree with that.
> 
> > 
> > >   drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
> > >   1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > > index 3626469c4cc2..675d793d925e 100644
> > > --- a/drivers/gpu/drm/panel/panel-edp.c
> > > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > > @@ -1854,6 +1854,12 @@ static const struct panel_delay delay_100_500_e200 = {
> > >          .enable = 200,
> > >   };
> > > 
> > > +static const struct panel_delay delay_200_500_e200 = {
> > > +       .hpd_absent = 200,
> > > +       .unprepare = 500,
> > > +       .enable = 200,
> > > +};
> > > +
> > >   #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
> > >   { \
> > >          .name = _name, \
> > > @@ -1882,6 +1888,8 @@ static const struct edp_panel_entry edp_panels[] = {
> > > 
> > >          EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay, "N116BCA-EA1"),
> > > 
> > > +       EDP_PANEL_ENTRY('I', 'V', 'O', 0x057d, &delay_200_500_e200, "R140NWF5 RH"),
> > > +
> > 
> > This looks fine to me:
> > 
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > 
> > I'm happy to apply this in a day or two assuming you're OK with my
> > explanation above.
> 
> Thank you for the long mail, your explanation was truly helpful!
> (especially for me being paranoid :-P)
> 
> So, I agree to go with that one, for which:
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> Nic, green light?

Yep.

I haven't seen any issues with keeping the hpd_reliable as 0 in the machine I
have access to, and Douglas' explanation cleared up all the doubts of how this
all works, so, Douglas, please feel free to merge this as is.

In that case, since patch 3 was also merged already I'll send a v2 just for
patch 2 separately.

Thanks,
Nícolas

Re: [PATCH 1/3] drm/panel-edp: Add panel entry for R140NWF5 RH

[Nícolas F. R. A. Prado]

On Wed, Jul 20, 2022 at 09:49:35AM +0200, AngeloGioacchino Del Regno wrote:
> Il 20/07/22 00:40, Doug Anderson ha scritto:
> > Hi,
> > 
> > On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
> > <nfraprado@collabora.com> wrote:
> > > 
> > > Add panel identification entry for the IVO R140NWF5 RH (product ID:
> > > 0x057d) panel.
> > > 
> > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > > 
> > > ---
> > > The comments on the driver indicate that the T3 timing should be set on
> > > hpd_absent, while hpd_reliable would have a shorter time just while the
> > > HPD line stabilizes on low after power is supplied.
> > 
> > Right. Ideally hpd_reliable is 0 unless you've got a badly-designed panel.
> > 
> > 
> > > But can we really assume that the HPD line will be reliable at all
> > > before the DDIC is done booting up, at which point the HPD line is
> > > brought up? IOW, shouldn't we always delay T3 (by setting hpd_reliable =
> > > T3), since only then we're really sure that the DDIC is done setting up
> > > and the HPD line is reliable?
> > 
> > If the panel is hooked up properly, then the HPD pin should be pulled
> > low at the start and then should only go high after the panel is ready
> > for us to talk to it, right? So it's not like the DDIC has to boot up
> > and actively init the state. I would assume that the initial state of
> > the "HPD output" from the panel's IC would be one of the following:
> > * A floating input.
> > * A pulled down input.
> > * An output driven low.
> > 
> > In any of those cases just adding a pull down on the line would be
> > enough to ensure that the HPD line is reliable until the panel comes
> > around and actively drives the line high.
> > 
> > Remember, this is eDP and it's not something that's hot-plugged, so
> > there's no debouncing involved and in a properly designed system there
> > should be no time needed for the signal to stabilize. I would also
> > point out that on the oficial eDP docs the eDP timing diagram doesn't
> > show the initial state of "HPD" as "unknown". It shows it as low.
> > 
> > Now, that all being said, I have seen at least one panel that glitched
> > itself at bootup. After you powered it on it would blip its HPD line
> > high before it had actually finished booting. Then the HPD would go
> > low again before finally going high after the panel finished booting.
> > This is the reason for "hpd_reliable".
> > 
> > If you've got a board with a well-designed panel but the hookup
> > between the panel and the board is wrong (maybe the board is missing a
> > pulldown on the HPD line?) then you can just set the "no-hpd" property
> > for your board. That will tell the kernel to just always delay the
> > "hpd-absent" delay.
> > 

Thank you for the detailed explanation, this does clear all doubts from what me
and Angelo were discussing.

> 
> We were concerned exactly about glitchy HPD during DDIC init, as I didn't
> want to trust it because the only testing we could do was on just two units...
> 
> ...but if you're sure that I was too much paranoid about that, that's good,
> as it means I will be a bit more "relaxed" on this topic next time :-)
> 
> > > I've set the T3 delay to hpd_absent in this series, following what's
> > > instructed in the comments, but I'd like to discuss whether we shouldn't
> > > be setting T3 on hpd_reliable instead, for all panels, to be on the
> > > safer side.
> > 
> > The way it's specified right now is more flexible, though, isn't it?
> > This way if you're on a board where the HPD truly _isn't_ stable then
> > you can just set the "no-hpd" and it will automatically use the
> > "hpd_absent" delay, right?
> > 

Yes, indeed. I just wasn't sure that flexibility brought us anything, but after
your explanation above it makes much more sense now, thanks!

> 
> For Chromebooks, that's totally doable, thanks to the bootloader seeking for
> proper machine compatibles, so yes I agree with that.
> 
> > 
> > >   drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
> > >   1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > > index 3626469c4cc2..675d793d925e 100644
> > > --- a/drivers/gpu/drm/panel/panel-edp.c
> > > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > > @@ -1854,6 +1854,12 @@ static const struct panel_delay delay_100_500_e200 = {
> > >          .enable = 200,
> > >   };
> > > 
> > > +static const struct panel_delay delay_200_500_e200 = {
> > > +       .hpd_absent = 200,
> > > +       .unprepare = 500,
> > > +       .enable = 200,
> > > +};
> > > +
> > >   #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
> > >   { \
> > >          .name = _name, \
> > > @@ -1882,6 +1888,8 @@ static const struct edp_panel_entry edp_panels[] = {
> > > 
> > >          EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay, "N116BCA-EA1"),
> > > 
> > > +       EDP_PANEL_ENTRY('I', 'V', 'O', 0x057d, &delay_200_500_e200, "R140NWF5 RH"),
> > > +
> > 
> > This looks fine to me:
> > 
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > 
> > I'm happy to apply this in a day or two assuming you're OK with my
> > explanation above.
> 
> Thank you for the long mail, your explanation was truly helpful!
> (especially for me being paranoid :-P)
> 
> So, I agree to go with that one, for which:
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> Nic, green light?

Yep.

I haven't seen any issues with keeping the hpd_reliable as 0 in the machine I
have access to, and Douglas' explanation cleared up all the doubts of how this
all works, so, Douglas, please feel free to merge this as is.

In that case, since patch 3 was also merged already I'll send a v2 just for
patch 2 separately.

Thanks,
Nícolas

Re: [PATCH 1/3] drm/panel-edp: Add panel entry for R140NWF5 RH

[Doug Anderson]

Hi,

On Wed, Jul 20, 2022 at 11:52 AM Nícolas F. R. A. Prado
<nfraprado@collabora.com> wrote:
>
> On Wed, Jul 20, 2022 at 09:49:35AM +0200, AngeloGioacchino Del Regno wrote:
> > Il 20/07/22 00:40, Doug Anderson ha scritto:
> > > Hi,
> > >
> > > On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
> > > <nfraprado@collabora.com> wrote:
> > > >
> > > > Add panel identification entry for the IVO R140NWF5 RH (product ID:
> > > > 0x057d) panel.
> > > >
> > > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > > >
> > > > ---
> > > > The comments on the driver indicate that the T3 timing should be set on
> > > > hpd_absent, while hpd_reliable would have a shorter time just while the
> > > > HPD line stabilizes on low after power is supplied.
> > >
> > > Right. Ideally hpd_reliable is 0 unless you've got a badly-designed panel.
> > >
> > >
> > > > But can we really assume that the HPD line will be reliable at all
> > > > before the DDIC is done booting up, at which point the HPD line is
> > > > brought up? IOW, shouldn't we always delay T3 (by setting hpd_reliable =
> > > > T3), since only then we're really sure that the DDIC is done setting up
> > > > and the HPD line is reliable?
> > >
> > > If the panel is hooked up properly, then the HPD pin should be pulled
> > > low at the start and then should only go high after the panel is ready
> > > for us to talk to it, right? So it's not like the DDIC has to boot up
> > > and actively init the state. I would assume that the initial state of
> > > the "HPD output" from the panel's IC would be one of the following:
> > > * A floating input.
> > > * A pulled down input.
> > > * An output driven low.
> > >
> > > In any of those cases just adding a pull down on the line would be
> > > enough to ensure that the HPD line is reliable until the panel comes
> > > around and actively drives the line high.
> > >
> > > Remember, this is eDP and it's not something that's hot-plugged, so
> > > there's no debouncing involved and in a properly designed system there
> > > should be no time needed for the signal to stabilize. I would also
> > > point out that on the oficial eDP docs the eDP timing diagram doesn't
> > > show the initial state of "HPD" as "unknown". It shows it as low.
> > >
> > > Now, that all being said, I have seen at least one panel that glitched
> > > itself at bootup. After you powered it on it would blip its HPD line
> > > high before it had actually finished booting. Then the HPD would go
> > > low again before finally going high after the panel finished booting.
> > > This is the reason for "hpd_reliable".
> > >
> > > If you've got a board with a well-designed panel but the hookup
> > > between the panel and the board is wrong (maybe the board is missing a
> > > pulldown on the HPD line?) then you can just set the "no-hpd" property
> > > for your board. That will tell the kernel to just always delay the
> > > "hpd-absent" delay.
> > >
>
> Thank you for the detailed explanation, this does clear all doubts from what me
> and Angelo were discussing.
>
> >
> > We were concerned exactly about glitchy HPD during DDIC init, as I didn't
> > want to trust it because the only testing we could do was on just two units...
> >
> > ...but if you're sure that I was too much paranoid about that, that's good,
> > as it means I will be a bit more "relaxed" on this topic next time :-)
> >
> > > > I've set the T3 delay to hpd_absent in this series, following what's
> > > > instructed in the comments, but I'd like to discuss whether we shouldn't
> > > > be setting T3 on hpd_reliable instead, for all panels, to be on the
> > > > safer side.
> > >
> > > The way it's specified right now is more flexible, though, isn't it?
> > > This way if you're on a board where the HPD truly _isn't_ stable then
> > > you can just set the "no-hpd" and it will automatically use the
> > > "hpd_absent" delay, right?
> > >
>
> Yes, indeed. I just wasn't sure that flexibility brought us anything, but after
> your explanation above it makes much more sense now, thanks!
>
> >
> > For Chromebooks, that's totally doable, thanks to the bootloader seeking for
> > proper machine compatibles, so yes I agree with that.
> >
> > >
> > > >   drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
> > > >   1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > > > index 3626469c4cc2..675d793d925e 100644
> > > > --- a/drivers/gpu/drm/panel/panel-edp.c
> > > > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > > > @@ -1854,6 +1854,12 @@ static const struct panel_delay delay_100_500_e200 = {
> > > >          .enable = 200,
> > > >   };
> > > >
> > > > +static const struct panel_delay delay_200_500_e200 = {
> > > > +       .hpd_absent = 200,
> > > > +       .unprepare = 500,
> > > > +       .enable = 200,
> > > > +};
> > > > +
> > > >   #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
> > > >   { \
> > > >          .name = _name, \
> > > > @@ -1882,6 +1888,8 @@ static const struct edp_panel_entry edp_panels[] = {
> > > >
> > > >          EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay, "N116BCA-EA1"),
> > > >
> > > > +       EDP_PANEL_ENTRY('I', 'V', 'O', 0x057d, &delay_200_500_e200, "R140NWF5 RH"),
> > > > +
> > >
> > > This looks fine to me:
> > >
> > > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > >
> > > I'm happy to apply this in a day or two assuming you're OK with my
> > > explanation above.
> >
> > Thank you for the long mail, your explanation was truly helpful!
> > (especially for me being paranoid :-P)
> >
> > So, I agree to go with that one, for which:
> >
> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >
> > Nic, green light?
>
> Yep.
>
> I haven't seen any issues with keeping the hpd_reliable as 0 in the machine I
> have access to, and Douglas' explanation cleared up all the doubts of how this
> all works, so, Douglas, please feel free to merge this as is.
>
> In that case, since patch 3 was also merged already I'll send a v2 just for
> patch 2 separately.

Great! I went ahead and applied to drm-misc-next then.

f6ff4570e567 drm/panel-edp: Add panel entry for R140NWF5 RH

Re: [PATCH 1/3] drm/panel-edp: Add panel entry for R140NWF5 RH

[Doug Anderson]

Hi,

On Wed, Jul 20, 2022 at 11:52 AM Nícolas F. R. A. Prado
<nfraprado@collabora.com> wrote:
>
> On Wed, Jul 20, 2022 at 09:49:35AM +0200, AngeloGioacchino Del Regno wrote:
> > Il 20/07/22 00:40, Doug Anderson ha scritto:
> > > Hi,
> > >
> > > On Tue, Jul 19, 2022 at 1:39 PM Nícolas F. R. A. Prado
> > > <nfraprado@collabora.com> wrote:
> > > >
> > > > Add panel identification entry for the IVO R140NWF5 RH (product ID:
> > > > 0x057d) panel.
> > > >
> > > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > > >
> > > > ---
> > > > The comments on the driver indicate that the T3 timing should be set on
> > > > hpd_absent, while hpd_reliable would have a shorter time just while the
> > > > HPD line stabilizes on low after power is supplied.
> > >
> > > Right. Ideally hpd_reliable is 0 unless you've got a badly-designed panel.
> > >
> > >
> > > > But can we really assume that the HPD line will be reliable at all
> > > > before the DDIC is done booting up, at which point the HPD line is
> > > > brought up? IOW, shouldn't we always delay T3 (by setting hpd_reliable =
> > > > T3), since only then we're really sure that the DDIC is done setting up
> > > > and the HPD line is reliable?
> > >
> > > If the panel is hooked up properly, then the HPD pin should be pulled
> > > low at the start and then should only go high after the panel is ready
> > > for us to talk to it, right? So it's not like the DDIC has to boot up
> > > and actively init the state. I would assume that the initial state of
> > > the "HPD output" from the panel's IC would be one of the following:
> > > * A floating input.
> > > * A pulled down input.
> > > * An output driven low.
> > >
> > > In any of those cases just adding a pull down on the line would be
> > > enough to ensure that the HPD line is reliable until the panel comes
> > > around and actively drives the line high.
> > >
> > > Remember, this is eDP and it's not something that's hot-plugged, so
> > > there's no debouncing involved and in a properly designed system there
> > > should be no time needed for the signal to stabilize. I would also
> > > point out that on the oficial eDP docs the eDP timing diagram doesn't
> > > show the initial state of "HPD" as "unknown". It shows it as low.
> > >
> > > Now, that all being said, I have seen at least one panel that glitched
> > > itself at bootup. After you powered it on it would blip its HPD line
> > > high before it had actually finished booting. Then the HPD would go
> > > low again before finally going high after the panel finished booting.
> > > This is the reason for "hpd_reliable".
> > >
> > > If you've got a board with a well-designed panel but the hookup
> > > between the panel and the board is wrong (maybe the board is missing a
> > > pulldown on the HPD line?) then you can just set the "no-hpd" property
> > > for your board. That will tell the kernel to just always delay the
> > > "hpd-absent" delay.
> > >
>
> Thank you for the detailed explanation, this does clear all doubts from what me
> and Angelo were discussing.
>
> >
> > We were concerned exactly about glitchy HPD during DDIC init, as I didn't
> > want to trust it because the only testing we could do was on just two units...
> >
> > ...but if you're sure that I was too much paranoid about that, that's good,
> > as it means I will be a bit more "relaxed" on this topic next time :-)
> >
> > > > I've set the T3 delay to hpd_absent in this series, following what's
> > > > instructed in the comments, but I'd like to discuss whether we shouldn't
> > > > be setting T3 on hpd_reliable instead, for all panels, to be on the
> > > > safer side.
> > >
> > > The way it's specified right now is more flexible, though, isn't it?
> > > This way if you're on a board where the HPD truly _isn't_ stable then
> > > you can just set the "no-hpd" and it will automatically use the
> > > "hpd_absent" delay, right?
> > >
>
> Yes, indeed. I just wasn't sure that flexibility brought us anything, but after
> your explanation above it makes much more sense now, thanks!
>
> >
> > For Chromebooks, that's totally doable, thanks to the bootloader seeking for
> > proper machine compatibles, so yes I agree with that.
> >
> > >
> > > >   drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
> > > >   1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> > > > index 3626469c4cc2..675d793d925e 100644
> > > > --- a/drivers/gpu/drm/panel/panel-edp.c
> > > > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > > > @@ -1854,6 +1854,12 @@ static const struct panel_delay delay_100_500_e200 = {
> > > >          .enable = 200,
> > > >   };
> > > >
> > > > +static const struct panel_delay delay_200_500_e200 = {
> > > > +       .hpd_absent = 200,
> > > > +       .unprepare = 500,
> > > > +       .enable = 200,
> > > > +};
> > > > +
> > > >   #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
> > > >   { \
> > > >          .name = _name, \
> > > > @@ -1882,6 +1888,8 @@ static const struct edp_panel_entry edp_panels[] = {
> > > >
> > > >          EDP_PANEL_ENTRY('C', 'M', 'N', 0x114c, &innolux_n116bca_ea1.delay, "N116BCA-EA1"),
> > > >
> > > > +       EDP_PANEL_ENTRY('I', 'V', 'O', 0x057d, &delay_200_500_e200, "R140NWF5 RH"),
> > > > +
> > >
> > > This looks fine to me:
> > >
> > > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > >
> > > I'm happy to apply this in a day or two assuming you're OK with my
> > > explanation above.
> >
> > Thank you for the long mail, your explanation was truly helpful!
> > (especially for me being paranoid :-P)
> >
> > So, I agree to go with that one, for which:
> >
> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> >
> > Nic, green light?
>
> Yep.
>
> I haven't seen any issues with keeping the hpd_reliable as 0 in the machine I
> have access to, and Douglas' explanation cleared up all the doubts of how this
> all works, so, Douglas, please feel free to merge this as is.
>
> In that case, since patch 3 was also merged already I'll send a v2 just for
> patch 2 separately.

Great! I went ahead and applied to drm-misc-next then.

f6ff4570e567 drm/panel-edp: Add panel entry for R140NWF5 RH