Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-23 Thread Herton Ronaldo Krzesinski
On Thu, Aug 23, 2012 at 10:10:18AM +0200, Daniel Vetter wrote:
> On Thu, Aug 23, 2012 at 12:44 AM, Herton Ronaldo Krzesinski
>  wrote:
> > Really sorry about this, but it was a hardware+bios setting problem
> > here. I finished investigating what was happening here, and is related to
> > the DRAM installed on this machine. The memory installed is DDR3-1333,
> > and on bios it was configured as that, but turns out the chipset (G41)
> > only supports up to DDR3-1066 (as on specs, although the motherboard
> > lists the 1333 as supported, but as "OC", and allows configuring this
> > way automatically or manually).
> >
> > After setting manually back to 1066, I stopped seeing all the weird
> > behaviour, like the writes to the start address don't trigger anymore
> > setting on the head offset. I'm sorry about that, and really it's a
> > brown paper bag time for me :(. The machine worked fine and passed
> > through memtest and never gave any problems, this was the first
> > time (seems only the video device didn't like the memory setting, and
> > the code change probably changed timing in the right way to pop up this
> > now, also never had graphics corruption or other noticeable problems).
> > I retested kernels that gave problems and they run fine now.
> 
> Well, at least we could clear this up.
> 
> Greg, please pick up the following patches for 3.0 (if you haven't already):
> 
> f01db988ef6f6c70a6cc36ee71e4a98a68901229 and
> 0d8957c8a90bbb5d34fab9a304459448a5131e06
> 
> Note that all kernels that need f01db backported also need
> b7884eb45ec98c0d34c7f49005ae9d4b4b4e38f6 (to fix a regression
> introduce by the former).

b7884eb45ec98c0d34c7f49005ae9d4b4b4e38f6 needs some backporting for 3.0,
I did it in advance, build tested and installed here on a sandybridge
based laptop on x86_64:

>From 3adf724d83de569510f2df0b91a097de92372970 Mon Sep 17 00:00:00 2001
From: Daniel Vetter 
Date: Mon, 4 Jun 2012 11:18:15 +0200
Subject: [PATCH 2/2] drm/i915: hold forcewake around ring hw init

commit b7884eb45ec98c0d34c7f49005ae9d4b4b4e38f6 upstream.

Empirical evidence suggests that we need to: On at least one ivb
machine when running the hangman i-g-t test, the rings don't properly
initialize properly - the RING_START registers seems to be stuck at
all zeros.

Holding forcewake around this register init sequences makes chip reset
reliable again. Note that this is not the first such issue:

commit f01db988ef6f6c70a6cc36ee71e4a98a68901229
Author: Sean Paul 
Date:   Fri Mar 16 12:43:22 2012 -0400

drm/i915: Add wait_for in init_ring_common

added delay loops to make RING_START and RING_CTL initialization
reliable on the blt ring at boot-up. So I guess it won't hurt if we do
this unconditionally for all force_wake needing gpus.

To avoid copy of the HAS_FORCE_WAKE check I've added a new
intel_info bit for that.

v2: Fixup missing commas in static struct and properly handling the
error case in init_ring_common, both noticed by Jani Nikula.

Cc: sta...@vger.kernel.org
Reported-and-tested-by: Yang Guang 
Reviewed-by: Eugeni Dodonov 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50522
Signed-Off-by: Daniel Vetter 
[herton: backport to 3.0:
 - adjust for different struct intel_device_info layouts
 - drop changes to Haswell, not present in 3.0
 - NEEDS_FORCE_WAKE is on i915_drv.h, and doesn't have IS_VALLEYVIEW ]
Signed-off-by: Herton Ronaldo Krzesinski 
---
 drivers/gpu/drm/i915/i915_drv.c |4 
 drivers/gpu/drm/i915/i915_drv.h |9 ++---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   16 +---
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 111686a..9111d4c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -181,6 +181,7 @@ static const struct intel_device_info 
intel_sandybridge_d_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.has_bsd_ring = 1,
.has_blt_ring = 1,
+   .has_force_wake = 1,
 };
 
 static const struct intel_device_info intel_sandybridge_m_info = {
@@ -189,6 +190,7 @@ static const struct intel_device_info 
intel_sandybridge_m_info = {
.has_fbc = 1,
.has_bsd_ring = 1,
.has_blt_ring = 1,
+   .has_force_wake = 1,
 };
 
 static const struct intel_device_info intel_ivybridge_d_info = {
@@ -196,6 +198,7 @@ static const struct intel_device_info 
intel_ivybridge_d_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.has_bsd_ring = 1,
.has_blt_ring = 1,
+   .has_force_wake = 1,
 };
 
 static const struct intel_device_info intel_ivybridge_m_info = {
@@ -204,6 +207,7 @@ static const struct intel_device_info 
intel_ivybridge_m_info = {
.has_fbc = 0,   /* FBC is not enabled on Ivybridge mobile yet */
.has_bsd_ring = 1,
.has_blt_ring = 1,
+   .has_force_wake = 1,
 };
 
 static const struct pci_device_id pciidlist[] = {  /* aka */
diff --git 

Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-23 Thread Daniel Vetter
On Thu, Aug 23, 2012 at 12:44 AM, Herton Ronaldo Krzesinski
 wrote:
> Really sorry about this, but it was a hardware+bios setting problem
> here. I finished investigating what was happening here, and is related to
> the DRAM installed on this machine. The memory installed is DDR3-1333,
> and on bios it was configured as that, but turns out the chipset (G41)
> only supports up to DDR3-1066 (as on specs, although the motherboard
> lists the 1333 as supported, but as "OC", and allows configuring this
> way automatically or manually).
>
> After setting manually back to 1066, I stopped seeing all the weird
> behaviour, like the writes to the start address don't trigger anymore
> setting on the head offset. I'm sorry about that, and really it's a
> brown paper bag time for me :(. The machine worked fine and passed
> through memtest and never gave any problems, this was the first
> time (seems only the video device didn't like the memory setting, and
> the code change probably changed timing in the right way to pop up this
> now, also never had graphics corruption or other noticeable problems).
> I retested kernels that gave problems and they run fine now.

Well, at least we could clear this up.

Greg, please pick up the following patches for 3.0 (if you haven't already):

f01db988ef6f6c70a6cc36ee71e4a98a68901229 and
0d8957c8a90bbb5d34fab9a304459448a5131e06

Note that all kernels that need f01db backported also need
b7884eb45ec98c0d34c7f49005ae9d4b4b4e38f6 (to fix a regression
introduce by the former).

Thanks, Daniel
-- 
Daniel Vetter
daniel.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-23 Thread Daniel Vetter
On Thu, Aug 23, 2012 at 12:44 AM, Herton Ronaldo Krzesinski
herton.krzesin...@canonical.com wrote:
 Really sorry about this, but it was a hardware+bios setting problem
 here. I finished investigating what was happening here, and is related to
 the DRAM installed on this machine. The memory installed is DDR3-1333,
 and on bios it was configured as that, but turns out the chipset (G41)
 only supports up to DDR3-1066 (as on specs, although the motherboard
 lists the 1333 as supported, but as OC, and allows configuring this
 way automatically or manually).

 After setting manually back to 1066, I stopped seeing all the weird
 behaviour, like the writes to the start address don't trigger anymore
 setting on the head offset. I'm sorry about that, and really it's a
 brown paper bag time for me :(. The machine worked fine and passed
 through memtest and never gave any problems, this was the first
 time (seems only the video device didn't like the memory setting, and
 the code change probably changed timing in the right way to pop up this
 now, also never had graphics corruption or other noticeable problems).
 I retested kernels that gave problems and they run fine now.

Well, at least we could clear this up.

Greg, please pick up the following patches for 3.0 (if you haven't already):

f01db988ef6f6c70a6cc36ee71e4a98a68901229 and
0d8957c8a90bbb5d34fab9a304459448a5131e06

Note that all kernels that need f01db backported also need
b7884eb45ec98c0d34c7f49005ae9d4b4b4e38f6 (to fix a regression
introduce by the former).

Thanks, Daniel
-- 
Daniel Vetter
daniel.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-23 Thread Herton Ronaldo Krzesinski
On Thu, Aug 23, 2012 at 10:10:18AM +0200, Daniel Vetter wrote:
 On Thu, Aug 23, 2012 at 12:44 AM, Herton Ronaldo Krzesinski
 herton.krzesin...@canonical.com wrote:
  Really sorry about this, but it was a hardware+bios setting problem
  here. I finished investigating what was happening here, and is related to
  the DRAM installed on this machine. The memory installed is DDR3-1333,
  and on bios it was configured as that, but turns out the chipset (G41)
  only supports up to DDR3-1066 (as on specs, although the motherboard
  lists the 1333 as supported, but as OC, and allows configuring this
  way automatically or manually).
 
  After setting manually back to 1066, I stopped seeing all the weird
  behaviour, like the writes to the start address don't trigger anymore
  setting on the head offset. I'm sorry about that, and really it's a
  brown paper bag time for me :(. The machine worked fine and passed
  through memtest and never gave any problems, this was the first
  time (seems only the video device didn't like the memory setting, and
  the code change probably changed timing in the right way to pop up this
  now, also never had graphics corruption or other noticeable problems).
  I retested kernels that gave problems and they run fine now.
 
 Well, at least we could clear this up.
 
 Greg, please pick up the following patches for 3.0 (if you haven't already):
 
 f01db988ef6f6c70a6cc36ee71e4a98a68901229 and
 0d8957c8a90bbb5d34fab9a304459448a5131e06
 
 Note that all kernels that need f01db backported also need
 b7884eb45ec98c0d34c7f49005ae9d4b4b4e38f6 (to fix a regression
 introduce by the former).

b7884eb45ec98c0d34c7f49005ae9d4b4b4e38f6 needs some backporting for 3.0,
I did it in advance, build tested and installed here on a sandybridge
based laptop on x86_64:

From 3adf724d83de569510f2df0b91a097de92372970 Mon Sep 17 00:00:00 2001
From: Daniel Vetter daniel.vet...@ffwll.ch
Date: Mon, 4 Jun 2012 11:18:15 +0200
Subject: [PATCH 2/2] drm/i915: hold forcewake around ring hw init

commit b7884eb45ec98c0d34c7f49005ae9d4b4b4e38f6 upstream.

Empirical evidence suggests that we need to: On at least one ivb
machine when running the hangman i-g-t test, the rings don't properly
initialize properly - the RING_START registers seems to be stuck at
all zeros.

Holding forcewake around this register init sequences makes chip reset
reliable again. Note that this is not the first such issue:

commit f01db988ef6f6c70a6cc36ee71e4a98a68901229
Author: Sean Paul seanp...@chromium.org
Date:   Fri Mar 16 12:43:22 2012 -0400

drm/i915: Add wait_for in init_ring_common

added delay loops to make RING_START and RING_CTL initialization
reliable on the blt ring at boot-up. So I guess it won't hurt if we do
this unconditionally for all force_wake needing gpus.

To avoid copypasting of the HAS_FORCE_WAKE check I've added a new
intel_info bit for that.

v2: Fixup missing commas in static struct and properly handling the
error case in init_ring_common, both noticed by Jani Nikula.

Cc: sta...@vger.kernel.org
Reported-and-tested-by: Yang Guang guang.a.y...@intel.com
Reviewed-by: Eugeni Dodonov eugeni.dodo...@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50522
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
[herton: backport to 3.0:
 - adjust for different struct intel_device_info layouts
 - drop changes to Haswell, not present in 3.0
 - NEEDS_FORCE_WAKE is on i915_drv.h, and doesn't have IS_VALLEYVIEW ]
Signed-off-by: Herton Ronaldo Krzesinski herton.krzesin...@canonical.com
---
 drivers/gpu/drm/i915/i915_drv.c |4 
 drivers/gpu/drm/i915/i915_drv.h |9 ++---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   16 +---
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 111686a..9111d4c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -181,6 +181,7 @@ static const struct intel_device_info 
intel_sandybridge_d_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.has_bsd_ring = 1,
.has_blt_ring = 1,
+   .has_force_wake = 1,
 };
 
 static const struct intel_device_info intel_sandybridge_m_info = {
@@ -189,6 +190,7 @@ static const struct intel_device_info 
intel_sandybridge_m_info = {
.has_fbc = 1,
.has_bsd_ring = 1,
.has_blt_ring = 1,
+   .has_force_wake = 1,
 };
 
 static const struct intel_device_info intel_ivybridge_d_info = {
@@ -196,6 +198,7 @@ static const struct intel_device_info 
intel_ivybridge_d_info = {
.need_gfx_hws = 1, .has_hotplug = 1,
.has_bsd_ring = 1,
.has_blt_ring = 1,
+   .has_force_wake = 1,
 };
 
 static const struct intel_device_info intel_ivybridge_m_info = {
@@ -204,6 +207,7 @@ static const struct intel_device_info 
intel_ivybridge_m_info = {
.has_fbc = 0,   /* FBC is not enabled on Ivybridge mobile yet */
.has_bsd_ring = 1,
.has_blt_ring 

Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-22 Thread Herton Ronaldo Krzesinski
On Wed, Aug 22, 2012 at 01:50:16AM -0300, Herton Ronaldo Krzesinski wrote:
> On Tue, Aug 21, 2012 at 06:55:30PM +0200, Daniel Vetter wrote:
> > On Tue, Aug 21, 2012 at 3:11 PM, Herton Ronaldo Krzesinski
> >  wrote:
> > > On Tue, Aug 21, 2012 at 08:42:35AM +0200, Daniel Vetter wrote:
> > >> On Tue, Aug 21, 2012 at 7:13 AM, Herton Ronaldo Krzesinski
> > >>  wrote:
> > >> > I had the same problem as on 3.2 with this change, i915 stopped working
> > >> > unable to initialize render ring, eg. on one of the boots here:
> > >> > [drm:init_ring_common] *ERROR* render ring initialization failed ctl 
> > >> > 0001f003 head 1020 tail  start 1000
> > >> >
> > >> > But unlike I was expecting as with 3.2 case, picking commit
> > >> > f01db988ef6f6c70a6cc36ee71e4a98a68901229 ("drm/i915: Add wait_for in
> > >> > init_ring_common") here isn't enough, it continues to fail even if I
> > >> > try to increase the delay in the wait_for, I'm not sure why yet... may
> > >> > be something else is going on, or 3.0 has something else missing.
> > >> >
> > >> > Also the same proposed patch for 3.4.10 gives the same problem, but
> > >> > picking f01db988ef6f6c70a6cc36ee71e4a98a68901229 there made things work
> > >> > again like happend on first 3.2.28 proposed update. Only 3.0
> > >> > is misteriously failing either way here.
> > >>
> > >> I guess we're missing something then still in the stable backports for
> > >> 3.0. Herton, what machine do you have exaclty (lspci -nn)?
> > >
> > > It's a G41 based board:
> > 
> > Hm, I've reviewed git log and bug reports and I have no idea what's
> > missing on 3.0. I guess the best course of action is to not apply this
> > patch to 3.0 stable - it fixes an ivb issue anyway, and 3.0 is a
> > rather old kernel for ivb support anyway (we generally recommend 3.2.x
> > for ivb).
> > -Daniel
> 
> Yeah, 3.0 being old, if it was only for ivb, supported only later, makes
> sense. I continued investigating this today, and some things are looking
> very strange to me. Here is what I discovered and narrowed down so far:
[...]

Really sorry about this, but it was a hardware+bios setting problem
here. I finished investigating what was happening here, and is related to
the DRAM installed on this machine. The memory installed is DDR3-1333,
and on bios it was configured as that, but turns out the chipset (G41)
only supports up to DDR3-1066 (as on specs, although the motherboard
lists the 1333 as supported, but as "OC", and allows configuring this
way automatically or manually).

After setting manually back to 1066, I stopped seeing all the weird
behaviour, like the writes to the start address don't trigger anymore
setting on the head offset. I'm sorry about that, and really it's a
brown paper bag time for me :(. The machine worked fine and passed
through memtest and never gave any problems, this was the first
time (seems only the video device didn't like the memory setting, and
the code change probably changed timing in the right way to pop up this
now, also never had graphics corruption or other noticeable problems).
I retested kernels that gave problems and they run fine now.

-- 
[]'s
Herton
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-22 Thread Herton Ronaldo Krzesinski
On Wed, Aug 22, 2012 at 01:50:16AM -0300, Herton Ronaldo Krzesinski wrote:
 On Tue, Aug 21, 2012 at 06:55:30PM +0200, Daniel Vetter wrote:
  On Tue, Aug 21, 2012 at 3:11 PM, Herton Ronaldo Krzesinski
  herton.krzesin...@canonical.com wrote:
   On Tue, Aug 21, 2012 at 08:42:35AM +0200, Daniel Vetter wrote:
   On Tue, Aug 21, 2012 at 7:13 AM, Herton Ronaldo Krzesinski
   herton.krzesin...@canonical.com wrote:
I had the same problem as on 3.2 with this change, i915 stopped working
unable to initialize render ring, eg. on one of the boots here:
[drm:init_ring_common] *ERROR* render ring initialization failed ctl 
0001f003 head 1020 tail  start 1000
   
But unlike I was expecting as with 3.2 case, picking commit
f01db988ef6f6c70a6cc36ee71e4a98a68901229 (drm/i915: Add wait_for in
init_ring_common) here isn't enough, it continues to fail even if I
try to increase the delay in the wait_for, I'm not sure why yet... may
be something else is going on, or 3.0 has something else missing.
   
Also the same proposed patch for 3.4.10 gives the same problem, but
picking f01db988ef6f6c70a6cc36ee71e4a98a68901229 there made things work
again like happend on first 3.2.28 proposed update. Only 3.0
is misteriously failing either way here.
  
   I guess we're missing something then still in the stable backports for
   3.0. Herton, what machine do you have exaclty (lspci -nn)?
  
   It's a G41 based board:
  
  Hm, I've reviewed git log and bug reports and I have no idea what's
  missing on 3.0. I guess the best course of action is to not apply this
  patch to 3.0 stable - it fixes an ivb issue anyway, and 3.0 is a
  rather old kernel for ivb support anyway (we generally recommend 3.2.x
  for ivb).
  -Daniel
 
 Yeah, 3.0 being old, if it was only for ivb, supported only later, makes
 sense. I continued investigating this today, and some things are looking
 very strange to me. Here is what I discovered and narrowed down so far:
[...]

Really sorry about this, but it was a hardware+bios setting problem
here. I finished investigating what was happening here, and is related to
the DRAM installed on this machine. The memory installed is DDR3-1333,
and on bios it was configured as that, but turns out the chipset (G41)
only supports up to DDR3-1066 (as on specs, although the motherboard
lists the 1333 as supported, but as OC, and allows configuring this
way automatically or manually).

After setting manually back to 1066, I stopped seeing all the weird
behaviour, like the writes to the start address don't trigger anymore
setting on the head offset. I'm sorry about that, and really it's a
brown paper bag time for me :(. The machine worked fine and passed
through memtest and never gave any problems, this was the first
time (seems only the video device didn't like the memory setting, and
the code change probably changed timing in the right way to pop up this
now, also never had graphics corruption or other noticeable problems).
I retested kernels that gave problems and they run fine now.

-- 
[]'s
Herton
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-21 Thread Herton Ronaldo Krzesinski
On Tue, Aug 21, 2012 at 06:55:30PM +0200, Daniel Vetter wrote:
> On Tue, Aug 21, 2012 at 3:11 PM, Herton Ronaldo Krzesinski
>  wrote:
> > On Tue, Aug 21, 2012 at 08:42:35AM +0200, Daniel Vetter wrote:
> >> On Tue, Aug 21, 2012 at 7:13 AM, Herton Ronaldo Krzesinski
> >>  wrote:
> >> > I had the same problem as on 3.2 with this change, i915 stopped working
> >> > unable to initialize render ring, eg. on one of the boots here:
> >> > [drm:init_ring_common] *ERROR* render ring initialization failed ctl 
> >> > 0001f003 head 1020 tail  start 1000
> >> >
> >> > But unlike I was expecting as with 3.2 case, picking commit
> >> > f01db988ef6f6c70a6cc36ee71e4a98a68901229 ("drm/i915: Add wait_for in
> >> > init_ring_common") here isn't enough, it continues to fail even if I
> >> > try to increase the delay in the wait_for, I'm not sure why yet... may
> >> > be something else is going on, or 3.0 has something else missing.
> >> >
> >> > Also the same proposed patch for 3.4.10 gives the same problem, but
> >> > picking f01db988ef6f6c70a6cc36ee71e4a98a68901229 there made things work
> >> > again like happend on first 3.2.28 proposed update. Only 3.0
> >> > is misteriously failing either way here.
> >>
> >> I guess we're missing something then still in the stable backports for
> >> 3.0. Herton, what machine do you have exaclty (lspci -nn)?
> >
> > It's a G41 based board:
> 
> Hm, I've reviewed git log and bug reports and I have no idea what's
> missing on 3.0. I guess the best course of action is to not apply this
> patch to 3.0 stable - it fixes an ivb issue anyway, and 3.0 is a
> rather old kernel for ivb support anyway (we generally recommend 3.2.x
> for ivb).
> -Daniel

Yeah, 3.0 being old, if it was only for ivb, supported only later, makes
sense. I continued investigating this today, and some things are looking
very strange to me. Here is what I discovered and narrowed down so far:

* There is difference of behaviour depending on which environment I
  build 3.0: Originally I built the 3.0 kernel on Ubuntu Oneiric, gcc is
  4.6.1, binutils 2.21.53.20110810 (I'm just saying the versions, not sure
  if it makes a difference or not, and the distro toolchain usually is
  patched so the versions may not mean much). Then I tried building the
  3.0 kernel on a newer environment/distro version, Ubuntu Precise, gcc is
  4.6.3, binutils 2.22.
- Building the stock 3.0.42 proposed kernel on both Ubuntu distros
  (Oneiric and Precise), and testing them, I get the same render ring
  initialization failure.
- Building 3.0.42 with commit f01db988ef6f6c70a6cc36ee71e4a98a68901229
  picked on top gives a different result though: The kernel built on
  Oneiric continues to fail, while the one built on Precise then works.
This was very weird, and I suspected of the toolchain. But the generated
assembly is the same, I also compared the objdump output of the built
i915 module, and there were no differences in the init_common_ring
function. There were some differences in other places, but no difference
in the section of code patched between the working case to the
non-working case. Really didn't made sense, and probably something else
is going on, related to code size or timing or alignment somewhere, I
don't know, but I was unable yet to detect really what's the cause.

I also noticed something else. I started to patch the init_ring_common
function reading back and printing the render ring buffer pointer
values that were being set, with the attached patch, and building the
kernel on the environment which the bug happens (Oneiric). This is the
relevant output I got, I numbered by hand to comment after:

1)
[   34.872786] i915: ctl  head  tail  start 
[   34.872789] i915: ctl  head  tail  start 
[   34.872792] i915: ctl  head  tail  start 
[   34.872793] i915: head = 
[   34.872795] i915: ctl  head  tail  start 
2)
[   34.872797] i915: head = 1000
[   34.872798] i915: ctl  head 1000 tail  start 1000
3)
[   35.880034] i915: timeout
[   35.936111] [drm:init_ring_common] *ERROR* render ring initialization failed 
ctl 0001f003 head 7cc4 tail  start 1000
[   35.936174] i915: 0001 1000 7cc4
[   35.936229] vga_switcheroo: disabled
[   35.936232] [drm:i915_driver_load] *ERROR* failed to init modeset
[   35.954182] i915 :00:02.0: PCI INT A disabled
[   35.954188] i915: probe of :00:02.0 failed with error -5

1) At first run, the render ring buffer is "stopped", everything is
zero. The zeros written to ctl, head and tail changes nothing, everything
stays zero, as expected.

2) These are the values read after I915_WRITE_START runs. For some
reason on this machine here, after start address was written, the head
is set the same as the start (?), and this is the root of the failing
case here. I was reading the documentation, and it says when 

Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-21 Thread Daniel Vetter
On Tue, Aug 21, 2012 at 3:11 PM, Herton Ronaldo Krzesinski
 wrote:
> On Tue, Aug 21, 2012 at 08:42:35AM +0200, Daniel Vetter wrote:
>> On Tue, Aug 21, 2012 at 7:13 AM, Herton Ronaldo Krzesinski
>>  wrote:
>> > I had the same problem as on 3.2 with this change, i915 stopped working
>> > unable to initialize render ring, eg. on one of the boots here:
>> > [drm:init_ring_common] *ERROR* render ring initialization failed ctl 
>> > 0001f003 head 1020 tail  start 1000
>> >
>> > But unlike I was expecting as with 3.2 case, picking commit
>> > f01db988ef6f6c70a6cc36ee71e4a98a68901229 ("drm/i915: Add wait_for in
>> > init_ring_common") here isn't enough, it continues to fail even if I
>> > try to increase the delay in the wait_for, I'm not sure why yet... may
>> > be something else is going on, or 3.0 has something else missing.
>> >
>> > Also the same proposed patch for 3.4.10 gives the same problem, but
>> > picking f01db988ef6f6c70a6cc36ee71e4a98a68901229 there made things work
>> > again like happend on first 3.2.28 proposed update. Only 3.0
>> > is misteriously failing either way here.
>>
>> I guess we're missing something then still in the stable backports for
>> 3.0. Herton, what machine do you have exaclty (lspci -nn)?
>
> It's a G41 based board:

Hm, I've reviewed git log and bug reports and I have no idea what's
missing on 3.0. I guess the best course of action is to not apply this
patch to 3.0 stable - it fixes an ivb issue anyway, and 3.0 is a
rather old kernel for ivb support anyway (we generally recommend 3.2.x
for ivb).
-Daniel

>
> 00:00.0 Host bridge [0600]: Intel Corporation 4 Series Chipset DRAM 
> Controller [8086:2e30] (rev 03)
> 00:02.0 VGA compatible controller [0300]: Intel Corporation 4 Series Chipset 
> Integrated Graphics Controller [8086:2e32] (rev 03)
> 00:1b.0 Audio device [0403]: Intel Corporation N10/ICH 7 Family High 
> Definition Audio Controller [8086:27d8] (rev 01)
> 00:1c.0 PCI bridge [0604]: Intel Corporation N10/ICH 7 Family PCI Express 
> Port 1 [8086:27d0] (rev 01)
> 00:1c.2 PCI bridge [0604]: Intel Corporation N10/ICH 7 Family PCI Express 
> Port 3 [8086:27d4] (rev 01)
> 00:1d.0 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
> Controller #1 [8086:27c8] (rev 01)
> 00:1d.1 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
> Controller #2 [8086:27c9] (rev 01)
> 00:1d.2 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
> Controller #3 [8086:27ca] (rev 01)
> 00:1d.3 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
> Controller #4 [8086:27cb] (rev 01)
> 00:1d.7 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB2 EHCI 
> Controller [8086:27cc] (rev 01)
> 00:1e.0 PCI bridge [0604]: Intel Corporation 82801 PCI Bridge [8086:244e] 
> (rev e1)
> 00:1f.0 ISA bridge [0601]: Intel Corporation 82801GB/GR (ICH7 Family) LPC 
> Interface Bridge [8086:27b8] (rev 01)
> 00:1f.2 IDE interface [0101]: Intel Corporation N10/ICH7 Family SATA IDE 
> Controller [8086:27c0] (rev 01)
> 00:1f.3 SMBus [0c05]: Intel Corporation N10/ICH 7 Family SMBus Controller 
> [8086:27da] (rev 01)
> 02:00.0 Ethernet controller [0200]: Atheros Communications AR8151 v1.0 
> Gigabit Ethernet [1969:1073] (rev c0)
>
>>
>> Greg, I think for now it's better if you hold off on merging this
>> patch to 3.0 until this is sorted out.
>>
>> Thanks, Daniel
>> --
>> Daniel Vetter
>> daniel.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>>
>
> --
> []'s
> Herton



-- 
Daniel Vetter
daniel.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-21 Thread Herton Ronaldo Krzesinski
On Tue, Aug 21, 2012 at 08:42:35AM +0200, Daniel Vetter wrote:
> On Tue, Aug 21, 2012 at 7:13 AM, Herton Ronaldo Krzesinski
>  wrote:
> > I had the same problem as on 3.2 with this change, i915 stopped working
> > unable to initialize render ring, eg. on one of the boots here:
> > [drm:init_ring_common] *ERROR* render ring initialization failed ctl 
> > 0001f003 head 1020 tail  start 1000
> >
> > But unlike I was expecting as with 3.2 case, picking commit
> > f01db988ef6f6c70a6cc36ee71e4a98a68901229 ("drm/i915: Add wait_for in
> > init_ring_common") here isn't enough, it continues to fail even if I
> > try to increase the delay in the wait_for, I'm not sure why yet... may
> > be something else is going on, or 3.0 has something else missing.
> >
> > Also the same proposed patch for 3.4.10 gives the same problem, but
> > picking f01db988ef6f6c70a6cc36ee71e4a98a68901229 there made things work
> > again like happend on first 3.2.28 proposed update. Only 3.0
> > is misteriously failing either way here.
> 
> I guess we're missing something then still in the stable backports for
> 3.0. Herton, what machine do you have exaclty (lspci -nn)?

It's a G41 based board:

00:00.0 Host bridge [0600]: Intel Corporation 4 Series Chipset DRAM Controller 
[8086:2e30] (rev 03)
00:02.0 VGA compatible controller [0300]: Intel Corporation 4 Series Chipset 
Integrated Graphics Controller [8086:2e32] (rev 03)
00:1b.0 Audio device [0403]: Intel Corporation N10/ICH 7 Family High Definition 
Audio Controller [8086:27d8] (rev 01)
00:1c.0 PCI bridge [0604]: Intel Corporation N10/ICH 7 Family PCI Express Port 
1 [8086:27d0] (rev 01)
00:1c.2 PCI bridge [0604]: Intel Corporation N10/ICH 7 Family PCI Express Port 
3 [8086:27d4] (rev 01)
00:1d.0 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #1 [8086:27c8] (rev 01)
00:1d.1 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #2 [8086:27c9] (rev 01)
00:1d.2 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #3 [8086:27ca] (rev 01)
00:1d.3 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #4 [8086:27cb] (rev 01)
00:1d.7 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB2 EHCI 
Controller [8086:27cc] (rev 01)
00:1e.0 PCI bridge [0604]: Intel Corporation 82801 PCI Bridge [8086:244e] (rev 
e1)
00:1f.0 ISA bridge [0601]: Intel Corporation 82801GB/GR (ICH7 Family) LPC 
Interface Bridge [8086:27b8] (rev 01)
00:1f.2 IDE interface [0101]: Intel Corporation N10/ICH7 Family SATA IDE 
Controller [8086:27c0] (rev 01)
00:1f.3 SMBus [0c05]: Intel Corporation N10/ICH 7 Family SMBus Controller 
[8086:27da] (rev 01)
02:00.0 Ethernet controller [0200]: Atheros Communications AR8151 v1.0 Gigabit 
Ethernet [1969:1073] (rev c0)

> 
> Greg, I think for now it's better if you hold off on merging this
> patch to 3.0 until this is sorted out.
> 
> Thanks, Daniel
> -- 
> Daniel Vetter
> daniel.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> 

-- 
[]'s
Herton
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-21 Thread Daniel Vetter
On Tue, Aug 21, 2012 at 7:13 AM, Herton Ronaldo Krzesinski
 wrote:
> On Sun, Aug 19, 2012 at 08:56:03PM -0700, Greg Kroah-Hartman wrote:
>> From: Greg KH 
>>
>> 3.0-stable review patch.  If anyone has any objections, please let me know.
>>
>> --
>>
>> From: Daniel Vetter 
>>
>> commit 0d8957c8a90bbb5d34fab9a304459448a5131e06 upstream.
>>
>> We may only start to set up the new register values after having
>> confirmed that the ring is truely off. Otherwise the hw might lose the
>> newly written register values. This is caught later on in the init
>> sequence, when we check whether the register writes have stuck.
>>
>> Reviewed-by: Jani Nikula 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50522
>> Tested-by: Yang Guang 
>> Signed-off-by: Daniel Vetter 
>> Signed-off-by: Greg Kroah-Hartman 
> [...]
>
> I had the same problem as on 3.2 with this change, i915 stopped working
> unable to initialize render ring, eg. on one of the boots here:
> [drm:init_ring_common] *ERROR* render ring initialization failed ctl 0001f003 
> head 1020 tail  start 1000
>
> But unlike I was expecting as with 3.2 case, picking commit
> f01db988ef6f6c70a6cc36ee71e4a98a68901229 ("drm/i915: Add wait_for in
> init_ring_common") here isn't enough, it continues to fail even if I
> try to increase the delay in the wait_for, I'm not sure why yet... may
> be something else is going on, or 3.0 has something else missing.
>
> Also the same proposed patch for 3.4.10 gives the same problem, but
> picking f01db988ef6f6c70a6cc36ee71e4a98a68901229 there made things work
> again like happend on first 3.2.28 proposed update. Only 3.0
> is misteriously failing either way here.

I guess we're missing something then still in the stable backports for
3.0. Herton, what machine do you have exaclty (lspci -nn)?

Greg, I think for now it's better if you hold off on merging this
patch to 3.0 until this is sorted out.

Thanks, Daniel
-- 
Daniel Vetter
daniel.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-21 Thread Herton Ronaldo Krzesinski
On Tue, Aug 21, 2012 at 06:55:30PM +0200, Daniel Vetter wrote:
 On Tue, Aug 21, 2012 at 3:11 PM, Herton Ronaldo Krzesinski
 herton.krzesin...@canonical.com wrote:
  On Tue, Aug 21, 2012 at 08:42:35AM +0200, Daniel Vetter wrote:
  On Tue, Aug 21, 2012 at 7:13 AM, Herton Ronaldo Krzesinski
  herton.krzesin...@canonical.com wrote:
   I had the same problem as on 3.2 with this change, i915 stopped working
   unable to initialize render ring, eg. on one of the boots here:
   [drm:init_ring_common] *ERROR* render ring initialization failed ctl 
   0001f003 head 1020 tail  start 1000
  
   But unlike I was expecting as with 3.2 case, picking commit
   f01db988ef6f6c70a6cc36ee71e4a98a68901229 (drm/i915: Add wait_for in
   init_ring_common) here isn't enough, it continues to fail even if I
   try to increase the delay in the wait_for, I'm not sure why yet... may
   be something else is going on, or 3.0 has something else missing.
  
   Also the same proposed patch for 3.4.10 gives the same problem, but
   picking f01db988ef6f6c70a6cc36ee71e4a98a68901229 there made things work
   again like happend on first 3.2.28 proposed update. Only 3.0
   is misteriously failing either way here.
 
  I guess we're missing something then still in the stable backports for
  3.0. Herton, what machine do you have exaclty (lspci -nn)?
 
  It's a G41 based board:
 
 Hm, I've reviewed git log and bug reports and I have no idea what's
 missing on 3.0. I guess the best course of action is to not apply this
 patch to 3.0 stable - it fixes an ivb issue anyway, and 3.0 is a
 rather old kernel for ivb support anyway (we generally recommend 3.2.x
 for ivb).
 -Daniel

Yeah, 3.0 being old, if it was only for ivb, supported only later, makes
sense. I continued investigating this today, and some things are looking
very strange to me. Here is what I discovered and narrowed down so far:

* There is difference of behaviour depending on which environment I
  build 3.0: Originally I built the 3.0 kernel on Ubuntu Oneiric, gcc is
  4.6.1, binutils 2.21.53.20110810 (I'm just saying the versions, not sure
  if it makes a difference or not, and the distro toolchain usually is
  patched so the versions may not mean much). Then I tried building the
  3.0 kernel on a newer environment/distro version, Ubuntu Precise, gcc is
  4.6.3, binutils 2.22.
- Building the stock 3.0.42 proposed kernel on both Ubuntu distros
  (Oneiric and Precise), and testing them, I get the same render ring
  initialization failure.
- Building 3.0.42 with commit f01db988ef6f6c70a6cc36ee71e4a98a68901229
  picked on top gives a different result though: The kernel built on
  Oneiric continues to fail, while the one built on Precise then works.
This was very weird, and I suspected of the toolchain. But the generated
assembly is the same, I also compared the objdump output of the built
i915 module, and there were no differences in the init_common_ring
function. There were some differences in other places, but no difference
in the section of code patched between the working case to the
non-working case. Really didn't made sense, and probably something else
is going on, related to code size or timing or alignment somewhere, I
don't know, but I was unable yet to detect really what's the cause.

I also noticed something else. I started to patch the init_ring_common
function reading back and printing the render ring buffer pointer
values that were being set, with the attached patch, and building the
kernel on the environment which the bug happens (Oneiric). This is the
relevant output I got, I numbered by hand to comment after:

1)
[   34.872786] i915: ctl  head  tail  start 
[   34.872789] i915: ctl  head  tail  start 
[   34.872792] i915: ctl  head  tail  start 
[   34.872793] i915: head = 
[   34.872795] i915: ctl  head  tail  start 
2)
[   34.872797] i915: head = 1000
[   34.872798] i915: ctl  head 1000 tail  start 1000
3)
[   35.880034] i915: timeout
[   35.936111] [drm:init_ring_common] *ERROR* render ring initialization failed 
ctl 0001f003 head 7cc4 tail  start 1000
[   35.936174] i915: 0001 1000 7cc4
[   35.936229] vga_switcheroo: disabled
[   35.936232] [drm:i915_driver_load] *ERROR* failed to init modeset
[   35.954182] i915 :00:02.0: PCI INT A disabled
[   35.954188] i915: probe of :00:02.0 failed with error -5

1) At first run, the render ring buffer is stopped, everything is
zero. The zeros written to ctl, head and tail changes nothing, everything
stays zero, as expected.

2) These are the values read after I915_WRITE_START runs. For some
reason on this machine here, after start address was written, the head
is set the same as the start (?), and this is the root of the failing
case here. I was reading the documentation, and it says when you
write the start address, 

Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-21 Thread Daniel Vetter
On Tue, Aug 21, 2012 at 7:13 AM, Herton Ronaldo Krzesinski
herton.krzesin...@canonical.com wrote:
 On Sun, Aug 19, 2012 at 08:56:03PM -0700, Greg Kroah-Hartman wrote:
 From: Greg KH gre...@linuxfoundation.org

 3.0-stable review patch.  If anyone has any objections, please let me know.

 --

 From: Daniel Vetter daniel.vet...@ffwll.ch

 commit 0d8957c8a90bbb5d34fab9a304459448a5131e06 upstream.

 We may only start to set up the new register values after having
 confirmed that the ring is truely off. Otherwise the hw might lose the
 newly written register values. This is caught later on in the init
 sequence, when we check whether the register writes have stuck.

 Reviewed-by: Jani Nikula jani.nik...@intel.com
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50522
 Tested-by: Yang Guang guang.a.y...@intel.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
 [...]

 I had the same problem as on 3.2 with this change, i915 stopped working
 unable to initialize render ring, eg. on one of the boots here:
 [drm:init_ring_common] *ERROR* render ring initialization failed ctl 0001f003 
 head 1020 tail  start 1000

 But unlike I was expecting as with 3.2 case, picking commit
 f01db988ef6f6c70a6cc36ee71e4a98a68901229 (drm/i915: Add wait_for in
 init_ring_common) here isn't enough, it continues to fail even if I
 try to increase the delay in the wait_for, I'm not sure why yet... may
 be something else is going on, or 3.0 has something else missing.

 Also the same proposed patch for 3.4.10 gives the same problem, but
 picking f01db988ef6f6c70a6cc36ee71e4a98a68901229 there made things work
 again like happend on first 3.2.28 proposed update. Only 3.0
 is misteriously failing either way here.

I guess we're missing something then still in the stable backports for
3.0. Herton, what machine do you have exaclty (lspci -nn)?

Greg, I think for now it's better if you hold off on merging this
patch to 3.0 until this is sorted out.

Thanks, Daniel
-- 
Daniel Vetter
daniel.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-21 Thread Herton Ronaldo Krzesinski
On Tue, Aug 21, 2012 at 08:42:35AM +0200, Daniel Vetter wrote:
 On Tue, Aug 21, 2012 at 7:13 AM, Herton Ronaldo Krzesinski
 herton.krzesin...@canonical.com wrote:
  I had the same problem as on 3.2 with this change, i915 stopped working
  unable to initialize render ring, eg. on one of the boots here:
  [drm:init_ring_common] *ERROR* render ring initialization failed ctl 
  0001f003 head 1020 tail  start 1000
 
  But unlike I was expecting as with 3.2 case, picking commit
  f01db988ef6f6c70a6cc36ee71e4a98a68901229 (drm/i915: Add wait_for in
  init_ring_common) here isn't enough, it continues to fail even if I
  try to increase the delay in the wait_for, I'm not sure why yet... may
  be something else is going on, or 3.0 has something else missing.
 
  Also the same proposed patch for 3.4.10 gives the same problem, but
  picking f01db988ef6f6c70a6cc36ee71e4a98a68901229 there made things work
  again like happend on first 3.2.28 proposed update. Only 3.0
  is misteriously failing either way here.
 
 I guess we're missing something then still in the stable backports for
 3.0. Herton, what machine do you have exaclty (lspci -nn)?

It's a G41 based board:

00:00.0 Host bridge [0600]: Intel Corporation 4 Series Chipset DRAM Controller 
[8086:2e30] (rev 03)
00:02.0 VGA compatible controller [0300]: Intel Corporation 4 Series Chipset 
Integrated Graphics Controller [8086:2e32] (rev 03)
00:1b.0 Audio device [0403]: Intel Corporation N10/ICH 7 Family High Definition 
Audio Controller [8086:27d8] (rev 01)
00:1c.0 PCI bridge [0604]: Intel Corporation N10/ICH 7 Family PCI Express Port 
1 [8086:27d0] (rev 01)
00:1c.2 PCI bridge [0604]: Intel Corporation N10/ICH 7 Family PCI Express Port 
3 [8086:27d4] (rev 01)
00:1d.0 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #1 [8086:27c8] (rev 01)
00:1d.1 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #2 [8086:27c9] (rev 01)
00:1d.2 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #3 [8086:27ca] (rev 01)
00:1d.3 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
Controller #4 [8086:27cb] (rev 01)
00:1d.7 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB2 EHCI 
Controller [8086:27cc] (rev 01)
00:1e.0 PCI bridge [0604]: Intel Corporation 82801 PCI Bridge [8086:244e] (rev 
e1)
00:1f.0 ISA bridge [0601]: Intel Corporation 82801GB/GR (ICH7 Family) LPC 
Interface Bridge [8086:27b8] (rev 01)
00:1f.2 IDE interface [0101]: Intel Corporation N10/ICH7 Family SATA IDE 
Controller [8086:27c0] (rev 01)
00:1f.3 SMBus [0c05]: Intel Corporation N10/ICH 7 Family SMBus Controller 
[8086:27da] (rev 01)
02:00.0 Ethernet controller [0200]: Atheros Communications AR8151 v1.0 Gigabit 
Ethernet [1969:1073] (rev c0)

 
 Greg, I think for now it's better if you hold off on merging this
 patch to 3.0 until this is sorted out.
 
 Thanks, Daniel
 -- 
 Daniel Vetter
 daniel.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
 

-- 
[]'s
Herton
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-21 Thread Daniel Vetter
On Tue, Aug 21, 2012 at 3:11 PM, Herton Ronaldo Krzesinski
herton.krzesin...@canonical.com wrote:
 On Tue, Aug 21, 2012 at 08:42:35AM +0200, Daniel Vetter wrote:
 On Tue, Aug 21, 2012 at 7:13 AM, Herton Ronaldo Krzesinski
 herton.krzesin...@canonical.com wrote:
  I had the same problem as on 3.2 with this change, i915 stopped working
  unable to initialize render ring, eg. on one of the boots here:
  [drm:init_ring_common] *ERROR* render ring initialization failed ctl 
  0001f003 head 1020 tail  start 1000
 
  But unlike I was expecting as with 3.2 case, picking commit
  f01db988ef6f6c70a6cc36ee71e4a98a68901229 (drm/i915: Add wait_for in
  init_ring_common) here isn't enough, it continues to fail even if I
  try to increase the delay in the wait_for, I'm not sure why yet... may
  be something else is going on, or 3.0 has something else missing.
 
  Also the same proposed patch for 3.4.10 gives the same problem, but
  picking f01db988ef6f6c70a6cc36ee71e4a98a68901229 there made things work
  again like happend on first 3.2.28 proposed update. Only 3.0
  is misteriously failing either way here.

 I guess we're missing something then still in the stable backports for
 3.0. Herton, what machine do you have exaclty (lspci -nn)?

 It's a G41 based board:

Hm, I've reviewed git log and bug reports and I have no idea what's
missing on 3.0. I guess the best course of action is to not apply this
patch to 3.0 stable - it fixes an ivb issue anyway, and 3.0 is a
rather old kernel for ivb support anyway (we generally recommend 3.2.x
for ivb).
-Daniel


 00:00.0 Host bridge [0600]: Intel Corporation 4 Series Chipset DRAM 
 Controller [8086:2e30] (rev 03)
 00:02.0 VGA compatible controller [0300]: Intel Corporation 4 Series Chipset 
 Integrated Graphics Controller [8086:2e32] (rev 03)
 00:1b.0 Audio device [0403]: Intel Corporation N10/ICH 7 Family High 
 Definition Audio Controller [8086:27d8] (rev 01)
 00:1c.0 PCI bridge [0604]: Intel Corporation N10/ICH 7 Family PCI Express 
 Port 1 [8086:27d0] (rev 01)
 00:1c.2 PCI bridge [0604]: Intel Corporation N10/ICH 7 Family PCI Express 
 Port 3 [8086:27d4] (rev 01)
 00:1d.0 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
 Controller #1 [8086:27c8] (rev 01)
 00:1d.1 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
 Controller #2 [8086:27c9] (rev 01)
 00:1d.2 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
 Controller #3 [8086:27ca] (rev 01)
 00:1d.3 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB UHCI 
 Controller #4 [8086:27cb] (rev 01)
 00:1d.7 USB Controller [0c03]: Intel Corporation N10/ICH 7 Family USB2 EHCI 
 Controller [8086:27cc] (rev 01)
 00:1e.0 PCI bridge [0604]: Intel Corporation 82801 PCI Bridge [8086:244e] 
 (rev e1)
 00:1f.0 ISA bridge [0601]: Intel Corporation 82801GB/GR (ICH7 Family) LPC 
 Interface Bridge [8086:27b8] (rev 01)
 00:1f.2 IDE interface [0101]: Intel Corporation N10/ICH7 Family SATA IDE 
 Controller [8086:27c0] (rev 01)
 00:1f.3 SMBus [0c05]: Intel Corporation N10/ICH 7 Family SMBus Controller 
 [8086:27da] (rev 01)
 02:00.0 Ethernet controller [0200]: Atheros Communications AR8151 v1.0 
 Gigabit Ethernet [1969:1073] (rev c0)


 Greg, I think for now it's better if you hold off on merging this
 patch to 3.0 until this is sorted out.

 Thanks, Daniel
 --
 Daniel Vetter
 daniel.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch


 --
 []'s
 Herton



-- 
Daniel Vetter
daniel.vet...@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-20 Thread Herton Ronaldo Krzesinski
On Sun, Aug 19, 2012 at 08:56:03PM -0700, Greg Kroah-Hartman wrote:
> From: Greg KH 
> 
> 3.0-stable review patch.  If anyone has any objections, please let me know.
> 
> --
> 
> From: Daniel Vetter 
> 
> commit 0d8957c8a90bbb5d34fab9a304459448a5131e06 upstream.
> 
> We may only start to set up the new register values after having
> confirmed that the ring is truely off. Otherwise the hw might lose the
> newly written register values. This is caught later on in the init
> sequence, when we check whether the register writes have stuck.
> 
> Reviewed-by: Jani Nikula 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50522
> Tested-by: Yang Guang 
> Signed-off-by: Daniel Vetter 
> Signed-off-by: Greg Kroah-Hartman 
[...]

I had the same problem as on 3.2 with this change, i915 stopped working
unable to initialize render ring, eg. on one of the boots here:
[drm:init_ring_common] *ERROR* render ring initialization failed ctl 0001f003 
head 1020 tail  start 1000 

But unlike I was expecting as with 3.2 case, picking commit
f01db988ef6f6c70a6cc36ee71e4a98a68901229 ("drm/i915: Add wait_for in
init_ring_common") here isn't enough, it continues to fail even if I
try to increase the delay in the wait_for, I'm not sure why yet... may
be something else is going on, or 3.0 has something else missing.

Also the same proposed patch for 3.4.10 gives the same problem, but
picking f01db988ef6f6c70a6cc36ee71e4a98a68901229 there made things work
again like happend on first 3.2.28 proposed update. Only 3.0
is misteriously failing either way here.

-- 
[]'s
Herton
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [ 04/16] drm/i915: correctly order the ring init sequence

2012-08-20 Thread Herton Ronaldo Krzesinski
On Sun, Aug 19, 2012 at 08:56:03PM -0700, Greg Kroah-Hartman wrote:
 From: Greg KH gre...@linuxfoundation.org
 
 3.0-stable review patch.  If anyone has any objections, please let me know.
 
 --
 
 From: Daniel Vetter daniel.vet...@ffwll.ch
 
 commit 0d8957c8a90bbb5d34fab9a304459448a5131e06 upstream.
 
 We may only start to set up the new register values after having
 confirmed that the ring is truely off. Otherwise the hw might lose the
 newly written register values. This is caught later on in the init
 sequence, when we check whether the register writes have stuck.
 
 Reviewed-by: Jani Nikula jani.nik...@intel.com
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50522
 Tested-by: Yang Guang guang.a.y...@intel.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
[...]

I had the same problem as on 3.2 with this change, i915 stopped working
unable to initialize render ring, eg. on one of the boots here:
[drm:init_ring_common] *ERROR* render ring initialization failed ctl 0001f003 
head 1020 tail  start 1000 

But unlike I was expecting as with 3.2 case, picking commit
f01db988ef6f6c70a6cc36ee71e4a98a68901229 (drm/i915: Add wait_for in
init_ring_common) here isn't enough, it continues to fail even if I
try to increase the delay in the wait_for, I'm not sure why yet... may
be something else is going on, or 3.0 has something else missing.

Also the same proposed patch for 3.4.10 gives the same problem, but
picking f01db988ef6f6c70a6cc36ee71e4a98a68901229 there made things work
again like happend on first 3.2.28 proposed update. Only 3.0
is misteriously failing either way here.

-- 
[]'s
Herton
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[ 04/16] drm/i915: correctly order the ring init sequence

2012-08-19 Thread Greg Kroah-Hartman
From: Greg KH 

3.0-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Vetter 

commit 0d8957c8a90bbb5d34fab9a304459448a5131e06 upstream.

We may only start to set up the new register values after having
confirmed that the ring is truely off. Otherwise the hw might lose the
newly written register values. This is caught later on in the init
sequence, when we check whether the register writes have stuck.

Reviewed-by: Jani Nikula 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50522
Tested-by: Yang Guang 
Signed-off-by: Daniel Vetter 
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/gpu/drm/i915/intel_ringbuffer.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -150,8 +150,6 @@ static int init_ring_common(struct intel
I915_WRITE_HEAD(ring, 0);
ring->write_tail(ring, 0);
 
-   /* Initialize the ring. */
-   I915_WRITE_START(ring, obj->gtt_offset);
head = I915_READ_HEAD(ring) & HEAD_ADDR;
 
/* G45 ring initialization fails to reset head to zero */
@@ -177,6 +175,11 @@ static int init_ring_common(struct intel
}
}
 
+   /* Initialize the ring. This must happen _after_ we've cleared the ring
+* registers with the above sequence (the readback of the HEAD registers
+* also enforces ordering), otherwise the hw might lose the new ring
+* register values. */
+   I915_WRITE_START(ring, obj->gtt_offset);
I915_WRITE_CTL(ring,
((ring->size - PAGE_SIZE) & RING_NR_PAGES)
| RING_REPORT_64K | RING_VALID);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[ 04/16] drm/i915: correctly order the ring init sequence

2012-08-19 Thread Greg Kroah-Hartman
From: Greg KH gre...@linuxfoundation.org

3.0-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Vetter daniel.vet...@ffwll.ch

commit 0d8957c8a90bbb5d34fab9a304459448a5131e06 upstream.

We may only start to set up the new register values after having
confirmed that the ring is truely off. Otherwise the hw might lose the
newly written register values. This is caught later on in the init
sequence, when we check whether the register writes have stuck.

Reviewed-by: Jani Nikula jani.nik...@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50522
Tested-by: Yang Guang guang.a.y...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org

---
 drivers/gpu/drm/i915/intel_ringbuffer.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -150,8 +150,6 @@ static int init_ring_common(struct intel
I915_WRITE_HEAD(ring, 0);
ring-write_tail(ring, 0);
 
-   /* Initialize the ring. */
-   I915_WRITE_START(ring, obj-gtt_offset);
head = I915_READ_HEAD(ring)  HEAD_ADDR;
 
/* G45 ring initialization fails to reset head to zero */
@@ -177,6 +175,11 @@ static int init_ring_common(struct intel
}
}
 
+   /* Initialize the ring. This must happen _after_ we've cleared the ring
+* registers with the above sequence (the readback of the HEAD registers
+* also enforces ordering), otherwise the hw might lose the new ring
+* register values. */
+   I915_WRITE_START(ring, obj-gtt_offset);
I915_WRITE_CTL(ring,
((ring-size - PAGE_SIZE)  RING_NR_PAGES)
| RING_REPORT_64K | RING_VALID);


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/