[Intel-gfx] [PATCH] drm/i915/skl: Bypass debug message if scalers are not requested

[Daniel Vetter]

On Fri, Apr 24, 2015 at 06:07:05PM +0000, Konduru, Chandra wrote:
> 
> 
> > -----Original Message-----
> > From: Konduru, Chandra
> > Sent: Friday, April 24, 2015 10:53 AM
> > To: 'Tvrtko Ursulin'; Intel-gfx at lists.freedesktop.org
> > Cc: Ursulin, Tvrtko
> > Subject: RE: [PATCH] drm/i915/skl: Bypass debug message if scalers are not
> > requested
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Tvrtko Ursulin [mailto:tvrtko.ursulin at linux.intel.com]
> > > Sent: Friday, April 24, 2015 9:34 AM
> > > To: Konduru, Chandra; Intel-gfx at lists.freedesktop.org
> > > Cc: Ursulin, Tvrtko
> > > Subject: Re: [PATCH] drm/i915/skl: Bypass debug message if scalers are
> > > not requested
> > >
> > >
> > > On 04/24/2015 05:30 PM, Konduru, Chandra wrote:
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: Tvrtko Ursulin [mailto:tvrtko.ursulin at linux.intel.com]
> > > >> Sent: Friday, April 24, 2015 9:08 AM
> > > >> To: Intel-gfx at lists.freedesktop.org
> > > >> Cc: Ursulin, Tvrtko; Konduru, Chandra
> > > >> Subject: [PATCH] drm/i915/skl: Bypass debug message if scalers are
> > > >> not requested
> > > >>
> > > >> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > > >>
> > > >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > > >> Cc: Chandra Konduru <chandra.konduru at intel.com>
> > > >> ---
> > > >> Up for discussion I suppose, but like it is, with typical drm.debug
> > > >> = 0xe, it logs one line per cursor movement while the log would
> > > >> otherwise be
> > > quiet.
> > > >> ---
> > > >>   drivers/gpu/drm/i915/intel_atomic.c | 3 +++
> > > >>   1 file changed, 3 insertions(+)
> > > >>
> > > >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> > > >> b/drivers/gpu/drm/i915/intel_atomic.c
> > > >> index 3c4b7cd..7284c6d 100644
> > > >> --- a/drivers/gpu/drm/i915/intel_atomic.c
> > > >> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > > >> @@ -302,6 +302,9 @@ int intel_atomic_setup_scalers(struct
> > > >> drm_device
> > > *dev,
> > > >>   	scaler_state = &crtc_state->scaler_state;
> > > >>   	drm_state = crtc_state->base.state;
> > > >>
> > > >> +	if (!scaler_state->scaler_users)
> > > >> +		return 0;
> > > >
> > > > This will cause issue because scalers will never get freed if they
> > > > are in use
> > > before and no more required now.
> > > > I put the debug print to help debug variety of state related issues
> > > > while we are in development, but perhaps debug print can be deleted.
> > >
> > > Doesn't the loop below skip everything anyway when no bits are set in
> > > scaler_state->scaler_users ?
> > Oh, that's right, ignore my prev comment (with updated scaler design above
> > shouldn't cause issues).
> > By the way, can you pls run kms_panel_fit and kms_plane_scaling to make sure
> > they pass and there are no related warnings or errors in kernel log?
> > For kms_plane_scaling, pls apply two pending patches that aren't merged:
> > v5 [PATCH 13/14] drm/i915: skylake primary plane scaling using shared scalers
> > v5 [PATCH 14/14] drm/i915: skylake sprite plane scaling using shared scalers
> > 
> One more: This change address logs when no scaler is required,
> but might comeback when a scaler is active (panel fitting or plane scaling is enabled).
> I don't know general policy in these kind of situations, but perhaps debug print
> can be deleted.

Atomic is really complicated, but doing fully diagnostics for each frame
is also way too noisy. For that reason we've add a DRM_DEBUG_ATOMIC which
can be used for all these state tracking debug lines.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch