diff mbox series

drm/i915/hdcp: Retry first read and writes to downstream

Message ID 20240926071400.1406949-2-suraj.kandpal@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/hdcp: Retry first read and writes to downstream | expand

Commit Message

Kandpal, Suraj Sept. 26, 2024, 7:14 a.m. UTC
Retry the first read and write to downstream at least 10 times
with a 50ms delay if not hdcp2 capable(dock decides to stop advertising
hdcp2 capability for some reason). The reason being that
during suspend resume Dock usually keep the HDCP2 registers inaccesible
causing AUX error. This wouldn't be a big problem if the userspace
just kept retrying with some delay while it continues to play low
values content but most userpace applications end up throwing an error
when it receives one from KMD. This makes sure we give the dock
and the sink devices to complete its power cycle and then try HDCP
authentication.

--v2
-Add more details in comment [Jani]
-fix looping condition [Jani]
-optimize loop exit condition [Jani]

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_hdcp.c | 24 ++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

Comments

Nautiyal, Ankit K Sept. 27, 2024, 4:52 a.m. UTC | #1
On 9/26/2024 12:44 PM, Suraj Kandpal wrote:
> Retry the first read and write to downstream at least 10 times
> with a 50ms delay if not hdcp2 capable(dock decides to stop advertising
> hdcp2 capability for some reason). The reason being that
> during suspend resume Dock usually keep the HDCP2 registers inaccesible
> causing AUX error. This wouldn't be a big problem if the userspace
> just kept retrying with some delay while it continues to play low
> values content but most userpace applications end up throwing an error
> when it receives one from KMD. This makes sure we give the dock
> and the sink devices to complete its power cycle and then try HDCP
> authentication.
>
> --v2
> -Add more details in comment [Jani]
> -fix looping condition [Jani]
> -optimize loop exit condition [Jani]
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_hdcp.c | 24 ++++++++++++++++-------
>   1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 2afa92321b08..46b6f30f9776 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -1512,7 +1512,7 @@ static int hdcp2_authentication_key_exchange(struct intel_connector *connector)
>   	} msgs;
>   	const struct intel_hdcp_shim *shim = hdcp->shim;
>   	size_t size;
> -	int ret;
> +	int ret, i;
>   
>   	/* Init for seq_num */
>   	hdcp->seq_num_v = 0;
> @@ -1522,13 +1522,23 @@ static int hdcp2_authentication_key_exchange(struct intel_connector *connector)
>   	if (ret < 0)
>   		return ret;
>   
> -	ret = shim->write_2_2_msg(connector, &msgs.ake_init,
> -				  sizeof(msgs.ake_init));
> -	if (ret < 0)
> -		return ret;
> +	for (i = 0; i < 10; i++) {

I think lets add as a note for the reasoning for reading caps before 
writing the message, and trying 10 times with 50 ms sleep, similar to 
the commit message.

Overall the change looks good to me.

With the above note:

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>



> +		if (!intel_hdcp2_get_capability(connector)) {
> +			msleep(50);
> +			continue;
> +		}
> +
> +		ret = shim->write_2_2_msg(connector, &msgs.ake_init,
> +					  sizeof(msgs.ake_init));
> +		if (ret < 0)
> +			continue;
> +
> +		ret = shim->read_2_2_msg(connector, HDCP_2_2_AKE_SEND_CERT,
> +					 &msgs.send_cert, sizeof(msgs.send_cert));
> +		if (ret > 0)
> +			break;
> +	}
>   
> -	ret = shim->read_2_2_msg(connector, HDCP_2_2_AKE_SEND_CERT,
> -				 &msgs.send_cert, sizeof(msgs.send_cert));
>   	if (ret < 0)
>   		return ret;
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 2afa92321b08..46b6f30f9776 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -1512,7 +1512,7 @@  static int hdcp2_authentication_key_exchange(struct intel_connector *connector)
 	} msgs;
 	const struct intel_hdcp_shim *shim = hdcp->shim;
 	size_t size;
-	int ret;
+	int ret, i;
 
 	/* Init for seq_num */
 	hdcp->seq_num_v = 0;
@@ -1522,13 +1522,23 @@  static int hdcp2_authentication_key_exchange(struct intel_connector *connector)
 	if (ret < 0)
 		return ret;
 
-	ret = shim->write_2_2_msg(connector, &msgs.ake_init,
-				  sizeof(msgs.ake_init));
-	if (ret < 0)
-		return ret;
+	for (i = 0; i < 10; i++) {
+		if (!intel_hdcp2_get_capability(connector)) {
+			msleep(50);
+			continue;
+		}
+
+		ret = shim->write_2_2_msg(connector, &msgs.ake_init,
+					  sizeof(msgs.ake_init));
+		if (ret < 0)
+			continue;
+
+		ret = shim->read_2_2_msg(connector, HDCP_2_2_AKE_SEND_CERT,
+					 &msgs.send_cert, sizeof(msgs.send_cert));
+		if (ret > 0)
+			break;
+	}
 
-	ret = shim->read_2_2_msg(connector, HDCP_2_2_AKE_SEND_CERT,
-				 &msgs.send_cert, sizeof(msgs.send_cert));
 	if (ret < 0)
 		return ret;