| Message ID | 20241104083143.631760-2-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] drm/i915/xe3lpd: Disable HDCP Line Rekeying for Xe3 | expand |
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@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 >
> -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@intel.com> > Sent: Monday, November 4, 2024 11:45 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Subject: Re: [PATCH 2/2] drm/i915/hdcp: Handle HDCP Line Rekeying for HDCP > 1.4 > > 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@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). Sure will separate this out as a different patch altogether as for the stepping specific implementation In bspec 50054 it specifically state for which steppings MTL_CHICKEN_TRANS needs to have its bit set/unset Regards, Suraj Kandpal > > > 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
> -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@intel.com> > Sent: Monday, November 4, 2024 11:45 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Subject: Re: [PATCH 2/2] drm/i915/hdcp: Handle HDCP Line Rekeying for HDCP > 1.4 > > 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@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). Also since the constraints remain same for both enablement and disablement we can use the same if else checks as before using the above mentioned method if that sounds good. Regards, Suraj Kandpal > > > 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
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) { + 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)) + 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);
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@intel.com> --- drivers/gpu/drm/i915/display/intel_hdcp.c | 28 +++++++++++++++++++++++ 1 file changed, 28 insertions(+)