Message ID | 20190619090420.6667-2-kraxel@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/virtio: switch from ttm to gem shmem helpers. | expand |
Hi Gerd, On 2019/06/19, Gerd Hoffmann wrote: > +/** > + * drm_gem_array_from_handles -- lookup an array of gem handles. > + * > + * @drm_file: drm file-private structure to use for the handle look up > + * @handles: the array of handles to lookup. > + * @nents: the numer of handles. > + * > + * Returns: An array of gem objects on success, NULL on failure. > + */ > +struct drm_gem_object_array* > +drm_gem_array_from_handles(struct drm_file *drm_file, u32 *handles, u32 nents) > +{ > + struct drm_gem_object_array *objs; > + u32 i; > + > + objs = drm_gem_array_alloc(nents); > + if (!objs) > + return NULL; > + > + for (i = 0; i < nents; i++) { > + objs->objs[i] = drm_gem_object_lookup(drm_file, handles[i]); > + if (!objs->objs[i]) { Missing object put for the 0..i-1 handles. Personally I would: objs->nents = i; drm_gem_array_put_free(objs); return NULL; > + drm_gem_array_put_free(objs); > + return NULL; > + } > + } > + return objs; > +} Missing EXPORT_SYMBOL? > + > +/** > + * drm_gem_array_put_free -- put gem objects and free array. > + * > + * @objs: the gem object array. > + */ > +void drm_gem_array_put_free(struct drm_gem_object_array *objs) > +{ > + u32 i; > + > + for (i = 0; i < objs->nents; i++) { > + if (!objs->objs[i]) > + continue; > + drm_gem_object_put_unlocked(objs->objs[i]); > + } > + drm_gem_array_free(objs); > +} Ditto? HTH Emil
On Wed, Jun 19, 2019 at 11:04:09AM +0200, Gerd Hoffmann wrote: > Add struct and helper functions to manage an array of gem objects. > See added kernel docs for details. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Hm, feels like jumping ahead here, I think there's too much still in-flight: - Christian is pondering some improved ww_mutex lock/unlock helpers. That'll probably change a lot of this code too. - If we do more helpers, then I think we should have a consistent story across everything. These here don't really fit into the existing gem lock/unlock helpers. - We probably want to design something coherent to replace all the ttm execbuf utils, i.e. bo lookup, locking, updating reservation objects, all that. - I think this needs more than one driver to proof itself. Maybe long-term we could have a drm_gem_eu_helper.c or so which contains all that. But that's still some ways off. I'd go back to the virtio-only conversion, and once we have more of this stuff settled, we can look at how to properly design some nice&consistent helpers. Cheers, Daniel > --- > include/drm/drm_gem_array_helper.h | 15 +++++ > drivers/gpu/drm/drm_gem_array_helper.c | 76 ++++++++++++++++++++++++++ > drivers/gpu/drm/Makefile | 3 +- > 3 files changed, 93 insertions(+), 1 deletion(-) > create mode 100644 include/drm/drm_gem_array_helper.h > create mode 100644 drivers/gpu/drm/drm_gem_array_helper.c > > diff --git a/include/drm/drm_gem_array_helper.h b/include/drm/drm_gem_array_helper.h > new file mode 100644 > index 000000000000..adf7961247b3 > --- /dev/null > +++ b/include/drm/drm_gem_array_helper.h > @@ -0,0 +1,15 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __DRM_GEM_ARRAY_HELPER_H__ > +#define __DRM_GEM_ARRAY_HELPER_H__ > + > +struct drm_gem_object_array { > + u32 nents; > + struct drm_gem_object *objs[]; > +}; > + > +struct drm_gem_object_array *drm_gem_array_alloc(u32 nents); > +struct drm_gem_object_array * > +drm_gem_array_from_handles(struct drm_file *drm_file, u32 *handles, u32 nents); > +void drm_gem_array_put_free(struct drm_gem_object_array *objs); > + > +#endif /* __DRM_GEM_ARRAY_HELPER_H__ */ > diff --git a/drivers/gpu/drm/drm_gem_array_helper.c b/drivers/gpu/drm/drm_gem_array_helper.c > new file mode 100644 > index 000000000000..d35c77c4a02d > --- /dev/null > +++ b/drivers/gpu/drm/drm_gem_array_helper.c > @@ -0,0 +1,76 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#include <drm/drm_gem.h> > +#include <drm/drm_gem_array_helper.h> > + > +/** > + * drm_gem_array_alloc -- allocate gem object array of the given size. > + * > + * @nents: number of entries needed. > + * > + * Returns: An array of gem objects on success, NULL on failure. > + */ > +struct drm_gem_object_array *drm_gem_array_alloc(u32 nents) > +{ > + struct drm_gem_object_array *objs; > + size_t size = sizeof(*objs) + sizeof(objs->objs[0]) * nents; > + > + objs = kzalloc(size, GFP_KERNEL); > + if (!objs) > + return NULL; > + > + objs->nents = nents; > + return objs; > +} > +EXPORT_SYMBOL(drm_gem_array_alloc); > + > +static void drm_gem_array_free(struct drm_gem_object_array *objs) > +{ > + kfree(objs); > +} > + > +/** > + * drm_gem_array_from_handles -- lookup an array of gem handles. > + * > + * @drm_file: drm file-private structure to use for the handle look up > + * @handles: the array of handles to lookup. > + * @nents: the numer of handles. > + * > + * Returns: An array of gem objects on success, NULL on failure. > + */ > +struct drm_gem_object_array* > +drm_gem_array_from_handles(struct drm_file *drm_file, u32 *handles, u32 nents) > +{ > + struct drm_gem_object_array *objs; > + u32 i; > + > + objs = drm_gem_array_alloc(nents); > + if (!objs) > + return NULL; > + > + for (i = 0; i < nents; i++) { > + objs->objs[i] = drm_gem_object_lookup(drm_file, handles[i]); > + if (!objs->objs[i]) { > + drm_gem_array_put_free(objs); > + return NULL; > + } > + } > + return objs; > +} > + > +/** > + * drm_gem_array_put_free -- put gem objects and free array. > + * > + * @objs: the gem object array. > + */ > +void drm_gem_array_put_free(struct drm_gem_object_array *objs) > +{ > + u32 i; > + > + for (i = 0; i < objs->nents; i++) { > + if (!objs->objs[i]) > + continue; > + drm_gem_object_put_unlocked(objs->objs[i]); > + } > + drm_gem_array_free(objs); > +} > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 9d630a28a788..d32e7de0937b 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -43,7 +43,8 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_dsc.o drm_probe_helper > drm_simple_kms_helper.o drm_modeset_helper.o \ > drm_scdc_helper.o drm_gem_framebuffer_helper.o \ > drm_atomic_state_helper.o drm_damage_helper.o \ > - drm_format_helper.o drm_self_refresh_helper.o > + drm_format_helper.o drm_self_refresh_helper.o \ > + drm_gem_array_helper.o > > drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o > drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o > -- > 2.18.1 >
> > +struct drm_gem_object_array* > > +drm_gem_array_from_handles(struct drm_file *drm_file, u32 *handles, u32 nents) > > +{ > > + struct drm_gem_object_array *objs; > > + u32 i; > > + > > + objs = drm_gem_array_alloc(nents); > > + if (!objs) > > + return NULL; > > + > > + for (i = 0; i < nents; i++) { > > + objs->objs[i] = drm_gem_object_lookup(drm_file, handles[i]); > > + if (!objs->objs[i]) { > Missing object put for the 0..i-1 handles. Personally I would: No. drm_gem_array_alloc initializes objs->nents and drm_gem_array_put_free() loops over the whole array, skipping NULL pointers. > > + drm_gem_array_put_free(objs); > > + return NULL; > > + } > > + } > > + return objs; > > +} > Missing EXPORT_SYMBOL? Oops. I had that fixed. Possibly squashed into the wrong patch. > Ditto? Yes. cheers, Gerd
diff --git a/include/drm/drm_gem_array_helper.h b/include/drm/drm_gem_array_helper.h new file mode 100644 index 000000000000..adf7961247b3 --- /dev/null +++ b/include/drm/drm_gem_array_helper.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __DRM_GEM_ARRAY_HELPER_H__ +#define __DRM_GEM_ARRAY_HELPER_H__ + +struct drm_gem_object_array { + u32 nents; + struct drm_gem_object *objs[]; +}; + +struct drm_gem_object_array *drm_gem_array_alloc(u32 nents); +struct drm_gem_object_array * +drm_gem_array_from_handles(struct drm_file *drm_file, u32 *handles, u32 nents); +void drm_gem_array_put_free(struct drm_gem_object_array *objs); + +#endif /* __DRM_GEM_ARRAY_HELPER_H__ */ diff --git a/drivers/gpu/drm/drm_gem_array_helper.c b/drivers/gpu/drm/drm_gem_array_helper.c new file mode 100644 index 000000000000..d35c77c4a02d --- /dev/null +++ b/drivers/gpu/drm/drm_gem_array_helper.c @@ -0,0 +1,76 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#include <drm/drm_gem.h> +#include <drm/drm_gem_array_helper.h> + +/** + * drm_gem_array_alloc -- allocate gem object array of the given size. + * + * @nents: number of entries needed. + * + * Returns: An array of gem objects on success, NULL on failure. + */ +struct drm_gem_object_array *drm_gem_array_alloc(u32 nents) +{ + struct drm_gem_object_array *objs; + size_t size = sizeof(*objs) + sizeof(objs->objs[0]) * nents; + + objs = kzalloc(size, GFP_KERNEL); + if (!objs) + return NULL; + + objs->nents = nents; + return objs; +} +EXPORT_SYMBOL(drm_gem_array_alloc); + +static void drm_gem_array_free(struct drm_gem_object_array *objs) +{ + kfree(objs); +} + +/** + * drm_gem_array_from_handles -- lookup an array of gem handles. + * + * @drm_file: drm file-private structure to use for the handle look up + * @handles: the array of handles to lookup. + * @nents: the numer of handles. + * + * Returns: An array of gem objects on success, NULL on failure. + */ +struct drm_gem_object_array* +drm_gem_array_from_handles(struct drm_file *drm_file, u32 *handles, u32 nents) +{ + struct drm_gem_object_array *objs; + u32 i; + + objs = drm_gem_array_alloc(nents); + if (!objs) + return NULL; + + for (i = 0; i < nents; i++) { + objs->objs[i] = drm_gem_object_lookup(drm_file, handles[i]); + if (!objs->objs[i]) { + drm_gem_array_put_free(objs); + return NULL; + } + } + return objs; +} + +/** + * drm_gem_array_put_free -- put gem objects and free array. + * + * @objs: the gem object array. + */ +void drm_gem_array_put_free(struct drm_gem_object_array *objs) +{ + u32 i; + + for (i = 0; i < objs->nents; i++) { + if (!objs->objs[i]) + continue; + drm_gem_object_put_unlocked(objs->objs[i]); + } + drm_gem_array_free(objs); +} diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 9d630a28a788..d32e7de0937b 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -43,7 +43,8 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_dsc.o drm_probe_helper drm_simple_kms_helper.o drm_modeset_helper.o \ drm_scdc_helper.o drm_gem_framebuffer_helper.o \ drm_atomic_state_helper.o drm_damage_helper.o \ - drm_format_helper.o drm_self_refresh_helper.o + drm_format_helper.o drm_self_refresh_helper.o \ + drm_gem_array_helper.o drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
Add struct and helper functions to manage an array of gem objects. See added kernel docs for details. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- include/drm/drm_gem_array_helper.h | 15 +++++ drivers/gpu/drm/drm_gem_array_helper.c | 76 ++++++++++++++++++++++++++ drivers/gpu/drm/Makefile | 3 +- 3 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 include/drm/drm_gem_array_helper.h create mode 100644 drivers/gpu/drm/drm_gem_array_helper.c