Re: [PATCH 14/22] drm/i915: Don't treat differently sized rotated views as equal

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Oct 15, 2015 at 02:24:51PM +0200, Daniel Vetter wrote:
> On Thu, Oct 15, 2015 at 03:06:11PM +0300, Ville Syrjälä wrote:
> > On Thu, Oct 15, 2015 at 02:02:24PM +0200, Daniel Vetter wrote:
> > > On Thu, Oct 15, 2015 at 12:18:41PM +0100, Tvrtko Ursulin wrote:
> > > > 
> > > > On 14/10/15 17:29, ville.syrjala@linux.intel.com wrote:
> > > > >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > >
> > > > >In case we have multiple different rotated views into the same object,
> > > > >each one may need its own vma due to being of different sizes. So don't
> > > > >treat all rotated views as equal.
> > > > >
> > > > >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > >---
> > > > >  drivers/gpu/drm/i915/i915_gem_gtt.h | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > >diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > > > >index caa182f..68de734 100644
> > > > >--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> > > > >+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > > > >@@ -553,6 +553,8 @@ i915_ggtt_view_equal(const struct i915_ggtt_view *a,
> > > > >
> > > > >  	if (a->type != b->type)
> > > > >  		return false;
> > > > >+	if (a->type == I915_GGTT_VIEW_ROTATED)
> > > > >+		return !memcmp(&a->rotated, &b->rotated, sizeof(a->rotated));
> > > > >  	if (a->type == I915_GGTT_VIEW_PARTIAL)
> > > > >  		return !memcmp(&a->partial, &b->partial, sizeof(a->partial));
> > > > >  	return true;
> > > > 
> > > > This is where anonymous union causes problems since you have to build a
> > > > switch statement before memcmp while the original goal was for memcmp to
> > > > elegantly avoid that.
> > > 
> > > Yup, mentioned the same on irc. I much prefer we do the memcmp on type !=
> > > NORMAL, to avoid mistakes when adding more views in the future.
> > 
> > I just dislike the .partial. extra stuff all over. But whatever, it's a
> > minor detail in my book. We can go with the non-anon union if people
> > prefer that.
> 
> Here it would be
> 	if (a->type != NORMA))
> 		return !memcmp(&a->params, &b->params);
> 
> Seems at least in this case cleaner. But yet everywhere else you'll get a
> params., but that can be alleviated a bit with local variables.

We can go with your patches that preserve the named union. I can then
rebase my stuff on top.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx