Re: [PATCH] drm/i915/vlv: Enable/disable VGA hotplugging properly

[Lyude Paul <cpaul@redhat.com>]

Looks like we might not need to worry about this patch anymore actually, looks
like this problem got fixed by accident by one of the other vlv fixes you
pushed. Now it's not always modesetting on hotplug when it was before though :(,
so I'll get to work on bisecting that.

On Thu, 2016-04-14 at 20:59 +0300, Ville Syrjälä wrote:
> On Tue, Mar 29, 2016 at 04:46:30PM -0400, Lyude wrote:
> > 
> > On Valleyview, VGA hotplugging is controlled through a seperate register
> > than everything else, VLV_ADPA, which must be explicitly set.
> > 
> > While VGA hotplugging worked(ish) before, it looks like that was mainly
> > because we'd unintentionally enable it in
> > valleyview_crt_detect_hotplug() when we did a force trigger. This
> > doesn't work reliably enough because whenever the display powerwell on
> > vlv gets disabled, the values set in VLV_ADPA get cleared and
> > consequently VGA hotplugging gets disabled. This causes bugs such as one
> > we found on an Intel NUC, where doing the following sequence of
> > hotplugs:
> > 
> > 	- Disconnect all monitors
> > 	- Connect VGA
> > 	- Disconnect VGA
> > 	- Connect HDMI
> > 
> > Would result in hotplugging getting disabled, due to the display
> > powerwells getting toggled in the process of connecting HDMI.
> > 
> > CC: stable@vger.kernel.org
> > Signed-off-by: Lyude <cpaul@redhat.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 5aa4239..60592a4 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -3611,6 +3611,7 @@ static void valleyview_display_irqs_install(struct
> > drm_i915_private *dev_priv)
> >  {
> >  	u32 pipestat_mask;
> >  	u32 iir_mask;
> > +	u32 adpa_reg;
> >  	enum pipe pipe;
> >  
> >  	pipestat_mask = PIPESTAT_INT_STATUS_MASK |
> > @@ -3627,6 +3628,12 @@ static void valleyview_display_irqs_install(struct
> > drm_i915_private *dev_priv)
> >  	for_each_pipe(dev_priv, pipe)
> >  		      i915_enable_pipestat(dev_priv, pipe, pipestat_mask);
> >  
> > +	if (IS_VALLEYVIEW(dev_priv)) {
> > +		adpa_reg = I915_READ(VLV_ADPA);
> > +		adpa_reg |= ADPA_CRT_HOTPLUG_ENABLE;
> > +		I915_WRITE(VLV_ADPA, adpa_reg);
> > +	}
> We might not want to enable that when there's no VGA connector.
> 
> Seems like we should just be calling intel_crt_reset() here. We
> definitely don't want to call the reset for hooks for all the other
> connectors so drm_mode_config_reset() is out. Also the connector
> locking might be problematic here, so I might suggest adjusting
> intel_crt_reset() to take an encoder instead of connector, and then
> we should be able to walk the encoder list without any troubles.
> 
> > 
> > +
> >  	iir_mask = I915_DISPLAY_PORT_INTERRUPT |
> >  		   I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> >  		   I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
> > @@ -3645,8 +3652,15 @@ static void valleyview_display_irqs_uninstall(struct
> > drm_i915_private *dev_priv)
> >  {
> >  	u32 pipestat_mask;
> >  	u32 iir_mask;
> > +	u32 adpa_reg;
> >  	enum pipe pipe;
> >  
> > +	if (IS_VALLEYVIEW(dev_priv)) {
> > +		adpa_reg = I915_READ(VLV_ADPA);
> > +		adpa_reg &= ~ADPA_CRT_HOTPLUG_ENABLE;
> > +		I915_WRITE(VLV_ADPA, adpa_reg);
> > +	}
> > +
> >  	iir_mask = I915_DISPLAY_PORT_INTERRUPT |
> >  		   I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
> >  		   I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
> > -- 
> > 2.5.5
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel