Re: [PATCH] drm/xe/hdcp: Add check to remove hdcp2 compatibility

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 10/22/2024 12:29 AM, Suraj Kandpal wrote:
> Add check to remove HDCP2 compatibility from BMG as it does not
> have GSC which ends up causing warning when we try to get reference
> of GSC FW.
>
> Fixes: 89d030804831 ("drm/xe/hdcp: Fix condition for hdcp gsc cs requirement")
> Fixes: 883631771038 ("drm/i915/mtl: Add HDCP GSC interface")
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++-
>   drivers/gpu/drm/xe/display/xe_hdcp_gsc.c      | 4 +++-
>   2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> index 55965844d829..2c1d0ee8cec2 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> @@ -21,7 +21,8 @@ struct intel_hdcp_gsc_message {
>   
>   bool intel_hdcp_gsc_cs_required(struct intel_display *display)
>   {
> -	return DISPLAY_VER(display) >= 14;
> +	return DISPLAY_VER(display) >= 14 &&
> +		DISPLAY_VER_FULL(display) != IP_VER(14, 1);
>   }
>   
>   bool intel_hdcp_gsc_check_status(struct intel_display *display)
> diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
> index 231677129a35..efa3441c249c 100644
> --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
> +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
> @@ -8,6 +8,7 @@
>   #include <linux/delay.h>
>   
>   #include "abi/gsc_command_header_abi.h"
> +#include "i915_drv.h"
>   #include "intel_hdcp_gsc.h"
>   #include "intel_hdcp_gsc_message.h"
>   #include "xe_bo.h"
> @@ -32,7 +33,8 @@ struct intel_hdcp_gsc_message {
>   
>   bool intel_hdcp_gsc_cs_required(struct intel_display *display)
>   {
> -	return DISPLAY_VER(display) >= 14;
> +	return DISPLAY_VER(display) >= 14 &&
> +		DISPLAY_VER_FULL(display) != IP_VER(14, 1);

I don't think this is the correct check or the correct location. BMG 
does require the GSC for HDCP, so intel_hdcp_gsc_cs_required() should 
still return true; it's just that we've decided not to support GSC FW 
loading on the platform, so we can't support HDCP2.x. Also note that the 
this might change and/or it might apply to other platform in the future, 
so any check needs to be done based on GSC support and not 
platform/display ID.

IMO when intel_hdcp_gsc_cs_required() returns true, the caller should 
check if the GSC FW is defined (or if the GSCCS is available) and if it 
is not return that hdcp2 is not supported due to unmet prerequsites and 
fallback to 1.4 without printing any errors.

Daniele

>   }
>   
>   bool intel_hdcp_gsc_check_status(struct intel_display *display)