From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lyude Paul Subject: Re: [PATCH] drm/i915/vlv: Enable/disable VGA hotplugging properly Date: Fri, 15 Apr 2016 09:47:51 -0400 Message-ID: <1460728071.26502.1.camel@redhat.com> References: <1459284390-14485-1-git-send-email-cpaul@redhat.com> <20160414175922.GT4329@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <20160414175922.GT4329@intel.com> Sender: stable-owner@vger.kernel.org To: Ville =?ISO-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org, "open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list" , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org 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 y= ou pushed. Now it's not always modesetting on hotplug when it was before t= hough :(, so I'll get to work on bisecting that. On Thu, 2016-04-14 at 20:59 +0300, Ville Syrj=C3=A4l=C3=A4 wrote: > On Tue, Mar 29, 2016 at 04:46:30PM -0400, Lyude wrote: > >=20 > > On Valleyview, VGA hotplugging is controlled through a seperate reg= ister > > than everything else, VLV_ADPA, which must be explicitly set. > >=20 > > While VGA hotplugging worked(ish) before, it looks like that was ma= inly > > 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 a= s one > > we found on an Intel NUC, where doing the following sequence of > > hotplugs: > >=20 > > - Disconnect all monitors > > - Connect VGA > > - Disconnect VGA > > - Connect HDMI > >=20 > > Would result in hotplugging getting disabled, due to the display > > powerwells getting toggled in the process of connecting HDMI. > >=20 > > CC: stable@vger.kernel.org > > Signed-off-by: Lyude > > --- > > =C2=A0drivers/gpu/drm/i915/i915_irq.c | 14 ++++++++++++++ > > =C2=A01 file changed, 14 insertions(+) > >=20 > > 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(s= truct > > drm_i915_private *dev_priv) > > =C2=A0{ > > =C2=A0 u32 pipestat_mask; > > =C2=A0 u32 iir_mask; > > + u32 adpa_reg; > > =C2=A0 enum pipe pipe; > > =C2=A0 > > =C2=A0 pipestat_mask =3D PIPESTAT_INT_STATUS_MASK | > > @@ -3627,6 +3628,12 @@ static void valleyview_display_irqs_install(= struct > > drm_i915_private *dev_priv) > > =C2=A0 for_each_pipe(dev_priv, pipe) > > =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0i915_enable_pipestat(de= v_priv, pipe, pipestat_mask); > > =C2=A0 > > + if (IS_VALLEYVIEW(dev_priv)) { > > + adpa_reg =3D I915_READ(VLV_ADPA); > > + adpa_reg |=3D ADPA_CRT_HOTPLUG_ENABLE; > > + I915_WRITE(VLV_ADPA, adpa_reg); > > + } > We might not want to enable that when there's no VGA connector. >=20 > 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. >=20 > >=20 > > + > > =C2=A0 iir_mask =3D I915_DISPLAY_PORT_INTERRUPT | > > =C2=A0 =C2=A0=C2=A0=C2=A0I915_DISPLAY_PIPE_A_EVENT_INTERRUPT | > > =C2=A0 =C2=A0=C2=A0=C2=A0I915_DISPLAY_PIPE_B_EVENT_INTERRUPT; > > @@ -3645,8 +3652,15 @@ static void valleyview_display_irqs_uninstal= l(struct > > drm_i915_private *dev_priv) > > =C2=A0{ > > =C2=A0 u32 pipestat_mask; > > =C2=A0 u32 iir_mask; > > + u32 adpa_reg; > > =C2=A0 enum pipe pipe; > > =C2=A0 > > + if (IS_VALLEYVIEW(dev_priv)) { > > + adpa_reg =3D I915_READ(VLV_ADPA); > > + adpa_reg &=3D ~ADPA_CRT_HOTPLUG_ENABLE; > > + I915_WRITE(VLV_ADPA, adpa_reg); > > + } > > + > > =C2=A0 iir_mask =3D I915_DISPLAY_PORT_INTERRUPT | > > =C2=A0 =C2=A0=C2=A0=C2=A0I915_DISPLAY_PIPE_A_EVENT_INTERRUPT | > > =C2=A0 =C2=A0=C2=A0=C2=A0I915_DISPLAY_PIPE_B_EVENT_INTERRUPT; > > --=C2=A0 > > 2.5.5 > >=20 > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel