drm/amd/display: fw locality check refactors

[why]
There are some new changes for HDCP2 firmware locality check. The
implementation doesn't perfectly fit the intended design and clarity.

1. Clarify and consolidate variable responsibilities.
The previous implementation introduced the following variables:
- config.ddc.funcs.atomic_write_poll_read_i2c (optional pointer)
- hdcp->config.ddc.funcs.atomic_write_poll_read_aux (optional pointer)
- hdcp->connection.link.adjust.hdcp2.force_sw_locality_check (bool)
- hdcp->config.debug.lc_enable_sw_fallback (bool)
- use_fw (bool)
They will be used together to determine two operations:
- Whether to use FW locality check
- Whether to use SW fallback on FW locality check failure
The refactor streamlines this by introducing two variables in the hdcp2
link adjustment, while ensuring function pointers are always assigned
and remain independent from policy decisions:
- use_fw_locality_check (bool) -> true if fw locality should be used.
- use_sw_locality_fallback (bool) -> true to reset use_fw_locality_check
back to false and retry on fw locality check failure.

2. Mixed meanings of l_prime_read transition input
l_prime_read originally means if l_prime is read when sw locality check
is used. When FW locality check is used, l_prime_read means if lc init
write, l prime poll and l_prime read combo operation is successful. The
mix of meanings is confusing. The refactor introduces a new variable
l_prime_combo_read to isolate the second meaning into its own variable.

3. Missing specific error code on firmware locality error.
The original change reuses the generic DDC failure error code when
firmware fails to return locality check result. This is not ideal as
DDC failure indicates an error occurred during an I2C/AUX transaction.
FW locality failure could be caused by polling timeout in firmware or
failure to acquire firmware access. Which sits at a higher level of
abstraction above DDC hardware. An incorrect error code could mislead
the debug into a wrong direction.

4. Correcting misplaced comments. The previous implementation of the
firmware locality check resulted in some comments in hdcp2_transition
being incorrectly positioned. This refactor relocates those comments to
their appropriate locations for better clarity.

Reviewed-by: Aric Cyr <aric.cyr@amd.com>
Signed-off-by: Wenjing Liu <wenjing.liu@amd.com>
Signed-off-by: Ray Wu <ray.wu@amd.com>
Tested-by: Daniel Wheeler <daniel.wheeler@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
index 19038f336155320042c8228bed8c5ccf79e5420b..85ce558cefc58c73d009d5eb51616650f09564bb 100644 (file)
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
@@ -201,6 +201,7 @@ void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
        struct mod_hdcp_link_adjustment link_adjust;
        struct mod_hdcp_display_adjustment display_adjust;
        unsigned int conn_index = aconnector->base.index;
+       const struct dc *dc = aconnector->dc_link->dc;
 
        guard(mutex)(&hdcp_w->mutex);
        drm_connector_get(&aconnector->base);
@@ -231,6 +232,9 @@ void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
                        link_adjust.hdcp1.disable = 1;
                        link_adjust.hdcp2.force_type = MOD_HDCP_FORCE_TYPE_1;
                }
+               link_adjust.hdcp2.use_fw_locality_check =
+                               (dc->caps.fused_io_supported || dc->debug.hdcp_lc_force_fw_enable);
+               link_adjust.hdcp2.use_sw_locality_fallback = dc->debug.hdcp_lc_enable_sw_fallback;
 
                schedule_delayed_work(&hdcp_w->property_validate_dwork,
                                      msecs_to_jiffies(DRM_HDCP_CHECK_PERIOD_MS));
@@ -534,6 +538,7 @@ static void update_config(void *handle, struct cp_psp_stream_config *config)
        struct hdcp_workqueue *hdcp_w = &hdcp_work[link_index];
        struct dc_sink *sink = NULL;
        bool link_is_hdcp14 = false;
+       const struct dc *dc = aconnector->dc_link->dc;
 
        if (config->dpms_off) {
                hdcp_remove_display(hdcp_work, link_index, aconnector);
@@ -575,6 +580,8 @@ static void update_config(void *handle, struct cp_psp_stream_config *config)
        link->adjust.auth_delay = 2;
        link->adjust.retry_limit = MAX_NUM_OF_ATTEMPTS;
        link->adjust.hdcp1.disable = 0;
+       link->adjust.hdcp2.use_fw_locality_check = (dc->caps.fused_io_supported || dc->debug.hdcp_lc_force_fw_enable);
+       link->adjust.hdcp2.use_sw_locality_fallback = dc->debug.hdcp_lc_enable_sw_fallback;
        hdcp_w->encryption_status[display->index] = MOD_HDCP_ENCRYPTION_STATUS_HDCP_OFF;
 
        DRM_DEBUG_DRIVER("[HDCP_DM] display %d, CP %d, type %d\n", aconnector->base.index,
@@ -786,15 +793,8 @@ struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev,
                ddc_funcs->read_i2c = lp_read_i2c;
                ddc_funcs->write_dpcd = lp_write_dpcd;
                ddc_funcs->read_dpcd = lp_read_dpcd;
-
-               config->debug.lc_enable_sw_fallback = dc->debug.hdcp_lc_enable_sw_fallback;
-               if (dc->caps.fused_io_supported || dc->debug.hdcp_lc_force_fw_enable) {
-                       ddc_funcs->atomic_write_poll_read_i2c = lp_atomic_write_poll_read_i2c;
-                       ddc_funcs->atomic_write_poll_read_aux = lp_atomic_write_poll_read_aux;
-               } else {
-                       ddc_funcs->atomic_write_poll_read_i2c = NULL;
-                       ddc_funcs->atomic_write_poll_read_aux = NULL;
-               }
+               ddc_funcs->atomic_write_poll_read_i2c = lp_atomic_write_poll_read_i2c;
+               ddc_funcs->atomic_write_poll_read_aux = lp_atomic_write_poll_read_aux;
 
                memset(hdcp_work[i].aconnector, 0,
                       sizeof(struct amdgpu_dm_connector *) *

diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h
index b883d626f1c372377341eba0b19c1b47873797cf..26a351a184f33467e01aa998e42078e746c8b05d 100644 (file)
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp.h
@@ -88,6 +88,7 @@ struct mod_hdcp_transition_input_hdcp2 {
        uint8_t lc_init_write;
        uint8_t l_prime_available_poll;
        uint8_t l_prime_read;
+       uint8_t l_prime_combo_read;
        uint8_t l_prime_validation;
        uint8_t eks_prepare;
        uint8_t eks_write;

diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c
index 5628f0ef73fd1edd79f06fa526ceed36d3499673..27500abf9fee36b3ba6f307e924e40176e8d0eb8 100644 (file)
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c
@@ -465,54 +465,11 @@ out:
        return status;
 }
 
-static enum mod_hdcp_status locality_check_sw(struct mod_hdcp *hdcp,
-               struct mod_hdcp_event_context *event_ctx,
-               struct mod_hdcp_transition_input_hdcp2 *input)
-{
-       enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS;
-
-       if (!mod_hdcp_execute_and_set(mod_hdcp_write_lc_init,
-                       &input->lc_init_write, &status,
-                        hdcp, "lc_init_write"))
-               goto out;
-       if (is_dp_hdcp(hdcp))
-               msleep(16);
-       else
-               if (!mod_hdcp_execute_and_set(poll_l_prime_available,
-                               &input->l_prime_available_poll, &status,
-                               hdcp, "l_prime_available_poll"))
-                       goto out;
-       if (!mod_hdcp_execute_and_set(mod_hdcp_read_l_prime,
-                       &input->l_prime_read, &status,
-                       hdcp, "l_prime_read"))
-               goto out;
-out:
-       return status;
-}
-
-static enum mod_hdcp_status locality_check_fw(struct mod_hdcp *hdcp,
-               struct mod_hdcp_event_context *event_ctx,
-               struct mod_hdcp_transition_input_hdcp2 *input)
-{
-       enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS;
-
-       if (!mod_hdcp_execute_and_set(mod_hdcp_write_poll_read_lc_fw,
-                       &input->l_prime_read, &status,
-                       hdcp, "l_prime_read"))
-               goto out;
-
-out:
-       return status;
-}
-
 static enum mod_hdcp_status locality_check(struct mod_hdcp *hdcp,
                struct mod_hdcp_event_context *event_ctx,
                struct mod_hdcp_transition_input_hdcp2 *input)
 {
        enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS;
-       const bool use_fw = hdcp->config.ddc.funcs.atomic_write_poll_read_i2c
-                       && hdcp->config.ddc.funcs.atomic_write_poll_read_aux
-                       && !hdcp->connection.link.adjust.hdcp2.force_sw_locality_check;
 
        if (event_ctx->event != MOD_HDCP_EVENT_CALLBACK) {
                event_ctx->unexpected_event = 1;
@@ -524,9 +481,28 @@ static enum mod_hdcp_status locality_check(struct mod_hdcp *hdcp,
                        hdcp, "lc_init_prepare"))
                goto out;
 
-       status = (use_fw ? locality_check_fw : locality_check_sw)(hdcp, event_ctx, input);
-       if (status != MOD_HDCP_STATUS_SUCCESS)
-               goto out;
+       if (hdcp->connection.link.adjust.hdcp2.use_fw_locality_check) {
+               if (!mod_hdcp_execute_and_set(mod_hdcp_write_poll_read_lc_fw,
+                               &input->l_prime_combo_read, &status,
+                               hdcp, "l_prime_combo_read"))
+                       goto out;
+       } else {
+               if (!mod_hdcp_execute_and_set(mod_hdcp_write_lc_init,
+                               &input->lc_init_write, &status,
+                               hdcp, "lc_init_write"))
+                       goto out;
+               if (is_dp_hdcp(hdcp))
+                       msleep(16);
+               else
+                       if (!mod_hdcp_execute_and_set(poll_l_prime_available,
+                                       &input->l_prime_available_poll, &status,
+                                       hdcp, "l_prime_available_poll"))
+                               goto out;
+               if (!mod_hdcp_execute_and_set(mod_hdcp_read_l_prime,
+                               &input->l_prime_read, &status,
+                               hdcp, "l_prime_read"))
+                       goto out;
+       }
 
        if (!mod_hdcp_execute_and_set(mod_hdcp_hdcp2_validate_l_prime,
                        &input->l_prime_validation, &status,

diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_transition.c b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_transition.c
index 89ffb89e193256a9cf395d87952df4163d8d5d56..9316312a4df5b7f7ec2afb819fb311ed704f6daa 100644 (file)
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_transition.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_transition.c
@@ -184,31 +184,33 @@ enum mod_hdcp_status mod_hdcp_hdcp2_transition(struct mod_hdcp *hdcp,
                callback_in_ms(0, output);
                set_state_id(hdcp, output, H2_A2_LOCALITY_CHECK);
                break;
-       case H2_A2_LOCALITY_CHECK: {
-               const bool use_fw = hdcp->config.ddc.funcs.atomic_write_poll_read_i2c
-                               && !adjust->hdcp2.force_sw_locality_check;
-
-               /*
-                * 1A-05: consider disconnection after LC init a failure
-                * 1A-13-1: consider invalid l' a failure
-                * 1A-13-2: consider l' timeout a failure
-                */
+       case H2_A2_LOCALITY_CHECK:
+               /* 1A-05: consider disconnection after LC init a failure */
                if (hdcp->state.stay_count > 10 ||
-                               input->lc_init_prepare != PASS ||
-                               (!use_fw && input->lc_init_write != PASS) ||
-                               (!use_fw && input->l_prime_available_poll != PASS)) {
+                               input->lc_init_prepare != PASS) {
                        fail_and_restart_in_ms(0, &status, output);
                        break;
-               } else if (input->l_prime_read != PASS) {
-                       if (use_fw && hdcp->config.debug.lc_enable_sw_fallback) {
-                               adjust->hdcp2.force_sw_locality_check = true;
+               } else if (adjust->hdcp2.use_fw_locality_check &&
+                               input->l_prime_combo_read != PASS) {
+                       /* 1A-13-2: consider l' timeout a failure */
+                       if (adjust->hdcp2.use_sw_locality_fallback) {
+                               /* switch to software locality check */
+                               adjust->hdcp2.use_fw_locality_check = 0;
                                callback_in_ms(0, output);
+                               increment_stay_counter(hdcp);
                                break;
                        }
-
+                       fail_and_restart_in_ms(0, &status, output);
+                       break;
+               } else if (!adjust->hdcp2.use_fw_locality_check &&
+                                       (input->lc_init_write != PASS ||
+                                       input->l_prime_available_poll != PASS ||
+                                       input->l_prime_read != PASS)) {
+                       /* 1A-13-2: consider l' timeout a failure */
                        fail_and_restart_in_ms(0, &status, output);
                        break;
                } else if (input->l_prime_validation != PASS) {
+                       /* 1A-13-1: consider invalid l' a failure */
                        callback_in_ms(0, output);
                        increment_stay_counter(hdcp);
                        break;
@@ -216,7 +218,6 @@ enum mod_hdcp_status mod_hdcp_hdcp2_transition(struct mod_hdcp *hdcp,
                callback_in_ms(0, output);
                set_state_id(hdcp, output, H2_A3_EXCHANGE_KS_AND_TEST_FOR_REPEATER);
                break;
-       }
        case H2_A3_EXCHANGE_KS_AND_TEST_FOR_REPEATER:
                if (input->eks_prepare != PASS ||
                                input->eks_write != PASS) {
@@ -510,26 +511,29 @@ enum mod_hdcp_status mod_hdcp_hdcp2_dp_transition(struct mod_hdcp *hdcp,
                callback_in_ms(0, output);
                set_state_id(hdcp, output, D2_A2_LOCALITY_CHECK);
                break;
-       case D2_A2_LOCALITY_CHECK: {
-               const bool use_fw = hdcp->config.ddc.funcs.atomic_write_poll_read_aux
-                               && !adjust->hdcp2.force_sw_locality_check;
-
+       case D2_A2_LOCALITY_CHECK:
                if (hdcp->state.stay_count > 10 ||
-                               input->lc_init_prepare != PASS ||
-                               (!use_fw && input->lc_init_write != PASS)) {
-                       /* 1A-12: consider invalid l' a failure */
+                               input->lc_init_prepare != PASS) {
                        fail_and_restart_in_ms(0, &status, output);
                        break;
-               } else if (input->l_prime_read != PASS) {
-                       if (use_fw && hdcp->config.debug.lc_enable_sw_fallback) {
-                               adjust->hdcp2.force_sw_locality_check = true;
+               } else if (adjust->hdcp2.use_fw_locality_check &&
+                               input->l_prime_combo_read != PASS) {
+                       if (adjust->hdcp2.use_sw_locality_fallback) {
+                               /* switch to software locality check */
+                               adjust->hdcp2.use_fw_locality_check = 0;
                                callback_in_ms(0, output);
+                               increment_stay_counter(hdcp);
                                break;
                        }
-
+                       fail_and_restart_in_ms(0, &status, output);
+                       break;
+               } else if (!adjust->hdcp2.use_fw_locality_check &&
+                                       (input->lc_init_write != PASS ||
+                                       input->l_prime_read != PASS)) {
                        fail_and_restart_in_ms(0, &status, output);
                        break;
                } else if (input->l_prime_validation != PASS) {
+                       /* 1A-12: consider invalid l' a failure */
                        callback_in_ms(0, output);
                        increment_stay_counter(hdcp);
                        break;
@@ -537,7 +541,6 @@ enum mod_hdcp_status mod_hdcp_hdcp2_dp_transition(struct mod_hdcp *hdcp,
                callback_in_ms(0, output);
                set_state_id(hdcp, output, D2_A34_EXCHANGE_KS_AND_TEST_FOR_REPEATER);
                break;
-       }
        case D2_A34_EXCHANGE_KS_AND_TEST_FOR_REPEATER:
                if (input->eks_prepare != PASS ||
                                input->eks_write != PASS) {

diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_ddc.c b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_ddc.c
index 2e64085791948db4735dfcfe85f00b2e9e74626b..0ca39873f80733ccf86d4694b69403301f9f01b5 100644 (file)
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_ddc.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_ddc.c
@@ -758,6 +758,6 @@ enum mod_hdcp_status mod_hdcp_write_poll_read_lc_fw(struct mod_hdcp *hdcp)
 {
        const bool success = (is_dp_hdcp(hdcp) ? write_stall_read_lc_fw_aux : write_poll_read_lc_fw_i2c)(hdcp);
 
-       return success ? MOD_HDCP_STATUS_SUCCESS : MOD_HDCP_STATUS_DDC_FAILURE;
+       return success ? MOD_HDCP_STATUS_SUCCESS : MOD_HDCP_STATUS_HDCP2_LOCALITY_COMBO_READ_FAILURE;
 }
 

diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c
index 6b3b5f610907d7b4e8787055ff5500986d79b6fb..ac44ee1532fdad6801abec9ac7dede8347fc23d7 100644 (file)
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c
@@ -248,6 +248,8 @@ char *mod_hdcp_status_to_str(int32_t status)
                return "MOD_HDCP_STATUS_HDCP2_DEVICE_COUNT_MISMATCH_FAILURE";
        case MOD_HDCP_STATUS_UNSUPPORTED_PSP_VER_FAILURE:
                return "MOD_HDCP_STATUS_UNSUPPORTED_PSP_VER_FAILURE";
+       case MOD_HDCP_STATUS_HDCP2_LOCALITY_COMBO_READ_FAILURE:
+               return "MOD_HDCP_STATUS_HDCP2_LOCALITY_COMBO_READ_FAILURE";
        default:
                return "MOD_HDCP_STATUS_UNKNOWN";
        }

diff --git a/drivers/gpu/drm/amd/display/modules/inc/mod_hdcp.h b/drivers/gpu/drm/amd/display/modules/inc/mod_hdcp.h
index 46e52fb3a1180913897888cf3a36675b94da6de5..264348989e9bc313ee043c19f2373299d32312aa 100644 (file)
--- a/drivers/gpu/drm/amd/display/modules/inc/mod_hdcp.h
+++ b/drivers/gpu/drm/amd/display/modules/inc/mod_hdcp.h
@@ -98,6 +98,7 @@ enum mod_hdcp_status {
        MOD_HDCP_STATUS_HDCP2_REAUTH_LINK_INTEGRITY_FAILURE,
        MOD_HDCP_STATUS_HDCP2_DEVICE_COUNT_MISMATCH_FAILURE,
        MOD_HDCP_STATUS_UNSUPPORTED_PSP_VER_FAILURE,
+       MOD_HDCP_STATUS_HDCP2_LOCALITY_COMBO_READ_FAILURE,
 };
 
 struct mod_hdcp_displayport {
@@ -214,8 +215,9 @@ struct mod_hdcp_link_adjustment_hdcp2 {
        uint8_t force_type              : 2;
        uint8_t force_no_stored_km      : 1;
        uint8_t increase_h_prime_timeout: 1;
-       uint8_t force_sw_locality_check : 1;
-       uint8_t reserved                : 2;
+       uint8_t use_fw_locality_check   : 1;
+       uint8_t use_sw_locality_fallback: 1;
+       uint8_t reserved                : 1;
 };
 
 struct mod_hdcp_link_adjustment {
@@ -317,10 +319,6 @@ struct mod_hdcp_display_query {
 struct mod_hdcp_config {
        struct mod_hdcp_psp psp;
        struct mod_hdcp_ddc ddc;
-       struct {
-               uint8_t lc_enable_sw_fallback : 1;
-               uint8_t reserved : 7;
-       } debug;
        uint8_t index;
 };