On Mon, 22 Oct 2012 18:34:10 -0700
Ben Widawsky <ben at bwidawsk.net> wrote:
> In order to handle differences in pte encoding between architectures it
> is desirable to have one helper function, pte_encode, do it all for us.
> As such, this commit moves the code around so we're in good shape to do
> that.
>
> Luckily the ppgtt pte and the ggtt pte look very similar.
>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 56 ++++++++++++++++++-------------------
> 1 file changed, 27 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index da9c1fa..89c273f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -33,11 +33,32 @@ typedef uint32_t gtt_pte_t;
>
> static inline gtt_pte_t pte_encode(struct drm_device *dev,
> dma_addr_t addr,
> - gtt_pte_t cache_bits)
> + enum i915_cache_level level)
> {
> gtt_pte_t pte = GEN6_PTE_VALID;
> pte |= GEN6_PTE_ADDR_ENCODE(addr);
> - pte |= cache_bits;
> +
> + switch (level) {
> + case I915_CACHE_LLC_MLC:
> + /* Haswell doesn't set L3 this way */
> + if (IS_HASWELL(dev))
> + pte |= GEN6_PTE_CACHE_LLC;
> + else
> + pte |= GEN6_PTE_CACHE_LLC_MLC;
> + break;
> + case I915_CACHE_LLC:
> + pte |= GEN6_PTE_CACHE_LLC;
> + break;
> + case I915_CACHE_NONE:
> + if (IS_HASWELL(dev))
> + pte |= HSW_PTE_UNCACHED;
> + else
> + pte |= GEN6_PTE_UNCACHED;
> + break;
> + default:
> + BUG();
> + }
> +
>
> return pte;
> }
> @@ -54,7 +75,7 @@ static void i915_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
> unsigned last_pte, i;
>
> scratch_pte = pte_encode(ppgtt->dev, ppgtt->scratch_page_dma_addr,
> - GEN6_PTE_CACHE_LLC);
> + I915_CACHE_LLC);
>
> while (num_entries) {
> last_pte = first_pte + num_entries;
> @@ -183,7 +204,7 @@ void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev)
> static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
> const struct sg_table *pages,
> unsigned first_entry,
> - gtt_pte_t pte_flags)
> + enum i915_cache_level cache_level)
> {
> gtt_pte_t *pt_vaddr;
> unsigned act_pd = first_entry / I915_PPGTT_PT_ENTRIES;
> @@ -204,7 +225,7 @@ static void i915_ppgtt_insert_sg_entries(struct i915_hw_ppgtt *ppgtt,
> for (j = first_pte; j < I915_PPGTT_PT_ENTRIES; j++) {
> page_addr = sg_dma_address(sg) + (m << PAGE_SHIFT);
> pt_vaddr[j] = pte_encode(ppgtt->dev, page_addr,
> - pte_flags);
> + cache_level);
>
> /* grab the next page */
> if (++m == segment_len) {
> @@ -228,33 +249,10 @@ void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
> struct drm_i915_gem_object *obj,
> enum i915_cache_level cache_level)
> {
> - gtt_pte_t pte_flags = GEN6_PTE_VALID;
> -
> - switch (cache_level) {
> - case I915_CACHE_LLC_MLC:
> - /* Haswell doesn't set L3 this way */
> - if (IS_HASWELL(ppgtt->dev))
> - pte_flags |= GEN6_PTE_CACHE_LLC;
> - else
> - pte_flags |= GEN6_PTE_CACHE_LLC_MLC;
> - break;
> - case I915_CACHE_LLC:
> - pte_flags |= GEN6_PTE_CACHE_LLC;
> - break;
> - case I915_CACHE_NONE:
> - if (IS_HASWELL(ppgtt->dev))
> - pte_flags |= HSW_PTE_UNCACHED;
> - else
> - pte_flags |= GEN6_PTE_UNCACHED;
> - break;
> - default:
> - BUG();
> - }
> -
> i915_ppgtt_insert_sg_entries(ppgtt,
> obj->pages,
> obj->gtt_space->start >> PAGE_SHIFT,
> - pte_flags);
> + cache_level);
> }
>
> void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
