[virglrenderer-devel] [PATCH] vrend: Initial support for multi planar.
Lepton Wu
lepton at chromium.org
Wed Sep 5 17:35:11 UTC 2018
On Wed, Aug 29, 2018 at 4:38 PM Gurchetan Singh <gurchetansingh at chromium.org>
wrote:
> Good idea, we do need a way to handle YUV buffers. IIRC zachr at + also
> ran into the limitations of DRM_IOCTL_VIRTGPU_RESOURCE_CREATE. Maybe
> a DRM_IOCTL_VIRTGPU_RESOURCE_CREATE2 that handles:
>
> - YUV formats
> - format modifiers
> - stride
>
> is warranted.
>
I am not sure about "format modifiers" since actually I don't know what's
that. But I think it's possible to
handle multi planar format and stride in current
DRM_IOCTL_VIRTGPU_RESOURCE_CREATE ioctl.
> On Wed, Aug 29, 2018 at 1:40 PM Lepton Wu <lepton at chromium.org> wrote:
> >
> > On Wed, Aug 29, 2018 at 1:23 PM Lepton Wu <lepton at chromium.org> wrote:
> > >
> > > On Wed, Aug 29, 2018 at 1:15 PM Lepton Wu <lepton at chromium.org> wrote:
> > > >
> > > > With this and other necessary patches to kernel, mesa etc, video
> player
> > > > under android can play videos with YV12 format under virgl + qemu.
> > > Patches for kerne/mesa/minigbm can be found here for your reference.
> > >
> > > kernel:
> https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1194613
> > > mesa:
> https://chromium-review.googlesource.com/c/chromiumos/third_party/mesa/+/1195011
> > > minigbm:
> https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/1195587
> > BTW, currently drm_virtgpu_resource_create doesn't really use stride
> > info even it has a stride field.
> > This information is needed since android has some alignment
> > requirement for yv12 format.
> > I think this could be addressed by a separate CL to kernel.
> > >
> > > My plan is to get virglrenderer change done first since I could get
> > > comments/suggestion to change interface or
> > > even implementation detail. After virglrender side change done here ,
> > > then I will send patch to upstream kernel,
> > > then mesa, minigbm. Also perhaps libdrm since I could get suggestion
> > > for adding new ioctl. Now I am trying to
> > > reuse some old ioctl.
> > >
> > > > The basic idea is:
> > > > DRM_IOCTL_VIRTGPU_RESOURCE_CREATE can be used to create additional
> planes
> > > > at some offset of an existed bo. Every plane is backed by a host
> side texture.
> > > > When guest try to get res_handle for an existed bo, it will get the
> actual
> > > > res_handle for that plane by checking offset.
> > > > ---
> > > > src/virgl_hw.h | 1 +
> > > > src/virglrenderer.h | 12 +++++++--
> > > > src/vrend_renderer.c | 64
> ++++++++++++++++++++++++++++++++++++++++++++
> > > > src/vrend_renderer.h | 26 ++++++++++++++++--
> > > > 4 files changed, 99 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/src/virgl_hw.h b/src/virgl_hw.h
> > > > index 4add191..e8df498 100644
> > > > --- a/src/virgl_hw.h
> > > > +++ b/src/virgl_hw.h
> > > > @@ -342,4 +342,5 @@ enum virgl_ctx_errors {
> > > >
> > > >
> > > > #define VIRGL_RESOURCE_Y_0_TOP (1 << 0)
> > > > +#define VIRGL_RESOURCE_PLANE (1 << 1)
> > > > #endif
> > > > diff --git a/src/virglrenderer.h b/src/virglrenderer.h
> > > > index 5baecdd..c41523d 100644
> > > > --- a/src/virglrenderer.h
> > > > +++ b/src/virglrenderer.h
> > > > @@ -93,8 +93,16 @@ VIRGL_EXPORT int
> virgl_renderer_get_fd_for_texture2(uint32_t tex_id, int *fd, in
> > > >
> > > > struct virgl_renderer_resource_create_args {
> > > > uint32_t handle;
> > > > - uint32_t target;
> > > > - uint32_t format;
> > > > + union {
> > > > + struct {
> > > > + uint32_t target;
> > > > + uint32_t format;
> > > > + };
> > > > + struct {
> > > > + uint32_t main_res;
> > > > + uint32_t offset;
> > > > + } plane;
> > > > + };
> > > > uint32_t bind;
> > > > uint32_t width;
> > > > uint32_t height;
> > > > diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> > > > index 4086abd..b38b37e 100644
> > > > --- a/src/vrend_renderer.c
> > > > +++ b/src/vrend_renderer.c
> > > > @@ -5144,10 +5144,50 @@ static int
> vrend_renderer_resource_allocate_texture(struct vrend_resource *gr,
> > > > return 0;
> > > > }
> > > >
> > > > +static int check_plane_valid(struct
> vrend_renderer_resource_create_args *args,
> > > > + struct
> vrend_renderer_resource_create_args
> > > > + *plane_args, struct
> vrend_plane_resource** pr) {
> > > > + struct vrend_resource *res;
> > > > + res = vrend_resource_lookup(args->plane.main_res, 0);
> > > > + if (!res)
> > > > + return -1;
> > > > + *plane_args = *args;
> > > > + /* Assume same target/format for sub planes */
> > > > + plane_args->target = res->base.target;
> > > > + plane_args->format = res->base.format;
> > > > + plane_args->flags &= ~VIRGL_RESOURCE_PLANE;
> > > > + for (int i = 0; i < VR_MAX_SUB_PLANES; ++i) {
> > > > + // Only support to add planes in order to make life simple.
> > > > + if (args->plane.offset <= res->planes[i].offset) {
> > > > + fprintf(stderr, "Add planes in wrong order\n");
> > > > + return -1;
> > > > + }
> > > > + if (res->planes[i].offset == 0) {
> > > > + *pr = &res->planes[i];
> > > > + return 0;
> > > > + }
> > > > + }
> > > > + return -1;
> > > > +}
> > > > +
> > > > int vrend_renderer_resource_create(struct
> vrend_renderer_resource_create_args *args, struct iovec *iov, uint32_t
> num_iovs, void *image_oes)
> > > > {
> > > > struct vrend_resource *gr;
> > > > int ret;
> > > > + struct vrend_renderer_resource_create_args plane_args;
> > > > + struct vrend_plane_resource *pr;
> > > > +
> > > > + if (args->flags & VIRGL_RESOURCE_PLANE) {
> > > > + ret = check_plane_valid(args, &plane_args, &pr);
> > > > + if (ret)
> > > > + return EINVAL;
> > > > + ret = vrend_renderer_resource_create(&plane_args, iov,
> num_iovs, image_oes);
> > > > + if (ret == 0) {
> > > > + pr->handle = args->handle;
> > > > + pr->offset = args->plane.offset;
> > > > + }
> > > > + return ret;
> > > > + }
> > > >
> > > > ret = check_resource_valid(args);
> > > > if (ret)
> > > > @@ -6067,6 +6107,22 @@ int vrend_renderer_transfer_iov(const struct
> vrend_transfer_info *info,
> > > > return EINVAL;
> > > > }
> > > >
> > > > + /* If this is a request for sub planes, read/write sub planes.
> > > > + */
> > > > + struct vrend_plane_resource* plane = NULL;
> > > > + for (int i = 0; i < VR_MAX_SUB_PLANES && res->planes[i].offset;
> i++) {
> > > > + if (info->offset >= res->planes[i].offset)
> > > > + plane = &res->planes[i];
> > > > + }
> > > > + if (plane) {
> > > > + struct vrend_transfer_info tmp;
> > > > + tmp = *info;
> > > > + tmp.handle = plane->handle;
> > > > + tmp.iovec = iov;
> > > > + tmp.iovec_cnt = num_iovs;
> > > > + return vrend_renderer_transfer_iov(&tmp, transfer_mode);
> > > > + }
> > > > +
> > > > if (!check_transfer_bounds(res, info))
> > > > return EINVAL;
> > > >
> > > > @@ -7886,6 +7942,10 @@ void vrend_renderer_attach_res_ctx(int
> ctx_id, int resource_id)
> > > > return;
> > > >
> > > > vrend_object_insert_nofree(ctx->res_hash, res, sizeof(*res),
> resource_id, 1, false);
> > > > +
> > > > + for (int i = 0; i < VR_MAX_SUB_PLANES && res->planes[i].offset;
> ++i) {
> > > > + vrend_renderer_attach_res_ctx(ctx_id, res->planes[i].handle);
> > > > + }
> > > > }
> > > >
> > > > static void vrend_renderer_detach_res_ctx_p(struct vrend_context
> *ctx, int res_handle)
> > > > @@ -7895,6 +7955,10 @@ static void
> vrend_renderer_detach_res_ctx_p(struct vrend_context *ctx, int res_h
> > > > if (!res)
> > > > return;
> > > >
> > > > + for (int i = 0; i < VR_MAX_SUB_PLANES && res->planes[i].offset;
> ++i) {
> > > > + vrend_renderer_detach_res_ctx_p(ctx, res->planes[i].handle);
> > > > + }
> > > > +
> > > > vrend_object_remove(ctx->res_hash, res_handle, 1);
> > > > }
> > > >
> > > > diff --git a/src/vrend_renderer.h b/src/vrend_renderer.h
> > > > index bb76b44..21dceb6 100644
> > > > --- a/src/vrend_renderer.h
> > > > +++ b/src/vrend_renderer.h
> > > > @@ -49,6 +49,19 @@ struct vrend_context;
> > > > */
> > > > #define VR_MAX_TEXTURE_2D_LEVELS 15
> > > >
> > > > +/* For multi-planar formats like YV12, we just use vrend_resource
> for the first
> > > > + * plane as "main" vrend_resource and embed information for
> additional planes
> > > > + * as "sub" planes in "main" vrend_resource. So for YV12, we only
> need 2 "sub"
> > > > + * planes.
> > > > + */
> > > > +#define VR_MAX_SUB_PLANES 2
> > > > +
> > > > +struct vrend_plane_resource {
> > > > + GLuint handle;
> > > > + uint32_t offset;
> > > > + uint32_t size;
> > > > +};
> > > > +
> > > > struct vrend_resource {
> > > > struct pipe_resource base;
> > > > GLuint id;
> > > > @@ -68,6 +81,7 @@ struct vrend_resource {
> > > > struct iovec *iov;
> > > > uint32_t num_iovs;
> > > > uint64_t mipmap_offsets[VR_MAX_TEXTURE_2D_LEVELS];
> > > > + struct vrend_plane_resource planes[VR_MAX_SUB_PLANES];
> > > > };
> > > >
> > > > /* assume every format is sampler friendly */
> > > > @@ -160,8 +174,16 @@ void vrend_renderer_context_destroy(uint32_t
> handle);
> > > >
> > > > struct vrend_renderer_resource_create_args {
> > > > uint32_t handle;
> > > > - enum pipe_texture_target target;
> > > > - uint32_t format;
> > > > + union {
> > > > + struct {
> > > > + enum pipe_texture_target target;
> > > > + uint32_t format;
> > > > + };
> > > > + struct {
> > > > + uint32_t main_res;
> > > > + uint32_t offset;
> > > > + } plane;
> > > > + };
> > > > uint32_t bind;
> > > > uint32_t width;
> > > > uint32_t height;
> > > > --
> > > > 2.19.0.rc0.228.g281dcd1b4d0-goog
> > > >
> > _______________________________________________
> > virglrenderer-devel mailing list
> > virglrenderer-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/virglrenderer-devel/attachments/20180905/96c99dcd/attachment.html>
More information about the virglrenderer-devel
mailing list