[PATCH 2/2] drm/xe: Introduce GGTT documentation

Ghimiray, Himal Prasad himal.prasad.ghimiray at intel.com
Fri Jun 14 03:42:20 UTC 2024



On 14-06-2024 03:17, Rodrigo Vivi wrote:
> Document xe_ggtt and ensure it is part of the built kernel docs.
> 
> Cc: Matthew Brost <matthew.brost at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>   Documentation/gpu/xe/xe_mm.rst     | 15 +++++
>   drivers/gpu/drm/xe/xe_ggtt.c       | 99 ++++++++++++++++++++++++++++--
>   drivers/gpu/drm/xe/xe_ggtt_types.h | 26 ++++++--
>   3 files changed, 129 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/gpu/xe/xe_mm.rst b/Documentation/gpu/xe/xe_mm.rst
> index 6c8fd8b4a466..95864a4502dd 100644
> --- a/Documentation/gpu/xe/xe_mm.rst
> +++ b/Documentation/gpu/xe/xe_mm.rst
> @@ -7,6 +7,21 @@ Memory Management
>   .. kernel-doc:: drivers/gpu/drm/xe/xe_bo_doc.h
>      :doc: Buffer Objects (BO)
>   
> +GGTT
> +====
> +
> +.. kernel-doc:: drivers/gpu/drm/xe/xe_ggtt.c
> +   :doc: Global Graphics Translation Table (GGTT)
> +
> +GGTT Internal API
> +-----------------
> +
> +.. kernel-doc:: drivers/gpu/drm/xe/xe_ggtt_types.h
> +   :internal:
> +
> +.. kernel-doc:: drivers/gpu/drm/xe/xe_ggtt.c
> +   :internal:
> +
>   Pagetable building
>   ==================
>   
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 2d5e802f862d..950af831113d 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -27,6 +27,19 @@
>   #include "xe_sriov.h"
>   #include "xe_wopcm.h"
>   
> +/**
> + * DOC: Global Graphics Translation Table (GGTT)
> + *
> + * Xe GGTT implements the support for a Global Virtual Address space that is used
> + * for resources that are accessible to privileged (i.e. kernel-mode) processes, and
> + * not tied to a specific user-level process. For example, the Graphics
> + * micro-Controller (GuC) and Display Engine (if present) utilize this Global
> + * address space.
> + *
> + * The Global GTT (GGTT) translates from the Global virtual address to a physical
> + * address that can be accessed by HW. The GGTT is a flat, single-level table.
> + */
> +
>   static u64 xelp_ggtt_pte_encode_bo(struct xe_bo *bo, u64 bo_offset,
>   				   u16 pat_index)
>   {
> @@ -69,6 +82,12 @@ static unsigned int probe_gsm_size(struct pci_dev *pdev)
>   	return ggms ? SZ_1M << ggms : 0;
>   }
>   
> +/**
> + * xe_ggtt_set_pte - Allow directly writing to a Page Table Entry (PTE)
> + * @ggtt: the &xe_ggtt with the PTE to be set
> + * @addr: address of the GSM
> + * @pte: value to be set directly to the PTE
> + */
>   void xe_ggtt_set_pte(struct xe_ggtt *ggtt, u64 addr, u64 pte)
>   {
>   	xe_tile_assert(ggtt->tile, !(addr & XE_PTE_MASK));
> @@ -130,12 +149,16 @@ static const struct xe_ggtt_pt_ops xelpg_pt_ops = {
>   	.pte_encode_bo = xelpg_ggtt_pte_encode_bo,
>   };
>   
> -/*
> - * Early GGTT initialization, which allows to create new mappings usable by the
> - * GuC.
> - * Mappings are not usable by the HW engines, as it doesn't have scratch /
> +/**
> + * xe_ggtt_init_early - Early GGTT initialization
> + * @ggtt: the &xe_ggtt to be initialized
> + *
> + * It allows to create new mappings usable by the GuC.
> + * Mappings are not usable by the HW engines, as it doesn't have scratch nor
>    * initial clear done to it yet. That will happen in the regular, non-early
> - * GGTT init.
> + * GGTT initialization.
> + *
> + * Return: 0 on success or a negative error code on failure.
>    */
>   int xe_ggtt_init_early(struct xe_ggtt *ggtt)
>   {
> @@ -225,6 +248,12 @@ static void xe_ggtt_initial_clear(struct xe_ggtt *ggtt)
>   	mutex_unlock(&ggtt->lock);
>   }
>   
> +/**
> + * xe_ggtt_init - Regular non-early GGTT initialization
> + * @ggtt: the &xe_ggtt to be initialized
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
>   int xe_ggtt_init(struct xe_ggtt *ggtt)
>   {
>   	struct xe_device *xe = tile_to_xe(ggtt->tile);
> @@ -345,6 +374,18 @@ void xe_ggtt_deballoon(struct xe_ggtt *ggtt, struct drm_mm_node *node)
>   	mutex_unlock(&ggtt->lock);
>   }
>   
> +/**
> + * xe_ggtt_insert_special_node_locked - Locked version to insert a &drm_mm_node into the GGTT
> + * @ggtt: the &xe_ggtt where node will be inserted
> + * @node: the &drm_mm_node to be inserted
> + * @size: size of the node
> + * @align: alignment constrain of the node
> + * @mm_flags: flags to control the node behavior
> + *
> + * To be used in cases where ggtt->lock is already taken.


How about using lockdep_assert_held to ensure we get warning if used 
without locking ?

This patch appears good to me in its own.
Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>


> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
>   int xe_ggtt_insert_special_node_locked(struct xe_ggtt *ggtt, struct drm_mm_node *node,
>   				       u32 size, u32 align, u32 mm_flags)
>   {
> @@ -352,6 +393,15 @@ int xe_ggtt_insert_special_node_locked(struct xe_ggtt *ggtt, struct drm_mm_node
>   					  mm_flags);
>   }
>   
> +/**
> + * xe_ggtt_insert_special_node - Insert a &drm_mm_node into the GGTT
> + * @ggtt: the &xe_ggtt where node will be inserted
> + * @node: the &drm_mm_node to be inserted
> + * @size: size of the node
> + * @align: alignment constrain of the node
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
>   int xe_ggtt_insert_special_node(struct xe_ggtt *ggtt, struct drm_mm_node *node,
>   				u32 size, u32 align)
>   {
> @@ -365,6 +415,11 @@ int xe_ggtt_insert_special_node(struct xe_ggtt *ggtt, struct drm_mm_node *node,
>   	return ret;
>   }
>   
> +/**
> + * xe_ggtt_map_bo - Map the BO into GGTT
> + * @ggtt: the &xe_ggtt where node will be mapped
> + * @bo: the &xe_bo to be mapped
> + */
>   void xe_ggtt_map_bo(struct xe_ggtt *ggtt, struct xe_bo *bo)
>   {
>   	u16 cache_mode = bo->flags & XE_BO_FLAG_NEEDS_UC ? XE_CACHE_NONE : XE_CACHE_WB;
> @@ -412,17 +467,39 @@ static int __xe_ggtt_insert_bo_at(struct xe_ggtt *ggtt, struct xe_bo *bo,
>   	return err;
>   }
>   
> +/**
> + * xe_ggtt_insert_bo_at - Insert BO at a specific GGTT space
> + * @ggtt: the &xe_ggtt where bo will be inserted
> + * @bo: the &xe_bo to be inserted
> + * @start: address where it will be inserted
> + * @end: end of the range where it will be inserted
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
>   int xe_ggtt_insert_bo_at(struct xe_ggtt *ggtt, struct xe_bo *bo,
>   			 u64 start, u64 end)
>   {
>   	return __xe_ggtt_insert_bo_at(ggtt, bo, start, end);
>   }
>   
> +/**
> + * xe_ggtt_insert_bo - Insert BO into GGTT
> + * @ggtt: the &xe_ggtt where bo will be inserted
> + * @bo: the &xe_bo to be inserted
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
>   int xe_ggtt_insert_bo(struct xe_ggtt *ggtt, struct xe_bo *bo)
>   {
>   	return __xe_ggtt_insert_bo_at(ggtt, bo, 0, U64_MAX);
>   }
>   
> +/**
> + * xe_ggtt_remove_node - Remove a &drm_mm_node from the GGTT
> + * @ggtt: the &xe_ggtt where node will be removed
> + * @node: the &drm_mm_node to be removed
> + * @invalidate: if node needs invalidation upon removal
> + */
>   void xe_ggtt_remove_node(struct xe_ggtt *ggtt, struct drm_mm_node *node,
>   			 bool invalidate)
>   {
> @@ -451,6 +528,11 @@ void xe_ggtt_remove_node(struct xe_ggtt *ggtt, struct drm_mm_node *node,
>   	drm_dev_exit(idx);
>   }
>   
> +/**
> + * xe_ggtt_remove_bo - Remove a BO from the GGTT
> + * @ggtt: the &xe_ggtt where node will be removed
> + * @bo: the &xe_bo to be removed
> + */
>   void xe_ggtt_remove_bo(struct xe_ggtt *ggtt, struct xe_bo *bo)
>   {
>   	if (XE_WARN_ON(!bo->ggtt_node.size))
> @@ -507,6 +589,13 @@ void xe_ggtt_assign(struct xe_ggtt *ggtt, const struct drm_mm_node *node, u16 vf
>   }
>   #endif
>   
> +/**
> + * xe_ggtt_dump - Dump GGTT for debug
> + * @ggtt: the &xe_ggtt to be dumped
> + * @p: the &drm_mm_printer helper handle to be used to dump the information
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
>   int xe_ggtt_dump(struct xe_ggtt *ggtt, struct drm_printer *p)
>   {
>   	int err;
> diff --git a/drivers/gpu/drm/xe/xe_ggtt_types.h b/drivers/gpu/drm/xe/xe_ggtt_types.h
> index d8c584d9a8c3..5718726e8b3a 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt_types.h
> +++ b/drivers/gpu/drm/xe/xe_ggtt_types.h
> @@ -13,26 +13,40 @@
>   struct xe_bo;
>   struct xe_gt;
>   
> +/**
> + * struct xe_ggtt_pt_ops - GGTT Page table operations
> + * Which can vary from platform to platform.
> + */
>   struct xe_ggtt_pt_ops {
> +	/** @pte_encode_bo: Encode PTE address for a given BO */
>   	u64 (*pte_encode_bo)(struct xe_bo *bo, u64 bo_offset, u16 pat_index);
>   };
>   
> +/**
> + * struct xe_ggtt - Main GGTT struct
> + * In general, each tile can contains its own GGTT instance.
> + *
> + * The @flags can be:
> + * - %XE_GGTT_FLAGS_64K - if PTE size is 64K. Otherwise, regular is 4K.
> + */
>   struct xe_ggtt {
> +	/** @tile: Back pointer to tile where this GGTT belongs */
>   	struct xe_tile *tile;
> -
> +	/** @size: Total size of this GGTT */
>   	u64 size;
>   
>   #define XE_GGTT_FLAGS_64K BIT(0)
> +	/** @flags: Flags for this GGTT */
>   	unsigned int flags;
> -
> +	/** @scratch: A BO as scratch page */
>   	struct xe_bo *scratch;
> -
> +	/** @lock: Mutex lock to protect GGTT data */
>   	struct mutex lock;
> -
> +	/** @gsm: The iomem pointer for easy PTE manipulation */
>   	u64 __iomem *gsm;
> -
> +	/** @pt_ops: Page Table operations per platform */
>   	const struct xe_ggtt_pt_ops *pt_ops;
> -
> +	/** @mm: The &drm_mm struct */
>   	struct drm_mm mm;
>   };
>   


More information about the Intel-xe mailing list