On Tue, Sep 19, 2017 at 08:49:21AM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of > >Maarten Lankhorst > >Sent: Tuesday, September 19, 2017 12:56 PM > >To: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Srinivas, Vidya <vidya.srinivas@xxxxxxxxx> > >Subject: Re: [PATCH 1/2] drm/i915: Enable scanline read for gen9 dsi > > > >Op 18-09-17 om 16:24 schreef Ville Syrjälä: > >> On Mon, Sep 18, 2017 at 03:57:38PM +0200, Maarten Lankhorst wrote: > >>> Op 18-09-17 om 15:32 schreef Vidya Srinivas: > >>>> From: Uma Shankar <uma.shankar@xxxxxxxxx> <snip> > >>>> + WARN_ON(!crtc_vtotal); > >>>> + if (!crtc_vtotal) > >>>> + return scanline; > >>>> + > >>>> + /* To avoid the race condition where we might cross into the > >>>> + * next vblank just between the PIPE_FRMTMSTMP and TIMESTAMP_CTR > >>>> + * reads. We make sure we read PIPE_FRMTMSTMP and > >TIMESTAMP_CTR > >>>> + * during the same frame. > >>>> + */ > >>>> + do { > >>>> + /* > >>>> + * This field provides read back of the display > >>>> + * pipe frame time stamp. The time stamp value > >>>> + * is sampled at every start of vertical blank. > >>>> + */ > >>>> + scan_prev_time = I915_READ_FW(PIPE_FRMTMSTMP(crtc- > >>pipe)); > >>>> + > >>>> + /* > >>>> + * The TIMESTAMP_CTR register has the current > >>>> + * time stamp value. > >>>> + */ > >>>> + scan_curr_time = I915_READ_FW(GEN7_TIMESTAMP_CTR); > >>>> + > >>>> + scan_post_time = I915_READ_FW(PIPE_FRMTMSTMP(crtc- > >>pipe)); > >>>> + } while (scan_post_time != scan_prev_time); > >>>> + > >>>> + /* > >>>> + * Since the register is 32 bit and the values > >>>> + * can overflow and wrap around, making sure > >>>> + * current time accounts for the register > >>>> + * wrap > >>>> + */ > >>>> + if (scan_curr_time < scan_prev_time) > >>>> + scan_curr_time += 0x100000000; > >>>> + > >>>> + scanline = div_u64(mul_u64_u32_shr((scan_curr_time - > >>>> +scan_prev_time), > >>> Isn't mul_u64_u32_div exactly what you want here? > >> I think we'd actually want a mul_u32_u32_div(). But that doesn't seem > >> to exist. > >Yeah, couldn't find it either. :( > > > >Why do we even use the macros, can't we simply do the multiplications and > >divide without? > >i915 is only on x86 anyway. :) > > mul_u32_u32 handles it differently using x86 assembly instructions (arch/x86/include/asm/div64.h). > I guess it's more precise than normal u32*u32 multiplication. It's a workaround for modern gcc which (for whatever reason) can no longer figure out that on a 32bit machine we can do this witha a normal 32x32->64 multiplication rather than a full blown 64 bit multiplication, which would be more expensive. Earlier gcc versions did work this out correctly without handholding, but somehow they broke it at some point. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx