Re: [PATCH 2/2] drm/i915: Only recalculate wm's for planes part of the state, v2.

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: "Zanoni, Paulo R" <paulo.r.zanoni@intel.com>,
	"Roper, Matthew D" <matthew.d.roper@intel.com>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/i915: Only recalculate wm's for planes part of the state, v2.
Date: Thu, 3 Mar 2016 09:23:52 +0100	[thread overview]
Message-ID: <56D7F498.4020901@linux.intel.com> (raw)
In-Reply-To: <1456952909.2408.36.camel@intel.com>

Op 02-03-16 om 22:08 schreef Zanoni, Paulo R:
> Em Ter, 2016-03-01 às 14:28 -0800, Matt Roper escreveu:
>> On Tue, Mar 01, 2016 at 11:07:22AM +0100, Maarten Lankhorst wrote:
>>> Only planes that are part of the state should be used for
>>> recalculating
>>> watermarks. For planes not part of the state the previous patch
>>> allows
>>> us to re-use the old values since they're calculated even for
>>> levels
>>> that are not actively used.
>>>
>>> Changes since v1:
>>> - Remove big if from intel_crtc_atomic_check.
>>> - Remove extra newline.
>>> - Remove memset in ilk_compute_pipe_wm.
>>>
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com
>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> I haven't thought through this too carefully yet, but off the top of
>> my
>> head I'm not sure if this will work for SKL once we transition it to
>> also use a more atomic style.  Changes to other state might result in
>> changes to DDB allocation, making our previously-calculated values
>> invalid.
>>
>> I think this is okay for the current code (where only ILK is atomic),
>> so
>>
>>     Acknowledged-by: Matt Roper <matthew.d.roper@intel.com>
>>
>> for the time being.  I'll be back to looking at SKL-style watermarks
>> in
>> the next day or two and I might have to backtrack somewhat in cases
>> where a DDB partitioning changes results in a full recompute, but I
>> need
>> to think through the details a bit more about how best to handle
>> that.
> I spent some time looking at that early return from invalid pipe
> watermarks, but I suppose that return means we'll completely discard
> anything just computed by the function, right?
>
> The patch seems to do what it says on the box, so if we assume we
> actually want the patch:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> I'll let you and Matt decide whether we actually want the patch or not.
>
This was a bug. I sent a fix for that one. When wm calculation fails -EINVAL is returned now,
and the invalid wm's discarded.

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx