* [PATCH 0/2] A couple additional patches for atomisp
@ 2020-05-30 19:32 Mauro Carvalho Chehab
2020-05-30 19:32 ` [PATCH 1/2] staging: media: atomisp: fix stack overflow in init_pipe_defaults() Mauro Carvalho Chehab
2020-05-30 19:32 ` [PATCH 2/2] media: atomisp: get rid of sh_css_pipe.c Mauro Carvalho Chehab
0 siblings, 2 replies; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2020-05-30 19:32 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Jeremy Sowden, Arnd Bergmann,
linux-kernel, clang-built-linux, devel, Sakari Ailus,
Greg Kroah-Hartman
Patch 1 was sent by Arnd already, but, while it improved stack usage with
clang, it broke compilation with gcc ;-(
Each compiler seems to have different issues with different types of
struct initalizers, so we ended needing some discussions in order to find
something that would work for both.
So, the new version works fine with both compilers.
While here, I noticed a file that has nothing but comments. The second
patch gets rid of it.
Arnd Bergmann (1):
staging: media: atomisp: fix stack overflow in init_pipe_defaults()
Mauro Carvalho Chehab (1):
media: atomisp: get rid of sh_css_pipe.c
drivers/staging/media/atomisp/Makefile | 1 -
.../media/atomisp/pci/ia_css_frame_public.h | 6 ++---
.../staging/media/atomisp/pci/ia_css_pipe.h | 15 ++++-------
.../media/atomisp/pci/ia_css_pipe_public.h | 6 ++---
.../staging/media/atomisp/pci/ia_css_types.h | 6 ++---
.../sdis/common/ia_css_sdis_common_types.h | 9 +++----
.../runtime/binary/interface/ia_css_binary.h | 3 +--
.../pipeline/interface/ia_css_pipeline.h | 3 +--
.../pci/runtime/pipeline/src/pipeline.c | 13 ++++++---
drivers/staging/media/atomisp/pci/sh_css.c | 27 +++++++++++++------
.../staging/media/atomisp/pci/sh_css_pipe.c | 17 ------------
11 files changed, 46 insertions(+), 60 deletions(-)
delete mode 100644 drivers/staging/media/atomisp/pci/sh_css_pipe.c
--
2.26.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/2] staging: media: atomisp: fix stack overflow in init_pipe_defaults()
2020-05-30 19:32 [PATCH 0/2] A couple additional patches for atomisp Mauro Carvalho Chehab
@ 2020-05-30 19:32 ` Mauro Carvalho Chehab
2020-05-30 19:32 ` [PATCH 2/2] media: atomisp: get rid of sh_css_pipe.c Mauro Carvalho Chehab
1 sibling, 0 replies; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2020-05-30 19:32 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Arnd Bergmann, Mauro Carvalho Chehab, Sakari Ailus,
Greg Kroah-Hartman, Jeremy Sowden, devel, linux-kernel,
clang-built-linux
From: Arnd Bergmann <arnd@arndb.de>
When building with clang, multiple copies of the structures to be
initialized are passed around on the stack and copied locally, using an
insane amount of stack space:
drivers/staging/media/atomisp/pci/sh_css.c:2371:1: error: stack frame size of 26864 bytes in function 'create_pipe' [-Werror,-Wframe-larger-than=]
Use constantly-allocated variables plus an explicit memcpy()
to avoid that.
Fixes: 6dc9a2568f84 ("media: atomisp: convert default struct values to use compound-literals with designated initializers")
Co-authored-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
.../media/atomisp/pci/ia_css_frame_public.h | 6 ++---
.../staging/media/atomisp/pci/ia_css_pipe.h | 15 ++++-------
.../media/atomisp/pci/ia_css_pipe_public.h | 6 ++---
.../staging/media/atomisp/pci/ia_css_types.h | 6 ++---
.../sdis/common/ia_css_sdis_common_types.h | 9 +++----
.../runtime/binary/interface/ia_css_binary.h | 3 +--
.../pipeline/interface/ia_css_pipeline.h | 3 +--
.../pci/runtime/pipeline/src/pipeline.c | 13 ++++++---
drivers/staging/media/atomisp/pci/sh_css.c | 27 +++++++++++++------
9 files changed, 46 insertions(+), 42 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/ia_css_frame_public.h b/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
index d822aa235ffe..96c86f0dc81c 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
@@ -122,8 +122,7 @@ struct ia_css_frame_info {
struct ia_css_crop_info crop_info;
};
-#define IA_CSS_BINARY_DEFAULT_FRAME_INFO \
-(struct ia_css_frame_info) { \
+#define IA_CSS_BINARY_DEFAULT_FRAME_INFO { \
.format = IA_CSS_FRAME_FORMAT_NUM, \
.raw_bayer_order = IA_CSS_BAYER_ORDER_NUM, \
}
@@ -185,8 +184,7 @@ struct ia_css_frame {
info.format */
};
-#define DEFAULT_FRAME \
-(struct ia_css_frame) { \
+#define DEFAULT_FRAME { \
.info = IA_CSS_BINARY_DEFAULT_FRAME_INFO, \
.dynamic_queue_id = SH_CSS_INVALID_QUEUE_ID, \
.buf_type = IA_CSS_BUFFER_TYPE_INVALID, \
diff --git a/drivers/staging/media/atomisp/pci/ia_css_pipe.h b/drivers/staging/media/atomisp/pci/ia_css_pipe.h
index 9c0c4ccf4571..bb0abf9bffb1 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_pipe.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_pipe.h
@@ -40,8 +40,7 @@ struct ia_css_preview_settings {
struct ia_css_pipe *acc_pipe;
};
-#define IA_CSS_DEFAULT_PREVIEW_SETTINGS \
-(struct ia_css_preview_settings) { \
+#define IA_CSS_DEFAULT_PREVIEW_SETTINGS { \
.copy_binary = IA_CSS_BINARY_DEFAULT_SETTINGS, \
.preview_binary = IA_CSS_BINARY_DEFAULT_SETTINGS, \
.vf_pp_binary = IA_CSS_BINARY_DEFAULT_SETTINGS, \
@@ -65,8 +64,7 @@ struct ia_css_capture_settings {
unsigned int num_yuv_scaler;
};
-#define IA_CSS_DEFAULT_CAPTURE_SETTINGS \
-(struct ia_css_capture_settings) { \
+#define IA_CSS_DEFAULT_CAPTURE_SETTINGS { \
.copy_binary = IA_CSS_BINARY_DEFAULT_SETTINGS, \
.primary_binary = {IA_CSS_BINARY_DEFAULT_SETTINGS}, \
.pre_isp_binary = IA_CSS_BINARY_DEFAULT_SETTINGS, \
@@ -91,8 +89,7 @@ struct ia_css_video_settings {
unsigned int num_yuv_scaler;
};
-#define IA_CSS_DEFAULT_VIDEO_SETTINGS \
-(struct ia_css_video_settings) { \
+#define IA_CSS_DEFAULT_VIDEO_SETTINGS { \
.copy_binary = IA_CSS_BINARY_DEFAULT_SETTINGS, \
.video_binary = IA_CSS_BINARY_DEFAULT_SETTINGS, \
.vf_pp_binary = IA_CSS_BINARY_DEFAULT_SETTINGS, \
@@ -108,8 +105,7 @@ struct ia_css_yuvpp_settings {
unsigned int num_output;
};
-#define IA_CSS_DEFAULT_YUVPP_SETTINGS \
-(struct ia_css_yuvpp_settings) { \
+#define IA_CSS_DEFAULT_YUVPP_SETTINGS { \
.copy_binary = IA_CSS_BINARY_DEFAULT_SETTINGS, \
}
@@ -157,8 +153,7 @@ struct ia_css_pipe {
unsigned int pipe_num;
};
-#define IA_CSS_DEFAULT_PIPE \
-(struct ia_css_pipe) { \
+#define IA_CSS_DEFAULT_PIPE { \
.config = DEFAULT_PIPE_CONFIG, \
.info = DEFAULT_PIPE_INFO, \
.mode = IA_CSS_PIPE_ID_ACC, /* (pipe_id) */ \
diff --git a/drivers/staging/media/atomisp/pci/ia_css_pipe_public.h b/drivers/staging/media/atomisp/pci/ia_css_pipe_public.h
index 17b0941382bf..4affd21f9e3f 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_pipe_public.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_pipe_public.h
@@ -149,8 +149,7 @@ struct ia_css_pipe_config {
/**
* Default settings for newly created pipe configurations.
*/
-#define DEFAULT_PIPE_CONFIG \
-(struct ia_css_pipe_config) { \
+#define DEFAULT_PIPE_CONFIG { \
.mode = IA_CSS_PIPE_MODE_PREVIEW, \
.isp_pipe_version = 1, \
.output_info = {IA_CSS_BINARY_DEFAULT_FRAME_INFO}, \
@@ -203,8 +202,7 @@ struct ia_css_pipe_info {
/**
* Defaults for ia_css_pipe_info structs.
*/
-#define DEFAULT_PIPE_INFO \
-(struct ia_css_pipe_info) { \
+#define DEFAULT_PIPE_INFO {\
.output_info = {IA_CSS_BINARY_DEFAULT_FRAME_INFO}, \
.vf_output_info = {IA_CSS_BINARY_DEFAULT_FRAME_INFO}, \
.raw_output_info = IA_CSS_BINARY_DEFAULT_FRAME_INFO, \
diff --git a/drivers/staging/media/atomisp/pci/ia_css_types.h b/drivers/staging/media/atomisp/pci/ia_css_types.h
index bac0a41add92..6e34d401f9df 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_types.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_types.h
@@ -392,8 +392,7 @@ struct ia_css_grid_info {
};
/* defaults for ia_css_grid_info structs */
-#define DEFAULT_GRID_INFO \
-(struct ia_css_grid_info) { \
+#define DEFAULT_GRID_INFO { \
.dvs_grid = DEFAULT_DVS_GRID_INFO, \
.vamem_type = IA_CSS_VAMEM_TYPE_1 \
}
@@ -485,8 +484,7 @@ struct ia_css_capture_config {
};
/* default settings for ia_css_capture_config structs */
-#define DEFAULT_CAPTURE_CONFIG \
-(struct ia_css_capture_config) { \
+#define DEFAULT_CAPTURE_CONFIG { \
.mode = IA_CSS_CAPTURE_MODE_PRIMARY, \
}
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/sdis/common/ia_css_sdis_common_types.h b/drivers/staging/media/atomisp/pci/isp/kernels/sdis/common/ia_css_sdis_common_types.h
index c92f5836059b..c2ec30b4abd4 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/sdis/common/ia_css_sdis_common_types.h
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/sdis/common/ia_css_sdis_common_types.h
@@ -197,12 +197,11 @@ struct ia_css_dvs_stat_grid_info {
/* DVS statistics generated by accelerator default grid info
*/
-#define DEFAULT_DVS_GRID_INFO \
-(union ia_css_dvs_grid_u) { \
- .dvs_stat_grid_info = (struct ia_css_dvs_stat_grid_info) { \
+#define DEFAULT_DVS_GRID_INFO { \
+ .dvs_stat_grid_info = { \
.fe_roi_cfg = { \
- [1] = (struct dvs_stat_public_dvs_level_fe_roi_cfg) { \
- .x_start = 4 \
+ [1] = { \
+ .x_start = 4 \
} \
} \
} \
diff --git a/drivers/staging/media/atomisp/pci/runtime/binary/interface/ia_css_binary.h b/drivers/staging/media/atomisp/pci/runtime/binary/interface/ia_css_binary.h
index 560580023b29..b44099dbdacd 100644
--- a/drivers/staging/media/atomisp/pci/runtime/binary/interface/ia_css_binary.h
+++ b/drivers/staging/media/atomisp/pci/runtime/binary/interface/ia_css_binary.h
@@ -144,8 +144,7 @@ struct ia_css_binary {
struct ia_css_isp_param_css_segments css_params;
};
-#define IA_CSS_BINARY_DEFAULT_SETTINGS \
-(struct ia_css_binary) { \
+#define IA_CSS_BINARY_DEFAULT_SETTINGS { \
.input_format = ATOMISP_INPUT_FORMAT_YUV420_8_LEGACY, \
.in_frame_info = IA_CSS_BINARY_DEFAULT_FRAME_INFO, \
.internal_frame_info = IA_CSS_BINARY_DEFAULT_FRAME_INFO, \
diff --git a/drivers/staging/media/atomisp/pci/runtime/pipeline/interface/ia_css_pipeline.h b/drivers/staging/media/atomisp/pci/runtime/pipeline/interface/ia_css_pipeline.h
index 1f4afd163ffa..18a7d18e197e 100644
--- a/drivers/staging/media/atomisp/pci/runtime/pipeline/interface/ia_css_pipeline.h
+++ b/drivers/staging/media/atomisp/pci/runtime/pipeline/interface/ia_css_pipeline.h
@@ -57,8 +57,7 @@ struct ia_css_pipeline {
u32 pipe_qos_config;
};
-#define DEFAULT_PIPELINE \
-(struct ia_css_pipeline) { \
+#define DEFAULT_PIPELINE { \
.pipe_id = IA_CSS_PIPE_ID_PREVIEW, \
.in_frame = DEFAULT_FRAME, \
.out_frame = {DEFAULT_FRAME}, \
diff --git a/drivers/staging/media/atomisp/pci/runtime/pipeline/src/pipeline.c b/drivers/staging/media/atomisp/pci/runtime/pipeline/src/pipeline.c
index 29207fe120a0..4b8e85bc2122 100644
--- a/drivers/staging/media/atomisp/pci/runtime/pipeline/src/pipeline.c
+++ b/drivers/staging/media/atomisp/pci/runtime/pipeline/src/pipeline.c
@@ -665,6 +665,8 @@ static int pipeline_stage_create(
return err;
}
+static const struct ia_css_frame ia_css_default_frame = DEFAULT_FRAME;
+
static void pipeline_init_defaults(
struct ia_css_pipeline *pipeline,
enum ia_css_pipe_id pipe_id,
@@ -677,10 +679,15 @@ static void pipeline_init_defaults(
pipeline->stages = NULL;
pipeline->stop_requested = false;
pipeline->current_stage = NULL;
- pipeline->in_frame = DEFAULT_FRAME;
+
+ memcpy(&pipeline->in_frame, &ia_css_default_frame,
+ sizeof(ia_css_default_frame));
+
for (i = 0; i < IA_CSS_PIPE_MAX_OUTPUT_STAGE; i++) {
- pipeline->out_frame[i] = DEFAULT_FRAME;
- pipeline->vf_frame[i] = DEFAULT_FRAME;
+ memcpy(&pipeline->out_frame[i], &ia_css_default_frame,
+ sizeof(ia_css_default_frame));
+ memcpy(&pipeline->vf_frame[i], &ia_css_default_frame,
+ sizeof(ia_css_default_frame));
}
pipeline->num_execs = -1;
pipeline->acquire_isp_each_stage = true;
diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 31a13a486b70..9c754e29fa00 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -2231,10 +2231,17 @@ create_host_pipeline(struct ia_css_stream *stream) {
return err;
}
+static const struct ia_css_pipe default_pipe = IA_CSS_DEFAULT_PIPE;
+static const struct ia_css_preview_settings preview = IA_CSS_DEFAULT_PREVIEW_SETTINGS;
+static const struct ia_css_capture_settings capture = IA_CSS_DEFAULT_CAPTURE_SETTINGS;
+static const struct ia_css_video_settings video = IA_CSS_DEFAULT_VIDEO_SETTINGS;
+static const struct ia_css_yuvpp_settings yuvpp = IA_CSS_DEFAULT_YUVPP_SETTINGS;
+
static int
init_pipe_defaults(enum ia_css_pipe_mode mode,
struct ia_css_pipe *pipe,
bool copy_pipe) {
+
if (!pipe)
{
IA_CSS_ERROR("NULL pipe parameter");
@@ -2242,14 +2249,14 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
}
/* Initialize pipe to pre-defined defaults */
- *pipe = IA_CSS_DEFAULT_PIPE;
+ memcpy(pipe, &default_pipe, sizeof(default_pipe));
/* TODO: JB should not be needed, but temporary backward reference */
switch (mode)
{
case IA_CSS_PIPE_MODE_PREVIEW:
pipe->mode = IA_CSS_PIPE_ID_PREVIEW;
- pipe->pipe_settings.preview = IA_CSS_DEFAULT_PREVIEW_SETTINGS;
+ memcpy(&pipe->pipe_settings.preview, &preview, sizeof(preview));
break;
case IA_CSS_PIPE_MODE_CAPTURE:
if (copy_pipe) {
@@ -2257,11 +2264,11 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
} else {
pipe->mode = IA_CSS_PIPE_ID_CAPTURE;
}
- pipe->pipe_settings.capture = IA_CSS_DEFAULT_CAPTURE_SETTINGS;
+ memcpy(&pipe->pipe_settings.capture, &capture, sizeof(capture));
break;
case IA_CSS_PIPE_MODE_VIDEO:
pipe->mode = IA_CSS_PIPE_ID_VIDEO;
- pipe->pipe_settings.video = IA_CSS_DEFAULT_VIDEO_SETTINGS;
+ memcpy(&pipe->pipe_settings.video, &video, sizeof(video));
break;
case IA_CSS_PIPE_MODE_ACC:
pipe->mode = IA_CSS_PIPE_ID_ACC;
@@ -2271,7 +2278,7 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
break;
case IA_CSS_PIPE_MODE_YUVPP:
pipe->mode = IA_CSS_PIPE_ID_YUVPP;
- pipe->pipe_settings.yuvpp = IA_CSS_DEFAULT_YUVPP_SETTINGS;
+ memcpy(&pipe->pipe_settings.yuvpp, &yuvpp, sizeof(yuvpp));
break;
default:
return -EINVAL;
@@ -8769,12 +8776,16 @@ sh_css_init_host_sp_control_vars(void) {
}
/*
- * create the internal structures and fill in the configuration data
- */
+ * create the internal structures and fill in the configuration data
+ */
+
+static const struct
+ia_css_pipe_config ia_css_pipe_default_config = DEFAULT_PIPE_CONFIG;
+
void ia_css_pipe_config_defaults(struct ia_css_pipe_config *pipe_config)
{
ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "ia_css_pipe_config_defaults()\n");
- *pipe_config = DEFAULT_PIPE_CONFIG;
+ memcpy(pipe_config, &ia_css_pipe_default_config, sizeof(*pipe_config));
}
void
--
2.26.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 2/2] media: atomisp: get rid of sh_css_pipe.c
2020-05-30 19:32 [PATCH 0/2] A couple additional patches for atomisp Mauro Carvalho Chehab
2020-05-30 19:32 ` [PATCH 1/2] staging: media: atomisp: fix stack overflow in init_pipe_defaults() Mauro Carvalho Chehab
@ 2020-05-30 19:32 ` Mauro Carvalho Chehab
1 sibling, 0 replies; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2020-05-30 19:32 UTC (permalink / raw)
To: Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
linux-kernel, devel
There's nothing there, just comments.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/media/atomisp/Makefile | 1 -
drivers/staging/media/atomisp/pci/sh_css_pipe.c | 17 -----------------
2 files changed, 18 deletions(-)
delete mode 100644 drivers/staging/media/atomisp/pci/sh_css_pipe.c
diff --git a/drivers/staging/media/atomisp/Makefile b/drivers/staging/media/atomisp/Makefile
index 0b62020f9b8b..9dc8072799e3 100644
--- a/drivers/staging/media/atomisp/Makefile
+++ b/drivers/staging/media/atomisp/Makefile
@@ -37,7 +37,6 @@ atomisp-objs += \
pci/sh_css_param_dvs.o \
pci/sh_css_param_shading.o \
pci/sh_css_params.o \
- pci/sh_css_pipe.o \
pci/sh_css_properties.o \
pci/sh_css_shading.o \
pci/sh_css_sp.o \
diff --git a/drivers/staging/media/atomisp/pci/sh_css_pipe.c b/drivers/staging/media/atomisp/pci/sh_css_pipe.c
deleted file mode 100644
index b6bd47d9b01c..000000000000
--- a/drivers/staging/media/atomisp/pci/sh_css_pipe.c
+++ /dev/null
@@ -1,17 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Support for Intel Camera Imaging ISP subsystem.
- * Copyright (c) 2015, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- */
-
-/* This file will contain the code to implement the functions declared in ia_css_pipe.h and ia_css_pipe_public.h
- and associated helper functions */
--
2.26.2
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-30 19:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-30 19:32 [PATCH 0/2] A couple additional patches for atomisp Mauro Carvalho Chehab
2020-05-30 19:32 ` [PATCH 1/2] staging: media: atomisp: fix stack overflow in init_pipe_defaults() Mauro Carvalho Chehab
2020-05-30 19:32 ` [PATCH 2/2] media: atomisp: get rid of sh_css_pipe.c Mauro Carvalho Chehab
Unnamed repository; edit this file 'description' to name the repository.
This inbox may be cloned and mirrored by anyone:
git clone --mirror http://archive.lwn.net:8080/linux-media/0 linux-media/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 linux-media linux-media/ http://archive.lwn.net:8080/linux-media \
linux-media@vger.kernel.org lwn-linux-media@archive.lwn.net
public-inbox-index linux-media
Example config snippet for mirrors.
Newsgroup available over NNTP:
nntp://archive.lwn.net/lwn.kernel.linux-media
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git