[Intel-gfx] [PATCH 3/6] drm/i915: Compact memcmp in i915_vma_compare()

[Chris Wilson]

On Thu, Jan 12, 2017 at 07:14:55AM +0000, Tvrtko Ursulin wrote:
> 
> On 12/01/2017 07:08, Tvrtko Ursulin wrote:
> >
> >On 11/01/2017 21:51, 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 3187a260e6e1..36d85599ffc9 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 */
> >>@@ -184,10 +178,16 @@ static inline u64
> >>intel_partial_get_page_offset(const struct intel_partial_info
> >>     return pi->offset_size >> INTEL_PARTIAL_SIZE_BITS;
> >> }
> >>
> >>+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),
> >>+};
> >
> >I just realized this will uglify the debugfs output. In the light of
> >that I think we should leave the enum human readable and instead you
> >could have a static map in i915_vma_compare. Hopefully code generation
> >wouldn't be affected by it.
> 
> Another alternative, perhaps better, a map of view type to printable
> names in i915_debugfs.c.

Yeah, when in doubt remove guess work from the reader.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre