[Mesa-dev] [PATCH 02/10] dri_interface: add __DRI_IMAGE_TRANSFER_USER_STRIDE

[Gurchetan Singh]

On Wed, Apr 25, 2018 at 2:16 PM, Marek Olšák <maraeo at gmail.com> wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> Allow the caller to specify the row stride (in bytes) with which an image
> should be mapped. Note that completely ignoring USER_STRIDE is a valid
> implementation of mapImage.
>
> This is horrible API design. Unfortunately, cros_gralloc does indeed have
> a horrible API design -- in that arbitrary images should be allowed to be
> mapped with the stride that a linear image of the same width would have.

Yes, unfortunately the gralloc API doesn't return the stride when
(*lock) is called.  However, the stride is returned during (*alloc).
Currently, for the dri backend, minigbm uses
__DRI_IMAGE_ATTRIB_STRIDE to compute the pixel stride given to
Android.

Is AMD seeing problems with the current approach (I haven't seen any
bugs filed for this issue)?

Another possible solution is to call mapImage()/unmapImage right after
allocating the image, and use the stride returned by mapImage() to
compute the pixel stride.  That could also fix whatever bugs AMD is
seeing.

>
> There is no separate capability bit because it's unclear how stricter
> requirements should be defined.
> ---
>  include/GL/internal/dri_interface.h | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
> index 07dfd74f9d8..4247e61415f 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -1213,21 +1213,21 @@ struct __DRIdri2ExtensionRec {
>      */
>     __DRIcreateNewScreen2Func            createNewScreen2;
>  };
>
>
>  /**
>   * This extension provides functionality to enable various EGLImage
>   * extensions.
>   */
>  #define __DRI_IMAGE "DRI_IMAGE"
> -#define __DRI_IMAGE_VERSION 17
> +#define __DRI_IMAGE_VERSION 18
>
>  /**
>   * These formats correspond to the similarly named MESA_FORMAT_*
>   * tokens, except in the native endian of the CPU.  For example, on
>   * little endian __DRI_IMAGE_FORMAT_XRGB8888 corresponds to
>   * MESA_FORMAT_XRGB8888, but MESA_FORMAT_XRGB8888_REV on big endian.
>   *
>   * __DRI_IMAGE_FORMAT_NONE is for images that aren't directly usable
>   * by the driver (YUV planar formats) but serve as a base image for
>   * creating sub-images for the different planes within the image.
> @@ -1263,20 +1263,21 @@ struct __DRIdri2ExtensionRec {
>   * in contrary to gbm buffers, front buffers and fake front buffers, which
>   * could be read after a flush."
>   */
>  #define __DRI_IMAGE_USE_BACKBUFFER      0x0010
>
>
>  #define __DRI_IMAGE_TRANSFER_READ            0x1
>  #define __DRI_IMAGE_TRANSFER_WRITE           0x2
>  #define __DRI_IMAGE_TRANSFER_READ_WRITE      \
>          (__DRI_IMAGE_TRANSFER_READ | __DRI_IMAGE_TRANSFER_WRITE)
> +#define __DRI_IMAGE_TRANSFER_USER_STRIDE     0x4 /* since version 18 */
>
>  /**
>   * Four CC formats that matches with WL_DRM_FORMAT_* from wayland_drm.h,
>   * GBM_FORMAT_* from gbm.h, and DRM_FORMAT_* from drm_fourcc.h. Used with
>   * createImageFromNames.
>   *
>   * \since 5
>   */
>
>  #define __DRI_IMAGE_FOURCC_R8          0x20203852
> @@ -1554,22 +1555,31 @@ struct __DRIimageExtensionRec {
>     /**
>      * Returns a map of the specified region of a __DRIimage for the specified usage.
>      *
>      * flags may include __DRI_IMAGE_TRANSFER_READ, which will populate the
>      * mapping with the current buffer content. If __DRI_IMAGE_TRANSFER_READ
>      * is not included in the flags, the buffer content at map time is
>      * undefined. Users wanting to modify the mapping must include
>      * __DRI_IMAGE_TRANSFER_WRITE; if __DRI_IMAGE_TRANSFER_WRITE is not
>      * included, behaviour when writing the mapping is undefined.
>      *
> -    * Returns the byte stride in *stride, and an opaque pointer to data
> -    * tracking the mapping in **data, which must be passed to unmapImage().
> +    * When __DRI_IMAGE_TRANSFER_USER_STRIDE is set in \p flags (since version 18),
> +    * the driver should attempt to map the image with the byte stride given in
> +    * *stride. The caller must ensure that *stride is large enough to hold a
> +    * row of the mapping. If the requested stride is not supported, the mapping
> +    * may fail, or a mapping with a different stride may be created (in which
> +    * case the actual stride is returned in *stride).
> +    *
> +    * Returns an opaque pointer to data tracking the mapping in **data, which
> +    * must be passed to unmapImage().
> +    *
> +    * Returns the byte stride in *stride.
>      *
>      * Returns NULL on error.
>      *
>      * \since 12
>      */
>     void *(*mapImage)(__DRIcontext *context, __DRIimage *image,
>                       int x0, int y0, int width, int height,
>                       unsigned int flags, int *stride, void **data);
>
>     /**
> --
> 2.17.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev