[PATCH v3 1/3] drm: panel: simple: Allow timing constraints, not fixed delays

[PATCH v3 1/3] drm: panel: simple: Allow timing constraints, not fixed delays

[Douglas Anderson]

The simple panel code currently allows panels to define fixed delays
at certain stages of initialization.  These work OK, but they don't
really map all that clearly to the requirements presented in many
panel datasheets.  Instead of defining a fixed delay, those datasheets
provide a timing diagram and specify a minimum amount of time that
needs to pass from event A to event B.

Because of the way things are currently defined, most panels end up
over-delaying.  One prime example here is that a number of panels I've
looked at define the amount of time that must pass between turning a
panel off and turning it back on again.  Since there is no way to
specify this, many developers have listed this as the "unprepare"
delay.  However, if nobody ever tried to turn the panel on again in
the next 500 ms (or whatever the delay was) then this delay was
pointless.  It's better to do the delay only in the case that someone
tried to turn the panel on too quickly.

Let's support specifying delays as constraints.  We'll start with the
one above and also a second one: the minimum time between prepare
being done and doing the enable.  On the panel I'm looking at, there's
an 80 ms minimum time between HPD being asserted by the panel and
setting the backlight enable GPIO.  By specifying as a constraint we
can enforce this without over-delaying.  Specifically the link
training is allowed to happen in parallel with this delay so adding a
fixed 80 ms delay isn't ideal.

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

Changes in v3:
- Fixed totally backwards "if" tests.  :(

Changes in v2:
- Inline the kernel doc for the two new members.
- Beefed up kernel doc saying exactly when the delay happens.
- Removed "_ms" from the end of members to shorten them.
- Renamed "timing_constraints" to "min_times" to shorten it.
- Renamed "enforce_constraint()" to "wait_min_time()" to shorten it.
- Check "prepared_time" against 0 to see if we've been prepared.

 drivers/gpu/drm/panel/panel-simple.c | 99 ++++++++++++++++++++++++++--
 1 file changed, 92 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 2be358fb46f7..bb1e3d15f793 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -92,6 +92,68 @@ struct panel_desc {
 		unsigned int unprepare;
 	} delay;
 
+	struct {
+		/**
+		 * @prepare_to_enable: Time between prepare and enable.
+		 *
+		 * The minimum time, in milliseconds, that needs to have passed
+		 * between when prepare finished and enable may begin. If at
+		 * enable time less time has passed since prepare finished,
+		 * the driver waits for the remaining time.
+		 *
+		 * If a fixed enable delay is also specified, we'll start
+		 * counting before delaying for the fixed delay.
+		 *
+		 * If a fixed prepare delay is also specified, we won't start
+		 * counting until after the fixed delay. We can't overlap this
+		 * fixed delay with the min time because the fixed delay
+		 * doesn't happen at the end of the function if a HPD GPIO was
+		 * specified.
+		 *
+		 * In other words:
+		 *   prepare()
+		 *     ...
+		 *     // do fixed prepare delay
+		 *     // wait for HPD GPIO if applicable
+		 *     // start counting for prepare_to_enable
+		 *
+		 *   enable()
+		 *     // do fixed enable delay
+		 *     // enforce prepare_to_enable min time
+		 */
+		unsigned int prepare_to_enable;
+
+		/**
+		 * @unprepare_to_prepare: Time between unprepare and prepare.
+		 *
+		 * The minimum time, in milliseconds, that needs to have passed
+		 * between when unprepare finished and prepare may begin. If at
+		 * prepare time less time has passed since unprepare finished,
+		 * the driver waits for the remaining time.
+		 *
+		 * If a fixed unprepare delay is also specified, we'll start
+		 * counting before delaying for the fixed delay.
+		 *
+		 * If a fixed prepare delay is also specified, it will happen
+		 * separately and after we've enforced this minimum. We can't
+		 * overlap this fixed delay with the min time because the
+		 * fixed delay doesn't happen at the start of the function
+		 * if a regulator or enable GPIO was specified.
+		 *
+		 * In other words:
+		 *   unprepare():
+		 *     ...
+		 *     // start counting for unprepare_to_prepare
+		 *     // do fixed unprepare delay
+		 *
+		 *   prepare():
+		 *     // enforce unprepare_to_prepare min time
+		 *     // turn on regulator / set enable GPIO if applicable
+		 *     // do fixed prepare delay
+		 */
+		unsigned int unprepare_to_prepare;
+	} min_times;
+
 	u32 bus_format;
 	u32 bus_flags;
 	int connector_type;
@@ -99,10 +161,12 @@ struct panel_desc {
 
 struct panel_simple {
 	struct drm_panel base;
-	bool prepared;
 	bool enabled;
 	bool no_hpd;
 
+	ktime_t prepared_time;
+	ktime_t unprepared_time;
+
 	const struct panel_desc *desc;
 
 	struct regulator *supply;
@@ -230,6 +294,20 @@ static int panel_simple_get_non_edid_modes(struct panel_simple *panel,
 	return num;
 }
 
+static void panel_simple_wait_min_time(ktime_t start_ktime, unsigned int min_ms)
+{
+	ktime_t now_ktime, min_ktime;
+
+	if (!min_ms)
+		return;
+
+	min_ktime = ktime_add(start_ktime, ms_to_ktime(min_ms));
+	now_ktime = ktime_get();
+
+	if (ktime_before(now_ktime, min_ktime))
+		msleep(ktime_to_ms(ktime_sub(min_ktime, now_ktime)) + 1);
+}
+
 static int panel_simple_disable(struct drm_panel *panel)
 {
 	struct panel_simple *p = to_panel_simple(panel);
@@ -249,18 +327,19 @@ static int panel_simple_unprepare(struct drm_panel *panel)
 {
 	struct panel_simple *p = to_panel_simple(panel);
 
-	if (!p->prepared)
+	if (p->prepared_time == 0)
 		return 0;
 
 	gpiod_set_value_cansleep(p->enable_gpio, 0);
 
 	regulator_disable(p->supply);
 
+	p->prepared_time = 0;
+	p->unprepared_time = ktime_get();
+
 	if (p->desc->delay.unprepare)
 		msleep(p->desc->delay.unprepare);
 
-	p->prepared = false;
-
 	return 0;
 }
 
@@ -296,9 +375,12 @@ static int panel_simple_prepare(struct drm_panel *panel)
 	int err;
 	int hpd_asserted;
 
-	if (p->prepared)
+	if (p->prepared_time != 0)
 		return 0;
 
+	panel_simple_wait_min_time(p->unprepared_time,
+				   p->desc->min_times.unprepare_to_prepare);
+
 	err = regulator_enable(p->supply);
 	if (err < 0) {
 		dev_err(panel->dev, "failed to enable supply: %d\n", err);
@@ -333,7 +415,7 @@ static int panel_simple_prepare(struct drm_panel *panel)
 		}
 	}
 
-	p->prepared = true;
+	p->prepared_time = ktime_get();
 
 	return 0;
 }
@@ -348,6 +430,9 @@ static int panel_simple_enable(struct drm_panel *panel)
 	if (p->desc->delay.enable)
 		msleep(p->desc->delay.enable);
 
+	panel_simple_wait_min_time(p->prepared_time,
+				   p->desc->min_times.prepare_to_enable);
+
 	p->enabled = true;
 
 	return 0;
@@ -514,7 +599,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 		return -ENOMEM;
 
 	panel->enabled = false;
-	panel->prepared = false;
+	panel->prepared_time = 0;
 	panel->desc = desc;
 
 	panel->no_hpd = of_property_read_bool(dev->of_node, "no-hpd");
-- 
2.29.1.341.ge80a0c044ae-goog

[PATCH v3 1/3] drm: panel: simple: Allow timing constraints, not fixed delays

[Douglas Anderson]

The simple panel code currently allows panels to define fixed delays
at certain stages of initialization.  These work OK, but they don't
really map all that clearly to the requirements presented in many
panel datasheets.  Instead of defining a fixed delay, those datasheets
provide a timing diagram and specify a minimum amount of time that
needs to pass from event A to event B.

Because of the way things are currently defined, most panels end up
over-delaying.  One prime example here is that a number of panels I've
looked at define the amount of time that must pass between turning a
panel off and turning it back on again.  Since there is no way to
specify this, many developers have listed this as the "unprepare"
delay.  However, if nobody ever tried to turn the panel on again in
the next 500 ms (or whatever the delay was) then this delay was
pointless.  It's better to do the delay only in the case that someone
tried to turn the panel on too quickly.

Let's support specifying delays as constraints.  We'll start with the
one above and also a second one: the minimum time between prepare
being done and doing the enable.  On the panel I'm looking at, there's
an 80 ms minimum time between HPD being asserted by the panel and
setting the backlight enable GPIO.  By specifying as a constraint we
can enforce this without over-delaying.  Specifically the link
training is allowed to happen in parallel with this delay so adding a
fixed 80 ms delay isn't ideal.

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

Changes in v3:
- Fixed totally backwards "if" tests.  :(

Changes in v2:
- Inline the kernel doc for the two new members.
- Beefed up kernel doc saying exactly when the delay happens.
- Removed "_ms" from the end of members to shorten them.
- Renamed "timing_constraints" to "min_times" to shorten it.
- Renamed "enforce_constraint()" to "wait_min_time()" to shorten it.
- Check "prepared_time" against 0 to see if we've been prepared.

 drivers/gpu/drm/panel/panel-simple.c | 99 ++++++++++++++++++++++++++--
 1 file changed, 92 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 2be358fb46f7..bb1e3d15f793 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -92,6 +92,68 @@ struct panel_desc {
 		unsigned int unprepare;
 	} delay;
 
+	struct {
+		/**
+		 * @prepare_to_enable: Time between prepare and enable.
+		 *
+		 * The minimum time, in milliseconds, that needs to have passed
+		 * between when prepare finished and enable may begin. If at
+		 * enable time less time has passed since prepare finished,
+		 * the driver waits for the remaining time.
+		 *
+		 * If a fixed enable delay is also specified, we'll start
+		 * counting before delaying for the fixed delay.
+		 *
+		 * If a fixed prepare delay is also specified, we won't start
+		 * counting until after the fixed delay. We can't overlap this
+		 * fixed delay with the min time because the fixed delay
+		 * doesn't happen at the end of the function if a HPD GPIO was
+		 * specified.
+		 *
+		 * In other words:
+		 *   prepare()
+		 *     ...
+		 *     // do fixed prepare delay
+		 *     // wait for HPD GPIO if applicable
+		 *     // start counting for prepare_to_enable
+		 *
+		 *   enable()
+		 *     // do fixed enable delay
+		 *     // enforce prepare_to_enable min time
+		 */
+		unsigned int prepare_to_enable;
+
+		/**
+		 * @unprepare_to_prepare: Time between unprepare and prepare.
+		 *
+		 * The minimum time, in milliseconds, that needs to have passed
+		 * between when unprepare finished and prepare may begin. If at
+		 * prepare time less time has passed since unprepare finished,
+		 * the driver waits for the remaining time.
+		 *
+		 * If a fixed unprepare delay is also specified, we'll start
+		 * counting before delaying for the fixed delay.
+		 *
+		 * If a fixed prepare delay is also specified, it will happen
+		 * separately and after we've enforced this minimum. We can't
+		 * overlap this fixed delay with the min time because the
+		 * fixed delay doesn't happen at the start of the function
+		 * if a regulator or enable GPIO was specified.
+		 *
+		 * In other words:
+		 *   unprepare():
+		 *     ...
+		 *     // start counting for unprepare_to_prepare
+		 *     // do fixed unprepare delay
+		 *
+		 *   prepare():
+		 *     // enforce unprepare_to_prepare min time
+		 *     // turn on regulator / set enable GPIO if applicable
+		 *     // do fixed prepare delay
+		 */
+		unsigned int unprepare_to_prepare;
+	} min_times;
+
 	u32 bus_format;
 	u32 bus_flags;
 	int connector_type;
@@ -99,10 +161,12 @@ struct panel_desc {
 
 struct panel_simple {
 	struct drm_panel base;
-	bool prepared;
 	bool enabled;
 	bool no_hpd;
 
+	ktime_t prepared_time;
+	ktime_t unprepared_time;
+
 	const struct panel_desc *desc;
 
 	struct regulator *supply;
@@ -230,6 +294,20 @@ static int panel_simple_get_non_edid_modes(struct panel_simple *panel,
 	return num;
 }
 
+static void panel_simple_wait_min_time(ktime_t start_ktime, unsigned int min_ms)
+{
+	ktime_t now_ktime, min_ktime;
+
+	if (!min_ms)
+		return;
+
+	min_ktime = ktime_add(start_ktime, ms_to_ktime(min_ms));
+	now_ktime = ktime_get();
+
+	if (ktime_before(now_ktime, min_ktime))
+		msleep(ktime_to_ms(ktime_sub(min_ktime, now_ktime)) + 1);
+}
+
 static int panel_simple_disable(struct drm_panel *panel)
 {
 	struct panel_simple *p = to_panel_simple(panel);
@@ -249,18 +327,19 @@ static int panel_simple_unprepare(struct drm_panel *panel)
 {
 	struct panel_simple *p = to_panel_simple(panel);
 
-	if (!p->prepared)
+	if (p->prepared_time == 0)
 		return 0;
 
 	gpiod_set_value_cansleep(p->enable_gpio, 0);
 
 	regulator_disable(p->supply);
 
+	p->prepared_time = 0;
+	p->unprepared_time = ktime_get();
+
 	if (p->desc->delay.unprepare)
 		msleep(p->desc->delay.unprepare);
 
-	p->prepared = false;
-
 	return 0;
 }
 
@@ -296,9 +375,12 @@ static int panel_simple_prepare(struct drm_panel *panel)
 	int err;
 	int hpd_asserted;
 
-	if (p->prepared)
+	if (p->prepared_time != 0)
 		return 0;
 
+	panel_simple_wait_min_time(p->unprepared_time,
+				   p->desc->min_times.unprepare_to_prepare);
+
 	err = regulator_enable(p->supply);
 	if (err < 0) {
 		dev_err(panel->dev, "failed to enable supply: %d\n", err);
@@ -333,7 +415,7 @@ static int panel_simple_prepare(struct drm_panel *panel)
 		}
 	}
 
-	p->prepared = true;
+	p->prepared_time = ktime_get();
 
 	return 0;
 }
@@ -348,6 +430,9 @@ static int panel_simple_enable(struct drm_panel *panel)
 	if (p->desc->delay.enable)
 		msleep(p->desc->delay.enable);
 
+	panel_simple_wait_min_time(p->prepared_time,
+				   p->desc->min_times.prepare_to_enable);
+
 	p->enabled = true;
 
 	return 0;
@@ -514,7 +599,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 		return -ENOMEM;
 
 	panel->enabled = false;
-	panel->prepared = false;
+	panel->prepared_time = 0;
 	panel->desc = desc;
 
 	panel->no_hpd = of_property_read_bool(dev->of_node, "no-hpd");
-- 
2.29.1.341.ge80a0c044ae-goog

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

[PATCH v3 2/3] drm: panel: simple: Add BOE NV110WTM-N61

[Douglas Anderson]

Add support for the BOE NV110WTM-N61 panel.  The EDID lists two modes
(one for 60 Hz refresh rate and one for 40 Hz), so we'll list both of
them here.

Note that the panel datasheet requires 80 ms between HPD asserting and
the backlight power being turned on.  We'll use the new timing
constraints structure to do this cleanly.  This assumes that the
backlight will be enabled _after_ the panel enable finishes.  This is
how it works today and seems a sane assumption.

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

(no changes since v2)

Changes in v2:
- Adjust for shorter names in patch #1.

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

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index bb1e3d15f793..db3f0e2b6001 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1401,6 +1401,51 @@ static const struct panel_desc boe_nv101wxmn51 = {
 	},
 };
 
+static const struct drm_display_mode boe_nv110wtm_n61_modes[] = {
+	{
+		.clock = 207800,
+		.hdisplay = 2160,
+		.hsync_start = 2160 + 48,
+		.hsync_end = 2160 + 48 + 32,
+		.htotal = 2160 + 48 + 32 + 100,
+		.vdisplay = 1440,
+		.vsync_start = 1440 + 3,
+		.vsync_end = 1440 + 3 + 6,
+		.vtotal = 1440 + 3 + 6 + 31,
+	},
+	{
+		.clock = 138500,
+		.hdisplay = 2160,
+		.hsync_start = 2160 + 48,
+		.hsync_end = 2160 + 48 + 32,
+		.htotal = 2160 + 48 + 32 + 100,
+		.vdisplay = 1440,
+		.vsync_start = 1440 + 3,
+		.vsync_end = 1440 + 3 + 6,
+		.vtotal = 1440 + 3 + 6 + 31,
+	},
+};
+
+static const struct panel_desc boe_nv110wtm_n61 = {
+	.modes = boe_nv110wtm_n61_modes,
+	.num_modes = ARRAY_SIZE(boe_nv110wtm_n61_modes),
+	.bpc = 8,
+	.size = {
+		.width = 233,
+		.height = 155,
+	},
+	.delay = {
+		.hpd_absent_delay = 200,
+	},
+	.min_times = {
+		.prepare_to_enable = 80,
+		.unprepare_to_prepare = 500,
+	},
+	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+	.bus_flags = DRM_BUS_FLAG_DATA_MSB_TO_LSB,
+	.connector_type = DRM_MODE_CONNECTOR_eDP,
+};
+
 /* Also used for boe_nv133fhm_n62 */
 static const struct drm_display_mode boe_nv133fhm_n61_modes = {
 	.clock = 147840,
@@ -4063,6 +4108,9 @@ static const struct of_device_id platform_of_match[] = {
 	}, {
 		.compatible = "boe,nv101wxmn51",
 		.data = &boe_nv101wxmn51,
+	}, {
+		.compatible = "boe,nv110wtm-n61",
+		.data = &boe_nv110wtm_n61,
 	}, {
 		.compatible = "boe,nv133fhm-n61",
 		.data = &boe_nv133fhm_n61,
-- 
2.29.1.341.ge80a0c044ae-goog

[PATCH v3 2/3] drm: panel: simple: Add BOE NV110WTM-N61

[Douglas Anderson]

Add support for the BOE NV110WTM-N61 panel.  The EDID lists two modes
(one for 60 Hz refresh rate and one for 40 Hz), so we'll list both of
them here.

Note that the panel datasheet requires 80 ms between HPD asserting and
the backlight power being turned on.  We'll use the new timing
constraints structure to do this cleanly.  This assumes that the
backlight will be enabled _after_ the panel enable finishes.  This is
how it works today and seems a sane assumption.

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

(no changes since v2)

Changes in v2:
- Adjust for shorter names in patch #1.

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

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index bb1e3d15f793..db3f0e2b6001 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1401,6 +1401,51 @@ static const struct panel_desc boe_nv101wxmn51 = {
 	},
 };
 
+static const struct drm_display_mode boe_nv110wtm_n61_modes[] = {
+	{
+		.clock = 207800,
+		.hdisplay = 2160,
+		.hsync_start = 2160 + 48,
+		.hsync_end = 2160 + 48 + 32,
+		.htotal = 2160 + 48 + 32 + 100,
+		.vdisplay = 1440,
+		.vsync_start = 1440 + 3,
+		.vsync_end = 1440 + 3 + 6,
+		.vtotal = 1440 + 3 + 6 + 31,
+	},
+	{
+		.clock = 138500,
+		.hdisplay = 2160,
+		.hsync_start = 2160 + 48,
+		.hsync_end = 2160 + 48 + 32,
+		.htotal = 2160 + 48 + 32 + 100,
+		.vdisplay = 1440,
+		.vsync_start = 1440 + 3,
+		.vsync_end = 1440 + 3 + 6,
+		.vtotal = 1440 + 3 + 6 + 31,
+	},
+};
+
+static const struct panel_desc boe_nv110wtm_n61 = {
+	.modes = boe_nv110wtm_n61_modes,
+	.num_modes = ARRAY_SIZE(boe_nv110wtm_n61_modes),
+	.bpc = 8,
+	.size = {
+		.width = 233,
+		.height = 155,
+	},
+	.delay = {
+		.hpd_absent_delay = 200,
+	},
+	.min_times = {
+		.prepare_to_enable = 80,
+		.unprepare_to_prepare = 500,
+	},
+	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+	.bus_flags = DRM_BUS_FLAG_DATA_MSB_TO_LSB,
+	.connector_type = DRM_MODE_CONNECTOR_eDP,
+};
+
 /* Also used for boe_nv133fhm_n62 */
 static const struct drm_display_mode boe_nv133fhm_n61_modes = {
 	.clock = 147840,
@@ -4063,6 +4108,9 @@ static const struct of_device_id platform_of_match[] = {
 	}, {
 		.compatible = "boe,nv101wxmn51",
 		.data = &boe_nv101wxmn51,
+	}, {
+		.compatible = "boe,nv110wtm-n61",
+		.data = &boe_nv110wtm_n61,
 	}, {
 		.compatible = "boe,nv133fhm-n61",
 		.data = &boe_nv133fhm_n61,
-- 
2.29.1.341.ge80a0c044ae-goog

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

[PATCH v3 3/3] dt-bindings: dt-bindings: display: simple: Add BOE NV110WTM-N61

[Douglas Anderson]

Add yet another eDP panel.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Rob Herring <robh@kernel.org>
---

(no changes since v1)

 .../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 edb53ab0d9eb..93e244c67e8a 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -76,6 +76,8 @@ properties:
         # BOE OPTOELECTRONICS TECHNOLOGY 10.1" WXGA TFT LCD panel
       - boe,nv101wxmn51
         # BOE NV133FHM-N61 13.3" FHD (1920x1080) TFT LCD Panel
+      - boe,nv110wtm-n61
+        # BOE NV110WTM-N61 11.0" 2160x1440 TFT LCD Panel
       - boe,nv133fhm-n61
         # BOE NV133FHM-N62 13.3" FHD (1920x1080) TFT LCD Panel
       - boe,nv133fhm-n62
-- 
2.29.1.341.ge80a0c044ae-goog

[PATCH v3 3/3] dt-bindings: dt-bindings: display: simple: Add BOE NV110WTM-N61

[Douglas Anderson]

Add yet another eDP panel.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Rob Herring <robh@kernel.org>
---

(no changes since v1)

 .../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 edb53ab0d9eb..93e244c67e8a 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -76,6 +76,8 @@ properties:
         # BOE OPTOELECTRONICS TECHNOLOGY 10.1" WXGA TFT LCD panel
       - boe,nv101wxmn51
         # BOE NV133FHM-N61 13.3" FHD (1920x1080) TFT LCD Panel
+      - boe,nv110wtm-n61
+        # BOE NV110WTM-N61 11.0" 2160x1440 TFT LCD Panel
       - boe,nv133fhm-n61
         # BOE NV133FHM-N62 13.3" FHD (1920x1080) TFT LCD Panel
       - boe,nv133fhm-n62
-- 
2.29.1.341.ge80a0c044ae-goog

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

Re: [PATCH v3 1/3] drm: panel: simple: Allow timing constraints, not fixed delays

[Sam Ravnborg]

Hi Douglas.

On Thu, Nov 05, 2020 at 01:57:39PM -0800, Douglas Anderson wrote:
> The simple panel code currently allows panels to define fixed delays
> at certain stages of initialization.  These work OK, but they don't
> really map all that clearly to the requirements presented in many
> panel datasheets.  Instead of defining a fixed delay, those datasheets
> provide a timing diagram and specify a minimum amount of time that
> needs to pass from event A to event B.
> 
> Because of the way things are currently defined, most panels end up
> over-delaying.  One prime example here is that a number of panels I've
> looked at define the amount of time that must pass between turning a
> panel off and turning it back on again.  Since there is no way to
> specify this, many developers have listed this as the "unprepare"
> delay.  However, if nobody ever tried to turn the panel on again in
> the next 500 ms (or whatever the delay was) then this delay was
> pointless.  It's better to do the delay only in the case that someone
> tried to turn the panel on too quickly.
> 
> Let's support specifying delays as constraints.  We'll start with the
> one above and also a second one: the minimum time between prepare
> being done and doing the enable.  On the panel I'm looking at, there's
> an 80 ms minimum time between HPD being asserted by the panel and
> setting the backlight enable GPIO.  By specifying as a constraint we
> can enforce this without over-delaying.  Specifically the link
> training is allowed to happen in parallel with this delay so adding a
> fixed 80 ms delay isn't ideal.
Nice and detaild explanation - thanks.

Reading through this a few times it seems that a simpler approach would
be to change the semantics of the dealys we already have a little and
only add one new field:

       struct {
                unsigned int prepare;
                unsigned int hpd_absent_delay;
		unsigned int prepare_to_enable;
                unsigned int enable;
                unsigned int disable;
                unsigned int unprepare;
        } delay;

The lines marked "//*" are new or changed:

prepare()
//*enforce unprepare time
// enable regulator
// set enable gpio
// do fixed prepare delay (this is time until we can trust hpd)
// wait for HPD GPIO if applicable, otherwise do fixed hpd_absent_delay
//*start counting for prepare_to_enable

enable()
//*enforce prepare_to_enable min time
// enable backlight if applicable

panel shows nice pictures of kitties

disable()
// disable backlight is applicable
// fixed disable delay

unprepare()
// unset enable gpio
// disable regulator
//*start counting for unprepare delay


This should not break any exisitng panels - and we avoid that we have
two delays that are almost the same.

Would that work for you?

Note that no new struct was introduced - this is all dealys so keep it in
the same struct.

A dew details in the following.

	Sam

> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
> Changes in v3:
> - Fixed totally backwards "if" tests.  :(
> 
> Changes in v2:
> - Inline the kernel doc for the two new members.
A nice follow-up patch would be to move the remaining fields as inline
doc. But let us settle on this first.

>  struct panel_simple {
>  	struct drm_panel base;
> -	bool prepared;
>  	bool enabled;
>  	bool no_hpd;
>  
> +	ktime_t prepared_time;
> +	ktime_t unprepared_time;
> +
>  	const struct panel_desc *desc;
>  
>  	struct regulator *supply;
> @@ -230,6 +294,20 @@ static int panel_simple_get_non_edid_modes(struct panel_simple *panel,
>  	return num;
>  }
>  
> +static void panel_simple_wait_min_time(ktime_t start_ktime, unsigned int min_ms)
> +{
> +	ktime_t now_ktime, min_ktime;
> +
> +	if (!min_ms)
> +		return;
> +
> +	min_ktime = ktime_add(start_ktime, ms_to_ktime(min_ms));
> +	now_ktime = ktime_get();
> +
> +	if (ktime_before(now_ktime, min_ktime))
> +		msleep(ktime_to_ms(ktime_sub(min_ktime, now_ktime)) + 1);
> +}
panel_simple_wait()?


> +
>  static int panel_simple_disable(struct drm_panel *panel)
>  {
>  	struct panel_simple *p = to_panel_simple(panel);
> @@ -249,18 +327,19 @@ static int panel_simple_unprepare(struct drm_panel *panel)
>  {
>  	struct panel_simple *p = to_panel_simple(panel);
>  
> -	if (!p->prepared)
> +	if (p->prepared_time == 0)
>  		return 0;
>  
>  	gpiod_set_value_cansleep(p->enable_gpio, 0);
>  
>  	regulator_disable(p->supply);
>  
> +	p->prepared_time = 0;
> +	p->unprepared_time = ktime_get();
> +
>  	if (p->desc->delay.unprepare)
>  		msleep(p->desc->delay.unprepare);
>  
> -	p->prepared = false;
> -
>  	return 0;
>  }
>  
> @@ -296,9 +375,12 @@ static int panel_simple_prepare(struct drm_panel *panel)
>  	int err;
>  	int hpd_asserted;
>  
> -	if (p->prepared)
> +	if (p->prepared_time != 0)
>  		return 0;
>  
> +	panel_simple_wait_min_time(p->unprepared_time,
> +				   p->desc->min_times.unprepare_to_prepare);
> +
>  	err = regulator_enable(p->supply);
>  	if (err < 0) {
>  		dev_err(panel->dev, "failed to enable supply: %d\n", err);
> @@ -333,7 +415,7 @@ static int panel_simple_prepare(struct drm_panel *panel)
>  		}
>  	}
>  
> -	p->prepared = true;
> +	p->prepared_time = ktime_get();
>  
>  	return 0;
>  }
> @@ -348,6 +430,9 @@ static int panel_simple_enable(struct drm_panel *panel)
>  	if (p->desc->delay.enable)
>  		msleep(p->desc->delay.enable);
>  
> +	panel_simple_wait_min_time(p->prepared_time,
> +				   p->desc->min_times.prepare_to_enable);
> +
>  	p->enabled = true;
>  
>  	return 0;
> @@ -514,7 +599,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
>  		return -ENOMEM;
>  
>  	panel->enabled = false;
> -	panel->prepared = false;
> +	panel->prepared_time = 0;
>  	panel->desc = desc;
>  
>  	panel->no_hpd = of_property_read_bool(dev->of_node, "no-hpd");
> -- 
> 2.29.1.341.ge80a0c044ae-goog
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/3] drm: panel: simple: Allow timing constraints, not fixed delays

[Sam Ravnborg]

Hi Douglas.

On Thu, Nov 05, 2020 at 01:57:39PM -0800, Douglas Anderson wrote:
> The simple panel code currently allows panels to define fixed delays
> at certain stages of initialization.  These work OK, but they don't
> really map all that clearly to the requirements presented in many
> panel datasheets.  Instead of defining a fixed delay, those datasheets
> provide a timing diagram and specify a minimum amount of time that
> needs to pass from event A to event B.
> 
> Because of the way things are currently defined, most panels end up
> over-delaying.  One prime example here is that a number of panels I've
> looked at define the amount of time that must pass between turning a
> panel off and turning it back on again.  Since there is no way to
> specify this, many developers have listed this as the "unprepare"
> delay.  However, if nobody ever tried to turn the panel on again in
> the next 500 ms (or whatever the delay was) then this delay was
> pointless.  It's better to do the delay only in the case that someone
> tried to turn the panel on too quickly.
> 
> Let's support specifying delays as constraints.  We'll start with the
> one above and also a second one: the minimum time between prepare
> being done and doing the enable.  On the panel I'm looking at, there's
> an 80 ms minimum time between HPD being asserted by the panel and
> setting the backlight enable GPIO.  By specifying as a constraint we
> can enforce this without over-delaying.  Specifically the link
> training is allowed to happen in parallel with this delay so adding a
> fixed 80 ms delay isn't ideal.
Nice and detaild explanation - thanks.

Reading through this a few times it seems that a simpler approach would
be to change the semantics of the dealys we already have a little and
only add one new field:

       struct {
                unsigned int prepare;
                unsigned int hpd_absent_delay;
		unsigned int prepare_to_enable;
                unsigned int enable;
                unsigned int disable;
                unsigned int unprepare;
        } delay;

The lines marked "//*" are new or changed:

prepare()
//*enforce unprepare time
// enable regulator
// set enable gpio
// do fixed prepare delay (this is time until we can trust hpd)
// wait for HPD GPIO if applicable, otherwise do fixed hpd_absent_delay
//*start counting for prepare_to_enable

enable()
//*enforce prepare_to_enable min time
// enable backlight if applicable

panel shows nice pictures of kitties

disable()
// disable backlight is applicable
// fixed disable delay

unprepare()
// unset enable gpio
// disable regulator
//*start counting for unprepare delay


This should not break any exisitng panels - and we avoid that we have
two delays that are almost the same.

Would that work for you?

Note that no new struct was introduced - this is all dealys so keep it in
the same struct.

A dew details in the following.

	Sam

> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
> Changes in v3:
> - Fixed totally backwards "if" tests.  :(
> 
> Changes in v2:
> - Inline the kernel doc for the two new members.
A nice follow-up patch would be to move the remaining fields as inline
doc. But let us settle on this first.

>  struct panel_simple {
>  	struct drm_panel base;
> -	bool prepared;
>  	bool enabled;
>  	bool no_hpd;
>  
> +	ktime_t prepared_time;
> +	ktime_t unprepared_time;
> +
>  	const struct panel_desc *desc;
>  
>  	struct regulator *supply;
> @@ -230,6 +294,20 @@ static int panel_simple_get_non_edid_modes(struct panel_simple *panel,
>  	return num;
>  }
>  
> +static void panel_simple_wait_min_time(ktime_t start_ktime, unsigned int min_ms)
> +{
> +	ktime_t now_ktime, min_ktime;
> +
> +	if (!min_ms)
> +		return;
> +
> +	min_ktime = ktime_add(start_ktime, ms_to_ktime(min_ms));
> +	now_ktime = ktime_get();
> +
> +	if (ktime_before(now_ktime, min_ktime))
> +		msleep(ktime_to_ms(ktime_sub(min_ktime, now_ktime)) + 1);
> +}
panel_simple_wait()?


> +
>  static int panel_simple_disable(struct drm_panel *panel)
>  {
>  	struct panel_simple *p = to_panel_simple(panel);
> @@ -249,18 +327,19 @@ static int panel_simple_unprepare(struct drm_panel *panel)
>  {
>  	struct panel_simple *p = to_panel_simple(panel);
>  
> -	if (!p->prepared)
> +	if (p->prepared_time == 0)
>  		return 0;
>  
>  	gpiod_set_value_cansleep(p->enable_gpio, 0);
>  
>  	regulator_disable(p->supply);
>  
> +	p->prepared_time = 0;
> +	p->unprepared_time = ktime_get();
> +
>  	if (p->desc->delay.unprepare)
>  		msleep(p->desc->delay.unprepare);
>  
> -	p->prepared = false;
> -
>  	return 0;
>  }
>  
> @@ -296,9 +375,12 @@ static int panel_simple_prepare(struct drm_panel *panel)
>  	int err;
>  	int hpd_asserted;
>  
> -	if (p->prepared)
> +	if (p->prepared_time != 0)
>  		return 0;
>  
> +	panel_simple_wait_min_time(p->unprepared_time,
> +				   p->desc->min_times.unprepare_to_prepare);
> +
>  	err = regulator_enable(p->supply);
>  	if (err < 0) {
>  		dev_err(panel->dev, "failed to enable supply: %d\n", err);
> @@ -333,7 +415,7 @@ static int panel_simple_prepare(struct drm_panel *panel)
>  		}
>  	}
>  
> -	p->prepared = true;
> +	p->prepared_time = ktime_get();
>  
>  	return 0;
>  }
> @@ -348,6 +430,9 @@ static int panel_simple_enable(struct drm_panel *panel)
>  	if (p->desc->delay.enable)
>  		msleep(p->desc->delay.enable);
>  
> +	panel_simple_wait_min_time(p->prepared_time,
> +				   p->desc->min_times.prepare_to_enable);
> +
>  	p->enabled = true;
>  
>  	return 0;
> @@ -514,7 +599,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
>  		return -ENOMEM;
>  
>  	panel->enabled = false;
> -	panel->prepared = false;
> +	panel->prepared_time = 0;
>  	panel->desc = desc;
>  
>  	panel->no_hpd = of_property_read_bool(dev->of_node, "no-hpd");
> -- 
> 2.29.1.341.ge80a0c044ae-goog
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel