* [PATCH 1/3] media: atomisp: csi2-bridge: Add DMI quirk for t4ka3 on Xiaomi Mipad2
@ 2024-09-02 9:52 Hans de Goede
2024-09-02 9:52 ` [PATCH 2/3] media: atomisp: Drop dev_dbg() calls from hmm_[alloc|free]() Hans de Goede
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Hans de Goede @ 2024-09-02 9:52 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus
Cc: Hans de Goede, Tsuchiya Yuto, Andy Shevchenko, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
linux-staging
The t4ka3 sensor on the Xiaomi Mipad2 is used as a back facing sensor,
it uses 4 CSI lanes, but the _DSM has CsiLanes set to 2. Extend
the existing Xiaomi Mipad2 DMI quirk to override the wrong _DSM setting.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c b/drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c
index 31c9e05a1435..b2a3243ae2d4 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c
@@ -109,6 +109,8 @@ static struct gmin_cfg_var lenovo_ideapad_miix_310_vars[] = {
static struct gmin_cfg_var xiaomi_mipad2_vars[] = {
/* _DSM contains the wrong CsiPort for the front facing OV5693 sensor */
{ "INT33BE:00", "CsiPort", "0" },
+ /* _DSM contains the wrong CsiLanes for the back facing T4KA3 sensor */
+ { "XMCC0003:00", "CsiLanes", "4" },
{}
};
--
2.46.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] media: atomisp: Drop dev_dbg() calls from hmm_[alloc|free]()
2024-09-02 9:52 [PATCH 1/3] media: atomisp: csi2-bridge: Add DMI quirk for t4ka3 on Xiaomi Mipad2 Hans de Goede
@ 2024-09-02 9:52 ` Hans de Goede
2024-09-02 11:06 ` Andy Shevchenko
2024-09-02 9:52 ` [PATCH 3/3] media: atomisp: Improve binary finding debug logging Hans de Goede
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Hans de Goede @ 2024-09-02 9:52 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus
Cc: Hans de Goede, Tsuchiya Yuto, Andy Shevchenko, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
linux-staging
Debug logging every alloc + free just polutes the debug logs without
adding much value, remove the alloc + free dev_dbg() calls.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/staging/media/atomisp/pci/hmm/hmm.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm.c b/drivers/staging/media/atomisp/pci/hmm/hmm.c
index 3e2899ad8517..e8c5d728fd55 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm.c
@@ -204,9 +204,6 @@ static ia_css_ptr __hmm_alloc(size_t bytes, enum hmm_bo_type type,
goto bind_err;
}
- dev_dbg(atomisp_dev, "pages: 0x%08x (%zu bytes), type: %d, vmalloc %p\n",
- bo->start, bytes, type, vmalloc_noprof);
-
return bo->start;
bind_err:
@@ -231,8 +228,6 @@ void hmm_free(ia_css_ptr virt)
{
struct hmm_buffer_object *bo;
- dev_dbg(atomisp_dev, "%s: free 0x%08x\n", __func__, virt);
-
if (WARN_ON(virt == mmgr_EXCEPTION))
return;
--
2.46.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] media: atomisp: Improve binary finding debug logging
2024-09-02 9:52 [PATCH 1/3] media: atomisp: csi2-bridge: Add DMI quirk for t4ka3 on Xiaomi Mipad2 Hans de Goede
2024-09-02 9:52 ` [PATCH 2/3] media: atomisp: Drop dev_dbg() calls from hmm_[alloc|free]() Hans de Goede
@ 2024-09-02 9:52 ` Hans de Goede
2024-09-02 12:03 ` Andy Shevchenko
2024-09-02 10:37 ` [PATCH 1/3] media: atomisp: csi2-bridge: Add DMI quirk for t4ka3 on Xiaomi Mipad2 Hans de Goede
2024-09-02 11:06 ` Andy Shevchenko
3 siblings, 1 reply; 10+ messages in thread
From: Hans de Goede @ 2024-09-02 9:52 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus
Cc: Hans de Goede, Tsuchiya Yuto, Andy Shevchenko, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
linux-staging
The atomisp firmware contains a number of different pipeline binaries
inside its firmware file and the driver selects the right one depending
on the selected pipeline configuration.
Sometimes (e.g. when the selected output resolution is too big) it fails
to find a binary. This happens especially when adding support for new
sensors.
Improve the logging when this happens to make debugging easier:
1. Replace ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, ...) with standard
dev_dbg() calls so that the logs can be enabled with dyndbg
2. Do not dump_stack() when this fails, doing so adds no useful extra
info
3. With the dump_stack() call gone, remove the wrapper and rename
__ia_css_binary_find() to ia_css_binary_find()
4. On error use dev_err() instead of dev_dbg() so that when things
fail it is clear why they fail without needing to enable dyndbg
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note the log messages could also do with a bit of rewording and
reflowing them to put more arguments on a single line to use less
lines. I consider that out of scope for this patch which already
enough (see the 1-4 enough) something like this should be done
in a follow-up patch IMHO.
---
.../atomisp/pci/runtime/binary/src/binary.c | 295 +++++++++---------
1 file changed, 139 insertions(+), 156 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
index b0f904a5e442..1c72c2b5817f 100644
--- a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
+++ b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
@@ -923,8 +923,9 @@ ia_css_binary_fill_info(const struct ia_css_binary_xinfo *xinfo,
return 0;
}
-static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
- struct ia_css_binary *binary) {
+int ia_css_binary_find(struct ia_css_binary_descr *descr,
+ struct ia_css_binary *binary)
+{
int mode;
bool online;
bool two_ppc;
@@ -953,10 +954,10 @@ static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
/* MW: used after an error check, may accept NULL, but doubtfull */
assert(binary);
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() enter: descr=%p, (mode=%d), binary=%p\n",
- descr, descr->mode,
- binary);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() enter: descr=%p, (mode=%d), binary=%p\n",
+ descr, descr->mode,
+ binary);
mode = descr->mode;
online = descr->online;
@@ -1001,15 +1002,15 @@ static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
}
/* print a map of the binary file */
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "BINARY INFO:\n");
+ dev_dbg(atomisp_dev, "BINARY INFO:\n");
for (i = 0; i < IA_CSS_BINARY_NUM_MODES; i++) {
xcandidate = binary_infos[i];
if (xcandidate) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "%d:\n", i);
+ dev_dbg(atomisp_dev, "%d:\n", i);
while (xcandidate) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, " Name:%s Type:%d Cont:%d\n",
- xcandidate->blob->name, xcandidate->type,
- xcandidate->sp.enable.continuous);
+ dev_dbg(atomisp_dev, " Name:%s Type:%d Cont:%d\n",
+ xcandidate->blob->name, xcandidate->type,
+ xcandidate->sp.enable.continuous);
xcandidate = xcandidate->next;
}
}
@@ -1021,9 +1022,9 @@ static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
struct ia_css_binary_info *candidate = &xcandidate->sp;
/* printf("sh_css_binary_find: evaluating candidate:
* %d\n",candidate->id); */
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() candidate = %p, mode = %d ID = %d\n",
- candidate, candidate->pipeline.mode, candidate->id);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() candidate = %p, mode = %d ID = %d\n",
+ candidate, candidate->pipeline.mode, candidate->id);
/*
* MW: Only a limited set of jointly configured binaries can
@@ -1032,17 +1033,17 @@ static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
*/
if (!candidate->enable.continuous &&
continuous && (mode != IA_CSS_BINARY_MODE_COPY)) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: !%d && %d && (%d != %d)\n",
- __LINE__, candidate->enable.continuous,
- continuous, mode,
- IA_CSS_BINARY_MODE_COPY);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: !%d && %d && (%d != %d)\n",
+ __LINE__, candidate->enable.continuous,
+ continuous, mode,
+ IA_CSS_BINARY_MODE_COPY);
continue;
}
if (striped && candidate->iterator.num_stripes == 1) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: binary is not striped\n",
- __LINE__);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: binary is not striped\n",
+ __LINE__);
continue;
}
@@ -1050,58 +1051,58 @@ static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
(mode != IA_CSS_BINARY_MODE_COPY) &&
(mode != IA_CSS_BINARY_MODE_CAPTURE_PP) &&
(mode != IA_CSS_BINARY_MODE_VF_PP)) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: (%d != %d)\n",
- __LINE__,
- candidate->pipeline.isp_pipe_version, isp_pipe_version);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: (%d != %d)\n",
+ __LINE__,
+ candidate->pipeline.isp_pipe_version, isp_pipe_version);
continue;
}
if (!candidate->enable.reduced_pipe && enable_reduced_pipe) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: !%d && %d\n",
- __LINE__,
- candidate->enable.reduced_pipe,
- enable_reduced_pipe);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: !%d && %d\n",
+ __LINE__,
+ candidate->enable.reduced_pipe,
+ enable_reduced_pipe);
continue;
}
if (!candidate->enable.dvs_6axis && enable_dvs_6axis) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: !%d && %d\n",
- __LINE__,
- candidate->enable.dvs_6axis,
- enable_dvs_6axis);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: !%d && %d\n",
+ __LINE__,
+ candidate->enable.dvs_6axis,
+ enable_dvs_6axis);
continue;
}
if (candidate->enable.high_speed && !enable_high_speed) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: %d && !%d\n",
- __LINE__,
- candidate->enable.high_speed,
- enable_high_speed);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: %d && !%d\n",
+ __LINE__,
+ candidate->enable.high_speed,
+ enable_high_speed);
continue;
}
if (!candidate->enable.xnr && need_xnr) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: %d && !%d\n",
- __LINE__,
- candidate->enable.xnr,
- need_xnr);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: %d && !%d\n",
+ __LINE__,
+ candidate->enable.xnr,
+ need_xnr);
continue;
}
if (!(candidate->enable.ds & 2) && enable_yuv_ds) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: !%d && %d\n",
- __LINE__,
- ((candidate->enable.ds & 2) != 0),
- enable_yuv_ds);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: !%d && %d\n",
+ __LINE__,
+ ((candidate->enable.ds & 2) != 0),
+ enable_yuv_ds);
continue;
}
if ((candidate->enable.ds & 2) && !enable_yuv_ds) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: %d && !%d\n",
- __LINE__,
- ((candidate->enable.ds & 2) != 0),
- enable_yuv_ds);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: %d && !%d\n",
+ __LINE__,
+ ((candidate->enable.ds & 2) != 0),
+ enable_yuv_ds);
continue;
}
@@ -1115,100 +1116,100 @@ static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
candidate->vf_dec.is_variable ||
/* or more than one output pin. */
xcandidate->num_output_pins > 1)) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: (%p != NULL) && !(%d || %d || (%d >%d))\n",
- __LINE__, req_vf_info,
- candidate->enable.vf_veceven,
- candidate->vf_dec.is_variable,
- xcandidate->num_output_pins, 1);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: (%p != NULL) && !(%d || %d || (%d >%d))\n",
+ __LINE__, req_vf_info,
+ candidate->enable.vf_veceven,
+ candidate->vf_dec.is_variable,
+ xcandidate->num_output_pins, 1);
continue;
}
if (!candidate->enable.dvs_envelope && need_dvs) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: !%d && %d\n",
- __LINE__,
- candidate->enable.dvs_envelope, (int)need_dvs);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: !%d && %d\n",
+ __LINE__,
+ candidate->enable.dvs_envelope, (int)need_dvs);
continue;
}
/* internal_res check considers input, output, and dvs envelope sizes */
ia_css_binary_internal_res(req_in_info, req_bds_out_info,
req_bin_out_info, &dvs_env, candidate, &internal_res);
if (internal_res.width > candidate->internal.max_width) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: (%d > %d)\n",
- __LINE__, internal_res.width,
- candidate->internal.max_width);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: (%d > %d)\n",
+ __LINE__, internal_res.width,
+ candidate->internal.max_width);
continue;
}
if (internal_res.height > candidate->internal.max_height) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: (%d > %d)\n",
- __LINE__, internal_res.height,
- candidate->internal.max_height);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: (%d > %d)\n",
+ __LINE__, internal_res.height,
+ candidate->internal.max_height);
continue;
}
if (!candidate->enable.ds && need_ds && !(xcandidate->num_output_pins > 1)) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: !%d && %d\n",
- __LINE__, candidate->enable.ds, (int)need_ds);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: !%d && %d\n",
+ __LINE__, candidate->enable.ds, (int)need_ds);
continue;
}
if (!candidate->enable.uds && !candidate->enable.dvs_6axis && need_dz) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: !%d && !%d && %d\n",
- __LINE__, candidate->enable.uds,
- candidate->enable.dvs_6axis, (int)need_dz);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: !%d && !%d && %d\n",
+ __LINE__, candidate->enable.uds,
+ candidate->enable.dvs_6axis, (int)need_dz);
continue;
}
if (online && candidate->input.source == IA_CSS_BINARY_INPUT_MEMORY) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: %d && (%d == %d)\n",
- __LINE__, online, candidate->input.source,
- IA_CSS_BINARY_INPUT_MEMORY);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: %d && (%d == %d)\n",
+ __LINE__, online, candidate->input.source,
+ IA_CSS_BINARY_INPUT_MEMORY);
continue;
}
if (!online && candidate->input.source == IA_CSS_BINARY_INPUT_SENSOR) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: !%d && (%d == %d)\n",
- __LINE__, online, candidate->input.source,
- IA_CSS_BINARY_INPUT_SENSOR);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: !%d && (%d == %d)\n",
+ __LINE__, online, candidate->input.source,
+ IA_CSS_BINARY_INPUT_SENSOR);
continue;
}
if (req_bin_out_info->res.width < candidate->output.min_width ||
req_bin_out_info->res.width > candidate->output.max_width) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: (%d > %d) || (%d < %d)\n",
- __LINE__,
- req_bin_out_info->padded_width,
- candidate->output.min_width,
- req_bin_out_info->padded_width,
- candidate->output.max_width);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: (%d > %d) || (%d < %d)\n",
+ __LINE__,
+ req_bin_out_info->padded_width,
+ candidate->output.min_width,
+ req_bin_out_info->padded_width,
+ candidate->output.max_width);
continue;
}
if (xcandidate->num_output_pins > 1 &&
/* in case we have a second output pin, */
req_vf_info) { /* and we need vf output. */
if (req_vf_info->res.width > candidate->output.max_width) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: (%d < %d)\n",
- __LINE__,
- req_vf_info->res.width,
- candidate->output.max_width);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: (%d < %d)\n",
+ __LINE__,
+ req_vf_info->res.width,
+ candidate->output.max_width);
continue;
}
}
if (req_in_info->padded_width > candidate->input.max_width) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: (%d > %d)\n",
- __LINE__, req_in_info->padded_width,
- candidate->input.max_width);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: (%d > %d)\n",
+ __LINE__, req_in_info->padded_width,
+ candidate->input.max_width);
continue;
}
if (!binary_supports_output_format(xcandidate, req_bin_out_info->format)) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: !%d\n",
- __LINE__,
- binary_supports_output_format(xcandidate, req_bin_out_info->format));
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: !%d\n",
+ __LINE__,
+ binary_supports_output_format(xcandidate, req_bin_out_info->format));
continue;
}
if (xcandidate->num_output_pins > 1 &&
@@ -1217,11 +1218,11 @@ static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
/* check if the required vf format
is supported. */
!binary_supports_output_format(xcandidate, req_vf_info->format)) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: (%d > %d) && (%p != NULL) && !%d\n",
- __LINE__, xcandidate->num_output_pins, 1,
- req_vf_info,
- binary_supports_output_format(xcandidate, req_vf_info->format));
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: (%d > %d) && (%p != NULL) && !%d\n",
+ __LINE__, xcandidate->num_output_pins, 1,
+ req_vf_info,
+ binary_supports_output_format(xcandidate, req_vf_info->format));
continue;
}
@@ -1229,11 +1230,11 @@ static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
if (xcandidate->num_output_pins == 1 &&
req_vf_info && candidate->enable.vf_veceven &&
!binary_supports_vf_format(xcandidate, req_vf_info->format)) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: (%d == %d) && (%p != NULL) && %d && !%d\n",
- __LINE__, xcandidate->num_output_pins, 1,
- req_vf_info, candidate->enable.vf_veceven,
- binary_supports_vf_format(xcandidate, req_vf_info->format));
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: (%d == %d) && (%p != NULL) && %d && !%d\n",
+ __LINE__, xcandidate->num_output_pins, 1,
+ req_vf_info, candidate->enable.vf_veceven,
+ binary_supports_vf_format(xcandidate, req_vf_info->format));
continue;
}
@@ -1241,37 +1242,37 @@ static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
if (xcandidate->num_output_pins == 1 &&
req_vf_info && candidate->enable.vf_veceven) { /* and we need vf output. */
if (req_vf_info->res.width > candidate->output.max_width) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: (%d < %d)\n",
- __LINE__,
- req_vf_info->res.width,
- candidate->output.max_width);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: (%d < %d)\n",
+ __LINE__,
+ req_vf_info->res.width,
+ candidate->output.max_width);
continue;
}
}
if (!supports_bds_factor(candidate->bds.supported_bds_factors,
descr->required_bds_factor)) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: 0x%x & 0x%x)\n",
- __LINE__, candidate->bds.supported_bds_factors,
- descr->required_bds_factor);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: 0x%x & 0x%x)\n",
+ __LINE__, candidate->bds.supported_bds_factors,
+ descr->required_bds_factor);
continue;
}
if (!candidate->enable.dpc && need_dpc) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: 0x%x & 0x%x)\n",
- __LINE__, candidate->enable.dpc,
- descr->enable_dpc);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: 0x%x & 0x%x)\n",
+ __LINE__, candidate->enable.dpc,
+ descr->enable_dpc);
continue;
}
if (candidate->uds.use_bci && enable_capture_pp_bli) {
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() [%d] continue: 0x%x & 0x%x)\n",
- __LINE__, candidate->uds.use_bci,
- descr->enable_capture_pp_bli);
+ dev_dbg(atomisp_dev,
+ "ia_css_binary_find() [%d] continue: 0x%x & 0x%x)\n",
+ __LINE__, candidate->uds.use_bci,
+ descr->enable_capture_pp_bli);
continue;
}
@@ -1290,13 +1291,6 @@ static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
break;
}
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() selected = %p, mode = %d ID = %d\n",
- xcandidate, xcandidate ? xcandidate->sp.pipeline.mode : 0, xcandidate ? xcandidate->sp.id : 0);
-
- ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
- "ia_css_binary_find() leave: return_err=%d\n", err);
-
if (!err && xcandidate)
dev_dbg(atomisp_dev,
"Using binary %s (id %d), type %d, mode %d, continuous %s\n",
@@ -1306,23 +1300,12 @@ static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
xcandidate->sp.pipeline.mode,
xcandidate->sp.enable.continuous ? "true" : "false");
+ if (err)
+ dev_err(atomisp_dev, "Failed to find a firmware binary matching the pipeline parameters\n");
return err;
}
-int ia_css_binary_find(struct ia_css_binary_descr *descr,
- struct ia_css_binary *binary)
-{
- int ret = __ia_css_binary_find(descr, binary);
-
- if (unlikely(ret)) {
- dev_dbg(atomisp_dev, "Seeking for binary failed at:");
- dump_stack();
- }
-
- return ret;
-}
-
unsigned
ia_css_binary_max_vf_width(void)
{
--
2.46.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] media: atomisp: csi2-bridge: Add DMI quirk for t4ka3 on Xiaomi Mipad2
2024-09-02 9:52 [PATCH 1/3] media: atomisp: csi2-bridge: Add DMI quirk for t4ka3 on Xiaomi Mipad2 Hans de Goede
2024-09-02 9:52 ` [PATCH 2/3] media: atomisp: Drop dev_dbg() calls from hmm_[alloc|free]() Hans de Goede
2024-09-02 9:52 ` [PATCH 3/3] media: atomisp: Improve binary finding debug logging Hans de Goede
@ 2024-09-02 10:37 ` Hans de Goede
2024-09-02 11:06 ` Andy Shevchenko
3 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2024-09-02 10:37 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus
Cc: Tsuchiya Yuto, Andy Shevchenko, Yury Luneff, Nable,
andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
linux-staging
Hi,
On 9/2/24 11:52 AM, Hans de Goede wrote:
> The t4ka3 sensor on the Xiaomi Mipad2 is used as a back facing sensor,
> it uses 4 CSI lanes, but the _DSM has CsiLanes set to 2. Extend
> the existing Xiaomi Mipad2 DMI quirk to override the wrong _DSM setting.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
I have merged these 3 patches in my media-atomisp branch:
https://git.kernel.org/pub/scm/linux/kernel/git/hansg/linux.git/log/?h=media-atomisp
now.
And this/these will be included in my next pull-request to
Mauro (to media subsystem maintainer)
Regards,
Hans
> ---
> drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c b/drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c
> index 31c9e05a1435..b2a3243ae2d4 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_csi2_bridge.c
> @@ -109,6 +109,8 @@ static struct gmin_cfg_var lenovo_ideapad_miix_310_vars[] = {
> static struct gmin_cfg_var xiaomi_mipad2_vars[] = {
> /* _DSM contains the wrong CsiPort for the front facing OV5693 sensor */
> { "INT33BE:00", "CsiPort", "0" },
> + /* _DSM contains the wrong CsiLanes for the back facing T4KA3 sensor */
> + { "XMCC0003:00", "CsiLanes", "4" },
> {}
> };
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] media: atomisp: csi2-bridge: Add DMI quirk for t4ka3 on Xiaomi Mipad2
2024-09-02 9:52 [PATCH 1/3] media: atomisp: csi2-bridge: Add DMI quirk for t4ka3 on Xiaomi Mipad2 Hans de Goede
` (2 preceding siblings ...)
2024-09-02 10:37 ` [PATCH 1/3] media: atomisp: csi2-bridge: Add DMI quirk for t4ka3 on Xiaomi Mipad2 Hans de Goede
@ 2024-09-02 11:06 ` Andy Shevchenko
3 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2024-09-02 11:06 UTC (permalink / raw)
To: Hans de Goede
Cc: Mauro Carvalho Chehab, Sakari Ailus, Tsuchiya Yuto, Yury Luneff,
Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
linux-staging
On Mon, Sep 02, 2024 at 11:52:27AM +0200, Hans de Goede wrote:
> The t4ka3 sensor on the Xiaomi Mipad2 is used as a back facing sensor,
> it uses 4 CSI lanes, but the _DSM has CsiLanes set to 2. Extend
> the existing Xiaomi Mipad2 DMI quirk to override the wrong _DSM setting.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] media: atomisp: Drop dev_dbg() calls from hmm_[alloc|free]()
2024-09-02 9:52 ` [PATCH 2/3] media: atomisp: Drop dev_dbg() calls from hmm_[alloc|free]() Hans de Goede
@ 2024-09-02 11:06 ` Andy Shevchenko
0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2024-09-02 11:06 UTC (permalink / raw)
To: Hans de Goede
Cc: Mauro Carvalho Chehab, Sakari Ailus, Tsuchiya Yuto, Yury Luneff,
Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
linux-staging
On Mon, Sep 02, 2024 at 11:52:28AM +0200, Hans de Goede wrote:
> Debug logging every alloc + free just polutes the debug logs without
> adding much value, remove the alloc + free dev_dbg() calls.
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Agree on the point and ideally we should remove/replace most of (all?) the
messages by trace points.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] media: atomisp: Improve binary finding debug logging
2024-09-02 9:52 ` [PATCH 3/3] media: atomisp: Improve binary finding debug logging Hans de Goede
@ 2024-09-02 12:03 ` Andy Shevchenko
2024-09-03 9:27 ` Hans de Goede
0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2024-09-02 12:03 UTC (permalink / raw)
To: Hans de Goede
Cc: Mauro Carvalho Chehab, Sakari Ailus, Tsuchiya Yuto, Yury Luneff,
Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
linux-staging
On Mon, Sep 02, 2024 at 11:52:29AM +0200, Hans de Goede wrote:
> The atomisp firmware contains a number of different pipeline binaries
> inside its firmware file and the driver selects the right one depending
> on the selected pipeline configuration.
>
> Sometimes (e.g. when the selected output resolution is too big) it fails
> to find a binary. This happens especially when adding support for new
> sensors.
>
> Improve the logging when this happens to make debugging easier:
>
> 1. Replace ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, ...) with standard
> dev_dbg() calls so that the logs can be enabled with dyndbg
>
> 2. Do not dump_stack() when this fails, doing so adds no useful extra
> info
>
> 3. With the dump_stack() call gone, remove the wrapper and rename
> __ia_css_binary_find() to ia_css_binary_find()
>
> 4. On error use dev_err() instead of dev_dbg() so that when things
> fail it is clear why they fail without needing to enable dyndbg
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Note the log messages could also do with a bit of rewording and
> reflowing them to put more arguments on a single line to use less
> lines. I consider that out of scope for this patch which already
> enough (see the 1-4 enough) something like this should be done
> in a follow-up patch IMHO.
Yes, but...
...
> -static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
> - struct ia_css_binary *binary) {
> +int ia_css_binary_find(struct ia_css_binary_descr *descr,
> + struct ia_css_binary *binary)
...for example, in this case you touched both lines, so making them a single
line just reduces the future churn.
...
> - ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
> - "ia_css_binary_find() enter: descr=%p, (mode=%d), binary=%p\n",
> - descr, descr->mode,
> - binary);
> + dev_dbg(atomisp_dev,
> + "ia_css_binary_find() enter: descr=%p, (mode=%d), binary=%p\n",
> + descr, descr->mode,
> + binary);
So does this.
...
> /* print a map of the binary file */
> - ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "BINARY INFO:\n");
> + dev_dbg(atomisp_dev, "BINARY INFO:\n");
TAB instead of space.
...
> - ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
> - "ia_css_binary_find() [%d] continue: !%d && %d && (%d != %d)\n",
> - __LINE__, candidate->enable.continuous,
> - continuous, mode,
> - IA_CSS_BINARY_MODE_COPY);
> + dev_dbg(atomisp_dev,
> + "ia_css_binary_find() [%d] continue: !%d && %d && (%d != %d)\n",
> + __LINE__, candidate->enable.continuous,
> + continuous, mode,
> + IA_CSS_BINARY_MODE_COPY);
Now one line?
...
> - ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
> - "ia_css_binary_find() [%d] continue: binary is not striped\n",
> - __LINE__);
> + dev_dbg(atomisp_dev,
> + "ia_css_binary_find() [%d] continue: binary is not striped\n",
> + __LINE__);
Now the __LINE__ argument is redundant as one may run-time turn it on and off.
So, trimming it while converting to dev_dbg() makes sense to me as a one
logical change. Ditto for other __LINE__ cases.
...
Otherwise it's a good clean up!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] media: atomisp: Improve binary finding debug logging
2024-09-02 12:03 ` Andy Shevchenko
@ 2024-09-03 9:27 ` Hans de Goede
2024-09-03 10:30 ` Andy Shevchenko
0 siblings, 1 reply; 10+ messages in thread
From: Hans de Goede @ 2024-09-03 9:27 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mauro Carvalho Chehab, Sakari Ailus, Tsuchiya Yuto, Yury Luneff,
Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
linux-staging
Hi Andy,
Thank you for the reviews.
On 9/2/24 2:03 PM, Andy Shevchenko wrote:
> On Mon, Sep 02, 2024 at 11:52:29AM +0200, Hans de Goede wrote:
>> The atomisp firmware contains a number of different pipeline binaries
>> inside its firmware file and the driver selects the right one depending
>> on the selected pipeline configuration.
>>
>> Sometimes (e.g. when the selected output resolution is too big) it fails
>> to find a binary. This happens especially when adding support for new
>> sensors.
>>
>> Improve the logging when this happens to make debugging easier:
>>
>> 1. Replace ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, ...) with standard
>> dev_dbg() calls so that the logs can be enabled with dyndbg
>>
>> 2. Do not dump_stack() when this fails, doing so adds no useful extra
>> info
>>
>> 3. With the dump_stack() call gone, remove the wrapper and rename
>> __ia_css_binary_find() to ia_css_binary_find()
>>
>> 4. On error use dev_err() instead of dev_dbg() so that when things
>> fail it is clear why they fail without needing to enable dyndbg
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Note the log messages could also do with a bit of rewording and
>> reflowing them to put more arguments on a single line to use less
>> lines. I consider that out of scope for this patch which already
>> enough (see the 1-4 enough) something like this should be done
>> in a follow-up patch IMHO.
>
> Yes, but...
>
> ...
>
>> -static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
>> - struct ia_css_binary *binary) {
>> +int ia_css_binary_find(struct ia_css_binary_descr *descr,
>> + struct ia_css_binary *binary)
>
> ...for example, in this case you touched both lines, so making them a single
> line just reduces the future churn.
Ack, fixed in my media-atomisp branch.
>
> ...
>
>> - ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
>> - "ia_css_binary_find() enter: descr=%p, (mode=%d), binary=%p\n",
>> - descr, descr->mode,
>> - binary);
>> + dev_dbg(atomisp_dev,
>> + "ia_css_binary_find() enter: descr=%p, (mode=%d), binary=%p\n",
>
>> + descr, descr->mode,
>> + binary);
>
> So does this.
Ack, fixed in my media-atomisp branch.
>
> ...
>
>> /* print a map of the binary file */
>> - ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "BINARY INFO:\n");
>> + dev_dbg(atomisp_dev, "BINARY INFO:\n");
>
> TAB instead of space.
Ack, fixed in my media-atomisp branch.
> ...
>
>> - ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
>> - "ia_css_binary_find() [%d] continue: !%d && %d && (%d != %d)\n",
>> - __LINE__, candidate->enable.continuous,
>> - continuous, mode,
>> - IA_CSS_BINARY_MODE_COPY);
>> + dev_dbg(atomisp_dev,
>> + "ia_css_binary_find() [%d] continue: !%d && %d && (%d != %d)\n",
>> + __LINE__, candidate->enable.continuous,
>
>> + continuous, mode,
>> + IA_CSS_BINARY_MODE_COPY);
>
> Now one line?
Ack, fixed in my media-atomisp branch.
> ...
>
>> - ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
>> - "ia_css_binary_find() [%d] continue: binary is not striped\n",
>> - __LINE__);
>> + dev_dbg(atomisp_dev,
>> + "ia_css_binary_find() [%d] continue: binary is not striped\n",
>> + __LINE__);
>
> Now the __LINE__ argument is redundant as one may run-time turn it on and off.
> So, trimming it while converting to dev_dbg() makes sense to me as a one
> logical change. Ditto for other __LINE__ cases.
Dropping __LINE__ is the bit which I want to postpone to a follow-up patch,
the messages rejecting certain binaries as not suitable are not very
unique and the __LINE__ print is necessary (atm) to help find which check
exactly is failing. Not ideal I know, something for the TODO list.
> Otherwise it's a good clean up!
Thanks.
Regards,
Hans
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] media: atomisp: Improve binary finding debug logging
2024-09-03 9:27 ` Hans de Goede
@ 2024-09-03 10:30 ` Andy Shevchenko
2024-09-03 11:12 ` Hans de Goede
0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2024-09-03 10:30 UTC (permalink / raw)
To: Hans de Goede
Cc: Andy Shevchenko, Mauro Carvalho Chehab, Sakari Ailus,
Tsuchiya Yuto, Yury Luneff, Nable, andrey.i.trufanov, Fabio Aiuto,
Kate Hsuan, linux-media, linux-staging
On Tue, Sep 3, 2024 at 12:27 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 9/2/24 2:03 PM, Andy Shevchenko wrote:
> > On Mon, Sep 02, 2024 at 11:52:29AM +0200, Hans de Goede wrote:
...
> >> -static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
> >> - struct ia_css_binary *binary) {
> >> +int ia_css_binary_find(struct ia_css_binary_descr *descr,
> >> + struct ia_css_binary *binary)
> >
> > ...for example, in this case you touched both lines, so making them a single
> > line just reduces the future churn.
>
> Ack, fixed in my media-atomisp branch.
(Actually there are more opportunities like this that I haven't
commented on in the previous reply. I hope you have already found them
and fixed accordingly)
...
> >> + dev_dbg(atomisp_dev,
> >> + "ia_css_binary_find() [%d] continue: binary is not striped\n",
> >> + __LINE__);
> >
> > Now the __LINE__ argument is redundant as one may run-time turn it on and off.
> > So, trimming it while converting to dev_dbg() makes sense to me as a one
> > logical change. Ditto for other __LINE__ cases.
>
> Dropping __LINE__ is the bit which I want to postpone to a follow-up patch,
> the messages rejecting certain binaries as not suitable are not very
> unique and the __LINE__ print is necessary (atm) to help find which check
> exactly is failing. Not ideal I know, something for the TODO list.
Yes, it's fine, after I sent the previous reply I saw that the
previous implementation was a simple wrapper over dev_dbg() and
__LINE__ "issue" was already there. I.o.w. I agree on the followup!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] media: atomisp: Improve binary finding debug logging
2024-09-03 10:30 ` Andy Shevchenko
@ 2024-09-03 11:12 ` Hans de Goede
0 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2024-09-03 11:12 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, Mauro Carvalho Chehab, Sakari Ailus,
Tsuchiya Yuto, Yury Luneff, Nable, andrey.i.trufanov, Fabio Aiuto,
Kate Hsuan, linux-media, linux-staging
Hi,
On 9/3/24 12:30 PM, Andy Shevchenko wrote:
> On Tue, Sep 3, 2024 at 12:27 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 9/2/24 2:03 PM, Andy Shevchenko wrote:
>>> On Mon, Sep 02, 2024 at 11:52:29AM +0200, Hans de Goede wrote:
>
> ...
>
>>>> -static int __ia_css_binary_find(struct ia_css_binary_descr *descr,
>>>> - struct ia_css_binary *binary) {
>>>> +int ia_css_binary_find(struct ia_css_binary_descr *descr,
>>>> + struct ia_css_binary *binary)
>>>
>>> ...for example, in this case you touched both lines, so making them a single
>>> line just reduces the future churn.
>>
>> Ack, fixed in my media-atomisp branch.
>
> (Actually there are more opportunities like this that I haven't
> commented on in the previous reply. I hope you have already found them
> and fixed accordingly)
Yes I've gone over the entire patch and made lines longer to avoid
needing multiple lines where possible.
Regards,
Hans
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-09-03 11:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-02 9:52 [PATCH 1/3] media: atomisp: csi2-bridge: Add DMI quirk for t4ka3 on Xiaomi Mipad2 Hans de Goede
2024-09-02 9:52 ` [PATCH 2/3] media: atomisp: Drop dev_dbg() calls from hmm_[alloc|free]() Hans de Goede
2024-09-02 11:06 ` Andy Shevchenko
2024-09-02 9:52 ` [PATCH 3/3] media: atomisp: Improve binary finding debug logging Hans de Goede
2024-09-02 12:03 ` Andy Shevchenko
2024-09-03 9:27 ` Hans de Goede
2024-09-03 10:30 ` Andy Shevchenko
2024-09-03 11:12 ` Hans de Goede
2024-09-02 10:37 ` [PATCH 1/3] media: atomisp: csi2-bridge: Add DMI quirk for t4ka3 on Xiaomi Mipad2 Hans de Goede
2024-09-02 11:06 ` Andy Shevchenko
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).