Add drm_edid based block count and data access helper functions that take the EDID allocated size into account.

At the moment, the allocated size should always match the EDID size indicated by the extension count, but this will change in the future.

Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 42 +++++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 929fc0e46751..682d954a9e42 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1613,6 +1613,35 @@ static const void *edid_extension_block_data(const struct edid *edid, int index) return edid_block_data(edid, index + 1); }

+static int drm_edid_block_count(const struct drm_edid *drm_edid) +{ + int num_blocks; + + /* Starting point */ + num_blocks = edid_block_count(drm_edid->edid); + + /* Limit by allocated size */ + num_blocks = min(num_blocks, (int)drm_edid->size / EDID_LENGTH); + + return num_blocks; +} + +static int drm_edid_extension_block_count(const struct drm_edid *drm_edid) +{ + return drm_edid_block_count(drm_edid) - 1; +} + +static const void *drm_edid_block_data(const struct drm_edid *drm_edid, int index) +{ + return edid_block_data(drm_edid->edid, index); +} + +static const void *drm_edid_extension_block_data(const struct drm_edid *drm_edid, + int index) +{ + return edid_extension_block_data(drm_edid->edid, index); +} + /* * Initializer helper for legacy interfaces, where we have no choice but to * trust edid size. Not for general purpose use. @@ -1665,8 +1694,8 @@ static const void *__drm_edid_iter_next(struct drm_edid_iter *iter) if (!iter->drm_edid) return NULL;

- if (iter->index < edid_block_count(iter->drm_edid->edid)) - block = edid_block_data(iter->drm_edid->edid, iter->index++); + if (iter->index < drm_edid_block_count(iter->drm_edid)) + block = drm_edid_block_data(iter->drm_edid, iter->index++);

return block; } @@ -3574,22 +3603,21 @@ static int add_detailed_modes(struct drm_connector *connector, const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid, int ext_id, int *ext_index) { - const struct edid *edid = drm_edid ? drm_edid->edid : NULL; const u8 *edid_ext = NULL; int i;

/* No EDID or EDID extensions */ - if (!edid || !edid_extension_block_count(edid)) + if (!drm_edid || !drm_edid_extension_block_count(drm_edid)) return NULL;

/* Find CEA extension */ - for (i = *ext_index; i < edid_extension_block_count(edid); i++) { - edid_ext = edid_extension_block_data(edid, i); + for (i = *ext_index; i < drm_edid_extension_block_count(drm_edid); i++) { + edid_ext = drm_edid_extension_block_data(drm_edid, i); if (edid_block_tag(edid_ext) == ext_id) break; }

- if (i >= edid_extension_block_count(edid)) + if (i >= drm_edid_extension_block_count(drm_edid)) return NULL;

*ext_index = i + 1;

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 2132a38ed701..0d640e7dcff7 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 c61e75ab8f63..b729e18968dd 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -581,6 +581,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);

Add default action when .get_modes() not set. This also defines what a .get_modes() hook should do.

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_probe_helper.c | 14 +++++++++++++- include/drm/drm_modeset_helper_vtables.h | 4 ++++ 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 836bcd5b4cb6..9df17f0ae225 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -360,7 +360,19 @@ static int drm_helper_probe_get_modes(struct drm_connector *connector) connector->helper_private; int count;

- count = connector_funcs->get_modes(connector); + if (connector_funcs->get_modes) { + count = connector_funcs->get_modes(connector); + } else { + const struct drm_edid *drm_edid; + + /* Note: This requires connector->ddc is set */ + drm_edid = drm_edid_read(connector); + + /* Update modes etc. from the EDID */ + count = drm_edid_connector_update(connector, drm_edid); + + drm_edid_free(drm_edid); + }

/* * Fallback for when DDC probe failed in drm_get_edid() and thus skipped diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index fafa70ac1337..b4402bc64e57 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -894,6 +894,10 @@ struct drm_connector_helper_funcs { * libraries always call this with the &drm_mode_config.connection_mutex * held. Because of this it's safe to inspect &drm_connector->state. * + * This callback is optional. By default, it reads the EDID using + * drm_edid_read() and updates the connector display info, modes, and + * properties using drm_edid_connector_update(). + * * RETURNS: * * The number of modes added by calling drm_mode_probed_add().

Add a hook for custom .get_edid() when .get_modes() is not set.

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_probe_helper.c | 11 +++++++++-- include/drm/drm_modeset_helper_vtables.h | 21 ++++++++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 9df17f0ae225..42481dc9e6db 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -361,12 +361,19 @@ static int drm_helper_probe_get_modes(struct drm_connector *connector) int count;

if (connector_funcs->get_modes) { + /* No point in having both set */ + drm_WARN_ON_ONCE(connector->dev, connector_funcs->get_edid); + count = connector_funcs->get_modes(connector); } else { const struct drm_edid *drm_edid;

- /* Note: This requires connector->ddc is set */ - drm_edid = drm_edid_read(connector); + if (connector_funcs->get_edid) { + drm_edid = connector_funcs->get_edid(connector); + } else { + /* Note: This requires connector->ddc is set */ + drm_edid = drm_edid_read(connector); + }

/* Update modes etc. from the EDID */ count = drm_edid_connector_update(connector, drm_edid); diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index b4402bc64e57..f4defbdf1768 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -49,6 +49,7 @@ */

enum mode_set_atomic; +struct drm_edid; struct drm_writeback_connector; struct drm_writeback_job;

@@ -894,9 +895,10 @@ struct drm_connector_helper_funcs { * libraries always call this with the &drm_mode_config.connection_mutex * held. Because of this it's safe to inspect &drm_connector->state. * - * This callback is optional. By default, it reads the EDID using - * drm_edid_read() and updates the connector display info, modes, and - * properties using drm_edid_connector_update(). + * This callback is optional. By default, it reads the EDID using the + * .get_edid() callback if set, drm_edid_read() otherwise, and updates + * the connector display info, modes, and properties using + * drm_edid_connector_update(). * * RETURNS: * @@ -904,6 +906,19 @@ struct drm_connector_helper_funcs { */ int (*get_modes)(struct drm_connector *connector);

+ /** + * @get_edid: + * + * If the get_modes() callback is not set, this function gets called to + * retrieve the EDID. This callback is optional. By default, + * drm_edid_read() is used. + * + * This function must return a copy of the EDID; the returned pointer + * will be freed using drm_edid_free(). Usually it would be a copy of a + * previously cached EDID. + */ + const struct drm_edid *(*get_edid)(struct drm_connector *connector); + /** * @detect_ctx: *

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 | 72 ++++++++++--------- drivers/gpu/drm/i915/display/intel_hdmi.c | 26 +++---- drivers/gpu/drm/i915/display/intel_lvds.c | 35 ++++----- 5 files changed, 77 insertions(+), 64 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 408152f9f46a..b72724918761 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -519,8 +519,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 e4a79c11fd25..771857ed0052 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -3511,12 +3511,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) @@ -4395,7 +4394,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; @@ -4406,18 +4405,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, @@ -4517,21 +4520,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->base); 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); @@ -4546,7 +4552,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; @@ -4710,12 +4716,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))) @@ -4724,7 +4729,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;

@@ -5131,7 +5136,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; @@ -5164,26 +5169,29 @@ 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); 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_panel_add_edid_fixed_modes(intel_connector, dev_priv->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 e8478161f8b9..58070f8858e6 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,24 +948,27 @@ 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;

/* Try EDID first */ intel_panel_add_edid_fixed_modes(intel_connector,

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.

Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_edid.c | 81 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 5e0a91da565e..ba0c880dc133 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) @@ -4868,6 +4904,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) && @@ -4902,6 +4944,39 @@ 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. + * + * 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; + + /* + * 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) {

We need to stop duplicating EDID validation and parsing all over the subsystem in various broken ways.

Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Jani Nikula jani.nikula@intel.com --- Documentation/gpu/todo.rst | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 513b20ccef1e..982e4e29825f 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -480,6 +480,31 @@ Contact: Thomas Zimmermann tzimmermann@suse.de

Level: Starter

+Convert core and drivers from struct edid to struct drm_edid +------------------------------------------------------------ + +Go through all drivers and drm core KMS code to convert all raw struct edid +usage to the opaque struct drm_edid. See commit e4ccf9a777d3 ("drm/edid: add +struct drm_edid container") for rationale. + +Convert drm_get_edid() and drm_do_get_edid() usage to drm_edid_read(), +drm_edid_read_ddc(), or drm_edid_read_custom(). + +Convert drm_add_edid_modes() and drm_connector_update_edid_property() to +drm_edid_connector_update(). See drm_helper_probe_get_modes() for reference for +converting the ->get_modes() hooks. + +Convert decentralized, direct struct edid parsing to centralized parsing in +drm_edid.c. Prefer one-time parsing as part of drm_edid_connector_update() and +storing the result in drm_connector->display_info over adding individual, +exported parser functions. + +During the transition period, it may be necessary to use drm_edid_raw(), but do +use it sparingly. Eventually, all of them need to go. + +Contact: Jani Nikula jani.nikula@intel.com + +Level: Intermediate

Core refactorings =================