[Intel-gfx] [PATCH] drm/i915/vlv: Added write-enable pte bit support

[Goel, Akash]

> Don't cause us to rewrite the PTE for the batch buffer between each execbuffer (ro for the batch, rw next time it gets used as a texture).
> In fact, do not change ro without user intervention.
> gt_old_ro is unused
Sorry we didn't consider this scenario, that a batch buffer could be subsequently used as a different type of buffer also, like as a texture buffer. But is the remap going to have a significant overhead.

>> Use byt_pte_encode() instead of hacking the result of ppgtt->pte_encode().
>> Consider expanding i915_cache_level so that it included the concept of PROT_READ | PROT_WRITE.
Thanks, actually we initially thought of doing it in a same way. Can we overload the i915_cache_level enum with the RO flag, while calling 'insert_entries' for VALLEYVIEW platform. 

>> Also, what's the exact use-case for this here? And if we need to expose this to userspace, then it needs a testcase.
>> -Daniel
Since we have this RO bit available for our disposal on BYT platform, we thought we can avail it in order to have an extra protection on the buffers.
We initially used it only for Ring & Batch buffers as we know, without User's input, that they are supposed to be read-only from GPU side.
For extending this for other buffers, we need a Libdrm level change. 
Or can we use the Read/Write domains information in relocation entries to decide this internally on the Driver side, but that will require some change in the exec-buffer path.

Best Regards
Akash

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
Sent: Thursday, February 06, 2014 11:06 PM
To: Chris Wilson; Goel, Akash; intel-gfx at lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: Added write-enable pte bit support

On Thu, Feb 06, 2014 at 11:56:37AM +0000, Chris Wilson wrote:
> On Thu, Feb 06, 2014 at 10:22:28AM +0000, Goel, Akash wrote:
> > Please kindly review this patch.
> > 
> > Best regards
> > Akash
> > -----Original Message-----
> > From: Goel, Akash
> > Sent: Thursday, January 09, 2014 5:55 PM
> > To: intel-gfx at lists.freedesktop.org
> > Cc: Goel, Akash
> > Subject: [PATCH] drm/i915/vlv: Added write-enable pte bit support
> > 
> > From: Akash Goel <akash.goel at intel.com>
> > 
> > This adds support for using the write-enable bit in the GTT entry for VLV.
> > This is handled via a read-only flag in the GEM buffer object which is then used to check if the write-enable bit has to be set or not when writing the GTT entries.
> > Currently by default only the Batch buffer & Ring buffers are being marked as read only.
> 
> Don't cause us to rewrite the PTE for the batch buffer between each 
> execbuffer (ro for the batch, rw next time it gets used as a texture).
> In fact, do not change ro without user intervention.
> 
> gt_old_ro is unused
> 
> Use byt_pte_encode() instead of hacking the result of
> ppgtt->pte_encode().
> 
> Consider expanding i915_cache_level so that it included the concept of 
> PROT_READ | PROT_WRITE.

Also, what's the exact use-case for this here? And if we need to expose this to userspace, then it needs a testcase.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch