On Wed, May 13, 2015 at 01:10:17AM +0300, Dmitry V. Levin wrote: > On Tue, May 12, 2015 at 07:37:59PM +0200, Gabriel Laskar wrote: > > On Tue, 12 May 2015 14:35:28 +0200, Patrik Jakobsson wrote: > > > On Mon, May 11, 2015 at 08:08:19PM +0200, Gabriel Laskar wrote: > > > > On Mon, 11 May 2015 15:54:24 +0200, Patrik Jakobsson wrote: > > > > > On Mon, May 11, 2015 at 12:50:36PM +0200, Gabriel Laskar wrote: > > > > > > On Wed, 6 May 2015 16:48:01 +0200, Patrik Jakobsson wrote: > > > > > > > > > > > > > This patch set aims to make strace more useful when tracing i915 ioctls. > > > > > > > The ioctl type is first checked for being drm and then the driver > > > > > > > backing the opened device is identified by looking at sysfs. Other > > > > > > > drivers than i915 can easily be added. > > > > > > > > > > > > > > Only a subset of the i915 ioctls are included. I will extend this patch > > > > > > > set if the approach looks ok. The generic drm ioctls are also missing. > > > > > > > > > > > > > > Give it a spin with: > > > > > > > strace -e trace=ioctl -p `pidof X` > > > > > > > > > > > > > > Patrik Jakobsson (2): > > > > > > > strace/drm: Print extended info for drm and i915 ioctls > > > > > > > strace/drm: Print args for most common i915 ioctls > > > > > > > > > > > > > > Makefile.am | 2 + > > > > > > > defs.h | 2 + > > > > > > > drm.c | 104 +++++++++++++++++ > > > > > > > drm_i915.c | 278 +++++++++++++++++++++++++++++++++++++++++++++ > > > > > > > ioctl.c | 5 + > > > > > > > xlat/drm_i915_getparams.in | 28 +++++ > > > > > > > xlat/drm_i915_ioctls.in | 51 +++++++++ > > > > > > > xlat/drm_i915_setparams.in | 4 + > > > > > > > 8 files changed, 474 insertions(+) > > > > > > > create mode 100644 drm.c > > > > > > > create mode 100644 drm_i915.c > > > > > > > create mode 100644 xlat/drm_i915_getparams.in > > > > > > > create mode 100644 xlat/drm_i915_ioctls.in > > > > > > > create mode 100644 xlat/drm_i915_setparams.in > > > > > > > > > > > > > > > > > > > This is a great start! We need this kind of decoding. Do you plan to > > > > > > add also the generic drm ioctl decoding? > > > > > > > > > > Thanks for the review. Yes, my plan is to add generic drm ioctls as well. > > > > > > > > > > > > > > > > > Some issues though: > > > > > > > > > > > > * The way you avoid the ioctl request decoding is quite hard to follow, > > > > > > but it seems that you don't have much of a choice, except that in > > > > > > drm_ioctl(), the code from SYS_FUNC(ioctl) is duplicated. It seems it > > > > > > needs some work here, to allow a simpler code path. Maybe this would > > > > > > be clearer if the decoding/drm_get_driver_name, etc… was in > > > > > > ioctl_decode_command_number(). Also, with the actual code, if you are > > > > > > on i915 with an invalid ioctl number, it will be printed as > > > > > > "I915_IOCTL_???" and not "_IOC(...)" (see below for an example.) This > > > > > > will also add an inconsistent result depending whether /sys is > > > > > > mounted or not. > > > > > > > > > > Yes, moving it to ioctl_decode_command_number() makes sense. I'll do that. > > > > > And I'll make the output consistent and skip the I915_IOCTL_???. It comes with > > > > > the drawback of possibly duplicated entries when doing the lookup even though we > > > > > know we're talking to i915, but it is still nicer than _???. > > > > > > > > If you call all the request decoding code from > > > > ioctl_decode_command_number() you will still be able to determine if > > > > you are on i915, and write the correct request, but the fallback code > > > > will be no longer duplicated. You will have to call xlookup() instead of > > > > printxval(), in order to be able to know when the decoding fail though. > > > > > > Unfortunately I cannot check for i915 in _decode_command_number since I don't > > > have the full tcb context (cannot figure out the path, etc.). That's probably > > > why I had to duplicate the decoding in the first place. The only other solution > > > I see is to detect i915 early and store it globally somewhere but that is rather > > > nasty. Not sure how to do this in a better way. What am I missing? > > > > Oh yes, I see. We can add tcb context to ioctl_decode_command_number(), > > it should not be a problem. We didn't have the need for it for the > > moment, that's all. > > > > Dmitry? What do you think of this? > > I had no chance to have a look at the code, but anyway, passing > tcb context down to ioctl_decode_command_number() shouldn't be a problem > at all. Sounds good. Then I'll use ioctl_decode_command_number as the dispatcher. Thanks Patrik > > > -- > ldv _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
