[Intel-gfx] [PATCH] drm/i915: Implement stolen reserved detection for ctg/elk

[Paulo Zanoni]

2015-09-14 16:34 GMT-03:00  <ville.syrjala at linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Finally managed to dig up enough hints as to where the stolen
> reserved stuff lives on ctg/elk. So add the code to decode it.
> This was a combination of old chipset specs, diggin up an old
> elk grits release with an ctg/elk AubLoad etc.
>
> This was only tested on an elk as I don't have a ctg here
> unfortunately.
>
> This leaves ilk as the only platform that doesn't have a way
> to detect this stuff. Looking at the register contents on my
> ilk, it might be that the elk way works there too, but I
> can't be sure since I can't affect the amount of reserved
> memory on that machine, and if I am to trust the register
> contents, by default it would reserve 0 bytes.
>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 31 ++++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/i915_reg.h        |  5 +++++
>  2 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index bf26ecc..2e721ff 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -186,6 +186,29 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
>         drm_mm_takedown(&dev_priv->mm.stolen);
>  }
>
> +static void g4x_get_stolen_reserved(struct drm_i915_private *dev_priv,
> +                                   unsigned long *base, unsigned long *size)
> +{
> +       uint32_t reg_val = I915_READ(IS_GM45(dev_priv) ?
> +                                    CTG_STOLEN_RESERVED :
> +                                    ELK_STOLEN_RESERVED);
> +       unsigned long stolen_top = dev_priv->mm.stolen_base +
> +               dev_priv->gtt.stolen_size;
> +
> +       *base = (reg_val & G4X_STOLEN_RESERVED_ADDR2_MASK) << 16;
> +
> +       WARN_ON_ONCE((reg_val & G4X_STOLEN_RESERVED_ADDR1_MASK) < *base);

Since we're only going to run this code at driver init, not using the
_ONCE version of the macro will save us a static variable.

Part of the code seems to match one of the docs you pointed, and I
couldn't find the docs for the other part. Also, I'm always way too
confused when I have to deal with the various gen4 sub-gens, so I'll
never be confident enough to give a full R-B, so:
Acked-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

More importantly, I see the patches to allow user space to allocate
stolen are back on the mailing list. Once that lands, we'll be able to
write simple IGT tests that allocate all of stolen and check if
everything is usable (check if we read what we just wrote). Or we can
also think about other stuff such as allocating an extra page for
every stolen memory allocation and write/check magic words on it in
order to try to catch corruptions. Just ideas...

> +
> +       /* On these platforms, the register doesn't have a size field, so the
> +        * size is the distance between the base and the top of the stolen
> +        * memory. We also have the genuine case where base is zero and there's
> +        * nothing reserved. */
> +       if (*base == 0)
> +               *size = 0;
> +       else
> +               *size = stolen_top - *base;
> +}
> +
>  static void gen6_get_stolen_reserved(struct drm_i915_private *dev_priv,
>                                      unsigned long *base, unsigned long *size)
>  {
> @@ -305,10 +328,12 @@ int i915_gem_init_stolen(struct drm_device *dev)
>         switch (INTEL_INFO(dev_priv)->gen) {
>         case 2:
>         case 3:
> +               break;
>         case 4:
> -               if (!IS_G4X(dev))
> -                       break;
> -               /* fall through */
> +               if (IS_G4X(dev))
> +                       g4x_get_stolen_reserved(dev_priv, &reserved_base,
> +                                               &reserved_size);
> +               break;
>         case 5:
>                 /* Assume the gen6 maximum for the older platforms. */
>                 reserved_size = 1024 * 1024;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 812b7b2..4f570b2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2494,6 +2494,11 @@ enum skl_disp_power_wells {
>
>  #define MCHBAR_MIRROR_BASE_SNB 0x140000
>
> +#define CTG_STOLEN_RESERVED            (MCHBAR_MIRROR_BASE + 0x34)
> +#define ELK_STOLEN_RESERVED            (MCHBAR_MIRROR_BASE + 0x48)
> +#define G4X_STOLEN_RESERVED_ADDR1_MASK (0xFFFF << 16)
> +#define G4X_STOLEN_RESERVED_ADDR2_MASK (0xFFF << 4)
> +
>  /* Memory controller frequency in MCHBAR for Haswell (possible SNB+) */
>  #define DCLK (MCHBAR_MIRROR_BASE_SNB + 0x5e04)
>
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni