On Fri, Feb 10, 2012 at 12:50:01PM -0500, Yufeng Shen wrote: > GMBUS has several ports and each has it's own corresponding > I2C adpater. When multiple I2C adapters call gmbus_xfer() at > the same time there is a race condition in using the underlying > GMBUS controller. Fixing this by adding a mutex lock when calling > gmbus_xfer(). > > Signed-off-by: Yufeng Shen <miletus at chromium.org>
2 more nitpicks: - patch doesn't apply cleanly - can you please rebase against drm-intel-next-queued available at http://cgit.freedesktop.org/~danvet/drm-intel/ - please move the new gmbus_mutex to the other gmbus stuff in drm_i915_private and add a small comment to that it explains against concurrent use (from e.g. userspace) of the single gmbus controller. Yours, Daniel > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_i2c.c | 23 +++++++++++++++++------ > 2 files changed, 19 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 559fb6f..4ed9fd9 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -722,6 +722,8 @@ typedef struct drm_i915_private { > u8 corr; > spinlock_t *mchdev_lock; > > + struct mutex gmbus_mutex; > + > enum no_fbc_reason no_fbc_reason; > > struct drm_mm_node *compressed_fb; > diff --git a/drivers/gpu/drm/i915/intel_i2c.c > b/drivers/gpu/drm/i915/intel_i2c.c > index d98cee6..42569b1 100644 > --- a/drivers/gpu/drm/i915/intel_i2c.c > +++ b/drivers/gpu/drm/i915/intel_i2c.c > @@ -232,11 +232,15 @@ gmbus_xfer(struct i2c_adapter *adapter, > struct intel_gmbus, > adapter); > struct drm_i915_private *dev_priv = adapter->algo_data; > - int i, reg_offset; > + int i, reg_offset, ret; > > - if (bus->force_bit) > - return intel_i2c_quirk_xfer(dev_priv, > + mutex_lock(&dev_priv->gmbus_mutex); > + > + if (bus->force_bit) { > + ret = intel_i2c_quirk_xfer(dev_priv, > bus->force_bit, msgs, num); > + goto out; > + } > > reg_offset = HAS_PCH_SPLIT(dev_priv->dev) ? PCH_GMBUS0 - GMBUS0 : 0; > > @@ -320,7 +324,8 @@ done: > * start of the next xfer, till then let it sleep. > */ > I915_WRITE(GMBUS0 + reg_offset, 0); > - return i; > + ret = i; > + goto out; > > timeout: > DRM_INFO("GMBUS timed out, falling back to bit banging on pin %d > [%s]\n", > @@ -330,9 +335,13 @@ timeout: > /* Hardware may not support GMBUS over these pins? Try GPIO bitbanging > instead. */ > bus->force_bit = intel_gpio_create(dev_priv, bus->reg0 & 0xff); > if (!bus->force_bit) > - return -ENOMEM; > + ret = -ENOMEM; > + else > + ret = intel_i2c_quirk_xfer(dev_priv, bus->force_bit, msgs, num); > > - return intel_i2c_quirk_xfer(dev_priv, bus->force_bit, msgs, num); > +out: > + mutex_unlock(&dev_priv->gmbus_mutex); > + return ret; > } > > static u32 gmbus_func(struct i2c_adapter *adapter) > @@ -379,6 +388,8 @@ int intel_setup_gmbus(struct drm_device *dev) > if (dev_priv->gmbus == NULL) > return -ENOMEM; > > + mutex_init(&dev_priv->gmbus_mutex); > + > for (i = 0; i < GMBUS_NUM_PORTS; i++) { > struct intel_gmbus *bus = &dev_priv->gmbus[i]; > > -- > 1.7.3.4 > -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
