* [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler @ 2014-06-04 22:29 clinton.a.taylor 2014-06-05 9:26 ` Daniel Vetter 0 siblings, 1 reply; 9+ messages in thread From: clinton.a.taylor @ 2014-06-04 22:29 UTC (permalink / raw) To: Intel-gfx 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> --- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler 2014-06-04 22:29 [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler clinton.a.taylor @ 2014-06-05 9:26 ` Daniel Vetter 2014-06-06 9:41 ` Jani Nikula ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Daniel Vetter @ 2014-06-05 9:26 UTC (permalink / raw) To: clinton.a.taylor; +Cc: Intel-gfx 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. -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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler 2014-06-05 9:26 ` Daniel Vetter @ 2014-06-06 9:41 ` Jani Nikula 2014-06-06 18:13 ` Clint Taylor 2014-06-06 16:36 ` Clint Taylor [not found] ` <5390AB8D.4080908@outlook.or.com> 2 siblings, 1 reply; 9+ messages in thread From: Jani Nikula @ 2014-06-06 9:41 UTC (permalink / raw) To: Daniel Vetter, clinton.a.taylor; +Cc: Intel-gfx 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]. 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 -- Jani Nikula, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler 2014-06-06 9:41 ` Jani Nikula @ 2014-06-06 18:13 ` Clint Taylor 2014-06-06 19:54 ` Jani Nikula 0 siblings, 1 reply; 9+ messages in thread From: Clint Taylor @ 2014-06-06 18:13 UTC (permalink / raw) To: Jani Nikula, Daniel Vetter; +Cc: Intel-gfx 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler 2014-06-06 18:13 ` Clint Taylor @ 2014-06-06 19:54 ` Jani Nikula 2014-06-06 20:14 ` Dave Airlie 0 siblings, 1 reply; 9+ messages in thread From: Jani Nikula @ 2014-06-06 19:54 UTC (permalink / raw) To: Clint Taylor, Daniel Vetter; +Cc: Intel-gfx On Fri, 06 Jun 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote: > 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. 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. 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@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 >> > -- Jani Nikula, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler 2014-06-06 19:54 ` Jani Nikula @ 2014-06-06 20:14 ` Dave Airlie 2014-06-06 20:17 ` Daniel Vetter 0 siblings, 1 reply; 9+ messages in thread From: Dave Airlie @ 2014-06-06 20:14 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx@lists.freedesktop.org On 7 June 2014 05:54, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Fri, 06 Jun 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote: >> 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. > > 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@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 >>> >> > > -- > Jani Nikula, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler 2014-06-06 20:14 ` Dave Airlie @ 2014-06-06 20:17 ` Daniel Vetter 0 siblings, 0 replies; 9+ messages in thread From: Daniel Vetter @ 2014-06-06 20:17 UTC (permalink / raw) To: Dave Airlie; +Cc: intel-gfx@lists.freedesktop.org On Sat, Jun 07, 2014 at 06:14:42AM +1000, Dave Airlie wrote: > My edp panel bits are all cut-n-paste anyways. maybe we should hold the > edp on for long periods around that whole probing. We have a delayed put on the edp panel power. So keeping it just around the bits where we need it doesn't have perf downsides and imo makes it clearer why we need it - we've had some good amounts of fun in the past where functions assumed that the panel is on but not all callers ensured that. We've pushed the edp panel power handling down the callstacks due to that. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler 2014-06-05 9:26 ` Daniel Vetter 2014-06-06 9:41 ` Jani Nikula @ 2014-06-06 16:36 ` Clint Taylor [not found] ` <5390AB8D.4080908@outlook.or.com> 2 siblings, 0 replies; 9+ messages in thread From: Clint Taylor @ 2014-06-06 16:36 UTC (permalink / raw) To: Daniel Vetter; +Cc: Intel-gfx On 06/05/2014 02:26 AM, Daniel Vetter 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. > This is a Port A panel on ValleyView. > How does this exactly blow up for you? Or is this port D? > [ 219.723752] [drm:ironlake_panel_vdd_off_sync], PP_STATUS: 0x00000000 PP_CONTROL: 0xabcd0000 [ 219.823937] [drm:valleyview_irq_handler], hotplug event received, stat 0x20100000 [ 219.823964] [drm:intel_hpd_irq_handler], Received HPD interrupt on PIN 5 - cnt: 0 [ 219.824153] [drm:i915_hotplug_work_func], running encoder hotplug functions [ 219.824175] [drm:i915_hotplug_work_func], Connector eDP-1 (pin 5) received hotplug event. [ 219.824193] [drm:i915_hotplug_work_func], Connector HDMI-A-1 (pin 5) received hotplug event. [ 219.824213] [drm:intel_dp_detect], [CONNECTOR:14:eDP-1] [ 219.824229] [drm:ironlake_edp_panel_vdd_on], Turn eDP VDD on The valleyview_irq_handler() is receiving the HPD event and i915_hotplug_work_func() is calling intel_dp_detect(). > 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. We could filter if the lid is closed, but it seemed a lot safer to remove the function. Or we could move the Sink and branch OUI reads to the training function. If OUI is necessary we should probably also save the values to the context. > -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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <5390AB8D.4080908@outlook.or.com>]
* Re: [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler [not found] ` <5390AB8D.4080908@outlook.or.com> @ 2014-06-06 17:24 ` Daniel Vetter 0 siblings, 0 replies; 9+ messages in thread From: Daniel Vetter @ 2014-06-06 17:24 UTC (permalink / raw) To: Clint Taylor; +Cc: Intel-gfx On Thu, Jun 05, 2014 at 10:40:29AM -0700, Clint Taylor wrote: > On 06/05/2014 02:26 AM, Daniel Vetter 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. > > This is a Port A panel on ValleyView. Ah, that might actually be wired up hdp-wise. edp on big core (well at least on port A) definitely isn't. > >How does this exactly blow up for you? Or is this port D? > > > > [ 219.723752] [drm:ironlake_panel_vdd_off_sync], PP_STATUS: 0x00000000 > PP_CONTROL: 0xabcd0000 > [ 219.823937] [drm:valleyview_irq_handler], hotplug event received, stat > 0x20100000 > [ 219.823964] [drm:intel_hpd_irq_handler], Received HPD interrupt on PIN 5 > - cnt: 0 > [ 219.824153] [drm:i915_hotplug_work_func], running encoder hotplug > functions > [ 219.824175] [drm:i915_hotplug_work_func], Connector eDP-1 (pin 5) > received hotplug event. > [ 219.824193] [drm:i915_hotplug_work_func], Connector HDMI-A-1 (pin 5) > received hotplug event. > [ 219.824213] [drm:intel_dp_detect], [CONNECTOR:14:eDP-1] > [ 219.824229] [drm:ironlake_edp_panel_vdd_on], Turn eDP VDD on > > The valleyview_irq_handler() is receiving the HPD event and > i915_hotplug_work_func() is calling intel_dp_detect(). > > >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. > > We could filter if the lid is closed, but it seemed a lot safer to remove > the function. Is reading the OUI required as part of MST for DP? I think we should filter the OUI read if it's an eDP panel since we might wake those up. With a comment ofc why we do that special-cases. Checking for edp will also catch all-in-ones where edp is on port D. And yeah, we need OUI for a bunch of things apparently ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-06-06 20:17 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-04 22:29 [PATCH] drm/i915: revert intel_dp_probe_oui call during HPD interrupt handler clinton.a.taylor
2014-06-05 9:26 ` Daniel Vetter
2014-06-06 9:41 ` Jani Nikula
2014-06-06 18:13 ` Clint Taylor
2014-06-06 19:54 ` Jani Nikula
2014-06-06 20:14 ` Dave Airlie
2014-06-06 20:17 ` Daniel Vetter
2014-06-06 16:36 ` Clint Taylor
[not found] ` <5390AB8D.4080908@outlook.or.com>
2014-06-06 17:24 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox