On Thu, Jul 12, 2018 at 09:46:58PM +0200, Daniel Vetter wrote:
> 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@xxxxxxxxxxxxxx>
>
> 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.
Main reason was that this is used for devcoredump which has its own similar but
not quite seq_file compatible callback. If there is synergy to be had there
that would be great because reinventing the wheel isn't fun.
> 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.
Can do. Thanks.
> -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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
