From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matt Roper Subject: Re: [PATCH i-g-t] kms_cursor_crc: Move igt_pipe_crc_{new, free} to init Date: Thu, 22 May 2014 10:06:37 -0700 Message-ID: <20140522170637.GM31484@intel.com> References: <1400777259-20091-1-git-send-email-matthew.d.roper@intel.com> <20140522170109.GY27580@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id E92A46EC8E for ; Thu, 22 May 2014 10:05:30 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140522170109.GY27580@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, May 22, 2014 at 08:01:09PM +0300, Ville Syrj=E4l=E4 wrote: > On Thu, May 22, 2014 at 09:47:39AM -0700, Matt Roper wrote: > > If a subtest fails, cleanup_crtc() never gets called. Currently that > > also causes igt_pipe_crc_free() to never be called, leading all > > subsequent subtests to also fail with -EBUSY at igt_pipe_crc_new(). > > Move the calls to igt_pipe_crc_{new,free} into igt_main so that > > we don't need to worry about closing and reopening the CRC > > filedescriptor for each subtest. > = > IIRC we can't call them at init because when you call igt_pipe_crc_new() > the pipe->port mapping has to be properly set up so that the auto crc > source will know what to do. > = > The subtest failure case is the reason why at the start of every subtest > we call igt_pipe_crc_free() before calling igt_pipe_crc_new(). Are you > saying that's not working as intended? Right, you're working on a fresh test_data structure for each call of run_test(), so you've already lost the test_data->pipe_crc pointer that you're trying to check for NULL there. I guess the right fix is to move the pipe_crc pointer up into data_t rather than test_data_t? Matt > = > > = > > Signed-off-by: Matt Roper > > --- > > tests/kms_cursor_crc.c | 21 ++++++++++++++------- > > 1 file changed, 14 insertions(+), 7 deletions(-) > > = > > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c > > index 06625ee..1b90baa 100644 > > --- a/tests/kms_cursor_crc.c > > +++ b/tests/kms_cursor_crc.c > > @@ -39,11 +39,14 @@ > > #define DRM_CAP_CURSOR_HEIGHT 0x9 > > #endif > > = > > +#define MAX_PIPES 3 > > + > > typedef struct { > > int drm_fd; > > igt_display_t display; > > struct igt_fb primary_fb; > > struct igt_fb fb; > > + igt_pipe_crc_t *pipe_crcs[MAX_PIPES]; > > } data_t; > > = > > typedef struct { > > @@ -251,12 +254,6 @@ static bool prepare_crtc(test_data_t *test_data, i= gt_output_t *output, > > = > > igt_display_commit(display); > > = > > - /* create the pipe_crc object for this pipe */ > > - if (test_data->pipe_crc) > > - igt_pipe_crc_free(test_data->pipe_crc); > > - > > - test_data->pipe_crc =3D igt_pipe_crc_new(test_data->pipe, > > - INTEL_PIPE_CRC_SOURCE_AUTO); > > if (!test_data->pipe_crc) { > > igt_info("auto crc not supported on this connector with pipe %i\n", > > test_data->pipe); > > @@ -289,7 +286,6 @@ static void cleanup_crtc(test_data_t *test_data, ig= t_output_t *output) > > igt_display_t *display =3D &data->display; > > igt_plane_t *primary; > > = > > - igt_pipe_crc_free(test_data->pipe_crc); > > test_data->pipe_crc =3D NULL; > > = > > igt_remove_fb(data->drm_fd, &data->primary_fb); > > @@ -315,6 +311,7 @@ static void run_test(data_t *data, void (*testfunc)= (test_data_t *), int cursor_w > > test_data.output =3D output; > > for (p =3D 0; p < igt_display_get_n_pipes(display); p++) { > > test_data.pipe =3D p; > > + test_data.pipe_crc =3D data->pipe_crcs[p]; > > = > > if (!prepare_crtc(&test_data, output, cursor_w, cursor_h)) > > continue; > > @@ -385,6 +382,7 @@ igt_main > > uint64_t cursor_width =3D 64, cursor_height =3D 64; > > data_t data =3D {}; > > int ret; > > + int p; > > = > > igt_skip_on_simulation(); > > = > > @@ -405,11 +403,20 @@ igt_main > > igt_require_pipe_crc(); > > = > > igt_display_init(&data.display, data.drm_fd); > > + > > + for (p =3D 0; p < igt_display_get_n_pipes(&data.display); p++) > > + data.pipe_crcs[p] =3D igt_pipe_crc_new(p, > > + INTEL_PIPE_CRC_SOURCE_AUTO); > > } > > = > > run_test_generic(&data, cursor_width); > > = > > igt_fixture { > > + for (p =3D 0; p < igt_display_get_n_pipes(&data.display); p++) { > > + if (data.pipe_crcs[p]) > > + igt_pipe_crc_free(data.pipe_crcs[p]); > > + } > > + > > igt_display_fini(&data.display); > > } > > } > > -- = > > 1.8.5.1 > > = > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > = > -- = > Ville Syrj=E4l=E4 > Intel OTC -- = Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795