Re: [PATCH] drm: Don't force all planes to be added to the state due to zpos

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>,
	stable <stable@vger.kernel.org>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Lyude <cpaul@redhat.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Vincent Abriou <vincent.abriou@st.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH] drm: Don't force all planes to be added to the state due to zpos
Date: Mon, 10 Oct 2016 15:56:43 +0300	[thread overview]
Message-ID: <20161010125643.GE4329@intel.com> (raw)
In-Reply-To: <CAKMK7uHu3jDtzHSUtQRyky_562S6wNx3+P_oHbQNYzsFJ4cRvw@mail.gmail.com>

On Mon, Oct 10, 2016 at 02:46:35PM +0200, Daniel Vetter wrote:
> On Mon, Oct 10, 2016 at 2:19 PM,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We don't want all planes to be added to the state whenever a
> > plane with fixed zpos gets enabled/disabled. This is true
> > especially for eg. cursor planes on i915, as we want cursor
> > updates to go through w/o throttling. Same holds for drivers
> > that don't support zpos at all (i915 actually falls into this
> > category right now since we've not yet added zpos support).
> >
> > Allow drivers more freedom by letting them deal with zpos
> > themselves instead of doing it in drm_atomic_helper_check_planes()
> > unconditionally. Easiest solution seems to be to move the call
> > up to drm_atomic_helper_check(). But as some drivers might want
> > to use that function without the zpos handling, let's provide
> > two variants: the normal one, and one that deals with zpos.
> >
> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> > Cc: Vincent Abriou <vincent.abriou@st.com>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Inki Dae <inki.dae@samsung.com>
> > Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> > Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > Cc: Lyude <cpaul@redhat.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: stable@vger.kernel.org
> > Fixes: 44d1240d006c ("drm: add generic zpos property")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Seems a bit fragile, and then drivers still need to not overshot when
> they do zpos (which we want eventually in i915 too).

The only platform where we can do it is pre-g4x, and vlv/chv. And I'm
thinking I can do a better job of it in the driver.

> I think the
> proper way is to keep track of a per-plane zpos changed (or compute
> that ad-hoc, we have both states). And only grab more planes if a zpos
> value changed.

Doesn't work with normalized zpos. The plane's actual zpos may be
unchanged even if the normalized zpos changes.

> 
> That would fix the issue at the source, also work for us in the
> future, and it should be contained to just the helper function itself.
> Win all around ;-)
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

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