[Intel-gfx] [PATCH] drm/i915: Initialize primary plane src/dst coords when reading hw state

[Daniel Vetter]

On Tue, Jan 13, 2015 at 01:24:12PM +0200, Ville Syrjälä wrote:
> On Tue, Jan 13, 2015 at 12:34:06AM +0100, Daniel Vetter wrote:
> > On Mon, Jan 12, 2015 at 05:36:52PM +0200, Ander Conselvan de Oliveira wrote:
> > > Otherwise setting the rotation property will cause the primary plane to
> > > be disabled, caused by having a 0x0 initial value.
> > >
> > > Cc: stable at vger.kernel.org
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662
> > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >
> > I guess long term we need to consolidate all our plane state related
> > readout functions, atm it's splattered all over. But that's maybe
> > something for after all the atomic work has stablized a bit.
>
> We would also need to consider the user requested vs. current state
> issue during resume. If someone suspends with a bunch of planes
> enabled we should restore them during resume rather than overwrite
> the user requested state with the current hardware state.
>
> So the patch isn't entirely correct in that sense, but given that
> we overwrite these values again based on the crtc->x/y and crtc->mode
> when we restore the mode, so it should come out ok in this case. I
> suppose we don't yet track the current vs. user state sufficiently
> to do things in an entirely correct way.

Hm right I've missed that interaction completely. Do we have a testcase
which sets the primary planes to a specified place, does a suspend/resume
and then checks the placement with CRCs? Might need to do a full pass over
all the crc plane (primary, cursor, sprite) we have already and make sure
we have subtests against suspend for all interesting cases.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch