Re: [PATCH v3 2/4] drm/i915: Detect the hdcp capability of sink and src

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Oct 22, 2018 at 07:46:03PM +0530, Ramalingam C wrote:
> Implements a function to detect the sink and src's hdcp1.4
> capabilities.
> 
> v2:
>   const qualifier is maintained at init.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_hdcp.c | 33 +++++++++++++++++++++++++++------
>  2 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b2ca9f278b36..b97d32bb7ff4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1956,6 +1956,7 @@ int intel_hdcp_enable(struct intel_connector *connector);
>  int intel_hdcp_disable(struct intel_connector *connector);
>  int intel_hdcp_check_link(struct intel_connector *connector);
>  bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
> +bool intel_hdcp_capable(struct intel_connector *connector);
>  
>  /* intel_psr.c */
>  #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> index 20908ff018e6..deca5581cde3 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -17,6 +17,12 @@
>  #define KEY_LOAD_TRIES	5
>  
>  static
> +struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector)

Hm, maybe we should stuff this one intel intel_connector.h perhaps? Future
cleanup patch ofc.

> +{
> +	return enc_to_dig_port(&intel_attached_encoder(&connector->base)->base);
> +}
> +
> +static
>  bool intel_hdcp_is_ksv_valid(u8 *ksv)
>  {
>  	int i, ones = 0;
> @@ -51,6 +57,27 @@ int intel_hdcp_read_valid_bksv(struct intel_digital_port *intel_dig_port,
>  	return 0;
>  }
>  
> +/* Is HDCP1.4 capable on Platform and Sink */
> +bool intel_hdcp_capable(struct intel_connector *connector)

Adding a function without caller is kinda confusing. Two possible fixes:

- Use this also in intel_hdcp_auth(), which is what I recommend, since
  cleaner imo. Probably best if we just squash this in with patch 1, and
  switch the parameter to intel_dig_port. And we'd need to move
  conn_to_dig_port() into intel_connector.h (so that patch 4 can use it).

- Or squash it in with patch 4, which actually uses it.

Either works fine with me.
-Daniel

> +{
> +	struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> +	const struct intel_hdcp_shim *shim = connector->hdcp_shim;
> +	bool capable = false;
> +	u8 bksv[5];
> +
> +	if (!shim)
> +		return capable;
> +
> +	if (shim->hdcp_capable) {
> +		shim->hdcp_capable(intel_dig_port, &capable);
> +	} else {
> +		if (!intel_hdcp_read_valid_bksv(intel_dig_port, shim, bksv))
> +			capable = true;
> +	}
> +
> +	return capable;
> +}
> +
>  static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
>  				    const struct intel_hdcp_shim *shim)
>  {
> @@ -632,12 +659,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port,
>  	return 0;
>  }
>  
> -static
> -struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector)
> -{
> -	return enc_to_dig_port(&intel_attached_encoder(&connector->base)->base);
> -}
> -
>  static int _intel_hdcp_disable(struct intel_connector *connector)
>  {
>  	struct drm_i915_private *dev_priv = connector->base.dev->dev_private;
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx