| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87d1a452b5.fsf@eliezer.anholt.net>
Date: Thu, 15 Jun 2017 15:11:26 -0700
From: Eric Anholt <eric@...olt.net>
To: Boris Brezillon <boris.brezillon@...e-electrons.com>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] drm/vc4: Add get/set tiling ioctls.
Boris Brezillon <boris.brezillon@...e-electrons.com> writes:
> Hi Eric,
>
> On Wed, 7 Jun 2017 17:13:36 -0700
> Eric Anholt <eric@...olt.net> 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@...olt.net>
>> ---
>>
>> 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.
Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)
Powered by blists - more mailing lists