1. Patchew
  2. linux
  3. [PATCH v4 00/11] fix HDCP CTS fail items and add MCCS support
  4. View patch

[PATCH v4 02/11] drm/bridge: it6505: fix aux operation for edid read

Hermes Wu posted 11 patches 2 months ago
Only 4 patches received!
[PATCH v4 02/11] drm/bridge: it6505: fix aux operation for edid read
Posted by Hermes Wu 2 months ago 
From: Hermes Wu <Hermes.wu@ite.com.tw>

The EDID read operation can reach the maximum size of the AUX FIFO buffer.

Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
---
 drivers/gpu/drm/bridge/ite-it6505.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index 28a8043229d3..b451d3c2ac1d 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -1078,8 +1078,11 @@ static ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
 	int i, ret_size, ret = 0, request_size;
 
 	mutex_lock(&it6505->aux_lock);
-	for (i = 0; i < size; i += 4) {
-		request_size = min((int)size - i, 4);
+	for (i = 0; i < size; ) {
+		if (cmd == CMD_AUX_I2C_EDID_READ)
+			request_size = min_t(int, (int)size - i, AUX_FIFO_MAX_SIZE);
+		else
+			request_size = min_t(int, (int)size - i, 4);
 		ret_size = it6505_aux_operation(it6505, cmd, address + i,
 						buffer + i, request_size,
 						reply);
@@ -1088,6 +1091,7 @@ static ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
 			goto aux_op_err;
 		}
 
+		i += request_size;
 		ret += ret_size;
 	}
 
-- 
2.34.1
Re: [PATCH v4 02/11] drm/bridge: it6505: fix aux operation for edid read
Posted by AngeloGioacchino Del Regno 2 months ago 
Il 26/09/24 09:47, Hermes Wu ha scritto:
> From: Hermes Wu <Hermes.wu@ite.com.tw>
> 
> The EDID read operation can reach the maximum size of the AUX FIFO buffer.
> 
> Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> ---
>   drivers/gpu/drm/bridge/ite-it6505.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index 28a8043229d3..b451d3c2ac1d 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -1078,8 +1078,11 @@ static ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
>   	int i, ret_size, ret = 0, request_size;

int fifo_max_sz = (cmd == CMD_AUX_I2C_EDID_READ) ?
		  AUX_FIFO_MAX_SIZE : 4;

which later becomes

int fifo_max_sz = (cmd == CMD_AUX_I2C_EDID_READ || cmd == CMD_AUX_GET_KSV_LIST) ?
		  AUX_FIFO_MAX_SIZE : 4;

otherwise you can do it "the long way"...

if (cmd == .....)
	fifo_max_sz = AUX_FIFO_MAX_SIZE;
else
	fifo_max_sz = 4;

The point is, cmd won't change during iterations, so it's useless to put that if
branch inside of that loop below.

>   
>   	mutex_lock(&it6505->aux_lock);
> -	for (i = 0; i < size; i += 4) {
> -		request_size = min((int)size - i, 4);

		request_size = min_t(int, (int)size - i, fifo_max_sz);

P.S.: Also, I'd consider changing this to a do-while instead...

Cheers,
Angelo

> +	for (i = 0; i < size; ) {
> +		if (cmd == CMD_AUX_I2C_EDID_READ)
> +			request_size = min_t(int, (int)size - i, AUX_FIFO_MAX_SIZE);
> +		else
> +			request_size = min_t(int, (int)size - i, 4);
>   		ret_size = it6505_aux_operation(it6505, cmd, address + i,
>   						buffer + i, request_size,
>   						reply);
> @@ -1088,6 +1091,7 @@ static ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
>   			goto aux_op_err;
>   		}
>   
> +		i += request_size;
>   		ret += ret_size;
>   	}
>
Re: [PATCH v4 02/11] drm/bridge: it6505: fix aux operation for edid read
Posted by Dmitry Baryshkov 2 months ago 
On Thu, Sep 26, 2024 at 03:47:52PM GMT, Hermes Wu wrote:
> From: Hermes Wu <Hermes.wu@ite.com.tw>
> 
> The EDID read operation can reach the maximum size of the AUX FIFO buffer.

And? Commit message should describe why the change is necessary and what
is being done. Just providing a statement is not enough.

> 
> Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> ---
>  drivers/gpu/drm/bridge/ite-it6505.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index 28a8043229d3..b451d3c2ac1d 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -1078,8 +1078,11 @@ static ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
>  	int i, ret_size, ret = 0, request_size;
>  
>  	mutex_lock(&it6505->aux_lock);
> -	for (i = 0; i < size; i += 4) {
> -		request_size = min((int)size - i, 4);
> +	for (i = 0; i < size; ) {
> +		if (cmd == CMD_AUX_I2C_EDID_READ)
> +			request_size = min_t(int, (int)size - i, AUX_FIFO_MAX_SIZE);
> +		else
> +			request_size = min_t(int, (int)size - i, 4);
>  		ret_size = it6505_aux_operation(it6505, cmd, address + i,
>  						buffer + i, request_size,
>  						reply);
> @@ -1088,6 +1091,7 @@ static ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
>  			goto aux_op_err;
>  		}
>  
> +		i += request_size;
>  		ret += ret_size;
>  	}
>  
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry
RE: [PATCH v4 02/11] drm/bridge: it6505: fix aux operation for edid read
Posted by Hermes.Wu@ite.com.tw 2 months ago 
>On Thu, Sep 26, 2024 at 03:47:52PM GMT, Hermes Wu wrote:
>> From: Hermes Wu <Hermes.wu@ite.com.tw>
>> 
>> The EDID read operation can reach the maximum size of the AUX FIFO buffer.
>
>And? Commit message should describe why the change is necessary and what is being done. Just providing a statement is not enough.
>

The original AUX operation treat all reads by using data registers will be limited at 4 bytes.
AUX operation command CMD_AUX_I2C_EDID_READ using AUX FIFO is capable of reads data by 16 bytes each time.
It can improve speed of read EDID.


>> 
>> Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
>> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
>> ---
>>  drivers/gpu/drm/bridge/ite-it6505.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c 
>> b/drivers/gpu/drm/bridge/ite-it6505.c
>> index 28a8043229d3..b451d3c2ac1d 100644
>> --- a/drivers/gpu/drm/bridge/ite-it6505.c
>> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
>> @@ -1078,8 +1078,11 @@ static ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
>>  	int i, ret_size, ret = 0, request_size;
>>  
>>  	mutex_lock(&it6505->aux_lock);
>> -	for (i = 0; i < size; i += 4) {
>> -		request_size = min((int)size - i, 4);
>> +	for (i = 0; i < size; ) {
>> +		if (cmd == CMD_AUX_I2C_EDID_READ)
>> +			request_size = min_t(int, (int)size - i, AUX_FIFO_MAX_SIZE);
>> +		else
>> +			request_size = min_t(int, (int)size - i, 4);
>>  		ret_size = it6505_aux_operation(it6505, cmd, address + i,
>>  						buffer + i, request_size,
>>  						reply);
>> @@ -1088,6 +1091,7 @@ static ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
>>  			goto aux_op_err;
>>  		}
>>  
>> +		i += request_size;
>>  		ret += ret_size;
>>  	}
>>  
>> --
>> 2.34.1
>> 
>
>-- 
>With best wishes
>Dmitry
>

BR,
Hermes
Re: [PATCH v4 02/11] drm/bridge: it6505: fix aux operation for edid read
Posted by Dmitry Baryshkov 2 months ago 
On Thu, 26 Sept 2024 at 10:04, <Hermes.Wu@ite.com.tw> wrote:
>
> >On Thu, Sep 26, 2024 at 03:47:52PM GMT, Hermes Wu wrote:
> >> From: Hermes Wu <Hermes.wu@ite.com.tw>
> >>
> >> The EDID read operation can reach the maximum size of the AUX FIFO buffer.
> >
> >And? Commit message should describe why the change is necessary and what is being done. Just providing a statement is not enough.
> >
>
> The original AUX operation treat all reads by using data registers will be limited at 4 bytes.
> AUX operation command CMD_AUX_I2C_EDID_READ using AUX FIFO is capable of reads data by 16 bytes each time.
> It can improve speed of read EDID.

improves, rather than "can improve". Please add this to the commit message.

>
>
> >>
> >> Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
> >> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> >> ---
> >>  drivers/gpu/drm/bridge/ite-it6505.c | 8 ++++++--
> >>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
> >> b/drivers/gpu/drm/bridge/ite-it6505.c
> >> index 28a8043229d3..b451d3c2ac1d 100644
> >> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> >> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> >> @@ -1078,8 +1078,11 @@ static ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
> >>      int i, ret_size, ret = 0, request_size;
> >>
> >>      mutex_lock(&it6505->aux_lock);
> >> -    for (i = 0; i < size; i += 4) {
> >> -            request_size = min((int)size - i, 4);
> >> +    for (i = 0; i < size; ) {
> >> +            if (cmd == CMD_AUX_I2C_EDID_READ)
> >> +                    request_size = min_t(int, (int)size - i, AUX_FIFO_MAX_SIZE);
> >> +            else
> >> +                    request_size = min_t(int, (int)size - i, 4);
> >>              ret_size = it6505_aux_operation(it6505, cmd, address + i,
> >>                                              buffer + i, request_size,
> >>                                              reply);
> >> @@ -1088,6 +1091,7 @@ static ssize_t it6505_aux_do_transfer(struct it6505 *it6505,
> >>                      goto aux_op_err;
> >>              }
> >>
> >> +            i += request_size;
> >>              ret += ret_size;
> >>      }
> >>
> >> --
> >> 2.34.1
> >>
> >
> >--
> >With best wishes
> >Dmitry
> >
>
> BR,
> Hermes
>


-- 
With best wishes
Dmitry

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.