[Freedreno] [PATCH 02/13] drm: drm_printer: Add printer for devcoredump

[Daniel Vetter]

On Thu, Jul 12, 2018 at 12:59:19PM -0600, Jordan Crouse wrote:
> Add a drm printer suitable for use with the read callback for
> devcoredump or other suitable buffer based output format that
> isn't otherwise covered by seq_file.
>
> Signed-off-by: Jordan Crouse <jcrouse at codeaurora.org>

Hm, why not add seq_file support to dev_coredump? Neither git blame nor
google sched any light on why seq_file wasn't picked over the custom read
interface ...

Adding Johannes and Greg about this.

If we go with this, one comment below.

> ---
>  drivers/gpu/drm/drm_print.c | 74 +++++++++++++++++++++++++++++++++++++
>  include/drm/drm_print.h     | 27 ++++++++++++++
>  2 files changed, 101 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index b25f98f33f6c..03d1f98e5ac7 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -30,6 +30,80 @@
>  #include <drm/drmP.h>
>  #include <drm/drm_print.h>
>
> +void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf)
> +{
> + struct drm_print_iterator *iterator = p->arg;
> + ssize_t len;
> +
> + if (!iterator->remain)
> + return;
> +
> + /* Figure out how big the string will be */
> + len = snprintf(NULL, 0, "%pV", vaf);
> +
> + if (iterator->offset < iterator->start) {
> + char *buf;
> + ssize_t copy;
> +
> + if (iterator->offset + len <= iterator->start) {
> + iterator->offset += len;
> + return;
> + }
> +
> + /* Print the string into a temporary buffer */
> + buf = kmalloc(len + 1,
> + GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
> + if (!buf)
> + return;
> +
> + snprintf(buf, len + 1, "%pV", vaf);
> +
> + copy = len - (iterator->start - iterator->offset);
> +
> + if (copy > iterator->remain)
> + copy = iterator->remain;
> +
> + /* Copy out the bit of the string that we need */
> + memcpy(iterator->data,
> + buf + (iterator->start - iterator->offset), copy);
> +
> + iterator->offset = iterator->start + copy;
> + iterator->remain -= copy;
> +
> + kfree(buf);
> + } else {
> + char *buf;
> + ssize_t pos = iterator->offset - iterator->start;
> +
> + if (len < iterator->remain) {
> + snprintf(((char *) iterator->data) + pos,
> + iterator->remain, "%pV", vaf);
> +
> + iterator->offset += len;
> + iterator->remain -= len;
> +
> + return;
> + }
> +
> + /* Print the string into a temporary buffer */
> + buf = kmalloc(len + 1,
> + GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
> + if (!buf)
> + return;
> +
> + snprintf(buf, len + 1, "%pV", vaf);
> +
> + /* Copy out the remaining bits */
> + memcpy(iterator->data + pos, buf, iterator->remain);
> +
> + iterator->offset += iterator->remain;
> + iterator->remain = 0;
> +
> + kfree(buf);
> + }
> +}
> +EXPORT_SYMBOL(__drm_printfn_coredump);
> +
>  void __drm_printfn_seq_file(struct drm_printer *p, struct va_format *vaf)
>  {
>   seq_printf(p->arg, "%pV", vaf);
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index e1a46e9991cc..0ea440fb5ec3 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -73,6 +73,7 @@ struct drm_printer {
>   const char *prefix;
>  };
>
> +void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf);
>  void __drm_printfn_seq_file(struct drm_printer *p, struct va_format *vaf);
>  void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf);
>  void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
> @@ -104,6 +105,32 @@ drm_vprintf(struct drm_printer *p, const char *fmt, va_list *va)
>  #define drm_printf_indent(printer, indent, fmt, ...) \
>   drm_printf((printer), "%.*s" fmt, (indent), "\t\t\t\t\tX", ##__VA_ARGS__)
>
> +struct drm_print_iterator {
> + void *data;
> +
> + ssize_t start;
> + ssize_t offset;
> + ssize_t remain;
> +};
> +
> +/**
> + * drm_coredump_printer - construct a &drm_printer that can output to a buffer
> + * from the read function for devcoredump
> + * @iter: A pointer to a struct drm_print_iterator for the read instance

Bit more flesh for the kerneldoc would be good here, maybe even with a
small in-line example. Definitely a link to dev_coredumpm() which I assume
is the function you're going to use this with.

Pls also make sure it all looks nice using make htmldocs.
-Daniel


> + *
> + * RETURNS:
> + * The &drm_printer object
> + */
> +static inline struct drm_printer
> +drm_coredump_printer(struct drm_print_iterator *iter)
> +{
> + struct drm_printer p = {
> + .printfn = __drm_printfn_coredump,
> + .arg = iter,
> + };
> + return p;
> +}
> +
>  /**
>   * drm_seq_file_printer - construct a &drm_printer that outputs to &seq_file
>   * @f:  the &struct seq_file to output to
> --
> 2.17.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch