[PATCH 1/2] drm/i915: stolen_reserved_base is also dma_addr_t

[PATCH 1/2] drm/i915: stolen_reserved_base is also dma_addr_t

[Paulo Zanoni]

The chunk added by this patch was missed by these commits:
* 920bcd1820a6 ("drm/i915: make i915_stolen_to_physical() return phys_addr_t")
* c88473878d47 ("drm/i915: Treat stolen memory as DMA addresses")

The stolen_reserved_base variable contains a memory address for stolen
memory and our code uses dma_addr_t for pointers to stolen, so it
makes sense to make it also be dma_addr_t. Notice that the local
variable inside i915_gem_init_stolen() that stores this pointer is
even already dma_addr_t.

This change essentially converts the size to 64 bits where applicable,
making sure things work in case we ever get crazy enough to put stolen
that far in memory.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index f62fb90..dd2ef5b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -365,7 +365,7 @@ struct i915_ggtt {
 	 */
 	u32 stolen_size;		/* Total size of stolen memory */
 	u32 stolen_usable_size;	/* Total size minus reserved ranges */
-	u32 stolen_reserved_base;
+	dma_addr_t stolen_reserved_base;
 	u32 stolen_reserved_size;
 
 	/** "Graphics Stolen Memory" holds the global PTEs */
-- 
2.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: use size_t instead of u32 for stolen memory size variables

[Paulo Zanoni]

Stolen memory pointers are dma_addr_t, which means they can be 64 bit
things. By using u32 we leave room for bugs in case we ever get huge
amounts of stolen memory. By using size_t we don't risk running into
those problems.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/char/agp/intel-gtt.c           | 10 +++++-----
 drivers/gpu/drm/i915/i915_gem_gtt.c    |  2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.h    |  6 +++---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 19 +++++++++----------
 include/drm/intel-gtt.h                |  2 +-
 5 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 9b6b602..a1db230 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -80,7 +80,7 @@ static struct _intel_private {
 	unsigned int needs_dmar : 1;
 	phys_addr_t gma_bus_addr;
 	/*  Size of memory reserved for graphics by the BIOS */
-	unsigned int stolen_size;
+	size_t stolen_size;
 	/* Total number of gtt entries. */
 	unsigned int gtt_total_entries;
 	/* Part of the gtt that is mappable by the cpu, for those chips where
@@ -333,13 +333,13 @@ static void i810_write_entry(dma_addr_t addr, unsigned int entry,
 	writel_relaxed(addr | pte_flags, intel_private.gtt + entry);
 }
 
-static unsigned int intel_gtt_stolen_size(void)
+static size_t intel_gtt_stolen_size(void)
 {
 	u16 gmch_ctrl;
 	u8 rdct;
 	int local = 0;
 	static const int ddt[4] = { 0, 16, 32, 64 };
-	unsigned int stolen_size = 0;
+	size_t stolen_size = 0;
 
 	if (INTEL_GTT_GEN == 1)
 		return 0; /* no stolen mem on i81x */
@@ -417,7 +417,7 @@ static unsigned int intel_gtt_stolen_size(void)
 	}
 
 	if (stolen_size > 0) {
-		dev_info(&intel_private.bridge_dev->dev, "detected %dK %s memory\n",
+		dev_info(&intel_private.bridge_dev->dev, "detected %zuK %s memory\n",
 		       stolen_size / KB(1), local ? "local" : "stolen");
 	} else {
 		dev_info(&intel_private.bridge_dev->dev,
@@ -1422,7 +1422,7 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
 EXPORT_SYMBOL(intel_gmch_probe);
 
 void intel_gtt_get(u64 *gtt_total,
-		   u32 *stolen_size,
+		   size_t *stolen_size,
 		   phys_addr_t *mappable_base,
 		   u64 *mappable_end)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 64d7852..733fbff 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3316,7 +3316,7 @@ int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv)
 	DRM_INFO("Memory usable by graphics device = %lluM\n",
 		 ggtt->base.total >> 20);
 	DRM_DEBUG_DRIVER("GMADR size = %lldM\n", ggtt->mappable_end >> 20);
-	DRM_DEBUG_DRIVER("GTT stolen size = %uM\n", ggtt->stolen_size >> 20);
+	DRM_DEBUG_DRIVER("GTT stolen size = %zuM\n", ggtt->stolen_size >> 20);
 	if (intel_vtd_active())
 		DRM_INFO("VT-d active for gfx access\n");
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index dd2ef5b..d0356be7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -363,10 +363,10 @@ struct i915_ggtt {
 	 * avoid the first page! The upper end of stolen memory is reserved for
 	 * hardware functions and similarly removed from the accessible range.
 	 */
-	u32 stolen_size;		/* Total size of stolen memory */
-	u32 stolen_usable_size;	/* Total size minus reserved ranges */
+	size_t stolen_size;		/* Total size of stolen memory */
+	size_t stolen_usable_size;	/* Total size minus reserved ranges */
 	dma_addr_t stolen_reserved_base;
-	u32 stolen_reserved_size;
+	size_t stolen_reserved_size;
 
 	/** "Graphics Stolen Memory" holds the global PTEs */
 	void __iomem *gsm;
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 507c9f0..7c9913a 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -286,7 +286,7 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
 }
 
 static void g4x_get_stolen_reserved(struct drm_i915_private *dev_priv,
-				    dma_addr_t *base, u32 *size)
+				    dma_addr_t *base, size_t *size)
 {
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	uint32_t reg_val = I915_READ(IS_GM45(dev_priv) ?
@@ -309,7 +309,7 @@ static void g4x_get_stolen_reserved(struct drm_i915_private *dev_priv,
 }
 
 static void gen6_get_stolen_reserved(struct drm_i915_private *dev_priv,
-				     dma_addr_t *base, u32 *size)
+				     dma_addr_t *base, size_t *size)
 {
 	uint32_t reg_val = I915_READ(GEN6_STOLEN_RESERVED);
 
@@ -335,7 +335,7 @@ static void gen6_get_stolen_reserved(struct drm_i915_private *dev_priv,
 }
 
 static void gen7_get_stolen_reserved(struct drm_i915_private *dev_priv,
-				     dma_addr_t *base, u32 *size)
+				     dma_addr_t *base, size_t *size)
 {
 	uint32_t reg_val = I915_READ(GEN6_STOLEN_RESERVED);
 
@@ -355,7 +355,7 @@ static void gen7_get_stolen_reserved(struct drm_i915_private *dev_priv,
 }
 
 static void chv_get_stolen_reserved(struct drm_i915_private *dev_priv,
-				    dma_addr_t *base, u32 *size)
+				    dma_addr_t *base, size_t *size)
 {
 	uint32_t reg_val = I915_READ(GEN6_STOLEN_RESERVED);
 
@@ -381,7 +381,7 @@ static void chv_get_stolen_reserved(struct drm_i915_private *dev_priv,
 }
 
 static void bdw_get_stolen_reserved(struct drm_i915_private *dev_priv,
-				    dma_addr_t *base, u32 *size)
+				    dma_addr_t *base, size_t *size)
 {
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	uint32_t reg_val = I915_READ(GEN6_STOLEN_RESERVED);
@@ -405,8 +405,8 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
 {
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	dma_addr_t reserved_base, stolen_top;
-	u32 reserved_total, reserved_size;
-	u32 stolen_usable_start;
+	size_t reserved_total, reserved_size;
+	size_t stolen_usable_start;
 
 	mutex_init(&dev_priv->mm.stolen_lock);
 
@@ -486,7 +486,7 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
 	 * memory, so just consider the start. */
 	reserved_total = stolen_top - reserved_base;
 
-	DRM_DEBUG_KMS("Memory reserved for graphics device: %uK, usable: %uK\n",
+	DRM_DEBUG_KMS("Memory reserved for graphics device: %zuK, usable: %zuK\n",
 		      ggtt->stolen_size >> 10,
 		      (ggtt->stolen_size - reserved_total) >> 10);
 
@@ -506,8 +506,7 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
 }
 
 static struct sg_table *
-i915_pages_create_for_stolen(struct drm_device *dev,
-			     u32 offset, u32 size)
+i915_pages_create_for_stolen(struct drm_device *dev, size_t offset, size_t size)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct sg_table *st;
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
index b3bf717..1f11151 100644
--- a/include/drm/intel-gtt.h
+++ b/include/drm/intel-gtt.h
@@ -4,7 +4,7 @@
 #define	_DRM_INTEL_GTT_H
 
 void intel_gtt_get(u64 *gtt_total,
-		   u32 *stolen_size,
+		   size_t *stolen_size,
 		   phys_addr_t *mappable_base,
 		   u64 *mappable_end);
 
-- 
2.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: stolen_reserved_base is also dma_addr_t

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: stolen_reserved_base is also dma_addr_t
URL   : https://patchwork.freedesktop.org/series/30923/
State : success

== Summary ==

Series 30923v1 series starting with [1/2] drm/i915: stolen_reserved_base is also dma_addr_t
https://patchwork.freedesktop.org/api/1.0/series/30923/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514
Test pm_rpm:
        Subgroup basic-rte:
                dmesg-warn -> PASS       (fi-cfl-s) fdo#102294
Test drv_module_reload:
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (fi-glk-1) fdo#102777

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294
fdo#102777 https://bugs.freedesktop.org/show_bug.cgi?id=102777

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:441s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:470s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:416s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:513s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:278s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:496s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:494s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:494s
fi-cfl-s         total:289  pass:223  dwarn:34  dfail:0   fail:0   skip:32  time:539s
fi-cnl-y         total:289  pass:256  dwarn:0   dfail:0   fail:6   skip:27  time:652s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:413s
fi-glk-1         total:289  pass:259  dwarn:1   dfail:0   fail:0   skip:29  time:562s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:424s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:404s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:433s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:490s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:461s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:459s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:575s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:591s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:546s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:458s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:754s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:491s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:473s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:567s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:413s

c4c623d58e38d49e692dc9141250b35e39170e6b drm-tip: 2017y-09m-26d-16h-37m-12s UTC integration manifest
104afd136698 drm/i915: use size_t instead of u32 for stolen memory size variables
4d7c08aaaaff drm/i915: stolen_reserved_base is also dma_addr_t

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5824/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: stolen_reserved_base is also dma_addr_t

[Chris Wilson]

Quoting Paulo Zanoni (2017-09-26 20:29:07)
> The chunk added by this patch was missed by these commits:
> * 920bcd1820a6 ("drm/i915: make i915_stolen_to_physical() return phys_addr_t")
> * c88473878d47 ("drm/i915: Treat stolen memory as DMA addresses")
> 
> The stolen_reserved_base variable contains a memory address for stolen
> memory and our code uses dma_addr_t for pointers to stolen, so it
> makes sense to make it also be dma_addr_t. Notice that the local
> variable inside i915_gem_init_stolen() that stores this pointer is
> even already dma_addr_t.
> 
> This change essentially converts the size to 64 bits where applicable,
> making sure things work in case we ever get crazy enough to put stolen
> that far in memory.

It's defined as residing in the low 4G. Do you have a platform in mind?
-Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: use size_t instead of u32 for stolen memory size variables

[Chris Wilson]

Quoting Paulo Zanoni (2017-09-26 20:29:08)
> Stolen memory pointers are dma_addr_t, which means they can be 64 bit
> things. By using u32 we leave room for bugs in case we ever get huge
> amounts of stolen memory. By using size_t we don't risk running into
> those problems.

What platform?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: use size_t instead of u32 for stolen memory size variables

[Chris Wilson]

Quoting Paulo Zanoni (2017-09-26 20:29:08)
> Stolen memory pointers are dma_addr_t, which means they can be 64 bit
> things. By using u32 we leave room for bugs in case we ever get huge
> amounts of stolen memory. By using size_t we don't risk running into
> those problems.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/char/agp/intel-gtt.c           | 10 +++++-----
>  drivers/gpu/drm/i915/i915_gem_gtt.c    |  2 +-
>  drivers/gpu/drm/i915/i915_gem_gtt.h    |  6 +++---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 19 +++++++++----------
>  include/drm/intel-gtt.h                |  2 +-
>  5 files changed, 19 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> index 9b6b602..a1db230 100644
> --- a/drivers/char/agp/intel-gtt.c
> +++ b/drivers/char/agp/intel-gtt.c
> @@ -80,7 +80,7 @@ static struct _intel_private {
>         unsigned int needs_dmar : 1;
>         phys_addr_t gma_bus_addr;
>         /*  Size of memory reserved for graphics by the BIOS */
> -       unsigned int stolen_size;
> +       size_t stolen_size;

What is size_t? How does that correspond to a physical or dma addr?
You either meant kernel_size_t or unsigned long, or a proper type for
the address space.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: stolen_reserved_base is also dma_addr_t

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: stolen_reserved_base is also dma_addr_t
URL   : https://patchwork.freedesktop.org/series/30923/
State : failure

== Summary ==

Test perf:
        Subgroup polling:
                pass       -> FAIL       (shard-hsw) fdo#102252 +1
Test kms_flip:
        Subgroup flip-vs-expired-vblank:
                fail       -> PASS       (shard-hsw) fdo#102367
        Subgroup plain-flip-ts-check-interruptible:
                fail       -> PASS       (shard-hsw)
Test gem_exec_capture:
        Subgroup capture-render:
                pass       -> INCOMPLETE (shard-hsw)

fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#102367 https://bugs.freedesktop.org/show_bug.cgi?id=102367

shard-hsw        total:2375 pass:1297 dwarn:1   dfail:0   fail:10  skip:1066 time:9618s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5824/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: use size_t instead of u32 for stolen memory size variables

[Joonas Lahtinen]

On Tue, 2017-09-26 at 21:13 +0100, Chris Wilson wrote:
> Quoting Paulo Zanoni (2017-09-26 20:29:08)
> > Stolen memory pointers are dma_addr_t, which means they can be 64 bit
> > things. By using u32 we leave room for bugs in case we ever get huge
> > amounts of stolen memory. By using size_t we don't risk running into
> > those problems.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/char/agp/intel-gtt.c           | 10 +++++-----
> >  drivers/gpu/drm/i915/i915_gem_gtt.c    |  2 +-
> >  drivers/gpu/drm/i915/i915_gem_gtt.h    |  6 +++---
> >  drivers/gpu/drm/i915/i915_gem_stolen.c | 19 +++++++++----------
> >  include/drm/intel-gtt.h                |  2 +-
> >  5 files changed, 19 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> > index 9b6b602..a1db230 100644
> > --- a/drivers/char/agp/intel-gtt.c
> > +++ b/drivers/char/agp/intel-gtt.c
> > @@ -80,7 +80,7 @@ static struct _intel_private {
> >         unsigned int needs_dmar : 1;
> >         phys_addr_t gma_bus_addr;
> >         /*  Size of memory reserved for graphics by the BIOS */
> > -       unsigned int stolen_size;
> > +       size_t stolen_size;
> 
> What is size_t? How does that correspond to a physical or dma addr?
> You either meant kernel_size_t or unsigned long, or a proper type for
> the address space.

We're using phys_addr_t + size_t in early-quirks.c too, so we want to
keep both places consistent. If we're using something else than size_t,
then we should update both places (it's still on my todo to get rid of
the code duplication).

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: use size_t instead of u32 for stolen memory size variables

[Chris Wilson]

Quoting Joonas Lahtinen (2017-09-29 10:23:10)
> On Tue, 2017-09-26 at 21:13 +0100, Chris Wilson wrote:
> > Quoting Paulo Zanoni (2017-09-26 20:29:08)
> > > Stolen memory pointers are dma_addr_t, which means they can be 64 bit
> > > things. By using u32 we leave room for bugs in case we ever get huge
> > > amounts of stolen memory. By using size_t we don't risk running into
> > > those problems.
> > > 
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  drivers/char/agp/intel-gtt.c           | 10 +++++-----
> > >  drivers/gpu/drm/i915/i915_gem_gtt.c    |  2 +-
> > >  drivers/gpu/drm/i915/i915_gem_gtt.h    |  6 +++---
> > >  drivers/gpu/drm/i915/i915_gem_stolen.c | 19 +++++++++----------
> > >  include/drm/intel-gtt.h                |  2 +-
> > >  5 files changed, 19 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> > > index 9b6b602..a1db230 100644
> > > --- a/drivers/char/agp/intel-gtt.c
> > > +++ b/drivers/char/agp/intel-gtt.c
> > > @@ -80,7 +80,7 @@ static struct _intel_private {
> > >         unsigned int needs_dmar : 1;
> > >         phys_addr_t gma_bus_addr;
> > >         /*  Size of memory reserved for graphics by the BIOS */
> > > -       unsigned int stolen_size;
> > > +       size_t stolen_size;
> > 
> > What is size_t? How does that correspond to a physical or dma addr?
> > You either meant kernel_size_t or unsigned long, or a proper type for
> > the address space.
> 
> We're using phys_addr_t + size_t in early-quirks.c too, so we want to
> keep both places consistent. If we're using something else than size_t,
> then we should update both places (it's still on my todo to get rid of
> the code duplication).
> 

Re duplication: move the discovery into early-quirks and export the
resource_t ?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: use size_t instead of u32 for stolen memory size variables

[Joonas Lahtinen]

On Fri, 2017-09-29 at 10:40 +0100, Chris Wilson wrote:
> Quoting Joonas Lahtinen (2017-09-29 10:23:10)
> > On Tue, 2017-09-26 at 21:13 +0100, Chris Wilson wrote:
> > > Quoting Paulo Zanoni (2017-09-26 20:29:08)
> > > > Stolen memory pointers are dma_addr_t, which means they can be 64 bit
> > > > things. By using u32 we leave room for bugs in case we ever get huge
> > > > amounts of stolen memory. By using size_t we don't risk running into
> > > > those problems.
> > > > 
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > > ---
> > > >  drivers/char/agp/intel-gtt.c           | 10 +++++-----
> > > >  drivers/gpu/drm/i915/i915_gem_gtt.c    |  2 +-
> > > >  drivers/gpu/drm/i915/i915_gem_gtt.h    |  6 +++---
> > > >  drivers/gpu/drm/i915/i915_gem_stolen.c | 19 +++++++++----------
> > > >  include/drm/intel-gtt.h                |  2 +-
> > > >  5 files changed, 19 insertions(+), 20 deletions(-)
> > > > 
> > > > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> > > > index 9b6b602..a1db230 100644
> > > > --- a/drivers/char/agp/intel-gtt.c
> > > > +++ b/drivers/char/agp/intel-gtt.c
> > > > @@ -80,7 +80,7 @@ static struct _intel_private {
> > > >         unsigned int needs_dmar : 1;
> > > >         phys_addr_t gma_bus_addr;
> > > >         /*  Size of memory reserved for graphics by the BIOS */
> > > > -       unsigned int stolen_size;
> > > > +       size_t stolen_size;
> > > 
> > > What is size_t? How does that correspond to a physical or dma addr?
> > > You either meant kernel_size_t or unsigned long, or a proper type for
> > > the address space.
> > 
> > We're using phys_addr_t + size_t in early-quirks.c too, so we want to
> > keep both places consistent. If we're using something else than size_t,
> > then we should update both places (it's still on my todo to get rid of
> > the code duplication).
> > 
> 
> Re duplication: move the discovery into early-quirks and export the
> resource_t ?

Might be an idea, the biggest hurdle is that now early quirks are
__init so if you don't load i915, they have no impact on resident code
size, they get overridden. So pretty much any way will increase either
resident code or data memory size, so I've so far been looking to just
share the codebase some way. Because the code applies to _all_ x86.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx