From: Sean Paul <[email protected]>
The shim functions return error codes, but they are discarded in
intel_hdcp.c. This patch plumbs the return codes through so they are
properly handled.
Acked-by: Jani Nikula <[email protected]>
Reviewed-by: Rodrigo Vivi <[email protected]>
Signed-off-by: Sean Paul <[email protected]>
Signed-off-by: Mark Yacoub <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] #v1
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] #v2
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] #v3
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] #v4
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] #v5
Changes in v2:
-None
Changes in v3:
-None
Changes in v4:
-None
Changes in v5:
-None
Changes in v6:
-Rebased
---
.../drm/i915/display/intel_display_debugfs.c | 9 +++-
drivers/gpu/drm/i915/display/intel_hdcp.c | 51 ++++++++++---------
drivers/gpu/drm/i915/display/intel_hdcp.h | 4 +-
3 files changed, 37 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 7c7253a2541c..13a4153bb76e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -492,6 +492,7 @@ static void intel_panel_info(struct seq_file *m,
static void intel_hdcp_info(struct seq_file *m,
struct intel_connector *intel_connector)
{
+ int ret;
bool hdcp_cap, hdcp2_cap;
if (!intel_connector->hdcp.shim) {
@@ -499,8 +500,12 @@ static void intel_hdcp_info(struct seq_file *m,
goto out;
}
- hdcp_cap = intel_hdcp_capable(intel_connector);
- hdcp2_cap = intel_hdcp2_capable(intel_connector);
+ ret = intel_hdcp_capable(intel_connector, &hdcp_cap);
+ if (ret)
+ hdcp_cap = false;
+ ret = intel_hdcp2_capable(intel_connector, &hdcp2_cap);
+ if (ret)
+ hdcp2_cap = false;
if (hdcp_cap)
seq_puts(m, "HDCP1.4 ");
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 0a20bc41be55..61a862ae1f28 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -177,50 +177,49 @@ int intel_hdcp_read_valid_bksv(struct intel_digital_port *dig_port,
}
/* Is HDCP1.4 capable on Platform and Sink */
-bool intel_hdcp_capable(struct intel_connector *connector)
+int intel_hdcp_capable(struct intel_connector *connector, bool *capable)
{
struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
const struct intel_hdcp_shim *shim = connector->hdcp.shim;
- bool capable = false;
u8 bksv[5];
+ *capable = false;
+
if (!shim)
- return capable;
+ return 0;
- if (shim->hdcp_capable) {
- shim->hdcp_capable(dig_port, &capable);
- } else {
- if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
- capable = true;
- }
+ if (shim->hdcp_capable)
+ return shim->hdcp_capable(dig_port, capable);
+
+ if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
+ *capable = true;
- return capable;
+ return 0;
}
/* Is HDCP2.2 capable on Platform and Sink */
-bool intel_hdcp2_capable(struct intel_connector *connector)
+int intel_hdcp2_capable(struct intel_connector *connector, bool *capable)
{
struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
struct intel_hdcp *hdcp = &connector->hdcp;
- bool capable = false;
+
+ *capable = false;
/* I915 support for HDCP2.2 */
if (!hdcp->hdcp2_supported)
- return false;
+ return 0;
/* MEI interface is solid */
mutex_lock(&dev_priv->display.hdcp.comp_mutex);
if (!dev_priv->display.hdcp.comp_added || !dev_priv->display.hdcp.master) {
mutex_unlock(&dev_priv->display.hdcp.comp_mutex);
- return false;
+ return 0;
}
mutex_unlock(&dev_priv->display.hdcp.comp_mutex);
/* Sink's capability for HDCP2.2 */
- hdcp->shim->hdcp_2_2_capable(dig_port, &capable);
-
- return capable;
+ return hdcp->shim->hdcp_2_2_capable(dig_port, capable);
}
static bool intel_hdcp_in_use(struct drm_i915_private *dev_priv,
@@ -2355,6 +2354,7 @@ int intel_hdcp_enable(struct intel_connector *connector,
struct intel_digital_port *dig_port = intel_attached_dig_port(connector);
struct intel_hdcp *hdcp = &connector->hdcp;
unsigned long check_link_interval = DRM_HDCP_CHECK_PERIOD_MS;
+ bool capable;
int ret = -EINVAL;
if (!hdcp->shim)
@@ -2373,21 +2373,27 @@ int intel_hdcp_enable(struct intel_connector *connector,
* Considering that HDCP2.2 is more secure than HDCP1.4, If the setup
* is capable of HDCP2.2, it is preferred to use HDCP2.2.
*/
- if (intel_hdcp2_capable(connector)) {
+ ret = intel_hdcp2_capable(connector, &capable);
+ if (capable) {
ret = _intel_hdcp2_enable(connector);
- if (!ret)
+ if (!ret) {
check_link_interval = DRM_HDCP2_CHECK_PERIOD_MS;
+ goto out;
+ }
}
/*
* When HDCP2.2 fails and Content Type is not Type1, HDCP1.4 will
* be attempted.
*/
- if (ret && intel_hdcp_capable(connector) &&
- hdcp->content_type != DRM_MODE_HDCP_CONTENT_TYPE1) {
+ ret = intel_hdcp_capable(connector, &capable);
+ if (ret)
+ goto out;
+
+ if (capable && hdcp->content_type != DRM_MODE_HDCP_CONTENT_TYPE1)
ret = _intel_hdcp_enable(connector);
- }
+out:
if (!ret) {
schedule_delayed_work(&hdcp->check_work, check_link_interval);
intel_hdcp_update_value(connector,
@@ -2395,7 +2401,6 @@ int intel_hdcp_enable(struct intel_connector *connector,
true);
}
-out:
mutex_unlock(&dig_port->hdcp_mutex);
mutex_unlock(&hdcp->mutex);
return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h b/drivers/gpu/drm/i915/display/intel_hdcp.h
index 7c5fd84a7b65..f06f6e5a2b1a 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.h
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
@@ -33,8 +33,8 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state,
const struct intel_crtc_state *crtc_state,
const struct drm_connector_state *conn_state);
bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
-bool intel_hdcp_capable(struct intel_connector *connector);
-bool intel_hdcp2_capable(struct intel_connector *connector);
+int intel_hdcp_capable(struct intel_connector *connector, bool *capable);
+int intel_hdcp2_capable(struct intel_connector *connector, bool *capable);
void intel_hdcp_component_init(struct drm_i915_private *dev_priv);
void intel_hdcp_component_fini(struct drm_i915_private *dev_priv);
void intel_hdcp_cleanup(struct intel_connector *connector);
--
2.39.0.246.g2a6d74b583-goog
> Subject: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable return codes
>
> From: Sean Paul <[email protected]>
>
> The shim functions return error codes, but they are discarded in
> intel_hdcp.c. This patch plumbs the return codes through so they are
> properly handled.
>
> Acked-by: Jani Nikula <[email protected]>
> Reviewed-by: Rodrigo Vivi <[email protected]>
> Signed-off-by: Sean Paul <[email protected]>
> Signed-off-by: Mark Yacoub <[email protected]>
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-7-
> [email protected] #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-7-
> [email protected] #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-7-
> [email protected] #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-
> [email protected] #v4
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-
> [email protected] #v5
>
> Changes in v2:
> -None
> Changes in v3:
> -None
> Changes in v4:
> -None
> Changes in v5:
> -None
> Changes in v6:
> -Rebased
>
> ---
> .../drm/i915/display/intel_display_debugfs.c | 9 +++-
> drivers/gpu/drm/i915/display/intel_hdcp.c | 51 ++++++++++---------
> drivers/gpu/drm/i915/display/intel_hdcp.h | 4 +-
> 3 files changed, 37 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 7c7253a2541c..13a4153bb76e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -492,6 +492,7 @@ static void intel_panel_info(struct seq_file *m, static
> void intel_hdcp_info(struct seq_file *m,
> struct intel_connector *intel_connector) {
> + int ret;
> bool hdcp_cap, hdcp2_cap;
>
> if (!intel_connector->hdcp.shim) {
> @@ -499,8 +500,12 @@ static void intel_hdcp_info(struct seq_file *m,
> goto out;
> }
>
> - hdcp_cap = intel_hdcp_capable(intel_connector);
> - hdcp2_cap = intel_hdcp2_capable(intel_connector);
> + ret = intel_hdcp_capable(intel_connector, &hdcp_cap);
> + if (ret)
> + hdcp_cap = false;
> + ret = intel_hdcp2_capable(intel_connector, &hdcp2_cap);
> + if (ret)
> + hdcp2_cap = false;
>
This does not seem to be adding value here as this error which you referred to as being ignored
is used both in case of hdmi and dp is being to determine if hdcp_cap or hdcp2 cap is true or false
which you basically reiterate here too
check the intel_dp_hdcp2_capable and intel_hdmi_hdcp2_capable .
this change in itself can be removed.
Regards,
Suraj Kandpal
> if (hdcp_cap)
> seq_puts(m, "HDCP1.4 ");
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 0a20bc41be55..61a862ae1f28 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -177,50 +177,49 @@ int intel_hdcp_read_valid_bksv(struct
> intel_digital_port *dig_port, }
>
> /* Is HDCP1.4 capable on Platform and Sink */ -bool
> intel_hdcp_capable(struct intel_connector *connector)
> +int intel_hdcp_capable(struct intel_connector *connector, bool
> +*capable)
> {
> struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> const struct intel_hdcp_shim *shim = connector->hdcp.shim;
> - bool capable = false;
> u8 bksv[5];
>
> + *capable = false;
> +
> if (!shim)
> - return capable;
> + return 0;
>
> - if (shim->hdcp_capable) {
> - shim->hdcp_capable(dig_port, &capable);
> - } else {
> - if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> - capable = true;
> - }
> + if (shim->hdcp_capable)
> + return shim->hdcp_capable(dig_port, capable);
> +
> + if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> + *capable = true;
>
> - return capable;
> + return 0;
> }
>
> /* Is HDCP2.2 capable on Platform and Sink */ -bool
> intel_hdcp2_capable(struct intel_connector *connector)
> +int intel_hdcp2_capable(struct intel_connector *connector, bool
> +*capable)
> {
> struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> struct intel_hdcp *hdcp = &connector->hdcp;
> - bool capable = false;
> +
> + *capable = false;
>
> /* I915 support for HDCP2.2 */
> if (!hdcp->hdcp2_supported)
> - return false;
> + return 0;
>
> /* MEI interface is solid */
> mutex_lock(&dev_priv->display.hdcp.comp_mutex);
> if (!dev_priv->display.hdcp.comp_added || !dev_priv-
> >display.hdcp.master) {
> mutex_unlock(&dev_priv->display.hdcp.comp_mutex);
> - return false;
> + return 0;
> }
> mutex_unlock(&dev_priv->display.hdcp.comp_mutex);
>
> /* Sink's capability for HDCP2.2 */
> - hdcp->shim->hdcp_2_2_capable(dig_port, &capable);
> -
> - return capable;
> + return hdcp->shim->hdcp_2_2_capable(dig_port, capable);
> }
>
> static bool intel_hdcp_in_use(struct drm_i915_private *dev_priv, @@ -
> 2355,6 +2354,7 @@ int intel_hdcp_enable(struct intel_connector
> *connector,
> struct intel_digital_port *dig_port =
> intel_attached_dig_port(connector);
> struct intel_hdcp *hdcp = &connector->hdcp;
> unsigned long check_link_interval = DRM_HDCP_CHECK_PERIOD_MS;
> + bool capable;
> int ret = -EINVAL;
>
> if (!hdcp->shim)
> @@ -2373,21 +2373,27 @@ int intel_hdcp_enable(struct intel_connector
> *connector,
> * Considering that HDCP2.2 is more secure than HDCP1.4, If the
> setup
> * is capable of HDCP2.2, it is preferred to use HDCP2.2.
> */
> - if (intel_hdcp2_capable(connector)) {
> + ret = intel_hdcp2_capable(connector, &capable);
> + if (capable) {
> ret = _intel_hdcp2_enable(connector);
> - if (!ret)
> + if (!ret) {
> check_link_interval =
> DRM_HDCP2_CHECK_PERIOD_MS;
> + goto out;
> + }
> }
>
> /*
> * When HDCP2.2 fails and Content Type is not Type1, HDCP1.4 will
> * be attempted.
> */
> - if (ret && intel_hdcp_capable(connector) &&
> - hdcp->content_type != DRM_MODE_HDCP_CONTENT_TYPE1) {
> + ret = intel_hdcp_capable(connector, &capable);
> + if (ret)
> + goto out;
> +
> + if (capable && hdcp->content_type !=
> DRM_MODE_HDCP_CONTENT_TYPE1)
> ret = _intel_hdcp_enable(connector);
> - }
>
> +out:
> if (!ret) {
> schedule_delayed_work(&hdcp->check_work,
> check_link_interval);
> intel_hdcp_update_value(connector,
> @@ -2395,7 +2401,6 @@ int intel_hdcp_enable(struct intel_connector
> *connector,
> true);
> }
>
> -out:
> mutex_unlock(&dig_port->hdcp_mutex);
> mutex_unlock(&hdcp->mutex);
> return ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h
> b/drivers/gpu/drm/i915/display/intel_hdcp.h
> index 7c5fd84a7b65..f06f6e5a2b1a 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.h
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
> @@ -33,8 +33,8 @@ void intel_hdcp_update_pipe(struct intel_atomic_state
> *state,
> const struct intel_crtc_state *crtc_state,
> const struct drm_connector_state *conn_state);
> bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port
> port); -bool intel_hdcp_capable(struct intel_connector *connector); -bool
> intel_hdcp2_capable(struct intel_connector *connector);
> +int intel_hdcp_capable(struct intel_connector *connector, bool
> +*capable); int intel_hdcp2_capable(struct intel_connector *connector,
> +bool *capable);
> void intel_hdcp_component_init(struct drm_i915_private *dev_priv); void
> intel_hdcp_component_fini(struct drm_i915_private *dev_priv); void
> intel_hdcp_cleanup(struct intel_connector *connector);
> --
> 2.39.0.246.g2a6d74b583-goog
> -----Original Message-----
> From: Kandpal, Suraj
> Sent: Friday, March 10, 2023 1:55 PM
> To: Mark Yacoub <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; Souza, Jose
> <[email protected]>; [email protected];
> [email protected]; [email protected]; Vasut, Marek
> <[email protected]>; Dixit, Ashutosh <[email protected]>;
> [email protected]; [email protected]; [email protected]; Murthy,
> Arun R <[email protected]>; Lisovskiy, Stanislav
> <[email protected]>; [email protected];
> [email protected]; Nautiyal, Ankit K <[email protected]>;
> Nikula, Jani <[email protected]>; De Marchi, Lucas
> <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; Vivi, Rodrigo
> <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; Sharma, Swati2 <[email protected]>;
> Navare, Manasi D <[email protected]>; [email protected];
> Modem, Bhanuprakash <[email protected]>;
> [email protected]; [email protected]
> Subject: RE: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable return
> codes
>
> > Subject: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable return
> > codes
> >
> > From: Sean Paul <[email protected]>
> >
> > The shim functions return error codes, but they are discarded in
> > intel_hdcp.c. This patch plumbs the return codes through so they are
> > properly handled.
> >
> > Acked-by: Jani Nikula <[email protected]>
> > Reviewed-by: Rodrigo Vivi <[email protected]>
> > Signed-off-by: Sean Paul <[email protected]>
> > Signed-off-by: Mark Yacoub <[email protected]>
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-7-
> > [email protected] #v1
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-7-
> > [email protected] #v2
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-7-
> > [email protected] #v3
> > Link:
> >
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-
> > [email protected] #v4
> > Link:
> >
> https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-
> > [email protected] #v5
> >
> > Changes in v2:
> > -None
> > Changes in v3:
> > -None
> > Changes in v4:
> > -None
> > Changes in v5:
> > -None
> > Changes in v6:
> > -Rebased
> >
> > ---
> > .../drm/i915/display/intel_display_debugfs.c | 9 +++-
> > drivers/gpu/drm/i915/display/intel_hdcp.c | 51 ++++++++++---------
> > drivers/gpu/drm/i915/display/intel_hdcp.h | 4 +-
> > 3 files changed, 37 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > index 7c7253a2541c..13a4153bb76e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -492,6 +492,7 @@ static void intel_panel_info(struct seq_file *m,
> > static void intel_hdcp_info(struct seq_file *m,
> > struct intel_connector *intel_connector) {
> > + int ret;
> > bool hdcp_cap, hdcp2_cap;
> >
> > if (!intel_connector->hdcp.shim) {
> > @@ -499,8 +500,12 @@ static void intel_hdcp_info(struct seq_file *m,
> > goto out;
> > }
> >
> > - hdcp_cap = intel_hdcp_capable(intel_connector);
> > - hdcp2_cap = intel_hdcp2_capable(intel_connector);
> > + ret = intel_hdcp_capable(intel_connector, &hdcp_cap);
> > + if (ret)
> > + hdcp_cap = false;
> > + ret = intel_hdcp2_capable(intel_connector, &hdcp2_cap);
> > + if (ret)
> > + hdcp2_cap = false;
> >
>
> This does not seem to be adding value here as this error which you referred
> to as being ignored is used both in case of hdmi and dp is being to determine
> if hdcp_cap or hdcp2 cap is true or false which you basically reiterate here too
> check the intel_dp_hdcp2_capable and intel_hdmi_hdcp2_capable .
> this change in itself can be removed.
>
> Regards,
> Suraj Kandpal
>
> > if (hdcp_cap)
> > seq_puts(m, "HDCP1.4 ");
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index 0a20bc41be55..61a862ae1f28 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -177,50 +177,49 @@ int intel_hdcp_read_valid_bksv(struct
> > intel_digital_port *dig_port, }
> >
> > /* Is HDCP1.4 capable on Platform and Sink */ -bool
> > intel_hdcp_capable(struct intel_connector *connector)
> > +int intel_hdcp_capable(struct intel_connector *connector, bool
> > +*capable)
> > {
> > struct intel_digital_port *dig_port =
> > intel_attached_dig_port(connector);
> > const struct intel_hdcp_shim *shim = connector->hdcp.shim;
> > - bool capable = false;
> > u8 bksv[5];
> >
> > + *capable = false;
> > +
> > if (!shim)
> > - return capable;
> > + return 0;
> >
> > - if (shim->hdcp_capable) {
> > - shim->hdcp_capable(dig_port, &capable);
> > - } else {
> > - if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> > - capable = true;
> > - }
> > + if (shim->hdcp_capable)
> > + return shim->hdcp_capable(dig_port, capable);
> > +
> > + if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> > + *capable = true;
> >
> > - return capable;
> > + return 0;
> > }
> >
> > /* Is HDCP2.2 capable on Platform and Sink */ -bool
> > intel_hdcp2_capable(struct intel_connector *connector)
> > +int intel_hdcp2_capable(struct intel_connector *connector, bool
> > +*capable)
> > {
> > struct intel_digital_port *dig_port =
> > intel_attached_dig_port(connector);
> > struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > struct intel_hdcp *hdcp = &connector->hdcp;
> > - bool capable = false;
> > +
> > + *capable = false;
> >
> > /* I915 support for HDCP2.2 */
> > if (!hdcp->hdcp2_supported)
> > - return false;
> > + return 0;
> >
> > /* MEI interface is solid */
> > mutex_lock(&dev_priv->display.hdcp.comp_mutex);
> > if (!dev_priv->display.hdcp.comp_added || !dev_priv-
> > >display.hdcp.master) {
> > mutex_unlock(&dev_priv->display.hdcp.comp_mutex);
> > - return false;
> > + return 0;
> > }
> > mutex_unlock(&dev_priv->display.hdcp.comp_mutex);
> >
> > /* Sink's capability for HDCP2.2 */
> > - hdcp->shim->hdcp_2_2_capable(dig_port, &capable);
> > -
> > - return capable;
> > + return hdcp->shim->hdcp_2_2_capable(dig_port, capable);
> > }
> >
> > static bool intel_hdcp_in_use(struct drm_i915_private *dev_priv, @@ -
> > 2355,6 +2354,7 @@ int intel_hdcp_enable(struct intel_connector
> > *connector,
> > struct intel_digital_port *dig_port =
> > intel_attached_dig_port(connector);
> > struct intel_hdcp *hdcp = &connector->hdcp;
> > unsigned long check_link_interval =
> DRM_HDCP_CHECK_PERIOD_MS;
> > + bool capable;
> > int ret = -EINVAL;
> >
> > if (!hdcp->shim)
> > @@ -2373,21 +2373,27 @@ int intel_hdcp_enable(struct intel_connector
> > *connector,
> > * Considering that HDCP2.2 is more secure than HDCP1.4, If the
> > setup
> > * is capable of HDCP2.2, it is preferred to use HDCP2.2.
> > */
> > - if (intel_hdcp2_capable(connector)) {
> > + ret = intel_hdcp2_capable(connector, &capable);
> > + if (capable) {
> > ret = _intel_hdcp2_enable(connector);
> > - if (!ret)
> > + if (!ret) {
> > check_link_interval =
> > DRM_HDCP2_CHECK_PERIOD_MS;
> > + goto out;
> > + }
HI,
Just noticed another changed here if any error is returned with intel_hdc2_capable
You directly jump to out which will stop us from enabling hdcp 1.4 we should check
for hdcp 1.4 capability even if hdcp 2.2 capability is returned with an error one other
reason I don't think the handling of error codes are adding value here.
Regards,
Suraj Kandpal
> > }
> >
> > /*
> > * When HDCP2.2 fails and Content Type is not Type1, HDCP1.4 will
> > * be attempted.
> > */
> > - if (ret && intel_hdcp_capable(connector) &&
> > - hdcp->content_type != DRM_MODE_HDCP_CONTENT_TYPE1) {
> > + ret = intel_hdcp_capable(connector, &capable);
> > + if (ret)
> > + goto out;
> > +
> > + if (capable && hdcp->content_type !=
> > DRM_MODE_HDCP_CONTENT_TYPE1)
> > ret = _intel_hdcp_enable(connector);
> > - }
> >
> > +out:
> > if (!ret) {
> > schedule_delayed_work(&hdcp->check_work,
> > check_link_interval);
> > intel_hdcp_update_value(connector,
> > @@ -2395,7 +2401,6 @@ int intel_hdcp_enable(struct intel_connector
> > *connector,
> > true);
> > }
> >
> > -out:
> > mutex_unlock(&dig_port->hdcp_mutex);
> > mutex_unlock(&hdcp->mutex);
> > return ret;
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h
> > b/drivers/gpu/drm/i915/display/intel_hdcp.h
> > index 7c5fd84a7b65..f06f6e5a2b1a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
> > @@ -33,8 +33,8 @@ void intel_hdcp_update_pipe(struct
> > intel_atomic_state *state,
> > const struct intel_crtc_state *crtc_state,
> > const struct drm_connector_state *conn_state);
> bool
> > is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
> > -bool intel_hdcp_capable(struct intel_connector *connector); -bool
> > intel_hdcp2_capable(struct intel_connector *connector);
> > +int intel_hdcp_capable(struct intel_connector *connector, bool
> > +*capable); int intel_hdcp2_capable(struct intel_connector *connector,
> > +bool *capable);
> > void intel_hdcp_component_init(struct drm_i915_private *dev_priv);
> > void intel_hdcp_component_fini(struct drm_i915_private *dev_priv);
> > void intel_hdcp_cleanup(struct intel_connector *connector);
> > --
> > 2.39.0.246.g2a6d74b583-goog
On Thu, Mar 23, 2023 at 3:18 AM Kandpal, Suraj <[email protected]> wrote:
>
>
>
> > -----Original Message-----
> > From: Kandpal, Suraj
> > Sent: Friday, March 10, 2023 1:55 PM
> > To: Mark Yacoub <[email protected]>; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; linux-
> > [email protected]; [email protected]
> > Cc: [email protected]; [email protected]; Souza, Jose
> > <[email protected]>; [email protected];
> > [email protected]; [email protected]; Vasut, Marek
> > <[email protected]>; Dixit, Ashutosh <[email protected]>;
> > [email protected]; [email protected]; [email protected]; Murthy,
> > Arun R <[email protected]>; Lisovskiy, Stanislav
> > <[email protected]>; [email protected];
> > [email protected]; Nautiyal, Ankit K <[email protected]>;
> > Nikula, Jani <[email protected]>; De Marchi, Lucas
> > <[email protected]>; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; Vivi, Rodrigo
> > <[email protected]>; [email protected];
> > [email protected]; [email protected];
> > [email protected]; Sharma, Swati2 <[email protected]>;
> > Navare, Manasi D <[email protected]>; [email protected];
> > Modem, Bhanuprakash <[email protected]>;
> > [email protected]; [email protected]
> > Subject: RE: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable return
> > codes
> >
> > > Subject: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable return
> > > codes
> > >
> > > From: Sean Paul <[email protected]>
> > >
> > > The shim functions return error codes, but they are discarded in
> > > intel_hdcp.c. This patch plumbs the return codes through so they are
> > > properly handled.
> > >
> > > Acked-by: Jani Nikula <[email protected]>
> > > Reviewed-by: Rodrigo Vivi <[email protected]>
> > > Signed-off-by: Sean Paul <[email protected]>
> > > Signed-off-by: Mark Yacoub <[email protected]>
> > > Link:
> > > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-7-
> > > [email protected] #v1
> > > Link:
> > > https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-7-
> > > [email protected] #v2
> > > Link:
> > > https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-7-
> > > [email protected] #v3
> > > Link:
> > >
> > https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-
> > > [email protected] #v4
> > > Link:
> > >
> > https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-
> > > [email protected] #v5
> > >
> > > Changes in v2:
> > > -None
> > > Changes in v3:
> > > -None
> > > Changes in v4:
> > > -None
> > > Changes in v5:
> > > -None
> > > Changes in v6:
> > > -Rebased
> > >
> > > ---
> > > .../drm/i915/display/intel_display_debugfs.c | 9 +++-
> > > drivers/gpu/drm/i915/display/intel_hdcp.c | 51 ++++++++++---------
> > > drivers/gpu/drm/i915/display/intel_hdcp.h | 4 +-
> > > 3 files changed, 37 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > index 7c7253a2541c..13a4153bb76e 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > @@ -492,6 +492,7 @@ static void intel_panel_info(struct seq_file *m,
> > > static void intel_hdcp_info(struct seq_file *m,
> > > struct intel_connector *intel_connector) {
> > > + int ret;
> > > bool hdcp_cap, hdcp2_cap;
> > >
> > > if (!intel_connector->hdcp.shim) {
> > > @@ -499,8 +500,12 @@ static void intel_hdcp_info(struct seq_file *m,
> > > goto out;
> > > }
> > >
> > > - hdcp_cap = intel_hdcp_capable(intel_connector);
> > > - hdcp2_cap = intel_hdcp2_capable(intel_connector);
> > > + ret = intel_hdcp_capable(intel_connector, &hdcp_cap);
> > > + if (ret)
> > > + hdcp_cap = false;
> > > + ret = intel_hdcp2_capable(intel_connector, &hdcp2_cap);
> > > + if (ret)
> > > + hdcp2_cap = false;
> > >
> >
> > This does not seem to be adding value here as this error which you referred
> > to as being ignored is used both in case of hdmi and dp is being to determine
> > if hdcp_cap or hdcp2 cap is true or false which you basically reiterate here too
> > check the intel_dp_hdcp2_capable and intel_hdmi_hdcp2_capable .
> > this change in itself can be removed.
> >
> > Regards,
> > Suraj Kandpal
> >
Hey Suraj, what we're trying to do here is to have a distinction
between 2 things:
1. were we able to check of the capability or not. like did the
connection work well
2. if the check went well, what capability were were able to read
We may or may not need both info. But since we moved to common DRM, it
might be best keep the distinction and each driver can handle it as it
sees fit.
> > > if (hdcp_cap)
> > > seq_puts(m, "HDCP1.4 ");
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > index 0a20bc41be55..61a862ae1f28 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > @@ -177,50 +177,49 @@ int intel_hdcp_read_valid_bksv(struct
> > > intel_digital_port *dig_port, }
> > >
> > > /* Is HDCP1.4 capable on Platform and Sink */ -bool
> > > intel_hdcp_capable(struct intel_connector *connector)
> > > +int intel_hdcp_capable(struct intel_connector *connector, bool
> > > +*capable)
> > > {
> > > struct intel_digital_port *dig_port =
> > > intel_attached_dig_port(connector);
> > > const struct intel_hdcp_shim *shim = connector->hdcp.shim;
> > > - bool capable = false;
> > > u8 bksv[5];
> > >
> > > + *capable = false;
> > > +
> > > if (!shim)
> > > - return capable;
> > > + return 0;
> > >
> > > - if (shim->hdcp_capable) {
> > > - shim->hdcp_capable(dig_port, &capable);
> > > - } else {
> > > - if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> > > - capable = true;
> > > - }
> > > + if (shim->hdcp_capable)
> > > + return shim->hdcp_capable(dig_port, capable);
> > > +
> > > + if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> > > + *capable = true;
> > >
> > > - return capable;
> > > + return 0;
> > > }
> > >
> > > /* Is HDCP2.2 capable on Platform and Sink */ -bool
> > > intel_hdcp2_capable(struct intel_connector *connector)
> > > +int intel_hdcp2_capable(struct intel_connector *connector, bool
> > > +*capable)
> > > {
> > > struct intel_digital_port *dig_port =
> > > intel_attached_dig_port(connector);
> > > struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > > struct intel_hdcp *hdcp = &connector->hdcp;
> > > - bool capable = false;
> > > +
> > > + *capable = false;
> > >
> > > /* I915 support for HDCP2.2 */
> > > if (!hdcp->hdcp2_supported)
> > > - return false;
> > > + return 0;
> > >
> > > /* MEI interface is solid */
> > > mutex_lock(&dev_priv->display.hdcp.comp_mutex);
> > > if (!dev_priv->display.hdcp.comp_added || !dev_priv-
> > > >display.hdcp.master) {
> > > mutex_unlock(&dev_priv->display.hdcp.comp_mutex);
> > > - return false;
> > > + return 0;
> > > }
> > > mutex_unlock(&dev_priv->display.hdcp.comp_mutex);
> > >
> > > /* Sink's capability for HDCP2.2 */
> > > - hdcp->shim->hdcp_2_2_capable(dig_port, &capable);
> > > -
> > > - return capable;
> > > + return hdcp->shim->hdcp_2_2_capable(dig_port, capable);
> > > }
> > >
> > > static bool intel_hdcp_in_use(struct drm_i915_private *dev_priv, @@ -
> > > 2355,6 +2354,7 @@ int intel_hdcp_enable(struct intel_connector
> > > *connector,
> > > struct intel_digital_port *dig_port =
> > > intel_attached_dig_port(connector);
> > > struct intel_hdcp *hdcp = &connector->hdcp;
> > > unsigned long check_link_interval =
> > DRM_HDCP_CHECK_PERIOD_MS;
> > > + bool capable;
> > > int ret = -EINVAL;
> > >
> > > if (!hdcp->shim)
> > > @@ -2373,21 +2373,27 @@ int intel_hdcp_enable(struct intel_connector
> > > *connector,
> > > * Considering that HDCP2.2 is more secure than HDCP1.4, If the
> > > setup
> > > * is capable of HDCP2.2, it is preferred to use HDCP2.2.
> > > */
> > > - if (intel_hdcp2_capable(connector)) {
> > > + ret = intel_hdcp2_capable(connector, &capable);
> > > + if (capable) {
> > > ret = _intel_hdcp2_enable(connector);
> > > - if (!ret)
> > > + if (!ret) {
> > > check_link_interval =
> > > DRM_HDCP2_CHECK_PERIOD_MS;
> > > + goto out;
> > > + }
>
> HI,
> Just noticed another changed here if any error is returned with intel_hdc2_capable
> You directly jump to out which will stop us from enabling hdcp 1.4 we should check
> for hdcp 1.4 capability even if hdcp 2.2 capability is returned with an error one other
> reason I don't think the handling of error codes are adding value here.
>
> Regards,
> Suraj Kandpal
Hey Suraj - the goto happens if we know that the device is hdcp2
capable. If it's capable, we do enable it. If we have no error
returned, that's when we skip hdcp1.4
otherwise, if it's not capable, or the enable returned with an error
code, we don't goto out but move on to try on hdcp 1.4
Thanks!
> > > }
> > >
> > > /*
> > > * When HDCP2.2 fails and Content Type is not Type1, HDCP1.4 will
> > > * be attempted.
> > > */
> > > - if (ret && intel_hdcp_capable(connector) &&
> > > - hdcp->content_type != DRM_MODE_HDCP_CONTENT_TYPE1) {
> > > + ret = intel_hdcp_capable(connector, &capable);
> > > + if (ret)
> > > + goto out;
> > > +
> > > + if (capable && hdcp->content_type !=
> > > DRM_MODE_HDCP_CONTENT_TYPE1)
> > > ret = _intel_hdcp_enable(connector);
> > > - }
> > >
> > > +out:
> > > if (!ret) {
> > > schedule_delayed_work(&hdcp->check_work,
> > > check_link_interval);
> > > intel_hdcp_update_value(connector,
> > > @@ -2395,7 +2401,6 @@ int intel_hdcp_enable(struct intel_connector
> > > *connector,
> > > true);
> > > }
> > >
> > > -out:
> > > mutex_unlock(&dig_port->hdcp_mutex);
> > > mutex_unlock(&hdcp->mutex);
> > > return ret;
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h
> > > b/drivers/gpu/drm/i915/display/intel_hdcp.h
> > > index 7c5fd84a7b65..f06f6e5a2b1a 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
> > > @@ -33,8 +33,8 @@ void intel_hdcp_update_pipe(struct
> > > intel_atomic_state *state,
> > > const struct intel_crtc_state *crtc_state,
> > > const struct drm_connector_state *conn_state);
> > bool
> > > is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
> > > -bool intel_hdcp_capable(struct intel_connector *connector); -bool
> > > intel_hdcp2_capable(struct intel_connector *connector);
> > > +int intel_hdcp_capable(struct intel_connector *connector, bool
> > > +*capable); int intel_hdcp2_capable(struct intel_connector *connector,
> > > +bool *capable);
> > > void intel_hdcp_component_init(struct drm_i915_private *dev_priv);
> > > void intel_hdcp_component_fini(struct drm_i915_private *dev_priv);
> > > void intel_hdcp_cleanup(struct intel_connector *connector);
> > > --
> > > 2.39.0.246.g2a6d74b583-goog
>
> -----Original Message-----
> From: Mark Yacoub <[email protected]>
> Sent: Saturday, March 25, 2023 12:57 AM
> To: Kandpal, Suraj <[email protected]>
> Cc: [email protected]; [email protected]; dri-
> [email protected]; [email protected];
> [email protected]; [email protected]; intel-
> [email protected]; [email protected];
> [email protected]; Souza, Jose <[email protected]>;
> [email protected]; [email protected];
> [email protected]; Vasut, Marek <[email protected]>; Dixit, Ashutosh
> <[email protected]>; [email protected]; [email protected];
> [email protected]; Murthy, Arun R <[email protected]>; Lisovskiy,
> Stanislav <[email protected]>; [email protected];
> [email protected]; Nautiyal, Ankit K <[email protected]>;
> Nikula, Jani <[email protected]>; De Marchi, Lucas
> <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; Vivi, Rodrigo
> <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; Sharma, Swati2 <[email protected]>;
> Navare, Manasi D <[email protected]>; [email protected];
> Modem, Bhanuprakash <[email protected]>;
> [email protected]; [email protected]
> Subject: Re: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable return
> codes
>
> On Thu, Mar 23, 2023 at 3:18 AM Kandpal, Suraj <[email protected]>
> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Kandpal, Suraj
> > > Sent: Friday, March 10, 2023 1:55 PM
> > > To: Mark Yacoub <[email protected]>;
> [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected]; linux-
> > > [email protected]; [email protected]
> > > Cc: [email protected]; [email protected]; Souza,
> > > Jose <[email protected]>; [email protected];
> > > [email protected]; [email protected]; Vasut, Marek
> > > <[email protected]>; Dixit, Ashutosh <[email protected]>;
> > > [email protected]; [email protected]; [email protected];
> > > Murthy, Arun R <[email protected]>; Lisovskiy, Stanislav
> > > <[email protected]>; [email protected];
> > > [email protected]; Nautiyal, Ankit K
> > > <[email protected]>; Nikula, Jani <[email protected]>;
> > > De Marchi, Lucas <[email protected]>;
> > > [email protected]; [email protected];
> [email protected];
> > > [email protected]; [email protected]; Vivi, Rodrigo
> > > <[email protected]>; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; Sharma, Swati2 <[email protected]>;
> > > Navare, Manasi D <[email protected]>;
> [email protected];
> > > Modem, Bhanuprakash <[email protected]>;
> > > [email protected]; [email protected]
> > > Subject: RE: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable
> > > return codes
> > >
> > > > Subject: [PATCH v6 06/10] drm/i915/hdcp: Retain hdcp_capable
> > > > return codes
> > > >
> > > > From: Sean Paul <[email protected]>
> > > >
> > > > The shim functions return error codes, but they are discarded in
> > > > intel_hdcp.c. This patch plumbs the return codes through so they
> > > > are properly handled.
> > > >
> > > > Acked-by: Jani Nikula <[email protected]>
> > > > Reviewed-by: Rodrigo Vivi <[email protected]>
> > > > Signed-off-by: Sean Paul <[email protected]>
> > > > Signed-off-by: Mark Yacoub <[email protected]>
> > > > Link:
> > > >
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456
> > > > -7-
> > > > [email protected] #v1
> > > > Link:
> > > >
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-
> > > > 7-
> > > > [email protected] #v2
> > > > Link:
> > > >
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916
> > > > -7-
> > > > [email protected] #v3
> > > > Link:
> > > >
> > >
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845
> > > -
> > > > [email protected] #v4
> > > > Link:
> > > >
> > >
> https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308
> > > -
> > > > [email protected] #v5
> > > >
> > > > Changes in v2:
> > > > -None
> > > > Changes in v3:
> > > > -None
> > > > Changes in v4:
> > > > -None
> > > > Changes in v5:
> > > > -None
> > > > Changes in v6:
> > > > -Rebased
> > > >
> > > > ---
> > > > .../drm/i915/display/intel_display_debugfs.c | 9 +++-
> > > > drivers/gpu/drm/i915/display/intel_hdcp.c | 51 ++++++++++---------
> > > > drivers/gpu/drm/i915/display/intel_hdcp.h | 4 +-
> > > > 3 files changed, 37 insertions(+), 27 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > > index 7c7253a2541c..13a4153bb76e 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > > @@ -492,6 +492,7 @@ static void intel_panel_info(struct seq_file
> > > > *m, static void intel_hdcp_info(struct seq_file *m,
> > > > struct intel_connector *intel_connector)
> > > > {
> > > > + int ret;
> > > > bool hdcp_cap, hdcp2_cap;
> > > >
> > > > if (!intel_connector->hdcp.shim) { @@ -499,8 +500,12 @@ static
> > > > void intel_hdcp_info(struct seq_file *m,
> > > > goto out;
> > > > }
> > > >
> > > > - hdcp_cap = intel_hdcp_capable(intel_connector);
> > > > - hdcp2_cap = intel_hdcp2_capable(intel_connector);
> > > > + ret = intel_hdcp_capable(intel_connector, &hdcp_cap);
> > > > + if (ret)
> > > > + hdcp_cap = false;
> > > > + ret = intel_hdcp2_capable(intel_connector, &hdcp2_cap);
> > > > + if (ret)
> > > > + hdcp2_cap = false;
> > > >
> > >
> > > This does not seem to be adding value here as this error which you
> > > referred to as being ignored is used both in case of hdmi and dp is
> > > being to determine if hdcp_cap or hdcp2 cap is true or false which
> > > you basically reiterate here too check the intel_dp_hdcp2_capable and
> intel_hdmi_hdcp2_capable .
> > > this change in itself can be removed.
> > >
> > > Regards,
> > > Suraj Kandpal
> > >
> Hey Suraj, what we're trying to do here is to have a distinction between 2
> things:
> 1. were we able to check of the capability or not. like did the connection work
> well 2. if the check went well, what capability were were able to read We
> may or may not need both info. But since we moved to common DRM, it
> might be best keep the distinction and each driver can handle it as it sees fit.
Hi Mark,
Hmm I think it make sense to allow each driver to decide how to handle it then.
> > > > if (hdcp_cap)
> > > > seq_puts(m, "HDCP1.4 "); diff --git
> > > > a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > index 0a20bc41be55..61a862ae1f28 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > > > @@ -177,50 +177,49 @@ int intel_hdcp_read_valid_bksv(struct
> > > > intel_digital_port *dig_port, }
> > > >
> > > > /* Is HDCP1.4 capable on Platform and Sink */ -bool
> > > > intel_hdcp_capable(struct intel_connector *connector)
> > > > +int intel_hdcp_capable(struct intel_connector *connector, bool
> > > > +*capable)
> > > > {
> > > > struct intel_digital_port *dig_port =
> > > > intel_attached_dig_port(connector);
> > > > const struct intel_hdcp_shim *shim = connector->hdcp.shim;
> > > > - bool capable = false;
> > > > u8 bksv[5];
> > > >
> > > > + *capable = false;
> > > > +
> > > > if (!shim)
> > > > - return capable;
> > > > + return 0;
> > > >
> > > > - if (shim->hdcp_capable) {
> > > > - shim->hdcp_capable(dig_port, &capable);
> > > > - } else {
> > > > - if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> > > > - capable = true;
> > > > - }
> > > > + if (shim->hdcp_capable)
> > > > + return shim->hdcp_capable(dig_port, capable);
> > > > +
> > > > + if (!intel_hdcp_read_valid_bksv(dig_port, shim, bksv))
> > > > + *capable = true;
> > > >
> > > > - return capable;
> > > > + return 0;
> > > > }
> > > >
> > > > /* Is HDCP2.2 capable on Platform and Sink */ -bool
> > > > intel_hdcp2_capable(struct intel_connector *connector)
> > > > +int intel_hdcp2_capable(struct intel_connector *connector, bool
> > > > +*capable)
> > > > {
> > > > struct intel_digital_port *dig_port =
> > > > intel_attached_dig_port(connector);
> > > > struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > > > struct intel_hdcp *hdcp = &connector->hdcp;
> > > > - bool capable = false;
> > > > +
> > > > + *capable = false;
> > > >
> > > > /* I915 support for HDCP2.2 */
> > > > if (!hdcp->hdcp2_supported)
> > > > - return false;
> > > > + return 0;
> > > >
> > > > /* MEI interface is solid */
> > > > mutex_lock(&dev_priv->display.hdcp.comp_mutex);
> > > > if (!dev_priv->display.hdcp.comp_added || !dev_priv-
> > > > >display.hdcp.master) {
> > > > mutex_unlock(&dev_priv->display.hdcp.comp_mutex);
> > > > - return false;
> > > > + return 0;
> > > > }
> > > > mutex_unlock(&dev_priv->display.hdcp.comp_mutex);
> > > >
> > > > /* Sink's capability for HDCP2.2 */
> > > > - hdcp->shim->hdcp_2_2_capable(dig_port, &capable);
> > > > -
> > > > - return capable;
> > > > + return hdcp->shim->hdcp_2_2_capable(dig_port, capable);
> > > > }
> > > >
> > > > static bool intel_hdcp_in_use(struct drm_i915_private *dev_priv,
> > > > @@ -
> > > > 2355,6 +2354,7 @@ int intel_hdcp_enable(struct intel_connector
> > > > *connector,
> > > > struct intel_digital_port *dig_port =
> > > > intel_attached_dig_port(connector);
> > > > struct intel_hdcp *hdcp = &connector->hdcp;
> > > > unsigned long check_link_interval =
> > > DRM_HDCP_CHECK_PERIOD_MS;
> > > > + bool capable;
> > > > int ret = -EINVAL;
> > > >
> > > > if (!hdcp->shim)
> > > > @@ -2373,21 +2373,27 @@ int intel_hdcp_enable(struct
> > > > intel_connector *connector,
> > > > * Considering that HDCP2.2 is more secure than HDCP1.4, If
> > > > the setup
> > > > * is capable of HDCP2.2, it is preferred to use HDCP2.2.
> > > > */
> > > > - if (intel_hdcp2_capable(connector)) {
> > > > + ret = intel_hdcp2_capable(connector, &capable);
> > > > + if (capable) {
> > > > ret = _intel_hdcp2_enable(connector);
> > > > - if (!ret)
> > > > + if (!ret) {
> > > > check_link_interval =
> > > > DRM_HDCP2_CHECK_PERIOD_MS;
> > > > + goto out;
> > > > + }
> >
> > HI,
> > Just noticed another changed here if any error is returned with
> > intel_hdc2_capable You directly jump to out which will stop us from
> > enabling hdcp 1.4 we should check for hdcp 1.4 capability even if hdcp
> > 2.2 capability is returned with an error one other reason I don't think the
> handling of error codes are adding value here.
> >
> > Regards,
> > Suraj Kandpal
> Hey Suraj - the goto happens if we know that the device is hdcp2 capable. If
> it's capable, we do enable it. If we have no error returned, that's when we
> skip hdcp1.4 otherwise, if it's not capable, or the enable returned with an
> error code, we don't goto out but move on to try on hdcp 1.4 Thanks!
This logic will not work as hdcp2 capability is determined by reading RxCaps register
and hdcp1.x capability is determined by looking at the devices bksv which means
there is a possibility hdcp2 may throw an error reading the rxcaps register but not issue an
error while reading the bksv and since hdcp2 devices also supports hdcp 1.x in this case we
should try enable hdcp1.4 if error is issued in at the time of hdcp2 but this code makes that
impossible.
Regards,
Suraj Kandpal
> > > > }
> > > >
> > > > /*
> > > > * When HDCP2.2 fails and Content Type is not Type1, HDCP1.4 will
> > > > * be attempted.
> > > > */
> > > > - if (ret && intel_hdcp_capable(connector) &&
> > > > - hdcp->content_type != DRM_MODE_HDCP_CONTENT_TYPE1) {
> > > > + ret = intel_hdcp_capable(connector, &capable);
> > > > + if (ret)
> > > > + goto out;
> > > > +
> > > > + if (capable && hdcp->content_type !=
> > > > DRM_MODE_HDCP_CONTENT_TYPE1)
> > > > ret = _intel_hdcp_enable(connector);
> > > > - }
> > > >
> > > > +out:
> > > > if (!ret) {
> > > > schedule_delayed_work(&hdcp->check_work,
> > > > check_link_interval);
> > > > intel_hdcp_update_value(connector,
> > > > @@ -2395,7 +2401,6 @@ int intel_hdcp_enable(struct intel_connector
> > > > *connector,
> > > > true);
> > > > }
> > > >
> > > > -out:
> > > > mutex_unlock(&dig_port->hdcp_mutex);
> > > > mutex_unlock(&hdcp->mutex);
> > > > return ret;
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h
> > > > b/drivers/gpu/drm/i915/display/intel_hdcp.h
> > > > index 7c5fd84a7b65..f06f6e5a2b1a 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
> > > > @@ -33,8 +33,8 @@ void intel_hdcp_update_pipe(struct
> > > > intel_atomic_state *state,
> > > > const struct intel_crtc_state *crtc_state,
> > > > const struct drm_connector_state
> > > > *conn_state);
> > > bool
> > > > is_hdcp_supported(struct drm_i915_private *dev_priv, enum port
> > > > port); -bool intel_hdcp_capable(struct intel_connector
> > > > *connector); -bool intel_hdcp2_capable(struct intel_connector
> > > > *connector);
> > > > +int intel_hdcp_capable(struct intel_connector *connector, bool
> > > > +*capable); int intel_hdcp2_capable(struct intel_connector
> > > > +*connector, bool *capable);
> > > > void intel_hdcp_component_init(struct drm_i915_private
> > > > *dev_priv); void intel_hdcp_component_fini(struct drm_i915_private
> > > > *dev_priv); void intel_hdcp_cleanup(struct intel_connector
> > > > *connector);
> > > > --
> > > > 2.39.0.246.g2a6d74b583-goog
> >