Re: [PATCH v6] media: platform: Renesas IMR driver

2017-07-08 Thread Konstantin Kozhevnikov

Hello all,

the sample is made publicly available, and can be taken from 
https://github.com/CogentEmbedded/imr-sv-utest/blob/master/utest/utest-imr.c.


It doesn't show how luminance/chrominance correction actually works, 
however. That feature has been tested once a while ago, and probably we 
are going to release that soon.


Regarding usage of "chromacity" word in the comments, I actually meant 
"chrominance" or "chroma". The difference for non-native speaker is 
probably a bit vague, just like one between "luminance" and 
"luminosity". In the R-Car manual it is referred to as "hue", but what 
is meant is the "luma" and "chroma". These short forms seem a bit weird 
to me, hence I was using the words "luminance" and "chromacity". If 
that's confusing, I don't mind them be replaced with just "luma"/"chroma".


For documentation part, I am not 100% sure Renesas is okay with 
disclosing each and every detail, it might be the case we should obtain 
a permit from their legals. Still, I believe the person who is about to 
use the driver is having an access to at least Technical Reference 
Manual of R-Car Gen2/3, so adding a reference to a chapter in TRM will 
most likely be sufficient.


The question about usage of "user-pointer" buffers (V4L2_MEMORY_USERPTR) 
is a bit confusing. Indeed, right now we are using that scheme, though I 
wouldn't say we are absolutely required to do that. Specifically, we are 
allocating the contiguous buffers using Renesas' proprietary "mmngr" 
driver (it's not a rocket science thing, but it's made proprietary for 
some reason). Then we are using the buffers for various persons, both in 
EGL and in IMR. I guess we are okay with moving completely to DMA-fd 
(given the fact we have an accompanying driver "mmngrbuf" which serves 
for translation of memory pointers to DMA-fds for further cross-process 
sharing and stuff). I mean, if USERPTR is tagged as an obsolete / 
deprecated function, we are fine with dropping it. However, there is one 
thing I'd like to understand from V4L2 experts. I do see sometimes 
(during application closing or shortly after it) the bunches of warnings 
from kernel regarding "corrupted" MMU state (don't recall exactly, but 
it sounds like page which is supposed to be free gets somehow 
corrupted). Is that something that is related to (mis)use of USERPTR? I 
was trying to find out if there is any memory corruption caused by 
application logic, came to conclusion it's not. To me it looks like a 
race condition between unmapping of VMAs and V4L2 buffers deallocation 
which yields sometimes unpredictable results. Can you please provide 
some details about possible issues with usage of USERPTR with 
DMA-contiguous buffer driver, it would be good to find a match.


(Sorry, it got pretty long)

Sincerely,
Kostya

On 06/07/17 21:16, Sergei Shtylyov wrote:

Hello!

On 07/03/2017 03:43 PM, Hans Verkuil wrote:


Index: media_tree/Documentation/media/v4l-drivers/rcar_imr.rst
===
--- /dev/null
+++ media_tree/Documentation/media/v4l-drivers/rcar_imr.rst
@@ -0,0 +1,86 @@
+Renesas R-Car Image Rendeder (IMR) Driver


Rendeder -> Renderer


   Oops, sorry. :-)


+=
+
+This file documents some driver-specific aspects of the IMR driver, 
such as

+driver-specific ioctls.
+
+The ioctl reference
+~~~
+
+VIDIOC_IMR_MESH - Set mapping data
+^^
+
+Argument: struct imr_map_desc
+
+**Description**:
+
+This ioctl sets up the mesh using which the input frames will be


s/using/through/

+transformed into the output frames. The mesh can be strictly 
rectangular
+(when IMR_MAP_MESH bit is set in imr_map_desc::type) or 
arbitrary (when

+that bit is not set).
+
+A rectangular mesh consists of the imr_mesh structure followed 
by M*N
+vertex objects (where M is imr_mesh::rows and N is 
imr_mesh::columns).

+In case either IMR_MAP_AUTOSG or IMR_MAP_AUTODG bits were set in
+imr_map_desc::type, imr_mesh::{x|y}0 specify the coordinates of 
the top
+left corner of the auto-generated mesh and imr_mesh::d{x|y} 
specify the

+mesh's X/Y steps.


What if any of the other types are used like IMR_MAP_LUCE?


   IMR_MAP_LUCE only affects the vertex object.


Is this documented in a Renesas datasheet?


   Yes.


If so, add a reference to that in this
documentation.


   Unfortunately it's not publicly available.


+
+An arbitrary mesh consists of the imr_vbo structure followed by N
+triangle objects (where N is imr_vbo::num), consisting of 3 vertex
+objects each.
+
+A vertex object has a complex structure:
+
+.. code-block:: none
+
+__u16vvertical   \ source coordinates (only present
+__u16uhorizontal / if IMR_MAP_AUTOSG isn't set)
+__u16Yvertical   \ destination coordinates (only here
+__u16Xhorizontal / if IMR_MAP_AUTODG isn't set)

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

2017-02-13 Thread 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 <konstantin.kozhevni...@cogentembedded.com>

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
<konstantin.kozhevni...@cogentembedded.com> Signed-off-by: Sergei Shtylyov
<sergei.shtyl...@cogentembedded.com>

---
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(+)