Re: [PATCH 2/9] drm/i915: Dump in-flight plane state while dumping in-flight CRTC state

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Dec 03, 2015 at 01:27:52PM +0100, Maarten Lankhorst wrote:
> Op 25-11-15 om 17:48 schreef Matt Roper:
> > The intel_dump_pipe_config() always dumps the currently active plane
> > state for each plane on a CRTC.  If we're calling this function to dump
> > CRTC state that is in-flight (not yet active), then this mismatch can be
> > misleading and confusing.  Let's pay attention to whether the state
> > we're dumping is part of an in-flight transaction (because
> > crtc_state->state is non-NULL); if it is, we'll dump the corresponding
> > in-flight plane state instead of the active state.
> >
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d03a235..0e74287 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -12162,11 +12162,23 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
> >  
> >  	DRM_DEBUG_KMS("planes on this crtc\n");
> >  	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
> > +		struct drm_plane_state *ps = NULL;
> > +
> >  		intel_plane = to_intel_plane(plane);
> >  		if (intel_plane->pipe != crtc->pipe)
> >  			continue;
> >  
> > -		state = to_intel_plane_state(plane->state);
> > +		/* Get in-flight plane state, if any */
> > +		if (pipe_config->base.state)
> > +			ps = drm_atomic_get_existing_plane_state(pipe_config->base.state,
> > +								 plane);
> > +
> > +		/* If no in-flight state, use active state instead */
> > +		if (!ps)
> > +			ps = plane->state;
> > +
> Could plane state be removed from dump_pipe_config instead? There's no good way to do it race free and
> it's better to do it in intel_plane_atomic_calc_changes.

Yeah dump_pipe_config is meant for debugging crtc_config mismatches. No
point in dumping plane state. Especially since we can validate all the
plane stuff through CRC based testcases anyway.
-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