[PATCH] drm/i915/lspcon: Enable AUX interrupts for resume time initialization

[PATCH] drm/i915/lspcon: Enable AUX interrupts for resume time initialization

[Imre Deak]

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

[PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization

[Imre Deak]

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

Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization

[Ville Syrjälä]

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

Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization

[Imre Deak]

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

Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization

[Ville Syrjälä]

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

Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization

[Imre Deak]

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

Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization

[Ville Syrjälä]

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

✓ Fi.CI.BAT: success for drm/i915/lspcon: Enable AUX interrupts for resume time initialization (rev2)

[Patchwork]

== 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

Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization

[Imre Deak]

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

Re: [PATCH v2] drm/i915/lspcon: Enable AUX interrupts for resume time initialization

[David Weinehall]

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

Re: ✓ Fi.CI.BAT: success for drm/i915/lspcon: Enable AUX interrupts for resume time initialization (rev2)

[Imre Deak]

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