Looks good to me and tested on Baytrail. On Sun, Mar 16, 2014 at 11:09 PM, Zhao, Halley <halley.z...@intel.com> wrote: > Thanks Yakui。 > > > -----Original Message----- > From: Zhao, Yakui > Sent: Monday, March 17, 2014 10:25 AM > To: Zhao, Halley > Cc: libva@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.z...@intel.com> > > Thanks. > Yakui > >> >> Thanks. >> Yakui >> >> >> >> >> -----Original Message----- >> From: libva-boun...@lists.freedesktop.org >> [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of Zhao, Halley >> Sent: Friday, March 14, 2014 5:19 PM >> To: libva@lists.freedesktop.org >> Subject: [Libva] [PATCH 1/2] va: User specified tiling and stride support. >> >> From: "Zhao, Halley" <halley.z...@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@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/libva >> _______________________________________________ >> Libva mailing list >> Libva@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/libva > > > _______________________________________________ > Libva mailing list > Libva@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/libva
-- Sean V. Kelley <sean.v.kel...@intel.com> Open Source Technology Center / SSG Intel Corp. _______________________________________________ Libva mailing list Libva@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libva