All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
@ 2021-11-30  9:52 ` Thomas Zimmermann
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-11-30  9:52 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, airlied, daniel, yuq825, robh,
	tomeu.vizoso, steven.price, alyssa.rosenzweig, emma, kraxel,
	gurchetansingh, olvaffe, dan.carpenter
  Cc: Thomas Zimmermann, lima, dri-devel, virtualization

GEM helper libraries use struct drm_driver.gem_create_object to let
drivers override GEM object allocation. On failure, the call returns
NULL.

Change the semantics to make the calls return a pointer-encoded error.
This aligns the callback with its callers. Fixes the ingenic driver,
which already returns an error pointer.

Also update the callers to handle the involved types more strictly.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
There is an alternative patch at [1] that updates the value returned
by ingenics' gem_create_object to NULL. Fixing the interface to return
an errno code is more consistent with the rest of the GEM functions.

[1] https://lore.kernel.org/dri-devel/20211118111522.GD1147@kili/
---
 drivers/gpu/drm/drm_gem_cma_helper.c    | 17 ++++++++++-------
 drivers/gpu/drm/drm_gem_shmem_helper.c  | 17 ++++++++++-------
 drivers/gpu/drm/drm_gem_vram_helper.c   |  4 ++--
 drivers/gpu/drm/lima/lima_gem.c         |  2 +-
 drivers/gpu/drm/panfrost/panfrost_gem.c |  2 +-
 drivers/gpu/drm/v3d/v3d_bo.c            |  4 ++--
 drivers/gpu/drm/vc4/vc4_bo.c            |  2 +-
 drivers/gpu/drm/vgem/vgem_drv.c         |  2 +-
 drivers/gpu/drm/virtio/virtgpu_object.c |  2 +-
 include/drm/drm_drv.h                   |  5 +++--
 10 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 7d4895de9e0d..cefd0cbf9deb 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -67,18 +67,21 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size, bool private)
 	struct drm_gem_object *gem_obj;
 	int ret = 0;

-	if (drm->driver->gem_create_object)
+	if (drm->driver->gem_create_object) {
 		gem_obj = drm->driver->gem_create_object(drm, size);
-	else
-		gem_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
-	if (!gem_obj)
-		return ERR_PTR(-ENOMEM);
+		if (IS_ERR(gem_obj))
+			return ERR_CAST(gem_obj);
+		cma_obj = to_drm_gem_cma_obj(gem_obj);
+	} else {
+		cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
+		if (!cma_obj)
+			return ERR_PTR(-ENOMEM);
+		gem_obj = &cma_obj->base;
+	}

 	if (!gem_obj->funcs)
 		gem_obj->funcs = &drm_gem_cma_default_funcs;

-	cma_obj = container_of(gem_obj, struct drm_gem_cma_object, base);
-
 	if (private) {
 		drm_gem_private_object_init(drm, gem_obj, size);

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 0eeda1012364..7915047cb041 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -56,14 +56,17 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private)

 	size = PAGE_ALIGN(size);

-	if (dev->driver->gem_create_object)
+	if (dev->driver->gem_create_object) {
 		obj = dev->driver->gem_create_object(dev, size);
-	else
-		obj = kzalloc(sizeof(*shmem), GFP_KERNEL);
-	if (!obj)
-		return ERR_PTR(-ENOMEM);
-
-	shmem = to_drm_gem_shmem_obj(obj);
+		if (IS_ERR(obj))
+			return ERR_CAST(obj);
+		shmem = to_drm_gem_shmem_obj(obj);
+	} else {
+		shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
+		if (!shmem)
+			return ERR_PTR(-ENOMEM);
+		obj = &shmem->base;
+	}

 	if (!obj->funcs)
 		obj->funcs = &drm_gem_shmem_funcs;
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index bfa386b98134..3f00192215d1 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -197,8 +197,8 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,

 	if (dev->driver->gem_create_object) {
 		gem = dev->driver->gem_create_object(dev, size);
-		if (!gem)
-			return ERR_PTR(-ENOMEM);
+		if (IS_ERR(gem))
+			return ERR_CAST(gem);
 		gbo = drm_gem_vram_of_gem(gem);
 	} else {
 		gbo = kzalloc(sizeof(*gbo), GFP_KERNEL);
diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
index 2723d333c608..f9a9198ef198 100644
--- a/drivers/gpu/drm/lima/lima_gem.c
+++ b/drivers/gpu/drm/lima/lima_gem.c
@@ -221,7 +221,7 @@ struct drm_gem_object *lima_gem_create_object(struct drm_device *dev, size_t siz

 	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
 	if (!bo)
-		return NULL;
+		return ERR_PTR(-ENOMEM);

 	mutex_init(&bo->lock);
 	INIT_LIST_HEAD(&bo->va);
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 6d9bdb9180cb..ead65f5fa2bc 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -223,7 +223,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t

 	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
 	if (!obj)
-		return NULL;
+		return ERR_PTR(-ENOMEM);

 	INIT_LIST_HEAD(&obj->mappings.list);
 	mutex_init(&obj->mappings.lock);
diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index 0d9af62f69ad..6e3113f419f4 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -70,11 +70,11 @@ struct drm_gem_object *v3d_create_object(struct drm_device *dev, size_t size)
 	struct drm_gem_object *obj;

 	if (size == 0)
-		return NULL;
+		return ERR_PTR(-EINVAL);

 	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
 	if (!bo)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 	obj = &bo->base.base;

 	obj->funcs = &v3d_gem_funcs;
diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 8520e440dbd1..6d1281a343e9 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -391,7 +391,7 @@ struct drm_gem_object *vc4_create_object(struct drm_device *dev, size_t size)

 	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
 	if (!bo)
-		return NULL;
+		return ERR_PTR(-ENOMEM);

 	bo->madv = VC4_MADV_WILLNEED;
 	refcount_set(&bo->usecnt, 0);
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index a87eafa89e9f..c5e3e5457737 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -97,7 +97,7 @@ static struct drm_gem_object *vgem_gem_create_object(struct drm_device *dev, siz

 	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
 	if (!obj)
-		return NULL;
+		return ERR_PTR(-ENOMEM);

 	/*
 	 * vgem doesn't have any begin/end cpu access ioctls, therefore must use
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index 187e10da2f17..baef2c5f2aaf 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -139,7 +139,7 @@ struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev,

 	shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
 	if (!shmem)
-		return NULL;
+		return ERR_PTR(-ENOMEM);

 	dshmem = &shmem->base.base;
 	dshmem->base.funcs = &virtio_gpu_shmem_funcs;
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index da0c836fe8e1..f6159acb8856 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -291,8 +291,9 @@ struct drm_driver {
 	/**
 	 * @gem_create_object: constructor for gem objects
 	 *
-	 * Hook for allocating the GEM object struct, for use by the CMA and
-	 * SHMEM GEM helpers.
+	 * Hook for allocating the GEM object struct, for use by the CMA
+	 * and SHMEM GEM helpers. Returns a GEM object on success, or an
+	 * ERR_PTR()-encoded error code otherwise.
 	 */
 	struct drm_gem_object *(*gem_create_object)(struct drm_device *dev,
 						    size_t size);
--
2.34.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
@ 2021-11-30  9:52 ` Thomas Zimmermann
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-11-30  9:52 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, airlied, daniel, yuq825, robh,
	tomeu.vizoso, steven.price, alyssa.rosenzweig, emma, kraxel,
	gurchetansingh, olvaffe, dan.carpenter
  Cc: Thomas Zimmermann, lima, dri-devel, virtualization

GEM helper libraries use struct drm_driver.gem_create_object to let
drivers override GEM object allocation. On failure, the call returns
NULL.

Change the semantics to make the calls return a pointer-encoded error.
This aligns the callback with its callers. Fixes the ingenic driver,
which already returns an error pointer.

Also update the callers to handle the involved types more strictly.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
There is an alternative patch at [1] that updates the value returned
by ingenics' gem_create_object to NULL. Fixing the interface to return
an errno code is more consistent with the rest of the GEM functions.

[1] https://lore.kernel.org/dri-devel/20211118111522.GD1147@kili/
---
 drivers/gpu/drm/drm_gem_cma_helper.c    | 17 ++++++++++-------
 drivers/gpu/drm/drm_gem_shmem_helper.c  | 17 ++++++++++-------
 drivers/gpu/drm/drm_gem_vram_helper.c   |  4 ++--
 drivers/gpu/drm/lima/lima_gem.c         |  2 +-
 drivers/gpu/drm/panfrost/panfrost_gem.c |  2 +-
 drivers/gpu/drm/v3d/v3d_bo.c            |  4 ++--
 drivers/gpu/drm/vc4/vc4_bo.c            |  2 +-
 drivers/gpu/drm/vgem/vgem_drv.c         |  2 +-
 drivers/gpu/drm/virtio/virtgpu_object.c |  2 +-
 include/drm/drm_drv.h                   |  5 +++--
 10 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 7d4895de9e0d..cefd0cbf9deb 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -67,18 +67,21 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size, bool private)
 	struct drm_gem_object *gem_obj;
 	int ret = 0;

-	if (drm->driver->gem_create_object)
+	if (drm->driver->gem_create_object) {
 		gem_obj = drm->driver->gem_create_object(drm, size);
-	else
-		gem_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
-	if (!gem_obj)
-		return ERR_PTR(-ENOMEM);
+		if (IS_ERR(gem_obj))
+			return ERR_CAST(gem_obj);
+		cma_obj = to_drm_gem_cma_obj(gem_obj);
+	} else {
+		cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
+		if (!cma_obj)
+			return ERR_PTR(-ENOMEM);
+		gem_obj = &cma_obj->base;
+	}

 	if (!gem_obj->funcs)
 		gem_obj->funcs = &drm_gem_cma_default_funcs;

-	cma_obj = container_of(gem_obj, struct drm_gem_cma_object, base);
-
 	if (private) {
 		drm_gem_private_object_init(drm, gem_obj, size);

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 0eeda1012364..7915047cb041 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -56,14 +56,17 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private)

 	size = PAGE_ALIGN(size);

-	if (dev->driver->gem_create_object)
+	if (dev->driver->gem_create_object) {
 		obj = dev->driver->gem_create_object(dev, size);
-	else
-		obj = kzalloc(sizeof(*shmem), GFP_KERNEL);
-	if (!obj)
-		return ERR_PTR(-ENOMEM);
-
-	shmem = to_drm_gem_shmem_obj(obj);
+		if (IS_ERR(obj))
+			return ERR_CAST(obj);
+		shmem = to_drm_gem_shmem_obj(obj);
+	} else {
+		shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
+		if (!shmem)
+			return ERR_PTR(-ENOMEM);
+		obj = &shmem->base;
+	}

 	if (!obj->funcs)
 		obj->funcs = &drm_gem_shmem_funcs;
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index bfa386b98134..3f00192215d1 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -197,8 +197,8 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,

 	if (dev->driver->gem_create_object) {
 		gem = dev->driver->gem_create_object(dev, size);
-		if (!gem)
-			return ERR_PTR(-ENOMEM);
+		if (IS_ERR(gem))
+			return ERR_CAST(gem);
 		gbo = drm_gem_vram_of_gem(gem);
 	} else {
 		gbo = kzalloc(sizeof(*gbo), GFP_KERNEL);
diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
index 2723d333c608..f9a9198ef198 100644
--- a/drivers/gpu/drm/lima/lima_gem.c
+++ b/drivers/gpu/drm/lima/lima_gem.c
@@ -221,7 +221,7 @@ struct drm_gem_object *lima_gem_create_object(struct drm_device *dev, size_t siz

 	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
 	if (!bo)
-		return NULL;
+		return ERR_PTR(-ENOMEM);

 	mutex_init(&bo->lock);
 	INIT_LIST_HEAD(&bo->va);
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 6d9bdb9180cb..ead65f5fa2bc 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -223,7 +223,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t

 	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
 	if (!obj)
-		return NULL;
+		return ERR_PTR(-ENOMEM);

 	INIT_LIST_HEAD(&obj->mappings.list);
 	mutex_init(&obj->mappings.lock);
diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index 0d9af62f69ad..6e3113f419f4 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -70,11 +70,11 @@ struct drm_gem_object *v3d_create_object(struct drm_device *dev, size_t size)
 	struct drm_gem_object *obj;

 	if (size == 0)
-		return NULL;
+		return ERR_PTR(-EINVAL);

 	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
 	if (!bo)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 	obj = &bo->base.base;

 	obj->funcs = &v3d_gem_funcs;
diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 8520e440dbd1..6d1281a343e9 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -391,7 +391,7 @@ struct drm_gem_object *vc4_create_object(struct drm_device *dev, size_t size)

 	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
 	if (!bo)
-		return NULL;
+		return ERR_PTR(-ENOMEM);

 	bo->madv = VC4_MADV_WILLNEED;
 	refcount_set(&bo->usecnt, 0);
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index a87eafa89e9f..c5e3e5457737 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -97,7 +97,7 @@ static struct drm_gem_object *vgem_gem_create_object(struct drm_device *dev, siz

 	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
 	if (!obj)
-		return NULL;
+		return ERR_PTR(-ENOMEM);

 	/*
 	 * vgem doesn't have any begin/end cpu access ioctls, therefore must use
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index 187e10da2f17..baef2c5f2aaf 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -139,7 +139,7 @@ struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev,

 	shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
 	if (!shmem)
-		return NULL;
+		return ERR_PTR(-ENOMEM);

 	dshmem = &shmem->base.base;
 	dshmem->base.funcs = &virtio_gpu_shmem_funcs;
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index da0c836fe8e1..f6159acb8856 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -291,8 +291,9 @@ struct drm_driver {
 	/**
 	 * @gem_create_object: constructor for gem objects
 	 *
-	 * Hook for allocating the GEM object struct, for use by the CMA and
-	 * SHMEM GEM helpers.
+	 * Hook for allocating the GEM object struct, for use by the CMA
+	 * and SHMEM GEM helpers. Returns a GEM object on success, or an
+	 * ERR_PTR()-encoded error code otherwise.
 	 */
 	struct drm_gem_object *(*gem_create_object)(struct drm_device *dev,
 						    size_t size);
--
2.34.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
  2021-11-30  9:52 ` Thomas Zimmermann
  (?)
@ 2021-12-01 15:04 ` Maxime Ripard
  -1 siblings, 0 replies; 18+ messages in thread
From: Maxime Ripard @ 2021-12-01 15:04 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: kraxel, emma, tomeu.vizoso, airlied, dri-devel, steven.price,
	lima, yuq825, gurchetansingh, virtualization, dan.carpenter,
	alyssa.rosenzweig

[-- Attachment #1: Type: text/plain, Size: 905 bytes --]

On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann wrote:
> GEM helper libraries use struct drm_driver.gem_create_object to let
> drivers override GEM object allocation. On failure, the call returns
> NULL.
> 
> Change the semantics to make the calls return a pointer-encoded error.
> This aligns the callback with its callers. Fixes the ingenic driver,
> which already returns an error pointer.
> 
> Also update the callers to handle the involved types more strictly.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> There is an alternative patch at [1] that updates the value returned
> by ingenics' gem_create_object to NULL. Fixing the interface to return
> an errno code is more consistent with the rest of the GEM functions.
> 
> [1] https://lore.kernel.org/dri-devel/20211118111522.GD1147@kili/

Acked-by: Maxime Ripard <maxime@cerno.tech>

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
  2021-11-30  9:52 ` Thomas Zimmermann
  (?)
  (?)
@ 2021-12-01 15:16 ` Steven Price
  -1 siblings, 0 replies; 18+ messages in thread
From: Steven Price @ 2021-12-01 15:16 UTC (permalink / raw)
  To: Thomas Zimmermann, maarten.lankhorst, mripard, airlied, daniel,
	yuq825, robh, tomeu.vizoso, alyssa.rosenzweig, emma, kraxel,
	gurchetansingh, olvaffe, dan.carpenter
  Cc: lima, dri-devel, virtualization

On 30/11/2021 09:52, Thomas Zimmermann wrote:
> GEM helper libraries use struct drm_driver.gem_create_object to let
> drivers override GEM object allocation. On failure, the call returns
> NULL.
> 
> Change the semantics to make the calls return a pointer-encoded error.
> This aligns the callback with its callers. Fixes the ingenic driver,
> which already returns an error pointer.
> 
> Also update the callers to handle the involved types more strictly.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Steven Price <steven.price@arm.com>

> ---
> There is an alternative patch at [1] that updates the value returned
> by ingenics' gem_create_object to NULL. Fixing the interface to return
> an errno code is more consistent with the rest of the GEM functions.
> 
> [1] https://lore.kernel.org/dri-devel/20211118111522.GD1147@kili/
> ---
>  drivers/gpu/drm/drm_gem_cma_helper.c    | 17 ++++++++++-------
>  drivers/gpu/drm/drm_gem_shmem_helper.c  | 17 ++++++++++-------
>  drivers/gpu/drm/drm_gem_vram_helper.c   |  4 ++--
>  drivers/gpu/drm/lima/lima_gem.c         |  2 +-
>  drivers/gpu/drm/panfrost/panfrost_gem.c |  2 +-
>  drivers/gpu/drm/v3d/v3d_bo.c            |  4 ++--
>  drivers/gpu/drm/vc4/vc4_bo.c            |  2 +-
>  drivers/gpu/drm/vgem/vgem_drv.c         |  2 +-
>  drivers/gpu/drm/virtio/virtgpu_object.c |  2 +-
>  include/drm/drm_drv.h                   |  5 +++--
>  10 files changed, 32 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> index 7d4895de9e0d..cefd0cbf9deb 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -67,18 +67,21 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size, bool private)
>  	struct drm_gem_object *gem_obj;
>  	int ret = 0;
> 
> -	if (drm->driver->gem_create_object)
> +	if (drm->driver->gem_create_object) {
>  		gem_obj = drm->driver->gem_create_object(drm, size);
> -	else
> -		gem_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
> -	if (!gem_obj)
> -		return ERR_PTR(-ENOMEM);
> +		if (IS_ERR(gem_obj))
> +			return ERR_CAST(gem_obj);
> +		cma_obj = to_drm_gem_cma_obj(gem_obj);
> +	} else {
> +		cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
> +		if (!cma_obj)
> +			return ERR_PTR(-ENOMEM);
> +		gem_obj = &cma_obj->base;
> +	}
> 
>  	if (!gem_obj->funcs)
>  		gem_obj->funcs = &drm_gem_cma_default_funcs;
> 
> -	cma_obj = container_of(gem_obj, struct drm_gem_cma_object, base);
> -
>  	if (private) {
>  		drm_gem_private_object_init(drm, gem_obj, size);
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 0eeda1012364..7915047cb041 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -56,14 +56,17 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private)
> 
>  	size = PAGE_ALIGN(size);
> 
> -	if (dev->driver->gem_create_object)
> +	if (dev->driver->gem_create_object) {
>  		obj = dev->driver->gem_create_object(dev, size);
> -	else
> -		obj = kzalloc(sizeof(*shmem), GFP_KERNEL);
> -	if (!obj)
> -		return ERR_PTR(-ENOMEM);
> -
> -	shmem = to_drm_gem_shmem_obj(obj);
> +		if (IS_ERR(obj))
> +			return ERR_CAST(obj);
> +		shmem = to_drm_gem_shmem_obj(obj);
> +	} else {
> +		shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
> +		if (!shmem)
> +			return ERR_PTR(-ENOMEM);
> +		obj = &shmem->base;
> +	}
> 
>  	if (!obj->funcs)
>  		obj->funcs = &drm_gem_shmem_funcs;
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index bfa386b98134..3f00192215d1 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -197,8 +197,8 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
> 
>  	if (dev->driver->gem_create_object) {
>  		gem = dev->driver->gem_create_object(dev, size);
> -		if (!gem)
> -			return ERR_PTR(-ENOMEM);
> +		if (IS_ERR(gem))
> +			return ERR_CAST(gem);
>  		gbo = drm_gem_vram_of_gem(gem);
>  	} else {
>  		gbo = kzalloc(sizeof(*gbo), GFP_KERNEL);
> diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
> index 2723d333c608..f9a9198ef198 100644
> --- a/drivers/gpu/drm/lima/lima_gem.c
> +++ b/drivers/gpu/drm/lima/lima_gem.c
> @@ -221,7 +221,7 @@ struct drm_gem_object *lima_gem_create_object(struct drm_device *dev, size_t siz
> 
>  	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
>  	if (!bo)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
> 
>  	mutex_init(&bo->lock);
>  	INIT_LIST_HEAD(&bo->va);
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
> index 6d9bdb9180cb..ead65f5fa2bc 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
> @@ -223,7 +223,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t
> 
>  	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
>  	if (!obj)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
> 
>  	INIT_LIST_HEAD(&obj->mappings.list);
>  	mutex_init(&obj->mappings.lock);
> diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
> index 0d9af62f69ad..6e3113f419f4 100644
> --- a/drivers/gpu/drm/v3d/v3d_bo.c
> +++ b/drivers/gpu/drm/v3d/v3d_bo.c
> @@ -70,11 +70,11 @@ struct drm_gem_object *v3d_create_object(struct drm_device *dev, size_t size)
>  	struct drm_gem_object *obj;
> 
>  	if (size == 0)
> -		return NULL;
> +		return ERR_PTR(-EINVAL);
> 
>  	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
>  	if (!bo)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
>  	obj = &bo->base.base;
> 
>  	obj->funcs = &v3d_gem_funcs;
> diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
> index 8520e440dbd1..6d1281a343e9 100644
> --- a/drivers/gpu/drm/vc4/vc4_bo.c
> +++ b/drivers/gpu/drm/vc4/vc4_bo.c
> @@ -391,7 +391,7 @@ struct drm_gem_object *vc4_create_object(struct drm_device *dev, size_t size)
> 
>  	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
>  	if (!bo)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
> 
>  	bo->madv = VC4_MADV_WILLNEED;
>  	refcount_set(&bo->usecnt, 0);
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index a87eafa89e9f..c5e3e5457737 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -97,7 +97,7 @@ static struct drm_gem_object *vgem_gem_create_object(struct drm_device *dev, siz
> 
>  	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
>  	if (!obj)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
> 
>  	/*
>  	 * vgem doesn't have any begin/end cpu access ioctls, therefore must use
> diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
> index 187e10da2f17..baef2c5f2aaf 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_object.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_object.c
> @@ -139,7 +139,7 @@ struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev,
> 
>  	shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
>  	if (!shmem)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
> 
>  	dshmem = &shmem->base.base;
>  	dshmem->base.funcs = &virtio_gpu_shmem_funcs;
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index da0c836fe8e1..f6159acb8856 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -291,8 +291,9 @@ struct drm_driver {
>  	/**
>  	 * @gem_create_object: constructor for gem objects
>  	 *
> -	 * Hook for allocating the GEM object struct, for use by the CMA and
> -	 * SHMEM GEM helpers.
> +	 * Hook for allocating the GEM object struct, for use by the CMA
> +	 * and SHMEM GEM helpers. Returns a GEM object on success, or an
> +	 * ERR_PTR()-encoded error code otherwise.
>  	 */
>  	struct drm_gem_object *(*gem_create_object)(struct drm_device *dev,
>  						    size_t size);
> --
> 2.34.0
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
  2021-11-30  9:52 ` Thomas Zimmermann
@ 2021-12-06 10:42   ` Dan Carpenter
  -1 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-12-06 10:42 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: robh, emma, tomeu.vizoso, airlied, dri-devel, maarten.lankhorst,
	mripard, steven.price, lima, yuq825, daniel, gurchetansingh,
	virtualization, olvaffe, alyssa.rosenzweig

On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann wrote:
> GEM helper libraries use struct drm_driver.gem_create_object to let
> drivers override GEM object allocation. On failure, the call returns
> NULL.
> 
> Change the semantics to make the calls return a pointer-encoded error.
> This aligns the callback with its callers. Fixes the ingenic driver,
> which already returns an error pointer.
> 
> Also update the callers to handle the involved types more strictly.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> There is an alternative patch at [1] that updates the value returned
> by ingenics' gem_create_object to NULL. Fixing the interface to return
> an errno code is more consistent with the rest of the GEM functions.
> 
> [1] https://lore.kernel.org/dri-devel/20211118111522.GD1147@kili/ 

My fix was already applied and backported to -stable etc...  Your
patch is not developed against a current tree so you broke it.

That's the tricky thing with changing the API because say people wrote
their code last week where returning NULL was correct.  When they submit
their driver upstream, everything will merge and build but it will break
at runtime.

For now, it's only vc4_create_object() which is broken.

regards,
dan carpenter

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
@ 2021-12-06 10:42   ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-12-06 10:42 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: kraxel, emma, tomeu.vizoso, airlied, dri-devel, steven.price,
	lima, yuq825, gurchetansingh, virtualization, alyssa.rosenzweig

On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann wrote:
> GEM helper libraries use struct drm_driver.gem_create_object to let
> drivers override GEM object allocation. On failure, the call returns
> NULL.
> 
> Change the semantics to make the calls return a pointer-encoded error.
> This aligns the callback with its callers. Fixes the ingenic driver,
> which already returns an error pointer.
> 
> Also update the callers to handle the involved types more strictly.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> There is an alternative patch at [1] that updates the value returned
> by ingenics' gem_create_object to NULL. Fixing the interface to return
> an errno code is more consistent with the rest of the GEM functions.
> 
> [1] https://lore.kernel.org/dri-devel/20211118111522.GD1147@kili/ 

My fix was already applied and backported to -stable etc...  Your
patch is not developed against a current tree so you broke it.

That's the tricky thing with changing the API because say people wrote
their code last week where returning NULL was correct.  When they submit
their driver upstream, everything will merge and build but it will break
at runtime.

For now, it's only vc4_create_object() which is broken.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
  2021-12-06 10:42   ` Dan Carpenter
@ 2021-12-06 11:16     ` Thomas Zimmermann
  -1 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-12-06 11:16 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: kraxel, emma, tomeu.vizoso, airlied, dri-devel, steven.price,
	lima, yuq825, gurchetansingh, virtualization, alyssa.rosenzweig


[-- Attachment #1.1: Type: text/plain, Size: 2008 bytes --]

Hi

Am 06.12.21 um 11:42 schrieb Dan Carpenter:
> On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann wrote:
>> GEM helper libraries use struct drm_driver.gem_create_object to let
>> drivers override GEM object allocation. On failure, the call returns
>> NULL.
>>
>> Change the semantics to make the calls return a pointer-encoded error.
>> This aligns the callback with its callers. Fixes the ingenic driver,
>> which already returns an error pointer.
>>
>> Also update the callers to handle the involved types more strictly.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>> There is an alternative patch at [1] that updates the value returned
>> by ingenics' gem_create_object to NULL. Fixing the interface to return
>> an errno code is more consistent with the rest of the GEM functions.
>>
>> [1] https://lore.kernel.org/dri-devel/20211118111522.GD1147@kili/
> 
> My fix was already applied and backported to -stable etc...  Your
> patch is not developed against a current tree so you broke it.

Do you have a specific link? I just checked the stable tree at [1] and 
there no trace of your patch.

Patches for DRM should go through through DRM trees; drm-misc-fixes in 
this case. Exceptions should at least be announce on dri-devel. Neither 
is the case here.

Best regards
Thomas

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/ingenic/ingenic-drm-drv.c

> 
> That's the tricky thing with changing the API because say people wrote
> their code last week where returning NULL was correct.  When they submit
> their driver upstream, everything will merge and build but it will break
> at runtime.
> 
> For now, it's only vc4_create_object() which is broken.
> 
> regards,
> dan carpenter
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
@ 2021-12-06 11:16     ` Thomas Zimmermann
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-12-06 11:16 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: robh, emma, tomeu.vizoso, airlied, dri-devel, maarten.lankhorst,
	mripard, steven.price, lima, yuq825, daniel, gurchetansingh,
	virtualization, olvaffe, alyssa.rosenzweig


[-- Attachment #1.1.1: Type: text/plain, Size: 2008 bytes --]

Hi

Am 06.12.21 um 11:42 schrieb Dan Carpenter:
> On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann wrote:
>> GEM helper libraries use struct drm_driver.gem_create_object to let
>> drivers override GEM object allocation. On failure, the call returns
>> NULL.
>>
>> Change the semantics to make the calls return a pointer-encoded error.
>> This aligns the callback with its callers. Fixes the ingenic driver,
>> which already returns an error pointer.
>>
>> Also update the callers to handle the involved types more strictly.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>> There is an alternative patch at [1] that updates the value returned
>> by ingenics' gem_create_object to NULL. Fixing the interface to return
>> an errno code is more consistent with the rest of the GEM functions.
>>
>> [1] https://lore.kernel.org/dri-devel/20211118111522.GD1147@kili/
> 
> My fix was already applied and backported to -stable etc...  Your
> patch is not developed against a current tree so you broke it.

Do you have a specific link? I just checked the stable tree at [1] and 
there no trace of your patch.

Patches for DRM should go through through DRM trees; drm-misc-fixes in 
this case. Exceptions should at least be announce on dri-devel. Neither 
is the case here.

Best regards
Thomas

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/ingenic/ingenic-drm-drv.c

> 
> That's the tricky thing with changing the API because say people wrote
> their code last week where returning NULL was correct.  When they submit
> their driver upstream, everything will merge and build but it will break
> at runtime.
> 
> For now, it's only vc4_create_object() which is broken.
> 
> regards,
> dan carpenter
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
  2021-12-06 11:16     ` Thomas Zimmermann
@ 2021-12-06 14:40       ` Dan Carpenter
  -1 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-12-06 14:40 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: robh, emma, tomeu.vizoso, airlied, dri-devel, maarten.lankhorst,
	mripard, steven.price, lima, yuq825, daniel, gurchetansingh,
	virtualization, olvaffe, alyssa.rosenzweig

On Mon, Dec 06, 2021 at 12:16:24PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 06.12.21 um 11:42 schrieb Dan Carpenter:
> > On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann wrote:
> > > GEM helper libraries use struct drm_driver.gem_create_object to let
> > > drivers override GEM object allocation. On failure, the call returns
> > > NULL.
> > > 
> > > Change the semantics to make the calls return a pointer-encoded error.
> > > This aligns the callback with its callers. Fixes the ingenic driver,
> > > which already returns an error pointer.
> > > 
> > > Also update the callers to handle the involved types more strictly.
> > > 
> > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > ---
> > > There is an alternative patch at [1] that updates the value returned
> > > by ingenics' gem_create_object to NULL. Fixing the interface to return
> > > an errno code is more consistent with the rest of the GEM functions.
> > > 
> > > [1] https://lore.kernel.org/dri-devel/20211118111522.GD1147@kili/
> > 
> > My fix was already applied and backported to -stable etc...  Your
> > patch is not developed against a current tree so you broke it.
> 
> Do you have a specific link? I just checked the stable tree at [1] and there
> no trace of your patch.

It's in 5.15.6 and probably all the other supported -stable trees.

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/vc4/vc4_bo.c?h=v5.15.6#n387

> 
> Patches for DRM should go through through DRM trees; drm-misc-fixes in this
> case. Exceptions should at least be announce on dri-devel. Neither is the
> case here.

Yeah.  That's a good question.  I don't know, because I just work
against linux-next...

regards,
dan carpenter


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
@ 2021-12-06 14:40       ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-12-06 14:40 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: kraxel, emma, tomeu.vizoso, airlied, dri-devel, steven.price,
	lima, yuq825, gurchetansingh, virtualization, alyssa.rosenzweig

On Mon, Dec 06, 2021 at 12:16:24PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 06.12.21 um 11:42 schrieb Dan Carpenter:
> > On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann wrote:
> > > GEM helper libraries use struct drm_driver.gem_create_object to let
> > > drivers override GEM object allocation. On failure, the call returns
> > > NULL.
> > > 
> > > Change the semantics to make the calls return a pointer-encoded error.
> > > This aligns the callback with its callers. Fixes the ingenic driver,
> > > which already returns an error pointer.
> > > 
> > > Also update the callers to handle the involved types more strictly.
> > > 
> > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > ---
> > > There is an alternative patch at [1] that updates the value returned
> > > by ingenics' gem_create_object to NULL. Fixing the interface to return
> > > an errno code is more consistent with the rest of the GEM functions.
> > > 
> > > [1] https://lore.kernel.org/dri-devel/20211118111522.GD1147@kili/
> > 
> > My fix was already applied and backported to -stable etc...  Your
> > patch is not developed against a current tree so you broke it.
> 
> Do you have a specific link? I just checked the stable tree at [1] and there
> no trace of your patch.

It's in 5.15.6 and probably all the other supported -stable trees.

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/vc4/vc4_bo.c?h=v5.15.6#n387

> 
> Patches for DRM should go through through DRM trees; drm-misc-fixes in this
> case. Exceptions should at least be announce on dri-devel. Neither is the
> case here.

Yeah.  That's a good question.  I don't know, because I just work
against linux-next...

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
  2021-12-06 14:40       ` Dan Carpenter
@ 2021-12-07  8:24         ` Thomas Zimmermann
  -1 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-12-07  8:24 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: emma, tomeu.vizoso, airlied, alyssa.rosenzweig, dri-devel,
	steven.price, gurchetansingh, yuq825, virtualization, lima


[-- Attachment #1.1.1: Type: text/plain, Size: 2424 bytes --]

Hi

Am 06.12.21 um 15:40 schrieb Dan Carpenter:
> On Mon, Dec 06, 2021 at 12:16:24PM +0100, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 06.12.21 um 11:42 schrieb Dan Carpenter:
>>> On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann wrote:
>>>> GEM helper libraries use struct drm_driver.gem_create_object to let
>>>> drivers override GEM object allocation. On failure, the call returns
>>>> NULL.
>>>>
>>>> Change the semantics to make the calls return a pointer-encoded error.
>>>> This aligns the callback with its callers. Fixes the ingenic driver,
>>>> which already returns an error pointer.
>>>>
>>>> Also update the callers to handle the involved types more strictly.
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> ---
>>>> There is an alternative patch at [1] that updates the value returned
>>>> by ingenics' gem_create_object to NULL. Fixing the interface to return
>>>> an errno code is more consistent with the rest of the GEM functions.
>>>>
>>>> [1] https://lore.kernel.org/dri-devel/20211118111522.GD1147@kili/
>>>
>>> My fix was already applied and backported to -stable etc...  Your
>>> patch is not developed against a current tree so you broke it.
>>
>> Do you have a specific link? I just checked the stable tree at [1] and there
>> no trace of your patch.
> 
> It's in 5.15.6 and probably all the other supported -stable trees.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/vc4/vc4_bo.c?h=v5.15.6#n387

I'm not sure that I understand the problem.

The URL points to vc4, but my link was to your patch for ingenic. vc4 is 
being updated here as well to ERR_PTR. The ingenic patch never made it 
into any tree. It actually was the reason to fix the interface.

When the current patch makes it through the trees, it should fix all the 
affected drivers.

Best regards
Thomas

> 
>>
>> Patches for DRM should go through through DRM trees; drm-misc-fixes in this
>> case. Exceptions should at least be announce on dri-devel. Neither is the
>> case here.
> 
> Yeah.  That's a good question.  I don't know, because I just work
> against linux-next...
> 
> regards,
> dan carpenter
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
@ 2021-12-07  8:24         ` Thomas Zimmermann
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-12-07  8:24 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: emma, tomeu.vizoso, airlied, alyssa.rosenzweig, dri-devel,
	steven.price, gurchetansingh, kraxel, yuq825, virtualization,
	lima


[-- Attachment #1.1: Type: text/plain, Size: 2424 bytes --]

Hi

Am 06.12.21 um 15:40 schrieb Dan Carpenter:
> On Mon, Dec 06, 2021 at 12:16:24PM +0100, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 06.12.21 um 11:42 schrieb Dan Carpenter:
>>> On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann wrote:
>>>> GEM helper libraries use struct drm_driver.gem_create_object to let
>>>> drivers override GEM object allocation. On failure, the call returns
>>>> NULL.
>>>>
>>>> Change the semantics to make the calls return a pointer-encoded error.
>>>> This aligns the callback with its callers. Fixes the ingenic driver,
>>>> which already returns an error pointer.
>>>>
>>>> Also update the callers to handle the involved types more strictly.
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> ---
>>>> There is an alternative patch at [1] that updates the value returned
>>>> by ingenics' gem_create_object to NULL. Fixing the interface to return
>>>> an errno code is more consistent with the rest of the GEM functions.
>>>>
>>>> [1] https://lore.kernel.org/dri-devel/20211118111522.GD1147@kili/
>>>
>>> My fix was already applied and backported to -stable etc...  Your
>>> patch is not developed against a current tree so you broke it.
>>
>> Do you have a specific link? I just checked the stable tree at [1] and there
>> no trace of your patch.
> 
> It's in 5.15.6 and probably all the other supported -stable trees.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/vc4/vc4_bo.c?h=v5.15.6#n387

I'm not sure that I understand the problem.

The URL points to vc4, but my link was to your patch for ingenic. vc4 is 
being updated here as well to ERR_PTR. The ingenic patch never made it 
into any tree. It actually was the reason to fix the interface.

When the current patch makes it through the trees, it should fix all the 
affected drivers.

Best regards
Thomas

> 
>>
>> Patches for DRM should go through through DRM trees; drm-misc-fixes in this
>> case. Exceptions should at least be announce on dri-devel. Neither is the
>> case here.
> 
> Yeah.  That's a good question.  I don't know, because I just work
> against linux-next...
> 
> regards,
> dan carpenter
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
  2021-12-07  8:24         ` Thomas Zimmermann
@ 2021-12-07  8:55           ` Dan Carpenter
  -1 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-12-07  8:55 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: emma, tomeu.vizoso, airlied, alyssa.rosenzweig, dri-devel,
	steven.price, gurchetansingh, yuq825, virtualization, lima

I appologize.  This thread has been really frustrating.  I got mixed up
because I recently sent patches for ingenic and vc4.  Also we are
working against different trees so maybe that is part of the problem?

I'm looking at today's linux-next.  Your patch has been applied.

Yes.  You updated all the drivers.  But somehow the vc4 chunk from your
patch was dropped.  It was *NOT* dropped by Stephen Rothwell.  It got
dropped earlier.  I am including the `git format-patch -1 <hash>` output
from the commit.

regards,
dan carpenter


From 4ff22f487f8c26b99cbe1678344595734c001a39 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Tue, 30 Nov 2021 10:52:55 +0100
Subject: [PATCH] drm: Return error codes from struct
 drm_driver.gem_create_object

GEM helper libraries use struct drm_driver.gem_create_object to let
drivers override GEM object allocation. On failure, the call returns
NULL.

Change the semantics to make the calls return a pointer-encoded error.
This aligns the callback with its callers. Fixes the ingenic driver,
which already returns an error pointer.

Also update the callers to handle the involved types more strictly.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Steven Price <steven.price@arm.com>
Acked-by: Maxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20211130095255.26710-1-tzimmermann@suse.de
---
 drivers/gpu/drm/drm_gem_cma_helper.c    | 17 ++++++++++-------
 drivers/gpu/drm/drm_gem_shmem_helper.c  | 17 ++++++++++-------
 drivers/gpu/drm/drm_gem_vram_helper.c   |  4 ++--
 drivers/gpu/drm/lima/lima_gem.c         |  2 +-
 drivers/gpu/drm/panfrost/panfrost_gem.c |  2 +-
 drivers/gpu/drm/v3d/v3d_bo.c            |  4 ++--
 drivers/gpu/drm/vgem/vgem_drv.c         |  2 +-
 drivers/gpu/drm/virtio/virtgpu_object.c |  2 +-
 include/drm/drm_drv.h                   |  5 +++--
 9 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 5f42e44b2ab3..6f18f143dd30 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -67,18 +67,21 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size, bool private)
 	struct drm_gem_object *gem_obj;
 	int ret = 0;
 
-	if (drm->driver->gem_create_object)
+	if (drm->driver->gem_create_object) {
 		gem_obj = drm->driver->gem_create_object(drm, size);
-	else
-		gem_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
-	if (!gem_obj)
-		return ERR_PTR(-ENOMEM);
+		if (IS_ERR(gem_obj))
+			return ERR_CAST(gem_obj);
+		cma_obj = to_drm_gem_cma_obj(gem_obj);
+	} else {
+		cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
+		if (!cma_obj)
+			return ERR_PTR(-ENOMEM);
+		gem_obj = &cma_obj->base;
+	}
 
 	if (!gem_obj->funcs)
 		gem_obj->funcs = &drm_gem_cma_default_funcs;
 
-	cma_obj = container_of(gem_obj, struct drm_gem_cma_object, base);
-
 	if (private) {
 		drm_gem_private_object_init(drm, gem_obj, size);
 
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 0eeda1012364..7915047cb041 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -56,14 +56,17 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private)
 
 	size = PAGE_ALIGN(size);
 
-	if (dev->driver->gem_create_object)
+	if (dev->driver->gem_create_object) {
 		obj = dev->driver->gem_create_object(dev, size);
-	else
-		obj = kzalloc(sizeof(*shmem), GFP_KERNEL);
-	if (!obj)
-		return ERR_PTR(-ENOMEM);
-
-	shmem = to_drm_gem_shmem_obj(obj);
+		if (IS_ERR(obj))
+			return ERR_CAST(obj);
+		shmem = to_drm_gem_shmem_obj(obj);
+	} else {
+		shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
+		if (!shmem)
+			return ERR_PTR(-ENOMEM);
+		obj = &shmem->base;
+	}
 
 	if (!obj->funcs)
 		obj->funcs = &drm_gem_shmem_funcs;
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index bfa386b98134..3f00192215d1 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -197,8 +197,8 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
 
 	if (dev->driver->gem_create_object) {
 		gem = dev->driver->gem_create_object(dev, size);
-		if (!gem)
-			return ERR_PTR(-ENOMEM);
+		if (IS_ERR(gem))
+			return ERR_CAST(gem);
 		gbo = drm_gem_vram_of_gem(gem);
 	} else {
 		gbo = kzalloc(sizeof(*gbo), GFP_KERNEL);
diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
index 2723d333c608..f9a9198ef198 100644
--- a/drivers/gpu/drm/lima/lima_gem.c
+++ b/drivers/gpu/drm/lima/lima_gem.c
@@ -221,7 +221,7 @@ struct drm_gem_object *lima_gem_create_object(struct drm_device *dev, size_t siz
 
 	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
 	if (!bo)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	mutex_init(&bo->lock);
 	INIT_LIST_HEAD(&bo->va);
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 6d9bdb9180cb..ead65f5fa2bc 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -223,7 +223,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t
 
 	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
 	if (!obj)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	INIT_LIST_HEAD(&obj->mappings.list);
 	mutex_init(&obj->mappings.lock);
diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index 0d9af62f69ad..6e3113f419f4 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -70,11 +70,11 @@ struct drm_gem_object *v3d_create_object(struct drm_device *dev, size_t size)
 	struct drm_gem_object *obj;
 
 	if (size == 0)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
 	if (!bo)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 	obj = &bo->base.base;
 
 	obj->funcs = &v3d_gem_funcs;
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index a87eafa89e9f..c5e3e5457737 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -97,7 +97,7 @@ static struct drm_gem_object *vgem_gem_create_object(struct drm_device *dev, siz
 
 	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
 	if (!obj)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	/*
 	 * vgem doesn't have any begin/end cpu access ioctls, therefore must use
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index 187e10da2f17..baef2c5f2aaf 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -139,7 +139,7 @@ struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev,
 
 	shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
 	if (!shmem)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	dshmem = &shmem->base.base;
 	dshmem->base.funcs = &virtio_gpu_shmem_funcs;
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index da0c836fe8e1..f6159acb8856 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -291,8 +291,9 @@ struct drm_driver {
 	/**
 	 * @gem_create_object: constructor for gem objects
 	 *
-	 * Hook for allocating the GEM object struct, for use by the CMA and
-	 * SHMEM GEM helpers.
+	 * Hook for allocating the GEM object struct, for use by the CMA
+	 * and SHMEM GEM helpers. Returns a GEM object on success, or an
+	 * ERR_PTR()-encoded error code otherwise.
 	 */
 	struct drm_gem_object *(*gem_create_object)(struct drm_device *dev,
 						    size_t size);
-- 
2.20.1


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
@ 2021-12-07  8:55           ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2021-12-07  8:55 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: emma, tomeu.vizoso, airlied, alyssa.rosenzweig, dri-devel,
	steven.price, gurchetansingh, kraxel, yuq825, virtualization,
	lima

I appologize.  This thread has been really frustrating.  I got mixed up
because I recently sent patches for ingenic and vc4.  Also we are
working against different trees so maybe that is part of the problem?

I'm looking at today's linux-next.  Your patch has been applied.

Yes.  You updated all the drivers.  But somehow the vc4 chunk from your
patch was dropped.  It was *NOT* dropped by Stephen Rothwell.  It got
dropped earlier.  I am including the `git format-patch -1 <hash>` output
from the commit.

regards,
dan carpenter


From 4ff22f487f8c26b99cbe1678344595734c001a39 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Tue, 30 Nov 2021 10:52:55 +0100
Subject: [PATCH] drm: Return error codes from struct
 drm_driver.gem_create_object

GEM helper libraries use struct drm_driver.gem_create_object to let
drivers override GEM object allocation. On failure, the call returns
NULL.

Change the semantics to make the calls return a pointer-encoded error.
This aligns the callback with its callers. Fixes the ingenic driver,
which already returns an error pointer.

Also update the callers to handle the involved types more strictly.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Steven Price <steven.price@arm.com>
Acked-by: Maxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20211130095255.26710-1-tzimmermann@suse.de
---
 drivers/gpu/drm/drm_gem_cma_helper.c    | 17 ++++++++++-------
 drivers/gpu/drm/drm_gem_shmem_helper.c  | 17 ++++++++++-------
 drivers/gpu/drm/drm_gem_vram_helper.c   |  4 ++--
 drivers/gpu/drm/lima/lima_gem.c         |  2 +-
 drivers/gpu/drm/panfrost/panfrost_gem.c |  2 +-
 drivers/gpu/drm/v3d/v3d_bo.c            |  4 ++--
 drivers/gpu/drm/vgem/vgem_drv.c         |  2 +-
 drivers/gpu/drm/virtio/virtgpu_object.c |  2 +-
 include/drm/drm_drv.h                   |  5 +++--
 9 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 5f42e44b2ab3..6f18f143dd30 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -67,18 +67,21 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size, bool private)
 	struct drm_gem_object *gem_obj;
 	int ret = 0;
 
-	if (drm->driver->gem_create_object)
+	if (drm->driver->gem_create_object) {
 		gem_obj = drm->driver->gem_create_object(drm, size);
-	else
-		gem_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
-	if (!gem_obj)
-		return ERR_PTR(-ENOMEM);
+		if (IS_ERR(gem_obj))
+			return ERR_CAST(gem_obj);
+		cma_obj = to_drm_gem_cma_obj(gem_obj);
+	} else {
+		cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
+		if (!cma_obj)
+			return ERR_PTR(-ENOMEM);
+		gem_obj = &cma_obj->base;
+	}
 
 	if (!gem_obj->funcs)
 		gem_obj->funcs = &drm_gem_cma_default_funcs;
 
-	cma_obj = container_of(gem_obj, struct drm_gem_cma_object, base);
-
 	if (private) {
 		drm_gem_private_object_init(drm, gem_obj, size);
 
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 0eeda1012364..7915047cb041 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -56,14 +56,17 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private)
 
 	size = PAGE_ALIGN(size);
 
-	if (dev->driver->gem_create_object)
+	if (dev->driver->gem_create_object) {
 		obj = dev->driver->gem_create_object(dev, size);
-	else
-		obj = kzalloc(sizeof(*shmem), GFP_KERNEL);
-	if (!obj)
-		return ERR_PTR(-ENOMEM);
-
-	shmem = to_drm_gem_shmem_obj(obj);
+		if (IS_ERR(obj))
+			return ERR_CAST(obj);
+		shmem = to_drm_gem_shmem_obj(obj);
+	} else {
+		shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
+		if (!shmem)
+			return ERR_PTR(-ENOMEM);
+		obj = &shmem->base;
+	}
 
 	if (!obj->funcs)
 		obj->funcs = &drm_gem_shmem_funcs;
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index bfa386b98134..3f00192215d1 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -197,8 +197,8 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
 
 	if (dev->driver->gem_create_object) {
 		gem = dev->driver->gem_create_object(dev, size);
-		if (!gem)
-			return ERR_PTR(-ENOMEM);
+		if (IS_ERR(gem))
+			return ERR_CAST(gem);
 		gbo = drm_gem_vram_of_gem(gem);
 	} else {
 		gbo = kzalloc(sizeof(*gbo), GFP_KERNEL);
diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
index 2723d333c608..f9a9198ef198 100644
--- a/drivers/gpu/drm/lima/lima_gem.c
+++ b/drivers/gpu/drm/lima/lima_gem.c
@@ -221,7 +221,7 @@ struct drm_gem_object *lima_gem_create_object(struct drm_device *dev, size_t siz
 
 	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
 	if (!bo)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	mutex_init(&bo->lock);
 	INIT_LIST_HEAD(&bo->va);
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 6d9bdb9180cb..ead65f5fa2bc 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -223,7 +223,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t
 
 	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
 	if (!obj)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	INIT_LIST_HEAD(&obj->mappings.list);
 	mutex_init(&obj->mappings.lock);
diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index 0d9af62f69ad..6e3113f419f4 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -70,11 +70,11 @@ struct drm_gem_object *v3d_create_object(struct drm_device *dev, size_t size)
 	struct drm_gem_object *obj;
 
 	if (size == 0)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
 	if (!bo)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 	obj = &bo->base.base;
 
 	obj->funcs = &v3d_gem_funcs;
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index a87eafa89e9f..c5e3e5457737 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -97,7 +97,7 @@ static struct drm_gem_object *vgem_gem_create_object(struct drm_device *dev, siz
 
 	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
 	if (!obj)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	/*
 	 * vgem doesn't have any begin/end cpu access ioctls, therefore must use
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index 187e10da2f17..baef2c5f2aaf 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -139,7 +139,7 @@ struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev,
 
 	shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
 	if (!shmem)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	dshmem = &shmem->base.base;
 	dshmem->base.funcs = &virtio_gpu_shmem_funcs;
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index da0c836fe8e1..f6159acb8856 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -291,8 +291,9 @@ struct drm_driver {
 	/**
 	 * @gem_create_object: constructor for gem objects
 	 *
-	 * Hook for allocating the GEM object struct, for use by the CMA and
-	 * SHMEM GEM helpers.
+	 * Hook for allocating the GEM object struct, for use by the CMA
+	 * and SHMEM GEM helpers. Returns a GEM object on success, or an
+	 * ERR_PTR()-encoded error code otherwise.
 	 */
 	struct drm_gem_object *(*gem_create_object)(struct drm_device *dev,
 						    size_t size);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
  2021-12-07  8:55           ` Dan Carpenter
@ 2021-12-07  9:24             ` Thomas Zimmermann
  -1 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-12-07  9:24 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: emma, tomeu.vizoso, airlied, alyssa.rosenzweig, dri-devel,
	steven.price, gurchetansingh, yuq825, virtualization, lima


[-- Attachment #1.1.1: Type: text/plain, Size: 1336 bytes --]

Hi

Am 07.12.21 um 09:55 schrieb Dan Carpenter:
> I appologize.  This thread has been really frustrating.  I got mixed up
> because I recently sent patches for ingenic and vc4.  Also we are
> working against different trees so maybe that is part of the problem?
> 
> I'm looking at today's linux-next.  Your patch has been applied.
> 
> Yes.  You updated all the drivers.  But somehow the vc4 chunk from your
> patch was dropped.  It was *NOT* dropped by Stephen Rothwell.  It got
> dropped earlier.  I am including the `git format-patch -1 <hash>` output
> from the commit.

My vc4 change is in drm-misc-next, [1] but not in drm-tip. [2] Your vc4 
patch went through drm-misc-fixes.

drm-tip is build automatically by our DRM tools from the various trees. 
The tools took my patch from drm-misc-next and your patch from 
drm-misc-fixes and the result was broken.

Thanks for bringing this up. I'll see what I can do about it.

Best regards
Thomas

[1] 
https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/vc4/vc4_bo.c#n394
[2] 
https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/vc4/vc4_bo.c


-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
@ 2021-12-07  9:24             ` Thomas Zimmermann
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-12-07  9:24 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: emma, tomeu.vizoso, airlied, alyssa.rosenzweig, dri-devel,
	steven.price, gurchetansingh, kraxel, yuq825, virtualization,
	lima


[-- Attachment #1.1: Type: text/plain, Size: 1336 bytes --]

Hi

Am 07.12.21 um 09:55 schrieb Dan Carpenter:
> I appologize.  This thread has been really frustrating.  I got mixed up
> because I recently sent patches for ingenic and vc4.  Also we are
> working against different trees so maybe that is part of the problem?
> 
> I'm looking at today's linux-next.  Your patch has been applied.
> 
> Yes.  You updated all the drivers.  But somehow the vc4 chunk from your
> patch was dropped.  It was *NOT* dropped by Stephen Rothwell.  It got
> dropped earlier.  I am including the `git format-patch -1 <hash>` output
> from the commit.

My vc4 change is in drm-misc-next, [1] but not in drm-tip. [2] Your vc4 
patch went through drm-misc-fixes.

drm-tip is build automatically by our DRM tools from the various trees. 
The tools took my patch from drm-misc-next and your patch from 
drm-misc-fixes and the result was broken.

Thanks for bringing this up. I'll see what I can do about it.

Best regards
Thomas

[1] 
https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/vc4/vc4_bo.c#n394
[2] 
https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/vc4/vc4_bo.c


-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
  2021-12-07  9:24             ` Thomas Zimmermann
@ 2021-12-08 10:22               ` Thomas Zimmermann
  -1 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-12-08 10:22 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: yuq825, emma, tomeu.vizoso, airlied, dri-devel, steven.price,
	lima, alyssa.rosenzweig, gurchetansingh, virtualization, kraxel


[-- Attachment #1.1: Type: text/plain, Size: 1689 bytes --]

Hi

Am 07.12.21 um 10:24 schrieb Thomas Zimmermann:
> Hi
> 
> Am 07.12.21 um 09:55 schrieb Dan Carpenter:
>> I appologize.  This thread has been really frustrating.  I got mixed up
>> because I recently sent patches for ingenic and vc4.  Also we are
>> working against different trees so maybe that is part of the problem?
>>
>> I'm looking at today's linux-next.  Your patch has been applied.
>>
>> Yes.  You updated all the drivers.  But somehow the vc4 chunk from your
>> patch was dropped.  It was *NOT* dropped by Stephen Rothwell.  It got
>> dropped earlier.  I am including the `git format-patch -1 <hash>` output
>> from the commit.
> 
> My vc4 change is in drm-misc-next, [1] but not in drm-tip. [2] Your vc4 
> patch went through drm-misc-fixes.
> 
> drm-tip is build automatically by our DRM tools from the various trees. 
> The tools took my patch from drm-misc-next and your patch from 
> drm-misc-fixes and the result was broken.
> 
> Thanks for bringing this up. I'll see what I can do about it.

FYI I fixed drm-tip to return a pointer-encoded error in vc4. [1]

Best regards
Thomas

[1] 
https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/vc4/vc4_bo.c?id=cc3b9eb186e3c8dfb0bcc7d54fa4bfbe52c0b58b#n394


> 
> Best regards
> Thomas
> 
> [1] 
> https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/vc4/vc4_bo.c#n394 
> 
> [2] 
> https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/vc4/vc4_bo.c
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
@ 2021-12-08 10:22               ` Thomas Zimmermann
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Zimmermann @ 2021-12-08 10:22 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: yuq825, emma, tomeu.vizoso, airlied, dri-devel, steven.price,
	lima, alyssa.rosenzweig, gurchetansingh, virtualization


[-- Attachment #1.1.1: Type: text/plain, Size: 1689 bytes --]

Hi

Am 07.12.21 um 10:24 schrieb Thomas Zimmermann:
> Hi
> 
> Am 07.12.21 um 09:55 schrieb Dan Carpenter:
>> I appologize.  This thread has been really frustrating.  I got mixed up
>> because I recently sent patches for ingenic and vc4.  Also we are
>> working against different trees so maybe that is part of the problem?
>>
>> I'm looking at today's linux-next.  Your patch has been applied.
>>
>> Yes.  You updated all the drivers.  But somehow the vc4 chunk from your
>> patch was dropped.  It was *NOT* dropped by Stephen Rothwell.  It got
>> dropped earlier.  I am including the `git format-patch -1 <hash>` output
>> from the commit.
> 
> My vc4 change is in drm-misc-next, [1] but not in drm-tip. [2] Your vc4 
> patch went through drm-misc-fixes.
> 
> drm-tip is build automatically by our DRM tools from the various trees. 
> The tools took my patch from drm-misc-next and your patch from 
> drm-misc-fixes and the result was broken.
> 
> Thanks for bringing this up. I'll see what I can do about it.

FYI I fixed drm-tip to return a pointer-encoded error in vc4. [1]

Best regards
Thomas

[1] 
https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/vc4/vc4_bo.c?id=cc3b9eb186e3c8dfb0bcc7d54fa4bfbe52c0b58b#n394


> 
> Best regards
> Thomas
> 
> [1] 
> https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/vc4/vc4_bo.c#n394 
> 
> [2] 
> https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/vc4/vc4_bo.c
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-12-08 10:22 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-30  9:52 [PATCH] drm: Return error codes from struct drm_driver.gem_create_object Thomas Zimmermann
2021-11-30  9:52 ` Thomas Zimmermann
2021-12-01 15:04 ` Maxime Ripard
2021-12-01 15:16 ` Steven Price
2021-12-06 10:42 ` Dan Carpenter
2021-12-06 10:42   ` Dan Carpenter
2021-12-06 11:16   ` Thomas Zimmermann
2021-12-06 11:16     ` Thomas Zimmermann
2021-12-06 14:40     ` Dan Carpenter
2021-12-06 14:40       ` Dan Carpenter
2021-12-07  8:24       ` Thomas Zimmermann
2021-12-07  8:24         ` Thomas Zimmermann
2021-12-07  8:55         ` Dan Carpenter
2021-12-07  8:55           ` Dan Carpenter
2021-12-07  9:24           ` Thomas Zimmermann
2021-12-07  9:24             ` Thomas Zimmermann
2021-12-08 10:22             ` Thomas Zimmermann
2021-12-08 10:22               ` Thomas Zimmermann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.