Re: [igt-dev] [PATCH 1/2] lib/ioctl_wrappers: Query if device supports set/get legacy tiling

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Thu, 14 Nov 2019 21:50:39 -0800, Vanshidhar Konda wrote:
>
> On Thu, Nov 14, 2019 at 09:42:24PM -0800, Dixit, Ashutosh wrote:
> > On Thu, 14 Nov 2019 21:33:25 -0800, Vanshidhar Konda wrote:
> >>
> >> Add a method to query if the device supports setting and
> >> getting legacy tiling formats for buffer objects.
> >>
> >> Signed-off-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
> >> ---
> >>  lib/ioctl_wrappers.c | 16 ++++++++++++++++
> >>  lib/ioctl_wrappers.h |  1 +
> >>  2 files changed, 17 insertions(+)
> >>
> >> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> >> index 628f8b83..837ef2cf 100644
> >> --- a/lib/ioctl_wrappers.c
> >> +++ b/lib/ioctl_wrappers.c
> >> @@ -133,6 +133,22 @@ __gem_get_tiling(int fd, struct drm_i915_gem_get_tiling *arg)
> >>	return err;
> >>  }
> >>
> >> +/**
> >> + * gem_has_legacy_hw_tiling:
> >> + * @fd: open i915 drm file descriptor
> >> + *
> >> + * Feature check to query if the device supports setting/getting
> >> + * legacy tiling formats for buffer objects
> >> + *
> >> + * Returns: True if tiling is supported
> >> + */
> >> +bool
> >> +gem_has_legacy_hw_tiling(int fd)
> >> +{
> >> +	struct drm_i915_gem_get_tiling arg = {};
> >> +	return (__gem_get_tiling(fd, &arg) != -EOPNOTSUPP);
> >
> > Probably just
> >
> >	return !__gem_get_tiling(fd, &arg);
>
> No. Only this specific error means it is not supported by the hardware.
> Other errors could mean that I made a mistake with the arguments - I've
> not set them to valid values.

__gem_get_tiling() returns 0 if tiling is supported, it returns -EOPNOTSUPP
if tiling is not supported and if it returns anything else we don't know if
tiling is supported or not supported (error in arguments etc.). So if
gem_has_legacy_hw_tiling() is to definitively indicate tiling is supported
it should just:

	return !__gem_get_tiling(fd, &arg);

For example, if __gem_get_tiling() returns -EINVAL I don't think we want to
return true from gem_has_legacy_hw_tiling().


_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev