Re: [PATCH v2 2/2] Documentation/sphinx: link dma-buf rsts
On Mon, Aug 22, 2016 at 12:49:30PM -0300, Mauro Carvalho Chehab wrote: > Em Mon, 22 Aug 2016 20:41:45 +0530 > Sumit Semwal escreveu: > > > Include dma-buf sphinx documentation into top level index. > > > > Signed-off-by: Sumit Semwal > > --- > > Documentation/index.rst | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/Documentation/index.rst b/Documentation/index.rst > > index e0fc72963e87..8d05070122c2 100644 > > --- a/Documentation/index.rst > > +++ b/Documentation/index.rst > > @@ -14,6 +14,8 @@ Contents: > > :maxdepth: 2 > > > > kernel-documentation > > + dma-buf/intro > > + dma-buf/guide > > media/media_uapi > > media/media_kapi > > media/dvb-drivers/index > > IMHO, the best would be, instead, to add an index with a toctree > with both intro and guide, and add dma-buf/index instead. > > We did that for media too (patches not merged upstream yet), together > with a patchset that will allow building just a subset of the books. I'm also not too sure about whether dma-buf really should be it's own subdirectory. It's plucked from the device-drivers.tmpl, I think an overall device-drivers/ for all the misc subsystems and support code would be better. Then one toc there, which fans out to either kernel-doc and overview docs. -Daniel > > Regards, > Mauro > > PS.: That's the content of our index.rst file, at > Documentation/media/index.rst: > > Linux Media Subsystem Documentation > === > > Contents: > > .. toctree:: >:maxdepth: 2 > >media_uapi >media_kapi >dvb-drivers/index >v4l-drivers/index > > .. only:: subproject > >Indices >=== > >* :ref:`genindex` > > > Thanks, > Mauro > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] media: vidc: add Venus HFI files
On Mon 22 Aug 06:13 PDT 2016, Stanimir Varbanov wrote:
> Here is the implementation of Venus video accelerator low-level
> functionality. It contanins code which setup the registers and
> startup uthe processor, allocate and manipulates with the shared
> memory used for sending commands and receiving messages.
>
> Signed-off-by: Stanimir Varbanov
> ---
> drivers/media/platform/qcom/vidc/hfi_venus.c| 1539
> +++
> drivers/media/platform/qcom/vidc/hfi_venus.h| 25 +
> drivers/media/platform/qcom/vidc/hfi_venus_io.h | 98 ++
> 3 files changed, 1662 insertions(+)
> create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus.c
> create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus.h
> create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus_io.h
>
> diff --git a/drivers/media/platform/qcom/vidc/hfi_venus.c
> b/drivers/media/platform/qcom/vidc/hfi_venus.c
[..]
> +
> +static const struct hfi_ops venus_hfi_ops = {
> + .core_init = venus_hfi_core_init,
> + .core_deinit= venus_hfi_core_deinit,
> + .core_ping = venus_hfi_core_ping,
> + .core_trigger_ssr = venus_hfi_core_trigger_ssr,
> +
> + .session_init = venus_hfi_session_init,
> + .session_end= venus_hfi_session_end,
> + .session_abort = venus_hfi_session_abort,
> + .session_flush = venus_hfi_session_flush,
> + .session_start = venus_hfi_session_start,
> + .session_stop = venus_hfi_session_stop,
> + .session_etb= venus_hfi_session_etb,
> + .session_ftb= venus_hfi_session_ftb,
> + .session_set_buffers= venus_hfi_session_set_buffers,
> + .session_release_buffers= venus_hfi_session_release_buffers,
> + .session_load_res = venus_hfi_session_load_res,
> + .session_release_res= venus_hfi_session_release_res,
> + .session_parse_seq_hdr = venus_hfi_session_parse_seq_hdr,
> + .session_get_seq_hdr= venus_hfi_session_get_seq_hdr,
> + .session_set_property = venus_hfi_session_set_property,
> + .session_get_property = venus_hfi_session_get_property,
> +
> + .resume = venus_hfi_resume,
> + .suspend= venus_hfi_suspend,
> +
> + .isr= venus_isr,
> + .isr_thread = venus_isr_thread,
> +};
> +
> +void venus_hfi_destroy(struct hfi_core *hfi)
> +{
> + struct venus_hfi_device *hdev = to_hfi_priv(hfi);
> +
> + venus_interface_queues_release(hdev);
> + mutex_destroy(&hdev->lock);
> + kfree(hdev);
> +}
> +
> +int venus_hfi_create(struct hfi_core *hfi, const struct vidc_resources *res,
> + void __iomem *base)
> +{
Rather than having the core figure out which *_hfi_create() to call I
think this should be the probe() entry point, calling into the core
registering the venus_hfi_ops - a common-probe() in the hfi/vidc core
could still do most of the heavy lifting.
Probing the driver up from the transport rather than for the highest
logical layer allows us to inject a separate hfi_ops for the apr tal
case.
> + struct venus_hfi_device *hdev;
> + int ret;
> +
> + hdev = kzalloc(sizeof(*hdev), GFP_KERNEL);
> + if (!hdev)
> + return -ENOMEM;
> +
> + mutex_init(&hdev->lock);
> +
> + hdev->res = res;
> + hdev->pkt_ops = hfi->pkt_ops;
> + hdev->packetization_type = HFI_PACKETIZATION_LEGACY;
> + hdev->base = base;
> + hdev->dev = hfi->dev;
> + hdev->suspended = true;
> +
> + hfi->priv = hdev;
> + hfi->ops = &venus_hfi_ops;
> + hfi->core_caps = VIDC_ENC_ROTATION_CAPABILITY |
> + VIDC_ENC_SCALING_CAPABILITY |
> + VIDC_ENC_DEINTERLACE_CAPABILITY |
> + VIDC_DEC_MULTI_STREAM_CAPABILITY;
> +
> + ret = venus_interface_queues_init(hdev);
> + if (ret)
> + goto err_kfree;
> +
> + return 0;
> +
> +err_kfree:
> + kfree(hdev);
> + hfi->priv = NULL;
> + hfi->ops = NULL;
> + return ret;
> +}
I'll try to find some time to do a more detailed review of the
implementation.
Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/8] media: vidc: add Host Firmware Interface (HFI)
On Mon 22 Aug 06:13 PDT 2016, Stanimir Varbanov wrote:
> This is the implementation of HFI. It is loaded with the
> responsibility to comunicate with the firmware through an
> interface commands and messages.
>
> - hfi.c has interface functions used by the core, decoder
> and encoder parts to comunicate with the firmware. For example
> there are functions for session and core initialisation.
>
I can't help feeling that the split between core.c and hfi.c is a
remnant of a vidc driver supporting both HFI and pre-HFI with the same
v4l code.
What do you think about merging vidc_core with hfi_core and vidc_inst
with hfi_inst? Both seems to be in a 1:1 relationship.
> - hfi_cmds has packetization operations which preparing
> packets to be send from host to firmware.
>
> - hfi_msgs takes care of messages sent from firmware to the
> host.
>
[..]
> diff --git a/drivers/media/platform/qcom/vidc/hfi_cmds.c
> b/drivers/media/platform/qcom/vidc/hfi_cmds.c
[..]
> +
> +static const struct hfi_packetization_ops hfi_default = {
> + .sys_init = pkt_sys_init,
> + .sys_pc_prep = pkt_sys_pc_prep,
> + .sys_idle_indicator = pkt_sys_idle_indicator,
> + .sys_power_control = pkt_sys_power_control,
> + .sys_set_resource = pkt_sys_set_resource,
> + .sys_release_resource = pkt_sys_unset_resource,
> + .sys_debug_config = pkt_sys_debug_config,
> + .sys_coverage_config = pkt_sys_coverage_config,
> + .sys_ping = pkt_sys_ping,
> + .sys_image_version = pkt_sys_image_version,
> + .ssr_cmd = pkt_ssr_cmd,
> + .session_init = pkt_session_init,
> + .session_cmd = pkt_session_cmd,
> + .session_set_buffers = pkt_session_set_buffers,
> + .session_release_buffers = pkt_session_release_buffers,
> + .session_etb_decoder = pkt_session_etb_decoder,
> + .session_etb_encoder = pkt_session_etb_encoder,
> + .session_ftb = pkt_session_ftb,
> + .session_parse_seq_header = pkt_session_parse_seq_header,
> + .session_get_seq_hdr = pkt_session_get_seq_hdr,
> + .session_flush = pkt_session_flush,
> + .session_get_property = pkt_session_get_property,
> + .session_set_property = pkt_session_set_property,
> +};
> +
> +static const struct hfi_packetization_ops *get_3xx_ops(void)
> +{
> + static struct hfi_packetization_ops hfi_3xx;
> +
> + hfi_3xx = hfi_default;
> + hfi_3xx.session_set_property = pkt_session_set_property_3xx;
> +
> + return &hfi_3xx;
> +}
> +
> +const struct hfi_packetization_ops *
> +hfi_get_pkt_ops(enum hfi_packetization_type type)
The only reasonable argument I can come up with for not just exposing
these as global functions would be that there are 23 of them... Can we
skip the jump table?
> +{
> + switch (type) {
> + case HFI_PACKETIZATION_LEGACY:
> + return &hfi_default;
> + case HFI_PACKETIZATION_3XX:
> + return get_3xx_ops();
> + }
> +
> + return NULL;
> +}
Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: ERRORS
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 Aug 23 04:00:21 CEST 2016 git branch: test git hash: 0d1c7d60c287d13cfea27b50f75dbff411544488 gcc version:i686-linux-gcc (GCC) 5.4.0 sparse version: v0.5.0-56-g7647c77 smatch version: v0.5.0-3428-gdfe27cf host hardware: x86_64 host os:4.6.0-164 linux-git-arm-at91: ERRORS linux-git-arm-davinci: OK linux-git-arm-multi: ERRORS linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: WARNINGS linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12.23-i686: OK linux-3.13.11-i686: OK linux-3.14.9-i686: OK linux-3.15.2-i686: OK linux-3.16.7-i686: OK linux-3.17.8-i686: OK linux-3.18.7-i686: OK linux-3.19-i686: OK linux-4.0-i686: OK linux-4.1.1-i686: OK linux-4.2-i686: OK linux-4.3-i686: OK linux-4.4-i686: OK linux-4.5-i686: OK linux-4.6-i686: OK linux-4.7-i686: OK linux-4.8-rc1-i686: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12.23-x86_64: OK linux-3.13.11-x86_64: OK linux-3.14.9-x86_64: OK linux-3.15.2-x86_64: OK linux-3.16.7-x86_64: OK linux-3.17.8-x86_64: OK linux-3.18.7-x86_64: OK linux-3.19-x86_64: OK linux-4.0-x86_64: OK linux-4.1.1-x86_64: OK linux-4.2-x86_64: OK linux-4.3-x86_64: OK linux-4.4-x86_64: OK linux-4.5-x86_64: OK linux-4.6-x86_64: OK linux-4.7-x86_64: OK linux-4.8-rc1-x86_64: OK apps: OK spec-git: ERRORS sparse: WARNINGS smatch: 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 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8] media: vidc: adding core part and helper functions
On Mon 22 Aug 06:13 PDT 2016, Stanimir Varbanov wrote:
Hi Stan,
> This adds core part of the vidc driver common helper functions
> used by encoder and decoder specific files.
I believe "vidc" is short for "video core" and this is not the only
"video core" from Qualcomm. This driver is the v4l2 <-> hfi interface and
uses either two ram based fifos _or_ apr tal for communication with the
implementation.
In the case of apr, the other side is not the venus core but rather the
"VIDC" apr service on the Hexagon DSP. In this case the hfi packets are
encapsulated in apr packets. Although this is not used in 8916 it would
be nice to be able to add this later...
But I think we should call this driver "hfi" - or at least venus, as
it's not compatible with e.g the "blackbird" found in 8064, which is
also called "vidc".
>
> - core.c has implemented the platform dirver methods, file
> operations and v4l2 registration.
>
> - helpers.c has implemented common helper functions for
> buffer management, vb2_ops and functions for format propagation.
>
> - int_bufs.c implements functions for allocating and freeing
> buffers for internal usage. The buffer parameters describing
> internal buffers depends on current format, resolution and
> codec.
>
> - load.c consists functions for calculation of current load
> of the hardware. Depending on the count of instances and
> resolutions it selects the best clock rate for the video
> core.
>
> - mem.c has two functions for memory allocation, currently
> those functions are used for internal buffers and to allocate
> the shared memory for communication with firmware via HFI
> (Host Firmware Interface) interface commands.
Please drop this; see comments on mem_alloc()
>
> - resources.c exports a structure describing the details
> specific to platform and SoC.
>
> Signed-off-by: Stanimir Varbanov
> ---
This doesn't compile, as it depends on later patches. Also there are
plenty of functions that are related to later patches and would with be
better to include there, to keep the size of this patch down.
> drivers/media/platform/qcom/vidc/core.c | 548
> +++
> drivers/media/platform/qcom/vidc/core.h | 196 ++
> drivers/media/platform/qcom/vidc/helpers.c | 394 +++
> drivers/media/platform/qcom/vidc/helpers.h | 43 +++
> drivers/media/platform/qcom/vidc/int_bufs.c | 325
> drivers/media/platform/qcom/vidc/int_bufs.h | 23 ++
> drivers/media/platform/qcom/vidc/load.c | 104 +
> drivers/media/platform/qcom/vidc/load.h | 22 ++
> drivers/media/platform/qcom/vidc/mem.c | 64
> drivers/media/platform/qcom/vidc/mem.h | 32 ++
> drivers/media/platform/qcom/vidc/resources.c | 46 +++
> drivers/media/platform/qcom/vidc/resources.h | 46 +++
> 12 files changed, 1843 insertions(+)
> create mode 100644 drivers/media/platform/qcom/vidc/core.c
> create mode 100644 drivers/media/platform/qcom/vidc/core.h
> create mode 100644 drivers/media/platform/qcom/vidc/helpers.c
> create mode 100644 drivers/media/platform/qcom/vidc/helpers.h
> create mode 100644 drivers/media/platform/qcom/vidc/int_bufs.c
> create mode 100644 drivers/media/platform/qcom/vidc/int_bufs.h
> create mode 100644 drivers/media/platform/qcom/vidc/load.c
> create mode 100644 drivers/media/platform/qcom/vidc/load.h
> create mode 100644 drivers/media/platform/qcom/vidc/mem.c
> create mode 100644 drivers/media/platform/qcom/vidc/mem.h
> create mode 100644 drivers/media/platform/qcom/vidc/resources.c
> create mode 100644 drivers/media/platform/qcom/vidc/resources.h
>
> diff --git a/drivers/media/platform/qcom/vidc/core.c
> b/drivers/media/platform/qcom/vidc/core.c
> new file mode 100644
> index ..e005be178fc0
> --- /dev/null
> +++ b/drivers/media/platform/qcom/vidc/core.c
> @@ -0,0 +1,548 @@
> +/*
> + * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2016 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "core.h"
> +#include "resources.h"
> +#include "vdec.h"
> +#include "venc.h"
> +
> +static void vidc_add_inst(struct vidc_core *core, struct vidc_inst *inst)
> +{
> + mutex_lock(&core->lock);
> + list_add_tail(&inst->list, &core->instances);
There are two different "instances" lists in this implementa
[PATCH v2] docs-rst: kernel-doc: better output struct members
Right now, for a struct, kernel-doc produces the following output:
.. c:type:: struct v4l2_prio_state
stores the priority states
**Definition**
::
struct v4l2_prio_state {
atomic_t prios[4];
};
**Members**
``atomic_t prios[4]``
array with elements to store the array priorities
Putting a member name in verbatim and adding a continuation line
causes the LaTeX output to generate something like:
item[atomic_t prios\[4\]] array with elements to store the array
priorities
Everything inside "item" is non-breakable, with may produce
lines bigger than the column width.
Also, for function members, like:
int (* rx_read) (struct v4l2_subdev *sd, u8 *buf, size_t count,ssize_t
*num);
It puts the name of the member at the end, like:
int (*) (struct v4l2_subdev *sd, u8 *buf, size_t count,ssize_t *num)
read
With is very confusing.
The best is to highlight what really matters: the member name.
is a secondary information.
So, change kernel-doc, for it to produce the output on a different way:
**Members**
``prios[4]``
array with elements to store the array priorities
Also, as the type is not part of LaTeX "item[]", LaTeX will split it into
multiple lines, if needed.
So, both LaTeX/PDF and HTML outputs will look good.
It should be noticed, however, that the way Sphinx LaTeX output handles
things like:
Foo
bar
is different than the HTML output. On HTML, it will produce something
like:
**Foo**
bar
While, on LaTeX, it puts both foo and bar at the same line, like:
**Foo** bar
Signed-off-by: Mauro Carvalho Chehab
---
scripts/kernel-doc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index ba081c7636a2..d225e178aa1b 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -2000,7 +2000,7 @@ sub output_struct_rst(%) {
($args{'parameterdescs'}{$parameter_name} ne $undescribed) || next;
$type = $args{'parametertypes'}{$parameter};
print_lineno($parameterdesc_start_lines{$parameter_name});
- print "``$type $parameter``\n";
+ print "``" . $parameter . "``\n";
output_highlight_rst($args{'parameterdescs'}{$parameter_name});
print "\n";
}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] docs-rst: kernel-doc: better output struct members
Em Mon, 22 Aug 2016 21:41:51 -0300 Mauro Carvalho Chehab escreveu: > Right now, for a struct, kernel-doc produces the following output: Please discard this one... the description is wrong. Just sent a v2. Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] docs-rst: kernel-doc: better output struct members
Right now, for a struct, kernel-doc produces the following output:
.. c:type:: struct v4l2_prio_state
stores the priority states
**Definition**
::
struct v4l2_prio_state {
atomic_t prios[4];
};
**Members**
``atomic_t prios[4]``
array with elements to store the array priorities
Putting a member name in verbatim and adding a continuation line
causes the LaTeX output to generate something like:
item[atomic_t prios\[4\]] array with elements to store the array
priorities
Everything inside "item" is non-breakable, with may produce
lines bigger than the column width.
Also, for function members, like:
int (* rx_read) (struct v4l2_subdev *sd, u8 *buf, size_t count,ssize_t
*num);
It puts the name of the member at the end, like:
int (*) (struct v4l2_subdev *sd, u8 *buf, size_t count,ssize_t *num)
read
With is very confusing.
The best is to highlight what really matters: the member name; the type
is a secondary information.
So, change kernel-doc, for it to produce the output on a different way:
**Members**
``prios[4]``
- **type**: ``atomic_t``
array with elements to store the array priorities
With such change, the name of the member will be the first visible
thing, and will be in bold style. The type will still be there, inside
a list.
Also, as the type is not part of LaTeX "item[]", LaTeX will split it into
multiple lines, if needed.
So, both LaTeX/PDF and HTML outputs will look good.
It should be noticed, however, that the way Sphinx LaTeX output handles
things like:
Foo
bar
is different than the HTML output. On HTML, it will produce something
like:
**Foo**
bar
While, on LaTeX, it puts both foo and bar at the same line, like:
**Foo** bar
By starting the second line with a dash, both HTML and LaTeX output
will do the same thing.
Signed-off-by: Mauro Carvalho Chehab
---
scripts/kernel-doc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index ba081c7636a2..d225e178aa1b 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -2000,7 +2000,7 @@ sub output_struct_rst(%) {
($args{'parameterdescs'}{$parameter_name} ne $undescribed) || next;
$type = $args{'parametertypes'}{$parameter};
print_lineno($parameterdesc_start_lines{$parameter_name});
- print "``$type $parameter``\n";
+ print "``" . $parameter . "``\n";
output_highlight_rst($args{'parameterdescs'}{$parameter_name});
print "\n";
}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] docs-rst: kernel-doc: better output struct members
Em Mon, 22 Aug 2016 15:34:21 -0600 Jonathan Corbet escreveu: > On Sun, 21 Aug 2016 09:11:57 -0300 > Mauro Carvalho Chehab wrote: > > > So, change kernel-doc, for it to produce the output on a different way: > > > > **Members** > > > > ``prios[4]`` > > - **type**: ``atomic_t`` > > > > array with elements to store the array priorities > > > > With such change, the name of the member will be the first visible > > thing, and will be in bold style. The type will still be there, inside > > a list. > > OK, I'll confess to not being 100% convinced on this one. I certainly > sympathize with the problem that drives this change, but I think the > result is a bit on the noisy and visually distracting side. > > I wonder if we might be better off to just leave the "type:" bulleted > line out entirely? The type information already appears in the structure > listing directly above, so it's arguably redundant here. If formatting > the type is getting in the way here, perhaps the right answer is just > "don't do that"? I almost stripped the type on the first version of this patch, as I had the same doubt as you ;) I ended keeping it just because I didn't have a strong argument to strip it. There is another reason too... just stripping it will produce a little difference at the output: With HTML, the output is: *struct v4l2_subdev_tuner_ops* Callbacks used when v4l device was opened in radio mode. ... *Members* *s_radio* callback for VIDIOC_S_RADIO ioctl handler code. ... On LaTeX/PDF, is displayed as: *struct v4l2_subdev_tuner_ops* Callbacks used when v4l device was opened in radio mode. ... *Members* *s_radio* callback for VIDIOC_S_RADIO ioctl handler code. Anyway, I'm OK on just stripping the type. I'm sending a second version of it. Thanks! Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] docs-rst: kernel-doc: better output struct members
On Sun, 21 Aug 2016 09:11:57 -0300 Mauro Carvalho Chehab wrote: > So, change kernel-doc, for it to produce the output on a different way: > > **Members** > > ``prios[4]`` > - **type**: ``atomic_t`` > > array with elements to store the array priorities > > With such change, the name of the member will be the first visible > thing, and will be in bold style. The type will still be there, inside > a list. OK, I'll confess to not being 100% convinced on this one. I certainly sympathize with the problem that drives this change, but I think the result is a bit on the noisy and visually distracting side. I wonder if we might be better off to just leave the "type:" bulleted line out entirely? The type information already appears in the structure listing directly above, so it's arguably redundant here. If formatting the type is getting in the way here, perhaps the right answer is just "don't do that"? jon -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] docs-rst: conf.py: adjust the size of .. note:: tag
On Fri, 19 Aug 2016 09:49:38 -0300 Mauro Carvalho Chehab wrote: > While the current implementation works well when using as a > paragraph, it doesn't work properly if inside a table. As we > have quite a few such cases, fix the logic to take the column > size into account. Applied to the docs tree, thanks. jon -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] docs-rst: add support for LaTeX output
On Thu, 18 Aug 2016 11:53:39 -0300 Mauro Carvalho Chehab wrote: > Sphinx supports LaTeX output. Sometimes, it is interesting to > call it directly, instead of also generating a PDF. As it comes > for free, add a target for it. > Applied to the docs tree, thanks. jon -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/5] doc-rst: improvements Sphinx's C-domain
On Mon, 15 Aug 2016 16:08:23 +0200 Markus Heiser wrote: > this is my approach to eliminate some distortions we have with the c/cpp > Sphinx > domains. The C domain is simple: it assumes that all functions, enums, etc > are global, e. g. there should be just one function called "ioctl", or "open". > With the 'name' option e.g.: > > .. c:function:: int ioctl( int fd, int request ) >:name: VIDIOC_LOG_STATUS > > we can rename those functions. Another nice feature around this *global* > namespace topic is, that the *duplicate C object description* warnings for > function declarations are moved to the nitpicky mode. I've applied these to the docs tree, thanks. jon -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
drivers/media/v4l2-core/videobuf2-dma-contig.c:486:2: error: implicit declaration of function 'dma_get_cache_alignment'
Hi Hans,
FYI, the error/warning still remains.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: fa8410b355251fd30341662a40ac6b22d3e38468
commit: c1023ba74fc77dc56dc317bd98f5060aab889ac1 [media]
drivers/media/platform/Kconfig: fix VIDEO_MEDIATEK_VCODEC dependency
date: 6 weeks ago
config: m32r-allmodconfig (attached as .config)
compiler: m32r-linux-gcc (GCC) 4.9.0
reproduce:
wget
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout c1023ba74fc77dc56dc317bd98f5060aab889ac1
# save the attached .config to linux build tree
make.cross ARCH=m32r
All errors (new ones prefixed by >>):
drivers/media/v4l2-core/videobuf2-dma-contig.c: In function
'vb2_dc_get_userptr':
>> drivers/media/v4l2-core/videobuf2-dma-contig.c:486:2: error: implicit
>> declaration of function 'dma_get_cache_alignment'
>> [-Werror=implicit-function-declaration]
unsigned long dma_align = dma_get_cache_alignment();
^
cc1: some warnings being treated as errors
vim +/dma_get_cache_alignment +486
drivers/media/v4l2-core/videobuf2-dma-contig.c
774d2301 drivers/media/v4l2-core/videobuf2-dma-contig.c Marek Szyprowski
2013-06-19 470 {
774d2301 drivers/media/v4l2-core/videobuf2-dma-contig.c Marek Szyprowski
2013-06-19 471 /* really, we cannot do anything better at this point */
774d2301 drivers/media/v4l2-core/videobuf2-dma-contig.c Marek Szyprowski
2013-06-19 472 return (dma_addr_t)(pfn) << PAGE_SHIFT;
774d2301 drivers/media/v4l2-core/videobuf2-dma-contig.c Marek Szyprowski
2013-06-19 473 }
774d2301 drivers/media/v4l2-core/videobuf2-dma-contig.c Marek Szyprowski
2013-06-19 474 #endif
774d2301 drivers/media/v4l2-core/videobuf2-dma-contig.c Marek Szyprowski
2013-06-19 475
36c0f8b3 drivers/media/v4l2-core/videobuf2-dma-contig.c Hans Verkuil
2016-04-15 476 static void *vb2_dc_get_userptr(struct device *dev, unsigned
long vaddr,
cd474037 drivers/media/v4l2-core/videobuf2-dma-contig.c Hans Verkuil
2014-11-18 477 unsigned long size, enum dma_data_direction dma_dir)
1a758d4e drivers/media/video/videobuf2-dma-contig.c Pawel Osciak
2010-10-11 478 {
1a758d4e drivers/media/video/videobuf2-dma-contig.c Pawel Osciak
2010-10-11 479 struct vb2_dc_buf *buf;
fb639eb3 drivers/media/v4l2-core/videobuf2-dma-contig.c Jan Kara
2015-07-13 480 struct frame_vector *vec;
e15dab75 drivers/media/v4l2-core/videobuf2-dma-contig.c Tomasz Stanislawski
2012-06-14 481 unsigned long offset;
fb639eb3 drivers/media/v4l2-core/videobuf2-dma-contig.c Jan Kara
2015-07-13 482 int n_pages, i;
e15dab75 drivers/media/v4l2-core/videobuf2-dma-contig.c Tomasz Stanislawski
2012-06-14 483 int ret = 0;
e15dab75 drivers/media/v4l2-core/videobuf2-dma-contig.c Tomasz Stanislawski
2012-06-14 484 struct sg_table *sgt;
e15dab75 drivers/media/v4l2-core/videobuf2-dma-contig.c Tomasz Stanislawski
2012-06-14 485 unsigned long contig_size;
d81e870d drivers/media/v4l2-core/videobuf2-dma-contig.c Marek Szyprowski
2012-06-12 @486 unsigned long dma_align = dma_get_cache_alignment();
251a79f8 drivers/media/v4l2-core/videobuf2-dma-contig.c Hans Verkuil
2014-11-18 487 DEFINE_DMA_ATTRS(attrs);
251a79f8 drivers/media/v4l2-core/videobuf2-dma-contig.c Hans Verkuil
2014-11-18 488
251a79f8 drivers/media/v4l2-core/videobuf2-dma-contig.c Hans Verkuil
2014-11-18 489 dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
d81e870d drivers/media/v4l2-core/videobuf2-dma-contig.c Marek Szyprowski
2012-06-12 490
d81e870d drivers/media/v4l2-core/videobuf2-dma-contig.c Marek Szyprowski
2012-06-12 491 /* Only cache aligned DMA transfers are reliable */
d81e870d drivers/media/v4l2-core/videobuf2-dma-contig.c Marek Szyprowski
2012-06-12 492 if (!IS_ALIGNED(vaddr | size, dma_align)) {
d81e870d drivers/media/v4l2-core/videobuf2-dma-contig.c Marek Szyprowski
2012-06-12 493 pr_debug("user data must be aligned to %lu
bytes\n", dma_align);
d81e870d drivers/media/v4l2-core/videobuf2-dma-contig.c Marek Szyprowski
2012-06-12 494 return ERR_PTR(-EINVAL);
:: The code at line 486 was first introduced by commit
:: d81e870d5afa1b0a95ea94c4052d3c7e973fae8c [media] v4l: vb2-dma-contig:
fail if user ptr buffer is not correctly aligned
:: TO: Marek Szyprowski
:: CC: Mauro Carvalho Chehab
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: Binary data
Re: [PATCH v4 2/4] dt-bindings: Add a binding for Mediatek MDP
On 22/08/16 03:08, Minghsiu Tsai wrote: On Fri, 2016-08-19 at 09:16 -0500, Rob Herring wrote: On Fri, Aug 19, 2016 at 07:39:25PM +0800, Minghsiu Tsai wrote: Add a DT binding documentation of MDP for the MT8173 SoC from Mediatek Signed-off-by: Minghsiu Tsai --- .../devicetree/bindings/media/mediatek-mdp.txt | 109 1 file changed, 109 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/mediatek-mdp.txt Please add acks when posting new versions. Rob Sorry for my mistake. I will add it in next version. No need to provide a new version just because of this. If the driver will be taken as is, I can fix up the ack when applying. Thanks, Matthias -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] media: tw686x: Support frame sizes and frame intervals enumeration
This commit adds support for VIDIOC_ENUM_FRAMESIZES
and VIDIOC_ENUM_FRAMEINTERVALS enumeration ioctls.
Signed-off-by: Ezequiel Garcia
---
drivers/media/pci/tw686x/tw686x-video.c | 38 +
1 file changed, 38 insertions(+)
diff --git a/drivers/media/pci/tw686x/tw686x-video.c
b/drivers/media/pci/tw686x/tw686x-video.c
index be257d0257a6..e89560d42d16 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -906,6 +906,42 @@ static int tw686x_g_std(struct file *file, void *priv,
v4l2_std_id *id)
return 0;
}
+static int tw686x_enum_framesizes(struct file *file, void *priv,
+ struct v4l2_frmsizeenum *fsize)
+{
+ struct tw686x_video_channel *vc = video_drvdata(file);
+
+ if (fsize->index)
+ return -EINVAL;
+ fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
+ fsize->stepwise.max_width = TW686X_VIDEO_WIDTH;
+ fsize->stepwise.min_width = fsize->stepwise.max_width / 2;
+ fsize->stepwise.step_width = fsize->stepwise.min_width;
+ fsize->stepwise.max_height = TW686X_VIDEO_HEIGHT(vc->video_standard);
+ fsize->stepwise.min_height = fsize->stepwise.max_height / 2;
+ fsize->stepwise.step_height = fsize->stepwise.min_height;
+ return 0;
+}
+
+static int tw686x_enum_frameintervals(struct file *file, void *priv,
+ struct v4l2_frmivalenum *ival)
+{
+ struct tw686x_video_channel *vc = video_drvdata(file);
+ int max_fps = TW686X_MAX_FPS(vc->video_standard);
+ int max_rates = DIV_ROUND_UP(max_fps, 2);
+
+ if (ival->index >= max_rates)
+ return -EINVAL;
+
+ ival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
+ ival->discrete.numerator = 1;
+ if (ival->index < (max_rates - 1))
+ ival->discrete.denominator = (ival->index + 1) * 2;
+ else
+ ival->discrete.denominator = max_fps;
+ return 0;
+}
+
static int tw686x_g_parm(struct file *file, void *priv,
struct v4l2_streamparm *sp)
{
@@ -1034,6 +1070,8 @@ static const struct v4l2_ioctl_ops tw686x_video_ioctl_ops
= {
.vidioc_g_parm = tw686x_g_parm,
.vidioc_s_parm = tw686x_s_parm,
+ .vidioc_enum_framesizes = tw686x_enum_framesizes,
+ .vidioc_enum_frameintervals = tw686x_enum_frameintervals,
.vidioc_enum_input = tw686x_enum_input,
.vidioc_g_input = tw686x_g_input,
--
2.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8] media: vidc: adding core part and helper functions
Hi Hans,
Thanks for the express comments!
>> +
>> +struct vidc_core {
>> +struct list_head list;
>> +void __iomem *base;
>> +int irq;
>> +struct clk *clks[VIDC_CLKS_NUM_MAX];
>> +struct mutex lock;
>> +struct hfi_core hfi;
>> +struct video_device vdev_dec;
>> +struct video_device vdev_enc;
>
> I know that many drivers embed struct video_device, but this can cause subtle
> refcounting problems. I recommend changing this to a pointer and using
> video_device_alloc().
>
> I have plans to reorganize the way video_devices are allocated and registered
> in
> the near future, and you might just as well prepare this driver for that by
> switching
> to a pointer.
OK, thanks for the info, I will change to pointers.
>> +
>> +int vidc_set_color_format(struct vidc_inst *inst, u32 type, u32 pixfmt)
>> +{
>> +struct hfi_uncompressed_format_select fmt;
>> +struct hfi_core *hfi = &inst->core->hfi;
>> +u32 ptype = HFI_PROPERTY_PARAM_UNCOMPRESSED_FORMAT_SELECT;
>> +int ret;
>> +
>> +fmt.buffer_type = type;
>> +
>> +switch (pixfmt) {
>> +case V4L2_PIX_FMT_NV12:
>> +fmt.format = HFI_COLOR_FORMAT_NV12;
>> +break;
>> +case V4L2_PIX_FMT_NV21:
>> +fmt.format = HFI_COLOR_FORMAT_NV21;
>> +break;
>> +default:
>> +return -ENOTSUPP;
>
> I'm not really sure how this error code is used, but normally -EINVAL is
> returned
> for invalid pixel formats. -ENOTSUPP is not used by V4L2.
>
you are right, I need to change this to EINVAL.
--
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] Documentation/sphinx: link dma-buf rsts
Em Mon, 22 Aug 2016 20:41:45 +0530 Sumit Semwal escreveu: > Include dma-buf sphinx documentation into top level index. > > Signed-off-by: Sumit Semwal > --- > Documentation/index.rst | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/index.rst b/Documentation/index.rst > index e0fc72963e87..8d05070122c2 100644 > --- a/Documentation/index.rst > +++ b/Documentation/index.rst > @@ -14,6 +14,8 @@ Contents: > :maxdepth: 2 > > kernel-documentation > + dma-buf/intro > + dma-buf/guide > media/media_uapi > media/media_kapi > media/dvb-drivers/index IMHO, the best would be, instead, to add an index with a toctree with both intro and guide, and add dma-buf/index instead. We did that for media too (patches not merged upstream yet), together with a patchset that will allow building just a subset of the books. Regards, Mauro PS.: That's the content of our index.rst file, at Documentation/media/index.rst: Linux Media Subsystem Documentation === Contents: .. toctree:: :maxdepth: 2 media_uapi media_kapi dvb-drivers/index v4l-drivers/index .. only:: subproject Indices === * :ref:`genindex` Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] Documentation: move dma-buf documentation to rst
Branch out dma-buf related documentation into its own rst file to allow adding it to the sphinx documentation generated. While at it, move dma-buf-sharing.txt into rst as the dma-buf guide too; adjust MAINTAINERS accordingly. v2: - Removed authorship as suggested by Jani, - Address review comments from Jonathan Corbet and Markus Heiser. Signed-off-by: Sumit Semwal --- Documentation/DocBook/device-drivers.tmpl | 41 --- Documentation/dma-buf-sharing.txt | 482 Documentation/dma-buf/guide.rst | 507 ++ Documentation/dma-buf/intro.rst | 82 + MAINTAINERS | 2 +- 5 files changed, 590 insertions(+), 524 deletions(-) delete mode 100644 Documentation/dma-buf-sharing.txt create mode 100644 Documentation/dma-buf/guide.rst create mode 100644 Documentation/dma-buf/intro.rst diff --git a/Documentation/DocBook/device-drivers.tmpl b/Documentation/DocBook/device-drivers.tmpl index 9c10030eb2be..a93fbffa9742 100644 --- a/Documentation/DocBook/device-drivers.tmpl +++ b/Documentation/DocBook/device-drivers.tmpl @@ -128,47 +128,6 @@ X!Edrivers/base/interface.c !Edrivers/base/platform.c !Edrivers/base/bus.c - - Buffer Sharing and Synchronization - - The dma-buf subsystem provides the framework for sharing buffers - for hardware (DMA) access across multiple device drivers and - subsystems, and for synchronizing asynchronous hardware access. - - - This is used, for example, by drm "prime" multi-GPU support, but - is of course not limited to GPU use cases. - - - The three main components of this are: (1) dma-buf, representing - a sg_table and exposed to userspace as a file descriptor to allow - passing between devices, (2) fence, which provides a mechanism - to signal when one device as finished access, and (3) reservation, - which manages the shared or exclusive fence(s) associated with - the buffer. - - dma-buf -!Edrivers/dma-buf/dma-buf.c -!Iinclude/linux/dma-buf.h - - reservation -!Pdrivers/dma-buf/reservation.c Reservation Object Overview -!Edrivers/dma-buf/reservation.c -!Iinclude/linux/reservation.h - - fence -!Edrivers/dma-buf/fence.c -!Iinclude/linux/fence.h -!Edrivers/dma-buf/seqno-fence.c -!Iinclude/linux/seqno-fence.h -!Edrivers/dma-buf/fence-array.c -!Iinclude/linux/fence-array.h -!Edrivers/dma-buf/reservation.c -!Iinclude/linux/reservation.h -!Edrivers/dma-buf/sync_file.c -!Iinclude/linux/sync_file.h - - Device Drivers DMA Management !Edrivers/base/dma-coherent.c !Edrivers/base/dma-mapping.c diff --git a/Documentation/dma-buf-sharing.txt b/Documentation/dma-buf-sharing.txt deleted file mode 100644 index ca44c5820585.. --- a/Documentation/dma-buf-sharing.txt +++ /dev/null @@ -1,482 +0,0 @@ -DMA Buffer Sharing API Guide - - -Sumit Semwal - - - -This document serves as a guide to device-driver writers on what is the dma-buf -buffer sharing API, how to use it for exporting and using shared buffers. - -Any device driver which wishes to be a part of DMA buffer sharing, can do so as -either the 'exporter' of buffers, or the 'user' of buffers. - -Say a driver A wants to use buffers created by driver B, then we call B as the -exporter, and A as buffer-user. - -The exporter -- implements and manages operations[1] for the buffer -- allows other users to share the buffer by using dma_buf sharing APIs, -- manages the details of buffer allocation, -- decides about the actual backing storage where this allocation happens, -- takes care of any migration of scatterlist - for all (shared) users of this - buffer, - -The buffer-user -- is one of (many) sharing users of the buffer. -- doesn't need to worry about how the buffer is allocated, or where. -- needs a mechanism to get access to the scatterlist that makes up this buffer - in memory, mapped into its own address space, so it can access the same area - of memory. - -dma-buf operations for device dma only --- - -The dma_buf buffer sharing API usage contains the following steps: - -1. Exporter announces that it wishes to export a buffer -2. Userspace gets the file descriptor associated with the exported buffer, and - passes it around to potential buffer-users based on use case -3. Each buffer-user 'connects' itself to the buffer -4. When needed, buffer-user requests access to the buffer from exporter -5. When finished with its use, the buffer-user notifies end-of-DMA to exporter -6. when buffer-user is done using this buffer completely, it 'disconnects' - itself from the buffer. - - -1. Exporter's announcement of buffer export - - The buffer exporter announces its wi
[PATCH v2 0/2] doc: dma-buf: sphinx conversion
Convert dma-buf documentation over to sphinx. While at that, convert dma-buf-sharing.txt as well, and make it the dma-buf API guide. There is no content change yet; only format conversion and creation of some hyperlinks. v2: Address review comments from Jonathan Corbet and Markus Heiser. Sumit Semwal (2): Documentation: move dma-buf documentation to rst Documentation/sphinx: link dma-buf rsts Documentation/DocBook/device-drivers.tmpl | 41 --- Documentation/dma-buf-sharing.txt | 482 Documentation/dma-buf/guide.rst | 507 ++ Documentation/dma-buf/intro.rst | 82 + Documentation/index.rst | 2 + MAINTAINERS | 2 +- 6 files changed, 592 insertions(+), 524 deletions(-) delete mode 100644 Documentation/dma-buf-sharing.txt create mode 100644 Documentation/dma-buf/guide.rst create mode 100644 Documentation/dma-buf/intro.rst -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] Documentation/sphinx: link dma-buf rsts
Include dma-buf sphinx documentation into top level index. Signed-off-by: Sumit Semwal --- Documentation/index.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/index.rst b/Documentation/index.rst index e0fc72963e87..8d05070122c2 100644 --- a/Documentation/index.rst +++ b/Documentation/index.rst @@ -14,6 +14,8 @@ Contents: :maxdepth: 2 kernel-documentation + dma-buf/intro + dma-buf/guide media/media_uapi media/media_kapi media/dvb-drivers/index -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] media: vidc: decoder: add video decoder files
On 08/22/2016 03:13 PM, Stanimir Varbanov wrote:
> This consists of video decoder implementation plus decoder
> controls.
>
> Signed-off-by: Stanimir Varbanov
> ---
> drivers/media/platform/qcom/vidc/vdec.c | 1100
> +
> drivers/media/platform/qcom/vidc/vdec.h | 27 +
> drivers/media/platform/qcom/vidc/vdec_ctrls.c | 200 +
> drivers/media/platform/qcom/vidc/vdec_ctrls.h | 21 +
> 4 files changed, 1348 insertions(+)
> create mode 100644 drivers/media/platform/qcom/vidc/vdec.c
> create mode 100644 drivers/media/platform/qcom/vidc/vdec.h
> create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.c
> create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.h
>
> diff --git a/drivers/media/platform/qcom/vidc/vdec.c
> b/drivers/media/platform/qcom/vidc/vdec.c
> new file mode 100644
> index ..a631a354742f
> --- /dev/null
> +++ b/drivers/media/platform/qcom/vidc/vdec.c
> @@ -0,0 +1,1100 @@
> +/*
> + * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2016 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "core.h"
> +#include "helpers.h"
> +#include "load.h"
> +#include "vdec.h"
> +#include "vdec_ctrls.h"
> +
> +#define MACROBLOCKS_PER_PIXEL(16 * 16)
> +
> +static u32 get_framesize_nv12(int plane, u32 height, u32 width)
> +{
> + u32 y_stride, uv_stride, y_plane;
> + u32 y_sclines, uv_sclines, uv_plane;
> + u32 size;
> +
> + y_stride = ALIGN(width, 128);
> + uv_stride = ALIGN(width, 128);
> + y_sclines = ALIGN(height, 32);
> + uv_sclines = ALIGN(((height + 1) >> 1), 16);
> +
> + y_plane = y_stride * y_sclines;
> + uv_plane = uv_stride * uv_sclines + SZ_4K;
> + size = y_plane + uv_plane + SZ_8K;
> +
> + return ALIGN(size, SZ_4K);
> +}
> +
> +static u32 get_framesize_compressed(u32 mbs_per_frame)
> +{
> + return ((mbs_per_frame * MACROBLOCKS_PER_PIXEL * 3 / 2) / 2) + 128;
> +}
> +
> +static const struct vidc_format vdec_formats[] = {
> + {
> + .pixfmt = V4L2_PIX_FMT_NV12,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_MPEG4,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_MPEG2,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_H263,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_H264,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_VP8,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + }, {
> + .pixfmt = V4L2_PIX_FMT_XVID,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> + },
> +};
> +
> +static const struct vidc_format *find_format(u32 pixfmt, u32 type)
> +{
> + const struct vidc_format *fmt = vdec_formats;
> + unsigned int size = ARRAY_SIZE(vdec_formats);
> + unsigned int i;
> +
> + for (i = 0; i < size; i++) {
> + if (fmt[i].pixfmt == pixfmt)
> + break;
> + }
> +
> + if (i == size || fmt[i].type != type)
> + return NULL;
> +
> + return &fmt[i];
> +}
> +
> +static const struct vidc_format *find_format_by_index(int index, u32 type)
> +{
> + const struct vidc_format *fmt = vdec_formats;
> + unsigned int size = ARRAY_SIZE(vdec_formats);
> + int i, k = 0;
> +
> + if (index < 0 || index > size)
> + return NULL;
> +
> + for (i = 0; i < size; i++) {
> + if (fmt[i].type != type)
> + continue;
> + if (k == index)
> + break;
> + k++;
> + }
> +
> + if (i == size)
> + return NULL;
> +
> + return &fmt[i];
> +}
Re: [RFC v2 17/17] omap3isp: Don't rely on devm for memory resource management
On Mon, Aug 22, 2016 at 05:02:31PM +0300, Sakari Ailus wrote:
> Hi Hans,
>
> On Mon, Aug 22, 2016 at 02:40:39PM +0200, Hans Verkuil wrote:
> > On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> > > devm functions are fine for managing resources that are directly related
> > > to the device at hand and that have no other dependencies. However, a
> > > process holding a file handle to a device created by a driver for a device
> > > may result in the file handle left behind after the device is long gone.
> > > This will result in accessing released (and potentially reallocated)
> > > memory.
> > >
> > > Instead, rely on the media device which will stick around until all users
> > > are gone.
> > >
> > > Signed-off-by: Sakari Ailus
> > > ---
> > > drivers/media/platform/omap3isp/isp.c | 38
> > > ---
> > > drivers/media/platform/omap3isp/ispccp2.c | 3 ++-
> > > drivers/media/platform/omap3isp/isph3a_aewb.c | 20 +-
> > > drivers/media/platform/omap3isp/isph3a_af.c | 20 +-
> > > drivers/media/platform/omap3isp/isphist.c | 5 ++--
> > > drivers/media/platform/omap3isp/ispstat.c | 2 ++
> > > 6 files changed, 63 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/omap3isp/isp.c
> > > b/drivers/media/platform/omap3isp/isp.c
> > > index 217d4da..3488ed3 100644
> > > --- a/drivers/media/platform/omap3isp/isp.c
> > > +++ b/drivers/media/platform/omap3isp/isp.c
> > > @@ -1370,7 +1370,7 @@ static int isp_get_clocks(struct isp_device *isp)
> > > unsigned int i;
> > >
> > > for (i = 0; i < ARRAY_SIZE(isp_clocks); ++i) {
> > > - clk = devm_clk_get(isp->dev, isp_clocks[i]);
> >
> > I wonder, would it be possible to use the media device itself for these
> > devm_
> > functions? Since the media device is the last one to be released...
>
> Do you happen to mean... struct media_device->devnode.dev?
>
> Interesting idea, I can't see why not. That'd actually make the required
> driver changes to fix the drivers quite a bit easier to make. And we could
> still use devm_() functions.
That might have been a little bit too fast.
In fact, device_release() in drivers/base/core.c does release resources
bound to device (devm) *before* calling the driver's release callback.
This means that the driver's device release callback may not access
resources allocated with devm functions. Considering that devm_() functions
have been broken and fixed before [1], could this be more than a bug?
The comments there seem off topic mostly.
[1] a525a3ddeaca69f405d98442ab3c0746e53168dc
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] docs-rst: add package adjustbox
We need adjustbox to allow adjusting the size of tables that
are bigger than the line width. There are quite a few of them
at the media books.
Signed-off-by: Mauro Carvalho Chehab
---
PS.: This changeset were fold into one of the media patches on a previous
patch series. However, in order to avoid merge conflicts during he merge
window, better to split and submit it via docs-next tree.
Documentation/conf.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/conf.py b/Documentation/conf.py
index 42045c26581b..c25e95d46272 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -326,6 +326,9 @@ latex_elements = {
\\setromanfont{DejaVu Sans}
\\setmonofont{DejaVu Sans Mono}
+ % To allow adjusting table sizes
+ \\usepackage{adjustbox}
+
'''
}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 17/17] omap3isp: Don't rely on devm for memory resource management
Hi Hans,
On Mon, Aug 22, 2016 at 02:40:39PM +0200, Hans Verkuil wrote:
> On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> > devm functions are fine for managing resources that are directly related
> > to the device at hand and that have no other dependencies. However, a
> > process holding a file handle to a device created by a driver for a device
> > may result in the file handle left behind after the device is long gone.
> > This will result in accessing released (and potentially reallocated)
> > memory.
> >
> > Instead, rely on the media device which will stick around until all users
> > are gone.
> >
> > Signed-off-by: Sakari Ailus
> > ---
> > drivers/media/platform/omap3isp/isp.c | 38
> > ---
> > drivers/media/platform/omap3isp/ispccp2.c | 3 ++-
> > drivers/media/platform/omap3isp/isph3a_aewb.c | 20 +-
> > drivers/media/platform/omap3isp/isph3a_af.c | 20 +-
> > drivers/media/platform/omap3isp/isphist.c | 5 ++--
> > drivers/media/platform/omap3isp/ispstat.c | 2 ++
> > 6 files changed, 63 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/media/platform/omap3isp/isp.c
> > b/drivers/media/platform/omap3isp/isp.c
> > index 217d4da..3488ed3 100644
> > --- a/drivers/media/platform/omap3isp/isp.c
> > +++ b/drivers/media/platform/omap3isp/isp.c
> > @@ -1370,7 +1370,7 @@ static int isp_get_clocks(struct isp_device *isp)
> > unsigned int i;
> >
> > for (i = 0; i < ARRAY_SIZE(isp_clocks); ++i) {
> > - clk = devm_clk_get(isp->dev, isp_clocks[i]);
>
> I wonder, would it be possible to use the media device itself for these devm_
> functions? Since the media device is the last one to be released...
Do you happen to mean... struct media_device->devnode.dev?
Interesting idea, I can't see why not. That'd actually make the required
driver changes to fix the drivers quite a bit easier to make. And we could
still use devm_() functions.
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 08/17] media-device: Make devnode.dev->kobj parent of devnode.cdev
On Mon, Aug 22, 2016 at 02:17:28PM +0200, Hans Verkuil wrote: > On 08/19/2016 12:23 PM, Sakari Ailus wrote: > > The struct cdev embedded in struct media_devnode contains its own kobj. > > Instead of trying to manage its lifetime separately from struct > > media_devnode, make the cdev kobj a parent of the struct media_device.dev > > kobj. > > > > The cdev will thus be released during unregistering the media_devnode, not > > in media_devnode.dev kobj's release callback. > > > > Signed-off-by: Sakari Ailus > > --- > > drivers/media/media-devnode.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c > > index aa8030b..69f84a7 100644 > > --- a/drivers/media/media-devnode.c > > +++ b/drivers/media/media-devnode.c > > @@ -63,9 +63,6 @@ static void media_devnode_release(struct device *cd) > > > > mutex_lock(&media_devnode_lock); > > > > - /* Delete the cdev on this minor as well */ > > - cdev_del(&devnode->cdev); > > - > > /* Mark device node number as free */ > > clear_bit(devnode->minor, media_devnode_nums); > > > > @@ -246,6 +243,7 @@ int __must_check media_devnode_register(struct > > media_devnode *devnode, > > > > /* Part 2: Initialize and register the character device */ > > cdev_init(&devnode->cdev, &media_devnode_fops); > > + devnode->cdev.kobj.parent = &devnode->dev.kobj; > > devnode->cdev.owner = owner; > > > > ret = cdev_add(&devnode->cdev, MKDEV(MAJOR(media_dev_t), > > devnode->minor), 1); > > @@ -291,6 +289,7 @@ void media_devnode_unregister(struct media_devnode > > *devnode) > > clear_bit(MEDIA_FLAG_REGISTERED, &devnode->flags); > > mutex_unlock(&media_devnode_lock); > > device_unregister(&devnode->dev); > > + cdev_del(&devnode->cdev); > > Are you sure about this order? Shouldn't cdev_del be called first? > > The register() calls cdev_add() before device_add(), so I would expect the > reverse order here. This is also what cec-core.c does. Correct. It's possible that device_unregister() releases the last reference to the struct device, which in turn causes the memory to be released. I'll fix that. -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/6] ARM64: dts: amlogic: add the input pin for the IR remote
On Sat, Aug 20, 2016 at 11:54 AM, Martin Blumenstingl wrote: > Signed-off-by: Martin Blumenstingl Acked-by: Linus Walleij Merge this through the ARM SoC tree. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/6] pinctrl: amlogic: gxbb: add the IR remote input pin
On Sat, Aug 20, 2016 at 11:54 AM, Martin Blumenstingl wrote: > This adds the IR remote receiver to the AO domain devices. > > Signed-off-by: Martin Blumenstingl > Reviewed-by: Kevin Hilman Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8] media: vidc: adding core part and helper functions
Hi Stanimir,
Thanks for this patch series!
I have some review comments:
On 08/22/2016 03:13 PM, Stanimir Varbanov wrote:
> diff --git a/drivers/media/platform/qcom/vidc/core.h
> b/drivers/media/platform/qcom/vidc/core.h
> new file mode 100644
> index ..5dc8e05f8c36
> --- /dev/null
> +++ b/drivers/media/platform/qcom/vidc/core.h
> @@ -0,0 +1,196 @@
> +/*
> + * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2016 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#ifndef __VIDC_CORE_H_
> +#define __VIDC_CORE_H_
> +
> +#include
> +#include
> +#include
> +
> +#include "resources.h"
> +#include "hfi.h"
> +
> +#define VIDC_DRV_NAME"vidc"
> +
> +struct vidc_list {
> + struct list_head list;
> + struct mutex lock;
> +};
> +
> +struct vidc_format {
> + u32 pixfmt;
> + int num_planes;
> + u32 type;
> +};
> +
> +struct vidc_core {
> + struct list_head list;
> + void __iomem *base;
> + int irq;
> + struct clk *clks[VIDC_CLKS_NUM_MAX];
> + struct mutex lock;
> + struct hfi_core hfi;
> + struct video_device vdev_dec;
> + struct video_device vdev_enc;
I know that many drivers embed struct video_device, but this can cause subtle
refcounting problems. I recommend changing this to a pointer and using
video_device_alloc().
I have plans to reorganize the way video_devices are allocated and registered in
the near future, and you might just as well prepare this driver for that by
switching
to a pointer.
> + struct v4l2_device v4l2_dev;
> + struct list_head instances;
> + const struct vidc_resources *res;
> + struct rproc *rproc;
> + bool rproc_booted;
> + struct device *dev;
> +};
> +
> +struct vdec_controls {
> + u32 post_loop_deb_mode;
> + u32 profile;
> + u32 level;
> +};
> +
> +struct venc_controls {
> + u16 gop_size;
> + u32 idr_period;
> + u32 num_p_frames;
> + u32 num_b_frames;
> + u32 bitrate_mode;
> + u32 bitrate;
> + u32 bitrate_peak;
> +
> + u32 h264_i_period;
> + u32 h264_entropy_mode;
> + u32 h264_i_qp;
> + u32 h264_p_qp;
> + u32 h264_b_qp;
> + u32 h264_min_qp;
> + u32 h264_max_qp;
> + u32 h264_loop_filter_mode;
> + u32 h264_loop_filter_alpha;
> + u32 h264_loop_filter_beta;
> +
> + u32 vp8_min_qp;
> + u32 vp8_max_qp;
> +
> + u32 multi_slice_mode;
> + u32 multi_slice_max_bytes;
> + u32 multi_slice_max_mb;
> +
> + u32 header_mode;
> +
> + u32 profile;
> + u32 level;
> +};
> +
> +struct vidc_inst {
> + struct list_head list;
> + struct mutex lock;
> + struct vidc_core *core;
> +
> + struct vidc_list scratchbufs;
> + struct vidc_list persistbufs;
> + struct vidc_list registeredbufs;
> +
> + struct list_head bufqueue;
> + struct mutex bufqueue_lock;
> +
> + int streamoff;
> + int streamon;
> + struct vb2_queue bufq_out;
> + struct vb2_queue bufq_cap;
> +
> + struct v4l2_ctrl_handler ctrl_handler;
> + union {
> + struct vdec_controls dec;
> + struct venc_controls enc;
> + } controls;
> + struct v4l2_fh fh;
> +
> + struct hfi_inst *hfi_inst;
> +
> + /* session fields */
> + u32 session_type;
> + u32 width;
> + u32 height;
> + u32 out_width;
> + u32 out_height;
> + u32 colorspace;
> + u8 ycbcr_enc;
> + u8 quantization;
> + u8 xfer_func;
> + u64 fps;
> + struct v4l2_fract timeperframe;
> + const struct vidc_format *fmt_out;
> + const struct vidc_format *fmt_cap;
> + unsigned int num_input_bufs;
> + unsigned int num_output_bufs;
> + bool in_reconfig;
> + u32 reconfig_width;
> + u32 reconfig_height;
> + u64 sequence;
> +};
> +
> +#define ctrl_to_inst(ctrl) \
> + container_of(ctrl->handler, struct vidc_inst, ctrl_handler)
> +
> +struct vidc_ctrl {
> + u32 id;
> + enum v4l2_ctrl_type type;
> + s32 min;
> + s32 max;
> + s32 def;
> + u32 step;
> + u64 menu_skip_mask;
> + u32 flags;
> + const char * const *qmenu;
> +};
> +
> +/*
> + * Offset base for buffers on the destination queue - used to distinguish
> + * between source and destination buffers when mmapping - they receive the
> same
> + * offsets but for different queues
> + */
> +#define DST_QUEUE_OFF_BASE (1 << 30)
> +
> +extern const struct v4l2_file_operations vidc_fops;
> +
> +static inline void INIT_VIDC_LIST(struct vidc_list *mlist)
[PATCH 8/8] media: vidc: enable building of the video codec driver
This adds changes in v4l2 platform directory to include the vidc driver and show it in kernel config. Signed-off-by: Stanimir Varbanov --- drivers/media/platform/Kconfig | 1 + drivers/media/platform/Makefile | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index f25344bc7912..e52640417b0a 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -111,6 +111,7 @@ source "drivers/media/platform/s5p-tv/Kconfig" source "drivers/media/platform/am437x/Kconfig" source "drivers/media/platform/xilinx/Kconfig" source "drivers/media/platform/rcar-vin/Kconfig" +source "drivers/media/platform/qcom/Kconfig" config VIDEO_TI_CAL tristate "TI CAL (Camera Adaptation Layer) driver" diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index 21771c1a13fb..d8fc75ddcc73 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -51,6 +51,7 @@ obj-$(CONFIG_VIDEO_RENESAS_JPU) += rcar_jpu.o obj-$(CONFIG_VIDEO_RENESAS_VSP1) += vsp1/ obj-y += omap/ +obj-y += qcom/ obj-$(CONFIG_VIDEO_AM437X_VPFE)+= am437x/ -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] media: vidc: add Venus HFI files
Here is the implementation of Venus video accelerator low-level
functionality. It contanins code which setup the registers and
startup uthe processor, allocate and manipulates with the shared
memory used for sending commands and receiving messages.
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/vidc/hfi_venus.c| 1539 +++
drivers/media/platform/qcom/vidc/hfi_venus.h| 25 +
drivers/media/platform/qcom/vidc/hfi_venus_io.h | 98 ++
3 files changed, 1662 insertions(+)
create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus.c
create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus_io.h
diff --git a/drivers/media/platform/qcom/vidc/hfi_venus.c
b/drivers/media/platform/qcom/vidc/hfi_venus.c
new file mode 100644
index ..1193fa136711
--- /dev/null
+++ b/drivers/media/platform/qcom/vidc/hfi_venus.c
@@ -0,0 +1,1539 @@
+/*
+ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "hfi_cmds.h"
+#include "hfi_msgs.h"
+#include "mem.h"
+#include "hfi_venus.h"
+#include "hfi_venus_io.h"
+
+#define HFI_MASK_QHDR_TX_TYPE 0xff00
+#define HFI_MASK_QHDR_RX_TYPE 0x00ff
+#define HFI_MASK_QHDR_PRI_TYPE 0xff00
+#define HFI_MASK_QHDR_ID_TYPE 0x00ff
+
+#define HFI_HOST_TO_CTRL_CMD_Q 0
+#define HFI_CTRL_TO_HOST_MSG_Q 1
+#define HFI_CTRL_TO_HOST_DBG_Q 2
+#define HFI_MASK_QHDR_STATUS 0x00ff
+
+#define IFACEQ_NUM 3
+#define IFACEQ_CMD_IDX 0
+#define IFACEQ_MSG_IDX 1
+#define IFACEQ_DBG_IDX 2
+#define IFACEQ_MAX_BUF_COUNT 50
+#define IFACEQ_MAX_PARALLEL_CLNTS 16
+#define IFACEQ_DFLT_QHDR 0x0101
+
+#define POLL_INTERVAL_US 50
+
+#define IFACEQ_MAX_PKT_SIZE1024
+#define IFACEQ_MED_PKT_SIZE768
+#define IFACEQ_MIN_PKT_SIZE8
+#define IFACEQ_VAR_SMALL_PKT_SIZE 100
+#define IFACEQ_VAR_LARGE_PKT_SIZE 512
+#define IFACEQ_VAR_HUGE_PKT_SIZE (1024 * 12)
+
+enum tzbsp_video_state {
+ TZBSP_VIDEO_STATE_SUSPEND = 0,
+ TZBSP_VIDEO_STATE_RESUME
+};
+
+struct hfi_queue_table_header {
+ u32 version;
+ u32 size;
+ u32 qhdr0_offset;
+ u32 qhdr_size;
+ u32 num_q;
+ u32 num_active_q;
+};
+
+struct hfi_queue_header {
+ u32 status;
+ u32 start_addr;
+ u32 type;
+ u32 q_size;
+ u32 pkt_size;
+ u32 pkt_drop_cnt;
+ u32 rx_wm;
+ u32 tx_wm;
+ u32 rx_req;
+ u32 tx_req;
+ u32 rx_irq_status;
+ u32 tx_irq_status;
+ u32 read_idx;
+ u32 write_idx;
+};
+
+#define IFACEQ_TABLE_SIZE \
+ (sizeof(struct hfi_queue_table_header) +\
+sizeof(struct hfi_queue_header) * IFACEQ_NUM)
+
+#define IFACEQ_QUEUE_SIZE (IFACEQ_MAX_PKT_SIZE * \
+ IFACEQ_MAX_BUF_COUNT * IFACEQ_MAX_PARALLEL_CLNTS)
+
+#define IFACEQ_GET_QHDR_START_ADDR(ptr, i) \
+ (void *)(((ptr) + sizeof(struct hfi_queue_table_header)) + \
+ ((i) * sizeof(struct hfi_queue_header)))
+
+#define QDSS_SIZE SZ_4K
+#define SFR_SIZE SZ_4K
+#define QUEUE_SIZE \
+ (IFACEQ_TABLE_SIZE + (IFACEQ_QUEUE_SIZE * IFACEQ_NUM))
+
+#define ALIGNED_QDSS_SIZE ALIGN(QDSS_SIZE, SZ_4K)
+#define ALIGNED_SFR_SIZE ALIGN(SFR_SIZE, SZ_4K)
+#define ALIGNED_QUEUE_SIZE ALIGN(QUEUE_SIZE, SZ_4K)
+#define SHARED_QSIZE ALIGN(ALIGNED_SFR_SIZE + ALIGNED_QUEUE_SIZE + \
+ ALIGNED_QDSS_SIZE, SZ_1M)
+
+struct mem_desc {
+ u32 da; /* device address */
+ void *kva; /* kernel virtual address */
+ u32 size;
+ struct vidc_mem *mem;
+};
+
+struct iface_queue {
+ struct hfi_queue_header *qhdr;
+ struct mem_desc qmem;
+};
+
+enum venus_state {
+ VENUS_STATE_DEINIT = 1,
+ VENUS_STATE_INIT,
+};
+
+struct venus_hfi_device {
+ struct device *dev;
+ void __iomem *base;
+ u32 irq_status;
+ u32 last_packet_type;
+ bool power_enabled;
+ bool suspended;
+ struct completion pwr_collapse_prep;
+ struct completion release_resource;
+ struct mutex lock;
+ struct mem_desc ifa
[PATCH 7/8] media: vidc: add Makefiles and Kconfig files
Makefile and Kconfig files to build the video codec driver. Signed-off-by: Stanimir Varbanov --- drivers/media/platform/qcom/Kconfig | 8 drivers/media/platform/qcom/Makefile | 6 ++ drivers/media/platform/qcom/vidc/Makefile | 19 +++ 3 files changed, 33 insertions(+) create mode 100644 drivers/media/platform/qcom/Kconfig create mode 100644 drivers/media/platform/qcom/Makefile create mode 100644 drivers/media/platform/qcom/vidc/Makefile diff --git a/drivers/media/platform/qcom/Kconfig b/drivers/media/platform/qcom/Kconfig new file mode 100644 index ..4bad5c0f68e4 --- /dev/null +++ b/drivers/media/platform/qcom/Kconfig @@ -0,0 +1,8 @@ +comment "Qualcomm V4L2 drivers" + +menuconfig QCOM_VIDC +tristate "Qualcomm V4L2 encoder/decoder driver" +depends on ARCH_QCOM && VIDEO_V4L2 +depends on IOMMU_DMA +depends on QCOM_VENUS_PIL +select VIDEOBUF2_DMA_SG diff --git a/drivers/media/platform/qcom/Makefile b/drivers/media/platform/qcom/Makefile new file mode 100644 index ..150892f6533b --- /dev/null +++ b/drivers/media/platform/qcom/Makefile @@ -0,0 +1,6 @@ +# +# Makefile for the QCOM spcific video device drivers +# based on V4L2. +# + +obj-$(CONFIG_QCOM_VIDC) += vidc/ diff --git a/drivers/media/platform/qcom/vidc/Makefile b/drivers/media/platform/qcom/vidc/Makefile new file mode 100644 index ..56f5c3924855 --- /dev/null +++ b/drivers/media/platform/qcom/vidc/Makefile @@ -0,0 +1,19 @@ +# Makefile for Qualcomm vidc driver + +vidc-objs += \ + core.o \ + int_bufs.o \ + helpers.o \ + vdec.o \ + vdec_ctrls.o \ + venc.o \ + venc_ctrls.o \ + mem.o \ + resources.o \ + load.o \ + hfi_venus.o \ + hfi_msgs.o \ + hfi_cmds.o \ + hfi.o \ + +obj-$(CONFIG_QCOM_VIDC) += vidc.o -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/8] doc: DT: vidc: binding document for Qualcomm video driver
Adds binding document for vidc video encoder/decoder driver
Cc: Rob Herring
Cc: Mark Rutland
Cc: devicet...@vger.kernel.org
Signed-off-by: Stanimir Varbanov
---
.../devicetree/bindings/media/qcom,vidc.txt| 61 ++
1 file changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/qcom,vidc.txt
diff --git a/Documentation/devicetree/bindings/media/qcom,vidc.txt
b/Documentation/devicetree/bindings/media/qcom,vidc.txt
new file mode 100644
index ..0d50a7b2e3ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,vidc.txt
@@ -0,0 +1,61 @@
+* Qualcomm video encoder/decoder accelerator
+
+- compatible:
+ Usage: required
+ Value type:
+ Definition: Value should contain
+ - "qcom,vidc-msm8916"
+ - "qcom,vidc-msm8996"
+- reg:
+ Usage: required
+ Value type:
+ Definition: Register ranges as listed in the reg-names property
+
+- interrupts:
+ Usage: required
+ Value type:
+ Definition:
+
+- power-domains:
+ Usage: required
+ Value type:
+ Definition: A phandle and power domain specifier pairs to the
+ power domain which is responsible for collapsing
+ and restoring power to the peripheral
+
+- clocks:
+ Usage: required
+ Value type:
+ Definition: List of phandle and clock specifier pairs as listed
+ in clock-names property
+- clock-names:
+ Usage: required
+ Value type:
+ Definition: Should contain the following entries
+ - "core" Core video accelerator clock
+ - "iface" Video accelerator AHB clock
+ - "bus" Video accelerator AXI clock
+- rproc:
+ Usage: required
+ Value type:
+ Definition: A phandle to remote processor responsible for
+ firmware loading
+
+- iommus:
+ Usage: required
+ Value type:
+ Definition: A list of phandle and IOMMU specifier pairs
+
+* An Example
+ qcom,vidc@1d0 {
+ compatible = "qcom,vidc-msm8916";
+ reg = <0x01d0 0xff000>;
+ clocks = <&gcc GCC_VENUS0_VCODEC0_CLK>,
+<&gcc GCC_VENUS0_AHB_CLK>,
+<&gcc GCC_VENUS0_AXI_CLK>;
+ clock-names = "core", "iface", "bus";
+ interrupts = ;
+ power-domains = <&gcc VENUS_GDSC>;
+ rproc = <&vidc_rproc>;
+ iommus = <&apps_iommu 5>;
+ };
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/8] media: vidc: encoder: add video encoder files
This adds encoder part of the driver plus encoder controls.
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/vidc/venc.c | 1261 +
drivers/media/platform/qcom/vidc/venc.h | 27 +
drivers/media/platform/qcom/vidc/venc_ctrls.c | 396
drivers/media/platform/qcom/vidc/venc_ctrls.h | 23 +
4 files changed, 1707 insertions(+)
create mode 100644 drivers/media/platform/qcom/vidc/venc.c
create mode 100644 drivers/media/platform/qcom/vidc/venc.h
create mode 100644 drivers/media/platform/qcom/vidc/venc_ctrls.c
create mode 100644 drivers/media/platform/qcom/vidc/venc_ctrls.h
diff --git a/drivers/media/platform/qcom/vidc/venc.c
b/drivers/media/platform/qcom/vidc/venc.c
new file mode 100644
index ..bc44f419089e
--- /dev/null
+++ b/drivers/media/platform/qcom/vidc/venc.c
@@ -0,0 +1,1261 @@
+/*
+ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "core.h"
+#include "helpers.h"
+#include "load.h"
+#include "venc_ctrls.h"
+
+#define NUM_B_FRAMES_MAX 4
+
+static u32 get_framesize_nv12(int plane, u32 height, u32 width)
+{
+ u32 y_stride, uv_stride, y_plane;
+ u32 y_sclines, uv_sclines, uv_plane;
+ u32 size;
+
+ y_stride = ALIGN(width, 128);
+ uv_stride = ALIGN(width, 128);
+ y_sclines = ALIGN(height, 32);
+ uv_sclines = ALIGN(((height + 1) >> 1), 16);
+
+ y_plane = y_stride * y_sclines;
+ uv_plane = uv_stride * uv_sclines + SZ_4K;
+ size = y_plane + uv_plane + SZ_8K;
+ size = ALIGN(size, SZ_4K);
+
+ return size;
+}
+
+static u32 get_framesize_compressed(u32 height, u32 width)
+{
+ u32 sz = ALIGN(height, 32) * ALIGN(width, 32) * 3 / 2;
+
+ return ALIGN(sz, SZ_4K);
+}
+
+static const struct vidc_format venc_formats[] = {
+ {
+ .pixfmt = V4L2_PIX_FMT_NV12,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_MPEG4,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_H263,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_H264,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_VP8,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+ },
+};
+
+static const struct vidc_format *find_format(u32 pixfmt, int type)
+{
+ const struct vidc_format *fmt = venc_formats;
+ unsigned int size = ARRAY_SIZE(venc_formats);
+ unsigned int i;
+
+ for (i = 0; i < size; i++) {
+ if (fmt[i].pixfmt == pixfmt)
+ break;
+ }
+
+ if (i == size || fmt[i].type != type)
+ return NULL;
+
+ return &fmt[i];
+}
+
+static const struct vidc_format *find_format_by_index(int index, int type)
+{
+ const struct vidc_format *fmt = venc_formats;
+ unsigned int size = ARRAY_SIZE(venc_formats);
+ int i, k = 0;
+
+ if (index < 0 || index > size)
+ return NULL;
+
+ for (i = 0; i < size; i++) {
+ if (fmt[i].type != type)
+ continue;
+ if (k == index)
+ break;
+ k++;
+ }
+
+ if (i == size)
+ return NULL;
+
+ return &fmt[i];
+}
+
+static int venc_v4l2_to_hfi(int id, int value)
+{
+ switch (id) {
+ case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
+ switch (value) {
+ case V4L2_MPEG_VIDEO_MPEG4_LEVEL_0:
+ default:
+ return HFI_MPEG4_LEVEL_0;
+ case V4L2_MPEG_VIDEO_MPEG4_LEVEL_0B:
+ return HFI_MPEG4_LEVEL_0b;
+ case V4L2_MPEG_VIDEO_MPEG4_LEVEL_1:
+ return HFI_MPEG4_LEVEL_1;
+ case V4L2_MPEG_VIDEO_MPEG4_LEVEL_2:
+ return HFI_MPEG4_LEVEL_2;
+ case V4L2_MPEG_VIDEO_MPEG4_LEVEL_3:
+ return HFI_MPEG4_LEVEL_3;
+ case V4L2_MPEG_VIDEO_MPEG4_LEVEL_4:
+ return HFI_MPEG4_LEVEL_4;
+ ca
[PATCH 5/8] media: vidc: add Host Firmware Interface (HFI)
This is the implementation of HFI. It is loaded with the
responsibility to comunicate with the firmware through an
interface commands and messages.
- hfi.c has interface functions used by the core, decoder
and encoder parts to comunicate with the firmware. For example
there are functions for session and core initialisation.
- hfi_cmds has packetization operations which preparing
packets to be send from host to firmware.
- hfi_msgs takes care of messages sent from firmware to the
host.
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/vidc/hfi.c| 622
drivers/media/platform/qcom/vidc/hfi.h| 272 ++
drivers/media/platform/qcom/vidc/hfi_cmds.c | 1261 +
drivers/media/platform/qcom/vidc/hfi_cmds.h | 338 +++
drivers/media/platform/qcom/vidc/hfi_helper.h | 1143 ++
drivers/media/platform/qcom/vidc/hfi_msgs.c | 1072 +
drivers/media/platform/qcom/vidc/hfi_msgs.h | 298 ++
7 files changed, 5006 insertions(+)
create mode 100644 drivers/media/platform/qcom/vidc/hfi.c
create mode 100644 drivers/media/platform/qcom/vidc/hfi.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi_cmds.c
create mode 100644 drivers/media/platform/qcom/vidc/hfi_cmds.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi_helper.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi_msgs.c
create mode 100644 drivers/media/platform/qcom/vidc/hfi_msgs.h
diff --git a/drivers/media/platform/qcom/vidc/hfi.c
b/drivers/media/platform/qcom/vidc/hfi.c
new file mode 100644
index ..030b50082ff2
--- /dev/null
+++ b/drivers/media/platform/qcom/vidc/hfi.c
@@ -0,0 +1,622 @@
+/*
+ * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "hfi.h"
+#include "hfi_cmds.h"
+#include "hfi_venus.h"
+
+#define TIMEOUTmsecs_to_jiffies(1000)
+
+static u32 to_codec_type(u32 pixfmt)
+{
+ switch (pixfmt) {
+ case V4L2_PIX_FMT_H264:
+ case V4L2_PIX_FMT_H264_NO_SC:
+ return HFI_VIDEO_CODEC_H264;
+ case V4L2_PIX_FMT_H263:
+ return HFI_VIDEO_CODEC_H263;
+ case V4L2_PIX_FMT_MPEG1:
+ return HFI_VIDEO_CODEC_MPEG1;
+ case V4L2_PIX_FMT_MPEG2:
+ return HFI_VIDEO_CODEC_MPEG2;
+ case V4L2_PIX_FMT_MPEG4:
+ return HFI_VIDEO_CODEC_MPEG4;
+ case V4L2_PIX_FMT_VC1_ANNEX_G:
+ case V4L2_PIX_FMT_VC1_ANNEX_L:
+ return HFI_VIDEO_CODEC_VC1;
+ case V4L2_PIX_FMT_VP8:
+ return HFI_VIDEO_CODEC_VP8;
+ case V4L2_PIX_FMT_XVID:
+ return HFI_VIDEO_CODEC_DIVX;
+ default:
+ return 0;
+ }
+}
+
+int vidc_hfi_core_init(struct hfi_core *hfi)
+{
+ int ret = 0;
+
+ mutex_lock(&hfi->lock);
+
+ if (hfi->state >= CORE_INIT)
+ goto unlock;
+
+ init_completion(&hfi->done);
+
+ ret = call_hfi_op(hfi, core_init, hfi);
+ if (ret)
+ goto unlock;
+
+ ret = wait_for_completion_timeout(&hfi->done, TIMEOUT);
+ if (!ret) {
+ ret = -ETIMEDOUT;
+ goto unlock;
+ }
+
+ ret = 0;
+
+ if (hfi->error != HFI_ERR_NONE) {
+ ret = -EIO;
+ goto unlock;
+ }
+
+ hfi->state = CORE_INIT;
+unlock:
+ mutex_unlock(&hfi->lock);
+ return ret;
+}
+
+int vidc_hfi_core_deinit(struct hfi_core *hfi)
+{
+ struct device *dev = hfi->dev;
+ int ret = 0;
+
+ mutex_lock(&hfi->lock);
+
+ if (hfi->state == CORE_UNINIT)
+ goto unlock;
+
+ if (!list_empty(&hfi->instances)) {
+ ret = -EBUSY;
+ goto unlock;
+ }
+
+ /*
+* Delay unloading of firmware. This is useful
+* in avoiding firmware download delays in cases where we
+* will have a burst of back to back video playback sessions
+* e.g. thumbnail generation.
+*/
+ ret = call_hfi_op(hfi, core_deinit, hfi);
+ if (ret)
+ dev_err(dev, "core deinit failed: %d\n", ret);
+
+ hfi->state = CORE_UNINIT;
+
+unlock:
+ mutex_unlock(&hfi->lock);
+ return ret;
+}
+
+int vidc_hfi_core_suspend(struct hfi_core *hfi)
+{
+ return call_hfi_op(hfi, suspend, hfi);
+}
+
+int vidc_hfi_core_resume(struct hfi_core *hfi)
+
[PATCH 3/8] media: vidc: decoder: add video decoder files
This consists of video decoder implementation plus decoder
controls.
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/vidc/vdec.c | 1100 +
drivers/media/platform/qcom/vidc/vdec.h | 27 +
drivers/media/platform/qcom/vidc/vdec_ctrls.c | 200 +
drivers/media/platform/qcom/vidc/vdec_ctrls.h | 21 +
4 files changed, 1348 insertions(+)
create mode 100644 drivers/media/platform/qcom/vidc/vdec.c
create mode 100644 drivers/media/platform/qcom/vidc/vdec.h
create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.c
create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.h
diff --git a/drivers/media/platform/qcom/vidc/vdec.c
b/drivers/media/platform/qcom/vidc/vdec.c
new file mode 100644
index ..a631a354742f
--- /dev/null
+++ b/drivers/media/platform/qcom/vidc/vdec.c
@@ -0,0 +1,1100 @@
+/*
+ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "core.h"
+#include "helpers.h"
+#include "load.h"
+#include "vdec.h"
+#include "vdec_ctrls.h"
+
+#define MACROBLOCKS_PER_PIXEL (16 * 16)
+
+static u32 get_framesize_nv12(int plane, u32 height, u32 width)
+{
+ u32 y_stride, uv_stride, y_plane;
+ u32 y_sclines, uv_sclines, uv_plane;
+ u32 size;
+
+ y_stride = ALIGN(width, 128);
+ uv_stride = ALIGN(width, 128);
+ y_sclines = ALIGN(height, 32);
+ uv_sclines = ALIGN(((height + 1) >> 1), 16);
+
+ y_plane = y_stride * y_sclines;
+ uv_plane = uv_stride * uv_sclines + SZ_4K;
+ size = y_plane + uv_plane + SZ_8K;
+
+ return ALIGN(size, SZ_4K);
+}
+
+static u32 get_framesize_compressed(u32 mbs_per_frame)
+{
+ return ((mbs_per_frame * MACROBLOCKS_PER_PIXEL * 3 / 2) / 2) + 128;
+}
+
+static const struct vidc_format vdec_formats[] = {
+ {
+ .pixfmt = V4L2_PIX_FMT_NV12,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_MPEG4,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_MPEG2,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_H263,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_H264,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_VP8,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_XVID,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ },
+};
+
+static const struct vidc_format *find_format(u32 pixfmt, u32 type)
+{
+ const struct vidc_format *fmt = vdec_formats;
+ unsigned int size = ARRAY_SIZE(vdec_formats);
+ unsigned int i;
+
+ for (i = 0; i < size; i++) {
+ if (fmt[i].pixfmt == pixfmt)
+ break;
+ }
+
+ if (i == size || fmt[i].type != type)
+ return NULL;
+
+ return &fmt[i];
+}
+
+static const struct vidc_format *find_format_by_index(int index, u32 type)
+{
+ const struct vidc_format *fmt = vdec_formats;
+ unsigned int size = ARRAY_SIZE(vdec_formats);
+ int i, k = 0;
+
+ if (index < 0 || index > size)
+ return NULL;
+
+ for (i = 0; i < size; i++) {
+ if (fmt[i].type != type)
+ continue;
+ if (k == index)
+ break;
+ k++;
+ }
+
+ if (i == size)
+ return NULL;
+
+ return &fmt[i];
+}
+
+static int vdec_set_properties(struct vidc_inst *inst)
+{
+ struct vdec_controls *ctr = &inst->controls.dec;
+ struct hfi_core *hfi = &inst->core->hfi;
+ struct hfi_enable en = { .enable = 1 };
[PATCH 2/8] media: vidc: adding core part and helper functions
This adds core part of the vidc driver common helper functions
used by encoder and decoder specific files.
- core.c has implemented the platform dirver methods, file
operations and v4l2 registration.
- helpers.c has implemented common helper functions for
buffer management, vb2_ops and functions for format propagation.
- int_bufs.c implements functions for allocating and freeing
buffers for internal usage. The buffer parameters describing
internal buffers depends on current format, resolution and
codec.
- load.c consists functions for calculation of current load
of the hardware. Depending on the count of instances and
resolutions it selects the best clock rate for the video
core.
- mem.c has two functions for memory allocation, currently
those functions are used for internal buffers and to allocate
the shared memory for communication with firmware via HFI
(Host Firmware Interface) interface commands.
- resources.c exports a structure describing the details
specific to platform and SoC.
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/vidc/core.c | 548 +++
drivers/media/platform/qcom/vidc/core.h | 196 ++
drivers/media/platform/qcom/vidc/helpers.c | 394 +++
drivers/media/platform/qcom/vidc/helpers.h | 43 +++
drivers/media/platform/qcom/vidc/int_bufs.c | 325
drivers/media/platform/qcom/vidc/int_bufs.h | 23 ++
drivers/media/platform/qcom/vidc/load.c | 104 +
drivers/media/platform/qcom/vidc/load.h | 22 ++
drivers/media/platform/qcom/vidc/mem.c | 64
drivers/media/platform/qcom/vidc/mem.h | 32 ++
drivers/media/platform/qcom/vidc/resources.c | 46 +++
drivers/media/platform/qcom/vidc/resources.h | 46 +++
12 files changed, 1843 insertions(+)
create mode 100644 drivers/media/platform/qcom/vidc/core.c
create mode 100644 drivers/media/platform/qcom/vidc/core.h
create mode 100644 drivers/media/platform/qcom/vidc/helpers.c
create mode 100644 drivers/media/platform/qcom/vidc/helpers.h
create mode 100644 drivers/media/platform/qcom/vidc/int_bufs.c
create mode 100644 drivers/media/platform/qcom/vidc/int_bufs.h
create mode 100644 drivers/media/platform/qcom/vidc/load.c
create mode 100644 drivers/media/platform/qcom/vidc/load.h
create mode 100644 drivers/media/platform/qcom/vidc/mem.c
create mode 100644 drivers/media/platform/qcom/vidc/mem.h
create mode 100644 drivers/media/platform/qcom/vidc/resources.c
create mode 100644 drivers/media/platform/qcom/vidc/resources.h
diff --git a/drivers/media/platform/qcom/vidc/core.c
b/drivers/media/platform/qcom/vidc/core.c
new file mode 100644
index ..e005be178fc0
--- /dev/null
+++ b/drivers/media/platform/qcom/vidc/core.c
@@ -0,0 +1,548 @@
+/*
+ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "core.h"
+#include "resources.h"
+#include "vdec.h"
+#include "venc.h"
+
+static void vidc_add_inst(struct vidc_core *core, struct vidc_inst *inst)
+{
+ mutex_lock(&core->lock);
+ list_add_tail(&inst->list, &core->instances);
+ mutex_unlock(&core->lock);
+}
+
+static void vidc_del_inst(struct vidc_core *core, struct vidc_inst *inst)
+{
+ struct vidc_inst *pos, *n;
+
+ mutex_lock(&core->lock);
+ list_for_each_entry_safe(pos, n, &core->instances, list) {
+ if (pos == inst)
+ list_del(&inst->list);
+ }
+ mutex_unlock(&core->lock);
+}
+
+static int vidc_rproc_boot(struct vidc_core *core)
+{
+ int ret;
+
+ if (core->rproc_booted)
+ return 0;
+
+ ret = rproc_boot(core->rproc);
+ if (ret)
+ return ret;
+
+ core->rproc_booted = true;
+
+ return 0;
+}
+
+static void vidc_rproc_shutdown(struct vidc_core *core)
+{
+ if (!core->rproc_booted)
+ return;
+
+ rproc_shutdown(core->rproc);
+ core->rproc_booted = false;
+}
+
+struct vidc_sys_error {
+ struct vidc_core *core;
+ struct delayed_work work;
+};
+
+static void vidc_sys_error_handler(struct work_struct *work)
+{
+ struct vidc_sys_error *handler =
+ container_of(work, struct vidc_sys_error, work.work);
+ struct vidc_core *core = handler->core;
+ struct hfi_core *hfi = &core->hfi;
[PATCH 0/8] Qualcomm video decoder/encoder driver
This patchset introduces a basic support for Qualcomm video acceleration hardware used for video stream decoding/encoding. The video IP can found on various qcom SoCs like apq8084, msm8916 and msm8996, hence it is widly distributed but the driver is missing in the mainline. The v4l2 driver is something like a wrapper over Host Firmware Interface. The HFI itself is a set of command and message packets send/received through shared memory, and its purpose is to comunicate with the firmware which is run on remote processor. The Venus is the name of the video hardware IP that doing the video acceleration. >From the software point of view the HFI interface is implemented in the files with prefix hfi_xxx. It acts as a translation layer between HFI and v4l2 layer. There is one special file in the driver called hfi_venus which doing most of the driver orchestration work. Something more it setups Venus core, run it and handle commands and messages from low-level point of view with the help of provided functions by HFI interface. I think that the driver is in good shape for mainline kernel, and I hope the review comments will help to improve it, so please do review and make comments. The driver depends on: - venus remoteproc driver posted at [1]. - out-of-tree qcom IOMMU driver and IOMMU probe deferral support at [2]. The driver has been tested on db410c (with apq8016 SoC) with simple v4l2 test applications and with gstreamer v4l2 videodec plugin, and v4l2 h264 out-of-tree gstreamer videoenc plugin. The output of v4l2-compliance test looks like: root@dragonboard-410c:/home/linaro# ./v4l2-compliance -d /dev/video0 v4l2-compliance SHA : ee1ab491019f80052834d14c76bdd1c1b46f2158 Driver Info: Driver name : vidc Card type : video decoder Bus info : platform:vidc Driver version: 4.8.0 Capabilities : 0x84204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Device Capabilities Device Caps : 0x04204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Compliance test for device /dev/video0 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video 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: 7 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK 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 (Not Supported) test Composing: OK (Not Supported) 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 input 0: Total: 43, Succeeded: 43, Failed: 0, Warnings: 0 root@dragonboard-410c:/home/linaro# ./v4l2-compliance -d /dev/video1 v4l2-compliance SHA : ee1ab491019f80052834d14c76bdd1c1b46f2158 Driver Info: Driver name : vidc Card type : video encoder Bus info : platform:vidc Driver version: 4.8.0 Capabilities : 0x84204000
Re: [PATCH v4 1/5] media: Determine early whether an IOCTL is supported
On 08/11/2016 10:29 PM, Sakari Ailus wrote:
> Preparation for refactoring media IOCTL handling to unify common parts.
>
> Reviewed-by: Laurent Pinchart
> Signed-off-by: Sakari Ailus
Acked-by: Hans Verkuil
> ---
> drivers/media/media-device.c | 54
> ++--
> 1 file changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 1795abe..aedd64e 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -419,13 +419,41 @@ static long media_device_get_topology(struct
> media_device *mdev,
> return 0;
> }
>
> -static long media_device_ioctl(struct file *filp, unsigned int cmd,
> -unsigned long arg)
> +#define MEDIA_IOC(__cmd) \
> + [_IOC_NR(MEDIA_IOC_##__cmd)] = { .cmd = MEDIA_IOC_##__cmd }
> +
> +/* the table is indexed by _IOC_NR(cmd) */
> +struct media_ioctl_info {
> + unsigned int cmd;
> +};
> +
> +static const struct media_ioctl_info ioctl_info[] = {
> + MEDIA_IOC(DEVICE_INFO),
> + MEDIA_IOC(ENUM_ENTITIES),
> + MEDIA_IOC(ENUM_LINKS),
> + MEDIA_IOC(SETUP_LINK),
> + MEDIA_IOC(G_TOPOLOGY),
> +};
> +
> +static inline long is_valid_ioctl(const struct media_ioctl_info *info,
> + unsigned int cmd)
> +{
> + return (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info)
> + || info[_IOC_NR(cmd)].cmd != cmd) ? -ENOIOCTLCMD : 0;
> +}
> +
> +static long __media_device_ioctl(
> + struct file *filp, unsigned int cmd, void __user *arg,
> + const struct media_ioctl_info *info_array)
> {
> struct media_devnode *devnode = media_devnode_data(filp);
> struct media_device *dev = devnode->media_dev;
> long ret;
>
> + ret = is_valid_ioctl(info_array, cmd);
> + if (ret)
> + return ret;
> +
> mutex_lock(&dev->graph_mutex);
> switch (cmd) {
> case MEDIA_IOC_DEVICE_INFO:
> @@ -461,6 +489,13 @@ static long media_device_ioctl(struct file *filp,
> unsigned int cmd,
> return ret;
> }
>
> +static long media_device_ioctl(struct file *filp, unsigned int cmd,
> +unsigned long arg)
> +{
> + return __media_device_ioctl(
> + filp, cmd, (void __user *)arg, ioctl_info);
> +}
> +
> #ifdef CONFIG_COMPAT
>
> struct media_links_enum32 {
> @@ -491,6 +526,14 @@ static long media_device_enum_links32(struct
> media_device *mdev,
>
> #define MEDIA_IOC_ENUM_LINKS32 _IOWR('|', 0x02, struct
> media_links_enum32)
>
> +static const struct media_ioctl_info compat_ioctl_info[] = {
> + MEDIA_IOC(DEVICE_INFO),
> + MEDIA_IOC(ENUM_ENTITIES),
> + MEDIA_IOC(ENUM_LINKS32),
> + MEDIA_IOC(SETUP_LINK),
> + MEDIA_IOC(G_TOPOLOGY),
> +};
> +
> static long media_device_compat_ioctl(struct file *filp, unsigned int cmd,
> unsigned long arg)
> {
> @@ -498,6 +541,13 @@ static long media_device_compat_ioctl(struct file *filp,
> unsigned int cmd,
> struct media_device *dev = devnode->media_dev;
> long ret;
>
> + /*
> + * The number of supported IOCTLs is the same for both regular and
> + * compat cases. Instead of passing the sizes around, ensure that
> + * they match.
> + */
> + BUILD_BUG_ON(ARRAY_SIZE(ioctl_info) != ARRAY_SIZE(compat_ioctl_info));
> +
> switch (cmd) {
> case MEDIA_IOC_ENUM_LINKS32:
> mutex_lock(&dev->graph_mutex);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/5] media: Support variable size IOCTL arguments
On 08/22/2016 02:55 PM, Hans Verkuil wrote:
> On 08/11/2016 10:29 PM, Sakari Ailus wrote:
>> Maintain a list of supported IOCTL argument sizes and allow only those in
>> the list.
>>
>> As an additional bonus, IOCTL handlers will be able to check whether the
>> caller actually set (using the argument size) the field vs. assigning it
>> to zero. Separate macro can be provided for that.
>>
>> This will be easier for applications as well since there is no longer the
>> problem of setting the reserved fields zero, or at least it is a lesser
>> problem.
>>
>> Signed-off-by: Sakari Ailus
>
> Acked-by: Hans Verkuil
As discussed in v3, I will probably park this patch for now since we don't need
this functionality today.
Regards,
Hans
>
>> ---
>> drivers/media/media-device.c | 56
>>
>> 1 file changed, 51 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
>> index 6f565a2..aa37520 100644
>> --- a/drivers/media/media-device.c
>> +++ b/drivers/media/media-device.c
>> @@ -384,22 +384,36 @@ static long copy_arg_to_user(void __user *uarg, void
>> *karg, unsigned int cmd)
>> /* Do acquire the graph mutex */
>> #define MEDIA_IOC_FL_GRAPH_MUTEXBIT(0)
>>
>> -#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user) \
>> +#define MEDIA_IOC_SZ_ARG(__cmd, func, fl, alt_sz, from_user, to_user)
>> \
>> [_IOC_NR(MEDIA_IOC_##__cmd)] = {\
>> .cmd = MEDIA_IOC_##__cmd, \
>> .fn = (long (*)(struct media_device *, void *))func,\
>> .flags = fl,\
>> +.alt_arg_sizes = alt_sz,\
>> .arg_from_user = from_user, \
>> .arg_to_user = to_user, \
>> }
>>
>> -#define MEDIA_IOC(__cmd, func, fl) \
>> -MEDIA_IOC_ARG(__cmd, func, fl, copy_arg_from_user, copy_arg_to_user)
>> +#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user) \
>> +MEDIA_IOC_SZ_ARG(__cmd, func, fl, NULL, from_user, to_user)
>> +
>> +#define MEDIA_IOC_SZ(__cmd, func, fl, alt_sz) \
>> +MEDIA_IOC_SZ_ARG(__cmd, func, fl, alt_sz, \
>> + copy_arg_from_user, copy_arg_to_user)
>> +
>> +#define MEDIA_IOC(__cmd, func, fl) \
>> +MEDIA_IOC_ARG(__cmd, func, fl, \
>> + copy_arg_from_user, copy_arg_to_user)
>>
>> /* the table is indexed by _IOC_NR(cmd) */
>> struct media_ioctl_info {
>> unsigned int cmd;
>> unsigned short flags;
>> +/*
>> + * Sizes of the alternative arguments. If there are none, this
>> + * pointer is NULL.
>> + */
>> +const unsigned short *alt_arg_sizes;
>> long (*fn)(struct media_device *dev, void *arg);
>> long (*arg_from_user)(void *karg, void __user *uarg, unsigned int cmd);
>> long (*arg_to_user)(void __user *uarg, void *karg, unsigned int cmd);
>> @@ -413,11 +427,40 @@ static const struct media_ioctl_info ioctl_info[] = {
>> MEDIA_IOC(G_TOPOLOGY, media_device_get_topology,
>> MEDIA_IOC_FL_GRAPH_MUTEX),
>> };
>>
>> +#define MASK_IOC_SIZE(cmd) \
>> +((cmd) & ~(_IOC_SIZEMASK << _IOC_SIZESHIFT))
>> +
>> static inline long is_valid_ioctl(const struct media_ioctl_info *info,
>>unsigned int cmd)
>> {
>> -return (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info)
>> -|| info[_IOC_NR(cmd)].cmd != cmd) ? -ENOIOCTLCMD : 0;
>> +const unsigned short *alt_arg_sizes;
>> +
>> +if (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info))
>> +return -ENOIOCTLCMD;
>> +
>> +info += _IOC_NR(cmd);
>> +
>> +if (info->cmd == cmd)
>> +return 0;
>> +
>> +/*
>> + * Verify that the size-dependent patch of the IOCTL command
>> + * matches and that the size does not exceed the principal
>> + * argument size.
>> + */
>> +if (MASK_IOC_SIZE(info->cmd) != MASK_IOC_SIZE(cmd)
>> +|| _IOC_SIZE(info->cmd) < _IOC_SIZE(cmd))
>> +return -ENOIOCTLCMD;
>> +
>> +alt_arg_sizes = info->alt_arg_sizes;
>> +if (!alt_arg_sizes)
>> +return -ENOIOCTLCMD;
>> +
>> +for (; *alt_arg_sizes; alt_arg_sizes++)
>> +if (_IOC_SIZE(cmd) == *alt_arg_sizes)
>> +return 0;
>> +
>> +return -ENOIOCTLCMD;
>> }
>>
>> static long __media_device_ioctl(
>> @@ -448,6 +491,9 @@ static long __media_device_ioctl(
>> goto out_free;
>> }
>>
>> +/* Set the rest of the argument struct to zero */
>> +memset(karg + _IOC_SIZE(cmd), 0, _IOC_SIZE(info->cmd) - _IOC_SIZE(cmd));
>> +
>> if (info->flags & MEDIA_IOC_FL_GRAPH_MUTEX)
>> mutex_lock(&dev->graph_mutex);
>>
>>
Re: [PATCH v4 5/5] media: Support variable size IOCTL arguments
On 08/11/2016 10:29 PM, Sakari Ailus wrote:
> Maintain a list of supported IOCTL argument sizes and allow only those in
> the list.
>
> As an additional bonus, IOCTL handlers will be able to check whether the
> caller actually set (using the argument size) the field vs. assigning it
> to zero. Separate macro can be provided for that.
>
> This will be easier for applications as well since there is no longer the
> problem of setting the reserved fields zero, or at least it is a lesser
> problem.
>
> Signed-off-by: Sakari Ailus
Acked-by: Hans Verkuil
> ---
> drivers/media/media-device.c | 56
>
> 1 file changed, 51 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 6f565a2..aa37520 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -384,22 +384,36 @@ static long copy_arg_to_user(void __user *uarg, void
> *karg, unsigned int cmd)
> /* Do acquire the graph mutex */
> #define MEDIA_IOC_FL_GRAPH_MUTEX BIT(0)
>
> -#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user) \
> +#define MEDIA_IOC_SZ_ARG(__cmd, func, fl, alt_sz, from_user, to_user)
> \
> [_IOC_NR(MEDIA_IOC_##__cmd)] = {\
> .cmd = MEDIA_IOC_##__cmd, \
> .fn = (long (*)(struct media_device *, void *))func,\
> .flags = fl,\
> + .alt_arg_sizes = alt_sz,\
> .arg_from_user = from_user, \
> .arg_to_user = to_user, \
> }
>
> -#define MEDIA_IOC(__cmd, func, fl) \
> - MEDIA_IOC_ARG(__cmd, func, fl, copy_arg_from_user, copy_arg_to_user)
> +#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user) \
> + MEDIA_IOC_SZ_ARG(__cmd, func, fl, NULL, from_user, to_user)
> +
> +#define MEDIA_IOC_SZ(__cmd, func, fl, alt_sz)\
> + MEDIA_IOC_SZ_ARG(__cmd, func, fl, alt_sz, \
> + copy_arg_from_user, copy_arg_to_user)
> +
> +#define MEDIA_IOC(__cmd, func, fl) \
> + MEDIA_IOC_ARG(__cmd, func, fl, \
> + copy_arg_from_user, copy_arg_to_user)
>
> /* the table is indexed by _IOC_NR(cmd) */
> struct media_ioctl_info {
> unsigned int cmd;
> unsigned short flags;
> + /*
> + * Sizes of the alternative arguments. If there are none, this
> + * pointer is NULL.
> + */
> + const unsigned short *alt_arg_sizes;
> long (*fn)(struct media_device *dev, void *arg);
> long (*arg_from_user)(void *karg, void __user *uarg, unsigned int cmd);
> long (*arg_to_user)(void __user *uarg, void *karg, unsigned int cmd);
> @@ -413,11 +427,40 @@ static const struct media_ioctl_info ioctl_info[] = {
> MEDIA_IOC(G_TOPOLOGY, media_device_get_topology,
> MEDIA_IOC_FL_GRAPH_MUTEX),
> };
>
> +#define MASK_IOC_SIZE(cmd) \
> + ((cmd) & ~(_IOC_SIZEMASK << _IOC_SIZESHIFT))
> +
> static inline long is_valid_ioctl(const struct media_ioctl_info *info,
> unsigned int cmd)
> {
> - return (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info)
> - || info[_IOC_NR(cmd)].cmd != cmd) ? -ENOIOCTLCMD : 0;
> + const unsigned short *alt_arg_sizes;
> +
> + if (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info))
> + return -ENOIOCTLCMD;
> +
> + info += _IOC_NR(cmd);
> +
> + if (info->cmd == cmd)
> + return 0;
> +
> + /*
> + * Verify that the size-dependent patch of the IOCTL command
> + * matches and that the size does not exceed the principal
> + * argument size.
> + */
> + if (MASK_IOC_SIZE(info->cmd) != MASK_IOC_SIZE(cmd)
> + || _IOC_SIZE(info->cmd) < _IOC_SIZE(cmd))
> + return -ENOIOCTLCMD;
> +
> + alt_arg_sizes = info->alt_arg_sizes;
> + if (!alt_arg_sizes)
> + return -ENOIOCTLCMD;
> +
> + for (; *alt_arg_sizes; alt_arg_sizes++)
> + if (_IOC_SIZE(cmd) == *alt_arg_sizes)
> + return 0;
> +
> + return -ENOIOCTLCMD;
> }
>
> static long __media_device_ioctl(
> @@ -448,6 +491,9 @@ static long __media_device_ioctl(
> goto out_free;
> }
>
> + /* Set the rest of the argument struct to zero */
> + memset(karg + _IOC_SIZE(cmd), 0, _IOC_SIZE(info->cmd) - _IOC_SIZE(cmd));
> +
> if (info->flags & MEDIA_IOC_FL_GRAPH_MUTEX)
> mutex_lock(&dev->graph_mutex);
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 17/17] omap3isp: Don't rely on devm for memory resource management
On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> devm functions are fine for managing resources that are directly related
> to the device at hand and that have no other dependencies. However, a
> process holding a file handle to a device created by a driver for a device
> may result in the file handle left behind after the device is long gone.
> This will result in accessing released (and potentially reallocated)
> memory.
>
> Instead, rely on the media device which will stick around until all users
> are gone.
>
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/platform/omap3isp/isp.c | 38
> ---
> drivers/media/platform/omap3isp/ispccp2.c | 3 ++-
> drivers/media/platform/omap3isp/isph3a_aewb.c | 20 +-
> drivers/media/platform/omap3isp/isph3a_af.c | 20 +-
> drivers/media/platform/omap3isp/isphist.c | 5 ++--
> drivers/media/platform/omap3isp/ispstat.c | 2 ++
> 6 files changed, 63 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c
> index 217d4da..3488ed3 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -1370,7 +1370,7 @@ static int isp_get_clocks(struct isp_device *isp)
> unsigned int i;
>
> for (i = 0; i < ARRAY_SIZE(isp_clocks); ++i) {
> - clk = devm_clk_get(isp->dev, isp_clocks[i]);
I wonder, would it be possible to use the media device itself for these devm_
functions? Since the media device is the last one to be released...
Regards,
Hans
> + clk = clk_get(isp->dev, isp_clocks[i]);
> if (IS_ERR(clk)) {
> dev_err(isp->dev, "clk_get %s failed\n", isp_clocks[i]);
> return PTR_ERR(clk);
> @@ -1382,6 +1382,14 @@ static int isp_get_clocks(struct isp_device *isp)
> return 0;
> }
>
> +static void isp_put_clocks(struct isp_device *isp)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(isp_clocks); ++i)
> + clk_put(isp->clock[i]);
> +}
> +
> /*
> * omap3isp_get - Acquire the ISP resource.
> *
> @@ -1596,7 +1604,6 @@ static void isp_unregister_entities(struct isp_device
> *isp)
> omap3isp_stat_unregister_entities(&isp->isp_af);
> omap3isp_stat_unregister_entities(&isp->isp_hist);
>
> - v4l2_device_unregister(&isp->v4l2_dev);
> media_device_unregister(isp->media_dev);
> }
>
> @@ -1952,6 +1959,8 @@ static void isp_release(struct media_device *mdev)
> {
> struct isp_device *isp = media_device_priv(mdev);
>
> + v4l2_device_unregister(&isp->v4l2_dev);
> +
> isp_cleanup_modules(isp);
> isp_xclk_cleanup(isp);
>
> @@ -1960,6 +1969,10 @@ static void isp_release(struct media_device *mdev)
> __omap3isp_put(isp, false);
>
> media_entity_enum_cleanup(&isp->crashed);
> +
> + isp_put_clocks(isp);
> +
> + kfree(isp);
> }
>
> static int isp_attach_iommu(struct isp_device *isp)
> @@ -2212,7 +2225,7 @@ static int isp_probe(struct platform_device *pdev)
> int ret;
> int i, m;
>
> - isp = devm_kzalloc(&pdev->dev, sizeof(*isp), GFP_KERNEL);
> + isp = kzalloc(sizeof(*isp), GFP_KERNEL);
> if (!isp) {
> dev_err(&pdev->dev, "could not allocate memory\n");
> return -ENOMEM;
> @@ -2221,21 +2234,23 @@ static int isp_probe(struct platform_device *pdev)
> ret = of_property_read_u32(pdev->dev.of_node, "ti,phy-type",
> &isp->phy_type);
> if (ret)
> - return ret;
> + goto error_release_isp;
>
> isp->syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> "syscon");
> - if (IS_ERR(isp->syscon))
> - return PTR_ERR(isp->syscon);
> + if (IS_ERR(isp->syscon)) {
> + ret = PTR_ERR(isp->syscon);
> + goto error_release_isp;
> + }
>
> ret = of_property_read_u32_index(pdev->dev.of_node, "syscon", 1,
>&isp->syscon_offset);
> if (ret)
> - return ret;
> + goto error_release_isp;
>
> ret = isp_of_parse_nodes(&pdev->dev, &isp->notifier);
> if (ret < 0)
> - return ret;
> + goto error_release_isp;
>
> isp->autoidle = autoidle;
>
> @@ -2252,8 +2267,8 @@ static int isp_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, isp);
>
> /* Regulators */
> - isp->isp_csiphy1.vdd = devm_regulator_get(&pdev->dev, "vdd-csiphy1");
> - isp->isp_csiphy2.vdd = devm_regulator_get(&pdev->dev, "vdd-csiphy2");
> + isp->isp_csiphy1.vdd = regulator_get(&pdev->dev, "vdd-csiphy1");
> + isp->isp_csiphy2.vdd = regulator_get(&pdev->dev, "vdd-csiphy2");
>
> /* Clocks
>*
> @@ -2385,6 +2400,9 @@ error_isp:
> __omap3isp_put(isp, fals
Re: [RFC v2 06/17] media: Dynamically allocate the media device
On Mon, Aug 22, 2016 at 03:38:20PM +0300, Sakari Ailus wrote:
> Hi Hans,
>
> On Mon, Aug 22, 2016 at 02:01:44PM +0200, Hans Verkuil wrote:
> > On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> > > From: Sakari Ailus
> > >
> > > Allow allocating the media device dynamically. As the struct media_device
> > > embeds struct media_devnode, the lifetime of that object is that same than
> > > that of the media_device.
> > >
> > > Signed-off-by: Sakari Ailus
> > > ---
> > > drivers/media/media-device.c | 22 ++
> > > include/media/media-device.h | 38 ++
> > > 2 files changed, 60 insertions(+)
> > >
> > > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> > > index a431775..5a86d36 100644
> > > --- a/drivers/media/media-device.c
> > > +++ b/drivers/media/media-device.c
> > > @@ -544,7 +544,15 @@ static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
> > >
> > > static void media_device_release(struct media_devnode *devnode)
> > > {
> > > + struct media_device *mdev = to_media_device(devnode);
> > > +
> > > + ida_destroy(&mdev->entity_internal_idx);
> > > + mdev->entity_internal_idx_max = 0;
> > > + media_entity_graph_walk_cleanup(&mdev->pm_count_walk);
> > > + mutex_destroy(&mdev->graph_mutex);
> > > dev_dbg(devnode->parent, "Media device released\n");
> > > +
> > > + kfree(mdev);
> >
> > Doesn't this break bisect? mdev is now freed, but media_device_alloc isn't
> > called yet.
> >
> > That doesn't seem right.
>
> You're right.
>
> media_device_release() actually may only be called for drivers that use
> media_device_init(). I'll fix that.
s/media_device_init/media_device_alloc/
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 06/17] media: Dynamically allocate the media device
Hi Hans,
On Mon, Aug 22, 2016 at 02:01:44PM +0200, Hans Verkuil wrote:
> On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> > From: Sakari Ailus
> >
> > Allow allocating the media device dynamically. As the struct media_device
> > embeds struct media_devnode, the lifetime of that object is that same than
> > that of the media_device.
> >
> > Signed-off-by: Sakari Ailus
> > ---
> > drivers/media/media-device.c | 22 ++
> > include/media/media-device.h | 38 ++
> > 2 files changed, 60 insertions(+)
> >
> > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> > index a431775..5a86d36 100644
> > --- a/drivers/media/media-device.c
> > +++ b/drivers/media/media-device.c
> > @@ -544,7 +544,15 @@ static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
> >
> > static void media_device_release(struct media_devnode *devnode)
> > {
> > + struct media_device *mdev = to_media_device(devnode);
> > +
> > + ida_destroy(&mdev->entity_internal_idx);
> > + mdev->entity_internal_idx_max = 0;
> > + media_entity_graph_walk_cleanup(&mdev->pm_count_walk);
> > + mutex_destroy(&mdev->graph_mutex);
> > dev_dbg(devnode->parent, "Media device released\n");
> > +
> > + kfree(mdev);
>
> Doesn't this break bisect? mdev is now freed, but media_device_alloc isn't
> called yet.
>
> That doesn't seem right.
You're right.
media_device_release() actually may only be called for drivers that use
media_device_init(). I'll fix that.
>
> Regards,
>
> Hans
>
> > }
> >
> > /**
> > @@ -691,6 +699,20 @@ void media_device_init(struct media_device *mdev)
> > }
> > EXPORT_SYMBOL_GPL(media_device_init);
> >
> > +struct media_device *media_device_alloc(void)
> > +{
> > + struct media_device *mdev;
> > +
> > + mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> > + if (!mdev)
> > + return NULL;
> > +
> > + media_device_init(mdev);
> > +
> > + return mdev;
> > +}
> > +EXPORT_SYMBOL_GPL(media_device_alloc);
> > +
> > void media_device_cleanup(struct media_device *mdev)
> > {
> > ida_destroy(&mdev->entity_internal_idx);
> > diff --git a/include/media/media-device.h b/include/media/media-device.h
> > index 4eee613..d1d45ab 100644
> > --- a/include/media/media-device.h
> > +++ b/include/media/media-device.h
> > @@ -197,6 +197,42 @@ static inline __must_check int media_entity_enum_init(
> > void media_device_init(struct media_device *mdev);
> >
> > /**
> > + * media_device_alloc() - Allocate and initialise a media device
> > + *
> > + * Allocate and initialise a media device. Returns a media device.
> > + * The media device is refcounted, and this function returns a media
> > + * device the refcount of which is one (1).
> > + *
> > + * References are taken and given using media_device_get() and
> > + * media_device_put().
> > + */
> > +struct media_device *media_device_alloc(void);
> > +
> > +/**
> > + * media_device_get() - Get a reference to a media device
> > + *
> > + * mdev: media device
> > + */
> > +#define media_device_get(mdev)
> > \
> > + do {\
> > + dev_dbg((mdev)->dev, "%s: get media device %s\n", \
> > + __func__, (mdev)->bus_info);\
> > + get_device(&(mdev)->devnode.dev); \
> > + } while (0)
> > +
> > +/**
> > + * media_device_put() - Put a reference to a media device
> > + *
> > + * mdev: media device
> > + */
> > +#define media_device_put(mdev)
> > \
> > + do {\
> > + dev_dbg((mdev)->dev, "%s: put media device %s\n", \
> > + __func__, (mdev)->bus_info);\
> > + put_device(&(mdev)->devnode.dev); \
> > + } while (0)
> > +
> > +/**
> > * media_device_cleanup() - Cleanups a media device element
> > *
> > * @mdev: pointer to struct &media_device
> > @@ -425,6 +461,8 @@ void __media_device_usb_init(struct media_device *mdev,
> > const char *driver_name);
> >
> > #else
> > +#define media_device_get(mdev) do { } while (0)
> > +#define media_device_put(mdev) do { } while (0)
> > static inline int media_device_register(struct media_device *mdev)
> > {
> > return 0;
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 15/17] omap3isp: Allocate the media device dynamically
No commit message?
On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/platform/omap3isp/isp.c | 25 +
> drivers/media/platform/omap3isp/isp.h | 2 +-
> drivers/media/platform/omap3isp/ispvideo.c | 2 +-
> 3 files changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c
> index aa32537..1e42d37 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -1597,8 +1597,7 @@ static void isp_unregister_entities(struct isp_device
> *isp)
> omap3isp_stat_unregister_entities(&isp->isp_hist);
>
> v4l2_device_unregister(&isp->v4l2_dev);
> - media_device_unregister(&isp->media_dev);
> - media_device_cleanup(&isp->media_dev);
> + media_device_unregister(isp->media_dev);
> }
>
> static int isp_link_entity(
> @@ -1676,14 +1675,16 @@ static int isp_register_entities(struct isp_device
> *isp)
> {
> int ret;
>
> - isp->media_dev.dev = isp->dev;
> - strlcpy(isp->media_dev.model, "TI OMAP3 ISP",
> - sizeof(isp->media_dev.model));
> - isp->media_dev.hw_revision = isp->revision;
> - isp->media_dev.link_notify = v4l2_pipeline_link_notify;
> - media_device_init(&isp->media_dev);
> + isp->media_dev = media_device_alloc(isp->dev, isp, 0);
> + if (!isp->media_dev)
> + return -ENOMEM;
> +
> + strlcpy(isp->media_dev->model, "TI OMAP3 ISP",
> + sizeof(isp->media_dev->model));
> + isp->media_dev->hw_revision = isp->revision;
> + isp->media_dev->link_notify = v4l2_pipeline_link_notify;
>
> - isp->v4l2_dev.mdev = &isp->media_dev;
> + isp->v4l2_dev.mdev = isp->media_dev;
> ret = v4l2_device_register(isp->dev, &isp->v4l2_dev);
> if (ret < 0) {
> dev_err(isp->dev, "%s: V4L2 device registration failed (%d)\n",
> @@ -1728,7 +1729,7 @@ static int isp_register_entities(struct isp_device *isp)
> done:
> if (ret < 0) {
> isp_unregister_entities(isp);
> - media_device_put(&isp->media_dev);
> + media_device_put(isp->media_dev);
I am not sure if calling media_device_put is correct here, This only works if
the release callback is set for the media devnode, but that's done when the
media device is registered, which happens later.
I could be mistaken, though. It's hard to tell.
Regards,
Hans
> }
>
> return ret;
> @@ -2163,7 +2164,7 @@ static int isp_subdev_notifier_complete(struct
> v4l2_async_notifier *async)
> struct isp_bus_cfg *bus;
> int ret;
>
> - ret = media_entity_enum_init(&isp->crashed, &isp->media_dev);
> + ret = media_entity_enum_init(&isp->crashed, isp->media_dev);
> if (ret)
> return ret;
>
> @@ -2181,7 +2182,7 @@ static int isp_subdev_notifier_complete(struct
> v4l2_async_notifier *async)
> if (ret < 0)
> return ret;
>
> - return media_device_register(&isp->media_dev);
> + return media_device_register(isp->media_dev);
> }
>
> /*
> diff --git a/drivers/media/platform/omap3isp/isp.h
> b/drivers/media/platform/omap3isp/isp.h
> index 7e6f663..7378279 100644
> --- a/drivers/media/platform/omap3isp/isp.h
> +++ b/drivers/media/platform/omap3isp/isp.h
> @@ -176,7 +176,7 @@ struct isp_xclk {
> struct isp_device {
> struct v4l2_device v4l2_dev;
> struct v4l2_async_notifier notifier;
> - struct media_device media_dev;
> + struct media_device *media_dev;
> struct device *dev;
> u32 revision;
>
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c
> index 7d9f359..45ef38c 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -1077,7 +1077,7 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
> pipe = video->video.entity.pipe
>? to_isp_pipeline(&video->video.entity) : &video->pipe;
>
> - ret = media_entity_enum_init(&pipe->ent_enum, &video->isp->media_dev);
> + ret = media_entity_enum_init(&pipe->ent_enum, video->isp->media_dev);
> if (ret)
> goto err_enum_init;
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 11/17] media: Add release callback for media device
Hi Hans, On Mon, Aug 22, 2016 at 02:24:59PM +0200, Hans Verkuil wrote: > On 08/19/2016 12:23 PM, Sakari Ailus wrote: > > The release callback may be used by the driver to signal the release of > > the media device. This makes it possible to embed a driver private struct > > to the same memory allocation. > > This is a bit weird: you either add support for private data with a priv > pointer as in the previous patch, or you allow for larger structs. > > Doing both doesn't seem right. In both cases you want the release callback, > so that's fine. But I think you should pick which method you want to keep > private data around. > > I have a slight preference for a priv pointer, but I'm OK with either > solution. I'll drop the size parameter in the next version. -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 12/17] v4l: Acquire a reference to the media device for every video device
On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> The video device depends on the existence of its media device --- if there
> is one. Acquire a reference to it.
>
> Signed-off-by: Sakari Ailus
Acked-by: Hans Verkuil
> ---
> drivers/media/v4l2-core/v4l2-dev.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c
> b/drivers/media/v4l2-core/v4l2-dev.c
> index e6da353..cda04ff 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -171,6 +171,9 @@ static void v4l2_device_release(struct device *cd)
> {
> struct video_device *vdev = to_video_device(cd);
> struct v4l2_device *v4l2_dev = vdev->v4l2_dev;
> +#ifdef CONFIG_MEDIA_CONTROLLER
> + struct media_device *mdev = v4l2_dev->mdev;
> +#endif
>
> mutex_lock(&videodev_lock);
> if (WARN_ON(video_device[vdev->minor] != vdev)) {
> @@ -193,8 +196,8 @@ static void v4l2_device_release(struct device *cd)
>
> mutex_unlock(&videodev_lock);
>
> -#if defined(CONFIG_MEDIA_CONTROLLER)
> - if (v4l2_dev->mdev) {
> +#ifdef CONFIG_MEDIA_CONTROLLER
> + if (mdev) {
> /* Remove interfaces and interface links */
> media_devnode_remove(vdev->intf_devnode);
> if (vdev->entity.function != MEDIA_ENT_F_UNKNOWN)
> @@ -220,6 +223,11 @@ static void v4l2_device_release(struct device *cd)
> /* Decrease v4l2_device refcount */
> if (v4l2_dev)
> v4l2_device_put(v4l2_dev);
> +
> +#ifdef CONFIG_MEDIA_CONTROLLER
> + if (mdev)
> + media_device_put(mdev);
> +#endif
> }
>
> static struct class video_class = {
> @@ -808,6 +816,7 @@ static int video_register_media_controller(struct
> video_device *vdev, int type)
>
> /* FIXME: how to create the other interface links? */
>
> + media_device_get(vdev->v4l2_dev->mdev);
> #endif
> return 0;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 14/17] media-device: Postpone graph object removal until free
On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> The media device itself will be unregistered based on it being unbound and
> driver's remove callback being called. The graph objects themselves may
> still be in use; rely on the media device release callback to release
> them.
>
> Signed-off-by: Sakari Ailus
Acked-by: Hans Verkuil
> ---
> drivers/media/media-device.c | 44
>
> 1 file changed, 20 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 0656daf..cbbc397 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -756,6 +756,26 @@ EXPORT_SYMBOL_GPL(media_device_cleanup);
> static void media_device_release(struct media_devnode *devnode)
> {
> struct media_device *mdev = to_media_device(devnode);
> + struct media_entity *entity;
> + struct media_entity *next;
> + struct media_interface *intf, *tmp_intf;
> + struct media_entity_notify *notify, *nextp;
> +
> + /* Remove all entities from the media device */
> + list_for_each_entry_safe(entity, next, &mdev->entities, graph_obj.list)
> + __media_device_unregister_entity(entity);
> +
> + /* Remove all entity_notify callbacks from the media device */
> + list_for_each_entry_safe(notify, nextp, &mdev->entity_notify, list)
> + __media_device_unregister_entity_notify(mdev, notify);
> +
> + /* Remove all interfaces from the media device */
> + list_for_each_entry_safe(intf, tmp_intf, &mdev->interfaces,
> + graph_obj.list) {
> + __media_remove_intf_links(intf);
> + media_gobj_destroy(&intf->graph_obj);
> + kfree(intf);
> + }
>
> ida_destroy(&mdev->entity_internal_idx);
> mdev->entity_internal_idx_max = 0;
> @@ -800,38 +820,14 @@ EXPORT_SYMBOL_GPL(__media_device_register);
>
> void media_device_unregister(struct media_device *mdev)
> {
> - struct media_entity *entity;
> - struct media_entity *next;
> - struct media_interface *intf, *tmp_intf;
> - struct media_entity_notify *notify, *nextp;
> -
> if (mdev == NULL)
> return;
>
> mutex_lock(&mdev->graph_mutex);
> -
> - /* Check if mdev was ever registered at all */
> if (!media_devnode_is_registered(&mdev->devnode)) {
> mutex_unlock(&mdev->graph_mutex);
> return;
> }
> -
> - /* Remove all entities from the media device */
> - list_for_each_entry_safe(entity, next, &mdev->entities, graph_obj.list)
> - __media_device_unregister_entity(entity);
> -
> - /* Remove all entity_notify callbacks from the media device */
> - list_for_each_entry_safe(notify, nextp, &mdev->entity_notify, list)
> - __media_device_unregister_entity_notify(mdev, notify);
> -
> - /* Remove all interfaces from the media device */
> - list_for_each_entry_safe(intf, tmp_intf, &mdev->interfaces,
> - graph_obj.list) {
> - __media_remove_intf_links(intf);
> - media_gobj_destroy(&intf->graph_obj);
> - kfree(intf);
> - }
> -
> mutex_unlock(&mdev->graph_mutex);
>
> device_remove_file(&mdev->devnode.dev, &dev_attr_model);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 13/17] media: Shuffle functions around
On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> As the call paths of the functions in question will change, move them
> around in anticipation of that. No other changes.
>
> Signed-off-by: Sakari Ailus
Acked-by: Hans Verkuil
> ---
> drivers/media/media-device.c | 88
> ++--
> 1 file changed, 44 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 7f90cb82..0656daf 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -542,22 +542,6 @@ static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
> * Registration/unregistration
> */
>
> -static void media_device_release(struct media_devnode *devnode)
> -{
> - struct media_device *mdev = to_media_device(devnode);
> -
> - ida_destroy(&mdev->entity_internal_idx);
> - mdev->entity_internal_idx_max = 0;
> - media_entity_graph_walk_cleanup(&mdev->pm_count_walk);
> - mutex_destroy(&mdev->graph_mutex);
> - dev_dbg(devnode->parent, "Media device released\n");
> -
> - if (mdev->release)
> - mdev->release(mdev);
> -
> - kfree(mdev);
> -}
> -
> /**
> * media_device_register_entity - Register an entity with a media device
> * @mdev:The media device
> @@ -678,6 +662,34 @@ void media_device_unregister_entity(struct media_entity
> *entity)
> }
> EXPORT_SYMBOL_GPL(media_device_unregister_entity);
>
> +int __must_check media_device_register_entity_notify(struct media_device
> *mdev,
> + struct media_entity_notify *nptr)
> +{
> + mutex_lock(&mdev->graph_mutex);
> + list_add_tail(&nptr->list, &mdev->entity_notify);
> + mutex_unlock(&mdev->graph_mutex);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(media_device_register_entity_notify);
> +
> +/*
> + * Note: Should be called with mdev->lock held.
> + */
> +static void __media_device_unregister_entity_notify(struct media_device
> *mdev,
> + struct media_entity_notify *nptr)
> +{
> + list_del(&nptr->list);
> +}
> +
> +void media_device_unregister_entity_notify(struct media_device *mdev,
> + struct media_entity_notify *nptr)
> +{
> + mutex_lock(&mdev->graph_mutex);
> + __media_device_unregister_entity_notify(mdev, nptr);
> + mutex_unlock(&mdev->graph_mutex);
> +}
> +EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);
> +
> /**
> * media_device_init() - initialize a media device
> * @mdev:The media device
> @@ -741,6 +753,22 @@ void media_device_cleanup(struct media_device *mdev)
> }
> EXPORT_SYMBOL_GPL(media_device_cleanup);
>
> +static void media_device_release(struct media_devnode *devnode)
> +{
> + struct media_device *mdev = to_media_device(devnode);
> +
> + ida_destroy(&mdev->entity_internal_idx);
> + mdev->entity_internal_idx_max = 0;
> + media_entity_graph_walk_cleanup(&mdev->pm_count_walk);
> + mutex_destroy(&mdev->graph_mutex);
> + dev_dbg(devnode->parent, "Media device released\n");
> +
> + if (mdev->release)
> + mdev->release(mdev);
> +
> + kfree(mdev);
> +}
> +
> int __must_check __media_device_register(struct media_device *mdev,
>struct module *owner)
> {
> @@ -770,34 +798,6 @@ int __must_check __media_device_register(struct
> media_device *mdev,
> }
> EXPORT_SYMBOL_GPL(__media_device_register);
>
> -int __must_check media_device_register_entity_notify(struct media_device
> *mdev,
> - struct media_entity_notify *nptr)
> -{
> - mutex_lock(&mdev->graph_mutex);
> - list_add_tail(&nptr->list, &mdev->entity_notify);
> - mutex_unlock(&mdev->graph_mutex);
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL(media_device_register_entity_notify);
> -
> -/*
> - * Note: Should be called with mdev->lock held.
> - */
> -static void __media_device_unregister_entity_notify(struct media_device
> *mdev,
> - struct media_entity_notify *nptr)
> -{
> - list_del(&nptr->list);
> -}
> -
> -void media_device_unregister_entity_notify(struct media_device *mdev,
> - struct media_entity_notify *nptr)
> -{
> - mutex_lock(&mdev->graph_mutex);
> - __media_device_unregister_entity_notify(mdev, nptr);
> - mutex_unlock(&mdev->graph_mutex);
> -}
> -EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);
> -
> void media_device_unregister(struct media_device *mdev)
> {
> struct media_entity *entity;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 11/17] media: Add release callback for media device
On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> The release callback may be used by the driver to signal the release of
> the media device. This makes it possible to embed a driver private struct
> to the same memory allocation.
This is a bit weird: you either add support for private data with a priv
pointer as in the previous patch, or you allow for larger structs.
Doing both doesn't seem right. In both cases you want the release callback,
so that's fine. But I think you should pick which method you want to keep
private data around.
I have a slight preference for a priv pointer, but I'm OK with either solution.
Regards,
Hans
>
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/media-device.c | 11 ++-
> include/media/media-device.h | 8 +++-
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 27d5214..7f90cb82 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -552,6 +552,9 @@ static void media_device_release(struct media_devnode
> *devnode)
> mutex_destroy(&mdev->graph_mutex);
> dev_dbg(devnode->parent, "Media device released\n");
>
> + if (mdev->release)
> + mdev->release(mdev);
> +
> kfree(mdev);
> }
>
> @@ -699,10 +702,16 @@ void media_device_init(struct media_device *mdev)
> }
> EXPORT_SYMBOL_GPL(media_device_init);
>
> -struct media_device *media_device_alloc(struct device *dev, void *priv)
> +struct media_device *media_device_alloc(struct device *dev, void *priv,
> + size_t size)
> {
> struct media_device *mdev;
>
> + if (!size)
> + size = sizeof(*mdev);
> + else if (WARN_ON(size < sizeof(*mdev)))
> + return NULL;
> +
> dev = get_device(dev);
> if (!dev)
> return NULL;
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index cfcec1b..3b66232 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -152,6 +152,7 @@ struct media_device {
>
> int (*link_notify)(struct media_link *link, u32 flags,
> unsigned int notification);
> + void (*release)(struct media_device *mdev);
> };
>
> /* We don't need to include pci.h or usb.h here */
> @@ -203,15 +204,20 @@ void media_device_init(struct media_device *mdev);
> *
> * @dev: The associated struct device pointer
> * @priv:pointer to a driver private data structure
> + * @size:size of a driver structure containing the media device
> *
> * Allocate and initialise a media device. Returns a media device.
> * The media device is refcounted, and this function returns a media
> * device the refcount of which is one (1).
> *
> + * The size parameter can be zero if the media_device is not embedded
> + * in another struct.
> + *
> * References are taken and given using media_device_get() and
> * media_device_put().
> */
> -struct media_device *media_device_alloc(struct device *dev, void *priv);
> +struct media_device *media_device_alloc(struct device *dev, void *priv,
> + size_t size);
>
> /**
> * media_device_get() - Get a reference to a media device
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC? [PATCH] docs-rst: kernel-doc: better output struct members
Am 22.08.2016 um 14:15 schrieb Mauro Carvalho Chehab :
> Em Mon, 22 Aug 2016 14:52:42 +0300
> Jani Nikula escreveu:
>
>> On Mon, 22 Aug 2016, Markus Heiser wrote:
>>> Am 22.08.2016 um 13:16 schrieb Jani Nikula :
>>>
On Mon, 22 Aug 2016, Mauro Carvalho Chehab
wrote:
> Markus,
>
> Em Mon, 22 Aug 2016 10:56:01 +0200
> Markus Heiser escreveu:
>
>> Am 21.08.2016 um 14:11 schrieb Mauro Carvalho Chehab
>> :
>>
>>> Right now, for a struct, kernel-doc produces the following output:
>>>
>>> .. c:type:: struct v4l2_prio_state
>>>
>>>stores the priority states
>>>
>>> **Definition**
>>>
>>> ::
>>>
>>> struct v4l2_prio_state {
>>> atomic_t prios[4];
>>> };
>>>
>>> **Members**
>>>
>>> ``atomic_t prios[4]``
>>> array with elements to store the array priorities
>>>
>>> Putting a member name in verbatim and adding a continuation line
>>> causes the LaTeX output to generate something like:
>>> item[atomic_t prios\[4\]] array with elements to store the
>>> array priorities
>>
>>
>> Right now, the description of C-struct members is a simple
>> rest-definition-list
>> (not in the c-domain). It might be better to use the c-domain for
>> members:
>>
>> http://www.sphinx-doc.org/en/stable/domains.html#directive-c:member
>>
>> But this is not the only thing we have to consider. To make a valid
>> C-struct
>> description (with targets/references in the c-domain) we need a more
>> *structured* reST markup where the members are described in the
>> block-content
>> of the struct directive. E.g:
>>
>> ---
>> |.. c:type:: struct v4l2_subdev_ir_ops
>> |
>> | operations for IR subdevices
>> |
>> | .. c:member:: int (* rx_read) (struct v4l2_subdev *sd, u8 *buf,
>> size_t count,ssize_t *num)
>> |
>> ---
>>
>> By this small example, you see, that we have to discuss the whole markup
>> produced by the kernel-doc script (function arguments, unions etc.).
>> IMHO, since kernel-doc is widely used, this should be a RFC.
>
> I tried using c:member. It won't work on LaTeX output, as it will
> still put everything into a LaTeX item, with doesn't do line breaks.
I've tried c:member before, and I'm not convinced it buys us anything
useful. I'm also not convinced we'd need more structured rst markup
within struct or function descriptions in addition to what we currently
have. Keep it simple.
BR,
Jani.
>>>
>>> It buys, that we stay in the c-domain and we can refer to the members
>>> with the :c:member role. E.g :c:member:`v4l2_subdev_ir_ops.rx_read`.
>>
>> Yes, it allows anchors to members, while detaching the member
>> descriptions from the struct descriptions. In the output, there is no
>> perceivable parent-child relationship between the struct and its
>> members. Arguably the resulting documentation is harder to follow with
>> c:member than without. I think it's sufficient to link to the struct
>> descriptions. It's not enough to say that theoretically using c:member
>> is the right thing; it needs to be better in practice too.
>
> Linking to the member is interesting, specially when describing too
> big structures, like the callback ones, specially when we have the
> descriptions for such things at the rst file (like we have on media kAPI
> book). However, the way Sphinx outputs a :c:member: is not nice, IMHO.
>
> I'd say that we should only use .. c:member:: if we can override its behavior
> via the C domain extension in a way that it will create a C-domain
> reference, but keep producing an output like:
>
> *member_name*
> type
> some description
The output is (nearly) independent from the way the reST is parsed.
Normally this should be realized in the builder. But we wan't
touch the latex builder and as I said, the name/type detection
of the c-parser is broken and I don't want to implement a third
c-parser in our toolchain.
May it is more simple to refer only the structure, even if
the struct is big.
-- Markus --
> on all output formats without any fancy colored struct-like style.
>
> Also, IMHO, it should not add any entry to the genindex, to avoid
> polluting it with thousands entries.
>
> If such thing can't be done, let's just stick to what we have right now,
> after this patch.
>
>
> Thanks,
> Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 10/17] media: Provide a way to the driver to set a private pointer
On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> Now that the media device can be allocated dynamically, drivers have no
> longer a way to conveniently obtain the driver private data structure.
> Provide one again in the form of a private pointer passed to the
> media_device_alloc() function.
>
> Signed-off-by: Sakari Ailus
Acked-by: Hans Verkuil
> ---
> drivers/media/media-device.c | 3 ++-
> include/media/media-device.h | 15 ++-
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 6c8b689..27d5214 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -699,7 +699,7 @@ void media_device_init(struct media_device *mdev)
> }
> EXPORT_SYMBOL_GPL(media_device_init);
>
> -struct media_device *media_device_alloc(struct device *dev)
> +struct media_device *media_device_alloc(struct device *dev, void *priv)
> {
> struct media_device *mdev;
>
> @@ -716,6 +716,7 @@ struct media_device *media_device_alloc(struct device
> *dev)
> media_devnode_init(&mdev->devnode);
> mdev->dev = dev;
> media_device_init(mdev);
> + mdev->priv = priv;
>
> return mdev;
> }
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index 8ccc8e8..cfcec1b 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -52,6 +52,7 @@ struct media_entity_notify {
> * struct media_device - Media device
> * @dev: Parent device
> * @devnode: Media device node
> + * @priv:A pointer to driver private data
> * @driver_name: Optional device driver name. If not set, calls to
> * %MEDIA_IOC_DEVICE_INFO will return dev->driver->name.
> * This is needed for USB drivers for example, as otherwise
> @@ -117,6 +118,7 @@ struct media_device {
> /* dev->driver_data points to this struct. */
> struct device *dev;
> struct media_devnode devnode;
> + void *priv;
>
> char model[32];
> char driver_name[32];
> @@ -200,6 +202,7 @@ void media_device_init(struct media_device *mdev);
> * media_device_alloc() - Allocate and initialise a media device
> *
> * @dev: The associated struct device pointer
> + * @priv:pointer to a driver private data structure
> *
> * Allocate and initialise a media device. Returns a media device.
> * The media device is refcounted, and this function returns a media
> @@ -208,7 +211,7 @@ void media_device_init(struct media_device *mdev);
> * References are taken and given using media_device_get() and
> * media_device_put().
> */
> -struct media_device *media_device_alloc(struct device *dev);
> +struct media_device *media_device_alloc(struct device *dev, void *priv);
>
> /**
> * media_device_get() - Get a reference to a media device
> @@ -235,6 +238,16 @@ struct media_device *media_device_alloc(struct device
> *dev);
> } while (0)
>
> /**
> + * media_device_priv() - Obtain the driver private pointer
> + *
> + * Returns a pointer passed to the media_device_alloc() function.
> + */
> +static inline void *media_device_priv(struct media_device *mdev)
> +{
> + return mdev->priv;
> +}
> +
> +/**
> * media_device_cleanup() - Cleanups a media device element
> *
> * @mdev:pointer to struct &media_device
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 09/17] media-device: struct media_device requires struct device
On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> The media device always has a device around. Require one as an argument
> for media_device_alloc().
>
> Signed-off-by: Sakari Ailus
Acked-by: Hans Verkuil
> ---
> drivers/media/media-device.c | 12 ++--
> include/media/media-device.h | 4 +++-
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index d527491..6c8b689 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -699,15 +699,22 @@ void media_device_init(struct media_device *mdev)
> }
> EXPORT_SYMBOL_GPL(media_device_init);
>
> -struct media_device *media_device_alloc(void)
> +struct media_device *media_device_alloc(struct device *dev)
> {
> struct media_device *mdev;
>
> + dev = get_device(dev);
> + if (!dev)
> + return NULL;
> +
> mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> - if (!mdev)
> + if (!mdev) {
> + put_device(dev);
> return NULL;
> + }
>
> media_devnode_init(&mdev->devnode);
> + mdev->dev = dev;
> media_device_init(mdev);
>
> return mdev;
> @@ -720,6 +727,7 @@ void media_device_cleanup(struct media_device *mdev)
> mdev->entity_internal_idx_max = 0;
> media_entity_graph_walk_cleanup(&mdev->pm_count_walk);
> mutex_destroy(&mdev->graph_mutex);
> + put_device(mdev->dev);
> }
> EXPORT_SYMBOL_GPL(media_device_cleanup);
>
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index d1d45ab..8ccc8e8 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -199,6 +199,8 @@ void media_device_init(struct media_device *mdev);
> /**
> * media_device_alloc() - Allocate and initialise a media device
> *
> + * @dev: The associated struct device pointer
> + *
> * Allocate and initialise a media device. Returns a media device.
> * The media device is refcounted, and this function returns a media
> * device the refcount of which is one (1).
> @@ -206,7 +208,7 @@ void media_device_init(struct media_device *mdev);
> * References are taken and given using media_device_get() and
> * media_device_put().
> */
> -struct media_device *media_device_alloc(void);
> +struct media_device *media_device_alloc(struct device *dev);
>
> /**
> * media_device_get() - Get a reference to a media device
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC? [PATCH] docs-rst: kernel-doc: better output struct members
Am 22.08.2016 um 13:52 schrieb Jani Nikula :
> On Mon, 22 Aug 2016, Markus Heiser wrote:
>> Am 22.08.2016 um 13:16 schrieb Jani Nikula :
>>
>>> On Mon, 22 Aug 2016, Mauro Carvalho Chehab wrote:
Markus,
Em Mon, 22 Aug 2016 10:56:01 +0200
Markus Heiser escreveu:
> Am 21.08.2016 um 14:11 schrieb Mauro Carvalho Chehab
> :
>
>> Right now, for a struct, kernel-doc produces the following output:
>>
>> .. c:type:: struct v4l2_prio_state
>>
>> stores the priority states
>>
>> **Definition**
>>
>> ::
>>
>>struct v4l2_prio_state {
>> atomic_t prios[4];
>>};
>>
>> **Members**
>>
>> ``atomic_t prios[4]``
>>array with elements to store the array priorities
>>
>> Putting a member name in verbatim and adding a continuation line
>> causes the LaTeX output to generate something like:
>> item[atomic_t prios\[4\]] array with elements to store the array
>> priorities
>
>
> Right now, the description of C-struct members is a simple
> rest-definition-list
> (not in the c-domain). It might be better to use the c-domain for members:
>
> http://www.sphinx-doc.org/en/stable/domains.html#directive-c:member
>
> But this is not the only thing we have to consider. To make a valid
> C-struct
> description (with targets/references in the c-domain) we need a more
> *structured* reST markup where the members are described in the
> block-content
> of the struct directive. E.g:
>
> ---
> |.. c:type:: struct v4l2_subdev_ir_ops
> |
> | operations for IR subdevices
> |
> | .. c:member:: int (* rx_read) (struct v4l2_subdev *sd, u8 *buf,
> size_t count,ssize_t *num)
> |
> ---
>
> By this small example, you see, that we have to discuss the whole markup
> produced by the kernel-doc script (function arguments, unions etc.).
> IMHO, since kernel-doc is widely used, this should be a RFC.
I tried using c:member. It won't work on LaTeX output, as it will
still put everything into a LaTeX item, with doesn't do line breaks.
>>>
>>> I've tried c:member before, and I'm not convinced it buys us anything
>>> useful. I'm also not convinced we'd need more structured rst markup
>>> within struct or function descriptions in addition to what we currently
>>> have. Keep it simple.
>>>
>>> BR,
>>> Jani.
>>
>> It buys, that we stay in the c-domain and we can refer to the members
>> with the :c:member role. E.g :c:member:`v4l2_subdev_ir_ops.rx_read`.
>
> Yes, it allows anchors to members, while detaching the member
> descriptions from the struct descriptions.
May I misunderstood you "detaching"? .. As far as I know, the members
has to be in the (indented) block of struct description (like above).
Anyway, I realized (last mail), that the c-parser of Sphinx is totally
broken in type / name detecting from signatures, with we have no reliable
anchors and it makes no more sense to follow my first intention.
To summarize: after a few thoughts ... I agree with your KIS strategy ;-)
-- Markus --
> In the output, there is no
> perceivable parent-child relationship between the struct and its
> members. Arguably the resulting documentation is harder to follow with
> c:member than without. I think it's sufficient to link to the struct
> descriptions. It's not enough to say that theoretically using c:member
> is the right thing; it needs to be better in practice too.
>
> BR,
> Jani.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 08/17] media-device: Make devnode.dev->kobj parent of devnode.cdev
On 08/19/2016 12:23 PM, Sakari Ailus wrote: > The struct cdev embedded in struct media_devnode contains its own kobj. > Instead of trying to manage its lifetime separately from struct > media_devnode, make the cdev kobj a parent of the struct media_device.dev > kobj. > > The cdev will thus be released during unregistering the media_devnode, not > in media_devnode.dev kobj's release callback. > > Signed-off-by: Sakari Ailus > --- > drivers/media/media-devnode.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c > index aa8030b..69f84a7 100644 > --- a/drivers/media/media-devnode.c > +++ b/drivers/media/media-devnode.c > @@ -63,9 +63,6 @@ static void media_devnode_release(struct device *cd) > > mutex_lock(&media_devnode_lock); > > - /* Delete the cdev on this minor as well */ > - cdev_del(&devnode->cdev); > - > /* Mark device node number as free */ > clear_bit(devnode->minor, media_devnode_nums); > > @@ -246,6 +243,7 @@ int __must_check media_devnode_register(struct > media_devnode *devnode, > > /* Part 2: Initialize and register the character device */ > cdev_init(&devnode->cdev, &media_devnode_fops); > + devnode->cdev.kobj.parent = &devnode->dev.kobj; > devnode->cdev.owner = owner; > > ret = cdev_add(&devnode->cdev, MKDEV(MAJOR(media_dev_t), > devnode->minor), 1); > @@ -291,6 +289,7 @@ void media_devnode_unregister(struct media_devnode > *devnode) > clear_bit(MEDIA_FLAG_REGISTERED, &devnode->flags); > mutex_unlock(&media_devnode_lock); > device_unregister(&devnode->dev); > + cdev_del(&devnode->cdev); Are you sure about this order? Shouldn't cdev_del be called first? The register() calls cdev_add() before device_add(), so I would expect the reverse order here. This is also what cec-core.c does. Regards, Hans > } > > /* > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC? [PATCH] docs-rst: kernel-doc: better output struct members
Em Mon, 22 Aug 2016 14:52:42 +0300
Jani Nikula escreveu:
> On Mon, 22 Aug 2016, Markus Heiser wrote:
> > Am 22.08.2016 um 13:16 schrieb Jani Nikula :
> >
> >> On Mon, 22 Aug 2016, Mauro Carvalho Chehab
> >> wrote:
> >>> Markus,
> >>>
> >>> Em Mon, 22 Aug 2016 10:56:01 +0200
> >>> Markus Heiser escreveu:
> >>>
> Am 21.08.2016 um 14:11 schrieb Mauro Carvalho Chehab
> :
>
> > Right now, for a struct, kernel-doc produces the following output:
> >
> > .. c:type:: struct v4l2_prio_state
> >
> >stores the priority states
> >
> > **Definition**
> >
> > ::
> >
> > struct v4l2_prio_state {
> > atomic_t prios[4];
> > };
> >
> > **Members**
> >
> > ``atomic_t prios[4]``
> > array with elements to store the array priorities
> >
> > Putting a member name in verbatim and adding a continuation line
> > causes the LaTeX output to generate something like:
> > item[atomic_t prios\[4\]] array with elements to store the
> > array priorities
>
>
> Right now, the description of C-struct members is a simple
> rest-definition-list
> (not in the c-domain). It might be better to use the c-domain for
> members:
>
> http://www.sphinx-doc.org/en/stable/domains.html#directive-c:member
>
> But this is not the only thing we have to consider. To make a valid
> C-struct
> description (with targets/references in the c-domain) we need a more
> *structured* reST markup where the members are described in the
> block-content
> of the struct directive. E.g:
>
> ---
> |.. c:type:: struct v4l2_subdev_ir_ops
> |
> | operations for IR subdevices
> |
> | .. c:member:: int (* rx_read) (struct v4l2_subdev *sd, u8 *buf,
> size_t count,ssize_t *num)
> |
> ---
>
> By this small example, you see, that we have to discuss the whole markup
> produced by the kernel-doc script (function arguments, unions etc.).
> IMHO, since kernel-doc is widely used, this should be a RFC.
> >>>
> >>> I tried using c:member. It won't work on LaTeX output, as it will
> >>> still put everything into a LaTeX item, with doesn't do line breaks.
> >>
> >> I've tried c:member before, and I'm not convinced it buys us anything
> >> useful. I'm also not convinced we'd need more structured rst markup
> >> within struct or function descriptions in addition to what we currently
> >> have. Keep it simple.
> >>
> >> BR,
> >> Jani.
> >
> > It buys, that we stay in the c-domain and we can refer to the members
> > with the :c:member role. E.g :c:member:`v4l2_subdev_ir_ops.rx_read`.
>
> Yes, it allows anchors to members, while detaching the member
> descriptions from the struct descriptions. In the output, there is no
> perceivable parent-child relationship between the struct and its
> members. Arguably the resulting documentation is harder to follow with
> c:member than without. I think it's sufficient to link to the struct
> descriptions. It's not enough to say that theoretically using c:member
> is the right thing; it needs to be better in practice too.
Linking to the member is interesting, specially when describing too
big structures, like the callback ones, specially when we have the
descriptions for such things at the rst file (like we have on media kAPI
book). However, the way Sphinx outputs a :c:member: is not nice, IMHO.
I'd say that we should only use .. c:member:: if we can override its behavior
via the C domain extension in a way that it will create a C-domain
reference, but keep producing an output like:
*member_name*
type
some description
on all output formats without any fancy colored struct-like style.
Also, IMHO, it should not add any entry to the genindex, to avoid
polluting it with thousands entries.
If such thing can't be done, let's just stick to what we have right now,
after this patch.
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 07/17] media: Split initialisation and adding device
On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> As registering a device node of an entity belonging to a media device
> will require a reference to the struct device. Taking that reference is
> only possible once the device has been initialised, which took place only
> when it was registered. Split this in two, and initialise the device when
> the media device is allocated.
>
> Signed-off-by: Sakari Ailus
Acked-by: Hans Verkuil
> ---
> drivers/media/media-device.c | 1 +
> drivers/media/media-devnode.c | 15 ++-
> drivers/media/platform/omap3isp/isp.c | 4 +++-
> include/media/media-devnode.h | 15 +++
> 4 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 5a86d36..d527491 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -707,6 +707,7 @@ struct media_device *media_device_alloc(void)
> if (!mdev)
> return NULL;
>
> + media_devnode_init(&mdev->devnode);
> media_device_init(mdev);
>
> return mdev;
> diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
> index 7481c96..aa8030b 100644
> --- a/drivers/media/media-devnode.c
> +++ b/drivers/media/media-devnode.c
> @@ -219,6 +219,11 @@ static const struct file_operations media_devnode_fops =
> {
> .llseek = no_llseek,
> };
>
> +void media_devnode_init(struct media_devnode *devnode)
> +{
> + device_initialize(&devnode->dev);
> +}
> +
> int __must_check media_devnode_register(struct media_devnode *devnode,
> struct module *owner)
> {
> @@ -256,7 +261,7 @@ int __must_check media_devnode_register(struct
> media_devnode *devnode,
> if (devnode->parent)
> devnode->dev.parent = devnode->parent;
> dev_set_name(&devnode->dev, "media%d", devnode->minor);
> - ret = device_register(&devnode->dev);
> + ret = device_add(&devnode->dev);
> if (ret < 0) {
> pr_err("%s: device_register failed\n", __func__);
> goto error;
> @@ -291,7 +296,7 @@ void media_devnode_unregister(struct media_devnode
> *devnode)
> /*
> * Initialise media for linux
> */
> -static int __init media_devnode_init(void)
> +static int __init media_devnode_module_init(void)
> {
> int ret;
>
> @@ -313,14 +318,14 @@ static int __init media_devnode_init(void)
> return 0;
> }
>
> -static void __exit media_devnode_exit(void)
> +static void __exit media_devnode_module_exit(void)
> {
> bus_unregister(&media_bus_type);
> unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES);
> }
>
> -subsys_initcall(media_devnode_init);
> -module_exit(media_devnode_exit)
> +subsys_initcall(media_devnode_module_init);
> +module_exit(media_devnode_module_exit)
>
> MODULE_AUTHOR("Laurent Pinchart ");
> MODULE_DESCRIPTION("Device node registration for media drivers");
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c
> index 5d54e2c..aa32537 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -1726,8 +1726,10 @@ static int isp_register_entities(struct isp_device
> *isp)
> goto done;
>
> done:
> - if (ret < 0)
> + if (ret < 0) {
> isp_unregister_entities(isp);
> + media_device_put(&isp->media_dev);
> + }
>
> return ret;
> }
> diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
> index a0f6823..5253a4b 100644
> --- a/include/media/media-devnode.h
> +++ b/include/media/media-devnode.h
> @@ -102,6 +102,17 @@ struct media_devnode {
> #define to_media_devnode(cd) container_of(cd, struct media_devnode, dev)
>
> /**
> + * media_devnode_init - initialise a media devnode
> + *
> + * @devnode: struct media_devnode we want to initialise
> + *
> + * Initialise a media devnode. Note that after initialising the media
> + * devnode is refcounted. Releaseing references to it may be done
> + * using put_device.
> + */
> +void media_devnode_init(struct media_devnode *devnode);
> +
> +/**
> * media_devnode_register - register a media device node
> *
> * @devnode: struct media_devnode we want to register a device node
> @@ -112,10 +123,6 @@ struct media_devnode {
> * or if the registration of the device node fails.
> *
> * Zero is returned on success.
> - *
> - * Note that if the media_devnode_register call fails, the release()
> callback of
> - * the media_devnode structure is *not* called, so the caller is responsible
> for
> - * freeing any data.
> */
> int __must_check media_devnode_register(struct media_devnode *devnode,
> struct module *owner);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.
Re: [RFC v2 06/17] media: Dynamically allocate the media device
On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> From: Sakari Ailus
>
> Allow allocating the media device dynamically. As the struct media_device
> embeds struct media_devnode, the lifetime of that object is that same than
> that of the media_device.
>
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/media-device.c | 22 ++
> include/media/media-device.h | 38 ++
> 2 files changed, 60 insertions(+)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index a431775..5a86d36 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -544,7 +544,15 @@ static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
>
> static void media_device_release(struct media_devnode *devnode)
> {
> + struct media_device *mdev = to_media_device(devnode);
> +
> + ida_destroy(&mdev->entity_internal_idx);
> + mdev->entity_internal_idx_max = 0;
> + media_entity_graph_walk_cleanup(&mdev->pm_count_walk);
> + mutex_destroy(&mdev->graph_mutex);
> dev_dbg(devnode->parent, "Media device released\n");
> +
> + kfree(mdev);
Doesn't this break bisect? mdev is now freed, but media_device_alloc isn't
called yet.
That doesn't seem right.
Regards,
Hans
> }
>
> /**
> @@ -691,6 +699,20 @@ void media_device_init(struct media_device *mdev)
> }
> EXPORT_SYMBOL_GPL(media_device_init);
>
> +struct media_device *media_device_alloc(void)
> +{
> + struct media_device *mdev;
> +
> + mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> + if (!mdev)
> + return NULL;
> +
> + media_device_init(mdev);
> +
> + return mdev;
> +}
> +EXPORT_SYMBOL_GPL(media_device_alloc);
> +
> void media_device_cleanup(struct media_device *mdev)
> {
> ida_destroy(&mdev->entity_internal_idx);
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index 4eee613..d1d45ab 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -197,6 +197,42 @@ static inline __must_check int media_entity_enum_init(
> void media_device_init(struct media_device *mdev);
>
> /**
> + * media_device_alloc() - Allocate and initialise a media device
> + *
> + * Allocate and initialise a media device. Returns a media device.
> + * The media device is refcounted, and this function returns a media
> + * device the refcount of which is one (1).
> + *
> + * References are taken and given using media_device_get() and
> + * media_device_put().
> + */
> +struct media_device *media_device_alloc(void);
> +
> +/**
> + * media_device_get() - Get a reference to a media device
> + *
> + * mdev: media device
> + */
> +#define media_device_get(mdev)
> \
> + do {\
> + dev_dbg((mdev)->dev, "%s: get media device %s\n", \
> + __func__, (mdev)->bus_info);\
> + get_device(&(mdev)->devnode.dev); \
> + } while (0)
> +
> +/**
> + * media_device_put() - Put a reference to a media device
> + *
> + * mdev: media device
> + */
> +#define media_device_put(mdev)
> \
> + do {\
> + dev_dbg((mdev)->dev, "%s: put media device %s\n", \
> + __func__, (mdev)->bus_info);\
> + put_device(&(mdev)->devnode.dev); \
> + } while (0)
> +
> +/**
> * media_device_cleanup() - Cleanups a media device element
> *
> * @mdev:pointer to struct &media_device
> @@ -425,6 +461,8 @@ void __media_device_usb_init(struct media_device *mdev,
>const char *driver_name);
>
> #else
> +#define media_device_get(mdev) do { } while (0)
> +#define media_device_put(mdev) do { } while (0)
> static inline int media_device_register(struct media_device *mdev)
> {
> return 0;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 04/17] media: Remove useless curly braces and parentheses
On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus
Acked-by: Hans Verkuil
> ---
> drivers/media/media-device.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index a1cd50f..8bdc316 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -596,9 +596,8 @@ int __must_check media_device_register_entity(struct
> media_device *mdev,
> &entity->pads[i].graph_obj);
>
> /* invoke entity_notify callbacks */
> - list_for_each_entry_safe(notify, next, &mdev->entity_notify, list) {
> - (notify)->notify(entity, notify->notify_data);
> - }
> + list_for_each_entry_safe(notify, next, &mdev->entity_notify, list)
> + notify->notify(entity, notify->notify_data);
>
> if (mdev->entity_internal_idx_max
> >= mdev->pm_count_walk.ent_enum.idx_max) {
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 05/17] media: devnode: Rename mdev argument as devnode
On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> Historically, mdev argument name was being used on both struct
> media_device and struct media_devnode. Recently most occurrences of mdev
> referring to struct media_devnode were replaced by devnode, which makes
> more sense. Fix the last remaining occurrence.
>
> Fixes: 163f1e93e9950 ("[media] media-devnode: fix namespace mess")
> Signed-off-by: Sakari Ailus
Acked-by: Hans Verkuil
> ---
> drivers/media/media-device.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 8bdc316..a431775 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -542,9 +542,9 @@ static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
> * Registration/unregistration
> */
>
> -static void media_device_release(struct media_devnode *mdev)
> +static void media_device_release(struct media_devnode *devnode)
> {
> - dev_dbg(mdev->parent, "Media device released\n");
> + dev_dbg(devnode->parent, "Media device released\n");
> }
>
> /**
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC? [PATCH] docs-rst: kernel-doc: better output struct members
On Mon, 22 Aug 2016, Markus Heiser wrote:
> Am 22.08.2016 um 13:16 schrieb Jani Nikula :
>
>> On Mon, 22 Aug 2016, Mauro Carvalho Chehab wrote:
>>> Markus,
>>>
>>> Em Mon, 22 Aug 2016 10:56:01 +0200
>>> Markus Heiser escreveu:
>>>
Am 21.08.2016 um 14:11 schrieb Mauro Carvalho Chehab
:
> Right now, for a struct, kernel-doc produces the following output:
>
> .. c:type:: struct v4l2_prio_state
>
> stores the priority states
>
> **Definition**
>
> ::
>
> struct v4l2_prio_state {
> atomic_t prios[4];
> };
>
> **Members**
>
> ``atomic_t prios[4]``
> array with elements to store the array priorities
>
> Putting a member name in verbatim and adding a continuation line
> causes the LaTeX output to generate something like:
> item[atomic_t prios\[4\]] array with elements to store the array
> priorities
Right now, the description of C-struct members is a simple
rest-definition-list
(not in the c-domain). It might be better to use the c-domain for members:
http://www.sphinx-doc.org/en/stable/domains.html#directive-c:member
But this is not the only thing we have to consider. To make a valid
C-struct
description (with targets/references in the c-domain) we need a more
*structured* reST markup where the members are described in the
block-content
of the struct directive. E.g:
---
|.. c:type:: struct v4l2_subdev_ir_ops
|
| operations for IR subdevices
|
| .. c:member:: int (* rx_read) (struct v4l2_subdev *sd, u8 *buf,
size_t count,ssize_t *num)
|
---
By this small example, you see, that we have to discuss the whole markup
produced by the kernel-doc script (function arguments, unions etc.).
IMHO, since kernel-doc is widely used, this should be a RFC.
>>>
>>> I tried using c:member. It won't work on LaTeX output, as it will
>>> still put everything into a LaTeX item, with doesn't do line breaks.
>>
>> I've tried c:member before, and I'm not convinced it buys us anything
>> useful. I'm also not convinced we'd need more structured rst markup
>> within struct or function descriptions in addition to what we currently
>> have. Keep it simple.
>>
>> BR,
>> Jani.
>
> It buys, that we stay in the c-domain and we can refer to the members
> with the :c:member role. E.g :c:member:`v4l2_subdev_ir_ops.rx_read`.
Yes, it allows anchors to members, while detaching the member
descriptions from the struct descriptions. In the output, there is no
perceivable parent-child relationship between the struct and its
members. Arguably the resulting documentation is harder to follow with
c:member than without. I think it's sufficient to link to the struct
descriptions. It's not enough to say that theoretically using c:member
is the right thing; it needs to be better in practice too.
BR,
Jani.
>
> -- Markus --
>
>
>
--
Jani Nikula, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC? [PATCH] docs-rst: kernel-doc: better output struct members
Am 22.08.2016 um 12:06 schrieb Mauro Carvalho Chehab :
> Markus,
>
> Em Mon, 22 Aug 2016 10:56:01 +0200
> Markus Heiser escreveu:
>
>> Am 21.08.2016 um 14:11 schrieb Mauro Carvalho Chehab
>> :
>>
>>> Right now, for a struct, kernel-doc produces the following output:
>>>
>>> .. c:type:: struct v4l2_prio_state
>>>
>>>stores the priority states
>>>
>>> **Definition**
>>>
>>> ::
>>>
>>> struct v4l2_prio_state {
>>> atomic_t prios[4];
>>> };
>>>
>>> **Members**
>>>
>>> ``atomic_t prios[4]``
>>> array with elements to store the array priorities
>>>
>>> Putting a member name in verbatim and adding a continuation line
>>> causes the LaTeX output to generate something like:
>>> item[atomic_t prios\[4\]] array with elements to store the array
>>> priorities
>>
>>
>> Right now, the description of C-struct members is a simple
>> rest-definition-list
>> (not in the c-domain). It might be better to use the c-domain for members:
>>
>> http://www.sphinx-doc.org/en/stable/domains.html#directive-c:member
>>
>> But this is not the only thing we have to consider. To make a valid C-struct
>> description (with targets/references in the c-domain) we need a more
>> *structured* reST markup where the members are described in the block-content
>> of the struct directive. E.g:
>>
>> ---
>> |.. c:type:: struct v4l2_subdev_ir_ops
>> |
>> | operations for IR subdevices
>> |
>> | .. c:member:: int (* rx_read) (struct v4l2_subdev *sd, u8 *buf, size_t
>> count,ssize_t *num)
>> |
>> ---
>>
>> By this small example, you see, that we have to discuss the whole markup
>> produced by the kernel-doc script (function arguments, unions etc.).
>> IMHO, since kernel-doc is widely used, this should be a RFC.
>
> I tried using c:member. It won't work on LaTeX output, as it will
> still put everything into a LaTeX item, with doesn't do line breaks.
>
> Also, according to Sphinx manual at
> http://www.sphinx-doc.org/en/stable/domains.html
> The syntax is:
>
> .. c:member:: type name
>
> Describes a C struct member. Example signature:
>
> .. c:member:: PyObject* PyTypeObject.tp_bases
>
> So, it expects as arguments. If the manual is right, it
> would be expecting, instead, the weird syntax:
>
> .. c:member:: int (*) (struct v4l2_subdev *sd, u8 *buf, size_t
> count,ssize_t *num) rx_read
>
> With hurts my eyes.
Argh, you are right ... but it is very confusing. I made a small test::
.. c:type:: struct v4l2_subdev_ir_ops_001
.. c:member:: int (*) (struct v4l2_subdev *sd, u8 *buf, size_t
count,ssize_t *num) rx_read
lorem ipsum
.. c:type:: struct v4l2_subdev_ir_ops_002
operations for IR subdevices
.. c:member:: int (* rx_read) (struct v4l2_subdev *sd, u8 *buf, size_t
count,ssize_t *num)
lorem ipsum
Test refs
=
* :c:member:`v4l2_subdev_ir_ops_001.rx_read`
* :c:member:`v4l2_subdev_ir_ops_002.rx_read`
It seems, the c-parser is very dump, the v4l2_subdev_ir_ops_002.rx_read link
is closed, the 001 not ... a look at the sources:
* reg-exprs:
https://github.com/sphinx-doc/sphinx/blob/master/sphinx/domains/c.py#L29
And the following snippet is all about handling signatures (of func, type,
member .. directives)
m = c_funcptr_sig_re.match(sig)
if m is None:
m = c_sig_re.match(sig)
if m is None:
raise ValueError('no match')
rettype, name, arglist, const = m.groups()
In short: since the signature parsing is such worse, we can't predict
how type and name is parsed ... argh.
> As I guess we don't want to maintain ourselves a LaTeX output Sphinx
> plugin forked from upstream,
Agree
> I guess that a more definitive solution
> would involve overriding the parser for c:member in a way that it would
> produce an output like the one in this path, while creating the proper
> c domain reference for the structure member inside the C domain.
Summarized:
* The c-parser of the Sphinx c-domain, is worse. BTW this is the next
c-parser implementation used in the toolchain (parse-headers, kernel-doc)
https://www.mail-archive.com/linux-media@vger.kernel.org/msg101440.html
* Even if we use a member-directive in the block of a type directive,
it will nod be rendered well in the latex / pdf output.
OK, with this in mind, I agree with Jani ... "keep it simple" ;-)
-- Markus --
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC? [PATCH] docs-rst: kernel-doc: better output struct members
Am 22.08.2016 um 13:16 schrieb Jani Nikula :
> On Mon, 22 Aug 2016, Mauro Carvalho Chehab wrote:
>> Markus,
>>
>> Em Mon, 22 Aug 2016 10:56:01 +0200
>> Markus Heiser escreveu:
>>
>>> Am 21.08.2016 um 14:11 schrieb Mauro Carvalho Chehab
>>> :
>>>
Right now, for a struct, kernel-doc produces the following output:
.. c:type:: struct v4l2_prio_state
stores the priority states
**Definition**
::
struct v4l2_prio_state {
atomic_t prios[4];
};
**Members**
``atomic_t prios[4]``
array with elements to store the array priorities
Putting a member name in verbatim and adding a continuation line
causes the LaTeX output to generate something like:
item[atomic_t prios\[4\]] array with elements to store the array
priorities
>>>
>>>
>>> Right now, the description of C-struct members is a simple
>>> rest-definition-list
>>> (not in the c-domain). It might be better to use the c-domain for members:
>>>
>>> http://www.sphinx-doc.org/en/stable/domains.html#directive-c:member
>>>
>>> But this is not the only thing we have to consider. To make a valid C-struct
>>> description (with targets/references in the c-domain) we need a more
>>> *structured* reST markup where the members are described in the
>>> block-content
>>> of the struct directive. E.g:
>>>
>>> ---
>>> |.. c:type:: struct v4l2_subdev_ir_ops
>>> |
>>> | operations for IR subdevices
>>> |
>>> | .. c:member:: int (* rx_read) (struct v4l2_subdev *sd, u8 *buf, size_t
>>> count,ssize_t *num)
>>> |
>>> ---
>>>
>>> By this small example, you see, that we have to discuss the whole markup
>>> produced by the kernel-doc script (function arguments, unions etc.).
>>> IMHO, since kernel-doc is widely used, this should be a RFC.
>>
>> I tried using c:member. It won't work on LaTeX output, as it will
>> still put everything into a LaTeX item, with doesn't do line breaks.
>
> I've tried c:member before, and I'm not convinced it buys us anything
> useful. I'm also not convinced we'd need more structured rst markup
> within struct or function descriptions in addition to what we currently
> have. Keep it simple.
>
> BR,
> Jani.
It buys, that we stay in the c-domain and we can refer to the members
with the :c:member role. E.g :c:member:`v4l2_subdev_ir_ops.rx_read`.
-- Markus --
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC? [PATCH] docs-rst: kernel-doc: better output struct members
On Mon, 22 Aug 2016, Mauro Carvalho Chehab wrote:
> Markus,
>
> Em Mon, 22 Aug 2016 10:56:01 +0200
> Markus Heiser escreveu:
>
>> Am 21.08.2016 um 14:11 schrieb Mauro Carvalho Chehab
>> :
>>
>> > Right now, for a struct, kernel-doc produces the following output:
>> >
>> >.. c:type:: struct v4l2_prio_state
>> >
>> > stores the priority states
>> >
>> >**Definition**
>> >
>> >::
>> >
>> > struct v4l2_prio_state {
>> >atomic_t prios[4];
>> > };
>> >
>> >**Members**
>> >
>> >``atomic_t prios[4]``
>> > array with elements to store the array priorities
>> >
>> > Putting a member name in verbatim and adding a continuation line
>> > causes the LaTeX output to generate something like:
>> >item[atomic_t prios\[4\]] array with elements to store the array
>> > priorities
>>
>>
>> Right now, the description of C-struct members is a simple
>> rest-definition-list
>> (not in the c-domain). It might be better to use the c-domain for members:
>>
>> http://www.sphinx-doc.org/en/stable/domains.html#directive-c:member
>>
>> But this is not the only thing we have to consider. To make a valid C-struct
>> description (with targets/references in the c-domain) we need a more
>> *structured* reST markup where the members are described in the block-content
>> of the struct directive. E.g:
>>
>> ---
>> |.. c:type:: struct v4l2_subdev_ir_ops
>> |
>> | operations for IR subdevices
>> |
>> | .. c:member:: int (* rx_read) (struct v4l2_subdev *sd, u8 *buf, size_t
>> count,ssize_t *num)
>> |
>> ---
>>
>> By this small example, you see, that we have to discuss the whole markup
>> produced by the kernel-doc script (function arguments, unions etc.).
>> IMHO, since kernel-doc is widely used, this should be a RFC.
>
> I tried using c:member. It won't work on LaTeX output, as it will
> still put everything into a LaTeX item, with doesn't do line breaks.
I've tried c:member before, and I'm not convinced it buys us anything
useful. I'm also not convinced we'd need more structured rst markup
within struct or function descriptions in addition to what we currently
have. Keep it simple.
BR,
Jani.
>
> Also, according to Sphinx manual at
> http://www.sphinx-doc.org/en/stable/domains.html
> The syntax is:
>
> .. c:member:: type name
>
> Describes a C struct member. Example signature:
>
> .. c:member:: PyObject* PyTypeObject.tp_bases
>
> So, it expects as arguments. If the manual is right, it
> would be expecting, instead, the weird syntax:
>
>.. c:member:: int (*) (struct v4l2_subdev *sd, u8 *buf, size_t
> count,ssize_t *num) rx_read
>
> With hurts my eyes.
>
> As I guess we don't want to maintain ourselves a LaTeX output Sphinx
> plugin forked from upstream, I guess that a more definitive solution
> would involve overriding the parser for c:member in a way that it would
> produce an output like the one in this path, while creating the proper
> c domain reference for the structure member inside the C domain.
>
> Regards,
> Mauro
--
Jani Nikula, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] [media] vimc: Virtual Media Controller core, capture and sensor
Hi Helen,
A few small code comments are below.
Note that if I try to capture I see these two messages in the kernel log:
[588197.368145] vimc vimc.0: Entity type for entity Sensor A was not
initialized!
[588197.368169] vimc vimc.0: Entity type for entity Sensor B was not
initialized!
I also can't capture anything: v4l2-ctl --stream-mmap just sits there, waiting
for
frames, I guess.
I'm not sure if that has to do with the two warnings above.
I am assuming that the initial pipeline is correct and that you should be able
to start streaming. If not, then attempting to start streaming should return an
error.
On 08/18/2016 12:09 AM, Helen Koike wrote:
> From: Helen Fornazier
>
> First version of the Virtual Media Controller.
> Add a simple version of the core of the driver, the capture and
> sensor nodes in the topology, generating a grey image in a hardcoded
> format.
>
> Signed-off-by: Helen Koike
> +static int vimc_cap_querycap(struct file *file, void *priv,
> + struct v4l2_capability *cap)
> +{
> + struct vimc_cap_device *vcap = video_drvdata(file);
> +
> + strlcpy(cap->driver, KBUILD_MODNAME, sizeof(cap->driver));
> + strlcpy(cap->card, KBUILD_MODNAME, sizeof(cap->card));
> + snprintf(cap->bus_info, sizeof(cap->bus_info),
> + "platform:%s", vcap->v4l2_dev->name);
> +
> + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
This line should be moved to vimc_cap_create:
vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
This is new. The v4l2 core will fill in the querycap capabilities for you
based on vdev->device_caps.
> +
> + return 0;
> +}
> +static int vimc_device_register(struct vimc_device *vimc)
> +{
> + unsigned int i;
> + int ret = 0;
> +
> + /* Allocate memory for the vimc_ent_devices pointers */
> + vimc->ved = devm_kcalloc(vimc->mdev.dev, vimc->pipe_cfg->num_ents,
> + sizeof(*vimc->ved), GFP_KERNEL);
> + if (!vimc->ved)
> + return -ENOMEM;
> +
> + /* Register the media device */
> + ret = media_device_register(&vimc->mdev);
> + if (ret) {
> + dev_err(vimc->mdev.dev,
> + "media device register failed (err=%d)\n", ret);
> + return ret;
> + }
> +
> + /* Link the media device within the v4l2_device */
> + vimc->v4l2_dev.mdev = &vimc->mdev;
> +
> + /* Register the v4l2 struct */
> + ret = v4l2_device_register(vimc->mdev.dev, &vimc->v4l2_dev);
> + if (ret) {
> + dev_err(vimc->mdev.dev,
> + "v4l2 device register failed (err=%d)\n", ret);
> + return ret;
> + }
> +
> + /* Initialize entities */
> + for (i = 0; i < vimc->pipe_cfg->num_ents; i++) {
> + struct vimc_ent_device *(*create_func)(struct v4l2_device *,
> +const char *const,
> +u16,
> +const unsigned long *);
> +
> + /* Register the specific node */
> + switch (vimc->pipe_cfg->ents[i].node) {
> + case VIMC_ENT_NODE_SENSOR:
> + create_func = vimc_sen_create;
> + break;
> +
> + case VIMC_ENT_NODE_CAPTURE:
> + create_func = vimc_cap_create;
> + break;
> +
> + /* TODO: Instantiate the specific topology node */
> + case VIMC_ENT_NODE_INPUT:
> + case VIMC_ENT_NODE_DEBAYER:
> + case VIMC_ENT_NODE_SCALER:
> + default:
> + /* TODO: remove this when all the entities specific
> + * code are implemented
> + */
> + create_func = vimc_raw_create;
> + break;
> + }
> +
> + vimc->ved[i] = create_func(&vimc->v4l2_dev,
> +vimc->pipe_cfg->ents[i].name,
> +vimc->pipe_cfg->ents[i].pads_qty,
> +vimc->pipe_cfg->ents[i].pads_flag);
> + if (IS_ERR(vimc->ved[i])) {
> + ret = PTR_ERR(vimc->ved[i]);
> + vimc->ved[i] = NULL;
> + goto err;
> + }
> +
> + /* Set use_count to keep track of the ved structure */
> + vimc->ved[i]->ent->use_count = i;
> + }
> +
> + /* Initialize the links between entities */
> + for (i = 0; i < vimc->pipe_cfg->num_links; i++) {
> + const struct vimc_ent_link *link = &vimc->pipe_cfg->links[i];
> +
> + ret = media_create_pad_link(vimc->ved[link->src_ent]->ent,
> + link->src_pad,
> + vimc->ved[link->sink_ent]->ent,
> +
[ANNOUNCE] Linux Kernel Media Subsystem documentation now available in PDF format
Hi all, As you know, the media documentation is daily-built from the Linux Kernel sources, in HTML format, from its ReST files. Starting today, it will also be building the PDF book with all the Kernel Media documentation on it. Despite having 1031 pages, the PDF file has only 3.3 MB. I prefer myself handling the documentation in PDF format, as we have everything there on just one file. Yet, a few tables have small fonts. There are still some things that need to be done to improve the PDF format, specially with regards to its index and to the way it numerates the document parts and sections, but not sure if we'll have any short term solution, as the Sphinx tool, used to build a LaTeX file is pretty much limited for such format, at least up to its version 1.4.6. Anyway, if you want to look into it, The PDF file is available at: https://linuxtv.org/downloads/v4l-dvb-apis-new/media.pdf Thanks, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC? [PATCH] docs-rst: kernel-doc: better output struct members
Markus,
Em Mon, 22 Aug 2016 10:56:01 +0200
Markus Heiser escreveu:
> Am 21.08.2016 um 14:11 schrieb Mauro Carvalho Chehab
> :
>
> > Right now, for a struct, kernel-doc produces the following output:
> >
> > .. c:type:: struct v4l2_prio_state
> >
> >stores the priority states
> >
> > **Definition**
> >
> > ::
> >
> > struct v4l2_prio_state {
> > atomic_t prios[4];
> > };
> >
> > **Members**
> >
> > ``atomic_t prios[4]``
> > array with elements to store the array priorities
> >
> > Putting a member name in verbatim and adding a continuation line
> > causes the LaTeX output to generate something like:
> > item[atomic_t prios\[4\]] array with elements to store the array
> > priorities
>
>
> Right now, the description of C-struct members is a simple
> rest-definition-list
> (not in the c-domain). It might be better to use the c-domain for members:
>
> http://www.sphinx-doc.org/en/stable/domains.html#directive-c:member
>
> But this is not the only thing we have to consider. To make a valid C-struct
> description (with targets/references in the c-domain) we need a more
> *structured* reST markup where the members are described in the block-content
> of the struct directive. E.g:
>
> ---
> |.. c:type:: struct v4l2_subdev_ir_ops
> |
> | operations for IR subdevices
> |
> | .. c:member:: int (* rx_read) (struct v4l2_subdev *sd, u8 *buf, size_t
> count,ssize_t *num)
> |
> ---
>
> By this small example, you see, that we have to discuss the whole markup
> produced by the kernel-doc script (function arguments, unions etc.).
> IMHO, since kernel-doc is widely used, this should be a RFC.
I tried using c:member. It won't work on LaTeX output, as it will
still put everything into a LaTeX item, with doesn't do line breaks.
Also, according to Sphinx manual at
http://www.sphinx-doc.org/en/stable/domains.html
The syntax is:
.. c:member:: type name
Describes a C struct member. Example signature:
.. c:member:: PyObject* PyTypeObject.tp_bases
So, it expects as arguments. If the manual is right, it
would be expecting, instead, the weird syntax:
.. c:member:: int (*) (struct v4l2_subdev *sd, u8 *buf, size_t
count,ssize_t *num) rx_read
With hurts my eyes.
As I guess we don't want to maintain ourselves a LaTeX output Sphinx
plugin forked from upstream, I guess that a more definitive solution
would involve overriding the parser for c:member in a way that it would
produce an output like the one in this path, while creating the proper
c domain reference for the structure member inside the C domain.
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] v4l-utils: fixed dvbv5 vdr format
Am 16.08.2016 um 09:10 schrieb Markus Heiser :
> Am 10.08.2016 um 11:52 schrieb Markus Heiser :
>
>> The vdr format was broken, I got '(null)' entries
>>
>> HD:11494:S1HC23I0M5N1O35:S:(null):22000:5101:5102,5103,5106,5108:0:0:10301:0:0:0:
>> 0-:1:2--:3:4-:
>>
>> refering to the VDR Wikis ...
>>
>> * LinuxTV: http://www.linuxtv.org/vdrwiki/index.php/Syntax_of_channels.conf
>> * german comunity Wiki:
>> http://www.vdr-wiki.de/wiki/index.php/Channels.conf#Parameter_ab_VDR-1.7.4
>>
>> There is no field at position 4 / in between "Source" and "SRate" which
>> might have a value. I suppose the '(null):' is the result of pointing
>> to *nothing*.
>>
>> An other mistake is the ending colon (":") at the line. It is not
>> explicit specified but adding an collon to the end of an channel entry
>> will prevent players (like mpv or mplayer) from parsing the line (they
>> will ignore these lines).
>>
>> At least: generating a channel list with
>>
>> dvbv5-scan --output-format=vdr ...
>>
>> will result in the same defective channel entry, containing "(null):"
>> and the leading collon ":".
>>
>
> Hi,
>
> please apply this patch or give me at least some feedback / thanks.
>
> -- Markus
Sorry for bumping ... but, since there is no reaction on the ML
I have to bump :-(
Please, please, please ... apply this patch. The VDR format of
the libdvbv5 is definitely broken!
https://patchwork.linuxtv.org/patch/36293/
-- Markus
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL FOR v4.9] Add touch API and atmel_mxt_ts/synaptics drivers (v2)
This patch series adds support for /dev/v4l-touchX devices. Regards, Hans Changes since v1: Updated the sur40 patch to fix a compiler error. The following changes since commit b6aa39228966e0d3f0bc3306be1892f87792903a: Merge tag 'v4.8-rc1' into patchwork (2016-08-08 07:30:25 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git touch for you to fetch changes up to 29064fa6376528929d1aa560f6faa7bddc333d69: Documentation: add support for V4L touch devices (2016-08-22 11:29:22 +0200) Nick Dyer (11): Input: atmel_mxt_ts - update MAINTAINERS email address v4l2-core: Add support for touch devices Input: atmel_mxt_ts - add support for T37 diagnostic data Input: atmel_mxt_ts - output diagnostic debug via V4L2 device Input: atmel_mxt_ts - read touchscreen size Input: atmel_mxt_ts - handle diagnostic data orientation Input: atmel_mxt_ts - add diagnostic data support for mXT1386 Input: atmel_mxt_ts - add support for reference data Input: synaptics-rmi4 - add support for F54 diagnostics Input: sur40 - use new V4L2 touch input type Documentation: add support for V4L touch devices Documentation/media/kapi/v4l2-dev.rst | 1 + Documentation/media/uapi/mediactl/media-types.rst | 24 +- Documentation/media/uapi/v4l/dev-touch.rst| 56 Documentation/media/uapi/v4l/devices.rst | 1 + Documentation/media/uapi/v4l/pixfmt-tch-td08.rst | 80 ++ Documentation/media/uapi/v4l/pixfmt-tch-td16.rst | 111 Documentation/media/uapi/v4l/pixfmt-tch-tu08.rst | 78 ++ Documentation/media/uapi/v4l/pixfmt-tch-tu16.rst | 110 Documentation/media/uapi/v4l/pixfmt.rst | 1 + Documentation/media/uapi/v4l/tch-formats.rst | 18 ++ Documentation/media/uapi/v4l/vidioc-enuminput.rst | 8 + Documentation/media/uapi/v4l/vidioc-querycap.rst | 8 + Documentation/media/videodev2.h.rst.exceptions| 2 + MAINTAINERS | 6 +- drivers/input/rmi4/Kconfig| 11 + drivers/input/rmi4/Makefile | 1 + drivers/input/rmi4/rmi_bus.c | 3 + drivers/input/rmi4/rmi_driver.h | 1 + drivers/input/rmi4/rmi_f54.c | 756 ++ drivers/input/touchscreen/Kconfig | 8 + drivers/input/touchscreen/atmel_mxt_ts.c | 520 ++ drivers/input/touchscreen/sur40.c | 124 ++--- drivers/media/media-entity.c | 2 + drivers/media/v4l2-core/v4l2-dev.c| 14 +- drivers/media/v4l2-core/v4l2-ioctl.c | 36 ++- include/media/v4l2-dev.h | 4 +- include/uapi/linux/media.h| 1 + include/uapi/linux/videodev2.h| 9 + 28 files changed, 1942 insertions(+), 52 deletions(-) create mode 100644 Documentation/media/uapi/v4l/dev-touch.rst create mode 100644 Documentation/media/uapi/v4l/pixfmt-tch-td08.rst create mode 100644 Documentation/media/uapi/v4l/pixfmt-tch-td16.rst create mode 100644 Documentation/media/uapi/v4l/pixfmt-tch-tu08.rst create mode 100644 Documentation/media/uapi/v4l/pixfmt-tch-tu16.rst create mode 100644 Documentation/media/uapi/v4l/tch-formats.rst create mode 100644 drivers/input/rmi4/rmi_f54.c -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5_2 10/12] [media] videodev2.h Add HSV encoding
Some hardware maps the Hue between 0 and 255 instead of 0-179. Support
this format with a new field hsv_enc.
Signed-off-by: Ricardo Ribalda Delgado
---
v5_2: s/s_rgb_or_yuv/s_rgb_or_hsv/
Thanks Hans!!
include/uapi/linux/videodev2.h | 32 +++-
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 58ed8aedc196..baa874d91299 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -335,6 +335,19 @@ enum v4l2_ycbcr_encoding {
};
/*
+ * enum v4l2_hsv_encoding values should not collide with the ones from
+ * enum v4l2_ycbcr_encoding.
+ */
+enum v4l2_hsv_encoding {
+
+ /* Hue mapped to 0 - 179 */
+ V4L2_HSV_ENC_180= 128,
+
+ /* Hue mapped to 0-255 */
+ V4L2_HSV_ENC_256= 129,
+};
+
+/*
* Determine how YCBCR_ENC_DEFAULT should map to a proper Y'CbCr encoding.
* This depends on the colorspace.
*/
@@ -362,9 +375,10 @@ enum v4l2_quantization {
* This depends on whether the image is RGB or not, the colorspace and the
* Y'CbCr encoding.
*/
-#define V4L2_MAP_QUANTIZATION_DEFAULT(is_rgb, colsp, ycbcr_enc) \
- (((is_rgb) && (colsp) == V4L2_COLORSPACE_BT2020) ?
V4L2_QUANTIZATION_LIM_RANGE : \
-(((is_rgb) || (ycbcr_enc) == V4L2_YCBCR_ENC_XV601 || \
+#define V4L2_MAP_QUANTIZATION_DEFAULT(is_rgb_or_hsv, colsp, ycbcr_enc) \
+ (((is_rgb_or_hsv) && (colsp) == V4L2_COLORSPACE_BT2020) ? \
+V4L2_QUANTIZATION_LIM_RANGE : \
+(((is_rgb_or_hsv) || (ycbcr_enc) == V4L2_YCBCR_ENC_XV601 || \
(ycbcr_enc) == V4L2_YCBCR_ENC_XV709 || (colsp) ==
V4L2_COLORSPACE_JPEG) || \
(colsp) == V4L2_COLORSPACE_ADOBERGB || (colsp) ==
V4L2_COLORSPACE_SRGB ? \
V4L2_QUANTIZATION_FULL_RANGE : V4L2_QUANTIZATION_LIM_RANGE))
@@ -460,7 +474,12 @@ struct v4l2_pix_format {
__u32 colorspace; /* enum v4l2_colorspace */
__u32 priv; /* private data, depends on
pixelformat */
__u32 flags; /* format flags
(V4L2_PIX_FMT_FLAG_*) */
- __u32 ycbcr_enc; /* enum v4l2_ycbcr_encoding */
+ union {
+ /* enum v4l2_ycbcr_encoding */
+ __u32 ycbcr_enc;
+ /* enum v4l2_hsv_encoding */
+ __u32 hsv_enc;
+ };
__u32 quantization; /* enum v4l2_quantization */
__u32 xfer_func; /* enum v4l2_xfer_func */
};
@@ -1993,7 +2012,10 @@ struct v4l2_pix_format_mplane {
struct v4l2_plane_pix_formatplane_fmt[VIDEO_MAX_PLANES];
__u8num_planes;
__u8flags;
- __u8ycbcr_enc;
+union {
+ __u8ycbcr_enc;
+ __u8hsv_enc;
+ };
__u8quantization;
__u8xfer_func;
__u8reserved[7];
--
2.8.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 10/12] [media] videodev2.h Add HSV encoding
Hello Hans: > > That should be is_rgb_or_hsv. Sorry about that! I am resending v5_2 with only that patch fixed > > All other patches look OK. Thanks > > It would be useful though if you could rebase on top of > https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=sycc. > I have a pull request outstanding for that tree, and it will conflict with > this patch. It should be already be rebased over that branch: ricardo@neopili:~/curro/linux-upstream$ git rebase hans/sycc Current branch vivid-hsv-v5 is up to date. Best regards! -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL FOR v4.9] rcar-vin: clean up and prepare for Gen3
See cover letter of this patch series for more details: http://www.spinics.net/lists/linux-renesas-soc/msg06449.html Hans The following changes since commit b6aa39228966e0d3f0bc3306be1892f87792903a: Merge tag 'v4.8-rc1' into patchwork (2016-08-08 07:30:25 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git rcarvin for you to fetch changes up to 77434b5e30b57eaed9932cc95d03004702993950: rcar-vin: move media bus information to struct rvin_graph_entity (2016-08-19 16:08:48 +0200) Niklas Söderlund (10): rcar-vin: fix indentation errors in rcar-v4l2.c rcar-vin: reduce indentation in rvin_s_dv_timings() rcar-vin: arrange enum chip_id in chronological order rcar-vin: rename entity to digital rcar-vin: return correct error from platform_get_irq() rcar-vin: do not use v4l2_device_call_until_err() rcar-vin: add dependency on MEDIA_CONTROLLER rcar-vin: move chip check for pixelformat support rcar-vin: rework how subdeivce is found and bound rcar-vin: move media bus information to struct rvin_graph_entity drivers/media/platform/rcar-vin/Kconfig | 2 +- drivers/media/platform/rcar-vin/rcar-core.c | 258 ++-- drivers/media/platform/rcar-vin/rcar-dma.c | 18 ++-- drivers/media/platform/rcar-vin/rcar-v4l2.c | 91 ++-- drivers/media/platform/rcar-vin/rcar-vin.h | 25 +++--- 5 files changed, 186 insertions(+), 208 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] rcar-vin: add legacy mode for wrong media bus formats
On 07/20/2016 02:36 PM, Hans Verkuil wrote: > On 07/20/2016 02:29 PM, Niklas Söderlund wrote: >> Hi Hans, >> >> Thanks for your feedback. >> >> On 2016-07-20 11:48:40 +0200, Hans Verkuil wrote: >>> On 07/08/2016 12:43 PM, Niklas Söderlund wrote: A recent bugfix to adv7180 brought to light that the rcar-vin driver are looking for the wrong media bus format. It was looking for a YUVU format but then expecting UYVY data. The bugfix for adv7180 will break the usage of rcar-vin together with a adv7180 as found on Renesas R-Car2 Koelsch boards for example. This patch fix the rcar-vin driver to look for the correct UYVU formats and adds a legacy mode. The legacy mode is needed since I don't know if other devices provide a incorrect media bus format and I don't want to break any working configurations. Hopefully the legacy mode can be removed sometime in the future. >>> >>> I'd rather have a version without the legacy code. You have to assume that >>> subdevs return correct values otherwise what's the point of the mediabus >>> formats? >>> >>> So this is simply an adv7180 bug fix + this r-car fix to stay consistent >>> with the adv7180. >> >> On principal I agree with you. My goal with this patch is just to make >> sure there is no case where the rcar-vin driver won't work with the >> adv7180. The plan was to drop the legacy mode in a separate patch after >> both the adv7182 and rcar-vin patches where picked up. >> >> I'm happy to drop the 'legacy support' for the wrong formats from this >> patch as long as I can be sure that there is no breaking. Should I >> rewrite this patch to drop the wrong formats and submit it as a series >> together with the adv7180 patch so they can be picked up together? Or do >> you know of a better way? > > Why not combine this patch and the adv7180 patch in a single patch? Just keep > Steve's Signed-off-by line together with yours. That way everything stays > in sync. The only other user of the adv7180 doesn't look at the mediabus > formats at all, so it isn't affected. Niklas, Were you planning to make a combined adv7180/rcar-vin patch for this? I would prefer this solution rather than keeping legacy code around. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] cec-follower: check for Routing Information from TV
A TV shall not send a Routing Information message as initiator.
Signed-off-by: Johan Fjeldtvedt
---
utils/cec-follower/cec-processing.cpp | 6 ++
1 file changed, 6 insertions(+)
diff --git a/utils/cec-follower/cec-processing.cpp
b/utils/cec-follower/cec-processing.cpp
index 771eb2d..34d65e4 100644
--- a/utils/cec-follower/cec-processing.cpp
+++ b/utils/cec-follower/cec-processing.cpp
@@ -408,6 +408,12 @@ static void processMsg(struct node *node, struct cec_msg
&msg, unsigned me)
dev_info("Stream Path is directed to this device\n");
return;
}
+ case CEC_MSG_ROUTING_INFORMATION: {
+ __u8 la = cec_msg_initiator(&msg);
+
+ if (cec_has_tv(1 << la) && la_info[la].phys_addr == 0)
+ warn("TV (0) at 0.0.0.0 sent Routing Information.");
+ }
/* System Information */
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 2/4] pulse8-cec: serialize communication with adapter
Make sending messages to the adapter serialized within the driver.
send_and_wait is split into send_and_wait_once, which only sends once
and checks for the result, and the higher level send_and_wait, which
performs locking and retries.
Signed-off-by: Johan Fjeldtvedt
---
drivers/staging/media/pulse8-cec/pulse8-cec.c | 52 ---
1 file changed, 32 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/media/pulse8-cec/pulse8-cec.c
b/drivers/staging/media/pulse8-cec/pulse8-cec.c
index ed8bd95..37c8418 100644
--- a/drivers/staging/media/pulse8-cec/pulse8-cec.c
+++ b/drivers/staging/media/pulse8-cec/pulse8-cec.c
@@ -99,6 +99,7 @@ struct pulse8 {
unsigned int idx;
bool escape;
bool started;
+ struct mutex write_lock;
};
static void pulse8_irq_work_handler(struct work_struct *work)
@@ -233,8 +234,9 @@ static int pulse8_send(struct serio *serio, const u8
*command, u8 cmd_len)
return err;
}
-static int pulse8_send_and_wait(struct pulse8 *pulse8,
- const u8 *cmd, u8 cmd_len, u8 response, u8 size)
+static int pulse8_send_and_wait_once(struct pulse8 *pulse8,
+const u8 *cmd, u8 cmd_len,
+u8 response, u8 size)
{
int err;
@@ -250,24 +252,8 @@ static int pulse8_send_and_wait(struct pulse8 *pulse8,
if ((pulse8->data[0] & 0x3f) == MSGCODE_COMMAND_REJECTED &&
cmd[0] != MSGCODE_SET_CONTROLLED &&
cmd[0] != MSGCODE_SET_AUTO_ENABLED &&
- cmd[0] != MSGCODE_GET_BUILDDATE) {
- u8 cmd_sc[2];
-
- cmd_sc[0] = MSGCODE_SET_CONTROLLED;
- cmd_sc[1] = 1;
- err = pulse8_send_and_wait(pulse8, cmd_sc, 2,
- MSGCODE_COMMAND_ACCEPTED, 1);
- if (err)
- return err;
- init_completion(&pulse8->cmd_done);
-
- err = pulse8_send(pulse8->serio, cmd, cmd_len);
- if (err)
- return err;
-
- if (!wait_for_completion_timeout(&pulse8->cmd_done, HZ))
- return -ETIMEDOUT;
- }
+ cmd[0] != MSGCODE_GET_BUILDDATE)
+ return -ENOTTY;
if (response &&
((pulse8->data[0] & 0x3f) != response || pulse8->len < size + 1)) {
dev_info(pulse8->dev, "transmit: failed %02x\n",
@@ -277,6 +263,31 @@ static int pulse8_send_and_wait(struct pulse8 *pulse8,
return 0;
}
+static int pulse8_send_and_wait(struct pulse8 *pulse8,
+ const u8 *cmd, u8 cmd_len, u8 response, u8 size)
+{
+ u8 cmd_sc[2];
+ int err;
+
+ mutex_lock(&pulse8->write_lock);
+ err = pulse8_send_and_wait_once(pulse8, cmd, cmd_len, response, size);
+
+ if (err == -ENOTTY) {
+ cmd_sc[0] = MSGCODE_SET_CONTROLLED;
+ cmd_sc[1] = 1;
+ err = pulse8_send_and_wait_once(pulse8, cmd_sc, 2,
+ MSGCODE_COMMAND_ACCEPTED, 1);
+ if (err)
+ goto unlock;
+ err = pulse8_send_and_wait_once(pulse8, cmd, cmd_len,
+ response, size);
+ }
+
+unlock:
+ mutex_unlock(&pulse8->write_lock);
+ return err == -ENOTTY ? -EIO : err;
+}
+
static int pulse8_setup(struct pulse8 *pulse8, struct serio *serio)
{
u8 *data = pulse8->data + 1;
@@ -453,6 +464,7 @@ static int pulse8_connect(struct serio *serio, struct
serio_driver *drv)
pulse8->dev = &serio->dev;
serio_set_drvdata(serio, pulse8);
INIT_WORK(&pulse8->work, pulse8_irq_work_handler);
+ mutex_init(&pulse8->write_lock);
err = serio_open(serio, drv);
if (err)
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 3/4] pulse8-cec: add notes about behavior in autonomous mode
The pulse8 dongle has some quirky behaviors when in autonomous mode. Document these. Signed-off-by: Johan Fjeldtvedt --- drivers/staging/media/pulse8-cec/pulse8-cec.c | 23 +++ 1 file changed, 23 insertions(+) diff --git a/drivers/staging/media/pulse8-cec/pulse8-cec.c b/drivers/staging/media/pulse8-cec/pulse8-cec.c index 37c8418..aa679a3 100644 --- a/drivers/staging/media/pulse8-cec/pulse8-cec.c +++ b/drivers/staging/media/pulse8-cec/pulse8-cec.c @@ -10,6 +10,29 @@ * this archive for more details. */ +/* + * Notes: + * + * - Devices with firmware version < 2 do not store their configuration in + * EEPROM. + * + * - In autonomous mode, only messages from a TV will be acknowledged, even + * polling messages. Upon receiving a message from a TV, the dongle will + * respond to messages from any logical address. + * + * - In autonomous mode, the dongle will by default reply Feature Abort + * [Unrecognized Opcode] when it receives Give Device Vendor ID. It will + * however observe vendor ID's reported by other devices and possibly + * alter this behavior. When TV's (and TV's only) report that their vendor ID + * is LG (0x00e091), the dongle will itself reply that it has the same vendor + * ID, and it will respond to at least one vendor specific command. + * + * - In autonomous mode, the dongle is known to attempt wakeup if it receives + *["Power On"], ["Power] or ["Power Toggle"], or if it + * receives with its own physical address. It also does this + * if it receives [0x03 0x00] from an LG TV. + */ + #include #include #include -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 0/4] pulse8-cec: Add support for storing and optionally restoring config
This adds support for storing the CEC adapter config in the Pulse-Eight dongle's persistent memory, and to optionally restore it when the device is (re)connected. This allows the dongle to continue operating with the same settings when the PC is suspended. Changes in v2: - Fix checkpatch warnings and spelling error - Add missing break - Don't propagate internal error code to user space Johan Fjeldtvedt (4): cec: allow configuration both from within driver and from user space pulse8-cec: serialize communication with adapter pulse8-cec: add notes about behavior in autonomous mode pulse8-cec: sync configuration with adapter drivers/staging/media/cec/cec-adap.c | 4 - drivers/staging/media/pulse8-cec/pulse8-cec.c | 341 +- 2 files changed, 283 insertions(+), 62 deletions(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 1/4] cec: allow configuration both from within driver and from user space
It makes sense for adapters such as the Pulse-Eight to be configurable
both from within the driver and from user space, so remove the
requirement that drivers only can call cec_s_log_addrs or
cec_s_phys_addr if they don't expose those capabilities to user space.
Signed-off-by: Johan Fjeldtvedt
---
drivers/staging/media/cec/cec-adap.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/staging/media/cec/cec-adap.c
b/drivers/staging/media/cec/cec-adap.c
index b2393bb..608e3e7 100644
--- a/drivers/staging/media/cec/cec-adap.c
+++ b/drivers/staging/media/cec/cec-adap.c
@@ -1153,8 +1153,6 @@ void cec_s_phys_addr(struct cec_adapter *adap, u16
phys_addr, bool block)
if (IS_ERR_OR_NULL(adap))
return;
- if (WARN_ON(adap->capabilities & CEC_CAP_PHYS_ADDR))
- return;
mutex_lock(&adap->lock);
__cec_s_phys_addr(adap, phys_addr, block);
mutex_unlock(&adap->lock);
@@ -1295,8 +1293,6 @@ int cec_s_log_addrs(struct cec_adapter *adap,
{
int err;
- if (WARN_ON(adap->capabilities & CEC_CAP_LOG_ADDRS))
- return -EINVAL;
mutex_lock(&adap->lock);
err = __cec_s_log_addrs(adap, log_addrs, block);
mutex_unlock(&adap->lock);
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 4/4] pulse8-cec: sync configuration with adapter
When the configuration is changed, they are also written to the adapter.
This allows the adapter to continue operating in autonomous mode with
the same settings when it is disconnected from the driver (typically by
going into suspend). For adapters with firmware version 2 or greater, the
settings are also persisted in EEPROM.
A new module parameter is added to optionally also use the configuration
already present in the adapter when it is connected. This option is
enabled by default.
When a new configuration is written, the autonomous mode is
automatically enabled. When the device is unconfigured, autonomous mode
is disabled.
Signed-off-by: Johan Fjeldtvedt
---
drivers/staging/media/pulse8-cec/pulse8-cec.c | 266 ++
1 file changed, 228 insertions(+), 38 deletions(-)
diff --git a/drivers/staging/media/pulse8-cec/pulse8-cec.c
b/drivers/staging/media/pulse8-cec/pulse8-cec.c
index aa679a3..193f4d1 100644
--- a/drivers/staging/media/pulse8-cec/pulse8-cec.c
+++ b/drivers/staging/media/pulse8-cec/pulse8-cec.c
@@ -51,8 +51,11 @@ MODULE_DESCRIPTION("Pulse Eight HDMI CEC driver");
MODULE_LICENSE("GPL");
static int debug;
+static int persistent_config = 1;
module_param(debug, int, 0644);
+module_param(persistent_config, int, 0644);
MODULE_PARM_DESC(debug, "debug level (0-1)");
+MODULE_PARM_DESC(persistent_config, "read config from persistent memory
(0-1)");
enum pulse8_msgcodes {
MSGCODE_NOTHING = 0,
@@ -109,12 +112,16 @@ enum pulse8_msgcodes {
#define DATA_SIZE 256
+#define PING_PERIOD(15 * HZ)
+
struct pulse8 {
struct device *dev;
struct serio *serio;
struct cec_adapter *adap;
+ unsigned int vers;
struct completion cmd_done;
struct work_struct work;
+ struct delayed_work ping_eeprom_work;
struct cec_msg rx_msg;
u8 data[DATA_SIZE];
unsigned int len;
@@ -122,9 +129,15 @@ struct pulse8 {
unsigned int idx;
bool escape;
bool started;
+ struct mutex config_lock;
struct mutex write_lock;
+ bool config_pending;
+ bool restoring_config;
+ bool autonomous;
};
+static void pulse8_ping_eeprom_work_handler(struct work_struct *work);
+
static void pulse8_irq_work_handler(struct work_struct *work)
{
struct pulse8 *pulse8 =
@@ -229,6 +242,7 @@ static void pulse8_disconnect(struct serio *serio)
struct pulse8 *pulse8 = serio_get_drvdata(serio);
cec_unregister_adapter(pulse8->adap);
+ cancel_delayed_work_sync(&pulse8->ping_eeprom_work);
dev_info(&serio->dev, "disconnected\n");
serio_close(serio);
serio_set_drvdata(serio, NULL);
@@ -311,14 +325,15 @@ unlock:
return err == -ENOTTY ? -EIO : err;
}
-static int pulse8_setup(struct pulse8 *pulse8, struct serio *serio)
+static int pulse8_setup(struct pulse8 *pulse8, struct serio *serio,
+ struct cec_log_addrs *log_addrs, u16 *pa)
{
u8 *data = pulse8->data + 1;
- unsigned int count = 0;
- unsigned int vers = 0;
u8 cmd[2];
int err;
+ pulse8->vers = 0;
+
cmd[0] = MSGCODE_PING;
err = pulse8_send_and_wait(pulse8, cmd, 1,
MSGCODE_COMMAND_ACCEPTED, 0);
@@ -328,10 +343,10 @@ static int pulse8_setup(struct pulse8 *pulse8, struct
serio *serio)
if (err)
return err;
- vers = (data[0] << 8) | data[1];
+ pulse8->vers = (data[0] << 8) | data[1];
- dev_info(pulse8->dev, "Firmware version %04x\n", vers);
- if (vers < 2)
+ dev_info(pulse8->dev, "Firmware version %04x\n", pulse8->vers);
+ if (pulse8->vers < 2)
return 0;
cmd[0] = MSGCODE_GET_BUILDDATE;
@@ -348,37 +363,103 @@ static int pulse8_setup(struct pulse8 *pulse8, struct
serio *serio)
tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
tm.tm_hour, tm.tm_min, tm.tm_sec);
}
+ if (err)
+ return err;
- do {
- if (count)
- msleep(500);
- cmd[0] = MSGCODE_SET_AUTO_ENABLED;
- cmd[1] = 0;
- err = pulse8_send_and_wait(pulse8, cmd, 2,
- MSGCODE_COMMAND_ACCEPTED, 1);
- if (err && count == 0) {
- dev_info(pulse8->dev, "No Auto Enabled supported\n");
- return 0;
- }
+ dev_dbg(pulse8->dev, "Persistent config:\n");
+ cmd[0] = MSGCODE_GET_AUTO_ENABLED;
+ err = pulse8_send_and_wait(pulse8, cmd, 1, cmd[0], 1);
+ if (err)
+ return err;
+ pulse8->autonomous = data[0];
+ dev_dbg(pulse8->dev, "Autonomous mode: %s",
+ data[0] ? "on" : "off");
- cmd[0] = MSGCODE_GET_AUTO_ENABLED;
- if (!err)
- err = pulse8_send_and_wait(pulse8, cmd,
[PATCH] vivid: update EDID
Update the vivid EDID, fixing various incorrect values (wrong name,
product code, various video capabilities).
Signed-off-by: Hans Verkuil
---
diff --git a/drivers/media/platform/vivid/vivid-core.c
b/drivers/media/platform/vivid/vivid-core.c
index 7f93713..b1df8cc 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -163,38 +163,38 @@ const struct v4l2_rect vivid_max_rect = {
static const u8 vivid_hdmi_edid[256] = {
0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00,
- 0x63, 0x3a, 0xaa, 0x55, 0x00, 0x00, 0x00, 0x00,
- 0x0a, 0x18, 0x01, 0x03, 0x80, 0x10, 0x09, 0x78,
- 0x0e, 0x00, 0xb2, 0xa0, 0x57, 0x49, 0x9b, 0x26,
- 0x10, 0x48, 0x4f, 0x2f, 0xcf, 0x00, 0x31, 0x59,
+ 0x31, 0xd8, 0x34, 0x12, 0x00, 0x00, 0x00, 0x00,
+ 0x22, 0x1a, 0x01, 0x03, 0x80, 0x60, 0x36, 0x78,
+ 0x1f, 0xee, 0x91, 0xa3, 0x54, 0x4c, 0x99, 0x26,
+ 0x0f, 0x50, 0x54, 0x2f, 0xcf, 0x00, 0x31, 0x59,
0x45, 0x59, 0x81, 0x80, 0x81, 0x40, 0x90, 0x40,
- 0x95, 0x00, 0xa9, 0x40, 0xb3, 0x00, 0x02, 0x3a,
- 0x80, 0x18, 0x71, 0x38, 0x2d, 0x40, 0x58, 0x2c,
- 0x46, 0x00, 0x10, 0x09, 0x00, 0x00, 0x00, 0x1e,
+ 0x95, 0x00, 0xa9, 0x40, 0xb3, 0x00, 0x08, 0xe8,
+ 0x00, 0x30, 0xf2, 0x70, 0x5a, 0x80, 0xb0, 0x58,
+ 0x8a, 0x00, 0xc0, 0x1c, 0x32, 0x00, 0x00, 0x1e,
0x00, 0x00, 0x00, 0xfd, 0x00, 0x18, 0x55, 0x18,
- 0x5e, 0x11, 0x00, 0x0a, 0x20, 0x20, 0x20, 0x20,
- 0x20, 0x20, 0x00, 0x00, 0x00, 0xfc, 0x00, 'v',
- '4', 'l', '2', '-', 'h', 'd', 'm', 'i',
- 0x0a, 0x0a, 0x0a, 0x0a, 0x00, 0x00, 0x00, 0x10,
+ 0x87, 0x3c, 0x00, 0x0a, 0x20, 0x20, 0x20, 0x20,
+ 0x20, 0x20, 0x00, 0x00, 0x00, 0xfc, 0x00, 0x76,
+ 0x69, 0x76, 0x69, 0x64, 0x0a, 0x20, 0x20, 0x20,
+ 0x20, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0x10,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0xf0,
-
- 0x02, 0x03, 0x1a, 0xc0, 0x48, 0xa2, 0x10, 0x04,
- 0x02, 0x01, 0x21, 0x14, 0x13, 0x23, 0x09, 0x07,
- 0x07, 0x65, 0x03, 0x0c, 0x00, 0x10, 0x00, 0xe2,
- 0x00, 0x2a, 0x01, 0x1d, 0x00, 0x80, 0x51, 0xd0,
- 0x1c, 0x20, 0x40, 0x80, 0x35, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x1e, 0x8c, 0x0a, 0xd0, 0x8a,
- 0x20, 0xe0, 0x2d, 0x10, 0x10, 0x3e, 0x96, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xd7
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x6b,
+
+ 0x02, 0x03, 0x3f, 0xf0, 0x51, 0x61, 0x60, 0x5f,
+ 0x5e, 0x5d, 0x10, 0x1f, 0x04, 0x13, 0x22, 0x21,
+ 0x20, 0x05, 0x14, 0x02, 0x11, 0x01, 0x23, 0x09,
+ 0x07, 0x07, 0x83, 0x01, 0x00, 0x00, 0x6d, 0x03,
+ 0x0c, 0x00, 0x10, 0x00, 0x00, 0x78, 0x21, 0x00,
+ 0x60, 0x01, 0x02, 0x03, 0x67, 0xd8, 0x5d, 0xc4,
+ 0x01, 0x78, 0x00, 0x00, 0xe2, 0x00, 0xea, 0xe3,
+ 0x05, 0x00, 0x00, 0xe3, 0x06, 0x01, 0x00, 0x4d,
+ 0xd0, 0x00, 0xa0, 0xf0, 0x70, 0x3e, 0x80, 0x30,
+ 0x20, 0x35, 0x00, 0xc0, 0x1c, 0x32, 0x00, 0x00,
+ 0x1e, 0x1a, 0x36, 0x80, 0xa0, 0x70, 0x38, 0x1f,
+ 0x40, 0x30, 0x20, 0x35, 0x00, 0xc0, 0x1c, 0x32,
+ 0x00, 0x00, 0x1a, 0x1a, 0x1d, 0x00, 0x80, 0x51,
+ 0xd0, 0x1c, 0x20, 0x40, 0x80, 0x35, 0x00, 0xc0,
+ 0x1c, 0x32, 0x00, 0x00, 0x1c, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x27,
};
static int vidioc_querycap(struct file *file, void *priv,
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RFC? [PATCH] docs-rst: kernel-doc: better output struct members
Am 21.08.2016 um 14:11 schrieb Mauro Carvalho Chehab :
> Right now, for a struct, kernel-doc produces the following output:
>
> .. c:type:: struct v4l2_prio_state
>
> stores the priority states
>
> **Definition**
>
> ::
>
> struct v4l2_prio_state {
> atomic_t prios[4];
> };
>
> **Members**
>
> ``atomic_t prios[4]``
> array with elements to store the array priorities
>
> Putting a member name in verbatim and adding a continuation line
> causes the LaTeX output to generate something like:
> item[atomic_t prios\[4\]] array with elements to store the array
> priorities
Right now, the description of C-struct members is a simple rest-definition-list
(not in the c-domain). It might be better to use the c-domain for members:
http://www.sphinx-doc.org/en/stable/domains.html#directive-c:member
But this is not the only thing we have to consider. To make a valid C-struct
description (with targets/references in the c-domain) we need a more
*structured* reST markup where the members are described in the block-content
of the struct directive. E.g:
---
|.. c:type:: struct v4l2_subdev_ir_ops
|
| operations for IR subdevices
|
| .. c:member:: int (* rx_read) (struct v4l2_subdev *sd, u8 *buf, size_t
count,ssize_t *num)
|
---
By this small example, you see, that we have to discuss the whole markup
produced by the kernel-doc script (function arguments, unions etc.).
IMHO, since kernel-doc is widely used, this should be a RFC.
-- Markus --
>
> Everything inside "item" is non-breakable, with may produce
> lines bigger than the column width.
>
> Also, for function members, like:
>
>int (* rx_read) (struct v4l2_subdev *sd, u8 *buf, size_t count,ssize_t
> *num);
>
> It puts the name of the member at the end, like:
>
>int (*) (struct v4l2_subdev *sd, u8 *buf, size_t count,ssize_t *num)
> read
>
> With is very confusing.
>
> The best is to highlight what really matters: the member name; the type
> is a secondary information.
>
> So, change kernel-doc, for it to produce the output on a different way:
>
> **Members**
>
> ``prios[4]``
> - **type**: ``atomic_t``
>
> array with elements to store the array priorities
>
> With such change, the name of the member will be the first visible
> thing, and will be in bold style. The type will still be there, inside
> a list.
>
> Also, as the type is not part of LaTeX "item[]", LaTeX will split it into
> multiple lines, if needed.
>
> So, both LaTeX/PDF and HTML outputs will look good.
>
> It should be noticed, however, that the way Sphinx LaTeX output handles
> things like:
>
> Foo
> bar
>
> is different than the HTML output. On HTML, it will produce something
> like:
>
> **Foo**
> bar
>
> While, on LaTeX, it puts both foo and bar at the same line, like:
>
> **Foo** bar
>
> By starting the second line with a dash, both HTML and LaTeX output
> will do the same thing.
>
> Signed-off-by: Mauro Carvalho Chehab
> ---
> scripts/kernel-doc | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index ba081c7636a2..78e355281e1a 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -2000,7 +2000,8 @@ sub output_struct_rst(%) {
> ($args{'parameterdescs'}{$parameter_name} ne $undescribed) || next;
> $type = $args{'parametertypes'}{$parameter};
> print_lineno($parameterdesc_start_lines{$parameter_name});
> - print "``$type $parameter``\n";
> + print "``" . $parameter . "``\n";
> + print " - **type**: ``$type``\n\n";
> output_highlight_rst($args{'parameterdescs'}{$parameter_name});
> print "\n";
> }
> --
> 2.7.4
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] cobalt: update EDID
Update the cobalt EDID, fixing various incorrect values (wrong name,
product code, various video capabilities).
Signed-off-by: Hans Verkuil
---
diff --git a/drivers/media/pci/cobalt/cobalt-driver.c
b/drivers/media/pci/cobalt/cobalt-driver.c
index 476f7f0..2475553 100644
--- a/drivers/media/pci/cobalt/cobalt-driver.c
+++ b/drivers/media/pci/cobalt/cobalt-driver.c
@@ -60,30 +60,31 @@ MODULE_DESCRIPTION("cobalt driver");
MODULE_LICENSE("GPL");
static u8 edid[256] = {
- 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00,
- 0x50, 0x21, 0x9C, 0x27, 0x00, 0x00, 0x00, 0x00,
- 0x19, 0x12, 0x01, 0x03, 0x80, 0x00, 0x00, 0x78,
- 0x0E, 0x00, 0xB2, 0xA0, 0x57, 0x49, 0x9B, 0x26,
- 0x10, 0x48, 0x4F, 0x2F, 0xCF, 0x00, 0x31, 0x59,
+ 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00,
+ 0x50, 0x21, 0x32, 0x27, 0x00, 0x00, 0x00, 0x00,
+ 0x22, 0x1a, 0x01, 0x03, 0x80, 0x30, 0x1b, 0x78,
+ 0x1f, 0xee, 0x91, 0xa3, 0x54, 0x4c, 0x99, 0x26,
+ 0x0f, 0x50, 0x54, 0x2f, 0xcf, 0x00, 0x31, 0x59,
0x45, 0x59, 0x61, 0x59, 0x81, 0x99, 0x01, 0x01,
- 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x02, 0x3A,
- 0x80, 0x18, 0x71, 0x38, 0x2D, 0x40, 0x58, 0x2C,
- 0x46, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1E,
- 0x00, 0x00, 0x00, 0xFD, 0x00, 0x31, 0x55, 0x18,
- 0x5E, 0x11, 0x00, 0x0A, 0x20, 0x20, 0x20, 0x20,
- 0x20, 0x20, 0x00, 0x00, 0x00, 0xFC, 0x00, 0x43,
- 0x20, 0x39, 0x30, 0x0A, 0x0A, 0x0A, 0x0A, 0x0A,
- 0x0A, 0x0A, 0x0A, 0x0A, 0x00, 0x00, 0x00, 0x10,
+ 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x02, 0x3a,
+ 0x80, 0x18, 0x71, 0x38, 0x2d, 0x40, 0x58, 0x2c,
+ 0x46, 0x00, 0xe0, 0x0e, 0x11, 0x00, 0x00, 0x1e,
+ 0x00, 0x00, 0x00, 0xfd, 0x00, 0x18, 0x55, 0x18,
+ 0x5e, 0x11, 0x00, 0x0a, 0x20, 0x20, 0x20, 0x20,
+ 0x20, 0x20, 0x00, 0x00, 0x00, 0xfc, 0x00, 0x63,
+ 0x6f, 0x62, 0x61, 0x6c, 0x74, 0x0a, 0x20, 0x20,
+ 0x20, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0x10,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x8c,
+
+ 0x02, 0x03, 0x1f, 0xf0, 0x4a, 0x90, 0x1f, 0x04,
+ 0x13, 0x22, 0x21, 0x20, 0x02, 0x11, 0x01, 0x23,
+ 0x09, 0x07, 0x07, 0x68, 0x03, 0x0c, 0x00, 0x10,
+ 0x00, 0x00, 0x22, 0x0f, 0xe2, 0x00, 0xea, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x68,
- 0x02, 0x03, 0x1a, 0xc0, 0x48, 0xa2, 0x10, 0x04,
- 0x02, 0x01, 0x21, 0x14, 0x13, 0x23, 0x09, 0x07,
- 0x07, 0x65, 0x03, 0x0c, 0x00, 0x10, 0x00, 0xe2,
- 0x00, 0x2a, 0x01, 0x1d, 0x00, 0x80, 0x51, 0xd0,
- 0x1c, 0x20, 0x40, 0x80, 0x35, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x1e, 0x8c, 0x0a, 0xd0, 0x8a,
- 0x20, 0xe0, 0x2d, 0x10, 0x10, 0x3e, 0x96, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
@@ -91,7 +92,7 @@ static u8 edid[256] = {
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xd7
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xa7,
};
static void cobalt_set_interrupt(struct cobalt *cobalt, bool enable)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pwc over musb: 100% frame drop (lost) on high resolution stream
2016-08-22 1:00 GMT+03:00 Alan Stern : > On Sun, 21 Aug 2016, Matwey V. Kornilov wrote: > >> In both cases (with or without HCD_BH), usb_hcd_giveback_urb is called >> every 0.01 sec. It is not clear why behavior is so different. > > What behavior are you asking about? The difference between HCD_BH set > and not set? > The difference between HCD_BH set and not set is that when it is not set then usb_hcd_giveback_urb() receive zero-length URBs. And this breaks my pwc webcam. And the question is how to fix it. As far as I can see, usb_hcd_giveback_urb is being called with the same rate in both cases, so zero-length URBs are probably supposed to be data-carrying. > Alan Stern > -- With best regards, Matwey V. Kornilov. Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia 119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] pulse8-cec: sync configuration with adapter
On 08/19/2016 06:36 PM, Johan Fjeldtvedt wrote:
> When the configuration is changed, they are also written to the adapter.
> This allows the adapter to continue operating in autonomous mode with
> the same settings when it is disconnected from the driver (typically by
> going into suspend). For adapters with firmware version 2 or greater, the
> settings are also persisted in EEPROM.
>
> A new module parameter is added to optionally also use the configuration
> already present in the adapter when it is connected. This option is
> enabled by default.
>
> When a new configuration is written, the autonomous mode is
> automatically enabled. When the device is unconfigured, autonomous mode
> is disabled.
>
> Signed-off-by: Johan Fjeldtvedt
> ---
> drivers/staging/media/pulse8-cec/pulse8-cec.c | 259
> ++
> 1 file changed, 221 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/staging/media/pulse8-cec/pulse8-cec.c
> b/drivers/staging/media/pulse8-cec/pulse8-cec.c
> index 4d20e72..531377a 100644
> --- a/drivers/staging/media/pulse8-cec/pulse8-cec.c
> +++ b/drivers/staging/media/pulse8-cec/pulse8-cec.c
> @@ -51,8 +51,11 @@ MODULE_DESCRIPTION("Pulse Eight HDMI CEC driver");
> MODULE_LICENSE("GPL");
>
> static int debug;
> +static int persistent_config = 1;
> module_param(debug, int, 0644);
> +module_param(persistent_config, int, 0644);
> MODULE_PARM_DESC(debug, "debug level (0-1)");
> +MODULE_PARM_DESC(persitent_config, "read config from persitent memory
> (0-1)");
s/persitent/persistent/g
>
> enum pulse8_msgcodes {
> MSGCODE_NOTHING = 0,
> @@ -109,12 +112,16 @@ enum pulse8_msgcodes {
>
> #define DATA_SIZE 256
>
> +#define PING_PERIOD 15 * HZ
> +
> struct pulse8 {
> struct device *dev;
> struct serio *serio;
> struct cec_adapter *adap;
> + unsigned int vers;
> struct completion cmd_done;
> struct work_struct work;
> + struct delayed_work ping_eeprom_work;
> struct cec_msg rx_msg;
> u8 data[DATA_SIZE];
> unsigned int len;
> @@ -122,9 +129,15 @@ struct pulse8 {
> unsigned int idx;
> bool escape;
> bool started;
> + struct mutex config_lock;
> struct mutex write_lock;
> + bool config_pending;
> + bool restoring_config;
> + bool autonomous;
> };
>
> +static void pulse8_ping_eeprom_work_handler(struct work_struct *work);
> +
> static void pulse8_irq_work_handler(struct work_struct *work)
> {
> struct pulse8 *pulse8 =
> @@ -229,6 +242,7 @@ static void pulse8_disconnect(struct serio *serio)
> struct pulse8 *pulse8 = serio_get_drvdata(serio);
>
> cec_unregister_adapter(pulse8->adap);
> + cancel_delayed_work_sync(&pulse8->ping_eeprom_work);
> dev_info(&serio->dev, "disconnected\n");
> serio_close(serio);
> serio_set_drvdata(serio, NULL);
> @@ -309,14 +323,14 @@ static int pulse8_send_and_wait(struct pulse8 *pulse8,
> return err;
> }
>
> -static int pulse8_setup(struct pulse8 *pulse8, struct serio *serio)
> +static int pulse8_setup(struct pulse8 *pulse8, struct serio *serio, struct
> cec_log_addrs *log_addrs, u16 *pa)
> {
> u8 *data = pulse8->data + 1;
> - unsigned int count = 0;
> - unsigned int vers = 0;
> u8 cmd[2];
> int err;
>
> + pulse8->vers = 0;
> +
> cmd[0] = MSGCODE_PING;
> err = pulse8_send_and_wait(pulse8, cmd, 1,
> MSGCODE_COMMAND_ACCEPTED, 0);
> @@ -326,10 +340,10 @@ static int pulse8_setup(struct pulse8 *pulse8, struct
> serio *serio)
> if (err)
> return err;
>
> - vers = (data[0] << 8) | data[1];
> + pulse8->vers = (data[0] << 8) | data[1];
>
> - dev_info(pulse8->dev, "Firmware version %04x\n", vers);
> - if (vers < 2)
> + dev_info(pulse8->dev, "Firmware version %04x\n", pulse8->vers);
> + if (pulse8->vers < 2)
> return 0;
>
> cmd[0] = MSGCODE_GET_BUILDDATE;
> @@ -346,37 +360,98 @@ static int pulse8_setup(struct pulse8 *pulse8, struct
> serio *serio)
>tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
>tm.tm_hour, tm.tm_min, tm.tm_sec);
> }
> + if (err)
> + return err;
>
> - do {
> - if (count)
> - msleep(500);
> - cmd[0] = MSGCODE_SET_AUTO_ENABLED;
> - cmd[1] = 0;
> - err = pulse8_send_and_wait(pulse8, cmd, 2,
> -MSGCODE_COMMAND_ACCEPTED, 1);
> - if (err && count == 0) {
> - dev_info(pulse8->dev, "No Auto Enabled supported\n");
> - return 0;
> - }
> + dev_dbg(pulse8->dev, "Persistent config:\n");
> + cmd[0] = MSGCODE_GET_AUTO_ENABLED;
> + err = pulse8_send_and_wait(pulse8, cmd, 1, cmd[0], 1);
> + if (err)
> + return err;
> + pulse8->autonomous = data[0];
> + dev_dbg(pulse8->dev
Re: [PATCH 2/4] pulse8-cec: serialize communication with adapter
On 08/19/2016 06:36 PM, Johan Fjeldtvedt wrote:
> Make sending messages to the adapter serialized within the driver.
>
> send_and_wait is split into send_and_wait_once, which only sends once
> and checks for the result, and the higher level send_and_wait, which
> performs locking and retries.
>
> Signed-off-by: Johan Fjeldtvedt
> ---
> drivers/staging/media/pulse8-cec/pulse8-cec.c | 50
> ---
> 1 file changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/staging/media/pulse8-cec/pulse8-cec.c
> b/drivers/staging/media/pulse8-cec/pulse8-cec.c
> index ed8bd95..fdb2407 100644
> --- a/drivers/staging/media/pulse8-cec/pulse8-cec.c
> +++ b/drivers/staging/media/pulse8-cec/pulse8-cec.c
> @@ -99,6 +99,7 @@ struct pulse8 {
> unsigned int idx;
> bool escape;
> bool started;
> + struct mutex write_lock;
> };
>
> static void pulse8_irq_work_handler(struct work_struct *work)
> @@ -233,8 +234,8 @@ static int pulse8_send(struct serio *serio, const u8
> *command, u8 cmd_len)
> return err;
> }
>
> -static int pulse8_send_and_wait(struct pulse8 *pulse8,
> - const u8 *cmd, u8 cmd_len, u8 response, u8 size)
> +static int pulse8_send_and_wait_once(struct pulse8 *pulse8,
> + const u8 *cmd, u8 cmd_len, u8 response, u8
> size)
> {
> int err;
>
> @@ -250,24 +251,8 @@ static int pulse8_send_and_wait(struct pulse8 *pulse8,
> if ((pulse8->data[0] & 0x3f) == MSGCODE_COMMAND_REJECTED &&
> cmd[0] != MSGCODE_SET_CONTROLLED &&
> cmd[0] != MSGCODE_SET_AUTO_ENABLED &&
> - cmd[0] != MSGCODE_GET_BUILDDATE) {
> - u8 cmd_sc[2];
> -
> - cmd_sc[0] = MSGCODE_SET_CONTROLLED;
> - cmd_sc[1] = 1;
> - err = pulse8_send_and_wait(pulse8, cmd_sc, 2,
> -MSGCODE_COMMAND_ACCEPTED, 1);
> - if (err)
> - return err;
> - init_completion(&pulse8->cmd_done);
> -
> - err = pulse8_send(pulse8->serio, cmd, cmd_len);
> - if (err)
> - return err;
> -
> - if (!wait_for_completion_timeout(&pulse8->cmd_done, HZ))
> - return -ETIMEDOUT;
> - }
> + cmd[0] != MSGCODE_GET_BUILDDATE)
> + return -ENOTTY;
> if (response &&
> ((pulse8->data[0] & 0x3f) != response || pulse8->len < size + 1)) {
> dev_info(pulse8->dev, "transmit: failed %02x\n",
> @@ -277,6 +262,30 @@ static int pulse8_send_and_wait(struct pulse8 *pulse8,
> return 0;
> }
>
> +static int pulse8_send_and_wait(struct pulse8 *pulse8,
> + const u8 *cmd, u8 cmd_len, u8 response, u8 size)
> +{
> + u8 cmd_sc[2];
> + int err;
> +
> + mutex_lock(&pulse8->write_lock);
> + err = pulse8_send_and_wait_once(pulse8, cmd, cmd_len, response, size);
> +
> + if (err == -ENOTTY) {
> + cmd_sc[0] = MSGCODE_SET_CONTROLLED;
> + cmd_sc[1] = 1;
> + err = pulse8_send_and_wait_once(pulse8, cmd_sc, 2,
> + MSGCODE_COMMAND_ACCEPTED, 1);
> + if (err)
> + goto unlock;
> + err = pulse8_send_and_wait_once(pulse8, cmd, cmd_len, response,
> size);
> + }
> +
> + unlock:
> + mutex_unlock(&pulse8->write_lock);
> + return err;
This should be:
return err == -ENOTTY ? -EIO : err;
We don't want the ENOTTY to end up in userspace.
Regards,
Hans
> +}
> +
> static int pulse8_setup(struct pulse8 *pulse8, struct serio *serio)
> {
> u8 *data = pulse8->data + 1;
> @@ -453,6 +462,7 @@ static int pulse8_connect(struct serio *serio, struct
> serio_driver *drv)
> pulse8->dev = &serio->dev;
> serio_set_drvdata(serio, pulse8);
> INIT_WORK(&pulse8->work, pulse8_irq_work_handler);
> + mutex_init(&pulse8->write_lock);
>
> err = serio_open(serio, drv);
> if (err)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
