Re: [i-g-t PATCH v2 1/2] lib: update kmstest_get_pipe_from_crtc_id

[Tomeu Vizoso <tomeu.vizoso@collabora.com>]

On 18 April 2016 at 14:09, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Mar 09, 2016 at 04:57:37PM +0100, Tomeu Vizoso wrote:
>> From: Micah Fedke <micah.fedke@collabora.co.uk>
>>
>> This function uses an intel-specific ioctl to fetch a mapping between pipes and
>> crtc ids, but this technique is outdated as the crtc id is now always
>> equivalent to its index in the array of crtcs returned by the kernel.
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>
> Ok, there's 2 use-cases for this one really:
> - intel hw specific stuff, like CS scanline waits (like in the overlay, or
>   other places)
> - to figure out the pipe ID for the vblank ioctl
>
> The former is i915 specific, the later is generic. I think the correct way
> to fix this is to have a helper to encode a drmCrtc into the vblank flags.
> That's needed anyway since the rules are silly: First a flag to select
> between pipe 0 and 1, then a bitfield for 2 and later.
>
> Once we have that helper (maybe call it kmstests_vblank_flags_for_crtc or
> similar) we can roll it out to all the places that use the vblank ioctl.
> Or which check vblank events (from atomic or legacy page flips), and
> should be left with just the i915-specific usage.
>
> A bit more work, but I think cleaner in intent.
>
> Thoughts?

Sounds good in principle, will give it a try.

Thanks,

Tomeu

> -Daniel
>
>> ---
>>
>> Changes in v2: None
>>
>>  lib/igt_kms.c | 33 ++++++++++++++++++++++++---------
>>  1 file changed, 24 insertions(+), 9 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 9f18aef72ea0..1d9acce31676 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -261,20 +261,35 @@ void kmstest_dump_mode(drmModeModeInfo *mode)
>>   * @fd: DRM fd
>>   * @crtc_id: DRM CRTC id
>>   *
>> - * Returns: The pipe number for the given DRM CRTC @crtc_id. This maps directly
>> - * to an enum pipe value used in other helper functions.
>> + * Returns: The crtc index for the given DRM CRTC ID @crtc_id. The crtc index
>> + * is the equivalent of the pipe id.  This value maps directly to an enum pipe
>> + * value used in other helper functions.  Returns 0 if the index could not be
>> + * determined.
>>   */
>> +
>>  int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id)
>>  {
>> -     struct drm_i915_get_pipe_from_crtc_id pfci;
>> -     int ret;
>> +     drmModeRes *res;
>> +     drmModeCrtc *crtc;
>> +     int i, cur_id;
>> +
>> +     res = drmModeGetResources(fd);
>> +     igt_assert(res);
>> +
>> +     for (i = 0; i < res->count_crtcs; i++) {
>> +             crtc = drmModeGetCrtc(fd, res->crtcs[i]);
>> +             igt_assert(crtc);
>> +             cur_id = crtc->crtc_id;
>> +             drmModeFreeCrtc(crtc);
>> +             if (cur_id == crtc_id)
>> +                     break;
>> +     }
>> +
>> +     drmModeFreeResources(res);
>>
>> -     memset(&pfci, 0, sizeof(pfci));
>> -     pfci.crtc_id = crtc_id;
>> -     ret = drmIoctl(fd, DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID, &pfci);
>> -     igt_assert(ret == 0);
>> +     igt_assert(i < res->count_crtcs);
>>
>> -     return pfci.pipe;
>> +     return i;
>>  }
>>
>>  /**
>> --
>> 2.5.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx