On Fri, Jan 22, 2021 at 4:40 PM Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx> wrote: > > The guidance for i915 at this time is to start phasing out pread/pwrite > ioctl's, the rationale being (a) the functionality can be done entirely in > userspace with a combination of mmap + memcpy, and (b) no existing user > mode clients actually use the pread/pwrite interface. > > In this patch we disable these ioctls for dGfx platforms with the > expectation that this will be done for all future platforms (both discrete > and integrated). IGT changes which handle this kernel change have also been > submitted [1]. > > [1] https://patchwork.freedesktop.org/series/81384/ > > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem.c | 6 ++++++ > drivers/gpu/drm/i915/i915_pci.c | 3 ++- > drivers/gpu/drm/i915/intel_device_info.h | 1 + > 4 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 7efb501e22d2..66edffadf4a0 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1674,6 +1674,7 @@ tgl_stepping_get(struct drm_i915_private *dev_priv) > #define HAS_SECURE_BATCHES(dev_priv) (INTEL_GEN(dev_priv) < 6) > #define HAS_WT(dev_priv) HAS_EDRAM(dev_priv) > > +#define HAS_PREAD_PWRITE(dev_priv) (!INTEL_INFO(dev_priv)->no_pread_pwrite) > #define HWS_NEEDS_PHYSICAL(dev_priv) (INTEL_INFO(dev_priv)->hws_needs_physical) > > #define HAS_LOGICAL_RING_CONTEXTS(dev_priv) \ > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 9b04dff5eb32..9f3ef68fadf3 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -382,6 +382,9 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, > struct drm_i915_gem_object *obj; > int ret; > > + if (!HAS_PREAD_PWRITE(to_i915(dev))) > + return -EOPNOTSUPP; > + > if (args->size == 0) > return 0; > > @@ -687,6 +690,9 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, > struct drm_i915_gem_object *obj; > int ret; > > + if (!HAS_PREAD_PWRITE(to_i915(dev))) > + return -EOPNOTSUPP; > + > if (args->size == 0) > return 0; > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index 6cff7cf0f17b..3231d989275c 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -910,7 +910,8 @@ static const struct intel_device_info rkl_info = { > .has_master_unit_irq = 1, \ > .has_llc = 0, \ > .has_snoop = 1, \ > - .is_dgfx = 1 > + .is_dgfx = 1, \ > + .no_pread_pwrite = 1 > > static const struct intel_device_info dg1_info __maybe_unused = { > GEN12_DGFX_FEATURES, > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h > index e6ca1023ffcf..8edd2cfb0bab 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.h > +++ b/drivers/gpu/drm/i915/intel_device_info.h > @@ -114,6 +114,7 @@ enum intel_ppgtt_type { > func(is_lp); \ > func(require_force_probe); \ > func(is_dgfx); \ > + func(no_pread_pwrite); \ Is it really better to add a device_info flag here or would we be better off making it dependent on HW generation? This is what I did for relocs: /* Relocations are disallowed for all platforms after TGL-LP */ if (INTEL_GEN(eb->i915) >= 12 && !IS_TIGERLAKE(eb->i915)) return -EINVAL; /* All discrete memory platforms are Gen12 or above */ BUG_ON(HAS_LMEM(eb->i915)); At least to me, that makes the code more obvious. --Jason _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
