From: "Hogander, Jouni" <jouni.hogander@intel.com>
To: "igt-dev@lists.freedesktop.org" <igt-dev@lists.freedesktop.org>,
"B, Jeevan" <jeevan.b@intel.com>
Subject: Re: [igt-dev] [PATCH i-g-t] tests/i915/kms_psr2_su: Cleanup
Date: Mon, 14 Nov 2022 09:00:20 +0000 [thread overview]
Message-ID: <9587b4d40abd9b6364bb4bea590494ac2150a114.camel@intel.com> (raw)
In-Reply-To: <20221111162409.15242-1-jeevan.b@intel.com>
On Fri, 2022-11-11 at 21:54 +0530, Jeevan B wrote:
> From: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>
> Sanitize the system state before starting the subtest.
I think this patch is doing much more than what is in your commit
message. Please explain what this patch is actually doing in commit
message. Also if you have some specific reason for that you could
mention that too.
>
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> ---
> tests/i915/kms_psr2_su.c | 112 +++++++++++++++----------------------
> --
> 1 file changed, 44 insertions(+), 68 deletions(-)
>
> diff --git a/tests/i915/kms_psr2_su.c b/tests/i915/kms_psr2_su.c
> index 84dc30c3..785dff30 100644
> --- a/tests/i915/kms_psr2_su.c
> +++ b/tests/i915/kms_psr2_su.c
> @@ -92,41 +92,20 @@ typedef struct {
> uint32_t screen_changes;
> } data_t;
>
> -static void setup_output(data_t *data)
> -{
> - igt_display_t *display = &data->display;
> - igt_output_t *output;
> - enum pipe pipe;
> -
> - for_each_pipe_with_valid_output(display, pipe, output) {
> - drmModeConnectorPtr c = output->config.connector;
> -
> - if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> - continue;
> -
> - igt_output_set_pipe(output, pipe);
> - data->output = output;
> - data->mode = igt_output_get_mode(output);
> -
> - return;
> - }
> -}
> -
> -static void display_init(data_t *data)
> -{
> - igt_display_require(&data->display, data->drm_fd);
> - setup_output(data);
> -}
> -
> static void display_fini(data_t *data)
> {
> igt_display_fini(&data->display);
> }
>
> -static void prepare(data_t *data, igt_output_t *output)
> +static void prepare(data_t *data, enum pipe pipe, igt_output_t
> *output)
> {
> igt_plane_t *primary;
> + igt_display_t *display = &data->display;
> +
> + igt_display_reset(display);
> + igt_output_set_pipe(output, pipe);
>
> + data->mode = igt_output_get_mode(output);
> /* all green frame */
> igt_create_color_fb(data->drm_fd,
> data->mode->hdisplay, data->mode-
> >vdisplay,
> @@ -243,13 +222,14 @@ static void run(data_t *data, igt_output_t
> *output)
> "No matching selective update blocks read from
> debugfs\n");
> }
>
> -static void cleanup(data_t *data, igt_output_t *output)
> +static void cleanup(data_t *data, enum pipe pipe, igt_output_t
> *output)
> {
> igt_plane_t *primary;
>
> primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> igt_plane_set_fb(primary, NULL);
> + igt_output_set_pipe(output, pipe);
> igt_display_commit2(&data->display, COMMIT_ATOMIC);
>
> if (data->op == PAGE_FLIP)
> @@ -262,18 +242,17 @@ static void cleanup(data_t *data, igt_output_t
> *output)
>
> static int check_psr2_support(data_t *data, enum pipe pipe)
> {
> - int status;
> + bool status = false;
>
> - igt_output_t *output;
> - igt_display_t *display = &data->display;
> + igt_output_t *output = data->output;
> + drmModeConnectorPtr c = data->output->config.connector;
>
> - igt_display_reset(display);
> - output = data->output;
> - igt_output_set_pipe(output, pipe);
> + if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> + return status;
>
> - prepare(data, output);
> + prepare(data, pipe, output);
> status = psr_wait_entry(data->debugfs_fd, PSR_MODE_2);
> - cleanup(data, output);
> + cleanup(data, pipe, output);
>
> return status;
> }
> @@ -282,10 +261,7 @@ igt_main
> {
> data_t data = {};
> enum pipe pipe;
> - int r, i;
> - igt_output_t *outputs[IGT_MAX_PIPES * IGT_MAX_PIPES];
> - int pipes[IGT_MAX_PIPES * IGT_MAX_PIPES];
> - int n_pipes = 0;
> + int r;
>
> igt_fixture {
> struct itimerspec interval;
> @@ -301,7 +277,8 @@ igt_main
> igt_require_f(intel_display_ver(intel_get_drm_devid(d
> ata.drm_fd)) < 13,
> "Registers used by this test do not
> work on display 13\n");
>
> - display_init(&data);
> + igt_display_require(&data.display, data.drm_fd);
> + igt_display_require_output(&data.display);
>
> /* Test if PSR2 can be enabled */
> igt_require_f(psr_enable(data.drm_fd,
> @@ -320,43 +297,42 @@ igt_main
> interval.it_interval.tv_sec =
> interval.it_value.tv_sec;
> r = timerfd_settime(data.change_screen_timerfd, 0,
> &interval, NULL);
> igt_require_f(r != -1, "Error setting timerfd\n");
> -
> - for_each_pipe_with_valid_output(&data.display, pipe,
> data.output) {
> - if (check_psr2_support(&data, pipe)) {
> - pipes[n_pipes] = pipe;
> - outputs[n_pipes] = data.output;
> - n_pipes++;
> - }
> - }
> }
>
> for (data.op = PAGE_FLIP; data.op < LAST; data.op++) {
> const uint32_t *format = formats[data.op];
> + igt_fixture {
> + if (data.op == FRONTBUFFER &&
> +
> intel_display_ver(intel_get_drm_devid(data.drm_fd)) >= 12) {
> + /*
> + * FIXME: Display 12+ platforms now
> have PSR2
> + * selective fetch enabled by default
> but we
> + * still can't properly handle
> frontbuffer
> + * rendering, so right it does full
> frame
> + * fetches at every frontbuffer
> rendering.
> + * So it is expected that this test
> will fail
> + * in display 12+ platform for now.
> + */
> + igt_info("PSR2 selective fetch is
> doing full frame fetches for frontbuffer rendering\n");
> + continue;
> + }
> + }
>
> while (*format != DRM_FORMAT_INVALID) {
> data.format = *format++;
> igt_describe("Test that selective update
> works when screen changes");
> igt_subtest_with_dynamic_f("%s-%s",
> op_str(data.op), igt_format_str(data.format)) {
> - for (i = 0; i < n_pipes; i++) {
> - igt_dynamic_f("pipe-%s-%s",
> kmstest_pipe_name(pipes[i]),
> -
> igt_output_name
> (outputs[i])) {
> -
> igt_output_set_pipe(out
> puts[i], pipes[i]);
> - if (data.op ==
> FRONTBUFFER &&
> -
> intel_display_ver(intel_get_drm_devid(data.drm_fd)) >= 12) {
> - /*
> - * FIXME:
> Display 12+ platforms now have PSR2
> - * selective
> fetch enabled by default but we
> - * still
> can't properly handle frontbuffer
> - * rendering,
> so right it does full frame
> - * fetches at
> every frontbuffer rendering.
> - * So it is
> expected that this test will fail
> - * in display
> 12+ platform for now.
> - */
> -
> igt_skip("PSR2
> selective fetch is doing full frame fetches for frontbuffer
> rendering\n");
> + for_each_pipe(&data.display, pipe) {
> + for_each_valid_output_on_pipe
> (&data.display, pipe, data.output) {
> + if
> (!check_psr2_support(&data, pipe))
> + continue;
> +
> + igt_dynamic_f("pipe-
> %s-%s", kmstest_pipe_name(pipe),
> + igt_o
> utput_name(data.output)) {
> + prepare(&data
> , pipe, data.output);
> + run(&data,
> data.output);
> + cleanup(&data
> , pipe, data.output);
> }
> - prepare(&data,
> outputs[i]);
> - run(&data,
> outputs[i]);
> - cleanup(&data,
> outputs[i]);
> }
> }
> }
prev parent reply other threads:[~2022-11-14 9:00 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-11 16:24 [igt-dev] [PATCH i-g-t] tests/i915/kms_psr2_su: Cleanup Jeevan B
2022-11-11 19:09 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
2022-11-14 9:00 ` Hogander, Jouni [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9587b4d40abd9b6364bb4bea590494ac2150a114.camel@intel.com \
--to=jouni.hogander@intel.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=jeevan.b@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox