On Fri, Nov 10, 2017 at 12:24:24PM -0800, Rodrigo Vivi wrote:
> On Fri, Nov 10, 2017 at 08:13:44PM +0000, Ville Syrjälä wrote:
> > On Thu, Nov 09, 2017 at 02:26:32PM -0800, Rodrigo Vivi wrote:
> > > Display is not sending a PMRsp when a PMReq is received
> > > at the same time that all planes are turned off.
> > > State machine in the dcprunit is stuck in the WAIT4DONE
> > > state which means that there is no fill_done.
> > >
> > > WA: disable arbiter clock gating, set bit [27] of 0x46530
> > >
> > > v2: As Ville pointed out, based on the description the issue
> > > can happen when disabling the planes, similar to
> > > WaRsPkgCStateDisplayPMReq:hsw
> > > Also description of the issue was updated on commit
> > > message to make it more clear that we need this
> > > earlier.
> >
> > I guess we don't know exactly if this has the same ordering requirements
> > as WaRsPkgCStateDisplayPMReq:hsw. But playing it safe can't hurt I
> > suppose.
> >
> > >
> > > Cc: Radhakrishna Sripada <radhakrishna.sripada@xxxxxxxxx>
> > > Cc: Imre Deak <imre.deak@xxxxxxxxx>
> > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > > drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++++++---------
> > > 2 files changed, 16 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 6ef33422f762..fc8c5f8260f6 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -3819,6 +3819,7 @@ enum {
> > > * GEN9 clock gating regs
> > > */
> > > #define GEN9_CLKGATE_DIS_0 _MMIO(0x46530)
> > > +#define DARBF_GATING_DIS (1 << 27)
> > > #define PWM2_GATING_DIS (1 << 14)
> > > #define PWM1_GATING_DIS (1 << 13)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 84817ccc5305..a038610b66cc 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -15080,6 +15080,20 @@ get_encoder_power_domains(struct drm_i915_private *dev_priv)
> > > }
> > > }
> > >
> > > +/* System hang if this isn't done before disabling all planes! */
> > > +static void intel_early_display_was(struct drm_i915_private *dev_priv)
> > > +{
> > > + /* Display WA #1185 WaDisableDARBFClkGating:cnl,glk */
> > > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> >
> > Bspec says GLK isn't affected.
>
> Bspec refers to GLK as Gen10 display...
> so GEN10:All is also GLK...
> but if you look 2 columns ahead you see this:
> GEN:BUG:1947072 [CNL, GLK]
Ah. And the hsd does indeed list glk as well.
>
> >
> > > + I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
> > > + DARBF_GATING_DIS);
> > > +
> > > + /* WaRsPkgCStateDisplayPMReq:hsw */
> >
> > Please keep the comment about the hsw system hangs here. The next guy to
> > touch this code is probably not going to be aware of it, or may simply
> > have forgotten it already.
>
> makes sense...
> should I keep the one in the function comment as well? and duplicate
> the comment on the new one or since we are not that sure we just leave
> the comment close to the hsw one?
I'd just keep it on the hsw one since we know that's how it works
on hsw. Having it on the cnl/glk one would be more speculation at
this point.
>
> >
> > With that those addresses this is
> > Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> thanks
>
> >
> > > + if (IS_HASWELL(dev_priv))
> > > + I915_WRITE(CHICKEN_PAR1_1,
> > > + I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
> > > +}
> > > +
> > > /* Scan out the current hw modeset state,
> > > * and sanitizes it to the current state
> > > */
> > > @@ -15093,15 +15107,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
> > > struct intel_encoder *encoder;
> > > int i;
> > >
> > > - if (IS_HASWELL(dev_priv)) {
> > > - /*
> > > - * WaRsPkgCStateDisplayPMReq:hsw
> > > - * System hang if this isn't done before disabling all planes!
> > > - */
> > > - I915_WRITE(CHICKEN_PAR1_1,
> > > - I915_READ(CHICKEN_PAR1_1) | FORCE_ARB_IDLE_PLANES);
> > > - }
> > > -
> > > + intel_early_display_was(dev_priv);
> > > intel_modeset_readout_hw_state(dev);
> > >
> > > /* HW state is read out, now we need to sanitize this mess. */
> > > --
> > > 2.13.6
> >
> > --
> > Ville Syrjälä
> > Intel OTC
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
