Re: [PATCH v2 2/5] drm: etnaviv: Switch to use drm_gem_object reservation_object
Am Sa., 2. Feb. 2019 um 16:42 Uhr schrieb Rob Herring :
>
> Now that the base struct drm_gem_object has a reservation_object, use it
> and remove the private BO one.
>
> Cc: Lucas Stach
> Cc: Russell King
> Cc: Christian Gmeiner
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: etna...@lists.freedesktop.org
> Signed-off-by: Rob Herring
Reviewed-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_drv.c| 1 -
> drivers/gpu/drm/etnaviv/etnaviv_drv.h| 1 -
> drivers/gpu/drm/etnaviv/etnaviv_gem.c| 16 +-
> drivers/gpu/drm/etnaviv/etnaviv_gem.h| 4
> drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 7 ---
> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 22 ++--
> 6 files changed, 16 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 18c27f795cf6..9f42f7538236 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -473,7 +473,6 @@ static struct drm_driver etnaviv_drm_driver = {
> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> .gem_prime_export = drm_gem_prime_export,
> .gem_prime_import = drm_gem_prime_import,
> - .gem_prime_res_obj = etnaviv_gem_prime_res_obj,
> .gem_prime_pin = etnaviv_gem_prime_pin,
> .gem_prime_unpin= etnaviv_gem_prime_unpin,
> .gem_prime_get_sg_table = etnaviv_gem_prime_get_sg_table,
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index 4bf698de5996..351db0c5822d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -61,7 +61,6 @@ void *etnaviv_gem_prime_vmap(struct drm_gem_object *obj);
> void etnaviv_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
> int etnaviv_gem_prime_mmap(struct drm_gem_object *obj,
>struct vm_area_struct *vma);
> -struct reservation_object *etnaviv_gem_prime_res_obj(struct drm_gem_object
> *obj);
> struct drm_gem_object *etnaviv_gem_prime_import_sg_table(struct drm_device
> *dev,
> struct dma_buf_attachment *attach, struct sg_table *sg);
> int etnaviv_gem_prime_pin(struct drm_gem_object *obj);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 1fa74226db91..3e954adcb2cc 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -397,13 +397,13 @@ int etnaviv_gem_cpu_prep(struct drm_gem_object *obj,
> u32 op,
> }
>
> if (op & ETNA_PREP_NOSYNC) {
> - if (!reservation_object_test_signaled_rcu(etnaviv_obj->resv,
> + if (!reservation_object_test_signaled_rcu(obj->resv,
> write))
> return -EBUSY;
> } else {
> unsigned long remain = etnaviv_timeout_to_jiffies(timeout);
>
> - ret = reservation_object_wait_timeout_rcu(etnaviv_obj->resv,
> + ret = reservation_object_wait_timeout_rcu(obj->resv,
> write, true,
> remain);
> if (ret <= 0)
> return ret == 0 ? -ETIMEDOUT : ret;
> @@ -459,7 +459,7 @@ static void etnaviv_gem_describe_fence(struct dma_fence
> *fence,
> static void etnaviv_gem_describe(struct drm_gem_object *obj, struct seq_file
> *m)
> {
> struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj);
> - struct reservation_object *robj = etnaviv_obj->resv;
> + struct reservation_object *robj = obj->resv;
> struct reservation_object_list *fobj;
> struct dma_fence *fence;
> unsigned long off = drm_vma_node_start(&obj->vma_node);
> @@ -549,8 +549,6 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
>
> drm_gem_free_mmap_offset(obj);
> etnaviv_obj->ops->release(etnaviv_obj);
> - if (etnaviv_obj->resv == &etnaviv_obj->_resv)
> - reservation_object_fini(&etnaviv_obj->_resv);
> drm_gem_object_release(obj);
>
> kfree(etnaviv_obj);
> @@ -596,12 +594,8 @@ static int etnaviv_gem_new_impl(struct drm_device *dev,
> u32 size, u32 flags,
>
> etnaviv_obj->flags = flags;
> etnaviv_obj->ops = ops;
> - if (robj) {
> - etnaviv_obj->resv = robj;
> - } else {
> - etnaviv_obj->resv = &etnaviv_obj->_resv;
> - reservation_object_init(&etnaviv_obj->_resv);
> - }
> + if (robj)
> + etnaviv_obj->base.resv = robj;
>
> mutex_init(&etnaviv_obj->lock);
> INIT_LIST_HEAD(&etnaviv_obj->vram_list);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index 76079c2291f8..7015837ccc1c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
>
Re: [PATCH v2 1/5] drm: Add reservation_object to drm_gem_object
Am Di., 12. Feb. 2019 um 15:16 Uhr schrieb Daniel Vetter :
>
> On Fri, Feb 08, 2019 at 08:23:44AM +0100, Christian Gmeiner wrote:
> > Am Sa., 2. Feb. 2019 um 16:42 Uhr schrieb Rob Herring :
> > >
> > > Many users of drm_gem_object embed a struct reservation_object into
> > > their subclassed struct, so let's add one to struct drm_gem_object.
> > > This will allow removing the reservation object from the subclasses
> > > and removing the ->gem_prime_res_obj callback.
> > >
> > > With the addition, add a drm_gem_reservation_object_wait() helper
> > > function for drivers to use in wait ioctls.
> > >
> > > Cc: Maarten Lankhorst
> > > Cc: Maxime Ripard
> > > Cc: Sean Paul
> > > Cc: David Airlie
> > > Cc: Daniel Vetter
> > > Signed-off-by: Rob Herring
> >
> > Reviewed-by: Christian Gmeiner
>
> Does this extend to 2/5 to update etnaviv?
oopps... yes it does - see my reply to 2/5.
> -Daniel
>
> >
> > > ---
> > > v2:
> > > - Fix missing kerneldoc
> > > - Reword todo with what is left todo.
> > > - Fix timeout error handling (added to drm_gem_reservation_object_wait)
> > >
> > > Documentation/gpu/todo.rst | 8 +++
> > > drivers/gpu/drm/drm_gem.c | 43 +
> > > drivers/gpu/drm/drm_prime.c | 1 +
> > > include/drm/drm_gem.h | 21 ++
> > > 4 files changed, 69 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> > > index 14191b64446d..c9ed55605641 100644
> > > --- a/Documentation/gpu/todo.rst
> > > +++ b/Documentation/gpu/todo.rst
> > > @@ -209,12 +209,12 @@ Would be great to refactor this all into a set of
> > > small common helpers.
> > >
> > > Contact: Daniel Vetter
> > >
> > > -Put a reservation_object into drm_gem_object
> > > +Remove the ->gem_prime_res_obj callback
> > >
> > >
> > > -This would remove the need for the ->gem_prime_res_obj callback. It
> > > would also
> > > -allow us to implement generic helpers for waiting for a bo, allowing for
> > > quite a
> > > -bit of refactoring in the various wait ioctl implementations.
> > > +The ->gem_prime_res_obj callback can be removed from drivers by using the
> > > +reservation_object in the drm_gem_object. It may also be possible to use
> > > the
> > > +generic drm_gem_reservation_object_wait helper for waiting for a bo.
> > >
> > > Contact: Daniel Vetter
> > >
> > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > > index 8b55ece97967..b09acbc5a655 100644
> > > --- a/drivers/gpu/drm/drm_gem.c
> > > +++ b/drivers/gpu/drm/drm_gem.c
> > > @@ -170,6 +170,10 @@ void drm_gem_private_object_init(struct drm_device
> > > *dev,
> > > kref_init(&obj->refcount);
> > > obj->handle_count = 0;
> > > obj->size = size;
> > > + reservation_object_init(&obj->_resv);
> > > + if (!obj->resv)
> > > + obj->resv = &obj->_resv;
> > > +
> > > drm_vma_node_reset(&obj->vma_node);
> > > }
> > > EXPORT_SYMBOL(drm_gem_private_object_init);
> > > @@ -657,6 +661,44 @@ drm_gem_object_lookup(struct drm_file *filp, u32
> > > handle)
> > > }
> > > EXPORT_SYMBOL(drm_gem_object_lookup);
> > >
> > > +/**
> > > + * drm_gem_reservation_object_wait - Wait on GEM object's reservation's
> > > objects
> > > + * shared and/or exclusive fences.
> > > + * @filep: DRM file private date
> > > + * @handle: userspace handle
> > > + * @wait_all: if true, wait on all fences, else wait on just exclusive
> > > fence
> > > + * @timeout: timeout value in jiffies or zero to return immediately
> > > + *
> > > + * Returns:
> > > + *
> > > + * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or
> > > + * greater than 0 on success.
> > > + */
> > > +long drm_gem_reservation_object_wait(struct drm_file *filep, u32 handle,
> > > + bool wait_all, unsigned long timeout)
> > > +{
> > > + long ret;
> > > + struct drm_gem_object *obj;
> > > +
> > > + obj = drm_gem_object_lookup(filep, handle);
> > > + if (!obj) {
> > > + DRM_DEBUG("Failed to look up GEM BO %d\n", handle);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = reservation_object_wait_timeout_rcu(obj->resv, wait_all,
> > > + true, timeout);
> > > + if (ret == 0)
> > > + ret = -ETIME;
> > > + else if (ret > 0)
> > > + ret = 0;
> > > +
> > > + drm_gem_object_put_unlocked(obj);
> > > +
> > > + return ret;
> > > +}
> > > +EXPORT_SYMBOL(drm_gem_reservation_object_wait);
> > > +
> > > /**
> > > * drm_gem_close_ioctl - implementation of the GEM_CLOSE ioctl
> > > * @dev: drm_device
> > > @@ -821,6 +863,7 @@ drm_gem_object_release(struct drm_gem_object *obj)
> > > if (obj->filp)
> > > fput(obj->filp);
> > >
> > > + reservation_object_fini(&obj->_resv);
> > >
