Re: i.MX6Q + ov5640 CSI-2 Framerate (on 5.4-rc3)
Hi Adam, Fabio, On Fri, 2019-10-18 at 16:00 -0300, Fabio Estevam wrote: > Hi Adam, > > Adding Steve and Philipp in case they can help. > > On Tue, Oct 15, 2019 at 1:52 AM Adam Ford wrote: > > I have an i.MX6Q with an ov5640 connected to the mipi-csi2 interface. > > > > I am routing ov5640 -> ipu1_csi0_mux -> ip1_csi0 -> ip1_csi0 capture. > > > > I am trying to go through the steps to attempt to get 60fps at > > 640x480, but the best I can appear to acheive is 30fps. > > > > > > v4l2-ctl --all > > > > Streaming Parameters Video Capture: > > Capabilities : timeperframe > > Frames per second: 30.000 (30/1) > > Read buffers : 0 > > > > I have tried setting both the ov5640 and the ipu1_csi0 to 1/60 without > > success. > > > > Can someone tell me if it's even possible on this platform? When I > > stream the video to the HDMI monitor, I am only using 3% of the ARM, > > so I don't think it's processor limited. That should be possible. The sensor and dual-lane CSI-2 link can support 1920x1080p30 or 1280x720p60, only the driver currently limits 60 fps capture to 640x480. The format has to be propagated through the media control graph from the sensor to the capture device. Since the CSI-2 receiver and multiplexers are not frame rate aware, the CSI has to be told the correct frame interval at its input. What does your configuration look like? I'd expect something like: media-ctl --set-v4l2 "'ov5640 ?-00??':0[fmt:UYVY8_1X16/640x480@1/60]" media-ctl --set-v4l2 "'imx6-mipi-csi2':1[fmt:UYVY8_1X16/640x480]" media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY8_1X16/640x480]" media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY8_1X16/640x480@1/60]" media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/60]" What is the output of "media-ctl --print-topology"? regards Philipp
Re: [PATCH] media: uapi: h264: clarify num_ref_idx_l[01]_(default_)active fields
On Wed, 2019-10-16 at 15:37 +0200, Paul Kocialkowski wrote: [...] > > > The bottomline is that we have use cases for each of the two set of fields > > > independently, so I feel like this is reason enough to avoid mixing them > > > together. > > > > What do you mean by mixing together? Hardware parsing the slices > > always uses num_ref_idx_l[01]_default_active_minus1 from the PPS. > > Hardware not parsing the slices always sets override to 1 and uses > > num_ref_idx_l[01]_active_minus1 from the slice header struct. > > To summarize, what I don't understand is why it's worth re-purposing > the slice header's num_ref_idx_l[01]_active_minus1 to contain > num_ref_idx_l[01]_default_active_minus1 when the flag is not set in the > initial > bitstream instead of exposing the flag. > > There's hardware (like cedrus) which takes both fields and the flag directly > in-registers, so it's really not a simplification here. And even in cases > where > the hardware only takes one field, I believe that the downside of re-purposing > the field of the control is much greater than the benefit of the supposed > simplification. > > I know this sounds quite futile, but I thought there was an implicit agreement > that controls must stick as close as possible to the bitstream. This is an > occurence where we are diverging for no particularly strong reason. FWIW, I agree with Paul on this. That drivers for codecs which do not parse the slice headers always completely ignore the num_ref_idx_l[01]_default_active_minus1 fields, but instead expect the num_ref_idx_l[01]_active_minus1 field to be repurposed to contain the default values if the corresponding fields do not exist in the slice header (that is, when the num_ref_idx_active_override_flag is not set), confused me at first [1]. This seems to follow what libva does [2], and it does simplify drivers a tiny bit, but I'd still prefer to explicitly have the num_ref_idx_active_override_flag contained in the API, and to have the num_ref_idx_l[01]_active_minus1 fields only be used for num_ref_idx_l[01]_active_minus1, and not have them sometimes contain the values of another field. [1] https://patchwork.linuxtv.org/patch/58580/ [2] https://github.com/intel/libva/blob/95eb8cf469367b532b391042fa0e77ca513ac94e/va/va.h#L3138 > Expecting that userspace does this pre-processing of fields feels quite > counter-intuitive and confusing for people wishing to use the API, too. > One would certainly naively expect that the fields in the controls carry the > same meaning as in the bitstream when they have the same name. I certainly naively did. regards Philipp
Re: [PATCH] media: hantro: relax s_fmt(out) busy error
Hi Tomasz,
On Tue, 2019-10-08 at 23:05 +0900, Tomasz Figa wrote:
> Hi Philipp,
>
> On Tue, Oct 8, 2019 at 9:38 PM Philipp Zabel wrote:
> >
> > Setting the output format resets the capture queue, so we return -EBUSY
> > while the capture queue has buffers allocated. If capture dimensions
> > and pixel format don't change though, we can allow setting the output
> > format without reallocating the capture queue.
> >
> > Signed-off-by: Philipp Zabel
> > ---
> > This applies on top of https://patchwork.linuxtv.org/patch/59337/
> > ("media: hantro: Fix s_fmt for dynamic resolution changes").
> > ---
> > drivers/staging/media/hantro/hantro_v4l2.c | 14 +++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/media/hantro/hantro_v4l2.c
> > b/drivers/staging/media/hantro/hantro_v4l2.c
> > index 586d243cc3cc..05c3edce27a9 100644
> > --- a/drivers/staging/media/hantro/hantro_v4l2.c
> > +++ b/drivers/staging/media/hantro/hantro_v4l2.c
> > @@ -368,7 +368,7 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv,
> > struct v4l2_format *f)
> > struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > struct hantro_ctx *ctx = fh_to_ctx(priv);
> > struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
> > - const struct hantro_fmt *formats;
> > + const struct hantro_fmt *raw_vpu_fmt, *formats;
> > unsigned int num_fmts;
> > int ret;
> >
> > @@ -394,8 +394,16 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv,
> > struct v4l2_format *f)
> > */
> > peer_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
> >
> > V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> > - if (vb2_is_busy(peer_vq))
> > - return -EBUSY;
> > + if (vb2_is_busy(peer_vq)) {
> > + formats = hantro_get_formats(ctx, &num_fmts);
> > + raw_vpu_fmt = hantro_get_default_fmt(formats,
> > num_fmts,
> > +false);
> > +
> > + if (ctx->dst_fmt.width != pix_mp->width ||
> > + ctx->dst_fmt.height != pix_mp->height ||
> > + ctx->dst_fmt.pixelformat != raw_vpu_fmt->fourcc)
>
> First of all, thanks for the patch! I'd like to ask you to clarify a few
> things:
> 1) What's the use case for S_FMT(OUTPUT) without changing neither
> resolution nor pixelformat?
Userspace that currently does not follow the stateless codec spec
correctly, and allocates capture buffers before setting the output
format because libva:
https://github.com/bootlin/libva-v4l2-request/pull/26
It would be better to lazily allocate the capture buffers when the
context is spun up there, it just seemed strange that S_FMT(OUTPUT)
would error even with identical parameters.
> 2) Is it correct to compare dst_fmt with pix_fmt? My understanding is
> that width/height of the OUTPUT queue is the coded size of the stream
> (a stream parameter), while width/height of the CAPTURE queue is the
> frame buffer size, which can be different from the stream coded size.
> Perhaps we should compare with ctx->src_fmt instead?
A call to hantro_reset_raw_fmt() will set dst_fmt width/height to
src_fmt width/height later in this function, so this should make no
difference.
regards
Philipp
[PATCH] media: hantro: relax s_fmt(out) busy error
Setting the output format resets the capture queue, so we return -EBUSY
while the capture queue has buffers allocated. If capture dimensions
and pixel format don't change though, we can allow setting the output
format without reallocating the capture queue.
Signed-off-by: Philipp Zabel
---
This applies on top of https://patchwork.linuxtv.org/patch/59337/
("media: hantro: Fix s_fmt for dynamic resolution changes").
---
drivers/staging/media/hantro/hantro_v4l2.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro_v4l2.c
b/drivers/staging/media/hantro/hantro_v4l2.c
index 586d243cc3cc..05c3edce27a9 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -368,7 +368,7 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv,
struct v4l2_format *f)
struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
struct hantro_ctx *ctx = fh_to_ctx(priv);
struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
- const struct hantro_fmt *formats;
+ const struct hantro_fmt *raw_vpu_fmt, *formats;
unsigned int num_fmts;
int ret;
@@ -394,8 +394,16 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv,
struct v4l2_format *f)
*/
peer_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- if (vb2_is_busy(peer_vq))
- return -EBUSY;
+ if (vb2_is_busy(peer_vq)) {
+ formats = hantro_get_formats(ctx, &num_fmts);
+ raw_vpu_fmt = hantro_get_default_fmt(formats, num_fmts,
+false);
+
+ if (ctx->dst_fmt.width != pix_mp->width ||
+ ctx->dst_fmt.height != pix_mp->height ||
+ ctx->dst_fmt.pixelformat != raw_vpu_fmt->fourcc)
+ return -EBUSY;
+ }
} else {
/*
* The encoder doesn't admit a format change if
--
2.20.1
Re: [PATCH 2/2] media: uapi: h264: clarify pic_order_cnt_bit_size field
Hi Hans,
On Thu, 2019-09-26 at 15:23 +0200, Hans Verkuil wrote:
> On 9/5/19 3:12 PM, Philipp Zabel wrote:
> > Since pic_order_cnt_bit_size is not a syntax element itself, explicitly
> > state that it is the total size in bits of the pic_order_cnt_lsb,
> > delta_pic_order_cnt_bottom, delta_pic_order_cnt[0], and
> > delta_pic_order_cnt[1] syntax elements contained in the slice.
> >
> > Signed-off-by: Philipp Zabel
> > ---
> > Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > index c281bc7ed1b3..08b49b2bbfa8 100644
> > --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > @@ -1799,7 +1799,8 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> >- Size in bits of the dec_ref_pic_marking() syntax element.
> > * - __u32
> >- ``pic_order_cnt_bit_size``
> > - -
> > + - Size in bits of the pic_order_cnt_lsb, delta_pic_order_cnt_bottom,
> > +delta_pic_order_cnt[0], and delta_pic_order_cnt[1] syntax elements.
>
> It's a little bit ambiguous: this field contains the size in bits of all these
> fields combined, right? Not the size in bits of each field separately.
Yes.
> I.e. if these fields are all 8 bits, then pic_order_cnt_bit_size is 4*8 and
> not just 8.
The size of pic_order_cnt_lsb is determined by another field's value
(log2_max_pic_order_cnt_lsb_minus4), and the other three are
exponential-Golomb coded, so each can be of different length (including
zero).
> I think this should be rephrased.
Ok, how about:
"Combined size in bits of the picture order count related syntax
elements: pic_order_cnt_lsb, delta_pic_order_cnt_bottom,
delta_pic_order_cnt[0], and delta_pic_order_cnt[1]."
Actually, there's either pic_order_cnt_lsb + (optionally)
delta_pic_order_cnt_bottom present, or delta_pic_order_cnt[0] +
(optionally) delta_pic_order_cnt[1], never all four. Describing that in
the uapi docs seemed overly verbose, though. The relevant part in the
slice_header() syntax spec looks like this:
if (pic_order_cnt_type == 0) {
pic_order_cnt_lsb
if (bottom_field_pic_order_in_frame_present_flag && !field_pic_flag)
delta_pic_order_cnt_bottom
}
if (pic_order_cnt_type == 1 && !delta_pic_order_always_zero_flag) {
delta_pic_order_cnt[0]
if (bottom_field_pic_order_in_frame_present_flag && !field_pic_flag)
delta_pic_order_cnt[1]
}
pic_order_cnt_bit_size is supposed to be the size in bits of this whole
block. pic_order_cnt_type and log2_max_pic_order_cnt_lsb_minus4 are from
the SPS header, bottom_field_pic_order_in_frame_present_flag is from the
PPS header, and field_pic_flag is contained earlier in the slice header.
regards
Philipp
Re: [PATCH v4 1/3] media: v4l2-subdev: add g_csi_active_lanes() op
Hi Jacopo,
On Wed, 2019-09-25 at 16:51 +0200, Jacopo Mondi wrote:
> Hello,
>sorry for having missed this
Thank you and Laurent for completing the list of interested parties, I
had completely forgotten about the frame descriptors.
> On Wed, Sep 25, 2019 at 04:41:13PM +0300, Laurent Pinchart wrote:
> > Hi Philipp,
> >
> > (CC'ing Sakari, Jacopo, Kieran and Niklas)
> >
> > Thank you for the patch.
> >
> > On Tue, Sep 24, 2019 at 01:49:53PM +0200, Philipp Zabel wrote:
> > > Add a subdevice video operation that allows to query the number
> > > of data lanes a MIPI CSI-2 TX is actively transmitting on.
> > >
> > > Suggested-by: Hans Verkuil
> > > Signed-off-by: Philipp Zabel
> > > ---
> > > New in v4.
> > > ---
> > > include/media/v4l2-subdev.h | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > > index 71f1f2f0da53..bb71eedc38f6 100644
> > > --- a/include/media/v4l2-subdev.h
> > > +++ b/include/media/v4l2-subdev.h
> > > @@ -411,6 +411,8 @@ struct v4l2_mbus_frame_desc {
> > > * @s_rx_buffer: set a host allocated memory buffer for the subdev. The
> > > subdev
> > > * can adjust @size to a lower value and must not write more data
> > > to the
> > > * buffer starting at @data than the original value of @size.
> > > + *
> > > + * @g_csi_active_lanes: Get number of currently active MIPI CSI-2 data
> > > lanes.
> > > */
> > > struct v4l2_subdev_video_ops {
> > > int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32
> > > config);
> > > @@ -441,6 +443,7 @@ struct v4l2_subdev_video_ops {
> > >const struct v4l2_mbus_config *cfg);
> > > int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
> > > unsigned int *size);
> > > + int (*g_csi_active_lanes)(struct v4l2_subdev *sd, u32 *lanes);
> >
> > This shouldn't be a video operation but a pad operation, as a subdev
> > could have multiple CSI-2 pads.
> >
> > Furthermore, you need to define the semantics of this operation more
> > precisely. When can it be called, when is the information valid ? Can
> > the subdev change the number of lanes it supports at runtime ? If so,
> > how are race conditions avoided ? All this needs to be documented.
> >
> > Finally, the number of lanes is far from being the only information
> > about a physical bus that could be interesting for a remote subdev. I
> > would much prefer a more generic operation to retrieve bus
> > information/configuration, with a data structure that we will be able to
> > extend later.
> >
>
> For the record we tried to get those information from the frame
> descriptor (the number of used data lanes and the clock
> continous/non-continous information to be precise)
>
> This is the RFC series I sent
> https://patchwork.kernel.org/project/linux-media/list/?series=92501
>
> Which depends on Sakari's patches:
> https://patchwork.kernel.org/patch/10875871/
> https://patchwork.kernel.org/patch/10875873/
>
> The latest two were part of a much bigger series that tried to add
> support for multiplexed streams. Honestly, it now seems to be that
> part could be breakout without involving streams, and re-use that
> portion to negotiate the csi-2 bus configuration. I might be wrong
> though, and the two parts could not be separate easily (from a very
> quick look, after months, it does not seem so).
>
> In the past I spoke against this solution as I would have preferred
> leaving frame_desc alone and introduce a bus configuration operation.
> I tried a few times and I ended up implementing g_mbus_format but on
> pads and not video. Right now with Sakari's and Laurent's ack I would
> say re-using frame_desc might actually work and get use a feature
> which is needed by many (cc also Dave, as he had a similar issue with
> TC358743 iirc)
That looks like it should work just as well. If there is agreement to
add the number of data lanes, (non-)continuous clock flag, and possibly
the chosen link frequency v4l2_mbus_frame_desc, I'll happily dig up
these patches and switch to .get_frame_desc().
regards
Philipp
Re: [PATCH v4 1/3] media: v4l2-subdev: add g_csi_active_lanes() op
Hi Laurent,
On Wed, 2019-09-25 at 16:41 +0300, Laurent Pinchart wrote:
> Hi Philipp,
>
> (CC'ing Sakari, Jacopo, Kieran and Niklas)
>
> Thank you for the patch.
>
> On Tue, Sep 24, 2019 at 01:49:53PM +0200, Philipp Zabel wrote:
> > Add a subdevice video operation that allows to query the number
> > of data lanes a MIPI CSI-2 TX is actively transmitting on.
> >
> > Suggested-by: Hans Verkuil
> > Signed-off-by: Philipp Zabel
> > ---
> > New in v4.
> > ---
> > include/media/v4l2-subdev.h | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index 71f1f2f0da53..bb71eedc38f6 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -411,6 +411,8 @@ struct v4l2_mbus_frame_desc {
> > * @s_rx_buffer: set a host allocated memory buffer for the subdev. The
> > subdev
> > * can adjust @size to a lower value and must not write more data to the
> > * buffer starting at @data than the original value of @size.
> > + *
> > + * @g_csi_active_lanes: Get number of currently active MIPI CSI-2 data
> > lanes.
> > */
> > struct v4l2_subdev_video_ops {
> > int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32
> > config);
> > @@ -441,6 +443,7 @@ struct v4l2_subdev_video_ops {
> > const struct v4l2_mbus_config *cfg);
> > int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
> >unsigned int *size);
> > + int (*g_csi_active_lanes)(struct v4l2_subdev *sd, u32 *lanes);
>
> This shouldn't be a video operation but a pad operation, as a subdev
> could have multiple CSI-2 pads.
Right this should be pad specific.
> Furthermore, you need to define the semantics of this operation more
> precisely. When can it be called,
The downstream subdevice connected to this pad is expected to call this
in its s_stream(enable=1) op, right before enabling the MIPI CSI-2 RX,
and then calling s_stream(enable=1) on the same upstream subdevice.
The returned value is a decision by the upstream subdevice driver based
on external factors such as available link-frequencies and mbus frame
format, so it can change whenever those are changed, but not by itself.
> when is the information valid ?
It is valid until the next time the pad's mbus frame format or link
frequency are changed. Since the caller
> Can the subdev change the number of lanes it supports at runtime ?
At least for MIPI CSI-2, no. Are there any buses that can dynamically
change bus width while active?
> If so, how are race conditions avoided ? All this needs to be documented.
I think no, so the only possible race conditions would be with
reconfiguration, which should already be avoided by requiring this to be
called from s_stream(),as the media pipeline is already started and
all configuration locked in place at this point.
> Finally, the number of lanes is far from being the only information
> about a physical bus that could be interesting for a remote subdev. I
> would much prefer a more generic operation to retrieve bus
> information/configuration, with a data structure that we will be able to
> extend later.
This is specifically about configuration chosen by the transmitter, not
physical bus properties, which can be specified in the device tree. The
chosen link frequency (if more than one is specified in DT) could be one
of those values.
regards
Philipp
[PATCH v4 1/3] media: v4l2-subdev: add g_csi_active_lanes() op
Add a subdevice video operation that allows to query the number
of data lanes a MIPI CSI-2 TX is actively transmitting on.
Suggested-by: Hans Verkuil
Signed-off-by: Philipp Zabel
---
New in v4.
---
include/media/v4l2-subdev.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 71f1f2f0da53..bb71eedc38f6 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -411,6 +411,8 @@ struct v4l2_mbus_frame_desc {
* @s_rx_buffer: set a host allocated memory buffer for the subdev. The subdev
* can adjust @size to a lower value and must not write more data to the
* buffer starting at @data than the original value of @size.
+ *
+ * @g_csi_active_lanes: Get number of currently active MIPI CSI-2 data lanes.
*/
struct v4l2_subdev_video_ops {
int (*s_routing)(struct v4l2_subdev *sd, u32 input, u32 output, u32
config);
@@ -441,6 +443,7 @@ struct v4l2_subdev_video_ops {
const struct v4l2_mbus_config *cfg);
int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
unsigned int *size);
+ int (*g_csi_active_lanes)(struct v4l2_subdev *sd, u32 *lanes);
};
/**
--
2.20.1
[PATCH v4 0/3] Add g_csi_active_lanes for dynamic active lanes
Some MIPI CSI-2 transmitters, such as TC358743 can dynamically change the number of active data lanes depending on the bandwidth needs for the selected video format. This patchset adds a subdevice video operation g_csi_active_lanes() to let the MIPI CSI-2 receiver query the number of active lanes and change its configuration accordingly. Changes since v3 [1]: - Add g_csi_active_lanes() subdevice video operation, implement it in tc358743, and use it in imx6-mipi-csi2. [1] https://patchwork.linuxtv.org/patch/53331/ regards Philipp Philipp Zabel (3): media: v4l2-subdev: add g_csi_active_lanes() op media: tc358743: fix connected/active CSI-2 lane reporting media: imx: ask source subdevice for number of active data lanes drivers/media/i2c/tc358743.c | 44 -- drivers/staging/media/imx/imx6-mipi-csi2.c | 8 ++-- include/media/v4l2-subdev.h| 3 ++ 3 files changed, 39 insertions(+), 16 deletions(-) -- 2.20.1
[PATCH v4 2/3] media: tc358743: fix connected/active CSI-2 lane reporting
g_mbus_config was supposed to indicate all supported lane numbers, not
only the number of those currently in active use. Since the TC358743
can dynamically reduce the number of active lanes if the required
bandwidth allows for it, report all lane numbers up to the connected
number of lanes as supported in pdata mode.
In device tree mode, do not report lane count and clock mode at all, as
the receiver driver can determine these from the device tree.
To allow communicating the number of currently active lanes, use the
newly added g_csi_active_lanes() video op.
Signed-off-by: Philipp Zabel
---
Changes since v3:
- Use g_csi_active_lanes instead of storing the number of active lanes
in mbus_config flags.
---
drivers/media/i2c/tc358743.c | 44 ++--
1 file changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index dbbab75f135e..c2db6419a4b4 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1607,23 +1607,31 @@ static int tc358743_g_mbus_config(struct v4l2_subdev
*sd,
{
struct tc358743_state *state = to_state(sd);
+ if (state->csi_lanes_in_use > state->bus.num_data_lanes)
+ return -EINVAL;
+
cfg->type = V4L2_MBUS_CSI2_DPHY;
+ cfg->flags = 0;
- /* Support for non-continuous CSI-2 clock is missing in the driver */
- cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+ /* In DT mode, there is no need to report the number of data lanes */
+ if (sd->dev->of_node)
+ return 0;
- switch (state->csi_lanes_in_use) {
- case 1:
- cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
- break;
- case 2:
- cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
- break;
+ /* Support for non-continuous CSI-2 clock is missing in pdata mode */
+ cfg->flags |= V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+
+ switch (state->bus.num_data_lanes) {
+ case 4:
+ cfg->flags |= V4L2_MBUS_CSI2_LANES;
+ /* fallthrough */
case 3:
cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
- break;
- case 4:
- cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
+ /* fallthrough */
+ case 2:
+ cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
+ /* fallthrough */
+ case 1:
+ cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
break;
default:
return -EINVAL;
@@ -1632,6 +1640,16 @@ static int tc358743_g_mbus_config(struct v4l2_subdev *sd,
return 0;
}
+static int tc358743_g_csi_active_lanes(struct v4l2_subdev *sd,
+ u32 *lanes)
+{
+ struct tc358743_state *state = to_state(sd);
+
+ *lanes = state->csi_lanes_in_use;
+
+ return 0;
+}
+
static int tc358743_s_stream(struct v4l2_subdev *sd, int enable)
{
enable_stream(sd, enable);
@@ -1838,6 +1856,7 @@ static const struct v4l2_subdev_video_ops
tc358743_video_ops = {
.query_dv_timings = tc358743_query_dv_timings,
.g_mbus_config = tc358743_g_mbus_config,
.s_stream = tc358743_s_stream,
+ .g_csi_active_lanes = tc358743_g_csi_active_lanes,
};
static const struct v4l2_subdev_pad_ops tc358743_pad_ops = {
@@ -2052,6 +2071,7 @@ static int tc358743_probe(struct i2c_client *client)
if (pdata) {
state->pdata = *pdata;
state->bus.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+ state->bus.num_data_lanes = 4;
} else {
err = tc358743_probe_of(state);
if (err == -ENODEV)
--
2.20.1
[PATCH v4 3/3] media: imx: ask source subdevice for number of active data lanes
Use the newly added g_csi_active_lanes() video op to determine the
number of active data lanes used by the transmitter. If this subdev
call is not supported or does not return the number of active lanes,
default to using all connected data lanes as before.
Signed-off-by: Philipp Zabel
---
Changes since v3:
- Use g_csi_active_lanes instead of g_mbus_config.
---
drivers/staging/media/imx/imx6-mipi-csi2.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c
b/drivers/staging/media/imx/imx6-mipi-csi2.c
index bfa4b254c4e4..aa4bf2f89695 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -131,10 +131,8 @@ static void csi2_enable(struct csi2_dev *csi2, bool enable)
}
}
-static void csi2_set_lanes(struct csi2_dev *csi2)
+static void csi2_set_lanes(struct csi2_dev *csi2, int lanes)
{
- int lanes = csi2->bus.num_data_lanes;
-
writel(lanes - 1, csi2->base + CSI2_N_LANES);
}
@@ -295,6 +293,7 @@ static void csi2ipu_gasket_init(struct csi2_dev *csi2)
static int csi2_start(struct csi2_dev *csi2)
{
+ u32 lanes = 0;
int ret;
ret = clk_prepare_enable(csi2->pix_clk);
@@ -310,7 +309,8 @@ static int csi2_start(struct csi2_dev *csi2)
goto err_disable_clk;
/* Step 4 */
- csi2_set_lanes(csi2);
+ v4l2_subdev_call(csi2->src_sd, video, g_csi_active_lanes, &lanes);
+ csi2_set_lanes(csi2, lanes ?: csi2->bus.num_data_lanes);
csi2_enable(csi2, true);
/* Step 5 */
--
2.20.1
Re: coda9 jpeg support?
Hi Tim, On Tue, 2019-09-17 at 12:00 -0700, Tim Harvey wrote: [...] > I have pulled your branch and boot-tested it. I see the 2 new video > devices but noticed that the JPEG decoder shows up as an element for > video4linux2 the JPEG encoder doesn't show up (gstreamer v1.14.5) - > any idea why that would be? Yes, v4l2jpegenc was added in 1.16 regards Philipp
Re: coda9 jpeg support?
Hi Tim, On Fri, 2019-09-13 at 09:00 -0700, Tim Harvey wrote: > Greetings, > > What would need to be done to support JPEG enc/dec for coda9? here is a WIP that still needs some cleanup for upstreaming: https://git.pengutronix.de/cgit/pza/linux/log/?h=coda/jpeg Basically I'd like to avoid adding yet another JPEG header parser to the kernel, as we already have at least three: drivers/media/platform/rcar_jpu.c drivers/media/platform/mtk-jpeg/mtk_jpeg_parse.c drivers/media/platform/s5p-jpeg/jpeg-core.c I want to allow probing without the BIT processor firmware for blobless JPEG-only support, and I'd like the JPEG codec to be able to run concurrently with the BIT processor codec. I'm working on this this week. regards Philipp
[PATCH 2/3] media: imx: fix and simplify pixel format enumeration
Merge yuv_formats and rgb_formats into a single array. Always loop over
all entries, skipping those that are incompatible with the requested
enumeration. This simplifies the code, lets us get rid of the manual
counting of array entries, and stops accidentally ignoring some non-mbus
RGB formats.
Before:
$ v4l2-ctl -d /dev/video14 --list-formats-out
ioctl: VIDIOC_ENUM_FMT
Type: Video Output
[0]: 'UYVY' (UYVY 4:2:2)
[1]: 'YUYV' (YUYV 4:2:2)
[2]: 'YU12' (Planar YUV 4:2:0)
[3]: 'YV12' (Planar YVU 4:2:0)
[4]: '422P' (Planar YUV 4:2:2)
[5]: 'NV12' (Y/CbCr 4:2:0)
[6]: 'NV16' (Y/CbCr 4:2:2)
[7]: 'RGBP' (16-bit RGB 5-6-5)
[8]: 'RGB3' (24-bit RGB 8-8-8)
[9]: 'BX24' (32-bit XRGB 8-8-8-8)
After:
$ v4l2-ctl -d /dev/video14 --list-formats-out
ioctl: VIDIOC_ENUM_FMT
Type: Video Output
[0]: 'UYVY' (UYVY 4:2:2)
[1]: 'YUYV' (YUYV 4:2:2)
[2]: 'YU12' (Planar YUV 4:2:0)
[3]: 'YV12' (Planar YVU 4:2:0)
[4]: '422P' (Planar YUV 4:2:2)
[5]: 'NV12' (Y/CbCr 4:2:0)
[6]: 'NV16' (Y/CbCr 4:2:2)
[7]: 'RGBP' (16-bit RGB 5-6-5)
[8]: 'RGB3' (24-bit RGB 8-8-8)
[9]: 'BGR3' (24-bit BGR 8-8-8)
[10]: 'BX24' (32-bit XRGB 8-8-8-8)
[11]: 'XR24' (32-bit BGRX 8-8-8-8)
[12]: 'RX24' (32-bit XBGR 8-8-8-8)
[13]: 'XB24' (32-bit RGBX 8-8-8-8)
Signed-off-by: Philipp Zabel
---
drivers/staging/media/imx/imx-media-utils.c | 170 ++--
1 file changed, 45 insertions(+), 125 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-utils.c
b/drivers/staging/media/imx/imx-media-utils.c
index 0788a1874557..d61a8f4533dc 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -9,12 +9,9 @@
/*
* List of supported pixel formats for the subdevs.
- *
- * In all of these tables, the non-mbus formats (with no
- * mbus codes) must all fall at the end of the table.
*/
-
-static const struct imx_media_pixfmt yuv_formats[] = {
+static const struct imx_media_pixfmt pixel_formats[] = {
+ /*** YUV formats start here ***/
{
.fourcc = V4L2_PIX_FMT_UYVY,
.codes = {
@@ -31,12 +28,7 @@ static const struct imx_media_pixfmt yuv_formats[] = {
},
.cs = IPUV3_COLORSPACE_YUV,
.bpp= 16,
- },
- /***
-* non-mbus YUV formats start here. NOTE! when adding non-mbus
-* formats, NUM_NON_MBUS_YUV_FORMATS must be updated below.
-***/
- {
+ }, {
.fourcc = V4L2_PIX_FMT_YUV420,
.cs = IPUV3_COLORSPACE_YUV,
.bpp= 12,
@@ -62,13 +54,7 @@ static const struct imx_media_pixfmt yuv_formats[] = {
.bpp= 16,
.planar = true,
},
-};
-
-#define NUM_NON_MBUS_YUV_FORMATS 5
-#define NUM_YUV_FORMATS ARRAY_SIZE(yuv_formats)
-#define NUM_MBUS_YUV_FORMATS (NUM_YUV_FORMATS - NUM_NON_MBUS_YUV_FORMATS)
-
-static const struct imx_media_pixfmt rgb_formats[] = {
+ /*** RGB formats start here ***/
{
.fourcc = V4L2_PIX_FMT_RGB565,
.codes = {MEDIA_BUS_FMT_RGB565_2X8_LE},
@@ -83,12 +69,28 @@ static const struct imx_media_pixfmt rgb_formats[] = {
},
.cs = IPUV3_COLORSPACE_RGB,
.bpp= 24,
+ }, {
+ .fourcc = V4L2_PIX_FMT_BGR24,
+ .cs = IPUV3_COLORSPACE_RGB,
+ .bpp= 24,
}, {
.fourcc = V4L2_PIX_FMT_XRGB32,
.codes = {MEDIA_BUS_FMT_ARGB_1X32},
.cs = IPUV3_COLORSPACE_RGB,
.bpp= 32,
.ipufmt = true,
+ }, {
+ .fourcc = V4L2_PIX_FMT_XBGR32,
+ .cs = IPUV3_COLORSPACE_RGB,
+ .bpp= 32,
+ }, {
+ .fourcc = V4L2_PIX_FMT_BGRX32,
+ .cs = IPUV3_COLORSPACE_RGB,
+ .bpp= 32,
+ }, {
+ .fourcc = V4L2_PIX_FMT_RGBX32,
+ .cs = IPUV3_COLORSPACE_RGB,
+ .bpp= 32,
},
/*** raw bayer and grayscale formats start here ***/
{
@@ -175,33 +177,8 @@ static const struct imx_media_pixfmt rgb_formats[] = {
.bpp= 16,
.bayer = true,
},
- /***
-* non-mbus RGB formats start here. NOTE! when adding non-mbus
-* formats, NUM_NON_MBUS_RGB_FORMATS must be updated below.
-***/
- {
- .fourcc = V4L2_PIX_FMT_BGR24,
- .cs = IPUV3_COLORSPACE_RGB,
-
[PATCH 3/3] media: imx: fix media bus format enumeration
Iterate over all media bus formats, not just over the first format in
each imx_media_pixfmt entry.
Before:
$ v4l2-ctl -d $(media-ctl -e ipu1_csi0) --list-subdev-mbus-codes 0
ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0)
0x2006: MEDIA_BUS_FMT_UYVY8_2X8
0x2008: MEDIA_BUS_FMT_YUYV8_2X8
0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE
0x100a: MEDIA_BUS_FMT_RGB888_1X24
0x100d: MEDIA_BUS_FMT_ARGB_1X32
0x3001: MEDIA_BUS_FMT_SBGGR8_1X8
0x3013: MEDIA_BUS_FMT_SGBRG8_1X8
0x3002: MEDIA_BUS_FMT_SGRBG8_1X8
0x3014: MEDIA_BUS_FMT_SRGGB8_1X8
0x3007: MEDIA_BUS_FMT_SBGGR10_1X10
0x300e: MEDIA_BUS_FMT_SGBRG10_1X10
0x300a: MEDIA_BUS_FMT_SGRBG10_1X10
0x300f: MEDIA_BUS_FMT_SRGGB10_1X10
0x2001: MEDIA_BUS_FMT_Y8_1X8
0x200a: MEDIA_BUS_FMT_Y10_1X10
After:
$ v4l2-ctl -d $(media-ctl -e ipu1_csi0) --list-subdev-mbus-codes 0
ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0)
0x2006: MEDIA_BUS_FMT_UYVY8_2X8
0x200f: MEDIA_BUS_FMT_UYVY8_1X16
0x2008: MEDIA_BUS_FMT_YUYV8_2X8
0x2011: MEDIA_BUS_FMT_YUYV8_1X16
0x1008: MEDIA_BUS_FMT_RGB565_2X8_LE
0x100a: MEDIA_BUS_FMT_RGB888_1X24
0x100c: MEDIA_BUS_FMT_RGB888_2X12_LE
0x100d: MEDIA_BUS_FMT_ARGB_1X32
0x3001: MEDIA_BUS_FMT_SBGGR8_1X8
0x3013: MEDIA_BUS_FMT_SGBRG8_1X8
0x3002: MEDIA_BUS_FMT_SGRBG8_1X8
0x3014: MEDIA_BUS_FMT_SRGGB8_1X8
0x3007: MEDIA_BUS_FMT_SBGGR10_1X10
0x3008: MEDIA_BUS_FMT_SBGGR12_1X12
0x3019: MEDIA_BUS_FMT_SBGGR14_1X14
0x301d: MEDIA_BUS_FMT_SBGGR16_1X16
0x300e: MEDIA_BUS_FMT_SGBRG10_1X10
0x3010: MEDIA_BUS_FMT_SGBRG12_1X12
0x301a: MEDIA_BUS_FMT_SGBRG14_1X14
0x301e: MEDIA_BUS_FMT_SGBRG16_1X16
0x300a: MEDIA_BUS_FMT_SGRBG10_1X10
0x3011: MEDIA_BUS_FMT_SGRBG12_1X12
0x301b: MEDIA_BUS_FMT_SGRBG14_1X14
0x301f: MEDIA_BUS_FMT_SGRBG16_1X16
0x300f: MEDIA_BUS_FMT_SRGGB10_1X10
0x3012: MEDIA_BUS_FMT_SRGGB12_1X12
0x301c: MEDIA_BUS_FMT_SRGGB14_1X14
0x3020: MEDIA_BUS_FMT_SRGGB16_1X16
0x2001: MEDIA_BUS_FMT_Y8_1X8
0x200a: MEDIA_BUS_FMT_Y10_1X10
0x2013: MEDIA_BUS_FMT_Y12_1X12
Signed-off-by: Philipp Zabel
---
drivers/staging/media/imx/imx-media-utils.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-utils.c
b/drivers/staging/media/imx/imx-media-utils.c
index d61a8f4533dc..5f8604db4dd4 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -254,7 +254,7 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
bool allow_bayer)
{
const struct imx_media_pixfmt *fmt;
- unsigned int i, j = 0;
+ unsigned int i, j, k = 0;
for (i = 0; i < ARRAY_SIZE(pixel_formats); i++) {
fmt = &pixel_formats[i];
@@ -264,18 +264,29 @@ static int enum_format(u32 *fourcc, u32 *code, u32 index,
(!allow_bayer && fmt->bayer))
continue;
- if (index == j)
+ if (fourcc && index == k)
break;
- j++;
+ if (!code) {
+ k++;
+ continue;
+ }
+
+ for (j = 0; j < ARRAY_SIZE(fmt->codes) && fmt->codes[j]; j++) {
+ if (index == k)
+ goto out;
+
+ k++;
+ }
}
if (i == ARRAY_SIZE(pixel_formats))
return -EINVAL;
+out:
if (fourcc)
*fourcc = fmt->fourcc;
if (code)
- *code = fmt->codes[0];
+ *code = fmt->codes[j];
return 0;
}
--
2.20.1
[PATCH 1/3] media: imx: enable V4L2_PIX_FMT_XBGR32, _BGRX32, and _RGBX32
Now that proper V4L2 pixel formats exist for all 32-bit RGB
permutations, drop support for the poorly defined legacy format
V4L2_PIX_FMT_BGR32.
Signed-off-by: Philipp Zabel
---
drivers/staging/media/imx/imx-media-utils.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/imx/imx-media-utils.c
b/drivers/staging/media/imx/imx-media-utils.c
index 4cc6a7462ae2..0788a1874557 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -184,7 +184,15 @@ static const struct imx_media_pixfmt rgb_formats[] = {
.cs = IPUV3_COLORSPACE_RGB,
.bpp= 24,
}, {
- .fourcc = V4L2_PIX_FMT_BGR32,
+ .fourcc = V4L2_PIX_FMT_XBGR32,
+ .cs = IPUV3_COLORSPACE_RGB,
+ .bpp= 32,
+ }, {
+ .fourcc = V4L2_PIX_FMT_BGRX32,
+ .cs = IPUV3_COLORSPACE_RGB,
+ .bpp= 32,
+ }, {
+ .fourcc = V4L2_PIX_FMT_RGBX32,
.cs = IPUV3_COLORSPACE_RGB,
.bpp= 32,
},
--
2.20.1
Re: [RFC] V4L2 & Metadata: switch to /dev/v4l-metaX instead of /dev/videoX
On Thu, 2019-09-12 at 16:49 +0200, Hans Verkuil wrote: > On 9/12/19 4:21 PM, Mauro Carvalho Chehab wrote: > > Em Thu, 12 Sep 2019 14:16:11 +0100 > > Kieran Bingham escreveu: > > > > > Hi Hans, > > > > > > On 12/09/2019 08:48, Hans Verkuil wrote: > > > > Hi all, > > > > > > > > I am increasingly unhappy about the choice of /dev/videoX for metadata > > > > devices. > > > > > > > > It is confusing for end-users (especially w.r.t. the common uvc driver) > > > > and > > > > if we want to change this, then we need to do it soon. > > > > Kernel has (about) nothing to do with how the userspace devnodes are > > named, as the actual name is given by udev. > > To my knowledge the standard udev rules do not rename anything for media > devices, so in reality it IS the kernel that decides this. > > But this is why I suggested to put it under a kernel config option. > > > > > Anyway, from Kernel standpoint, it sounds too late to change the name > > of the devices from "videoX" to something else, as a change like that > > may break existing apps. > > > > It could make sense to have something like that at udev rules. > > But wouldn't that break existing apps as well? > > A bigger problem is that it isn't easy to detect the difference between > a regular video device and a metadata device: you'd have to call QUERYCAP > to determine that. v4l_id does call VIDIOC_QUERYCAP [1] to fill the ID_V4L_CAPABILITIES colon separated list. [1] https://github.com/systemd/systemd/blob/master/src/udev/v4l_id/v4l_id.c#L66 regards Philipp
[PATCH v2] media: uapi: h264: clarify num_ref_idx_l[01]_(default_)active fields
Since the uapi does not contain the num_ref_idx_active_override_flag, drivers for decoders that do not parse slices themselves don't know how to choose between the num_ref_idx_l[01]_default_active and the num_ref_idx_l[01]_active override fields. Instead, userspace must set the override fields to the default values if the slice does not have the num_ref_idx_active_override flag set. The drivers will then always enable the override internally and ignore the default fields completely. Clarify this requirement in the API documentation. Signed-off-by: Philipp Zabel --- Changes since v1: - Drop sentences about num_ref_idx_l[011](_default)_active_minus1 fields only being used by decoders that do/do not parse slice headers. --- Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst index bc5dd8e76567..6b3bb71655a3 100644 --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst @@ -1820,10 +1820,12 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - - * - __u8 - ``num_ref_idx_l0_active_minus1`` - - + - If num_ref_idx_active_override_flag is not set, this field must be +set to the value of num_ref_idx_l0_default_active_minus1. * - __u8 - ``num_ref_idx_l1_active_minus1`` - - + - If num_ref_idx_active_override_flag is not set, this field must be +set to the value of num_ref_idx_l1_default_active_minus1. * - __u32 - ``slice_group_change_cycle`` - -- 2.20.1
[PATCH] media: uapi: h264: clarify V4L2_PIX_FMT_H264_SLICE format
Document that the slice headers must be included for the benefit of decoders that parse them (partially) in hardware, and that the start code is optional. Add a link to the ITU-T Rec. H.264 specification section that describes the slice format. Signed-off-by: Philipp Zabel --- Documentation/media/uapi/v4l/pixfmt-compressed.rst | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Documentation/media/uapi/v4l/pixfmt-compressed.rst b/Documentation/media/uapi/v4l/pixfmt-compressed.rst index 292fdc116c77..55d8d690f22f 100644 --- a/Documentation/media/uapi/v4l/pixfmt-compressed.rst +++ b/Documentation/media/uapi/v4l/pixfmt-compressed.rst @@ -61,10 +61,10 @@ Compressed Formats - ``V4L2_PIX_FMT_H264_SLICE`` - 'S264' - - H264 parsed slice data, without the start code and as - extracted from the H264 bitstream. This format is adapted for - stateless video decoders that implement an H264 pipeline - (using the :ref:`mem2mem` and :ref:`media-request-api`). + - H264 parsed slice data, including slice headers, either with or + without the start code, as extracted from the H264 bitstream. + This format is adapted for stateless video decoders that implement an + H264 pipeline (using the :ref:`mem2mem` and :ref:`media-request-api`). This pixelformat has two modifiers that must be set at least once through the ``V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE`` and ``V4L2_CID_MPEG_VIDEO_H264_START_CODE`` controls. @@ -80,6 +80,10 @@ Compressed Formats appropriate number of macroblocks to decode a full corresponding frame to the matching capture buffer. + The syntax for this format is documented in :ref:`h264`, section + 7.3.2.8 "Slice layer without partitioning RBSP syntax" and the following + sections. + .. note:: This format is not yet part of the public kernel API and it -- 2.20.1
Re: [PATCH] media: uapi: h264: clarify num_ref_idx_l[01]_(default_)active fields
On Mon, 2019-09-09 at 14:43 +0200, Hans Verkuil wrote: > On 9/9/19 2:27 PM, Philipp Zabel wrote: > > On Mon, 2019-09-09 at 14:09 +0200, Hans Verkuil wrote: > > > On 9/5/19 1:42 PM, Philipp Zabel wrote: [...] > > > > @@ -1820,10 +1820,14 @@ enum > > > > v4l2_mpeg_video_h264_hierarchical_coding_type - > > > >- > > > > * - __u8 > > > >- ``num_ref_idx_l0_active_minus1`` > > > > - - > > > > + - This field is used by decoders that do not parse slices > > > > themselves. > > > > +If num_ref_idx_active_override_flag is not set, this field > > > > must be > > > > +set to the value of num_ref_idx_l0_default_active_minus1. > > > > > > I don't think you can know if the decoder parses the slices. > > > > That is correct. > > > > > Wouldn't it be better to just delete the 'This field is only used by > > > decoders > > > that parse slices themselves.' sentence? Drivers for HW that handle this > > > can > > > just ignore these fields. > > > > If this has no place in the API documentation, or if it just might > > confuse the user in a different way, it's indeed better drop these. > > Is there another place where this could be clarified instead, perhaps > > the kerneldoc comments? > > A code comment in those drivers where the HW parses this would make > sense since that explains why that driver ignores these fields. > > But I would not mention this at all in the userspace API. > > The 'If num_ref_idx_active_override_flag is not set, this field must be > set to the value of num_ref_idx_l0_default_active_minus1.' addition is > of course fine. Ok. I'll revise the patch accordingly. > I'm a bit confused, though: you say some HW can parse this, but how? > It's part of the slice_header, so it ends up in struct > v4l2_ctrl_h264_slice_params, > right? So how can the HW parse this without also providing the > num_ref_idx_active_override_flag value? The complete slice queued via VIDIOC_QBUF still contains all these fields (and more). Presumably that's where the Hantro G1 reads the num_ref_idx_active_override_flag from, as well as other fields that it doesn't use from v4l2_ctrl_h264_slice_params. G1 can not parse the slice header completely by itself though, it needs to be told the total size of the (pic_order_cnt_lsb / delta_pic_order_cnt_bottom / delta_pic_order_cnt0 / delta_pic_order_cnt1) syntax elements and the size of the dec_ref_pic_marking() syntax element, as well as the values of pic_parameter_set_id, frame_num, and idr_pic_id, and some flags. The num_ref_idx_l[01]_active_minus1 fields are among those parsed from the vb2 buffer directly. That's why the hantro-vpu driver ignores the header_bit_size field, whereas cedrus has to use it to tell the hardware how to skip the header. Cedrus completely ignores the num_ref_idx_l[01]_default_active_minus1 fields, and always uses the values passed via num_ref_idx_l[01]_active_minus1, see cedrus_h264.c +343: /* * FIXME: the kernel headers are allowing the default value to * be passed, but the libva doesn't give us that. */ reg |= (slice->num_ref_idx_l0_active_minus1 & 0x1f) << 10; reg |= (slice->num_ref_idx_l1_active_minus1 & 0x1f) << 5; cedrus_write(dev, VE_H264_PPS, reg); and +388: reg |= VE_H264_SHS2_NUM_REF_IDX_ACTIVE_OVRD; reg |= (slice->num_ref_idx_l0_active_minus1 & 0x1f) << 24; reg |= (slice->num_ref_idx_l1_active_minus1 & 0x1f) << 16; cedrus_write(dev, VE_H264_SHS2, reg); ^ that's the override flag being set unconditionally, to select the values from SHS2 over those from PPS. regards Philipp
Re: [PATCH] media: uapi: h264: clarify num_ref_idx_l[01]_(default_)active fields
On Mon, 2019-09-09 at 14:09 +0200, Hans Verkuil wrote: > On 9/5/19 1:42 PM, Philipp Zabel wrote: > > To explain why num_ref_idx_active_override_flag is not part of the API, > > describe how the num_ref_idx_l[01]_active_minus1 fields and the > > num_ref_idx_l[01]_default_active_minus1 fields are used, depending on > > whether the decoder parses slice headers itself or not. > > > > Signed-off-by: Philipp Zabel > > --- > > Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 12 > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > index bc5dd8e76567..b9834625a939 100644 > > --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > @@ -1630,10 +1630,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - > >- > > * - __u8 > >- ``num_ref_idx_l0_default_active_minus1`` > > - - > > + - This field is only used by decoders that parse slices themselves. > > How do you know that the decoder does this? We don't, so usespace has to provide it unconditionally. This was meant as an explanation why. As you can see from the "media: hantro: h264: per-slice num_ref_idx_l[01]_active override" thread I found this a bit unintuitive. > > * - __u8 > >- ``num_ref_idx_l1_default_active_minus1`` > > - - > > + - This field is only used by decoders that parse slices themselves. > > * - __u8 > >- ``weighted_bipred_idc`` > >- > > @@ -1820,10 +1820,14 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - > >- > > * - __u8 > >- ``num_ref_idx_l0_active_minus1`` > > - - > > + - This field is used by decoders that do not parse slices themselves. > > +If num_ref_idx_active_override_flag is not set, this field must be > > +set to the value of num_ref_idx_l0_default_active_minus1. > > I don't think you can know if the decoder parses the slices. That is correct. > Wouldn't it be better to just delete the 'This field is only used by decoders > that parse slices themselves.' sentence? Drivers for HW that handle this can > just ignore these fields. If this has no place in the API documentation, or if it just might confuse the user in a different way, it's indeed better drop these. Is there another place where this could be clarified instead, perhaps the kerneldoc comments? Basically I was confused about why both the default and the override have to be provided, and how the kernel driver determines which one to choose, since the override flag is not part of the kernel API. As it turns out, it doesn't have to choose; depending on whether the hardware parses slices, the driver can pick either one or the other, and always use that. regards Philipp
Re: [PATCH 3/3] media: hantro: h264: Fix the frame_num wraparound case
Hi Boris,
On Mon, 2019-09-09 at 09:28 +0200, Boris Brezillon wrote:
> Step '8.2.4.1 Decoding process for picture numbers' was missing in the
> reflist creation logic, leading to invalid P reflists when a
> ->frame_num wraparound happens.
>
> Fixes: a9471e25629b ("media: hantro: Add core bits to support H264 decoding")
> Reported-by: Francois Buergisser
> Signed-off-by: Boris Brezillon
Thank you, excellent timing. I've just seen this corruption for the
first time last Friday. All three patches
Reviewed-by: Philipp Zabel
Tested-by: Philipp Zabel
(on i.MX8MQ)
regards
Philipp
Re: [PATCH 1/2] media: uapi: h264: Add num_ref_idx_active_override_flag
On Mon, 2019-09-09 at 16:09 +0900, Tomasz Figa wrote:
> On Thu, Sep 5, 2019 at 11:17 PM Nicolas Dufresne
> wrote:
> >
> > Le jeudi 05 septembre 2019 à 12:39 +0200, Philipp Zabel a écrit :
> > > On Thu, 2019-09-05 at 19:31 +0900, Tomasz Figa wrote:
> > > > On Thu, Sep 5, 2019 at 7:15 PM Philipp Zabel
> > > > wrote:
> > > > > This flag tells the kernel whether the slice header contained the
> > > > > num_ref_idx_l[01]_active_minus1 syntax elements, or whether the
> > > > > num_ref_idx_l[01]_default_active_minus1 from PPS should be used
> > > > > instead.
> > > > >
> > > > > Signed-off-by: Philipp Zabel
> > > > > ---
> > > > > Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 3 +++
> > > > > include/media/h264-ctrls.h | 1 +
> > > > > 2 files changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > > > > b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > > > > index bc5dd8e76567..451a5b0f2a35 100644
> > > > > --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > > > > +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
> > > > > @@ -1860,6 +1860,9 @@ enum
> > > > > v4l2_mpeg_video_h264_hierarchical_coding_type -
> > > > > * - ``V4L2_H264_SLICE_FLAG_SP_FOR_SWITCH``
> > > > >- 0x0008
> > > > >-
> > > > > +* - ``V4L2_H264_SLICE_FLAG_NUM_REF_IDX_ACTIVE_OVERRIDE
> > > > > + - 0x0010
> > > > > + - Corresponds to the num_ref_idx_active_override_flag syntax
> > > > > element.
> > > > >
> > > >
> > > > As far as I remember, the idea was for the userspace to always put the
> > > > right num_ref_idx in the slice_params and the drivers always use that.
> > > > Was there any problem with that?
> > >
> > > I don't think so, at least for currently known hardware.
> > >
> > > In that case we should drop the unused
> > > num_ref_idx_l[01]_default_active_minus1 fields from struct
> > > v4l2_ctrl_h264_pps and document that userspace should fill
> > > the defaults into v4l2_ctrl_h264_slice_params themselves if
> > > num_ref_idx_active_override_flag wasn't set.
> >
> > It might have been added in a previous effort to allow reconstructing
> > the bitstream from the structures.
>
> Wouldn't one still be able to reconstruct a valid (but not exact)
> stream without that flag, given the assumption above?
If the Hantro G1, as appears to be the case, parses the slice header and
decides itself whether to use the override from the slice or the default
that was written to a register, it needs the
num_ref_idx_l[01]_default_active_minus1 field to fill the register, but
doesn't need either the num_ref_idx_l[01]_active_minus1 override nor the
flag.
A decoder that doesn't parse the slice header can always be told to use
the override (thus no need to have the flag in the uapi), if userspace
fills the default into the override fields as a fallback. Such a decoder
does need the num_ref_idx_l[01]_active_minus1 override, but doesn't need
the num_ref_idx_l[01]_default_active_minus1 field nor the flag.
That is my current understanding of the intention behind this interface,
I hope this is accurate.
I've tried to make the docs reflect this in ("media: uapi: h264: clarify
num_ref_idx_l[01]_(default_)active fields") [1].
[1]
https://lore.kernel.org/linux-media/20190905114210.9232-1-p.za...@pengutronix.de/T/#u
regards
Philipp
[PATCH 2/2] media: uapi: h264: clarify pic_order_cnt_bit_size field
Since pic_order_cnt_bit_size is not a syntax element itself, explicitly state that it is the total size in bits of the pic_order_cnt_lsb, delta_pic_order_cnt_bottom, delta_pic_order_cnt[0], and delta_pic_order_cnt[1] syntax elements contained in the slice. Signed-off-by: Philipp Zabel --- Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst index c281bc7ed1b3..08b49b2bbfa8 100644 --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst @@ -1799,7 +1799,8 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - - Size in bits of the dec_ref_pic_marking() syntax element. * - __u32 - ``pic_order_cnt_bit_size`` - - + - Size in bits of the pic_order_cnt_lsb, delta_pic_order_cnt_bottom, +delta_pic_order_cnt[0], and delta_pic_order_cnt[1] syntax elements. * - __u8 - ``cabac_init_idc`` - -- 2.20.1
[PATCH 1/2] media: uapi: h264: clarify dec_ref_pic_marking_bit_size fields
Since dec_ref_pic_marking_bit_size is not a syntax element itself, explicitly state that this is the size in bits of the dec_ref_pic_marking() syntax element contained in the slice. Signed-off-by: Philipp Zabel --- Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst index b9834625a939..c281bc7ed1b3 100644 --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst @@ -1796,7 +1796,7 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - - * - __u32 - ``dec_ref_pic_marking_bit_size`` - - + - Size in bits of the dec_ref_pic_marking() syntax element. * - __u32 - ``pic_order_cnt_bit_size`` - -- 2.20.1
[PATCH] media: uapi: h264: clarify num_ref_idx_l[01]_(default_)active fields
To explain why num_ref_idx_active_override_flag is not part of the API, describe how the num_ref_idx_l[01]_active_minus1 fields and the num_ref_idx_l[01]_default_active_minus1 fields are used, depending on whether the decoder parses slice headers itself or not. Signed-off-by: Philipp Zabel --- Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst index bc5dd8e76567..b9834625a939 100644 --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst @@ -1630,10 +1630,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - - * - __u8 - ``num_ref_idx_l0_default_active_minus1`` - - + - This field is only used by decoders that parse slices themselves. * - __u8 - ``num_ref_idx_l1_default_active_minus1`` - - + - This field is only used by decoders that parse slices themselves. * - __u8 - ``weighted_bipred_idc`` - @@ -1820,10 +1820,14 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - - * - __u8 - ``num_ref_idx_l0_active_minus1`` - - + - This field is used by decoders that do not parse slices themselves. +If num_ref_idx_active_override_flag is not set, this field must be +set to the value of num_ref_idx_l0_default_active_minus1. * - __u8 - ``num_ref_idx_l1_active_minus1`` - - + - This field is used by decoders that do not parse slices themselves. +If num_ref_idx_active_override_flag is not set, this field must be +set to the value of num_ref_idx_l1_default_active_minus1. * - __u32 - ``slice_group_change_cycle`` - -- 2.20.1
Re: [PATCH 2/2] media: hantro: h264: per-slice num_ref_idx_l[01]_active override
On Thu, 2019-09-05 at 19:34 +0900, Tomasz Figa wrote:
> On Thu, Sep 5, 2019 at 7:15 PM Philipp Zabel wrote:
> >
> > If the slice header had the num_ref_idx_active_override flag set, we
> > should use the num_ref_idx_l[01]_active_minus1 fields instead of the
> > defaults from the PPS.
> >
> > Signed-off-by: Philipp Zabel
> > ---
> > drivers/staging/media/hantro/hantro_g1_h264_dec.c | 13 +++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> > b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> > index 7ab534936843..544cf92ab62c 100644
> > --- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> > +++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
> > @@ -28,6 +28,8 @@ static void set_params(struct hantro_ctx *ctx)
> > const struct v4l2_ctrl_h264_pps *pps = ctrls->pps;
> > struct vb2_v4l2_buffer *src_buf = hantro_get_src_buf(ctx);
> > struct hantro_dev *vpu = ctx->dev;
> > + unsigned char refidx0_active;
> > + unsigned char refidx1_active;
> > u32 reg;
> >
> > /* Decoder control register 0. */
> > @@ -101,9 +103,16 @@ static void set_params(struct hantro_ctx *ctx)
> > vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL5);
> >
> > /* Decoder control register 6. */
> > + if (sps->flags & V4L2_H264_SLICE_FLAG_NUM_REF_IDX_ACTIVE_OVERRIDE) {
> > + refidx0_active = slices[0].num_ref_idx_l0_active_minus1 + 1;
> > + refidx1_active = slices[0].num_ref_idx_l1_active_minus1 + 1;
> > + } else {
> > + refidx0_active = pps->num_ref_idx_l0_default_active_minus1
> > + 1;
> > + refidx1_active = pps->num_ref_idx_l1_default_active_minus1
> > + 1;
> > + }
> > reg = G1_REG_DEC_CTRL6_PPS_ID(slices[0].pic_parameter_set_id) |
> > -
> > G1_REG_DEC_CTRL6_REFIDX0_ACTIVE(pps->num_ref_idx_l0_default_active_minus1 +
> > 1) |
> > -
> > G1_REG_DEC_CTRL6_REFIDX1_ACTIVE(pps->num_ref_idx_l1_default_active_minus1 +
> > 1) |
> > + G1_REG_DEC_CTRL6_REFIDX0_ACTIVE(refidx0_active) |
> > + G1_REG_DEC_CTRL6_REFIDX1_ACTIVE(refidx1_active) |
> > G1_REG_DEC_CTRL6_POC_LENGTH(slices[0].pic_order_cnt_bit_size);
> > vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL6);
>
> My understanding was that Hantro G1 parses the slices itself and
> handles the override flag internally.
Oh, in that case this patch is incorrect and the first one is indeed
unnecessary.
regards
Philipp
Re: [PATCH 1/2] media: uapi: h264: Add num_ref_idx_active_override_flag
On Thu, 2019-09-05 at 19:31 +0900, Tomasz Figa wrote: > On Thu, Sep 5, 2019 at 7:15 PM Philipp Zabel wrote: > > > > This flag tells the kernel whether the slice header contained the > > num_ref_idx_l[01]_active_minus1 syntax elements, or whether the > > num_ref_idx_l[01]_default_active_minus1 from PPS should be used > > instead. > > > > Signed-off-by: Philipp Zabel > > --- > > Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 3 +++ > > include/media/h264-ctrls.h | 1 + > > 2 files changed, 4 insertions(+) > > > > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > index bc5dd8e76567..451a5b0f2a35 100644 > > --- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > +++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst > > @@ -1860,6 +1860,9 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type - > > * - ``V4L2_H264_SLICE_FLAG_SP_FOR_SWITCH`` > >- 0x0008 > >- > > +* - ``V4L2_H264_SLICE_FLAG_NUM_REF_IDX_ACTIVE_OVERRIDE > > + - 0x0010 > > + - Corresponds to the num_ref_idx_active_override_flag syntax element. > > > > As far as I remember, the idea was for the userspace to always put the > right num_ref_idx in the slice_params and the drivers always use that. > Was there any problem with that? I don't think so, at least for currently known hardware. In that case we should drop the unused num_ref_idx_l[01]_default_active_minus1 fields from struct v4l2_ctrl_h264_pps and document that userspace should fill the defaults into v4l2_ctrl_h264_slice_params themselves if num_ref_idx_active_override_flag wasn't set. regards Philipp
[PATCH] media: hantro: streamline open, reuse error path
This adds a label to the error path to avoid calling
v4l2_m2m_ctx_init() and kfree(ctx) in two places each.
Signed-off-by: Philipp Zabel
---
drivers/staging/media/hantro/hantro_drv.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro_drv.c
b/drivers/staging/media/hantro/hantro_drv.c
index d8b6816b643b..4bf2a5d938e9 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -413,20 +413,18 @@ static int hantro_open(struct file *filp)
if (func->id == MEDIA_ENT_F_PROC_VIDEO_ENCODER) {
allowed_codecs = vpu->variant->codec & HANTRO_ENCODERS;
ctx->buf_finish = hantro_enc_buf_finish;
- ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(vpu->m2m_dev, ctx,
- queue_init);
} else if (func->id == MEDIA_ENT_F_PROC_VIDEO_DECODER) {
allowed_codecs = vpu->variant->codec & HANTRO_DECODERS;
ctx->buf_finish = hantro_dec_buf_finish;
- ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(vpu->m2m_dev, ctx,
- queue_init);
} else {
- ctx->fh.m2m_ctx = ERR_PTR(-ENODEV);
+ ret = -ENODEV;
+ goto err_ctx_free;
}
+
+ ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(vpu->m2m_dev, ctx, queue_init);
if (IS_ERR(ctx->fh.m2m_ctx)) {
ret = PTR_ERR(ctx->fh.m2m_ctx);
- kfree(ctx);
- return ret;
+ goto err_ctx_free;
}
v4l2_fh_init(&ctx->fh, vdev);
@@ -447,6 +445,7 @@ static int hantro_open(struct file *filp)
err_fh_free:
v4l2_fh_del(&ctx->fh);
v4l2_fh_exit(&ctx->fh);
+err_ctx_free:
kfree(ctx);
return ret;
}
--
2.20.1
[PATCH 2/2] media: hantro: h264: per-slice num_ref_idx_l[01]_active override
If the slice header had the num_ref_idx_active_override flag set, we
should use the num_ref_idx_l[01]_active_minus1 fields instead of the
defaults from the PPS.
Signed-off-by: Philipp Zabel
---
drivers/staging/media/hantro/hantro_g1_h264_dec.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
index 7ab534936843..544cf92ab62c 100644
--- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
@@ -28,6 +28,8 @@ static void set_params(struct hantro_ctx *ctx)
const struct v4l2_ctrl_h264_pps *pps = ctrls->pps;
struct vb2_v4l2_buffer *src_buf = hantro_get_src_buf(ctx);
struct hantro_dev *vpu = ctx->dev;
+ unsigned char refidx0_active;
+ unsigned char refidx1_active;
u32 reg;
/* Decoder control register 0. */
@@ -101,9 +103,16 @@ static void set_params(struct hantro_ctx *ctx)
vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL5);
/* Decoder control register 6. */
+ if (sps->flags & V4L2_H264_SLICE_FLAG_NUM_REF_IDX_ACTIVE_OVERRIDE) {
+ refidx0_active = slices[0].num_ref_idx_l0_active_minus1 + 1;
+ refidx1_active = slices[0].num_ref_idx_l1_active_minus1 + 1;
+ } else {
+ refidx0_active = pps->num_ref_idx_l0_default_active_minus1 + 1;
+ refidx1_active = pps->num_ref_idx_l1_default_active_minus1 + 1;
+ }
reg = G1_REG_DEC_CTRL6_PPS_ID(slices[0].pic_parameter_set_id) |
-
G1_REG_DEC_CTRL6_REFIDX0_ACTIVE(pps->num_ref_idx_l0_default_active_minus1 + 1) |
-
G1_REG_DEC_CTRL6_REFIDX1_ACTIVE(pps->num_ref_idx_l1_default_active_minus1 + 1) |
+ G1_REG_DEC_CTRL6_REFIDX0_ACTIVE(refidx0_active) |
+ G1_REG_DEC_CTRL6_REFIDX1_ACTIVE(refidx1_active) |
G1_REG_DEC_CTRL6_POC_LENGTH(slices[0].pic_order_cnt_bit_size);
vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL6);
--
2.20.1
[PATCH 1/2] media: uapi: h264: Add num_ref_idx_active_override_flag
This flag tells the kernel whether the slice header contained the
num_ref_idx_l[01]_active_minus1 syntax elements, or whether the
num_ref_idx_l[01]_default_active_minus1 from PPS should be used
instead.
Signed-off-by: Philipp Zabel
---
Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 3 +++
include/media/h264-ctrls.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
index bc5dd8e76567..451a5b0f2a35 100644
--- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
+++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
@@ -1860,6 +1860,9 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
* - ``V4L2_H264_SLICE_FLAG_SP_FOR_SWITCH``
- 0x0008
-
+* - ``V4L2_H264_SLICE_FLAG_NUM_REF_IDX_ACTIVE_OVERRIDE
+ - 0x0010
+ - Corresponds to the num_ref_idx_active_override_flag syntax element.
``Prediction Weight Table``
diff --git a/include/media/h264-ctrls.h b/include/media/h264-ctrls.h
index e877bf1d537c..dab519aea9bf 100644
--- a/include/media/h264-ctrls.h
+++ b/include/media/h264-ctrls.h
@@ -133,6 +133,7 @@ struct v4l2_h264_pred_weight_table {
#define V4L2_H264_SLICE_FLAG_BOTTOM_FIELD 0x02
#define V4L2_H264_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED0x04
#define V4L2_H264_SLICE_FLAG_SP_FOR_SWITCH 0x08
+#define V4L2_H264_SLICE_FLAG_NUM_REF_IDX_ACTIVE_OVERRIDE 0x10
struct v4l2_ctrl_h264_slice_params {
/* Size in bytes, including header */
--
2.20.1
Re: [PATCH 2/4] media: hantro: Simplify buffer helpers
On Tue, 2019-09-03 at 15:17 -0300, Ezequiel Garcia wrote: > Modify hantro_get_ref() and hantro_h264_get_ref_buf() helpers > to return the buffer DMA address, this makes the code simpler > and at the same time will allow easier enablement of the > post-processing feature. > > Signed-off-by: Ezequiel Garcia Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH 1/4] media: hantro: Simplify macroblock macros
On Tue, 2019-09-03 at 15:17 -0300, Ezequiel Garcia wrote: > It seems all codecs are using a 16x16 size macroblock, > and so it's possible to have just one set of macroblock macros. > > Signed-off-by: Ezequiel Garcia Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH 3/4] media: hantro: Add helper for the H264 motion vectors allocation
On Tue, 2019-09-03 at 15:17 -0300, Ezequiel Garcia wrote:
> Introduce a helper to allow easier enablement of the post-processing
> feature. No functional changes intended.
>
> Signed-off-by: Ezequiel Garcia
> ---
> drivers/staging/media/hantro/hantro.h | 6 ++
> drivers/staging/media/hantro/hantro_v4l2.c | 4 ++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/hantro/hantro.h
> b/drivers/staging/media/hantro/hantro.h
> index deb90ae37859..e8872f24e351 100644
> --- a/drivers/staging/media/hantro/hantro.h
> +++ b/drivers/staging/media/hantro/hantro.h
> @@ -381,4 +381,10 @@ hantro_get_dst_buf(struct hantro_ctx *ctx)
> return v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> }
>
> +static inline unsigned int
> +hantro_h264_buffer_extra_size(unsigned int width, unsigned int height)
> +{
> + return 128 * MB_WIDTH(width) * MB_HEIGHT(height);
> +}
This doesn't seem to be used or modified by patch 4 at all?
> +
> #endif /* HANTRO_H_ */
> diff --git a/drivers/staging/media/hantro/hantro_v4l2.c
> b/drivers/staging/media/hantro/hantro_v4l2.c
> index d48b548842cf..59adecba0e85 100644
> --- a/drivers/staging/media/hantro/hantro_v4l2.c
> +++ b/drivers/staging/media/hantro/hantro_v4l2.c
> @@ -246,8 +246,8 @@ static int vidioc_try_fmt(struct file *file, void *priv,
> struct v4l2_format *f,
>*/
> if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_H264_SLICE)
> pix_mp->plane_fmt[0].sizeimage +=
> - 128 * DIV_ROUND_UP(pix_mp->width, 16) *
> - DIV_ROUND_UP(pix_mp->height, 16);
> + hantro_h264_buffer_extra_size(pix_mp->width,
> + pix_mp->height);
> } else if (!pix_mp->plane_fmt[0].sizeimage) {
> /*
>* For coded formats the application can specify
regards
Philipp
Re: [PATCH v3 5/7] media: v4l2-core: Add new helper for area controls
On Fri, 2019-08-23 at 15:05 +0200, Ricardo Ribalda Delgado wrote:
> On Fri, Aug 23, 2019 at 2:56 PM Philipp Zabel wrote:
> >
> > On Fri, 2019-08-23 at 14:37 +0200, Ricardo Ribalda Delgado wrote:
> > > Adding a V4L2_CID_UNIT_CELL_SIZE control requires a lot of boilerplate,
> > > try to minimize it by adding a new helper.
> > >
> > > Suggested-by: Philipp Zabel
> > > Signed-off-by: Ricardo Ribalda Delgado
> > > ---
> > > drivers/media/v4l2-core/v4l2-ctrls.c | 25 -
> > > include/media/v4l2-ctrls.h | 16
> > > 2 files changed, 40 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> > > b/drivers/media/v4l2-core/v4l2-ctrls.c
> > > index b3bf458df7f7..33e48f0aec1a 100644
> > > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > > @@ -2660,7 +2660,6 @@ struct v4l2_ctrl
> > > *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl,
> > > }
> > > EXPORT_SYMBOL(v4l2_ctrl_new_std_menu_items);
> > >
> > > -/* Helper function for standard integer menu controls */
> >
> > Why move this ...
> >
> > > struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct v4l2_ctrl_handler *hdl,
> > > const struct v4l2_ctrl_ops *ops,
> > > u32 id, u8 _max, u8 _def, const s64 *qmenu_int)
> > > @@ -2684,6 +2683,30 @@ struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct
> > > v4l2_ctrl_handler *hdl,
> > > }
> > > EXPORT_SYMBOL(v4l2_ctrl_new_int_menu);
> > >
> > > +static void area_init(const struct v4l2_ctrl *ctrl, u32 idx,
> > > + union v4l2_ctrl_ptr ptr)
> > > +{
> > > + memcpy(ptr.p_area, ctrl->priv, sizeof(*ptr.p_area));
> > > +}
> > > +
> > > +static const struct v4l2_ctrl_type_ops area_ops = {
> > > + .init = area_init,
> > > +};
> > > +
> > > +struct v4l2_ctrl *v4l2_ctrl_new_area(struct v4l2_ctrl_handler *hdl,
> > > + const struct v4l2_ctrl_ops *ops,
> > > + u32 id, const struct v4l2_area *area)
> > > +{
> > > + static struct v4l2_ctrl_config ctrl = {
> > > + .id = V4L2_CID_UNIT_CELL_SIZE,
> > > + .type_ops = &area_ops,
> > > + };
> > > +
> > > + return v4l2_ctrl_new_custom(hdl, &ctrl, (void *)area);
> > > +}
> > > +EXPORT_SYMBOL(v4l2_ctrl_new_area);
> > > +
> > > +/* Helper function for standard integer menu controls */
> >
> > ... here?
>
> Because I screwed up :). Let me fix that sorry.
>
> I will push all your changes to:
>
> https://github.com/ribalda/linux/tree/unit-size-v4
>
> plus any other comment and then I will wait 2-3 days for resend
Awesome, thanks! Feel free to add
Reviewed-by: Philipp Zabel
regards
Philipp
Re: [PATCH v3 4/7] media: imx214: Add new control with V4L2_CID_UNIT_CELL_SIZE
On Fri, 2019-08-23 at 14:37 +0200, Ricardo Ribalda Delgado wrote: > According to the product brief, the unit cell size is 1120 nanometers^2. > > https://www.sony-semicon.co.jp/products_en/IS/sensor1/img/products/ProductBrief_IMX214_20150428.pdf > > Signed-off-by: Ricardo Ribalda Delgado If the v4l2_ctrl_new_area helper is accepted, I'd reorder this afterwards and squash it together with patch 7. regards Philipp
Re: [PATCH v3 3/7] Documentation: media: Document V4L2_CTRL_TYPE_AREA
On Fri, 2019-08-23 at 14:37 +0200, Ricardo Ribalda Delgado wrote: > A struct v4l2_area containing the width and the height of a rectangular > area. > > Signed-off-by: Ricardo Ribalda Delgado > Suggested-by: Hans Verkuil > --- > Documentation/media/uapi/v4l/vidioc-queryctrl.rst | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/media/uapi/v4l/vidioc-queryctrl.rst > b/Documentation/media/uapi/v4l/vidioc-queryctrl.rst > index a3d56ffbf4cc..c09d06ef2b08 100644 > --- a/Documentation/media/uapi/v4l/vidioc-queryctrl.rst > +++ b/Documentation/media/uapi/v4l/vidioc-queryctrl.rst > @@ -443,6 +443,12 @@ See also the examples in :ref:`control`. >- n/a >- A struct :c:type:`v4l2_ctrl_mpeg2_quantization`, containing MPEG-2 > quantization matrices for stateless video decoders. > +* - ``V4L2_CTRL_TYPE_AREA`` > + - n/a > + - n/a > + - n/a > + - A struct :c:type:`v4l2_area`, containing the width and the height > +of a rectangular area. Maybe explicitly mention that units depend on the use case? regards Philipp
Re: [PATCH v3 2/7] Documentation: media: Describe V4L2_CID_UNIT_CELL_SIZE
On Fri, 2019-08-23 at 14:37 +0200, Ricardo Ribalda Delgado wrote: > New control to pass to userspace the width/height of a pixel. Which is > needed for calibration and lens selection. > > Signed-off-by: Ricardo Ribalda Delgado > --- > Documentation/media/uapi/v4l/ext-ctrls-image-source.rst | 8 > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/media/uapi/v4l/ext-ctrls-image-source.rst > b/Documentation/media/uapi/v4l/ext-ctrls-image-source.rst > index 2c3ab5796d76..6e728accf67b 100644 > --- a/Documentation/media/uapi/v4l/ext-ctrls-image-source.rst > +++ b/Documentation/media/uapi/v4l/ext-ctrls-image-source.rst > @@ -55,3 +55,11 @@ Image Source Control IDs > > ``V4L2_CID_TEST_PATTERN_GREENB (integer)`` > Test pattern green (next to blue) colour component. > + > +``V4L2_CID_UNIT_CELL_SIZE (struct)`` > +This control returns the unit cell size in nanometres. The struct > provides > +the width and the height in separated fields to take into consideration > +asymmetric pixels and/or hardware binning. > +The unit cell consists of the whole area of the pixel, sensitive and > +non-sensitive. > +This control is required for automatic calibration sensors/cameras. Can we have a link from here to the struct v4l2_area documentation? regards Philipp
Re: [PATCH v3 5/7] media: v4l2-core: Add new helper for area controls
On Fri, 2019-08-23 at 14:37 +0200, Ricardo Ribalda Delgado wrote:
> Adding a V4L2_CID_UNIT_CELL_SIZE control requires a lot of boilerplate,
> try to minimize it by adding a new helper.
>
> Suggested-by: Philipp Zabel
> Signed-off-by: Ricardo Ribalda Delgado
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 25 -
> include/media/v4l2-ctrls.h | 16
> 2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index b3bf458df7f7..33e48f0aec1a 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -2660,7 +2660,6 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct
> v4l2_ctrl_handler *hdl,
> }
> EXPORT_SYMBOL(v4l2_ctrl_new_std_menu_items);
>
> -/* Helper function for standard integer menu controls */
Why move this ...
> struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct v4l2_ctrl_handler *hdl,
> const struct v4l2_ctrl_ops *ops,
> u32 id, u8 _max, u8 _def, const s64 *qmenu_int)
> @@ -2684,6 +2683,30 @@ struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct
> v4l2_ctrl_handler *hdl,
> }
> EXPORT_SYMBOL(v4l2_ctrl_new_int_menu);
>
> +static void area_init(const struct v4l2_ctrl *ctrl, u32 idx,
> + union v4l2_ctrl_ptr ptr)
> +{
> + memcpy(ptr.p_area, ctrl->priv, sizeof(*ptr.p_area));
> +}
> +
> +static const struct v4l2_ctrl_type_ops area_ops = {
> + .init = area_init,
> +};
> +
> +struct v4l2_ctrl *v4l2_ctrl_new_area(struct v4l2_ctrl_handler *hdl,
> + const struct v4l2_ctrl_ops *ops,
> + u32 id, const struct v4l2_area *area)
> +{
> + static struct v4l2_ctrl_config ctrl = {
> + .id = V4L2_CID_UNIT_CELL_SIZE,
> + .type_ops = &area_ops,
> + };
> +
> + return v4l2_ctrl_new_custom(hdl, &ctrl, (void *)area);
> +}
> +EXPORT_SYMBOL(v4l2_ctrl_new_area);
> +
> +/* Helper function for standard integer menu controls */
... here?
Looks to me like this comment should stay attached to
v4l2_ctrl_new_int_menu.
regards
Philipp
Re: [PATCH v9] media: imx: add csc/scaler mem2mem device
Hi Steve, On Fri, 2019-08-16 at 16:17 -0700, Steve Longerbeam wrote: > Hi Philipp, > > If you haven't already, can you please test rotation with this version, > with both non-tiled and tiled scaling conversions. I found rotation was > broken in v8. > > Steve I've tested a few resolutions with both 90° and 270° rotations. Upscale: 32x24 320x240 640x480 768x320 768x432 720x404 720x480 1024x768 -> 768x1024 768x1280 1080x1920 1600x2560 1280x720 -> 1080x1920 Downscale: 3840x2160 -> 1080x1920 1920x1080 -> 1080x1280 768x1024 576x1024 576x1024 540x960 Note that for horizontal flips the seam hiding does not work properly, due to not being able to overshoot the input scanline backwards. This is apparent when scaling 32x24 -> 768x1280 1080x1920 with 180° or 270° rotation, for example. regards Philipp
Re: [PATCH v9] media: imx: add csc/scaler mem2mem device
On Wed, 2019-08-14 at 14:24 +0200, Philipp Zabel wrote: > Add a single imx-media mem2mem video device that uses the IPU IC PP > (image converter post processing) task for scaling and colorspace > conversion. > On i.MX6Q/DL SoCs with two IPUs currently only the first IPU is used. [...] Output of 'v4l2-compliance -d 10 -e 10 -s -P', with the ipu-v3 fixes at [1] applied. 'v4l2-compliance -f' works for most format combinations, but fails with CMA trying to allocate physically contiguous 4096x4096 32-bit RGB buffers on both queues, on my test system: [1] https://patchwork.kernel.org/series/159687/mbox --8<-- v4l2-compliance SHA: 8d411705aa29f3ad77a4d48797b7ad07bfcce471, 32 bits Compliance test for imx-media-csc-s device /dev/video10: Driver Info: Driver name : imx-media-csc-s Card type: imx-media-csc-scaler Bus info : platform:imx-media-csc-scaler Driver version : 5.3.0 Capabilities : 0x84208000 Video Memory-to-Memory Streaming Extended Pix Format Device Capabilities Device Caps : 0x04208000 Video Memory-to-Memory Streaming Extended Pix Format Media Driver Info: Driver name : imx-media Model: imx-media Serial : Bus info : Media version: 5.3.0 Hardware revision: 0x (0) Driver version : 5.3.0 Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video10 open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 4 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK (Not Supported) test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK test Composing: OK test Scaling: OK Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK test Requests: OK (Not Supported) Test input 0: Streaming ioctls: test read/write: OK (Not Supported) test blocking wait: OK Video Capture: Captured 58 buffers test MMAP (no poll): OK Video Capture: Captured 58 buffers test MMAP (select): OK Video Capture: Captured 58 buffers test MMAP (epoll): OK test USERPTR (no poll): OK (Not Supported) test USERPTR (select): OK (Not Supported) Video Capture: Captured 58 buffers test DMABUF (no poll): OK Video Capture: Captured 58 buffers test DMABUF (select): OK Total for imx-media-csc-s device /dev/video10: 53, Succeeded: 53, Failed: 0, Warnings: 0 -->8-- regards Philipp
[PATCH v9] media: imx: add csc/scaler mem2mem device
Add a single imx-media mem2mem video device that uses the IPU IC PP
(image converter post processing) task for scaling and colorspace
conversion.
On i.MX6Q/DL SoCs with two IPUs currently only the first IPU is used.
The hardware only supports writing to destination buffers up to
1024x1024 pixels in a single pass, arbitrary sizes can be achieved
by rendering multiple tiles per frame.
Signed-off-by: Philipp Zabel
[slongerb...@gmail.com: use ipu_image_convert_adjust(), fix
device_run() error handling, add missing media-device header,
unregister and remove the mem2mem device in error paths in
imx_media_probe_complete() and in imx_media_remove(), updated
for sync subdev registration]
Signed-off-by: Steve Longerbeam
---
Changes since v8 [1]:
- Change subject to be more descriptive.
- Call video_unregister_device unconditionally, it handles gracefully
handles unregistered video devices.
- Rebased onto commit 6d01b7ff5233 ("media: staging/imx: Switch to sync
registration for IPU subdevs")
- propagate colorimetry info from queue format to image converter
- replace devm_kzalloc by kzalloc and release on video_device.release
to avoid priv being freed immediately when the driver is unbound
while a video device is still opened. This allows to get rid of
imx_media_csc_scaler_device_remove() altogether.
- update SPDX license identifier
[1] https://patchwork.linuxtv.org/patch/55776/
---
drivers/staging/media/imx/Kconfig | 1 +
drivers/staging/media/imx/Makefile| 3 +-
.../staging/media/imx/imx-media-csc-scaler.c | 925 ++
drivers/staging/media/imx/imx-media-dev.c | 28 +-
.../staging/media/imx/imx-media-internal-sd.c | 4 +
drivers/staging/media/imx/imx-media.h | 12 +
6 files changed, 971 insertions(+), 2 deletions(-)
create mode 100644 drivers/staging/media/imx/imx-media-csc-scaler.c
diff --git a/drivers/staging/media/imx/Kconfig
b/drivers/staging/media/imx/Kconfig
index 4c726345dc25..56b4d7ea3022 100644
--- a/drivers/staging/media/imx/Kconfig
+++ b/drivers/staging/media/imx/Kconfig
@@ -7,6 +7,7 @@ config VIDEO_IMX_MEDIA
depends on HAS_DMA
select VIDEOBUF2_DMA_CONTIG
select V4L2_FWNODE
+ select V4L2_MEM2MEM_DEV
help
Say yes here to enable support for video4linux media controller
driver for the i.MX5/6 SOC.
diff --git a/drivers/staging/media/imx/Makefile
b/drivers/staging/media/imx/Makefile
index aa6c4b4ad37e..9bd9e873ba7c 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
imx6-media-objs := imx-media-dev.o imx-media-internal-sd.o \
- imx-ic-common.o imx-ic-prp.o imx-ic-prpencvf.o imx-media-vdic.o
+ imx-ic-common.o imx-ic-prp.o imx-ic-prpencvf.o imx-media-vdic.o \
+ imx-media-csc-scaler.o
imx-media-common-objs := imx-media-capture.o imx-media-dev-common.o \
imx-media-of.o imx-media-utils.o
diff --git a/drivers/staging/media/imx/imx-media-csc-scaler.c
b/drivers/staging/media/imx/imx-media-csc-scaler.c
new file mode 100644
index ..0cebb013831d
--- /dev/null
+++ b/drivers/staging/media/imx/imx-media-csc-scaler.c
@@ -0,0 +1,925 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * i.MX IPUv3 IC PP mem2mem CSC/Scaler driver
+ *
+ * Copyright (C) 2011 Pengutronix, Sascha Hauer
+ * Copyright (C) 2018 Pengutronix, Philipp Zabel
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "imx-media.h"
+
+#define fh_to_ctx(__fh)container_of(__fh, struct ipu_csc_scaler_ctx,
fh)
+
+enum {
+ V4L2_M2M_SRC = 0,
+ V4L2_M2M_DST = 1,
+};
+
+struct ipu_csc_scaler_priv {
+ struct imx_media_video_dev vdev;
+
+ struct v4l2_m2m_dev *m2m_dev;
+ struct device *dev;
+
+ struct imx_media_dev*md;
+
+ struct mutexmutex; /* mem2mem device mutex */
+};
+
+#define vdev_to_priv(v) container_of(v, struct ipu_csc_scaler_priv, vdev)
+
+/* Per-queue, driver-specific private data */
+struct ipu_csc_scaler_q_data {
+ struct v4l2_pix_format cur_fmt;
+ struct v4l2_rectrect;
+};
+
+struct ipu_csc_scaler_ctx {
+ struct ipu_csc_scaler_priv *priv;
+
+ struct v4l2_fh fh;
+ struct ipu_csc_scaler_q_dataq_data[2];
+ struct ipu_image_convert_ctx*icc;
+
+ struct v4l2_ctrl_handlerctrl_hdlr;
+ int rotate;
+ boolhflip;
+ boolvflip;
+ enum ipu_rotate_moderot_mode;
+ unsigned intsequence;
+};
+
+static struct ipu_csc_scaler_q_data *get_q_data(struct ipu
[PATCH v4l-utils] v4l-helpers.h: count mappings separately from buffers
This is required to avoid leaking mapped buffers when trying to unmap
after reqbufs(0), to test buffer orphaning.
Fixes: 6300b376cb3e ("v4l2-compliance: test orphaned buffer support");
Signed-off-by: Philipp Zabel
---
utils/common/v4l-helpers.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/utils/common/v4l-helpers.h b/utils/common/v4l-helpers.h
index d12f2745ef23..c37124e41efe 100644
--- a/utils/common/v4l-helpers.h
+++ b/utils/common/v4l-helpers.h
@@ -1406,6 +1406,7 @@ struct v4l_queue {
unsigned type;
unsigned memory;
unsigned buffers;
+ unsigned mappings;
unsigned num_planes;
unsigned capabilities;
@@ -1432,6 +1433,7 @@ static inline void v4l_queue_init(struct v4l_queue *q,
static inline unsigned v4l_queue_g_type(const struct v4l_queue *q) { return
q->type; }
static inline unsigned v4l_queue_g_memory(const struct v4l_queue *q) { return
q->memory; }
static inline unsigned v4l_queue_g_buffers(const struct v4l_queue *q) { return
q->buffers; }
+static inline unsigned v4l_queue_g_mappings(const struct v4l_queue *q) {
return q->mappings; }
static inline unsigned v4l_queue_g_num_planes(const struct v4l_queue *q) {
return q->num_planes; }
static inline unsigned v4l_queue_g_capabilities(const struct v4l_queue *q) {
return q->capabilities; }
@@ -1452,7 +1454,7 @@ static inline void v4l_queue_s_mmapping(struct v4l_queue
*q, unsigned index, uns
static inline void *v4l_queue_g_mmapping(const struct v4l_queue *q, unsigned
index, unsigned plane)
{
- if (index >= v4l_queue_g_buffers(q) || plane >=
v4l_queue_g_num_planes(q))
+ if (index >= v4l_queue_g_mappings(q) || plane >=
v4l_queue_g_num_planes(q))
return NULL;
return q->mmappings[index][plane];
}
@@ -1591,6 +1593,7 @@ static inline int v4l_queue_mmap_bufs(struct v4l_fd *f,
v4l_queue_s_mmapping(q, b, p, m);
}
}
+ q->mappings = b;
return 0;
}
static inline int v4l_queue_munmap_bufs(struct v4l_fd *f, struct v4l_queue *q,
@@ -1602,7 +1605,7 @@ static inline int v4l_queue_munmap_bufs(struct v4l_fd *f,
struct v4l_queue *q,
if (q->memory != V4L2_MEMORY_MMAP && q->memory != V4L2_MEMORY_DMABUF)
return 0;
- for (b = from; b < v4l_queue_g_buffers(q); b++) {
+ for (b = from; b < v4l_queue_g_mappings(q); b++) {
for (p = 0; p < v4l_queue_g_num_planes(q); p++) {
void *m = v4l_queue_g_mmapping(q, b, p);
@@ -1618,6 +1621,7 @@ static inline int v4l_queue_munmap_bufs(struct v4l_fd *f,
struct v4l_queue *q,
v4l_queue_s_mmapping(q, b, p, NULL);
}
}
+ q->mappings = from;
return 0;
}
--
2.20.1
Re: [PATCH v3] media: imx: mipi csi-2: Don't fail if initial state times-out
Hi Sakari, On Thu, 2019-08-08 at 11:26 +0300, Sakari Ailus wrote: [...] > > > Have you checked how it works if you simply leave out this test? Whether this works or not depends on the sensor used, and for some sensor/drivers may depend on timing (or random factors influencing it). See below. [...] > Some devices can be commanded to enter LP-11 state but some will just > briefly visit that state. The LP-11 state is mandatory but software should > not be involved in detecting it if at all possible. This is a good point. Devices that can be set to LP-11 state immediately, but that don't stay there long enough (either because they wait for less than the required by spec 100µs, or because system load causes this check to be executed too late) may end up working reliably even though the warning fires. > So if the hardware does not require further initialisation to be done in > LP-11 state, you should remove the check. > > > We had to fix at least OV5645 and OV5640 recently because of this, > > and I can imagine more drivers will have the same issue. > > This is actually an issue in the IMX driver (or hardware), not in the > sensor driver. It may be that sometimes it's easier to work around it in > the sensor driver. > > So, I'd like to know whether the check itself is a driver bug, or something > that the hardware requires. The fact that you're sending this patch > suggests the former. This is something that the hardware requires, according to the reference manual. See the comment in drivers/staging/media/imx/imx6-mipi-csi2.c, especially step 5.: /* * The required sequence of MIPI CSI-2 startup as specified in the i.MX6 * reference manual is as follows: * * 1. Deassert presetn signal (global reset). *It's not clear what this "global reset" signal is (maybe APB *global reset), but in any case this step would be probably *be carried out during driver load in csi2_probe(). * * 2. Configure MIPI Camera Sensor to put all Tx lanes in LP-11 state. *This must be carried out by the MIPI sensor's s_power(ON) subdev *op. * * 3. D-PHY initialization. * 4. CSI2 Controller programming (Set N_LANES, deassert PHY_SHUTDOWNZ, *deassert PHY_RSTZ, deassert CSI2_RESETN). * 5. Read the PHY status register (PHY_STATE) to confirm that all data and *clock lanes of the D-PHY are in LP-11 state. * 6. Configure the MIPI Camera Sensor to start transmitting a clock on the *D-PHY clock lane. * 7. CSI2 Controller programming - Read the PHY status register (PHY_STATE) *to confirm that the D-PHY is receiving a clock on the D-PHY clock lane. */ I read this as the hardware needing to see the LP-11 -> HS transition after the DPHY reset has been released, and before the CSI2 RX controller is programmed. If not all lanes are in LP-11 state at step 5., we can't guarantee that the DPHY has already seen this transition nor that it will see it in time before we start enabling the CSI2 RX. Since this can lead to anything from sporadic to 100% startup failure, depending on timing or whether the sensor is configured correctly to produce this transition at all, I'd like this warning to stay. It has been helpful before when developing sensor drivers. regards Philipp
Re: [PATCH v2] media: i2c: ov5645: Fix power sequence
On Wed, 2019-07-10 at 15:40 -0300, Ezequiel Garcia wrote: > This is mostly a port of Jacopo's fix: > > commit aa4bb8b8838ffcc776a79f49a4d7476b82405349 > Author: Jacopo Mondi > Date: Fri Jul 6 05:51:52 2018 -0400 > > media: ov5640: Re-work MIPI startup sequence > > In the OV5645 case, the changes are: > > - Move OV5645_IO_MIPI_CTRL00 (0x300e) out of the initial setting blob. > - At set_power(1) time power up MIPI Tx/Rx and set data and clock lanes in > LP11 during 'sleep' and 'idle' with MIPI clock in non-continuous mode. > - At set_power(0) time power down MIPI Tx/Rx (in addition to the current > power down of regulators and clock gating). > - At s_stream time enable/disable the MIPI interface output. > > With this commit the sensor is able to enter LP-11 mode during power up, > as expected by some CSI-2 controllers. > > Many thanks to Fabio Estevam for his help debugging this issue. > > Tested-by: Fabio Estevam > Signed-off-by: Ezequiel Garcia > --- > Changes in v2: > * As suggested by Philipp, move the initial configuration > to the ov5645_global_init_setting array. Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH 01/14] v4l2-ioctl.c: OR flags in v4l_fill_fmtdesc(), not don't overwrite
On Wed, 2019-07-24 at 15:30 +0200, Hans Verkuil wrote: > On 7/24/19 3:22 PM, Philipp Zabel wrote: > > On Wed, 2019-07-24 at 13:05 +0200, Hans Verkuil wrote: > > > If a driver sets a FMT flag in the enum_fmt op, then that will be > > > ignored since v4l_fill_fmtdesc() overwrites it again. > > > > > > v4l_fill_fmtdesc() should OR its flag, not overwrite it. > > > > > > Signed-off-by: Hans Verkuil > > > --- > > > drivers/media/v4l2-core/v4l2-ioctl.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c > > > b/drivers/media/v4l2-core/v4l2-ioctl.c > > > index 80efc581e3f9..911a20f915c5 100644 > > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > > > @@ -1390,7 +1390,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc > > > *fmt) > > > > > > if (descr) > > > WARN_ON(strscpy(fmt->description, descr, sz) < 0); > > > - fmt->flags = flags; > > > + fmt->flags |= flags; > > > } > > > > > > static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops, > > > > If the enum_fmt op does not write fmt->flags, will it not contain the > > value provided by userspace at this point? I think p->flags must be > > cleared in v4l2_enum_fmt() with this change, before the enum_fmt op is > > called. > > All fields after 'type' in struct v4l2_fmtdesc are cleared by the core: > search for INFO_FL_CLEAR(v4l2_fmtdesc, type) in v4l2-ioctl.c. > > So 'flags' is already zeroed when this function is called. Got it, thanks. In that case, Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH 01/14] v4l2-ioctl.c: OR flags in v4l_fill_fmtdesc(), not don't overwrite
On Wed, 2019-07-24 at 13:05 +0200, Hans Verkuil wrote: > If a driver sets a FMT flag in the enum_fmt op, then that will be > ignored since v4l_fill_fmtdesc() overwrites it again. > > v4l_fill_fmtdesc() should OR its flag, not overwrite it. > > Signed-off-by: Hans Verkuil > --- > drivers/media/v4l2-core/v4l2-ioctl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c > b/drivers/media/v4l2-core/v4l2-ioctl.c > index 80efc581e3f9..911a20f915c5 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -1390,7 +1390,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) > > if (descr) > WARN_ON(strscpy(fmt->description, descr, sz) < 0); > - fmt->flags = flags; > + fmt->flags |= flags; > } > > static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops, If the enum_fmt op does not write fmt->flags, will it not contain the value provided by userspace at this point? I think p->flags must be cleared in v4l2_enum_fmt() with this change, before the enum_fmt op is called. regards Philipp
Re: [PATCH] media: i2c: ov5645: Fix power up sequence
Hi Ezequiel,
On Wed, 2019-07-03 at 10:10 -0300, Ezequiel Garcia wrote:
> This is mostly a port of Jacopo's fix:
>
> commit aa4bb8b8838ffcc776a79f49a4d7476b82405349
> Author: Jacopo Mondi
> Date: Fri Jul 6 05:51:52 2018 -0400
>
> media: ov5640: Re-work MIPI startup sequence
>
> In the OV5645 case, the changes are:
>
> - Move OV5645_IO_MIPI_CTRL00 (0x300e) out of the initial setting blob.
> - At set_power(1) time power up MIPI Tx/Rx and set data and clock lanes in
> LP11 during 'sleep' and 'idle' with MIPI clock in non-continuous mode.
> - At set_power(0) time power down MIPI Tx/Rx (in addition to the current
> power down of regulators and clock gating).
> - At s_stream time enable/disable the MIPI interface output.
>
> With this commit the sensor is able to enter LP-11 mode during power up,
> as expected by some CSI-2 controllers.
>
> Many thanks to Fabio Estevam for his help debugging this issue.
>
> Signed-off-by: Ezequiel Garcia
> ---
> drivers/media/i2c/ov5645.c | 34 +++---
> 1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
> index 124c8df04633..05430a81c977 100644
> --- a/drivers/media/i2c/ov5645.c
> +++ b/drivers/media/i2c/ov5645.c
> @@ -45,6 +45,8 @@
> #define OV5645_CHIP_ID_HIGH_BYTE0x56
> #define OV5645_CHIP_ID_LOW 0x300b
> #define OV5645_CHIP_ID_LOW_BYTE 0x45
> +#define OV5645_IO_MIPI_CTRL000x300e
> +#define OV5645_PAD_OUTPUT00 0x3019
> #define OV5645_AWB_MANUAL_CONTROL0x3406
> #define OV5645_AWB_MANUAL_ENABLEBIT(0)
> #define OV5645_AEC_PK_MANUAL 0x3503
> @@ -55,6 +57,7 @@
> #define OV5645_ISP_VFLIPBIT(2)
> #define OV5645_TIMING_TC_REG21 0x3821
> #define OV5645_SENSOR_MIRRORBIT(1)
> +#define OV5645_MIPI_CTRL00 0x4800
> #define OV5645_PRE_ISP_TEST_SETTING_10x503d
> #define OV5645_TEST_PATTERN_MASK0x3
> #define OV5645_SET_TEST_PATTERN(x) ((x) &
> OV5645_TEST_PATTERN_MASK)
> @@ -121,7 +124,6 @@ static const struct reg_value
> ov5645_global_init_setting[] = {
> { 0x3503, 0x07 },
> { 0x3002, 0x1c },
> { 0x3006, 0xc3 },
> - { 0x300e, 0x45 },
> { 0x3017, 0x00 },
> { 0x3018, 0x00 },
> { 0x302e, 0x0b },
> @@ -737,13 +739,30 @@ static int ov5645_s_power(struct v4l2_subdev *sd, int
> on)
> goto exit;
> }
>
> - ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
> -OV5645_SYSTEM_CTRL0_STOP);
> + ret = ov5645_write_reg(ov5645,
> +OV5645_IO_MIPI_CTRL00, 0x40);
> if (ret < 0) {
> ov5645_set_power_off(ov5645);
> goto exit;
> }
> +
> + ret = ov5645_write_reg(ov5645,
> +OV5645_MIPI_CTRL00, 0x24);
> + if (ret < 0) {
> + ov5645_set_power_off(ov5645);
> + goto exit;
> + }
> +
> + ret = ov5645_write_reg(ov5645,
> +OV5645_PAD_OUTPUT00, 0x70);
> + if (ret < 0) {
> + ov5645_set_power_off(ov5645);
> + goto exit;
> + }
Could this just be added to the end of ov5645_global_init_setting?
--8<--
diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
index 05430a81c977..d978f7aa44c3 100644
--- a/drivers/media/i2c/ov5645.c
+++ b/drivers/media/i2c/ov5645.c
@@ -352,7 +352,10 @@ static const struct reg_value ov5645_global_init_setting[]
= {
{ 0x3a1f, 0x14 },
{ 0x0601, 0x02 },
{ 0x3008, 0x42 },
- { 0x3008, 0x02 }
+ { 0x3008, 0x02 },
+ { OV5645_IO_MIPI_CTRL00, 0x40 },
+ { OV5645_MIPI_CTRL00,0x24 },
+ { OV5645_PAD_OUTPUT00, 0x70 }
};
static const struct reg_value ov5645_setting_sxga[] = {
@@ -739,27 +742,6 @@ static int ov5645_s_power(struct v4l2_subdev *sd, int on)
goto exit;
}
- ret = ov5645_write_reg(ov5645,
- OV5645_IO_MIPI_CTRL00, 0x40);
- if (ret < 0) {
- ov5645_set_power_off(ov5645);
- goto exit;
- }
-
- ret = ov5645_write_reg(ov5645,
- OV5645_MIPI_CTRL00, 0x24);
- if (ret < 0) {
- ov5645
Re: [RFC] Stateful codecs and requirements for compressed formats
On Wed, 2019-07-03 at 17:32 +0900, Tomasz Figa wrote: > Hi Hans, > > On Fri, Jun 28, 2019 at 11:34 PM Hans Verkuil wrote: > > > > Hi all, > > > > I hope I Cc-ed everyone with a stake in this issue. > > > > One recurring question is how a stateful encoder fills buffers and how a > > stateful > > decoder consumes buffers. > > > > The most generic case is that an encoder produces a bitstream and just > > fills each > > CAPTURE buffer to the brim before continuing with the next buffer. > > > > I don't think there are drivers that do this, I believe that all drivers > > just > > output a single compressed frame. For interlaced formats I understand it is > > either > > one compressed field per buffer, or two compressed fields per buffer (this > > is > > what I heard, I don't know if this is true). > > > > In any case, I don't think this is specified anywhere. Please correct me if > > I am > > wrong. > > > > The latest stateful codec spec is here: > > > > https://hverkuil.home.xs4all.nl/codec-api/uapi/v4l/dev-mem2mem.html > > > > Assuming what I described above is indeed the case, then I think this should > > be documented. I don't know enough if a flag is needed somewhere to describe > > the behavior for interlaced formats, or can we leave this open and have > > userspace > > detect this? > > > > From Chromium perspective, we don't have any use case for encoding > interlaced contents, so we'll be okay with whatever the interested > parties decide on. :) > > > > > For decoders it is more complicated. The stateful decoder spec is written > > with > > the assumption that userspace can just fill each OUTPUT buffer to the brim > > with > > the compressed bitstream. I.e., no need to split at frame or other > > boundaries. > > > > See section 4.5.1.7 in the spec. > > > > But I understand that various HW decoders *do* have limitations. I would > > really > > like to know about those, since that needs to be exposed to userspace > > somehow. > > AFAIK mtk-vcodec needs H.264 SPS and PPS to be split into their own > separate buffers. I believe it also needs 1 buffer to contain exactly > 1 frame and 1 frame to be fully contained inside 1 buffer. > > Venus also needed 1 buffer to contain exactly 1 frame and 1 frame to > be fully contained inside 1 buffer. It used to have some specific > requirements regarding SPS and PPS too, but I think that was fixed in > the firmware. > > > > > Specifically, the venus decoder needs to know the resolution of the coded > > video > > beforehand > > I don't think that's true for venus. It does parsing and can detect > the resolution. > > However that's probably the case for coda... Yes, it is currently true for the coda driver. But I believe it is not actually necessary for coda hardware / firmware. I have already started to split sequence initialization (where the firmare parses the bitstream headers) from internal frame buffer allocation (which have to match capture buffers in size), and I think it should be possible to completely decouple the two and postpone buffer allocation far enough to allow output stream start without prior knowledge of the resolution. The decoder coda firmware fully parses the bitstream, but the driver has to copy it from the external output buffers into an internal bitstream ringbuffer anyway, and a few workarounds are necessary to make it always succeed regardless of whether the first buffer presented to it only contains headers, headers and a very small frame, or enough data to completely fill the bitstream reader's prefetch buffer. For this the driver has to parse the NAL start headers to a certain degree. Due to this bitstream copy in the driver, in theory there are no limits on how the input data is split into v4l2 buffers, but in practice only single frame per v4l2 output buffer use cases are actually tested regularly. The encoder produces a single compressed frame per buffer. There is no support for B frames in the firmware, as far as I can tell. There is no driver support for interlaced formats currently, I'm not sure whether the firmware supports interlacing. regards Philipp
Re: [PATCH] media: imx: mipi csi-2: Don't fail if initial state times-out
On Thu, 2019-06-27 at 15:12 -0700, Steve Longerbeam wrote: > > On 6/27/19 5:56 AM, Philipp Zabel wrote: > > Hi Fabio, > > > > On Thu, 2019-06-27 at 09:38 -0300, Fabio Estevam wrote: > > > Hi Philipp, > > > > > > On Thu, Jun 27, 2019 at 5:43 AM Philipp Zabel > > > wrote: > > > > > > > Are there any visual artifacts in the first frame(s) in this case? > > > > > > I do not observe visual artifacts when running gst-launch-1.0 v4l2src ! > > > kmssink > > > > > > > > So in my opinion the next version of this patch should make LP-11 > > > > > timeout a warning only, but keep the error return on clock lane > > > > > timeouts. > > > > > > > > I agree. > > > > > > Here is a reworked version of Ezequiel's patch as per the suggestions: > > > http://code.bulix.org/g5qap5-780475 > > > > > > Does this one look good? > > > > Limiting the change to wait_stopstate is fine, the actual message > > makes assumptions that could be misleading. How about: > > > > "Timeout waiting for LP-11 state on all active lanes. > > This is most likely caused by a bug in the sensor driver. > > Capture might fail or contain visual artifacts." > > Yes I agree that is more descriptive, if a bit wordy for a kernel error > message. Haha, yes, I remember that thought crossing my mind. > I think it could be reduced, something like: > > "LP-11 wait timeout on all lanes, likely a sensor driver bug, expect > capture failures." Much better, thanks. regards Philipp
Re: [PATCH v3] media: imx: mipi csi-2: Don't fail if initial state times-out
On Thu, 2019-06-27 at 19:29 -0300, Fabio Estevam wrote: > From: Ezequiel Garcia > > Not all sensors will be able to guarantee a proper initial state. > This may be either because the driver is not properly written, > or (probably unlikely) because the hardware won't support it. > > While the right solution in the former case is to fix the sensor > driver, the real world not always allows right solutions, due to lack > of available documentation and support on these sensors. > > Let's relax this requirement, and allow the driver to support stream start, > even if the sensor initial sequence wasn't the expected. > > Also improve the warning message to better explain the problem and provide > a hint that the sensor driver needs to be fixed. > > Signed-off-by: Ezequiel Garcia > Signed-off-by: Fabio Estevam Reviewed-by: Philipp Zabel thanks Philipp
Re: [PATCH] media: imx: mipi csi-2: Don't fail if initial state times-out
Hi Fabio, On Thu, 2019-06-27 at 09:38 -0300, Fabio Estevam wrote: > Hi Philipp, > > On Thu, Jun 27, 2019 at 5:43 AM Philipp Zabel wrote: > > > Are there any visual artifacts in the first frame(s) in this case? > > I do not observe visual artifacts when running gst-launch-1.0 v4l2src ! > kmssink > > > > So in my opinion the next version of this patch should make LP-11 > > > timeout a warning only, but keep the error return on clock lane timeouts. > > > > I agree. > > Here is a reworked version of Ezequiel's patch as per the suggestions: > http://code.bulix.org/g5qap5-780475 > > Does this one look good? Limiting the change to wait_stopstate is fine, the actual message makes assumptions that could be misleading. How about: "Timeout waiting for LP-11 state on all active lanes. This is most likely caused by a bug in the sensor driver. Capture might fail or contain visual artifacts." I'd like to keep the phy_state register output though, if only as dev_dbg(). It contains useful output for debugging, for example if only some of the lanes are in stop state, which could indicate an issue with connections or lane configuration. regards Philipp
Re: [PATCH] media: imx: mipi csi-2: Don't fail if initial state times-out
On Wed, 2019-06-26 at 16:29 -0700, Steve Longerbeam wrote: > Hi Fabio, > > On 6/26/19 4:22 PM, Fabio Estevam wrote: > > Hi Steve, > > > > On Wed, Jun 26, 2019 at 6:19 PM Steve Longerbeam > > wrote: > > > > > Did you only get the LP-11 timeout warning message with this patch on > > > the OV5645, or both the LP-11 timeout and clock lane timeout warnings? > > > > With this patch applied I get only the LP-11 timeout warnings, not > > clock lane timeouts. > > Ok thanks for the confirmation that the imx6 CSI-2 receiver is able to > successfully move to stream on without seeing the LP-11 state in this > case. Are there any visual artifacts in the first frame(s) in this case? > So in my opinion the next version of this patch should make LP-11 > timeout a warning only, but keep the error return on clock lane timeouts. I agree. regards Philipp
Re: [PATCH] v4l2-ioctl: call v4l_pix_format_touch() for TRY_FMT
On Wed, 2019-06-26 at 11:48 +0200, Hans Verkuil wrote: > The function v4l_pix_format_touch() is called for S_FMT to set > v4l2_pix_format fields to default values for a v4l-touch device, > but it wasn't called for TRY_FMT. Add this. > > Signed-off-by: Hans Verkuil > --- > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c > b/drivers/media/v4l2-core/v4l2-ioctl.c > index b1f4b991dba6..c5c8c8ab7cf6 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -1661,6 +1661,8 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops, > ret = ops->vidioc_try_fmt_vid_cap(file, fh, arg); > /* just in case the driver zeroed it again */ > p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC; > + if (vfd->vfl_type == VFL_TYPE_TOUCH) > + v4l_pix_format_touch(&p->fmt.pix); Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH 02/16] coda: set device_caps in struct video_device
On Wed, 2019-06-26 at 09:44 +0200, Hans Verkuil wrote: > Instead of filling in the struct v4l2_capability device_caps > field, fill in the struct video_device device_caps field. > > That way the V4L2 core knows what the capabilities of the > video device are. > > Signed-off-by: Hans Verkuil > Cc: Philipp Zabel > --- > drivers/media/platform/coda/coda-common.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/media/platform/coda/coda-common.c > b/drivers/media/platform/coda/coda-common.c > index 01428de2596e..73222c0615c0 100644 > --- a/drivers/media/platform/coda/coda-common.c > +++ b/drivers/media/platform/coda/coda-common.c > @@ -390,9 +390,6 @@ static int coda_querycap(struct file *file, void *priv, > strscpy(cap->card, coda_product_name(ctx->dev->devtype->product), > sizeof(cap->card)); > strscpy(cap->bus_info, "platform:" CODA_NAME, sizeof(cap->bus_info)); > - cap->device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING; > - cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS; > - > return 0; > } > > @@ -2699,6 +2696,7 @@ static int coda_register_device(struct coda_dev *dev, > int i) > vfd->lock = &dev->dev_mutex; > vfd->v4l2_dev = &dev->v4l2_dev; > vfd->vfl_dir= VFL_DIR_M2M; > + vfd->device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING; > video_set_drvdata(vfd, dev); > > /* Not applicable, use the selection API instead */ Reviewed-by: Philipp Zabel regards Philipp
Re: [PATCH] media: imx: mipi csi-2: Don't fail if initial state times-out
On Tue, 2019-06-25 at 17:39 -0300, Ezequiel Garcia wrote:
> Not all sensors will be able to guarantee a proper initial state.
> This may be either because the driver is not properly written,
> or (probably unlikely) because the hardware won't support it.
>
> While the right solution in the former case is to fix the sensor
> driver, the real world not always allows right solutions, due to lack
> of available documentation and support on these sensors.
Do you have a real world example for this?
> Let's relax this requirement, and allow the driver to support stream start,
> even if the sensor initial sequence wasn't the expected.
> A warning is still emitted, so users should be hinted that something is off.
I think the messages could use a note that they may be due to a sensor
or sensor driver bug, and that there might be image artifacts or
outright failure to capture as a consequence.
> Signed-off-by: Ezequiel Garcia
Reviewed-by: Philipp Zabel
regards
Philipp
> ---
> drivers/staging/media/imx/imx6-mipi-csi2.c | 33 ++
> 1 file changed, 9 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c
> b/drivers/staging/media/imx/imx6-mipi-csi2.c
> index f29e28df36ed..10342434e797 100644
> --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> @@ -243,7 +243,7 @@ static int __maybe_unused csi2_dphy_wait_ulp(struct
> csi2_dev *csi2)
> }
>
> /* Waits for low-power LP-11 state on data and clock lanes. */
> -static int csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
> +static void csi2_dphy_wait_stopstate(struct csi2_dev *csi2)
> {
> u32 mask, reg;
> int ret;
> @@ -253,29 +253,21 @@ static int csi2_dphy_wait_stopstate(struct csi2_dev
> *csi2)
>
> ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
>(reg & mask) == mask, 0, 50);
> - if (ret) {
> - v4l2_err(&csi2->sd, "LP-11 timeout, phy_state = 0x%08x\n", reg);
> - return ret;
> - }
> -
> - return 0;
> + if (ret)
> + v4l2_warn(&csi2->sd, "LP-11 timeout, phy_state = 0x%08x\n",
> reg);
> }
>
> /* Wait for active clock on the clock lane. */
> -static int csi2_dphy_wait_clock_lane(struct csi2_dev *csi2)
> +static void csi2_dphy_wait_clock_lane(struct csi2_dev *csi2)
> {
> u32 reg;
> int ret;
>
> ret = readl_poll_timeout(csi2->base + CSI2_PHY_STATE, reg,
>(reg & PHY_RXCLKACTIVEHS), 0, 50);
> - if (ret) {
> - v4l2_err(&csi2->sd, "clock lane timeout, phy_state = 0x%08x\n",
> - reg);
> - return ret;
> - }
> -
> - return 0;
> + if (ret)
> + v4l2_warn(&csi2->sd, "clock lane timeout, phy_state = 0x%08x\n",
> + reg);
> }
>
> /* Setup the i.MX CSI2IPU Gasket */
> @@ -316,9 +308,7 @@ static int csi2_start(struct csi2_dev *csi2)
> csi2_enable(csi2, true);
>
> /* Step 5 */
> - ret = csi2_dphy_wait_stopstate(csi2);
> - if (ret)
> - goto err_assert_reset;
> + csi2_dphy_wait_stopstate(csi2);
>
> /* Step 6 */
> ret = v4l2_subdev_call(csi2->src_sd, video, s_stream, 1);
> @@ -327,14 +317,9 @@ static int csi2_start(struct csi2_dev *csi2)
> goto err_assert_reset;
>
> /* Step 7 */
> - ret = csi2_dphy_wait_clock_lane(csi2);
> - if (ret)
> - goto err_stop_upstream;
> -
> + csi2_dphy_wait_clock_lane(csi2);
> return 0;
>
> -err_stop_upstream:
> - v4l2_subdev_call(csi2->src_sd, video, s_stream, 0);
> err_assert_reset:
> csi2_enable(csi2, false);
> err_disable_clk:
[PATCH 13/28] media: coda: do not enforce 512-byte initial bitstream payload on CODA960
On CODA960, sequence initialization can succeed if less than 512 bytes
are ready in the bitstream ring buffer.
On other variants, warn about too small payload in start_streaming.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index b30945fa0a70..dc9bce896003 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1790,7 +1790,9 @@ static int coda_start_streaming(struct vb2_queue *q,
unsigned int count)
coda_fill_bitstream(ctx, &list);
mutex_unlock(&ctx->bitstream_mutex);
- if (coda_get_bitstream_payload(ctx) < 512) {
+ if (ctx->dev->devtype->product != CODA_960 &&
+ coda_get_bitstream_payload(ctx) < 512) {
+ v4l2_err(v4l2_dev, "start payload < 512\n");
ret = -EINVAL;
goto err;
}
--
2.20.1
[PATCH 01/28] media: coda: implement CMD_START to restart decoding
From: Michael Tretter
The CMD_START shall be used to start the processing after a drain that
was initiated with CMD_STOP.
Up until now, a drain on coda could only be finished with a
STREAMOFF-STREAMON, which resulted in a reset of the device.
Signed-off-by: Michael Tretter
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 751b0be1c2ea..a5e0d5c1528e 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1059,16 +1059,29 @@ static int coda_decoder_cmd(struct file *file, void *fh,
struct v4l2_decoder_cmd *dc)
{
struct coda_ctx *ctx = fh_to_ctx(fh);
+ struct vb2_queue *dst_vq;
int ret;
ret = coda_try_decoder_cmd(file, fh, dc);
if (ret < 0)
return ret;
- /* Set the stream-end flag on this context */
- coda_bit_stream_end_flag(ctx);
- ctx->hold = false;
- v4l2_m2m_try_schedule(ctx->fh.m2m_ctx);
+ switch (dc->cmd) {
+ case V4L2_DEC_CMD_START:
+ dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
+V4L2_BUF_TYPE_VIDEO_CAPTURE);
+ vb2_clear_last_buffer_dequeued(dst_vq);
+ ctx->bit_stream_param &= ~CODA_BIT_STREAM_END_FLAG;
+ break;
+ case V4L2_DEC_CMD_STOP:
+ /* Set the stream-end flag on this context */
+ coda_bit_stream_end_flag(ctx);
+ ctx->hold = false;
+ v4l2_m2m_try_schedule(ctx->fh.m2m_ctx);
+ break;
+ default:
+ return -EINVAL;
+ }
return 0;
}
--
2.20.1
[PATCH 14/28] media: coda: flush bitstream ring buffer on decoder restart
The bitstream ringbuffer might be in an underrun state after draining,
or it might still contain unread data if the previous decoder stop
command was flagged as immediate. Flush the bitstream ring buffer
during V4L2_DEC_CMD_START to get into a well defined state. Also fill
the bitstream with buffers that have been queued during draining,
to resume decoding immediately.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c| 20
drivers/media/platform/coda/coda-common.c | 7 +++
drivers/media/platform/coda/coda.h| 1 +
3 files changed, 28 insertions(+)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 5a1016243032..843f92312f47 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -199,6 +199,26 @@ static int coda_h264_bitstream_pad(struct coda_ctx *ctx,
u32 size)
return (n < size) ? -ENOSPC : 0;
}
+int coda_bitstream_flush(struct coda_ctx *ctx)
+{
+ int ret;
+
+ if (ctx->inst_type != CODA_INST_DECODER || !ctx->use_bit)
+ return 0;
+
+ ret = coda_command_sync(ctx, CODA_COMMAND_DEC_BUF_FLUSH);
+ if (ret < 0) {
+ v4l2_err(&ctx->dev->v4l2_dev, "failed to flush bitstream\n");
+ return ret;
+ }
+
+ kfifo_init(&ctx->bitstream_fifo, ctx->bitstream.vaddr,
+ ctx->bitstream.size);
+ coda_kfifo_sync_to_device_full(ctx);
+
+ return 0;
+}
+
static int coda_bitstream_queue(struct coda_ctx *ctx, const u8 *buf, u32 size)
{
u32 n = kfifo_in(&ctx->bitstream_fifo, buf, size);
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index dc9bce896003..ddd819ea13f2 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1052,6 +1052,7 @@ static int coda_decoder_cmd(struct file *file, void *fh,
struct v4l2_decoder_cmd *dc)
{
struct coda_ctx *ctx = fh_to_ctx(fh);
+ struct coda_dev *dev = ctx->dev;
struct vb2_queue *dst_vq;
int ret;
@@ -1061,10 +1062,16 @@ static int coda_decoder_cmd(struct file *file, void *fh,
switch (dc->cmd) {
case V4L2_DEC_CMD_START:
+ mutex_lock(&ctx->bitstream_mutex);
+ mutex_lock(&dev->coda_mutex);
+ coda_bitstream_flush(ctx);
+ mutex_unlock(&dev->coda_mutex);
dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
V4L2_BUF_TYPE_VIDEO_CAPTURE);
vb2_clear_last_buffer_dequeued(dst_vq);
ctx->bit_stream_param &= ~CODA_BIT_STREAM_END_FLAG;
+ coda_fill_bitstream(ctx, NULL);
+ mutex_unlock(&ctx->bitstream_mutex);
break;
case V4L2_DEC_CMD_STOP:
/* Set the stream-end flag on this context */
diff --git a/drivers/media/platform/coda/coda.h
b/drivers/media/platform/coda/coda.h
index 12bbd3129269..6911c1c811ce 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -322,6 +322,7 @@ static inline bool coda_bitstream_can_fetch_past(struct
coda_ctx *ctx,
}
bool coda_bitstream_can_fetch_past(struct coda_ctx *ctx, unsigned int pos);
+int coda_bitstream_flush(struct coda_ctx *ctx);
void coda_bit_stream_end_flag(struct coda_ctx *ctx);
--
2.20.1
[PATCH 07/28] media: coda: split decoder sequence initialization out of start decoding
The sequence initialization already has to happen during the
initialization phase, after headers have been queued on the OUTPUT
queue. This means that sequence initialization has to be queued as
a work item from QBUF on the OUTPUT queue. The internal framebuffer
setup should be done later during VIDIOC_REQBUFS() on the CAPTURE
queue.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 30 ++
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 45ffe2e87e0a..cecfd51e3838 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1661,7 +1661,7 @@ static bool coda_reorder_enable(struct coda_ctx *ctx)
return profile > V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE;
}
-static int __coda_start_decoding(struct coda_ctx *ctx)
+static int __coda_decoder_seq_init(struct coda_ctx *ctx)
{
struct coda_q_data *q_data_src, *q_data_dst;
u32 bitstream_buf, bitstream_size;
@@ -1684,8 +1684,6 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
src_fourcc = q_data_src->fourcc;
dst_fourcc = q_data_dst->fourcc;
- coda_write(dev, ctx->parabuf.paddr, CODA_REG_BIT_PARA_BUF_ADDR);
-
/* Update coda bitstream read and write pointers from kfifo */
coda_kfifo_sync_to_device_full(ctx);
@@ -1823,6 +1821,29 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
coda_update_profile_level_ctrls(ctx, profile, level);
}
+ return 0;
+}
+
+static int __coda_start_decoding(struct coda_ctx *ctx)
+{
+ struct coda_q_data *q_data_src, *q_data_dst;
+ struct coda_dev *dev = ctx->dev;
+ u32 src_fourcc, dst_fourcc;
+ int ret;
+
+ if (!ctx->initialized) {
+ ret = __coda_decoder_seq_init(ctx);
+ if (ret < 0)
+ return ret;
+ }
+
+ q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
+ q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
+ src_fourcc = q_data_src->fourcc;
+ dst_fourcc = q_data_dst->fourcc;
+
+ coda_write(dev, ctx->parabuf.paddr, CODA_REG_BIT_PARA_BUF_ADDR);
+
ret = coda_alloc_framebuffers(ctx, q_data_dst, src_fourcc);
if (ret < 0) {
v4l2_err(&dev->v4l2_dev, "failed to allocate framebuffers\n");
@@ -1831,7 +1852,8 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
/* Tell the decoder how many frame buffers we allocated. */
coda_write(dev, ctx->num_internal_frames, CODA_CMD_SET_FRAME_BUF_NUM);
- coda_write(dev, width, CODA_CMD_SET_FRAME_BUF_STRIDE);
+ coda_write(dev, round_up(q_data_dst->rect.width, 16),
+ CODA_CMD_SET_FRAME_BUF_STRIDE);
if (dev->devtype->product != CODA_DX6) {
/* Set secondary AXI IRAM */
--
2.20.1
[PATCH 17/28] media: coda: mark the last output buffer on decoder stop command
Mark the last output buffer to be decoded and only copy pending queued
output buffers into the bitstream ring buffer in the BIT processor
decoder case.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c| 3 +++
drivers/media/platform/coda/coda-common.c | 6 ++
2 files changed, 9 insertions(+)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index bfe6019e68a8..cbcec571a014 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -312,6 +312,9 @@ static bool coda_bitstream_try_queue(struct coda_ctx *ctx,
if (ctx == v4l2_m2m_get_curr_priv(ctx->dev->m2m_dev))
coda_kfifo_sync_to_device_write(ctx);
+ /* Set the stream-end flag after the last buffer is queued */
+ if (src_buf->flags & V4L2_BUF_FLAG_LAST)
+ coda_bit_stream_end_flag(ctx);
ctx->hold = false;
return true;
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 29d050fbb899..9b32b5862aa8 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1071,6 +1071,7 @@ static int coda_decoder_cmd(struct file *file, void *fh,
{
struct coda_ctx *ctx = fh_to_ctx(fh);
struct coda_dev *dev = ctx->dev;
+ struct vb2_v4l2_buffer *buf;
struct vb2_queue *dst_vq;
int ret;
@@ -1092,6 +1093,11 @@ static int coda_decoder_cmd(struct file *file, void *fh,
mutex_unlock(&ctx->bitstream_mutex);
break;
case V4L2_DEC_CMD_STOP:
+ buf = v4l2_m2m_last_src_buf(ctx->fh.m2m_ctx);
+ if (buf)
+ /* Mark last buffer */
+ buf->flags |= V4L2_BUF_FLAG_LAST;
+
/* Set the stream-end flag on this context */
coda_bit_stream_end_flag(ctx);
ctx->hold = false;
--
2.20.1
[PATCH 16/28] media: coda: allow flagging last output buffer internally
Since V4L2_BUF_FLAG_LAST is a CAPTURE only flag, clear it from OUTPUT
buffers in QBUF and DQBUF. This allows to use the flag internally to
signal the last buffer to decode after a decoder stop command was
issued.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index ddd819ea13f2..29d050fbb899 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -879,9 +879,27 @@ static int coda_qbuf(struct file *file, void *priv,
{
struct coda_ctx *ctx = fh_to_ctx(priv);
+ if (ctx->inst_type == CODA_INST_DECODER &&
+ buf->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+ buf->flags &= ~V4L2_BUF_FLAG_LAST;
+
return v4l2_m2m_qbuf(file, ctx->fh.m2m_ctx, buf);
}
+static int coda_dqbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
+{
+ struct coda_ctx *ctx = fh_to_ctx(priv);
+ int ret;
+
+ ret = v4l2_m2m_dqbuf(file, ctx->fh.m2m_ctx, buf);
+
+ if (ctx->inst_type == CODA_INST_DECODER &&
+ buf->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+ buf->flags &= ~V4L2_BUF_FLAG_LAST;
+
+ return ret;
+}
+
static bool coda_buf_is_end_of_stream(struct coda_ctx *ctx,
struct vb2_v4l2_buffer *buf)
{
@@ -1295,7 +1313,7 @@ static const struct v4l2_ioctl_ops coda_ioctl_ops = {
.vidioc_qbuf= coda_qbuf,
.vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
- .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
+ .vidioc_dqbuf = coda_dqbuf,
.vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
.vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
--
2.20.1
[PATCH 22/28] media: coda: flag the last encoded buffer
Use the flagged last output buffer to also flag the corresponding
capture buffer after encoding. This causes the end of stream event
to be issued and the buffer to be dequeued with the last flag set.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index cbcec571a014..7bcdfe8dcf3d 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1565,13 +1565,14 @@ static void coda_finish_encode(struct coda_ctx *ctx)
coda_read(dev, CODA_RET_ENC_PIC_SLICE_NUM);
coda_read(dev, CODA_RET_ENC_PIC_FLAG);
- if (coda_read(dev, CODA_RET_ENC_PIC_TYPE) == 0) {
+ dst_buf->flags &= ~(V4L2_BUF_FLAG_KEYFRAME |
+ V4L2_BUF_FLAG_PFRAME |
+ V4L2_BUF_FLAG_LAST);
+ if (coda_read(dev, CODA_RET_ENC_PIC_TYPE) == 0)
dst_buf->flags |= V4L2_BUF_FLAG_KEYFRAME;
- dst_buf->flags &= ~V4L2_BUF_FLAG_PFRAME;
- } else {
+ else
dst_buf->flags |= V4L2_BUF_FLAG_PFRAME;
- dst_buf->flags &= ~V4L2_BUF_FLAG_KEYFRAME;
- }
+ dst_buf->flags |= src_buf->flags & V4L2_BUF_FLAG_LAST;
v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, false);
@@ -1584,8 +1585,9 @@ static void coda_finish_encode(struct coda_ctx *ctx)
if (ctx->gopcounter < 0)
ctx->gopcounter = ctx->params.gop_size - 1;
- coda_dbg(1, ctx, "job finished: encoded %c frame (%d)\n",
-coda_frame_type_char(dst_buf->flags), dst_buf->sequence);
+ coda_dbg(1, ctx, "job finished: encoded %c frame (%d)%s\n",
+coda_frame_type_char(dst_buf->flags), dst_buf->sequence,
+(dst_buf->flags & V4L2_BUF_FLAG_LAST) ? " (last)" : "");
}
static void coda_seq_end_work(struct work_struct *work)
--
2.20.1
[PATCH 25/28] media: coda: mark last returned frame
If reordering is not enabled, the last decoded frame has to be the last
returned buffer. Otherwise wait for the firmware to report no more
frame to display. In that case the return buffer is the last one as
well, and can be reported as such.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 1157454e3bc8..167a92772c84 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -2381,6 +2381,23 @@ static void coda_finish_decode(struct coda_ctx *ctx)
V4L2_BUF_FLAG_BFRAME);
dst_buf->flags |= ready_frame->type;
meta = &ready_frame->meta;
+ if (meta->last && !coda_reorder_enable(ctx)) {
+ /*
+* If this was the last decoded frame, and reordering
+* is disabled, this will be the last display frame.
+*/
+ coda_dbg(1, ctx, "last meta, marking as last frame\n");
+ dst_buf->flags |= V4L2_BUF_FLAG_LAST;
+ } else if (ctx->bit_stream_param & CODA_BIT_STREAM_END_FLAG &&
+ display_idx == -1) {
+ /*
+* If there is no designated presentation frame anymore,
+* this frame has to be the last one.
+*/
+ coda_dbg(1, ctx,
+"no more frames to return, marking as last
frame\n");
+ dst_buf->flags |= V4L2_BUF_FLAG_LAST;
+ }
dst_buf->timecode = meta->timecode;
dst_buf->vb2_buf.timestamp = meta->timestamp;
--
2.20.1
[PATCH 15/28] media: coda: increment sequence offset for the last returned frame
If no more frames are decoded in bitstream end mode, and a previously
decoded frame has been returned, the firmware still increments the frame
number. To avoid a sequence number mismatch after decoder restart,
increment the sequence_offset correction parameter.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 843f92312f47..bfe6019e68a8 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -2280,6 +2280,9 @@ static void coda_finish_decode(struct coda_ctx *ctx)
else if (ctx->display_idx < 0)
ctx->hold = true;
} else if (decoded_idx == -2) {
+ if (ctx->display_idx >= 0 &&
+ ctx->display_idx < ctx->num_internal_frames)
+ ctx->sequence_offset++;
/* no frame was decoded, we still return remaining buffers */
} else if (decoded_idx < 0 || decoded_idx >= ctx->num_internal_frames) {
v4l2_err(&dev->v4l2_dev,
--
2.20.1
[PATCH 03/28] media: coda: fix mpeg2 sequence number handling
Sequence number handling assumed that the BIT processor frame number
starts counting at 1, but this is not true for the MPEG-2 decoder,
which starts at 0. Fix the sequence counter offset detection to handle
this.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 06a352659bae..45ffe2e87e0a 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1746,6 +1746,7 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
v4l2_err(&dev->v4l2_dev, "CODA_COMMAND_SEQ_INIT timeout\n");
return ret;
}
+ ctx->sequence_offset = ~0U;
ctx->initialized = 1;
/* Update kfifo out pointer from coda bitstream read pointer */
@@ -2169,7 +2170,9 @@ static void coda_finish_decode(struct coda_ctx *ctx)
v4l2_err(&dev->v4l2_dev,
"decoded frame index out of range: %d\n", decoded_idx);
} else {
- val = coda_read(dev, CODA_RET_DEC_PIC_FRAME_NUM) - 1;
+ val = coda_read(dev, CODA_RET_DEC_PIC_FRAME_NUM);
+ if (ctx->sequence_offset == -1)
+ ctx->sequence_offset = val;
val -= ctx->sequence_offset;
spin_lock(&ctx->buffer_meta_lock);
if (!list_empty(&ctx->buffer_meta_list)) {
--
2.20.1
[PATCH 19/28] media: coda: mark the last output buffer on encoder stop command
Mark the last output buffer to be encoded.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 4002a5b8c9ea..c55124e8b4c8 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1037,12 +1037,17 @@ static int coda_encoder_cmd(struct file *file, void *fh,
struct v4l2_encoder_cmd *ec)
{
struct coda_ctx *ctx = fh_to_ctx(fh);
+ struct vb2_v4l2_buffer *buf;
int ret;
ret = coda_try_encoder_cmd(file, fh, ec);
if (ret < 0)
return ret;
+ buf = v4l2_m2m_last_src_buf(ctx->fh.m2m_ctx);
+ if (buf)
+ buf->flags |= V4L2_BUF_FLAG_LAST;
+
/* Set the stream-end flag on this context */
ctx->bit_stream_param |= CODA_BIT_STREAM_END_FLAG;
--
2.20.1
[PATCH 12/28] media: coda: pad first buffer with repeated MPEG headers to fix sequence init
If the first buffer contains only headers, the sequence initialization
command fails. On CodaHx4 the buffer must be padded to at least 512
bytes, on CODA960 it seems to be enough to just repeat the sequence and
extension headers (MPEG-2) or the VOS and VO headers (MPEG-4) once for
for sequence initialization to succeed without further bitstream data.
On CodaHx4 the headers can be repeated multiple times until the 512 byte
mark is reached.
A similar issue was solved for h.264 by padding with a filler NAL in
commit 0eef89403ece ("[media] coda: pad first h.264 buffer to 512
bytes").
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 59 ++--
drivers/media/platform/coda/coda-mpeg2.c | 43 +
drivers/media/platform/coda/coda-mpeg4.c | 38 +++
drivers/media/platform/coda/coda.h | 2 +
4 files changed, 139 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 4f96f808b4dd..5a1016243032 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -180,7 +180,7 @@ static void coda_kfifo_sync_to_device_write(struct coda_ctx
*ctx)
coda_write(dev, wr_ptr, CODA_REG_BIT_WR_PTR(ctx->reg_idx));
}
-static int coda_bitstream_pad(struct coda_ctx *ctx, u32 size)
+static int coda_h264_bitstream_pad(struct coda_ctx *ctx, u32 size)
{
unsigned char *buf;
u32 n;
@@ -206,12 +206,34 @@ static int coda_bitstream_queue(struct coda_ctx *ctx,
const u8 *buf, u32 size)
return (n < size) ? -ENOSPC : 0;
}
+static u32 coda_buffer_parse_headers(struct coda_ctx *ctx,
+struct vb2_v4l2_buffer *src_buf,
+u32 payload)
+{
+ u8 *vaddr = vb2_plane_vaddr(&src_buf->vb2_buf, 0);
+ u32 size = 0;
+
+ switch (ctx->codec->src_fourcc) {
+ case V4L2_PIX_FMT_MPEG2:
+ size = coda_mpeg2_parse_headers(ctx, vaddr, payload);
+ break;
+ case V4L2_PIX_FMT_MPEG4:
+ size = coda_mpeg4_parse_headers(ctx, vaddr, payload);
+ break;
+ default:
+ break;
+ }
+
+ return size;
+}
+
static bool coda_bitstream_try_queue(struct coda_ctx *ctx,
struct vb2_v4l2_buffer *src_buf)
{
unsigned long payload = vb2_get_plane_payload(&src_buf->vb2_buf, 0);
u8 *vaddr = vb2_plane_vaddr(&src_buf->vb2_buf, 0);
int ret;
+ int i;
if (coda_get_bitstream_payload(ctx) + payload + 512 >=
ctx->bitstream.size)
@@ -222,10 +244,41 @@ static bool coda_bitstream_try_queue(struct coda_ctx *ctx,
return true;
}
- /* Add zero padding before the first H.264 buffer, if it is too small */
+ if (ctx->qsequence == 0 && payload < 512) {
+ /*
+* Add padding after the first buffer, if it is too small to be
+* fetched by the CODA, by repeating the headers. Without
+* repeated headers, or the first frame already queued, decoder
+* sequence initialization fails with error code 0x2000 on i.MX6
+* or error code 0x1 on i.MX51.
+*/
+ u32 header_size = coda_buffer_parse_headers(ctx, src_buf,
+ payload);
+
+ if (header_size) {
+ coda_dbg(1, ctx, "pad with %u-byte header\n",
+header_size);
+ for (i = payload; i < 512; i += header_size) {
+ ret = coda_bitstream_queue(ctx, vaddr,
+ header_size);
+ if (ret < 0) {
+ v4l2_err(&ctx->dev->v4l2_dev,
+"bitstream buffer overflow\n");
+ return false;
+ }
+ if (ctx->dev->devtype->product == CODA_960)
+ break;
+ }
+ } else {
+ coda_dbg(1, ctx,
+"could not parse header, sequence
initialization might fail\n");
+ }
+ }
+
+ /* Add padding before the first buffer, if it is too small */
if (ctx->qsequence == 0 && payload < 512 &&
ctx->codec->src_fourcc == V4L2_PIX_FMT_H264)
- coda_bitstream_pad(ctx, 512 - payload);
+ coda_h264_bitstream_pad(ctx, 512 - payload);
ret = coda_bitstream_queue(ctx, vaddr, payload);
if (ret <
[PATCH 08/28] media: coda: add sequence initialization work
Add a sequence initialization work item to be run when OUTPUT buffers
are queued in the initialization state.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c| 25 +++
drivers/media/platform/coda/coda-common.c | 24 ++
drivers/media/platform/coda/coda.h| 2 ++
3 files changed, 51 insertions(+)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index cecfd51e3838..9f8e2342d175 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1824,6 +1824,30 @@ static int __coda_decoder_seq_init(struct coda_ctx *ctx)
return 0;
}
+static void coda_dec_seq_init_work(struct work_struct *work)
+{
+ struct coda_ctx *ctx = container_of(work,
+ struct coda_ctx, seq_init_work);
+ struct coda_dev *dev = ctx->dev;
+ int ret;
+
+ mutex_lock(&ctx->buffer_mutex);
+ mutex_lock(&dev->coda_mutex);
+
+ if (ctx->initialized == 1)
+ goto out;
+
+ ret = __coda_decoder_seq_init(ctx);
+ if (ret < 0)
+ goto out;
+
+ ctx->initialized = 1;
+
+out:
+ mutex_unlock(&dev->coda_mutex);
+ mutex_unlock(&ctx->buffer_mutex);
+}
+
static int __coda_start_decoding(struct coda_ctx *ctx)
{
struct coda_q_data *q_data_src, *q_data_dst;
@@ -2352,6 +2376,7 @@ const struct coda_context_ops coda_bit_decode_ops = {
.prepare_run = coda_prepare_decode,
.finish_run = coda_finish_decode,
.run_timeout = coda_decode_timeout,
+ .seq_init_work = coda_dec_seq_init_work,
.seq_end_work = coda_seq_end_work,
.release = coda_bit_release,
};
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 232bda4b7016..f8cfafa2ce42 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1684,6 +1684,19 @@ static void coda_buf_queue(struct vb2_buffer *vb)
/* This set buf->sequence = ctx->qsequence++ */
coda_fill_bitstream(ctx, NULL);
mutex_unlock(&ctx->bitstream_mutex);
+
+ if (!ctx->initialized) {
+ /*
+* Run sequence initialization in case the queued
+* buffer contained headers.
+*/
+ if (vb2_is_streaming(vb->vb2_queue) &&
+ ctx->ops->seq_init_work) {
+ queue_work(ctx->dev->workqueue,
+ &ctx->seq_init_work);
+ flush_work(&ctx->seq_init_work);
+ }
+ }
} else {
if (ctx->inst_type == CODA_INST_ENCODER &&
vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
@@ -1761,6 +1774,15 @@ static int coda_start_streaming(struct vb2_queue *q,
unsigned int count)
ret = -EINVAL;
goto err;
}
+
+ if (!ctx->initialized) {
+ /* Run sequence initialization */
+ if (ctx->ops->seq_init_work) {
+ queue_work(ctx->dev->workqueue,
+ &ctx->seq_init_work);
+ flush_work(&ctx->seq_init_work);
+ }
+ }
}
ctx->streamon_out = 1;
@@ -2317,6 +2339,8 @@ static int coda_open(struct file *file)
ctx->use_bit = !ctx->cvd->direct;
init_completion(&ctx->completion);
INIT_WORK(&ctx->pic_run_work, coda_pic_run_work);
+ if (ctx->ops->seq_init_work)
+ INIT_WORK(&ctx->seq_init_work, ctx->ops->seq_init_work);
if (ctx->ops->seq_end_work)
INIT_WORK(&ctx->seq_end_work, ctx->ops->seq_end_work);
v4l2_fh_init(&ctx->fh, video_devdata(file));
diff --git a/drivers/media/platform/coda/coda.h
b/drivers/media/platform/coda/coda.h
index 0c2cace53ce8..c97ea3e24b80 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -185,6 +185,7 @@ struct coda_context_ops {
int (*prepare_run)(struct coda_ctx *ctx);
void (*finish_run)(struct coda_ctx *ctx);
void (*run_timeout)(struct coda_ctx *ctx);
+ void (*seq_init_work)(struct work_struct *work);
void (*seq_end_work)(struct work_struct *work);
void (*release)(struct coda_ctx *ctx);
};
@@ -193,6 +194,7 @@ struct coda_ctx {
[PATCH 05/28] media: coda: add coda_wake_up_capture_queue
Combine setting the last_buffer_dequeued flag on the capture video
queue and waking up its done workqueue into a helper function.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index d3b1e7b87a8a..7dbeb80d40f4 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1004,11 +1004,21 @@ static int coda_try_encoder_cmd(struct file *file, void
*fh,
return v4l2_m2m_ioctl_try_encoder_cmd(file, fh, ec);
}
+static void coda_wake_up_capture_queue(struct coda_ctx *ctx)
+{
+ struct vb2_queue *dst_vq;
+
+ coda_dbg(1, ctx, "waking up capture queue\n");
+
+ dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
+ dst_vq->last_buffer_dequeued = true;
+ wake_up(&dst_vq->done_wq);
+}
+
static int coda_encoder_cmd(struct file *file, void *fh,
struct v4l2_encoder_cmd *ec)
{
struct coda_ctx *ctx = fh_to_ctx(fh);
- struct vb2_queue *dst_vq;
int ret;
ret = coda_try_encoder_cmd(file, fh, ec);
@@ -1021,12 +1031,8 @@ static int coda_encoder_cmd(struct file *file, void *fh,
flush_work(&ctx->pic_run_work);
/* If there is no buffer in flight, wake up */
- if (!ctx->streamon_out || ctx->qsequence == ctx->osequence) {
- dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
-V4L2_BUF_TYPE_VIDEO_CAPTURE);
- dst_vq->last_buffer_dequeued = true;
- wake_up(&dst_vq->done_wq);
- }
+ if (!ctx->streamon_out || ctx->qsequence == ctx->osequence)
+ coda_wake_up_capture_queue(ctx);
return 0;
}
--
2.20.1
[PATCH 04/28] media: coda: fix last buffer handling in V4L2_ENC_CMD_STOP
From: Marco Felsch
coda_encoder_cmd() is racy, as the last scheduled picture run worker can
still be in-flight while the ENC_CMD_STOP command is issued. Depending
on the exact timing the sequence numbers might already be changed, but
the last buffer might not have been put on the destination queue yet.
In this case the current implementation would prematurely wake the
destination queue with last_buffer_dequeued=true, causing userspace to
call streamoff before the last buffer is handled.
Close this race window by synchronizing with the pic_run_worker before
doing the sequence check.
Signed-off-by: Marco Felsch
[l.st...@pengutronix.de: switch to flush_work, reword commit message]
Signed-off-by: Lucas Stach
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index e09d48170e5c..d3b1e7b87a8a 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1018,6 +1018,8 @@ static int coda_encoder_cmd(struct file *file, void *fh,
/* Set the stream-end flag on this context */
ctx->bit_stream_param |= CODA_BIT_STREAM_END_FLAG;
+ flush_work(&ctx->pic_run_work);
+
/* If there is no buffer in flight, wake up */
if (!ctx->streamon_out || ctx->qsequence == ctx->osequence) {
dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
--
2.20.1
[PATCH 23/28] media: coda: lock capture queue wakeup against encoder stop command
Make sure that an encoder stop command running concurrently with an
encoder finish_run always either flags the last returned buffer or wakes
up the capture queue to signal the end of stream condition afterwards.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c| 7 +++
drivers/media/platform/coda/coda-common.c | 19 ++-
drivers/media/platform/coda/coda.h| 6 ++
3 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 7bcdfe8dcf3d..44085e3d43d5 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1540,6 +1540,12 @@ static void coda_finish_encode(struct coda_ctx *ctx)
struct coda_dev *dev = ctx->dev;
u32 wr_ptr, start_ptr;
+ /*
+* Lock to make sure that an encoder stop command running in parallel
+* will either already have marked src_buf as last, or it will wake up
+* the capture queue after the buffers are returned.
+*/
+ mutex_lock(&ctx->wakeup_mutex);
src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
@@ -1580,6 +1586,7 @@ static void coda_finish_encode(struct coda_ctx *ctx)
dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
coda_m2m_buf_done(ctx, dst_buf, VB2_BUF_STATE_DONE);
+ mutex_unlock(&ctx->wakeup_mutex);
ctx->gopcounter--;
if (ctx->gopcounter < 0)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 0d64ddc49494..829d1e565911 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1034,19 +1034,27 @@ static int coda_encoder_cmd(struct file *file, void *fh,
if (ret < 0)
return ret;
+ mutex_lock(&ctx->wakeup_mutex);
buf = v4l2_m2m_last_src_buf(ctx->fh.m2m_ctx);
if (buf) {
+ /*
+* If the last output buffer is still on the queue, make sure
+* that decoder finish_run will see the last flag and report it
+* to userspace.
+*/
buf->flags |= V4L2_BUF_FLAG_LAST;
} else {
/* Set the stream-end flag on this context */
ctx->bit_stream_param |= CODA_BIT_STREAM_END_FLAG;
- flush_work(&ctx->pic_run_work);
-
- /* If there is no buffer in flight, wake up */
- if (!ctx->streamon_out || ctx->qsequence == ctx->osequence)
- coda_wake_up_capture_queue(ctx);
+ /*
+* If the last output buffer has already been taken from the
+* queue, wake up the capture queue and signal end of stream
+* via the -EPIPE mechanism.
+*/
+ coda_wake_up_capture_queue(ctx);
}
+ mutex_unlock(&ctx->wakeup_mutex);
return 0;
}
@@ -2466,6 +2474,7 @@ static int coda_open(struct file *file)
mutex_init(&ctx->bitstream_mutex);
mutex_init(&ctx->buffer_mutex);
+ mutex_init(&ctx->wakeup_mutex);
INIT_LIST_HEAD(&ctx->buffer_meta_list);
spin_lock_init(&ctx->buffer_meta_lock);
diff --git a/drivers/media/platform/coda/coda.h
b/drivers/media/platform/coda/coda.h
index 6911c1c811ce..97845e58fb8b 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -258,6 +258,12 @@ struct coda_ctx {
booluse_bit;
booluse_vdoa;
struct vdoa_ctx *vdoa;
+ /*
+* wakeup mutex used to serialize encoder stop command and finish_run,
+* ensures that finish_run always either flags the last returned buffer
+* or wakes up the capture queue to signal EOS afterwards.
+*/
+ struct mutexwakeup_mutex;
};
extern int coda_debug;
--
2.20.1
[PATCH 10/28] media: coda: integrate internal frame metadata into a structure
Combine the separate auxiliary buffer, buffer meta, frame type, and
decode error arrays into an array of struct coda_internal_frame.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 62 +-
drivers/media/platform/coda/coda.h | 12 +++--
2 files changed, 40 insertions(+), 34 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 9f8e2342d175..494bc130c7af 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -395,7 +395,7 @@ static void coda_free_framebuffers(struct coda_ctx *ctx)
int i;
for (i = 0; i < CODA_MAX_FRAMEBUFFERS; i++)
- coda_free_aux_buf(ctx->dev, &ctx->internal_frames[i]);
+ coda_free_aux_buf(ctx->dev, &ctx->internal_frames[i].buf);
}
static int coda_alloc_framebuffers(struct coda_ctx *ctx,
@@ -435,7 +435,7 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
coda_free_framebuffers(ctx);
return -ENOMEM;
}
- ret = coda_alloc_context_buf(ctx, &ctx->internal_frames[i],
+ ret = coda_alloc_context_buf(ctx, &ctx->internal_frames[i].buf,
size, name);
kfree(name);
if (ret < 0) {
@@ -449,7 +449,7 @@ static int coda_alloc_framebuffers(struct coda_ctx *ctx,
u32 y, cb, cr, mvcol;
/* Start addresses of Y, Cb, Cr planes */
- y = ctx->internal_frames[i].paddr;
+ y = ctx->internal_frames[i].buf.paddr;
cb = y + ysize;
cr = y + ysize + ysize/4;
mvcol = y + ysize + ysize/4 + ysize/4;
@@ -1202,9 +1202,9 @@ static int coda_start_encoding(struct coda_ctx *ctx)
coda9_set_frame_cache(ctx, q_data_src->fourcc);
/* FIXME */
- coda_write(dev, ctx->internal_frames[2].paddr,
+ coda_write(dev, ctx->internal_frames[2].buf.paddr,
CODA9_CMD_SET_FRAME_SUBSAMP_A);
- coda_write(dev, ctx->internal_frames[3].paddr,
+ coda_write(dev, ctx->internal_frames[3].buf.paddr,
CODA9_CMD_SET_FRAME_SUBSAMP_B);
}
}
@@ -1993,7 +1993,7 @@ static int coda_prepare_decode(struct coda_ctx *ctx)
ctx->display_idx < ctx->num_internal_frames) {
vdoa_device_run(ctx->vdoa,
vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0),
- ctx->internal_frames[ctx->display_idx].paddr);
+
ctx->internal_frames[ctx->display_idx].buf.paddr);
} else {
if (dev->devtype->product == CODA_960) {
/*
@@ -2091,6 +2091,7 @@ static void coda_finish_decode(struct coda_ctx *ctx)
int width, height;
int decoded_idx;
int display_idx;
+ struct coda_internal_frame *decoded_frame = NULL;
u32 src_fourcc;
int success;
u32 err_mb;
@@ -2216,6 +2217,8 @@ static void coda_finish_decode(struct coda_ctx *ctx)
v4l2_err(&dev->v4l2_dev,
"decoded frame index out of range: %d\n", decoded_idx);
} else {
+ decoded_frame = &ctx->internal_frames[decoded_idx];
+
val = coda_read(dev, CODA_RET_DEC_PIC_FRAME_NUM);
if (ctx->sequence_offset == -1)
ctx->sequence_offset = val;
@@ -2240,28 +2243,25 @@ static void coda_finish_decode(struct coda_ctx *ctx)
val, ctx->sequence_offset,
meta->sequence);
}
- ctx->frame_metas[decoded_idx] = *meta;
+ decoded_frame->meta = *meta;
kfree(meta);
} else {
spin_unlock(&ctx->buffer_meta_lock);
v4l2_err(&dev->v4l2_dev, "empty timestamp list!\n");
- memset(&ctx->frame_metas[decoded_idx], 0,
+ memset(&decoded_frame->meta, 0,
sizeof(struct coda_buffer_meta));
- ctx->frame_metas[decoded_idx].sequence = val;
+ decoded_frame->meta.sequence = val;
ctx->sequence_offset++;
}
- trace_coda_dec_pic_done(ctx, &ctx->frame_metas[decoded_idx]);
+ trace_coda_dec_pic_done(ctx, &decoded_frame->meta);
[PATCH 28/28] media: coda: encoder parameter change support
Add support for dynamically changing the GOP size, bitrate, frame rate,
constant intra quantization parameter, number of intra refresh macro
blocks and slice mode parameters.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c| 83 +++
drivers/media/platform/coda/coda-common.c | 7 ++
drivers/media/platform/coda/coda.h| 7 ++
drivers/media/platform/coda/coda_regs.h | 18 +
4 files changed, 115 insertions(+)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index b59cb16f75a1..00c7bed3dd57 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -698,6 +698,79 @@ static u32 coda_slice_mode(struct coda_ctx *ctx)
((1 & CODA_SLICING_MODE_MASK) << CODA_SLICING_MODE_OFFSET);
}
+static int coda_enc_param_change(struct coda_ctx *ctx)
+{
+ struct coda_dev *dev = ctx->dev;
+ u32 change_enable = 0;
+ u32 success;
+ int ret;
+
+ if (ctx->params.gop_size_changed) {
+ change_enable |= CODA_PARAM_CHANGE_RC_GOP;
+ coda_write(dev, ctx->params.gop_size,
+ CODA_CMD_ENC_PARAM_RC_GOP);
+ ctx->gopcounter = ctx->params.gop_size - 1;
+ ctx->params.gop_size_changed = false;
+ }
+ if (ctx->params.h264_intra_qp_changed) {
+ coda_dbg(1, ctx, "parameter change: intra Qp %u\n",
+ctx->params.h264_intra_qp);
+
+ if (ctx->params.bitrate) {
+ change_enable |= CODA_PARAM_CHANGE_RC_INTRA_QP;
+ coda_write(dev, ctx->params.h264_intra_qp,
+ CODA_CMD_ENC_PARAM_RC_INTRA_QP);
+ }
+ ctx->params.h264_intra_qp_changed = false;
+ }
+ if (ctx->params.bitrate_changed) {
+ coda_dbg(1, ctx, "parameter change: bitrate %u kbit/s\n",
+ctx->params.bitrate);
+ change_enable |= CODA_PARAM_CHANGE_RC_BITRATE;
+ coda_write(dev, ctx->params.bitrate,
+ CODA_CMD_ENC_PARAM_RC_BITRATE);
+ ctx->params.bitrate_changed = false;
+ }
+ if (ctx->params.framerate_changed) {
+ coda_dbg(1, ctx, "parameter change: frame rate %u/%u Hz\n",
+ctx->params.framerate & 0x,
+(ctx->params.framerate >> 16) + 1);
+ change_enable |= CODA_PARAM_CHANGE_RC_FRAME_RATE;
+ coda_write(dev, ctx->params.framerate,
+ CODA_CMD_ENC_PARAM_RC_FRAME_RATE);
+ ctx->params.framerate_changed = false;
+ }
+ if (ctx->params.intra_refresh_changed) {
+ coda_dbg(1, ctx, "parameter change: intra refresh MBs %u\n",
+ctx->params.intra_refresh);
+ change_enable |= CODA_PARAM_CHANGE_INTRA_MB_NUM;
+ coda_write(dev, ctx->params.intra_refresh,
+ CODA_CMD_ENC_PARAM_INTRA_MB_NUM);
+ ctx->params.intra_refresh_changed = false;
+ }
+ if (ctx->params.slice_mode_changed) {
+ change_enable |= CODA_PARAM_CHANGE_SLICE_MODE;
+ coda_write(dev, coda_slice_mode(ctx),
+ CODA_CMD_ENC_PARAM_SLICE_MODE);
+ ctx->params.slice_mode_changed = false;
+ }
+
+ if (!change_enable)
+ return 0;
+
+ coda_write(dev, change_enable, CODA_CMD_ENC_PARAM_CHANGE_ENABLE);
+
+ ret = coda_command_sync(ctx, CODA_COMMAND_RC_CHANGE_PARAMETER);
+ if (ret < 0)
+ return ret;
+
+ success = coda_read(dev, CODA_RET_ENC_PARAM_CHANGE_SUCCESS);
+ if (success != 1)
+ coda_dbg(1, ctx, "parameter change failed: %u\n", success);
+
+ return 0;
+}
+
static phys_addr_t coda_iram_alloc(struct coda_iram_info *iram, size_t size)
{
phys_addr_t ret;
@@ -1143,6 +1216,9 @@ static int coda_start_encoding(struct coda_ctx *ctx)
}
if (ctx->params.bitrate) {
+ ctx->params.bitrate_changed = false;
+ ctx->params.h264_intra_qp_changed = false;
+
/* Rate control enabled */
value = (ctx->params.bitrate & CODA_RATECONTROL_BITRATE_MASK)
<< CODA_RATECONTROL_BITRATE_OFFSET;
@@ -1397,6 +1473,13 @@ static int coda_prepare_encode(struct coda_ctx *ctx)
u32 rot_mode = 0;
u32 dst_fourcc;
u32 reg;
+ int ret;
+
+ ret = coda_enc_param_change(ctx);
+ if (ret < 0) {
+ v4l2_warn(&ctx->dev->v4l2_dev, "parameter change failed: %d\n",
+ ret);
+ }
[PATCH 20/28] media: coda: retire coda_buf_is_end_of_stream
Using the output queue sequence counter to determine the last buffer to
be encoded or decoded always was fragile at best. Now that we have the
last buffer flag propagating from the output queue to the capture queue
correctly, this is not needed anymore.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index c55124e8b4c8..9349df773077 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -900,13 +900,6 @@ static int coda_dqbuf(struct file *file, void *priv,
struct v4l2_buffer *buf)
return ret;
}
-static bool coda_buf_is_end_of_stream(struct coda_ctx *ctx,
- struct vb2_v4l2_buffer *buf)
-{
- return ((ctx->bit_stream_param & CODA_BIT_STREAM_END_FLAG) &&
- (buf->sequence == (ctx->qsequence - 1)));
-}
-
void coda_m2m_buf_done(struct coda_ctx *ctx, struct vb2_v4l2_buffer *buf,
enum vb2_buffer_state state)
{
@@ -914,11 +907,8 @@ void coda_m2m_buf_done(struct coda_ctx *ctx, struct
vb2_v4l2_buffer *buf,
.type = V4L2_EVENT_EOS
};
- if (coda_buf_is_end_of_stream(ctx, buf)) {
- buf->flags |= V4L2_BUF_FLAG_LAST;
-
+ if (buf->flags & V4L2_BUF_FLAG_LAST)
v4l2_event_queue_fh(&ctx->fh, &eos_event);
- }
v4l2_m2m_buf_done(buf, state);
}
--
2.20.1
[PATCH 09/28] media: coda: implement decoder source change event
The stateful decoder API requires decoders to signal detection
of stream dimensions via the V4L2_EVENT_SOURCE_CHANGE event.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index f8cfafa2ce42..b30945fa0a70 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1255,9 +1255,16 @@ static int coda_s_parm(struct file *file, void *fh,
struct v4l2_streamparm *a)
static int coda_subscribe_event(struct v4l2_fh *fh,
const struct v4l2_event_subscription *sub)
{
+ struct coda_ctx *ctx = fh_to_ctx(fh);
+
switch (sub->type) {
case V4L2_EVENT_EOS:
return v4l2_event_subscribe(fh, sub, 0, NULL);
+ case V4L2_EVENT_SOURCE_CHANGE:
+ if (ctx->inst_type == CODA_INST_DECODER)
+ return v4l2_event_subscribe(fh, sub, 0, NULL);
+ else
+ return -EINVAL;
default:
return v4l2_ctrl_subscribe_event(fh, sub);
}
@@ -1642,6 +1649,16 @@ void coda_update_profile_level_ctrls(struct coda_ctx
*ctx, u8 profile_idc,
}
}
+static void coda_queue_source_change_event(struct coda_ctx *ctx)
+{
+ static const struct v4l2_event source_change_event = {
+ .type = V4L2_EVENT_SOURCE_CHANGE,
+ .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION,
+ };
+
+ v4l2_event_queue_fh(&ctx->fh, &source_change_event);
+}
+
static void coda_buf_queue(struct vb2_buffer *vb)
{
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -1696,6 +1713,9 @@ static void coda_buf_queue(struct vb2_buffer *vb)
&ctx->seq_init_work);
flush_work(&ctx->seq_init_work);
}
+
+ if (ctx->initialized)
+ coda_queue_source_change_event(ctx);
}
} else {
if (ctx->inst_type == CODA_INST_ENCODER &&
--
2.20.1
[PATCH 21/28] media: coda: only wake up capture queue if no pending buffers to encode
If there are no pending queued output buffers to be encoded, waking up
the capture queue with -EPIPE signals end of stream. If there are
pending buffers on the other hand, setting the V4L2_BUF_FLAG_LAST on
the resulting encoded capture buffers is all that is needed.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 9349df773077..0d64ddc49494 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1035,17 +1035,18 @@ static int coda_encoder_cmd(struct file *file, void *fh,
return ret;
buf = v4l2_m2m_last_src_buf(ctx->fh.m2m_ctx);
- if (buf)
+ if (buf) {
buf->flags |= V4L2_BUF_FLAG_LAST;
+ } else {
+ /* Set the stream-end flag on this context */
+ ctx->bit_stream_param |= CODA_BIT_STREAM_END_FLAG;
- /* Set the stream-end flag on this context */
- ctx->bit_stream_param |= CODA_BIT_STREAM_END_FLAG;
-
- flush_work(&ctx->pic_run_work);
+ flush_work(&ctx->pic_run_work);
- /* If there is no buffer in flight, wake up */
- if (!ctx->streamon_out || ctx->qsequence == ctx->osequence)
- coda_wake_up_capture_queue(ctx);
+ /* If there is no buffer in flight, wake up */
+ if (!ctx->streamon_out || ctx->qsequence == ctx->osequence)
+ coda_wake_up_capture_queue(ctx);
+ }
return 0;
}
--
2.20.1
[PATCH 00/28] media: coda: fixes and improvements
Hi, this series contains a few fixes for MPEG-2 sequence number counting, decoder/encoder stop command race conditions and some more work to further move towards V4L2 stateful decoder API compliance: Sequence initialization, which lets the firmware parse the bitstream headers, is reworked to run before the capture queue is streaming, with some padding acrobatics to work around issues when only headers or very small buffers are queued initially. A SOURCE_CHANGE event is issued after sequence initialization completes. The sequence counting mechanism used to determine the last processed buffer is replaced with a last buffer flag that is set on the last queued output buffer and carried over to the corresponding bitstream metadata, to the decoded internal tiled frame, and finally to the returned linear capture buffer. Dynamic parameter change support is added for encoder rate control or quantization parameters, slice mode, and cyclic intra refresh interval. Inbetween there are a few code cleanup patches that introduce, change, or make use of helper functions, and improve driver structures. regards Philipp Marco Felsch (2): media: coda: fix last buffer handling in V4L2_ENC_CMD_STOP media: coda: fix V4L2_DEC_CMD_STOP when all buffers are already consumed Michael Tretter (1): media: coda: implement CMD_START to restart decoding Philipp Zabel (25): media: coda: use mem2mem try_en/decoder_cmd helpers media: coda: fix mpeg2 sequence number handling media: coda: add coda_wake_up_capture_queue media: coda: split decoder sequence initialization out of start decoding media: coda: add sequence initialization work media: coda: implement decoder source change event media: coda: integrate internal frame metadata into a structure media: coda: make coda_bitstream_queue more versatile media: coda: pad first buffer with repeated MPEG headers to fix sequence init media: coda: do not enforce 512-byte initial bitstream payload on CODA960 media: coda: flush bitstream ring buffer on decoder restart media: coda: increment sequence offset for the last returned frame media: coda: allow flagging last output buffer internally media: coda: mark the last output buffer on decoder stop command media: coda: only set the stream end flags if there are no more pending output buffers media: coda: mark the last output buffer on encoder stop command media: coda: retire coda_buf_is_end_of_stream media: coda: only wake up capture queue if no pending buffers to encode media: coda: flag the last encoded buffer media: coda: lock capture queue wakeup against encoder stop command media: coda: mark last pending buffer or last meta on decoder stop command media: coda: mark last returned frame media: coda: store device pointer in driver structure instead of pdev media: coda: add coda_slice_mode() function media: coda: encoder parameter change support drivers/media/platform/coda/coda-bit.c| 403 +- drivers/media/platform/coda/coda-common.c | 244 ++--- drivers/media/platform/coda/coda-mpeg2.c | 43 +++ drivers/media/platform/coda/coda-mpeg4.c | 38 ++ drivers/media/platform/coda/coda.h| 33 +- drivers/media/platform/coda/coda_regs.h | 18 + 6 files changed, 637 insertions(+), 142 deletions(-) -- 2.20.1
[PATCH 27/28] media: coda: add coda_slice_mode() function
Changing slice mode dynamically while encoding will require to calculate
the register value again, so split it out into a separate function.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 45 ++
1 file changed, 24 insertions(+), 21 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index de6a4216a182..b59cb16f75a1 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -675,6 +675,29 @@ static int coda_encode_header(struct coda_ctx *ctx, struct
vb2_v4l2_buffer *buf,
return 0;
}
+static u32 coda_slice_mode(struct coda_ctx *ctx)
+{
+ int size, unit;
+
+ switch (ctx->params.slice_mode) {
+ case V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_SINGLE:
+ default:
+ return 0;
+ case V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_MAX_MB:
+ size = ctx->params.slice_max_mb;
+ unit = 1;
+ break;
+ case V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_MAX_BYTES:
+ size = ctx->params.slice_max_bits;
+ unit = 0;
+ break;
+ }
+
+ return ((size & CODA_SLICING_SIZE_MASK) << CODA_SLICING_SIZE_OFFSET) |
+ ((unit & CODA_SLICING_UNIT_MASK) << CODA_SLICING_UNIT_OFFSET) |
+ ((1 & CODA_SLICING_MODE_MASK) << CODA_SLICING_MODE_OFFSET);
+}
+
static phys_addr_t coda_iram_alloc(struct coda_iram_info *iram, size_t size)
{
phys_addr_t ret;
@@ -1113,27 +1136,7 @@ static int coda_start_encoding(struct coda_ctx *ctx)
* in JPEG mode
*/
if (dst_fourcc != V4L2_PIX_FMT_JPEG) {
- switch (ctx->params.slice_mode) {
- case V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_SINGLE:
- value = 0;
- break;
- case V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_MAX_MB:
- value = (ctx->params.slice_max_mb &
- CODA_SLICING_SIZE_MASK)
-<< CODA_SLICING_SIZE_OFFSET;
- value |= (1 & CODA_SLICING_UNIT_MASK)
-<< CODA_SLICING_UNIT_OFFSET;
- value |= 1 & CODA_SLICING_MODE_MASK;
- break;
- case V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_MAX_BYTES:
- value = (ctx->params.slice_max_bits &
- CODA_SLICING_SIZE_MASK)
-<< CODA_SLICING_SIZE_OFFSET;
- value |= (0 & CODA_SLICING_UNIT_MASK)
-<< CODA_SLICING_UNIT_OFFSET;
- value |= 1 & CODA_SLICING_MODE_MASK;
- break;
- }
+ value = coda_slice_mode(ctx);
coda_write(dev, value, CODA_CMD_ENC_SEQ_SLICE_MODE);
value = ctx->params.gop_size;
coda_write(dev, value, CODA_CMD_ENC_SEQ_GOP_SIZE);
--
2.20.1
[PATCH 26/28] media: coda: store device pointer in driver structure instead of pdev
From: Philipp Zabel
Currently the platform device pointer is stored in struct coda_dev,
only to convert it into a device pointer wherever it is used. Just
store the device pointer directly.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c| 7 +++--
drivers/media/platform/coda/coda-common.c | 33 +++
drivers/media/platform/coda/coda.h| 2 +-
3 files changed, 19 insertions(+), 23 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 167a92772c84..de6a4216a182 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -1667,8 +1667,7 @@ static int coda_alloc_bitstream_buffer(struct coda_ctx
*ctx,
return 0;
ctx->bitstream.size = roundup_pow_of_two(q_data->sizeimage * 2);
- ctx->bitstream.vaddr = dma_alloc_wc(&ctx->dev->plat_dev->dev,
- ctx->bitstream.size,
+ ctx->bitstream.vaddr = dma_alloc_wc(ctx->dev->dev, ctx->bitstream.size,
&ctx->bitstream.paddr, GFP_KERNEL);
if (!ctx->bitstream.vaddr) {
v4l2_err(&ctx->dev->v4l2_dev,
@@ -1686,8 +1685,8 @@ static void coda_free_bitstream_buffer(struct coda_ctx
*ctx)
if (ctx->bitstream.vaddr == NULL)
return;
- dma_free_wc(&ctx->dev->plat_dev->dev, ctx->bitstream.size,
- ctx->bitstream.vaddr, ctx->bitstream.paddr);
+ dma_free_wc(ctx->dev->dev, ctx->bitstream.size, ctx->bitstream.vaddr,
+ ctx->bitstream.paddr);
ctx->bitstream.vaddr = NULL;
kfifo_init(&ctx->bitstream_fifo, NULL, 0);
}
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index cd33c260409e..8060a3425325 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1413,7 +1413,7 @@ static void coda_pic_run_work(struct work_struct *work)
if (!wait_for_completion_timeout(&ctx->completion,
msecs_to_jiffies(1000))) {
- dev_err(&dev->plat_dev->dev, "CODA PIC_RUN timeout\n");
+ dev_err(dev->dev, "CODA PIC_RUN timeout\n");
ctx->hold = true;
@@ -1797,7 +1797,7 @@ static void coda_buf_queue(struct vb2_buffer *vb)
int coda_alloc_aux_buf(struct coda_dev *dev, struct coda_aux_buf *buf,
size_t size, const char *name, struct dentry *parent)
{
- buf->vaddr = dma_alloc_coherent(&dev->plat_dev->dev, size, &buf->paddr,
+ buf->vaddr = dma_alloc_coherent(dev->dev, size, &buf->paddr,
GFP_KERNEL);
if (!buf->vaddr) {
v4l2_err(&dev->v4l2_dev,
@@ -1814,7 +1814,7 @@ int coda_alloc_aux_buf(struct coda_dev *dev, struct
coda_aux_buf *buf,
buf->dentry = debugfs_create_blob(name, 0644, parent,
&buf->blob);
if (!buf->dentry)
- dev_warn(&dev->plat_dev->dev,
+ dev_warn(dev->dev,
"failed to create debugfs entry %s\n", name);
}
@@ -1825,8 +1825,7 @@ void coda_free_aux_buf(struct coda_dev *dev,
struct coda_aux_buf *buf)
{
if (buf->vaddr) {
- dma_free_coherent(&dev->plat_dev->dev, buf->size,
- buf->vaddr, buf->paddr);
+ dma_free_coherent(dev->dev, buf->size, buf->vaddr, buf->paddr);
buf->vaddr = NULL;
buf->size = 0;
debugfs_remove(buf->dentry);
@@ -2344,7 +2343,7 @@ static int coda_queue_init(struct coda_ctx *ctx, struct
vb2_queue *vq)
* queues to have at least one buffer queued.
*/
vq->min_buffers_needed = 1;
- vq->dev = &ctx->dev->plat_dev->dev;
+ vq->dev = ctx->dev->dev;
return vb2_queue_init(vq);
}
@@ -2469,7 +2468,7 @@ static int coda_open(struct file *file)
ctx->use_vdoa = false;
/* Power up and upload firmware if necessary */
- ret = pm_runtime_get_sync(&dev->plat_dev->dev);
+ ret = pm_runtime_get_sync(dev->dev);
if (ret < 0) {
v4l2_err(&dev->v4l2_dev, "failed to power up: %d\n", ret);
goto err_pm_get;
@@ -2517,7 +2516,7 @@ static int coda_open(struct file *file)
err_clk_ahb:
clk_disable_unprepare(dev->clk_per);
err_clk_per:
- pm_runtime_put_sync(&dev->plat_dev->dev);
+
[PATCH 06/28] media: coda: fix V4L2_DEC_CMD_STOP when all buffers are already consumed
From: Marco Felsch When the DEC_CMD_STOP command is issued after the context has already consumed all the queued buffers, we need to make sure to wake the destination queue with last_buffer_dequeued set, to allow userspace to make progress in its EOS handling. As there might still be picture run workers pending at that point, we need to synchronize with them, so the sequence number comparison reads stable values. Signed-off-by: Marco Felsch [l.st...@pengutronix.de: rewrite to fix multi-context use-cases, reword commit message] Signed-off-by: Lucas Stach Signed-off-by: Philipp Zabel --- drivers/media/platform/coda/coda-common.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c index 7dbeb80d40f4..232bda4b7016 100644 --- a/drivers/media/platform/coda/coda-common.c +++ b/drivers/media/platform/coda/coda-common.c @@ -1071,6 +1071,12 @@ static int coda_decoder_cmd(struct file *file, void *fh, coda_bit_stream_end_flag(ctx); ctx->hold = false; v4l2_m2m_try_schedule(ctx->fh.m2m_ctx); + + flush_work(&ctx->pic_run_work); + + /* If there is no buffer in flight, wake up */ + if (!ctx->streamon_out || ctx->qsequence == ctx->osequence) + coda_wake_up_capture_queue(ctx); break; default: return -EINVAL; -- 2.20.1
[PATCH 18/28] media: coda: only set the stream end flags if there are no more pending output buffers
If there are still queued output buffers pending to be copied into the
bitstream ring buffer, setting the stream end flag should be deferred
until the marked last output buffer is written into the bitstream ring
buffer.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-common.c | 20
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 9b32b5862aa8..4002a5b8c9ea 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1098,16 +1098,20 @@ static int coda_decoder_cmd(struct file *file, void *fh,
/* Mark last buffer */
buf->flags |= V4L2_BUF_FLAG_LAST;
- /* Set the stream-end flag on this context */
- coda_bit_stream_end_flag(ctx);
- ctx->hold = false;
- v4l2_m2m_try_schedule(ctx->fh.m2m_ctx);
+ if (v4l2_m2m_num_src_bufs_ready(ctx->fh.m2m_ctx) == 0) {
+ /* Set the stream-end flag on this context */
+ coda_bit_stream_end_flag(ctx);
+ ctx->hold = false;
+ v4l2_m2m_try_schedule(ctx->fh.m2m_ctx);
- flush_work(&ctx->pic_run_work);
+ flush_work(&ctx->pic_run_work);
+
+ /* If there is no buffer in flight, wake up */
+ if (!ctx->streamon_out ||
+ ctx->qsequence == ctx->osequence)
+ coda_wake_up_capture_queue(ctx);
+ }
- /* If there is no buffer in flight, wake up */
- if (!ctx->streamon_out || ctx->qsequence == ctx->osequence)
- coda_wake_up_capture_queue(ctx);
break;
default:
return -EINVAL;
--
2.20.1
[PATCH 11/28] media: coda: make coda_bitstream_queue more versatile
Pass vaddr and size to coda_bitstream_queue instead of a struct
vb2_v4l2_buffer to make it reusable for queueing data that is
not exactly a whole v4l2 buffer into the bitstream ringbuffer.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c | 23 +--
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 494bc130c7af..4f96f808b4dd 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -199,33 +199,25 @@ static int coda_bitstream_pad(struct coda_ctx *ctx, u32
size)
return (n < size) ? -ENOSPC : 0;
}
-static int coda_bitstream_queue(struct coda_ctx *ctx,
- struct vb2_v4l2_buffer *src_buf)
+static int coda_bitstream_queue(struct coda_ctx *ctx, const u8 *buf, u32 size)
{
- u32 src_size = vb2_get_plane_payload(&src_buf->vb2_buf, 0);
- u32 n;
-
- n = kfifo_in(&ctx->bitstream_fifo,
- vb2_plane_vaddr(&src_buf->vb2_buf, 0), src_size);
- if (n < src_size)
- return -ENOSPC;
+ u32 n = kfifo_in(&ctx->bitstream_fifo, buf, size);
- src_buf->sequence = ctx->qsequence++;
-
- return 0;
+ return (n < size) ? -ENOSPC : 0;
}
static bool coda_bitstream_try_queue(struct coda_ctx *ctx,
struct vb2_v4l2_buffer *src_buf)
{
unsigned long payload = vb2_get_plane_payload(&src_buf->vb2_buf, 0);
+ u8 *vaddr = vb2_plane_vaddr(&src_buf->vb2_buf, 0);
int ret;
if (coda_get_bitstream_payload(ctx) + payload + 512 >=
ctx->bitstream.size)
return false;
- if (vb2_plane_vaddr(&src_buf->vb2_buf, 0) == NULL) {
+ if (!vaddr) {
v4l2_err(&ctx->dev->v4l2_dev, "trying to queue empty buffer\n");
return true;
}
@@ -235,11 +227,14 @@ static bool coda_bitstream_try_queue(struct coda_ctx *ctx,
ctx->codec->src_fourcc == V4L2_PIX_FMT_H264)
coda_bitstream_pad(ctx, 512 - payload);
- ret = coda_bitstream_queue(ctx, src_buf);
+ ret = coda_bitstream_queue(ctx, vaddr, payload);
if (ret < 0) {
v4l2_err(&ctx->dev->v4l2_dev, "bitstream buffer overflow\n");
return false;
}
+
+ src_buf->sequence = ctx->qsequence++;
+
/* Sync read pointer to device */
if (ctx == v4l2_m2m_get_curr_priv(ctx->dev->m2m_dev))
coda_kfifo_sync_to_device_write(ctx);
--
2.20.1
[PATCH 24/28] media: coda: mark last pending buffer or last meta on decoder stop command
If there is still a buffer pending, mark it as the last buffer. It will
create a meta that is flagged as last when the buffer is copied into the
bitstream ring buffer. If there are no more buffers pending, find the
last bitstream meta and mark it as last. If there is no bitstream meta
either, wake up the capture queue as there will be no more decoded
frames.
Signed-off-by: Philipp Zabel
---
drivers/media/platform/coda/coda-bit.c| 4 +++
drivers/media/platform/coda/coda-common.c | 44 +++
drivers/media/platform/coda/coda.h| 1 +
3 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/coda/coda-bit.c
b/drivers/media/platform/coda/coda-bit.c
index 44085e3d43d5..1157454e3bc8 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -402,6 +402,9 @@ void coda_fill_bitstream(struct coda_ctx *ctx, struct
list_head *buffer_list)
meta->timestamp = src_buf->vb2_buf.timestamp;
meta->start = start;
meta->end = ctx->bitstream_fifo.kfifo.in;
+ meta->last = src_buf->flags &
V4L2_BUF_FLAG_LAST;
+ if (meta->last)
+ coda_dbg(1, ctx, "marking last meta");
spin_lock(&ctx->buffer_meta_lock);
list_add_tail(&meta->list,
&ctx->buffer_meta_list);
@@ -2334,6 +2337,7 @@ static void coda_finish_decode(struct coda_ctx *ctx)
memset(&decoded_frame->meta, 0,
sizeof(struct coda_buffer_meta));
decoded_frame->meta.sequence = val;
+ decoded_frame->meta.last = false;
ctx->sequence_offset++;
}
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 829d1e565911..cd33c260409e 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1077,6 +1077,8 @@ static int coda_decoder_cmd(struct file *file, void *fh,
struct coda_dev *dev = ctx->dev;
struct vb2_v4l2_buffer *buf;
struct vb2_queue *dst_vq;
+ bool stream_end;
+ bool wakeup;
int ret;
ret = coda_try_decoder_cmd(file, fh, dc);
@@ -1097,23 +1099,51 @@ static int coda_decoder_cmd(struct file *file, void *fh,
mutex_unlock(&ctx->bitstream_mutex);
break;
case V4L2_DEC_CMD_STOP:
+ stream_end = false;
+ wakeup = false;
+
buf = v4l2_m2m_last_src_buf(ctx->fh.m2m_ctx);
- if (buf)
+ if (buf) {
+ coda_dbg(1, ctx, "marking last pending buffer\n");
+
/* Mark last buffer */
buf->flags |= V4L2_BUF_FLAG_LAST;
- if (v4l2_m2m_num_src_bufs_ready(ctx->fh.m2m_ctx) == 0) {
+ if (v4l2_m2m_num_src_bufs_ready(ctx->fh.m2m_ctx) == 0) {
+ coda_dbg(1, ctx, "all remaining buffers
queued\n");
+ stream_end = true;
+ }
+ } else {
+ coda_dbg(1, ctx, "marking last meta\n");
+
+ /* Mark last meta */
+ spin_lock(&ctx->buffer_meta_lock);
+ if (!list_empty(&ctx->buffer_meta_list)) {
+ struct coda_buffer_meta *meta;
+
+ meta = list_last_entry(&ctx->buffer_meta_list,
+ struct coda_buffer_meta,
+ list);
+ meta->last = true;
+ stream_end = true;
+ } else {
+ wakeup = true;
+ }
+ spin_unlock(&ctx->buffer_meta_lock);
+ }
+
+ if (stream_end) {
+ coda_dbg(1, ctx, "all remaining buffers queued\n");
+
/* Set the stream-end flag on this context */
coda_bit_stream_end_flag(ctx);
ctx->hold = false;
v4l2_m2m_try_schedule(ctx->fh.m2m_ctx);
+ }
- flush_work(&ctx->pic_run_work);
-
+ if (wakeup) {
/* If there is no buffer in flight, wake up */
- if (!ctx->streamon_out ||
- ctx->qsequence =
[PATCH 02/28] media: coda: use mem2mem try_en/decoder_cmd helpers
Use mem2mem try_en/decoder_cmd helpers to ensure consistent behaviour with other video codec drivers. Signed-off-by: Philipp Zabel --- drivers/media/platform/coda/coda-common.c | 19 ++- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c index a5e0d5c1528e..e09d48170e5c 100644 --- a/drivers/media/platform/coda/coda-common.c +++ b/drivers/media/platform/coda/coda-common.c @@ -1001,13 +1001,7 @@ static int coda_try_encoder_cmd(struct file *file, void *fh, if (ctx->inst_type != CODA_INST_ENCODER) return -ENOTTY; - if (ec->cmd != V4L2_ENC_CMD_STOP) - return -EINVAL; - - if (ec->flags & V4L2_ENC_CMD_STOP_AT_GOP_END) - return -EINVAL; - - return 0; + return v4l2_m2m_ioctl_try_encoder_cmd(file, fh, ec); } static int coda_encoder_cmd(struct file *file, void *fh, @@ -1043,16 +1037,7 @@ static int coda_try_decoder_cmd(struct file *file, void *fh, if (ctx->inst_type != CODA_INST_DECODER) return -ENOTTY; - if (dc->cmd != V4L2_DEC_CMD_STOP) - return -EINVAL; - - if (dc->flags & V4L2_DEC_CMD_STOP_TO_BLACK) - return -EINVAL; - - if (!(dc->flags & V4L2_DEC_CMD_STOP_IMMEDIATELY) && (dc->stop.pts != 0)) - return -EINVAL; - - return 0; + return v4l2_m2m_ioctl_try_decoder_cmd(file, fh, dc); } static int coda_decoder_cmd(struct file *file, void *fh, -- 2.20.1
Re: [PATCH v8] media: imx: add mem2mem device
Hi Steve, On Tue, 2019-06-11 at 18:08 -0700, Steve Longerbeam wrote: > Hi Philipp, > > Version 9 will also need to fix merge conflicts due to the recent module > re-org and the switch to sync registration for the IPU internal subdevs. > > I've done that work already, feel free to cherry-pick it from my github > fork if you agree with the merge fixes: > > g...@github.com:slongerbeam/mediatree.git, branch imx/mem2mem.v8. > > Btw, some bugs have been found and fixed in ipu-image-convert.c. I will > be posting a patch-set shortly. You can review branch imx/bgthree-2136 > in my fork for the changes. Thank you, I'll resend the mem2mem next week. I had rebased already, but I haven't tested unbinding/rebinding yet, as imx-media exploded on me even without the mem2mem patch when I try tried. regards Philipp
Re: [PATCH v4 00/10] Rename Rockchip VPU driver to Hantro, add initial i.MX8M support
On Wed, 2019-06-12 at 10:00 +0200, Hans Verkuil wrote:
> On 6/12/19 9:55 AM, Hans Verkuil wrote:
> > On 6/11/19 2:50 PM, Philipp Zabel wrote:
> > > There are several other SoCs that contain Hantro IP based VPUs, such as
> > > NXP i.MX8MQ (Hantro G1 and G2) and i.MX8MM (Hantro G1, G2, and H1). To
> > > maximize code sharing, add initial support for these SoCs to the
> > > Rockchip VPU driver, after renaming it to Hantro VPU.
> > >
> > > This series is based on the br-v5.3g tag, commit e568d2cc1ef6
> > > ("rockchip/vpu: Add support for MPEG-2 decoding on RK3288") with
> > > https://patchwork.linuxtv.org/patch/56402/ ("rockchip/vpu: Add support
> > > for MPEG-2 decoding on RK3328") applied on top. It supports MPEG-2
> > > decoding on i.MX8MQ. MPEG-2 decoding and JPEG encoding on i.MX8MM may
> > > or may not work, I don't have the hardware to test.
> > >
> > > Changes since v3:
> > > - Split rk3288_vpu_regs.h into hantro_g1_regs.h and hantro_h1_regs.h,
> > >rename VDPU register defines to G1 and VEPU register defines to H1.
> > > - Make Rockchip / i.MX8M support configurable.
> > > - Keep staging/media Kconfig in alphabetic order.
> > > - Rename bases to reg_bases
> > > - Move dynamic clocks before i.MX8M support.
> >
> > Can you please rebase this on top of the media_tree master? This series
> > doesn't apply there.
> >
> > I'd like to get this rename series in asap since keeping this out-of-tree
> > is a pain.
> >
> > It looks in good shape in general.
>
> In fact, I would like to hold off on applying any other rockchip patches
> until this is in (and that means any pending rockchip patches probably
> need rebasing).
>
> So if a v5 applies cleanly and doesn't throw up new sparse/smatch issues,
> then I'll take it.
I've rebased onto media-tree master, checked sparse/smatch, and resent.
The RK3328 patch is pulled out from under it and sent separately on top
of the series.
regards
Philipp
[PATCH v7] media: hantro: Add support for MPEG-2 decoding on RK3328
From: Jonas Karlman
Add necessary bits to support MPEG2 decoding on RK3328.
Signed-off-by: Jonas Karlman
Signed-off-by: Ezequiel Garcia
Signed-off-by: Philipp Zabel
---
Changes since v6 [1]:
- Rebased onto Hantro rename series [2]
[1] https://patchwork.linuxtv.org/patch/56402/
[2] https://patchwork.linuxtv.org/cover/56825/
---
drivers/staging/media/hantro/Kconfig | 2 +-
drivers/staging/media/hantro/hantro_drv.c| 1 +
drivers/staging/media/hantro/rk3399_vpu_hw.c | 17 +
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/hantro/Kconfig
b/drivers/staging/media/hantro/Kconfig
index e0627ce454fc..99aed9a5b0b9 100644
--- a/drivers/staging/media/hantro/Kconfig
+++ b/drivers/staging/media/hantro/Kconfig
@@ -28,4 +28,4 @@ config VIDEO_HANTRO_ROCKCHIP
depends on ARCH_ROCKCHIP || COMPILE_TEST
default y
help
- Enable support for RK3288 and RK3399 SoCs.
+ Enable support for RK3288, RK3328, and RK3399 SoCs.
diff --git a/drivers/staging/media/hantro/hantro_drv.c
b/drivers/staging/media/hantro/hantro_drv.c
index af26672b4538..1187a6ca68be 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -417,6 +417,7 @@ static const struct v4l2_file_operations hantro_fops = {
static const struct of_device_id of_hantro_match[] = {
#ifdef CONFIG_VIDEO_HANTRO_ROCKCHIP
{ .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, },
+ { .compatible = "rockchip,rk3328-vpu", .data = &rk3328_vpu_variant, },
{ .compatible = "rockchip,rk3288-vpu", .data = &rk3288_vpu_variant, },
#endif
#ifdef CONFIG_VIDEO_HANTRO_IMX8M
diff --git a/drivers/staging/media/hantro/rk3399_vpu_hw.c
b/drivers/staging/media/hantro/rk3399_vpu_hw.c
index 5718f8063542..f8400e49bc50 100644
--- a/drivers/staging/media/hantro/rk3399_vpu_hw.c
+++ b/drivers/staging/media/hantro/rk3399_vpu_hw.c
@@ -184,3 +184,20 @@ const struct hantro_variant rk3399_vpu_variant = {
.clk_names = rk3399_clk_names,
.num_clocks = ARRAY_SIZE(rk3399_clk_names)
};
+
+static const struct hantro_irq rk3328_irqs[] = {
+ { "vdpu", rk3399_vdpu_irq },
+};
+
+const struct hantro_variant rk3328_vpu_variant = {
+ .dec_offset = 0x400,
+ .dec_fmts = rk3399_vpu_dec_fmts,
+ .num_dec_fmts = ARRAY_SIZE(rk3399_vpu_dec_fmts),
+ .codec = HANTRO_MPEG2_DECODER,
+ .codec_ops = rk3399_vpu_codec_ops,
+ .irqs = rk3328_irqs,
+ .num_irqs = ARRAY_SIZE(rk3328_irqs),
+ .init = rk3399_vpu_hw_init,
+ .clk_names = rk3399_clk_names,
+ .num_clocks = ARRAY_SIZE(rk3399_clk_names),
+};
--
2.20.1
[PATCH v5 06/10] media: hantro: add support for separate control block
On i.MX8MQ/MM a separate control block contains registers for per-core
resets, clock gating, and fuse register control.
Signed-off-by: Philipp Zabel
Reviewed-by: Boris Brezillon
---
drivers/staging/media/hantro/hantro.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/media/hantro/hantro.h
b/drivers/staging/media/hantro/hantro.h
index e8eb747f22ef..5c2f87272ce2 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -167,6 +167,7 @@ hantro_vdev_to_func(struct video_device *vdev)
* @reg_bases: Mapped addresses of VPU registers.
* @enc_base: Mapped address of VPU encoder register for convenience.
* @dec_base: Mapped address of VPU decoder register for convenience.
+ * @ctrl_base: Mapped address of VPU control block.
* @vpu_mutex: Mutex to synchronize V4L2 calls.
* @irqlock: Spinlock to synchronize access to data structures
* shared with interrupt handlers.
@@ -185,6 +186,7 @@ struct hantro_dev {
void __iomem **reg_bases;
void __iomem *enc_base;
void __iomem *dec_base;
+ void __iomem *ctrl_base;
struct mutex vpu_mutex; /* video_device lock */
spinlock_t irqlock;
--
2.20.1
[PATCH v5 04/10] media: hantro: make irq names configurable
The i.MX8MQ bindings will use different IRQ names ("g1" instead of
"vdpu", and "g2"), so make them configurable. This also allows to
register more than two IRQs, which will be required for i.MX8MM support
later (it will add "h1" instead of "vepu").
Signed-off-by: Philipp Zabel
Reviewed-by: Boris Brezillon
---
drivers/staging/media/hantro/hantro.h| 19 ---
drivers/staging/media/hantro/hantro_drv.c| 33 +++-
drivers/staging/media/hantro/rk3288_vpu_hw.c | 9 --
drivers/staging/media/hantro/rk3399_vpu_hw.c | 9 --
4 files changed, 40 insertions(+), 30 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro.h
b/drivers/staging/media/hantro/hantro.h
index 296b9ffad547..d041d36a0805 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -44,6 +44,17 @@ struct hantro_codec_ops;
#define HANTRO_MPEG2_DECODER BIT(16)
#define HANTRO_DECODERS0x
+/**
+ * struct hantro_irq - irq handler and name
+ *
+ * @name: irq name for device tree lookup
+ * @handler: interrupt handler
+ */
+struct hantro_irq {
+ const char *name;
+ irqreturn_t (*handler)(int irq, void *priv);
+};
+
/**
* struct hantro_variant - information about VPU hardware variant
*
@@ -57,8 +68,8 @@ struct hantro_codec_ops;
* @codec_ops: Codec ops.
* @init: Initialize hardware.
* @runtime_resume:reenable hardware after power gating
- * @vepu_irq: encoder interrupt handler
- * @vdpu_irq: decoder interrupt handler
+ * @irqs: array of irq names and interrupt handlers
+ * @num_irqs: number of irqs in the array
* @clk_names: array of clock names
* @num_clocks:number of clocks in the array
*/
@@ -73,8 +84,8 @@ struct hantro_variant {
const struct hantro_codec_ops *codec_ops;
int (*init)(struct hantro_dev *vpu);
int (*runtime_resume)(struct hantro_dev *vpu);
- irqreturn_t (*vepu_irq)(int irq, void *priv);
- irqreturn_t (*vdpu_irq)(int irq, void *priv);
+ const struct hantro_irq *irqs;
+ int num_irqs;
const char *clk_names[HANTRO_MAX_CLOCKS];
int num_clocks;
};
diff --git a/drivers/staging/media/hantro/hantro_drv.c
b/drivers/staging/media/hantro/hantro_drv.c
index ed10052dc1c8..4ed39728da3d 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -706,36 +706,25 @@ static int hantro_probe(struct platform_device *pdev)
return ret;
}
- if (vpu->variant->vdpu_irq) {
+ for (i = 0; i < vpu->variant->num_irqs; i++) {
+ const char *irq_name = vpu->variant->irqs[i].name;
int irq;
- irq = platform_get_irq_byname(vpu->pdev, "vdpu");
- if (irq <= 0) {
- dev_err(vpu->dev, "Could not get vdpu IRQ.\n");
- return -ENXIO;
- }
-
- ret = devm_request_irq(vpu->dev, irq, vpu->variant->vdpu_irq,
- 0, dev_name(vpu->dev), vpu);
- if (ret) {
- dev_err(vpu->dev, "Could not request vdpu IRQ.\n");
- return ret;
- }
- }
-
- if (vpu->variant->vepu_irq) {
- int irq;
+ if (!vpu->variant->irqs[i].handler)
+ continue;
- irq = platform_get_irq_byname(vpu->pdev, "vepu");
+ irq = platform_get_irq_byname(vpu->pdev, irq_name);
if (irq <= 0) {
- dev_err(vpu->dev, "Could not get vepu IRQ.\n");
+ dev_err(vpu->dev, "Could not get %s IRQ.\n", irq_name);
return -ENXIO;
}
- ret = devm_request_irq(vpu->dev, irq, vpu->variant->vepu_irq,
- 0, dev_name(vpu->dev), vpu);
+ ret = devm_request_irq(vpu->dev, irq,
+ vpu->variant->irqs[i].handler, 0,
+ dev_name(vpu->dev), vpu);
if (ret) {
- dev_err(vpu->dev, "Could not request vepu IRQ.\n");
+ dev_err(vpu->dev, "Could not request %s IRQ.\n",
+ irq_name);
return ret;
}
}
diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c
b/drivers/staging/media/hantro/rk3288_vpu_hw.c
index f0d3f0eec07b..c5473bc1ac29 100644
--- a/drivers/staging/m
[PATCH v5 03/10] media: hantro: add PM runtime resume callback
It seems that on i.MX8MQ the power domain controller does not propagate
resets to the VPU cores on resume. Add a callback to allow implementing
manual reset of the VPU cores after ungating the power domain.
Signed-off-by: Philipp Zabel
Reviewed-by: Boris Brezillon
---
drivers/staging/media/hantro/hantro.h | 2 ++
drivers/staging/media/hantro/hantro_drv.c | 13 +
2 files changed, 15 insertions(+)
diff --git a/drivers/staging/media/hantro/hantro.h
b/drivers/staging/media/hantro/hantro.h
index 14e685428203..296b9ffad547 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -56,6 +56,7 @@ struct hantro_codec_ops;
* @codec: Supported codecs
* @codec_ops: Codec ops.
* @init: Initialize hardware.
+ * @runtime_resume:reenable hardware after power gating
* @vepu_irq: encoder interrupt handler
* @vdpu_irq: decoder interrupt handler
* @clk_names: array of clock names
@@ -71,6 +72,7 @@ struct hantro_variant {
unsigned int codec;
const struct hantro_codec_ops *codec_ops;
int (*init)(struct hantro_dev *vpu);
+ int (*runtime_resume)(struct hantro_dev *vpu);
irqreturn_t (*vepu_irq)(int irq, void *priv);
irqreturn_t (*vdpu_irq)(int irq, void *priv);
const char *clk_names[HANTRO_MAX_CLOCKS];
diff --git a/drivers/staging/media/hantro/hantro_drv.c
b/drivers/staging/media/hantro/hantro_drv.c
index 0eadcc530e63..ed10052dc1c8 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -832,9 +832,22 @@ static int hantro_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM
+static int hantro_runtime_resume(struct device *dev)
+{
+ struct hantro_dev *vpu = dev_get_drvdata(dev);
+
+ if (vpu->variant->runtime_resume)
+ return vpu->variant->runtime_resume(vpu);
+
+ return 0;
+}
+#endif
+
static const struct dev_pm_ops hantro_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
pm_runtime_force_resume)
+ SET_RUNTIME_PM_OPS(NULL, hantro_runtime_resume, NULL)
};
static struct platform_driver hantro_driver = {
--
2.20.1
[PATCH v5 07/10] media: hantro: allow arbitrary number of clocks
Dynamically allocate clocks and move clock names out of struct
hantro_variant. This lifts the four clock limit and allows to use
ARRAY_SIZE() to fill .num_clocks to reduce the risk of mismatches.
Signed-off-by: Philipp Zabel
Reviewed-by: Boris Brezillon
---
Changes since v4 [1]:
- Rebased onto media-tree master
[1] https://patchwork.linuxtv.org/patch/56808/
---
drivers/staging/media/hantro/hantro.h| 6 ++
drivers/staging/media/hantro/hantro_drv.c| 5 +
drivers/staging/media/hantro/rk3288_vpu_hw.c | 8 ++--
drivers/staging/media/hantro/rk3399_vpu_hw.c | 8 ++--
4 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro.h
b/drivers/staging/media/hantro/hantro.h
index 5c2f87272ce2..62dcca9ff19c 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -25,8 +25,6 @@
#include "hantro_hw.h"
-#define HANTRO_MAX_CLOCKS 4
-
#define MPEG2_MB_DIM 16
#define MPEG2_MB_WIDTH(w) DIV_ROUND_UP(w, MPEG2_MB_DIM)
#define MPEG2_MB_HEIGHT(h) DIV_ROUND_UP(h, MPEG2_MB_DIM)
@@ -88,7 +86,7 @@ struct hantro_variant {
int (*runtime_resume)(struct hantro_dev *vpu);
const struct hantro_irq *irqs;
int num_irqs;
- const char *clk_names[HANTRO_MAX_CLOCKS];
+ const char * const *clk_names;
int num_clocks;
const char * const *reg_names;
int num_regs;
@@ -182,7 +180,7 @@ struct hantro_dev {
struct hantro_func *decoder;
struct platform_device *pdev;
struct device *dev;
- struct clk_bulk_data clocks[HANTRO_MAX_CLOCKS];
+ struct clk_bulk_data *clocks;
void __iomem **reg_bases;
void __iomem *enc_base;
void __iomem *dec_base;
diff --git a/drivers/staging/media/hantro/hantro_drv.c
b/drivers/staging/media/hantro/hantro_drv.c
index fc8f3ed8e80c..1d3af881d088 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -687,6 +687,11 @@ static int hantro_probe(struct platform_device *pdev)
INIT_DELAYED_WORK(&vpu->watchdog_work, hantro_watchdog);
+ vpu->clocks = devm_kcalloc(&pdev->dev, vpu->variant->num_clocks,
+ sizeof(*vpu->clocks), GFP_KERNEL);
+ if (!vpu->clocks)
+ return -ENOMEM;
+
for (i = 0; i < vpu->variant->num_clocks; i++)
vpu->clocks[i].id = vpu->variant->clk_names[i];
ret = devm_clk_bulk_get(&pdev->dev, vpu->variant->num_clocks,
diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c
b/drivers/staging/media/hantro/rk3288_vpu_hw.c
index c5473bc1ac29..bcacc4f51093 100644
--- a/drivers/staging/media/hantro/rk3288_vpu_hw.c
+++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c
@@ -166,6 +166,10 @@ static const struct hantro_irq rk3288_irqs[] = {
{ "vdpu", rk3288_vdpu_irq },
};
+static const char * const rk3288_clk_names[] = {
+ "aclk", "hclk"
+};
+
const struct hantro_variant rk3288_vpu_variant = {
.enc_offset = 0x0,
.enc_fmts = rk3288_vpu_enc_fmts,
@@ -178,6 +182,6 @@ const struct hantro_variant rk3288_vpu_variant = {
.irqs = rk3288_irqs,
.num_irqs = ARRAY_SIZE(rk3288_irqs),
.init = rk3288_vpu_hw_init,
- .clk_names = {"aclk", "hclk"},
- .num_clocks = 2
+ .clk_names = rk3288_clk_names,
+ .num_clocks = ARRAY_SIZE(rk3288_clk_names)
};
diff --git a/drivers/staging/media/hantro/rk3399_vpu_hw.c
b/drivers/staging/media/hantro/rk3399_vpu_hw.c
index 965030e21ea9..5718f8063542 100644
--- a/drivers/staging/media/hantro/rk3399_vpu_hw.c
+++ b/drivers/staging/media/hantro/rk3399_vpu_hw.c
@@ -165,6 +165,10 @@ static const struct hantro_irq rk3399_irqs[] = {
{ "vdpu", rk3399_vdpu_irq },
};
+static const char * const rk3399_clk_names[] = {
+ "aclk", "hclk"
+};
+
const struct hantro_variant rk3399_vpu_variant = {
.enc_offset = 0x0,
.enc_fmts = rk3399_vpu_enc_fmts,
@@ -177,6 +181,6 @@ const struct hantro_variant rk3399_vpu_variant = {
.irqs = rk3399_irqs,
.num_irqs = ARRAY_SIZE(rk3399_irqs),
.init = rk3399_vpu_hw_init,
- .clk_names = {"aclk", "hclk"},
- .num_clocks = 2
+ .clk_names = rk3399_clk_names,
+ .num_clocks = ARRAY_SIZE(rk3399_clk_names)
};
--
2.20.1
[PATCH v5 05/10] media: hantro: add support for named register ranges
Add support for multiple register ranges with SoC specific names.
Signed-off-by: Philipp Zabel
Reviewed-by: Boris Brezillon
---
drivers/staging/media/hantro/hantro.h | 8 ++--
drivers/staging/media/hantro/hantro_drv.c | 24 +--
2 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro.h
b/drivers/staging/media/hantro/hantro.h
index d041d36a0805..e8eb747f22ef 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -72,6 +72,8 @@ struct hantro_irq {
* @num_irqs: number of irqs in the array
* @clk_names: array of clock names
* @num_clocks:number of clocks in the array
+ * @reg_names: array of register range names
+ * @num_regs: number of register range names in the array
*/
struct hantro_variant {
unsigned int enc_offset;
@@ -88,6 +90,8 @@ struct hantro_variant {
int num_irqs;
const char *clk_names[HANTRO_MAX_CLOCKS];
int num_clocks;
+ const char * const *reg_names;
+ int num_regs;
};
/**
@@ -160,7 +164,7 @@ hantro_vdev_to_func(struct video_device *vdev)
* @dev: Pointer to device for convenient logging using
* dev_ macros.
* @clocks:Array of clock handles.
- * @base: Mapped address of VPU registers.
+ * @reg_bases: Mapped addresses of VPU registers.
* @enc_base: Mapped address of VPU encoder register for convenience.
* @dec_base: Mapped address of VPU decoder register for convenience.
* @vpu_mutex: Mutex to synchronize V4L2 calls.
@@ -178,7 +182,7 @@ struct hantro_dev {
struct platform_device *pdev;
struct device *dev;
struct clk_bulk_data clocks[HANTRO_MAX_CLOCKS];
- void __iomem *base;
+ void __iomem **reg_bases;
void __iomem *enc_base;
void __iomem *dec_base;
diff --git a/drivers/staging/media/hantro/hantro_drv.c
b/drivers/staging/media/hantro/hantro_drv.c
index 4ed39728da3d..fc8f3ed8e80c 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -670,6 +670,7 @@ static int hantro_probe(struct platform_device *pdev)
const struct of_device_id *match;
struct hantro_dev *vpu;
struct resource *res;
+ int num_bases;
int i, ret;
vpu = devm_kzalloc(&pdev->dev, sizeof(*vpu), GFP_KERNEL);
@@ -693,12 +694,23 @@ static int hantro_probe(struct platform_device *pdev)
if (ret)
return ret;
- res = platform_get_resource(vpu->pdev, IORESOURCE_MEM, 0);
- vpu->base = devm_ioremap_resource(vpu->dev, res);
- if (IS_ERR(vpu->base))
- return PTR_ERR(vpu->base);
- vpu->enc_base = vpu->base + vpu->variant->enc_offset;
- vpu->dec_base = vpu->base + vpu->variant->dec_offset;
+ num_bases = vpu->variant->num_regs ?: 1;
+ vpu->reg_bases = devm_kcalloc(&pdev->dev, num_bases,
+ sizeof(*vpu->reg_bases), GFP_KERNEL);
+ if (!vpu->reg_bases)
+ return -ENOMEM;
+
+ for (i = 0; i < num_bases; i++) {
+ res = vpu->variant->reg_names ?
+ platform_get_resource_byname(vpu->pdev, IORESOURCE_MEM,
+ vpu->variant->reg_names[i]) :
+ platform_get_resource(vpu->pdev, IORESOURCE_MEM, 0);
+ vpu->reg_bases[i] = devm_ioremap_resource(vpu->dev, res);
+ if (IS_ERR(vpu->reg_bases[i]))
+ return PTR_ERR(vpu->reg_bases[i]);
+ }
+ vpu->enc_base = vpu->reg_bases[0] + vpu->variant->enc_offset;
+ vpu->dec_base = vpu->reg_bases[0] + vpu->variant->dec_offset;
ret = dma_set_coherent_mask(vpu->dev, DMA_BIT_MASK(32));
if (ret) {
--
2.20.1
[PATCH v5 09/10] media: hantro: add initial i.MX8MQ support
For now this just enables MPEG-2 decoding on the Hantro G1 on i.MX8MQ.
Signed-off-by: Philipp Zabel
--
Changes since v4 [1]:
- Fix duplicated num_irqs initializer
[1] https://patchwork.linuxtv.org/patch/56802/
---
drivers/staging/media/hantro/Kconfig| 16 +-
drivers/staging/media/hantro/Makefile | 3 +
drivers/staging/media/hantro/hantro_drv.c | 3 +
drivers/staging/media/hantro/hantro_hw.h| 1 +
drivers/staging/media/hantro/imx8m_vpu_hw.c | 171
5 files changed, 190 insertions(+), 4 deletions(-)
create mode 100644 drivers/staging/media/hantro/imx8m_vpu_hw.c
diff --git a/drivers/staging/media/hantro/Kconfig
b/drivers/staging/media/hantro/Kconfig
index be133bbaa68a..e0627ce454fc 100644
--- a/drivers/staging/media/hantro/Kconfig
+++ b/drivers/staging/media/hantro/Kconfig
@@ -1,19 +1,27 @@
# SPDX-License-Identifier: GPL-2.0
config VIDEO_HANTRO
tristate "Hantro VPU driver"
- depends on ARCH_ROCKCHIP || COMPILE_TEST
+ depends on ARCH_MXC || ARCH_ROCKCHIP || COMPILE_TEST
depends on VIDEO_DEV && VIDEO_V4L2 && MEDIA_CONTROLLER
depends on MEDIA_CONTROLLER_REQUEST_API
select VIDEOBUF2_DMA_CONTIG
select VIDEOBUF2_VMALLOC
select V4L2_MEM2MEM_DEV
help
- Support for the Hantro IP based Video Processing Unit present on
- Rockchip SoC, which accelerates video and image encoding and
- decoding.
+ Support for the Hantro IP based Video Processing Units present on
+ Rockchip and NXP i.MX8M SoCs, which accelerate video and image
+ encoding and decoding.
To compile this driver as a module, choose M here: the module
will be called hantro-vpu.
+config VIDEO_HANTRO_IMX8M
+ bool "Hantro VPU i.MX8M support"
+ depends on VIDEO_HANTRO
+ depends on ARCH_MXC || COMPILE_TEST
+ default y
+ help
+ Enable support for i.MX8M SoCs.
+
config VIDEO_HANTRO_ROCKCHIP
bool "Hantro VPU Rockchip support"
depends on VIDEO_HANTRO
diff --git a/drivers/staging/media/hantro/Makefile
b/drivers/staging/media/hantro/Makefile
index 1584acdbf4a3..b63dbccece75 100644
--- a/drivers/staging/media/hantro/Makefile
+++ b/drivers/staging/media/hantro/Makefile
@@ -10,6 +10,9 @@ hantro-vpu-y += \
hantro_jpeg.o \
hantro_mpeg2.o
+hantro-vpu-$(CONFIG_VIDEO_HANTRO_IMX8M) += \
+ imx8m_vpu_hw.o
+
hantro-vpu-$(CONFIG_VIDEO_HANTRO_ROCKCHIP) += \
rk3288_vpu_hw.o \
rk3399_vpu_hw.o
diff --git a/drivers/staging/media/hantro/hantro_drv.c
b/drivers/staging/media/hantro/hantro_drv.c
index 1d3af881d088..8049fd9ce07a 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -418,6 +418,9 @@ static const struct of_device_id of_hantro_match[] = {
#ifdef CONFIG_VIDEO_HANTRO_ROCKCHIP
{ .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, },
{ .compatible = "rockchip,rk3288-vpu", .data = &rk3288_vpu_variant, },
+#endif
+#ifdef CONFIG_VIDEO_HANTRO_IMX8M
+ { .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
#endif
{ /* sentinel */ }
};
diff --git a/drivers/staging/media/hantro/hantro_hw.h
b/drivers/staging/media/hantro/hantro_hw.h
index 3c361c2e9b88..fd6ad017a1cf 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -81,6 +81,7 @@ enum hantro_enc_fmt {
extern const struct hantro_variant rk3399_vpu_variant;
extern const struct hantro_variant rk3328_vpu_variant;
extern const struct hantro_variant rk3288_vpu_variant;
+extern const struct hantro_variant imx8mq_vpu_variant;
void hantro_watchdog(struct work_struct *work);
void hantro_run(struct hantro_ctx *ctx);
diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c
b/drivers/staging/media/hantro/imx8m_vpu_hw.c
new file mode 100644
index ..7377e8c7c6eb
--- /dev/null
+++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Hantro VPU codec driver
+ *
+ * Copyright (C) 2019 Pengutronix, Philipp Zabel
+ */
+
+#include
+#include
+
+#include "hantro.h"
+#include "hantro_jpeg.h"
+#include "hantro_g1_regs.h"
+
+#define CTRL_SOFT_RESET0x00
+#define RESET_G1 BIT(1)
+#define RESET_G2 BIT(0)
+
+#define CTRL_CLOCK_ENABLE 0x04
+#define CLOCK_G1 BIT(1)
+#define CLOCK_G2 BIT(0)
+
+#define CTRL_G1_DEC_FUSE 0x08
+#define CTRL_G1_PP_FUSE0x0c
+#define CTRL_G2_DEC_FUSE 0x10
+
+static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits)
+{
+ u32 val;
+
+ /* Assert */
+ val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
+ val &= ~reset_bits;
+ w
[PATCH v5 08/10] media: dt-bindings: Document i.MX8MQ and i.MX8MM VPU bindings
Add devicetree binding documentation for the Hantro G1/G2 VPU on i.MX8MQ
and for the Hantro G1/G2/H1 VPU on i.MX8MM.
Signed-off-by: Philipp Zabel
---
.../devicetree/bindings/media/imx8m-vpu.txt | 56 +++
1 file changed, 56 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/imx8m-vpu.txt
diff --git a/Documentation/devicetree/bindings/media/imx8m-vpu.txt
b/Documentation/devicetree/bindings/media/imx8m-vpu.txt
new file mode 100644
index ..659bd28dd002
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx8m-vpu.txt
@@ -0,0 +1,56 @@
+device-tree bindings for Hantro G1/G2/H1 VPU codecs implemented on i.MX8M SoCs
+
+Required properties:
+- compatible: value should be one of the following
+ "nxp,imx8mq-vpu",
+ "nxp,imx8mm-vpu";
+- regs: VPU core and control block register ranges
+- reg-names: should be
+ "g1", "g2", "ctrl" on i.MX8MQ,
+ "g1", "g2", "h1", "ctrl" on i.MX8MM.
+- interrupts: encoding and decoding interrupt specifiers
+- interrupt-names: should be
+ "g1", "g2" on i.MX8MQ,
+ "g1", "g2", "h1" on i.MX8MM.
+- clocks: phandle to VPU core clocks and bus clock
+- clock-names: should be
+ "g1", "g2", "bus" on i.MX8MQ,
+ "g1", "g2", "h1", "bus" on i.MX8MM.
+- power-domains: phandle to power domain node
+
+Examples:
+
+ vpu: vpu@3830 {
+ compatible = "nxp,imx8mq-vpu";
+ reg = <0x3830 0x1>,
+ <0x3831 0x1>,
+ <0x3832 0x1>;
+ reg-names = "g1", "g2", "ctrl";
+ interrupts = ,
+;
+ interrupt-names = "g1", "g2";
+ clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
+<&clk IMX8MQ_CLK_VPU_G2_ROOT>,
+<&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
+ clock-names = "g1", "g2", "bus";
+ power-domains = <&pgc_vpu>;
+ };
+
+ vpu: vpu@3830 {
+ compatible = "nxp,imx8mm-vpu";
+ reg = <0x3830 0x1>,
+ <0x3831 0x1>,
+ <0x3832 0x1>;
+ <0x3833 0x1>;
+ reg-names = "g1", "g2", "h1", "ctrl";
+ interrupts = ,
+;
+;
+ interrupt-names = "g1", "g2", "h1";
+ clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
+<&clk IMX8MQ_CLK_VPU_G2_ROOT>,
+<&clk IMX8MQ_CLK_VPU_H1_ROOT>,
+<&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
+ clock-names = "g1", "g2", "h1", "bus";
+ power-domains = <&pgc_vpu>;
+ };
--
2.20.1
[PATCH v5 10/10] media: hantro: add initial i.MX8MM support (untested)
This should enable MPEG-2 decoding on the Hantro G1 and JPEG encoding on
the Hantro H1 on i.MX8MM.
Signed-off-by: Philipp Zabel
---
drivers/staging/media/hantro/hantro_drv.c | 1 +
drivers/staging/media/hantro/hantro_hw.h| 1 +
drivers/staging/media/hantro/imx8m_vpu_hw.c | 139
3 files changed, 141 insertions(+)
diff --git a/drivers/staging/media/hantro/hantro_drv.c
b/drivers/staging/media/hantro/hantro_drv.c
index 8049fd9ce07a..af26672b4538 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -421,6 +421,7 @@ static const struct of_device_id of_hantro_match[] = {
#endif
#ifdef CONFIG_VIDEO_HANTRO_IMX8M
{ .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
+ { .compatible = "nxp,imx8mm-vpu", .data = &imx8mm_vpu_variant, },
#endif
{ /* sentinel */ }
};
diff --git a/drivers/staging/media/hantro/hantro_hw.h
b/drivers/staging/media/hantro/hantro_hw.h
index fd6ad017a1cf..1c69669dba54 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -82,6 +82,7 @@ extern const struct hantro_variant rk3399_vpu_variant;
extern const struct hantro_variant rk3328_vpu_variant;
extern const struct hantro_variant rk3288_vpu_variant;
extern const struct hantro_variant imx8mq_vpu_variant;
+extern const struct hantro_variant imx8mm_vpu_variant;
void hantro_watchdog(struct work_struct *work);
void hantro_run(struct hantro_ctx *ctx);
diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c
b/drivers/staging/media/hantro/imx8m_vpu_hw.c
index 7377e8c7c6eb..397a140341dc 100644
--- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
+++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
@@ -11,18 +11,22 @@
#include "hantro.h"
#include "hantro_jpeg.h"
#include "hantro_g1_regs.h"
+#include "hantro_h1_regs.h"
#define CTRL_SOFT_RESET0x00
#define RESET_G1 BIT(1)
#define RESET_G2 BIT(0)
+#define RESET_H1 BIT(2)
#define CTRL_CLOCK_ENABLE 0x04
#define CLOCK_G1 BIT(1)
#define CLOCK_G2 BIT(0)
+#define CLOCK_H1 BIT(2)
#define CTRL_G1_DEC_FUSE 0x08
#define CTRL_G1_PP_FUSE0x0c
#define CTRL_G2_DEC_FUSE 0x10
+#define CTRL_H1_ENC_FUSE 0x14
static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits)
{
@@ -73,6 +77,30 @@ static int imx8mq_runtime_resume(struct hantro_dev *vpu)
return 0;
}
+static int imx8mm_runtime_resume(struct hantro_dev *vpu)
+{
+ int ret;
+
+ ret = clk_bulk_prepare_enable(vpu->variant->num_clocks, vpu->clocks);
+ if (ret) {
+ dev_err(vpu->dev, "Failed to enable clocks\n");
+ return ret;
+ }
+
+ imx8m_soft_reset(vpu, RESET_G1 | RESET_G2 | RESET_H1);
+ imx8m_clk_enable(vpu, CLOCK_G1 | CLOCK_G2 | RESET_H1);
+
+ /* Set values of the fuse registers */
+ writel(0x, vpu->ctrl_base + CTRL_G1_DEC_FUSE);
+ writel(0x, vpu->ctrl_base + CTRL_G1_PP_FUSE);
+ writel(0x, vpu->ctrl_base + CTRL_G2_DEC_FUSE);
+ writel(0x, vpu->ctrl_base + CTRL_H1_ENC_FUSE);
+
+ clk_bulk_disable_unprepare(vpu->variant->num_clocks, vpu->clocks);
+
+ return 0;
+}
+
/*
* Supported formats.
*/
@@ -97,6 +125,43 @@ static const struct hantro_fmt imx8m_vpu_dec_fmts[] = {
},
};
+static const struct hantro_fmt imx8mm_vpu_enc_fmts[] = {
+ {
+ .fourcc = V4L2_PIX_FMT_YUV420M,
+ .codec_mode = HANTRO_MODE_NONE,
+ .enc_fmt = RK3288_VPU_ENC_FMT_YUV420P,
+ },
+ {
+ .fourcc = V4L2_PIX_FMT_NV12M,
+ .codec_mode = HANTRO_MODE_NONE,
+ .enc_fmt = RK3288_VPU_ENC_FMT_YUV420SP,
+ },
+ {
+ .fourcc = V4L2_PIX_FMT_YUYV,
+ .codec_mode = HANTRO_MODE_NONE,
+ .enc_fmt = RK3288_VPU_ENC_FMT_YUYV422,
+ },
+ {
+ .fourcc = V4L2_PIX_FMT_UYVY,
+ .codec_mode = HANTRO_MODE_NONE,
+ .enc_fmt = RK3288_VPU_ENC_FMT_UYVY422,
+ },
+ {
+ .fourcc = V4L2_PIX_FMT_JPEG,
+ .codec_mode = HANTRO_MODE_JPEG_ENC,
+ .max_depth = 2,
+ .header_size = JPEG_HEADER_SIZE,
+ .frmsize = {
+ .min_width = 96,
+ .max_width = 8192,
+ .step_width = JPEG_MB_DIM,
+ .min_height = 32,
+ .max_height = 8192,
+ .step_height = JPEG_MB_DIM,
+ },
+ },
+};
+
static irqreturn_t imx8m_vpu_g1_irq(int irq, void *dev_id)
{
struct hantro_dev *vpu = dev_id;
@@ -115,6 +180,25 @@ static irqreturn_t imx8m_v
[PATCH v5 02/10] media: hantro: print video device name in addition to device node
It can be helpful to know which video device was registered at which device node. Signed-off-by: Philipp Zabel Reviewed-by: Boris Brezillon --- drivers/staging/media/hantro/hantro_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c index d325f63c7412..0eadcc530e63 100644 --- a/drivers/staging/media/hantro/hantro_drv.c +++ b/drivers/staging/media/hantro/hantro_drv.c @@ -607,7 +607,8 @@ static int hantro_add_func(struct hantro_dev *vpu, unsigned int funcid) goto err_unreg_dev; } - v4l2_info(&vpu->v4l2_dev, "registered as /dev/video%d\n", vfd->num); + v4l2_info(&vpu->v4l2_dev, "registered %s as /dev/video%d\n", vfd->name, + vfd->num); return 0; -- 2.20.1
[PATCH v5 00/10] Rename Rockchip VPU driver to Hantro, add initial i.MX8M support
There are several other SoCs that contain Hantro IP based VPUs, such as
NXP i.MX8MQ (Hantro G1 and G2) and i.MX8MM (Hantro G1, G2, and H1). To
maximize code sharing, add initial support for these SoCs to the
Rockchip VPU driver, after renaming it to Hantro VPU.
This series is based on the br-v5.3g tag, commit e568d2cc1ef6
("rockchip/vpu: Add support for MPEG-2 decoding on RK3288") with
https://patchwork.linuxtv.org/patch/56402/ ("rockchip/vpu: Add support
for MPEG-2 decoding on RK3328") applied on top. It supports MPEG-2
decoding on i.MX8MQ. MPEG-2 decoding and JPEG encoding on i.MX8MM may
or may not work, I don't have the hardware to test.
Changes since v4:
- Rebased onto media-tree master,
RK3328 support will have to be rebased as well
- Fixed duplicated num_irqs initializer in imx8mq_vpu_variant
regards
Philipp
Philipp Zabel (10):
rockchip/vpu: rename from rockchip to hantro
media: hantro: print video device name in addition to device node
media: hantro: add PM runtime resume callback
media: hantro: make irq names configurable
media: hantro: add support for named register ranges
media: hantro: add support for separate control block
media: hantro: allow arbitrary number of clocks
media: dt-bindings: Document i.MX8MQ and i.MX8MM VPU bindings
media: hantro: add initial i.MX8MQ support
media: hantro: add initial i.MX8MM support (untested)
.../devicetree/bindings/media/imx8m-vpu.txt | 56 +++
MAINTAINERS | 4 +-
drivers/staging/media/Kconfig | 4 +-
drivers/staging/media/Makefile| 2 +-
drivers/staging/media/hantro/Kconfig | 31 ++
drivers/staging/media/hantro/Makefile | 18 +
.../media/{rockchip/vpu => hantro}/TODO | 0
.../vpu/rockchip_vpu.h => hantro/hantro.h}| 167 ---
.../hantro_drv.c} | 335 ++---
.../media/hantro/hantro_g1_mpeg2_dec.c| 260 ++
drivers/staging/media/hantro/hantro_g1_regs.h | 301
.../staging/media/hantro/hantro_h1_jpeg_enc.c | 125 +
drivers/staging/media/hantro/hantro_h1_regs.h | 154 ++
drivers/staging/media/hantro/hantro_hw.h | 104
.../hantro_jpeg.c}| 18 +-
drivers/staging/media/hantro/hantro_jpeg.h| 13 +
.../hantro_mpeg2.c} | 14 +-
.../hantro_v4l2.c}| 234 +
.../hantro_v4l2.h}| 16 +-
drivers/staging/media/hantro/imx8m_vpu_hw.c | 310
drivers/staging/media/hantro/rk3288_vpu_hw.c | 187
.../{rockchip/vpu => hantro}/rk3399_vpu_hw.c | 77 +--
.../vpu => hantro}/rk3399_vpu_hw_jpeg_enc.c | 32 +-
.../vpu => hantro}/rk3399_vpu_hw_mpeg2_dec.c | 37 +-
.../vpu => hantro}/rk3399_vpu_regs.h | 2 +-
drivers/staging/media/rockchip/vpu/Kconfig| 14 -
drivers/staging/media/rockchip/vpu/Makefile | 14 -
.../media/rockchip/vpu/rk3288_vpu_hw.c| 177 ---
.../rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c | 125 -
.../rockchip/vpu/rk3288_vpu_hw_mpeg2_dec.c| 261 ---
.../media/rockchip/vpu/rk3288_vpu_regs.h | 443 --
.../media/rockchip/vpu/rockchip_vpu_hw.h | 102
.../media/rockchip/vpu/rockchip_vpu_jpeg.h| 14 -
33 files changed, 2050 insertions(+), 1601 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/imx8m-vpu.txt
create mode 100644 drivers/staging/media/hantro/Kconfig
create mode 100644 drivers/staging/media/hantro/Makefile
rename drivers/staging/media/{rockchip/vpu => hantro}/TODO (100%)
rename drivers/staging/media/{rockchip/vpu/rockchip_vpu.h => hantro/hantro.h}
(66%)
rename drivers/staging/media/{rockchip/vpu/rockchip_vpu_drv.c =>
hantro/hantro_drv.c} (69%)
create mode 100644 drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
create mode 100644 drivers/staging/media/hantro/hantro_g1_regs.h
create mode 100644 drivers/staging/media/hantro/hantro_h1_jpeg_enc.c
create mode 100644 drivers/staging/media/hantro/hantro_h1_regs.h
create mode 100644 drivers/staging/media/hantro/hantro_hw.h
rename drivers/staging/media/{rockchip/vpu/rockchip_vpu_jpeg.c =>
hantro/hantro_jpeg.c} (95%)
create mode 100644 drivers/staging/media/hantro/hantro_jpeg.h
rename drivers/staging/media/{rockchip/vpu/rockchip_vpu_mpeg2.c =>
hantro/hantro_mpeg2.c} (79%)
rename drivers/staging/media/{rockchip/vpu/rockchip_vpu_v4l2.c =>
hantro/hantro_v4l2.c} (69%)
rename drivers/staging/media/{rockchip/vpu/rockchip_vpu_v4l2.h =>
hantro/hantro_v4l2.h} (53%)
create mode 100644 drivers/staging/media/hantro/imx8m_vpu_hw.c
create mode 100644 drivers/staging/media/hantro/rk3288_vpu_hw.c
rename drivers/staging/media/{rockchip/vpu => hantro}/rk3399_vpu_hw.c (65%)
rename drivers/staging/media/{rockchip/vpu => hantro}/rk3399_vpu_hw_jpeg_enc.c
(86%)
rename drivers/staging/m
[PATCH v4 09/10] media: hantro: add initial i.MX8MQ support
For now this just enables MPEG-2 decoding on the Hantro G1 on i.MX8MQ.
Signed-off-by: Philipp Zabel
---
drivers/staging/media/hantro/Kconfig| 16 +-
drivers/staging/media/hantro/Makefile | 3 +
drivers/staging/media/hantro/hantro_drv.c | 3 +
drivers/staging/media/hantro/hantro_hw.h| 1 +
drivers/staging/media/hantro/imx8m_vpu_hw.c | 172
5 files changed, 191 insertions(+), 4 deletions(-)
create mode 100644 drivers/staging/media/hantro/imx8m_vpu_hw.c
diff --git a/drivers/staging/media/hantro/Kconfig
b/drivers/staging/media/hantro/Kconfig
index de77fe6554e7..99aed9a5b0b9 100644
--- a/drivers/staging/media/hantro/Kconfig
+++ b/drivers/staging/media/hantro/Kconfig
@@ -1,19 +1,27 @@
# SPDX-License-Identifier: GPL-2.0
config VIDEO_HANTRO
tristate "Hantro VPU driver"
- depends on ARCH_ROCKCHIP || COMPILE_TEST
+ depends on ARCH_MXC || ARCH_ROCKCHIP || COMPILE_TEST
depends on VIDEO_DEV && VIDEO_V4L2 && MEDIA_CONTROLLER
depends on MEDIA_CONTROLLER_REQUEST_API
select VIDEOBUF2_DMA_CONTIG
select VIDEOBUF2_VMALLOC
select V4L2_MEM2MEM_DEV
help
- Support for the Hantro IP based Video Processing Unit present on
- Rockchip SoC, which accelerates video and image encoding and
- decoding.
+ Support for the Hantro IP based Video Processing Units present on
+ Rockchip and NXP i.MX8M SoCs, which accelerate video and image
+ encoding and decoding.
To compile this driver as a module, choose M here: the module
will be called hantro-vpu.
+config VIDEO_HANTRO_IMX8M
+ bool "Hantro VPU i.MX8M support"
+ depends on VIDEO_HANTRO
+ depends on ARCH_MXC || COMPILE_TEST
+ default y
+ help
+ Enable support for i.MX8M SoCs.
+
config VIDEO_HANTRO_ROCKCHIP
bool "Hantro VPU Rockchip support"
depends on VIDEO_HANTRO
diff --git a/drivers/staging/media/hantro/Makefile
b/drivers/staging/media/hantro/Makefile
index 1584acdbf4a3..b63dbccece75 100644
--- a/drivers/staging/media/hantro/Makefile
+++ b/drivers/staging/media/hantro/Makefile
@@ -10,6 +10,9 @@ hantro-vpu-y += \
hantro_jpeg.o \
hantro_mpeg2.o
+hantro-vpu-$(CONFIG_VIDEO_HANTRO_IMX8M) += \
+ imx8m_vpu_hw.o
+
hantro-vpu-$(CONFIG_VIDEO_HANTRO_ROCKCHIP) += \
rk3288_vpu_hw.o \
rk3399_vpu_hw.o
diff --git a/drivers/staging/media/hantro/hantro_drv.c
b/drivers/staging/media/hantro/hantro_drv.c
index d1f061dd8ede..1829e35113a0 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -419,6 +419,9 @@ static const struct of_device_id of_hantro_match[] = {
{ .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, },
{ .compatible = "rockchip,rk3328-vpu", .data = &rk3328_vpu_variant, },
{ .compatible = "rockchip,rk3288-vpu", .data = &rk3288_vpu_variant, },
+#endif
+#ifdef CONFIG_VIDEO_HANTRO_IMX8M
+ { .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },
#endif
{ /* sentinel */ }
};
diff --git a/drivers/staging/media/hantro/hantro_hw.h
b/drivers/staging/media/hantro/hantro_hw.h
index 3c361c2e9b88..fd6ad017a1cf 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -81,6 +81,7 @@ enum hantro_enc_fmt {
extern const struct hantro_variant rk3399_vpu_variant;
extern const struct hantro_variant rk3328_vpu_variant;
extern const struct hantro_variant rk3288_vpu_variant;
+extern const struct hantro_variant imx8mq_vpu_variant;
void hantro_watchdog(struct work_struct *work);
void hantro_run(struct hantro_ctx *ctx);
diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c
b/drivers/staging/media/hantro/imx8m_vpu_hw.c
new file mode 100644
index ..85a4ecf70392
--- /dev/null
+++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
@@ -0,0 +1,172 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Hantro VPU codec driver
+ *
+ * Copyright (C) 2019 Pengutronix, Philipp Zabel
+ */
+
+#include
+#include
+
+#include "hantro.h"
+#include "hantro_jpeg.h"
+#include "hantro_g1_regs.h"
+
+#define CTRL_SOFT_RESET0x00
+#define RESET_G1 BIT(1)
+#define RESET_G2 BIT(0)
+
+#define CTRL_CLOCK_ENABLE 0x04
+#define CLOCK_G1 BIT(1)
+#define CLOCK_G2 BIT(0)
+
+#define CTRL_G1_DEC_FUSE 0x08
+#define CTRL_G1_PP_FUSE0x0c
+#define CTRL_G2_DEC_FUSE 0x10
+
+static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits)
+{
+ u32 val;
+
+ /* Assert */
+ val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
+ val &= ~reset_bits;
+ writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
+
+
[PATCH v4 00/10] Rename Rockchip VPU driver to Hantro, add initial i.MX8M support
There are several other SoCs that contain Hantro IP based VPUs, such as
NXP i.MX8MQ (Hantro G1 and G2) and i.MX8MM (Hantro G1, G2, and H1). To
maximize code sharing, add initial support for these SoCs to the
Rockchip VPU driver, after renaming it to Hantro VPU.
This series is based on the br-v5.3g tag, commit e568d2cc1ef6
("rockchip/vpu: Add support for MPEG-2 decoding on RK3288") with
https://patchwork.linuxtv.org/patch/56402/ ("rockchip/vpu: Add support
for MPEG-2 decoding on RK3328") applied on top. It supports MPEG-2
decoding on i.MX8MQ. MPEG-2 decoding and JPEG encoding on i.MX8MM may
or may not work, I don't have the hardware to test.
Changes since v3:
- Split rk3288_vpu_regs.h into hantro_g1_regs.h and hantro_h1_regs.h,
rename VDPU register defines to G1 and VEPU register defines to H1.
- Make Rockchip / i.MX8M support configurable.
- Keep staging/media Kconfig in alphabetic order.
- Rename bases to reg_bases
- Move dynamic clocks before i.MX8M support.
regards
Philipp
Philipp Zabel (10):
rockchip/vpu: rename from rockchip to hantro
media: hantro: print video device name in addition to device node
media: hantro: add PM runtime resume callback
media: hantro: make irq names configurable
media: hantro: add support for named register ranges
media: hantro: add support for separate control block
media: hantro: allow arbitrary number of clocks
media: dt-bindings: Document i.MX8MQ and i.MX8MM VPU bindings
media: hantro: add initial i.MX8MQ support
media: hantro: add initial i.MX8MM support (untested)
.../devicetree/bindings/media/imx8m-vpu.txt | 56 +++
MAINTAINERS | 4 +-
drivers/staging/media/Kconfig | 4 +-
drivers/staging/media/Makefile| 2 +-
drivers/staging/media/hantro/Kconfig | 31 ++
drivers/staging/media/hantro/Makefile | 18 +
.../media/{rockchip/vpu => hantro}/TODO | 0
.../vpu/rockchip_vpu.h => hantro/hantro.h}| 167 ---
.../hantro_drv.c} | 335 ++---
.../media/hantro/hantro_g1_mpeg2_dec.c| 260 ++
drivers/staging/media/hantro/hantro_g1_regs.h | 301
.../staging/media/hantro/hantro_h1_jpeg_enc.c | 125 +
drivers/staging/media/hantro/hantro_h1_regs.h | 154 ++
drivers/staging/media/hantro/hantro_hw.h | 104
.../hantro_jpeg.c}| 18 +-
drivers/staging/media/hantro/hantro_jpeg.h| 13 +
.../hantro_mpeg2.c} | 14 +-
.../hantro_v4l2.c}| 234 +
.../hantro_v4l2.h}| 16 +-
drivers/staging/media/hantro/imx8m_vpu_hw.c | 311
drivers/staging/media/hantro/rk3288_vpu_hw.c | 187
.../{rockchip/vpu => hantro}/rk3399_vpu_hw.c | 92 ++--
.../vpu => hantro}/rk3399_vpu_hw_jpeg_enc.c | 32 +-
.../vpu => hantro}/rk3399_vpu_hw_mpeg2_dec.c | 37 +-
.../vpu => hantro}/rk3399_vpu_regs.h | 2 +-
drivers/staging/media/rockchip/vpu/Kconfig| 14 -
drivers/staging/media/rockchip/vpu/Makefile | 14 -
.../media/rockchip/vpu/rk3288_vpu_hw.c| 177 ---
.../rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c | 125 -
.../rockchip/vpu/rk3288_vpu_hw_mpeg2_dec.c| 261 ---
.../media/rockchip/vpu/rk3288_vpu_regs.h | 443 --
.../media/rockchip/vpu/rockchip_vpu_hw.h | 103
.../media/rockchip/vpu/rockchip_vpu_jpeg.h| 14 -
33 files changed, 2061 insertions(+), 1607 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/imx8m-vpu.txt
create mode 100644 drivers/staging/media/hantro/Kconfig
create mode 100644 drivers/staging/media/hantro/Makefile
rename drivers/staging/media/{rockchip/vpu => hantro}/TODO (100%)
rename drivers/staging/media/{rockchip/vpu/rockchip_vpu.h => hantro/hantro.h}
(66%)
rename drivers/staging/media/{rockchip/vpu/rockchip_vpu_drv.c =>
hantro/hantro_drv.c} (69%)
create mode 100644 drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
create mode 100644 drivers/staging/media/hantro/hantro_g1_regs.h
create mode 100644 drivers/staging/media/hantro/hantro_h1_jpeg_enc.c
create mode 100644 drivers/staging/media/hantro/hantro_h1_regs.h
create mode 100644 drivers/staging/media/hantro/hantro_hw.h
rename drivers/staging/media/{rockchip/vpu/rockchip_vpu_jpeg.c =>
hantro/hantro_jpeg.c} (95%)
create mode 100644 drivers/staging/media/hantro/hantro_jpeg.h
rename drivers/staging/media/{rockchip/vpu/rockchip_vpu_mpeg2.c =>
hantro/hantro_mpeg2.c} (79%)
rename drivers/staging/media/{rockchip/vpu/rockchip_vpu_v4l2.c =>
hantro/hantro_v4l2.c} (69%)
rename drivers/staging/media/{rockchip/vpu/rockchip_vpu_v4l2.h =>
hantro/hantro_v4l2.h} (53%)
create mode 100644 drivers/staging/media/hantro/imx8m_vpu_hw.c
create mode 100644 drivers/staging/media/hantro/rk3288_vpu_hw.c
rename drivers/stag
