[Intel-gfx] [PATCH v2 07/10] drm/i915: Disable FDI after the CRT port on LPT-H

[Ville Syrjälä]

On Mon, Dec 07, 2015 at 08:57:59PM +0200, Ville Syrjälä wrote:
> On Mon, Dec 07, 2015 at 03:51:06PM -0200, Paulo Zanoni wrote:
> > 2015-12-04 18:20 GMT-02:00  <ville.syrjala at linux.intel.com>:
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > >
> > > Bspec modeset sequence tells us to disable the PCH transcoder and
> > > FDI after the CRT port on LPT-H, so let's do that. And the CRT port
> > > should be disabled after the pipe, as we do on other PCH platforms
> > > too since
> > > commit 1ea56e269e13 ("drm/i915: Disable CRT port after pipe on PCH platforms")
> > >
> > > commit 00490c22b1b5 ("drm/i915: Consider SPLL as another shared pll, v2.")
> > > moved the SPLL disaable from the .post_disable() hook to some upper
> > 
> > disaable
> > 
> > > laeve code, so we can just move the CRT port disabling into the
> > 
> > laeve
> > 
> > > .post_disable() hook. If we still had the non-shared SPLL, it would have
> > > needed to be moved into the .post_pll_disable() hook.
> > >
> > > v2: Actually move the CRT port disable to the .post_disable() hook,
> > >     and amend the commit message with more details (Paulo)
> > 
> > Since I seem to recall that the DDI disable sequence for CRT was
> > correct at some point in a distant past, I suppose your commit is a
> > regression fix, and maybe we'd like a backportable version. Maybe we
> > could also provide explicit information on the first bad commit.
> 
> Well, even the spec has changed since the early days, at least w.r.t.
> the FDI RX disable, so not sure if something else has changed in there
> as well. I didn't go digging through history to see if we accidentally
> changed the order of some steps at some point.

I tried to look through the hisrtory a bit, but didn't spot any place
where any significant reordering took place (ie. we always seemed to
disable the port before the pipe). So I'm leaning towards the "spec
got changed" theory here, or perhaps the "copy paste" theory since
we did get this order wrong for all earlier PCH platforms as well.

> 
> > 
> > Anyway, now it looks correct:
> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > 
> > >
> > > Cc: Paulo Zanoni <przanoni at gmail.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_crt.c     |  2 +-
> > >  drivers/gpu/drm/i915/intel_display.c | 11 +++++------
> > >  2 files changed, 6 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> > > index 12008af797bd..cef359958c73 100644
> > > --- a/drivers/gpu/drm/i915/intel_crt.c
> > > +++ b/drivers/gpu/drm/i915/intel_crt.c
> > > @@ -844,7 +844,7 @@ void intel_crt_init(struct drm_device *dev)
> > >         crt->adpa_reg = adpa_reg;
> > >
> > >         crt->base.compute_config = intel_crt_compute_config;
> > > -       if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev)) {
> > > +       if (HAS_PCH_SPLIT(dev)) {
> > >                 crt->base.disable = pch_disable_crt;
> > >                 crt->base.post_disable = pch_post_disable_crt;
> > >         } else {
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 3391ab0ca752..42ed799390e5 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5166,18 +5166,17 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> > >         if (!intel_crtc->config->has_dsi_encoder)
> > >                 intel_ddi_disable_pipe_clock(intel_crtc);
> > >
> > > -       if (intel_crtc->config->has_pch_encoder) {
> > > -               lpt_disable_pch_transcoder(dev_priv);
> > > -               intel_ddi_fdi_disable(crtc);
> > > -       }
> > > -
> > >         for_each_encoder_on_crtc(dev, crtc, encoder)
> > >                 if (encoder->post_disable)
> > >                         encoder->post_disable(encoder);
> > >
> > > -       if (intel_crtc->config->has_pch_encoder)
> > > +       if (intel_crtc->config->has_pch_encoder) {
> > > +               lpt_disable_pch_transcoder(dev_priv);
> > > +               intel_ddi_fdi_disable(crtc);
> > > +
> > >                 intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
> > >                                                       true);
> > > +       }
> > >
> > >         intel_fbc_disable_crtc(intel_crtc);
> > >  }
> > > --
> > > 2.4.10
> > >
> > 
> > 
> > 
> > -- 
> > Paulo Zanoni
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC