Hi Simon,
Thank you very much for your review. I'll fix the issue in the next version.
On Mon, May 20, 2019 at 3:22 AM Ser, Simon <simon.ser@xxxxxxxxx> wrote:
>
> Hi Rodrigo,
>
> On Sun, 2019-05-19 at 20:00 -0300, Rodrigo Siqueira wrote:
> > The function __kms_addfb() and drmModeAddFB2WithModifiers() have a
> > similar code. Due to this similarity, this commit replaces all the
> > occurrences of __kms_addfb() by drmModeAddFB2WithModifiers() and adds
> > the required adaptations.
> >
> > V1: Arkadiusz Hiler
> > - Fix array initialization
> > - Drop __kms_addfb()
> >
> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@xxxxxxxxx>
> > ---
> > lib/igt_fb.c | 14 +++++++++-----
> > lib/ioctl_wrappers.c | 33 ---------------------------------
> > lib/ioctl_wrappers.h | 11 -----------
> > tests/kms_available_modes_crc.c | 14 +++++++++-----
> > tests/kms_draw_crc.c | 10 ++++++----
> > tests/prime_vgem.c | 14 ++++++++------
> > 6 files changed, 32 insertions(+), 64 deletions(-)
> >
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index d4929019..bac3b21c 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -1235,6 +1235,9 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
> > struct igt_fb *fb, uint64_t bo_size,
> > unsigned bo_stride)
> > {
> > + uint32_t handles[4] = {};
> > + uint64_t modifiers[4] = {};
> > +
> > /* 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;
> > @@ -1262,11 +1265,12 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
> > if (fb->modifier || igt_has_fb_modifiers(fd))
> > flags = LOCAL_DRM_MODE_FB_MODIFIERS;
> >
> > - do_or_die(__kms_addfb(fb->fd, fb->gem_handle,
> > - fb->width, fb->height,
> > - fb->drm_format, fb->modifier,
> > - fb->strides, fb->offsets, fb->num_planes, flags,
> > - &fb->fb_id));
> > + memset(handles, fb->gem_handle, fb->num_planes);
> > + memset(modifiers, modifier, fb->num_planes);
>
> memset is only able to fill bytes. This won't work for larger values.
>
> > + do_or_die(drmModeAddFB2WithModifiers(fb->fd, fb->width, fb->height,
> > + fb->drm_format, handles,
> > + fb->strides, fb->offsets,
> > + modifiers, &fb->fb_id, flags));
> >
> > return fb->fb_id;
> > }
> > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> > index 280fdd62..0f00971d 100644
> > --- a/lib/ioctl_wrappers.c
> > +++ b/lib/ioctl_wrappers.c
> > @@ -1453,36 +1453,3 @@ void igt_require_fb_modifiers(int fd)
> > {
> > igt_require(igt_has_fb_modifiers(fd));
> > }
> > -
> > -int __kms_addfb(int fd, uint32_t handle,
> > - uint32_t width, uint32_t height,
> > - uint32_t pixel_format, uint64_t modifier,
> > - uint32_t strides[4], uint32_t offsets[4],
> > - int num_planes, uint32_t flags, uint32_t *buf_id)
> > -{
> > - struct drm_mode_fb_cmd2 f;
> > - int ret, i;
> > -
> > - if (flags & DRM_MODE_FB_MODIFIERS)
> > - igt_require_fb_modifiers(fd);
> > -
> > - memset(&f, 0, sizeof(f));
> > -
> > - f.width = width;
> > - f.height = height;
> > - f.pixel_format = pixel_format;
> > - f.flags = flags;
> > -
> > - for (i = 0; i < num_planes; i++) {
> > - f.handles[i] = handle;
> > - f.modifier[i] = modifier;
> > - f.pitches[i] = strides[i];
> > - f.offsets[i] = offsets[i];
> > - }
> > -
> > - ret = igt_ioctl(fd, DRM_IOCTL_MODE_ADDFB2, &f);
> > -
> > - *buf_id = f.fb_id;
> > -
> > - return ret < 0 ? -errno : ret;
> > -}
> > diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> > index 03211c97..4afc3e09 100644
> > --- a/lib/ioctl_wrappers.h
> > +++ b/lib/ioctl_wrappers.h
> > @@ -209,17 +209,6 @@ struct local_drm_mode_fb_cmd2 {
> > bool igt_has_fb_modifiers(int fd);
> > void igt_require_fb_modifiers(int fd);
> >
> > -/**
> > - * __kms_addfb:
> > - *
> > - * Creates a framebuffer object.
> > - */
> > -int __kms_addfb(int fd, uint32_t handle,
> > - uint32_t width, uint32_t height,
> > - uint32_t pixel_format, uint64_t modifier,
> > - uint32_t strides[4], uint32_t offsets[4],
> > - int num_planes, uint32_t flags, uint32_t *buf_id);
> > -
> > /**
> > * to_user_pointer:
> > *
> > diff --git a/tests/kms_available_modes_crc.c b/tests/kms_available_modes_crc.c
> > index 50b5522a..9e5f1fd5 100644
> > --- a/tests/kms_available_modes_crc.c
> > +++ b/tests/kms_available_modes_crc.c
> > @@ -172,9 +172,10 @@ static bool setup_fb(data_t *data, igt_output_t *output, igt_plane_t *plane,
> > {
> > drmModeModeInfo *mode;
> > uint64_t w, h;
> > + uint32_t handles[4] = {};
> > + uint64_t modifiers[4] = {};
> > signed ret, gemsize = 0;
> > unsigned tile_width, tile_height;
> > - int num_planes = 1;
> > uint64_t tiling;
> > int bpp = 0;
> > int i;
> > @@ -225,10 +226,13 @@ static bool setup_fb(data_t *data, igt_output_t *output, igt_plane_t *plane,
> >
> > igt_assert_eq(ret, 0);
> >
> > - ret = __kms_addfb(data->gfx_fd, data->gem_handle, w, h,
> > - format, tiling, data->fb.strides, data->fb.offsets,
> > - num_planes, LOCAL_DRM_MODE_FB_MODIFIERS,
> > - &data->fb.fb_id);
> > + memset(handles, data->gem_handle, 1);
> > + memset(modifiers, tiling, 1);
>
> Ditto.
>
> > + ret = drmModeAddFB2WithModifiers(data->gfx_fd, w, h, format,
> > + handles, data->fb.strides,
> > + data->fb.offsets, modifiers,
> > + &data->fb.fb_id,
> > + LOCAL_DRM_MODE_FB_MODIFIERS);
> >
> > if(ret < 0) {
> > igt_info("Creating fb for format %s failed, return code %d\n",
> > diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
> > index ea14db9a..507e6a7c 100644
> > --- a/tests/kms_draw_crc.c
> > +++ b/tests/kms_draw_crc.c
> > @@ -155,17 +155,19 @@ static void get_method_crc(enum igt_draw_method method, uint32_t drm_format,
> >
> > static bool format_is_supported(uint32_t format, uint64_t modifier)
> > {
> > - uint32_t gem_handle, fb_id;
> > + uint32_t gem_handle, fb_id, handles[4] = {};
> > unsigned int offsets[4] = {};
> > unsigned int strides[4] = {};
> > + uint64_t modifiers[4] = {};
> > int ret;
> >
> > gem_handle = igt_create_bo_with_dimensions(drm_fd, 64, 64,
> > format, modifier,
> > 0, NULL, &strides[0], NULL);
> > - ret = __kms_addfb(drm_fd, gem_handle, 64, 64,
> > - format, modifier, strides, offsets, 1,
> > - LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id);
> > + memset(handles, gem_handle, 1);
>
> Ditto
>
> > + ret = drmModeAddFB2WithModifiers(drm_fd, 64, 64, format, handles,
> > + strides, offsets, modifiers, &fb_id,
> > + LOCAL_DRM_MODE_FB_MODIFIERS);
> > drmModeRmFB(drm_fd, fb_id);
> > gem_close(drm_fd, gem_handle);
> >
> > diff --git a/tests/prime_vgem.c b/tests/prime_vgem.c
> > index 09e3373b..286957e8 100644
> > --- a/tests/prime_vgem.c
> > +++ b/tests/prime_vgem.c
> > @@ -761,6 +761,7 @@ static void test_flip(int i915, int vgem, unsigned hang)
> > for (int i = 0; i < 2; i++) {
> > uint32_t strides[4] = {};
> > uint32_t offsets[4] = {};
> > + uint32_t handles[4] = {};
> > int fd;
> >
> > bo[i].width = mode->hdisplay;
> > @@ -776,12 +777,13 @@ static void test_flip(int i915, int vgem, unsigned hang)
> > strides[0] = bo[i].pitch;
> >
> > /* May skip if i915 has no displays */
> > - igt_require(__kms_addfb(i915, handle[i],
> > - bo[i].width, bo[i].height,
> > - DRM_FORMAT_XRGB8888, I915_TILING_NONE,
> > - strides, offsets, 1,
> > - LOCAL_DRM_MODE_FB_MODIFIERS,
> > - &fb_id[i]) == 0);
> > + memset(handles, handle[i],1);
>
> Ditto
>
> (Style: missing space after comma)
>
> > + igt_require(drmModeAddFB2WithModifiers(i915, bo[i].width, bo[i].height,
> > + DRM_FORMAT_XRGB8888,
> > + handles, strides, offsets,
> > + I915_TILING_NONE, &fb_id[i],
> > + LOCAL_DRM_MODE_FB_MODIFIERS) == 0);
> > +
> > igt_assert(fb_id[i]);
> > }
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
--
Rodrigo Siqueira
https://siqueira.tech
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
