lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y1f57R7SUPHsS7Lv@aptenodytes>
Date:   Tue, 25 Oct 2022 16:59:57 +0200
From:   Paul Kocialkowski <paul.kocialkowski@...tlin.com>
To:     Jernej Skrabec <jernej.skrabec@...il.com>
Cc:     mripard@...nel.org, mchehab@...nel.org, gregkh@...uxfoundation.org,
        wens@...e.org, samuel@...lland.org, hverkuil-cisco@...all.nl,
        linux-media@...r.kernel.org, linux-staging@...ts.linux.dev,
        linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 06/11] media: cedrus: set codec ops immediately

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@...tlin.com>

Cheers,

Paul

> Signed-off-by: Jernej Skrabec <jernej.skrabec@...il.com>
> ---
>  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

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ