[Intel-gfx] [PATCH 1/5] drm/i915/hsw: Set correct Haswell PTE encodings.

[Rodrigo Vivi]

Hi Ben,

sorry for taking so long to look at your patches.
Well, since I changed my TI password I'm not able to see bspec
anymore, so I couldn't verify many things that I'm going to ask many
questions.
If the answer is on spec or any other doc please send me in pvt!


On Thu, Jul 4, 2013 at 3:02 PM, Ben Widawsky <ben at bwidawsk.net> wrote:
> From: Ben Widawsky <benjamin.widawsky at intel.com>
>
> The cacheability controls have changed, and the bits have been
> rearranged in general.
>
> v2: Remove comments for snb/ivb cache leves, that's a separate change.
>
> v3: Resolve conflicts due to patch series reordering.
>
> v4: Rebased on top of Kenneth Graunke's ->pet_encode refactoring.

pet like in leper?! :P
just kidding never mind.

>
> v5: Removed eLLC bits for separate patch.
>
> In the internal repository this was:
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 66929ea..42262d0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -33,6 +33,7 @@
>
>  /* PPGTT stuff */
>  #define GEN6_GTT_ADDR_ENCODE(addr)     ((addr) | (((addr) >> 28) & 0xff0))
> +#define HSW_GTT_ADDR_ENCODE(addr)      ((addr) | (((addr) >> 28) & 0x7f0))

why not masking bit 10 anymore?

>
>  #define GEN6_PDE_VALID                 (1 << 0)
>  /* gen6+ has bit 11-4 for physical addr bit 39-32 */
> @@ -44,6 +45,14 @@
>  #define GEN6_PTE_CACHE_LLC             (2 << 1)
>  #define GEN6_PTE_CACHE_LLC_MLC         (3 << 1)
>  #define GEN6_PTE_ADDR_ENCODE(addr)     GEN6_GTT_ADDR_ENCODE(addr)
> +#define HSW_PTE_ADDR_ENCODE(addr)      HSW_GTT_ADDR_ENCODE(addr)
> +
> +/* Cacheability Control is a 4-bit value. The low three bits are stored in *
> + * bits 3:1 of the PTE, while the fourth bit is stored in bit 11 of the PTE.
> + */
> +#define HSW_CACHEABILITY_CONTROL(bits) ((((bits) & 0x7) << 1) | \
> +                                        (((bits) & 0x8) << (11 - 3)))
> +#define HSW_WB_LLC_AGE0                        HSW_CACHEABILITY_CONTROL(0x3)

where did you get that? and are you really using that in any other patch?

>
>  static gen6_gtt_pte_t gen6_pte_encode(dma_addr_t addr,
>                                       enum i915_cache_level level)
> @@ -92,10 +101,10 @@ static gen6_gtt_pte_t hsw_pte_encode(dma_addr_t addr,
>                                      enum i915_cache_level level)
>  {
>         gen6_gtt_pte_t pte = GEN6_PTE_VALID;
> -       pte |= GEN6_PTE_ADDR_ENCODE(addr);
> +       pte |= HSW_PTE_ADDR_ENCODE(addr);
>
>         if (level != I915_CACHE_NONE)
> -               pte |= GEN6_PTE_CACHE_LLC;
> +               pte |= HSW_WB_LLC_AGE0;
>
>         return pte;
>  }
> --
> 1.8.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br