[igt-dev] [PATCH i-g-t v2 1/2] tests/kms_properties: Add additional prperty test infrastructure

[igt-dev] [PATCH i-g-t v2 1/2] tests/kms_properties: Add additional prperty test infrastructure

[Radhakrishna Sripada]

We currently test the existimg properties by setting them with default value.
Add infrastructure to perform additional test on a desired property.

v2: Fix the strcmp logic

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 tests/kms_properties.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/tests/kms_properties.c b/tests/kms_properties.c
index 9548cf44017a..f5a86236f5c8 100644
--- a/tests/kms_properties.c
+++ b/tests/kms_properties.c
@@ -29,6 +29,13 @@
 #include <string.h>
 #include <time.h>
 
+struct additional_test {
+	char *name;
+	uint32_t obj_type;
+	void (*prop_test)(int fd, uint32_t id, uint32_t type, drmModePropertyPtr prop,
+			  uint32_t prop_id, uint64_t prop_value, bool atomic);
+};
+
 static void prepare_pipe(igt_display_t *display, enum pipe pipe, igt_output_t *output, struct igt_fb *fb)
 {
 	drmModeModeInfo *mode = igt_output_get_mode(output);
@@ -75,11 +82,27 @@ static bool ignore_property(uint32_t obj_type, uint32_t prop_flags,
 	return false;
 }
 
+static const struct additional_test property_functional_test[] = {};
+
+static bool has_additional_test_lookup(uint32_t obj_type, const char *name,
+				bool atomic, int *index)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(property_functional_test); i++)
+		if (property_functional_test[i].obj_type == obj_type &&
+		    !strcmp(name, property_functional_test[i].name)) {
+			*index = i;
+			return true;
+		}
+
+	return false;
+}
 static void test_properties(int fd, uint32_t type, uint32_t id, bool atomic)
 {
 	drmModeObjectPropertiesPtr props =
 		drmModeObjectGetProperties(fd, id, type);
-	int i, ret;
+	int i, j, ret;
 	drmModeAtomicReqPtr req = NULL;
 
 	igt_assert(props);
@@ -114,6 +137,9 @@ static void test_properties(int fd, uint32_t type, uint32_t id, bool atomic)
 			igt_assert_eq(ret, 0);
 		}
 
+		if (has_additional_test_lookup(type, prop->name, atomic, &j))
+			property_functional_test[j].prop_test(fd, id, type, prop, prop_id, prop_value, atomic);
+
 		drmModeFreeProperty(prop);
 	}
 
-- 
2.9.3

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v2 2/2] tests/kms_properties: Add functional test for "max bpc" property

[Radhakrishna Sripada]

Test the values in the range advertised by the "max bpc" property.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 tests/kms_properties.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/tests/kms_properties.c b/tests/kms_properties.c
index f5a86236f5c8..503a5c251e10 100644
--- a/tests/kms_properties.c
+++ b/tests/kms_properties.c
@@ -82,7 +82,37 @@ static bool ignore_property(uint32_t obj_type, uint32_t prop_flags,
 	return false;
 }
 
-static const struct additional_test property_functional_test[] = {};
+static void max_bpc_prop_test(int fd, uint32_t id, uint32_t type, drmModePropertyPtr prop,
+			      uint32_t prop_id, uint64_t prop_value, bool atomic)
+{
+	drmModeAtomicReqPtr req = NULL;
+	int i, ret;
+
+	if (atomic)
+		req = drmModeAtomicAlloc();
+
+	for ( i = prop->values[0]; i < prop->values[1] ; i++) {
+		if (!atomic) {
+			ret = drmModeObjectSetProperty(fd, id, type, prop_id, i);
+
+			igt_assert_eq(ret, 0);
+		} else {
+			ret = drmModeAtomicAddProperty(req, id, prop_id, i);
+			igt_assert(ret >= 0);
+
+			ret = drmModeAtomicCommit(fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
+			igt_assert_eq(ret, 0);
+		}
+	}
+
+	if (atomic)
+		drmModeAtomicFree(req);
+}
+
+static const struct additional_test property_functional_test[] = {
+									{"max bpc", DRM_MODE_OBJECT_CONNECTOR,
+									 max_bpc_prop_test},
+								 };
 
 static bool has_additional_test_lookup(uint32_t obj_type, const char *name,
 				bool atomic, int *index)
-- 
2.9.3

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/2] tests/kms_properties: Add additional prperty test infrastructure

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,v2,1/2] tests/kms_properties: Add additional prperty test infrastructure
URL   : https://patchwork.freedesktop.org/series/50899/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4973 -> IGTPW_1945 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50899/revisions/1/mbox/

== Known issues ==

  Here are the changes found in IGTPW_1945 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       PASS -> DMESG-WARN (fdo#107139, fdo#105128)

    igt@kms_chamelium@dp-edid-read:
      fi-kbl-7500u:       PASS -> FAIL (fdo#102672, fdo#103841)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362, fdo#103191)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     FAIL (fdo#103167) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    
  fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362


== Participating hosts (43 -> 41) ==

  Additional (3): fi-skl-guc fi-icl-u fi-gdg-551 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4673 -> IGTPW_1945

  CI_DRM_4973: 60ba18212b324d02e961232953f190612d7a6ca3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1945: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1945/
  IGT_4673: 54cb1aeb4e50dea9f3abae632e317875d147c4ab @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1945/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,v2,1/2] tests/kms_properties: Add additional prperty test infrastructure

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,v2,1/2] tests/kms_properties: Add additional prperty test infrastructure
URL   : https://patchwork.freedesktop.org/series/50899/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4673_full -> IGTPW_1945_full =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1945_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1945_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50899/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1945_full:

  === IGT changes ===

    ==== Warnings ====

    igt@perf_pmu@rc6:
      shard-kbl:          SKIP -> PASS

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in IGTPW_1945_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665, fdo#106023)

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-apl:          PASS -> FAIL (fdo#106641)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
      shard-apl:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_cursor_crc@cursor-256x256-random:
      shard-glk:          PASS -> FAIL (fdo#103232) +4
      shard-apl:          PASS -> FAIL (fdo#103232) +3

    igt@kms_cursor_crc@cursor-256x85-sliding:
      shard-kbl:          PASS -> FAIL (fdo#103232) +1

    igt@kms_cursor_crc@cursor-64x64-onscreen:
      shard-hsw:          PASS -> DMESG-FAIL (fdo#103232, fdo#102614)

    igt@kms_cursor_crc@cursor-64x64-suspend:
      shard-apl:          PASS -> FAIL (fdo#103191, fdo#103232) +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
      shard-kbl:          PASS -> FAIL (fdo#103167) +1
      shard-apl:          PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
      shard-glk:          PASS -> FAIL (fdo#103167) +3

    igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
      shard-kbl:          PASS -> FAIL (fdo#108145)
      shard-apl:          PASS -> FAIL (fdo#108145)

    igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
      shard-glk:          PASS -> FAIL (fdo#108145) +1

    igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
      shard-kbl:          NOTRUN -> FAIL (fdo#108146)

    igt@kms_universal_plane@universal-plane-pipe-b-functional:
      shard-apl:          PASS -> FAIL (fdo#103166)

    
    ==== Possible fixes ====

    igt@gem_exec_basic@readonly-bsd:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    igt@gem_mmap@bad-object:
      shard-apl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS

    igt@kms_addfb_basic@bo-too-small-due-to-tiling:
      shard-snb:          DMESG-WARN (fdo#107469) -> PASS +4

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-snb:          FAIL (fdo#106641) -> PASS

    igt@kms_chv_cursor_fail@pipe-c-128x128-right-edge:
      shard-glk:          INCOMPLETE (fdo#103359, k.org#198133) -> PASS

    igt@kms_color@pipe-c-legacy-gamma:
      shard-kbl:          FAIL (fdo#104782) -> PASS
      shard-apl:          FAIL (fdo#104782) -> PASS

    igt@kms_cursor_crc@cursor-128x128-random:
      shard-apl:          FAIL (fdo#103232) -> PASS +1

    igt@kms_cursor_crc@cursor-128x128-suspend:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_cursor_legacy@cursora-vs-flipa-toggle:
      shard-glk:          DMESG-WARN (fdo#105763, fdo#106538) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-cpu:
      shard-glk:          FAIL (fdo#103167) -> PASS +3

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
      shard-kbl:          FAIL (fdo#103167) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
      shard-apl:          FAIL (fdo#103167) -> PASS +2

    igt@kms_plane@plane-position-covered-pipe-c-planes:
      shard-apl:          FAIL (fdo#103166) -> PASS +2

    igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
      shard-glk:          FAIL (fdo#103166) -> PASS +6

    igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
      shard-kbl:          FAIL (fdo#103166) -> PASS

    igt@kms_setmode@basic:
      shard-kbl:          FAIL (fdo#99912) -> PASS

    igt@perf_pmu@rc6-runtime-pm:
      shard-apl:          FAIL (fdo#105010) -> PASS

    igt@perf_pmu@rc6-runtime-pm-long:
      shard-kbl:          FAIL (fdo#105010) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
  fdo#105010 https://bugs.freedesktop.org/show_bug.cgi?id=105010
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#107469 https://bugs.freedesktop.org/show_bug.cgi?id=107469
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108146 https://bugs.freedesktop.org/show_bug.cgi?id=108146
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (6 -> 5) ==

  Missing    (1): shard-skl 


== Build changes ==

    * IGT: IGT_4673 -> IGTPW_1945
    * Linux: CI_DRM_4958 -> CI_DRM_4973

  CI_DRM_4958: 9990e1665029dc2ef4a9c0632b8a2f516263e595 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4973: 60ba18212b324d02e961232953f190612d7a6ca3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1945: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1945/
  IGT_4673: 54cb1aeb4e50dea9f3abae632e317875d147c4ab @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1945/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_properties: Add functional test for "max bpc" property

[Rodrigo Vivi]

On Fri, Oct 12, 2018 at 12:18:46AM -0700, Radhakrishna Sripada wrote:
> Test the values in the range advertised by the "max bpc" property.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  tests/kms_properties.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/kms_properties.c b/tests/kms_properties.c
> index f5a86236f5c8..503a5c251e10 100644
> --- a/tests/kms_properties.c
> +++ b/tests/kms_properties.c
> @@ -82,7 +82,37 @@ static bool ignore_property(uint32_t obj_type, uint32_t prop_flags,
>  	return false;
>  }
>  
> -static const struct additional_test property_functional_test[] = {};
> +static void max_bpc_prop_test(int fd, uint32_t id, uint32_t type, drmModePropertyPtr prop,
> +			      uint32_t prop_id, uint64_t prop_value, bool atomic)
> +{
> +	drmModeAtomicReqPtr req = NULL;
> +	int i, ret;
> +
> +	if (atomic)
> +		req = drmModeAtomicAlloc();
> +
> +	for ( i = prop->values[0]; i < prop->values[1] ; i++) {
> +		if (!atomic) {
> +			ret = drmModeObjectSetProperty(fd, id, type, prop_id, i);
> +
> +			igt_assert_eq(ret, 0);
> +		} else {
> +			ret = drmModeAtomicAddProperty(req, id, prop_id, i);
> +			igt_assert(ret >= 0);
> +
> +			ret = drmModeAtomicCommit(fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> +			igt_assert_eq(ret, 0);

Should we read it back to see if it sticks?

or should we be checking with some debugfs to see all
conditions and to see if the max is getting really respected?

( really questions not a request ;) )

> +		}
> +	}
> +
> +	if (atomic)
> +		drmModeAtomicFree(req);
> +}
> +
> +static const struct additional_test property_functional_test[] = {
> +									{"max bpc", DRM_MODE_OBJECT_CONNECTOR,
> +									 max_bpc_prop_test},
> +								 };
>  
>  static bool has_additional_test_lookup(uint32_t obj_type, const char *name,
>  				bool atomic, int *index)
> -- 
> 2.9.3
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_properties: Add functional test for "max bpc" property

[Radhakrishna Sripada]

On Wed, Oct 24, 2018 at 03:45:35PM -0700, Rodrigo Vivi wrote:
> On Fri, Oct 12, 2018 at 12:18:46AM -0700, Radhakrishna Sripada wrote:
> > Test the values in the range advertised by the "max bpc" property.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > ---
> >  tests/kms_properties.c | 32 +++++++++++++++++++++++++++++++-
> >  1 file changed, 31 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/kms_properties.c b/tests/kms_properties.c
> > index f5a86236f5c8..503a5c251e10 100644
> > --- a/tests/kms_properties.c
> > +++ b/tests/kms_properties.c
> > @@ -82,7 +82,37 @@ static bool ignore_property(uint32_t obj_type, uint32_t prop_flags,
> >  	return false;
> >  }
> >  
> > -static const struct additional_test property_functional_test[] = {};
> > +static void max_bpc_prop_test(int fd, uint32_t id, uint32_t type, drmModePropertyPtr prop,
> > +			      uint32_t prop_id, uint64_t prop_value, bool atomic)
> > +{
> > +	drmModeAtomicReqPtr req = NULL;
> > +	int i, ret;
> > +
> > +	if (atomic)
> > +		req = drmModeAtomicAlloc();
> > +
> > +	for ( i = prop->values[0]; i < prop->values[1] ; i++) {
> > +		if (!atomic) {
> > +			ret = drmModeObjectSetProperty(fd, id, type, prop_id, i);
> > +
> > +			igt_assert_eq(ret, 0);
> > +		} else {
> > +			ret = drmModeAtomicAddProperty(req, id, prop_id, i);
> > +			igt_assert(ret >= 0);
> > +
> > +			ret = drmModeAtomicCommit(fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> > +			igt_assert_eq(ret, 0);
> 
> Should we read it back to see if it sticks?

Reading back the property to see if the value sticks IMO is validation 
of the drm_property infrastructure which is being done by the sanity sub tests.
> or should we be checking with some debugfs to see all
> conditions and to see if the max is getting really respected?
> 
We could do this additional check for this property. However it would involve 
obtaining the corresponding CRTC id and parsing the i915_display_info debugs 
file to get the appropriate bpp for the CRTC and make sure the bpp is less 
than 8 * prop->values[i] making the test bulky. Do you suggest a simpler way?

Thanks,
Radhakrishna(RK) Sripada
> ( really questions not a request ;) )
> 
> > +		}
> > +	}
> > +
> > +	if (atomic)
> > +		drmModeAtomicFree(req);
> > +}
> > +
> > +static const struct additional_test property_functional_test[] = {
> > +									{"max bpc", DRM_MODE_OBJECT_CONNECTOR,
> > +									 max_bpc_prop_test},
> > +								 };
> >  
> >  static bool has_additional_test_lookup(uint32_t obj_type, const char *name,
> >  				bool atomic, int *index)
> > -- 
> > 2.9.3
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_properties: Add functional test for "max bpc" property

[Rodrigo Vivi]

On Mon, Oct 29, 2018 at 04:23:18AM -0700, Radhakrishna Sripada wrote:
> On Wed, Oct 24, 2018 at 03:45:35PM -0700, Rodrigo Vivi wrote:
> > On Fri, Oct 12, 2018 at 12:18:46AM -0700, Radhakrishna Sripada wrote:
> > > Test the values in the range advertised by the "max bpc" property.
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > ---
> > >  tests/kms_properties.c | 32 +++++++++++++++++++++++++++++++-
> > >  1 file changed, 31 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/kms_properties.c b/tests/kms_properties.c
> > > index f5a86236f5c8..503a5c251e10 100644
> > > --- a/tests/kms_properties.c
> > > +++ b/tests/kms_properties.c
> > > @@ -82,7 +82,37 @@ static bool ignore_property(uint32_t obj_type, uint32_t prop_flags,
> > >  	return false;
> > >  }
> > >  
> > > -static const struct additional_test property_functional_test[] = {};
> > > +static void max_bpc_prop_test(int fd, uint32_t id, uint32_t type, drmModePropertyPtr prop,
> > > +			      uint32_t prop_id, uint64_t prop_value, bool atomic)
> > > +{
> > > +	drmModeAtomicReqPtr req = NULL;
> > > +	int i, ret;
> > > +
> > > +	if (atomic)
> > > +		req = drmModeAtomicAlloc();
> > > +
> > > +	for ( i = prop->values[0]; i < prop->values[1] ; i++) {
> > > +		if (!atomic) {
> > > +			ret = drmModeObjectSetProperty(fd, id, type, prop_id, i);
> > > +
> > > +			igt_assert_eq(ret, 0);
> > > +		} else {
> > > +			ret = drmModeAtomicAddProperty(req, id, prop_id, i);
> > > +			igt_assert(ret >= 0);
> > > +
> > > +			ret = drmModeAtomicCommit(fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> > > +			igt_assert_eq(ret, 0);
> > 
> > Should we read it back to see if it sticks?
> 
> Reading back the property to see if the value sticks IMO is validation 
> of the drm_property infrastructure which is being done by the sanity sub tests.

ahh cool...

> > or should we be checking with some debugfs to see all
> > conditions and to see if the max is getting really respected?
> > 
> We could do this additional check for this property. However it would involve 
> obtaining the corresponding CRTC id and parsing the i915_display_info debugs 
> file to get the appropriate bpp for the CRTC and make sure the bpp is less 
> than 8 * prop->values[i] making the test bulky. Do you suggest a simpler way?

probably a place where selftests could be useful for display?

I don't have the confidence that this test case is validating the feature
itself. We set the max, but we are not sure if the max gets respected.

But it is better to have at least this side of the validation.
For this reasons:

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

> 
> Thanks,
> Radhakrishna(RK) Sripada
> > ( really questions not a request ;) )
> > 
> > > +		}
> > > +	}
> > > +
> > > +	if (atomic)
> > > +		drmModeAtomicFree(req);
> > > +}
> > > +
> > > +static const struct additional_test property_functional_test[] = {
> > > +									{"max bpc", DRM_MODE_OBJECT_CONNECTOR,
> > > +									 max_bpc_prop_test},
> > > +								 };
> > >  
> > >  static bool has_additional_test_lookup(uint32_t obj_type, const char *name,
> > >  				bool atomic, int *index)
> > > -- 
> > > 2.9.3
> > > 
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_properties: Add functional test for "max bpc" property

[Daniel Vetter]

On Wed, Oct 31, 2018 at 12:41:44PM -0700, Rodrigo Vivi wrote:
> On Mon, Oct 29, 2018 at 04:23:18AM -0700, Radhakrishna Sripada wrote:
> > On Wed, Oct 24, 2018 at 03:45:35PM -0700, Rodrigo Vivi wrote:
> > > On Fri, Oct 12, 2018 at 12:18:46AM -0700, Radhakrishna Sripada wrote:
> > > > Test the values in the range advertised by the "max bpc" property.
> > > > 
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > ---
> > > >  tests/kms_properties.c | 32 +++++++++++++++++++++++++++++++-
> > > >  1 file changed, 31 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/tests/kms_properties.c b/tests/kms_properties.c
> > > > index f5a86236f5c8..503a5c251e10 100644
> > > > --- a/tests/kms_properties.c
> > > > +++ b/tests/kms_properties.c
> > > > @@ -82,7 +82,37 @@ static bool ignore_property(uint32_t obj_type, uint32_t prop_flags,
> > > >  	return false;
> > > >  }
> > > >  
> > > > -static const struct additional_test property_functional_test[] = {};
> > > > +static void max_bpc_prop_test(int fd, uint32_t id, uint32_t type, drmModePropertyPtr prop,
> > > > +			      uint32_t prop_id, uint64_t prop_value, bool atomic)
> > > > +{
> > > > +	drmModeAtomicReqPtr req = NULL;
> > > > +	int i, ret;
> > > > +
> > > > +	if (atomic)
> > > > +		req = drmModeAtomicAlloc();
> > > > +
> > > > +	for ( i = prop->values[0]; i < prop->values[1] ; i++) {
> > > > +		if (!atomic) {
> > > > +			ret = drmModeObjectSetProperty(fd, id, type, prop_id, i);
> > > > +
> > > > +			igt_assert_eq(ret, 0);
> > > > +		} else {
> > > > +			ret = drmModeAtomicAddProperty(req, id, prop_id, i);
> > > > +			igt_assert(ret >= 0);
> > > > +
> > > > +			ret = drmModeAtomicCommit(fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> > > > +			igt_assert_eq(ret, 0);
> > > 
> > > Should we read it back to see if it sticks?
> > 
> > Reading back the property to see if the value sticks IMO is validation 
> > of the drm_property infrastructure which is being done by the sanity sub tests.
> 
> ahh cool...
> 
> > > or should we be checking with some debugfs to see all
> > > conditions and to see if the max is getting really respected?
> > > 
> > We could do this additional check for this property. However it would involve 
> > obtaining the corresponding CRTC id and parsing the i915_display_info debugs 
> > file to get the appropriate bpp for the CRTC and make sure the bpp is less 
> > than 8 * prop->values[i] making the test bulky. Do you suggest a simpler way?
> 
> probably a place where selftests could be useful for display?
> 
> I don't have the confidence that this test case is validating the feature
> itself. We set the max, but we are not sure if the max gets respected.
> 
> But it is better to have at least this side of the validation.
> For this reasons:

Yeah for full end-to-end testing we'd need to ask chamelium what bpc it
receives. Or something like that.

But also: This test here at least exercises all the code, and with the hw
state readback and cross-check we do catch quiet a few bugs.
-Daniel

> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> > 
> > Thanks,
> > Radhakrishna(RK) Sripada
> > > ( really questions not a request ;) )
> > > 
> > > > +		}
> > > > +	}
> > > > +
> > > > +	if (atomic)
> > > > +		drmModeAtomicFree(req);
> > > > +}
> > > > +
> > > > +static const struct additional_test property_functional_test[] = {
> > > > +									{"max bpc", DRM_MODE_OBJECT_CONNECTOR,
> > > > +									 max_bpc_prop_test},
> > > > +								 };
> > > >  
> > > >  static bool has_additional_test_lookup(uint32_t obj_type, const char *name,
> > > >  				bool atomic, int *index)
> > > > -- 
> > > > 2.9.3
> > > > 
> > > > _______________________________________________
> > > > igt-dev mailing list
> > > > igt-dev@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_properties: Add functional test for "max bpc" property

[Rodrigo Vivi]

On Fri, Nov 02, 2018 at 10:32:21AM +0100, Daniel Vetter wrote:
> On Wed, Oct 31, 2018 at 12:41:44PM -0700, Rodrigo Vivi wrote:
> > On Mon, Oct 29, 2018 at 04:23:18AM -0700, Radhakrishna Sripada wrote:
> > > On Wed, Oct 24, 2018 at 03:45:35PM -0700, Rodrigo Vivi wrote:
> > > > On Fri, Oct 12, 2018 at 12:18:46AM -0700, Radhakrishna Sripada wrote:
> > > > > Test the values in the range advertised by the "max bpc" property.
> > > > > 
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > > > ---
> > > > >  tests/kms_properties.c | 32 +++++++++++++++++++++++++++++++-
> > > > >  1 file changed, 31 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/tests/kms_properties.c b/tests/kms_properties.c
> > > > > index f5a86236f5c8..503a5c251e10 100644
> > > > > --- a/tests/kms_properties.c
> > > > > +++ b/tests/kms_properties.c
> > > > > @@ -82,7 +82,37 @@ static bool ignore_property(uint32_t obj_type, uint32_t prop_flags,
> > > > >  	return false;
> > > > >  }
> > > > >  
> > > > > -static const struct additional_test property_functional_test[] = {};
> > > > > +static void max_bpc_prop_test(int fd, uint32_t id, uint32_t type, drmModePropertyPtr prop,
> > > > > +			      uint32_t prop_id, uint64_t prop_value, bool atomic)
> > > > > +{
> > > > > +	drmModeAtomicReqPtr req = NULL;
> > > > > +	int i, ret;
> > > > > +
> > > > > +	if (atomic)
> > > > > +		req = drmModeAtomicAlloc();
> > > > > +
> > > > > +	for ( i = prop->values[0]; i < prop->values[1] ; i++) {
> > > > > +		if (!atomic) {
> > > > > +			ret = drmModeObjectSetProperty(fd, id, type, prop_id, i);
> > > > > +
> > > > > +			igt_assert_eq(ret, 0);
> > > > > +		} else {
> > > > > +			ret = drmModeAtomicAddProperty(req, id, prop_id, i);
> > > > > +			igt_assert(ret >= 0);
> > > > > +
> > > > > +			ret = drmModeAtomicCommit(fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> > > > > +			igt_assert_eq(ret, 0);
> > > > 
> > > > Should we read it back to see if it sticks?
> > > 
> > > Reading back the property to see if the value sticks IMO is validation 
> > > of the drm_property infrastructure which is being done by the sanity sub tests.
> > 
> > ahh cool...
> > 
> > > > or should we be checking with some debugfs to see all
> > > > conditions and to see if the max is getting really respected?
> > > > 
> > > We could do this additional check for this property. However it would involve 
> > > obtaining the corresponding CRTC id and parsing the i915_display_info debugs 
> > > file to get the appropriate bpp for the CRTC and make sure the bpp is less 
> > > than 8 * prop->values[i] making the test bulky. Do you suggest a simpler way?
> > 
> > probably a place where selftests could be useful for display?
> > 
> > I don't have the confidence that this test case is validating the feature
> > itself. We set the max, but we are not sure if the max gets respected.
> > 
> > But it is better to have at least this side of the validation.
> > For this reasons:
> 
> Yeah for full end-to-end testing we'd need to ask chamelium what bpc it
> receives. Or something like that.
> 
> But also: This test here at least exercises all the code, and with the hw
> state readback and cross-check we do catch quiet a few bugs.

makes sense...

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

^ for the series...

and pushed.

> > 
> > > 
> > > Thanks,
> > > Radhakrishna(RK) Sripada
> > > > ( really questions not a request ;) )
> > > > 
> > > > > +		}
> > > > > +	}
> > > > > +
> > > > > +	if (atomic)
> > > > > +		drmModeAtomicFree(req);
> > > > > +}
> > > > > +
> > > > > +static const struct additional_test property_functional_test[] = {
> > > > > +									{"max bpc", DRM_MODE_OBJECT_CONNECTOR,
> > > > > +									 max_bpc_prop_test},
> > > > > +								 };
> > > > >  
> > > > >  static bool has_additional_test_lookup(uint32_t obj_type, const char *name,
> > > > >  				bool atomic, int *index)
> > > > > -- 
> > > > > 2.9.3
> > > > > 
> > > > > _______________________________________________
> > > > > igt-dev mailing list
> > > > > igt-dev@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev