[PATCH 0/2] drm: remove drm_connector_helper_get_modes_from_ddc()

[PATCH 0/2] drm: remove drm_connector_helper_get_modes_from_ddc()

[Jani Nikula]

Convert mgag200 to use struct drm_edid directly, and drop
drm_connector_helper_get_modes_from_ddc(), as suggested by Thomas.

This supersedes [1].

BR,
Jani.


[1] https://lore.kernel.org/all/20231114105815.4188901-1-jani.nikula@intel.com/

Jani Nikula (2):
  drm/mgag200: convert get modes to struct drm_edid
  drm/probe-helper: remove unused
    drm_connector_helper_get_modes_from_ddc()

 drivers/gpu/drm/drm_probe_helper.c     | 36 --------------------------
 drivers/gpu/drm/mgag200/mgag200_mode.c | 14 +++++++---
 include/drm/drm_probe_helper.h         |  1 -
 3 files changed, 10 insertions(+), 41 deletions(-)

-- 
2.39.2

[PATCH 1/2] drm/mgag200: convert get modes to struct drm_edid

[Jani Nikula]

Convert mgag200_vga_connector_helper_get_modes() to use struct drm_edid
based functions directly.

Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 0f0d59938c3a..0eb769dd76ce 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -14,13 +14,13 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_damage_helper.h>
+#include <drm/drm_edid.h>
 #include <drm/drm_format_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_print.h>
-#include <drm/drm_probe_helper.h>
 
 #include "mgag200_drv.h"
 
@@ -717,17 +717,23 @@ void mgag200_crtc_atomic_destroy_state(struct drm_crtc *crtc, struct drm_crtc_st
 int mgag200_vga_connector_helper_get_modes(struct drm_connector *connector)
 {
 	struct mga_device *mdev = to_mga_device(connector->dev);
-	int ret;
+	const struct drm_edid *drm_edid;
+	int count;
 
 	/*
 	 * Protect access to I/O registers from concurrent modesetting
 	 * by acquiring the I/O-register lock.
 	 */
 	mutex_lock(&mdev->rmmio_lock);
-	ret = drm_connector_helper_get_modes_from_ddc(connector);
+
+	drm_edid = drm_edid_read(connector);
+	drm_edid_connector_update(connector, drm_edid);
+	count = drm_edid_connector_add_modes(connector);
+	drm_edid_free(drm_edid);
+
 	mutex_unlock(&mdev->rmmio_lock);
 
-	return ret;
+	return count;
 }
 
 /*
-- 
2.39.2

[PATCH 2/2] drm/probe-helper: remove unused drm_connector_helper_get_modes_from_ddc()

[Jani Nikula]

Remove the unused drm_connector_helper_get_modes_from_ddc()
function. Most drivers should probably have this functionality split to
detect and get modes parts, so the helper is not the best abstraction.

Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_probe_helper.c | 36 ------------------------------
 include/drm/drm_probe_helper.h     |  1 -
 2 files changed, 37 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 3f479483d7d8..d1e1ade66f81 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -1100,42 +1100,6 @@ enum drm_mode_status drm_crtc_helper_mode_valid_fixed(struct drm_crtc *crtc,
 }
 EXPORT_SYMBOL(drm_crtc_helper_mode_valid_fixed);
 
-/**
- * drm_connector_helper_get_modes_from_ddc - Updates the connector's EDID
- *                                           property from the connector's
- *                                           DDC channel
- * @connector: The connector
- *
- * Returns:
- * The number of detected display modes.
- *
- * Uses a connector's DDC channel to retrieve EDID data and update the
- * connector's EDID property and display modes. Drivers can use this
- * function to implement struct &drm_connector_helper_funcs.get_modes
- * for connectors with a DDC channel.
- */
-int drm_connector_helper_get_modes_from_ddc(struct drm_connector *connector)
-{
-	struct edid *edid;
-	int count = 0;
-
-	if (!connector->ddc)
-		return 0;
-
-	edid = drm_get_edid(connector, connector->ddc);
-
-	// clears property if EDID is NULL
-	drm_connector_update_edid_property(connector, edid);
-
-	if (edid) {
-		count = drm_add_edid_modes(connector, edid);
-		kfree(edid);
-	}
-
-	return count;
-}
-EXPORT_SYMBOL(drm_connector_helper_get_modes_from_ddc);
-
 /**
  * drm_connector_helper_get_modes_fixed - Duplicates a display mode for a connector
  * @connector: the connector
diff --git a/include/drm/drm_probe_helper.h b/include/drm/drm_probe_helper.h
index fad3c4003b2b..62741a88796b 100644
--- a/include/drm/drm_probe_helper.h
+++ b/include/drm/drm_probe_helper.h
@@ -32,7 +32,6 @@ enum drm_mode_status drm_crtc_helper_mode_valid_fixed(struct drm_crtc *crtc,
 						      const struct drm_display_mode *mode,
 						      const struct drm_display_mode *fixed_mode);
 
-int drm_connector_helper_get_modes_from_ddc(struct drm_connector *connector);
 int drm_connector_helper_get_modes_fixed(struct drm_connector *connector,
 					 const struct drm_display_mode *fixed_mode);
 int drm_connector_helper_get_modes(struct drm_connector *connector);
-- 
2.39.2

Re: [PATCH 0/2] drm: remove drm_connector_helper_get_modes_from_ddc()

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1091 bytes --]

Hi,

for the series:

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Thanks for cleaning up.

Best regards
Thomas

Am 05.01.24 um 17:57 schrieb Jani Nikula:
> Convert mgag200 to use struct drm_edid directly, and drop
> drm_connector_helper_get_modes_from_ddc(), as suggested by Thomas.
> 
> This supersedes [1].
> 
> BR,
> Jani.
> 
> 
> [1] https://lore.kernel.org/all/20231114105815.4188901-1-jani.nikula@intel.com/
> 
> Jani Nikula (2):
>    drm/mgag200: convert get modes to struct drm_edid
>    drm/probe-helper: remove unused
>      drm_connector_helper_get_modes_from_ddc()
> 
>   drivers/gpu/drm/drm_probe_helper.c     | 36 --------------------------
>   drivers/gpu/drm/mgag200/mgag200_mode.c | 14 +++++++---
>   include/drm/drm_probe_helper.h         |  1 -
>   3 files changed, 10 insertions(+), 41 deletions(-)
> 

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 0/2] drm: remove drm_connector_helper_get_modes_from_ddc()

[Jani Nikula]

On Mon, 08 Jan 2024, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Hi,
>
> for the series:
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> Thanks for cleaning up.

Thanks for the review, pushed to drm-misc-next.

BR,
Jani.


-- 
Jani Nikula, Intel