[Intel-gfx] [PATCH 09/16] drm/i915: Store current watermark state in dev_priv->wm

Paulo Zanoni przanoni at gmail.com
Tue Oct 15 18:49:10 CEST 2013


2013/10/15 Daniel Vetter <daniel at ffwll.ch>:
> On Fri, Oct 11, 2013 at 11:21:04AM -0300, Paulo Zanoni wrote:
>> 2013/10/9  <ville.syrjala at 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



More information about the Intel-gfx mailing list