Revert "drm/i915: Enable GMBUS for post-gen2 chipsets"
On Fri, Jun 17, 2011 at 6:11 AM, Jean Delvare wrote: > Hi Dave, > > On Tue, 14 Jun 2011 13:39:35 +1000, Dave Airlie wrote: >> On Sat, Jun 11, 2011 at 10:58 PM, Jean Delvare wrote: >> > Hi Florian, >> > >> > On Sat, 11 Jun 2011 13:28:15 +0200, Florian Mickler wrote: >> >> On Sat, 04 Jun 2011 19:34:56 - >> >> Jean Delvare wrote: >> >> >> >> > Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a >> >> > hang when loading the eeprom driver (see bug #35572.) GMBUS will be >> >> > re-enabled later, differently. >> >> > >> >> > Signed-off-by: Jean Delvare >> >> > Reported-by: Marek Otahal >> >> > Tested-by: Yermandu Patapitafious >> >> > Tested-by: Andrew Lutomirski >> >> > Acked-by: Chris Wilson >> >> > Cc: David Airlie >> >> >> >> is this[1] resolved some other way in the meantime? >> >> >> >> Regards, >> >> Flo >> >> >> >> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=35572 >> > >> > Not that I know of (and I don't see any other way at least for 2.6.39.) >> > This is a shame, really, my revert patch should have been applied >> > several days ago already. >> > >> > Keith, Chris, David, can you please get it rolling? This is a >> > regression presumably affecting a lot of users, we should really fix it >> > quickly, both in 2.6.39.x and 3.0-rc. >> >> This patch really had no info other than the bug link to tell me wtf its >> doing, > > The patch I sent on June 4th (Message-ID: > <20110604213456.7ac5588e at endymion.delvare>) says: > > Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a > hang when loading the eeprom driver (see bug #35572.) GMBUS will be > re-enabled later, differently. > > Seems clear enough to me. Hi Jean, yeah I talked to Chris and I then spotted I was applying this in the wrong place when looking at the code by hand, Once Chris explained I got it, will push soon. Dave.
Revert "drm/i915: Enable GMBUS for post-gen2 chipsets"
Hi Dave, On Tue, 14 Jun 2011 13:39:35 +1000, Dave Airlie wrote: > On Sat, Jun 11, 2011 at 10:58 PM, Jean Delvare wrote: > > Hi Florian, > > > > On Sat, 11 Jun 2011 13:28:15 +0200, Florian Mickler wrote: > >> On Sat, 04 Jun 2011 19:34:56 - > >> Jean Delvare wrote: > >> > >> > Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a > >> > hang when loading the eeprom driver (see bug #35572.) GMBUS will be > >> > re-enabled later, differently. > >> > > >> > Signed-off-by: Jean Delvare > >> > Reported-by: Marek Otahal > >> > Tested-by: Yermandu Patapitafious > >> > Tested-by: Andrew Lutomirski > >> > Acked-by: Chris Wilson > >> > Cc: David Airlie > >> > >> is this[1] resolved some other way in the meantime? > >> > >> Regards, > >> Flo > >> > >> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=35572 > > > > Not that I know of (and I don't see any other way at least for 2.6.39.) > > This is a shame, really, my revert patch should have been applied > > several days ago already. > > > > Keith, Chris, David, can you please get it rolling? This is a > > regression presumably affecting a lot of users, we should really fix it > > quickly, both in 2.6.39.x and 3.0-rc. > > This patch really had no info other than the bug link to tell me wtf its > doing, The patch I sent on June 4th (Message-ID: <20110604213456.7ac5588e at endymion.delvare>) says: Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a hang when loading the eeprom driver (see bug #35572.) GMBUS will be re-enabled later, differently. Seems clear enough to me. > I actually don't think reverting this is the correct fix, since it looks like > the code path thats screws with the mutex still happens on GEN2 machines > which unlucky for them. No. Without the revert, the problem happens on every chip except GEN2. With the revert, the problems shouldn't happen on any chip. At least this is how I understand the code. Anyway, you may not like the fix, but the fact is that commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f caused a regression, so at least for kernel 2.6.39, reverting it is the right thing to do. For 3.0.0, either someone comes up with an alternative fix and we can apply it, or reverting the faulty commit is the way to go too. We have a known regression affecting many users, we have the fix, let's please apply it quickly. Further discussions should happen _later_. > Chris, also I don't see an ack anywhere on the list, only some > discussion in the bug, I would indeed love to get a ack from Chris. > I suspect the correct fix is to remove all the offending code instead > of just putting back the piece > of plaster that fell out of the wall. Which "offending code" are you talking about? We are fixing a regression here. It has to do with being fast and safe, not with "the correct fix". Thanks, -- Jean Delvare
Revert "drm/i915: Enable GMBUS for post-gen2 chipsets"
On Thu, 16 Jun 2011 22:11:07 +0200, Jean Delvare wrote: > Hi Dave, > > On Tue, 14 Jun 2011 13:39:35 +1000, Dave Airlie wrote: > > Chris, also I don't see an ack anywhere on the list, only some > > discussion in the bug, > > I would indeed love to get a ack from Chris. Sure, let me ack it again. Hmm, was the last ack on linux-kernel, not dri-devel? > > I suspect the correct fix is to remove all the offending code instead > > of just putting back the piece > > of plaster that fell out of the wall. > > Which "offending code" are you talking about? Dave and myself chatted on irc and hopefully we cleared up the confusion that this patch is indeed just restoring the code to 2.6.39. And that further works needs to be done before trying GMBUS again. -Chris -- Chris Wilson, Intel Open Source Technology Centre
Re: Revert drm/i915: Enable GMBUS for post-gen2 chipsets
Hi Dave, On Tue, 14 Jun 2011 13:39:35 +1000, Dave Airlie wrote: On Sat, Jun 11, 2011 at 10:58 PM, Jean Delvare kh...@linux-fr.org wrote: Hi Florian, On Sat, 11 Jun 2011 13:28:15 +0200, Florian Mickler wrote: On Sat, 04 Jun 2011 19:34:56 - Jean Delvare kh...@linux-fr.org wrote: Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a hang when loading the eeprom driver (see bug #35572.) GMBUS will be re-enabled later, differently. Signed-off-by: Jean Delvare kh...@linux-fr.org Reported-by: Marek Otahal markota...@gmail.com Tested-by: Yermandu Patapitafious yermandu@gmail.com Tested-by: Andrew Lutomirski l...@mit.edu Acked-by: Chris Wilson ch...@chris-wilson.co.uk Cc: David Airlie airl...@linux.ie is this[1] resolved some other way in the meantime? Regards, Flo [1]: https://bugzilla.kernel.org/show_bug.cgi?id=35572 Not that I know of (and I don't see any other way at least for 2.6.39.) This is a shame, really, my revert patch should have been applied several days ago already. Keith, Chris, David, can you please get it rolling? This is a regression presumably affecting a lot of users, we should really fix it quickly, both in 2.6.39.x and 3.0-rc. This patch really had no info other than the bug link to tell me wtf its doing, The patch I sent on June 4th (Message-ID: 20110604213456.7ac5588e@endymion.delvare) says: Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a hang when loading the eeprom driver (see bug #35572.) GMBUS will be re-enabled later, differently. Seems clear enough to me. I actually don't think reverting this is the correct fix, since it looks like the code path thats screws with the mutex still happens on GEN2 machines which unlucky for them. No. Without the revert, the problem happens on every chip except GEN2. With the revert, the problems shouldn't happen on any chip. At least this is how I understand the code. Anyway, you may not like the fix, but the fact is that commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f caused a regression, so at least for kernel 2.6.39, reverting it is the right thing to do. For 3.0.0, either someone comes up with an alternative fix and we can apply it, or reverting the faulty commit is the way to go too. We have a known regression affecting many users, we have the fix, let's please apply it quickly. Further discussions should happen _later_. Chris, also I don't see an ack anywhere on the list, only some discussion in the bug, I would indeed love to get a ack from Chris. I suspect the correct fix is to remove all the offending code instead of just putting back the piece of plaster that fell out of the wall. Which offending code are you talking about? We are fixing a regression here. It has to do with being fast and safe, not with the correct fix. Thanks, -- Jean Delvare ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Revert drm/i915: Enable GMBUS for post-gen2 chipsets
On Thu, 16 Jun 2011 22:11:07 +0200, Jean Delvare kh...@linux-fr.org wrote: Hi Dave, On Tue, 14 Jun 2011 13:39:35 +1000, Dave Airlie wrote: Chris, also I don't see an ack anywhere on the list, only some discussion in the bug, I would indeed love to get a ack from Chris. Sure, let me ack it again. Hmm, was the last ack on linux-kernel, not dri-devel? I suspect the correct fix is to remove all the offending code instead of just putting back the piece of plaster that fell out of the wall. Which offending code are you talking about? Dave and myself chatted on irc and hopefully we cleared up the confusion that this patch is indeed just restoring the code to 2.6.39. And that further works needs to be done before trying GMBUS again. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Revert drm/i915: Enable GMBUS for post-gen2 chipsets
On Fri, Jun 17, 2011 at 6:11 AM, Jean Delvare kh...@linux-fr.org wrote: Hi Dave, On Tue, 14 Jun 2011 13:39:35 +1000, Dave Airlie wrote: On Sat, Jun 11, 2011 at 10:58 PM, Jean Delvare kh...@linux-fr.org wrote: Hi Florian, On Sat, 11 Jun 2011 13:28:15 +0200, Florian Mickler wrote: On Sat, 04 Jun 2011 19:34:56 - Jean Delvare kh...@linux-fr.org wrote: Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a hang when loading the eeprom driver (see bug #35572.) GMBUS will be re-enabled later, differently. Signed-off-by: Jean Delvare kh...@linux-fr.org Reported-by: Marek Otahal markota...@gmail.com Tested-by: Yermandu Patapitafious yermandu@gmail.com Tested-by: Andrew Lutomirski l...@mit.edu Acked-by: Chris Wilson ch...@chris-wilson.co.uk Cc: David Airlie airl...@linux.ie is this[1] resolved some other way in the meantime? Regards, Flo [1]: https://bugzilla.kernel.org/show_bug.cgi?id=35572 Not that I know of (and I don't see any other way at least for 2.6.39.) This is a shame, really, my revert patch should have been applied several days ago already. Keith, Chris, David, can you please get it rolling? This is a regression presumably affecting a lot of users, we should really fix it quickly, both in 2.6.39.x and 3.0-rc. This patch really had no info other than the bug link to tell me wtf its doing, The patch I sent on June 4th (Message-ID: 20110604213456.7ac5588e@endymion.delvare) says: Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a hang when loading the eeprom driver (see bug #35572.) GMBUS will be re-enabled later, differently. Seems clear enough to me. Hi Jean, yeah I talked to Chris and I then spotted I was applying this in the wrong place when looking at the code by hand, Once Chris explained I got it, will push soon. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Revert "drm/i915: Enable GMBUS for post-gen2 chipsets"
On Sat, Jun 11, 2011 at 10:58 PM, Jean Delvare wrote: > Hi Florian, > > On Sat, 11 Jun 2011 13:28:15 +0200, Florian Mickler wrote: >> On Sat, 04 Jun 2011 19:34:56 - >> Jean Delvare wrote: >> >> > Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a >> > hang when loading the eeprom driver (see bug #35572.) GMBUS will be >> > re-enabled later, differently. >> > >> > Signed-off-by: Jean Delvare >> > Reported-by: Marek Otahal >> > Tested-by: Yermandu Patapitafious >> > Tested-by: Andrew Lutomirski >> > Acked-by: Chris Wilson >> > Cc: David Airlie >> >> is this[1] resolved some other way in the meantime? >> >> Regards, >> Flo >> >> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=35572 > > Not that I know of (and I don't see any other way at least for 2.6.39.) > This is a shame, really, my revert patch should have been applied > several days ago already. > > Keith, Chris, David, can you please get it rolling? This is a > regression presumably affecting a lot of users, we should really fix it > quickly, both in 2.6.39.x and 3.0-rc. This patch really had no info other than the bug link to tell me wtf its doing, I actually don't think reverting this is the correct fix, since it looks like the code path thats screws with the mutex still happens on GEN2 machines which unlucky for them. Chris, also I don't see an ack anywhere on the list, only some discussion in the bug, I suspect the correct fix is to remove all the offending code instead of just putting back the piece of plaster that fell out of the wall. Dave.
Re: Revert drm/i915: Enable GMBUS for post-gen2 chipsets
On Sat, Jun 11, 2011 at 10:58 PM, Jean Delvare kh...@linux-fr.org wrote: Hi Florian, On Sat, 11 Jun 2011 13:28:15 +0200, Florian Mickler wrote: On Sat, 04 Jun 2011 19:34:56 - Jean Delvare kh...@linux-fr.org wrote: Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a hang when loading the eeprom driver (see bug #35572.) GMBUS will be re-enabled later, differently. Signed-off-by: Jean Delvare kh...@linux-fr.org Reported-by: Marek Otahal markota...@gmail.com Tested-by: Yermandu Patapitafious yermandu@gmail.com Tested-by: Andrew Lutomirski l...@mit.edu Acked-by: Chris Wilson ch...@chris-wilson.co.uk Cc: David Airlie airl...@linux.ie is this[1] resolved some other way in the meantime? Regards, Flo [1]: https://bugzilla.kernel.org/show_bug.cgi?id=35572 Not that I know of (and I don't see any other way at least for 2.6.39.) This is a shame, really, my revert patch should have been applied several days ago already. Keith, Chris, David, can you please get it rolling? This is a regression presumably affecting a lot of users, we should really fix it quickly, both in 2.6.39.x and 3.0-rc. This patch really had no info other than the bug link to tell me wtf its doing, I actually don't think reverting this is the correct fix, since it looks like the code path thats screws with the mutex still happens on GEN2 machines which unlucky for them. Chris, also I don't see an ack anywhere on the list, only some discussion in the bug, I suspect the correct fix is to remove all the offending code instead of just putting back the piece of plaster that fell out of the wall. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Revert "drm/i915: Enable GMBUS for post-gen2 chipsets"
Hi Florian, On Sat, 11 Jun 2011 13:28:15 +0200, Florian Mickler wrote: > On Sat, 04 Jun 2011 19:34:56 - > Jean Delvare wrote: > > > Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a > > hang when loading the eeprom driver (see bug #35572.) GMBUS will be > > re-enabled later, differently. > > > > Signed-off-by: Jean Delvare > > Reported-by: Marek Otahal > > Tested-by: Yermandu Patapitafious > > Tested-by: Andrew Lutomirski > > Acked-by: Chris Wilson > > Cc: David Airlie > > is this[1] resolved some other way in the meantime? > > Regards, > Flo > > [1]: https://bugzilla.kernel.org/show_bug.cgi?id=35572 Not that I know of (and I don't see any other way at least for 2.6.39.) This is a shame, really, my revert patch should have been applied several days ago already. Keith, Chris, David, can you please get it rolling? This is a regression presumably affecting a lot of users, we should really fix it quickly, both in 2.6.39.x and 3.0-rc. -- Jean Delvare
Revert "drm/i915: Enable GMBUS for post-gen2 chipsets"
On Sat, 04 Jun 2011 19:34:56 - Jean Delvare wrote: > Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a > hang when loading the eeprom driver (see bug #35572.) GMBUS will be > re-enabled later, differently. > > Signed-off-by: Jean Delvare > Reported-by: Marek Otahal > Tested-by: Yermandu Patapitafious > Tested-by: Andrew Lutomirski > Acked-by: Chris Wilson > Cc: David Airlie Hi, is this[1] resolved some other way in the meantime? Regards, Flo [1]: https://bugzilla.kernel.org/show_bug.cgi?id=35572
Re: Revert drm/i915: Enable GMBUS for post-gen2 chipsets
Hi Florian, On Sat, 11 Jun 2011 13:28:15 +0200, Florian Mickler wrote: On Sat, 04 Jun 2011 19:34:56 - Jean Delvare kh...@linux-fr.org wrote: Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a hang when loading the eeprom driver (see bug #35572.) GMBUS will be re-enabled later, differently. Signed-off-by: Jean Delvare kh...@linux-fr.org Reported-by: Marek Otahal markota...@gmail.com Tested-by: Yermandu Patapitafious yermandu@gmail.com Tested-by: Andrew Lutomirski l...@mit.edu Acked-by: Chris Wilson ch...@chris-wilson.co.uk Cc: David Airlie airl...@linux.ie is this[1] resolved some other way in the meantime? Regards, Flo [1]: https://bugzilla.kernel.org/show_bug.cgi?id=35572 Not that I know of (and I don't see any other way at least for 2.6.39.) This is a shame, really, my revert patch should have been applied several days ago already. Keith, Chris, David, can you please get it rolling? This is a regression presumably affecting a lot of users, we should really fix it quickly, both in 2.6.39.x and 3.0-rc. -- Jean Delvare ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Revert "drm/i915: Enable GMBUS for post-gen2 chipsets"
Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a hang when loading the eeprom driver (see bug #35572.) GMBUS will be re-enabled later, differently. Signed-off-by: Jean Delvare Reported-by: Marek Otahal Tested-by: Yermandu Patapitafious Tested-by: Andrew Lutomirski Acked-by: Chris Wilson Cc: David Airlie --- drivers/gpu/drm/i915/intel_i2c.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) --- linux-2.6.39.orig/drivers/gpu/drm/i915/intel_i2c.c 2011-05-20 10:42:40.0 +0200 +++ linux-2.6.39/drivers/gpu/drm/i915/intel_i2c.c 2011-06-02 16:26:56.0 +0200 @@ -401,8 +401,7 @@ int intel_setup_gmbus(struct drm_device bus->reg0 = i | GMBUS_RATE_100KHZ; /* XXX force bit banging until GMBUS is fully debugged */ - if (IS_GEN2(dev)) - bus->force_bit = intel_gpio_create(dev_priv, i); + bus->force_bit = intel_gpio_create(dev_priv, i); } intel_i2c_reset(dev_priv->dev); -- Jean Delvare
[PATCH] Revert drm/i915: Enable GMBUS for post-gen2 chipsets
Revert commit 8f9a3f9b63b8cd3f03be9dc53533f90bd4120e5f. This fixes a hang when loading the eeprom driver (see bug #35572.) GMBUS will be re-enabled later, differently. Signed-off-by: Jean Delvare kh...@linux-fr.org Reported-by: Marek Otahal markota...@gmail.com Tested-by: Yermandu Patapitafious yermandu@gmail.com Tested-by: Andrew Lutomirski l...@mit.edu Acked-by: Chris Wilson ch...@chris-wilson.co.uk Cc: David Airlie airl...@linux.ie --- drivers/gpu/drm/i915/intel_i2c.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) --- linux-2.6.39.orig/drivers/gpu/drm/i915/intel_i2c.c 2011-05-20 10:42:40.0 +0200 +++ linux-2.6.39/drivers/gpu/drm/i915/intel_i2c.c 2011-06-02 16:26:56.0 +0200 @@ -401,8 +401,7 @@ int intel_setup_gmbus(struct drm_device bus-reg0 = i | GMBUS_RATE_100KHZ; /* XXX force bit banging until GMBUS is fully debugged */ - if (IS_GEN2(dev)) - bus-force_bit = intel_gpio_create(dev_priv, i); + bus-force_bit = intel_gpio_create(dev_priv, i); } intel_i2c_reset(dev_priv-dev); -- Jean Delvare ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
