[Intel-gfx] [RFC-v23 13/13] drm/i915/pxp: Add plane decryption support

[Ville Syrjälä]

On Tue, Jan 19, 2021 at 09:35:18AM +0000, Gupta, Anshuman wrote:
> Jani/Ville
> I had received an offline comment form Gaurav on this patch,
> See below,
> > -----Original Message-----
> > From: Huang, Sean Z <sean.z.huang at intel.com>
> > Sent: Tuesday, January 19, 2021 1:13 PM
> > To: Intel-gfx at lists.freedesktop.org
> > Cc: Gaurav, Kumar <kumar.gaurav at intel.com>; Gupta, Anshuman
> > <anshuman.gupta at intel.com>; Bommu, Krishnaiah
> > <krishnaiah.bommu at intel.com>; Huang, Sean Z <sean.z.huang at intel.com>
> > Subject: [RFC-v23 13/13] drm/i915/pxp: Add plane decryption support
> > 
> > From: Anshuman Gupta <anshuman.gupta at intel.com>
> > 
> > Add support to enable/disable PLANE_SURF Decryption Request bit.
> > It requires only to enable plane decryption support when following
> > condition met.
> > 1. PXP session is enabled.
> > 2. Buffer object is protected.
> > 
> > v2:
> > - Rebased to libva_cp-drm-tip_tgl_cp tree.
> > - Used gen fb obj user_flags instead gem_object_metadata. [Krishna]
> > 
> > v3:
> > - intel_pxp_gem_object_status() API changes.
> > 
> > Cc: Bommu Krishnaiah <krishnaiah.bommu at intel.com>
> > Cc: Huang Sean Z <sean.z.huang at intel.com>
> > Cc: Gaurav Kumar <kumar.gaurav at intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_sprite.c | 21 ++++++++++++++++++---
> >  drivers/gpu/drm/i915/i915_reg.h             |  1 +
> >  2 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > index cf3589fd0ddb..39f8c922ce66 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > @@ -39,6 +39,8 @@
> >  #include <drm/drm_plane_helper.h>
> >  #include <drm/drm_rect.h>
> > 
> > +#include "pxp/intel_pxp.h"
> > +
> >  #include "i915_drv.h"
> >  #include "i915_trace.h"
> >  #include "i915_vgpu.h"
> > @@ -768,6 +770,11 @@ icl_program_input_csc(struct intel_plane *plane,
> >  			  PLANE_INPUT_CSC_POSTOFF(pipe, plane_id, 2),
> > 0x0);  }
> > 
> > +static bool intel_fb_obj_protected(const struct drm_i915_gem_object
> > +*obj) {
> > +	return obj->user_flags & I915_BO_PROTECTED ? true : false; }
> > +
> >  static void
> >  skl_plane_async_flip(struct intel_plane *plane,
> >  		     const struct intel_crtc_state *crtc_state, @@ -804,6
> > +811,7 @@ skl_program_plane(struct intel_plane *plane,
> >  	u32 surf_addr = plane_state->color_plane[color_plane].offset;
> >  	u32 stride = skl_plane_stride(plane_state, color_plane);
> >  	const struct drm_framebuffer *fb = plane_state->hw.fb;
> > +	const struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> >  	int aux_plane = intel_main_to_aux_plane(fb, color_plane);
> >  	int crtc_x = plane_state->uapi.dst.x1;
> >  	int crtc_y = plane_state->uapi.dst.y1; @@ -814,7 +822,7 @@
> > skl_program_plane(struct intel_plane *plane,
> >  	u8 alpha = plane_state->hw.alpha >> 8;
> >  	u32 plane_color_ctl = 0, aux_dist = 0;
> >  	unsigned long irqflags;
> > -	u32 keymsk, keymax;
> > +	u32 keymsk, keymax, plane_surf;
> >  	u32 plane_ctl = plane_state->ctl;
> > 
> >  	plane_ctl |= skl_plane_ctl_crtc(crtc_state); @@ -890,8 +898,15
> > @@ skl_program_plane(struct intel_plane *plane,
> >  	 * the control register just before the surface register.
> >  	 */
> >  	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl);
> > -	intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id),
> > -			  intel_plane_ggtt_offset(plane_state) + surf_addr);
> > +	plane_surf = intel_plane_ggtt_offset(plane_state) + surf_addr;
> > +
> > +	if (intel_pxp_gem_object_status(dev_priv) &&
> > +	    intel_fb_obj_protected(obj))
> > +		plane_surf |= PLANE_SURF_DECRYPTION_ENABLED;
> Here in case of if fb obj is protected but pxp session is not enabled i.e intel_pxp_gem_object_status() returns false, request to show the black frame buffer on display instead of corrupted data.
>                             plane_surf = 0xXXX; //Pointer to black framebuffer
> But above approach would be a hack. 
> @Jani and @Ville could you please guide with the general way of handling this as pxp session keys can be invalidated at any time.

Would need such a black buffer to be always pinned into the gtt, which
is seems a bit wasteful. We could perhaps just force the plane to output
black eg. by using the plane gamma. I think we should always have the
per-plane gamma available on skl+ universal planes. Cursor may be a
different story.

-- 
Ville Syrjälä
Intel