i915: Fix obj size vs. alignment for drm_pci_alloc()

Commit Message

Ville Syrjala Sept. 7, 2017, 2:32 p.m. UTC

From: Ville Syrjälä <[e-mail]>

drm_pci_alloc() refuses to cooperate if the passed alignment exceeds the
object size. So round up the obj size to the next power of two as well
to make this actually work.

Obviously things work just fine as long as the size was a power of two
to begin with. However kms_cursor_crc doesn't always use power of two
sizes so we hit a failure when we try to allocate the phys memory.

Testcase: igt/kms_cursor_crc
Signed-off-by: Ville Syrjälä <[e-mail]>
---
 drivers/gpu/drm/i915/i915_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Chris Wilson Sept. 7, 2017, 2:43 p.m. UTC | #1

Quoting [e-mail] (2017-09-07 15:32:03)
> From: Ville Syrjälä <[e-mail]>
> 
> drm_pci_alloc() refuses to cooperate if the passed alignment exceeds the
> object size. So round up the obj size to the next power of two as well
> to make this actually work.

'Tis true.

> to begin with. However kms_cursor_crc doesn't always use power of two
> sizes so we hit a failure when we try to allocate the phys memory.
> 
> Testcase: igt/kms_cursor_crc
> Signed-off-by: Ville Syrjälä <[e-mail]>
Reviewed-by: Chris Wilson <[e-mail]>

Shouldn't we remove the restriction from drm_pci.c? Seem like it is
second guessing the actual dma allocator. We should just kill it
entirely...
-Chris

Ville Syrjala Sept. 7, 2017, 2:53 p.m. UTC | #2

On Thu, Sep 07, 2017 at 03:43:26PM +0100, Chris Wilson wrote:
> Quoting [e-mail] (2017-09-07 15:32:03)
> > From: Ville Syrjälä <[e-mail]>
> > 
> > drm_pci_alloc() refuses to cooperate if the passed alignment exceeds the
> > object size. So round up the obj size to the next power of two as well
> > to make this actually work.
> 
> 'Tis true.
> 
> > to begin with. However kms_cursor_crc doesn't always use power of two
> > sizes so we hit a failure when we try to allocate the phys memory.
> > 
> > Testcase: igt/kms_cursor_crc
> > Signed-off-by: Ville Syrjälä <[e-mail]>
> Reviewed-by: Chris Wilson <[e-mail]>
> 
> Shouldn't we remove the restriction from drm_pci.c? Seem like it is
> second guessing the actual dma allocator. We should just kill it
> entirely...

Perhaps. I was feeling lazy today though so left it at this.

Ville Syrjala Sept. 7, 2017, 6:58 p.m. UTC | #3

On Thu, Sep 07, 2017 at 05:53:30PM +0300, Ville Syrjälä wrote:
> On Thu, Sep 07, 2017 at 03:43:26PM +0100, Chris Wilson wrote:
> > Quoting [e-mail] (2017-09-07 15:32:03)
> > > From: Ville Syrjälä <[e-mail]>
> > > 
> > > drm_pci_alloc() refuses to cooperate if the passed alignment exceeds the
> > > object size. So round up the obj size to the next power of two as well
> > > to make this actually work.
> > 
> > 'Tis true.
> > 
> > > to begin with. However kms_cursor_crc doesn't always use power of two
> > > sizes so we hit a failure when we try to allocate the phys memory.
> > > 
> > > Testcase: igt/kms_cursor_crc
> > > Signed-off-by: Ville Syrjälä <[e-mail]>
> > Reviewed-by: Chris Wilson <[e-mail]>
> > 
> > Shouldn't we remove the restriction from drm_pci.c? Seem like it is
> > second guessing the actual dma allocator. We should just kill it
> > entirely...
> 
> Perhaps. I was feeling lazy today though so left it at this.

And now pushed to dinq. Thanks for the review.

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4dffebae5601..822719fa1b52 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -179,7 +179,7 @@  i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
 	 * the alignment of the buddy allocation will naturally match.
 	 */
 	phys = drm_pci_alloc(obj->base.dev,
-			     obj->base.size,
+			     roundup_pow_of_two(obj->base.size),
 			     roundup_pow_of_two(obj->base.size));
 	if (!phys)
 		return ERR_PTR(-ENOMEM);