On Thu, 07 Sep 2023, Donald Robson <Donald.Robson@xxxxxxxxxx> wrote: > On Thu, 2023-09-07 at 15:14 +0300, Jani Nikula wrote: >> On Wed, 06 Sep 2023, Sarah Walker <sarah.walker@xxxxxxxxxx> wrote: >> > From: Donald Robson <donald.robson@xxxxxxxxxx> >> > >> > Signed-off-by: Donald Robson <donald.robson@xxxxxxxxxx> >> > --- >> > include/drm/drm_gpuva_mgr.h | 27 +++++++++++++++++++++++++++ >> > 1 file changed, 27 insertions(+) >> > >> > diff --git a/include/drm/drm_gpuva_mgr.h b/include/drm/drm_gpuva_mgr.h >> > index ed8d50200cc3..be7b3a6d7e67 100644 >> > --- a/include/drm/drm_gpuva_mgr.h >> > +++ b/include/drm/drm_gpuva_mgr.h >> > @@ -703,4 +703,31 @@ void drm_gpuva_remap(struct drm_gpuva *prev, >> > >> > void drm_gpuva_unmap(struct drm_gpuva_op_unmap *op); >> > >> > +/** >> > + * drm_gpuva_op_remap_get_unmap_range() - Helper to get the start and range of >> > + * the unmap stage of a remap op. >> > + * @op: Remap op. >> > + * @start_addr: Output pointer for the start of the required unmap. >> > + * @range: Output pointer for the length of the required unmap. >> > + * >> > + * These parameters can then be used by the caller to unmap memory pages that >> > + * are no longer required. >> > + */ >> > +static __always_inline void >> >> IMO __always_inline *always* requires a justification in the commit >> message. >> >> BR, >> Jani. > > Hi Jani, > I went with __always_inline because I can't see this being used more than once per driver. > I can add that to the commit message, but is that suitable justification? I could move > it to the source file or make it a macro if you prefer. My personal opinion is that static inlines in general should always have a performance justification. If there isn't one, it should be a regular function. Static inlines leak the abstractions and often make the header dependencies worse. Not everyone agrees, of course. More than anything I was looking for justification on __always_inline rather than just inline, though. BR, Jani. > Thanks, > Donald >> >> >> > +drm_gpuva_op_remap_get_unmap_range(const struct drm_gpuva_op_remap *op, >> > + u64 *start_addr, u64 *range) >> > +{ >> > + const u64 va_start = op->prev ? >> > + op->prev->va.addr + op->prev->va.range : >> > + op->unmap->va->va.addr; >> > + const u64 va_end = op->next ? >> > + op->next->va.addr : >> > + op->unmap->va->va.addr + op->unmap->va->va.range; >> > + >> > + if (start_addr) >> > + *start_addr = va_start; >> > + if (range) >> > + *range = va_end - va_start; >> > +} >> > + >> > #endif /* __DRM_GPUVA_MGR_H__ */ -- Jani Nikula, Intel Open Source Graphics Center