| Message ID | 20210427104504.2720-1-anshuman.gupta@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Tue, Apr 27, 2021 at 04:15:04PM +0530, Anshuman Gupta wrote: > When protected sufaces has flipped and pxp session is disabled, > display black pixels by using plane color CTM correction. > > v2: > - Display black pixels in aysnc flip too. We can't change any of that with an async flip. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Gaurav Kumar <kumar.gaurav@intel.com> > Cc: Shankar Uma <uma.shankar@intel.com> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > --- > .../drm/i915/display/skl_universal_plane.c | 51 ++++++++++++++++++- > drivers/gpu/drm/i915/i915_reg.h | 46 +++++++++++++++++ > 2 files changed, 95 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c > index 74489217e580..a666b86df726 100644 > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > @@ -934,6 +934,33 @@ static u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, > return plane_color_ctl; > } > > +static void intel_load_plane_csc_black(struct intel_plane *intel_plane) > +{ > + struct drm_i915_private *dev_priv = to_i915(intel_plane->base.dev); > + enum pipe pipe = intel_plane->pipe; > + enum plane_id plane = intel_plane->id; > + u16 postoff = 0; > + > + drm_dbg_kms(&dev_priv->drm, "plane color CTM to black %s:%d\n", > + intel_plane->base.name, plane); > + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 0), 0); > + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 1), 0); > + > + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 2), 0); > + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 3), 0); > + > + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 4), 0); > + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 5), 0); > + > + intel_de_write_fw(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 0), 0); > + intel_de_write_fw(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 1), 0); > + intel_de_write_fw(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 2), 0); > + > + intel_de_write_fw(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 0), postoff); > + intel_de_write_fw(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 1), postoff); > + intel_de_write_fw(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 2), postoff); > +} > + > static void > skl_program_plane(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state, > @@ -1039,13 +1066,22 @@ skl_program_plane(struct intel_plane *plane, > */ > intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl); > plane_surf = intel_plane_ggtt_offset(plane_state) + surf_addr; > + plane_color_ctl = intel_de_read_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id)); > > if (intel_pxp_is_active(&dev_priv->gt.pxp) && > - plane_state->plane_decryption) > + plane_state->plane_decryption) { > plane_surf |= PLANE_SURF_DECRYPTION_ENABLED; > - else > + plane_color_ctl &= ~PLANE_COLOR_PLANE_CSC_ENABLE; > + } else if (plane_state->plane_decryption) { > + intel_load_plane_csc_black(plane); > + plane_color_ctl |= PLANE_COLOR_PLANE_CSC_ENABLE; > + } else { > plane_surf &= ~PLANE_SURF_DECRYPTION_ENABLED; > + plane_color_ctl &= ~PLANE_COLOR_PLANE_CSC_ENABLE; > + } > > + intel_de_write_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id), > + plane_color_ctl); > intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf); > > if (plane_state->scaler_id >= 0) > @@ -1066,6 +1102,7 @@ skl_plane_async_flip(struct intel_plane *plane, > enum pipe pipe = plane->pipe; > u32 surf_addr = plane_state->view.color_plane[0].offset; > u32 plane_ctl = plane_state->ctl; > + u32 plane_color_ctl = 0; > > plane_ctl |= skl_plane_ctl_crtc(crtc_state); > > @@ -1075,6 +1112,16 @@ skl_plane_async_flip(struct intel_plane *plane, > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl); > + > + if (!intel_pxp_is_active(&dev_priv->gt.pxp) && > + plane_state->plane_decryption) { > + plane_color_ctl = intel_de_read_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id)); > + intel_load_plane_csc_black(plane); > + plane_color_ctl |= PLANE_COLOR_PLANE_CSC_ENABLE; > + intel_de_write_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id), > + plane_color_ctl); > + } > + > intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), > intel_plane_ggtt_offset(plane_state) + surf_addr); > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index fbaf9199001d..0a4deca1098b 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7119,6 +7119,7 @@ enum { > #define _PLANE_COLOR_CTL_3_A 0x703CC /* GLK+ */ > #define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-ICL */ > #define PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE (1 << 28) > +#define PLANE_COLOR_PLANE_CSC_ENABLE (1 << 21) /* ICL+ */ > #define PLANE_COLOR_INPUT_CSC_ENABLE (1 << 20) /* ICL+ */ > #define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23) /* Pre-ICL */ > #define PLANE_COLOR_CSC_MODE_BYPASS (0 << 17) > @@ -11191,6 +11192,51 @@ enum skl_power_gate { > _PAL_PREC_MULTI_SEG_DATA_A, \ > _PAL_PREC_MULTI_SEG_DATA_B) > > +#define _MMIO_PLANE_GAMC(plane, i, a, b) _MMIO(_PIPE(plane, a, b) + (i) * 4) > + > +/* Plane CSC Registers */ > +#define _PLANE_CSC_RY_GY_1_A 0x70210 > +#define _PLANE_CSC_RY_GY_2_A 0x70310 > + > +#define _PLANE_CSC_RY_GY_1_B 0x71210 > +#define _PLANE_CSC_RY_GY_2_B 0x71310 > + > +#define _PLANE_CSC_RY_GY_1(pipe) _PIPE(pipe, _PLANE_CSC_RY_GY_1_A, \ > + _PLANE_CSC_RY_GY_1_B) > +#define _PLANE_CSC_RY_GY_2(pipe) _PIPE(pipe, _PLANE_INPUT_CSC_RY_GY_2_A, \ > + _PLANE_INPUT_CSC_RY_GY_2_B) > +#define PLANE_CSC_COEFF(pipe, plane, index) _MMIO_PLANE(plane, \ > + _PLANE_CSC_RY_GY_1(pipe) + (index) * 4, \ > + _PLANE_CSC_RY_GY_2(pipe) + (index) * 4) > + > +#define _PLANE_CSC_PREOFF_HI_1_A 0x70228 > +#define _PLANE_CSC_PREOFF_HI_2_A 0x70328 > + > +#define _PLANE_CSC_PREOFF_HI_1_B 0x71228 > +#define _PLANE_CSC_PREOFF_HI_2_B 0x71328 > + > +#define _PLANE_CSC_PREOFF_HI_1(pipe) _PIPE(pipe, _PLANE_CSC_PREOFF_HI_1_A, \ > + _PLANE_CSC_PREOFF_HI_1_B) > +#define _PLANE_CSC_PREOFF_HI_2(pipe) _PIPE(pipe, _PLANE_CSC_PREOFF_HI_2_A, \ > + _PLANE_CSC_PREOFF_HI_2_B) > +#define PLANE_CSC_PREOFF(pipe, plane, index) _MMIO_PLANE(plane, _PLANE_CSC_PREOFF_HI_1(pipe) + \ > + (index) * 4, _PLANE_CSC_PREOFF_HI_2(pipe) + \ > + (index) * 4) > + > +#define _PLANE_CSC_POSTOFF_HI_1_A 0x70234 > +#define _PLANE_CSC_POSTOFF_HI_2_A 0x70334 > + > +#define _PLANE_CSC_POSTOFF_HI_1_B 0x71234 > +#define _PLANE_CSC_POSTOFF_HI_2_B 0x71334 > + > +#define _PLANE_CSC_POSTOFF_HI_1(pipe) _PIPE(pipe, _PLANE_CSC_POSTOFF_HI_1_A, \ > + _PLANE_CSC_POSTOFF_HI_1_B) > +#define _PLANE_CSC_POSTOFF_HI_2(pipe) _PIPE(pipe, _PLANE_CSC_POSTOFF_HI_2_A, \ > + _PLANE_CSC_POSTOFF_HI_2_B) > +#define PLANE_CSC_POSTOFF(pipe, plane, index) _MMIO_PLANE(plane, _PLANE_CSC_POSTOFF_HI_1(pipe) + \ > + (index) * 4, _PLANE_CSC_POSTOFF_HI_2(pipe) + \ > + (index) * 4) > + > /* pipe CSC & degamma/gamma LUTs on CHV */ > #define _CGM_PIPE_A_CSC_COEFF01 (VLV_DISPLAY_BASE + 0x67900) > #define _CGM_PIPE_A_CSC_COEFF23 (VLV_DISPLAY_BASE + 0x67904) > -- > 2.26.2
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Wednesday, April 28, 2021 12:26 AM > To: Gupta, Anshuman <anshuman.gupta@intel.com> > Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo <rodrigo.vivi@intel.com>; > Gaurav, Kumar <kumar.gaurav@intel.com>; Shankar, Uma > <uma.shankar@intel.com>; Ceraolo Spurio, Daniele > <daniele.ceraolospurio@intel.com> > Subject: Re: [PATCH v3 15/16] drm/i915/pxp: black pixels on pxp disabled > > On Tue, Apr 27, 2021 at 04:15:04PM +0530, Anshuman Gupta wrote: > > When protected sufaces has flipped and pxp session is disabled, > > display black pixels by using plane color CTM correction. > > > > v2: > > - Display black pixels in aysnc flip too. > > We can't change any of that with an async flip. I was thinking of an scenario , when application flip the protected surfaces with synchronous flips and driver has enable the plane decryption, can application issue an intermediate async flip with protected surfaces afterwards ? If above is possible, is it possible to display black pixels in case of pxp session invalidation at the time of Plane commit? Thanks, Anshuman > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Gaurav Kumar <kumar.gaurav@intel.com> > > Cc: Shankar Uma <uma.shankar@intel.com> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > > Signed-off-by: Daniele Ceraolo Spurio > > <daniele.ceraolospurio@intel.com> > > --- > > .../drm/i915/display/skl_universal_plane.c | 51 ++++++++++++++++++- > > drivers/gpu/drm/i915/i915_reg.h | 46 +++++++++++++++++ > > 2 files changed, 95 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c > > b/drivers/gpu/drm/i915/display/skl_universal_plane.c > > index 74489217e580..a666b86df726 100644 > > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > > @@ -934,6 +934,33 @@ static u32 glk_plane_color_ctl(const struct > intel_crtc_state *crtc_state, > > return plane_color_ctl; > > } > > > > +static void intel_load_plane_csc_black(struct intel_plane > > +*intel_plane) { > > + struct drm_i915_private *dev_priv = to_i915(intel_plane->base.dev); > > + enum pipe pipe = intel_plane->pipe; > > + enum plane_id plane = intel_plane->id; > > + u16 postoff = 0; > > + > > + drm_dbg_kms(&dev_priv->drm, "plane color CTM to black %s:%d\n", > > + intel_plane->base.name, plane); > > + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 0), 0); > > + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 1), 0); > > + > > + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 2), 0); > > + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 3), 0); > > + > > + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 4), 0); > > + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 5), 0); > > + > > + intel_de_write_fw(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 0), 0); > > + intel_de_write_fw(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 1), 0); > > + intel_de_write_fw(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 2), 0); > > + > > + intel_de_write_fw(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 0), > postoff); > > + intel_de_write_fw(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 1), > postoff); > > + intel_de_write_fw(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 2), > > +postoff); } > > + > > static void > > skl_program_plane(struct intel_plane *plane, > > const struct intel_crtc_state *crtc_state, @@ -1039,13 > +1066,22 > > @@ skl_program_plane(struct intel_plane *plane, > > */ > > intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl); > > plane_surf = intel_plane_ggtt_offset(plane_state) + surf_addr; > > + plane_color_ctl = intel_de_read_fw(dev_priv, PLANE_COLOR_CTL(pipe, > > +plane_id)); > > > > if (intel_pxp_is_active(&dev_priv->gt.pxp) && > > - plane_state->plane_decryption) > > + plane_state->plane_decryption) { > > plane_surf |= PLANE_SURF_DECRYPTION_ENABLED; > > - else > > + plane_color_ctl &= ~PLANE_COLOR_PLANE_CSC_ENABLE; > > + } else if (plane_state->plane_decryption) { > > + intel_load_plane_csc_black(plane); > > + plane_color_ctl |= PLANE_COLOR_PLANE_CSC_ENABLE; > > + } else { > > plane_surf &= ~PLANE_SURF_DECRYPTION_ENABLED; > > + plane_color_ctl &= ~PLANE_COLOR_PLANE_CSC_ENABLE; > > + } > > > > + intel_de_write_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id), > > + plane_color_ctl); > > intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf); > > > > if (plane_state->scaler_id >= 0) > > @@ -1066,6 +1102,7 @@ skl_plane_async_flip(struct intel_plane *plane, > > enum pipe pipe = plane->pipe; > > u32 surf_addr = plane_state->view.color_plane[0].offset; > > u32 plane_ctl = plane_state->ctl; > > + u32 plane_color_ctl = 0; > > > > plane_ctl |= skl_plane_ctl_crtc(crtc_state); > > > > @@ -1075,6 +1112,16 @@ skl_plane_async_flip(struct intel_plane *plane, > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > > > intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl); > > + > > + if (!intel_pxp_is_active(&dev_priv->gt.pxp) && > > + plane_state->plane_decryption) { > > + plane_color_ctl = intel_de_read_fw(dev_priv, > PLANE_COLOR_CTL(pipe, plane_id)); > > + intel_load_plane_csc_black(plane); > > + plane_color_ctl |= PLANE_COLOR_PLANE_CSC_ENABLE; > > + intel_de_write_fw(dev_priv, PLANE_COLOR_CTL(pipe, > plane_id), > > + plane_color_ctl); > > + } > > + > > intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), > > intel_plane_ggtt_offset(plane_state) + surf_addr); > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h index fbaf9199001d..0a4deca1098b > > 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -7119,6 +7119,7 @@ enum { > > #define _PLANE_COLOR_CTL_3_A 0x703CC /* GLK+ */ > > #define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30) /* > Pre-ICL */ > > #define PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE (1 << 28) > > +#define PLANE_COLOR_PLANE_CSC_ENABLE (1 << > 21) /* ICL+ */ > > #define PLANE_COLOR_INPUT_CSC_ENABLE (1 << 20) /* ICL+ */ > > #define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23) /* Pre-ICL */ > > #define PLANE_COLOR_CSC_MODE_BYPASS (0 << 17) > > @@ -11191,6 +11192,51 @@ enum skl_power_gate { > > _PAL_PREC_MULTI_SEG_DATA_A, \ > > _PAL_PREC_MULTI_SEG_DATA_B) > > > > +#define _MMIO_PLANE_GAMC(plane, i, a, b) _MMIO(_PIPE(plane, a, b) + > > +(i) * 4) > > + > > +/* Plane CSC Registers */ > > +#define _PLANE_CSC_RY_GY_1_A 0x70210 > > +#define _PLANE_CSC_RY_GY_2_A 0x70310 > > + > > +#define _PLANE_CSC_RY_GY_1_B 0x71210 > > +#define _PLANE_CSC_RY_GY_2_B 0x71310 > > + > > +#define _PLANE_CSC_RY_GY_1(pipe) _PIPE(pipe, _PLANE_CSC_RY_GY_1_A, > \ > > + _PLANE_CSC_RY_GY_1_B) > > +#define _PLANE_CSC_RY_GY_2(pipe) _PIPE(pipe, > _PLANE_INPUT_CSC_RY_GY_2_A, \ > > + _PLANE_INPUT_CSC_RY_GY_2_B) > > +#define PLANE_CSC_COEFF(pipe, plane, index) _MMIO_PLANE(plane, > \ > > + > _PLANE_CSC_RY_GY_1(pipe) + (index) * 4, \ > > + > _PLANE_CSC_RY_GY_2(pipe) + (index) * 4) > > + > > +#define _PLANE_CSC_PREOFF_HI_1_A 0x70228 > > +#define _PLANE_CSC_PREOFF_HI_2_A 0x70328 > > + > > +#define _PLANE_CSC_PREOFF_HI_1_B 0x71228 > > +#define _PLANE_CSC_PREOFF_HI_2_B 0x71328 > > + > > +#define _PLANE_CSC_PREOFF_HI_1(pipe) _PIPE(pipe, > _PLANE_CSC_PREOFF_HI_1_A, \ > > + _PLANE_CSC_PREOFF_HI_1_B) > > +#define _PLANE_CSC_PREOFF_HI_2(pipe) _PIPE(pipe, > _PLANE_CSC_PREOFF_HI_2_A, \ > > + _PLANE_CSC_PREOFF_HI_2_B) > > +#define PLANE_CSC_PREOFF(pipe, plane, index) _MMIO_PLANE(plane, > _PLANE_CSC_PREOFF_HI_1(pipe) + \ > > + (index) * 4, > _PLANE_CSC_PREOFF_HI_2(pipe) + \ > > + (index) * 4) > > + > > +#define _PLANE_CSC_POSTOFF_HI_1_A 0x70234 > > +#define _PLANE_CSC_POSTOFF_HI_2_A 0x70334 > > + > > +#define _PLANE_CSC_POSTOFF_HI_1_B 0x71234 > > +#define _PLANE_CSC_POSTOFF_HI_2_B 0x71334 > > + > > +#define _PLANE_CSC_POSTOFF_HI_1(pipe) _PIPE(pipe, > _PLANE_CSC_POSTOFF_HI_1_A, \ > > + _PLANE_CSC_POSTOFF_HI_1_B) > > +#define _PLANE_CSC_POSTOFF_HI_2(pipe) _PIPE(pipe, > _PLANE_CSC_POSTOFF_HI_2_A, \ > > + _PLANE_CSC_POSTOFF_HI_2_B) > > +#define PLANE_CSC_POSTOFF(pipe, plane, index) _MMIO_PLANE(plane, > _PLANE_CSC_POSTOFF_HI_1(pipe) + \ > > + (index) * 4, > _PLANE_CSC_POSTOFF_HI_2(pipe) + \ > > + (index) * 4) > > + > > /* pipe CSC & degamma/gamma LUTs on CHV */ > > #define _CGM_PIPE_A_CSC_COEFF01 (VLV_DISPLAY_BASE + 0x67900) > > #define _CGM_PIPE_A_CSC_COEFF23 (VLV_DISPLAY_BASE + 0x67904) > > -- > > 2.26.2 > > -- > Ville Syrjälä > Intel
On Fri, Apr 30, 2021 at 07:12:53AM +0000, Gupta, Anshuman wrote: > > > > -----Original Message----- > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Sent: Wednesday, April 28, 2021 12:26 AM > > To: Gupta, Anshuman <anshuman.gupta@intel.com> > > Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo <rodrigo.vivi@intel.com>; > > Gaurav, Kumar <kumar.gaurav@intel.com>; Shankar, Uma > > <uma.shankar@intel.com>; Ceraolo Spurio, Daniele > > <daniele.ceraolospurio@intel.com> > > Subject: Re: [PATCH v3 15/16] drm/i915/pxp: black pixels on pxp disabled > > > > On Tue, Apr 27, 2021 at 04:15:04PM +0530, Anshuman Gupta wrote: > > > When protected sufaces has flipped and pxp session is disabled, > > > display black pixels by using plane color CTM correction. > > > > > > v2: > > > - Display black pixels in aysnc flip too. > > > > We can't change any of that with an async flip. > I was thinking of an scenario , when application flip the protected surfaces with synchronous flips > and driver has enable the plane decryption, can application issue an intermediate async flip with > protected surfaces afterwards ? > If above is possible, is it possible to display black pixels in case of pxp session invalidation at the time of > Plane commit? We'll just have to refuse the async flip if the session has been invalidated.
On Fri, Apr 30, 2021 at 03:55:28PM +0300, Ville Syrjälä wrote: > On Fri, Apr 30, 2021 at 07:12:53AM +0000, Gupta, Anshuman wrote: > > > > > > > -----Original Message----- > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Sent: Wednesday, April 28, 2021 12:26 AM > > > To: Gupta, Anshuman <anshuman.gupta@intel.com> > > > Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo <rodrigo.vivi@intel.com>; > > > Gaurav, Kumar <kumar.gaurav@intel.com>; Shankar, Uma > > > <uma.shankar@intel.com>; Ceraolo Spurio, Daniele > > > <daniele.ceraolospurio@intel.com> > > > Subject: Re: [PATCH v3 15/16] drm/i915/pxp: black pixels on pxp disabled > > > > > > On Tue, Apr 27, 2021 at 04:15:04PM +0530, Anshuman Gupta wrote: > > > > When protected sufaces has flipped and pxp session is disabled, > > > > display black pixels by using plane color CTM correction. > > > > > > > > v2: > > > > - Display black pixels in aysnc flip too. > > > > > > We can't change any of that with an async flip. > > I was thinking of an scenario , when application flip the protected surfaces with synchronous flips > > and driver has enable the plane decryption, can application issue an intermediate async flip with > > protected surfaces afterwards ? > > If above is possible, is it possible to display black pixels in case of pxp session invalidation at the time of > > Plane commit? > > We'll just have to refuse the async flip if the session has > been invalidated. This seems the simplest way... but the effect would be different right? We wouldn't get the desired blank screen, but frozen screen?! Any possibility of a blank screen on this scenario? > > -- > Ville Syrjälä > Intel > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, 2021-05-07 at 14:42 -0400, Rodrigo Vivi wrote: > > > On Fri, Apr 30, 2021 at 03:55:28PM +0300, Ville Syrjälä wrote: > > On Fri, Apr 30, 2021 at 07:12:53AM +0000, Gupta, Anshuman wrote: > > > > > > > -----Original Message----- > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Sent: Wednesday, April 28, 2021 12:26 AM > > > > To: Gupta, Anshuman <anshuman.gupta@intel.com> > > > > Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo < > > > > rodrigo.vivi@intel.com>; > > > > Gaurav, Kumar <kumar.gaurav@intel.com>; Shankar, Uma > > > > <uma.shankar@intel.com>; Ceraolo Spurio, Daniele > > > > <daniele.ceraolospurio@intel.com> > > > > Subject: Re: [PATCH v3 15/16] drm/i915/pxp: black pixels on pxp > > > > disabled > > > > > > > > On Tue, Apr 27, 2021 at 04:15:04PM +0530, Anshuman Gupta wrote: > > > > > When protected sufaces has flipped and pxp session is > > > > > disabled, > > > > > display black pixels by using plane color CTM correction. > > > > > > > > > > v2: > > > > > - Display black pixels in aysnc flip too. > > > > > > > > We can't change any of that with an async flip. > > > I was thinking of an scenario , when application flip the > > > protected surfaces with synchronous flips > > > and driver has enable the plane decryption, can application issue > > > an intermediate async flip with > > > protected surfaces afterwards ? > > > If above is possible, is it possible to display black pixels in > > > case of pxp session invalidation at the time of > > > Plane commit? > > > > We'll just have to refuse the async flip if the session has > > been invalidated. > > This seems the simplest way... but the effect would be different > right? > We wouldn't get the desired blank screen, but frozen screen?! > > Any possibility of a blank screen on this scenario? > Not sure if this opinion offers an option: I assume when we say "refuse the async flip", we mean return a failure on but dont change the HW state... this would mean the user observes a frozen-and-corrupted- looking screen since all buffers in the app's swapchain would be encrypted and invalid at once (the typical case) - including the current frontbuffer. Along the same lines, if the app + compositor was paused/idle with no async flips coming in momentarily, a pxp session invalidation event would then cause the same symptom. Perhaps we need a uevent drm-master can hook onto specifically for the pxp-teardown so that drm-master would be able to replace the current front buffer so long as the session hasnt been re-established. Although this might be big enough to be seperate patch series after this?
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c index 74489217e580..a666b86df726 100644 --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c @@ -934,6 +934,33 @@ static u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state, return plane_color_ctl; } +static void intel_load_plane_csc_black(struct intel_plane *intel_plane) +{ + struct drm_i915_private *dev_priv = to_i915(intel_plane->base.dev); + enum pipe pipe = intel_plane->pipe; + enum plane_id plane = intel_plane->id; + u16 postoff = 0; + + drm_dbg_kms(&dev_priv->drm, "plane color CTM to black %s:%d\n", + intel_plane->base.name, plane); + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 0), 0); + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 1), 0); + + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 2), 0); + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 3), 0); + + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 4), 0); + intel_de_write_fw(dev_priv, PLANE_CSC_COEFF(pipe, plane, 5), 0); + + intel_de_write_fw(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 0), 0); + intel_de_write_fw(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 1), 0); + intel_de_write_fw(dev_priv, PLANE_CSC_PREOFF(pipe, plane, 2), 0); + + intel_de_write_fw(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 0), postoff); + intel_de_write_fw(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 1), postoff); + intel_de_write_fw(dev_priv, PLANE_CSC_POSTOFF(pipe, plane, 2), postoff); +} + static void skl_program_plane(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, @@ -1039,13 +1066,22 @@ skl_program_plane(struct intel_plane *plane, */ intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl); plane_surf = intel_plane_ggtt_offset(plane_state) + surf_addr; + plane_color_ctl = intel_de_read_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id)); if (intel_pxp_is_active(&dev_priv->gt.pxp) && - plane_state->plane_decryption) + plane_state->plane_decryption) { plane_surf |= PLANE_SURF_DECRYPTION_ENABLED; - else + plane_color_ctl &= ~PLANE_COLOR_PLANE_CSC_ENABLE; + } else if (plane_state->plane_decryption) { + intel_load_plane_csc_black(plane); + plane_color_ctl |= PLANE_COLOR_PLANE_CSC_ENABLE; + } else { plane_surf &= ~PLANE_SURF_DECRYPTION_ENABLED; + plane_color_ctl &= ~PLANE_COLOR_PLANE_CSC_ENABLE; + } + intel_de_write_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id), + plane_color_ctl); intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf); if (plane_state->scaler_id >= 0) @@ -1066,6 +1102,7 @@ skl_plane_async_flip(struct intel_plane *plane, enum pipe pipe = plane->pipe; u32 surf_addr = plane_state->view.color_plane[0].offset; u32 plane_ctl = plane_state->ctl; + u32 plane_color_ctl = 0; plane_ctl |= skl_plane_ctl_crtc(crtc_state); @@ -1075,6 +1112,16 @@ skl_plane_async_flip(struct intel_plane *plane, spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl); + + if (!intel_pxp_is_active(&dev_priv->gt.pxp) && + plane_state->plane_decryption) { + plane_color_ctl = intel_de_read_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id)); + intel_load_plane_csc_black(plane); + plane_color_ctl |= PLANE_COLOR_PLANE_CSC_ENABLE; + intel_de_write_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id), + plane_color_ctl); + } + intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), intel_plane_ggtt_offset(plane_state) + surf_addr); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index fbaf9199001d..0a4deca1098b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7119,6 +7119,7 @@ enum { #define _PLANE_COLOR_CTL_3_A 0x703CC /* GLK+ */ #define PLANE_COLOR_PIPE_GAMMA_ENABLE (1 << 30) /* Pre-ICL */ #define PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE (1 << 28) +#define PLANE_COLOR_PLANE_CSC_ENABLE (1 << 21) /* ICL+ */ #define PLANE_COLOR_INPUT_CSC_ENABLE (1 << 20) /* ICL+ */ #define PLANE_COLOR_PIPE_CSC_ENABLE (1 << 23) /* Pre-ICL */ #define PLANE_COLOR_CSC_MODE_BYPASS (0 << 17) @@ -11191,6 +11192,51 @@ enum skl_power_gate { _PAL_PREC_MULTI_SEG_DATA_A, \ _PAL_PREC_MULTI_SEG_DATA_B) +#define _MMIO_PLANE_GAMC(plane, i, a, b) _MMIO(_PIPE(plane, a, b) + (i) * 4) + +/* Plane CSC Registers */ +#define _PLANE_CSC_RY_GY_1_A 0x70210 +#define _PLANE_CSC_RY_GY_2_A 0x70310 + +#define _PLANE_CSC_RY_GY_1_B 0x71210 +#define _PLANE_CSC_RY_GY_2_B 0x71310 + +#define _PLANE_CSC_RY_GY_1(pipe) _PIPE(pipe, _PLANE_CSC_RY_GY_1_A, \ + _PLANE_CSC_RY_GY_1_B) +#define _PLANE_CSC_RY_GY_2(pipe) _PIPE(pipe, _PLANE_INPUT_CSC_RY_GY_2_A, \ + _PLANE_INPUT_CSC_RY_GY_2_B) +#define PLANE_CSC_COEFF(pipe, plane, index) _MMIO_PLANE(plane, \ + _PLANE_CSC_RY_GY_1(pipe) + (index) * 4, \ + _PLANE_CSC_RY_GY_2(pipe) + (index) * 4) + +#define _PLANE_CSC_PREOFF_HI_1_A 0x70228 +#define _PLANE_CSC_PREOFF_HI_2_A 0x70328 + +#define _PLANE_CSC_PREOFF_HI_1_B 0x71228 +#define _PLANE_CSC_PREOFF_HI_2_B 0x71328 + +#define _PLANE_CSC_PREOFF_HI_1(pipe) _PIPE(pipe, _PLANE_CSC_PREOFF_HI_1_A, \ + _PLANE_CSC_PREOFF_HI_1_B) +#define _PLANE_CSC_PREOFF_HI_2(pipe) _PIPE(pipe, _PLANE_CSC_PREOFF_HI_2_A, \ + _PLANE_CSC_PREOFF_HI_2_B) +#define PLANE_CSC_PREOFF(pipe, plane, index) _MMIO_PLANE(plane, _PLANE_CSC_PREOFF_HI_1(pipe) + \ + (index) * 4, _PLANE_CSC_PREOFF_HI_2(pipe) + \ + (index) * 4) + +#define _PLANE_CSC_POSTOFF_HI_1_A 0x70234 +#define _PLANE_CSC_POSTOFF_HI_2_A 0x70334 + +#define _PLANE_CSC_POSTOFF_HI_1_B 0x71234 +#define _PLANE_CSC_POSTOFF_HI_2_B 0x71334 + +#define _PLANE_CSC_POSTOFF_HI_1(pipe) _PIPE(pipe, _PLANE_CSC_POSTOFF_HI_1_A, \ + _PLANE_CSC_POSTOFF_HI_1_B) +#define _PLANE_CSC_POSTOFF_HI_2(pipe) _PIPE(pipe, _PLANE_CSC_POSTOFF_HI_2_A, \ + _PLANE_CSC_POSTOFF_HI_2_B) +#define PLANE_CSC_POSTOFF(pipe, plane, index) _MMIO_PLANE(plane, _PLANE_CSC_POSTOFF_HI_1(pipe) + \ + (index) * 4, _PLANE_CSC_POSTOFF_HI_2(pipe) + \ + (index) * 4) + /* pipe CSC & degamma/gamma LUTs on CHV */ #define _CGM_PIPE_A_CSC_COEFF01 (VLV_DISPLAY_BASE + 0x67900) #define _CGM_PIPE_A_CSC_COEFF23 (VLV_DISPLAY_BASE + 0x67904)