From mboxrd@z Thu Jan 1 00:00:00 1970 From: Clint Taylor Subject: Re: [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler Date: Fri, 06 Jun 2014 11:13:17 -0700 Message-ID: <539204BD.3060308@intel.com> References: <1401920981-3137-1-git-send-email-clinton.a.taylor@intel.com> <20140605092629.GV7416@phenom.ffwll.local> <871tv2e5ot.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 4F9E36EAAD for ; Fri, 6 Jun 2014 11:13:55 -0700 (PDT) In-Reply-To: <871tv2e5ot.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Jani Nikula , Daniel Vetter Cc: Intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 06/06/2014 02:41 AM, Jani Nikula wrote: > On Thu, 05 Jun 2014, Daniel Vetter wrote: >> On Wed, Jun 04, 2014 at 03:29:41PM -0700, clinton.a.taylor@intel.com wrote: >>> From: Clint Taylor >>> >>> 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 >> >> 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 >