On Wed, Jul 30, 2025 at 06:30:29PM +0530, Himal Prasad Ghimiray wrote:
> - DRM_GPUVM_SM_MAP_OPS_FLAG_SPLIT_MADVISE: This flag is used by
> drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the
> user-provided range and split the existing non-GEM object VMA if the
> start or end of the input range lies within it. The operations can
> create up to 2 REMAPS and 2 MAPs. The purpose of this operation is to be
> used by the Xe driver to assign attributes to GPUVMA's within the
> user-defined range. Unlike drm_gpuvm_sm_map_ops_flags in default mode,
> the operation with this flag will never have UNMAPs and
> merges, and can be without any final operations.
>
> v2
> - use drm_gpuvm_sm_map_ops_create with flags instead of defining new
> ops_create (Danilo)
> - Add doc (Danilo)
>
> v3
> - Fix doc
> - Fix unmapping check
>
> v4
> - Fix mapping for non madvise ops
>
> v5
> - Fix mapping (Matthew Brost)
> - Rebase on top of struct changes
>
> Cc: Danilo Krummrich <d...@redhat.com>
> Cc: Matthew Brost <matthew.br...@intel.com>
I think this patch looks good to me, but will need a rebase based on
discussions in patch #1 of this series.
Going to hold off on the RB until the next rev.
Matt
> Cc: Boris Brezillon <bbrezil...@kernel.org>
> Cc: <dri-devel@lists.freedesktop.org>
> Signed-off-by: Himal Prasad Ghimiray<himal.prasad.ghimi...@intel.com>
> ---
> drivers/gpu/drm/drm_gpuvm.c | 87 +++++++++++++++++++++++++++++++------
> include/drm/drm_gpuvm.h | 11 ++++-
> 2 files changed, 83 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
> index f04d80a3a63b..2aeae8c2296f 100644
> --- a/drivers/gpu/drm/drm_gpuvm.c
> +++ b/drivers/gpu/drm/drm_gpuvm.c
> @@ -2110,6 +2110,8 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
> {
> struct drm_gpuva *va, *next;
> u64 req_end = req->va.addr + req->va.range;
> + bool is_madvise_ops = (req->flags &
> DRM_GPUVM_SM_MAP_OPS_FLAG_SPLIT_MADVISE);
> + bool needs_map = !is_madvise_ops;
> int ret;
>
> if (unlikely(!drm_gpuvm_range_valid(gpuvm, req->va.addr,
> req->va.range)))
> @@ -2122,26 +2124,35 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
> u64 range = va->va.range;
> u64 end = addr + range;
> bool merge = !!va->gem.obj;
> + bool skip_madvise_ops = is_madvise_ops && merge;
>
> + needs_map = !is_madvise_ops;
> if (addr == req->va.addr) {
> merge &= obj == req->gem.obj &&
> offset == req->gem.offset;
>
> if (end == req_end) {
> - ret = op_unmap_cb(ops, priv, va, merge);
> - if (ret)
> - return ret;
> + if (!is_madvise_ops) {
> + ret = op_unmap_cb(ops, priv, va, merge);
> + if (ret)
> + return ret;
> + }
> break;
> }
>
> if (end < req_end) {
> - ret = op_unmap_cb(ops, priv, va, merge);
> - if (ret)
> - return ret;
> + if (!is_madvise_ops) {
> + ret = op_unmap_cb(ops, priv, va, merge);
> + if (ret)
> + return ret;
> + }
> continue;
> }
>
> if (end > req_end) {
> + if (skip_madvise_ops)
> + break;
> +
> struct drm_gpuva_op_map n = {
> .va.addr = req_end,
> .va.range = range - req->va.range,
> @@ -2156,6 +2167,9 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
> ret = op_remap_cb(ops, priv, NULL, &n, &u);
> if (ret)
> return ret;
> +
> + if (is_madvise_ops)
> + needs_map = true;
> break;
> }
> } else if (addr < req->va.addr) {
> @@ -2173,20 +2187,45 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
> u.keep = merge;
>
> if (end == req_end) {
> + if (skip_madvise_ops)
> + break;
> +
> ret = op_remap_cb(ops, priv, &p, NULL, &u);
> if (ret)
> return ret;
> +
> + if (is_madvise_ops)
> + needs_map = true;
> +
> break;
> }
>
> if (end < req_end) {
> + if (skip_madvise_ops)
> + continue;
> +
> ret = op_remap_cb(ops, priv, &p, NULL, &u);
> if (ret)
> return ret;
> +
> + if (is_madvise_ops) {
> + struct drm_gpuva_op_map map_req = {
> + .va.addr = req->va.addr,
> + .va.range = end - req->va.addr,
> + };
> +
> + ret = op_map_cb(ops, priv, &map_req);
> + if (ret)
> + return ret;
> + }
> +
> continue;
> }
>
> if (end > req_end) {
> + if (skip_madvise_ops)
> + break;
> +
> struct drm_gpuva_op_map n = {
> .va.addr = req_end,
> .va.range = end - req_end,
> @@ -2198,6 +2237,9 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
> ret = op_remap_cb(ops, priv, &p, &n, &u);
> if (ret)
> return ret;
> +
> + if (is_madvise_ops)
> + needs_map = true;
> break;
> }
> } else if (addr > req->va.addr) {
> @@ -2206,20 +2248,29 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
> (addr - req->va.addr);
>
> if (end == req_end) {
> - ret = op_unmap_cb(ops, priv, va, merge);
> - if (ret)
> - return ret;
> + if (!is_madvise_ops) {
> + ret = op_unmap_cb(ops, priv, va, merge);
> + if (ret)
> + return ret;
> + }
> +
> break;
> }
>
> if (end < req_end) {
> - ret = op_unmap_cb(ops, priv, va, merge);
> - if (ret)
> - return ret;
> + if (!is_madvise_ops) {
> + ret = op_unmap_cb(ops, priv, va, merge);
> + if (ret)
> + return ret;
> + }
> +
> continue;
> }
>
> if (end > req_end) {
> + if (skip_madvise_ops)
> + break;
> +
> struct drm_gpuva_op_map n = {
> .va.addr = req_end,
> .va.range = end - req_end,
> @@ -2234,12 +2285,20 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
> ret = op_remap_cb(ops, priv, NULL, &n, &u);
> if (ret)
> return ret;
> +
> + if (is_madvise_ops) {
> + struct drm_gpuva_op_map map_req = {
> + .va.addr = addr,
> + .va.range = req_end - addr,
> + };
> +
> + return op_map_cb(ops, priv, &map_req);
> + }
> break;
> }
> }
> }
> -
> - return op_map_cb(ops, priv, req);
> + return needs_map ? op_map_cb(ops, priv, req) : 0;
> }
>
> static int
> diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
> index 75c616fdc119..a8e9f70501ef 100644
> --- a/include/drm/drm_gpuvm.h
> +++ b/include/drm/drm_gpuvm.h
> @@ -811,10 +811,19 @@ enum drm_gpuva_op_type {
> };
>
> /** DOC: flags for struct drm_gpuva_op_map
> - * %DRM_GPUVM_SM_MAP_OPS_FLAG_NONE DEFAULT split and merge,
> + * %DRM_GPUVM_SM_MAP_OPS_FLAG_NONE: DEFAULT split and merge,
> * It cannot be combined with other flags.
> + *
> + * %DRM_GPUVM_SM_MAP_OPS_FLAG_SPLIT_MADVISE: This flag is used by
> + * drm_gpuvm_sm_map_ops_create to iterate over GPUVMA's in the user-provided
> + * range and split the existing non-GEM object VMA if the start or end of
> + * the input range lies within it. The operations can create up to 2 REMAPS
> + * and 2 MAPs. Unlike DRM_GPUVM_SM_MAP_OPS_FLAG_NONE flag, the operation
> with
> + * this flag will never have UNMAPs and merges, and can be without any final
> + * operations.
> */
> #define DRM_GPUVM_SM_MAP_OPS_FLAG_NONE 0
> +#define DRM_GPUVM_SM_MAP_OPS_FLAG_SPLIT_MADVISE BIT(0)
>
> /**
> * struct drm_gpuva_op_map - GPU VA map operation
> --
> 2.34.1
>
