drm/edid: add separate drm_edid_connector_add_modes()

The original goal with drm_edid_connector_update() was to have a single
call for updating the connector and adding probed modes, in this order,
but that turned out to be problematic. Drivers that need to update the
connector in the .detect() callback would end up updating the probed
modes as well. Turns out the callback may be called so many times that
the probed mode list fills up without bounds, and this is amplified by
add_alternate_cea_modes() duplicating the CEA modes on every call,
actually running out of memory on some machines.

Kudos to Imre Deak <imre.deak@intel.com> for explaining this to me.

Go back to having separate drm_edid_connector_update() and
drm_edid_connector_add_modes() calls. The former may be called from
.detect(), .force(), or .get_modes(), but the latter only from
.get_modes().

Unlike drm_add_edid_modes(), have drm_edid_connector_add_modes() update
the probed modes from the EDID property instead of the passed in
EDID. This is mainly to enforce two things:

1) drm_edid_connector_update() must be called before
   drm_edid_connector_add_modes().

   Display info and quirks are needed for parsing the modes, and we
   don't want to call update_display_info() again to ensure the info is
   available, like drm_add_edid_modes() does.

2) The same EDID is used for both updating the connector and adding the
   probed modes.

Fortunately, the change is easy, because no driver has actually adopted
drm_edid_connector_update(). Not even i915, and that's mainly because of
the problem described above.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/e86fff1579f14ebf6334692526c8f6831cd02cac.1674144945.git.jani.nikula@intel.com

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index f9502df3dbca8590eb956ce53029029befe44d06..d0c21d27b978f00ae786a34bc55240ff3ee0fd84 100644 (file)
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6735,30 +6735,54 @@ out:
  * @connector: Connector
  * @drm_edid: EDID
  *
- * Update the connector mode list, display info, ELD, HDR metadata, relevant
- * properties, etc. from the passed in EDID.
+ * Update the connector display info, ELD, HDR metadata, relevant properties,
+ * etc. from the passed in EDID.
  *
  * If EDID is NULL, reset the information.
  *
- * Return: The number of modes added or 0 if we couldn't find any.
+ * Must be called before calling drm_edid_connector_add_modes().
+ *
+ * Return: 0 on success, negative error on errors.
  */
 int drm_edid_connector_update(struct drm_connector *connector,
                              const struct drm_edid *drm_edid)
 {
+       update_display_info(connector, drm_edid);
+
+       _drm_update_tile_info(connector, drm_edid);
+
+       return _drm_edid_connector_property_update(connector, drm_edid);
+}
+EXPORT_SYMBOL(drm_edid_connector_update);
+
+/**
+ * drm_edid_connector_add_modes - Update probed modes from the EDID property
+ * @connector: Connector
+ *
+ * Add the modes from the previously updated EDID property to the connector
+ * probed modes list.
+ *
+ * drm_edid_connector_update() must have been called before this to update the
+ * EDID property.
+ *
+ * Return: The number of modes added, or 0 if we couldn't find any.
+ */
+int drm_edid_connector_add_modes(struct drm_connector *connector)
+{
+       const struct drm_edid *drm_edid = NULL;
        int count;
 
-       update_display_info(connector, drm_edid);
+       if (connector->edid_blob_ptr)
+               drm_edid = drm_edid_alloc(connector->edid_blob_ptr->data,
+                                         connector->edid_blob_ptr->length);
 
        count = _drm_edid_connector_add_modes(connector, drm_edid);
 
-       _drm_update_tile_info(connector, drm_edid);
-
-       /* Note: Ignore errors for now. */
-       _drm_edid_connector_property_update(connector, drm_edid);
+       drm_edid_free(drm_edid);
 
        return count;
 }
-EXPORT_SYMBOL(drm_edid_connector_update);
+EXPORT_SYMBOL(drm_edid_connector_add_modes);
 
 static int _drm_connector_update_edid_property(struct drm_connector *connector,
                                               const struct drm_edid *drm_edid)
@@ -6813,7 +6837,7 @@ EXPORT_SYMBOL(drm_connector_update_edid_property);
  * &drm_display_info structure and ELD in @connector with any information which
  * can be derived from the edid.
  *
- * This function is deprecated. Use drm_edid_connector_update() instead.
+ * This function is deprecated. Use drm_edid_connector_add_modes() instead.
  *
  * Return: The number of modes added or 0 if we couldn't find any.
  */

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 7973f2589ced35e499bd9146c97ce5866ebd86d1..95aeeed33cf57edd8b766c5b2c3b940836ce2ede 100644 (file)
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -1163,7 +1163,9 @@ int drm_connector_helper_get_modes(struct drm_connector *connector)
         * EDID. Otherwise, if the EDID is NULL, clear the connector
         * information.
         */
-       count = drm_edid_connector_update(connector, drm_edid);
+       drm_edid_connector_update(connector, drm_edid);
+
+       count = drm_edid_connector_add_modes(connector);
 
        drm_edid_free(drm_edid);
 

diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 372963600f1dd8cfa512243af212e4e22431c792..70ae6c290bdc3801cfa46cc4fb1d6facf6b4b0cf 100644 (file)
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -609,6 +609,8 @@ const struct drm_edid *drm_edid_read_custom(struct drm_connector *connector,
                                            void *context);
 int drm_edid_connector_update(struct drm_connector *connector,
                              const struct drm_edid *edid);
+int drm_edid_connector_add_modes(struct drm_connector *connector);
+
 const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
                                  int ext_id, int *ext_index);