[PATCH 2/2] drm/i915/hdcp: Handle HDCP Line Rekeying for HDCP 1.4

[Matt Roper]

On Mon, Nov 04, 2024 at 02:01:43PM +0530, Suraj Kandpal wrote:
> TRANS_DDI_FUNC_CTL asks us to disable hdcp line rekeying when not in
> hdcp 2.2 and we are not using an hdmi transcoder and it need to be
> enabled when we are using an HDMI transcoder to enable HDCP 1.4.
> We use intel_de_rmw cycles to update TRANS_DDI_FUNC_CTL register so
> we cannot depend on the value being 0 by default everytime which calls
> for seprate handling of HDCP 1.4 case.
> 
> Bspec: 69964, 50493, 50054
> Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdcp.c | 28 +++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 8bca532d1176..54efba65ef5a 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -31,6 +31,32 @@
>  #define KEY_LOAD_TRIES	5
>  #define HDCP2_LC_RETRY_CNT			3
>  
> +static void
> +intel_hdcp_enable_hdcp_line_rekeying(struct intel_encoder *encoder,
> +				     struct intel_hdcp *hdcp)
> +{
> +	struct intel_display *display = to_intel_display(encoder);
> +
> +	/* Here we assume HDMI is in TMDS mode of operation */
> +	if (encoder->type != INTEL_OUTPUT_HDMI)
> +		return;
> +
> +	if (DISPLAY_VER(display) >= 14) {

As noted on the previous patch, this outer 'if' doesn't do anything
since none of the nested if's will match versions less than 14.

> +		if (DISPLAY_VER(display) >= 30)
> +			intel_de_rmw(display,
> +				     TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder),
> +				     XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE, 0);
> +		else if (IS_DISPLAY_VERx100_STEP(display, 1400, STEP_D0, STEP_FOREVER))
> +			intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp->cpu_transcoder),
> +				     HDCP_LINE_REKEY_DISABLE, 0);
> +		else if (IS_DISPLAY_VERx100_STEP(display, 1401, STEP_B0, STEP_FOREVER) ||
> +			 IS_DISPLAY_VERx100_STEP(display, 2000, STEP_B0, STEP_FOREVER))

For new code we should definitely be ordering if/else ladders in
descending order.  So the Xe2 clause here should come before the MTL
clause.

Although it might be cleaner to just have a single function that takes a
boolean parameter to enable/disable rekeying?  E.g., something along the
lines of:

    static void
    intel_hdcp_adjust_hdcp_line_rekeying(struct intel_encoder *encoder,
                                         struct intel_hdcp *hdcp,
                                         bool enable)
    {
        struct intel_reg reky_reg;
        u32 rekey_bit;

        if (DISPLAY_VER(display) >= 30) {
            rekey_reg = TRANS_DDI_FUNC_CTL;
            rekey_bit = XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE;
        } else if (DISPLAY_VERx100(display) >= 1401) {
            rekey_reg = TRANS_DDI_FUNC_CTL;
            rekey_bit = TRANS_DDI_HDCP_LINE_REKEY_DISABLE;
        } else if (DISPLAY_VERx100(display) == 1400)
            rekey_reg = MTL_CHICKEN_TRANS(hdcp->cpu_transcoder);
            rekey_bit = HDCP_LINE_REKEY_DISABLE;
        } else {
            return;
        }

        intel_de_rmw(display, rekey_reg, rekey_bit,
                     enable ? 0 : rekey_bit);
     }

And we can move the stepping-specific workaround implementation to the
callsite to make it clear that the implementation of enabling/disabling
is separate from the decision whether to enable/disable (as impacted by
the workaround).


Matt

> +			intel_de_rmw(display,
> +				     TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder),
> +				     TRANS_DDI_HDCP_LINE_REKEY_DISABLE, 0);
> +	}
> +}
> +
>  static void
>  intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder,
>  				      struct intel_hdcp *hdcp)
> @@ -1051,6 +1077,8 @@ static int intel_hdcp1_enable(struct intel_connector *connector)
>  		return ret;
>  	}
>  
> +	intel_hdcp_enable_hdcp_line_rekeying(connector->encoder, hdcp);
> +
>  	/* Incase of authentication failures, HDCP spec expects reauth. */
>  	for (i = 0; i < tries; i++) {
>  		ret = intel_hdcp_auth(connector);
> -- 
> 2.34.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation