Re: [PATCH 06/11] media: cedrus: set codec ops immediately

From: Paul Kocialkowski
Date: Tue Oct 25 2022 - 11:00:14 EST

Next message: wangkailong: "[PATCH] ALSA/ASoC: replace ternary operator with min()"
Previous message: Petr Mladek: "Re: [PATCH printk v2 26/38] kdb: use srcu console list iterator"
In reply to: Jernej Skrabec: "[PATCH 06/11] media: cedrus: set codec ops immediately"
Next in thread: Jernej Skrabec: "[PATCH 08/11] media: cedrus: prefer untiled capture format"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Jernej,

On Mon 24 Oct 22, 22:15, Jernej Skrabec wrote:
> We'll need codec ops soon after output format is set in following
> commits. Let's move current codec setup to set output format callback.
> While at it, let's remove one level of indirection by changing
> current_codec to point to codec ops structure directly.

This looks much better, thanks!

Acked-by: Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx>

Cheers,

Paul

> Signed-off-by: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
> ---
> drivers/staging/media/sunxi/cedrus/cedrus.c | 5 ---
> drivers/staging/media/sunxi/cedrus/cedrus.h | 3 +-
> .../staging/media/sunxi/cedrus/cedrus_dec.c | 4 +-
> .../staging/media/sunxi/cedrus/cedrus_hw.c | 6 +--
> .../staging/media/sunxi/cedrus/cedrus_video.c | 44 ++++++++-----------
> 5 files changed, 25 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index 023566b02dc5..8cfe47574c39 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -455,11 +455,6 @@ static int cedrus_probe(struct platform_device *pdev)
> return ret;
> }
>
> - dev->dec_ops[CEDRUS_CODEC_MPEG2] = &cedrus_dec_ops_mpeg2;
> - dev->dec_ops[CEDRUS_CODEC_H264] = &cedrus_dec_ops_h264;
> - dev->dec_ops[CEDRUS_CODEC_H265] = &cedrus_dec_ops_h265;
> - dev->dec_ops[CEDRUS_CODEC_VP8] = &cedrus_dec_ops_vp8;
> -
> mutex_init(&dev->dev_mutex);
>
> INIT_DELAYED_WORK(&dev->watchdog_work, cedrus_watchdog);
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index 7a1619967513..0b082b1fae22 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -126,7 +126,7 @@ struct cedrus_ctx {
>
> struct v4l2_pix_format src_fmt;
> struct v4l2_pix_format dst_fmt;
> - enum cedrus_codec current_codec;
> + struct cedrus_dec_ops *current_codec;
>
> struct v4l2_ctrl_handler hdl;
> struct v4l2_ctrl **ctrls;
> @@ -185,7 +185,6 @@ struct cedrus_dev {
> struct platform_device *pdev;
> struct device *dev;
> struct v4l2_m2m_dev *m2m_dev;
> - struct cedrus_dec_ops *dec_ops[CEDRUS_CODEC_LAST];
>
> /* Device file mutex */
> struct mutex dev_mutex;
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> index e7f7602a5ab4..fbbf9e6f0f50 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> @@ -94,7 +94,7 @@ void cedrus_device_run(void *priv)
>
> cedrus_dst_format_set(dev, &ctx->dst_fmt);
>
> - error = dev->dec_ops[ctx->current_codec]->setup(ctx, &run);
> + error = ctx->current_codec->setup(ctx, &run);
> if (error)
> v4l2_err(&ctx->dev->v4l2_dev,
> "Failed to setup decoding job: %d\n", error);
> @@ -110,7 +110,7 @@ void cedrus_device_run(void *priv)
> schedule_delayed_work(&dev->watchdog_work,
> msecs_to_jiffies(2000));
>
> - dev->dec_ops[ctx->current_codec]->trigger(ctx);
> + ctx->current_codec->trigger(ctx);
> } else {
> v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev,
> ctx->fh.m2m_ctx,
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> index a6470a89851e..c3387cd1e80f 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> @@ -132,12 +132,12 @@ static irqreturn_t cedrus_irq(int irq, void *data)
> return IRQ_NONE;
> }
>
> - status = dev->dec_ops[ctx->current_codec]->irq_status(ctx);
> + status = ctx->current_codec->irq_status(ctx);
> if (status == CEDRUS_IRQ_NONE)
> return IRQ_NONE;
>
> - dev->dec_ops[ctx->current_codec]->irq_disable(ctx);
> - dev->dec_ops[ctx->current_codec]->irq_clear(ctx);
> + ctx->current_codec->irq_disable(ctx);
> + ctx->current_codec->irq_clear(ctx);
>
> if (status == CEDRUS_IRQ_ERROR)
> state = VB2_BUF_STATE_ERROR;
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> index 04b7b87ef0b7..3591bf9d7d9c 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -335,6 +335,21 @@ static int cedrus_s_fmt_vid_out_p(struct cedrus_ctx *ctx,
> break;
> }
>
> + switch (ctx->src_fmt.pixelformat) {
> + case V4L2_PIX_FMT_MPEG2_SLICE:
> + ctx->current_codec = &cedrus_dec_ops_mpeg2;
> + break;
> + case V4L2_PIX_FMT_H264_SLICE:
> + ctx->current_codec = &cedrus_dec_ops_h264;
> + break;
> + case V4L2_PIX_FMT_HEVC_SLICE:
> + ctx->current_codec = &cedrus_dec_ops_h265;
> + break;
> + case V4L2_PIX_FMT_VP8_FRAME:
> + ctx->current_codec = &cedrus_dec_ops_vp8;
> + break;
> + }
> +
> /* Propagate format information to capture. */
> ctx->dst_fmt.colorspace = pix_fmt->colorspace;
> ctx->dst_fmt.xfer_func = pix_fmt->xfer_func;
> @@ -493,34 +508,13 @@ static int cedrus_start_streaming(struct vb2_queue *vq, unsigned int count)
> struct cedrus_dev *dev = ctx->dev;
> int ret = 0;
>
> - switch (ctx->src_fmt.pixelformat) {
> - case V4L2_PIX_FMT_MPEG2_SLICE:
> - ctx->current_codec = CEDRUS_CODEC_MPEG2;
> - break;
> -
> - case V4L2_PIX_FMT_H264_SLICE:
> - ctx->current_codec = CEDRUS_CODEC_H264;
> - break;
> -
> - case V4L2_PIX_FMT_HEVC_SLICE:
> - ctx->current_codec = CEDRUS_CODEC_H265;
> - break;
> -
> - case V4L2_PIX_FMT_VP8_FRAME:
> - ctx->current_codec = CEDRUS_CODEC_VP8;
> - break;
> -
> - default:
> - return -EINVAL;
> - }
> -
> if (V4L2_TYPE_IS_OUTPUT(vq->type)) {
> ret = pm_runtime_resume_and_get(dev->dev);
> if (ret < 0)
> goto err_cleanup;
>
> - if (dev->dec_ops[ctx->current_codec]->start) {
> - ret = dev->dec_ops[ctx->current_codec]->start(ctx);
> + if (ctx->current_codec->start) {
> + ret = ctx->current_codec->start(ctx);
> if (ret)
> goto err_pm;
> }
> @@ -542,8 +536,8 @@ static void cedrus_stop_streaming(struct vb2_queue *vq)
> struct cedrus_dev *dev = ctx->dev;
>
> if (V4L2_TYPE_IS_OUTPUT(vq->type)) {
> - if (dev->dec_ops[ctx->current_codec]->stop)
> - dev->dec_ops[ctx->current_codec]->stop(ctx);
> + if (ctx->current_codec->stop)
> + ctx->current_codec->stop(ctx);
>
> pm_runtime_put(dev->dev);
> }
> --
> 2.38.1
>

--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Attachment: signature.asc
Description: PGP signature

Next message: wangkailong: "[PATCH] ALSA/ASoC: replace ternary operator with min()"
Previous message: Petr Mladek: "Re: [PATCH printk v2 26/38] kdb: use srcu console list iterator"
In reply to: Jernej Skrabec: "[PATCH 06/11] media: cedrus: set codec ops immediately"
Next in thread: Jernej Skrabec: "[PATCH 08/11] media: cedrus: prefer untiled capture format"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]