Re: [PATCH] drm/i915: Use ktime on wait_for

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Sagar Arun Kamble (2018-04-20 11:23:50)
>> 
>> 
>> On 4/20/2018 3:24 PM, Mika Kuoppala wrote:
>> > We use jiffies to determine when wait expires. However
>> > Imre did find out that jiffies can and will do a >1
>> > increments on certain situations [1]. When this happens
>> > in a wait_for loop, we return timeout errorneously
>> > much earlier than what the real wallclock would say.
>> >
>> > We can't afford our waits to timeout prematurely.
>> > Discard jiffies and change to ktime to detect timeouts.
>> >
>> > Reported-by: Imre Deak <imre.deak@intel.com>
>> > References: https://lkml.org/lkml/2018/4/18/798 [1]
>> > Cc: Imre Deak <imre.deak@intel.com>
>> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> > ---
>> >   drivers/gpu/drm/i915/intel_drv.h | 4 ++--
>> >   1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> > index 8b20824e806e..ac7565220aa3 100644
>> > --- a/drivers/gpu/drm/i915/intel_drv.h
>> > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> > @@ -49,12 +49,12 @@
>> >    * check the condition before the timeout.
>> >    */
>> >   #define __wait_for(OP, COND, US, Wmin, Wmax) ({ \
>> > -     unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1;   \
>> > +     const ktime_t end__ = ktime_add_ns(ktime_get_raw(), 1000ll * (US)); \
>> Is ktime_get_raw() monotonic? Thomas suggested ktime_get()
>
> It proclaims to be monotonic, without the clock drift calibration. For
> the milliseconds we should be sleeping at most, I hope that is
> immaterial.

Agreed. And even if we would be affected by drift, we would want
to extend the end a little instead of accepting a drift calibration
while we are waiting.

Here is how the code size is affected:

add/remove: 0/0 grow/shrink: 5/29 up/down: 77/-1099 (-1022)
Function                                     old     new   delta
__intel_wait_for_register_fw                 462     496     +34
swsci                                        576     596     +20
skl_pcode_request                            488     507     +19
intel_hdcp_auth                             3806    3809      +3
__intel_wait_for_register                    436     437      +1
hsw_power_well_disable                       548     539      -9
i915_gem_idle_work_handler                   608     593     -15
guc_fw_xfer                                  875     854     -21
bxt_ddi_pll_disable                          324     303     -21
chv_set_cdclk                                357     334     -23
vlv_wm_get_hw_state                         2474    2450     -24
gmbus_wait                                   564     538     -26
cnl_cdclk_pll_enable                         286     259     -27
cnl_cdclk_pll_disable                        247     220     -27
hsw_enable_pc8                              1699    1671     -28
bdw_set_cdclk                               1006     977     -29
wait_for_pipe_scanline_moving                388     358     -30
intel_enable_dsi_pll                        1060    1028     -32
i915_gem_wait_for_idle                       348     314     -34
chv_set_memory_dvfs                          258     223     -35
vlv_set_power_well                           321     282     -39
intel_hdmi_hdcp_check_link                   485     446     -39
chv_set_pipe_power_well.constprop            359     320     -39
g33_do_reset                                 216     175     -41
vlv_wait_for_pw_status                       210     168     -42
intel_engines_park                           348     306     -42
vlv_set_cdclk                                711     662     -49
lspcon_wait_mode                             265     211     -54
lpt_init_pch_refclk                         1542    1488     -54
g4x_do_reset                                 595     541     -54
bxt_ddi_pll_enable                          2407    2353     -54
ironlake_crtc_enable                        3279    3211     -68
i915_do_reset                                443     375     -68
intel_guc_send_ct                           1691    1616     -75
Total: Before=1232834, After=1231812, chg -0.08%

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