Re: [PATCH i-g-t] kms_cursor_crc: Move igt_pipe_crc_{new, free} to init

Matt Roper
2014-05-22T17:05:30Z

On Thu, May 22, 2014 at 08:01:09PM +0300, Ville Syrjälä 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 <matthew.d.roper@intel.com>
> > ---
> > 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, igt_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 = 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, igt_output_t *output)
> > igt_display_t *display = &data->display;
> > igt_plane_t *primary;
> >
> > - igt_pipe_crc_free(test_data->pipe_crc);
> > test_data->pipe_crc = 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 = output;
> > for (p = 0; p < igt_display_get_n_pipes(display); p++) {
> > test_data.pipe = p;
> > + test_data.pipe_crc = 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 = 64, cursor_height = 64;
> > data_t data = {};
> > 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 = 0; p < igt_display_get_n_pipes(&data.display); p++)
> > + data.pipe_crcs[p] = igt_pipe_crc_new(p,
> > + INTEL_PIPE_CRC_SOURCE_AUTO);
> > }
> >
> > run_test_generic(&data, cursor_width);
> >
> > igt_fixture {
> > + for (p = 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älä
> Intel OTC

--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795