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

[Daniel Vetter]

On Wed, Aug 05, 2015 at 02:20:17PM +0100, Tvrtko Ursulin wrote:
> 
> On 05/04/2015 03:29 PM, Daniel Vetter wrote:
> >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.
> 
> We didn't do anything here and I just noticed kernel is still too spammy
> with regards to this issue.
> 
> Should we just merge my patch? Still looks completely safe to me...

doesn't seem to apply any more:(
-Daniel

> 
> Regards,
> 
> Tvrtko

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch