On Fri, Sep 30, 2022 at 03:10:41PM +0100, Matthew Auld wrote:
> On 30/09/2022 15:00, Ville Syrjälä wrote:
> > On Fri, Sep 30, 2022 at 02:47:30PM +0100, Matthew Auld wrote:
> >> For these types of display buffers, we need to able to CPU access some
> >> part of the backing memory in prepare_plane_clear_colors(). As a result
> >> we need to ensure we always place in the mappable part of lmem, which
> >> becomes necessary on small-bar systems.
> >>
> >> Fixes: eb1c535f0d69 ("drm/i915: turn on small BAR support")
> >> Reported-by: Jianshui Yu <jianshui.yu@xxxxxxxxx>
> >> Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx>
> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >> Cc: Nirmoy Das <nirmoy.das@xxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/i915/display/intel_fb_pin.c | 11 ++++--
> >> drivers/gpu/drm/i915/gem/i915_gem_object.c | 37 ++++++++++++++++++-
> >> drivers/gpu/drm/i915/gem/i915_gem_object.h | 4 ++
> >> .../gpu/drm/i915/gem/i915_gem_object_types.h | 3 +-
> >> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 5 ++-
> >> 5 files changed, 53 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c b/drivers/gpu/drm/i915/display/intel_fb_pin.c
> >> index c86e5d4ee016..f83cf41ddd63 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_fb_pin.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c
> >> @@ -139,9 +139,14 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
> >> ret = i915_gem_object_lock(obj, &ww);
> >> if (!ret && phys_cursor)
> >> ret = i915_gem_object_attach_phys(obj, alignment);
> >> - else if (!ret && HAS_LMEM(dev_priv))
> >> - ret = i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0);
> >> - /* TODO: Do we need to sync when migration becomes async? */
> >
> > Why is the TODO being removed?
>
> Just because we now know we do a fence sync below, when binding into the
> GGTT (this comment was from before we had async moves/migrations). I can
> a make a note of that in the commit message. Or perhaps change the
> comment to "Should we rather make this async, currently we sync below
> which is maybe not optimal?" :)
Shrug. As long as the commit message will get some explanation
we should be good. Could even be a separate patch since it
seems entirely orthogonal to the actual contents of this patch.
--
Ville Syrjälä
Intel
