Dave Airlie skrev:
> From: Dave Airlie <airl...@redhat.com>
>
> This adds color tiling support for buffers in VRAM, it enables
> a color tiled fbcon and a color tiled X frontbuffer.
>
> It changes the API:
> adds two new parameters to the object creation API (is this better than
>  a set/get tiling?) we probably still need a get but not sure for what yet.
> relocs are required for 2D DST_PITCH_OFFSET and SRC_PITCH_OFFSET type-0,
> and 3D COLORPITCH registers.
>
> TTM:
> adds a new check_tiling call to TTM, gets called at fault and around
> bo moves.
>
>   
Dave,
Since I'm working on a driver with driver-private GPU maps, I wonder 
whether we could use some more generic callbacks here. Also we should 
protect them with a NULL check.

> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index c1c407f..e440b8b 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -304,6 +304,9 @@ static int ttm_bo_handle_move_mem(struct 
> ttm_buffer_object *bo,
>  
>       }
>  
> +     /* drop tiling around bo move */
> +     bdev->driver->check_tiling(bo, 0, 1);
> +
>       if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
>           !(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
>               ret = ttm_bo_move_ttm(bo, evict, no_wait, mem);
>   

What about naming the callback
bdev::driver::move_notify(bo, mem);
The driver can then compare bo::mem and mem to see any action is needed. 
In my case it would take down the driver-private GPU maps. In your case 
it would take down tiling.
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index 27b146c..c10d96c 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -154,6 +154,7 @@ static int ttm_bo_vm_fault(struct vm_area_struct *vma, 
> struct vm_fault *vmf)
>        */
>  
>       if (is_iomem) {
> +             bdev->driver->check_tiling(bo, 0, 0);
>               vma->vm_page_prot = ttm_io_prot(bo->mem.placement,
>                                               vma->vm_page_prot);
>       } else {
> diff --git a/include/drm/radeon_drm.h b/include/drm/radeon_drm.h
> index 41862e9..1ed43c7 100644
>
>   
A driver may want to move a buffer in the fault handler or set up, for 
example, a VRAM aperture to simulate that the buffer is always mappable. 
That would happen just after bo::reserve. Could we move this callback to 
just after bo::reserve and call it bo_driver::fault_notify_reserved or 
something like that?

One important to remember with callbacks from the fault handler is that 
they will be subject to the infamous mmap_sem locking inversion, So if 
there are any driver-private locks within check_tiling their locking 
order will be _after_ mmap sem, and _after_ bo::reserve.

> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -353,6 +353,14 @@ struct ttm_bo_driver {
>       int (*sync_obj_flush) (void *sync_obj, void *sync_arg);
>       void (*sync_obj_unref) (void **sync_obj);
>       void *(*sync_obj_ref) (void *sync_obj);
> +
> +     /* hook to check tiling registers are correct
> +      * @bo : Pointer to a buffer object
> +      * @has_moved : buffer has just been moved
> +      * @force_drop: drop the surface
> +      */
> +     int (*check_tiling)(struct ttm_buffer_object *bo,
> +                         bool has_moved, bool force_drop);
>  };
>  
>  #define TTM_NUM_MEM_TYPES 8
>   
Thanks,
Thomas


------------------------------------------------------------------------------
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to