[PATCH 03/11] drm/msm: Add hint to DRM_IOCTL_MSM_GEM_INFO to return an object IOVA

[Emil Velikov]

On 6 February 2017 at 19:57, Rob Clark <robdclark at gmail.com> wrote:
> On Mon, Feb 6, 2017 at 2:20 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> Hi Jordan,
>>
>> On 6 February 2017 at 17:39, Jordan Crouse <jcrouse at 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 <jcrouse at 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 ?
>
> if gpu has not loaded (for example, missing firmware, or kernel does
> not have iommu, etc)
>
Thanks Rob. I was under the impression that in such cases the driver
will/should fail to load.

>>> +       __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 */
>>         };
>> };
>
> is anon union legit for uabi?  I was under the impression that for
> some reason it was not.  But I could be wrong.
>
Haven't seen any wording against it and we do have a few instances in
DRM UABI land.
Either way it was just an idea.

Regards,
Emil