| Message ID | 1376105574-14630-1-git-send-email-marcheu@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Aug 09, 2013 at 08:32:54PM -0700, Stéphane Marchesin wrote: > This is a partial revert of b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 > (drm/i915: load boot context at driver init time) > > This bit breaks hardware video decode for me after resume. > > Signed-off-by: Stéphane Marchesin <marcheu@chromium.org> > --- > drivers/gpu/drm/i915/intel_pm.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index f895d15..ffa4ab2 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4614,10 +4614,6 @@ static void gen6_init_clock_gating(struct drm_device *dev) > ILK_DPARBUNIT_CLOCK_GATE_ENABLE | > ILK_DPFDUNIT_CLOCK_GATE_ENABLE); > > - /* WaMbcDriverBootEnable:snb */ > - I915_WRITE(GEN6_MBCTL, I915_READ(GEN6_MBCTL) | > - GEN6_MBCTL_ENABLE_BOOT_FETCH); > - > g4x_disable_trickle_feed(dev); > > /* The default value should be 0x200 according to docs, but the two I was looking at this a bit with Stéphane. One thing we both see is that the bit isn't sticking as it should. Clearly doing the write is having an effect, but something is seriously wrong. Writing the bit manually with IGT does make the bit stick. Stéphane, could you try to write the bit with IGT before and after resume, and see if it helps? Adding Jesse to the CC, since he wrote the original patch, maybe he has an idea why it doesn't stick.
On Fri, Aug 9, 2013 at 9:55 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > On Fri, Aug 09, 2013 at 08:32:54PM -0700, Stéphane Marchesin wrote: >> This is a partial revert of b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 >> (drm/i915: load boot context at driver init time) >> >> This bit breaks hardware video decode for me after resume. >> >> Signed-off-by: Stéphane Marchesin <marcheu@chromium.org> >> --- >> drivers/gpu/drm/i915/intel_pm.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index f895d15..ffa4ab2 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -4614,10 +4614,6 @@ static void gen6_init_clock_gating(struct drm_device *dev) >> ILK_DPARBUNIT_CLOCK_GATE_ENABLE | >> ILK_DPFDUNIT_CLOCK_GATE_ENABLE); >> >> - /* WaMbcDriverBootEnable:snb */ >> - I915_WRITE(GEN6_MBCTL, I915_READ(GEN6_MBCTL) | >> - GEN6_MBCTL_ENABLE_BOOT_FETCH); >> - >> g4x_disable_trickle_feed(dev); >> >> /* The default value should be 0x200 according to docs, but the two > > I was looking at this a bit with Stéphane. One thing we both see is that > the bit isn't sticking as it should. Clearly doing the write is having > an effect, but something is seriously wrong. Writing the bit manually > with IGT does make the bit stick. > > Stéphane, could you try to write the bit with IGT before and after > resume, and see if it helps? It doesn't seem to stick, and so seems to have no effect. The confusing thing is that video decode works fine before suspend, even though that reg is 0. And after resume, it is broken, and that reg is still 0. So something else is going on. Maybe this reg is write-once-ever? Stéphane
On Mon, Aug 12, 2013 at 10:33:37AM -0700, Stéphane Marchesin wrote: > On Fri, Aug 9, 2013 at 9:55 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > > On Fri, Aug 09, 2013 at 08:32:54PM -0700, Stéphane Marchesin wrote: > >> This is a partial revert of b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 > >> (drm/i915: load boot context at driver init time) > >> > >> This bit breaks hardware video decode for me after resume. > >> > >> Signed-off-by: Stéphane Marchesin <marcheu@chromium.org> > >> --- > >> drivers/gpu/drm/i915/intel_pm.c | 4 ---- > >> 1 file changed, 4 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > >> index f895d15..ffa4ab2 100644 > >> --- a/drivers/gpu/drm/i915/intel_pm.c > >> +++ b/drivers/gpu/drm/i915/intel_pm.c > >> @@ -4614,10 +4614,6 @@ static void gen6_init_clock_gating(struct drm_device *dev) > >> ILK_DPARBUNIT_CLOCK_GATE_ENABLE | > >> ILK_DPFDUNIT_CLOCK_GATE_ENABLE); > >> > >> - /* WaMbcDriverBootEnable:snb */ > >> - I915_WRITE(GEN6_MBCTL, I915_READ(GEN6_MBCTL) | > >> - GEN6_MBCTL_ENABLE_BOOT_FETCH); > >> - > >> g4x_disable_trickle_feed(dev); > >> > >> /* The default value should be 0x200 according to docs, but the two > > > > I was looking at this a bit with Stéphane. One thing we both see is that > > the bit isn't sticking as it should. Clearly doing the write is having > > an effect, but something is seriously wrong. Writing the bit manually > > with IGT does make the bit stick. > > > > Stéphane, could you try to write the bit with IGT before and after > > resume, and see if it helps? > > It doesn't seem to stick, and so seems to have no effect. > > The confusing thing is that video decode works fine before suspend, > even though that reg is 0. And after resume, it is broken, and that > reg is still 0. So something else is going on. Maybe this reg is > write-once-ever? BSpec says "This Bit is cleared by the Hardware once the Boot fetch is complete."
On Tue, Aug 13, 2013 at 1:11 AM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Mon, Aug 12, 2013 at 10:33:37AM -0700, Stéphane Marchesin wrote: >> On Fri, Aug 9, 2013 at 9:55 PM, Ben Widawsky <ben@bwidawsk.net> wrote: >> > On Fri, Aug 09, 2013 at 08:32:54PM -0700, Stéphane Marchesin wrote: >> >> This is a partial revert of b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 >> >> (drm/i915: load boot context at driver init time) >> >> >> >> This bit breaks hardware video decode for me after resume. >> >> >> >> Signed-off-by: Stéphane Marchesin <marcheu@chromium.org> >> >> --- >> >> drivers/gpu/drm/i915/intel_pm.c | 4 ---- >> >> 1 file changed, 4 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> >> index f895d15..ffa4ab2 100644 >> >> --- a/drivers/gpu/drm/i915/intel_pm.c >> >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> >> @@ -4614,10 +4614,6 @@ static void gen6_init_clock_gating(struct drm_device *dev) >> >> ILK_DPARBUNIT_CLOCK_GATE_ENABLE | >> >> ILK_DPFDUNIT_CLOCK_GATE_ENABLE); >> >> >> >> - /* WaMbcDriverBootEnable:snb */ >> >> - I915_WRITE(GEN6_MBCTL, I915_READ(GEN6_MBCTL) | >> >> - GEN6_MBCTL_ENABLE_BOOT_FETCH); >> >> - >> >> g4x_disable_trickle_feed(dev); >> >> >> >> /* The default value should be 0x200 according to docs, but the two >> > >> > I was looking at this a bit with Stéphane. One thing we both see is that >> > the bit isn't sticking as it should. Clearly doing the write is having >> > an effect, but something is seriously wrong. Writing the bit manually >> > with IGT does make the bit stick. >> > >> > Stéphane, could you try to write the bit with IGT before and after >> > resume, and see if it helps? >> >> It doesn't seem to stick, and so seems to have no effect. >> >> The confusing thing is that video decode works fine before suspend, >> even though that reg is 0. And after resume, it is broken, and that >> reg is still 0. So something else is going on. Maybe this reg is >> write-once-ever? > > BSpec says "This Bit is cleared by the Hardware once the Boot fetch is > complete." So it seems like the boot fetch doesn't work correctly, but still clears the bit? Stéphane
On Mon, 12 Aug 2013 10:33:37 -0700 Stéphane Marchesin <marcheu@chromium.org> wrote: > On Fri, Aug 9, 2013 at 9:55 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > > On Fri, Aug 09, 2013 at 08:32:54PM -0700, Stéphane Marchesin wrote: > >> This is a partial revert of b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 > >> (drm/i915: load boot context at driver init time) > >> > >> This bit breaks hardware video decode for me after resume. > >> > >> Signed-off-by: Stéphane Marchesin <marcheu@chromium.org> > >> --- > >> drivers/gpu/drm/i915/intel_pm.c | 4 ---- > >> 1 file changed, 4 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > >> index f895d15..ffa4ab2 100644 > >> --- a/drivers/gpu/drm/i915/intel_pm.c > >> +++ b/drivers/gpu/drm/i915/intel_pm.c > >> @@ -4614,10 +4614,6 @@ static void gen6_init_clock_gating(struct drm_device *dev) > >> ILK_DPARBUNIT_CLOCK_GATE_ENABLE | > >> ILK_DPFDUNIT_CLOCK_GATE_ENABLE); > >> > >> - /* WaMbcDriverBootEnable:snb */ > >> - I915_WRITE(GEN6_MBCTL, I915_READ(GEN6_MBCTL) | > >> - GEN6_MBCTL_ENABLE_BOOT_FETCH); > >> - > >> g4x_disable_trickle_feed(dev); > >> > >> /* The default value should be 0x200 according to docs, but the two > > > > I was looking at this a bit with Stéphane. One thing we both see is that > > the bit isn't sticking as it should. Clearly doing the write is having > > an effect, but something is seriously wrong. Writing the bit manually > > with IGT does make the bit stick. > > > > Stéphane, could you try to write the bit with IGT before and after > > resume, and see if it helps? > > It doesn't seem to stick, and so seems to have no effect. > > The confusing thing is that video decode works fine before suspend, > even though that reg is 0. And after resume, it is broken, and that > reg is still 0. So something else is going on. Maybe this reg is > write-once-ever? Apparently it is. And if the BIOS writes that bit, we're not supposed to write it either. However, since it's WO, we can't easily tell if it's been written! I'm trying to get some more info on this, but at the very least it seems like we might need to avoid writing it on resume...
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index f895d15..ffa4ab2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4614,10 +4614,6 @@ static void gen6_init_clock_gating(struct drm_device *dev) ILK_DPARBUNIT_CLOCK_GATE_ENABLE | ILK_DPFDUNIT_CLOCK_GATE_ENABLE); - /* WaMbcDriverBootEnable:snb */ - I915_WRITE(GEN6_MBCTL, I915_READ(GEN6_MBCTL) | - GEN6_MBCTL_ENABLE_BOOT_FETCH); - g4x_disable_trickle_feed(dev); /* The default value should be 0x200 according to docs, but the two
This is a partial revert of b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 (drm/i915: load boot context at driver init time) This bit breaks hardware video decode for me after resume. Signed-off-by: Stéphane Marchesin <marcheu@chromium.org> --- drivers/gpu/drm/i915/intel_pm.c | 4 ---- 1 file changed, 4 deletions(-)