Re: [PATCH v4 3/6] drm/xe: Add xe_ggtt_alloc

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 09.10.2024 14:51, Maarten Lankhorst wrote:
> Instead of allocating inside xe_tile, create a new function that returns
> an allocated struct xe_ggtt from xe_ggtt.c
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_ggtt.c | 16 ++++++++++++++++
>  drivers/gpu/drm/xe/xe_ggtt.h |  2 ++
>  drivers/gpu/drm/xe/xe_tile.c |  4 +---
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 16be6f53b75b2..c9fa9d15b69de 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -160,6 +160,22 @@ static void xe_ggtt_clear(struct xe_ggtt *ggtt, u64 start, u64 size)
>  	}
>  }
>  
> +/**
> + * xe_ggtt_alloc - Allocate a GGTT for a given &xe_tile

nit: this comment is little confusing, as you're allocating the xe_ggtt
object that represents the GGTT, not the GGTT itself

> + * @tile: &xe_tile
> + *
> + * Allocates a &xe_ggtt for a given tile.

that sounds better

> + *
> + * Return: &xe_ggtt on success, or NULL when out of memory.

Return: pointer to the &xe_ggtt on success, or NULL when out of memory.

> + */
> +struct xe_ggtt *xe_ggtt_alloc(struct xe_tile *tile)
> +{
> +	struct xe_ggtt *ggtt = drmm_kzalloc(&tile_to_xe(tile)->drm, sizeof(*ggtt), GFP_KERNEL);

"Missing a blank line after declarations

> +	if (ggtt)
> +		ggtt->tile = tile;
> +	return ggtt;
> +}
> +
>  static void ggtt_fini_early(struct drm_device *drm, void *arg)
>  {
>  	struct xe_ggtt *ggtt = arg;
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.h b/drivers/gpu/drm/xe/xe_ggtt.h
> index 62c8ce636939a..0bab1fd7cc817 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.h
> +++ b/drivers/gpu/drm/xe/xe_ggtt.h
> @@ -9,7 +9,9 @@
>  #include "xe_ggtt_types.h"
>  
>  struct drm_printer;
> +struct xe_tile;
>  
> +struct xe_ggtt *xe_ggtt_alloc(struct xe_tile *tile);
>  int xe_ggtt_init_early(struct xe_ggtt *ggtt);
>  int xe_ggtt_init(struct xe_ggtt *ggtt);
>  
> diff --git a/drivers/gpu/drm/xe/xe_tile.c b/drivers/gpu/drm/xe/xe_tile.c
> index 07cf7cfe4abd5..19814bfbab423 100644
> --- a/drivers/gpu/drm/xe/xe_tile.c
> +++ b/drivers/gpu/drm/xe/xe_tile.c
> @@ -88,11 +88,9 @@ static int xe_tile_alloc(struct xe_tile *tile)
>  {
>  	struct drm_device *drm = &tile_to_xe(tile)->drm;
>  
> -	tile->mem.ggtt = drmm_kzalloc(drm, sizeof(*tile->mem.ggtt),
> -				      GFP_KERNEL);
> +	tile->mem.ggtt = xe_ggtt_alloc(tile);
>  	if (!tile->mem.ggtt)
>  		return -ENOMEM;
> -	tile->mem.ggtt->tile = tile;
>  
>  	tile->mem.vram_mgr = drmm_kzalloc(drm, sizeof(*tile->mem.vram_mgr), GFP_KERNEL);
>  	if (!tile->mem.vram_mgr)