Re: [PATCH v2 4/5] drm/i915: Add a partial GGTT view type

Joonas Lahtinen
2015-05-06T11:40:50Z

On ke, 2015-05-06 at 12:20 +0200, Daniel Vetter wrote:
> On Thu, Apr 30, 2015 at 01:16:30PM +0100, Tvrtko Ursulin wrote:
> > On 04/30/2015 12:20 PM, Joonas Lahtinen wrote:
> > >@@ -495,7 +503,10 @@ i915_ggtt_view_equal(const struct i915_ggtt_view *a,
> > > if (WARN_ON(!a || !b))
> > > return false;
> > >
> > >- return a->type == b->type;
> > >+ if (a->type != b->type)
> > >+ return false;
> > >+
> > >+ return !memcmp(&a->params, &b->params, sizeof(a->params));
> >
> > Still don't like this, would this be so bad:
> >
> > if (a->type != PARTIAL)
> > return a->type == b->type;
> > else
> > return !memcmp(...)
>
> Why do we even need this? memcmp implies comparing just one part of the
> struct, doesn't it? Of course this means we need to clear it, but imo
> that's the rtdt anyway.

I only noticed this comment now (just sent v3 of the series out), I'm
fine with leaving it like it was in the revision 1, which means that the
last line is return !memcmp(..), so if the patch is otherwise OK, shall
I make a one more revision, or will you merge manually?

Regards, Joonas

> -Daniel