[igt-dev] [PATCH i-g-t 18/23] i915/perf: Fix DG2 A0 report header

[Lionel Landwerlin]

On 06/09/2022 21:37, Umesh Nerlige Ramappa wrote:
> On Tue, Sep 06, 2022 at 05:17:14PM +0300, Lionel Landwerlin wrote:
>> On 23/08/2022 21:30, Umesh Nerlige Ramappa wrote:
>>> Timestamp is still 32 bits in DG2 A0. Fix that. Specify variants of DG2
>>> that have the OA format header bug
>>>
>>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>>> ---
>>>  tests/i915/perf.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 44 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/i915/perf.c b/tests/i915/perf.c
>>> index 52de5d38..6951735b 100644
>>> --- a/tests/i915/perf.c
>>> +++ b/tests/i915/perf.c
>>> @@ -40,6 +40,7 @@
>>>  #include "i915/gem.h"
>>>  #include "i915/perf.h"
>>> +#include "i915_pciids.h"
>>>  #include "igt.h"
>>>  #include "igt_perf.h"
>>>  #include "igt_sysfs.h"
>>> @@ -503,6 +504,47 @@ cs_timestamp_frequency(int fd)
>>>      return value;
>>>  }
>>> +/*
>>> + * Hacky ways until IGT is fully aware of steppings:
>>> + *
>>> + * WA is permanent for G11. For other DG2s, bug is fixed from B0 
>>> onwards.
>>> + * Steps A0 and A1 correspond to 0 and 1 revid values.
>>
>>
>> Do you have the Wa number for reference?
>
> Should be - 1608133521, the corresponding kernel patch is
> https://patchwork.freedesktop.org/patch/498940/?series=107584&rev=2
>
> Thanks,
> Umesh


If you can put it in the comment :


Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>


Thanks!


>>
>>
>>> + */
>>> +struct perf_dev_info {
>>> +    bool is_dg2_g11;
>>> +};
>>> +
>>> +static const struct perf_dev_info dg2_g11_info = {
>>> +    .is_dg2_g11 = true,
>>> +};
>>> +
>>> +static const struct pci_id_match perf_dev_match[] = {
>>> +    INTEL_DG2_G11_IDS(&dg2_g11_info),
>>> +};
>>> +
>>> +static const struct perf_dev_info *__dev_info(uint32_t device_id)
>>> +{
>>> +    static const struct perf_dev_info *cache;
>>> +    static int _iter;
>>> +
>>> +    if (_iter)
>>> +        goto out;
>>> +
>>> +    for (_iter = 0; _iter < ARRAY_SIZE(perf_dev_match); _iter++)
>>> +        if (device_id == perf_dev_match[_iter].device_id)
>>> +            cache = (void *) perf_dev_match[_iter].match_data;
>>> +
>>> +out:
>>> +    return cache;
>>> +}
>>> +
>>> +#define IS_DG2_G11(__d) (__dev_info(__d) ? 
>>> __dev_info(__d)->is_dg2_g11 : false)
>>> +#define IS_DG2_A0(__d) (IS_DG2(__d) && 
>>> (intel_perf->devinfo.revision == 0))
>>> +#define IS_DG2_A1(__d) (IS_DG2(__d) && 
>>> (intel_perf->devinfo.revision == 1))
>>> +
>>> +#define HAS_OA_FORMAT_64BIT_HDR_BUG(__d) \
>>> +    (IS_DG2_G11(__d) || IS_DG2_A0(__d) || IS_DG2_A1(__d))
>>> +
>>>  static uint64_t
>>>  cs_timebase_scale(uint32_t u32_delta)
>>>  {
>>> @@ -514,7 +556,8 @@ oa_timestamp(const uint32_t *report, enum 
>>> drm_i915_oa_format format)
>>>  {
>>>      struct oa_format fmt = get_oa_format(format);
>>> -    return fmt.report_hdr_64bit ? *(uint64_t *)&report[2] : report[1];
>>> +    return fmt.report_hdr_64bit && 
>>> !HAS_OA_FORMAT_64BIT_HDR_BUG(devid) ?
>>> +        *(uint64_t *)&report[2] : report[1];
>>>  }
>>>  static uint64_t
>>
>>