Re: [PATCH v2 2/2] staging: erofs: complete POSIX ACL support
Hi, On 2019/1/29 2:30, Dan Carpenter wrote: > Are you serious? The standard fault injection doesn't do that??? > > Please fix it instead of creating a duplicate better implementation > which only your filesystem can use. I would have thought that obviously > any fault injection framework could at least be configured to test > specific code... > > regards, > dan carpenter Since Chao is now on the short business trip, I will resend v4 and change erofs_kmalloc into kmalloc temporarily to make sure ACL is usable first in the staging tree. As for the EROFS fault injection, let's wait Chao's reply... Thanks, Gao Xiang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/7] binder: create userspace-to-binder-buffer copy function
On Mon, Jan 28, 2019 at 04:49:28PM -0800, Todd Kjos wrote:
> +/**
> + * binder_alloc_copy_user_to_buffer() - copy src user to tgt user
> + * @alloc: binder_alloc for this proc
> + * @buffer: binder buffer to be accessed
> + * @buffer_offset: offset into @buffer data
> + * @from: userspace pointer to source buffer
> + * @bytes: bytes to copy
> + *
> + * Copy bytes from source userspace to target buffer.
> + *
> + * Return: bytes remaining to be copied
> + */
> +unsigned long
> +binder_alloc_copy_user_to_buffer(struct binder_alloc *alloc,
> + struct binder_buffer *buffer,
> + binder_size_t buffer_offset,
> + const void __user *from,
> + size_t bytes)
> +{
> + if (!check_buffer(alloc, buffer, buffer_offset, bytes))
> + return bytes;
> +
> + while (bytes) {
> + unsigned long size;
> + unsigned long ret;
> + struct page *page;
> + pgoff_t pgoff;
> + void *kptr;
> +
> + page = binder_alloc_get_page(alloc, buffer,
> + buffer_offset, &pgoff);
> + size = min(bytes, (size_t)(PAGE_SIZE - pgoff));
This code has so much more casting than necessary. To me casting says
that we haven't got the types correct or something. I've just pulled
this function out as an example really, but none of the casts here are
required... This could just be:
size = min(bytes, PAGE_SIZE - pgoff);
Btw, if you really need to do a cast inside a min() (you don't in this
cast) then use min_t().
> + kptr = (void *)((uintptr_t)kmap(page) + pgoff);
This would be a lot cleaner as:
kptr = kmap(page) + pgoff;
> + ret = copy_from_user(kptr, (const void __user *)(uintptr_t)from,
> + size);
Remove the cast:
ret = copy_from_user(kptr, from, size);
> + kunmap(page);
> + if (ret)
> + return bytes - size + ret;
> + bytes -= size;
> + from = (void __user *)(uintptr_t)from + size;
Remove the cast:
from += size;
> + buffer_offset += size;
> + }
> + return 0;
> +}
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 2/2] staging: erofs: complete POSIX ACL support
Let's add .get_acl() to read the file's acl from its xattrs
to make POSIX ACL usable.
Here is the on-disk detail,
fullname: system.posix_acl_access
struct erofs_xattr_entry:
.e_name_len = 0
.e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS (2)
fullname: system.posix_acl_default
struct erofs_xattr_entry:
.e_name_len = 0
.e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT (3)
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
change log v4:
- leave kmalloc and wait the conclusion of fault injection
pointed out by Dan Carpenter;
change log v3:
- kvmalloc -> erofs_kmalloc suggested by Chao;
- update SB_POSIXACL for the remount case as well;
change log v2:
- add the missing SB_POSIXACL flag;
- fix the on-disk detail, use .e_name_len == 0 and proper prefix value;
- tested ok with xattr enabled erofs_mkfs;
.../erofs/Documentation/filesystems/erofs.txt | 2 ++
drivers/staging/erofs/inode.c | 3 ++
drivers/staging/erofs/namei.c | 1 +
drivers/staging/erofs/super.c | 10 ++
drivers/staging/erofs/xattr.c | 37 ++
drivers/staging/erofs/xattr.h | 6
6 files changed, 59 insertions(+)
diff --git a/drivers/staging/erofs/Documentation/filesystems/erofs.txt
b/drivers/staging/erofs/Documentation/filesystems/erofs.txt
index 803988d74c21..961ec4da7705 100644
--- a/drivers/staging/erofs/Documentation/filesystems/erofs.txt
+++ b/drivers/staging/erofs/Documentation/filesystems/erofs.txt
@@ -36,6 +36,8 @@ Here is the main features of EROFS:
- Support xattr inline and tail-end data inline for all files;
+ - Support POSIX.1e ACLs by using xattrs;
+
- Support transparent file compression as an option:
LZ4 algorithm with 4 KB fixed-output compression for high performance;
diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
index 4f04f7c38cf2..924b8dfc7a8f 100644
--- a/drivers/staging/erofs/inode.c
+++ b/drivers/staging/erofs/inode.c
@@ -287,6 +287,7 @@ const struct inode_operations erofs_generic_iops = {
#ifdef CONFIG_EROFS_FS_XATTR
.listxattr = erofs_listxattr,
#endif
+ .get_acl = erofs_get_acl,
};
const struct inode_operations erofs_symlink_iops = {
@@ -294,6 +295,7 @@ const struct inode_operations erofs_symlink_iops = {
#ifdef CONFIG_EROFS_FS_XATTR
.listxattr = erofs_listxattr,
#endif
+ .get_acl = erofs_get_acl,
};
const struct inode_operations erofs_fast_symlink_iops = {
@@ -301,5 +303,6 @@ const struct inode_operations erofs_fast_symlink_iops = {
#ifdef CONFIG_EROFS_FS_XATTR
.listxattr = erofs_listxattr,
#endif
+ .get_acl = erofs_get_acl,
};
diff --git a/drivers/staging/erofs/namei.c b/drivers/staging/erofs/namei.c
index 7fed1f996ab0..b1752adc5934 100644
--- a/drivers/staging/erofs/namei.c
+++ b/drivers/staging/erofs/namei.c
@@ -238,5 +238,6 @@ const struct inode_operations erofs_dir_iops = {
#ifdef CONFIG_EROFS_FS_XATTR
.listxattr = erofs_listxattr,
#endif
+ .get_acl = erofs_get_acl,
};
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 176fca2af379..15c784fba879 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -398,6 +398,11 @@ static int erofs_read_super(struct super_block *sb,
if (!silent)
infoln("root inode @ nid %llu", ROOT_NID(sbi));
+ if (test_opt(sbi, POSIX_ACL))
+ sb->s_flags |= SB_POSIXACL;
+ else
+ sb->s_flags &= ~SB_POSIXACL;
+
#ifdef CONFIG_EROFS_FS_ZIP
INIT_RADIX_TREE(&sbi->workstn_tree, GFP_ATOMIC);
#endif
@@ -646,6 +651,11 @@ static int erofs_remount(struct super_block *sb, int
*flags, char *data)
if (err)
goto out;
+ if (test_opt(sbi, POSIX_ACL))
+ sb->s_flags |= SB_POSIXACL;
+ else
+ sb->s_flags &= ~SB_POSIXACL;
+
*flags |= SB_RDONLY;
return 0;
out:
diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
index 7de46690d972..e748df8d2b44 100644
--- a/drivers/staging/erofs/xattr.c
+++ b/drivers/staging/erofs/xattr.c
@@ -643,3 +643,40 @@ ssize_t erofs_listxattr(struct dentry *dentry,
return shared_listxattr(&it);
}
+#ifdef CONFIG_EROFS_FS_POSIX_ACL
+struct posix_acl *erofs_get_acl(struct inode *inode, int type)
+{
+ struct posix_acl *acl;
+ int prefix, rc;
+ char *value = NULL;
+
+ switch (type) {
+ case ACL_TYPE_ACCESS:
+ prefix = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS;
+ break;
+ case ACL_TYPE_DEFAULT:
+ prefix = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT;
+ break;
+ default:
+ return ERR_PTR(-EINVAL);
+ }
+
+ rc = erofs_getxattr(inode, prefix, "", NULL, 0);
+ if (rc > 0) {
+ value = kmalloc(rc, GFP_KERNEL);
+ if (!
[PATCH v4 1/2] staging: erofs: use xattr_prefix to wrap up
Let's use xattr_prefix instead of open code.
No logic changes.
Reviewed-by: Chao Yu
Signed-off-by: Gao Xiang
---
[resend v2 as v4]
change log v2:
- remove the confusing line according to Dan Carpenter;
drivers/staging/erofs/xattr.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
index 1c9498e38f0e..7de46690d972 100644
--- a/drivers/staging/erofs/xattr.c
+++ b/drivers/staging/erofs/xattr.c
@@ -520,8 +520,7 @@ static int xattr_entrylist(struct xattr_iter *_it,
if (h == NULL || (h->list != NULL && !h->list(it->dentry)))
return 1;
- /* Note that at least one of 'prefix' and 'name' should be non-NULL */
- prefix = h->prefix != NULL ? h->prefix : h->name;
+ prefix = xattr_prefix(h);
prefix_len = strlen(prefix);
if (it->buffer == NULL) {
--
2.14.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: wilc1000: remove redundant reset of station statistics
On Mon, Jan 28, 2019 at 10:38:20PM +, adham.aboza...@microchip.com wrote:
> From: Adham Abozaeid
>
> Driver sends configuration wids to reset connection statistics in the
> device, but the device already resets it when starting a new connection
>
> Signed-off-by: Adham Abozaeid
> ---
> drivers/staging/wilc1000/host_interface.c | 20 +---
> 1 file changed, 1 insertion(+), 19 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/host_interface.c
> b/drivers/staging/wilc1000/host_interface.c
> index fbb61de20304..286685e426c1 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -342,29 +342,11 @@ static int wilc_send_connect_wid(struct wilc_vif *vif)
> {
> int result = 0;
> struct wid wid_list[8];
^^^
We could make this array smaller as well.
> - u32 wid_cnt = 0, dummyval = 0;
> + u32 wid_cnt = 0;
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 2/2] staging: erofs: complete POSIX ACL support
On Tue, Jan 29, 2019 at 04:35:20PM +0800, Gao Xiang wrote: > Let's add .get_acl() to read the file's acl from its xattrs > to make POSIX ACL usable. > > Here is the on-disk detail, > fullname: system.posix_acl_access > struct erofs_xattr_entry: > .e_name_len = 0 > .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS (2) > > fullname: system.posix_acl_default > struct erofs_xattr_entry: > .e_name_len = 0 > .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT (3) > > Reviewed-by: Chao Yu > Signed-off-by: Gao Xiang > --- > change log v4: > - leave kmalloc and wait the conclusion of fault injection >pointed out by Dan Carpenter; I do think this is the right thing, but I also wasn't going to ask you do redo v3. Anyway, thanks! regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 2/2] staging: erofs: complete POSIX ACL support
On 2019/1/29 16:43, Dan Carpenter wrote: > On Tue, Jan 29, 2019 at 04:35:20PM +0800, Gao Xiang wrote: >> Let's add .get_acl() to read the file's acl from its xattrs >> to make POSIX ACL usable. >> >> Here is the on-disk detail, >> fullname: system.posix_acl_access >> struct erofs_xattr_entry: >> .e_name_len = 0 >> .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS (2) >> >> fullname: system.posix_acl_default >> struct erofs_xattr_entry: >> .e_name_len = 0 >> .e_name_index = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT (3) >> >> Reviewed-by: Chao Yu >> Signed-off-by: Gao Xiang >> --- >> change log v4: >> - leave kmalloc and wait the conclusion of fault injection >>pointed out by Dan Carpenter; > > I do think this is the right thing, but I also wasn't going to ask you > do redo v3. Anyway, thanks! > > regards, > dan carpenter OK, let's wait the Chao's idea then... Thanks, Gao Xiang > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls
On Tue, Jan 29, 2019 at 04:44:35PM +0900, Alexandre Courbot wrote: > On Fri, Jan 25, 2019 at 10:04 PM Paul Kocialkowski > > On Thu, 2019-01-24 at 20:23 +0800, Ayaka wrote: > > > > > > Sent from my iPad > > > > > > > On Jan 24, 2019, at 6:27 PM, Paul Kocialkowski > > > > wrote: > > > > > > > > Hi, > > > > > > > > > On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote: > > > > > I forget a important thing, for the rkvdec and rk hevc decoder, it > > > > > would > > > > > requests cabac table, scaling list, picture parameter set and > > > > > reference > > > > > picture storing in one or various of DMA buffers. I am not talking > > > > > about > > > > > the data been parsed, the decoder would requests a raw data. > > > > > > > > > > For the pps and rps, it is possible to reuse the slice header, just > > > > > let > > > > > the decoder know the offset from the bitstream bufer, I would suggest > > > > > to > > > > > add three properties(with sps) for them. But I think we need a method > > > > > to > > > > > mark a OUTPUT side buffer for those aux data. > > > > > > > > I'm quite confused about the hardware implementation then. From what > > > > you're saying, it seems that it takes the raw bitstream elements rather > > > > than parsed elements. Is it really a stateless implementation? > > > > > > > > The stateless implementation was designed with the idea that only the > > > > raw slice data should be passed in bitstream form to the decoder. For > > > > H.264, it seems that some decoders also need the slice header in raw > > > > bitstream form (because they take the full slice NAL unit), see the > > > > discussions in this thread: > > > > media: docs-rst: Document m2m stateless video decoder interface > > > > > > Stateless just mean it won’t track the previous result, but I don’t > > > think you can define what a date the hardware would need. Even you > > > just build a dpb for the decoder, it is still stateless, but parsing > > > less or more data from the bitstream doesn’t stop a decoder become a > > > stateless decoder. > > > > Yes fair enough, the format in which the hardware decoder takes the > > bitstream parameters does not make it stateless or stateful per-se. > > It's just that stateless decoders should have no particular reason for > > parsing the bitstream on their own since the hardware can be designed > > with registers for each relevant bitstream element to configure the > > decoding pipeline. That's how GPU-based decoder implementations are > > implemented (VAAPI/VDPAU/NVDEC, etc). > > > > So the format we have agreed on so far for the stateless interface is > > to pass parsed elements via v4l2 control structures. > > > > If the hardware can only work by parsing the bitstream itself, I'm not > > sure what the best solution would be. Reconstructing the bitstream in > > the kernel is a pretty bad option, but so is parsing in the kernel or > > having the data both in parsed and raw forms. Do you see another > > possibility? > > Is reconstructing the bitstream so bad? The v4l2 controls provide a > generic interface to an encoded format which the driver needs to > convert into a sequence that the hardware can understand. Typically > this is done by populating hardware-specific structures. Can't we > consider that in this specific instance, the hardware-specific > structure just happens to be identical to the original bitstream > format? > > I agree that this is not strictly optimal for that particular > hardware, but such is the cost of abstractions, and in this specific > case I don't believe the cost would be particularly high? I mean, that argument can be made for the rockchip driver as well. If reconstructing the bitstream is something we can do, and if we don't care about being suboptimal for one particular hardware, then why the rockchip driver doesn't just recreate the bitstream from that API? After all, this is just a hardware specific header that happens to be identical to the original bitstream format Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Sat, Jan 26, 2019 at 8:13 PM Jonathan Cameron wrote: > > On Fri, 25 Jan 2019 22:14:32 -0200 > Rodrigo Ribeiro wrote: > > > Em sex, 25 de jan de 2019 às 21:46, Rodrigo Ribeiro > > escreveu: > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > escreveu: > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > > > wrote: > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > Hey, > > > > > > Hi, > > > > > > Thanks for answering. > > > > > > > A bit curios about this one. > > > > Are you using this chip/driver ? > > > > > > No, I am just a student at USP (University of São Paulo) starting in Linux > > > Kernel and a new member of the USP Linux Kernel group that has contributed > > > for a few months. > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > > > marked for removal (since it's still in staging). > > > > But if there are users for this driver, it could be left around for a > > > > while. > > > > > > This is my first patch in Linux Kernel, but if the driver will be > > > removed, I > > > can send a patch for another driver. Is there any driver that I can > > > fix a style warning? > > > > Maybe, one checkstyle patch is enough, right? Which drivers can I truly > > contribute to? > > How about the ad7150? That one is still listed as production. > What do you think Alex, you probably have better visibility on the > status of these parts and their drivers than I do! > > (note I haven't even opened that one for a few years so no idea > what state the driver is in!) > ad7150 is a good alternative. At a first glance over the driver it looks like it could do with some polish/conversions to newer IIO constructs (like IIO triggers, maybe some newer event handling mechanisms?). I'll sync with Stefan [Popa] to see about this stuff at a later point in time. I'd also add here the adxl345 driver. This is mostly informational for anyone who'd find this interesting. There are 2 drivers for this chip, one in IIO [drivers/iio/accel/adxl345] and another one in "drivers/misc/adxl34x.c" as part of the input sub-system. What would be interesting here is to finalize the IIO driver [ I think some features are lacking behind the input driver], and make the input driver a consumer of the IIO driver. From my side, both these variants are fine to take on. The ad7150 is a good idea as a starter project, and the adxl one is more of a long-term medium-level project. Thanks Alex > Jonathan > > > > > > Thanks > > > > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > escreveu: > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > > > wrote: > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > Hey, > > > > > > > > A bit curios about this one. > > > > Are you using this chip/driver ? > > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > > > marked for removal (since it's still in staging). > > > > But if there are users for this driver, it could be left around for a > > > > while. > > > > > > > > Thanks > > > > Alex > > > > > > > > > > > > > > Signed-off-by: Rodrigo Ribeiro > > > > > Signed-off-by: Rafael Tsuha > > > > > --- > > > > > This macro is not used anywhere. Should we just correct the > > > > > spelling or remove the macro? > > > > > > > > > > drivers/staging/iio/cdc/ad7152.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/staging/iio/cdc/ad7152.c > > > > > b/drivers/staging/iio/cdc/ad7152.c > > > > > index 25f51db..c9da6d4 100644 > > > > > --- a/drivers/staging/iio/cdc/ad7152.c > > > > > +++ b/drivers/staging/iio/cdc/ad7152.c > > > > > @@ -35,7 +35,7 @@ > > > > > #define AD7152_REG_CH2_GAIN_HIGH 12 > > > > > #define AD7152_REG_CH2_SETUP 14 > > > > > #define AD7152_REG_CFG 15 > > > > > -#define AD7152_REG_RESEVERD16 > > > > > +#define AD7152_REG_RESERVED16 > > > > > #define AD7152_REG_CAPDAC_POS 17 > > > > > #define AD7152_REG_CAPDAC_NEG 18 > > > > > #define AD7152_REG_CFG226 > > > > > -- > > > > > 2.7.4 > > > > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls
On Tue, Jan 29, 2019 at 5:09 PM Maxime Ripard wrote: > > On Tue, Jan 29, 2019 at 04:44:35PM +0900, Alexandre Courbot wrote: > > On Fri, Jan 25, 2019 at 10:04 PM Paul Kocialkowski > > > On Thu, 2019-01-24 at 20:23 +0800, Ayaka wrote: > > > > > > > > Sent from my iPad > > > > > > > > > On Jan 24, 2019, at 6:27 PM, Paul Kocialkowski > > > > > wrote: > > > > > > > > > > Hi, > > > > > > > > > > > On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote: > > > > > > I forget a important thing, for the rkvdec and rk hevc decoder, it > > > > > > would > > > > > > requests cabac table, scaling list, picture parameter set and > > > > > > reference > > > > > > picture storing in one or various of DMA buffers. I am not talking > > > > > > about > > > > > > the data been parsed, the decoder would requests a raw data. > > > > > > > > > > > > For the pps and rps, it is possible to reuse the slice header, just > > > > > > let > > > > > > the decoder know the offset from the bitstream bufer, I would > > > > > > suggest to > > > > > > add three properties(with sps) for them. But I think we need a > > > > > > method to > > > > > > mark a OUTPUT side buffer for those aux data. > > > > > > > > > > I'm quite confused about the hardware implementation then. From what > > > > > you're saying, it seems that it takes the raw bitstream elements > > > > > rather > > > > > than parsed elements. Is it really a stateless implementation? > > > > > > > > > > The stateless implementation was designed with the idea that only the > > > > > raw slice data should be passed in bitstream form to the decoder. For > > > > > H.264, it seems that some decoders also need the slice header in raw > > > > > bitstream form (because they take the full slice NAL unit), see the > > > > > discussions in this thread: > > > > > media: docs-rst: Document m2m stateless video decoder interface > > > > > > > > Stateless just mean it won’t track the previous result, but I don’t > > > > think you can define what a date the hardware would need. Even you > > > > just build a dpb for the decoder, it is still stateless, but parsing > > > > less or more data from the bitstream doesn’t stop a decoder become a > > > > stateless decoder. > > > > > > Yes fair enough, the format in which the hardware decoder takes the > > > bitstream parameters does not make it stateless or stateful per-se. > > > It's just that stateless decoders should have no particular reason for > > > parsing the bitstream on their own since the hardware can be designed > > > with registers for each relevant bitstream element to configure the > > > decoding pipeline. That's how GPU-based decoder implementations are > > > implemented (VAAPI/VDPAU/NVDEC, etc). > > > > > > So the format we have agreed on so far for the stateless interface is > > > to pass parsed elements via v4l2 control structures. > > > > > > If the hardware can only work by parsing the bitstream itself, I'm not > > > sure what the best solution would be. Reconstructing the bitstream in > > > the kernel is a pretty bad option, but so is parsing in the kernel or > > > having the data both in parsed and raw forms. Do you see another > > > possibility? > > > > Is reconstructing the bitstream so bad? The v4l2 controls provide a > > generic interface to an encoded format which the driver needs to > > convert into a sequence that the hardware can understand. Typically > > this is done by populating hardware-specific structures. Can't we > > consider that in this specific instance, the hardware-specific > > structure just happens to be identical to the original bitstream > > format? > > > > I agree that this is not strictly optimal for that particular > > hardware, but such is the cost of abstractions, and in this specific > > case I don't believe the cost would be particularly high? > > I mean, that argument can be made for the rockchip driver as well. If > reconstructing the bitstream is something we can do, and if we don't > care about being suboptimal for one particular hardware, then why the > rockchip driver doesn't just recreate the bitstream from that API? > > After all, this is just a hardware specific header that happens to be > identical to the original bitstream format I think in another thread (about H.264 I believe), we realized that it could be a good idea to just include the Slice NAL units in the Annex.B format in the buffers and that should work for all the hardware we could think of (given offsets to particular parts inside of the buffer). Wouldn't something similar work here for HEVC? I don't really get the meaning of "raw" for "cabac table, scaling list, picture parameter set and reference picture", since those are parts of the bitstream, which needs to be parsed to obtain those. Best regards, Tomasz ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: ipu3-imgu 0000:00:05.0: required queues are disabled
On 01/28/2019 11:45 PM, Kai Heng Feng wrote: Hi Kieran, On Jan 28, 2019, at 4:48 PM, Kieran Bingham wrote: Hi Kai-Heng, On 27/01/2019 05:56, Kai-Heng Feng wrote: Hi, We have a bug report [1] that the ipu3 doesn’t work. Does ipu3 need special userspace to work? Yes, it will need further userspace support to configure the pipeline, and to provide 3A algorithms for white balance, focus, and exposure times to the sensor. We are developing a stack called libcamera [0] to support this, but it's still in active development and not yet ready for use. Fortunately however, IPU3 is one of our primary initial targets. Thanks for the info. Hi, Kai-Heng, Like Bingham said, for IPU3 some heavy control from the userspace is needed. libcamera is a very good start. If you just want to verify the driver firstly, you can use script to take a try. [0] https://www.libcamera.org/ [1] https://bugs.launchpad.net/bugs/1812114 I have reported similar information to the launchpad bug entry. It might help if we can get hold of a Dell 7275 sometime although I think Mauro at least has one ? If this is a priority for Canonical, please contact us directly. Not really, just raise issues from Launchpad to appropriate mailing list. Kai-Heng Kai-Heng -- Regards Kieran ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
FINANCIAL MANAGEMENT AND ADVISORY SERVICES LIMITED 1
Dear de...@driverdev.osuosl.org, Please pardon me for this unsolicited communique. I do have the trusteeship of a PRIVATE investor with a stormy political background to outsource individuals with sound Financial Management abilities to manage over US$1.3B devoid of his name. These funds can be invested in tranches of US $100M or a tranche that is suitable for the portfolio manager. If you have Financial Management abilities, credible project in need of funding or existing business requiring expansion, your feedback would be appreciated. Sincerely, Martin Jones Managing Partner FINANCIAL MANAGEMENT AND ADVISORY SERVICES LIMITED ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] MIPS: Select PINCTRL_RT2880 when RALINK is enabled
This patch selects config PINCTRL_RT2880 when config RALINK is enabled as per drivers/staging/mt7621-pinctrl/TODO list. PINCTRL is also selected when RALINK is enabled to avoid config dependency warnings. Signed-off-by: Nishad Kamdar --- arch/mips/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index e2fc88da0223..cea529cf6284 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -623,6 +623,8 @@ config RALINK select CLKDEV_LOOKUP select ARCH_HAS_RESET_CONTROLLER select RESET_CONTROLLER + select PINCTRL + select PINCTRL_RT2880 config SGI_IP22 bool "SGI IP22 (Indy/Indigo2)" -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: mt7621-pinctrl: Test devm_kzalloc for failure while improving the code
This patch tests the return of devm_kzalloc for failure and
improves the code.
Signed-off-by: Nishad Kamdar
---
drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c
b/drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c
index 3e959fa73703..9b52d44abef1 100644
--- a/drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c
+++ b/drivers/staging/mt7621-pinctrl/pinctrl-rt2880.c
@@ -350,7 +350,6 @@ static int rt2880_pinmux_probe(struct platform_device *pdev)
for_each_compatible_node(np, NULL, "ralink,rt2880-gpio") {
const __be32 *ngpio, *gpiobase;
struct pinctrl_gpio_range *range;
- char *name;
if (!of_device_is_available(np))
continue;
@@ -362,9 +361,10 @@ static int rt2880_pinmux_probe(struct platform_device
*pdev)
return -EINVAL;
}
- range = devm_kzalloc(p->dev, sizeof(*range) + 4, GFP_KERNEL);
- range->name = name = (char *)&range[1];
- sprintf(name, "pio");
+ range = devm_kzalloc(p->dev, sizeof(*range), GFP_KERNEL);
+ if (!range)
+ return -ENOMEM;
+ range->name = "pio";
range->npins = __be32_to_cpu(*ngpio);
range->base = __be32_to_cpu(*gpiobase);
range->pin_base = range->base;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: erofs: keep corrupted fs from crashing kernel in erofs_namei()
As Al pointed out, "
... and while we are at it, what happens to
unsigned int nameoff = le16_to_cpu(de[mid].nameoff);
unsigned int matched = min(startprfx, endprfx);
struct qstr dname = QSTR_INIT(data + nameoff,
unlikely(mid >= ndirents - 1) ?
maxsize - nameoff :
le16_to_cpu(de[mid + 1].nameoff) - nameoff);
/* string comparison without already matched prefix */
int ret = dirnamecmp(name, &dname, &matched);
if le16_to_cpu(de[...].nameoff) is not monotonically increasing? I.e.
what's to prevent e.g. (unsigned)-1 ending up in dname.len?
Corrupted fs image shouldn't oops the kernel.. "
Revisit the related lookup flow to address the issue.
Fixes: d72d1ce60174 ("staging: erofs: add namei functions")
Cc: # 4.19+
Suggested-by: Al Viro
Signed-off-by: Gao Xiang
---
TODO:
- fix similar issue in readdir of dir.c with another patch.
drivers/staging/erofs/namei.c | 167 ++
1 file changed, 89 insertions(+), 78 deletions(-)
diff --git a/drivers/staging/erofs/namei.c b/drivers/staging/erofs/namei.c
index 5596c52e246d..a1300c420e63 100644
--- a/drivers/staging/erofs/namei.c
+++ b/drivers/staging/erofs/namei.c
@@ -15,74 +15,76 @@
#include
-/* based on the value of qn->len is accurate */
-static inline int dirnamecmp(struct qstr *qn,
- struct qstr *qd, unsigned int *matched)
+struct erofs_qstr {
+ const unsigned char *name;
+ const unsigned char *end;
+};
+
+/* based on the end of qn is accurate and it must have the trailing '\0' */
+static inline int dirnamecmp(const struct erofs_qstr *qn,
+const struct erofs_qstr *qd,
+unsigned int *matched)
{
- unsigned int i = *matched, len = min(qn->len, qd->len);
-loop:
- if (unlikely(i >= len)) {
- *matched = i;
- if (qn->len < qd->len) {
- /*
-* actually (qn->len == qd->len)
-* when qd->name[i] == '\0'
-*/
- return qd->name[i] == '\0' ? 0 : -1;
+ unsigned int i = *matched;
+
+ /*
+* on-disk error, let's only BUG_ON in the debugging mode.
+* otherwise, it will return 1 to just skip the invalid name
+* and go on (in consideration of the lookup performance).
+*/
+ DBG_BUGON(qd->name > qd->end);
+
+ /* qd could not have trailing '\0' */
+ /* However it is absolutely safe if < qd->end */
+ while (qd->name + i < qd->end && qd->name[i] != '\0') {
+ if (qn->name[i] != qd->name[i]) {
+ *matched = i;
+ return qn->name[i] > qd->name[i] ? 1 : -1;
}
- return (qn->len > qd->len);
+ ++i;
}
-
- if (qn->name[i] != qd->name[i]) {
- *matched = i;
- return qn->name[i] > qd->name[i] ? 1 : -1;
- }
-
- ++i;
- goto loop;
+ *matched = i;
+ /* See comments in __d_alloc on the terminating NUL character */
+ return qn->name[i] == '\0' ? 0 : 1;
}
-static struct erofs_dirent *find_target_dirent(
- struct qstr *name,
- u8 *data, int maxsize)
+#define nameoff_from_disk(off, sz) (le16_to_cpu(off) & ((sz) - 1))
+
+static struct erofs_dirent *find_target_dirent(struct erofs_qstr *name,
+ u8 *data,
+ unsigned int dirblksize,
+ const int ndirents)
{
- unsigned int ndirents, head, back;
+ int head, back;
unsigned int startprfx, endprfx;
struct erofs_dirent *const de = (struct erofs_dirent *)data;
- /* make sure that maxsize is valid */
- BUG_ON(maxsize < sizeof(struct erofs_dirent));
-
- ndirents = le16_to_cpu(de->nameoff) / sizeof(*de);
-
- /* corrupted dir (may be unnecessary...) */
- BUG_ON(!ndirents);
-
head = 0;
back = ndirents - 1;
startprfx = endprfx = 0;
while (head <= back) {
- unsigned int mid = head + (back - head) / 2;
- unsigned int nameoff = le16_to_cpu(de[mid].nameoff);
+ const int mid = head + (back - head) / 2;
+ const int nameoff = nameoff_from_disk(de[mid].nameoff,
+ dirblksize);
unsigned int matched = min(startprfx, endprfx);
-
- struct qstr dname = QSTR_INIT(data + nameoff,
- unlikely(mid >= ndirents - 1) ?
- maxsize - nameoff :
- le16_to_cpu(de[mid + 1].nameoff) - nameoff);
+ struct erofs_qstr dname = {
+ .name = data + nameoff,
+ .end = unlikely(mid >=
REVERT BACK FOR MORE DETAILS .
DEAR MY NAME IS STANLEY PITCH I GOT YOUR EMAIL ADDRESS FROM THE INTERNET I AM A LAWYER AND A FINANCIAL INSTRUMENTS PROCUREMENT CONSULTANTS I AM CONTACTING YOU WITH THIS OFFER IN CASE YOU HAVE NEED FOR LOAN FOR YOUR PROJECTS. THIS IS TO INFORM YOU THAT ONE OF MY CLIENTS IN HONG KONG HAS CLEAN/CLEAR FUNDS REALIZED FROM CRUDE OLS TRANSACTIONS TO LOAN OUT TO POTENTIAL CLIENTS WITH PROJECTS AND FUND MANAGERS. THE LOAN DURATION WILL BE FOR 5 -10 YEARS PERIOD. I AM IN POSSESSION OF THE POF AND ATV OF THIS FUNDS MADE FROM EXCESS CRUDE OIL SALES/LIFTING. THE POF IS FOR 50B HE IS LOANING OUT THE FUND AT THE ANNUAL INTEREST RATE OF 2% IF YOU ARE INTERESTED, REVERT FOR DETAILS. YOU WILL HAVE A REASONABLE PERCENTAGE FOR THIS HITCH-FREE TRANSACTION ALL DOCUMENTS FOR THE FUNDS ARE INTACT AS CLEAN, CLEAR AND FREE FROM DRUG RELATED FUNDS REVERT BACK FOR MORE DETAILS WAITING STANLEY ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging:iio:ad7152: align line to match open parenthesis
From: Feulo Align broken line to match upper line parenthesis. Solves the checkpatch.pl's message CHECK: Alignment should match open parenthesis. Signed-off-by: Feulo Signed-off-by: Camylla Cantanheide --- drivers/staging/iio/cdc/ad7152.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c index 25f51db..e7f082f 100644 --- a/drivers/staging/iio/cdc/ad7152.c +++ b/drivers/staging/iio/cdc/ad7152.c @@ -362,7 +362,7 @@ static int ad7152_read_raw(struct iio_dev *indio_dev, /* Trigger a single read */ regval |= AD7152_CONF_MODE_SINGLE_CONV; ret = i2c_smbus_write_byte_data(chip->client, AD7152_REG_CFG, - regval); + regval); if (ret < 0) goto out; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: wilc1000: remove redundant reset of station statistics
On 1/29/19 1:39 AM, Dan Carpenter wrote:
> On Mon, Jan 28, 2019 at 10:38:20PM +, adham.aboza...@microchip.com wrote:
>> From: Adham Abozaeid
>>
>> Driver sends configuration wids to reset connection statistics in the
>> device, but the device already resets it when starting a new connection
>>
>> Signed-off-by: Adham Abozaeid
>> ---
>> drivers/staging/wilc1000/host_interface.c | 20 +---
>> 1 file changed, 1 insertion(+), 19 deletions(-)
>>
>> diff --git a/drivers/staging/wilc1000/host_interface.c
>> b/drivers/staging/wilc1000/host_interface.c
>> index fbb61de20304..286685e426c1 100644
>> --- a/drivers/staging/wilc1000/host_interface.c
>> +++ b/drivers/staging/wilc1000/host_interface.c
>> @@ -342,29 +342,11 @@ static int wilc_send_connect_wid(struct wilc_vif *vif)
>> {
>> int result = 0;
>> struct wid wid_list[8];
>^^^
> We could make this array smaller as well.
Agree. Will send v2 with smaller array.
>
>> -u32 wid_cnt = 0, dummyval = 0;
>> +u32 wid_cnt = 0;
> regards,
> dan carpenter
Thanks,
Adham
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: wilc1000: remove redundant reset of station statistics
From: Adham Abozaeid
Driver sends configuration wids to reset connection statistics in the
device, but the device already resets it when starting a new connection
Signed-off-by: Adham Abozaeid
---
Changes in v2:
- Decrease the size of wid_list array
drivers/staging/wilc1000/host_interface.c | 22 ++
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index fbb61de20304..e958f9b935cc 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -341,30 +341,12 @@ int wilc_scan(struct wilc_vif *vif, u8 scan_source, u8
scan_type,
static int wilc_send_connect_wid(struct wilc_vif *vif)
{
int result = 0;
- struct wid wid_list[8];
- u32 wid_cnt = 0, dummyval = 0;
+ struct wid wid_list[4];
+ u32 wid_cnt = 0;
struct host_if_drv *hif_drv = vif->hif_drv;
struct wilc_conn_info *conn_attr = &hif_drv->conn_info;
struct wilc_join_bss_param *bss_param = conn_attr->param;
- wid_list[wid_cnt].id = WID_SUCCESS_FRAME_COUNT;
- wid_list[wid_cnt].type = WID_INT;
- wid_list[wid_cnt].size = sizeof(u32);
- wid_list[wid_cnt].val = (s8 *)(&(dummyval));
- wid_cnt++;
-
- wid_list[wid_cnt].id = WID_RECEIVED_FRAGMENT_COUNT;
- wid_list[wid_cnt].type = WID_INT;
- wid_list[wid_cnt].size = sizeof(u32);
- wid_list[wid_cnt].val = (s8 *)(&(dummyval));
- wid_cnt++;
-
- wid_list[wid_cnt].id = WID_FAILED_COUNT;
- wid_list[wid_cnt].type = WID_INT;
- wid_list[wid_cnt].size = sizeof(u32);
- wid_list[wid_cnt].val = (s8 *)(&(dummyval));
- wid_cnt++;
-
wid_list[wid_cnt].id = WID_INFO_ELEMENT_ASSOCIATE;
wid_list[wid_cnt].type = WID_BIN_DATA;
wid_list[wid_cnt].val = conn_attr->req_ies;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show functions
> From: Kimberly Brown > > ... > > But as you pointed, at least for sub-channels, channel->ringbuffer_page > > can indeed disappear in vmbus_close() -> ... -> vmbus_free_ring(), and > > the "attribute->show()" could crash when the race happens. > > Adding channel_mutex here seems to be able to fix the race for > > sub-channels, as the same channel_mutex is used in > vmbus_disconnect_ring(). > > > > For a primary channel, vmbus_close() -> vmbus_free_ring() can still > > free channel->ringbuffer_page without notifying the "attribute->show()". > > We may also need to acquire/release the channel_mutex in vmbus_close()? > > > > > Actually, there should be checks that "chan" is not null and that > > > "chan->state" is CHANNEL_OPENED_STATE within the locked section. I'll > > > need to fix that. > > I suppose "chan" can not be NULL here (see the above). > > > > Checking "chan->state" may not help to completely fix the race, because > > there is no locking/synchronization code in > > vmbus_close_internal() when we test and change "channel->state". > > > > The calls to vmbus_close_internal() for the subchannels and the primary > channel are protected with channel_mutex in vmbus_disconnect_ring(). > This prevents "channel->state" from changing while "attribute->show()" is > running. Ah, I think you're right. > > I guess we may need to check if channel->ringbuffer_page is NULL in > > the "attribute->show()". > > > > For the primary channel, vmbus_free_ring() is called after the > return from vmbus_disconnect_ring(). Therefore, the primary channel's > state is changed before "channel->ringbuffer_page" is set to NULL. > Checking the channel state should be sufficient to prevent the ring > buffers from being freed while "attribute->show()" is running. The > ring buffers can't be freed until the channel's state is changed, and > the channel state change is protected by the mutex. I think you're right (I noticed in a previous mail you mentioned you would improve your patch to check "chan->state" with the mutax held). > I think checking that "channel->ringbuffer_page" is not NULL would > also work, but, as you stated, we would need to aquire/release > channel_mutex in vmbus_close(). Then it looks unnecessary to check "channel->ringbuffer_page". > > PS, to prove that a race condition does exist and can really cause a panic > > or > > something, I usually add some msleep() delays in different paths so that I > > can reproduce the crash every time I take a special action, e.g. when I read > > the sysfs files of a NIC, I try to remove hv_netvsc driver. This way, I can > > prove > > a patch can indeed help, at least it can fix the crash which would happen > > without the patch. :-) > > > > Thanks! I was able to free the ring buffers while "attribute->show()" > was running, which caused a null pointer dereference bug. As expected, > the mutex lock fixed it. Awesome! -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v3 1/2] PCI: hv: Replace hv_vp_set with hv_vpset
From: Maya Nakamura Sent: Monday, January 28, 2019 11:18 PM > > Remove a duplicate definition of VP set (hv_vp_set) and use the common > definition (hv_vpset) that is used in other places. > > Change the order of the members in struct hv_pcibus_device so that the > declaration of retarget_msi_interrupt_params is the last member. Struct > hv_vpset, which contains a flexible array, is nested two levels deep in > struct hv_pcibus_device via retarget_msi_interrupt_params. > > Add a comment that retarget_msi_interrupt_params should be the last member > of struct hv_pcibus_device. > > Signed-off-by: Maya Nakamura > --- > Change in v3: > - Correct the v2 change log. > > Change in v2: > - Update the commit message. > Reviewed-by: Michael Kelley ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v3 2/2] PCI: hv: Refactor hv_irq_unmask() to use cpumask_to_vpset()
From: Maya Nakamura Sent: Monday, January 28, 2019 11:21 PM > > Remove the duplicate implementation of cpumask_to_vpset() and use the > shared implementation. Export hv_max_vp_index, which is required by > cpumask_to_vpset(). > > Apply changes to hv_irq_unmask() based on feedback. > > Signed-off-by: Maya Nakamura > --- > Changes in v3: > - Modify to catch all failures from cpumask_to_vpset(). > - Correct the v2 change log about the commit message. > > Changes in v2: > - Remove unnecessary nr_bank initialization. > - Delete two unnecessary dev_err()'s. > - Unlock before returning. > - Update the commit message. > Reviewed-by: Michael Kelley ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls
Le mardi 29 janvier 2019 à 16:44 +0900, Alexandre Courbot a écrit : > On Fri, Jan 25, 2019 at 10:04 PM Paul Kocialkowski > wrote: > > Hi, > > > > On Thu, 2019-01-24 at 20:23 +0800, Ayaka wrote: > > > Sent from my iPad > > > > > > > On Jan 24, 2019, at 6:27 PM, Paul Kocialkowski > > > > wrote: > > > > > > > > Hi, > > > > > > > > > On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote: > > > > > I forget a important thing, for the rkvdec and rk hevc decoder, it > > > > > would > > > > > requests cabac table, scaling list, picture parameter set and > > > > > reference > > > > > picture storing in one or various of DMA buffers. I am not talking > > > > > about > > > > > the data been parsed, the decoder would requests a raw data. > > > > > > > > > > For the pps and rps, it is possible to reuse the slice header, just > > > > > let > > > > > the decoder know the offset from the bitstream bufer, I would suggest > > > > > to > > > > > add three properties(with sps) for them. But I think we need a method > > > > > to > > > > > mark a OUTPUT side buffer for those aux data. > > > > > > > > I'm quite confused about the hardware implementation then. From what > > > > you're saying, it seems that it takes the raw bitstream elements rather > > > > than parsed elements. Is it really a stateless implementation? > > > > > > > > The stateless implementation was designed with the idea that only the > > > > raw slice data should be passed in bitstream form to the decoder. For > > > > H.264, it seems that some decoders also need the slice header in raw > > > > bitstream form (because they take the full slice NAL unit), see the > > > > discussions in this thread: > > > > media: docs-rst: Document m2m stateless video decoder interface > > > > > > Stateless just mean it won’t track the previous result, but I don’t > > > think you can define what a date the hardware would need. Even you > > > just build a dpb for the decoder, it is still stateless, but parsing > > > less or more data from the bitstream doesn’t stop a decoder become a > > > stateless decoder. > > > > Yes fair enough, the format in which the hardware decoder takes the > > bitstream parameters does not make it stateless or stateful per-se. > > It's just that stateless decoders should have no particular reason for > > parsing the bitstream on their own since the hardware can be designed > > with registers for each relevant bitstream element to configure the > > decoding pipeline. That's how GPU-based decoder implementations are > > implemented (VAAPI/VDPAU/NVDEC, etc). > > > > So the format we have agreed on so far for the stateless interface is > > to pass parsed elements via v4l2 control structures. > > > > If the hardware can only work by parsing the bitstream itself, I'm not > > sure what the best solution would be. Reconstructing the bitstream in > > the kernel is a pretty bad option, but so is parsing in the kernel or > > having the data both in parsed and raw forms. Do you see another > > possibility? > > Is reconstructing the bitstream so bad? The v4l2 controls provide a > generic interface to an encoded format which the driver needs to > convert into a sequence that the hardware can understand. Typically > this is done by populating hardware-specific structures. Can't we > consider that in this specific instance, the hardware-specific > structure just happens to be identical to the original bitstream > format? At maximum allowed bitrate for let's say HEVC (940MB/s iirc), yes, it would be really really bad. In GStreamer project we have discussed for a while (but have never done anything about) adding the ability through a bitmask to select which part of the stream need to be parsed, as parsing itself was causing some overhead. Maybe similar thing applies, though as per our new design, it's the fourcc that dictate the driver behaviour, we'd need yet another fourcc for drivers that wants the full bitstream (which seems odd if you have already parsed everything, I think this need some clarification). > > I agree that this is not strictly optimal for that particular > hardware, but such is the cost of abstractions, and in this specific > case I don't believe the cost would be particularly high? signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: staging/intel-ipu3: Implement lock for stream on/off operations
Currently concurrent stream off operations on ImgU nodes are not
synchronized, leading to use-after-free bugs (as reported by KASAN).
[ 250.090724] BUG: KASAN: use-after-free in ipu3_dmamap_free+0xc5/0x116
[ipu3_imgu]
[ 250.090726] Read of size 8 at addr 888127b29bc0 by task yavta/18836
[ 250.090731] Hardware name: HP Soraka/Soraka, BIOS Google_Soraka.10431.17.0
03/22/2018
[ 250.090732] Call Trace:
[ 250.090735] dump_stack+0x6a/0xb1
[ 250.090739] print_address_description+0x8e/0x279
[ 250.090743] ? ipu3_dmamap_free+0xc5/0x116 [ipu3_imgu]
[ 250.090746] kasan_report+0x260/0x28a
[ 250.090750] ipu3_dmamap_free+0xc5/0x116 [ipu3_imgu]
[ 250.090754] ipu3_css_pool_cleanup+0x24/0x37 [ipu3_imgu]
[ 250.090759] ipu3_css_pipeline_cleanup+0x61/0xb9 [ipu3_imgu]
[ 250.090763] ipu3_css_stop_streaming+0x1f2/0x321 [ipu3_imgu]
[ 250.090768] imgu_s_stream+0x94/0x443 [ipu3_imgu]
[ 250.090772] ? ipu3_vb2_buf_queue+0x280/0x280 [ipu3_imgu]
[ 250.090775] ? vb2_dma_sg_unmap_dmabuf+0x16/0x6f [videobuf2_dma_sg]
[ 250.090778] ? vb2_buffer_in_use+0x36/0x58 [videobuf2_common]
[ 250.090782] ipu3_vb2_stop_streaming+0xf9/0x135 [ipu3_imgu]
Implemented a lock to synchronize imgu stream on / off operations and
the modification of streaming flag (in struct imgu_device), to prevent
these issues.
Reported-by: Laurent Pinchart
Suggested-by: Laurent Pinchart
Signed-off-by: Rajmohan Mani
---
drivers/staging/media/ipu3/ipu3-v4l2.c | 6 ++
drivers/staging/media/ipu3/ipu3.c | 3 +++
drivers/staging/media/ipu3/ipu3.h | 4
3 files changed, 13 insertions(+)
diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c
b/drivers/staging/media/ipu3/ipu3-v4l2.c
index c7936032beb9..cf7e917cd0c8 100644
--- a/drivers/staging/media/ipu3/ipu3-v4l2.c
+++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
@@ -507,12 +507,15 @@ static int ipu3_vb2_start_streaming(struct vb2_queue *vq,
unsigned int count)
goto fail_stop_pipeline;
}
+ mutex_lock(&imgu->streaming_lock);
+
/* Start streaming of the whole pipeline now */
dev_dbg(dev, "IMGU streaming is ready to start");
r = imgu_s_stream(imgu, true);
if (!r)
imgu->streaming = true;
+ mutex_unlock(&imgu->streaming_lock);
return 0;
fail_stop_pipeline:
@@ -543,6 +546,8 @@ static void ipu3_vb2_stop_streaming(struct vb2_queue *vq)
dev_err(&imgu->pci_dev->dev,
"failed to stop subdev streaming\n");
+ mutex_lock(&imgu->streaming_lock);
+
/* Was this the first node with streaming disabled? */
if (imgu->streaming && ipu3_all_nodes_streaming(imgu, node)) {
/* Yes, really stop streaming now */
@@ -552,6 +557,7 @@ static void ipu3_vb2_stop_streaming(struct vb2_queue *vq)
imgu->streaming = false;
}
+ mutex_unlock(&imgu->streaming_lock);
ipu3_return_all_buffers(imgu, node, VB2_BUF_STATE_ERROR);
media_pipeline_stop(&node->vdev.entity);
}
diff --git a/drivers/staging/media/ipu3/ipu3.c
b/drivers/staging/media/ipu3/ipu3.c
index d521b3afb8b1..2daee51cd845 100644
--- a/drivers/staging/media/ipu3/ipu3.c
+++ b/drivers/staging/media/ipu3/ipu3.c
@@ -635,6 +635,7 @@ static int imgu_pci_probe(struct pci_dev *pci_dev,
return r;
mutex_init(&imgu->lock);
+ mutex_init(&imgu->streaming_lock);
atomic_set(&imgu->qbuf_barrier, 0);
init_waitqueue_head(&imgu->buf_drain_wq);
@@ -699,6 +700,7 @@ static int imgu_pci_probe(struct pci_dev *pci_dev,
ipu3_css_set_powerdown(&pci_dev->dev, imgu->base);
out_mutex_destroy:
mutex_destroy(&imgu->lock);
+ mutex_destroy(&imgu->streaming_lock);
return r;
}
@@ -716,6 +718,7 @@ static void imgu_pci_remove(struct pci_dev *pci_dev)
ipu3_dmamap_exit(imgu);
ipu3_mmu_exit(imgu->mmu);
mutex_destroy(&imgu->lock);
+ mutex_destroy(&imgu->streaming_lock);
}
static int __maybe_unused imgu_suspend(struct device *dev)
diff --git a/drivers/staging/media/ipu3/ipu3.h
b/drivers/staging/media/ipu3/ipu3.h
index 04fc99f47ebb..f732315f0701 100644
--- a/drivers/staging/media/ipu3/ipu3.h
+++ b/drivers/staging/media/ipu3/ipu3.h
@@ -146,6 +146,10 @@ struct imgu_device {
* vid_buf.list and css->queue
*/
struct mutex lock;
+
+ /* Lock to protect writes to streaming flag in this struct */
+ struct mutex streaming_lock;
+
/* Forbit streaming and buffer queuing during system suspend. */
atomic_t qbuf_barrier;
/* Indicate if system suspend take place while imgu is streaming. */
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] MIPS: Select PINCTRL_RT2880 when RALINK is enabled
Hi Nishad, On Tue, Jan 29, 2019 at 08:55:27PM +0530, Nishad Kamdar wrote: > This patch selects config PINCTRL_RT2880 when config RALINK is > enabled as per drivers/staging/mt7621-pinctrl/TODO list. PINCTRL > is also selected when RALINK is enabled to avoid config dependency > warnings. > > Signed-off-by: Nishad Kamdar > --- > arch/mips/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig > index e2fc88da0223..cea529cf6284 100644 > --- a/arch/mips/Kconfig > +++ b/arch/mips/Kconfig > @@ -623,6 +623,8 @@ config RALINK > select CLKDEV_LOOKUP > select ARCH_HAS_RESET_CONTROLLER > select RESET_CONTROLLER > + select PINCTRL > + select PINCTRL_RT2880 > > config SGI_IP22 > bool "SGI IP22 (Indy/Indigo2)" Wouldn't this also require selecting STAGING? Perhaps that's why it wasn't done in the first place? Thanks, Paul ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: android: ion: Restrict cache maintenance to dma mapped memory
On Fri, 18 Jan 2019, Liam Mark wrote:
> On Fri, 18 Jan 2019, Andrew F. Davis wrote:
>
> > On 1/18/19 12:37 PM, Liam Mark wrote:
> > > The ION begin_cpu_access and end_cpu_access functions use the
> > > dma_sync_sg_for_cpu and dma_sync_sg_for_device APIs to perform cache
> > > maintenance.
> > >
> > > Currently it is possible to apply cache maintenance, via the
> > > begin_cpu_access and end_cpu_access APIs, to ION buffers which are not
> > > dma mapped.
> > >
> > > The dma sync sg APIs should not be called on sg lists which have not been
> > > dma mapped as this can result in cache maintenance being applied to the
> > > wrong address. If an sg list has not been dma mapped then its dma_address
> > > field has not been populated, some dma ops such as the swiotlb_dma_ops ops
> > > use the dma_address field to calculate the address onto which to apply
> > > cache maintenance.
> > >
> > > Also I don’t think we want CMOs to be applied to a buffer which is not
> > > dma mapped as the memory should already be coherent for access from the
> > > CPU. Any CMOs required for device access taken care of in the
> > > dma_buf_map_attachment and dma_buf_unmap_attachment calls.
> > > So really it only makes sense for begin_cpu_access and end_cpu_access to
> > > apply CMOs if the buffer is dma mapped.
> > >
> > > Fix the ION begin_cpu_access and end_cpu_access functions to only apply
> > > cache maintenance to buffers which are dma mapped.
> > >
> > > Fixes: 2a55e7b5e544 ("staging: android: ion: Call dma_map_sg for syncing
> > > and mapping")
> > > Signed-off-by: Liam Mark
> > > ---
> > > drivers/staging/android/ion/ion.c | 26 +-
> > > 1 file changed, 21 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/staging/android/ion/ion.c
> > > b/drivers/staging/android/ion/ion.c
> > > index 6f5afab7c1a1..1fe633a7fdba 100644
> > > --- a/drivers/staging/android/ion/ion.c
> > > +++ b/drivers/staging/android/ion/ion.c
> > > @@ -210,6 +210,7 @@ struct ion_dma_buf_attachment {
> > > struct device *dev;
> > > struct sg_table *table;
> > > struct list_head list;
> > > + bool dma_mapped;
> > > };
> > >
> > > static int ion_dma_buf_attach(struct dma_buf *dmabuf,
> > > @@ -231,6 +232,7 @@ static int ion_dma_buf_attach(struct dma_buf *dmabuf,
> > >
> > > a->table = table;
> > > a->dev = attachment->dev;
> > > + a->dma_mapped = false;
> > > INIT_LIST_HEAD(&a->list);
> > >
> > > attachment->priv = a;
> > > @@ -261,12 +263,18 @@ static struct sg_table *ion_map_dma_buf(struct
> > > dma_buf_attachment *attachment,
> > > {
> > > struct ion_dma_buf_attachment *a = attachment->priv;
> > > struct sg_table *table;
> > > + struct ion_buffer *buffer = attachment->dmabuf->priv;
> > >
> > > table = a->table;
> > >
> > > + mutex_lock(&buffer->lock);
> > > if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
> > > - direction))
> > > + direction)) {
> > > + mutex_unlock(&buffer->lock);
> > > return ERR_PTR(-ENOMEM);
> > > + }
> > > + a->dma_mapped = true;
> > > + mutex_unlock(&buffer->lock);
> > >
> > > return table;
> > > }
> > > @@ -275,7 +283,13 @@ static void ion_unmap_dma_buf(struct
> > > dma_buf_attachment *attachment,
> > > struct sg_table *table,
> > > enum dma_data_direction direction)
> > > {
> > > + struct ion_dma_buf_attachment *a = attachment->priv;
> > > + struct ion_buffer *buffer = attachment->dmabuf->priv;
> > > +
> > > + mutex_lock(&buffer->lock);
> > > dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
> > > + a->dma_mapped = false;
> > > + mutex_unlock(&buffer->lock);
> > > }
> > >
> > > static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
> > > @@ -346,8 +360,9 @@ static int ion_dma_buf_begin_cpu_access(struct
> > > dma_buf *dmabuf,
> > >
> > > mutex_lock(&buffer->lock);
> > > list_for_each_entry(a, &buffer->attachments, list) {
> >
> > When no devices are attached then buffer->attachments is empty and the
> > below does not run, so if I understand this patch correctly then what
> > you are protecting against is CPU access in the window after
> > dma_buf_attach but before dma_buf_map.
> >
>
> Yes
>
> > This is the kind of thing that again makes me think a couple more
> > ordering requirements on DMA-BUF ops are needed. DMA-BUFs do not require
> > the backing memory to be allocated until map time, this is why the
> > dma_address field would still be null as you note in the commit message.
> > So why should the CPU be performing accesses on a buffer that is not
> > actually backed yet?
> >
> > I can think of two solutions:
> >
> > 1) Only allow CPU access (mmap, kmap, {begin,end}_cpu_access) while at
> > least one device is mapped.
> >
>
> Would be quite limiting to clients.
>
> > 2) Treat the CPU access request like the a device map request and
> > trigger the allocation of backing memory ju
RE: [PATCH v3] staging: android: ion: Add implementation of dma_buf_vmap and dma_buf_vunmap
On Fri, 4 Jan 2019, Skidanov, Alexey wrote: > > > > -Original Message- > > From: Liam Mark [mailto:lm...@codeaurora.org] > > Sent: Friday, January 04, 2019 19:42 > > To: Skidanov, Alexey > > Cc: Laura Abbott ; Greg KH ; > > de...@driverdev.osuosl.org; tk...@android.com; r...@android.com; linux- > > ker...@vger.kernel.org; m...@android.com; sumit.sem...@linaro.org > > Subject: Re: [PATCH v3] staging: android: ion: Add implementation of > > dma_buf_vmap and > > dma_buf_vunmap > > > > On Tue, 18 Dec 2018, Alexey Skidanov wrote: > > > > > >>> I was wondering if we could re-open the discussion on adding support > > > >>> to > > > >>> ION for dma_buf_vmap. > > > >>> It seems like the patch was not taken as the reviewers wanted more > > > >>> evidence of an upstream use case. > > > >>> > > > >>> Here would be my upstream usage argument for including dma_buf_vmap > > > >>> support in ION. > > > >>> > > > >>> Currently all calls to ion_dma_buf_begin_cpu_access result in the > > > >>> creation > > > >>> of a kernel mapping for the buffer, unfortunately the resulting call > > > >>> to > > > >>> alloc_vmap_area can be quite expensive and this has caused a > > > >>> performance > > > >>> regression for certain clients when they have moved to the new > > > >>> version of > > > >>> ION. > > > >>> > > > >>> The kernel mapping is not actually needed in > > > >>> ion_dma_buf_begin_cpu_access, > > > >>> and generally isn't needed by clients. So if we remove the creation > > > >>> of the > > > >>> kernel mapping in ion_dma_buf_begin_cpu_access and only create it when > > > >>> needed we can speed up the calls to ion_dma_buf_begin_cpu_access. > > > >>> > > > >>> An additional benefit of removing the creation of kernel mappings from > > > >>> ion_dma_buf_begin_cpu_access is that it makes the ION code more > > > >>> secure. > > > >>> Currently a malicious client could call the DMA_BUF_IOCTL_SYNC IOCTL > > > >>> with > > > >>> flags DMA_BUF_SYNC_END multiple times to cause the ION buffer > > > >>> kmap_cnt to > > > >>> go negative which could lead to undesired behavior. > > > >>> > > > >>> One disadvantage of the above change is that a kernel mapping is not > > > >>> already created when a client calls dma_buf_kmap. So the following > > > >>> dma_buf_kmap contract can't be satisfied. > > > >>> > > > >>> /** > > > >>> * dma_buf_kmap - Map a page of the buffer object into kernel address > > > >>> space. The > > > >>> * same restrictions as for kmap and friends apply. > > > >>> * @dmabuf:[in]buffer to map page from. > > > >>> * @page_num: [in]page in PAGE_SIZE units to map. > > > >>> * > > > >>> * This call must always succeed, any necessary preparations that might > > > >>> fail > > > >>> * need to be done in begin_cpu_access. > > > >>> */ > > > >>> > > > >>> But hopefully we can work around this by moving clients to > > > >>> dma_buf_vmap. > > > >> I think the problem is with the contract. We can't ensure that the call > > > >> is always succeeds regardless the implementation - any mapping might > > > >> fail. Probably this is why *all* clients of dma_buf_kmap() check the > > > >> return value (so it's safe to return NULL in case of failure). > > > >> > > > > > > > > I think currently the call to dma_buf_kmap will always succeed since the > > > > DMA-Buf contract requires that the client first successfully call > > > > dma_buf_begin_cpu_access(), and if dma_buf_begin_cpu_access() succeeds > > > > then dma_buf_kmap will succeed. > > > > > > > >> I would suggest to fix the contract and to keep the dma_buf_kmap() > > > >> support in ION. > > > > > > > > I will leave it to the DMA-Buf maintainers as to whether they want to > > > > change their contract. > > > > > > > > Liam > > > > > > > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > > > > a Linux Foundation Collaborative Project > > > > > > > > > > Ok. We need the list of the clients using the ION in the mainline tree. > > > > > > > Looks to me like the only functions which might be calling > > dma_buf_kmap/dma_buf_kunmap on ION buffers are > > tegra_bo_kmap/tegra_bo_kunmap, I assume Tegra is used in some Android > > automotive products. > > > > Looks like these functions could be moved over to using > > dma_buf_vmap/dma_buf_vunmap but it wouldn't be very clean and would add a > > performance hit. > > > > Liam > > > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > > a Linux Foundation Collaborative Project > > I'm a little bit confused. Why making the buffer accessible by CPU (mapping > the buffer) > and making the content of the buffer valid (coherent) are so tightly coupled > in DMA-BUF? > Hi Sumit, Hope you are feeling better. I was wondering if you would be open to changes to to the DMA-BUF contract so that we can remove the creation of kernel mappings in begin_cpu_access. This would have the benefit of improving the performance of begin_cpu_access and removing the ability for use
Re: [PATCH 1/7] binder: create userspace-to-binder-buffer copy function
On Tue, Jan 29, 2019 at 12:12 AM Dan Carpenter wrote:
>
> On Mon, Jan 28, 2019 at 04:49:28PM -0800, Todd Kjos wrote:
> > +/**
> > + * binder_alloc_copy_user_to_buffer() - copy src user to tgt user
> > + * @alloc: binder_alloc for this proc
> > + * @buffer: binder buffer to be accessed
> > + * @buffer_offset: offset into @buffer data
> > + * @from: userspace pointer to source buffer
> > + * @bytes: bytes to copy
> > + *
> > + * Copy bytes from source userspace to target buffer.
> > + *
> > + * Return: bytes remaining to be copied
> > + */
> > +unsigned long
> > +binder_alloc_copy_user_to_buffer(struct binder_alloc *alloc,
> > + struct binder_buffer *buffer,
> > + binder_size_t buffer_offset,
> > + const void __user *from,
> > + size_t bytes)
> > +{
> > + if (!check_buffer(alloc, buffer, buffer_offset, bytes))
> > + return bytes;
> > +
> > + while (bytes) {
> > + unsigned long size;
> > + unsigned long ret;
> > + struct page *page;
> > + pgoff_t pgoff;
> > + void *kptr;
> > +
> > + page = binder_alloc_get_page(alloc, buffer,
> > + buffer_offset, &pgoff);
> > + size = min(bytes, (size_t)(PAGE_SIZE - pgoff));
>
> This code has so much more casting than necessary. To me casting says
> that we haven't got the types correct or something. I've just pulled
> this function out as an example really, but none of the casts here are
> required... This could just be:
>
> size = min(bytes, PAGE_SIZE - pgoff);
Dan,
Thanks for the feedback.
The one above: "size = min(bytes, PAGE_SIZE - pgoff);", results in
32-bit compile warnings:
./include/linux/kernel.h:846:29: warning: comparison of distinct
pointer types lacks a cast
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
[...]
drivers/android/binder_alloc.c:1157:10: note: in expansion of macro ‘min’
size = min(bytes, PAGE_SIZE - pgoff);
so, I took your suggestion:
size = min_t(size_t, bytes, PAGE_SIZE - pgoff);
>
> Btw, if you really need to do a cast inside a min() (you don't in this
> cast) then use min_t().
>
> > + kptr = (void *)((uintptr_t)kmap(page) + pgoff);
>
> This would be a lot cleaner as:
>
> kptr = kmap(page) + pgoff;
It definitely is cleaner, but the clean version is doing arithmetic on
a void pointer which is generally not considered good practice (but is
supported by gcc). Is this acceptable in the kernel? I don't see any
statement on it in Documentation/process/coding-style.rst.
If so, I agree a bunch of these casts can go as you suggest.
-Todd
[...]
>
> regards,
> dan carpenter
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] nfit: acpi_nfit_ctl(): check out_obj->type in the right place
In the case of ND_CMD_CALL, we should also check out_obj->type.
The patch uses out_obj->type, which is a short alias to
out_obj->package.type.
Fixes: 31eca76ba2fc ("nfit, libnvdimm: limited/whitelisted dimm command
marshaling mechanism")
Cc:
Signed-off-by: Dexuan Cui
---
drivers/acpi/nfit/core.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 0a49c57334cc..1fa378435dd2 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -554,6 +554,13 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
return -EINVAL;
}
+ if (out_obj->type != ACPI_TYPE_BUFFER) {
+ dev_dbg(dev, "%s unexpected output object type cmd: %s type:
%d\n",
+ dimm_name, cmd_name, out_obj->type);
+ rc = -EINVAL;
+ goto out;
+ }
+
if (call_pkg) {
call_pkg->nd_fw_size = out_obj->buffer.length;
memcpy(call_pkg->nd_payload + call_pkg->nd_size_in,
@@ -572,13 +579,6 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
return 0;
}
- if (out_obj->package.type != ACPI_TYPE_BUFFER) {
- dev_dbg(dev, "%s unexpected output object type cmd: %s type:
%d\n",
- dimm_name, cmd_name, out_obj->type);
- rc = -EINVAL;
- goto out;
- }
-
dev_dbg(dev, "%s cmd: %s output length: %d\n", dimm_name,
cmd_name, out_obj->buffer.length);
print_hex_dump_debug(cmd_name, DUMP_PREFIX_OFFSET, 4, 4,
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls
On Wed, Jan 30, 2019 at 6:41 AM Nicolas Dufresne wrote: > > Le mardi 29 janvier 2019 à 16:44 +0900, Alexandre Courbot a écrit : > > On Fri, Jan 25, 2019 at 10:04 PM Paul Kocialkowski > > wrote: > > > Hi, > > > > > > On Thu, 2019-01-24 at 20:23 +0800, Ayaka wrote: > > > > Sent from my iPad > > > > > > > > > On Jan 24, 2019, at 6:27 PM, Paul Kocialkowski > > > > > wrote: > > > > > > > > > > Hi, > > > > > > > > > > > On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote: > > > > > > I forget a important thing, for the rkvdec and rk hevc decoder, it > > > > > > would > > > > > > requests cabac table, scaling list, picture parameter set and > > > > > > reference > > > > > > picture storing in one or various of DMA buffers. I am not talking > > > > > > about > > > > > > the data been parsed, the decoder would requests a raw data. > > > > > > > > > > > > For the pps and rps, it is possible to reuse the slice header, just > > > > > > let > > > > > > the decoder know the offset from the bitstream bufer, I would > > > > > > suggest to > > > > > > add three properties(with sps) for them. But I think we need a > > > > > > method to > > > > > > mark a OUTPUT side buffer for those aux data. > > > > > > > > > > I'm quite confused about the hardware implementation then. From what > > > > > you're saying, it seems that it takes the raw bitstream elements > > > > > rather > > > > > than parsed elements. Is it really a stateless implementation? > > > > > > > > > > The stateless implementation was designed with the idea that only the > > > > > raw slice data should be passed in bitstream form to the decoder. For > > > > > H.264, it seems that some decoders also need the slice header in raw > > > > > bitstream form (because they take the full slice NAL unit), see the > > > > > discussions in this thread: > > > > > media: docs-rst: Document m2m stateless video decoder interface > > > > > > > > Stateless just mean it won’t track the previous result, but I don’t > > > > think you can define what a date the hardware would need. Even you > > > > just build a dpb for the decoder, it is still stateless, but parsing > > > > less or more data from the bitstream doesn’t stop a decoder become a > > > > stateless decoder. > > > > > > Yes fair enough, the format in which the hardware decoder takes the > > > bitstream parameters does not make it stateless or stateful per-se. > > > It's just that stateless decoders should have no particular reason for > > > parsing the bitstream on their own since the hardware can be designed > > > with registers for each relevant bitstream element to configure the > > > decoding pipeline. That's how GPU-based decoder implementations are > > > implemented (VAAPI/VDPAU/NVDEC, etc). > > > > > > So the format we have agreed on so far for the stateless interface is > > > to pass parsed elements via v4l2 control structures. > > > > > > If the hardware can only work by parsing the bitstream itself, I'm not > > > sure what the best solution would be. Reconstructing the bitstream in > > > the kernel is a pretty bad option, but so is parsing in the kernel or > > > having the data both in parsed and raw forms. Do you see another > > > possibility? > > > > Is reconstructing the bitstream so bad? The v4l2 controls provide a > > generic interface to an encoded format which the driver needs to > > convert into a sequence that the hardware can understand. Typically > > this is done by populating hardware-specific structures. Can't we > > consider that in this specific instance, the hardware-specific > > structure just happens to be identical to the original bitstream > > format? > > At maximum allowed bitrate for let's say HEVC (940MB/s iirc), yes, it > would be really really bad. In GStreamer project we have discussed for > a while (but have never done anything about) adding the ability through > a bitmask to select which part of the stream need to be parsed, as > parsing itself was causing some overhead. Maybe similar thing applies, > though as per our new design, it's the fourcc that dictate the driver > behaviour, we'd need yet another fourcc for drivers that wants the full > bitstream (which seems odd if you have already parsed everything, I > think this need some clarification). Note that I am not proposing to rebuild the *entire* bitstream in-kernel. What I am saying is that if the hardware interprets some structures (like SPS/PPS) in their raw format, this raw format could be reconstructed from the structures passed by userspace at negligible cost. Such manipulation would only happen on a small amount of data. Exposing finer-grained driver requirements through a bitmask may deserve more exploring. Maybe we could end with a spectrum of capabilities that would allow us to cover the range from fully stateless to fully stateful IPs more smoothly. Right now we have two specifications that only consider the extremes of that range. ___ devel mailing list de...@linuxdriverp
Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls
On Wed, Jan 30, 2019 at 11:29 AM Alexandre Courbot wrote: > > On Wed, Jan 30, 2019 at 6:41 AM Nicolas Dufresne wrote: > > > > Le mardi 29 janvier 2019 à 16:44 +0900, Alexandre Courbot a écrit : > > > On Fri, Jan 25, 2019 at 10:04 PM Paul Kocialkowski > > > wrote: > > > > Hi, > > > > > > > > On Thu, 2019-01-24 at 20:23 +0800, Ayaka wrote: > > > > > Sent from my iPad > > > > > > > > > > > On Jan 24, 2019, at 6:27 PM, Paul Kocialkowski > > > > > > wrote: > > > > > > > > > > > > Hi, > > > > > > > > > > > > > On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote: > > > > > > > I forget a important thing, for the rkvdec and rk hevc decoder, > > > > > > > it would > > > > > > > requests cabac table, scaling list, picture parameter set and > > > > > > > reference > > > > > > > picture storing in one or various of DMA buffers. I am not > > > > > > > talking about > > > > > > > the data been parsed, the decoder would requests a raw data. > > > > > > > > > > > > > > For the pps and rps, it is possible to reuse the slice header, > > > > > > > just let > > > > > > > the decoder know the offset from the bitstream bufer, I would > > > > > > > suggest to > > > > > > > add three properties(with sps) for them. But I think we need a > > > > > > > method to > > > > > > > mark a OUTPUT side buffer for those aux data. > > > > > > > > > > > > I'm quite confused about the hardware implementation then. From what > > > > > > you're saying, it seems that it takes the raw bitstream elements > > > > > > rather > > > > > > than parsed elements. Is it really a stateless implementation? > > > > > > > > > > > > The stateless implementation was designed with the idea that only > > > > > > the > > > > > > raw slice data should be passed in bitstream form to the decoder. > > > > > > For > > > > > > H.264, it seems that some decoders also need the slice header in raw > > > > > > bitstream form (because they take the full slice NAL unit), see the > > > > > > discussions in this thread: > > > > > > media: docs-rst: Document m2m stateless video decoder interface > > > > > > > > > > Stateless just mean it won’t track the previous result, but I don’t > > > > > think you can define what a date the hardware would need. Even you > > > > > just build a dpb for the decoder, it is still stateless, but parsing > > > > > less or more data from the bitstream doesn’t stop a decoder become a > > > > > stateless decoder. > > > > > > > > Yes fair enough, the format in which the hardware decoder takes the > > > > bitstream parameters does not make it stateless or stateful per-se. > > > > It's just that stateless decoders should have no particular reason for > > > > parsing the bitstream on their own since the hardware can be designed > > > > with registers for each relevant bitstream element to configure the > > > > decoding pipeline. That's how GPU-based decoder implementations are > > > > implemented (VAAPI/VDPAU/NVDEC, etc). > > > > > > > > So the format we have agreed on so far for the stateless interface is > > > > to pass parsed elements via v4l2 control structures. > > > > > > > > If the hardware can only work by parsing the bitstream itself, I'm not > > > > sure what the best solution would be. Reconstructing the bitstream in > > > > the kernel is a pretty bad option, but so is parsing in the kernel or > > > > having the data both in parsed and raw forms. Do you see another > > > > possibility? > > > > > > Is reconstructing the bitstream so bad? The v4l2 controls provide a > > > generic interface to an encoded format which the driver needs to > > > convert into a sequence that the hardware can understand. Typically > > > this is done by populating hardware-specific structures. Can't we > > > consider that in this specific instance, the hardware-specific > > > structure just happens to be identical to the original bitstream > > > format? > > > > At maximum allowed bitrate for let's say HEVC (940MB/s iirc), yes, it > > would be really really bad. In GStreamer project we have discussed for > > a while (but have never done anything about) adding the ability through > > a bitmask to select which part of the stream need to be parsed, as > > parsing itself was causing some overhead. Maybe similar thing applies, > > though as per our new design, it's the fourcc that dictate the driver > > behaviour, we'd need yet another fourcc for drivers that wants the full > > bitstream (which seems odd if you have already parsed everything, I > > think this need some clarification). > > Note that I am not proposing to rebuild the *entire* bitstream > in-kernel. What I am saying is that if the hardware interprets some > structures (like SPS/PPS) in their raw format, this raw format could > be reconstructed from the structures passed by userspace at negligible > cost. Such manipulation would only happen on a small amount of data. > > Exposing finer-grained driver requirements through a bitmask may > deserve more exploring. Maybe we could end with a spectrum of > capabi
Re: [PATCH 1/7] binder: create userspace-to-binder-buffer copy function
On Tue, Jan 29, 2019 at 04:32:09PM -0800, Todd Kjos wrote:
> On Tue, Jan 29, 2019 at 12:12 AM Dan Carpenter
> wrote:
> >
> > On Mon, Jan 28, 2019 at 04:49:28PM -0800, Todd Kjos wrote:
> > > +/**
> > > + * binder_alloc_copy_user_to_buffer() - copy src user to tgt user
> > > + * @alloc: binder_alloc for this proc
> > > + * @buffer: binder buffer to be accessed
> > > + * @buffer_offset: offset into @buffer data
> > > + * @from: userspace pointer to source buffer
> > > + * @bytes: bytes to copy
> > > + *
> > > + * Copy bytes from source userspace to target buffer.
> > > + *
> > > + * Return: bytes remaining to be copied
> > > + */
> > > +unsigned long
> > > +binder_alloc_copy_user_to_buffer(struct binder_alloc *alloc,
> > > + struct binder_buffer *buffer,
> > > + binder_size_t buffer_offset,
> > > + const void __user *from,
> > > + size_t bytes)
> > > +{
> > > + if (!check_buffer(alloc, buffer, buffer_offset, bytes))
> > > + return bytes;
> > > +
> > > + while (bytes) {
> > > + unsigned long size;
> > > + unsigned long ret;
> > > + struct page *page;
> > > + pgoff_t pgoff;
> > > + void *kptr;
> > > +
> > > + page = binder_alloc_get_page(alloc, buffer,
> > > + buffer_offset, &pgoff);
> > > + size = min(bytes, (size_t)(PAGE_SIZE - pgoff));
> >
> > This code has so much more casting than necessary. To me casting says
> > that we haven't got the types correct or something. I've just pulled
> > this function out as an example really, but none of the casts here are
> > required... This could just be:
> >
> > size = min(bytes, PAGE_SIZE - pgoff);
>
> Dan,
>
> Thanks for the feedback.
>
> The one above: "size = min(bytes, PAGE_SIZE - pgoff);", results in
> 32-bit compile warnings:
>
> ./include/linux/kernel.h:846:29: warning: comparison of distinct
> pointer types lacks a cast
>(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
> [...]
> drivers/android/binder_alloc.c:1157:10: note: in expansion of macro ‘min’
>size = min(bytes, PAGE_SIZE - pgoff);
>
> so, I took your suggestion:
>
> size = min_t(size_t, bytes, PAGE_SIZE - pgoff);
>
> >
> > Btw, if you really need to do a cast inside a min() (you don't in this
> > cast) then use min_t().
> >
> > > + kptr = (void *)((uintptr_t)kmap(page) + pgoff);
> >
> > This would be a lot cleaner as:
> >
> > kptr = kmap(page) + pgoff;
>
> It definitely is cleaner, but the clean version is doing arithmetic on
> a void pointer which is generally not considered good practice (but is
> supported by gcc). Is this acceptable in the kernel? I don't see any
> statement on it in Documentation/process/coding-style.rst.
Yeah, it's a GCCism but to me it seems so nice. We do it all the time.
We'd like to get Clang to compile the kernel, but we've done very little
work to make that happen. Removing variable length arrays was really
done for other reasons...
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] nfit: acpi_nfit_ctl(): check out_obj->type in the right place
On Tue, Jan 29, 2019 at 5:23 PM Dexuan Cui wrote:
>
>
> In the case of ND_CMD_CALL, we should also check out_obj->type.
>
> The patch uses out_obj->type, which is a short alias to
> out_obj->package.type.
>
> Fixes: 31eca76ba2fc ("nfit, libnvdimm: limited/whitelisted dimm command
> marshaling mechanism")
> Cc:
> Signed-off-by: Dexuan Cui
Looks good to me, applied.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] nfit: add Hyper-V NVDIMM DSM command set to white list
On Mon, Jan 28, 2019 at 4:56 PM Dexuan Cui wrote: > > > Add the Hyper-V _DSM command set to the white list of NVDIMM command > sets. > > This command set is documented at http://www.uefi.org/RFIC_LIST > (see "Virtual NVDIMM 0x1901"). > > Thanks Dan Williams for writing the > comment change. > > Signed-off-by: Dexuan Cui > Reviewed-by: Michael Kelley > --- > > Changes in v2: > Updated the comment and changelog (Thanks, Dan!) > Rebased to the tag libnvdimm-fixes-5.0-rc4 of the nvdimm tree. Thanks for the re-spin, applied. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls
Sent from my iPad > On Jan 30, 2019, at 11:35 AM, Tomasz Figa wrote: > > On Wed, Jan 30, 2019 at 11:29 AM Alexandre Courbot > wrote: >> >>> On Wed, Jan 30, 2019 at 6:41 AM Nicolas Dufresne >>> wrote: >>> Le mardi 29 janvier 2019 à 16:44 +0900, Alexandre Courbot a écrit : On Fri, Jan 25, 2019 at 10:04 PM Paul Kocialkowski wrote: > Hi, > >> On Thu, 2019-01-24 at 20:23 +0800, Ayaka wrote: >> Sent from my iPad >> >>> On Jan 24, 2019, at 6:27 PM, Paul Kocialkowski >>> wrote: >>> >>> Hi, >>> On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote: I forget a important thing, for the rkvdec and rk hevc decoder, it would requests cabac table, scaling list, picture parameter set and reference picture storing in one or various of DMA buffers. I am not talking about the data been parsed, the decoder would requests a raw data. For the pps and rps, it is possible to reuse the slice header, just let the decoder know the offset from the bitstream bufer, I would suggest to add three properties(with sps) for them. But I think we need a method to mark a OUTPUT side buffer for those aux data. >>> >>> I'm quite confused about the hardware implementation then. From what >>> you're saying, it seems that it takes the raw bitstream elements rather >>> than parsed elements. Is it really a stateless implementation? >>> >>> The stateless implementation was designed with the idea that only the >>> raw slice data should be passed in bitstream form to the decoder. For >>> H.264, it seems that some decoders also need the slice header in raw >>> bitstream form (because they take the full slice NAL unit), see the >>> discussions in this thread: >>> media: docs-rst: Document m2m stateless video decoder interface >> >> Stateless just mean it won’t track the previous result, but I don’t >> think you can define what a date the hardware would need. Even you >> just build a dpb for the decoder, it is still stateless, but parsing >> less or more data from the bitstream doesn’t stop a decoder become a >> stateless decoder. > > Yes fair enough, the format in which the hardware decoder takes the > bitstream parameters does not make it stateless or stateful per-se. > It's just that stateless decoders should have no particular reason for > parsing the bitstream on their own since the hardware can be designed > with registers for each relevant bitstream element to configure the > decoding pipeline. That's how GPU-based decoder implementations are > implemented (VAAPI/VDPAU/NVDEC, etc). > > So the format we have agreed on so far for the stateless interface is > to pass parsed elements via v4l2 control structures. > > If the hardware can only work by parsing the bitstream itself, I'm not > sure what the best solution would be. Reconstructing the bitstream in > the kernel is a pretty bad option, but so is parsing in the kernel or > having the data both in parsed and raw forms. Do you see another > possibility? Is reconstructing the bitstream so bad? The v4l2 controls provide a generic interface to an encoded format which the driver needs to convert into a sequence that the hardware can understand. Typically this is done by populating hardware-specific structures. Can't we consider that in this specific instance, the hardware-specific structure just happens to be identical to the original bitstream format? >>> >>> At maximum allowed bitrate for let's say HEVC (940MB/s iirc), yes, it >>> would be really really bad. In GStreamer project we have discussed for >>> a while (but have never done anything about) adding the ability through >>> a bitmask to select which part of the stream need to be parsed, as >>> parsing itself was causing some overhead. Maybe similar thing applies, >>> though as per our new design, it's the fourcc that dictate the driver >>> behaviour, we'd need yet another fourcc for drivers that wants the full >>> bitstream (which seems odd if you have already parsed everything, I >>> think this need some clarification). >> >> Note that I am not proposing to rebuild the *entire* bitstream >> in-kernel. What I am saying is that if the hardware interprets some >> structures (like SPS/PPS) in their raw format, this raw format could >> be reconstructed from the structures passed by userspace at negligible >> cost. Such manipulation would only happen on a small amount of data. >> >> Exposing finer-grained driver requirements through a bitmask may >> deserve more exploring. Maybe we could end with a spectrum of >> capabilities that would allow us to cover the range from fully >> stateless to fully stateful IPs more smoothly. Right now we have two >> specifications that only con
Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls
Sent from my iPad > On Jan 30, 2019, at 5:41 AM, Nicolas Dufresne wrote: > >> Le mardi 29 janvier 2019 à 16:44 +0900, Alexandre Courbot a écrit : >> On Fri, Jan 25, 2019 at 10:04 PM Paul Kocialkowski >> wrote: >>> Hi, >>> On Thu, 2019-01-24 at 20:23 +0800, Ayaka wrote: Sent from my iPad > On Jan 24, 2019, at 6:27 PM, Paul Kocialkowski > wrote: > > Hi, > >> On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote: >> I forget a important thing, for the rkvdec and rk hevc decoder, it would >> requests cabac table, scaling list, picture parameter set and reference >> picture storing in one or various of DMA buffers. I am not talking about >> the data been parsed, the decoder would requests a raw data. >> >> For the pps and rps, it is possible to reuse the slice header, just let >> the decoder know the offset from the bitstream bufer, I would suggest to >> add three properties(with sps) for them. But I think we need a method to >> mark a OUTPUT side buffer for those aux data. > > I'm quite confused about the hardware implementation then. From what > you're saying, it seems that it takes the raw bitstream elements rather > than parsed elements. Is it really a stateless implementation? > > The stateless implementation was designed with the idea that only the > raw slice data should be passed in bitstream form to the decoder. For > H.264, it seems that some decoders also need the slice header in raw > bitstream form (because they take the full slice NAL unit), see the > discussions in this thread: > media: docs-rst: Document m2m stateless video decoder interface Stateless just mean it won’t track the previous result, but I don’t think you can define what a date the hardware would need. Even you just build a dpb for the decoder, it is still stateless, but parsing less or more data from the bitstream doesn’t stop a decoder become a stateless decoder. >>> >>> Yes fair enough, the format in which the hardware decoder takes the >>> bitstream parameters does not make it stateless or stateful per-se. >>> It's just that stateless decoders should have no particular reason for >>> parsing the bitstream on their own since the hardware can be designed >>> with registers for each relevant bitstream element to configure the >>> decoding pipeline. That's how GPU-based decoder implementations are >>> implemented (VAAPI/VDPAU/NVDEC, etc). >>> >>> So the format we have agreed on so far for the stateless interface is >>> to pass parsed elements via v4l2 control structures. >>> >>> If the hardware can only work by parsing the bitstream itself, I'm not >>> sure what the best solution would be. Reconstructing the bitstream in >>> the kernel is a pretty bad option, but so is parsing in the kernel or >>> having the data both in parsed and raw forms. Do you see another >>> possibility? >> >> Is reconstructing the bitstream so bad? The v4l2 controls provide a >> generic interface to an encoded format which the driver needs to >> convert into a sequence that the hardware can understand. Typically >> this is done by populating hardware-specific structures. Can't we >> consider that in this specific instance, the hardware-specific >> structure just happens to be identical to the original bitstream >> format? > > At maximum allowed bitrate for let's say HEVC (940MB/s iirc), yes, it Lucky, most of hardware won’t be able to processing such a big buffer. General speaking, the register is 24bits for stream length in bytes. > would be really really bad. In GStreamer project we have discussed for > a while (but have never done anything about) adding the ability through > a bitmask to select which part of the stream need to be parsed, as > parsing itself was causing some overhead. Maybe similar thing applies, > though as per our new design, it's the fourcc that dictate the driver > behaviour, we'd need yet another fourcc for drivers that wants the full > bitstream (which seems odd if you have already parsed everything, I > think this need some clarification). > >> >> I agree that this is not strictly optimal for that particular >> hardware, but such is the cost of abstractions, and in this specific >> case I don't believe the cost would be particularly high? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls
On Wed, Jan 30, 2019 at 3:28 PM Ayaka wrote: > > > > Sent from my iPad > > > On Jan 30, 2019, at 11:35 AM, Tomasz Figa wrote: > > > > On Wed, Jan 30, 2019 at 11:29 AM Alexandre Courbot > > wrote: > >> > >>> On Wed, Jan 30, 2019 at 6:41 AM Nicolas Dufresne > >>> wrote: > >>> > Le mardi 29 janvier 2019 à 16:44 +0900, Alexandre Courbot a écrit : > On Fri, Jan 25, 2019 at 10:04 PM Paul Kocialkowski > wrote: > > Hi, > > > >> On Thu, 2019-01-24 at 20:23 +0800, Ayaka wrote: > >> Sent from my iPad > >> > >>> On Jan 24, 2019, at 6:27 PM, Paul Kocialkowski > >>> wrote: > >>> > >>> Hi, > >>> > On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote: > I forget a important thing, for the rkvdec and rk hevc decoder, it > would > requests cabac table, scaling list, picture parameter set and > reference > picture storing in one or various of DMA buffers. I am not talking > about > the data been parsed, the decoder would requests a raw data. > > For the pps and rps, it is possible to reuse the slice header, just > let > the decoder know the offset from the bitstream bufer, I would > suggest to > add three properties(with sps) for them. But I think we need a > method to > mark a OUTPUT side buffer for those aux data. > >>> > >>> I'm quite confused about the hardware implementation then. From what > >>> you're saying, it seems that it takes the raw bitstream elements > >>> rather > >>> than parsed elements. Is it really a stateless implementation? > >>> > >>> The stateless implementation was designed with the idea that only the > >>> raw slice data should be passed in bitstream form to the decoder. For > >>> H.264, it seems that some decoders also need the slice header in raw > >>> bitstream form (because they take the full slice NAL unit), see the > >>> discussions in this thread: > >>> media: docs-rst: Document m2m stateless video decoder interface > >> > >> Stateless just mean it won’t track the previous result, but I don’t > >> think you can define what a date the hardware would need. Even you > >> just build a dpb for the decoder, it is still stateless, but parsing > >> less or more data from the bitstream doesn’t stop a decoder become a > >> stateless decoder. > > > > Yes fair enough, the format in which the hardware decoder takes the > > bitstream parameters does not make it stateless or stateful per-se. > > It's just that stateless decoders should have no particular reason for > > parsing the bitstream on their own since the hardware can be designed > > with registers for each relevant bitstream element to configure the > > decoding pipeline. That's how GPU-based decoder implementations are > > implemented (VAAPI/VDPAU/NVDEC, etc). > > > > So the format we have agreed on so far for the stateless interface is > > to pass parsed elements via v4l2 control structures. > > > > If the hardware can only work by parsing the bitstream itself, I'm not > > sure what the best solution would be. Reconstructing the bitstream in > > the kernel is a pretty bad option, but so is parsing in the kernel or > > having the data both in parsed and raw forms. Do you see another > > possibility? > > Is reconstructing the bitstream so bad? The v4l2 controls provide a > generic interface to an encoded format which the driver needs to > convert into a sequence that the hardware can understand. Typically > this is done by populating hardware-specific structures. Can't we > consider that in this specific instance, the hardware-specific > structure just happens to be identical to the original bitstream > format? > >>> > >>> At maximum allowed bitrate for let's say HEVC (940MB/s iirc), yes, it > >>> would be really really bad. In GStreamer project we have discussed for > >>> a while (but have never done anything about) adding the ability through > >>> a bitmask to select which part of the stream need to be parsed, as > >>> parsing itself was causing some overhead. Maybe similar thing applies, > >>> though as per our new design, it's the fourcc that dictate the driver > >>> behaviour, we'd need yet another fourcc for drivers that wants the full > >>> bitstream (which seems odd if you have already parsed everything, I > >>> think this need some clarification). > >> > >> Note that I am not proposing to rebuild the *entire* bitstream > >> in-kernel. What I am saying is that if the hardware interprets some > >> structures (like SPS/PPS) in their raw format, this raw format could > >> be reconstructed from the structures passed by userspace at negligible > >> cost. Such manipulation would only happen on a small amount of data. > >> > >> Exposing finer-grained driver requireme
Re: [linux-sunxi] [PATCH v2 1/2] media: v4l: Add definitions for the HEVC slice format and controls
On Wed, Jan 30, 2019 at 12:35:41PM +0900, Tomasz Figa wrote: > On Wed, Jan 30, 2019 at 11:29 AM Alexandre Courbot > wrote: > > > > On Wed, Jan 30, 2019 at 6:41 AM Nicolas Dufresne > > wrote: > > > > > > Le mardi 29 janvier 2019 à 16:44 +0900, Alexandre Courbot a écrit : > > > > On Fri, Jan 25, 2019 at 10:04 PM Paul Kocialkowski > > > > wrote: > > > > > Hi, > > > > > > > > > > On Thu, 2019-01-24 at 20:23 +0800, Ayaka wrote: > > > > > > Sent from my iPad > > > > > > > > > > > > > On Jan 24, 2019, at 6:27 PM, Paul Kocialkowski > > > > > > > wrote: > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > On Thu, 2019-01-10 at 21:32 +0800, ayaka wrote: > > > > > > > > I forget a important thing, for the rkvdec and rk hevc decoder, > > > > > > > > it would > > > > > > > > requests cabac table, scaling list, picture parameter set and > > > > > > > > reference > > > > > > > > picture storing in one or various of DMA buffers. I am not > > > > > > > > talking about > > > > > > > > the data been parsed, the decoder would requests a raw data. > > > > > > > > > > > > > > > > For the pps and rps, it is possible to reuse the slice header, > > > > > > > > just let > > > > > > > > the decoder know the offset from the bitstream bufer, I would > > > > > > > > suggest to > > > > > > > > add three properties(with sps) for them. But I think we need a > > > > > > > > method to > > > > > > > > mark a OUTPUT side buffer for those aux data. > > > > > > > > > > > > > > I'm quite confused about the hardware implementation then. From > > > > > > > what > > > > > > > you're saying, it seems that it takes the raw bitstream elements > > > > > > > rather > > > > > > > than parsed elements. Is it really a stateless implementation? > > > > > > > > > > > > > > The stateless implementation was designed with the idea that only > > > > > > > the > > > > > > > raw slice data should be passed in bitstream form to the decoder. > > > > > > > For > > > > > > > H.264, it seems that some decoders also need the slice header in > > > > > > > raw > > > > > > > bitstream form (because they take the full slice NAL unit), see > > > > > > > the > > > > > > > discussions in this thread: > > > > > > > media: docs-rst: Document m2m stateless video decoder interface > > > > > > > > > > > > Stateless just mean it won’t track the previous result, but I don’t > > > > > > think you can define what a date the hardware would need. Even you > > > > > > just build a dpb for the decoder, it is still stateless, but parsing > > > > > > less or more data from the bitstream doesn’t stop a decoder become a > > > > > > stateless decoder. > > > > > > > > > > Yes fair enough, the format in which the hardware decoder takes the > > > > > bitstream parameters does not make it stateless or stateful per-se. > > > > > It's just that stateless decoders should have no particular reason for > > > > > parsing the bitstream on their own since the hardware can be designed > > > > > with registers for each relevant bitstream element to configure the > > > > > decoding pipeline. That's how GPU-based decoder implementations are > > > > > implemented (VAAPI/VDPAU/NVDEC, etc). > > > > > > > > > > So the format we have agreed on so far for the stateless interface is > > > > > to pass parsed elements via v4l2 control structures. > > > > > > > > > > If the hardware can only work by parsing the bitstream itself, I'm not > > > > > sure what the best solution would be. Reconstructing the bitstream in > > > > > the kernel is a pretty bad option, but so is parsing in the kernel or > > > > > having the data both in parsed and raw forms. Do you see another > > > > > possibility? > > > > > > > > Is reconstructing the bitstream so bad? The v4l2 controls provide a > > > > generic interface to an encoded format which the driver needs to > > > > convert into a sequence that the hardware can understand. Typically > > > > this is done by populating hardware-specific structures. Can't we > > > > consider that in this specific instance, the hardware-specific > > > > structure just happens to be identical to the original bitstream > > > > format? > > > > > > At maximum allowed bitrate for let's say HEVC (940MB/s iirc), yes, it > > > would be really really bad. In GStreamer project we have discussed for > > > a while (but have never done anything about) adding the ability through > > > a bitmask to select which part of the stream need to be parsed, as > > > parsing itself was causing some overhead. Maybe similar thing applies, > > > though as per our new design, it's the fourcc that dictate the driver > > > behaviour, we'd need yet another fourcc for drivers that wants the full > > > bitstream (which seems odd if you have already parsed everything, I > > > think this need some clarification). > > > > Note that I am not proposing to rebuild the *entire* bitstream > > in-kernel. What I am saying is that if the hardware interprets some > > structures (like SPS/PPS) in their raw format, this raw f
