From mboxrd@z Thu Jan  1 00:00:00 1970
From: Chris Wilson <chris@chris-wilson.co.uk>
Subject: Re: [PATCH 2/6] intel_gpu_top: suport command line
	parameters and variable samples per second
Date: Mon, 05 Sep 2011 22:44:27 +0100
Message-ID: <e0d58a$1b5ndg@orsmga002.jf.intel.com>
References: <1315253973-18950-1-git-send-email-eugeni@dodonov.net>
	<1315253973-18950-3-git-send-email-eugeni@dodonov.net>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
	by gabe.freedesktop.org (Postfix) with ESMTP id 1187AA0A22
	for <intel-gfx@lists.freedesktop.org>;
	Mon,  5 Sep 2011 14:44:34 -0700 (PDT)
In-Reply-To: <1315253973-18950-3-git-send-email-eugeni@dodonov.net>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
To: Eugeni Dodonov <eugeni@dodonov.net>, intel-gfx@lists.freedesktop.org
Cc: Eugeni Dodonov <eugeni.dodonov@intel.com>
List-Id: intel-gfx@lists.freedesktop.org

On Mon,  5 Sep 2011 17:19:29 -0300, Eugeni Dodonov <eugeni@dodonov.net> wrote:
> From: Eugeni Dodonov <eugeni.dodonov@intel.com>
> 
> This patch adds support for getopt, and adds two default parameters to it:
> -h to show usage notes; and -s to allow user to define number of samples
> to acquire per second.

Just a minor style issue, otherwise it looks good. All I need is
someway to correlate GPU activity with batches (and especially the
contents of those batches) and with even higher level code and then I'd
be happy. Oh, and integrated with a timeline of CPU activity, of course.
:-)
 
> +	/* Parse options? */
> +	while ((ch = getopt(argc, argv, "s:h")) != -1)
> +	{
> +		switch (ch)
> +		{
> +			case 's': samples_per_sec = atoi(optarg);

In the modules we own, we have adopted the kernel CODING_STYLE as our
standard. 8 space indents, 80 cols line (except where readibility is
improved by going over), braces on the same line as the control flow,
/*
 * This style of long comments.
 */
and case statements should being at the same indentation as the switch
and so should the parameters of a multiline function...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre