Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-22 Thread Laura Abbott 

On 10/22/15 3:36 AM, and...@ncrmnt.org wrote:

20 октября 2015 г., 19:34, "Mitchel Humpherys"  
написал:

On Tue, Oct 13 2015 at 11:14:23 AM, Andrew  wrote:


On 2015-10-12 21:39, Mitchel Humpherys wrote:

On Tue, Oct 06 2015 at 05:35:41 PM, Rob Herring 
wrote:

On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
wrote:


[...]


+Example:
+
+ ion {
+ compatbile = "linux,ion";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ion-system-heap {
+ linux,ion-heap-id = <0>;
+ linux,ion-heap-type = ;
+ linux,ion-heap-name = "system";


How does this vary across platforms? Is all of this being pushed down
to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be hardcoded
as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.


The heap-id<->heap-type mapping isn't necessarily 1:1. As Laura
indicated elsewhere on this thread, a given heap might need to be
contiguous on one platform but not on another. In that case you just
swap out the heap-type here and there's no need for userspace to change.

The heap-name, OTOH, could be derived from the heap-id, which is what we
hackishly do here [1] and here[2].


By the way, since we agreed that heap id and heap type mappings
are not 1:1 - we have a problem with the current API.

In userspace we currently have this:

int ion_alloc(int fd, size_t len, size_t align, unsigned int heap_mask,
unsigned int flags, ion_user_handle_t *handle);

We do not specify here what TYPE of heap we want the allocation to come
from.
This may lead to very unpleasant stuff when porting from one platfrom to
another.


Okay, I may be totally missing some point here then.


What "unpleasant stuff" are you referring to, exactly?


It's not really clear for me how (and at where - kernel or userspace)
we should properly sort out cases when the next device the pipeline
introduces some constraints on the buffer it can use.

For instance: camera can save data into a non-contiguous buffer, but the
image processing hardware (that may or may not be involved) expects the
buffer to be contiguous.



You've just hit on one of the open problems. There isn't a good answer
right now for how that is supposed to work. Sumit was working on
cenalloc as an answer to some of that. I think we've decided that
may become more of a long term theoretical problem to solve rather
than a pressing practical problem. These days IOMMUs and the like are
much more common across the entire system so it's becoming rarer to
have a case where some hardware can have discontiguous buffers but
some cannot. This isn't to say it's not a problem that needs to be
solved, but we're focusing on other efforts of Ion right now.

In general though we're working off the assumption that the
kernel knows the constraints and if userspace is requesting memory from
a particular heap the kernel will allocate the correct memory. This
means that it's up to the kernel to set up the heaps correctly for
a particular platform (once again, an open problem).

Thanks,
Laura


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-22 Thread andrew 
20 октября 2015 г., 19:34, "Mitchel Humpherys"  
написал:
> On Tue, Oct 13 2015 at 11:14:23 AM, Andrew  wrote:
> 
>> On 2015-10-12 21:39, Mitchel Humpherys wrote:
>>> On Tue, Oct 06 2015 at 05:35:41 PM, Rob Herring 
>>> wrote:
 On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
 wrote:
>>> 
>>> [...]
>>> 
> +Example:
> +
> + ion {
> + compatbile = "linux,ion";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ion-system-heap {
> + linux,ion-heap-id = <0>;
> + linux,ion-heap-type = ;
> + linux,ion-heap-name = "system";
 
 How does this vary across platforms? Is all of this being pushed down
 to DT, because there is no coordination of this at the kernel ABI
 level across platforms. In other words, why can't heap 0 be hardcoded
 as system heap in the driver. It seems to me any 1 of these 3
 properties could be used to derive the other 2.
>>> 
>>> The heap-id<->heap-type mapping isn't necessarily 1:1. As Laura
>>> indicated elsewhere on this thread, a given heap might need to be
>>> contiguous on one platform but not on another. In that case you just
>>> swap out the heap-type here and there's no need for userspace to change.
>>> 
>>> The heap-name, OTOH, could be derived from the heap-id, which is what we
>>> hackishly do here [1] and here[2].
>> 
>> By the way, since we agreed that heap id and heap type mappings
>> are not 1:1 - we have a problem with the current API.
>> 
>> In userspace we currently have this:
>> 
>> int ion_alloc(int fd, size_t len, size_t align, unsigned int heap_mask,
>> unsigned int flags, ion_user_handle_t *handle);
>> 
>> We do not specify here what TYPE of heap we want the allocation to come
>> from.
>> This may lead to very unpleasant stuff when porting from one platfrom to
>> another.

Okay, I may be totally missing some point here then.

> What "unpleasant stuff" are you referring to, exactly? 

It's not really clear for me how (and at where - kernel or userspace) 
we should properly sort out cases when the next device the pipeline 
introduces some constraints on the buffer it can use. 

For instance: camera can save data into a non-contiguous buffer, but the 
image processing hardware (that may or may not be involved) expects the 
buffer to be contiguous.

> Abstracting the
> heap type away from userspace has actually made our lives easier since
> userspace doesn't need to know anything about the properties of the
> underlying platform. It just asks for a buffer from the "camera" heap,
> for example. On some platforms that's contiguous, on others it's not.
> 
> -Mitch
> 
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project

Regards, 
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-22 Thread Laura Abbott 

On 10/22/15 3:36 AM, and...@ncrmnt.org wrote:

20 октября 2015 г., 19:34, "Mitchel Humpherys"  
написал:

On Tue, Oct 13 2015 at 11:14:23 AM, Andrew  wrote:


On 2015-10-12 21:39, Mitchel Humpherys wrote:

On Tue, Oct 06 2015 at 05:35:41 PM, Rob Herring 
wrote:

On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
wrote:


[...]


+Example:
+
+ ion {
+ compatbile = "linux,ion";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ion-system-heap {
+ linux,ion-heap-id = <0>;
+ linux,ion-heap-type = ;
+ linux,ion-heap-name = "system";


How does this vary across platforms? Is all of this being pushed down
to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be hardcoded
as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.


The heap-id<->heap-type mapping isn't necessarily 1:1. As Laura
indicated elsewhere on this thread, a given heap might need to be
contiguous on one platform but not on another. In that case you just
swap out the heap-type here and there's no need for userspace to change.

The heap-name, OTOH, could be derived from the heap-id, which is what we
hackishly do here [1] and here[2].


By the way, since we agreed that heap id and heap type mappings
are not 1:1 - we have a problem with the current API.

In userspace we currently have this:

int ion_alloc(int fd, size_t len, size_t align, unsigned int heap_mask,
unsigned int flags, ion_user_handle_t *handle);

We do not specify here what TYPE of heap we want the allocation to come
from.
This may lead to very unpleasant stuff when porting from one platfrom to
another.


Okay, I may be totally missing some point here then.


What "unpleasant stuff" are you referring to, exactly?


It's not really clear for me how (and at where - kernel or userspace)
we should properly sort out cases when the next device the pipeline
introduces some constraints on the buffer it can use.

For instance: camera can save data into a non-contiguous buffer, but the
image processing hardware (that may or may not be involved) expects the
buffer to be contiguous.



You've just hit on one of the open problems. There isn't a good answer
right now for how that is supposed to work. Sumit was working on
cenalloc as an answer to some of that. I think we've decided that
may become more of a long term theoretical problem to solve rather
than a pressing practical problem. These days IOMMUs and the like are
much more common across the entire system so it's becoming rarer to
have a case where some hardware can have discontiguous buffers but
some cannot. This isn't to say it's not a problem that needs to be
solved, but we're focusing on other efforts of Ion right now.

In general though we're working off the assumption that the
kernel knows the constraints and if userspace is requesting memory from
a particular heap the kernel will allocate the correct memory. This
means that it's up to the kernel to set up the heaps correctly for
a particular platform (once again, an open problem).

Thanks,
Laura


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-22 Thread andrew 
20 октября 2015 г., 19:34, "Mitchel Humpherys"  
написал:
> On Tue, Oct 13 2015 at 11:14:23 AM, Andrew  wrote:
> 
>> On 2015-10-12 21:39, Mitchel Humpherys wrote:
>>> On Tue, Oct 06 2015 at 05:35:41 PM, Rob Herring 
>>> wrote:
 On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
 wrote:
>>> 
>>> [...]
>>> 
> +Example:
> +
> + ion {
> + compatbile = "linux,ion";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ion-system-heap {
> + linux,ion-heap-id = <0>;
> + linux,ion-heap-type = ;
> + linux,ion-heap-name = "system";
 
 How does this vary across platforms? Is all of this being pushed down
 to DT, because there is no coordination of this at the kernel ABI
 level across platforms. In other words, why can't heap 0 be hardcoded
 as system heap in the driver. It seems to me any 1 of these 3
 properties could be used to derive the other 2.
>>> 
>>> The heap-id<->heap-type mapping isn't necessarily 1:1. As Laura
>>> indicated elsewhere on this thread, a given heap might need to be
>>> contiguous on one platform but not on another. In that case you just
>>> swap out the heap-type here and there's no need for userspace to change.
>>> 
>>> The heap-name, OTOH, could be derived from the heap-id, which is what we
>>> hackishly do here [1] and here[2].
>> 
>> By the way, since we agreed that heap id and heap type mappings
>> are not 1:1 - we have a problem with the current API.
>> 
>> In userspace we currently have this:
>> 
>> int ion_alloc(int fd, size_t len, size_t align, unsigned int heap_mask,
>> unsigned int flags, ion_user_handle_t *handle);
>> 
>> We do not specify here what TYPE of heap we want the allocation to come
>> from.
>> This may lead to very unpleasant stuff when porting from one platfrom to
>> another.

Okay, I may be totally missing some point here then.

> What "unpleasant stuff" are you referring to, exactly? 

It's not really clear for me how (and at where - kernel or userspace) 
we should properly sort out cases when the next device the pipeline 
introduces some constraints on the buffer it can use. 

For instance: camera can save data into a non-contiguous buffer, but the 
image processing hardware (that may or may not be involved) expects the 
buffer to be contiguous.

> Abstracting the
> heap type away from userspace has actually made our lives easier since
> userspace doesn't need to know anything about the properties of the
> underlying platform. It just asks for a buffer from the "camera" heap,
> for example. On some platforms that's contiguous, on others it's not.
> 
> -Mitch
> 
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project

Regards, 
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-20 Thread Mitchel Humpherys 
On Tue, Oct 13 2015 at 11:14:23 AM, Andrew  wrote:
> On 2015-10-12 21:39, Mitchel Humpherys wrote:
>> On Tue, Oct 06 2015 at 05:35:41 PM, Rob Herring 
>> wrote:
>>> On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
>>> wrote:
>>
>> [...]
>>
 +Example:
 +
 +   ion {
 +   compatbile = "linux,ion";
 +   #address-cells = <1>;
 +   #size-cells = <0>;
 +
 +   ion-system-heap {
 +   linux,ion-heap-id = <0>;
 +   linux,ion-heap-type = ;
 +   linux,ion-heap-name = "system";
>>>
>>> How does this vary across platforms? Is all of this being pushed down
>>> to DT, because there is no coordination of this at the kernel ABI
>>> level across platforms. In other words, why can't heap 0 be hardcoded
>>> as system heap in the driver. It seems to me any 1 of these 3
>>> properties could be used to derive the other 2.
>>
>> The heap-id<->heap-type mapping isn't necessarily 1:1.  As Laura
>> indicated elsewhere on this thread, a given heap might need to be
>> contiguous on one platform but not on another.  In that case you just
>> swap out the heap-type here and there's no need for userspace to change.
>>
>> The heap-name, OTOH, could be derived from the heap-id, which is what we
>> hackishly do here [1] and here[2].
>
> By the way, since we agreed that heap id and heap type mappings
> are not 1:1 - we have a problem with the current API.
>
> In userspace we currently have this:
>
> int ion_alloc(int fd, size_t len, size_t align, unsigned int heap_mask,
>   unsigned int flags, ion_user_handle_t *handle);
>
> We do not specify here what TYPE of heap we want the allocation to come
> from.
> This may lead to very unpleasant stuff when porting from one platfrom to
> another.

What "unpleasant stuff" are you referring to, exactly?  Abstracting the
heap type away from userspace has actually made our lives easier since
userspace doesn't need to know anything about the properties of the
underlying platform.  It just asks for a buffer from the "camera" heap,
for example.  On some platforms that's contiguous, on others it's not.


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-20 Thread Mitchel Humpherys 
On Tue, Oct 13 2015 at 11:14:23 AM, Andrew  wrote:
> On 2015-10-12 21:39, Mitchel Humpherys wrote:
>> On Tue, Oct 06 2015 at 05:35:41 PM, Rob Herring 
>> wrote:
>>> On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
>>> wrote:
>>
>> [...]
>>
 +Example:
 +
 +   ion {
 +   compatbile = "linux,ion";
 +   #address-cells = <1>;
 +   #size-cells = <0>;
 +
 +   ion-system-heap {
 +   linux,ion-heap-id = <0>;
 +   linux,ion-heap-type = ;
 +   linux,ion-heap-name = "system";
>>>
>>> How does this vary across platforms? Is all of this being pushed down
>>> to DT, because there is no coordination of this at the kernel ABI
>>> level across platforms. In other words, why can't heap 0 be hardcoded
>>> as system heap in the driver. It seems to me any 1 of these 3
>>> properties could be used to derive the other 2.
>>
>> The heap-id<->heap-type mapping isn't necessarily 1:1.  As Laura
>> indicated elsewhere on this thread, a given heap might need to be
>> contiguous on one platform but not on another.  In that case you just
>> swap out the heap-type here and there's no need for userspace to change.
>>
>> The heap-name, OTOH, could be derived from the heap-id, which is what we
>> hackishly do here [1] and here[2].
>
> By the way, since we agreed that heap id and heap type mappings
> are not 1:1 - we have a problem with the current API.
>
> In userspace we currently have this:
>
> int ion_alloc(int fd, size_t len, size_t align, unsigned int heap_mask,
>   unsigned int flags, ion_user_handle_t *handle);
>
> We do not specify here what TYPE of heap we want the allocation to come
> from.
> This may lead to very unpleasant stuff when porting from one platfrom to
> another.

What "unpleasant stuff" are you referring to, exactly?  Abstracting the
heap type away from userspace has actually made our lives easier since
userspace doesn't need to know anything about the properties of the
underlying platform.  It just asks for a buffer from the "camera" heap,
for example.  On some platforms that's contiguous, on others it's not.


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-13 Thread Andrew 

On 2015-10-12 21:39, Mitchel Humpherys wrote:
On Tue, Oct 06 2015 at 05:35:41 PM, Rob Herring  
wrote:
On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
 wrote:


[...]


+Example:
+
+   ion {
+   compatbile = "linux,ion";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ion-system-heap {
+   linux,ion-heap-id = <0>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "system";


How does this vary across platforms? Is all of this being pushed down
to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be hardcoded
as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.


The heap-id<->heap-type mapping isn't necessarily 1:1.  As Laura
indicated elsewhere on this thread, a given heap might need to be
contiguous on one platform but not on another.  In that case you just
swap out the heap-type here and there's no need for userspace to 
change.


The heap-name, OTOH, could be derived from the heap-id, which is what 
we

hackishly do here [1] and here[2].


By the way, since we agreed that heap id and heap type mappings
are not 1:1 - we have a problem with the current API.

In userspace we currently have this:

int ion_alloc(int fd, size_t len, size_t align, unsigned int heap_mask,
  unsigned int flags, ion_user_handle_t *handle);

We do not specify here what TYPE of heap we want the allocation to come 
from.
This may lead to very unpleasant stuff when porting from one platfrom to 
another.


But, manual heap id control is VERY neat, since it allows to cover a lot 
of

platform-specific cases, where allocating memory from certain heaps can
yield performance boost.

In other words - maybe we should specify both suitable heap types and 
heap ids?



[1]
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.14/tree/drivers/staging/android/ion/msm/msm_ion.c?h=msm-3.14#n53
[2]
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.14/tree/drivers/staging/android/ion/msm/msm_ion.c?h=msm-3.14#n398


-Mitch


--
Regards,
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-13 Thread Andrew 

On 2015-10-12 21:39, Mitchel Humpherys wrote:
On Tue, Oct 06 2015 at 05:35:41 PM, Rob Herring  
wrote:
On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
 wrote:


[...]


+Example:
+
+   ion {
+   compatbile = "linux,ion";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ion-system-heap {
+   linux,ion-heap-id = <0>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "system";


How does this vary across platforms? Is all of this being pushed down
to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be hardcoded
as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.


The heap-id<->heap-type mapping isn't necessarily 1:1.  As Laura
indicated elsewhere on this thread, a given heap might need to be
contiguous on one platform but not on another.  In that case you just
swap out the heap-type here and there's no need for userspace to 
change.


The heap-name, OTOH, could be derived from the heap-id, which is what 
we

hackishly do here [1] and here[2].


By the way, since we agreed that heap id and heap type mappings
are not 1:1 - we have a problem with the current API.

In userspace we currently have this:

int ion_alloc(int fd, size_t len, size_t align, unsigned int heap_mask,
  unsigned int flags, ion_user_handle_t *handle);

We do not specify here what TYPE of heap we want the allocation to come 
from.
This may lead to very unpleasant stuff when porting from one platfrom to 
another.


But, manual heap id control is VERY neat, since it allows to cover a lot 
of

platform-specific cases, where allocating memory from certain heaps can
yield performance boost.

In other words - maybe we should specify both suitable heap types and 
heap ids?



[1]
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.14/tree/drivers/staging/android/ion/msm/msm_ion.c?h=msm-3.14#n53
[2]
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.14/tree/drivers/staging/android/ion/msm/msm_ion.c?h=msm-3.14#n398


-Mitch


--
Regards,
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-12 Thread Mitchel Humpherys 
On Tue, Oct 06 2015 at 05:35:41 PM, Rob Herring  wrote:
> On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott  
> wrote:

[...]

>> +Example:
>> +
>> +   ion {
>> +   compatbile = "linux,ion";
>> +   #address-cells = <1>;
>> +   #size-cells = <0>;
>> +
>> +   ion-system-heap {
>> +   linux,ion-heap-id = <0>;
>> +   linux,ion-heap-type = ;
>> +   linux,ion-heap-name = "system";
>
> How does this vary across platforms? Is all of this being pushed down
> to DT, because there is no coordination of this at the kernel ABI
> level across platforms. In other words, why can't heap 0 be hardcoded
> as system heap in the driver. It seems to me any 1 of these 3
> properties could be used to derive the other 2.

The heap-id<->heap-type mapping isn't necessarily 1:1.  As Laura
indicated elsewhere on this thread, a given heap might need to be
contiguous on one platform but not on another.  In that case you just
swap out the heap-type here and there's no need for userspace to change.

The heap-name, OTOH, could be derived from the heap-id, which is what we
hackishly do here [1] and here[2].

[1] 
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.14/tree/drivers/staging/android/ion/msm/msm_ion.c?h=msm-3.14#n53
[2] 
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.14/tree/drivers/staging/android/ion/msm/msm_ion.c?h=msm-3.14#n398


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-12 Thread Mitchel Humpherys 
On Tue, Oct 06 2015 at 05:35:41 PM, Rob Herring  wrote:
> On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott  
> wrote:

[...]

>> +Example:
>> +
>> +   ion {
>> +   compatbile = "linux,ion";
>> +   #address-cells = <1>;
>> +   #size-cells = <0>;
>> +
>> +   ion-system-heap {
>> +   linux,ion-heap-id = <0>;
>> +   linux,ion-heap-type = ;
>> +   linux,ion-heap-name = "system";
>
> How does this vary across platforms? Is all of this being pushed down
> to DT, because there is no coordination of this at the kernel ABI
> level across platforms. In other words, why can't heap 0 be hardcoded
> as system heap in the driver. It seems to me any 1 of these 3
> properties could be used to derive the other 2.

The heap-id<->heap-type mapping isn't necessarily 1:1.  As Laura
indicated elsewhere on this thread, a given heap might need to be
contiguous on one platform but not on another.  In that case you just
swap out the heap-type here and there's no need for userspace to change.

The heap-name, OTOH, could be derived from the heap-id, which is what we
hackishly do here [1] and here[2].

[1] 
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.14/tree/drivers/staging/android/ion/msm/msm_ion.c?h=msm-3.14#n53
[2] 
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.14/tree/drivers/staging/android/ion/msm/msm_ion.c?h=msm-3.14#n398


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-07 Thread Laura Abbott 

On 10/7/15 11:36 AM, Rob Herring wrote:

On Wed, Oct 7, 2015 at 5:36 AM, Andrew  wrote:

On 2015-10-07 02:01, Laura Abbott wrote:


On 10/6/15 3:35 PM, Rob Herring wrote:


On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
wrote:


From: Laura Abbott 


This adds a base set of devicetree bindings for the Ion memory
manager. This supports setting up the generic set of heaps and
their properties.

Signed-off-by: Laura Abbott 
Signed-off-by: Andrew Andrianov 
---
   drivers/staging/android/ion/devicetree.txt | 53
++



I have no issue with this going in here, but I do have lots of issues
with this binding.


   1 file changed, 53 insertions(+)
   create mode 100644 drivers/staging/android/ion/devicetree.txt

diff --git a/drivers/staging/android/ion/devicetree.txt
b/drivers/staging/android/ion/devicetree.txt
new file mode 100644
index 000..4a0c941
--- /dev/null
+++ b/drivers/staging/android/ion/devicetree.txt
@@ -0,0 +1,53 @@
+Ion Memory Manager
+
+Ion is a memory manager that allows for sharing of buffers via dma-buf.
+Ion allows for different types of allocation via an abstraction called
+a 'heap'. A heap represents a specific type of memory. Each heap has
+a different type. There can be multiple instances of the same heap
+type.
+
+Required properties for Ion
+
+- compatible: "linux,ion"
+
+All child nodes of a linux,ion node are interpreted as heaps
+
+required properties for heaps
+
+- linux,ion-heap-id: The Ion heap id used for allocation selection
+- linux,ion-heap-type: Ion heap type defined in ion.h
+- linux,ion-heap-name: Human readble name of the heap
+
+
+Optional properties
+- memory-region: A phandle to a memory region. Required for DMA heap
type
+(see reserved-memory.txt for details on the reservation)
+- linux,ion-heap-align: Alignment for the heap.
+
+Example:
+
+   ion {
+   compatbile = "linux,ion";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ion-system-heap {
+   linux,ion-heap-id = <0>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "system";



How does this vary across platforms? Is all of this being pushed down
to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be hardcoded
as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.



Right now there is no guarantee heap IDs will be the same across
platforms. The heap IDs are currently part of the userspace ABI
as well since userspace clients must pass in a mask of the heap
IDs to allocate from. If we assume all existing clients could change,
heaps such as the system heap could be mandated to have the same
heap ID but we'd still run into problems if you have multiple
heaps of the same type (e.g. multiple carveouts)


Vendors largely ignore the kernel-userspace ABI and anything in
staging is not a ABI. So arguments about what the ABI is currently is
pointless IMO.

Pushing an inconsistent kernel ABI to DT is not the answer.



I'm not sure I agree it's inconsistent because it varies across
platforms. IRQs vary across platforms (yes I know something something
hardware description). Vendors really should be caring about ABIs
and it's kind of a chicken and egg problem about when staging
driver ABIs should be considered stable. Perhaps it should be
"Pushing bad kernel ABIs to DT is not the answer" which is a
fair objection to Ion.



I don't really like the idea of enforcing any IDs here. As of now
heap ids are generally something VERY platform-specific
(or even product-specific). Personally I'd prefer something like this
for userspace apps:

int id1 = ion_get_heap_id("camera");
if (id1 < 0) {
   fprintf(stderr, "Invalid heap id");
   exit(1);
}

int id2 = ion_get_heap_id("backup-heap");
if (id2 < 0) {
   fprintf(stderr, "Invalid heap id");
   exit(1);
}


We've learned that creating number spaces like this are bad (irqs,
gpios, /dev nodes). We should move away from that. Why should
userspace care about IDs or what the IDs are? An ID is just encoding
certain implicit requirements. So are the strings here. Users should
express what capabilities, restrictions, etc. they have, and then the
kernel can find the best heap.



I'd argue that the heap IDs are expressing capabilities and
restrictions. A heap ID for camera could be contiguous on
one system and discontiguous on another based on the system.
The user only gives the ID, not the type so it's ultimately
up to the kernel to decide what that heap ID means on a particular
system. Ion allows or'ing of heap IDs together and the kernel
selects them currently based on priority. What's your idea for
expressing the capabilities without resorting IDs?

I'm going to give some more thought to this but I think I
might look into just having each heap be a compatible
string. This might fit in better with

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-07 Thread Andrew 

On 2015-10-07 21:36, Rob Herring wrote:

On Wed, Oct 7, 2015 at 5:36 AM, Andrew  wrote:

On 2015-10-07 02:01, Laura Abbott wrote:


On 10/6/15 3:35 PM, Rob Herring wrote:


On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 


wrote:


From: Laura Abbott 


This adds a base set of devicetree bindings for the Ion memory
manager. This supports setting up the generic set of heaps and
their properties.

Signed-off-by: Laura Abbott 
Signed-off-by: Andrew Andrianov 
---
  drivers/staging/android/ion/devicetree.txt | 53
++



I have no issue with this going in here, but I do have lots of 
issues

with this binding.


  1 file changed, 53 insertions(+)
  create mode 100644 drivers/staging/android/ion/devicetree.txt

diff --git a/drivers/staging/android/ion/devicetree.txt
b/drivers/staging/android/ion/devicetree.txt
new file mode 100644
index 000..4a0c941
--- /dev/null
+++ b/drivers/staging/android/ion/devicetree.txt
@@ -0,0 +1,53 @@
+Ion Memory Manager
+
+Ion is a memory manager that allows for sharing of buffers via 
dma-buf.
+Ion allows for different types of allocation via an abstraction 
called
+a 'heap'. A heap represents a specific type of memory. Each heap 
has

+a different type. There can be multiple instances of the same heap
+type.
+
+Required properties for Ion
+
+- compatible: "linux,ion"
+
+All child nodes of a linux,ion node are interpreted as heaps
+
+required properties for heaps
+
+- linux,ion-heap-id: The Ion heap id used for allocation selection
+- linux,ion-heap-type: Ion heap type defined in ion.h
+- linux,ion-heap-name: Human readble name of the heap
+
+
+Optional properties
+- memory-region: A phandle to a memory region. Required for DMA 
heap

type
+(see reserved-memory.txt for details on the reservation)
+- linux,ion-heap-align: Alignment for the heap.
+
+Example:
+
+   ion {
+   compatbile = "linux,ion";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ion-system-heap {
+   linux,ion-heap-id = <0>;
+   linux,ion-heap-type = 
;

+   linux,ion-heap-name = "system";



How does this vary across platforms? Is all of this being pushed 
down

to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be 
hardcoded

as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.



Right now there is no guarantee heap IDs will be the same across
platforms. The heap IDs are currently part of the userspace ABI
as well since userspace clients must pass in a mask of the heap
IDs to allocate from. If we assume all existing clients could change,
heaps such as the system heap could be mandated to have the same
heap ID but we'd still run into problems if you have multiple
heaps of the same type (e.g. multiple carveouts)


Vendors largely ignore the kernel-userspace ABI and anything in
staging is not a ABI. So arguments about what the ABI is currently is
pointless IMO.

Pushing an inconsistent kernel ABI to DT is not the answer.


Totally agree here.





I don't really like the idea of enforcing any IDs here. As of now
heap ids are generally something VERY platform-specific
(or even product-specific). Personally I'd prefer something like this
for userspace apps:

int id1 = ion_get_heap_id("camera");
if (id1 < 0) {
  fprintf(stderr, "Invalid heap id");
  exit(1);
}

int id2 = ion_get_heap_id("backup-heap");
if (id2 < 0) {
  fprintf(stderr, "Invalid heap id");
  exit(1);
}


We've learned that creating number spaces like this are bad (irqs,
gpios, /dev nodes). We should move away from that. Why should
userspace care about IDs or what the IDs are? An ID is just encoding
certain implicit requirements. So are the strings here. Users should
express what capabilities, restrictions, etc. they have, and then the
kernel can find the best heap.


I'd argue about that point, since sometimes kernel might NOT know
all the hardware details behind some of the heaps or how they are going 
to
be used. Hence it can't decide a proper heap. And that's where things 
get ugly.


Real-world example: There are several on-chip SRAM banks that make up 
several

heaps. Say, IM0, IM1, IM2.

Technically - they are all DMA, and all work. But the hardware guys hand 
you

a handful of weird recommendations, like:
* Decoder will work faster if you use bank IM2 for internal buffers,
and prefer DDR bank A for decoded frames.
* DSP should stick with IM1, and please prefer DDR bank B for buffers

When several such devices are involved in one chain - things may get
even weirder. Having manual control over where allocations are made 
allows
us to keep all these voodoo magicks away from the kernel and (hopefully) 
keep

vendors from dirty hacks into ion itself.




...

int ret = ion_alloc(fd, 0x100, 0x4,
  (id1 | id2),
  0, );


What concerns kernel

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-07 Thread Rob Herring 
On Wed, Oct 7, 2015 at 5:36 AM, Andrew  wrote:
> On 2015-10-07 02:01, Laura Abbott wrote:
>>
>> On 10/6/15 3:35 PM, Rob Herring wrote:
>>>
>>> On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
>>> wrote:

 From: Laura Abbott 


 This adds a base set of devicetree bindings for the Ion memory
 manager. This supports setting up the generic set of heaps and
 their properties.

 Signed-off-by: Laura Abbott 
 Signed-off-by: Andrew Andrianov 
 ---
   drivers/staging/android/ion/devicetree.txt | 53
 ++
>>>
>>>
>>> I have no issue with this going in here, but I do have lots of issues
>>> with this binding.
>>>
   1 file changed, 53 insertions(+)
   create mode 100644 drivers/staging/android/ion/devicetree.txt

 diff --git a/drivers/staging/android/ion/devicetree.txt
 b/drivers/staging/android/ion/devicetree.txt
 new file mode 100644
 index 000..4a0c941
 --- /dev/null
 +++ b/drivers/staging/android/ion/devicetree.txt
 @@ -0,0 +1,53 @@
 +Ion Memory Manager
 +
 +Ion is a memory manager that allows for sharing of buffers via dma-buf.
 +Ion allows for different types of allocation via an abstraction called
 +a 'heap'. A heap represents a specific type of memory. Each heap has
 +a different type. There can be multiple instances of the same heap
 +type.
 +
 +Required properties for Ion
 +
 +- compatible: "linux,ion"
 +
 +All child nodes of a linux,ion node are interpreted as heaps
 +
 +required properties for heaps
 +
 +- linux,ion-heap-id: The Ion heap id used for allocation selection
 +- linux,ion-heap-type: Ion heap type defined in ion.h
 +- linux,ion-heap-name: Human readble name of the heap
 +
 +
 +Optional properties
 +- memory-region: A phandle to a memory region. Required for DMA heap
 type
 +(see reserved-memory.txt for details on the reservation)
 +- linux,ion-heap-align: Alignment for the heap.
 +
 +Example:
 +
 +   ion {
 +   compatbile = "linux,ion";
 +   #address-cells = <1>;
 +   #size-cells = <0>;
 +
 +   ion-system-heap {
 +   linux,ion-heap-id = <0>;
 +   linux,ion-heap-type = ;
 +   linux,ion-heap-name = "system";
>>>
>>>
>>> How does this vary across platforms? Is all of this being pushed down
>>> to DT, because there is no coordination of this at the kernel ABI
>>> level across platforms. In other words, why can't heap 0 be hardcoded
>>> as system heap in the driver. It seems to me any 1 of these 3
>>> properties could be used to derive the other 2.
>>>
>>
>> Right now there is no guarantee heap IDs will be the same across
>> platforms. The heap IDs are currently part of the userspace ABI
>> as well since userspace clients must pass in a mask of the heap
>> IDs to allocate from. If we assume all existing clients could change,
>> heaps such as the system heap could be mandated to have the same
>> heap ID but we'd still run into problems if you have multiple
>> heaps of the same type (e.g. multiple carveouts)

Vendors largely ignore the kernel-userspace ABI and anything in
staging is not a ABI. So arguments about what the ABI is currently is
pointless IMO.

Pushing an inconsistent kernel ABI to DT is not the answer.

>
> I don't really like the idea of enforcing any IDs here. As of now
> heap ids are generally something VERY platform-specific
> (or even product-specific). Personally I'd prefer something like this
> for userspace apps:
>
> int id1 = ion_get_heap_id("camera");
> if (id1 < 0) {
>   fprintf(stderr, "Invalid heap id");
>   exit(1);
> }
>
> int id2 = ion_get_heap_id("backup-heap");
> if (id2 < 0) {
>   fprintf(stderr, "Invalid heap id");
>   exit(1);
> }

We've learned that creating number spaces like this are bad (irqs,
gpios, /dev nodes). We should move away from that. Why should
userspace care about IDs or what the IDs are? An ID is just encoding
certain implicit requirements. So are the strings here. Users should
express what capabilities, restrictions, etc. they have, and then the
kernel can find the best heap.

> ...
>
> int ret = ion_alloc(fd, 0x100, 0x4,
>   (id1 | id2),
>   0, );
>
>
> What concerns kernel stuff, things are simpler - we may just pass the heap
> to use
> by a reference in devicetree node for that driver. Something like that:
>
> ...
>ion-decoder-region : region_ddr {
>linux,ion-heap-id = <1>;
>linux,ion-heap-type = ;
>linux,ion-heap-name = "decoder_mem"
>memory-region = <_region>;
> };
> ...
> video_decoder@8018 {
> compatible = "acme,h266dec";
> reg =

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-07 Thread Andrew 

On 2015-10-07 02:01, Laura Abbott wrote:

On 10/6/15 3:35 PM, Rob Herring wrote:
On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
 wrote:

From: Laura Abbott 


This adds a base set of devicetree bindings for the Ion memory
manager. This supports setting up the generic set of heaps and
their properties.

Signed-off-by: Laura Abbott 
Signed-off-by: Andrew Andrianov 
---
  drivers/staging/android/ion/devicetree.txt | 53 
++


I have no issue with this going in here, but I do have lots of issues
with this binding.


  1 file changed, 53 insertions(+)
  create mode 100644 drivers/staging/android/ion/devicetree.txt

diff --git a/drivers/staging/android/ion/devicetree.txt 
b/drivers/staging/android/ion/devicetree.txt

new file mode 100644
index 000..4a0c941
--- /dev/null
+++ b/drivers/staging/android/ion/devicetree.txt
@@ -0,0 +1,53 @@
+Ion Memory Manager
+
+Ion is a memory manager that allows for sharing of buffers via 
dma-buf.
+Ion allows for different types of allocation via an abstraction 
called

+a 'heap'. A heap represents a specific type of memory. Each heap has
+a different type. There can be multiple instances of the same heap
+type.
+
+Required properties for Ion
+
+- compatible: "linux,ion"
+
+All child nodes of a linux,ion node are interpreted as heaps
+
+required properties for heaps
+
+- linux,ion-heap-id: The Ion heap id used for allocation selection
+- linux,ion-heap-type: Ion heap type defined in ion.h
+- linux,ion-heap-name: Human readble name of the heap
+
+
+Optional properties
+- memory-region: A phandle to a memory region. Required for DMA heap 
type

+(see reserved-memory.txt for details on the reservation)
+- linux,ion-heap-align: Alignment for the heap.
+
+Example:
+
+   ion {
+   compatbile = "linux,ion";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ion-system-heap {
+   linux,ion-heap-id = <0>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "system";


How does this vary across platforms? Is all of this being pushed down
to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be hardcoded
as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.



Right now there is no guarantee heap IDs will be the same across
platforms. The heap IDs are currently part of the userspace ABI
as well since userspace clients must pass in a mask of the heap
IDs to allocate from. If we assume all existing clients could change,
heaps such as the system heap could be mandated to have the same
heap ID but we'd still run into problems if you have multiple
heaps of the same type (e.g. multiple carveouts)


I don't really like the idea of enforcing any IDs here. As of now
heap ids are generally something VERY platform-specific
(or even product-specific). Personally I'd prefer something like this
for userspace apps:

int id1 = ion_get_heap_id("camera");
if (id1 < 0) {
  fprintf(stderr, "Invalid heap id");
  exit(1);
}

int id2 = ion_get_heap_id("backup-heap");
if (id2 < 0) {
  fprintf(stderr, "Invalid heap id");
  exit(1);
}

...

int ret = ion_alloc(fd, 0x100, 0x4,
  (id1 | id2),
  0, );


What concerns kernel stuff, things are simpler - we may just pass the 
heap to use

by a reference in devicetree node for that driver. Something like that:

...
   ion-decoder-region : region_ddr {
   linux,ion-heap-id = <1>;
   linux,ion-heap-type = ;
   linux,ion-heap-name = "decoder_mem"
   memory-region = <_region>;
};
...
video_decoder@8018 {
compatible = "acme,h266dec";
reg = <0x8018 0x2>,
reg-names = "registers";
interrupts = <12>;
interrupt-parent = <>;
ion-heaps-for-buffers = <>
};





An alternative might be to have each heap just be a compatible string
and pull everything (id, type etc.) into C files for setup. I debated
doing that but decided to try putting everything in DT for my first
pass.




+   };
+
+   ion-camera-region {
+   linux,ion-heap-id = <1>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "camera"
+   memory-region = <_region>;


Couldn't the memory-region node with addition properties or some
standardization of existing ones provide enough information for ION's
needs?



I think we could probably derive most of it from the memory-region 
right

now. If it's reusable, it's DMA, if not it goes to a carveout. Name can
come from the node name. heap ID and whether or not a region is a

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-07 Thread Laura Abbott 

On 10/7/15 11:36 AM, Rob Herring wrote:

On Wed, Oct 7, 2015 at 5:36 AM, Andrew  wrote:

On 2015-10-07 02:01, Laura Abbott wrote:


On 10/6/15 3:35 PM, Rob Herring wrote:


On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
wrote:


From: Laura Abbott 


This adds a base set of devicetree bindings for the Ion memory
manager. This supports setting up the generic set of heaps and
their properties.

Signed-off-by: Laura Abbott 
Signed-off-by: Andrew Andrianov 
---
   drivers/staging/android/ion/devicetree.txt | 53
++



I have no issue with this going in here, but I do have lots of issues
with this binding.


   1 file changed, 53 insertions(+)
   create mode 100644 drivers/staging/android/ion/devicetree.txt

diff --git a/drivers/staging/android/ion/devicetree.txt
b/drivers/staging/android/ion/devicetree.txt
new file mode 100644
index 000..4a0c941
--- /dev/null
+++ b/drivers/staging/android/ion/devicetree.txt
@@ -0,0 +1,53 @@
+Ion Memory Manager
+
+Ion is a memory manager that allows for sharing of buffers via dma-buf.
+Ion allows for different types of allocation via an abstraction called
+a 'heap'. A heap represents a specific type of memory. Each heap has
+a different type. There can be multiple instances of the same heap
+type.
+
+Required properties for Ion
+
+- compatible: "linux,ion"
+
+All child nodes of a linux,ion node are interpreted as heaps
+
+required properties for heaps
+
+- linux,ion-heap-id: The Ion heap id used for allocation selection
+- linux,ion-heap-type: Ion heap type defined in ion.h
+- linux,ion-heap-name: Human readble name of the heap
+
+
+Optional properties
+- memory-region: A phandle to a memory region. Required for DMA heap
type
+(see reserved-memory.txt for details on the reservation)
+- linux,ion-heap-align: Alignment for the heap.
+
+Example:
+
+   ion {
+   compatbile = "linux,ion";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ion-system-heap {
+   linux,ion-heap-id = <0>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "system";



How does this vary across platforms? Is all of this being pushed down
to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be hardcoded
as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.



Right now there is no guarantee heap IDs will be the same across
platforms. The heap IDs are currently part of the userspace ABI
as well since userspace clients must pass in a mask of the heap
IDs to allocate from. If we assume all existing clients could change,
heaps such as the system heap could be mandated to have the same
heap ID but we'd still run into problems if you have multiple
heaps of the same type (e.g. multiple carveouts)


Vendors largely ignore the kernel-userspace ABI and anything in
staging is not a ABI. So arguments about what the ABI is currently is
pointless IMO.

Pushing an inconsistent kernel ABI to DT is not the answer.



I'm not sure I agree it's inconsistent because it varies across
platforms. IRQs vary across platforms (yes I know something something
hardware description). Vendors really should be caring about ABIs
and it's kind of a chicken and egg problem about when staging
driver ABIs should be considered stable. Perhaps it should be
"Pushing bad kernel ABIs to DT is not the answer" which is a
fair objection to Ion.



I don't really like the idea of enforcing any IDs here. As of now
heap ids are generally something VERY platform-specific
(or even product-specific). Personally I'd prefer something like this
for userspace apps:

int id1 = ion_get_heap_id("camera");
if (id1 < 0) {
   fprintf(stderr, "Invalid heap id");
   exit(1);
}

int id2 = ion_get_heap_id("backup-heap");
if (id2 < 0) {
   fprintf(stderr, "Invalid heap id");
   exit(1);
}


We've learned that creating number spaces like this are bad (irqs,
gpios, /dev nodes). We should move away from that. Why should
userspace care about IDs or what the IDs are? An ID is just encoding
certain implicit requirements. So are the strings here. Users should
express what capabilities, restrictions, etc. they have, and then the
kernel can find the best heap.



I'd argue that the heap IDs are expressing capabilities and
restrictions. A heap ID for camera could be contiguous on
one system and discontiguous on another based on the system.
The user only gives the ID, not the type so it's ultimately
up to the kernel to decide what that heap ID means on a particular
system. Ion allows or'ing of heap IDs together and the kernel
selects them currently based on priority. What's your idea for
expressing the capabilities without resorting IDs?

I'm going to give some more thought to this but I

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-07 Thread Andrew 

On 2015-10-07 21:36, Rob Herring wrote:

On Wed, Oct 7, 2015 at 5:36 AM, Andrew  wrote:

On 2015-10-07 02:01, Laura Abbott wrote:


On 10/6/15 3:35 PM, Rob Herring wrote:


On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 


wrote:


From: Laura Abbott 


This adds a base set of devicetree bindings for the Ion memory
manager. This supports setting up the generic set of heaps and
their properties.

Signed-off-by: Laura Abbott 
Signed-off-by: Andrew Andrianov 
---
  drivers/staging/android/ion/devicetree.txt | 53
++



I have no issue with this going in here, but I do have lots of 
issues

with this binding.


  1 file changed, 53 insertions(+)
  create mode 100644 drivers/staging/android/ion/devicetree.txt

diff --git a/drivers/staging/android/ion/devicetree.txt
b/drivers/staging/android/ion/devicetree.txt
new file mode 100644
index 000..4a0c941
--- /dev/null
+++ b/drivers/staging/android/ion/devicetree.txt
@@ -0,0 +1,53 @@
+Ion Memory Manager
+
+Ion is a memory manager that allows for sharing of buffers via 
dma-buf.
+Ion allows for different types of allocation via an abstraction 
called
+a 'heap'. A heap represents a specific type of memory. Each heap 
has

+a different type. There can be multiple instances of the same heap
+type.
+
+Required properties for Ion
+
+- compatible: "linux,ion"
+
+All child nodes of a linux,ion node are interpreted as heaps
+
+required properties for heaps
+
+- linux,ion-heap-id: The Ion heap id used for allocation selection
+- linux,ion-heap-type: Ion heap type defined in ion.h
+- linux,ion-heap-name: Human readble name of the heap
+
+
+Optional properties
+- memory-region: A phandle to a memory region. Required for DMA 
heap

type
+(see reserved-memory.txt for details on the reservation)
+- linux,ion-heap-align: Alignment for the heap.
+
+Example:
+
+   ion {
+   compatbile = "linux,ion";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ion-system-heap {
+   linux,ion-heap-id = <0>;
+   linux,ion-heap-type = 
;

+   linux,ion-heap-name = "system";



How does this vary across platforms? Is all of this being pushed 
down

to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be 
hardcoded

as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.



Right now there is no guarantee heap IDs will be the same across
platforms. The heap IDs are currently part of the userspace ABI
as well since userspace clients must pass in a mask of the heap
IDs to allocate from. If we assume all existing clients could change,
heaps such as the system heap could be mandated to have the same
heap ID but we'd still run into problems if you have multiple
heaps of the same type (e.g. multiple carveouts)


Vendors largely ignore the kernel-userspace ABI and anything in
staging is not a ABI. So arguments about what the ABI is currently is
pointless IMO.

Pushing an inconsistent kernel ABI to DT is not the answer.


Totally agree here.





I don't really like the idea of enforcing any IDs here. As of now
heap ids are generally something VERY platform-specific
(or even product-specific). Personally I'd prefer something like this
for userspace apps:

int id1 = ion_get_heap_id("camera");
if (id1 < 0) {
  fprintf(stderr, "Invalid heap id");
  exit(1);
}

int id2 = ion_get_heap_id("backup-heap");
if (id2 < 0) {
  fprintf(stderr, "Invalid heap id");
  exit(1);
}


We've learned that creating number spaces like this are bad (irqs,
gpios, /dev nodes). We should move away from that. Why should
userspace care about IDs or what the IDs are? An ID is just encoding
certain implicit requirements. So are the strings here. Users should
express what capabilities, restrictions, etc. they have, and then the
kernel can find the best heap.


I'd argue about that point, since sometimes kernel might NOT know
all the hardware details behind some of the heaps or how they are going 
to
be used. Hence it can't decide a proper heap. And that's where things 
get ugly.


Real-world example: There are several on-chip SRAM banks that make up 
several

heaps. Say, IM0, IM1, IM2.

Technically - they are all DMA, and all work. But the hardware guys hand 
you

a handful of weird recommendations, like:
* Decoder will work faster if you use bank IM2 for internal buffers,
and prefer DDR bank A for decoded frames.
* DSP should stick with IM1, and please prefer DDR bank B for buffers

When several such devices are involved in one chain - things may get
even weirder. Having manual control over where allocations are made 
allows
us to keep all these voodoo magicks away from the kernel and (hopefully) 
keep

vendors from dirty hacks into ion itself.




...

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-07 Thread Andrew 

On 2015-10-07 02:01, Laura Abbott wrote:

On 10/6/15 3:35 PM, Rob Herring wrote:
On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
 wrote:

From: Laura Abbott 


This adds a base set of devicetree bindings for the Ion memory
manager. This supports setting up the generic set of heaps and
their properties.

Signed-off-by: Laura Abbott 
Signed-off-by: Andrew Andrianov 
---
  drivers/staging/android/ion/devicetree.txt | 53 
++


I have no issue with this going in here, but I do have lots of issues
with this binding.


  1 file changed, 53 insertions(+)
  create mode 100644 drivers/staging/android/ion/devicetree.txt

diff --git a/drivers/staging/android/ion/devicetree.txt 
b/drivers/staging/android/ion/devicetree.txt

new file mode 100644
index 000..4a0c941
--- /dev/null
+++ b/drivers/staging/android/ion/devicetree.txt
@@ -0,0 +1,53 @@
+Ion Memory Manager
+
+Ion is a memory manager that allows for sharing of buffers via 
dma-buf.
+Ion allows for different types of allocation via an abstraction 
called

+a 'heap'. A heap represents a specific type of memory. Each heap has
+a different type. There can be multiple instances of the same heap
+type.
+
+Required properties for Ion
+
+- compatible: "linux,ion"
+
+All child nodes of a linux,ion node are interpreted as heaps
+
+required properties for heaps
+
+- linux,ion-heap-id: The Ion heap id used for allocation selection
+- linux,ion-heap-type: Ion heap type defined in ion.h
+- linux,ion-heap-name: Human readble name of the heap
+
+
+Optional properties
+- memory-region: A phandle to a memory region. Required for DMA heap 
type

+(see reserved-memory.txt for details on the reservation)
+- linux,ion-heap-align: Alignment for the heap.
+
+Example:
+
+   ion {
+   compatbile = "linux,ion";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ion-system-heap {
+   linux,ion-heap-id = <0>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "system";


How does this vary across platforms? Is all of this being pushed down
to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be hardcoded
as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.



Right now there is no guarantee heap IDs will be the same across
platforms. The heap IDs are currently part of the userspace ABI
as well since userspace clients must pass in a mask of the heap
IDs to allocate from. If we assume all existing clients could change,
heaps such as the system heap could be mandated to have the same
heap ID but we'd still run into problems if you have multiple
heaps of the same type (e.g. multiple carveouts)


I don't really like the idea of enforcing any IDs here. As of now
heap ids are generally something VERY platform-specific
(or even product-specific). Personally I'd prefer something like this
for userspace apps:

int id1 = ion_get_heap_id("camera");
if (id1 < 0) {
  fprintf(stderr, "Invalid heap id");
  exit(1);
}

int id2 = ion_get_heap_id("backup-heap");
if (id2 < 0) {
  fprintf(stderr, "Invalid heap id");
  exit(1);
}

...

int ret = ion_alloc(fd, 0x100, 0x4,
  (id1 | id2),
  0, );


What concerns kernel stuff, things are simpler - we may just pass the 
heap to use

by a reference in devicetree node for that driver. Something like that:

...
   ion-decoder-region : region_ddr {
   linux,ion-heap-id = <1>;
   linux,ion-heap-type = ;
   linux,ion-heap-name = "decoder_mem"
   memory-region = <_region>;
};
...
video_decoder@8018 {
compatible = "acme,h266dec";
reg = <0x8018 0x2>,
reg-names = "registers";
interrupts = <12>;
interrupt-parent = <>;
ion-heaps-for-buffers = <>
};





An alternative might be to have each heap just be a compatible string
and pull everything (id, type etc.) into C files for setup. I debated
doing that but decided to try putting everything in DT for my first
pass.




+   };
+
+   ion-camera-region {
+   linux,ion-heap-id = <1>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "camera"
+   memory-region = <_region>;


Couldn't the memory-region node with addition properties or some
standardization of existing ones provide enough information for ION's
needs?



I think we could probably derive most of it from the memory-region 
right

now. If it's reusable, it's DMA, if not it goes to a

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-07 Thread Rob Herring 
On Wed, Oct 7, 2015 at 5:36 AM, Andrew  wrote:
> On 2015-10-07 02:01, Laura Abbott wrote:
>>
>> On 10/6/15 3:35 PM, Rob Herring wrote:
>>>
>>> On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott 
>>> wrote:

 From: Laura Abbott 


 This adds a base set of devicetree bindings for the Ion memory
 manager. This supports setting up the generic set of heaps and
 their properties.

 Signed-off-by: Laura Abbott 
 Signed-off-by: Andrew Andrianov 
 ---
   drivers/staging/android/ion/devicetree.txt | 53
 ++
>>>
>>>
>>> I have no issue with this going in here, but I do have lots of issues
>>> with this binding.
>>>
   1 file changed, 53 insertions(+)
   create mode 100644 drivers/staging/android/ion/devicetree.txt

 diff --git a/drivers/staging/android/ion/devicetree.txt
 b/drivers/staging/android/ion/devicetree.txt
 new file mode 100644
 index 000..4a0c941
 --- /dev/null
 +++ b/drivers/staging/android/ion/devicetree.txt
 @@ -0,0 +1,53 @@
 +Ion Memory Manager
 +
 +Ion is a memory manager that allows for sharing of buffers via dma-buf.
 +Ion allows for different types of allocation via an abstraction called
 +a 'heap'. A heap represents a specific type of memory. Each heap has
 +a different type. There can be multiple instances of the same heap
 +type.
 +
 +Required properties for Ion
 +
 +- compatible: "linux,ion"
 +
 +All child nodes of a linux,ion node are interpreted as heaps
 +
 +required properties for heaps
 +
 +- linux,ion-heap-id: The Ion heap id used for allocation selection
 +- linux,ion-heap-type: Ion heap type defined in ion.h
 +- linux,ion-heap-name: Human readble name of the heap
 +
 +
 +Optional properties
 +- memory-region: A phandle to a memory region. Required for DMA heap
 type
 +(see reserved-memory.txt for details on the reservation)
 +- linux,ion-heap-align: Alignment for the heap.
 +
 +Example:
 +
 +   ion {
 +   compatbile = "linux,ion";
 +   #address-cells = <1>;
 +   #size-cells = <0>;
 +
 +   ion-system-heap {
 +   linux,ion-heap-id = <0>;
 +   linux,ion-heap-type = ;
 +   linux,ion-heap-name = "system";
>>>
>>>
>>> How does this vary across platforms? Is all of this being pushed down
>>> to DT, because there is no coordination of this at the kernel ABI
>>> level across platforms. In other words, why can't heap 0 be hardcoded
>>> as system heap in the driver. It seems to me any 1 of these 3
>>> properties could be used to derive the other 2.
>>>
>>
>> Right now there is no guarantee heap IDs will be the same across
>> platforms. The heap IDs are currently part of the userspace ABI
>> as well since userspace clients must pass in a mask of the heap
>> IDs to allocate from. If we assume all existing clients could change,
>> heaps such as the system heap could be mandated to have the same
>> heap ID but we'd still run into problems if you have multiple
>> heaps of the same type (e.g. multiple carveouts)

Vendors largely ignore the kernel-userspace ABI and anything in
staging is not a ABI. So arguments about what the ABI is currently is
pointless IMO.

Pushing an inconsistent kernel ABI to DT is not the answer.

>
> I don't really like the idea of enforcing any IDs here. As of now
> heap ids are generally something VERY platform-specific
> (or even product-specific). Personally I'd prefer something like this
> for userspace apps:
>
> int id1 = ion_get_heap_id("camera");
> if (id1 < 0) {
>   fprintf(stderr, "Invalid heap id");
>   exit(1);
> }
>
> int id2 = ion_get_heap_id("backup-heap");
> if (id2 < 0) {
>   fprintf(stderr, "Invalid heap id");
>   exit(1);
> }

We've learned that creating number spaces like this are bad (irqs,
gpios, /dev nodes). We should move away from that. Why should
userspace care about IDs or what the IDs are? An ID is just encoding
certain implicit requirements. So are the strings here. Users should
express what capabilities, restrictions, etc. they have, and then the
kernel can find the best heap.

> ...
>
> int ret = ion_alloc(fd, 0x100, 0x4,
>   (id1 | id2),
>   0, );
>
>
> What concerns kernel stuff, things are simpler - we may just pass the heap
> to use
> by a reference in devicetree node for that driver. Something like that:
>
> ...
>ion-decoder-region : region_ddr {
>linux,ion-heap-id = <1>;
>linux,ion-heap-type = ;
>linux,ion-heap-name = "decoder_mem"
>memory-region = <_region>;
> };
> ...
>

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-06 Thread Laura Abbott 

On 10/6/15 3:35 PM, Rob Herring wrote:

On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott  wrote:

From: Laura Abbott 


This adds a base set of devicetree bindings for the Ion memory
manager. This supports setting up the generic set of heaps and
their properties.

Signed-off-by: Laura Abbott 
Signed-off-by: Andrew Andrianov 
---
  drivers/staging/android/ion/devicetree.txt | 53 ++


I have no issue with this going in here, but I do have lots of issues
with this binding.


  1 file changed, 53 insertions(+)
  create mode 100644 drivers/staging/android/ion/devicetree.txt

diff --git a/drivers/staging/android/ion/devicetree.txt 
b/drivers/staging/android/ion/devicetree.txt
new file mode 100644
index 000..4a0c941
--- /dev/null
+++ b/drivers/staging/android/ion/devicetree.txt
@@ -0,0 +1,53 @@
+Ion Memory Manager
+
+Ion is a memory manager that allows for sharing of buffers via dma-buf.
+Ion allows for different types of allocation via an abstraction called
+a 'heap'. A heap represents a specific type of memory. Each heap has
+a different type. There can be multiple instances of the same heap
+type.
+
+Required properties for Ion
+
+- compatible: "linux,ion"
+
+All child nodes of a linux,ion node are interpreted as heaps
+
+required properties for heaps
+
+- linux,ion-heap-id: The Ion heap id used for allocation selection
+- linux,ion-heap-type: Ion heap type defined in ion.h
+- linux,ion-heap-name: Human readble name of the heap
+
+
+Optional properties
+- memory-region: A phandle to a memory region. Required for DMA heap type
+(see reserved-memory.txt for details on the reservation)
+- linux,ion-heap-align: Alignment for the heap.
+
+Example:
+
+   ion {
+   compatbile = "linux,ion";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ion-system-heap {
+   linux,ion-heap-id = <0>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "system";


How does this vary across platforms? Is all of this being pushed down
to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be hardcoded
as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.



Right now there is no guarantee heap IDs will be the same across
platforms. The heap IDs are currently part of the userspace ABI
as well since userspace clients must pass in a mask of the heap
IDs to allocate from. If we assume all existing clients could change,
heaps such as the system heap could be mandated to have the same
heap ID but we'd still run into problems if you have multiple
heaps of the same type (e.g. multiple carveouts)

An alternative might be to have each heap just be a compatible string
and pull everything (id, type etc.) into C files for setup. I debated
doing that but decided to try putting everything in DT for my first
pass.




+   };
+
+   ion-camera-region {
+   linux,ion-heap-id = <1>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "camera"
+   memory-region = <_region>;


Couldn't the memory-region node with addition properties or some
standardization of existing ones provide enough information for ION's
needs?



I think we could probably derive most of it from the memory-region right
now. If it's reusable, it's DMA, if not it goes to a carveout. Name can
come from the node name. heap ID and whether or not a region is a chunk
heap could be added as properties.

We'd still need to be able to get the same information for heaps that
don't correspond to a specific region like the system heap.


+   };
+
+   ion-fb-region {
+   linux,ion-heap-id = <2>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "fb"
+   memory-region = <_region>;
+   };
+   }
--
2.4.3



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-06 Thread Rob Herring 
On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott  wrote:
> From: Laura Abbott 
>
>
> This adds a base set of devicetree bindings for the Ion memory
> manager. This supports setting up the generic set of heaps and
> their properties.
>
> Signed-off-by: Laura Abbott 
> Signed-off-by: Andrew Andrianov 
> ---
>  drivers/staging/android/ion/devicetree.txt | 53 
> ++

I have no issue with this going in here, but I do have lots of issues
with this binding.

>  1 file changed, 53 insertions(+)
>  create mode 100644 drivers/staging/android/ion/devicetree.txt
>
> diff --git a/drivers/staging/android/ion/devicetree.txt 
> b/drivers/staging/android/ion/devicetree.txt
> new file mode 100644
> index 000..4a0c941
> --- /dev/null
> +++ b/drivers/staging/android/ion/devicetree.txt
> @@ -0,0 +1,53 @@
> +Ion Memory Manager
> +
> +Ion is a memory manager that allows for sharing of buffers via dma-buf.
> +Ion allows for different types of allocation via an abstraction called
> +a 'heap'. A heap represents a specific type of memory. Each heap has
> +a different type. There can be multiple instances of the same heap
> +type.
> +
> +Required properties for Ion
> +
> +- compatible: "linux,ion"
> +
> +All child nodes of a linux,ion node are interpreted as heaps
> +
> +required properties for heaps
> +
> +- linux,ion-heap-id: The Ion heap id used for allocation selection
> +- linux,ion-heap-type: Ion heap type defined in ion.h
> +- linux,ion-heap-name: Human readble name of the heap
> +
> +
> +Optional properties
> +- memory-region: A phandle to a memory region. Required for DMA heap type
> +(see reserved-memory.txt for details on the reservation)
> +- linux,ion-heap-align: Alignment for the heap.
> +
> +Example:
> +
> +   ion {
> +   compatbile = "linux,ion";
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   ion-system-heap {
> +   linux,ion-heap-id = <0>;
> +   linux,ion-heap-type = ;
> +   linux,ion-heap-name = "system";

How does this vary across platforms? Is all of this being pushed down
to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be hardcoded
as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.


> +   };
> +
> +   ion-camera-region {
> +   linux,ion-heap-id = <1>;
> +   linux,ion-heap-type = ;
> +   linux,ion-heap-name = "camera"
> +   memory-region = <_region>;

Couldn't the memory-region node with addition properties or some
standardization of existing ones provide enough information for ION's
needs?

> +   };
> +
> +   ion-fb-region {
> +   linux,ion-heap-id = <2>;
> +   linux,ion-heap-type = ;
> +   linux,ion-heap-name = "fb"
> +   memory-region = <_region>;
> +   };
> +   }
> --
> 2.4.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-06 Thread Rob Herring 
On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott  wrote:
> From: Laura Abbott 
>
>
> This adds a base set of devicetree bindings for the Ion memory
> manager. This supports setting up the generic set of heaps and
> their properties.
>
> Signed-off-by: Laura Abbott 
> Signed-off-by: Andrew Andrianov 
> ---
>  drivers/staging/android/ion/devicetree.txt | 53 
> ++

I have no issue with this going in here, but I do have lots of issues
with this binding.

>  1 file changed, 53 insertions(+)
>  create mode 100644 drivers/staging/android/ion/devicetree.txt
>
> diff --git a/drivers/staging/android/ion/devicetree.txt 
> b/drivers/staging/android/ion/devicetree.txt
> new file mode 100644
> index 000..4a0c941
> --- /dev/null
> +++ b/drivers/staging/android/ion/devicetree.txt
> @@ -0,0 +1,53 @@
> +Ion Memory Manager
> +
> +Ion is a memory manager that allows for sharing of buffers via dma-buf.
> +Ion allows for different types of allocation via an abstraction called
> +a 'heap'. A heap represents a specific type of memory. Each heap has
> +a different type. There can be multiple instances of the same heap
> +type.
> +
> +Required properties for Ion
> +
> +- compatible: "linux,ion"
> +
> +All child nodes of a linux,ion node are interpreted as heaps
> +
> +required properties for heaps
> +
> +- linux,ion-heap-id: The Ion heap id used for allocation selection
> +- linux,ion-heap-type: Ion heap type defined in ion.h
> +- linux,ion-heap-name: Human readble name of the heap
> +
> +
> +Optional properties
> +- memory-region: A phandle to a memory region. Required for DMA heap type
> +(see reserved-memory.txt for details on the reservation)
> +- linux,ion-heap-align: Alignment for the heap.
> +
> +Example:
> +
> +   ion {
> +   compatbile = "linux,ion";
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   ion-system-heap {
> +   linux,ion-heap-id = <0>;
> +   linux,ion-heap-type = ;
> +   linux,ion-heap-name = "system";

How does this vary across platforms? Is all of this being pushed down
to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be hardcoded
as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.


> +   };
> +
> +   ion-camera-region {
> +   linux,ion-heap-id = <1>;
> +   linux,ion-heap-type = ;
> +   linux,ion-heap-name = "camera"
> +   memory-region = <_region>;

Couldn't the memory-region node with addition properties or some
standardization of existing ones provide enough information for ION's
needs?

> +   };
> +
> +   ion-fb-region {
> +   linux,ion-heap-id = <2>;
> +   linux,ion-heap-type = ;
> +   linux,ion-heap-name = "fb"
> +   memory-region = <_region>;
> +   };
> +   }
> --
> 2.4.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC][PATCH 1/2] WIP: Devicetree bindings for Ion

2015-10-06 Thread Laura Abbott 

On 10/6/15 3:35 PM, Rob Herring wrote:

On Tue, Oct 6, 2015 at 3:47 PM, Laura Abbott  wrote:

From: Laura Abbott 


This adds a base set of devicetree bindings for the Ion memory
manager. This supports setting up the generic set of heaps and
their properties.

Signed-off-by: Laura Abbott 
Signed-off-by: Andrew Andrianov 
---
  drivers/staging/android/ion/devicetree.txt | 53 ++


I have no issue with this going in here, but I do have lots of issues
with this binding.


  1 file changed, 53 insertions(+)
  create mode 100644 drivers/staging/android/ion/devicetree.txt

diff --git a/drivers/staging/android/ion/devicetree.txt 
b/drivers/staging/android/ion/devicetree.txt
new file mode 100644
index 000..4a0c941
--- /dev/null
+++ b/drivers/staging/android/ion/devicetree.txt
@@ -0,0 +1,53 @@
+Ion Memory Manager
+
+Ion is a memory manager that allows for sharing of buffers via dma-buf.
+Ion allows for different types of allocation via an abstraction called
+a 'heap'. A heap represents a specific type of memory. Each heap has
+a different type. There can be multiple instances of the same heap
+type.
+
+Required properties for Ion
+
+- compatible: "linux,ion"
+
+All child nodes of a linux,ion node are interpreted as heaps
+
+required properties for heaps
+
+- linux,ion-heap-id: The Ion heap id used for allocation selection
+- linux,ion-heap-type: Ion heap type defined in ion.h
+- linux,ion-heap-name: Human readble name of the heap
+
+
+Optional properties
+- memory-region: A phandle to a memory region. Required for DMA heap type
+(see reserved-memory.txt for details on the reservation)
+- linux,ion-heap-align: Alignment for the heap.
+
+Example:
+
+   ion {
+   compatbile = "linux,ion";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ion-system-heap {
+   linux,ion-heap-id = <0>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "system";


How does this vary across platforms? Is all of this being pushed down
to DT, because there is no coordination of this at the kernel ABI
level across platforms. In other words, why can't heap 0 be hardcoded
as system heap in the driver. It seems to me any 1 of these 3
properties could be used to derive the other 2.



Right now there is no guarantee heap IDs will be the same across
platforms. The heap IDs are currently part of the userspace ABI
as well since userspace clients must pass in a mask of the heap
IDs to allocate from. If we assume all existing clients could change,
heaps such as the system heap could be mandated to have the same
heap ID but we'd still run into problems if you have multiple
heaps of the same type (e.g. multiple carveouts)

An alternative might be to have each heap just be a compatible string
and pull everything (id, type etc.) into C files for setup. I debated
doing that but decided to try putting everything in DT for my first
pass.




+   };
+
+   ion-camera-region {
+   linux,ion-heap-id = <1>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "camera"
+   memory-region = <_region>;


Couldn't the memory-region node with addition properties or some
standardization of existing ones provide enough information for ION's
needs?



I think we could probably derive most of it from the memory-region right
now. If it's reusable, it's DMA, if not it goes to a carveout. Name can
come from the node name. heap ID and whether or not a region is a chunk
heap could be added as properties.

We'd still need to be able to get the same information for heaps that
don't correspond to a specific region like the system heap.


+   };
+
+   ion-fb-region {
+   linux,ion-heap-id = <2>;
+   linux,ion-heap-type = ;
+   linux,ion-heap-name = "fb"
+   memory-region = <_region>;
+   };
+   }
--
2.4.3



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/