Re: [Intel-gfx] [PATCH 3/3] drm/i915: Clean up early plane debugs

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915: Clean up early plane debugs
Date: Tue, 2 Oct 2018 17:42:12 +0300	[thread overview]
Message-ID: <20181002144212.GV9144@intel.com> (raw)
In-Reply-To: <20181002122132.GO11082@phenom.ffwll.local>

On Tue, Oct 02, 2018 at 02:21:32PM +0200, Daniel Vetter wrote:
> On Mon, Oct 01, 2018 at 05:31:27PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Print the plane hw state readout results in the common format
> > we already use for pipes and encoders. Also print some clearer
> > debug messages when we disable planes during the early phases
> > of state readout/sanitization.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index c72be8cd1f54..1e9628f0cb47 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2772,10 +2772,6 @@ intel_set_plane_visible(struct intel_crtc_state *crtc_state,
> >  		crtc_state->base.plane_mask &= ~drm_plane_mask(&plane->base);
> >  		crtc_state->active_planes &= ~BIT(plane->id);
> >  	}
> > -
> > -	DRM_DEBUG_KMS("%s active planes 0x%x\n",
> > -		      crtc_state->base.crtc->name,
> > -		      crtc_state->active_planes);
> 
> There's a 3rd call for intel_set_plane_visible in the plane_obj code. Do
> we not care? Is that call redundant (kinda a comment on the previous patch
> really, just only realized it here)?

Yeah, looks like that call is redundant. The only way to reach it
is if the plane was already enabled.

> -Daniel
> 
> >  }
> >  
> >  static void intel_plane_disable_noatomic(struct intel_crtc *crtc,
> > @@ -2786,6 +2782,10 @@ static void intel_plane_disable_noatomic(struct intel_crtc *crtc,
> >  	struct intel_plane_state *plane_state =
> >  		to_intel_plane_state(plane->base.state);
> >  
> > +	DRM_DEBUG_KMS("Disabling [PLANE:%d:%s] on [CRTC:%d:%s]\n",
> > +		      plane->base.base.id, plane->base.name,
> > +		      crtc->base.base.id, crtc->base.name);
> > +
> >  	intel_set_plane_visible(crtc_state, plane_state, false);
> >  
> >  	if (plane->id == PLANE_PRIMARY)
> > @@ -15507,8 +15507,8 @@ intel_sanitize_plane_mapping(struct drm_i915_private *dev_priv)
> >  		if (pipe == crtc->pipe)
> >  			continue;
> >  
> > -		DRM_DEBUG_KMS("%s attached to the wrong pipe, disabling plane\n",
> > -			      plane->base.name);
> > +		DRM_DEBUG_KMS("[PLANE:%d:%s] attached to the wrong pipe, disabling plane\n",
> > +			      plane->base.base.id, plane->base.name);
> >  
> >  		plane_crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> >  		intel_plane_disable_noatomic(plane_crtc, plane);
> > @@ -15705,6 +15705,10 @@ static void readout_plane_state(struct drm_i915_private *dev_priv)
> >  		crtc_state = to_intel_crtc_state(crtc->base.state);
> >  
> >  		intel_set_plane_visible(crtc_state, plane_state, visible);
> > +
> > +		DRM_DEBUG_KMS("[PLANE:%d:%s] hw state readout: %s, pipe %c\n",
> > +			      plane->base.base.id, plane->base.name,
> > +			      enableddisabled(visible), pipe_name(pipe));
> >  	}
> >  
> >  	for_each_intel_crtc(&dev_priv->drm, crtc) {
> > -- 
> > 2.16.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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