On Wed, Jun 08, 2022 at 10:50:34AM +0300, Jani Nikula wrote:

We'll want to return the allocated buffer size in the future. Keep track of it.

Signed-off-by: Jani Nikula jani.nikula@intel.com

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

---

drivers/gpu/drm/drm_edid.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index f44ada4bfa5b..2beaa48301c1 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2021,13 +2021,16 @@ bool drm_edid_is_valid(struct edid *edid) EXPORT_SYMBOL(drm_edid_is_valid);

static struct edid *edid_filter_invalid_blocks(const struct edid *edid,

• 			       int invalid_blocks)
  

• 			       int invalid_blocks,
  

• 			       size_t *alloc_size)
  

{ struct edid *new, *dest_block; int valid_extensions = edid->extensions - invalid_blocks; int i;

• new = kmalloc(edid_size_by_blocks(valid_extensions + 1), GFP_KERNEL);

• *alloc_size = edid_size_by_blocks(valid_extensions + 1);
• new = kmalloc(*alloc_size, GFP_KERNEL); if (!new) goto out;

@@ -2140,7 +2143,8 @@ static void connector_bad_edid(struct drm_connector *connector, }

/* Get override or firmware EDID */ -static struct edid *drm_get_override_edid(struct drm_connector *connector) +static struct edid *drm_get_override_edid(struct drm_connector *connector,

• 			  size_t *alloc_size)
  

{ struct edid *override = NULL;

@@ -2150,6 +2154,10 @@ static struct edid *drm_get_override_edid(struct drm_connector *connector) if (!override) override = drm_load_edid_firmware(connector);

• /* FIXME: Get alloc size from deeper down the stack */
• if (!IS_ERR_OR_NULL(override) && alloc_size)

• *alloc_size = edid_size(override);
  

• return IS_ERR(override) ? NULL : override;

}

@@ -2169,7 +2177,7 @@ int drm_add_override_edid_modes(struct drm_connector *connector) struct edid *override; int num_modes = 0;

• override = drm_get_override_edid(connector);

• override = drm_get_override_edid(connector, NULL); if (override) { drm_connector_update_edid_property(connector, override); num_modes = drm_add_edid_modes(connector, override);

@@ -2245,12 +2253,13 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, enum edid_block_status status; int i, invalid_blocks = 0; struct edid *edid, *new;

• size_t alloc_size = EDID_LENGTH;

• edid = drm_get_override_edid(connector);

• edid = drm_get_override_edid(connector, &alloc_size); if (edid) goto ok;

• edid = kmalloc(EDID_LENGTH, GFP_KERNEL);

• edid = kmalloc(alloc_size, GFP_KERNEL); if (!edid) return NULL;

@@ -2278,7 +2287,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, if (!edid_extension_block_count(edid)) goto ok;

• new = krealloc(edid, edid_size(edid), GFP_KERNEL);

• alloc_size = edid_size(edid);
• new = krealloc(edid, alloc_size, GFP_KERNEL); if (!new) goto fail; edid = new;

@@ -2300,7 +2310,8 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, if (invalid_blocks) { connector_bad_edid(connector, edid, edid_block_count(edid));

• edid = edid_filter_invalid_blocks(edid, invalid_blocks);
  

• edid = edid_filter_invalid_blocks(edid, invalid_blocks,
  

• 				  &alloc_size);
  

  }

ok:

2.30.2

On Wed, Jun 08, 2022 at 10:50:35AM +0300, Jani Nikula wrote:

Add new functions drm_edid_read(), drm_edid_read_ddc(), and drm_edid_read_custom() to replace drm_get_edid() and drm_do_get_edid() for reading the EDID. The transition is expected to happen over a fairly long time.

Note that the new drm_edid_read*() functions do not do any of the connector updates anymore. The reading and parsing will be completely separated from each other.

Add new functions drm_edid_alloc(), drm_edid_dup(), and drm_edid_free() for allocating and freeing drm_edid containers.

Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Jani Nikula jani.nikula@intel.com

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

---

drivers/gpu/drm/drm_edid.c | 245 +++++++++++++++++++++++++++++++++---- include/drm/drm_edid.h | 9 ++ 2 files changed, 230 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2beaa48301c1..2bdaf1e34a9d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2226,29 +2226,9 @@ static enum edid_block_status edid_block_read(void *block, unsigned int block_nu return status; }

-/**

• • drm_do_get_edid - get EDID data using a custom EDID block read function
• • @connector: connector we're probing
• • @read_block: EDID block read function
• • @context: private data passed to the block read function
• • When the I2C adapter connected to the DDC bus is hidden behind a device that
• • exposes a different interface to read EDID blocks this function can be used
• • to get EDID data using a custom block read function.
• • As in the general case the DDC bus is accessible by the kernel at the I2C
• • level, drivers must make all reasonable efforts to expose it as an I2C
• • adapter and use drm_get_edid() instead of abusing this function.
• • The EDID may be overridden using debugfs override_edid or firmware EDID
• • (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
• • order. Having either of them bypasses actual EDID reads.
• • Return: Pointer to valid EDID or NULL if we couldn't find any.
• */

-struct edid *drm_do_get_edid(struct drm_connector *connector,

• 	     read_block_fn read_block,
  

• 	     void *context)
  

+static struct edid *_drm_do_get_edid(struct drm_connector *connector,

• 		     read_block_fn read_block, void *context,
  

• 		     size_t *size)
  

{ enum edid_block_status status; int i, invalid_blocks = 0; @@ -2315,14 +2295,125 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, }

ok:

• if (size)

• *size = alloc_size;
  

• return edid;

fail: kfree(edid); return NULL; }

+/**

• • drm_do_get_edid - get EDID data using a custom EDID block read function
• • @connector: connector we're probing
• • @read_block: EDID block read function
• • @context: private data passed to the block read function
• • When the I2C adapter connected to the DDC bus is hidden behind a device that
• • exposes a different interface to read EDID blocks this function can be used
• • to get EDID data using a custom block read function.
• • As in the general case the DDC bus is accessible by the kernel at the I2C
• • level, drivers must make all reasonable efforts to expose it as an I2C
• • adapter and use drm_get_edid() instead of abusing this function.
• • The EDID may be overridden using debugfs override_edid or firmware EDID
• • (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
• • order. Having either of them bypasses actual EDID reads.
• • Return: Pointer to valid EDID or NULL if we couldn't find any.
• */

+struct edid *drm_do_get_edid(struct drm_connector *connector,

• 	     read_block_fn read_block,
  

• 	     void *context)
  

+{

• return _drm_do_get_edid(connector, read_block, context, NULL);

+} EXPORT_SYMBOL_GPL(drm_do_get_edid);

+/* Allocate struct drm_edid container *without* duplicating the edid data */ +static const struct drm_edid *_drm_edid_alloc(const void *edid, size_t size) +{

• struct drm_edid *drm_edid;
• if (!edid || !size || size < EDID_LENGTH)

• return NULL;
  

• drm_edid = kzalloc(sizeof(*drm_edid), GFP_KERNEL);
• if (drm_edid) {

• drm_edid->edid = edid;
  

• drm_edid->size = size;
  

• }
• return drm_edid;

+}

+/**

• • drm_edid_alloc - Allocate a new drm_edid container
• • @edid: Pointer to raw EDID data
• • @size: Size of memory allocated for EDID
• • Allocate a new drm_edid container. Do not calculate edid size from edid, pass
• • the actual size that has been allocated for the data. There is no validation
• • of the raw EDID data against the size, but at least the EDID base block must
• • fit in the buffer.
• • The returned pointer must be freed using drm_edid_free().
• • Return: drm_edid container, or NULL on errors
• */

+const struct drm_edid *drm_edid_alloc(const void *edid, size_t size) +{

• const struct drm_edid *drm_edid;
• if (!edid || !size || size < EDID_LENGTH)

• return NULL;
  

• edid = kmemdup(edid, size, GFP_KERNEL);
• if (!edid)

• return NULL;
  

• drm_edid = _drm_edid_alloc(edid, size);
• if (!drm_edid)

• kfree(edid);
  

• return drm_edid;

+} +EXPORT_SYMBOL(drm_edid_alloc);

+/**

• • drm_edid_dup - Duplicate a drm_edid container
• • @drm_edid: EDID to duplicate
• • The returned pointer must be freed using drm_edid_free().
• • Returns: drm_edid container copy, or NULL on errors
• */

+const struct drm_edid *drm_edid_dup(const struct drm_edid *drm_edid) +{

• if (!drm_edid)

• return NULL;
  

• return drm_edid_alloc(drm_edid->edid, drm_edid->size);

+} +EXPORT_SYMBOL(drm_edid_dup);

+/**

• • drm_edid_free - Free the drm_edid container
• • @drm_edid: EDID to free
• */

+void drm_edid_free(const struct drm_edid *drm_edid) +{

• if (!drm_edid)

• return;
  

• kfree(drm_edid->edid);
• kfree(drm_edid);

+} +EXPORT_SYMBOL(drm_edid_free);

/**

• drm_probe_ddc() - probe DDC presence
• @adapter: I2C adapter to probe

@@ -2359,12 +2450,118 @@ struct edid *drm_get_edid(struct drm_connector *connector, if (connector->force == DRM_FORCE_UNSPECIFIED && !drm_probe_ddc(adapter)) return NULL;

• edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter);

• edid = _drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter, NULL); drm_connector_update_edid_property(connector, edid); return edid;

} EXPORT_SYMBOL(drm_get_edid);

+/**

• • drm_edid_read_custom - Read EDID data using given EDID block read function
• • @connector: Connector to use
• • @read_block: EDID block read function
• • @context: Private data passed to the block read function
• • When the I2C adapter connected to the DDC bus is hidden behind a device that
• • exposes a different interface to read EDID blocks this function can be used
• • to get EDID data using a custom block read function.
• • As in the general case the DDC bus is accessible by the kernel at the I2C
• • level, drivers must make all reasonable efforts to expose it as an I2C
• • adapter and use drm_edid_read() or drm_edid_read_ddc() instead of abusing
• • this function.
• • The EDID may be overridden using debugfs override_edid or firmware EDID
• • (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
• • order. Having either of them bypasses actual EDID reads.
• • The returned pointer must be freed using drm_edid_free().
• • Return: Pointer to EDID, or NULL if probe/read failed.
• */

+const struct drm_edid *drm_edid_read_custom(struct drm_connector *connector,

• 			    read_block_fn read_block,
  

• 			    void *context)
  

+{

• const struct drm_edid *drm_edid;
• struct edid *edid;
• size_t size = 0;
• edid = _drm_do_get_edid(connector, read_block, context, &size);
• if (!edid)

• return NULL;
  

• /* Sanity check for now */
• drm_WARN_ON(connector->dev, !size);
• drm_edid = _drm_edid_alloc(edid, size);
• if (!drm_edid)

• kfree(edid);
  

• return drm_edid;

+} +EXPORT_SYMBOL(drm_edid_read_custom);

+/**

• • drm_edid_read_ddc - Read EDID data using given I2C adapter
• • @connector: Connector to use
• • @adapter: I2C adapter to use for DDC
• • Read EDID using the given I2C adapter.
• • The EDID may be overridden using debugfs override_edid or firmware EDID
• • (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
• • order. Having either of them bypasses actual EDID reads.
• • Prefer initializing connector->ddc with drm_connector_init_with_ddc() and
• • using drm_edid_read() instead of this function.
• • The returned pointer must be freed using drm_edid_free().
• • Return: Pointer to EDID, or NULL if probe/read failed.
• */

+const struct drm_edid *drm_edid_read_ddc(struct drm_connector *connector,

• 			 struct i2c_adapter *adapter)
  

+{

• const struct drm_edid *drm_edid;
• if (connector->force == DRM_FORCE_OFF)

• return NULL;
  

• if (connector->force == DRM_FORCE_UNSPECIFIED && !drm_probe_ddc(adapter))

• return NULL;
  

• drm_edid = drm_edid_read_custom(connector, drm_do_probe_ddc_edid, adapter);
• /* Note: Do *not* call connector updates here. */
• return drm_edid;

+} +EXPORT_SYMBOL(drm_edid_read_ddc);

+/**

• • drm_edid_read - Read EDID data using connector's I2C adapter
• • @connector: Connector to use
• • Read EDID using the connector's I2C adapter.
• • The EDID may be overridden using debugfs override_edid or firmware EDID
• • (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
• • order. Having either of them bypasses actual EDID reads.
• • The returned pointer must be freed using drm_edid_free().
• • Return: Pointer to EDID, or NULL if probe/read failed.
• */

+const struct drm_edid *drm_edid_read(struct drm_connector *connector) +{

• if (drm_WARN_ON(connector->dev, !connector->ddc))

• return NULL;
  

• return drm_edid_read_ddc(connector, connector->ddc);

+} +EXPORT_SYMBOL(drm_edid_read);

static u32 edid_extract_panel_id(const struct edid *edid) { /* diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 95ac09ef41b2..9d2d78135dee 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -594,6 +594,15 @@ drm_display_mode_from_cea_vic(struct drm_device *dev, u8 video_code);

/* Interface based on struct drm_edid */ +const struct drm_edid *drm_edid_alloc(const void *edid, size_t size); +const struct drm_edid *drm_edid_dup(const struct drm_edid *drm_edid); +void drm_edid_free(const struct drm_edid *drm_edid); +const struct drm_edid *drm_edid_read(struct drm_connector *connector); +const struct drm_edid *drm_edid_read_ddc(struct drm_connector *connector,

• 			 struct i2c_adapter *adapter);
  

+const struct drm_edid *drm_edid_read_custom(struct drm_connector *connector,

• 			    int (*read_block)(void *context, u8 *buf, unsigned int block, size_t len),
  

• 			    void *context);
  

const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid, int ext_id, int *ext_index);

-- 2.30.2

On Fri, 10 Jun 2022, Ville Syrjälä ville.syrjala@linux.intel.com wrote:

On Wed, Jun 08, 2022 at 10:50:35AM +0300, Jani Nikula wrote:

...

Add new functions drm_edid_read(), drm_edid_read_ddc(), and drm_edid_read_custom() to replace drm_get_edid() and drm_do_get_edid() for reading the EDID. The transition is expected to happen over a fairly long time.

Note that the new drm_edid_read*() functions do not do any of the connector updates anymore. The reading and parsing will be completely separated from each other.

Add new functions drm_edid_alloc(), drm_edid_dup(), and drm_edid_free() for allocating and freeing drm_edid containers.

Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Jani Nikula jani.nikula@intel.com

---

drivers/gpu/drm/drm_edid.c | 245 +++++++++++++++++++++++++++++++++---- include/drm/drm_edid.h | 9 ++ 2 files changed, 230 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2beaa48301c1..2bdaf1e34a9d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2226,29 +2226,9 @@ static enum edid_block_status edid_block_read(void *block, unsigned int block_nu return status; }

-/**

• • drm_do_get_edid - get EDID data using a custom EDID block read function
• • @connector: connector we're probing
• • @read_block: EDID block read function
• • @context: private data passed to the block read function
• • When the I2C adapter connected to the DDC bus is hidden behind a device that
• • exposes a different interface to read EDID blocks this function can be used
• • to get EDID data using a custom block read function.
• • As in the general case the DDC bus is accessible by the kernel at the I2C
• • level, drivers must make all reasonable efforts to expose it as an I2C
• • adapter and use drm_get_edid() instead of abusing this function.
• • The EDID may be overridden using debugfs override_edid or firmware EDID
• • (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
• • order. Having either of them bypasses actual EDID reads.
• • Return: Pointer to valid EDID or NULL if we couldn't find any.
• */

-struct edid *drm_do_get_edid(struct drm_connector *connector,

• 	     read_block_fn read_block,
  

• 	     void *context)
  

+static struct edid *_drm_do_get_edid(struct drm_connector *connector,

• 		     read_block_fn read_block, void *context,
  

• 		     size_t *size)
  

{ enum edid_block_status status; int i, invalid_blocks = 0; @@ -2315,14 +2295,125 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, }

ok:

• if (size)

• *size = alloc_size;
  

• return edid;

fail: kfree(edid); return NULL; }

+/**

• • drm_do_get_edid - get EDID data using a custom EDID block read function
• • @connector: connector we're probing
• • @read_block: EDID block read function
• • @context: private data passed to the block read function
• • When the I2C adapter connected to the DDC bus is hidden behind a device that
• • exposes a different interface to read EDID blocks this function can be used
• • to get EDID data using a custom block read function.
• • As in the general case the DDC bus is accessible by the kernel at the I2C
• • level, drivers must make all reasonable efforts to expose it as an I2C
• • adapter and use drm_get_edid() instead of abusing this function.
• • The EDID may be overridden using debugfs override_edid or firmware EDID
• • (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
• • order. Having either of them bypasses actual EDID reads.
• • Return: Pointer to valid EDID or NULL if we couldn't find any.
• */

+struct edid *drm_do_get_edid(struct drm_connector *connector,

• 	     read_block_fn read_block,
  

• 	     void *context)
  

+{

• return _drm_do_get_edid(connector, read_block, context, NULL);

+} EXPORT_SYMBOL_GPL(drm_do_get_edid);

+/* Allocate struct drm_edid container *without* duplicating the edid data */ +static const struct drm_edid *_drm_edid_alloc(const void *edid, size_t size) +{

• struct drm_edid *drm_edid;
• if (!edid || !size || size < EDID_LENGTH)

• return NULL;
  

• drm_edid = kzalloc(sizeof(*drm_edid), GFP_KERNEL);
• if (drm_edid) {

• drm_edid->edid = edid;
  

• drm_edid->size = size;
  

• }
• return drm_edid;

+}

+/**

• • drm_edid_alloc - Allocate a new drm_edid container
• • @edid: Pointer to raw EDID data
• • @size: Size of memory allocated for EDID
• • Allocate a new drm_edid container. Do not calculate edid size from edid, pass
• • the actual size that has been allocated for the data. There is no validation
• • of the raw EDID data against the size, but at least the EDID base block must
• • fit in the buffer.
• • The returned pointer must be freed using drm_edid_free().
• • Return: drm_edid container, or NULL on errors
• */

+const struct drm_edid *drm_edid_alloc(const void *edid, size_t size) +{

• const struct drm_edid *drm_edid;
• if (!edid || !size || size < EDID_LENGTH)

• return NULL;
  

• edid = kmemdup(edid, size, GFP_KERNEL);
• if (!edid)

• return NULL;
  

• drm_edid = _drm_edid_alloc(edid, size);
• if (!drm_edid)

• kfree(edid);
  

• return drm_edid;

+} +EXPORT_SYMBOL(drm_edid_alloc);

+/**

• • drm_edid_dup - Duplicate a drm_edid container
• • @drm_edid: EDID to duplicate
• • The returned pointer must be freed using drm_edid_free().
• • Returns: drm_edid container copy, or NULL on errors
• */

+const struct drm_edid *drm_edid_dup(const struct drm_edid *drm_edid) +{

• if (!drm_edid)

• return NULL;
  

• return drm_edid_alloc(drm_edid->edid, drm_edid->size);

+} +EXPORT_SYMBOL(drm_edid_dup);

+/**

• • drm_edid_free - Free the drm_edid container
• • @drm_edid: EDID to free
• */

+void drm_edid_free(const struct drm_edid *drm_edid) +{

• if (!drm_edid)

• return;
  

• kfree(drm_edid->edid);
• kfree(drm_edid);

+} +EXPORT_SYMBOL(drm_edid_free);

/**

• drm_probe_ddc() - probe DDC presence
• @adapter: I2C adapter to probe

@@ -2359,12 +2450,118 @@ struct edid *drm_get_edid(struct drm_connector *connector, if (connector->force == DRM_FORCE_UNSPECIFIED && !drm_probe_ddc(adapter)) return NULL;

• edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter);

• edid = _drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter, NULL); drm_connector_update_edid_property(connector, edid); return edid;

} EXPORT_SYMBOL(drm_get_edid);

+/**

• • drm_edid_read_custom - Read EDID data using given EDID block read function
• • @connector: Connector to use
• • @read_block: EDID block read function
• • @context: Private data passed to the block read function
• • When the I2C adapter connected to the DDC bus is hidden behind a device that
• • exposes a different interface to read EDID blocks this function can be used
• • to get EDID data using a custom block read function.
• • As in the general case the DDC bus is accessible by the kernel at the I2C
• • level, drivers must make all reasonable efforts to expose it as an I2C
• • adapter and use drm_edid_read() or drm_edid_read_ddc() instead of abusing
• • this function.
• • The EDID may be overridden using debugfs override_edid or firmware EDID
• • (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
• • order. Having either of them bypasses actual EDID reads.
• • The returned pointer must be freed using drm_edid_free().
• • Return: Pointer to EDID, or NULL if probe/read failed.
• */

+const struct drm_edid *drm_edid_read_custom(struct drm_connector *connector,

• 			    read_block_fn read_block,
  

• 			    void *context)
  

+{

• const struct drm_edid *drm_edid;
• struct edid *edid;
• size_t size = 0;
• edid = _drm_do_get_edid(connector, read_block, context, &size);
• if (!edid)

• return NULL;
  

• /* Sanity check for now */
• drm_WARN_ON(connector->dev, !size);
• drm_edid = _drm_edid_alloc(edid, size);
• if (!drm_edid)

• kfree(edid);
  

• return drm_edid;

+} +EXPORT_SYMBOL(drm_edid_read_custom);

+/**

• • drm_edid_read_ddc - Read EDID data using given I2C adapter
• • @connector: Connector to use
• • @adapter: I2C adapter to use for DDC
• • Read EDID using the given I2C adapter.
• • The EDID may be overridden using debugfs override_edid or firmware EDID
• • (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
• • order. Having either of them bypasses actual EDID reads.
• • Prefer initializing connector->ddc with drm_connector_init_with_ddc() and
• • using drm_edid_read() instead of this function.
• • The returned pointer must be freed using drm_edid_free().
• • Return: Pointer to EDID, or NULL if probe/read failed.
• */

+const struct drm_edid *drm_edid_read_ddc(struct drm_connector *connector,

• 			 struct i2c_adapter *adapter)
  

+{

• const struct drm_edid *drm_edid;
• if (connector->force == DRM_FORCE_OFF)

• return NULL;
  

• if (connector->force == DRM_FORCE_UNSPECIFIED && !drm_probe_ddc(adapter))

• return NULL;
  

• drm_edid = drm_edid_read_custom(connector, drm_do_probe_ddc_edid, adapter);
• /* Note: Do *not* call connector updates here. */
• return drm_edid;

+} +EXPORT_SYMBOL(drm_edid_read_ddc);

+/**

• • drm_edid_read - Read EDID data using connector's I2C adapter
• • @connector: Connector to use
• • Read EDID using the connector's I2C adapter.
• • The EDID may be overridden using debugfs override_edid or firmware EDID
• • (drm_load_edid_firmware() and drm.edid_firmware parameter), in this priority
• • order. Having either of them bypasses actual EDID reads.
• • The returned pointer must be freed using drm_edid_free().
• • Return: Pointer to EDID, or NULL if probe/read failed.
• */

+const struct drm_edid *drm_edid_read(struct drm_connector *connector) +{

• if (drm_WARN_ON(connector->dev, !connector->ddc))

• return NULL;
  

• return drm_edid_read_ddc(connector, connector->ddc);

+} +EXPORT_SYMBOL(drm_edid_read);

I'm wondering if we need this drm_edid_read() vs. drm_edid_read_ddc() split? Ie. are there cases where connector->ddc wouldn't be populated for some reason but we still want to use drm_edid_read_ddc()?

I want to promote initializing connector->ddc and using drm_edid_read() with that. However, for e.g. i915, HDMI is the only connector where we have connector->ddc initialized. I just didn't want to take on converting that part too, not yet, not as part of this series.

BR, Jani.

...

static u32 edid_extract_panel_id(const struct edid *edid) { /* diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 95ac09ef41b2..9d2d78135dee 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -594,6 +594,15 @@ drm_display_mode_from_cea_vic(struct drm_device *dev, u8 video_code);

/* Interface based on struct drm_edid */ +const struct drm_edid *drm_edid_alloc(const void *edid, size_t size); +const struct drm_edid *drm_edid_dup(const struct drm_edid *drm_edid); +void drm_edid_free(const struct drm_edid *drm_edid); +const struct drm_edid *drm_edid_read(struct drm_connector *connector); +const struct drm_edid *drm_edid_read_ddc(struct drm_connector *connector,

• 			 struct i2c_adapter *adapter);
  

+const struct drm_edid *drm_edid_read_custom(struct drm_connector *connector,

• 			    int (*read_block)(void *context, u8 *buf, unsigned int block, size_t len),
  

• 			    void *context);
  

const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid, int ext_id, int *ext_index);

-- 2.30.2

On Wed, Jun 08, 2022 at 10:50:36AM +0300, Jani Nikula wrote:

Add a new function drm_edid_connector_update() to replace the combination of calls drm_connector_update_edid_property() and drm_add_edid_modes(). Usually they are called in the drivers in this order, however the former needs information from the latter.

Since the new drm_edid_read*() functions no longer call the connector updates directly, and the read and update are separated, we'll need this new function for the connector update.

This is all in drm_edid.c simply to keep struct drm_edid opaque.

Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Jani Nikula jani.nikula@intel.com

---

drivers/gpu/drm/drm_connector.c | 2 + drivers/gpu/drm/drm_edid.c | 71 +++++++++++++++++++++++++++++++-- include/drm/drm_edid.h | 2 + 3 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 1c48d162c77e..ae9c640a641a 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2088,6 +2088,8 @@ EXPORT_SYMBOL(drm_connector_set_tile_property);

• set the connector's tile property here. See drm_connector_set_tile_property()
• for more details.

• • This function is deprecated. Use drm_edid_connector_update() instead.
• • Returns:
  • Zero on success, negative errno on failure.
  */

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2bdaf1e34a9d..952724788963 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -6143,8 +6143,8 @@ static int add_displayid_detailed_modes(struct drm_connector *connector, return num_modes; }

-static int drm_edid_connector_update(struct drm_connector *connector,

• 		     const struct drm_edid *drm_edid)
  

+static int _drm_edid_connector_update(struct drm_connector *connector,

• 		      const struct drm_edid *drm_edid)
  

{ int num_modes = 0; u32 quirks; @@ -6207,6 +6207,67 @@ static int drm_edid_connector_update(struct drm_connector *connector, return num_modes; }

+static void _drm_update_tile_info(struct drm_connector *connector,

• 		  const struct drm_edid *drm_edid);
  

+/**

• • drm_edid_connector_update - Update connector information from EDID
• • @connector: Connector
• • @drm_edid: EDID
• • Update the connector mode list, 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.
• */

+int drm_edid_connector_update(struct drm_connector *connector,

• 	      const struct drm_edid *drm_edid)
  

+{

• struct drm_device *dev = connector->dev;
• const struct edid *old_edid = connector->edid_blob_ptr ?

• connector->edid_blob_ptr->data : NULL;
  

• const struct edid *edid = drm_edid ? drm_edid->edid : NULL;
• size_t size = drm_edid ? drm_edid->size : 0;
• int count, ret;
• count = _drm_edid_connector_update(connector, drm_edid);
• _drm_update_tile_info(connector, drm_edid);
• if (old_edid && !drm_edid_are_equal(edid, old_edid)) {

The old_edid check looks a bit odd. Can't really see why we'd not want to bump the epoch counter when we go from not having and EDID to having one. This issue already seems to exist in the current code, although that one also skips the epoch counter bump if the new EDID is missing, so even more odd.

This also brings me to the slight annoynace of having this code duplicated in two places. I worry we'll end up accumulating different bugs in each copy. Can't we refactor to have just one copy?

• connector->epoch_counter++;
  

• drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID changed, epoch counter %llu\n",
  

• 	    connector->base.id, connector->name,
  

• 	    connector->epoch_counter);
  

• }
• ret = drm_property_replace_global_blob(dev, &connector->edid_blob_ptr,

• 			       size, edid,
  

• 			       &connector->base,
  

• 			       dev->mode_config.edid_property);
  

• if (ret)

• drm_dbg_kms(dev, "[CONNECTOR:%d:%s] EDID property update failed (%d)\n",
  

• 	    connector->base.id, connector->name, ret);
  

• ret = drm_object_property_set_value(&connector->base,

• 			    dev->mode_config.non_desktop_property,
  

• 			    connector->display_info.non_desktop);
  

• if (ret)

• drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Non-desktop property update failed (%d)\n",
  

• 	    connector->base.id, connector->name, ret);
  

• ret = drm_connector_set_tile_property(connector);
• if (ret)

• drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Tile property update failed (%d)\n",
  

• 	    connector->base.id, connector->name, ret);
  

• return count;

+} +EXPORT_SYMBOL(drm_edid_connector_update);

/**

• drm_add_edid_modes - add modes from EDID data, if available
• @connector: connector we're probing

@@ -6216,6 +6277,8 @@ static int drm_edid_connector_update(struct drm_connector *connector,

• &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.
• • Return: The number of modes added or 0 if we couldn't find any.
  */

int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) @@ -6228,8 +6291,8 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) edid = NULL; }

• return drm_edid_connector_update(connector,

• 			 drm_edid_legacy_init(&drm_edid, edid));
  

• return _drm_edid_connector_update(connector,

• 			  drm_edid_legacy_init(&drm_edid, edid));
  

} EXPORT_SYMBOL(drm_add_edid_modes);

diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 9d2d78135dee..aeb2fa95bc04 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -603,6 +603,8 @@ const struct drm_edid *drm_edid_read_ddc(struct drm_connector *connector, const struct drm_edid *drm_edid_read_custom(struct drm_connector *connector, int (*read_block)(void *context, u8 *buf, unsigned int block, size_t len), void *context); +int drm_edid_connector_update(struct drm_connector *connector,

• 	      const struct drm_edid *edid);
  

const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid, int ext_id, int *ext_index);

-- 2.30.2

On Wed, Jun 08, 2022 at 10:50:37AM +0300, Jani Nikula wrote:

Abstract the .get_modes() connector helper call, including the override/firmware EDID fallback, for clarity.

Signed-off-by: Jani Nikula jani.nikula@intel.com

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

---

drivers/gpu/drm/drm_probe_helper.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 75a71649b64d..a8d26b29bfa0 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -354,6 +354,24 @@ drm_helper_probe_detect(struct drm_connector *connector, } EXPORT_SYMBOL(drm_helper_probe_detect);

+static int drm_helper_probe_get_modes(struct drm_connector *connector) +{

• const struct drm_connector_helper_funcs *connector_funcs =

• connector->helper_private;
  

• int count;
• count = connector_funcs->get_modes(connector);
• /*

• * Fallback for when DDC probe failed in drm_get_edid() and thus skipped
  

• * override/firmware EDID.
  

• */
  

• if (count == 0 && connector->status == connector_status_connected)

• count = drm_add_override_edid_modes(connector);
  

• return count;

+}

static int __drm_helper_update_and_validate(struct drm_connector *connector, uint32_t maxX, uint32_t maxY, struct drm_modeset_acquire_ctx *ctx) @@ -473,8 +491,6 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, { struct drm_device *dev = connector->dev; struct drm_display_mode *mode;

• const struct drm_connector_helper_funcs *connector_funcs =

• connector->helper_private;
  

  int count = 0, ret; enum drm_connector_status old_status; struct drm_modeset_acquire_ctx ctx;

@@ -559,14 +575,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, goto exit; }

• count = (*connector_funcs->get_modes)(connector);
• /*

• * Fallback for when DDC probe failed in drm_get_edid() and thus skipped
  

• * override/firmware EDID.
  

• */
  

• if (count == 0 && connector->status == connector_status_connected)

• count = drm_add_override_edid_modes(connector);
  

• count = drm_helper_probe_get_modes(connector);

  if (count == 0 && (connector->status == connector_status_connected || connector->status == connector_status_unknown)) {

-- 2.30.2

On Wed, Jun 08, 2022 at 10:50:38AM +0300, Jani Nikula wrote:

Add a helper function to be used as the "default" .get_modes() hook. This also works as an example of what the driver .get_modes() hooks are supposed to do regarding the new drm_edid_read*() and drm_edid_connector_update() calls.

Signed-off-by: Jani Nikula jani.nikula@intel.com

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

---

drivers/gpu/drm/drm_probe_helper.c | 34 ++++++++++++++++++++++++++++++ include/drm/drm_probe_helper.h | 1 + 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index a8d26b29bfa0..e6b8f2923aa7 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -1049,3 +1049,37 @@ int drm_connector_helper_get_modes_from_ddc(struct drm_connector *connector) return count; } EXPORT_SYMBOL(drm_connector_helper_get_modes_from_ddc);

+/**

• • drm_connector_helper_get_modes - Read EDID and update connector.
• • @connector: The connector
• • Read the EDID using drm_edid_read() (which requires that connector->ddc is
• • set), and update the connector using the EDID.
• • This can be used as the "default" connector helper .get_modes() hook if the
• • driver does not need any special processing. This is sets the example what
• • custom .get_modes() hooks should do regarding EDID read and connector update.
• • Returns: Number of modes.
• */

+int drm_connector_helper_get_modes(struct drm_connector *connector) +{

• const struct drm_edid *drm_edid;
• int count;
• drm_edid = drm_edid_read(connector);
• /*

• * Unconditionally update the connector. If the EDID was read
  

• * succesfully, fill in the connector information derived from the
  

• * EDID. Otherwise, if the EDID is NULL, clear the connector
  

• * information.
  

• */
  

• count = drm_edid_connector_update(connector, drm_edid);
• drm_edid_free(drm_edid);
• return count;

+} +EXPORT_SYMBOL(drm_connector_helper_get_modes); diff --git a/include/drm/drm_probe_helper.h b/include/drm/drm_probe_helper.h index c80cab7a53b7..8075e02aa865 100644 --- a/include/drm/drm_probe_helper.h +++ b/include/drm/drm_probe_helper.h @@ -27,5 +27,6 @@ void drm_kms_helper_poll_enable(struct drm_device *dev); bool drm_kms_helper_is_poll_worker(void);

int drm_connector_helper_get_modes_from_ddc(struct drm_connector *connector); +int drm_connector_helper_get_modes(struct drm_connector *connector);

#endif

2.30.2

On Wed, Jun 08, 2022 at 10:50:39AM +0300, Jani Nikula wrote:

Unfortunately, there are still plenty of interfaces around that require a struct edid pointer, and it's impossible to change them all at once. Add an accessor to the raw EDID data to help the transition.

While there are no such cases now, be defensive against raw EDID extension count indicating bigger EDID than is actually allocated.

Signed-off-by: Jani Nikula jani.nikula@intel.com

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

---

drivers/gpu/drm/drm_edid.c | 26 ++++++++++++++++++++++++++ include/drm/drm_edid.h | 1 + 2 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 952724788963..4e788c5cbf25 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2333,6 +2333,32 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, } EXPORT_SYMBOL_GPL(drm_do_get_edid);

+/**

• • drm_edid_raw - Get a pointer to the raw EDID data.
• • @drm_edid: drm_edid container
• • Get a pointer to the raw EDID data.
• • This is for transition only. Avoid using this like the plague.
• • Return: Pointer to raw EDID data.
• */

+const struct edid *drm_edid_raw(const struct drm_edid *drm_edid) +{

• if (!drm_edid || !drm_edid->size)

• return NULL;
  

• /*

• * Do not return pointers where relying on EDID extension count would
  

• * lead to buffer overflow.
  

• */
  

• if (WARN_ON(edid_size(drm_edid->edid) > drm_edid->size))

• return NULL;
  

• return drm_edid->edid;

+} +EXPORT_SYMBOL(drm_edid_raw);

/* Allocate struct drm_edid container *without* duplicating the edid data */ static const struct drm_edid *_drm_edid_alloc(const void *edid, size_t size) { diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index aeb2fa95bc04..2181977ae683 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -597,6 +597,7 @@ drm_display_mode_from_cea_vic(struct drm_device *dev, const struct drm_edid *drm_edid_alloc(const void *edid, size_t size); const struct drm_edid *drm_edid_dup(const struct drm_edid *drm_edid); void drm_edid_free(const struct drm_edid *drm_edid); +const struct edid *drm_edid_raw(const struct drm_edid *drm_edid); const struct drm_edid *drm_edid_read(struct drm_connector *connector); const struct drm_edid *drm_edid_read_ddc(struct drm_connector *connector, struct i2c_adapter *adapter); -- 2.30.2

On Wed, Jun 08, 2022 at 10:50:40AM +0300, Jani Nikula wrote:

Convert all the connectors that use cached connector edid and detect_edid to drm_edid.

Signed-off-by: Jani Nikula jani.nikula@intel.com

.../gpu/drm/i915/display/intel_connector.c | 4 +- .../drm/i915/display/intel_display_types.h | 4 +- drivers/gpu/drm/i915/display/intel_dp.c | 74 ++++++++++--------- drivers/gpu/drm/i915/display/intel_hdmi.c | 26 ++++--- drivers/gpu/drm/i915/display/intel_lvds.c | 37 +++++----- 5 files changed, 79 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c index 1dcc268927a2..d83b2a64f618 100644 --- a/drivers/gpu/drm/i915/display/intel_connector.c +++ b/drivers/gpu/drm/i915/display/intel_connector.c @@ -95,12 +95,12 @@ void intel_connector_destroy(struct drm_connector *connector) { struct intel_connector *intel_connector = to_intel_connector(connector);

• kfree(intel_connector->detect_edid);

• drm_edid_free(intel_connector->detect_edid);

  intel_hdcp_cleanup(intel_connector);

  if (!IS_ERR_OR_NULL(intel_connector->edid))

• kfree(intel_connector->edid);
  

• drm_edid_free(intel_connector->edid);
  
  

  intel_panel_fini(intel_connector);

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 9b44358e8d9e..dffbc7d59587 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -590,8 +590,8 @@ struct intel_connector { struct intel_panel panel;

/* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */

• struct edid *edid;
• struct edid *detect_edid;

• const struct drm_edid *edid;

• const struct drm_edid *detect_edid;

  /* Number of times hotplug detection was tried after an HPD interrupt */ int hotplug_retries;

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index e198c6d7e3b5..64b6481225f1 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3534,12 +3534,11 @@ static u8 intel_dp_autotest_edid(struct intel_dp *intel_dp) intel_dp->aux.i2c_defer_count); intel_dp->compliance.test_data.edid = INTEL_DP_RESOLUTION_FAILSAFE; } else {

• struct edid *block = intel_connector->detect_edid;
  

• /* FIXME: Get rid of drm_edid_raw() */
  

• const struct edid *block = drm_edid_raw(intel_connector->detect_edid);
  
  

• /* We have to write the checksum
  

•  * of the last block read
  

•  */
  

• block += intel_connector->detect_edid->extensions;
  

• /* We have to write the checksum of the last block read */
  

• block += block->extensions;
  
  

  if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_EDID_CHECKSUM, block->checksum) <= 0)

@@ -4418,7 +4417,7 @@ bool intel_digital_port_connected(struct intel_encoder *encoder) return is_connected; }

-static struct edid * +static const struct drm_edid * intel_dp_get_edid(struct intel_dp *intel_dp) { struct intel_connector *intel_connector = intel_dp->attached_connector; @@ -4429,18 +4428,22 @@ intel_dp_get_edid(struct intel_dp *intel_dp) if (IS_ERR(intel_connector->edid)) return NULL;

• return drm_edid_duplicate(intel_connector->edid);
  

• return drm_edid_dup(intel_connector->edid);
  

  } else

• return drm_get_edid(&intel_connector->base,
  

• 		    &intel_dp->aux.ddc);
  

• return drm_edid_read_ddc(&intel_connector->base,
  

• 			 &intel_dp->aux.ddc);
  

}

static void intel_dp_update_dfp(struct intel_dp *intel_dp,

•     const struct edid *edid)
  

•     const struct drm_edid *drm_edid)
  

{ struct drm_i915_private *i915 = dp_to_i915(intel_dp); struct intel_connector *connector = intel_dp->attached_connector;

• const struct edid *edid;

• /* FIXME: Get rid of drm_edid_raw() */

• edid = drm_edid_raw(drm_edid);

  intel_dp->dfp.max_bpc = drm_dp_downstream_max_bpc(intel_dp->dpcd,

@@ -4540,21 +4543,24 @@ intel_dp_set_edid(struct intel_dp *intel_dp) { struct drm_i915_private *i915 = dp_to_i915(intel_dp); struct intel_connector *connector = intel_dp->attached_connector;

• struct edid *edid;

• const struct drm_edid *drm_edid;

• const struct edid *edid; bool vrr_capable;

  intel_dp_unset_edid(intel_dp);

• edid = intel_dp_get_edid(intel_dp);
• connector->detect_edid = edid;

• drm_edid = intel_dp_get_edid(intel_dp);

• connector->detect_edid = drm_edid;

  vrr_capable = intel_vrr_is_capable(connector); drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] VRR capable: %s\n", connector->base.base.id, connector->base.name, str_yes_no(vrr_capable)); drm_connector_set_vrr_capable_property(&connector->base, vrr_capable);

• intel_dp_update_dfp(intel_dp, edid);

• intel_dp_update_dfp(intel_dp, drm_edid); intel_dp_update_420(intel_dp);

• /* FIXME: Get rid of drm_edid_raw() */

• edid = drm_edid_raw(drm_edid); if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { intel_dp->has_hdmi_sink = drm_detect_hdmi_monitor(edid); intel_dp->has_audio = drm_detect_monitor_audio(edid);

@@ -4569,7 +4575,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp) struct intel_connector *connector = intel_dp->attached_connector;

drm_dp_cec_unset_edid(&intel_dp->aux);

• kfree(connector->detect_edid);

• drm_edid_free(connector->detect_edid); connector->detect_edid = NULL;

  intel_dp->has_hdmi_sink = false;

@@ -4733,12 +4739,11 @@ intel_dp_force(struct drm_connector *connector) static int intel_dp_get_modes(struct drm_connector *connector) { struct intel_connector *intel_connector = to_intel_connector(connector);

• struct edid *edid;

• const struct drm_edid *drm_edid; int num_modes = 0;

• edid = intel_connector->detect_edid;
• if (edid)

• num_modes = intel_connector_update_modes(connector, edid);
  

• drm_edid = intel_connector->detect_edid;

• num_modes = drm_edid_connector_update(connector, drm_edid);

  /* Also add fixed mode, which may or may not be present in EDID */ if (intel_dp_is_edp(intel_attached_dp(intel_connector)))

@@ -4747,7 +4752,7 @@ static int intel_dp_get_modes(struct drm_connector *connector) if (num_modes) return num_modes;

• if (!edid) {

• if (!drm_edid) { struct intel_dp *intel_dp = intel_attached_dp(intel_connector); struct drm_display_mode *mode;

@@ -5154,7 +5159,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, struct drm_display_mode *fixed_mode; bool has_dpcd; enum pipe pipe = INVALID_PIPE;

• struct edid *edid;

• const struct drm_edid *drm_edid;

  if (!intel_dp_is_edp(intel_dp)) return true;

@@ -5187,29 +5192,32 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, }

mutex_lock(&dev->mode_config.mutex);

• edid = drm_get_edid(connector, &intel_dp->aux.ddc);
• if (!edid) {

• drm_edid = drm_edid_read_ddc(connector, &intel_dp->aux.ddc);
• if (!drm_edid) {

• const struct edid *edid;
  

• /* Fallback to EDID from ACPI OpRegion, if any */

• /* FIXME: Make intel_opregion_get_edid() return drm_edid */
  

  edid = intel_opregion_get_edid(intel_connector);

• if (edid)
  

• if (edid) {
  

• 	drm_edid = drm_edid_alloc(edid, (edid->extensions + 1) * EDID_LENGTH);
  

Didn't drm_edid_alloc() duplicate the whole edid? Are we potentially leaking here now?

	drm_dbg_kms(&dev_priv->drm,
		    "[CONNECTOR:%d:%s] Using OpRegion EDID\n",
		    connector->base.id, connector->name);

• }
  

  }

• if (edid) {

• if (drm_add_edid_modes(connector, edid)) {
  

• 	drm_connector_update_edid_property(connector, edid);
  

• } else {
  

• 	kfree(edid);
  

• 	edid = ERR_PTR(-EINVAL);
  

• if (drm_edid) {

• if (!drm_edid_connector_update(connector, drm_edid)) {
  

• 	drm_edid_free(drm_edid);
  

• 	drm_edid = ERR_PTR(-EINVAL);
  

  } } else {

• edid = ERR_PTR(-ENOENT);
  

• drm_edid = ERR_PTR(-ENOENT);
  

  }

• intel_connector->edid = edid;

• intel_connector->edid = drm_edid;

  intel_bios_init_panel(dev_priv, &intel_connector->panel,

• 	      IS_ERR(edid) ? NULL : edid);
  

• 	      IS_ERR_OR_NULL(drm_edid) ? NULL : drm_edid_raw(drm_edid));
  
  

  intel_panel_add_edid_fixed_modes(intel_connector, intel_connector->panel.vbt.drrs_type != DRRS_TYPE_NONE,

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 1ae09431f53a..db33a0ccd7ee 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2340,7 +2340,7 @@ intel_hdmi_unset_edid(struct drm_connector *connector) intel_hdmi->dp_dual_mode.type = DRM_DP_DUAL_MODE_NONE; intel_hdmi->dp_dual_mode.max_tmds_clock = 0;

• kfree(to_intel_connector(connector)->detect_edid);

• drm_edid_free(to_intel_connector(connector)->detect_edid); to_intel_connector(connector)->detect_edid = NULL;

}

@@ -2407,7 +2407,8 @@ intel_hdmi_set_edid(struct drm_connector *connector) struct drm_i915_private *dev_priv = to_i915(connector->dev); struct intel_hdmi *intel_hdmi = intel_attached_hdmi(to_intel_connector(connector)); intel_wakeref_t wakeref;

• struct edid *edid;

• const struct drm_edid *drm_edid;
• const struct edid *edid; bool connected = false; struct i2c_adapter *i2c;

@@ -2415,21 +2416,24 @@ intel_hdmi_set_edid(struct drm_connector *connector)

i2c = intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);

• edid = drm_get_edid(connector, i2c);

• drm_edid = drm_edid_read_ddc(connector, i2c);

• if (!edid && !intel_gmbus_is_forced_bit(i2c)) {

• if (!drm_edid && !intel_gmbus_is_forced_bit(i2c)) { drm_dbg_kms(&dev_priv->drm, "HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n"); intel_gmbus_force_bit(i2c, true);

• edid = drm_get_edid(connector, i2c);
  

• drm_edid = drm_edid_read_ddc(connector, i2c);
  

  intel_gmbus_force_bit(i2c, false); }

• intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);

• intel_hdmi_dp_dual_mode_detect(connector, drm_edid != NULL);

  intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);

• to_intel_connector(connector)->detect_edid = edid;

• to_intel_connector(connector)->detect_edid = drm_edid;
• /* FIXME: Get rid of drm_edid_raw() */
• edid = drm_edid_raw(drm_edid); if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { intel_hdmi->has_audio = drm_detect_monitor_audio(edid); intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);

@@ -2501,13 +2505,11 @@ intel_hdmi_force(struct drm_connector *connector)

static int intel_hdmi_get_modes(struct drm_connector *connector) {

• struct edid *edid;

• const struct drm_edid *drm_edid;

• edid = to_intel_connector(connector)->detect_edid;
• if (edid == NULL)

• return 0;
  

• drm_edid = to_intel_connector(connector)->detect_edid;

• return intel_connector_update_modes(connector, edid);

• return drm_edid_connector_update(connector, drm_edid);

}

static struct i2c_adapter * diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c index e55802b45461..d4389054bf59 100644 --- a/drivers/gpu/drm/i915/display/intel_lvds.c +++ b/drivers/gpu/drm/i915/display/intel_lvds.c @@ -479,7 +479,7 @@ static int intel_lvds_get_modes(struct drm_connector *connector)

/* use cached edid if we have one */ if (!IS_ERR_OR_NULL(intel_connector->edid))

• return drm_add_edid_modes(connector, intel_connector->edid);
  

• return drm_edid_connector_update(connector, intel_connector->edid);
  
  

  return intel_panel_get_modes(intel_connector);

} @@ -829,7 +829,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv) struct intel_connector *intel_connector; struct drm_connector *connector; struct drm_encoder *encoder;

• struct edid *edid;

• const struct drm_edid *drm_edid; i915_reg_t lvds_reg; u32 lvds; u8 pin;

@@ -948,27 +948,30 @@ void intel_lvds_init(struct drm_i915_private *dev_priv) * preferred mode is the right one. */ mutex_lock(&dev->mode_config.mutex);

• if (vga_switcheroo_handler_flags() & VGA_SWITCHEROO_CAN_SWITCH_DDC)

• if (vga_switcheroo_handler_flags() & VGA_SWITCHEROO_CAN_SWITCH_DDC) {

• const struct edid *edid;
  

• /* FIXME: Make drm_get_edid_switcheroo() return drm_edid */
  

  edid = drm_get_edid_switcheroo(connector,

• 		    intel_gmbus_get_adapter(dev_priv, pin));
  

• else

• edid = drm_get_edid(connector,
  

• 		    intel_gmbus_get_adapter(dev_priv, pin));
  

• if (edid) {

• if (drm_add_edid_modes(connector, edid)) {
  

• 	drm_connector_update_edid_property(connector,
  

• 						edid);
  

• } else {
  

• 	kfree(edid);
  

• 	edid = ERR_PTR(-EINVAL);
  

• 			       intel_gmbus_get_adapter(dev_priv, pin));
  

• if (edid)
  

• 	drm_edid = drm_edid_alloc(edid, (edid->extensions + 1) * EDID_LENGTH);
  

• } else {

• drm_edid = drm_edid_read_ddc(connector,
  

• 			     intel_gmbus_get_adapter(dev_priv, pin));
  

• }
• if (drm_edid) {

• if (!drm_edid_connector_update(connector, drm_edid)) {
  

• 	drm_edid_free(drm_edid);
  

• 	drm_edid = ERR_PTR(-EINVAL);
  

  } } else {

• edid = ERR_PTR(-ENOENT);
  

• drm_edid = ERR_PTR(-ENOENT);
  

  }

• intel_connector->edid = edid;

• intel_connector->edid = drm_edid;

  intel_bios_init_panel(dev_priv, &intel_connector->panel,

• 	      IS_ERR(edid) ? NULL : edid);
  

• 	      IS_ERR_OR_NULL(drm_edid) ? NULL : drm_edid_raw(drm_edid));
  
  

  /* Try EDID first */ intel_panel_add_edid_fixed_modes(intel_connector,

-- 2.30.2

On Wed, Jun 08, 2022 at 10:50:41AM +0300, Jani Nikula wrote:

Try to use struct drm_edid where possible, even if having to fall back to looking into struct edid down low via drm_edid_raw().

Signed-off-by: Jani Nikula jani.nikula@intel.com

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

---

drivers/gpu/drm/i915/display/intel_bios.c | 23 ++++++++++++----------- drivers/gpu/drm/i915/display/intel_bios.h | 4 ++-- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- drivers/gpu/drm/i915/display/intel_lvds.c | 2 +- 4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index c42b9e7d0dce..be0b4264d526 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -604,13 +604,13 @@ get_lfp_data_tail(const struct bdb_lvds_lfp_data *data, }

static int opregion_get_panel_type(struct drm_i915_private *i915,

• 		   const struct edid *edid)
  

• 		   const struct drm_edid *drm_edid)
  

{ return intel_opregion_get_panel_type(i915); }

static int vbt_get_panel_type(struct drm_i915_private *i915,

• 	      const struct edid *edid)
  

• 	      const struct drm_edid *drm_edid)
  

{ const struct bdb_lvds_options *lvds_options;

@@ -629,12 +629,13 @@ static int vbt_get_panel_type(struct drm_i915_private *i915, }

static int pnpid_get_panel_type(struct drm_i915_private *i915,

• 		const struct edid *edid)
  

• 		const struct drm_edid *drm_edid)
  

{ const struct bdb_lvds_lfp_data *data; const struct bdb_lvds_lfp_data_ptrs *ptrs; const struct lvds_pnp_id *edid_id; struct lvds_pnp_id edid_id_nodate;

• const struct edid *edid = drm_edid_raw(drm_edid); /* FIXME */ int i, best = -1;

  if (!edid)

@@ -675,7 +676,7 @@ static int pnpid_get_panel_type(struct drm_i915_private *i915, }

static int fallback_get_panel_type(struct drm_i915_private *i915,

• 		   const struct edid *edid)
  

• 		   const struct drm_edid *drm_edid)
  

{ return 0; } @@ -688,12 +689,12 @@ enum panel_type { };

static int get_panel_type(struct drm_i915_private *i915,

• 	  const struct edid *edid)
  

• 	  const struct drm_edid *drm_edid)
  

{ struct { const char *name; int (*get_panel_type)(struct drm_i915_private *i915,

• 		      const struct edid *edid);
  

• 		      const struct drm_edid *drm_edid);
  

  int panel_type; } panel_types[] = { [PANEL_TYPE_OPREGION] = {

@@ -716,7 +717,7 @@ static int get_panel_type(struct drm_i915_private *i915, int i;

for (i = 0; i < ARRAY_SIZE(panel_types); i++) {

• panel_types[i].panel_type = panel_types[i].get_panel_type(i915, edid);
  

• panel_types[i].panel_type = panel_types[i].get_panel_type(i915, drm_edid);
  
  

  drm_WARN_ON(&i915->drm, panel_types[i].panel_type > 0xf && panel_types[i].panel_type != 0xff);

@@ -747,7 +748,7 @@ static int get_panel_type(struct drm_i915_private *i915, static void parse_panel_options(struct drm_i915_private *i915, struct intel_panel *panel,

•     const struct edid *edid)
  

•     const struct drm_edid *drm_edid)
  

{ const struct bdb_lvds_options *lvds_options; int panel_type; @@ -759,7 +760,7 @@ parse_panel_options(struct drm_i915_private *i915,

panel->vbt.lvds_dither = lvds_options->pixel_dither;

• panel_type = get_panel_type(i915, edid);

• panel_type = get_panel_type(i915, drm_edid);

  panel->vbt.panel_type = panel_type;

@@ -3092,11 +3093,11 @@ void intel_bios_init(struct drm_i915_private *i915)

void intel_bios_init_panel(struct drm_i915_private *i915, struct intel_panel *panel,

• 	   const struct edid *edid)
  

• 	   const struct drm_edid *drm_edid)
  

{ init_vbt_panel_defaults(panel);

• parse_panel_options(i915, panel, edid);

• parse_panel_options(i915, panel, drm_edid); parse_generic_dtd(i915, panel); parse_lfp_data(i915, panel); parse_lfp_backlight(i915, panel);

diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h index b112200ae0a0..7bcc818e8d80 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.h +++ b/drivers/gpu/drm/i915/display/intel_bios.h @@ -32,8 +32,8 @@

#include <linux/types.h>

+struct drm_edid; struct drm_i915_private; -struct edid; struct intel_bios_encoder_data; struct intel_crtc_state; struct intel_encoder; @@ -234,7 +234,7 @@ struct mipi_pps_data { void intel_bios_init(struct drm_i915_private *dev_priv); void intel_bios_init_panel(struct drm_i915_private *dev_priv, struct intel_panel *panel,

• 	   const struct edid *edid);
  

• 	   const struct drm_edid *drm_edid);
  

void intel_bios_fini_panel(struct intel_panel *panel); void intel_bios_driver_remove(struct drm_i915_private *dev_priv); bool intel_bios_is_valid_vbt(const void *buf, size_t size); diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 64b6481225f1..2f47f1e7607b 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5217,7 +5217,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, intel_connector->edid = drm_edid;

intel_bios_init_panel(dev_priv, &intel_connector->panel,

• 	      IS_ERR_OR_NULL(drm_edid) ? NULL : drm_edid_raw(drm_edid));
  

• 	      IS_ERR(drm_edid) ? NULL : drm_edid);
  
  

  intel_panel_add_edid_fixed_modes(intel_connector, intel_connector->panel.vbt.drrs_type != DRRS_TYPE_NONE,

diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c index d4389054bf59..ed5fceb93952 100644 --- a/drivers/gpu/drm/i915/display/intel_lvds.c +++ b/drivers/gpu/drm/i915/display/intel_lvds.c @@ -971,7 +971,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv) intel_connector->edid = drm_edid;

intel_bios_init_panel(dev_priv, &intel_connector->panel,

• 	      IS_ERR_OR_NULL(drm_edid) ? NULL : drm_edid_raw(drm_edid));
  

• 	      IS_ERR(drm_edid) ? NULL : drm_edid);
  
  

  /* Try EDID first */ intel_panel_add_edid_fixed_modes(intel_connector,

-- 2.30.2

On Wed, Jun 08, 2022 at 10:50:42AM +0300, Jani Nikula wrote:

Rewrite edid_filter_invalid_blocks() to filter invalid blocks in-place. The main motivation is to not rely on passed in information on invalid block count or the allocation size, which will be helpful in follow-up work on HF-EEODB.

Signed-off-by: Jani Nikula jani.nikula@intel.com

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

---

drivers/gpu/drm/drm_edid.c | 43 ++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 4e788c5cbf25..77ec5b0e436d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2020,33 +2020,37 @@ bool drm_edid_is_valid(struct edid *edid) } EXPORT_SYMBOL(drm_edid_is_valid);

-static struct edid *edid_filter_invalid_blocks(const struct edid *edid,

• 			       int invalid_blocks,
  

+static struct edid *edid_filter_invalid_blocks(struct edid *edid, size_t *alloc_size) {

• struct edid *new, *dest_block;
• int valid_extensions = edid->extensions - invalid_blocks;
• int i;

• struct edid *new;
• int i, valid_blocks = 0;

• *alloc_size = edid_size_by_blocks(valid_extensions + 1);

• for (i = 0; i < edid_block_count(edid); i++) {

• const void *src_block = edid_block_data(edid, i);
  
  

• new = kmalloc(*alloc_size, GFP_KERNEL);
• if (!new)

• goto out;
  

• if (edid_block_valid(src_block, i == 0)) {
  

• 	void *dst_block = (void *)edid_block_data(edid, valid_blocks);
  
  

• dest_block = new;
• for (i = 0; i < edid_block_count(edid); i++) {

• const void *block = edid_block_data(edid, i);
  

• 	memmove(dst_block, src_block, EDID_LENGTH);
  

• 	valid_blocks++;
  

• }
  

• }

• if (edid_block_valid(block, i == 0))
  

• 	memcpy(dest_block++, block, EDID_LENGTH);
  

• /* We already trusted the base block to be valid here... */
• if (WARN_ON(!valid_blocks)) {

• kfree(edid);
  

• return NULL;
  

  }

• new->extensions = valid_extensions;
• new->checksum = edid_block_compute_checksum(new);

• edid->extensions = valid_blocks - 1;
• edid->checksum = edid_block_compute_checksum(edid);

-out:

• kfree(edid);

• *alloc_size = edid_size_by_blocks(valid_blocks);

• new = krealloc(edid, *alloc_size, GFP_KERNEL);

• if (!new)

• kfree(edid);
  
  

  return new;

} @@ -2290,8 +2294,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector, if (invalid_blocks) { connector_bad_edid(connector, edid, edid_block_count(edid));

• edid = edid_filter_invalid_blocks(edid, invalid_blocks,
  

• 				  &alloc_size);
  

• edid = edid_filter_invalid_blocks(edid, &alloc_size);
  

  }

ok:

2.30.2

On Wed, Jun 08, 2022 at 10:50:43AM +0300, Jani Nikula wrote:

HDMI 2.1 section 10.3.6 defines an HDMI Forum EDID Extension Override Data Block, which may contain a different extension count than the base block claims. Add support for reading more EDID data if available. The extra blocks aren't parsed yet, though.

Hard-coding the EEODB parsing instead of using the iterators we have is a bit of a bummer, but we have to be able to do this on a partially allocated EDID while reading it.

v2:

• Check for CEA Data Block Collection size (Ville)
• Amend commit message and comment about hard-coded parsing

Signed-off-by: Jani Nikula jani.nikula@intel.com

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

---

drivers/gpu/drm/drm_edid.c | 89 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 86 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 77ec5b0e436d..5cac357e50b0 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1581,6 +1581,15 @@ static bool version_greater(const struct drm_edid *drm_edid, (edid->version == version && edid->revision > revision); }

+static int edid_hfeeodb_extension_block_count(const struct edid *edid);

+static int edid_hfeeodb_block_count(const struct edid *edid) +{

• int eeodb = edid_hfeeodb_extension_block_count(edid);
• return eeodb ? eeodb + 1 : 0;

+}

static int edid_extension_block_count(const struct edid *edid) { return edid->extensions; @@ -2026,6 +2035,11 @@ static struct edid *edid_filter_invalid_blocks(struct edid *edid, struct edid *new; int i, valid_blocks = 0;

• /*

• * Note: If the EDID uses HF-EEODB, but has invalid blocks, we'll revert
  

• * back to regular extension count here. We don't want to start
  

• * modifying the HF-EEODB extension too.
  

• */
  

  for (i = 0; i < edid_block_count(edid); i++) { const void *src_block = edid_block_data(edid, i);

@@ -2235,7 +2249,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector, size_t *size) { enum edid_block_status status;

• int i, invalid_blocks = 0;

• int i, num_blocks, invalid_blocks = 0; struct edid *edid, *new; size_t alloc_size = EDID_LENGTH;

@@ -2277,7 +2291,8 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector, goto fail; edid = new;

• for (i = 1; i < edid_block_count(edid); i++) {

• num_blocks = edid_block_count(edid);

• for (i = 1; i < num_blocks; i++) { void *block = (void *)edid_block_data(edid, i);

  status = edid_block_read(block, i, read_block, context);

@@ -2288,11 +2303,31 @@ static struct edid *_drm_do_get_edid(struct drm_connector *connector, if (status == EDID_BLOCK_READ_FAIL) goto fail; invalid_blocks++;

• } else if (i == 1) {
  

• 	/*
  

• 	 * If the first EDID extension is a CTA extension, and
  

• 	 * the first Data Block is HF-EEODB, override the
  

• 	 * extension block count.
  

• 	 *
  

• 	 * Note: HF-EEODB could specify a smaller extension
  

• 	 * count too, but we can't risk allocating a smaller
  

• 	 * amount.
  

• 	 */
  

• 	int eeodb = edid_hfeeodb_block_count(edid);
  

• 	if (eeodb > num_blocks) {
  

• 		num_blocks = eeodb;
  

• 		alloc_size = edid_size_by_blocks(num_blocks);
  

• 		new = krealloc(edid, alloc_size, GFP_KERNEL);
  

• 		if (!new)
  

• 			goto fail;
  

• 		edid = new;
  

• 	}
  

  } }

  if (invalid_blocks) {

• connector_bad_edid(connector, edid, edid_block_count(edid));
  

• connector_bad_edid(connector, edid, num_blocks);
  
  

  edid = edid_filter_invalid_blocks(edid, &alloc_size); }

@@ -3825,6 +3860,7 @@ static int add_detailed_modes(struct drm_connector *connector, #define CTA_EXT_DB_HDR_STATIC_METADATA 6 #define CTA_EXT_DB_420_VIDEO_DATA 14 #define CTA_EXT_DB_420_VIDEO_CAP_MAP 15 +#define CTA_EXT_DB_HF_EEODB 0x78 #define CTA_EXT_DB_HF_SCDB 0x79

#define EDID_BASIC_AUDIO (1 << 6) @@ -4884,6 +4920,12 @@ static bool cea_db_is_hdmi_forum_vsdb(const struct cea_db *db) cea_db_payload_len(db) >= 7; }

+static bool cea_db_is_hdmi_forum_eeodb(const void *db) +{

• return cea_db_is_extended_tag(db, CTA_EXT_DB_HF_EEODB) &&

• cea_db_payload_len(db) >= 2;
  

+}

static bool cea_db_is_microsoft_vsdb(const struct cea_db *db) { return cea_db_is_vendor(db, MICROSOFT_IEEE_OUI) && @@ -4918,6 +4960,47 @@ static bool cea_db_is_hdmi_hdr_metadata_block(const struct cea_db *db) cea_db_payload_len(db) >= 3; }

+/*

• • Get the HF-EEODB override extension block count from EDID.
• • The passed in EDID may be partially read, as long as it has at least two
• • blocks (base block and one extension block) if EDID extension count is > 0.
• • Note that this is *not* how you should parse CTA Data Blocks in general; this
• • is only to handle partially read EDIDs. Normally, use the CTA Data Block
• • iterators instead.
• • References:
• • • HDMI 2.1 section 10.3.6 HDMI Forum EDID Extension Override Data Block
• */

+static int edid_hfeeodb_extension_block_count(const struct edid *edid) +{

• const u8 *cta;
• /* No extensions according to base block, no HF-EEODB. */
• if (!edid_extension_block_count(edid))

• return 0;
  

• /* HF-EEODB is always in the first EDID extension block only */
• cta = edid_extension_block_data(edid, 0);
• if (edid_block_tag(cta) != CEA_EXT || cea_revision(cta) < 3)

• return 0;
  

• /* Need to have the data block collection, and at least 3 bytes. */
• if (cea_db_collection_size(cta) < 3)

• return 0;
  

• /*

• * Sinks that include the HF-EEODB in their E-EDID shall include one and
  

• * only one instance of the HF-EEODB in the E-EDID, occupying bytes 4
  

• * through 6 of Block 1 of the E-EDID.
  

• */
  

• if (!cea_db_is_hdmi_forum_eeodb(&cta[4]))

• return 0;
  

• return cta[4 + 2];

+}

static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector, const u8 *db) { -- 2.30.2

On Wed, Jun 08, 2022 at 10:50:44AM +0300, Jani Nikula wrote:

Take the HF-EEODB extension count override into account.

Signed-off-by: Jani Nikula jani.nikula@intel.com

drivers/gpu/drm/drm_edid.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 5cac357e50b0..b7b1f0639115 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1629,6 +1629,19 @@ static int drm_edid_block_count(const struct drm_edid *drm_edid) /* Starting point */ num_blocks = edid_block_count(drm_edid->edid);

• /* HF-EEODB override */
• if (drm_edid->size >= edid_size_by_blocks(2)) {

• int eeodb;
  

• /*
  

•  * Note: HF-EEODB may specify a smaller extension count than the
  

•  * regular one. Unlike in buffer allocation, here we can use it.
  

•  */
  

Such a sink would be in violation of the spec. But the spec does explicitly say sources are to ignore the normal ext count in those cases. So this would seem to do what the spec asks of us.

Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

• eeodb = edid_hfeeodb_block_count(drm_edid->edid);
  

• if (eeodb)
  

• 	num_blocks = eeodb;
  

• }
• /* Limit by allocated size */ num_blocks = min(num_blocks, (int)drm_edid->size / EDID_LENGTH);

-- 2.30.2