diff mbox series

[RFC,2/2] drm/i915: Account for platform without mappable aperture when returning size

Message ID 20190307191115.35336-2-antonio.argenziano@intel.com (mailing list archive)
State New, archived
Headers show
Series [RFC,1/2] drm/i915: Return mappable aperture size | expand

Commit Message

Antonio Argenziano March 7, 2019, 7:11 p.m. UTC
Some devices will not expose a mappable aperture anymore so we need to
return an appropriate value in that case.

Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>

Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Chris Wilson March 7, 2019, 9:59 p.m. UTC | #1
Quoting Antonio Argenziano (2019-03-07 19:11:15)
> Some devices will not expose a mappable aperture anymore so we need to
> return an appropriate value in that case.
> 
> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> 
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index d109f6dbe992..5125a5329100 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -221,7 +221,8 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
>         args->aper_size = ggtt->vm.total;
>         args->aper_available_size = args->aper_size - pinned;
>  
> -       args->mappable_aperture_size = ggtt->mappable_end;
> +       args->mappable_aperture_size =
> +                       HAS_MAPPABLE_APERTURE(to_i915(dev)) ? ggtt->mappable_end : (__u64)-ENODEV;

Looking above, args->aper_size will also be 0 so that'll be a good clue
for ENODEV. So maybe we can get away with the ambiguous 0 here as we can
use aper_size to differentiate the classes of HW.
-Chris
Antonio Argenziano March 7, 2019, 10:21 p.m. UTC | #2
On 07/03/19 13:59, Chris Wilson wrote:
> Quoting Antonio Argenziano (2019-03-07 19:11:15)
>> Some devices will not expose a mappable aperture anymore so we need to
>> return an appropriate value in that case.
>>
>> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
>>
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/i915_gem.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index d109f6dbe992..5125a5329100 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -221,7 +221,8 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
>>          args->aper_size = ggtt->vm.total;
>>          args->aper_available_size = args->aper_size - pinned;
>>   
>> -       args->mappable_aperture_size = ggtt->mappable_end;
>> +       args->mappable_aperture_size =
>> +                       HAS_MAPPABLE_APERTURE(to_i915(dev)) ? ggtt->mappable_end : (__u64)-ENODEV;
> 
> Looking above, args->aper_size will also be 0 so that'll be a good clue

Is it? ggtt should still be there so that shouldn't be zero.

Antonio

> for ENODEV. So maybe we can get away with the ambiguous 0 here as we can
> use aper_size to differentiate the classes of HW.
> -Chris
>
Chris Wilson March 7, 2019, 10:36 p.m. UTC | #3
Quoting Antonio Argenziano (2019-03-07 22:21:17)
> 
> 
> On 07/03/19 13:59, Chris Wilson wrote:
> > Quoting Antonio Argenziano (2019-03-07 19:11:15)
> >> Some devices will not expose a mappable aperture anymore so we need to
> >> return an appropriate value in that case.
> >>
> >> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> >>
> >> Cc: Matthew Auld <matthew.auld@intel.com>
> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> ---
> >>   drivers/gpu/drm/i915/i915_gem.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >> index d109f6dbe992..5125a5329100 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem.c
> >> +++ b/drivers/gpu/drm/i915/i915_gem.c
> >> @@ -221,7 +221,8 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
> >>          args->aper_size = ggtt->vm.total;
> >>          args->aper_available_size = args->aper_size - pinned;
> >>   
> >> -       args->mappable_aperture_size = ggtt->mappable_end;
> >> +       args->mappable_aperture_size =
> >> +                       HAS_MAPPABLE_APERTURE(to_i915(dev)) ? ggtt->mappable_end : (__u64)-ENODEV;
> > 
> > Looking above, args->aper_size will also be 0 so that'll be a good clue
> 
> Is it? ggtt should still be there so that shouldn't be zero.

Fair enough. We could lie though. Userspace isn't allowed to directly
access aper after gen7 (full-ppgtt), only via GGTT mmaps. With no more
GGTT mmaps, aper_size is ostensibly meaningless.

However, you never know, so keeping the information we do have and
reporting no mappable HW as ENODEV has a certain charm. Let's see if
anyone else has a better idea.
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d109f6dbe992..5125a5329100 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -221,7 +221,8 @@  i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
 	args->aper_size = ggtt->vm.total;
 	args->aper_available_size = args->aper_size - pinned;
 
-	args->mappable_aperture_size = ggtt->mappable_end;
+	args->mappable_aperture_size =
+			HAS_MAPPABLE_APERTURE(to_i915(dev)) ? ggtt->mappable_end : (__u64)-ENODEV;
 
 	return 0;
 }