drm/i915/bdw: Don't allow the FBC base to be 0
Subsystems (2)
PATCH 1/1 drm/i915/bdw: Don't allow the FBC base to be 0
"Restriction :
The offset must be greater than 4K bytes, avoiding the first 4KB of stolen memory."
Since it looks like we currently allocate an overlay out of stolen before we get the compressed framebuffer, I believe this is not currently an issue which fixes anything. We simply want to make the code as future-proof, and as clear as possible, by match the spec.
--- drivers/gpu/drm/i915/i915_gem_stolen.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
drivers/gpu/drm/i915/i915_gem_stolen.c +10 -5
@@ -107,19 +107,24 @@ static int i915_setup_compression(struct drm_device *dev, int size) {struct drm_i915_private *dev_priv = dev->dev_private;struct drm_mm_node *compressed_fb, *uninitialized_var (compressed_llb ); +unsigned long start =0 ;int ret; compressed_fb =kzalloc (sizeof (*compressed_fb),GFP_KERNEL );if (!compressed_fb) goto err_llb; +if (IS_BROADWELL (dev)) + start =0x1000 ; +/* Try to over-allocate to reduce reallocations and fragmentation */ - ret =drm_mm_insert_node (&dev_priv->mm.stolen, compressed_fb, - size <<=1 ,4096 ,DRM_MM_SEARCH_DEFAULT ); + ret =drm_mm_insert_node_in_range (&dev_priv->mm.stolen, compressed_fb, + size <<=1 ,4096 , start, dev_priv->gtt.stolen_size, +DRM_MM_SEARCH_DEFAULT );if (ret) - ret =drm_mm_insert_node (&dev_priv->mm.stolen, compressed_fb, - size >>=1 ,4096 , -DRM_MM_SEARCH_DEFAULT ); + ret =drm_mm_insert_node_in_range (&dev_priv->mm.stolen, compressed_fb, + size >>=1 ,4096 , start, dev_priv->gtt.stolen_size, +DRM_MM_SEARCH_DEFAULT );if (ret) goto err_llb;
Signature · 1 line
1.9.2
Quoted 3 lines · Ben Widawsky
"Restriction :
The offset must be greater than 4K bytes, avoiding the first 4KB of stolen memory."
Isn't this a more generic issue that we must avoid the first 4k? If so I think we should just reserve the first 4k permanently at driver init time.
Quoted 53 lines
Since it looks like we currently allocate an overlay out of stolen before we get the compressed framebuffer, I believe this is not currently an issue which fixes anything. We simply want to make the code as future-proof, and as clear as possible, by match the spec.
Cc Art Runyan <[email protected]>Signed-off-by Ben Widawsky <[email protected]>--- drivers/gpu/drm/i915/i915_gem_stolen.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)drivers/gpu/drm/i915/i915_gem_stolen.c +10
-5@@ -107,19 +107,24 @@ static int i915_setup_compression(struct drm_device *dev, int size) {struct drm_i915_private *dev_priv = dev->dev_private;struct drm_mm_node *compressed_fb, *uninitialized_var (compressed_llb ); +unsigned long start =0 ;int ret; compressed_fb =kzalloc (sizeof (*compressed_fb),GFP_KERNEL );if (!compressed_fb) goto err_llb; +if (IS_BROADWELL (dev)) + start =0x1000 ; +/* Try to over-allocate to reduce reallocations and fragmentation */ - ret =drm_mm_insert_node (&dev_priv->mm.stolen, compressed_fb, - size <<=1 ,4096 ,DRM_MM_SEARCH_DEFAULT ); + ret =drm_mm_insert_node_in_range (&dev_priv->mm.stolen, compressed_fb, + size <<=1 ,4096 , start, dev_priv->gtt.stolen_size, +DRM_MM_SEARCH_DEFAULT );if (ret) - ret =drm_mm_insert_node (&dev_priv->mm.stolen, compressed_fb, - size >>=1 ,4096 , -DRM_MM_SEARCH_DEFAULT ); + ret =drm_mm_insert_node_in_range (&dev_priv->mm.stolen, compressed_fb, + size >>=1 ,4096 , start, dev_priv->gtt.stolen_size, +DRM_MM_SEARCH_DEFAULT );if (ret) goto err_llb;Signature · 6 lines
1.9.2 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 2 lines
Ville Syrjälä Intel OTC
Quoted 8 lines · Ville Syrjälä
Quoted 3 lines · Ben Widawsky
"Restriction :
The offset must be greater than 4K bytes, avoiding the first 4KB of stolen memory."Isn't this a more generic issue that we must avoid the first 4k? If so I think we should just reserve the first 4k permanently at driver init time.
What? On many machines the vga framebuffer is allocated from offset 0. I think some explanation is in order.
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
Quoted 6 lines · Chris Wilson
Quoted 5 lines · Ville Syrjälä
Quoted 3 lines · Ben Widawsky
"Restriction :
The offset must be greater than 4K bytes, avoiding the first 4KB of stolen memory."
Quoted 6 lines
Quoted 3 lines
Isn't this a more generic issue that we must avoid the first 4k? If so I think we should just reserve the first 4k permanently at driver init time.
What? On many machines the vga framebuffer is allocated from offset 0. I think some explanation is in order.
WaSkipStolenMemoryFirstPage
Signature · 2 lines
Ville Syrjälä Intel OTC
Quoted 10 lines · Ville Syrjälä
Quoted 9 lines · Chris Wilson
Quoted 8 lines · Ville Syrjälä
Quoted 3 lines · Ben Widawsky
"Restriction :
The offset must be greater than 4K bytes, avoiding the first 4KB of stolen memory."Isn't this a more generic issue that we must avoid the first 4k? If so I think we should just reserve the first 4k permanently at driver init time.
Is anyone opposed to this plan? Realistically it won't make a difference.
Quoted 3 lines
Quoted 3 lines
What? On many machines the vga framebuffer is allocated from offset 0. I think some explanation is in order.
The chopped off part of the commit message explained it (if I understood your point).
Quoted 6 lines
WaSkipStolenMemoryFirstPage
Signature · 2 lines
Ville Syrjälä Intel OTC
Signature · 5 lines
Ben Widawsky, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 16 lines · Ben Widawsky
Quoted 10 lines · Ville Syrjälä
Quoted 9 lines · Chris Wilson
Quoted 8 lines · Ville Syrjälä
Quoted 3 lines · Ben Widawsky
"Restriction :
The offset must be greater than 4K bytes, avoiding the first 4KB of stolen memory."Isn't this a more generic issue that we must avoid the first 4k? If so I think we should just reserve the first 4k permanently at driver init time.
Is anyone opposed to this plan? Realistically it won't make a difference.
Quoted 1 line
Quoted 1 line
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines
What? On many machines the vga framebuffer is allocated from offset 0. I think some explanation is in order.
The chopped off part of the commit message explained it (if I understood your point).
How do we handle the inherited fb if it starts at offset 0? -Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
Quoted 17 lines · Chris Wilson
Quoted 16 lines · Ben Widawsky
Quoted 10 lines · Ville Syrjälä
Quoted 9 lines · Chris Wilson
Quoted 8 lines · Ville Syrjälä
Quoted 3 lines · Ben Widawsky
"Restriction :
The offset must be greater than 4K bytes, avoiding the first 4KB of stolen memory."Isn't this a more generic issue that we must avoid the first 4k? If so I think we should just reserve the first 4k permanently at driver init time.
Is anyone opposed to this plan? Realistically it won't make a difference.
Quoted 1 line
Quoted 1 line
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines
What? On many machines the vga framebuffer is allocated from offset 0. I think some explanation is in order.
The chopped off part of the commit message explained it (if I understood your point).
How do we handle the inherited fb if it starts at offset 0? -Chris
I must be missing something important. The FBC buffer is the only one requiring a non-zero offset from the base of stolen memory.
Signature · 5 lines
Ben Widawsky, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 24 lines · Ben Widawsky
Quoted 23 lines · Chris Wilson
Quoted 21 lines · Ben Widawsky
Quoted 10 lines · Ville Syrjälä
Quoted 9 lines · Chris Wilson
Quoted 8 lines · Ville Syrjälä
Quoted 3 lines · Ben Widawsky
"Restriction :
The offset must be greater than 4K bytes, avoiding the first 4KB of stolen memory."Isn't this a more generic issue that we must avoid the first 4k? If so I think we should just reserve the first 4k permanently at driver init time.
Is anyone opposed to this plan? Realistically it won't make a difference.
Quoted 3 lines
Quoted 3 lines
What? On many machines the vga framebuffer is allocated from offset 0. I think some explanation is in order.
The chopped off part of the commit message explained it (if I understood your point).
Quoted 6 lines
Quoted 3 lines
How do we handle the inherited fb if it starts at offset 0? -Chris
I must be missing something important. The FBC buffer is the only one requiring a non-zero offset from the base of stolen memory.
Nothing important can be placed there since the CS apparently writes there without anyone explicitly telling it to do so. That's what I gathered from the hsd anyway.
I don't have a real answer for the inherited config thing. Should we just not inherit in that case? One would hope that the BIOS doesn't put the FB at offset 0, but this being the BIOS I'm pretty sure it does. I guess we could always make a copy and flip, but it would be nice if we didn't have to. Or maybe just copy the first page somewhere else and set up the PTE to compensate? Though that might violate some assumption we have about stolen being always contiguous...
On a related note our FB takeover code is anywya rather optimistic since it blindly assumes that the ggtt offset matches the stolen offet. We never actually look at the PTEs to confirm that.
Signature · 2 lines
Ville Syrjälä Intel OTC
Quoted 6 lines · Ville Syrjälä
Quoted 2 lines
I must be missing something important. The FBC buffer is the only one requiring a non-zero offset from the base of stolen memory.
Nothing important can be placed there since the CS apparently writes there without anyone explicitly telling it to do so. That's what I gathered from the hsd anyway.
FWIW, that's what Art said in person as well a couple of weeks back. So, indeed, we need to reserve the first page of stolen on BDW.
Signature · 1 line
Damien