Re: [Intel-gfx] [PATCH] drm/i915/gvt: remove duplicate entry of trace
Reviewed-by: Yan Zhao > -Original Message- > From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On > Behalf Of Zhenyu Wang > Sent: Wednesday, June 12, 2019 11:23 AM > To: Hariprasad Kelam > Cc: David Airlie ; intel-gfx@lists.freedesktop.org; Joonas > Lahtinen ; linux-ker...@vger.kernel.org; Jani > Nikula ; dri-de...@lists.freedesktop.org; Daniel > Vetter ; Vivi, Rodrigo ; intel-gvt- > d...@lists.freedesktop.org; Wang, Zhi A > Subject: Re: [PATCH] drm/i915/gvt: remove duplicate entry of trace > > On 2019.05.26 13:26:33 +0530, Hariprasad Kelam wrote: > > Remove duplicate include of trace.h > > > > Issue identified by includecheck > > > > Signed-off-by: Hariprasad Kelam > > --- > > drivers/gpu/drm/i915/gvt/trace_points.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/gvt/trace_points.c > > b/drivers/gpu/drm/i915/gvt/trace_points.c > > index a3deed69..569f5e3 100644 > > --- a/drivers/gpu/drm/i915/gvt/trace_points.c > > +++ b/drivers/gpu/drm/i915/gvt/trace_points.c > > @@ -32,5 +32,4 @@ > > > > #ifndef __CHECKER__ > > #define CREATE_TRACE_POINTS > > -#include "trace.h" > > #endif > > -- > > This actually caused build issue like > ERROR: "__tracepoint_gma_index" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_render_mmio" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_gvt_command" [drivers/gpu/drm/i915/i915.ko] > undefined! > ERROR: "__tracepoint_spt_guest_change" [drivers/gpu/drm/i915/i915.ko] > undefined! > ERROR: "__tracepoint_gma_translate" [drivers/gpu/drm/i915/i915.ko] > undefined! > ERROR: "__tracepoint_spt_alloc" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_spt_change" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_oos_sync" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_write_ir" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_propagate_event" [drivers/gpu/drm/i915/i915.ko] > undefined! > ERROR: "__tracepoint_inject_msi" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_spt_refcount" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_spt_free" [drivers/gpu/drm/i915/i915.ko] undefined! > ERROR: "__tracepoint_oos_change" [drivers/gpu/drm/i915/i915.ko] undefined! > scripts/Makefile.modpost:91: recipe for target '__modpost' failed > > Looks we need fix like below. > > Subject: [PATCH] drm/i915/gvt: remove duplicate include of trace.h > > This removes duplicate include of trace.h. Found by Hariprasad Kelam with > includecheck. > > Reported-by: Hariprasad Kelam > Signed-off-by: Zhenyu Wang > --- > drivers/gpu/drm/i915/gvt/trace_points.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/trace_points.c > b/drivers/gpu/drm/i915/gvt/trace_points.c > index a3deed692b9c..fe552e877e09 100644 > --- a/drivers/gpu/drm/i915/gvt/trace_points.c > +++ b/drivers/gpu/drm/i915/gvt/trace_points.c > @@ -28,8 +28,6 @@ > * > */ > > -#include "trace.h" > - > #ifndef __CHECKER__ > #define CREATE_TRACE_POINTS > #include "trace.h" > -- > 2.20.1 > > -- > Open Source Technology Center, Intel ltd. > > $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/gvt: give the cmd parser cmd_info a const treatment
Looks good to me.
Reviewed-by: Yan Zhao
> -Original Message-
> From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
> Behalf Of Jani Nikula
> Sent: Tuesday, January 8, 2019 10:12 PM
> To: intel-gvt-...@lists.freedesktop.org
> Cc: Nikula, Jani ; intel-gfx@lists.freedesktop.org;
> Wang,
> Zhi A ; zhen...@linux.intel.com
> Subject: [PATCH 2/2] drm/i915/gvt: give the cmd parser cmd_info a const
> treatment
>
> It doesn't need to be changed, make it const. The string literals should
> anyway
> be referred to as const data.
>
> The following gets moved to rodata section:
>
> 0080 l O .rodata 1c00 cmd_info
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/gvt/cmd_parser.c | 24
> drivers/gpu/drm/i915/gvt/trace.h | 2 +-
> 2 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> index 98415d465a09..cae00e6debaf 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> @@ -375,7 +375,7 @@ typedef int (*parser_cmd_handler)(struct
> parser_exec_state *s); #define ADDR_FIX_5(x1, x2, x3, x4, x5)
> (ADDR_FIX_1(x1)
> | ADDR_FIX_4(x2, x3, x4, x5))
>
> struct cmd_info {
> - char *name;
> + const char *name;
> u32 opcode;
>
> #define F_LEN_MASK (1U<<0)
> @@ -425,7 +425,7 @@ struct cmd_info {
>
> struct cmd_entry {
> struct hlist_node hlist;
> - struct cmd_info *info;
> + const struct cmd_info *info;
> };
>
> enum {
> @@ -474,7 +474,7 @@ struct parser_exec_state {
> int saved_buf_addr_type;
> bool is_ctx_wa;
>
> - struct cmd_info *info;
> + const struct cmd_info *info;
>
> struct intel_vgpu_workload *workload;
> };
> @@ -625,7 +625,7 @@ static inline u32 get_opcode(u32 cmd, int ring_id)
> return cmd >> (32 - d_info->op_len);
> }
>
> -static inline struct cmd_info *find_cmd_entry(struct intel_gvt *gvt,
> +static inline const struct cmd_info *find_cmd_entry(struct intel_gvt
> +*gvt,
> unsigned int opcode, int ring_id)
> {
> struct cmd_entry *e;
> @@ -638,7 +638,7 @@ static inline struct cmd_info *find_cmd_entry(struct
> intel_gvt *gvt,
> return NULL;
> }
>
> -static inline struct cmd_info *get_cmd_info(struct intel_gvt *gvt,
> +static inline const struct cmd_info *get_cmd_info(struct intel_gvt
> +*gvt,
> u32 cmd, int ring_id)
> {
> u32 opcode;
> @@ -776,7 +776,7 @@ static inline int ip_gma_advance(struct
> parser_exec_state *s,
> return 0;
> }
>
> -static inline int get_cmd_length(struct cmd_info *info, u32 cmd)
> +static inline int get_cmd_length(const struct cmd_info *info, u32 cmd)
> {
> if ((info->flag & F_LEN_MASK) == F_LEN_CONST)
> return info->len;
> @@ -1643,7 +1643,7 @@ static int batch_buffer_needs_scan(struct
> parser_exec_state *s) static int find_bb_size(struct parser_exec_state *s,
> unsigned long *bb_size) {
> unsigned long gma = 0;
> - struct cmd_info *info;
> + const struct cmd_info *info;
> uint32_t cmd_len = 0;
> bool bb_end = false;
> struct intel_vgpu *vgpu = s->vgpu;
> @@ -1842,7 +1842,7 @@ static int cmd_handler_mi_batch_buffer_start(struct
> parser_exec_state *s)
>
> static int mi_noop_index;
>
> -static struct cmd_info cmd_info[] = {
> +static const struct cmd_info cmd_info[] = {
> {"MI_NOOP", OP_MI_NOOP, F_LEN_CONST, R_ALL, D_ALL, 0, 1, NULL},
>
> {"MI_SET_PREDICATE", OP_MI_SET_PREDICATE, F_LEN_CONST, R_ALL,
> D_ALL, @@ -2521,7 +2521,7 @@ static void add_cmd_entry(struct intel_gvt
> *gvt, struct cmd_entry *e) static int cmd_parser_exec(struct
> parser_exec_state
> *s) {
> struct intel_vgpu *vgpu = s->vgpu;
> - struct cmd_info *info;
> + const struct cmd_info *info;
> u32 cmd;
> int ret = 0;
>
> @@ -2895,10 +2895,10 @@ int intel_gvt_scan_and_shadow_wa_ctx(struct
> intel_shadow_wa_ctx *wa_ctx)
> return 0;
> }
>
> -static struct cmd_info *find_cmd_entry_any_ring(struct intel_gvt *gvt,
> +static const struct cmd_info *find_cmd_entry_any_ring(struct intel_gvt
> +*gvt,
> unsigned int opcode, unsigned long rings) {
> - struct cmd_info *info = NULL;
> + const struct cmd_info *info = NULL;
> unsigned int ring;
>
> for_each_set_bit(ring, &rings, I915_NUM_ENGINES) { @@ -2913,7
> +2913,7 @@ static int init_cmd_table(struct intel_gvt *gvt) {
> int i;
> struct cmd_entry *e;
> - struct cmd_info *info;
> + const struct cmd_info *info;
> unsigned int gen_type;
>
> gen_type = intel_gvt_get_device_type(gvt); diff --git
> a/drivers/gpu/drm/i915/gvt/trace.h b/drivers/gpu/drm/i915/gvt/trace.h
> index 1fd64202d74e..6d787750d279 100644
> --- a/drivers/gpu/drm/i915/gvt/trace.h
> +++ b/drivers/gpu/drm/i915/gvt/trace.h
> @@ -228,7 +228,7 @@ TRACE_EVENT(oos_sync, TRACE_E
Re: [Intel-gfx] [PATCH 1/2] drm/i915/gvt: give the cmd parser decode_info a const treatment
Looks good to me.
Reviewed-by: Yan Zhao
> -Original Message-
> From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
> Behalf Of Jani Nikula
> Sent: Tuesday, January 8, 2019 10:12 PM
> To: intel-gvt-...@lists.freedesktop.org
> Cc: Nikula, Jani ; intel-gfx@lists.freedesktop.org;
> Wang,
> Zhi A ; zhen...@linux.intel.com
> Subject: [PATCH 1/2] drm/i915/gvt: give the cmd parser decode_info a const
> treatment
>
> It doesn't need to be changed, make it const. The string literals should
> anyway
> be referred to as const data.
>
> The following gets moved to rodata section:
>
> 0410 l O .rodata 0018 decode_info_mi
> 0390 l O .rodata 0018
> decode_info_3d_media
> 03e0 l O .rodata 0018 decode_info_2d
> 0330 l O .rodata 0018
> decode_info_mfx_vc
> 02e0 l O .rodata 0018
> decode_info_vebox
> 0300 l O .rodata 0028 sub_op_vebox
> 0360 l O .rodata 0028 sub_op_mfx_vc
> 03c0 l O .rodata 0020 sub_op_3d_media
> 0400 l O .rodata 0010 sub_op_2d
> 0430 l O .rodata 0010 sub_op_mi
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/gvt/cmd_parser.c | 30 +--
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> index 77ae634eb11c..98415d465a09 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> @@ -55,10 +55,10 @@ struct sub_op_bits {
> int low;
> };
> struct decode_info {
> - char *name;
> + const char *name;
> int op_len;
> int nr_sub_op;
> - struct sub_op_bits *sub_op;
> + const struct sub_op_bits *sub_op;
> };
>
> #define MAX_CMD_BUDGET 0x7fff
> @@ -485,12 +485,12 @@ struct parser_exec_state { static unsigned long
> bypass_scan_mask = 0;
>
> /* ring ALL, type = 0 */
> -static struct sub_op_bits sub_op_mi[] = {
> +static const struct sub_op_bits sub_op_mi[] = {
> {31, 29},
> {28, 23},
> };
>
> -static struct decode_info decode_info_mi = {
> +static const struct decode_info decode_info_mi = {
> "MI",
> OP_LEN_MI,
> ARRAY_SIZE(sub_op_mi),
> @@ -498,12 +498,12 @@ static struct decode_info decode_info_mi = { };
>
> /* ring RCS, command type 2 */
> -static struct sub_op_bits sub_op_2d[] = {
> +static const struct sub_op_bits sub_op_2d[] = {
> {31, 29},
> {28, 22},
> };
>
> -static struct decode_info decode_info_2d = {
> +static const struct decode_info decode_info_2d = {
> "2D",
> OP_LEN_2D,
> ARRAY_SIZE(sub_op_2d),
> @@ -511,14 +511,14 @@ static struct decode_info decode_info_2d = { };
>
> /* ring RCS, command type 3 */
> -static struct sub_op_bits sub_op_3d_media[] = {
> +static const struct sub_op_bits sub_op_3d_media[] = {
> {31, 29},
> {28, 27},
> {26, 24},
> {23, 16},
> };
>
> -static struct decode_info decode_info_3d_media = {
> +static const struct decode_info decode_info_3d_media = {
> "3D_Media",
> OP_LEN_3D_MEDIA,
> ARRAY_SIZE(sub_op_3d_media),
> @@ -526,7 +526,7 @@ static struct decode_info decode_info_3d_media = { };
>
> /* ring VCS, command type 3 */
> -static struct sub_op_bits sub_op_mfx_vc[] = {
> +static const struct sub_op_bits sub_op_mfx_vc[] = {
> {31, 29},
> {28, 27},
> {26, 24},
> @@ -534,7 +534,7 @@ static struct sub_op_bits sub_op_mfx_vc[] = {
> {20, 16},
> };
>
> -static struct decode_info decode_info_mfx_vc = {
> +static const struct decode_info decode_info_mfx_vc = {
> "MFX_VC",
> OP_LEN_MFX_VC,
> ARRAY_SIZE(sub_op_mfx_vc),
> @@ -542,7 +542,7 @@ static struct decode_info decode_info_mfx_vc = { };
>
> /* ring VECS, command type 3 */
> -static struct sub_op_bits sub_op_vebox[] = {
> +static const struct sub_op_bits sub_op_vebox[] = {
> {31, 29},
> {28, 27},
> {26, 24},
> @@ -550,14 +550,14 @@ static struct sub_op_bits sub_op_vebox[] = {
> {20, 16},
> };
>
> -static struct decode_info decode_info_vebox = {
> +static const struct decode_info decode_info_vebox = {
> "VEBOX",
> OP_LEN_VEBOX,
> ARRAY_SIZE(sub_op_vebox),
> sub_op_vebox,
> };
>
> -static struct decode_info *ring_decode_info[I915_NUM_ENGINES][8] = {
> +static const struct decode_info *ring_decode_info[I915_NUM_ENGINES][8]
> += {
> [RCS] = {
> &decode_info_mi,
> NULL,
> @@ -616,7 +616,7 @@ static struct decode_info
> *ring_decode_info[I915_NUM_ENGINES][8] = {
>
> static inline u32 get_opcode(u32 cmd, int ring_id) {
> - struct decode_info *d_info;
> + const struct decode_info *d_info;
