| Message ID | bcdc925d3a50a40714637c9bf7c71e4c534bd223.1655895388.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/edid: expand on struct drm_edid usage | expand |
On Wed, Jun 22, 2022 at 01:59:19PM +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. > > v2: > - Share code with drm_connector_update_edid_property() (Ville) > - Add comment about override EDID handling > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> Had to take notes to figure who did/does what. But it does look like non-static stuff should end up doing the same thing before and after this patch, apart from the new function that is. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/drm_edid.c | 103 ++++++++++++++++++++++++++++--------- > include/drm/drm_edid.h | 2 + > 2 files changed, 81 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index c3f0f0a5a8a9..41b3de52b8f1 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -6160,8 +6160,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; > @@ -6227,31 +6227,12 @@ static int drm_edid_connector_update(struct drm_connector *connector, > static void _drm_update_tile_info(struct drm_connector *connector, > const struct drm_edid *drm_edid); > > -static int _drm_connector_update_edid_property(struct drm_connector *connector, > +static int _drm_edid_connector_property_update(struct drm_connector *connector, > const struct drm_edid *drm_edid) > { > struct drm_device *dev = connector->dev; > int ret; > > - /* ignore requests to set edid when overridden */ > - if (connector->override_edid) > - return 0; > - > - /* > - * Set the display info, using edid if available, otherwise resetting > - * the values to defaults. This duplicates the work done in > - * drm_add_edid_modes, but that function is not consistently called > - * before this one in all drivers and the computation is cheap enough > - * that it seems better to duplicate it rather than attempt to ensure > - * some arbitrary ordering of calls. > - */ > - if (drm_edid) > - update_display_info(connector, drm_edid); > - else > - drm_reset_display_info(connector); > - > - _drm_update_tile_info(connector, drm_edid); > - > if (connector->edid_blob_ptr) { > const struct edid *old_edid = connector->edid_blob_ptr->data; > > @@ -6297,6 +6278,76 @@ static int _drm_connector_update_edid_property(struct drm_connector *connector, > return ret; > } > > +/** > + * 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) > +{ > + int count; > + > + /* > + * FIXME: Reconcile the differences in override_edid handling between > + * this and drm_connector_update_edid_property(). > + * > + * If override_edid is set, and the EDID passed in here originates from > + * drm_edid_read() and friends, it will be the override EDID, and there > + * are no issues. drm_connector_update_edid_property() ignoring requests > + * to set the EDID dates back to a time when override EDID was not > + * handled at the low level EDID read. > + * > + * The only way the EDID passed in here can be different from the > + * override EDID is when a driver passes in an EDID that does *not* > + * originate from drm_edid_read() and friends, or passes in a stale > + * cached version. This, in turn, is a question of when an override EDID > + * set via debugfs should take effect. > + */ > + > + count = _drm_edid_connector_update(connector, drm_edid); > + > + _drm_update_tile_info(connector, drm_edid); > + > + /* Note: Ignore errors for now. */ > + _drm_edid_connector_property_update(connector, drm_edid); > + > + return count; > +} > +EXPORT_SYMBOL(drm_edid_connector_update); > + > +static int _drm_connector_update_edid_property(struct drm_connector *connector, > + const struct drm_edid *drm_edid) > +{ > + /* ignore requests to set edid when overridden */ > + if (connector->override_edid) > + return 0; > + > + /* > + * Set the display info, using edid if available, otherwise resetting > + * the values to defaults. This duplicates the work done in > + * drm_add_edid_modes, but that function is not consistently called > + * before this one in all drivers and the computation is cheap enough > + * that it seems better to duplicate it rather than attempt to ensure > + * some arbitrary ordering of calls. > + */ > + if (drm_edid) > + update_display_info(connector, drm_edid); > + else > + drm_reset_display_info(connector); > + > + _drm_update_tile_info(connector, drm_edid); > + > + return _drm_edid_connector_property_update(connector, drm_edid); > +} > + > /** > * drm_connector_update_edid_property - update the edid property of a connector > * @connector: drm connector > @@ -6308,6 +6359,8 @@ static int _drm_connector_update_edid_property(struct drm_connector *connector, > * 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. > */ > @@ -6330,6 +6383,8 @@ EXPORT_SYMBOL(drm_connector_update_edid_property); > * &drm_display_info structure and ELD in @connector with any information which > * can be derived from the edid. > * > + * This function is deprecated. Use drm_edid_connector_update() instead. > + * > * 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) > @@ -6342,8 +6397,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
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c3f0f0a5a8a9..41b3de52b8f1 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -6160,8 +6160,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; @@ -6227,31 +6227,12 @@ static int drm_edid_connector_update(struct drm_connector *connector, static void _drm_update_tile_info(struct drm_connector *connector, const struct drm_edid *drm_edid); -static int _drm_connector_update_edid_property(struct drm_connector *connector, +static int _drm_edid_connector_property_update(struct drm_connector *connector, const struct drm_edid *drm_edid) { struct drm_device *dev = connector->dev; int ret; - /* ignore requests to set edid when overridden */ - if (connector->override_edid) - return 0; - - /* - * Set the display info, using edid if available, otherwise resetting - * the values to defaults. This duplicates the work done in - * drm_add_edid_modes, but that function is not consistently called - * before this one in all drivers and the computation is cheap enough - * that it seems better to duplicate it rather than attempt to ensure - * some arbitrary ordering of calls. - */ - if (drm_edid) - update_display_info(connector, drm_edid); - else - drm_reset_display_info(connector); - - _drm_update_tile_info(connector, drm_edid); - if (connector->edid_blob_ptr) { const struct edid *old_edid = connector->edid_blob_ptr->data; @@ -6297,6 +6278,76 @@ static int _drm_connector_update_edid_property(struct drm_connector *connector, return ret; } +/** + * 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) +{ + int count; + + /* + * FIXME: Reconcile the differences in override_edid handling between + * this and drm_connector_update_edid_property(). + * + * If override_edid is set, and the EDID passed in here originates from + * drm_edid_read() and friends, it will be the override EDID, and there + * are no issues. drm_connector_update_edid_property() ignoring requests + * to set the EDID dates back to a time when override EDID was not + * handled at the low level EDID read. + * + * The only way the EDID passed in here can be different from the + * override EDID is when a driver passes in an EDID that does *not* + * originate from drm_edid_read() and friends, or passes in a stale + * cached version. This, in turn, is a question of when an override EDID + * set via debugfs should take effect. + */ + + count = _drm_edid_connector_update(connector, drm_edid); + + _drm_update_tile_info(connector, drm_edid); + + /* Note: Ignore errors for now. */ + _drm_edid_connector_property_update(connector, drm_edid); + + return count; +} +EXPORT_SYMBOL(drm_edid_connector_update); + +static int _drm_connector_update_edid_property(struct drm_connector *connector, + const struct drm_edid *drm_edid) +{ + /* ignore requests to set edid when overridden */ + if (connector->override_edid) + return 0; + + /* + * Set the display info, using edid if available, otherwise resetting + * the values to defaults. This duplicates the work done in + * drm_add_edid_modes, but that function is not consistently called + * before this one in all drivers and the computation is cheap enough + * that it seems better to duplicate it rather than attempt to ensure + * some arbitrary ordering of calls. + */ + if (drm_edid) + update_display_info(connector, drm_edid); + else + drm_reset_display_info(connector); + + _drm_update_tile_info(connector, drm_edid); + + return _drm_edid_connector_property_update(connector, drm_edid); +} + /** * drm_connector_update_edid_property - update the edid property of a connector * @connector: drm connector @@ -6308,6 +6359,8 @@ static int _drm_connector_update_edid_property(struct drm_connector *connector, * 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. */ @@ -6330,6 +6383,8 @@ EXPORT_SYMBOL(drm_connector_update_edid_property); * &drm_display_info structure and ELD in @connector with any information which * can be derived from the edid. * + * This function is deprecated. Use drm_edid_connector_update() instead. + * * 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) @@ -6342,8 +6397,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);
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. v2: - Share code with drm_connector_update_edid_property() (Ville) - Add comment about override EDID handling Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/drm_edid.c | 103 ++++++++++++++++++++++++++++--------- include/drm/drm_edid.h | 2 + 2 files changed, 81 insertions(+), 24 deletions(-)