Re: [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>,
	Ben Widawsky <ben@bwidawsk.net>
Subject: Re: [PATCH 2/2] drm/i915: Fail addfb ioctl if color and CCS buffers overlap
Date: Fri, 22 Sep 2017 14:54:37 +0300	[thread overview]
Message-ID: <20170922115437.GL4914@intel.com> (raw)
In-Reply-To: <87h8vv9uf4.fsf@dilma.collabora.co.uk>

On Fri, Sep 22, 2017 at 02:25:19AM -0300, Gabriel Krisman Bertazi wrote:
> Ville Syrjälä <ville.syrjala@linux.intel.com> writes:
> 
> > On Tue, Sep 05, 2017 at 08:36:54AM +0100, Daniel Stone wrote:
> >> Hi Ville,
> >> 
> >> On 4 September 2017 at 17:37, Ville Syrjälä
> >> <ville.syrjala@linux.intel.com> wrote:
> >> > On Thu, Aug 31, 2017 at 04:52:15PM -0300, Gabriel Krisman Bertazi wrote:
> >> >> With this patch the new testcase igt@kms_ccs@pipe-X-invalid-ccs-offset
> >> >> succeeds.
> >> >
> >> > I don't think we actually want to reject overlap. I had a patch for that
> >> > years ago, but I decided to drop it because people might want to
> >> > interleave the planes in some interesting ways. Making the overlap
> >> > check accurate enough to allow that would be to total overkill. So IMO
> >> > it's perfectly fine to let the user shoot himself in the foot if they
> >> > mess up the offsets.
> >> 
> >> Is that actually supported by any hardware renderer? If not, maybe the
> >> check should only be enabled for generations who support it.
> >
> > Not sure I understand the question. You can program your offsets/strides
> > any which way you want,
> 
> Hi Ville,
> 
> Sorry for the delay.  Reviewing documentation at [1] I see the
> following, in page 177, regarding CCS structure:
> "The CCS is always placed after the main surface and is 4K page
> aligned".

The hardware doesn't really have such a requirement. The only thing it
cares about is that AUX_DIST must be positive which means the AUX
surface always starts >= plane SURF. That doesn't imply that the two
can't potentially overlap, and actually doesn't even imply that we
couldn't place the aux surface below the main surface (we can do that as
long as we are able to adjust the plane SURF and AUX_DIST appropriately).

But the main point is that the user can still ess up the AUX plane
offset in other ways, so trying to protect them against this one way
of messing up is rather pointless.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx