Re: [RFC 3/8] drm/i915: Add the data structures for async OA capture mode

From: Daniel Vetter <daniel@ffwll.ch>
To: sourab.gupta@intel.com
Cc: intel-gfx@lists.freedesktop.org, Insoo Woo <insoo.woo@intel.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Jabin Wu <jabin.wu@intel.com>
Subject: Re: [RFC 3/8] drm/i915: Add the data structures for async OA capture mode
Date: Mon, 22 Jun 2015 18:01:40 +0200	[thread overview]
Message-ID: <20150622160140.GE25769@phenom.ffwll.local> (raw)
In-Reply-To: <1434966619-3979-4-git-send-email-sourab.gupta@intel.com>

On Mon, Jun 22, 2015 at 03:20:14PM +0530, sourab.gupta@intel.com wrote:
> From: Sourab Gupta <sourab.gupta@intel.com>
> 
> This patch introduces the data structures for capturing asynchronous OA
> snapshots
> 
> The data captured will be organized into nodes. Each node has the field for OA
> report alongwith metadata information such as ctx_id, pid, etc. The metadata
> information can be extended to provided any additional information.
> The data is organized to have a queue header at beginning, which will have
> information about size, data offset, number of nodes captured etc.
> 
> Signed-off-by: Sourab Gupta <sourab.gupta@intel.com>

Please don't add data structures without code, it essentially makes this
patch here unreviable without looking at other patches. Which just
increases the review burden for no gain.

If you create a big new structure instead only add the new fields you're
using in each patch, and by doing so slowly build up the entire thing.

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h | 21 +++++++++++++++++++++
>  include/uapi/drm/i915_drm.h     |  5 +++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ee4a5d3..da150bc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1677,6 +1677,27 @@ extern const struct i915_oa_reg i915_oa_sampler_balance_mux_config_hsw[];
>  extern const int i915_oa_sampler_balance_mux_config_hsw_len;
>  extern const struct i915_oa_reg i915_oa_sampler_balance_b_counter_config_hsw[];
>  extern const int i915_oa_sampler_balance_b_counter_config_hsw_len;
> +
> +
> +struct drm_i915_oa_async_queue_header {
> +	__u64 size_in_bytes;
> +	/* Byte offset, start of queue header to first node */
> +	__u64 data_offset;
> +	__u32 node_count;
> +	__u32 wrap_count;
> +	__u32 pad[10];
> +};
> +
> +struct drm_i915_oa_async_node_info {
> +	__u32 pid;
> +	__u32 ctx_id;
> +	__u32 pad[14];
> +};
> +
> +struct drm_i915_oa_async_node {
> +	struct drm_i915_oa_async_node_info node_info;
> +	__u32 report_perf[64]; /* Must be aligned to 64-byte boundary */
> +};
>  #endif
>  
>  struct drm_i915_private {
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 354dc3a..c91b427 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -124,6 +124,11 @@ enum drm_i915_oa_event_type {
>  	I915_OA_RECORD_MAX,			/* non-ABI */
>  };
>  
> +struct drm_i915_oa_async_node_footer {
> +	__u32 pid;
> +	__u32 ctx_id;
> +};
> +
>  /* Each region is a minimum of 16k, and there are at most 255 of them.
>   */
>  #define I915_NR_TEX_REGIONS 255	/* table size 2k - maximum due to use
> -- 
> 1.8.5.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx