[v2] media: platform: rcar_imr: add IMR-LSX3 support

Add support for the image renderer light SRAM extended 3 (IMR-LSX3) found
only in the R-Car V2H (R8A7792) SoC.  It differs  from IMR-LX4 in that it
supports only planar video formats but can use the video capture data for
the textures.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
This patch  is against the 'media_tree.git' repo's 'master' branch plus the
latest version of  the Renesas IMR driver...

Changes in version 2:
- renamed *enum* 'imr_gen' to 'imr_type' and the *struct* field of this type
  from 'gen' to 'type';
- rename *struct* 'imr_type' to 'imr_info' and the fields/variables of this type
  from 'type' to 'info';
- added comments to IMR-LX4 only CMRCR2 bits;
- added IMR type check to the WTS instruction writing to CMRCCR2.

 Documentation/devicetree/bindings/media/rcar_imr.txt |   11 +
 drivers/media/platform/rcar_imr.c                    |  106 +++++++++++++++----
 2 files changed, 97 insertions(+), 20 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 03/16/17 19:59, Sergei Shtylyov wrote:
> Add support for the image renderer light SRAM extended 3 (IMR-LSX3) found
> only in the R-Car V2H (R8A7792) SoC.  It differs  from IMR-LX4 in that it
> supports only planar video formats but can use the video capture data for
> the textures.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> This patch  is against the 'media_tree.git' repo's 'master' branch plus the
> latest version of  the Renesas IMR driver...
> 
> Changes in version 2:
> - renamed *enum* 'imr_gen' to 'imr_type' and the *struct* field of this type
>   from 'gen' to 'type';
> - rename *struct* 'imr_type' to 'imr_info' and the fields/variables of this type
>   from 'type' to 'info';
> - added comments to IMR-LX4 only CMRCR2 bits;
> - added IMR type check to the WTS instruction writing to CMRCCR2.
> 
>  Documentation/devicetree/bindings/media/rcar_imr.txt |   11 +
>  drivers/media/platform/rcar_imr.c                    |  106 +++++++++++++++----
>  2 files changed, 97 insertions(+), 20 deletions(-)
> 
> Index: media_tree/Documentation/devicetree/bindings/media/rcar_imr.txt
> ===================================================================
> --- media_tree.orig/Documentation/devicetree/bindings/media/rcar_imr.txt
> +++ media_tree/Documentation/devicetree/bindings/media/rcar_imr.txt
> @@ -8,9 +8,14 @@ and drawing with respect to any shape th
>  
>  Required properties:
>  
> -- compatible: "renesas,<soctype>-imr-lx4", "renesas,imr-lx4" as a fallback for
> -  the image renderer light extended 4 (IMR-LX4) found in the R-Car gen3 SoCs,
> -  where the examples with <soctype> are:
> +- compatible:
> +  "renesas,<soctype>-imr-lsx3", "renesas,imr-lsx3" as a fallback for the image
> +  renderer light SRAM extended 3 (IMR-LSX3) found in the R-Car gen2 SoCs, where
> +  the examples with <soctype> are:
> +  - "renesas,r8a7792-imr-lsx3" for R-Car V2H;
> +  "renesas,<soctype>-imr-lx4", "renesas,imr-lx4" as a fallback for the image
> +  renderer light extended 4 (IMR-LX4) found in the R-Car gen3 SoCs, where the
> +  examples with <soctype> are:
>    - "renesas,r8a7795-imr-lx4" for R-Car H3,
>    - "renesas,r8a7796-imr-lx4" for R-Car M3-W.
>  - reg: offset and length of the register block;
> Index: media_tree/drivers/media/platform/rcar_imr.c
> ===================================================================
> --- media_tree.orig/drivers/media/platform/rcar_imr.c
> +++ media_tree/drivers/media/platform/rcar_imr.c
> @@ -1,5 +1,5 @@
>  /*
> - * rcar_imr.c -- R-Car IMR-LX4 Driver
> + * rcar_imr.c -- R-Car IMR-LSX3/LX4 Driver
>   *
>   * Copyright (C) 2015-2017 Cogent Embedded, Inc. <source@cogentembedded.com>
>   *
> @@ -14,7 +14,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/platform_device.h>
> +#include <linux/of_device.h>
>  #include <linux/rcar_imr.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
> @@ -81,8 +81,21 @@ struct imr_format_info {
>  	u32			flags;
>  };
>  
> +enum imr_type {
> +	IMR_LSX3,
> +	IMR_LX4,
> +};
> +
> +/* IMR type specific data */
> +struct imr_info {
> +	enum imr_type		type;
> +	const struct imr_format_info *formats;
> +	unsigned int		num_formats;
> +};
> +
>  /* per-device data */
>  struct imr_device {
> +	const struct imr_info	*info;
>  	struct device		*dev;
>  	struct clk		*clock;
>  	void __iomem		*mmio;
> @@ -180,6 +193,7 @@ struct imr_ctx {
>  #define IMR_IMR_IEM		BIT(1)
>  #define IMR_IMR_INM		BIT(2)
>  
> +#define IMR_CMRCR_TXTM		BIT(0)		/* IMR-LSX3 only */
>  #define IMR_CMRCR_LUCE		BIT(1)
>  #define IMR_CMRCR_CLCE		BIT(2)
>  #define IMR_CMRCR_DUV_SHIFT	3
> @@ -219,11 +233,11 @@ struct imr_ctx {
>  #define IMR_CPDPOR_YLDPO	GENMASK(10, 8)
>  
>  #define IMR_CMRCR2_LUTE		BIT(0)
> -#define IMR_CMRCR2_YUV422E	BIT(2)
> -#define IMR_CMRCR2_YUV422FORM	BIT(5)
> -#define IMR_CMRCR2_UVFORM	BIT(6)
> -#define IMR_CMRCR2_TCTE		BIT(12)
> -#define IMR_CMRCR2_DCTE		BIT(15)
> +#define IMR_CMRCR2_YUV422E	BIT(2)		/* IMR-LX4 only */
> +#define IMR_CMRCR2_YUV422FORM	BIT(5)		/* IMR-LX4 only */
> +#define IMR_CMRCR2_UVFORM	BIT(6)		/* IMR-LX4 only */
> +#define IMR_CMRCR2_TCTE		BIT(12)		/* IMR-LX4 only */
> +#define IMR_CMRCR2_DCTE		BIT(15)		/* IMR-LX4 only */
>  
>  /*******************************************************************************
>   * Display list commands
> @@ -282,6 +296,34 @@ static u32 __imr_flags_common(u32 iflags
>  	return iflags & oflags & IMR_F_PLANES_MASK;
>  }
>  
> +static const struct imr_format_info imr_lsx3_formats[] = {
> +	{
> +		.name	= "YUV 4:2:2 semiplanar (NV16)",
> +		.fourcc	= V4L2_PIX_FMT_NV16,
> +		.flags	= IMR_F_Y8 | IMR_F_UV8 | IMR_F_PLANAR,
> +	},
> +	{
> +		.name	= "Greyscale 8-bit",
> +		.fourcc	= V4L2_PIX_FMT_GREY,
> +		.flags	= IMR_F_Y8 | IMR_F_PLANAR,
> +	},
> +	{
> +		.name	= "Greyscale 10-bit",
> +		.fourcc	= V4L2_PIX_FMT_Y10,
> +		.flags	= IMR_F_Y8 | IMR_F_Y10 | IMR_F_PLANAR,
> +	},
> +	{
> +		.name	= "Greyscale 12-bit",
> +		.fourcc	= V4L2_PIX_FMT_Y12,
> +		.flags	= IMR_F_Y8 | IMR_F_Y10 | IMR_F_Y12 | IMR_F_PLANAR,
> +	},
> +	{
> +		.name	= "Chrominance UV 8-bit",
> +		.fourcc	= V4L2_PIX_FMT_UV8,
> +		.flags	= IMR_F_UV8 | IMR_F_PLANAR,
> +	},
> +};
> +
>  static const struct imr_format_info imr_lx4_formats[] = {
>  	{
>  		.name	= "YUV 4:2:2 semiplanar (NV16)",
> @@ -335,6 +377,18 @@ static const struct imr_format_info imr_
>  	},
>  };
>  
> +static const struct imr_info imr_lsx3 = {
> +	.type		= IMR_LSX3,
> +	.formats	= imr_lsx3_formats,
> +	.num_formats	= ARRAY_SIZE(imr_lsx3_formats),
> +};
> +
> +static const struct imr_info imr_lx4 = {
> +	.type		= IMR_LX4,
> +	.formats	= imr_lx4_formats,
> +	.num_formats	= ARRAY_SIZE(imr_lx4_formats),
> +};
> +
>  /* mesh configuration constructor */
>  static struct imr_cfg *imr_cfg_create(struct imr_ctx *ctx,
>  				      u32 dl_size, u32 dl_start)
> @@ -767,7 +821,8 @@ static void imr_dl_program_setup(struct
>  		 "setup %ux%u -> %ux%u mapping (type=%x)\n", w, h, W, H, type);
>  
>  	/* set triangle mode register from user-supplied descriptor */
> -	*dl++ = IMR_OP_WTS(IMR_TRIMCR, 0x004F);
> +	*dl++ = IMR_OP_WTS(IMR_TRIMCR,
> +			   ctx->imr->info->type == IMR_LX4 ? 0x004F : 0x007F);
>  
>  	/* set automatic source/destination coordinates generation flags */
>  	*dl++ = IMR_OP_WTS(IMR_TRIMSR, __imr_auto_sg_dg_tcm(type) |
> @@ -781,7 +836,8 @@ static void imr_dl_program_setup(struct
>  
>  	/* reset rendering mode registers */
>  	*dl++ = IMR_OP_WTS(IMR_CMRCCR,  0xDBFE);
> -	*dl++ = IMR_OP_WTS(IMR_CMRCCR2, 0x9065);
> +	*dl++ = IMR_OP_WTS(IMR_CMRCCR2, ctx->imr->info->type == IMR_LX4 ?
> +			   0x9065 : IMR_CMRCR2_LUTE);
>  
>  	/* set source/destination addresses of Y/UV plane */
>  	*dl++ = IMR_OP_WTL(IMR_DSAR, 2);
> @@ -823,7 +879,7 @@ static void imr_dl_program_setup(struct
>  			*dl++ = IMR_OP_WTS(IMR_SSTR,
>  					   w << (iflags & IMR_F_UV10 ? 1 : 0));
>  		}
> -	} else {
> +	} else if (ctx->imr->info->type == IMR_LX4) {
>  		u16 src_fmt = (iflags & IMR_F_UV_SWAP ? IMR_CMRCR2_UVFORM : 0) |
>  			      (iflags & IMR_F_YUV_SWAP ?
>  			       IMR_CMRCR2_YUV422FORM : 0);
> @@ -864,6 +920,9 @@ static void imr_dl_program_setup(struct
>  			*dl++ = IMR_OP_WTS(IMR_DSTR,
>  					   W << (cflags & IMR_F_Y10 ? 2 : 1));
>  		}
> +	} else	{
> +		/* this shouldn't happen! */
> +		BUG();

Can you find a better way? The use of 'BUG' is frowned upon. It's better
to return an error here. Also, are you sure this can't happen? At least for
me it is not obvious from the code.

>  	}
>  
>  	/*
> @@ -1114,6 +1173,7 @@ out:
>  /* test if a format is supported */
>  static int __imr_try_fmt(struct imr_ctx *ctx, struct v4l2_format *f)
>  {
> +	const struct imr_info	*info = ctx->imr->info;
>  	struct v4l2_pix_format	*pix = &f->fmt.pix;
>  	u32			fourcc = pix->pixelformat;
>  	int			i;
> @@ -1122,8 +1182,8 @@ static int __imr_try_fmt(struct imr_ctx
>  	 * both output and capture interface have the same set of
>  	 * supported formats
>  	 */
> -	for (i = 0; i < ARRAY_SIZE(imr_lx4_formats); i++) {
> -		if (fourcc == imr_lx4_formats[i].fourcc) {
> +	for (i = 0; i < info->num_formats; i++) {
> +		if (fourcc == info->formats[i].fourcc) {
>  			/* fix up format specification as needed */
>  			pix->field = V4L2_FIELD_NONE;
>  
> @@ -1174,9 +1234,12 @@ static int imr_querycap(struct file *fil
>  /* enumerate supported formats */
>  static int imr_enum_fmt(struct file *file, void *priv, struct v4l2_fmtdesc *f)
>  {
> +	struct imr_ctx		*ctx = fh_to_ctx(priv);
> +	const struct imr_info	*info = ctx->imr->info;
> +
>  	/* no distinction between output/capture formats */
> -	if (f->index < ARRAY_SIZE(imr_lx4_formats)) {
> -		const struct imr_format_info *fmt = &imr_lx4_formats[f->index];
> +	if (f->index < info->num_formats) {
> +		const struct imr_format_info *fmt = &info->formats[f->index];
>  
>  		strlcpy(f->description, fmt->name, sizeof(f->description));
>  		f->pixelformat = fmt->fourcc;
> @@ -1246,7 +1309,7 @@ static int imr_s_fmt(struct file *file,
>  
>  	/* processing is locked? TBD */
>  	q_data->fmt = f->fmt.pix;
> -	q_data->flags = imr_lx4_formats[i].flags;
> +	q_data->flags = ctx->imr->info->formats[i].flags;
>  
>  	/* set default crop factors */
>  	if (!V4L2_TYPE_IS_OUTPUT(f->type)) {
> @@ -1622,6 +1685,8 @@ static void imr_device_run(void *priv)
>  	wmb();
>  
>  	/* start rendering operation */
> +	if (imr->info->type != IMR_LX4)
> +		iowrite32(IMR_CMRCR_TXTM, imr->mmio + IMR_CMRCSR);
>  	iowrite32(IMR_CR_RS, imr->mmio + IMR_CR);
>  
>  	/* timestamp input buffer */
> @@ -1776,16 +1841,22 @@ handled:
>  
>  static int imr_probe(struct platform_device *pdev)
>  {
> +	const struct imr_info	*info;
>  	struct imr_device	*imr;
>  	struct resource		*res;
>  	int			ret;
>  
> +	info = of_device_get_match_data(&pdev->dev);
> +	if (!info)
> +		return -ENODEV;
> +
>  	imr = devm_kzalloc(&pdev->dev, sizeof(*imr), GFP_KERNEL);
>  	if (!imr)
>  		return -ENOMEM;
>  
>  	mutex_init(&imr->mutex);
>  	spin_lock_init(&imr->lock);
> +	imr->info = info;
>  	imr->dev = &pdev->dev;
>  
>  	/* memory-mapped registers */
> @@ -1919,7 +1990,8 @@ static const struct dev_pm_ops imr_pm_op
>  
>  /* device table */
>  static const struct of_device_id imr_of_match[] = {
> -	{ .compatible = "renesas,imr-lx4" },
> +	{ .compatible = "renesas,imr-lsx3", .data = &imr_lsx3, },
> +	{ .compatible = "renesas,imr-lx4",  .data = &imr_lx4,  },
>  	{ },
>  };
>  
> @@ -1939,5 +2011,5 @@ module_platform_driver(imr_platform_driv
>  
>  MODULE_ALIAS("imr");
>  MODULE_AUTHOR("Cogent Embedded Inc. <sources@cogentembedded.com>");
> -MODULE_DESCRIPTION("Renesas IMR-LX4 Driver");
> +MODULE_DESCRIPTION("Renesas IMR-LSX3/LX4 Driver");
>  MODULE_LICENSE("GPL");
> 

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 03/17/2017 05:33 PM, Hans Verkuil wrote:

>> Add support for the image renderer light SRAM extended 3 (IMR-LSX3) found
>> only in the R-Car V2H (R8A7792) SoC.  It differs  from IMR-LX4 in that it
>> supports only planar video formats but can use the video capture data for
>> the textures.
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

[...]
>> Index: media_tree/drivers/media/platform/rcar_imr.c
>> ===================================================================
>> --- media_tree.orig/drivers/media/platform/rcar_imr.c
>> +++ media_tree/drivers/media/platform/rcar_imr.c
[...]
>> @@ -282,6 +296,34 @@ static u32 __imr_flags_common(u32 iflags
>>  	return iflags & oflags & IMR_F_PLANES_MASK;
>>  }
>>
>> +static const struct imr_format_info imr_lsx3_formats[] = {
>> +	{
>> +		.name	= "YUV 4:2:2 semiplanar (NV16)",
>> +		.fourcc	= V4L2_PIX_FMT_NV16,
>> +		.flags	= IMR_F_Y8 | IMR_F_UV8 | IMR_F_PLANAR,
>> +	},
>> +	{
>> +		.name	= "Greyscale 8-bit",
>> +		.fourcc	= V4L2_PIX_FMT_GREY,
>> +		.flags	= IMR_F_Y8 | IMR_F_PLANAR,
>> +	},
>> +	{
>> +		.name	= "Greyscale 10-bit",
>> +		.fourcc	= V4L2_PIX_FMT_Y10,
>> +		.flags	= IMR_F_Y8 | IMR_F_Y10 | IMR_F_PLANAR,
>> +	},
>> +	{
>> +		.name	= "Greyscale 12-bit",
>> +		.fourcc	= V4L2_PIX_FMT_Y12,
>> +		.flags	= IMR_F_Y8 | IMR_F_Y10 | IMR_F_Y12 | IMR_F_PLANAR,
>> +	},
>> +	{
>> +		.name	= "Chrominance UV 8-bit",
>> +		.fourcc	= V4L2_PIX_FMT_UV8,
>> +		.flags	= IMR_F_UV8 | IMR_F_PLANAR,
>> +	},
>> +};
>> +
>>  static const struct imr_format_info imr_lx4_formats[] = {
>>  	{
>>  		.name	= "YUV 4:2:2 semiplanar (NV16)",
>> @@ -335,6 +377,18 @@ static const struct imr_format_info imr_
>>  	},
>>  };
>>
>> +static const struct imr_info imr_lsx3 = {
>> +	.type		= IMR_LSX3,
>> +	.formats	= imr_lsx3_formats,
>> +	.num_formats	= ARRAY_SIZE(imr_lsx3_formats),
>> +};
>> +
>> +static const struct imr_info imr_lx4 = {
>> +	.type		= IMR_LX4,
>> +	.formats	= imr_lx4_formats,
>> +	.num_formats	= ARRAY_SIZE(imr_lx4_formats),
>> +};
>> +
>>  /* mesh configuration constructor */
>>  static struct imr_cfg *imr_cfg_create(struct imr_ctx *ctx,
>>  				      u32 dl_size, u32 dl_start)
[...]
>> @@ -823,7 +879,7 @@ static void imr_dl_program_setup(struct
>>  			*dl++ = IMR_OP_WTS(IMR_SSTR,
>>  					   w << (iflags & IMR_F_UV10 ? 1 : 0));
>>  		}
>> -	} else {
>> +	} else if (ctx->imr->info->type == IMR_LX4) {
>>  		u16 src_fmt = (iflags & IMR_F_UV_SWAP ? IMR_CMRCR2_UVFORM : 0) |
>>  			      (iflags & IMR_F_YUV_SWAP ?
>>  			       IMR_CMRCR2_YUV422FORM : 0);
>> @@ -864,6 +920,9 @@ static void imr_dl_program_setup(struct
>>  			*dl++ = IMR_OP_WTS(IMR_DSTR,
>>  					   W << (cflags & IMR_F_Y10 ? 2 : 1));
>>  		}
>> +	} else	{
>> +		/* this shouldn't happen! */
>> +		BUG();
>
> Can you find a better way? The use of 'BUG' is frowned upon. It's better
> to return an error here.

    OK, I'll try to return -EINVAL here...
    BTW, the main driver patch also has BUG_ON()...

> Also, are you sure this can't happen?  At least for
 > me it is not obvious from the code.

    Absolutely. Interleaved formats are not supported by IMR-L[S]X3, so the 
format array has IMR_F_PLANAR set for all formats supported by these cores. 
Interleaved formats are onlt supported by IMR-LX4 (for which we check above).

[...]

> Regards,
>
> 	Hans

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, Mar 16, 2017 at 09:59:50PM +0300, Sergei Shtylyov wrote:
> Add support for the image renderer light SRAM extended 3 (IMR-LSX3) found
> only in the R-Car V2H (R8A7792) SoC.  It differs  from IMR-LX4 in that it
> supports only planar video formats but can use the video capture data for
> the textures.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> This patch  is against the 'media_tree.git' repo's 'master' branch plus the
> latest version of  the Renesas IMR driver...
> 
> Changes in version 2:
> - renamed *enum* 'imr_gen' to 'imr_type' and the *struct* field of this type
>   from 'gen' to 'type';
> - rename *struct* 'imr_type' to 'imr_info' and the fields/variables of this type
>   from 'type' to 'info';
> - added comments to IMR-LX4 only CMRCR2 bits;
> - added IMR type check to the WTS instruction writing to CMRCCR2.
> 
>  Documentation/devicetree/bindings/media/rcar_imr.txt |   11 +

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/media/platform/rcar_imr.c                    |  106 +++++++++++++++----
>  2 files changed, 97 insertions(+), 20 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html