[igt-dev] [i-g-t] lib/i915: Use FIXED mapping only for discrete memory

[Petri Latvala]

On Mon, Aug 30, 2021 at 12:12:25PM +0300, Surendrakumar Upadhyay, TejaskumarX wrote:
> 
> 
> > -----Original Message-----
> > From: Latvala, Petri <petri.latvala at intel.com>
> > Sent: 30 August 2021 14:07
> > To: Surendrakumar Upadhyay, TejaskumarX
> > <tejaskumarx.surendrakumar.upadhyay at intel.com>
> > Cc: igt-dev at lists.freedesktop.org
> > Subject: Re: [igt-dev] [i-g-t] lib/i915: Use FIXED mapping only for discrete
> > memory
> >
> > On Fri, Aug 27, 2021 at 06:30:02PM +0530, Tejas Upadhyay wrote:
> > > The FIXED mapping is only used for discrete, and mapping type is
> > > pre-defined. This disables the other type of mmap offsets when
> > > discrete memory is used.
> > >
> > > Taken from kernel commit:
> > > commit 7961c5b60f23 ("drm/i915: Add TTM offset argument to mmap.")
> > >
> > > Signed-off-by: Tejas Upadhyay
> > > <tejaskumarx.surendrakumar.upadhyay at intel.com>
> > > ---
> > >  include/drm-uapi/i915_drm.h | 1 +
> > >  lib/i915/gem_mman.c         | 8 +++++++-
> > >  2 files changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
> > > index a1c0030c..b46367f2 100644
> > > --- a/include/drm-uapi/i915_drm.h
> > > +++ b/include/drm-uapi/i915_drm.h
> > > @@ -871,6 +871,7 @@ struct drm_i915_gem_mmap_offset {  #define
> > > I915_MMAP_OFFSET_WC  1  #define I915_MMAP_OFFSET_WB  2  #define
> > > I915_MMAP_OFFSET_UC  3
> > > +#define I915_MMAP_OFFSET_FIXED 4
> > >
> > >     /*
> > >      * Zero-terminated chain of extensions.
> > > diff --git a/lib/i915/gem_mman.c b/lib/i915/gem_mman.c index
> > > 0406a0b9..1917c24d 100644
> > > --- a/lib/i915/gem_mman.c
> > > +++ b/lib/i915/gem_mman.c
> > > @@ -75,7 +75,13 @@ bool gem_has_legacy_mmap(int fd)
> > >
> > >  bool gem_has_mmap_offset_type(int fd, const struct mmap_offset *t)  {
> > > -   return gem_has_mmap_offset(fd) || t->type ==
> > I915_MMAP_OFFSET_GTT;
> > > +   if (gem_has_mmap_offset(fd))
> > > +           if (!gem_has_lmem(fd))
> > > +                   return !(t->type == I915_MMAP_OFFSET_FIXED);
> > > +           else
> > > +                   return !(t->type != I915_MMAP_OFFSET_FIXED);
> >
> > Why the !(a == b) stuff?
> >        ^^^
> 
> If its discrete memory and offset type is fixed then only its valid combination.  Reverse if not discrete then any offset type except fixed is valid combination.


I understand the goal, but this is imho more readable:

if (gem_has_lmem(fd))
  return t->type == I915_MMAP_OFFSET_FIXED;
else
  return t->type != I915_MMAP_OFFSET_FIXED;



-- 
Petri Latvala