Hi Suraj,
> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>
> Sent: Wednesday, April 24, 2024 10:42 AM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@xxxxxxxxx>; Kandpal,
> Suraj <suraj.kandpal@xxxxxxxxx>
> Subject: [PATCH] drm/i915/hdcp: Disable HDCP Line Rekeying for HDCP2.2 on
> HDMI
>
> Disable HDCP Line Rekeying when HDCP ver > 1.4 and when we are on HDMI
> TMDS operation for DISPLAY_VER >= 14.
>
> --v2
> -Wa to be mentioned in comments not in commit message [Jani] -Remove
> blankline [Jani]
>
> --v3
> -No need to write what is being done in comments when code is self
> explanatory [Jani]
>
> --v4
> -Add comment regarding need of this WA when in TMDS mode [Chaitanya] -
> Write in chicken register for MTL [CHaitanya]
>
> Bspec: 49273
> Bspec: 69964
> Signed-off-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_hdcp.c | 26 +++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_reg.h | 4 ++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index d5ed4c7dfbc0..0d8ae6962d34 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -30,6 +30,30 @@
> #define KEY_LOAD_TRIES 5
> #define HDCP2_LC_RETRY_CNT 3
>
> +/*
> + * WA: 16022217614
> + * Disable HDCP line rekeying if we are using HDMI and in
> + * TMDS mode.
> + */
Thank you for the change. My idea was more to document the fact that here we assume that HDMI is in TDMS mode (which is currently true but might change later)
and not the WA itself. More on that below.
> +static void
> +intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder,
> + struct intel_hdcp *hdcp)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
Something on the lines of
/* Here we assume that HDMI is in TMDS mode */
But you are welcome to come up with something better.
Also, s/dev_priv/i915/
> + if (encoder->type != INTEL_OUTPUT_HDMI)
> + return;
> +
> + if (DISPLAY_VER(dev_priv) >= 14) {
> + if (IS_METEORLAKE(dev_priv))
> + intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(hdcp-
> >cpu_transcoder),
> + HDCP_LINE_REKEY_DISABLE, 1);
Since we are only setting the HDCP_LINE_REKEY_DISABLE bit (and clearing nothing) the correct use of the function would be
intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(hdcp->cpu_transcoder), 0, HDCP_LINE_REKEY_DISABLE);
> + else
> + intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(hdcp-
> >cpu_transcoder),
> + TRANS_DDI_HDCP_LINE_REKEY_DISABLE,
> 1);
Same here.
> + }
> +}
> +
> static int intel_conn_to_vcpi(struct intel_atomic_state *state,
> struct intel_connector *connector) { @@ -2005,6
> +2029,8 @@ static int _intel_hdcp2_enable(struct intel_atomic_state *state,
> connector->base.base.id, connector->base.name,
> hdcp->content_type);
>
> + intel_hdcp_disable_hdcp_line_rekeying(connector->encoder, hdcp);
> +
> ret = hdcp2_authenticate_and_encrypt(state, connector);
> if (ret) {
> drm_dbg_kms(&i915->drm, "HDCP2 Type%d Enabling Failed.
> (%d)\n", diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index 3f34efcd7d6c..39b1a2d516fe
> 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4613,6 +4613,8 @@
> [TRANSCODER_D] =
> _CHICKEN_TRANS_D))
> #define _MTL_CHICKEN_TRANS_A 0x604e0
> #define _MTL_CHICKEN_TRANS_B 0x614e0
> +#define _MTL_CHICKEN_TRANS_C 0x624e0
> +#define _MTL_CHICKEN_TRANS_D 0x634e0
These look good.
Regards
Chaitanya
> #define MTL_CHICKEN_TRANS(trans) _MMIO_TRANS((trans), \
> _MTL_CHICKEN_TRANS_A,
> \
> _MTL_CHICKEN_TRANS_B)
> @@ -4631,6 +4633,7 @@
> #define DP_FEC_BS_JITTER_WA REG_BIT(15)
> #define PSR2_VSC_ENABLE_PROG_HEADER REG_BIT(12)
> #define DP_DSC_INSERT_SF_AT_EOL_WA REG_BIT(4)
> +#define HDCP_LINE_REKEY_DISABLE REG_BIT(0)
>
> #define DISP_ARB_CTL _MMIO(0x45000)
> #define DISP_FBC_MEMORY_WAKE REG_BIT(31)
> @@ -5630,6 +5633,7 @@ enum skl_power_gate {
> #define TRANS_DDI_EDP_INPUT_B_ONOFF (5 << 12)
> #define TRANS_DDI_EDP_INPUT_C_ONOFF (6 << 12)
> #define TRANS_DDI_EDP_INPUT_D_ONOFF (7 << 12)
> +#define TRANS_DDI_HDCP_LINE_REKEY_DISABLE REG_BIT(12)
> #define TRANS_DDI_MST_TRANSPORT_SELECT_MASK REG_GENMASK(11,
> 10)
> #define TRANS_DDI_MST_TRANSPORT_SELECT(trans) \
> REG_FIELD_PREP(TRANS_DDI_MST_TRANSPORT_SELECT_MASK,
> trans)
> --
> 2.43.2
