> -----Original Message----- > From: Thomas Wood [mailto:thomas.wood@xxxxxxxxx] > Sent: Wednesday, April 22, 2015 9:58 AM > To: Konduru, Chandra > Cc: Intel Graphics Development > Subject: Re: [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@xxxxxxxxx> > wrote: > > From: chandra konduru <chandra.konduru@xxxxxxxxx> > > > > Adding rotation to kms_plane_scaling test. > > > > Signed-off-by: chandra konduru <chandra.konduru@xxxxxxxxx> > > --- > > 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@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx