| Message ID | 20241029085304.219168-1-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/hdcp: Add the loop only for DP encoders | expand |
> -----Original Message----- > From: Kandpal, Suraj <suraj.kandpal@intel.com> > Sent: Tuesday, October 29, 2024 2:23 PM > To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; Kandpal, > Suraj <suraj.kandpal@intel.com> > Subject: [PATCH] drm/i915/hdcp: Add the loop only for DP encoders > > Add the loop on the first read and write only for DP/DPMST encoders as this > issue is only observed here no need to burden other encoders with extra > retries. It would make the motivation of the patch clearer if we describe "this issue" here. > > Fixes: 9d5a05f86d2f ("drm/i915/hdcp: Retry first read and writes to > downstream") > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_hdcp.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > b/drivers/gpu/drm/i915/display/intel_hdcp.c > index ed6aa87403e2..fcfb5a9f303a 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -1503,6 +1503,8 @@ static int hdcp2_deauthenticate_port(struct > intel_connector *connector) static int > hdcp2_authentication_key_exchange(struct intel_connector *connector) { > struct intel_display *display = to_intel_display(connector); > + struct intel_digital_port *dig_port = > + intel_attached_dig_port(connector); > struct intel_hdcp *hdcp = &connector->hdcp; > union { > struct hdcp2_ake_init ake_init; > @@ -1513,12 +1515,18 @@ static int > hdcp2_authentication_key_exchange(struct intel_connector *connector) > } msgs; > const struct intel_hdcp_shim *shim = hdcp->shim; > size_t size; > - int ret, i; > + int ret, i, max_retries; > > /* Init for seq_num */ > hdcp->seq_num_v = 0; > hdcp->seq_num_m = 0; > > + if (intel_encoder_is_dp(&dig_port->base) || > + intel_encoder_is_mst(&dig_port->base)) > + max_retries = 10; > + else > + max_retries = 1; > + > ret = hdcp2_prepare_ake_init(connector, &msgs.ake_init); > if (ret < 0) > return ret; > @@ -1536,7 +1544,7 @@ static int > hdcp2_authentication_key_exchange(struct intel_connector *connector) > * authentication. The values of 10 and delay of 50ms was decided > based > * on multiple trial and errors. > */ Perhaps this comment can use an update too. Functionally LGTM. Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> > - for (i = 0; i < 10; i++) { > + for (i = 0; i < max_retries; i++) { > if (!intel_hdcp2_get_capability(connector)) { > msleep(50); > continue; > -- > 2.34.1
On Tue, 29 Oct 2024, "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com> wrote: >> -----Original Message----- >> From: Kandpal, Suraj <suraj.kandpal@intel.com> >> Sent: Tuesday, October 29, 2024 2:23 PM >> To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org >> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com>; Kandpal, >> Suraj <suraj.kandpal@intel.com> >> Subject: [PATCH] drm/i915/hdcp: Add the loop only for DP encoders >> >> Add the loop on the first read and write only for DP/DPMST encoders as this >> issue is only observed here no need to burden other encoders with extra >> retries. > > It would make the motivation of the patch clearer if we describe "this issue" here. Ditto for "the loop". > >> >> Fixes: 9d5a05f86d2f ("drm/i915/hdcp: Retry first read and writes to >> downstream") >> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_hdcp.c | 12 ++++++++++-- >> 1 file changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c >> b/drivers/gpu/drm/i915/display/intel_hdcp.c >> index ed6aa87403e2..fcfb5a9f303a 100644 >> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c >> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c >> @@ -1503,6 +1503,8 @@ static int hdcp2_deauthenticate_port(struct >> intel_connector *connector) static int >> hdcp2_authentication_key_exchange(struct intel_connector *connector) { >> struct intel_display *display = to_intel_display(connector); >> + struct intel_digital_port *dig_port = >> + intel_attached_dig_port(connector); >> struct intel_hdcp *hdcp = &connector->hdcp; >> union { >> struct hdcp2_ake_init ake_init; >> @@ -1513,12 +1515,18 @@ static int >> hdcp2_authentication_key_exchange(struct intel_connector *connector) >> } msgs; >> const struct intel_hdcp_shim *shim = hdcp->shim; >> size_t size; >> - int ret, i; >> + int ret, i, max_retries; >> >> /* Init for seq_num */ >> hdcp->seq_num_v = 0; >> hdcp->seq_num_m = 0; >> >> + if (intel_encoder_is_dp(&dig_port->base) || >> + intel_encoder_is_mst(&dig_port->base)) >> + max_retries = 10; >> + else >> + max_retries = 1; >> + >> ret = hdcp2_prepare_ake_init(connector, &msgs.ake_init); >> if (ret < 0) >> return ret; >> @@ -1536,7 +1544,7 @@ static int >> hdcp2_authentication_key_exchange(struct intel_connector *connector) >> * authentication. The values of 10 and delay of 50ms was decided >> based >> * on multiple trial and errors. >> */ > > Perhaps this comment can use an update too. > > Functionally LGTM. > > Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> > >> - for (i = 0; i < 10; i++) { >> + for (i = 0; i < max_retries; i++) { >> if (!intel_hdcp2_get_capability(connector)) { >> msleep(50); >> continue; >> -- >> 2.34.1 >
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index ed6aa87403e2..fcfb5a9f303a 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -1503,6 +1503,8 @@ static int hdcp2_deauthenticate_port(struct intel_connector *connector) static int hdcp2_authentication_key_exchange(struct intel_connector *connector) { struct intel_display *display = to_intel_display(connector); + struct intel_digital_port *dig_port = + intel_attached_dig_port(connector); struct intel_hdcp *hdcp = &connector->hdcp; union { struct hdcp2_ake_init ake_init; @@ -1513,12 +1515,18 @@ static int hdcp2_authentication_key_exchange(struct intel_connector *connector) } msgs; const struct intel_hdcp_shim *shim = hdcp->shim; size_t size; - int ret, i; + int ret, i, max_retries; /* Init for seq_num */ hdcp->seq_num_v = 0; hdcp->seq_num_m = 0; + if (intel_encoder_is_dp(&dig_port->base) || + intel_encoder_is_mst(&dig_port->base)) + max_retries = 10; + else + max_retries = 1; + ret = hdcp2_prepare_ake_init(connector, &msgs.ake_init); if (ret < 0) return ret; @@ -1536,7 +1544,7 @@ static int hdcp2_authentication_key_exchange(struct intel_connector *connector) * authentication. The values of 10 and delay of 50ms was decided based * on multiple trial and errors. */ - for (i = 0; i < 10; i++) { + for (i = 0; i < max_retries; i++) { if (!intel_hdcp2_get_capability(connector)) { msleep(50); continue;
Add the loop on the first read and write only for DP/DPMST encoders as this issue is only observed here no need to burden other encoders with extra retries. Fixes: 9d5a05f86d2f ("drm/i915/hdcp: Retry first read and writes to downstream") Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- drivers/gpu/drm/i915/display/intel_hdcp.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)