Hi Noralf,
Than you for the patch.
On Monday, 30 October 2017 18:29:40 EET Noralf Trønnes wrote:
> Add drm_gem_cma_print_info() for debugfs printing
> struct drm_gem_cma_object specific info.
>
> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_gem_cma_helper.c | 19 +++++++++++++++++++
> include/drm/drm_gem_cma_helper.h | 5 ++++-
> 2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> b/drivers/gpu/drm/drm_gem_cma_helper.c index 020e7668dfab..89dc7f04fae6
> 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -423,6 +423,25 @@ void drm_gem_cma_describe(struct drm_gem_cma_object
> *cma_obj, EXPORT_SYMBOL_GPL(drm_gem_cma_describe);
> #endif
>
> +/**
> + * drm_gem_cma_print_info() - Print &drm_gem_cma_object info for debugfs
> + * @p: DRM printer
> + * @indent: Tab indentation level
> + * @gem: GEM object
> + *
> + * This function can be used as the &drm_driver->gem_print_info callback.
> + * It prints paddr and vaddr for use in e.g. debugfs output.
> + */
> +void drm_gem_cma_print_info(struct drm_printer *p, unsigned int indent,
> + const struct drm_gem_object *obj)
> +{
> + struct drm_gem_cma_object *cma_obj = to_drm_gem_cma_obj(obj);
> +
> + drm_printf_indent(p, indent, "paddr=%pad\n", &cma_obj->paddr);
> + drm_printf_indent(p, indent, "vaddr=%p\n", cma_obj->vaddr);
> +}
> +EXPORT_SYMBOL(drm_gem_cma_print_info);
> +
> /**
> * drm_gem_cma_prime_get_sg_table - provide a scatter/gather table of
> pinned * pages for a CMA GEM object
> diff --git a/include/drm/drm_gem_cma_helper.h
> b/include/drm/drm_gem_cma_helper.h index 58a739bf15f1..bc47e6eba271 100644
> --- a/include/drm/drm_gem_cma_helper.h
> +++ b/include/drm/drm_gem_cma_helper.h
> @@ -21,7 +21,7 @@ struct drm_gem_cma_object {
> };
>
> static inline struct drm_gem_cma_object *
> -to_drm_gem_cma_obj(struct drm_gem_object *gem_obj)
> +to_drm_gem_cma_obj(const struct drm_gem_object *gem_obj)
This will happily return a non-const pointer to the drm_gem_cma_object based
on a const pointer to the contained drm_gem_object, thus creating const-safety
problems.
There was an attempt to fix the problem in the container_of() macro itself
(see https://lkml.org/lkml/2017/5/19/381) but the patch seems to have fallen
through the cracks. It would require turning this inline function into a
macro.
I don't think we need to wait for the container_of() patch to land, but it
would be useful to turn the inline function into a macro already to
automatically benefit from the change, instead of introducting a const-safety
problem that we will all forget about until it breaks something in the future.
> {
> return container_of(gem_obj, struct drm_gem_cma_object, base);
> }
> @@ -94,6 +94,9 @@ unsigned long drm_gem_cma_get_unmapped_area(struct file
> *filp, void drm_gem_cma_describe(struct drm_gem_cma_object *obj, struct
> seq_file *m); #endif
>
> +void drm_gem_cma_print_info(struct drm_printer *p, unsigned int indent,
> + const struct drm_gem_object *obj);
> +
> struct sg_table *drm_gem_cma_prime_get_sg_table(struct drm_gem_object
> *obj);
> struct drm_gem_object *
> drm_gem_cma_prime_import_sg_table(struct drm_device *dev,
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
