[Intel-gfx] [PATCH v2 3/7] drm/i915: Compact memcmp in i915_vma_compare()
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Fri Jan 13 08:57:49 UTC 2017
On 12/01/2017 21:35, Chris Wilson wrote:
> In preparation for the next patch to convert to using an anonymous union
> and leaving the excess bytes in the union uninitialised, we first need
> to make sure we do not compare using those uninitialised bytes. We also
> want to preserve the compactness of the code, avoiding a second call to
> memcmp or introducing a switch, so we take advantage of using the type
> as an encoded size (as well as a unique identifier for each type of view).
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.h | 14 +++++++-------
> drivers/gpu/drm/i915/i915_vma.h | 15 +++++++++------
> 2 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 57cbd532dae1..20c03c30ce4e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -145,12 +145,6 @@ typedef uint64_t gen8_ppgtt_pml4e_t;
>
> struct sg_table;
>
> -enum i915_ggtt_view_type {
> - I915_GGTT_VIEW_NORMAL = 0,
> - I915_GGTT_VIEW_ROTATED,
> - I915_GGTT_VIEW_PARTIAL,
> -};
> -
> struct intel_rotation_info {
> struct intel_rotation_plane_info {
> /* tiles */
> @@ -173,10 +167,16 @@ static inline void assert_intel_partial_info_is_packed(void)
> BUILD_BUG_ON(sizeof(struct intel_partial_info) != sizeof(u64) + sizeof(unsigned int));
> }
>
> +enum i915_ggtt_view_type {
> + I915_GGTT_VIEW_NORMAL = 0,
> + I915_GGTT_VIEW_ROTATED = sizeof(struct intel_rotation_info),
> + I915_GGTT_VIEW_PARTIAL = sizeof(struct intel_partial_info),
> +};
> +
> struct i915_ggtt_view {
> enum i915_ggtt_view_type type;
> -
> union {
> + /* Members need to contain no holes/padding */
> struct intel_partial_info partial;
> struct intel_rotation_info rotated;
> } params;
> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index a969bbb65871..19f049cef9e3 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -199,15 +199,18 @@ i915_vma_compare(struct i915_vma *vma,
> if (cmp)
> return cmp;
>
> + BUILD_BUG_ON(I915_GGTT_VIEW_NORMAL != 0);
> + cmp = vma->ggtt_view.type;
> if (!view)
> - return vma->ggtt_view.type;
> + return cmp;
> +
> + cmp -= view->type;
> + if (cmp)
> + return cmp;
>
> - if (vma->ggtt_view.type != view->type)
> - return vma->ggtt_view.type - view->type;
> + BUILD_BUG_ON(I915_GGTT_VIEW_PARTIAL == I915_GGTT_VIEW_ROTATED);
>
> - return memcmp(&vma->ggtt_view.params,
> - &view->params,
> - sizeof(view->params));
> + return memcmp(&vma->ggtt_view.params, &view->params, view->type);
Worth a comment at the memcmp callsite about how type for size is
intended and why. Otherwise it is only in the commit message which is
more digging.
> }
>
> int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
>
With the comment added,
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Regards,
Tvrtko
More information about the Intel-gfx
mailing list