[Intel-gfx] [PATCH 12/19] drm/i915/lmem: Bypass aperture when lmem is available

[Tvrtko Ursulin]

On 16/04/2021 15:25, Matthew Auld wrote:
> On 14/04/2021 16:33, Tvrtko Ursulin wrote:
>>
>> On 12/04/2021 10:05, Matthew Auld wrote:
>>> From: Anusha Srivatsa <anusha.srivatsa at intel.com>
>>>
>>> In the scenario where local memory is available, we have
>>> rely on CPU access via lmem directly instead of aperture.
>>>
>>> v2:
>>> gmch is only relevant for much older hw, therefore we can drop the
>>> has_aperture check since it should always be present on such platforms.
>>> (Chris)
>>>
>>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
>>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>> Cc: Chris P Wilson <chris.p.wilson at intel.com>
>>> Cc: Daniel Vetter <daniel.vetter at intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>> Cc: CQ Tang <cq.tang at intel.com>
>>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_fbdev.c | 22 +++++++++++++++-------
>>>   drivers/gpu/drm/i915/gem/i915_gem_lmem.c   | 15 +++++++++++++++
>>>   drivers/gpu/drm/i915/gem/i915_gem_lmem.h   |  5 +++++
>>>   drivers/gpu/drm/i915/i915_vma.c            | 19 +++++++++++++------
>>>   4 files changed, 48 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c 
>>> b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>> index 2b37959da747..4af40229f5ec 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>> @@ -139,14 +139,22 @@ static int intelfb_alloc(struct drm_fb_helper 
>>> *helper,
>>>       size = mode_cmd.pitches[0] * mode_cmd.height;
>>>       size = PAGE_ALIGN(size);
>>> -    /* If the FB is too big, just don't use it since fbdev is not very
>>> -     * important and we should probably use that space with FBC or 
>>> other
>>> -     * features. */
>>>       obj = ERR_PTR(-ENODEV);
>>> -    if (size * 2 < dev_priv->stolen_usable_size)
>>> -        obj = i915_gem_object_create_stolen(dev_priv, size);
>>> -    if (IS_ERR(obj))
>>> -        obj = i915_gem_object_create_shmem(dev_priv, size);
>>> +    if (HAS_LMEM(dev_priv)) {
>>> +        obj = i915_gem_object_create_lmem(dev_priv, size,
>>> +                          I915_BO_ALLOC_CONTIGUOUS);
>>
>> Has to be contiguous? Question for display experts I guess.
>>
>> [Comes back later.] Ah for iomap? Put a comment to that effect perhaps?
> 
> I don't think it has to be, since we could in theory just use pin_map() 
> underneath, which can already deal with non-contiguous chunks of lmem, 
> although that might bring in ww locking. I think for now just add a 
> comment and mark this as XXX, and potentially revisit as follow up?

Sure.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko