Re: [Intel-gfx] [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
On Fri, Apr 30, 2021 at 06:56:10AM +, Gupta, Anshuman wrote:
>
>
> > -Original Message-
> > From: Ceraolo Spurio, Daniele
> > Sent: Thursday, April 29, 2021 2:10 AM
> > To: Ville Syrjälä
> > Cc: Gupta, Anshuman ; Vivi, Rodrigo
> > ; intel-gfx@lists.freedesktop.org; Bommu, Krishnaiah
> > ; Gaurav, Kumar
> > Subject: Re: [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
> >
> >
> >
> > On 4/28/2021 1:04 PM, Ville Syrjälä wrote:
> > > On Wed, Apr 28, 2021 at 10:32:46AM -0700, Daniele Ceraolo Spurio wrote:
> > >>
> > >> On 4/28/2021 5:03 AM, Ville Syrjälä wrote:
> > >>> On Wed, Apr 28, 2021 at 11:25:23AM +, Gupta, Anshuman wrote:
> > > -Original Message-
> > > From: Ville Syrjälä
> > > Sent: Wednesday, April 28, 2021 12:25 AM
> > > To: Gupta, Anshuman
> > > Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo
> > > ; Bommu, Krishnaiah
> > > ; Huang Sean Z
> > > ; Gaurav, Kumar ;
> > > Ceraolo Spurio, Daniele
> > > Subject: Re: [PATCH v3 14/16] drm/i915/pxp: Add plane decryption
> > > support
> > Thanks Ville for review, Below are some doubts with respect to pxp
> > state.
> > > On Tue, Apr 27, 2021 at 04:13:11PM +0530, Anshuman Gupta wrote:
> > >> 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:
> > >> - Used gen fb obj user_flags instead gem_object_metadata.
> > >> [Krishna]
> > >>
> > >> v3:
> > >> - intel_pxp_gem_object_status() API changes.
> > >>
> > >> v4: use intel_pxp_is_active (Daniele)
> > >>
> > >> v5: rebase and use the new protected object status checker
> > >> (Daniele)
> > >>
> > >> v6: used plane state for plane_decryption to handle async flip
> > >> as suggested by Ville.
> > >>
> > >> Cc: Bommu Krishnaiah
> > >> Cc: Huang Sean Z
> > >> Cc: Gaurav Kumar
> > >> Cc: Ville Syrjälä
> > >> Signed-off-by: Anshuman Gupta
> > >> Signed-off-by: Daniele Ceraolo Spurio
> > >>
> > >> ---
> > >>.../gpu/drm/i915/display/intel_atomic_plane.c | 3 ++
> > >> drivers/gpu/drm/i915/display/intel_display.c | 5 +++
> > >>.../drm/i915/display/intel_display_types.h| 3 ++
> > >>.../drm/i915/display/skl_universal_plane.c| 32
> > >> +-
> > -
> > >>.../drm/i915/display/skl_universal_plane.h| 1 +
> > >>drivers/gpu/drm/i915/i915_reg.h | 1 +
> > >>6 files changed, 42 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > >> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > >> index 7bfb26ca0bd0..7057077a2b71 100644
> > >> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > >> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > >> @@ -394,6 +394,7 @@ int intel_plane_atomic_check(struct
> > > intel_atomic_state *state,
> > >> intel_atomic_get_old_crtc_state(state, crtc);
> > >> struct intel_crtc_state *new_crtc_state =
> > >> intel_atomic_get_new_crtc_state(state, crtc);
> > >> +const struct drm_framebuffer *fb = new_plane_state->hw.fb;
> > >>
> > >> if (new_crtc_state && new_crtc_state->bigjoiner_slave) {
> > >> struct intel_plane *master_plane = @@ -409,6 +410,8
> > @@ int
> > >> intel_plane_atomic_check(struct
> > > intel_atomic_state *state,
> > >> intel_plane_copy_uapi_to_hw_state(new_plane_state,
> > >>new_master_plane_state,
> > >>crtc);
> > >> +new_plane_state->plane_decryption =
> > >> +
> > i915_gem_object_has_valid_protection(intel_fb_obj(fb));
> > >>
> > >> new_plane_state->uapi.visible = false;
> > >> if (!new_crtc_state)
> > >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > >> b/drivers/gpu/drm/i915/display/intel_display.c
> > >> index a10e26380ef3..55ab2d0b92d8 100644
> > >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> > >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > >> @@ -9367,6 +9367,10 @@ static int intel_atomic_check_async(struct
> > > intel_atomic_state *state)
> > >> drm_dbg_kms(>drm, "Color range
> > cannot be
> > > changed in async flip\n");
> > >> return -EINVAL;
> > >> }
> > >> +
> > >> +/* plane decryption is allow to change only in
> > synchronous
> > >> +flips
> > > */
> > >> +if (old_plane_state->plane_decryption !=
> > new_plane_state-
> > >> plane_decryption)
> >
Re: [Intel-gfx] [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
> -Original Message-
> From: Ville Syrjälä
> Sent: Wednesday, April 28, 2021 12:25 AM
> To: Gupta, Anshuman
> Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo ;
> Bommu, Krishnaiah ; Huang Sean Z
> ; Gaurav, Kumar ;
> Ceraolo Spurio, Daniele
> Subject: Re: [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
>
> On Tue, Apr 27, 2021 at 04:13:11PM +0530, Anshuman Gupta wrote:
> > 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:
> > - Used gen fb obj user_flags instead gem_object_metadata. [Krishna]
> >
> > v3:
> > - intel_pxp_gem_object_status() API changes.
> >
> > v4: use intel_pxp_is_active (Daniele)
> >
> > v5: rebase and use the new protected object status checker (Daniele)
> >
> > v6: used plane state for plane_decryption to handle async flip
> > as suggested by Ville.
> >
> > Cc: Bommu Krishnaiah
> > Cc: Huang Sean Z
> > Cc: Gaurav Kumar
> > Cc: Ville Syrjälä
> > Signed-off-by: Anshuman Gupta
> > Signed-off-by: Daniele Ceraolo Spurio
> >
> > ---
> > .../gpu/drm/i915/display/intel_atomic_plane.c | 3 ++
> > drivers/gpu/drm/i915/display/intel_display.c | 5 +++
> > .../drm/i915/display/intel_display_types.h| 3 ++
> > .../drm/i915/display/skl_universal_plane.c| 32 +--
> > .../drm/i915/display/skl_universal_plane.h| 1 +
> > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > 6 files changed, 42 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > index 7bfb26ca0bd0..7057077a2b71 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > @@ -394,6 +394,7 @@ int intel_plane_atomic_check(struct
> intel_atomic_state *state,
> > intel_atomic_get_old_crtc_state(state, crtc);
> > struct intel_crtc_state *new_crtc_state =
> > intel_atomic_get_new_crtc_state(state, crtc);
> > + const struct drm_framebuffer *fb = new_plane_state->hw.fb;
> >
> > if (new_crtc_state && new_crtc_state->bigjoiner_slave) {
> > struct intel_plane *master_plane =
> > @@ -409,6 +410,8 @@ int intel_plane_atomic_check(struct
> intel_atomic_state *state,
> > intel_plane_copy_uapi_to_hw_state(new_plane_state,
> > new_master_plane_state,
> > crtc);
> > + new_plane_state->plane_decryption =
> > + i915_gem_object_has_valid_protection(intel_fb_obj(fb));
> >
> > new_plane_state->uapi.visible = false;
> > if (!new_crtc_state)
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index a10e26380ef3..55ab2d0b92d8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -9367,6 +9367,10 @@ static int intel_atomic_check_async(struct
> intel_atomic_state *state)
> > drm_dbg_kms(>drm, "Color range cannot be
> changed in async flip\n");
> > return -EINVAL;
> > }
> > +
> > + /* plane decryption is allow to change only in synchronous flips
> */
> > + if (old_plane_state->plane_decryption != new_plane_state-
> >plane_decryption)
> > + return -EINVAL;
> > }
> >
> > return 0;
> > @@ -12350,6 +12354,7 @@ static void readout_plane_state(struct
> > drm_i915_private *dev_priv)
> >
> > crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> > crtc_state = to_intel_crtc_state(crtc->base.state);
> > + intel_plane_read_hw_decryption(plane_state);
>
> We don't have real plane state readout anyway, so seems pointless.
could u please guide me the correct place to read the old plane decrypt
State, some where we do need to populate the plane decrypt state with plane
surface register decrypt bit.
How about plane->get_hw_state ?
Thanks,
Anshuman Gupta.
>
> >
> > intel_set_plane_visible(crtc_state, plane_state, visible);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index e2e707c4dff5..76b3bb64a36a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -617,6 +617,9 @@ struct intel_plane_state {
> >
> > struct intel_fb_view view;
> >
> > + /* Plane pxp decryption state */
> > + bool plane_decryption;
> > +
>
> It's all about the plane, so the plane_ prefix is entirely redundant.
> Could just call it "decrypt" I guess.
>
> > /* plane control register */
> > u32 ctl;
> >
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> >
Re: [Intel-gfx] [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
> -Original Message-
> From: Ceraolo Spurio, Daniele
> Sent: Thursday, April 29, 2021 2:10 AM
> To: Ville Syrjälä
> Cc: Gupta, Anshuman ; Vivi, Rodrigo
> ; intel-gfx@lists.freedesktop.org; Bommu, Krishnaiah
> ; Gaurav, Kumar
> Subject: Re: [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
>
>
>
> On 4/28/2021 1:04 PM, Ville Syrjälä wrote:
> > On Wed, Apr 28, 2021 at 10:32:46AM -0700, Daniele Ceraolo Spurio wrote:
> >>
> >> On 4/28/2021 5:03 AM, Ville Syrjälä wrote:
> >>> On Wed, Apr 28, 2021 at 11:25:23AM +, Gupta, Anshuman wrote:
> > -Original Message-
> > From: Ville Syrjälä
> > Sent: Wednesday, April 28, 2021 12:25 AM
> > To: Gupta, Anshuman
> > Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo
> > ; Bommu, Krishnaiah
> > ; Huang Sean Z
> > ; Gaurav, Kumar ;
> > Ceraolo Spurio, Daniele
> > Subject: Re: [PATCH v3 14/16] drm/i915/pxp: Add plane decryption
> > support
> Thanks Ville for review, Below are some doubts with respect to pxp state.
> > On Tue, Apr 27, 2021 at 04:13:11PM +0530, Anshuman Gupta wrote:
> >> 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:
> >> - Used gen fb obj user_flags instead gem_object_metadata.
> >> [Krishna]
> >>
> >> v3:
> >> - intel_pxp_gem_object_status() API changes.
> >>
> >> v4: use intel_pxp_is_active (Daniele)
> >>
> >> v5: rebase and use the new protected object status checker
> >> (Daniele)
> >>
> >> v6: used plane state for plane_decryption to handle async flip
> >> as suggested by Ville.
> >>
> >> Cc: Bommu Krishnaiah
> >> Cc: Huang Sean Z
> >> Cc: Gaurav Kumar
> >> Cc: Ville Syrjälä
> >> Signed-off-by: Anshuman Gupta
> >> Signed-off-by: Daniele Ceraolo Spurio
> >>
> >> ---
> >>.../gpu/drm/i915/display/intel_atomic_plane.c | 3 ++
> >> drivers/gpu/drm/i915/display/intel_display.c | 5 +++
> >>.../drm/i915/display/intel_display_types.h| 3 ++
> >>.../drm/i915/display/skl_universal_plane.c| 32
> >> +-
> -
> >>.../drm/i915/display/skl_universal_plane.h| 1 +
> >>drivers/gpu/drm/i915/i915_reg.h | 1 +
> >>6 files changed, 42 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> >> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> >> index 7bfb26ca0bd0..7057077a2b71 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> >> @@ -394,6 +394,7 @@ int intel_plane_atomic_check(struct
> > intel_atomic_state *state,
> >>intel_atomic_get_old_crtc_state(state, crtc);
> >>struct intel_crtc_state *new_crtc_state =
> >>intel_atomic_get_new_crtc_state(state, crtc);
> >> + const struct drm_framebuffer *fb = new_plane_state->hw.fb;
> >>
> >>if (new_crtc_state && new_crtc_state->bigjoiner_slave) {
> >>struct intel_plane *master_plane = @@ -409,6 +410,8
> @@ int
> >> intel_plane_atomic_check(struct
> > intel_atomic_state *state,
> >>intel_plane_copy_uapi_to_hw_state(new_plane_state,
> >> new_master_plane_state,
> >> crtc);
> >> + new_plane_state->plane_decryption =
> >> +
> i915_gem_object_has_valid_protection(intel_fb_obj(fb));
> >>
> >>new_plane_state->uapi.visible = false;
> >>if (!new_crtc_state)
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> >> b/drivers/gpu/drm/i915/display/intel_display.c
> >> index a10e26380ef3..55ab2d0b92d8 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >> @@ -9367,6 +9367,10 @@ static int intel_atomic_check_async(struct
> > intel_atomic_state *state)
> >>drm_dbg_kms(>drm, "Color range
> cannot be
> > changed in async flip\n");
> >>return -EINVAL;
> >>}
> >> +
> >> + /* plane decryption is allow to change only in
> synchronous
> >> +flips
> > */
> >> + if (old_plane_state->plane_decryption !=
> new_plane_state-
> >> plane_decryption)
> >> + return -EINVAL;
> >>}
> >>
> >>return 0;
> >> @@ -12350,6 +12354,7 @@ static void readout_plane_state(struct
> >> drm_i915_private *dev_priv)
> >>
> >>crtc =
Re: [Intel-gfx] [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
On 4/28/2021 1:04 PM, Ville Syrjälä wrote:
On Wed, Apr 28, 2021 at 10:32:46AM -0700, Daniele Ceraolo Spurio wrote:
On 4/28/2021 5:03 AM, Ville Syrjälä wrote:
On Wed, Apr 28, 2021 at 11:25:23AM +, Gupta, Anshuman wrote:
-Original Message-
From: Ville Syrjälä
Sent: Wednesday, April 28, 2021 12:25 AM
To: Gupta, Anshuman
Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo ;
Bommu, Krishnaiah ; Huang Sean Z
; Gaurav, Kumar ;
Ceraolo Spurio, Daniele
Subject: Re: [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
Thanks Ville for review, Below are some doubts with respect to pxp state.
On Tue, Apr 27, 2021 at 04:13:11PM +0530, Anshuman Gupta wrote:
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:
- Used gen fb obj user_flags instead gem_object_metadata. [Krishna]
v3:
- intel_pxp_gem_object_status() API changes.
v4: use intel_pxp_is_active (Daniele)
v5: rebase and use the new protected object status checker (Daniele)
v6: used plane state for plane_decryption to handle async flip
as suggested by Ville.
Cc: Bommu Krishnaiah
Cc: Huang Sean Z
Cc: Gaurav Kumar
Cc: Ville Syrjälä
Signed-off-by: Anshuman Gupta
Signed-off-by: Daniele Ceraolo Spurio
---
.../gpu/drm/i915/display/intel_atomic_plane.c | 3 ++
drivers/gpu/drm/i915/display/intel_display.c | 5 +++
.../drm/i915/display/intel_display_types.h| 3 ++
.../drm/i915/display/skl_universal_plane.c| 32 +--
.../drm/i915/display/skl_universal_plane.h| 1 +
drivers/gpu/drm/i915/i915_reg.h | 1 +
6 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 7bfb26ca0bd0..7057077a2b71 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -394,6 +394,7 @@ int intel_plane_atomic_check(struct
intel_atomic_state *state,
intel_atomic_get_old_crtc_state(state, crtc);
struct intel_crtc_state *new_crtc_state =
intel_atomic_get_new_crtc_state(state, crtc);
+ const struct drm_framebuffer *fb = new_plane_state->hw.fb;
if (new_crtc_state && new_crtc_state->bigjoiner_slave) {
struct intel_plane *master_plane =
@@ -409,6 +410,8 @@ int intel_plane_atomic_check(struct
intel_atomic_state *state,
intel_plane_copy_uapi_to_hw_state(new_plane_state,
new_master_plane_state,
crtc);
+ new_plane_state->plane_decryption =
+ i915_gem_object_has_valid_protection(intel_fb_obj(fb));
new_plane_state->uapi.visible = false;
if (!new_crtc_state)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index a10e26380ef3..55ab2d0b92d8 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -9367,6 +9367,10 @@ static int intel_atomic_check_async(struct
intel_atomic_state *state)
drm_dbg_kms(>drm, "Color range cannot be
changed in async flip\n");
return -EINVAL;
}
+
+ /* plane decryption is allow to change only in synchronous flips
*/
+ if (old_plane_state->plane_decryption != new_plane_state-
plane_decryption)
+ return -EINVAL;
}
return 0;
@@ -12350,6 +12354,7 @@ static void readout_plane_state(struct
drm_i915_private *dev_priv)
crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
crtc_state = to_intel_crtc_state(crtc->base.state);
+ intel_plane_read_hw_decryption(plane_state);
We don't have real plane state readout anyway, so seems pointless.
intel_set_plane_visible(crtc_state, plane_state, visible);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index e2e707c4dff5..76b3bb64a36a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -617,6 +617,9 @@ struct intel_plane_state {
struct intel_fb_view view;
+ /* Plane pxp decryption state */
+ bool plane_decryption;
+
It's all about the plane, so the plane_ prefix is entirely redundant.
Could just call it "decrypt" I guess.
/* plane control register */
u32 ctl;
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 75d3ca3dbb37..74489217e580 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -17,6 +17,7 @@
Re: [Intel-gfx] [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
On Wed, Apr 28, 2021 at 10:32:46AM -0700, Daniele Ceraolo Spurio wrote:
>
>
> On 4/28/2021 5:03 AM, Ville Syrjälä wrote:
> > On Wed, Apr 28, 2021 at 11:25:23AM +, Gupta, Anshuman wrote:
> >>
> >>> -Original Message-
> >>> From: Ville Syrjälä
> >>> Sent: Wednesday, April 28, 2021 12:25 AM
> >>> To: Gupta, Anshuman
> >>> Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo
> >>> ;
> >>> Bommu, Krishnaiah ; Huang Sean Z
> >>> ; Gaurav, Kumar ;
> >>> Ceraolo Spurio, Daniele
> >>> Subject: Re: [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
> >> Thanks Ville for review, Below are some doubts with respect to pxp state.
> >>> On Tue, Apr 27, 2021 at 04:13:11PM +0530, Anshuman Gupta wrote:
> 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:
> - Used gen fb obj user_flags instead gem_object_metadata. [Krishna]
>
> v3:
> - intel_pxp_gem_object_status() API changes.
>
> v4: use intel_pxp_is_active (Daniele)
>
> v5: rebase and use the new protected object status checker (Daniele)
>
> v6: used plane state for plane_decryption to handle async flip
> as suggested by Ville.
>
> Cc: Bommu Krishnaiah
> Cc: Huang Sean Z
> Cc: Gaurav Kumar
> Cc: Ville Syrjälä
> Signed-off-by: Anshuman Gupta
> Signed-off-by: Daniele Ceraolo Spurio
>
> ---
> .../gpu/drm/i915/display/intel_atomic_plane.c | 3 ++
> drivers/gpu/drm/i915/display/intel_display.c | 5 +++
> .../drm/i915/display/intel_display_types.h| 3 ++
> .../drm/i915/display/skl_universal_plane.c| 32 +--
> .../drm/i915/display/skl_universal_plane.h| 1 +
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> 6 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 7bfb26ca0bd0..7057077a2b71 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -394,6 +394,7 @@ int intel_plane_atomic_check(struct
> >>> intel_atomic_state *state,
> intel_atomic_get_old_crtc_state(state, crtc);
> struct intel_crtc_state *new_crtc_state =
> intel_atomic_get_new_crtc_state(state, crtc);
> +const struct drm_framebuffer *fb = new_plane_state->hw.fb;
>
> if (new_crtc_state && new_crtc_state->bigjoiner_slave) {
> struct intel_plane *master_plane =
> @@ -409,6 +410,8 @@ int intel_plane_atomic_check(struct
> >>> intel_atomic_state *state,
> intel_plane_copy_uapi_to_hw_state(new_plane_state,
> new_master_plane_state,
> crtc);
> +new_plane_state->plane_decryption =
> +i915_gem_object_has_valid_protection(intel_fb_obj(fb));
>
> new_plane_state->uapi.visible = false;
> if (!new_crtc_state)
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index a10e26380ef3..55ab2d0b92d8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -9367,6 +9367,10 @@ static int intel_atomic_check_async(struct
> >>> intel_atomic_state *state)
> drm_dbg_kms(>drm, "Color range cannot be
> >>> changed in async flip\n");
> return -EINVAL;
> }
> +
> +/* plane decryption is allow to change only in
> synchronous flips
> >>> */
> +if (old_plane_state->plane_decryption !=
> new_plane_state-
> plane_decryption)
> +return -EINVAL;
> }
>
> return 0;
> @@ -12350,6 +12354,7 @@ static void readout_plane_state(struct
> drm_i915_private *dev_priv)
>
> crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> crtc_state = to_intel_crtc_state(crtc->base.state);
> +intel_plane_read_hw_decryption(plane_state);
> >>> We don't have real plane state readout anyway, so seems pointless.
> >>>
> intel_set_plane_visible(crtc_state, plane_state,
> visible);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index e2e707c4dff5..76b3bb64a36a 100644
> ---
Re: [Intel-gfx] [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
On 4/28/2021 5:03 AM, Ville Syrjälä wrote:
On Wed, Apr 28, 2021 at 11:25:23AM +, Gupta, Anshuman wrote:
-Original Message-
From: Ville Syrjälä
Sent: Wednesday, April 28, 2021 12:25 AM
To: Gupta, Anshuman
Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo ;
Bommu, Krishnaiah ; Huang Sean Z
; Gaurav, Kumar ;
Ceraolo Spurio, Daniele
Subject: Re: [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
Thanks Ville for review, Below are some doubts with respect to pxp state.
On Tue, Apr 27, 2021 at 04:13:11PM +0530, Anshuman Gupta wrote:
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:
- Used gen fb obj user_flags instead gem_object_metadata. [Krishna]
v3:
- intel_pxp_gem_object_status() API changes.
v4: use intel_pxp_is_active (Daniele)
v5: rebase and use the new protected object status checker (Daniele)
v6: used plane state for plane_decryption to handle async flip
as suggested by Ville.
Cc: Bommu Krishnaiah
Cc: Huang Sean Z
Cc: Gaurav Kumar
Cc: Ville Syrjälä
Signed-off-by: Anshuman Gupta
Signed-off-by: Daniele Ceraolo Spurio
---
.../gpu/drm/i915/display/intel_atomic_plane.c | 3 ++
drivers/gpu/drm/i915/display/intel_display.c | 5 +++
.../drm/i915/display/intel_display_types.h| 3 ++
.../drm/i915/display/skl_universal_plane.c| 32 +--
.../drm/i915/display/skl_universal_plane.h| 1 +
drivers/gpu/drm/i915/i915_reg.h | 1 +
6 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 7bfb26ca0bd0..7057077a2b71 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -394,6 +394,7 @@ int intel_plane_atomic_check(struct
intel_atomic_state *state,
intel_atomic_get_old_crtc_state(state, crtc);
struct intel_crtc_state *new_crtc_state =
intel_atomic_get_new_crtc_state(state, crtc);
+ const struct drm_framebuffer *fb = new_plane_state->hw.fb;
if (new_crtc_state && new_crtc_state->bigjoiner_slave) {
struct intel_plane *master_plane =
@@ -409,6 +410,8 @@ int intel_plane_atomic_check(struct
intel_atomic_state *state,
intel_plane_copy_uapi_to_hw_state(new_plane_state,
new_master_plane_state,
crtc);
+ new_plane_state->plane_decryption =
+ i915_gem_object_has_valid_protection(intel_fb_obj(fb));
new_plane_state->uapi.visible = false;
if (!new_crtc_state)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index a10e26380ef3..55ab2d0b92d8 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -9367,6 +9367,10 @@ static int intel_atomic_check_async(struct
intel_atomic_state *state)
drm_dbg_kms(>drm, "Color range cannot be
changed in async flip\n");
return -EINVAL;
}
+
+ /* plane decryption is allow to change only in synchronous flips
*/
+ if (old_plane_state->plane_decryption != new_plane_state-
plane_decryption)
+ return -EINVAL;
}
return 0;
@@ -12350,6 +12354,7 @@ static void readout_plane_state(struct
drm_i915_private *dev_priv)
crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
crtc_state = to_intel_crtc_state(crtc->base.state);
+ intel_plane_read_hw_decryption(plane_state);
We don't have real plane state readout anyway, so seems pointless.
intel_set_plane_visible(crtc_state, plane_state, visible);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index e2e707c4dff5..76b3bb64a36a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -617,6 +617,9 @@ struct intel_plane_state {
struct intel_fb_view view;
+ /* Plane pxp decryption state */
+ bool plane_decryption;
+
It's all about the plane, so the plane_ prefix is entirely redundant.
Could just call it "decrypt" I guess.
/* plane control register */
u32 ctl;
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 75d3ca3dbb37..74489217e580 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -17,6 +17,7 @@
#include "intel_sprite.h"
#include "skl_scaler.h"
#include "skl_universal_plane.h"
+#include "pxp/intel_pxp.h"
static
Re: [Intel-gfx] [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
On Wed, Apr 28, 2021 at 11:25:23AM +, Gupta, Anshuman wrote:
>
>
> > -Original Message-
> > From: Ville Syrjälä
> > Sent: Wednesday, April 28, 2021 12:25 AM
> > To: Gupta, Anshuman
> > Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo ;
> > Bommu, Krishnaiah ; Huang Sean Z
> > ; Gaurav, Kumar ;
> > Ceraolo Spurio, Daniele
> > Subject: Re: [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
> Thanks Ville for review, Below are some doubts with respect to pxp state.
> >
> > On Tue, Apr 27, 2021 at 04:13:11PM +0530, Anshuman Gupta wrote:
> > > 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:
> > > - Used gen fb obj user_flags instead gem_object_metadata. [Krishna]
> > >
> > > v3:
> > > - intel_pxp_gem_object_status() API changes.
> > >
> > > v4: use intel_pxp_is_active (Daniele)
> > >
> > > v5: rebase and use the new protected object status checker (Daniele)
> > >
> > > v6: used plane state for plane_decryption to handle async flip
> > > as suggested by Ville.
> > >
> > > Cc: Bommu Krishnaiah
> > > Cc: Huang Sean Z
> > > Cc: Gaurav Kumar
> > > Cc: Ville Syrjälä
> > > Signed-off-by: Anshuman Gupta
> > > Signed-off-by: Daniele Ceraolo Spurio
> > >
> > > ---
> > > .../gpu/drm/i915/display/intel_atomic_plane.c | 3 ++
> > > drivers/gpu/drm/i915/display/intel_display.c | 5 +++
> > > .../drm/i915/display/intel_display_types.h| 3 ++
> > > .../drm/i915/display/skl_universal_plane.c| 32 +--
> > > .../drm/i915/display/skl_universal_plane.h| 1 +
> > > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > > 6 files changed, 42 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > index 7bfb26ca0bd0..7057077a2b71 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > @@ -394,6 +394,7 @@ int intel_plane_atomic_check(struct
> > intel_atomic_state *state,
> > > intel_atomic_get_old_crtc_state(state, crtc);
> > > struct intel_crtc_state *new_crtc_state =
> > > intel_atomic_get_new_crtc_state(state, crtc);
> > > + const struct drm_framebuffer *fb = new_plane_state->hw.fb;
> > >
> > > if (new_crtc_state && new_crtc_state->bigjoiner_slave) {
> > > struct intel_plane *master_plane =
> > > @@ -409,6 +410,8 @@ int intel_plane_atomic_check(struct
> > intel_atomic_state *state,
> > > intel_plane_copy_uapi_to_hw_state(new_plane_state,
> > > new_master_plane_state,
> > > crtc);
> > > + new_plane_state->plane_decryption =
> > > + i915_gem_object_has_valid_protection(intel_fb_obj(fb));
> > >
> > > new_plane_state->uapi.visible = false;
> > > if (!new_crtc_state)
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index a10e26380ef3..55ab2d0b92d8 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -9367,6 +9367,10 @@ static int intel_atomic_check_async(struct
> > intel_atomic_state *state)
> > > drm_dbg_kms(>drm, "Color range cannot be
> > changed in async flip\n");
> > > return -EINVAL;
> > > }
> > > +
> > > + /* plane decryption is allow to change only in synchronous flips
> > */
> > > + if (old_plane_state->plane_decryption != new_plane_state-
> > >plane_decryption)
> > > + return -EINVAL;
> > > }
> > >
> > > return 0;
> > > @@ -12350,6 +12354,7 @@ static void readout_plane_state(struct
> > > drm_i915_private *dev_priv)
> > >
> > > crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> > > crtc_state = to_intel_crtc_state(crtc->base.state);
> > > + intel_plane_read_hw_decryption(plane_state);
> >
> > We don't have real plane state readout anyway, so seems pointless.
> >
> > >
> > > intel_set_plane_visible(crtc_state, plane_state, visible);
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index e2e707c4dff5..76b3bb64a36a 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -617,6 +617,9 @@ struct intel_plane_state {
> > >
> > > struct intel_fb_view view;
> > >
> > > + /* Plane pxp decryption state */
> > > + bool plane_decryption;
> > > +
> >
> > It's all about the plane, so the plane_ prefix is entirely redundant.
> > Could just call it "decrypt" I guess.
> >
> > > /* plane control register */
> > > u32 ctl;
Re: [Intel-gfx] [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
> -Original Message-
> From: Ville Syrjälä
> Sent: Wednesday, April 28, 2021 12:25 AM
> To: Gupta, Anshuman
> Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo ;
> Bommu, Krishnaiah ; Huang Sean Z
> ; Gaurav, Kumar ;
> Ceraolo Spurio, Daniele
> Subject: Re: [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
Thanks Ville for review, Below are some doubts with respect to pxp state.
>
> On Tue, Apr 27, 2021 at 04:13:11PM +0530, Anshuman Gupta wrote:
> > 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:
> > - Used gen fb obj user_flags instead gem_object_metadata. [Krishna]
> >
> > v3:
> > - intel_pxp_gem_object_status() API changes.
> >
> > v4: use intel_pxp_is_active (Daniele)
> >
> > v5: rebase and use the new protected object status checker (Daniele)
> >
> > v6: used plane state for plane_decryption to handle async flip
> > as suggested by Ville.
> >
> > Cc: Bommu Krishnaiah
> > Cc: Huang Sean Z
> > Cc: Gaurav Kumar
> > Cc: Ville Syrjälä
> > Signed-off-by: Anshuman Gupta
> > Signed-off-by: Daniele Ceraolo Spurio
> >
> > ---
> > .../gpu/drm/i915/display/intel_atomic_plane.c | 3 ++
> > drivers/gpu/drm/i915/display/intel_display.c | 5 +++
> > .../drm/i915/display/intel_display_types.h| 3 ++
> > .../drm/i915/display/skl_universal_plane.c| 32 +--
> > .../drm/i915/display/skl_universal_plane.h| 1 +
> > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > 6 files changed, 42 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > index 7bfb26ca0bd0..7057077a2b71 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > @@ -394,6 +394,7 @@ int intel_plane_atomic_check(struct
> intel_atomic_state *state,
> > intel_atomic_get_old_crtc_state(state, crtc);
> > struct intel_crtc_state *new_crtc_state =
> > intel_atomic_get_new_crtc_state(state, crtc);
> > + const struct drm_framebuffer *fb = new_plane_state->hw.fb;
> >
> > if (new_crtc_state && new_crtc_state->bigjoiner_slave) {
> > struct intel_plane *master_plane =
> > @@ -409,6 +410,8 @@ int intel_plane_atomic_check(struct
> intel_atomic_state *state,
> > intel_plane_copy_uapi_to_hw_state(new_plane_state,
> > new_master_plane_state,
> > crtc);
> > + new_plane_state->plane_decryption =
> > + i915_gem_object_has_valid_protection(intel_fb_obj(fb));
> >
> > new_plane_state->uapi.visible = false;
> > if (!new_crtc_state)
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index a10e26380ef3..55ab2d0b92d8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -9367,6 +9367,10 @@ static int intel_atomic_check_async(struct
> intel_atomic_state *state)
> > drm_dbg_kms(>drm, "Color range cannot be
> changed in async flip\n");
> > return -EINVAL;
> > }
> > +
> > + /* plane decryption is allow to change only in synchronous flips
> */
> > + if (old_plane_state->plane_decryption != new_plane_state-
> >plane_decryption)
> > + return -EINVAL;
> > }
> >
> > return 0;
> > @@ -12350,6 +12354,7 @@ static void readout_plane_state(struct
> > drm_i915_private *dev_priv)
> >
> > crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> > crtc_state = to_intel_crtc_state(crtc->base.state);
> > + intel_plane_read_hw_decryption(plane_state);
>
> We don't have real plane state readout anyway, so seems pointless.
>
> >
> > intel_set_plane_visible(crtc_state, plane_state, visible);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index e2e707c4dff5..76b3bb64a36a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -617,6 +617,9 @@ struct intel_plane_state {
> >
> > struct intel_fb_view view;
> >
> > + /* Plane pxp decryption state */
> > + bool plane_decryption;
> > +
>
> It's all about the plane, so the plane_ prefix is entirely redundant.
> Could just call it "decrypt" I guess.
>
> > /* plane control register */
> > u32 ctl;
> >
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index 75d3ca3dbb37..74489217e580 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++
Re: [Intel-gfx] [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
On Tue, Apr 27, 2021 at 04:13:11PM +0530, Anshuman Gupta wrote:
> 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:
> - Used gen fb obj user_flags instead gem_object_metadata. [Krishna]
>
> v3:
> - intel_pxp_gem_object_status() API changes.
>
> v4: use intel_pxp_is_active (Daniele)
>
> v5: rebase and use the new protected object status checker (Daniele)
>
> v6: used plane state for plane_decryption to handle async flip
> as suggested by Ville.
>
> Cc: Bommu Krishnaiah
> Cc: Huang Sean Z
> Cc: Gaurav Kumar
> Cc: Ville Syrjälä
> Signed-off-by: Anshuman Gupta
> Signed-off-by: Daniele Ceraolo Spurio
> ---
> .../gpu/drm/i915/display/intel_atomic_plane.c | 3 ++
> drivers/gpu/drm/i915/display/intel_display.c | 5 +++
> .../drm/i915/display/intel_display_types.h| 3 ++
> .../drm/i915/display/skl_universal_plane.c| 32 +--
> .../drm/i915/display/skl_universal_plane.h| 1 +
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> 6 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 7bfb26ca0bd0..7057077a2b71 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -394,6 +394,7 @@ int intel_plane_atomic_check(struct intel_atomic_state
> *state,
> intel_atomic_get_old_crtc_state(state, crtc);
> struct intel_crtc_state *new_crtc_state =
> intel_atomic_get_new_crtc_state(state, crtc);
> + const struct drm_framebuffer *fb = new_plane_state->hw.fb;
>
> if (new_crtc_state && new_crtc_state->bigjoiner_slave) {
> struct intel_plane *master_plane =
> @@ -409,6 +410,8 @@ int intel_plane_atomic_check(struct intel_atomic_state
> *state,
> intel_plane_copy_uapi_to_hw_state(new_plane_state,
> new_master_plane_state,
> crtc);
> + new_plane_state->plane_decryption =
> + i915_gem_object_has_valid_protection(intel_fb_obj(fb));
>
> new_plane_state->uapi.visible = false;
> if (!new_crtc_state)
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index a10e26380ef3..55ab2d0b92d8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -9367,6 +9367,10 @@ static int intel_atomic_check_async(struct
> intel_atomic_state *state)
> drm_dbg_kms(>drm, "Color range cannot be changed
> in async flip\n");
> return -EINVAL;
> }
> +
> + /* plane decryption is allow to change only in synchronous
> flips */
> + if (old_plane_state->plane_decryption !=
> new_plane_state->plane_decryption)
> + return -EINVAL;
> }
>
> return 0;
> @@ -12350,6 +12354,7 @@ static void readout_plane_state(struct
> drm_i915_private *dev_priv)
>
> crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> crtc_state = to_intel_crtc_state(crtc->base.state);
> + intel_plane_read_hw_decryption(plane_state);
We don't have real plane state readout anyway, so seems pointless.
>
> intel_set_plane_visible(crtc_state, plane_state, visible);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index e2e707c4dff5..76b3bb64a36a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -617,6 +617,9 @@ struct intel_plane_state {
>
> struct intel_fb_view view;
>
> + /* Plane pxp decryption state */
> + bool plane_decryption;
> +
It's all about the plane, so the plane_ prefix is entirely redundant.
Could just call it "decrypt" I guess.
> /* plane control register */
> u32 ctl;
>
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 75d3ca3dbb37..74489217e580 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -17,6 +17,7 @@
> #include "intel_sprite.h"
> #include "skl_scaler.h"
> #include "skl_universal_plane.h"
> +#include "pxp/intel_pxp.h"
>
> static const u32 skl_plane_formats[] = {
> DRM_FORMAT_C8,
> @@ -956,7 +957,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
[Intel-gfx] [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
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:
- Used gen fb obj user_flags instead gem_object_metadata. [Krishna]
v3:
- intel_pxp_gem_object_status() API changes.
v4: use intel_pxp_is_active (Daniele)
v5: rebase and use the new protected object status checker (Daniele)
v6: used plane state for plane_decryption to handle async flip
as suggested by Ville.
Cc: Bommu Krishnaiah
Cc: Huang Sean Z
Cc: Gaurav Kumar
Cc: Ville Syrjälä
Signed-off-by: Anshuman Gupta
Signed-off-by: Daniele Ceraolo Spurio
---
.../gpu/drm/i915/display/intel_atomic_plane.c | 3 ++
drivers/gpu/drm/i915/display/intel_display.c | 5 +++
.../drm/i915/display/intel_display_types.h| 3 ++
.../drm/i915/display/skl_universal_plane.c| 32 +--
.../drm/i915/display/skl_universal_plane.h| 1 +
drivers/gpu/drm/i915/i915_reg.h | 1 +
6 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 7bfb26ca0bd0..7057077a2b71 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -394,6 +394,7 @@ int intel_plane_atomic_check(struct intel_atomic_state
*state,
intel_atomic_get_old_crtc_state(state, crtc);
struct intel_crtc_state *new_crtc_state =
intel_atomic_get_new_crtc_state(state, crtc);
+ const struct drm_framebuffer *fb = new_plane_state->hw.fb;
if (new_crtc_state && new_crtc_state->bigjoiner_slave) {
struct intel_plane *master_plane =
@@ -409,6 +410,8 @@ int intel_plane_atomic_check(struct intel_atomic_state
*state,
intel_plane_copy_uapi_to_hw_state(new_plane_state,
new_master_plane_state,
crtc);
+ new_plane_state->plane_decryption =
+ i915_gem_object_has_valid_protection(intel_fb_obj(fb));
new_plane_state->uapi.visible = false;
if (!new_crtc_state)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index a10e26380ef3..55ab2d0b92d8 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -9367,6 +9367,10 @@ static int intel_atomic_check_async(struct
intel_atomic_state *state)
drm_dbg_kms(>drm, "Color range cannot be changed
in async flip\n");
return -EINVAL;
}
+
+ /* plane decryption is allow to change only in synchronous
flips */
+ if (old_plane_state->plane_decryption !=
new_plane_state->plane_decryption)
+ return -EINVAL;
}
return 0;
@@ -12350,6 +12354,7 @@ static void readout_plane_state(struct drm_i915_private
*dev_priv)
crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
crtc_state = to_intel_crtc_state(crtc->base.state);
+ intel_plane_read_hw_decryption(plane_state);
intel_set_plane_visible(crtc_state, plane_state, visible);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index e2e707c4dff5..76b3bb64a36a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -617,6 +617,9 @@ struct intel_plane_state {
struct intel_fb_view view;
+ /* Plane pxp decryption state */
+ bool plane_decryption;
+
/* plane control register */
u32 ctl;
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 75d3ca3dbb37..74489217e580 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -17,6 +17,7 @@
#include "intel_sprite.h"
#include "skl_scaler.h"
#include "skl_universal_plane.h"
+#include "pxp/intel_pxp.h"
static const u32 skl_plane_formats[] = {
DRM_FORMAT_C8,
@@ -956,7 +957,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);
@@ -1037,8 +1038,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),
-
Re: [Intel-gfx] [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
On Tue, Apr 20, 2021 at 10:48:08AM -0400, Rodrigo Vivi wrote:
> On Sun, Mar 28, 2021 at 03:57:06PM -0700, Daniele Ceraolo Spurio wrote:
> > From: Anshuman Gupta
> >
> > 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:
> > - Used gen fb obj user_flags instead gem_object_metadata. [Krishna]
> >
> > v3:
> > - intel_pxp_gem_object_status() API changes.
> >
> > v4: use intel_pxp_is_active (Daniele)
> > v5: rebase and use the new protected object status checker (Daniele)
> >
> > Cc: Bommu Krishnaiah
> > Cc: Huang Sean Z
> > Cc: Gaurav Kumar
> > Signed-off-by: Anshuman Gupta
> > Signed-off-by: Daniele Ceraolo Spurio
> > ---
> > drivers/gpu/drm/i915/display/skl_universal_plane.c | 14 +++---
> > drivers/gpu/drm/i915/i915_reg.h| 1 +
> > 2 files changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index c6d7b6c054b5..b21bfb5be876 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -16,6 +16,7 @@
> > #include "intel_sprite.h"
> > #include "skl_scaler.h"
> > #include "skl_universal_plane.h"
> > +#include "pxp/intel_pxp.h"
> >
> > static const u32 skl_plane_formats[] = {
> > DRM_FORMAT_C8,
> > @@ -971,7 +972,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);
> > @@ -1051,8 +1052,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_is_active(_priv->gt.pxp) &&
> > + i915_gem_object_has_valid_protection(intel_fb_obj(fb)))
> > + plane_surf |= PLANE_SURF_DECRYPTION_ENABLED;
> > + else
> > + plane_surf &= ~PLANE_SURF_DECRYPTION_ENABLED;
>
> This part looks right.
The &=~ thing is pointless.
> But what should we do about the async_flip path?
> Jani? Ville? Uma? thoughts?
Presumably it can't be actually changed by an async flip? So
probbly we want to track it in the plane_state and reject async
flips if it has changed.
>
> > +
> > + intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf);
> >
> > if (plane_state->scaler_id >= 0)
> > skl_program_plane_scaler(plane, crtc_state, plane_state);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 1fe42f4a4e4b..a0313d718905 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7234,6 +7234,7 @@ enum {
> > #define _PLANE_SURF_3(pipe)_PIPE(pipe, _PLANE_SURF_3_A,
> > _PLANE_SURF_3_B)
> > #define PLANE_SURF(pipe, plane)\
> > _MMIO_PLANE(plane, _PLANE_SURF_1(pipe), _PLANE_SURF_2(pipe))
> > +#define PLANE_SURF_DECRYPTION_ENABLEDREG_BIT(2)
> >
> > #define _PLANE_OFFSET_1_B 0x711a4
> > #define _PLANE_OFFSET_2_B 0x712a4
> > --
> > 2.29.2
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
On Sun, Mar 28, 2021 at 03:57:06PM -0700, Daniele Ceraolo Spurio wrote:
> From: Anshuman Gupta
>
> 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:
> - Used gen fb obj user_flags instead gem_object_metadata. [Krishna]
>
> v3:
> - intel_pxp_gem_object_status() API changes.
>
> v4: use intel_pxp_is_active (Daniele)
> v5: rebase and use the new protected object status checker (Daniele)
>
> Cc: Bommu Krishnaiah
> Cc: Huang Sean Z
> Cc: Gaurav Kumar
> Signed-off-by: Anshuman Gupta
> Signed-off-by: Daniele Ceraolo Spurio
> ---
> drivers/gpu/drm/i915/display/skl_universal_plane.c | 14 +++---
> drivers/gpu/drm/i915/i915_reg.h| 1 +
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index c6d7b6c054b5..b21bfb5be876 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -16,6 +16,7 @@
> #include "intel_sprite.h"
> #include "skl_scaler.h"
> #include "skl_universal_plane.h"
> +#include "pxp/intel_pxp.h"
>
> static const u32 skl_plane_formats[] = {
> DRM_FORMAT_C8,
> @@ -971,7 +972,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);
> @@ -1051,8 +1052,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_is_active(_priv->gt.pxp) &&
> + i915_gem_object_has_valid_protection(intel_fb_obj(fb)))
> + plane_surf |= PLANE_SURF_DECRYPTION_ENABLED;
> + else
> + plane_surf &= ~PLANE_SURF_DECRYPTION_ENABLED;
This part looks right. But what should we do about the async_flip path?
Jani? Ville? Uma? thoughts?
> +
> + intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf);
>
> if (plane_state->scaler_id >= 0)
> skl_program_plane_scaler(plane, crtc_state, plane_state);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1fe42f4a4e4b..a0313d718905 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7234,6 +7234,7 @@ enum {
> #define _PLANE_SURF_3(pipe) _PIPE(pipe, _PLANE_SURF_3_A, _PLANE_SURF_3_B)
> #define PLANE_SURF(pipe, plane) \
> _MMIO_PLANE(plane, _PLANE_SURF_1(pipe), _PLANE_SURF_2(pipe))
> +#define PLANE_SURF_DECRYPTION_ENABLED REG_BIT(2)
>
> #define _PLANE_OFFSET_1_B0x711a4
> #define _PLANE_OFFSET_2_B0x712a4
> --
> 2.29.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 14/16] drm/i915/pxp: Add plane decryption support
From: Anshuman Gupta
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:
- Used gen fb obj user_flags instead gem_object_metadata. [Krishna]
v3:
- intel_pxp_gem_object_status() API changes.
v4: use intel_pxp_is_active (Daniele)
v5: rebase and use the new protected object status checker (Daniele)
Cc: Bommu Krishnaiah
Cc: Huang Sean Z
Cc: Gaurav Kumar
Signed-off-by: Anshuman Gupta
Signed-off-by: Daniele Ceraolo Spurio
---
drivers/gpu/drm/i915/display/skl_universal_plane.c | 14 +++---
drivers/gpu/drm/i915/i915_reg.h| 1 +
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index c6d7b6c054b5..b21bfb5be876 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -16,6 +16,7 @@
#include "intel_sprite.h"
#include "skl_scaler.h"
#include "skl_universal_plane.h"
+#include "pxp/intel_pxp.h"
static const u32 skl_plane_formats[] = {
DRM_FORMAT_C8,
@@ -971,7 +972,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);
@@ -1051,8 +1052,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_is_active(_priv->gt.pxp) &&
+ i915_gem_object_has_valid_protection(intel_fb_obj(fb)))
+ plane_surf |= PLANE_SURF_DECRYPTION_ENABLED;
+ else
+ plane_surf &= ~PLANE_SURF_DECRYPTION_ENABLED;
+
+ intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf);
if (plane_state->scaler_id >= 0)
skl_program_plane_scaler(plane, crtc_state, plane_state);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1fe42f4a4e4b..a0313d718905 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7234,6 +7234,7 @@ enum {
#define _PLANE_SURF_3(pipe)_PIPE(pipe, _PLANE_SURF_3_A, _PLANE_SURF_3_B)
#define PLANE_SURF(pipe, plane)\
_MMIO_PLANE(plane, _PLANE_SURF_1(pipe), _PLANE_SURF_2(pipe))
+#define PLANE_SURF_DECRYPTION_ENABLEDREG_BIT(2)
#define _PLANE_OFFSET_1_B 0x711a4
#define _PLANE_OFFSET_2_B 0x712a4
--
2.29.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
