[Intel-xe] [PATCH] drm/xe: Allow scanout of bos that can be migrated to lmem

[Rodrigo Vivi]

On Wed, May 17, 2023 at 09:29:18AM -0700, José Roberto de Souza wrote:

The patch itself looks good, but I believe that I might be confused
by the commit message. so a few questions here:

> OpenGL stack makes scanout buffers exported as well because usually
> it will be exported from client application to compositor.
> 
> Exported buffers needs to be placed in SMEM so it can be accessed
> by different GPUs if needed,

so, if it needs SMEM, why are you migrating to LMEM?

> so in discrete GPUs it is placed in
> SMEM+LMEM.

I see the code migrating to LMEM, but not migrating LMEM to SMEM,
what am I missing?

> 
> But this combination is causing aplications like kmscube to be able
> to create framebuffers.

to be able or to *not* be able? what's the real current issue?

> 
> So here doing the same handling as i915 and allowing bos that can
> migrate to lmem0/vram0 to be promoted to framebuffer.

okay, this phrase is why I believe the commit is doing what it should,
but the above messages confused me a bit....

> 
> At least all current discrete GPUs with display only have one lmem
> region, so we don't need to check for vram1 but we might need to
> extend it in future.

ack on this part.

> 
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fb.c | 5 +++--
>  drivers/gpu/drm/xe/display/xe_fb_pin.c  | 7 ++++++-
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
> index 015aacbad1b11..feb8a0f5fbdf6 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb.c
> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> @@ -2121,8 +2121,9 @@ intel_user_framebuffer_create(struct drm_device *dev,
>  		return ERR_PTR(-ENOENT);
>  
>  	obj = gem_to_xe_bo(gem);
> -	/* Require vram exclusive objects, but allow dma-buf imports */
> -	if (IS_DGFX(i915) && obj->flags & XE_BO_CREATE_SYSTEM_BIT &&
> +	/* Require vram placement or dma-buf import */
> +	if (IS_DGFX(i915) &&
> +	    !xe_bo_can_migrate(gem_to_xe_bo(gem), XE_PL_VRAM0) &&
>  	    obj->ttm.type != ttm_bo_type_sg) {
>  		drm_gem_object_put(gem);
>  		return ERR_PTR(-EREMOTE);
> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> index ed691d28b34d7..c7c4df18e439b 100644
> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> @@ -183,6 +183,8 @@ static int __xe_pin_fb_vma_ggtt(struct intel_framebuffer *fb,
>  static struct i915_vma *__xe_pin_fb_vma(struct intel_framebuffer *fb,
>  					const struct i915_gtt_view *view)
>  {
> +	struct drm_device *dev = fb->base.dev;
> +	struct xe_device *xe = to_xe_device(dev);
>  	struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL);
>  	struct xe_bo *bo = intel_fb_obj(&fb->base);
>  	int ret;
> @@ -204,7 +206,10 @@ static struct i915_vma *__xe_pin_fb_vma(struct intel_framebuffer *fb,
>  	if (ret)
>  		goto err;
>  
> -	ret = xe_bo_validate(bo, NULL, true);
> +	if (IS_DGFX(xe))
> +		ret = xe_bo_migrate(bo, XE_PL_VRAM0);
> +	else
> +		ret = xe_bo_validate(bo, NULL, true);
>  	if (!ret)
>  		ttm_bo_pin(&bo->ttm);
>  	ttm_bo_unreserve(&bo->ttm);
> -- 
> 2.40.1
>