[PATCH v4 0/7] drm/panel: simple: Add mode support to devicetree

[PATCH v4 0/7] drm/panel: simple: Add mode support to devicetree

[Douglas Anderson]

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

[PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel

[Douglas Anderson]

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

[PATCH v4 2/7] drm/panel: simple: Add ability to override typical timing

[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>
---

 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

[PATCH v4 3/7] arm64: dts: rockchip: Specify override mode for kevin panel

[Douglas Anderson]

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

[PATCH v4 6/7] ARM: dts: rockchip: Specify rk3288-veyron-jerry's display timings

[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 ++++++++++++++
 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

[PATCH v4 7/7] ARM: dts: rockchip: Specify rk3288-veyron-minnie's display timings

[Douglas Anderson]

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

Re: [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel

[Ezequiel Garcia]

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>;
> +		};
>  	};

Re: [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel

[Doug Anderson]

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

Re: [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel

[Rob Herring]

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

Re: [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel

[Doug Anderson]

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

Re: [PATCH v4 2/7] drm/panel: simple: Add ability to override typical timing

[Heiko Stübner]

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

Re: [PATCH v4 6/7] ARM: dts: rockchip: Specify rk3288-veyron-jerry's display timings

[Heiko Stübner]

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