[Intel-gfx] [PATCH 2/6] drm/i915: Add HDMI probe function
Rodrigo Vivi
rodrigo.vivi at gmail.com
Wed Sep 9 11:55:56 PDT 2015
I liked the approach and agree with Daniel, so with his suggestions feel
free to use:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
On Fri, Sep 4, 2015 at 7:46 AM Daniel Vetter <daniel at ffwll.ch> wrote:
> On Fri, Sep 04, 2015 at 06:56:12PM +0530, Sonika Jindal wrote:
> > From: Shashank Sharma <shashank.sharma at intel.com>
> >
> > This patch adds a separate probe function for HDMI
> > EDID read over DDC channel. This function has been
> > registered as a .hot_plug handler for HDMI encoder.
> >
> > The current implementation of hdmi_detect()
> > function re-sets the cached HDMI edid (in connector->detect_edid) in
> > every detect call.This function gets called many times, sometimes
> > directly from userspace probes, forcing drivers to read EDID every
> > detect function call.This causes several problems like:
> >
> > 1. Race conditions in multiple hot_plug / unplug cases, between
> > interrupts bottom halves and userspace detections.
> > 2. Many Un-necessary EDID reads for single hotplug/unplug
> > 3. HDMI complaince failures which expects only one EDID read per hotplug
> >
> > This function will be serving the purpose of really reading the EDID
> > by really probing the DDC channel, and updating the cached EDID.
> >
> > The plan is to:
> > 1. i915 IRQ handler bottom half function already calls
> > intel_encoder->hotplug() function. Adding This probe function which
> > will read the EDID only in case of a hotplug / unplug.
> > 2. During init_connector his probe will be called to read the edid
> > 3. Reuse the cached EDID in hdmi_detect() function.
> >
> > The "< gen7" check is there because this was tested only for >=gen7
> > platforms. For older platforms the hotplug/reading edid path remains
> same.
> >
> > v2: Calling set_edid instead of hdmi_probe during init.
> > Also, for platforms having DDI, intel_encoder for DP and HDMI is same
> > (taken from intel_dig_port), so for DP also, hot_plug function gets
> called
> > which is not intended here. So, check for HDMI in intel_hdmi_probe
> > Rely on HPD for updating edid only for platforms gen > 8 and also for
> VLV.
> >
> > v3: Dropping the gen < 8 || !VLV check. Now all platforms should rely on
> > hotplug or init for updating the edid.(Daniel)
> > Also, calling hdmi_probe in init instead of set_edid
> >
> > Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
> > Signed-off-by: Sonika Jindal <sonika.jindal at intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_hdmi.c | 46
> ++++++++++++++++++++++++++++++++-----
> > 1 file changed, 40 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 5bdb386..1eda71a 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1368,23 +1368,53 @@ intel_hdmi_set_edid(struct drm_connector
> *connector)
> > return connected;
> > }
> >
> > +void intel_hdmi_probe(struct intel_encoder *intel_encoder)
>
> Please call it _hot_plug if it's for the _hot_plug path.
>
> > +{
> > + struct intel_hdmi *intel_hdmi =
> > + enc_to_intel_hdmi(&intel_encoder->base);
> > + struct intel_connector *intel_connector =
> > + intel_hdmi->attached_connector;
> > +
> > + /*
> > + * We are here, means there is a hotplug or a force
> > + * detection. Clear the cached EDID and probe the
> > + * DDC bus to check the current status of HDMI.
> > + */
> > + intel_hdmi_unset_edid(&intel_connector->base);
> > + if (intel_hdmi_set_edid(&intel_connector->base))
> > + DRM_DEBUG_DRIVER("DDC probe: got EDID\n");
> > + else
> > + DRM_DEBUG_DRIVER("DDC probe: no EDID\n");
> > +}
> > +
> > 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 of 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, rely on hotplugs and init to read edid.
> > + * Check connector status based on availability of cached EDID.
> > + */
> >
> > - 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;
> > }
> > @@ -2104,11 +2134,15 @@ void intel_hdmi_init_connector(struct
> intel_digital_port *intel_dig_port,
> > intel_connector->unregister = intel_connector_unregister;
> >
> > intel_hdmi_add_properties(intel_hdmi, connector);
> > + intel_encoder->hot_plug = intel_hdmi_probe;
> >
> > intel_connector_attach_encoder(intel_connector, intel_encoder);
> > drm_connector_register(connector);
> > intel_hdmi->attached_connector = intel_connector;
> >
> > + /* Set edid during init */
> > + intel_hdmi_probe(intel_encoder);
>
> Separate patch, but we really don't want this, initial probe should be
> done async to avoid stalling the driver.
> -Daniel
>
> > +
> > /* For G4X desktop chip, PEG_BAND_GAP_DATA 3:0 must first be
> written
> > * 0xd. Failure to do so will result in spurious interrupts being
> > * generated on the port when a cable is not attached.
> > --
> > 1.7.10.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20150909/ca16b474/attachment-0001.html>
More information about the Intel-gfx
mailing list