Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

20 participants
2956 discussions

[PATCH v3] drm/framebuffer: Acquire internal references on GEM handles

by Thomas Zimmermann

Acquire GEM handles in drm_framebuffer_init() and release them in the corresponding drm_framebuffer_cleanup(). Ties the handle's lifetime to the framebuffer. Not all GEM buffer objects have GEM handles. If not set, no refcounting takes place. This is the case for some fbdev emulation. This is not a problem as these GEM objects do not use dma-bufs and drivers will not release them while fbdev emulation is running. Framebuffer flags keep a bit per color plane of which the framebuffer holds a GEM handle reference. As all drivers use drm_framebuffer_init(), they will now all hold dma-buf references as fixed in commit 5307dce878d4 ("drm/gem: Acquire references on GEM handles for framebuffers"). In the GEM framebuffer helpers, restore the original ref counting on buffer objects. As the helpers for handle refcounting are now no longer called from outside the DRM core, unexport the symbols. v3: - don't mix internal flags with mode flags (Christian) v2: - track framebuffer handle refs by flag - drop gma500 cleanup (Christian) Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> Fixes: 5307dce878d4 ("drm/gem: Acquire references on GEM handles for framebuffers") Reported-by: Bert Karwatzki <spasswolf(a)web.de> Closes: https://lore.kernel.org/dri-devel/20250703115915.3096-1-spasswolf@web.de/ Tested-by: Bert Karwatzki <spasswolf(a)web.de> Tested-by: Mario Limonciello <superm1(a)kernel.org> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Anusha Srivatsa <asrivats(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: <stable(a)vger.kernel.org> --- drivers/gpu/drm/drm_framebuffer.c | 31 ++++++++++++++-- drivers/gpu/drm/drm_gem.c | 38 ++++++++++++-------- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 16 ++++----- drivers/gpu/drm/drm_internal.h | 2 +- include/drm/drm_framebuffer.h | 7 ++++ 5 files changed, 68 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index b781601946db..63a70f285cce 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -862,11 +862,23 @@ EXPORT_SYMBOL_FOR_TESTS_ONLY(drm_framebuffer_free); int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb, const struct drm_framebuffer_funcs *funcs) { + unsigned int i; int ret; + bool exists; if (WARN_ON_ONCE(fb->dev != dev || !fb->format)) return -EINVAL; + for (i = 0; i < fb->format->num_planes; i++) { + if (drm_WARN_ON_ONCE(dev, fb->internal_flags & DRM_FRAMEBUFFER_HAS_HANDLE_REF(i))) + fb->internal_flags &= ~DRM_FRAMEBUFFER_HAS_HANDLE_REF(i); + if (fb->obj[i]) { + exists = drm_gem_object_handle_get_if_exists_unlocked(fb->obj[i]); + if (exists) + fb->internal_flags |= DRM_FRAMEBUFFER_HAS_HANDLE_REF(i); + } + } + INIT_LIST_HEAD(&fb->filp_head); fb->funcs = funcs; @@ -875,7 +887,7 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb, ret = __drm_mode_object_add(dev, &fb->base, DRM_MODE_OBJECT_FB, false, drm_framebuffer_free); if (ret) - goto out; + goto err; mutex_lock(&dev->mode_config.fb_lock); dev->mode_config.num_fb++; @@ -883,7 +895,16 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb, mutex_unlock(&dev->mode_config.fb_lock); drm_mode_object_register(dev, &fb->base); -out: + + return 0; + +err: + for (i = 0; i < fb->format->num_planes; i++) { + if (fb->internal_flags & DRM_FRAMEBUFFER_HAS_HANDLE_REF(i)) { + drm_gem_object_handle_put_unlocked(fb->obj[i]); + fb->internal_flags &= ~DRM_FRAMEBUFFER_HAS_HANDLE_REF(i); + } + } return ret; } EXPORT_SYMBOL(drm_framebuffer_init); @@ -960,6 +981,12 @@ EXPORT_SYMBOL(drm_framebuffer_unregister_private); void drm_framebuffer_cleanup(struct drm_framebuffer *fb) { struct drm_device *dev = fb->dev; + unsigned int i; + + for (i = 0; i < fb->format->num_planes; i++) { + if (fb->internal_flags & DRM_FRAMEBUFFER_HAS_HANDLE_REF(i)) + drm_gem_object_handle_put_unlocked(fb->obj[i]); + } mutex_lock(&dev->mode_config.fb_lock); list_del(&fb->head); diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index bc505d938b3e..41cdab6088ae 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -224,23 +224,34 @@ static void drm_gem_object_handle_get(struct drm_gem_object *obj) } /** - * drm_gem_object_handle_get_unlocked - acquire reference on user-space handles + * drm_gem_object_handle_get_if_exists_unlocked - acquire reference on user-space handle, if any * @obj: GEM object * - * Acquires a reference on the GEM buffer object's handle. Required - * to keep the GEM object alive. Call drm_gem_object_handle_put_unlocked() - * to release the reference. + * Acquires a reference on the GEM buffer object's handle. Required to keep + * the GEM object alive. Call drm_gem_object_handle_put_if_exists_unlocked() + * to release the reference. Does nothing if the buffer object has no handle. + * + * Returns: + * True if a handle exists, or false otherwise */ -void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj) +bool drm_gem_object_handle_get_if_exists_unlocked(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; guard(mutex)(&dev->object_name_lock); - drm_WARN_ON(dev, !obj->handle_count); /* first ref taken in create-tail helper */ + /* + * First ref taken during GEM object creation, if any. Some + * drivers set up internal framebuffers with GEM objects that + * do not have a GEM handle. Hence, this counter can be zero. + */ + if (!obj->handle_count) + return false; + drm_gem_object_handle_get(obj); + + return true; } -EXPORT_SYMBOL(drm_gem_object_handle_get_unlocked); /** * drm_gem_object_handle_free - release resources bound to userspace handles @@ -273,7 +284,7 @@ static void drm_gem_object_exported_dma_buf_free(struct drm_gem_object *obj) } /** - * drm_gem_object_handle_put_unlocked - releases reference on user-space handles + * drm_gem_object_handle_put_unlocked - releases reference on user-space handle * @obj: GEM object * * Releases a reference on the GEM buffer object's handle. Possibly releases @@ -284,14 +295,14 @@ void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj) struct drm_device *dev = obj->dev; bool final = false; - if (WARN_ON(READ_ONCE(obj->handle_count) == 0)) + if (drm_WARN_ON(dev, READ_ONCE(obj->handle_count) == 0)) return; /* - * Must bump handle count first as this may be the last - * ref, in which case the object would disappear before we - * checked for a name - */ + * Must bump handle count first as this may be the last + * ref, in which case the object would disappear before + * we checked for a name. + */ mutex_lock(&dev->object_name_lock); if (--obj->handle_count == 0) { @@ -304,7 +315,6 @@ void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj) if (final) drm_gem_object_put(obj); } -EXPORT_SYMBOL(drm_gem_object_handle_put_unlocked); /* * Called at device or object close to release the file's diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index c60d0044d036..618ce725cd75 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -100,7 +100,7 @@ void drm_gem_fb_destroy(struct drm_framebuffer *fb) unsigned int i; for (i = 0; i < fb->format->num_planes; i++) - drm_gem_object_handle_put_unlocked(fb->obj[i]); + drm_gem_object_put(fb->obj[i]); drm_framebuffer_cleanup(fb); kfree(fb); @@ -183,10 +183,8 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev, if (!objs[i]) { drm_dbg_kms(dev, "Failed to lookup GEM object\n"); ret = -ENOENT; - goto err_gem_object_handle_put_unlocked; + goto err_gem_object_put; } - drm_gem_object_handle_get_unlocked(objs[i]); - drm_gem_object_put(objs[i]); min_size = (height - 1) * mode_cmd->pitches[i] + drm_format_info_min_pitch(info, i, width) @@ -196,22 +194,22 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev, drm_dbg_kms(dev, "GEM object size (%zu) smaller than minimum size (%u) for plane %d\n", objs[i]->size, min_size, i); - drm_gem_object_handle_put_unlocked(objs[i]); + drm_gem_object_put(objs[i]); ret = -EINVAL; - goto err_gem_object_handle_put_unlocked; + goto err_gem_object_put; } } ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs); if (ret) - goto err_gem_object_handle_put_unlocked; + goto err_gem_object_put; return 0; -err_gem_object_handle_put_unlocked: +err_gem_object_put: while (i > 0) { --i; - drm_gem_object_handle_put_unlocked(objs[i]); + drm_gem_object_put(objs[i]); } return ret; } diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index f921cc73f8b8..e79c3c623c9a 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -161,7 +161,7 @@ void drm_sysfs_lease_event(struct drm_device *dev); /* drm_gem.c */ int drm_gem_init(struct drm_device *dev); -void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj); +bool drm_gem_object_handle_get_if_exists_unlocked(struct drm_gem_object *obj); void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj); int drm_gem_handle_create_tail(struct drm_file *file_priv, struct drm_gem_object *obj, diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h index 668077009fce..38b24fc8978d 100644 --- a/include/drm/drm_framebuffer.h +++ b/include/drm/drm_framebuffer.h @@ -23,6 +23,7 @@ #ifndef __DRM_FRAMEBUFFER_H__ #define __DRM_FRAMEBUFFER_H__ +#include <linux/bits.h> #include <linux/ctype.h> #include <linux/list.h> #include <linux/sched.h> @@ -100,6 +101,8 @@ struct drm_framebuffer_funcs { unsigned num_clips); }; +#define DRM_FRAMEBUFFER_HAS_HANDLE_REF(_i) BIT(0u + (_i)) + /** * struct drm_framebuffer - frame buffer object * @@ -188,6 +191,10 @@ struct drm_framebuffer { * DRM_MODE_FB_MODIFIERS. */ int flags; + /** + * @internal_flags: Framebuffer flags like DRM_FRAMEBUFFER_HAS_HANDLE_REF. + */ + unsigned int internal_flags; /** * @filp_head: Placed on &drm_file.fbs, protected by &drm_file.fbs_lock. */ -- 2.50.0

1 week

[PATCH v2] drm/framebuffer: Acquire internal references on GEM handles

by Thomas Zimmermann

Acquire GEM handles in drm_framebuffer_init() and release them in the corresponding drm_framebuffer_cleanup(). Ties the handle's lifetime to the framebuffer. Not all GEM buffer objects have GEM handles. If not set, no refcounting takes place. This is the case for some fbdev emulation. This is not a problem as these GEM objects do not use dma-bufs and drivers will not release them while fbdev emulation is running. Framebuffer flags keep a bit per color plane of which the framebuffer holds a GEM handle reference. As all drivers use drm_framebuffer_init(), they will now all hold dma-buf references as fixed in commit 5307dce878d4 ("drm/gem: Acquire references on GEM handles for framebuffers"). In the GEM framebuffer helpers, restore the original ref counting on buffer objects. As the helpers for handle refcounting are now no longer called from outside the DRM core, unexport the symbols. v2: - track framebuffer handle refs by flag - drop gma500 cleanup (Christian) Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> Fixes: 5307dce878d4 ("drm/gem: Acquire references on GEM handles for framebuffers") Reported-by: Bert Karwatzki <spasswolf(a)web.de> Closes: https://lore.kernel.org/dri-devel/20250703115915.3096-1-spasswolf@web.de/ Tested-by: Bert Karwatzki <spasswolf(a)web.de> Tested-by: Mario Limonciello <superm1(a)kernel.org> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Anusha Srivatsa <asrivats(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: <stable(a)vger.kernel.org> --- drivers/gpu/drm/drm_framebuffer.c | 31 ++++++++++++++-- drivers/gpu/drm/drm_gem.c | 38 ++++++++++++-------- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 16 ++++----- drivers/gpu/drm/drm_internal.h | 2 +- drivers/gpu/drm/drm_modeset_helper.c | 2 +- include/drm/drm_framebuffer.h | 9 +++++ 6 files changed, 71 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index b781601946db..23b56cde21d7 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -862,11 +862,23 @@ EXPORT_SYMBOL_FOR_TESTS_ONLY(drm_framebuffer_free); int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb, const struct drm_framebuffer_funcs *funcs) { + unsigned int i; int ret; + bool exists; if (WARN_ON_ONCE(fb->dev != dev || !fb->format)) return -EINVAL; + for (i = 0; i < fb->format->num_planes; i++) { + if (drm_WARN_ON_ONCE(dev, fb->flags & DRM_FRAMEBUFFER_HAS_HANDLE_REF(i))) + fb->flags &= ~DRM_FRAMEBUFFER_HAS_HANDLE_REF(i); + if (fb->obj[i]) { + exists = drm_gem_object_handle_get_if_exists_unlocked(fb->obj[i]); + if (exists) + fb->flags |= DRM_FRAMEBUFFER_HAS_HANDLE_REF(i); + } + } + INIT_LIST_HEAD(&fb->filp_head); fb->funcs = funcs; @@ -875,7 +887,7 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb, ret = __drm_mode_object_add(dev, &fb->base, DRM_MODE_OBJECT_FB, false, drm_framebuffer_free); if (ret) - goto out; + goto err; mutex_lock(&dev->mode_config.fb_lock); dev->mode_config.num_fb++; @@ -883,7 +895,16 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb, mutex_unlock(&dev->mode_config.fb_lock); drm_mode_object_register(dev, &fb->base); -out: + + return 0; + +err: + for (i = 0; i < fb->format->num_planes; i++) { + if (fb->flags & DRM_FRAMEBUFFER_HAS_HANDLE_REF(i)) { + drm_gem_object_handle_put_unlocked(fb->obj[i]); + fb->flags &= ~DRM_FRAMEBUFFER_HAS_HANDLE_REF(i); + } + } return ret; } EXPORT_SYMBOL(drm_framebuffer_init); @@ -960,6 +981,12 @@ EXPORT_SYMBOL(drm_framebuffer_unregister_private); void drm_framebuffer_cleanup(struct drm_framebuffer *fb) { struct drm_device *dev = fb->dev; + unsigned int i; + + for (i = 0; i < fb->format->num_planes; i++) { + if (fb->flags & DRM_FRAMEBUFFER_HAS_HANDLE_REF(i)) + drm_gem_object_handle_put_unlocked(fb->obj[i]); + } mutex_lock(&dev->mode_config.fb_lock); list_del(&fb->head); diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index bc505d938b3e..41cdab6088ae 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -224,23 +224,34 @@ static void drm_gem_object_handle_get(struct drm_gem_object *obj) } /** - * drm_gem_object_handle_get_unlocked - acquire reference on user-space handles + * drm_gem_object_handle_get_if_exists_unlocked - acquire reference on user-space handle, if any * @obj: GEM object * - * Acquires a reference on the GEM buffer object's handle. Required - * to keep the GEM object alive. Call drm_gem_object_handle_put_unlocked() - * to release the reference. + * Acquires a reference on the GEM buffer object's handle. Required to keep + * the GEM object alive. Call drm_gem_object_handle_put_if_exists_unlocked() + * to release the reference. Does nothing if the buffer object has no handle. + * + * Returns: + * True if a handle exists, or false otherwise */ -void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj) +bool drm_gem_object_handle_get_if_exists_unlocked(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; guard(mutex)(&dev->object_name_lock); - drm_WARN_ON(dev, !obj->handle_count); /* first ref taken in create-tail helper */ + /* + * First ref taken during GEM object creation, if any. Some + * drivers set up internal framebuffers with GEM objects that + * do not have a GEM handle. Hence, this counter can be zero. + */ + if (!obj->handle_count) + return false; + drm_gem_object_handle_get(obj); + + return true; } -EXPORT_SYMBOL(drm_gem_object_handle_get_unlocked); /** * drm_gem_object_handle_free - release resources bound to userspace handles @@ -273,7 +284,7 @@ static void drm_gem_object_exported_dma_buf_free(struct drm_gem_object *obj) } /** - * drm_gem_object_handle_put_unlocked - releases reference on user-space handles + * drm_gem_object_handle_put_unlocked - releases reference on user-space handle * @obj: GEM object * * Releases a reference on the GEM buffer object's handle. Possibly releases @@ -284,14 +295,14 @@ void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj) struct drm_device *dev = obj->dev; bool final = false; - if (WARN_ON(READ_ONCE(obj->handle_count) == 0)) + if (drm_WARN_ON(dev, READ_ONCE(obj->handle_count) == 0)) return; /* - * Must bump handle count first as this may be the last - * ref, in which case the object would disappear before we - * checked for a name - */ + * Must bump handle count first as this may be the last + * ref, in which case the object would disappear before + * we checked for a name. + */ mutex_lock(&dev->object_name_lock); if (--obj->handle_count == 0) { @@ -304,7 +315,6 @@ void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj) if (final) drm_gem_object_put(obj); } -EXPORT_SYMBOL(drm_gem_object_handle_put_unlocked); /* * Called at device or object close to release the file's diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index c60d0044d036..618ce725cd75 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -100,7 +100,7 @@ void drm_gem_fb_destroy(struct drm_framebuffer *fb) unsigned int i; for (i = 0; i < fb->format->num_planes; i++) - drm_gem_object_handle_put_unlocked(fb->obj[i]); + drm_gem_object_put(fb->obj[i]); drm_framebuffer_cleanup(fb); kfree(fb); @@ -183,10 +183,8 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev, if (!objs[i]) { drm_dbg_kms(dev, "Failed to lookup GEM object\n"); ret = -ENOENT; - goto err_gem_object_handle_put_unlocked; + goto err_gem_object_put; } - drm_gem_object_handle_get_unlocked(objs[i]); - drm_gem_object_put(objs[i]); min_size = (height - 1) * mode_cmd->pitches[i] + drm_format_info_min_pitch(info, i, width) @@ -196,22 +194,22 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev, drm_dbg_kms(dev, "GEM object size (%zu) smaller than minimum size (%u) for plane %d\n", objs[i]->size, min_size, i); - drm_gem_object_handle_put_unlocked(objs[i]); + drm_gem_object_put(objs[i]); ret = -EINVAL; - goto err_gem_object_handle_put_unlocked; + goto err_gem_object_put; } } ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs); if (ret) - goto err_gem_object_handle_put_unlocked; + goto err_gem_object_put; return 0; -err_gem_object_handle_put_unlocked: +err_gem_object_put: while (i > 0) { --i; - drm_gem_object_handle_put_unlocked(objs[i]); + drm_gem_object_put(objs[i]); } return ret; } diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index f921cc73f8b8..e79c3c623c9a 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -161,7 +161,7 @@ void drm_sysfs_lease_event(struct drm_device *dev); /* drm_gem.c */ int drm_gem_init(struct drm_device *dev); -void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj); +bool drm_gem_object_handle_get_if_exists_unlocked(struct drm_gem_object *obj); void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj); int drm_gem_handle_create_tail(struct drm_file *file_priv, struct drm_gem_object *obj, diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c index ef32f6af10d4..1e8822c4b370 100644 --- a/drivers/gpu/drm/drm_modeset_helper.c +++ b/drivers/gpu/drm/drm_modeset_helper.c @@ -94,7 +94,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_device *dev, fb->offsets[i] = mode_cmd->offsets[i]; } fb->modifier = mode_cmd->modifier[0]; - fb->flags = mode_cmd->flags; + fb->flags = mode_cmd->flags & DRM_FRAMEBUFFER_FLAGS_UAPI_MASK; } EXPORT_SYMBOL(drm_helper_mode_fill_fb_struct); diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h index 668077009fce..11fa20d21c58 100644 --- a/include/drm/drm_framebuffer.h +++ b/include/drm/drm_framebuffer.h @@ -23,6 +23,7 @@ #ifndef __DRM_FRAMEBUFFER_H__ #define __DRM_FRAMEBUFFER_H__ +#include <linux/bits.h> #include <linux/ctype.h> #include <linux/list.h> #include <linux/sched.h> @@ -100,6 +101,14 @@ struct drm_framebuffer_funcs { unsigned num_clips); }; +#define __DRM_FRAMEBUFFER_FLAGS_BIT_OFFSET 16 + +#define DRM_FRAMEBUFFER_FLAGS_UAPI_MASK \ + GENMASK(__DRM_FRAMEBUFFER_FLAGS_BIT_OFFSET - 1, 0) + +#define DRM_FRAMEBUFFER_HAS_HANDLE_REF(_i) \ + BIT((__DRM_FRAMEBUFFER_FLAGS_BIT_OFFSET + (_i))) + /** * struct drm_framebuffer - frame buffer object * -- 2.50.0

1 week

Re: [PATCH v2 08/13] media: platform: mediatek: add isp_7x camsys unit

by Krzysztof Kozlowski

On 07/07/2025 03:31, shangyao lin wrote: > +static int mtk_cam_create_links(struct mtk_cam_device *cam) > +{ > + struct v4l2_subdev *sd; > + int ret = 0; > + > + v4l2_device_for_each_subdev(sd, &cam->v4l2_dev) { > + if (sd->entity.function == MEDIA_ENT_F_VID_IF_BRIDGE) > + ret = config_bridge_pad_links(cam, sd); > + } > + > + return ret; > +} > + > +static int mtk_cam_master_register(struct device *dev) > +{ > + struct mtk_cam_device *cam_dev = dev_get_drvdata(dev); > + struct media_device *media_dev = &cam_dev->media_dev; > + int ret; > + > + dev_info(dev, "camsys | start %s\n", __func__); NAK, nothing improved, you already got comments on that. Provide detailed changelog next time. And respond to EACH comment acknowledging that you understood it. This code still has so many trivial issues and terrible coding style, including wrong indentation, wrong alignment/wrapping. > + > + media_dev->dev = cam_dev->dev; > + strscpy(media_dev->model, dev_driver_string(dev), > + sizeof(media_dev->model)); > + snprintf(media_dev->bus_info, sizeof(media_dev->bus_info), > + "platform:%s", dev_name(dev)); > + media_dev->hw_revision = 0; > + media_dev->ops = &mtk_cam_dev_ops; > + media_device_init(media_dev); > + > + cam_dev->v4l2_dev.mdev = media_dev; > + ret = v4l2_device_register(cam_dev->dev, &cam_dev->v4l2_dev); > + if (ret) { > + dev_dbg(dev, "Failed to register V4L2 device: %d\n", ret); > + goto fail_media_device_cleanup; > + } > + > + ret = media_device_register(media_dev); > + if (ret) { > + dev_dbg(dev, "Failed to register media device: %d\n", > + ret); > + goto fail_v4l2_device_unreg; > + } > + > + dev_info(dev, "%s success\n", __func__); > + return 0; > + > +fail_v4l2_device_unreg: > + v4l2_device_unregister(&cam_dev->v4l2_dev); > + > +fail_media_device_cleanup: > + media_device_cleanup(&cam_dev->media_dev); > + > + return ret; > +} > + > +static void mtk_cam_master_unregister(struct device *dev) > +{ > + struct mtk_cam_device *cam_dev = dev_get_drvdata(dev); > + > + dev_info(dev, "camsys | start %s\n", __func__); NAK > + > + media_device_unregister(&cam_dev->media_dev); > + v4l2_device_unregister(&cam_dev->v4l2_dev); > + media_device_cleanup(&cam_dev->media_dev); > +} > + > +static int mtk_cam_async_add_by_driver(struct device *dev, > + struct platform_driver *drv, > + struct v4l2_async_notifier *notifier) > +{ > + struct fwnode_handle *fwnode; > + struct device *p; > + struct v4l2_async_connection *asc; > + int dev_num = 0; > + > + p = platform_find_device_by_driver(NULL, &drv->driver); > + while (p) { > + dev_info(dev, "camsys | %s add %s\n", __func__, p->kobj.name); > + > + fwnode = dev_fwnode(p); > + asc = v4l2_async_nf_add_fwnode(notifier, fwnode, > + struct v4l2_async_connection); > + put_device(p); > + ... > + > + cam_dev->base = devm_ioremap_resource(dev, res); > + if (IS_ERR(cam_dev->base)) { > + dev_err(dev, "failed to map register base\n"); > + return PTR_ERR(cam_dev->base); > + } > + > + cam_dev->dev = dev; > + dev_set_drvdata(dev, cam_dev); > + > + cam_dev->composer_cnt = 0; > + cam_dev->num_seninf_devices = 0; > + > + cam_dev->max_stream_num = MTKCAM_SUBDEV_MAX; > + cam_dev->ctxs = devm_kcalloc(dev, cam_dev->max_stream_num, > + sizeof(*cam_dev->ctxs), GFP_KERNEL); > + if (!cam_dev->ctxs) > + return -ENOMEM; > + > + cam_dev->streaming_ctx = 0; > + for (i = 0; i < cam_dev->max_stream_num; i++) > + mtk_cam_ctx_init(cam_dev->ctxs + i, cam_dev, i); > + > + cam_dev->running_job_count = 0; > + spin_lock_init(&cam_dev->pending_job_lock); > + spin_lock_init(&cam_dev->running_job_lock); > + INIT_LIST_HEAD(&cam_dev->pending_job_list); > + INIT_LIST_HEAD(&cam_dev->running_job_list); > + > + cam_dev->dma_processing_count = 0; > + spin_lock_init(&cam_dev->dma_pending_lock); > + spin_lock_init(&cam_dev->dma_processing_lock); > + INIT_LIST_HEAD(&cam_dev->dma_pending); > + INIT_LIST_HEAD(&cam_dev->dma_processing); > + > + mutex_init(&cam_dev->queue_lock); > + > + pm_runtime_enable(dev); > + > + ret = mtk_cam_of_rproc(cam_dev, pdev); > + if (ret) > + goto fail_destroy_mutex; > + > + ret = register_sub_drivers(dev); > + if (ret) { > + dev_err(dev, "fail to register_sub_drivers\n"); > + goto fail_destroy_mutex; > + } > + > + cam_dev->clks_pdev = platform_device_register_data( > + dev, "clk-mt8188-cam", PLATFORM_DEVID_AUTO, NULL, 0); > + if (IS_ERR(cam_dev->clks_pdev)) { > + ret = PTR_ERR(cam_dev->clks_pdev); > + dev_err(dev, "Failed to register cam clk device: %d\n", ret); > + goto fail_unregister_sub_drivers; > + } > + > + /* register mtk_cam as all isp subdev async parent */ > + cam_dev->notifier.ops = &mtk_cam_async_nf_ops; > + v4l2_async_nf_init(&cam_dev->notifier, &cam_dev->v4l2_dev); > + ret = mtk_cam_async_subdev_add(dev); /* wait all isp sub drivers */ > + if (ret) { > + dev_err(dev, "%s failed mtk_cam_async_subdev_add\n", __func__); > + goto fail_unregister_clks; > + } > + > + ret = v4l2_async_nf_register(&cam_dev->notifier); > + if (ret) { > + dev_err(dev, "%s async_nf_register ret:%d\n", __func__, ret); > + v4l2_async_nf_cleanup(&cam_dev->notifier); > + goto fail_unregister_clks; > + } > + > + dev_info(dev, "camsys | [%s] success\n", __func__); You already got comments. Nothing improved here. > + > + return 0; > + > +fail_unregister_clks: > + if (cam_dev->clks_pdev) > + platform_device_unregister(cam_dev->clks_pdev); > + Best regards, Krzysztof

1 week

Re: [PATCH v2 00/13] Add MediaTek ISP7.x camera system support

by Krzysztof Kozlowski

On 07/07/2025 03:31, shangyao lin wrote: > Based on linux-next tag: next-20250630 > > This patch set adds the MediaTek ISP7.x camera system hardware driver. > > The driver sets up ISP hardware, handles interrupts, and initializes > V4L2 device nodes and functions. It also implements a V4L2 standard video > driver utilizing the media framework APIs, connects sensors and the ISP > via the seninf interface, and communicates with the SCP co-processor to > compose ISP registers in firmware. > Now I found v1. How did you address comment about compliance report? Best regards, Krzysztof

1 week

Re: [PATCH v2 05/13] media: platform: mediatek: add isp_7x seninf unit

by Krzysztof Kozlowski

On Mon, Jul 07, 2025 at 09:31:46AM +0800, shangyao lin wrote: > From: "shangyao.lin" <shangyao.lin(a)mediatek.com> My most frequent comment last year to Mediatek. I even asked to come with some internal procedure so you will not keep repating the same mistake in author's name. Any success? > > Introduce support for the MediaTek sensor interface (seninf) in the SoC camera ... > > --- > > Note: > The PHY operations have been refactored and separated from the seninf driver, > but there are still some issues to confirm with reviewers in this v2 patch > (dt-bindings: media: mediatek: add seninf-core binding). The PHY part will be > moved to drivers/phy/mediatek/ in v3. > > Signed-off-by: shangyao.lin <shangyao.lin(a)mediatek.com> and here the same. Best regards, Krzysztof

1 week

Re: [PATCH v2 03/13] dt-bindings: media: mediatek: add cam-raw binding

by Krzysztof Kozlowski

On Mon, Jul 07, 2025 at 09:31:44AM +0800, shangyao lin wrote: > From: "shangyao.lin" <shangyao.lin(a)mediatek.com> > > Add camera isp7x module device document. > > --- > > Changes in v2: > - Rename binding file to mediatek,mt8188-cam-raw.yaml > - Various fixes per review comments Which fixes? This is way too vague, considering how buggy and poor this code is. Where was v1 of this? Best regards, Krzysztof

1 week

Re: [PATCH v2 04/13] dt-bindings: media: mediatek: add cam-yuv binding

by Krzysztof Kozlowski

On Mon, Jul 07, 2025 at 09:31:45AM +0800, shangyao lin wrote: > From: "shangyao.lin" <shangyao.lin(a)mediatek.com> > > Add camera isp7x module device document. > > --- > > Changes in v2: > - Rename binding file to mediatek,mt8188-cam-yuv.yaml > - Various fixes per review comments > - Update maintainers list Where did you post v1? Please use standard email subjects, so with the PATCH keyword in the title. 'git format-patch -vX' helps here to create proper versioned patches. Another useful tool is b4. Skipping the PATCH keyword makes filtering of emails more difficult thus making the review process less convenient. A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/b… > +properties: > + compatible: > + const: mediatek,mt8188-cam-yuv > + > + reg: > + minItems: 1 What, why? Look at other bindings. > + maxItems: 2 > + description: > + Base address and optional inner base address of the cam-yuv hardware block. Why are you stating obvious? From where did you take it? > + > + reg-names: > + items: > + - const: base > + - const: inner_base > + minItems: 1 > + maxItems: 2 No, really no. You did not follow any existing patterns and this binding does not look at all as anything else. Why making this things up? Just use recently reviewed binding as starting point. Best regards, Krzysztof

1 week

Re: [PATCH v2 02/13] dt-bindings: media: mediatek: add seninf-core binding

by Krzysztof Kozlowski

On Mon, Jul 07, 2025 at 09:31:43AM +0800, shangyao lin wrote: > From: "shangyao.lin" <shangyao.lin(a)mediatek.com> > > 1. Add camera isp7x module device document > Still no SoB, still not tested, still no checkpatch... and also: ... > + > +... > \ No newline at end of file Look, you have patch warnings. Review your patches before you post them. You should reach internally to mediatek colleagues to explain you how this process works. I really do not understand why mediatek has so poor quality of work and cannot improve over last 1-2 years! > -- > 2.18.0 >

1 week

Re: [PATCH v2 01/13] dt-bindings: media: mediatek: add camisp binding

by Krzysztof Kozlowski

On Mon, Jul 07, 2025 at 09:31:42AM +0800, shangyao lin wrote: > From: "shangyao.lin" <shangyao.lin(a)mediatek.com> > > Add camera isp7x module device document. Missing SoB, missing checkpatch. > > --- > > Changes in v2: > - Rename binding file to mediatek,mt8188-camisp.yaml > - Split bindings into four separate patches (one per YAML file) > - Various fixes per review comments > - Update maintainers list > > Signed-off-by: shangyao.lin <shangyao.lin(a)mediatek.com> Apply this patch and see by yourself. This wasn't tested either, so I will skip review. Best regards, Krzysztof

1 week

Re: [PATCH v2 04/13] dt-bindings: media: mediatek: add cam-yuv binding

by Rob Herring (Arm)

On Mon, 07 Jul 2025 09:31:45 +0800, shangyao lin wrote: > From: "shangyao.lin" <shangyao.lin(a)mediatek.com> > > Add camera isp7x module device document. > > --- > > Changes in v2: > - Rename binding file to mediatek,mt8188-cam-yuv.yaml > - Various fixes per review comments > - Update maintainers list > > Signed-off-by: shangyao.lin <shangyao.lin(a)mediatek.com> > --- > .../mediatek/mediatek,mt8188-cam-yuv.yaml | 134 ++++++++++++++++++ > 1 file changed, 134 insertions(+) > create mode 100755 Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.yaml: properties:reg-names: {'items': [{'const': 'base'}, {'const': 'inner_base'}], 'minItems': 1, 'maxItems': 2, 'description': 'Names for each register region. Must be "base" and optionally "inner_base".'} should not be valid under {'required': ['maxItems']} hint: "maxItems" is not needed with an "items" list from schema $id: http://devicetree.org/meta-schemas/items.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.yaml: properties:clock-names: {'items': [{'const': 'camsys_cam2mm0_cgpdn'}, {'const': 'camsys_cam2mm1_cgpdn'}, {'const': 'camsys_cam2sys_cgpdn'}, {'const': 'camsys_yuva_larbx'}, {'const': 'camsys_yuva_cam_cgpdn'}, {'const': 'camsys_yuva_camtg_cgpdn'}, {'const': 'camsys_yuvb_larbx_cgpdn'}, {'const': 'camsys_yuvb_cam_cgpdn'}, {'const': 'camsys_yuvb_camtg_cgpdn'}], 'minItems': 4, 'maxItems': 16, 'description': 'Names of the clocks, must match the order of the clocks property.'} should not be valid under {'required': ['maxItems']} hint: "maxItems" is not needed with an "items" list from schema $id: http://devicetree.org/meta-schemas/items.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.yaml: $id: Cannot determine base path from $id, relative path/filename doesn't match actual path or filename $id: http://devicetree.org/schemas/media/mediatek/mediatek,cam-yuv.yaml file: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.yaml Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.example.dts:33.13-43: Warning (reg_format): /example-0/soc/yuv@16050000:reg: property has invalid length (16 bytes) (#address-cells == 2, #size-cells == 1) Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.example.dts:37.13-59: Warning (dma_ranges_format): /example-0/soc/yuv@16050000:dma-ranges: "dma-ranges" property has invalid length (24 bytes) (parent #address-cells == 2, child #address-cells == 2, #size-cells == 1) Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.example.dts:31.24-58.13: Warning (avoid_default_addr_size): /example-0/soc/yuv@16050000: Relying on default #address-cells value Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.example.dts:31.24-58.13: Warning (avoid_default_addr_size): /example-0/soc/yuv@16050000: Relying on default #size-cells value Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size' /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.example.dtb: yuv@16050000 (mediatek,mt8188-cam-yuv): clock-names:3: 'camsys_yuva_larbx' was expected from schema $id: http://devicetree.org/schemas/media/mediatek/mediatek,cam-yuv.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/mediatek/mediatek,mt8188-cam-yuv.example.dtb: yuv@16050000 (mediatek,mt8188-cam-yuv): dma-ranges: [[2], [0], [0], [1073741824], [1], [0]] is too long from schema $id: http://devicetree.org/schemas/media/mediatek/mediatek,cam-yuv.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/202507070131… The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.

1 week

← Newer
1
2
3
4
5
6
7
...
296
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig