Re: [PATCH v4 06/11] drm/cma-helper: Add drm_gem_cma_print_info()

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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@ffwll.ch>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel