[Intel-xe] [PATCH 0/4] xe&i915 display pxp cleanup

[Intel-xe] [PATCH 0/4] xe&i915 display pxp cleanup

[Jani Nikula]

Add slightly cleaner abstractions for plane protection.

Not to be merged before patches 2&3 have been merged to i915 upstream.

Jani Nikula (4):
  Revert "FIXME: drm/i915/pxp: some hack to bypass pxp"
  drm/i915: abstract plane protection check
  drm/i915: remove excess functions from plane protection check
  fixup! drm/xe/display: Implement display support

 .../drm/i915/display/skl_universal_plane.c    | 31 +++++++------------
 .../xe/compat-i915-headers/pxp/intel_pxp.h    | 28 +++++++++++++++++
 drivers/gpu/drm/xe/xe_device_types.h          |  2 ++
 3 files changed, 41 insertions(+), 20 deletions(-)
 create mode 100644 drivers/gpu/drm/xe/compat-i915-headers/pxp/intel_pxp.h

-- 
2.39.2

[Intel-xe] [PATCH 1/4] Revert "FIXME: drm/i915/pxp: some hack to bypass pxp"

[Jani Nikula]

This reverts commit 7c700778c3b455c853c9ada8b85e83a550171c03.

To be replaced with a slightly cleaner abstraction.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/skl_universal_plane.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 68eabe089aaf..a4d0e2ede2f7 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -21,11 +21,7 @@
 #include "skl_scaler.h"
 #include "skl_universal_plane.h"
 #include "skl_watermark.h"
-#ifdef I915
 #include "pxp/intel_pxp.h"
-#else
-// TODO: pxp?
-#endif
 
 static const u32 skl_plane_formats[] = {
 	DRM_FORMAT_C8,
@@ -1864,14 +1860,9 @@ static bool skl_fb_scalable(const struct drm_framebuffer *fb)
 
 static bool bo_has_valid_encryption(struct drm_i915_gem_object *obj)
 {
-#ifdef I915
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 
 	return intel_pxp_key_check(i915->pxp, obj, false) == 0;
-#else
-#define i915_gem_object_is_protected(x) ((x) && 0)
-	return false;
-#endif
 }
 
 static bool pxp_is_borked(struct drm_i915_gem_object *obj)
-- 
2.39.2

[Intel-xe] [PATCH 2/4] drm/i915: abstract plane protection check

[Jani Nikula]

Centralize the conditions in a function.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20231109160722.3372379-1-jani.nikula@intel.com
---
 .../gpu/drm/i915/display/skl_universal_plane.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index a4d0e2ede2f7..c1d426dbcc62 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1870,6 +1870,19 @@ static bool pxp_is_borked(struct drm_i915_gem_object *obj)
 	return i915_gem_object_is_protected(obj) && !bo_has_valid_encryption(obj);
 }
 
+static void check_protection(struct intel_plane_state *plane_state)
+{
+	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
+	struct drm_i915_private *i915 = to_i915(plane->base.dev);
+	const struct drm_framebuffer *fb = plane_state->hw.fb;
+
+	if (DISPLAY_VER(i915) < 11)
+		return;
+
+	plane_state->decrypt = bo_has_valid_encryption(intel_fb_obj(fb));
+	plane_state->force_black = pxp_is_borked(intel_fb_obj(fb));
+}
+
 static int skl_plane_check(struct intel_crtc_state *crtc_state,
 			   struct intel_plane_state *plane_state)
 {
@@ -1914,10 +1927,7 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
-	if (DISPLAY_VER(dev_priv) >= 11) {
-		plane_state->decrypt = bo_has_valid_encryption(intel_fb_obj(fb));
-		plane_state->force_black = pxp_is_borked(intel_fb_obj(fb));
-	}
+	check_protection(plane_state);
 
 	/* HW only has 8 bits pixel precision, disable plane if invisible */
 	if (!(plane_state->hw.alpha >> 8))
-- 
2.39.2

Re: [Intel-xe] [PATCH 2/4] drm/i915: abstract plane protection check

[Rodrigo Vivi]

On Thu, Nov 09, 2023 at 06:17:56PM +0200, Jani Nikula wrote:
> Centralize the conditions in a function.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20231109160722.3372379-1-jani.nikula@intel.com
> ---
>  .../gpu/drm/i915/display/skl_universal_plane.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index a4d0e2ede2f7..c1d426dbcc62 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1870,6 +1870,19 @@ static bool pxp_is_borked(struct drm_i915_gem_object *obj)
>  	return i915_gem_object_is_protected(obj) && !bo_has_valid_encryption(obj);
>  }
>  
> +static void check_protection(struct intel_plane_state *plane_state)

only not sure about this function name. maybe something with content_protection on it?!

but on the lack of a better suggestion:


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
(for the series)

> +{
> +	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
> +	struct drm_i915_private *i915 = to_i915(plane->base.dev);
> +	const struct drm_framebuffer *fb = plane_state->hw.fb;
> +
> +	if (DISPLAY_VER(i915) < 11)
> +		return;
> +
> +	plane_state->decrypt = bo_has_valid_encryption(intel_fb_obj(fb));
> +	plane_state->force_black = pxp_is_borked(intel_fb_obj(fb));
> +}
> +
>  static int skl_plane_check(struct intel_crtc_state *crtc_state,
>  			   struct intel_plane_state *plane_state)
>  {
> @@ -1914,10 +1927,7 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state,
>  	if (ret)
>  		return ret;
>  
> -	if (DISPLAY_VER(dev_priv) >= 11) {
> -		plane_state->decrypt = bo_has_valid_encryption(intel_fb_obj(fb));
> -		plane_state->force_black = pxp_is_borked(intel_fb_obj(fb));
> -	}
> +	check_protection(plane_state);
>  
>  	/* HW only has 8 bits pixel precision, disable plane if invisible */
>  	if (!(plane_state->hw.alpha >> 8))
> -- 
> 2.39.2
> 

[Intel-xe] [PATCH 3/4] drm/i915: remove excess functions from plane protection check

[Jani Nikula]

Reduce the function calls by reusing ->decrypt.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20231109160722.3372379-2-jani.nikula@intel.com
---
 .../gpu/drm/i915/display/skl_universal_plane.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index c1d426dbcc62..301b3f2303ea 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1858,29 +1858,19 @@ static bool skl_fb_scalable(const struct drm_framebuffer *fb)
 	}
 }
 
-static bool bo_has_valid_encryption(struct drm_i915_gem_object *obj)
-{
-	struct drm_i915_private *i915 = to_i915(obj->base.dev);
-
-	return intel_pxp_key_check(i915->pxp, obj, false) == 0;
-}
-
-static bool pxp_is_borked(struct drm_i915_gem_object *obj)
-{
-	return i915_gem_object_is_protected(obj) && !bo_has_valid_encryption(obj);
-}
-
 static void check_protection(struct intel_plane_state *plane_state)
 {
 	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
 	struct drm_i915_private *i915 = to_i915(plane->base.dev);
 	const struct drm_framebuffer *fb = plane_state->hw.fb;
+	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 
 	if (DISPLAY_VER(i915) < 11)
 		return;
 
-	plane_state->decrypt = bo_has_valid_encryption(intel_fb_obj(fb));
-	plane_state->force_black = pxp_is_borked(intel_fb_obj(fb));
+	plane_state->decrypt = intel_pxp_key_check(i915->pxp, obj, false) == 0;
+	plane_state->force_black = i915_gem_object_is_protected(obj) &&
+		!plane_state->decrypt;
 }
 
 static int skl_plane_check(struct intel_crtc_state *crtc_state,
-- 
2.39.2

[Intel-xe] [PATCH 4/4] fixup! drm/xe/display: Implement display support

[Jani Nikula]

Wrappers for pxp.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../xe/compat-i915-headers/pxp/intel_pxp.h    | 28 +++++++++++++++++++
 drivers/gpu/drm/xe/xe_device_types.h          |  2 ++
 2 files changed, 30 insertions(+)
 create mode 100644 drivers/gpu/drm/xe/compat-i915-headers/pxp/intel_pxp.h

diff --git a/drivers/gpu/drm/xe/compat-i915-headers/pxp/intel_pxp.h b/drivers/gpu/drm/xe/compat-i915-headers/pxp/intel_pxp.h
new file mode 100644
index 000000000000..c2c30ece8f77
--- /dev/null
+++ b/drivers/gpu/drm/xe/compat-i915-headers/pxp/intel_pxp.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2023 Intel Corporation
+ */
+
+#ifndef __INTEL_PXP_H__
+#define __INTEL_PXP_H__
+
+#include <linux/errno.h>
+#include <linux/types.h>
+
+struct drm_i915_gem_object;
+struct intel_pxp;
+
+static inline int intel_pxp_key_check(struct intel_pxp *pxp,
+				      struct drm_i915_gem_object *obj,
+				      bool assign)
+{
+	return -ENODEV;
+}
+
+static inline bool
+i915_gem_object_is_protected(const struct drm_i915_gem_object *obj)
+{
+	return false;
+}
+
+#endif
diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 4119ef03fb7e..be11cadccbd4 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -480,6 +480,8 @@ struct xe_device {
 	struct {
 		const char *dmc_firmware_path;
 	} params;
+
+	void *pxp;
 #endif
 };
 
-- 
2.39.2

[Intel-xe] ✗ CI.Patch_applied: failure for xe&i915 display pxp cleanup

[Patchwork]

== Series Details ==

Series: xe&i915 display pxp cleanup
URL   : https://patchwork.freedesktop.org/series/126208/
State : failure

== Summary ==

=== Applying kernel patches on branch 'drm-xe-next' with base: ===
Base commit: 2096aea11 fixup! drm/xe/gsc: add gsc device support
=== git am output follows ===
Applying: Revert "FIXME: drm/i915/pxp: some hack to bypass pxp"
Applying: drm/i915: abstract plane protection check
Applying: drm/i915: remove excess functions from plane protection check
Applying: fixup! drm/xe/display: Implement display support