[Beignet] [PATCH] Handle the difference timestamp count, got from drm_intel_reg_read, between 32bit system and 64bit system.

[Yang, Rong R]

Yes, you are right.
I have sent out a new patch, suppose could handle these special cases.
Thanks.

-----Original Message-----
From: michael.j.ferguson at L-3com.com [mailto:michael.j.ferguson at L-3com.com] 
Sent: Tuesday, June 10, 2014 10:57 PM
To: zhigang.gong at linux.intel.com; junyan.he at inbox.com
Cc: Yang, Rong R; beignet at lists.freedesktop.org
Subject: RE: [Beignet] [PATCH] Handle the difference timestamp count, got from drm_intel_reg_read, between 32bit system and 64bit system.

This did *not* work on the Baytrail-I E3827. After applying your patch I had to further apply the following to make it work. It looks like Baytrail keeps the timestamp count in the lower 32 bits, like i386, even though it is an x86_64 processor:

diff --git a/src/intel/intel_gpgpu.c b/src/intel/intel_gpgpu.c index 7aa5563..fac7b47 100644
--- a/src/intel/intel_gpgpu.c
+++ b/src/intel/intel_gpgpu.c
@@ -1175,7 +1175,7 @@ intel_gpgpu_event_get_gpu_cur_timestamp(intel_gpgpu_t* gpgpu, uint64_t* ret_ts)
      i386 system. It seems the kernel readq bug. So shift 32 bit in x86_64, and only remain
      32 bits data in i386.
   */
-#ifdef __i386__
+#if defined(__i386__) || defined(PCI_CHIP_BAYTRAIL_T)
   result = result & 0x0ffffffff;
 #else
   result = result >> 32; 

-----Original Message-----
From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com]
Sent: Tuesday, June 10, 2014 1:33 AM
To: He Junyan
Cc: Yang Rong; beignet at lists.freedesktop.org; Ferguson, Michael J @ CSG - CSW
Subject: Re: [Beignet] [PATCH] Handle the difference timestamp count, got from drm_intel_reg_read, between 32bit system and 64bit system.

Thanks for the testing report.
cc to michael, could you try this patch and check whether it works on your Baytrail platform Thanks.

On Tue, Jun 10, 2014 at 04:14:26PM +0800, He Junyan wrote:
> I verified this patch on both my 32bits and 64bits IVB platforms.
> Both results are OK and pass the profiling_exec test case.
> 
> 
> On Tue, 2014-06-10 at 16:17 +0800, Yang Rong wrote:
> > In x86_64 system, the low 32bits of timestamp count are stored in 
> > the high 32 bits of result which got from drm_intel_reg_read, and 32-35 bits are lost; but in i386 system, the timestamp count match bspec.
> > It seems the kernel readq bug. So shift 32 bit in x86_64, and only remain 32 bits data in i386.
> > 
> > Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> > ---
> >  src/intel/intel_gpgpu.c | 31 ++++++++++++++++---------------
> >  1 file changed, 16 insertions(+), 15 deletions(-)
> > 
> > diff --git a/src/intel/intel_gpgpu.c b/src/intel/intel_gpgpu.c index
> > a1bd672..7aa5563 100644
> > --- a/src/intel/intel_gpgpu.c
> > +++ b/src/intel/intel_gpgpu.c
> > @@ -1170,12 +1170,16 @@ intel_gpgpu_event_get_gpu_cur_timestamp(intel_gpgpu_t* gpgpu, uint64_t* ret_ts)
> >    drm_intel_bufmgr *bufmgr = gpgpu->drv->bufmgr;
> >  
> >    drm_intel_reg_read(bufmgr, TIMESTAMP_ADDR, &result);
> > -  if (IS_HASWELL(gpgpu->drv->device_id)) {
> > -    result = result & 0x0000000FFFFFFFFF;
> > -  } else {
> > -    result = result & 0xFFFFFFFFF0000000;
> > -    result = result >> 28;
> > -  }
> > +  /* In x86_64 system, the low 32bits of timestamp count are stored in the high 32 bits of
> > +     result which got from drm_intel_reg_read, and 32-35 bits are lost; but match bspec in
> > +     i386 system. It seems the kernel readq bug. So shift 32 bit in x86_64, and only remain
> > +     32 bits data in i386.
> > +  */
> > +#ifdef __i386__
> > +  result = result & 0x0ffffffff;
> > +#else
> > +  result = result >> 32;
> > +#endif  /* __i386__  */
> >    result *= 80;
> >  
> >    *ret_ts = result;
> > @@ -1195,15 +1199,12 @@ intel_gpgpu_event_get_exec_timestamp(intel_gpgpu_t* gpgpu, intel_event_t *event,
> >    uint64_t* ptr = event->ts_buf->virtual;
> >    result = ptr[index];
> >  
> > -  if (IS_HASWELL(gpgpu->drv->device_id))
> > -    result = (result & 0xFFFFFFFFF) * 80; //convert to nanoseconds
> > -  else
> > -    /* According to BSpec, the timestamp counter should be 36 bits,
> > -       but comparing to the timestamp counter from IO control reading,
> > -       we find the first 4 bits seems to be fake. In order to keep the
> > -       timestamp counter conformable, we just skip the first 4 bits.
> > -     */
> > -    result = ((result & 0x0FFFFFFFF) << 4) * 80; //convert to nanoseconds
> > +  /* According to BSpec, the timestamp counter should be 36 bits,
> > +     but comparing to the timestamp counter from IO control reading,
> > +     we find the first 4 bits seems to be fake. In order to keep the
> > +     timestamp counter conformable, we just skip the first 4 bits.
> > +  */
> > +  result = (result & 0x0FFFFFFFF) * 80; //convert to nanoseconds
> >    *ret_ts = result;
> >  
> >    drm_intel_gem_bo_unmap_gtt(event->ts_buf);
> 
> 
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet