From: Mika Kahola <mika.kahola@intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH i-g-t v4 4/7] lib/igt_kms: Rework connector properties to be more atomic, v2.
Date: Mon, 02 Oct 2017 15:22:05 +0300 [thread overview]
Message-ID: <1506946925.3274.26.camel@intel.com> (raw)
In-Reply-To: <20170929095937.15702-5-maarten.lankhorst@linux.intel.com>
On Fri, 2017-09-29 at 11:59 +0200, Maarten Lankhorst wrote:
> In the future I want to allow tests to commit more properties,
> but for this to work I have to fix all properties to work better
> with atomic commit. Instead of special casing each
> property make a bitmask for all property changed flags, and try to
> commit all properties.
>
> Changs since v1:
> - Mention which properties we set to what.
> - Assert the property to be set is valid.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Mika Kahola <mika.kahola@intel.com>
> ---
> lib/igt_kms.c | 44 +++++++++++++++++++-----------
> ----------
> lib/igt_kms.h | 35 +++++++++++++++++++-----------
> --
> tests/kms_atomic_interruptible.c | 4 ++--
> tests/kms_panel_fitting.c | 2 +-
> 4 files changed, 45 insertions(+), 40 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index d25090b05c70..07d2074c2b1a 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -248,7 +248,7 @@ igt_atomic_fill_connector_props(igt_display_t
> *display, igt_output_t *output,
> if (strcmp(prop->name, conn_prop_names[j])
> != 0)
> continue;
>
> - output->config.atomic_props_connector[j] =
> props->props[i];
> + output->props[j] = props->props[i];
> break;
> }
>
> @@ -1834,7 +1834,7 @@ void igt_display_init(igt_display_t *display,
> int drm_fd)
>
> igt_output_refresh(output);
>
> - output->config.pipe_changed = true;
> + igt_output_set_prop_changed(output,
> IGT_CONNECTOR_CRTC_ID);
> }
>
> drmModeFreePlaneResources(plane_resources);
> @@ -2514,23 +2514,24 @@ static void
> igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj, drmModeAtomicRe
> static void igt_atomic_prepare_connector_commit(igt_output_t
> *output, drmModeAtomicReq *req)
> {
>
> - struct kmstest_connector_config *config = &output->config;
> + int i;
>
> - if (config->connector_scaling_mode_changed)
> - igt_atomic_populate_connector_req(req, output,
> IGT_CONNECTOR_SCALING_MODE, config->connector_scaling_mode);
> + for (i = 0; i < IGT_NUM_CONNECTOR_PROPS; i++) {
> + if (!igt_output_is_prop_changed(output, i))
> + continue;
>
> - if (config->pipe_changed) {
> - uint32_t crtc_id = 0;
> + /* it's an error to try an unsupported feature */
> + igt_assert(output->props[i]);
>
> - if (output->config.pipe != PIPE_NONE)
> - crtc_id = output->config.crtc->crtc_id;
> + igt_debug("%s: Setting property \"%s\" to
> 0x%"PRIx64"/%"PRIi64"\n",
> + igt_output_name(output),
> igt_connector_prop_names[i],
> + output->values[i], output->values[i]);
>
> - igt_atomic_populate_connector_req(req, output,
> IGT_CONNECTOR_CRTC_ID, crtc_id);
> + igt_assert_lt(0, drmModeAtomicAddProperty(req,
> + output->config.connector-
> >connector_id,
> + output->props[i],
> + output->values[i]));
> }
> - /*
> - * TODO: Add all other connector level properties
> here
> - */
> -
> }
>
> /*
> @@ -2625,11 +2626,10 @@ display_commit_changed(igt_display_t
> *display, enum igt_commit_style s)
> for (i = 0; i < display->n_outputs; i++) {
> igt_output_t *output = &display->outputs[i];
>
> - if (s != COMMIT_UNIVERSAL)
> - output->config.pipe_changed = false;
> -
> if (s == COMMIT_ATOMIC)
> - output-
> >config.connector_scaling_mode_changed = false;
> + output->changed = 0;
> + else if (s != COMMIT_UNIVERSAL)
> + igt_output_clear_prop_changed(output,
> IGT_CONNECTOR_CRTC_ID);
> }
> }
>
> @@ -2873,18 +2873,16 @@ void igt_output_set_pipe(igt_output_t
> *output, enum pipe pipe)
> if (pipe != PIPE_NONE)
> display->pipes[pipe].mode_changed = true;
>
> - output->config.pipe_changed = true;
> + igt_output_set_prop_value(output, IGT_CONNECTOR_CRTC_ID,
> pipe == PIPE_NONE ? 0 : display->pipes[pipe].crtc_id);
>
> igt_output_refresh(output);
> }
>
> void igt_output_set_scaling_mode(igt_output_t *output, uint64_t
> scaling_mode)
> {
> - output->config.connector_scaling_mode_changed = true;
> -
> - output->config.connector_scaling_mode = scaling_mode;
> + igt_output_set_prop_value(output,
> IGT_CONNECTOR_SCALING_MODE, scaling_mode);
>
> - igt_require(output-
> >config.atomic_props_connector[IGT_CONNECTOR_SCALING_MODE]);
> + igt_require(output->props[IGT_CONNECTOR_SCALING_MODE]);
> }
>
> igt_plane_t *igt_output_get_plane(igt_output_t *output, int
> plane_idx)
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 8dc118c961b7..1ef10e7d525c 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -131,10 +131,7 @@ struct kmstest_connector_config {
> drmModeConnector *connector;
> drmModeEncoder *encoder;
> drmModeModeInfo default_mode;
> - uint64_t connector_scaling_mode;
> - bool connector_scaling_mode_changed;
> - bool pipe_changed;
> - uint32_t atomic_props_connector[IGT_NUM_CONNECTOR_PROPS];
> +
> int pipe;
> unsigned valid_crtc_idx_mask;
> };
> @@ -364,6 +361,12 @@ typedef struct {
> enum pipe pending_pipe;
> bool use_override_mode;
> drmModeModeInfo override_mode;
> +
> + /* bitmask of changed properties */
> + uint64_t changed;
> +
> + uint32_t props[IGT_NUM_CONNECTOR_PROPS];
> + uint64_t values[IGT_NUM_CONNECTOR_PROPS];
> } igt_output_t;
>
> struct igt_display {
> @@ -545,16 +548,20 @@ static inline bool
> igt_output_is_connected(igt_output_t *output)
> #define igt_atomic_populate_crtc_req(req, pipe, prop, value) \
> igt_assert_lt(0, drmModeAtomicAddProperty(req, pipe-
> >crtc_id,\
> pipe-
> >atomic_props_crtc[prop], value))
> -/**
> - * igt_atomic_populate_connector_req:
> - * @req: A pointer to drmModeAtomicReq
> - * @output: A pointer igt_output_t
> - * @prop: one of igt_atomic_connector_properties
> - * @value: the value to add
> - */
> -#define igt_atomic_populate_connector_req(req, output, prop, value)
> \
> - igt_assert_lt(0, drmModeAtomicAddProperty(req, output-
> >config.connector->connector_id,\
> - output-
> >config.atomic_props_connector[prop], value))
> +
> +#define igt_output_is_prop_changed(output, prop) \
> + (!!((output)->changed & (1 << (prop))))
> +#define igt_output_set_prop_changed(output, prop) \
> + (output)->changed |= 1 << (prop)
> +
> +#define igt_output_clear_prop_changed(output, prop) \
> + (output)->changed &= ~(1 << (prop))
> +
> +#define igt_output_set_prop_value(output, prop, value) \
> + do { \
> + (output)->values[prop] = (value); \
> + igt_output_set_prop_changed(output, prop); \
> + } while (0)
>
> /*
> * igt_pipe_refresh:
> diff --git a/tests/kms_atomic_interruptible.c
> b/tests/kms_atomic_interruptible.c
> index 4e06ee4e2d6b..2d19fe967809 100644
> --- a/tests/kms_atomic_interruptible.c
> +++ b/tests/kms_atomic_interruptible.c
> @@ -159,7 +159,7 @@ static void run_plane_test(igt_display_t
> *display, enum pipe pipe, igt_output_t
> plane->pipe-
> >atomic_props_crtc[IGT_CRTC_MODE_ID],
> plane->pipe-
> >atomic_props_crtc[IGT_CRTC_ACTIVE],
> /* connector: 1 prop */
> - output-
> >config.atomic_props_connector[IGT_CONNECTOR_CRTC_ID],
> + output-
> >props[IGT_CONNECTOR_CRTC_ID],
> /* plane: remainder props */
> plane-
> >atomic_props_plane[IGT_PLANE_CRTC_ID],
> plane-
> >atomic_props_plane[IGT_PLANE_FB_ID],
> @@ -204,7 +204,7 @@ static void run_plane_test(igt_display_t
> *display, enum pipe pipe, igt_output_t
> case test_legacy_dpms: {
> struct
> drm_mode_connector_set_property prop = {
> .value = DRM_MODE_DPMS_OFF,
> - .prop_id = output-
> >config.atomic_props_connector[IGT_CONNECTOR_DPMS],
> + .prop_id = output-
> >props[IGT_CONNECTOR_DPMS],
> .connector_id = output->id,
> };
>
> diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
> index 85a231e60ea2..e4ea355611c3 100644
> --- a/tests/kms_panel_fitting.c
> +++ b/tests/kms_panel_fitting.c
> @@ -275,7 +275,7 @@ static void test_atomic_fastset(igt_display_t
> *display)
> igt_require(intel_gen(intel_get_drm_devid(display->drm_fd))
> >= 5);
>
> for_each_pipe_with_valid_output(display, pipe, output) {
> - if (!output-
> >config.atomic_props_connector[IGT_CONNECTOR_SCALING_MODE])
> + if (!output->props[IGT_CONNECTOR_SCALING_MODE])
> continue;
>
> test_panel_fitting_fastset(display, pipe, output);
--
Mika Kahola - Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2017-10-02 12:20 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-29 9:59 [PATCH i-g-t v4 0/7] lib/igt_kms: Convert properties to be more atomic-like Maarten Lankhorst
2017-09-29 9:59 ` [PATCH i-g-t v4 1/7] tests: Stop looking at plane private members Maarten Lankhorst
2017-09-29 13:13 ` Mika Kahola
2017-10-02 9:31 ` Maarten Lankhorst
2017-09-29 9:59 ` [PATCH i-g-t v4 2/7] lib/igt_kms: Change output->pending_crtc_idx_mask to output->pending_pipe Maarten Lankhorst
2017-10-02 10:19 ` Mika Kahola
2017-09-29 9:59 ` [PATCH i-g-t v4 3/7] lib/igt_kms: Commit primary plane when a modeset is forced on a pipe Maarten Lankhorst
2017-10-02 11:02 ` Mika Kahola
2017-09-29 9:59 ` [PATCH i-g-t v4 4/7] lib/igt_kms: Rework connector properties to be more atomic, v2 Maarten Lankhorst
2017-10-02 12:22 ` Mika Kahola [this message]
2017-09-29 9:59 ` [PATCH i-g-t v4 5/7] lib/igt_kms: Rework plane properties to be more atomic, v4 Maarten Lankhorst
2017-10-03 12:05 ` Mika Kahola
2017-10-04 7:26 ` Maarten Lankhorst
2017-10-04 13:54 ` [PATCH i-g-t] lib/igt_kms: Rework plane properties to be more atomic, v5 Maarten Lankhorst
2017-09-29 9:59 ` [PATCH i-g-t v4 6/7] lib/igt_kms: Rework pipe properties to be more atomic, v4.1 Maarten Lankhorst
2017-10-03 7:45 ` [PATCH i-g-t] lib/igt_kms: Rework pipe properties to be more atomic, v5 Maarten Lankhorst
2017-10-04 13:55 ` [PATCH i-g-t] lib/igt_kms: Rework pipe properties to be more atomic, v6 Maarten Lankhorst
2017-09-29 9:59 ` [PATCH i-g-t v4 7/7] igt/kms_rotation_crc : Fix flip tests for sprite plane Maarten Lankhorst
2017-09-29 10:58 ` Mika Kahola
2017-09-29 11:03 ` ✓ Fi.CI.BAT: success for lib/igt_kms: Convert properties to be more atomic-like. (rev8) Patchwork
2017-09-29 12:13 ` ✗ Fi.CI.IGT: failure " Patchwork
2017-10-03 9:08 ` ✗ Fi.CI.BAT: failure for lib/igt_kms: Convert properties to be more atomic-like. (rev9) Patchwork
2017-10-03 10:22 ` Petri Latvala
2017-10-04 18:00 ` ✓ Fi.CI.BAT: success for lib/igt_kms: Convert properties to be more atomic-like. (rev11) Patchwork
2017-10-04 22:08 ` ✗ Fi.CI.IGT: warning " Patchwork
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=1506946925.3274.26.camel@intel.com \
--to=mika.kahola@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=maarten.lankhorst@linux.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