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

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Matthew Auld <matthew.auld@intel.com>, intel-gfx@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	dri-devel@lists.freedesktop.org,
	Chris P Wilson <chris.p.wilson@intel.com>,
	Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Subject: Re: [Intel-gfx] [PATCH 12/19] drm/i915/lmem: Bypass aperture when lmem is available
Date: Mon, 19 Apr 2021 15:16:48 +0100	[thread overview]
Message-ID: <a8ffc582-e5d2-1d7f-44a0-c9bfd714f0fa@linux.intel.com> (raw)
In-Reply-To: <30d80560-f4ab-b20b-b80c-ca1e22128d26@intel.com>

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@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@linux.intel.com>
>>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Cc: Chris P Wilson <chris.p.wilson@intel.com>
>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>> Cc: CQ Tang <cq.tang@intel.com>
>>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@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@intel.com>

Regards,

Tvrtko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel