[PATCH 07/10] drm/gma500: Inline psb_gtt_{alloc,free}_range() into rsp callers
Thomas Zimmermann
tzimmermann at suse.de
Mon Oct 4 06:23:09 UTC 2021
Hi
Am 03.10.21 um 00:14 schrieb Patrik Jakobsson:
> On Tue, Sep 28, 2021 at 10:44 AM Thomas Zimmermann <tzimmermann at suse.de> wrote:
>>
>> psb_gtt_alloc_range() allocates struct gtt_range, create the GTT resource
>> and performs some half-baked initialization. Inline the function into its
>> only caller psb_gem_create(). For creating the GTT resource, introduce a
>> new helper, psb_gtt_alloc_resource() that hides the details of the GTT.
>>
>> For psb_gtt_free_range(), inline the function into its only caller
>> psb_gem_free_object(). While at it, remove the explicit invocation of
>> drm_gem_free_mmap_offset(). The mmap offset is already released by
>> drm_gem_object_release().
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>> drivers/gpu/drm/gma500/gem.c | 94 ++++++++++++------------------------
>> drivers/gpu/drm/gma500/gtt.c | 27 +++++++++++
>> drivers/gpu/drm/gma500/gtt.h | 6 +++
>> 3 files changed, 65 insertions(+), 62 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
>> index a48d7d5ed026..46209e10dcc2 100644
>> --- a/drivers/gpu/drm/gma500/gem.c
>> +++ b/drivers/gpu/drm/gma500/gem.c
>> @@ -87,30 +87,22 @@ void psb_gem_unpin(struct gtt_range *gt)
>> mutex_unlock(&dev_priv->gtt_mutex);
>> }
>>
>> -static void psb_gtt_free_range(struct drm_device *dev, struct gtt_range *gt)
>> -{
>> - /* Undo the mmap pin if we are destroying the object */
>> - if (gt->mmapping) {
>> - psb_gem_unpin(gt);
>> - gt->mmapping = 0;
>> - }
>> - WARN_ON(gt->in_gart && !gt->stolen);
>> - release_resource(>->resource);
>> - kfree(gt);
>> -}
>> -
>> static vm_fault_t psb_gem_fault(struct vm_fault *vmf);
>>
>> static void psb_gem_free_object(struct drm_gem_object *obj)
>> {
>> - struct gtt_range *gtt = to_gtt_range(obj);
>> + struct gtt_range *gt = to_gtt_range(obj);
>>
>> - /* Remove the list map if one is present */
>> - drm_gem_free_mmap_offset(obj);
>> drm_gem_object_release(obj);
>>
>> - /* This must occur last as it frees up the memory of the GEM object */
>> - psb_gtt_free_range(obj->dev, gtt);
>> + /* Undo the mmap pin if we are destroying the object */
>> + if (gt->mmapping)
>> + psb_gem_unpin(gt);
>> +
>> + WARN_ON(gt->in_gart && !gt->stolen);
>> +
>> + release_resource(>->resource);
>> + kfree(gt);
>> }
>>
>> static const struct vm_operations_struct psb_gem_vm_ops = {
>> @@ -124,59 +116,35 @@ static const struct drm_gem_object_funcs psb_gem_object_funcs = {
>> .vm_ops = &psb_gem_vm_ops,
>> };
>>
>> -static struct gtt_range *psb_gtt_alloc_range(struct drm_device *dev, int len,
>> - const char *name, int backed, u32 align)
>> -{
>> - struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>> - struct gtt_range *gt;
>> - struct resource *r = dev_priv->gtt_mem;
>> - int ret;
>> - unsigned long start, end;
>> -
>> - if (backed) {
>> - /* The start of the GTT is the stolen pages */
>> - start = r->start;
>> - end = r->start + dev_priv->gtt.stolen_size - 1;
>> - } else {
>> - /* The rest we will use for GEM backed objects */
>> - start = r->start + dev_priv->gtt.stolen_size;
>> - end = r->end;
>> - }
>> -
>> - gt = kzalloc(sizeof(struct gtt_range), GFP_KERNEL);
>> - if (gt == NULL)
>> - return NULL;
>> - gt->resource.name = name;
>> - gt->stolen = backed;
>> - gt->in_gart = backed;
>> - /* Ensure this is set for non GEM objects */
>> - gt->gem.dev = dev;
>> - ret = allocate_resource(dev_priv->gtt_mem, >->resource,
>> - len, start, end, align, NULL, NULL);
>> - if (ret == 0) {
>> - gt->offset = gt->resource.start - r->start;
>> - return gt;
>> - }
>> - kfree(gt);
>> - return NULL;
>> -}
>> -
>> struct gtt_range *
>> psb_gem_create(struct drm_device *dev, u64 size, const char *name, bool stolen, u32 align)
>> {
>> + struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>> struct gtt_range *gt;
>> struct drm_gem_object *obj;
>> int ret;
>>
>> size = roundup(size, PAGE_SIZE);
>>
>> - gt = psb_gtt_alloc_range(dev, size, name, stolen, align);
>> - if (!gt) {
>> - dev_err(dev->dev, "no memory for %lld byte GEM object\n", size);
>> - return ERR_PTR(-ENOSPC);
>> - }
>> + gt = kzalloc(sizeof(*gt), GFP_KERNEL);
>> + if (!gt)
>> + return ERR_PTR(-ENOMEM);
>> obj = >->gem;
>>
>> + /* GTT resource */
>> +
>> + ret = psb_gtt_allocate_resource(dev_priv, >->resource, name, size, align, stolen,
>> + >->offset);
>> + if (ret)
>> + goto err_kfree;
>> +
>> + if (stolen) {
>> + gt->stolen = true;
>> + gt->in_gart = 1;
>> + }
>> +
>> + /* GEM object */
>> +
>> obj->funcs = &psb_gem_object_funcs;
>>
>> if (stolen) {
>> @@ -184,7 +152,7 @@ psb_gem_create(struct drm_device *dev, u64 size, const char *name, bool stolen,
>> } else {
>> ret = drm_gem_object_init(dev, obj, size);
>> if (ret)
>> - goto err_psb_gtt_free_range;
>> + goto err_release_resource;
>>
>> /* Limit the object to 32-bit mappings */
>> mapping_set_gfp_mask(obj->filp->f_mapping, GFP_KERNEL | __GFP_DMA32);
>> @@ -192,8 +160,10 @@ psb_gem_create(struct drm_device *dev, u64 size, const char *name, bool stolen,
>>
>> return gt;
>>
>> -err_psb_gtt_free_range:
>> - psb_gtt_free_range(dev, gt);
>> +err_release_resource:
>> + release_resource(>->resource);
>> +err_kfree:
>> + kfree(gt);
>> return ERR_PTR(ret);
>> }
>>
>> diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
>> index 0aacf7122e32..5d940fdbe6b8 100644
>> --- a/drivers/gpu/drm/gma500/gtt.c
>> +++ b/drivers/gpu/drm/gma500/gtt.c
>> @@ -18,6 +18,33 @@
>> * GTT resource allocator - manage page mappings in GTT space
>> */
>>
>> +int psb_gtt_allocate_resource(struct drm_psb_private *pdev, struct resource *res,
>> + const char *name, resource_size_t size, resource_size_t align,
>> + bool stolen, u32 offset[static 1])
>
> Why [static 1]?
[static N] a notation that tells the C compiler that there are at least
N elements in the array. Passing an array with less or NULL results in
an error (with clang at least). IMHO we shoudld use it everywhere, but
then we'd get complains on coding style, I guess.
Best regards
Thomas
>
>
>> +{
>> + struct resource *root = pdev->gtt_mem;
>> + resource_size_t start, end;
>> + int ret;
>> +
>> + if (stolen) {
>> + /* The start of the GTT is backed by stolen pages. */
>> + start = root->start;
>> + end = root->start + pdev->gtt.stolen_size - 1;
>> + } else {
>> + /* The rest is backed by system pages. */
>> + start = root->start + pdev->gtt.stolen_size;
>> + end = root->end;
>> + }
>> +
>> + res->name = name;
>> + ret = allocate_resource(root, res, size, start, end, align, NULL, NULL);
>> + if (ret)
>> + return ret;
>> + *offset = res->start - root->start;
>> +
>> + return 0;
>> +}
>> +
>> /**
>> * psb_gtt_mask_pte - generate GTT pte entry
>> * @pfn: page number to encode
>> diff --git a/drivers/gpu/drm/gma500/gtt.h b/drivers/gpu/drm/gma500/gtt.h
>> index 36162b545570..459a03141e8b 100644
>> --- a/drivers/gpu/drm/gma500/gtt.h
>> +++ b/drivers/gpu/drm/gma500/gtt.h
>> @@ -10,6 +10,8 @@
>>
>> #include <drm/drm_gem.h>
>>
>> +struct drm_psb_private;
>> +
>> /* This wants cleaning up with respect to the psb_dev and un-needed stuff */
>> struct psb_gtt {
>> uint32_t gatt_start;
>> @@ -43,6 +45,10 @@ struct gtt_range {
>>
>> extern int psb_gtt_restore(struct drm_device *dev);
>>
>> +int psb_gtt_allocate_resource(struct drm_psb_private *pdev, struct resource *res,
>> + const char *name, resource_size_t size, resource_size_t align,
>> + bool stolen, u32 offset[static 1]);
>> +
>> int psb_gtt_insert(struct drm_device *dev, struct gtt_range *r, int resume);
>> void psb_gtt_remove(struct drm_device *dev, struct gtt_range *r);
>>
>> --
>> 2.33.0
>>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20211004/1ab6e77f/attachment-0001.sig>
More information about the dri-devel
mailing list