Re: [PATCH 4/4] drm/i915: Clear crtc atomic flags at beginning of transaction

From: Daniel Vetter <daniel@ffwll.ch>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/4] drm/i915: Clear crtc atomic flags at beginning of transaction
Date: Thu, 9 Apr 2015 17:51:52 +0200	[thread overview]
Message-ID: <20150409155152.GT12038@phenom.ffwll.local> (raw)
In-Reply-To: <20150409150335.GP11603@intel.com>

On Thu, Apr 09, 2015 at 08:03:35AM -0700, Matt Roper wrote:
> On Thu, Apr 09, 2015 at 02:48:43PM +0200, Daniel Vetter wrote:
> > On Wed, Apr 08, 2015 at 06:56:54PM -0700, Matt Roper wrote:
> > > Once we have full atomic modeset, these kind of flags should be in a
> > > real intel_crtc_state that's tracked properly.  In the meantime, make
> > > sure we clear out any old flags at the beginning of a transaction so
> > > that we don't wind up seeing leftover flags from old transactions that
> > > were checked, but never went to the commit step.
> > > 
> > > A simple memset would have done here, but I expect there to be a few
> > > more things that we *don't* want to clear that get added into this
> > > structure before we're ready to kill off and roll everything into the
> > > CRTC state.
> > > 
> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > 
> > Not sure whether this helps a lot with moving intel_crtc->atomic into
> > intel_crtc_state. We probably want to just move things one-by-one to
> > reduce the overall churn, and then we can add them one-by-one to the intel
> > crtc_state_duplicate function.
> > 
> > Or do you have some bigger plans here?
> > -Daniel
> 
> My in-progress WM work had to stick some other things in
> intel_crtc->atomic, but maybe with Ander's latest patch sets we're close
> enough that I can rewrite those to use crtc_state instead; I'll have to
> go back and check.
> 
> Regardless, I think we do still have a bug today where an uncommitted
> transaction (e.g., because check or prepare fail) will leave stale flags
> in intel_crtc->atomic that the next transaction doesn't realize it needs
> to clear.  We either need to clear those out (as I'm doing here,
> although a simple memset would work too), or we need to transition all
> those flags over to CRTC state and kill off the hacky intel_crtc->atomic
> structure.

Yeah I agree that we need the memset. Can you resend just that one,
without the wrapping please?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx