Hello everyone,
drm_edid_override_connector_update seem return a incorrect value.
drivers/gpu/drm/drm_edid.c (Linux 6.7-rc4)
2294 /**
2295 * drm_edid_override_connector_update - add modes from override/firmware EDID
2296 * @connector: connector we're probing
2297 *
2298 * Add modes from the override/firmware EDID, if available. Only to be used from
2299 * drm_helper_probe_single_connector_modes() as a fallback for when DDC probe
2300 * failed during drm_get_edid() and caused the override/firmware EDID to be
2301 * skipped.
2302 *
2303 * Return: The number of modes added or 0 if we couldn't find any.
2304 */
2305 int drm_edid_override_connector_update(struct drm_connector *connector)
2306 {
2307 const struct drm_edid *override;
2308 int num_modes = 0;
2309
2310 override = drm_edid_override_get(connector);
2311 if (override) {
2312 num_modes = drm_edid_connector_update(connector, override);
2313
2314 drm_edid_free(override);
2315
2316 drm_dbg_kms(connector->dev,
2317 "[CONNECTOR:%d:%s] adding %d modes via fallback override/firmware EDID\n",
2318 connector->base.id, connector->name, num_modes);
2319 }
2320
2321 return num_modes;
2322 }
2323 EXPORT_SYMBOL(drm_edid_override_connector_update);
The comment describes that it will return the number of modes added
However the function calls drm_edid_connector_update, will return 0 upon successful execution.
drivers/gpu/drm/drm_edid.c
6813 /**
6814 * drm_edid_connector_update - Update connector information from EDID
6815 * @connector: Connector
6816 * @drm_edid: EDID
6817 *
6818 * Update the connector display info, ELD, HDR metadata, relevant properties,
6819 * etc. from the passed in EDID.
6820 *
6821 * If EDID is NULL, reset the information.
6822 *
6823 * Must be called before calling drm_edid_connector_add_modes().
6824 *
6825 * Return: 0 on success, negative error on errors.
6826 */
6827 int drm_edid_connector_update(struct drm_connector *connector,
6828 const struct drm_edid *drm_edid)
6829 {
6830 update_display_info(connector, drm_edid);
6831
6832 _drm_update_tile_info(connector, drm_edid);
6833
6834 return _drm_edid_connector_property_update(connector, drm_edid);
6835 }
6836 EXPORT_SYMBOL(drm_edid_connector_update);
This will break the EDID override behavior on Nvidia graphics cards.
NVIDIA/open-gpu-kernel-modules:
kernel-open/nvidia-drm/nvidia-drm-connector.c:
103 #if defined(NV_DRM_CONNECTOR_HAS_OVERRIDE_EDID) 104 if
(connector->override_edid) { 105 #else 106 if
(drm_edid_override_connector_update(connector) > 0) { 107 #endif
108 const struct drm_property_blob *edid =
connector->edid_blob_ptr; 109
drm_edid_override_connector_update(connector) will return zero here.
regards,
bbaa
On Thu, 07 Dec 2023, bbaa <[email protected]> wrote:
> Hello everyone,
>
> drm_edid_override_connector_update seem return a incorrect value.
>
> drivers/gpu/drm/drm_edid.c (Linux 6.7-rc4)
> 2294 /**
> 2295 * drm_edid_override_connector_update - add modes from override/firmware EDID
> 2296 * @connector: connector we're probing
> 2297 *
> 2298 * Add modes from the override/firmware EDID, if available. Only to be used from
> 2299 * drm_helper_probe_single_connector_modes() as a fallback for when DDC probe
> 2300 * failed during drm_get_edid() and caused the override/firmware EDID to be
> 2301 * skipped.
> 2302 *
> 2303 * Return: The number of modes added or 0 if we couldn't find any.
> 2304 */
Thanks for the report. I've sent a patch to hopefully fix this [1].
[1] https://patchwork.freedesktop.org/patch/msgid/[email protected]
However, please read the documentation comment above: "Only to be used
from drm_helper_probe_single_connector_modes() ..."
The function is a fallback for a *very* specific and rare scenario.
> This will break the EDID override behavior on Nvidia graphics cards.
>
> NVIDIA/open-gpu-kernel-modules:
> kernel-open/nvidia-drm/nvidia-drm-connector.c:
> 103 #if defined(NV_DRM_CONNECTOR_HAS_OVERRIDE_EDID) 104 if
> (connector->override_edid) { 105 #else 106 if
> (drm_edid_override_connector_update(connector) > 0) { 107 #endif
> 108 const struct drm_property_blob *edid =
> connector->edid_blob_ptr; 109
> drm_edid_override_connector_update(connector) will return zero here.
That's an out-of-tree driver that doesn't follow the documentation
above. Drivers have no business calling the function.
All of the override/firmware EDID handling should be covered
transparently via the drm_edid_read*() and drm_get_edid() functions, and
the drivers shouldn't have to ever care about overrides, at all. Drivers
shouldn't really use connector->edid_blob_ptr directly either.
Please report and get that fixed downstream.
BR,
Jani.
--
Jani Nikula, Intel
On Thu, 07 Dec 2023, bbaa <[email protected]> wrote:
> Hello everyone,
>
> drm_edid_override_connector_update seem return a incorrect value.
You seem to have posted this twice; already replied at
https://lore.kernel.org/r/[email protected]
--
Jani Nikula, Intel