Re: [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler

[Clint Taylor <clinton.a.taylor@intel.com>]

On 06/06/2014 02:41 AM, Jani Nikula wrote:
> On Thu, 05 Jun 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Wed, Jun 04, 2014 at 03:29:41PM -0700, clinton.a.taylor@intel.com wrote:
>>> From: Clint Taylor <clinton.a.taylor@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@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.

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@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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>