[Intel-gfx] [PATCH 33/33] drm/i915: Compress GPU objects in error state

[Joonas Lahtinen]

On ke, 2016-08-10 at 11:52 +0100, Chris Wilson wrote:
> On Wed, Aug 10, 2016 at 01:32:29PM +0300, Joonas Lahtinen wrote:
> > 
> > On su, 2016-08-07 at 15:45 +0100, Chris Wilson wrote:
> > > 
> > > @@ -309,12 +310,30 @@ void i915_error_printf(struct drm_i915_error_state_buf *e, const char *f, ...)
> > >  	va_end(args);
> > >  }
> > >  
> > > +static bool
> > > +ascii85_encode(u32 in, char *out)
> > 
> > base64 is more de facto and I bet userland "expects" it too.
> No. It expects a standard zlib compressed ascii85 stream.

Right, seems there is some standard being followed. Other comments
about making the function more general purpose apply though.

> > > -static int compress_page(void *src, struct drm_i915_error_object *dst)
> > > +static int compress_page(struct z_stream_s *zstream,
> > > +			 void *src,
> > > +			 struct drm_i915_error_object *dst)
> > >  {
> > > -	unsigned long page;
> > > +	zstream->next_in = src;
> > > +	zstream->avail_in = PAGE_SIZE;
> > >  
> > > -	page = __get_free_page(GFP_ATOMIC | __GFP_NOWARN);
> > > -	if (!page)
> > > -		return -ENOMEM;
> > > +	do {
> > > +		if (zstream->avail_out == 0) {
> > > +			unsigned long page;
> > > +
> > > +			page = __get_free_page(GFP_ATOMIC | __GFP_NOWARN);
> > > +			if (!page)
> > > +				return -ENOMEM;
> > > +
> > > +			dst->pages[dst->page_count++] = (void *)page;
> > Why is not dst->pages of different type?
> You want dst->pages[] as an array of unsigned long?

Wouldn't that be more convenient?

> > > @@ -629,6 +680,18 @@ i915_error_object_create(struct drm_i915_private *i915,
> > >  
> > >  	dst->gtt_offset = vma->node.start;
> > >  	dst->page_count = 0;
> > > +	dst->unused = 0;
> > > +
> > > +	memset(&zstream, 0, sizeof(zstream));
> > > +	zstream.workspace = kmalloc(zlib_deflate_workspacesize(MAX_WBITS,
> > > +							       MAX_MEM_LEVEL),
> > > +				    GFP_ATOMIC | __GFP_NOWARN);
> > Wouldn't look better with an intermediate variable?

Right. It's not exactly wrong, so go ahead...

This could be split in its own series, too... These mega series are
horrible to try to get reviewed.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation