Op 17-02-17 om 14:39 schreef Mika Kahola:
> We need to make sure that TEST_ONLY really only touches the free-standing
> state objects and nothing else. Test approach here is the following:
>
> - Create a config and submit it with TEST_ONLY.
> - do dpms off/on cycle with the current config to reconfigure hw
> - read back all legacy state to make sure none of that is clobbered
>
> Signed-off-by: Mika Kahola <mika.kah...@intel.com>
> ---
>  tests/kms_atomic.c | 79 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
>
> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
> index d6273f4..3531fa4 100644
> --- a/tests/kms_atomic.c
> +++ b/tests/kms_atomic.c
> @@ -831,6 +831,25 @@ static uint32_t plane_get_igt_format(struct 
> kms_atomic_plane_state *plane)
>       return ret;
>  }
>  
> +static void
> +set_dpms(int fd, int mode)
> +{
> +     int i;
> +     drmModeConnector *connector;
> +     uint32_t id;
> +     drmModeRes *resources = drmModeGetResources(fd);
> +
> +     for (i = 0; i < resources->count_connectors; i++) {
> +             id = resources->connectors[i];
> +
> +             connector = drmModeGetConnectorCurrent(fd, id);
> +
> +             kmstest_set_connector_dpms(fd, connector, mode);
> +
> +             drmModeFreeConnector(connector);
> +     }
> +}
> +
>  static void plane_overlay(struct kms_atomic_crtc_state *crtc,
>                         struct kms_atomic_plane_state *plane_old)
>  {
> @@ -930,6 +949,54 @@ static void plane_primary(struct kms_atomic_crtc_state 
> *crtc,
>       drmModeAtomicFree(req);
>  }
>  
> +static void plane_primary_state_check(struct kms_atomic_crtc_state *crtc,
> +                                   struct kms_atomic_plane_state *plane_old)
> +{
> +     struct drm_mode_modeinfo *mode = crtc->mode.data;
> +     struct kms_atomic_plane_state plane = *plane_old;
> +     uint32_t format = plane_get_igt_format(&plane);
> +     drmModeAtomicReq *req = drmModeAtomicAlloc();
> +     struct igt_fb fb;
> +     int ret;
> +
> +     igt_require(format != 0);
> +
> +     plane.src_x = 0;
> +     plane.src_y = 0;
> +     plane.src_w = mode->hdisplay << 16;
> +     plane.src_h = mode->vdisplay << 16;
> +     plane.crtc_x = 0;
> +     plane.crtc_y = 0;
> +     plane.crtc_w = mode->hdisplay;
> +     plane.crtc_h = mode->vdisplay;
> +     plane.crtc_id = crtc->obj;
> +     plane.fb_id = igt_create_pattern_fb(plane.state->desc->fd,
> +                                         plane.crtc_w, plane.crtc_h,
> +                                         format, I915_TILING_NONE, &fb);
> +
> +     drmModeAtomicSetCursor(req, 0);
> +     crtc_populate_req(crtc, req);
> +     plane_populate_req(&plane, req);
> +     ret = drmModeAtomicCommit(crtc->state->desc->fd, req,
> +                               DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> +
> +     igt_assert_eq(ret, 0);
> +
> +     /* go through dpms off/on cycle */
> +     set_dpms(crtc->state->desc->fd, DRM_MODE_DPMS_OFF);
> +     set_dpms(crtc->state->desc->fd, DRM_MODE_DPMS_ON);
Is this required? If the state was changed, shouldn't we be able to tell 
through the properties?
> +     /* check the state */
> +     crtc_check_current_state(crtc, plane_old, CRTC_RELAX_MODE);
> +     plane_check_current_state(plane_old, CRTC_RELAX_MODE);
Copy paste the relax states? Are the RELAXes required since you only set/unset 
the current mode?
> +     /* Re-enable the plane through the legacy CRTC/primary-plane API, and
> +      * verify through atomic. */
> +     crtc_commit_legacy(crtc, plane_old, CRTC_RELAX_MODE);
> +
> +     drmModeAtomicFree(req);
> +}
> +
>  static void plane_cursor(struct kms_atomic_crtc_state *crtc,
>                        struct kms_atomic_plane_state *plane_old)
>  {
> @@ -1427,6 +1494,18 @@ igt_main
>               atomic_state_free(scratch);
>       }
>  
> +     igt_subtest("plane_primary_state_check") {
> +             struct kms_atomic_state *scratch = atomic_state_dup(current);
> +             struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);
> +             struct kms_atomic_plane_state *plane =
> +                     find_plane(scratch, PLANE_TYPE_PRIMARY, crtc);
> +
> +             igt_require(crtc);
> +             igt_require(plane);
> +             plane_primary_state_check(crtc, plane);
> +             atomic_state_free(scratch);
> +     }
Test (and function) name doesn't match the description. It's hard to tell what 
this function is doing from the name. :)
> +
>       igt_subtest("plane_cursor_legacy") {
>               struct kms_atomic_state *scratch = atomic_state_dup(current);
>               struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);

With some improvements, test looks sane. :)

~Maarten

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to