On Fri, 2017-10-06 at 15:50 +0100, Matthew Auld wrote:
> In preparation for supporting huge gtt pages for the ppgtt, we introduce
> page size members for gem objects. We fill in the page sizes by
> scanning the sg table.
>
> v2: pass the sg_mask to set_pages
>
> v3: calculate the sg_mask inline with populating the sg_table where
> possible, and pass to set_pages along with the pages.
>
> v4: bunch of improvements from Joonas
>
> v5: fix num_pages blunder
> introduce i915_sg_page_sizes helper
>
> v6: prefer GEM_BUG_ON(sizes == 0)
>
> Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
<SNIP>
> @@ -3101,6 +3116,10 @@ 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) ({ \
> + GEM_BUG_ON((sizes) == 0); \
> + ((sizes) & ~(dev_priv)->info.page_sizes) == 0; \
> +})
Maybe,
#define HAS_PAGE_SIZES(dev_priv, sizes) (\
BUILD_BUG_ON_ZERO((sizes) == 0) +
...
)
To avoid the compound statement. Unlikely to be used in static const
expressions, but no reason not to have it flxible from the beginning.
> @@ -2266,6 +2266,8 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
> if (!IS_ERR(pages))
> obj->ops->put_pages(obj, pages);
>
> + obj->mm.page_sizes.phys = obj->mm.page_sizes.sg = 0;
Could be "obj->mm.page_sizes = { .phys = 0, .sg = 0 };" Or at least
split this to two lines so checkpatch won't complain.
> @@ -2308,6 +2310,7 @@ static int i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
> struct page *page;
> unsigned long last_pfn = 0; /* suppress gcc warning */
> unsigned int max_segment = i915_sg_segment_size();
> + unsigned int sg_mask;
Was 'sg_page_sizes' considered?
> @@ -2460,8 +2468,13 @@ static int i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
> }
>
> 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 = INTEL_INFO(i915)->page_sizes;
'supported_sizes' might be more descriptive if this ever grows bigger.
Only thing I really feel strongly about is the renaming of
s/sg_mask/sg_page_sizes/. That fixed;
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
