| Message ID | 20180501203915.160788-1-tarun.vyas@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 01, 2018 at 01:39:15PM -0700, Tarun Vyas wrote: > Just a minor knit. Stumbled across the kernel doc for schedule_timeout() which > quotes "In all cases the return value is guaranteed to be non-negative". Also, > the return code of schedule_timeout() already checks for negative values > "return timeout < 0 ? 0 : timeout;" and returns 0 in such cases. So, > let's do away with the redundant check for an atomic pipe update. Makes sense to me. Also in the very first iteration where it gets timeout from msecs_to_jiffies_timeout, that should return an unsigned long value and then onwards schedule_timeout should return a non negative value. So I agree that we can get rid of the <=0 check. Might be better to say clearly in the commit message "No functional change". With that, Reviewed-by: Manasi Navare <manasi.d.navare@intel.com> Manasi > > Signed-off-by: Tarun Vyas <tarun.vyas@intel.com> > --- > drivers/gpu/drm/i915/intel_sprite.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index aa1dfaa692b9..9cd4be020840 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -131,7 +131,7 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state) > if (scanline < min || scanline > max) > break; > > - if (timeout <= 0) { > + if (!timeout) { > DRM_ERROR("Potential atomic update failure on pipe %c\n", > pipe_name(crtc->pipe)); > break; > -- > 2.13.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index aa1dfaa692b9..9cd4be020840 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -131,7 +131,7 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state) if (scanline < min || scanline > max) break; - if (timeout <= 0) { + if (!timeout) { DRM_ERROR("Potential atomic update failure on pipe %c\n", pipe_name(crtc->pipe)); break;
Just a minor knit. Stumbled across the kernel doc for schedule_timeout() which quotes "In all cases the return value is guaranteed to be non-negative". Also, the return code of schedule_timeout() already checks for negative values "return timeout < 0 ? 0 : timeout;" and returns 0 in such cases. So, let's do away with the redundant check for an atomic pipe update. Signed-off-by: Tarun Vyas <tarun.vyas@intel.com> --- drivers/gpu/drm/i915/intel_sprite.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)