* [PATCH 0/2] RK3288 VP8 decoding support
@ 2019-06-13 15:10 Ezequiel Garcia
2019-06-13 15:10 ` [PATCH 1/2] media: uapi: Add VP8 stateless decoder API Ezequiel Garcia
2019-06-13 15:10 ` [PATCH 2/2] media: hantro: Add support for VP8 decoding on rk3288 Ezequiel Garcia
0 siblings, 2 replies; 7+ messages in thread
From: Ezequiel Garcia @ 2019-06-13 15:10 UTC (permalink / raw)
To: linux-media, Hans Verkuil
Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
Ezequiel Garcia
This patchset adds support for Hantro G1 VP8 stateless decoding,
as available on RK3288 SoC.
In order to support VP8 stateless decoding, a new pixel format
is introduced V4L2_PIX_FMT_VP8_FRAME, to be used with a new control
V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR.
The VP8 stateless uAPI was submitted as an RFC. The changes
in this v1 are:
Changes from RFC:
* Verify the various ABIs.
* Move entropy coder state fields to a struct.
* Move key_frame field to the flags.
* Remove unneeded first_part_offset field.
* Add documentation.
The ABI has been verified with Maxime Ripard's tools:
https://gitlab.collabora.com/ezequiel/v4l2-ctrl-abi-check
Pawel Osciak (1):
media: uapi: Add VP8 stateless decoder API
ZhiChao Yu (1):
media: hantro: Add support for VP8 decoding on rk3288
Documentation/media/uapi/v4l/biblio.rst | 10 +
.../media/uapi/v4l/ext-ctrls-codec.rst | 311 ++++++++++
.../media/uapi/v4l/pixfmt-compressed.rst | 20 +
drivers/media/v4l2-core/v4l2-ctrls.c | 8 +
drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
drivers/staging/media/hantro/Makefile | 4 +-
drivers/staging/media/hantro/hantro.h | 5 +
drivers/staging/media/hantro/hantro_drv.c | 6 +
.../staging/media/hantro/hantro_g1_vp8_dec.c | 548 ++++++++++++++++++
drivers/staging/media/hantro/hantro_hw.h | 17 +
drivers/staging/media/hantro/hantro_v4l2.c | 1 +
drivers/staging/media/hantro/hantro_vp8.c | 188 ++++++
drivers/staging/media/hantro/rk3288_vpu_hw.c | 22 +-
include/media/v4l2-ctrls.h | 3 +
include/media/vp8-ctrls.h | 110 ++++
15 files changed, 1252 insertions(+), 2 deletions(-)
create mode 100644 drivers/staging/media/hantro/hantro_g1_vp8_dec.c
create mode 100644 drivers/staging/media/hantro/hantro_vp8.c
create mode 100644 include/media/vp8-ctrls.h
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] media: uapi: Add VP8 stateless decoder API
2019-06-13 15:10 [PATCH 0/2] RK3288 VP8 decoding support Ezequiel Garcia
@ 2019-06-13 15:10 ` Ezequiel Garcia
2019-06-17 13:23 ` Boris Brezillon
2019-06-13 15:10 ` [PATCH 2/2] media: hantro: Add support for VP8 decoding on rk3288 Ezequiel Garcia
1 sibling, 1 reply; 7+ messages in thread
From: Ezequiel Garcia @ 2019-06-13 15:10 UTC (permalink / raw)
To: linux-media, Hans Verkuil
Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
Pawel Osciak, Ezequiel Garcia
From: Pawel Osciak <posciak@chromium.org>
Add the parsed VP8 frame pixel format and controls, to be used
with the new stateless decoder API for VP8 to provide parameters
for accelerator (aka stateless) codecs.
Signed-off-by: Pawel Osciak <posciak@chromium.org>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
--
Changes from RFC:
* Make sure the uAPI has the same size on x86, x86_64, arm and arm64.
* Move entropy coder state fields to a struct.
* Move key_frame field to the flags.
* Remove unneeded first_part_offset field.
* Add documentation.
---
Documentation/media/uapi/v4l/biblio.rst | 10 +
.../media/uapi/v4l/ext-ctrls-codec.rst | 311 ++++++++++++++++++
.../media/uapi/v4l/pixfmt-compressed.rst | 20 ++
drivers/media/v4l2-core/v4l2-ctrls.c | 8 +
drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
include/media/v4l2-ctrls.h | 3 +
include/media/vp8-ctrls.h | 110 +++++++
7 files changed, 463 insertions(+)
create mode 100644 include/media/vp8-ctrls.h
diff --git a/Documentation/media/uapi/v4l/biblio.rst b/Documentation/media/uapi/v4l/biblio.rst
index 8f4eb8823d82..ad2ff258afa8 100644
--- a/Documentation/media/uapi/v4l/biblio.rst
+++ b/Documentation/media/uapi/v4l/biblio.rst
@@ -395,3 +395,13 @@ colimg
:title: Color Imaging: Fundamentals and Applications
:author: Erik Reinhard et al.
+
+.. _vp8:
+
+VP8
+===
+
+
+:title: RFC 6386: "VP8 Data Format and Decoding Guide"
+
+:author: J. Bankoski et al.
diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
index d6ea2ffd65c5..7a1947f5be96 100644
--- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
+++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
@@ -2234,6 +2234,317 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
Quantization parameter for a P frame for FWHT. Valid range: from 1
to 31.
+.. _v4l2-mpeg-vp8:
+
+``V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR (struct)``
+ Specifies the frame parameters for the associated VP8 parsed frame data.
+ This includes the necessary parameters for
+ configuring a stateless hardware decoding pipeline for VP8.
+ The bitstream parameters are defined according to :ref:`vp8`.
+
+ .. note::
+
+ This compound control is not yet part of the public kernel API and
+ it is expected to change.
+
+.. c:type:: v4l2_ctrl_vp8_frame_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{5.8cm}|p{4.8cm}|p{6.6cm}|
+
+.. flat-table:: struct v4l2_ctrl_vp8_frame_header
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 1 1 2
+
+ * - __u8
+ - ``version``
+ - Bitstream version.
+ * - __u16
+ - ``width``
+ - The width of the frame
+ * - __u16
+ - ``height``
+ - The height of the frame
+ * - __u8
+ - ``horizontal_scale``
+ - Horizontal scale.
+ * - __u8
+ - ``vertical_scale``
+ - Vertical scale.
+ * - struct :c:type:`v4l2_vp8_segment_header`
+ - ``segment_header``
+ - Structure with segment-based adjustments metadata.
+ * - struct :c:type:`v4l2_vp8_loopfilter_header`
+ - ``loopfilter_header``
+ - Structure with loop filter level adjustments metadata.
+ * - struct :c:type:`v4l2_vp8_quantization_header`
+ - ``quant_header``
+ - Structure with VP8 dequantization indices metadata.
+ * - struct :c:type:`v4l2_vp8_entropy_header`
+ - ``entropy_header``
+ - Structure with VP8 entropy coder probabilities metadata.
+ * - __u16
+ - ``sign_bias_golden``
+ - Sign bias flag for golden reference frame.
+ * - __u16
+ - ``sign_bias_alternate``
+ - Sign bias flag for alternate reference frame.
+ * - __u8
+ - ``prob_skip_false``
+ -
+ * - __u8
+ - ``prob_intra``
+ -
+ * - __u8
+ - ``prob_last``
+ -
+ * - __u8
+ - ``prob_gf``
+ -
+ * - __u32
+ - ``first_part_size``
+ -
+ * - __u32
+ - ``macroblock_bit_offset``
+ - Offset in bits of macroblock data in first partition.
+ * - __u32
+ - ``dct_part_sizes[8]``
+ - DCT coefficients sizes.
+ * - __u8
+ - ``num_dct_parts``
+ - Number of DCT coefficients partitions.
+ * - struct :c:type:`v4l2_vp8_entropy_coder_state`
+ - ``coder_state``
+ - Structure with VP8 entropy coder state.
+ * - __u64
+ - ``last_frame_ts``
+ - Timestamp for the V4L2 capture buffer to use as last reference frame, used
+ with inter-coded frames. The timestamp refers to the ``timestamp`` field in
+ struct :c:type:`v4l2_buffer`. Use the :c:func:`v4l2_timeval_to_ns()`
+ function to convert the struct :c:type:`timeval` in struct
+ :c:type:`v4l2_buffer` to a __u64.
+ * - __u64
+ - ``golden_frame_ts``
+ - Timestamp for the V4L2 capture buffer to use as last reference frame, used
+ with inter-coded frames. The timestamp refers to the ``timestamp`` field in
+ struct :c:type:`v4l2_buffer`. Use the :c:func:`v4l2_timeval_to_ns()`
+ function to convert the struct :c:type:`timeval` in struct
+ :c:type:`v4l2_buffer` to a __u64.
+ * - __u64
+ - ``alt_frame_ts``
+ - Timestamp for the V4L2 capture buffer to use as alternate reference frame, used
+ with inter-coded frames. The timestamp refers to the ``timestamp`` field in
+ struct :c:type:`v4l2_buffer`. Use the :c:func:`v4l2_timeval_to_ns()`
+ function to convert the struct :c:type:`timeval` in struct
+ :c:type:`v4l2_buffer` to a __u64.
+ * - __u64
+ - ``flags``
+ - flags
+ * - __u64
+ - ``flags``
+ - See :ref:`Frame Header Flags <vp8_frame_header_flags>`
+
+.. _vp8_frame_header_flags:
+
+``Frame Header Flags``
+
+.. cssclass:: longtable
+
+.. flat-table::
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 1 1 2
+
+ * - ``V4L2_VP8_FRAME_HDR_FLAG_KEY_FRAME``
+ - 0x01
+ - Inidicates if the frame is a key frame.
+ * - ``V4L2_VP8_FRAME_HDR_FLAG_EXPERIMENTAL``
+ - 0x02
+ - Experimental bitstream.
+ * - ``V4L2_VP8_FRAME_HDR_FLAG_SHOW_FRAME``
+ - 0x04
+ - Show frame flag, indicates if the frame is for display.
+ * - ``V4L2_VP8_FRAME_HDR_FLAG_MB_NO_SKIP_COEFF``
+ - 0x08
+ - Enable/disable skipping of macroblocks with no non-zero coefficients.
+
+.. c:type:: v4l2_vp8_entropy_coder_state
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_entropy_coder_state
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 1 1 2
+
+ * - __u8
+ - ``range``
+ -
+ * - __u8
+ - ``value``
+ -
+ * - __u8
+ - ``bit_count``
+ -
+
+.. c:type:: v4l2_vp8_segment_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_segment_header
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 1 1 2
+
+ * - __u8
+ - ``segment_feature_mode``
+ - Indicates the mode of segment feature data. 1 for absolute value
+ mode, 0 for delta value mode.
+ * - __s8
+ - ``quant_update[4]``
+ - Signed quantizer value update.
+ * - __s8
+ - ``lf_update[4]``
+ - Signed loop filter level value update.
+ * - __u8
+ - ``segment_probs[3]``
+ - Segment probabilities.
+ * - __u32
+ - ``flags``
+ - See :ref:`Segment Header Flags <vp8_segment_header_flags>`
+
+.. _vp8_segment_header_flags:
+
+``Segment Header Flags``
+
+.. cssclass:: longtable
+
+.. flat-table::
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 1 1 2
+
+ * - ``V4L2_VP8_SEGMNT_HDR_FLAG_ENABLED``
+ - 0x01
+ - Enable/disable segment-based adjustments.
+ * - ``V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_MAP``
+ - 0x02
+ - Indicates if the macroblock segmentation map is updated in this frame.
+ * - ``V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_FEATURE_DATA``
+ - 0x04
+ - Indicates if the segment feature data is updated in this frame.
+
+.. c:type:: v4l2_vp8_loopfilter_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_loopfilter_header
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 1 1 2
+
+ * - __u16
+ - ``type``
+ - Filter type.
+ * - __u8
+ - ``level``
+ - Filter level
+ * - __u8
+ - ``sharpness_level``
+ - Sharpness level
+ * - __s8
+ - ``ref_frm_delta_magnitude[4]``
+ -
+ * - __s8
+ - ``mb_mode_delta_magnitude[4]``
+ -
+ * - __u16
+ - ``flags``
+ - See :ref:`Loopfilter Header Flags <vp8_loopfilter_header_flags>`
+
+.. _vp8_loopfilter_header_flags:
+
+``Loopfilter Header Flags``
+
+.. cssclass:: longtable
+
+.. flat-table::
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 1 1 2
+
+ * - ``V4L2_VP8_LF_HDR_ADJ_ENABLE``
+ - 0x01
+ - Enable/disable macroblock-level loop filter adjustment.
+ * - ``V4L2_VP8_LF_HDR_DELTA_UPDATE``
+ - 0x02
+ - Indicates if the delta values used in an adjustment are updated.
+
+.. c:type:: v4l2_vp8_quantization_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_quantization_header
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 1 1 2
+
+ * - __u8
+ - ``y_ac_qi``
+ -
+ * - __s8
+ - ``y_dc_delta``
+ -
+ * - __s8
+ - ``y2_dc_delta``
+ -
+ * - __s8
+ - ``y2_ac_delta``
+ -
+ * - __s8
+ - ``uv_dc_delta``
+ -
+ * - __s8
+ - ``uv_ac_delta``
+ -
+ * - __u16
+ - ``dequant_factors[4][3][2]``
+ -
+
+.. c:type:: v4l2_vp8_entropy_header
+
+.. cssclass:: longtable
+
+.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
+
+.. flat-table:: struct v4l2_vp8_entropy_header
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 1 1 2
+
+ * - __u8
+ - ``coeff_probs[4][8][3][11]``
+ -
+ * - __u8
+ - ``y_mode_probs[4]``
+ -
+ * - __u8
+ - ``uv_mode_probs[3]``
+ -
+ * - __u8
+ - ``mv_probs[2][19]``
+ -
+
.. raw:: latex
\normalsize
diff --git a/Documentation/media/uapi/v4l/pixfmt-compressed.rst b/Documentation/media/uapi/v4l/pixfmt-compressed.rst
index 4b701fc7653e..f349ad7c20cf 100644
--- a/Documentation/media/uapi/v4l/pixfmt-compressed.rst
+++ b/Documentation/media/uapi/v4l/pixfmt-compressed.rst
@@ -133,6 +133,26 @@ Compressed Formats
- ``V4L2_PIX_FMT_VP8``
- 'VP80'
- VP8 video elementary stream.
+ * .. _V4L2-PIX-FMT-VP8-FRAME:
+
+ - ``V4L2_PIX_FMT_VP8_FRAME``
+ - 'VP8F'
+ - VP8 parsed frame, as extracted from the container.
+ This format is adapted for stateless video decoders that implement a
+ VP8 pipeline (using the :ref:`mem2mem` and :ref:`media-request-api`).
+ Metadata associated with the frame to decode is required to be passed
+ through the ``V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR`` control.
+ See the :ref:`associated Codec Control IDs <v4l2-mpeg-vp8>`.
+ Exactly one output and one capture buffer must be provided for use with
+ this pixel format. The output buffer must contain the appropriate number
+ of macroblocks to decode a full corresponding frame to the matching
+ capture buffer.
+
+ .. note::
+
+ This format is not yet part of the public kernel API and it
+ is expected to change.
+
* .. _V4L2-PIX-FMT-VP9:
- ``V4L2_PIX_FMT_VP9``
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index f2b9bdedbf8c..bb623a972b9e 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -885,6 +885,7 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_MPEG_VIDEO_VPX_P_FRAME_QP: return "VPX P-Frame QP Value";
case V4L2_CID_MPEG_VIDEO_VP8_PROFILE: return "VP8 Profile";
case V4L2_CID_MPEG_VIDEO_VP9_PROFILE: return "VP9 Profile";
+ case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR: return "VP8 Frame Header";
/* HEVC controls */
case V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP: return "HEVC I-Frame QP Value";
@@ -1345,6 +1346,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
case V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS:
*type = V4L2_CTRL_TYPE_H264_DECODE_PARAMS;
break;
+ case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR:
+ *type = V4L2_CTRL_TYPE_VP8_FRAME_HDR;
+ break;
default:
*type = V4L2_CTRL_TYPE_INTEGER;
break;
@@ -1737,6 +1741,7 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
case V4L2_CTRL_TYPE_H264_SCALING_MATRIX:
case V4L2_CTRL_TYPE_H264_SLICE_PARAMS:
case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
+ case V4L2_CTRL_TYPE_VP8_FRAME_HDR:
return 0;
default:
@@ -2337,6 +2342,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
case V4L2_CTRL_TYPE_H264_DECODE_PARAMS:
elem_size = sizeof(struct v4l2_ctrl_h264_decode_params);
break;
+ case V4L2_CTRL_TYPE_VP8_FRAME_HDR:
+ elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
+ break;
default:
if (type < V4L2_CTRL_COMPOUND_TYPES)
elem_size = sizeof(s32);
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index b1f4b991dba6..436a13204921 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1331,6 +1331,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
case V4L2_PIX_FMT_VC1_ANNEX_G: descr = "VC-1 (SMPTE 412M Annex G)"; break;
case V4L2_PIX_FMT_VC1_ANNEX_L: descr = "VC-1 (SMPTE 412M Annex L)"; break;
case V4L2_PIX_FMT_VP8: descr = "VP8"; break;
+ case V4L2_PIX_FMT_VP8_FRAME: descr = "VP8 FRAME"; break;
case V4L2_PIX_FMT_VP9: descr = "VP9"; break;
case V4L2_PIX_FMT_HEVC: descr = "HEVC"; break; /* aka H.265 */
case V4L2_PIX_FMT_FWHT: descr = "FWHT"; break; /* used in vicodec */
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index b4433483af23..6e9dc9c44bb1 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -20,6 +20,7 @@
#include <media/mpeg2-ctrls.h>
#include <media/fwht-ctrls.h>
#include <media/h264-ctrls.h>
+#include <media/vp8-ctrls.h>
/* forward references */
struct file;
@@ -48,6 +49,7 @@ struct poll_table_struct;
* @p_h264_scaling_matrix: Pointer to a struct v4l2_ctrl_h264_scaling_matrix.
* @p_h264_slice_params: Pointer to a struct v4l2_ctrl_h264_slice_params.
* @p_h264_decode_params: Pointer to a struct v4l2_ctrl_h264_decode_params.
+ * @p_vp8_frame_header: Pointer to a VP8 frame header structure.
* @p: Pointer to a compound value.
*/
union v4l2_ctrl_ptr {
@@ -65,6 +67,7 @@ union v4l2_ctrl_ptr {
struct v4l2_ctrl_h264_scaling_matrix *p_h264_scaling_matrix;
struct v4l2_ctrl_h264_slice_params *p_h264_slice_params;
struct v4l2_ctrl_h264_decode_params *p_h264_decode_params;
+ struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
void *p;
};
diff --git a/include/media/vp8-ctrls.h b/include/media/vp8-ctrls.h
new file mode 100644
index 000000000000..3b0dcc125e25
--- /dev/null
+++ b/include/media/vp8-ctrls.h
@@ -0,0 +1,110 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * These are the VP8 state controls for use with stateless VP8
+ * codec drivers.
+ *
+ * It turns out that these structs are not stable yet and will undergo
+ * more changes. So keep them private until they are stable and ready to
+ * become part of the official public API.
+ */
+
+#ifndef _VP8_CTRLS_H_
+#define _VP8_CTRLS_H_
+
+#include <linux/v4l2-controls.h>
+
+#define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frames */
+
+#define V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR (V4L2_CID_MPEG_BASE + 2000)
+#define V4L2_CTRL_TYPE_VP8_FRAME_HDR 0x301
+
+#define V4L2_VP8_SEGMNT_HDR_FLAG_ENABLED 0x01
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_MAP 0x02
+#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_FEATURE_DATA 0x04
+
+struct v4l2_vp8_segment_header {
+ __u8 segment_feature_mode;
+ __s8 quant_update[4];
+ __s8 lf_update[4];
+ __u8 segment_probs[3];
+ __u32 flags;
+};
+
+#define V4L2_VP8_LF_HDR_ADJ_ENABLE 0x01
+#define V4L2_VP8_LF_HDR_DELTA_UPDATE 0x02
+struct v4l2_vp8_loopfilter_header {
+ __u16 type;
+ __u8 level;
+ __u8 sharpness_level;
+ __s8 ref_frm_delta_magnitude[4];
+ __s8 mb_mode_delta_magnitude[4];
+ __u16 flags;
+};
+
+struct v4l2_vp8_quantization_header {
+ __u8 y_ac_qi;
+ __s8 y_dc_delta;
+ __s8 y2_dc_delta;
+ __s8 y2_ac_delta;
+ __s8 uv_dc_delta;
+ __s8 uv_ac_delta;
+ __u16 dequant_factors[4][3][2];
+};
+
+struct v4l2_vp8_entropy_header {
+ __u8 coeff_probs[4][8][3][11];
+ __u8 y_mode_probs[4];
+ __u8 uv_mode_probs[3];
+ __u8 mv_probs[2][19];
+};
+
+struct v4l2_vp8_entropy_coder_state {
+ __u8 range;
+ __u8 value;
+ __u8 bit_count;
+};
+
+#define V4L2_VP8_FRAME_HDR_FLAG_KEY_FRAME 0x01
+#define V4L2_VP8_FRAME_HDR_FLAG_EXPERIMENTAL 0x02
+#define V4L2_VP8_FRAME_HDR_FLAG_SHOW_FRAME 0x04
+#define V4L2_VP8_FRAME_HDR_FLAG_MB_NO_SKIP_COEFF 0x08
+
+#define VP8_FRAME_IS_KEY_FRAME(hdr) (!!(hdr->flags & V4L2_VP8_FRAME_HDR_FLAG_KEY_FRAME))
+
+struct v4l2_ctrl_vp8_frame_header {
+ __u8 version;
+
+ /* Populated also if not a key frame */
+ __u16 width;
+ __u16 height;
+ __u8 horizontal_scale;
+ __u8 vertical_scale;
+
+ struct v4l2_vp8_segment_header segment_header;
+ struct v4l2_vp8_loopfilter_header lf_header;
+ struct v4l2_vp8_quantization_header quant_header;
+ struct v4l2_vp8_entropy_header entropy_header;
+
+ __u16 sign_bias_golden;
+ __u16 sign_bias_alternate;
+
+ __u8 prob_skip_false;
+ __u8 prob_intra;
+ __u8 prob_last;
+ __u8 prob_gf;
+
+ __u32 first_part_size;
+ __u32 macroblock_bit_offset;
+ __u32 dct_part_sizes[8];
+ __u8 num_dct_parts;
+
+ struct v4l2_vp8_entropy_coder_state coder_state;
+
+ __u64 last_frame_ts;
+ __u64 golden_frame_ts;
+ __u64 alt_frame_ts;
+
+ __u64 flags;
+};
+
+#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] media: hantro: Add support for VP8 decoding on rk3288
2019-06-13 15:10 [PATCH 0/2] RK3288 VP8 decoding support Ezequiel Garcia
2019-06-13 15:10 ` [PATCH 1/2] media: uapi: Add VP8 stateless decoder API Ezequiel Garcia
@ 2019-06-13 15:10 ` Ezequiel Garcia
2019-06-17 13:40 ` Boris Brezillon
1 sibling, 1 reply; 7+ messages in thread
From: Ezequiel Garcia @ 2019-06-13 15:10 UTC (permalink / raw)
To: linux-media, Hans Verkuil
Cc: kernel, Nicolas Dufresne, Tomasz Figa, linux-rockchip,
Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
ZhiChao Yu, Ezequiel Garcia
From: ZhiChao Yu <zhichao.yu@rock-chips.com>
Introduce VP8 decoding support in RK3288.
Signed-off-by: ZhiChao Yu <zhichao.yu@rock-chips.com>
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
drivers/staging/media/hantro/Makefile | 4 +-
drivers/staging/media/hantro/hantro.h | 5 +
drivers/staging/media/hantro/hantro_drv.c | 6 +
.../staging/media/hantro/hantro_g1_vp8_dec.c | 548 ++++++++++++++++++
drivers/staging/media/hantro/hantro_hw.h | 17 +
drivers/staging/media/hantro/hantro_v4l2.c | 1 +
drivers/staging/media/hantro/hantro_vp8.c | 188 ++++++
drivers/staging/media/hantro/rk3288_vpu_hw.c | 22 +-
8 files changed, 789 insertions(+), 2 deletions(-)
create mode 100644 drivers/staging/media/hantro/hantro_g1_vp8_dec.c
create mode 100644 drivers/staging/media/hantro/hantro_vp8.c
diff --git a/drivers/staging/media/hantro/Makefile b/drivers/staging/media/hantro/Makefile
index 1584acdbf4a3..a627aee77f75 100644
--- a/drivers/staging/media/hantro/Makefile
+++ b/drivers/staging/media/hantro/Makefile
@@ -5,10 +5,12 @@ hantro-vpu-y += \
hantro_v4l2.o \
hantro_h1_jpeg_enc.o \
hantro_g1_mpeg2_dec.o \
+ hantro_g1_vp8_dec.o \
rk3399_vpu_hw_jpeg_enc.o \
rk3399_vpu_hw_mpeg2_dec.o \
hantro_jpeg.o \
- hantro_mpeg2.o
+ hantro_mpeg2.o \
+ hantro_vp8.o
hantro-vpu-$(CONFIG_VIDEO_HANTRO_ROCKCHIP) += \
rk3288_vpu_hw.o \
diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index 62dcca9ff19c..f903e82c7760 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -40,6 +40,7 @@ struct hantro_codec_ops;
#define HANTRO_ENCODERS 0x0000ffff
#define HANTRO_MPEG2_DECODER BIT(16)
+#define HANTRO_VP8_DECODER BIT(17)
#define HANTRO_DECODERS 0xffff0000
/**
@@ -97,11 +98,13 @@ struct hantro_variant {
* @HANTRO_MODE_NONE: No operating mode. Used for RAW video formats.
* @HANTRO_MODE_JPEG_ENC: JPEG encoder.
* @HANTRO_MODE_MPEG2_DEC: MPEG-2 decoder.
+ * @HANTRO_MODE_VP8_DEC: VP8 decoder.
*/
enum hantro_codec_mode {
HANTRO_MODE_NONE = -1,
HANTRO_MODE_JPEG_ENC,
HANTRO_MODE_MPEG2_DEC,
+ HANTRO_MODE_VP8_DEC,
};
/*
@@ -215,6 +218,7 @@ struct hantro_dev {
* @codec_ops: Set of operations related to codec mode.
* @jpeg_enc: JPEG-encoding context.
* @mpeg2_dec: MPEG-2-decoding context.
+ * @vp8_dec: VP8-decoding context.
*/
struct hantro_ctx {
struct hantro_dev *dev;
@@ -241,6 +245,7 @@ struct hantro_ctx {
union {
struct hantro_jpeg_enc_hw_ctx jpeg_enc;
struct hantro_mpeg2_dec_hw_ctx mpeg2_dec;
+ struct hantro_vp8_dec_hw_ctx vp8_dec;
};
};
diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index 1d3af881d088..ab0aa7408a7d 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -282,6 +282,12 @@ static struct hantro_ctrl controls[] = {
.cfg = {
.elem_size = sizeof(struct v4l2_ctrl_mpeg2_quantization),
},
+ }, {
+ .id = V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR,
+ .codec = HANTRO_VP8_DECODER,
+ .cfg = {
+ .elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header),
+ },
},
};
diff --git a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
new file mode 100644
index 000000000000..b6e1bcf4c071
--- /dev/null
+++ b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
@@ -0,0 +1,548 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Hantro VP8 codec driver
+ *
+ * Copyright (C) 2019 Rockchip Electronics Co., Ltd.
+ * ZhiChao Yu <zhichao.yu@rock-chips.com>
+ *
+ * Copyright (C) 2019 Google, Inc.
+ * Tomasz Figa <tfiga@chromium.org>
+ */
+
+#include <media/v4l2-mem2mem.h>
+#include <media/vp8-ctrls.h>
+
+#include "hantro_hw.h"
+#include "hantro.h"
+#include "hantro_g1_regs.h"
+
+#define DEC_8190_ALIGN_MASK 0x07U
+
+struct vp8_dec_reg {
+ u32 base;
+ u32 shift;
+ u32 mask;
+};
+
+/* dct partition base address regs */
+static const struct vp8_dec_reg vp8_dec_dct_base[8] = {
+ { G1_REG_ADDR_STR, 0, 0xffffffff },
+ { G1_REG_ADDR_REF(8), 0, 0xffffffff },
+ { G1_REG_ADDR_REF(9), 0, 0xffffffff },
+ { G1_REG_ADDR_REF(10), 0, 0xffffffff },
+ { G1_REG_ADDR_REF(11), 0, 0xffffffff },
+ { G1_REG_ADDR_REF(12), 0, 0xffffffff },
+ { G1_REG_ADDR_REF(14), 0, 0xffffffff },
+ { G1_REG_ADDR_REF(15), 0, 0xffffffff },
+};
+
+/* loop filter level regs */
+static const struct vp8_dec_reg vp8_dec_lf_level[4] = {
+ { G1_REG_REF_PIC(2), 18, 0x3f },
+ { G1_REG_REF_PIC(2), 12, 0x3f },
+ { G1_REG_REF_PIC(2), 6, 0x3f },
+ { G1_REG_REF_PIC(2), 0, 0x3f },
+};
+
+/* macroblock loop filter level adjustment regs */
+static const struct vp8_dec_reg vp8_dec_mb_adj[4] = {
+ { G1_REG_REF_PIC(0), 21, 0x7f },
+ { G1_REG_REF_PIC(0), 14, 0x7f },
+ { G1_REG_REF_PIC(0), 7, 0x7f },
+ { G1_REG_REF_PIC(0), 0, 0x7f },
+};
+
+/* reference frame adjustment regs */
+static const struct vp8_dec_reg vp8_dec_ref_adj[4] = {
+ { G1_REG_REF_PIC(1), 21, 0x7f },
+ { G1_REG_REF_PIC(1), 14, 0x7f },
+ { G1_REG_REF_PIC(1), 7, 0x7f },
+ { G1_REG_REF_PIC(1), 0, 0x7f },
+};
+
+/* quantizer regs */
+static const struct vp8_dec_reg vp8_dec_quant[4] = {
+ { G1_REG_REF_PIC(3), 11, 0x7ff },
+ { G1_REG_REF_PIC(3), 0, 0x7ff },
+ { G1_REG_BD_REF_PIC(4), 11, 0x7ff },
+ { G1_REG_BD_REF_PIC(4), 0, 0x7ff },
+};
+
+/* quantizer delta regs */
+static const struct vp8_dec_reg vp8_dec_quant_delta[5] = {
+ { G1_REG_REF_PIC(3), 27, 0x1f },
+ { G1_REG_REF_PIC(3), 22, 0x1f },
+ { G1_REG_BD_REF_PIC(4), 27, 0x1f },
+ { G1_REG_BD_REF_PIC(4), 22, 0x1f },
+ { G1_REG_BD_P_REF_PIC, 27, 0x1f },
+};
+
+/* dct partition start bits regs */
+static const struct vp8_dec_reg vp8_dec_dct_start_bits[8] = {
+ { G1_REG_DEC_CTRL2, 26, 0x3f }, { G1_REG_DEC_CTRL4, 26, 0x3f },
+ { G1_REG_DEC_CTRL4, 20, 0x3f }, { G1_REG_DEC_CTRL7, 24, 0x3f },
+ { G1_REG_DEC_CTRL7, 18, 0x3f }, { G1_REG_DEC_CTRL7, 12, 0x3f },
+ { G1_REG_DEC_CTRL7, 6, 0x3f }, { G1_REG_DEC_CTRL7, 0, 0x3f },
+};
+
+/* precision filter tap regs */
+static const struct vp8_dec_reg vp8_dec_pred_bc_tap[8][4] = {
+ {
+ { G1_REG_PRED_FLT, 22, 0x3ff },
+ { G1_REG_PRED_FLT, 12, 0x3ff },
+ { G1_REG_PRED_FLT, 2, 0x3ff },
+ { G1_REG_REF_PIC(4), 22, 0x3ff },
+ },
+ {
+ { G1_REG_REF_PIC(4), 12, 0x3ff },
+ { G1_REG_REF_PIC(4), 2, 0x3ff },
+ { G1_REG_REF_PIC(5), 22, 0x3ff },
+ { G1_REG_REF_PIC(5), 12, 0x3ff },
+ },
+ {
+ { G1_REG_REF_PIC(5), 2, 0x3ff },
+ { G1_REG_REF_PIC(6), 22, 0x3ff },
+ { G1_REG_REF_PIC(6), 12, 0x3ff },
+ { G1_REG_REF_PIC(6), 2, 0x3ff },
+ },
+ {
+ { G1_REG_REF_PIC(7), 22, 0x3ff },
+ { G1_REG_REF_PIC(7), 12, 0x3ff },
+ { G1_REG_REF_PIC(7), 2, 0x3ff },
+ { G1_REG_LT_REF, 22, 0x3ff },
+ },
+ {
+ { G1_REG_LT_REF, 12, 0x3ff },
+ { G1_REG_LT_REF, 2, 0x3ff },
+ { G1_REG_VALID_REF, 22, 0x3ff },
+ { G1_REG_VALID_REF, 12, 0x3ff },
+ },
+ {
+ { G1_REG_VALID_REF, 2, 0x3ff },
+ { G1_REG_BD_REF_PIC(0), 22, 0x3ff },
+ { G1_REG_BD_REF_PIC(0), 12, 0x3ff },
+ { G1_REG_BD_REF_PIC(0), 2, 0x3ff },
+ },
+ {
+ { G1_REG_BD_REF_PIC(1), 22, 0x3ff },
+ { G1_REG_BD_REF_PIC(1), 12, 0x3ff },
+ { G1_REG_BD_REF_PIC(1), 2, 0x3ff },
+ { G1_REG_BD_REF_PIC(2), 22, 0x3ff },
+ },
+ {
+ { G1_REG_BD_REF_PIC(2), 12, 0x3ff },
+ { G1_REG_BD_REF_PIC(2), 2, 0x3ff },
+ { G1_REG_BD_REF_PIC(3), 22, 0x3ff },
+ { G1_REG_BD_REF_PIC(3), 12, 0x3ff },
+ },
+};
+
+/*
+ * filter taps taken to 7-bit precision,
+ * reference RFC6386#Page-16, filters[8][6]
+ */
+static const u32 vp8_dec_mc_filter[8][6] = {
+ { 0, 0, 128, 0, 0, 0 },
+ { 0, -6, 123, 12, -1, 0 },
+ { 2, -11, 108, 36, -8, 1 },
+ { 0, -9, 93, 50, -6, 0 },
+ { 3, -16, 77, 77, -16, 3 },
+ { 0, -6, 50, 93, -9, 0 },
+ { 1, -8, 36, 108, -11, 2 },
+ { 0, -1, 12, 123, -6, 0 }
+};
+
+static inline void vp8_dec_reg_write(struct hantro_dev *vpu,
+ const struct vp8_dec_reg *reg, u32 val)
+{
+ u32 v;
+
+ v = vdpu_read(vpu, reg->base);
+ v &= ~(reg->mask << reg->shift);
+ v |= ((val & reg->mask) << reg->shift);
+ vdpu_write_relaxed(vpu, v, reg->base);
+}
+
+/*
+ * set loop filters
+ */
+static void cfg_lf(struct hantro_ctx *ctx,
+ const struct v4l2_ctrl_vp8_frame_header *hdr)
+{
+ struct hantro_dev *vpu = ctx->dev;
+ u32 reg;
+ int i;
+
+ if (!(hdr->segment_header.flags & V4L2_VP8_SEGMNT_HDR_FLAG_ENABLED)) {
+ vp8_dec_reg_write(vpu, &vp8_dec_lf_level[0], hdr->lf_header.level);
+ } else if (hdr->segment_header.segment_feature_mode) {
+ /* absolute mode */
+ for (i = 0; i < 4; i++)
+ vp8_dec_reg_write(vpu, &vp8_dec_lf_level[i],
+ hdr->segment_header.lf_update[i]);
+ } else {
+ /* delta mode */
+ for (i = 0; i < 4; i++)
+ vp8_dec_reg_write(vpu, &vp8_dec_lf_level[i],
+ clamp(hdr->lf_header.level
+ + hdr->segment_header.lf_update[i], 0, 63));
+ }
+
+ reg = G1_REG_REF_PIC_FILT_SHARPNESS(hdr->lf_header.sharpness_level);
+ if (hdr->lf_header.type)
+ reg |= G1_REG_REF_PIC_FILT_TYPE_E;
+ vdpu_write_relaxed(vpu, reg, G1_REG_REF_PIC(0));
+
+ if (hdr->lf_header.flags & V4L2_VP8_LF_HDR_ADJ_ENABLE) {
+ for (i = 0; i < 4; i++) {
+ vp8_dec_reg_write(vpu, &vp8_dec_mb_adj[i],
+ hdr->lf_header.mb_mode_delta_magnitude[i]);
+ vp8_dec_reg_write(vpu, &vp8_dec_ref_adj[i],
+ hdr->lf_header.ref_frm_delta_magnitude[i]);
+ }
+ }
+}
+
+/*
+ * set quantization parameters
+ */
+static void cfg_qp(struct hantro_ctx *ctx,
+ const struct v4l2_ctrl_vp8_frame_header *hdr)
+{
+ struct hantro_dev *vpu = ctx->dev;
+ int i;
+
+ if (!(hdr->segment_header.flags & V4L2_VP8_SEGMNT_HDR_FLAG_ENABLED)) {
+ vp8_dec_reg_write(vpu, &vp8_dec_quant[0], hdr->quant_header.y_ac_qi);
+ } else if (hdr->segment_header.segment_feature_mode) {
+ /* absolute mode */
+ for (i = 0; i < 4; i++)
+ vp8_dec_reg_write(vpu, &vp8_dec_quant[i],
+ hdr->segment_header.quant_update[i]);
+ } else {
+ /* delta mode */
+ for (i = 0; i < 4; i++)
+ vp8_dec_reg_write(vpu, &vp8_dec_quant[i],
+ clamp(hdr->quant_header.y_ac_qi
+ + hdr->segment_header.quant_update[i],
+ 0, 127));
+ }
+
+ vp8_dec_reg_write(vpu, &vp8_dec_quant_delta[0], hdr->quant_header.y_dc_delta);
+ vp8_dec_reg_write(vpu, &vp8_dec_quant_delta[1], hdr->quant_header.y2_dc_delta);
+ vp8_dec_reg_write(vpu, &vp8_dec_quant_delta[2], hdr->quant_header.y2_ac_delta);
+ vp8_dec_reg_write(vpu, &vp8_dec_quant_delta[3], hdr->quant_header.uv_dc_delta);
+ vp8_dec_reg_write(vpu, &vp8_dec_quant_delta[4], hdr->quant_header.uv_ac_delta);
+}
+
+/*
+ * set control partition and dct partition regs
+ *
+ * VP8 frame stream data layout:
+ *
+ * first_part_size parttion_sizes[0]
+ * ^ ^
+ * src_dma | |
+ * ^ +--------+------+ +-----+-----+
+ * | | control part | | |
+ * +--------+----------------+------------------+-----------+-----+-----------+
+ * | tag 3B | extra 7B | hdr | mb_data | dct sz | dct part0 | ... | dct partn |
+ * +--------+-----------------------------------+-----------+-----+-----------+
+ * | | | |
+ * v +----+---+ v
+ * mb_start | src_dma_end
+ * v
+ * dct size part
+ * (num_dct-1)*3B
+ * Note:
+ * 1. only key frame has extra 7 bytes
+ * 2. all offsets are base on src_dma
+ * 3. number of dct parts is 1, 2, 4 or 8
+ * 4. the addresses set to vpu must be 64bits alignment
+ */
+static void cfg_parts(struct hantro_ctx *ctx,
+ const struct v4l2_ctrl_vp8_frame_header *hdr)
+{
+ struct hantro_dev *vpu = ctx->dev;
+ struct vb2_v4l2_buffer *vb2_src;
+ u32 first_part_offset = VP8_FRAME_IS_KEY_FRAME(hdr) ? 10 : 3;
+ u32 dct_part_total_len = 0;
+ u32 dct_size_part_size = 0;
+ u32 dct_part_offset = 0;
+ u32 mb_offset_bytes = 0;
+ u32 mb_offset_bits = 0;
+ u32 mb_start_bits = 0;
+ struct vp8_dec_reg reg;
+ dma_addr_t src_dma;
+ u32 mb_size = 0;
+ u32 count = 0;
+ u32 i;
+
+ vb2_src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
+ src_dma = vb2_dma_contig_plane_dma_addr(&vb2_src->vb2_buf, 0);
+
+ /*
+ * Calculate control partition mb data info
+ * @macroblock_bit_offset: bits offset of mb data from first
+ * part start pos
+ * @mb_offset_bits: bits offset of mb data from src_dma
+ * base addr
+ * @mb_offset_byte: bytes offset of mb data from src_dma
+ * base addr
+ * @mb_start_bits: bits offset of mb data from mb data
+ * 64bits alignment addr
+ */
+ mb_offset_bits = first_part_offset * 8
+ + hdr->macroblock_bit_offset + 8;
+ mb_offset_bytes = mb_offset_bits / 8;
+ mb_start_bits = mb_offset_bits
+ - (mb_offset_bytes & (~DEC_8190_ALIGN_MASK)) * 8;
+ mb_size = hdr->first_part_size
+ - (mb_offset_bytes - first_part_offset)
+ + (mb_offset_bytes & DEC_8190_ALIGN_MASK);
+
+ /* mb data aligned base addr */
+ vdpu_write_relaxed(vpu, (mb_offset_bytes & (~DEC_8190_ALIGN_MASK))
+ + src_dma, G1_REG_ADDR_REF(13));
+
+ /* mb data start bits */
+ reg.base = G1_REG_DEC_CTRL2;
+ reg.mask = 0x3f;
+ reg.shift = 18;
+ vp8_dec_reg_write(vpu, ®, mb_start_bits);
+
+ /* mb aligned data length */
+ reg.base = G1_REG_DEC_CTRL6;
+ reg.mask = 0x3fffff;
+ reg.shift = 0;
+ vp8_dec_reg_write(vpu, ®, mb_size + 1);
+
+ /*
+ * Calculate dct partition info
+ * @dct_size_part_size: Containing sizes of dct part, every dct part
+ * has 3 bytes to store its size, except the last
+ * dct part
+ * @dct_part_offset: bytes offset of dct parts from src_dma base addr
+ * @dct_part_total_len: total size of all dct parts
+ */
+ dct_size_part_size = (hdr->num_dct_parts - 1) * 3;
+ dct_part_offset = first_part_offset + hdr->first_part_size;
+ for (i = 0; i < hdr->num_dct_parts; i++)
+ dct_part_total_len += hdr->dct_part_sizes[i];
+ dct_part_total_len += dct_size_part_size;
+ dct_part_total_len += (dct_part_offset & DEC_8190_ALIGN_MASK);
+
+ /* number of dct partitions */
+ reg.base = G1_REG_DEC_CTRL6;
+ reg.mask = 0xf;
+ reg.shift = 24;
+ vp8_dec_reg_write(vpu, ®, hdr->num_dct_parts - 1);
+
+ /* dct partition length */
+ vdpu_write_relaxed(vpu,
+ G1_REG_DEC_CTRL3_STREAM_LEN(dct_part_total_len),
+ G1_REG_DEC_CTRL3);
+
+ /* dct partitions base address */
+ for (i = 0; i < hdr->num_dct_parts; i++) {
+ u32 byte_offset = dct_part_offset + dct_size_part_size + count;
+ u32 base_addr = byte_offset + src_dma;
+
+ vp8_dec_reg_write(vpu, &vp8_dec_dct_base[i],
+ base_addr & (~DEC_8190_ALIGN_MASK));
+
+ vp8_dec_reg_write(vpu, &vp8_dec_dct_start_bits[i],
+ (byte_offset & DEC_8190_ALIGN_MASK) * 8);
+
+ count += hdr->dct_part_sizes[i];
+ }
+}
+
+/*
+ * prediction filter taps
+ * normal 6-tap filters
+ */
+static void cfg_tap(struct hantro_ctx *ctx,
+ const struct v4l2_ctrl_vp8_frame_header *hdr)
+{
+ struct hantro_dev *vpu = ctx->dev;
+ struct vp8_dec_reg reg;
+ u32 val = 0;
+ int i, j;
+
+ reg.base = G1_REG_BD_REF_PIC(3);
+ reg.mask = 0xf;
+
+ if ((hdr->version & 0x03) != 0)
+ return; /* Tap filter not used. */
+
+ for (i = 0; i < 8; i++) {
+ val = (vp8_dec_mc_filter[i][0] << 2) | vp8_dec_mc_filter[i][5];
+
+ for (j = 0; j < 4; j++)
+ vp8_dec_reg_write(vpu, &vp8_dec_pred_bc_tap[i][j],
+ vp8_dec_mc_filter[i][j + 1]);
+
+ switch (i) {
+ case 2:
+ reg.shift = 8;
+ break;
+ case 4:
+ reg.shift = 4;
+ break;
+ case 6:
+ reg.shift = 0;
+ break;
+ default:
+ continue;
+ }
+
+ vp8_dec_reg_write(vpu, ®, val);
+ }
+}
+
+/* set reference frame */
+static void cfg_ref(struct hantro_ctx *ctx,
+ const struct v4l2_ctrl_vp8_frame_header *hdr)
+{
+ struct vb2_queue *cap_q = &ctx->fh.m2m_ctx->cap_q_ctx.q;
+ struct hantro_dev *vpu = ctx->dev;
+ struct vb2_v4l2_buffer *vb2_dst;
+ u32 reg;
+
+ vb2_dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+
+ /* set last frame address */
+ reg = hantro_get_ref(cap_q, hdr->last_frame_ts);
+ if (!reg)
+ reg = vb2_dma_contig_plane_dma_addr(&vb2_dst->vb2_buf, 0);
+ vdpu_write_relaxed(vpu, reg, G1_REG_ADDR_REF(0));
+
+ /* set golden reference frame buffer address */
+ reg = hantro_get_ref(cap_q, hdr->golden_frame_ts);
+ WARN_ON(!reg && hdr->golden_frame_ts);
+ if (!reg)
+ reg = vb2_dma_contig_plane_dma_addr(&vb2_dst->vb2_buf, 0);
+ if (hdr->sign_bias_golden)
+ reg |= G1_REG_ADDR_REF_TOPC_E;
+ vdpu_write_relaxed(vpu, reg, G1_REG_ADDR_REF(4));
+
+ /* set alternate reference frame buffer address */
+ reg = hantro_get_ref(cap_q, hdr->alt_frame_ts);
+ WARN_ON(!reg && hdr->alt_frame_ts);
+ if (!reg)
+ reg = vb2_dma_contig_plane_dma_addr(&vb2_dst->vb2_buf, 0);
+ if (hdr->sign_bias_alternate)
+ reg |= G1_REG_ADDR_REF_TOPC_E;
+ vdpu_write_relaxed(vpu, reg, G1_REG_ADDR_REF(5));
+}
+
+static void cfg_buffers(struct hantro_ctx *ctx,
+ const struct v4l2_ctrl_vp8_frame_header *hdr)
+{
+ struct hantro_dev *vpu = ctx->dev;
+ struct vb2_v4l2_buffer *vb2_dst;
+ u32 reg;
+
+ vb2_dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+
+ /* set probability table buffer address */
+ vdpu_write_relaxed(vpu, ctx->vp8_dec.prob_tbl.dma,
+ G1_REG_ADDR_QTABLE);
+
+ /* set segment map address */
+ reg = G1_REG_FWD_PIC1_SEGMENT_BASE(ctx->vp8_dec.segment_map.dma);
+ if (hdr->segment_header.flags & V4L2_VP8_SEGMNT_HDR_FLAG_ENABLED) {
+ reg |= G1_REG_FWD_PIC1_SEGMENT_E;
+ if (hdr->segment_header.flags & V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_MAP)
+ reg |= G1_REG_FWD_PIC1_SEGMENT_UPD_E;
+ }
+ vdpu_write_relaxed(vpu, reg, G1_REG_FWD_PIC(0));
+
+ /* set output frame buffer address */
+ vdpu_write_relaxed(vpu,
+ vb2_dma_contig_plane_dma_addr(&vb2_dst->vb2_buf, 0),
+ G1_REG_ADDR_DST);
+}
+
+void hantro_g1_vp8_dec_run(struct hantro_ctx *ctx)
+{
+ const struct v4l2_ctrl_vp8_frame_header *hdr;
+ struct hantro_dev *vpu = ctx->dev;
+ size_t height = ctx->dst_fmt.height;
+ size_t width = ctx->dst_fmt.width;
+ struct vb2_v4l2_buffer *vb2_src;
+ u32 mb_width, mb_height;
+ u32 reg;
+
+ vb2_src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
+ v4l2_ctrl_request_setup(vb2_src->vb2_buf.req_obj.req,
+ &ctx->ctrl_handler);
+
+ hdr = hantro_get_ctrl(ctx, V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR);
+ if (WARN_ON(!hdr))
+ return;
+
+ /* reset segment_map buffer in keyframe */
+ if (VP8_FRAME_IS_KEY_FRAME(hdr) && ctx->vp8_dec.segment_map.cpu)
+ memset(ctx->vp8_dec.segment_map.cpu, 0,
+ ctx->vp8_dec.segment_map.size);
+
+ hantro_vp8_prob_update(ctx, hdr);
+
+ reg = G1_REG_CONFIG_DEC_TIMEOUT_E |
+ G1_REG_CONFIG_DEC_STRENDIAN_E |
+ G1_REG_CONFIG_DEC_INSWAP32_E |
+ G1_REG_CONFIG_DEC_STRSWAP32_E |
+ G1_REG_CONFIG_DEC_OUTSWAP32_E |
+ G1_REG_CONFIG_DEC_CLK_GATE_E |
+ G1_REG_CONFIG_DEC_IN_ENDIAN |
+ G1_REG_CONFIG_DEC_OUT_ENDIAN |
+ G1_REG_CONFIG_DEC_MAX_BURST(16);
+ vdpu_write_relaxed(vpu, reg, G1_REG_CONFIG);
+
+ reg = G1_REG_DEC_CTRL0_DEC_MODE(10);
+ if (!VP8_FRAME_IS_KEY_FRAME(hdr))
+ reg |= G1_REG_DEC_CTRL0_PIC_INTER_E;
+ if (!(hdr->flags & V4L2_VP8_FRAME_HDR_FLAG_MB_NO_SKIP_COEFF))
+ reg |= G1_REG_DEC_CTRL0_SKIP_MODE;
+ if (hdr->lf_header.level == 0)
+ reg |= G1_REG_DEC_CTRL0_FILTERING_DIS;
+ vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0);
+
+ /* frame dimensions */
+ mb_width = DIV_ROUND_UP(width, 16);
+ mb_height = DIV_ROUND_UP(height, 16);
+ reg = G1_REG_DEC_CTRL1_PIC_MB_WIDTH(mb_width) |
+ G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(mb_height) |
+ G1_REG_DEC_CTRL1_PIC_MB_W_EXT(mb_width >> 9) |
+ G1_REG_DEC_CTRL1_PIC_MB_H_EXT(mb_height >> 8);
+ vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL1);
+
+ /* bool decode info */
+ reg = G1_REG_DEC_CTRL2_BOOLEAN_RANGE(hdr->coder_state.range)
+ | G1_REG_DEC_CTRL2_BOOLEAN_VALUE(hdr->coder_state.value);
+ vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL2);
+
+ reg = 0;
+ if (hdr->version != 3)
+ reg |= G1_REG_DEC_CTRL4_VC1_HEIGHT_EXT;
+ if (hdr->version & 0x3)
+ reg |= G1_REG_DEC_CTRL4_BILIN_MC_E;
+ vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL4);
+
+ cfg_lf(ctx, hdr);
+ cfg_qp(ctx, hdr);
+ cfg_parts(ctx, hdr);
+ cfg_tap(ctx, hdr);
+ cfg_ref(ctx, hdr);
+ cfg_buffers(ctx, hdr);
+
+ /* Controls no longer in-use, we can complete them */
+ v4l2_ctrl_request_complete(vb2_src->vb2_buf.req_obj.req,
+ &ctx->ctrl_handler);
+
+ schedule_delayed_work(&vpu->watchdog_work, msecs_to_jiffies(2000));
+
+ vdpu_write(vpu, G1_REG_INTERRUPT_DEC_E, G1_REG_INTERRUPT);
+}
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index 3c361c2e9b88..7849852affde 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -12,6 +12,7 @@
#include <linux/interrupt.h>
#include <linux/v4l2-controls.h>
#include <media/mpeg2-ctrls.h>
+#include <media/vp8-ctrls.h>
#include <media/videobuf2-core.h>
struct hantro_dev;
@@ -47,6 +48,16 @@ struct hantro_mpeg2_dec_hw_ctx {
struct hantro_aux_buf qtable;
};
+/**
+ * struct hantro_vp8d_hw_ctx
+ * @segment_map: Segment map buffer.
+ * @prob_tbl: Probability table buffer.
+ */
+struct hantro_vp8_dec_hw_ctx {
+ struct hantro_aux_buf segment_map;
+ struct hantro_aux_buf prob_tbl;
+};
+
/**
* struct hantro_codec_ops - codec mode specific operations
*
@@ -99,4 +110,10 @@ void hantro_mpeg2_dec_copy_qtable(u8 *qtable,
int hantro_mpeg2_dec_init(struct hantro_ctx *ctx);
void hantro_mpeg2_dec_exit(struct hantro_ctx *ctx);
+void hantro_g1_vp8_dec_run(struct hantro_ctx *ctx);
+int hantro_vp8_dec_init(struct hantro_ctx *ctx);
+void hantro_vp8_dec_exit(struct hantro_ctx *ctx);
+void hantro_vp8_prob_update(struct hantro_ctx *ctx,
+ const struct v4l2_ctrl_vp8_frame_header *hdr);
+
#endif /* HANTRO_HW_H_ */
diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
index b4af8c659a98..507369c1d17d 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -344,6 +344,7 @@ hantro_update_requires_request(struct hantro_ctx *ctx, u32 fourcc)
ctx->fh.m2m_ctx->out_q_ctx.q.requires_requests = false;
break;
case V4L2_PIX_FMT_MPEG2_SLICE:
+ case V4L2_PIX_FMT_VP8_FRAME:
ctx->fh.m2m_ctx->out_q_ctx.q.requires_requests = true;
break;
default:
diff --git a/drivers/staging/media/hantro/hantro_vp8.c b/drivers/staging/media/hantro/hantro_vp8.c
new file mode 100644
index 000000000000..be445a89d860
--- /dev/null
+++ b/drivers/staging/media/hantro/hantro_vp8.c
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Hantro VPU codec driver
+ *
+ * Copyright (C) 2018 Rockchip Electronics Co., Ltd.
+ */
+
+#include "hantro.h"
+
+/*
+ * probs table with packed
+ */
+struct vp8_prob_tbl_packed {
+ u8 prob_mb_skip_false;
+ u8 prob_intra;
+ u8 prob_ref_last;
+ u8 prob_ref_golden;
+ u8 prob_segment[3];
+ u8 padding0;
+
+ u8 prob_luma_16x16_pred_mode[4];
+ u8 prob_chroma_pred_mode[3];
+ u8 padding1;
+
+ /* mv prob */
+ u8 prob_mv_context[2][19];
+ u8 padding2[2];
+
+ /* coeff probs */
+ u8 prob_coeffs[4][8][3][11];
+ u8 padding3[96];
+};
+
+void hantro_vp8_prob_update(struct hantro_ctx *ctx,
+ const struct v4l2_ctrl_vp8_frame_header *hdr)
+{
+ const struct v4l2_vp8_entropy_header *entropy_hdr = &hdr->entropy_header;
+ u32 i, j, k;
+ u8 *dst;
+
+ /* first probs */
+ dst = ctx->vp8_dec.prob_tbl.cpu;
+
+ dst[0] = hdr->prob_skip_false;
+ dst[1] = hdr->prob_intra;
+ dst[2] = hdr->prob_last;
+ dst[3] = hdr->prob_gf;
+ dst[4] = hdr->segment_header.segment_probs[0];
+ dst[5] = hdr->segment_header.segment_probs[1];
+ dst[6] = hdr->segment_header.segment_probs[2];
+ dst[7] = 0;
+
+ dst += 8;
+ dst[0] = entropy_hdr->y_mode_probs[0];
+ dst[1] = entropy_hdr->y_mode_probs[1];
+ dst[2] = entropy_hdr->y_mode_probs[2];
+ dst[3] = entropy_hdr->y_mode_probs[3];
+ dst[4] = entropy_hdr->uv_mode_probs[0];
+ dst[5] = entropy_hdr->uv_mode_probs[1];
+ dst[6] = entropy_hdr->uv_mode_probs[2];
+ dst[7] = 0; /*unused */
+
+ /* mv probs */
+ dst += 8;
+ dst[0] = entropy_hdr->mv_probs[0][0]; /* is short */
+ dst[1] = entropy_hdr->mv_probs[1][0];
+ dst[2] = entropy_hdr->mv_probs[0][1]; /* sign */
+ dst[3] = entropy_hdr->mv_probs[1][1];
+ dst[4] = entropy_hdr->mv_probs[0][8 + 9];
+ dst[5] = entropy_hdr->mv_probs[0][9 + 9];
+ dst[6] = entropy_hdr->mv_probs[1][8 + 9];
+ dst[7] = entropy_hdr->mv_probs[1][9 + 9];
+ dst += 8;
+ for (i = 0; i < 2; ++i) {
+ for (j = 0; j < 8; j += 4) {
+ dst[0] = entropy_hdr->mv_probs[i][j + 9 + 0];
+ dst[1] = entropy_hdr->mv_probs[i][j + 9 + 1];
+ dst[2] = entropy_hdr->mv_probs[i][j + 9 + 2];
+ dst[3] = entropy_hdr->mv_probs[i][j + 9 + 3];
+ dst += 4;
+ }
+ }
+ for (i = 0; i < 2; ++i) {
+ dst[0] = entropy_hdr->mv_probs[i][0 + 2];
+ dst[1] = entropy_hdr->mv_probs[i][1 + 2];
+ dst[2] = entropy_hdr->mv_probs[i][2 + 2];
+ dst[3] = entropy_hdr->mv_probs[i][3 + 2];
+ dst[4] = entropy_hdr->mv_probs[i][4 + 2];
+ dst[5] = entropy_hdr->mv_probs[i][5 + 2];
+ dst[6] = entropy_hdr->mv_probs[i][6 + 2];
+ dst[7] = 0; /*unused */
+ dst += 8;
+ }
+
+ /* coeff probs (header part) */
+ dst = ctx->vp8_dec.prob_tbl.cpu;
+ dst += (8 * 7);
+ for (i = 0; i < 4; ++i) {
+ for (j = 0; j < 8; ++j) {
+ for (k = 0; k < 3; ++k) {
+ dst[0] = entropy_hdr->coeff_probs[i][j][k][0];
+ dst[1] = entropy_hdr->coeff_probs[i][j][k][1];
+ dst[2] = entropy_hdr->coeff_probs[i][j][k][2];
+ dst[3] = entropy_hdr->coeff_probs[i][j][k][3];
+ dst += 4;
+ }
+ }
+ }
+
+ /* coeff probs (footer part) */
+ dst = ctx->vp8_dec.prob_tbl.cpu;
+ dst += (8 * 55);
+ for (i = 0; i < 4; ++i) {
+ for (j = 0; j < 8; ++j) {
+ for (k = 0; k < 3; ++k) {
+ dst[0] = entropy_hdr->coeff_probs[i][j][k][4];
+ dst[1] = entropy_hdr->coeff_probs[i][j][k][5];
+ dst[2] = entropy_hdr->coeff_probs[i][j][k][6];
+ dst[3] = entropy_hdr->coeff_probs[i][j][k][7];
+ dst[4] = entropy_hdr->coeff_probs[i][j][k][8];
+ dst[5] = entropy_hdr->coeff_probs[i][j][k][9];
+ dst[6] = entropy_hdr->coeff_probs[i][j][k][10];
+ dst[7] = 0; /*unused */
+ dst += 8;
+ }
+ }
+ }
+}
+
+int hantro_vp8_dec_init(struct hantro_ctx *ctx)
+{
+ struct hantro_dev *vpu = ctx->dev;
+ struct hantro_aux_buf *aux_buf;
+ unsigned int mb_width, mb_height;
+ size_t segment_map_size;
+ int ret;
+
+ /* segment map table size calculation */
+ mb_width = DIV_ROUND_UP(ctx->dst_fmt.width, 16);
+ mb_height = DIV_ROUND_UP(ctx->dst_fmt.height, 16);
+ segment_map_size = round_up(DIV_ROUND_UP(mb_width * mb_height, 4), 64);
+
+ /*
+ * In context init the dma buffer for segment map must be allocated.
+ * And the data in segment map buffer must be set to all zero.
+ */
+ aux_buf = &ctx->vp8_dec.segment_map;
+ aux_buf->size = segment_map_size;
+ aux_buf->cpu = dma_alloc_coherent(vpu->dev, aux_buf->size,
+ &aux_buf->dma, GFP_KERNEL);
+ if (!aux_buf->cpu)
+ return -ENOMEM;
+
+ memset(aux_buf->cpu, 0, aux_buf->size);
+
+ /*
+ * Allocate probability table buffer,
+ * total 1208 bytes, 4K page is far enough.
+ */
+ aux_buf = &ctx->vp8_dec.prob_tbl;
+ aux_buf->size = sizeof(struct vp8_prob_tbl_packed);
+ aux_buf->cpu = dma_alloc_coherent(vpu->dev, aux_buf->size,
+ &aux_buf->dma, GFP_KERNEL);
+ if (!aux_buf->cpu) {
+ ret = -ENOMEM;
+ goto err_free_seg_map;
+ }
+
+ return 0;
+
+err_free_seg_map:
+ dma_free_coherent(vpu->dev, ctx->vp8_dec.segment_map.size,
+ ctx->vp8_dec.segment_map.cpu,
+ ctx->vp8_dec.segment_map.dma);
+
+ return ret;
+}
+
+void hantro_vp8_dec_exit(struct hantro_ctx *ctx)
+{
+ struct hantro_vp8_dec_hw_ctx *vp8_dec = &ctx->vp8_dec;
+ struct hantro_dev *vpu = ctx->dev;
+
+ dma_free_coherent(vpu->dev, vp8_dec->segment_map.size,
+ vp8_dec->segment_map.cpu, vp8_dec->segment_map.dma);
+ dma_free_coherent(vpu->dev, vp8_dec->prob_tbl.size,
+ vp8_dec->prob_tbl.cpu, vp8_dec->prob_tbl.dma);
+}
diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c b/drivers/staging/media/hantro/rk3288_vpu_hw.c
index bcacc4f51093..470e803e25a6 100644
--- a/drivers/staging/media/hantro/rk3288_vpu_hw.c
+++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c
@@ -74,6 +74,19 @@ static const struct hantro_fmt rk3288_vpu_dec_fmts[] = {
.step_height = MPEG2_MB_DIM,
},
},
+ {
+ .fourcc = V4L2_PIX_FMT_VP8_FRAME,
+ .codec_mode = HANTRO_MODE_VP8_DEC,
+ .max_depth = 2,
+ .frmsize = {
+ .min_width = 48,
+ .max_width = 3840,
+ .step_width = 16,
+ .min_height = 48,
+ .max_height = 2160,
+ .step_height = 16,
+ },
+ },
};
static irqreturn_t rk3288_vepu_irq(int irq, void *dev_id)
@@ -155,6 +168,12 @@ static const struct hantro_codec_ops rk3288_vpu_codec_ops[] = {
.init = hantro_mpeg2_dec_init,
.exit = hantro_mpeg2_dec_exit,
},
+ [HANTRO_MODE_VP8_DEC] = {
+ .run = hantro_g1_vp8_dec_run,
+ .reset = rk3288_vpu_dec_reset,
+ .init = hantro_vp8_dec_init,
+ .exit = hantro_vp8_dec_exit,
+ },
};
/*
@@ -177,7 +196,8 @@ const struct hantro_variant rk3288_vpu_variant = {
.dec_offset = 0x400,
.dec_fmts = rk3288_vpu_dec_fmts,
.num_dec_fmts = ARRAY_SIZE(rk3288_vpu_dec_fmts),
- .codec = HANTRO_JPEG_ENCODER | HANTRO_MPEG2_DECODER,
+ .codec = HANTRO_JPEG_ENCODER | HANTRO_MPEG2_DECODER |
+ HANTRO_VP8_DECODER,
.codec_ops = rk3288_vpu_codec_ops,
.irqs = rk3288_irqs,
.num_irqs = ARRAY_SIZE(rk3288_irqs),
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: uapi: Add VP8 stateless decoder API
2019-06-13 15:10 ` [PATCH 1/2] media: uapi: Add VP8 stateless decoder API Ezequiel Garcia
@ 2019-06-17 13:23 ` Boris Brezillon
2019-06-20 3:46 ` Ezequiel Garcia
0 siblings, 1 reply; 7+ messages in thread
From: Boris Brezillon @ 2019-06-17 13:23 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: linux-media, Hans Verkuil, kernel, Nicolas Dufresne, Tomasz Figa,
linux-rockchip, Heiko Stuebner, Jonas Karlman, Philipp Zabel,
Pawel Osciak
On Thu, 13 Jun 2019 12:10:39 -0300
Ezequiel Garcia <ezequiel@collabora.com> wrote:
> From: Pawel Osciak <posciak@chromium.org>
>
> Add the parsed VP8 frame pixel format and controls, to be used
> with the new stateless decoder API for VP8 to provide parameters
> for accelerator (aka stateless) codecs.
>
> Signed-off-by: Pawel Osciak <posciak@chromium.org>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> --
> Changes from RFC:
> * Make sure the uAPI has the same size on x86, x86_64, arm and arm64.
> * Move entropy coder state fields to a struct.
> * Move key_frame field to the flags.
> * Remove unneeded first_part_offset field.
> * Add documentation.
> ---
> Documentation/media/uapi/v4l/biblio.rst | 10 +
> .../media/uapi/v4l/ext-ctrls-codec.rst | 311 ++++++++++++++++++
> .../media/uapi/v4l/pixfmt-compressed.rst | 20 ++
> drivers/media/v4l2-core/v4l2-ctrls.c | 8 +
> drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
> include/media/v4l2-ctrls.h | 3 +
> include/media/vp8-ctrls.h | 110 +++++++
> 7 files changed, 463 insertions(+)
> create mode 100644 include/media/vp8-ctrls.h
>
> diff --git a/Documentation/media/uapi/v4l/biblio.rst b/Documentation/media/uapi/v4l/biblio.rst
> index 8f4eb8823d82..ad2ff258afa8 100644
> --- a/Documentation/media/uapi/v4l/biblio.rst
> +++ b/Documentation/media/uapi/v4l/biblio.rst
> @@ -395,3 +395,13 @@ colimg
> :title: Color Imaging: Fundamentals and Applications
>
> :author: Erik Reinhard et al.
> +
> +.. _vp8:
> +
> +VP8
> +===
> +
> +
> +:title: RFC 6386: "VP8 Data Format and Decoding Guide"
> +
> +:author: J. Bankoski et al.
> diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> index d6ea2ffd65c5..7a1947f5be96 100644
> --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> @@ -2234,6 +2234,317 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> Quantization parameter for a P frame for FWHT. Valid range: from 1
> to 31.
>
> +.. _v4l2-mpeg-vp8:
> +
> +``V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR (struct)``
> + Specifies the frame parameters for the associated VP8 parsed frame data.
> + This includes the necessary parameters for
> + configuring a stateless hardware decoding pipeline for VP8.
> + The bitstream parameters are defined according to :ref:`vp8`.
> +
> + .. note::
> +
> + This compound control is not yet part of the public kernel API and
> + it is expected to change.
> +
> +.. c:type:: v4l2_ctrl_vp8_frame_header
> +
> +.. cssclass:: longtable
> +
> +.. tabularcolumns:: |p{5.8cm}|p{4.8cm}|p{6.6cm}|
> +
> +.. flat-table:: struct v4l2_ctrl_vp8_frame_header
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 1 1 2
> +
> + * - __u8
> + - ``version``
> + - Bitstream version.
> + * - __u16
> + - ``width``
> + - The width of the frame
Nit: sometimes you finish your description with a final dot sometimes
not. It's probably better to make that consistent.
[...]
> diff --git a/include/media/vp8-ctrls.h b/include/media/vp8-ctrls.h
> new file mode 100644
> index 000000000000..3b0dcc125e25
> --- /dev/null
> +++ b/include/media/vp8-ctrls.h
> @@ -0,0 +1,110 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * These are the VP8 state controls for use with stateless VP8
> + * codec drivers.
> + *
> + * It turns out that these structs are not stable yet and will undergo
> + * more changes. So keep them private until they are stable and ready to
> + * become part of the official public API.
> + */
> +
> +#ifndef _VP8_CTRLS_H_
> +#define _VP8_CTRLS_H_
> +
> +#include <linux/v4l2-controls.h>
> +
> +#define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frames */
> +
> +#define V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR (V4L2_CID_MPEG_BASE + 2000)
> +#define V4L2_CTRL_TYPE_VP8_FRAME_HDR 0x301
> +
> +#define V4L2_VP8_SEGMNT_HDR_FLAG_ENABLED 0x01
> +#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_MAP 0x02
> +#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_FEATURE_DATA 0x04
> +
> +struct v4l2_vp8_segment_header {
> + __u8 segment_feature_mode;
> + __s8 quant_update[4];
> + __s8 lf_update[4];
> + __u8 segment_probs[3];
> + __u32 flags;
> +};
> +
> +#define V4L2_VP8_LF_HDR_ADJ_ENABLE 0x01
> +#define V4L2_VP8_LF_HDR_DELTA_UPDATE 0x02
> +struct v4l2_vp8_loopfilter_header {
> + __u16 type;
> + __u8 level;
> + __u8 sharpness_level;
> + __s8 ref_frm_delta_magnitude[4];
> + __s8 mb_mode_delta_magnitude[4];
> + __u16 flags;
> +};
> +
> +struct v4l2_vp8_quantization_header {
> + __u8 y_ac_qi;
> + __s8 y_dc_delta;
> + __s8 y2_dc_delta;
> + __s8 y2_ac_delta;
> + __s8 uv_dc_delta;
> + __s8 uv_ac_delta;
Don't know what the policy in V4L, but it's usually a good thing to
have structs used to exchange data with userspace aligned on 64-bits.
Maybe you can add 2 padding bytes here. Note that the compiler might
anyway add padding to align struct fields to their natural alignment in
v4l2_ctrl_vp8_frame_header, so it's probably better to have these
padding bytes explicitly defined and mandate that they be set to 0 so
we can check them and complain when they're not 0.
> + __u16 dequant_factors[4][3][2];
> +};
> +
> +struct v4l2_vp8_entropy_header {
> + __u8 coeff_probs[4][8][3][11];
> + __u8 y_mode_probs[4];
> + __u8 uv_mode_probs[3];
> + __u8 mv_probs[2][19];
Same here
> +};
> +
> +struct v4l2_vp8_entropy_coder_state {
> + __u8 range;
> + __u8 value;
> + __u8 bit_count;
and here.
> +};
> +
> +#define V4L2_VP8_FRAME_HDR_FLAG_KEY_FRAME 0x01
> +#define V4L2_VP8_FRAME_HDR_FLAG_EXPERIMENTAL 0x02
> +#define V4L2_VP8_FRAME_HDR_FLAG_SHOW_FRAME 0x04
> +#define V4L2_VP8_FRAME_HDR_FLAG_MB_NO_SKIP_COEFF 0x08
> +
> +#define VP8_FRAME_IS_KEY_FRAME(hdr) (!!(hdr->flags & V4L2_VP8_FRAME_HDR_FLAG_KEY_FRAME))
> +
> +struct v4l2_ctrl_vp8_frame_header {
> + __u8 version;
> +
Maybe we should try to pack things so that the compiler does not
implicitly add extra padding bytes. That implies trying to group u8
fields by 2, 4 or 8 depending on what the next field natural alignment
is.
> + /* Populated also if not a key frame */
> + __u16 width;
> + __u16 height;
> + __u8 horizontal_scale;
> + __u8 vertical_scale;
> +
> + struct v4l2_vp8_segment_header segment_header;
> + struct v4l2_vp8_loopfilter_header lf_header;
> + struct v4l2_vp8_quantization_header quant_header;
> + struct v4l2_vp8_entropy_header entropy_header;
> +
> + __u16 sign_bias_golden;
> + __u16 sign_bias_alternate;
> +
> + __u8 prob_skip_false;
> + __u8 prob_intra;
> + __u8 prob_last;
> + __u8 prob_gf;
> +
> + __u32 first_part_size;
> + __u32 macroblock_bit_offset;
> + __u32 dct_part_sizes[8];
> + __u8 num_dct_parts;
> +
> + struct v4l2_vp8_entropy_coder_state coder_state;
> +
> + __u64 last_frame_ts;
> + __u64 golden_frame_ts;
> + __u64 alt_frame_ts;
> +
> + __u64 flags;
> +};
> +
> +#endif
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: hantro: Add support for VP8 decoding on rk3288
2019-06-13 15:10 ` [PATCH 2/2] media: hantro: Add support for VP8 decoding on rk3288 Ezequiel Garcia
@ 2019-06-17 13:40 ` Boris Brezillon
2019-06-20 3:47 ` Ezequiel Garcia
0 siblings, 1 reply; 7+ messages in thread
From: Boris Brezillon @ 2019-06-17 13:40 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: linux-media, Hans Verkuil, kernel, Nicolas Dufresne, Tomasz Figa,
linux-rockchip, Heiko Stuebner, Jonas Karlman, Philipp Zabel,
ZhiChao Yu
On Thu, 13 Jun 2019 12:10:40 -0300
Ezequiel Garcia <ezequiel@collabora.com> wrote:
> +static void cfg_parts(struct hantro_ctx *ctx,
> + const struct v4l2_ctrl_vp8_frame_header *hdr)
> +{
> + struct hantro_dev *vpu = ctx->dev;
> + struct vb2_v4l2_buffer *vb2_src;
> + u32 first_part_offset = VP8_FRAME_IS_KEY_FRAME(hdr) ? 10 : 3;
> + u32 dct_part_total_len = 0;
> + u32 dct_size_part_size = 0;
> + u32 dct_part_offset = 0;
> + u32 mb_offset_bytes = 0;
> + u32 mb_offset_bits = 0;
> + u32 mb_start_bits = 0;
> + struct vp8_dec_reg reg;
> + dma_addr_t src_dma;
> + u32 mb_size = 0;
> + u32 count = 0;
> + u32 i;
> +
> + vb2_src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> + src_dma = vb2_dma_contig_plane_dma_addr(&vb2_src->vb2_buf, 0);
> +
> + /*
> + * Calculate control partition mb data info
> + * @macroblock_bit_offset: bits offset of mb data from first
> + * part start pos
> + * @mb_offset_bits: bits offset of mb data from src_dma
> + * base addr
> + * @mb_offset_byte: bytes offset of mb data from src_dma
> + * base addr
> + * @mb_start_bits: bits offset of mb data from mb data
> + * 64bits alignment addr
> + */
> + mb_offset_bits = first_part_offset * 8
> + + hdr->macroblock_bit_offset + 8;
> + mb_offset_bytes = mb_offset_bits / 8;
> + mb_start_bits = mb_offset_bits
> + - (mb_offset_bytes & (~DEC_8190_ALIGN_MASK)) * 8;
> + mb_size = hdr->first_part_size
> + - (mb_offset_bytes - first_part_offset)
> + + (mb_offset_bytes & DEC_8190_ALIGN_MASK);
Nit: can we have operators placed at the end of a line instead of
the beginning of the following line?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: uapi: Add VP8 stateless decoder API
2019-06-17 13:23 ` Boris Brezillon
@ 2019-06-20 3:46 ` Ezequiel Garcia
0 siblings, 0 replies; 7+ messages in thread
From: Ezequiel Garcia @ 2019-06-20 3:46 UTC (permalink / raw)
To: Boris Brezillon
Cc: linux-media, Hans Verkuil, kernel, Nicolas Dufresne, Tomasz Figa,
linux-rockchip, Heiko Stuebner, Jonas Karlman, Philipp Zabel,
Pawel Osciak
Hi Boris,
Thanks for the review.
On Mon, 2019-06-17 at 15:23 +0200, Boris Brezillon wrote:
> On Thu, 13 Jun 2019 12:10:39 -0300
> Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> > From: Pawel Osciak <posciak@chromium.org>
> >
> > Add the parsed VP8 frame pixel format and controls, to be used
> > with the new stateless decoder API for VP8 to provide parameters
> > for accelerator (aka stateless) codecs.
> >
> > Signed-off-by: Pawel Osciak <posciak@chromium.org>
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > --
> > Changes from RFC:
> > * Make sure the uAPI has the same size on x86, x86_64, arm and arm64.
> > * Move entropy coder state fields to a struct.
> > * Move key_frame field to the flags.
> > * Remove unneeded first_part_offset field.
> > * Add documentation.
> > ---
> > Documentation/media/uapi/v4l/biblio.rst | 10 +
> > .../media/uapi/v4l/ext-ctrls-codec.rst | 311 ++++++++++++++++++
> > .../media/uapi/v4l/pixfmt-compressed.rst | 20 ++
> > drivers/media/v4l2-core/v4l2-ctrls.c | 8 +
> > drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
> > include/media/v4l2-ctrls.h | 3 +
> > include/media/vp8-ctrls.h | 110 +++++++
> > 7 files changed, 463 insertions(+)
> > create mode 100644 include/media/vp8-ctrls.h
> >
> > diff --git a/Documentation/media/uapi/v4l/biblio.rst b/Documentation/media/uapi/v4l/biblio.rst
> > index 8f4eb8823d82..ad2ff258afa8 100644
> > --- a/Documentation/media/uapi/v4l/biblio.rst
> > +++ b/Documentation/media/uapi/v4l/biblio.rst
> > @@ -395,3 +395,13 @@ colimg
> > :title: Color Imaging: Fundamentals and Applications
> >
> > :author: Erik Reinhard et al.
> > +
> > +.. _vp8:
> > +
> > +VP8
> > +===
> > +
> > +
> > +:title: RFC 6386: "VP8 Data Format and Decoding Guide"
> > +
> > +:author: J. Bankoski et al.
> > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > index d6ea2ffd65c5..7a1947f5be96 100644
> > --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > @@ -2234,6 +2234,317 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> > Quantization parameter for a P frame for FWHT. Valid range: from 1
> > to 31.
> >
> > +.. _v4l2-mpeg-vp8:
> > +
> > +``V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR (struct)``
> > + Specifies the frame parameters for the associated VP8 parsed frame data.
> > + This includes the necessary parameters for
> > + configuring a stateless hardware decoding pipeline for VP8.
> > + The bitstream parameters are defined according to :ref:`vp8`.
> > +
> > + .. note::
> > +
> > + This compound control is not yet part of the public kernel API and
> > + it is expected to change.
> > +
> > +.. c:type:: v4l2_ctrl_vp8_frame_header
> > +
> > +.. cssclass:: longtable
> > +
> > +.. tabularcolumns:: |p{5.8cm}|p{4.8cm}|p{6.6cm}|
> > +
> > +.. flat-table:: struct v4l2_ctrl_vp8_frame_header
> > + :header-rows: 0
> > + :stub-columns: 0
> > + :widths: 1 1 2
> > +
> > + * - __u8
> > + - ``version``
> > + - Bitstream version.
> > + * - __u16
> > + - ``width``
> > + - The width of the frame
>
> Nit: sometimes you finish your description with a final dot sometimes
> not. It's probably better to make that consistent.
>
>
Yes, this is an overlook on my side. I'll fix this.
> [...]
>
> > diff --git a/include/media/vp8-ctrls.h b/include/media/vp8-ctrls.h
> > new file mode 100644
> > index 000000000000..3b0dcc125e25
> > --- /dev/null
> > +++ b/include/media/vp8-ctrls.h
> > @@ -0,0 +1,110 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * These are the VP8 state controls for use with stateless VP8
> > + * codec drivers.
> > + *
> > + * It turns out that these structs are not stable yet and will undergo
> > + * more changes. So keep them private until they are stable and ready to
> > + * become part of the official public API.
> > + */
> > +
> > +#ifndef _VP8_CTRLS_H_
> > +#define _VP8_CTRLS_H_
> > +
> > +#include <linux/v4l2-controls.h>
> > +
> > +#define V4L2_PIX_FMT_VP8_FRAME v4l2_fourcc('V', 'P', '8', 'F') /* VP8 parsed frames */
> > +
> > +#define V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR (V4L2_CID_MPEG_BASE + 2000)
> > +#define V4L2_CTRL_TYPE_VP8_FRAME_HDR 0x301
> > +
> > +#define V4L2_VP8_SEGMNT_HDR_FLAG_ENABLED 0x01
> > +#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_MAP 0x02
> > +#define V4L2_VP8_SEGMNT_HDR_FLAG_UPDATE_FEATURE_DATA 0x04
> > +
> > +struct v4l2_vp8_segment_header {
> > + __u8 segment_feature_mode;
> > + __s8 quant_update[4];
> > + __s8 lf_update[4];
> > + __u8 segment_probs[3];
> > + __u32 flags;
> > +};
> > +
> > +#define V4L2_VP8_LF_HDR_ADJ_ENABLE 0x01
> > +#define V4L2_VP8_LF_HDR_DELTA_UPDATE 0x02
> > +struct v4l2_vp8_loopfilter_header {
> > + __u16 type;
> > + __u8 level;
> > + __u8 sharpness_level;
> > + __s8 ref_frm_delta_magnitude[4];
> > + __s8 mb_mode_delta_magnitude[4];
> > + __u16 flags;
> > +};
> > +
> > +struct v4l2_vp8_quantization_header {
> > + __u8 y_ac_qi;
> > + __s8 y_dc_delta;
> > + __s8 y2_dc_delta;
> > + __s8 y2_ac_delta;
> > + __s8 uv_dc_delta;
> > + __s8 uv_ac_delta;
>
> Don't know what the policy in V4L, but it's usually a good thing to
> have structs used to exchange data with userspace aligned on 64-bits.
> Maybe you can add 2 padding bytes here. Note that the compiler might
> anyway add padding to align struct fields to their natural alignment in
> v4l2_ctrl_vp8_frame_header, so it's probably better to have these
> padding bytes explicitly defined and mandate that they be set to 0 so
> we can check them and complain when they're not 0.
>
AFAIK, there is no such requirement. There is however the requirement
of making sure the structs layout is the same on 32 and 64 architectures [1].
This is confirmed by these [2] tests which are based in Maxime's work.
[1] http://lkml.iu.edu/hypermail/linux/kernel/1806.1/05059.html
[2] https://gitlab.collabora.com/ezequiel/v4l2-ctrl-abi-check
> >
> > + __u16 dequant_factors[4][3][2];
> > +};
> > +
> > +struct v4l2_vp8_entropy_header {
> > + __u8 coeff_probs[4][8][3][11];
> > + __u8 y_mode_probs[4];
> > + __u8 uv_mode_probs[3];
> > + __u8 mv_probs[2][19];
>
> Same here
>
> > +};
> > +
> > +struct v4l2_vp8_entropy_coder_state {
> > + __u8 range;
> > + __u8 value;
> > + __u8 bit_count;
>
> and here.
>
> > +};
> > +
> > +#define V4L2_VP8_FRAME_HDR_FLAG_KEY_FRAME 0x01
> > +#define V4L2_VP8_FRAME_HDR_FLAG_EXPERIMENTAL 0x02
> > +#define V4L2_VP8_FRAME_HDR_FLAG_SHOW_FRAME 0x04
> > +#define V4L2_VP8_FRAME_HDR_FLAG_MB_NO_SKIP_COEFF 0x08
> > +
> > +#define VP8_FRAME_IS_KEY_FRAME(hdr) (!!(hdr->flags & V4L2_VP8_FRAME_HDR_FLAG_KEY_FRAME))
> > +
> > +struct v4l2_ctrl_vp8_frame_header {
> > + __u8 version;
> > +
>
> Maybe we should try to pack things so that the compiler does not
> implicitly add extra padding bytes. That implies trying to group u8
> fields by 2, 4 or 8 depending on what the next field natural alignment
> is.
>
Yes, I guess it makes sense to avoid implicit padding if possible.
> > + /* Populated also if not a key frame */
Note to self, this comment should be dropped and moved to the documentation.
> > + __u16 width;
> > + __u16 height;
> > + __u8 horizontal_scale;
> > + __u8 vertical_scale;
> > +
> > + struct v4l2_vp8_segment_header segment_header;
> > + struct v4l2_vp8_loopfilter_header lf_header;
> > + struct v4l2_vp8_quantization_header quant_header;
> > + struct v4l2_vp8_entropy_header entropy_header;
> > +
> > + __u16 sign_bias_golden;
> > + __u16 sign_bias_alternate;
> > +
> > + __u8 prob_skip_false;
> > + __u8 prob_intra;
> > + __u8 prob_last;
> > + __u8 prob_gf;
> > +
> > + __u32 first_part_size;
> > + __u32 macroblock_bit_offset;
> > + __u32 dct_part_sizes[8];
> > + __u8 num_dct_parts;
> > +
> > + struct v4l2_vp8_entropy_coder_state coder_state;
> > +
> > + __u64 last_frame_ts;
> > + __u64 golden_frame_ts;
> > + __u64 alt_frame_ts;
> > +
> > + __u64 flags;
> > +};
> > +
> > +#endif
Thanks,
Ezequiel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: hantro: Add support for VP8 decoding on rk3288
2019-06-17 13:40 ` Boris Brezillon
@ 2019-06-20 3:47 ` Ezequiel Garcia
0 siblings, 0 replies; 7+ messages in thread
From: Ezequiel Garcia @ 2019-06-20 3:47 UTC (permalink / raw)
To: Boris Brezillon
Cc: linux-media, Hans Verkuil, kernel, Nicolas Dufresne, Tomasz Figa,
linux-rockchip, Heiko Stuebner, Jonas Karlman, Philipp Zabel,
ZhiChao Yu
On Mon, 2019-06-17 at 15:40 +0200, Boris Brezillon wrote:
> On Thu, 13 Jun 2019 12:10:40 -0300
> Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
>
> > +static void cfg_parts(struct hantro_ctx *ctx,
> > + const struct v4l2_ctrl_vp8_frame_header *hdr)
> > +{
> > + struct hantro_dev *vpu = ctx->dev;
> > + struct vb2_v4l2_buffer *vb2_src;
> > + u32 first_part_offset = VP8_FRAME_IS_KEY_FRAME(hdr) ? 10 : 3;
> > + u32 dct_part_total_len = 0;
> > + u32 dct_size_part_size = 0;
> > + u32 dct_part_offset = 0;
> > + u32 mb_offset_bytes = 0;
> > + u32 mb_offset_bits = 0;
> > + u32 mb_start_bits = 0;
> > + struct vp8_dec_reg reg;
> > + dma_addr_t src_dma;
> > + u32 mb_size = 0;
> > + u32 count = 0;
> > + u32 i;
> > +
> > + vb2_src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > + src_dma = vb2_dma_contig_plane_dma_addr(&vb2_src->vb2_buf, 0);
> > +
> > + /*
> > + * Calculate control partition mb data info
> > + * @macroblock_bit_offset: bits offset of mb data from first
> > + * part start pos
> > + * @mb_offset_bits: bits offset of mb data from src_dma
> > + * base addr
> > + * @mb_offset_byte: bytes offset of mb data from src_dma
> > + * base addr
> > + * @mb_start_bits: bits offset of mb data from mb data
> > + * 64bits alignment addr
> > + */
> > + mb_offset_bits = first_part_offset * 8
> > + + hdr->macroblock_bit_offset + 8;
> > + mb_offset_bytes = mb_offset_bits / 8;
> > + mb_start_bits = mb_offset_bits
> > + - (mb_offset_bytes & (~DEC_8190_ALIGN_MASK)) * 8;
> > + mb_size = hdr->first_part_size
> > + - (mb_offset_bytes - first_part_offset)
> > + + (mb_offset_bytes & DEC_8190_ALIGN_MASK);
>
> Nit: can we have operators placed at the end of a line instead of
> the beginning of the following line?
Sure.
Thanks for the review,
Eze
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-06-20 3:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-13 15:10 [PATCH 0/2] RK3288 VP8 decoding support Ezequiel Garcia
2019-06-13 15:10 ` [PATCH 1/2] media: uapi: Add VP8 stateless decoder API Ezequiel Garcia
2019-06-17 13:23 ` Boris Brezillon
2019-06-20 3:46 ` Ezequiel Garcia
2019-06-13 15:10 ` [PATCH 2/2] media: hantro: Add support for VP8 decoding on rk3288 Ezequiel Garcia
2019-06-17 13:40 ` Boris Brezillon
2019-06-20 3:47 ` Ezequiel Garcia
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).