drm: conversions to struct drm_edid

PATCH In review Jani Nikula 5 participants 19 messages Aug 22, 2024 – Sep 4, 2024

Subsystems (10)

DRM drivers DRM misc Arm i.MX and Layerscape Arm Samsung Exynos +6 more subsystems

lore

Share & tools

lore

Jani Nikula

author

2024-08-22 17:43 UTC #0 lore raw

cover drm: conversions to struct drm_edid

The umerged patches from [1] and then some.

BR,
Jani.

[1] https://lore.kernel.org/r/[email protected]

Jani Nikula (6):
drm/sti/sti_hdmi: convert to struct drm_edid drm/exynos: hdmi: use display_info for printing display dimensions drm/exynos: hdmi: convert to struct drm_edid
drm/tegra: convert to struct drm_edid drm/ipuv3/parallel: convert to struct drm_edid drm/tiny/gm12u320: convert to struct drm_edid

 drivers/gpu/drm/exynos/exynos_hdmi.c         | 25 ++++++++++-------
 drivers/gpu/drm/imx/ipuv3/parallel-display.c | 14 ++++++----
 drivers/gpu/drm/sti/sti_hdmi.c               | 24 +++++++++-------
 drivers/gpu/drm/tegra/drm.h                  |  2 +-
 drivers/gpu/drm/tegra/output.c               | 29 ++++++++++++--------
 drivers/gpu/drm/tiny/gm12u320.c              | 13 +++++++--
 6 files changed, 66 insertions(+), 41 deletions(-)

Signature · 1 line

2.39.2

Jani Nikula

author

2024-08-22 17:43 UTC #1 lore raw

PATCH 1/6 drm/sti/sti_hdmi: convert to struct drm_edid

Prefer the struct drm_edid based functions for reading the EDID and updating the connector.

The functional change is that the CEC physical address gets invalidated when the EDID could not be read.

v2: Use drm_edid_read() instead of drm_edid_read_ddc() (Sima)

Reviewed-by Daniel Vetter <[email protected]>

Signed-off-by Jani Nikula <[email protected]>

---

Cc Alain Volmat <[email protected]>

Cc Maarten Lankhorst <[email protected]>

Cc Maxime Ripard <[email protected]>

Cc Thomas Zimmermann <[email protected]>

---
 drivers/gpu/drm/sti/sti_hdmi.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

drivers/gpu/drm/sti/sti_hdmi.c +14 ~~-10~~

index b0d84440a87b..847470f747c0 100644
@@ -974,28 +974,32 @@ static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = {
 
 static int sti_hdmi_connector_get_modes(struct drm_connector *connector)
 {
+	const struct drm_display_info *info = &connector->display_info;
 	struct sti_hdmi_connector *hdmi_connector
 		= to_sti_hdmi_connector(connector);
 	struct sti_hdmi *hdmi = hdmi_connector->hdmi;
-	struct edid *edid;
+	const struct drm_edid *drm_edid;
 	int count;
 
 	DRM_DEBUG_DRIVER("\n");
 
-	edid = drm_get_edid(connector, hdmi->ddc_adapt);
-	if (!edid)
-		goto fail;
+	drm_edid = drm_edid_read(connector);
+
+	drm_edid_connector_update(connector, drm_edid);
 
-	cec_notifier_set_phys_addr_from_edid(hdmi->notifier, edid);
+	cec_notifier_set_phys_addr(hdmi->notifier,
+				   connector->display_info.source_physical_address);
+
+	if (!drm_edid)
+		goto fail;
 
-	count = drm_add_edid_modes(connector, edid);
-	drm_connector_update_edid_property(connector, edid);
+	count = drm_edid_connector_add_modes(connector);
 
 	DRM_DEBUG_KMS("%s : %dx%d cm\n",
-		      (connector->display_info.is_hdmi ? "hdmi monitor" : "dvi monitor"),
-		      edid->width_cm, edid->height_cm);
+		      info->is_hdmi ? "hdmi monitor" : "dvi monitor",
+		      info->width_mm / 10, info->height_mm / 10);
 
-	kfree(edid);
+	drm_edid_free(drm_edid);
 	return count;
 
 fail:

Signature · 1 line

2.39.2

Jani Nikula

author

2024-08-22 17:43 UTC #2 lore raw

PATCH 2/6 drm/exynos: hdmi: use display_info for printing display dimensions

In reply to Jani Nikula

Look up display dimensions from display_info instead of the EDID directly. This will be helpful for the follow-up work.

Signed-off-by Jani Nikula <[email protected]>

---

Cc Inki Dae <[email protected]>

Cc Seung-Woo Kim <[email protected]>

Cc Kyungmin Park <[email protected]>

Cc Krzysztof Kozlowski <[email protected]>

Cc Alim Akhtar <[email protected]>

Cc [email protected]

---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/gpu/drm/exynos/exynos_hdmi.c +3 -2

index 1e26cd4f8347..25d508b25921 100644
@@ -883,6 +883,7 @@ static const struct drm_connector_funcs hdmi_connector_funcs = {
 static int hdmi_get_modes(struct drm_connector *connector)
 {
 	struct hdmi_context *hdata = connector_to_hdmi(connector);
+	const struct drm_display_info *info = &connector->display_info;
 	struct edid *edid;
 	int ret;

@@ -893,10 +894,10 @@ static int hdmi_get_modes(struct drm_connector *connector)
 	if (!edid)
 		goto no_edid;
 
-	hdata->dvi_mode = !connector->display_info.is_hdmi;
+	hdata->dvi_mode = !info->is_hdmi;
 	DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n",
 			  (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"),
-			  edid->width_cm, edid->height_cm);
+			  info->width_mm / 10, info->height_mm / 10);
 
 	drm_connector_update_edid_property(connector, edid);
 	cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid);

Signature · 1 line

2.39.2

Daniel Vetter

Reviewed-by on v1 2/6

2024-08-28 12:44 UTC #3 lore raw

Re: [PATCH 2/6] drm/exynos: hdmi: use display_info for printing display dimensions

Quoted 4 lines · Jani Nikula

Look up display dimensions from display_info instead of the EDID directly. This will be helpful for the follow-up work.
Signed-off-by Jani Nikula <[email protected]>

Reviewed-by Daniel Vetter <[email protected]>

Quoted 42 lines

---

Cc Inki Dae <[email protected]>

Cc Seung-Woo Kim <[email protected]>

Cc Kyungmin Park <[email protected]>

Cc Krzysztof Kozlowski <[email protected]>

Cc Alim Akhtar <[email protected]>

Cc [email protected]

---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/gpu/drm/exynos/exynos_hdmi.c +3 -2

index 1e26cd4f8347..25d508b25921 100644
@@ -883,6 +883,7 @@ static const struct drm_connector_funcs hdmi_connector_funcs = {
 static int hdmi_get_modes(struct drm_connector *connector)
 {
 	struct hdmi_context *hdata = connector_to_hdmi(connector);
+	const struct drm_display_info *info = &connector->display_info;
 	struct edid *edid;
 	int ret;

@@ -893,10 +894,10 @@ static int hdmi_get_modes(struct drm_connector *connector)
 	if (!edid)
 		goto no_edid;
 
-	hdata->dvi_mode = !connector->display_info.is_hdmi;
+	hdata->dvi_mode = !info->is_hdmi;
 	DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n",
 			  (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"),
-			  edid->width_cm, edid->height_cm);
+			  info->width_mm / 10, info->height_mm / 10);
 
 	drm_connector_update_edid_property(connector, edid);
 	cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid);

Signature · 1 line

2.39.2

Signature · 3 lines

Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Jani Nikula

author

2024-08-22 17:43 UTC #4 lore raw

PATCH 3/6 drm/exynos: hdmi: convert to struct drm_edid

In reply to Jani Nikula

Prefer the struct drm_edid based functions for reading the EDID and updating the connector.

The functional change is that the CEC physical address gets invalidated when the EDID could not be read.

v2:
- display info usage was split to a separate patch
- check drm_edid_connector_update() return value

Signed-off-by Jani Nikula <[email protected]>

---

Cc Inki Dae <[email protected]>

Cc Seung-Woo Kim <[email protected]>

Cc Kyungmin Park <[email protected]>

Cc Krzysztof Kozlowski <[email protected]>

Cc Alim Akhtar <[email protected]>

Cc [email protected]

---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

drivers/gpu/drm/exynos/exynos_hdmi.c +12 -8

index 25d508b25921..c9d4b9146df9 100644
@@ -884,14 +884,21 @@ static int hdmi_get_modes(struct drm_connector *connector)
 {
 	struct hdmi_context *hdata = connector_to_hdmi(connector);
 	const struct drm_display_info *info = &connector->display_info;
-	struct edid *edid;
+	const struct drm_edid *drm_edid;
 	int ret;
 
 	if (!hdata->ddc_adpt)
 		goto no_edid;
 
-	edid = drm_get_edid(connector, hdata->ddc_adpt);
-	if (!edid)
+	drm_edid = drm_edid_read_ddc(connector, hdata->ddc_adpt);
+
+	ret = drm_edid_connector_update(connector, drm_edid);
+	if (ret)
+		return 0;
+
+	cec_notifier_set_phys_addr(hdata->notifier, info->source_physical_address);
+
+	if (!drm_edid)
 		goto no_edid;
 
 	hdata->dvi_mode = !info->is_hdmi;

@@ -899,12 +906,9 @@ static int hdmi_get_modes(struct drm_connector *connector)
 			  (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"),
 			  info->width_mm / 10, info->height_mm / 10);
 
-	drm_connector_update_edid_property(connector, edid);
-	cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid);
-
-	ret = drm_add_edid_modes(connector, edid);
+	ret = drm_edid_connector_add_modes(connector);
 
-	kfree(edid);
+	drm_edid_free(drm_edid);
 
 	return ret;

Signature · 1 line

2.39.2

Daniel Vetter

Reviewed-by on v1 3/6

2024-08-28 12:45 UTC #5 lore raw

Re: [PATCH 3/6] drm/exynos: hdmi: convert to struct drm_edid

Quoted 33 lines · Jani Nikula

Prefer the struct drm_edid based functions for reading the EDID and updating the connector.
The functional change is that the CEC physical address gets invalidated when the EDID could not be read.
v2:
- display info usage was split to a separate patch
- check drm_edid_connector_update() return value
Signed-off-by Jani Nikula <[email protected]>
---
Cc Inki Dae <[email protected]>
Cc Seung-Woo Kim <[email protected]>
Cc Kyungmin Park <[email protected]>
Cc Krzysztof Kozlowski <[email protected]>
Cc Alim Akhtar <[email protected]>
Cc [email protected]
Cc [email protected]
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)
drivers/gpu/drm/exynos/exynos_hdmi.c +0 -0
index 25d508b25921..c9d4b9146df9 100644
@@ -884,14 +884,21 @@ static int hdmi_get_modes(struct drm_connector *connector)
 {
 	struct hdmi_context *hdata = connector_to_hdmi(connector);
 	const struct drm_display_info *info = &connector->display_info;

drivers/gpu/drm/exynos/exynos_hdmi.c static int hdmi_get_modes(struct drm_connector *connector) · line 893

Quoted 10 lines

drivers/gpu/drm/exynos/exynos_hdmi.c +2 -3

-	struct edid *edid;
+	const struct drm_edid *drm_edid;
 	int ret;
 
 	if (!hdata->ddc_adpt)
 		goto no_edid;
 
-	edid = drm_get_edid(connector, hdata->ddc_adpt);
-	if (!edid)
+	drm_edid = drm_edid_read_ddc(connector, hdata->ddc_adpt);

hdmi_create_connector seems to set up the right ddc already, so I think drm_edid_read should be working. With that:

Reviewed-by Daniel Vetter <[email protected]>

Quoted 29 lines

drivers/gpu/drm/exynos/exynos_hdmi.c +10 -5

+
+	ret = drm_edid_connector_update(connector, drm_edid);
+	if (ret)
+		return 0;
+
+	cec_notifier_set_phys_addr(hdata->notifier, info->source_physical_address);
+
+	if (!drm_edid)
 		goto no_edid;
 
 	hdata->dvi_mode = !info->is_hdmi;

@@ -899,12 +906,9 @@ static int hdmi_get_modes(struct drm_connector *connector)
 			  (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"),
 			  info->width_mm / 10, info->height_mm / 10);
 
-	drm_connector_update_edid_property(connector, edid);
-	cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid);
-
-	ret = drm_add_edid_modes(connector, edid);
+	ret = drm_edid_connector_add_modes(connector);
 
-	kfree(edid);
+	drm_edid_free(drm_edid);
 
 	return ret;

Signature · 1 line

2.39.2

Signature · 3 lines

Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Jani Nikula

author

2024-08-22 17:43 UTC #6 lore raw

PATCH 4/6 drm/tegra: convert to struct drm_edid

In reply to Jani Nikula

Prefer the struct drm_edid based functions for reading the EDID and updating the connector.

Signed-off-by Jani Nikula <[email protected]>

---

Cc Thierry Reding <[email protected]>

Cc Mikko Perttunen <[email protected]>

Cc Jonathan Hunter <[email protected]>

Cc [email protected]

---
 drivers/gpu/drm/tegra/drm.h    |  2 +-
 drivers/gpu/drm/tegra/output.c | 29 +++++++++++++++++------------
 2 files changed, 18 insertions(+), 13 deletions(-)

drivers/gpu/drm/tegra/drm.h +1 -1

index 682011166a8f..2f3781e04b0a 100644
@@ -133,7 +133,7 @@ struct tegra_output {
 	struct drm_bridge *bridge;
 	struct drm_panel *panel;
 	struct i2c_adapter *ddc;
-	const struct edid *edid;
+	const struct drm_edid *drm_edid;
 	struct cec_notifier *cec;
 	unsigned int hpd_irq;
 	struct gpio_desc *hpd_gpio;

drivers/gpu/drm/tegra/output.c +17 ~~-12~~

index 4da3c3d1abbc..e6b5863fec71 100644
@@ -21,7 +21,7 @@
 int tegra_output_connector_get_modes(struct drm_connector *connector)
 {
 	struct tegra_output *output = connector_to_output(connector);
-	struct edid *edid = NULL;
+	const struct drm_edid *drm_edid;
 	int err = 0;
 
 	/*

@@ -34,18 +34,17 @@ int tegra_output_connector_get_modes(struct drm_connector *connector)
 			return err;
 	}
 
-	if (output->edid)
-		edid = kmemdup(output->edid, sizeof(*edid), GFP_KERNEL);
+	if (output->drm_edid)
+		drm_edid = drm_edid_dup(output->drm_edid);
 	else if (output->ddc)
-		edid = drm_get_edid(connector, output->ddc);
+		drm_edid = drm_edid_read_ddc(connector, output->ddc);
 
-	cec_notifier_set_phys_addr_from_edid(output->cec, edid);
-	drm_connector_update_edid_property(connector, edid);
+	drm_edid_connector_update(connector, drm_edid);
+	cec_notifier_set_phys_addr(output->cec,
+				   connector->display_info.source_physical_address);
 
-	if (edid) {
-		err = drm_add_edid_modes(connector, edid);
-		kfree(edid);
-	}
+	err = drm_edid_connector_add_modes(connector);
+	drm_edid_free(drm_edid);
 
 	return err;
 }

@@ -98,6 +97,7 @@ static irqreturn_t hpd_irq(int irq, void *data)
 int tegra_output_probe(struct tegra_output *output)
 {
 	struct device_node *ddc, *panel;
+	const void *edid;
 	unsigned long flags;
 	int err, size;

@@ -124,8 +124,6 @@ int tegra_output_probe(struct tegra_output *output)
 			return PTR_ERR(output->panel);
 	}
 
-	output->edid = of_get_property(output->of_node, "nvidia,edid", &size);
-
 	ddc = of_parse_phandle(output->of_node, "nvidia,ddc-i2c-bus", 0);
 	if (ddc) {
 		output->ddc = of_get_i2c_adapter_by_node(ddc);

@@ -137,6 +135,9 @@ int tegra_output_probe(struct tegra_output *output)
 		}
 	}
 
+	edid = of_get_property(output->of_node, "nvidia,edid", &size);
+	output->drm_edid = drm_edid_alloc(edid, size);
+
 	output->hpd_gpio = devm_fwnode_gpiod_get(output->dev,
 					of_fwnode_handle(output->of_node),
 					"nvidia,hpd",

@@ -187,6 +188,8 @@ int tegra_output_probe(struct tegra_output *output)
 	if (output->ddc)
 		i2c_put_adapter(output->ddc);
 
+	drm_edid_free(output->drm_edid);
+
 	return err;
 }

@@ -197,6 +200,8 @@ void tegra_output_remove(struct tegra_output *output)
 
 	if (output->ddc)
 		i2c_put_adapter(output->ddc);
+
+	drm_edid_free(output->drm_edid);
 }
 
 int tegra_output_init(struct drm_device *drm, struct tegra_output *output)

Signature · 1 line

2.39.2

Daniel Vetter

Reviewed-by on v1 4/6

2024-08-28 12:46 UTC #7 lore raw

Re: [PATCH 4/6] drm/tegra: convert to struct drm_edid

Quoted 4 lines · Jani Nikula

Prefer the struct drm_edid based functions for reading the EDID and updating the connector.
Signed-off-by Jani Nikula <[email protected]>

This time a slightly less blind ...

Reviewed-by Daniel Vetter <[email protected]>

:-)

Quoted 113 lines

---

Cc Thierry Reding <[email protected]>

Cc Mikko Perttunen <[email protected]>

Cc Jonathan Hunter <[email protected]>

Cc [email protected]

---
 drivers/gpu/drm/tegra/drm.h    |  2 +-
 drivers/gpu/drm/tegra/output.c | 29 +++++++++++++++++------------
 2 files changed, 18 insertions(+), 13 deletions(-)

drivers/gpu/drm/tegra/drm.h +1 -1

index 682011166a8f..2f3781e04b0a 100644
@@ -133,7 +133,7 @@ struct tegra_output {
 	struct drm_bridge *bridge;
 	struct drm_panel *panel;
 	struct i2c_adapter *ddc;
-	const struct edid *edid;
+	const struct drm_edid *drm_edid;
 	struct cec_notifier *cec;
 	unsigned int hpd_irq;
 	struct gpio_desc *hpd_gpio;

drivers/gpu/drm/tegra/output.c +17 ~~-12~~

index 4da3c3d1abbc..e6b5863fec71 100644
@@ -21,7 +21,7 @@
 int tegra_output_connector_get_modes(struct drm_connector *connector)
 {
 	struct tegra_output *output = connector_to_output(connector);
-	struct edid *edid = NULL;
+	const struct drm_edid *drm_edid;
 	int err = 0;
 
 	/*

@@ -34,18 +34,17 @@ int tegra_output_connector_get_modes(struct drm_connector *connector)
 			return err;
 	}
 
-	if (output->edid)
-		edid = kmemdup(output->edid, sizeof(*edid), GFP_KERNEL);
+	if (output->drm_edid)
+		drm_edid = drm_edid_dup(output->drm_edid);
 	else if (output->ddc)
-		edid = drm_get_edid(connector, output->ddc);
+		drm_edid = drm_edid_read_ddc(connector, output->ddc);
 
-	cec_notifier_set_phys_addr_from_edid(output->cec, edid);
-	drm_connector_update_edid_property(connector, edid);
+	drm_edid_connector_update(connector, drm_edid);
+	cec_notifier_set_phys_addr(output->cec,
+				   connector->display_info.source_physical_address);
 
-	if (edid) {
-		err = drm_add_edid_modes(connector, edid);
-		kfree(edid);
-	}
+	err = drm_edid_connector_add_modes(connector);
+	drm_edid_free(drm_edid);
 
 	return err;
 }

@@ -98,6 +97,7 @@ static irqreturn_t hpd_irq(int irq, void *data)
 int tegra_output_probe(struct tegra_output *output)
 {
 	struct device_node *ddc, *panel;
+	const void *edid;
 	unsigned long flags;
 	int err, size;

@@ -124,8 +124,6 @@ int tegra_output_probe(struct tegra_output *output)
 			return PTR_ERR(output->panel);
 	}
 
-	output->edid = of_get_property(output->of_node, "nvidia,edid", &size);
-
 	ddc = of_parse_phandle(output->of_node, "nvidia,ddc-i2c-bus", 0);
 	if (ddc) {
 		output->ddc = of_get_i2c_adapter_by_node(ddc);

@@ -137,6 +135,9 @@ int tegra_output_probe(struct tegra_output *output)
 		}
 	}
 
+	edid = of_get_property(output->of_node, "nvidia,edid", &size);
+	output->drm_edid = drm_edid_alloc(edid, size);
+
 	output->hpd_gpio = devm_fwnode_gpiod_get(output->dev,
 					of_fwnode_handle(output->of_node),
 					"nvidia,hpd",

@@ -187,6 +188,8 @@ int tegra_output_probe(struct tegra_output *output)
 	if (output->ddc)
 		i2c_put_adapter(output->ddc);
 
+	drm_edid_free(output->drm_edid);
+
 	return err;
 }

@@ -197,6 +200,8 @@ void tegra_output_remove(struct tegra_output *output)
 
 	if (output->ddc)
 		i2c_put_adapter(output->ddc);
+
+	drm_edid_free(output->drm_edid);
 }
 
 int tegra_output_init(struct drm_device *drm, struct tegra_output *output)

Signature · 1 line

2.39.2

Signature · 3 lines

Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Thierry Reding

Acked-by on v1 4/6

2024-08-29 08:55 UTC #8 lore raw

Re: [PATCH 4/6] drm/tegra: convert to struct drm_edid

In reply to Jani Nikula

Quoted 21 lines · Jani Nikula

Prefer the struct drm_edid based functions for reading the EDID and updating the connector.
Signed-off-by Jani Nikula <[email protected]>
---
Cc Thierry Reding <[email protected]>
Cc Mikko Perttunen <[email protected]>
Cc Jonathan Hunter <[email protected]>
Cc [email protected]
---
 drivers/gpu/drm/tegra/drm.h    |  2 +-
 drivers/gpu/drm/tegra/output.c | 29 +++++++++++++++++------------
 2 files changed, 18 insertions(+), 13 deletions(-)
drivers/gpu/drm/tegra/drm.h +0 -0
index 682011166a8f..2f3781e04b0a 100644

drivers/gpu/drm/tegra/drm.h struct tegra_output { · line 136

Quoted 5 lines

drivers/gpu/drm/tegra/drm.h +1 -1

@@ -133,7 +133,7 @@ struct tegra_output {
 	struct drm_bridge *bridge;
 	struct drm_panel *panel;
 	struct i2c_adapter *ddc;
-	const struct edid *edid;
+	const struct drm_edid *drm_edid;

It perhaps might've been less confusing if this wasn't gratuitously renamed, but either way (and assuming you want to take this through drm-misc):

Acked-by Thierry Reding <[email protected]>

Jani Nikula

author

2024-08-29 11:43 UTC #9 lore raw

Re: [PATCH 4/6] drm/tegra: convert to struct drm_edid

Quoted 17 lines · Thierry Reding

Quoted 16 lines · Jani Nikula
Prefer the struct drm_edid based functions for reading the EDID and updating the connector.
Signed-off-by Jani Nikula <[email protected]>
---
Cc Thierry Reding <[email protected]>
Cc Mikko Perttunen <[email protected]>
Cc Jonathan Hunter <[email protected]>
Cc [email protected]
---
 drivers/gpu/drm/tegra/drm.h    |  2 +-
 drivers/gpu/drm/tegra/output.c | 29 +++++++++++++++++------------
 2 files changed, 18 insertions(+), 13 deletions(-)

Quoted 14 lines

drivers/gpu/drm/tegra/drm.h struct tegra_output { · line 136
Quoted 10 lines
drivers/gpu/drm/tegra/drm.h +1 -1
index 682011166a8f..2f3781e04b0a 100644
@@ -133,7 +133,7 @@ struct tegra_output {
 	struct drm_bridge *bridge;
 	struct drm_panel *panel;
 	struct i2c_adapter *ddc;
-	const struct edid *edid;
+	const struct drm_edid *drm_edid;
It perhaps might've been less confusing if this wasn't gratuitously renamed, but either way (and assuming you want to take this through drm-misc):

I've been doing that to ensure all call sites, especially kfree(), get changed.

Quoted 1 line

Acked-by Thierry Reding <[email protected]>

Thanks, pushed to drm-misc-next.

BR,
Jani.

Signature · 1 line

Jani Nikula, Intel

Jani Nikula

author

2024-08-22 17:43 UTC #10 lore raw

PATCH 5/6 drm/ipuv3/parallel: convert to struct drm_edid

In reply to Jani Nikula

Prefer the struct drm_edid based functions for storing the EDID and updating the connector.

Signed-off-by Jani Nikula <[email protected]>

---

Cc Philipp Zabel <[email protected]>

Cc Maarten Lankhorst <[email protected]>

Cc Maxime Ripard <[email protected]>

Cc Thomas Zimmermann <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Sascha Hauer <[email protected]>

Cc Pengutronix Kernel Team <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc [email protected]

---
 drivers/gpu/drm/imx/ipuv3/parallel-display.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

drivers/gpu/drm/imx/ipuv3/parallel-display.c +9 -5

index 55dedd73f528..91d7808a2d8d 100644
@@ -34,7 +34,7 @@ struct imx_parallel_display_encoder {
 
 struct imx_parallel_display {
 	struct device *dev;
-	void *edid;
+	const struct drm_edid *drm_edid;
 	u32 bus_format;
 	u32 bus_flags;
 	struct drm_display_mode mode;

@@ -62,9 +62,9 @@ static int imx_pd_connector_get_modes(struct drm_connector *connector)
 	if (num_modes > 0)
 		return num_modes;
 
-	if (imxpd->edid) {
-		drm_connector_update_edid_property(connector, imxpd->edid);
-		num_modes = drm_add_edid_modes(connector, imxpd->edid);
+	if (imxpd->drm_edid) {
+		drm_edid_connector_update(connector, imxpd->drm_edid);
+		num_modes = drm_edid_connector_add_modes(connector);
 	}
 
 	if (np) {

@@ -331,7 +331,7 @@ static int imx_pd_probe(struct platform_device *pdev)
 
 	edidp = of_get_property(np, "edid", &edid_len);
 	if (edidp)
-		imxpd->edid = devm_kmemdup(dev, edidp, edid_len, GFP_KERNEL);
+		imxpd->drm_edid = drm_edid_alloc(edidp, edid_len);
 
 	ret = of_property_read_string(np, "interface-pix-fmt", &fmt);
 	if (!ret) {

@@ -355,7 +355,11 @@ static int imx_pd_probe(struct platform_device *pdev)
 
 static void imx_pd_remove(struct platform_device *pdev)
 {
+	struct imx_parallel_display *imxpd = platform_get_drvdata(pdev);
+
 	component_del(&pdev->dev, &imx_pd_ops);
+
+	drm_edid_free(imxpd->drm_edid);
 }
 
 static const struct of_device_id imx_pd_dt_ids[] = {

Signature · 1 line

2.39.2

Daniel Vetter

Reviewed-by on v1 5/6

2024-08-28 12:48 UTC #11 lore raw

Re: [PATCH 5/6] drm/ipuv3/parallel: convert to struct drm_edid

Quoted 49 lines · Jani Nikula

Prefer the struct drm_edid based functions for storing the EDID and updating the connector.
Signed-off-by Jani Nikula <[email protected]>
---
Cc Philipp Zabel <[email protected]>
Cc Maarten Lankhorst <[email protected]>
Cc Maxime Ripard <[email protected]>
Cc Thomas Zimmermann <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Pengutronix Kernel Team <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc [email protected]
Cc [email protected]
Cc [email protected]
---
 drivers/gpu/drm/imx/ipuv3/parallel-display.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)
drivers/gpu/drm/imx/ipuv3/parallel-display.c +4 -4
index 55dedd73f528..91d7808a2d8d 100644
@@ -34,7 +34,7 @@ struct imx_parallel_display_encoder {
 
 struct imx_parallel_display {
 	struct device *dev;
-	void *edid;
+	const struct drm_edid *drm_edid;
 	u32 bus_format;
 	u32 bus_flags;
 	struct drm_display_mode mode;
@@ -62,9 +62,9 @@ static int imx_pd_connector_get_modes(struct drm_connector *connector)
 	if (num_modes > 0)
 		return num_modes;
 
-	if (imxpd->edid) {
-		drm_connector_update_edid_property(connector, imxpd->edid);
-		num_modes = drm_add_edid_modes(connector, imxpd->edid);
+	if (imxpd->drm_edid) {
+		drm_edid_connector_update(connector, imxpd->drm_edid);
+		num_modes = drm_edid_connector_add_modes(connector);
 	}
 
 	if (np) {

drivers/gpu/drm/imx/ipuv3/parallel-display.c static int imx_pd_probe(struct platform_device *pdev) · line 334

Quoted 5 lines

drivers/gpu/drm/imx/ipuv3/parallel-display.c +1 -1

@@ -331,7 +331,7 @@ static int imx_pd_probe(struct platform_device *pdev)
 
 	edidp = of_get_property(np, "edid", &edid_len);
 	if (edidp)
-		imxpd->edid = devm_kmemdup(dev, edidp, edid_len, GFP_KERNEL);
+		imxpd->drm_edid = drm_edid_alloc(edidp, edid_len);

Randomly wondering whether drm_edid_alloc should at least sanity check the edid size ... but for another time. On this:

Reviewed-by Daniel Vetter <[email protected]>

Quoted 18 lines

drivers/gpu/drm/imx/ipuv3/parallel-display.c +4 -0

 	ret = of_property_read_string(np, "interface-pix-fmt", &fmt);
 	if (!ret) {

@@ -355,7 +355,11 @@ static int imx_pd_probe(struct platform_device *pdev)
 
 static void imx_pd_remove(struct platform_device *pdev)
 {
+	struct imx_parallel_display *imxpd = platform_get_drvdata(pdev);
+
 	component_del(&pdev->dev, &imx_pd_ops);
+
+	drm_edid_free(imxpd->drm_edid);
 }
 
 static const struct of_device_id imx_pd_dt_ids[] = {

Signature · 1 line

2.39.2

Signature · 3 lines

Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Jani Nikula

author

2024-08-22 17:43 UTC #12 lore raw

PATCH 6/6 drm/tiny/gm12u320: convert to struct drm_edid

In reply to Jani Nikula

Prefer the struct drm_edid based functions for allocating the EDID and updating the connector.

Signed-off-by Jani Nikula <[email protected]>

---

Cc Hans de Goede <[email protected]>

Cc Maarten Lankhorst <[email protected]>

Cc Maxime Ripard <[email protected]>

Cc Thomas Zimmermann <[email protected]>

Cc [email protected]

---
 drivers/gpu/drm/tiny/gm12u320.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

drivers/gpu/drm/tiny/gm12u320.c +10 -3

index e0defb1d134f..0bd7707c053e 100644
@@ -464,7 +464,7 @@ static int gm12u320_set_ecomode(struct gm12u320_device *gm12u320)
  * Note this assumes this driver is only ever used with the Acer C120, if we
  * add support for other devices the vendor and model should be parameterized.
  */
-static struct edid gm12u320_edid = {
+static const struct edid gm12u320_edid = {
 	.header		= { 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00 },
 	.mfg_id		= { 0x04, 0x72 },	/* "ACR" */
 	.prod_code	= { 0x20, 0xc1 },	/* C120h */

@@ -523,8 +523,15 @@ static struct edid gm12u320_edid = {
 
 static int gm12u320_conn_get_modes(struct drm_connector *connector)
 {
-	drm_connector_update_edid_property(connector, &gm12u320_edid);
-	return drm_add_edid_modes(connector, &gm12u320_edid);
+	const struct drm_edid *drm_edid;
+	int count;
+
+	drm_edid = drm_edid_alloc(&gm12u320_edid, sizeof(gm12u320_edid));
+	drm_edid_connector_update(connector, drm_edid);
+	count = drm_edid_connector_add_modes(connector);
+	drm_edid_free(drm_edid);
+
+	return count;
 }
 
 static const struct drm_connector_helper_funcs gm12u320_conn_helper_funcs = {

Signature · 1 line

2.39.2

Daniel Vetter

Reviewed-by on v1 6/6

2024-08-28 12:49 UTC #13 lore raw

Re: [PATCH 6/6] drm/tiny/gm12u320: convert to struct drm_edid

Quoted 4 lines · Jani Nikula

Prefer the struct drm_edid based functions for allocating the EDID and updating the connector.
Signed-off-by Jani Nikula <[email protected]>

Reviewed-by Daniel Vetter <[email protected]>

Quoted 46 lines

---

Cc Hans de Goede <[email protected]>

Cc Maarten Lankhorst <[email protected]>

Cc Maxime Ripard <[email protected]>

Cc Thomas Zimmermann <[email protected]>

Cc [email protected]

---
 drivers/gpu/drm/tiny/gm12u320.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

drivers/gpu/drm/tiny/gm12u320.c +10 -3

index e0defb1d134f..0bd7707c053e 100644
@@ -464,7 +464,7 @@ static int gm12u320_set_ecomode(struct gm12u320_device *gm12u320)
  * Note this assumes this driver is only ever used with the Acer C120, if we
  * add support for other devices the vendor and model should be parameterized.
  */
-static struct edid gm12u320_edid = {
+static const struct edid gm12u320_edid = {
 	.header		= { 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00 },
 	.mfg_id		= { 0x04, 0x72 },	/* "ACR" */
 	.prod_code	= { 0x20, 0xc1 },	/* C120h */

@@ -523,8 +523,15 @@ static struct edid gm12u320_edid = {
 
 static int gm12u320_conn_get_modes(struct drm_connector *connector)
 {
-	drm_connector_update_edid_property(connector, &gm12u320_edid);
-	return drm_add_edid_modes(connector, &gm12u320_edid);
+	const struct drm_edid *drm_edid;
+	int count;
+
+	drm_edid = drm_edid_alloc(&gm12u320_edid, sizeof(gm12u320_edid));
+	drm_edid_connector_update(connector, drm_edid);
+	count = drm_edid_connector_add_modes(connector);
+	drm_edid_free(drm_edid);
+
+	return count;
 }
 
 static const struct drm_connector_helper_funcs gm12u320_conn_helper_funcs = {

Signature · 1 line

2.39.2

Signature · 3 lines

Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Thomas Zimmermann

maintainer

2024-08-28 12:52 UTC #14 lore raw

Re: [PATCH 6/6] drm/tiny/gm12u320: convert to struct drm_edid

In reply to Jani Nikula

Am 22.08.24 um 19:42 schrieb Jani Nikula:

Quoted 30 lines

Prefer the struct drm_edid based functions for allocating the EDID and updating the connector.
Signed-off-by Jani Nikula <[email protected]>
---
Cc Hans de Goede <[email protected]>
Cc Maarten Lankhorst <[email protected]>
Cc Maxime Ripard <[email protected]>
Cc Thomas Zimmermann <[email protected]>
Cc [email protected]
---
  drivers/gpu/drm/tiny/gm12u320.c | 13 ++++++++++---
  1 file changed, 10 insertions(+), 3 deletions(-)
drivers/gpu/drm/tiny/gm12u320.c +1 -1
index e0defb1d134f..0bd7707c053e 100644
@@ -464,7 +464,7 @@ static int gm12u320_set_ecomode(struct gm12u320_device *gm12u320)
   * Note this assumes this driver is only ever used with the Acer C120, if we
   * add support for other devices the vendor and model should be parameterized.
   */
-static struct edid gm12u320_edid = {
+static const struct edid gm12u320_edid = {
  	.header		= { 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00 },
  	.mfg_id		= { 0x04, 0x72 },	/* "ACR" */
  	.prod_code	= { 0x20, 0xc1 },	/* C120h */

drivers/gpu/drm/tiny/gm12u320.c static struct edid gm12u320_edid = { · line 529

Quoted 9 lines

drivers/gpu/drm/tiny/gm12u320.c +4 -2

@@ -523,8 +523,15 @@ static struct edid gm12u320_edid = {
 
  static int gm12u320_conn_get_modes(struct drm_connector *connector)
  {
-	drm_connector_update_edid_property(connector, &gm12u320_edid);
-	return drm_add_edid_modes(connector, &gm12u320_edid);
+	const struct drm_edid *drm_edid;
+	int count;
+
+	drm_edid = drm_edid_alloc(&gm12u320_edid, sizeof(gm12u320_edid));

Wouldn't it be better to use drm_edid_read_custom() to get the connector's edid override, if any?

Best regards
Thomas

Quoted 8 lines

drivers/gpu/drm/tiny/gm12u320.c +5 -0

+	drm_edid_connector_update(connector, drm_edid);
+	count = drm_edid_connector_add_modes(connector);
+	drm_edid_free(drm_edid);
+
+	return count;
  }
 
  static const struct drm_connector_helper_funcs gm12u320_conn_helper_funcs = {

Signature · 7 lines

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Jani Nikula

author

2024-08-28 13:36 UTC #15 lore raw

Re: [PATCH 6/6] drm/tiny/gm12u320: convert to struct drm_edid

Quoted 34 lines · Thomas Zimmermann

Hi
Am 22.08.24 um 19:42 schrieb Jani Nikula:
Quoted 31 lines
Prefer the struct drm_edid based functions for allocating the EDID and updating the connector.
Signed-off-by Jani Nikula <[email protected]>
---
Cc Hans de Goede <[email protected]>
Cc Maarten Lankhorst <[email protected]>
Cc Maxime Ripard <[email protected]>
Cc Thomas Zimmermann <[email protected]>
Cc [email protected]
---
  drivers/gpu/drm/tiny/gm12u320.c | 13 ++++++++++---
  1 file changed, 10 insertions(+), 3 deletions(-)
drivers/gpu/drm/tiny/gm12u320.c +1 -1
index e0defb1d134f..0bd7707c053e 100644
@@ -464,7 +464,7 @@ static int gm12u320_set_ecomode(struct gm12u320_device *gm12u320)
   * Note this assumes this driver is only ever used with the Acer C120, if we
   * add support for other devices the vendor and model should be parameterized.
   */
-static struct edid gm12u320_edid = {
+static const struct edid gm12u320_edid = {
  	.header		= { 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00 },
  	.mfg_id		= { 0x04, 0x72 },	/* "ACR" */
  	.prod_code	= { 0x20, 0xc1 },	/* C120h */
@@ -523,8 +523,15 @@ static struct edid gm12u320_edid = {
 

Quoted 11 lines

drivers/gpu/drm/tiny/gm12u320.c static struct edid gm12u320_edid = { · line 529
Quoted 8 lines
drivers/gpu/drm/tiny/gm12u320.c +4 -2
  static int gm12u320_conn_get_modes(struct drm_connector *connector)
  {
-	drm_connector_update_edid_property(connector, &gm12u320_edid);
-	return drm_add_edid_modes(connector, &gm12u320_edid);
+	const struct drm_edid *drm_edid;
+	int count;
+
+	drm_edid = drm_edid_alloc(&gm12u320_edid, sizeof(gm12u320_edid));
Wouldn't it be better to use drm_edid_read_custom() to get the connector's edid override, if any?

It would, no doubt. But it's more work, and the conversion is more error prone. I don't have the devices to test any of this, and I don't see many people jumping in to help with that either.

So I'm opting for the minimum but positive incremental improvements with the drm_edid conversions.

BR,
Jani.

Quoted 12 lines

Best regards
Thomas

Quoted 8 lines

drivers/gpu/drm/tiny/gm12u320.c +5 -0

+	drm_edid_connector_update(connector, drm_edid);
+	count = drm_edid_connector_add_modes(connector);
+	drm_edid_free(drm_edid);
+
+	return count;
  }
 
  static const struct drm_connector_helper_funcs gm12u320_conn_helper_funcs = {

Signature · 1 line

Jani Nikula, Intel

Jani Nikula

author

2024-08-29 11:44 UTC #16 lore raw

In reply to Jani Nikula

Quoted 5 lines · Jani Nikula

The umerged patches from [1] and then some.
BR,
Jani.

Quoted 7 lines

[1] https://lore.kernel.org/r/[email protected]
Jani Nikula (6):
drm/sti/sti_hdmi: convert to struct drm_edid
drm/tegra: convert to struct drm_edid
drm/ipuv3/parallel: convert to struct drm_edid
drm/tiny/gm12u320: convert to struct drm_edid

Pushed the above to drm-misc-next, thanks for the reviews and acks.

Quoted 2 lines

drm/exynos: hdmi: use display_info for printing display dimensions drm/exynos: hdmi: convert to struct drm_edid

I'll wait for Inki to check these.

BR,
Jani.

Quoted 8 lines

 drivers/gpu/drm/exynos/exynos_hdmi.c         | 25 ++++++++++-------
 drivers/gpu/drm/imx/ipuv3/parallel-display.c | 14 ++++++----
 drivers/gpu/drm/sti/sti_hdmi.c               | 24 +++++++++-------
 drivers/gpu/drm/tegra/drm.h                  |  2 +-
 drivers/gpu/drm/tegra/output.c               | 29 ++++++++++++--------
 drivers/gpu/drm/tiny/gm12u320.c              | 13 +++++++--
 6 files changed, 66 insertions(+), 41 deletions(-)

Signature · 1 line

Jani Nikula, Intel

대인기/Tizen Platform Lab(SR)/삼성전자

maintainer Acked-by

2024-09-04 02:27 UTC #17 lore raw

Hi Jani,

Sorry for being late.

Quoted 21 lines

-----Original Message-----
From: Jani Nikula <[email protected]>
Sent: Thursday, August 29, 2024 8:45 PM
To: [email protected]
Cc: Daniel Vetter <[email protected]>; [email protected]
Subject: Re: [PATCH 0/6] drm: conversions to struct drm_edid
Quoted 5 lines · Jani Nikula
The umerged patches from [1] and then some.
BR,
Jani.
Quoted 7 lines
[1] https://lore.kernel.org/r/[email protected]
Jani Nikula (6):
drm/sti/sti_hdmi: convert to struct drm_edid
drm/tegra: convert to struct drm_edid
drm/ipuv3/parallel: convert to struct drm_edid
drm/tiny/gm12u320: convert to struct drm_edid

Quoted 6 lines

Pushed the above to drm-misc-next, thanks for the reviews and acks.
Quoted 2 lines
drm/exynos: hdmi: use display_info for printing display dimensions drm/exynos: hdmi: convert to struct drm_edid
I'll wait for Inki to check these.

You could push them to drm-misc-next.

Acked-by Inki Dae <[email protected]>

Thanks,
Inki Dae

Quoted 15 lines

BR,
Jani.

Quoted 8 lines

 drivers/gpu/drm/exynos/exynos_hdmi.c         | 25 ++++++++++-------
 drivers/gpu/drm/imx/ipuv3/parallel-display.c | 14 ++++++----
 drivers/gpu/drm/sti/sti_hdmi.c               | 24 +++++++++-------
 drivers/gpu/drm/tegra/drm.h                  |  2 +-
 drivers/gpu/drm/tegra/output.c               | 29 ++++++++++++--------
 drivers/gpu/drm/tiny/gm12u320.c              | 13 +++++++--
 6 files changed, 66 insertions(+), 41 deletions(-)

--
Jani Nikula, Intel

Jani Nikula

author

2024-09-04 08:33 UTC #18 lore raw

Quoted 8 lines · 대인기/Tizen Platform Lab(SR)/삼성전자

Quoted 4 lines
Quoted 2 lines
drm/exynos: hdmi: use display_info for printing display dimensions drm/exynos: hdmi: convert to struct drm_edid
I'll wait for Inki to check these.
You could push them to drm-misc-next.
Acked-by Inki Dae <[email protected]>

Thanks, pushed.

BR,
Jani.

Signature · 1 line

Jani Nikula, Intel