* [PATCH] drm/i915/lspcon: Enable AUX interrupts for resume time initialization @ 2016-11-29 14:52 Imre Deak 2016-11-29 19:40 ` [PATCH v2] " Imre Deak 2016-11-29 21:53 ` ✓ Fi.CI.BAT: success for drm/i915/lspcon: Enable AUX interrupts for resume time initialization (rev2) Patchwork 0 siblings, 2 replies; 11+ messages in thread From: Imre Deak @ 2016-11-29 14:52 UTC (permalink / raw) To: intel-gfx For LSPCON initialization during system resume we need AUX functionality, but we call the corresponding encoder reset hook with all interrupts disabled. Without interrupts we'll do a poll-wait for AUX transfer completions, which adds a significant delay if the transfers timeout/need to be retried for some reason. Fix this by enabling interrupts before calling the reset hooks. Note that while this will enable AUX interrupts it will keep HPD interrupts disabled, in a similar way to the init time output setup code. This issue existed since LSPCON support was added. Cc: Shashank Sharma <shashank.sharma@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index b893e67..1e76e29 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1580,18 +1580,21 @@ static int i915_drm_resume(struct drm_device *dev) intel_opregion_setup(dev_priv); intel_init_pch_refclk(dev); - drm_mode_config_reset(dev); /* * Interrupts have to be enabled before any batches are run. If not the * GPU will hang. i915_gem_init_hw() will initiate batches to * update/restore the context. * + * drm_mode_config_reset() needs AUX interrupts. + * * Modeset enabling in intel_modeset_init_hw() also needs working * interrupts. */ intel_runtime_pm_enable_interrupts(dev_priv); + drm_mode_config_reset(dev); + mutex_lock(&dev->struct_mutex); if (i915_gem_init_hw(dev)) { DRM_ERROR("failed to re-initialize GPU, declaring wedged!\n"); -- 2.5.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization 2016-11-29 14:52 [PATCH] drm/i915/lspcon: Enable AUX interrupts for resume time initialization Imre Deak @ 2016-11-29 19:40 ` Imre Deak 2016-11-29 19:55 ` Ville Syrjälä 2016-12-01 13:18 ` David Weinehall 2016-11-29 21:53 ` ✓ Fi.CI.BAT: success for drm/i915/lspcon: Enable AUX interrupts for resume time initialization (rev2) Patchwork 1 sibling, 2 replies; 11+ messages in thread From: Imre Deak @ 2016-11-29 19:40 UTC (permalink / raw) To: intel-gfx For LSPCON initialization during system resume we need AUX functionality, but we call the corresponding encoder reset hook with all interrupts disabled. Without interrupts we'll do a poll-wait for AUX transfer completions, which adds a significant delay if the transfers timeout/need to be retried for some reason. Fix this by enabling interrupts before calling the reset hooks. Note that while this will enable AUX interrupts it will keep HPD interrupts disabled, in a similar way to the init time output setup code. This issue existed since LSPCON support was added. v2: - Rebased on drm-tip. Cc: Shashank Sharma <shashank.sharma@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 8dac298..2cea2ef 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1582,18 +1582,21 @@ static int i915_drm_resume(struct drm_device *dev) intel_opregion_setup(dev_priv); intel_init_pch_refclk(dev_priv); - drm_mode_config_reset(dev); /* * Interrupts have to be enabled before any batches are run. If not the * GPU will hang. i915_gem_init_hw() will initiate batches to * update/restore the context. * + * drm_mode_config_reset() needs AUX interrupts. + * * Modeset enabling in intel_modeset_init_hw() also needs working * interrupts. */ intel_runtime_pm_enable_interrupts(dev_priv); + drm_mode_config_reset(dev); + mutex_lock(&dev->struct_mutex); if (i915_gem_init_hw(dev)) { DRM_ERROR("failed to re-initialize GPU, declaring wedged!\n"); -- 2.5.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization 2016-11-29 19:40 ` [PATCH v2] " Imre Deak @ 2016-11-29 19:55 ` Ville Syrjälä 2016-11-29 20:14 ` Imre Deak 2016-12-01 13:18 ` David Weinehall 1 sibling, 1 reply; 11+ messages in thread From: Ville Syrjälä @ 2016-11-29 19:55 UTC (permalink / raw) To: Imre Deak; +Cc: intel-gfx On Tue, Nov 29, 2016 at 09:40:29PM +0200, Imre Deak wrote: > For LSPCON initialization during system resume we need AUX > functionality, but we call the corresponding encoder reset hook with all > interrupts disabled. Without interrupts we'll do a poll-wait for AUX > transfer completions, which adds a significant delay if the transfers > timeout/need to be retried for some reason. > > Fix this by enabling interrupts before calling the reset hooks. Note > that while this will enable AUX interrupts it will keep HPD interrupts > disabled, in a similar way to the init time output setup code. > > This issue existed since LSPCON support was added. > > v2: > - Rebased on drm-tip. > > Cc: Shashank Sharma <shashank.sharma@intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 8dac298..2cea2ef 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1582,18 +1582,21 @@ static int i915_drm_resume(struct drm_device *dev) > intel_opregion_setup(dev_priv); > > intel_init_pch_refclk(dev_priv); > - drm_mode_config_reset(dev); > > /* > * Interrupts have to be enabled before any batches are run. If not the > * GPU will hang. i915_gem_init_hw() will initiate batches to > * update/restore the context. > * > + * drm_mode_config_reset() needs AUX interrupts. > + * > * Modeset enabling in intel_modeset_init_hw() also needs working > * interrupts. > */ > intel_runtime_pm_enable_interrupts(dev_priv); > > + drm_mode_config_reset(dev); > + Didn't look like anything should seriously explode. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> There is a slight concern on g4x/vlv/chv that an AUX interrupts would trigger the hpd irq handler, which doesn't realize it's supposed to ignore the actual hpd bits in PORT_HOTPLUG_STAT. So any aux before we enable hpd processing for real could do something bad. So I guess we should add some kind of software tracking for that stuff like we have for PIPESTAT. > mutex_lock(&dev->struct_mutex); > if (i915_gem_init_hw(dev)) { > DRM_ERROR("failed to re-initialize GPU, declaring wedged!\n"); > -- > 2.5.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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] 11+ messages in thread
* Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization 2016-11-29 19:55 ` Ville Syrjälä @ 2016-11-29 20:14 ` Imre Deak 2016-11-29 20:27 ` Ville Syrjälä 0 siblings, 1 reply; 11+ messages in thread From: Imre Deak @ 2016-11-29 20:14 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx On Tue, 2016-11-29 at 21:55 +0200, Ville Syrjälä wrote: > On Tue, Nov 29, 2016 at 09:40:29PM +0200, Imre Deak wrote: > > For LSPCON initialization during system resume we need AUX > > functionality, but we call the corresponding encoder reset hook with all > > interrupts disabled. Without interrupts we'll do a poll-wait for AUX > > transfer completions, which adds a significant delay if the transfers > > timeout/need to be retried for some reason. > > > > Fix this by enabling interrupts before calling the reset hooks. Note > > that while this will enable AUX interrupts it will keep HPD interrupts > > disabled, in a similar way to the init time output setup code. > > > > This issue existed since LSPCON support was added. > > > > v2: > > - Rebased on drm-tip. > > > > Cc: Shashank Sharma <shashank.sharma@intel.com> > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 8dac298..2cea2ef 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -1582,18 +1582,21 @@ static int i915_drm_resume(struct drm_device *dev) > > intel_opregion_setup(dev_priv); > > > > intel_init_pch_refclk(dev_priv); > > - drm_mode_config_reset(dev); > > > > /* > > * Interrupts have to be enabled before any batches are run. If not the > > * GPU will hang. i915_gem_init_hw() will initiate batches to > > * update/restore the context. > > * > > + * drm_mode_config_reset() needs AUX interrupts. > > + * > > * Modeset enabling in intel_modeset_init_hw() also needs working > > * interrupts. > > */ > > intel_runtime_pm_enable_interrupts(dev_priv); > > > > + drm_mode_config_reset(dev); > > + > > Didn't look like anything should seriously explode. > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > There is a slight concern on g4x/vlv/chv that an AUX interrupts would > trigger the hpd irq handler, which doesn't realize it's supposed to > ignore the actual hpd bits in PORT_HOTPLUG_STAT. So any aux before we > enable hpd processing for real could do something bad. So I guess we > should add some kind of software tracking for that stuff like we have > for PIPESTAT. Didn't think about that, but BSpec tells me those are masked by the HPD IRQ enable bits in PORT_HOTPLUG_EN and those we enable only later. Otherwise this would be also a problem during output setup time. --Imre > > > mutex_lock(&dev->struct_mutex); > > if (i915_gem_init_hw(dev)) { > > DRM_ERROR("failed to re-initialize GPU, declaring wedged!\n"); > > -- > > 2.5.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization 2016-11-29 20:14 ` Imre Deak @ 2016-11-29 20:27 ` Ville Syrjälä 2016-11-29 20:41 ` Imre Deak 0 siblings, 1 reply; 11+ messages in thread From: Ville Syrjälä @ 2016-11-29 20:27 UTC (permalink / raw) To: Imre Deak; +Cc: intel-gfx On Tue, Nov 29, 2016 at 10:14:23PM +0200, Imre Deak wrote: > On Tue, 2016-11-29 at 21:55 +0200, Ville Syrjälä wrote: > > On Tue, Nov 29, 2016 at 09:40:29PM +0200, Imre Deak wrote: > > > For LSPCON initialization during system resume we need AUX > > > functionality, but we call the corresponding encoder reset hook with all > > > interrupts disabled. Without interrupts we'll do a poll-wait for AUX > > > transfer completions, which adds a significant delay if the transfers > > > timeout/need to be retried for some reason. > > > > > > Fix this by enabling interrupts before calling the reset hooks. Note > > > that while this will enable AUX interrupts it will keep HPD interrupts > > > disabled, in a similar way to the init time output setup code. > > > > > > This issue existed since LSPCON support was added. > > > > > > v2: > > > - Rebased on drm-tip. > > > > > > Cc: Shashank Sharma <shashank.sharma@intel.com> > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_drv.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > > index 8dac298..2cea2ef 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -1582,18 +1582,21 @@ static int i915_drm_resume(struct drm_device *dev) > > > intel_opregion_setup(dev_priv); > > > > > > intel_init_pch_refclk(dev_priv); > > > - drm_mode_config_reset(dev); > > > > > > /* > > > * Interrupts have to be enabled before any batches are run. If not the > > > * GPU will hang. i915_gem_init_hw() will initiate batches to > > > * update/restore the context. > > > * > > > + * drm_mode_config_reset() needs AUX interrupts. > > > + * > > > * Modeset enabling in intel_modeset_init_hw() also needs working > > > * interrupts. > > > */ > > > intel_runtime_pm_enable_interrupts(dev_priv); > > > > > > + drm_mode_config_reset(dev); > > > + > > > > Didn't look like anything should seriously explode. > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > There is a slight concern on g4x/vlv/chv that an AUX interrupts would > > trigger the hpd irq handler, which doesn't realize it's supposed to > > ignore the actual hpd bits in PORT_HOTPLUG_STAT. So any aux before we > > enable hpd processing for real could do something bad. So I guess we > > should add some kind of software tracking for that stuff like we have > > for PIPESTAT. > > Didn't think about that, but BSpec tells me those are masked by the HPD > IRQ enable bits in PORT_HOTPLUG_EN and those we enable only later. > Otherwise this would be also a problem during output setup time. Hmm. Are they really masked? I though it's just an IER effectively. > > --Imre > > > > > > mutex_lock(&dev->struct_mutex); > > > if (i915_gem_init_hw(dev)) { > > > DRM_ERROR("failed to re-initialize GPU, declaring wedged!\n"); > > > -- > > > 2.5.0 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- 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] 11+ messages in thread
* Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization 2016-11-29 20:27 ` Ville Syrjälä @ 2016-11-29 20:41 ` Imre Deak 2016-11-29 21:00 ` Ville Syrjälä 0 siblings, 1 reply; 11+ messages in thread From: Imre Deak @ 2016-11-29 20:41 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx On Tue, 2016-11-29 at 22:27 +0200, Ville Syrjälä wrote: > On Tue, Nov 29, 2016 at 10:14:23PM +0200, Imre Deak wrote: > > On Tue, 2016-11-29 at 21:55 +0200, Ville Syrjälä wrote: > > > On Tue, Nov 29, 2016 at 09:40:29PM +0200, Imre Deak wrote: > > > > For LSPCON initialization during system resume we need AUX > > > > functionality, but we call the corresponding encoder reset hook > > > > with all > > > > interrupts disabled. Without interrupts we'll do a poll-wait > > > > for AUX > > > > transfer completions, which adds a significant delay if the > > > > transfers > > > > timeout/need to be retried for some reason. > > > > > > > > Fix this by enabling interrupts before calling the reset hooks. > > > > Note > > > > that while this will enable AUX interrupts it will keep HPD > > > > interrupts > > > > disabled, in a similar way to the init time output setup code. > > > > > > > > This issue existed since LSPCON support was added. > > > > > > > > v2: > > > > - Rebased on drm-tip. > > > > > > > > Cc: Shashank Sharma <shashank.sharma@intel.com> > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/i915_drv.c | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > index 8dac298..2cea2ef 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > @@ -1582,18 +1582,21 @@ static int i915_drm_resume(struct > > > > drm_device *dev) > > > > intel_opregion_setup(dev_priv); > > > > > > > > intel_init_pch_refclk(dev_priv); > > > > - drm_mode_config_reset(dev); > > > > > > > > /* > > > > * Interrupts have to be enabled before any batches > > > > are run. If not the > > > > * GPU will hang. i915_gem_init_hw() will initiate > > > > batches to > > > > * update/restore the context. > > > > * > > > > + * drm_mode_config_reset() needs AUX interrupts. > > > > + * > > > > * Modeset enabling in intel_modeset_init_hw() also > > > > needs working > > > > * interrupts. > > > > */ > > > > intel_runtime_pm_enable_interrupts(dev_priv); > > > > > > > > + drm_mode_config_reset(dev); > > > > + > > > > > > Didn't look like anything should seriously explode. > > > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > There is a slight concern on g4x/vlv/chv that an AUX interrupts > > > would > > > trigger the hpd irq handler, which doesn't realize it's supposed > > > to > > > ignore the actual hpd bits in PORT_HOTPLUG_STAT. So any aux > > > before we > > > enable hpd processing for real could do something bad. So I guess > > > we > > > should add some kind of software tracking for that stuff like we > > > have > > > for PIPESTAT. > > > > Didn't think about that, but BSpec tells me those are masked by the > > HPD > > IRQ enable bits in PORT_HOTPLUG_EN and those we enable only later. > > Otherwise this would be also a problem during output setup time. > > Hmm. Are they really masked? I though it's just an IER effectively. I only tried for real on BXT/SKL where I had to enable the interrupts (in PCH_PORT_HOTPLUG) for HPD sensing. The CHV BSpec suggests the same for the live state bits, but yes it's not clear if the long/short detect bits are completely masked by the enable flags or they are just not propagated if not enabled. Will give it a try tomorrow. > > > > > --Imre > > > > > > > > > mutex_lock(&dev->struct_mutex); > > > > if (i915_gem_init_hw(dev)) { > > > > DRM_ERROR("failed to re-initialize GPU, > > > > declaring wedged!\n"); > > > > -- > > > > 2.5.0 > > > > > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization 2016-11-29 20:41 ` Imre Deak @ 2016-11-29 21:00 ` Ville Syrjälä 2016-11-30 12:27 ` Imre Deak 0 siblings, 1 reply; 11+ messages in thread From: Ville Syrjälä @ 2016-11-29 21:00 UTC (permalink / raw) To: Imre Deak; +Cc: intel-gfx On Tue, Nov 29, 2016 at 10:41:45PM +0200, Imre Deak wrote: > On Tue, 2016-11-29 at 22:27 +0200, Ville Syrjälä wrote: > > On Tue, Nov 29, 2016 at 10:14:23PM +0200, Imre Deak wrote: > > > On Tue, 2016-11-29 at 21:55 +0200, Ville Syrjälä wrote: > > > > On Tue, Nov 29, 2016 at 09:40:29PM +0200, Imre Deak wrote: > > > > > For LSPCON initialization during system resume we need AUX > > > > > functionality, but we call the corresponding encoder reset hook > > > > > with all > > > > > interrupts disabled. Without interrupts we'll do a poll-wait > > > > > for AUX > > > > > transfer completions, which adds a significant delay if the > > > > > transfers > > > > > timeout/need to be retried for some reason. > > > > > > > > > > Fix this by enabling interrupts before calling the reset hooks. > > > > > Note > > > > > that while this will enable AUX interrupts it will keep HPD > > > > > interrupts > > > > > disabled, in a similar way to the init time output setup code. > > > > > > > > > > This issue existed since LSPCON support was added. > > > > > > > > > > v2: > > > > > - Rebased on drm-tip. > > > > > > > > > > Cc: Shashank Sharma <shashank.sharma@intel.com> > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/i915_drv.c | 5 ++++- > > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > > index 8dac298..2cea2ef 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > > @@ -1582,18 +1582,21 @@ static int i915_drm_resume(struct > > > > > drm_device *dev) > > > > > intel_opregion_setup(dev_priv); > > > > > > > > > > intel_init_pch_refclk(dev_priv); > > > > > - drm_mode_config_reset(dev); > > > > > > > > > > /* > > > > > * Interrupts have to be enabled before any batches > > > > > are run. If not the > > > > > * GPU will hang. i915_gem_init_hw() will initiate > > > > > batches to > > > > > * update/restore the context. > > > > > * > > > > > + * drm_mode_config_reset() needs AUX interrupts. > > > > > + * > > > > > * Modeset enabling in intel_modeset_init_hw() also > > > > > needs working > > > > > * interrupts. > > > > > */ > > > > > intel_runtime_pm_enable_interrupts(dev_priv); > > > > > > > > > > + drm_mode_config_reset(dev); > > > > > + > > > > > > > > Didn't look like anything should seriously explode. > > > > > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > There is a slight concern on g4x/vlv/chv that an AUX interrupts > > > > would > > > > trigger the hpd irq handler, which doesn't realize it's supposed > > > > to > > > > ignore the actual hpd bits in PORT_HOTPLUG_STAT. So any aux > > > > before we > > > > enable hpd processing for real could do something bad. So I guess > > > > we > > > > should add some kind of software tracking for that stuff like we > > > > have > > > > for PIPESTAT. > > > > > > Didn't think about that, but BSpec tells me those are masked by the > > > HPD > > > IRQ enable bits in PORT_HOTPLUG_EN and those we enable only later. > > > Otherwise this would be also a problem during output setup time. > > > > Hmm. Are they really masked? I though it's just an IER effectively. > > I only tried for real on BXT/SKL where I had to enable the interrupts > (in PCH_PORT_HOTPLUG) for HPD sensing. The CHV BSpec suggests the same > for the live state bits, but yes it's not clear if the long/short > detect bits are completely masked by the enable flags or they are just > not propagated if not enabled. Will give it a try tomorrow. Hmm. Yeah, we did in fact chat about this. Already forgot. Spec seems to suggest you are correct. But checking on actual hw is always a good idea. > > > > > > > > > --Imre > > > > > > > > > > > > mutex_lock(&dev->struct_mutex); > > > > > if (i915_gem_init_hw(dev)) { > > > > > DRM_ERROR("failed to re-initialize GPU, > > > > > declaring wedged!\n"); > > > > > -- > > > > > 2.5.0 > > > > > > > > > > _______________________________________________ > > > > > Intel-gfx mailing list > > > > > Intel-gfx@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > -- 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] 11+ messages in thread
* Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization 2016-11-29 21:00 ` Ville Syrjälä @ 2016-11-30 12:27 ` Imre Deak 0 siblings, 0 replies; 11+ messages in thread From: Imre Deak @ 2016-11-30 12:27 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx On ti, 2016-11-29 at 23:00 +0200, Ville Syrjälä wrote: > [...] > > > > > There is a slight concern on g4x/vlv/chv that an AUX interrupts > > > > > would > > > > > trigger the hpd irq handler, which doesn't realize it's supposed > > > > > to > > > > > ignore the actual hpd bits in PORT_HOTPLUG_STAT. So any aux > > > > > before we > > > > > enable hpd processing for real could do something bad. So I guess > > > > > we > > > > > should add some kind of software tracking for that stuff like we > > > > > have > > > > > for PIPESTAT. > > > > > > > > Didn't think about that, but BSpec tells me those are masked by the > > > > HPD > > > > IRQ enable bits in PORT_HOTPLUG_EN and those we enable only later. > > > > Otherwise this would be also a problem during output setup time. > > > > > > Hmm. Are they really masked? I though it's just an IER effectively. > > > > I only tried for real on BXT/SKL where I had to enable the interrupts > > (in PCH_PORT_HOTPLUG) for HPD sensing. The CHV BSpec suggests the same > > for the live state bits, but yes it's not clear if the long/short > > detect bits are completely masked by the enable flags or they are just > > not propagated if not enabled. Will give it a try tomorrow. > > Hmm. Yeah, we did in fact chat about this. Already forgot. Spec seems to > suggest you are correct. But checking on actual hw is always a good > idea. Checked now both on BXT and VLV both the live state and short/long detect bits in the hotplug_stat reg are masked by the hotplug_en bits. (And we clear any stale short/long bits during IRQ reset.) --Imre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization 2016-11-29 19:40 ` [PATCH v2] " Imre Deak 2016-11-29 19:55 ` Ville Syrjälä @ 2016-12-01 13:18 ` David Weinehall 1 sibling, 0 replies; 11+ messages in thread From: David Weinehall @ 2016-12-01 13:18 UTC (permalink / raw) To: Imre Deak; +Cc: intel-gfx On Tue, Nov 29, 2016 at 09:40:29PM +0200, Imre Deak wrote: > For LSPCON initialization during system resume we need AUX > functionality, but we call the corresponding encoder reset hook with all > interrupts disabled. Without interrupts we'll do a poll-wait for AUX > transfer completions, which adds a significant delay if the transfers > timeout/need to be retried for some reason. > > Fix this by enabling interrupts before calling the reset hooks. Note > that while this will enable AUX interrupts it will keep HPD interrupts > disabled, in a similar way to the init time output setup code. > > This issue existed since LSPCON support was added. > > v2: > - Rebased on drm-tip. > > Cc: Shashank Sharma <shashank.sharma@intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> Tested-by: David Weinehall <david.weinehall@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 8dac298..2cea2ef 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1582,18 +1582,21 @@ static int i915_drm_resume(struct drm_device *dev) > intel_opregion_setup(dev_priv); > > intel_init_pch_refclk(dev_priv); > - drm_mode_config_reset(dev); > > /* > * Interrupts have to be enabled before any batches are run. If not the > * GPU will hang. i915_gem_init_hw() will initiate batches to > * update/restore the context. > * > + * drm_mode_config_reset() needs AUX interrupts. > + * > * Modeset enabling in intel_modeset_init_hw() also needs working > * interrupts. > */ > intel_runtime_pm_enable_interrupts(dev_priv); > > + drm_mode_config_reset(dev); > + > mutex_lock(&dev->struct_mutex); > if (i915_gem_init_hw(dev)) { > DRM_ERROR("failed to re-initialize GPU, declaring wedged!\n"); > -- > 2.5.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/lspcon: Enable AUX interrupts for resume time initialization (rev2) 2016-11-29 14:52 [PATCH] drm/i915/lspcon: Enable AUX interrupts for resume time initialization Imre Deak 2016-11-29 19:40 ` [PATCH v2] " Imre Deak @ 2016-11-29 21:53 ` Patchwork 2016-12-01 13:33 ` Imre Deak 1 sibling, 1 reply; 11+ messages in thread From: Patchwork @ 2016-11-29 21:53 UTC (permalink / raw) To: Imre Deak; +Cc: intel-gfx == Series Details == Series: drm/i915/lspcon: Enable AUX interrupts for resume time initialization (rev2) URL : https://patchwork.freedesktop.org/series/16106/ State : success == Summary == Series 16106v2 drm/i915/lspcon: Enable AUX interrupts for resume time initialization https://patchwork.freedesktop.org/api/1.0/series/16106/revisions/2/mbox/ fi-bdw-5557u total:245 pass:230 dwarn:0 dfail:0 fail:0 skip:15 fi-bsw-n3050 total:245 pass:205 dwarn:0 dfail:0 fail:0 skip:40 fi-byt-j1900 total:245 pass:217 dwarn:0 dfail:0 fail:0 skip:28 fi-byt-n2820 total:245 pass:213 dwarn:0 dfail:0 fail:0 skip:32 fi-hsw-4770 total:245 pass:225 dwarn:0 dfail:0 fail:0 skip:20 fi-hsw-4770r total:245 pass:225 dwarn:0 dfail:0 fail:0 skip:20 fi-ilk-650 total:245 pass:192 dwarn:0 dfail:0 fail:0 skip:53 fi-ivb-3520m total:245 pass:223 dwarn:0 dfail:0 fail:0 skip:22 fi-ivb-3770 total:245 pass:223 dwarn:0 dfail:0 fail:0 skip:22 fi-kbl-7500u total:245 pass:223 dwarn:0 dfail:0 fail:0 skip:22 fi-skl-6260u total:245 pass:231 dwarn:0 dfail:0 fail:0 skip:14 fi-skl-6700hq total:245 pass:224 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6700k total:245 pass:223 dwarn:1 dfail:0 fail:0 skip:21 fi-skl-6770hq total:245 pass:231 dwarn:0 dfail:0 fail:0 skip:14 fi-snb-2520m total:245 pass:213 dwarn:0 dfail:0 fail:0 skip:32 fi-snb-2600 total:245 pass:212 dwarn:0 dfail:0 fail:0 skip:33 41799fbb771e82427273492bfad8f2e2ae3027ef drm-tip: 2016y-11m-29d-20h-54m-07s UTC integration manifest d61aedb drm/i915/lspcon: Enable AUX interrupts for resume time initialization == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3147/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ✓ Fi.CI.BAT: success for drm/i915/lspcon: Enable AUX interrupts for resume time initialization (rev2) 2016-11-29 21:53 ` ✓ Fi.CI.BAT: success for drm/i915/lspcon: Enable AUX interrupts for resume time initialization (rev2) Patchwork @ 2016-12-01 13:33 ` Imre Deak 0 siblings, 0 replies; 11+ messages in thread From: Imre Deak @ 2016-12-01 13:33 UTC (permalink / raw) To: intel-gfx, Ville Syrjälä, David Weinehall On ti, 2016-11-29 at 21:53 +0000, Patchwork wrote: > == Series Details == > > Series: drm/i915/lspcon: Enable AUX interrupts for resume time initialization (rev2) > URL : https://patchwork.freedesktop.org/series/16106/ > State : success > > == Summary == > > Series 16106v2 drm/i915/lspcon: Enable AUX interrupts for resume time initialization > https://patchwork.freedesktop.org/api/1.0/series/16106/revisions/2/mbox/ I pushed the patch to -dinq, thanks for the testing and review. --Imre > > > fi-bdw-5557u total:245 pass:230 dwarn:0 dfail:0 fail:0 skip:15 > fi-bsw-n3050 total:245 pass:205 dwarn:0 dfail:0 fail:0 skip:40 > fi-byt-j1900 total:245 pass:217 dwarn:0 dfail:0 fail:0 skip:28 > fi-byt-n2820 total:245 pass:213 dwarn:0 dfail:0 fail:0 skip:32 > fi-hsw-4770 total:245 pass:225 dwarn:0 dfail:0 fail:0 skip:20 > fi-hsw-4770r total:245 pass:225 dwarn:0 dfail:0 fail:0 skip:20 > fi-ilk-650 total:245 pass:192 dwarn:0 dfail:0 fail:0 skip:53 > fi-ivb-3520m total:245 pass:223 dwarn:0 dfail:0 fail:0 skip:22 > fi-ivb-3770 total:245 pass:223 dwarn:0 dfail:0 fail:0 skip:22 > fi-kbl-7500u total:245 pass:223 dwarn:0 dfail:0 fail:0 skip:22 > fi-skl-6260u total:245 pass:231 dwarn:0 dfail:0 fail:0 skip:14 > fi-skl-6700hq total:245 pass:224 dwarn:0 dfail:0 fail:0 skip:21 > fi-skl-6700k total:245 pass:223 dwarn:1 dfail:0 fail:0 skip:21 > fi-skl-6770hq total:245 pass:231 dwarn:0 dfail:0 fail:0 skip:14 > fi-snb-2520m total:245 pass:213 dwarn:0 dfail:0 fail:0 skip:32 > fi-snb-2600 total:245 pass:212 dwarn:0 dfail:0 fail:0 skip:33 > > 41799fbb771e82427273492bfad8f2e2ae3027ef drm-tip: 2016y-11m-29d-20h-54m-07s UTC integration manifest > d61aedb drm/i915/lspcon: Enable AUX interrupts for resume time initialization > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3147/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-12-01 13:33 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-11-29 14:52 [PATCH] drm/i915/lspcon: Enable AUX interrupts for resume time initialization Imre Deak 2016-11-29 19:40 ` [PATCH v2] " Imre Deak 2016-11-29 19:55 ` Ville Syrjälä 2016-11-29 20:14 ` Imre Deak 2016-11-29 20:27 ` Ville Syrjälä 2016-11-29 20:41 ` Imre Deak 2016-11-29 21:00 ` Ville Syrjälä 2016-11-30 12:27 ` Imre Deak 2016-12-01 13:18 ` David Weinehall 2016-11-29 21:53 ` ✓ Fi.CI.BAT: success for drm/i915/lspcon: Enable AUX interrupts for resume time initialization (rev2) Patchwork 2016-12-01 13:33 ` Imre Deak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).