* [PATCHv2 0/2] v4l: vsp1: Add HGT support @ 2016-09-06 14:38 Niklas Söderlund 2016-09-06 14:38 ` [PATCHv2 1/2] v4l: Define a pixel format for the R-Car VSP1 2-D histogram engine Niklas Söderlund 2016-09-06 14:38 ` [PATCHv2 2/2] v4l: vsp1: Add HGT support Niklas Söderlund 0 siblings, 2 replies; 7+ messages in thread From: Niklas Söderlund @ 2016-09-06 14:38 UTC (permalink / raw) To: linux-renesas-soc, linux-media, laurent.pinchart Cc: corbet, mchehab, sakari.ailus, hans.verkuil, Niklas Söderlund Hi, This series add support for the VSP1 2-D histogram engine HGT. It's based on top of Laurent Pinchart tree at git://linuxtv.org/pinchartl/media.git hgo. And depends on Laurents patch '[PATCH] v4l: vsp1: Move subdev operations from HGO to common histogram code'. It is tested on Koelsch using a modified vsp-tests suite package, modifications can be found at https://git.ragnatech.se/vsp-tests hgt. * Changes since v1 - Rebased on top of Laurents patch which made all subdev operations common for HGO and HGT. This removed a lot of code that is now shared. - Removed the Hue area configuration for the histogram pixel format. These values are set by userspace so it already knows them. - Updated pixel format documentation after input from Laurent. - Better aligned the code to the existing VSP code base. - Simplify check that hue areas are valid for the hardware. - Fixed spelling. Niklas Söderlund (2): v4l: Define a pixel format for the R-Car VSP1 2-D histogram engine v4l: vsp1: Add HGT support Documentation/media/uapi/v4l/meta-formats.rst | 1 + .../media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst | 122 ++++++++++++ drivers/media/platform/vsp1/Makefile | 2 +- drivers/media/platform/vsp1/vsp1.h | 3 + drivers/media/platform/vsp1/vsp1_drv.c | 33 +++- drivers/media/platform/vsp1/vsp1_entity.c | 33 +++- drivers/media/platform/vsp1/vsp1_entity.h | 1 + drivers/media/platform/vsp1/vsp1_hgt.c | 217 +++++++++++++++++++++ drivers/media/platform/vsp1/vsp1_hgt.h | 42 ++++ drivers/media/platform/vsp1/vsp1_pipe.c | 16 ++ drivers/media/platform/vsp1/vsp1_pipe.h | 2 + drivers/media/platform/vsp1/vsp1_regs.h | 9 + drivers/media/platform/vsp1/vsp1_video.c | 10 +- drivers/media/v4l2-core/v4l2-ioctl.c | 1 + include/uapi/linux/videodev2.h | 3 +- 15 files changed, 478 insertions(+), 17 deletions(-) create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.c create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.h -- 2.9.3 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCHv2 1/2] v4l: Define a pixel format for the R-Car VSP1 2-D histogram engine 2016-09-06 14:38 [PATCHv2 0/2] v4l: vsp1: Add HGT support Niklas Söderlund @ 2016-09-06 14:38 ` Niklas Söderlund 2016-09-06 14:54 ` Laurent Pinchart 2016-09-06 14:38 ` [PATCHv2 2/2] v4l: vsp1: Add HGT support Niklas Söderlund 1 sibling, 1 reply; 7+ messages in thread From: Niklas Söderlund @ 2016-09-06 14:38 UTC (permalink / raw) To: linux-renesas-soc, linux-media, laurent.pinchart Cc: corbet, mchehab, sakari.ailus, hans.verkuil, Niklas Söderlund The format is used on the R-Car VSP1 video queues that carry 2-D histogram statistics data. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- Documentation/media/uapi/v4l/meta-formats.rst | 1 + .../media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst | 122 +++++++++++++++++++++ drivers/media/v4l2-core/v4l2-ioctl.c | 1 + include/uapi/linux/videodev2.h | 3 +- 4 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst diff --git a/Documentation/media/uapi/v4l/meta-formats.rst b/Documentation/media/uapi/v4l/meta-formats.rst index 05ab91e..01e24e3 100644 --- a/Documentation/media/uapi/v4l/meta-formats.rst +++ b/Documentation/media/uapi/v4l/meta-formats.rst @@ -13,3 +13,4 @@ These formats are used for the :ref:`metadata` interface only. :maxdepth: 1 pixfmt-meta-vsp1-hgo + pixfmt-meta-vsp1-hgt diff --git a/Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst b/Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst new file mode 100644 index 0000000..0393148 --- /dev/null +++ b/Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst @@ -0,0 +1,122 @@ +.. -*- coding: utf-8; mode: rst -*- + +.. _v4l2-meta-fmt-vsp1-hgt: + +******************************* +V4L2_META_FMT_VSP1_HGT ('VSPT') +******************************* + +*man V4L2_META_FMT_VSP1_HGT(2)* + +Renesas R-Car VSP1 2-D Histogram Data + + +Description +=========== + +This format describes histogram data generated by the Renesas R-Car VSP1 +2-D Histogram (HGT) engine. + +The VSP1 HGT is a histogram computation engine that operates on HSV +data. It operates on a possibly cropped and subsampled input image and +computes the sum, maximum and minimum of the S component as well as a +weighted frequency histogram based on the H and S components. + +The histogram is a matrix of 6 Hue and 32 Saturation buckets, 192 in +total. Each HSV value is added to one or more buckets with a weight +between 1 and 16 depending on the Hue areas configuration. Finding the +corresponding buckets is done by inspecting the H and S value independently. + +The Saturation position **n** (0 - 31) of the bucket in the matrix is +found by the expression: + + n = S / 8 + +The Hue position **m** (0 - 5) of the bucket in the matrix depends on +how the HGT Hue areas are configured. There are 6 user configurable Hue +Areas which can be configured to cover overlapping Hue values: + +:: + + Area 0 Area 1 Area 2 Area 3 Area 4 Area 5 + ________ ________ ________ ________ ________ ________ + \ /| |\ /| |\ /| |\ /| |\ /| |\ /| |\ / + \ / | | \ / | | \ / | | \ / | | \ / | | \ / | | \ / + X | | X | | X | | X | | X | | X | | X + / \ | | / \ | | / \ | | / \ | | / \ | | / \ | | / \ + / \| |/ \| |/ \| |/ \| |/ \| |/ \| |/ \ + 5U 0L 0U 1L 1U 2L 2U 3L 3U 4L 4U 5L 5U 0L + <0..............................Hue Value............................255> + +When two consecutive areas don't overlap (n+1L is equal to nU) the boundary +value is considered as part of the lower area. + +Pixels with a hue value included in the centre of an area (between nL and nU +included) are are attributed to that single area and given a weight of 16. +Pixels with a hue value included in the overlapping region between two areas +(between n+1L and nU excluded) are attributed to both areas and given a weight +for each of these areas proportional to their position along the diagonal +lines (rounded down)." + +The Hue area setup must match one of the following constrains: + +:: + + 0L <= 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U + +:: + + 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U <= 0L + +**Byte Order.** +All data is stored in memory in little endian format. Each cell in the tables +contains one byte. + +.. flat-table:: VSP1 HGT Data - (776 bytes) + :header-rows: 2 + :stub-columns: 0 + + * - Offset + - :cspan:`4` Memory + * - + - [31:24] + - [23:16] + - [15:8] + - [7:0] + * - 0 + - - + - S max [7:0] + - - + - S min [7:0] + * - 4 + - :cspan:`4` S sum [31:0] + * - 8 + - :cspan:`4` Histogram bucket (m=0, n=0) [31:0] + * - 12 + - :cspan:`4` Histogram bucket (m=0, n=1) [31:0] + * - + - :cspan:`4` ... + * - 132 + - :cspan:`4` Histogram bucket (m=0, n=31) [31:0] + * - 136 + - :cspan:`4` Histogram bucket (m=1, n=0) [31:0] + * - + - :cspan:`4` ... + * - 264 + - :cspan:`4` Histogram bucket (m=2, n=0) [31:0] + * - + - :cspan:`4` ... + * - 392 + - :cspan:`4` Histogram bucket (m=3, n=0) [31:0] + * - + - :cspan:`4` ... + * - 520 + - :cspan:`4` Histogram bucket (m=4, n=0) [31:0] + * - + - :cspan:`4` ... + * - 648 + - :cspan:`4` Histogram bucket (m=5, n=0) [31:0] + * - + - :cspan:`4` ... + * - 772 + - :cspan:`4` Histogram bucket (m=5, n=31) [31:0] diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index b7f7d5f..f459c4f 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -1259,6 +1259,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) case V4L2_SDR_FMT_CS14LE: descr = "Complex S14LE"; break; case V4L2_SDR_FMT_RU12LE: descr = "Real U12LE"; break; case V4L2_META_FMT_VSP1_HGO: descr = "R-Car VSP1 1-D Histogram"; break; + case V4L2_META_FMT_VSP1_HGT: descr = "R-Car VSP1 2-D Histogram"; break; default: /* Compressed formats */ diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 1dbe52a..c8c046c 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -638,7 +638,8 @@ struct v4l2_pix_format { #define V4L2_SDR_FMT_RU12LE v4l2_fourcc('R', 'U', '1', '2') /* real u12le */ /* Meta-data formats */ -#define V4L2_META_FMT_VSP1_HGO v4l2_fourcc('V', 'S', 'P', 'H') /* R-Car VSP1 Histogram */ +#define V4L2_META_FMT_VSP1_HGO v4l2_fourcc('V', 'S', 'P', 'H') /* R-Car VSP1 1-D Histogram */ +#define V4L2_META_FMT_VSP1_HGT v4l2_fourcc('V', 'S', 'P', 'T') /* R-Car VSP1 2-D Histogram */ /* priv field value to indicates that subsequent fields are valid. */ #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe -- 2.9.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCHv2 1/2] v4l: Define a pixel format for the R-Car VSP1 2-D histogram engine 2016-09-06 14:38 ` [PATCHv2 1/2] v4l: Define a pixel format for the R-Car VSP1 2-D histogram engine Niklas Söderlund @ 2016-09-06 14:54 ` Laurent Pinchart 0 siblings, 0 replies; 7+ messages in thread From: Laurent Pinchart @ 2016-09-06 14:54 UTC (permalink / raw) To: Niklas Söderlund Cc: linux-renesas-soc, linux-media, corbet, mchehab, sakari.ailus, hans.verkuil Hi Niklas, Thank you for the patch. On Tuesday 06 Sep 2016 16:38:55 Niklas Söderlund wrote: > The format is used on the R-Car VSP1 video queues that carry > 2-D histogram statistics data. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > Documentation/media/uapi/v4l/meta-formats.rst | 1 + > .../media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst | 122 ++++++++++++++++++ > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > include/uapi/linux/videodev2.h | 3 +- > 4 files changed, 126 insertions(+), 1 deletion(-) > create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst > > diff --git a/Documentation/media/uapi/v4l/meta-formats.rst > b/Documentation/media/uapi/v4l/meta-formats.rst index 05ab91e..01e24e3 > 100644 > --- a/Documentation/media/uapi/v4l/meta-formats.rst > +++ b/Documentation/media/uapi/v4l/meta-formats.rst > @@ -13,3 +13,4 @@ These formats are used for the :ref:`metadata` interface > only. > :maxdepth: 1 > > pixfmt-meta-vsp1-hgo > + pixfmt-meta-vsp1-hgt > diff --git a/Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst > b/Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst new file mode > 100644 > index 0000000..0393148 > --- /dev/null > +++ b/Documentation/media/uapi/v4l/pixfmt-meta-vsp1-hgt.rst > @@ -0,0 +1,122 @@ > +.. -*- coding: utf-8; mode: rst -*- > + > +.. _v4l2-meta-fmt-vsp1-hgt: > + > +******************************* > +V4L2_META_FMT_VSP1_HGT ('VSPT') > +******************************* > + > +*man V4L2_META_FMT_VSP1_HGT(2)* > + > +Renesas R-Car VSP1 2-D Histogram Data > + > + > +Description > +=========== > + > +This format describes histogram data generated by the Renesas R-Car VSP1 > +2-D Histogram (HGT) engine. > + > +The VSP1 HGT is a histogram computation engine that operates on HSV > +data. It operates on a possibly cropped and subsampled input image and > +computes the sum, maximum and minimum of the S component as well as a > +weighted frequency histogram based on the H and S components. > + > +The histogram is a matrix of 6 Hue and 32 Saturation buckets, 192 in > +total. Each HSV value is added to one or more buckets with a weight > +between 1 and 16 depending on the Hue areas configuration. Finding the > +corresponding buckets is done by inspecting the H and S value > independently. + > +The Saturation position **n** (0 - 31) of the bucket in the matrix is > +found by the expression: > + > + n = S / 8 > + > +The Hue position **m** (0 - 5) of the bucket in the matrix depends on > +how the HGT Hue areas are configured. There are 6 user configurable Hue > +Areas which can be configured to cover overlapping Hue values: > + > +:: > + > + Area 0 Area 1 Area 2 Area 3 Area 4 > Area 5 + ________ ________ ________ ________ > ________ ________ + \ /| |\ /| |\ /| |\ /| > |\ /| |\ /| |\ / + \ / | | \ / | | \ / | > | \ / | | \ / | | \ / | | \ / + X | | X | | > X | | X | | X | | X | | X + / \ | | / > \ | | / \ | | / \ | | / \ | | / \ | | / \ + / > \| |/ \| |/ \| |/ \| |/ \| |/ \| |/ > \ + 5U 0L 0U 1L 1U 2L 2U 3L 3U 4L 4U > 5L 5U 0L + <0..............................Hue > Value............................255> + > +When two consecutive areas don't overlap (n+1L is equal to nU) the boundary > +value is considered as part of the lower area. > + > +Pixels with a hue value included in the centre of an area (between nL and > nU +included) are are attributed to that single area and given a weight of s/are are/are/ > 16. +Pixels with a hue value included in the overlapping region between two > areas +(between n+1L and nU excluded) are attributed to both areas and > given a weight +for each of these areas proportional to their position > along the diagonal +lines (rounded down)." s/"// Apart from that, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> and applied to my tree with the above fixes. > + > +The Hue area setup must match one of the following constrains: > + > +:: > + > + 0L <= 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U > + > +:: > + > + 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U <= 0L > + > +**Byte Order.** > +All data is stored in memory in little endian format. Each cell in the > tables +contains one byte. > + > +.. flat-table:: VSP1 HGT Data - (776 bytes) > + :header-rows: 2 > + :stub-columns: 0 > + > + * - Offset > + - :cspan:`4` Memory > + * - > + - [31:24] > + - [23:16] > + - [15:8] > + - [7:0] > + * - 0 > + - - > + - S max [7:0] > + - - > + - S min [7:0] > + * - 4 > + - :cspan:`4` S sum [31:0] > + * - 8 > + - :cspan:`4` Histogram bucket (m=0, n=0) [31:0] > + * - 12 > + - :cspan:`4` Histogram bucket (m=0, n=1) [31:0] > + * - > + - :cspan:`4` ... > + * - 132 > + - :cspan:`4` Histogram bucket (m=0, n=31) [31:0] > + * - 136 > + - :cspan:`4` Histogram bucket (m=1, n=0) [31:0] > + * - > + - :cspan:`4` ... > + * - 264 > + - :cspan:`4` Histogram bucket (m=2, n=0) [31:0] > + * - > + - :cspan:`4` ... > + * - 392 > + - :cspan:`4` Histogram bucket (m=3, n=0) [31:0] > + * - > + - :cspan:`4` ... > + * - 520 > + - :cspan:`4` Histogram bucket (m=4, n=0) [31:0] > + * - > + - :cspan:`4` ... > + * - 648 > + - :cspan:`4` Histogram bucket (m=5, n=0) [31:0] > + * - > + - :cspan:`4` ... > + * - 772 > + - :cspan:`4` Histogram bucket (m=5, n=31) [31:0] > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c > b/drivers/media/v4l2-core/v4l2-ioctl.c index b7f7d5f..f459c4f 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -1259,6 +1259,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) > case V4L2_SDR_FMT_CS14LE: descr = "Complex S14LE"; break; > case V4L2_SDR_FMT_RU12LE: descr = "Real U12LE"; break; > case V4L2_META_FMT_VSP1_HGO: descr = "R-Car VSP1 1-D Histogram"; break; > + case V4L2_META_FMT_VSP1_HGT: descr = "R-Car VSP1 2-D Histogram"; break; > > default: > /* Compressed formats */ > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 1dbe52a..c8c046c 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -638,7 +638,8 @@ struct v4l2_pix_format { > #define V4L2_SDR_FMT_RU12LE v4l2_fourcc('R', 'U', '1', '2') /* real > u12le */ > > /* Meta-data formats */ > -#define V4L2_META_FMT_VSP1_HGO v4l2_fourcc('V', 'S', 'P', 'H') /* R-Car > VSP1 Histogram */ +#define V4L2_META_FMT_VSP1_HGO v4l2_fourcc('V', 'S', > 'P', 'H') /* R-Car VSP1 1-D Histogram */ +#define V4L2_META_FMT_VSP1_HGT > v4l2_fourcc('V', 'S', 'P', 'T') /* R-Car VSP1 2-D Histogram */ > > /* priv field value to indicates that subsequent fields are valid. */ > #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCHv2 2/2] v4l: vsp1: Add HGT support 2016-09-06 14:38 [PATCHv2 0/2] v4l: vsp1: Add HGT support Niklas Söderlund 2016-09-06 14:38 ` [PATCHv2 1/2] v4l: Define a pixel format for the R-Car VSP1 2-D histogram engine Niklas Söderlund @ 2016-09-06 14:38 ` Niklas Söderlund 2016-09-06 19:59 ` Laurent Pinchart 1 sibling, 1 reply; 7+ messages in thread From: Niklas Söderlund @ 2016-09-06 14:38 UTC (permalink / raw) To: linux-renesas-soc, linux-media, laurent.pinchart Cc: corbet, mchehab, sakari.ailus, hans.verkuil, Niklas Söderlund The HGT is a Histogram Generator Two-Dimensions. It computes a weighted frequency histograms for hue and saturation areas over a configurable region of the image with optional subsampling. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/media/platform/vsp1/Makefile | 2 +- drivers/media/platform/vsp1/vsp1.h | 3 + drivers/media/platform/vsp1/vsp1_drv.c | 33 ++++- drivers/media/platform/vsp1/vsp1_entity.c | 33 +++-- drivers/media/platform/vsp1/vsp1_entity.h | 1 + drivers/media/platform/vsp1/vsp1_hgt.c | 217 ++++++++++++++++++++++++++++++ drivers/media/platform/vsp1/vsp1_hgt.h | 42 ++++++ drivers/media/platform/vsp1/vsp1_pipe.c | 16 +++ drivers/media/platform/vsp1/vsp1_pipe.h | 2 + drivers/media/platform/vsp1/vsp1_regs.h | 9 ++ drivers/media/platform/vsp1/vsp1_video.c | 10 +- 11 files changed, 352 insertions(+), 16 deletions(-) create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.c create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.h diff --git a/drivers/media/platform/vsp1/Makefile b/drivers/media/platform/vsp1/Makefile index 8ab6a06..a33afc3 100644 --- a/drivers/media/platform/vsp1/Makefile +++ b/drivers/media/platform/vsp1/Makefile @@ -3,7 +3,7 @@ vsp1-y += vsp1_dl.o vsp1_drm.o vsp1_video.o vsp1-y += vsp1_rpf.o vsp1_rwpf.o vsp1_wpf.o vsp1-y += vsp1_clu.o vsp1_hsit.o vsp1_lut.o vsp1-y += vsp1_bru.o vsp1_sru.o vsp1_uds.o -vsp1-y += vsp1_hgo.o vsp1_histo.o +vsp1-y += vsp1_hgo.o vsp1_hgt.o vsp1_histo.o vsp1-y += vsp1_lif.o obj-$(CONFIG_VIDEO_RENESAS_VSP1) += vsp1.o diff --git a/drivers/media/platform/vsp1/vsp1.h b/drivers/media/platform/vsp1/vsp1.h index 9dce3ea..012ce40 100644 --- a/drivers/media/platform/vsp1/vsp1.h +++ b/drivers/media/platform/vsp1/vsp1.h @@ -33,6 +33,7 @@ struct vsp1_platform_data; struct vsp1_bru; struct vsp1_clu; struct vsp1_hgo; +struct vsp1_hgt; struct vsp1_hsit; struct vsp1_lif; struct vsp1_lut; @@ -52,6 +53,7 @@ struct vsp1_uds; #define VSP1_HAS_WPF_VFLIP (1 << 5) #define VSP1_HAS_WPF_HFLIP (1 << 6) #define VSP1_HAS_HGO (1 << 7) +#define VSP1_HAS_HGT (1 << 8) struct vsp1_device_info { u32 version; @@ -74,6 +76,7 @@ struct vsp1_device { struct vsp1_bru *bru; struct vsp1_clu *clu; struct vsp1_hgo *hgo; + struct vsp1_hgt *hgt; struct vsp1_hsit *hsi; struct vsp1_hsit *hst; struct vsp1_lif *lif; diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c index 9ea4244..df97b57 100644 --- a/drivers/media/platform/vsp1/vsp1_drv.c +++ b/drivers/media/platform/vsp1/vsp1_drv.c @@ -31,6 +31,7 @@ #include "vsp1_dl.h" #include "vsp1_drm.h" #include "vsp1_hgo.h" +#include "vsp1_hgt.h" #include "vsp1_hsit.h" #include "vsp1_lif.h" #include "vsp1_lut.h" @@ -107,6 +108,7 @@ static int vsp1_create_sink_links(struct vsp1_device *vsp1, continue; if (source->type == VSP1_ENTITY_HGO || + source->type == VSP1_ENTITY_HGT || source->type == VSP1_ENTITY_LIF || source->type == VSP1_ENTITY_WPF) continue; @@ -160,6 +162,16 @@ static int vsp1_uapi_create_links(struct vsp1_device *vsp1) return ret; } + if (vsp1->hgt) { + ret = media_create_pad_link(&vsp1->hgt->histo.entity.subdev.entity, + HISTO_PAD_SOURCE, + &vsp1->hgt->histo.video.entity, 0, + MEDIA_LNK_FL_ENABLED | + MEDIA_LNK_FL_IMMUTABLE); + if (ret < 0) + return ret; + } + if (vsp1->lif) { ret = media_create_pad_link(&vsp1->wpf[0]->entity.subdev.entity, RWPF_PAD_SOURCE, @@ -301,6 +313,17 @@ static int vsp1_create_entities(struct vsp1_device *vsp1) &vsp1->entities); } + if (vsp1->info->features & VSP1_HAS_HGT && vsp1->info->uapi) { + vsp1->hgt = vsp1_hgt_create(vsp1); + if (IS_ERR(vsp1->hgt)) { + ret = PTR_ERR(vsp1->hgt); + goto done; + } + + list_add_tail(&vsp1->hgt->histo.entity.list_dev, + &vsp1->entities); + } + /* The LIF is only supported when used in conjunction with the DU, in * which case the userspace API is disabled. If the userspace API is * enabled skip the LIF, even when present. @@ -584,7 +607,8 @@ static const struct vsp1_device_info vsp1_device_infos[] = { .version = VI6_IP_VERSION_MODEL_VSPS_H2, .gen = 2, .features = VSP1_HAS_BRU | VSP1_HAS_CLU | VSP1_HAS_HGO - | VSP1_HAS_LUT | VSP1_HAS_SRU | VSP1_HAS_WPF_VFLIP, + | VSP1_HAS_HGT | VSP1_HAS_LUT | VSP1_HAS_SRU + | VSP1_HAS_WPF_VFLIP, .rpf_count = 5, .uds_count = 3, .wpf_count = 4, @@ -613,7 +637,8 @@ static const struct vsp1_device_info vsp1_device_infos[] = { .version = VI6_IP_VERSION_MODEL_VSPS_M2, .gen = 2, .features = VSP1_HAS_BRU | VSP1_HAS_CLU | VSP1_HAS_HGO - | VSP1_HAS_LUT | VSP1_HAS_SRU | VSP1_HAS_WPF_VFLIP, + | VSP1_HAS_HGT | VSP1_HAS_LUT | VSP1_HAS_SRU + | VSP1_HAS_WPF_VFLIP, .rpf_count = 5, .uds_count = 1, .wpf_count = 4, @@ -622,8 +647,8 @@ static const struct vsp1_device_info vsp1_device_infos[] = { }, { .version = VI6_IP_VERSION_MODEL_VSPI_GEN3, .gen = 3, - .features = VSP1_HAS_CLU | VSP1_HAS_HGO | VSP1_HAS_LUT - | VSP1_HAS_SRU | VSP1_HAS_WPF_HFLIP + .features = VSP1_HAS_CLU | VSP1_HAS_HGO | VSP1_HAS_HGT + | VSP1_HAS_LUT | VSP1_HAS_SRU | VSP1_HAS_WPF_HFLIP | VSP1_HAS_WPF_VFLIP, .rpf_count = 1, .uds_count = 1, diff --git a/drivers/media/platform/vsp1/vsp1_entity.c b/drivers/media/platform/vsp1/vsp1_entity.c index a6b7d87..0a0c46e 100644 --- a/drivers/media/platform/vsp1/vsp1_entity.c +++ b/drivers/media/platform/vsp1/vsp1_entity.c @@ -49,6 +49,18 @@ void vsp1_entity_route_setup(struct vsp1_entity *entity, vsp1_dl_list_write(dl, VI6_DPR_HGO_SMPPT, smppt); return; + } else if (entity->type == VSP1_ENTITY_HGT) { + u32 smppt; + + /* The HGT is a special case, its routing is configured on the + * sink pad. + */ + source = media_entity_to_vsp1_entity(entity->sources[0]); + smppt = (pipe->output->entity.index << VI6_DPR_SMPPT_TGW_SHIFT) + | (source->route->output << VI6_DPR_SMPPT_PT_SHIFT); + + vsp1_dl_list_write(dl, VI6_DPR_HGT_SMPPT, smppt); + return; } source = entity; @@ -302,9 +314,10 @@ static int vsp1_entity_link_setup_source(const struct media_pad *source_pad, = media_entity_to_vsp1_entity(sink_pad->entity); /* Fan-out is limited to one for the normal data path plus an - * optional HGO. We ignore the HGO here. + * optional HGO or HGT. We ignore the HGO and HGT here. */ - if (sink->type != VSP1_ENTITY_HGO) { + if (sink->type != VSP1_ENTITY_HGO && + sink->type != VSP1_ENTITY_HGT) { if (source->sink) return -EBUSY; source->sink = sink_pad->entity; @@ -357,10 +370,10 @@ int vsp1_entity_link_setup(struct media_entity *entity, * links originating or terminating at that pad until an enabled link is found. * * Our link setup implementation guarantees that the output fan-out will not be - * higher than one for the data pipelines, except for the link to the HGO that - * can be enabled in addition to a regular data link. When traversing outgoing - * links this function ignores HGO entities and should thus be used in place of - * the generic media_entity_remote_pad() function when traversing data + * higher than one for the data pipelines, except for the link to the HGO or HGT + * that can be enabled in addition to a regular data link. When traversing + * outgoing links this function ignores HGO entities and should thus be used in + * place of the generic media_entity_remote_pad() function when traversing data * pipelines. * * Return a pointer to the pad at the remote end of the first found enabled @@ -376,19 +389,20 @@ struct media_pad *vsp1_entity_remote_pad(struct media_pad *pad) if (!(link->flags & MEDIA_LNK_FL_ENABLED)) continue; - /* If we're the sink the source will never be an HGO. */ + /* If we're the sink the source will never be an HGO or HGT. */ if (link->sink == pad) return link->source; if (link->source != pad) continue; - /* If the sink isn't a subdevice it can't be an HGO. */ + /* If the sink isn't a subdevice it can't be an HGO or HGT. */ if (!is_media_entity_v4l2_subdev(link->sink->entity)) return link->sink; entity = media_entity_to_vsp1_entity(link->sink->entity); - if (entity->type != VSP1_ENTITY_HGO) + if (entity->type != VSP1_ENTITY_HGO && + entity->type != VSP1_ENTITY_HGT) return link->sink; } @@ -423,6 +437,7 @@ static const struct vsp1_route vsp1_routes[] = { VI6_DPR_NODE_BRU_IN(4) }, VI6_DPR_NODE_BRU_OUT }, VSP1_ENTITY_ROUTE(CLU), { VSP1_ENTITY_HGO, 0, 0, { 0, }, 0 }, + { VSP1_ENTITY_HGT, 0, 0, { 0, }, 0 }, VSP1_ENTITY_ROUTE(HSI), VSP1_ENTITY_ROUTE(HST), { VSP1_ENTITY_LIF, 0, 0, { VI6_DPR_NODE_LIF, }, VI6_DPR_NODE_LIF }, diff --git a/drivers/media/platform/vsp1/vsp1_entity.h b/drivers/media/platform/vsp1/vsp1_entity.h index 4fc2cd1..0682400 100644 --- a/drivers/media/platform/vsp1/vsp1_entity.h +++ b/drivers/media/platform/vsp1/vsp1_entity.h @@ -26,6 +26,7 @@ enum vsp1_entity_type { VSP1_ENTITY_BRU, VSP1_ENTITY_CLU, VSP1_ENTITY_HGO, + VSP1_ENTITY_HGT, VSP1_ENTITY_HSI, VSP1_ENTITY_HST, VSP1_ENTITY_LIF, diff --git a/drivers/media/platform/vsp1/vsp1_hgt.c b/drivers/media/platform/vsp1/vsp1_hgt.c new file mode 100644 index 0000000..4e3f762 --- /dev/null +++ b/drivers/media/platform/vsp1/vsp1_hgt.c @@ -0,0 +1,217 @@ +/* + * vsp1_hgt.c -- R-Car VSP1 Histogram Generator 2D + * + * Copyright (C) 2016 Renesas Electronics Corporation + * + * Contact: Niklas Söderlund (niklas.soderlund@ragnatech.se) + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include <linux/device.h> +#include <linux/gfp.h> + +#include <media/v4l2-subdev.h> +#include <media/videobuf2-vmalloc.h> + +#include "vsp1.h" +#include "vsp1_dl.h" +#include "vsp1_hgt.h" + +#define HGT_DATA_SIZE ((2 + 6 * 32) * 4) + +/* ----------------------------------------------------------------------------- + * Device Access + */ + +static inline u32 vsp1_hgt_read(struct vsp1_hgt *hgt, u32 reg) +{ + return vsp1_read(hgt->histo.entity.vsp1, reg); +} + +static inline void vsp1_hgt_write(struct vsp1_hgt *hgt, struct vsp1_dl_list *dl, + u32 reg, u32 data) +{ + vsp1_dl_list_write(dl, reg, data); +} + +/* ----------------------------------------------------------------------------- + * Frame End Handler + */ + +void vsp1_hgt_frame_end(struct vsp1_entity *entity) +{ + struct vsp1_hgt *hgt = to_hgt(&entity->subdev); + struct vsp1_histogram_buffer *buf; + unsigned int m; + unsigned int n; + u32 *data; + + buf = vsp1_histogram_buffer_get(&hgt->histo); + if (!buf) + return; + + data = buf->addr; + + *data++ = vsp1_hgt_read(hgt, VI6_HGT_MAXMIN); + *data++ = vsp1_hgt_read(hgt, VI6_HGT_SUM); + + for (m = 0; m < 6; ++m) + for (n = 0; n < 32; ++n) + *data++ = vsp1_hgt_read(hgt, VI6_HGT_HISTO(m, n)); + + vsp1_histogram_buffer_complete(&hgt->histo, buf, HGT_DATA_SIZE); +} + +/* ----------------------------------------------------------------------------- + * Controls + */ + +#define V4L2_CID_VSP1_HGT_HUE_AREAS (V4L2_CID_USER_BASE | 0x1001) + +static int hgt_hue_areas_s_ctrl(struct v4l2_ctrl *ctrl) +{ + struct vsp1_hgt *hgt = container_of(ctrl->handler, struct vsp1_hgt, + ctrls); + u8 *value = ctrl->p_new.p_u8; + unsigned int i; + bool ok = true; + + /* + * Make sure values meet one of two possible hardware constrains + * 0L <= 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U + * 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U <= 0L + */ + + if ((value[0] > value[1]) && (value[11] > value[0])) + ok = false; + for (i = 1; i < (HGT_NUM_HUE_AREAS * 2) - 1; ++i) + if (value[i] > value[i+1]) + ok = false; + + /* Values do not match hardware, adjust to valid settings. */ + if (!ok) { + for (i = 0; i < (HGT_NUM_HUE_AREAS * 2) - 1; ++i) { + if (value[i] > value[i+1]) + value[i] = value[i+1]; + } + } + + memcpy(hgt->hue_areas, ctrl->p_new.p_u8, sizeof(hgt->hue_areas)); + + return 0; +} + +static const struct v4l2_ctrl_ops hgt_hue_areas_ctrl_ops = { + .s_ctrl = hgt_hue_areas_s_ctrl, +}; + +static const struct v4l2_ctrl_config hgt_hue_areas = { + .ops = &hgt_hue_areas_ctrl_ops, + .id = V4L2_CID_VSP1_HGT_HUE_AREAS, + .name = "Boundary Values for Hue Area", + .type = V4L2_CTRL_TYPE_U8, + .min = 0, + .max = 255, + .def = 0, + .step = 1, + .dims = { 12 }, +}; + +/* ----------------------------------------------------------------------------- + * VSP1 Entity Operations + */ + +static void hgt_configure(struct vsp1_entity *entity, + struct vsp1_pipeline *pipe, + struct vsp1_dl_list *dl, bool full) +{ + struct vsp1_hgt *hgt = to_hgt(&entity->subdev); + struct v4l2_rect *compose; + struct v4l2_rect *crop; + unsigned int hratio; + unsigned int vratio; + u8 lower; + u8 upper; + unsigned int i; + + if (!full) + return; + + crop = vsp1_entity_get_pad_selection(entity, entity->config, + HISTO_PAD_SINK, V4L2_SEL_TGT_CROP); + compose = vsp1_entity_get_pad_selection(entity, entity->config, + HISTO_PAD_SINK, + V4L2_SEL_TGT_COMPOSE); + + vsp1_hgt_write(hgt, dl, VI6_HGT_REGRST, VI6_HGT_REGRST_RCLEA); + + vsp1_hgt_write(hgt, dl, VI6_HGT_OFFSET, + (crop->left << VI6_HGT_OFFSET_HOFFSET_SHIFT) | + (crop->top << VI6_HGT_OFFSET_VOFFSET_SHIFT)); + vsp1_hgt_write(hgt, dl, VI6_HGT_SIZE, + (crop->width << VI6_HGT_SIZE_HSIZE_SHIFT) | + (crop->height << VI6_HGT_SIZE_VSIZE_SHIFT)); + + mutex_lock(hgt->ctrls.lock); + for (i = 0; i < HGT_NUM_HUE_AREAS; ++i) { + lower = hgt->hue_areas[i*2 + 0]; + upper = hgt->hue_areas[i*2 + 1]; + vsp1_hgt_write(hgt, dl, VI6_HGT_HUE_AREA(i), + (lower << VI6_HGT_HUE_AREA_LOWER_SHIFT) | + (upper << VI6_HGT_HUE_AREA_UPPER_SHIFT)); + } + mutex_unlock(hgt->ctrls.lock); + + hratio = crop->width * 2 / compose->width / 3; + vratio = crop->height * 2 / compose->height / 3; + vsp1_hgt_write(hgt, dl, VI6_HGT_MODE, + (hratio << VI6_HGT_MODE_HRATIO_SHIFT) | + (vratio << VI6_HGT_MODE_VRATIO_SHIFT)); +} + +static const struct vsp1_entity_operations hgt_entity_ops = { + .configure = hgt_configure, + .destroy = vsp1_histogram_destroy, +}; + +/* ----------------------------------------------------------------------------- + * Initialization and Cleanup + */ + +static const unsigned int hgt_mbus_formats[] = { + MEDIA_BUS_FMT_AHSV8888_1X32, +}; + +struct vsp1_hgt *vsp1_hgt_create(struct vsp1_device *vsp1) +{ + struct vsp1_hgt *hgt; + int ret; + + hgt = devm_kzalloc(vsp1->dev, sizeof(*hgt), GFP_KERNEL); + if (hgt == NULL) + return ERR_PTR(-ENOMEM); + + /* Initialize the control handler. */ + v4l2_ctrl_handler_init(&hgt->ctrls, 1); + v4l2_ctrl_new_custom(&hgt->ctrls, &hgt_hue_areas, NULL); + + hgt->histo.entity.subdev.ctrl_handler = &hgt->ctrls; + + /* Initialize the video device and queue for statistics data. */ + ret = vsp1_histogram_init(vsp1, &hgt->histo, VSP1_ENTITY_HGT, "hgt", + &hgt_entity_ops, hgt_mbus_formats, + ARRAY_SIZE(hgt_mbus_formats), + HGT_DATA_SIZE, V4L2_META_FMT_VSP1_HGT); + if (ret < 0) { + vsp1_entity_destroy(&hgt->histo.entity); + return ERR_PTR(ret); + } + + v4l2_ctrl_handler_setup(&hgt->ctrls); + + return hgt; +} diff --git a/drivers/media/platform/vsp1/vsp1_hgt.h b/drivers/media/platform/vsp1/vsp1_hgt.h new file mode 100644 index 0000000..83f2e13 --- /dev/null +++ b/drivers/media/platform/vsp1/vsp1_hgt.h @@ -0,0 +1,42 @@ +/* + * vsp1_hgt.h -- R-Car VSP1 Histogram Generator 2D + * + * Copyright (C) 2016 Renesas Electronics Corporation + * + * Contact: Niklas Söderlund (niklas.soderlund@ragnatech.se) + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ +#ifndef __VSP1_HGT_H__ +#define __VSP1_HGT_H__ + +#include <media/media-entity.h> +#include <media/v4l2-ctrls.h> +#include <media/v4l2-subdev.h> + +#include "vsp1_histo.h" + +struct vsp1_device; + +#define HGT_NUM_HUE_AREAS 6 + +struct vsp1_hgt { + struct vsp1_histogram histo; + + struct v4l2_ctrl_handler ctrls; + + u8 hue_areas[HGT_NUM_HUE_AREAS * 2]; +}; + +static inline struct vsp1_hgt *to_hgt(struct v4l2_subdev *subdev) +{ + return container_of(subdev, struct vsp1_hgt, histo.entity.subdev); +} + +struct vsp1_hgt *vsp1_hgt_create(struct vsp1_device *vsp1); +void vsp1_hgt_frame_end(struct vsp1_entity *hgt); + +#endif /* __VSP1_HGT_H__ */ diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c b/drivers/media/platform/vsp1/vsp1_pipe.c index 0dd7c16..052a603 100644 --- a/drivers/media/platform/vsp1/vsp1_pipe.c +++ b/drivers/media/platform/vsp1/vsp1_pipe.c @@ -24,6 +24,7 @@ #include "vsp1_dl.h" #include "vsp1_entity.h" #include "vsp1_hgo.h" +#include "vsp1_hgt.h" #include "vsp1_pipe.h" #include "vsp1_rwpf.h" #include "vsp1_uds.h" @@ -191,12 +192,19 @@ void vsp1_pipeline_reset(struct vsp1_pipeline *pipe) hgo->histo.pipe = NULL; } + if (pipe->hgt) { + struct vsp1_hgt *hgt = to_hgt(&pipe->hgt->subdev); + + hgt->histo.pipe = NULL; + } + INIT_LIST_HEAD(&pipe->entities); pipe->state = VSP1_PIPELINE_STOPPED; pipe->buffers_ready = 0; pipe->num_inputs = 0; pipe->bru = NULL; pipe->hgo = NULL; + pipe->hgt = NULL; pipe->lif = NULL; pipe->uds = NULL; } @@ -278,6 +286,11 @@ int vsp1_pipeline_stop(struct vsp1_pipeline *pipe) (7 << VI6_DPR_SMPPT_TGW_SHIFT) | (VI6_DPR_NODE_UNUSED << VI6_DPR_SMPPT_PT_SHIFT)); + if (pipe->hgt) + vsp1_write(vsp1, VI6_DPR_HGT_SMPPT, + (7 << VI6_DPR_SMPPT_TGW_SHIFT) | + (VI6_DPR_NODE_UNUSED << VI6_DPR_SMPPT_PT_SHIFT)); + v4l2_subdev_call(&pipe->output->entity.subdev, video, s_stream, 0); return ret; @@ -304,6 +317,9 @@ void vsp1_pipeline_frame_end(struct vsp1_pipeline *pipe) if (pipe->hgo) vsp1_hgo_frame_end(pipe->hgo); + if (pipe->hgt) + vsp1_hgt_frame_end(pipe->hgt); + if (pipe->frame_end) pipe->frame_end(pipe); diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h index bd42eff..740bde2 100644 --- a/drivers/media/platform/vsp1/vsp1_pipe.h +++ b/drivers/media/platform/vsp1/vsp1_pipe.h @@ -73,6 +73,7 @@ enum vsp1_pipeline_state { * @output: WPF at the output of the pipeline * @bru: BRU entity, if present * @hgo: HGO entity, if present + * @hgt: HGT entity, if present * @lif: LIF entity, if present * @uds: UDS entity, if present * @uds_input: entity at the input of the UDS, if the UDS is present @@ -99,6 +100,7 @@ struct vsp1_pipeline { struct vsp1_rwpf *output; struct vsp1_entity *bru; struct vsp1_entity *hgo; + struct vsp1_entity *hgt; struct vsp1_entity *lif; struct vsp1_entity *uds; struct vsp1_entity *uds_input; diff --git a/drivers/media/platform/vsp1/vsp1_regs.h b/drivers/media/platform/vsp1/vsp1_regs.h index d821348..d1d9af9 100644 --- a/drivers/media/platform/vsp1/vsp1_regs.h +++ b/drivers/media/platform/vsp1/vsp1_regs.h @@ -628,9 +628,17 @@ */ #define VI6_HGT_OFFSET 0x3400 +#define VI6_HGT_OFFSET_HOFFSET_SHIFT 16 +#define VI6_HGT_OFFSET_VOFFSET_SHIFT 0 #define VI6_HGT_SIZE 0x3404 +#define VI6_HGT_SIZE_HSIZE_SHIFT 16 +#define VI6_HGT_SIZE_VSIZE_SHIFT 0 #define VI6_HGT_MODE 0x3408 +#define VI6_HGT_MODE_HRATIO_SHIFT 2 +#define VI6_HGT_MODE_VRATIO_SHIFT 0 #define VI6_HGT_HUE_AREA(n) (0x340c + (n) * 4) +#define VI6_HGT_HUE_AREA_LOWER_SHIFT 16 +#define VI6_HGT_HUE_AREA_UPPER_SHIFT 0 #define VI6_HGT_LB_TH 0x3424 #define VI6_HGT_LBn_H(n) (0x3438 + (n) * 8) #define VI6_HGT_LBn_V(n) (0x342c + (n) * 8) @@ -639,6 +647,7 @@ #define VI6_HGT_SUM 0x3754 #define VI6_HGT_LB_DET 0x3758 #define VI6_HGT_REGRST 0x37fc +#define VI6_HGT_REGRST_RCLEA (1 << 0) /* ----------------------------------------------------------------------------- * LIF Control Registers diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c index 75e6e6c..7215e08 100644 --- a/drivers/media/platform/vsp1/vsp1_video.c +++ b/drivers/media/platform/vsp1/vsp1_video.c @@ -32,6 +32,7 @@ #include "vsp1_dl.h" #include "vsp1_entity.h" #include "vsp1_hgo.h" +#include "vsp1_hgt.h" #include "vsp1_pipe.h" #include "vsp1_rwpf.h" #include "vsp1_uds.h" @@ -326,7 +327,7 @@ static int vsp1_video_pipeline_build_branch(struct vsp1_pipeline *pipe, if (ret < 0) return ret; - /* The main data path doesn't include the HGO, use + /* The main data path doesn't include the HGO or HGT, use * vsp1_entity_remote_pad() to traverse the graph. */ @@ -382,7 +383,7 @@ static int vsp1_video_pipeline_build_branch(struct vsp1_pipeline *pipe, : &input->entity; } - /* Follow the source link, ignoring any HGO. */ + /* Follow the source link, ignoring any HGO or HGT. */ pad = &entity->pads[entity->source_pad]; pad = vsp1_entity_remote_pad(pad); } @@ -444,6 +445,11 @@ static int vsp1_video_pipeline_build(struct vsp1_pipeline *pipe, pipe->hgo = e; hgo->histo.pipe = pipe; + } else if (e->type == VSP1_ENTITY_HGT) { + struct vsp1_hgt *hgt = to_hgt(subdev); + + pipe->hgt = e; + hgt->histo.pipe = pipe; } } -- 2.9.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCHv2 2/2] v4l: vsp1: Add HGT support 2016-09-06 14:38 ` [PATCHv2 2/2] v4l: vsp1: Add HGT support Niklas Söderlund @ 2016-09-06 19:59 ` Laurent Pinchart 2016-09-07 10:05 ` Niklas Söderlund 0 siblings, 1 reply; 7+ messages in thread From: Laurent Pinchart @ 2016-09-06 19:59 UTC (permalink / raw) To: Niklas Söderlund Cc: linux-renesas-soc, linux-media, corbet, mchehab, sakari.ailus, hans.verkuil Hi Niklas, Thank you for the patch. On Tuesday 06 Sep 2016 16:38:56 Niklas Söderlund wrote: > The HGT is a Histogram Generator Two-Dimensions. It computes a weighted > frequency histograms for hue and saturation areas over a configurable > region of the image with optional subsampling. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/media/platform/vsp1/Makefile | 2 +- > drivers/media/platform/vsp1/vsp1.h | 3 + > drivers/media/platform/vsp1/vsp1_drv.c | 33 ++++- > drivers/media/platform/vsp1/vsp1_entity.c | 33 +++-- > drivers/media/platform/vsp1/vsp1_entity.h | 1 + > drivers/media/platform/vsp1/vsp1_hgt.c | 217 +++++++++++++++++++++++++++ > drivers/media/platform/vsp1/vsp1_hgt.h | 42 ++++++ > drivers/media/platform/vsp1/vsp1_pipe.c | 16 +++ > drivers/media/platform/vsp1/vsp1_pipe.h | 2 + > drivers/media/platform/vsp1/vsp1_regs.h | 9 ++ > drivers/media/platform/vsp1/vsp1_video.c | 10 +- > 11 files changed, 352 insertions(+), 16 deletions(-) > create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.c > create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.h [snip] > diff --git a/drivers/media/platform/vsp1/vsp1_hgt.c > b/drivers/media/platform/vsp1/vsp1_hgt.c new file mode 100644 > index 0000000..4e3f762 > --- /dev/null > +++ b/drivers/media/platform/vsp1/vsp1_hgt.c [snip] > +/* ------------------------------------------------------------------------ > + * Controls > + */ > + > +#define V4L2_CID_VSP1_HGT_HUE_AREAS (V4L2_CID_USER_BASE | 0x1001) > + > +static int hgt_hue_areas_s_ctrl(struct v4l2_ctrl *ctrl) > +{ > + struct vsp1_hgt *hgt = container_of(ctrl->handler, struct vsp1_hgt, > + ctrls); > + u8 *value = ctrl->p_new.p_u8; Nitpicking, I'd call the variable values. > + unsigned int i; > + bool ok = true; > + > + /* > + * Make sure values meet one of two possible hardware constrains s/constrains/constraints./ > + * 0L <= 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U > + * 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U <= 0L > + */ > + > + if ((value[0] > value[1]) && (value[11] > value[0])) > + ok = false; > + for (i = 1; i < (HGT_NUM_HUE_AREAS * 2) - 1; ++i) > + if (value[i] > value[i+1]) > + ok = false; > + > + /* Values do not match hardware, adjust to valid settings. */ > + if (!ok) { > + for (i = 0; i < (HGT_NUM_HUE_AREAS * 2) - 1; ++i) { > + if (value[i] > value[i+1]) > + value[i] = value[i+1]; > + } > + } I'm afraid this won't work. Let's assume value[0] = 100, value[1] = 50, value[2] = 25. The loop will unroll to if (value[0] /* 100 */ > value[1] /* 50 */) value[0] = value[1] /* 50 */; if (value[1] /* 50 */ > value[2] /* 25 */) value[1] = value[2] /* 25 */; You will end up with value[0] = 50, value[1] = 25, value[2] = 25, which doesn't match the hardware constraints. How about the following, which tests and fixes the values in a single operation ? static int hgt_hue_areas_s_ctrl(struct v4l2_ctrl *ctrl) { struct vsp1_hgt *hgt = container_of(ctrl->handler, struct vsp1_hgt, ctrls); u8 *values = ctrl->p_new.p_u8; unsigned int i; /* * Adjust the values if they don't meet the hardware constraints: * * 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U */ for (i = 1; i < (HGT_NUM_HUE_AREAS * 2) - 1; ++i) { if (values[i] > values[i+1]) values[i+1] = values[i]; } /* 0L <= 0U or 5U <= 0L */ if (values[0] > values[1] && values[11] > values[0]) values[0] = values[1]; memcpy(hgt->hue_areas, ctrl->p_new.p_u8, sizeof(hgt->hue_areas)); return 0; } I'm also beginning to wonder whether it wouldn't make sense to return -EINVAL when the values don't match the constraints instead of trying to fix them. > + memcpy(hgt->hue_areas, ctrl->p_new.p_u8, sizeof(hgt->hue_areas)); > + > + return 0; > +} [snip] -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv2 2/2] v4l: vsp1: Add HGT support 2016-09-06 19:59 ` Laurent Pinchart @ 2016-09-07 10:05 ` Niklas Söderlund 2016-09-07 10:35 ` Laurent Pinchart 0 siblings, 1 reply; 7+ messages in thread From: Niklas Söderlund @ 2016-09-07 10:05 UTC (permalink / raw) To: Laurent Pinchart Cc: linux-renesas-soc, linux-media, corbet, mchehab, sakari.ailus, hans.verkuil Hi Laurent, Thanks for your review. On 2016-09-06 22:59:22 +0300, Laurent Pinchart wrote: > Hi Niklas, > > Thank you for the patch. > > On Tuesday 06 Sep 2016 16:38:56 Niklas Söderlund wrote: > > The HGT is a Histogram Generator Two-Dimensions. It computes a weighted > > frequency histograms for hue and saturation areas over a configurable > > region of the image with optional subsampling. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > --- > > drivers/media/platform/vsp1/Makefile | 2 +- > > drivers/media/platform/vsp1/vsp1.h | 3 + > > drivers/media/platform/vsp1/vsp1_drv.c | 33 ++++- > > drivers/media/platform/vsp1/vsp1_entity.c | 33 +++-- > > drivers/media/platform/vsp1/vsp1_entity.h | 1 + > > drivers/media/platform/vsp1/vsp1_hgt.c | 217 +++++++++++++++++++++++++++ > > drivers/media/platform/vsp1/vsp1_hgt.h | 42 ++++++ > > drivers/media/platform/vsp1/vsp1_pipe.c | 16 +++ > > drivers/media/platform/vsp1/vsp1_pipe.h | 2 + > > drivers/media/platform/vsp1/vsp1_regs.h | 9 ++ > > drivers/media/platform/vsp1/vsp1_video.c | 10 +- > > 11 files changed, 352 insertions(+), 16 deletions(-) > > create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.c > > create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.h > > [snip] > > > diff --git a/drivers/media/platform/vsp1/vsp1_hgt.c > > b/drivers/media/platform/vsp1/vsp1_hgt.c new file mode 100644 > > index 0000000..4e3f762 > > --- /dev/null > > +++ b/drivers/media/platform/vsp1/vsp1_hgt.c > > [snip] > > > +/* ------------------------------------------------------------------------ > > + * Controls > > + */ > > + > > +#define V4L2_CID_VSP1_HGT_HUE_AREAS (V4L2_CID_USER_BASE | 0x1001) > > + > > +static int hgt_hue_areas_s_ctrl(struct v4l2_ctrl *ctrl) > > +{ > > + struct vsp1_hgt *hgt = container_of(ctrl->handler, struct vsp1_hgt, > > + ctrls); > > + u8 *value = ctrl->p_new.p_u8; > > Nitpicking, I'd call the variable values. > > > + unsigned int i; > > + bool ok = true; > > + > > + /* > > + * Make sure values meet one of two possible hardware constrains > > s/constrains/constraints./ > > > + * 0L <= 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= > 5U > > + * 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U <= > 0L > > + */ > > + > > + if ((value[0] > value[1]) && (value[11] > value[0])) > > + ok = false; > > + for (i = 1; i < (HGT_NUM_HUE_AREAS * 2) - 1; ++i) > > + if (value[i] > value[i+1]) > > + ok = false; > > + > > + /* Values do not match hardware, adjust to valid settings. */ > > + if (!ok) { > > + for (i = 0; i < (HGT_NUM_HUE_AREAS * 2) - 1; ++i) { > > + if (value[i] > value[i+1]) > > + value[i] = value[i+1]; > > + } > > + } > > I'm afraid this won't work. Let's assume value[0] = 100, value[1] = 50, > value[2] = 25. The loop will unroll to > > if (value[0] /* 100 */ > value[1] /* 50 */) > value[0] = value[1] /* 50 */; > if (value[1] /* 50 */ > value[2] /* 25 */) > value[1] = value[2] /* 25 */; > > You will end up with value[0] = 50, value[1] = 25, value[2] = 25, which > doesn't match the hardware constraints. > > How about the following, which tests and fixes the values in a single > operation ? > > static int hgt_hue_areas_s_ctrl(struct v4l2_ctrl *ctrl) > { > struct vsp1_hgt *hgt = container_of(ctrl->handler, struct vsp1_hgt, > ctrls); > u8 *values = ctrl->p_new.p_u8; > unsigned int i; > > /* > * Adjust the values if they don't meet the hardware constraints: > * > * 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U > */ > for (i = 1; i < (HGT_NUM_HUE_AREAS * 2) - 1; ++i) { > if (values[i] > values[i+1]) > values[i+1] = values[i]; > } > > /* 0L <= 0U or 5U <= 0L */ > if (values[0] > values[1] && values[11] > values[0]) > values[0] = values[1]; > > memcpy(hgt->hue_areas, ctrl->p_new.p_u8, sizeof(hgt->hue_areas)); > > return 0; > } > > I'm also beginning to wonder whether it wouldn't make sense to return -EINVAL > when the values don't match the constraints instead of trying to fix them. I'm fine with either solution. I looked at a few other drivers and it seems the most common way is to correct the control value. But maybe in this case it's better to just return -EINVAL. Let me know what you think and I will make it so and send a v3. > > > + memcpy(hgt->hue_areas, ctrl->p_new.p_u8, sizeof(hgt->hue_areas)); > > + > > + return 0; > > +} > > [snip] > -- Regards, Niklas Söderlund ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv2 2/2] v4l: vsp1: Add HGT support 2016-09-07 10:05 ` Niklas Söderlund @ 2016-09-07 10:35 ` Laurent Pinchart 0 siblings, 0 replies; 7+ messages in thread From: Laurent Pinchart @ 2016-09-07 10:35 UTC (permalink / raw) To: Niklas Söderlund Cc: linux-renesas-soc, linux-media, corbet, mchehab, sakari.ailus, hans.verkuil Hi Niklas, On Wednesday 07 Sep 2016 12:05:26 Niklas Söderlund wrote: > On 2016-09-06 22:59:22 +0300, Laurent Pinchart wrote: > > On Tuesday 06 Sep 2016 16:38:56 Niklas Söderlund wrote: > >> The HGT is a Histogram Generator Two-Dimensions. It computes a weighted > >> frequency histograms for hue and saturation areas over a configurable > >> region of the image with optional subsampling. > >> > >> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > >> --- > >> > >> drivers/media/platform/vsp1/Makefile | 2 +- > >> drivers/media/platform/vsp1/vsp1.h | 3 + > >> drivers/media/platform/vsp1/vsp1_drv.c | 33 ++++- > >> drivers/media/platform/vsp1/vsp1_entity.c | 33 +++-- > >> drivers/media/platform/vsp1/vsp1_entity.h | 1 + > >> drivers/media/platform/vsp1/vsp1_hgt.c | 217 +++++++++++++++++++++++ > >> drivers/media/platform/vsp1/vsp1_hgt.h | 42 ++++++ > >> drivers/media/platform/vsp1/vsp1_pipe.c | 16 +++ > >> drivers/media/platform/vsp1/vsp1_pipe.h | 2 + > >> drivers/media/platform/vsp1/vsp1_regs.h | 9 ++ > >> drivers/media/platform/vsp1/vsp1_video.c | 10 +- > >> 11 files changed, 352 insertions(+), 16 deletions(-) > >> create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.c > >> create mode 100644 drivers/media/platform/vsp1/vsp1_hgt.h > > > > [snip] > > > >> diff --git a/drivers/media/platform/vsp1/vsp1_hgt.c > >> b/drivers/media/platform/vsp1/vsp1_hgt.c new file mode 100644 > >> index 0000000..4e3f762 > >> --- /dev/null > >> +++ b/drivers/media/platform/vsp1/vsp1_hgt.c > > > > [snip] > > > >> +/* --------------------------------------------------------------------- > >> + * Controls > >> + */ > >> + > >> +#define V4L2_CID_VSP1_HGT_HUE_AREAS (V4L2_CID_USER_BASE | 0x1001) > >> + > >> +static int hgt_hue_areas_s_ctrl(struct v4l2_ctrl *ctrl) > >> +{ > >> + struct vsp1_hgt *hgt = container_of(ctrl->handler, struct vsp1_hgt, > >> + ctrls); > >> + u8 *value = ctrl->p_new.p_u8; > > > > Nitpicking, I'd call the variable values. > > > >> + unsigned int i; > >> + bool ok = true; > >> + > >> + /* > >> + * Make sure values meet one of two possible hardware constrains > > > > s/constrains/constraints./ > > > >> + * 0L <= 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= > >> 5U > >> + * 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U <= > >> 0L > >> + */ > >> + > >> + if ((value[0] > value[1]) && (value[11] > value[0])) > >> + ok = false; > >> + for (i = 1; i < (HGT_NUM_HUE_AREAS * 2) - 1; ++i) > >> + if (value[i] > value[i+1]) > >> + ok = false; > >> + > >> + /* Values do not match hardware, adjust to valid settings. */ > >> + if (!ok) { > >> + for (i = 0; i < (HGT_NUM_HUE_AREAS * 2) - 1; ++i) { > >> + if (value[i] > value[i+1]) > >> + value[i] = value[i+1]; > >> + } > >> + } > > > > I'm afraid this won't work. Let's assume value[0] = 100, value[1] = 50, > > value[2] = 25. The loop will unroll to > > > > if (value[0] /* 100 */ > value[1] /* 50 */) > > value[0] = value[1] /* 50 */; > > > > if (value[1] /* 50 */ > value[2] /* 25 */) > > value[1] = value[2] /* 25 */; > > > > You will end up with value[0] = 50, value[1] = 25, value[2] = 25, which > > doesn't match the hardware constraints. > > > > How about the following, which tests and fixes the values in a single > > operation ? > > > > static int hgt_hue_areas_s_ctrl(struct v4l2_ctrl *ctrl) > > { > > struct vsp1_hgt *hgt = container_of(ctrl->handler, struct vsp1_hgt, > > ctrls); > > u8 *values = ctrl->p_new.p_u8; > > unsigned int i; > > > > /* > > * Adjust the values if they don't meet the hardware constraints: > > * > > * 0U <= 1L <= 1U <= 2L <= 2U <= 3L <= 3U <= 4L <= 4U <= 5L <= 5U > > */ > > for (i = 1; i < (HGT_NUM_HUE_AREAS * 2) - 1; ++i) { > > if (values[i] > values[i+1]) > > values[i+1] = values[i]; > > } > > > > /* 0L <= 0U or 5U <= 0L */ > > if (values[0] > values[1] && values[11] > values[0]) > > values[0] = values[1]; > > > > memcpy(hgt->hue_areas, ctrl->p_new.p_u8, sizeof(hgt->hue_areas)); > > > > return 0; > > } > > > > I'm also beginning to wonder whether it wouldn't make sense to return > > -EINVAL when the values don't match the constraints instead of trying to > > fix them. > > I'm fine with either solution. I looked at a few other drivers and it > seems the most common way is to correct the control value. But maybe in > this case it's better to just return -EINVAL. > > Let me know what you think and I will make it so and send a v3. Given that fixed values would result in a different histogram that would likely be unusable by a userspace application that doesn't expect the control values to be changed (and if it did, it should set correct values to start with), I think -EINVAL is better. As Hans pointed out on IRC, this should be implemented in the .try_ctrl() handler. > > > + memcpy(hgt->hue_areas, ctrl->p_new.p_u8, sizeof(hgt->hue_areas)); > > > + > > > + return 0; > > > +} > > > > [snip] -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-09-07 10:34 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-09-06 14:38 [PATCHv2 0/2] v4l: vsp1: Add HGT support Niklas Söderlund 2016-09-06 14:38 ` [PATCHv2 1/2] v4l: Define a pixel format for the R-Car VSP1 2-D histogram engine Niklas Söderlund 2016-09-06 14:54 ` Laurent Pinchart 2016-09-06 14:38 ` [PATCHv2 2/2] v4l: vsp1: Add HGT support Niklas Söderlund 2016-09-06 19:59 ` Laurent Pinchart 2016-09-07 10:05 ` Niklas Söderlund 2016-09-07 10:35 ` Laurent Pinchart
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).