[Mesa-dev] [PATCH v2 2/3] etnaviv: gallium driver for Vivante GPUs

[Christian Gmeiner]

Hi Emil,

thanks for your review!


2017-01-09 21:00 GMT+01:00 Emil Velikov <emil.l.velikov at gmail.com>:
> Hi Christian,
>
> There's a few nitpicks. Note that neither is a blocker so feel free to
> send patches in the week(s) to come.
>
> On 23 December 2016 at 22:04, Christian Gmeiner
> <christian.gmeiner at gmail.com> wrote:
>
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -76,6 +76,7 @@ LIBDRM_NVVIEUX_REQUIRED=2.4.66
>>  LIBDRM_NOUVEAU_REQUIRED=2.4.66
>>  LIBDRM_FREEDRENO_REQUIRED=2.4.74
>>  LIBDRM_VC4_REQUIRED=2.4.69
>> +LIBDRM_ETNAVIV_REQUIRED=2.4.74
>>  DRI2PROTO_REQUIRED=2.6
>>  DRI3PROTO_REQUIRED=1.0
>>  PRESENTPROTO_REQUIRED=1.0
>> @@ -2506,6 +2507,11 @@ if test -n "$with_gallium_drivers"; then
>>              PKG_CHECK_MODULES([FREEDRENO], [libdrm_freedreno >= $LIBDRM_FREEDRENO_REQUIRED])
>>              require_libdrm "freedreno"
>>              ;;
>> +        xetnaviv)
>> +            HAVE_GALLIUM_ETNAVIV=yes
>> +            PKG_CHECK_MODULES([ETNAVIV], [libdrm_etnaviv >= $LIBDRM_ETNAVIV_REQUIRED])
>> +            require_libdrm "etnaviv"
>> +            ;;
> - There's a comment/documentation missing why we want/need
> etnaviv_dri.so anywhere.
> Please add something - be that in configure or a readme file in
> {drivers,winsys}/etnaviv.
>
> Pretty much anything will do ;-)
>
> - This and 3/3 will need to update the "AC_ARG_WITH([gallium-drivers]"
> help string.
> Again - perfectly fine to address with separate commit.
>

Added something :)

>
>> --- /dev/null
>> +++ b/src/gallium/drivers/etnaviv/Makefile.am
>
>> +
>> +libetnaviv_la_SOURCES = $(C_SOURCES) $(CPP_SOURCES)
>> +
> CPP_SOURCES is empty. Please remove it (with later commit ?)
>

Fixed in separate patch which gets squashed before pushing.

>> +noinst_PROGRAMS = etnaviv_compiler
>> +
>> +etnaviv_compiler_SOURCES = \
>> +       etnaviv_compiler_cmdline.c
>> +
>> +etnaviv_compiler_LDADD = \
>> +       libetnaviv.la \
>> +       ../../auxiliary/libgallium.la \
> Don't use relative paths, use the below instead.
>
> $(top_builddir)/src/gallium/auxiliary/libgallium.la
>

Fixed in separate patch which gets squashed before pushing.

>
>> --- /dev/null
>> +++ b/src/gallium/drivers/etnaviv/Makefile.sources
>> @@ -0,0 +1,26 @@
>> +C_SOURCES :=  \
>> +       etnaviv_asm.c \
>> +       etnaviv_blend.c \
>> +       etnaviv_clear_blit.c \
>> +       etnaviv_compiler.c \
>> +       etnaviv_context.c \
>> +       etnaviv_disasm.c \
>> +       etnaviv_emit.c \
>> +       etnaviv_fence.c \
>> +       etnaviv_format.c \
>> +       etnaviv_query.c \
>> +       etnaviv_query_sw.c \
>> +       etnaviv_rasterizer.c \
>> +       etnaviv_resource.c \
>> +       etnaviv_rs.c \
>> +       etnaviv_screen.c \
>> +       etnaviv_shader.c \
>> +       etnaviv_state.c \
>> +       etnaviv_surface.c \
>> +       etnaviv_tiling.c \
>> +       etnaviv_texture.c \
>> +       etnaviv_transfer.c \
>> +       etnaviv_uniforms.c \
>> +       etnaviv_zsa.c
>> +
> Iirc earlier commits had all the files (headers including) listed here.
> Can we have those back please ?
>

Fixed in separate patch which gets squashed before pushing.

>
>> --- /dev/null
>> +++ b/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c
>
>> +static struct pipe_screen *
>> +screen_create(struct renderonly *ro)
>> +{
>> +   struct etna_device *dev;
>> +   struct etna_gpu *gpu;
>> +   uint64_t val;
>> +   int i;
>> +
>> +   dev = etna_device_new_dup(ro->gpu_fd);
>> +   if (!dev) {
>> +      fprintf(stderr, "Error creating device\n");
>> +      return NULL;
>> +   }
>> +
>> +   for (i = 0;; i++) {
>> +      gpu = etna_gpu_new(dev, i);
>> +      if (!gpu) {
>> +         fprintf(stderr, "Error creating gpu\n");
>> +         return NULL;
>> +      }
>> +
>> +      /* Look for a 3D capable GPU */
>> +      if (etna_gpu_get_param(gpu, ETNA_GPU_FEATURES_0, &val) == 0 &&
>> +            val & (1 << 2))
> We cannot used the VIV_FEATURE macro yet, but can we replace "1 << 2"
> with the corresponding define ?
>

Fixed in separate patch which gets squashed before pushing.

greets
--
Christian Gmeiner, MSc

https://www.youtube.com/user/AloryOFFICIAL
https://soundcloud.com/christian-gmeiner