[Intel-gfx] [PATCH] drm/i915: Check live status before reading edid
Jindal, Sonika
sonika.jindal at intel.com
Mon Sep 14 02:14:01 PDT 2015
On 9/14/2015 2:12 PM, Daniel Vetter wrote:
> On Fri, Sep 11, 2015 at 05:56:47PM +0000, Rodrigo Vivi wrote:
>> Thanks
>>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>>
>>
>> On Fri, Sep 11, 2015 at 4:38 AM Sonika Jindal <sonika.jindal at intel.com>
>> wrote:
>>
>>> The Bspec is very clear that Live status must be checked about before
>>> trying to read EDID over DDC channel. This patch makes sure that HDMI
>>> EDID is read only when live status is up.
>>>
>>> The live status doesn't seem to perform very consistent across various
>>> platforms when tested with different monitors. The reason behind that is
>>> some monitors are late to provide right voltage to set live_status up.
>>> So, after getting the interrupt, for a small duration, live status reg
>>> fluctuates, and then settles down showing the correct staus.
>>>
>>> This is explained here in, in a rough way:
>>> HPD line ________________
>>> |\ T1 = Monitor Hotplug causing IRQ
>>> | \______________________________________
>>> | |
>>> | |
>>> | | T2 = Live status is stable
>>> | | _____________________________________
>>> | | /|
>>> Live status _____________|_|/ |
>>> | | |
>>> | | |
>>> | | |
>>> T0 T1 T2
>>>
>>> (Between T1 and T2 Live status fluctuates or can be even low, depending on
>>> the monitor)
>>>
>>> After several experiments, we have concluded that a max delay
>>> of 30ms is enough to allow the live status to settle down with
>>> most of the monitors. This total delay of 30ms has been split into
>>> a resolution of 3 retries of 10ms each, for the better cases.
>>>
>>> This delay is kept at 30ms, keeping in consideration that, HDCP compliance
>>> expect the HPD handler to respond a plug out in 100ms, by disabling port.
>>>
>>> v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
>>> to check digital port status. Adding a separate function to get bxt live
>>> status (Daniel)
>>> v3: Using intel_encoder->hpd_pin to check the live status (Siva)
>>> Moving the live status read to intel_hdmi_probe and passing parameter
>>> to read/not to read the edid. (me)
>>> v4:
>>> * Added live status check for all platforms using
>>> intel_digital_port_connected.
>>> * Rebased on top of Jani's DP cleanup series
>>> * Some monitors take time in setting the live status. So retry for few
>>> times if this is a connect HPD
>>> v5: Removed extra "drm/i915" from commit message. Adding Shashank's sob
>>> which was missed.
>>> v6: Drop the (!detect_edid && !live_status check) check because for DDI
>>> ports which are enumerated as hdmi as well as DP, we don't have a
>>> mechanism to differentiate between DP and hdmi inside the encoder's
>>> hot_plug. This leads to call to the hdmi's hot_plug hook for DP as well
>>> as hdmi which leads to issues during unplug because of the above check.
>>> v7: Make intel_digital_port_connected global in this patch, some
>>> reformatting of while loop, adding a print when live status is not
>>> up. (Rodrigo)
>>>
>>> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
>>> Signed-off-by: Sonika Jindal <sonika.jindal at intel.com>
>
> Since this is tricky stuff and other patches in this series are blocked
> until we have a clearer picture can you please rebase this to be the first
> patch on top of -nightly so that I can pull it in directly?
>
> I tried to do that but proved a bit too messy.
> -Daniel
>
Hmm, since rebasing this will require these changes (check for live
status) to go in detect and then later when we reach a conclusion on
hot_plug hook, we would need to move it to that function.
I think lets wait for the conclusion on the placement of that function.
What do you suggest?
Regards,
Sonika
>>> ---
>>> drivers/gpu/drm/i915/intel_dp.c | 2 +-
>>> drivers/gpu/drm/i915/intel_drv.h | 2 ++
>>> drivers/gpu/drm/i915/intel_hdmi.c | 26 +++++++++++++++++++-------
>>> 3 files changed, 22 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>>> b/drivers/gpu/drm/i915/intel_dp.c
>>> index bf17030..fedf6d1 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -4695,7 +4695,7 @@ static bool bxt_digital_port_connected(struct
>>> drm_i915_private *dev_priv,
>>> *
>>> * Return %true if @port is connected, %false otherwise.
>>> */
>>> -static bool intel_digital_port_connected(struct drm_i915_private
>>> *dev_priv,
>>> +bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
>>> struct intel_digital_port *port)
>>> {
>>> if (HAS_PCH_IBX(dev_priv))
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>>> b/drivers/gpu/drm/i915/intel_drv.h
>>> index b6c2c20..ac6d748 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -1210,6 +1210,8 @@ void intel_edp_drrs_disable(struct intel_dp
>>> *intel_dp);
>>> void intel_edp_drrs_invalidate(struct drm_device *dev,
>>> unsigned frontbuffer_bits);
>>> void intel_edp_drrs_flush(struct drm_device *dev, unsigned
>>> frontbuffer_bits);
>>> +bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
>>> + struct intel_digital_port *port);
>> g>
>>> /* intel_dp_mst.c */
>>> int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port,
>>> int conn_id);
>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
>>> b/drivers/gpu/drm/i915/intel_hdmi.c
>>> index 1eda71a..d366ca5 100644
>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>> @@ -1329,22 +1329,23 @@ intel_hdmi_unset_edid(struct drm_connector
>>> *connector)
>>> }
>>>
>>> static bool
>>> -intel_hdmi_set_edid(struct drm_connector *connector)
>>> +intel_hdmi_set_edid(struct drm_connector *connector, bool force)
>>> {
>>> struct drm_i915_private *dev_priv = to_i915(connector->dev);
>>> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
>>> struct intel_encoder *intel_encoder =
>>> &hdmi_to_dig_port(intel_hdmi)->base;
>>> enum intel_display_power_domain power_domain;
>>> - struct edid *edid;
>>> + struct edid *edid = NULL;
>>> bool connected = false;
>>>
>>> power_domain = intel_display_port_power_domain(intel_encoder);
>>> intel_display_power_get(dev_priv, power_domain);
>>>
>>> - edid = drm_get_edid(connector,
>>> - intel_gmbus_get_adapter(dev_priv,
>>> - intel_hdmi->ddc_bus));
>>> + if (force)
>>> + edid = drm_get_edid(connector,
>>> + intel_gmbus_get_adapter(dev_priv,
>>> + intel_hdmi->ddc_bus));
>>>
>>> intel_display_power_put(dev_priv, power_domain);
>>>
>>> @@ -1374,14 +1375,25 @@ void intel_hdmi_probe(struct intel_encoder
>>> *intel_encoder)
>>> enc_to_intel_hdmi(&intel_encoder->base);
>>> struct intel_connector *intel_connector =
>>> intel_hdmi->attached_connector;
>>> + struct drm_i915_private *dev_priv =
>>> to_i915(intel_encoder->base.dev);
>>> + bool live_status = false;
>>> + unsigned int retry = 3;
>>> +
>>> + while (!live_status && --retry) {
>>> + live_status = intel_digital_port_connected(dev_priv,
>>> + hdmi_to_dig_port(intel_hdmi));
>>> + mdelay(10);
>>> + }
>>>
>>> + if (!live_status)
>>> + DRM_DEBUG_KMS("Live status not up!");
>>> /*
>>> * 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))
>>> + if (intel_hdmi_set_edid(&intel_connector->base, live_status))
>>> DRM_DEBUG_DRIVER("DDC probe: got EDID\n");
>>> else
>>> DRM_DEBUG_DRIVER("DDC probe: no EDID\n");
>>> @@ -1432,7 +1444,7 @@ intel_hdmi_force(struct drm_connector *connector)
>>> if (connector->status != connector_status_connected)
>>> return;
>>>
>>> - intel_hdmi_set_edid(connector);
>>> + intel_hdmi_set_edid(connector, true);
>>> hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
>>> }
>>>
>>> --
>>> 1.7.10.4
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
More information about the Intel-gfx
mailing list