[igt-dev] [PATCH i-g-t 17/17] tests/i915_pxp: CRC validation for display tests.

[Teres Alexis, Alan Previn]

On Fri, 2021-06-04 at 10:40 -0400, Rodrigo Vivi wrote:
> On Tue, May 18, 2021 at 03:33:44AM -0700, Alan Previn wrote:
> > From: Karthik B S <karthik.b.s at intel.com>
> > 
> > Added subtests to validate pxp using CRC validation.
> > 
> > Signed-off-by: Karthik B S <karthik.b.s at intel.com>
> Cc: Uma Shankar <uma.shankar at intel.com>
> 
> > ---
> >  tests/i915/gem_pxp.c | 286
> > ++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 283 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/i915/gem_pxp.c b/tests/i915/gem_pxp.c
> > index 179fb742..728c925a 100644
> > +		if (hdcp) {
> > +			/* Disable HDCP and collect CRC */
> > +			igt_output_set_prop_value(hdcp_output[0],
> > IGT_CONNECTOR_CONTENT_PROTECTION,
> > +						  CP_UNDESIRED);
> > +			igt_display_commit2(display, COMMIT_ATOMIC);
> > +			ret = wait_for_prop_value(hdcp_output[0],
> > CP_UNDESIRED,
> > +						  KERNEL_DISABLE_TIME_A
> > LLOWED_MSEC);
> > +			igt_assert_f(ret, "Content Protection not
> > cleared\n");
> > +
> > +			igt_plane_set_fb(plane, &protected_fb);
> > +			igt_fb_set_size(&protected_fb, plane, mode-
> > >hdisplay, mode->vdisplay);
> > +			igt_plane_set_size(plane, mode->hdisplay, mode-
> > >vdisplay);
> > +
> > +			igt_display_commit2(display, COMMIT_ATOMIC);
> > +			igt_pipe_crc_collect_crc(pipe_crc, &new_crc);
> > +			igt_assert_f(!igt_check_crc_equal(&ref_crc,
> > &new_crc),
> > +				    "CRC's expected to be different
> > after HDCP is disabled\n");
> 
> The CRC is at the pipe level, but I believe it is right to expect
> that
> after disabling the HDCP we will get different CRC.
> 
> +Uma to help me to confirm this.
> 
> But also, I'm asking myself if this test case here is really bringing
> any value to PXP itself. To me, it looks like it is only testing
> HDCP disable scenario... nothing to do with the PXP flow.
> 
> With the HDCP removed, or with Uma confirming my assumption is right
> and that I'm wrong about the value-added, feel free to use:
> 

If i recall our earlier conversations on test requirements, we wanted
to ensure that the HW had indeed changed the key or completely disabled
plane decryption in response to the teardown (i.e. disabling hdcp).
With that in mind, I thought pipe crc is not a reflection of hdcp
encryption state - i thought that was the transcoder CRC. So Pipe CRC
really does reflect the content (and, as such, plane decryption) unless
the plane is disabled or occluded by another plane (which is never the
case in this controlled IGT environment). But lets hear from Uma - i
maybe crc input-signal.

> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> 
> >