On Mon, Mar 29, 2021 at 07:53:40PM -0700, Douglas Anderson wrote:
> Each time we call drm_get_edid() we:
> 1. Go out to the bus and ask for the EDID.
> 2. Cache the EDID.
>
> We can improve this to actually use the cached EDID so that if
> drm_get_edid() is called multiple times then we don't need to go out
> to the bus over and over again.
>
> In normal DP/HDMI cases reading the EDID over and over again isn't
> _that_ expensive so, presumably, this wasn't that critical in the
> past. However for eDP going out to the bus can be expensive. This is
> because eDP panels might be powered off before the EDID was requested
> so we need to do power sequencing in addition to the transfer.
>
> In theory we should be able to cache the EDID for all types of
> displays. There is already code throwing the cache away when we detect
> that a display was unplugged. However, it can be noted that it's
> _extra_ safe to cache the EDID for eDP since eDP isn't a hot-pluggable
> interface. If we get the EDID once then we've got the EDID and we
> should never need to read it again. For now we'll only use the cache
> for eDP both because it's more important and extra safe.
>
> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> ---
>
> (no changes since v1)
>
> drivers/gpu/drm/drm_edid.c | 32 ++++++++++++++++++++++++++++----
> 1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index c2bbe7bee7b6..fcbf468d73c9 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2049,15 +2049,39 @@ struct edid *drm_get_edid(struct drm_connector *connector,
> struct i2c_adapter *adapter)
> {
> struct edid *edid;
> + size_t old_edid_size;
> + const struct edid *old_edid;
>
> if (connector->force == DRM_FORCE_OFF)
> return NULL;
>
> - if (connector->force == DRM_FORCE_UNSPECIFIED && !drm_probe_ddc(adapter))
> - return NULL;
> + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP &&
> + connector->edid_blob_ptr) {
> + /*
> + * eDP devices are non-removable, or at least not something
> + * that's expected to be hot-pluggable. We can freely use
> + * the cached EDID.
> + *
> + * NOTE: technically we could probably even use the cached
> + * EDID even for non-eDP because the cached EDID should be
> + * cleared if we ever notice a display is not connected, but
> + * we'll use an abundance of caution and only do it for eDP.
> + * It's more important for eDP anyway because the EDID may not
> + * always be readable, like when the panel is powered down.
> + */
> + old_edid = (const struct edid *)connector->edid_blob_ptr->data;
> + old_edid_size = ksize(old_edid);
> + edid = kmalloc(old_edid_size, GFP_KERNEL);
> + if (edid)
> + memcpy(edid, old_edid, old_edid_size);
> + } else {
> + if (connector->force == DRM_FORCE_UNSPECIFIED && !drm_probe_ddc(adapter))
> + return NULL;
> +
> + edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter);
> + drm_connector_update_edid_property(connector, edid);
> + }
This is a pretty low level function. Too low level for this caching
IMO. So I think better just do it a bit higher up like other drivers.
>
> - edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter);
> - drm_connector_update_edid_property(connector, edid);
> return edid;
> }
> EXPORT_SYMBOL(drm_get_edid);
> --
> 2.31.0.291.g576ba9dcdaf-goog
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
