* [PATCH 1/2] media: atomisp: better describe get_frame_info issues
@ 2021-10-29 8:49 Mauro Carvalho Chehab
2021-10-29 8:49 ` [PATCH 2/2] media: atomisp: set default mode Mauro Carvalho Chehab
2021-10-29 9:01 ` [PATCH 1/2] media: atomisp: better describe get_frame_info issues Andy Shevchenko
0 siblings, 2 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2021-10-29 8:49 UTC (permalink / raw)
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Aditya Srivastava,
Alex Dewar, Aline Santana Cordeiro, Andy Shevchenko,
Arnd Bergmann, Baokun Li, Greg Kroah-Hartman, Hans Verkuil,
Kaixu Xia, Mauro Carvalho Chehab, Sakari Ailus, Tomi Valkeinen,
Tsuchiya Yuto, Yang Li, linux-kernel, linux-media, linux-staging
When atomisp is used by a normal client, it fails to get
frame info. However, the information is confusing and misleading,
as there are several wrappers for such function, and the error
could be on different places.
So, improve the error log in order to allow narrowing down
where the error is actually occuring.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
.../staging/media/atomisp/pci/atomisp_cmd.c | 4 +-
.../media/atomisp/pci/atomisp_compat_css20.c | 67 ++++++++++---------
2 files changed, 39 insertions(+), 32 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index a30dfcce54dd..32cae6908465 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -5444,9 +5444,9 @@ static int atomisp_set_fmt_to_isp(struct video_device *vdev,
else
ret = get_frame_info(asd, output_info);
if (ret) {
- dev_err(isp->dev, "get_frame_info %ux%u (padded to %u) returned %d\n",
+ dev_err(isp->dev, "__get_frame_info %ux%u (padded to %u) returned %d\n",
pix->width, pix->height, pix->bytesperline, ret);
- return -EINVAL;
+ return ret;
}
atomisp_update_grid_info(asd, pipe_id, source_pad);
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index 99a632f33d2d..1309855bb6c8 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -2657,41 +2657,48 @@ static int __get_frame_info(struct atomisp_sub_device *asd,
if (__destroy_pipes(asd, true))
dev_warn(isp->dev, "destroy pipe failed.\n");
- if (__create_pipes(asd))
+ if (__create_pipes(asd)) {
+ dev_err(isp->dev, "can't create pipes\n");
return -EINVAL;
+ }
+
+ if (__create_streams(asd)) {
+ dev_err(isp->dev, "can't create streams\n");
+ goto stream_err;
+ }
- if (__create_streams(asd))
+ ret = ia_css_pipe_get_info(asd->stream_env[stream_index].pipes[pipe_id],
+ &p_info);
+ if (ret) {
+ dev_err(isp->dev, "can't get info from pipe\n");
goto stream_err;
+ }
- ret = ia_css_pipe_get_info(
- asd->stream_env[stream_index]
- .pipes[pipe_id], &p_info);
- if (!ret) {
- switch (type) {
- case ATOMISP_CSS_VF_FRAME:
- *info = p_info.vf_output_info[0];
- dev_dbg(isp->dev, "getting vf frame info.\n");
- break;
- case ATOMISP_CSS_SECOND_VF_FRAME:
- *info = p_info.vf_output_info[1];
- dev_dbg(isp->dev, "getting second vf frame info.\n");
- break;
- case ATOMISP_CSS_OUTPUT_FRAME:
- *info = p_info.output_info[0];
- dev_dbg(isp->dev, "getting main frame info.\n");
- break;
- case ATOMISP_CSS_SECOND_OUTPUT_FRAME:
- *info = p_info.output_info[1];
- dev_dbg(isp->dev, "getting second main frame info.\n");
- break;
- case ATOMISP_CSS_RAW_FRAME:
- *info = p_info.raw_output_info;
- dev_dbg(isp->dev, "getting raw frame info.\n");
- }
- dev_dbg(isp->dev, "get frame info: w=%d, h=%d, num_invalid_frames %d.\n",
- info->res.width, info->res.height, p_info.num_invalid_frames);
- return 0;
+ switch (type) {
+ case ATOMISP_CSS_VF_FRAME:
+ *info = p_info.vf_output_info[0];
+ dev_dbg(isp->dev, "getting vf frame info.\n");
+ break;
+ case ATOMISP_CSS_SECOND_VF_FRAME:
+ *info = p_info.vf_output_info[1];
+ dev_dbg(isp->dev, "getting second vf frame info.\n");
+ break;
+ case ATOMISP_CSS_OUTPUT_FRAME:
+ *info = p_info.output_info[0];
+ dev_dbg(isp->dev, "getting main frame info.\n");
+ break;
+ case ATOMISP_CSS_SECOND_OUTPUT_FRAME:
+ *info = p_info.output_info[1];
+ dev_dbg(isp->dev, "getting second main frame info.\n");
+ break;
+ case ATOMISP_CSS_RAW_FRAME:
+ *info = p_info.raw_output_info;
+ dev_dbg(isp->dev, "getting raw frame info.\n");
}
+ dev_dbg(isp->dev, "get frame info: w=%d, h=%d, num_invalid_frames %d.\n",
+ info->res.width, info->res.height, p_info.num_invalid_frames);
+
+ return 0;
stream_err:
__destroy_pipes(asd, true);
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] media: atomisp: set default mode
2021-10-29 8:49 [PATCH 1/2] media: atomisp: better describe get_frame_info issues Mauro Carvalho Chehab
@ 2021-10-29 8:49 ` Mauro Carvalho Chehab
2021-10-29 19:27 ` Mauro Carvalho Chehab
2021-10-29 9:01 ` [PATCH 1/2] media: atomisp: better describe get_frame_info issues Andy Shevchenko
1 sibling, 1 reply; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2021-10-29 8:49 UTC (permalink / raw)
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Alan,
Arnd Bergmann, Greg Kroah-Hartman, Hans Verkuil,
Laurent Pinchart, Mauro Carvalho Chehab, Sakari Ailus,
Tomi Valkeinen, Tsuchiya Yuto, linux-kernel, linux-media,
linux-staging
Without setting a default mode at open(), applications that
don't call VIDIOC_SET_PARM with a custom atomisp parameters
won't work, as the pipeline won't be set:
atomisp-isp2 0000:00:03.0: can't create streams
atomisp-isp2 0000:00:03.0: __get_frame_info 1600x1200 (padded to 0) returned -22
So, as an step to allow generic apps to use this driver, put
the device's run_mode in preview after open.
After this patch, using v4l2grab starts to work:
$ v4l2grab -D -f 'NV12' -x 1600 -y 1200 -d /dev/video2 -u
$ nvt/raw2pnm -x1600 -y1200 -f NV12 out017.raw out017.pnm
$ feh out017.pnm
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/media/atomisp/pci/atomisp_fops.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index 72cbdce2142a..7df982c80b1a 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -893,6 +893,11 @@ static int atomisp_open(struct file *file)
else
pipe->users++;
rt_mutex_unlock(&isp->mutex);
+
+ /* Ensure that a mode is set */
+ if (asd)
+ v4l2_ctrl_s_ctrl(asd->run_mode, ATOMISP_RUN_MODE_PREVIEW);
+
return 0;
css_error:
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] media: atomisp: set default mode
2021-10-29 8:49 ` [PATCH 2/2] media: atomisp: set default mode Mauro Carvalho Chehab
@ 2021-10-29 19:27 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2021-10-29 19:27 UTC (permalink / raw)
Cc: linuxarm, mauro.chehab, Alan, Arnd Bergmann, Greg Kroah-Hartman,
Hans Verkuil, Laurent Pinchart, Mauro Carvalho Chehab,
Sakari Ailus, Tomi Valkeinen, Tsuchiya Yuto, linux-kernel,
linux-media, linux-staging
Em Fri, 29 Oct 2021 09:49:36 +0100
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> Without setting a default mode at open(), applications that
> don't call VIDIOC_SET_PARM with a custom atomisp parameters
> won't work, as the pipeline won't be set:
>
> atomisp-isp2 0000:00:03.0: can't create streams
> atomisp-isp2 0000:00:03.0: __get_frame_info 1600x1200 (padded to 0) returned -22
>
> So, as an step to allow generic apps to use this driver, put
> the device's run_mode in preview after open.
>
> After this patch, using v4l2grab starts to work:
>
> $ v4l2grab -D -f 'NV12' -x 1600 -y 1200 -d /dev/video2 -u
> $ nvt/raw2pnm -x1600 -y1200 -f NV12 out017.raw out017.pnm
> $ feh out017.pnm
Added support for YUYV at v4l2grab (at contrib/test, under v4l-utils).
The above can now be just:
$ v4l2grab -f YUYV -x 1600 -y 1200 -d /dev/video2 -u
$ feh out017.pnm
(on a side note, the colorspace conversion from YUYV seems ackward,
but this can be adjusted later on at the toolset and/or at atomisp)
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
> drivers/staging/media/atomisp/pci/atomisp_fops.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
> index 72cbdce2142a..7df982c80b1a 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
> @@ -893,6 +893,11 @@ static int atomisp_open(struct file *file)
> else
> pipe->users++;
> rt_mutex_unlock(&isp->mutex);
> +
> + /* Ensure that a mode is set */
> + if (asd)
> + v4l2_ctrl_s_ctrl(asd->run_mode, ATOMISP_RUN_MODE_PREVIEW);
> +
> return 0;
>
> css_error:
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] media: atomisp: better describe get_frame_info issues
2021-10-29 8:49 [PATCH 1/2] media: atomisp: better describe get_frame_info issues Mauro Carvalho Chehab
2021-10-29 8:49 ` [PATCH 2/2] media: atomisp: set default mode Mauro Carvalho Chehab
@ 2021-10-29 9:01 ` Andy Shevchenko
2021-11-02 6:54 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2021-10-29 9:01 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linuxarm, mauro.chehab, Aditya Srivastava, Alex Dewar,
Aline Santana Cordeiro, Andy Shevchenko, Arnd Bergmann,
Baokun Li, Greg Kroah-Hartman, Hans Verkuil, Kaixu Xia,
Mauro Carvalho Chehab, Sakari Ailus, Tomi Valkeinen,
Tsuchiya Yuto, Yang Li, Linux Kernel Mailing List,
Linux Media Mailing List, linux-staging
On Fri, Oct 29, 2021 at 11:50 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> When atomisp is used by a normal client, it fails to get
> frame info. However, the information is confusing and misleading,
> as there are several wrappers for such function, and the error
> could be on different places.
>
> So, improve the error log in order to allow narrowing down
> where the error is actually occuring.
...
> + switch (type) {
> + case ATOMISP_CSS_VF_FRAME:
> + *info = p_info.vf_output_info[0];
> + dev_dbg(isp->dev, "getting vf frame info.\n");
> + break;
> + case ATOMISP_CSS_SECOND_VF_FRAME:
> + *info = p_info.vf_output_info[1];
> + dev_dbg(isp->dev, "getting second vf frame info.\n");
> + break;
> + case ATOMISP_CSS_OUTPUT_FRAME:
> + *info = p_info.output_info[0];
> + dev_dbg(isp->dev, "getting main frame info.\n");
> + break;
> + case ATOMISP_CSS_SECOND_OUTPUT_FRAME:
> + *info = p_info.output_info[1];
> + dev_dbg(isp->dev, "getting second main frame info.\n");
> + break;
> + case ATOMISP_CSS_RAW_FRAME:
> + *info = p_info.raw_output_info;
> + dev_dbg(isp->dev, "getting raw frame info.\n");
Can we get break; here followed by default case?
> }
> + dev_dbg(isp->dev, "get frame info: w=%d, h=%d, num_invalid_frames %d.\n",
> + info->res.width, info->res.height, p_info.num_invalid_frames);
> +
> + return 0;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] media: atomisp: better describe get_frame_info issues
2021-10-29 9:01 ` [PATCH 1/2] media: atomisp: better describe get_frame_info issues Andy Shevchenko
@ 2021-11-02 6:54 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2021-11-02 6:54 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linuxarm, mauro.chehab, Aditya Srivastava, Alex Dewar,
Aline Santana Cordeiro, Andy Shevchenko, Arnd Bergmann,
Baokun Li, Greg Kroah-Hartman, Hans Verkuil, Kaixu Xia,
Mauro Carvalho Chehab, Sakari Ailus, Tomi Valkeinen,
Tsuchiya Yuto, Yang Li, Linux Kernel Mailing List,
Linux Media Mailing List, linux-staging
Em Fri, 29 Oct 2021 12:01:57 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> escreveu:
> > + switch (type) {
> > + case ATOMISP_CSS_VF_FRAME:
> > + *info = p_info.vf_output_info[0];
> > + dev_dbg(isp->dev, "getting vf frame info.\n");
> > + break;
> > + case ATOMISP_CSS_SECOND_VF_FRAME:
> > + *info = p_info.vf_output_info[1];
> > + dev_dbg(isp->dev, "getting second vf frame info.\n");
> > + break;
> > + case ATOMISP_CSS_OUTPUT_FRAME:
> > + *info = p_info.output_info[0];
> > + dev_dbg(isp->dev, "getting main frame info.\n");
> > + break;
> > + case ATOMISP_CSS_SECOND_OUTPUT_FRAME:
> > + *info = p_info.output_info[1];
> > + dev_dbg(isp->dev, "getting second main frame info.\n");
> > + break;
> > + case ATOMISP_CSS_RAW_FRAME:
> > + *info = p_info.raw_output_info;
> > + dev_dbg(isp->dev, "getting raw frame info.\n");
>
> Can we get break; here followed by default case?
Surely. I'll do such change on a separate patch.
Regards,
Mauro
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-02 6:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-29 8:49 [PATCH 1/2] media: atomisp: better describe get_frame_info issues Mauro Carvalho Chehab
2021-10-29 8:49 ` [PATCH 2/2] media: atomisp: set default mode Mauro Carvalho Chehab
2021-10-29 19:27 ` Mauro Carvalho Chehab
2021-10-29 9:01 ` [PATCH 1/2] media: atomisp: better describe get_frame_info issues Andy Shevchenko
2021-11-02 6:54 ` Mauro Carvalho Chehab
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).