[Intel-gfx] [PATCH] drm/i915: Sink CRC: tune down error message at stop to debug_kms.

[Daniel Vetter]

On Wed, Jan 20, 2016 at 12:37:39PM +0000, Zanoni, Paulo R wrote:
> Em Ter, 2016-01-19 às 22:21 +0000, Vivi, Rodrigo escreveu:
> > On Tue, 2016-01-19 at 17:46 +0000, Zanoni, Paulo R wrote:
> > > Em Qua, 2016-01-13 às 14:05 -0800, Rodrigo Vivi escreveu:
> > > > When we stop the sink CRC calculation we wait a while until the
> > > > counter
> > > > is reset to zero and return -ETIMEDOUT. However the sink crc was
> > > > calculated already by this point so we just ignore this return at
> > > > the main function.
> > > > 
> > > > So, let's also ignore the message and put it as a debug message
> > > > instead
> > > > of an error one. The message might still be useful when debuging
> > > > test failures so we could be able to know something was not
> > > > going 
> > > > so
> > > > well with sink crc stop.
> > > > Reference: https://bugs.freedesktop.org/show_bug.cgi?id=93694
> > > > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index c8f58ab..22f6887 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -4009,7 +4009,7 @@ static int intel_dp_sink_crc_stop(struct
> > > > intel_dp *intel_dp)
> > > >  	} while (--attempts && count);
> > > 
> > > Now I'm wondering if "count" stays static or changes during the 10
> > > failed attempts. I know this is against the spec, but would make
> > > some
> > > sense to see it.
> > > 
> > > Another possibility is that we're not changing the contents of the
> > > frame, so maybe we need to give it something new in order for the 
> > > count
> > > to really reset. This would make more sense for PSR.
> > 
> > Well, on the machines that sink crc isn't reliable I tried many
> > attempts like to change the screen content, enable and disable again,
> > sait more time, etc and there is no reliable way to make sure it
> > reset
> > to zero. Sometimes it just keep increasing even with test zeroed... 
> > not following dp spec at all... 
> > 
> > > 
> > > Anyway, this "error" shouldn't be a problem for our test suite. If
> > > it
> > > impacts the next CRC calculations, then we'll notice this through 
> > > other
> > > means, so:
> > > Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > 
> > Thanks, but on IRC daniel had complained that the message is not good
> > for debug purpose. So before merging it I'd like to check with you if
> > it would be better with something like:
> > 
> > DRM_DEBUG_KMS("TIMEOUT: Sink CRC counter is not zeroed after
> > calculation is stopped\n");
> 
> Since he's the one doing the suggestion, I'll let him answer.

tbh I've forgotten the context of this, so probably not important. Just go
ahead with what seems fine. Either seems fine.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch