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

Daniel Vetter
2015-05-07T16:13:31Z

On Wed, May 06, 2015 at 02:40:48PM +0300, Joonas Lahtinen wrote:
> 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?

Already pulled in, please do a follow-up patch.

Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation