All of lore.kernel.org
 help / color / mirror / Atom feed
* media: platform/rockchip/rkisp1 - v4l-compliance reports errors
@ 2021-07-29 12:36 Jens Korinth
  2021-07-29 22:38 ` Ezequiel Garcia
  0 siblings, 1 reply; 7+ messages in thread
From: Jens Korinth @ 2021-07-29 12:36 UTC (permalink / raw)
  To: linux-media

[-- Attachment #1: Type: text/plain, Size: 965 bytes --]

Hi *,

I am working on a camera system on Rockchip RK3399 board (Firefly ROC-RK3399-PC-Plus). Tried to use the rkisp1 driver, but was unable to connect to the rkisp1_mainpath output node, because format negotiation failed; so I ran v4l-compliance next and found that it reports several errors (see attached report).

Upon closer inspection I noticed in the VIDIOC_ENUM_FMT handler in drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c:1169+ that

1) the "reserved" member is not zeroed,
2) the userspace pointer to the v4l2_fmtdesc f is not checked via access_ok, and
3) it isn't copied from/to userspace using copy_from_user/copy_to_user.

I'm not sure if this is necessary in general, but at least on my platform the zeroing of the reserved member only worked correctly when I added the userspace copies. But even after these fixes, v4l-compliance reports further issues in format enumeration and negotiation. Is this a known issue?

Thanks!
-Jens

[-- Attachment #2: v4l2-compliance-report.txt --]
[-- Type: text/plain, Size: 3858 bytes --]

v4l2-compliance SHA: not available, 64 bits

Compliance test for rkisp1 device /dev/video0:

Driver Info:
	Driver name      : rkisp1
	Card type        : rkisp1
	Bus info         : platform:rkisp1
	Driver version   : 5.12.12
	Capabilities     : 0xa4201000
		Video Capture Multiplanar
		Streaming
		Extended Pix Format
		Device Capabilities
	Device Caps      : 0x24201000
		Video Capture Multiplanar
		Streaming
		Extended Pix Format
Media Driver Info:
	Driver name      : rkisp1
	Model            : rkisp1
	Serial           : 
	Bus info         : platform:rkisp1
	Media version    : 5.12.12
	Hardware revision: 0x0000000a (10)
	Driver version   : 5.12.12
Interface Info:
	ID               : 0x0300000d
	Type             : V4L Video
Entity Info:
	ID               : 0x0000000c (12)
	Name             : rkisp1_mainpath
	Function         : V4L2 I/O
	Pad 0x0100000f   : 0: Sink
	  Link 0x02000022: from remote pad 0x1000008 of entity 'rkisp1_resizer_mainpath': Data, Enabled, Immutable

Required ioctls:
	test MC information (see 'Media Driver Info' above): OK
	test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
	test second /dev/video0 open: OK
	test VIDIOC_QUERYCAP: OK
	test VIDIOC_G/S_PRIORITY: OK
	test for unlimited opens: OK

Debug ioctls:
	test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
	test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
	test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
	test VIDIOC_ENUMAUDIO: OK (Not Supported)
	test VIDIOC_G/S/ENUMINPUT: OK
	test VIDIOC_G/S_AUDIO: OK (Not Supported)
	Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
	test VIDIOC_G/S_MODULATOR: OK (Not Supported)
	test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
	test VIDIOC_ENUMAUDOUT: OK (Not Supported)
	test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
	test VIDIOC_G/S_AUDOUT: OK (Not Supported)
	Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
	test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
	test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
	test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
	test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls (Input 0):
	test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
	test VIDIOC_QUERYCTRL: OK (Not Supported)
	test VIDIOC_G/S_CTRL: OK (Not Supported)
	test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
	test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
	test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
	Standard Controls: 0 Private Controls: 0

Format ioctls (Input 0):
		fail: v4l2-test-formats.cpp(311): Video Capture Multiplanar cap set, but no Video Capture Multiplanar formats defined
	test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: FAIL
	test VIDIOC_G/S_PARM: OK (Not Supported)
	test VIDIOC_G_FBUF: OK (Not Supported)
		fail: v4l2-test-formats.cpp(461): pixelformat 56595559 (YUYV) for buftype 9 not reported by ENUM_FMT
	test VIDIOC_G_FMT: FAIL
		fail: v4l2-test-formats.cpp(461): pixelformat 56595559 (YUYV) for buftype 9 not reported by ENUM_FMT
	test VIDIOC_TRY_FMT: FAIL
		fail: v4l2-test-formats.cpp(461): pixelformat 56595559 (YUYV) for buftype 9 not reported by ENUM_FMT
	test VIDIOC_S_FMT: FAIL
	test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
	test Cropping: OK (Not Supported)
	test Composing: OK (Not Supported)
	test Scaling: OK

Codec ioctls (Input 0):
	test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
	test VIDIOC_G_ENC_INDEX: OK (Not Supported)
	test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 0):
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
	test VIDIOC_EXPBUF: OK
	test Requests: OK (Not Supported)

Total for rkisp1 device /dev/video0: 45, Succeeded: 41, Failed: 4, Warnings: 0


^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: media: platform/rockchip/rkisp1 - v4l-compliance reports errors
  2021-07-29 12:36 media: platform/rockchip/rkisp1 - v4l-compliance reports errors Jens Korinth
@ 2021-07-29 22:38 ` Ezequiel Garcia
  2021-07-30 10:20   ` Dafna Hirschfeld
  0 siblings, 1 reply; 7+ messages in thread
From: Ezequiel Garcia @ 2021-07-29 22:38 UTC (permalink / raw)
  To: Jens Korinth; +Cc: linux-media, Dafna Hirschfeld, Helen Koike

(Adding Dafna and Helen)

On Thu, 29 Jul 2021 at 09:36, Jens Korinth <jens.korinth@trinamix.de> wrote:
>
> Hi *,
>
> I am working on a camera system on Rockchip RK3399 board (Firefly ROC-RK3399-PC-Plus). Tried to use the rkisp1 driver, but was unable to connect to the rkisp1_mainpath output node, because format negotiation failed; so I ran v4l-compliance next and found that it reports several errors (see attached report).
>
> Upon closer inspection I noticed in the VIDIOC_ENUM_FMT handler in drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c:1169+ that
>
> 1) the "reserved" member is not zeroed,
> 2) the userspace pointer to the v4l2_fmtdesc f is not checked via access_ok, and
> 3) it isn't copied from/to userspace using copy_from_user/copy_to_user.
>
> I'm not sure if this is necessary in general, but at least on my platform the zeroing of the reserved member only worked correctly when I added the userspace copies. But even after these fixes, v4l-compliance reports further issues in format enumeration and negotiation. Is this a known issue?
>
> Thanks!
> -Jens

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: media: platform/rockchip/rkisp1 - v4l-compliance reports errors
  2021-07-29 22:38 ` Ezequiel Garcia
@ 2021-07-30 10:20   ` Dafna Hirschfeld
       [not found]     ` <AM0PR04MB58250C5C17446561D4A18EDA98EC9@AM0PR04MB5825.eurprd04.prod.outlook.com>
  0 siblings, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2021-07-30 10:20 UTC (permalink / raw)
  To: Ezequiel Garcia, Jens Korinth; +Cc: linux-media, Helen Koike

Hi,


On 30.07.21 00:38, Ezequiel Garcia wrote:
> (Adding Dafna and Helen)
> 
> On Thu, 29 Jul 2021 at 09:36, Jens Korinth <jens.korinth@trinamix.de> wrote:
>>
>> Hi *,
>>
>> I am working on a camera system on Rockchip RK3399 board (Firefly ROC-RK3399-PC-Plus). Tried to use the rkisp1 driver, but was unable to connect to the rkisp1_mainpath output node, because format negotiation failed; so I ran v4l-compliance next and found that it reports several errors (see attached report).

Hi, thanks for testing and reporting. We added some new features in order to supported the driver that also needed new code in v4l-utils in order
to use v4l2-ctl and to pass compliance. Therefore you should clone the repo and compile those tools in order to use them for the driver:

git://linuxtv.org/v4l-utils.git


>>
>> Upon closer inspection I noticed in the VIDIOC_ENUM_FMT handler in drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c:1169+ that

The file rkisp1-dev.c does only the probe/remove function. The callbacks are implemented in other files.

>>
>> 1) the "reserved" member is not zeroed,
>> 2) the userspace pointer to the v4l2_fmtdesc f is not checked via access_ok, and
>> 3) it isn't copied from/to userspace using copy_from_user/copy_to_user.

Those things seems to me like something that should be in the v4l2-core.

Thanks,
Dafna

>>
>> I'm not sure if this is necessary in general, but at least on my platform the zeroing of the reserved member only worked correctly when I added the userspace copies. But even after these fixes, v4l-compliance reports further issues in format enumeration and negotiation. Is this a known issue?
>>
>> Thanks!
>> -Jens

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [EXT] Re: media: platform/rockchip/rkisp1 - v4l-compliance reports errors
       [not found]     ` <AM0PR04MB58250C5C17446561D4A18EDA98EC9@AM0PR04MB5825.eurprd04.prod.outlook.com>
@ 2021-08-02  9:48       ` Dafna Hirschfeld
  2021-08-06  8:07         ` Jens Korinth
  0 siblings, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2021-08-02  9:48 UTC (permalink / raw)
  To: Jens Korinth; +Cc: Collabora Kernel ML, Linux Media Mailing List



On 30.07.21 17:33, Jens Korinth wrote:
> Hi Dafna,
> 
> Thanks for your quick answer!
> 
>> Therefore you should clone the repo and compile those tools in order to use them for the driver:
> 
> Thought I did that, but it turns out I didn't pay attention to the PATH order and the system version was used, thanks!
> 
> The camera I need to support is the OV9282, which is Y10 monochrome sensor. I've had some success passing it through the ISP as SRGGB10_1x10, but it does not feel like the right way(tm). 😊 I need the raw data with as little processing as possible (though 16b extension would be nice) - any advice?

Hi,
Currently the driver does not support Y10 format. But I think that configuring the isp subdevice with SRGGB10_1x10 should work.
I think it is possible to just add Y10 format to the list of supported formats in rkisp1-isp1.c with identical data as SRGGB10_1x10:


diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
index d596bc040005..051f8d45e3cc 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
@@ -59,6 +59,13 @@ static const struct rkisp1_isp_mbus_info rkisp1_isp_formats[] = {
                 .mbus_code      = MEDIA_BUS_FMT_YUYV8_2X8,
                 .pixel_enc      = V4L2_PIXEL_ENC_YUV,
                 .direction      = RKISP1_ISP_SD_SRC,
+       }, {
+               .mbus_code      = MEDIA_BUS_FMT_Y10_1X10,
+               .pixel_enc      = V4L2_PIXEL_ENC_BAYER,
+               .mipi_dt        = RKISP1_CIF_CSI2_DT_RAW10,
+               .bayer_pat      = RKISP1_RAW_RGGB,
+               .bus_width      = 10,
+               .direction      = RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
         }, {
                 .mbus_code      = MEDIA_BUS_FMT_SRGGB10_1X10,
                 .pixel_enc      = V4L2_PIXEL_ENC_BAYER,


But I don't have a monochrome camera to test it.

Thanks,
Dafna

> 
> Thanks a lot!
> Jens
> 
> -----Original Message-----
> From: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Sent: Friday, July 30, 2021 12:21 PM
> To: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>; Jens Korinth <jens.korinth@trinamix.de>
> Cc: linux-media@vger.kernel.org; Helen Koike <helen.koike@collabora.com>
> Subject: [EXT] Re: media: platform/rockchip/rkisp1 - v4l-compliance reports errors
> 
> Hi,
> 
> 
> On 30.07.21 00:38, Ezequiel Garcia wrote:
>> (Adding Dafna and Helen)
>>
>> On Thu, 29 Jul 2021 at 09:36, Jens Korinth <jens.korinth@trinamix.de> wrote:
>>>
>>> Hi *,
>>>
>>> I am working on a camera system on Rockchip RK3399 board (Firefly ROC-RK3399-PC-Plus). Tried to use the rkisp1 driver, but was unable to connect to the rkisp1_mainpath output node, because format negotiation failed; so I ran v4l-compliance next and found that it reports several errors (see attached report).
> 
> Hi, thanks for testing and reporting. We added some new features in order to supported the driver that also needed new code in v4l-utils in order to use v4l2-ctl and to pass compliance. Therefore you should clone the repo and compile those tools in order to use them for the driver:
> 
> git://linuxtv.org/v4l-utils.git
> 
> 
>>>
>>> Upon closer inspection I noticed in the VIDIOC_ENUM_FMT handler in
>>> drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c:1169+ that
> 
> The file rkisp1-dev.c does only the probe/remove function. The callbacks are implemented in other files.
> 
>>>
>>> 1) the "reserved" member is not zeroed,
>>> 2) the userspace pointer to the v4l2_fmtdesc f is not checked via
>>> access_ok, and
>>> 3) it isn't copied from/to userspace using copy_from_user/copy_to_user.
> 
> Those things seems to me like something that should be in the v4l2-core.
> 
> Thanks,
> Dafna
> 
>>>
>>> I'm not sure if this is necessary in general, but at least on my platform the zeroing of the reserved member only worked correctly when I added the userspace copies. But even after these fixes, v4l-compliance reports further issues in format enumeration and negotiation. Is this a known issue?
>>>
>>> Thanks!
>>> -Jens

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [EXT] Re: media: platform/rockchip/rkisp1 - v4l-compliance reports errors
  2021-08-02  9:48       ` [EXT] " Dafna Hirschfeld
@ 2021-08-06  8:07         ` Jens Korinth
  2021-08-06  8:54           ` Dafna Hirschfeld
  2021-08-06  8:55           ` Dafna Hirschfeld
  0 siblings, 2 replies; 7+ messages in thread
From: Jens Korinth @ 2021-08-06  8:07 UTC (permalink / raw)
  To: Dafna Hirschfeld; +Cc: Collabora Kernel ML, Linux Media Mailing List

Hi Dafna!

> I think it is possible to just add Y10 format to the list of supported formats in rkisp1-isp1.c with identical data as SRGGB10_1x10:

I tested the patch, and it seems to work on the rkisp1_isp side, so I tried to add Y10 to the mainpath capture node:

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
index 5f6c9d1623e4..4c3079ace9eb 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
@@ -176,6 +176,10 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_mp_fmts[] = {
                .fourcc = V4L2_PIX_FMT_SRGGB10,
                .write_format = RKISP1_MI_CTRL_MP_WRITE_RAW12,
                .mbus = MEDIA_BUS_FMT_SRGGB10_1X10,
+       }, {
+               .fourcc = V4L2_PIX_FMT_Y10,
+               .write_format = RKISP1_MI_CTRL_MP_WRITE_RAW12,
+               .mbus = MEDIA_BUS_FMT_Y10_1X10,
        }, {
                .fourcc = V4L2_PIX_FMT_SGRBG10,
                .write_format = RKISP1_MI_CTRL_MP_WRITE_RAW12,
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
index 2e5b57e3aedc..8e9c2abfe0d2 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
@@ -66,6 +66,13 @@ static const struct rkisp1_isp_mbus_info rkisp1_isp_formats[] = {
                .bayer_pat      = RKISP1_RAW_RGGB,
                .bus_width      = 10,
                .direction      = RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
+       }, {
+               .mbus_code      = MEDIA_BUS_FMT_Y10_1X10,
+               .pixel_enc      = V4L2_PIXEL_ENC_BAYER,
+               .mipi_dt        = RKISP1_CIF_CSI2_DT_RAW10,
+               .bayer_pat      = RKISP1_RAW_RGGB,
+               .bus_width      = 10,
+               .direction      = RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
        }, {
                .mbus_code      = MEDIA_BUS_FMT_SBGGR10_1X10,
                .pixel_enc      = V4L2_PIXEL_ENC_BAYER,

But when I tried to set the format via `v4l2-ctl -v width=1280,height=720,pixelformat="Y10 "` I get a kernel oops:

[  201.868529] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000005
[  201.869382] Mem abort info:
[  201.869654]   ESR = 0x96000004
[  201.869952]   EC = 0x25: DABT (current EL), IL = 32 bits
[  201.870585]   SET = 0, FnV = 0
[  201.870895]   EA = 0, S1PTW = 0
[  201.871202] Data abort info:
[  201.871682]   ISV = 0, ISS = 0x00000004
[  201.872057]   CM = 0, WnR = 0
[  201.872347] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000058c8000
[  201.872951] [0000000000000005] pgd=0000000000000000, p4d=0000000000000000
[  201.873605] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[  201.874135] Modules linked in: trinamix_ov9282(E) rockchip_isp1(E) rfkill governor_performance snd_soc_hdmi_codec videobuf2_dma_contig videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_common snd_soc_simple_card dw_hdmi_i2s_audio panfrost dw_hdmi_cec snd_soc_rockchip_i2s snd_soc_simple_card_utils gpu_sched snd_soc_rockchip_pcm snd_soc_core snd_pcm_dmaengine snd_pcm snd_timer snd soundcore fusb302 tcpm typec cpufreq_dt zram sch_fq_codel v4l2_fwnode videodev mc sunrpc ip_tables x_tables autofs4 realtek phy_rockchip_dphy_rx0 rockchipdrm analogix_dp dw_hdmi dw_mipi_dsi mp8859 drm_kms_helper cec rc_core dwmac_rk stmmac_platform drm stmmac pcs_xpcs drm_panel_orientation_quirks pwm_bl adc_keys [last unloaded: rockchip_isp1]
[  201.880487] CPU: 0 PID: 1998 Comm: v4l2-ctl Tainted: G            E     5.12.12-rockchip64 #trunk
[  201.881316] Hardware name: trinamiX Kiwi (DT)
[  201.881729] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
[  201.882298] pc : rkisp1_try_fmt.isra.0+0x124/0x260 [rockchip_isp1]
[  201.882928] lr : rkisp1_try_fmt.isra.0+0x124/0x260 [rockchip_isp1]
[  201.883543] sp : ffff8000127cbb20
[  201.883862] x29: ffff8000127cbb20 x28: 0000000000000000

Message from syslogd@trinamix-kiwi at Aug  6 07:42:02 ...
 kernel:[  201.873605] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[  201.884380] x27: ffff000001088700 x26: ffff000004c1c000
[  201.884897] x25: 0000000000000000 x24: ffff000004f395e0
[  201.885413] x23: ffff000004f395e8 x22: ffff000010495700
[  201.885931] x21: ffff8000092c87bc x20: ffff0000054e0d1c
[  201.886449] x19: ffff0000054e0d08 x18: 0000000000000000
[  201.886965] x17: 0000000000000000 x16: 0000000000000000
[  201.887481] x15: 0000000000000000 x14: 0000000000000000
[  201.887996] x13: 0000000100000000 x12: 0000000000000000
[  201.888512] x11: 0000000000000000 x10: 0000000000000000
[  201.889027] x9 : 0000000000000000 x8 : 0000000000000000
[  201.889543] x7 : 0000000000000000 x6 : 0000000000000010
[  201.890059] x5 : 0000000000000001 x4 : ffff800009182a50
[  201.890575] x3 : 0000000032314752 x2 : ffff800009183050
[  201.891092] x1 : 0000000000000040 x0 : 0000000000000000
[  201.891609] Call trace:
[  201.891851]  rkisp1_try_fmt.isra.0+0x124/0x260 [rockchip_isp1]
[  201.892439]  rkisp1_set_fmt+0x30/0x70 [rockchip_isp1]
[  201.892955]  rkisp1_s_fmt_vid_cap_mplane+0x2c/0x48 [rockchip_isp1]
[  201.893571]  v4l_s_fmt+0x478/0x548 [videodev]
[  201.894190]  __video_do_ioctl+0x184/0x3d8 [videodev]
[  201.894844]  video_usercopy+0x160/0x6e0 [videodev]
[  201.895486]  video_ioctl2+0x18/0x30 [videodev]

Message from syslogd@trinamix-kiwi at Aug  6 07:42:02 ...
 kernel:[  201.898923] Code: a9087e9f a9097e9f b9400a60 97faea1a (39401402)
[  201.896100]  v4l2_ioctl+0x40/0x60 [videodev]
[  201.896692]  __arm64_sys_ioctl+0xa8/0xe8
[  201.897085]  el0_svc_common.constprop.2+0x8c/0x128
[  201.897556]  do_el0_svc+0x24/0x90
[  201.897892]  el0_svc+0x24/0x38
[  201.898202]  el0_sync_handler+0x90/0xb8
[  201.898580]  el0_sync+0x160/0x180
[  201.898923] Code: a9087e9f a9097e9f b9400a60 97faea1a (39401402)
[  201.899497] ---[ end trace 4eecb08abf2063c0 ]---

Thanks a lot for your help!
-Jens

________________________________________
From: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Sent: Monday, August 2, 2021 11:48 AM
To: Jens Korinth
Cc: Collabora Kernel ML; Linux Media Mailing List
Subject: Re: [EXT] Re: media: platform/rockchip/rkisp1 - v4l-compliance reports errors



On 30.07.21 17:33, Jens Korinth wrote:
> Hi Dafna,
>
> Thanks for your quick answer!
>
>> Therefore you should clone the repo and compile those tools in order to use them for the driver:
>
> Thought I did that, but it turns out I didn't pay attention to the PATH order and the system version was used, thanks!
>
> The camera I need to support is the OV9282, which is Y10 monochrome sensor. I've had some success passing it through the ISP as SRGGB10_1x10, but it does not feel like the right way(tm). 😊 I need the raw data with as little processing as possible (though 16b extension would be nice) - any advice?

Hi,
Currently the driver does not support Y10 format. But I think that configuring the isp subdevice with SRGGB10_1x10 should work.
I think it is possible to just add Y10 format to the list of supported formats in rkisp1-isp1.c with identical data as SRGGB10_1x10:


diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
index d596bc040005..051f8d45e3cc 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
@@ -59,6 +59,13 @@ static const struct rkisp1_isp_mbus_info rkisp1_isp_formats[] = {
                 .mbus_code      = MEDIA_BUS_FMT_YUYV8_2X8,
                 .pixel_enc      = V4L2_PIXEL_ENC_YUV,
                 .direction      = RKISP1_ISP_SD_SRC,
+       }, {
+               .mbus_code      = MEDIA_BUS_FMT_Y10_1X10,
+               .pixel_enc      = V4L2_PIXEL_ENC_BAYER,
+               .mipi_dt        = RKISP1_CIF_CSI2_DT_RAW10,
+               .bayer_pat      = RKISP1_RAW_RGGB,
+               .bus_width      = 10,
+               .direction      = RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
         }, {
                 .mbus_code      = MEDIA_BUS_FMT_SRGGB10_1X10,
                 .pixel_enc      = V4L2_PIXEL_ENC_BAYER,


But I don't have a monochrome camera to test it.

Thanks,
Dafna

>
> Thanks a lot!
> Jens
>
> -----Original Message-----
> From: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Sent: Friday, July 30, 2021 12:21 PM
> To: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>; Jens Korinth <jens.korinth@trinamix.de>
> Cc: linux-media@vger.kernel.org; Helen Koike <helen.koike@collabora.com>
> Subject: [EXT] Re: media: platform/rockchip/rkisp1 - v4l-compliance reports errors
>
> Hi,
>
>
> On 30.07.21 00:38, Ezequiel Garcia wrote:
>> (Adding Dafna and Helen)
>>
>> On Thu, 29 Jul 2021 at 09:36, Jens Korinth <jens.korinth@trinamix.de> wrote:
>>>
>>> Hi *,
>>>
>>> I am working on a camera system on Rockchip RK3399 board (Firefly ROC-RK3399-PC-Plus). Tried to use the rkisp1 driver, but was unable to connect to the rkisp1_mainpath output node, because format negotiation failed; so I ran v4l-compliance next and found that it reports several errors (see attached report).
>
> Hi, thanks for testing and reporting. We added some new features in order to supported the driver that also needed new code in v4l-utils in order to use v4l2-ctl and to pass compliance. Therefore you should clone the repo and compile those tools in order to use them for the driver:
>
> git://linuxtv.org/v4l-utils.git
>
>
>>>
>>> Upon closer inspection I noticed in the VIDIOC_ENUM_FMT handler in
>>> drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c:1169+ that
>
> The file rkisp1-dev.c does only the probe/remove function. The callbacks are implemented in other files.
>
>>>
>>> 1) the "reserved" member is not zeroed,
>>> 2) the userspace pointer to the v4l2_fmtdesc f is not checked via
>>> access_ok, and
>>> 3) it isn't copied from/to userspace using copy_from_user/copy_to_user.
>
> Those things seems to me like something that should be in the v4l2-core.
>
> Thanks,
> Dafna
>
>>>
>>> I'm not sure if this is necessary in general, but at least on my platform the zeroing of the reserved member only worked correctly when I added the userspace copies. But even after these fixes, v4l-compliance reports further issues in format enumeration and negotiation. Is this a known issue?
>>>
>>> Thanks!
>>> -Jens

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [EXT] Re: media: platform/rockchip/rkisp1 - v4l-compliance reports errors
  2021-08-06  8:07         ` Jens Korinth
@ 2021-08-06  8:54           ` Dafna Hirschfeld
  2021-08-06  8:55           ` Dafna Hirschfeld
  1 sibling, 0 replies; 7+ messages in thread
From: Dafna Hirschfeld @ 2021-08-06  8:54 UTC (permalink / raw)
  To: Jens Korinth; +Cc: Collabora Kernel ML, Linux Media Mailing List



On 06.08.21 10:07, Jens Korinth wrote:
> Hi Dafna!
> 
>> I think it is possible to just add Y10 format to the list of supported formats in rkisp1-isp1.c with identical data as SRGGB10_1x10:
> 
> I tested the patch, and it seems to work on the rkisp1_isp side, so I tried to add Y10 to the mainpath capture node:
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> index 5f6c9d1623e4..4c3079ace9eb 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> @@ -176,6 +176,10 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_mp_fmts[] = {
>                  .fourcc = V4L2_PIX_FMT_SRGGB10,
>                  .write_format = RKISP1_MI_CTRL_MP_WRITE_RAW12,
>                  .mbus = MEDIA_BUS_FMT_SRGGB10_1X10,
> +       }, {
> +               .fourcc = V4L2_PIX_FMT_Y10,
> +               .write_format = RKISP1_MI_CTRL_MP_WRITE_RAW12,
> +               .mbus = MEDIA_BUS_FMT_Y10_1X10,
>          }, {
>                  .fourcc = V4L2_PIX_FMT_SGRBG10,
>                  .write_format = RKISP1_MI_CTRL_MP_WRITE_RAW12,
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index 2e5b57e3aedc..8e9c2abfe0d2 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -66,6 +66,13 @@ static const struct rkisp1_isp_mbus_info rkisp1_isp_formats[] = {
>                  .bayer_pat      = RKISP1_RAW_RGGB,
>                  .bus_width      = 10,
>                  .direction      = RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
> +       }, {
> +               .mbus_code      = MEDIA_BUS_FMT_Y10_1X10,
> +               .pixel_enc      = V4L2_PIXEL_ENC_BAYER,
> +               .mipi_dt        = RKISP1_CIF_CSI2_DT_RAW10,
> +               .bayer_pat      = RKISP1_RAW_RGGB,
> +               .bus_width      = 10,
> +               .direction      = RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>          }, {
>                  .mbus_code      = MEDIA_BUS_FMT_SBGGR10_1X10,
>                  .pixel_enc      = V4L2_PIXEL_ENC_BAYER,
> 
> But when I tried to set the format via `v4l2-ctl -v width=1280,height=720,pixelformat="Y10 "` I get a kernel oops:
> 
> [  201.868529] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000005
> [  201.869382] Mem abort info:
> [  201.869654]   ESR = 0x96000004
> [  201.869952]   EC = 0x25: DABT (current EL), IL = 32 bits
> [  201.870585]   SET = 0, FnV = 0
> [  201.870895]   EA = 0, S1PTW = 0
> [  201.871202] Data abort info:
> [  201.871682]   ISV = 0, ISS = 0x00000004
> [  201.872057]   CM = 0, WnR = 0
> [  201.872347] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000058c8000
> [  201.872951] [0000000000000005] pgd=0000000000000000, p4d=0000000000000000
> [  201.873605] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> [  201.874135] Modules linked in: trinamix_ov9282(E) rockchip_isp1(E) rfkill governor_performance snd_soc_hdmi_codec videobuf2_dma_contig videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_common snd_soc_simple_card dw_hdmi_i2s_audio panfrost dw_hdmi_cec snd_soc_rockchip_i2s snd_soc_simple_card_utils gpu_sched snd_soc_rockchip_pcm snd_soc_core snd_pcm_dmaengine snd_pcm snd_timer snd soundcore fusb302 tcpm typec cpufreq_dt zram sch_fq_codel v4l2_fwnode videodev mc sunrpc ip_tables x_tables autofs4 realtek phy_rockchip_dphy_rx0 rockchipdrm analogix_dp dw_hdmi dw_mipi_dsi mp8859 drm_kms_helper cec rc_core dwmac_rk stmmac_platform drm stmmac pcs_xpcs drm_panel_orientation_quirks pwm_bl adc_keys [last unloaded: rockchip_isp1]
> [  201.880487] CPU: 0 PID: 1998 Comm: v4l2-ctl Tainted: G            E     5.12.12-rockchip64 #trunk
> [  201.881316] Hardware name: trinamiX Kiwi (DT)
> [  201.881729] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
> [  201.882298] pc : rkisp1_try_fmt.isra.0+0x124/0x260 [rockchip_isp1]
> [  201.882928] lr : rkisp1_try_fmt.isra.0+0x124/0x260 [rockchip_isp1]
> [  201.883543] sp : ffff8000127cbb20
> [  201.883862] x29: ffff8000127cbb20 x28: 0000000000000000
> 
> Message from syslogd@trinamix-kiwi at Aug  6 07:42:02 ...
>   kernel:[  201.873605] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> [  201.884380] x27: ffff000001088700 x26: ffff000004c1c000
> [  201.884897] x25: 0000000000000000 x24: ffff000004f395e0
> [  201.885413] x23: ffff000004f395e8 x22: ffff000010495700
> [  201.885931] x21: ffff8000092c87bc x20: ffff0000054e0d1c
> [  201.886449] x19: ffff0000054e0d08 x18: 0000000000000000
> [  201.886965] x17: 0000000000000000 x16: 0000000000000000
> [  201.887481] x15: 0000000000000000 x14: 0000000000000000
> [  201.887996] x13: 0000000100000000 x12: 0000000000000000
> [  201.888512] x11: 0000000000000000 x10: 0000000000000000
> [  201.889027] x9 : 0000000000000000 x8 : 0000000000000000
> [  201.889543] x7 : 0000000000000000 x6 : 0000000000000010
> [  201.890059] x5 : 0000000000000001 x4 : ffff800009182a50
> [  201.890575] x3 : 0000000032314752 x2 : ffff800009183050
> [  201.891092] x1 : 0000000000000040 x0 : 0000000000000000
> [  201.891609] Call trace:
> [  201.891851]  rkisp1_try_fmt.isra.0+0x124/0x260 [rockchip_isp1]
> [  201.892439]  rkisp1_set_fmt+0x30/0x70 [rockchip_isp1]
> [  201.892955]  rkisp1_s_fmt_vid_cap_mplane+0x2c/0x48 [rockchip_isp1]
> [  201.893571]  v4l_s_fmt+0x478/0x548 [videodev]
> [  201.894190]  __video_do_ioctl+0x184/0x3d8 [videodev]
> [  201.894844]  video_usercopy+0x160/0x6e0 [videodev]
> [  201.895486]  video_ioctl2+0x18/0x30 [videodev]
> 
> Message from syslogd@trinamix-kiwi at Aug  6 07:42:02 ...
>   kernel:[  201.898923] Code: a9087e9f a9097e9f b9400a60 97faea1a (39401402)
> [  201.896100]  v4l2_ioctl+0x40/0x60 [videodev]
> [  201.896692]  __arm64_sys_ioctl+0xa8/0xe8
> [  201.897085]  el0_svc_common.constprop.2+0x8c/0x128
> [  201.897556]  do_el0_svc+0x24/0x90
> [  201.897892]  el0_svc+0x24/0x38
> [  201.898202]  el0_sync_handler+0x90/0xb8
> [  201.898580]  el0_sync+0x160/0x180
> [  201.898923] Code: a9087e9f a9097e9f b9400a60 97faea1a (39401402)
> [  201.899497] ---[ end trace 4eecb08abf2063c0 ]---
> 
> Thanks a lot for your help!
> -Jens
> 

Hi, this is because the code calls v4l2_format_info to get information about the format.
The problem is that the function v4l2_format_info in v4l2-common.c does not have an entry for that Y10 format
so it returns null. You can add an entry for it in that function by:

{ .format = V4L2_PIX_FMT_Y10,       .pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },

Thanks,
Dafna

> ________________________________________
> From: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Sent: Monday, August 2, 2021 11:48 AM
> To: Jens Korinth
> Cc: Collabora Kernel ML; Linux Media Mailing List
> Subject: Re: [EXT] Re: media: platform/rockchip/rkisp1 - v4l-compliance reports errors
> 
> 
> 
> On 30.07.21 17:33, Jens Korinth wrote:
>> Hi Dafna,
>>
>> Thanks for your quick answer!
>>
>>> Therefore you should clone the repo and compile those tools in order to use them for the driver:
>>
>> Thought I did that, but it turns out I didn't pay attention to the PATH order and the system version was used, thanks!
>>
>> The camera I need to support is the OV9282, which is Y10 monochrome sensor. I've had some success passing it through the ISP as SRGGB10_1x10, but it does not feel like the right way(tm). 😊 I need the raw data with as little processing as possible (though 16b extension would be nice) - any advice?
> 
> Hi,
> Currently the driver does not support Y10 format. But I think that configuring the isp subdevice with SRGGB10_1x10 should work.
> I think it is possible to just add Y10 format to the list of supported formats in rkisp1-isp1.c with identical data as SRGGB10_1x10:
> 
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index d596bc040005..051f8d45e3cc 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -59,6 +59,13 @@ static const struct rkisp1_isp_mbus_info rkisp1_isp_formats[] = {
>                   .mbus_code      = MEDIA_BUS_FMT_YUYV8_2X8,
>                   .pixel_enc      = V4L2_PIXEL_ENC_YUV,
>                   .direction      = RKISP1_ISP_SD_SRC,
> +       }, {
> +               .mbus_code      = MEDIA_BUS_FMT_Y10_1X10,
> +               .pixel_enc      = V4L2_PIXEL_ENC_BAYER,
> +               .mipi_dt        = RKISP1_CIF_CSI2_DT_RAW10,
> +               .bayer_pat      = RKISP1_RAW_RGGB,
> +               .bus_width      = 10,
> +               .direction      = RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>           }, {
>                   .mbus_code      = MEDIA_BUS_FMT_SRGGB10_1X10,
>                   .pixel_enc      = V4L2_PIXEL_ENC_BAYER,
> 
> 
> But I don't have a monochrome camera to test it.
> 
> Thanks,
> Dafna
> 
>>
>> Thanks a lot!
>> Jens
>>
>> -----Original Message-----
>> From: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> Sent: Friday, July 30, 2021 12:21 PM
>> To: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>; Jens Korinth <jens.korinth@trinamix.de>
>> Cc: linux-media@vger.kernel.org; Helen Koike <helen.koike@collabora.com>
>> Subject: [EXT] Re: media: platform/rockchip/rkisp1 - v4l-compliance reports errors
>>
>> Hi,
>>
>>
>> On 30.07.21 00:38, Ezequiel Garcia wrote:
>>> (Adding Dafna and Helen)
>>>
>>> On Thu, 29 Jul 2021 at 09:36, Jens Korinth <jens.korinth@trinamix.de> wrote:
>>>>
>>>> Hi *,
>>>>
>>>> I am working on a camera system on Rockchip RK3399 board (Firefly ROC-RK3399-PC-Plus). Tried to use the rkisp1 driver, but was unable to connect to the rkisp1_mainpath output node, because format negotiation failed; so I ran v4l-compliance next and found that it reports several errors (see attached report).
>>
>> Hi, thanks for testing and reporting. We added some new features in order to supported the driver that also needed new code in v4l-utils in order to use v4l2-ctl and to pass compliance. Therefore you should clone the repo and compile those tools in order to use them for the driver:
>>
>> git://linuxtv.org/v4l-utils.git
>>
>>
>>>>
>>>> Upon closer inspection I noticed in the VIDIOC_ENUM_FMT handler in
>>>> drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c:1169+ that
>>
>> The file rkisp1-dev.c does only the probe/remove function. The callbacks are implemented in other files.
>>
>>>>
>>>> 1) the "reserved" member is not zeroed,
>>>> 2) the userspace pointer to the v4l2_fmtdesc f is not checked via
>>>> access_ok, and
>>>> 3) it isn't copied from/to userspace using copy_from_user/copy_to_user.
>>
>> Those things seems to me like something that should be in the v4l2-core.
>>
>> Thanks,
>> Dafna
>>
>>>>
>>>> I'm not sure if this is necessary in general, but at least on my platform the zeroing of the reserved member only worked correctly when I added the userspace copies. But even after these fixes, v4l-compliance reports further issues in format enumeration and negotiation. Is this a known issue?
>>>>
>>>> Thanks!
>>>> -Jens

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [EXT] Re: media: platform/rockchip/rkisp1 - v4l-compliance reports errors
  2021-08-06  8:07         ` Jens Korinth
  2021-08-06  8:54           ` Dafna Hirschfeld
@ 2021-08-06  8:55           ` Dafna Hirschfeld
  1 sibling, 0 replies; 7+ messages in thread
From: Dafna Hirschfeld @ 2021-08-06  8:55 UTC (permalink / raw)
  To: Jens Korinth; +Cc: Collabora Kernel ML, Linux Media Mailing List



On 06.08.21 10:07, Jens Korinth wrote:
> Hi Dafna!
> 
>> I think it is possible to just add Y10 format to the list of supported formats in rkisp1-isp1.c with identical data as SRGGB10_1x10:
> 
> I tested the patch, and it seems to work on the rkisp1_isp side, so I tried to add Y10 to the mainpath capture node:
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> index 5f6c9d1623e4..4c3079ace9eb 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> @@ -176,6 +176,10 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_mp_fmts[] = {
>                  .fourcc = V4L2_PIX_FMT_SRGGB10,
>                  .write_format = RKISP1_MI_CTRL_MP_WRITE_RAW12,
>                  .mbus = MEDIA_BUS_FMT_SRGGB10_1X10,
> +       }, {
> +               .fourcc = V4L2_PIX_FMT_Y10,
> +               .write_format = RKISP1_MI_CTRL_MP_WRITE_RAW12,
> +               .mbus = MEDIA_BUS_FMT_Y10_1X10,
>          }, {
>                  .fourcc = V4L2_PIX_FMT_SGRBG10,
>                  .write_format = RKISP1_MI_CTRL_MP_WRITE_RAW12,
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index 2e5b57e3aedc..8e9c2abfe0d2 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -66,6 +66,13 @@ static const struct rkisp1_isp_mbus_info rkisp1_isp_formats[] = {
>                  .bayer_pat      = RKISP1_RAW_RGGB,
>                  .bus_width      = 10,
>                  .direction      = RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
> +       }, {
> +               .mbus_code      = MEDIA_BUS_FMT_Y10_1X10,
> +               .pixel_enc      = V4L2_PIXEL_ENC_BAYER,
> +               .mipi_dt        = RKISP1_CIF_CSI2_DT_RAW10,
> +               .bayer_pat      = RKISP1_RAW_RGGB,
> +               .bus_width      = 10,
> +               .direction      = RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>          }, {
>                  .mbus_code      = MEDIA_BUS_FMT_SBGGR10_1X10,
>                  .pixel_enc      = V4L2_PIXEL_ENC_BAYER,
> 
> But when I tried to set the format via `v4l2-ctl -v width=1280,height=720,pixelformat="Y10 "` I get a kernel oops:
> 
> [  201.868529] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000005
> [  201.869382] Mem abort info:
> [  201.869654]   ESR = 0x96000004
> [  201.869952]   EC = 0x25: DABT (current EL), IL = 32 bits
> [  201.870585]   SET = 0, FnV = 0
> [  201.870895]   EA = 0, S1PTW = 0
> [  201.871202] Data abort info:
> [  201.871682]   ISV = 0, ISS = 0x00000004
> [  201.872057]   CM = 0, WnR = 0
> [  201.872347] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000058c8000
> [  201.872951] [0000000000000005] pgd=0000000000000000, p4d=0000000000000000
> [  201.873605] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> [  201.874135] Modules linked in: trinamix_ov9282(E) rockchip_isp1(E) rfkill governor_performance snd_soc_hdmi_codec videobuf2_dma_contig videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_common snd_soc_simple_card dw_hdmi_i2s_audio panfrost dw_hdmi_cec snd_soc_rockchip_i2s snd_soc_simple_card_utils gpu_sched snd_soc_rockchip_pcm snd_soc_core snd_pcm_dmaengine snd_pcm snd_timer snd soundcore fusb302 tcpm typec cpufreq_dt zram sch_fq_codel v4l2_fwnode videodev mc sunrpc ip_tables x_tables autofs4 realtek phy_rockchip_dphy_rx0 rockchipdrm analogix_dp dw_hdmi dw_mipi_dsi mp8859 drm_kms_helper cec rc_core dwmac_rk stmmac_platform drm stmmac pcs_xpcs drm_panel_orientation_quirks pwm_bl adc_keys [last unloaded: rockchip_isp1]
> [  201.880487] CPU: 0 PID: 1998 Comm: v4l2-ctl Tainted: G            E     5.12.12-rockchip64 #trunk
> [  201.881316] Hardware name: trinamiX Kiwi (DT)
> [  201.881729] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
> [  201.882298] pc : rkisp1_try_fmt.isra.0+0x124/0x260 [rockchip_isp1]
> [  201.882928] lr : rkisp1_try_fmt.isra.0+0x124/0x260 [rockchip_isp1]
> [  201.883543] sp : ffff8000127cbb20
> [  201.883862] x29: ffff8000127cbb20 x28: 0000000000000000
> 
> Message from syslogd@trinamix-kiwi at Aug  6 07:42:02 ...
>   kernel:[  201.873605] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> [  201.884380] x27: ffff000001088700 x26: ffff000004c1c000
> [  201.884897] x25: 0000000000000000 x24: ffff000004f395e0
> [  201.885413] x23: ffff000004f395e8 x22: ffff000010495700
> [  201.885931] x21: ffff8000092c87bc x20: ffff0000054e0d1c
> [  201.886449] x19: ffff0000054e0d08 x18: 0000000000000000
> [  201.886965] x17: 0000000000000000 x16: 0000000000000000
> [  201.887481] x15: 0000000000000000 x14: 0000000000000000
> [  201.887996] x13: 0000000100000000 x12: 0000000000000000
> [  201.888512] x11: 0000000000000000 x10: 0000000000000000
> [  201.889027] x9 : 0000000000000000 x8 : 0000000000000000
> [  201.889543] x7 : 0000000000000000 x6 : 0000000000000010
> [  201.890059] x5 : 0000000000000001 x4 : ffff800009182a50
> [  201.890575] x3 : 0000000032314752 x2 : ffff800009183050
> [  201.891092] x1 : 0000000000000040 x0 : 0000000000000000
> [  201.891609] Call trace:
> [  201.891851]  rkisp1_try_fmt.isra.0+0x124/0x260 [rockchip_isp1]
> [  201.892439]  rkisp1_set_fmt+0x30/0x70 [rockchip_isp1]
> [  201.892955]  rkisp1_s_fmt_vid_cap_mplane+0x2c/0x48 [rockchip_isp1]
> [  201.893571]  v4l_s_fmt+0x478/0x548 [videodev]
> [  201.894190]  __video_do_ioctl+0x184/0x3d8 [videodev]
> [  201.894844]  video_usercopy+0x160/0x6e0 [videodev]
> [  201.895486]  video_ioctl2+0x18/0x30 [videodev]
> 
> Message from syslogd@trinamix-kiwi at Aug  6 07:42:02 ...
>   kernel:[  201.898923] Code: a9087e9f a9097e9f b9400a60 97faea1a (39401402)
> [  201.896100]  v4l2_ioctl+0x40/0x60 [videodev]
> [  201.896692]  __arm64_sys_ioctl+0xa8/0xe8
> [  201.897085]  el0_svc_common.constprop.2+0x8c/0x128
> [  201.897556]  do_el0_svc+0x24/0x90
> [  201.897892]  el0_svc+0x24/0x38
> [  201.898202]  el0_sync_handler+0x90/0xb8
> [  201.898580]  el0_sync+0x160/0x180
> [  201.898923] Code: a9087e9f a9097e9f b9400a60 97faea1a (39401402)
> [  201.899497] ---[ end trace 4eecb08abf2063c0 ]---
> 
> Thanks a lot for your help!
> -Jens
> 

Hi, this is because the code calls v4l2_format_info to get information about the format.
The problem is that the function v4l2_format_info in v4l2-common.c does not have an entry for that Y10 format
so it returns null. You can add an entry for it in that function by:

{ .format = V4L2_PIX_FMT_Y10,       .pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },

Thanks,
Dafna

> ________________________________________
> From: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Sent: Monday, August 2, 2021 11:48 AM
> To: Jens Korinth
> Cc: Collabora Kernel ML; Linux Media Mailing List
> Subject: Re: [EXT] Re: media: platform/rockchip/rkisp1 - v4l-compliance reports errors
> 
> 
> 
> On 30.07.21 17:33, Jens Korinth wrote:
>> Hi Dafna,
>>
>> Thanks for your quick answer!
>>
>>> Therefore you should clone the repo and compile those tools in order to use them for the driver:
>>
>> Thought I did that, but it turns out I didn't pay attention to the PATH order and the system version was used, thanks!
>>
>> The camera I need to support is the OV9282, which is Y10 monochrome sensor. I've had some success passing it through the ISP as SRGGB10_1x10, but it does not feel like the right way(tm). 😊 I need the raw data with as little processing as possible (though 16b extension would be nice) - any advice?
> 
> Hi,
> Currently the driver does not support Y10 format. But I think that configuring the isp subdevice with SRGGB10_1x10 should work.
> I think it is possible to just add Y10 format to the list of supported formats in rkisp1-isp1.c with identical data as SRGGB10_1x10:
> 
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index d596bc040005..051f8d45e3cc 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -59,6 +59,13 @@ static const struct rkisp1_isp_mbus_info rkisp1_isp_formats[] = {
>                   .mbus_code      = MEDIA_BUS_FMT_YUYV8_2X8,
>                   .pixel_enc      = V4L2_PIXEL_ENC_YUV,
>                   .direction      = RKISP1_ISP_SD_SRC,
> +       }, {
> +               .mbus_code      = MEDIA_BUS_FMT_Y10_1X10,
> +               .pixel_enc      = V4L2_PIXEL_ENC_BAYER,
> +               .mipi_dt        = RKISP1_CIF_CSI2_DT_RAW10,
> +               .bayer_pat      = RKISP1_RAW_RGGB,
> +               .bus_width      = 10,
> +               .direction      = RKISP1_ISP_SD_SINK | RKISP1_ISP_SD_SRC,
>           }, {
>                   .mbus_code      = MEDIA_BUS_FMT_SRGGB10_1X10,
>                   .pixel_enc      = V4L2_PIXEL_ENC_BAYER,
> 
> 
> But I don't have a monochrome camera to test it.
> 
> Thanks,
> Dafna
> 
>>
>> Thanks a lot!
>> Jens
>>
>> -----Original Message-----
>> From: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> Sent: Friday, July 30, 2021 12:21 PM
>> To: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>; Jens Korinth <jens.korinth@trinamix.de>
>> Cc: linux-media@vger.kernel.org; Helen Koike <helen.koike@collabora.com>
>> Subject: [EXT] Re: media: platform/rockchip/rkisp1 - v4l-compliance reports errors
>>
>> Hi,
>>
>>
>> On 30.07.21 00:38, Ezequiel Garcia wrote:
>>> (Adding Dafna and Helen)
>>>
>>> On Thu, 29 Jul 2021 at 09:36, Jens Korinth <jens.korinth@trinamix.de> wrote:
>>>>
>>>> Hi *,
>>>>
>>>> I am working on a camera system on Rockchip RK3399 board (Firefly ROC-RK3399-PC-Plus). Tried to use the rkisp1 driver, but was unable to connect to the rkisp1_mainpath output node, because format negotiation failed; so I ran v4l-compliance next and found that it reports several errors (see attached report).
>>
>> Hi, thanks for testing and reporting. We added some new features in order to supported the driver that also needed new code in v4l-utils in order to use v4l2-ctl and to pass compliance. Therefore you should clone the repo and compile those tools in order to use them for the driver:
>>
>> git://linuxtv.org/v4l-utils.git
>>
>>
>>>>
>>>> Upon closer inspection I noticed in the VIDIOC_ENUM_FMT handler in
>>>> drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c:1169+ that
>>
>> The file rkisp1-dev.c does only the probe/remove function. The callbacks are implemented in other files.
>>
>>>>
>>>> 1) the "reserved" member is not zeroed,
>>>> 2) the userspace pointer to the v4l2_fmtdesc f is not checked via
>>>> access_ok, and
>>>> 3) it isn't copied from/to userspace using copy_from_user/copy_to_user.
>>
>> Those things seems to me like something that should be in the v4l2-core.
>>
>> Thanks,
>> Dafna
>>
>>>>
>>>> I'm not sure if this is necessary in general, but at least on my platform the zeroing of the reserved member only worked correctly when I added the userspace copies. But even after these fixes, v4l-compliance reports further issues in format enumeration and negotiation. Is this a known issue?
>>>>
>>>> Thanks!
>>>> -Jens

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-06  8:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29 12:36 media: platform/rockchip/rkisp1 - v4l-compliance reports errors Jens Korinth
2021-07-29 22:38 ` Ezequiel Garcia
2021-07-30 10:20   ` Dafna Hirschfeld
     [not found]     ` <AM0PR04MB58250C5C17446561D4A18EDA98EC9@AM0PR04MB5825.eurprd04.prod.outlook.com>
2021-08-02  9:48       ` [EXT] " Dafna Hirschfeld
2021-08-06  8:07         ` Jens Korinth
2021-08-06  8:54           ` Dafna Hirschfeld
2021-08-06  8:55           ` Dafna Hirschfeld

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.