[Intel-gfx] [PATCH 2/4] drm/i915: Support for creating Stolen memory backed objects

[Chris Wilson]

On Thu, Jul 02, 2015 at 10:37:56AM +0100, Tvrtko Ursulin wrote:
> 
> On 07/01/2015 05:19 PM, Chris Wilson wrote:
> >On Wed, Jul 01, 2015 at 04:06:49PM +0100, Tvrtko Ursulin wrote:
> >>>diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> >>>index c5349fa..6045749 100644
> >>>--- a/drivers/gpu/drm/i915/i915_dma.c
> >>>+++ b/drivers/gpu/drm/i915/i915_dma.c
> >>>@@ -167,6 +167,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
> >>>  		value = i915.enable_hangcheck &&
> >>>  			intel_has_gpu_reset(dev);
> >>>  		break;
> >>>+	case I915_PARAM_CREATE_VERSION:
> >>>+		value = 1;
> >>
> >>Shouldn't it be 2?
> >
> >But 1 is the 2nd number, discounting all those pesky negative versions :)
> 
> It would be more obvious I think, even though
> I915_PARAM_CREATE_VERSION which returns 1 would never exist.

I don't disagree. I was just used to always starting my versions at 1, 0
being the unversioned.
 
> >>>+		ret = i915_gem_exec_clear_object(obj, file->driver_priv);
> >>
> >>I would put a comment here saying why it is important to clear
> >>stolen memory.
> >
> >Userspace ABI (and kernel ABI in general) is that we do not hand back
> >uncleared buffers. Something to do with bank card details I guess.
> >So just:
> 
> Yes thats obvious - but where it is done for normal objects? Can't
> find it... is it hidden in shmemfs somewhere? If so reinforces the
> need for a comment here.

Yes, it is deep within shmemfs, shmem_getpage_gfp() will call
clear_highpage() on new pages. It is an unfortunate cost that we want to
avoid for frequently allocated internal objects (such as contexts, pdp)
where we just want a simple allocator rather than shmemfs.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre