Re: [PATCH 3/3] drm/i915: FB backing gem obj should reside in LMEM

[Ramalingam C <ramalingam.c@intel.com>]

On 2019-09-26 at 10:13:28 +0100, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-09-26 09:53:03)
> > 
> > On 26/09/2019 06:21, Ramalingam C wrote:
> > > If Local memory is supported by hardware, we want framebuffer backing
> > > gem objects out of local memory.
> > > 
> > > If local memory is supported and gem object if not from local memory we
> > > migrate the obj into local memory. And once framebuffer is created we
> > > block the migration of the associated object out of local memory.
> > > 
> > > This is developed on top of v3 LMEM series
> > > https://patchwork.freedesktop.org/series/56683/
> > > 
> > > cc: Matthew Auld <matthew.auld@intel.com>
> > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/display/intel_display.c | 25 +++++++++
> > >   drivers/gpu/drm/i915/gem/i915_gem_object.c   | 58 ++++++++++++--------
> > >   drivers/gpu/drm/i915/gem/i915_gem_object.h   |  3 +
> > >   3 files changed, 62 insertions(+), 24 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 1cc74844d3ea..d1921a317066 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -56,6 +56,8 @@
> > >   #include "display/intel_tv.h"
> > >   #include "display/intel_vdsc.h"
> > >   
> > > +#include "gem/i915_gem_object.h"
> > > +
> > >   #include "i915_drv.h"
> > >   #include "i915_trace.h"
> > >   #include "intel_acpi.h"
> > > @@ -15496,6 +15498,10 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
> > >   static void intel_user_framebuffer_destroy(struct drm_framebuffer *fb)
> > >   {
> > >       struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> > > +     struct drm_i915_private *dev_priv = to_i915(fb->dev);
> > > +
> > > +     /* removing the FB memory region restriction on obj, if any */
> > > +     intel_fb->front_buffer->obj = INTEL_INFO(dev_priv)->memory_regions;
> > 
> > Is this right, assigning bitmask to something called obj?
> > 
> > >   
> > >       drm_framebuffer_cleanup(fb);
> > >       intel_frontbuffer_put(intel_fb->frontbuffer);
> > > @@ -15543,11 +15549,26 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
> > >   {
> > >       struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
> > >       struct drm_framebuffer *fb = &intel_fb->base;
> > > +     u32 *region_map;
> > >       u32 max_stride;
> > >       unsigned int tiling, stride;
> > >       int ret = -EINVAL;
> > >       int i;
> > >   
> > > +     /* GEM Obj for frame buffer is expected to be in LMEM. */
> > > +     if (HAS_LMEM(dev_priv))
> > > +             if (obj->mm.region->type != INTEL_LMEM) {
> > > +                     region_map = &intel_region_map[INTEL_MEMORY_LMEM];
> > > +                     ret = i915_gem_object_mem_region_migrate(dev_priv, obj,
> > > +                                                              region_map,
> > > +                                                              1);
> > > +                     if (ret) {
> > > +                             DRM_ERROR("FB migration to LMEM Failed(%d)\n",
> > > +                                       ret);
> > 
> > Probably should be just debug level since it is imaginably user 
> > triggerable and not really an error for the kernel as such.
> 
> This would be a part of pin_to_display? That's where we do the other
> conversions required for scanout objects.

I will relook through it chris. But I am just wondering when the FB is
created/destroyed itself could we fix/release the memory region required?

-Ram.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx