Re: [igt-dev] [PATCH i-g-t 3/4] lib/i915/perf: fill up reader devinfo default field

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Thu, Jan 07, 2021 at 12:03:56PM +0200, Lionel Landwerlin wrote:
>On 07/01/2021 02:23, Umesh Nerlige Ramappa wrote:
>>On Mon, Dec 28, 2020 at 05:19:39AM +0200, Lionel Landwerlin wrote:
>>>A bunch of fields were left to 0...
>>>
>>>Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>
>>
>>What is different between the devinfo in intel_perf vs the devinfo 
>>in intel_perf_data_reader. Since you already have a intel_perf 
>>object inside intel_perf_data_reader, wondering why you need to copy 
>>it over and then set the devid and timestamp_frequency. Why not just 
>>set it in reader->perf->devinfo.
>>
>>Irrespective,
>>
>>Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>
>>Thanks,
>>Umesh
>
>
>I don't remember my exact reasoning for having this twice....
>
>As far as I can tell they're exactly the same, I could just copy it 
>straight.
>
>
>Would that be okay as a replacement for this commit?
>

Yes, that helps since copying it just implies there is not difference 
between the two.

Thanks,
Umesh
>
>-Lionel
>
>
>>
>>>Fixes: 43116ee368585d ("lib/i915-perf: add i915 perf data reader")
>>>---
>>>lib/i915/perf_data_reader.c | 9 +++++----
>>>1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>>diff --git a/lib/i915/perf_data_reader.c b/lib/i915/perf_data_reader.c
>>>index 3b05a2e54..4b68fb502 100644
>>>--- a/lib/i915/perf_data_reader.c
>>>+++ b/lib/i915/perf_data_reader.c
>>>@@ -119,12 +119,13 @@ find_metric_set(struct intel_perf *perf, 
>>>const char *symbol_name)
>>>
>>>static void
>>>init_devinfo(struct intel_perf_devinfo *perf_devinfo,
>>>+         struct intel_perf *perf,
>>>         const struct intel_device_info *devinfo,
>>>         uint32_t devid,
>>>         uint64_t timestamp_frequency)
>>>{
>>>+    *perf_devinfo = perf->devinfo;
>>>    perf_devinfo->devid = devid;
>>>-    perf_devinfo->gen = devinfo->gen;
>>>    perf_devinfo->timestamp_frequency = timestamp_frequency;
>>>}
>>>
>>>@@ -203,15 +204,15 @@ parse_data(struct intel_perf_data_reader *reader)
>>>        return false;
>>>    }
>>>
>>>-    init_devinfo(&reader->devinfo, devinfo,
>>>-             record_info->device_id,
>>>-             record_info->timestamp_frequency);
>>>    reader->perf = intel_perf_for_devinfo(record_info->device_id,
>>>                          record_info->device_revision,
>>>                          record_info->timestamp_frequency,
>>>                          record_info->gt_min_frequency,
>>>                          record_info->gt_max_frequency,
>>>                          &record_topology->topology);
>>>+    init_devinfo(&reader->devinfo, reader->perf, devinfo,
>>>+             record_info->device_id,
>>>+             record_info->timestamp_frequency);
>>>
>>>    reader->metric_set_name = record_info->metric_set_name;
>>>    reader->metric_set_uuid = record_info->metric_set_uuid;
>>>-- 
>>>2.30.0.rc2
>>>
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev