* [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect @ 2016-12-07 0:43 Manasi Navare 2016-12-07 1:15 ` ✓ Fi.CI.BAT: success for " Patchwork ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Manasi Navare @ 2016-12-07 0:43 UTC (permalink / raw) To: intel-gfx; +Cc: Daniel Vetter The detect_done flag was introduced in the commit 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c in order to avoid multiple detects on hotplug where intel_dp_long_pulse() was called from HPD handler as well as in intel_dp_detect. So this detect_done flag was required to make sure intel_dp_detect() did not call long pulse handler again if it was already been called from HPD handler. However commit 1015811609c0328b5ed670d07748591b837e74eb differs the long hpd handling entirely until the hotplug work runs to avoid the double long hpd handling the "detect_done" flag is trying to prevent. So now we do not need to reset the detect_done flag to false in intel_dp_detect. It will be reset in the intel_dp_hpd_pulse so that intel_dp_detect does a full detect. However if the .detect gets called during mode enumeration then we do not need to do a full detect. This patch avoids the WARNS_ONS during connected boot case when it calls intel_dp_check_link_status() due to multiple detects and also avoids DP compliance failures. It avoids doing a full detect every single time on .detect(). Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Jani Nikula <jani.nikula@linux.intel.com> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index db75bb9..9c9277e 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4470,8 +4470,6 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, if (!intel_dp->detect_done) status = intel_dp_long_pulse(intel_dp->attached_connector); - intel_dp->detect_done = false; - return status; } -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 12+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Do not reset detect_done flag in intel_dp_detect 2016-12-07 0:43 [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect Manasi Navare @ 2016-12-07 1:15 ` Patchwork 2016-12-07 8:52 ` [PATCH] " Jani Nikula 2016-12-07 14:05 ` Ville Syrjälä 2 siblings, 0 replies; 12+ messages in thread From: Patchwork @ 2016-12-07 1:15 UTC (permalink / raw) To: Navare, Manasi D; +Cc: intel-gfx == Series Details == Series: drm/i915: Do not reset detect_done flag in intel_dp_detect URL : https://patchwork.freedesktop.org/series/16456/ State : success == Summary == Series 16456v1 drm/i915: Do not reset detect_done flag in intel_dp_detect https://patchwork.freedesktop.org/api/1.0/series/16456/revisions/1/mbox/ fi-bdw-5557u total:247 pass:218 dwarn:0 dfail:0 fail:0 skip:29 fi-bsw-n3050 total:247 pass:194 dwarn:0 dfail:0 fail:0 skip:53 fi-bxt-t5700 total:247 pass:205 dwarn:0 dfail:0 fail:0 skip:42 fi-byt-j1900 total:247 pass:205 dwarn:0 dfail:0 fail:0 skip:42 fi-byt-n2820 total:247 pass:201 dwarn:0 dfail:0 fail:0 skip:46 fi-hsw-4770 total:247 pass:213 dwarn:0 dfail:0 fail:0 skip:34 fi-hsw-4770r total:247 pass:213 dwarn:0 dfail:0 fail:0 skip:34 fi-ilk-650 total:247 pass:180 dwarn:0 dfail:0 fail:0 skip:67 fi-ivb-3520m total:247 pass:212 dwarn:0 dfail:0 fail:0 skip:35 fi-ivb-3770 total:247 pass:211 dwarn:0 dfail:0 fail:0 skip:36 fi-kbl-7500u total:247 pass:211 dwarn:0 dfail:0 fail:0 skip:36 fi-skl-6260u total:247 pass:219 dwarn:0 dfail:0 fail:0 skip:28 fi-skl-6700hq total:247 pass:213 dwarn:0 dfail:0 fail:0 skip:34 fi-skl-6700k total:247 pass:209 dwarn:3 dfail:0 fail:0 skip:35 fi-skl-6770hq total:247 pass:219 dwarn:0 dfail:0 fail:0 skip:28 fi-snb-2520m total:247 pass:201 dwarn:0 dfail:0 fail:0 skip:46 fi-snb-2600 total:247 pass:200 dwarn:0 dfail:0 fail:0 skip:47 08aa3ac5c18159c9105252903713cb21c27e9442 drm-tip: 2016y-12m-06d-18h-32m-52s UTC integration manifest 9e30950 drm/i915: Do not reset detect_done flag in intel_dp_detect == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3210/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect 2016-12-07 0:43 [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect Manasi Navare 2016-12-07 1:15 ` ✓ Fi.CI.BAT: success for " Patchwork @ 2016-12-07 8:52 ` Jani Nikula 2016-12-07 14:05 ` Ville Syrjälä 2 siblings, 0 replies; 12+ messages in thread From: Jani Nikula @ 2016-12-07 8:52 UTC (permalink / raw) To: Manasi Navare, intel-gfx; +Cc: Daniel Vetter On Wed, 07 Dec 2016, Manasi Navare <manasi.d.navare@intel.com> wrote: > The detect_done flag was introduced in the commit > 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c in order to avoid The preferred format to cite commits is: 7d23e3c37bb3 ("drm/i915: Cleaning up intel_dp_hpd_pulse") or commit 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c Author: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com> Date: Wed Mar 30 18:05:23 2016 +0530 drm/i915: Cleaning up intel_dp_hpd_pulse > multiple detects on hotplug where intel_dp_long_pulse() > was called from HPD handler as well as in intel_dp_detect. > So this detect_done flag was required to make sure intel_dp_detect() > did not call long pulse handler again if it was already been called > from HPD handler. However commit 1015811609c0328b5ed670d07748591b837e74eb 1015811609c0 ("drm/i915: Move long hpd handling into the hotplug work") > differs the long hpd handling entirely until the hotplug work runs to > avoid the double long hpd handling the "detect_done" flag is trying > to prevent. > > So now we do not need to reset the detect_done flag to false in > intel_dp_detect. It will be reset in the intel_dp_hpd_pulse so > that intel_dp_detect does a full detect. However if the .detect > gets called during mode enumeration then we do not need to do a > full detect. This patch avoids the WARNS_ONS during connected boot Please include such a backtrace in the commit message; it makes matching bugs and fixes so much easier. IIUC the warnings were introduced by 1015811609c0 ("drm/i915: Move long hpd handling into the hotplug work"), and that's cc: stable, so this one should be too, along with Fixes: 1015811609c0 ("drm/i915: Move long hpd handling into the hotplug work") Cc: stable@vger.kernel.org Of course, someone(tm) will still need to make sure this is the right fix... BR, Jani. > case when it calls intel_dp_check_link_status() due to multiple > detects and also avoids DP compliance failures. It avoids doing > a full detect every single time on .detect(). > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index db75bb9..9c9277e 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4470,8 +4470,6 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > if (!intel_dp->detect_done) > status = intel_dp_long_pulse(intel_dp->attached_connector); > > - intel_dp->detect_done = false; > - > return status; > } -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect 2016-12-07 0:43 [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect Manasi Navare 2016-12-07 1:15 ` ✓ Fi.CI.BAT: success for " Patchwork 2016-12-07 8:52 ` [PATCH] " Jani Nikula @ 2016-12-07 14:05 ` Ville Syrjälä 2016-12-07 20:46 ` Manasi Navare 2016-12-08 3:51 ` Manasi Navare 2 siblings, 2 replies; 12+ messages in thread From: Ville Syrjälä @ 2016-12-07 14:05 UTC (permalink / raw) To: Manasi Navare; +Cc: Daniel Vetter, intel-gfx On Tue, Dec 06, 2016 at 04:43:51PM -0800, Manasi Navare wrote: > The detect_done flag was introduced in the commit > 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c in order to avoid > multiple detects on hotplug where intel_dp_long_pulse() > was called from HPD handler as well as in intel_dp_detect. > So this detect_done flag was required to make sure intel_dp_detect() > did not call long pulse handler again if it was already been called > from HPD handler. However commit 1015811609c0328b5ed670d07748591b837e74eb > differs the long hpd handling entirely until the hotplug work runs to > avoid the double long hpd handling the "detect_done" flag is trying > to prevent. That sentence doesn't parse here. Anyways, the flag indeed is now a nop and your patch is pretty much the same what I did here: https://patchwork.freedesktop.org/patch/101476/ > > So now we do not need to reset the detect_done flag to false in > intel_dp_detect. It will be reset in the intel_dp_hpd_pulse so > that intel_dp_detect does a full detect. However if the .detect > gets called during mode enumeration then we do not need to do a > full detect. This patch avoids the WARNS_ONS during connected boot > case when it calls intel_dp_check_link_status() due to multiple > detects How exactly does it do that? Also we shouldn't sweep that under the rug anyway. Instead someone should actually fix the problem that causes the WARN. > and also avoids DP compliance failures. It avoids doing > a full detect every single time on .detect(). > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index db75bb9..9c9277e 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4470,8 +4470,6 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > if (!intel_dp->detect_done) > status = intel_dp_long_pulse(intel_dp->attached_connector); > > - intel_dp->detect_done = false; > - > return status; > } > > -- > 1.9.1 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect 2016-12-07 14:05 ` Ville Syrjälä @ 2016-12-07 20:46 ` Manasi Navare 2016-12-08 3:51 ` Manasi Navare 1 sibling, 0 replies; 12+ messages in thread From: Manasi Navare @ 2016-12-07 20:46 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Daniel Vetter, intel-gfx On Wed, Dec 07, 2016 at 04:05:56PM +0200, Ville Syrjälä wrote: > On Tue, Dec 06, 2016 at 04:43:51PM -0800, Manasi Navare wrote: > > The detect_done flag was introduced in the commit > > 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c in order to avoid > > multiple detects on hotplug where intel_dp_long_pulse() > > was called from HPD handler as well as in intel_dp_detect. > > So this detect_done flag was required to make sure intel_dp_detect() > > did not call long pulse handler again if it was already been called > > from HPD handler. However commit 1015811609c0328b5ed670d07748591b837e74eb > > differs the long hpd handling entirely until the hotplug work runs to > > avoid the double long hpd handling the "detect_done" flag is trying > > to prevent. > > That sentence doesn't parse here. Anyways, the flag indeed is now a nop > and your patch is pretty much the same what I did here: > https://patchwork.freedesktop.org/patch/101476/ > I looked at this patch and yes it is the same thing except that it keeps the name of the flag unchanged plus previous patch is too old and wont apply cleanly unless you rebase it. I will add the reseting of this flag in the resume case. Thoughts? > > > > So now we do not need to reset the detect_done flag to false in > > intel_dp_detect. It will be reset in the intel_dp_hpd_pulse so > > that intel_dp_detect does a full detect. However if the .detect > > gets called during mode enumeration then we do not need to do a > > full detect. This patch avoids the WARNS_ONS during connected boot > > case when it calls intel_dp_check_link_status() due to multiple > > detects > > How exactly does it do that? Also we shouldn't sweep that under the rug > anyway. Instead someone should actually fix the problem that causes the > WARN. > Actually an additional call to intel_dp_long_pulse from intel_dp_detect() because we are not resetting the detect_done flag, makes the function call intel_dp_check_link_status() in the case when intel_dp->lane_count is 0 and throws the WARN about that. So by resetting the flag we avoid this unwanted call to intel_dp_long_pulse and that gets rid of the warning. Manasi > > and also avoids DP compliance failures. It avoids doing > > a full detect every single time on .detect(). > > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index db75bb9..9c9277e 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -4470,8 +4470,6 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > > if (!intel_dp->detect_done) > > status = intel_dp_long_pulse(intel_dp->attached_connector); > > > > - intel_dp->detect_done = false; > > - > > return status; > > } > > > > -- > > 1.9.1 > > -- > Ville Syrjälä > Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect 2016-12-07 14:05 ` Ville Syrjälä 2016-12-07 20:46 ` Manasi Navare @ 2016-12-08 3:51 ` Manasi Navare 2016-12-08 15:58 ` Ville Syrjälä 1 sibling, 1 reply; 12+ messages in thread From: Manasi Navare @ 2016-12-08 3:51 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Daniel Vetter, intel-gfx Ville, You mentioned that this detect_done should be set to false also in the resume case so in i915_display_resume where it calls intel_hpd_init that eventually schedules the hpd_poll_init_work and calls drm_helper_hpd_irq_event() which then calls .detect(). So in this case if I have to reset this detect_done in the hpd_poll_init_work, how do I make sure I do it only if the connector is DP? Manasi On Wed, Dec 07, 2016 at 04:05:56PM +0200, Ville Syrjälä wrote: > On Tue, Dec 06, 2016 at 04:43:51PM -0800, Manasi Navare wrote: > > The detect_done flag was introduced in the commit > > 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c in order to avoid > > multiple detects on hotplug where intel_dp_long_pulse() > > was called from HPD handler as well as in intel_dp_detect. > > So this detect_done flag was required to make sure intel_dp_detect() > > did not call long pulse handler again if it was already been called > > from HPD handler. However commit 1015811609c0328b5ed670d07748591b837e74eb > > differs the long hpd handling entirely until the hotplug work runs to > > avoid the double long hpd handling the "detect_done" flag is trying > > to prevent. > > That sentence doesn't parse here. Anyways, the flag indeed is now a nop > and your patch is pretty much the same what I did here: > https://patchwork.freedesktop.org/patch/101476/ > > > > > So now we do not need to reset the detect_done flag to false in > > intel_dp_detect. It will be reset in the intel_dp_hpd_pulse so > > that intel_dp_detect does a full detect. However if the .detect > > gets called during mode enumeration then we do not need to do a > > full detect. This patch avoids the WARNS_ONS during connected boot > > case when it calls intel_dp_check_link_status() due to multiple > > detects > > How exactly does it do that? Also we shouldn't sweep that under the rug > anyway. Instead someone should actually fix the problem that causes the > WARN. > > > and also avoids DP compliance failures. It avoids doing > > a full detect every single time on .detect(). > > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index db75bb9..9c9277e 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -4470,8 +4470,6 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > > if (!intel_dp->detect_done) > > status = intel_dp_long_pulse(intel_dp->attached_connector); > > > > - intel_dp->detect_done = false; > > - > > return status; > > } > > > > -- > > 1.9.1 > > -- > Ville Syrjälä > Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect 2016-12-08 3:51 ` Manasi Navare @ 2016-12-08 15:58 ` Ville Syrjälä 2016-12-08 17:50 ` Manasi Navare 0 siblings, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2016-12-08 15:58 UTC (permalink / raw) To: Manasi Navare; +Cc: Daniel Vetter, intel-gfx On Wed, Dec 07, 2016 at 07:51:21PM -0800, Manasi Navare wrote: > Ville, > You mentioned that this detect_done should be set to false also > in the resume case so in i915_display_resume where it calls > intel_hpd_init that eventually schedules the hpd_poll_init_work and > calls drm_helper_hpd_irq_event() which then calls .detect(). > So in this case if I have to reset this detect_done in the > hpd_poll_init_work, how do I make sure I do it only if the > connector is DP? We have the .reset hook for these sort of things. > > Manasi > > On Wed, Dec 07, 2016 at 04:05:56PM +0200, Ville Syrjälä wrote: > > On Tue, Dec 06, 2016 at 04:43:51PM -0800, Manasi Navare wrote: > > > The detect_done flag was introduced in the commit > > > 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c in order to avoid > > > multiple detects on hotplug where intel_dp_long_pulse() > > > was called from HPD handler as well as in intel_dp_detect. > > > So this detect_done flag was required to make sure intel_dp_detect() > > > did not call long pulse handler again if it was already been called > > > from HPD handler. However commit 1015811609c0328b5ed670d07748591b837e74eb > > > differs the long hpd handling entirely until the hotplug work runs to > > > avoid the double long hpd handling the "detect_done" flag is trying > > > to prevent. > > > > That sentence doesn't parse here. Anyways, the flag indeed is now a nop > > and your patch is pretty much the same what I did here: > > https://patchwork.freedesktop.org/patch/101476/ > > > > > > > > So now we do not need to reset the detect_done flag to false in > > > intel_dp_detect. It will be reset in the intel_dp_hpd_pulse so > > > that intel_dp_detect does a full detect. However if the .detect > > > gets called during mode enumeration then we do not need to do a > > > full detect. This patch avoids the WARNS_ONS during connected boot > > > case when it calls intel_dp_check_link_status() due to multiple > > > detects > > > > How exactly does it do that? Also we shouldn't sweep that under the rug > > anyway. Instead someone should actually fix the problem that causes the > > WARN. > > > > > and also avoids DP compliance failures. It avoids doing > > > a full detect every single time on .detect(). > > > > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_dp.c | 2 -- > > > 1 file changed, 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > index db75bb9..9c9277e 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > @@ -4470,8 +4470,6 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > > > if (!intel_dp->detect_done) > > > status = intel_dp_long_pulse(intel_dp->attached_connector); > > > > > > - intel_dp->detect_done = false; > > > - > > > return status; > > > } > > > > > > -- > > > 1.9.1 > > > > -- > > Ville Syrjälä > > Intel OTC -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect 2016-12-08 15:58 ` Ville Syrjälä @ 2016-12-08 17:50 ` Manasi Navare 2016-12-20 2:42 ` [PATCH v2] drm/i915/dp: " Dhinakaran Pandiyan 0 siblings, 1 reply; 12+ messages in thread From: Manasi Navare @ 2016-12-08 17:50 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Daniel Vetter, intel-gfx On Thu, Dec 08, 2016 at 05:58:44PM +0200, Ville Syrjälä wrote: > On Wed, Dec 07, 2016 at 07:51:21PM -0800, Manasi Navare wrote: > > Ville, > > You mentioned that this detect_done should be set to false also > > in the resume case so in i915_display_resume where it calls > > intel_hpd_init that eventually schedules the hpd_poll_init_work and > > calls drm_helper_hpd_irq_event() which then calls .detect(). > > So in this case if I have to reset this detect_done in the > > hpd_poll_init_work, how do I make sure I do it only if the > > connector is DP? > > We have the .reset hook for these sort of things. > How can I use .reset hook to set the intel_dp->detect_done flag to false only for DP connector before sending drm_hpd_irq_event()? Manasi > > > > Manasi > > > > On Wed, Dec 07, 2016 at 04:05:56PM +0200, Ville Syrjälä wrote: > > > On Tue, Dec 06, 2016 at 04:43:51PM -0800, Manasi Navare wrote: > > > > The detect_done flag was introduced in the commit > > > > 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c in order to avoid > > > > multiple detects on hotplug where intel_dp_long_pulse() > > > > was called from HPD handler as well as in intel_dp_detect. > > > > So this detect_done flag was required to make sure intel_dp_detect() > > > > did not call long pulse handler again if it was already been called > > > > from HPD handler. However commit 1015811609c0328b5ed670d07748591b837e74eb > > > > differs the long hpd handling entirely until the hotplug work runs to > > > > avoid the double long hpd handling the "detect_done" flag is trying > > > > to prevent. > > > > > > That sentence doesn't parse here. Anyways, the flag indeed is now a nop > > > and your patch is pretty much the same what I did here: > > > https://patchwork.freedesktop.org/patch/101476/ > > > > > > > > > > > So now we do not need to reset the detect_done flag to false in > > > > intel_dp_detect. It will be reset in the intel_dp_hpd_pulse so > > > > that intel_dp_detect does a full detect. However if the .detect > > > > gets called during mode enumeration then we do not need to do a > > > > full detect. This patch avoids the WARNS_ONS during connected boot > > > > case when it calls intel_dp_check_link_status() due to multiple > > > > detects > > > > > > How exactly does it do that? Also we shouldn't sweep that under the rug > > > anyway. Instead someone should actually fix the problem that causes the > > > WARN. > > > > > > > and also avoids DP compliance failures. It avoids doing > > > > a full detect every single time on .detect(). > > > > > > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/intel_dp.c | 2 -- > > > > 1 file changed, 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > > index db75bb9..9c9277e 100644 > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > @@ -4470,8 +4470,6 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > > > > if (!intel_dp->detect_done) > > > > status = intel_dp_long_pulse(intel_dp->attached_connector); > > > > > > > > - intel_dp->detect_done = false; > > > > - > > > > return status; > > > > } > > > > > > > > -- > > > > 1.9.1 > > > > > > -- > > > Ville Syrjälä > > > Intel OTC > > -- > Ville Syrjälä > Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] drm/i915/dp: Do not reset detect_done flag in intel_dp_detect 2016-12-08 17:50 ` Manasi Navare @ 2016-12-20 2:42 ` Dhinakaran Pandiyan 2016-12-20 8:08 ` Jani Nikula 0 siblings, 1 reply; 12+ messages in thread From: Dhinakaran Pandiyan @ 2016-12-20 2:42 UTC (permalink / raw) To: intel-gfx; +Cc: Daniel Vetter, Dhinakaran Pandiyan From: "Navare, Manasi D" <manasi.d.navare@intel.com> The detect_done flag was introduced in the 'commit 7d23e3c37bb3 ("drm/i915: Cleaning up intel_dp_hpd_pulse")' in order to avoid multiple detects on hotplug where intel_dp_long_pulse() was called from HPD handler as well as intel_dp_detect(). Later, 'commit 1015811609c0 ("drm/i915: Move long hpd handling into the hotplug work")' deferred long hpd handling to hotplug work to avoid handling it twice. But, resetting the flag after long hpd handling leads to the code being executed again during mode enumeration. So, do not reset the detect_done flag to false in intel_dp_detect. The flag is reset in intel_dp_hpd_pulse to allow a full detect and set when the hotplug work does a full DPCD detect. However if .detect() gets called during enumeration we return the cached status. v2: Allow full detect after encoder reset. (Ville) Set the detect_done even connector if disconnected (DK) Commit message changes. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Jani Nikula <jani.nikula@linux.intel.com> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 45ebc96..5689cf3 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4473,7 +4473,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) intel_dp_set_edid(intel_dp); if (is_edp(intel_dp) || intel_connector->detect_edid) status = connector_status_connected; - intel_dp->detect_done = true; /* Try to read the source of the interrupt */ if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && @@ -4508,10 +4507,10 @@ intel_dp_detect(struct drm_connector *connector, bool force) connector->base.id, connector->name); /* If full detect is not performed yet, do a full detect */ - if (!intel_dp->detect_done) + if (!intel_dp->detect_done) { + intel_dp->detect_done = true; status = intel_dp_long_pulse(intel_dp->attached_connector); - - intel_dp->detect_done = false; + } return status; } @@ -4803,6 +4802,8 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder) if (lspcon->active) lspcon_resume(lspcon); + intel_dp->detect_done = false; + if (to_intel_encoder(encoder)->type != INTEL_OUTPUT_EDP) return; -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915/dp: Do not reset detect_done flag in intel_dp_detect 2016-12-20 2:42 ` [PATCH v2] drm/i915/dp: " Dhinakaran Pandiyan @ 2016-12-20 8:08 ` Jani Nikula 2016-12-20 21:05 ` Pandiyan, Dhinakaran 2016-12-21 1:57 ` [PATCH v3] " Dhinakaran Pandiyan 0 siblings, 2 replies; 12+ messages in thread From: Jani Nikula @ 2016-12-20 8:08 UTC (permalink / raw) To: intel-gfx; +Cc: Daniel Vetter, Dhinakaran Pandiyan On Tue, 20 Dec 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote: > From: "Navare, Manasi D" <manasi.d.navare@intel.com> > > The detect_done flag was introduced in the 'commit 7d23e3c37bb3 > ("drm/i915: Cleaning up intel_dp_hpd_pulse")' in order to avoid multiple > detects on hotplug where intel_dp_long_pulse() was called from HPD handler > as well as intel_dp_detect(). Later, 'commit 1015811609c0 > ("drm/i915: Move long hpd handling into the hotplug work")' deferred long > hpd handling to hotplug work to avoid handling it twice. But, resetting the > flag after long hpd handling leads to the code being executed again during > mode enumeration. > > So, do not reset the detect_done flag to false in intel_dp_detect. The flag > is reset in intel_dp_hpd_pulse to allow a full detect and set when the > hotplug work does a full DPCD detect. However if .detect() gets called > during enumeration we return the cached status. > > v2: > Allow full detect after encoder reset. (Ville) > Set the detect_done even connector if disconnected (DK) > Commit message changes. > Fixes: ? > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 45ebc96..5689cf3 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4473,7 +4473,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) > intel_dp_set_edid(intel_dp); > if (is_edp(intel_dp) || intel_connector->detect_edid) > status = connector_status_connected; > - intel_dp->detect_done = true; > > /* Try to read the source of the interrupt */ > if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && > @@ -4508,10 +4507,10 @@ intel_dp_detect(struct drm_connector *connector, bool force) > connector->base.id, connector->name); > > /* If full detect is not performed yet, do a full detect */ > - if (!intel_dp->detect_done) > + if (!intel_dp->detect_done) { > + intel_dp->detect_done = true; > status = intel_dp_long_pulse(intel_dp->attached_connector); > - > - intel_dp->detect_done = false; > + } > > return status; > } > @@ -4803,6 +4802,8 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder) > if (lspcon->active) > lspcon_resume(lspcon); > > + intel_dp->detect_done = false; > + > if (to_intel_encoder(encoder)->type != INTEL_OUTPUT_EDP) > return; -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] drm/i915/dp: Do not reset detect_done flag in intel_dp_detect 2016-12-20 8:08 ` Jani Nikula @ 2016-12-20 21:05 ` Pandiyan, Dhinakaran 2016-12-21 1:57 ` [PATCH v3] " Dhinakaran Pandiyan 1 sibling, 0 replies; 12+ messages in thread From: Pandiyan, Dhinakaran @ 2016-12-20 21:05 UTC (permalink / raw) To: jani.nikula; +Cc: daniel.vetter, intel-gfx On Tue, 2016-12-20 at 10:08 +0200, Jani Nikula wrote: > On Tue, 20 Dec 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote: > > From: "Navare, Manasi D" <manasi.d.navare@intel.com> > > > > The detect_done flag was introduced in the 'commit 7d23e3c37bb3 > > ("drm/i915: Cleaning up intel_dp_hpd_pulse")' in order to avoid multiple > > detects on hotplug where intel_dp_long_pulse() was called from HPD handler > > as well as intel_dp_detect(). Later, 'commit 1015811609c0 > > ("drm/i915: Move long hpd handling into the hotplug work")' deferred long > > hpd handling to hotplug work to avoid handling it twice. But, resetting the > > flag after long hpd handling leads to the code being executed again during > > mode enumeration. > > > > So, do not reset the detect_done flag to false in intel_dp_detect. The flag > > is reset in intel_dp_hpd_pulse to allow a full detect and set when the > > hotplug work does a full DPCD detect. However if .detect() gets called > > during enumeration we return the cached status. > > > > v2: > > Allow full detect after encoder reset. (Ville) > > Set the detect_done even connector if disconnected (DK) > > Commit message changes. > > > > Fixes: ? > There have been too many changes here, I don't know which commit to really blame. Most likely Fixes: commit 7d23e3c37bb3 ("drm/i915: Cleaning up intel_dp_hpd_pulse") -DK > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 45ebc96..5689cf3 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -4473,7 +4473,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) > > intel_dp_set_edid(intel_dp); > > if (is_edp(intel_dp) || intel_connector->detect_edid) > > status = connector_status_connected; > > - intel_dp->detect_done = true; > > > > /* Try to read the source of the interrupt */ > > if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && > > @@ -4508,10 +4507,10 @@ intel_dp_detect(struct drm_connector *connector, bool force) > > connector->base.id, connector->name); > > > > /* If full detect is not performed yet, do a full detect */ > > - if (!intel_dp->detect_done) > > + if (!intel_dp->detect_done) { > > + intel_dp->detect_done = true; > > status = intel_dp_long_pulse(intel_dp->attached_connector); > > - > > - intel_dp->detect_done = false; > > + } > > > > return status; > > } > > @@ -4803,6 +4802,8 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder) > > if (lspcon->active) > > lspcon_resume(lspcon); > > > > + intel_dp->detect_done = false; > > + > > if (to_intel_encoder(encoder)->type != INTEL_OUTPUT_EDP) > > return; > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3] drm/i915/dp: Do not reset detect_done flag in intel_dp_detect 2016-12-20 8:08 ` Jani Nikula 2016-12-20 21:05 ` Pandiyan, Dhinakaran @ 2016-12-21 1:57 ` Dhinakaran Pandiyan 1 sibling, 0 replies; 12+ messages in thread From: Dhinakaran Pandiyan @ 2016-12-21 1:57 UTC (permalink / raw) To: intel-gfx Cc: Navare, Manasi D, stable, Ville Syrjala, Ander Conselvande Oliveira, Jani Nikula, Dhinakaran Pandiyan From: "Navare, Manasi D" <manasi.d.navare@intel.com> The detect_done flag was introduced in the 'commit 7d23e3c37bb3 ("drm/i915: Cleaning up intel_dp_hpd_pulse")' in order to avoid multiple detects on hotplug where intel_dp_long_pulse() was called from HPD handler as well as intel_dp_detect(). Later, 'commit 1015811609c0 ("drm/i915: Move long hpd handling into the hotplug work")' deferred long hpd handling to hotplug work to avoid handling it twice. But, resetting the flag after long hpd handling leads to the code being executed again during mode enumeration. So, do not reset the detect_done flag to False in intel_dp_detect(). The flag is reset in intel_dp_hpd_pulse() to allow a full detect and set when the hotplug work does a full DPCD detect. However if ->detect() gets called during mode enumeration after a DPCD detect, return the cached connector status. Resetting the flag in the encoder's reset callback should take care of hotplug between suspend/resume. v2: Allow full detect after encoder reset. (Ville) Set the detect_done flag for connector disconnected case too. (DK) Commit message changes. Cc: stable@vger.kernel.org Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Ander Conselvande Oliveira <ander.conselvan.de.oliveira@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Fixes: commit 7d23e3c37bb3 ("drm/i915: Cleaning up intel_dp_hpd_pulse") Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 45ebc96..5689cf3 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4473,7 +4473,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) intel_dp_set_edid(intel_dp); if (is_edp(intel_dp) || intel_connector->detect_edid) status = connector_status_connected; - intel_dp->detect_done = true; /* Try to read the source of the interrupt */ if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && @@ -4508,10 +4507,10 @@ intel_dp_detect(struct drm_connector *connector, bool force) connector->base.id, connector->name); /* If full detect is not performed yet, do a full detect */ - if (!intel_dp->detect_done) + if (!intel_dp->detect_done) { + intel_dp->detect_done = true; status = intel_dp_long_pulse(intel_dp->attached_connector); - - intel_dp->detect_done = false; + } return status; } @@ -4803,6 +4802,8 @@ void intel_dp_encoder_reset(struct drm_encoder *encoder) if (lspcon->active) lspcon_resume(lspcon); + intel_dp->detect_done = false; + if (to_intel_encoder(encoder)->type != INTEL_OUTPUT_EDP) return; -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-12-21 1:58 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-07 0:43 [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect Manasi Navare 2016-12-07 1:15 ` ✓ Fi.CI.BAT: success for " Patchwork 2016-12-07 8:52 ` [PATCH] " Jani Nikula 2016-12-07 14:05 ` Ville Syrjälä 2016-12-07 20:46 ` Manasi Navare 2016-12-08 3:51 ` Manasi Navare 2016-12-08 15:58 ` Ville Syrjälä 2016-12-08 17:50 ` Manasi Navare 2016-12-20 2:42 ` [PATCH v2] drm/i915/dp: " Dhinakaran Pandiyan 2016-12-20 8:08 ` Jani Nikula 2016-12-20 21:05 ` Pandiyan, Dhinakaran 2016-12-21 1:57 ` [PATCH v3] " Dhinakaran Pandiyan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.