[Intel-gfx] [PATCH 1/2] drm/i915: Optimize HDMI EDID reads
Daniel Vetter
daniel at ffwll.ch
Wed Aug 13 14:14:56 CEST 2014
On Tue, Aug 12, 2014 at 06:08:20PM +0530, shashank.sharma at intel.com wrote:
> From: Shashank Sharma <shashank.sharma at intel.com>
>
> The current hdmi_detect() function is getting called from
> many places, few of these are:
> 1. HDMI hot plug interrupt bottom half
> 2. get_resources() IOCTL family
> 3. drm_helper_probe_single_connector_modes() family
> 4. output_poll_execute()
> 5. status_show() etc...
>
> Every time this function is called, it goes and reads HDMI EDID over
> DDC channel. Ideally, reading EDID is only required when there is a
> real hot plug, and then for all IOCTL and userspace detect functions
> can be answered using this same EDID.
>
> The current patch adds EDID caching for a finite duration (1 minute)
> This is how it works:
> 1. With in this caching duration, when usespace get_resource and other
> modeset_detect calls get called, we can use the cached EDID.
> 2. Even the get_mode function can use the cached EDID.
> 3. A delayed work will clear the cached EDID after the timeout.
> 4. If there is a real HDMI hotplug, within the caching duration, the
> cached EDID is updates, and a new delayed work is scheduled.
>
> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
This has a bunch of changes compared to what I've proposed, and so will
not actually work. Also, keying off the source platform (with the gen6
checks) is useless if we're dealing with random brokeness in existing hdmi
sinks here.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_drv.h | 4 ++
> drivers/gpu/drm/i915/intel_hdmi.c | 92 ++++++++++++++++++++++++++++++++++++---
> 2 files changed, 90 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 28d185d..185a45a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -110,6 +110,8 @@
> #define INTEL_DSI_VIDEO_MODE 0
> #define INTEL_DSI_COMMAND_MODE 1
>
> +#define INTEL_HDMI_EDID_CACHING_SEC 60
> +
> struct intel_framebuffer {
> struct drm_framebuffer base;
> struct drm_i915_gem_object *obj;
> @@ -507,6 +509,8 @@ struct intel_hdmi {
> enum hdmi_force_audio force_audio;
> bool rgb_quant_range_selectable;
> enum hdmi_picture_aspect aspect_ratio;
> + struct edid *edid;
> + struct delayed_work edid_work;
> void (*write_infoframe)(struct drm_encoder *encoder,
> enum hdmi_infoframe_type type,
> const void *frame, ssize_t len);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 5f47d35..8dc3970 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -962,6 +962,22 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> return true;
> }
>
> +/* Work function to invalidate EDID caching */
> +static void intel_hdmi_invalidate_edid(struct work_struct *work)
> +{
> + struct intel_hdmi *intel_hdmi = container_of(to_delayed_work(work),
> + struct intel_hdmi, edid_work);
> + struct drm_device *dev = intel_hdmi_to_dev(intel_hdmi);
> + struct drm_mode_config *mode_config = &dev->mode_config;
> +
> + mutex_lock(&mode_config->mutex);
> + /* Checkpatch says kfree is NULL protected */
> + kfree(intel_hdmi->edid);
> + intel_hdmi->edid = NULL;
> + mutex_unlock(&mode_config->mutex);
> + DRM_DEBUG_DRIVER("cleaned up cached EDID\n");
> +}
> +
> static enum drm_connector_status
> intel_hdmi_detect(struct drm_connector *connector, bool force)
> {
> @@ -978,15 +994,58 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> connector->base.id, connector->name);
>
> + /*
> + * hdmi_detect() gets called from both get_resource()
> + * and HDMI hpd bottom half work function.
> + * Its not required to read EDID for every detect call until it's is
> + * from a hot plug. Lets cache the EDID as soon as we get
> + * a HPD, and then try to re-use this for all the non hpd detact calls
> + * coming with in a finite duration.
> + */
> + if (INTEL_INFO(dev)->gen < 6)
> + /* Do not break old platforms */
> + goto skip_optimization;
> +
> + /* If call is from HPD, do check real status by reading EDID */
> + if (!force)
> + goto skip_optimization;
> +
> + /* This is a non-hpd call, lets see if we can optimize this */
> + if (intel_hdmi->edid) {
> + /*
> + * So this is a non-hpd call, within the duration when
> + * EDID caching is valid. So previous status (valid)
> + * of connector is re-usable.
> + */
> + if (connector->status != connector_status_unknown) {
> + DRM_DEBUG_DRIVER("Reporting force status\n");
> + return connector->status;
> + }
> + }
> +
> +skip_optimization:
> power_domain = intel_display_port_power_domain(intel_encoder);
> intel_display_power_get(dev_priv, power_domain);
>
> intel_hdmi->has_hdmi_sink = false;
> intel_hdmi->has_audio = false;
> intel_hdmi->rgb_quant_range_selectable = false;
> +
> + /*
> + * You are well deserving, dear code, as you have survived
> + * all the optimizations. Now go and enjoy reading EDID
> + */
> edid = drm_get_edid(connector,
> - intel_gmbus_get_adapter(dev_priv,
> - intel_hdmi->ddc_bus));
> + intel_gmbus_get_adapter(dev_priv,
> + intel_hdmi->ddc_bus));
> + /*
> + * Now when we have read new EDID, update cached EDID with
> + * latest (both NULL or non NULL). Cancel the delayed work
> + * which cleans up the cached EDID. Re-schedule if required.
> + */
> + kfree(intel_hdmi->edid);
> + intel_hdmi->edid = edid;
> + cancel_delayed_work_sync(&intel_hdmi->edid_work);
>
> if (edid) {
> if (edid->input & DRM_EDID_INPUT_DIGITAL) {
> @@ -997,8 +1056,17 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
> intel_hdmi->rgb_quant_range_selectable =
> drm_rgb_quant_range_selectable(edid);
> + /*
> + * Allow re-use of cached EDID for 60 sec, as
> + * userspace modeset should happen within this
> + * duration, and multiple detect calls will be
> + * handled using cached EDID.
> + */
> + schedule_delayed_work(&intel_hdmi->edid_work,
> + msecs_to_jiffies(
> + INTEL_HDMI_EDID_CACHING_SEC
> + * 1000));
> }
> - kfree(edid);
> }
>
> if (status == connector_status_connected) {
> @@ -1027,13 +1095,22 @@ static int intel_hdmi_get_modes(struct drm_connector *connector)
>
> power_domain = intel_display_port_power_domain(intel_encoder);
> intel_display_power_get(dev_priv, power_domain);
> -
> - ret = intel_ddc_get_modes(connector,
> + /*
> + * GEN6 and + have software support for EDID caching, so
> + * use cached_edid from detect call, if available.
> + */
> + if (intel_hdmi->edid && (INTEL_INFO(connector->dev)->gen >= 6)) {
> + ret = intel_connector_update_modes(connector,
> + intel_hdmi->edid);
> + DRM_DEBUG_DRIVER("Using cached EDID, got %d modes\n", ret);
> + } else {
> + ret = intel_ddc_get_modes(connector,
> intel_gmbus_get_adapter(dev_priv,
> intel_hdmi->ddc_bus));
> + DRM_DEBUG_DRIVER("Read EDID, got %d modes\n", ret);
> + }
>
> intel_display_power_put(dev_priv, power_domain);
> -
> return ret;
> }
>
> @@ -1661,5 +1738,8 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
> intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
> intel_dig_port->dp.output_reg = 0;
>
> + /* Work function to invalidate cached EDID after timeout */
> + INIT_DELAYED_WORK(&(intel_dig_port->hdmi.edid_work),
> + intel_hdmi_invalidate_edid);
> intel_hdmi_init_connector(intel_dig_port, intel_connector);
> }
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list