diff mbox series

[v3,3/3] drm/bridge: it6505: Add MCCS support

Message ID 20240923094826.13471-4-Hermes.Wu@ite.com.tw (mailing list archive)
State New, archived
Headers show
Series drm/bridge: it6505: update dp aux funxtion | expand

Commit Message

Hermes Wu Sept. 23, 2024, 9:48 a.m. UTC
From: Hermes Wu <Hermes.wu@ite.com.tw>

Changes in v3:
 -remove non used definition for aux i2x cmd reply

Add Aux-I2C functionality to support MCCS.

Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
---
 drivers/gpu/drm/bridge/ite-it6505.c | 174 +++++++++++++++++++++++++++-
 1 file changed, 172 insertions(+), 2 deletions(-)

Comments

Dmitry Baryshkov Sept. 23, 2024, noon UTC | #1
On Mon, Sep 23, 2024 at 05:48:29PM GMT, Hermes Wu wrote:
> From: Hermes Wu <Hermes.wu@ite.com.tw>
> 
> Changes in v3:
>  -remove non used definition for aux i2x cmd reply
> 
> Add Aux-I2C functionality to support MCCS.
> 
> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> ---
>  drivers/gpu/drm/bridge/ite-it6505.c | 174 +++++++++++++++++++++++++++-
>  1 file changed, 172 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index 156440c6517e..5aedc5570739 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -125,6 +125,9 @@
>  #define REG_AUX_ADR_16_19 0x26
>  #define REG_AUX_OUT_DATA0 0x27
>  
> +#define REG_AUX_I2C_ADR 0x25
> +#define REG_AUX_I2C_OP 0x26
> +

Are these registers CMD-specific? Because I see that you already have
defines for 0x25 and 0x26.

>  #define REG_AUX_CMD_REQ 0x2B
>  #define AUX_BUSY BIT(5)
>  
> @@ -266,6 +269,19 @@
>  #define REG_SSC_CTRL1 0x189
>  #define REG_SSC_CTRL2 0x18A
>  
> +#define REG_AUX_USER_CTRL 0x190
> +#define EN_USER_AUX BIT(0)
> +#define USER_AUX_DONE BIT(1)
> +#define AUX_EVENT BIT(4)
> +
> +#define REG_AUX_USER_DATA_REC 0x191
> +#define M_AUX_IN_REC   0xF0
> +#define M_AUX_OUT_REC  0x0F
> +
> +#define REG_AUX_USER_TXB 0x190

And two defines for 0x190 too.

> +#define REG_AUX_USER_REPLY 0x19A
> +#define REG_AUX_USER_RXB(n) (n + 0x19B)
> +
>  #define RBR DP_LINK_BW_1_62
>  #define HBR DP_LINK_BW_2_7
>  #define HBR2 DP_LINK_BW_5_4
> @@ -301,6 +317,8 @@
>  #define MAX_EQ_LEVEL 0x03
>  #define AUX_WAIT_TIMEOUT_MS 15
>  #define AUX_FIFO_MAX_SIZE 16
> +#define AUX_I2C_MAX_SIZE 4
> +#define AUX_I2C_DEFER_RETRY 4
>  #define PIXEL_CLK_DELAY 1
>  #define PIXEL_CLK_INVERSE 0
>  #define ADJUST_PHASE_THRESHOLD 80000
> @@ -323,7 +341,12 @@
>  enum aux_cmd_type {
>  	CMD_AUX_NATIVE_READ = 0x0,
>  	CMD_AUX_NATIVE_WRITE = 0x5,
> +	CMD_AUX_GI2C_ADR = 0x08,
> +	CMD_AUX_GI2C_READ = 0x09,
> +	CMD_AUX_GI2C_WRITE = 0x0A,
>  	CMD_AUX_I2C_EDID_READ = 0xB,
> +	CMD_AUX_I2C_READ = 0x0D,
> +	CMD_AUX_I2C_WRITE = 0x0C,
>  
>  	/* KSV list read using AUX native read with FIFO */
>  	CMD_AUX_GET_KSV_LIST = 0x10,
> @@ -1106,6 +1129,154 @@ static ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
>  	return ret;
>  }
>  
> +static int it6505_aux_i2c_wait(struct it6505 *it6505, u8 *reply)
> +{
> +	int err = 0;

Skip assignment here.

> +	unsigned long timeout;
> +	struct device *dev = it6505->dev;
> +
> +	timeout = jiffies + msecs_to_jiffies(AUX_WAIT_TIMEOUT_MS) + 1;
> +
> +	do {
> +		if (it6505_read(it6505, REG_AUX_USER_CTRL) & AUX_EVENT)
> +			break;
> +		if (time_after(jiffies, timeout)) {
> +			dev_err(dev, "Timed out waiting AUX I2C, BUSY = %X\n",
> +				it6505_aux_op_finished(it6505));
> +			err = -ETIMEDOUT;
> +			goto end_aux_i2c_wait;
> +		}
> +		usleep_range(300, 800);
> +	} while (!it6505_aux_op_finished(it6505));
> +
> +	if (!reply)
> +		goto end_aux_i2c_wait;
> +
> +	*reply = it6505_read(it6505, REG_AUX_USER_REPLY) >> 4;
> +
> +	if (*reply == 0)
> +		goto end_aux_i2c_wait;

assign err = 0 here, so that it's obvious that it's a successfull case.

> +
> +	if ((*reply == DP_AUX_NATIVE_REPLY_DEFER) ||
> +	    (*reply == DP_AUX_I2C_REPLY_DEFER))
> +		err = -EBUSY;
> +	else if ((*reply == DP_AUX_NATIVE_REPLY_NACK) ||
> +		 (*reply == DP_AUX_I2C_REPLY_NACK))
> +		err = -ENXIO;
> +
> +end_aux_i2c_wait:
> +	it6505_set_bits(it6505, REG_AUX_USER_CTRL, USER_AUX_DONE, USER_AUX_DONE);
> +	return err;
> +}
> +
> +static int it6505_aux_i2c_readb(struct it6505 *it6505, u8 *buf, size_t size, u8 *reply)
> +{
> +	int ret, i;
> +	int retry = 0;

Skip the init

> +
> +	for (retry = 0; retry < AUX_I2C_DEFER_RETRY; retry++) {
> +		it6505_write(it6505, REG_AUX_CMD_REQ, CMD_AUX_GI2C_READ);

empty line

> +		ret = it6505_aux_i2c_wait(it6505, reply);
> +		if ((*reply == DP_AUX_NATIVE_REPLY_DEFER) ||
> +		    (*reply == DP_AUX_I2C_REPLY_DEFER))

These two lines keep on being repeated over and over. Please consider
defining a helper function.

> +			continue;
> +		if (ret >= 0)
> +			break;
> +	}
> +
> +	for (i = 0; i < size; i++)
> +		buf[i] =  (u8)it6505_read(it6505, REG_AUX_USER_RXB(0 + i));

Single space, drop type conversion.

> +
> +	return size;
> +}
> +
> +static int it6505_aux_i2c_writeb(struct it6505 *it6505, u8 *buf, size_t size, u8 *reply)
> +{
> +	int i, ret;
> +	int retry = 0;
> +
> +	for (i = 0; i < size; i++)
> +		it6505_write(it6505, REG_AUX_OUT_DATA0 + i, buf[i]);
> +
> +	for (retry = 0; retry < AUX_I2C_DEFER_RETRY; retry++) {
> +		it6505_write(it6505, REG_AUX_CMD_REQ, CMD_AUX_GI2C_WRITE);

empty line

> +		ret = it6505_aux_i2c_wait(it6505, reply);
> +		if ((*reply == DP_AUX_NATIVE_REPLY_DEFER) ||
> +		    (*reply == DP_AUX_I2C_REPLY_DEFER))
> +			continue;
> +		if (ret >= 0)
> +			break;
> +	}
> +	return size;
> +}
> +
> +static ssize_t it6505_aux_i2c_operation(struct it6505 *it6505,
> +					struct drm_dp_aux_msg *msg)
> +{
> +	int ret;
> +	ssize_t request_size, data_cnt = 0;
> +	u8 *buffer = msg->buffer;
> +
> +	/* set AUX user mode */
> +	it6505_set_bits(it6505, REG_AUX_CTRL,
> +			AUX_USER_MODE | AUX_NO_SEGMENT_WR, AUX_USER_MODE);
> +	it6505_set_bits(it6505, REG_AUX_USER_CTRL, EN_USER_AUX, EN_USER_AUX);
> +	/* clear AUX FIFO */
> +	it6505_set_bits(it6505, REG_AUX_CTRL,
> +			AUX_EN_FIFO_READ | CLR_EDID_FIFO,
> +			AUX_EN_FIFO_READ | CLR_EDID_FIFO);
> +
> +	it6505_set_bits(it6505, REG_AUX_CTRL,
> +			AUX_EN_FIFO_READ | CLR_EDID_FIFO, 0x00);
> +
> +	it6505_write(it6505, REG_AUX_ADR_0_7, 0x00);
> +	it6505_write(it6505, REG_AUX_I2C_ADR, msg->address << 1);
> +
> +	if (msg->size == 0) {
> +		/* IIC Start/STOP dummy write */
> +		it6505_write(it6505, REG_AUX_I2C_OP, msg->request);
> +		it6505_write(it6505, REG_AUX_CMD_REQ, CMD_AUX_GI2C_ADR);
> +		ret = it6505_aux_i2c_wait(it6505, &msg->reply);
> +		goto end_aux_i2c_transfer;
> +	}
> +
> +	/* IIC data transfer */
> +	for (data_cnt = 0; data_cnt < msg->size; ) {
> +		request_size = min_t(ssize_t, msg->size - data_cnt, AUX_I2C_MAX_SIZE);
> +		it6505_write(it6505, REG_AUX_I2C_OP,
> +			     msg->request | ((request_size - 1) << 4));
> +		if ((msg->request & DP_AUX_I2C_READ) == DP_AUX_I2C_READ)
> +			ret = it6505_aux_i2c_readb(it6505, &buffer[data_cnt],
> +						   request_size, &msg->reply);
> +		else
> +			ret = it6505_aux_i2c_writeb(it6505, &buffer[data_cnt],
> +						    request_size, &msg->reply);
> +
> +		if (ret < 0)
> +			goto end_aux_i2c_transfer;
> +
> +		data_cnt += request_size;
> +	}
> +	ret = data_cnt;
> +end_aux_i2c_transfer:
> +
> +	it6505_set_bits(it6505, REG_AUX_USER_CTRL, EN_USER_AUX, 0);
> +	it6505_set_bits(it6505, REG_AUX_CTRL, AUX_USER_MODE, 0);
> +	return ret;
> +}
> +
> +static ssize_t it6505_aux_i2c_transfer(struct drm_dp_aux *aux,
> +				       struct drm_dp_aux_msg *msg)
> +{
> +	struct it6505 *it6505 = container_of(aux, struct it6505, aux);
> +	int ret;
> +
> +	mutex_lock(&it6505->aux_lock);
> +	ret = it6505_aux_i2c_operation(it6505, msg);
> +	mutex_unlock(&it6505->aux_lock);

Is it enough to protect from other commands sending data in parallel?
Also as much as I don't like it, in this case using guard() from
cleanup.h will remove a need for a separte function.

> +	return ret;
> +}
> +
>  static ssize_t it6505_aux_transfer(struct drm_dp_aux *aux,
>  				   struct drm_dp_aux_msg *msg)
>  {
> @@ -1115,9 +1286,8 @@ static ssize_t it6505_aux_transfer(struct drm_dp_aux *aux,
>  	int ret;
>  	enum aux_cmd_reply reply;
>  
> -	/* IT6505 doesn't support arbitrary I2C read / write. */
>  	if (is_i2c)
> -		return -EINVAL;
> +		return it6505_aux_i2c_transfer(aux, msg);
>  
>  	switch (msg->request) {
>  	case DP_AUX_NATIVE_READ:
> -- 
> 2.34.1
>
Hermes Wu Sept. 24, 2024, 3:52 a.m. UTC | #2
>On Mon, Sep 23, 2024 at 05:48:29PM GMT, Hermes Wu wrote:
>> From: Hermes Wu <Hermes.wu@ite.com.tw>
>> 
>> Changes in v3:
>>  -remove non used definition for aux i2x cmd reply
>> 
>> Add Aux-I2C functionality to support MCCS.
>> 
>> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
>> ---
>>  drivers/gpu/drm/bridge/ite-it6505.c | 174 
>> +++++++++++++++++++++++++++-
>>  1 file changed, 172 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c 
>> b/drivers/gpu/drm/bridge/ite-it6505.c
>> index 156440c6517e..5aedc5570739 100644
>> --- a/drivers/gpu/drm/bridge/ite-it6505.c
>> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
>> @@ -125,6 +125,9 @@
>>  #define REG_AUX_ADR_16_19 0x26
>>  #define REG_AUX_OUT_DATA0 0x27
>>  
>> +#define REG_AUX_I2C_ADR 0x25
>> +#define REG_AUX_I2C_OP 0x26
>> +
>
>Are these registers CMD-specific? Because I see that you already have defines for 0x25 and 0x26.
>

The AUX packet i2c into aux transfer frames,
and I think it's easier to understand how it6505_aux_i2c_operation() packet i2c request into aux frame.

>>  #define REG_AUX_CMD_REQ 0x2B
>>  #define AUX_BUSY BIT(5)
>>  
>> @@ -266,6 +269,19 @@
>>  #define REG_SSC_CTRL1 0x189
>>  #define REG_SSC_CTRL2 0x18A
>>  
>> +#define REG_AUX_USER_CTRL 0x190
>> +#define EN_USER_AUX BIT(0)
>> +#define USER_AUX_DONE BIT(1)
>> +#define AUX_EVENT BIT(4)
>> +
>> +#define REG_AUX_USER_DATA_REC 0x191
>> +#define M_AUX_IN_REC   0xF0
>> +#define M_AUX_OUT_REC  0x0F
>> +
>> +#define REG_AUX_USER_TXB 0x190
>
>And two defines for 0x190 too.
>
>> +#define REG_AUX_USER_REPLY 0x19A
>> +#define REG_AUX_USER_RXB(n) (n + 0x19B)
>> +
>>  #define RBR DP_LINK_BW_1_62
>>  #define HBR DP_LINK_BW_2_7
>>  #define HBR2 DP_LINK_BW_5_4
>> @@ -301,6 +317,8 @@
>>  #define MAX_EQ_LEVEL 0x03
>>  #define AUX_WAIT_TIMEOUT_MS 15
>>  #define AUX_FIFO_MAX_SIZE 16
>> +#define AUX_I2C_MAX_SIZE 4
>> +#define AUX_I2C_DEFER_RETRY 4
>>  #define PIXEL_CLK_DELAY 1
>>  #define PIXEL_CLK_INVERSE 0
>>  #define ADJUST_PHASE_THRESHOLD 80000
>> @@ -323,7 +341,12 @@
>>  enum aux_cmd_type {
>>  	CMD_AUX_NATIVE_READ = 0x0,
>>  	CMD_AUX_NATIVE_WRITE = 0x5,
>> +	CMD_AUX_GI2C_ADR = 0x08,
>> +	CMD_AUX_GI2C_READ = 0x09,
>> +	CMD_AUX_GI2C_WRITE = 0x0A,
>>  	CMD_AUX_I2C_EDID_READ = 0xB,
>> +	CMD_AUX_I2C_READ = 0x0D,
>> +	CMD_AUX_I2C_WRITE = 0x0C,
>>  
>>  	/* KSV list read using AUX native read with FIFO */
>>  	CMD_AUX_GET_KSV_LIST = 0x10,
>> @@ -1106,6 +1129,154 @@ static ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
>>  	return ret;
>>  }
>>  
>> +static int it6505_aux_i2c_wait(struct it6505 *it6505, u8 *reply) {
>> +	int err = 0;
>
>Skip assignment here.
>
>> +	unsigned long timeout;
>> +	struct device *dev = it6505->dev;
>> +
>> +	timeout = jiffies + msecs_to_jiffies(AUX_WAIT_TIMEOUT_MS) + 1;
>> +
>> +	do {
>> +		if (it6505_read(it6505, REG_AUX_USER_CTRL) & AUX_EVENT)
>> +			break;
>> +		if (time_after(jiffies, timeout)) {
>> +			dev_err(dev, "Timed out waiting AUX I2C, BUSY = %X\n",
>> +				it6505_aux_op_finished(it6505));
>> +			err = -ETIMEDOUT;
>> +			goto end_aux_i2c_wait;
>> +		}
>> +		usleep_range(300, 800);
>> +	} while (!it6505_aux_op_finished(it6505));
>> +
>> +	if (!reply)
>> +		goto end_aux_i2c_wait;
>> +
>> +	*reply = it6505_read(it6505, REG_AUX_USER_REPLY) >> 4;
>> +
>> +	if (*reply == 0)
>> +		goto end_aux_i2c_wait;
>
>assign err = 0 here, so that it's obvious that it's a successfull case.
>
>> +
>> +	if ((*reply == DP_AUX_NATIVE_REPLY_DEFER) ||
>> +	    (*reply == DP_AUX_I2C_REPLY_DEFER))
>> +		err = -EBUSY;
>> +	else if ((*reply == DP_AUX_NATIVE_REPLY_NACK) ||
>> +		 (*reply == DP_AUX_I2C_REPLY_NACK))
>> +		err = -ENXIO;
>> +
>> +end_aux_i2c_wait:
>> +	it6505_set_bits(it6505, REG_AUX_USER_CTRL, USER_AUX_DONE, USER_AUX_DONE);
>> +	return err;
>> +}
>> +
>> +static int it6505_aux_i2c_readb(struct it6505 *it6505, u8 *buf, 
>> +size_t size, u8 *reply) {
>> +	int ret, i;
>> +	int retry = 0;
>
>Skip the init
>
>> +
>> +	for (retry = 0; retry < AUX_I2C_DEFER_RETRY; retry++) {
>> +		it6505_write(it6505, REG_AUX_CMD_REQ, CMD_AUX_GI2C_READ);
>
>empty line
>
>> +		ret = it6505_aux_i2c_wait(it6505, reply);
>> +		if ((*reply == DP_AUX_NATIVE_REPLY_DEFER) ||
>> +		    (*reply == DP_AUX_I2C_REPLY_DEFER))
>
>These two lines keep on being repeated over and over. Please consider defining a helper function.
>
>> +			continue;
>> +		if (ret >= 0)
>> +			break;
>> +	}
>> +
>> +	for (i = 0; i < size; i++)
>> +		buf[i] =  (u8)it6505_read(it6505, REG_AUX_USER_RXB(0 + i));
>
>Single space, drop type conversion.
>
>> +
>> +	return size;
>> +}
>> +
>> +static int it6505_aux_i2c_writeb(struct it6505 *it6505, u8 *buf, 
>> +size_t size, u8 *reply) {
>> +	int i, ret;
>> +	int retry = 0;
>> +
>> +	for (i = 0; i < size; i++)
>> +		it6505_write(it6505, REG_AUX_OUT_DATA0 + i, buf[i]);
>> +
>> +	for (retry = 0; retry < AUX_I2C_DEFER_RETRY; retry++) {
>> +		it6505_write(it6505, REG_AUX_CMD_REQ, CMD_AUX_GI2C_WRITE);
>
>empty line
>
>> +		ret = it6505_aux_i2c_wait(it6505, reply);
>> +		if ((*reply == DP_AUX_NATIVE_REPLY_DEFER) ||
>> +		    (*reply == DP_AUX_I2C_REPLY_DEFER))
>> +			continue;
>> +		if (ret >= 0)
>> +			break;
>> +	}
>> +	return size;
>> +}
>> +
>> +static ssize_t it6505_aux_i2c_operation(struct it6505 *it6505,
>> +					struct drm_dp_aux_msg *msg)
>> +{
>> +	int ret;
>> +	ssize_t request_size, data_cnt = 0;
>> +	u8 *buffer = msg->buffer;
>> +
>> +	/* set AUX user mode */
>> +	it6505_set_bits(it6505, REG_AUX_CTRL,
>> +			AUX_USER_MODE | AUX_NO_SEGMENT_WR, AUX_USER_MODE);
>> +	it6505_set_bits(it6505, REG_AUX_USER_CTRL, EN_USER_AUX, EN_USER_AUX);
>> +	/* clear AUX FIFO */
>> +	it6505_set_bits(it6505, REG_AUX_CTRL,
>> +			AUX_EN_FIFO_READ | CLR_EDID_FIFO,
>> +			AUX_EN_FIFO_READ | CLR_EDID_FIFO);
>> +
>> +	it6505_set_bits(it6505, REG_AUX_CTRL,
>> +			AUX_EN_FIFO_READ | CLR_EDID_FIFO, 0x00);
>> +
>> +	it6505_write(it6505, REG_AUX_ADR_0_7, 0x00);
>> +	it6505_write(it6505, REG_AUX_I2C_ADR, msg->address << 1);
>> +
>> +	if (msg->size == 0) {
>> +		/* IIC Start/STOP dummy write */
>> +		it6505_write(it6505, REG_AUX_I2C_OP, msg->request);
>> +		it6505_write(it6505, REG_AUX_CMD_REQ, CMD_AUX_GI2C_ADR);
>> +		ret = it6505_aux_i2c_wait(it6505, &msg->reply);
>> +		goto end_aux_i2c_transfer;
>> +	}
>> +
>> +	/* IIC data transfer */
>> +	for (data_cnt = 0; data_cnt < msg->size; ) {
>> +		request_size = min_t(ssize_t, msg->size - data_cnt, AUX_I2C_MAX_SIZE);
>> +		it6505_write(it6505, REG_AUX_I2C_OP,
>> +			     msg->request | ((request_size - 1) << 4));
>> +		if ((msg->request & DP_AUX_I2C_READ) == DP_AUX_I2C_READ)
>> +			ret = it6505_aux_i2c_readb(it6505, &buffer[data_cnt],
>> +						   request_size, &msg->reply);
>> +		else
>> +			ret = it6505_aux_i2c_writeb(it6505, &buffer[data_cnt],
>> +						    request_size, &msg->reply);
>> +
>> +		if (ret < 0)
>> +			goto end_aux_i2c_transfer;
>> +
>> +		data_cnt += request_size;
>> +	}
>> +	ret = data_cnt;
>> +end_aux_i2c_transfer:
>> +
>> +	it6505_set_bits(it6505, REG_AUX_USER_CTRL, EN_USER_AUX, 0);
>> +	it6505_set_bits(it6505, REG_AUX_CTRL, AUX_USER_MODE, 0);
>> +	return ret;
>> +}
>> +
>> +static ssize_t it6505_aux_i2c_transfer(struct drm_dp_aux *aux,
>> +				       struct drm_dp_aux_msg *msg) {
>> +	struct it6505 *it6505 = container_of(aux, struct it6505, aux);
>> +	int ret;
>> +
>> +	mutex_lock(&it6505->aux_lock);
>> +	ret = it6505_aux_i2c_operation(it6505, msg);
>> +	mutex_unlock(&it6505->aux_lock);
>
>Is it enough to protect from other commands sending data in parallel?
>Also as much as I don't like it, in this case using guard() from cleanup.h will remove a need for a separte function.
>
>> +	return ret;
>> +}
>> +
>>  static ssize_t it6505_aux_transfer(struct drm_dp_aux *aux,
>>  				   struct drm_dp_aux_msg *msg)
>>  {
>> @@ -1115,9 +1286,8 @@ static ssize_t it6505_aux_transfer(struct drm_dp_aux *aux,
>>  	int ret;
>>  	enum aux_cmd_reply reply;
>>  
>> -	/* IT6505 doesn't support arbitrary I2C read / write. */
>>  	if (is_i2c)
>> -		return -EINVAL;
>> +		return it6505_aux_i2c_transfer(aux, msg);
>>  
>>  	switch (msg->request) {
>>  	case DP_AUX_NATIVE_READ:
>> --
>> 2.34.1
>> 
>
>-- 
>With best wishes
>Dmitry
>

BR,
Hermes
Dmitry Baryshkov Sept. 24, 2024, 9:59 a.m. UTC | #3
On Tue, Sep 24, 2024 at 03:52:34AM GMT, Hermes.Wu@ite.com.tw wrote:
> >On Mon, Sep 23, 2024 at 05:48:29PM GMT, Hermes Wu wrote:
> >> From: Hermes Wu <Hermes.wu@ite.com.tw>
> >> 
> >> Changes in v3:
> >>  -remove non used definition for aux i2x cmd reply
> >> 
> >> Add Aux-I2C functionality to support MCCS.
> >> 
> >> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> >> ---
> >>  drivers/gpu/drm/bridge/ite-it6505.c | 174 
> >> +++++++++++++++++++++++++++-
> >>  1 file changed, 172 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c 
> >> b/drivers/gpu/drm/bridge/ite-it6505.c
> >> index 156440c6517e..5aedc5570739 100644
> >> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> >> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> >> @@ -125,6 +125,9 @@
> >>  #define REG_AUX_ADR_16_19 0x26
> >>  #define REG_AUX_OUT_DATA0 0x27
> >>  
> >> +#define REG_AUX_I2C_ADR 0x25
> >> +#define REG_AUX_I2C_OP 0x26
> >> +
> >
> >Are these registers CMD-specific? Because I see that you already have defines for 0x25 and 0x26.
> >
> 
> The AUX packet i2c into aux transfer frames,
> and I think it's easier to understand how it6505_aux_i2c_operation() packet i2c request into aux frame.

I'm really sorry, but I don't think I can parse this or how this answers
my question. If for the user I2C a part of the register space gets
repurposed, please comment that before the defines (and maybe separate
such defines so that it's obvious to anybody reading the driver).

> 
> >>  #define REG_AUX_CMD_REQ 0x2B
> >>  #define AUX_BUSY BIT(5)
> >>  
> >> @@ -266,6 +269,19 @@
> >>  #define REG_SSC_CTRL1 0x189
> >>  #define REG_SSC_CTRL2 0x18A
> >>  
> >> +#define REG_AUX_USER_CTRL 0x190
> >> +#define EN_USER_AUX BIT(0)
> >> +#define USER_AUX_DONE BIT(1)
> >> +#define AUX_EVENT BIT(4)
> >> +
> >> +#define REG_AUX_USER_DATA_REC 0x191
> >> +#define M_AUX_IN_REC   0xF0
> >> +#define M_AUX_OUT_REC  0x0F
> >> +
> >> +#define REG_AUX_USER_TXB 0x190
> >
> >And two defines for 0x190 too.
> >
> >> +#define REG_AUX_USER_REPLY 0x19A
> >> +#define REG_AUX_USER_RXB(n) (n + 0x19B)
> >> +
> >>  #define RBR DP_LINK_BW_1_62
> >>  #define HBR DP_LINK_BW_2_7
> >>  #define HBR2 DP_LINK_BW_5_4
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index 156440c6517e..5aedc5570739 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -125,6 +125,9 @@ 
 #define REG_AUX_ADR_16_19 0x26
 #define REG_AUX_OUT_DATA0 0x27
 
+#define REG_AUX_I2C_ADR 0x25
+#define REG_AUX_I2C_OP 0x26
+
 #define REG_AUX_CMD_REQ 0x2B
 #define AUX_BUSY BIT(5)
 
@@ -266,6 +269,19 @@ 
 #define REG_SSC_CTRL1 0x189
 #define REG_SSC_CTRL2 0x18A
 
+#define REG_AUX_USER_CTRL 0x190
+#define EN_USER_AUX BIT(0)
+#define USER_AUX_DONE BIT(1)
+#define AUX_EVENT BIT(4)
+
+#define REG_AUX_USER_DATA_REC 0x191
+#define M_AUX_IN_REC   0xF0
+#define M_AUX_OUT_REC  0x0F
+
+#define REG_AUX_USER_TXB 0x190
+#define REG_AUX_USER_REPLY 0x19A
+#define REG_AUX_USER_RXB(n) (n + 0x19B)
+
 #define RBR DP_LINK_BW_1_62
 #define HBR DP_LINK_BW_2_7
 #define HBR2 DP_LINK_BW_5_4
@@ -301,6 +317,8 @@ 
 #define MAX_EQ_LEVEL 0x03
 #define AUX_WAIT_TIMEOUT_MS 15
 #define AUX_FIFO_MAX_SIZE 16
+#define AUX_I2C_MAX_SIZE 4
+#define AUX_I2C_DEFER_RETRY 4
 #define PIXEL_CLK_DELAY 1
 #define PIXEL_CLK_INVERSE 0
 #define ADJUST_PHASE_THRESHOLD 80000
@@ -323,7 +341,12 @@ 
 enum aux_cmd_type {
 	CMD_AUX_NATIVE_READ = 0x0,
 	CMD_AUX_NATIVE_WRITE = 0x5,
+	CMD_AUX_GI2C_ADR = 0x08,
+	CMD_AUX_GI2C_READ = 0x09,
+	CMD_AUX_GI2C_WRITE = 0x0A,
 	CMD_AUX_I2C_EDID_READ = 0xB,
+	CMD_AUX_I2C_READ = 0x0D,
+	CMD_AUX_I2C_WRITE = 0x0C,
 
 	/* KSV list read using AUX native read with FIFO */
 	CMD_AUX_GET_KSV_LIST = 0x10,
@@ -1106,6 +1129,154 @@  static ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
 	return ret;
 }
 
+static int it6505_aux_i2c_wait(struct it6505 *it6505, u8 *reply)
+{
+	int err = 0;
+	unsigned long timeout;
+	struct device *dev = it6505->dev;
+
+	timeout = jiffies + msecs_to_jiffies(AUX_WAIT_TIMEOUT_MS) + 1;
+
+	do {
+		if (it6505_read(it6505, REG_AUX_USER_CTRL) & AUX_EVENT)
+			break;
+		if (time_after(jiffies, timeout)) {
+			dev_err(dev, "Timed out waiting AUX I2C, BUSY = %X\n",
+				it6505_aux_op_finished(it6505));
+			err = -ETIMEDOUT;
+			goto end_aux_i2c_wait;
+		}
+		usleep_range(300, 800);
+	} while (!it6505_aux_op_finished(it6505));
+
+	if (!reply)
+		goto end_aux_i2c_wait;
+
+	*reply = it6505_read(it6505, REG_AUX_USER_REPLY) >> 4;
+
+	if (*reply == 0)
+		goto end_aux_i2c_wait;
+
+	if ((*reply == DP_AUX_NATIVE_REPLY_DEFER) ||
+	    (*reply == DP_AUX_I2C_REPLY_DEFER))
+		err = -EBUSY;
+	else if ((*reply == DP_AUX_NATIVE_REPLY_NACK) ||
+		 (*reply == DP_AUX_I2C_REPLY_NACK))
+		err = -ENXIO;
+
+end_aux_i2c_wait:
+	it6505_set_bits(it6505, REG_AUX_USER_CTRL, USER_AUX_DONE, USER_AUX_DONE);
+	return err;
+}
+
+static int it6505_aux_i2c_readb(struct it6505 *it6505, u8 *buf, size_t size, u8 *reply)
+{
+	int ret, i;
+	int retry = 0;
+
+	for (retry = 0; retry < AUX_I2C_DEFER_RETRY; retry++) {
+		it6505_write(it6505, REG_AUX_CMD_REQ, CMD_AUX_GI2C_READ);
+		ret = it6505_aux_i2c_wait(it6505, reply);
+		if ((*reply == DP_AUX_NATIVE_REPLY_DEFER) ||
+		    (*reply == DP_AUX_I2C_REPLY_DEFER))
+			continue;
+		if (ret >= 0)
+			break;
+	}
+
+	for (i = 0; i < size; i++)
+		buf[i] =  (u8)it6505_read(it6505, REG_AUX_USER_RXB(0 + i));
+
+	return size;
+}
+
+static int it6505_aux_i2c_writeb(struct it6505 *it6505, u8 *buf, size_t size, u8 *reply)
+{
+	int i, ret;
+	int retry = 0;
+
+	for (i = 0; i < size; i++)
+		it6505_write(it6505, REG_AUX_OUT_DATA0 + i, buf[i]);
+
+	for (retry = 0; retry < AUX_I2C_DEFER_RETRY; retry++) {
+		it6505_write(it6505, REG_AUX_CMD_REQ, CMD_AUX_GI2C_WRITE);
+		ret = it6505_aux_i2c_wait(it6505, reply);
+		if ((*reply == DP_AUX_NATIVE_REPLY_DEFER) ||
+		    (*reply == DP_AUX_I2C_REPLY_DEFER))
+			continue;
+		if (ret >= 0)
+			break;
+	}
+	return size;
+}
+
+static ssize_t it6505_aux_i2c_operation(struct it6505 *it6505,
+					struct drm_dp_aux_msg *msg)
+{
+	int ret;
+	ssize_t request_size, data_cnt = 0;
+	u8 *buffer = msg->buffer;
+
+	/* set AUX user mode */
+	it6505_set_bits(it6505, REG_AUX_CTRL,
+			AUX_USER_MODE | AUX_NO_SEGMENT_WR, AUX_USER_MODE);
+	it6505_set_bits(it6505, REG_AUX_USER_CTRL, EN_USER_AUX, EN_USER_AUX);
+	/* clear AUX FIFO */
+	it6505_set_bits(it6505, REG_AUX_CTRL,
+			AUX_EN_FIFO_READ | CLR_EDID_FIFO,
+			AUX_EN_FIFO_READ | CLR_EDID_FIFO);
+
+	it6505_set_bits(it6505, REG_AUX_CTRL,
+			AUX_EN_FIFO_READ | CLR_EDID_FIFO, 0x00);
+
+	it6505_write(it6505, REG_AUX_ADR_0_7, 0x00);
+	it6505_write(it6505, REG_AUX_I2C_ADR, msg->address << 1);
+
+	if (msg->size == 0) {
+		/* IIC Start/STOP dummy write */
+		it6505_write(it6505, REG_AUX_I2C_OP, msg->request);
+		it6505_write(it6505, REG_AUX_CMD_REQ, CMD_AUX_GI2C_ADR);
+		ret = it6505_aux_i2c_wait(it6505, &msg->reply);
+		goto end_aux_i2c_transfer;
+	}
+
+	/* IIC data transfer */
+	for (data_cnt = 0; data_cnt < msg->size; ) {
+		request_size = min_t(ssize_t, msg->size - data_cnt, AUX_I2C_MAX_SIZE);
+		it6505_write(it6505, REG_AUX_I2C_OP,
+			     msg->request | ((request_size - 1) << 4));
+		if ((msg->request & DP_AUX_I2C_READ) == DP_AUX_I2C_READ)
+			ret = it6505_aux_i2c_readb(it6505, &buffer[data_cnt],
+						   request_size, &msg->reply);
+		else
+			ret = it6505_aux_i2c_writeb(it6505, &buffer[data_cnt],
+						    request_size, &msg->reply);
+
+		if (ret < 0)
+			goto end_aux_i2c_transfer;
+
+		data_cnt += request_size;
+	}
+	ret = data_cnt;
+end_aux_i2c_transfer:
+
+	it6505_set_bits(it6505, REG_AUX_USER_CTRL, EN_USER_AUX, 0);
+	it6505_set_bits(it6505, REG_AUX_CTRL, AUX_USER_MODE, 0);
+	return ret;
+}
+
+static ssize_t it6505_aux_i2c_transfer(struct drm_dp_aux *aux,
+				       struct drm_dp_aux_msg *msg)
+{
+	struct it6505 *it6505 = container_of(aux, struct it6505, aux);
+	int ret;
+
+	mutex_lock(&it6505->aux_lock);
+	ret = it6505_aux_i2c_operation(it6505, msg);
+	mutex_unlock(&it6505->aux_lock);
+	return ret;
+}
+
 static ssize_t it6505_aux_transfer(struct drm_dp_aux *aux,
 				   struct drm_dp_aux_msg *msg)
 {
@@ -1115,9 +1286,8 @@  static ssize_t it6505_aux_transfer(struct drm_dp_aux *aux,
 	int ret;
 	enum aux_cmd_reply reply;
 
-	/* IT6505 doesn't support arbitrary I2C read / write. */
 	if (is_i2c)
-		return -EINVAL;
+		return it6505_aux_i2c_transfer(aux, msg);
 
 	switch (msg->request) {
 	case DP_AUX_NATIVE_READ: