Re: [PATCH RESEND 1/1] media: platform: Renesas IMR driver

[Sergei Shtylyov]

Hello!

On 02/22/2017 10:05 PM, Sergei Shtylyov wrote:


From: Konstantin Kozhevnikov 

The image renderer light extended 4 (IMR-LX4) or the distortion correction
engine is a drawing processor with a simple  instruction system capable of
referencing data on an external memory as 2D texture data and performing
texture mapping and drawing with respect to any shape that is split into
triangular objects.

This V4L2 memory-to-memory device driver only supports image renderer found
in the R-Car gen3 SoCs; the R-Car gen2 support  can be added later...

[Sergei: merged 2 original patches, added the patch description, removed
unrelated parts,  added the binding document, ported the driver to the
modern kernel, renamed the UAPI header file and the guard  macros to match
the driver name, extended the copyrights, fixed up Kconfig prompt/depends/
help, made use of the BIT()/GENMASK() macros, sorted #include's, removed
leading  dots and fixed grammar in the comments, fixed up indentation to
use tabs where possible, renamed IMR_DLSR to IMR_DLPR to match the manual,
separated the register offset/bit #define's, removed *inline* from .c file,
fixed lines over 80 columns, removed useless parens, operators, casts,
braces, variables, #include's, (commented out) statements, and even
function, inserted empty line after desclaration, removed extra empty
lines, reordered some local variable desclarations, removed calls to
4l2_err() on kmalloc() failure, fixed the error returned by imr_default(),
avoided code duplication in the IRQ handler, used '__packed' for the UAPI
structures, enclosed the macro parameters in parens, exchanged the values
of IMR_MAP_AUTO[SD]G macros.]

Signed-off-by: Konstantin Kozhevnikov

Signed-off-by: Sergei Shtylyov 

---
This patch is against the 'media_tree.git' repo's 'master' branch.

 Documentation/devicetree/bindings/media/rcar_imr.txt |   23
 drivers/media/platform/Kconfig   |   13
 drivers/media/platform/Makefile  |1
 drivers/media/platform/rcar_imr.c| 1923
+++
 include/uapi/linux/rcar_imr.h|   94
 5 files changed, 2054 insertions(+)

Index: media_tree/Documentation/devicetree/bindings/media/rcar_imr.txt
===
--- /dev/null
+++ media_tree/Documentation/devicetree/bindings/media/rcar_imr.txt
@@ -0,0 +1,23 @@
+Renesas R-Car Image Renderer (Distortion Correction Engine)
+---

[...]

+
+Required properties:
+- compatible: must be "renesas,imr-lx4" for the image renderer light
extended 4
+  (IMR-LX4)  found in the R-Car gen3 SoCs;


Needs an SoC specific compatible string too.


   Strings, to be precise -- there are several SoCs but the IMR-LX4 core seems
the same among them. Well, if you say so...


The description is above, so you just need to list the compatible
strings.


   There's (most probably) gonna be other versions of the IMR core supported,
(this core can be forund in gen2 SoCs too)...


   Seriously, I strongly doubt that we need the SoC specific compatibles in 
this case -- they don't add any value and seem to only clutter the bindings 
(more so with adding support for the other variants of the IMR core). The 
manuals don't seem to have any real differences between the SoCs for any given 
variant of the IMR core...


MBR, Sergei

Re: [PATCH RESEND 1/1] media: platform: Renesas IMR driver

[Sergei Shtylyov]

Hello!

On 02/22/2017 05:25 PM, Rob Herring wrote:


From: Konstantin Kozhevnikov 

The image renderer light extended 4 (IMR-LX4) or the distortion correction
engine is a drawing processor with a simple  instruction system capable of
referencing data on an external memory as 2D texture data and performing
texture mapping and drawing with respect to any shape that is split into
triangular objects.

This V4L2 memory-to-memory device driver only supports image renderer found
in the R-Car gen3 SoCs; the R-Car gen2 support  can be added later...

[Sergei: merged 2 original patches, added the patch description, removed
unrelated parts,  added the binding document, ported the driver to the
modern kernel, renamed the UAPI header file and the guard  macros to match
the driver name, extended the copyrights, fixed up Kconfig prompt/depends/
help, made use of the BIT()/GENMASK() macros, sorted #include's, removed
leading  dots and fixed grammar in the comments, fixed up indentation to
use tabs where possible, renamed IMR_DLSR to IMR_DLPR to match the manual,
separated the register offset/bit #define's, removed *inline* from .c file,
fixed lines over 80 columns, removed useless parens, operators, casts,
braces, variables, #include's, (commented out) statements, and even
function, inserted empty line after desclaration, removed extra empty
lines, reordered some local variable desclarations, removed calls to
4l2_err() on kmalloc() failure, fixed the error returned by imr_default(),
avoided code duplication in the IRQ handler, used '__packed' for the UAPI
structures, enclosed the macro parameters in parens, exchanged the values
of IMR_MAP_AUTO[SD]G macros.]

Signed-off-by: Konstantin Kozhevnikov 

Signed-off-by: Sergei Shtylyov 

---
This patch is against the 'media_tree.git' repo's 'master' branch.

 Documentation/devicetree/bindings/media/rcar_imr.txt |   23
 drivers/media/platform/Kconfig   |   13
 drivers/media/platform/Makefile  |1
 drivers/media/platform/rcar_imr.c| 1923 +++
 include/uapi/linux/rcar_imr.h|   94
 5 files changed, 2054 insertions(+)

Index: media_tree/Documentation/devicetree/bindings/media/rcar_imr.txt
===
--- /dev/null
+++ media_tree/Documentation/devicetree/bindings/media/rcar_imr.txt
@@ -0,0 +1,23 @@
+Renesas R-Car Image Renderer (Distortion Correction Engine)
+---
+
+The image renderer  or the distortion correction  engine is a drawing processor
+with a simple  instruction system capable of referencing data in external 
memory
+as  2D texture data and performing texture mapping and drawing with respect to
+any shape that is split into triangular objects.


Please fix extra spaces in here.


   OK. Seems to be your pet peeve? :-)


+
+Required properties:
+- compatible: must be "renesas,imr-lx4" for the image renderer light extended 4
+  (IMR-LX4)  found in the R-Car gen3 SoCs;


Needs an SoC specific compatible string too.


   Strings, to be precise -- there are several SoCs but the IMR-LX4 core 
seems the same among them. Well, if you say so...



The description is above, so you just need to list the compatible
strings.


   There's (most probably) gonna be other versions of the IMR core supported, 
(this core can be forund in gen2 SoCs too)...



+- reg: offset and length of the register block;
+- interrupts: interrupt specifier;


How many interrupts?


   I thought it was clear from using singular.


+- clocks: clock phandle and specifier pair.


How many clocks?


   Two, perhaps. I meant a single clock by using singular again.

[...]

MBR, Sergei

Re: [PATCH RESEND 1/1] media: platform: Renesas IMR driver

[Rob Herring]

On Sat, Feb 11, 2017 at 11:02:01PM +0300, Sergei Shtylyov wrote:
> From: Konstantin Kozhevnikov 
> 
> The image renderer light extended 4 (IMR-LX4) or the distortion correction
> engine is a drawing processor with a simple  instruction system capable of
> referencing data on an external memory as 2D texture data and performing
> texture mapping and drawing with respect to any shape that is split into
> triangular objects.
> 
> This V4L2 memory-to-memory device driver only supports image renderer found
> in the R-Car gen3 SoCs; the R-Car gen2 support  can be added later...
> 
> [Sergei: merged 2 original patches, added the patch description, removed
> unrelated parts,  added the binding document, ported the driver to the
> modern kernel, renamed the UAPI header file and the guard  macros to match
> the driver name, extended the copyrights, fixed up Kconfig prompt/depends/
> help, made use of the BIT()/GENMASK() macros, sorted #include's, removed
> leading  dots and fixed grammar in the comments, fixed up indentation to
> use tabs where possible, renamed IMR_DLSR to IMR_DLPR to match the manual,
> separated the register offset/bit #define's, removed *inline* from .c file,
> fixed lines over 80 columns, removed useless parens, operators, casts,
> braces, variables, #include's, (commented out) statements, and even
> function, inserted empty line after desclaration, removed extra empty
> lines, reordered some local variable desclarations, removed calls to
> 4l2_err() on kmalloc() failure, fixed the error returned by imr_default(),
> avoided code duplication in the IRQ handler, used '__packed' for the UAPI
> structures, enclosed the macro parameters in parens, exchanged the values
> of IMR_MAP_AUTO[SD]G macros.]
> 
> Signed-off-by: Konstantin Kozhevnikov 
> 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch is against the 'media_tree.git' repo's 'master' branch.
> 
>  Documentation/devicetree/bindings/media/rcar_imr.txt |   23 
>  drivers/media/platform/Kconfig   |   13 
>  drivers/media/platform/Makefile  |1 
>  drivers/media/platform/rcar_imr.c| 1923 
> +++
>  include/uapi/linux/rcar_imr.h|   94 
>  5 files changed, 2054 insertions(+)
> 
> Index: media_tree/Documentation/devicetree/bindings/media/rcar_imr.txt
> ===
> --- /dev/null
> +++ media_tree/Documentation/devicetree/bindings/media/rcar_imr.txt
> @@ -0,0 +1,23 @@
> +Renesas R-Car Image Renderer (Distortion Correction Engine)
> +---
> +
> +The image renderer  or the distortion correction  engine is a drawing 
> processor
> +with a simple  instruction system capable of referencing data in external 
> memory
> +as  2D texture data and performing texture mapping and drawing with respect 
> to
> +any shape that is split into triangular objects.

Please fix extra spaces in here.

> +
> +Required properties:
> +- compatible: must be "renesas,imr-lx4" for the image renderer light 
> extended 4
> +  (IMR-LX4)  found in the R-Car gen3 SoCs;

Needs an SoC specific compatible string too.

The description is above, so you just need to list the compatible 
strings.

> +- reg: offset and length of the register block;
> +- interrupts: interrupt specifier;

How many interrupts?

> +- clocks: clock phandle and specifier pair.

How many clocks?

> +
> +Example:
> +
> + imr-lx4@fe86 {
> + compatible = "renesas,imr-lx4";
> + reg = <0 0xfe86 0 0x2000>;
> + interrupts = ;
> + clocks = < CPG_MOD 823>;
> + };

Re: [PATCH RESEND 1/1] media: platform: Renesas IMR driver

[Konstantin Kozhevnikov]

Hello Laurent,

regarding your main question, I think the name "image renderer" is 
misguiding. This IP-block has nothing to do with rendering per se, it is 
rather an image processing module, designed specifically for image 
undistortion (lens correction). We use that as a plain memory-to-memory 
device, and it fits well into V4L2 framework. More complex 
implementation (the one we thought of, but had no capacity to implement) 
would allow integration of VIN (as well as other modules like 
H.264/MPEG4 decoders) and IMR engines into a single processing pipeline 
(so it would not look like a M2M device but would be a sort of 
"smart-VIN"). From that perspective it is no more DRM device driver than 
VSP, which resides in the same "drivers/platform/media" directory.


Sincerely,
Kostya

On 02/12/2017 06:51 PM, Laurent Pinchart wrote:

Hi Sergei,

(CC'ing the dri-evel mailing list)

Thank you for the patch.

On Saturday 11 Feb 2017 23:02:01 Sergei Shtylyov wrote:

From: Konstantin Kozhevnikov 

The image renderer light extended 4 (IMR-LX4) or the distortion correction
engine is a drawing processor with a simple  instruction system capable of
referencing data on an external memory as 2D texture data and performing
texture mapping and drawing with respect to any shape that is split into
triangular objects.

This V4L2 memory-to-memory device driver only supports image renderer found
in the R-Car gen3 SoCs; the R-Car gen2 support  can be added later...

Let's start with the main question : given that this is a rendering engine, it
looks like it should use the DRM subsystem.


[Sergei: merged 2 original patches, added the patch description, removed
unrelated parts,  added the binding document, ported the driver to the
modern kernel, renamed the UAPI header file and the guard  macros to match
the driver name, extended the copyrights, fixed up Kconfig prompt/depends/
help, made use of the BIT()/GENMASK() macros, sorted #include's, removed
leading  dots and fixed grammar in the comments, fixed up indentation to
use tabs where possible, renamed IMR_DLSR to IMR_DLPR to match the manual,
separated the register offset/bit #define's, removed *inline* from .c file,
fixed lines over 80 columns, removed useless parens, operators, casts,
braces, variables, #include's, (commented out) statements, and even
function, inserted empty line after desclaration, removed extra empty
lines, reordered some local variable desclarations, removed calls to
4l2_err() on kmalloc() failure, fixed the error returned by imr_default(),
avoided code duplication in the IRQ handler, used '__packed' for the UAPI
structures, enclosed the macro parameters in parens, exchanged the values
of IMR_MAP_AUTO[SD]G macros.]

Signed-off-by: Konstantin Kozhevnikov
 Signed-off-by: Sergei Shtylyov


---
This patch is against the 'media_tree.git' repo's 'master' branch.

  Documentation/devicetree/bindings/media/rcar_imr.txt |   23
  drivers/media/platform/Kconfig   |   13
  drivers/media/platform/Makefile  |1
  drivers/media/platform/rcar_imr.c| 1923 +++
  include/uapi/linux/rcar_imr.h|   94
  5 files changed, 2054 insertions(+)

Re: [PATCH RESEND 1/1] media: platform: Renesas IMR driver

[Laurent Pinchart]

Hi Sergei,

(CC'ing the dri-evel mailing list)

Thank you for the patch.

On Saturday 11 Feb 2017 23:02:01 Sergei Shtylyov wrote:
> From: Konstantin Kozhevnikov 
> 
> The image renderer light extended 4 (IMR-LX4) or the distortion correction
> engine is a drawing processor with a simple  instruction system capable of
> referencing data on an external memory as 2D texture data and performing
> texture mapping and drawing with respect to any shape that is split into
> triangular objects.
> 
> This V4L2 memory-to-memory device driver only supports image renderer found
> in the R-Car gen3 SoCs; the R-Car gen2 support  can be added later...

Let's start with the main question : given that this is a rendering engine, it 
looks like it should use the DRM subsystem.

> [Sergei: merged 2 original patches, added the patch description, removed
> unrelated parts,  added the binding document, ported the driver to the
> modern kernel, renamed the UAPI header file and the guard  macros to match
> the driver name, extended the copyrights, fixed up Kconfig prompt/depends/
> help, made use of the BIT()/GENMASK() macros, sorted #include's, removed
> leading  dots and fixed grammar in the comments, fixed up indentation to
> use tabs where possible, renamed IMR_DLSR to IMR_DLPR to match the manual,
> separated the register offset/bit #define's, removed *inline* from .c file,
> fixed lines over 80 columns, removed useless parens, operators, casts,
> braces, variables, #include's, (commented out) statements, and even
> function, inserted empty line after desclaration, removed extra empty
> lines, reordered some local variable desclarations, removed calls to
> 4l2_err() on kmalloc() failure, fixed the error returned by imr_default(),
> avoided code duplication in the IRQ handler, used '__packed' for the UAPI
> structures, enclosed the macro parameters in parens, exchanged the values
> of IMR_MAP_AUTO[SD]G macros.]
> 
> Signed-off-by: Konstantin Kozhevnikov
>  Signed-off-by: Sergei Shtylyov
> 
> 
> ---
> This patch is against the 'media_tree.git' repo's 'master' branch.
> 
>  Documentation/devicetree/bindings/media/rcar_imr.txt |   23
>  drivers/media/platform/Kconfig   |   13
>  drivers/media/platform/Makefile  |1
>  drivers/media/platform/rcar_imr.c| 1923 +++
>  include/uapi/linux/rcar_imr.h|   94
>  5 files changed, 2054 insertions(+)

-- 
Regards,

Laurent Pinchart

[PATCH RESEND 1/1] media: platform: Renesas IMR driver

[Sergei Shtylyov]

From: Konstantin Kozhevnikov 

The image renderer light extended 4 (IMR-LX4) or the distortion correction
engine is a drawing processor with a simple  instruction system capable of
referencing data on an external memory as 2D texture data and performing
texture mapping and drawing with respect to any shape that is split into
triangular objects.

This V4L2 memory-to-memory device driver only supports image renderer found
in the R-Car gen3 SoCs; the R-Car gen2 support  can be added later...

[Sergei: merged 2 original patches, added the patch description, removed
unrelated parts,  added the binding document, ported the driver to the
modern kernel, renamed the UAPI header file and the guard  macros to match
the driver name, extended the copyrights, fixed up Kconfig prompt/depends/
help, made use of the BIT()/GENMASK() macros, sorted #include's, removed
leading  dots and fixed grammar in the comments, fixed up indentation to
use tabs where possible, renamed IMR_DLSR to IMR_DLPR to match the manual,
separated the register offset/bit #define's, removed *inline* from .c file,
fixed lines over 80 columns, removed useless parens, operators, casts,
braces, variables, #include's, (commented out) statements, and even
function, inserted empty line after desclaration, removed extra empty
lines, reordered some local variable desclarations, removed calls to
4l2_err() on kmalloc() failure, fixed the error returned by imr_default(),
avoided code duplication in the IRQ handler, used '__packed' for the UAPI
structures, enclosed the macro parameters in parens, exchanged the values
of IMR_MAP_AUTO[SD]G macros.]

Signed-off-by: Konstantin Kozhevnikov 

Signed-off-by: Sergei Shtylyov 

---
This patch is against the 'media_tree.git' repo's 'master' branch.

 Documentation/devicetree/bindings/media/rcar_imr.txt |   23 
 drivers/media/platform/Kconfig   |   13 
 drivers/media/platform/Makefile  |1 
 drivers/media/platform/rcar_imr.c| 1923 +++
 include/uapi/linux/rcar_imr.h|   94 
 5 files changed, 2054 insertions(+)

Index: media_tree/Documentation/devicetree/bindings/media/rcar_imr.txt
===
--- /dev/null
+++ media_tree/Documentation/devicetree/bindings/media/rcar_imr.txt
@@ -0,0 +1,23 @@
+Renesas R-Car Image Renderer (Distortion Correction Engine)
+---
+
+The image renderer  or the distortion correction  engine is a drawing processor
+with a simple  instruction system capable of referencing data in external 
memory
+as  2D texture data and performing texture mapping and drawing with respect to
+any shape that is split into triangular objects.
+
+Required properties:
+- compatible: must be "renesas,imr-lx4" for the image renderer light extended 4
+  (IMR-LX4)  found in the R-Car gen3 SoCs;
+- reg: offset and length of the register block;
+- interrupts: interrupt specifier;
+- clocks: clock phandle and specifier pair.
+
+Example:
+
+   imr-lx4@fe86 {
+   compatible = "renesas,imr-lx4";
+   reg = <0 0xfe86 0 0x2000>;
+   interrupts = ;
+   clocks = < CPG_MOD 823>;
+   };
Index: media_tree/drivers/media/platform/Kconfig
===
--- media_tree.orig/drivers/media/platform/Kconfig
+++ media_tree/drivers/media/platform/Kconfig
@@ -399,6 +399,19 @@ config VIDEO_RENESAS_FCP
  To compile this driver as a module, choose M here: the module
  will be called rcar-fcp.
 
+config VIDEO_RENESAS_IMR
+   tristate "Renesas Image Renderer (Distortion Correction Engine)"
+   depends on VIDEO_DEV && VIDEO_V4L2
+   depends on ARCH_RENESAS || COMPILE_TEST
+   select VIDEOBUF2_DMA_CONTIG
+   select V4L2_MEM2MEM_DEV
+   ---help---
+ This is a V4L2 driver for the Renesas Image Renderer Light Extended 4
+ (IMR-LX4).
+
+ To compile this driver as a module, choose M here: the module
+ will be called rcar_imr.
+
 config VIDEO_RENESAS_VSP1
tristate "Renesas VSP1 Video Processing Engine"
depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && HAS_DMA
Index: media_tree/drivers/media/platform/Makefile
===
--- media_tree.orig/drivers/media/platform/Makefile
+++ media_tree/drivers/media/platform/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_SOC_CAMERA)  += soc_camera/
 obj-$(CONFIG_VIDEO_RENESAS_FCP)+= rcar-fcp.o
 obj-$(CONFIG_VIDEO_RENESAS_FDP1)   += rcar_fdp1.o
 obj-$(CONFIG_VIDEO_RENESAS_JPU)+= rcar_jpu.o
+obj-$(CONFIG_VIDEO_RENESAS_IMR)+= rcar_imr.o
 obj-$(CONFIG_VIDEO_RENESAS_VSP1)   += vsp1/
 
 obj-y  += omap/
Index: