diff mbox

[01/10] include: Move ascii85 functions from i915 to linux/ascii85.h

Message ID 20180405220056.29423-2-jcrouse@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jordan Crouse April 5, 2018, 10 p.m. UTC 
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.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
 drivers/gpu/drm/i915/i915_gpu_error.c | 35 ++++---------------------------
 include/linux/ascii85.h               | 39 +++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 31 deletions(-)
 create mode 100644 include/linux/ascii85.h

Comments

Jordan Crouse April 5, 2018, 10:06 p.m. UTC | #1 
On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote:
> 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.

In a previous version of this patch, Chris asked what tree I wanted this applied
to, and the answer is: I'm not sure?  I'm hoping that Rob will be cool with
picking the rest up for msm-next for 4.18 but I'm okay with putting this
particular patch wherever it is easiest for the maintainers.

Jordan

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
>  drivers/gpu/drm/i915/i915_gpu_error.c | 35 ++++---------------------------
>  include/linux/ascii85.h               | 39 +++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 31 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 67c902412193..969d967e58c7 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -31,7 +31,7 @@
>  #include <linux/stop_machine.h>
>  #include <linux/zlib.h>
>  #include <drm/drm_print.h>
> -
> +#include <linux/ascii85.h>
>  #include "i915_drv.h"
>  
>  static inline const struct intel_engine_cs *
> @@ -518,35 +518,12 @@ 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,
>  			    struct drm_i915_error_object *obj)
>  {
> -	char out[6];
> +	char out[ASCII85_BUFSZ];
>  	int page;
>  
>  	if (!obj)
> @@ -568,12 +545,8 @@ static void print_error_obj(struct drm_i915_error_state_buf *m,
>  			len -= obj->unused;
>  		len = ascii85_encode_len(len);
>  
> -		for (i = 0; i < len; i++) {
> -			if (ascii85_encode(obj->pages[page][i], out))
> -				err_puts(m, out);
> -			else
> -				err_puts(m, "z");
> -		}
> +		for (i = 0; i < len; i++)
> +			error_puts(m, ascii85_encode(obj->pages[page][i], out));
>  	}
>  	err_puts(m, "\n");
>  }
> diff --git a/include/linux/ascii85.h b/include/linux/ascii85.h
> new file mode 100644
> index 000000000000..322bbed731ae
> --- /dev/null
> +++ b/include/linux/ascii85.h
> @@ -0,0 +1,39 @@
> +
> +/*
> + * SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (c) 2008 Intel Corporation
> + * Copyright (c) The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef _ASCII85_H_
> +#define _ASCII85_H_
> +
> +#include <linux/kernel.h>
> +
> +#define ASCII85_BUFSZ 6
> +
> +static inline long
> +ascii85_encode_len(long len)
> +{
> +	return DIV_ROUND_UP(len, 4);
> +}
> +
> +static inline char *
> +ascii85_encode(u32 in, char *out)
> +{
> +	int i;
> +
> +	if (in == 0)
> +		return "z";
> +
> +	out[5] = '\0';
> +	for (i = 5; i--; ) {
> +		out[i] = '!' + in % 85;
> +		in /= 85;
> +	}
> +
> +	return out;
> +}
> +
> +#endif
> -- 
> 2.16.1
> 
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
Chris Wilson April 6, 2018, 10:39 a.m. UTC | #2 
Quoting Jordan Crouse (2018-04-05 23:06:53)
> On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote:
> > 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.
> 
> In a previous version of this patch, Chris asked what tree I wanted this applied
> to, and the answer is: I'm not sure?  I'm hoping that Rob will be cool with
> picking the rest up for msm-next for 4.18 but I'm okay with putting this
> particular patch wherever it is easiest for the maintainers.

We are a bit late to sneak it into the 4.17 drm base via i915. I don't
anticipate any problems (for i915) with this patch going in through
msm-next, so happy to have it land there and percolate back to i915 3
months later.
-Chris
kernel test robot April 9, 2018, 7:19 a.m. UTC | #3 
Hi Jordan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robclark/msm-next]
[also build test ERROR on v4.16 next-20180406]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jordan-Crouse/drm-msm-GPU-crash-state/20180406-170513
base:   git://people.freedesktop.org/~robclark/linux msm-next
config: x86_64-rhel (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu//drm/i915/i915_gpu_error.c: In function 'print_error_obj':
>> drivers/gpu//drm/i915/i915_gpu_error.c:549:4: error: implicit declaration of function 'error_puts'; did you mean 'err_puts'? [-Werror=implicit-function-declaration]
       error_puts(m, ascii85_encode(obj->pages[page][i], out));
       ^~~~~~~~~~
       err_puts
   cc1: some warnings being treated as errors

vim +549 drivers/gpu//drm/i915/i915_gpu_error.c

   520	
   521	static void print_error_obj(struct drm_i915_error_state_buf *m,
   522				    struct intel_engine_cs *engine,
   523				    const char *name,
   524				    struct drm_i915_error_object *obj)
   525	{
   526		char out[ASCII85_BUFSZ];
   527		int page;
   528	
   529		if (!obj)
   530			return;
   531	
   532		if (name) {
   533			err_printf(m, "%s --- %s = 0x%08x %08x\n",
   534				   engine ? engine->name : "global", name,
   535				   upper_32_bits(obj->gtt_offset),
   536				   lower_32_bits(obj->gtt_offset));
   537		}
   538	
   539		err_compression_marker(m);
   540		for (page = 0; page < obj->page_count; page++) {
   541			int i, len;
   542	
   543			len = PAGE_SIZE;
   544			if (page == obj->page_count - 1)
   545				len -= obj->unused;
   546			len = ascii85_encode_len(len);
   547	
   548			for (i = 0; i < len; i++)
 > 549				error_puts(m, ascii85_encode(obj->pages[page][i], out));
   550		}
   551		err_puts(m, "\n");
   552	}
   553	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Eric Anholt April 16, 2018, 5:52 p.m. UTC | #4 
Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Jordan Crouse (2018-04-05 23:06:53)
>> On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote:
>> > 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.
>> 
>> In a previous version of this patch, Chris asked what tree I wanted this applied
>> to, and the answer is: I'm not sure?  I'm hoping that Rob will be cool with
>> picking the rest up for msm-next for 4.18 but I'm okay with putting this
>> particular patch wherever it is easiest for the maintainers.
>
> We are a bit late to sneak it into the 4.17 drm base via i915. I don't
> anticipate any problems (for i915) with this patch going in through
> msm-next, so happy to have it land there and percolate back to i915 3
> months later.

I'd love to have it in drm-misc-next, so I can build a similar hang dump
interface for vc5.  But most importantly, I'd like to have it somewhere
soon.
Daniel Vetter April 17, 2018, 8:03 a.m. UTC | #5 
On Mon, Apr 16, 2018 at 10:52:59AM -0700, Eric Anholt wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Jordan Crouse (2018-04-05 23:06:53)
> >> On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote:
> >> > 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.
> >> 
> >> In a previous version of this patch, Chris asked what tree I wanted this applied
> >> to, and the answer is: I'm not sure?  I'm hoping that Rob will be cool with
> >> picking the rest up for msm-next for 4.18 but I'm okay with putting this
> >> particular patch wherever it is easiest for the maintainers.
> >
> > We are a bit late to sneak it into the 4.17 drm base via i915. I don't
> > anticipate any problems (for i915) with this patch going in through
> > msm-next, so happy to have it land there and percolate back to i915 3
> > months later.
> 
> I'd love to have it in drm-misc-next, so I can build a similar hang dump
> interface for vc5.  But most importantly, I'd like to have it somewhere
> soon.

Maarten will likely send the first pull for drm-misc-next at the end of
this week. So if you just push it there today, there's no hold-up for msm
(or anyone else) really.
-Daniel
Jordan Crouse April 17, 2018, 2:54 p.m. UTC | #6 
On Mon, Apr 16, 2018 at 10:52:59AM -0700, Eric Anholt wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Jordan Crouse (2018-04-05 23:06:53)
> >> On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote:
> >> > 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.
> >> 
> >> In a previous version of this patch, Chris asked what tree I wanted this applied
> >> to, and the answer is: I'm not sure?  I'm hoping that Rob will be cool with
> >> picking the rest up for msm-next for 4.18 but I'm okay with putting this
> >> particular patch wherever it is easiest for the maintainers.
> >
> > We are a bit late to sneak it into the 4.17 drm base via i915. I don't
> > anticipate any problems (for i915) with this patch going in through
> > msm-next, so happy to have it land there and percolate back to i915 3
> > months later.
> 
> I'd love to have it in drm-misc-next, so I can build a similar hang dump
> interface for vc5.  But most importantly, I'd like to have it somewhere
> soon.

I'll fix the bot error and push it up again today.

Jordan
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 67c902412193..969d967e58c7 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -31,7 +31,7 @@ 
 #include <linux/stop_machine.h>
 #include <linux/zlib.h>
 #include <drm/drm_print.h>
-
+#include <linux/ascii85.h>
 #include "i915_drv.h"
 
 static inline const struct intel_engine_cs *
@@ -518,35 +518,12 @@  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,
 			    struct drm_i915_error_object *obj)
 {
-	char out[6];
+	char out[ASCII85_BUFSZ];
 	int page;
 
 	if (!obj)
@@ -568,12 +545,8 @@  static void print_error_obj(struct drm_i915_error_state_buf *m,
 			len -= obj->unused;
 		len = ascii85_encode_len(len);
 
-		for (i = 0; i < len; i++) {
-			if (ascii85_encode(obj->pages[page][i], out))
-				err_puts(m, out);
-			else
-				err_puts(m, "z");
-		}
+		for (i = 0; i < len; i++)
+			error_puts(m, ascii85_encode(obj->pages[page][i], out));
 	}
 	err_puts(m, "\n");
 }
diff --git a/include/linux/ascii85.h b/include/linux/ascii85.h
new file mode 100644
index 000000000000..322bbed731ae
--- /dev/null
+++ b/include/linux/ascii85.h
@@ -0,0 +1,39 @@ 
+
+/*
+ * SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (c) 2008 Intel Corporation
+ * Copyright (c) The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _ASCII85_H_
+#define _ASCII85_H_
+
+#include <linux/kernel.h>
+
+#define ASCII85_BUFSZ 6
+
+static inline long
+ascii85_encode_len(long len)
+{
+	return DIV_ROUND_UP(len, 4);
+}
+
+static inline char *
+ascii85_encode(u32 in, char *out)
+{
+	int i;
+
+	if (in == 0)
+		return "z";
+
+	out[5] = '\0';
+	for (i = 5; i--; ) {
+		out[i] = '!' + in % 85;
+		in /= 85;
+	}
+
+	return out;
+}
+
+#endif