[PATCH for v4.2] cobalt: set Kconfig default to n
Since this board is a Cisco-internal board there is no point in having it compiled by default. So set the Kconfig default to n. Signed-off-by: Hans Verkuil diff --git a/drivers/media/pci/cobalt/Kconfig b/drivers/media/pci/cobalt/Kconfig index 3be1b2c..4d0777a 100644 --- a/drivers/media/pci/cobalt/Kconfig +++ b/drivers/media/pci/cobalt/Kconfig @@ -7,6 +7,7 @@ config VIDEO_COBALT select VIDEO_ADV7511 select VIDEO_ADV7842 select VIDEOBUF2_DMA_SG + default n ---help--- This is a video4linux driver for the Cisco PCIe Cobalt card. -- 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 v9 2/8] media: Add registration helpers for V4L2 flash sub-devices
Hi Alexey,
On 06/18/2015 07:45 PM, Alexey Klimov wrote:
Hi Jacek,
On Mon, May 25, 2015 at 6:13 PM, Jacek Anaszewski
wrote:
This patch adds helper functions for registering/unregistering
LED Flash class devices as V4L2 sub-devices. The functions should
be called from the LED subsystem device driver. In case the
support for V4L2 Flash sub-devices is disabled in the kernel
config the functions' empty versions will be used.
Signed-off-by: Jacek Anaszewski
Acked-by: Kyungmin Park
Cc: Sakari Ailus
Cc: Hans Verkuil
---
drivers/media/v4l2-core/Kconfig| 11 +
drivers/media/v4l2-core/Makefile |2 +
drivers/media/v4l2-core/v4l2-flash-led-class.c | 671
include/media/v4l2-flash-led-class.h | 148 ++
4 files changed, 832 insertions(+)
create mode 100644 drivers/media/v4l2-core/v4l2-flash-led-class.c
create mode 100644 include/media/v4l2-flash-led-class.h
[...]
+struct v4l2_flash *v4l2_flash_init(
+ struct device *dev, struct device_node *of_node,
+ struct led_classdev_flash *fled_cdev,
+ struct led_classdev_flash *iled_cdev,
+ const struct v4l2_flash_ops *ops,
+ struct v4l2_flash_config *config)
+{
+ struct v4l2_flash *v4l2_flash;
+ struct led_classdev *led_cdev = &fled_cdev->led_cdev;
+ struct v4l2_subdev *sd;
+ int ret;
+
+ if (!fled_cdev || !ops || !config)
+ return ERR_PTR(-EINVAL);
Could you please if it is correct? You're checking fled_cdev but four
lines above you're using fled_cdev and taking led_cdev pointer from
there. Maybe it's better to move calculation of led_cdev down and
place after if-check?
Thanks for spotting this. I'll submit fixed version soon.
--
Best Regards,
Jacek Anaszewski
--
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 2/2] v4l2-utils: extend set-dv-timing options for RB version
On 06/16/2015 11:30 AM, Prashant Laddha wrote:
> To support the timings calculations for reduced blanking version 2
> (RB v2), extended the command line options to include flag indicating
> whether to use RB V2 or not. Updated the command usage for the same.
>
> Cc: Hans Verkuil
> Signed-off-by: Prashant Laddha
> ---
> utils/v4l2-ctl/v4l2-ctl-stds.cpp | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/utils/v4l2-ctl/v4l2-ctl-stds.cpp
> b/utils/v4l2-ctl/v4l2-ctl-stds.cpp
> index c0e919b..9734c80 100644
> --- a/utils/v4l2-ctl/v4l2-ctl-stds.cpp
> +++ b/utils/v4l2-ctl/v4l2-ctl-stds.cpp
> @@ -41,7 +41,10 @@ void stds_usage(void)
> " index=: use the index as provided
> by --list-dv-timings\n"
> " or specify timings using cvt/gtf options
> as follows:\n"
> "
> cvt/gtf,width=,height=,fps=\n"
> -" interlaced=<0/1>,reduced-blanking=<0/1>\n"
> +"
> interlaced=<0/1>,reduced-blanking=<0/1>,use-rb-v2=<0/1>\n"
> +" use-rb-v2 indicates whether to use reduced
> blanking version 2\n"
> +" or not. This flag is relevant only for cvt
> timings and has\n"
> +" effect only if reduced-blanking=1\n"
Why not just allow a value of 2 for the reduced-blanking argument instead
of introducing a new argument? For gtf 1 and 2 mean the same thing, for cvt 1
will use the standard RB and 2 RBv2.
Seems simpler to me. It also means that calc_cvt_modeline doesn't need a new
argument, just that bool reduced_blanking becomes int reduced_blanking.
Other than this it looks good to me.
Regards,
Hans
> " or give a fully specified timings:\n"
> "
> width=,height=,interlaced=<0/1>,\n"
> " polarities= mask>,pixelclock=,\n"
> @@ -148,6 +151,7 @@ enum timing_opts {
> GTF,
> FPS,
> REDUCED_BLANK,
> + USE_RB_V2,
> };
>
> static int parse_timing_subopt(char **subopt_str, int *value)
> @@ -175,6 +179,7 @@ static int parse_timing_subopt(char **subopt_str, int
> *value)
> "gtf",
> "fps",
> "reduced-blanking",
> + "use-rb-v2",
> NULL
> };
>
> @@ -205,6 +210,7 @@ static void get_cvt_gtf_timings(char *subopt, int
> standard,
> int fps = 0;
> int r_blank = 0;
> int interlaced = 0;
> + int use_rb_v2 = 0;
>
> bool timings_valid = false;
>
> @@ -231,6 +237,8 @@ static void get_cvt_gtf_timings(char *subopt, int
> standard,
> case INTERLACED:
> interlaced = opt_val;
> break;
> + case USE_RB_V2:
> + use_rb_v2 = opt_val;
> default:
> break;
> }
> @@ -240,7 +248,8 @@ static void get_cvt_gtf_timings(char *subopt, int
> standard,
> timings_valid = calc_cvt_modeline(width, height, fps,
> r_blank == 1 ? true : false,
> interlaced == 1 ? true : false,
> - false, bt);
> + use_rb_v2 == 1 ? true : false,
> + bt);
> } else {
> timings_valid = calc_gtf_modeline(width, height, fps,
> r_blank == 1 ? true : false,
>
--
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: OK
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: Fri Jun 19 04:00:21 CEST 2015 git branch: test git hash: 6f32a8c97f11eb074027052b6b507891e5c9d8b1 gcc version:i686-linux-gcc (GCC) 5.1.0 sparse version: v0.5.0-44-g40791b9 smatch version: 0.4.1-3153-g7d56ab3 host hardware: x86_64 host os:4.0.0-3.slh.1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK 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: OK linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK 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-rc1-i686: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: 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-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS smatch: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.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
[linuxtv-media:media-controller 1243/1278] drivers/media/dvb-frontends/ts2020.c:60:2: error: implicit declaration of function 'i2c_unregister_device'
tree: git://linuxtv.org/media_tree.git media-controller
head: 027cf164d8f3a80c2127cc92e4620e04e883e829
commit: f158cbceb165f318a89a8bb831dc3ea27823b3f8 [1243/1278] [media] ts2020:
convert to regmap I2C API
config: x86_64-randconfig-r0-06190935 (attached as .config)
reproduce:
git checkout f158cbceb165f318a89a8bb831dc3ea27823b3f8
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
warning: (DVB_TS2020 && SND_SOC_ADAU1761_I2C && SND_SOC_ADAU1781_I2C &&
SND_SOC_ADAU1977_I2C && SND_SOC_RT5677 && EXTCON_MAX14577 && EXTCON_MAX77693 &&
EXTCON_MAX77843) selects REGMAP_I2C which has unmet direct dependencies (I2C)
drivers/base/regmap/regmap-i2c.c: In function 'regmap_smbus_byte_reg_read':
drivers/base/regmap/regmap-i2c.c:29:2: error: implicit declaration of
function 'i2c_smbus_read_byte_data' [-Werror=implicit-function-declaration]
ret = i2c_smbus_read_byte_data(i2c, reg);
^
drivers/base/regmap/regmap-i2c.c: In function 'regmap_smbus_byte_reg_write':
drivers/base/regmap/regmap-i2c.c:47:2: error: implicit declaration of
function 'i2c_smbus_write_byte_data' [-Werror=implicit-function-declaration]
return i2c_smbus_write_byte_data(i2c, reg, val);
^
drivers/base/regmap/regmap-i2c.c: In function 'regmap_smbus_word_reg_read':
drivers/base/regmap/regmap-i2c.c:65:2: error: implicit declaration of
function 'i2c_smbus_read_word_data' [-Werror=implicit-function-declaration]
ret = i2c_smbus_read_word_data(i2c, reg);
^
drivers/base/regmap/regmap-i2c.c: In function 'regmap_smbus_word_reg_write':
drivers/base/regmap/regmap-i2c.c:83:2: error: implicit declaration of
function 'i2c_smbus_write_word_data' [-Werror=implicit-function-declaration]
return i2c_smbus_write_word_data(i2c, reg, val);
^
drivers/base/regmap/regmap-i2c.c: In function
'regmap_smbus_word_read_swapped':
>> drivers/base/regmap/regmap-i2c.c:101:2: error: implicit declaration of
>> function 'i2c_smbus_read_word_swapped'
>> [-Werror=implicit-function-declaration]
ret = i2c_smbus_read_word_swapped(i2c, reg);
^
drivers/base/regmap/regmap-i2c.c: In function
'regmap_smbus_word_write_swapped':
>> drivers/base/regmap/regmap-i2c.c:119:2: error: implicit declaration of
>> function 'i2c_smbus_write_word_swapped'
>> [-Werror=implicit-function-declaration]
return i2c_smbus_write_word_swapped(i2c, reg, val);
^
drivers/base/regmap/regmap-i2c.c: In function 'regmap_i2c_write':
drivers/base/regmap/regmap-i2c.c:133:2: error: implicit declaration of
function 'i2c_master_send' [-Werror=implicit-function-declaration]
ret = i2c_master_send(i2c, data, count);
^
drivers/base/regmap/regmap-i2c.c: In function 'regmap_i2c_gather_write':
drivers/base/regmap/regmap-i2c.c:154:2: error: implicit declaration of
function 'i2c_check_functionality' [-Werror=implicit-function-declaration]
if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_NOSTART))
^
drivers/base/regmap/regmap-i2c.c:167:2: error: implicit declaration of
function 'i2c_transfer' [-Werror=implicit-function-declaration]
ret = i2c_transfer(i2c->adapter, xfer, 2);
^
cc1: some warnings being treated as errors
--
drivers/media/dvb-frontends/ts2020.c: In function 'ts2020_release':
>> drivers/media/dvb-frontends/ts2020.c:60:2: error: implicit declaration of
>> function 'i2c_unregister_device' [-Werror=implicit-function-declaration]
i2c_unregister_device(client);
^
drivers/media/dvb-frontends/ts2020.c: In function 'ts2020_attach':
>> drivers/media/dvb-frontends/ts2020.c:376:2: error: implicit declaration of
>> function 'i2c_new_device' [-Werror=implicit-function-declaration]
client = i2c_new_device(i2c, &board_info);
^
>> drivers/media/dvb-frontends/ts2020.c:376:9: warning: assignment makes
>> pointer from integer without a cast
client = i2c_new_device(i2c, &board_info);
^
drivers/media/dvb-frontends/ts2020.c: At top level:
>> drivers/media/dvb-frontends/ts2020.c:577:1: warning: data definition has no
>> type or storage class
module_i2c_driver(ts2020_driver);
^
>> drivers/media/dvb-frontends/ts2020.c:577:1: error: type defaults to 'int' in
>> declaration of 'module_i2c_driver' [-Werror=implicit-int]
>> drivers/media/dvb-frontends/ts2020.c:577:1: warning: parameter names
>> (without types) in function declaration
>> drivers/media/dvb-frontends/ts2020.c:567:26: warning: 'ts2020_driver'
>> defined but not used [-Wunused-variable]
static struct i2c_driver ts2020_driver = {
^
cc1: some warnings being treated as errors
vim +/i2c_unregister_device +60 drivers/media/dvb-frontends/ts2020.c
b858c331 Igor M. Liplianin 2012-12-28 54 {
e6ad9ce3 Antti Palosaari 2015-03-26 55 struct ts2020_priv *priv =
fe->tuner_priv;
e6ad9ce3 Antti Palosaari 2015-03-26 56 struct i2c_client *client =
priv->client;
e6ad9ce3 Antti Palosaa
[linuxtv-media:media-controller 1277/1278] drivers/media/usb/au0828/au0828-video.c:674:7: warning: 'entity' may be used uninitialized in this function
tree: git://linuxtv.org/media_tree.git media-controller
head: 027cf164d8f3a80c2127cc92e4620e04e883e829
commit: 61891e24706322835582f8b6b184be9adbf78427 [1277/1278] [media] au0828:
Cache the decoder info at au0828 dev structure
config: blackfin-allyesconfig (attached as .config)
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 61891e24706322835582f8b6b184be9adbf78427
# save the attached .config to linux build tree
make.cross ARCH=blackfin
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/media/usb/au0828/au0828-video.c: In function
'au0828_enable_analog_tuner':
>> drivers/media/usb/au0828/au0828-video.c:674:7: warning: 'entity' may be used
>> uninitialized in this function [-Wuninitialized]
vim +/entity +674 drivers/media/usb/au0828/au0828-video.c
61891e24 Mauro Carvalho Chehab 2015-06-18 658 for (i = 0; i <
dev->decoder->num_links; i++) {
61891e24 Mauro Carvalho Chehab 2015-06-18 659 link =
&dev->decoder->links[i];
61891e24 Mauro Carvalho Chehab 2015-06-18 660 if
(link->sink->entity == dev->decoder) {
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 661
found_link = link;
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 662
if (link->flags & MEDIA_LNK_FL_ENABLED)
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 663
active_links++;
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 664
break;
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 665 }
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 666 }
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 667
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 668 if
(active_links == 1 || !found_link)
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 669 return
0;
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 670
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 671 source =
found_link->source->entity;
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 672 for (i = 0; i <
source->num_links; i++) {
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 673 struct
media_entity *sink;
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 @674 int
flags = 0;
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 675
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 676 link =
&source->links[i];
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 677 sink =
link->sink->entity;
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 678
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 679 if
(sink == entity)
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 680
flags = MEDIA_LNK_FL_ENABLED;
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 681
1d8ab6f0 Rafael Lourenço de Lima Chehab 2015-06-08 682 ret =
media_entity_setup_link(link, flags);
:: The code at line 674 was first introduced by commit
:: 1d8ab6f0a0dfd8f976a51b677bb5ba13dae01b4f [media] au0828: Add support for
media controller
:: TO: Rafael Lourenço de Lima Chehab
:: CC: Mauro Carvalho Chehab
---
0-DAY kernel test infrastructureOpen Source Technology Center
http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
#
# Automatically generated file; DO NOT EDIT.
# Linux/blackfin 4.1.0-rc3 Kernel Configuration
#
# CONFIG_MMU is not set
# CONFIG_FPU is not set
CONFIG_RWSEM_GENERIC_SPINLOCK=y
# CONFIG_RWSEM_XCHGADD_ALGORITHM is not set
CONFIG_BLACKFIN=y
CONFIG_GENERIC_CSUM=y
CONFIG_GENERIC_BUG=y
CONFIG_ZONE_DMA=y
CONFIG_FORCE_MAX_ZONEORDER=14
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_TRACE_IRQFLAGS_SUPPORT=y
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_CONSTRUCTORS=y
CONFIG_IRQ_WORK=y
#
# General setup
#
CONFIG_BROKEN_ON_SMP=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
CONFIG_COMPILE_TEST=y
CONFIG_LOCALVERSION=""
CONFIG_LOCALVERSION_AUTO=y
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_KERNEL_GZIP=y
# CONFIG_KERNEL_BZIP2 is not set
# CONFIG_KERNEL_LZMA is not set
# CONFIG_KERNEL_LZO is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_FHANDLE=y
CONFIG_USELIB=y
CONFIG_AUDIT=y
#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_IRQ_CHIP=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_DOMAIN_DEBUG=y
CONFIG_GENERIC_CLOCKEVENTS=y
#
# Timers
Re: [PATCH v3 1/7] rc: rc-ir-raw: Add scancode encoder callback
Em Sun, 14 Jun 2015 01:44:54 +0200 David Härdeman escreveu: > On 2015-05-23 13:34, Antti Seppälä wrote: > > On 22 May 2015 at 13:33, David Härdeman wrote: > >> On 2015-05-22 07:27, Antti Seppälä wrote: > >>> I think that approach too is far from perfect as it leaves us with > >>> questions such as: How do we let the user know what variant of > >>> protocol the label "rc-6" really means? If in nvt we hardcode it to > >>> mean RC6-0-16 and a new driver cames along which chooses > >>> RC_TYPE_RC6_6A_24 how do we tell the user that the implementations > >>> differ? What if the scancode that was fed was really RC_TYPE_RC6_MCE? > >> > >> > >> I never claimed it was perfect. > >> > >> For another (short-term, per-driver) solution, see the winbond-cir > >> driver. > >> > > > > Heh, funny you should mention that... Back in late 2013/early 2014 I > > submitted a patch for nuvoton which was modeled after winbond-cir. The > > feedback from that could be summarized as: > > - Scancodes should be used instead of raw pulse/spaces (the initial > > version of the patch worked without encoding) > > - Encoders should be generalized for others to use them too > > - Sysfs -api should be used instead of module parameters > > > > So the suggestions were a pretty much the exact opposite of what > > winbond-cir does. > > Again, it was a short-term suggestion. A long-term "real" solution > requires a definitive identification of the intended protocol. > > One idea that I've had in the back of my head for a long time is to use > the "flags" member of "struct rc_keymap_entry" in the new > EVIOC[GS]KEYCODE_V2 ioctl variant (see > http://www.spinics.net/lists/linux-media/msg88452.html). > > If a RC_POWERON flag was defined, it could be used for that purpose... > > >> It's entirely broken in the sense that a user has no idea of knowing > >> if the > >> hardware has been properly configured to wake the computer or not. > >> It's just > >> as broken as the connect() system call would be if it randomly rewrote > >> the > >> destination address passed by the user, optionally connected, and most > >> of > >> the time returned zero independently of the result. > >> > > > > I think you may be misunderstanding the sysfs api or at least the > > connect() analogue here as well as the userspace-kernelspace example > > above are actually not how the api works. Remember that userspace does > > not know about the protocol variants. > > Userspace most definately does. Keymaps are a good example of that. > Distributing keymaps for a certain remote should be possible. > > > Let me try to use your example and work-out how it really goes: > > * Userspace: please set the hardware to wake up if the scancode > > 0x800f040c is received. I know this is RC6 scancode because it came > > from the rc-6 decoder but I don't know the variant (and I don't really > > care) > > First of all...you assume that the only way of generating a valid wakeup > scancode is by using the same remote on the same hardware first to see > what it generates (which - thanks to things like boneheaded decisions on > NEC scancode generation and the previously mentioned heuristics - may > differ). So distributing a keymap from a central repository or just > looking up scancodes from a vendor datasheet is no longer feasible with > this API. > > Second, you have absolutely nothing that ensures that the same > heuristics are used in the decode/encode code paths and that the > heuristics will remain in sync. > > > * Kernel: Ok (btw. based on the length of the scancode and the > > bit-pattern in the front I've determined this to be rc6-mce type > > scancode and encoded it accordingly) > > * Userspace: Got it > > The whole "btw" part won't be passed to userspace...so there's nothing > to "get" > > > So no changing anything behind users back or not leading to > > misconfigured hardware AFAICT. > ... > > I'm sorry that the encoding functionality clashes with your intentions > > of improving the rc-core. I guess Mauro likes encoders more than > > improving rc-core fundamentals :) > > Kidding aside the fact that this series got merged might suggest that > > you and Mauro don't necessarily share the same views about the future > > and possible api breaks of rc-core. > > If you've followed the development of rc-core during the last few years > it should be pretty clear that Mauro has little to no long-term plan, > understanding of the current issues or willingness to fix them. I > wouldn't read too much into the fact that the code was merged. You completely missed the point. Adding new drivers and new features don't require much time to review, and don't require testing. What you're trying to send as "fixes" is actually series of patches that change the behavior of the subsystem, with would cause regressions. Btw, a lot of the pull requests you've sent over the past years did cause regressions. So, I can only review/apply them when I have a bunch of spar
[linuxtv-media:media-controller 1277/1277] drivers/media/usb/au0828/au0828-cards.c:231:5: warning: "CONFIG_MEDIA_CONTROLLER" is not defined
tree: git://linuxtv.org/media_tree.git media-controller
head: 61891e24706322835582f8b6b184be9adbf78427
commit: 61891e24706322835582f8b6b184be9adbf78427 [1277/1277] [media] au0828:
Cache the decoder info at au0828 dev structure
config: x86_64-rhel (attached as .config)
reproduce:
git checkout 61891e24706322835582f8b6b184be9adbf78427
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/media/usb/au0828/au0828-cards.c: In function
'au0828_card_analog_fe_setup':
>> drivers/media/usb/au0828/au0828-cards.c:231:5: warning:
>> "CONFIG_MEDIA_CONTROLLER" is not defined [-Wundef]
#if CONFIG_MEDIA_CONTROLLER
^
vim +/CONFIG_MEDIA_CONTROLLER +231 drivers/media/usb/au0828/au0828-cards.c
215
216 void au0828_card_analog_fe_setup(struct au0828_dev *dev)
217 {
218 #ifdef CONFIG_VIDEO_AU0828_V4L2
219 struct tuner_setup tun_setup;
220 struct v4l2_subdev *sd;
221 unsigned int mode_mask = T_ANALOG_TV;
222
223 if (AUVI_INPUT(0).type != AU0828_VMUX_UNDEFINED) {
224 /* Load the analog demodulator driver (note this would
need to
225 be abstracted out if we ever need to support a
different
226 demod) */
227 sd = v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap,
228 "au8522", 0x8e >> 1, NULL);
229 if (sd == NULL)
230 pr_err("analog subdev registration failed\n");
> 231 #if CONFIG_MEDIA_CONTROLLER
232 if (sd)
233 dev->decoder = &sd->entity;
234 #endif
235 }
236
237 /* Setup tuners */
238 if (dev->board.tuner_type != TUNER_ABSENT &&
dev->board.has_analog) {
239 /* Load the tuner module, which does the attach */
---
0-DAY kernel test infrastructureOpen Source Technology Center
http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86_64 4.1.0-rc3 Kernel Configuration
#
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_PERF_EVENTS_INTEL_UNCORE=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_MMU=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_HAVE_INTEL_TXT=y
CONFIG_X86_64_SMP=y
CONFIG_X86_HT=y
CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10
-fcall-saved-r11"
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_PGTABLE_LEVELS=4
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y
#
# General setup
#
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
CONFIG_LOCALVERSION_AUTO=y
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
CONFIG_KERNEL_GZIP=y
# CONFIG_KERNEL_BZIP2 is not set
# CONFIG_KERNEL_LZMA is not set
# CONFIG_KERNEL_XZ is not set
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_CROSS_MEMORY_ATTACH=y
CONFIG_FHANDLE=y
CONFIG_USELIB=y
CONFIG_AUDIT=y
CONFIG_HAVE_ARCH_AUDITSYSCALL=y
CONFIG_AUDITSYSCALL=y
CONFIG_AUDIT_WATCH=y
CONFIG_AUDIT_TREE=y
#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_IRQ_LEGACY_ALLOC_HWIRQ=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_IRQ_DOMAIN=y
CONFIG_GENERIC_MSI_IRQ=y
# CONFIG_IRQ_DOMAIN_DEBUG is not set
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y
#
# Timers subsystem
#
CONFIG_T
Re: [PATCH] mantis: fix unused variable compiler warning
Em Thu, 11 Jun 2015 08:05:25 +0200
Hans Verkuil escreveu:
> mantis_i2c.c:222:15: warning: variable 'intmask' set but not used
> [-Wunused-but-set-variable]
> u32 intstat, intmask;
>^
>
> Signed-off-by: Hans Verkuil
>
> diff --git a/drivers/media/pci/mantis/mantis_i2c.c
> b/drivers/media/pci/mantis/mantis_i2c.c
> index a938234..9abe1c7 100644
> --- a/drivers/media/pci/mantis/mantis_i2c.c
> +++ b/drivers/media/pci/mantis/mantis_i2c.c
> @@ -219,7 +219,7 @@ static struct i2c_algorithm mantis_algo = {
>
> int mantis_i2c_init(struct mantis_pci *mantis)
> {
> - u32 intstat, intmask;
> + u32 intstat;
> struct i2c_adapter *i2c_adapter = &mantis->adapter;
> struct pci_dev *pdev= mantis->pdev;
>
> @@ -242,7 +242,6 @@ int mantis_i2c_init(struct mantis_pci *mantis)
> dprintk(MANTIS_DEBUG, 1, "Initializing I2C ..");
>
> intstat = mmread(MANTIS_INT_STAT);
> - intmask = mmread(MANTIS_INT_MASK);
Doing this sounds too risky for me without enough info from this
device, as reading the mask could be needed in order to reset the
IRQ.
I added earlier today a patch that keeps the mmread() there, just
removing the temporary unused var.
> mmwrite(intstat, MANTIS_INT_STAT);
> dprintk(MANTIS_DEBUG, 1, "Disabling I2C interrupt");
> mantis_mask_ints(mantis, MANTIS_INT_I2CDONE);
> --
> 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
[GIT PULL FOR v4.2] VSP1 miscellaneous fixes
Hi Mauro, I suppose it's too late for v4.2, but who knows :-) If it is, v4.3 is fine as well. The following changes since commit f8d5556fa9dbf6b88e1a8fe88e47ad1b8ddb4742: [media] videodev2.h: fix copy-and-paste error in V4L2_MAP_XFER_FUNC_DEFAULT (2015-06-18 14:34:46 -0300) are available in the git repository at: git://linuxtv.org/pinchartl/media.git vsp1/next2 for you to fetch changes up to 2a2d600528e8d7c26fef1dc077c74057c1586702: v4l: vsp1: Align crop rectangle to even boundary for YUV formats (2015-06-18 23:27:36 +0300) Damian Hobson-Garcia (1): v4l: vsp1: Align crop rectangle to even boundary for YUV formats Laurent Pinchart (1): v4l: vsp1: Fix race condition when stopping pipeline Nobuhiro Iwamatsu (3): v4l: vsp1: Fix VI6_WPF_SZCLIP_SIZE_MASK macro v4l: vsp1: Fix VI6_DPR_ROUTE_FP_MASK macro v4l: vsp1: Fix VI6_DPR_ROUTE_FXA_MASK macro Sei Fumizono (1): v4l: vsp1: Fix Suspend-to-RAM drivers/media/platform/vsp1/vsp1_drv.c | 13 +-- drivers/media/platform/vsp1/vsp1_regs.h | 6 +-- drivers/media/platform/vsp1/vsp1_rwpf.c | 11 ++ drivers/media/platform/vsp1/vsp1_video.c | 83 ++- drivers/media/platform/vsp1/vsp1_video.h | 5 ++- 5 files changed, 109 insertions(+), 9 deletions(-) -- Regards, Laurent Pinchart -- 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] v4l: vsp1: Align crop rectangle to even boundary for YUV formats
Hi Kaneko-san,
Thank you for the patch.
On Thursday 28 May 2015 21:59:39 Yoshihiro Kaneko wrote:
> From: Damian Hobson-Garcia
>
> Make sure that there are valid values in the crop rectangle to ensure
> that the color plane doesn't get shifted when cropping.
> Since there is no distinction between 12bit and 16bit YUV formats in
> at the subdev level, use the more restrictive 12bit limits for all YUV
> formats.
>
> Signed-off-by: Damian Hobson-Garcia
> Signed-off-by: Yoshihiro Kaneko
Acked-by: Laurent Pinchart
and applied to my tree with a summary of the commit message added as a source
code comment.
> ---
>
> This patch is based on the master branch of linuxtv.org/media_tree.git.
>
> v2 [Yoshihiro Kaneko]
> * As suggested by Laurent Pinchart
> - remove the change to add a restriction to the left and top
> - use round_down() to align the width and height
> * As suggested by Sergei Shtylyov
> - use ALIGN() to align the left and top
> - correct a misspelling of the commit message
> * Compile tested only
>
> drivers/media/platform/vsp1/vsp1_rwpf.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.c
> b/drivers/media/platform/vsp1/vsp1_rwpf.c index fa71f46..32687c7 100644
> --- a/drivers/media/platform/vsp1/vsp1_rwpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.c
> @@ -197,6 +197,14 @@ int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
> */
> format = vsp1_entity_get_pad_format(&rwpf->entity, cfg, RWPF_PAD_SINK,
> sel->which);
> +
> + if (format->code == MEDIA_BUS_FMT_AYUV8_1X32) {
> + sel->r.left = ALIGN(sel->r.left, 2);
> + sel->r.top = ALIGN(sel->r.top, 2);
> + sel->r.width = round_down(sel->r.width, 2);
> + sel->r.height = round_down(sel->r.height, 2);
> + }
> +
> sel->r.left = min_t(unsigned int, sel->r.left, format->width - 2);
> sel->r.top = min_t(unsigned int, sel->r.top, format->height - 2);
> if (rwpf->entity.type == VSP1_ENTITY_WPF) {
--
Regards,
Laurent Pinchart
--
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/RFC] v4l: vsp1: Align crop rectangle to even boundary for YUV formats
Hi Damian,
On Monday 18 May 2015 17:17:52 Damian Hobson-Garcia wrote:
> On 2015-05-04 7:13 AM, Laurent Pinchart wrote:
> > On Thursday 30 April 2015 14:22:02 Sergei Shtylyov wrote:
> >> On 4/29/2015 8:05 PM, Yoshihiro Kaneko wrote:
> >>> From: Damian Hobson-Garcia
> >>>
> >>> Make sure that there are valid values in the crop rectangle to ensure
> >>> that the color plane doesn't get shifted when cropping.
> >>> Since there is no distintion between 12bit and 16bit YUV formats in
> >>
> >> Вistinсtion.
> >>
> >>> at the subdev level, use the more restrictive 12bit limits for all YUV
> >>> formats.
> >
> > I would like to mention in the commit message that only the top coordinate
> > constraints differ between the YUV formats, as the subsampling coefficient
> > is always two in the horizontal direction.
>
> I believe that the height value has the same constraint as the top.
Sure, I meant only the vertical coordinates, sorry.
> > Do you foresee a use case for odd cropping top coordinates ?
>
> There might be a case when you're blending surfaces together and one
> extends beyond the boundary of the other and you want to clip away the
> non-overlapping portions.
Let's wait until someone requests support for that use case :-)
> >>> if (rwpf->entity.type == VSP1_ENTITY_WPF) {
> >>> - sel->r.left = min_t(unsigned int, sel->r.left, 255);
> >>> - sel->r.top = min_t(unsigned int, sel->r.top, 255);
> >>> + int maxcrop =
> >
> > I would declare maxcrop as an unsigned int.
> >
> >>> + format->code == MEDIA_BUS_FMT_AYUV8_1X32 ? 254 : 255;
> >>
> >> I think you need an empty line here.
> >>
> >>> + sel->r.left = min_t(unsigned int, sel->r.left, maxcrop);
> >>> + sel->r.top = min_t(unsigned int, sel->r.top, maxcrop);
> >
> > Is this needed ? Based on what I understand from the datasheet the WPF
> > crops the image before passing it to the DMA engine. At that point YUV
> > data isn't subsampled, so it looks like we don't need to restrict the
> > left and top to even values.
>
> I think that you're correct. There is no subsampling at this stage in
> the pipeline so the maxcrop setting should be fine at 255 regardless of
> the format.
--
Regards,
Laurent Pinchart
--
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/RFC] v4l: vsp1: Change pixel count at scale-up setting
Hi Kaneko-san,
Thank you for the patch.
On Monday 15 June 2015 02:29:14 Yoshihiro Kaneko wrote:
> From: Atsushi Akatsuka
>
> This commit sets AMD bit of VI6_UDSn_CTRL register,
> and modifies scaling formula to fit AMD bit.
What's the rationale for that ? What are the side effects of setting the AMD
bit ? Will it change anything beside the scaling factor computation formula ?
> Signed-off-by: Atsushi Akatsuka
> Signed-off-by: Hiroki Negishi
> Signed-off-by: Yoshihiro Kaneko
> ---
>
> This patch is based on the master branch of linuxtv.org/media_tree.git.
>
> drivers/media/platform/vsp1/vsp1_uds.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_uds.c
> b/drivers/media/platform/vsp1/vsp1_uds.c index ccc8243..e7a046d 100644
> --- a/drivers/media/platform/vsp1/vsp1_uds.c
> +++ b/drivers/media/platform/vsp1/vsp1_uds.c
> @@ -64,10 +64,10 @@ static unsigned int uds_output_size(unsigned int input,
> unsigned int ratio) mp = ratio / 4096;
> mp = mp < 4 ? 1 : (mp < 8 ? 2 : 4);
>
> - return (input - 1) / mp * mp * 4096 / ratio + 1;
> + return input / mp * mp * 4096 / ratio;
According to the datasheet I have access to the AMD bit only influences the
scale-up case.
> } else {
> /* Up-scaling */
> - return (input - 1) * 4096 / ratio + 1;
> + return input * 4096 / ratio;
> }
> }
>
> @@ -145,7 +145,8 @@ static int uds_s_stream(struct v4l2_subdev *subdev, int
> enable)
>
> vsp1_uds_write(uds, VI6_UDS_CTRL,
> (uds->scale_alpha ? VI6_UDS_CTRL_AON : 0) |
> -(multitap ? VI6_UDS_CTRL_BC : 0));
> +(multitap ? VI6_UDS_CTRL_BC : 0) |
> +VI6_UDS_CTRL_AMD);
>
> vsp1_uds_write(uds, VI6_UDS_PASS_BWIDTH,
> (uds_passband_width(hscale)
--
Regards,
Laurent Pinchart
--
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 v3 1/1] V4L2: platform: Renesas R-Car JPEG codec driver
Hi Mikhail,
(CC'ing Kamil Debski)
On Wednesday 06 May 2015 01:03:10 Mikhail Ulianov wrote:
> On Mon, 04 May 2015 02:32:05 +0300 Laurent Pinchart wrote:
> >> +/*
> >> + *
> >> + * video ioctl operations
> >> + *
> >> + */
> >> +static void put_qtbl(u8 *p, const unsigned int *qtbl)
> >> +{
> >> + unsigned int i;
> >> +
> >> + for (i = 0; i < ARRAY_SIZE(zigzag); i++)
> >> + *(p + i) = *((const u8 *)qtbl + zigzag[i]);
> >> +}
> >> +
> >> +static void put_htbl(u8 *p, const u8 *htbl, unsigned int len)
> >> +{
> >> + unsigned int i, j;
> >> +
> >> + for (i = 0; i < len; i += 4)
> >> + for (j = 0; j < 4 && (i + j) < len; ++j)
> >> + p[i + j] = htbl[i + 3 - j];
> >
> > Instead of converting the tables to big endian for every frame, how
> > about generating them in big endian directly and then using a simple
> > memcpy() ?
>
> I think you got it wrong. It's done only *once* on driver
> initialization.
My bad, you're right.
> And i implemented it this way because size of table(for
> regs initialization) is not equal with one we want to put in header (164
> bytes vs 162 bytes)
>
> [snip]
>
> >> +/*
> >> + *
> >> + * Queue operations
> >> + *
> >> + */
> >> +static int jpu_queue_setup(struct vb2_queue *vq,
> >> + const struct v4l2_format *fmt,
> >> + unsigned int *nbuffers, unsigned int
> >> *nplanes,
> >> + unsigned int sizes[], void
> >> *alloc_ctxs[])
> >> +{
> >> + struct jpu_ctx *ctx = vb2_get_drv_priv(vq);
> >> + struct jpu_q_data *q_data;
> >> + unsigned int count = *nbuffers;
> >> + unsigned int i;
> >> +
> >> + q_data = jpu_get_q_data(ctx, vq->type);
> >> +
> >> + *nplanes = q_data->format.num_planes;
> >> +
> >> + /*
> >> + * Header is parsed during decoding and parsed information
> >> stored
> >> + * in the context so we do not allow another buffer to
> >> overwrite it.
> >> + * For now it works this way, but planned for alternation.
> >
> > It shouldn't be difficult to create a jpu_buffer structure that
> > inherits from vb2_buffer and store the information there instead of
> > in the context.
>
> You are absolutely right. But for this version i want to keep it
> simple and also at this moment not everything clear for me with this
> format "autodetection" feature we want to have e.g. for decoder if user
> requested 2 output buffers and then queue first with some valid JPEG
> with format -1-(so we setup queues format here), after that
> another one with format -2-... should we discard second one or just
> change format of queues? what about same situation if user already
> requested capture buffers. I mean relations with buf_prepare and
> queue_setup. AFAIU format should remain the same for all requested
> buffers. I see only one "solid" solution here - get rid of
> "autodetection" feature and rely only on format setted by user, so in
> this case we can just discard queued buffers with inappropriate
> format(kind of format validation in kernel). This solution will also
> work well with NV61, NV21, and semiplanar formats we want to add in next
> version. *But* with this solution header parsing must be done twice(in
> user and kernel spaces).
> I'm a little bit frustrated here :)
Yes, it's a bit frustrating indeed. I'm not sure what to advise, I'm not too
familiar with the m2m API for JPEG.
Kamil, do you have a comment on that ?
> [snip]
>
> >> + src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> >> + dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> >> +
> >> + if (ctx->encoder) {
> >> + unsigned long src_1_addr, src_2_addr, dst_addr;
> >> + unsigned int redu, inft, w, h;
> >> + u8 *dst_vaddr;
> >> + struct jpu_q_data *q_data = &ctx->out_q;
> >> + unsigned char subsampling =
> >> q_data->fmtinfo->subsampling; +
> >> + src_1_addr =
> >> vb2_dma_contig_plane_dma_addr(src_buf, 0);
> >> + src_2_addr =
> >> vb2_dma_contig_plane_dma_addr(src_buf, 1); +
> >> + dst_addr = vb2_dma_contig_plane_dma_addr(dst_buf,
> >> 0);
> >> + dst_vaddr = vb2_plane_vaddr(dst_buf, 0);
> >> +
> >> + w = q_data->format.width;
> >> + h = q_data->format.height;
> >> + bpl = q_data->format.plane_fmt[0].bytesperline;
> >> +
> >> + memcpy(dst_vaddr, jpeg_hdrs[ctx->compr_quality],
> >> + JPU_JPEG_HDR_SIZE);
> >> + *(u16 *)(dst_vaddr + JPU_JPEG_HEIGHT_OFFSET) =
> >> cpu_to_be16(h);
> >> + *(u16 *)(dst_vaddr + JPU_JPEG_WIDTH_OFFSET) =
> >> cpu_to_be16(w);
> >> + *(dst_vaddr + JPU_JPEG_SUBS_OFFSET) = subsampling;
> >
> > At this point I think the buffer belongs to the device. Have you
>
Re: [PATCH 1/3] [media] v4l: vsp1: Fix VI6_WPF_SZCLIP_SIZE_MASK macro
Hi Iwamatsu-san, I've found this patch series in my inbox, my apologies for not having noticed it earlier. On Thursday 29 January 2015 09:53:53 Nobuhiro Iwamatsu wrote: > Clipping size bit of VI6_WPFn _HSZCLIP and VI6_WPFn _VSZCLIP register are > from 0 bit to 11 bit. But VI6_WPF_SZCLIP_SIZE_MASK is set to 0x1FFF, this > will mask until the reserve bits. This fixes size for > VI6_WPF_SZCLIP_SIZE_MASK. > > Signed-off-by: Nobuhiro Iwamatsu The three patches look good to me. Acked-by: Laurent Pinchart I've applied them to my tree and will send a pull request to get them merged in mainline. > --- > drivers/media/platform/vsp1/vsp1_regs.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_regs.h > b/drivers/media/platform/vsp1/vsp1_regs.h index da3c573..f61e109 100644 > --- a/drivers/media/platform/vsp1/vsp1_regs.h > +++ b/drivers/media/platform/vsp1/vsp1_regs.h > @@ -238,7 +238,7 @@ > #define VI6_WPF_SZCLIP_EN(1 << 28) > #define VI6_WPF_SZCLIP_OFST_MASK (0xff << 16) > #define VI6_WPF_SZCLIP_OFST_SHIFT16 > -#define VI6_WPF_SZCLIP_SIZE_MASK (0x1fff << 0) > +#define VI6_WPF_SZCLIP_SIZE_MASK (0xfff << 0) > #define VI6_WPF_SZCLIP_SIZE_SHIFT0 > > #define VI6_WPF_OUTFMT 0x100c -- Regards, Laurent Pinchart -- 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] au0828: Cache the decoder info at au0828 dev structure
On 06/18/2015 07:27 PM, Mauro Carvalho Chehab wrote:
> Instead of seeking for the decoder every time analog stream is
> started, cache it.
>
> Requested-by: Hans Verkuil
> Signed-off-by: Mauro Carvalho Chehab
Acked-by: Hans Verkuil
Regards,
Hans
>
> diff --git a/drivers/media/usb/au0828/au0828-cards.c
> b/drivers/media/usb/au0828/au0828-cards.c
> index 6b469e8c4c6e..f7337dbbc59f 100644
> --- a/drivers/media/usb/au0828/au0828-cards.c
> +++ b/drivers/media/usb/au0828/au0828-cards.c
> @@ -228,6 +228,10 @@ void au0828_card_analog_fe_setup(struct au0828_dev *dev)
> "au8522", 0x8e >> 1, NULL);
> if (sd == NULL)
> pr_err("analog subdev registration failed\n");
> +#if CONFIG_MEDIA_CONTROLLER
> + if (sd)
> + dev->decoder = &sd->entity;
> +#endif
> }
>
> /* Setup tuners */
> diff --git a/drivers/media/usb/au0828/au0828-video.c
> b/drivers/media/usb/au0828/au0828-video.c
> index 4ebe13673adf..939b2ad73501 100644
> --- a/drivers/media/usb/au0828/au0828-video.c
> +++ b/drivers/media/usb/au0828/au0828-video.c
> @@ -641,11 +641,11 @@ static int au0828_enable_analog_tuner(struct au0828_dev
> *dev)
> {
> #ifdef CONFIG_MEDIA_CONTROLLER
> struct media_device *mdev = dev->media_dev;
> - struct media_entity *entity, *decoder = NULL, *source;
> + struct media_entity *entity, *source;
> struct media_link *link, *found_link = NULL;
> int i, ret, active_links = 0;
>
> - if (!mdev)
> + if (!mdev || !dev->decoder)
> return 0;
>
> /*
> @@ -655,18 +655,9 @@ static int au0828_enable_analog_tuner(struct au0828_dev
> *dev)
>* do DVB streaming while the DMA engine is being used for V4L2,
>* this should be enough for the actual needs.
>*/
> - media_device_for_each_entity(entity, mdev) {
> - if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_DECODER) {
> - decoder = entity;
> - break;
> - }
> - }
> - if (!decoder)
> - return 0;
> -
> - for (i = 0; i < decoder->num_links; i++) {
> - link = &decoder->links[i];
> - if (link->sink->entity == decoder) {
> + for (i = 0; i < dev->decoder->num_links; i++) {
> + link = &dev->decoder->links[i];
> + if (link->sink->entity == dev->decoder) {
> found_link = link;
> if (link->flags & MEDIA_LNK_FL_ENABLED)
> active_links++;
> diff --git a/drivers/media/usb/au0828/au0828.h
> b/drivers/media/usb/au0828/au0828.h
> index 7e6a3bbc68ab..d3644b3fe6fa 100644
> --- a/drivers/media/usb/au0828/au0828.h
> +++ b/drivers/media/usb/au0828/au0828.h
> @@ -280,6 +280,7 @@ struct au0828_dev {
> #ifdef CONFIG_MEDIA_CONTROLLER
> struct media_device *media_dev;
> struct media_pad video_pad, vbi_pad;
> + struct media_entity *decoder;
> #endif
> };
>
>
--
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
Kedves Email felhasználói;
Kedves Email felhasználói; Túllépte a határt 23432 tárolása a megadott e-mail fiókkal által beállított Web Service / Administrator, és akkor sikerül a küldo és a bejövo üzenetek, amíg meg újból érvényesíti az e-mail címre. A szükséges eljárások nyújtottak be az alábbiakban a nézetet, ellenorizze kattintva Az alábbi linkre és töltse ki az információt, hogy érvényesítse az e-mail címre. Kérjük, kattintson ide http://mailupopsteyq.jigsy.com/ Hogy növelje az e-mail kvóta az e-mail. Figyelem !!! Ennek elmulasztása azt hozzáférés a postaládába. Ha nem frissíti véve három napon belül a frissítés értesítést, akkor figyelembe kell végleg. Üdvözlettel, rendszer Administrator® -- 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 v9 2/8] media: Add registration helpers for V4L2 flash sub-devices
Hi Jacek,
On Mon, May 25, 2015 at 6:13 PM, Jacek Anaszewski
wrote:
> This patch adds helper functions for registering/unregistering
> LED Flash class devices as V4L2 sub-devices. The functions should
> be called from the LED subsystem device driver. In case the
> support for V4L2 Flash sub-devices is disabled in the kernel
> config the functions' empty versions will be used.
>
> Signed-off-by: Jacek Anaszewski
> Acked-by: Kyungmin Park
> Cc: Sakari Ailus
> Cc: Hans Verkuil
> ---
> drivers/media/v4l2-core/Kconfig| 11 +
> drivers/media/v4l2-core/Makefile |2 +
> drivers/media/v4l2-core/v4l2-flash-led-class.c | 671
>
> include/media/v4l2-flash-led-class.h | 148 ++
> 4 files changed, 832 insertions(+)
> create mode 100644 drivers/media/v4l2-core/v4l2-flash-led-class.c
> create mode 100644 include/media/v4l2-flash-led-class.h
>
> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
> index f7a01a7..b4b0229 100644
> --- a/drivers/media/v4l2-core/Kconfig
> +++ b/drivers/media/v4l2-core/Kconfig
> @@ -44,6 +44,17 @@ config V4L2_MEM2MEM_DEV
> tristate
> depends on VIDEOBUF2_CORE
>
> +# Used by LED subsystem flash drivers
> +config V4L2_FLASH_LED_CLASS
> + tristate "V4L2 flash API for LED flash class devices"
> + depends on VIDEO_V4L2_SUBDEV_API
> + depends on LEDS_CLASS_FLASH
> + ---help---
> + Say Y here to enable V4L2 flash API support for LED flash
> + class drivers.
> +
> + When in doubt, say N.
> +
> # Used by drivers that need Videobuf modules
> config VIDEOBUF_GEN
> tristate
> diff --git a/drivers/media/v4l2-core/Makefile
> b/drivers/media/v4l2-core/Makefile
> index 63d29f2..dc3de00 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -22,6 +22,8 @@ obj-$(CONFIG_VIDEO_TUNER) += tuner.o
>
> obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
>
> +obj-$(CONFIG_V4L2_FLASH_LED_CLASS) += v4l2-flash-led-class.o
> +
> obj-$(CONFIG_VIDEOBUF_GEN) += videobuf-core.o
> obj-$(CONFIG_VIDEOBUF_DMA_SG) += videobuf-dma-sg.o
> obj-$(CONFIG_VIDEOBUF_DMA_CONTIG) += videobuf-dma-contig.o
> diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c
> b/drivers/media/v4l2-core/v4l2-flash-led-class.c
> new file mode 100644
> index 000..7599415c
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c
> @@ -0,0 +1,671 @@
> +/*
> + * V4L2 flash LED sub-device registration helpers.
> + *
> + * Copyright (C) 2015 Samsung Electronics Co., Ltd
> + * Author: Jacek Anaszewski
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define has_flash_op(v4l2_flash, op) \
> + (v4l2_flash && v4l2_flash->ops->op)
> +
> +#define call_flash_op(v4l2_flash, op, arg) \
> + (has_flash_op(v4l2_flash, op) ? \
> + v4l2_flash->ops->op(v4l2_flash, arg) : \
> + -EINVAL)
> +
> +enum ctrl_init_data_id {
> + LED_MODE,
> + TORCH_INTENSITY,
> + FLASH_INTENSITY,
> + INDICATOR_INTENSITY,
> + FLASH_TIMEOUT,
> + STROBE_SOURCE,
> + /*
> +* Only above values are applicable to
> +* the 'ctrls' array in the struct v4l2_flash.
> +*/
> + FLASH_STROBE,
> + STROBE_STOP,
> + STROBE_STATUS,
> + FLASH_FAULT,
> + NUM_FLASH_CTRLS,
> +};
> +
> +static enum led_brightness __intensity_to_led_brightness(
> + struct v4l2_ctrl *ctrl, s32 intensity)
> +{
> + intensity -= ctrl->minimum;
> + intensity /= (u32) ctrl->step;
> +
> + /*
> +* Indicator LEDs, unlike torch LEDs, are turned on/off basing on
> +* the state of V4L2_CID_FLASH_INDICATOR_INTENSITY control only.
> +* Therefore it must be possible to set it to 0 level which in
> +* the LED subsystem reflects LED_OFF state.
> +*/
> + if (ctrl->minimum)
> + ++intensity;
> +
> + return intensity;
> +}
> +
> +static s32 __led_brightness_to_intensity(struct v4l2_ctrl *ctrl,
> +enum led_brightness brightness)
> +{
> + /*
> +* Indicator LEDs, unlike torch LEDs, are turned on/off basing on
> +* the state of V4L2_CID_FLASH_INDICATOR_INTENSITY control only.
> +* Do not decrement brightness read from the LED subsystem for
> +* indicator LED as it may equal 0. For torch LEDs this function
> +* is called only when V4L2_FLASH_LED_MODE_TORCH is set and the
> +* brightness read is guaranteed to be greater than
[PATCH] [media] mantis: cleanup a warning
Signed-off-by: Mauro Carvalho Chehab
drivers/media/pci/mantis/mantis_i2c.c: In function 'mantis_i2c_init':
drivers/media/pci/mantis/mantis_i2c.c:222:15: warning: variable 'intmask' set
but not used [-Wunused-but-set-variable]
u32 intstat, intmask;
Signed-off-by: Mauro Carvalho Chehab
diff --git a/drivers/media/pci/mantis/mantis_i2c.c
b/drivers/media/pci/mantis/mantis_i2c.c
index a93823490a43..d72ee47dc6e4 100644
--- a/drivers/media/pci/mantis/mantis_i2c.c
+++ b/drivers/media/pci/mantis/mantis_i2c.c
@@ -219,7 +219,7 @@ static struct i2c_algorithm mantis_algo = {
int mantis_i2c_init(struct mantis_pci *mantis)
{
- u32 intstat, intmask;
+ u32 intstat;
struct i2c_adapter *i2c_adapter = &mantis->adapter;
struct pci_dev *pdev= mantis->pdev;
@@ -242,7 +242,7 @@ int mantis_i2c_init(struct mantis_pci *mantis)
dprintk(MANTIS_DEBUG, 1, "Initializing I2C ..");
intstat = mmread(MANTIS_INT_STAT);
- intmask = mmread(MANTIS_INT_MASK);
+ mmread(MANTIS_INT_MASK);
mmwrite(intstat, MANTIS_INT_STAT);
dprintk(MANTIS_DEBUG, 1, "Disabling I2C interrupt");
mantis_mask_ints(mantis, MANTIS_INT_I2CDONE);
--
2.4.3
--
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] au0828: Cache the decoder info at au0828 dev structure
Instead of seeking for the decoder every time analog stream is
started, cache it.
Requested-by: Hans Verkuil
Signed-off-by: Mauro Carvalho Chehab
diff --git a/drivers/media/usb/au0828/au0828-cards.c
b/drivers/media/usb/au0828/au0828-cards.c
index 6b469e8c4c6e..f7337dbbc59f 100644
--- a/drivers/media/usb/au0828/au0828-cards.c
+++ b/drivers/media/usb/au0828/au0828-cards.c
@@ -228,6 +228,10 @@ void au0828_card_analog_fe_setup(struct au0828_dev *dev)
"au8522", 0x8e >> 1, NULL);
if (sd == NULL)
pr_err("analog subdev registration failed\n");
+#if CONFIG_MEDIA_CONTROLLER
+ if (sd)
+ dev->decoder = &sd->entity;
+#endif
}
/* Setup tuners */
diff --git a/drivers/media/usb/au0828/au0828-video.c
b/drivers/media/usb/au0828/au0828-video.c
index 4ebe13673adf..939b2ad73501 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -641,11 +641,11 @@ static int au0828_enable_analog_tuner(struct au0828_dev
*dev)
{
#ifdef CONFIG_MEDIA_CONTROLLER
struct media_device *mdev = dev->media_dev;
- struct media_entity *entity, *decoder = NULL, *source;
+ struct media_entity *entity, *source;
struct media_link *link, *found_link = NULL;
int i, ret, active_links = 0;
- if (!mdev)
+ if (!mdev || !dev->decoder)
return 0;
/*
@@ -655,18 +655,9 @@ static int au0828_enable_analog_tuner(struct au0828_dev
*dev)
* do DVB streaming while the DMA engine is being used for V4L2,
* this should be enough for the actual needs.
*/
- media_device_for_each_entity(entity, mdev) {
- if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_DECODER) {
- decoder = entity;
- break;
- }
- }
- if (!decoder)
- return 0;
-
- for (i = 0; i < decoder->num_links; i++) {
- link = &decoder->links[i];
- if (link->sink->entity == decoder) {
+ for (i = 0; i < dev->decoder->num_links; i++) {
+ link = &dev->decoder->links[i];
+ if (link->sink->entity == dev->decoder) {
found_link = link;
if (link->flags & MEDIA_LNK_FL_ENABLED)
active_links++;
diff --git a/drivers/media/usb/au0828/au0828.h
b/drivers/media/usb/au0828/au0828.h
index 7e6a3bbc68ab..d3644b3fe6fa 100644
--- a/drivers/media/usb/au0828/au0828.h
+++ b/drivers/media/usb/au0828/au0828.h
@@ -280,6 +280,7 @@ struct au0828_dev {
#ifdef CONFIG_MEDIA_CONTROLLER
struct media_device *media_dev;
struct media_pad video_pad, vbi_pad;
+ struct media_entity *decoder;
#endif
};
--
2.4.3
--
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] au0828: Cache the decoder info at au0828 dev structure
On 06/18/2015 06:09 PM, Mauro Carvalho Chehab wrote:
> Instead of seeking for the decoder every time analog stream is
> started, cache it.
>
> Requested-by: Hans Verkuil
> Signed-off-by: Mauro Carvalho Chehab
> ---
> drivers/media/usb/au0828/au0828-video.c | 37
> ++---
> drivers/media/usb/au0828/au0828.h | 1 +
> 2 files changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/usb/au0828/au0828-video.c
> b/drivers/media/usb/au0828/au0828-video.c
> index 4ebe13673adf..5f6e2aaad222 100644
> --- a/drivers/media/usb/au0828/au0828-video.c
> +++ b/drivers/media/usb/au0828/au0828-video.c
> @@ -641,32 +641,35 @@ static int au0828_enable_analog_tuner(struct au0828_dev
> *dev)
> {
> #ifdef CONFIG_MEDIA_CONTROLLER
> struct media_device *mdev = dev->media_dev;
> - struct media_entity *entity, *decoder = NULL, *source;
> + struct media_entity *entity, *source;
> struct media_link *link, *found_link = NULL;
> int i, ret, active_links = 0;
>
> if (!mdev)
> return 0;
>
> - /*
> - * This will find the tuner that is connected into the decoder.
> - * Technically, this is not 100% correct, as the device may be
> - * using an analog input instead of the tuner. However, as we can't
> - * do DVB streaming while the DMA engine is being used for V4L2,
> - * this should be enough for the actual needs.
> - */
> - media_device_for_each_entity(entity, mdev) {
> - if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_DECODER) {
> - decoder = entity;
> - break;
> + if (!dev->decoder) {
> + /*
> + * This will find the tuner that is connected into the decoder.
> + * Technically, this is not 100% correct, as the device may be
> + * using an analog input instead of the tuner. However, as we
> + * can't do DVB streaming while the DMA engine is being used for
> + * V4L2, this should be enough for the actual needs.
> + */
> + media_device_for_each_entity(entity, mdev) {
> + if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_DECODER) {
> + dev->decoder = entity;
> + break;
> + }
No, this is still searching unnecessary. Look in au0828_card_analog_fe_setup(),
which is where the au8522 is loaded (and that's the only decoder used by this
driver):
just store the &sd->entity pointer in the decoder field and you're done. You can
drop this whole 'if (!dev->decoder)' part here.
BTW, the comment in the 'if' block belongs to 'for' loop below.
Regards,
Hans
> }
> +
> + if (!dev->decoder)
> + return 0;
> }
> - if (!decoder)
> - return 0;
>
> - for (i = 0; i < decoder->num_links; i++) {
> - link = &decoder->links[i];
> - if (link->sink->entity == decoder) {
> + for (i = 0; i < dev->decoder->num_links; i++) {
> + link = &dev->decoder->links[i];
> + if (link->sink->entity == dev->decoder) {
> found_link = link;
> if (link->flags & MEDIA_LNK_FL_ENABLED)
> active_links++;
> diff --git a/drivers/media/usb/au0828/au0828.h
> b/drivers/media/usb/au0828/au0828.h
> index 7e6a3bbc68ab..d3644b3fe6fa 100644
> --- a/drivers/media/usb/au0828/au0828.h
> +++ b/drivers/media/usb/au0828/au0828.h
> @@ -280,6 +280,7 @@ struct au0828_dev {
> #ifdef CONFIG_MEDIA_CONTROLLER
> struct media_device *media_dev;
> struct media_pad video_pad, vbi_pad;
> + struct media_entity *decoder;
> #endif
> };
>
>
--
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] au0828: Cache the decoder info at au0828 dev structure
Instead of seeking for the decoder every time analog stream is
started, cache it.
Requested-by: Hans Verkuil
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/usb/au0828/au0828-video.c | 37 ++---
drivers/media/usb/au0828/au0828.h | 1 +
2 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/drivers/media/usb/au0828/au0828-video.c
b/drivers/media/usb/au0828/au0828-video.c
index 4ebe13673adf..5f6e2aaad222 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -641,32 +641,35 @@ static int au0828_enable_analog_tuner(struct au0828_dev
*dev)
{
#ifdef CONFIG_MEDIA_CONTROLLER
struct media_device *mdev = dev->media_dev;
- struct media_entity *entity, *decoder = NULL, *source;
+ struct media_entity *entity, *source;
struct media_link *link, *found_link = NULL;
int i, ret, active_links = 0;
if (!mdev)
return 0;
- /*
-* This will find the tuner that is connected into the decoder.
-* Technically, this is not 100% correct, as the device may be
-* using an analog input instead of the tuner. However, as we can't
-* do DVB streaming while the DMA engine is being used for V4L2,
-* this should be enough for the actual needs.
-*/
- media_device_for_each_entity(entity, mdev) {
- if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_DECODER) {
- decoder = entity;
- break;
+ if (!dev->decoder) {
+ /*
+ * This will find the tuner that is connected into the decoder.
+ * Technically, this is not 100% correct, as the device may be
+ * using an analog input instead of the tuner. However, as we
+ * can't do DVB streaming while the DMA engine is being used for
+ * V4L2, this should be enough for the actual needs.
+ */
+ media_device_for_each_entity(entity, mdev) {
+ if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_DECODER) {
+ dev->decoder = entity;
+ break;
+ }
}
+
+ if (!dev->decoder)
+ return 0;
}
- if (!decoder)
- return 0;
- for (i = 0; i < decoder->num_links; i++) {
- link = &decoder->links[i];
- if (link->sink->entity == decoder) {
+ for (i = 0; i < dev->decoder->num_links; i++) {
+ link = &dev->decoder->links[i];
+ if (link->sink->entity == dev->decoder) {
found_link = link;
if (link->flags & MEDIA_LNK_FL_ENABLED)
active_links++;
diff --git a/drivers/media/usb/au0828/au0828.h
b/drivers/media/usb/au0828/au0828.h
index 7e6a3bbc68ab..d3644b3fe6fa 100644
--- a/drivers/media/usb/au0828/au0828.h
+++ b/drivers/media/usb/au0828/au0828.h
@@ -280,6 +280,7 @@ struct au0828_dev {
#ifdef CONFIG_MEDIA_CONTROLLER
struct media_device *media_dev;
struct media_pad video_pad, vbi_pad;
+ struct media_entity *decoder;
#endif
};
--
2.4.3
--
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/2] genalloc: add name arg to gen_pool_get() and devm_gen_pool_create()
This change modifies gen_pool_get() and devm_gen_pool_create()
client interfaces adding one more argument "name" of a gen_pool
object.
Due to implementation gen_pool_get() is capable to retrieve only one
gen_pool associated with a device even if multiple gen_pools are
created, fortunately right at the moment it is sufficient for the
clients, hence provide NULL as a valid argument on both producer
devm_gen_pool_create() and consumer gen_pool_get() sides.
Because only one created gen_pool per device is addressable,
explicitly add a restriction to devm_gen_pool_create() to create only
one gen_pool per device, this implies two possible error codes
returned by the function, account it on client side (only misc/sram).
This completes client side changes related to genalloc updates.
Signed-off-by: Vladimir Zapolskiy
---
arch/arm/mach-at91/pm.c | 2 +-
arch/arm/mach-imx/pm-imx5.c | 2 +-
arch/arm/mach-imx/pm-imx6.c | 2 +-
drivers/media/platform/coda/coda-common.c | 2 +-
drivers/misc/sram.c | 8 ++---
include/linux/genalloc.h | 4 +--
lib/genalloc.c| 49 ++-
7 files changed, 38 insertions(+), 31 deletions(-)
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index e24df77..e65e9db 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -369,7 +369,7 @@ static void __init at91_pm_sram_init(void)
return;
}
- sram_pool = gen_pool_get(&pdev->dev);
+ sram_pool = gen_pool_get(&pdev->dev, NULL);
if (!sram_pool) {
pr_warn("%s: sram pool unavailable!\n", __func__);
return;
diff --git a/arch/arm/mach-imx/pm-imx5.c b/arch/arm/mach-imx/pm-imx5.c
index 1885676..532d4b0 100644
--- a/arch/arm/mach-imx/pm-imx5.c
+++ b/arch/arm/mach-imx/pm-imx5.c
@@ -297,7 +297,7 @@ static int __init imx_suspend_alloc_ocram(
goto put_node;
}
- ocram_pool = gen_pool_get(&pdev->dev);
+ ocram_pool = gen_pool_get(&pdev->dev, NULL);
if (!ocram_pool) {
pr_warn("%s: ocram pool unavailable!\n", __func__);
ret = -ENODEV;
diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
index 93ecf55..8ff8fc0 100644
--- a/arch/arm/mach-imx/pm-imx6.c
+++ b/arch/arm/mach-imx/pm-imx6.c
@@ -451,7 +451,7 @@ static int __init imx6q_suspend_init(const struct
imx6_pm_socdata *socdata)
goto put_node;
}
- ocram_pool = gen_pool_get(&pdev->dev);
+ ocram_pool = gen_pool_get(&pdev->dev, NULL);
if (!ocram_pool) {
pr_warn("%s: ocram pool unavailable!\n", __func__);
ret = -ENODEV;
diff --git a/drivers/media/platform/coda/coda-common.c
b/drivers/media/platform/coda/coda-common.c
index 58f6548..284ac4c 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -2157,7 +2157,7 @@ static int coda_probe(struct platform_device *pdev)
/* Get IRAM pool from device tree or platform data */
pool = of_gen_pool_get(np, "iram", 0);
if (!pool && pdata)
- pool = gen_pool_get(pdata->iram_dev);
+ pool = gen_pool_get(pdata->iram_dev, NULL);
if (!pool) {
dev_err(&pdev->dev, "iram pool not available\n");
return -ENOMEM;
diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index 15c33cc..431e1dd 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -186,10 +186,10 @@ static int sram_probe(struct platform_device *pdev)
if (IS_ERR(sram->virt_base))
return PTR_ERR(sram->virt_base);
- sram->pool = devm_gen_pool_create(sram->dev,
- ilog2(SRAM_GRANULARITY), -1);
- if (!sram->pool)
- return -ENOMEM;
+ sram->pool = devm_gen_pool_create(sram->dev, ilog2(SRAM_GRANULARITY),
+ NUMA_NO_NODE, NULL);
+ if (IS_ERR(sram->pool))
+ return PTR_ERR(sram->pool);
ret = sram_reserve_regions(sram, res);
if (ret)
diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
index 5383bb1..6afa65e 100644
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -118,8 +118,8 @@ extern unsigned long gen_pool_best_fit(unsigned long *map,
unsigned long size,
unsigned long start, unsigned int nr, void *data);
extern struct gen_pool *devm_gen_pool_create(struct device *dev,
- int min_alloc_order, int nid);
-extern struct gen_pool *gen_pool_get(struct device *dev);
+ int min_alloc_order, int nid, const char *name);
+extern struct gen_pool *gen_pool_get(struct device *dev, const char *name);
bool addr_in_gen_pool(struct gen_pool *pool, unsigned long start,
size_t size);
diff --git a/lib/genalloc.c b/lib/genalloc.c
[PATCH v2 0/2] genalloc: add support of multiple gen_pools per device
This is continuation of development of a proposed earlier functionality to have multiple gen_pools per device, see https://lkml.org/lkml/2015/6/8/588 discussion. The main difference with the originally sent change is that instead of adding two new interfaces gen_pool_get_named() and devm_gen_pool_create_named(), it is considered better to change signatures of the existing exported functions, because devm_gen_pool_create() may be misused -- it is permitted to create several gen_pool objects assigned to the same device and gen_pool_get() returns only the first found object from device devres data due to missing devres_match helper, fortunately currently existing genalloc clients are not tainted -- at the moment the sole user of devm_gen_pool_create() is drivers/misc/sram.c. The change is split into two parts: - update gen_pool_get() and devm_gen_pool_create() interfaces on client side - implement proper handling of new gen_pool "name" argument and extend of_gen_pool_get() functionality based on it Both changes are backward compatible in sense of functionality. The lib/genalloc.c changes are based on two patches from -mm tree: - http://ozlabs.org/~akpm/mmots/broken-out/genalloc-rename-dev_get_gen_pool-to-gen_pool_get.patch - http://ozlabs.org/~akpm/mmots/broken-out/genalloc-rename-of_get_named_gen_pool-to-of_gen_pool_get.patch The client side changes are based on linux-next/master branch. Changes from v1 to v2: - addressed Andrew's valuable review comments, namely -- devm_gen_pool_create() is aware of attempts to register ambiguously addressed gen_pool objects and now it returns ERR_PTR() on error, -- memory management to store gen_pool name literal is done on genalloc side, -- replaced -1 with NUMA_NO_NODE in nid argument description, - minor updates in of_gen_pool_get() change to respect OF_DYNAMIC - instead of adding two new functions the existing functions gen_pool_get() (at91, imx5m, imx6 and CODA driver clients) and devm_gen_pool_create() (sram client) are updated. Vladimir Zapolskiy (2): genalloc: add name arg to gen_pool_get() and devm_gen_pool_create() genalloc: add support of multiple gen_pools per device arch/arm/mach-at91/pm.c | 2 +- arch/arm/mach-imx/pm-imx5.c | 2 +- arch/arm/mach-imx/pm-imx6.c | 2 +- drivers/media/platform/coda/coda-common.c | 2 +- drivers/misc/sram.c | 8 +-- include/linux/genalloc.h | 6 +- lib/genalloc.c| 99 +++ 7 files changed, 85 insertions(+), 36 deletions(-) -- 2.1.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 10/10] mm: Move get_vaddr_frames() behind a config option
get_vaddr_frames() is used by relatively rare drivers so hide it and the related functions behind a config option that is selected only by drivers that need the infrastructure. The saving are: add/remove: 0/6 grow/shrink: 0/0 up/down: 0/-868 (-868) function old new delta frame_vector_destroy 55 - -55 frame_vector_to_pfns 56 - -56 frame_vector_create 81 - -81 put_vaddr_frames 93 - -93 frame_vector_to_pages 98 - -98 get_vaddr_frames 485 --485 Suggested-by: Andrew Morton Signed-off-by: Jan Kara --- drivers/gpu/drm/exynos/Kconfig | 1 + drivers/media/platform/omap/Kconfig | 1 + drivers/media/v4l2-core/Kconfig | 1 + mm/Kconfig | 3 + mm/Makefile | 1 + mm/frame_vector.c | 231 mm/gup.c| 222 -- 7 files changed, 238 insertions(+), 222 deletions(-) create mode 100644 mm/frame_vector.c diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig index 0a6780367d28..fc678289cf79 100644 --- a/drivers/gpu/drm/exynos/Kconfig +++ b/drivers/gpu/drm/exynos/Kconfig @@ -71,6 +71,7 @@ config DRM_EXYNOS_VIDI config DRM_EXYNOS_G2D bool "Exynos DRM G2D" depends on DRM_EXYNOS && !VIDEO_SAMSUNG_S5P_G2D + select FRAME_VECTOR help Choose this option if you want to use Exynos G2D for DRM. diff --git a/drivers/media/platform/omap/Kconfig b/drivers/media/platform/omap/Kconfig index dc2aaab54aef..217d613b0fe7 100644 --- a/drivers/media/platform/omap/Kconfig +++ b/drivers/media/platform/omap/Kconfig @@ -10,6 +10,7 @@ config VIDEO_OMAP2_VOUT select OMAP2_DSS if HAS_IOMEM && ARCH_OMAP2PLUS select OMAP2_VRFB if ARCH_OMAP2 || ARCH_OMAP3 select VIDEO_OMAP2_VOUT_VRFB if VIDEO_OMAP2_VOUT && OMAP2_VRFB + select FRAME_VECTOR default n ---help--- V4L2 Display driver support for OMAP2/3 based boards. diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig index ba7e21a73023..0cb22add650b 100644 --- a/drivers/media/v4l2-core/Kconfig +++ b/drivers/media/v4l2-core/Kconfig @@ -73,6 +73,7 @@ config VIDEOBUF2_CORE config VIDEOBUF2_MEMOPS tristate + select FRAME_VECTOR config VIDEOBUF2_DMA_CONTIG tristate diff --git a/mm/Kconfig b/mm/Kconfig index 390214da4546..2ca52e9986f0 100644 --- a/mm/Kconfig +++ b/mm/Kconfig @@ -635,3 +635,6 @@ config MAX_STACK_SIZE_MB changed to a smaller value in which case that is used. A sane initial value is 80 MB. + +config FRAME_VECTOR + bool diff --git a/mm/Makefile b/mm/Makefile index 98c4eaeabdcb..be5d5c866305 100644 --- a/mm/Makefile +++ b/mm/Makefile @@ -78,3 +78,4 @@ obj-$(CONFIG_CMA) += cma.o obj-$(CONFIG_MEMORY_BALLOON) += balloon_compaction.o obj-$(CONFIG_PAGE_EXTENSION) += page_ext.o obj-$(CONFIG_CMA_DEBUGFS) += cma_debug.o +obj-$(CONFIG_FRAME_VECTOR) += frame_vector.o diff --git a/mm/frame_vector.c b/mm/frame_vector.c new file mode 100644 index ..f76b579e46f1 --- /dev/null +++ b/mm/frame_vector.c @@ -0,0 +1,231 @@ +#include +#include +#include +#include +#include +#include +#include +#include + +/* + * get_vaddr_frames() - map virtual addresses to pfns + * @start: starting user address + * @nr_frames: number of pages / pfns from start to map + * @write: whether pages will be written to by the caller + * @force: whether to force write access even if user mapping is + * readonly. See description of the same argument of + get_user_pages(). + * @vec: structure which receives pages / pfns of the addresses mapped. + * It should have space for at least nr_frames entries. + * + * This function maps virtual addresses from @start and fills @vec structure + * with page frame numbers or page pointers to corresponding pages (choice + * depends on the type of the vma underlying the virtual address). If @start + * belongs to a normal vma, the function grabs reference to each of the pages + * to pin them in memory. If @start belongs to VM_IO | VM_PFNMAP vma, we don't + * touch page structures and the caller must make sure pfns aren't reused for + * anything else while he is using them. + * + * The function returns number of pages mapped which may be less than + * @nr_frames. In particular we stop mapping if there are more vmas of + * different type underlying the specified range of virtual addresses. + * When the function isn't able to map a single page, it returns error. + * + * This function takes care of grabbing mmap_sem as necessary. + */ +int get_vaddr_frames(unsigned long start, unsigned int nr_frames, +
[PATCH 8/10] media: vb2: Remove unused functions
Conversion to the use of pinned pfns made some functions unused. Remove
them. Also there's no need to lock mmap_sem in __buf_prepare() anymore.
Acked-by: Marek Szyprowski
Tested-by: Marek Szyprowski
Signed-off-by: Jan Kara
---
drivers/media/v4l2-core/videobuf2-memops.c | 114 -
include/media/videobuf2-memops.h | 6 --
2 files changed, 120 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-memops.c
b/drivers/media/v4l2-core/videobuf2-memops.c
index 0ec186d41b9b..48c6a49c4928 100644
--- a/drivers/media/v4l2-core/videobuf2-memops.c
+++ b/drivers/media/v4l2-core/videobuf2-memops.c
@@ -23,120 +23,6 @@
#include
/**
- * vb2_get_vma() - acquire and lock the virtual memory area
- * @vma: given virtual memory area
- *
- * This function attempts to acquire an area mapped in the userspace for
- * the duration of a hardware operation. The area is "locked" by performing
- * the same set of operation that are done when process calls fork() and
- * memory areas are duplicated.
- *
- * Returns a copy of a virtual memory region on success or NULL.
- */
-struct vm_area_struct *vb2_get_vma(struct vm_area_struct *vma)
-{
- struct vm_area_struct *vma_copy;
-
- vma_copy = kmalloc(sizeof(*vma_copy), GFP_KERNEL);
- if (vma_copy == NULL)
- return NULL;
-
- if (vma->vm_ops && vma->vm_ops->open)
- vma->vm_ops->open(vma);
-
- if (vma->vm_file)
- get_file(vma->vm_file);
-
- memcpy(vma_copy, vma, sizeof(*vma));
-
- vma_copy->vm_mm = NULL;
- vma_copy->vm_next = NULL;
- vma_copy->vm_prev = NULL;
-
- return vma_copy;
-}
-EXPORT_SYMBOL_GPL(vb2_get_vma);
-
-/**
- * vb2_put_userptr() - release a userspace virtual memory area
- * @vma: virtual memory region associated with the area to be released
- *
- * This function releases the previously acquired memory area after a hardware
- * operation.
- */
-void vb2_put_vma(struct vm_area_struct *vma)
-{
- if (!vma)
- return;
-
- if (vma->vm_ops && vma->vm_ops->close)
- vma->vm_ops->close(vma);
-
- if (vma->vm_file)
- fput(vma->vm_file);
-
- kfree(vma);
-}
-EXPORT_SYMBOL_GPL(vb2_put_vma);
-
-/**
- * vb2_get_contig_userptr() - lock physically contiguous userspace mapped
memory
- * @vaddr: starting virtual address of the area to be verified
- * @size: size of the area
- * @res_paddr: will return physical address for the given vaddr
- * @res_vma: will return locked copy of struct vm_area for the given area
- *
- * This function will go through memory area of size @size mapped at @vaddr and
- * verify that the underlying physical pages are contiguous. If they are
- * contiguous the virtual memory area is locked and a @res_vma is filled with
- * the copy and @res_pa set to the physical address of the buffer.
- *
- * Returns 0 on success.
- */
-int vb2_get_contig_userptr(unsigned long vaddr, unsigned long size,
- struct vm_area_struct **res_vma, dma_addr_t *res_pa)
-{
- struct mm_struct *mm = current->mm;
- struct vm_area_struct *vma;
- unsigned long offset, start, end;
- unsigned long this_pfn, prev_pfn;
- dma_addr_t pa = 0;
-
- start = vaddr;
- offset = start & ~PAGE_MASK;
- end = start + size;
-
- vma = find_vma(mm, start);
-
- if (vma == NULL || vma->vm_end < end)
- return -EFAULT;
-
- for (prev_pfn = 0; start < end; start += PAGE_SIZE) {
- int ret = follow_pfn(vma, start, &this_pfn);
- if (ret)
- return ret;
-
- if (prev_pfn == 0)
- pa = this_pfn << PAGE_SHIFT;
- else if (this_pfn != prev_pfn + 1)
- return -EFAULT;
-
- prev_pfn = this_pfn;
- }
-
- /*
-* Memory is contigous, lock vma and return to the caller
-*/
- *res_vma = vb2_get_vma(vma);
- if (*res_vma == NULL)
- return -ENOMEM;
-
- *res_pa = pa + offset;
- return 0;
-}
-EXPORT_SYMBOL_GPL(vb2_get_contig_userptr);
-
-/**
* vb2_create_framevec() - map virtual addresses to pfns
* @start: Virtual user address where we start mapping
* @length:Length of a range to map
diff --git a/include/media/videobuf2-memops.h b/include/media/videobuf2-memops.h
index 2f0564ff5f31..830b5239fd8b 100644
--- a/include/media/videobuf2-memops.h
+++ b/include/media/videobuf2-memops.h
@@ -31,12 +31,6 @@ struct vb2_vmarea_handler {
extern const struct vm_operations_struct vb2_common_vm_ops;
-int vb2_get_contig_userptr(unsigned long vaddr, unsigned long size,
- struct vm_area_struct **res_vma, dma_addr_t *res_pa);
-
-struct vm_area_struct *vb2_get_vma(struct vm_area_struct *vma);
-void vb2_put_vma(struct vm_area_struct *vma);
-
struct frame_vector *vb2_create_framevec(unsigned long st
[PATCH 5/10] media: vb2: Convert vb2_dma_sg_get_userptr() to use frame vector
Acked-by: Marek Szyprowski
Tested-by: Marek Szyprowski
Signed-off-by: Jan Kara
---
drivers/media/v4l2-core/videobuf2-dma-sg.c | 95 +-
1 file changed, 15 insertions(+), 80 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index cdcf5ad79012..4ee1b3fbfe2a 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -38,6 +38,7 @@ struct vb2_dma_sg_buf {
struct device *dev;
void*vaddr;
struct page **pages;
+ struct frame_vector *vec;
int offset;
enum dma_data_direction dma_dir;
struct sg_table sg_table;
@@ -51,7 +52,6 @@ struct vb2_dma_sg_buf {
unsigned intnum_pages;
atomic_trefcount;
struct vb2_vmarea_handler handler;
- struct vm_area_struct *vma;
struct dma_buf_attachment *db_attach;
};
@@ -224,25 +224,17 @@ static void vb2_dma_sg_finish(void *buf_priv)
dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
}
-static inline int vma_is_io(struct vm_area_struct *vma)
-{
- return !!(vma->vm_flags & (VM_IO | VM_PFNMAP));
-}
-
static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
unsigned long size,
enum dma_data_direction dma_dir)
{
struct vb2_dma_sg_conf *conf = alloc_ctx;
struct vb2_dma_sg_buf *buf;
- unsigned long first, last;
- int num_pages_from_user;
- struct vm_area_struct *vma;
struct sg_table *sgt;
DEFINE_DMA_ATTRS(attrs);
+ struct frame_vector *vec;
dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
-
buf = kzalloc(sizeof *buf, GFP_KERNEL);
if (!buf)
return NULL;
@@ -253,63 +245,19 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
unsigned long vaddr,
buf->offset = vaddr & ~PAGE_MASK;
buf->size = size;
buf->dma_sgt = &buf->sg_table;
+ vec = vb2_create_framevec(vaddr, size, buf->dma_dir == DMA_FROM_DEVICE);
+ if (IS_ERR(vec))
+ goto userptr_fail_pfnvec;
+ buf->vec = vec;
- first = (vaddr & PAGE_MASK) >> PAGE_SHIFT;
- last = ((vaddr + size - 1) & PAGE_MASK) >> PAGE_SHIFT;
- buf->num_pages = last - first + 1;
-
- buf->pages = kzalloc(buf->num_pages * sizeof(struct page *),
-GFP_KERNEL);
- if (!buf->pages)
- goto userptr_fail_alloc_pages;
-
- down_read(¤t->mm->mmap_sem);
- vma = find_vma(current->mm, vaddr);
- if (!vma) {
- dprintk(1, "no vma for address %lu\n", vaddr);
- goto userptr_fail_find_vma;
- }
-
- if (vma->vm_end < vaddr + size) {
- dprintk(1, "vma at %lu is too small for %lu bytes\n",
- vaddr, size);
- goto userptr_fail_find_vma;
- }
-
- buf->vma = vb2_get_vma(vma);
- if (!buf->vma) {
- dprintk(1, "failed to copy vma\n");
- goto userptr_fail_find_vma;
- }
-
- if (vma_is_io(buf->vma)) {
- for (num_pages_from_user = 0;
-num_pages_from_user < buf->num_pages;
-++num_pages_from_user, vaddr += PAGE_SIZE) {
- unsigned long pfn;
-
- if (follow_pfn(vma, vaddr, &pfn)) {
- dprintk(1, "no page for address %lu\n", vaddr);
- break;
- }
- buf->pages[num_pages_from_user] = pfn_to_page(pfn);
- }
- } else
- num_pages_from_user = get_user_pages(current, current->mm,
-vaddr & PAGE_MASK,
-buf->num_pages,
-buf->dma_dir == DMA_FROM_DEVICE,
-1, /* force */
-buf->pages,
-NULL);
- up_read(¤t->mm->mmap_sem);
-
- if (num_pages_from_user != buf->num_pages)
- goto userptr_fail_get_user_pages;
+ buf->pages = frame_vector_pages(vec);
+ if (IS_ERR(buf->pages))
+ goto userptr_fail_sgtable;
+ buf->num_pages = frame_vector_count(vec);
if (sg_alloc_table_from_pages(buf->dma_sgt, buf->pages,
buf->num_pages, buf->offset, size, 0))
- goto userptr_fail_alloc_table_from_pages;
+ goto userptr_fail_sgtable;
sgt = &buf->sg_table;
/*
@@ -323,19 +271,9 @@ static void
[PATCH 3/10] media: omap_vout: Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns()
Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns() instead of
hand made mapping of virtual address to physical address. Also the
function leaked page reference from get_user_pages() so fix that by
properly release the reference when omap_vout_buffer_release() is
called.
Signed-off-by: Jan Kara
---
drivers/media/platform/omap/omap_vout.c | 67 +++--
1 file changed, 31 insertions(+), 36 deletions(-)
diff --git a/drivers/media/platform/omap/omap_vout.c
b/drivers/media/platform/omap/omap_vout.c
index 17b189a81ec5..0e4b3cfacc5d 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -195,46 +195,34 @@ static int omap_vout_try_format(struct v4l2_pix_format
*pix)
}
/*
- * omap_vout_uservirt_to_phys: This inline function is used to convert user
- * space virtual address to physical address.
+ * omap_vout_get_userptr: Convert user space virtual address to physical
+ * address.
*/
-static unsigned long omap_vout_uservirt_to_phys(unsigned long virtp)
+static int omap_vout_get_userptr(struct videobuf_buffer *vb, u32 virtp,
+u32 *physp)
{
- unsigned long physp = 0;
- struct vm_area_struct *vma;
- struct mm_struct *mm = current->mm;
+ struct frame_vector *vec;
+ int ret;
/* For kernel direct-mapped memory, take the easy way */
- if (virtp >= PAGE_OFFSET)
- return virt_to_phys((void *) virtp);
-
- down_read(¤t->mm->mmap_sem);
- vma = find_vma(mm, virtp);
- if (vma && (vma->vm_flags & VM_IO) && vma->vm_pgoff) {
- /* this will catch, kernel-allocated, mmaped-to-usermode
- addresses */
- physp = (vma->vm_pgoff << PAGE_SHIFT) + (virtp - vma->vm_start);
- up_read(¤t->mm->mmap_sem);
- } else {
- /* otherwise, use get_user_pages() for general userland pages */
- int res, nr_pages = 1;
- struct page *pages;
+ if (virtp >= PAGE_OFFSET) {
+ *physp = virt_to_phys((void *)virtp);
+ return 0;
+ }
- res = get_user_pages(current, current->mm, virtp, nr_pages, 1,
- 0, &pages, NULL);
- up_read(¤t->mm->mmap_sem);
+ vec = frame_vector_create(1);
+ if (!vec)
+ return -ENOMEM;
- if (res == nr_pages) {
- physp = __pa(page_address(&pages[0]) +
- (virtp & ~PAGE_MASK));
- } else {
- printk(KERN_WARNING VOUT_NAME
- "get_user_pages failed\n");
- return 0;
- }
+ ret = get_vaddr_frames(virtp, 1, true, false, vec);
+ if (ret != 1) {
+ frame_vector_destroy(vec);
+ return -EINVAL;
}
+ *physp = __pfn_to_phys(frame_vector_pfns(vec)[0]);
+ vb->priv = vec;
- return physp;
+ return 0;
}
/*
@@ -788,11 +776,15 @@ static int omap_vout_buffer_prepare(struct videobuf_queue
*q,
* address of the buffer
*/
if (V4L2_MEMORY_USERPTR == vb->memory) {
+ int ret;
+
if (0 == vb->baddr)
return -EINVAL;
/* Physical address */
- vout->queued_buf_addr[vb->i] = (u8 *)
- omap_vout_uservirt_to_phys(vb->baddr);
+ ret = omap_vout_get_userptr(vb, vb->baddr,
+ (u32 *)&vout->queued_buf_addr[vb->i]);
+ if (ret < 0)
+ return ret;
} else {
unsigned long addr, dma_addr;
unsigned long size;
@@ -841,9 +833,12 @@ static void omap_vout_buffer_release(struct videobuf_queue
*q,
struct omap_vout_device *vout = q->priv_data;
vb->state = VIDEOBUF_NEEDS_INIT;
+ if (vb->memory == V4L2_MEMORY_USERPTR && vb->priv) {
+ struct frame_vector *vec = vb->priv;
- if (V4L2_MEMORY_MMAP != vout->memory)
- return;
+ put_vaddr_frames(vec);
+ frame_vector_destroy(vec);
+ }
}
/*
--
2.1.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/10] media: vb2: Convert vb2_vmalloc_get_userptr() to use frame vector
Convert vb2_vmalloc_get_userptr() to use frame vector infrastructure.
When we are doing that there's no need to allocate page array and some
code can be simplified.
Acked-by: Marek Szyprowski
Tested-by: Marek Szyprowski
Signed-off-by: Jan Kara
---
drivers/media/v4l2-core/videobuf2-vmalloc.c | 92 +++--
1 file changed, 36 insertions(+), 56 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c
b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index 3199c379cd47..d2ce81fa2cdf 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -23,11 +23,9 @@
struct vb2_vmalloc_buf {
void*vaddr;
- struct page **pages;
- struct vm_area_struct *vma;
+ struct frame_vector *vec;
enum dma_data_direction dma_dir;
unsigned long size;
- unsigned intn_pages;
atomic_trefcount;
struct vb2_vmarea_handler handler;
struct dma_buf *dbuf;
@@ -76,10 +74,8 @@ static void *vb2_vmalloc_get_userptr(void *alloc_ctx,
unsigned long vaddr,
enum dma_data_direction dma_dir)
{
struct vb2_vmalloc_buf *buf;
- unsigned long first, last;
- int n_pages, offset;
- struct vm_area_struct *vma;
- dma_addr_t physp;
+ struct frame_vector *vec;
+ int n_pages, offset, i;
buf = kzalloc(sizeof(*buf), GFP_KERNEL);
if (!buf)
@@ -88,53 +84,36 @@ static void *vb2_vmalloc_get_userptr(void *alloc_ctx,
unsigned long vaddr,
buf->dma_dir = dma_dir;
offset = vaddr & ~PAGE_MASK;
buf->size = size;
-
- down_read(¤t->mm->mmap_sem);
- vma = find_vma(current->mm, vaddr);
- if (vma && (vma->vm_flags & VM_PFNMAP) && (vma->vm_pgoff)) {
- if (vb2_get_contig_userptr(vaddr, size, &vma, &physp))
- goto fail_pages_array_alloc;
- buf->vma = vma;
- buf->vaddr = (__force void *)ioremap_nocache(physp, size);
- if (!buf->vaddr)
- goto fail_pages_array_alloc;
+ vec = vb2_create_framevec(vaddr, size, dma_dir == DMA_FROM_DEVICE);
+ if (IS_ERR(vec))
+ goto fail_pfnvec_create;
+ buf->vec = vec;
+ n_pages = frame_vector_count(vec);
+ if (frame_vector_to_pages(vec) < 0) {
+ unsigned long *nums = frame_vector_pfns(vec);
+
+ /*
+* We cannot get page pointers for these pfns. Check memory is
+* physically contiguous and use direct mapping.
+*/
+ for (i = 1; i < n_pages; i++)
+ if (nums[i-1] + 1 != nums[i])
+ goto fail_map;
+ buf->vaddr = (__force void *)
+ ioremap_nocache(nums[0] << PAGE_SHIFT, size);
} else {
- first = vaddr >> PAGE_SHIFT;
- last = (vaddr + size - 1) >> PAGE_SHIFT;
- buf->n_pages = last - first + 1;
- buf->pages = kzalloc(buf->n_pages * sizeof(struct page *),
-GFP_KERNEL);
- if (!buf->pages)
- goto fail_pages_array_alloc;
-
- /* current->mm->mmap_sem is taken by videobuf2 core */
- n_pages = get_user_pages(current, current->mm,
-vaddr & PAGE_MASK, buf->n_pages,
-dma_dir == DMA_FROM_DEVICE,
-1, /* force */
-buf->pages, NULL);
- if (n_pages != buf->n_pages)
- goto fail_get_user_pages;
-
- buf->vaddr = vm_map_ram(buf->pages, buf->n_pages, -1,
+ buf->vaddr = vm_map_ram(frame_vector_pages(vec), n_pages, -1,
PAGE_KERNEL);
- if (!buf->vaddr)
- goto fail_get_user_pages;
}
- up_read(¤t->mm->mmap_sem);
+ if (!buf->vaddr)
+ goto fail_map;
buf->vaddr += offset;
return buf;
-fail_get_user_pages:
- pr_debug("get_user_pages requested/got: %d/%d]\n", n_pages,
-buf->n_pages);
- while (--n_pages >= 0)
- put_page(buf->pages[n_pages]);
- kfree(buf->pages);
-
-fail_pages_array_alloc:
- up_read(¤t->mm->mmap_sem);
+fail_map:
+ vb2_destroy_framevec(vec);
+fail_pfnvec_create:
kfree(buf);
return NULL;
@@ -145,20 +124,21 @@ static void vb2_vmalloc_put_userptr(void *buf_priv)
struct vb2_vmalloc_buf *buf = buf_priv;
unsigned long vaddr = (unsigned long)buf->vaddr & PAGE_MASK;
unsigned int i;
+ struct page
[PATCH 7/10] media: vb2: Convert vb2_dc_get_userptr() to use frame vector
Convert vb2_dc_get_userptr() to use frame vector infrastructure. When we
are doing that there's no need to allocate page array and some code can
be simplified.
Acked-by: Marek Szyprowski
Tested-by: Marek Szyprowski
Signed-off-by: Jan Kara
---
drivers/media/v4l2-core/videobuf2-dma-contig.c | 212 -
1 file changed, 34 insertions(+), 178 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 8e660f033d3c..e6cea452302b 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -32,15 +32,13 @@ struct vb2_dc_buf {
dma_addr_t dma_addr;
enum dma_data_direction dma_dir;
struct sg_table *dma_sgt;
+ struct frame_vector *vec;
/* MMAP related */
struct vb2_vmarea_handler handler;
atomic_trefcount;
struct sg_table *sgt_base;
- /* USERPTR related */
- struct vm_area_struct *vma;
-
/* DMABUF related */
struct dma_buf_attachment *db_attach;
};
@@ -49,24 +47,6 @@ struct vb2_dc_buf {
/*scatterlist table functions*/
/*/
-
-static void vb2_dc_sgt_foreach_page(struct sg_table *sgt,
- void (*cb)(struct page *pg))
-{
- struct scatterlist *s;
- unsigned int i;
-
- for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
- struct page *page = sg_page(s);
- unsigned int n_pages = PAGE_ALIGN(s->offset + s->length)
- >> PAGE_SHIFT;
- unsigned int j;
-
- for (j = 0; j < n_pages; ++j, ++page)
- cb(page);
- }
-}
-
static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt)
{
struct scatterlist *s;
@@ -429,92 +409,12 @@ static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv,
unsigned long flags)
/* callbacks for USERPTR buffers */
/*/
-static inline int vma_is_io(struct vm_area_struct *vma)
-{
- return !!(vma->vm_flags & (VM_IO | VM_PFNMAP));
-}
-
-static int vb2_dc_get_user_pfn(unsigned long start, int n_pages,
- struct vm_area_struct *vma, unsigned long *res)
-{
- unsigned long pfn, start_pfn, prev_pfn;
- unsigned int i;
- int ret;
-
- if (!vma_is_io(vma))
- return -EFAULT;
-
- ret = follow_pfn(vma, start, &pfn);
- if (ret)
- return ret;
-
- start_pfn = pfn;
- start += PAGE_SIZE;
-
- for (i = 1; i < n_pages; ++i, start += PAGE_SIZE) {
- prev_pfn = pfn;
- ret = follow_pfn(vma, start, &pfn);
-
- if (ret) {
- pr_err("no page for address %lu\n", start);
- return ret;
- }
- if (pfn != prev_pfn + 1)
- return -EINVAL;
- }
-
- *res = start_pfn;
- return 0;
-}
-
-static int vb2_dc_get_user_pages(unsigned long start, struct page **pages,
- int n_pages, struct vm_area_struct *vma,
- enum dma_data_direction dma_dir)
-{
- if (vma_is_io(vma)) {
- unsigned int i;
-
- for (i = 0; i < n_pages; ++i, start += PAGE_SIZE) {
- unsigned long pfn;
- int ret = follow_pfn(vma, start, &pfn);
-
- if (!pfn_valid(pfn))
- return -EINVAL;
-
- if (ret) {
- pr_err("no page for address %lu\n", start);
- return ret;
- }
- pages[i] = pfn_to_page(pfn);
- }
- } else {
- int n;
-
- n = get_user_pages(current, current->mm, start & PAGE_MASK,
- n_pages, dma_dir == DMA_FROM_DEVICE, 1, pages, NULL);
- /* negative error means that no page was pinned */
- n = max(n, 0);
- if (n != n_pages) {
- pr_err("got only %d of %d user pages\n", n, n_pages);
- while (n)
- put_page(pages[--n]);
- return -EFAULT;
- }
- }
-
- return 0;
-}
-
-static void vb2_dc_put_dirty_page(struct page *page)
-{
- set_page_dirty_lock(page);
- put_page(page);
-}
-
static void vb2_dc_put_userptr(void *buf_priv)
{
struct vb2_dc_buf *buf = buf_priv;
struct sg_table *sgt = buf->dma_sgt;
+ int i;
+ struct page **pages;
if (sgt) {
DEFINE_DMA_ATTRS(attrs);
@@ -526,13 +426,15 @@ static void vb2_dc_put_userptr(void *buf_priv)
*/
dma_unmap_sg_attrs(buf->dev,
[PATCH 1/10] [media] vb2: Push mmap_sem down to memops
Currently vb2 core acquires mmap_sem just around call to
__qbuf_userptr(). However since commit f035eb4e976ef5 (videobuf2: fix
lockdep warning) it isn't necessary to acquire it so early as we no
longer have to drop queue mutex before acquiring mmap_sem. So push
acquisition of mmap_sem down into .get_userptr memop so that the
semaphore is acquired for a shorter time and it is clearer what it is
needed for.
Note that we also need mmap_sem in .put_userptr memop since that ends up
calling vb2_put_vma() which calls vma->vm_ops->close() which should be
called with mmap_sem held. However we didn't hold mmap_sem in some code
paths anyway (e.g. when called via vb2_ioctl_reqbufs() ->
__vb2_queue_free() -> vb2_dma_sg_put_userptr()) and getting mmap_sem in
put_userptr() introduces a lock inversion with queue->mmap_lock in the
above mentioned call path.
Luckily this whole locking mess will get resolved once we convert
videobuf2 core to the new mm helper which avoids the need for mmap_sem
in .put_userptr memop altogether.
Signed-off-by: Jan Kara
---
drivers/media/v4l2-core/videobuf2-core.c | 2 --
drivers/media/v4l2-core/videobuf2-dma-contig.c | 5 +
drivers/media/v4l2-core/videobuf2-dma-sg.c | 4
drivers/media/v4l2-core/videobuf2-vmalloc.c| 4 +++-
4 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 66ada01c796c..20cdbc0900ea 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1657,9 +1657,7 @@ static int __buf_prepare(struct vb2_buffer *vb, const
struct v4l2_buffer *b)
ret = __qbuf_mmap(vb, b);
break;
case V4L2_MEMORY_USERPTR:
- down_read(¤t->mm->mmap_sem);
ret = __qbuf_userptr(vb, b);
- up_read(¤t->mm->mmap_sem);
break;
case V4L2_MEMORY_DMABUF:
ret = __qbuf_dmabuf(vb, b);
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 644dec73d220..8e660f033d3c 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -616,6 +616,7 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned
long vaddr,
goto fail_buf;
}
+ down_read(¤t->mm->mmap_sem);
/* current->mm->mmap_sem is taken by videobuf2 core */
vma = find_vma(current->mm, vaddr);
if (!vma) {
@@ -642,6 +643,7 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned
long vaddr,
if (ret) {
unsigned long pfn;
if (vb2_dc_get_user_pfn(start, n_pages, vma, &pfn) == 0) {
+ up_read(¤t->mm->mmap_sem);
buf->dma_addr = vb2_dc_pfn_to_dma(buf->dev, pfn);
buf->size = size;
kfree(pages);
@@ -651,6 +653,7 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned
long vaddr,
pr_err("failed to get user pages\n");
goto fail_vma;
}
+ up_read(¤t->mm->mmap_sem);
sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
if (!sgt) {
@@ -713,10 +716,12 @@ fail_get_user_pages:
while (n_pages)
put_page(pages[--n_pages]);
+ down_read(¤t->mm->mmap_sem);
fail_vma:
vb2_put_vma(buf->vma);
fail_pages:
+ up_read(¤t->mm->mmap_sem);
kfree(pages); /* kfree is NULL-proof */
fail_buf:
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 45c708e463b9..cdcf5ad79012 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -263,6 +263,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
unsigned long vaddr,
if (!buf->pages)
goto userptr_fail_alloc_pages;
+ down_read(¤t->mm->mmap_sem);
vma = find_vma(current->mm, vaddr);
if (!vma) {
dprintk(1, "no vma for address %lu\n", vaddr);
@@ -301,6 +302,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
unsigned long vaddr,
1, /* force */
buf->pages,
NULL);
+ up_read(¤t->mm->mmap_sem);
if (num_pages_from_user != buf->num_pages)
goto userptr_fail_get_user_pages;
@@ -328,8 +330,10 @@ userptr_fail_get_user_pages:
if (!vma_is_io(buf->vma))
while (--num_pages_from_user >= 0)
put_page(buf->pages[num_pages_from_user]);
+ down_read(¤t->mm->mmap_sem);
vb2_put_vma(buf->vma);
userptr_fail_find_vma:
+ up_read(¤t->mm->mmap_sem);
kfree(buf->pages);
userptr_fail_alloc_pages:
kfree(buf);
diff --git a/drivers/media/v4l2-
[PATCH 9/10] drm/exynos: Convert g2d_userptr_get_dma_addr() to use get_vaddr_frames()
Convert g2d_userptr_get_dma_addr() to pin pages using get_vaddr_frames().
This removes the knowledge about vmas and mmap_sem locking from exynos
driver. Also it fixes a problem that the function has been mapping user
provided address without holding mmap_sem.
Signed-off-by: Jan Kara
---
drivers/gpu/drm/exynos/exynos_drm_g2d.c | 91 ++-
drivers/gpu/drm/exynos/exynos_drm_gem.c | 97 -
2 files changed, 29 insertions(+), 159 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 81a250830808..810e1ee7c07d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -190,10 +190,8 @@ struct g2d_cmdlist_userptr {
dma_addr_t dma_addr;
unsigned long userptr;
unsigned long size;
- struct page **pages;
- unsigned intnpages;
+ struct frame_vector *vec;
struct sg_table *sgt;
- struct vm_area_struct *vma;
atomic_trefcount;
boolin_pool;
boolout_of_list;
@@ -363,6 +361,7 @@ static void g2d_userptr_put_dma_addr(struct drm_device
*drm_dev,
{
struct g2d_cmdlist_userptr *g2d_userptr =
(struct g2d_cmdlist_userptr *)obj;
+ struct page **pages;
if (!obj)
return;
@@ -382,19 +381,21 @@ out:
exynos_gem_unmap_sgt_from_dma(drm_dev, g2d_userptr->sgt,
DMA_BIDIRECTIONAL);
- exynos_gem_put_pages_to_userptr(g2d_userptr->pages,
- g2d_userptr->npages,
- g2d_userptr->vma);
+ pages = frame_vector_pages(g2d_userptr->vec);
+ if (!IS_ERR(pages)) {
+ int i;
- exynos_gem_put_vma(g2d_userptr->vma);
+ for (i = 0; i < frame_vector_count(g2d_userptr->vec); i++)
+ set_page_dirty_lock(pages[i]);
+ }
+ put_vaddr_frames(g2d_userptr->vec);
+ frame_vector_destroy(g2d_userptr->vec);
if (!g2d_userptr->out_of_list)
list_del_init(&g2d_userptr->list);
sg_free_table(g2d_userptr->sgt);
kfree(g2d_userptr->sgt);
-
- drm_free_large(g2d_userptr->pages);
kfree(g2d_userptr);
}
@@ -408,9 +409,7 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct
drm_device *drm_dev,
struct exynos_drm_g2d_private *g2d_priv = file_priv->g2d_priv;
struct g2d_cmdlist_userptr *g2d_userptr;
struct g2d_data *g2d;
- struct page **pages;
struct sg_table *sgt;
- struct vm_area_struct *vma;
unsigned long start, end;
unsigned int npages, offset;
int ret;
@@ -456,65 +455,38 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct
drm_device *drm_dev,
return ERR_PTR(-ENOMEM);
atomic_set(&g2d_userptr->refcount, 1);
+ g2d_userptr->size = size;
start = userptr & PAGE_MASK;
offset = userptr & ~PAGE_MASK;
end = PAGE_ALIGN(userptr + size);
npages = (end - start) >> PAGE_SHIFT;
- g2d_userptr->npages = npages;
-
- pages = drm_calloc_large(npages, sizeof(struct page *));
- if (!pages) {
- DRM_ERROR("failed to allocate pages.\n");
- ret = -ENOMEM;
+ g2d_userptr->vec = frame_vector_create(npages);
+ if (!vec)
goto err_free;
- }
- down_read(¤t->mm->mmap_sem);
- vma = find_vma(current->mm, userptr);
- if (!vma) {
- up_read(¤t->mm->mmap_sem);
- DRM_ERROR("failed to get vm region.\n");
+ ret = get_vaddr_frames(start, npages, true, true, g2d_userptr->vec);
+ if (ret != npages) {
+ DRM_ERROR("failed to get user pages from userptr.\n");
+ if (ret < 0)
+ goto err_destroy_framevec;
ret = -EFAULT;
- goto err_free_pages;
+ goto err_put_framevec;
}
-
- if (vma->vm_end < userptr + size) {
- up_read(¤t->mm->mmap_sem);
- DRM_ERROR("vma is too small.\n");
+ if (frame_vector_to_pages(g2d_userptr->vec) < 0) {
ret = -EFAULT;
- goto err_free_pages;
+ goto err_put_framevec;
}
- g2d_userptr->vma = exynos_gem_get_vma(vma);
- if (!g2d_userptr->vma) {
- up_read(¤t->mm->mmap_sem);
- DRM_ERROR("failed to copy vma.\n");
- ret = -ENOMEM;
- goto err_free_pages;
- }
-
- g2d_userptr->size = size;
-
- ret = exynos_gem_get_pages_from_userptr(start & PAGE_MASK,
- npages, pages, vma);
- if (ret < 0) {
- up_read(¤t-
[PATCH 2/10] mm: Provide new get_vaddr_frames() helper
Provide new function get_vaddr_frames(). This function maps virtual
addresses from given start and fills given array with page frame numbers of
the corresponding pages. If given start belongs to a normal vma, the function
grabs reference to each of the pages to pin them in memory. If start
belongs to VM_IO | VM_PFNMAP vma, we don't touch page structures. Caller
must make sure pfns aren't reused for anything else while he is using
them.
This function is created for various drivers to simplify handling of
their buffers.
Acked-by: Mel Gorman
Acked-by: Vlastimil Babka
Signed-off-by: Jan Kara
---
include/linux/mm.h | 44 +++
mm/gup.c | 223 +
2 files changed, 267 insertions(+)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0755b9fd03a7..dcd1f02a78e9 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
struct mempolicy;
struct anon_vma;
@@ -1197,6 +1198,49 @@ long get_user_pages_unlocked(struct task_struct *tsk,
struct mm_struct *mm,
int write, int force, struct page **pages);
int get_user_pages_fast(unsigned long start, int nr_pages, int write,
struct page **pages);
+
+/* Container for pinned pfns / pages */
+struct frame_vector {
+ unsigned int nr_allocated; /* Number of frames we have space for */
+ unsigned int nr_frames; /* Number of frames stored in ptrs array */
+ bool got_ref; /* Did we pin pages by getting page ref? */
+ bool is_pfns; /* Does array contain pages or pfns? */
+ void *ptrs[0]; /* Array of pinned pfns / pages. Use
+* pfns_vector_pages() or pfns_vector_pfns()
+* for access */
+};
+
+struct frame_vector *frame_vector_create(unsigned int nr_frames);
+void frame_vector_destroy(struct frame_vector *vec);
+int get_vaddr_frames(unsigned long start, unsigned int nr_pfns,
+bool write, bool force, struct frame_vector *vec);
+void put_vaddr_frames(struct frame_vector *vec);
+int frame_vector_to_pages(struct frame_vector *vec);
+void frame_vector_to_pfns(struct frame_vector *vec);
+
+static inline unsigned int frame_vector_count(struct frame_vector *vec)
+{
+ return vec->nr_frames;
+}
+
+static inline struct page **frame_vector_pages(struct frame_vector *vec)
+{
+ if (vec->is_pfns) {
+ int err = frame_vector_to_pages(vec);
+
+ if (err)
+ return ERR_PTR(err);
+ }
+ return (struct page **)(vec->ptrs);
+}
+
+static inline unsigned long *frame_vector_pfns(struct frame_vector *vec)
+{
+ if (!vec->is_pfns)
+ frame_vector_to_pfns(vec);
+ return (unsigned long *)(vec->ptrs);
+}
+
struct kvec;
int get_kernel_pages(const struct kvec *iov, int nr_pages, int write,
struct page **pages);
diff --git a/mm/gup.c b/mm/gup.c
index 6297f6bccfb1..a7a4ac6ae9d0 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
@@ -936,6 +937,228 @@ int __mm_populate(unsigned long start, unsigned long len,
int ignore_errors)
return ret; /* 0 or negative error code */
}
+/*
+ * get_vaddr_frames() - map virtual addresses to pfns
+ * @start: starting user address
+ * @nr_frames: number of pages / pfns from start to map
+ * @write: whether pages will be written to by the caller
+ * @force: whether to force write access even if user mapping is
+ * readonly. See description of the same argument of
+ get_user_pages().
+ * @vec: structure which receives pages / pfns of the addresses mapped.
+ * It should have space for at least nr_frames entries.
+ *
+ * This function maps virtual addresses from @start and fills @vec structure
+ * with page frame numbers or page pointers to corresponding pages (choice
+ * depends on the type of the vma underlying the virtual address). If @start
+ * belongs to a normal vma, the function grabs reference to each of the pages
+ * to pin them in memory. If @start belongs to VM_IO | VM_PFNMAP vma, we don't
+ * touch page structures and the caller must make sure pfns aren't reused for
+ * anything else while he is using them.
+ *
+ * The function returns number of pages mapped which may be less than
+ * @nr_frames. In particular we stop mapping if there are more vmas of
+ * different type underlying the specified range of virtual addresses.
+ * When the function isn't able to map a single page, it returns error.
+ *
+ * This function takes care of grabbing mmap_sem as necessary.
+ */
+int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
+bool write, bool force, struct frame_vector *vec)
+{
+ struct mm_struct *mm = current->mm;
+ struct vm_area_struct *vma;
+ in
[PATCH 4/10] vb2: Provide helpers for mapping virtual addresses
Provide simple helper functions to map virtual address range into an
array of pfns / pages.
Acked-by: Marek Szyprowski
Tested-by: Marek Szyprowski
Signed-off-by: Jan Kara
---
drivers/media/v4l2-core/videobuf2-memops.c | 58 ++
include/media/videobuf2-memops.h | 5 +++
2 files changed, 63 insertions(+)
diff --git a/drivers/media/v4l2-core/videobuf2-memops.c
b/drivers/media/v4l2-core/videobuf2-memops.c
index 81c1ad8b2cf1..0ec186d41b9b 100644
--- a/drivers/media/v4l2-core/videobuf2-memops.c
+++ b/drivers/media/v4l2-core/videobuf2-memops.c
@@ -137,6 +137,64 @@ int vb2_get_contig_userptr(unsigned long vaddr, unsigned
long size,
EXPORT_SYMBOL_GPL(vb2_get_contig_userptr);
/**
+ * vb2_create_framevec() - map virtual addresses to pfns
+ * @start: Virtual user address where we start mapping
+ * @length:Length of a range to map
+ * @write: Should we map for writing into the area
+ *
+ * This function allocates and fills in a vector with pfns corresponding to
+ * virtual address range passed in arguments. If pfns have corresponding pages,
+ * page references are also grabbed to pin pages in memory. The function
+ * returns pointer to the vector on success and error pointer in case of
+ * failure. Returned vector needs to be freed via vb2_destroy_pfnvec().
+ */
+struct frame_vector *vb2_create_framevec(unsigned long start,
+unsigned long length,
+bool write)
+{
+ int ret;
+ unsigned long first, last;
+ unsigned long nr;
+ struct frame_vector *vec;
+
+ first = start >> PAGE_SHIFT;
+ last = (start + length - 1) >> PAGE_SHIFT;
+ nr = last - first + 1;
+ vec = frame_vector_create(nr);
+ if (!vec)
+ return ERR_PTR(-ENOMEM);
+ ret = get_vaddr_frames(start, nr, write, 1, vec);
+ if (ret < 0)
+ goto out_destroy;
+ /* We accept only complete set of PFNs */
+ if (ret != nr) {
+ ret = -EFAULT;
+ goto out_release;
+ }
+ return vec;
+out_release:
+ put_vaddr_frames(vec);
+out_destroy:
+ frame_vector_destroy(vec);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(vb2_create_framevec);
+
+/**
+ * vb2_destroy_framevec() - release vector of mapped pfns
+ * @vec: vector of pfns / pages to release
+ *
+ * This releases references to all pages in the vector @vec (if corresponding
+ * pfns are backed by pages) and frees the passed vector.
+ */
+void vb2_destroy_framevec(struct frame_vector *vec)
+{
+ put_vaddr_frames(vec);
+ frame_vector_destroy(vec);
+}
+EXPORT_SYMBOL(vb2_destroy_framevec);
+
+/**
* vb2_common_vm_open() - increase refcount of the vma
* @vma: virtual memory region for the mapping
*
diff --git a/include/media/videobuf2-memops.h b/include/media/videobuf2-memops.h
index f05444ca8c0c..2f0564ff5f31 100644
--- a/include/media/videobuf2-memops.h
+++ b/include/media/videobuf2-memops.h
@@ -15,6 +15,7 @@
#define _MEDIA_VIDEOBUF2_MEMOPS_H
#include
+#include
/**
* vb2_vmarea_handler - common vma refcount tracking handler
@@ -36,5 +37,9 @@ int vb2_get_contig_userptr(unsigned long vaddr, unsigned long
size,
struct vm_area_struct *vb2_get_vma(struct vm_area_struct *vma);
void vb2_put_vma(struct vm_area_struct *vma);
+struct frame_vector *vb2_create_framevec(unsigned long start,
+unsigned long length,
+bool write);
+void vb2_destroy_framevec(struct frame_vector *vec);
#endif
--
2.1.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 0/10 v6] Helper to abstract vma handling in media layer
Hello, I'm sending the sixth version of my patch series to abstract vma handling from the various media drivers. Since the previous version I have added a patch to move mm helpers into a separate file and behind a config option. I also changed patch pushing mmap_sem down in videobuf2 core to avoid lockdep warning and NULL dereference Hans found in his testing. I've also included small fixups Andrew was carrying. After this patch set drivers have to know much less details about vmas, their types, and locking. Also quite some code is removed from them. As a bonus drivers get automatically VM_FAULT_RETRY handling. The primary motivation for this series is to remove knowledge about mmap_sem locking from as many places a possible so that we can change it with reasonable effort. The core of the series is the new helper get_vaddr_frames() which is given a virtual address and it fills in PFNs / struct page pointers (depending on VMA type) into the provided array. If PFNs correspond to normal pages it also grabs references to these pages. The difference from get_user_pages() is that this function can also deal with pfnmap, and io mappings which is what the media drivers need. I have tested the patches with vivid driver so at least vb2 code got some exposure. Conversion of other drivers was just compile-tested (for x86 so e.g. exynos driver which is only for Samsung platform is completely untested). Andrew, can you please update the patches in mm three? Thanks! Honza Changes since v5: * Moved mm helper into a separate file and behind a config option * Changed the first patch pushing mmap_sem down in videobuf2 core to avoid possible deadlock Changes since v4: * Minor cleanups and fixes pointed out by Mel and Vlasta * Added Acked-by tags Changes since v3: * Added include into mm/gup.c as it's needed for some archs * Fixed error path for exynos driver Changes since v2: * Renamed functions and structures as Mel suggested * Other minor changes suggested by Mel * Rebased on top of 4.1-rc2 * Changed functions to get pointer to array of pages / pfns to perform conversion if necessary. This fixes possible issue in the omap I may have introduced in v2 and generally makes the API less errorprone. -- 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 9/9] [media] mm: Move get_vaddr_frames() behind a config option
On Wed 10-06-15 09:37:20, Josh Triplett wrote: > On Wed, Jun 10, 2015 at 06:20:52AM -0300, Mauro Carvalho Chehab wrote: > > From: Jan Kara > > > > get_vaddr_frames() is used by relatively rare drivers so hide it and the > > related functions behind a config option that is selected only by > > drivers that need the infrastructure. > > > > Suggested-by: Andrew Morton > > > > Signed-off-by: Jan Kara > > Signed-off-by: Hans Verkuil > > Signed-off-by: Mauro Carvalho Chehab > > Seems sensible to me. > > Since this patch makes the kernel smaller, can you include the delta > from bloat-o-meter between allnoconfig with and without this patch? The results are: add/remove: 0/6 grow/shrink: 0/0 up/down: 0/-868 (-868) function old new delta frame_vector_destroy 55 - -55 frame_vector_to_pfns 56 - -56 frame_vector_create 81 - -81 put_vaddr_frames 93 - -93 frame_vector_to_pages 98 - -98 get_vaddr_frames 485 --485 I've added it to the changelog of the patch. > Also, I assume you've compile-tested the kernel with allyesconfig minus > the three options that now have "select FRAME_VECTOR", to make sure it > builds? I did not because the config option VIDEOBUF2_MEMOPS is a virtual one selected transitively by quite a few video drivers and I didn't bother with tracking down all of them... But since that config option guards compilation of the code I modified I'm pretty confident I got it right. Honza > > create mode 100644 mm/frame_vector.c > > > > diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig > > index 0a6780367d28..fc678289cf79 100644 > > --- a/drivers/gpu/drm/exynos/Kconfig > > +++ b/drivers/gpu/drm/exynos/Kconfig > > @@ -71,6 +71,7 @@ config DRM_EXYNOS_VIDI > > config DRM_EXYNOS_G2D > > bool "Exynos DRM G2D" > > depends on DRM_EXYNOS && !VIDEO_SAMSUNG_S5P_G2D > > + select FRAME_VECTOR > > help > > Choose this option if you want to use Exynos G2D for DRM. > > > > diff --git a/drivers/media/platform/omap/Kconfig > > b/drivers/media/platform/omap/Kconfig > > index dc2aaab54aef..217d613b0fe7 100644 > > --- a/drivers/media/platform/omap/Kconfig > > +++ b/drivers/media/platform/omap/Kconfig > > @@ -10,6 +10,7 @@ config VIDEO_OMAP2_VOUT > > select OMAP2_DSS if HAS_IOMEM && ARCH_OMAP2PLUS > > select OMAP2_VRFB if ARCH_OMAP2 || ARCH_OMAP3 > > select VIDEO_OMAP2_VOUT_VRFB if VIDEO_OMAP2_VOUT && OMAP2_VRFB > > + select FRAME_VECTOR > > default n > > ---help--- > > V4L2 Display driver support for OMAP2/3 based boards. > > diff --git a/drivers/media/v4l2-core/Kconfig > > b/drivers/media/v4l2-core/Kconfig > > index f7a01a72eb9e..f38f6e387f04 100644 > > --- a/drivers/media/v4l2-core/Kconfig > > +++ b/drivers/media/v4l2-core/Kconfig > > @@ -73,6 +73,7 @@ config VIDEOBUF2_CORE > > > > config VIDEOBUF2_MEMOPS > > tristate > > + select FRAME_VECTOR > > > > config VIDEOBUF2_DMA_CONTIG > > tristate > > diff --git a/mm/Kconfig b/mm/Kconfig > > index 390214da4546..2ca52e9986f0 100644 > > --- a/mm/Kconfig > > +++ b/mm/Kconfig > > @@ -635,3 +635,6 @@ config MAX_STACK_SIZE_MB > > changed to a smaller value in which case that is used. > > > > A sane initial value is 80 MB. > > + > > +config FRAME_VECTOR > > + bool > > diff --git a/mm/Makefile b/mm/Makefile > > index 98c4eaeabdcb..be5d5c866305 100644 > > --- a/mm/Makefile > > +++ b/mm/Makefile > > @@ -78,3 +78,4 @@ obj-$(CONFIG_CMA) += cma.o > > obj-$(CONFIG_MEMORY_BALLOON) += balloon_compaction.o > > obj-$(CONFIG_PAGE_EXTENSION) += page_ext.o > > obj-$(CONFIG_CMA_DEBUGFS) += cma_debug.o > > +obj-$(CONFIG_FRAME_VECTOR) += frame_vector.o > > diff --git a/mm/frame_vector.c b/mm/frame_vector.c > > new file mode 100644 > > index ..31a2bd5f41d5 > > --- /dev/null > > +++ b/mm/frame_vector.c > > @@ -0,0 +1,232 @@ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +/* > > + * get_vaddr_frames() - map virtual addresses to pfns > > + * @start: starting user address > > + * @nr_frames: number of pages / pfns from start to map > > + * @write: whether pages will be written to by the caller > > + * @force: whether to force write access even if user mapping is > > + * readonly. See description of the same argument of > > + get_user_pages(). > > + * @vec: structure which receives pages / pfns of the addresses mapped. > > + * It should have space for at least nr_frames entries. > > + * > > + * This function maps virtual addresses from @start and fills @vec > > structure > > + * with page frame numbers or page pointers to corresponding pages (choice > > + * d
Re: [PATCH 1/2] media: s5p-mfc: add return value check in mfc_sys_init_cmd
On 3 June 2015 at 11:36, Marek Szyprowski wrote:
> alloc_dev_context_buffer method might fail, so add proper return value
> check.
>
> Signed-off-by: Marek Szyprowski
Acked-by: Kamil Debski
> ---
> drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> index f176096..b1b1491 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> @@ -37,8 +37,12 @@ static int s5p_mfc_sys_init_cmd_v6(struct s5p_mfc_dev *dev)
> {
> struct s5p_mfc_cmd_args h2r_args;
> struct s5p_mfc_buf_size_v6 *buf_size = dev->variant->buf_size->priv;
> + int ret;
> +
> + ret = s5p_mfc_hw_call(dev->mfc_ops, alloc_dev_context_buffer, dev);
> + if (ret)
> + return ret;
>
> - s5p_mfc_hw_call(dev->mfc_ops, alloc_dev_context_buffer, dev);
> mfc_write(dev, dev->ctx_buf.dma, S5P_FIMV_CONTEXT_MEM_ADDR_V6);
> mfc_write(dev, buf_size->dev_ctx, S5P_FIMV_CONTEXT_MEM_SIZE_V6);
> return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SYS_INIT_V6,
> --
> 1.9.2
>
--
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] media: s5p-mfc: add additional check for incorrect memory configuration
On 3 June 2015 at 11:36, Marek Szyprowski wrote:
> MFC hardware is known to trash random memory if one tries to use a
> buffer buffer, which has lower DMA addresses than the configured DMA
> base address. This patch adds a check for this case and proper error
> handling.
>
> Signed-off-by: Marek Szyprowski
Acked-by: Kamil Debski
> ---
> drivers/media/platform/s5p-mfc/s5p_mfc_opr.c| 11 +--
> drivers/media/platform/s5p-mfc/s5p_mfc_opr.h| 2 +-
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 12 +++-
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 8 +---
> 4 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
> index 00a1d8b..8d27f88 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
> @@ -37,10 +37,9 @@ void s5p_mfc_init_regs(struct s5p_mfc_dev *dev)
> dev->mfc_regs = s5p_mfc_init_regs_v6_plus(dev);
> }
>
> -int s5p_mfc_alloc_priv_buf(struct device *dev,
> +int s5p_mfc_alloc_priv_buf(struct device *dev, dma_addr_t base,
> struct s5p_mfc_priv_buf *b)
> {
> -
> mfc_debug(3, "Allocating priv: %zu\n", b->size);
>
> b->virt = dma_alloc_coherent(dev, b->size, &b->dma, GFP_KERNEL);
> @@ -50,6 +49,14 @@ int s5p_mfc_alloc_priv_buf(struct device *dev,
> return -ENOMEM;
> }
>
> + if (b->dma < base) {
> + mfc_err("Invaling memory configuration!\n");
> + mfc_err("Allocated buffer (%pad) is lower than memory base
> addres (%pad)\n",
> + &b->dma, &base);
> + dma_free_coherent(dev, b->size, b->virt, b->dma);
> + return -ENOMEM;
> + }
> +
> mfc_debug(3, "Allocated addr %p %pad\n", b->virt, &b->dma);
> return 0;
> }
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
> b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
> index 22dfb3e..77a08b1 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
> @@ -334,7 +334,7 @@ struct s5p_mfc_hw_ops {
>
> void s5p_mfc_init_hw_ops(struct s5p_mfc_dev *dev);
> void s5p_mfc_init_regs(struct s5p_mfc_dev *dev);
> -int s5p_mfc_alloc_priv_buf(struct device *dev,
> +int s5p_mfc_alloc_priv_buf(struct device *dev, dma_addr_t base,
> struct s5p_mfc_priv_buf *b);
> void s5p_mfc_release_priv_buf(struct device *dev,
> struct s5p_mfc_priv_buf *b);
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> index c7adc3d..b3f6700 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> @@ -41,7 +41,7 @@ static int s5p_mfc_alloc_dec_temp_buffers_v5(struct
> s5p_mfc_ctx *ctx)
> int ret;
>
> ctx->dsc.size = buf_size->dsc;
> - ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->dsc);
> + ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->dsc);
> if (ret) {
> mfc_err("Failed to allocate temporary buffer\n");
> return ret;
> @@ -172,7 +172,8 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct
> s5p_mfc_ctx *ctx)
> /* Allocate only if memory from bank 1 is necessary */
> if (ctx->bank1.size > 0) {
>
> - ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->bank1);
> + ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1,
> +&ctx->bank1);
> if (ret) {
> mfc_err("Failed to allocate Bank1 temporary
> buffer\n");
> return ret;
> @@ -181,7 +182,8 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct
> s5p_mfc_ctx *ctx)
> }
> /* Allocate only if memory from bank 2 is necessary */
> if (ctx->bank2.size > 0) {
> - ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_r, &ctx->bank2);
> + ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_r, dev->bank2,
> +&ctx->bank2);
> if (ret) {
> mfc_err("Failed to allocate Bank2 temporary
> buffer\n");
> s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l,
> &ctx->bank1);
> @@ -212,7 +214,7 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct
> s5p_mfc_ctx *ctx)
> else
> ctx->ctx.size = buf_size->non_h264_ctx;
>
> - ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->ctx);
> + ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->ctx);
> if (ret) {
> mfc_err("Failed to allocate instance buffer\n");
> return ret;
> @@ -225,7 +227,7 @@ static int s5p_mfc_a
Re: [git:media_tree/master] [media] vb2: Push mmap_sem down to memops
Hi Jan,
Please keep Andrew in the loop. The patch series is on his tree.
Regards,
Mauro
Em Thu, 18 Jun 2015 12:12:08 +0200
Jan Kara escreveu:
> On Thu 11-06-15 10:52:22, Hans Verkuil wrote:
> > Jan,
> >
> > This patch causes a regressing in videobuf2-dma-sg with a potential
> > deadlock:
> >
> > [ 82.290231] ==
> > [ 82.290232] [ INFO: possible circular locking dependency detected ]
> > [ 82.290235] 4.1.0-rc3-tb1 #12 Not tainted
> > [ 82.290236] ---
> > [ 82.290238] qv4l2/1262 is trying to acquire lock:
> > [ 82.290240] (&mm->mmap_sem){++}, at: []
> > vb2_dma_sg_put_userptr+0xf0/0x170 [videobuf2_dma_sg]
> > [ 82.290247]
> >but task is already holding lock:
> > [ 82.290249] (&q->mmap_lock){+.+.+.}, at: []
> > __reqbufs.isra.13+0x7c/0x410 [videobuf2_core]
> > [ 82.290255]
> >which lock already depends on the new lock.
>
> Thanks for the report! So I finally got to this after returning from
> vacation and dealing with more urgent stuff. Looking more at the code it
> seems to me that this particular code path going through
> vb2_ioctl_reqbufs() didn't have previously any mmap_sem protection. Thus we
> could call vma->vm_ops->close() from vb2_put_vma() without holding mmap_sem
> which is against the locking protocol.
>
> My patch unintentionally fixed this but that introduced the lock inversion
> lockdep complains about. Now the full series removes the need for getting
> VMA reference in videobuf2 code so we don't need mmap_sem in put_userptr
> callbacks at all. So when the whole series is applied we are fine again.
>
> So how shall we proceed? Just slap this patch at the beginning of the
> series to make sure it gets merged at once so the lock inversion isn't
> there for long? Or alternatively I can just remove mmap_sem from
> put_userptr() in this series. That will somewhat increase the amount of
> calls to vma->vm_ops->close() without mmap_sem util the whole series is
> applied. Any opinions guys?
>
> Honza
> > [ 82.290257]
> >the existing dependency chain (in reverse order) is:
> > [ 82.290259]
> >-> #1 (&q->mmap_lock){+.+.+.}:
> > [ 82.290262][] lock_acquire+0xc9/0x290
> > [ 82.290267][] mutex_lock_nested+0x4e/0x3f0
> > [ 82.290270][] vb2_mmap+0x232/0x350
> > [videobuf2_core]
> > [ 82.290273][] vb2_fop_mmap+0x25/0x30
> > [videobuf2_core]
> > [ 82.290276][] v4l2_mmap+0x5a/0x90 [videodev]
> > [ 82.290281][] mmap_region+0x3bb/0x5f0
> > [ 82.290285][] do_mmap_pgoff+0x31f/0x400
> > [ 82.290288][] vm_mmap_pgoff+0x90/0xc0
> > [ 82.290291][] SyS_mmap_pgoff+0x1df/0x290
> > [ 82.290294][] SyS_mmap+0x22/0x30
> > [ 82.290297][] system_call_fastpath+0x12/0x6f
> > [ 82.290300]
> >-> #0 (&mm->mmap_sem){++}:
> > [ 82.290303][] __lock_acquire+0x1d53/0x1fe0
> > [ 82.290306][] lock_acquire+0xc9/0x290
> > [ 82.290308][] down_read+0x34/0x50
> > [ 82.290311][]
> > vb2_dma_sg_put_userptr+0xf0/0x170 [videobuf2_dma_sg]
> > [ 82.290314][] __vb2_queue_free+0x156/0x5f0
> > [videobuf2_core]
> > [ 82.290317][] __reqbufs.isra.13+0x9f/0x410
> > [videobuf2_core]
> > [ 82.290320][] vb2_ioctl_reqbufs+0x74/0xb0
> > [videobuf2_core]
> > [ 82.290323][] v4l_reqbufs+0x43/0x50 [videodev]
> > [ 82.290327][] __video_do_ioctl+0x274/0x310
> > [videodev]
> > [ 82.290331][] video_usercopy+0x378/0x8f0
> > [videodev]
> > [ 82.290336][] video_ioctl2+0x15/0x20 [videodev]
> > [ 82.290340][] v4l2_ioctl+0xd0/0xf0 [videodev]
> > [ 82.290343][] do_vfs_ioctl+0x308/0x540
> > [ 82.290347][] SyS_ioctl+0x81/0xa0
> > [ 82.290349][] system_call_fastpath+0x12/0x6f
> > [ 82.290352]
> >other info that might help us debug this:
> >
> > [ 82.290354] Possible unsafe locking scenario:
> >
> > [ 82.290356]CPU0CPU1
> > [ 82.290357]
> > [ 82.290358] lock(&q->mmap_lock);
> > [ 82.290360]lock(&mm->mmap_sem);
> > [ 82.290362]lock(&q->mmap_lock);
> > [ 82.290365] lock(&mm->mmap_sem);
> > [ 82.290367]
> > *** DEADLOCK ***
> >
> > [ 82.290369] 2 locks held by qv4l2/1262:
> > [ 82.290370] #0: (&s->lock){+.+.+.}, at: []
> > v4l2_ioctl+0x5f/0xf0 [videodev]
> > [ 82.290376] #1: (&q->mmap_lock){+.+.+.}, at: []
> > __reqbufs.isra.13+0x7c/0x410 [videobuf2_core]
> > [ 82.290382]
> >stack backtrace:
> > [ 82.290385] CPU: 1 PID: 1262 Comm: qv4l2 Not tainted 4.1.0-rc3-tb1 #12
> > [ 82.290387] Hardware name: /DH
Re: [PATCH] Revert "[media] vb2: Push mmap_sem down to memops"
On Thu 18-06-15 12:45:26, Hans Verkuil wrote: > On 06/18/2015 12:33 PM, Jan Kara wrote: > > On Mon 15-06-15 09:24:55, Hans Verkuil wrote: > >> This reverts commit 48b25a3a713b90988b6882d318f7c0a6bed9aabc. > >> > >> That commit caused two regressions. The first is a BUG: > >> > >> BUG: unable to handle kernel NULL pointer dereference at 0100 > >> IP: [] __lock_acquire+0x2f0/0x2070 > >> PGD 0 > >> Oops: [#1] PREEMPT SMP > >> Modules linked in: vivid v4l2_dv_timings videobuf2_vmalloc > >> videobuf2_memops videobuf2_core v4l2_common videodev media vmw_balloon > >> vmw_vmci acpi_cpufreq processor button > >> CPU: 0 PID: 1542 Comm: v4l2-ctl Not tainted 4.1.0-rc3-test-media #1190 > >> Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop > >> Reference Platform, BIOS 6.00 05/20/2014 > >> task: 880220ce4200 ti: 88021d16c000 task.ti: 88021d16c000 > >> RIP: 0010:[] [] > >> __lock_acquire+0x2f0/0x2070 > >> RSP: 0018:88021d16f9b8 EFLAGS: 00010002 > >> RAX: 0046 RBX: 0292 RCX: 0001 > >> RDX: RSI: 0001 RDI: 0100 > >> RBP: 88021d16fa88 R08: 0001 R09: > >> R10: 0001 R11: R12: 0001 > >> R13: 880220ce4200 R14: 0100 R15: > >> FS: 7f2441e7f740() GS:880236e0() > >> knlGS: > >> CS: 0010 DS: ES: CR0: 8005003b > >> CR2: 0100 CR3: 01e0b000 CR4: 001406f0 > >> Stack: > >> 88021d16fa98 810d6543 0006 0246 > >> 88021d16fa08 810d532d 880220ce4a78 8802 > >> 88020001 0001 0093a4a0 > >> Call Trace: > >> [] ? __lock_acquire+0xb63/0x2070 > >> [] ? mark_held_locks+0x6d/0xa0 > >> [] ? __lock_is_held+0x58/0x80 > >> [] lock_acquire+0x6c/0xa0 > >> [] ? vb2_vmalloc_put_userptr+0x36/0x110 > >> [videobuf2_vmalloc] > >> [] down_read+0x42/0x60 > >> [] ? vb2_vmalloc_put_userptr+0x36/0x110 > >> [videobuf2_vmalloc] > >> [] ? mutex_lock_nested+0x2b1/0x560 > >> [] ? vb2_queue_release+0x25/0x40 [videobuf2_core] > >> [] vb2_vmalloc_put_userptr+0x36/0x110 > >> [videobuf2_vmalloc] > >> [] __vb2_queue_free+0x146/0x5e0 [videobuf2_core] > >> [] vb2_queue_release+0x33/0x40 [videobuf2_core] > >> [] _vb2_fop_release+0x95/0xb0 [videobuf2_core] > >> [] vb2_fop_release+0x29/0x50 [videobuf2_core] > >> [] vivid_fop_release+0x92/0x230 [vivid] > >> [] v4l2_release+0x30/0x80 [videodev] > >> [] __fput+0xe5/0x200 > >> [] fput+0x9/0x10 > >> [] task_work_run+0xc4/0xf0 > >> [] do_exit+0x3a0/0xaf0 > >> [] ? _raw_spin_unlock_irq+0x2b/0x60 > >> [] do_group_exit+0x4f/0xe0 > >> [] get_signal+0x200/0x8c0 > >> [] ? __mutex_unlock_slowpath+0xf5/0x240 > >> [] do_signal+0x23/0x820 > >> [] ? mutex_unlock+0x9/0x10 > >> [] ? v4l2_ioctl+0x78/0xf0 [videodev] > >> [] ? int_very_careful+0x5/0x46 > >> [] ? trace_hardirqs_on_caller+0x15d/0x200 > >> [] do_notify_resume+0x50/0x60 > >> [] int_signal+0x12/0x17 > >> Code: ca 81 31 c0 e8 7a e2 8c 00 e8 aa 1d 8d 00 0f 1f 44 00 00 31 db 48 81 > >> c4 a8 00 00 00 89 d8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 66 90 <49> 81 3e 40 > >> 4e 02 82 b8 00 00 00 00 44 0f 44 e0 41 83 ff 01 0f > >> RIP [] __lock_acquire+0x2f0/0x2070 > >> RSP > >> CR2: 0100 > >> ---[ end trace 25595c2b8560cb57 ]--- > >> Fixing recursive fault but reboot is needed! > > > > Ah, that's tricky. We can end up calling task_work_run() via > > exit_task_work() after mm has been shut down. And the task work will be > > dropping the last reference to all file descriptors which ends up shutting > > down vb2 after current->mm has been cleaned up. > > > > So in the light of this it's probably better for the initial patch to > > completely avoid grabbing mmap_sem in put_userptr(). It breaks locking for > > vma->vm_ops->close() but that's already broken in vb2 as I explained in my > > other email. And the remainder of the patch set will make sure we don't > > need mmap_sem in put_userptr() at all and thus fixes the whole issue. > > > > This also explains why I never saw the problem in my testing - I was always > > testing the patch set as a whole. > > > > I'll send an updated first patch later today. > > OK, good. I'm thinking: if it is OK with Andrew, then the low-level mm changes > can be merged for 4.2 through his tree since that doesn't affect anything else > (right?), but the vb2 changes I prefer to postpone to 4.3. I'd like to give it > enough time for testing and shake-out any remaining issues (hopefully there > aren't any, of course). Yeah, the mm changes just provide the infrastructure so they don't depend on anything. I think they are fine for 4.2. > The omap_vout patch can go in for 4.3 as well (this driver might be > removed in the near future, so there is no hurry there), and it's up to > you what to do with
Re: [PATCH] Revert "[media] vb2: Push mmap_sem down to memops"
On 06/18/2015 12:33 PM, Jan Kara wrote: > On Mon 15-06-15 09:24:55, Hans Verkuil wrote: >> This reverts commit 48b25a3a713b90988b6882d318f7c0a6bed9aabc. >> >> That commit caused two regressions. The first is a BUG: >> >> BUG: unable to handle kernel NULL pointer dereference at 0100 >> IP: [] __lock_acquire+0x2f0/0x2070 >> PGD 0 >> Oops: [#1] PREEMPT SMP >> Modules linked in: vivid v4l2_dv_timings videobuf2_vmalloc videobuf2_memops >> videobuf2_core v4l2_common videodev media vmw_balloon vmw_vmci acpi_cpufreq >> processor button >> CPU: 0 PID: 1542 Comm: v4l2-ctl Not tainted 4.1.0-rc3-test-media #1190 >> Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference >> Platform, BIOS 6.00 05/20/2014 >> task: 880220ce4200 ti: 88021d16c000 task.ti: 88021d16c000 >> RIP: 0010:[] [] >> __lock_acquire+0x2f0/0x2070 >> RSP: 0018:88021d16f9b8 EFLAGS: 00010002 >> RAX: 0046 RBX: 0292 RCX: 0001 >> RDX: RSI: 0001 RDI: 0100 >> RBP: 88021d16fa88 R08: 0001 R09: >> R10: 0001 R11: R12: 0001 >> R13: 880220ce4200 R14: 0100 R15: >> FS: 7f2441e7f740() GS:880236e0() knlGS: >> CS: 0010 DS: ES: CR0: 8005003b >> CR2: 0100 CR3: 01e0b000 CR4: 001406f0 >> Stack: >> 88021d16fa98 810d6543 0006 0246 >> 88021d16fa08 810d532d 880220ce4a78 8802 >> 88020001 0001 0093a4a0 >> Call Trace: >> [] ? __lock_acquire+0xb63/0x2070 >> [] ? mark_held_locks+0x6d/0xa0 >> [] ? __lock_is_held+0x58/0x80 >> [] lock_acquire+0x6c/0xa0 >> [] ? vb2_vmalloc_put_userptr+0x36/0x110 >> [videobuf2_vmalloc] >> [] down_read+0x42/0x60 >> [] ? vb2_vmalloc_put_userptr+0x36/0x110 >> [videobuf2_vmalloc] >> [] ? mutex_lock_nested+0x2b1/0x560 >> [] ? vb2_queue_release+0x25/0x40 [videobuf2_core] >> [] vb2_vmalloc_put_userptr+0x36/0x110 [videobuf2_vmalloc] >> [] __vb2_queue_free+0x146/0x5e0 [videobuf2_core] >> [] vb2_queue_release+0x33/0x40 [videobuf2_core] >> [] _vb2_fop_release+0x95/0xb0 [videobuf2_core] >> [] vb2_fop_release+0x29/0x50 [videobuf2_core] >> [] vivid_fop_release+0x92/0x230 [vivid] >> [] v4l2_release+0x30/0x80 [videodev] >> [] __fput+0xe5/0x200 >> [] fput+0x9/0x10 >> [] task_work_run+0xc4/0xf0 >> [] do_exit+0x3a0/0xaf0 >> [] ? _raw_spin_unlock_irq+0x2b/0x60 >> [] do_group_exit+0x4f/0xe0 >> [] get_signal+0x200/0x8c0 >> [] ? __mutex_unlock_slowpath+0xf5/0x240 >> [] do_signal+0x23/0x820 >> [] ? mutex_unlock+0x9/0x10 >> [] ? v4l2_ioctl+0x78/0xf0 [videodev] >> [] ? int_very_careful+0x5/0x46 >> [] ? trace_hardirqs_on_caller+0x15d/0x200 >> [] do_notify_resume+0x50/0x60 >> [] int_signal+0x12/0x17 >> Code: ca 81 31 c0 e8 7a e2 8c 00 e8 aa 1d 8d 00 0f 1f 44 00 00 31 db 48 81 >> c4 a8 00 00 00 89 d8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 66 90 <49> 81 3e 40 4e >> 02 82 b8 00 00 00 00 44 0f 44 e0 41 83 ff 01 0f >> RIP [] __lock_acquire+0x2f0/0x2070 >> RSP >> CR2: 0100 >> ---[ end trace 25595c2b8560cb57 ]--- >> Fixing recursive fault but reboot is needed! > > Ah, that's tricky. We can end up calling task_work_run() via > exit_task_work() after mm has been shut down. And the task work will be > dropping the last reference to all file descriptors which ends up shutting > down vb2 after current->mm has been cleaned up. > > So in the light of this it's probably better for the initial patch to > completely avoid grabbing mmap_sem in put_userptr(). It breaks locking for > vma->vm_ops->close() but that's already broken in vb2 as I explained in my > other email. And the remainder of the patch set will make sure we don't > need mmap_sem in put_userptr() at all and thus fixes the whole issue. > > This also explains why I never saw the problem in my testing - I was always > testing the patch set as a whole. > > I'll send an updated first patch later today. OK, good. I'm thinking: if it is OK with Andrew, then the low-level mm changes can be merged for 4.2 through his tree since that doesn't affect anything else (right?), but the vb2 changes I prefer to postpone to 4.3. I'd like to give it enough time for testing and shake-out any remaining issues (hopefully there aren't any, of course). The omap_vout patch can go in for 4.3 as well (this driver might be removed in the near future, so there is no hurry there), and it's up to you what to do with the exynos drm driver. Does this make sense? 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: [PATCH] Revert "[media] vb2: Push mmap_sem down to memops"
On Mon 15-06-15 09:24:55, Hans Verkuil wrote:
> This reverts commit 48b25a3a713b90988b6882d318f7c0a6bed9aabc.
>
> That commit caused two regressions. The first is a BUG:
>
> BUG: unable to handle kernel NULL pointer dereference at 0100
> IP: [] __lock_acquire+0x2f0/0x2070
> PGD 0
> Oops: [#1] PREEMPT SMP
> Modules linked in: vivid v4l2_dv_timings videobuf2_vmalloc videobuf2_memops
> videobuf2_core v4l2_common videodev media vmw_balloon vmw_vmci acpi_cpufreq
> processor button
> CPU: 0 PID: 1542 Comm: v4l2-ctl Not tainted 4.1.0-rc3-test-media #1190
> Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference
> Platform, BIOS 6.00 05/20/2014
> task: 880220ce4200 ti: 88021d16c000 task.ti: 88021d16c000
> RIP: 0010:[] []
> __lock_acquire+0x2f0/0x2070
> RSP: 0018:88021d16f9b8 EFLAGS: 00010002
> RAX: 0046 RBX: 0292 RCX: 0001
> RDX: RSI: 0001 RDI: 0100
> RBP: 88021d16fa88 R08: 0001 R09:
> R10: 0001 R11: R12: 0001
> R13: 880220ce4200 R14: 0100 R15:
> FS: 7f2441e7f740() GS:880236e0() knlGS:
> CS: 0010 DS: ES: CR0: 8005003b
> CR2: 0100 CR3: 01e0b000 CR4: 001406f0
> Stack:
> 88021d16fa98 810d6543 0006 0246
> 88021d16fa08 810d532d 880220ce4a78 8802
> 88020001 0001 0093a4a0
> Call Trace:
> [] ? __lock_acquire+0xb63/0x2070
> [] ? mark_held_locks+0x6d/0xa0
> [] ? __lock_is_held+0x58/0x80
> [] lock_acquire+0x6c/0xa0
> [] ? vb2_vmalloc_put_userptr+0x36/0x110 [videobuf2_vmalloc]
> [] down_read+0x42/0x60
> [] ? vb2_vmalloc_put_userptr+0x36/0x110 [videobuf2_vmalloc]
> [] ? mutex_lock_nested+0x2b1/0x560
> [] ? vb2_queue_release+0x25/0x40 [videobuf2_core]
> [] vb2_vmalloc_put_userptr+0x36/0x110 [videobuf2_vmalloc]
> [] __vb2_queue_free+0x146/0x5e0 [videobuf2_core]
> [] vb2_queue_release+0x33/0x40 [videobuf2_core]
> [] _vb2_fop_release+0x95/0xb0 [videobuf2_core]
> [] vb2_fop_release+0x29/0x50 [videobuf2_core]
> [] vivid_fop_release+0x92/0x230 [vivid]
> [] v4l2_release+0x30/0x80 [videodev]
> [] __fput+0xe5/0x200
> [] fput+0x9/0x10
> [] task_work_run+0xc4/0xf0
> [] do_exit+0x3a0/0xaf0
> [] ? _raw_spin_unlock_irq+0x2b/0x60
> [] do_group_exit+0x4f/0xe0
> [] get_signal+0x200/0x8c0
> [] ? __mutex_unlock_slowpath+0xf5/0x240
> [] do_signal+0x23/0x820
> [] ? mutex_unlock+0x9/0x10
> [] ? v4l2_ioctl+0x78/0xf0 [videodev]
> [] ? int_very_careful+0x5/0x46
> [] ? trace_hardirqs_on_caller+0x15d/0x200
> [] do_notify_resume+0x50/0x60
> [] int_signal+0x12/0x17
> Code: ca 81 31 c0 e8 7a e2 8c 00 e8 aa 1d 8d 00 0f 1f 44 00 00 31 db 48 81 c4
> a8 00 00 00 89 d8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 66 90 <49> 81 3e 40 4e 02
> 82 b8 00 00 00 00 44 0f 44 e0 41 83 ff 01 0f
> RIP [] __lock_acquire+0x2f0/0x2070
> RSP
> CR2: 0100
> ---[ end trace 25595c2b8560cb57 ]---
> Fixing recursive fault but reboot is needed!
Ah, that's tricky. We can end up calling task_work_run() via
exit_task_work() after mm has been shut down. And the task work will be
dropping the last reference to all file descriptors which ends up shutting
down vb2 after current->mm has been cleaned up.
So in the light of this it's probably better for the initial patch to
completely avoid grabbing mmap_sem in put_userptr(). It breaks locking for
vma->vm_ops->close() but that's already broken in vb2 as I explained in my
other email. And the remainder of the patch set will make sure we don't
need mmap_sem in put_userptr() at all and thus fixes the whole issue.
This also explains why I never saw the problem in my testing - I was always
testing the patch set as a whole.
I'll send an updated first patch later today.
Honza
> This can be reproduced by loading the vivid driver and running:
>
> v4l2-ctl --stream-user
>
> and pressing Ctrl-C. You may have to try a few times, but in my experience
> this BUG
> is triggered quite quickly.
>
> The second is a possible deadlock:
>
> Jun 14 18:44:07 test-media kernel: [ 49.376650]
> ==
> Jun 14 18:44:07 test-media kernel: [ 49.376651] [ INFO: possible circular
> locking dependency detected ]
> Jun 14 18:44:07 test-media kernel: [ 49.376653] 4.1.0-rc3-test-media #1190
> Not tainted
> Jun 14 18:44:07 test-media kernel: [ 49.376654]
> ---
> Jun 14 18:44:07 test-media kernel: [ 49.376655] v4l2-compliance/1468 is
> trying to acquire lock:
> Jun 14 18:44:07 test-media kernel: [ 49.376657] (&mm->mmap_sem){++},
> at: [] vb2_vmalloc_put_userptr+0x36/0x110
> [videobuf2_vmalloc]
> Jun 14 18:44:07
Re: [git:media_tree/master] [media] vb2: Push mmap_sem down to memops
On Thu 11-06-15 10:52:22, Hans Verkuil wrote:
> Jan,
>
> This patch causes a regressing in videobuf2-dma-sg with a potential deadlock:
>
> [ 82.290231] ==
> [ 82.290232] [ INFO: possible circular locking dependency detected ]
> [ 82.290235] 4.1.0-rc3-tb1 #12 Not tainted
> [ 82.290236] ---
> [ 82.290238] qv4l2/1262 is trying to acquire lock:
> [ 82.290240] (&mm->mmap_sem){++}, at: []
> vb2_dma_sg_put_userptr+0xf0/0x170 [videobuf2_dma_sg]
> [ 82.290247]
>but task is already holding lock:
> [ 82.290249] (&q->mmap_lock){+.+.+.}, at: []
> __reqbufs.isra.13+0x7c/0x410 [videobuf2_core]
> [ 82.290255]
>which lock already depends on the new lock.
Thanks for the report! So I finally got to this after returning from
vacation and dealing with more urgent stuff. Looking more at the code it
seems to me that this particular code path going through
vb2_ioctl_reqbufs() didn't have previously any mmap_sem protection. Thus we
could call vma->vm_ops->close() from vb2_put_vma() without holding mmap_sem
which is against the locking protocol.
My patch unintentionally fixed this but that introduced the lock inversion
lockdep complains about. Now the full series removes the need for getting
VMA reference in videobuf2 code so we don't need mmap_sem in put_userptr
callbacks at all. So when the whole series is applied we are fine again.
So how shall we proceed? Just slap this patch at the beginning of the
series to make sure it gets merged at once so the lock inversion isn't
there for long? Or alternatively I can just remove mmap_sem from
put_userptr() in this series. That will somewhat increase the amount of
calls to vma->vm_ops->close() without mmap_sem util the whole series is
applied. Any opinions guys?
Honza
> [ 82.290257]
>the existing dependency chain (in reverse order) is:
> [ 82.290259]
>-> #1 (&q->mmap_lock){+.+.+.}:
> [ 82.290262][] lock_acquire+0xc9/0x290
> [ 82.290267][] mutex_lock_nested+0x4e/0x3f0
> [ 82.290270][] vb2_mmap+0x232/0x350
> [videobuf2_core]
> [ 82.290273][] vb2_fop_mmap+0x25/0x30
> [videobuf2_core]
> [ 82.290276][] v4l2_mmap+0x5a/0x90 [videodev]
> [ 82.290281][] mmap_region+0x3bb/0x5f0
> [ 82.290285][] do_mmap_pgoff+0x31f/0x400
> [ 82.290288][] vm_mmap_pgoff+0x90/0xc0
> [ 82.290291][] SyS_mmap_pgoff+0x1df/0x290
> [ 82.290294][] SyS_mmap+0x22/0x30
> [ 82.290297][] system_call_fastpath+0x12/0x6f
> [ 82.290300]
>-> #0 (&mm->mmap_sem){++}:
> [ 82.290303][] __lock_acquire+0x1d53/0x1fe0
> [ 82.290306][] lock_acquire+0xc9/0x290
> [ 82.290308][] down_read+0x34/0x50
> [ 82.290311][] vb2_dma_sg_put_userptr+0xf0/0x170
> [videobuf2_dma_sg]
> [ 82.290314][] __vb2_queue_free+0x156/0x5f0
> [videobuf2_core]
> [ 82.290317][] __reqbufs.isra.13+0x9f/0x410
> [videobuf2_core]
> [ 82.290320][] vb2_ioctl_reqbufs+0x74/0xb0
> [videobuf2_core]
> [ 82.290323][] v4l_reqbufs+0x43/0x50 [videodev]
> [ 82.290327][] __video_do_ioctl+0x274/0x310
> [videodev]
> [ 82.290331][] video_usercopy+0x378/0x8f0
> [videodev]
> [ 82.290336][] video_ioctl2+0x15/0x20 [videodev]
> [ 82.290340][] v4l2_ioctl+0xd0/0xf0 [videodev]
> [ 82.290343][] do_vfs_ioctl+0x308/0x540
> [ 82.290347][] SyS_ioctl+0x81/0xa0
> [ 82.290349][] system_call_fastpath+0x12/0x6f
> [ 82.290352]
>other info that might help us debug this:
>
> [ 82.290354] Possible unsafe locking scenario:
>
> [ 82.290356]CPU0CPU1
> [ 82.290357]
> [ 82.290358] lock(&q->mmap_lock);
> [ 82.290360]lock(&mm->mmap_sem);
> [ 82.290362]lock(&q->mmap_lock);
> [ 82.290365] lock(&mm->mmap_sem);
> [ 82.290367]
> *** DEADLOCK ***
>
> [ 82.290369] 2 locks held by qv4l2/1262:
> [ 82.290370] #0: (&s->lock){+.+.+.}, at: []
> v4l2_ioctl+0x5f/0xf0 [videodev]
> [ 82.290376] #1: (&q->mmap_lock){+.+.+.}, at: []
> __reqbufs.isra.13+0x7c/0x410 [videobuf2_core]
> [ 82.290382]
>stack backtrace:
> [ 82.290385] CPU: 1 PID: 1262 Comm: qv4l2 Not tainted 4.1.0-rc3-tb1 #12
> [ 82.290387] Hardware name: /DH67CF, BIOS
> BLH6710H.86A.0105.2011.0301.1654 03/01/2011
> [ 82.290388] 82c46890 8800b4bfb968 81a98687
> 0007
> [ 82.290392] 82c46890 8800b4bfb9b8 8110785d
>
> [ 82.290395] 8800b4bfba28 0001 8800d51ce718
> 0001
> [ 82.290399] Call Trace:
