All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Intel GFX <intel-gfx@lists.freedesktop.org>,
	Ben Widawsky <ben@bwidawsk.net>
Subject: Re: [PATCH 1/2] drm/i915: Make PTE valid encoding optional
Date: Fri, 18 Oct 2013 15:40:19 +0200	[thread overview]
Message-ID: <20131018134019.GX4830@phenom.ffwll.local> (raw)
In-Reply-To: <1381940302-1920-1-git-send-email-benjamin.widawsky@intel.com>

On Wed, Oct 16, 2013 at 09:18:21AM -0700, Ben Widawsky wrote:

Empty commit messages aren't cool, especially for a fix this late in the
-rc cycle. Patches simply need to be justified. I've filled something in
when merging.

> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     |  3 ++-
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 35 ++++++++++++++++++++---------------
>  2 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6106d3d..0cbeb0e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -517,7 +517,8 @@ struct i915_address_space {
>  
>  	/* FIXME: Need a more generic return type */
>  	gen6_gtt_pte_t (*pte_encode)(dma_addr_t addr,
> -				     enum i915_cache_level level);
> +				     enum i915_cache_level level,
> +				     bool valid); /* Create a valid PTE */
>  	void (*clear_range)(struct i915_address_space *vm,
>  			    unsigned int first_entry,
>  			    unsigned int num_entries);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index e999496..81dce29 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -58,9 +58,10 @@
>  #define HSW_WT_ELLC_LLC_AGE0		HSW_CACHEABILITY_CONTROL(0x6)
>  
>  static gen6_gtt_pte_t snb_pte_encode(dma_addr_t addr,
> -				     enum i915_cache_level level)
> +				     enum i915_cache_level level,
> +				     bool valid)
>  {
> -	gen6_gtt_pte_t pte = GEN6_PTE_VALID;
> +	gen6_gtt_pte_t pte = valid ? GEN6_PTE_VALID : 0;
>  	pte |= GEN6_PTE_ADDR_ENCODE(addr);
>  
>  	switch (level) {
> @@ -79,9 +80,10 @@ static gen6_gtt_pte_t snb_pte_encode(dma_addr_t addr,
>  }
>  
>  static gen6_gtt_pte_t ivb_pte_encode(dma_addr_t addr,
> -				     enum i915_cache_level level)
> +				     enum i915_cache_level level,
> +				     bool valid)
>  {
> -	gen6_gtt_pte_t pte = GEN6_PTE_VALID;
> +	gen6_gtt_pte_t pte = valid ? GEN6_PTE_VALID : 0;
>  	pte |= GEN6_PTE_ADDR_ENCODE(addr);
>  
>  	switch (level) {
> @@ -105,9 +107,10 @@ static gen6_gtt_pte_t ivb_pte_encode(dma_addr_t addr,
>  #define BYT_PTE_SNOOPED_BY_CPU_CACHES	(1 << 2)
>  
>  static gen6_gtt_pte_t byt_pte_encode(dma_addr_t addr,
> -				     enum i915_cache_level level)
> +				     enum i915_cache_level level,
> +				     bool valid)
>  {
> -	gen6_gtt_pte_t pte = GEN6_PTE_VALID;
> +	gen6_gtt_pte_t pte = valid ? GEN6_PTE_VALID : 0;
>  	pte |= GEN6_PTE_ADDR_ENCODE(addr);
>  
>  	/* Mark the page as writeable.  Other platforms don't have a
> @@ -122,9 +125,10 @@ static gen6_gtt_pte_t byt_pte_encode(dma_addr_t addr,
>  }
>  
>  static gen6_gtt_pte_t hsw_pte_encode(dma_addr_t addr,
> -				     enum i915_cache_level level)
> +				     enum i915_cache_level level,
> +				     bool valid)
>  {
> -	gen6_gtt_pte_t pte = GEN6_PTE_VALID;
> +	gen6_gtt_pte_t pte = valid ? GEN6_PTE_VALID : 0;
>  	pte |= HSW_PTE_ADDR_ENCODE(addr);
>  
>  	if (level != I915_CACHE_NONE)
> @@ -134,9 +138,10 @@ static gen6_gtt_pte_t hsw_pte_encode(dma_addr_t addr,
>  }
>  
>  static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr,
> -				      enum i915_cache_level level)
> +				      enum i915_cache_level level,
> +				      bool valid)
>  {
> -	gen6_gtt_pte_t pte = GEN6_PTE_VALID;
> +	gen6_gtt_pte_t pte = valid ? GEN6_PTE_VALID : 0;
>  	pte |= HSW_PTE_ADDR_ENCODE(addr);
>  
>  	switch (level) {
> @@ -245,7 +250,7 @@ static void gen6_ppgtt_clear_range(struct i915_address_space *vm,
>  	unsigned first_pte = first_entry % I915_PPGTT_PT_ENTRIES;
>  	unsigned last_pte, i;
>  
> -	scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC);
> +	scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, true);
>  
>  	while (num_entries) {
>  		last_pte = first_pte + num_entries;
> @@ -282,7 +287,7 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
>  		dma_addr_t page_addr;
>  
>  		page_addr = sg_page_iter_dma_address(&sg_iter);
> -		pt_vaddr[act_pte] = vm->pte_encode(page_addr, cache_level);
> +		pt_vaddr[act_pte] = vm->pte_encode(page_addr, cache_level, true);
>  		if (++act_pte == I915_PPGTT_PT_ENTRIES) {
>  			kunmap_atomic(pt_vaddr);
>  			act_pt++;
> @@ -536,7 +541,7 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
>  
>  	for_each_sg_page(st->sgl, &sg_iter, st->nents, 0) {
>  		addr = sg_page_iter_dma_address(&sg_iter);
> -		iowrite32(vm->pte_encode(addr, level), &gtt_entries[i]);
> +		iowrite32(vm->pte_encode(addr, level, true), &gtt_entries[i]);
>  		i++;
>  	}
>  
> @@ -548,7 +553,7 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
>  	 */
>  	if (i != 0)
>  		WARN_ON(readl(&gtt_entries[i-1]) !=
> -			vm->pte_encode(addr, level));
> +			vm->pte_encode(addr, level, true));
>  
>  	/* This next bit makes the above posting read even more important. We
>  	 * want to flush the TLBs only after we're certain all the PTE updates
> @@ -573,7 +578,7 @@ static void gen6_ggtt_clear_range(struct i915_address_space *vm,
>  		 first_entry, num_entries, max_entries))
>  		num_entries = max_entries;
>  
> -	scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC);
> +	scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC, true);
>  	for (i = 0; i < num_entries; i++)
>  		iowrite32(scratch_pte, &gtt_base[i]);
>  	readl(gtt_base);
> -- 
> 1.8.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

      parent reply	other threads:[~2013-10-18 13:40 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-16 16:18 [PATCH 1/2] drm/i915: Make PTE valid encoding optional Ben Widawsky
2013-10-16 16:18 ` [PATCH 2/2] drm/i915: Disable GGTT PTEs on GEN6+ suspend Ben Widawsky
2013-10-16 16:21   ` [PATCH 2/2] [v2] " Ben Widawsky
2013-10-16 16:58     ` Chris Wilson
2013-10-16 17:06       ` Ben Widawsky
2013-10-16 17:27         ` Chris Wilson
2013-10-17  7:41           ` Takashi Iwai
2013-10-17  9:24             ` Chris Wilson
2013-10-17 10:06               ` Takashi Iwai
2013-10-18 13:45         ` Daniel Vetter
2013-10-18  0:20     ` Todd Previte
2013-10-18 13:40 ` Daniel Vetter [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131018134019.GX4830@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=ben@bwidawsk.net \
    --cc=benjamin.widawsky@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.