On Thu, Nov 08, 2018 at 04:23:48PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-11-08 14:36:35)
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > IBX has a documented workaround which states that when we disable the
> > port we must change its transcoder select to A, otherwise it will
> > prevent the other port (DP vs. HDMI/SDVO) from using transcoder A.
> > We implement the workaround during encoder disable, but looks like
> > some BIOSen leave transcoder B selected even when the port wasn't
> > actually enabled by the BIOS. That will trip up our asserts
> > that attempt to make sure we never forget this w/a.
> >
> > Sanitize the transcoder select to A for all disabled PCH
> > DP/HDMI/SDVO ports. We assume that the port was never enabled
> > by the BIOS on transcoder B, because if it had we'd actually have
> > to toggle the port on and back off to properly switch it back to
> > transcoder A. That would cause some display flicker if transcoder A
> > is already enabled on some other port, so it's better not to do it
> > unless absolutely necessary. Since we have no indication that the
> > transcoder select is misbehaving on the affected machines we can
> > assume the port was never actually enabled by the BIOS.
> >
> > This cures warning like this during driver load:
> > IBX PCH DP C still using transcoder B
> > WARNING: CPU: 2 PID: 172 at drivers/gpu/drm/i915/intel_display.c:1279 assert_pch_dp_disabled+0x9e/0xb0 [i915]
> >
> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 61 ++++++++++++++++++++++++++++
> > 1 file changed, 61 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ae6d58dbf1ed..71b7bff85e52 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -15653,6 +15653,64 @@ static void intel_early_display_was(struct drm_i915_private *dev_priv)
> > }
> > }
> >
> > +static void ibx_sanitize_pch_hdmi_port(struct drm_i915_private *dev_priv,
> > + enum port port, i915_reg_t hdmi_reg)
> > +{
> > + u32 val = I915_READ(hdmi_reg);
> > +
> > + if (val & SDVO_ENABLE ||
> > + (val & SDVO_PIPE_SEL_MASK) == SDVO_PIPE_SEL(PIPE_A))
> > + return;
>
> Hmm, the w/a is also applied in intel_sdvo.c. Do we need to worry about
> those as well?
>
> Oh my, it's perfectly obvious SDVO is aliased to HDMI on ibx.
>
> Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>
> I would have s/hdmi/svdo/ here so that the labelling was all consistent
> and added a reminder in the comment below that hdmi is aliased to sdvo.
I wanted to be consistent with assert_pch_hdmi_disabled(). I think only
port B can be SDVO actually, or something along those lines. Not sure
we want to rename all of this for that reason. But a comment seems like
a very good idea at least. I'll add one.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
