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@xxxxxxxxxxxxxxxxxxxxx > > 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@xxxxxxxxxxxxxxx] > > > Sent: Friday, April 24, 2015 9:34 AM > > > To: Konduru, Chandra; Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > 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@xxxxxxxxxxxxxxx] > > > >> Sent: Friday, April 24, 2015 9:08 AM > > > >> To: Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > >> Cc: Ursulin, Tvrtko; Konduru, Chandra > > > >> Subject: [PATCH] drm/i915/skl: Bypass debug message if scalers are > > > >> not requested > > > >> > > > >> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > > >> > > > >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > > >> Cc: Chandra Konduru <chandra.konduru@xxxxxxxxx> > > > >> --- > > > >> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx