On Fri, Sep 30, 2022 at 05:00:40PM +0300, 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?
>
> > + else if (!ret && HAS_LMEM(dev_priv)) {
> > + unsigned int flags = obj->flags;
> > +
>
> It might not be super obvious what is going on here, so maybe add
> a comment stating the CPU needs to read the clear color from the bo?
>
> > + if (intel_fb_rc_ccs_cc_plane(fb) >= 0)
> > + flags &= ~I915_BO_ALLOC_GPU_ONLY;
>
> Hmm. Do we require the clear color plane to be in the same bo as the
> rest of the planes? I know we require the main and aux to be in the
> same bo, but dunno why we would require that also of the clear color
> plane (apart from being lazy perhaps).
I guess we must since we call this only once for the whole fb.
--
Ville Syrjälä
Intel
