[Mesa-dev] [PATCH 01/10] panfrost: Move scanout res creation out of panfrost_resource_create()

[Tomeu Vizoso]

Besides the leak, I think I preferred the open-coded version, which is
incidentally what all other drivers I've seen do. The modifiers work
is going to massively change this code anyway.

But I'm not against it, so

Reviewed-by: Tomeu Vizoso <tomeu.vizoso at collabora.com>

Cheers,

Tomeu

On Tue, 2 Jul 2019 at 15:24, Boris Brezillon
<boris.brezillon at collabora.com> wrote:
>
> Which improves readability and help us avoid a memory leak.
>
> Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> ---
>  src/gallium/drivers/panfrost/pan_resource.c | 79 ++++++++++++---------
>  1 file changed, 44 insertions(+), 35 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
> index 8db7e45af1b6..fae535ed4e29 100644
> --- a/src/gallium/drivers/panfrost/pan_resource.c
> +++ b/src/gallium/drivers/panfrost/pan_resource.c
> @@ -180,6 +180,37 @@ panfrost_surface_destroy(struct pipe_context *pipe,
>          ralloc_free(surf);
>  }
>
> +static struct pipe_resource *
> +panfrost_create_scanout_res(struct pipe_screen *screen,
> +                            const struct pipe_resource *template)
> +{
> +        struct panfrost_screen *pscreen = pan_screen(screen);
> +        struct pipe_resource scanout_templat = *template;
> +        struct renderonly_scanout *scanout;
> +        struct winsys_handle handle;
> +        struct pipe_resource *res;
> +
> +        scanout = renderonly_scanout_for_resource(&scanout_templat,
> +                                                  pscreen->ro, &handle);
> +        if (!scanout)
> +                return NULL;
> +
> +        assert(handle.type == WINSYS_HANDLE_TYPE_FD);
> +        /* TODO: handle modifiers? */
> +        res = screen->resource_from_handle(screen, template, &handle,
> +                                           PIPE_HANDLE_USAGE_FRAMEBUFFER_WRITE);
> +        close(handle.handle);
> +        if (!res)
> +                return NULL;
> +
> +        struct panfrost_resource *pres = pan_resource(res);
> +
> +        pres->scanout = scanout;
> +        pscreen->display_target = pres;
> +
> +        return res;
> +}
> +
>  /* Computes sizes for checksumming, which is 8 bytes per 16x16 tile */
>
>  #define CHECKSUM_TILE_WIDTH 16
> @@ -368,14 +399,6 @@ static struct pipe_resource *
>  panfrost_resource_create(struct pipe_screen *screen,
>                           const struct pipe_resource *template)
>  {
> -        struct panfrost_resource *so = rzalloc(screen, struct panfrost_resource);
> -        struct panfrost_screen *pscreen = (struct panfrost_screen *) screen;
> -
> -        so->base = *template;
> -        so->base.screen = screen;
> -
> -        pipe_reference_init(&so->base.reference, 1);
> -
>          /* Make sure we're familiar */
>          switch (template->target) {
>                  case PIPE_BUFFER:
> @@ -391,35 +414,21 @@ panfrost_resource_create(struct pipe_screen *screen,
>                          assert(0);
>          }
>
> +        if (template->bind &
> +            (PIPE_BIND_DISPLAY_TARGET | PIPE_BIND_SCANOUT | PIPE_BIND_SHARED))
> +                return panfrost_create_scanout_res(screen, template);
> +
> +        struct panfrost_resource *so = rzalloc(screen, struct panfrost_resource);
> +        struct panfrost_screen *pscreen = (struct panfrost_screen *) screen;
> +
> +        so->base = *template;
> +        so->base.screen = screen;
> +
> +        pipe_reference_init(&so->base.reference, 1);
> +
>          util_range_init(&so->valid_buffer_range);
>
> -        if (template->bind & PIPE_BIND_DISPLAY_TARGET ||
> -            template->bind & PIPE_BIND_SCANOUT ||
> -            template->bind & PIPE_BIND_SHARED) {
> -                struct pipe_resource scanout_templat = *template;
> -                struct renderonly_scanout *scanout;
> -                struct winsys_handle handle;
> -
> -                scanout = renderonly_scanout_for_resource(&scanout_templat,
> -                                                          pscreen->ro, &handle);
> -                if (!scanout)
> -                        return NULL;
> -
> -                assert(handle.type == WINSYS_HANDLE_TYPE_FD);
> -                /* TODO: handle modifiers? */
> -                so = pan_resource(screen->resource_from_handle(screen, template,
> -                                                                 &handle,
> -                                                                 PIPE_HANDLE_USAGE_FRAMEBUFFER_WRITE));
> -                close(handle.handle);
> -                if (!so)
> -                        return NULL;
> -
> -                so->scanout = scanout;
> -                pscreen->display_target = so;
> -        } else {
> -                so->bo = panfrost_create_bo(pscreen, template);
> -        }
> -
> +        so->bo = panfrost_create_bo(pscreen, template);
>          return (struct pipe_resource *)so;
>  }
>
> --
> 2.21.0
>