Re: [Intel-gfx] [PATCH v3 i-g-t] test/kms_plane_lowres: Fix display_commit_mode() so it returns the crc
On Tue, Dec 12, 2017 at 02:10:56PM +0200, Arkadiusz Hiler wrote: > On Tue, Dec 12, 2017 at 11:59:13AM +0100, Maarten Lankhorst wrote: > > From: Arkadiusz Hiler> > > > Compiler complained on crc_lowres and crc_hires2 being uninitialized > > and indeed, display_commit_mode() seems to have intention of retruning > > the value through the parameter that is only a single pointer. > > > > This couses only the local copy of the pointer, the one inside > > display_commit_mode(), to be overwritten. > > > > Let's fix that! > > > > Also add missing hires crc comparison (M. Kahola). > > > > v2: make display_commit_mode return just the last CRC > > v3: Don't do memory allocations, it's hard. (Maarten) > > > > Cc: Mika Kahola > > Cc: Maarten Lankhorst > > Signed-off-by: Arkadiusz Hiler > > Signed-off-by: Maarten Lankhorst > > --- > > So sorry, didn't like the memory allocations, hope this works.. else I'll > > commit v2.. > > > > tests/kms_plane_lowres.c | 25 - > > 1 file changed, 16 insertions(+), 9 deletions(-) > > > > diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c > > index 85d3145de4b6..f643bb4b0e8f 100644 > > --- a/tests/kms_plane_lowres.c > > +++ b/tests/kms_plane_lowres.c > > @@ -125,11 +125,12 @@ test_fini(data_t *data, igt_output_t *output, enum > > pipe pipe) > > data->fb = NULL; > > } > > > > -static int > > +static void > > display_commit_mode(igt_display_t *display, igt_pipe_crc_t *pipe_crc, > > - enum pipe pipe, int flags, igt_crc_t *crc) > > + enum pipe pipe, int flags, igt_crc_t *out_crc) > > { > > char buf[256]; > > + igt_crc_t *crcs; > > struct drm_event *e = (void *)buf; > > unsigned int vblank_start, vblank_stop; > > int n, ret; > > @@ -148,10 +149,12 @@ display_commit_mode(igt_display_t *display, > > igt_pipe_crc_t *pipe_crc, > > igt_assert_eq(e->type, DRM_EVENT_FLIP_COMPLETE); > > igt_reset_timeout(); > > > > - n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, ); > > + n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, ); > > igt_assert_eq(n, vblank_stop - vblank_start); > > > > - return n; > > + /* there is no need to return all the intermediary CRCs */ > > + /* so let's return just the last one */ > > + *out_crc = crcs[n-1]; > > free(crcs); ? With that fixed Reviewed-by: Arkadiusz Hiler ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 i-g-t] test/kms_plane_lowres: Fix display_commit_mode() so it returns the crc
On Tue, Dec 12, 2017 at 11:59:13AM +0100, Maarten Lankhorst wrote: > From: Arkadiusz Hiler> > Compiler complained on crc_lowres and crc_hires2 being uninitialized > and indeed, display_commit_mode() seems to have intention of retruning > the value through the parameter that is only a single pointer. > > This couses only the local copy of the pointer, the one inside > display_commit_mode(), to be overwritten. > > Let's fix that! > > Also add missing hires crc comparison (M. Kahola). > > v2: make display_commit_mode return just the last CRC > v3: Don't do memory allocations, it's hard. (Maarten) > > Cc: Mika Kahola > Cc: Maarten Lankhorst > Signed-off-by: Arkadiusz Hiler > Signed-off-by: Maarten Lankhorst > --- > So sorry, didn't like the memory allocations, hope this works.. else I'll > commit v2.. > > tests/kms_plane_lowres.c | 25 - > 1 file changed, 16 insertions(+), 9 deletions(-) > > diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c > index 85d3145de4b6..f643bb4b0e8f 100644 > --- a/tests/kms_plane_lowres.c > +++ b/tests/kms_plane_lowres.c > @@ -125,11 +125,12 @@ test_fini(data_t *data, igt_output_t *output, enum pipe > pipe) > data->fb = NULL; > } > > -static int > +static void > display_commit_mode(igt_display_t *display, igt_pipe_crc_t *pipe_crc, > - enum pipe pipe, int flags, igt_crc_t *crc) > + enum pipe pipe, int flags, igt_crc_t *out_crc) > { > char buf[256]; > + igt_crc_t *crcs; > struct drm_event *e = (void *)buf; > unsigned int vblank_start, vblank_stop; > int n, ret; > @@ -148,10 +149,12 @@ display_commit_mode(igt_display_t *display, > igt_pipe_crc_t *pipe_crc, > igt_assert_eq(e->type, DRM_EVENT_FLIP_COMPLETE); > igt_reset_timeout(); > > - n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, ); > + n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, ); > igt_assert_eq(n, vblank_stop - vblank_start); > > - return n; > + /* there is no need to return all the intermediary CRCs */ > + /* so let's return just the last one */ > + *out_crc = crcs[n-1]; free(crcs); ? > } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 i-g-t] test/kms_plane_lowres: Fix display_commit_mode() so it returns the crc
On Tue, Dec 12, 2017 at 11:59:13AM +0100, Maarten Lankhorst wrote: > From: Arkadiusz Hiler> > Compiler complained on crc_lowres and crc_hires2 being uninitialized > and indeed, display_commit_mode() seems to have intention of retruning ^^^ "returning" -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 i-g-t] test/kms_plane_lowres: Fix display_commit_mode() so it returns the crc
From: Arkadiusz HilerCompiler complained on crc_lowres and crc_hires2 being uninitialized and indeed, display_commit_mode() seems to have intention of retruning the value through the parameter that is only a single pointer. This couses only the local copy of the pointer, the one inside display_commit_mode(), to be overwritten. Let's fix that! Also add missing hires crc comparison (M. Kahola). v2: make display_commit_mode return just the last CRC v3: Don't do memory allocations, it's hard. (Maarten) Cc: Mika Kahola Cc: Maarten Lankhorst Signed-off-by: Arkadiusz Hiler Signed-off-by: Maarten Lankhorst --- So sorry, didn't like the memory allocations, hope this works.. else I'll commit v2.. tests/kms_plane_lowres.c | 25 - 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c index 85d3145de4b6..f643bb4b0e8f 100644 --- a/tests/kms_plane_lowres.c +++ b/tests/kms_plane_lowres.c @@ -125,11 +125,12 @@ test_fini(data_t *data, igt_output_t *output, enum pipe pipe) data->fb = NULL; } -static int +static void display_commit_mode(igt_display_t *display, igt_pipe_crc_t *pipe_crc, - enum pipe pipe, int flags, igt_crc_t *crc) + enum pipe pipe, int flags, igt_crc_t *out_crc) { char buf[256]; + igt_crc_t *crcs; struct drm_event *e = (void *)buf; unsigned int vblank_start, vblank_stop; int n, ret; @@ -148,10 +149,12 @@ display_commit_mode(igt_display_t *display, igt_pipe_crc_t *pipe_crc, igt_assert_eq(e->type, DRM_EVENT_FLIP_COMPLETE); igt_reset_timeout(); - n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, ); + n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, ); igt_assert_eq(n, vblank_stop - vblank_start); - return n; + /* there is no need to return all the intermediary CRCs */ + /* so let's return just the last one */ + *out_crc = crcs[n-1]; } static void @@ -218,8 +221,8 @@ static void test_plane_position_with_output(data_t *data, enum pipe pipe, igt_output_t *output, uint64_t modifier) { - igt_crc_t *crc_hires1, *crc_hires2; - igt_crc_t *crc_lowres; + igt_crc_t crc_hires1, crc_hires2, *crcs; + igt_crc_t crc_lowres; drmModeModeInfo mode_lowres; drmModeModeInfo *mode1, *mode2, *mode3; int ret, n; @@ -239,8 +242,10 @@ test_plane_position_with_output(data_t *data, enum pipe pipe, pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO); igt_pipe_crc_start(pipe_crc); - n = igt_pipe_crc_get_crcs(pipe_crc, 1, _hires1); + n = igt_pipe_crc_get_crcs(pipe_crc, 1, ); igt_assert_eq(1, n); + crc_hires1 = *crcs; + free(crcs); igt_assert_plane_visible(data->drm_fd, pipe, true); @@ -252,7 +257,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe, check_mode(_lowres, mode2); - display_commit_mode(>display, pipe_crc, pipe, flags, crc_lowres); + display_commit_mode(>display, pipe_crc, pipe, flags, _lowres); igt_assert_plane_visible(data->drm_fd, pipe, false); @@ -264,10 +269,12 @@ test_plane_position_with_output(data_t *data, enum pipe pipe, check_mode(mode1, mode3); - display_commit_mode(>display, pipe_crc, pipe, flags, crc_hires2); + display_commit_mode(>display, pipe_crc, pipe, flags, _hires2); igt_assert_plane_visible(data->drm_fd, pipe, true); + igt_assert_crc_equal(_hires1, _hires2); + igt_pipe_crc_stop(pipe_crc); igt_pipe_crc_free(pipe_crc); -- 2.15.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx