2014/1/7 Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>: > According to Art, we don't have a way to read back the state reliably at > runtime, through the control reg or the mailbox, at least not without risking > disabling it again. So drop the readout and checking on BDW. > > v2: drop TODO comment (Paulo) > move POSTING_READ of control reg under HSW branch in disable (Paulo) > always report IPS as enabled on BDW (Paulo) > > References: https://bugs.freedesktop.org/show_bug.cgi?id=71906 > Signed-off-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++-------- > 2 files changed, 12 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 7252f30..75a489e 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1361,7 +1361,7 @@ static int i915_ips_status(struct seq_file *m, void *unused) > return 0; > } > > - if (I915_READ(IPS_CTL) & IPS_ENABLE) > + if (IS_BROADWELL(dev) || I915_READ(IPS_CTL) & IPS_ENABLE) <bikeshedding> I'm not really sure if we need to print "enabled" here... It's more like "enabled and disabled". </bikeshedding> Anyway, your patch seems to fix the bug, so: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > seq_puts(m, "enabled\n"); > else > seq_puts(m, "disabled\n"); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 4d1357a..74137d5 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3431,9 +3431,8 @@ void hsw_enable_ips(struct intel_crtc *crtc) > mutex_unlock(&dev_priv->rps.hw_lock); > /* Quoting Art Runyan: "its not safe to expect any particular > * value in IPS_CTL bit 31 after enabling IPS through the > - * mailbox." Therefore we need to defer waiting on the state > - * change. > - * TODO: need to fix this for state checker > + * mailbox." Moreover, the mailbox may return a bogus state, > + * so we need to just enable it and continue on. > */ > } else { > I915_WRITE(IPS_CTL, IPS_ENABLE); > @@ -3460,9 +3459,10 @@ void hsw_disable_ips(struct intel_crtc *crtc) > mutex_lock(&dev_priv->rps.hw_lock); > WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0)); > mutex_unlock(&dev_priv->rps.hw_lock); > - } else > + } else { > I915_WRITE(IPS_CTL, 0); > - POSTING_READ(IPS_CTL); > + POSTING_READ(IPS_CTL); > + } > > /* We need to wait for a vblank before we can disable the plane. */ > intel_wait_for_vblank(dev, crtc->pipe); > @@ -7004,8 +7004,9 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, > if (intel_display_power_enabled(dev, pfit_domain)) > ironlake_get_pfit_config(crtc, pipe_config); > > - pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) && > - (I915_READ(IPS_CTL) & IPS_ENABLE); > + if (IS_HASWELL(dev)) > + pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) && > + (I915_READ(IPS_CTL) & IPS_ENABLE); > > pipe_config->pixel_multiplier = 1; > > @@ -9335,7 +9336,9 @@ intel_pipe_config_compare(struct drm_device *dev, > PIPE_CONF_CHECK_I(pch_pfit.size); > } > > - PIPE_CONF_CHECK_I(ips_enabled); > + /* BDW+ don't expose a synchronous way to read the state */ > + if (IS_HASWELL(dev)) > + PIPE_CONF_CHECK_I(ips_enabled); > > PIPE_CONF_CHECK_I(double_wide); > > -- > 1.8.3.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
