Re: [PATCH v2 00/13] drm: Eliminate plane->fb/crtc usage for atomic drivers

[Sinclair Yeh]

Thanks Ville.

This series: Reviewed-by: Sinclair Yeh 

On Fri, May 25, 2018 at 09:50:32PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Here are again the last (?) bits of eliminating the plane->fb/crtc
> usage for atomic drivers. I've pushed everything else (thanks to
> everyone who reviewed them). 
> 
> Deepak said he'd tested the vmwgfx stuff, so I think it should be
> safe to land. Just missing a bit of review...
> 
> Cc: Alex Deucher 
> Cc: amd-gfx@lists.freedesktop.org
> Cc: "Christian König" 
> Cc: Daniel Vetter 
> Cc: David Airlie 
> Cc: "David (ChunMing) Zhou" 
> Cc: Deepak Rawat 
> Cc: Eric Anholt 
> Cc: freedr...@lists.freedesktop.org
> Cc: Gerd Hoffmann 
> Cc: Harry Wentland 
> Cc: Inki Dae 
> Cc: Joonyoung Shim 
> Cc: Kyungmin Park 
> Cc: linux-arm-...@vger.kernel.org
> Cc: Rob Clark 
> Cc: Seung-Woo Kim 
> Cc: Sinclair Yeh 
> Cc: Thomas Hellstrom 
> Cc: virtualizat...@lists.linux-foundation.org
> Cc: VMware Graphics 
> 
> Ville Syrjälä (13):
>   drm/vmwgfx: Stop using plane->fb in vmw_kms_atomic_check_modeset()
>   drm/vmwgfx: Stop using plane->fb in vmw_kms_helper_dirty()
>   drm/vmwgfx: Stop using plane->fb in vmw_kms_update_implicit_fb()
>   drm/vmwgfx: Stop updating plane->fb
>   drm/vmwgfx: Stop using plane->fb in atomic_enable()
>   drm/vmwgfx: Stop messing about with plane->fb/old_fb/crtc
>   drm/amdgpu/dc: Stop updating plane->fb
>   drm/i915: Stop updating plane->fb/crtc
>   drm/exynos: Stop updating plane->crtc
>   drm/msm: Stop updating plane->fb/crtc
>   drm/virtio: Stop updating plane->crtc
>   drm/vc4: Stop updating plane->fb/crtc
>   drm: Stop updating plane->crtc/fb/old_fb on atomic drivers
> 
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 -
>  drivers/gpu/drm/drm_atomic.c  | 55 
> +++
>  drivers/gpu/drm/drm_atomic_helper.c   | 15 +--
>  drivers/gpu/drm/drm_crtc.c|  8 +++-
>  drivers/gpu/drm/drm_fb_helper.c   |  7 ---
>  drivers/gpu/drm/drm_framebuffer.c |  5 ---
>  drivers/gpu/drm/drm_plane.c   | 14 +++---
>  drivers/gpu/drm/drm_plane_helper.c|  4 +-
>  drivers/gpu/drm/exynos/exynos_drm_plane.c |  2 -
>  drivers/gpu/drm/i915/intel_atomic_plane.c | 12 -
>  drivers/gpu/drm/i915/intel_display.c  |  7 ++-
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c |  1 -
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c|  2 -
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c |  1 -
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c|  2 -
>  drivers/gpu/drm/vc4/vc4_crtc.c|  3 --
>  drivers/gpu/drm/virtio/virtgpu_display.c  |  2 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_fb.c| 24 --
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   | 24 +++---
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c  |  2 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c  |  5 +--
>  include/drm/drm_atomic.h  |  3 --
>  22 files changed, 46 insertions(+), 154 deletions(-)
> 
> -- 
> 2.16.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/7] drm/ttm: use an operation ctx for ttm_mem_global_alloc

[Sinclair Yeh]

Hi,

Sorry, I've been on a leave.  Catching up on emails.

In case this is still not yet merged, my comment is
be consist with the new variable name.  In some cases
it's "ttm_opt_ctx", in others it's "ctx".

I prefer ttm_opt_ctx because it's more descriptive.

And yes, some description on why this change is necessary in the
commit message is nice, too.

Sinclair

On Wed, Dec 20, 2017 at 02:33:37PM +0100, Christian König wrote:
> Commit message needed! Something like:
> 
> Forward the operation context to ttm_mem_global_alloc as well.
> 
> Am 20.12.2017 um 11:34 schrieb Roger He:
> > Change-Id: I5279b5cd3560c4082b00f822219575a5f9c3808a
> > Signed-off-by: Roger He 
> 
> With the commit message fixed, patch is Reviewed-by: Christian König
> .
> 
> I would like to get an rb or ab from Thomas and/or Sinclair as well, since
> this is touching a lot of vmwgfx code.
> 
> Regards,
> Christian.
> 
> > ---
> >   drivers/gpu/drm/ttm/ttm_bo.c|  2 +-
> >   drivers/gpu/drm/ttm/ttm_memory.c| 15 +--
> >   drivers/gpu/drm/ttm/ttm_object.c| 13 ++---
> >   drivers/gpu/drm/vmwgfx/vmwgfx_binding.c |  6 +-
> >   drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c  | 13 ++---
> >   drivers/gpu/drm/vmwgfx/vmwgfx_context.c |  6 +-
> >   drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c |  6 +-
> >   drivers/gpu/drm/vmwgfx/vmwgfx_fence.c   |  6 +-
> >   drivers/gpu/drm/vmwgfx/vmwgfx_shader.c  | 18 +++---
> >   drivers/gpu/drm/vmwgfx/vmwgfx_simple_resource.c |  6 +-
> >   drivers/gpu/drm/vmwgfx/vmwgfx_so.c  |  6 +-
> >   drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 12 ++--
> >   include/drm/ttm/ttm_memory.h|  3 ++-
> >   13 files changed, 87 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> > index fa57aa8..c59f572 100644
> > --- a/drivers/gpu/drm/ttm/ttm_bo.c
> > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> > @@ -1133,7 +1133,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
> > struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
> > bool locked;
> > -   ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false);
> > +   ret = ttm_mem_global_alloc(mem_glob, acc_size, ctx);
> > if (ret) {
> > pr_err("Out of kernel memory\n");
> > if (destroy)
> > diff --git a/drivers/gpu/drm/ttm/ttm_memory.c 
> > b/drivers/gpu/drm/ttm/ttm_memory.c
> > index 9130bdf..525d3b6 100644
> > --- a/drivers/gpu/drm/ttm/ttm_memory.c
> > +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> > @@ -508,7 +508,7 @@ static int ttm_mem_global_reserve(struct ttm_mem_global 
> > *glob,
> >   static int ttm_mem_global_alloc_zone(struct ttm_mem_global *glob,
> >  struct ttm_mem_zone *single_zone,
> >  uint64_t memory,
> > -bool no_wait, bool interruptible)
> > +struct ttm_operation_ctx *ctx)
> >   {
> > int count = TTM_MEMORY_ALLOC_RETRIES;
> > @@ -516,7 +516,7 @@ static int ttm_mem_global_alloc_zone(struct 
> > ttm_mem_global *glob,
> >single_zone,
> >memory, true)
> > != 0)) {
> > -   if (no_wait)
> > +   if (ctx->no_wait_gpu)
> > return -ENOMEM;
> > if (unlikely(count-- == 0))
> > return -ENOMEM;
> > @@ -527,15 +527,14 @@ static int ttm_mem_global_alloc_zone(struct 
> > ttm_mem_global *glob,
> >   }
> >   int ttm_mem_global_alloc(struct ttm_mem_global *glob, uint64_t memory,
> > -bool no_wait, bool interruptible)
> > +struct ttm_operation_ctx *ctx)
> >   {
> > /**
> >  * Normal allocations of kernel memory are registered in
> >  * all zones.
> >  */
> > -   return ttm_mem_global_alloc_zone(glob, NULL, memory, no_wait,
> > -interruptible);
> > +   return ttm_mem_global_alloc_zone(glob, NULL, memory, ctx);
> >   }
> >   EXPORT_SYMBOL(ttm_mem_global_alloc);
> > @@ -544,6 +543,10 @@ int ttm_mem_global_alloc_page(struct ttm_mem_global 
> > *glob,
> >   {
> > struct ttm_mem_zone *zone = NULL;
> > +   struct ttm_operation_ctx ctx = {
> > +   .interruptible = false,
> > +   .no_wait_gpu = false
> > +   };
> > /**
> >  * Page allocations may be registed in a single zone
> > @@ -557,7 +560,7 @@ int ttm_mem_global_alloc_page(struct ttm_mem_global 
> > *glob,
> > if (glob->zone_dma32 && page_to_pfn(page) > 0x0010UL)
> > zone = glob->zone_kernel;
> >   #endif
> > -   return ttm_mem_global_alloc_zone(glob, zone, size, false, false);
> > +   return ttm_mem_global_alloc_zone(glob, zone, size, );
> >   }
> >   void 

Re: [PATCH 6/6] drm/ttm: revert "add optional LRU removal callback v2"

[Sinclair Yeh]

Pathces 1, 2, 5, 6:
Reviewed-by: Sinclair Yeh <s...@vmware.com>

On Fri, Jan 13, 2017 at 10:51:11AM +0100, Christian König wrote:
> From: Christian König <christian.koe...@amd.com>
> 
> Without the custom LRU management the callback is not used any more.
> 
> Signed-off-by: Christian König <christian.koe...@amd.com>
> ---
>  drivers/gpu/drm/ttm/ttm_bo.c| 12 +---
>  include/drm/ttm/ttm_bo_driver.h |  6 --
>  2 files changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 59fac2f..dfaeac4 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -186,12 +186,8 @@ EXPORT_SYMBOL(ttm_bo_add_to_lru);
>  
>  int ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>  {
> - struct ttm_bo_device *bdev = bo->bdev;
>   int put_count = 0;
>  
> - if (bdev->driver->lru_removal)
> - bdev->driver->lru_removal(bo);
> -
>   if (!list_empty(>swap)) {
>   list_del_init(>swap);
>   ++put_count;
> @@ -201,6 +197,11 @@ int ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
>   ++put_count;
>   }
>  
> + /*
> +  * TODO: Add a driver hook to delete from
> +  * driver-specific LRU's here.
> +  */
> +
>   return put_count;
>  }
>  
> @@ -234,9 +235,6 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo)
>  
>   lockdep_assert_held(>resv->lock.base);
>  
> - if (bdev->driver->lru_removal)
> - bdev->driver->lru_removal(bo);
> -
>   put_count = ttm_bo_del_from_lru(bo);
>   ttm_bo_list_ref_sub(bo, put_count, true);
>   ttm_bo_add_to_lru(bo);
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index a1235fa..4395db1 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -462,12 +462,6 @@ struct ttm_bo_driver {
> struct ttm_mem_reg *mem);
>   void (*io_mem_free)(struct ttm_bo_device *bdev,
>   struct ttm_mem_reg *mem);
> -
> - /**
> -  * Optional driver callback for when BO is removed from the LRU.
> -  * Called with LRU lock held immediately before the removal.
> -  */
> - void (*lru_removal)(struct ttm_buffer_object *bo);
>  };
>  
>  /**
> -- 
> 2.7.4
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/6] drm/ttm: revert "implement LRU add callbacks v2"

[Sinclair Yeh]

Minor typo

On Fri, Jan 13, 2017 at 10:51:10AM +0100, Christian König wrote:
> From: Christian König 
> 
> The additional housekeeping had to much CPU overhead,
   ^


> let's use the BO priorities instead.
> 
> Signed-off-by: Christian König 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  2 --
>  drivers/gpu/drm/ast/ast_ttm.c   |  2 --
>  drivers/gpu/drm/bochs/bochs_mm.c|  2 --
>  drivers/gpu/drm/cirrus/cirrus_ttm.c |  2 --
>  drivers/gpu/drm/mgag200/mgag200_ttm.c   |  2 --
>  drivers/gpu/drm/nouveau/nouveau_bo.c|  2 --
>  drivers/gpu/drm/qxl/qxl_ttm.c   |  2 --
>  drivers/gpu/drm/radeon/radeon_ttm.c |  2 --
>  drivers/gpu/drm/ttm/ttm_bo.c| 19 +--
>  drivers/gpu/drm/virtio/virtgpu_ttm.c|  2 --
>  drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c  |  2 --
>  include/drm/ttm/ttm_bo_driver.h |  9 -
>  12 files changed, 5 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index dd086d8..0402a11 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1086,8 +1086,6 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
>   .fault_reserve_notify = _bo_fault_reserve_notify,
>   .io_mem_reserve = _ttm_io_mem_reserve,
>   .io_mem_free = _ttm_io_mem_free,
> - .lru_tail = _bo_default_lru_tail,
> - .swap_lru_tail = _bo_default_swap_lru_tail,
>  };
>  
>  int amdgpu_ttm_init(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
> index 2a1368f..50c910e 100644
> --- a/drivers/gpu/drm/ast/ast_ttm.c
> +++ b/drivers/gpu/drm/ast/ast_ttm.c
> @@ -236,8 +236,6 @@ struct ttm_bo_driver ast_bo_driver = {
>   .verify_access = ast_bo_verify_access,
>   .io_mem_reserve = _ttm_io_mem_reserve,
>   .io_mem_free = _ttm_io_mem_free,
> - .lru_tail = _bo_default_lru_tail,
> - .swap_lru_tail = _bo_default_swap_lru_tail,
>  };
>  
>  int ast_mm_init(struct ast_private *ast)
> diff --git a/drivers/gpu/drm/bochs/bochs_mm.c 
> b/drivers/gpu/drm/bochs/bochs_mm.c
> index 099a3c6..e4c1125 100644
> --- a/drivers/gpu/drm/bochs/bochs_mm.c
> +++ b/drivers/gpu/drm/bochs/bochs_mm.c
> @@ -205,8 +205,6 @@ struct ttm_bo_driver bochs_bo_driver = {
>   .verify_access = bochs_bo_verify_access,
>   .io_mem_reserve = _ttm_io_mem_reserve,
>   .io_mem_free = _ttm_io_mem_free,
> - .lru_tail = _bo_default_lru_tail,
> - .swap_lru_tail = _bo_default_swap_lru_tail,
>  };
>  
>  int bochs_mm_init(struct bochs_device *bochs)
> diff --git a/drivers/gpu/drm/cirrus/cirrus_ttm.c 
> b/drivers/gpu/drm/cirrus/cirrus_ttm.c
> index d6da848..f53aa8f 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_ttm.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_ttm.c
> @@ -236,8 +236,6 @@ struct ttm_bo_driver cirrus_bo_driver = {
>   .verify_access = cirrus_bo_verify_access,
>   .io_mem_reserve = _ttm_io_mem_reserve,
>   .io_mem_free = _ttm_io_mem_free,
> - .lru_tail = _bo_default_lru_tail,
> - .swap_lru_tail = _bo_default_swap_lru_tail,
>  };
>  
>  int cirrus_mm_init(struct cirrus_device *cirrus)
> diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c 
> b/drivers/gpu/drm/mgag200/mgag200_ttm.c
> index 5e20220..657598b 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
> @@ -236,8 +236,6 @@ struct ttm_bo_driver mgag200_bo_driver = {
>   .verify_access = mgag200_bo_verify_access,
>   .io_mem_reserve = _ttm_io_mem_reserve,
>   .io_mem_free = _ttm_io_mem_free,
> - .lru_tail = _bo_default_lru_tail,
> - .swap_lru_tail = _bo_default_swap_lru_tail,
>  };
>  
>  int mgag200_mm_init(struct mga_device *mdev)
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
> b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 5b0a28b..3949a74 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -1570,8 +1570,6 @@ struct ttm_bo_driver nouveau_bo_driver = {
>   .fault_reserve_notify = _ttm_fault_reserve_notify,
>   .io_mem_reserve = _ttm_io_mem_reserve,
>   .io_mem_free = _ttm_io_mem_free,
> - .lru_tail = _bo_default_lru_tail,
> - .swap_lru_tail = _bo_default_swap_lru_tail,
>  };
>  
>  struct nvkm_vma *
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index f3939a9..2955f91 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -395,8 +395,6 @@ static struct ttm_bo_driver qxl_bo_driver = {
>   .io_mem_reserve = _ttm_io_mem_reserve,
>   .io_mem_free = _ttm_io_mem_free,
>   .move_notify = _bo_move_notify,
> - .lru_tail = _bo_default_lru_tail,
> - .swap_lru_tail = _bo_default_swap_lru_tail,
>  };
>  
>  int qxl_ttm_init(struct qxl_device *qdev)
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
>