From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paulo Zanoni <przanoni@gmail.com>
Subject: Re: [PATCH 09/16] drm/i915: Store current watermark
 state in dev_priv->wm
Date: Tue, 15 Oct 2013 13:49:10 -0300
Message-ID: <CA+gsUGSVzk-W5TVTZ=TM5X2RH0TgOPgdKj60SYe6=vDZ5oL-Mw@mail.gmail.com>
References: <1381335490-4906-1-git-send-email-ville.syrjala@linux.intel.com>
	<1381335490-4906-10-git-send-email-ville.syrjala@linux.intel.com>
	<CA+gsUGSu5DwGX+bnZSnTC2ygYSsqef8FOn49DHEAAEvh59Ti7g@mail.gmail.com>
	<20131015082433.GA4830@phenom.ffwll.local>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
Received: from mail-oa0-f50.google.com (mail-oa0-f50.google.com
	[209.85.219.50])
	by gabe.freedesktop.org (Postfix) with ESMTP id E9EBFE77B1
	for <intel-gfx@lists.freedesktop.org>;
	Tue, 15 Oct 2013 09:49:10 -0700 (PDT)
Received: by mail-oa0-f50.google.com with SMTP id j1so6144099oag.9
	for <intel-gfx@lists.freedesktop.org>;
	Tue, 15 Oct 2013 09:49:10 -0700 (PDT)
In-Reply-To: <20131015082433.GA4830@phenom.ffwll.local>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
List-Id: intel-gfx@lists.freedesktop.org

2013/10/15 Daniel Vetter <daniel@ffwll.ch>:
> On Fri, Oct 11, 2013 at 11:21:04AM -0300, Paulo Zanoni wrote:
>> 2013/10/9  <ville.syrjala@linux.intel.com>:
>
> [snip]
>
>> > +       previous.enable_fbc_wm = !(I915_READ(DISP_ARB_CTL) & DISP_FBC_WM_DIS);
>> > +
>> > +       if (memcmp(results, &previous, sizeof(*results)) == 0)
>>
>> This may cause problems since we're also comparing the structure
>> paddings. It seems "results" is already zero-initialized, so if you
>> also zero-initialize "previous" we'll probably be fine with the
>> memcmp(). But my fear is that future code changes will break this, so
>> if you stick with the new memcmp please add a comment remembering us
>> that we rely on zero-initializing stuff. Or maybe keep the
>> old-big-ugly code.
>
> I've added a comment about this and then smashed your presumed r-b onto
> the patch. Please scream if the patch as merged isn't good enough.

Patch 10 removes the memcmp, so the problem is fixed. You should then
remove the comment on that patch.

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Paulo Zanoni