* [Bug Report] drm/edid: drm_edid_override_connector_update returns a incorrect value
@ 2023-12-06 17:23 bbaa
2023-12-07 10:07 ` Jani Nikula
0 siblings, 1 reply; 4+ messages in thread
From: bbaa @ 2023-12-06 17:23 UTC (permalink / raw)
To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel
Cc: linux-kernel, dri-devel
[-- Attachment #1: Type: text/plain, Size: 3122 bytes --]
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
NVIDIA MODULE:https://github.com/NVIDIA/open-gpu-kernel-modules/blob/4c29105/kernel-open/nvidia-drm/nvidia-drm-connector.c#L106
[-- Attachment #2: Type: text/html, Size: 4060 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Bug Report] drm/edid: drm_edid_override_connector_update returns a incorrect value
2023-12-06 17:23 [Bug Report] drm/edid: drm_edid_override_connector_update returns a incorrect value bbaa
@ 2023-12-07 10:07 ` Jani Nikula
0 siblings, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2023-12-07 10:07 UTC (permalink / raw)
To: bbaa, maarten.lankhorst, mripard, tzimmermann, airlied, daniel
Cc: linux-kernel, dri-devel
On Thu, 07 Dec 2023, bbaa <bbaa@bbaa.fun> 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/87jzpq1go5.fsf@intel.com
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Bug Report] drm/edid: drm_edid_override_connector_update returns a incorrect value
2023-12-06 17:28 bbaa
@ 2023-12-07 9:54 ` Jani Nikula
0 siblings, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2023-12-07 9:54 UTC (permalink / raw)
To: bbaa, maarten.lankhorst, mripard, tzimmermann, airlied, daniel
Cc: linux-kernel, dri-devel
On Thu, 07 Dec 2023, bbaa <bbaa@bbaa.fun> 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/20231207093821.2654267-1-jani.nikula@intel.com
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Bug Report] drm/edid: drm_edid_override_connector_update returns a incorrect value
@ 2023-12-06 17:28 bbaa
2023-12-07 9:54 ` Jani Nikula
0 siblings, 1 reply; 4+ messages in thread
From: bbaa @ 2023-12-06 17:28 UTC (permalink / raw)
To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel
Cc: linux-kernel, dri-devel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-12-07 10:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-06 17:23 [Bug Report] drm/edid: drm_edid_override_connector_update returns a incorrect value bbaa
2023-12-07 10:07 ` Jani Nikula
2023-12-06 17:28 bbaa
2023-12-07 9:54 ` Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).