[RFC] drm/dp: move hw_mutex up the call stack

[Rob Clark]

On Fri, Oct 16, 2015 at 2:54 PM, Rob Clark <robdclark at gmail.com> wrote:
> 1) don't let other threads trying to bang on aux channel interrupt the
> defer timeout/logic
> 2) don't let other threads interrupt the i2c over aux logic
>
> ---
> This wasn't actually fixing things w/ problematic monitor, but seems
> like generally a good idea.  At least AFAIU you shouldn't allow the
> sequence of transfers for i2c over aux be interrupted but unrelated
> chatter.

So, I got confirmation today that this patch actually does fix things
w/ a different problematic Dell monitor.  So I think it actually is
the right thing to do.

BR,
-R

> This applies on top of the DPCD logging patch.
>
>  drivers/gpu/drm/drm_dp_helper.c | 27 +++++++++++++++++----------
>  1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index ccb1f8a..e409b5f 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -163,8 +163,6 @@ static ssize_t aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  {
>         ssize_t ret;
>
> -       mutex_lock(&aux->hw_mutex);
> -
>         DRM_DEBUG_DPCD("%s: req=0x%02x, address=0x%05x, size=%zu\n", aux->name,
>                         msg->request, msg->address, msg->size);
>
> @@ -196,8 +194,6 @@ static ssize_t aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>                 }
>         }
>
> -       mutex_unlock(&aux->hw_mutex);
> -
>         return ret;
>  }
>
> @@ -220,7 +216,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>  {
>         struct drm_dp_aux_msg msg;
>         unsigned int retry;
> -       int err;
> +       int err = 0;
>
>         memset(&msg, 0, sizeof(msg));
>         msg.address = offset;
> @@ -228,6 +224,8 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>         msg.buffer = buffer;
>         msg.size = size;
>
> +       mutex_lock(&aux->hw_mutex);
> +
>         /*
>          * The specification doesn't give any recommendation on how often to
>          * retry native transactions. We used to retry 7 times like for
> @@ -241,18 +239,19 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>                         if (err == -EBUSY)
>                                 continue;
>
> -                       return err;
> +                       goto unlock;
>                 }
>
>                 switch (msg.reply & DP_AUX_NATIVE_REPLY_MASK) {
>                 case DP_AUX_NATIVE_REPLY_ACK:
>                         if (err < size)
> -                               return -EPROTO;
> -                       return err;
> +                               err = -EPROTO;
> +                       goto unlock;
>
>                 case DP_AUX_NATIVE_REPLY_NACK:
>                         DRM_DEBUG_DPCD("native nack (result=%d, size=%zu)\n", err, msg.size);
> -                       return -EIO;
> +                       err = -EIO;
> +                       goto unlock;
>
>                 case DP_AUX_NATIVE_REPLY_DEFER:
>                         DRM_DEBUG_DPCD("native defer\n");
> @@ -262,7 +261,11 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>         }
>
>         DRM_ERROR("DPCD: too many retries, giving up!\n");
> -       return -EIO;
> +       err = -EIO;
> +
> +unlock:
> +       mutex_unlock(&aux->hw_mutex);
> +       return err;
>  }
>
>  /**
> @@ -698,6 +701,8 @@ 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;
>                 msg.request = (msgs[i].flags & I2C_M_RD) ?
> @@ -741,6 +746,8 @@ 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;
>  }
>
> --
> 2.5.0
>