On Tue, Jun 30, 2015 at 11:13:58AM +0530, Sonika Jindal wrote:
> From: Shashank Sharma <contactshashanksharma@xxxxxxxxx>
>
> This patch makes sure that the HDMI detect function
> reads EDID only when its forced to do it. All the other
> times, it uses the connector->detect_edid which was cached
> during hotplug handling in the hdmi_probe() function. As the
> probe function gets called before detect in the interrupt handler
> and handles the EDID cacheing part, its absolutely safe to assume
> that presence of EDID reflects monitor connected and viceversa.
>
> This will save us from many race conditions between hotplug/unplug
> detect call handler threads and userspace calls for the same.
> The previous patch in this patch series explains this in detail.
>
> Signed-off-by: Shashank Sharma <contactshashanksharma@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_hdmi.c | 26 ++++++++++++++++++++------
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 064ddd8..1fb6919 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1362,19 +1362,33 @@ static enum drm_connector_status
> intel_hdmi_detect(struct drm_connector *connector, bool force)
> {
> enum drm_connector_status status;
> + struct intel_connector *intel_connector =
> + to_intel_connector(connector);
>
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> connector->base.id, connector->name);
> + /*
> + * There are many userspace calls which probe EDID from
> + * detect path. In case on multiple hotplug/unplug, these
> + * can cause race conditions while probing EDID. Also its
> + * waste of CPU cycles to read the EDID again and again
> + * unless there is a real hotplug.
> + * So until we are forced, check connector status
> + * based on availability of cached EDID. This will avoid many of
> + * these race conditions and timing problems.
> + */
> + if (force)
Userspace always sets force. Are you sure this actually improves anything?
Also the goal should be to keep things cache for a few calls from
userspace (since often it pokes a few times in a row unfortuantely), for
which we need a proper timeout to clear the edid again.
-Daniel
> + intel_hdmi_probe(intel_connector->encoder);
>
> - intel_hdmi_unset_edid(connector);
> -
> - if (intel_hdmi_set_edid(connector)) {
> + if (intel_connector->detect_edid) {
> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> -
> - hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> status = connector_status_connected;
> - } else
> + hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> + DRM_DEBUG_DRIVER("hdmi status = connected\n");
> + } else {
> status = connector_status_disconnected;
> + DRM_DEBUG_DRIVER("hdmi status = disconnected\n");
> + }
>
> return status;
> }
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
