drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++ drivers/staging/media/sunxi/cedrus/cedrus_h265.c | 15 ++++++++++++--- drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 7 ++++++- drivers/staging/media/sunxi/cedrus/cedrus_hw.h | 2 ++ 4 files changed, 22 insertions(+), 4 deletions(-)
The cedrus_h265_skip_bits() may get into infinite loop if decoding
parameters are incorrect. In this case we detect the loop and print
a error message, continuing the decoding that is fated to fail.
Will be cleaner to abort the decoding early. Propagate the error code
to cedrus_device_run() and reset hardware on the cedrus_h265_skip_bits()
failure.
Suggested-by: Jernej Škrabec <jernej.skrabec@gmail.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
drivers/staging/media/sunxi/cedrus/cedrus_h265.c | 15 ++++++++++++---
drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 7 ++++++-
drivers/staging/media/sunxi/cedrus/cedrus_hw.h | 2 ++
4 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
index e7f7602a5ab4..ae5df3dc01c0 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
@@ -112,6 +112,8 @@ void cedrus_device_run(void *priv)
dev->dec_ops[ctx->current_codec]->trigger(ctx);
} else {
+ cedrus_hw_reset(dev);
+
v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev,
ctx->fh.m2m_ctx,
VB2_BUF_STATE_ERROR);
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
index 4952fc17f3e6..f409f59452d8 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
@@ -224,9 +224,10 @@ static void cedrus_h265_pred_weight_write(struct cedrus_dev *dev,
}
}
-static void cedrus_h265_skip_bits(struct cedrus_dev *dev, int num)
+static int cedrus_h265_skip_bits(struct cedrus_dev *dev, int num)
{
int count = 0;
+ int err;
while (count < num) {
int tmp = min(num - count, 32);
@@ -235,11 +236,16 @@ static void cedrus_h265_skip_bits(struct cedrus_dev *dev, int num)
VE_DEC_H265_TRIGGER_FLUSH_BITS |
VE_DEC_H265_TRIGGER_TYPE_N_BITS(tmp));
- if (cedrus_wait_for(dev, VE_DEC_H265_STATUS, VE_DEC_H265_STATUS_VLD_BUSY))
+ err = cedrus_wait_for(dev, VE_DEC_H265_STATUS, VE_DEC_H265_STATUS_VLD_BUSY);
+ if (err) {
dev_err_ratelimited(dev->dev, "timed out waiting to skip bits\n");
+ return err;
+ }
count += tmp;
}
+
+ return 0;
}
static void cedrus_h265_write_scaling_list(struct cedrus_ctx *ctx,
@@ -408,6 +414,7 @@ static int cedrus_h265_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
u32 pic_order_cnt[2];
u8 *padding;
int count;
+ int err;
u32 reg;
sps = run->h265.sps;
@@ -534,7 +541,9 @@ static int cedrus_h265_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
/* Include the one bit. */
count++;
- cedrus_h265_skip_bits(dev, slice_params->data_byte_offset * 8 - count);
+ err = cedrus_h265_skip_bits(dev, slice_params->data_byte_offset * 8 - count);
+ if (err)
+ return err;
/* Bitstream parameters. */
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
index a6470a89851e..e9ceca332062 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
@@ -168,11 +168,16 @@ void cedrus_watchdog(struct work_struct *work)
VB2_BUF_STATE_ERROR);
}
+void cedrus_hw_reset(struct cedrus_dev *dev)
+{
+ reset_control_reset(dev->rstc);
+}
+
int cedrus_hw_suspend(struct device *device)
{
struct cedrus_dev *dev = dev_get_drvdata(device);
- reset_control_assert(dev->rstc);
+ cedrus_hw_reset(dev);
clk_disable_unprepare(dev->ram_clk);
clk_disable_unprepare(dev->mod_clk);
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
index 7c92f00e36da..919c4475f0d7 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
@@ -30,4 +30,6 @@ void cedrus_hw_remove(struct cedrus_dev *dev);
void cedrus_watchdog(struct work_struct *work);
+void cedrus_hw_reset(struct cedrus_dev *dev);
+
#endif
--
2.37.3
Hi Dmitry,
This patch has a conflict with this patch from Jernej:
https://patchwork.linuxtv.org/project/linux-media/patch/20221017194413.1198301-1-jernej.skrabec@gmail.com/
I decided to take Jernej's patch first. Can you make a v2 that sits on top of
that patch?
Thanks!
Hans
On 9/14/22 17:01, Dmitry Osipenko wrote:
> The cedrus_h265_skip_bits() may get into infinite loop if decoding
> parameters are incorrect. In this case we detect the loop and print
> a error message, continuing the decoding that is fated to fail.
>
> Will be cleaner to abort the decoding early. Propagate the error code
> to cedrus_device_run() and reset hardware on the cedrus_h265_skip_bits()
> failure.
>
> Suggested-by: Jernej Škrabec <jernej.skrabec@gmail.com>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
> drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
> drivers/staging/media/sunxi/cedrus/cedrus_h265.c | 15 ++++++++++++---
> drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 7 ++++++-
> drivers/staging/media/sunxi/cedrus/cedrus_hw.h | 2 ++
> 4 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> index e7f7602a5ab4..ae5df3dc01c0 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> @@ -112,6 +112,8 @@ void cedrus_device_run(void *priv)
>
> dev->dec_ops[ctx->current_codec]->trigger(ctx);
> } else {
> + cedrus_hw_reset(dev);
> +
> v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev,
> ctx->fh.m2m_ctx,
> VB2_BUF_STATE_ERROR);
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> index 4952fc17f3e6..f409f59452d8 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> @@ -224,9 +224,10 @@ static void cedrus_h265_pred_weight_write(struct cedrus_dev *dev,
> }
> }
>
> -static void cedrus_h265_skip_bits(struct cedrus_dev *dev, int num)
> +static int cedrus_h265_skip_bits(struct cedrus_dev *dev, int num)
> {
> int count = 0;
> + int err;
>
> while (count < num) {
> int tmp = min(num - count, 32);
> @@ -235,11 +236,16 @@ static void cedrus_h265_skip_bits(struct cedrus_dev *dev, int num)
> VE_DEC_H265_TRIGGER_FLUSH_BITS |
> VE_DEC_H265_TRIGGER_TYPE_N_BITS(tmp));
>
> - if (cedrus_wait_for(dev, VE_DEC_H265_STATUS, VE_DEC_H265_STATUS_VLD_BUSY))
> + err = cedrus_wait_for(dev, VE_DEC_H265_STATUS, VE_DEC_H265_STATUS_VLD_BUSY);
> + if (err) {
> dev_err_ratelimited(dev->dev, "timed out waiting to skip bits\n");
> + return err;
> + }
>
> count += tmp;
> }
> +
> + return 0;
> }
>
> static void cedrus_h265_write_scaling_list(struct cedrus_ctx *ctx,
> @@ -408,6 +414,7 @@ static int cedrus_h265_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
> u32 pic_order_cnt[2];
> u8 *padding;
> int count;
> + int err;
> u32 reg;
>
> sps = run->h265.sps;
> @@ -534,7 +541,9 @@ static int cedrus_h265_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
> /* Include the one bit. */
> count++;
>
> - cedrus_h265_skip_bits(dev, slice_params->data_byte_offset * 8 - count);
> + err = cedrus_h265_skip_bits(dev, slice_params->data_byte_offset * 8 - count);
> + if (err)
> + return err;
>
> /* Bitstream parameters. */
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> index a6470a89851e..e9ceca332062 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> @@ -168,11 +168,16 @@ void cedrus_watchdog(struct work_struct *work)
> VB2_BUF_STATE_ERROR);
> }
>
> +void cedrus_hw_reset(struct cedrus_dev *dev)
> +{
> + reset_control_reset(dev->rstc);
> +}
> +
> int cedrus_hw_suspend(struct device *device)
> {
> struct cedrus_dev *dev = dev_get_drvdata(device);
>
> - reset_control_assert(dev->rstc);
> + cedrus_hw_reset(dev);
>
> clk_disable_unprepare(dev->ram_clk);
> clk_disable_unprepare(dev->mod_clk);
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
> index 7c92f00e36da..919c4475f0d7 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
> @@ -30,4 +30,6 @@ void cedrus_hw_remove(struct cedrus_dev *dev);
>
> void cedrus_watchdog(struct work_struct *work);
>
> +void cedrus_hw_reset(struct cedrus_dev *dev);
> +
> #endif
Hi Hans,
Dne ponedeljek, 24. oktober 2022 ob 13:38:36 CEST je Hans Verkuil napisal(a):
> Hi Dmitry,
>
> This patch has a conflict with this patch from Jernej:
>
> https://patchwork.linuxtv.org/project/linux-media/patch/20221017194413.11983
> 01-1-jernej.skrabec@gmail.com/
>
> I decided to take Jernej's patch first. Can you make a v2 that sits on top
> of that patch?
I believe you already merged first version of the patch for 6.1. No need for
this version though, first version already solves main issue.
Best regards,
Jernej
>
> Thanks!
>
> Hans
>
> On 9/14/22 17:01, Dmitry Osipenko wrote:
> > The cedrus_h265_skip_bits() may get into infinite loop if decoding
> > parameters are incorrect. In this case we detect the loop and print
> > a error message, continuing the decoding that is fated to fail.
> >
> > Will be cleaner to abort the decoding early. Propagate the error code
> > to cedrus_device_run() and reset hardware on the cedrus_h265_skip_bits()
> > failure.
> >
> > Suggested-by: Jernej Škrabec <jernej.skrabec@gmail.com>
> > Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > ---
> >
> > drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
> > drivers/staging/media/sunxi/cedrus/cedrus_h265.c | 15 ++++++++++++---
> > drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 7 ++++++-
> > drivers/staging/media/sunxi/cedrus/cedrus_hw.h | 2 ++
> > 4 files changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c index
> > e7f7602a5ab4..ae5df3dc01c0 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > @@ -112,6 +112,8 @@ void cedrus_device_run(void *priv)
> >
> > dev->dec_ops[ctx->current_codec]->trigger(ctx);
> >
> > } else {
> >
> > + cedrus_hw_reset(dev);
> > +
> >
> > v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev,
> >
> > ctx-
>fh.m2m_ctx,
> >
VB2_BUF_STATE_ERROR);
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c index
> > 4952fc17f3e6..f409f59452d8 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> > @@ -224,9 +224,10 @@ static void cedrus_h265_pred_weight_write(struct
> > cedrus_dev *dev,>
> > }
> >
> > }
> >
> > -static void cedrus_h265_skip_bits(struct cedrus_dev *dev, int num)
> > +static int cedrus_h265_skip_bits(struct cedrus_dev *dev, int num)
> >
> > {
> >
> > int count = 0;
> >
> > + int err;
> >
> > while (count < num) {
> >
> > int tmp = min(num - count, 32);
> >
> > @@ -235,11 +236,16 @@ static void cedrus_h265_skip_bits(struct cedrus_dev
> > *dev, int num)>
> > VE_DEC_H265_TRIGGER_FLUSH_BITS |
> > VE_DEC_H265_TRIGGER_TYPE_N_BITS(tmp));
> >
> > - if (cedrus_wait_for(dev, VE_DEC_H265_STATUS,
> > VE_DEC_H265_STATUS_VLD_BUSY)) + err = cedrus_wait_for(dev,
> > VE_DEC_H265_STATUS, VE_DEC_H265_STATUS_VLD_BUSY); + if
(err) {
> >
> > dev_err_ratelimited(dev->dev, "timed out
waiting to skip bits\n");
> >
> > + return err;
> > + }
> >
> > count += tmp;
> >
> > }
> >
> > +
> > + return 0;
> >
> > }
> >
> > static void cedrus_h265_write_scaling_list(struct cedrus_ctx *ctx,
> >
> > @@ -408,6 +414,7 @@ static int cedrus_h265_setup(struct cedrus_ctx *ctx,
> > struct cedrus_run *run)>
> > u32 pic_order_cnt[2];
> > u8 *padding;
> > int count;
> >
> > + int err;
> >
> > u32 reg;
> >
> > sps = run->h265.sps;
> >
> > @@ -534,7 +541,9 @@ static int cedrus_h265_setup(struct cedrus_ctx *ctx,
> > struct cedrus_run *run)>
> > /* Include the one bit. */
> > count++;
> >
> > - cedrus_h265_skip_bits(dev, slice_params->data_byte_offset * 8 -
count);
> > + err = cedrus_h265_skip_bits(dev, slice_params->data_byte_offset * 8
-
> > count); + if (err)
> > + return err;
> >
> > /* Bitstream parameters. */
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c index
> > a6470a89851e..e9ceca332062 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> > @@ -168,11 +168,16 @@ void cedrus_watchdog(struct work_struct *work)
> >
> >
VB2_BUF_STATE_ERROR);
> >
> > }
> >
> > +void cedrus_hw_reset(struct cedrus_dev *dev)
> > +{
> > + reset_control_reset(dev->rstc);
> > +}
> > +
> >
> > int cedrus_hw_suspend(struct device *device)
> > {
> >
> > struct cedrus_dev *dev = dev_get_drvdata(device);
> >
> > - reset_control_assert(dev->rstc);
> > + cedrus_hw_reset(dev);
> >
> > clk_disable_unprepare(dev->ram_clk);
> > clk_disable_unprepare(dev->mod_clk);
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
> > b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h index
> > 7c92f00e36da..919c4475f0d7 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
> > @@ -30,4 +30,6 @@ void cedrus_hw_remove(struct cedrus_dev *dev);
> >
> > void cedrus_watchdog(struct work_struct *work);
> >
> > +void cedrus_hw_reset(struct cedrus_dev *dev);
> > +
> >
> > #endif
On 10/25/22 07:59, Jernej Škrabec wrote: > Dne ponedeljek, 24. oktober 2022 ob 13:38:36 CEST je Hans Verkuil napisal(a): >> Hi Dmitry, >> >> This patch has a conflict with this patch from Jernej: >> >> https://patchwork.linuxtv.org/project/linux-media/patch/20221017194413.11983 >> 01-1-jernej.skrabec@gmail.com/ >> >> I decided to take Jernej's patch first. Can you make a v2 that sits on top >> of that patch? > I believe you already merged first version of the patch for 6.1. No need for > this version though, first version already solves main issue. That was a followup to the first version. -- Best regards, Dmitry
On 10/25/22 11:34, Dmitry Osipenko wrote: > On 10/25/22 07:59, Jernej Škrabec wrote: >> Dne ponedeljek, 24. oktober 2022 ob 13:38:36 CEST je Hans Verkuil napisal(a): >>> Hi Dmitry, >>> >>> This patch has a conflict with this patch from Jernej: >>> >>> https://patchwork.linuxtv.org/project/linux-media/patch/20221017194413.11983 >>> 01-1-jernej.skrabec@gmail.com/ >>> >>> I decided to take Jernej's patch first. Can you make a v2 that sits on top >>> of that patch? >> I believe you already merged first version of the patch for 6.1. No need for >> this version though, first version already solves main issue. > > That was a followup to the first version. > I have no idea what you are both talking about :-) "I believe you already merged first version of the patch for 6.1" Which patch are you referring to? Regards, Hans
On 10/25/22 13:23, Hans Verkuil wrote: > On 10/25/22 11:34, Dmitry Osipenko wrote: >> On 10/25/22 07:59, Jernej Škrabec wrote: >>> Dne ponedeljek, 24. oktober 2022 ob 13:38:36 CEST je Hans Verkuil napisal(a): >>>> Hi Dmitry, >>>> >>>> This patch has a conflict with this patch from Jernej: >>>> >>>> https://patchwork.linuxtv.org/project/linux-media/patch/20221017194413.11983 >>>> 01-1-jernej.skrabec@gmail.com/ >>>> >>>> I decided to take Jernej's patch first. Can you make a v2 that sits on top >>>> of that patch? >>> I believe you already merged first version of the patch for 6.1. No need for >>> this version though, first version already solves main issue. >> >> That was a followup to the first version. >> > > I have no idea what you are both talking about :-) > > "I believe you already merged first version of the patch for 6.1" Which patch > are you referring to? This is about [1] that was already merged. This patch implements the error propagation that Jernej asked [2] when he reviewed [1]. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=91db7a3fc7fe670cf1770a398a43bb4a1f776bf1 [2] https://lore.kernel.org/lkml/1733932.VLH7GnMWUR@kista/ I'll prepare the v2 if this change is still desired by Jernej. -- Best regards, Dmitry
On 10/25/22 12:36, Dmitry Osipenko wrote: > On 10/25/22 13:23, Hans Verkuil wrote: >> On 10/25/22 11:34, Dmitry Osipenko wrote: >>> On 10/25/22 07:59, Jernej Škrabec wrote: >>>> Dne ponedeljek, 24. oktober 2022 ob 13:38:36 CEST je Hans Verkuil napisal(a): >>>>> Hi Dmitry, >>>>> >>>>> This patch has a conflict with this patch from Jernej: >>>>> >>>>> https://patchwork.linuxtv.org/project/linux-media/patch/20221017194413.11983 >>>>> 01-1-jernej.skrabec@gmail.com/ >>>>> >>>>> I decided to take Jernej's patch first. Can you make a v2 that sits on top >>>>> of that patch? >>>> I believe you already merged first version of the patch for 6.1. No need for >>>> this version though, first version already solves main issue. >>> >>> That was a followup to the first version. >>> >> >> I have no idea what you are both talking about :-) >> >> "I believe you already merged first version of the patch for 6.1" Which patch >> are you referring to? > > This is about [1] that was already merged. This patch implements the > error propagation that Jernej asked [2] when he reviewed [1]. > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=91db7a3fc7fe670cf1770a398a43bb4a1f776bf1 > > [2] https://lore.kernel.org/lkml/1733932.VLH7GnMWUR@kista/ > > I'll prepare the v2 if this change is still desired by Jernej. > Ah, OK. That was not obvious to me :-) I'll mark this v1 patch as 'Changes Requested' in patchwork. Thank you for the explanation! Regards, Hans
© 2016 - 2026 Red Hat, Inc.