Re: [PATCH v2 6/6] staging: media: omap4iss: ipipe: Expose the RGB2RGB blending matrix

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Hans,

On Wednesday 28 January 2015 14:18:20 Laurent Pinchart wrote:
> On Wednesday 28 January 2015 11:27:02 Hans Verkuil wrote:
> > On 01/28/15 10:17, Laurent Pinchart wrote:
> >> Expose the module as two controls, one for the 3x3 multiplier matrix and
> >> one for the 3x1 offset vector.
> >> 
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> ---
> >> 
> >>  drivers/staging/media/omap4iss/iss_ipipe.c | 129 +++++++++++++++++++++-
> >>  drivers/staging/media/omap4iss/iss_ipipe.h |  17 ++++
> >>  2 files changed, 144 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/staging/media/omap4iss/iss_ipipe.c
> >> b/drivers/staging/media/omap4iss/iss_ipipe.c index 73b165e..624c5d2
> >> 100644
> >> --- a/drivers/staging/media/omap4iss/iss_ipipe.c
> >> +++ b/drivers/staging/media/omap4iss/iss_ipipe.c
> >> @@ -119,6 +119,105 @@ static void ipipe_configure(struct
> >> iss_ipipe_device *ipipe)
> >>  }
> >>  
> >>  /* --------------------------------------------------------------------
> >> + * V4L2 controls
> >> + */
> >> +
> >> +#define OMAP4ISS_IPIPE_CID_BASE			(V4L2_CID_USER_BASE | 0xf000)
> > 
> > Private control ranges should be reserved in uapi/linux/v4l2-controls.h.
> > 
> > See e.g. V4L2_CID_USER_SAA7134_BASE.
> 
> My bad, I'll fix that.
> 
> >> +#define OMAP4ISS_IPIPE_CID_RGB2RGB_MULT		(OMAP4ISS_IPIPE_CID_BASE
> >> + 0)
> >> +#define OMAP4ISS_IPIPE_CID_RGB2RGB_OFFSET	(OMAP4ISS_IPIPE_CID_BASE
> >> + 1)
> 
> > Can you give some information how the values are interpreted? That should
> > be documented anyway, but I would like to see how this compares to the
> > adv drivers. This is something that we might want to make available as
> > standard controls. I will have to think about that a bit more.
> 
> Sure.
> 
> http://www.ti.com/lit/pdf/swpu235, section 8.3.3.4.6, page 1863.
> 
> /       \   /                         \   /      \   /          \
> | R_out |   | gain_RR gain_GR gain_BR |   | R_in |   | offset_R |
> | G_out | = | gain_RG gain_GG gain_BG | x | G_in | + | offset_G |
> | B_out |   | gain_RB gain_GB gain_BB |   | B_in |   | offset_B |
> \       /   \                         /   \      /   \          /
> 
> The two controls correspond to the multiplication matrix and offset vector.
> Coefficients are stored in 16 bits each and expressed as S3.8 (-4 to +3.996)
> for the gains and S11 (-1024 to 1023) for the offsets.
> 
> Note that the ISS IPIPE has two RGB to RGB blending matrices as shown on
> figure 8-132, page 1859. This patch implements support for the first one
> only. We should probably consider how to expose the second one as well.

Have you had a chance to compare this to the ADV* drivers ?

-- 
Regards,

Laurent Pinchart