[Libva] [PATCH 1/2] va: User specified tiling and stride support.

Sean V Kelley sean.v.kelley at intel.com
Mon Mar 17 13:31:35 PDT 2014


Looks good to me and tested on Baytrail.

On Sun, Mar 16, 2014 at 11:09 PM, Zhao, Halley <halley.zhao at intel.com> wrote:
> Thanks Yakui。
>
>
> -----Original Message-----
> From: Zhao, Yakui
> Sent: Monday, March 17, 2014 10:25 AM
> To: Zhao, Halley
> Cc: libva at lists.freedesktop.org
> Subject: Re: [Libva] [PATCH 1/2] va: User specified tiling and stride support.
>
> On Sun, 2014-03-16 at 19:10 -0600, Zhao, Yakui wrote:
>> Hi, Halley
>>       Thanks for your patch.
>>       After adding the patch, it is possible that the upper-application can pass the specific parameters when creating the surface. I agree with your point.
>>       But it seems that one thing is missing:
>>        >For the non-tiling scenario: it is still possible that upper-application specifies the pitch/height requirement. But this is missed.
>
> After relooking at the code, it seems that the pitch/height is already handled before calling the function of i965_surface_native_memory.
> So please ignore my comment.
>
> Now this patch looks good to me.
>
> Add: Reviewed-by: Zhao Yakui <yakui.zhao at intel.com>
>
> Thanks.
>     Yakui
>
>>
>> Thanks.
>>       Yakui
>>
>>
>>
>>
>> -----Original Message-----
>> From: libva-bounces at lists.freedesktop.org
>> [mailto:libva-bounces at lists.freedesktop.org] On Behalf Of Zhao, Halley
>> Sent: Friday, March 14, 2014 5:19 PM
>> To: libva at lists.freedesktop.org
>> Subject: [Libva] [PATCH 1/2] va: User specified tiling and stride support.
>>
>> From: "Zhao, Halley" <halley.zhao at intel.com>
>>
>> It is done by two VASurfaceAttrib:
>>  * one is buffer attribute described by VASurfaceAttribExternalBufferDescriptor.
>>    it covers strides and tiling or not.
>>  * another is buffer type to indicate that the buffer is allocated by va driver.
>>    VASurfaceAttribMemoryType:VA_SURFACE_ATTRIB_MEM_TYPE_VA
>> ---
>>  src/i965_drv_video.c | 81
>> +++++++++++++++++++++++++++++++++++++++++++++++++---
>>  src/i965_drv_video.h |  4 +++
>>  src/intel_driver.h   |  1 +
>>  3 files changed, 82 insertions(+), 4 deletions(-)  mode change 100755
>> => 100644 src/i965_drv_video.c
>>
>> diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c old mode
>> 100755 new mode 100644 index 0453c04..1f0ba96
>> --- a/src/i965_drv_video.c
>> +++ b/src/i965_drv_video.c
>> @@ -1009,6 +1009,39 @@ i965_suface_external_memory(VADriverContextP ctx,
>>      return VA_STATUS_SUCCESS;
>>  }
>>
>> +/* byte-per-pixel of the first plane */ static int
>> +bpp_1stplane_by_fourcc(unsigned int fourcc) {
>> +    switch (fourcc) {
>> +        case VA_FOURCC_RGBA:
>> +        case VA_FOURCC_RGBX:
>> +        case VA_FOURCC_BGRA:
>> +        case VA_FOURCC_BGRX:
>> +        case VA_FOURCC_ARGB:
>> +        case VA_FOURCC_XRGB:
>> +        case VA_FOURCC_ABGR:
>> +        case VA_FOURCC_XBGR:
>> +        case VA_FOURCC_AYUV:
>> +            return 4;
>> +
>> +        case VA_FOURCC_UYVY:
>> +        case VA_FOURCC_YUY2:
>> +            return 2;
>> +
>> +        case VA_FOURCC_YV12:
>> +        case VA_FOURCC_IMC3:
>> +        case VA_FOURCC_IYUV:
>> +        case VA_FOURCC_NV12:
>> +        case VA_FOURCC_NV11:
>> +            return 1;
>> +
>> +        default:
>> +            assert(0);
>> +            return 0;
>> +    }
>> +}
>> +
>>  static VAStatus
>>  i965_CreateSurfaces2(
>>      VADriverContextP    ctx,
>> @@ -1044,6 +1077,8 @@ i965_CreateSurfaces2(
>>                  memory_type = I965_SURFACE_MEM_GEM_FLINK; /* flinked GEM handle */
>>              else if (attrib_list[i].value.value.i == VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME)
>>                  memory_type = I965_SURFACE_MEM_DRM_PRIME; /* drm
>> prime fd */
>> +            else if (attrib_list[i].value.value.i == VA_SURFACE_ATTRIB_MEM_TYPE_VA)
>> +                memory_type = I965_SURFACE_MEM_NATIVE; /* va native
>> + memory, to be allocated */
>>          }
>>
>>          if ((attrib_list[i].type == VASurfaceAttribExternalBufferDescriptor) && @@ -1077,6 +1112,9 @@ i965_CreateSurfaces2(
>>          obj_surface->status = VASurfaceReady;
>>          obj_surface->orig_width = width;
>>          obj_surface->orig_height = height;
>> +        obj_surface->user_disable_tiling = false;
>> +        obj_surface->user_h_stride_set = false;
>> +        obj_surface->user_v_stride_set = false;
>>
>>          obj_surface->subpic_render_idx = 0;
>>          for(j = 0; j < I965_MAX_SUBPIC_SUM; j++){ @@ -1096,6 +1134,34
>> @@ i965_CreateSurfaces2(
>>
>>          switch (memory_type) {
>>          case I965_SURFACE_MEM_NATIVE:
>> +            if (memory_attibute) {
>> +                if (!(memory_attibute->flags & VA_SURFACE_EXTBUF_DESC_ENABLE_TILING))
>> +                    obj_surface->user_disable_tiling = true;
>> +
>> +                if (memory_attibute->pixel_format) {
>> +                    if (expected_fourcc)
>> +                        assert(memory_attibute->pixel_format == expected_fourcc);
>> +                    else
>> +                        expected_fourcc = memory_attibute->pixel_format;
>> +                }
>> +                assert(expected_fourcc);
>> +                if (memory_attibute->pitches[0]) {
>> +                    int bpp_1stplane = bpp_1stplane_by_fourcc(expected_fourcc);
>> +                    assert(bpp_1stplane);
>> +                    obj_surface->width = memory_attibute->pitches[0]/bpp_1stplane;
>> +                    obj_surface->user_h_stride_set = true;
>> +                    assert(IS_ALIGNED(obj_surface->width, 16));
>> +                    assert(obj_surface->width >= width);
>> +
>> +                    if (memory_attibute->offsets[1]) {
>> +                        assert(!memory_attibute->offsets[0]);
>> +                        obj_surface->height = memory_attibute->offsets[1]/memory_attibute->pitches[0];
>> +                        obj_surface->user_v_stride_set = true;
>> +                        assert(IS_ALIGNED(obj_surface->height, 16));
>> +                        assert(obj_surface->height >= height);
>> +                    }
>> +                }
>> +            }
>>              i965_surface_native_memory(ctx,
>>                                         obj_surface,
>>                                         format, @@ -2892,13 +2958,20 @@ i965_check_alloc_surface_bo(VADriverContextP ctx,
>>      obj_surface->x_cb_offset = 0; /* X offset is always 0 */
>>      obj_surface->x_cr_offset = 0;
>>
>> -    if (tiled) {
>> +    if ((tiled && !obj_surface->user_disable_tiling)) {
>>          assert(fourcc != VA_FOURCC('I', '4', '2', '0') &&
>>                 fourcc != VA_FOURCC('I', 'Y', 'U', 'V') &&
>>                 fourcc != VA_FOURCC('Y', 'V', '1', '2'));
>> +        if (obj_surface->user_h_stride_set) {
>> +            assert(IS_ALIGNED(obj_surface->width, 128));
>> +        } else
>> +            obj_surface->width = ALIGN(obj_surface->orig_width, 128);
>> +
>> +        if (obj_surface->user_v_stride_set) {
>> +            assert(IS_ALIGNED(obj_surface->height, 32));
>> +        } else
>> +            obj_surface->height = ALIGN(obj_surface->orig_height,
>> + 32);
>>
>> -        obj_surface->width = ALIGN(obj_surface->orig_width, 128);
>> -        obj_surface->height = ALIGN(obj_surface->orig_height, 32);
>>          region_height = obj_surface->height;
>>
>>          switch (fourcc) {
>> @@ -3092,7 +3165,7 @@ i965_check_alloc_surface_bo(VADriverContextP
>> ctx,
>>
>>      obj_surface->size = ALIGN(region_width * region_height, 0x1000);
>>
>> -    if (tiled) {
>> +    if ((tiled && !obj_surface->user_disable_tiling)) {
>>          uint32_t tiling_mode = I915_TILING_Y; /* always uses Y-tiled format */
>>          unsigned long pitch;
>>
>> diff --git a/src/i965_drv_video.h b/src/i965_drv_video.h index
>> f66970a..e1f2bda 100644
>> --- a/src/i965_drv_video.h
>> +++ b/src/i965_drv_video.h
>> @@ -228,6 +228,10 @@ struct object_surface
>>      int cb_cr_width;
>>      int cb_cr_height;
>>      int cb_cr_pitch;
>> +    /* user specified attributes see: VASurfaceAttribExternalBuffers/VA_SURFACE_ATTRIB_MEM_TYPE_VA */
>> +    uint32_t user_disable_tiling : 1;
>> +    uint32_t user_h_stride_set   : 1;
>> +    uint32_t user_v_stride_set   : 1;
>>  };
>>
>>  struct object_buffer
>> diff --git a/src/intel_driver.h b/src/intel_driver.h index
>> 851bef4..5d4c335 100644
>> --- a/src/intel_driver.h
>> +++ b/src/intel_driver.h
>> @@ -67,6 +67,7 @@
>>  struct intel_batchbuffer;
>>
>>  #define ALIGN(i, n)    (((i) + (n) - 1) & ~((n) - 1))
>> +#define IS_ALIGNED(i, n) (((i) & ((n)-1)) == 0)
>>  #define MIN(a, b) ((a) < (b) ? (a) : (b))  #define MAX(a, b) ((a) >
>> (b) ? (a) : (b))  #define ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0]))
>> --
>> 1.8.3.2
>>
>> _______________________________________________
>> Libva mailing list
>> Libva at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/libva
>> _______________________________________________
>> Libva mailing list
>> Libva at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/libva
>
>
> _______________________________________________
> Libva mailing list
> Libva at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libva



-- 
Sean V. Kelley <sean.v.kelley at intel.com>
Open Source Technology Center / SSG
Intel Corp.


More information about the Libva mailing list