Hi Jordan,
On 6 February 2017 at 17:39, Jordan Crouse <jcro...@codeaurora.org> wrote:
> Modify the 'pad' member of struct drm_msm_gem_info to 'hint'. If the
> user sets 'hint' to non-zero it means that they want a IOVA for the
> GEM object instead of a mmap() offset. Return the iova in the 'offset'
> member.
>
> Signed-off-by: Jordan Crouse <jcro...@codeaurora.org>
> ---
> drivers/gpu/drm/msm/msm_drv.c | 29 +++++++++++++++++++++++++----
> include/uapi/drm/msm_drm.h | 4 ++--
> 2 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index e29bb66..1e4e022 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -677,6 +677,17 @@ static int msm_ioctl_gem_cpu_fini(struct drm_device
> *dev, void *data,
> return ret;
> }
>
> +static int msm_ioctl_gem_info_iova(struct drm_device *dev,
> + struct drm_gem_object *obj, uint64_t *iova)
> +{
> + struct msm_drm_private *priv = dev->dev_private;
> +
> + if (!priv->gpu)
> + return -EINVAL;
> +
Not too familiar with msm so perhaps a silly question: how can we trigger this ?
> + return msm_gem_get_iova(obj, priv->gpu->aspace, iova);
> +}
> +
> static int msm_ioctl_gem_info(struct drm_device *dev, void *data,
> struct drm_file *file)
> {
> @@ -684,14 +695,24 @@ static int msm_ioctl_gem_info(struct drm_device *dev,
> void *data,
> struct drm_gem_object *obj;
> int ret = 0;
>
> - if (args->pad)
> - return -EINVAL;
> -
Please keep the input validation before doing any work (the lookup below).
> obj = drm_gem_object_lookup(file, args->handle);
> if (!obj)
> return -ENOENT;
>
> - args->offset = msm_gem_mmap_offset(obj);
> + /*
> + * If the hint variable is set, it means that the user wants a IOVA
> for
> + * this buffer. Return the address from the GPU because that is
> + * probably what it is looking for
> + */
> + if (args->hint) {
One could also rename hint to flags. Regardless of the name you can
use hint/flags as a bitmask.
> + uint64_t iova;
> +
> + ret = msm_ioctl_gem_info_iova(dev, obj, &iova);
> + if (!ret)
> + args->offset = iova;
> + } else {
> + args->offset = msm_gem_mmap_offset(obj);
> + }
>
> drm_gem_object_unreference_unlocked(obj);
>
> diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
> index 4d5d6a2..045ad20 100644
> --- a/include/uapi/drm/msm_drm.h
> +++ b/include/uapi/drm/msm_drm.h
> @@ -105,8 +105,8 @@ struct drm_msm_gem_new {
>
> struct drm_msm_gem_info {
> __u32 handle; /* in */
> - __u32 pad;
> - __u64 offset; /* out, offset to pass to mmap() */
> + __u32 hint; /* in */
Please add explicit #define for the currently valid hints/flags.
> + __u64 offset; /* out, mmap() offset if hint is 0, iova if 1 */
Other drivers have used anonymous unions to improve the naming, in
such situations.
struct drm_msm_gem_info {
__u32 handle; /* in */
__u32 hint; /* in */
union { /* out */
__u64 offset; /* offset if hint is FOO */
__u64 iova; /* iova if hint is BAR */
};
};
Thanks
Emil
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
