Re: [igt-dev] [PATCH i-g-t v5 09/13] igt: fb: Rework YUV i915 allocation path

Paul Kocialkowski
2019-01-25T15:34:49Z

Hi,

On Fri, 2019-01-25 at 15:58 +0100, Maxime Ripard wrote:
> We only need to allocate a buffer using the GEM API when we're in the
> condition that we currently test, and that we're running on i915.
>
> All the other cases can be handled by a fallback to a dumb buffer
> allocation. Let's simplify the code a bit to reflect that.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> ---
> lib/igt_fb.c | 25 ++++++++-----------------
> 1 file changed, 8 insertions(+), 17 deletions(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 65aa99ecf7ae..43e527d1a3dd 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -533,7 +533,8 @@ static int create_bo_for_fb(struct igt_fb *fb)
> unsigned *strides = &fb->strides[0];
> int fd = fb->fd;
>
> - if (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb->drm_format)) {
> + if (is_i915_device(fd) &&
> + (fb->tiling || fb->size || fb->strides[0] || igt_format_is_yuv(fb->drm_format))) {
> uint64_t size;
>
> size = calc_fb_size(fb);
> @@ -543,23 +544,12 @@ static int create_bo_for_fb(struct igt_fb *fb)
> fb->size = size;
>
> fb->is_dumb = false;
> + fb->gem_handle = gem_create(fd, fb->size);
> + gem_set_tiling(fd, fb->gem_handle,
> + igt_fb_mod_to_tiling(fb->tiling),
> + fb->strides[0]);
>
> - if (is_i915_device(fd)) {
> -
> - fb->gem_handle = gem_create(fd, fb->size);
> -
> - gem_set_tiling(fd, fb->gem_handle,
> - igt_fb_mod_to_tiling(fb->tiling),
> - fb->strides[0]);
> - clear_yuv_buffer(fd);
> -
> - return fb->gem_handle;
> - } else {
> - bool driver_has_gem_api = false;
> -
> - igt_require(driver_has_gem_api);
> - return -EINVAL;
> - }
> + goto out;
> }
>
> /*
> @@ -593,6 +583,7 @@ static int create_bo_for_fb(struct igt_fb *fb)
> fb->gem_handle = kmstest_dumb_create(fd, fb->width, fb->height,
> bpp, strides, &fb->size);
>
> +out:
> if (igt_format_is_yuv(fb->drm_format))
> clear_yuv_buffer(fb);
>
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering