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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.