>-----Original Message----- >From: Joonas Lahtinen [mailto:joonas.lahtinen@xxxxxxxxxxxxxxx] >Sent: Wednesday, October 4, 2017 5:52 AM >To: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>; Srivatsa, Anusha ><anusha.srivatsa@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx >Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>; Sundaresan, Sujaritha ><sujaritha.sundaresan@xxxxxxxxx> >Subject: Re: [PATCH 1/2] drm/i915/guc: Add GuC Load time to dmesg >log. > >On Wed, 2017-10-04 at 09:16 +0100, Tvrtko Ursulin wrote: >> On 04/10/2017 01:41, Anusha Srivatsa wrote: >> > Calculate the time that GuC takes to load using jiffies. This >> > information could be very useful in determining if GuC is taking >> > unreasonably long time to load in a certain platforms. >> > >> > v2: Calculate time before logs are collected. >> > Move the guc_load_time variable as a part of intel_uc_fw struct. >> > Store only final result which is to be exported to debugfs. (Michal) >> > Add the load time in the print message as well. >> > >> > v3: Remove debugfs entry. Remove local variable guc_finish_load. >> > (Daniel, Tvrtko) >> > >> > v4: Use ktime_get() instead of jiffies. Use DRM_NOTE if time taken >> > to load is more than the threshold. On load times within acceptable >> > range, use DRM_DEBUG_DRIVER >> > (Tvrtko) >> > >> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> >> > Cc: Tvrtko ursulin <tvrtko.ursulin@xxxxxxxxx> >> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> >> > Cc: Sujaritha Sundaresan <sujaritha.sundaresan@xxxxxxxxx> >> > Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx> >> > Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> >> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx> > ><SNIP> > >> > @@ -373,6 +379,8 @@ int intel_guc_init_hw(struct intel_guc *guc) >> > guc->fw.path, >> > guc->fw.major_ver_found, guc->fw.minor_ver_found); >> > >> > + DRM_DEBUG_DRIVER("GuC is loaded in: %lld ms\n",guc->fw.load_time); >> > + >> >> If you move this debug to where the DRM_NOTE is you don't have to >> store the load time in the global structure. Unless there will be a >> reason in the future to have the value stored? > >We decided not to expose it through , so let's try to avoid a variable, too. Hi Joonas, The order of prints will then be: GuC firmware status: fetch SUCCESS load PENDING GuC loaded in x ms GuC loaded/submission enabled firmware <path_to_firmware>_major.minor.bin version major.minor Compared to GuC firmware status: fetch SUCCESS load PENDING GuC loaded/submission enabled firmware <path_to_firmware>_platform_major.minor.bin version major.minor GuC loaded in x ms I felt the second one looked a better sequence, but I will change if that’s the majority. Thanks, Anusha >Regards, Joonas > >PS. Also, lets try to get to the habit of having the S-o-b: and Cc:s in chronological >order. Less fixing for committers when applying patches. Will keep in mind, thanks Anusha >Joonas Lahtinen >Open Source Technology Center >Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx