[Intel-gfx] [PATCH v5 1/2] drm/i915/hdcp: Move checks for gsc health status

[Jani Nikula]

On Mon, 09 Oct 2023, Suraj Kandpal <suraj.kandpal at intel.com> wrote:
> Move checks for gsc components required for HDCP 2.2
> to work into intel_hdcp_gsc.c. This will also help
> with XE refactor on HDCP's side.
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> Reviewed-by: Uma Shankar <uma.shankar at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdcp.c     |  8 +-------
>  drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 14 ++++++++++++++
>  drivers/gpu/drm/i915/display/intel_hdcp_gsc.h |  1 +
>  3 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 8cca4793cf92..c89da3568ebd 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -173,14 +173,8 @@ bool intel_hdcp2_capable(struct intel_connector *connector)
>  
>  	/* If MTL+ make sure gsc is loaded and proxy is setup */
>  	if (intel_hdcp_gsc_cs_required(i915)) {
> -		struct intel_gt *gt = i915->media_gt;
> -		struct intel_gsc_uc *gsc = gt ? &gt->uc.gsc : NULL;
> -
> -		if (!gsc || !intel_uc_fw_is_running(&gsc->fw)) {
> -			drm_dbg_kms(&i915->drm,
> -				    "GSC components required for HDCP2.2 are not ready\n");
> +		if (!intel_hdcp_gsc_check_status(i915))

Naming nitpick, though not intended to block the patch:

I always stop at functions named "check" that return a value. What does
the return value mean? The above code reads, "if not check status".

So I usually try to name functions so that it's obvious on both sides
what it means, when you read it aloud. For example, "if not status ok".

Just something to consider for the future.


BR,
Jani.



>  			return false;
> -		}
>  	}
>  
>  	/* MEI/GSC interface is solid depending on which is used */
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> index d753db3eef15..d355d610bc9f 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> @@ -17,6 +17,20 @@ bool intel_hdcp_gsc_cs_required(struct drm_i915_private *i915)
>  	return DISPLAY_VER(i915) >= 14;
>  }
>  
> +bool intel_hdcp_gsc_check_status(struct drm_i915_private *i915)
> +{
> +	struct intel_gt *gt = i915->media_gt;
> +	struct intel_gsc_uc *gsc = gt ? &gt->uc.gsc : NULL;
> +
> +	if (!gsc || !intel_uc_fw_is_running(&gsc->fw)) {
> +		drm_dbg_kms(&i915->drm,
> +			    "GSC components required for HDCP2.2 are not ready\n");
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  static int
>  gsc_hdcp_initiate_session(struct device *dev, struct hdcp_port_data *data,
>  			  struct hdcp2_ake_init *ake_data)
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h
> index cbf96551e534..eba2057c5a9e 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h
> @@ -23,5 +23,6 @@ ssize_t intel_hdcp_gsc_msg_send(struct drm_i915_private *i915, u8 *msg_in,
>  				size_t msg_out_len);
>  int intel_hdcp_gsc_init(struct drm_i915_private *i915);
>  void intel_hdcp_gsc_fini(struct drm_i915_private *i915);
> +bool intel_hdcp_gsc_check_status(struct drm_i915_private *i915);
>  
>  #endif /* __INTEL_HDCP_GCS_H__ */

-- 
Jani Nikula, Intel