diff mbox series

[v2,1/9] drm/i915/pxp: Add MTL PXP GSC-CS back-end skeleton

Message ID 20230111214226.907536-2-alan.previn.teres.alexis@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/pxp: Add MTL PXP Support | expand

Commit Message

Teres Alexis, Alan Previn Jan. 11, 2023, 9:42 p.m. UTC 
Add MTL PXP GSC-CS back-end stub functions hook them
up from PXP front-end and PXP session management functions.

Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
 drivers/gpu/drm/i915/Makefile                |  1 +
 drivers/gpu/drm/i915/pxp/intel_pxp.c         | 19 +++++++++++--
 drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c   | 23 +++++++++++++++
 drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h   | 30 ++++++++++++++++++++
 drivers/gpu/drm/i915/pxp/intel_pxp_session.c |  6 +++-
 drivers/gpu/drm/i915/pxp/intel_pxp_types.h   |  6 ++++
 6 files changed, 81 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
 create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h

Comments

Daniele Ceraolo Spurio Jan. 18, 2023, 5:51 p.m. UTC | #1 
On 1/11/2023 1:42 PM, Alan Previn wrote:
> Add MTL PXP GSC-CS back-end stub functions hook them
> up from PXP front-end and PXP session management functions.
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
>   drivers/gpu/drm/i915/Makefile                |  1 +
>   drivers/gpu/drm/i915/pxp/intel_pxp.c         | 19 +++++++++++--
>   drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c   | 23 +++++++++++++++
>   drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h   | 30 ++++++++++++++++++++
>   drivers/gpu/drm/i915/pxp/intel_pxp_session.c |  6 +++-
>   drivers/gpu/drm/i915/pxp/intel_pxp_types.h   |  6 ++++
>   6 files changed, 81 insertions(+), 4 deletions(-)

This patch is relatively small and it doesn't look like we have any 
benefits by introducing the stubs early (please correct me if I'm 
wrong), so I suggest to just squash it with the first patch that adds 
code to those functions.

>   create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
>   create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index f47f00b162a4..eae4325310e8 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -330,6 +330,7 @@ i915-y += \
>   i915-$(CONFIG_DRM_I915_PXP) += \
>   	pxp/intel_pxp_cmd.o \
>   	pxp/intel_pxp_debugfs.o \
> +	pxp/intel_pxp_gsccs.o \
>   	pxp/intel_pxp_irq.o \
>   	pxp/intel_pxp_pm.o \
>   	pxp/intel_pxp_session.o
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> index cfc9af8b3d21..be52bf92e847 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> @@ -12,6 +12,7 @@
>   #include "i915_drv.h"
>   
>   #include "intel_pxp.h"
> +#include "intel_pxp_gsccs.h"
>   #include "intel_pxp_irq.h"
>   #include "intel_pxp_session.h"
>   #include "intel_pxp_tee.h"
> @@ -132,7 +133,10 @@ static void pxp_init_full(struct intel_pxp *pxp)
>   	if (ret)
>   		return;
>   
> -	ret = intel_pxp_tee_component_init(pxp);
> +	if (pxp->uses_gsccs)
> +		ret = intel_pxp_gsccs_init(pxp);
> +	else
> +		ret = intel_pxp_tee_component_init(pxp);
>   	if (ret)
>   		goto out_context;
>   
> @@ -157,6 +161,11 @@ static struct intel_gt *find_gt_for_required_teelink(struct drm_i915_private *i9
>   	return NULL;
>   }
>   
> +static bool pxp_has_gsccs(struct drm_i915_private *i915)
> +{
> +	return (i915->media_gt && HAS_ENGINE(i915->media_gt, GSC0));
> +}
> +
>   static struct intel_gt *find_gt_for_required_protected_content(struct drm_i915_private *i915)
>   {
>   	if (!IS_ENABLED(CONFIG_DRM_I915_PXP) || !INTEL_INFO(i915)->has_pxp)
> @@ -167,7 +176,7 @@ static struct intel_gt *find_gt_for_required_protected_content(struct drm_i915_p
>   	 * on the media GT. NOTE: if we have a media-tile with a GSC-engine,
>   	 * the VDBOX is already present so skip that check
>   	 */
> -	if (i915->media_gt && HAS_ENGINE(i915->media_gt, GSC0))
> +	if (pxp_has_gsccs(i915))
>   		return i915->media_gt;
>   
>   	/*
> @@ -208,6 +217,7 @@ int intel_pxp_init(struct drm_i915_private *i915)
>   		return -ENOMEM;
>   
>   	i915->pxp->ctrl_gt = gt;
> +	i915->pxp->uses_gsccs = pxp_has_gsccs(i915);

At this point, if we're on a platform with the media GT we've already 
selected it, so you can just do HAS_ENGINE(pxp->ctrl_gt, GSC0).
Also, do we really need a local variable to store this info? Can't we 
just do HAS_ENGINE(...) where we need it?

Daniele

>   
>   	/*
>   	 * If full PXP feature is not available but HuC is loaded by GSC on pre-MTL
> @@ -229,7 +239,10 @@ void intel_pxp_fini(struct drm_i915_private *i915)
>   
>   	i915->pxp->arb_is_valid = false;
>   
> -	intel_pxp_tee_component_fini(i915->pxp);
> +	if (i915->pxp->uses_gsccs)
> +		intel_pxp_gsccs_fini(i915->pxp);
> +	else
> +		intel_pxp_tee_component_fini(i915->pxp);
>   
>   	destroy_vcs_context(i915->pxp);
>   
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
> new file mode 100644
> index 000000000000..21400650fc86
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright(c) 2023 Intel Corporation.
> + */
> +
> +#include "i915_drv.h"
> +#include "intel_pxp_types.h"
> +#include "intel_pxp_gsccs.h"
> +
> +int intel_pxp_gsccs_create_session(struct intel_pxp *pxp,
> +				   int arb_session_id)
> +{
> +	return -ENODEV;
> +}
> +
> +void intel_pxp_gsccs_fini(struct intel_pxp *pxp)
> +{
> +}
> +
> +int intel_pxp_gsccs_init(struct intel_pxp *pxp)
> +{
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
> new file mode 100644
> index 000000000000..07f8c9b6f636
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright(c) 2022, Intel Corporation. All rights reserved.
> + */
> +
> +#ifndef __INTEL_PXP_GSCCS_H__
> +#define __INTEL_PXP_GSCCS_H__
> +
> +#include <linux/types.h>
> +
> +struct intel_pxp;
> +
> +#ifdef CONFIG_DRM_I915_PXP
> +int intel_pxp_gsccs_create_session(struct intel_pxp *pxp,
> +				   int arb_session_id);
> +void intel_pxp_gsccs_fini(struct intel_pxp *pxp);
> +int intel_pxp_gsccs_init(struct intel_pxp *pxp);
> +
> +#else
> +static inline void intel_pxp_gsccs_fini(struct intel_pxp *pxp)
> +{
> +}
> +
> +static inline int intel_pxp_gsccs_init(struct intel_pxp *pxp)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#endif /*__INTEL_PXP_GSCCS_H__ */
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
> index ae413580b81a..080aa2209c5b 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
> @@ -7,6 +7,7 @@
>   
>   #include "intel_pxp.h"
>   #include "intel_pxp_cmd.h"
> +#include "intel_pxp_gsccs.h"
>   #include "intel_pxp_session.h"
>   #include "intel_pxp_tee.h"
>   #include "intel_pxp_types.h"
> @@ -66,7 +67,10 @@ static int pxp_create_arb_session(struct intel_pxp *pxp)
>   		return -EEXIST;
>   	}
>   
> -	ret = intel_pxp_tee_cmd_create_arb_session(pxp, ARB_SESSION);
> +	if (pxp->uses_gsccs)
> +		ret = intel_pxp_gsccs_create_session(pxp, ARB_SESSION);
> +	else
> +		ret = intel_pxp_tee_cmd_create_arb_session(pxp, ARB_SESSION);
>   	if (ret) {
>   		drm_err(&gt->i915->drm, "tee cmd for arb session creation failed\n");
>   		return ret;
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> index 7dc5f08d1583..43aa61c26de5 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> @@ -26,6 +26,12 @@ struct intel_pxp {
>   	 */
>   	struct intel_gt *ctrl_gt;
>   
> +	/**
> +	 * @uses_gsccs: PXP interface for firmware access and pxp-session controls is
> +	 * via the GSC-CS engine. This is for MTL+ platforms.
> +	 */
> +	bool uses_gsccs;
> +
>   	/**
>   	 * @pxp_component: i915_pxp_component struct of the bound mei_pxp
>   	 * module. Only set and cleared inside component bind/unbind functions,
Teres Alexis, Alan Previn Jan. 20, 2023, 12:20 a.m. UTC | #2 
Thanks for reviewing Daniele - will fix these on re-rev.
And you're right - we dont need a variable "gsccs" (so HAS_ENGINE should work fine).

On Wed, 2023-01-18 at 09:51 -0800, Ceraolo Spurio, Daniele wrote:
> 
> 
Alan: [snip]
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index f47f00b162a4..eae4325310e8 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -330,6 +330,7 @@  i915-y += \
 i915-$(CONFIG_DRM_I915_PXP) += \
 	pxp/intel_pxp_cmd.o \
 	pxp/intel_pxp_debugfs.o \
+	pxp/intel_pxp_gsccs.o \
 	pxp/intel_pxp_irq.o \
 	pxp/intel_pxp_pm.o \
 	pxp/intel_pxp_session.o
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
index cfc9af8b3d21..be52bf92e847 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
@@ -12,6 +12,7 @@ 
 #include "i915_drv.h"
 
 #include "intel_pxp.h"
+#include "intel_pxp_gsccs.h"
 #include "intel_pxp_irq.h"
 #include "intel_pxp_session.h"
 #include "intel_pxp_tee.h"
@@ -132,7 +133,10 @@  static void pxp_init_full(struct intel_pxp *pxp)
 	if (ret)
 		return;
 
-	ret = intel_pxp_tee_component_init(pxp);
+	if (pxp->uses_gsccs)
+		ret = intel_pxp_gsccs_init(pxp);
+	else
+		ret = intel_pxp_tee_component_init(pxp);
 	if (ret)
 		goto out_context;
 
@@ -157,6 +161,11 @@  static struct intel_gt *find_gt_for_required_teelink(struct drm_i915_private *i9
 	return NULL;
 }
 
+static bool pxp_has_gsccs(struct drm_i915_private *i915)
+{
+	return (i915->media_gt && HAS_ENGINE(i915->media_gt, GSC0));
+}
+
 static struct intel_gt *find_gt_for_required_protected_content(struct drm_i915_private *i915)
 {
 	if (!IS_ENABLED(CONFIG_DRM_I915_PXP) || !INTEL_INFO(i915)->has_pxp)
@@ -167,7 +176,7 @@  static struct intel_gt *find_gt_for_required_protected_content(struct drm_i915_p
 	 * on the media GT. NOTE: if we have a media-tile with a GSC-engine,
 	 * the VDBOX is already present so skip that check
 	 */
-	if (i915->media_gt && HAS_ENGINE(i915->media_gt, GSC0))
+	if (pxp_has_gsccs(i915))
 		return i915->media_gt;
 
 	/*
@@ -208,6 +217,7 @@  int intel_pxp_init(struct drm_i915_private *i915)
 		return -ENOMEM;
 
 	i915->pxp->ctrl_gt = gt;
+	i915->pxp->uses_gsccs = pxp_has_gsccs(i915);
 
 	/*
 	 * If full PXP feature is not available but HuC is loaded by GSC on pre-MTL
@@ -229,7 +239,10 @@  void intel_pxp_fini(struct drm_i915_private *i915)
 
 	i915->pxp->arb_is_valid = false;
 
-	intel_pxp_tee_component_fini(i915->pxp);
+	if (i915->pxp->uses_gsccs)
+		intel_pxp_gsccs_fini(i915->pxp);
+	else
+		intel_pxp_tee_component_fini(i915->pxp);
 
 	destroy_vcs_context(i915->pxp);
 
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
new file mode 100644
index 000000000000..21400650fc86
--- /dev/null
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
@@ -0,0 +1,23 @@ 
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright(c) 2023 Intel Corporation.
+ */
+
+#include "i915_drv.h"
+#include "intel_pxp_types.h"
+#include "intel_pxp_gsccs.h"
+
+int intel_pxp_gsccs_create_session(struct intel_pxp *pxp,
+				   int arb_session_id)
+{
+	return -ENODEV;
+}
+
+void intel_pxp_gsccs_fini(struct intel_pxp *pxp)
+{
+}
+
+int intel_pxp_gsccs_init(struct intel_pxp *pxp)
+{
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
new file mode 100644
index 000000000000..07f8c9b6f636
--- /dev/null
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
@@ -0,0 +1,30 @@ 
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright(c) 2022, Intel Corporation. All rights reserved.
+ */
+
+#ifndef __INTEL_PXP_GSCCS_H__
+#define __INTEL_PXP_GSCCS_H__
+
+#include <linux/types.h>
+
+struct intel_pxp;
+
+#ifdef CONFIG_DRM_I915_PXP
+int intel_pxp_gsccs_create_session(struct intel_pxp *pxp,
+				   int arb_session_id);
+void intel_pxp_gsccs_fini(struct intel_pxp *pxp);
+int intel_pxp_gsccs_init(struct intel_pxp *pxp);
+
+#else
+static inline void intel_pxp_gsccs_fini(struct intel_pxp *pxp)
+{
+}
+
+static inline int intel_pxp_gsccs_init(struct intel_pxp *pxp)
+{
+	return 0;
+}
+#endif
+
+#endif /*__INTEL_PXP_GSCCS_H__ */
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
index ae413580b81a..080aa2209c5b 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
@@ -7,6 +7,7 @@ 
 
 #include "intel_pxp.h"
 #include "intel_pxp_cmd.h"
+#include "intel_pxp_gsccs.h"
 #include "intel_pxp_session.h"
 #include "intel_pxp_tee.h"
 #include "intel_pxp_types.h"
@@ -66,7 +67,10 @@  static int pxp_create_arb_session(struct intel_pxp *pxp)
 		return -EEXIST;
 	}
 
-	ret = intel_pxp_tee_cmd_create_arb_session(pxp, ARB_SESSION);
+	if (pxp->uses_gsccs)
+		ret = intel_pxp_gsccs_create_session(pxp, ARB_SESSION);
+	else
+		ret = intel_pxp_tee_cmd_create_arb_session(pxp, ARB_SESSION);
 	if (ret) {
 		drm_err(&gt->i915->drm, "tee cmd for arb session creation failed\n");
 		return ret;
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
index 7dc5f08d1583..43aa61c26de5 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
@@ -26,6 +26,12 @@  struct intel_pxp {
 	 */
 	struct intel_gt *ctrl_gt;
 
+	/**
+	 * @uses_gsccs: PXP interface for firmware access and pxp-session controls is
+	 * via the GSC-CS engine. This is for MTL+ platforms.
+	 */
+	bool uses_gsccs;
+
 	/**
 	 * @pxp_component: i915_pxp_component struct of the bound mei_pxp
 	 * module. Only set and cleared inside component bind/unbind functions,