| Message ID | 1429139959-18220-1-git-send-email-chandra.konduru@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
"kms_plane_scaling" would be a better tag for this commit. You can still make sure that "i-g-t" appears in the subject line by using the --subject-prefix="PATCH i-g-t" option when using git send-email. On 16 April 2015 at 00:19, Chandra Konduru <chandra.konduru@intel.com> wrote: > From: chandra konduru <chandra.konduru@intel.com> > > Adding rotation to kms_plane_scaling test. > > Signed-off-by: chandra konduru <chandra.konduru@intel.com> > --- > tests/kms_plane_scaling.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c > index 00db5cb..8d22ba4 100644 > --- a/tests/kms_plane_scaling.c > +++ b/tests/kms_plane_scaling.c > @@ -101,11 +101,11 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe, > data->fb_id1 = igt_create_fb(data->drm_fd, > mode->hdisplay, mode->vdisplay, > DRM_FORMAT_XRGB8888, > - LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */ > + LOCAL_I915_FORMAT_MOD_Y_TILED, /* tiled */ > &data->fb1); > igt_assert(data->fb_id1); > > - paint_color(data, &data->fb1, mode->hdisplay, mode->vdisplay); > + paint_color(data, &data->fb1, data->fb1.width, data->fb1.height); This looks like an unrelated change. > > /* > * We always set the primary plane to actually enable the pipe as > @@ -209,7 +209,7 @@ static void test_plane_scaling(data_t *d) > cairo_surface_t *image; > enum pipe pipe; > int valid_tests = 0; > - int primary_plane_scaling = 0; /* For now */ > + int primary_plane_scaling = 1; Since this is now enabled and not set elsewhere, could it and the if statements that use it now be removed? Since it doesn't just affect the new rotation tests, it could be removed in a separate patch. > > igt_require(d->display.has_universal_planes); > igt_require(d->num_scalers); > @@ -250,18 +250,20 @@ static void test_plane_scaling(data_t *d) > prepare_crtc(d, output, pipe, d->plane1, mode, COMMIT_UNIVERSAL); > > if (primary_plane_scaling) { > - /* Primary plane upscaling */ > + /* Primary plane upscaling with rotation */ Isn't this now testing two things at once? Would it be better to test them separately? > igt_fb_set_position(&d->fb1, d->plane1, 100, 100); > igt_fb_set_size(&d->fb1, d->plane1, 500, 500); > igt_plane_set_position(d->plane1, 0, 0); > igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay); > + igt_plane_set_rotation(d->plane1, IGT_ROTATION_90); > igt_display_commit2(display, COMMIT_UNIVERSAL); > > - /* Primary plane 1:1 no scaling */ > + /* Primary plane 1:1 no scaling & no rotation */ > igt_fb_set_position(&d->fb1, d->plane1, 0, 0); > igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d->fb1.height); > igt_plane_set_position(d->plane1, 0, 0); > igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay); > + igt_plane_set_rotation(d->plane1, IGT_ROTATION_0); > igt_display_commit2(display, COMMIT_UNIVERSAL); > } > > @@ -318,10 +320,10 @@ static void test_plane_scaling(data_t *d) > igt_plane_set_position(d->plane2, 100, 100); > igt_plane_set_size(d->plane2, d->fb2.width-200, d->fb2.height-200); > > - igt_fb_set_position(&d->fb3, d->plane3, 100, 100); > - igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-400, d->fb3.height-400); > - igt_plane_set_position(d->plane3, 10, 10); > - igt_plane_set_size(d->plane3, mode->hdisplay-300, mode->vdisplay-300); > + igt_fb_set_position(&d->fb3, d->plane3, 0, 0); > + igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width, d->fb3.height); > + igt_plane_set_position(d->plane3, 500, 500); > + igt_plane_set_size(d->plane3, d->fb3.width * 2/3, d->fb3.height * 2/3); As above, these changes also look unrelated to rotation and should be in a separate patch. > igt_display_commit2(display, COMMIT_UNIVERSAL); > > if (primary_plane_scaling) { > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -----Original Message----- > From: Thomas Wood [mailto:thomas.wood@intel.com] > Sent: Wednesday, April 22, 2015 9:58 AM > To: Konduru, Chandra > Cc: Intel Graphics Development > Subject: Re: [Intel-gfx] [PATCH] i-g-t: Adding rotation to plane scaling test > > "kms_plane_scaling" would be a better tag for this commit. You can still make > sure that "i-g-t" appears in the subject line by using the --subject-prefix="PATCH > i-g-t" option when using git send-email. Submitted updated patch "[PREFIX i-g-t] Improvements to kms_plane_scaling" to address your feedback. Also resolved an issue Tvrtko reported. > > On 16 April 2015 at 00:19, Chandra Konduru <chandra.konduru@intel.com> > wrote: > > From: chandra konduru <chandra.konduru@intel.com> > > > > Adding rotation to kms_plane_scaling test. > > > > Signed-off-by: chandra konduru <chandra.konduru@intel.com> > > --- > > tests/kms_plane_scaling.c | 20 +++++++++++--------- > > 1 file changed, 11 insertions(+), 9 deletions(-) > > > > diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c > > index 00db5cb..8d22ba4 100644 > > --- a/tests/kms_plane_scaling.c > > +++ b/tests/kms_plane_scaling.c > > @@ -101,11 +101,11 @@ static void prepare_crtc(data_t *data, igt_output_t > *output, enum pipe pipe, > > data->fb_id1 = igt_create_fb(data->drm_fd, > > mode->hdisplay, mode->vdisplay, > > DRM_FORMAT_XRGB8888, > > - LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */ > > + LOCAL_I915_FORMAT_MOD_Y_TILED, /* tiled */ > > &data->fb1); > > igt_assert(data->fb_id1); > > > > - paint_color(data, &data->fb1, mode->hdisplay, mode->vdisplay); > > + paint_color(data, &data->fb1, data->fb1.width, > > + data->fb1.height); > > This looks like an unrelated change. > > > > > > /* > > * We always set the primary plane to actually enable the pipe > > as @@ -209,7 +209,7 @@ static void test_plane_scaling(data_t *d) > > cairo_surface_t *image; > > enum pipe pipe; > > int valid_tests = 0; > > - int primary_plane_scaling = 0; /* For now */ > > + int primary_plane_scaling = 1; > > Since this is now enabled and not set elsewhere, could it and the if statements > that use it now be removed? Since it doesn't just affect the new rotation tests, it > could be removed in a separate patch. > > > > > > igt_require(d->display.has_universal_planes); > > igt_require(d->num_scalers); > > @@ -250,18 +250,20 @@ static void test_plane_scaling(data_t *d) > > prepare_crtc(d, output, pipe, d->plane1, mode, > > COMMIT_UNIVERSAL); > > > > if (primary_plane_scaling) { > > - /* Primary plane upscaling */ > > + /* Primary plane upscaling with rotation */ > > Isn't this now testing two things at once? Would it be better to test them > separately? > > > > igt_fb_set_position(&d->fb1, d->plane1, 100, 100); > > igt_fb_set_size(&d->fb1, d->plane1, 500, 500); > > igt_plane_set_position(d->plane1, 0, 0); > > igt_plane_set_size(d->plane1, mode->hdisplay, > > mode->vdisplay); > > + igt_plane_set_rotation(d->plane1, > > + IGT_ROTATION_90); > > igt_display_commit2(display, > > COMMIT_UNIVERSAL); > > > > - /* Primary plane 1:1 no scaling */ > > + /* Primary plane 1:1 no scaling & no rotation > > + */ > > igt_fb_set_position(&d->fb1, d->plane1, 0, 0); > > igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d->fb1.height); > > igt_plane_set_position(d->plane1, 0, 0); > > igt_plane_set_size(d->plane1, mode->hdisplay, > > mode->vdisplay); > > + igt_plane_set_rotation(d->plane1, > > + IGT_ROTATION_0); > > igt_display_commit2(display, COMMIT_UNIVERSAL); > > } > > > > @@ -318,10 +320,10 @@ static void test_plane_scaling(data_t *d) > > igt_plane_set_position(d->plane2, 100, 100); > > igt_plane_set_size(d->plane2, d->fb2.width-200, > > d->fb2.height-200); > > > > - igt_fb_set_position(&d->fb3, d->plane3, 100, 100); > > - igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-400, d->fb3.height- > 400); > > - igt_plane_set_position(d->plane3, 10, 10); > > - igt_plane_set_size(d->plane3, mode->hdisplay-300, mode->vdisplay- > 300); > > + igt_fb_set_position(&d->fb3, d->plane3, 0, 0); > > + igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width, d->fb3.height); > > + igt_plane_set_position(d->plane3, 500, 500); > > + igt_plane_set_size(d->plane3, d->fb3.width * 2/3, > > + d->fb3.height * 2/3); > > As above, these changes also look unrelated to rotation and should be in a > separate patch. > > > > igt_display_commit2(display, COMMIT_UNIVERSAL); > > > > if (primary_plane_scaling) { > > -- > > 1.9.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c index 00db5cb..8d22ba4 100644 --- a/tests/kms_plane_scaling.c +++ b/tests/kms_plane_scaling.c @@ -101,11 +101,11 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe, data->fb_id1 = igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay, DRM_FORMAT_XRGB8888, - LOCAL_I915_FORMAT_MOD_X_TILED, /* tiled */ + LOCAL_I915_FORMAT_MOD_Y_TILED, /* tiled */ &data->fb1); igt_assert(data->fb_id1); - paint_color(data, &data->fb1, mode->hdisplay, mode->vdisplay); + paint_color(data, &data->fb1, data->fb1.width, data->fb1.height); /* * We always set the primary plane to actually enable the pipe as @@ -209,7 +209,7 @@ static void test_plane_scaling(data_t *d) cairo_surface_t *image; enum pipe pipe; int valid_tests = 0; - int primary_plane_scaling = 0; /* For now */ + int primary_plane_scaling = 1; igt_require(d->display.has_universal_planes); igt_require(d->num_scalers); @@ -250,18 +250,20 @@ static void test_plane_scaling(data_t *d) prepare_crtc(d, output, pipe, d->plane1, mode, COMMIT_UNIVERSAL); if (primary_plane_scaling) { - /* Primary plane upscaling */ + /* Primary plane upscaling with rotation */ igt_fb_set_position(&d->fb1, d->plane1, 100, 100); igt_fb_set_size(&d->fb1, d->plane1, 500, 500); igt_plane_set_position(d->plane1, 0, 0); igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay); + igt_plane_set_rotation(d->plane1, IGT_ROTATION_90); igt_display_commit2(display, COMMIT_UNIVERSAL); - /* Primary plane 1:1 no scaling */ + /* Primary plane 1:1 no scaling & no rotation */ igt_fb_set_position(&d->fb1, d->plane1, 0, 0); igt_fb_set_size(&d->fb1, d->plane1, d->fb1.width, d->fb1.height); igt_plane_set_position(d->plane1, 0, 0); igt_plane_set_size(d->plane1, mode->hdisplay, mode->vdisplay); + igt_plane_set_rotation(d->plane1, IGT_ROTATION_0); igt_display_commit2(display, COMMIT_UNIVERSAL); } @@ -318,10 +320,10 @@ static void test_plane_scaling(data_t *d) igt_plane_set_position(d->plane2, 100, 100); igt_plane_set_size(d->plane2, d->fb2.width-200, d->fb2.height-200); - igt_fb_set_position(&d->fb3, d->plane3, 100, 100); - igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width-400, d->fb3.height-400); - igt_plane_set_position(d->plane3, 10, 10); - igt_plane_set_size(d->plane3, mode->hdisplay-300, mode->vdisplay-300); + igt_fb_set_position(&d->fb3, d->plane3, 0, 0); + igt_fb_set_size(&d->fb3, d->plane3, d->fb3.width, d->fb3.height); + igt_plane_set_position(d->plane3, 500, 500); + igt_plane_set_size(d->plane3, d->fb3.width * 2/3, d->fb3.height * 2/3); igt_display_commit2(display, COMMIT_UNIVERSAL); if (primary_plane_scaling) {