Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] lib: Rework __kms_addfb() function
Quoting Arkadiusz Hiler (2019-04-10 12:57:03) > On Wed, Apr 10, 2019 at 12:34:07PM +0100, Chris Wilson wrote: > > Quoting Arkadiusz Hiler (2019-04-10 12:28:08) > > > On Wed, Apr 03, 2019 at 07:24:39PM -0300, Rodrigo Siqueira wrote: > > > > The function __kms_addfb() and drmModeAddFB2WithModifiers() have a > > > > similar code. Due to this similarity, this commit replace part of the > > > > code inside __kms_addfb() by using drmModeAddFB2WithModifiers(). > > > > > > igt master % grep '^libdrm_version' meson.build > > > libdrm_version = '>=2.4.82' > > > > Why introduce a dependency? The primary purpose of igt is breaking > > ioctls, and that typically requires avoiding any protective libraries. > > In particular, we want to be very, very clear about what igt is doing > > (in terms of kernel api/abi) and not obfuscate. > > -Chris > > We depend on one additional call from a library that we already use > widely across the whole codebase and it's straight up a simple wrapper. > Which seems to be a pattern - we us those simple wrapper where > available, and write our own if there is nothing suitable. I completely disagree with that assessment, and do not think we should undermine igt so lightly. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] lib: Rework __kms_addfb() function
On Wed, Apr 10, 2019 at 12:34:07PM +0100, Chris Wilson wrote: > Quoting Arkadiusz Hiler (2019-04-10 12:28:08) > > On Wed, Apr 03, 2019 at 07:24:39PM -0300, Rodrigo Siqueira wrote: > > > The function __kms_addfb() and drmModeAddFB2WithModifiers() have a > > > similar code. Due to this similarity, this commit replace part of the > > > code inside __kms_addfb() by using drmModeAddFB2WithModifiers(). > > > > igt master % grep '^libdrm_version' meson.build > > libdrm_version = '>=2.4.82' > > Why introduce a dependency? The primary purpose of igt is breaking > ioctls, and that typically requires avoiding any protective libraries. > In particular, we want to be very, very clear about what igt is doing > (in terms of kernel api/abi) and not obfuscate. > -Chris We depend on one additional call from a library that we already use widely across the whole codebase and it's straight up a simple wrapper. Which seems to be a pattern - we us those simple wrapper where available, and write our own if there is nothing suitable. It's ok to have some tests that poke the IOCTL more directly, like we kms_addfb_basic for ADDFB2 (which still uses drmIoctl, so where do we draw a line?). If any of thoe wrappers goes awry we can just drop-in replace it with the original, simple version anyway. -- Cheers, Arek ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] lib: Rework __kms_addfb() function
Quoting Arkadiusz Hiler (2019-04-10 12:28:08) > On Wed, Apr 03, 2019 at 07:24:39PM -0300, Rodrigo Siqueira wrote: > > The function __kms_addfb() and drmModeAddFB2WithModifiers() have a > > similar code. Due to this similarity, this commit replace part of the > > code inside __kms_addfb() by using drmModeAddFB2WithModifiers(). > > igt master % grep '^libdrm_version' meson.build > libdrm_version = '>=2.4.82' Why introduce a dependency? The primary purpose of igt is breaking ioctls, and that typically requires avoiding any protective libraries. In particular, we want to be very, very clear about what igt is doing (in terms of kernel api/abi) and not obfuscate. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
