| Message ID | 20241106125100.852609-1-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/hdcp: Handle HDCP Line Rekeying for HDCP 1.4 | expand |
On Wed, 06 Nov 2024, Suraj Kandpal <suraj.kandpal@intel.com> 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. > > --v2 > -Use the exising function and modify it based on a bool rather than > have a different function [Matt] > > --v3 > -No need for kzalloc [Jani] The code still needs to make sense! You can't just remove allocation and use whatever stack garbage rekey_reg happens to point at. Look at all the other code that uses i915_reg_t. You can't possibly assume I would've needed to say "it shouldn't be a pointer at all"? BR, Jani. > > Bspec: 69964, 50493, 50054 > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_hdcp.c | 36 +++++++++++++---------- > 1 file changed, 21 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c > index 4e937fbba4d2..6ad5efc9711c 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -32,27 +32,31 @@ > #define HDCP2_LC_RETRY_CNT 3 > > static void > -intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder, > - struct intel_hdcp *hdcp) > +intel_hdcp_adjust_hdcp_line_rekeying(struct intel_encoder *encoder, > + struct intel_hdcp *hdcp, > + bool enable) > { > struct intel_display *display = to_intel_display(encoder); > + i915_reg_t *rekey_reg; > + u32 rekey_bit; > > /* Here we assume HDMI is in TMDS mode of operation */ > if (encoder->type != INTEL_OUTPUT_HDMI) > return; > > - if (DISPLAY_VER(display) >= 30) > - intel_de_rmw(display, > - TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder), > - 0, XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > - 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), > - 0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > - else if (IS_DISPLAY_VERx100_STEP(display, 1400, STEP_D0, STEP_FOREVER)) > - intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp->cpu_transcoder), > - 0, HDCP_LINE_REKEY_DISABLE); > + if (DISPLAY_VER(display) >= 30) { > + *rekey_reg = TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder); > + rekey_bit = XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE; > + } else if (IS_DISPLAY_VERx100_STEP(display, 1401, STEP_B0, STEP_FOREVER) || > + IS_DISPLAY_VERx100_STEP(display, 2000, STEP_B0, STEP_FOREVER)) { > + *rekey_reg = TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder); > + rekey_bit = TRANS_DDI_HDCP_LINE_REKEY_DISABLE; > + } else if (IS_DISPLAY_VERx100_STEP(display, 1400, STEP_D0, STEP_FOREVER)) { > + *rekey_reg = MTL_CHICKEN_TRANS(hdcp->cpu_transcoder); > + rekey_bit = HDCP_LINE_REKEY_DISABLE; > + } > + > + intel_de_rmw(display, *rekey_reg, rekey_bit, enable ? 0 : rekey_bit); > } > > static int intel_conn_to_vcpi(struct intel_atomic_state *state, > @@ -1049,6 +1053,8 @@ static int intel_hdcp1_enable(struct intel_connector *connector) > return ret; > } > > + intel_hdcp_adjust_hdcp_line_rekeying(connector->encoder, hdcp, true); > + > /* Incase of authentication failures, HDCP spec expects reauth. */ > for (i = 0; i < tries; i++) { > ret = intel_hdcp_auth(connector); > @@ -2062,7 +2068,7 @@ 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); > + intel_hdcp_adjust_hdcp_line_rekeying(connector->encoder, hdcp, false); > > ret = hdcp2_authenticate_and_encrypt(state, connector); > if (ret) {
> -----Original Message----- > From: Nikula, Jani <jani.nikula@intel.com> > Sent: Wednesday, November 6, 2024 9:59 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-xe@lists.freedesktop.org; > intel-gfx@lists.freedesktop.org > Cc: Roper, Matthew D <matthew.d.roper@intel.com>; Kandpal, Suraj > <suraj.kandpal@intel.com> > Subject: Re: [PATCH] drm/i915/hdcp: Handle HDCP Line Rekeying for HDCP 1.4 > > On Wed, 06 Nov 2024, Suraj Kandpal <suraj.kandpal@intel.com> 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. > > > > --v2 > > -Use the exising function and modify it based on a bool rather than > > have a different function [Matt] > > > > --v3 > > -No need for kzalloc [Jani] > > The code still needs to make sense! You can't just remove allocation and use > whatever stack garbage rekey_reg happens to point at. Look at all the other > code that uses i915_reg_t. You can't possibly assume I would've needed to > say "it shouldn't be a pointer at all"? > Sure Jani ill fix that, I thought I had changed it from a pointer to a normal variable, Seems to have missed it. Regards, Suraj Kandpal > BR, > Jani. > > > > > Bspec: 69964, 50493, 50054 > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_hdcp.c | 36 > > +++++++++++++---------- > > 1 file changed, 21 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > index 4e937fbba4d2..6ad5efc9711c 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > @@ -32,27 +32,31 @@ > > #define HDCP2_LC_RETRY_CNT 3 > > > > static void > > -intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder, > > - struct intel_hdcp *hdcp) > > +intel_hdcp_adjust_hdcp_line_rekeying(struct intel_encoder *encoder, > > + struct intel_hdcp *hdcp, > > + bool enable) > > { > > struct intel_display *display = to_intel_display(encoder); > > + i915_reg_t *rekey_reg; > > + u32 rekey_bit; > > > > /* Here we assume HDMI is in TMDS mode of operation */ > > if (encoder->type != INTEL_OUTPUT_HDMI) > > return; > > > > - if (DISPLAY_VER(display) >= 30) > > - intel_de_rmw(display, > > - TRANS_DDI_FUNC_CTL(display, hdcp- > >cpu_transcoder), > > - 0, XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > > - 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), > > - 0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE); > > - else if (IS_DISPLAY_VERx100_STEP(display, 1400, STEP_D0, > STEP_FOREVER)) > > - intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp- > >cpu_transcoder), > > - 0, HDCP_LINE_REKEY_DISABLE); > > + if (DISPLAY_VER(display) >= 30) { > > + *rekey_reg = TRANS_DDI_FUNC_CTL(display, hdcp- > >cpu_transcoder); > > + rekey_bit = XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE; > > + } else if (IS_DISPLAY_VERx100_STEP(display, 1401, STEP_B0, > STEP_FOREVER) || > > + IS_DISPLAY_VERx100_STEP(display, 2000, STEP_B0, > STEP_FOREVER)) { > > + *rekey_reg = TRANS_DDI_FUNC_CTL(display, hdcp- > >cpu_transcoder); > > + rekey_bit = TRANS_DDI_HDCP_LINE_REKEY_DISABLE; > > + } else if (IS_DISPLAY_VERx100_STEP(display, 1400, STEP_D0, > STEP_FOREVER)) { > > + *rekey_reg = MTL_CHICKEN_TRANS(hdcp->cpu_transcoder); > > + rekey_bit = HDCP_LINE_REKEY_DISABLE; > > + } > > + > > + intel_de_rmw(display, *rekey_reg, rekey_bit, enable ? 0 : > > +rekey_bit); > > } > > > > static int intel_conn_to_vcpi(struct intel_atomic_state *state, @@ > > -1049,6 +1053,8 @@ static int intel_hdcp1_enable(struct intel_connector > *connector) > > return ret; > > } > > > > + intel_hdcp_adjust_hdcp_line_rekeying(connector->encoder, hdcp, > > +true); > > + > > /* Incase of authentication failures, HDCP spec expects reauth. */ > > for (i = 0; i < tries; i++) { > > ret = intel_hdcp_auth(connector); > > @@ -2062,7 +2068,7 @@ 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); > > + intel_hdcp_adjust_hdcp_line_rekeying(connector->encoder, hdcp, > > +false); > > > > ret = hdcp2_authenticate_and_encrypt(state, connector); > > if (ret) { > > -- > Jani Nikula, Intel
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 4e937fbba4d2..6ad5efc9711c 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -32,27 +32,31 @@ #define HDCP2_LC_RETRY_CNT 3 static void -intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder, - struct intel_hdcp *hdcp) +intel_hdcp_adjust_hdcp_line_rekeying(struct intel_encoder *encoder, + struct intel_hdcp *hdcp, + bool enable) { struct intel_display *display = to_intel_display(encoder); + i915_reg_t *rekey_reg; + u32 rekey_bit; /* Here we assume HDMI is in TMDS mode of operation */ if (encoder->type != INTEL_OUTPUT_HDMI) return; - if (DISPLAY_VER(display) >= 30) - intel_de_rmw(display, - TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder), - 0, XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE); - 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), - 0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE); - else if (IS_DISPLAY_VERx100_STEP(display, 1400, STEP_D0, STEP_FOREVER)) - intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp->cpu_transcoder), - 0, HDCP_LINE_REKEY_DISABLE); + if (DISPLAY_VER(display) >= 30) { + *rekey_reg = TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder); + rekey_bit = XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE; + } else if (IS_DISPLAY_VERx100_STEP(display, 1401, STEP_B0, STEP_FOREVER) || + IS_DISPLAY_VERx100_STEP(display, 2000, STEP_B0, STEP_FOREVER)) { + *rekey_reg = TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder); + rekey_bit = TRANS_DDI_HDCP_LINE_REKEY_DISABLE; + } else if (IS_DISPLAY_VERx100_STEP(display, 1400, STEP_D0, STEP_FOREVER)) { + *rekey_reg = MTL_CHICKEN_TRANS(hdcp->cpu_transcoder); + rekey_bit = HDCP_LINE_REKEY_DISABLE; + } + + intel_de_rmw(display, *rekey_reg, rekey_bit, enable ? 0 : rekey_bit); } static int intel_conn_to_vcpi(struct intel_atomic_state *state, @@ -1049,6 +1053,8 @@ static int intel_hdcp1_enable(struct intel_connector *connector) return ret; } + intel_hdcp_adjust_hdcp_line_rekeying(connector->encoder, hdcp, true); + /* Incase of authentication failures, HDCP spec expects reauth. */ for (i = 0; i < tries; i++) { ret = intel_hdcp_auth(connector); @@ -2062,7 +2068,7 @@ 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); + intel_hdcp_adjust_hdcp_line_rekeying(connector->encoder, hdcp, false); ret = hdcp2_authenticate_and_encrypt(state, connector); if (ret) {
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. --v2 -Use the exising function and modify it based on a bool rather than have a different function [Matt] --v3 -No need for kzalloc [Jani] Bspec: 69964, 50493, 50054 Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- drivers/gpu/drm/i915/display/intel_hdcp.c | 36 +++++++++++++---------- 1 file changed, 21 insertions(+), 15 deletions(-)