On Fri, 2019-04-12 at 19:38 +0200, Lucas Stach wrote:
> The 2D pipe is useful to implement fast planar and interleaved YUV buffer
> imports. Not all systems have a 2D capable core, so this is strictly
> optional.
>
> Signed-off-by: Lucas Stach <l.st...@pengutronix.de>
> ---
> src/gallium/drivers/etnaviv/etnaviv_context.c | 6 ++
> src/gallium/drivers/etnaviv/etnaviv_context.h | 1 +
> src/gallium/drivers/etnaviv/etnaviv_screen.c | 68 +++++++++++++++++++
> src/gallium/drivers/etnaviv/etnaviv_screen.h | 6 ++
> 4 files changed, 81 insertions(+)
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_context.c
> b/src/gallium/drivers/etnaviv/etnaviv_context.c
> index a59338490b62..631f551d0ad4 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_context.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_context.c
> @@ -78,6 +78,9 @@ etna_context_destroy(struct pipe_context *pctx)
> if (ctx->stream)
> etna_cmd_stream_del(ctx->stream);
>
> + if (ctx->stream2d)
> + etna_cmd_stream_del(ctx->stream2d);
> +
> slab_destroy_child(&ctx->transfer_pool);
>
> if (ctx->in_fence_fd != -1)
> @@ -434,6 +437,9 @@ etna_context_create(struct pipe_screen *pscreen, void
> *priv, unsigned flags)
> if (ctx->stream == NULL)
> goto fail;
>
> + if (screen->pipe2d)
> + ctx->stream2d = etna_cmd_stream_new(screen->pipe2d, 0x1000, NULL,
> NULL);
> +
> /* context ctxate setup */
> ctx->specs = screen->specs;
> ctx->screen = screen;
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_context.h
> b/src/gallium/drivers/etnaviv/etnaviv_context.h
> index a79d739100d9..2c6e5d6c3db1 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_context.h
> +++ b/src/gallium/drivers/etnaviv/etnaviv_context.h
> @@ -110,6 +110,7 @@ struct etna_context {
> struct etna_specs specs;
> struct etna_screen *screen;
> struct etna_cmd_stream *stream;
> + struct etna_cmd_stream *stream2d;
>
> /* which state objects need to be re-emit'd: */
> enum {
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c
> b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> index 62b6f1c80fae..0dea6056c75a 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> @@ -95,6 +95,12 @@ etna_screen_destroy(struct pipe_screen *pscreen)
> if (screen->gpu)
> etna_gpu_del(screen->gpu);
>
> + if (screen->pipe2d)
> + etna_pipe_del(screen->pipe2d);
> +
> + if (screen->gpu2d)
> + etna_gpu_del(screen->gpu2d);
> +
> if (screen->ro)
> FREE(screen->ro);
>
> @@ -891,6 +897,66 @@ etna_screen_bo_from_handle(struct pipe_screen *pscreen,
> return bo;
> }
>
> +static void etna_screen_init_2d(struct etna_screen *screen)
> +{
> + struct etna_gpu *gpu2d = NULL;
> + uint64_t val;
> + int ret, i;
> +
> + /* If the current GPU is a combined 2d/3D core, use it as 2D engine */
^ 2D
> + if (screen->features[0] & chipFeatures_PIPE_2D)
> + gpu2d = screen->gpu;
If the GPU is a combined 2D/3D core, is screen->gpu2d set anywhere?
I assume it isn't on purpose, so it can be used to check whether
etna_gpu_del(screen->gpu2d) must be called in etna_screen_destroy().
> +
> + /* otherwise search for a 2D capable core */
> + if (!gpu2d) {
This could just be
else {
/* otherwise search for a 2D capable core */
> + for (i = 0;; i++) {
> + gpu2d = etna_gpu_new(screen->dev, i);
> + if (!gpu2d)
> + return;
> +
> + ret = etna_gpu_get_param(gpu2d, ETNA_GPU_FEATURES_0, &val);
> + if (!ret && (val & chipFeatures_PIPE_2D)) {
> + screen->gpu2d = gpu2d;
> + break;
> + }
> +
> + etna_gpu_del(gpu2d);
> + }
> + }
> +
> + if (etna_gpu_get_param(screen->gpu2d, ETNA_GPU_FEATURES_0, &val))
If the GPU is a combined 2D/3D core, and screen->gpu2d == NULL at this
point, won't this fail trying to dereference the gpu NULL pointer in
etna_gpu_get_param()? I suppose these should be using the local gpu2d
variable instead of screen->gpu2d.
Further, if the GPU is a combined 2D/3D core, we have already queried
GPU features just before etna_screen_init_2d() was called,
in etna_screen_create(). We could just copy features to features2d.
Nitpick: if the gpu2d is 2D-only, the only way we can arrive here is
from the break in the loop above, just after val was already set to the
ETNA_GPU_FEATURES_0 value. In that case the first call to
etna_gpu_get_parm() is superfluous.
> + return;
> + screen->features2d[0] = val;
> +
> + if (etna_gpu_get_param(screen->gpu2d, ETNA_GPU_FEATURES_1, &val))
> + return;
> + screen->features2d[1] = val;
> +
> + if (etna_gpu_get_param(screen->gpu2d, ETNA_GPU_FEATURES_2, &val))
> + return;
> + screen->features2d[2] = val;
> +
> + if (etna_gpu_get_param(screen->gpu2d, ETNA_GPU_FEATURES_3, &val))
> + return;
> + screen->features2d[3] = val;
> +
> + if (etna_gpu_get_param(screen->gpu2d, ETNA_GPU_FEATURES_4, &val))
> + return;
> + screen->features2d[4] = val;
> +
> + if (etna_gpu_get_param(screen->gpu2d, ETNA_GPU_FEATURES_5, &val))
> + return;
> + screen->features2d[5] = val;
> +
> + if (etna_gpu_get_param(screen->gpu2d, ETNA_GPU_FEATURES_6, &val))
> + return;
> + screen->features2d[6] = val;
> +
> + screen->pipe2d = etna_pipe_new(gpu2d, ETNA_PIPE_2D);
> + if (!screen->pipe2d)
> + DBG("could not create 2d pipe");
> +}
> +
> struct pipe_screen *
> etna_screen_create(struct etna_device *dev, struct etna_gpu *gpu,
> struct renderonly *ro)
> @@ -984,6 +1050,8 @@ etna_screen_create(struct etna_device *dev, struct
> etna_gpu *gpu,
> }
> screen->features[6] = val;
>
> + etna_screen_init_2d(screen);
> +
> if (!etna_get_specs(screen))
> goto fail;
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.h
> b/src/gallium/drivers/etnaviv/etnaviv_screen.h
> index 9757985526ec..82733a379430 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_screen.h
> +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.h
> @@ -60,6 +60,9 @@ enum viv_features_word {
> #define VIV_FEATURE(screen, word, feature) \
> ((screen->features[viv_ ## word] & (word ## _ ## feature)) != 0)
>
> +#define VIV_2D_FEATURE(screen, word, feature) \
> + ((screen->features2d[viv_ ## word] & (word ## _ ## feature)) != 0)
> +
> struct etna_screen {
> struct pipe_screen base;
>
> @@ -68,7 +71,9 @@ struct etna_screen {
>
> struct etna_device *dev;
> struct etna_gpu *gpu;
> + struct etna_gpu *gpu2d;
> struct etna_pipe *pipe;
> + struct etna_pipe *pipe2d;
> struct etna_perfmon *perfmon;
> struct renderonly *ro;
>
> @@ -78,6 +83,7 @@ struct etna_screen {
> uint32_t model;
> uint32_t revision;
> uint32_t features[VIV_FEATURES_WORD_COUNT];
> + uint32_t features2d[VIV_FEATURES_WORD_COUNT];
>
> struct etna_specs specs;
>
regards
Philipp
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
