On Thu, Jul 18, 2019 at 9:03 AM Steven Price <steven.price@xxxxxxx> wrote:
>
> On 17/07/2019 19:33, Rob Herring wrote:
> > Setting the GPU VA when creating the GEM object doesn't allow for any
> > conditional adjustments. In preparation to support adjusting the
> > mapping, restructure the GEM object creation to map the GEM object after
> > we've created the base shmem object.
> >
> > Cc: Tomeu Vizoso <tomeu.vizoso@xxxxxxxxxxxxx>
> > Cc: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
> > Cc: Robin Murphy <robin.murphy@xxxxxxx>
> > Cc: Steven Price <steven.price@xxxxxxx>
> > Cc: Alyssa Rosenzweig <alyssa@xxxxxxxxxxxxx>
> > Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
>
> Hi Rob,
>
> I couldn't work out what base this series should be applied to, but I've
> tried manually applying it against Linus' tree and run a few tests.
It's drm-misc-next plus some fixes in drm-misc-fixes that haven't been
merged into drm-misc-next yet. I'll post a git branch on the next
version.
> PANFROST_BO_NOEXEC works as expected, but PANFROST_BO_HEAP seems to
> cause a memory leak.
>
> > ---
> > drivers/gpu/drm/panfrost/panfrost_drv.c | 21 +++------
> > drivers/gpu/drm/panfrost/panfrost_gem.c | 58 ++++++++++++++++++++-----
> > drivers/gpu/drm/panfrost/panfrost_gem.h | 5 +++
> > 3 files changed, 59 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > index cb43ff4ebf4a..d354b92964d5 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > @@ -46,29 +46,20 @@ static int panfrost_ioctl_get_param(struct drm_device *ddev, void *data, struct
> > static int panfrost_ioctl_create_bo(struct drm_device *dev, void *data,
> > struct drm_file *file)
> > {
> > - int ret;
> > - struct drm_gem_shmem_object *shmem;
> > + struct panfrost_gem_object *bo;
> > struct drm_panfrost_create_bo *args = data;
> >
> > if (!args->size || args->flags || args->pad)
> > return -EINVAL;
> >
> > - shmem = drm_gem_shmem_create_with_handle(file, dev, args->size,
> > - &args->handle);
> > - if (IS_ERR(shmem))
> > - return PTR_ERR(shmem);
> > -
> > - ret = panfrost_mmu_map(to_panfrost_bo(&shmem->base));
> > - if (ret)
> > - goto err_free;
> > + bo = panfrost_gem_create_with_handle(file, dev, args->size, args->flags,
> > + &args->handle);
> > + if (IS_ERR(bo))
> > + return PTR_ERR(bo);
> >
> > - args->offset = to_panfrost_bo(&shmem->base)->node.start << PAGE_SHIFT;
> > + args->offset = bo->node.start << PAGE_SHIFT;
> >
> > return 0;
> > -
> > -err_free:
> > - drm_gem_handle_delete(file, args->handle);
> > - return ret;
> > }
> >
> > /**
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
> > index 543ab1b81bd5..df70dcf3cb2f 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
> > @@ -23,7 +23,8 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj)
> > panfrost_mmu_unmap(bo);
> >
> > spin_lock(&pfdev->mm_lock);
> > - drm_mm_remove_node(&bo->node);
> > + if (drm_mm_node_allocated(&bo->node))
> > + drm_mm_remove_node(&bo->node);
>
> I'm not sure this change should be here - it looks more like it was
> meant as part of patch 4.
It's needed here because because we do the mapping later instead of
the .gem_create_object() hook. So when we free the object in case of
errors, the mapping may or may not have been created.
Rob
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
