[Intel-gfx] [PATCH 06/21] drm/i915: introduce page_size members

[Chris Wilson]

Quoting Chris Wilson (2017-10-05 17:12:25)
> Quoting Matthew Auld (2017-10-05 16:19:00)
> >  static inline unsigned int i915_sg_segment_size(void)
> >  {
> >         unsigned int size = swiotlb_max_segment();
> > @@ -3101,6 +3116,8 @@ intel_info(const struct drm_i915_private *dev_priv)
> >  #define USES_PPGTT(dev_priv)           (i915_modparams.enable_ppgtt)
> >  #define USES_FULL_PPGTT(dev_priv)      (i915_modparams.enable_ppgtt >= 2)
> >  #define USES_FULL_48BIT_PPGTT(dev_priv)        (i915_modparams.enable_ppgtt == 3)
> > +#define HAS_PAGE_SIZES(dev_priv, sizes) \
> > +       ((sizes) && (((sizes) & ~(dev_priv)->info.page_sizes)) == 0)
> 
> I don't think we ever want to ask if we support a mask of 0, so perhaps
> 
> (BUILD_BUG_ON_ZERO(!sizes) ?: (sizes) & ~(dev_priv)->info.page_sizes) == 0)
> 
> >  void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
> > -                                struct sg_table *pages)
> > +                                struct sg_table *pages,
> > +                                unsigned int sg_mask)
> >  {
> > +       struct drm_i915_private *i915 = to_i915(obj->base.dev);
> > +       unsigned long supported_page_sizes = INTEL_INFO(i915)->page_sizes;
> > +       int i;
> > +
> >         lockdep_assert_held(&obj->mm.lock);
> >  
> >         obj->mm.get_page.sg_pos = pages->sgl;
> > @@ -2475,6 +2488,25 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
> >                 __i915_gem_object_pin_pages(obj);
> >                 obj->mm.quirked = true;
> >         }
> > +
> > +       GEM_BUG_ON(!sg_mask);
> > +       obj->mm.page_sizes.phys = sg_mask;
> > +
> > +       /*
> > +        * Calculate the supported page-sizes which fit into the given sg_mask.
> > +        * This will give us the page-sizes which we may be able to use
> > +        * opportunistically when later inserting into the GTT. For example if
> > +        * phys=2G, then in theory we should be able to use 1G, 2M, 64K or 4K
> > +        * pages, although in practice this will depend on a number of other
> > +        * factors.
> > +        */
> > +       obj->mm.page_sizes.sg = 0;
> > +       for_each_set_bit(i, &supported_page_sizes, BITS_PER_LONG) {
> 
> s/BITS_PER_LONG/ilog2(MAX_PAGE_SIZE) + 1/ ?
> 
> BITS_PER_LONG is definitely inconsistent as the code v is using 0u and
> not 0ul :)
> 
> > +               if (obj->mm.page_sizes.phys & ~0u << i)
> > +                       obj->mm.page_sizes.sg |= BIT(i);
> > +       }
> 
> Ok, couldn't see anything but I'm going to run it before committing
> myself this time! ;)

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris