Setting context source and destination formats should only be done
in hantro_set_fmt_out() and hantro_set_fmt_cap() after check that
the targeted queue is not busy.
Remove these calls from hantro_reset_encoded_fmt() and
hantro_reset_raw_fmt() to clean the driver.
Signed-off-by: Benjamin Gaignard <[email protected]>
---
drivers/media/platform/verisilicon/hantro_v4l2.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index c0d427956210..d8aa42bd4cd4 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -382,13 +382,10 @@ hantro_reset_encoded_fmt(struct hantro_ctx *ctx)
vpu_fmt = hantro_get_default_fmt(ctx, true);
- if (ctx->is_encoder) {
- ctx->vpu_dst_fmt = vpu_fmt;
+ if (ctx->is_encoder)
fmt = &ctx->dst_fmt;
- } else {
- ctx->vpu_src_fmt = vpu_fmt;
+ else
fmt = &ctx->src_fmt;
- }
hantro_reset_fmt(fmt, vpu_fmt);
fmt->width = vpu_fmt->frmsize.min_width;
@@ -408,11 +405,9 @@ hantro_reset_raw_fmt(struct hantro_ctx *ctx)
raw_vpu_fmt = hantro_get_default_fmt(ctx, false);
if (ctx->is_encoder) {
- ctx->vpu_src_fmt = raw_vpu_fmt;
raw_fmt = &ctx->src_fmt;
encoded_fmt = &ctx->dst_fmt;
} else {
- ctx->vpu_dst_fmt = raw_vpu_fmt;
raw_fmt = &ctx->dst_fmt;
encoded_fmt = &ctx->src_fmt;
}
--
2.34.1
On Mon, Feb 20 2023 at 11:48:44 AM +0100, Benjamin Gaignard
<[email protected]> wrote:
> Setting context source and destination formats should only be done
> in hantro_set_fmt_out() and hantro_set_fmt_cap() after check that
> the targeted queue is not busy.
> Remove these calls from hantro_reset_encoded_fmt() and
> hantro_reset_raw_fmt() to clean the driver.
>
> Signed-off-by: Benjamin Gaignard <[email protected]>
Reviewed-by: Ezequiel Garcia <[email protected]>
> ---
> drivers/media/platform/verisilicon/hantro_v4l2.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c
> b/drivers/media/platform/verisilicon/hantro_v4l2.c
> index c0d427956210..d8aa42bd4cd4 100644
> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> @@ -382,13 +382,10 @@ hantro_reset_encoded_fmt(struct hantro_ctx *ctx)
>
> vpu_fmt = hantro_get_default_fmt(ctx, true);
>
> - if (ctx->is_encoder) {
> - ctx->vpu_dst_fmt = vpu_fmt;
> + if (ctx->is_encoder)
> fmt = &ctx->dst_fmt;
> - } else {
> - ctx->vpu_src_fmt = vpu_fmt;
> + else
> fmt = &ctx->src_fmt;
> - }
>
> hantro_reset_fmt(fmt, vpu_fmt);
> fmt->width = vpu_fmt->frmsize.min_width;
> @@ -408,11 +405,9 @@ hantro_reset_raw_fmt(struct hantro_ctx *ctx)
> raw_vpu_fmt = hantro_get_default_fmt(ctx, false);
>
> if (ctx->is_encoder) {
> - ctx->vpu_src_fmt = raw_vpu_fmt;
> raw_fmt = &ctx->src_fmt;
> encoded_fmt = &ctx->dst_fmt;
> } else {
> - ctx->vpu_dst_fmt = raw_vpu_fmt;
> raw_fmt = &ctx->dst_fmt;
> encoded_fmt = &ctx->src_fmt;
> }
> --
> 2.34.1
>
Hi,
On 20.02.2023 11:48, Benjamin Gaignard wrote:
> Setting context source and destination formats should only be done
> in hantro_set_fmt_out() and hantro_set_fmt_cap() after check that
> the targeted queue is not busy.
> Remove these calls from hantro_reset_encoded_fmt() and
> hantro_reset_raw_fmt() to clean the driver.
>
> Signed-off-by: Benjamin Gaignard <[email protected]>
This patch landed recently in linux-next as commit db6f68b51e5c ("media:
verisilicon: Do not set context src/dst formats in reset functions").
Unfortunately it causes the following regression during Debian boot on
Odroid-M1 board:
--->8---
hantro-vpu fdea0000.video-codec: Adding to iommu group 0
hantro-vpu fdea0000.video-codec: registered rockchip,rk3568-vpu-dec as
/dev/video0
hantro-vpu fdee0000.video-codec: Adding to iommu group 1
hantro-vpu fdee0000.video-codec: registered rockchip,rk3568-vepu-enc as
/dev/video1
Unable to handle kernel NULL pointer dereference at virtual address
0000000000000008
Mem abort info:
ESR = 0x0000000096000004
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x04: level 0 translation fault
Data abort info:
ISV = 0, ISS = 0x00000004
CM = 0, WnR = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=00000001f446f000
[0000000000000008] pgd=0000000000000000, p4d=0000000000000000
Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
Modules linked in: hantro_vpu v4l2_vp9 v4l2_h264 v4l2_mem2mem
videobuf2_dma_contig snd_soc_simple_card display_connector
snd_soc_simple_card_utils videobuf2_memops crct10dif_ce dwmac_rk
rockchip_thermal videobuf2_v4l2 stmmac_platform rockchip_saradc
industrialio_triggered_buffer kfifo_buf stmmac videodev pcs_xpcs
rtc_rk808 videobuf2_common rockchipdrm panfrost mc drm_shmem_helper
analogix_dp gpu_sched dw_mipi_dsi dw_hdmi drm_display_helper ip_tables
x_tables ipv6
CPU: 3 PID: 171 Comm: v4l_id Not tainted 6.3.0-rc2+ #13478
Hardware name: Hardkernel ODROID-M1 (DT)
pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : hantro_try_fmt+0xb4/0x280 [hantro_vpu]
lr : hantro_try_fmt+0xa8/0x280 [hantro_vpu]
...
Call trace:
hantro_try_fmt+0xb4/0x280 [hantro_vpu]
hantro_set_fmt_out+0x3c/0x278 [hantro_vpu]
hantro_reset_raw_fmt+0x94/0xb4 [hantro_vpu]
hantro_set_fmt_cap+0x23c/0x250 [hantro_vpu]
hantro_reset_fmts+0x94/0xcc [hantro_vpu]
hantro_open+0xd4/0x20c [hantro_vpu]
v4l2_open+0x80/0x120 [videodev]
chrdev_open+0xc0/0x22c
do_dentry_open+0x13c/0x490
vfs_open+0x2c/0x38
path_openat+0x550/0x938
do_filp_open+0x80/0x12c
do_sys_openat2+0xb4/0x16c
__arm64_sys_openat+0x64/0xac
invoke_syscall+0x48/0x114
el0_svc_common.constprop.0+0xfc/0x11c
do_el0_svc+0x38/0xa4
el0_svc+0x48/0xb8
el0t_64_sync_handler+0xb8/0xbc
el0t_64_sync+0x190/0x194
Code: 97fe726c f940aa80 52864a61 72a686c1 (b9400800)
---[ end trace 0000000000000000 ]---
I know that v4l_id tool, which is a part of systemd/udev, is known to
crash badly on various vendor kernels (fixing this would be a really
hard, especially assuming the brokenness of some vendor hacks), but I
hoped that at least it should not be able to crash the mainline kernel.
> ---
> drivers/media/platform/verisilicon/hantro_v4l2.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
> index c0d427956210..d8aa42bd4cd4 100644
> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> @@ -382,13 +382,10 @@ hantro_reset_encoded_fmt(struct hantro_ctx *ctx)
>
> vpu_fmt = hantro_get_default_fmt(ctx, true);
>
> - if (ctx->is_encoder) {
> - ctx->vpu_dst_fmt = vpu_fmt;
> + if (ctx->is_encoder)
> fmt = &ctx->dst_fmt;
> - } else {
> - ctx->vpu_src_fmt = vpu_fmt;
> + else
> fmt = &ctx->src_fmt;
> - }
>
> hantro_reset_fmt(fmt, vpu_fmt);
> fmt->width = vpu_fmt->frmsize.min_width;
> @@ -408,11 +405,9 @@ hantro_reset_raw_fmt(struct hantro_ctx *ctx)
> raw_vpu_fmt = hantro_get_default_fmt(ctx, false);
>
> if (ctx->is_encoder) {
> - ctx->vpu_src_fmt = raw_vpu_fmt;
> raw_fmt = &ctx->src_fmt;
> encoded_fmt = &ctx->dst_fmt;
> } else {
> - ctx->vpu_dst_fmt = raw_vpu_fmt;
> raw_fmt = &ctx->dst_fmt;
> encoded_fmt = &ctx->src_fmt;
> }
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Le 12/04/2023 à 18:14, Marek Szyprowski a écrit :
> Hi,
>
> On 20.02.2023 11:48, Benjamin Gaignard wrote:
>> Setting context source and destination formats should only be done
>> in hantro_set_fmt_out() and hantro_set_fmt_cap() after check that
>> the targeted queue is not busy.
>> Remove these calls from hantro_reset_encoded_fmt() and
>> hantro_reset_raw_fmt() to clean the driver.
>>
>> Signed-off-by: Benjamin Gaignard <[email protected]>
> This patch landed recently in linux-next as commit db6f68b51e5c ("media:
> verisilicon: Do not set context src/dst formats in reset functions").
Hi,
I do not have this board up and running with Hantro encoder but
I think the attached patch may solve the issue.
Could you tell me if it works ?
Regards,
Benjamin
>
> Unfortunately it causes the following regression during Debian boot on
> Odroid-M1 board:
>
> --->8---
>
> hantro-vpu fdea0000.video-codec: Adding to iommu group 0
> hantro-vpu fdea0000.video-codec: registered rockchip,rk3568-vpu-dec as
> /dev/video0
> hantro-vpu fdee0000.video-codec: Adding to iommu group 1
> hantro-vpu fdee0000.video-codec: registered rockchip,rk3568-vepu-enc as
> /dev/video1
> Unable to handle kernel NULL pointer dereference at virtual address
> 0000000000000008
> Mem abort info:
> ESR = 0x0000000096000004
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x04: level 0 translation fault
> Data abort info:
> ISV = 0, ISS = 0x00000004
> CM = 0, WnR = 0
> user pgtable: 4k pages, 48-bit VAs, pgdp=00000001f446f000
> [0000000000000008] pgd=0000000000000000, p4d=0000000000000000
> Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
> Modules linked in: hantro_vpu v4l2_vp9 v4l2_h264 v4l2_mem2mem
> videobuf2_dma_contig snd_soc_simple_card display_connector
> snd_soc_simple_card_utils videobuf2_memops crct10dif_ce dwmac_rk
> rockchip_thermal videobuf2_v4l2 stmmac_platform rockchip_saradc
> industrialio_triggered_buffer kfifo_buf stmmac videodev pcs_xpcs
> rtc_rk808 videobuf2_common rockchipdrm panfrost mc drm_shmem_helper
> analogix_dp gpu_sched dw_mipi_dsi dw_hdmi drm_display_helper ip_tables
> x_tables ipv6
> CPU: 3 PID: 171 Comm: v4l_id Not tainted 6.3.0-rc2+ #13478
> Hardware name: Hardkernel ODROID-M1 (DT)
> pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : hantro_try_fmt+0xb4/0x280 [hantro_vpu]
> lr : hantro_try_fmt+0xa8/0x280 [hantro_vpu]
> ...
> Call trace:
> hantro_try_fmt+0xb4/0x280 [hantro_vpu]
> hantro_set_fmt_out+0x3c/0x278 [hantro_vpu]
> hantro_reset_raw_fmt+0x94/0xb4 [hantro_vpu]
> hantro_set_fmt_cap+0x23c/0x250 [hantro_vpu]
> hantro_reset_fmts+0x94/0xcc [hantro_vpu]
> hantro_open+0xd4/0x20c [hantro_vpu]
> v4l2_open+0x80/0x120 [videodev]
> chrdev_open+0xc0/0x22c
> do_dentry_open+0x13c/0x490
> vfs_open+0x2c/0x38
> path_openat+0x550/0x938
> do_filp_open+0x80/0x12c
> do_sys_openat2+0xb4/0x16c
> __arm64_sys_openat+0x64/0xac
> invoke_syscall+0x48/0x114
> el0_svc_common.constprop.0+0xfc/0x11c
> do_el0_svc+0x38/0xa4
> el0_svc+0x48/0xb8
> el0t_64_sync_handler+0xb8/0xbc
> el0t_64_sync+0x190/0x194
> Code: 97fe726c f940aa80 52864a61 72a686c1 (b9400800)
> ---[ end trace 0000000000000000 ]---
>
> I know that v4l_id tool, which is a part of systemd/udev, is known to
> crash badly on various vendor kernels (fixing this would be a really
> hard, especially assuming the brokenness of some vendor hacks), but I
> hoped that at least it should not be able to crash the mainline kernel.
>
>
>> ---
>> drivers/media/platform/verisilicon/hantro_v4l2.c | 9 ++-------
>> 1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
>> index c0d427956210..d8aa42bd4cd4 100644
>> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
>> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
>> @@ -382,13 +382,10 @@ hantro_reset_encoded_fmt(struct hantro_ctx *ctx)
>>
>> vpu_fmt = hantro_get_default_fmt(ctx, true);
>>
>> - if (ctx->is_encoder) {
>> - ctx->vpu_dst_fmt = vpu_fmt;
>> + if (ctx->is_encoder)
>> fmt = &ctx->dst_fmt;
>> - } else {
>> - ctx->vpu_src_fmt = vpu_fmt;
>> + else
>> fmt = &ctx->src_fmt;
>> - }
>>
>> hantro_reset_fmt(fmt, vpu_fmt);
>> fmt->width = vpu_fmt->frmsize.min_width;
>> @@ -408,11 +405,9 @@ hantro_reset_raw_fmt(struct hantro_ctx *ctx)
>> raw_vpu_fmt = hantro_get_default_fmt(ctx, false);
>>
>> if (ctx->is_encoder) {
>> - ctx->vpu_src_fmt = raw_vpu_fmt;
>> raw_fmt = &ctx->src_fmt;
>> encoded_fmt = &ctx->dst_fmt;
>> } else {
>> - ctx->vpu_dst_fmt = raw_vpu_fmt;
>> raw_fmt = &ctx->dst_fmt;
>> encoded_fmt = &ctx->src_fmt;
>> }
> Best regards
Hi,
On 12.04.2023 18:40, Benjamin Gaignard wrote:
>
> Le 12/04/2023 à 18:14, Marek Szyprowski a écrit :
>> Hi,
>>
>> On 20.02.2023 11:48, Benjamin Gaignard wrote:
>>> Setting context source and destination formats should only be done
>>> in hantro_set_fmt_out() and hantro_set_fmt_cap() after check that
>>> the targeted queue is not busy.
>>> Remove these calls from hantro_reset_encoded_fmt() and
>>> hantro_reset_raw_fmt() to clean the driver.
>>>
>>> Signed-off-by: Benjamin Gaignard <[email protected]>
>> This patch landed recently in linux-next as commit db6f68b51e5c ("media:
>> verisilicon: Do not set context src/dst formats in reset functions").
>
> Hi,
>
> I do not have this board up and running with Hantro encoder but
> I think the attached patch may solve the issue.
> Could you tell me if it works ?
Yep, it fixes the issue.
Tested-by: Marek Szyprowski <[email protected]>
It looks that the code could be a bit more cleaned up, as with the
attached patch, there is such construction:
if (coded) {
pix_mp->num_planes = 1;
vpu_fmt = fmt;
} else if (ctx->is_encoder) {
vpu_fmt = fmt;
} else {
vpu_fmt = fmt;
/*
* Width/height on the CAPTURE end of a decoder are
ignored and
* replaced by the OUTPUT ones.
*/
pix_mp->width = ctx->src_fmt.width;
pix_mp->height = ctx->src_fmt.height;
}
Common 'vpu_fmt = fmt' can be moved out of the above if-else block.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi Benjamin,
On 20/02/23 16:18, Benjamin Gaignard wrote:
> Setting context source and destination formats should only be done
> in hantro_set_fmt_out() and hantro_set_fmt_cap() after check that
> the targeted queue is not busy.
> Remove these calls from hantro_reset_encoded_fmt() and
> hantro_reset_raw_fmt() to clean the driver.
>
> Signed-off-by: Benjamin Gaignard <[email protected]>
KernelCI found this patch causes a regression in the
baseline.dmesg.alert test [1] on rk3399-rock-pi-4b [2],
see the bisection report for more details [3].
Let us know if you have any questions.
[1] https://github.com/kernelci/kernelci-core/blob/main/config/rootfs/debos/overlays/baseline/opt/kernelci/dmesg.sh
[2] https://linux.kernelci.org/test/case/id/6442e825f19134d74c2e865d/
[3] https://groups.io/g/kernelci-results/message/40740
Thanks,
Shreeya Patel
#regzbot introduced: db6f68b51e5c
> ---
> drivers/media/platform/verisilicon/hantro_v4l2.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
> index c0d427956210..d8aa42bd4cd4 100644
> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> @@ -382,13 +382,10 @@ hantro_reset_encoded_fmt(struct hantro_ctx *ctx)
>
> vpu_fmt = hantro_get_default_fmt(ctx, true);
>
> - if (ctx->is_encoder) {
> - ctx->vpu_dst_fmt = vpu_fmt;
> + if (ctx->is_encoder)
> fmt = &ctx->dst_fmt;
> - } else {
> - ctx->vpu_src_fmt = vpu_fmt;
> + else
> fmt = &ctx->src_fmt;
> - }
>
> hantro_reset_fmt(fmt, vpu_fmt);
> fmt->width = vpu_fmt->frmsize.min_width;
> @@ -408,11 +405,9 @@ hantro_reset_raw_fmt(struct hantro_ctx *ctx)
> raw_vpu_fmt = hantro_get_default_fmt(ctx, false);
>
> if (ctx->is_encoder) {
> - ctx->vpu_src_fmt = raw_vpu_fmt;
> raw_fmt = &ctx->src_fmt;
> encoded_fmt = &ctx->dst_fmt;
> } else {
> - ctx->vpu_dst_fmt = raw_vpu_fmt;
> raw_fmt = &ctx->dst_fmt;
> encoded_fmt = &ctx->src_fmt;
> }
On 27.04.23 00:19, Shreeya Patel wrote:
> On 20/02/23 16:18, Benjamin Gaignard wrote:
>> Setting context source and destination formats should only be done
>> in hantro_set_fmt_out() and hantro_set_fmt_cap() after check that
>> the targeted queue is not busy.
>> Remove these calls from hantro_reset_encoded_fmt() and
>> hantro_reset_raw_fmt() to clean the driver.
>>
>> Signed-off-by: Benjamin Gaignard <[email protected]>
>
> KernelCI found this patch causes a regression in the
> baseline.dmesg.alert test [1] on rk3399-rock-pi-4b [2],
> see the bisection report for more details [3].
>
> Let us know if you have any questions.
>
>
> [1]
> https://github.com/kernelci/kernelci-core/blob/main/config/rootfs/debos/overlays/baseline/opt/kernelci/dmesg.sh
> [2] https://linux.kernelci.org/test/case/id/6442e825f19134d74c2e865d/
> [3] https://groups.io/g/kernelci-results/message/40740
Thx for the report. FWIW, regzbot noticed there is a patch that refers
to the culprit that might have been landed in next after your test ran
for the last time (and meanwhile it was mainlined): f100ce3bbd6 ("media:
verisilicon: Fix crash when probing encoder")
I wonder if that is related or might even fix the issue.
Ciao, Thorsten
>> ---
>> drivers/media/platform/verisilicon/hantro_v4l2.c | 9 ++-------
>> 1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c
>> b/drivers/media/platform/verisilicon/hantro_v4l2.c
>> index c0d427956210..d8aa42bd4cd4 100644
>> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
>> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
>> @@ -382,13 +382,10 @@ hantro_reset_encoded_fmt(struct hantro_ctx *ctx)
>> vpu_fmt = hantro_get_default_fmt(ctx, true);
>> - if (ctx->is_encoder) {
>> - ctx->vpu_dst_fmt = vpu_fmt;
>> + if (ctx->is_encoder)
>> fmt = &ctx->dst_fmt;
>> - } else {
>> - ctx->vpu_src_fmt = vpu_fmt;
>> + else
>> fmt = &ctx->src_fmt;
>> - }
>> hantro_reset_fmt(fmt, vpu_fmt);
>> fmt->width = vpu_fmt->frmsize.min_width;
>> @@ -408,11 +405,9 @@ hantro_reset_raw_fmt(struct hantro_ctx *ctx)
>> raw_vpu_fmt = hantro_get_default_fmt(ctx, false);
>> if (ctx->is_encoder) {
>> - ctx->vpu_src_fmt = raw_vpu_fmt;
>> raw_fmt = &ctx->src_fmt;
>> encoded_fmt = &ctx->dst_fmt;
>> } else {
>> - ctx->vpu_dst_fmt = raw_vpu_fmt;
>> raw_fmt = &ctx->dst_fmt;
>> encoded_fmt = &ctx->src_fmt;
>> }
>
>
Le 01/05/2023 à 09:21, Thorsten Leemhuis a écrit :
> On 27.04.23 00:19, Shreeya Patel wrote:
>> On 20/02/23 16:18, Benjamin Gaignard wrote:
>>> Setting context source and destination formats should only be done
>>> in hantro_set_fmt_out() and hantro_set_fmt_cap() after check that
>>> the targeted queue is not busy.
>>> Remove these calls from hantro_reset_encoded_fmt() and
>>> hantro_reset_raw_fmt() to clean the driver.
>>>
>>> Signed-off-by: Benjamin Gaignard <[email protected]>
>> KernelCI found this patch causes a regression in the
>> baseline.dmesg.alert test [1] on rk3399-rock-pi-4b [2],
>> see the bisection report for more details [3].
>>
>> Let us know if you have any questions.
>>
>>
>> [1]
>> https://github.com/kernelci/kernelci-core/blob/main/config/rootfs/debos/overlays/baseline/opt/kernelci/dmesg.sh
>> [2] https://linux.kernelci.org/test/case/id/6442e825f19134d74c2e865d/
>> [3] https://groups.io/g/kernelci-results/message/40740
> Thx for the report. FWIW, regzbot noticed there is a patch that refers
> to the culprit that might have been landed in next after your test ran
> for the last time (and meanwhile it was mainlined): f100ce3bbd6 ("media:
> verisilicon: Fix crash when probing encoder")
Yes that patch should fix the probing issue.
Marek is working on an additional one to fix pixel format negotiation
but that doesn't impact the boot.
Regards,
Benjamin
>
> I wonder if that is related or might even fix the issue.
>
> Ciao, Thorsten
>
>>> ---
>>> drivers/media/platform/verisilicon/hantro_v4l2.c | 9 ++-------
>>> 1 file changed, 2 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c
>>> b/drivers/media/platform/verisilicon/hantro_v4l2.c
>>> index c0d427956210..d8aa42bd4cd4 100644
>>> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
>>> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
>>> @@ -382,13 +382,10 @@ hantro_reset_encoded_fmt(struct hantro_ctx *ctx)
>>> vpu_fmt = hantro_get_default_fmt(ctx, true);
>>> - if (ctx->is_encoder) {
>>> - ctx->vpu_dst_fmt = vpu_fmt;
>>> + if (ctx->is_encoder)
>>> fmt = &ctx->dst_fmt;
>>> - } else {
>>> - ctx->vpu_src_fmt = vpu_fmt;
>>> + else
>>> fmt = &ctx->src_fmt;
>>> - }
>>> hantro_reset_fmt(fmt, vpu_fmt);
>>> fmt->width = vpu_fmt->frmsize.min_width;
>>> @@ -408,11 +405,9 @@ hantro_reset_raw_fmt(struct hantro_ctx *ctx)
>>> raw_vpu_fmt = hantro_get_default_fmt(ctx, false);
>>> if (ctx->is_encoder) {
>>> - ctx->vpu_src_fmt = raw_vpu_fmt;
>>> raw_fmt = &ctx->src_fmt;
>>> encoded_fmt = &ctx->dst_fmt;
>>> } else {
>>> - ctx->vpu_dst_fmt = raw_vpu_fmt;
>>> raw_fmt = &ctx->dst_fmt;
>>> encoded_fmt = &ctx->src_fmt;
>>> }
>>
On 02.05.23 08:56, Benjamin Gaignard wrote:
> Le 01/05/2023 à 09:21, Thorsten Leemhuis a écrit :
>> On 27.04.23 00:19, Shreeya Patel wrote:
>>> On 20/02/23 16:18, Benjamin Gaignard wrote:
>>>> Setting context source and destination formats should only be done
>>>> in hantro_set_fmt_out() and hantro_set_fmt_cap() after check that
>>>> the targeted queue is not busy.
>>>> Remove these calls from hantro_reset_encoded_fmt() and
>>>> hantro_reset_raw_fmt() to clean the driver.
>>>>
>>>> Signed-off-by: Benjamin Gaignard <[email protected]>
>>> KernelCI found this patch causes a regression in the
>>> baseline.dmesg.alert test [1] on rk3399-rock-pi-4b [2],
>>> see the bisection report for more details [3].
>>>
>>> Let us know if you have any questions.
>>>
>>> [1]
>>> https://github.com/kernelci/kernelci-core/blob/main/config/rootfs/debos/overlays/baseline/opt/kernelci/dmesg.sh
>>> [2] https://linux.kernelci.org/test/case/id/6442e825f19134d74c2e865d/
>>> [3] https://groups.io/g/kernelci-results/message/40740
>> Thx for the report. FWIW, regzbot noticed there is a patch that refers
>> to the culprit that might have been landed in next after your test ran
>> for the last time (and meanwhile it was mainlined): f100ce3bbd6 ("media:
>> verisilicon: Fix crash when probing encoder")
>
> Yes that patch should fix the probing issue.
> Marek is working on an additional one to fix pixel format negotiation
> but that doesn't impact the boot.
Great, thx for the reply.
Shreeya, normally I believe developers in cases like this and would have
included
#regzbot fix: f100ce3bbd6
in this mail (without the space in front of the #) to mark the
regression as resolved. Would that be okay for you and other kernel.ci
people? Or do you want to confirm this first?
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
Hi Thorsten,
On 02/05/23 16:42, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 02.05.23 08:56, Benjamin Gaignard wrote:
>> Le 01/05/2023 à 09:21, Thorsten Leemhuis a écrit :
>>> On 27.04.23 00:19, Shreeya Patel wrote:
>>>> On 20/02/23 16:18, Benjamin Gaignard wrote:
>>>>> Setting context source and destination formats should only be done
>>>>> in hantro_set_fmt_out() and hantro_set_fmt_cap() after check that
>>>>> the targeted queue is not busy.
>>>>> Remove these calls from hantro_reset_encoded_fmt() and
>>>>> hantro_reset_raw_fmt() to clean the driver.
>>>>>
>>>>> Signed-off-by: Benjamin Gaignard <[email protected]>
>>>> KernelCI found this patch causes a regression in the
>>>> baseline.dmesg.alert test [1] on rk3399-rock-pi-4b [2],
>>>> see the bisection report for more details [3].
>>>>
>>>> Let us know if you have any questions.
>>>>
>>>> [1]
>>>> https://github.com/kernelci/kernelci-core/blob/main/config/rootfs/debos/overlays/baseline/opt/kernelci/dmesg.sh
>>>> [2] https://linux.kernelci.org/test/case/id/6442e825f19134d74c2e865d/
>>>> [3] https://groups.io/g/kernelci-results/message/40740
>>> Thx for the report. FWIW, regzbot noticed there is a patch that refers
>>> to the culprit that might have been landed in next after your test ran
>>> for the last time (and meanwhile it was mainlined): f100ce3bbd6 ("media:
>>> verisilicon: Fix crash when probing encoder")
>> Yes that patch should fix the probing issue.
>> Marek is working on an additional one to fix pixel format negotiation
>> but that doesn't impact the boot.
> Great, thx for the reply.
>
> Shreeya, normally I believe developers in cases like this and would have
> included
>
> #regzbot fix: f100ce3bbd6
>
> in this mail (without the space in front of the #) to mark the
> regression as resolved. Would that be okay for you and other kernel.ci
> people? Or do you want to confirm this first?
I checked the commit pointed out and also double checked with Benjamin
internally so
we are good to mark this as fixed.
#regzbot fix: f100ce3bbd6a
Thanks,
Shreeya Patel
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.