>-----Original Message-----
>From: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>Sent: Monday, October 12, 2020 10:14 AM
>To: Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>; intel-
>gfx@xxxxxxxxxxxxxxxxxxxxx
>Subject: Re: [PATCH 10/61] drm/i915: Disable userptr pread/pwrite
>support.
>
>Op 02-10-2020 om 22:14 schreef Ruhl, Michael J:
>>> -----Original Message-----
>>> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
>>> Maarten Lankhorst
>>> Sent: Friday, October 2, 2020 8:59 AM
>>> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>>> Subject: [PATCH 10/61] drm/i915: Disable userptr pread/pwrite
>>> support.
>>>
>>> Userptr should not need the kernel for a userspace memcpy, userspace
>>> needs to call memcpy directly.
>>>
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>>> ---
>>> .../gpu/drm/i915/gem/i915_gem_object_types.h | 2 ++
>>> drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 20
>>> +++++++++++++++++++
>>> drivers/gpu/drm/i915/i915_gem.c | 5 +++++
>>> 3 files changed, 27 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>>> b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>>> index 62dde3585b51..dbb6f6171165 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>>> @@ -57,6 +57,8 @@ struct drm_i915_gem_object_ops {
>>>
>>> int (*pwrite)(struct drm_i915_gem_object *obj,
>>> const struct drm_i915_gem_pwrite *arg);
>>> + int (*pread)(struct drm_i915_gem_object *obj,
>>> + const struct drm_i915_gem_pread *arg);
>>>
>>> int (*dmabuf_export)(struct drm_i915_gem_object *obj);
>>> void (*release)(struct drm_i915_gem_object *obj);
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>>> b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>>> index 22008948be58..136a589e5d94 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>>> @@ -700,6 +700,24 @@ i915_gem_userptr_dmabuf_export(struct
>>> drm_i915_gem_object *obj)
>>> return i915_gem_userptr_init__mmu_notifier(obj, 0);
>>> }
>>>
>>> +static int
>>> +i915_gem_userptr_pwrite(struct drm_i915_gem_object *obj,
>>> + const struct drm_i915_gem_pwrite *args)
>>> +{
>>> + drm_dbg(obj->base.dev, "pwrite to userptr no longer allowed\n");
>>> +
>>> + return -EINVAL;
>> I have seen ENOSYS used for unsupported pread/pwrite (see
>radeon_gem.c).
>>
>> I have also seen ENOTSUPP for similar return values.
>>
>> Is EINVAL the correct response?
>
>It seems for some other things we use -ENXIO, I don't think it matters in the
>end.
>
>As long as we fail in some recognisable way, I'm fine with it. :)
>
>I chose -EINVAL as we return the same error with r/o objects.
Hi Maarten,
Sorry this got lost in the noise last week. 😐
If my comments are still relevant... 😊
Ok, I understand your reasoning here, but I do see differences.
EINVAL for r/o means I made bad request. ENOTSUPP means that the feature
I want to use is not available. Not terribly different, but slightly more "correct".
I don't see how ENXIO fits other than as a "more unique" error.
I do think that EINVAL is over used, making it difficult to find out why the error
was returned, but I can see its use in this case, so I am not opposed.
Sorry for the lateness of my reply,
Mike
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
