Re: [PATCH v7] media: imx: add mem2mem device

[Nicolas Dufresne <nicolas@ndufresne.ca>]

Le mardi 12 février 2019 à 11:01 -0800, Tim Harvey a écrit :
> On Thu, Jan 17, 2019 at 7:50 AM Philipp Zabel <p.zabel@pengutronix.de> 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.
> > 
> > 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 <p.zabel@pengutronix.de>
> > [slongerbeam@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()]
> > Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> > ---
> > Changes since v6 [1]:
> >  - Change driver name in querycap to imx-media-csc-scaler
> >  - Drop V4L2_SEL_TGT_COMPOSE_PADDED from vidioc_g_selection
> >  - Simplify error handling in ipu_csc_scaler_init_controls
> > 
> > [1] https://patchwork.linuxtv.org/patch/53757/
> > ---
> 
> Hi Philipp,
> 
> Thanks for this driver - this is providing support that I need to
> overcome direct CSI->IC limitations.
> 
> Can you give me some examples on how your using this? I'm testing this
> on top of linux-media and trying the following gstreamer pipelines
> (gstreamer recent master) and running into trouble but it could very
> likely be me doing something wrong in my pipelines:
> 
> # upscale
> gst-launch-1.0 videotestsrc ! video/x-raw,width=320,height=240 !
> v4l2convert output-io-mode=dmabuf-import !

Something important to note, is that there is no stride adaptation done
in OUTPUT device dmabuf importation path (yet). This is a difficult
task with the V4L2 API, and the reason this need to be opt-in by user.
You need to make sure yourself (for now) that the stride and buffer
alignment matches between the two drivers, and the only way to fix it
if not is by modifying the respective drivers (for now). Some initial
work has been done the other way around, we are trying to make sure we
cover all cases before implementing the other side.

> video/x-raw,width=640,height=480 ! kmssink
> ^^^ fails with
> ERROR: from element
> /GstPipeline:pipeline0/GstAutoVideoSink:autovideosink0/GstKMSSink:autovideosink0-actual-sink-kms:
> GStreamer encountered a general resource error.
> Additional debug info:
> gstkmssink.c(1529): gst_kms_sink_show_frame ():
> /GstPipeline:pipeline0/GstAutoVideoSink:autovideosink0/GstKMSSink:autovideosink0-actual-sink-kms:
> drmModeSetPlane failed: No space left on device (-28)
> perhaps this is something strange with kmssink or is a buffer size not
> being set properly in the mem2mem scaler?

Note that this one can happen randomly as HW may have limitation that
isn't grammatically exposed to userspace. That being said, use
GST_DEBUG="kmssink:7" to enable related debug traces, that should help
to see what is being done. Matching against some kernel trace is always
useful.

> 
> # downscale
> gst-launch-1.0 videotestsrc ! video/x-raw,width=640,height=480 !
> v4l2convert output-io-mode=dmabuf-import !
> video/x-raw,width=320,height=280 ! kmssink
> (gst-launch-1.0:15493): GStreamer-CRITICAL **: 18:06:49.029:
> gst_buffer_resize_range: assertion 'bufmax >= bufoffs + offset + size'

Would be really nice if you could report this, mentioning the GStreamer
version you are running. GStreamer-CRITICAL means that crash avoidance
code (assert) has been reached. This means you have reached a code path
that wasn't expected by the developers. It's a bit like the kernel
BUG_ON.

https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/new

> failed
> ERROR: from element
> /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0: Internal data
> stream error.
> Additional debug info:
> gstbasesrc.c(3064): gst_base_src_loop ():
> /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0:
> streaming stopped, reason error (-5)
> ERROR: pipeline doesn't want to preroll.
> Setting pipeline to NULL ...
> Freeing pipeline ...
> 
> # downscale using videotstsrc defaults
> gst-launch-1.0 videotestsrc ! v4l2convert output-io-mode=dmabuf-import
> ! video/x-raw,width=100,height=200 ! kmssink
> ^^^ works
> 
> # rotation
> gst-launch-1.0 videotestsrc ! v4l2convert output-io-mode=dmabuf-import
> extra-controls=cid,rotate=90 ! kmssink
> ^^^ shows no rotation in displayed video but kernel debugging shows
> ipu_csc_scaler_s_ctrl getting called with V4L2_CID_ROTATE,
> ctrl->val=90 and ipu_degrees_to_rot_mode sets rot_mode=IPU_ROT_BIT_90
> and returns no error. I also see that
> ipu_image_convert_adjust gets called with rot_mode=IPU_ROT_BIT_90

There is no support for 90 degree rotation in the GstV4l2Transform
class. 90 degree rotation must be implemented in the element to be
usable, as the capability negotiation must do the width/height
flipping.

There is also no support for arbitrary rotation, I believe one would
need to add code to suggest a large capture buffer size that fits the
rotated image. It's also ambiguous what would happen otherwise, and
that should be specified the Linux Media documentation in my opinion.

> 
> I'm also not sure how to specify hflip/vflip... I don't think
> extra-controls parses 'hflip', 'vflip' as ipu_csc_scaler_s_ctrl gets
> called with V4L2_CID_HFLIP/V4L2_CID_VFLIP but ctrl->val is always 0.

hflip/vflip was reported to work on vim2m by Mauro, not sure why it
wouldn't for your driver. Maybe both driver don't expose this the same
way ?

> 
> Thanks,
> 
> Tim