| Message ID | 1380231554-2678-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 26, 2013 at 02:39:14PM -0700, Jesse Barnes wrote: > This fixes resume on my test platform, since I think some DPIO bits need > recalibration. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=69166 > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index f52e6d4..320f729 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1359,6 +1359,17 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, > assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID); > } > > +static void intel_init_dpio(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (!IS_VALLEYVIEW(dev)) > + return; > + > + /* Reset in case DPIO was stuck across suspend/resume or boot */ > + I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_RESET); > +} > + > static void vlv_enable_pll(struct intel_crtc *crtc) > { > struct drm_device *dev = crtc->base.dev; > @@ -10279,6 +10290,8 @@ void intel_modeset_init_hw(struct drm_device *dev) > { > intel_prepare_ddi(dev); > > + intel_init_dpio(dev); > + > intel_init_clock_gating(dev); Can't you just put this into the clock_gate function? I'd like to cut down a bit on our general clutter in the setup code since tbh I've completely lost the overview of what goes where ... -Daniel > > mutex_lock(&dev->struct_mutex); > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 26 Sep 2013 23:51:52 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Sep 26, 2013 at 02:39:14PM -0700, Jesse Barnes wrote: > > This fixes resume on my test platform, since I think some DPIO bits need > > recalibration. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=69166 > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > --- > > drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index f52e6d4..320f729 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -1359,6 +1359,17 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, > > assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID); > > } > > > > +static void intel_init_dpio(struct drm_device *dev) > > +{ > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + > > + if (!IS_VALLEYVIEW(dev)) > > + return; > > + > > + /* Reset in case DPIO was stuck across suspend/resume or boot */ > > + I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_RESET); > > +} > > + > > static void vlv_enable_pll(struct intel_crtc *crtc) > > { > > struct drm_device *dev = crtc->base.dev; > > @@ -10279,6 +10290,8 @@ void intel_modeset_init_hw(struct drm_device *dev) > > { > > intel_prepare_ddi(dev); > > > > + intel_init_dpio(dev); > > + > > intel_init_clock_gating(dev); > > Can't you just put this into the clock_gate function? I'd like to cut down > a bit on our general clutter in the setup code since tbh I've completely > lost the overview of what goes where ... Seemed more appropriate for modeset code, but of course it could go anywhere.
On 09/26 14:58, Jesse Barnes wrote: > On Thu, 26 Sep 2013 23:51:52 +0200 > Daniel Vetter <daniel@ffwll.ch> wrote: > > > On Thu, Sep 26, 2013 at 02:39:14PM -0700, Jesse Barnes wrote: > > > This fixes resume on my test platform, since I think some DPIO bits need > > > recalibration. > > > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=69166 > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++ > > > 1 file changed, 13 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index f52e6d4..320f729 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -1359,6 +1359,17 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, > > > assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID); > > > } > > > > > > +static void intel_init_dpio(struct drm_device *dev) > > > +{ > > > + struct drm_i915_private *dev_priv = dev->dev_private; > > > + > > > + if (!IS_VALLEYVIEW(dev)) > > > + return; > > > + > > > + /* Reset in case DPIO was stuck across suspend/resume or boot */ > > > + I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_RESET); > > > +} > > > + > > > static void vlv_enable_pll(struct intel_crtc *crtc) > > > { > > > struct drm_device *dev = crtc->base.dev; > > > @@ -10279,6 +10290,8 @@ void intel_modeset_init_hw(struct drm_device *dev) > > > { > > > intel_prepare_ddi(dev); > > > > > > + intel_init_dpio(dev); > > > + > > > intel_init_clock_gating(dev); > > > > Can't you just put this into the clock_gate function? I'd like to cut down > > a bit on our general clutter in the setup code since tbh I've completely > > lost the overview of what goes where ... > > Seemed more appropriate for modeset code, but of course it could go > anywhere. > The patch below is doing the same thing, but reset in intel_uncore_sanitize. drm/i915: Send a DPIO cmnreset during driver load or system resume. Same as Jesse patch, it solves the resume issue also. If usnig Jesse patch, I will remove the DPIO reset in intel_uncore_sanitize. > -- > Jesse Barnes, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Sep 26, 2013 at 02:39:14PM -0700, Jesse Barnes wrote: > This fixes resume on my test platform, since I think some DPIO bits need > recalibration. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=69166 > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index f52e6d4..320f729 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1359,6 +1359,17 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, > assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID); > } > > +static void intel_init_dpio(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (!IS_VALLEYVIEW(dev)) > + return; > + > + /* Reset in case DPIO was stuck across suspend/resume or boot */ > + I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_RESET); This will deassert the common lane reset, so the comment is confusing, as is the name we have given this bit. > +} > + > static void vlv_enable_pll(struct intel_crtc *crtc) > { > struct drm_device *dev = crtc->base.dev; > @@ -10279,6 +10290,8 @@ void intel_modeset_init_hw(struct drm_device *dev) > { > intel_prepare_ddi(dev); > > + intel_init_dpio(dev); > + > intel_init_clock_gating(dev); > > mutex_lock(&dev->struct_mutex); > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Sep 27, 2013 at 12:34:31PM +0300, Ville Syrjälä wrote: > On Thu, Sep 26, 2013 at 02:39:14PM -0700, Jesse Barnes wrote: > > This fixes resume on my test platform, since I think some DPIO bits need > > recalibration. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=69166 > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > --- > > drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index f52e6d4..320f729 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -1359,6 +1359,17 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, > > assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID); > > } > > > > +static void intel_init_dpio(struct drm_device *dev) > > +{ > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + > > + if (!IS_VALLEYVIEW(dev)) > > + return; > > + > > + /* Reset in case DPIO was stuck across suspend/resume or boot */ > > + I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_RESET); > > This will deassert the common lane reset, so the comment is confusing, > as is the name we have given this bit. And the commit subject and text is also confusing in the same way. And we should do this after setting up the CRI clock. > > > +} > > + > > static void vlv_enable_pll(struct intel_crtc *crtc) > > { > > struct drm_device *dev = crtc->base.dev; > > @@ -10279,6 +10290,8 @@ void intel_modeset_init_hw(struct drm_device *dev) > > { > > intel_prepare_ddi(dev); > > > > + intel_init_dpio(dev); > > + > > intel_init_clock_gating(dev); > > > > mutex_lock(&dev->struct_mutex); > > -- > > 1.8.3.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 3 Oct 2013 19:38:26 +0300 Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Fri, Sep 27, 2013 at 12:34:31PM +0300, Ville Syrjälä wrote: > > On Thu, Sep 26, 2013 at 02:39:14PM -0700, Jesse Barnes wrote: > > > This fixes resume on my test platform, since I think some DPIO bits need > > > recalibration. > > > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=69166 > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++ > > > 1 file changed, 13 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index f52e6d4..320f729 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -1359,6 +1359,17 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, > > > assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID); > > > } > > > > > > +static void intel_init_dpio(struct drm_device *dev) > > > +{ > > > + struct drm_i915_private *dev_priv = dev->dev_private; > > > + > > > + if (!IS_VALLEYVIEW(dev)) > > > + return; > > > + > > > + /* Reset in case DPIO was stuck across suspend/resume or boot */ > > > + I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_RESET); > > > > This will deassert the common lane reset, so the comment is confusing, > > as is the name we have given this bit. > > And the commit subject and text is also confusing in the same way. > > And we should do this after setting up the CRI clock. So I'm still confused by this. The bit is called "CMNRST", which doesn't say anything to me about deasserting common lane reset, unless you mean setting it to 0, which the docs imply. But here we're setting it to on 1. But I guess you're talking about something than the other gunit register HAS? At any rate, it's not confusing to me now, though it may become so if I try to read a few more docs...
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f52e6d4..320f729 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1359,6 +1359,17 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID); } +static void intel_init_dpio(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + + if (!IS_VALLEYVIEW(dev)) + return; + + /* Reset in case DPIO was stuck across suspend/resume or boot */ + I915_WRITE(DPIO_CTL, I915_READ(DPIO_CTL) | DPIO_RESET); +} + static void vlv_enable_pll(struct intel_crtc *crtc) { struct drm_device *dev = crtc->base.dev; @@ -10279,6 +10290,8 @@ void intel_modeset_init_hw(struct drm_device *dev) { intel_prepare_ddi(dev); + intel_init_dpio(dev); + intel_init_clock_gating(dev); mutex_lock(&dev->struct_mutex);
This fixes resume on my test platform, since I think some DPIO bits need recalibration. References: https://bugs.freedesktop.org/show_bug.cgi?id=69166 Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)