Re: [Intel-gfx] [PATCH 01/10] drm/vblank: Data type fixes for 64-bit vblank sequences.

[Pandiyan, Dhinakaran]

On Thu, 2018-02-15 at 11:55 -0800, Rodrigo Vivi wrote:
> Dave Airlie  writes:
> 
> > On 6 February 2018 at 06:32, Rodrigo Vivi  wrote:
> >> On Sat, Feb 03, 2018 at 08:14:48AM +, Keith Packard wrote:
> >>> Dhinakaran Pandiyan  writes:
> >>>
> >>> > From: "Pandiyan, Dhinakaran" 
> >>> >
> >>> > drm_vblank_count() has an u32 type returning what is a 64-bit vblank 
> >>> > count.
> >>> > The effect of this is when drm_wait_vblank_ioctl() tries to widen the 
> >>> > user
> >>> > space requested vblank sequence using this clipped 32-bit count(when the
> >>> > value is >= 2^32) as reference, the requested sequence remains a 32-bit
> >>> > value and gets queued like that. However, the code that checks if the
> >>> > requested sequence has passed compares this against the 64-bit vblank
> >>> > count.
> >>>
> >>> For patches 1-7:
> >>>
> >>> Reviewed-by: Keith Packard 
> >>
> >> Dave, ack to merge them through drm-intel-next-queued ?
> >
> > Ack. do we know if any of those need to be in -fixes?
> 
> All patches merged to drm-intel-next-queued.
> Thanks for the patches, reviews and acks.
> 


Thanks everyone for the reviews and Acks!
-DK

> >
> > or too early to tell?
> >
> > Dave.
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/10] drm/vblank: Data type fixes for 64-bit vblank sequences.

[Rodrigo Vivi]

Dave Airlie  writes:

> On 6 February 2018 at 06:32, Rodrigo Vivi  wrote:
>> On Sat, Feb 03, 2018 at 08:14:48AM +, Keith Packard wrote:
>>> Dhinakaran Pandiyan  writes:
>>>
>>> > From: "Pandiyan, Dhinakaran" 
>>> >
>>> > drm_vblank_count() has an u32 type returning what is a 64-bit vblank 
>>> > count.
>>> > The effect of this is when drm_wait_vblank_ioctl() tries to widen the user
>>> > space requested vblank sequence using this clipped 32-bit count(when the
>>> > value is >= 2^32) as reference, the requested sequence remains a 32-bit
>>> > value and gets queued like that. However, the code that checks if the
>>> > requested sequence has passed compares this against the 64-bit vblank
>>> > count.
>>>
>>> For patches 1-7:
>>>
>>> Reviewed-by: Keith Packard 
>>
>> Dave, ack to merge them through drm-intel-next-queued ?
>
> Ack. do we know if any of those need to be in -fixes?

All patches merged to drm-intel-next-queued.
Thanks for the patches, reviews and acks.

>
> or too early to tell?
>
> Dave.
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/10] drm/vblank: Data type fixes for 64-bit vblank sequences.

[Keith Packard]

Rodrigo Vivi  writes:

> I didn't checked the drm one close enough yet to decide for that.
> DK or Keith? do you guys see anyone suitable for fixes?

Yeah, we should probably get 1, 3 and 7 into fixes. 2, 4, 5 and 6 add
explicit casts where the compiler would already introduce equivalent
implicit casts.

-- 
-keith


signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/10] drm/vblank: Data type fixes for 64-bit vblank sequences.

[Pandiyan, Dhinakaran]

On Mon, 2018-02-05 at 12:49 -0800, Rodrigo Vivi wrote:
> On Mon, Feb 05, 2018 at 08:37:13PM +, Dave Airlie wrote:
> > On 6 February 2018 at 06:32, Rodrigo Vivi  wrote:
> > > On Sat, Feb 03, 2018 at 08:14:48AM +, Keith Packard wrote:
> > >> Dhinakaran Pandiyan  writes:
> > >>
> > >> > From: "Pandiyan, Dhinakaran" 
> > >> >
> > >> > drm_vblank_count() has an u32 type returning what is a 64-bit vblank 
> > >> > count.
> > >> > The effect of this is when drm_wait_vblank_ioctl() tries to widen the 
> > >> > user
> > >> > space requested vblank sequence using this clipped 32-bit count(when 
> > >> > the
> > >> > value is >= 2^32) as reference, the requested sequence remains a 32-bit
> > >> > value and gets queued like that. However, the code that checks if the
> > >> > requested sequence has passed compares this against the 64-bit vblank
> > >> > count.
> > >>
> > >> For patches 1-7:
> > >>
> > >> Reviewed-by: Keith Packard 
> > >
> > > Dave, ack to merge them through drm-intel-next-queued ?
> > 
> > Ack. do we know if any of those need to be in -fixes?
> > 
> > or too early to tell?
> 
> I didn't checked the drm one close enough yet to decide for that.
> DK or Keith? do you guys see anyone suitable for fixes?

I was thinking patch 1 would be a candidate. But, it'd need the crtc to
be active continuously for > 2.2 years at 60 frames/s to trigger this.
The problem is exacerbated with PSR and PSR is disabled. So, I think we
can skip -fixes.

> 
> For the other work on top I believe we don't need to move to fixes
> since psr is still disabled.
> 
> > 
> > Dave.
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/10] drm/vblank: Data type fixes for 64-bit vblank sequences.

[Rodrigo Vivi]

On Mon, Feb 05, 2018 at 08:37:13PM +, Dave Airlie wrote:
> On 6 February 2018 at 06:32, Rodrigo Vivi  wrote:
> > On Sat, Feb 03, 2018 at 08:14:48AM +, Keith Packard wrote:
> >> Dhinakaran Pandiyan  writes:
> >>
> >> > From: "Pandiyan, Dhinakaran" 
> >> >
> >> > drm_vblank_count() has an u32 type returning what is a 64-bit vblank 
> >> > count.
> >> > The effect of this is when drm_wait_vblank_ioctl() tries to widen the 
> >> > user
> >> > space requested vblank sequence using this clipped 32-bit count(when the
> >> > value is >= 2^32) as reference, the requested sequence remains a 32-bit
> >> > value and gets queued like that. However, the code that checks if the
> >> > requested sequence has passed compares this against the 64-bit vblank
> >> > count.
> >>
> >> For patches 1-7:
> >>
> >> Reviewed-by: Keith Packard 
> >
> > Dave, ack to merge them through drm-intel-next-queued ?
> 
> Ack. do we know if any of those need to be in -fixes?
> 
> or too early to tell?

I didn't checked the drm one close enough yet to decide for that.
DK or Keith? do you guys see anyone suitable for fixes?

For the other work on top I believe we don't need to move to fixes
since psr is still disabled.

> 
> Dave.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/10] drm/vblank: Data type fixes for 64-bit vblank sequences.

[Dave Airlie]

On 6 February 2018 at 06:32, Rodrigo Vivi  wrote:
> On Sat, Feb 03, 2018 at 08:14:48AM +, Keith Packard wrote:
>> Dhinakaran Pandiyan  writes:
>>
>> > From: "Pandiyan, Dhinakaran" 
>> >
>> > drm_vblank_count() has an u32 type returning what is a 64-bit vblank count.
>> > The effect of this is when drm_wait_vblank_ioctl() tries to widen the user
>> > space requested vblank sequence using this clipped 32-bit count(when the
>> > value is >= 2^32) as reference, the requested sequence remains a 32-bit
>> > value and gets queued like that. However, the code that checks if the
>> > requested sequence has passed compares this against the 64-bit vblank
>> > count.
>>
>> For patches 1-7:
>>
>> Reviewed-by: Keith Packard 
>
> Dave, ack to merge them through drm-intel-next-queued ?

Ack. do we know if any of those need to be in -fixes?

or too early to tell?

Dave.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/10] drm/vblank: Data type fixes for 64-bit vblank sequences.

[Rodrigo Vivi]

On Sat, Feb 03, 2018 at 08:14:48AM +, Keith Packard wrote:
> Dhinakaran Pandiyan  writes:
> 
> > From: "Pandiyan, Dhinakaran" 
> >
> > drm_vblank_count() has an u32 type returning what is a 64-bit vblank count.
> > The effect of this is when drm_wait_vblank_ioctl() tries to widen the user
> > space requested vblank sequence using this clipped 32-bit count(when the
> > value is >= 2^32) as reference, the requested sequence remains a 32-bit
> > value and gets queued like that. However, the code that checks if the
> > requested sequence has passed compares this against the 64-bit vblank
> > count.
> 
> For patches 1-7:
> 
> Reviewed-by: Keith Packard 

Dave, ack to merge them through drm-intel-next-queued ?

Patches 8 to 10 are ready as well.

Thanks,
Rodrigo.

> 
> -- 
> -keith


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/10] drm/vblank: Data type fixes for 64-bit vblank sequences.

[Keith Packard]

Dhinakaran Pandiyan  writes:

> From: "Pandiyan, Dhinakaran" 
>
> drm_vblank_count() has an u32 type returning what is a 64-bit vblank count.
> The effect of this is when drm_wait_vblank_ioctl() tries to widen the user
> space requested vblank sequence using this clipped 32-bit count(when the
> value is >= 2^32) as reference, the requested sequence remains a 32-bit
> value and gets queued like that. However, the code that checks if the
> requested sequence has passed compares this against the 64-bit vblank
> count.

For patches 1-7:

Reviewed-by: Keith Packard 

-- 
-keith


signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 01/10] drm/vblank: Data type fixes for 64-bit vblank sequences.

[Dhinakaran Pandiyan]

From: "Pandiyan, Dhinakaran" 

drm_vblank_count() has an u32 type returning what is a 64-bit vblank count.
The effect of this is when drm_wait_vblank_ioctl() tries to widen the user
space requested vblank sequence using this clipped 32-bit count(when the
value is >= 2^32) as reference, the requested sequence remains a 32-bit
value and gets queued like that. However, the code that checks if the
requested sequence has passed compares this against the 64-bit vblank
count.

With drm_vblank_count() returning all bits of the vblank count, update
drm_crtc_accurate_vblank_count() so that drm_crtc_arm_vblank_event() queues
the correct sequence. Otherwise, this leads to prolonged waits for a vblank
sequence when the current count is >=2^32.

Finally, fix drm_wait_one_vblank() too.

v2: Commit message fix (Keith)
Squash commits (Rodrigo)

Fixes: 570e86963a51 ("drm: Widen vblank count to 64-bits [v3]")
Cc: Keith Packard 
Cc: Michel Dänzer 
Cc: Daniel Vetter 
Cc: Rodrigo Vivi 
Signed-off-by: Dhinakaran Pandiyan 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_vblank.c | 8 
 include/drm/drm_vblank.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 32d9bcf5be7f..f0d3ed5f2528 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -271,7 +271,7 @@ static void drm_update_vblank_count(struct drm_device *dev, 
unsigned int pipe,
store_vblank(dev, pipe, diff, t_vblank, cur_vblank);
 }
 
-static u32 drm_vblank_count(struct drm_device *dev, unsigned int pipe)
+static u64 drm_vblank_count(struct drm_device *dev, unsigned int pipe)
 {
struct drm_vblank_crtc *vblank = >vblank[pipe];
 
@@ -292,11 +292,11 @@ static u32 drm_vblank_count(struct drm_device *dev, 
unsigned int pipe)
  * This is mostly useful for hardware that can obtain the scanout position, but
  * doesn't have a hardware frame counter.
  */
-u32 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc)
+u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc)
 {
struct drm_device *dev = crtc->dev;
unsigned int pipe = drm_crtc_index(crtc);
-   u32 vblank;
+   u64 vblank;
unsigned long flags;
 
WARN_ONCE(drm_debug & DRM_UT_VBL && !dev->driver->get_vblank_timestamp,
@@ -1055,7 +1055,7 @@ void drm_wait_one_vblank(struct drm_device *dev, unsigned 
int pipe)
 {
struct drm_vblank_crtc *vblank = >vblank[pipe];
int ret;
-   u32 last;
+   u64 last;
 
if (WARN_ON(pipe >= dev->num_crtcs))
return;
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index 848b463a0af5..a4c3b0a0a197 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -179,7 +179,7 @@ void drm_crtc_wait_one_vblank(struct drm_crtc *crtc);
 void drm_crtc_vblank_off(struct drm_crtc *crtc);
 void drm_crtc_vblank_reset(struct drm_crtc *crtc);
 void drm_crtc_vblank_on(struct drm_crtc *crtc);
-u32 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc);
+u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc);
 
 bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
   unsigned int pipe, int *max_error,
-- 
2.14.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx