[igt-dev] [PATCH i-g-t 4/5] lib/igt_fb: Pass around igt_fb internally
Ville Syrjälä
ville.syrjala at linux.intel.com
Thu Sep 27 13:46:30 UTC 2018
On Tue, Sep 25, 2018 at 05:49:10PM -0700, Paulo Zanoni wrote:
> Em Ter, 2018-09-25 às 16:47 +0300, Ville Syrjala escreveu:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >
> > Instead of passing around a boatload of integers everywhere let's
> > just pass around the igt_fb struct. That obviously means we have to
> > populate it first sufficiently, to which end we'll add a small
> > helper.
> > Later on the stride/size calculations will consult the already
> > pre-populated igt_fb and fill in the rest as needed.
> >
> > This makes the whole thing a lot less error prone as it's impossible
> > to accidentally pass the arguments in the wrong order when there's
> > just the one of them, and it's a pointer.
>
> While I completely agree with these arguments, this change does not
> come without its own minor downsides. Code that deals-with/initializes
> igt_fb_t is now a little riskier due to the the fact that the order
> which things are initialized is way more important now. And we also
> moved a whole layer of our abstractions up, but that didn't seem to be
> a problem now so let's hope it still won't be a problem later.
>
> Still, a positive change even when downsides are considered.
>
> >
> > v2: Rebase due to uint64_t size
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> > lib/igt_draw.c | 2 +-
> > lib/igt_fb.c | 408 +++++++++++++++++++--------
> > ------------
> > lib/igt_fb.h | 4 +-
> > tests/kms_ccs.c | 2 +-
> > tests/kms_flip.c | 4 +-
> > tests/kms_frontbuffer_tracking.c | 6 +-
> > tests/pm_rpm.c | 2 +-
> > 7 files changed, 207 insertions(+), 221 deletions(-)
> >
> > diff --git a/lib/igt_draw.c b/lib/igt_draw.c
> > index c7d5770dca28..05821480bc80 100644
> > --- a/lib/igt_draw.c
> > +++ b/lib/igt_draw.c
> > @@ -720,7 +720,7 @@ void igt_draw_rect_fb(int fd, drm_intel_bufmgr
> > *bufmgr,
> > enum igt_draw_method method, int rect_x, int
> > rect_y,
> > int rect_w, int rect_h, uint32_t color)
> > {
> > - igt_draw_rect(fd, bufmgr, context, fb->gem_handle, fb->size,
> > fb->stride,
> > + igt_draw_rect(fd, bufmgr, context, fb->gem_handle, fb->size,
> > fb->strides[0],
> > method, rect_x, rect_y, rect_w, rect_h, color,
> > igt_drm_format_to_bpp(fb->drm_format));
> > }
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 3c6974503313..26019f0420dc 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -190,50 +190,72 @@ void igt_get_fb_tile_size(int fd, uint64_t
> > tiling, int fb_bpp,
> > }
> > }
> >
> > -static unsigned fb_plane_width(const struct format_desc_struct
> > *format,
> > - int plane, unsigned width)
> > +static unsigned fb_plane_width(const struct igt_fb *fb, int plane)
> > {
> > - if (format->drm_id == DRM_FORMAT_NV12 && plane == 1)
> > - return DIV_ROUND_UP(width, 2);
> > + if (fb->drm_format == DRM_FORMAT_NV12 && plane == 1)
> > + return DIV_ROUND_UP(fb->width, 2);
> >
> > - return width;
> > + return fb->width;
> > }
> >
> > -static unsigned fb_plane_bpp(const struct format_desc_struct
> > *format, int plane)
> > +static unsigned fb_plane_bpp(const struct igt_fb *fb, int plane)
> > {
> > + const struct format_desc_struct *format =
> > lookup_drm_format(fb->drm_format);
> > +
> > return format->plane_bpp[plane];
> > }
> >
> > -static unsigned fb_plane_min_stride(const struct format_desc_struct
> > *format,
> > - int plane, unsigned width)
> > +static unsigned fb_plane_height(const struct igt_fb *fb, int plane)
> > {
> > - unsigned cpp = fb_plane_bpp(format, plane) / 8;
> > + if (fb->drm_format == DRM_FORMAT_NV12 && plane == 1)
> > + return DIV_ROUND_UP(fb->height, 2);
> >
> > - return fb_plane_width(format, width, plane) * cpp;
> > + return fb->height;
> > }
> >
> > -static unsigned fb_plane_height(const struct format_desc_struct
> > *format,
> > - int plane, unsigned height)
> > +static int fb_num_planes(const struct igt_fb *fb)
> > {
> > - if (format->drm_id == DRM_FORMAT_NV12 && plane == 1)
> > - return DIV_ROUND_UP(height, 2);
> > + const struct format_desc_struct *format =
> > lookup_drm_format(fb->drm_format);
> >
> > - return height;
> > -}
> > -
> > -static int fb_num_planes(const struct format_desc_struct *format)
> > -{
> > return format->num_planes;
> > }
> >
> > -static unsigned calc_plane_stride(int fd,
> > - const struct format_desc_struct
> > *format,
> > - int width, uint64_t tiling, int
> > plane)
> > +static void fb_init(struct igt_fb *fb,
> > + int fd, int width, int height,
> > + uint32_t drm_format,
> > + uint64_t modifier,
> > + enum igt_color_encoding color_encoding,
> > + enum igt_color_range color_range)
> > {
> > - uint32_t min_stride = fb_plane_min_stride(format, width,
> > plane);
> > + const struct format_desc_struct *f =
> > lookup_drm_format(drm_format);
> >
> > - if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> > - intel_gen(intel_get_drm_devid(fd)) <= 3) {
> > + igt_assert_f(f, "DRM format %08x not found\n", drm_format);
> > +
> > + memset(fb, 0, sizeof(*fb));
> > +
> > + fb->width = width;
> > + fb->height = height;
> > + fb->tiling = modifier;
>
> Shouldn't we rename fb->tiling now? (of course, in a separate patch)
Yeah, would be good to unconfused the whole tiling vs. modifier
thing in this code.
>
> Everything else looks correct.
>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
Thanks.
>
> > + fb->drm_format = drm_format;
> > + fb->fd = fd;
> > + fb->num_planes = fb_num_planes(fb);
> > + fb->color_encoding = color_encoding;
> > + fb->color_range = color_range;
> > +
> > + for (int i = 0; i < fb->num_planes; i++) {
> > + fb->plane_bpp[i] = fb_plane_bpp(fb, i);
> > + fb->plane_height[i] = fb_plane_height(fb, i);
> > + fb->plane_width[i] = fb_plane_width(fb, i);
> > + }
> > +}
> > +
> > +static uint32_t calc_plane_stride(struct igt_fb *fb, int plane)
> > +{
> > + uint32_t min_stride = fb->plane_width[plane] *
> > + (fb->plane_bpp[plane] / 8);
> > +
> > + if (fb->tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> > + intel_gen(intel_get_drm_devid(fb->fd)) <= 3) {
> > uint32_t stride;
> >
> > /* Round the tiling up to the next power-of-two and
> > the region
> > @@ -251,22 +273,19 @@ static unsigned calc_plane_stride(int fd,
> > } else {
> > unsigned int tile_width, tile_height;
> >
> > - igt_get_fb_tile_size(fd, tiling,
> > - fb_plane_bpp(format, plane),
> > + igt_get_fb_tile_size(fb->fd, fb->tiling, fb-
> > >plane_bpp[plane],
> > &tile_width, &tile_height);
> >
> > return ALIGN(min_stride, tile_width);
> > }
> > }
> >
> > -static uint64_t calc_plane_size(int fd, int width, int height,
> > - const struct format_desc_struct
> > *format,
> > - uint64_t tiling, int plane,
> > - uint32_t stride)
> > +static uint64_t calc_plane_size(struct igt_fb *fb, int plane)
> > {
> > - if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> > - intel_gen(intel_get_drm_devid(fd)) <= 3) {
> > - uint64_t min_size = (uint64_t) stride * height;
> > + if (fb->tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> > + intel_gen(intel_get_drm_devid(fb->fd)) <= 3) {
> > + uint64_t min_size = (uint64_t) fb->strides[plane] *
> > + fb->plane_height[plane];
> > uint64_t size;
> >
> > /* Round the tiling up to the next power-of-two and
> > the region
> > @@ -284,38 +303,27 @@ static uint64_t calc_plane_size(int fd, int
> > width, int height,
> > } else {
> > unsigned int tile_width, tile_height;
> >
> > - igt_get_fb_tile_size(fd, tiling,
> > fb_plane_bpp(format, plane),
> > + igt_get_fb_tile_size(fb->fd, fb->tiling, fb-
> > >plane_bpp[plane],
> > &tile_width, &tile_height);
> >
> > - return (uint64_t) stride * ALIGN(height,
> > tile_height);
> > + return (uint64_t) fb->strides[plane] *
> > + ALIGN(fb->plane_height[plane], tile_height);
> > }
> > }
> >
> > -static uint64_t calc_fb_size(int fd, int width, int height,
> > - const struct format_desc_struct
> > *format,
> > - uint64_t tiling,
> > - uint32_t strides[4], uint32_t
> > offsets[4])
> > +static uint64_t calc_fb_size(struct igt_fb *fb)
> > {
> > uint64_t size = 0;
> > int plane;
> >
> > - for (plane = 0; plane < fb_num_planes(format); plane++) {
> > - if (!strides[plane])
> > - strides[plane] = calc_plane_stride(fd,
> > format,
> > - width,
> > tiling, plane);
> > + for (plane = 0; plane < fb->num_planes; plane++) {
> > + /* respect the stride requested by the caller */
> > + if (!fb->strides[plane])
> > + fb->strides[plane] = calc_plane_stride(fb,
> > plane);
> >
> > - if (offsets)
> > - offsets[plane] = size;
> > + fb->offsets[plane] = size;
> >
> > - size += calc_plane_size(fd, width, height,
> > - format, tiling, plane,
> > - strides[plane]);
> > - }
> > -
> > - for (; plane < ARRAY_SIZE(format->plane_bpp); plane++) {
> > - strides[plane] = 0;
> > - if (offsets)
> > - offsets[plane] = 0;
> > + size += calc_plane_size(fb, plane);
> > }
> >
> > return size;
> > @@ -337,13 +345,17 @@ static uint64_t calc_fb_size(int fd, int width,
> > int height,
> > void igt_calc_fb_size(int fd, int width, int height, uint32_t
> > drm_format, uint64_t tiling,
> > uint64_t *size_ret, unsigned *stride_ret)
> > {
> > - const struct format_desc_struct *format =
> > lookup_drm_format(drm_format);
> > - uint32_t strides[4] = {};
> > + struct igt_fb fb;
> >
> > - igt_assert(format);
> > + fb_init(&fb, fd, width, height, drm_format, tiling,
> > + IGT_COLOR_YCBCR_BT709,
> > IGT_COLOR_YCBCR_LIMITED_RANGE);
> >
> > - *size_ret = calc_fb_size(fd, width, height, format, tiling,
> > strides, NULL);
> > - *stride_ret = strides[0];
> > + fb.size = calc_fb_size(&fb);
> > +
> > + if (size_ret)
> > + *size_ret = fb.size;
> > + if (stride_ret)
> > + *stride_ret = fb.strides[0];
> > }
> >
> > /**
> > @@ -399,80 +411,64 @@ uint64_t igt_fb_tiling_to_mod(uint64_t tiling)
> > }
> >
> > /* helpers to create nice-looking framebuffers */
> > -static int create_bo_for_fb(int fd, int width, int height,
> > - enum igt_color_encoding color_encoding,
> > - enum igt_color_range color_range,
> > - const struct format_desc_struct *format,
> > - uint64_t tiling, uint64_t size, unsigned
> > stride,
> > - uint64_t *size_ret, unsigned
> > *stride_ret,
> > - uint32_t offsets[4], bool *is_dumb)
> > +static int create_bo_for_fb(struct igt_fb *fb)
> > {
> > - int bo;
> > + int fd = fb->fd;
> >
> > - igt_assert(format);
> > + if (fb->tiling || fb->size || fb->strides[0] ||
> > igt_format_is_yuv(fb->drm_format)) {
> > + uint64_t size;
> >
> > - if (offsets)
> > - memset(offsets, 0, ARRAY_SIZE(format->plane_bpp) *
> > sizeof(*offsets));
> > + size = calc_fb_size(fb);
> >
> > - if (tiling || size || stride || igt_format_is_yuv(format-
> > >drm_id)) {
> > - uint64_t calculated_size;
> > - uint32_t strides[4] = {
> > - stride,
> > - };
> > + /* respect the size requested by the caller */
> > + if (fb->size == 0)
> > + fb->size = size;
> >
> > - calculated_size = calc_fb_size(fd, width, height,
> > - format, tiling,
> > - strides, offsets);
> > -
> > - if (stride == 0)
> > - stride = strides[0];
> > - if (size == 0)
> > - size = calculated_size;
> > -
> > - if (is_dumb)
> > - *is_dumb = false;
> > + fb->is_dumb = false;
> >
> > if (is_i915_device(fd)) {
> > void *ptr;
> > - bool full_range = color_range ==
> > IGT_COLOR_YCBCR_FULL_RANGE;
> > + bool full_range = fb->color_range ==
> > IGT_COLOR_YCBCR_FULL_RANGE;
> >
> > - bo = gem_create(fd, size);
> > - gem_set_tiling(fd, bo,
> > igt_fb_mod_to_tiling(tiling), stride);
> > + fb->gem_handle = gem_create(fd, fb->size);
> >
> > - gem_set_domain(fd, bo,
> > + gem_set_tiling(fd, fb->gem_handle,
> > + igt_fb_mod_to_tiling(fb-
> > >tiling),
> > + fb->strides[0]);
> > +
> > + gem_set_domain(fd, fb->gem_handle,
> > I915_GEM_DOMAIN_GTT,
> > I915_GEM_DOMAIN_GTT);
> >
> > /* Ensure the framebuffer is preallocated */
> > - ptr = gem_mmap__gtt(fd, bo, size, PROT_READ
> > | PROT_WRITE);
> > + ptr = gem_mmap__gtt(fd, fb->gem_handle,
> > + fb->size, PROT_READ |
> > PROT_WRITE);
> > igt_assert(*(uint32_t *)ptr == 0);
> >
> > - switch (format->drm_id) {
> > + switch (fb->drm_format) {
> > case DRM_FORMAT_NV12:
> > - memset(ptr + offsets[0], full_range
> > ? 0x00 : 0x10,
> > - strides[0] * height);
> > - memset(ptr + offsets[1], 0x80,
> > - strides[1] * height/2);
> > + memset(ptr + fb->offsets[0],
> > + full_range ? 0x00 : 0x10,
> > + fb->strides[0] * fb-
> > >plane_height[0]);
> > + memset(ptr + fb->offsets[1],
> > + 0x80,
> > + fb->strides[1] * fb-
> > >plane_height[1]);
> > break;
> > case DRM_FORMAT_YUYV:
> > case DRM_FORMAT_YVYU:
> > - wmemset(ptr, full_range ? 0x80008000
> > : 0x80108010,
> > - strides[0] * height /
> > sizeof(wchar_t));
> > + wmemset(ptr + fb->offsets[0],
> > + full_range ? 0x80008000 :
> > 0x80108010,
> > + fb->strides[0] * fb-
> > >plane_height[0] / sizeof(wchar_t));
> > break;
> > case DRM_FORMAT_UYVY:
> > case DRM_FORMAT_VYUY:
> > - wmemset(ptr, full_range ? 0x00800080
> > : 0x10801080,
> > - strides[0] * height /
> > sizeof(wchar_t));
> > + wmemset(ptr + fb->offsets[0],
> > + full_range ? 0x00800080 :
> > 0x10801080,
> > + fb->strides[0] * fb-
> > >plane_height[0] / sizeof(wchar_t));
> > break;
> > }
> > - gem_munmap(ptr, size);
> > + gem_munmap(ptr, fb->size);
> >
> > - if (size_ret)
> > - *size_ret = size;
> > -
> > - if (stride_ret)
> > - *stride_ret = strides[0];
> > -
> > - return bo;
> > + return fb->gem_handle;
> > } else {
> > bool driver_has_gem_api = false;
> >
> > @@ -480,12 +476,13 @@ static int create_bo_for_fb(int fd, int width,
> > int height,
> > return -EINVAL;
> > }
> > } else {
> > - if (is_dumb)
> > - *is_dumb = true;
> > + fb->is_dumb = true;
> >
> > - return kmstest_dumb_create(fd, width, height,
> > - fb_plane_bpp(format, 0),
> > - stride_ret, size_ret);
> > + fb->gem_handle = kmstest_dumb_create(fd, fb->width,
> > fb->height,
> > + fb-
> > >plane_bpp[0],
> > + &fb-
> > >strides[0], &fb->size);
> > +
> > + return fb->gem_handle;
> > }
> > }
> >
> > @@ -512,11 +509,24 @@ int igt_create_bo_with_dimensions(int fd, int
> > width, int height,
> > unsigned stride, uint64_t
> > *size_ret,
> > unsigned *stride_ret, bool
> > *is_dumb)
> > {
> > - return create_bo_for_fb(fd, width, height,
> > - IGT_COLOR_YCBCR_BT709,
> > - IGT_COLOR_YCBCR_LIMITED_RANGE,
> > - lookup_drm_format(format),
> > - modifier, 0, stride, size_ret,
> > stride_ret, NULL, is_dumb);
> > + struct igt_fb fb;
> > +
> > + fb_init(&fb, fd, width, height, format, modifier,
> > + IGT_COLOR_YCBCR_BT709,
> > IGT_COLOR_YCBCR_LIMITED_RANGE);
> > +
> > + for (int i = 0; i < fb.num_planes; i++)
> > + fb.strides[i] = stride;
> > +
> > + create_bo_for_fb(&fb);
> > +
> > + if (size_ret)
> > + *size_ret = fb.size;
> > + if (stride_ret)
> > + *stride_ret = fb.strides[0];
> > + if (is_dumb)
> > + *is_dumb = fb.is_dumb;
> > +
> > + return fb.gem_handle;
> > }
> >
> > /**
> > @@ -860,52 +870,32 @@ igt_create_fb_with_bo_size(int fd, int width,
> > int height,
> > /* FIXME allow the caller to pass these in */
> > enum igt_color_encoding color_encoding =
> > IGT_COLOR_YCBCR_BT709;
> > enum igt_color_range color_range =
> > IGT_COLOR_YCBCR_LIMITED_RANGE;
> > - const struct format_desc_struct *f =
> > lookup_drm_format(format);
> > - uint32_t pitches[4];
> > - uint32_t fb_id;
> > - int i;
> >
> > - igt_assert_f(f, "DRM format %08x not found\n", format);
> > + fb_init(fb, fd, width, height, format, tiling,
> > + color_encoding, color_range);
> >
> > - memset(fb, 0, sizeof(*fb));
> > + for (int i = 0; i < fb->num_planes; i++)
> > + fb->strides[i] = bo_stride;
> > +
> > + fb->size = bo_size;
> >
> > igt_debug("%s(width=%d, height=%d, format=0x%x,
> > tiling=0x%"PRIx64", size=%"PRIu64")\n",
> > __func__, width, height, format, tiling, bo_size);
> > - fb->gem_handle = create_bo_for_fb(fd, width, height,
> > - color_encoding,
> > color_range,
> > - f, tiling, bo_size,
> > bo_stride,
> > - &fb->size, &fb->stride,
> > - fb->offsets, &fb-
> > >is_dumb);
> > +
> > + create_bo_for_fb(fb);
> > igt_assert(fb->gem_handle > 0);
> >
> > igt_debug("%s(handle=%d, pitch=%d)\n",
> > - __func__, fb->gem_handle, fb->stride);
> > + __func__, fb->gem_handle, fb->strides[0]);
> >
> > - for (i = 0; i < fb_num_planes(f); i++)
> > - pitches[i] = fb->stride;
> > + do_or_die(__kms_addfb(fb->fd, fb->gem_handle,
> > + fb->width, fb->height,
> > + fb->drm_format, fb->tiling,
> > + fb->strides, fb->offsets, fb-
> > >num_planes,
> > + LOCAL_DRM_MODE_FB_MODIFIERS,
> > + &fb->fb_id));
> >
> > - do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
> > - format, tiling, pitches, fb->offsets,
> > - fb_num_planes(f),
> > - LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
> > -
> > - fb->width = width;
> > - fb->height = height;
> > - fb->tiling = tiling;
> > - fb->drm_format = format;
> > - fb->fb_id = fb_id;
> > - fb->fd = fd;
> > - fb->num_planes = fb_num_planes(f);
> > - fb->color_encoding = color_encoding;
> > - fb->color_range = color_range;
> > -
> > - for (i = 0; i < fb_num_planes(f); i++) {
> > - fb->plane_bpp[i] = fb_plane_bpp(f, i);
> > - fb->plane_height[i] = fb_plane_height(f, height, i);
> > - fb->plane_width[i] = fb_plane_width(f, width, i);
> > - }
> > -
> > - return fb_id;
> > + return fb->fb_id;
> > }
> >
> > /**
> > @@ -1211,12 +1201,8 @@ static cairo_format_t
> > drm_format_to_cairo(uint32_t drm_format)
> > }
> >
> > struct fb_blit_linear {
> > - uint64_t size;
> > - uint32_t handle;
> > - unsigned int stride;
> > + struct igt_fb fb;
> > uint8_t *map;
> > - bool is_dumb;
> > - uint32_t offsets[4];
> > };
> >
> > struct fb_blit_upload {
> > @@ -1233,27 +1219,27 @@ static void free_linear_mapping(struct
> > fb_blit_upload *blit)
> > unsigned int obj_tiling = igt_fb_mod_to_tiling(fb->tiling);
> > int i;
> >
> > - gem_munmap(linear->map, linear->size);
> > - gem_set_domain(fd, linear->handle,
> > + gem_munmap(linear->map, linear->fb.size);
> > + gem_set_domain(fd, linear->fb.gem_handle,
> > I915_GEM_DOMAIN_GTT, 0);
> >
> > for (i = 0; i < fb->num_planes; i++)
> > igt_blitter_fast_copy__raw(fd,
> > - linear->handle,
> > - linear->offsets[i],
> > - linear->stride,
> > + linear->fb.gem_handle,
> > + linear->fb.offsets[i],
> > + linear->fb.strides[i],
> > I915_TILING_NONE,
> > 0, 0, /* src_x, src_y */
> > fb->plane_width[i], fb-
> > >plane_height[i],
> > fb->plane_bpp[i],
> > fb->gem_handle,
> > fb->offsets[i],
> > - fb->stride,
> > + fb->strides[i],
> > obj_tiling,
> > 0, 0 /* dst_x, dst_y */);
> >
> > - gem_sync(fd, linear->handle);
> > - gem_close(fd, linear->handle);
> > + gem_sync(fd, linear->fb.gem_handle);
> > + gem_close(fd, linear->fb.gem_handle);
> > }
> >
> > static void destroy_cairo_surface__blit(void *arg)
> > @@ -1277,42 +1263,42 @@ static void setup_linear_mapping(int fd,
> > struct igt_fb *fb, struct fb_blit_linea
> > * cairo). This linear bo will be then blitted to its final
> > * destination, tiling it at the same time.
> > */
> > - linear->handle = create_bo_for_fb(fd, fb->width, fb->height,
> > - fb->color_encoding, fb-
> > >color_range,
> > - lookup_drm_format(fb-
> > >drm_format),
> > - LOCAL_DRM_FORMAT_MOD_NONE,
> > 0,
> > - 0, &linear->size,
> > - &linear->stride,
> > - linear->offsets, &linear-
> > >is_dumb);
> >
> > - igt_assert(linear->handle > 0);
> > + fb_init(&linear->fb, fb->fd, fb->width, fb->height,
> > + fb->drm_format, LOCAL_DRM_FORMAT_MOD_NONE,
> > + fb->color_encoding, fb->color_range);
> > +
> > + create_bo_for_fb(&linear->fb);
> > +
> > + igt_assert(linear->fb.gem_handle > 0);
> >
> > /* Copy fb content to linear BO */
> > - gem_set_domain(fd, linear->handle,
> > + gem_set_domain(fd, linear->fb.gem_handle,
> > I915_GEM_DOMAIN_GTT, 0);
> >
> > for (i = 0; i < fb->num_planes; i++)
> > igt_blitter_fast_copy__raw(fd,
> > - fb->gem_handle,
> > - fb->offsets[i],
> > - fb->stride,
> > - obj_tiling,
> > - 0, 0, /* src_x, src_y */
> > - fb->plane_width[i], fb-
> > >plane_height[i],
> > - fb->plane_bpp[i],
> > - linear->handle, linear-
> > >offsets[i],
> > - linear->stride,
> > - I915_TILING_NONE,
> > - 0, 0 /* dst_x, dst_y */);
> > + fb->gem_handle,
> > + fb->offsets[i],
> > + fb->strides[i],
> > + obj_tiling,
> > + 0, 0, /* src_x, src_y */
> > + fb->plane_width[i], fb-
> > >plane_height[i],
> > + fb->plane_bpp[i],
> > + linear->fb.gem_handle,
> > + linear->fb.offsets[i],
> > + linear->fb.strides[i],
> > + I915_TILING_NONE,
> > + 0, 0 /* dst_x, dst_y */);
> >
> > - gem_sync(fd, linear->handle);
> > + gem_sync(fd, linear->fb.gem_handle);
> >
> > - gem_set_domain(fd, linear->handle,
> > + gem_set_domain(fd, linear->fb.gem_handle,
> > I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
> >
> > /* Setup cairo context */
> > - linear->map = gem_mmap__cpu(fd, linear->handle,
> > - 0, linear->size, PROT_READ |
> > PROT_WRITE);
> > + linear->map = gem_mmap__cpu(fd, linear->fb.gem_handle,
> > + 0, linear->fb.size, PROT_READ |
> > PROT_WRITE);
> > }
> >
> > static void create_cairo_surface__blit(int fd, struct igt_fb *fb)
> > @@ -1332,7 +1318,7 @@ static void create_cairo_surface__blit(int fd,
> > struct igt_fb *fb)
> > cairo_image_surface_create_for_data(blit-
> > >linear.map,
> > cairo_format,
> > fb->width, fb-
> > >height,
> > - blit-
> > >linear.stride);
> > + blit-
> > >linear.fb.strides[0]);
> > fb->domain = I915_GEM_DOMAIN_GTT;
> >
> > cairo_surface_set_user_data(fb->cairo_surface,
> > @@ -1382,7 +1368,7 @@ static void create_cairo_surface__gtt(int fd,
> > struct igt_fb *fb)
> > fb->cairo_surface =
> > cairo_image_surface_create_for_data(ptr,
> > drm_format_to_ca
> > iro(fb->drm_format),
> > - fb->width, fb-
> > >height, fb->stride);
> > + fb->width, fb-
> > >height, fb->strides[0]);
> > fb->domain = I915_GEM_DOMAIN_GTT;
> >
> > cairo_surface_set_user_data(fb->cairo_surface,
> > @@ -1425,8 +1411,8 @@ static void convert_nv12_to_rgb24(struct igt_fb
> > *fb, struct fb_convert_blit_uplo
> > int i, j;
> > const uint8_t *y, *uv;
> > uint8_t *rgb24 = blit->rgb24.map;
> > - unsigned rgb24_stride = blit->rgb24.stride, planar_stride =
> > blit->base.linear.stride;
> > - uint8_t *buf = malloc(blit->base.linear.size);
> > + unsigned rgb24_stride = blit->rgb24.stride, planar_stride =
> > blit->base.linear.fb.strides[0];
> > + uint8_t *buf = malloc(blit->base.linear.fb.size);
> > struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb-
> > >color_encoding,
> > fb-
> > >color_range);
> >
> > @@ -1435,9 +1421,9 @@ static void convert_nv12_to_rgb24(struct igt_fb
> > *fb, struct fb_convert_blit_uplo
> > * it's faster to copy the whole BO to a temporary buffer
> > and convert
> > * from there.
> > */
> > - igt_memcpy_from_wc(buf, blit->base.linear.map, blit-
> > >base.linear.size);
> > - y = &buf[blit->base.linear.offsets[0]];
> > - uv = &buf[blit->base.linear.offsets[1]];
> > + igt_memcpy_from_wc(buf, blit->base.linear.map, blit-
> > >base.linear.fb.size);
> > + y = &buf[blit->base.linear.fb.offsets[0]];
> > + uv = &buf[blit->base.linear.fb.offsets[1]];
> >
> > for (i = 0; i < fb->height / 2; i++) {
> > for (j = 0; j < fb->width / 2; j++) {
> > @@ -1531,11 +1517,11 @@ static void convert_nv12_to_rgb24(struct
> > igt_fb *fb, struct fb_convert_blit_uplo
> > static void convert_rgb24_to_nv12(struct igt_fb *fb, struct
> > fb_convert_blit_upload *blit)
> > {
> > int i, j;
> > - uint8_t *y = &blit->base.linear.map[blit-
> > >base.linear.offsets[0]];
> > - uint8_t *uv = &blit->base.linear.map[blit-
> > >base.linear.offsets[1]];
> > + uint8_t *y = &blit->base.linear.map[blit-
> > >base.linear.fb.offsets[0]];
> > + uint8_t *uv = &blit->base.linear.map[blit-
> > >base.linear.fb.offsets[1]];
> > const uint8_t *rgb24 = blit->rgb24.map;
> > unsigned rgb24_stride = blit->rgb24.stride;
> > - unsigned planar_stride = blit->base.linear.stride;
> > + unsigned planar_stride = blit->base.linear.fb.strides[0];
> > struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb-
> > >color_encoding,
> > fb-
> > >color_range);
> >
> > @@ -1662,8 +1648,8 @@ static void convert_yuyv_to_rgb24(struct igt_fb
> > *fb, struct fb_convert_blit_uplo
> > int i, j;
> > const uint8_t *yuyv;
> > uint8_t *rgb24 = blit->rgb24.map;
> > - unsigned rgb24_stride = blit->rgb24.stride, yuyv_stride =
> > blit->base.linear.stride;
> > - uint8_t *buf = malloc(blit->base.linear.size);
> > + unsigned rgb24_stride = blit->rgb24.stride, yuyv_stride =
> > blit->base.linear.fb.strides[0];
> > + uint8_t *buf = malloc(blit->base.linear.fb.size);
> > struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(fb-
> > >color_encoding,
> > fb-
> > >color_range);
> >
> > @@ -1672,7 +1658,7 @@ static void convert_yuyv_to_rgb24(struct igt_fb
> > *fb, struct fb_convert_blit_uplo
> > * it's faster to copy the whole BO to a temporary buffer
> > and convert
> > * from there.
> > */
> > - igt_memcpy_from_wc(buf, blit->base.linear.map, blit-
> > >base.linear.size);
> > + igt_memcpy_from_wc(buf, blit->base.linear.map, blit-
> > >base.linear.fb.size);
> > yuyv = buf;
> >
> > for (i = 0; i < fb->height; i++) {
> > @@ -1722,7 +1708,7 @@ static void convert_rgb24_to_yuyv(struct igt_fb
> > *fb, struct fb_convert_blit_uplo
> > uint8_t *yuyv = blit->base.linear.map;
> > const uint8_t *rgb24 = blit->rgb24.map;
> > unsigned rgb24_stride = blit->rgb24.stride;
> > - unsigned yuyv_stride = blit->base.linear.stride;
> > + unsigned yuyv_stride = blit->base.linear.fb.strides[0];
> > struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(fb-
> > >color_encoding,
> > fb-
> > >color_range);
> >
> > @@ -1791,7 +1777,7 @@ static void destroy_cairo_surface__convert(void
> > *arg)
> >
> > munmap(blit->rgb24.map, blit->rgb24.size);
> >
> > - if (blit->base.linear.handle)
> > + if (blit->base.linear.fb.gem_handle)
> > free_linear_mapping(&blit->base);
> > else
> > gem_munmap(blit->base.linear.map, fb->size);
> > @@ -1817,15 +1803,15 @@ static void create_cairo_surface__convert(int
> > fd, struct igt_fb *fb)
> > fb->tiling == LOCAL_I915_FORMAT_MOD_Yf_TILED) {
> > setup_linear_mapping(fd, fb, &blit->base.linear);
> > } else {
> > - blit->base.linear.handle = 0;
> > + blit->base.linear.fb.gem_handle = 0;
> > gem_set_domain(fd, fb->gem_handle,
> > I915_GEM_DOMAIN_GTT,
> > I915_GEM_DOMAIN_GTT);
> > blit->base.linear.map = gem_mmap__gtt(fd, fb-
> > >gem_handle, fb->size,
> > PROT_READ |
> > PROT_WRITE);
> > igt_assert(blit->base.linear.map);
> > - blit->base.linear.stride = fb->stride;
> > - blit->base.linear.size = fb->size;
> > - memcpy(blit->base.linear.offsets, fb->offsets,
> > sizeof(fb->offsets));
> > + blit->base.linear.fb.size = fb->size;
> > + memcpy(blit->base.linear.fb.strides, fb->strides,
> > sizeof(fb->strides));
> > + memcpy(blit->base.linear.fb.offsets, fb->offsets,
> > sizeof(fb->offsets));
> > }
> >
> > /* Convert to linear rgb! */
> > diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> > index 2343fe505f28..35bf307a930b 100644
> > --- a/lib/igt_fb.h
> > +++ b/lib/igt_fb.h
> > @@ -47,12 +47,12 @@
> > * @drm_format: DRM FOURCC code
> > * @width: width in pixels
> > * @height: height in pixels
> > - * @stride: line stride in bytes
> > * @tiling: tiling mode as a DRM framebuffer modifier
> > * @size: size in bytes of the underlying backing storage
> > * @cairo_surface: optionally attached cairo drawing surface
> > * @domain: current domain for cache flushing tracking on i915.ko
> > * @num_planes: Amount of planes on this fb. >1 for planar formats.
> > + * @strides: line stride for each plane in bytes
> > * @offsets: Offset for each plane in bytes.
> > * @plane_bpp: The bpp for each plane.
> > * @plane_width: The width for each plane.
> > @@ -70,12 +70,12 @@ typedef struct igt_fb {
> > int height;
> > enum igt_color_encoding color_encoding;
> > enum igt_color_range color_range;
> > - unsigned int stride;
> > uint64_t tiling;
> > uint64_t size;
> > cairo_surface_t *cairo_surface;
> > unsigned int domain;
> > unsigned int num_planes;
> > + uint32_t strides[4];
> > uint32_t offsets[4];
> > unsigned int plane_bpp[4];
> > unsigned int plane_width[4];
> > diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
> > index e1ee58801ac3..e03f947eae11 100644
> > --- a/tests/kms_ccs.c
> > +++ b/tests/kms_ccs.c
> > @@ -378,7 +378,7 @@ static void generate_fb(data_t *data, struct
> > igt_fb *fb,
> > fb->drm_format = f.pixel_format;
> > fb->width = f.width;
> > fb->height = f.height;
> > - fb->stride = f.pitches[0];
> > + fb->strides[0] = f.pitches[0];
> > fb->tiling = f.modifier[0];
> > fb->size = size[0];
> > fb->cairo_surface = NULL;
> > diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> > index 393d690ab535..f7d08a60aeea 100644
> > --- a/tests/kms_flip.c
> > +++ b/tests/kms_flip.c
> > @@ -592,7 +592,7 @@ static void recreate_fb(struct test_output *o)
> > igt_assert(r);
> >
> > do_or_die(drmModeAddFB(drm_fd, o->fb_width, o->fb_height, o-
> > >depth,
> > - o->bpp, fb_info->stride,
> > + o->bpp, fb_info->strides[0],
> > r->handle, &new_fb_id));
> >
> > gem_close(drm_fd, r->handle);
> > @@ -612,7 +612,7 @@ static void set_y_tiling(struct test_output *o,
> > int fb_idx)
> > r = drmModeGetFB(drm_fd, fb_info->fb_id);
> > igt_assert(r);
> > /* Newer kernels don't allow such shenagians any more, so
> > skip the test. */
> > - igt_require(__gem_set_tiling(drm_fd, r->handle,
> > I915_TILING_Y, fb_info->stride) == 0);
> > + igt_require(__gem_set_tiling(drm_fd, r->handle,
> > I915_TILING_Y, fb_info->strides[0]) == 0);
> > gem_close(drm_fd, r->handle);
> > drmFree(r);
> > }
> > diff --git a/tests/kms_frontbuffer_tracking.c
> > b/tests/kms_frontbuffer_tracking.c
> > index 1bce676081b4..265c313a3886 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -1157,7 +1157,7 @@ static void start_busy_thread(struct igt_fb
> > *fb)
> > busy_thread.stop = false;
> > busy_thread.handle = fb->gem_handle;
> > busy_thread.size = fb->size;
> > - busy_thread.stride = fb->stride;
> > + busy_thread.stride = fb->strides[0];
> > busy_thread.width = fb->width;
> > busy_thread.height = fb->height;
> > busy_thread.color = pick_color(fb, COLOR_PRIM_BG);
> > @@ -2859,7 +2859,7 @@ static void badstride_subtest(const struct
> > test_mode *t)
> >
> > create_fb(t->format, params->primary.fb->width + 4096,
> > params->primary.fb->height,
> > opt.tiling, t->plane, &wide_fb);
> > - igt_assert(wide_fb.stride > 16384);
> > + igt_assert(wide_fb.strides[0] > 16384);
> >
> > fill_fb(&wide_fb, COLOR_PRIM_BG);
> >
> > @@ -2928,7 +2928,7 @@ static void stridechange_subtest(const struct
> > test_mode *t)
> > opt.tiling, t->plane, &new_fb);
> > fill_fb(&new_fb, COLOR_PRIM_BG);
> >
> > - igt_assert(old_fb->stride != new_fb.stride);
> > + igt_assert(old_fb->strides[0] != new_fb.strides[0]);
> >
> > /* We can't assert that FBC will be enabled since there may
> > not be
> > * enough space for the CFB, but we can check the CRC. */
> > diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
> > index c24fd95bb0f3..2d6c5b49c9d5 100644
> > --- a/tests/pm_rpm.c
> > +++ b/tests/pm_rpm.c
> > @@ -1547,7 +1547,7 @@ static void cursor_subtest(bool dpms)
> > * hopefully it has some fences around it. */
> > rc = drmModeRmFB(drm_fd, cursor_fb3.fb_id);
> > igt_assert_eq(rc, 0);
> > - gem_set_tiling(drm_fd, cursor_fb3.gem_handle, false,
> > cursor_fb3.stride);
> > + gem_set_tiling(drm_fd, cursor_fb3.gem_handle, false,
> > cursor_fb3.strides[0]);
> > igt_assert(wait_for_suspended());
> >
> > rc = drmModeSetCursor(drm_fd, crtc_id,
> > cursor_fb3.gem_handle,
--
Ville Syrjälä
Intel
More information about the igt-dev
mailing list