Re: [RFC PATCH v3] media: docs-rst: Document m2m stateless video decoder interface

2018-10-22 Thread Tomasz Figa
Hi Alex,

On Fri, Oct 19, 2018 at 5:09 PM Alexandre Courbot  wrote:
>
> Thanks everyone for the feedback on v2! I have not replied to all the
> individual emails but hope this v3 will address some of the problems
> raised and become a continuation point for the topics still in
> discussion (probably during the ELCE Media Summit).

Thanks for your patch! Some further comments inline.

[snip]
> * The restriction of having to send full frames for each input buffer is
>   kept as-is. As Hans pointed, we currently have a hard limit of 32
>   buffers per queue, and it may be non-trivial to lift. Also some codecs
>   (at least Venus AFAIK) do have this restriction in hardware, so unless

Venus is a stateful decoder, so not very relevant for this interface.

However, Rockchip VPU is a stateless decoder that seems to have this
restriction. It seems to have only one base address and length
register and it seems to consume all the slices as laid out in the
original bitstream.

[snip]
> +3. The client may use :c:func:`VIDIOC_ENUM_FRAMESIZES` to detect supported
> +   resolutions for a given format, passing desired pixel format in
> +   :c:type:`v4l2_frmsizeenum`'s ``pixel_format``.
> +
> +   * Values returned by :c:func:`VIDIOC_ENUM_FRAMESIZES` on ``OUTPUT`` queue
> + must include all possible coded resolutions supported by the decoder
> + for the current coded pixel format.
> +
> +   * Values returned by :c:func:`VIDIOC_ENUM_FRAMESIZES` on ``CAPTURE`` queue
> + must include all possible frame buffer resolutions supported by the
> + decoder for given raw pixel format and coded format currently set on
> + ``OUTPUT`` queue.
> +
> +.. note::
> +
> +   The client may derive the supported resolution range for a
> +   combination of coded and raw format by setting width and height of
> +   ``OUTPUT`` format to 0 and calculating the intersection of
> +   resolutions returned from calls to :c:func:`VIDIOC_ENUM_FRAMESIZES`
> +   for the given coded and raw formats.

I've dropped this note in the stateful version, because it would
return something that is contradictory to what S_FMT would accept - it
only accepts the resolution matching the current stream. It also
wouldn't work for decoders which have built-in scaling capability,
because typically the possible range scaling ratios is fixed, so the
maximum and minimum output resolution would depend on the source
resolution.

[snip]
> +Decoding
> +
> +
> +For each frame, the client is responsible for submitting a request to which 
> the
> +following is attached:
> +
> +* Exactly one frame worth of encoded data in a buffer submitted to the
> +  ``OUTPUT`` queue,
> +* All the controls relevant to the format being decoded (see below for 
> details).
> +
> +.. note::
> +
> +   The API currently requires one frame of encoded data per ``OUTPUT`` 
> buffer,
> +   even though some encoded formats may present their data in smaller chunks
> +   (e.g. H.264's frames can be made of several slices that can be processed
> +   independently). It is currently the responsibility of the client to gather
> +   the different parts of a frame into a single ``OUTPUT`` buffer, if 
> required
> +   by the encoded format. This restriction may be lifted in the future.

And maybe we should explicitly say that it should be laid out the same
way as in the bitstream?

But now when I think of it, while still keeping the Rockchip VPU in
mind, AFAIR the slices in H.264 can arrive in separate packets, so
maybe the Rockchip VPU can just consume them separately too and in any
order, as long as they are laid out in a contiguous manner? The
hardware isn't unfortunately very well documented...

> +
> +``CAPTURE`` buffers must not be part of the request, and are queued
> +independently. The driver will always pick the least recently queued 
> ``CAPTURE``
> +buffer and decode the frame into it. ``CAPTURE`` buffers will be returned in
> +decode order (i.e. the same order as ``OUTPUT`` buffers were submitted),
> +therefore it is trivial for the client to know which ``CAPTURE`` buffer will
> +be used for a given frame. This point is essential for referencing frames 
> since
> +we use the ``CAPTURE`` buffer index for that.
> +
> +If the request is submitted without an ``OUTPUT`` buffer, then
> +:c:func:`MEDIA_REQUEST_IOC_QUEUE` will return ``-ENOENT``. If more than one
> +buffer is queued, or if some of the required controls are missing, then it 
> will
> +return ``-EINVAL``. Decoding errors are signaled by the ``CAPTURE`` buffers
> +being dequeued carrying the ``V4L2_BUF_FLAG_ERROR`` flag. If a reference 
> frame
> +has an error, then all other frames that refer to it will also have the
> +``V4L2_BUF_FLAG_ERROR`` flag set.

Perhaps we could want to specify whether those frames would be still
decoded or the whole request discarded?

[snip]
> +Dynamic resolution change
> +=
> +
> +If the client detects a resolution change in the stream, it will need 

cron job: media_tree daily build: OK

2018-10-22 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Tue Oct 23 05:00:11 CEST 2018
media-tree git hash:3b796aa60af087f5fec75aee9b17f2130f2b9adc
media_build git hash:   0c8bb27f3aaa682b9548b656f77505c3d1f11e71
v4l-utils git hash: c36dbbdfa8b30b2badd4f893b59d0bd4f0bd12aa
edid-decode git hash:   5eeb151a748788666534d6ea3da07f90400d24c2
gcc version:i686-linux-gcc (GCC) 8.2.0
sparse version: 0.5.2
smatch version: 0.5.1
host hardware:  x86_64
host os:4.18.0-2-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: OK
linux-git-i686: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
Check COMPILE_TEST: OK
linux-3.10.108-i686: OK
linux-3.10.108-x86_64: OK
linux-3.11.10-i686: OK
linux-3.11.10-x86_64: OK
linux-3.12.74-i686: OK
linux-3.12.74-x86_64: OK
linux-3.13.11-i686: OK
linux-3.13.11-x86_64: OK
linux-3.14.79-i686: OK
linux-3.14.79-x86_64: OK
linux-3.15.10-i686: OK
linux-3.15.10-x86_64: OK
linux-3.16.57-i686: OK
linux-3.16.57-x86_64: OK
linux-3.17.8-i686: OK
linux-3.17.8-x86_64: OK
linux-3.18.123-i686: OK
linux-3.18.123-x86_64: OK
linux-3.19.8-i686: OK
linux-3.19.8-x86_64: OK
linux-4.0.9-i686: OK
linux-4.0.9-x86_64: OK
linux-4.1.52-i686: OK
linux-4.1.52-x86_64: OK
linux-4.2.8-i686: OK
linux-4.2.8-x86_64: OK
linux-4.3.6-i686: OK
linux-4.3.6-x86_64: OK
linux-4.4.159-i686: OK
linux-4.4.159-x86_64: OK
linux-4.5.7-i686: OK
linux-4.5.7-x86_64: OK
linux-4.6.7-i686: OK
linux-4.6.7-x86_64: OK
linux-4.7.10-i686: OK
linux-4.7.10-x86_64: OK
linux-4.8.17-i686: OK
linux-4.8.17-x86_64: OK
linux-4.9.131-i686: OK
linux-4.9.131-x86_64: OK
linux-4.10.17-i686: OK
linux-4.10.17-x86_64: OK
linux-4.11.12-i686: OK
linux-4.11.12-x86_64: OK
linux-4.12.14-i686: OK
linux-4.12.14-x86_64: OK
linux-4.13.16-i686: OK
linux-4.13.16-x86_64: OK
linux-4.14.74-i686: OK
linux-4.14.74-x86_64: OK
linux-4.15.18-i686: OK
linux-4.15.18-x86_64: OK
linux-4.16.18-i686: OK
linux-4.16.18-x86_64: OK
linux-4.17.19-i686: OK
linux-4.17.19-x86_64: OK
linux-4.18.12-i686: OK
linux-4.18.12-x86_64: OK
linux-4.19-rc6-i686: OK
linux-4.19-rc6-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html


Re: [PATCH] media: venus: add support for key frame

2018-10-22 Thread Tomasz Figa
On Mon, Oct 22, 2018 at 3:15 PM Alexandre Courbot  wrote:
>
> On Fri, Oct 12, 2018 at 5:10 PM Stanimir Varbanov
>  wrote:
> >
> >
> >
> > On 10/12/2018 11:06 AM, Alexandre Courbot wrote:
> > > On Fri, Oct 12, 2018 at 4:37 PM Stanimir Varbanov
> > >  wrote:
> > >>
> > >> Hi Alex,
> > >>
> > >> On 10/12/2018 08:26 AM, Alexandre Courbot wrote:
> > >>> On Tue, Oct 9, 2018 at 4:54 PM Malathi Gottam  
> > >>> wrote:
> > 
> >  When client requests for a keyframe, set the property
> >  to hardware to generate the sync frame.
> > 
> >  Signed-off-by: Malathi Gottam 
> >  ---
> >   drivers/media/platform/qcom/venus/venc_ctrls.c | 13 +
> >   1 file changed, 13 insertions(+)
> > 
> >  diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c 
> >  b/drivers/media/platform/qcom/venus/venc_ctrls.c
> >  index 45910172..f332c8e 100644
> >  --- a/drivers/media/platform/qcom/venus/venc_ctrls.c
> >  +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c
> >  @@ -81,6 +81,8 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl)
> >  struct venc_controls *ctr = &inst->controls.enc;
> >  u32 bframes;
> >  int ret;
> >  +   void *ptr;
> >  +   u32 ptype;
> > 
> >  switch (ctrl->id) {
> >  case V4L2_CID_MPEG_VIDEO_BITRATE_MODE:
> >  @@ -173,6 +175,14 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl)
> > 
> >  ctr->num_b_frames = bframes;
> >  break;
> >  +   case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:
> >  +   ptype = HFI_PROPERTY_CONFIG_VENC_REQUEST_SYNC_FRAME;
> >  +   ret = hfi_session_set_property(inst, ptype, ptr);
> > >>>
> > >>> The test bot already said it, but ptr is passed to
> > >>> hfi_session_set_property() uninitialized. And as can be expected the
> > >>> call returns -EINVAL on my board.
> > >>>
> > >>> Looking at other uses of HFI_PROPERTY_CONFIG_VENC_REQUEST_SYNC_FRAME I
> > >>> see that the packet sent to the firmware does not have room for an
> > >>> argument, so I tried to pass NULL but got the same result.
> > >>
> > >> yes, because pdata cannot be NULL. I'd suggest to make a pointer to
> > >> struct hfi_enable and pass it to the set_property function.
> > >
> > > FWIW I also tried doing this and got the same error, strange...
> > >
> >
> > OK, when you calling the v4l control? It makes sense when you calling
> > it, because set_property checks does the session is on START state (i.e.
> > streamon on both queues).
>
> Do you mean that the property won't be actually applied unless both
> queues are streaming? In that case maybe it would make sense for the
> driver to save controls set before that and apply them when the
> conditions allow them to be effective?

Right. The driver cannot just drop a control setting on the floor if
it's not ready to apply it.

However, the V4L2 control framework already provides a tool to handle this:
 - the driver can ignore any .s_ctrl() calls when it can't apply the controls,
 - the driver must call v4l2_ctrl_handler_setup() when it initialized
the hardware, so that all the control values are applied in one go.

Best regards,
Tomasz


Re: [PATCH] media: venus: amend buffer size for bitstream plane

2018-10-22 Thread Tomasz Figa
Hi Malathi,

On Tue, Oct 9, 2018 at 4:58 PM Malathi Gottam  wrote:
>
> For lower resolutions, incase of encoder, the compressed
> frame size is more than half of the corresponding input
> YUV. Keep the size as same as YUV considering worst case.
>
> Signed-off-by: Malathi Gottam 
> ---
>  drivers/media/platform/qcom/venus/helpers.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/venus/helpers.c 
> b/drivers/media/platform/qcom/venus/helpers.c
> index 2679adb..05c5423 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -649,7 +649,7 @@ u32 venus_helper_get_framesz(u32 v4l2_fmt, u32 width, u32 
> height)
> }
>
> if (compressed) {
> -   sz = ALIGN(height, 32) * ALIGN(width, 32) * 3 / 2 / 2;
> +   sz = ALIGN(height, 32) * ALIGN(width, 32) * 3 / 2;
> return ALIGN(sz, SZ_4K);
> }

Note that the driver should not enforce one particular buffer size for
bitstream buffers unless it's a workaround for broken firmware or
hardware. The userspace should be able to select the desired size.

Best regards,
Tomasz


Re: [PATCH v2 0/2] Document memory-to-memory video codec interfaces

2018-10-22 Thread Tomasz Figa
Hi Hans,

On Tue, Oct 23, 2018 at 12:41 AM Hans Verkuil  wrote:
>
> Hi Tomasz, Alexandre,
>
> Thank you for all your work! Much appreciated.
>
> I've applied both the stateful and stateless patches on top of the 
> request_api branch
> and made the final result available here:
>
> https://hverkuil.home.xs4all.nl/request-api/
>
> Tomasz, I got two warnings when building the doc tree, the patch below fixes 
> it.
>
> Regards,
>
> Hans
>
> Signed-off-by: Hans Verkuil 
>
> diff --git a/Documentation/media/uapi/v4l/dev-decoder.rst 
> b/Documentation/media/uapi/v4l/dev-decoder.rst
> index 09c7a6621b8e..5522453ac39f 100644
> --- a/Documentation/media/uapi/v4l/dev-decoder.rst
> +++ b/Documentation/media/uapi/v4l/dev-decoder.rst
> @@ -972,11 +972,11 @@ sequence was started.
>
> .. warning::
>
> -   The sentence can be only initiated if both ``OUTPUT`` and ``CAPTURE`` 
> queues

This should also have been s/sentence/sequence/.

> -   are streaming. For compatibility reasons, the call to
> -   :c:func:`VIDIOC_DECODER_CMD` will not fail even if any of the queues is 
> not
> -   streaming, but at the same time it will not initiate the `Drain` sequence
> -   and so the steps described below would not be applicable.
> +  The sentence can be only initiated if both ``OUTPUT`` and ``CAPTURE`` 
> queues
> +  are streaming. For compatibility reasons, the call to
> +  :c:func:`VIDIOC_DECODER_CMD` will not fail even if any of the queues 
> is not
> +  streaming, but at the same time it will not initiate the `Drain` 
> sequence
> +  and so the steps described below would not be applicable.
>
>  2. Any ``OUTPUT`` buffers queued by the client before the
> :c:func:`VIDIOC_DECODER_CMD` was issued will be processed and decoded as
> diff --git a/Documentation/media/uapi/v4l/dev-encoder.rst 
> b/Documentation/media/uapi/v4l/dev-encoder.rst
> index 41139e5e48eb..7f49a7149067 100644
> --- a/Documentation/media/uapi/v4l/dev-encoder.rst
> +++ b/Documentation/media/uapi/v4l/dev-encoder.rst
> @@ -448,11 +448,11 @@ sequence was started.
>
> .. warning::
>
> -   The sentence can be only initiated if both ``OUTPUT`` and ``CAPTURE`` 
> queues

Ditto.

> -   are streaming. For compatibility reasons, the call to
> -   :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is 
> not
> -   streaming, but at the same time it will not initiate the `Drain` sequence
> -   and so the steps described below would not be applicable.
> +  The sentence can be only initiated if both ``OUTPUT`` and ``CAPTURE`` 
> queues
> +  are streaming. For compatibility reasons, the call to
> +  :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues 
> is not
> +  streaming, but at the same time it will not initiate the `Drain` 
> sequence
> +  and so the steps described below would not be applicable.

Last minute changes after proof reading...

Thanks for fixing up and uploading the html version!

Best regards,
Tomasz


Re: [RFC] Informal meeting during ELCE to discuss userspace support for stateless codecs

2018-10-22 Thread Hans Verkuil
On 10/22/2018 10:17 PM, Hans Verkuil wrote:
> A quick update:
> 
> As said in my previous email: we'll meet at 11 am at the registration desk.
> From there we go to the Platform 5 Cafe. If that's too crowded/noisy, then
> we'll try the Sheraton hotel.
> 
> Tomasz, I'll ping you on irc when we found a good spot and we can setup the
> Hangouts meeting.

I forgot to mention: the spec with the Request API and the stateful+stateless
codec API specification (latest patches from Tomasz and Alexandre) are available
here:

https://hverkuil.home.xs4all.nl/request-api/

Also CC-ed Alexandre and Phillip.

Regards,

Hans

> 
> Hope to see you all tomorrow,
> 
> Regards,
> 
>   Hans
> 
> On 10/10/2018 07:55 AM, Hans Verkuil wrote:
>> On 10/08/2018 01:53 PM, Hans Verkuil wrote:
>>> Hi all,
>>>
>>> I would like to meet up somewhere during the ELCE to discuss userspace 
>>> support
>>> for stateless (and perhaps stateful as well?) codecs.
>>>
>>> It is also planned as a topic during the summit, but I would prefer to 
>>> prepare
>>> for that in advance, esp. since I myself do not have any experience writing
>>> userspace SW for such devices.
>>>
>>> Nicolas, it would be really great if you can participate in this meeting
>>> since you probably have the most experience with this by far.
>>>
>>> Looking through the ELCE program I found two timeslots that are likely to 
>>> work
>>> for most of us (because the topics in the program appear to be boring for us
>>> media types!):
>>>
>>> Tuesday from 10:50-15:50
>>
>> Let's do this Tuesday. Let's meet at the Linux Foundation Registration
>> Desk at 11:00. I'll try to figure out where we can sit the day before.
>> Please check your email Tuesday morning for any last minute changes.
>>
>> Tomasz, it would be nice indeed if we can get you and Paul in as well
>> using Hangouts on my laptop.
>>
>> I would very much appreciate it if those who have experience with the
>> userspace support think about this beforehand and make some requirements
>> list of what you would like to see.
>>
>> Regards,
>>
>>  Hans
>>
>>>
>>> or:
>>>
>>> Monday from 15:45 onward
>>>
>>> My guess is that we need 2-3 hours or so. Hard to predict.
>>>
>>> The basic question that I would like to have answered is what the userspace
>>> component should look like? libv4l-like plugin or a library that userspace 
>>> can
>>> link with? Do we want more general support for stateful codecs as well that 
>>> deals
>>> with resolution changes and the more complex parts of the codec API?
>>>
>>> I've mailed this directly to those that I expect are most interested in 
>>> this,
>>> but if someone want to join in let me know.
>>>
>>> I want to keep the group small though, so you need to bring relevant 
>>> experience
>>> to the table.
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>
> 



Re: [RFC] Informal meeting during ELCE to discuss userspace support for stateless codecs

2018-10-22 Thread Hans Verkuil
A quick update:

As said in my previous email: we'll meet at 11 am at the registration desk.
>From there we go to the Platform 5 Cafe. If that's too crowded/noisy, then
we'll try the Sheraton hotel.

Tomasz, I'll ping you on irc when we found a good spot and we can setup the
Hangouts meeting.

Hope to see you all tomorrow,

Regards,

Hans

On 10/10/2018 07:55 AM, Hans Verkuil wrote:
> On 10/08/2018 01:53 PM, Hans Verkuil wrote:
>> Hi all,
>>
>> I would like to meet up somewhere during the ELCE to discuss userspace 
>> support
>> for stateless (and perhaps stateful as well?) codecs.
>>
>> It is also planned as a topic during the summit, but I would prefer to 
>> prepare
>> for that in advance, esp. since I myself do not have any experience writing
>> userspace SW for such devices.
>>
>> Nicolas, it would be really great if you can participate in this meeting
>> since you probably have the most experience with this by far.
>>
>> Looking through the ELCE program I found two timeslots that are likely to 
>> work
>> for most of us (because the topics in the program appear to be boring for us
>> media types!):
>>
>> Tuesday from 10:50-15:50
> 
> Let's do this Tuesday. Let's meet at the Linux Foundation Registration
> Desk at 11:00. I'll try to figure out where we can sit the day before.
> Please check your email Tuesday morning for any last minute changes.
> 
> Tomasz, it would be nice indeed if we can get you and Paul in as well
> using Hangouts on my laptop.
> 
> I would very much appreciate it if those who have experience with the
> userspace support think about this beforehand and make some requirements
> list of what you would like to see.
> 
> Regards,
> 
>   Hans
> 
>>
>> or:
>>
>> Monday from 15:45 onward
>>
>> My guess is that we need 2-3 hours or so. Hard to predict.
>>
>> The basic question that I would like to have answered is what the userspace
>> component should look like? libv4l-like plugin or a library that userspace 
>> can
>> link with? Do we want more general support for stateful codecs as well that 
>> deals
>> with resolution changes and the more complex parts of the codec API?
>>
>> I've mailed this directly to those that I expect are most interested in this,
>> but if someone want to join in let me know.
>>
>> I want to keep the group small though, so you need to bring relevant 
>> experience
>> to the table.
>>
>> Regards,
>>
>>  Hans
>>
> 



Custom Made logo products

2018-10-22 Thread Lilly Koller

Hi,

I didn’t know if you had received my email from last week?
Can you direct me to the person that handles your company  promo items?
Do you have any upcoming events, tradeshows or promotional needs?
We manufacture ALL custom LOGO and branded products.
The most asked about product that we make, is the custom printed USB flash
drives!

We can print your logo on them and load your digital images, videos and
files!
If you need marketing, advertising, gifts or incentives, USB flash drives
are the solution!

Here is what we include:
-Any size memory you need: 64MB up to 128GB
-We will print your logo on both sides, just ask!
-Very Low Order Minimums
-Need them quickly?  Not a problem, we offer Rush Service

NEW:   We can make a custom shaped USB drive to look like your Logo or
product!
Email over a copy of your logo and we will create a design mock up for you
at no cost!
Our higher memory sizes are a really good option right now!

Ask about the “Double Your Memory” upgrade promotion going on right
now!
Pricing is low right now, so let us know what you need and we will get you
a quick quote.

We will beat any competitors pricing, send us your last invoice and we will
beat it!

We always offer great rates for schools and nonprofits as well.
Regards,

Lilly Koller
Logo USB Account Manager



Re: [PATCH v2 0/2] Document memory-to-memory video codec interfaces

2018-10-22 Thread Hans Verkuil
Hi Tomasz, Alexandre,

Thank you for all your work! Much appreciated.

I've applied both the stateful and stateless patches on top of the request_api 
branch
and made the final result available here:

https://hverkuil.home.xs4all.nl/request-api/

Tomasz, I got two warnings when building the doc tree, the patch below fixes it.

Regards,

Hans

Signed-off-by: Hans Verkuil 

diff --git a/Documentation/media/uapi/v4l/dev-decoder.rst 
b/Documentation/media/uapi/v4l/dev-decoder.rst
index 09c7a6621b8e..5522453ac39f 100644
--- a/Documentation/media/uapi/v4l/dev-decoder.rst
+++ b/Documentation/media/uapi/v4l/dev-decoder.rst
@@ -972,11 +972,11 @@ sequence was started.

.. warning::

-   The sentence can be only initiated if both ``OUTPUT`` and ``CAPTURE`` queues
-   are streaming. For compatibility reasons, the call to
-   :c:func:`VIDIOC_DECODER_CMD` will not fail even if any of the queues is not
-   streaming, but at the same time it will not initiate the `Drain` sequence
-   and so the steps described below would not be applicable.
+  The sentence can be only initiated if both ``OUTPUT`` and ``CAPTURE`` 
queues
+  are streaming. For compatibility reasons, the call to
+  :c:func:`VIDIOC_DECODER_CMD` will not fail even if any of the queues is 
not
+  streaming, but at the same time it will not initiate the `Drain` sequence
+  and so the steps described below would not be applicable.

 2. Any ``OUTPUT`` buffers queued by the client before the
:c:func:`VIDIOC_DECODER_CMD` was issued will be processed and decoded as
diff --git a/Documentation/media/uapi/v4l/dev-encoder.rst 
b/Documentation/media/uapi/v4l/dev-encoder.rst
index 41139e5e48eb..7f49a7149067 100644
--- a/Documentation/media/uapi/v4l/dev-encoder.rst
+++ b/Documentation/media/uapi/v4l/dev-encoder.rst
@@ -448,11 +448,11 @@ sequence was started.

.. warning::

-   The sentence can be only initiated if both ``OUTPUT`` and ``CAPTURE`` queues
-   are streaming. For compatibility reasons, the call to
-   :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is not
-   streaming, but at the same time it will not initiate the `Drain` sequence
-   and so the steps described below would not be applicable.
+  The sentence can be only initiated if both ``OUTPUT`` and ``CAPTURE`` 
queues
+  are streaming. For compatibility reasons, the call to
+  :c:func:`VIDIOC_ENCODER_CMD` will not fail even if any of the queues is 
not
+  streaming, but at the same time it will not initiate the `Drain` sequence
+  and so the steps described below would not be applicable.

 2. Any ``OUTPUT`` buffers queued by the client before the
:c:func:`VIDIOC_ENCODER_CMD` was issued will be processed and encoded as


[PATCH v2 2/2] media: docs-rst: Document memory-to-memory video encoder interface

2018-10-22 Thread Tomasz Figa
Due to complexity of the video encoding process, the V4L2 drivers of
stateful encoder hardware require specific sequences of V4L2 API calls
to be followed. These include capability enumeration, initialization,
encoding, encode parameters change, drain and reset.

Specifics of the above have been discussed during Media Workshops at
LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
Conference Europe 2014 in Düsseldorf. The de facto Codec API that
originated at those events was later implemented by the drivers we already
have merged in mainline, such as s5p-mfc or coda.

The only thing missing was the real specification included as a part of
Linux Media documentation. Fix it now and document the encoder part of
the Codec API.

Signed-off-by: Tomasz Figa 
---
 Documentation/media/uapi/v4l/dev-encoder.rst  | 579 ++
 Documentation/media/uapi/v4l/devices.rst  |   1 +
 Documentation/media/uapi/v4l/pixfmt-v4l2.rst  |   5 +
 Documentation/media/uapi/v4l/v4l2.rst |   2 +
 .../media/uapi/v4l/vidioc-encoder-cmd.rst |  38 +-
 5 files changed, 610 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/media/uapi/v4l/dev-encoder.rst

diff --git a/Documentation/media/uapi/v4l/dev-encoder.rst 
b/Documentation/media/uapi/v4l/dev-encoder.rst
new file mode 100644
index ..41139e5e48eb
--- /dev/null
+++ b/Documentation/media/uapi/v4l/dev-encoder.rst
@@ -0,0 +1,579 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _encoder:
+
+*
+Memory-to-memory Stateful Video Encoder Interface
+*
+
+A stateful video encoder takes raw video frames in display order and encodes
+them into a bitstream. It generates complete chunks of the bitstream, including
+all metadata, headers, etc. The resulting bitstream does not require any
+further post-processing by the client.
+
+Performing software stream processing, header generation etc. in the driver
+in order to support this interface is strongly discouraged. In case such
+operations are needed, use of the Stateless Video Encoder Interface (in
+development) is strongly advised.
+
+Conventions and notation used in this document
+==
+
+1. The general V4L2 API rules apply if not specified in this document
+   otherwise.
+
+2. The meaning of words "must", "may", "should", etc. is as per RFC
+   2119.
+
+3. All steps not marked "optional" are required.
+
+4. :c:func:`VIDIOC_G_EXT_CTRLS`, :c:func:`VIDIOC_S_EXT_CTRLS` may be used
+   interchangeably with :c:func:`VIDIOC_G_CTRL`, :c:func:`VIDIOC_S_CTRL`,
+   unless specified otherwise.
+
+5. Single-plane API (see spec) and applicable structures may be used
+   interchangeably with Multi-plane API, unless specified otherwise,
+   depending on encoder capabilities and following the general V4L2
+   guidelines.
+
+6. i = [a..b]: sequence of integers from a to b, inclusive, i.e. i =
+   [0..2]: i = 0, 1, 2.
+
+7. Given an ``OUTPUT`` buffer A, A' represents a buffer on the ``CAPTURE``
+   queue containing data (encoded frame/stream) that resulted from processing
+   buffer A.
+
+Glossary
+
+
+Refer to :ref:`decoder-glossary`.
+
+State machine
+=
+
+.. kernel-render:: DOT
+   :alt: DOT digraph of encoder state machine
+   :caption: Encoder state machine
+
+   digraph encoder_state_machine {
+   node [shape = doublecircle, label="Encoding"] Encoding;
+
+   node [shape = circle, label="Initialization"] Initialization;
+   node [shape = circle, label="Stopped"] Stopped;
+   node [shape = circle, label="Drain"] Drain;
+   node [shape = circle, label="Reset"] Reset;
+
+   node [shape = point]; qi
+   qi -> Initialization [ label = "open()" ];
+
+   Initialization -> Encoding [ label = "Both queues streaming" ];
+
+   Encoding -> Drain [ label = "V4L2_DEC_CMD_STOP" ];
+   Encoding -> Reset [ label = "VIDIOC_STREAMOFF(CAPTURE)" ];
+   Encoding -> Stopped [ label = "VIDIOC_STREAMOFF(OUTPUT)" ];
+   Encoding -> Encoding;
+
+   Drain -> Stopped [ label = "All CAPTURE\nbuffers 
dequeued\nor\nVIDIOC_STREAMOFF(CAPTURE)" ];
+   Drain -> Reset [ label = "VIDIOC_STREAMOFF(CAPTURE)" ];
+
+   Reset -> Encoding [ label = "VIDIOC_STREAMON(CAPTURE)" ];
+   Reset -> Initialization [ label = "VIDIOC_REQBUFS(OUTPUT, 0)" ];
+
+   Stopped -> Encoding [ label = 
"V4L2_DEC_CMD_START\nor\nVIDIOC_STREAMON(OUTPUT)" ];
+   Stopped -> Reset [ label = "VIDIOC_STREAMOFF(CAPTURE)" ];
+   }
+
+Querying capabilities
+=
+
+1. To enumerate the set of coded formats supported by the encoder, the
+   client may call :c:func:`VIDIOC_ENUM_FMT` on ``CAPTURE``.
+
+   * The full set of supported formats will be returned, regardless of the
+ format set on ``OUTPUT``.
+
+2. To enumerate the set of supported raw formats, the client may call
+   :c:func:`VIDIOC_ENUM_FMT` on ``OUTPUT``.
+
+   * Only th

[PATCH v2 1/2] media: docs-rst: Document memory-to-memory video decoder interface

2018-10-22 Thread Tomasz Figa
Due to complexity of the video decoding process, the V4L2 drivers of
stateful decoder hardware require specific sequences of V4L2 API calls
to be followed. These include capability enumeration, initialization,
decoding, seek, pause, dynamic resolution change, drain and end of
stream.

Specifics of the above have been discussed during Media Workshops at
LinuxCon Europe 2012 in Barcelona and then later Embedded Linux
Conference Europe 2014 in Düsseldorf. The de facto Codec API that
originated at those events was later implemented by the drivers we already
have merged in mainline, such as s5p-mfc or coda.

The only thing missing was the real specification included as a part of
Linux Media documentation. Fix it now and document the decoder part of
the Codec API.

Signed-off-by: Tomasz Figa 
---
 Documentation/media/uapi/v4l/dev-decoder.rst  | 1082 +
 Documentation/media/uapi/v4l/devices.rst  |1 +
 Documentation/media/uapi/v4l/pixfmt-v4l2.rst  |5 +
 Documentation/media/uapi/v4l/v4l2.rst |   10 +-
 .../media/uapi/v4l/vidioc-decoder-cmd.rst |   40 +-
 Documentation/media/uapi/v4l/vidioc-g-fmt.rst |   14 +
 6 files changed, 1137 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/media/uapi/v4l/dev-decoder.rst

diff --git a/Documentation/media/uapi/v4l/dev-decoder.rst 
b/Documentation/media/uapi/v4l/dev-decoder.rst
new file mode 100644
index ..09c7a6621b8e
--- /dev/null
+++ b/Documentation/media/uapi/v4l/dev-decoder.rst
@@ -0,0 +1,1082 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _decoder:
+
+*
+Memory-to-memory Stateful Video Decoder Interface
+*
+
+A stateful video decoder takes complete chunks of the bitstream (e.g. Annex-B
+H.264/HEVC stream, raw VP8/9 stream) and decodes them into raw video frames in
+display order. The decoder is expected not to require any additional 
information
+from the client to process these buffers.
+
+Performing software parsing, processing etc. of the stream in the driver in
+order to support this interface is strongly discouraged. In case such
+operations are needed, use of the Stateless Video Decoder Interface (in
+development) is strongly advised.
+
+Conventions and notation used in this document
+==
+
+1. The general V4L2 API rules apply if not specified in this document
+   otherwise.
+
+2. The meaning of words “must”, “may”, “should”, etc. is as per RFC
+   2119.
+
+3. All steps not marked “optional” are required.
+
+4. :c:func:`VIDIOC_G_EXT_CTRLS`, :c:func:`VIDIOC_S_EXT_CTRLS` may be used
+   interchangeably with :c:func:`VIDIOC_G_CTRL`, :c:func:`VIDIOC_S_CTRL`,
+   unless specified otherwise.
+
+5. Single-plane API (see spec) and applicable structures may be used
+   interchangeably with Multi-plane API, unless specified otherwise,
+   depending on decoder capabilities and following the general V4L2
+   guidelines.
+
+6. i = [a..b]: sequence of integers from a to b, inclusive, i.e. i =
+   [0..2]: i = 0, 1, 2.
+
+7. Given an ``OUTPUT`` buffer A, A’ represents a buffer on the ``CAPTURE``
+   queue containing data (decoded frame/stream) that resulted from processing
+   buffer A.
+
+.. _decoder-glossary:
+
+Glossary
+
+
+CAPTURE
+   the destination buffer queue; for decoder, the queue of buffers containing
+   decoded frames; for encoder, the queue of buffers containing encoded
+   bitstream; ``V4L2_BUF_TYPE_VIDEO_CAPTURE or
+   ``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE``; data are captured from the hardware
+   into ``CAPTURE`` buffers
+
+client
+   application client communicating with the decoder or encoder implementing
+   this interface
+
+coded format
+   encoded/compressed video bitstream format (e.g. H.264, VP8, etc.); see
+   also: raw format
+
+coded height
+   height for given coded resolution
+
+coded resolution
+   stream resolution in pixels aligned to codec and hardware requirements;
+   typically visible resolution rounded up to full macroblocks;
+   see also: visible resolution
+
+coded width
+   width for given coded resolution
+
+decode order
+   the order in which frames are decoded; may differ from display order if the
+   coded format includes a feature of frame reordering; for decoders,
+   ``OUTPUT`` buffers must be queued by the client in decode order; for
+   encoders ``CAPTURE`` buffers must be returned by the encoder in decode order
+
+destination
+   data resulting from the decode process; ``CAPTURE``
+
+display order
+   the order in which frames must be displayed; for encoders, ``OUTPUT``
+   buffers must be queued by the client in display order; for decoders,
+   ``CAPTURE`` buffers must be returned by the decoder in display order
+
+DPB
+   Decoded Picture Buffer; an H.264 term for a buffer that stores a decoded
+   raw frame available for reference in further decoding steps.
+
+EOS
+   end of stream
+
+IDR
+   Instantaneous Decoder Re

[PATCH v2 0/2] Document memory-to-memory video codec interfaces

2018-10-22 Thread Tomasz Figa
It's been a while, but here is the v2 of the stateful mem2mem codec
interfaces documentation. Sorry for taking so long time to respin.

This series attempts to add the documentation of what was discussed
during Media Workshops at LinuxCon Europe 2012 in Barcelona and then
later Embedded Linux Conference Europe 2014 in Düsseldorf and then
eventually written down by Pawel Osciak and tweaked a bit by Chrome OS
video team (but mostly in a cosmetic way or making the document more
precise), during the several years of Chrome OS using the APIs in
production.

Note that most, if not all, of the API is already implemented in
existing mainline drivers, such as s5p-mfc or mtk-vcodec. Intention of
this series is just to formalize what we already have.

Thanks everyone for the huge amount of useful comments for the RFC and
v1. Much of the credits should go to Pawel Osciak too, for writing most
of the original text of the initial RFC.

Changes since v1:
(https://lore.kernel.org/patchwork/project/lkml/list/?series=360520)
Decoder:
 - Removed a note about querying all combinations of OUTPUT and CAPTURE
   frame sizes, since it would conflict with scaling/composiion support
   to be added later.
 - Removed the source change event after setting non-zero width and
   height on OUTPUT queue, since the change happens as a direct result
   of a client action.
 - Moved all the setup steps for CAPTURE queue out of Initialization
   and Dynamic resolution change into a common sequence called Capture
   setup, since they were mostly duplicate of each other.
 - Described steps to allocate buffers for higher resolution than the
   stream to prepare for future resolution changes.
 - Described a way to skip the initial header parsing and speculatively
   configure the CAPTURE queue (for gstreamer/ffmpeg compatibility).
 - Reordered CAPTURE setup steps so that all the driver queries are done
   first and only then a reconfiguration may be attempted or skipped.
 - Described VIDIOC_CREATE_BUFS as another way of allocating buffers.
 - Made the decoder signal the source change event as soon as the change
   is detected, to reduce pipeline stalls in case of buffers already
   good to continue decoding.
 - Stressed out the fact that a source change may happen even without a
   change in the coded resolution.
 - Described querying pixel aspect ratio using VIDIOC_CROPCAP.
 - Extended documentation of VIDIOC_DECODER_CMD and VIDIOC_G/S/TRY_FMT
   to more precisely describe the behavior of mem2mem decoders.
 - Clarified that 0 width and height are allowed for OUTPUT side of
   mem2mem decoders in the documentation of the v4l2_pix_fmt struct.

Encoder:
 - Removed width and height from CAPTURE (coded) format, since the coded
   resolution of the stream is an internal detail of the encoded stream.
 - Made the VIDIOC_S_FMT on OUTPUT mandatory, since the default format
   normally does not make sense (even if technically valid).
 - Changed the V4L2_SEL_TGT_CROP_BOUNDS and V4L2_SEL_TGT_CROP_DEFAULT
   selection targets to be equal to the full source frame to simplify
   internal handling in drivers for simple hardware.
 - Changed the V4L2_SEL_TGT_COMPOSE_DEFAULT selection target to be equal
   to |crop width|x|crop height|@(0,0) to simplify internal handling in
   drivers for simple hardware.
 - Removed V4L2_SEL_TGT_COMPOSE_PADDED, since the encoder does not write
   to the raw buffers.
 - Extended documentation of VIDIOC_ENCODER_CMD to more precisely
   describe the behavior of mem2mem encoders.
 - Clarified that 0 width and height are allowed for CAPTURE side of
   mem2mem encoders in the documentation of the v4l2_pix_fmt struct.

General:
 - Clarified that the Drain sequence valid only if both queues are
   streaming and stopping any of the queues would abort it, since there
   is nothing to drain, if OUTPUT is stopped and there is no way to
   signal the completion if CAPTURE is stopped.
 - Clarified that VIDIOC_STREAMON on any of the queues would resume the
   codec from stopped state, to be consistent with the documentation of
   VIDIOC_ENCODER/DECODER_CMD.
 - Documented the relation between timestamps of OUTPUT and CAPTURE
   buffers and how special cases of non-1:1 relation are handled.
 - Added missing sizeimage to bitstream format operations and removed
   the mistaken mentions from descriptions of respective REQBUFS calls.
 - Removed the Pause sections, since there is no notion of pause for
   mem2mem devices.
 - Added state machine diagrams.
 - Merged both glossaries into one in the decoder document and a
   reference to it in the encoder document.
 - Added missing terms to the glossary.
 - Added "Stateful" to the interface names.
 - Reworded the text to be more userspace-centric.
 - A number of other readability improvements suggested in review comments.

For changes since RFC see the v1:
https://lore.kernel.org/patchwork/project/lkml/list/?series=360520

Tomasz Figa (2):
  media: docs-rst: Document memory-to-memory video decoder interface

Re: [PATCH] media: rc: cec devices do not have a lirc chardev

2018-10-22 Thread Sean Young
On Mon, Oct 22, 2018 at 01:28:42PM +0100, Sean Young wrote:
> On Mon, Oct 22, 2018 at 12:30:29PM +0100, Hans Verkuil wrote:
> > On 10/22/2018 11:14 AM, Sean Young wrote:
> > > Would you be able to test the following patch please?
> > 
> > Sean,
> > 
> > I think you should be able to test this with the vivid driver. Load the 
> > vivid driver,
> > run:
> > 
> > cec-ctl --tv; cec-ctl -d1 --playback
> > 
> > Then:
> > 
> > cec-ctl -d1 -t0 --user-control-pressed ui-cmd=F5
> 
> Ah, thanks. That will help with testing/reproducing.
>  
> > That said, I tried this, but it doesn't crash for me, but perhaps I need to 
> > run
> > some RC command first...
> 
> Hmm I think those commands should be enough. It probably needs
> CONFIG_DEBUG_SPINLOCK to detect the uninitialized spinlock. I'm trying it now.

Yes, that turned out to work. With CONFIG_DEBUG_SPINLOCK on, it goes bang
every time. With the patch, the problem goes away.

Without CONFIG_DEBUG_SPINLOCK we're going into undefined behaviour, so 
Torbjorn you're only seeing the oops occassionally (and which is why it has
not been observed or reported before).

Thanks,

Sean


Re: [PATCH] media: rc: cec devices do not have a lirc chardev

2018-10-22 Thread Torbjorn Jansson

On 2018-10-22 14:28, Sean Young wrote:

On Mon, Oct 22, 2018 at 12:30:29PM +0100, Hans Verkuil wrote:

On 10/22/2018 11:14 AM, Sean Young wrote:

On Mon, Oct 22, 2018 at 11:44:22AM +0200, Torbjorn Jansson wrote:

On 2018-10-22 10:59, Sean Young wrote:

On Sat, Oct 20, 2018 at 11:12:16PM +0200, Hans Verkuil wrote:

Hi Sean,

Can you take a look at this, it appears to be an RC issue, see my analysis 
below.

On 10/20/2018 03:26 PM, Torbjorn Jansson wrote:

Hello

i'm using the pulse8 usb cec adapter to control my tv.
i have a few scripts that poll the power status of my tv and after a while it 
stops working returning errors when trying to check if tv is on or off.
this i think matches a kernel oops i'm seeing that i suspect is related to this.

i have sometimes been able to recover from this problem by completely cutting 
power to my tv and also unplugging the usb cec adapter.
i have a feeling that the tv is at least partly to blame for cec-ctl not 
working but in any case there shouldn't be a kernel oops.


also every now and then i see this in dmesg:
cec cec0: transmit: failed 05
cec cec0: transmit: failed 06
but that doesn't appear to do any harm as far as i can tell.

any idea whats causing the oops?

the ops:

BUG: unable to handle kernel NULL pointer dereference at 0038
PGD 0 P4D 0
Oops:  [#1] SMP PTI
CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P   OE 
4.18.12-200.fc28.x86_64 #1
Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE-F, BIOS 2.1a 06/15/2018
Workqueue: events pulse8_irq_work_handler [pulse8_cec]
RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0 [rc_core]


Huh. ir_lirc_scancode_event() calls spin_lock_irqsave(&dev->lirc_fh_lock, 
flags);

The spinlock dev->lirc_fh_lock is initialized in ir_lirc_register(), which is 
called
from rc_register_device(), except when the protocol is CEC:

  /* Ensure that the lirc kfifo is setup before we start the thread */
  if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
  rc = ir_lirc_register(dev);
  if (rc < 0)
  goto out_rx;
  }

So it looks like ir_lirc_scancode_event() fails because dev->lirc_fh_lock was 
never
initialized.

Could this be fall-out from the lirc changes you did not too long ago?


Yes, this is broken. My bad, sorry. I think this must have been broken since
v4.16. I can write a patch but I don't have a patch but I'm on the train
to ELCE in Edinburgh now, with no hardware to test on.


Sean



the kernel oops have been happening for a while now.
yesterday when i checked my logs i can see them at least back a couple of
months when i was running 4.17

also my scripts to poll status of my tv and turn it on/off works for a while
so it doesn't crash right away.
maybe it only crashes when i send cec command to turn on/off tv and only
polling for status is no problem.


i think i have a separate issue too because i had problems even before the
kernel oopses started.
but i suspect this is caused by my tv locking up the cec bus because
unplugging power to tv for a few minutes (i must wait or it will still be
just as broken) and then back used to resolve the cec errors from my
scripts.



Would you be able to test the following patch please?


Sean,

I think you should be able to test this with the vivid driver. Load the vivid 
driver,
run:

cec-ctl --tv; cec-ctl -d1 --playback

Then:

cec-ctl -d1 -t0 --user-control-pressed ui-cmd=F5


Ah, thanks. That will help with testing/reproducing.
  

That said, I tried this, but it doesn't crash for me, but perhaps I need to run
some RC command first...


Hmm I think those commands should be enough. It probably needs
CONFIG_DEBUG_SPINLOCK to detect the uninitialized spinlock. I'm trying it now.

Thanks,

Sean



FYI the commands i run is as follows.

getting status, this is run frequently several times per minute (once every 10 
or 15 seconds):

cec-ctl --to=0 --give-device-power-status

then when i want to power on or off this is run:
cec-ctl --to=0 --image-view-on
or
cec-ctl --to=0 --standby


it usually takes day or two before i get a kernel oops.
but i haven't studied super closely when exactly the oops happens in relation 
to what commands i sent since by now I'm used to it not working most of the time.




Re: i.MX6 MIPI-CSI2 OV5640 Camera testing on Mainline Linux

2018-10-22 Thread Fabio Estevam
Hi Adam,

On Mon, Oct 22, 2018 at 9:37 AM Adam Ford  wrote:

> Thank you!  This tutorial web site is exactly what I need.  The
> documentation page in Linux touched on the media-ctl links, but it
> didn't explain the syntax or the mapping.  This graphical
> interpretation really helps it make more sense.

Is capturing working well on your i.MX6 board now?


Re: [PATCH] media: rc: cec devices do not have a lirc chardev

2018-10-22 Thread Sean Young
On Mon, Oct 22, 2018 at 12:30:29PM +0100, Hans Verkuil wrote:
> On 10/22/2018 11:14 AM, Sean Young wrote:
> > On Mon, Oct 22, 2018 at 11:44:22AM +0200, Torbjorn Jansson wrote:
> >> On 2018-10-22 10:59, Sean Young wrote:
> >>> On Sat, Oct 20, 2018 at 11:12:16PM +0200, Hans Verkuil wrote:
>  Hi Sean,
> 
>  Can you take a look at this, it appears to be an RC issue, see my 
>  analysis below.
> 
>  On 10/20/2018 03:26 PM, Torbjorn Jansson wrote:
> > Hello
> >
> > i'm using the pulse8 usb cec adapter to control my tv.
> > i have a few scripts that poll the power status of my tv and after a 
> > while it stops working returning errors when trying to check if tv is 
> > on or off.
> > this i think matches a kernel oops i'm seeing that i suspect is related 
> > to this.
> >
> > i have sometimes been able to recover from this problem by completely 
> > cutting power to my tv and also unplugging the usb cec adapter.
> > i have a feeling that the tv is at least partly to blame for cec-ctl 
> > not working but in any case there shouldn't be a kernel oops.
> >
> >
> > also every now and then i see this in dmesg:
> > cec cec0: transmit: failed 05
> > cec cec0: transmit: failed 06
> > but that doesn't appear to do any harm as far as i can tell.
> >
> > any idea whats causing the oops?
> >
> > the ops:
> >
> > BUG: unable to handle kernel NULL pointer dereference at 
> > 0038
> > PGD 0 P4D 0
> > Oops:  [#1] SMP PTI
> > CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P   OE 
> > 4.18.12-200.fc28.x86_64 #1
> > Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE-F, BIOS 2.1a 06/15/2018
> > Workqueue: events pulse8_irq_work_handler [pulse8_cec]
> > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0 [rc_core]
> 
>  Huh. ir_lirc_scancode_event() calls 
>  spin_lock_irqsave(&dev->lirc_fh_lock, flags);
> 
>  The spinlock dev->lirc_fh_lock is initialized in ir_lirc_register(), 
>  which is called
>  from rc_register_device(), except when the protocol is CEC:
> 
>   /* Ensure that the lirc kfifo is setup before we start the 
>  thread */
>   if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
>   rc = ir_lirc_register(dev);
>   if (rc < 0)
>   goto out_rx;
>   }
> 
>  So it looks like ir_lirc_scancode_event() fails because 
>  dev->lirc_fh_lock was never
>  initialized.
> 
>  Could this be fall-out from the lirc changes you did not too long ago?
> >>>
> >>> Yes, this is broken. My bad, sorry. I think this must have been broken 
> >>> since
> >>> v4.16. I can write a patch but I don't have a patch but I'm on the train
> >>> to ELCE in Edinburgh now, with no hardware to test on.
> >>>
> >>>
> >>> Sean
> >>>
> >>
> >> the kernel oops have been happening for a while now.
> >> yesterday when i checked my logs i can see them at least back a couple of
> >> months when i was running 4.17
> >>
> >> also my scripts to poll status of my tv and turn it on/off works for a 
> >> while
> >> so it doesn't crash right away.
> >> maybe it only crashes when i send cec command to turn on/off tv and only
> >> polling for status is no problem.
> >>
> >>
> >> i think i have a separate issue too because i had problems even before the
> >> kernel oopses started.
> >> but i suspect this is caused by my tv locking up the cec bus because
> >> unplugging power to tv for a few minutes (i must wait or it will still be
> >> just as broken) and then back used to resolve the cec errors from my
> >> scripts.
> > 
> > 
> > Would you be able to test the following patch please?
> 
> Sean,
> 
> I think you should be able to test this with the vivid driver. Load the vivid 
> driver,
> run:
> 
> cec-ctl --tv; cec-ctl -d1 --playback
> 
> Then:
> 
> cec-ctl -d1 -t0 --user-control-pressed ui-cmd=F5

Ah, thanks. That will help with testing/reproducing.
 
> That said, I tried this, but it doesn't crash for me, but perhaps I need to 
> run
> some RC command first...

Hmm I think those commands should be enough. It probably needs
CONFIG_DEBUG_SPINLOCK to detect the uninitialized spinlock. I'm trying it now.

Thanks,

Sean


Re: i.MX6 MIPI-CSI2 OV5640 Camera testing on Mainline Linux

2018-10-22 Thread Adam Ford
On Mon, Oct 22, 2018 at 6:33 AM jacopo mondi  wrote:
>
> Hi Adam,
>
> On Fri, Oct 19, 2018 at 02:42:56PM -0500, Adam Ford wrote:
> > On Fri, Oct 19, 2018 at 8:45 AM Adam Ford  wrote:
> > >
> > > On Wed, Oct 17, 2018 at 3:01 AM jacopo mondi  wrote:
> > > >
> > > > Hi Adam, Seve,
> > > >
> > > > On Tue, Oct 16, 2018 at 05:13:24PM -0700, Steve Longerbeam wrote:
> > > > > Hi Adam,
> > > > >
> > > > >
> > > > > On 10/16/18 12:46 PM, Adam Ford wrote:
> > > > > >On Thu, Sep 20, 2018 at 9:58 AM jacopo mondi  
> > > > > >wrote:
> > > > > >>Hi imx6 people,
> > > > > >>
> > > > > >>On Thu, May 31, 2018 at 08:39:20PM +0530, Jagan Teki wrote:
> > > > > >>>Hi All,
> > > > > >>>
> > > > > >>>I'm trying to verify MIPI-CSI2 OV5640 camera on i.MX6 platform with
> > > > > >>>Mainline Linux.
> > > > > >>Sorry to resurect this, but before diving deep into details, do 
> > > > > >>anyone
> > > > > >>of you verified JPEG capture with ov5640 and i.MX6 platforms, and 
> > > > > >>has
> > > > > >>maybe a pipeline configuration to share :) ?
> > > > > >
> > > > > >I have a 4.14 kernel for my i.MX6D/Q using an ov5640 connected in a
> > > > > >similar way as the sabresd and I'm getting similar timeouts.
> > > > > >when executing
> > > > > >
> > > > > >media-ctl -l "'ov5640 2-0010':0 -> 'imx6-mipi-csi2':0[1]"
> > > > > >media-ctl -l "'imx6-mipi-csi2':2 -> 'ipu1_csi1':0[1]"
> > > > >
> > > > >
> > > > > You're routing through imx6-mipi-csi2 pad 2, which is CSI-2 virtual
> > > > > channel 1, so make sure the ov5640 is transmitting on that channel,
> > > > > see virtual_channel module parameter.
> > >
> > > First, I want to apologize for the spam.  I don't normally want to ask
> > > for hand-holding, but after spending 4 solid days on this, I'm getting
> > > frustrated, and I've tried to read the instructions, and the technical
> > > reference manual is huge and somewhat overwhelming.
> > >
> > > Once I get my hardware working and I develop a better understanding of
> > > how this system works, I'll be more than happy to volunteer to help
> > > test patches on my hardware.
> > >
> > > I am not sure I fully understand how the media-ctl handles the
> > > routing.  I just basically copied what I could find from some
> > > documentation.  I looked for some documentation but I wasn't able to
> > > find much.  Maybe you can point me to some.
> > >
> > > I can share with you some of my device tree, and I'll try to explain
> > > my connections.   Firstly, I have an i.MX6Q and i.MX6Q which share the
> > > same device tree.
> > >
> > > The CSI pins on the OV5640 camera go to i.MX6 pins:
> > > CSI_CLK0M / CSI_CLK0P,
> > > CSI_D0M / CSI_D0P,
> > > CSI_D1M / CSI_D1P,
> > >
> > > CSI_D2 and D3 pins on the processor are all floating, and CSI_REXT is
> > > grounded through a 6.04k pull-down resistor.
> > >
> > > I am not sure if these technically translate to CSI0, CSI1, or CSI2,
> > > but I assumed the CSI2 since that's how the SabreSD board appears to
> > > work.
> > >
> > > The ov5640 is connected to i2c3 with the following tree entry:
> > >
> > > ov5640: camera@10 {
> > > compatible = "ovti,ov5640";
> > > pinctrl-names = "default";
> > > pinctrl-0 = <&pinctrl_ov5640>;
> > > reg = <0x10>;
> > > clocks = <&clks IMX6QDL_CLK_CKO>;
> > > clock-names = "xclk";
> > > DOVDD-supply = <&mipi_pwr>;
> > > AVDD-supply = <&mipi_pwr>;
> > > DVDD-supply = <&mipi_pwr>;
> > > reset-gpios = <&gpio3 26 GPIO_ACTIVE_LOW>;
> > > powerdown-gpios = <&gpio3 27 GPIO_ACTIVE_HIGH>;
> > >
> > > port {
> > > ov5640_to_mipi_csi2: endpoint {
> > > remote-endpoint = <&mipi_csi2_in>;
> > > clock-lanes = <0>;
> > > data-lanes = <1 2>;
> > > };
> > > };
> > >
> > > I will be the first person to admit, I don't understand how the
> > > clock-lands and data-lanes interact with the mipi_csis and the camera,
> > > but I tried to match the sabresd board device tree.
> > >
> > > For the MIPI_CSI interface, I wasn't sure which ports are the proper
> > > reference.  Looking at the sabresd board,  I used it as an example.  I
> > > wasn't sure if port0 and reg 0 were the right options.
> > > &mipi_csi {
> > > status = "okay";
> > >
> > > port@0 {
> > > reg = <0>;
> > > mipi_csi2_in: endpoint {
> > > remote-endpoint = <&ov5640_to_mipi_csi2>;
> > > clock-lanes = <0>;
> > > data-lanes = <1 2>;
> > > };
> > > };
> > > };
> > >
> > > There was one section of the sabresd board that I wasn't sure I
> > > needed, because I am new to this camera stuff.  I wasn't thinking I
> > > needed it, but I copied it  because the sabresd board had it.  I know
> > > it has two cameras, but the interaction between the csi interface and
> > > the ipu isn't clear to me.
> > >
> > > &ipu1_csi1_from_mipi_vc1 {
> > > clock-lanes = <0>;
> > > data-lanes = <1 2>;
> > > };
> > >
> > >
> > > I am not 100% certain the following is correct, but I tried to disabl

[PATCH v2] media: venus: handle peak bitrate set property

2018-10-22 Thread Malathi Gottam
Max bitrate property is not supported for venus version 4xx.
Return unsupported from packetization layer. Handle it in 
hfi_venus layer to exit gracefully to venc layer.

Signed-off-by: Malathi Gottam 
---
 drivers/media/platform/qcom/venus/hfi_cmds.c  | 2 +-
 drivers/media/platform/qcom/venus/hfi_venus.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c 
b/drivers/media/platform/qcom/venus/hfi_cmds.c
index e8389d8..87a4414 100644
--- a/drivers/media/platform/qcom/venus/hfi_cmds.c
+++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
@@ -1215,7 +1215,7 @@ static int pkt_session_set_property_1x(struct 
hfi_session_set_property_pkt *pkt,
}
case HFI_PROPERTY_CONFIG_VENC_MAX_BITRATE:
/* not implemented on Venus 4xx */
-   break;
+   return -ENOTSUPP;
default:
return pkt_session_set_property_3xx(pkt, cookie, ptype, pdata);
}
diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c 
b/drivers/media/platform/qcom/venus/hfi_venus.c
index 1240855..9d086b9 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -1355,6 +1355,8 @@ static int venus_session_set_property(struct venus_inst 
*inst, u32 ptype,
pkt = (struct hfi_session_set_property_pkt *)packet;
 
ret = pkt_session_set_property(pkt, inst, ptype, pdata);
+   if (ret == -ENOTSUPP)
+   return 0;
if (ret)
return ret;
 
-- 
1.9.1



Re: [PATCH] media: venus: handle peak bitrate set property

2018-10-22 Thread mgottam

On 2018-10-09 20:29, Stanimir Varbanov wrote:

Hi Malathi,

Thanks for the patch!

On 10/09/2018 10:51 AM, Malathi Gottam wrote:

Max bitrate property is not supported for venus version 4xx.
Add a version check for the same.


I'd like to avoid version checks in this layer of the driver. Could 
just

black-list this property in pkt_session_set_property_4xx? Hint, see
HFI_PROPERTY_CONFIG_VENC_MAX_BITRATE in the same function.



Signed-off-by: Malathi Gottam 
---
 drivers/media/platform/qcom/venus/venc.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/venc.c 
b/drivers/media/platform/qcom/venus/venc.c

index ef11495..3f50cd0 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -757,18 +757,20 @@ static int venc_set_properties(struct venus_inst 
*inst)

if (ret)
return ret;

-   if (!ctr->bitrate_peak)
-   bitrate *= 2;
-   else
-   bitrate = ctr->bitrate_peak;
+   if (!IS_V4(inst->core)) {
+   if (!ctr->bitrate_peak)
+   bitrate *= 2;
+   else
+   bitrate = ctr->bitrate_peak;

-   ptype = HFI_PROPERTY_CONFIG_VENC_MAX_BITRATE;
-   brate.bitrate = bitrate;
-   brate.layer_id = 0;
+   ptype = HFI_PROPERTY_CONFIG_VENC_MAX_BITRATE;
+   brate.bitrate = bitrate;
+   brate.layer_id = 0;

-   ret = hfi_session_set_property(inst, ptype, &brate);
-   if (ret)
-   return ret;
+   ret = hfi_session_set_property(inst, ptype, &brate);
+   if (ret)
+   return ret;
+   }

if (inst->fmt_cap->pixfmt == V4L2_PIX_FMT_H264) {
profile = venc_v4l2_to_hfi(V4L2_CID_MPEG_VIDEO_H264_PROFILE,


Hi Stan,

Though this property is bypassed in the function 
"pkt_session_set_property_4xx", it is set to firmware in hfi_venus 
layer.


So we can return -ENOTSUPP from packet layer. If hfi_venus layer 
receives error as ENOTSUPP, treat it as normal and return 0 to venc 
layer.


I will post the updated patch, with this implementation.


Re: i.MX6 MIPI-CSI2 OV5640 Camera testing on Mainline Linux

2018-10-22 Thread jacopo mondi
Hi Adam,

On Fri, Oct 19, 2018 at 02:42:56PM -0500, Adam Ford wrote:
> On Fri, Oct 19, 2018 at 8:45 AM Adam Ford  wrote:
> >
> > On Wed, Oct 17, 2018 at 3:01 AM jacopo mondi  wrote:
> > >
> > > Hi Adam, Seve,
> > >
> > > On Tue, Oct 16, 2018 at 05:13:24PM -0700, Steve Longerbeam wrote:
> > > > Hi Adam,
> > > >
> > > >
> > > > On 10/16/18 12:46 PM, Adam Ford wrote:
> > > > >On Thu, Sep 20, 2018 at 9:58 AM jacopo mondi  wrote:
> > > > >>Hi imx6 people,
> > > > >>
> > > > >>On Thu, May 31, 2018 at 08:39:20PM +0530, Jagan Teki wrote:
> > > > >>>Hi All,
> > > > >>>
> > > > >>>I'm trying to verify MIPI-CSI2 OV5640 camera on i.MX6 platform with
> > > > >>>Mainline Linux.
> > > > >>Sorry to resurect this, but before diving deep into details, do anyone
> > > > >>of you verified JPEG capture with ov5640 and i.MX6 platforms, and has
> > > > >>maybe a pipeline configuration to share :) ?
> > > > >
> > > > >I have a 4.14 kernel for my i.MX6D/Q using an ov5640 connected in a
> > > > >similar way as the sabresd and I'm getting similar timeouts.
> > > > >when executing
> > > > >
> > > > >media-ctl -l "'ov5640 2-0010':0 -> 'imx6-mipi-csi2':0[1]"
> > > > >media-ctl -l "'imx6-mipi-csi2':2 -> 'ipu1_csi1':0[1]"
> > > >
> > > >
> > > > You're routing through imx6-mipi-csi2 pad 2, which is CSI-2 virtual
> > > > channel 1, so make sure the ov5640 is transmitting on that channel,
> > > > see virtual_channel module parameter.
> >
> > First, I want to apologize for the spam.  I don't normally want to ask
> > for hand-holding, but after spending 4 solid days on this, I'm getting
> > frustrated, and I've tried to read the instructions, and the technical
> > reference manual is huge and somewhat overwhelming.
> >
> > Once I get my hardware working and I develop a better understanding of
> > how this system works, I'll be more than happy to volunteer to help
> > test patches on my hardware.
> >
> > I am not sure I fully understand how the media-ctl handles the
> > routing.  I just basically copied what I could find from some
> > documentation.  I looked for some documentation but I wasn't able to
> > find much.  Maybe you can point me to some.
> >
> > I can share with you some of my device tree, and I'll try to explain
> > my connections.   Firstly, I have an i.MX6Q and i.MX6Q which share the
> > same device tree.
> >
> > The CSI pins on the OV5640 camera go to i.MX6 pins:
> > CSI_CLK0M / CSI_CLK0P,
> > CSI_D0M / CSI_D0P,
> > CSI_D1M / CSI_D1P,
> >
> > CSI_D2 and D3 pins on the processor are all floating, and CSI_REXT is
> > grounded through a 6.04k pull-down resistor.
> >
> > I am not sure if these technically translate to CSI0, CSI1, or CSI2,
> > but I assumed the CSI2 since that's how the SabreSD board appears to
> > work.
> >
> > The ov5640 is connected to i2c3 with the following tree entry:
> >
> > ov5640: camera@10 {
> > compatible = "ovti,ov5640";
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_ov5640>;
> > reg = <0x10>;
> > clocks = <&clks IMX6QDL_CLK_CKO>;
> > clock-names = "xclk";
> > DOVDD-supply = <&mipi_pwr>;
> > AVDD-supply = <&mipi_pwr>;
> > DVDD-supply = <&mipi_pwr>;
> > reset-gpios = <&gpio3 26 GPIO_ACTIVE_LOW>;
> > powerdown-gpios = <&gpio3 27 GPIO_ACTIVE_HIGH>;
> >
> > port {
> > ov5640_to_mipi_csi2: endpoint {
> > remote-endpoint = <&mipi_csi2_in>;
> > clock-lanes = <0>;
> > data-lanes = <1 2>;
> > };
> > };
> >
> > I will be the first person to admit, I don't understand how the
> > clock-lands and data-lanes interact with the mipi_csis and the camera,
> > but I tried to match the sabresd board device tree.
> >
> > For the MIPI_CSI interface, I wasn't sure which ports are the proper
> > reference.  Looking at the sabresd board,  I used it as an example.  I
> > wasn't sure if port0 and reg 0 were the right options.
> > &mipi_csi {
> > status = "okay";
> >
> > port@0 {
> > reg = <0>;
> > mipi_csi2_in: endpoint {
> > remote-endpoint = <&ov5640_to_mipi_csi2>;
> > clock-lanes = <0>;
> > data-lanes = <1 2>;
> > };
> > };
> > };
> >
> > There was one section of the sabresd board that I wasn't sure I
> > needed, because I am new to this camera stuff.  I wasn't thinking I
> > needed it, but I copied it  because the sabresd board had it.  I know
> > it has two cameras, but the interaction between the csi interface and
> > the ipu isn't clear to me.
> >
> > &ipu1_csi1_from_mipi_vc1 {
> > clock-lanes = <0>;
> > data-lanes = <1 2>;
> > };
> >
> >
> > I am not 100% certain the following is correct, but I tried to disable
> > unwanted features to help save power, but it's quite possible it's
> > interfering with the settings i have above.
> >
> > &ipu1_csi0 {
> > status = "disabled";
> > };
> >
> > &ipu2_csi0 {
> > status = "disabled";
> > };
> >
> > &mipi_dsi {
> > status = "disabled";
> > };
> >
> >
> > > >
> > > >
> > > > >media-ctl -l "'i

Re: [PATCH] media: rc: cec devices do not have a lirc chardev

2018-10-22 Thread Hans Verkuil
On 10/22/2018 11:14 AM, Sean Young wrote:
> On Mon, Oct 22, 2018 at 11:44:22AM +0200, Torbjorn Jansson wrote:
>> On 2018-10-22 10:59, Sean Young wrote:
>>> On Sat, Oct 20, 2018 at 11:12:16PM +0200, Hans Verkuil wrote:
 Hi Sean,

 Can you take a look at this, it appears to be an RC issue, see my analysis 
 below.

 On 10/20/2018 03:26 PM, Torbjorn Jansson wrote:
> Hello
>
> i'm using the pulse8 usb cec adapter to control my tv.
> i have a few scripts that poll the power status of my tv and after a 
> while it stops working returning errors when trying to check if tv is on 
> or off.
> this i think matches a kernel oops i'm seeing that i suspect is related 
> to this.
>
> i have sometimes been able to recover from this problem by completely 
> cutting power to my tv and also unplugging the usb cec adapter.
> i have a feeling that the tv is at least partly to blame for cec-ctl not 
> working but in any case there shouldn't be a kernel oops.
>
>
> also every now and then i see this in dmesg:
> cec cec0: transmit: failed 05
> cec cec0: transmit: failed 06
> but that doesn't appear to do any harm as far as i can tell.
>
> any idea whats causing the oops?
>
> the ops:
>
> BUG: unable to handle kernel NULL pointer dereference at 0038
> PGD 0 P4D 0
> Oops:  [#1] SMP PTI
> CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P   OE 
> 4.18.12-200.fc28.x86_64 #1
> Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE-F, BIOS 2.1a 06/15/2018
> Workqueue: events pulse8_irq_work_handler [pulse8_cec]
> RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0 [rc_core]

 Huh. ir_lirc_scancode_event() calls spin_lock_irqsave(&dev->lirc_fh_lock, 
 flags);

 The spinlock dev->lirc_fh_lock is initialized in ir_lirc_register(), which 
 is called
 from rc_register_device(), except when the protocol is CEC:

  /* Ensure that the lirc kfifo is setup before we start the thread 
 */
  if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
  rc = ir_lirc_register(dev);
  if (rc < 0)
  goto out_rx;
  }

 So it looks like ir_lirc_scancode_event() fails because dev->lirc_fh_lock 
 was never
 initialized.

 Could this be fall-out from the lirc changes you did not too long ago?
>>>
>>> Yes, this is broken. My bad, sorry. I think this must have been broken since
>>> v4.16. I can write a patch but I don't have a patch but I'm on the train
>>> to ELCE in Edinburgh now, with no hardware to test on.
>>>
>>>
>>> Sean
>>>
>>
>> the kernel oops have been happening for a while now.
>> yesterday when i checked my logs i can see them at least back a couple of
>> months when i was running 4.17
>>
>> also my scripts to poll status of my tv and turn it on/off works for a while
>> so it doesn't crash right away.
>> maybe it only crashes when i send cec command to turn on/off tv and only
>> polling for status is no problem.
>>
>>
>> i think i have a separate issue too because i had problems even before the
>> kernel oopses started.
>> but i suspect this is caused by my tv locking up the cec bus because
>> unplugging power to tv for a few minutes (i must wait or it will still be
>> just as broken) and then back used to resolve the cec errors from my
>> scripts.
> 
> 
> Would you be able to test the following patch please?

Sean,

I think you should be able to test this with the vivid driver. Load the vivid 
driver,
run:

cec-ctl --tv; cec-ctl -d1 --playback

Then:

cec-ctl -d1 -t0 --user-control-pressed ui-cmd=F5

That said, I tried this, but it doesn't crash for me, but perhaps I need to run
some RC command first...

Regards,

Hans

> 
> Thanks,
> 
> Sean
> ---
> From 1b8b20b606b30c0e301c80e18af8d77194269bc1 Mon Sep 17 00:00:00 2001
> From: Sean Young 
> Date: Mon, 22 Oct 2018 10:01:50 +0100
> Subject: [PATCH] media: rc: cec devices do not have a lirc chardev
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> This fixes an oops in ir_lirc_scancode_event().
> 
> BUG: unable to handle kernel NULL pointer dereference at 0038
> PGD 0 P4D 0
> Oops:  [#1] SMP PTI
> CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P   OE 
> 4.18.12-200.fc28.x86_64 #1
> Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE-F, BIOS 2.1a 06/15/2018
> Workqueue: events pulse8_irq_work_handler [pulse8_cec]
> RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0 [rc_core]
> Code: 8d ae b4 07 00 00 49 81 c6 b8 07 00 00 53 e8 4a df c3 d5 48 89 ef 49 89 
> 45 00 e8 4e 84 41 d6 49 8b 1e 49 89 c4 4c 39 f3 74 58 <8b> 43 38 8b 53 40 89 
> c1 2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
> RSP: 0018:aa10e3c07d58 EFLAGS: 00010017
> RAX: 0002 RBX:  RCX: 0018
> RDX: 0001 RSI: 00316

Re: [PATCH v12 0/5] Venus updates - PIL

2018-10-22 Thread Vikash Garodia

Thanks Alex.

On 2018-10-22 11:21, Alexandre Courbot wrote:

Hi Vikash,

On Wed, Oct 17, 2018 at 10:18 PM Vikash Garodia 
 wrote:


This version of the series
* updates the tz flag to unsigned

Stanimir Varbanov (1):
  venus: firmware: register separate platform_device for firmware 
loader


Vikash Garodia (4):
  venus: firmware: add routine to reset ARM9
  venus: firmware: move load firmware in a separate function
  venus: firmware: add no TZ boot and shutdown routine
  dt-bindings: media: Document bindings for venus firmware device

 .../devicetree/bindings/media/qcom,venus.txt   |  14 +-
 drivers/media/platform/qcom/venus/core.c   |  24 ++-
 drivers/media/platform/qcom/venus/core.h   |   6 +
 drivers/media/platform/qcom/venus/firmware.c   | 235 
+++--

 drivers/media/platform/qcom/venus/firmware.h   |  17 +-
 drivers/media/platform/qcom/venus/hfi_venus.c  |  13 +-
 drivers/media/platform/qcom/venus/hfi_venus_io.h   |   8 +
 7 files changed, 274 insertions(+), 43 deletions(-)


The series:

Reviewed-by: Alexandre Courbot 
Tested-by: Alexandre Courbot 


[PATCH] media: rc: cec devices do not have a lirc chardev

2018-10-22 Thread Sean Young
On Mon, Oct 22, 2018 at 11:44:22AM +0200, Torbjorn Jansson wrote:
> On 2018-10-22 10:59, Sean Young wrote:
> > On Sat, Oct 20, 2018 at 11:12:16PM +0200, Hans Verkuil wrote:
> > > Hi Sean,
> > > 
> > > Can you take a look at this, it appears to be an RC issue, see my 
> > > analysis below.
> > > 
> > > On 10/20/2018 03:26 PM, Torbjorn Jansson wrote:
> > > > Hello
> > > > 
> > > > i'm using the pulse8 usb cec adapter to control my tv.
> > > > i have a few scripts that poll the power status of my tv and after a 
> > > > while it stops working returning errors when trying to check if tv is 
> > > > on or off.
> > > > this i think matches a kernel oops i'm seeing that i suspect is related 
> > > > to this.
> > > > 
> > > > i have sometimes been able to recover from this problem by completely 
> > > > cutting power to my tv and also unplugging the usb cec adapter.
> > > > i have a feeling that the tv is at least partly to blame for cec-ctl 
> > > > not working but in any case there shouldn't be a kernel oops.
> > > > 
> > > > 
> > > > also every now and then i see this in dmesg:
> > > > cec cec0: transmit: failed 05
> > > > cec cec0: transmit: failed 06
> > > > but that doesn't appear to do any harm as far as i can tell.
> > > > 
> > > > any idea whats causing the oops?
> > > > 
> > > > the ops:
> > > > 
> > > > BUG: unable to handle kernel NULL pointer dereference at 
> > > > 0038
> > > > PGD 0 P4D 0
> > > > Oops:  [#1] SMP PTI
> > > > CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P   OE 
> > > > 4.18.12-200.fc28.x86_64 #1
> > > > Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE-F, BIOS 2.1a 06/15/2018
> > > > Workqueue: events pulse8_irq_work_handler [pulse8_cec]
> > > > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0 [rc_core]
> > > 
> > > Huh. ir_lirc_scancode_event() calls spin_lock_irqsave(&dev->lirc_fh_lock, 
> > > flags);
> > > 
> > > The spinlock dev->lirc_fh_lock is initialized in ir_lirc_register(), 
> > > which is called
> > > from rc_register_device(), except when the protocol is CEC:
> > > 
> > >  /* Ensure that the lirc kfifo is setup before we start the 
> > > thread */
> > >  if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
> > >  rc = ir_lirc_register(dev);
> > >  if (rc < 0)
> > >  goto out_rx;
> > >  }
> > > 
> > > So it looks like ir_lirc_scancode_event() fails because dev->lirc_fh_lock 
> > > was never
> > > initialized.
> > > 
> > > Could this be fall-out from the lirc changes you did not too long ago?
> > 
> > Yes, this is broken. My bad, sorry. I think this must have been broken since
> > v4.16. I can write a patch but I don't have a patch but I'm on the train
> > to ELCE in Edinburgh now, with no hardware to test on.
> > 
> > 
> > Sean
> > 
> 
> the kernel oops have been happening for a while now.
> yesterday when i checked my logs i can see them at least back a couple of
> months when i was running 4.17
> 
> also my scripts to poll status of my tv and turn it on/off works for a while
> so it doesn't crash right away.
> maybe it only crashes when i send cec command to turn on/off tv and only
> polling for status is no problem.
> 
> 
> i think i have a separate issue too because i had problems even before the
> kernel oopses started.
> but i suspect this is caused by my tv locking up the cec bus because
> unplugging power to tv for a few minutes (i must wait or it will still be
> just as broken) and then back used to resolve the cec errors from my
> scripts.


Would you be able to test the following patch please?

Thanks,

Sean
---
>From 1b8b20b606b30c0e301c80e18af8d77194269bc1 Mon Sep 17 00:00:00 2001
From: Sean Young 
Date: Mon, 22 Oct 2018 10:01:50 +0100
Subject: [PATCH] media: rc: cec devices do not have a lirc chardev
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This fixes an oops in ir_lirc_scancode_event().

BUG: unable to handle kernel NULL pointer dereference at 0038
PGD 0 P4D 0
Oops:  [#1] SMP PTI
CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P   OE 
4.18.12-200.fc28.x86_64 #1
Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE-F, BIOS 2.1a 06/15/2018
Workqueue: events pulse8_irq_work_handler [pulse8_cec]
RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0 [rc_core]
Code: 8d ae b4 07 00 00 49 81 c6 b8 07 00 00 53 e8 4a df c3 d5 48 89 ef 49 89 
45 00 e8 4e 84 41 d6 49 8b 1e 49 89 c4 4c 39 f3 74 58 <8b> 43 38 8b 53 40 89 c1 
2b 4b 3c 39 ca 72 41 21 d0 49 8b 7d 00 49
RSP: 0018:aa10e3c07d58 EFLAGS: 00010017
RAX: 0002 RBX:  RCX: 0018
RDX: 0001 RSI: 00316245397fa93c RDI: 966d31c8d7b4
RBP: 966d31c8d7b4 R08:  R09: 
R10: 0003 R11: aa10e3c07e28 R12: 0002
R13: aa10e3c07d88 R14: 966d31c8d7b8 R15: 0073
FS:  () GS:966d3f44() knlGS:
CS:  0010 DS:  E

Re: [RFP] Which V4L2 ioctls could be replaced by better versions?

2018-10-22 Thread Dave Stevenson
Hi Tomasz

On Mon, 22 Oct 2018 at 04:38, Tomasz Figa  wrote:
>
> Hi Philipp,
>
> On Mon, Oct 22, 2018 at 1:28 AM Philipp Zabel  wrote:
> >
> > On Wed, Oct 03, 2018 at 05:24:39PM +0900, Tomasz Figa wrote:
> > [...]
> > > > Yes, but that would fall in a complete redesign I guess. The buffer
> > > > allocation scheme is very inflexible. You can't have buffers of two
> > > > dimensions allocated at the same time for the same queue. Worst, you
> > > > cannot leave even 1 buffer as your scannout buffer while reallocating
> > > > new buffers, this is not permitted by the framework (in software). As a
> > > > side effect, there is no way to optimize the resolution changes, you
> > > > even have to copy your scannout buffer on the CPU, to free it in order
> > > > to proceed. Resolution changes are thus painfully slow, by design.
> > [...]
> > > Also, I fail to understand the scanout issue. If one exports a vb2
> > > buffer to a DMA-buf and import it to the scanout engine, it can keep
> > > scanning out from it as long as it want, because the DMA-buf will hold
> > > a reference on the buffer, even if it's removed from the vb2 queue.
> >
> > REQBUFS 0 fails if the vb2 buffer is still in use, including from dmabuf
> > attachments: vb2_buffer_in_use checks the num_users memop. The refcount
> > returned by num_users shared between the vmarea handler and dmabuf ops,
> > so any dmabuf attachment counts towards in_use.
>
> Ah, right. I've managed to completely forget about it, since we have a
> downstream patch that we attempted to upstream earlier [1], but didn't
> have a chance to follow up on the comments and there wasn't much
> interest in it in general.
>
> [1] https://lore.kernel.org/patchwork/patch/607853/
>
> Perhaps it would be worth reviving?

There's been recent interest in this from the LibreElec folk as they
are wanting to shift to using V4L2 M2M for codecs on as many platforms
as possible. They'll use it wrapped by FFmpeg, and they're working on
patches to get FFmpeg exporting dmabufs to be consumed by DRM.

Hans had pointed me at your patch, and I've been using it to get
around the issue.
I did start writing the requested docs changes [1], but I'm afraid
upstreaming stuff has been a low priority for me recently. If that
patch suffices, then feel free to pick it up. Otherwise I'll do so
when I get some time (likely to be a fair number of weeks away).

  Dave

[1] 
https://github.com/6by9/linux/commit/09619d9427d9d44ae5e72e0e85e64cb3ea9727da


Re: cec kernel oops with pulse8 usb cec adapter

2018-10-22 Thread Torbjorn Jansson

On 2018-10-22 10:59, Sean Young wrote:

On Sat, Oct 20, 2018 at 11:12:16PM +0200, Hans Verkuil wrote:

Hi Sean,

Can you take a look at this, it appears to be an RC issue, see my analysis 
below.

On 10/20/2018 03:26 PM, Torbjorn Jansson wrote:

Hello

i'm using the pulse8 usb cec adapter to control my tv.
i have a few scripts that poll the power status of my tv and after a while it 
stops working returning errors when trying to check if tv is on or off.
this i think matches a kernel oops i'm seeing that i suspect is related to this.

i have sometimes been able to recover from this problem by completely cutting 
power to my tv and also unplugging the usb cec adapter.
i have a feeling that the tv is at least partly to blame for cec-ctl not 
working but in any case there shouldn't be a kernel oops.


also every now and then i see this in dmesg:
cec cec0: transmit: failed 05
cec cec0: transmit: failed 06
but that doesn't appear to do any harm as far as i can tell.

any idea whats causing the oops?

the ops:

BUG: unable to handle kernel NULL pointer dereference at 0038
PGD 0 P4D 0
Oops:  [#1] SMP PTI
CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P   OE 
4.18.12-200.fc28.x86_64 #1
Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE-F, BIOS 2.1a 06/15/2018
Workqueue: events pulse8_irq_work_handler [pulse8_cec]
RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0 [rc_core]


Huh. ir_lirc_scancode_event() calls spin_lock_irqsave(&dev->lirc_fh_lock, 
flags);

The spinlock dev->lirc_fh_lock is initialized in ir_lirc_register(), which is 
called
from rc_register_device(), except when the protocol is CEC:

 /* Ensure that the lirc kfifo is setup before we start the thread */
 if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
 rc = ir_lirc_register(dev);
 if (rc < 0)
 goto out_rx;
 }

So it looks like ir_lirc_scancode_event() fails because dev->lirc_fh_lock was 
never
initialized.

Could this be fall-out from the lirc changes you did not too long ago?


Yes, this is broken. My bad, sorry. I think this must have been broken since
v4.16. I can write a patch but I don't have a patch but I'm on the train
to ELCE in Edinburgh now, with no hardware to test on.


Sean



the kernel oops have been happening for a while now.
yesterday when i checked my logs i can see them at least back a couple of 
months when i was running 4.17


also my scripts to poll status of my tv and turn it on/off works for a while so 
it doesn't crash right away.
maybe it only crashes when i send cec command to turn on/off tv and only 
polling for status is no problem.



i think i have a separate issue too because i had problems even before the 
kernel oopses started.
but i suspect this is caused by my tv locking up the cec bus because unplugging 
power to tv for a few minutes (i must wait or it will still be just as broken) 
and then back used to resolve the cec errors from my scripts.


Re: cec kernel oops with pulse8 usb cec adapter

2018-10-22 Thread Sean Young
On Sat, Oct 20, 2018 at 11:12:16PM +0200, Hans Verkuil wrote:
> Hi Sean,
> 
> Can you take a look at this, it appears to be an RC issue, see my analysis 
> below.
> 
> On 10/20/2018 03:26 PM, Torbjorn Jansson wrote:
> > Hello
> > 
> > i'm using the pulse8 usb cec adapter to control my tv.
> > i have a few scripts that poll the power status of my tv and after a while 
> > it stops working returning errors when trying to check if tv is on or off.
> > this i think matches a kernel oops i'm seeing that i suspect is related to 
> > this.
> > 
> > i have sometimes been able to recover from this problem by completely 
> > cutting power to my tv and also unplugging the usb cec adapter.
> > i have a feeling that the tv is at least partly to blame for cec-ctl not 
> > working but in any case there shouldn't be a kernel oops.
> > 
> > 
> > also every now and then i see this in dmesg:
> > cec cec0: transmit: failed 05
> > cec cec0: transmit: failed 06
> > but that doesn't appear to do any harm as far as i can tell.
> > 
> > any idea whats causing the oops?
> > 
> > the ops:
> > 
> > BUG: unable to handle kernel NULL pointer dereference at 0038
> > PGD 0 P4D 0
> > Oops:  [#1] SMP PTI
> > CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P   OE 
> > 4.18.12-200.fc28.x86_64 #1
> > Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE-F, BIOS 2.1a 06/15/2018
> > Workqueue: events pulse8_irq_work_handler [pulse8_cec]
> > RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0 [rc_core]
> 
> Huh. ir_lirc_scancode_event() calls spin_lock_irqsave(&dev->lirc_fh_lock, 
> flags);
> 
> The spinlock dev->lirc_fh_lock is initialized in ir_lirc_register(), which is 
> called
> from rc_register_device(), except when the protocol is CEC:
> 
> /* Ensure that the lirc kfifo is setup before we start the thread */
> if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
> rc = ir_lirc_register(dev);
> if (rc < 0)
> goto out_rx;
> }
> 
> So it looks like ir_lirc_scancode_event() fails because dev->lirc_fh_lock was 
> never
> initialized.
> 
> Could this be fall-out from the lirc changes you did not too long ago?

Yes, this is broken. My bad, sorry. I think this must have been broken since
v4.16. I can write a patch but I don't have a patch but I'm on the train
to ELCE in Edinburgh now, with no hardware to test on.


Sean


Re: [RFC PATCH v3] media: docs-rst: Document m2m stateless video decoder interface

2018-10-22 Thread Alexandre Courbot
On Mon, Oct 22, 2018 at 3:51 PM Tomasz Figa  wrote:
>
> On Mon, Oct 22, 2018 at 3:39 PM Alexandre Courbot  
> wrote:
> >
> > On Mon, Oct 22, 2018 at 3:22 PM Tomasz Figa  wrote:
> > >
> > > On Mon, Oct 22, 2018 at 3:05 PM Alexandre Courbot  
> > > wrote:
> > > >
> > > > On Fri, Oct 19, 2018 at 5:44 PM Hans Verkuil  wrote:
> > > > >
> > > > > On 10/19/18 10:09, Alexandre Courbot wrote:
> > > > > > Thanks everyone for the feedback on v2! I have not replied to all 
> > > > > > the
> > > > > > individual emails but hope this v3 will address some of the problems
> > > > > > raised and become a continuation point for the topics still in
> > > > > > discussion (probably during the ELCE Media Summit).
> > > > > >
> > > > > > This patch documents the protocol that user-space should follow when
> > > > > > communicating with stateless video decoders. It is based on the
> > > > > > following references:
> > > > > >
> > > > > > * The current protocol used by Chromium (converted from config 
> > > > > > store to
> > > > > >   request API)
> > > > > >
> > > > > > * The submitted Cedrus VPU driver
> > > > > >
> > > > > > As such, some things may not be entirely consistent with the current
> > > > > > state of drivers, so it would be great if all stakeholders could 
> > > > > > point
> > > > > > out these inconsistencies. :)
> > > > > >
> > > > > > This patch is supposed to be applied on top of the Request API V18 
> > > > > > as
> > > > > > well as the memory-to-memory video decoder interface series by 
> > > > > > Tomasz
> > > > > > Figa.
> > > > > >
> > > > > > Changes since v2:
> > > > > >
> > > > > > * Specify that the frame header controls should be set prior to
> > > > > >   enumerating the CAPTURE queue, instead of the profile which as 
> > > > > > Paul
> > > > > >   and Tomasz pointed out is not enough to know which raw formats 
> > > > > > will be
> > > > > >   usable.
> > > > > > * Change V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAM to
> > > > > >   V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS.
> > > > > > * Various rewording and rephrasing
> > > > > >
> > > > > > Two points being currently discussed have not been changed in this
> > > > > > revision due to lack of better idea. Of course this is open to 
> > > > > > change:
> > > > > >
> > > > > > * The restriction of having to send full frames for each input 
> > > > > > buffer is
> > > > > >   kept as-is. As Hans pointed, we currently have a hard limit of 32
> > > > > >   buffers per queue, and it may be non-trivial to lift. Also some 
> > > > > > codecs
> > > > > >   (at least Venus AFAIK) do have this restriction in hardware, so 
> > > > > > unless
> > > > > >   we want to do some buffer-rearranging in-kernel, it is probably 
> > > > > > better
> > > > > >   to keep the default behavior as-is. Finally, relaxing the rule 
> > > > > > should
> > > > > >   be easy enough if we add one extra control to query whether the
> > > > > >   hardware can work with slice units, as opposed to frame units.
> > > > >
> > > > > Makes sense, as long as the restriction can be lifted in the future.
> > > >
> > > > Lifting this limitation once we support more than 32 buffers should
> > > > not be an issue. Just add a new capability control and process things
> > > > in slice units. Right now we have hardware that can only work with
> > > > whole frames (venus)
> > >
> > > Note that venus is a stateful hardware and the restriction might just
> > > come from the firmware.
> >
> > Right, and it most certainly does indeed. Yet firmwares are not always
> > easy to get updated by vendors, so we may have to deal with it anyway.
> >
>
> Right. I'm just not convinced that venus is relevant to the stateless 
> interface.
>
> I'd use the Rockchip VPU hardware as an example of a hardware that
> seems to require all the slices in one buffer, tightly packed one
> after another, since it only accepts one address and size into its
> registers.

You're right that Venus is not relevant here. Rockchip VPU is a much
better example.

>
> > >
> > > > but I suspect that some slice-only hardware must
> > > > exist, so it may actually become a necessity at some point (lest
> > > > drivers do some splitting themselves).
> > > >
> > >
> > > The drivers could do it trivially, because the UAPI will include the
> > > array of slices, with offsets and sizes. It would just run the same
> > > OUTPUT buffer multiple time, once for each slice.
> >
> > Alignment issues notwithstanding. :)
>
> Good point. That could be handled by a memcpy() into a bounce buffer,
> but it wouldn't be as trivial as I suggested anymore indeed.

But at least the kernel won't have to try and parse the stream since
the slices' limits are clearly defined by user-space, so it's not that
big a deal.