Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> writes: > Hi Eric, > > On Wed, 7 Jun 2017 17:13:36 -0700 > Eric Anholt <eric@xxxxxxxxxx> wrote: > >> This allows mesa to set the tiling format for a BO and have that >> tiling format be respected by mesa on the other side of an >> import/export (and by vc4 scanout in the kernel), without defining a >> protocol to pass the tiling through userspace. >> >> Signed-off-by: Eric Anholt <eric@xxxxxxxxxx> >> --- >> >> igt tests (which caught two edge cases) submitted to intel-gfx. >> >> Mesa code: https://github.com/anholt/mesa/commits/drm-vc4-tiling >> >> drivers/gpu/drm/vc4/vc4_bo.c | 83 +++++++++++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/vc4/vc4_drv.c | 2 ++ >> drivers/gpu/drm/vc4/vc4_drv.h | 6 ++++ >> drivers/gpu/drm/vc4/vc4_kms.c | 41 ++++++++++++++++++++- >> include/uapi/drm/vc4_drm.h | 16 +++++++++ >> 5 files changed, 147 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c >> index 80b2f9e55c5c..21649109fd4f 100644 >> --- a/drivers/gpu/drm/vc4/vc4_bo.c >> +++ b/drivers/gpu/drm/vc4/vc4_bo.c >> @@ -347,6 +347,7 @@ void vc4_free_object(struct drm_gem_object *gem_bo) >> bo->validated_shader = NULL; >> } >> >> + bo->t_format = false; >> bo->free_time = jiffies; >> list_add(&bo->size_head, cache_list); >> list_add(&bo->unref_head, &vc4->bo_cache.time_list); >> @@ -572,6 +573,88 @@ vc4_create_shader_bo_ioctl(struct drm_device *dev, void *data, >> return ret; >> } >> >> +/** >> + * vc4_set_tiling_ioctl() - Sets the tiling modifier for a BO. >> + * @dev: DRM device >> + * @data: ioctl argument >> + * @file_priv: DRM file for this fd >> + * >> + * The tiling state of the BO decides the default modifier of an fb if >> + * no specific modifier was set by userspace, and the return value of >> + * vc4_get_tiling_ioctl() (so that userspace can treat a BO it >> + * received from dmabuf as the same tiling format as the producer >> + * used). >> + */ >> +int vc4_set_tiling_ioctl(struct drm_device *dev, void *data, >> + struct drm_file *file_priv) >> +{ >> + struct drm_vc4_set_tiling *args = data; >> + struct drm_gem_object *gem_obj; >> + struct vc4_bo *bo; >> + bool t_format; >> + >> + if (args->flags != 0) >> + return -EINVAL; >> + >> + switch (args->modifier) { >> + case DRM_FORMAT_MOD_NONE: >> + t_format = false; >> + break; >> + case DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED: >> + t_format = true; >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + gem_obj = drm_gem_object_lookup(file_priv, args->handle); >> + if (!gem_obj) { >> + DRM_ERROR("Failed to look up GEM BO %d\n", args->handle); >> + return -ENOENT; >> + } >> + bo = to_vc4_bo(gem_obj); >> + bo->t_format = t_format; >> + >> + drm_gem_object_unreference_unlocked(gem_obj); >> + >> + return 0; >> +} >> + >> +/** >> + * vc4_get_tiling_ioctl() - Gets the tiling modifier for a BO. >> + * @dev: DRM device >> + * @data: ioctl argument >> + * @file_priv: DRM file for this fd >> + * >> + * Returns the tiling modifier for a BO as set by vc4_set_tiling_ioctl(). >> + */ >> +int vc4_get_tiling_ioctl(struct drm_device *dev, void *data, >> + struct drm_file *file_priv) >> +{ >> + struct drm_vc4_get_tiling *args = data; >> + struct drm_gem_object *gem_obj; >> + struct vc4_bo *bo; >> + >> + if (args->flags != 0 || args->modifier != 0) >> + return -EINVAL; >> + >> + gem_obj = drm_gem_object_lookup(file_priv, args->handle); >> + if (!gem_obj) { >> + DRM_ERROR("Failed to look up GEM BO %d\n", args->handle); >> + return -ENOENT; >> + } >> + bo = to_vc4_bo(gem_obj); >> + >> + if (bo->t_format) >> + args->modifier = DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED; >> + else >> + args->modifier = DRM_FORMAT_MOD_NONE; >> + >> + drm_gem_object_unreference_unlocked(gem_obj); >> + >> + return 0; >> +} >> + >> void vc4_bo_cache_init(struct drm_device *dev) >> { >> struct vc4_dev *vc4 = to_vc4_dev(dev); >> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c >> index 136bb4213dc0..c6b487c3d2b7 100644 >> --- a/drivers/gpu/drm/vc4/vc4_drv.c >> +++ b/drivers/gpu/drm/vc4/vc4_drv.c >> @@ -138,6 +138,8 @@ static const struct drm_ioctl_desc vc4_drm_ioctls[] = { >> DRM_IOCTL_DEF_DRV(VC4_GET_HANG_STATE, vc4_get_hang_state_ioctl, >> DRM_ROOT_ONLY), >> DRM_IOCTL_DEF_DRV(VC4_GET_PARAM, vc4_get_param_ioctl, DRM_RENDER_ALLOW), >> + DRM_IOCTL_DEF_DRV(VC4_SET_TILING, vc4_set_tiling_ioctl, DRM_RENDER_ALLOW), >> + DRM_IOCTL_DEF_DRV(VC4_GET_TILING, vc4_get_tiling_ioctl, DRM_RENDER_ALLOW), >> }; >> >> static struct drm_driver vc4_drm_driver = { >> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h >> index a5bf2e5e0b57..df22698d62ee 100644 >> --- a/drivers/gpu/drm/vc4/vc4_drv.h >> +++ b/drivers/gpu/drm/vc4/vc4_drv.h >> @@ -148,6 +148,8 @@ struct vc4_bo { >> */ >> uint64_t write_seqno; >> >> + bool t_format; >> + > > Will we need the DRM_VC4_SET/GET_TILING ioctls when importing a BO > that is using H264 tile mode? If this is the case, we should probably > store the modifier directly. I'm not sure. Whoever is getting buffers from the ISP is going to be doing the prime import to vc4 for displaying it on a plane, so it seems about equal complexity ot me to do it either way. If we're using some existing dma-buf based media stack, it might support plane modifiers already, though.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
