[PATCH 1/2] drm/i915: compute wait_ioctl timeout correctly

[PATCH 1/2] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

We've lost the +1 required for correct timeouts in

commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Wed Jul 16 21:05:06 2014 +0000

    drm: i915: Use nsec based interfaces

    Use ktime_get_raw_ns() and get rid of the back and forth timespec
    conversions.

    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: John Stultz <john.stultz@linaro.org>

So fix this up by reinstating our handrolled _timeout function. While
at it bother with handling MAX_JIFFIES.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 10 ++++++++++
 drivers/gpu/drm/i915/i915_gem.c |  3 ++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 02b3cb32c8a6..caae337c0199 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3030,6 +3030,16 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
 	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
 }
 
+static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
+{
+	unsigned long j = nsecs_to_jiffies(m);
+
+	if (m > (u64)jiffies_to_usecs(MAX_JIFFY_OFFSET) * 1000)
+		return MAX_JIFFY_OFFSET;
+
+	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
+}
+
 static inline unsigned long
 timespec_to_jiffies_timeout(const struct timespec *value)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 23e17735a6a9..002730d1409b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1225,7 +1225,8 @@ int __i915_wait_seqno(struct intel_engine_cs *ring, u32 seqno,
 	if (i915_seqno_passed(ring->get_seqno(ring, true), seqno))
 		return 0;
 
-	timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
+	timeout_expire = timeout ?
+		jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
 
 	if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
 		gen6_rps_boost(dev_priv);
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: Handle inaccurate time conversion issues

[Daniel Vetter]

So apparently jiffies<->nsec<->ktime isn't accurate or something. At
elast if we timeout there's occasionally still a few hundred us left
(in a 2 second timeout).

Stuff I've tried and thrown out again:
- Sampling the before timestamp before jiffies. Doesn't improve test
  path rate at all.
- Using jiffies. Way to inaccurate, which means way too much drift
  with signals plus automatic ioctl restarting in userspace. In
  hindsight we should have used an absolute timeout, but hey we need
  something for v3 of the i915 gem wait interfaces ;-)
- Trying to figure out where accuracy gets lost. gl testcase really
  don't care all that much about this (as long as isn't not massively
  off), it's just that the testcase gets a bit upset if it receives an
  EITME with timeout > 0.

So as long as we're in the ballbark it's good enough. So patch
everything up if we're at most one jiffies off. I get's me a solid
test again.

This regression is probably introduced in

commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Wed Jul 16 21:05:06 2014 +0000

    drm: i915: Use nsec based interfaces

    Use ktime_get_raw_ns() and get rid of the back and forth timespec
    conversions.

    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: John Stultz <john.stultz@linaro.org>

Probably because I'm too lazy to confirm myself and still waiting for
QA ;-)

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 002730d1409b..abdc5bcdbd0d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1302,6 +1302,16 @@ int __i915_wait_seqno(struct intel_engine_cs *ring, u32 seqno,
 		s64 tres = *timeout - (now - before);
 
 		*timeout = tres < 0 ? 0 : tres;
+
+		/*
+		 * Apparently ktime isn't accurate enough and occasionally has a
+		 * bit of mismatch in the jiffies<->nsecs<->ktime loop. So patch
+		 * things up to make the test happy. We allow up to 1 jiffy.
+		 *
+		 * This is a regrssion from the timespec->ktime conversion.
+		 */
+		if (ret == -ETIME && *timeout < jiffies_to_usecs(1)*1000)
+			*timeout = 0;
 	}
 
 	return ret;
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: compute wait_ioctl timeout correctly

[Chris Wilson]

On Fri, Nov 28, 2014 at 10:29:54AM +0100, Daniel Vetter wrote:
> We've lost the +1 required for correct timeouts in
> 
> commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Wed Jul 16 21:05:06 2014 +0000
> 
>     drm: i915: Use nsec based interfaces
> 
>     Use ktime_get_raw_ns() and get rid of the back and forth timespec
>     conversions.
> 
>     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>     Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>     Signed-off-by: John Stultz <john.stultz@linaro.org>
> 
> So fix this up by reinstating our handrolled _timeout function. While
> at it bother with handling MAX_JIFFIES.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 10 ++++++++++
>  drivers/gpu/drm/i915/i915_gem.c |  3 ++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 02b3cb32c8a6..caae337c0199 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3030,6 +3030,16 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
>  	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>  }
>  
> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> +{
> +	unsigned long j = nsecs_to_jiffies(m);

Still u64 here.
> +
> +	if (m > (u64)jiffies_to_usecs(MAX_JIFFY_OFFSET) * 1000)
> +		return MAX_JIFFY_OFFSET;
> +
> +	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);

These two tests are doing identical jobs, so the first one is redundant
so long as we maintain u64 until the final check?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Handle inaccurate time conversion issues

[Chris Wilson]

On Fri, Nov 28, 2014 at 10:29:55AM +0100, Daniel Vetter wrote:
> So apparently jiffies<->nsec<->ktime isn't accurate or something. At
> elast if we timeout there's occasionally still a few hundred us left
> (in a 2 second timeout).
> 
> Stuff I've tried and thrown out again:
> - Sampling the before timestamp before jiffies. Doesn't improve test
>   path rate at all.
> - Using jiffies. Way to inaccurate, which means way too much drift
>   with signals plus automatic ioctl restarting in userspace. In
>   hindsight we should have used an absolute timeout, but hey we need
>   something for v3 of the i915 gem wait interfaces ;-)

I am still trying to get my head around an ioctl that only works in
jiffie intervals where the jiffie is too inaccurate.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Handle inaccurate time conversion issues

[Chris Wilson]

On Fri, Nov 28, 2014 at 10:29:55AM +0100, Daniel Vetter wrote:
>   In
>   hindsight we should have used an absolute timeout, but hey we need
>   something for v3 of the i915 gem wait interfaces ;-)

I think this is actually a more interesting solution: to change
internally to using an absolute timeout value and leave the massaging
between relative/absolute in the wait_ioctl caller.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Handle inaccurate time conversion issues

[Daniel Vetter]

On Fri, Nov 28, 2014 at 12:08 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> - Using jiffies. Way to inaccurate, which means way too much drift
>>   with signals plus automatic ioctl restarting in userspace. In
>>   hindsight we should have used an absolute timeout, but hey we need
>>   something for v3 of the i915 gem wait interfaces ;-)
>
> I am still trying to get my head around an ioctl that only works in
> jiffie intervals where the jiffie is too inaccurate.

The problem is the rounding down behaviour of jiffies - if you wake up
sufficiently often compared to HZ that adds up surprisingly fast.
Adding a flag for absolute timeouts might work better, but then we'd
still need to make sure that with the back&forth conversion we don't
end up rounding all the time.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: compute wait_ioctl timeout correctly

[Dave Gordon]

On 28/11/14 09:29, Daniel Vetter wrote:
> We've lost the +1 required for correct timeouts in
> 
> commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Wed Jul 16 21:05:06 2014 +0000
> 
>     drm: i915: Use nsec based interfaces
> 
>     Use ktime_get_raw_ns() and get rid of the back and forth timespec
>     conversions.
> 
>     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>     Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>     Signed-off-by: John Stultz <john.stultz@linaro.org>
> 
> So fix this up by reinstating our handrolled _timeout function. While
> at it bother with handling MAX_JIFFIES.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 10 ++++++++++
>  drivers/gpu/drm/i915/i915_gem.c |  3 ++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 02b3cb32c8a6..caae337c0199 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3030,6 +3030,16 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
>  	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>  }
>  
> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> +{
> +	unsigned long j = nsecs_to_jiffies(m);

nsecs_to_jiffies() may be (relatively) expensive (mul/div/etc), so I'd
be inclined to move the call until after the test below. It would be
nice if the test turned into a single comparison, since the RHS is a
constant for a given kernel build; but it looks like jiffies_to_usecs()
isn't expanded inline, since it's in time.c :-( In which case swapping
the lines around may also help the compiler keep 'j' live.

> +	if (m > (u64)jiffies_to_usecs(MAX_JIFFY_OFFSET) * 1000)

I think there's a problem with this line anyway. In kernel/time/time.c:

// Warning! Result type may be narrower than parameter type - DSG
unsigned int jiffies_to_usecs(const unsigned long j)
{
#if HZ <= USEC_PER_SEC && !(USEC_PER_SEC % HZ)
        return (USEC_PER_SEC / HZ) * j;
#elif HZ > USEC_PER_SEC && !(HZ % USEC_PER_SEC)
        return (j + (HZ / USEC_PER_SEC) - 1)/(HZ / USEC_PER_SEC);
#else
# if BITS_PER_LONG == 32
        return (HZ_TO_USEC_MUL32 * j) >> HZ_TO_USEC_SHR32;
# else
        return (j * HZ_TO_USEC_NUM) / HZ_TO_USEC_DEN;
# endif
#endif
}

Also, include/linux/jiffies.h:

#define MAX_JIFFY_OFFSET ((LONG_MAX >> 1)-1)

and include/linux/kernel.h:

#define LONG_MAX        ((long)(~0UL>>1))

So, on a 64-bit build we'll have LONG_MAX == 0x7fff_ffff_ffff_ffff and
MAX_JIFFY_OFFSET == 0x3fff_ffff_ffff_fffe. Multiplying that by 1000
gives an answer that doesn't fit in an unsigned int!

Even on a 32-bit build (where LONG_MAX == 0x7fff_ffff and
MAX_JIFFY_OFFSET == 0x3fff_fffe) MAX_JIFFY_OFFSET can't be multiplied by
any typical value of HZ (50, 60, 1000) without overflow!

I think the only way to get this right, give the somewhat broken nature
of the kernel function signatures and its lack of a u64 jiffies-to-nsecs
function, is to convert ONE jiffy to (unsigned int) usecs,
then widen to u64 before converting to nsecs and using that for the rest
of the calculations.

.Dave.

> +		return MAX_JIFFY_OFFSET;
> +
> +	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> +}

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Handle inaccurate time conversion

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -2              364/364              362/364
ILK              +1                 365/366              366/366
SNB                                  450/450              450/450
IVB                                  498/498              498/498
BYT                                  289/289              289/289
HSW                                  564/564              564/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gem_reloc_overflow_source-offset-big-reloc-gtt      PASS(2, M25)      NO_RESULT(1, M25)
*PNV  igt_gem_ringfill_blitter      PASS(2, M25)      NO_RESULT(1, M25)
 ILK  igt_kms_flip_wf_vblank-ts-check      DMESG_WARN(2, M26)PASS(4, M26M37)      PASS(1, M37)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

On Fri, Nov 28, 2014 at 01:46:27PM +0000, Dave Gordon wrote:
> On 28/11/14 09:29, Daniel Vetter wrote:
> > We've lost the +1 required for correct timeouts in
> > 
> > commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
> > Author: Thomas Gleixner <tglx@linutronix.de>
> > Date:   Wed Jul 16 21:05:06 2014 +0000
> > 
> >     drm: i915: Use nsec based interfaces
> > 
> >     Use ktime_get_raw_ns() and get rid of the back and forth timespec
> >     conversions.
> > 
> >     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> >     Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >     Signed-off-by: John Stultz <john.stultz@linaro.org>
> > 
> > So fix this up by reinstating our handrolled _timeout function. While
> > at it bother with handling MAX_JIFFIES.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: John Stultz <john.stultz@linaro.org>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h | 10 ++++++++++
> >  drivers/gpu/drm/i915/i915_gem.c |  3 ++-
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 02b3cb32c8a6..caae337c0199 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3030,6 +3030,16 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
> >  	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> >  }
> >  
> > +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> > +{
> > +	unsigned long j = nsecs_to_jiffies(m);
> 
> nsecs_to_jiffies() may be (relatively) expensive (mul/div/etc), so I'd
> be inclined to move the call until after the test below. It would be
> nice if the test turned into a single comparison, since the RHS is a
> constant for a given kernel build; but it looks like jiffies_to_usecs()
> isn't expanded inline, since it's in time.c :-( In which case swapping
> the lines around may also help the compiler keep 'j' live.

This is only called in code that's about to sleep. Wasting a few cpu
cycles is totally ok ;-) This is also the pattern the non-_timeout
functions in time.c use, so I think we should be ok

> > +	if (m > (u64)jiffies_to_usecs(MAX_JIFFY_OFFSET) * 1000)
> 
> I think there's a problem with this line anyway. In kernel/time/time.c:
> 
> // Warning! Result type may be narrower than parameter type - DSG
> unsigned int jiffies_to_usecs(const unsigned long j)
> {
> #if HZ <= USEC_PER_SEC && !(USEC_PER_SEC % HZ)
>         return (USEC_PER_SEC / HZ) * j;
> #elif HZ > USEC_PER_SEC && !(HZ % USEC_PER_SEC)
>         return (j + (HZ / USEC_PER_SEC) - 1)/(HZ / USEC_PER_SEC);
> #else
> # if BITS_PER_LONG == 32
>         return (HZ_TO_USEC_MUL32 * j) >> HZ_TO_USEC_SHR32;
> # else
>         return (j * HZ_TO_USEC_NUM) / HZ_TO_USEC_DEN;
> # endif
> #endif
> }
> 
> Also, include/linux/jiffies.h:
> 
> #define MAX_JIFFY_OFFSET ((LONG_MAX >> 1)-1)
> 
> and include/linux/kernel.h:
> 
> #define LONG_MAX        ((long)(~0UL>>1))
> 
> So, on a 64-bit build we'll have LONG_MAX == 0x7fff_ffff_ffff_ffff and
> MAX_JIFFY_OFFSET == 0x3fff_ffff_ffff_fffe. Multiplying that by 1000
> gives an answer that doesn't fit in an unsigned int!

Oh blergh I've missed that jiffies is a long.

> Even on a 32-bit build (where LONG_MAX == 0x7fff_ffff and
> MAX_JIFFY_OFFSET == 0x3fff_fffe) MAX_JIFFY_OFFSET can't be multiplied by
> any typical value of HZ (50, 60, 1000) without overflow!

Cast operator binds tigther than *, so this case actually works.

> I think the only way to get this right, give the somewhat broken nature
> of the kernel function signatures and its lack of a u64 jiffies-to-nsecs
> function, is to convert ONE jiffy to (unsigned int) usecs,
> then widen to u64 before converting to nsecs and using that for the rest
> of the calculations.

I think this still overflows. I'm somewhat inclined to just tell userspace
to sod off for large timeout values since we really don't care about
those. But I'll see whether I can fix it up first without too much
trouble.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

We've lost the +1 required for correct timeouts in

commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Wed Jul 16 21:05:06 2014 +0000

    drm: i915: Use nsec based interfaces

    Use ktime_get_raw_ns() and get rid of the back and forth timespec
    conversions.

    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: John Stultz <john.stultz@linaro.org>

So fix this up by reinstating our handrolled _timeout function. While
at it bother with handling MAX_JIFFIES.

v2: Convert to usecs (we don't care about the accuracy anyway) first
to avoid overflow issues Dave Gordon spotted.

Cc: Dave Gordon <david.s.gordon@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 11 +++++++++++
 drivers/gpu/drm/i915/i915_gem.c |  3 ++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 049482f5d9ac..ef1f00e0a7b3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3097,6 +3097,17 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
 	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
 }
 
+static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
+{
+	u64 usecs = div_u64(m + 999, 1000);
+	unsigned long j = usecs_to_jiffies(usecs);
+
+	if (usecs > (u64)jiffies_to_usecs(MAX_JIFFY_OFFSET))
+		return MAX_JIFFY_OFFSET;
+
+	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
+}
+
 static inline unsigned long
 timespec_to_jiffies_timeout(const struct timespec *value)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9d362d320d82..04a9f26407ae 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1226,7 +1226,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
 	if (i915_gem_request_completed(req, true))
 		return 0;
 
-	timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
+	timeout_expire = timeout ?
+		jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
 
 	if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
 		gen6_rps_boost(dev_priv);
-- 
1.9.3

[PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

We've lost the +1 required for correct timeouts in

commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Wed Jul 16 21:05:06 2014 +0000

    drm: i915: Use nsec based interfaces

    Use ktime_get_raw_ns() and get rid of the back and forth timespec
    conversions.

    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: John Stultz <john.stultz@linaro.org>

So fix this up by reinstating our handrolled _timeout function. While
at it bother with handling MAX_JIFFIES.

v2: Convert to usecs (we don't care about the accuracy anyway) first
to avoid overflow issues Dave Gordon spotted.

Cc: Dave Gordon <david.s.gordon@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 11 +++++++++++
 drivers/gpu/drm/i915/i915_gem.c |  3 ++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 049482f5d9ac..ef1f00e0a7b3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3097,6 +3097,17 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
 	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
 }
 
+static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
+{
+	u64 usecs = div_u64(m + 999, 1000);
+	unsigned long j = usecs_to_jiffies(usecs);
+
+	if (usecs > (u64)jiffies_to_usecs(MAX_JIFFY_OFFSET))
+		return MAX_JIFFY_OFFSET;
+
+	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
+}
+
 static inline unsigned long
 timespec_to_jiffies_timeout(const struct timespec *value)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9d362d320d82..04a9f26407ae 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1226,7 +1226,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
 	if (i915_gem_request_completed(req, true))
 		return 0;
 
-	timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
+	timeout_expire = timeout ?
+		jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
 
 	if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
 		gen6_rps_boost(dev_priv);
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

We've lost the +1 required for correct timeouts in

commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Wed Jul 16 21:05:06 2014 +0000

    drm: i915: Use nsec based interfaces

    Use ktime_get_raw_ns() and get rid of the back and forth timespec
    conversions.

    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: John Stultz <john.stultz@linaro.org>

So fix this up by reinstating our handrolled _timeout function. While
at it bother with handling MAX_JIFFIES.

v2: Convert to usecs (we don't care about the accuracy anyway) first
to avoid overflow issues Dave Gordon spotted.

v3: Drop the explicit MAX_JIFFY_OFFSET check, usecs_to_jiffies should
take care of that already. It might be a bit too enthusiastic about it
though.

Cc: Dave Gordon <david.s.gordon@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 8 ++++++++
 drivers/gpu/drm/i915/i915_gem.c | 3 ++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 049482f5d9ac..4ea14a8c31f7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3097,6 +3097,14 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
 	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
 }
 
+static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
+{
+	u64 usecs = div_u64(m + 999, 1000);
+	unsigned long j = usecs_to_jiffies(usecs);
+
+	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
+}
+
 static inline unsigned long
 timespec_to_jiffies_timeout(const struct timespec *value)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9d362d320d82..04a9f26407ae 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1226,7 +1226,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
 	if (i915_gem_request_completed(req, true))
 		return 0;
 
-	timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
+	timeout_expire = timeout ?
+		jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
 
 	if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
 		gen6_rps_boost(dev_priv);
-- 
1.9.3

[PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

We've lost the +1 required for correct timeouts in

commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Wed Jul 16 21:05:06 2014 +0000

    drm: i915: Use nsec based interfaces

    Use ktime_get_raw_ns() and get rid of the back and forth timespec
    conversions.

    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: John Stultz <john.stultz@linaro.org>

So fix this up by reinstating our handrolled _timeout function. While
at it bother with handling MAX_JIFFIES.

v2: Convert to usecs (we don't care about the accuracy anyway) first
to avoid overflow issues Dave Gordon spotted.

v3: Drop the explicit MAX_JIFFY_OFFSET check, usecs_to_jiffies should
take care of that already. It might be a bit too enthusiastic about it
though.

Cc: Dave Gordon <david.s.gordon@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 8 ++++++++
 drivers/gpu/drm/i915/i915_gem.c | 3 ++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 049482f5d9ac..4ea14a8c31f7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3097,6 +3097,14 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
 	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
 }
 
+static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
+{
+	u64 usecs = div_u64(m + 999, 1000);
+	unsigned long j = usecs_to_jiffies(usecs);
+
+	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
+}
+
 static inline unsigned long
 timespec_to_jiffies_timeout(const struct timespec *value)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9d362d320d82..04a9f26407ae 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1226,7 +1226,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
 	if (i915_gem_request_completed(req, true))
 		return 0;
 
-	timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
+	timeout_expire = timeout ?
+		jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
 
 	if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
 		gen6_rps_boost(dev_priv);
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Chris Wilson]

On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> We've lost the +1 required for correct timeouts in
> 
> commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Wed Jul 16 21:05:06 2014 +0000
> 
>     drm: i915: Use nsec based interfaces
> 
>     Use ktime_get_raw_ns() and get rid of the back and forth timespec
>     conversions.
> 
>     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>     Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>     Signed-off-by: John Stultz <john.stultz@linaro.org>
> 
> So fix this up by reinstating our handrolled _timeout function. While
> at it bother with handling MAX_JIFFIES.
> 
> v2: Convert to usecs (we don't care about the accuracy anyway) first
> to avoid overflow issues Dave Gordon spotted.
> 
> v3: Drop the explicit MAX_JIFFY_OFFSET check, usecs_to_jiffies should
> take care of that already. It might be a bit too enthusiastic about it
> though.
> 
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 8 ++++++++
>  drivers/gpu/drm/i915/i915_gem.c | 3 ++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 049482f5d9ac..4ea14a8c31f7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3097,6 +3097,14 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
>  	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>  }
>  
> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> +{
> +	u64 usecs = div_u64(m + 999, 1000);
> +	unsigned long j = usecs_to_jiffies(usecs);
> +
> +	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);

Or more concisely and review friendly:

static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
{
	return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
}
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Chris Wilson]

On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> We've lost the +1 required for correct timeouts in
> 
> commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Wed Jul 16 21:05:06 2014 +0000
> 
>     drm: i915: Use nsec based interfaces
> 
>     Use ktime_get_raw_ns() and get rid of the back and forth timespec
>     conversions.
> 
>     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>     Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>     Signed-off-by: John Stultz <john.stultz@linaro.org>
> 
> So fix this up by reinstating our handrolled _timeout function. While
> at it bother with handling MAX_JIFFIES.
> 
> v2: Convert to usecs (we don't care about the accuracy anyway) first
> to avoid overflow issues Dave Gordon spotted.
> 
> v3: Drop the explicit MAX_JIFFY_OFFSET check, usecs_to_jiffies should
> take care of that already. It might be a bit too enthusiastic about it
> though.
> 
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 8 ++++++++
>  drivers/gpu/drm/i915/i915_gem.c | 3 ++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 049482f5d9ac..4ea14a8c31f7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3097,6 +3097,14 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
>  	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>  }
>  
> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> +{
> +	u64 usecs = div_u64(m + 999, 1000);
> +	unsigned long j = usecs_to_jiffies(usecs);
> +
> +	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);

Or more concisely and review friendly:

static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
{
	return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
}
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[John Stultz]

On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
>> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
>> +{
>> +     u64 usecs = div_u64(m + 999, 1000);
>> +     unsigned long j = usecs_to_jiffies(usecs);
>> +
>> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>
> Or more concisely and review friendly:
>
> static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> {
>         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> }

Yea. This looks much nicer. Seems generic enough it might be better
added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
rather then in a driver header.

And clearly the header comment in nsec_to_jiffies() warning its only
for the scheduler and not for use for drivers (for exactly the reason
of this patch) are not obvious/memorable enough for me and Thomas
makes me wonder if we should change its name to be more clear that its
a sched only function.

thanks
-john

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  364/364              364/364
ILK              +1-1              365/366              365/366
SNB                                  450/450              450/450
IVB                                  498/498              498/498
BYT                                  289/289              289/289
HSW                                  564/564              564/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt_kms_flip_rcs-flip-vs-panning-interruptible      PASS(3, M26)      DMESG_WARN(1, M26)
 ILK  igt_kms_flip_wf_vblank-ts-check      DMESG_WARN(2, M26)PASS(12, M26M37)      PASS(1, M26)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -2              364/364              362/364
ILK              +1                 365/366              366/366
SNB                                  450/450              450/450
IVB                 -17              498/498              481/498
BYT                                  289/289              289/289
HSW                                  564/564              564/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_drm_vma_limiter      PASS(3, M25M7M23)      NO_RESULT(1, M23)
*PNV  igt_gem_partial_pwrite_pread_reads      PASS(2, M25M23)      NO_RESULT(1, M23)
 ILK  igt_kms_flip_wf_vblank-ts-check      DMESG_WARN(2, M26)PASS(12, M26M37)      PASS(1, M37)
 IVB  igt_kms_3d      DMESG_WARN(4, M34)PASS(1, M34)      DMESG_WARN(1, M34)
 IVB  igt_kms_cursor_crc_cursor-128x128-onscreen      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_cursor_crc_cursor-128x128-random      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_cursor_crc_cursor-128x128-sliding      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_cursor_crc_cursor-256x256-offscreen      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_cursor_crc_cursor-256x256-onscreen      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_cursor_crc_cursor-256x256-sliding      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_cursor_crc_cursor-64x64-offscreen      NSPT(4, M34)PASS(2, M34M21)      NSPT(1, M34)
 IVB  igt_kms_cursor_crc_cursor-64x64-onscreen      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_cursor_crc_cursor-64x64-random      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_cursor_crc_cursor-64x64-sliding      FAIL(1, M21)NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_cursor_crc_cursor-size-change      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_fence_pin_leak      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_rotation_crc_primary-rotation      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
 IVB  igt_kms_rotation_crc_sprite-rotation      NSPT(4, M34)PASS(1, M34)      NSPT(1, M34)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> >> +{
> >> +     u64 usecs = div_u64(m + 999, 1000);
> >> +     unsigned long j = usecs_to_jiffies(usecs);
> >> +
> >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> >
> > Or more concisely and review friendly:
> >
> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> > {
> >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> > }
> 
> Yea. This looks much nicer. Seems generic enough it might be better
> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
> rather then in a driver header.
> 
> And clearly the header comment in nsec_to_jiffies() warning its only
> for the scheduler and not for use for drivers (for exactly the reason
> of this patch) are not obvious/memorable enough for me and Thomas
> makes me wonder if we should change its name to be more clear that its
> a sched only function.

This bug here isn't about nsect_to_jiffies vs the 64 bit variant, but
about the +1 that we need to not have a short sleep. In i915 we have a
bunch of jiffies_timeout functions which do just the +1 compared to the
versions in time.c because we screwed this up too often.

Iirc I did float an rfc to move these to time.c once but it resulted in
some bikeshed fest (no, I'm not going to audit every single user of
existing _to_jiffies functions). If there's interest I could try again,
the i915 versions are in drivers/gpu/drm/i915/i915_drv.h.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> >> +{
> >> +     u64 usecs = div_u64(m + 999, 1000);
> >> +     unsigned long j = usecs_to_jiffies(usecs);
> >> +
> >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> >
> > Or more concisely and review friendly:
> >
> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> > {
> >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> > }
> 
> Yea. This looks much nicer. Seems generic enough it might be better
> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
> rather then in a driver header.
> 
> And clearly the header comment in nsec_to_jiffies() warning its only
> for the scheduler and not for use for drivers (for exactly the reason
> of this patch) are not obvious/memorable enough for me and Thomas
> makes me wonder if we should change its name to be more clear that its
> a sched only function.

This bug here isn't about nsect_to_jiffies vs the 64 bit variant, but
about the +1 that we need to not have a short sleep. In i915 we have a
bunch of jiffies_timeout functions which do just the +1 compared to the
versions in time.c because we screwed this up too often.

Iirc I did float an rfc to move these to time.c once but it resulted in
some bikeshed fest (no, I'm not going to audit every single user of
existing _to_jiffies functions). If there's interest I could try again,
the i915 versions are in drivers/gpu/drm/i915/i915_drv.h.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Imre Deak]

On Wed, 2014-12-03 at 10:22 +0100, Daniel Vetter wrote:
> On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
> > On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> > >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> > >> +{
> > >> +     u64 usecs = div_u64(m + 999, 1000);
> > >> +     unsigned long j = usecs_to_jiffies(usecs);
> > >> +
> > >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> > >
> > > Or more concisely and review friendly:
> > >
> > > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> > > {
> > >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> > > }
> > 
> > Yea. This looks much nicer. Seems generic enough it might be better
> > added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
> > rather then in a driver header.
> > 
> > And clearly the header comment in nsec_to_jiffies() warning its only
> > for the scheduler and not for use for drivers (for exactly the reason
> > of this patch) are not obvious/memorable enough for me and Thomas
> > makes me wonder if we should change its name to be more clear that its
> > a sched only function.
> 
> This bug here isn't about nsect_to_jiffies vs the 64 bit variant, but
> about the +1 that we need to not have a short sleep. In i915 we have a
> bunch of jiffies_timeout functions which do just the +1 compared to the
> versions in time.c because we screwed this up too often.
> 
> Iirc I did float an rfc to move these to time.c once but it resulted in
> some bikeshed fest (no, I'm not going to audit every single user of
> existing _to_jiffies functions). If there's interest I could try again,
> the i915 versions are in drivers/gpu/drm/i915/i915_drv.h.

There was at least this attempt:
https://lkml.org/lkml/2013/5/10/187

--Imre

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Imre Deak]

On Wed, 2014-12-03 at 10:22 +0100, Daniel Vetter wrote:
> On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
> > On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> > >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> > >> +{
> > >> +     u64 usecs = div_u64(m + 999, 1000);
> > >> +     unsigned long j = usecs_to_jiffies(usecs);
> > >> +
> > >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> > >
> > > Or more concisely and review friendly:
> > >
> > > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> > > {
> > >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> > > }
> > 
> > Yea. This looks much nicer. Seems generic enough it might be better
> > added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
> > rather then in a driver header.
> > 
> > And clearly the header comment in nsec_to_jiffies() warning its only
> > for the scheduler and not for use for drivers (for exactly the reason
> > of this patch) are not obvious/memorable enough for me and Thomas
> > makes me wonder if we should change its name to be more clear that its
> > a sched only function.
> 
> This bug here isn't about nsect_to_jiffies vs the 64 bit variant, but
> about the +1 that we need to not have a short sleep. In i915 we have a
> bunch of jiffies_timeout functions which do just the +1 compared to the
> versions in time.c because we screwed this up too often.
> 
> Iirc I did float an rfc to move these to time.c once but it resulted in
> some bikeshed fest (no, I'm not going to audit every single user of
> existing _to_jiffies functions). If there's interest I could try again,
> the i915 versions are in drivers/gpu/drm/i915/i915_drv.h.

There was at least this attempt:
https://lkml.org/lkml/2013/5/10/187

--Imre


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> >> +{
> >> +     u64 usecs = div_u64(m + 999, 1000);
> >> +     unsigned long j = usecs_to_jiffies(usecs);
> >> +
> >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> >
> > Or more concisely and review friendly:
> >
> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> > {
> >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> > }
> 
> Yea. This looks much nicer. Seems generic enough it might be better
> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
> rather then in a driver header.

Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
"Yea" above as an ack for adding that and pulling it in through
drm-intel.git?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> >> +{
> >> +     u64 usecs = div_u64(m + 999, 1000);
> >> +     unsigned long j = usecs_to_jiffies(usecs);
> >> +
> >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> >
> > Or more concisely and review friendly:
> >
> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> > {
> >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> > }
> 
> Yea. This looks much nicer. Seems generic enough it might be better
> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
> rather then in a driver header.

Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
"Yea" above as an ack for adding that and pulling it in through
drm-intel.git?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[John Stultz]

On Wed, Dec 3, 2014 at 6:30 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
>> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
>> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
>> >> +{
>> >> +     u64 usecs = div_u64(m + 999, 1000);
>> >> +     unsigned long j = usecs_to_jiffies(usecs);
>> >> +
>> >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>> >
>> > Or more concisely and review friendly:
>> >
>> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
>> > {
>> >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
>> > }
>>
>> Yea. This looks much nicer. Seems generic enough it might be better
>> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
>> rather then in a driver header.
>
> Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
> "Yea" above as an ack for adding that and pulling it in through
> drm-intel.git?

Do you need an EXPORT_SYMBOL if you add the _timeout version next to
nsecs_to_jiffies64 in time.c?

Otherwise no objections to the approach, but I'd like to properly do
an Acked-by: after I see the patch. :)

thanks
-john

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[John Stultz]

On Wed, Dec 3, 2014 at 6:30 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
>> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
>> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
>> >> +{
>> >> +     u64 usecs = div_u64(m + 999, 1000);
>> >> +     unsigned long j = usecs_to_jiffies(usecs);
>> >> +
>> >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>> >
>> > Or more concisely and review friendly:
>> >
>> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
>> > {
>> >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
>> > }
>>
>> Yea. This looks much nicer. Seems generic enough it might be better
>> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
>> rather then in a driver header.
>
> Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
> "Yea" above as an ack for adding that and pulling it in through
> drm-intel.git?

Do you need an EXPORT_SYMBOL if you add the _timeout version next to
nsecs_to_jiffies64 in time.c?

Otherwise no objections to the approach, but I'd like to properly do
an Acked-by: after I see the patch. :)

thanks
-john
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

We've lost the +1 required for correct timeouts in

commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Wed Jul 16 21:05:06 2014 +0000

    drm: i915: Use nsec based interfaces

    Use ktime_get_raw_ns() and get rid of the back and forth timespec
    conversions.

    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: John Stultz <john.stultz@linaro.org>

So fix this up by reinstating our handrolled _timeout function. While
at it bother with handling MAX_JIFFIES.

v2: Convert to usecs (we don't care about the accuracy anyway) first
to avoid overflow issues Dave Gordon spotted.

v3: Drop the explicit MAX_JIFFY_OFFSET check, usecs_to_jiffies should
take care of that already. It might be a bit too enthusiastic about it
though.

v4: Chris has a much nicer color, so use his implementation.

This requires to export nsec_to_jiffies from time.c.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Gordon <david.s.gordon@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 5 +++++
 drivers/gpu/drm/i915/i915_gem.c | 3 ++-
 kernel/time/time.c              | 1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 049482f5d9ac..564a45f4a0ab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3097,6 +3097,11 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
 	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
 }
 
+static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
+{
+        return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
+}
+
 static inline unsigned long
 timespec_to_jiffies_timeout(const struct timespec *value)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9d362d320d82..04a9f26407ae 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1226,7 +1226,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
 	if (i915_gem_request_completed(req, true))
 		return 0;
 
-	timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
+	timeout_expire = timeout ?
+		jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
 
 	if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
 		gen6_rps_boost(dev_priv);
diff --git a/kernel/time/time.c b/kernel/time/time.c
index a9ae20fb0b11..8fae82ca5cbf 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -745,6 +745,7 @@ u64 nsecs_to_jiffies64(u64 n)
 	return div_u64(n * 9, (9ull * NSEC_PER_SEC + HZ / 2) / HZ);
 #endif
 }
+EXPORT_SYMBOL(nsecs_to_jiffies64);
 
 /**
  * nsecs_to_jiffies - Convert nsecs in u64 to jiffies
-- 
1.9.3

[PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

We've lost the +1 required for correct timeouts in

commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Wed Jul 16 21:05:06 2014 +0000

    drm: i915: Use nsec based interfaces

    Use ktime_get_raw_ns() and get rid of the back and forth timespec
    conversions.

    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: John Stultz <john.stultz@linaro.org>

So fix this up by reinstating our handrolled _timeout function. While
at it bother with handling MAX_JIFFIES.

v2: Convert to usecs (we don't care about the accuracy anyway) first
to avoid overflow issues Dave Gordon spotted.

v3: Drop the explicit MAX_JIFFY_OFFSET check, usecs_to_jiffies should
take care of that already. It might be a bit too enthusiastic about it
though.

v4: Chris has a much nicer color, so use his implementation.

This requires to export nsec_to_jiffies from time.c.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Gordon <david.s.gordon@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 5 +++++
 drivers/gpu/drm/i915/i915_gem.c | 3 ++-
 kernel/time/time.c              | 1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 049482f5d9ac..564a45f4a0ab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3097,6 +3097,11 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
 	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
 }
 
+static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
+{
+        return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
+}
+
 static inline unsigned long
 timespec_to_jiffies_timeout(const struct timespec *value)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9d362d320d82..04a9f26407ae 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1226,7 +1226,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
 	if (i915_gem_request_completed(req, true))
 		return 0;
 
-	timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
+	timeout_expire = timeout ?
+		jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
 
 	if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
 		gen6_rps_boost(dev_priv);
diff --git a/kernel/time/time.c b/kernel/time/time.c
index a9ae20fb0b11..8fae82ca5cbf 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -745,6 +745,7 @@ u64 nsecs_to_jiffies64(u64 n)
 	return div_u64(n * 9, (9ull * NSEC_PER_SEC + HZ / 2) / HZ);
 #endif
 }
+EXPORT_SYMBOL(nsecs_to_jiffies64);
 
 /**
  * nsecs_to_jiffies - Convert nsecs in u64 to jiffies
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

On Wed, Dec 03, 2014 at 11:07:08AM -0800, John Stultz wrote:
> On Wed, Dec 3, 2014 at 6:30 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
> >> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> >> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> >> >> +{
> >> >> +     u64 usecs = div_u64(m + 999, 1000);
> >> >> +     unsigned long j = usecs_to_jiffies(usecs);
> >> >> +
> >> >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> >> >
> >> > Or more concisely and review friendly:
> >> >
> >> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> >> > {
> >> >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> >> > }
> >>
> >> Yea. This looks much nicer. Seems generic enough it might be better
> >> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
> >> rather then in a driver header.
> >
> > Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
> > "Yea" above as an ack for adding that and pulling it in through
> > drm-intel.git?
> 
> Do you need an EXPORT_SYMBOL if you add the _timeout version next to
> nsecs_to_jiffies64 in time.c?

I wouldn't but the patch from Imre to add all the _timeout was killed with
a few bikesheds so really not volunteering. And just moving this single
one doesn't make a lot of sense imo. Also the next patch I'll do is just
add the +1 that we lost to the code and call it a day, really ;-)

> Otherwise no objections to the approach, but I'd like to properly do
> an Acked-by: after I see the patch. :)

I'll send it out.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

On Wed, Dec 03, 2014 at 11:07:08AM -0800, John Stultz wrote:
> On Wed, Dec 3, 2014 at 6:30 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
> >> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
> >> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
> >> >> +{
> >> >> +     u64 usecs = div_u64(m + 999, 1000);
> >> >> +     unsigned long j = usecs_to_jiffies(usecs);
> >> >> +
> >> >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
> >> >
> >> > Or more concisely and review friendly:
> >> >
> >> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> >> > {
> >> >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> >> > }
> >>
> >> Yea. This looks much nicer. Seems generic enough it might be better
> >> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
> >> rather then in a driver header.
> >
> > Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
> > "Yea" above as an ack for adding that and pulling it in through
> > drm-intel.git?
> 
> Do you need an EXPORT_SYMBOL if you add the _timeout version next to
> nsecs_to_jiffies64 in time.c?

I wouldn't but the patch from Imre to add all the _timeout was killed with
a few bikesheds so really not volunteering. And just moving this single
one doesn't make a lot of sense imo. Also the next patch I'll do is just
add the +1 that we lost to the code and call it a day, really ;-)

> Otherwise no objections to the approach, but I'd like to properly do
> an Acked-by: after I see the patch. :)

I'll send it out.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  364/364              364/364
ILK                                  366/366              366/366
SNB                                  450/450              450/450
IVB              +17                 481/498              498/498
BYT                                  289/289              289/289
HSW                                  564/564              564/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 IVB  igt_kms_3d      DMESG_WARN(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-128x128-onscreen      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-128x128-random      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-128x128-sliding      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-256x256-offscreen      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-256x256-onscreen      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-256x256-sliding      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-64x64-offscreen      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-64x64-onscreen      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-64x64-random      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-64x64-sliding      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_cursor_crc_cursor-size-change      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_fence_pin_leak      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_rotation_crc_primary-rotation      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
 IVB  igt_kms_rotation_crc_sprite-rotation      NSPT(1, M34)PASS(4, M4M34M21)      PASS(1, M34)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[John Stultz]

On Thu, Dec 4, 2014 at 2:42 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Dec 03, 2014 at 11:07:08AM -0800, John Stultz wrote:
>> On Wed, Dec 3, 2014 at 6:30 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
>> >> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> >> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
>> >> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
>> >> >> +{
>> >> >> +     u64 usecs = div_u64(m + 999, 1000);
>> >> >> +     unsigned long j = usecs_to_jiffies(usecs);
>> >> >> +
>> >> >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>> >> >
>> >> > Or more concisely and review friendly:
>> >> >
>> >> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
>> >> > {
>> >> >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
>> >> > }
>> >>
>> >> Yea. This looks much nicer. Seems generic enough it might be better
>> >> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
>> >> rather then in a driver header.
>> >
>> > Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
>> > "Yea" above as an ack for adding that and pulling it in through
>> > drm-intel.git?
>>
>> Do you need an EXPORT_SYMBOL if you add the _timeout version next to
>> nsecs_to_jiffies64 in time.c?
>
> I wouldn't but the patch from Imre to add all the _timeout was killed with
> a few bikesheds so really not volunteering. And just moving this single
> one doesn't make a lot of sense imo. Also the next patch I'll do is just
> add the +1 that we lost to the code and call it a day, really ;-)
>

Sigh. So you're going to make me write a separate patch that moves it over?

I know you'll probably say this is bikeshedding, but the reason why
avoiding the EXPORT_SYMBOL on nsec_to_jiffies would be good is because
nsec_to_jiffies explicitly states in the comments that its not for any
use but the scheduler.

But still, I do see our change broke you here, so I'm not going to object.

thanks
-john

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[John Stultz]

On Thu, Dec 4, 2014 at 2:42 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Dec 03, 2014 at 11:07:08AM -0800, John Stultz wrote:
>> On Wed, Dec 3, 2014 at 6:30 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
>> >> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> >> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
>> >> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
>> >> >> +{
>> >> >> +     u64 usecs = div_u64(m + 999, 1000);
>> >> >> +     unsigned long j = usecs_to_jiffies(usecs);
>> >> >> +
>> >> >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>> >> >
>> >> > Or more concisely and review friendly:
>> >> >
>> >> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
>> >> > {
>> >> >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
>> >> > }
>> >>
>> >> Yea. This looks much nicer. Seems generic enough it might be better
>> >> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
>> >> rather then in a driver header.
>> >
>> > Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
>> > "Yea" above as an ack for adding that and pulling it in through
>> > drm-intel.git?
>>
>> Do you need an EXPORT_SYMBOL if you add the _timeout version next to
>> nsecs_to_jiffies64 in time.c?
>
> I wouldn't but the patch from Imre to add all the _timeout was killed with
> a few bikesheds so really not volunteering. And just moving this single
> one doesn't make a lot of sense imo. Also the next patch I'll do is just
> add the +1 that we lost to the code and call it a day, really ;-)
>

Sigh. So you're going to make me write a separate patch that moves it over?

I know you'll probably say this is bikeshedding, but the reason why
avoiding the EXPORT_SYMBOL on nsec_to_jiffies would be good is because
nsec_to_jiffies explicitly states in the comments that its not for any
use but the scheduler.

But still, I do see our change broke you here, so I'm not going to object.

thanks
-john
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[John Stultz]

On Thu, Dec 4, 2014 at 2:12 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We've lost the +1 required for correct timeouts in
>
> commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Wed Jul 16 21:05:06 2014 +0000
>
>     drm: i915: Use nsec based interfaces
>
>     Use ktime_get_raw_ns() and get rid of the back and forth timespec
>     conversions.
>
>     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>     Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>     Signed-off-by: John Stultz <john.stultz@linaro.org>
>
> So fix this up by reinstating our handrolled _timeout function. While
> at it bother with handling MAX_JIFFIES.
>
> v2: Convert to usecs (we don't care about the accuracy anyway) first
> to avoid overflow issues Dave Gordon spotted.
>
> v3: Drop the explicit MAX_JIFFY_OFFSET check, usecs_to_jiffies should
> take care of that already. It might be a bit too enthusiastic about it
> though.
>
> v4: Chris has a much nicer color, so use his implementation.
>
> This requires to export nsec_to_jiffies from time.c.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 5 +++++
>  drivers/gpu/drm/i915/i915_gem.c | 3 ++-
>  kernel/time/time.c              | 1 +
>  3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 049482f5d9ac..564a45f4a0ab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3097,6 +3097,11 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
>         return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>  }
>
> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> +{
> +        return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> +}
> +
>  static inline unsigned long
>  timespec_to_jiffies_timeout(const struct timespec *value)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9d362d320d82..04a9f26407ae 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1226,7 +1226,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
>         if (i915_gem_request_completed(req, true))
>                 return 0;
>
> -       timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
> +       timeout_expire = timeout ?
> +               jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
>
>         if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
>                 gen6_rps_boost(dev_priv);
> diff --git a/kernel/time/time.c b/kernel/time/time.c
> index a9ae20fb0b11..8fae82ca5cbf 100644
> --- a/kernel/time/time.c
> +++ b/kernel/time/time.c
> @@ -745,6 +745,7 @@ u64 nsecs_to_jiffies64(u64 n)
>         return div_u64(n * 9, (9ull * NSEC_PER_SEC + HZ / 2) / HZ);
>  #endif
>  }
> +EXPORT_SYMBOL(nsecs_to_jiffies64);


For the sake of the fix,
Acked-by: John Stultz <john.stultz@linaro.org>

But I'll likely follow this (eventually - since its going through the
drm tree) with a cleanup patch moving nsecs_to_jiffies_timeout over to
time.c.

thanks
-john

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[John Stultz]

On Thu, Dec 4, 2014 at 2:12 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We've lost the +1 required for correct timeouts in
>
> commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Wed Jul 16 21:05:06 2014 +0000
>
>     drm: i915: Use nsec based interfaces
>
>     Use ktime_get_raw_ns() and get rid of the back and forth timespec
>     conversions.
>
>     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>     Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>     Signed-off-by: John Stultz <john.stultz@linaro.org>
>
> So fix this up by reinstating our handrolled _timeout function. While
> at it bother with handling MAX_JIFFIES.
>
> v2: Convert to usecs (we don't care about the accuracy anyway) first
> to avoid overflow issues Dave Gordon spotted.
>
> v3: Drop the explicit MAX_JIFFY_OFFSET check, usecs_to_jiffies should
> take care of that already. It might be a bit too enthusiastic about it
> though.
>
> v4: Chris has a much nicer color, so use his implementation.
>
> This requires to export nsec_to_jiffies from time.c.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 5 +++++
>  drivers/gpu/drm/i915/i915_gem.c | 3 ++-
>  kernel/time/time.c              | 1 +
>  3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 049482f5d9ac..564a45f4a0ab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3097,6 +3097,11 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
>         return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>  }
>
> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> +{
> +        return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> +}
> +
>  static inline unsigned long
>  timespec_to_jiffies_timeout(const struct timespec *value)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9d362d320d82..04a9f26407ae 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1226,7 +1226,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
>         if (i915_gem_request_completed(req, true))
>                 return 0;
>
> -       timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
> +       timeout_expire = timeout ?
> +               jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
>
>         if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
>                 gen6_rps_boost(dev_priv);
> diff --git a/kernel/time/time.c b/kernel/time/time.c
> index a9ae20fb0b11..8fae82ca5cbf 100644
> --- a/kernel/time/time.c
> +++ b/kernel/time/time.c
> @@ -745,6 +745,7 @@ u64 nsecs_to_jiffies64(u64 n)
>         return div_u64(n * 9, (9ull * NSEC_PER_SEC + HZ / 2) / HZ);
>  #endif
>  }
> +EXPORT_SYMBOL(nsecs_to_jiffies64);


For the sake of the fix,
Acked-by: John Stultz <john.stultz@linaro.org>

But I'll likely follow this (eventually - since its going through the
drm tree) with a cleanup patch moving nsecs_to_jiffies_timeout over to
time.c.

thanks
-john
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <john.stultz@linaro.org> wrote:
> On Thu, Dec 4, 2014 at 2:42 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Wed, Dec 03, 2014 at 11:07:08AM -0800, John Stultz wrote:
>>> On Wed, Dec 3, 2014 at 6:30 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>>> > On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
>>> >> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>> >> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
>>> >> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
>>> >> >> +{
>>> >> >> +     u64 usecs = div_u64(m + 999, 1000);
>>> >> >> +     unsigned long j = usecs_to_jiffies(usecs);
>>> >> >> +
>>> >> >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>>> >> >
>>> >> > Or more concisely and review friendly:
>>> >> >
>>> >> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
>>> >> > {
>>> >> >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
>>> >> > }
>>> >>
>>> >> Yea. This looks much nicer. Seems generic enough it might be better
>>> >> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
>>> >> rather then in a driver header.
>>> >
>>> > Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
>>> > "Yea" above as an ack for adding that and pulling it in through
>>> > drm-intel.git?
>>>
>>> Do you need an EXPORT_SYMBOL if you add the _timeout version next to
>>> nsecs_to_jiffies64 in time.c?
>>
>> I wouldn't but the patch from Imre to add all the _timeout was killed with
>> a few bikesheds so really not volunteering. And just moving this single
>> one doesn't make a lot of sense imo. Also the next patch I'll do is just
>> add the +1 that we lost to the code and call it a day, really ;-)
>>
>
> Sigh. So you're going to make me write a separate patch that moves it over?

We've written it already, Imre posted the link to the old discussion:

https://lkml.org/lkml/2013/5/10/187

But if the first attempt doesn't sufficiently stick I tend to chase
the patches any more. But if you want to resurrect this I could ping
Imre and ask him to pick it up again or you could rebase his patches.

> I know you'll probably say this is bikeshedding, but the reason why
> avoiding the EXPORT_SYMBOL on nsec_to_jiffies would be good is because
> nsec_to_jiffies explicitly states in the comments that its not for any
> use but the scheduler.

Well I only export the 64 variant, the other one (with the too small
return type which would overflow) is already exported with

commit d560fed6abe0f9975b509e4fb824e08ac19adc93
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Wed Jul 16 21:04:31 2014 +0000

    time: Export nsecs_to_jiffies()

    Required for moving drivers to the nanosecond based interfaces.

    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Signed-off-by: John Stultz <john.stultz@linaro.org>

So I've figured this is ok.

> But still, I do see our change broke you here, so I'm not going to object.

Ok, thanks I'll pull this in through drm-intel for 3.19 (3.18 is kinda
done already I guess) with cc: stable.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <john.stultz@linaro.org> wrote:
> On Thu, Dec 4, 2014 at 2:42 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Wed, Dec 03, 2014 at 11:07:08AM -0800, John Stultz wrote:
>>> On Wed, Dec 3, 2014 at 6:30 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>>> > On Tue, Dec 02, 2014 at 08:54:13AM -0800, John Stultz wrote:
>>> >> On Tue, Dec 2, 2014 at 8:35 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>> >> > On Tue, Dec 02, 2014 at 04:36:22PM +0100, Daniel Vetter wrote:
>>> >> >> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 m)
>>> >> >> +{
>>> >> >> +     u64 usecs = div_u64(m + 999, 1000);
>>> >> >> +     unsigned long j = usecs_to_jiffies(usecs);
>>> >> >> +
>>> >> >> +     return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>>> >> >
>>> >> > Or more concisely and review friendly:
>>> >> >
>>> >> > static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
>>> >> > {
>>> >> >         return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
>>> >> > }
>>> >>
>>> >> Yea. This looks much nicer. Seems generic enough it might be better
>>> >> added next to nsec_to_jiffies64() in kernel/time/time.c or jiffies.h
>>> >> rather then in a driver header.
>>> >
>>> > Ok, that needs an EXPORT_SYMBOL for nsecs_to_jiffies64. Can I count your
>>> > "Yea" above as an ack for adding that and pulling it in through
>>> > drm-intel.git?
>>>
>>> Do you need an EXPORT_SYMBOL if you add the _timeout version next to
>>> nsecs_to_jiffies64 in time.c?
>>
>> I wouldn't but the patch from Imre to add all the _timeout was killed with
>> a few bikesheds so really not volunteering. And just moving this single
>> one doesn't make a lot of sense imo. Also the next patch I'll do is just
>> add the +1 that we lost to the code and call it a day, really ;-)
>>
>
> Sigh. So you're going to make me write a separate patch that moves it over?

We've written it already, Imre posted the link to the old discussion:

https://lkml.org/lkml/2013/5/10/187

But if the first attempt doesn't sufficiently stick I tend to chase
the patches any more. But if you want to resurrect this I could ping
Imre and ask him to pick it up again or you could rebase his patches.

> I know you'll probably say this is bikeshedding, but the reason why
> avoiding the EXPORT_SYMBOL on nsec_to_jiffies would be good is because
> nsec_to_jiffies explicitly states in the comments that its not for any
> use but the scheduler.

Well I only export the 64 variant, the other one (with the too small
return type which would overflow) is already exported with

commit d560fed6abe0f9975b509e4fb824e08ac19adc93
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Wed Jul 16 21:04:31 2014 +0000

    time: Export nsecs_to_jiffies()

    Required for moving drivers to the nanosecond based interfaces.

    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Signed-off-by: John Stultz <john.stultz@linaro.org>

So I've figured this is ok.

> But still, I do see our change broke you here, so I'm not going to object.

Ok, thanks I'll pull this in through drm-intel for 3.19 (3.18 is kinda
done already I guess) with cc: stable.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[John Stultz]

On Thu, Dec 4, 2014 at 9:50 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <john.stultz@linaro.org> wrote:
>> Sigh. So you're going to make me write a separate patch that moves it over?
>
> We've written it already, Imre posted the link to the old discussion:
>
> https://lkml.org/lkml/2013/5/10/187
>
> But if the first attempt doesn't sufficiently stick I tend to chase
> the patches any more. But if you want to resurrect this I could ping
> Imre and ask him to pick it up again or you could rebase his patches.

Well, last I saw the initial patch was buggy, no? I don't think I saw
it being resubmitted.


>> But still, I do see our change broke you here, so I'm not going to object.
>
> Ok, thanks I'll pull this in through drm-intel for 3.19 (3.18 is kinda
> done already I guess) with cc: stable.

You probably should submit it for 3.18 and let Linus decide if its too
late. I've already gotten yelled at by Ingo for pushing patches in the
merge window that cc stable. Even if its out of a desire to let the
patches get wider testing, its something of a hot-button item for
folks. :)

thanks
-john

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[John Stultz]

On Thu, Dec 4, 2014 at 9:50 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <john.stultz@linaro.org> wrote:
>> Sigh. So you're going to make me write a separate patch that moves it over?
>
> We've written it already, Imre posted the link to the old discussion:
>
> https://lkml.org/lkml/2013/5/10/187
>
> But if the first attempt doesn't sufficiently stick I tend to chase
> the patches any more. But if you want to resurrect this I could ping
> Imre and ask him to pick it up again or you could rebase his patches.

Well, last I saw the initial patch was buggy, no? I don't think I saw
it being resubmitted.


>> But still, I do see our change broke you here, so I'm not going to object.
>
> Ok, thanks I'll pull this in through drm-intel for 3.19 (3.18 is kinda
> done already I guess) with cc: stable.

You probably should submit it for 3.18 and let Linus decide if its too
late. I've already gotten yelled at by Ingo for pushing patches in the
merge window that cc stable. Even if its out of a desire to let the
patches get wider testing, its something of a hot-button item for
folks. :)

thanks
-john
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

On Thu, Dec 4, 2014 at 7:16 PM, John Stultz <john.stultz@linaro.org> wrote:
> On Thu, Dec 4, 2014 at 9:50 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <john.stultz@linaro.org> wrote:
>>> Sigh. So you're going to make me write a separate patch that moves it over?
>>
>> We've written it already, Imre posted the link to the old discussion:
>>
>> https://lkml.org/lkml/2013/5/10/187
>>
>> But if the first attempt doesn't sufficiently stick I tend to chase
>> the patches any more. But if you want to resurrect this I could ping
>> Imre and ask him to pick it up again or you could rebase his patches.
>
> Well, last I saw the initial patch was buggy, no? I don't think I saw
> it being resubmitted.

I didn't see your reply in that thread nor in the v2 follow up at
http://marc.info/?l=linux-kernel&m=136854294730957&w=2 Maybe I missed
it, but response seems to have been lukewarm overall.

>>> But still, I do see our change broke you here, so I'm not going to object.
>>
>> Ok, thanks I'll pull this in through drm-intel for 3.19 (3.18 is kinda
>> done already I guess) with cc: stable.
>
> You probably should submit it for 3.18 and let Linus decide if its too
> late. I've already gotten yelled at by Ingo for pushing patches in the
> merge window that cc stable. Even if its out of a desire to let the
> patches get wider testing, its something of a hot-button item for
> folks. :)

Oh I know, but if you count your regression rate in bugs-per-day you
end up with different standards ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

On Thu, Dec 4, 2014 at 7:16 PM, John Stultz <john.stultz@linaro.org> wrote:
> On Thu, Dec 4, 2014 at 9:50 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <john.stultz@linaro.org> wrote:
>>> Sigh. So you're going to make me write a separate patch that moves it over?
>>
>> We've written it already, Imre posted the link to the old discussion:
>>
>> https://lkml.org/lkml/2013/5/10/187
>>
>> But if the first attempt doesn't sufficiently stick I tend to chase
>> the patches any more. But if you want to resurrect this I could ping
>> Imre and ask him to pick it up again or you could rebase his patches.
>
> Well, last I saw the initial patch was buggy, no? I don't think I saw
> it being resubmitted.

I didn't see your reply in that thread nor in the v2 follow up at
http://marc.info/?l=linux-kernel&m=136854294730957&w=2 Maybe I missed
it, but response seems to have been lukewarm overall.

>>> But still, I do see our change broke you here, so I'm not going to object.
>>
>> Ok, thanks I'll pull this in through drm-intel for 3.19 (3.18 is kinda
>> done already I guess) with cc: stable.
>
> You probably should submit it for 3.18 and let Linus decide if its too
> late. I've already gotten yelled at by Ingo for pushing patches in the
> merge window that cc stable. Even if its out of a desire to let the
> patches get wider testing, its something of a hot-button item for
> folks. :)

Oh I know, but if you count your regression rate in bugs-per-day you
end up with different standards ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[John Stultz]

On Thu, Dec 4, 2014 at 10:51 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Thu, Dec 4, 2014 at 7:16 PM, John Stultz <john.stultz@linaro.org> wrote:
>> On Thu, Dec 4, 2014 at 9:50 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>> On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <john.stultz@linaro.org> wrote:
>>>> Sigh. So you're going to make me write a separate patch that moves it over?
>>>
>>> We've written it already, Imre posted the link to the old discussion:
>>>
>>> https://lkml.org/lkml/2013/5/10/187
>>>
>>> But if the first attempt doesn't sufficiently stick I tend to chase
>>> the patches any more. But if you want to resurrect this I could ping
>>> Imre and ask him to pick it up again or you could rebase his patches.
>>
>> Well, last I saw the initial patch was buggy, no? I don't think I saw
>> it being resubmitted.
>
> I didn't see your reply in that thread nor in the v2 follow up at
> http://marc.info/?l=linux-kernel&m=136854294730957&w=2 Maybe I missed
> it, but response seems to have been lukewarm overall.

Ok, I wasn't cc'ed on the v2, thanks for the pointer.  There's some
general lukewarmness to all things jiffies, since getting rid of them
has been a long term goal forever. But overall that patch set seemed
ok (though I'm not a fan of macro generation of functions). But minor
details..

thanks
-john

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[John Stultz]

On Thu, Dec 4, 2014 at 10:51 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Thu, Dec 4, 2014 at 7:16 PM, John Stultz <john.stultz@linaro.org> wrote:
>> On Thu, Dec 4, 2014 at 9:50 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>> On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <john.stultz@linaro.org> wrote:
>>>> Sigh. So you're going to make me write a separate patch that moves it over?
>>>
>>> We've written it already, Imre posted the link to the old discussion:
>>>
>>> https://lkml.org/lkml/2013/5/10/187
>>>
>>> But if the first attempt doesn't sufficiently stick I tend to chase
>>> the patches any more. But if you want to resurrect this I could ping
>>> Imre and ask him to pick it up again or you could rebase his patches.
>>
>> Well, last I saw the initial patch was buggy, no? I don't think I saw
>> it being resubmitted.
>
> I didn't see your reply in that thread nor in the v2 follow up at
> http://marc.info/?l=linux-kernel&m=136854294730957&w=2 Maybe I missed
> it, but response seems to have been lukewarm overall.

Ok, I wasn't cc'ed on the v2, thanks for the pointer.  There's some
general lukewarmness to all things jiffies, since getting rid of them
has been a long term goal forever. But overall that patch set seemed
ok (though I'm not a fan of macro generation of functions). But minor
details..

thanks
-john
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

On Thu, Dec 04, 2014 at 12:35:44PM -0800, John Stultz wrote:
> On Thu, Dec 4, 2014 at 10:51 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > On Thu, Dec 4, 2014 at 7:16 PM, John Stultz <john.stultz@linaro.org> wrote:
> >> On Thu, Dec 4, 2014 at 9:50 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >>> On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <john.stultz@linaro.org> wrote:
> >>>> Sigh. So you're going to make me write a separate patch that moves it over?
> >>>
> >>> We've written it already, Imre posted the link to the old discussion:
> >>>
> >>> https://lkml.org/lkml/2013/5/10/187
> >>>
> >>> But if the first attempt doesn't sufficiently stick I tend to chase
> >>> the patches any more. But if you want to resurrect this I could ping
> >>> Imre and ask him to pick it up again or you could rebase his patches.
> >>
> >> Well, last I saw the initial patch was buggy, no? I don't think I saw
> >> it being resubmitted.
> >
> > I didn't see your reply in that thread nor in the v2 follow up at
> > http://marc.info/?l=linux-kernel&m=136854294730957&w=2 Maybe I missed
> > it, but response seems to have been lukewarm overall.
> 
> Ok, I wasn't cc'ed on the v2, thanks for the pointer.  There's some
> general lukewarmness to all things jiffies, since getting rid of them
> has been a long term goal forever. But overall that patch set seemed
> ok (though I'm not a fan of macro generation of functions). But minor
> details..

btw have you seen the other fallout from the ktime->nsec conversion in
i915?

http://www.spinics.net/lists/intel-gfx/msg56445.html

Is this just the inaccuracy of nsec_to_jiffies (and why it explicitly
states that this is for the scheduler only) or is there some bigger fish
in there?

Insight very much appreciated.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Daniel Vetter]

On Thu, Dec 04, 2014 at 12:35:44PM -0800, John Stultz wrote:
> On Thu, Dec 4, 2014 at 10:51 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > On Thu, Dec 4, 2014 at 7:16 PM, John Stultz <john.stultz@linaro.org> wrote:
> >> On Thu, Dec 4, 2014 at 9:50 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >>> On Thu, Dec 4, 2014 at 6:42 PM, John Stultz <john.stultz@linaro.org> wrote:
> >>>> Sigh. So you're going to make me write a separate patch that moves it over?
> >>>
> >>> We've written it already, Imre posted the link to the old discussion:
> >>>
> >>> https://lkml.org/lkml/2013/5/10/187
> >>>
> >>> But if the first attempt doesn't sufficiently stick I tend to chase
> >>> the patches any more. But if you want to resurrect this I could ping
> >>> Imre and ask him to pick it up again or you could rebase his patches.
> >>
> >> Well, last I saw the initial patch was buggy, no? I don't think I saw
> >> it being resubmitted.
> >
> > I didn't see your reply in that thread nor in the v2 follow up at
> > http://marc.info/?l=linux-kernel&m=136854294730957&w=2 Maybe I missed
> > it, but response seems to have been lukewarm overall.
> 
> Ok, I wasn't cc'ed on the v2, thanks for the pointer.  There's some
> general lukewarmness to all things jiffies, since getting rid of them
> has been a long term goal forever. But overall that patch set seemed
> ok (though I'm not a fan of macro generation of functions). But minor
> details..

btw have you seen the other fallout from the ktime->nsec conversion in
i915?

http://www.spinics.net/lists/intel-gfx/msg56445.html

Is this just the inaccuracy of nsec_to_jiffies (and why it explicitly
states that this is for the scheduler only) or is there some bigger fish
in there?

Insight very much appreciated.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Handle inaccurate time conversion issues

[Jani Nikula]

On Fri, 28 Nov 2014, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> So apparently jiffies<->nsec<->ktime isn't accurate or something. At
> elast if we timeout there's occasionally still a few hundred us left
> (in a 2 second timeout).
>
> Stuff I've tried and thrown out again:
> - Sampling the before timestamp before jiffies. Doesn't improve test
>   path rate at all.
> - Using jiffies. Way to inaccurate, which means way too much drift
>   with signals plus automatic ioctl restarting in userspace. In
>   hindsight we should have used an absolute timeout, but hey we need
>   something for v3 of the i915 gem wait interfaces ;-)
> - Trying to figure out where accuracy gets lost. gl testcase really
>   don't care all that much about this (as long as isn't not massively
>   off), it's just that the testcase gets a bit upset if it receives an
>   EITME with timeout > 0.
>
> So as long as we're in the ballbark it's good enough. So patch
> everything up if we're at most one jiffies off. I get's me a solid
> test again.
>
> This regression is probably introduced in
>
> commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Wed Jul 16 21:05:06 2014 +0000
>
>     drm: i915: Use nsec based interfaces
>
>     Use ktime_get_raw_ns() and get rid of the back and forth timespec
>     conversions.
>
>     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>     Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>     Signed-off-by: John Stultz <john.stultz@linaro.org>
>
> Probably because I'm too lazy to confirm myself and still waiting for
> QA ;-)
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Stultz <john.stultz@linaro.org>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Pushed to drm-intel-next-fixes, thanks for the patch.

BR,
Jani.

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 002730d1409b..abdc5bcdbd0d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1302,6 +1302,16 @@ int __i915_wait_seqno(struct intel_engine_cs *ring, u32 seqno,
>  		s64 tres = *timeout - (now - before);
>  
>  		*timeout = tres < 0 ? 0 : tres;
> +
> +		/*
> +		 * Apparently ktime isn't accurate enough and occasionally has a
> +		 * bit of mismatch in the jiffies<->nsecs<->ktime loop. So patch
> +		 * things up to make the test happy. We allow up to 1 jiffy.
> +		 *
> +		 * This is a regrssion from the timespec->ktime conversion.
> +		 */
> +		if (ret == -ETIME && *timeout < jiffies_to_usecs(1)*1000)
> +			*timeout = 0;
>  	}
>  
>  	return ret;
> -- 
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Jani Nikula]

On Thu, 04 Dec 2014, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We've lost the +1 required for correct timeouts in
>
> commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Wed Jul 16 21:05:06 2014 +0000
>
>     drm: i915: Use nsec based interfaces
>
>     Use ktime_get_raw_ns() and get rid of the back and forth timespec
>     conversions.
>
>     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>     Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>     Signed-off-by: John Stultz <john.stultz@linaro.org>
>
> So fix this up by reinstating our handrolled _timeout function. While
> at it bother with handling MAX_JIFFIES.
>
> v2: Convert to usecs (we don't care about the accuracy anyway) first
> to avoid overflow issues Dave Gordon spotted.
>
> v3: Drop the explicit MAX_JIFFY_OFFSET check, usecs_to_jiffies should
> take care of that already. It might be a bit too enthusiastic about it
> though.
>
> v4: Chris has a much nicer color, so use his implementation.
>
> This requires to export nsec_to_jiffies from time.c.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Pushed to drm-intel-next-fixes, thanks for the patch.

BR,
Jani.

> ---
>  drivers/gpu/drm/i915/i915_drv.h | 5 +++++
>  drivers/gpu/drm/i915/i915_gem.c | 3 ++-
>  kernel/time/time.c              | 1 +
>  3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 049482f5d9ac..564a45f4a0ab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3097,6 +3097,11 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
>  	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>  }
>  
> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> +{
> +        return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> +}
> +
>  static inline unsigned long
>  timespec_to_jiffies_timeout(const struct timespec *value)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9d362d320d82..04a9f26407ae 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1226,7 +1226,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
>  	if (i915_gem_request_completed(req, true))
>  		return 0;
>  
> -	timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
> +	timeout_expire = timeout ?
> +		jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
>  
>  	if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
>  		gen6_rps_boost(dev_priv);
> diff --git a/kernel/time/time.c b/kernel/time/time.c
> index a9ae20fb0b11..8fae82ca5cbf 100644
> --- a/kernel/time/time.c
> +++ b/kernel/time/time.c
> @@ -745,6 +745,7 @@ u64 nsecs_to_jiffies64(u64 n)
>  	return div_u64(n * 9, (9ull * NSEC_PER_SEC + HZ / 2) / HZ);
>  #endif
>  }
> +EXPORT_SYMBOL(nsecs_to_jiffies64);
>  
>  /**
>   * nsecs_to_jiffies - Convert nsecs in u64 to jiffies
> -- 
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH] drm/i915: compute wait_ioctl timeout correctly

[Jani Nikula]

On Thu, 04 Dec 2014, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We've lost the +1 required for correct timeouts in
>
> commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date:   Wed Jul 16 21:05:06 2014 +0000
>
>     drm: i915: Use nsec based interfaces
>
>     Use ktime_get_raw_ns() and get rid of the back and forth timespec
>     conversions.
>
>     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>     Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>     Signed-off-by: John Stultz <john.stultz@linaro.org>
>
> So fix this up by reinstating our handrolled _timeout function. While
> at it bother with handling MAX_JIFFIES.
>
> v2: Convert to usecs (we don't care about the accuracy anyway) first
> to avoid overflow issues Dave Gordon spotted.
>
> v3: Drop the explicit MAX_JIFFY_OFFSET check, usecs_to_jiffies should
> take care of that already. It might be a bit too enthusiastic about it
> though.
>
> v4: Chris has a much nicer color, so use his implementation.
>
> This requires to export nsec_to_jiffies from time.c.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82749
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Pushed to drm-intel-next-fixes, thanks for the patch.

BR,
Jani.

> ---
>  drivers/gpu/drm/i915/i915_drv.h | 5 +++++
>  drivers/gpu/drm/i915/i915_gem.c | 3 ++-
>  kernel/time/time.c              | 1 +
>  3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 049482f5d9ac..564a45f4a0ab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3097,6 +3097,11 @@ static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m)
>  	return min_t(unsigned long, MAX_JIFFY_OFFSET, j + 1);
>  }
>  
> +static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
> +{
> +        return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
> +}
> +
>  static inline unsigned long
>  timespec_to_jiffies_timeout(const struct timespec *value)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9d362d320d82..04a9f26407ae 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1226,7 +1226,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
>  	if (i915_gem_request_completed(req, true))
>  		return 0;
>  
> -	timeout_expire = timeout ? jiffies + nsecs_to_jiffies((u64)*timeout) : 0;
> +	timeout_expire = timeout ?
> +		jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0;
>  
>  	if (INTEL_INFO(dev)->gen >= 6 && ring->id == RCS && can_wait_boost(file_priv)) {
>  		gen6_rps_boost(dev_priv);
> diff --git a/kernel/time/time.c b/kernel/time/time.c
> index a9ae20fb0b11..8fae82ca5cbf 100644
> --- a/kernel/time/time.c
> +++ b/kernel/time/time.c
> @@ -745,6 +745,7 @@ u64 nsecs_to_jiffies64(u64 n)
>  	return div_u64(n * 9, (9ull * NSEC_PER_SEC + HZ / 2) / HZ);
>  #endif
>  }
> +EXPORT_SYMBOL(nsecs_to_jiffies64);
>  
>  /**
>   * nsecs_to_jiffies - Convert nsecs in u64 to jiffies
> -- 
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx