* [PATCH 0/4] drm: bridge: display-connector: Support hardcoding EDID for VGA
@ 2026-07-05 21:35 Laurent Pinchart
2026-07-05 21:35 ` [PATCH 1/4] dt-bindings: display: vga-connector: Allow hardcoding EDID Laurent Pinchart
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Laurent Pinchart @ 2026-07-05 21:35 UTC (permalink / raw)
To: dri-devel, devicetree
Cc: Thuan Nguyen, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Andrzej Hajda, Neil Armstrong,
Robert Foss, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
linux-renesas-soc
Hello,
In a normal use case, the VGA DDC lines are used to read EDID from the
display. On some systems where the connector is internal to the device
and the VGA display is always present, the DDC lines may not be
connected, making display detection and EDID retrieval impossible.
This patch series adds the ability to hardcode EDID in the device tree
for VGA connectors on such systems. Patch 1/4 extends the vga-connector
DT bindings with an "edid" property. Patch 2/4 is a drive-by fix for a
resource leak in the display-connector driver, and patch 3/4 implements
support for the "edid" property in the driver.
I have tested this on a Renesas Salvator-XS M3N development board. The
board has an external VGA connector, but does not conenct the DDC lines,
exhibiting the problem that this series addresses. Patch 4/4
demonstrates how to set EDID in the device tree, for the VGA monitor I
used in my test environment. It is not meant to be merged as the display
is not internal to the device in this case.
Laurent Pinchart (4):
dt-bindings: display: vga-connector: Allow hardcoding EDID
drm/bridge: display-connector: Fix I2C adapter resource leak
drm/bridge: display-connector: Support hardcoded EDID for VGA
connectors
[DNI] arm64: dts: renesas: salvator: Add overlay for Dell P1911 VGA
display
.../display/connector/vga-connector.yaml | 15 +++++++
arch/arm64/boot/dts/renesas/Makefile | 2 +
.../dts/renesas/salvator-vga-dell-p1911.dtso | 21 ++++++++++
drivers/gpu/drm/bridge/display-connector.c | 42 ++++++++++++++++---
4 files changed, 75 insertions(+), 5 deletions(-)
create mode 100644 arch/arm64/boot/dts/renesas/salvator-vga-dell-p1911.dtso
base-commit: 44d19b8a7548aa25cbc6ebd5f27e958f7142c36b
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/4] dt-bindings: display: vga-connector: Allow hardcoding EDID
2026-07-05 21:35 [PATCH 0/4] drm: bridge: display-connector: Support hardcoding EDID for VGA Laurent Pinchart
@ 2026-07-05 21:35 ` Laurent Pinchart
2026-07-06 8:08 ` Geert Uytterhoeven
2026-07-06 8:52 ` Maxime Ripard
2026-07-05 21:35 ` [PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak Laurent Pinchart
` (2 subsequent siblings)
3 siblings, 2 replies; 19+ messages in thread
From: Laurent Pinchart @ 2026-07-05 21:35 UTC (permalink / raw)
To: dri-devel, devicetree
Cc: Thuan Nguyen, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Andrzej Hajda, Neil Armstrong,
Robert Foss, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
linux-renesas-soc
Since DDC version 2, introduced in 1996, VGA monitors have exposed EDID
data over an I2C bus. The bus is also used to detect the presence of a
connected monitor by trying to read the EDID data.
Some devices where the VGA display is integrated in the device and
always connected do not connect the DDC pins. Some development boards,
such as the Renesas M3N Salvator-XS, also do not connect the DDC pins.
To support those, add the ability to provide hardcoded EDID data in the
device tree. This is mutually exclusive with specifying a DDC bus, and
can only be done when the VGA display is guaranteed to be always
connected.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
.../bindings/display/connector/vga-connector.yaml | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/connector/vga-connector.yaml b/Documentation/devicetree/bindings/display/connector/vga-connector.yaml
index 25f868002000..7e1b8e798e5b 100644
--- a/Documentation/devicetree/bindings/display/connector/vga-connector.yaml
+++ b/Documentation/devicetree/bindings/display/connector/vga-connector.yaml
@@ -19,10 +19,25 @@ properties:
description: phandle link to the I2C controller used for DDC EDID probing
$ref: /schemas/types.yaml#/definitions/phandle
+ edid:
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+ description:
+ When the DDC signals are not wired to the connector, and the connected
+ display is not removable, this property is used to supply a binary EDID
+ blob for the display.
+
port:
$ref: /schemas/graph.yaml#/properties/port
description: Connection to controller providing VGA signals
+allOf:
+ - if:
+ required:
+ - ddc-i2c-bus
+ then:
+ properties:
+ edid: false
+
required:
- compatible
- port
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak
2026-07-05 21:35 [PATCH 0/4] drm: bridge: display-connector: Support hardcoding EDID for VGA Laurent Pinchart
2026-07-05 21:35 ` [PATCH 1/4] dt-bindings: display: vga-connector: Allow hardcoding EDID Laurent Pinchart
@ 2026-07-05 21:35 ` Laurent Pinchart
2026-07-05 21:48 ` sashiko-bot
2026-07-05 21:35 ` [PATCH 3/4] drm/bridge: display-connector: Support hardcoded EDID for VGA connectors Laurent Pinchart
2026-07-05 21:35 ` [PATCH 4/4] [DNI] arm64: dts: renesas: salvator: Add overlay for Dell P1911 VGA display Laurent Pinchart
3 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2026-07-05 21:35 UTC (permalink / raw)
To: dri-devel, devicetree
Cc: Thuan Nguyen, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Andrzej Hajda, Neil Armstrong,
Robert Foss, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
linux-renesas-soc, stable
If the probe function returns an error after getting the I2C adapter for
DDC, the reference to the adapter is never released. Fix it by releasing
it in the bridge .destroy() handler.
There is no need to test the ddc pointer with !IS_ERR(), as
of_get_i2c_adapter_by_node() returns NULL on error.
Fixes: 6de79dd3a920 ("drm/bridge: display-connector: add ddc-en gpio support")
Cc: stable@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/gpu/drm/bridge/display-connector.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
index d1e74dd2e051..eb3412ec30a0 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -43,6 +43,13 @@ static int display_connector_attach(struct drm_bridge *bridge,
return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
}
+static void display_connector_destroy(struct drm_bridge *bridge)
+{
+ struct display_connector *conn = to_display_connector(bridge);
+
+ i2c_put_adapter(conn->bridge.ddc);
+}
+
static enum drm_connector_status display_connector_detect(struct drm_bridge *bridge)
{
struct display_connector *conn = to_display_connector(bridge);
@@ -208,6 +215,7 @@ static u32 *display_connector_get_input_bus_fmts(struct drm_bridge *bridge,
static const struct drm_bridge_funcs display_connector_bridge_funcs = {
.attach = display_connector_attach,
+ .destroy = display_connector_destroy,
.detect = display_connector_bridge_detect,
.hpd_enable = display_connector_hpd_enable,
.hpd_disable = display_connector_hpd_disable,
@@ -439,9 +447,6 @@ static void display_connector_remove(struct platform_device *pdev)
regulator_disable(conn->supply);
drm_bridge_remove(&conn->bridge);
-
- if (!IS_ERR(conn->bridge.ddc))
- i2c_put_adapter(conn->bridge.ddc);
}
static const struct of_device_id display_connector_match[] = {
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/4] drm/bridge: display-connector: Support hardcoded EDID for VGA connectors
2026-07-05 21:35 [PATCH 0/4] drm: bridge: display-connector: Support hardcoding EDID for VGA Laurent Pinchart
2026-07-05 21:35 ` [PATCH 1/4] dt-bindings: display: vga-connector: Allow hardcoding EDID Laurent Pinchart
2026-07-05 21:35 ` [PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak Laurent Pinchart
@ 2026-07-05 21:35 ` Laurent Pinchart
2026-07-05 21:44 ` sashiko-bot
2026-07-05 21:35 ` [PATCH 4/4] [DNI] arm64: dts: renesas: salvator: Add overlay for Dell P1911 VGA display Laurent Pinchart
3 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2026-07-05 21:35 UTC (permalink / raw)
To: dri-devel, devicetree
Cc: Thuan Nguyen, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Andrzej Hajda, Neil Armstrong,
Robert Foss, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
linux-renesas-soc
The DT binding for VGA connectors allow providing an EDID blob in the
device tree when the DDC signals are not connected. This is useful for
devices where the VGA connector is internal and a display is guaranteed
to always be connected.
Support this feature in the driver by parsing the EDID from DT if
available. If a DDC bus is also specified, return an error from the
probe function as DDC and hardcoded EDID are mutually exclusive.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/gpu/drm/bridge/display-connector.c | 31 ++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
index eb3412ec30a0..225a8e987d08 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -27,6 +27,8 @@ struct display_connector {
struct regulator *supply;
struct gpio_desc *ddc_en;
+ const struct drm_edid *edid;
+
struct work_struct hpd_work;
};
@@ -48,6 +50,7 @@ static void display_connector_destroy(struct drm_bridge *bridge)
struct display_connector *conn = to_display_connector(bridge);
i2c_put_adapter(conn->bridge.ddc);
+ drm_edid_free(conn->edid);
}
static enum drm_connector_status display_connector_detect(struct drm_bridge *bridge)
@@ -64,6 +67,9 @@ static enum drm_connector_status display_connector_detect(struct drm_bridge *bri
if (conn->bridge.ddc && drm_probe_ddc(conn->bridge.ddc))
return connector_status_connected;
+ if (conn->edid)
+ return connector_status_connected;
+
switch (conn->bridge.type) {
case DRM_MODE_CONNECTOR_DVIA:
case DRM_MODE_CONNECTOR_DVID:
@@ -130,7 +136,10 @@ static const struct drm_edid *display_connector_edid_read(struct drm_bridge *bri
{
struct display_connector *conn = to_display_connector(bridge);
- return drm_edid_read_ddc(connector, conn->bridge.ddc);
+ if (conn->bridge.ddc)
+ return drm_edid_read_ddc(connector, conn->bridge.ddc);
+ else
+ return drm_edid_dup(conn->edid);
}
/*
@@ -376,6 +385,24 @@ static int display_connector_probe(struct platform_device *pdev)
}
}
+ /* Retrieve the hardcoded EDID for VGA connectors. */
+ if (type == DRM_MODE_CONNECTOR_VGA) {
+ const void *edid;
+ int size;
+
+ edid = of_get_property(pdev->dev.of_node, "edid", &size);
+ if (edid) {
+ if (conn->bridge.ddc)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "DDC and EDID are mutually exclusive\n");
+
+ conn->edid = drm_edid_alloc(edid, size);
+ if (!conn->edid)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "failed to parse EDID\n");
+ }
+ }
+
/* Get the DP PWR for DP connector. */
if (type == DRM_MODE_CONNECTOR_DisplayPort) {
int ret;
@@ -412,7 +439,7 @@ static int display_connector_probe(struct platform_device *pdev)
conn->bridge.of_node = pdev->dev.of_node;
- if (conn->bridge.ddc)
+ if (conn->bridge.ddc || conn->edid)
conn->bridge.ops |= DRM_BRIDGE_OP_EDID
| DRM_BRIDGE_OP_DETECT;
/* Detecting the monitor requires reading DPCD */
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/4] [DNI] arm64: dts: renesas: salvator: Add overlay for Dell P1911 VGA display
2026-07-05 21:35 [PATCH 0/4] drm: bridge: display-connector: Support hardcoding EDID for VGA Laurent Pinchart
` (2 preceding siblings ...)
2026-07-05 21:35 ` [PATCH 3/4] drm/bridge: display-connector: Support hardcoded EDID for VGA connectors Laurent Pinchart
@ 2026-07-05 21:35 ` Laurent Pinchart
2026-07-05 21:37 ` sashiko-bot
2026-07-06 8:16 ` Geert Uytterhoeven
3 siblings, 2 replies; 19+ messages in thread
From: Laurent Pinchart @ 2026-07-05 21:35 UTC (permalink / raw)
To: dri-devel, devicetree
Cc: Thuan Nguyen, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Andrzej Hajda, Neil Armstrong,
Robert Foss, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
linux-renesas-soc
Add an overlay that hardcodes the EDID of the Dell P1911 VGA display
connector to the VGA port of the Salvator board. This is meant to test
EDID hardcoding and should not be merged upstream.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
arch/arm64/boot/dts/renesas/Makefile | 2 ++
.../dts/renesas/salvator-vga-dell-p1911.dtso | 21 +++++++++++++++++++
2 files changed, 23 insertions(+)
create mode 100644 arch/arm64/boot/dts/renesas/salvator-vga-dell-p1911.dtso
diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
index 8bf155badd11..f6a6fb959a6d 100644
--- a/arch/arm64/boot/dts/renesas/Makefile
+++ b/arch/arm64/boot/dts/renesas/Makefile
@@ -58,6 +58,8 @@ dtb-$(CONFIG_ARCH_R8A77965) += r8a77965-salvator-x-panel-aa104xd12.dtb
dtb-$(CONFIG_ARCH_R8A77965) += r8a77965-salvator-xs.dtb
r8a77965-salvator-xs-panel-aa104xd12-dtbs := r8a77965-salvator-xs.dtb salvator-panel-aa104xd12.dtbo
dtb-$(CONFIG_ARCH_R8A77965) += r8a77965-salvator-xs-panel-aa104xd12.dtb
+r8a77965-salvator-xs-vga-dell-p1911-dtbs := r8a77965-salvator-xs.dtb salvator-vga-dell-p1911.dtbo
+dtb-$(CONFIG_ARCH_R8A77965) += r8a77965-salvator-xs-vga-dell-p1911.dtb
dtb-$(CONFIG_ARCH_R8A77965) += r8a77965-ulcb.dtb
dtb-$(CONFIG_ARCH_R8A77965) += r8a77965-ulcb-kf.dtb
dtb-$(CONFIG_ARCH_R8A77965) += r8a779md-geist.dtb
diff --git a/arch/arm64/boot/dts/renesas/salvator-vga-dell-p1911.dtso b/arch/arm64/boot/dts/renesas/salvator-vga-dell-p1911.dtso
new file mode 100644
index 000000000000..d963f5a65576
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/salvator-vga-dell-p1911.dtso
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Device Tree overlay for the Dell P1911 monitor connected to VGA port of a
+ * Salvator-X or Salvator-XS board
+ *
+ * Copyright 2026 Ideas on Board Oy
+ */
+
+/dts-v1/;
+/plugin/;
+
+&{/vga} {
+ edid = [00 ff ff ff ff ff ff 00 10 ac 74 a0 55 4b 55 30
+ 2b 14 01 03 80 29 1a 78 ee ee 95 a3 54 4c 99 26
+ 0f 50 54 bf ef 80 95 00 71 4f 81 80 95 0f 81 00
+ 01 01 01 01 01 01 9a 29 a0 d0 51 84 22 30 50 98
+ 36 00 98 ff 10 00 00 1c 00 00 00 ff 00 31 57 4b
+ 4a 56 30 41 4b 30 55 4b 55 0a 00 00 00 fc 00 44
+ 45 4c 4c 20 50 31 39 31 31 0a 20 20 00 00 00 fd
+ 00 38 4b 1e 53 0e 00 0a 20 20 20 20 20 20 00 ba];
+};
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] [DNI] arm64: dts: renesas: salvator: Add overlay for Dell P1911 VGA display
2026-07-05 21:35 ` [PATCH 4/4] [DNI] arm64: dts: renesas: salvator: Add overlay for Dell P1911 VGA display Laurent Pinchart
@ 2026-07-05 21:37 ` sashiko-bot
2026-07-05 23:39 ` Laurent Pinchart
2026-07-06 8:16 ` Geert Uytterhoeven
1 sibling, 1 reply; 19+ messages in thread
From: sashiko-bot @ 2026-07-05 21:37 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: devicetree, dri-devel, robh, conor+dt, wsa+renesas
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch is explicitly marked as test-only and should not be merged.
--
commit ec00db7021fbc2f550a150a0512dfc7596a53df2
Author: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
arm64: dts: renesas: salvator: Add overlay for Dell P1911 VGA display
This patch adds an overlay to hardcode the EDID of a Dell P1911 VGA display
connector to the VGA port of the Salvator board.
> Add an overlay that hardcodes the EDID of the Dell P1911 VGA display
> connector to the VGA port of the Salvator board. This is meant to test
> EDID hardcoding and should not be merged upstream.
[Severity: Medium]
The commit message explicitly states this is meant to test EDID hardcoding
and should not be merged upstream.
Was this patch accidentally included in this series, or is it intended to be
dropped by the maintainer before final merging?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260705213542.28987-1-laurent.pinchart+renesas@ideasonboard.com?part=4
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] drm/bridge: display-connector: Support hardcoded EDID for VGA connectors
2026-07-05 21:35 ` [PATCH 3/4] drm/bridge: display-connector: Support hardcoded EDID for VGA connectors Laurent Pinchart
@ 2026-07-05 21:44 ` sashiko-bot
2026-07-05 23:38 ` Laurent Pinchart
0 siblings, 1 reply; 19+ messages in thread
From: sashiko-bot @ 2026-07-05 21:44 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: robh, dri-devel, devicetree, conor+dt, wsa+renesas
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free of devm-managed resources and missing asynchronous cleanup during device removal when the driver is unbound.
--
commit 48f63e147d7554f73cd9df360e29fd0b0ddb5d8d
Author: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
drm/bridge: display-connector: Support hardcoded EDID for VGA connectors
This commit adds support for parsing a hardcoded EDID blob from the device tree
for VGA connectors when DDC signals are not connected. It also ensures that
specifying both a DDC bus and a hardcoded EDID returns an error during probe.
> diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
> index eb3412ec30a07..225a8e987d086 100644
> --- a/drivers/gpu/drm/bridge/display-connector.c
> +++ b/drivers/gpu/drm/bridge/display-connector.c
[ ... ]
> @@ -64,6 +67,9 @@ static enum drm_connector_status display_connector_detect(struct drm_bridge *bri
> if (conn->bridge.ddc && drm_probe_ddc(conn->bridge.ddc))
> return connector_status_connected;
>
> + if (conn->edid)
> + return connector_status_connected;
> +
[Severity: High]
This isn't a bug introduced by this patch, but does device removal have a
use-after-free vulnerability when the driver is unbound via sysfs?
Looking at display_connector_detect() in
drivers/gpu/drm/bridge/display-connector.c, if the driver is bound and attached
to a DRM encoder, and then unbound via sysfs:
1. display_connector_remove() calls drm_bridge_remove() but does not call
disable_irq() or cancel_work_sync(&conn->hpd_work).
2. The bridge structure is kept alive because the DRM core still holds a
reference count.
3. The driver core's devres_release_all() runs and frees conn->hpd_gpio and
the IRQ.
If the DRM core then polls detect, it will execute display_connector_detect():
if (conn->hpd_gpio) {
if (gpiod_get_value_cansleep(conn->hpd_gpio))
...
Will this dereference the devres-freed conn->hpd_gpio?
Additionally, if the concurrent hpd_work executes, will it attempt to lock
bridge->hpd_mutex which was already destroyed by drm_bridge_remove()?
> switch (conn->bridge.type) {
> case DRM_MODE_CONNECTOR_DVIA:
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260705213542.28987-1-laurent.pinchart+renesas@ideasonboard.com?part=3
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak
2026-07-05 21:35 ` [PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak Laurent Pinchart
@ 2026-07-05 21:48 ` sashiko-bot
2026-07-05 23:38 ` Laurent Pinchart
0 siblings, 1 reply; 19+ messages in thread
From: sashiko-bot @ 2026-07-05 21:48 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: devicetree, conor+dt, wsa+renesas, robh, dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free of devm-managed resources due to DRM bridge lifetime mismatch
- [Medium] Hardware state leak of `ddc_en` GPIO on probe error paths
--
commit 9c76898e592dfddd3f50beaa692cc32ae295e2c7
Author: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
drm/bridge: display-connector: Fix I2C adapter resource leak
If the probe function returns an error after getting the I2C adapter for
DDC, the reference to the adapter is never released. Fix it by releasing
it in the bridge .destroy() handler.
> diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
> index d1e74dd2e0518..eb3412ec30a07 100644
> --- a/drivers/gpu/drm/bridge/display-connector.c
> +++ b/drivers/gpu/drm/bridge/display-connector.c
> @@ -43,6 +43,13 @@ static int display_connector_attach(struct drm_bridge *bridge,
> return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
> }
>
> +static void display_connector_destroy(struct drm_bridge *bridge)
> +{
> + struct display_connector *conn = to_display_connector(bridge);
> +
> + i2c_put_adapter(conn->bridge.ddc);
> +}
> +
> static enum drm_connector_status display_connector_detect(struct drm_bridge *bridge)
> {
> struct display_connector *conn = to_display_connector(bridge);
[ ... ]
> @@ -439,9 +447,6 @@ static void display_connector_remove(struct platform_device *pdev)
> regulator_disable(conn->supply);
>
> drm_bridge_remove(&conn->bridge);
> -
> - if (!IS_ERR(conn->bridge.ddc))
> - i2c_put_adapter(conn->bridge.ddc);
> }
>
> static const struct of_device_id display_connector_match[] = {
[Severity: High]
This isn't a bug introduced by this patch, but does the bridge lifetime
mismatch with devm resources cause a use-after-free?
In display_connector_probe(), the bridge is allocated with
devm_drm_bridge_alloc(), tying its lifetime to the DRM bridge kref:
display_connector_probe() {
...
conn = devm_drm_bridge_alloc(&pdev->dev, struct display_connector, bridge,
&display_connector_bridge_funcs);
...
}
However, devm-managed resources like the HPD GPIO and IRQ are tied to the
platform device unbind. If the device is unbound via sysfs while the DRM
core still holds a reference to the bridge, devres will free the GPIO and
IRQ resources.
If the DRM core later invokes callbacks like detect or hpd_disable, would it
dereference the freed GPIO or attempt to disable the freed IRQ?
display_connector_detect() {
...
if (gpiod_get_value_cansleep(conn->hpd_gpio))
...
}
display_connector_hpd_disable() {
...
disable_irq(conn->hpd_irq);
...
}
[Severity: Medium]
This is a pre-existing issue, but is there a hardware state leak on the
probe error path?
In display_connector_probe(), the DDC enable GPIO is acquired and
immediately asserted high:
display_connector_probe() {
...
if (type == DRM_MODE_CONNECTOR_HDMIA) {
...
conn->ddc_en = devm_gpiod_get_optional(&pdev->dev, "ddc-en",
GPIOD_OUT_HIGH);
if (IS_ERR(conn->ddc_en)) {
...
}
ret = display_connector_get_supply(pdev, conn, "hdmi-pwr");
if (ret < 0)
return dev_err_probe(...);
...
}
If a subsequent step like display_connector_get_supply() or
regulator_enable() fails, the function returns an error. While devres will
free the GPIO descriptor, it doesn't revert the physical pin state.
Unlike display_connector_remove() which explicitly sets the GPIO to 0, does
this error path leave the DDC bus permanently enabled?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260705213542.28987-1-laurent.pinchart+renesas@ideasonboard.com?part=2
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak
2026-07-05 21:48 ` sashiko-bot
@ 2026-07-05 23:38 ` Laurent Pinchart
0 siblings, 0 replies; 19+ messages in thread
From: Laurent Pinchart @ 2026-07-05 23:38 UTC (permalink / raw)
To: sashiko-reviews; +Cc: devicetree, conor+dt, wsa+renesas, robh, dri-devel
Hello,
This is the friendly mail bot of Laurent Pinchart. You have sent him
what appears to be an e-mail generated by an LLM. In order to preserve
his mental health, the e-mail has been automatically ignored.
Thank you,
Laurent's spam bot
On Sun, 05 Jul 2026 21:48:25 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Use-After-Free of devm-managed resources due to DRM bridge lifetime mismatch
> - [Medium] Hardware state leak of `ddc_en` GPIO on probe error paths
> --
>
> commit 9c76898e592dfddd3f50beaa692cc32ae295e2c7
> Author: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> drm/bridge: display-connector: Fix I2C adapter resource leak
>
> If the probe function returns an error after getting the I2C adapter for
> DDC, the reference to the adapter is never released. Fix it by releasing
> it in the bridge .destroy() handler.
>
> > diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
> > index d1e74dd2e0518..eb3412ec30a07 100644
> > --- a/drivers/gpu/drm/bridge/display-connector.c
> > +++ b/drivers/gpu/drm/bridge/display-connector.c
> > @@ -43,6 +43,13 @@ static int display_connector_attach(struct drm_bridge *bridge,
> > return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
> > }
> >
> > +static void display_connector_destroy(struct drm_bridge *bridge)
> > +{
> > + struct display_connector *conn = to_display_connector(bridge);
> > +
> > + i2c_put_adapter(conn->bridge.ddc);
> > +}
> > +
> > static enum drm_connector_status display_connector_detect(struct drm_bridge *bridge)
> > {
> > struct display_connector *conn = to_display_connector(bridge);
>
> [ ... ]
>
> > @@ -439,9 +447,6 @@ static void display_connector_remove(struct platform_device *pdev)
> > regulator_disable(conn->supply);
> >
> > drm_bridge_remove(&conn->bridge);
> > -
> > - if (!IS_ERR(conn->bridge.ddc))
> > - i2c_put_adapter(conn->bridge.ddc);
> > }
> >
> > static const struct of_device_id display_connector_match[] = {
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does the bridge lifetime
> mismatch with devm resources cause a use-after-free?
>
> In display_connector_probe(), the bridge is allocated with
> devm_drm_bridge_alloc(), tying its lifetime to the DRM bridge kref:
>
> display_connector_probe() {
> ...
> conn = devm_drm_bridge_alloc(&pdev->dev, struct display_connector, bridge,
> &display_connector_bridge_funcs);
> ...
> }
>
> However, devm-managed resources like the HPD GPIO and IRQ are tied to the
> platform device unbind. If the device is unbound via sysfs while the DRM
> core still holds a reference to the bridge, devres will free the GPIO and
> IRQ resources.
>
> If the DRM core later invokes callbacks like detect or hpd_disable, would it
> dereference the freed GPIO or attempt to disable the freed IRQ?
>
> display_connector_detect() {
> ...
> if (gpiod_get_value_cansleep(conn->hpd_gpio))
> ...
> }
>
> display_connector_hpd_disable() {
> ...
> disable_irq(conn->hpd_irq);
> ...
> }
>
> [Severity: Medium]
> This is a pre-existing issue, but is there a hardware state leak on the
> probe error path?
>
> In display_connector_probe(), the DDC enable GPIO is acquired and
> immediately asserted high:
>
> display_connector_probe() {
> ...
> if (type == DRM_MODE_CONNECTOR_HDMIA) {
> ...
> conn->ddc_en = devm_gpiod_get_optional(&pdev->dev, "ddc-en",
> GPIOD_OUT_HIGH);
>
> if (IS_ERR(conn->ddc_en)) {
> ...
> }
>
> ret = display_connector_get_supply(pdev, conn, "hdmi-pwr");
> if (ret < 0)
> return dev_err_probe(...);
> ...
> }
>
> If a subsequent step like display_connector_get_supply() or
> regulator_enable() fails, the function returns an error. While devres will
> free the GPIO descriptor, it doesn't revert the physical pin state.
>
> Unlike display_connector_remove() which explicitly sets the GPIO to 0, does
> this error path leave the DDC bus permanently enabled?
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] drm/bridge: display-connector: Support hardcoded EDID for VGA connectors
2026-07-05 21:44 ` sashiko-bot
@ 2026-07-05 23:38 ` Laurent Pinchart
0 siblings, 0 replies; 19+ messages in thread
From: Laurent Pinchart @ 2026-07-05 23:38 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, dri-devel, devicetree, conor+dt, wsa+renesas
Hello,
This is the friendly mail bot of Laurent Pinchart. You have sent him
what appears to be an e-mail generated by an LLM. In order to preserve
his mental health, the e-mail has been automatically ignored.
Thank you,
Laurent's spam bot
On Sun, 05 Jul 2026 21:44:52 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Use-After-Free of devm-managed resources and missing asynchronous cleanup during device removal when the driver is unbound.
> --
>
> commit 48f63e147d7554f73cd9df360e29fd0b0ddb5d8d
> Author: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> drm/bridge: display-connector: Support hardcoded EDID for VGA connectors
>
> This commit adds support for parsing a hardcoded EDID blob from the device tree
> for VGA connectors when DDC signals are not connected. It also ensures that
> specifying both a DDC bus and a hardcoded EDID returns an error during probe.
>
> > diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
> > index eb3412ec30a07..225a8e987d086 100644
> > --- a/drivers/gpu/drm/bridge/display-connector.c
> > +++ b/drivers/gpu/drm/bridge/display-connector.c
>
> [ ... ]
>
> > @@ -64,6 +67,9 @@ static enum drm_connector_status display_connector_detect(struct drm_bridge *bri
> > if (conn->bridge.ddc && drm_probe_ddc(conn->bridge.ddc))
> > return connector_status_connected;
> >
> > + if (conn->edid)
> > + return connector_status_connected;
> > +
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does device removal have a
> use-after-free vulnerability when the driver is unbound via sysfs?
>
> Looking at display_connector_detect() in
> drivers/gpu/drm/bridge/display-connector.c, if the driver is bound and attached
> to a DRM encoder, and then unbound via sysfs:
>
> 1. display_connector_remove() calls drm_bridge_remove() but does not call
> disable_irq() or cancel_work_sync(&conn->hpd_work).
> 2. The bridge structure is kept alive because the DRM core still holds a
> reference count.
> 3. The driver core's devres_release_all() runs and frees conn->hpd_gpio and
> the IRQ.
>
> If the DRM core then polls detect, it will execute display_connector_detect():
>
> if (conn->hpd_gpio) {
> if (gpiod_get_value_cansleep(conn->hpd_gpio))
> ...
>
> Will this dereference the devres-freed conn->hpd_gpio?
>
> Additionally, if the concurrent hpd_work executes, will it attempt to lock
> bridge->hpd_mutex which was already destroyed by drm_bridge_remove()?
>
> > switch (conn->bridge.type) {
> > case DRM_MODE_CONNECTOR_DVIA:
>
> [ ... ]
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] [DNI] arm64: dts: renesas: salvator: Add overlay for Dell P1911 VGA display
2026-07-05 21:37 ` sashiko-bot
@ 2026-07-05 23:39 ` Laurent Pinchart
0 siblings, 0 replies; 19+ messages in thread
From: Laurent Pinchart @ 2026-07-05 23:39 UTC (permalink / raw)
To: sashiko-reviews; +Cc: devicetree, dri-devel, robh, conor+dt, wsa+renesas
Hello,
This is the friendly mail bot of Laurent Pinchart. You have sent him
what appears to be an e-mail generated by an LLM. In order to preserve
his mental health, the e-mail has been automatically ignored.
Thank you,
Laurent's spam bot
On Sun, 05 Jul 2026 21:37:40 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch is explicitly marked as test-only and should not be merged.
> --
>
> commit ec00db7021fbc2f550a150a0512dfc7596a53df2
> Author: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> arm64: dts: renesas: salvator: Add overlay for Dell P1911 VGA display
>
> This patch adds an overlay to hardcode the EDID of a Dell P1911 VGA display
> connector to the VGA port of the Salvator board.
>
> > Add an overlay that hardcodes the EDID of the Dell P1911 VGA display
> > connector to the VGA port of the Salvator board. This is meant to test
> > EDID hardcoding and should not be merged upstream.
>
> [Severity: Medium]
> The commit message explicitly states this is meant to test EDID hardcoding
> and should not be merged upstream.
>
> Was this patch accidentally included in this series, or is it intended to be
> dropped by the maintainer before final merging?
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] dt-bindings: display: vga-connector: Allow hardcoding EDID
2026-07-05 21:35 ` [PATCH 1/4] dt-bindings: display: vga-connector: Allow hardcoding EDID Laurent Pinchart
@ 2026-07-06 8:08 ` Geert Uytterhoeven
2026-07-06 10:03 ` Laurent Pinchart
2026-07-06 8:52 ` Maxime Ripard
1 sibling, 1 reply; 19+ messages in thread
From: Geert Uytterhoeven @ 2026-07-06 8:08 UTC (permalink / raw)
To: Laurent Pinchart
Cc: dri-devel, devicetree, Thuan Nguyen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Andrzej Hajda, Neil Armstrong, Robert Foss,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, linux-renesas-soc
Hi Laurent,
On Sun, 5 Jul 2026 at 23:35, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Since DDC version 2, introduced in 1996, VGA monitors have exposed EDID
> data over an I2C bus. The bus is also used to detect the presence of a
> connected monitor by trying to read the EDID data.
>
> Some devices where the VGA display is integrated in the device and
> always connected do not connect the DDC pins. Some development boards,
> such as the Renesas M3N Salvator-XS, also do not connect the DDC pins.
>
> To support those, add the ability to provide hardcoded EDID data in the
> device tree. This is mutually exclusive with specifying a DDC bus, and
> can only be done when the VGA display is guaranteed to be always
> connected.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Thanks for your patch!
> --- a/Documentation/devicetree/bindings/display/connector/vga-connector.yaml
> +++ b/Documentation/devicetree/bindings/display/connector/vga-connector.yaml
> @@ -19,10 +19,25 @@ properties:
> description: phandle link to the I2C controller used for DDC EDID probing
> $ref: /schemas/types.yaml#/definitions/phandle
>
> + edid:
> + $ref: /schemas/types.yaml#/definitions/uint8-array
> + description:
> + When the DDC signals are not wired to the connector, and the connected
> + display is not removable, this property is used to supply a binary EDID
> + blob for the display.
> +
> port:
> $ref: /schemas/graph.yaml#/properties/port
> description: Connection to controller providing VGA signals
>
> +allOf:
> + - if:
> + required:
> + - ddc-i2c-bus
> + then:
> + properties:
> + edid: false
> +
What if the DDC signals are wired to the connector on the provider side,
but not on the consumer side?
A DT overlay describing the consumer device can add an edid property,
but it cannot delete the ddc-i2c-bus property in the base DTB.
> required:
> - compatible
> - port
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] [DNI] arm64: dts: renesas: salvator: Add overlay for Dell P1911 VGA display
2026-07-05 21:35 ` [PATCH 4/4] [DNI] arm64: dts: renesas: salvator: Add overlay for Dell P1911 VGA display Laurent Pinchart
2026-07-05 21:37 ` sashiko-bot
@ 2026-07-06 8:16 ` Geert Uytterhoeven
2026-07-06 9:48 ` Laurent Pinchart
1 sibling, 1 reply; 19+ messages in thread
From: Geert Uytterhoeven @ 2026-07-06 8:16 UTC (permalink / raw)
To: Laurent Pinchart
Cc: dri-devel, devicetree, Thuan Nguyen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Andrzej Hajda, Neil Armstrong, Robert Foss,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, linux-renesas-soc
Hi Laurent,
On Sun, 5 Jul 2026 at 23:35, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Add an overlay that hardcodes the EDID of the Dell P1911 VGA display
> connector to the VGA port of the Salvator board. This is meant to test
> EDID hardcoding and should not be merged upstream.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Thanks for your patch!
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/salvator-vga-dell-p1911.dtso
I know this is not meant to be applied, but nothing below is
Salvator-specific (except for the comment). Hence this can be applied
to any "vga" connector, and I'd rather drop the "salvator-"-prefix
from the file name.
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Device Tree overlay for the Dell P1911 monitor connected to VGA port of a
> + * Salvator-X or Salvator-XS board
> + *
> + * Copyright 2026 Ideas on Board Oy
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +&{/vga} {
> + edid = [00 ff ff ff ff ff ff 00 10 ac 74 a0 55 4b 55 30
> + 2b 14 01 03 80 29 1a 78 ee ee 95 a3 54 4c 99 26
> + 0f 50 54 bf ef 80 95 00 71 4f 81 80 95 0f 81 00
> + 01 01 01 01 01 01 9a 29 a0 d0 51 84 22 30 50 98
> + 36 00 98 ff 10 00 00 1c 00 00 00 ff 00 31 57 4b
> + 4a 56 30 41 4b 30 55 4b 55 0a 00 00 00 fc 00 44
> + 45 4c 4c 20 50 31 39 31 31 0a 20 20 00 00 00 fd
> + 00 38 4b 1e 53 0e 00 0a 20 20 20 20 20 20 00 ba];
> +};
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] dt-bindings: display: vga-connector: Allow hardcoding EDID
2026-07-05 21:35 ` [PATCH 1/4] dt-bindings: display: vga-connector: Allow hardcoding EDID Laurent Pinchart
2026-07-06 8:08 ` Geert Uytterhoeven
@ 2026-07-06 8:52 ` Maxime Ripard
2026-07-06 9:45 ` Laurent Pinchart
1 sibling, 1 reply; 19+ messages in thread
From: Maxime Ripard @ 2026-07-06 8:52 UTC (permalink / raw)
To: Laurent Pinchart
Cc: dri-devel, devicetree, Thuan Nguyen, Maarten Lankhorst,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Andrzej Hajda, Neil Armstrong,
Robert Foss, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 1005 bytes --]
Hi,
On Mon, Jul 06, 2026 at 12:35:39AM +0300, Laurent Pinchart wrote:
> Since DDC version 2, introduced in 1996, VGA monitors have exposed EDID
> data over an I2C bus. The bus is also used to detect the presence of a
> connected monitor by trying to read the EDID data.
>
> Some devices where the VGA display is integrated in the device and
> always connected do not connect the DDC pins. Some development boards,
> such as the Renesas M3N Salvator-XS, also do not connect the DDC pins.
>
> To support those, add the ability to provide hardcoded EDID data in the
> device tree. This is mutually exclusive with specifying a DDC bus, and
> can only be done when the VGA display is guaranteed to be always
> connected.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
This feels redundant with the EDID firmware loading mechanism we have in
KMS already. It should at least be mentioned why we would need to set it
in the device tree at all.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] dt-bindings: display: vga-connector: Allow hardcoding EDID
2026-07-06 8:52 ` Maxime Ripard
@ 2026-07-06 9:45 ` Laurent Pinchart
2026-07-06 15:40 ` Maxime Ripard
0 siblings, 1 reply; 19+ messages in thread
From: Laurent Pinchart @ 2026-07-06 9:45 UTC (permalink / raw)
To: Maxime Ripard
Cc: dri-devel, devicetree, Thuan Nguyen, Maarten Lankhorst,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Andrzej Hajda, Neil Armstrong,
Robert Foss, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
linux-renesas-soc
Hi Maxime,
On Mon, Jul 06, 2026 at 10:52:32AM +0200, Maxime Ripard wrote:
> On Mon, Jul 06, 2026 at 12:35:39AM +0300, Laurent Pinchart wrote:
> > Since DDC version 2, introduced in 1996, VGA monitors have exposed EDID
> > data over an I2C bus. The bus is also used to detect the presence of a
> > connected monitor by trying to read the EDID data.
> >
> > Some devices where the VGA display is integrated in the device and
> > always connected do not connect the DDC pins. Some development boards,
> > such as the Renesas M3N Salvator-XS, also do not connect the DDC pins.
> >
> > To support those, add the ability to provide hardcoded EDID data in the
> > device tree. This is mutually exclusive with specifying a DDC bus, and
> > can only be done when the VGA display is guaranteed to be always
> > connected.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> This feels redundant with the EDID firmware loading mechanism we have in
> KMS already. It should at least be mentioned why we would need to set it
> in the device tree at all.
Very good question.
I assume you're talking about CONFIG_DRM_LOAD_EDID_FIRMWARE, as the
debugfs override_edid feature isn't meant for production. If there are
other relevant mechanisms I'm not aware of, please let me know.
In the use case at hand, the VGA display is an integral part of the
device, the same way an LVDS or DSI panel would be. Using
CONFIG_DRM_LOAD_EDID_FIRMWARE, the manufacturer would need to set the
drm.edid_firmware command line parameter, and provide EDID as a file in
/lib/firmware/ (possibly in an initramfs). Beside the complexity, and
the fact it won't be very friendly to people who run a different
userspace on the device, I think EDID counts in this case as system
description, the same way we support specifying panel timings in device
tree.
Does this make sense ? If so, I'll update the commit message accordingly
in v2.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] [DNI] arm64: dts: renesas: salvator: Add overlay for Dell P1911 VGA display
2026-07-06 8:16 ` Geert Uytterhoeven
@ 2026-07-06 9:48 ` Laurent Pinchart
0 siblings, 0 replies; 19+ messages in thread
From: Laurent Pinchart @ 2026-07-06 9:48 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: dri-devel, devicetree, Thuan Nguyen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Andrzej Hajda, Neil Armstrong, Robert Foss,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, linux-renesas-soc
On Mon, Jul 06, 2026 at 10:16:22AM +0200, Geert Uytterhoeven wrote:
> On Sun, 5 Jul 2026 at 23:35, Laurent Pinchart wrote:
> > Add an overlay that hardcodes the EDID of the Dell P1911 VGA display
> > connector to the VGA port of the Salvator board. This is meant to test
> > EDID hardcoding and should not be merged upstream.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/renesas/salvator-vga-dell-p1911.dtso
>
> I know this is not meant to be applied, but nothing below is
> Salvator-specific (except for the comment). Hence this can be applied
> to any "vga" connector, and I'd rather drop the "salvator-"-prefix
> from the file name.
Sure, but it won't be merged :-)
> > @@ -0,0 +1,21 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Device Tree overlay for the Dell P1911 monitor connected to VGA port of a
> > + * Salvator-X or Salvator-XS board
> > + *
> > + * Copyright 2026 Ideas on Board Oy
> > + */
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +
> > +&{/vga} {
> > + edid = [00 ff ff ff ff ff ff 00 10 ac 74 a0 55 4b 55 30
> > + 2b 14 01 03 80 29 1a 78 ee ee 95 a3 54 4c 99 26
> > + 0f 50 54 bf ef 80 95 00 71 4f 81 80 95 0f 81 00
> > + 01 01 01 01 01 01 9a 29 a0 d0 51 84 22 30 50 98
> > + 36 00 98 ff 10 00 00 1c 00 00 00 ff 00 31 57 4b
> > + 4a 56 30 41 4b 30 55 4b 55 0a 00 00 00 fc 00 44
> > + 45 4c 4c 20 50 31 39 31 31 0a 20 20 00 00 00 fd
> > + 00 38 4b 1e 53 0e 00 0a 20 20 20 20 20 20 00 ba];
> > +};
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] dt-bindings: display: vga-connector: Allow hardcoding EDID
2026-07-06 8:08 ` Geert Uytterhoeven
@ 2026-07-06 10:03 ` Laurent Pinchart
0 siblings, 0 replies; 19+ messages in thread
From: Laurent Pinchart @ 2026-07-06 10:03 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: dri-devel, devicetree, Thuan Nguyen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Andrzej Hajda, Neil Armstrong, Robert Foss,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, linux-renesas-soc
On Mon, Jul 06, 2026 at 10:08:57AM +0200, Geert Uytterhoeven wrote:
> On Sun, 5 Jul 2026 at 23:35, Laurent Pinchart wrote:
> > Since DDC version 2, introduced in 1996, VGA monitors have exposed EDID
> > data over an I2C bus. The bus is also used to detect the presence of a
> > connected monitor by trying to read the EDID data.
> >
> > Some devices where the VGA display is integrated in the device and
> > always connected do not connect the DDC pins. Some development boards,
> > such as the Renesas M3N Salvator-XS, also do not connect the DDC pins.
> >
> > To support those, add the ability to provide hardcoded EDID data in the
> > device tree. This is mutually exclusive with specifying a DDC bus, and
> > can only be done when the VGA display is guaranteed to be always
> > connected.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>
> Thanks for your patch!
>
> > --- a/Documentation/devicetree/bindings/display/connector/vga-connector.yaml
> > +++ b/Documentation/devicetree/bindings/display/connector/vga-connector.yaml
> > @@ -19,10 +19,25 @@ properties:
> > description: phandle link to the I2C controller used for DDC EDID probing
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
> > + edid:
> > + $ref: /schemas/types.yaml#/definitions/uint8-array
> > + description:
> > + When the DDC signals are not wired to the connector, and the connected
> > + display is not removable, this property is used to supply a binary EDID
> > + blob for the display.
> > +
> > port:
> > $ref: /schemas/graph.yaml#/properties/port
> > description: Connection to controller providing VGA signals
> >
> > +allOf:
> > + - if:
> > + required:
> > + - ddc-i2c-bus
> > + then:
> > + properties:
> > + edid: false
> > +
>
> What if the DDC signals are wired to the connector on the provider side,
> but not on the consumer side?
> A DT overlay describing the consumer device can add an edid property,
> but it cannot delete the ddc-i2c-bus property in the base DTB.
As the use case is devices where the VGA display is an integral part of
the device, I don't this this is a big issue. I would be quite surprised
if a manufacturer wanted to support different SKUs with multiple VGA
display overlays, with the DDC signals wired up in the base board on all
SKUs, and on the VGA display side in some SKUs only. If that happened,
they could set the ddc-i2c-bus property in the overlays that wire the
signals, implement /delete-property/ support for overlays (would be an
interesting feature in my opinion), or as a very last resort propose
relaxing the constraint here.
> > required:
> > - compatible
> > - port
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] dt-bindings: display: vga-connector: Allow hardcoding EDID
2026-07-06 9:45 ` Laurent Pinchart
@ 2026-07-06 15:40 ` Maxime Ripard
2026-07-06 16:02 ` Laurent Pinchart
0 siblings, 1 reply; 19+ messages in thread
From: Maxime Ripard @ 2026-07-06 15:40 UTC (permalink / raw)
To: Laurent Pinchart
Cc: dri-devel, devicetree, Thuan Nguyen, Maarten Lankhorst,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Andrzej Hajda, Neil Armstrong,
Robert Foss, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 2513 bytes --]
On Mon, Jul 06, 2026 at 12:45:22PM +0300, Laurent Pinchart wrote:
> On Mon, Jul 06, 2026 at 10:52:32AM +0200, Maxime Ripard wrote:
> > On Mon, Jul 06, 2026 at 12:35:39AM +0300, Laurent Pinchart wrote:
> > > Since DDC version 2, introduced in 1996, VGA monitors have exposed EDID
> > > data over an I2C bus. The bus is also used to detect the presence of a
> > > connected monitor by trying to read the EDID data.
> > >
> > > Some devices where the VGA display is integrated in the device and
> > > always connected do not connect the DDC pins. Some development boards,
> > > such as the Renesas M3N Salvator-XS, also do not connect the DDC pins.
> > >
> > > To support those, add the ability to provide hardcoded EDID data in the
> > > device tree. This is mutually exclusive with specifying a DDC bus, and
> > > can only be done when the VGA display is guaranteed to be always
> > > connected.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >
> > This feels redundant with the EDID firmware loading mechanism we have in
> > KMS already. It should at least be mentioned why we would need to set it
> > in the device tree at all.
>
> Very good question.
>
> I assume you're talking about CONFIG_DRM_LOAD_EDID_FIRMWARE, as the
> debugfs override_edid feature isn't meant for production. If there are
> other relevant mechanisms I'm not aware of, please let me know.
No, that's indeed what I meant.
> In the use case at hand, the VGA display is an integral part of the
> device, the same way an LVDS or DSI panel would be. Using
> CONFIG_DRM_LOAD_EDID_FIRMWARE, the manufacturer would need to set the
> drm.edid_firmware command line parameter, and provide EDID as a file in
> /lib/firmware/ (possibly in an initramfs). Beside the complexity, and
> the fact it won't be very friendly to people who run a different
> userspace on the device, I think EDID counts in this case as system
> description, the same way we support specifying panel timings in device
> tree.
It's *some* hardware description, but you have no idea whether it's
actually the hardware you're running from. What would be in that EDID
anyway?
There's another alternative we've used several times already, in
simple-bridge for example: just register any VESA mode up to a given
resolution:
https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/bridge/simple-bridge.c#L66
I guess it's what you would do with that EDID anyway?
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] dt-bindings: display: vga-connector: Allow hardcoding EDID
2026-07-06 15:40 ` Maxime Ripard
@ 2026-07-06 16:02 ` Laurent Pinchart
0 siblings, 0 replies; 19+ messages in thread
From: Laurent Pinchart @ 2026-07-06 16:02 UTC (permalink / raw)
To: Maxime Ripard
Cc: dri-devel, devicetree, Thuan Nguyen, Maarten Lankhorst,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Andrzej Hajda, Neil Armstrong,
Robert Foss, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
linux-renesas-soc
On Mon, Jul 06, 2026 at 05:40:30PM +0200, Maxime Ripard wrote:
> On Mon, Jul 06, 2026 at 12:45:22PM +0300, Laurent Pinchart wrote:
> > On Mon, Jul 06, 2026 at 10:52:32AM +0200, Maxime Ripard wrote:
> > > On Mon, Jul 06, 2026 at 12:35:39AM +0300, Laurent Pinchart wrote:
> > > > Since DDC version 2, introduced in 1996, VGA monitors have exposed EDID
> > > > data over an I2C bus. The bus is also used to detect the presence of a
> > > > connected monitor by trying to read the EDID data.
> > > >
> > > > Some devices where the VGA display is integrated in the device and
> > > > always connected do not connect the DDC pins. Some development boards,
> > > > such as the Renesas M3N Salvator-XS, also do not connect the DDC pins.
> > > >
> > > > To support those, add the ability to provide hardcoded EDID data in the
> > > > device tree. This is mutually exclusive with specifying a DDC bus, and
> > > > can only be done when the VGA display is guaranteed to be always
> > > > connected.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > >
> > > This feels redundant with the EDID firmware loading mechanism we have in
> > > KMS already. It should at least be mentioned why we would need to set it
> > > in the device tree at all.
> >
> > Very good question.
> >
> > I assume you're talking about CONFIG_DRM_LOAD_EDID_FIRMWARE, as the
> > debugfs override_edid feature isn't meant for production. If there are
> > other relevant mechanisms I'm not aware of, please let me know.
>
> No, that's indeed what I meant.
>
> > In the use case at hand, the VGA display is an integral part of the
> > device, the same way an LVDS or DSI panel would be. Using
> > CONFIG_DRM_LOAD_EDID_FIRMWARE, the manufacturer would need to set the
> > drm.edid_firmware command line parameter, and provide EDID as a file in
> > /lib/firmware/ (possibly in an initramfs). Beside the complexity, and
> > the fact it won't be very friendly to people who run a different
> > userspace on the device, I think EDID counts in this case as system
> > description, the same way we support specifying panel timings in device
> > tree.
>
> It's *some* hardware description, but you have no idea whether it's
> actually the hardware you're running from. What would be in that EDID
> anyway?
It would be the EDID corresponding to the connected display :-) I see it
as identical to how a user of the simple panel driver is responsible for
listing the timings corresponding to the hardware in the device tree.
> There's another alternative we've used several times already, in
> simple-bridge for example: just register any VESA mode up to a given
> resolution:
> https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/bridge/simple-bridge.c#L66
I don't see how that would help. The goal is to provide the kernel with
the data corresponding to the display integrated in the device.
> I guess it's what you would do with that EDID anyway?
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2026-07-06 16:02 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-05 21:35 [PATCH 0/4] drm: bridge: display-connector: Support hardcoding EDID for VGA Laurent Pinchart
2026-07-05 21:35 ` [PATCH 1/4] dt-bindings: display: vga-connector: Allow hardcoding EDID Laurent Pinchart
2026-07-06 8:08 ` Geert Uytterhoeven
2026-07-06 10:03 ` Laurent Pinchart
2026-07-06 8:52 ` Maxime Ripard
2026-07-06 9:45 ` Laurent Pinchart
2026-07-06 15:40 ` Maxime Ripard
2026-07-06 16:02 ` Laurent Pinchart
2026-07-05 21:35 ` [PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak Laurent Pinchart
2026-07-05 21:48 ` sashiko-bot
2026-07-05 23:38 ` Laurent Pinchart
2026-07-05 21:35 ` [PATCH 3/4] drm/bridge: display-connector: Support hardcoded EDID for VGA connectors Laurent Pinchart
2026-07-05 21:44 ` sashiko-bot
2026-07-05 23:38 ` Laurent Pinchart
2026-07-05 21:35 ` [PATCH 4/4] [DNI] arm64: dts: renesas: salvator: Add overlay for Dell P1911 VGA display Laurent Pinchart
2026-07-05 21:37 ` sashiko-bot
2026-07-05 23:39 ` Laurent Pinchart
2026-07-06 8:16 ` Geert Uytterhoeven
2026-07-06 9:48 ` Laurent Pinchart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox