From a85066840d29fc68d95ce7dbd6bcf15ef2775d66 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 26 Jul 2017 15:26:47 +0200 Subject: drm/i915: Rework sdvo proxy i2c locking lockdep complaints about a locking recursion for the i2c bus lock because both the sdvo ddc proxy bus and the gmbus nested within use the same locking class. It's not really a deadlock since we never nest the other way round, but it's annoying. Fix it by pulling the gmbus locking into the i2c lock_ops for both i2c_adapater and making sure that the ddc_proxy_xfer function is entirely lockless. Re-layouting the extracted function resulted in some whitespace cleanups, I figured we might as well keep them. v2: Review from Chris: - s/locked/unlocked/ since I got the naming backwards - Use the vfuncs of the proxied adatper instead of re-rolling copies. That's more consistent with the other proxying we're doing. Cc: Chris Wilson Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20170726132647.31833-1-daniel.vetter@ffwll.ch --- drivers/gpu/drm/i915/intel_i2c.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 3c9e00d4ba5a..6698826954e1 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -592,7 +592,6 @@ gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) int ret; intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); - mutex_lock(&dev_priv->gmbus_mutex); if (bus->force_bit) { ret = i2c_bit_algo.master_xfer(adapter, msgs, num); @@ -604,7 +603,6 @@ gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) bus->force_bit |= GMBUS_FORCE_BIT_RETRY; } - mutex_unlock(&dev_priv->gmbus_mutex); intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); return ret; @@ -624,6 +622,39 @@ static const struct i2c_algorithm gmbus_algorithm = { .functionality = gmbus_func }; +static void gmbus_lock_bus(struct i2c_adapter *adapter, + unsigned int flags) +{ + struct intel_gmbus *bus = to_intel_gmbus(adapter); + struct drm_i915_private *dev_priv = bus->dev_priv; + + mutex_lock(&dev_priv->gmbus_mutex); +} + +static int gmbus_trylock_bus(struct i2c_adapter *adapter, + unsigned int flags) +{ + struct intel_gmbus *bus = to_intel_gmbus(adapter); + struct drm_i915_private *dev_priv = bus->dev_priv; + + return mutex_trylock(&dev_priv->gmbus_mutex); +} + +static void gmbus_unlock_bus(struct i2c_adapter *adapter, + unsigned int flags) +{ + struct intel_gmbus *bus = to_intel_gmbus(adapter); + struct drm_i915_private *dev_priv = bus->dev_priv; + + mutex_unlock(&dev_priv->gmbus_mutex); +} + +const struct i2c_lock_operations gmbus_lock_ops = { + .lock_bus = gmbus_lock_bus, + .trylock_bus = gmbus_trylock_bus, + .unlock_bus = gmbus_unlock_bus, +}; + /** * intel_gmbus_setup - instantiate all Intel i2c GMBuses * @dev_priv: i915 device private @@ -665,6 +696,7 @@ int intel_setup_gmbus(struct drm_i915_private *dev_priv) bus->dev_priv = dev_priv; bus->adapter.algo = &gmbus_algorithm; + bus->adapter.lock_ops = &gmbus_lock_ops; /* * We wish to retry with bit banging -- cgit v1.2.3 From 43d57414b6e9f31809c2bd25b0545b887ce6ab09 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Fri, 1 Sep 2017 17:31:22 +0300 Subject: drm/i915: Make i2c lock ops static MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make gmbus_lock_ops and proxy_lock_ops static to appease sparse intel_i2c.c:652:34: warning: symbol 'gmbus_lock_ops' was not declared. Should it be static? intel_sdvo.c:2981:34: warning: symbol 'proxy_lock_ops' was not declared. Should it be static? Cc: Daniel Vetter Fixes: a85066840d29 ("drm/i915: Rework sdvo proxy i2c locking") Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20170901143123.7590-2-ville.syrjala@linux.intel.com Reviewed-by: Chris Wilson (cherry picked from commit 0db1aa424e3ee91fcb9d583edb30a933c64c5b88) Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_i2c.c | 2 +- drivers/gpu/drm/i915/intel_sdvo.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_i2c.c') diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 6698826954e1..eb5827110d8f 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -649,7 +649,7 @@ static void gmbus_unlock_bus(struct i2c_adapter *adapter, mutex_unlock(&dev_priv->gmbus_mutex); } -const struct i2c_lock_operations gmbus_lock_ops = { +static const struct i2c_lock_operations gmbus_lock_ops = { .lock_bus = gmbus_lock_bus, .trylock_bus = gmbus_trylock_bus, .unlock_bus = gmbus_unlock_bus, diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 3dc38c2ef4c3..29a3b0f5bec7 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -2980,7 +2980,7 @@ static void proxy_unlock_bus(struct i2c_adapter *adapter, sdvo->i2c->lock_ops->unlock_bus(sdvo->i2c, flags); } -const struct i2c_lock_operations proxy_lock_ops = { +static const struct i2c_lock_operations proxy_lock_ops = { .lock_bus = proxy_lock_bus, .trylock_bus = proxy_trylock_bus, .unlock_bus = proxy_unlock_bus, -- cgit v1.2.3