Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Jernej Škrabec" <jernej.skrabec@siol.net>
To: mchehab@kernel.org, mripard@kernel.org,
	paul.kocialkowski@bootlin.com, Hans Verkuil <hverkuil@xs4all.nl>
Cc: devel@driverdev.osuosl.org, gregkh@linuxfoundation.org,
	linux-kernel@vger.kernel.org, wens@csie.org,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org
Subject: Re: [PATCH v2 2/4] media: cedrus: hevc: Add support for scaling matrix
Date: Tue, 07 Jan 2020 18:10:42 +0100	[thread overview]
Message-ID: <2627039.Y6S9NjorxK@jernej-laptop> (raw)
In-Reply-To: <4ac91ed5-a220-6a04-b1da-de27a306f8f2@xs4all.nl>

Hi!

Dne torek, 07. januar 2020 ob 16:01:16 CET je Hans Verkuil napisal(a):
> On 12/13/19 5:04 PM, Jernej Skrabec wrote:
> > HEVC frames may use scaling list feature. Add support for it.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> > 
> >  drivers/staging/media/sunxi/cedrus/cedrus.c   |  7 ++
> >  drivers/staging/media/sunxi/cedrus/cedrus.h   |  1 +
> >  .../staging/media/sunxi/cedrus/cedrus_dec.c   |  2 +
> >  .../staging/media/sunxi/cedrus/cedrus_h265.c  | 70 ++++++++++++++++++-
> >  .../staging/media/sunxi/cedrus/cedrus_regs.h  |  2 +
> >  5 files changed, 81 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus.c index
> > c6ddd46eff82..bf68bc6b20c8 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> > @@ -116,6 +116,13 @@ static const struct cedrus_control cedrus_controls[]
> > = {> 
> >  		.codec		= CEDRUS_CODEC_H265,
> >  		.required	= true,
> >  	
> >  	},
> > 
> > +	{
> > +		.cfg = {
> > +			.id	= 
V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX,
> > +		},
> > +		.codec		= CEDRUS_CODEC_H265,
> > +		.required	= true,
> 
> Should this be true? This means that existing applications are now
> suddenly required to always pass the scaling matrix for every buffer.
> 
> Especially since the commit log says: 'HEVC frames *may* use scaling list
> feature', indicating that this is an optional feature.

True. Can you fix this when applying if this is the only issue?

Best regards,
Jernej

> 
> Regards,
> 
> 	Hans
> 
> > +	},
> > 
> >  	{
> >  	
> >  		.cfg = {
> >  		
> >  			.id	= 
V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE,
> > 
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > b/drivers/staging/media/sunxi/cedrus/cedrus.h index
> > 96765555ab8a..d945f4f0ff2d 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> > @@ -73,6 +73,7 @@ struct cedrus_h265_run {
> > 
> >  	const struct v4l2_ctrl_hevc_sps			*sps;
> >  	const struct v4l2_ctrl_hevc_pps			*pps;
> >  	const struct v4l2_ctrl_hevc_slice_params	*slice_params;
> > 
> > +	const struct v4l2_ctrl_hevc_scaling_matrix	
*scaling_matrix;
> > 
> >  };
> >  
> >  struct cedrus_run {
> > 
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c index
> > 4a2fc33a1d79..327ed6c264dc 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > @@ -66,6 +66,8 @@ void cedrus_device_run(void *priv)
> > 
> >  			V4L2_CID_MPEG_VIDEO_HEVC_PPS);
> >  		
> >  		run.h265.slice_params = cedrus_find_control_data(ctx,
> >  		
> >  			V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS);
> > 
> > +		run.h265.scaling_matrix = cedrus_find_control_data(ctx,
> > +			V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX);
> > 
> >  		break;
> >  	
> >  	default:
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c index
> > 6945dc74e1d7..888bfd5ca224 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> > @@ -220,6 +220,69 @@ static void cedrus_h265_pred_weight_write(struct
> > cedrus_dev *dev,> 
> >  	}
> >  
> >  }
> > 
> > +static void cedrus_h265_write_scaling_list(struct cedrus_ctx *ctx,
> > +					   struct cedrus_run 
*run)
> > +{
> > +	const struct v4l2_ctrl_hevc_scaling_matrix *scaling;
> > +	struct cedrus_dev *dev = ctx->dev;
> > +	u32 i, j, k, val;
> > +
> > +	scaling = run->h265.scaling_matrix;
> > +
> > +	cedrus_write(dev, VE_DEC_H265_SCALING_LIST_DC_COEF0,
> > +		     (scaling->scaling_list_dc_coef_32x32[1] << 24) |
> > +		     (scaling->scaling_list_dc_coef_32x32[0] << 16) |
> > +		     (scaling->scaling_list_dc_coef_16x16[1] << 8) |
> > +		     (scaling->scaling_list_dc_coef_16x16[0] << 0));
> > +
> > +	cedrus_write(dev, VE_DEC_H265_SCALING_LIST_DC_COEF1,
> > +		     (scaling->scaling_list_dc_coef_16x16[5] << 24) |
> > +		     (scaling->scaling_list_dc_coef_16x16[4] << 16) |
> > +		     (scaling->scaling_list_dc_coef_16x16[3] << 8) |
> > +		     (scaling->scaling_list_dc_coef_16x16[2] << 0));
> > +
> > +	cedrus_h265_sram_write_offset(dev,
> > VE_DEC_H265_SRAM_OFFSET_SCALING_LISTS); +
> > +	for (i = 0; i < 6; i++)
> > +		for (j = 0; j < 8; j++)
> > +			for (k = 0; k < 8; k += 4) {
> > +				val = ((u32)scaling-
>scaling_list_8x8[i][j + (k + 3) * 8] << 24) |
> > +				      ((u32)scaling-
>scaling_list_8x8[i][j + (k + 2) * 8] << 16) |
> > +				      ((u32)scaling-
>scaling_list_8x8[i][j + (k + 1) * 8] << 8) |
> > +				      scaling-
>scaling_list_8x8[i][j + k * 8];
> > +				cedrus_write(dev, 
VE_DEC_H265_SRAM_DATA, val);
> > +			}
> > +
> > +	for (i = 0; i < 2; i++)
> > +		for (j = 0; j < 8; j++)
> > +			for (k = 0; k < 8; k += 4) {
> > +				val = ((u32)scaling-
>scaling_list_32x32[i][j + (k + 3) * 8] << 24) |
> > +				      ((u32)scaling-
>scaling_list_32x32[i][j + (k + 2) * 8] << 16) |
> > +				      ((u32)scaling-
>scaling_list_32x32[i][j + (k + 1) * 8] << 8) |
> > +				      scaling-
>scaling_list_32x32[i][j + k * 8];
> > +				cedrus_write(dev, 
VE_DEC_H265_SRAM_DATA, val);
> > +			}
> > +
> > +	for (i = 0; i < 6; i++)
> > +		for (j = 0; j < 8; j++)
> > +			for (k = 0; k < 8; k += 4) {
> > +				val = ((u32)scaling-
>scaling_list_16x16[i][j + (k + 3) * 8] << 24) |
> > +				      ((u32)scaling-
>scaling_list_16x16[i][j + (k + 2) * 8] << 16) |
> > +				      ((u32)scaling-
>scaling_list_16x16[i][j + (k + 1) * 8] << 8) |
> > +				      scaling-
>scaling_list_16x16[i][j + k * 8];
> > +				cedrus_write(dev, 
VE_DEC_H265_SRAM_DATA, val);
> > +			}
> > +
> > +	for (i = 0; i < 6; i++)
> > +		for (j = 0; j < 4; j++) {
> > +			val = ((u32)scaling->scaling_list_4x4[i][j + 
12] << 24) |
> > +			      ((u32)scaling->scaling_list_4x4[i][j + 
8] << 16) |
> > +			      ((u32)scaling->scaling_list_4x4[i][j + 
4] << 8) |
> > +			      scaling->scaling_list_4x4[i][j];
> > +			cedrus_write(dev, VE_DEC_H265_SRAM_DATA, 
val);
> > +		}
> > +}
> > +
> > 
> >  static void cedrus_h265_setup(struct cedrus_ctx *ctx,
> >  
> >  			      struct cedrus_run *run)
> >  
> >  {
> > 
> > @@ -499,7 +562,12 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx,
> > 
> >  	/* Scaling list. */
> > 
> > -	reg = VE_DEC_H265_SCALING_LIST_CTRL0_DEFAULT;
> > +	if (sps->flags & V4L2_HEVC_SPS_FLAG_SCALING_LIST_ENABLED) {
> > +		cedrus_h265_write_scaling_list(ctx, run);
> > +		reg = VE_DEC_H265_SCALING_LIST_CTRL0_FLAG_ENABLED;
> > +	} else {
> > +		reg = VE_DEC_H265_SCALING_LIST_CTRL0_DEFAULT;
> > +	}
> > 
> >  	cedrus_write(dev, VE_DEC_H265_SCALING_LIST_CTRL0, reg);
> >  	
> >  	/* Neightbor information address. */
> > 
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
> > b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h index
> > 7beb03d3bb39..0d9449fe2b28 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
> > @@ -492,6 +492,8 @@
> > 
> >  #define VE_DEC_H265_ENTRY_POINT_OFFSET_ADDR	(VE_ENGINE_DEC_H265 + 
0x64)
> >  #define VE_DEC_H265_TILE_START_CTB		(VE_ENGINE_DEC_H265 + 
0x68)
> >  #define VE_DEC_H265_TILE_END_CTB		(VE_ENGINE_DEC_H265 + 
0x6c)
> > 
> > +#define VE_DEC_H265_SCALING_LIST_DC_COEF0	(VE_ENGINE_DEC_H265 + 
0x78)
> > +#define VE_DEC_H265_SCALING_LIST_DC_COEF1	(VE_ENGINE_DEC_H265 + 
0x7c)
> > 
> >  #define VE_DEC_H265_LOW_ADDR			(VE_ENGINE_DEC_H265 + 
0x80)





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-01-07 17:11 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-13 16:04 [PATCH v2 0/4] media: cedrus: hevc: Add support for scaling matrix and multi-slice frames Jernej Skrabec
2019-12-13 16:04 ` [PATCH v2 1/4] media: uapi: hevc: Add scaling matrix control Jernej Skrabec
2020-01-08 14:11   ` Mauro Carvalho Chehab
2020-01-08 14:43     ` Paul Kocialkowski
2020-01-09 15:17       ` Jernej Škrabec
2020-01-09 15:19         ` Hans Verkuil
2019-12-13 16:04 ` [PATCH v2 2/4] media: cedrus: hevc: Add support for scaling matrix Jernej Skrabec
2020-01-07 15:01   ` Hans Verkuil
2020-01-07 17:10     ` Jernej Škrabec [this message]
2020-01-08  7:48       ` Hans Verkuil
2020-01-08 14:46       ` Hans Verkuil
2020-01-16 19:49         ` Jernej Škrabec
2019-12-13 16:04 ` [PATCH v2 3/4] media: uapi: hevc: Add segment address field Jernej Skrabec
2020-01-08 14:31   ` Mauro Carvalho Chehab
2020-01-09 14:46     ` Jernej Škrabec
2019-12-13 16:04 ` [PATCH v2 4/4] media: cedrus: hevc: Add support for multiple slices Jernej Skrabec

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2627039.Y6S9NjorxK@jernej-laptop \
    --to=jernej.skrabec@siol.net \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hverkuil@xs4all.nl \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=mripard@kernel.org \
    --cc=paul.kocialkowski@bootlin.com \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox