[Mesa-dev] [PATCH 05/10] gbm: Add a native intel backend

[Ander Conselvan de Oliveira]

On 04/04/2014 02:06 PM, Emil Velikov wrote:
> On 04/04/14 09:36, Ander Conselvan de Oliveira wrote:
>> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
>>
> A few minor nits inline.
>
>> ---
>>   src/gbm/Makefile.am                |  14 +-
>>   src/gbm/backends/intel/gbm_intel.c | 258 +++++++++++++++++++++++++++++++++++++
>>   src/gbm/backends/intel/gbm_intel.h |  74 +++++++++++
>>   src/gbm/main/backend.c             |   2 +
>>   src/gbm/main/common_drm.h          |   1 +
>>   5 files changed, 347 insertions(+), 2 deletions(-)
>>   create mode 100644 src/gbm/backends/intel/gbm_intel.c
>>   create mode 100644 src/gbm/backends/intel/gbm_intel.h
>>
>> diff --git a/src/gbm/Makefile.am b/src/gbm/Makefile.am
>> index ea06ce1..b53a68d 100644
>> --- a/src/gbm/Makefile.am
>> +++ b/src/gbm/Makefile.am
>> @@ -49,6 +49,16 @@ libgbm_la_LIBADD += \
>>   	libgbm_dri.la $(top_builddir)/src/mapi/shared-glapi/libglapi.la $(LIBDRM_LIBS)
>>   endif
>>
>> -TESTS = gbm-symbols-check
>> +if HAVE_I965_DRI
>> +noinst_LTLIBRARIES = libgbm_intel.la
>> +libgbm_intel_la_SOURCES = \
>> +	backends/intel/gbm_intel.c
>> +
>> +libgbm_intel_la_CFLAGS = \
>> +	$(AM_CFLAGS) $(INTEL_CFLAGS)
>> +
>> +libgbm_la_LIBADD += \
>> +	libgbm_intel.la $(INTEL_LIBS)
>> +endif
>>
>> -include $(top_srcdir)/install-lib-links.mk
> You might want to keep this line.
>> +TESTS = gbm-symbols-check
>> diff --git a/src/gbm/backends/intel/gbm_intel.c b/src/gbm/backends/intel/gbm_intel.c
>> new file mode 100644
>> index 0000000..2e7ced3
>> --- /dev/null
>> +++ b/src/gbm/backends/intel/gbm_intel.c
>> @@ -0,0 +1,258 @@
>> +/*
>> + * Copyright © 2011-2013 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + *
>> + * Authors:
>> + *    Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
>> + */
>> +
>> +#include <errno.h>
>> +#include <stdlib.h>
>> +#include <stdint.h>
>> +#include <string.h>
>> +#include <sys/types.h>
>> +#include <unistd.h>
>> +
>> +#include "gbm_intel.h"
>> +
>> +#include "gbmint.h"
>> +
>> +static int
>> +gbm_intel_is_format_supported(struct gbm_device *gbm,
>> +                            uint32_t format,
>> +                            uint32_t usage)
>> +{
> Why do people avoid std_bool ?  The function name already indicates that the
> return value is true/false yet you use int.

This matches GBM's public interface for gbm_is_format_supported(). I 
don't know why stdbool wasn't used in the first place.

I'll fix the other issues and resend.

Thanks for reviewing.

Cheers,
Ander

>> +   switch (format) {
>> +   case GBM_BO_FORMAT_XRGB8888:
>> +   case GBM_FORMAT_XRGB8888:
>> +      break;
>> +   case GBM_BO_FORMAT_ARGB8888:
>> +   case GBM_FORMAT_ARGB8888:
>> +      if (usage & GBM_BO_USE_SCANOUT)
>> +         return 0;
>> +      break;
>> +   default:
>> +      return 0;
>> +   }
>> +
>> +   if (usage & GBM_BO_USE_CURSOR_64X64 &&
>> +       usage & GBM_BO_USE_RENDERING)
>> +      return 0;
>> +
>> +   return 1;
>> +}
>> +
> [snip]
>
>> +
>> +static inline int
>> +align(int value, int size)
>> +{
> Please use ALIGN from mesa/main/macros.h
>
>> +   return (value + size - 1) & ~(size - 1);
>> +}
>> +
> [snip]
>
>> +
>> +static struct gbm_device *
>> +gbm_intel_device_create(int fd)
>> +{
>> +   struct gbm_intel_device *igbm;
>> +
>> +   igbm = calloc(1, sizeof *igbm);
> +   if (igbm == NULL)
> +      return NULL;
>
> -Emil
>