* [PATCH] drm/i915: limit PPGTT size to 2GB in 32-bit platforms
@ 2015-05-28 15:24 Michel Thierry
2015-05-28 15:39 ` Chris Wilson
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Michel Thierry @ 2015-05-28 15:24 UTC (permalink / raw)
To: intel-gfx; +Cc: daniel.vetter, Mika Kuoppala
We already set this limit for the GGTT.
This is a temporary patch until a full replacement of size_t variables
(inadequate in 32-bit kernel) is in place.
Regression from:
commit a4e0bedca678c81eea4cd79a4bd502335639f73a
Author: Michel Thierry <michel.thierry@intel.com>
Date: Wed Apr 8 12:13:35 2015 +0100
drm/i915: Use complete address space in true PPGTT
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 17b7df0..ffecb87 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -951,7 +951,15 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
gen8_initialize_pd(&ppgtt->base, ppgtt->scratch_pd);
ppgtt->base.start = 0;
+
+#ifdef CONFIG_X86_32
+ /* Limit 32b platforms to GGTT size (2GB) */
+ struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
+
+ ppgtt->base.total = dev_priv->gtt.base.total;
+#else
ppgtt->base.total = 1ULL << 32;
+#endif
ppgtt->base.cleanup = gen8_ppgtt_cleanup;
ppgtt->base.allocate_va_range = gen8_alloc_va_range;
ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
--
2.4.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: limit PPGTT size to 2GB in 32-bit platforms
2015-05-28 15:24 [PATCH] drm/i915: limit PPGTT size to 2GB in 32-bit platforms Michel Thierry
@ 2015-05-28 15:39 ` Chris Wilson
2015-05-28 15:59 ` Michel Thierry
2015-05-28 17:09 ` [PATCH v2] " Michel Thierry
2015-05-29 13:15 ` [PATCH v3] " Michel Thierry
2 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2015-05-28 15:39 UTC (permalink / raw)
To: Michel Thierry; +Cc: daniel.vetter, intel-gfx, Mika Kuoppala
On Thu, May 28, 2015 at 04:24:47PM +0100, Michel Thierry wrote:
> We already set this limit for the GGTT.
>
> This is a temporary patch until a full replacement of size_t variables
> (inadequate in 32-bit kernel) is in place.
>
> Regression from:
> commit a4e0bedca678c81eea4cd79a4bd502335639f73a
> Author: Michel Thierry <michel.thierry@intel.com>
> Date: Wed Apr 8 12:13:35 2015 +0100
>
> drm/i915: Use complete address space in true PPGTT
>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 17b7df0..ffecb87 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -951,7 +951,15 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
> gen8_initialize_pd(&ppgtt->base, ppgtt->scratch_pd);
>
> ppgtt->base.start = 0;
> +
> +#ifdef CONFIG_X86_32
> + /* Limit 32b platforms to GGTT size (2GB) */
Forgot the comment about why the limit is inplace and when it can be
lifted. Remember comments are for *why* not *what*.
> + struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
Fancy new fangled C.
> + ppgtt->base.total = dev_priv->gtt.base.total;
ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
> +#else
> ppgtt->base.total = 1ULL << 32;
> +#endif
Better yet would be:
ppgtt->base.total = 1ULL << 32;
if (IS_ENABLED(CONFIG_X86_32)) {
/* While we have a proliferation of size_t variables
* we cannot represent the full ppgtt size on 32bit,
* so limit it to the same size as the GGTT (currently
* 2GiB).
*/
ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
}
However, I am afraid that it will become obsolete but be forgotten to be
removed.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: limit PPGTT size to 2GB in 32-bit platforms
2015-05-28 15:39 ` Chris Wilson
@ 2015-05-28 15:59 ` Michel Thierry
0 siblings, 0 replies; 10+ messages in thread
From: Michel Thierry @ 2015-05-28 15:59 UTC (permalink / raw)
To: Chris Wilson, intel-gfx, daniel.vetter, Mika Kuoppala
[-- Attachment #1.1: Type: text/plain, Size: 2237 bytes --]
On 5/28/2015 4:39 PM, Chris Wilson wrote:
> On Thu, May 28, 2015 at 04:24:47PM +0100, Michel Thierry wrote:
>> We already set this limit for the GGTT.
>>
>> This is a temporary patch until a full replacement of size_t variables
>> (inadequate in 32-bit kernel) is in place.
>>
>> Regression from:
>> commit a4e0bedca678c81eea4cd79a4bd502335639f73a
>> Author: Michel Thierry <michel.thierry@intel.com>
>> Date: Wed Apr 8 12:13:35 2015 +0100
>>
>> drm/i915: Use complete address space in true PPGTT
>>
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 17b7df0..ffecb87 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -951,7 +951,15 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>> gen8_initialize_pd(&ppgtt->base, ppgtt->scratch_pd);
>>
>> ppgtt->base.start = 0;
>> +
>> +#ifdef CONFIG_X86_32
>> + /* Limit 32b platforms to GGTT size (2GB) */
> Forgot the comment about why the limit is inplace and when it can be
> lifted. Remember comments are for *why* not *what*.
>
>> + struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
> Fancy new fangled C.
>
>> + ppgtt->base.total = dev_priv->gtt.base.total;
> ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
>
>> +#else
>> ppgtt->base.total = 1ULL << 32;
>> +#endif
> Better yet would be:
> ppgtt->base.total = 1ULL << 32;
> if (IS_ENABLED(CONFIG_X86_32)) {
> /* While we have a proliferation of size_t variables
> * we cannot represent the full ppgtt size on 32bit,
> * so limit it to the same size as the GGTT (currently
> * 2GiB).
> */
> ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
> }
>
> However, I am afraid that it will become obsolete but be forgotten to be
> removed.
Yes, this is just for 4.2, Mika started working on a patch to replace
the size_t variables.
I'll update this with your changes.
Thanks,
> -Chris
>
[-- Attachment #1.2: Type: text/html, Size: 23430 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] drm/i915: limit PPGTT size to 2GB in 32-bit platforms
2015-05-28 15:24 [PATCH] drm/i915: limit PPGTT size to 2GB in 32-bit platforms Michel Thierry
2015-05-28 15:39 ` Chris Wilson
@ 2015-05-28 17:09 ` Michel Thierry
2015-05-28 17:47 ` Damien Lespiau
2015-05-28 21:14 ` Chris Wilson
2015-05-29 13:15 ` [PATCH v3] " Michel Thierry
2 siblings, 2 replies; 10+ messages in thread
From: Michel Thierry @ 2015-05-28 17:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Mika Kuoppala
And prevent overflow warning during compilation. We already set this limit
for the GGTT.
This is a temporary patch until a full replacement of size_t variables
(inadequate in 32-bit kernel) is in place.
Regression from:
commit a4e0bedca678c81eea4cd79a4bd502335639f73a
Author: Michel Thierry <michel.thierry@intel.com>
Date: Wed Apr 8 12:13:35 2015 +0100
drm/i915: Use complete address space in true PPGTT
v2: Prettify code and explain why this is needed. (Chris)
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 17b7df0..0653c28 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -951,7 +951,16 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
gen8_initialize_pd(&ppgtt->base, ppgtt->scratch_pd);
ppgtt->base.start = 0;
- ppgtt->base.total = 1ULL << 32;
+#ifdef CONFIG_X86_32
+ /* While we have a proliferation of size_t variables
+ * we cannot represent the full ppgtt size on 32bit,
+ * so limit it to the same size as the GGTT (currently
+ * 2GiB).
+ */
+ ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
+#else
+ ppgtt->base.total = 1ULL << 32;
+#endif
ppgtt->base.cleanup = gen8_ppgtt_cleanup;
ppgtt->base.allocate_va_range = gen8_alloc_va_range;
ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
--
2.4.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915: limit PPGTT size to 2GB in 32-bit platforms
2015-05-28 17:09 ` [PATCH v2] " Michel Thierry
@ 2015-05-28 17:47 ` Damien Lespiau
2015-05-28 21:14 ` Chris Wilson
1 sibling, 0 replies; 10+ messages in thread
From: Damien Lespiau @ 2015-05-28 17:47 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx, Mika Kuoppala
On Thu, May 28, 2015 at 06:09:34PM +0100, Michel Thierry wrote:
> And prevent overflow warning during compilation. We already set this limit
> for the GGTT.
>
> This is a temporary patch until a full replacement of size_t variables
> (inadequate in 32-bit kernel) is in place.
>
> Regression from:
> commit a4e0bedca678c81eea4cd79a4bd502335639f73a
> Author: Michel Thierry <michel.thierry@intel.com>
> Date: Wed Apr 8 12:13:35 2015 +0100
>
> drm/i915: Use complete address space in true PPGTT
>
> v2: Prettify code and explain why this is needed. (Chris)
>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 17b7df0..0653c28 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -951,7 +951,16 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
> gen8_initialize_pd(&ppgtt->base, ppgtt->scratch_pd);
>
> ppgtt->base.start = 0;
> - ppgtt->base.total = 1ULL << 32;
> +#ifdef CONFIG_X86_32
FWIW, this is somewhat frowned upon (which is why Chris was using
IS_ENABLED(). It's documented in Documentation/CodingStyle, Chapter 20.
Within code, where possible, use the IS_ENABLED macro to convert a Kconfig
symbol into a C boolean expression, and use it in a normal C conditional:
if (IS_ENABLED(CONFIG_SOMETHING)) {
...
}
The compiler will constant-fold the conditional away, and include or exclude
the block of code just as with an #ifdef, so this will not add any runtime
overhead. However, this approach still allows the C compiler to see the code
inside the block, and check it for correctness (syntax, types, symbol
references, etc). Thus, you still have to use an #ifdef if the code inside the
block references symbols that will not exist if the condition is not met.
--
Damien
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915: limit PPGTT size to 2GB in 32-bit platforms
2015-05-28 17:09 ` [PATCH v2] " Michel Thierry
2015-05-28 17:47 ` Damien Lespiau
@ 2015-05-28 21:14 ` Chris Wilson
2015-05-29 8:58 ` Michel Thierry
1 sibling, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2015-05-28 21:14 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx, Mika Kuoppala
On Thu, May 28, 2015 at 06:09:34PM +0100, Michel Thierry wrote:
> And prevent overflow warning during compilation. We already set this limit
> for the GGTT.
>
> This is a temporary patch until a full replacement of size_t variables
> (inadequate in 32-bit kernel) is in place.
>
> Regression from:
> commit a4e0bedca678c81eea4cd79a4bd502335639f73a
> Author: Michel Thierry <michel.thierry@intel.com>
> Date: Wed Apr 8 12:13:35 2015 +0100
>
> drm/i915: Use complete address space in true PPGTT
>
> v2: Prettify code and explain why this is needed. (Chris)
>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 17b7df0..0653c28 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -951,7 +951,16 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
> gen8_initialize_pd(&ppgtt->base, ppgtt->scratch_pd);
>
> ppgtt->base.start = 0;
> - ppgtt->base.total = 1ULL << 32;
> +#ifdef CONFIG_X86_32
> + /* While we have a proliferation of size_t variables
> + * we cannot represent the full ppgtt size on 32bit,
> + * so limit it to the same size as the GGTT (currently
> + * 2GiB).
> + */
> + ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
> +#else
> + ppgtt->base.total = 1ULL << 32;
> +#endif
You missed the point of having it always compiled. Part of that is that
the compiler warning here is important.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915: limit PPGTT size to 2GB in 32-bit platforms
2015-05-28 21:14 ` Chris Wilson
@ 2015-05-29 8:58 ` Michel Thierry
2015-05-29 12:41 ` Chris Wilson
0 siblings, 1 reply; 10+ messages in thread
From: Michel Thierry @ 2015-05-29 8:58 UTC (permalink / raw)
To: Chris Wilson, intel-gfx, Mika Kuoppala
[-- Attachment #1.1: Type: text/plain, Size: 2030 bytes --]
On 5/28/2015 10:14 PM, Chris Wilson wrote:
> On Thu, May 28, 2015 at 06:09:34PM +0100, Michel Thierry wrote:
>> And prevent overflow warning during compilation. We already set this limit
>> for the GGTT.
>>
>> This is a temporary patch until a full replacement of size_t variables
>> (inadequate in 32-bit kernel) is in place.
>>
>> Regression from:
>> commit a4e0bedca678c81eea4cd79a4bd502335639f73a
>> Author: Michel Thierry <michel.thierry@intel.com>
>> Date: Wed Apr 8 12:13:35 2015 +0100
>>
>> drm/i915: Use complete address space in true PPGTT
>>
>> v2: Prettify code and explain why this is needed. (Chris)
>>
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 17b7df0..0653c28 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -951,7 +951,16 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>> gen8_initialize_pd(&ppgtt->base, ppgtt->scratch_pd);
>>
>> ppgtt->base.start = 0;
>> - ppgtt->base.total = 1ULL << 32;
>> +#ifdef CONFIG_X86_32
>> + /* While we have a proliferation of size_t variables
>> + * we cannot represent the full ppgtt size on 32bit,
>> + * so limit it to the same size as the GGTT (currently
>> + * 2GiB).
>> + */
>> + ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
>> +#else
>> + ppgtt->base.total = 1ULL << 32;
>> +#endif
> You missed the point of having it always compiled. Part of that is that
> the compiler warning here is important.
> -Chris
I was under the impression that Daniel wanted to get rid of the warning
too, that's why I didn't use IS_ENABLED().
[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5510 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915: limit PPGTT size to 2GB in 32-bit platforms
2015-05-29 8:58 ` Michel Thierry
@ 2015-05-29 12:41 ` Chris Wilson
0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2015-05-29 12:41 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx, Mika Kuoppala
On Fri, May 29, 2015 at 09:58:33AM +0100, Michel Thierry wrote:
> On 5/28/2015 10:14 PM, Chris Wilson wrote:
> >On Thu, May 28, 2015 at 06:09:34PM +0100, Michel Thierry wrote:
> >>And prevent overflow warning during compilation. We already set this limit
> >>for the GGTT.
> >>
> >>This is a temporary patch until a full replacement of size_t variables
> >>(inadequate in 32-bit kernel) is in place.
> >>
> >>Regression from:
> >> commit a4e0bedca678c81eea4cd79a4bd502335639f73a
> >> Author: Michel Thierry <michel.thierry@intel.com>
> >> Date: Wed Apr 8 12:13:35 2015 +0100
> >>
> >> drm/i915: Use complete address space in true PPGTT
> >>
> >>v2: Prettify code and explain why this is needed. (Chris)
> >>
> >>Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> >>Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >>Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> >>---
> >> drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++++++++-
> >> 1 file changed, 10 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>index 17b7df0..0653c28 100644
> >>--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>@@ -951,7 +951,16 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
> >> gen8_initialize_pd(&ppgtt->base, ppgtt->scratch_pd);
> >> ppgtt->base.start = 0;
> >>- ppgtt->base.total = 1ULL << 32;
> >>+#ifdef CONFIG_X86_32
> >>+ /* While we have a proliferation of size_t variables
> >>+ * we cannot represent the full ppgtt size on 32bit,
> >>+ * so limit it to the same size as the GGTT (currently
> >>+ * 2GiB).
> >>+ */
> >>+ ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
> >>+#else
> >>+ ppgtt->base.total = 1ULL << 32;
> >>+#endif
> >You missed the point of having it always compiled. Part of that is that
> >the compiler warning here is important.
> >-Chris
> I was under the impression that Daniel wanted to get rid of the
> warning too, that's why I didn't use IS_ENABLED().
It's bad form to have the #ifdef inside code, and in this case the
warning is good - it is a clear indicator that we have a large hole in
the code that I don't just want to sweep under the carpet.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3] drm/i915: limit PPGTT size to 2GB in 32-bit platforms
2015-05-28 15:24 [PATCH] drm/i915: limit PPGTT size to 2GB in 32-bit platforms Michel Thierry
2015-05-28 15:39 ` Chris Wilson
2015-05-28 17:09 ` [PATCH v2] " Michel Thierry
@ 2015-05-29 13:15 ` Michel Thierry
2015-05-29 17:08 ` Daniel Vetter
2 siblings, 1 reply; 10+ messages in thread
From: Michel Thierry @ 2015-05-29 13:15 UTC (permalink / raw)
To: intel-gfx; +Cc: Mika Kuoppala
We already set this limit for the GGTT.
This is a temporary patch until a full replacement of size_t variables
(inadequate in 32-bit kernel) is in place.
Regression from:
commit a4e0bedca678c81eea4cd79a4bd502335639f73a
Author: Michel Thierry <michel.thierry@intel.com>
Date: Wed Apr 8 12:13:35 2015 +0100
drm/i915: Use complete address space in true PPGTT
v2: Prettify code and explain why this is needed. (Chris)
v3: Don't hide the compilation warning in 32-bit. (Chris)
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 17b7df0..619dad1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -952,6 +952,13 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
ppgtt->base.start = 0;
ppgtt->base.total = 1ULL << 32;
+ if (IS_ENABLED(CONFIG_X86_32))
+ /* While we have a proliferation of size_t variables
+ * we cannot represent the full ppgtt size on 32bit,
+ * so limit it to the same size as the GGTT (currently
+ * 2GiB).
+ */
+ ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
ppgtt->base.cleanup = gen8_ppgtt_cleanup;
ppgtt->base.allocate_va_range = gen8_alloc_va_range;
ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
--
2.4.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3] drm/i915: limit PPGTT size to 2GB in 32-bit platforms
2015-05-29 13:15 ` [PATCH v3] " Michel Thierry
@ 2015-05-29 17:08 ` Daniel Vetter
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2015-05-29 17:08 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx, Mika Kuoppala
On Fri, May 29, 2015 at 02:15:05PM +0100, Michel Thierry wrote:
> We already set this limit for the GGTT.
>
> This is a temporary patch until a full replacement of size_t variables
> (inadequate in 32-bit kernel) is in place.
>
> Regression from:
> commit a4e0bedca678c81eea4cd79a4bd502335639f73a
> Author: Michel Thierry <michel.thierry@intel.com>
> Date: Wed Apr 8 12:13:35 2015 +0100
>
> drm/i915: Use complete address space in true PPGTT
>
> v2: Prettify code and explain why this is needed. (Chris)
> v3: Don't hide the compilation warning in 32-bit. (Chris)
>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 17b7df0..619dad1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -952,6 +952,13 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>
> ppgtt->base.start = 0;
> ppgtt->base.total = 1ULL << 32;
> + if (IS_ENABLED(CONFIG_X86_32))
> + /* While we have a proliferation of size_t variables
> + * we cannot represent the full ppgtt size on 32bit,
> + * so limit it to the same size as the GGTT (currently
> + * 2GiB).
> + */
> + ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
> ppgtt->base.cleanup = gen8_ppgtt_cleanup;
> ppgtt->base.allocate_va_range = gen8_alloc_va_range;
> ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
> --
> 2.4.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-05-29 17:06 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-28 15:24 [PATCH] drm/i915: limit PPGTT size to 2GB in 32-bit platforms Michel Thierry
2015-05-28 15:39 ` Chris Wilson
2015-05-28 15:59 ` Michel Thierry
2015-05-28 17:09 ` [PATCH v2] " Michel Thierry
2015-05-28 17:47 ` Damien Lespiau
2015-05-28 21:14 ` Chris Wilson
2015-05-29 8:58 ` Michel Thierry
2015-05-29 12:41 ` Chris Wilson
2015-05-29 13:15 ` [PATCH v3] " Michel Thierry
2015-05-29 17:08 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox