public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Skip Stolen Memory first page.
@ 2014-07-30 17:47 Rodrigo Vivi
  2014-07-31  3:57 ` Ben Widawsky
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Rodrigo Vivi @ 2014-07-30 17:47 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

WA to skip the first page of stolen memory due to sporadic HW write on *CS Idle

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 21c025a..3acefb3 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -290,6 +290,7 @@ int i915_gem_init_stolen(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int bios_reserved = 0;
+	int initial_reserved = 0;
 
 #ifdef CONFIG_INTEL_IOMMU
 	if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
@@ -314,9 +315,13 @@ int i915_gem_init_stolen(struct drm_device *dev)
 	if (WARN_ON(bios_reserved > dev_priv->gtt.stolen_size))
 		return 0;
 
+	/* WaSkipStolenMemoryFirstPage */
+	if (INTEL_INFO(dev)->gen >= 8)
+		initial_reserved = 4096;
+
 	/* Basic memrange allocator for stolen space */
-	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
-		    bios_reserved);
+	drm_mm_init(&dev_priv->mm.stolen, initial_reserved,
+		    dev_priv->gtt.stolen_size - bios_reserved);
 
 	return 0;
 }
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Skip Stolen Memory first page.
  2014-07-30 17:47 [PATCH] drm/i915: Skip Stolen Memory first page Rodrigo Vivi
@ 2014-07-31  3:57 ` Ben Widawsky
  2014-07-31  5:57 ` Chris Wilson
  2014-07-31 13:39 ` Ville Syrjälä
  2 siblings, 0 replies; 9+ messages in thread
From: Ben Widawsky @ 2014-07-31  3:57 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Wed, Jul 30, 2014 at 10:47:46AM -0700, Rodrigo Vivi wrote:
> WA to skip the first page of stolen memory due to sporadic HW write on *CS Idle
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 21c025a..3acefb3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -290,6 +290,7 @@ int i915_gem_init_stolen(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int bios_reserved = 0;
> +	int initial_reserved = 0;
>  
>  #ifdef CONFIG_INTEL_IOMMU
>  	if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
> @@ -314,9 +315,13 @@ int i915_gem_init_stolen(struct drm_device *dev)
>  	if (WARN_ON(bios_reserved > dev_priv->gtt.stolen_size))
>  		return 0;
>  
> +	/* WaSkipStolenMemoryFirstPage */
> +	if (INTEL_INFO(dev)->gen >= 8)
> +		initial_reserved = 4096;
> +
>  	/* Basic memrange allocator for stolen space */
> -	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
> -		    bios_reserved);
> +	drm_mm_init(&dev_priv->mm.stolen, initial_reserved,
> +		    dev_priv->gtt.stolen_size - bios_reserved);
>  
>  	return 0;
>  }

Actually, you need to fix the size in the initialization:
drm_mm_init(stolen, 4096, dev_priv->gtt.stolen_size - bios_reserved - 4096);

Also, I think we have a bug here today, you need to
dev_priv->gtt.stolen_size -= ...

Or else we don't have the correct assertion in
i915_pages_create_for_stolen(). I didn't look carefully if it's safe to
currently do it, but it does seem wrong to me as it is.

With the first fix (I didn't verify that's the real workaround name),
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

-- 
Ben Widawsky, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Skip Stolen Memory first page.
  2014-07-30 17:47 [PATCH] drm/i915: Skip Stolen Memory first page Rodrigo Vivi
  2014-07-31  3:57 ` Ben Widawsky
@ 2014-07-31  5:57 ` Chris Wilson
  2014-07-31 13:39 ` Ville Syrjälä
  2 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2014-07-31  5:57 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Wed, Jul 30, 2014 at 10:47:46AM -0700, Rodrigo Vivi wrote:
> WA to skip the first page of stolen memory due to sporadic HW write on *CS Idle
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 21c025a..3acefb3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -290,6 +290,7 @@ int i915_gem_init_stolen(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int bios_reserved = 0;
> +	int initial_reserved = 0;

start_reserved, end_reserved?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Skip Stolen Memory first page.
  2014-07-30 17:47 [PATCH] drm/i915: Skip Stolen Memory first page Rodrigo Vivi
  2014-07-31  3:57 ` Ben Widawsky
  2014-07-31  5:57 ` Chris Wilson
@ 2014-07-31 13:39 ` Ville Syrjälä
  2014-07-31 19:08   ` Rodrigo Vivi
  2 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2014-07-31 13:39 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Wed, Jul 30, 2014 at 10:47:46AM -0700, Rodrigo Vivi wrote:
> WA to skip the first page of stolen memory due to sporadic HW write on *CS Idle

How does this interact with the BIOS FB takeover? Do BDW systems even
place the BIOS FB at the start if stolen due to this W/A? We have no
code to verify if that's true or not. The takeover code just blindly
assumes that it's true.

If the BIOS places the FB at the start of stolen, then I had an idea
of copying the first page over just past the end of the BIOS FB and
fixing up the the GTT to account for that. The CS is then free to 
clobber the first page of stolen without corrupting the FB. But I'm
not sure our stolen object code can deal with >1 entry sg lists
currently. Also we'd probably need to copy the page first to some
temporary place (maybe just end of stolen), and only once we know
the size of the BIOS FB it could be copied to the final place. Or I
guess we could just leave it at the end of stolen since the GTT
will remap any access anyway. As long as we don't access the stolen
memory directly this should work just fine.

> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 21c025a..3acefb3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -290,6 +290,7 @@ int i915_gem_init_stolen(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int bios_reserved = 0;
> +	int initial_reserved = 0;
>  
>  #ifdef CONFIG_INTEL_IOMMU
>  	if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
> @@ -314,9 +315,13 @@ int i915_gem_init_stolen(struct drm_device *dev)
>  	if (WARN_ON(bios_reserved > dev_priv->gtt.stolen_size))
>  		return 0;
>  
> +	/* WaSkipStolenMemoryFirstPage */
> +	if (INTEL_INFO(dev)->gen >= 8)
> +		initial_reserved = 4096;
> +
>  	/* Basic memrange allocator for stolen space */
> -	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
> -		    bios_reserved);
> +	drm_mm_init(&dev_priv->mm.stolen, initial_reserved,
> +		    dev_priv->gtt.stolen_size - bios_reserved);
>  
>  	return 0;
>  }
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] drm/i915: Skip Stolen Memory first page.
  2014-07-31 13:39 ` Ville Syrjälä
@ 2014-07-31 19:08   ` Rodrigo Vivi
  2014-08-01 16:34     ` Jesse Barnes
  0 siblings, 1 reply; 9+ messages in thread
From: Rodrigo Vivi @ 2014-07-31 19:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

WA to skip the first page of stolen memory due to sporadic HW write on *CS Idle

v2: Improve variable names and fix allocated size.

Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 21c025a..82035b0 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -289,7 +289,8 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
 int i915_gem_init_stolen(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int bios_reserved = 0;
+	int start_rsvd = 0;
+	int end_rsvd = 0;
 
 #ifdef CONFIG_INTEL_IOMMU
 	if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
@@ -308,15 +309,19 @@ int i915_gem_init_stolen(struct drm_device *dev)
 	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
 		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
 
+	/* WaSkipStolenMemoryFirstPage */
+	if (INTEL_INFO(dev)->gen >= 8)
+		start_rsvd = 4096;
+
 	if (IS_VALLEYVIEW(dev))
-		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
+		end_rsvd = 1024*1024; /* top 1M on VLV/BYT */
 
-	if (WARN_ON(bios_reserved > dev_priv->gtt.stolen_size))
+	if (WARN_ON((start_rsvd + end_rsvd) > dev_priv->gtt.stolen_size))
 		return 0;
 
 	/* Basic memrange allocator for stolen space */
-	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
-		    bios_reserved);
+	drm_mm_init(&dev_priv->mm.stolen, start_rsvd,
+		    dev_priv->gtt.stolen_size - start_rsvd - end_rsvd);
 
 	return 0;
 }
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Skip Stolen Memory first page.
  2014-07-31 19:08   ` Rodrigo Vivi
@ 2014-08-01 16:34     ` Jesse Barnes
  2015-02-03 16:11       ` Siluvery, Arun
  0 siblings, 1 reply; 9+ messages in thread
From: Jesse Barnes @ 2014-08-01 16:34 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Thu, 31 Jul 2014 12:08:20 -0700
Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:

> WA to skip the first page of stolen memory due to sporadic HW write on *CS Idle
> 
> v2: Improve variable names and fix allocated size.
> 
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 21c025a..82035b0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -289,7 +289,8 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
>  int i915_gem_init_stolen(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int bios_reserved = 0;
> +	int start_rsvd = 0;
> +	int end_rsvd = 0;
>  
>  #ifdef CONFIG_INTEL_IOMMU
>  	if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
> @@ -308,15 +309,19 @@ int i915_gem_init_stolen(struct drm_device *dev)
>  	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
>  		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
>  
> +	/* WaSkipStolenMemoryFirstPage */
> +	if (INTEL_INFO(dev)->gen >= 8)
> +		start_rsvd = 4096;
> +
>  	if (IS_VALLEYVIEW(dev))
> -		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
> +		end_rsvd = 1024*1024; /* top 1M on VLV/BYT */
>  
> -	if (WARN_ON(bios_reserved > dev_priv->gtt.stolen_size))
> +	if (WARN_ON((start_rsvd + end_rsvd) > dev_priv->gtt.stolen_size))
>  		return 0;
>  
>  	/* Basic memrange allocator for stolen space */
> -	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
> -		    bios_reserved);
> +	drm_mm_init(&dev_priv->mm.stolen, start_rsvd,
> +		    dev_priv->gtt.stolen_size - start_rsvd - end_rsvd);
>  
>  	return 0;
>  }

Beyond the fastboot stuff Ville has already mentioned, the early
allocation of the existing fb from stolen will prevent us from
clobbering the currently displayed buffer with the contents of the
ringbuffers and whatever else we allocate out of stolen at early boot.

We might be able to avoid that by doing stolen allocations top down, or
by reserving the displayed fb even if we can't allocate an obj for it,
only freeing it after our first mode set.

Can you file a bug or JIRA for that to make sure we don't lose track of
the fastboot & boot corruption issues after this fix lands?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Skip Stolen Memory first page.
  2014-08-01 16:34     ` Jesse Barnes
@ 2015-02-03 16:11       ` Siluvery, Arun
  2015-02-03 17:40         ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Siluvery, Arun @ 2015-02-03 16:11 UTC (permalink / raw)
  To: Jesse Barnes, Rodrigo Vivi; +Cc: intel-gfx

On 01/08/2014 17:34, Jesse Barnes wrote:
> On Thu, 31 Jul 2014 12:08:20 -0700
> Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>
>> WA to skip the first page of stolen memory due to sporadic HW write on *CS Idle
>>
>> v2: Improve variable names and fix allocated size.
>>
>> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_gem_stolen.c | 15 ++++++++++-----
>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> index 21c025a..82035b0 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> @@ -289,7 +289,8 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
>>   int i915_gem_init_stolen(struct drm_device *dev)
>>   {
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>> -	int bios_reserved = 0;
>> +	int start_rsvd = 0;
>> +	int end_rsvd = 0;
>>
>>   #ifdef CONFIG_INTEL_IOMMU
>>   	if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
>> @@ -308,15 +309,19 @@ int i915_gem_init_stolen(struct drm_device *dev)
>>   	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
>>   		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
>>
>> +	/* WaSkipStolenMemoryFirstPage */
>> +	if (INTEL_INFO(dev)->gen >= 8)
>> +		start_rsvd = 4096;
>> +
>>   	if (IS_VALLEYVIEW(dev))
>> -		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
>> +		end_rsvd = 1024*1024; /* top 1M on VLV/BYT */
>>
>> -	if (WARN_ON(bios_reserved > dev_priv->gtt.stolen_size))
>> +	if (WARN_ON((start_rsvd + end_rsvd) > dev_priv->gtt.stolen_size))
>>   		return 0;
>>
>>   	/* Basic memrange allocator for stolen space */
>> -	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
>> -		    bios_reserved);
>> +	drm_mm_init(&dev_priv->mm.stolen, start_rsvd,
>> +		    dev_priv->gtt.stolen_size - start_rsvd - end_rsvd);
>>
>>   	return 0;
>>   }
>
> Beyond the fastboot stuff Ville has already mentioned, the early
> allocation of the existing fb from stolen will prevent us from
> clobbering the currently displayed buffer with the contents of the
> ringbuffers and whatever else we allocate out of stolen at early boot.
>
> We might be able to avoid that by doing stolen allocations top down, or
> by reserving the displayed fb even if we can't allocate an obj for it,
> only freeing it after our first mode set.
>
> Can you file a bug or JIRA for that to make sure we don't lose track of
> the fastboot & boot corruption issues after this fix lands?

Reviving an old thread,
Any particular reason why this patch is not merged to nightly?
Is it known to cause any other regressions?

regards
Arun


>
> Thanks,
>

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Skip Stolen Memory first page.
  2015-02-03 16:11       ` Siluvery, Arun
@ 2015-02-03 17:40         ` Ville Syrjälä
  2015-02-03 19:24           ` Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2015-02-03 17:40 UTC (permalink / raw)
  To: Siluvery, Arun; +Cc: intel-gfx, Rodrigo Vivi

On Tue, Feb 03, 2015 at 04:11:05PM +0000, Siluvery, Arun wrote:
> On 01/08/2014 17:34, Jesse Barnes wrote:
> > On Thu, 31 Jul 2014 12:08:20 -0700
> > Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> >
> >> WA to skip the first page of stolen memory due to sporadic HW write on *CS Idle
> >>
> >> v2: Improve variable names and fix allocated size.
> >>
> >> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/i915_gem_stolen.c | 15 ++++++++++-----
> >>   1 file changed, 10 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> >> index 21c025a..82035b0 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> >> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> >> @@ -289,7 +289,8 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
> >>   int i915_gem_init_stolen(struct drm_device *dev)
> >>   {
> >>   	struct drm_i915_private *dev_priv = dev->dev_private;
> >> -	int bios_reserved = 0;
> >> +	int start_rsvd = 0;
> >> +	int end_rsvd = 0;
> >>
> >>   #ifdef CONFIG_INTEL_IOMMU
> >>   	if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
> >> @@ -308,15 +309,19 @@ int i915_gem_init_stolen(struct drm_device *dev)
> >>   	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
> >>   		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
> >>
> >> +	/* WaSkipStolenMemoryFirstPage */
> >> +	if (INTEL_INFO(dev)->gen >= 8)
> >> +		start_rsvd = 4096;
> >> +
> >>   	if (IS_VALLEYVIEW(dev))
> >> -		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
> >> +		end_rsvd = 1024*1024; /* top 1M on VLV/BYT */
> >>
> >> -	if (WARN_ON(bios_reserved > dev_priv->gtt.stolen_size))
> >> +	if (WARN_ON((start_rsvd + end_rsvd) > dev_priv->gtt.stolen_size))
> >>   		return 0;
> >>
> >>   	/* Basic memrange allocator for stolen space */
> >> -	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
> >> -		    bios_reserved);
> >> +	drm_mm_init(&dev_priv->mm.stolen, start_rsvd,
> >> +		    dev_priv->gtt.stolen_size - start_rsvd - end_rsvd);
> >>
> >>   	return 0;
> >>   }
> >
> > Beyond the fastboot stuff Ville has already mentioned, the early
> > allocation of the existing fb from stolen will prevent us from
> > clobbering the currently displayed buffer with the contents of the
> > ringbuffers and whatever else we allocate out of stolen at early boot.
> >
> > We might be able to avoid that by doing stolen allocations top down, or
> > by reserving the displayed fb even if we can't allocate an obj for it,
> > only freeing it after our first mode set.
> >
> > Can you file a bug or JIRA for that to make sure we don't lose track of
> > the fastboot & boot corruption issues after this fix lands?
> 
> Reviving an old thread,
> Any particular reason why this patch is not merged to nightly?
> Is it known to cause any other regressions?

It breaks the BIOS fb takeover like I said several times.

If no one is willing to fix it properly I was thinking we might just
try to do the BIOS fb takeover, and if it succeeded we do nothing else,
otherwise we allocate an unused 1 page object to keep the rings/fbc
buffer/etc. away from the first page.

The first page corruption supposedly happens only when the CS is doing
stuff, so if the CS corrupts the fbcon a bit it's no big deal. And
since we don't accelerate the fbconf hooks the corruption shouldn't
really happen under normal conditions anyway. You could see it while
running some igts or something, but that's not a huge problem.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Skip Stolen Memory first page.
  2015-02-03 17:40         ` Ville Syrjälä
@ 2015-02-03 19:24           ` Daniel Vetter
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2015-02-03 19:24 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Rodrigo Vivi

On Tue, Feb 03, 2015 at 07:40:21PM +0200, Ville Syrjälä wrote:
> On Tue, Feb 03, 2015 at 04:11:05PM +0000, Siluvery, Arun wrote:
> > On 01/08/2014 17:34, Jesse Barnes wrote:
> > > On Thu, 31 Jul 2014 12:08:20 -0700
> > > Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > >
> > >> WA to skip the first page of stolen memory due to sporadic HW write on *CS Idle
> > >>
> > >> v2: Improve variable names and fix allocated size.
> > >>
> > >> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> > >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > >> ---
> > >>   drivers/gpu/drm/i915/i915_gem_stolen.c | 15 ++++++++++-----
> > >>   1 file changed, 10 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > >> index 21c025a..82035b0 100644
> > >> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > >> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > >> @@ -289,7 +289,8 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
> > >>   int i915_gem_init_stolen(struct drm_device *dev)
> > >>   {
> > >>   	struct drm_i915_private *dev_priv = dev->dev_private;
> > >> -	int bios_reserved = 0;
> > >> +	int start_rsvd = 0;
> > >> +	int end_rsvd = 0;
> > >>
> > >>   #ifdef CONFIG_INTEL_IOMMU
> > >>   	if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
> > >> @@ -308,15 +309,19 @@ int i915_gem_init_stolen(struct drm_device *dev)
> > >>   	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
> > >>   		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
> > >>
> > >> +	/* WaSkipStolenMemoryFirstPage */
> > >> +	if (INTEL_INFO(dev)->gen >= 8)
> > >> +		start_rsvd = 4096;
> > >> +
> > >>   	if (IS_VALLEYVIEW(dev))
> > >> -		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
> > >> +		end_rsvd = 1024*1024; /* top 1M on VLV/BYT */
> > >>
> > >> -	if (WARN_ON(bios_reserved > dev_priv->gtt.stolen_size))
> > >> +	if (WARN_ON((start_rsvd + end_rsvd) > dev_priv->gtt.stolen_size))
> > >>   		return 0;
> > >>
> > >>   	/* Basic memrange allocator for stolen space */
> > >> -	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
> > >> -		    bios_reserved);
> > >> +	drm_mm_init(&dev_priv->mm.stolen, start_rsvd,
> > >> +		    dev_priv->gtt.stolen_size - start_rsvd - end_rsvd);
> > >>
> > >>   	return 0;
> > >>   }
> > >
> > > Beyond the fastboot stuff Ville has already mentioned, the early
> > > allocation of the existing fb from stolen will prevent us from
> > > clobbering the currently displayed buffer with the contents of the
> > > ringbuffers and whatever else we allocate out of stolen at early boot.
> > >
> > > We might be able to avoid that by doing stolen allocations top down, or
> > > by reserving the displayed fb even if we can't allocate an obj for it,
> > > only freeing it after our first mode set.
> > >
> > > Can you file a bug or JIRA for that to make sure we don't lose track of
> > > the fastboot & boot corruption issues after this fix lands?
> > 
> > Reviving an old thread,
> > Any particular reason why this patch is not merged to nightly?
> > Is it known to cause any other regressions?
> 
> It breaks the BIOS fb takeover like I said several times.
> 
> If no one is willing to fix it properly I was thinking we might just
> try to do the BIOS fb takeover, and if it succeeded we do nothing else,
> otherwise we allocate an unused 1 page object to keep the rings/fbc
> buffer/etc. away from the first page.
> 
> The first page corruption supposedly happens only when the CS is doing
> stuff, so if the CS corrupts the fbcon a bit it's no big deal. And
> since we don't accelerate the fbconf hooks the corruption shouldn't
> really happen under normal conditions anyway. You could see it while
> running some igts or something, but that's not a huge problem.

Without fbcon we still reconstruct the fb, but will free it on the first
modeset when userspace provides a real framebuffer. So this approach
doesn't work.

Instead we need to teach the stolen allocation functions to respect the
limit. We can't just restrict the entire drm_mm like in this patch since
the preallocated stolen obj logic must keep on working. Otherwise we break
fastboot.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - 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] 9+ messages in thread

end of thread, other threads:[~2015-02-03 19:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-30 17:47 [PATCH] drm/i915: Skip Stolen Memory first page Rodrigo Vivi
2014-07-31  3:57 ` Ben Widawsky
2014-07-31  5:57 ` Chris Wilson
2014-07-31 13:39 ` Ville Syrjälä
2014-07-31 19:08   ` Rodrigo Vivi
2014-08-01 16:34     ` Jesse Barnes
2015-02-03 16:11       ` Siluvery, Arun
2015-02-03 17:40         ` Ville Syrjälä
2015-02-03 19:24           ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox