[Mesa-dev] [PATCH v2 3/3] imx: gallium driver for imx-drm scanout driver

[Christian Gmeiner]

Hi Emil,

thanks for your review!

2017-01-10 14:22 GMT+01:00 Emil Velikov <emil.l.velikov at gmail.com>:
> Hi Christian,
>
> Similar to 2/3 there's a few trivial nitpicks which can be addressed
> at a later stage.
>

I fixed them and will squash them.

> On 23 December 2016 at 22:04, Christian Gmeiner
> <christian.gmeiner at gmail.com> wrote:
>> Changes from V1 -> V2:
>>  - updated Copyright
>>  - added $(top_srcdir)/src/gallium/winsys to include path (suggested by Emil)
>>  - adapted driver to new renderonly API
>>
>> Signed-off-by: Christian Gmeiner <christian.gmeiner at gmail.com>
>> ---
>>  configure.ac                                       | 12 ++++++
>>  src/gallium/Makefile.am                            |  4 ++
>>  .../auxiliary/pipe-loader/pipe_loader_drm.c        |  5 +++
>>  src/gallium/auxiliary/target-helpers/drm_helper.h  | 24 +++++++++++
>>  .../auxiliary/target-helpers/drm_helper_public.h   |  3 ++
>>  src/gallium/drivers/imx/Automake.inc               |  9 ++++
>>  src/gallium/drivers/imx/Makefile.am                |  9 ++++
>>  src/gallium/targets/dri/Makefile.am                |  1 +
>>  src/gallium/targets/dri/target.c                   |  8 ++++
>>  src/gallium/winsys/imx/drm/Makefile.am             | 34 +++++++++++++++
>>  src/gallium/winsys/imx/drm/Makefile.sources        |  3 ++
>>  src/gallium/winsys/imx/drm/imx_drm_public.h        | 34 +++++++++++++++
>>  src/gallium/winsys/imx/drm/imx_drm_winsys.c        | 50 ++++++++++++++++++++++
>>  13 files changed, 196 insertions(+)
>>  create mode 100644 src/gallium/drivers/imx/Automake.inc
>>  create mode 100644 src/gallium/drivers/imx/Makefile.am
>>  create mode 100644 src/gallium/winsys/imx/drm/Makefile.am
>>  create mode 100644 src/gallium/winsys/imx/drm/Makefile.sources
>>  create mode 100644 src/gallium/winsys/imx/drm/imx_drm_public.h
>>  create mode 100644 src/gallium/winsys/imx/drm/imx_drm_winsys.c
>>
>> diff --git a/configure.ac b/configure.ac
>> index 0b98ce8..59c4064 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -2512,6 +2512,9 @@ if test -n "$with_gallium_drivers"; then
>>              PKG_CHECK_MODULES([ETNAVIV], [libdrm_etnaviv >= $LIBDRM_ETNAVIV_REQUIRED])
>>              require_libdrm "etnaviv"
>>              ;;
>> +       ximx)
>> +            HAVE_GALLIUM_IMX=yes
>> +            ;;
> Update help string for --with-gallium-drivers.
>

Fixed in separate patch which gets squashed before pushing.

>
>> --- /dev/null
>> +++ b/src/gallium/drivers/imx/Makefile.am
>> @@ -0,0 +1,9 @@
>> +include $(top_srcdir)/src/gallium/Automake.inc
>> +
>> +AM_CPPFLAGS = \
>> +       $(GALLIUM_CFLAGS) \
>> +       $(IMX_CFLAGS)
> IMX_CFLAGS is empty, please remove.
>

Fixed in separate patch which gets squashed before pushing.

>
>> new file mode 100644
>> index 0000000..d155b2e
>> --- /dev/null
>> +++ b/src/gallium/winsys/imx/drm/Makefile.am
>
>
>> +include Makefile.sources
>> +include $(top_srcdir)/src/gallium/Automake.inc
>> +
>> +AM_CFLAGS = \
>> +       -I$(top_srcdir)/src/gallium/drivers \
>> +       -I$(top_srcdir)/src/gallium/winsys \
>> +       $(GALLIUM_WINSYS_CFLAGS) \
>> +       $(IMX_CFLAGS)
> Ditto.
>

Fixed in separate patch which gets squashed before pushing.

>> +
>> +noinst_LTLIBRARIES = libimxdrm.la
>> +
>> +libimxdrm_la_SOURCES = $(C_SOURCES)
>> \ No newline at end of file
> Add newline ?
>

Fixed in separate patch which gets squashed before pushing.

>> diff --git a/src/gallium/winsys/imx/drm/Makefile.sources b/src/gallium/winsys/imx/drm/Makefile.sources
>> new file mode 100644
>> index 0000000..3c0d6fb
>> --- /dev/null
>> +++ b/src/gallium/winsys/imx/drm/Makefile.sources
>> @@ -0,0 +1,3 @@
>> +C_SOURCES := \
>> +   imx_drm_public.h \
>> +   imx_drm_winsys.c
>> \ No newline at end of file
> Ditto.
>

Fixed in separate patch which gets squashed before pushing.

>> --- /dev/null
>> +++ b/src/gallium/winsys/imx/drm/imx_drm_winsys.c
>
>> +struct pipe_screen *imx_drm_screen_create(int fd)
>> +{
>> +   struct renderonly ro = {
>> +      .create_for_resource = renderonly_create_kms_dumb_buffer_for_resource,
>> +      .kms_fd = fd,
>> +      .gpu_fd = open("/dev/dri/renderD128", O_RDWR | O_CLOEXEC)
> As we get Thierry's libdrm work we can polish the heuristics. But this
> will be fine for now.
>

Great!

greets
--
Christian Gmeiner, MSc

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