[Intel-gfx] [PATCH] include: Move ascii85 functions from i915 to linux/ascii85.h

[Chris Wilson]

Quoting Jordan Crouse (2018-01-26 20:59:22)
> The i915 DRM driver very cleverly used ascii85 encoding for their
> GPU state file. Move the encode functions to a general header file to
> support other drivers that might be interested in the same
> functionality.
> 
> Signed-off-by: Jordan Crouse <jcrouse at codeaurora.org>
> ---
>  drivers/gpu/drm/i915/i915_gpu_error.c | 24 +---------------
>  include/linux/ascii85.h               | 52 +++++++++++++++++++++++++++++++++++
>  2 files changed, 53 insertions(+), 23 deletions(-)
>  create mode 100644 include/linux/ascii85.h
> 
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 48418fb..2588f37 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -31,6 +31,7 @@
>  #include <linux/stop_machine.h>
>  #include <linux/zlib.h>
>  #include <drm/drm_print.h>
> +#include <linux/ascii85.h>
>  
>  #include "i915_drv.h"
>  
> @@ -501,29 +502,6 @@ void i915_error_printf(struct drm_i915_error_state_buf *e, const char *f, ...)
>         va_end(args);
>  }
>  
> -static int
> -ascii85_encode_len(int len)
> -{
> -       return DIV_ROUND_UP(len, 4);
> -}
> -
> -static bool
> -ascii85_encode(u32 in, char *out)
> -{
> -       int i;
> -
> -       if (in == 0)
> -               return false;
> -
> -       out[5] = '\0';
> -       for (i = 5; i--; ) {
> -               out[i] = '!' + in % 85;
> -               in /= 85;
> -       }
> -
> -       return true;
> -}
> -
>  static void print_error_obj(struct drm_i915_error_state_buf *m,
>                             struct intel_engine_cs *engine,
>                             const char *name,
> diff --git a/include/linux/ascii85.h b/include/linux/ascii85.h
> new file mode 100644
> index 0000000..7ee39f9
> --- /dev/null
> +++ b/include/linux/ascii85.h
> @@ -0,0 +1,52 @@
> +/*
> + * Copyright (c) 2008 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#ifndef _ASCII85_H_
> +#define _ASCII85_H_
> +
> +#include <linux/kernel.h>
> +
> +static inline int
> +ascii85_encode_len(int len)
> +{
> +       return DIV_ROUND_UP(len, 4);
> +}

Use longs for generic stuff.

> +
> +static inline bool
> +ascii85_encode(u32 in, char *out)
> +{
> +       int i;
> +
> +       if (in == 0)
> +               return false;
> +
> +       out[5] = '\0';
> +       for (i = 5; i--; ) {
> +               out[i] = '!' + in % 85;
> +               in /= 85;
> +       }
> +
> +       return true;
> +}

I think you'll want to capture the special case 0 == 'z' in the common
routines.

{
	char buf[ASCII85_BUFSZ];
	int i, len;

	len = ascii85_encode_len(PAGE_SIZE);
	for (i = 0; i < len; i++)
		err_puts(m, ascii85_encode(obj->pages[page][i], buf));
}	

Looks reasonable for the caller, so

#define ASCII85_BUFSZ 6

static inline const char *
ascii85_encode(u32 in, char *out)
{
	int i;

	/* check whether out[0] = 'z'; out[1] = '\0'; generates better code */
	if (in == 0)
		return "z";

	out[5] = '\0';
	for (i = 5; i--; ) {
		out[i] = '!' + in % 85;
		in /= 85;
	}

	return out;
}

-Chris