All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/7] drm/panel: simple: Add mode support to devicetree
@ 2019-03-28 17:17 ` Douglas Anderson
  0 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: linux-rockchip, Laurent Pinchart, dri-devel, Boris Brezillon,
	Ezequiel Garcia, Enric Balletbò,
	Rob Herring, mka, Douglas Anderson, devicetree, Brian Norris,
	Klaus Goger, linux-kernel, David Airlie, Dmitry Torokhov,
	Mark Rutland, Viresh Kumar, linux-arm-kernel, Daniel Vetter

I'm reviving Sean Paul's old patchset to get mode support in device
tree.  The cover letter for his v3 is at:
https://lists.freedesktop.org/archives/dri-devel/2018-February/165162.html

I've pulled together the patches that didn't land in v3, addressed
outstanding feedback, and reposted.  Atop them I've added patches for
rk3288-veyron-jerry and rk3288-veryon-minnie.

Please let me know how they look.

In general I have added people to the whole series who I think would
like the whole series and then let get_maintainer pick extra people it
thinks are relevant to each individual patch.  If I see you respond to
any of the patches in the series, though, I'll add you to the whole
series Cc list next time.

Changes in v4:
- Simplify desc. for when override should be used (Thierry/Laurent)
- Removed Rob H review since it's been a year and wording changed
- Don't add mode from timing if override was specified (Thierry)
- Add warning if timing and fixed mode was specified (Thierry)
- Don't add fixed mode if timing was specified (Thierry)
- Refactor/rename a bit to avoid extra indentation from "if" tests
- i should be unsigned (Thierry)
- Add annoying WARN_ONs for some cases (Thierry)
- Simplify 'No display_timing found' handling (Thierry)
- Rename to panel_simple_parse_override_mode() (Thierry)
- Rebase to top of Heiko's tree
- Converted changelog to after-the-cut for non-DRM change.
- display_timing for Innolux n116bge new for v4.
- display_timing for AUO b101ean01 new for v4.
- rk3288-veyron-jerry patch new for v4.
- rk3288-veyron-minnie patch new for v4.

Changes in v3:
- Go back to using the timing subnode directly, but rename to
  panel-timing (Rob)
- No longer parse display-timings subnode, use panel-timing (Rob)
- Unwrap the timing from display-timings and rename panel-timing (Rob)

Changes in v2:
- Split out the binding into a new patch (Rob)
- display-timings is a new section (Rob)
- Use the full display-timings subnode instead of picking the timing
  out (Rob/Thierry)
- Parse the full display-timings node (using the native-mode) (Rob)
- Wrap the timing in display-timings node to match binding (Rob/Thierry)

Douglas Anderson (4):
  drm/panel: simple: Use display_timing for Innolux n116bge
  drm/panel: simple: Use display_timing for AUO b101ean01
  ARM: dts: rockchip: Specify rk3288-veyron-jerry's display timings
  ARM: dts: rockchip: Specify rk3288-veyron-minnie's display timings

Sean Paul (3):
  dt-bindings: Add panel-timing subnode to simple-panel
  drm/panel: simple: Add ability to override typical timing
  arm64: dts: rockchip: Specify override mode for kevin panel

 .../bindings/display/panel/simple-panel.txt   |  24 +++
 .../boot/dts/rk3288-veyron-chromebook.dtsi    |  14 ++
 arch/arm/boot/dts/rk3288-veyron-minnie.dts    |  14 ++
 .../boot/dts/rockchip/rk3399-gru-kevin.dts    |  14 ++
 drivers/gpu/drm/panel/panel-simple.c          | 171 ++++++++++++++----
 5 files changed, 205 insertions(+), 32 deletions(-)

-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v4 0/7] drm/panel: simple: Add mode support to devicetree
@ 2019-03-28 17:17 ` Douglas Anderson
  0 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: Mark Rutland, devicetree, Dmitry Torokhov, Rob Herring,
	David Airlie, Viresh Kumar, Brian Norris, Douglas Anderson,
	dri-devel, linux-kernel, linux-rockchip, Boris Brezillon,
	Klaus Goger, Laurent Pinchart, Enric Balletbò,
	Ezequiel Garcia, mka, linux-arm-kernel

I'm reviving Sean Paul's old patchset to get mode support in device
tree.  The cover letter for his v3 is at:
https://lists.freedesktop.org/archives/dri-devel/2018-February/165162.html

I've pulled together the patches that didn't land in v3, addressed
outstanding feedback, and reposted.  Atop them I've added patches for
rk3288-veyron-jerry and rk3288-veryon-minnie.

Please let me know how they look.

In general I have added people to the whole series who I think would
like the whole series and then let get_maintainer pick extra people it
thinks are relevant to each individual patch.  If I see you respond to
any of the patches in the series, though, I'll add you to the whole
series Cc list next time.

Changes in v4:
- Simplify desc. for when override should be used (Thierry/Laurent)
- Removed Rob H review since it's been a year and wording changed
- Don't add mode from timing if override was specified (Thierry)
- Add warning if timing and fixed mode was specified (Thierry)
- Don't add fixed mode if timing was specified (Thierry)
- Refactor/rename a bit to avoid extra indentation from "if" tests
- i should be unsigned (Thierry)
- Add annoying WARN_ONs for some cases (Thierry)
- Simplify 'No display_timing found' handling (Thierry)
- Rename to panel_simple_parse_override_mode() (Thierry)
- Rebase to top of Heiko's tree
- Converted changelog to after-the-cut for non-DRM change.
- display_timing for Innolux n116bge new for v4.
- display_timing for AUO b101ean01 new for v4.
- rk3288-veyron-jerry patch new for v4.
- rk3288-veyron-minnie patch new for v4.

Changes in v3:
- Go back to using the timing subnode directly, but rename to
  panel-timing (Rob)
- No longer parse display-timings subnode, use panel-timing (Rob)
- Unwrap the timing from display-timings and rename panel-timing (Rob)

Changes in v2:
- Split out the binding into a new patch (Rob)
- display-timings is a new section (Rob)
- Use the full display-timings subnode instead of picking the timing
  out (Rob/Thierry)
- Parse the full display-timings node (using the native-mode) (Rob)
- Wrap the timing in display-timings node to match binding (Rob/Thierry)

Douglas Anderson (4):
  drm/panel: simple: Use display_timing for Innolux n116bge
  drm/panel: simple: Use display_timing for AUO b101ean01
  ARM: dts: rockchip: Specify rk3288-veyron-jerry's display timings
  ARM: dts: rockchip: Specify rk3288-veyron-minnie's display timings

Sean Paul (3):
  dt-bindings: Add panel-timing subnode to simple-panel
  drm/panel: simple: Add ability to override typical timing
  arm64: dts: rockchip: Specify override mode for kevin panel

 .../bindings/display/panel/simple-panel.txt   |  24 +++
 .../boot/dts/rk3288-veyron-chromebook.dtsi    |  14 ++
 arch/arm/boot/dts/rk3288-veyron-minnie.dts    |  14 ++
 .../boot/dts/rockchip/rk3399-gru-kevin.dts    |  14 ++
 drivers/gpu/drm/panel/panel-simple.c          | 171 ++++++++++++++----
 5 files changed, 205 insertions(+), 32 deletions(-)

-- 
2.21.0.392.gf8f6787159e-goog

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v4 0/7] drm/panel: simple: Add mode support to devicetree
@ 2019-03-28 17:17 ` Douglas Anderson
  0 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: Mark Rutland, devicetree, Dmitry Torokhov, Rob Herring,
	David Airlie, Viresh Kumar, Brian Norris, Douglas Anderson,
	dri-devel, linux-kernel, linux-rockchip, Boris Brezillon,
	Klaus Goger, Laurent Pinchart, Daniel Vetter, Enric Balletbò,
	Ezequiel Garcia, mka, linux-arm-kernel

I'm reviving Sean Paul's old patchset to get mode support in device
tree.  The cover letter for his v3 is at:
https://lists.freedesktop.org/archives/dri-devel/2018-February/165162.html

I've pulled together the patches that didn't land in v3, addressed
outstanding feedback, and reposted.  Atop them I've added patches for
rk3288-veyron-jerry and rk3288-veryon-minnie.

Please let me know how they look.

In general I have added people to the whole series who I think would
like the whole series and then let get_maintainer pick extra people it
thinks are relevant to each individual patch.  If I see you respond to
any of the patches in the series, though, I'll add you to the whole
series Cc list next time.

Changes in v4:
- Simplify desc. for when override should be used (Thierry/Laurent)
- Removed Rob H review since it's been a year and wording changed
- Don't add mode from timing if override was specified (Thierry)
- Add warning if timing and fixed mode was specified (Thierry)
- Don't add fixed mode if timing was specified (Thierry)
- Refactor/rename a bit to avoid extra indentation from "if" tests
- i should be unsigned (Thierry)
- Add annoying WARN_ONs for some cases (Thierry)
- Simplify 'No display_timing found' handling (Thierry)
- Rename to panel_simple_parse_override_mode() (Thierry)
- Rebase to top of Heiko's tree
- Converted changelog to after-the-cut for non-DRM change.
- display_timing for Innolux n116bge new for v4.
- display_timing for AUO b101ean01 new for v4.
- rk3288-veyron-jerry patch new for v4.
- rk3288-veyron-minnie patch new for v4.

Changes in v3:
- Go back to using the timing subnode directly, but rename to
  panel-timing (Rob)
- No longer parse display-timings subnode, use panel-timing (Rob)
- Unwrap the timing from display-timings and rename panel-timing (Rob)

Changes in v2:
- Split out the binding into a new patch (Rob)
- display-timings is a new section (Rob)
- Use the full display-timings subnode instead of picking the timing
  out (Rob/Thierry)
- Parse the full display-timings node (using the native-mode) (Rob)
- Wrap the timing in display-timings node to match binding (Rob/Thierry)

Douglas Anderson (4):
  drm/panel: simple: Use display_timing for Innolux n116bge
  drm/panel: simple: Use display_timing for AUO b101ean01
  ARM: dts: rockchip: Specify rk3288-veyron-jerry's display timings
  ARM: dts: rockchip: Specify rk3288-veyron-minnie's display timings

Sean Paul (3):
  dt-bindings: Add panel-timing subnode to simple-panel
  drm/panel: simple: Add ability to override typical timing
  arm64: dts: rockchip: Specify override mode for kevin panel

 .../bindings/display/panel/simple-panel.txt   |  24 +++
 .../boot/dts/rk3288-veyron-chromebook.dtsi    |  14 ++
 arch/arm/boot/dts/rk3288-veyron-minnie.dts    |  14 ++
 .../boot/dts/rockchip/rk3399-gru-kevin.dts    |  14 ++
 drivers/gpu/drm/panel/panel-simple.c          | 171 ++++++++++++++----
 5 files changed, 205 insertions(+), 32 deletions(-)

-- 
2.21.0.392.gf8f6787159e-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel
  2019-03-28 17:17 ` Douglas Anderson
  (?)
  (?)
@ 2019-03-28 17:17 ` Douglas Anderson
  2019-03-28 20:26   ` Ezequiel Garcia
  -1 siblings, 1 reply; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: linux-rockchip, Laurent Pinchart, dri-devel, Boris Brezillon,
	Ezequiel Garcia, Enric Balletbò,
	Rob Herring, mka, Doug Anderson, Eric Anholt, Jeffy Chen,
	Stéphane Marchesin, devicetree, linux-kernel, David Airlie,
	Mark Rutland, Daniel Vetter

From: Sean Paul <seanpaul@chromium.org>

This patch adds a new subnode to simple-panel allowing us to override
the typical timing expressed in the panel's display_timing.

Changes in v2:
 - Split out the binding into a new patch (Rob)
 - display-timings is a new section (Rob)
 - Use the full display-timings subnode instead of picking the timing
   out (Rob/Thierry)
Changes in v3:
 - Go back to using the timing subnode directly, but rename to
   panel-timing (Rob)
Changes in v4:
 - Simplify desc. for when override should be used (Thierry/Laurent)
 - Removed Rob H review since it's been a year and wording changed

Cc: Doug Anderson <dianders@chromium.org>
Cc: Eric Anholt <eric@anholt.net>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Stéphane Marchesin <marcheu@chromium.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-rockchip@lists.infradead.org
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 .../bindings/display/panel/simple-panel.txt   | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/simple-panel.txt b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
index b2b872c710f2..6157f86ddce4 100644
--- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt
+++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
@@ -15,6 +15,18 @@ Optional properties:
   (hot plug detect) signal, but the signal isn't hooked up so we should
   hardcode the max delay from the panel spec when powering up the panel.
 
+panel-timing subnode
+--------------------
+
+This optional subnode is for devices which require a mode differing
+from the panel's "typical" display timing.  The panel timings provided
+here will be ignored if they are found to be outside of allowable
+ranges for the given panel.
+
+Format information on the panel-timing subnode can be found in
+display-timing.txt.
+
+
 Example:
 
 	panel: panel {
@@ -25,4 +37,16 @@ Example:
 		enable-gpios = <&gpio 90 0>;
 
 		backlight = <&backlight>;
+
+		panel-timing {
+			clock-frequency = <266604720>;
+			hactive = <2400>;
+			hfront-porch = <48>;
+			hback-porch = <84>;
+			hsync-len = <32>;
+			vactive = <1600>;
+			vfront-porch = <3>;
+			vback-porch = <120>;
+			vsync-len = <10>;
+		};
 	};
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 2/7] drm/panel: simple: Add ability to override typical timing
  2019-03-28 17:17 ` Douglas Anderson
@ 2019-03-28 17:17   ` Douglas Anderson
  -1 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: linux-rockchip, Laurent Pinchart, dri-devel, Boris Brezillon,
	Ezequiel Garcia, Enric Balletbò,
	Rob Herring, mka, Doug Anderson, Eric Anholt, Jeffy Chen,
	Stéphane Marchesin, devicetree, David Airlie, linux-kernel,
	Daniel Vetter

From: Sean Paul <seanpaul@chromium.org>

This patch adds the ability to override the typical display timing for a
given panel. This is useful for devices which have timing constraints
that do not apply across the entire display driver (eg: to avoid
crosstalk between panel and digitizer on certain laptops). The rules are
as follows:

- panel must not specify fixed mode (since the override mode will
  either be the same as the fixed mode, or we'll be unable to
  check the bounds of the overried)
- panel must specify at least one display_timing range which will be
  used to ensure the override mode fits within its bounds

Changes in v2:
 - Parse the full display-timings node (using the native-mode) (Rob)
Changes in v3:
 - No longer parse display-timings subnode, use panel-timing (Rob)
Changes in v4:
 - Don't add mode from timing if override was specified (Thierry)
 - Add warning if timing and fixed mode was specified (Thierry)
 - Don't add fixed mode if timing was specified (Thierry)
 - Refactor/rename a bit to avoid extra indentation from "if" tests
 - i should be unsigned (Thierry)
 - Add annoying WARN_ONs for some cases (Thierry)
 - Simplify 'No display_timing found' handling (Thierry)
 - Rename to panel_simple_parse_override_mode() (Thierry)

Cc: Doug Anderson <dianders@chromium.org>
Cc: Eric Anholt <eric@anholt.net>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Stéphane Marchesin <marcheu@chromium.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/gpu/drm/panel/panel-simple.c | 109 +++++++++++++++++++++++++--
 1 file changed, 104 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 9e8218f6a3f2..ad4f4aac2d44 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -34,6 +34,7 @@
 #include <drm/drm_panel.h>
 
 #include <video/display_timing.h>
+#include <video/of_display_timing.h>
 #include <video/videomode.h>
 
 struct panel_desc {
@@ -91,6 +92,8 @@ struct panel_simple {
 	struct i2c_adapter *ddc;
 
 	struct gpio_desc *enable_gpio;
+
+	struct drm_display_mode override_mode;
 };
 
 static inline struct panel_simple *to_panel_simple(struct drm_panel *panel)
@@ -98,16 +101,13 @@ static inline struct panel_simple *to_panel_simple(struct drm_panel *panel)
 	return container_of(panel, struct panel_simple, base);
 }
 
-static int panel_simple_get_fixed_modes(struct panel_simple *panel)
+static unsigned int panel_simple_get_timings_modes(struct panel_simple *panel)
 {
 	struct drm_connector *connector = panel->base.connector;
 	struct drm_device *drm = panel->base.drm;
 	struct drm_display_mode *mode;
 	unsigned int i, num = 0;
 
-	if (!panel->desc)
-		return 0;
-
 	for (i = 0; i < panel->desc->num_timings; i++) {
 		const struct display_timing *dt = &panel->desc->timings[i];
 		struct videomode vm;
@@ -131,6 +131,16 @@ static int panel_simple_get_fixed_modes(struct panel_simple *panel)
 		num++;
 	}
 
+	return num;
+}
+
+static unsigned int panel_simple_get_fixed_modes(struct panel_simple *panel)
+{
+	struct drm_connector *connector = panel->base.connector;
+	struct drm_device *drm = panel->base.drm;
+	struct drm_display_mode *mode;
+	unsigned int i, num = 0;
+
 	for (i = 0; i < panel->desc->num_modes; i++) {
 		const struct drm_display_mode *m = &panel->desc->modes[i];
 
@@ -152,6 +162,44 @@ static int panel_simple_get_fixed_modes(struct panel_simple *panel)
 		num++;
 	}
 
+	return num;
+}
+
+static int panel_simple_get_non_edid_modes(struct panel_simple *panel)
+{
+	struct drm_connector *connector = panel->base.connector;
+	struct drm_device *drm = panel->base.drm;
+	struct drm_display_mode *mode;
+	bool has_override = panel->override_mode.type;
+	unsigned int num = 0;
+
+	if (!panel->desc)
+		return 0;
+
+	if (has_override) {
+		mode = drm_mode_duplicate(drm, &panel->override_mode);
+		if (mode) {
+			drm_mode_probed_add(connector, mode);
+			num = 1;
+		} else {
+			dev_err(drm->dev, "failed to add override mode\n");
+		}
+	}
+
+	/* Only add timings if override was not there or failed to validate */
+	if (num == 0 && panel->desc->num_timings)
+		num = panel_simple_get_timings_modes(panel);
+
+	/*
+	 * Only add fixed modes if timings/override added no mode.
+	 *
+	 * We should only ever have either the display timings specified
+	 * or a fixed mode. Anything else is rather bogus.
+	 */
+	WARN_ON(panel->desc->num_timings && panel->desc->num_modes);
+	if (num == 0)
+		num = panel_simple_get_fixed_modes(panel);
+
 	connector->display_info.bpc = panel->desc->bpc;
 	connector->display_info.width_mm = panel->desc->size.width;
 	connector->display_info.height_mm = panel->desc->size.height;
@@ -268,7 +316,7 @@ static int panel_simple_get_modes(struct drm_panel *panel)
 	}
 
 	/* add hard-coded panel modes */
-	num += panel_simple_get_fixed_modes(p);
+	num += panel_simple_get_non_edid_modes(p);
 
 	return num;
 }
@@ -299,10 +347,58 @@ static const struct drm_panel_funcs panel_simple_funcs = {
 	.get_timings = panel_simple_get_timings,
 };
 
+#define PANEL_SIMPLE_BOUNDS_CHECK(to_check, bounds, field) \
+	(to_check->field.typ >= bounds->field.min && \
+	 to_check->field.typ <= bounds->field.max)
+static void panel_simple_parse_override_mode(struct device *dev,
+					     struct panel_simple *panel,
+					     const struct display_timing *ot)
+{
+	const struct panel_desc *desc = panel->desc;
+	struct videomode vm;
+	unsigned int i;
+
+	if (WARN_ON(desc->num_modes)) {
+		dev_err(dev, "Reject override mode: panel has a fixed mode\n");
+		return;
+	}
+	if (WARN_ON(!desc->num_timings)) {
+		dev_err(dev, "Reject override mode: no timings specified\n");
+		return;
+	}
+
+	for (i = 0; i < panel->desc->num_timings; i++) {
+		const struct display_timing *dt = &panel->desc->timings[i];
+
+		if (!PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hactive) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hfront_porch) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hback_porch) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hsync_len) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vactive) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vfront_porch) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vback_porch) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vsync_len))
+			continue;
+
+		if (ot->flags != dt->flags)
+			continue;
+
+		videomode_from_timing(ot, &vm);
+		drm_display_mode_from_videomode(&vm, &panel->override_mode);
+		panel->override_mode.type |= DRM_MODE_TYPE_DRIVER |
+					     DRM_MODE_TYPE_PREFERRED;
+		break;
+	}
+
+	if (WARN_ON(!panel->override_mode.type))
+		dev_err(dev, "Reject override mode: No display_timing found\n");
+}
+
 static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 {
 	struct device_node *backlight, *ddc;
 	struct panel_simple *panel;
+	struct display_timing dt;
 	int err;
 
 	panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL);
@@ -348,6 +444,9 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 		}
 	}
 
+	if (!of_get_display_timing(dev->of_node, "panel-timing", &dt))
+		panel_simple_parse_override_mode(dev, panel, &dt);
+
 	drm_panel_init(&panel->base);
 	panel->base.dev = dev;
 	panel->base.funcs = &panel_simple_funcs;
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 2/7] drm/panel: simple: Add ability to override typical timing
@ 2019-03-28 17:17   ` Douglas Anderson
  0 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: devicetree, Rob Herring, David Airlie, Jeffy Chen, Doug Anderson,
	dri-devel, linux-kernel, linux-rockchip, Boris Brezillon,
	Laurent Pinchart, Enric Balletbò,
	Stéphane Marchesin, Ezequiel Garcia, mka

From: Sean Paul <seanpaul@chromium.org>

This patch adds the ability to override the typical display timing for a
given panel. This is useful for devices which have timing constraints
that do not apply across the entire display driver (eg: to avoid
crosstalk between panel and digitizer on certain laptops). The rules are
as follows:

- panel must not specify fixed mode (since the override mode will
  either be the same as the fixed mode, or we'll be unable to
  check the bounds of the overried)
- panel must specify at least one display_timing range which will be
  used to ensure the override mode fits within its bounds

Changes in v2:
 - Parse the full display-timings node (using the native-mode) (Rob)
Changes in v3:
 - No longer parse display-timings subnode, use panel-timing (Rob)
Changes in v4:
 - Don't add mode from timing if override was specified (Thierry)
 - Add warning if timing and fixed mode was specified (Thierry)
 - Don't add fixed mode if timing was specified (Thierry)
 - Refactor/rename a bit to avoid extra indentation from "if" tests
 - i should be unsigned (Thierry)
 - Add annoying WARN_ONs for some cases (Thierry)
 - Simplify 'No display_timing found' handling (Thierry)
 - Rename to panel_simple_parse_override_mode() (Thierry)

Cc: Doug Anderson <dianders@chromium.org>
Cc: Eric Anholt <eric@anholt.net>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Stéphane Marchesin <marcheu@chromium.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/gpu/drm/panel/panel-simple.c | 109 +++++++++++++++++++++++++--
 1 file changed, 104 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 9e8218f6a3f2..ad4f4aac2d44 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -34,6 +34,7 @@
 #include <drm/drm_panel.h>
 
 #include <video/display_timing.h>
+#include <video/of_display_timing.h>
 #include <video/videomode.h>
 
 struct panel_desc {
@@ -91,6 +92,8 @@ struct panel_simple {
 	struct i2c_adapter *ddc;
 
 	struct gpio_desc *enable_gpio;
+
+	struct drm_display_mode override_mode;
 };
 
 static inline struct panel_simple *to_panel_simple(struct drm_panel *panel)
@@ -98,16 +101,13 @@ static inline struct panel_simple *to_panel_simple(struct drm_panel *panel)
 	return container_of(panel, struct panel_simple, base);
 }
 
-static int panel_simple_get_fixed_modes(struct panel_simple *panel)
+static unsigned int panel_simple_get_timings_modes(struct panel_simple *panel)
 {
 	struct drm_connector *connector = panel->base.connector;
 	struct drm_device *drm = panel->base.drm;
 	struct drm_display_mode *mode;
 	unsigned int i, num = 0;
 
-	if (!panel->desc)
-		return 0;
-
 	for (i = 0; i < panel->desc->num_timings; i++) {
 		const struct display_timing *dt = &panel->desc->timings[i];
 		struct videomode vm;
@@ -131,6 +131,16 @@ static int panel_simple_get_fixed_modes(struct panel_simple *panel)
 		num++;
 	}
 
+	return num;
+}
+
+static unsigned int panel_simple_get_fixed_modes(struct panel_simple *panel)
+{
+	struct drm_connector *connector = panel->base.connector;
+	struct drm_device *drm = panel->base.drm;
+	struct drm_display_mode *mode;
+	unsigned int i, num = 0;
+
 	for (i = 0; i < panel->desc->num_modes; i++) {
 		const struct drm_display_mode *m = &panel->desc->modes[i];
 
@@ -152,6 +162,44 @@ static int panel_simple_get_fixed_modes(struct panel_simple *panel)
 		num++;
 	}
 
+	return num;
+}
+
+static int panel_simple_get_non_edid_modes(struct panel_simple *panel)
+{
+	struct drm_connector *connector = panel->base.connector;
+	struct drm_device *drm = panel->base.drm;
+	struct drm_display_mode *mode;
+	bool has_override = panel->override_mode.type;
+	unsigned int num = 0;
+
+	if (!panel->desc)
+		return 0;
+
+	if (has_override) {
+		mode = drm_mode_duplicate(drm, &panel->override_mode);
+		if (mode) {
+			drm_mode_probed_add(connector, mode);
+			num = 1;
+		} else {
+			dev_err(drm->dev, "failed to add override mode\n");
+		}
+	}
+
+	/* Only add timings if override was not there or failed to validate */
+	if (num == 0 && panel->desc->num_timings)
+		num = panel_simple_get_timings_modes(panel);
+
+	/*
+	 * Only add fixed modes if timings/override added no mode.
+	 *
+	 * We should only ever have either the display timings specified
+	 * or a fixed mode. Anything else is rather bogus.
+	 */
+	WARN_ON(panel->desc->num_timings && panel->desc->num_modes);
+	if (num == 0)
+		num = panel_simple_get_fixed_modes(panel);
+
 	connector->display_info.bpc = panel->desc->bpc;
 	connector->display_info.width_mm = panel->desc->size.width;
 	connector->display_info.height_mm = panel->desc->size.height;
@@ -268,7 +316,7 @@ static int panel_simple_get_modes(struct drm_panel *panel)
 	}
 
 	/* add hard-coded panel modes */
-	num += panel_simple_get_fixed_modes(p);
+	num += panel_simple_get_non_edid_modes(p);
 
 	return num;
 }
@@ -299,10 +347,58 @@ static const struct drm_panel_funcs panel_simple_funcs = {
 	.get_timings = panel_simple_get_timings,
 };
 
+#define PANEL_SIMPLE_BOUNDS_CHECK(to_check, bounds, field) \
+	(to_check->field.typ >= bounds->field.min && \
+	 to_check->field.typ <= bounds->field.max)
+static void panel_simple_parse_override_mode(struct device *dev,
+					     struct panel_simple *panel,
+					     const struct display_timing *ot)
+{
+	const struct panel_desc *desc = panel->desc;
+	struct videomode vm;
+	unsigned int i;
+
+	if (WARN_ON(desc->num_modes)) {
+		dev_err(dev, "Reject override mode: panel has a fixed mode\n");
+		return;
+	}
+	if (WARN_ON(!desc->num_timings)) {
+		dev_err(dev, "Reject override mode: no timings specified\n");
+		return;
+	}
+
+	for (i = 0; i < panel->desc->num_timings; i++) {
+		const struct display_timing *dt = &panel->desc->timings[i];
+
+		if (!PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hactive) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hfront_porch) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hback_porch) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, hsync_len) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vactive) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vfront_porch) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vback_porch) ||
+		    !PANEL_SIMPLE_BOUNDS_CHECK(ot, dt, vsync_len))
+			continue;
+
+		if (ot->flags != dt->flags)
+			continue;
+
+		videomode_from_timing(ot, &vm);
+		drm_display_mode_from_videomode(&vm, &panel->override_mode);
+		panel->override_mode.type |= DRM_MODE_TYPE_DRIVER |
+					     DRM_MODE_TYPE_PREFERRED;
+		break;
+	}
+
+	if (WARN_ON(!panel->override_mode.type))
+		dev_err(dev, "Reject override mode: No display_timing found\n");
+}
+
 static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 {
 	struct device_node *backlight, *ddc;
 	struct panel_simple *panel;
+	struct display_timing dt;
 	int err;
 
 	panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL);
@@ -348,6 +444,9 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
 		}
 	}
 
+	if (!of_get_display_timing(dev->of_node, "panel-timing", &dt))
+		panel_simple_parse_override_mode(dev, panel, &dt);
+
 	drm_panel_init(&panel->base);
 	panel->base.dev = dev;
 	panel->base.funcs = &panel_simple_funcs;
-- 
2.21.0.392.gf8f6787159e-goog

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 3/7] arm64: dts: rockchip: Specify override mode for kevin panel
  2019-03-28 17:17 ` Douglas Anderson
  (?)
@ 2019-03-28 17:17   ` Douglas Anderson
  -1 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: linux-rockchip, Laurent Pinchart, dri-devel, Boris Brezillon,
	Ezequiel Garcia, Enric Balletbò,
	Rob Herring, mka, Doug Anderson, Eric Anholt, Jeffy Chen,
	Stéphane Marchesin, devicetree, Brian Norris, Klaus Goger,
	linux-kernel, Dmitry Torokhov, Mark Rutland, Viresh Kumar,
	linux-arm-kernel

From: Sean Paul <seanpaul@chromium.org>

This patch adds an override mode for kevin devices. The mode increases
both back porches to allow a pixel clock of 26666kHz as opposed to the
'typical' value of 252750kHz. This is needed to avoid interference with
the touch digitizer on these laptops.

Cc: Doug Anderson <dianders@chromium.org>
Cc: Eric Anholt <eric@anholt.net>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Stéphane Marchesin <marcheu@chromium.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-rockchip@lists.infradead.org
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v4:
- Rebase to top of Heiko's tree
- Converted changelog to after-the-cut for non-DRM change.

Changes in v3:
- Unwrap the timing from display-timings and rename panel-timing (Rob)

Changes in v2:
- Wrap the timing in display-timings node to match binding (Rob/Thierry)

 arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
index 15e254a77391..454f4149585f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
@@ -43,6 +43,20 @@
 		backlight = <&backlight>;
 		power-supply = <&pp3300_disp>;
 
+		panel-timing {
+			clock-frequency = <266604720>;
+			hactive = <2400>;
+			hfront-porch = <48>;
+			hback-porch = <84>;
+			hsync-len = <32>;
+			hsync-active = <0>;
+			vactive = <1600>;
+			vfront-porch = <3>;
+			vback-porch = <120>;
+			vsync-len = <10>;
+			vsync-active = <0>;
+		};
+
 		port {
 			panel_in_edp: endpoint {
 				remote-endpoint = <&edp_out_panel>;
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 3/7] arm64: dts: rockchip: Specify override mode for kevin panel
@ 2019-03-28 17:17   ` Douglas Anderson
  0 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: linux-rockchip, Laurent Pinchart, dri-devel, Boris Brezillon,
	Ezequiel Garcia, Enric Balletbò,
	Rob Herring, mka, Doug Anderson, Eric Anholt, Jeffy Chen,
	Stéphane Marchesin, devicetree, Brian Norris, Klaus Goger,
	linux-kernel, Dmitry Torokhov, Mark Rutland

From: Sean Paul <seanpaul@chromium.org>

This patch adds an override mode for kevin devices. The mode increases
both back porches to allow a pixel clock of 26666kHz as opposed to the
'typical' value of 252750kHz. This is needed to avoid interference with
the touch digitizer on these laptops.

Cc: Doug Anderson <dianders@chromium.org>
Cc: Eric Anholt <eric@anholt.net>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Stéphane Marchesin <marcheu@chromium.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-rockchip@lists.infradead.org
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v4:
- Rebase to top of Heiko's tree
- Converted changelog to after-the-cut for non-DRM change.

Changes in v3:
- Unwrap the timing from display-timings and rename panel-timing (Rob)

Changes in v2:
- Wrap the timing in display-timings node to match binding (Rob/Thierry)

 arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
index 15e254a77391..454f4149585f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
@@ -43,6 +43,20 @@
 		backlight = <&backlight>;
 		power-supply = <&pp3300_disp>;
 
+		panel-timing {
+			clock-frequency = <266604720>;
+			hactive = <2400>;
+			hfront-porch = <48>;
+			hback-porch = <84>;
+			hsync-len = <32>;
+			hsync-active = <0>;
+			vactive = <1600>;
+			vfront-porch = <3>;
+			vback-porch = <120>;
+			vsync-len = <10>;
+			vsync-active = <0>;
+		};
+
 		port {
 			panel_in_edp: endpoint {
 				remote-endpoint = <&edp_out_panel>;
-- 
2.21.0.392.gf8f6787159e-goog

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 3/7] arm64: dts: rockchip: Specify override mode for kevin panel
@ 2019-03-28 17:17   ` Douglas Anderson
  0 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: Mark Rutland, devicetree, Brian Norris, Rob Herring, Eric Anholt,
	Viresh Kumar, Jeffy Chen, Doug Anderson, dri-devel, linux-kernel,
	linux-rockchip, Boris Brezillon, Klaus Goger, Laurent Pinchart,
	Enric Balletbò,
	Stéphane Marchesin, Dmitry Torokhov, Ezequiel Garcia, mka,
	linux-arm-kernel

From: Sean Paul <seanpaul@chromium.org>

This patch adds an override mode for kevin devices. The mode increases
both back porches to allow a pixel clock of 26666kHz as opposed to the
'typical' value of 252750kHz. This is needed to avoid interference with
the touch digitizer on these laptops.

Cc: Doug Anderson <dianders@chromium.org>
Cc: Eric Anholt <eric@anholt.net>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Stéphane Marchesin <marcheu@chromium.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-rockchip@lists.infradead.org
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v4:
- Rebase to top of Heiko's tree
- Converted changelog to after-the-cut for non-DRM change.

Changes in v3:
- Unwrap the timing from display-timings and rename panel-timing (Rob)

Changes in v2:
- Wrap the timing in display-timings node to match binding (Rob/Thierry)

 arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
index 15e254a77391..454f4149585f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
@@ -43,6 +43,20 @@
 		backlight = <&backlight>;
 		power-supply = <&pp3300_disp>;
 
+		panel-timing {
+			clock-frequency = <266604720>;
+			hactive = <2400>;
+			hfront-porch = <48>;
+			hback-porch = <84>;
+			hsync-len = <32>;
+			hsync-active = <0>;
+			vactive = <1600>;
+			vfront-porch = <3>;
+			vback-porch = <120>;
+			vsync-len = <10>;
+			vsync-active = <0>;
+		};
+
 		port {
 			panel_in_edp: endpoint {
 				remote-endpoint = <&edp_out_panel>;
-- 
2.21.0.392.gf8f6787159e-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 4/7] drm/panel: simple: Use display_timing for Innolux n116bge
  2019-03-28 17:17 ` Douglas Anderson
@ 2019-03-28 17:17   ` Douglas Anderson
  -1 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: linux-rockchip, Laurent Pinchart, dri-devel, Boris Brezillon,
	Ezequiel Garcia, Enric Balletbò,
	Rob Herring, mka, Douglas Anderson, David Airlie, linux-kernel,
	Daniel Vetter

Convert the Innolux n116bge from using a fixed mode to specifying a
display timing with min/typ/max values.

Note that the n116bge's datasheet doesn't fit too well into DRM's way
of specifying things.  Specifically the panel's datasheet just
specifies the vertical blanking period and horizontal blanking period
and doesn't break things out.  For now we'll leave everything as a
fixed value but just allow adjusting the pixel clock.  I've added a
comment on what the datasheet claims so someone could later expand
things to fit their needs if they wanted to test other blanking
periods.

The goal here is to be able to specify the panel timings in the device
tree for several rk3288 Chromebooks (like rk3288-veryon-jerry).  These
Chromebooks have all been running in the downstream kernel with the
standard porches and sync lengths but just with a slightly slower
pixel clock because the 76.42 MHz clock is not achievable from the
fixed PLL that was available.  These Chromebooks only achieve a
refresh rate of ~58 Hz.  While it's probable that we could adjust the
timings to achieve 60 Hz it's probably wisest to match what's been
running on these devices all these years.

I'll note that though the upstream kernel has always tried to achieve
76.42 MHz, it has actually been running at 74.25 MHz also since the
video processor is parented off the same fixed PLL.

Changes in v4:
 - display_timing for Innolux n116bge new for v4.

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

 drivers/gpu/drm/panel/panel-simple.c | 37 +++++++++++++++++-----------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index ad4f4aac2d44..7d407fab3628 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1550,23 +1550,32 @@ static const struct panel_desc innolux_g121x1_l03 = {
 	},
 };
 
-static const struct drm_display_mode innolux_n116bge_mode = {
-	.clock = 76420,
-	.hdisplay = 1366,
-	.hsync_start = 1366 + 136,
-	.hsync_end = 1366 + 136 + 30,
-	.htotal = 1366 + 136 + 30 + 60,
-	.vdisplay = 768,
-	.vsync_start = 768 + 8,
-	.vsync_end = 768 + 8 + 12,
-	.vtotal = 768 + 8 + 12 + 12,
-	.vrefresh = 60,
-	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+/*
+ * Datasheet specifies that at 60 Hz refresh rate:
+ * - total horizontal time: { 1506, 1592, 1716 }
+ * - total vertical time: { 788, 800, 868 }
+ *
+ * ...but doesn't go into exactly how that should be split into a front
+ * porch, back porch, or sync length.  For now we'll leave a single setting
+ * here which allows a bit of tweaking of the pixel clock at the expense of
+ * refresh rate.
+ */
+static const struct display_timing innolux_n116bge_timing = {
+	.pixelclock = { 72600000, 76420000, 80240000 },
+	.hactive = { 1366, 1366, 1366 },
+	.hfront_porch = { 136, 136, 136 },
+	.hback_porch = { 60, 60, 60 },
+	.hsync_len = { 30, 30, 30 },
+	.vactive = { 768, 768, 768 },
+	.vfront_porch = { 8, 8, 8 },
+	.vback_porch = { 12, 12, 12 },
+	.vsync_len = { 12, 12, 12 },
+	.flags = DISPLAY_FLAGS_VSYNC_LOW | DISPLAY_FLAGS_HSYNC_LOW,
 };
 
 static const struct panel_desc innolux_n116bge = {
-	.modes = &innolux_n116bge_mode,
-	.num_modes = 1,
+	.timings = &innolux_n116bge_timing,
+	.num_timings = 1,
 	.bpc = 6,
 	.size = {
 		.width = 256,
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 4/7] drm/panel: simple: Use display_timing for Innolux n116bge
@ 2019-03-28 17:17   ` Douglas Anderson
  0 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: Rob Herring, David Airlie, Douglas Anderson, dri-devel,
	linux-kernel, linux-rockchip, Boris Brezillon, Laurent Pinchart,
	Enric Balletbò,
	Ezequiel Garcia, mka

Convert the Innolux n116bge from using a fixed mode to specifying a
display timing with min/typ/max values.

Note that the n116bge's datasheet doesn't fit too well into DRM's way
of specifying things.  Specifically the panel's datasheet just
specifies the vertical blanking period and horizontal blanking period
and doesn't break things out.  For now we'll leave everything as a
fixed value but just allow adjusting the pixel clock.  I've added a
comment on what the datasheet claims so someone could later expand
things to fit their needs if they wanted to test other blanking
periods.

The goal here is to be able to specify the panel timings in the device
tree for several rk3288 Chromebooks (like rk3288-veryon-jerry).  These
Chromebooks have all been running in the downstream kernel with the
standard porches and sync lengths but just with a slightly slower
pixel clock because the 76.42 MHz clock is not achievable from the
fixed PLL that was available.  These Chromebooks only achieve a
refresh rate of ~58 Hz.  While it's probable that we could adjust the
timings to achieve 60 Hz it's probably wisest to match what's been
running on these devices all these years.

I'll note that though the upstream kernel has always tried to achieve
76.42 MHz, it has actually been running at 74.25 MHz also since the
video processor is parented off the same fixed PLL.

Changes in v4:
 - display_timing for Innolux n116bge new for v4.

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

 drivers/gpu/drm/panel/panel-simple.c | 37 +++++++++++++++++-----------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index ad4f4aac2d44..7d407fab3628 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1550,23 +1550,32 @@ static const struct panel_desc innolux_g121x1_l03 = {
 	},
 };
 
-static const struct drm_display_mode innolux_n116bge_mode = {
-	.clock = 76420,
-	.hdisplay = 1366,
-	.hsync_start = 1366 + 136,
-	.hsync_end = 1366 + 136 + 30,
-	.htotal = 1366 + 136 + 30 + 60,
-	.vdisplay = 768,
-	.vsync_start = 768 + 8,
-	.vsync_end = 768 + 8 + 12,
-	.vtotal = 768 + 8 + 12 + 12,
-	.vrefresh = 60,
-	.flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+/*
+ * Datasheet specifies that at 60 Hz refresh rate:
+ * - total horizontal time: { 1506, 1592, 1716 }
+ * - total vertical time: { 788, 800, 868 }
+ *
+ * ...but doesn't go into exactly how that should be split into a front
+ * porch, back porch, or sync length.  For now we'll leave a single setting
+ * here which allows a bit of tweaking of the pixel clock at the expense of
+ * refresh rate.
+ */
+static const struct display_timing innolux_n116bge_timing = {
+	.pixelclock = { 72600000, 76420000, 80240000 },
+	.hactive = { 1366, 1366, 1366 },
+	.hfront_porch = { 136, 136, 136 },
+	.hback_porch = { 60, 60, 60 },
+	.hsync_len = { 30, 30, 30 },
+	.vactive = { 768, 768, 768 },
+	.vfront_porch = { 8, 8, 8 },
+	.vback_porch = { 12, 12, 12 },
+	.vsync_len = { 12, 12, 12 },
+	.flags = DISPLAY_FLAGS_VSYNC_LOW | DISPLAY_FLAGS_HSYNC_LOW,
 };
 
 static const struct panel_desc innolux_n116bge = {
-	.modes = &innolux_n116bge_mode,
-	.num_modes = 1,
+	.timings = &innolux_n116bge_timing,
+	.num_timings = 1,
 	.bpc = 6,
 	.size = {
 		.width = 256,
-- 
2.21.0.392.gf8f6787159e-goog

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

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 5/7] drm/panel: simple: Use display_timing for AUO b101ean01
  2019-03-28 17:17 ` Douglas Anderson
                   ` (5 preceding siblings ...)
  (?)
@ 2019-03-28 17:17 ` Douglas Anderson
  -1 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: linux-rockchip, Laurent Pinchart, dri-devel, Boris Brezillon,
	Ezequiel Garcia, Enric Balletbò,
	Rob Herring, mka, Douglas Anderson, David Airlie, linux-kernel,
	Daniel Vetter

Convert the AUO b101ean01 from using a fixed mode to specifying a
display timing with min/typ/max values.

The AUO b101ean01's datasheet says:
* Vertical blanking min is 12
* Horizontal blanking min is 60
* Pixel clock is between 65.3 MHz and 75 MHz

The goal here is to be able to specify the proper timing in device
tree to use on rk3288-veyron-minnie to match what the downstream
kernel is using so that it can used the fixed PLL.

Changes in v4:
 - display_timing for AUO b101ean01 new for v4.

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

 drivers/gpu/drm/panel/panel-simple.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 7d407fab3628..c6c0625e1684 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -568,22 +568,21 @@ static const struct panel_desc auo_b101aw03 = {
 	},
 };
 
-static const struct drm_display_mode auo_b101ean01_mode = {
-	.clock = 72500,
-	.hdisplay = 1280,
-	.hsync_start = 1280 + 119,
-	.hsync_end = 1280 + 119 + 32,
-	.htotal = 1280 + 119 + 32 + 21,
-	.vdisplay = 800,
-	.vsync_start = 800 + 4,
-	.vsync_end = 800 + 4 + 20,
-	.vtotal = 800 + 4 + 20 + 8,
-	.vrefresh = 60,
+static const struct display_timing auo_b101ean01_timing = {
+	.pixelclock = { 65300000, 72500000, 75000000 },
+	.hactive = { 1280, 1280, 1280 },
+	.hfront_porch = { 18, 119, 119 },
+	.hback_porch = { 21, 21, 21 },
+	.hsync_len = { 32, 32, 32 },
+	.vactive = { 800, 800, 800 },
+	.vfront_porch = { 4, 4, 4 },
+	.vback_porch = { 8, 8, 8 },
+	.vsync_len = { 18, 20, 20 },
 };
 
 static const struct panel_desc auo_b101ean01 = {
-	.modes = &auo_b101ean01_mode,
-	.num_modes = 1,
+	.timings = &auo_b101ean01_timing,
+	.num_timings = 1,
 	.bpc = 6,
 	.size = {
 		.width = 217,
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 6/7] ARM: dts: rockchip: Specify rk3288-veyron-jerry's display timings
  2019-03-28 17:17 ` Douglas Anderson
@ 2019-03-28 17:17   ` Douglas Anderson
  -1 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: linux-rockchip, Laurent Pinchart, dri-devel, Boris Brezillon,
	Ezequiel Garcia, Enric Balletbò,
	Rob Herring, mka, Douglas Anderson, devicetree, linux-kernel,
	Mark Rutland, linux-arm-kernel

Let's document the display timings that jerry has been using out in
the field.  This uses the standard blankings but a slightly slower
clock rate, thus getting a refresh rate 58.3 Hz.

NOTE: this won't really do anything except cause DRM to properly
report the refresh rate since vop_crtc_mode_fixup() was rounding the
pixel clock to 74.25 MHz anyway.  Apparently the adjusted rate isn't
exposed to userspace so it's important that the rate we're trying to
achieve is mostly right.

For the downstream kernel change related to this see See
https://crrev.com/c/324558.

NOTE: minnie will be fixed up in a future patch, so for now we'll just
delete the panel timings there.

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

Changes in v4:
- rk3288-veyron-jerry patch new for v4.

Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi | 14 ++++++++++++++
 arch/arm/boot/dts/rk3288-veyron-minnie.dts      |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
index b54746df3661..0b1789b50c21 100644
--- a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
@@ -76,6 +76,20 @@
 		power-supply = <&vcc33_lcd>;
 		backlight = <&backlight>;
 
+		panel-timing {
+			clock-frequency = <74250000>;
+			hactive = <1366>;
+			hfront-porch = <136>;
+			hback-porch = <60>;
+			hsync-len = <30>;
+			hsync-active = <0>;
+			vactive = <768>;
+			vfront-porch = <8>;
+			vback-porch = <12>;
+			vsync-len = <12>;
+			vsync-active = <0>;
+		};
+
 		ports {
 			panel_in: port {
 				panel_in_edp: endpoint {
diff --git a/arch/arm/boot/dts/rk3288-veyron-minnie.dts b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
index f95d0c5fcf71..ca7512ade222 100644
--- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
@@ -142,6 +142,8 @@
 &panel {
 	compatible = "auo,b101ean01", "simple-panel";
 	power-supply= <&panel_regulator>;
+
+	/delete-node/ panel-timing;
 };
 
 &rk808 {
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 6/7] ARM: dts: rockchip: Specify rk3288-veyron-jerry's display timings
@ 2019-03-28 17:17   ` Douglas Anderson
  0 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: Mark Rutland, devicetree, Rob Herring, Douglas Anderson,
	dri-devel, linux-kernel, linux-rockchip, Boris Brezillon,
	Laurent Pinchart, Enric Balletbò,
	Ezequiel Garcia, mka, linux-arm-kernel

Let's document the display timings that jerry has been using out in
the field.  This uses the standard blankings but a slightly slower
clock rate, thus getting a refresh rate 58.3 Hz.

NOTE: this won't really do anything except cause DRM to properly
report the refresh rate since vop_crtc_mode_fixup() was rounding the
pixel clock to 74.25 MHz anyway.  Apparently the adjusted rate isn't
exposed to userspace so it's important that the rate we're trying to
achieve is mostly right.

For the downstream kernel change related to this see See
https://crrev.com/c/324558.

NOTE: minnie will be fixed up in a future patch, so for now we'll just
delete the panel timings there.

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

Changes in v4:
- rk3288-veyron-jerry patch new for v4.

Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi | 14 ++++++++++++++
 arch/arm/boot/dts/rk3288-veyron-minnie.dts      |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
index b54746df3661..0b1789b50c21 100644
--- a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
@@ -76,6 +76,20 @@
 		power-supply = <&vcc33_lcd>;
 		backlight = <&backlight>;
 
+		panel-timing {
+			clock-frequency = <74250000>;
+			hactive = <1366>;
+			hfront-porch = <136>;
+			hback-porch = <60>;
+			hsync-len = <30>;
+			hsync-active = <0>;
+			vactive = <768>;
+			vfront-porch = <8>;
+			vback-porch = <12>;
+			vsync-len = <12>;
+			vsync-active = <0>;
+		};
+
 		ports {
 			panel_in: port {
 				panel_in_edp: endpoint {
diff --git a/arch/arm/boot/dts/rk3288-veyron-minnie.dts b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
index f95d0c5fcf71..ca7512ade222 100644
--- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
@@ -142,6 +142,8 @@
 &panel {
 	compatible = "auo,b101ean01", "simple-panel";
 	power-supply= <&panel_regulator>;
+
+	/delete-node/ panel-timing;
 };
 
 &rk808 {
-- 
2.21.0.392.gf8f6787159e-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 7/7] ARM: dts: rockchip: Specify rk3288-veyron-minnie's display timings
  2019-03-28 17:17 ` Douglas Anderson
@ 2019-03-28 17:17   ` Douglas Anderson
  -1 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: linux-rockchip, Laurent Pinchart, dri-devel, Boris Brezillon,
	Ezequiel Garcia, Enric Balletbò,
	Rob Herring, mka, Douglas Anderson, devicetree, linux-kernel,
	Mark Rutland, linux-arm-kernel

Just like rk3288-veyron-jerry, we want to be able to use one of the
fixed PLLs in the system to make the pixel clock for minnie.

Specifying these timings matches us with how the display is used on
the downstream Chrome OS kernel.  See https://crrev.com/c/323211.

Unlike jerry, this CL actually changes the timings (though not the
pixel clock) that is used when using the upstream kernel.  Booting up
a minnie shows that it ended up with a 66.67 MHz pixel clock but it
was still using the porches/blankings it would have wanted for a 72.5
MHz pixel clock.

NOTE: compared to the downstream kernel, this seems to cause a
slightly different result reported in the 'modetest' command on a
Chromebook.  The downstream kernel shows:
  1280x800 60 1280 1298 1330 1351 800 804 822 830 66667

With this patch we have:
  1280x800 59 1280 1298 1330 1351 800 804 822 830 66666

Specifically modetest was reporting 60 Hz on the downstream kernel but
the upstream kernel does the math and comesup with 59 (because we
actually achieve 59.45 Hz).  Also upstream doesn't round the Hz up
when converting to kHz--it seems to truncate.

ALSO NOTE: when I look at the EDID from the datasheet, I see:
  -hsync -vsync
...but it seems like we've never actually run with that so I've
continued leaving that out.

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

Changes in v4:
- rk3288-veyron-minnie patch new for v4.

Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/rk3288-veyron-minnie.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-veyron-minnie.dts b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
index ca7512ade222..8179cf9f6e98 100644
--- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
@@ -144,6 +144,18 @@
 	power-supply= <&panel_regulator>;
 
 	/delete-node/ panel-timing;
+
+	panel-timing {
+		clock-frequency = <66666667>;
+		hactive = <1280>;
+		hfront-porch = <18>;
+		hback-porch = <21>;
+		hsync-len = <32>;
+		vactive = <800>;
+		vfront-porch = <4>;
+		vback-porch = <8>;
+		vsync-len = <18>;
+	};
 };
 
 &rk808 {
-- 
2.21.0.392.gf8f6787159e-goog


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v4 7/7] ARM: dts: rockchip: Specify rk3288-veyron-minnie's display timings
@ 2019-03-28 17:17   ` Douglas Anderson
  0 siblings, 0 replies; 29+ messages in thread
From: Douglas Anderson @ 2019-03-28 17:17 UTC (permalink / raw)
  To: Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: Mark Rutland, devicetree, Rob Herring, Douglas Anderson,
	dri-devel, linux-kernel, linux-rockchip, Boris Brezillon,
	Laurent Pinchart, Enric Balletbò,
	Ezequiel Garcia, mka, linux-arm-kernel

Just like rk3288-veyron-jerry, we want to be able to use one of the
fixed PLLs in the system to make the pixel clock for minnie.

Specifying these timings matches us with how the display is used on
the downstream Chrome OS kernel.  See https://crrev.com/c/323211.

Unlike jerry, this CL actually changes the timings (though not the
pixel clock) that is used when using the upstream kernel.  Booting up
a minnie shows that it ended up with a 66.67 MHz pixel clock but it
was still using the porches/blankings it would have wanted for a 72.5
MHz pixel clock.

NOTE: compared to the downstream kernel, this seems to cause a
slightly different result reported in the 'modetest' command on a
Chromebook.  The downstream kernel shows:
  1280x800 60 1280 1298 1330 1351 800 804 822 830 66667

With this patch we have:
  1280x800 59 1280 1298 1330 1351 800 804 822 830 66666

Specifically modetest was reporting 60 Hz on the downstream kernel but
the upstream kernel does the math and comesup with 59 (because we
actually achieve 59.45 Hz).  Also upstream doesn't round the Hz up
when converting to kHz--it seems to truncate.

ALSO NOTE: when I look at the EDID from the datasheet, I see:
  -hsync -vsync
...but it seems like we've never actually run with that so I've
continued leaving that out.

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

Changes in v4:
- rk3288-veyron-minnie patch new for v4.

Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/rk3288-veyron-minnie.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-veyron-minnie.dts b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
index ca7512ade222..8179cf9f6e98 100644
--- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
@@ -144,6 +144,18 @@
 	power-supply= <&panel_regulator>;
 
 	/delete-node/ panel-timing;
+
+	panel-timing {
+		clock-frequency = <66666667>;
+		hactive = <1280>;
+		hfront-porch = <18>;
+		hback-porch = <21>;
+		hsync-len = <32>;
+		vactive = <800>;
+		vfront-porch = <4>;
+		vback-porch = <8>;
+		vsync-len = <18>;
+	};
 };
 
 &rk808 {
-- 
2.21.0.392.gf8f6787159e-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel
  2019-03-28 17:17 ` [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel Douglas Anderson
@ 2019-03-28 20:26   ` Ezequiel Garcia
  2019-03-28 23:50       ` Doug Anderson
  0 siblings, 1 reply; 29+ messages in thread
From: Ezequiel Garcia @ 2019-03-28 20:26 UTC (permalink / raw)
  To: Douglas Anderson, Thierry Reding, Heiko Stuebner, Sean Paul
  Cc: linux-rockchip, Laurent Pinchart, dri-devel, Boris Brezillon,
	Enric Balletbò,
	Rob Herring, mka, Eric Anholt, Jeffy Chen,
	Stéphane Marchesin, devicetree, linux-kernel, David Airlie,
	Mark Rutland, Daniel Vetter

On Thu, 2019-03-28 at 10:17 -0700, Douglas Anderson wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> This patch adds a new subnode to simple-panel allowing us to override
> the typical timing expressed in the panel's display_timing.
> 
> Changes in v2:
>  - Split out the binding into a new patch (Rob)
>  - display-timings is a new section (Rob)
>  - Use the full display-timings subnode instead of picking the timing
>    out (Rob/Thierry)
> Changes in v3:
>  - Go back to using the timing subnode directly, but rename to
>    panel-timing (Rob)
> Changes in v4:
>  - Simplify desc. for when override should be used (Thierry/Laurent)
>  - Removed Rob H review since it's been a year and wording changed
> 
> Cc: Doug Anderson <dianders@chromium.org>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-rockchip@lists.infradead.org
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
>  .../bindings/display/panel/simple-panel.txt   | 24 +++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/simple-panel.txt b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> index b2b872c710f2..6157f86ddce4 100644
> --- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> +++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> @@ -15,6 +15,18 @@ Optional properties:
>    (hot plug detect) signal, but the signal isn't hooked up so we should
>    hardcode the max delay from the panel spec when powering up the panel.
>  
> +panel-timing subnode
> +--------------------
> +
> +This optional subnode is for devices which require a mode differing
> +from the panel's "typical" display timing.  The panel timings provided
> +here will be ignored if they are found to be outside of allowable
> +ranges for the given panel.
> +

Is it OK to put this comment about how the implementation
will behave when values are out of range, given this is just a binding
spec?

Perhaps -if needed- this sentence can be rephrased to state that,
e.g. the OS may not be able to apply these values, if the controller
or device is unable to?

> +Format information on the panel-timing subnode can be found in
> +display-timing.txt.
> +
> +
>  Example:
>  
>  	panel: panel {
> @@ -25,4 +37,16 @@ Example:
>  		enable-gpios = <&gpio 90 0>;
>  
>  		backlight = <&backlight>;
> +
> +		panel-timing {
> +			clock-frequency = <266604720>;
> +			hactive = <2400>;
> +			hfront-porch = <48>;
> +			hback-porch = <84>;
> +			hsync-len = <32>;
> +			vactive = <1600>;
> +			vfront-porch = <3>;
> +			vback-porch = <120>;
> +			vsync-len = <10>;
> +		};
>  	};



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel
  2019-03-28 20:26   ` Ezequiel Garcia
@ 2019-03-28 23:50       ` Doug Anderson
  0 siblings, 0 replies; 29+ messages in thread
From: Doug Anderson @ 2019-03-28 23:50 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Thierry Reding, Heiko Stuebner, Sean Paul,
	open list:ARM/Rockchip SoC...,
	Laurent Pinchart, dri-devel, Boris Brezillon, Enric Balletbò,
	Rob Herring, Matthias Kaehlcke, Eric Anholt, Jeffy Chen,
	Stéphane Marchesin, devicetree, LKML, David Airlie,
	Mark Rutland, Daniel Vetter

Hi,


On Thu, Mar 28, 2019 at 1:27 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> On Thu, 2019-03-28 at 10:17 -0700, Douglas Anderson wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> >
> > This patch adds a new subnode to simple-panel allowing us to override
> > the typical timing expressed in the panel's display_timing.
> >
> > Changes in v2:
> >  - Split out the binding into a new patch (Rob)
> >  - display-timings is a new section (Rob)
> >  - Use the full display-timings subnode instead of picking the timing
> >    out (Rob/Thierry)
> > Changes in v3:
> >  - Go back to using the timing subnode directly, but rename to
> >    panel-timing (Rob)
> > Changes in v4:
> >  - Simplify desc. for when override should be used (Thierry/Laurent)
> >  - Removed Rob H review since it's been a year and wording changed
> >
> > Cc: Doug Anderson <dianders@chromium.org>
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: Heiko Stuebner <heiko@sntech.de>
> > Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Stéphane Marchesin <marcheu@chromium.org>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: devicetree@vger.kernel.org
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-rockchip@lists.infradead.org
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
> >  .../bindings/display/panel/simple-panel.txt   | 24 +++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/simple-panel.txt b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > index b2b872c710f2..6157f86ddce4 100644
> > --- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > +++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > @@ -15,6 +15,18 @@ Optional properties:
> >    (hot plug detect) signal, but the signal isn't hooked up so we should
> >    hardcode the max delay from the panel spec when powering up the panel.
> >
> > +panel-timing subnode
> > +--------------------
> > +
> > +This optional subnode is for devices which require a mode differing
> > +from the panel's "typical" display timing.  The panel timings provided
> > +here will be ignored if they are found to be outside of allowable
> > +ranges for the given panel.
> > +
>
> Is it OK to put this comment about how the implementation
> will behave when values are out of range, given this is just a binding
> spec?
>
> Perhaps -if needed- this sentence can be rephrased to state that,
> e.g. the OS may not be able to apply these values, if the controller
> or device is unable to?

I will defer to Rob H. on this one, but I'm happy to simply remove the
last sentence.  I was trying to add a more OS-agnostic version of the
bullet points from V3 but agree that we could just remove this from
the bindings completely.


-Doug

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel
@ 2019-03-28 23:50       ` Doug Anderson
  0 siblings, 0 replies; 29+ messages in thread
From: Doug Anderson @ 2019-03-28 23:50 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Mark Rutland, devicetree, David Airlie, Jeffy Chen, LKML,
	dri-devel, open list:ARM/Rockchip SoC...,
	Thierry Reding, Sean Paul, Laurent Pinchart, Boris Brezillon,
	Enric Balletbò,
	Rob Herring, Matthias Kaehlcke, Stéphane Marchesin

Hi,


On Thu, Mar 28, 2019 at 1:27 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> On Thu, 2019-03-28 at 10:17 -0700, Douglas Anderson wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> >
> > This patch adds a new subnode to simple-panel allowing us to override
> > the typical timing expressed in the panel's display_timing.
> >
> > Changes in v2:
> >  - Split out the binding into a new patch (Rob)
> >  - display-timings is a new section (Rob)
> >  - Use the full display-timings subnode instead of picking the timing
> >    out (Rob/Thierry)
> > Changes in v3:
> >  - Go back to using the timing subnode directly, but rename to
> >    panel-timing (Rob)
> > Changes in v4:
> >  - Simplify desc. for when override should be used (Thierry/Laurent)
> >  - Removed Rob H review since it's been a year and wording changed
> >
> > Cc: Doug Anderson <dianders@chromium.org>
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: Heiko Stuebner <heiko@sntech.de>
> > Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Stéphane Marchesin <marcheu@chromium.org>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: devicetree@vger.kernel.org
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-rockchip@lists.infradead.org
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
> >  .../bindings/display/panel/simple-panel.txt   | 24 +++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/simple-panel.txt b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > index b2b872c710f2..6157f86ddce4 100644
> > --- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > +++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > @@ -15,6 +15,18 @@ Optional properties:
> >    (hot plug detect) signal, but the signal isn't hooked up so we should
> >    hardcode the max delay from the panel spec when powering up the panel.
> >
> > +panel-timing subnode
> > +--------------------
> > +
> > +This optional subnode is for devices which require a mode differing
> > +from the panel's "typical" display timing.  The panel timings provided
> > +here will be ignored if they are found to be outside of allowable
> > +ranges for the given panel.
> > +
>
> Is it OK to put this comment about how the implementation
> will behave when values are out of range, given this is just a binding
> spec?
>
> Perhaps -if needed- this sentence can be rephrased to state that,
> e.g. the OS may not be able to apply these values, if the controller
> or device is unable to?

I will defer to Rob H. on this one, but I'm happy to simply remove the
last sentence.  I was trying to add a more OS-agnostic version of the
bullet points from V3 but agree that we could just remove this from
the bindings completely.


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel
  2019-03-28 23:50       ` Doug Anderson
@ 2019-03-29 16:12         ` Rob Herring
  -1 siblings, 0 replies; 29+ messages in thread
From: Rob Herring @ 2019-03-29 16:12 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Ezequiel Garcia, Thierry Reding, Heiko Stuebner, Sean Paul,
	open list:ARM/Rockchip SoC...,
	Laurent Pinchart, dri-devel, Boris Brezillon, Enric Balletbò,
	Matthias Kaehlcke, Eric Anholt, Jeffy Chen,
	Stéphane Marchesin, devicetree, LKML, David Airlie,
	Mark Rutland, Daniel Vetter

On Thu, Mar 28, 2019 at 6:50 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
>
> On Thu, Mar 28, 2019 at 1:27 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> >
> > On Thu, 2019-03-28 at 10:17 -0700, Douglas Anderson wrote:
> > > From: Sean Paul <seanpaul@chromium.org>
> > >
> > > This patch adds a new subnode to simple-panel allowing us to override
> > > the typical timing expressed in the panel's display_timing.
> > >
> > > Changes in v2:
> > >  - Split out the binding into a new patch (Rob)
> > >  - display-timings is a new section (Rob)
> > >  - Use the full display-timings subnode instead of picking the timing
> > >    out (Rob/Thierry)
> > > Changes in v3:
> > >  - Go back to using the timing subnode directly, but rename to
> > >    panel-timing (Rob)
> > > Changes in v4:
> > >  - Simplify desc. for when override should be used (Thierry/Laurent)
> > >  - Removed Rob H review since it's been a year and wording changed
> > >
> > > Cc: Doug Anderson <dianders@chromium.org>
> > > Cc: Eric Anholt <eric@anholt.net>
> > > Cc: Heiko Stuebner <heiko@sntech.de>
> > > Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Stéphane Marchesin <marcheu@chromium.org>
> > > Cc: Thierry Reding <thierry.reding@gmail.com>
> > > Cc: devicetree@vger.kernel.org
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: linux-rockchip@lists.infradead.org
> > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > ---
> > >
> > >  .../bindings/display/panel/simple-panel.txt   | 24 +++++++++++++++++++
> > >  1 file changed, 24 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/panel/simple-panel.txt b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > index b2b872c710f2..6157f86ddce4 100644
> > > --- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > +++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > @@ -15,6 +15,18 @@ Optional properties:
> > >    (hot plug detect) signal, but the signal isn't hooked up so we should
> > >    hardcode the max delay from the panel spec when powering up the panel.
> > >
> > > +panel-timing subnode
> > > +--------------------
> > > +
> > > +This optional subnode is for devices which require a mode differing
> > > +from the panel's "typical" display timing.  The panel timings provided
> > > +here will be ignored if they are found to be outside of allowable
> > > +ranges for the given panel.
> > > +
> >
> > Is it OK to put this comment about how the implementation
> > will behave when values are out of range, given this is just a binding
> > spec?
> >
> > Perhaps -if needed- this sentence can be rephrased to state that,
> > e.g. the OS may not be able to apply these values, if the controller
> > or device is unable to?
>
> I will defer to Rob H. on this one, but I'm happy to simply remove the
> last sentence.  I was trying to add a more OS-agnostic version of the
> bullet points from V3 but agree that we could just remove this from
> the bindings completely.

Following my opinion that it's not the kernel's job to validate
bindings, I would say it's fine for the OS to blindly apply them if it
chooses.

Plus with schema, you can provide the ranges of values and validate
DTs up front (unless you want to validate some result of math
operations).

Rob

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel
@ 2019-03-29 16:12         ` Rob Herring
  0 siblings, 0 replies; 29+ messages in thread
From: Rob Herring @ 2019-03-29 16:12 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Ezequiel Garcia, Thierry Reding, Heiko Stuebner, Sean Paul,
	open list:ARM/Rockchip SoC...,
	Laurent Pinchart, dri-devel, Boris Brezillon, Enric Balletbò,
	Matthias Kaehlcke, Eric Anholt, Jeffy Chen,
	Stéphane Marchesin, devicetree, LKML, David Airlie,
	Mark Rutland, Daniel Vetter

On Thu, Mar 28, 2019 at 6:50 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
>
> On Thu, Mar 28, 2019 at 1:27 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> >
> > On Thu, 2019-03-28 at 10:17 -0700, Douglas Anderson wrote:
> > > From: Sean Paul <seanpaul@chromium.org>
> > >
> > > This patch adds a new subnode to simple-panel allowing us to override
> > > the typical timing expressed in the panel's display_timing.
> > >
> > > Changes in v2:
> > >  - Split out the binding into a new patch (Rob)
> > >  - display-timings is a new section (Rob)
> > >  - Use the full display-timings subnode instead of picking the timing
> > >    out (Rob/Thierry)
> > > Changes in v3:
> > >  - Go back to using the timing subnode directly, but rename to
> > >    panel-timing (Rob)
> > > Changes in v4:
> > >  - Simplify desc. for when override should be used (Thierry/Laurent)
> > >  - Removed Rob H review since it's been a year and wording changed
> > >
> > > Cc: Doug Anderson <dianders@chromium.org>
> > > Cc: Eric Anholt <eric@anholt.net>
> > > Cc: Heiko Stuebner <heiko@sntech.de>
> > > Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Stéphane Marchesin <marcheu@chromium.org>
> > > Cc: Thierry Reding <thierry.reding@gmail.com>
> > > Cc: devicetree@vger.kernel.org
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: linux-rockchip@lists.infradead.org
> > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > ---
> > >
> > >  .../bindings/display/panel/simple-panel.txt   | 24 +++++++++++++++++++
> > >  1 file changed, 24 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/panel/simple-panel.txt b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > index b2b872c710f2..6157f86ddce4 100644
> > > --- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > +++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > @@ -15,6 +15,18 @@ Optional properties:
> > >    (hot plug detect) signal, but the signal isn't hooked up so we should
> > >    hardcode the max delay from the panel spec when powering up the panel.
> > >
> > > +panel-timing subnode
> > > +--------------------
> > > +
> > > +This optional subnode is for devices which require a mode differing
> > > +from the panel's "typical" display timing.  The panel timings provided
> > > +here will be ignored if they are found to be outside of allowable
> > > +ranges for the given panel.
> > > +
> >
> > Is it OK to put this comment about how the implementation
> > will behave when values are out of range, given this is just a binding
> > spec?
> >
> > Perhaps -if needed- this sentence can be rephrased to state that,
> > e.g. the OS may not be able to apply these values, if the controller
> > or device is unable to?
>
> I will defer to Rob H. on this one, but I'm happy to simply remove the
> last sentence.  I was trying to add a more OS-agnostic version of the
> bullet points from V3 but agree that we could just remove this from
> the bindings completely.

Following my opinion that it's not the kernel's job to validate
bindings, I would say it's fine for the OS to blindly apply them if it
chooses.

Plus with schema, you can provide the ranges of values and validate
DTs up front (unless you want to validate some result of math
operations).

Rob

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel
  2019-03-29 16:12         ` Rob Herring
@ 2019-03-29 16:14           ` Doug Anderson
  -1 siblings, 0 replies; 29+ messages in thread
From: Doug Anderson @ 2019-03-29 16:14 UTC (permalink / raw)
  To: Rob Herring
  Cc: Ezequiel Garcia, Thierry Reding, Heiko Stuebner, Sean Paul,
	open list:ARM/Rockchip SoC...,
	Laurent Pinchart, dri-devel, Boris Brezillon, Enric Balletbò,
	Matthias Kaehlcke, Eric Anholt, Jeffy Chen,
	Stéphane Marchesin, devicetree, LKML, David Airlie,
	Mark Rutland, Daniel Vetter

Hi,

On Fri, Mar 29, 2019 at 9:13 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Thu, Mar 28, 2019 at 6:50 PM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> >
> > On Thu, Mar 28, 2019 at 1:27 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > >
> > > On Thu, 2019-03-28 at 10:17 -0700, Douglas Anderson wrote:
> > > > From: Sean Paul <seanpaul@chromium.org>
> > > >
> > > > This patch adds a new subnode to simple-panel allowing us to override
> > > > the typical timing expressed in the panel's display_timing.
> > > >
> > > > Changes in v2:
> > > >  - Split out the binding into a new patch (Rob)
> > > >  - display-timings is a new section (Rob)
> > > >  - Use the full display-timings subnode instead of picking the timing
> > > >    out (Rob/Thierry)
> > > > Changes in v3:
> > > >  - Go back to using the timing subnode directly, but rename to
> > > >    panel-timing (Rob)
> > > > Changes in v4:
> > > >  - Simplify desc. for when override should be used (Thierry/Laurent)
> > > >  - Removed Rob H review since it's been a year and wording changed
> > > >
> > > > Cc: Doug Anderson <dianders@chromium.org>
> > > > Cc: Eric Anholt <eric@anholt.net>
> > > > Cc: Heiko Stuebner <heiko@sntech.de>
> > > > Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
> > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > Cc: Stéphane Marchesin <marcheu@chromium.org>
> > > > Cc: Thierry Reding <thierry.reding@gmail.com>
> > > > Cc: devicetree@vger.kernel.org
> > > > Cc: dri-devel@lists.freedesktop.org
> > > > Cc: linux-rockchip@lists.infradead.org
> > > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > > ---
> > > >
> > > >  .../bindings/display/panel/simple-panel.txt   | 24 +++++++++++++++++++
> > > >  1 file changed, 24 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/panel/simple-panel.txt b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > > index b2b872c710f2..6157f86ddce4 100644
> > > > --- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > > +++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > > @@ -15,6 +15,18 @@ Optional properties:
> > > >    (hot plug detect) signal, but the signal isn't hooked up so we should
> > > >    hardcode the max delay from the panel spec when powering up the panel.
> > > >
> > > > +panel-timing subnode
> > > > +--------------------
> > > > +
> > > > +This optional subnode is for devices which require a mode differing
> > > > +from the panel's "typical" display timing.  The panel timings provided
> > > > +here will be ignored if they are found to be outside of allowable
> > > > +ranges for the given panel.
> > > > +
> > >
> > > Is it OK to put this comment about how the implementation
> > > will behave when values are out of range, given this is just a binding
> > > spec?
> > >
> > > Perhaps -if needed- this sentence can be rephrased to state that,
> > > e.g. the OS may not be able to apply these values, if the controller
> > > or device is unable to?
> >
> > I will defer to Rob H. on this one, but I'm happy to simply remove the
> > last sentence.  I was trying to add a more OS-agnostic version of the
> > bullet points from V3 but agree that we could just remove this from
> > the bindings completely.
>
> Following my opinion that it's not the kernel's job to validate
> bindings, I would say it's fine for the OS to blindly apply them if it
> chooses.
>
> Plus with schema, you can provide the ranges of values and validate
> DTs up front (unless you want to validate some result of math
> operations).

OK, I'll remove the sentence and repost sometime early next week.  Thanks!

-Doug

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel
@ 2019-03-29 16:14           ` Doug Anderson
  0 siblings, 0 replies; 29+ messages in thread
From: Doug Anderson @ 2019-03-29 16:14 UTC (permalink / raw)
  To: Rob Herring
  Cc: Ezequiel Garcia, Thierry Reding, Heiko Stuebner, Sean Paul,
	open list:ARM/Rockchip SoC...,
	Laurent Pinchart, dri-devel, Boris Brezillon, Enric Balletbò,
	Matthias Kaehlcke, Eric Anholt, Jeffy Chen,
	Stéphane Marchesin, devicetree, LKML, David Airlie,
	Mark Rutland, Daniel Vetter

Hi,

On Fri, Mar 29, 2019 at 9:13 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Thu, Mar 28, 2019 at 6:50 PM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> >
> > On Thu, Mar 28, 2019 at 1:27 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > >
> > > On Thu, 2019-03-28 at 10:17 -0700, Douglas Anderson wrote:
> > > > From: Sean Paul <seanpaul@chromium.org>
> > > >
> > > > This patch adds a new subnode to simple-panel allowing us to override
> > > > the typical timing expressed in the panel's display_timing.
> > > >
> > > > Changes in v2:
> > > >  - Split out the binding into a new patch (Rob)
> > > >  - display-timings is a new section (Rob)
> > > >  - Use the full display-timings subnode instead of picking the timing
> > > >    out (Rob/Thierry)
> > > > Changes in v3:
> > > >  - Go back to using the timing subnode directly, but rename to
> > > >    panel-timing (Rob)
> > > > Changes in v4:
> > > >  - Simplify desc. for when override should be used (Thierry/Laurent)
> > > >  - Removed Rob H review since it's been a year and wording changed
> > > >
> > > > Cc: Doug Anderson <dianders@chromium.org>
> > > > Cc: Eric Anholt <eric@anholt.net>
> > > > Cc: Heiko Stuebner <heiko@sntech.de>
> > > > Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
> > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > Cc: Stéphane Marchesin <marcheu@chromium.org>
> > > > Cc: Thierry Reding <thierry.reding@gmail.com>
> > > > Cc: devicetree@vger.kernel.org
> > > > Cc: dri-devel@lists.freedesktop.org
> > > > Cc: linux-rockchip@lists.infradead.org
> > > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > > ---
> > > >
> > > >  .../bindings/display/panel/simple-panel.txt   | 24 +++++++++++++++++++
> > > >  1 file changed, 24 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/panel/simple-panel.txt b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > > index b2b872c710f2..6157f86ddce4 100644
> > > > --- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > > +++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > > @@ -15,6 +15,18 @@ Optional properties:
> > > >    (hot plug detect) signal, but the signal isn't hooked up so we should
> > > >    hardcode the max delay from the panel spec when powering up the panel.
> > > >
> > > > +panel-timing subnode
> > > > +--------------------
> > > > +
> > > > +This optional subnode is for devices which require a mode differing
> > > > +from the panel's "typical" display timing.  The panel timings provided
> > > > +here will be ignored if they are found to be outside of allowable
> > > > +ranges for the given panel.
> > > > +
> > >
> > > Is it OK to put this comment about how the implementation
> > > will behave when values are out of range, given this is just a binding
> > > spec?
> > >
> > > Perhaps -if needed- this sentence can be rephrased to state that,
> > > e.g. the OS may not be able to apply these values, if the controller
> > > or device is unable to?
> >
> > I will defer to Rob H. on this one, but I'm happy to simply remove the
> > last sentence.  I was trying to add a more OS-agnostic version of the
> > bullet points from V3 but agree that we could just remove this from
> > the bindings completely.
>
> Following my opinion that it's not the kernel's job to validate
> bindings, I would say it's fine for the OS to blindly apply them if it
> chooses.
>
> Plus with schema, you can provide the ranges of values and validate
> DTs up front (unless you want to validate some result of math
> operations).

OK, I'll remove the sentence and repost sometime early next week.  Thanks!

-Doug

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 2/7] drm/panel: simple: Add ability to override typical timing
  2019-03-28 17:17   ` Douglas Anderson
@ 2019-03-29 19:13     ` Heiko Stübner
  -1 siblings, 0 replies; 29+ messages in thread
From: Heiko Stübner @ 2019-03-29 19:13 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Thierry Reding, Sean Paul, linux-rockchip, Laurent Pinchart,
	dri-devel, Boris Brezillon, Ezequiel Garcia, Enric Balletbò,
	Rob Herring, mka, Eric Anholt, Jeffy Chen,
	Stéphane Marchesin, devicetree, David Airlie, linux-kernel,
	Daniel Vetter

Am Donnerstag, 28. März 2019, 18:17:05 CET schrieb Douglas Anderson:
> From: Sean Paul <seanpaul@chromium.org>
> 
> This patch adds the ability to override the typical display timing for a
> given panel. This is useful for devices which have timing constraints
> that do not apply across the entire display driver (eg: to avoid
> crosstalk between panel and digitizer on certain laptops). The rules are
> as follows:
> 
> - panel must not specify fixed mode (since the override mode will
>   either be the same as the fixed mode, or we'll be unable to
>   check the bounds of the overried)
> - panel must specify at least one display_timing range which will be
>   used to ensure the override mode fits within its bounds
> 
> Changes in v2:
>  - Parse the full display-timings node (using the native-mode) (Rob)
> Changes in v3:
>  - No longer parse display-timings subnode, use panel-timing (Rob)
> Changes in v4:
>  - Don't add mode from timing if override was specified (Thierry)
>  - Add warning if timing and fixed mode was specified (Thierry)
>  - Don't add fixed mode if timing was specified (Thierry)
>  - Refactor/rename a bit to avoid extra indentation from "if" tests
>  - i should be unsigned (Thierry)
>  - Add annoying WARN_ONs for some cases (Thierry)
>  - Simplify 'No display_timing found' handling (Thierry)
>  - Rename to panel_simple_parse_override_mode() (Thierry)
> 
> Cc: Doug Anderson <dianders@chromium.org>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

on a rk3399-kevin and rk3288-jerry
Tested-by: Heiko Stuebner <heiko@sntech.de>



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 2/7] drm/panel: simple: Add ability to override typical timing
@ 2019-03-29 19:13     ` Heiko Stübner
  0 siblings, 0 replies; 29+ messages in thread
From: Heiko Stübner @ 2019-03-29 19:13 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: devicetree, Rob Herring, David Airlie, Jeffy Chen, linux-kernel,
	dri-devel, linux-rockchip, Thierry Reding, Sean Paul,
	Laurent Pinchart, Boris Brezillon, Enric Balletbò,
	Stéphane Marchesin, Ezequiel Garcia, mka

Am Donnerstag, 28. März 2019, 18:17:05 CET schrieb Douglas Anderson:
> From: Sean Paul <seanpaul@chromium.org>
> 
> This patch adds the ability to override the typical display timing for a
> given panel. This is useful for devices which have timing constraints
> that do not apply across the entire display driver (eg: to avoid
> crosstalk between panel and digitizer on certain laptops). The rules are
> as follows:
> 
> - panel must not specify fixed mode (since the override mode will
>   either be the same as the fixed mode, or we'll be unable to
>   check the bounds of the overried)
> - panel must specify at least one display_timing range which will be
>   used to ensure the override mode fits within its bounds
> 
> Changes in v2:
>  - Parse the full display-timings node (using the native-mode) (Rob)
> Changes in v3:
>  - No longer parse display-timings subnode, use panel-timing (Rob)
> Changes in v4:
>  - Don't add mode from timing if override was specified (Thierry)
>  - Add warning if timing and fixed mode was specified (Thierry)
>  - Don't add fixed mode if timing was specified (Thierry)
>  - Refactor/rename a bit to avoid extra indentation from "if" tests
>  - i should be unsigned (Thierry)
>  - Add annoying WARN_ONs for some cases (Thierry)
>  - Simplify 'No display_timing found' handling (Thierry)
>  - Rename to panel_simple_parse_override_mode() (Thierry)
> 
> Cc: Doug Anderson <dianders@chromium.org>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

on a rk3399-kevin and rk3288-jerry
Tested-by: Heiko Stuebner <heiko@sntech.de>


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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 4/7] drm/panel: simple: Use display_timing for Innolux n116bge
  2019-03-28 17:17   ` Douglas Anderson
@ 2019-03-29 19:17     ` Heiko Stübner
  -1 siblings, 0 replies; 29+ messages in thread
From: Heiko Stübner @ 2019-03-29 19:17 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Thierry Reding, Sean Paul, linux-rockchip, Laurent Pinchart,
	dri-devel, Boris Brezillon, Ezequiel Garcia, Enric Balletbò,
	Rob Herring, mka, David Airlie, linux-kernel, Daniel Vetter

Am Donnerstag, 28. März 2019, 18:17:07 CET schrieb Douglas Anderson:
> Convert the Innolux n116bge from using a fixed mode to specifying a
> display timing with min/typ/max values.
> 
> Note that the n116bge's datasheet doesn't fit too well into DRM's way
> of specifying things.  Specifically the panel's datasheet just
> specifies the vertical blanking period and horizontal blanking period
> and doesn't break things out.  For now we'll leave everything as a
> fixed value but just allow adjusting the pixel clock.  I've added a
> comment on what the datasheet claims so someone could later expand
> things to fit their needs if they wanted to test other blanking
> periods.
> 
> The goal here is to be able to specify the panel timings in the device
> tree for several rk3288 Chromebooks (like rk3288-veryon-jerry).  These
> Chromebooks have all been running in the downstream kernel with the
> standard porches and sync lengths but just with a slightly slower
> pixel clock because the 76.42 MHz clock is not achievable from the
> fixed PLL that was available.  These Chromebooks only achieve a
> refresh rate of ~58 Hz.  While it's probable that we could adjust the
> timings to achieve 60 Hz it's probably wisest to match what's been
> running on these devices all these years.
> 
> I'll note that though the upstream kernel has always tried to achieve
> 76.42 MHz, it has actually been running at 74.25 MHz also since the
> video processor is parented off the same fixed PLL.
> 
> Changes in v4:
>  - display_timing for Innolux n116bge new for v4.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

on a rk3288-jerry and rk3288-pinky
Tested-by: Heiko Stuebner <heiko@sntech.de>




^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 4/7] drm/panel: simple: Use display_timing for Innolux n116bge
@ 2019-03-29 19:17     ` Heiko Stübner
  0 siblings, 0 replies; 29+ messages in thread
From: Heiko Stübner @ 2019-03-29 19:17 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Rob Herring, David Airlie, linux-kernel, dri-devel,
	linux-rockchip, Thierry Reding, Sean Paul, Laurent Pinchart,
	Boris Brezillon, Enric Balletbò,
	Ezequiel Garcia, mka

Am Donnerstag, 28. März 2019, 18:17:07 CET schrieb Douglas Anderson:
> Convert the Innolux n116bge from using a fixed mode to specifying a
> display timing with min/typ/max values.
> 
> Note that the n116bge's datasheet doesn't fit too well into DRM's way
> of specifying things.  Specifically the panel's datasheet just
> specifies the vertical blanking period and horizontal blanking period
> and doesn't break things out.  For now we'll leave everything as a
> fixed value but just allow adjusting the pixel clock.  I've added a
> comment on what the datasheet claims so someone could later expand
> things to fit their needs if they wanted to test other blanking
> periods.
> 
> The goal here is to be able to specify the panel timings in the device
> tree for several rk3288 Chromebooks (like rk3288-veryon-jerry).  These
> Chromebooks have all been running in the downstream kernel with the
> standard porches and sync lengths but just with a slightly slower
> pixel clock because the 76.42 MHz clock is not achievable from the
> fixed PLL that was available.  These Chromebooks only achieve a
> refresh rate of ~58 Hz.  While it's probable that we could adjust the
> timings to achieve 60 Hz it's probably wisest to match what's been
> running on these devices all these years.
> 
> I'll note that though the upstream kernel has always tried to achieve
> 76.42 MHz, it has actually been running at 74.25 MHz also since the
> video processor is parented off the same fixed PLL.
> 
> Changes in v4:
>  - display_timing for Innolux n116bge new for v4.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

on a rk3288-jerry and rk3288-pinky
Tested-by: Heiko Stuebner <heiko@sntech.de>



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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 6/7] ARM: dts: rockchip: Specify rk3288-veyron-jerry's display timings
  2019-03-28 17:17   ` Douglas Anderson
@ 2019-03-29 19:20     ` Heiko Stübner
  -1 siblings, 0 replies; 29+ messages in thread
From: Heiko Stübner @ 2019-03-29 19:20 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Thierry Reding, Sean Paul, linux-rockchip, Laurent Pinchart,
	dri-devel, Boris Brezillon, Ezequiel Garcia, Enric Balletbò,
	Rob Herring, mka, devicetree, linux-kernel, Mark Rutland,
	linux-arm-kernel

Am Donnerstag, 28. März 2019, 18:17:09 CET schrieb Douglas Anderson:
> Let's document the display timings that jerry has been using out in
> the field.  This uses the standard blankings but a slightly slower
> clock rate, thus getting a refresh rate 58.3 Hz.
> 
> NOTE: this won't really do anything except cause DRM to properly
> report the refresh rate since vop_crtc_mode_fixup() was rounding the
> pixel clock to 74.25 MHz anyway.  Apparently the adjusted rate isn't
> exposed to userspace so it's important that the rate we're trying to
> achieve is mostly right.
> 
> For the downstream kernel change related to this see See
> https://crrev.com/c/324558.
> 
> NOTE: minnie will be fixed up in a future patch, so for now we'll just
> delete the panel timings there.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
> Changes in v4:
> - rk3288-veyron-jerry patch new for v4.
> 
> Changes in v3: None
> Changes in v2: None
> 
>  arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi | 14 ++++++++++++++

hmm, the commit message explicitly mentions jerry, but this is the
general panel definition for most veyron-chromebooks (jerry, pinky, jaq,...)?

It does work on both pinky and jerry for me, so I guess just the commit
message needs a bit adapting?


Heiko



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v4 6/7] ARM: dts: rockchip: Specify rk3288-veyron-jerry's display timings
@ 2019-03-29 19:20     ` Heiko Stübner
  0 siblings, 0 replies; 29+ messages in thread
From: Heiko Stübner @ 2019-03-29 19:20 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Mark Rutland, devicetree, Rob Herring, linux-kernel, dri-devel,
	linux-rockchip, Thierry Reding, Sean Paul, Laurent Pinchart,
	Boris Brezillon, Enric Balletbò,
	Ezequiel Garcia, mka, linux-arm-kernel

Am Donnerstag, 28. März 2019, 18:17:09 CET schrieb Douglas Anderson:
> Let's document the display timings that jerry has been using out in
> the field.  This uses the standard blankings but a slightly slower
> clock rate, thus getting a refresh rate 58.3 Hz.
> 
> NOTE: this won't really do anything except cause DRM to properly
> report the refresh rate since vop_crtc_mode_fixup() was rounding the
> pixel clock to 74.25 MHz anyway.  Apparently the adjusted rate isn't
> exposed to userspace so it's important that the rate we're trying to
> achieve is mostly right.
> 
> For the downstream kernel change related to this see See
> https://crrev.com/c/324558.
> 
> NOTE: minnie will be fixed up in a future patch, so for now we'll just
> delete the panel timings there.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
> Changes in v4:
> - rk3288-veyron-jerry patch new for v4.
> 
> Changes in v3: None
> Changes in v2: None
> 
>  arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi | 14 ++++++++++++++

hmm, the commit message explicitly mentions jerry, but this is the
general panel definition for most veyron-chromebooks (jerry, pinky, jaq,...)?

It does work on both pinky and jerry for me, so I guess just the commit
message needs a bit adapting?


Heiko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2019-03-29 19:20 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-28 17:17 [PATCH v4 0/7] drm/panel: simple: Add mode support to devicetree Douglas Anderson
2019-03-28 17:17 ` Douglas Anderson
2019-03-28 17:17 ` Douglas Anderson
2019-03-28 17:17 ` [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel Douglas Anderson
2019-03-28 20:26   ` Ezequiel Garcia
2019-03-28 23:50     ` Doug Anderson
2019-03-28 23:50       ` Doug Anderson
2019-03-29 16:12       ` Rob Herring
2019-03-29 16:12         ` Rob Herring
2019-03-29 16:14         ` Doug Anderson
2019-03-29 16:14           ` Doug Anderson
2019-03-28 17:17 ` [PATCH v4 2/7] drm/panel: simple: Add ability to override typical timing Douglas Anderson
2019-03-28 17:17   ` Douglas Anderson
2019-03-29 19:13   ` Heiko Stübner
2019-03-29 19:13     ` Heiko Stübner
2019-03-28 17:17 ` [PATCH v4 3/7] arm64: dts: rockchip: Specify override mode for kevin panel Douglas Anderson
2019-03-28 17:17   ` Douglas Anderson
2019-03-28 17:17   ` Douglas Anderson
2019-03-28 17:17 ` [PATCH v4 4/7] drm/panel: simple: Use display_timing for Innolux n116bge Douglas Anderson
2019-03-28 17:17   ` Douglas Anderson
2019-03-29 19:17   ` Heiko Stübner
2019-03-29 19:17     ` Heiko Stübner
2019-03-28 17:17 ` [PATCH v4 5/7] drm/panel: simple: Use display_timing for AUO b101ean01 Douglas Anderson
2019-03-28 17:17 ` [PATCH v4 6/7] ARM: dts: rockchip: Specify rk3288-veyron-jerry's display timings Douglas Anderson
2019-03-28 17:17   ` Douglas Anderson
2019-03-29 19:20   ` Heiko Stübner
2019-03-29 19:20     ` Heiko Stübner
2019-03-28 17:17 ` [PATCH v4 7/7] ARM: dts: rockchip: Specify rk3288-veyron-minnie's " Douglas Anderson
2019-03-28 17:17   ` Douglas Anderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.