| Message ID | 1444993189-18645-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Currently object tiling is inferred from the frame buffer modifier > and only for legacy X scanout. > > It is useful to support overriding this selection for certain tests > so add the capability. So you want to set up the object tiling differently from the fb tiling? Why is that? And don't we reject it in the kernel? If we don't need a fence for scanout (ie. FBC or gen2/3) we could allow it I suppose, but not sure it it really helps with anything. BTW I was thinking of allowing the test to pass in an already created bo to igt_create_fb to use in some fb->offsets[] tests. > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Vivek Kasireddy <vivek.kasireddy@intel.com> > --- > lib/igt_fb.c | 21 ++++++++++++++------- > lib/igt_fb.h | 3 ++- > tests/kms_flip.c | 3 ++- > 3 files changed, 18 insertions(+), 9 deletions(-) > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c > index d04f02c0d035..73be93cb6e63 100644 > --- a/lib/igt_fb.c > +++ b/lib/igt_fb.c > @@ -75,7 +75,8 @@ static struct format_desc_struct { > > /* helpers to create nice-looking framebuffers */ > static int create_bo_for_fb(int fd, int width, int height, int bpp, > - uint64_t tiling, unsigned bo_size, > + uint64_t tiling, unsigned int obj_tiling, > + unsigned bo_size, > uint32_t *gem_handle_ret, > unsigned *size_ret, > unsigned *stride_ret) > @@ -113,7 +114,9 @@ static int create_bo_for_fb(int fd, int width, int height, int bpp, > gem_handle = gem_create(fd, bo_size); > > if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED) > - ret = __gem_set_tiling(fd, gem_handle, I915_TILING_X, stride); > + obj_tiling = I915_TILING_X; > + if (obj_tiling != I915_TILING_NONE) > + ret = __gem_set_tiling(fd, gem_handle, obj_tiling, stride); > > *stride_ret = stride; > *size_ret = size; > @@ -414,7 +417,8 @@ void igt_paint_image(cairo_t *cr, const char *filename, > */ > unsigned int > igt_create_fb_with_bo_size(int fd, int width, int height, > - uint32_t format, uint64_t tiling, > + uint32_t format, > + uint64_t tiling, unsigned int obj_tiling, > struct igt_fb *fb, unsigned bo_size) > { > uint32_t fb_id; > @@ -426,8 +430,9 @@ igt_create_fb_with_bo_size(int fd, int width, int height, > > igt_debug("%s(width=%d, height=%d, format=0x%x [bpp=%d], tiling=0x%"PRIx64", size=%d)\n", > __func__, width, height, format, bpp, tiling, bo_size); > - do_or_die(create_bo_for_fb(fd, width, height, bpp, tiling, bo_size, > - &fb->gem_handle, &fb->size, &fb->stride)); > + do_or_die(create_bo_for_fb(fd, width, height, bpp, tiling, obj_tiling, > + bo_size, &fb->gem_handle, &fb->size, > + &fb->stride)); > > igt_debug("%s(handle=%d, pitch=%d)\n", > __func__, fb->gem_handle, fb->stride); > @@ -485,7 +490,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height, > unsigned int igt_create_fb(int fd, int width, int height, uint32_t format, > uint64_t tiling, struct igt_fb *fb) > { > - return igt_create_fb_with_bo_size(fd, width, height, format, tiling, fb, 0); > + return igt_create_fb_with_bo_size(fd, width, height, format, > + tiling, I915_TILING_NONE, fb, 0); > } > > /** > @@ -714,7 +720,8 @@ static void create_cairo_surface__blit(int fd, struct igt_fb *fb) > */ > bpp = igt_drm_format_to_bpp(fb->drm_format); > ret = create_bo_for_fb(fd, fb->width, fb->height, bpp, > - LOCAL_DRM_FORMAT_MOD_NONE, 0, > + LOCAL_DRM_FORMAT_MOD_NONE, I915_TILING_NONE, > + 0, > &blit->linear.handle, > &blit->linear.size, > &blit->linear.stride); > diff --git a/lib/igt_fb.h b/lib/igt_fb.h > index a07acd2444b8..fa92fd4efd5b 100644 > --- a/lib/igt_fb.h > +++ b/lib/igt_fb.h > @@ -71,7 +71,8 @@ enum igt_text_align { > > unsigned int > igt_create_fb_with_bo_size(int fd, int width, int height, > - uint32_t format, uint64_t tiling, > + uint32_t format, > + uint64_t tiling, unsigned int obj_tiling, > struct igt_fb *fb, unsigned bo_size); > unsigned int igt_create_fb(int fd, int width, int height, uint32_t format, > uint64_t tiling, struct igt_fb *fb); > diff --git a/tests/kms_flip.c b/tests/kms_flip.c > index a139d402608e..60577d23b9a4 100644 > --- a/tests/kms_flip.c > +++ b/tests/kms_flip.c > @@ -1366,7 +1366,8 @@ static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs, > tiling, &o->fb_info[0]); > o->fb_ids[1] = igt_create_fb_with_bo_size(drm_fd, o->fb_width, o->fb_height, > igt_bpp_depth_to_drm_format(o->bpp, o->depth), > - tiling, &o->fb_info[1], bo_size); > + tiling, I915_TILING_NONE, > + &o->fb_info[1], bo_size); > o->fb_ids[2] = igt_create_fb(drm_fd, o->fb_width, o->fb_height, > igt_bpp_depth_to_drm_format(o->bpp, o->depth), > LOCAL_I915_FORMAT_MOD_X_TILED, &o->fb_info[2]); > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi, On 16/10/15 13:03, Ville Syrjälä wrote: > On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Currently object tiling is inferred from the frame buffer modifier >> and only for legacy X scanout. >> >> It is useful to support overriding this selection for certain tests >> so add the capability. > > So you want to set up the object tiling differently from the fb > tiling? Why is that? And don't we reject it in the kernel? If we don't > need a fence for scanout (ie. FBC or gen2/3) we could allow it I > suppose, but not sure it it really helps with anything. Hm, yes and no. Only different in a sense that currently igt_fb leaves object tiling at linear regardless of the fb modifier tiling. (Apart for the legacy X where it requires that they match.) I needed a way of having Y tiled fb modifier and Y tiled object to hit a warning in i915_gem_object_get_fence when only the rotated view exists. Patch series is probably to invasive anyway, especially I did not spend any time evaluating if 2/3 is safe. So hopefully Vivek can refine his version of the testcase which would then be completely confined to kms_rotation_crc. Regards, Tvrtko
On Fri, Oct 16, 2015 at 01:19:35PM +0100, Tvrtko Ursulin wrote: > > Hi, > > On 16/10/15 13:03, Ville Syrjälä wrote: > > On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote: > >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >> > >> Currently object tiling is inferred from the frame buffer modifier > >> and only for legacy X scanout. > >> > >> It is useful to support overriding this selection for certain tests > >> so add the capability. > > > > So you want to set up the object tiling differently from the fb > > tiling? Why is that? And don't we reject it in the kernel? If we don't > > need a fence for scanout (ie. FBC or gen2/3) we could allow it I > > suppose, but not sure it it really helps with anything. > > Hm, yes and no. Only different in a sense that currently igt_fb leaves > object tiling at linear regardless of the fb modifier tiling. (Apart for > the legacy X where it requires that they match.) > > I needed a way of having Y tiled fb modifier and Y tiled object to hit a > warning in i915_gem_object_get_fence when only the rotated view exists. Is there a problem of just doing what X tiled did also for Y tiled? > > Patch series is probably to invasive anyway, especially I did not spend > any time evaluating if 2/3 is safe. So hopefully Vivek can refine his > version of the testcase which would then be completely confined to > kms_rotation_crc. > > Regards, > > Tvrtko
On 16/10/15 13:29, Ville Syrjälä wrote: > On Fri, Oct 16, 2015 at 01:19:35PM +0100, Tvrtko Ursulin wrote: >> >> Hi, >> >> On 16/10/15 13:03, Ville Syrjälä wrote: >>> On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote: >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>> >>>> Currently object tiling is inferred from the frame buffer modifier >>>> and only for legacy X scanout. >>>> >>>> It is useful to support overriding this selection for certain tests >>>> so add the capability. >>> >>> So you want to set up the object tiling differently from the fb >>> tiling? Why is that? And don't we reject it in the kernel? If we don't >>> need a fence for scanout (ie. FBC or gen2/3) we could allow it I >>> suppose, but not sure it it really helps with anything. >> >> Hm, yes and no. Only different in a sense that currently igt_fb leaves >> object tiling at linear regardless of the fb modifier tiling. (Apart for >> the legacy X where it requires that they match.) >> >> I needed a way of having Y tiled fb modifier and Y tiled object to hit a >> warning in i915_gem_object_get_fence when only the rotated view exists. > > Is there a problem of just doing what X tiled did also for Y tiled? What do you mean? In the kernel or igt_fb? If latter then it needs to be able to have object tiling as linear by default since that is how fb modifiers were intended to be used - decoupled from obj tiling. Is that what you meant? Regards, Tvrtko
On Fri, Oct 16, 2015 at 01:54:51PM +0100, Tvrtko Ursulin wrote: > > On 16/10/15 13:29, Ville Syrjälä wrote: > > On Fri, Oct 16, 2015 at 01:19:35PM +0100, Tvrtko Ursulin wrote: > >> > >> Hi, > >> > >> On 16/10/15 13:03, Ville Syrjälä wrote: > >>> On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote: > >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>>> > >>>> Currently object tiling is inferred from the frame buffer modifier > >>>> and only for legacy X scanout. > >>>> > >>>> It is useful to support overriding this selection for certain tests > >>>> so add the capability. > >>> > >>> So you want to set up the object tiling differently from the fb > >>> tiling? Why is that? And don't we reject it in the kernel? If we don't > >>> need a fence for scanout (ie. FBC or gen2/3) we could allow it I > >>> suppose, but not sure it it really helps with anything. > >> > >> Hm, yes and no. Only different in a sense that currently igt_fb leaves > >> object tiling at linear regardless of the fb modifier tiling. (Apart for > >> the legacy X where it requires that they match.) > >> > >> I needed a way of having Y tiled fb modifier and Y tiled object to hit a > >> warning in i915_gem_object_get_fence when only the rotated view exists. > > > > Is there a problem of just doing what X tiled did also for Y tiled? > > What do you mean? In the kernel or igt_fb? If latter then it needs to be > able to have object tiling as linear by default since that is how fb > modifiers were intended to be used - decoupled from obj tiling. Is that > what you meant? Just setting obj to Y tiled when fb is Y tiled from igt_fb.
On 16/10/15 14:01, Ville Syrjälä wrote: > On Fri, Oct 16, 2015 at 01:54:51PM +0100, Tvrtko Ursulin wrote: >> >> On 16/10/15 13:29, Ville Syrjälä wrote: >>> On Fri, Oct 16, 2015 at 01:19:35PM +0100, Tvrtko Ursulin wrote: >>>> >>>> Hi, >>>> >>>> On 16/10/15 13:03, Ville Syrjälä wrote: >>>>> On Fri, Oct 16, 2015 at 11:59:47AM +0100, Tvrtko Ursulin wrote: >>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>>>> >>>>>> Currently object tiling is inferred from the frame buffer modifier >>>>>> and only for legacy X scanout. >>>>>> >>>>>> It is useful to support overriding this selection for certain tests >>>>>> so add the capability. >>>>> >>>>> So you want to set up the object tiling differently from the fb >>>>> tiling? Why is that? And don't we reject it in the kernel? If we don't >>>>> need a fence for scanout (ie. FBC or gen2/3) we could allow it I >>>>> suppose, but not sure it it really helps with anything. >>>> >>>> Hm, yes and no. Only different in a sense that currently igt_fb leaves >>>> object tiling at linear regardless of the fb modifier tiling. (Apart for >>>> the legacy X where it requires that they match.) >>>> >>>> I needed a way of having Y tiled fb modifier and Y tiled object to hit a >>>> warning in i915_gem_object_get_fence when only the rotated view exists. >>> >>> Is there a problem of just doing what X tiled did also for Y tiled? >> >> What do you mean? In the kernel or igt_fb? If latter then it needs to be >> able to have object tiling as linear by default since that is how fb >> modifiers were intended to be used - decoupled from obj tiling. Is that >> what you meant? > > Just setting obj to Y tiled when fb is Y tiled from igt_fb. Right, so I explained that - we want to have Y tiled fb modifier + linear object in the majority of cases, _apart_ from this special test case which tries to hit a WARN_ON on !obj->map_and_fenceable. Which happens when obj tiling is Y, and fb tiling is Y, _and_ no normal VMA exists, just the rotate one. Regards, Tvrtko
diff --git a/lib/igt_fb.c b/lib/igt_fb.c index d04f02c0d035..73be93cb6e63 100644 --- a/lib/igt_fb.c +++ b/lib/igt_fb.c @@ -75,7 +75,8 @@ static struct format_desc_struct { /* helpers to create nice-looking framebuffers */ static int create_bo_for_fb(int fd, int width, int height, int bpp, - uint64_t tiling, unsigned bo_size, + uint64_t tiling, unsigned int obj_tiling, + unsigned bo_size, uint32_t *gem_handle_ret, unsigned *size_ret, unsigned *stride_ret) @@ -113,7 +114,9 @@ static int create_bo_for_fb(int fd, int width, int height, int bpp, gem_handle = gem_create(fd, bo_size); if (tiling == LOCAL_I915_FORMAT_MOD_X_TILED) - ret = __gem_set_tiling(fd, gem_handle, I915_TILING_X, stride); + obj_tiling = I915_TILING_X; + if (obj_tiling != I915_TILING_NONE) + ret = __gem_set_tiling(fd, gem_handle, obj_tiling, stride); *stride_ret = stride; *size_ret = size; @@ -414,7 +417,8 @@ void igt_paint_image(cairo_t *cr, const char *filename, */ unsigned int igt_create_fb_with_bo_size(int fd, int width, int height, - uint32_t format, uint64_t tiling, + uint32_t format, + uint64_t tiling, unsigned int obj_tiling, struct igt_fb *fb, unsigned bo_size) { uint32_t fb_id; @@ -426,8 +430,9 @@ igt_create_fb_with_bo_size(int fd, int width, int height, igt_debug("%s(width=%d, height=%d, format=0x%x [bpp=%d], tiling=0x%"PRIx64", size=%d)\n", __func__, width, height, format, bpp, tiling, bo_size); - do_or_die(create_bo_for_fb(fd, width, height, bpp, tiling, bo_size, - &fb->gem_handle, &fb->size, &fb->stride)); + do_or_die(create_bo_for_fb(fd, width, height, bpp, tiling, obj_tiling, + bo_size, &fb->gem_handle, &fb->size, + &fb->stride)); igt_debug("%s(handle=%d, pitch=%d)\n", __func__, fb->gem_handle, fb->stride); @@ -485,7 +490,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height, unsigned int igt_create_fb(int fd, int width, int height, uint32_t format, uint64_t tiling, struct igt_fb *fb) { - return igt_create_fb_with_bo_size(fd, width, height, format, tiling, fb, 0); + return igt_create_fb_with_bo_size(fd, width, height, format, + tiling, I915_TILING_NONE, fb, 0); } /** @@ -714,7 +720,8 @@ static void create_cairo_surface__blit(int fd, struct igt_fb *fb) */ bpp = igt_drm_format_to_bpp(fb->drm_format); ret = create_bo_for_fb(fd, fb->width, fb->height, bpp, - LOCAL_DRM_FORMAT_MOD_NONE, 0, + LOCAL_DRM_FORMAT_MOD_NONE, I915_TILING_NONE, + 0, &blit->linear.handle, &blit->linear.size, &blit->linear.stride); diff --git a/lib/igt_fb.h b/lib/igt_fb.h index a07acd2444b8..fa92fd4efd5b 100644 --- a/lib/igt_fb.h +++ b/lib/igt_fb.h @@ -71,7 +71,8 @@ enum igt_text_align { unsigned int igt_create_fb_with_bo_size(int fd, int width, int height, - uint32_t format, uint64_t tiling, + uint32_t format, + uint64_t tiling, unsigned int obj_tiling, struct igt_fb *fb, unsigned bo_size); unsigned int igt_create_fb(int fd, int width, int height, uint32_t format, uint64_t tiling, struct igt_fb *fb); diff --git a/tests/kms_flip.c b/tests/kms_flip.c index a139d402608e..60577d23b9a4 100644 --- a/tests/kms_flip.c +++ b/tests/kms_flip.c @@ -1366,7 +1366,8 @@ static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs, tiling, &o->fb_info[0]); o->fb_ids[1] = igt_create_fb_with_bo_size(drm_fd, o->fb_width, o->fb_height, igt_bpp_depth_to_drm_format(o->bpp, o->depth), - tiling, &o->fb_info[1], bo_size); + tiling, I915_TILING_NONE, + &o->fb_info[1], bo_size); o->fb_ids[2] = igt_create_fb(drm_fd, o->fb_width, o->fb_height, igt_bpp_depth_to_drm_format(o->bpp, o->depth), LOCAL_I915_FORMAT_MOD_X_TILED, &o->fb_info[2]);