Re: [PATCH] drm/radeon: add a i2c bus mutex

["Christian König" <deathsimple@vodafone.de>]

Am 08.05.2014 16:58, schrieb Alex Deucher:
> The i2c and aux buses use the same pads so add
> a mutex to protect access to the pads.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

I've applied this one, "drm/radeon: fix DCE83 check for mullins" and 
"drm/radeon: handle non-VGA class pci devices with ATRM" my 3.15 queue.

Thanks,
Christian.

> ---
>   drivers/gpu/drm/radeon/atombios_dp.c  | 18 ++++++++++++++----
>   drivers/gpu/drm/radeon/atombios_i2c.c | 17 +++++++++++++----
>   drivers/gpu/drm/radeon/radeon_i2c.c   |  9 +++++++++
>   drivers/gpu/drm/radeon/radeon_mode.h  |  1 +
>   4 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
> index 54e4f52..3bb0933 100644
> --- a/drivers/gpu/drm/radeon/atombios_dp.c
> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
> @@ -95,9 +95,12 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan *chan,
>   	int index = GetIndexIntoMasterTable(COMMAND, ProcessAuxChannelTransaction);
>   	unsigned char *base;
>   	int recv_bytes;
> +	int r = 0;
>   
>   	memset(&args, 0, sizeof(args));
>   
> +	mutex_lock(&chan->mutex);
> +
>   	base = (unsigned char *)(rdev->mode_info.atom_context->scratch + 1);
>   
>   	radeon_atom_copy_swap(base, send, send_bytes, true);
> @@ -117,19 +120,22 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan *chan,
>   	/* timeout */
>   	if (args.v1.ucReplyStatus == 1) {
>   		DRM_DEBUG_KMS("dp_aux_ch timeout\n");
> -		return -ETIMEDOUT;
> +		r = -ETIMEDOUT;
> +		goto done;
>   	}
>   
>   	/* flags not zero */
>   	if (args.v1.ucReplyStatus == 2) {
>   		DRM_DEBUG_KMS("dp_aux_ch flags not zero\n");
> -		return -EBUSY;
> +		r = -EBUSY;
> +		goto done;
>   	}
>   
>   	/* error */
>   	if (args.v1.ucReplyStatus == 3) {
>   		DRM_DEBUG_KMS("dp_aux_ch error\n");
> -		return -EIO;
> +		r = -EIO;
> +		goto done;
>   	}
>   
>   	recv_bytes = args.v1.ucDataOutLen;
> @@ -139,7 +145,11 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan *chan,
>   	if (recv && recv_size)
>   		radeon_atom_copy_swap(recv, base + 16, recv_bytes, false);
>   
> -	return recv_bytes;
> +	r = recv_bytes;
> +done:
> +	mutex_unlock(&chan->mutex);
> +
> +	return r;
>   }
>   
>   #define BARE_ADDRESS_SIZE 3
> diff --git a/drivers/gpu/drm/radeon/atombios_i2c.c b/drivers/gpu/drm/radeon/atombios_i2c.c
> index b5162c3..9c570fb 100644
> --- a/drivers/gpu/drm/radeon/atombios_i2c.c
> +++ b/drivers/gpu/drm/radeon/atombios_i2c.c
> @@ -43,15 +43,19 @@ static int radeon_process_i2c_ch(struct radeon_i2c_chan *chan,
>   	int index = GetIndexIntoMasterTable(COMMAND, ProcessI2cChannelTransaction);
>   	unsigned char *base;
>   	u16 out = cpu_to_le16(0);
> +	int r = 0;
>   
>   	memset(&args, 0, sizeof(args));
>   
> +	mutex_lock(&chan->mutex);
> +
>   	base = (unsigned char *)rdev->mode_info.atom_context->scratch;
>   
>   	if (flags & HW_I2C_WRITE) {
>   		if (num > ATOM_MAX_HW_I2C_WRITE) {
>   			DRM_ERROR("hw i2c: tried to write too many bytes (%d vs 3)\n", num);
> -			return -EINVAL;
> +			r = -EINVAL;
> +			goto done;
>   		}
>   		if (buf == NULL)
>   			args.ucRegIndex = 0;
> @@ -65,7 +69,8 @@ static int radeon_process_i2c_ch(struct radeon_i2c_chan *chan,
>   	} else {
>   		if (num > ATOM_MAX_HW_I2C_READ) {
>   			DRM_ERROR("hw i2c: tried to read too many bytes (%d vs 255)\n", num);
> -			return -EINVAL;
> +			r = -EINVAL;
> +			goto done;
>   		}
>   		args.ucRegIndex = 0;
>   		args.lpI2CDataOut = 0;
> @@ -82,13 +87,17 @@ static int radeon_process_i2c_ch(struct radeon_i2c_chan *chan,
>   	/* error */
>   	if (args.ucStatus != HW_ASSISTED_I2C_STATUS_SUCCESS) {
>   		DRM_DEBUG_KMS("hw_i2c error\n");
> -		return -EIO;
> +		r = -EIO;
> +		goto done;
>   	}
>   
>   	if (!(flags & HW_I2C_WRITE))
>   		radeon_atom_copy_swap(buf, base, num, false);
>   
> -	return 0;
> +done:
> +	mutex_unlock(&chan->mutex);
> +
> +	return r;
>   }
>   
>   int radeon_atom_hw_i2c_xfer(struct i2c_adapter *i2c_adap,
> diff --git a/drivers/gpu/drm/radeon/radeon_i2c.c b/drivers/gpu/drm/radeon/radeon_i2c.c
> index 7b94414..427ee4d 100644
> --- a/drivers/gpu/drm/radeon/radeon_i2c.c
> +++ b/drivers/gpu/drm/radeon/radeon_i2c.c
> @@ -94,6 +94,8 @@ static int pre_xfer(struct i2c_adapter *i2c_adap)
>   	struct radeon_i2c_bus_rec *rec = &i2c->rec;
>   	uint32_t temp;
>   
> +	mutex_lock(&i2c->mutex);
> +
>   	/* RV410 appears to have a bug where the hw i2c in reset
>   	 * holds the i2c port in a bad state - switch hw i2c away before
>   	 * doing DDC - do this for all r200s/r300s/r400s for safety sake
> @@ -170,6 +172,8 @@ static void post_xfer(struct i2c_adapter *i2c_adap)
>   	temp = RREG32(rec->mask_data_reg) & ~rec->mask_data_mask;
>   	WREG32(rec->mask_data_reg, temp);
>   	temp = RREG32(rec->mask_data_reg);
> +
> +	mutex_unlock(&i2c->mutex);
>   }
>   
>   static int get_clock(void *i2c_priv)
> @@ -813,6 +817,8 @@ static int radeon_hw_i2c_xfer(struct i2c_adapter *i2c_adap,
>   	struct radeon_i2c_bus_rec *rec = &i2c->rec;
>   	int ret = 0;
>   
> +	mutex_lock(&i2c->mutex);
> +
>   	switch (rdev->family) {
>   	case CHIP_R100:
>   	case CHIP_RV100:
> @@ -879,6 +885,8 @@ static int radeon_hw_i2c_xfer(struct i2c_adapter *i2c_adap,
>   		break;
>   	}
>   
> +	mutex_unlock(&i2c->mutex);
> +
>   	return ret;
>   }
>   
> @@ -919,6 +927,7 @@ struct radeon_i2c_chan *radeon_i2c_create(struct drm_device *dev,
>   	i2c->adapter.dev.parent = &dev->pdev->dev;
>   	i2c->dev = dev;
>   	i2c_set_adapdata(&i2c->adapter, i2c);
> +	mutex_init(&i2c->mutex);
>   	if (rec->mm_i2c ||
>   	    (rec->hw_capable &&
>   	     radeon_hw_i2c &&
> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
> index 6ddf31a..b265a8b 100644
> --- a/drivers/gpu/drm/radeon/radeon_mode.h
> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
> @@ -191,6 +191,7 @@ struct radeon_i2c_chan {
>   	struct radeon_i2c_bus_rec rec;
>   	struct drm_dp_aux aux;
>   	bool has_aux;
> +	struct mutex mutex;
>   };
>   
>   /* mostly for macs, but really any system without connector tables */