From 0c2f6f1a561136a09c5de91889d0aa35e13f73d4 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 17 Jun 2016 09:33:17 +0100 Subject: drm: Pass the drm_dp_aux->hw_mutex to i2c for its locking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than have both drm_dp_aux lock within its transfer, and i2c to lock around the transfer, use the same lock by filling in the locking callbacks that i2c wants to use. We require our own hw_mutex as we bypass i2c_transfer for drm_dp_dpcd_access(). Signed-off-by: Chris Wilson Cc: Dave Airlie Cc: Rafael Antognolli Cc: Ville Syrjälä Cc: dri-devel@lists.freedesktop.org Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1466152398-20157-2-git-send-email-chris@chris-wilson.co.uk --- drivers/gpu/drm/drm_dp_helper.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/drm_dp_helper.c') diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index eeaf5a7c3aa7..4b088afa21b2 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -708,8 +708,6 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, memset(&msg, 0, sizeof(msg)); - mutex_lock(&aux->hw_mutex); - for (i = 0; i < num; i++) { msg.address = msgs[i].addr; drm_dp_i2c_msg_set_request(&msg, &msgs[i]); @@ -764,8 +762,6 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, msg.size = 0; (void)drm_dp_i2c_do_msg(aux, &msg); - mutex_unlock(&aux->hw_mutex); - return err; } @@ -774,6 +770,26 @@ static const struct i2c_algorithm drm_dp_i2c_algo = { .master_xfer = drm_dp_i2c_xfer, }; +static struct drm_dp_aux *i2c_to_aux(struct i2c_adapter *i2c) +{ + return container_of(i2c, struct drm_dp_aux, ddc); +} + +static void lock_bus(struct i2c_adapter *i2c, unsigned int flags) +{ + mutex_lock(&i2c_to_aux(i2c)->hw_mutex); +} + +static int trylock_bus(struct i2c_adapter *i2c, unsigned int flags) +{ + return mutex_trylock(&i2c_to_aux(i2c)->hw_mutex); +} + +static void unlock_bus(struct i2c_adapter *i2c, unsigned int flags) +{ + mutex_unlock(&i2c_to_aux(i2c)->hw_mutex); +} + /** * drm_dp_aux_register() - initialise and register aux channel * @aux: DisplayPort AUX channel @@ -790,6 +806,10 @@ int drm_dp_aux_register(struct drm_dp_aux *aux) aux->ddc.algo_data = aux; aux->ddc.retries = 3; + aux->ddc.lock_bus = lock_bus; + aux->ddc.trylock_bus = trylock_bus; + aux->ddc.unlock_bus = unlock_bus; + aux->ddc.class = I2C_CLASS_DDC; aux->ddc.owner = THIS_MODULE; aux->ddc.dev.parent = aux->dev; -- cgit v1.2.3 From acd8f414c957406c8272cbc380dd50fea945dcf1 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 17 Jun 2016 09:33:18 +0100 Subject: drm: Minimally initialise drm_dp_aux MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When trying to split up the initialisation phase and the registration phase, one immediate problem encountered is trying to use our own i2c devices before registration with userspace (to read EDID during device discovery). drm_dp_aux in particular only offers an interface for setting up the device *after* we have exposed the connector via sysfs. In order to break the chicken-and-egg problem, export drm_dp_aux_init() to minimally prepare the i2c device for internal use before drm_connector_register(). Signed-off-by: Chris Wilson Cc: Dave Airlie Cc: Rafael Antognolli Cc: Ville Syrjälä Cc: dri-devel@lists.freedesktop.org [danvet: Amend kerneldoc slightly.] Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1466152398-20157-3-git-send-email-chris@chris-wilson.co.uk --- drivers/gpu/drm/drm_dp_helper.c | 28 +++++++++++++++++++++++----- include/drm/drm_dp_helper.h | 1 + 2 files changed, 24 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/drm_dp_helper.c') diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 4b088afa21b2..091053e995e5 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -791,15 +791,16 @@ static void unlock_bus(struct i2c_adapter *i2c, unsigned int flags) } /** - * drm_dp_aux_register() - initialise and register aux channel + * drm_dp_aux_init() - minimally initialise an aux channel * @aux: DisplayPort AUX channel * - * Returns 0 on success or a negative error code on failure. + * If you need to use the drm_dp_aux's i2c adapter prior to registering it + * with the outside world, call drm_dp_aux_init() first. You must still + * call drm_dp_aux_register() once the connector has been registered to + * allow userspace access to the auxiliary DP channel. */ -int drm_dp_aux_register(struct drm_dp_aux *aux) +void drm_dp_aux_init(struct drm_dp_aux *aux) { - int ret; - mutex_init(&aux->hw_mutex); aux->ddc.algo = &drm_dp_i2c_algo; @@ -809,6 +810,23 @@ int drm_dp_aux_register(struct drm_dp_aux *aux) aux->ddc.lock_bus = lock_bus; aux->ddc.trylock_bus = trylock_bus; aux->ddc.unlock_bus = unlock_bus; +} +EXPORT_SYMBOL(drm_dp_aux_init); + +/** + * drm_dp_aux_register() - initialise and register aux channel + * @aux: DisplayPort AUX channel + * + * Automatically calls drm_dp_aux_init() if this hasn't been done yet. + * + * Returns 0 on success or a negative error code on failure. + */ +int drm_dp_aux_register(struct drm_dp_aux *aux) +{ + int ret; + + if (!aux->ddc.algo) + drm_dp_aux_init(aux); aux->ddc.class = I2C_CLASS_DDC; aux->ddc.owner = THIS_MODULE; diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 5a848e734422..4d85cf2874af 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -805,6 +805,7 @@ int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link); int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link); int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link); +void drm_dp_aux_init(struct drm_dp_aux *aux); int drm_dp_aux_register(struct drm_dp_aux *aux); void drm_dp_aux_unregister(struct drm_dp_aux *aux); -- cgit v1.2.3 From a1f5524a66ff6284d1380cdd7723de82698ff9d3 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Thu, 28 Jul 2016 17:54:42 +0300 Subject: drm: aux ->transfer() can return 0, deal with it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restore the correct behaviour (as in check msg.reply) when aux ->transfer() returns 0. It got removed in commit 82922da39190 ("drm/dp_helper: Retry aux transactions on all errors") Now I can actually dump the "entire" DPCD on a Dell UP2314Q with ddrescue. It has some offsets in the DPCD that can't be read for some resaon, all you get is defers. Previously ddrescue would just give up at the first unredable offset on account of read() returning 0 means EOF. Here's the ddrescue log for the interested: 0x00000000 0x00001400 + 0x00001400 0x00000030 - 0x00001430 0x000001D0 + 0x00001600 0x00000030 - 0x00001630 0x0001F9D0 + 0x00021000 0x00000001 - 0x00021001 0x000DEFFF + Cc: Lyude Cc: Daniel Vetter Reviewed-by: Daniel Vetter Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_dp_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_dp_helper.c') diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 091053e995e5..8f11b8741e42 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -203,7 +203,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request, ret = aux->transfer(aux, &msg); - if (ret > 0) { + if (ret >= 0) { native_reply = msg.reply & DP_AUX_NATIVE_REPLY_MASK; if (native_reply == DP_AUX_NATIVE_REPLY_ACK) { if (ret == size) -- cgit v1.2.3