Re: [PATCH v1 0/2] NVIDIA Tegra20 video decoder driver

[Dmitry Osipenko]

On 27.09.2017 00:35, Greg Kroah-Hartman wrote:
> On Tue, Sep 26, 2017 at 03:32:23PM +0300, Dmitry Osipenko wrote:
>> On 26.09.2017 09:54, Greg Kroah-Hartman wrote:
>>> On Tue, Sep 26, 2017 at 01:15:41AM +0300, Dmitry Osipenko wrote:
 This driver provides accelerated video decoding to NVIDIA Tegra20 SoC's,
 it is a result of reverse-engineering efforts. Driver has been tested on
 Toshiba AC100 and Acer A500, it should work on any Tegra20 device.

 In userspace this driver is utilized by libvdpau-tegra [0] that implements
 VDPAU interface, so any video player that supports VDPAU can provide
 accelerated video decoding on Tegra20 on Linux.
>>>
>>> Why not use the v4l2 api instead?  Doesn't that provide the same needed
>>> user/kernel api here instead of creating yet-another-custom ioctl?
>>>
>>
>> 1) The HW doesn't generalize for the common API. Like for example, it isn't
>> capable of unpacking bitstream encoded with CABAC (Context-adaptive binary
>> arithmetic coding), so unpacking should be done in software and then VDE HW
>> isn't capable of decoding such a stream in a fully-automated manner, software
>> would have to feed engine with a chunks of macroblocks untill the whole 
>> frame is
>> decoded. That lameness is partially hidden in the BLOB's firmware, that 
>> firmware
>> actually is just a driver BTW.
>>
>> 2) We want to have decoding integrated with the presentation of the decoded
>> video frame. So having v4l interface for decoding would be just an extra
>> unnecessary shim, increasing CPU / memory resources usage and complexity of 
>> the
>> code.
>>
>> 3) The decoding and presentation are already implemented using VDPAU API and
>> proven to work decently in that way.
> 
> This sounds like something you should be talking over with the media
> driver developers, why are they not even cc:ed on this submission?
> 
> I need their ack on this new api before I can take this.
> 

Indeed, will cc media on V2.

-- 
Dmitry
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 0/2] NVIDIA Tegra20 video decoder driver

[Greg Kroah-Hartman]

On Tue, Sep 26, 2017 at 03:32:23PM +0300, Dmitry Osipenko wrote:
> On 26.09.2017 09:54, Greg Kroah-Hartman wrote:
> > On Tue, Sep 26, 2017 at 01:15:41AM +0300, Dmitry Osipenko wrote:
> >> This driver provides accelerated video decoding to NVIDIA Tegra20 SoC's,
> >> it is a result of reverse-engineering efforts. Driver has been tested on
> >> Toshiba AC100 and Acer A500, it should work on any Tegra20 device.
> >>
> >> In userspace this driver is utilized by libvdpau-tegra [0] that implements
> >> VDPAU interface, so any video player that supports VDPAU can provide
> >> accelerated video decoding on Tegra20 on Linux.
> > 
> > Why not use the v4l2 api instead?  Doesn't that provide the same needed
> > user/kernel api here instead of creating yet-another-custom ioctl?
> > 
> 
> 1) The HW doesn't generalize for the common API. Like for example, it isn't
> capable of unpacking bitstream encoded with CABAC (Context-adaptive binary
> arithmetic coding), so unpacking should be done in software and then VDE HW
> isn't capable of decoding such a stream in a fully-automated manner, software
> would have to feed engine with a chunks of macroblocks untill the whole frame 
> is
> decoded. That lameness is partially hidden in the BLOB's firmware, that 
> firmware
> actually is just a driver BTW.
> 
> 2) We want to have decoding integrated with the presentation of the decoded
> video frame. So having v4l interface for decoding would be just an extra
> unnecessary shim, increasing CPU / memory resources usage and complexity of 
> the
> code.
> 
> 3) The decoding and presentation are already implemented using VDPAU API and
> proven to work decently in that way.

This sounds like something you should be talking over with the media
driver developers, why are they not even cc:ed on this submission?

I need their ack on this new api before I can take this.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 02/22] drm/i915: introduce simple gemfs

[Greg Kroah-Hartman]

On Tue, Sep 26, 2017 at 04:21:47PM +0300, Joonas Lahtinen wrote:
> On Tue, 2017-09-26 at 09:52 +0200, Greg Kroah-Hartman wrote:
> > On Mon, Sep 25, 2017 at 07:47:17PM +0100, Matthew Auld wrote:
> > > Not a fully blown gemfs, just our very own tmpfs kernel mount. Doing so
> > > moves us away from the shmemfs shm_mnt, and gives us the much needed
> > > flexibility to do things like set our own mount options, namely huge=
> > > which should allow us to enable the use of transparent-huge-pages for
> > > our shmem backed objects.
> > > 
> > > v2: various improvements suggested by Joonas
> > > 
> > > v3: move gemfs instance to i915.mm and simplify now that we have
> > > file_setup_with_mnt
> > > 
> > > v4: fallback to tmpfs shm_mnt upon failure to setup gemfs
> > > 
> > > v5: make tmpfs fallback kinder
> > 
> > Why do this only for one specific driver?  Shouldn't the drm core handle
> > this for you, for all other drivers as well?  Otherwise trying to figure
> > out how to "contain" this type of thing is going to be a pain (mount
> > options, selinux options, etc.)
> 
> We actually started quite grande by making stripped down version of
> shmemfs for drm core, but kept running into nacks about how we were
> implementing it (after getting a recommendation to try implementing it
> some way). After a few iterations and massive engineering time, we have
> been progressively reducing the amount of changes outside i915 in the
> hopes to get this merged.
> 
> And all the while clock is ticking, so we thought the best way to get
> something to support our future work is to implement this first locally
> with minimal external changes outside i915 and then once we have
> something working, it'll be easier to generalize it for the drm core.
> Otherwise we'll never get to work with the huge page support, for which
> gemfs is the stepping stone here.
> 
> So we're not planning on sitting on top of it, we'll just incubate it
> under i915/ so that it'll then be less pain for others to adopt when
> the biggest hurdles with core MM interactions are sorted out.

But by doing this, you are now creating a new user/kernel api that you
have to support for forever, right?  Will it not change if you make it
"generic" to the drm core eventually?

Worse case, name it a generic name that everyone will end up using in
the future, and then you can just claim that all other drivers need to
implement it :)

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: iio: trigger: Move header file content to source file

[Harsha Sharma]

The contents of the header file are used only by this single source file.
Moved content into iio-trig-bfin-timer.c and removed iio-trig-bfin-timer.h

Signed-off-by: Harsha Sharma 
---
 drivers/staging/iio/trigger/iio-trig-bfin-timer.c | 20 ++-
 drivers/staging/iio/trigger/iio-trig-bfin-timer.h | 24 ---
 2 files changed, 19 insertions(+), 25 deletions(-)
 delete mode 100644 drivers/staging/iio/trigger/iio-trig-bfin-timer.h

diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c 
b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
index d80dcf8..2cedcaf 100644
--- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
+++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
@@ -19,7 +19,25 @@
 #include 
 #include 
 
-#include "iio-trig-bfin-timer.h"
+/**
+ * struct iio_bfin_timer_trigger_pdata - timer trigger platform data
+ * @output_enable: Enable external trigger pulse generation.
+ * @active_low: Whether the trigger pulse is active low.
+ * @duty_ns: Length of the trigger pulse in nanoseconds.
+ *
+ * This struct is used to configure the output pulse generation of the blackfin
+ * timer trigger. If output_enable is set to true an external trigger signal
+ * will generated on the pin corresponding to the timer. This is useful for
+ * converters which needs an external signal to start conversion. active_low 
and
+ * duty_ns are used to configure the type of the trigger pulse. If 
output_enable
+ * is set to false no external trigger pulse will be generated and active_low
+ * and duty_ns are ignored.
+ **/
+struct iio_bfin_timer_trigger_pdata {
+bool output_enable;
+bool active_low;
+unsigned int duty_ns;
+};
 
 struct bfin_timer {
unsigned short id, bit;
diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.h 
b/drivers/staging/iio/trigger/iio-trig-bfin-timer.h
deleted file mode 100644
index c07321f..000
--- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.h
+++ /dev/null
@@ -1,24 +0,0 @@
-#ifndef __IIO_BFIN_TIMER_TRIGGER_H__
-#define __IIO_BFIN_TIMER_TRIGGER_H__
-
-/**
- * struct iio_bfin_timer_trigger_pdata - timer trigger platform data
- * @output_enable: Enable external trigger pulse generation.
- * @active_low: Whether the trigger pulse is active low.
- * @duty_ns: Length of the trigger pulse in nanoseconds.
- *
- * This struct is used to configure the output pulse generation of the blackfin
- * timer trigger. If output_enable is set to true an external trigger signal
- * will generated on the pin corresponding to the timer. This is useful for
- * converters which needs an external signal to start conversion. active_low 
and
- * duty_ns are used to configure the type of the trigger pulse. If 
output_enable
- * is set to false no external trigger pulse will be generated and active_low
- * and duty_ns are ignored.
- **/
-struct iio_bfin_timer_trigger_pdata {
-   bool output_enable;
-   bool active_low;
-   unsigned int duty_ns;
-};
-
-#endif
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: wlan-ng: convert variable to le16 for assignment

[Alfonso Lima Astor]

Sparse was complaining about an incorrect assignment:
drivers/staging/wlan-ng/prism2sta.c:1450:29: warning: incorrect type in 
assignment (different base types)
drivers/staging/wlan-ng/prism2sta.c:1450:29:expected unsigned short 
[unsigned] [usertype] link_status_new
drivers/staging/wlan-ng/prism2sta.c:1450:29:got restricted __le16 
[usertype] linkstatus

Signed-off-by: Alfonso Lima Astor 
---
 drivers/staging/wlan-ng/prism2sta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wlan-ng/prism2sta.c 
b/drivers/staging/wlan-ng/prism2sta.c
index c9df450..070a237 100644
--- a/drivers/staging/wlan-ng/prism2sta.c
+++ b/drivers/staging/wlan-ng/prism2sta.c
@@ -1447,7 +1447,7 @@ static void prism2sta_inf_linkstatus(struct wlandevice 
*wlandev,
 {
struct hfa384x *hw = wlandev->priv;
 
-   hw->link_status_new = inf->info.linkstatus.linkstatus;
+   hw->link_status_new = le16_to_cpu(inf->info.linkstatus.linkstatus);
 
schedule_work(>link_bh);
 }
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 2/2] staging: ion: create one device entry per heap

[Laura Abbott]

On 09/26/2017 09:17 AM, Mark Brown wrote:

On Tue, Sep 26, 2017 at 02:07:05PM +0200, Benjamin Gaignard wrote:


version 4:
- add a configuration flag to switch between legacy Ion misc device
   and one device per heap version.


Should this be a switch or should it just be enabling and disabling the
legacy device with the per heap ones always availalbe?  I can't see that
the new devices would do any harm or have trouble interacting with the
per heap ones.  Being able to have both enabled would make things easier
for userspaces that are moving to the device per heap interface.



Agreed. We should be enabling the new interface unconditionally. The
old /dev/ion interface should coexist to allow for backwards
compatibility but keep it under a Kconfig to allow it to be turned off
for security or other reasons.

Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 2/2] staging: ion: create one device entry per heap

[Laura Abbott]

On 09/25/2017 11:56 PM, Greg KH wrote:

On Tue, Sep 26, 2017 at 07:09:14AM +0200, Daniel Vetter wrote:

On Mon, Sep 25, 2017 at 11:26:27AM -0700, Laura Abbott wrote:

On 09/20/2017 01:45 AM, Benjamin Gaignard wrote:

Instead a getting one common device "/dev/ion" for
all the heaps this patch allow to create one device
entry ("/dev/ionX") per heap.
Getting an entry per heap could allow to set security rules
per heap and global ones for all heaps.

Allocation requests will be only allowed if the mask_id
match with device minor.
Query request could be done on any of the devices.
Deivce node major will also change and that may impact init scripts.



We should start Cc linux-api for future changes since we're going
to have more than just /dev/ion.

Thinking about this some more, I think we need to allow backwards
compatibility. It's just not feasible to keep throwing workarounds
into userspace if we can avoid it. I'd propose keeping the old /dev/ion
misc interface available under a Kconfig and then creating the new
split heaps in parallel.

On a somewhat related note, there was some interest in possibly
having a sysfs interface for Ion long term. I don't think this
needs to happen right now but I'd like whatever we do to not
make adding that harder.


Not sure sysfs is a good idea for allocating buffers. The backlight
interface is in sysfs, which defacto makes it a root-only interface.
Distros really hate that, because it makes unpriviledged X/wayland really
hard to pull of. Passing a device file otoh from logind to the compositor
is dead simple (and how X et al get at e.g. the drm/input devices
already).


sysfs is not a good idea for allocating buffers.  We already have some
out-of-tree drm drivers doing horrid things in sysfs in ways that
totally abuse that api, and vendors have to do crazy things with selinux
rules to try to lock it down because of that.  A device node is fine, we
are used to that for graphics stuff :)

thanks,

greg k-h



I wasn't thinking of sysfs for allocation, this was for exposure of
other Ion information that might make more sense than debugfs. Like
I said, this was mostly forward thinking to make sure we aren't
stuck later.

Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 2/2] staging: ion: create one device entry per heap

[Mark Brown]

On Tue, Sep 26, 2017 at 02:07:05PM +0200, Benjamin Gaignard wrote:

> version 4:
> - add a configuration flag to switch between legacy Ion misc device
>   and one device per heap version.

Should this be a switch or should it just be enabling and disabling the
legacy device with the per heap ones always availalbe?  I can't see that
the new devices would do any harm or have trouble interacting with the 
per heap ones.  Being able to have both enabled would make things easier
for userspaces that are moving to the device per heap interface.


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 02/22] drm/i915: introduce simple gemfs

[Joonas Lahtinen]

On Tue, 2017-09-26 at 09:52 +0200, Greg Kroah-Hartman wrote:
> On Mon, Sep 25, 2017 at 07:47:17PM +0100, Matthew Auld wrote:
> > Not a fully blown gemfs, just our very own tmpfs kernel mount. Doing so
> > moves us away from the shmemfs shm_mnt, and gives us the much needed
> > flexibility to do things like set our own mount options, namely huge=
> > which should allow us to enable the use of transparent-huge-pages for
> > our shmem backed objects.
> > 
> > v2: various improvements suggested by Joonas
> > 
> > v3: move gemfs instance to i915.mm and simplify now that we have
> > file_setup_with_mnt
> > 
> > v4: fallback to tmpfs shm_mnt upon failure to setup gemfs
> > 
> > v5: make tmpfs fallback kinder
> 
> Why do this only for one specific driver?  Shouldn't the drm core handle
> this for you, for all other drivers as well?  Otherwise trying to figure
> out how to "contain" this type of thing is going to be a pain (mount
> options, selinux options, etc.)

We actually started quite grande by making stripped down version of
shmemfs for drm core, but kept running into nacks about how we were
implementing it (after getting a recommendation to try implementing it
some way). After a few iterations and massive engineering time, we have
been progressively reducing the amount of changes outside i915 in the
hopes to get this merged.

And all the while clock is ticking, so we thought the best way to get
something to support our future work is to implement this first locally
with minimal external changes outside i915 and then once we have
something working, it'll be easier to generalize it for the drm core.
Otherwise we'll never get to work with the huge page support, for which
gemfs is the stepping stone here.

So we're not planning on sitting on top of it, we'll just incubate it
under i915/ so that it'll then be less pain for others to adopt when
the biggest hurdles with core MM interactions are sorted out.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 0/2] NVIDIA Tegra20 video decoder driver

[Dmitry Osipenko]

On 26.09.2017 09:54, Greg Kroah-Hartman wrote:
> On Tue, Sep 26, 2017 at 01:15:41AM +0300, Dmitry Osipenko wrote:
>> This driver provides accelerated video decoding to NVIDIA Tegra20 SoC's,
>> it is a result of reverse-engineering efforts. Driver has been tested on
>> Toshiba AC100 and Acer A500, it should work on any Tegra20 device.
>>
>> In userspace this driver is utilized by libvdpau-tegra [0] that implements
>> VDPAU interface, so any video player that supports VDPAU can provide
>> accelerated video decoding on Tegra20 on Linux.
> 
> Why not use the v4l2 api instead?  Doesn't that provide the same needed
> user/kernel api here instead of creating yet-another-custom ioctl?
> 

1) The HW doesn't generalize for the common API. Like for example, it isn't
capable of unpacking bitstream encoded with CABAC (Context-adaptive binary
arithmetic coding), so unpacking should be done in software and then VDE HW
isn't capable of decoding such a stream in a fully-automated manner, software
would have to feed engine with a chunks of macroblocks untill the whole frame is
decoded. That lameness is partially hidden in the BLOB's firmware, that firmware
actually is just a driver BTW.

2) We want to have decoding integrated with the presentation of the decoded
video frame. So having v4l interface for decoding would be just an extra
unnecessary shim, increasing CPU / memory resources usage and complexity of the
code.

3) The decoding and presentation are already implemented using VDPAU API and
proven to work decently in that way.

-- 
Dmitry
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 2/2] staging: ion: create one device entry per heap

[Benjamin Gaignard]

Instead a getting one common device "/dev/ion" for
all the heaps this patch allow to create one device
entry ("/dev/ionX") per heap.
Getting an entry per heap could allow to set security rules
per heap and global ones for all heaps.

Allocation requests will be only allowed if the mask_id
match with device minor.
Query request could be done on any of the devices.

Signed-off-by: Benjamin Gaignard 
---
version 4:
- add a configuration flag to switch between legacy Ion misc device
  and one device per heap version.

version 3:
- change ion_device_add_heap prototype to return a possible error.

version 2:
- simplify ioctl check like propose by Dan
- make sure that we don't register more than ION_DEV_MAX heaps.

 drivers/staging/android/TODO|  1 -
 drivers/staging/android/ion/Kconfig |  8 
 drivers/staging/android/ion/ion-ioctl.c | 16 ++--
 drivers/staging/android/ion/ion.c   | 29 +++--
 drivers/staging/android/ion/ion.h   | 18 +-
 5 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index 5f14247..d770ffa 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -9,7 +9,6 @@ TODO:
 ion/
  - Add dt-bindings for remaining heaps (chunk and carveout heaps). This would
involve putting appropriate bindings in a memory node for Ion to find.
- - Split /dev/ion up into multiple nodes (e.g. /dev/ion/heap0)
  - Better test framework (integration with VGEM was suggested)
 
 Please send patches to Greg Kroah-Hartman  and Cc:
diff --git a/drivers/staging/android/ion/Kconfig 
b/drivers/staging/android/ion/Kconfig
index a517b2d..6bb07f6 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -10,6 +10,14 @@ menuconfig ION
  If you're not using Android its probably safe to
  say N here.
 
+config ION_ONE_DEVICE_ENTRY_PER_HEAP
+   bool "Create one Ion device per heap"
+   depends on ION
+   help
+ Choose this option to have one device entry per heap. Each
+ device with named "/dev/ionX" where X is heap ID.
+ Selecting this option remove the legacy Ion misc device.
+
 config ION_SYSTEM_HEAP
bool "Ion system heap"
depends on ION
diff --git a/drivers/staging/android/ion/ion-ioctl.c 
b/drivers/staging/android/ion/ion-ioctl.c
index e26b786..76492cc 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -25,7 +25,8 @@ union ion_ioctl_arg {
struct ion_heap_query query;
 };
 
-static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
+static int validate_ioctl_arg(struct file *filp,
+ unsigned int cmd, union ion_ioctl_arg *arg)
 {
switch (cmd) {
case ION_IOC_HEAP_QUERY:
@@ -34,6 +35,17 @@ static int validate_ioctl_arg(unsigned int cmd, union 
ion_ioctl_arg *arg)
arg->query.reserved2 )
return -EINVAL;
break;
+
+#ifdef CONFIG_ION_ONE_DEVICE_ENTRY_PER_HEAP
+   case ION_IOC_ALLOC:
+   {
+   int mask = 1 << iminor(filp->f_inode);
+
+   if (!(arg->allocation.heap_id_mask & mask))
+   return -EINVAL;
+   break;
+   }
+#endif
default:
break;
}
@@ -69,7 +81,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned 
long arg)
if (copy_from_user(, (void __user *)arg, _IOC_SIZE(cmd)))
return -EFAULT;
 
-   ret = validate_ioctl_arg(cmd, );
+   ret = validate_ioctl_arg(filp, cmd, );
if (WARN_ON_ONCE(ret))
return ret;
 
diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 93e2c90..18a20d2 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -40,6 +40,10 @@
 
 #include "ion.h"
 
+#ifdef CONFIG_ION_ONE_DEVICE_ENTRY_PER_HEAP
+#define ION_DEV_MAX 32
+#endif
+
 static struct ion_device *internal_dev;
 static int heap_id;
 
@@ -537,15 +541,30 @@ static int debug_shrink_get(void *data, u64 *val)
 DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get,
debug_shrink_set, "%llu\n");
 
-void ion_device_add_heap(struct ion_heap *heap)
+int ion_device_add_heap(struct ion_heap *heap)
 {
struct dentry *debug_file;
struct ion_device *dev = internal_dev;
+   int ret = 0;
 
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
   __func__);
 
+#ifdef CONFIG_ION_ONE_DEVICE_ENTRY_PER_HEAP
+   if (heap_id >= ION_DEV_MAX)
+   return -EBUSY;
+
+   heap->ddev.devt = MKDEV(MAJOR(dev->devt), heap_id);
+   dev_set_name(>ddev, "ion%d", heap_id);
+   device_initialize(>ddev);
+ 

[PATCH v4 0/2] staging: ion: get one device per heap

[Benjamin Gaignard]

version 4:
- add a configuration flag to switch between legacy Ion misc device
  and one device per heap version.
  This change has been suggested after Laura talks at XDC 2017.

version 3:
- change ion_device_add_heap prototype to return a possible error.

version 2:
- simplify ioctl check like propose by Dan
- make sure that we don't register more than ION_DEV_MAX heaps.

Until now all ion heaps are addressing using the same device "/dev/ion".
This way of working doesn't allow to give access rights (for example with
SElinux rules) per heap.
This series propose to have (under configuartion flag) one device "/dev/ionX"
per heap. Query heaps informations will be possible on each device node but
allocation request will only be possible if heap_mask_id match with device 
minor number.

That enable  thsi new flag change Ion ABI because:
- device name change
- allocation must be done on the correct device/heap.
- device major number will not be the same and that could impact init scripts.

Benjamin Gaignard (2):
  staging: ion: simplify ioctl args checking function
  staging: ion: create one device entry per heap

 drivers/staging/android/TODO|  1 -
 drivers/staging/android/ion/Kconfig |  8 
 drivers/staging/android/ion/ion-ioctl.c | 27 +++
 drivers/staging/android/ion/ion.c   | 29 +++--
 drivers/staging/android/ion/ion.h   | 18 +-
 5 files changed, 71 insertions(+), 12 deletions(-)

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 1/2] staging: ion: simplify ioctl args checking function

[Benjamin Gaignard]

Make arguments checking more easy to read.

Signed-off-by: Benjamin Gaignard 
---
 drivers/staging/android/ion/ion-ioctl.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/android/ion/ion-ioctl.c 
b/drivers/staging/android/ion/ion-ioctl.c
index d9f8b14..e26b786 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -27,19 +27,18 @@ union ion_ioctl_arg {
 
 static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
 {
-   int ret = 0;
-
switch (cmd) {
case ION_IOC_HEAP_QUERY:
-   ret = arg->query.reserved0 != 0;
-   ret |= arg->query.reserved1 != 0;
-   ret |= arg->query.reserved2 != 0;
+   if (arg->query.reserved0 ||
+   arg->query.reserved1 ||
+   arg->query.reserved2 )
+   return -EINVAL;
break;
default:
break;
}
 
-   return ret ? -EINVAL : 0;
+   return 0;
 }
 
 /* fix up the cases where the ioctl direction bits are incorrect */
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver

[Dmitry Osipenko]

On 26.09.2017 08:11, Stephen Warren wrote:
> On 09/25/2017 05:45 PM, Dmitry Osipenko wrote:
>> On 26.09.2017 02:01, Stephen Warren wrote:
>>> On 09/25/2017 04:15 PM, Dmitry Osipenko wrote:
 Video decoder, found on NVIDIA Tegra20 SoC, supports a standard set of
 video formats like H.264 / MPEG-4 / WMV / VC1. Currently driver supports
 decoding of CAVLC H.264 only.
>>>
>>> Note: I don't know anything much about video decoding on Tegra (just NV 
>>> desktop
>>> GPUs, and that was a while ago), but I had a couple small comments on the DT
>>> binding:
>>>
 diff --git
 a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
 b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
>>>
 +NVIDIA Tegra Video Decoder Engine
 +
 +Required properties:
 +- compatible : "nvidia,tegra20-vde"
 +- reg : Must contain 2 register ranges: registers and IRAM area.
 +- reg-names : Must include the following entries:
 +  - regs
 +  - iram
>>>
>>> I think the IRAM region needs more explanation: What is the region used for 
>>> and
>>> by what? Can it be moved, and if so does the move need to be co-ordinated 
>>> with
>>> any other piece of SW?
>>
>> IRAM region is used by Video Decoder HW for internal use and some of decoding
>> parameters are supplied via IRAM, like frames order list. AFAIK IRAM 
>> addresses
>> are hardwired in HW and aren't movable, it is not 100% but I'm pretty sure.
>> Should it be explained in the binding?
> 
> I think this should be briefly mentioned, yes. Otherwise at least people
> who don't know the VDE HW well (like me) will wonder why on earth VDE
> interacts with IRAM at all. I would have assumed all parameters were
> supplied via registers or via descriptors in DRAM.
> 
> Thanks.
> 

I also forgot to mention that VDE scrubs that IRAM region on HW reset. So yeah,
it's definitely a part of HW definition. I'll add a brief explanation to the
binding.

-- 
Dmitry
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 02/22] drm/i915: introduce simple gemfs

[Greg Kroah-Hartman]

On Mon, Sep 25, 2017 at 07:47:17PM +0100, Matthew Auld wrote:
> Not a fully blown gemfs, just our very own tmpfs kernel mount. Doing so
> moves us away from the shmemfs shm_mnt, and gives us the much needed
> flexibility to do things like set our own mount options, namely huge=
> which should allow us to enable the use of transparent-huge-pages for
> our shmem backed objects.
> 
> v2: various improvements suggested by Joonas
> 
> v3: move gemfs instance to i915.mm and simplify now that we have
> file_setup_with_mnt
> 
> v4: fallback to tmpfs shm_mnt upon failure to setup gemfs
> 
> v5: make tmpfs fallback kinder

Why do this only for one specific driver?  Shouldn't the drm core handle
this for you, for all other drivers as well?  Otherwise trying to figure
out how to "contain" this type of thing is going to be a pain (mount
options, selinux options, etc.)

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 05/14] mux: core: Add usb.h header with MUX_USB_* and and MUX_TYPEC_* state constants

[Heikki Krogerus]

Hi Hans,

Sorry about the late response.

On Fri, Sep 22, 2017 at 08:37:54PM +0200, Hans de Goede wrote:
> Add MUX_USB_* and MUX_TYPEC_* state constant defines, which can be used by
> USB device/host, resp. Type-C polarity/role/altmode mux drivers and
> consumers to ensure that they agree on the meaning of the
> mux_control_select() state argument.
> 
> Signed-off-by: Hans de Goede 
> ---
> Changes in v2:
> -Start numbering of defines at 0 not 1
> -Use a new usb.h header, rather then adding these to consumer.h
> -Add separate MUX_USB_* and MUX_TYPEC_* defines
> 
> Changes in v3:
> -Simplify MUX_TYPEC_* states, drop having separate USB HOST / DEVICE states
> ---
>  include/linux/mux/usb.h | 31 +++
>  1 file changed, 31 insertions(+)
>  create mode 100644 include/linux/mux/usb.h
> 
> diff --git a/include/linux/mux/usb.h b/include/linux/mux/usb.h
> new file mode 100644
> index ..2fec06846e14
> --- /dev/null
> +++ b/include/linux/mux/usb.h
> @@ -0,0 +1,31 @@
> +/*
> + * mux/usb.h - definitions for USB multiplexers
> + *
> + * Copyright (C) 2017 Hans de Goede 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef _LINUX_MUX_USB_H
> +#define _LINUX_MUX_USB_H
> +
> +/* Mux state values for USB device/host role muxes */
> +#define MUX_USB_DEVICE   (0) /* USB device mode */
> +#define MUX_USB_HOST (1) /* USB host mode */
> +#define MUX_USB_STATES   (2)
> +
> +/*
> + * Mux state values for Type-C polarity/role/altmode muxes.
> + *
> + * MUX_TYPEC_POLARITY_INV may be or-ed together with any other mux-state as
> + * inverted-polarity (Type-C plugged in upside down) can happen with any
> + * other mux-state.
> + */
> +#define MUX_TYPEC_POLARITY_INV   BIT(0)   /* Polarity inverted 
> bit */
> +#define MUX_TYPEC_USB(0 << 1) /* USB only mode */

Predefined values for the usb role are probable OK (maybe), but..

> +#define MUX_TYPEC_USB_AND_DP (1 << 1) /* USB host + 2 lanes DP */
> +#define MUX_TYPEC_DP (2 << 1) /* 4 lanes Display Port */
> +#define MUX_TYPEC_STATES (3 << 1)

..for alternate modes, no way. We don't need to try to keep a list of
all the possible states in one place. The pin configurations need to be
defined per alternate mode (per SVID), and we should not mix the
whole mux subsystem into that.

You are also only considering muxing. We need to deliver the
negotiated pin configurations to other components as well. For
example, in case of DisplayPort, the DP controller will need to know
at least the lane count, but also most likely the plug orientation.

And also, I think this is clear to everybody, but just in case it
isn't: Let's not mix TCPM or TCPC into the alternate mode specific pin
configuration handling at all. The alternate mode specific drivers can
take care of that. I think I need to prepare RFC out of my "alternate
mode bus" idea to give you guys an idea how that should work. Give me
a day or two.

But in any case, drop all alternate mode stuff from this series.
Let's move one step at the time.


Thanks,

-- 
heikki
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 2/2] staging: ion: create one device entry per heap

[Greg KH]

On Tue, Sep 26, 2017 at 07:09:14AM +0200, Daniel Vetter wrote:
> On Mon, Sep 25, 2017 at 11:26:27AM -0700, Laura Abbott wrote:
> > On 09/20/2017 01:45 AM, Benjamin Gaignard wrote:
> > > Instead a getting one common device "/dev/ion" for
> > > all the heaps this patch allow to create one device
> > > entry ("/dev/ionX") per heap.
> > > Getting an entry per heap could allow to set security rules
> > > per heap and global ones for all heaps.
> > > 
> > > Allocation requests will be only allowed if the mask_id
> > > match with device minor.
> > > Query request could be done on any of the devices.
> > > Deivce node major will also change and that may impact init scripts.
> > > 
> > 
> > We should start Cc linux-api for future changes since we're going
> > to have more than just /dev/ion.
> > 
> > Thinking about this some more, I think we need to allow backwards
> > compatibility. It's just not feasible to keep throwing workarounds
> > into userspace if we can avoid it. I'd propose keeping the old /dev/ion
> > misc interface available under a Kconfig and then creating the new
> > split heaps in parallel.
> > 
> > On a somewhat related note, there was some interest in possibly
> > having a sysfs interface for Ion long term. I don't think this
> > needs to happen right now but I'd like whatever we do to not
> > make adding that harder.
> 
> Not sure sysfs is a good idea for allocating buffers. The backlight
> interface is in sysfs, which defacto makes it a root-only interface.
> Distros really hate that, because it makes unpriviledged X/wayland really
> hard to pull of. Passing a device file otoh from logind to the compositor
> is dead simple (and how X et al get at e.g. the drm/input devices
> already).

sysfs is not a good idea for allocating buffers.  We already have some
out-of-tree drm drivers doing horrid things in sysfs in ways that
totally abuse that api, and vendors have to do crazy things with selinux
rules to try to lock it down because of that.  A device node is fine, we
are used to that for graphics stuff :)

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 0/2] NVIDIA Tegra20 video decoder driver

[Greg Kroah-Hartman]

On Tue, Sep 26, 2017 at 01:15:41AM +0300, Dmitry Osipenko wrote:
> This driver provides accelerated video decoding to NVIDIA Tegra20 SoC's,
> it is a result of reverse-engineering efforts. Driver has been tested on
> Toshiba AC100 and Acer A500, it should work on any Tegra20 device.
> 
> In userspace this driver is utilized by libvdpau-tegra [0] that implements
> VDPAU interface, so any video player that supports VDPAU can provide
> accelerated video decoding on Tegra20 on Linux.

Why not use the v4l2 api instead?  Doesn't that provide the same needed
user/kernel api here instead of creating yet-another-custom ioctl?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel