| Message ID | 20110604213456.7ac5588e@endymion.delvare (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, 04 Jun 2011 19:34:56 -0000 Jean Delvare <khali@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 <khali@linux-fr.org> > Reported-by: Marek Otahal <markotahal@gmail.com> > Tested-by: Yermandu Patapitafious <yermandu.dev@gmail.com> > Tested-by: Andrew Lutomirski <luto@mit.edu> > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: David Airlie <airlied@linux.ie> Hi, is this[1] resolved some other way in the meantime? Regards, Flo [1]: https://bugzilla.kernel.org/show_bug.cgi?id=35572
Hi Florian, On Sat, 11 Jun 2011 13:28:15 +0200, Florian Mickler wrote: > On Sat, 04 Jun 2011 19:34:56 -0000 > Jean Delvare <khali@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 <khali@linux-fr.org> > > Reported-by: Marek Otahal <markotahal@gmail.com> > > Tested-by: Yermandu Patapitafious <yermandu.dev@gmail.com> > > Tested-by: Andrew Lutomirski <luto@mit.edu> > > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: David Airlie <airlied@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.
On Sat, Jun 11, 2011 at 10:58 PM, Jean Delvare <khali@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 -0000 >> Jean Delvare <khali@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 <khali@linux-fr.org> >> > Reported-by: Marek Otahal <markotahal@gmail.com> >> > Tested-by: Yermandu Patapitafious <yermandu.dev@gmail.com> >> > Tested-by: Andrew Lutomirski <luto@mit.edu> >> > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> >> > Cc: David Airlie <airlied@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.
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 <khali@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 -0000 > >> Jean Delvare <khali@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 <khali@linux-fr.org> > >> > Reported-by: Marek Otahal <markotahal@gmail.com> > >> > Tested-by: Yermandu Patapitafious <yermandu.dev@gmail.com> > >> > Tested-by: Andrew Lutomirski <luto@mit.edu> > >> > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > >> > Cc: David Airlie <airlied@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,
On Thu, 16 Jun 2011 22:11:07 +0200, Jean Delvare <khali@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
On Fri, Jun 17, 2011 at 6:11 AM, Jean Delvare <khali@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 <khali@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 -0000 >> >> Jean Delvare <khali@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 <khali@linux-fr.org> >> >> > Reported-by: Marek Otahal <markotahal@gmail.com> >> >> > Tested-by: Yermandu Patapitafious <yermandu.dev@gmail.com> >> >> > Tested-by: Andrew Lutomirski <luto@mit.edu> >> >> > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> >> >> > Cc: David Airlie <airlied@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.
--- linux-2.6.39.orig/drivers/gpu/drm/i915/intel_i2c.c 2011-05-20 10:42:40.000000000 +0200 +++ linux-2.6.39/drivers/gpu/drm/i915/intel_i2c.c 2011-06-02 16:26:56.000000000 +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);