[Intel-gfx] [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler

[Dave Airlie]

On 7 June 2014 05:54, Jani Nikula <jani.nikula at linux.intel.com> wrote:
> On Fri, 06 Jun 2014, Clint Taylor <clinton.a.taylor at intel.com> wrote:
>> On 06/06/2014 02:41 AM, Jani Nikula wrote:
>>> On Thu, 05 Jun 2014, Daniel Vetter <daniel at ffwll.ch> wrote:
>>>> On Wed, Jun 04, 2014 at 03:29:41PM -0700, clinton.a.taylor at intel.com wrote:
>>>>> From: Clint Taylor <clinton.a.taylor at intel.com>
>>>>>
>>>>> Remove OUI read function from the lower half interrupt handler. Upon
>>>>> closing the eDP panel lid an HPD interrupt is generated. The lower half
>>>>> handler calls intel_dp_probe_oui() as part of intel_dp_detect().
>>>>> intel_dp_probe_oui() enables eDP VDD and subsequently disables eDP VDD
>>>>> causing another HPD interrupt. This cycle repeats every 3.6 seconds with
>>>>> VDD asserted for 3.5 of those seconds until the lid is opened again.
>>>>>
>>>>> Revert of 0d198328538276c4459ef5de081e68ae60e6c4c2
>>>>> Revert of 351cfc34db8decb0c5cc1aac7cf1780a0e45c8b1
>>>>>
>>>>> Signed-off-by: Clint Taylor <clinton.a.taylor at intel.com>
>>>>
>>>> Hm, this is funky ... we currently don't handle port A hotplug events, and
>>>> we filter hotplug events properly.
>>>>
>>>> How does this exactly blow up for you? Or is this port D?
>>>>
>>>> We might want to have some filtering here checking whether the edp panel
>>>> is on or off. Also the delayed work is _way_ too long.
>>>
>>> Dave just posted a patch that depends on the OUI [1].
>>
>> I didn't see any changes to intel_dp_probe_oui() in Dave's patches. The
>> function is still a NOP as it doesn't save the OUI reads.
>
> See the referenced "drm/i915/dp: add support for load detect Apple
> DP->VGA dongles" patch, it adds more logic to the end of
> intel_dp_probe_oui(). I'm not talking about the MST series.

My patch is actually broken on non-MST due to the OUI being read too late.

We only read the OUI if the intel_dp_detect passes as connected, however if
we have a dongle plugged in without any EDID on it we won't ever get that far.

MST branch fixes that anyways by probing the OUIs if we get the DPCD
without error.

My edp panel bits are all cut-n-paste anyways. maybe we should hold the
edp on for long periods around that whole probing.

Dave.


>
> BR,
> Jani.
>
>
>>
>> I'm also concerned about his intel_dp_probe_mst() as it also turns on
>> and off eDP VDD to read the DPCD registers. In the intel_dp_detect()
>> function of his patches he is now calling intel_dp_probe_oui() then
>> immediately calls intel_dp_probe_mst() resulting in the panel being
>> turned on and off twice.
>>
>> Clint
>>
>>>
>>> BR,
>>> Jani.
>>>
>>>
>>> [1] http://mid.gmane.org/1402023404-22324-1-git-send-email-airlied@gmail.com
>>>
>>>
>>>
>>>
>>>
>>>> -Daniel
>>>>
>>>>> ---
>>>>>   drivers/gpu/drm/i915/intel_dp.c |   23 -----------------------
>>>>>   1 file changed, 23 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>>>> index 2a00cb8..246d2c1 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>>> @@ -2867,27 +2867,6 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>>>>>    return true;
>>>>>   }
>>>>>
>>>>> -static void
>>>>> -intel_dp_probe_oui(struct intel_dp *intel_dp)
>>>>> -{
>>>>> -  u8 buf[3];
>>>>> -
>>>>> -  if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT))
>>>>> -          return;
>>>>> -
>>>>> -  intel_edp_panel_vdd_on(intel_dp);
>>>>> -
>>>>> -  if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3)
>>>>> -          DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n",
>>>>> -                        buf[0], buf[1], buf[2]);
>>>>> -
>>>>> -  if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3)
>>>>> -          DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n",
>>>>> -                        buf[0], buf[1], buf[2]);
>>>>> -
>>>>> -  edp_panel_vdd_off(intel_dp, false);
>>>>> -}
>>>>> -
>>>>>   int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>>>>>   {
>>>>>    struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>>>> @@ -3178,8 +3157,6 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>>>>>    if (status != connector_status_connected)
>>>>>            goto out;
>>>>>
>>>>> -  intel_dp_probe_oui(intel_dp);
>>>>> -
>>>>>    if (intel_dp->force_audio != HDMI_AUDIO_AUTO) {
>>>>>            intel_dp->has_audio = (intel_dp->force_audio == HDMI_AUDIO_ON);
>>>>>    } else {
>>>>> --
>>>>> 1.7.9.5
>>>>>
>>>>> _______________________________________________
>>>>> 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
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>>
>
> --
> Jani Nikula, Intel Open Source Technology Center