linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Terje Bergström" <tbergstrom@nvidia.com>
To: Thierry Reding <thierry.reding@avionic-design.de>
Cc: Arto Merilainen <amerilainen@nvidia.com>,
	"airlied@linux.ie" <airlied@linux.ie>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCHv5,RESEND 4/8] gpu: host1x: Add debug support
Date: Wed, 6 Feb 2013 12:58:19 -0800	[thread overview]
Message-ID: <5112C3EB.7020205@nvidia.com> (raw)
In-Reply-To: <20130205091555.GC20437@avionic-0098.mockup.avionic-design.de>

On 05.02.2013 01:15, Thierry Reding wrote:
> On Mon, Feb 04, 2013 at 08:41:25PM -0800, Terje Bergström wrote:
>> This is used by debugfs code to direct to debugfs, and
>> nvhost_debug_dump() to send via printk.
> 
> Yes, that was precisely my point. Why bother providing the same data via
> several output methods. debugfs is good for showing large amounts of
> data such as register dumps or a tabular representation of syncpoints
> for instance.
> 
> If, however, you want to interactively show debug information using
> printk the same format isn't very useful and something more reduced is
> often better.

debugfs is there to be able to get a reliable dump of host1x state
(f.ex. no lines intermixed with other output).

printk output is there because often we get just UART logs from failure
cases, and having as much information as possible in the logs speeds up
debugging.

Both of them need to output the values of sync points, and the channel
state. Dumping all of that consists of a lot of code, and I wouldn't
want to duplicate that for two output formats.

>> I have another suggestion. In downstream I removed the decoding part and
>> I just print out a string of hex. That removes quite a bit bunch of code
>> from kernel. It makes the debug output also more compact.
> I don't know. I think if you use in-kernel debugging facilities such as
> debugfs or printk, then the output should be self-explanatory. However I
> do see the usefulness of having a binary dump that can be decoded in
> userspace. But I think if we want to go that way we should make that a
> separate interface. USB provides something like that, which can then be
> fed to libpcap or wireshark to capture and analyze USB traffic. If done
> properly you get replay functionality for free. I don't know what infra-
> structure exists to help with implementing something similar.

It's not actually binary. I think I misrepresented the suggestion.

I'm suggesting that we'd display only the contents of command FIFO and
contents of gathers (i.e. all opcodes) in hex format, not decoded. All
other text would remain as is, so syncpt values, etc would be readable
by a glance.

The user space tool can then take the streams and decode them if needed.

We've noticed that the decoded opcodes format can be very long and
sometimes takes a minute to dump out via a slow console. The hex output
is much more compact and faster to dump.

Actual tracing or wireshark kind of capability would come via decoding
the ftrace log. When enabled, everything that is written to the channel,
is also written to ftrace.

>>>> +static void show_channel_word(struct output *o, int *state, int *count,
>>>> +		u32 addr, u32 val, struct host1x_cdma *cdma)
>>>> +{
>>>> +	static int start_count, dont_print;
>>>
>>> What if two processes read debug information at the same time?
>>
>> show_channels() acquires cdma.lock, so that shouldn't happen.
> 
> Okay. Another solution would be to pass around a debug context which
> keeps track of the variables. But if we opt for a more involved dump
> interface as discussed above this will no longer be relevant.

Actually, debugging process needs cdma.lock, because it goes through the
cdma queue. Also command FIFO dumping is something that must be done by
a single thread at a time.

> Okay. In the context of a channel dump interface this may not be
> relevant anymore. Can you think of any issue that wouldn't be detectable
> or debuggable by analyzing a binary dump of the data within a channel?
> I'm asking because at that point we wouldn't be able to access any of
> the in-kernel data structures but would have to rely on the data itself
> for diagnostics. IOMMU virtual addresses won't be available and so on.

In many cases, looking at syncpt values, and channel state
(active/waiting on a syncpt, etc) gives an indication on what is the
current state of hardware. But, very often problems are ripple effects
on something that happened earlier and the job that caused the problem
has already been freed and is not visible in the dump.

To get a full history, we need often need the ftrace log.

Terje

  reply	other threads:[~2013-02-06 20:58 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-15 11:43 [PATCHv5,RESEND 0/8] Support for Tegra 2D hardware Terje Bergstrom
2013-01-15 11:43 ` [PATCHv5,RESEND 1/8] gpu: host1x: Add host1x driver Terje Bergstrom
2013-02-04  9:09   ` Thierry Reding
2013-02-05  3:30     ` Terje Bergström
2013-02-05  7:43       ` Thierry Reding
2013-02-06 20:13         ` Terje Bergström
2013-01-15 11:43 ` [PATCHv5,RESEND 2/8] gpu: host1x: Add syncpoint wait and interrupts Terje Bergstrom
2013-02-04 10:30   ` Thierry Reding
2013-02-05  4:29     ` Terje Bergström
2013-02-05  8:42       ` Thierry Reding
2013-02-06 20:29         ` Terje Bergström
2013-02-06 20:38           ` Thierry Reding
2013-02-06 20:41             ` Terje Bergström
2013-01-15 11:43 ` [PATCHv5,RESEND 3/8] gpu: host1x: Add channel support Terje Bergstrom
2013-02-25 15:24   ` Thierry Reding
2013-02-26  9:48     ` Terje Bergström
2013-02-27  8:56       ` Thierry Reding
2013-03-08 16:16       ` Terje Bergström
2013-03-08 20:43         ` Thierry Reding
2013-03-11  6:29           ` Terje Bergström
2013-03-11  7:18             ` Thierry Reding
2013-03-11  9:21               ` Terje Bergström
2013-03-11  9:41                 ` Thierry Reding
2013-01-15 11:44 ` [PATCHv5,RESEND 4/8] gpu: host1x: Add debug support Terje Bergstrom
2013-02-04 11:03   ` Thierry Reding
2013-02-05  4:41     ` Terje Bergström
2013-02-05  9:15       ` Thierry Reding
2013-02-06 20:58         ` Terje Bergström [this message]
2013-02-08  6:54           ` Thierry Reding
2013-01-15 11:44 ` [PATCHv5,RESEND 5/8] drm: tegra: Move drm to live under host1x Terje Bergstrom
2013-02-04 11:08   ` Thierry Reding
2013-02-05  4:45     ` Terje Bergström
2013-02-05  9:26       ` Thierry Reding
2013-01-15 11:44 ` [PATCHv5,RESEND 6/8] gpu: host1x: Remove second host1x driver Terje Bergstrom
2013-02-04 11:23   ` Thierry Reding
2013-01-15 11:44 ` [PATCHv5,RESEND 7/8] ARM: tegra: Add board data and 2D clocks Terje Bergstrom
2013-02-04 11:26   ` Thierry Reding
2013-02-04 17:06     ` Stephen Warren
2013-02-05  4:47     ` Terje Bergström
2013-01-15 11:44 ` [PATCHv5,RESEND 8/8] drm: tegra: Add gr2d device Terje Bergstrom
2013-02-04 12:56   ` Thierry Reding
2013-02-05  5:17     ` Terje Bergström
2013-02-05  9:54       ` Thierry Reding
2013-02-06 21:23         ` Terje Bergström
2013-02-08  7:07           ` Thierry Reding
2013-02-11  0:42             ` Terje Bergström
2013-02-11  6:44               ` Thierry Reding
2013-02-11 15:40                 ` Terje Bergström
2013-01-22  9:03 ` [PATCHv5,RESEND 0/8] Support for Tegra 2D hardware Terje Bergström

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5112C3EB.7020205@nvidia.com \
    --to=tbergstrom@nvidia.com \
    --cc=airlied@linux.ie \
    --cc=amerilainen@nvidia.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=thierry.reding@avionic-design.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).