[igt-dev] [PATCH i-g-t v11 1/2] lib/igt_ioctl_wrappers: Add igt_has_drm_cap ioctl wrapper

[Karthik B S]

On 9/30/2020 11:48 AM, Karthik B S wrote:
> 
> 
> On 9/29/2020 10:07 PM, Ville Syrjälä wrote:
>> On Tue, Sep 29, 2020 at 03:16:56PM +0530, Karthik B S wrote:
>>>
>>>
>>> On 9/25/2020 8:38 PM, Ville Syrjälä wrote:
>>>> On Fri, Sep 25, 2020 at 09:56:12AM +0530, Karthik B S wrote:
>>>>> Add a generic helper for DRM_IOCTL_GET_CAP ioctl.
>>>>>
>>>>> v10: -No changes.
>>>>>
>>>>> v11: -Pass cap ID instead of the whole structure. (Ville)
>>>>>        -Fix the patch ordering. (Ville)
>>>>>
>>>>> Signed-off-by: Karthik B S <karthik.b.s at intel.com>
>>>>> ---
>>>>>    lib/ioctl_wrappers.c | 17 +++++++++++++++++
>>>>>    lib/ioctl_wrappers.h |  1 +
>>>>>    2 files changed, 18 insertions(+)
>>>>>
>>>>> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
>>>>> index 3781286d..3f35609a 100644
>>>>> --- a/lib/ioctl_wrappers.c
>>>>> +++ b/lib/ioctl_wrappers.c
>>>>> @@ -1318,3 +1318,20 @@ int __kms_addfb(int fd, uint32_t handle,
>>>>>        return ret < 0 ? -errno : ret;
>>>>>    }
>>>>> +
>>>>> +/**
>>>>> + * igt_has_drm_cap:
>>>>> + * @fd: Open DRM file descriptor.
>>>>> + * @cap: drm_get_cap structure.
>>>>> + *
>>>>> + * This helper verifies if the passed cap is supported by the kernel
>>>>> + *
>>>>> + * Returns: Whether the cap is supported or not.
>>>>> + */
>>>>> +bool igt_has_drm_cap(int fd, uint64_t flag)
>>>>
>>>> The docs and code here don't match. I fixed that up and
>>>> s/flag/capability/ for clarity.
>>>>
>>>> There was also an int vs. pointer compiler warning in the
>>>> othr patch  which I fixed.
>>>>
>>>> Series pushed to master. Thanks.
>>>>
>>>
>>> Thanks for the merge.
>>
>> CI is not very happy with these. Pls investigate.
>>
> 

I've sent a patch with minor tweaks to the existing logic,
to fix the failures seen on CI.
https://patchwork.freedesktop.org/series/82567/

Could you please take a look.

Thanks,
Karthik.B.S
> I went through the results for kms_async_flips on CI.
> Other than alternate-sync-async-flip subtest on SKL which is giving warn 
> in dmesg "Atomic update on pipe (A) took 325 us, max time under evasion 
> is 250 us", all the other failures are sporadic and failing due to one 
> of the async flips in between taking longer time.
> 
> Two things are coming to my mind to handle this.
> 1. Reduce the MIN_FLIPS_PER_FRAME to 5 may be and check if this can 
> handle the error across all platforms.
> 
> (Or)
> 
> 2. Remove the assert where we check the flip interval of each async flip.
> 
> For subtest async-flip-with-page-flip-events, anyway we're having a FPS 
> check at the end which measures the overall flip rate.
> 
> For subtest alternate-sync-async-flip, may be I add a buffer to allow a 
> couple of flips to take longer than the threshold before actually 
> failing the test.
> 
> Which approach would you suggest? or any other way you would suggest to 
> handle this better?
> 
> Also looks like some of the HW are faster than others.
> For Ex, we're not seeing any failures on ICL, whereas many failures are 
> seen on KBL. Would you suggest we have MIN_FLIPS_PER_FRAME at a platform 
> level?
> 
> I will also try to find out what is causing the dmesg warn only for SKL 
> in case of alternate async and sync flips.
> Any thing that you could point to, that should be changed for SKL in 
> particular to take care of the dmesg warning?
> 
> Thanks,
> Karthik.B.S
>>>
>>> Thanks,
>>> Karthik.B.S
>>>>> +{
>>>>> +    struct drm_get_cap cap = { .capability = flag };
>>>>> +
>>>>> +    igt_assert(drmIoctl(fd, DRM_IOCTL_GET_CAP, &cap) == 0);
>>>>> +    return cap.value;
>>>>> +}
>>>>> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
>>>>> index 870ac8b7..e0ec203d 100644
>>>>> --- a/lib/ioctl_wrappers.h
>>>>> +++ b/lib/ioctl_wrappers.h
>>>>> @@ -196,6 +196,7 @@ struct local_drm_mode_fb_cmd2 {
>>>>>    bool igt_has_fb_modifiers(int fd);
>>>>>    void igt_require_fb_modifiers(int fd);
>>>>> +bool igt_has_drm_cap(int fd, uint64_t flag);
>>>>>    /**
>>>>>     * __kms_addfb:
>>>>> -- 
>>>>> 2.22.0
>>>>
>>