[PATCH v4 05/11] drm/bridge: it6505: increase supports of HDCP repeater ksv devices

[PATCH v4 05/11] drm/bridge: it6505: increase supports of HDCP repeater ksv devices

[Hermes Wu]

From: Hermes Wu <Hermes.wu@ite.com.tw>

A HDCP source shall support max downstream device to 127.

Change definition of MAX_HDCP_DOWN_STREAM_COUNT to 127

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

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index d1f5220e04a6..5d5ce12cd054 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -296,7 +296,7 @@
 #define MAX_LANE_COUNT 4
 #define MAX_LINK_RATE HBR
 #define AUTO_TRAIN_RETRY 3
-#define MAX_HDCP_DOWN_STREAM_COUNT 10
+#define MAX_HDCP_DOWN_STREAM_COUNT 127
 #define MAX_CR_LEVEL 0x03
 #define MAX_EQ_LEVEL 0x03
 #define AUX_WAIT_TIMEOUT_MS 15
-- 
2.34.1

[PATCH v4 06/11] drm/bridge: it6505: fix HDCP Bstatus check.

[Hermes Wu]

From: Hermes Wu <Hermes.wu@ite.com.tw>

When HDCP is activated, A DisplayPort source receives CP_IRQ from the
sink shall check Bstatus from DPCD and process the corresponding value.


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

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index 5d5ce12cd054..d2ec3dfc57d6 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -2322,14 +2322,20 @@ static int it6505_process_hpd_irq(struct it6505 *it6505)
 	DRM_DEV_DEBUG_DRIVER(dev, "dp_irq_vector = 0x%02x", dp_irq_vector);
 
 	if (dp_irq_vector & DP_CP_IRQ) {
-		it6505_set_bits(it6505, REG_HDCP_TRIGGER, HDCP_TRIGGER_CPIRQ,
-				HDCP_TRIGGER_CPIRQ);
-
 		bstatus = it6505_dpcd_read(it6505, DP_AUX_HDCP_BSTATUS);
 		if (bstatus < 0)
 			return bstatus;
 
 		DRM_DEV_DEBUG_DRIVER(dev, "Bstatus = 0x%02x", bstatus);
+
+		/*check BSTATUS when recive CP_IRQ */
+		if (bstatus & DP_BSTATUS_R0_PRIME_READY &&
+		    it6505->hdcp_status == HDCP_AUTH_GOING)
+			it6505_set_bits(it6505, REG_HDCP_TRIGGER, HDCP_TRIGGER_CPIRQ,
+					HDCP_TRIGGER_CPIRQ);
+		else if (bstatus & (DP_BSTATUS_REAUTH_REQ | DP_BSTATUS_LINK_FAILURE) &&
+			 it6505->hdcp_status == HDCP_AUTH_DONE)
+			it6505_start_hdcp(it6505);
 	}
 
 	ret = drm_dp_dpcd_read_link_status(&it6505->aux, link_status);
-- 
2.34.1

Re: [PATCH v4 06/11] drm/bridge: it6505: fix HDCP Bstatus check.

[Dmitry Baryshkov]

On Thu, Sep 26, 2024 at 03:50:15PM GMT, Hermes Wu wrote:
> From: Hermes Wu <Hermes.wu@ite.com.tw>
> 
> When HDCP is activated, A DisplayPort source receives CP_IRQ from the
> sink shall check Bstatus from DPCD and process the corresponding value.
> 
> 
> Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> ---
>  drivers/gpu/drm/bridge/ite-it6505.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH v4 06/11] drm/bridge: it6505: fix HDCP Bstatus check.

[AngeloGioacchino Del Regno]

Il 26/09/24 09:50, Hermes Wu ha scritto:
> From: Hermes Wu <Hermes.wu@ite.com.tw>
> 
> When HDCP is activated, A DisplayPort source receives CP_IRQ from the
> sink shall check Bstatus from DPCD and process the corresponding value.
> 
> 
> Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> ---
>   drivers/gpu/drm/bridge/ite-it6505.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index 5d5ce12cd054..d2ec3dfc57d6 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -2322,14 +2322,20 @@ static int it6505_process_hpd_irq(struct it6505 *it6505)
>   	DRM_DEV_DEBUG_DRIVER(dev, "dp_irq_vector = 0x%02x", dp_irq_vector);
>   
>   	if (dp_irq_vector & DP_CP_IRQ) {
> -		it6505_set_bits(it6505, REG_HDCP_TRIGGER, HDCP_TRIGGER_CPIRQ,
> -				HDCP_TRIGGER_CPIRQ);
> -
>   		bstatus = it6505_dpcd_read(it6505, DP_AUX_HDCP_BSTATUS);
>   		if (bstatus < 0)
>   			return bstatus;
>   
>   		DRM_DEV_DEBUG_DRIVER(dev, "Bstatus = 0x%02x", bstatus);
> +
> +		/*check BSTATUS when recive CP_IRQ */

/* Check .... */

> +		if (bstatus & DP_BSTATUS_R0_PRIME_READY &&
> +		    it6505->hdcp_status == HDCP_AUTH_GOING)
> +			it6505_set_bits(it6505, REG_HDCP_TRIGGER, HDCP_TRIGGER_CPIRQ,
> +					HDCP_TRIGGER_CPIRQ);
> +		else if (bstatus & (DP_BSTATUS_REAUTH_REQ | DP_BSTATUS_LINK_FAILURE) &&
> +			 it6505->hdcp_status == HDCP_AUTH_DONE)
> +			it6505_start_hdcp(it6505);
>   	}
>   
>   	ret = drm_dp_dpcd_read_link_status(&it6505->aux, link_status);

[PATCH v4 07/11] drm/bridge: it6505: fix HDCP encription not enable when R0 ready

[Hermes Wu]

From: Hermes Wu <Hermes.wu@ite.com.tw>

When starting HDCP authentication, HDCP encryption should be enabled on
the link when R0' is checked.

Change encryption enables time at R0' ready.
Hardware HDCP engine trigger changes repeater fail to restart flow.


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

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index d2ec3dfc57d6..82986f28f653 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -2091,15 +2091,12 @@ static void it6505_hdcp_wait_ksv_list(struct work_struct *work)
 	ksv_list_check = it6505_hdcp_part2_ksvlist_check(it6505);
 	DRM_DEV_DEBUG_DRIVER(dev, "ksv list ready, ksv list check %s",
 			     ksv_list_check ? "pass" : "fail");
-	if (ksv_list_check) {
-		it6505_set_bits(it6505, REG_HDCP_TRIGGER,
-				HDCP_TRIGGER_KSV_DONE, HDCP_TRIGGER_KSV_DONE);
+
+	if (ksv_list_check)
 		return;
-	}
+
 timeout:
-	it6505_set_bits(it6505, REG_HDCP_TRIGGER,
-			HDCP_TRIGGER_KSV_DONE | HDCP_TRIGGER_KSV_FAIL,
-			HDCP_TRIGGER_KSV_DONE | HDCP_TRIGGER_KSV_FAIL);
+	it6505_start_hdcp(it6505);
 }
 
 static void it6505_hdcp_work(struct work_struct *work)
@@ -2472,7 +2469,11 @@ static void it6505_irq_hdcp_ksv_check(struct it6505 *it6505)
 {
 	struct device *dev = it6505->dev;
 
-	DRM_DEV_DEBUG_DRIVER(dev, "HDCP event Interrupt");
+	DRM_DEV_DEBUG_DRIVER(dev, "HDCP repeater R0 event Interrupt");
+	/* 1B01 HDCP encription should start when R0 is ready*/
+	it6505_set_bits(it6505, REG_HDCP_TRIGGER,
+			HDCP_TRIGGER_KSV_DONE, HDCP_TRIGGER_KSV_DONE);
+
 	schedule_work(&it6505->hdcp_wait_ksv_list);
 }
 
-- 
2.34.1

[PATCH v4 08/11] drm/bridge: it6505: fix HDCP KSV list did not read correctly.

[Hermes Wu]

From: Hermes Wu <Hermes.wu@ite.com.tw>

When running the HDCP CTS test on UNIGRAF DPR-100.
KSV list must be read from DPCD with 5 byte boundary.

The original aux operation using AUX_NATIVE_READ can not read back the
KSV list correctly.
Change to CMD_AUX_GET_KSV_LIST operaction.


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

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index 82986f28f653..143d58ed1b0e 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -1188,6 +1188,35 @@ static int it6505_get_edid_block(void *data, u8 *buf, unsigned int block,
 	return 0;
 }
 
+static int it6505_get_ksvlist(struct it6505 *it6505, u8 *buf, size_t len)
+{
+	int i, request_size, ret;
+	struct device *dev = it6505->dev;
+	enum aux_cmd_reply reply;
+
+	for (i = 0; i < len; ) {
+		request_size = min_t(int, (int)len - i, 15);
+
+		ret = it6505_aux_do_transfer(it6505, CMD_AUX_GET_KSV_LIST,
+					     DP_AUX_HDCP_KSV_FIFO,
+					     buf + i, request_size, &reply);
+
+		DRM_DEV_DEBUG_DRIVER(dev, "request_size = %d, ret =%d", request_size, ret);
+		if (ret < 0)
+			return ret;
+
+		i += request_size;
+	}
+
+	DRM_DEV_DEBUG_DRIVER(dev, "ksv read cnt = %d down_stream_cnt=%d ", i, i / 5);
+
+	for (i = 0 ; i < len; i += 5)
+		DRM_DEV_DEBUG_DRIVER(dev, "ksv[%d] = %02X%02X%02X%02X%02X",
+				     i / 5, buf[i], buf[i + 1], buf[i + 2], buf[i + 3], buf[i + 4]);
+
+	return len;
+}
+
 static void it6505_variable_config(struct it6505 *it6505)
 {
 	it6505->link_rate_bw_code = HBR;
@@ -1969,7 +1998,7 @@ static int it6505_setup_sha1_input(struct it6505 *it6505, u8 *sha1_input)
 {
 	struct device *dev = it6505->dev;
 	u8 binfo[2];
-	int down_stream_count, i, err, msg_count = 0;
+	int down_stream_count, err, msg_count = 0;
 
 	err = it6505_get_dpcd(it6505, DP_AUX_HDCP_BINFO, binfo,
 			      ARRAY_SIZE(binfo));
@@ -1994,18 +2023,11 @@ static int it6505_setup_sha1_input(struct it6505 *it6505, u8 *sha1_input)
 			down_stream_count);
 		return 0;
 	}
+	err =  it6505_get_ksvlist(it6505, sha1_input, down_stream_count * 5);
+	if (err < 0)
+		return err;
 
-	for (i = 0; i < down_stream_count; i++) {
-		err = it6505_get_dpcd(it6505, DP_AUX_HDCP_KSV_FIFO +
-				      (i % 3) * DRM_HDCP_KSV_LEN,
-				      sha1_input + msg_count,
-				      DRM_HDCP_KSV_LEN);
-
-		if (err < 0)
-			return err;
-
-		msg_count += 5;
-	}
+	msg_count += down_stream_count * 5;
 
 	it6505->hdcp_down_stream_count = down_stream_count;
 	sha1_input[msg_count++] = binfo[0];
-- 
2.34.1

Re: [PATCH v4 08/11] drm/bridge: it6505: fix HDCP KSV list did not read correctly.

[AngeloGioacchino Del Regno]

Il 26/09/24 09:50, Hermes Wu ha scritto:
> From: Hermes Wu <Hermes.wu@ite.com.tw>
> 
> When running the HDCP CTS test on UNIGRAF DPR-100.
> KSV list must be read from DPCD with 5 byte boundary.
> 
> The original aux operation using AUX_NATIVE_READ can not read back the
> KSV list correctly.
> Change to CMD_AUX_GET_KSV_LIST operaction.
> 
> 
> Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> ---
>   drivers/gpu/drm/bridge/ite-it6505.c | 46 +++++++++++++++++++++--------
>   1 file changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index 82986f28f653..143d58ed1b0e 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -1188,6 +1188,35 @@ static int it6505_get_edid_block(void *data, u8 *buf, unsigned int block,
>   	return 0;
>   }
>   
> +static int it6505_get_ksvlist(struct it6505 *it6505, u8 *buf, size_t len)
> +{
> +	int i, request_size, ret;
> +	struct device *dev = it6505->dev;
> +	enum aux_cmd_reply reply;

struct device *dev = it6505->dev;
enum aux_cmd_reply reply;
int request_size, ret;
int i = 0;

> +
> +	for (i = 0; i < len; ) {

do {

> +		request_size = min_t(int, (int)len - i, 15);
> +
> +		ret = it6505_aux_do_transfer(it6505, CMD_AUX_GET_KSV_LIST,
> +					     DP_AUX_HDCP_KSV_FIFO,
> +					     buf + i, request_size, &reply);
> +
> +		DRM_DEV_DEBUG_DRIVER(dev, "request_size = %d, ret =%d", request_size, ret);
> +		if (ret < 0)
> +			return ret;
> +
> +		i += request_size;
> +	}

} while (i < len);

> +
> +	DRM_DEV_DEBUG_DRIVER(dev, "ksv read cnt = %d down_stream_cnt=%d ", i, i / 5);
> +
> +	for (i = 0 ; i < len; i += 5)

Add braces to this loop, otherwise if DRM_DEV_DEBUG_DRIVER() suddenly becomes
empty (because of some configuration option in the future, etc) you'll get build
breakages.

> +		DRM_DEV_DEBUG_DRIVER(dev, "ksv[%d] = %02X%02X%02X%02X%02X",
> +				     i / 5, buf[i], buf[i + 1], buf[i + 2], buf[i + 3], buf[i + 4]);
> +
> +	return len;
> +}
> +
>   static void it6505_variable_config(struct it6505 *it6505)
>   {
>   	it6505->link_rate_bw_code = HBR;
> @@ -1969,7 +1998,7 @@ static int it6505_setup_sha1_input(struct it6505 *it6505, u8 *sha1_input)
>   {
>   	struct device *dev = it6505->dev;
>   	u8 binfo[2];
> -	int down_stream_count, i, err, msg_count = 0;
> +	int down_stream_count, err, msg_count = 0;
>   
>   	err = it6505_get_dpcd(it6505, DP_AUX_HDCP_BINFO, binfo,
>   			      ARRAY_SIZE(binfo));
> @@ -1994,18 +2023,11 @@ static int it6505_setup_sha1_input(struct it6505 *it6505, u8 *sha1_input)
>   			down_stream_count);
>   		return 0;
>   	}
> +	err =  it6505_get_ksvlist(it6505, sha1_input, down_stream_count * 5);
> +	if (err < 0)
> +		return err;
>   
> -	for (i = 0; i < down_stream_count; i++) {
> -		err = it6505_get_dpcd(it6505, DP_AUX_HDCP_KSV_FIFO +
> -				      (i % 3) * DRM_HDCP_KSV_LEN,
> -				      sha1_input + msg_count,
> -				      DRM_HDCP_KSV_LEN);
> -
> -		if (err < 0)
> -			return err;
> -
> -		msg_count += 5;
> -	}
> +	msg_count += down_stream_count * 5;
>   
>   	it6505->hdcp_down_stream_count = down_stream_count;
>   	sha1_input[msg_count++] = binfo[0];

[PATCH v4 09/11] drm/bridge: it6505: fix HDCP CTS compare V matching without retry

[Hermes Wu]

From: Hermes Wu <Hermes.wu@ite.com.tw>

When HDCP negotiation with a repeater device. Checking SHA V' matching
must retry 3 times before restarting HDCP.  


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

diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
index 143d58ed1b0e..e75bc1575aa8 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -2055,7 +2055,7 @@ static bool it6505_hdcp_part2_ksvlist_check(struct it6505 *it6505)
 {
 	struct device *dev = it6505->dev;
 	u8 av[5][4], bv[5][4];
-	int i, err;
+	int i, err, retry;
 
 	i = it6505_setup_sha1_input(it6505, it6505->sha1_input);
 	if (i <= 0) {
@@ -2064,22 +2064,28 @@ static bool it6505_hdcp_part2_ksvlist_check(struct it6505 *it6505)
 	}
 
 	it6505_sha1_digest(it6505, it6505->sha1_input, i, (u8 *)av);
+	/*1B-05 V' must retry 3 times */
+	for (retry = 0; retry < 3; retry++) {
+		err = it6505_get_dpcd(it6505, DP_AUX_HDCP_V_PRIME(0), (u8 *)bv,
+				      sizeof(bv));
 
-	err = it6505_get_dpcd(it6505, DP_AUX_HDCP_V_PRIME(0), (u8 *)bv,
-			      sizeof(bv));
+		if (err < 0) {
+			dev_err(dev, "Read V' value Fail %d", retry);
+			continue;
+		}
 
-	if (err < 0) {
-		dev_err(dev, "Read V' value Fail");
-		return false;
-	}
+		for (i = 0; i < 5; i++) {
+			if (bv[i][3] != av[i][0] || bv[i][2] != av[i][1] ||
+			    av[i][1] != av[i][2] || bv[i][0] != av[i][3])
+				break;
 
-	for (i = 0; i < 5; i++)
-		if (bv[i][3] != av[i][0] || bv[i][2] != av[i][1] ||
-		    bv[i][1] != av[i][2] || bv[i][0] != av[i][3])
-			return false;
+			DRM_DEV_DEBUG_DRIVER(dev, "V' all match!! %d, %d", retry, i);
+			return true;
+		}
+	}
 
-	DRM_DEV_DEBUG_DRIVER(dev, "V' all match!!");
-	return true;
+	DRM_DEV_DEBUG_DRIVER(dev, "V' NOT match!! %d", retry);
+	return false;
 }
 
 static void it6505_hdcp_wait_ksv_list(struct work_struct *work)
-- 
2.34.1

Re: [PATCH v4 05/11] drm/bridge: it6505: increase supports of HDCP repeater ksv devices

[Dmitry Baryshkov]

On Thu, Sep 26, 2024 at 03:50:14PM GMT, Hermes Wu wrote:
> From: Hermes Wu <Hermes.wu@ite.com.tw>
> 
> A HDCP source shall support max downstream device to 127.
> 
> Change definition of MAX_HDCP_DOWN_STREAM_COUNT to 127

This results in struct it6505 growth by ~0.5 KiB. Please mention it in
the commit message.

Is it really required to have sha1_input in the constantly allocated
structure? I think it's a temporary data, which isn't necessary after
processing.

> 
> Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
> ---
>  drivers/gpu/drm/bridge/ite-it6505.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index d1f5220e04a6..5d5ce12cd054 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -296,7 +296,7 @@
>  #define MAX_LANE_COUNT 4
>  #define MAX_LINK_RATE HBR
>  #define AUTO_TRAIN_RETRY 3
> -#define MAX_HDCP_DOWN_STREAM_COUNT 10
> +#define MAX_HDCP_DOWN_STREAM_COUNT 127
>  #define MAX_CR_LEVEL 0x03
>  #define MAX_EQ_LEVEL 0x03
>  #define AUX_WAIT_TIMEOUT_MS 15
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

RE: [PATCH v4 05/11] drm/bridge: it6505: increase supports of HDCP repeater ksv devices

[Hermes.Wu]

>
>-----Original Message-----
>From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 
>Sent: Thursday, September 26, 2024 4:07 PM
>To: Hermes Wu (吳佳宏) <Hermes.Wu@ite.com.tw>
>Cc: Kenneth Hung (洪家倫) <Kenneth.Hung@ite.com.tw>; Andrzej Hajda <andrzej.hajda@intel.com>; Neil Armstrong <neil.armstrong@linaro.org>; Robert Foss <rfoss@kernel.org>; Laurent Pinchart <Laurent.pinchart@ideasonboard.com>; Jonas Karlman <jonas@kwiboo.se>; Jernej Skrabec <jernej.skrabec@gmail.com>; Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David Airlie <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Allen Chen <allen.chen@ite.com.tw>; AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>; open list:DRM DRIVERS <dri-devel@lists.freedesktop.org>; open list <linux-kernel@vger.kernel.org>
>Subject: Re: [PATCH v4 05/11] drm/bridge: it6505: increase supports of HDCP repeater ksv devices
>
>On Thu, Sep 26, 2024 at 03:50:14PM GMT, Hermes Wu wrote:
>> From: Hermes Wu <Hermes.wu@ite.com.tw>
>> 
>> A HDCP source shall support max downstream device to 127.
>> 
>> Change definition of MAX_HDCP_DOWN_STREAM_COUNT to 127
>
>This results in struct it6505 growth by ~0.5 KiB. Please mention it in the commit message.
>
>Is it really required to have sha1_input in the constantly allocated structure? I think it's a temporary data, which isn't necessary after processing.

Change sha1_input with dynamic memory into one commit or 

change to use temporary data in one and change definition of MAX_HDCP_DOWN_STREAM_COUN in another?

>> 
>> Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
>> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
>> ---
>>  drivers/gpu/drm/bridge/ite-it6505.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c 
>> b/drivers/gpu/drm/bridge/ite-it6505.c
>> index d1f5220e04a6..5d5ce12cd054 100644
>> --- a/drivers/gpu/drm/bridge/ite-it6505.c
>> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
>> @@ -296,7 +296,7 @@
>>  #define MAX_LANE_COUNT 4
>>  #define MAX_LINK_RATE HBR
>>  #define AUTO_TRAIN_RETRY 3
>> -#define MAX_HDCP_DOWN_STREAM_COUNT 10
>> +#define MAX_HDCP_DOWN_STREAM_COUNT 127
>>  #define MAX_CR_LEVEL 0x03
>>  #define MAX_EQ_LEVEL 0x03
>>  #define AUX_WAIT_TIMEOUT_MS 15
>> --
>> 2.34.1
>> 
>
>-- 
>With best wishes
>Dmitry
>

BR,
Hermes

Re: [PATCH v4 05/11] drm/bridge: it6505: increase supports of HDCP repeater ksv devices

[Dmitry Baryshkov]

On Fri, Sep 27, 2024 at 02:18:54AM GMT, Hermes.Wu@ite.com.tw wrote:
> >
> >-----Original Message-----
> >From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 
> >Sent: Thursday, September 26, 2024 4:07 PM
> >To: Hermes Wu (吳佳宏) <Hermes.Wu@ite.com.tw>
> >Cc: Kenneth Hung (洪家倫) <Kenneth.Hung@ite.com.tw>; Andrzej Hajda <andrzej.hajda@intel.com>; Neil Armstrong <neil.armstrong@linaro.org>; Robert Foss <rfoss@kernel.org>; Laurent Pinchart <Laurent.pinchart@ideasonboard.com>; Jonas Karlman <jonas@kwiboo.se>; Jernej Skrabec <jernej.skrabec@gmail.com>; Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David Airlie <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Allen Chen <allen.chen@ite.com.tw>; AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>; open list:DRM DRIVERS <dri-devel@lists.freedesktop.org>; open list <linux-kernel@vger.kernel.org>
> >Subject: Re: [PATCH v4 05/11] drm/bridge: it6505: increase supports of HDCP repeater ksv devices
> >
> >On Thu, Sep 26, 2024 at 03:50:14PM GMT, Hermes Wu wrote:
> >> From: Hermes Wu <Hermes.wu@ite.com.tw>
> >> 
> >> A HDCP source shall support max downstream device to 127.
> >> 
> >> Change definition of MAX_HDCP_DOWN_STREAM_COUNT to 127
> >
> >This results in struct it6505 growth by ~0.5 KiB. Please mention it in the commit message.
> >
> >Is it really required to have sha1_input in the constantly allocated structure? I think it's a temporary data, which isn't necessary after processing.
> 
> Change sha1_input with dynamic memory into one commit or 
> 
> change to use temporary data in one and change definition of MAX_HDCP_DOWN_STREAM_COUN in another?

If it's needed during the lifetime, it's fine. Just explain the
necessity to grow the structure in 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 | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c 
> >> b/drivers/gpu/drm/bridge/ite-it6505.c
> >> index d1f5220e04a6..5d5ce12cd054 100644
> >> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> >> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> >> @@ -296,7 +296,7 @@
> >>  #define MAX_LANE_COUNT 4
> >>  #define MAX_LINK_RATE HBR
> >>  #define AUTO_TRAIN_RETRY 3
> >> -#define MAX_HDCP_DOWN_STREAM_COUNT 10
> >> +#define MAX_HDCP_DOWN_STREAM_COUNT 127
> >>  #define MAX_CR_LEVEL 0x03
> >>  #define MAX_EQ_LEVEL 0x03
> >>  #define AUX_WAIT_TIMEOUT_MS 15
> >> --
> >> 2.34.1
> >> 
> >
> >-- 
> >With best wishes
> >Dmitry
> >
> 
> BR,
> Hermes
> 

-- 
With best wishes
Dmitry

RE: [PATCH v4 05/11] drm/bridge: it6505: increase supports of HDCP repeater ksv devices

[Hermes.Wu]

>
>
>-----Original Message-----
>From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 
>Sent: Thursday, September 26, 2024 4:07 PM
>To: Hermes Wu (吳佳宏) <Hermes.Wu@ite.com.tw>
>Cc: Kenneth Hung (洪家倫) <Kenneth.Hung@ite.com.tw>; Andrzej Hajda <andrzej.hajda@intel.com>; Neil Armstrong <neil.armstrong@linaro.org>; Robert Foss <rfoss@kernel.org>; Laurent Pinchart <Laurent.pinchart@ideasonboard.com>; Jonas Karlman <jonas@kwiboo.se>; Jernej Skrabec <jernej.skrabec@gmail.com>; Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David Airlie <airlied@gmail.com>; Simona Vetter <simona@ffwll.ch>; Allen Chen <allen.chen@ite.com.tw>; AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>; open list:DRM DRIVERS <dri-devel@lists.freedesktop.org>; open list <linux-kernel@vger.kernel.org>
>Subject: Re: [PATCH v4 05/11] drm/bridge: it6505: increase supports of HDCP repeater ksv devices
>
>On Thu, Sep 26, 2024 at 03:50:14PM GMT, Hermes Wu wrote:
>> From: Hermes Wu <Hermes.wu@ite.com.tw>
>> 
>> A HDCP source shall support max downstream device to 127.
>> 
>> Change definition of MAX_HDCP_DOWN_STREAM_COUNT to 127
>
>This results in struct it6505 growth by ~0.5 KiB. Please mention it in the commit message.
>
>Is it really required to have sha1_input in the constantly allocated structure? I think it's a temporary data, which isn't necessary after processing.
>

For HDCP, KSV list must store for DRM check, constantly allocated is necessary.

>> 
>> Fixes: b5c84a9edcd4 ("drm/bridge: add it6505 driver")
>> Signed-off-by: Hermes Wu <Hermes.wu@ite.com.tw>
>> ---
>>  drivers/gpu/drm/bridge/ite-it6505.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c 
>> b/drivers/gpu/drm/bridge/ite-it6505.c
>> index d1f5220e04a6..5d5ce12cd054 100644
>> --- a/drivers/gpu/drm/bridge/ite-it6505.c
>> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
>> @@ -296,7 +296,7 @@
>>  #define MAX_LANE_COUNT 4
>>  #define MAX_LINK_RATE HBR
>>  #define AUTO_TRAIN_RETRY 3
>> -#define MAX_HDCP_DOWN_STREAM_COUNT 10
>> +#define MAX_HDCP_DOWN_STREAM_COUNT 127
>>  #define MAX_CR_LEVEL 0x03
>>  #define MAX_EQ_LEVEL 0x03
>>  #define AUX_WAIT_TIMEOUT_MS 15
>> --
>> 2.34.1
>> 
>
>-- 
>With best wishes
>Dmitry
>

BR,
Hermes