Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Chih-Wei Huang]

2018-03-22 16:23 GMT+08:00 Tomasz Figa :
> Hi Chih-Wei,
>
> On Thu, Feb 22, 2018 at 2:53 PM, Chih-Wei Huang  wrote:
>> 2018-02-21 3:03 GMT+08:00 Rob Herring :
>>>
>>> Perhaps worth revisiting. Given we've failed to progress at all since
>>> then may change opinions some. We already have to handle multiple
>>> opens share the same pipe_screen, so I don't think reusing the fd buys
>>> us anything.
>>>
>>> Maybe we're close to the point of removing the flink name support too.
>>> The android-x86 folks have been working to get dma-bufs working.
>>> Chih-Wei, any comments on this?
>>
>> Ah! Sorry. I didn't catch or understand the details.
>> Did you mean the attempts to use drm_hwcomposer
>> in android-x86?
>> My understanding so far is most x86 GPUs won't work
>> except some very limited models.
>> It's due to kernel driver issues which may never be solved.
>> So we can't drop the flink name support.
>> Please correct me if I am wrong.
>
> Could you elaborate a bit more on those GPUs that won't work and
> corresponding driver issues? We're running cros_gralloc on Intel and
> AMD GPUs, with DMA-buf and render-node only setup and we haven't seen
> any problems.

Hi Tomasz,
I remember (in our previous discussion) you said
CrOS uses your own hwcomposer (proprietary?) so
the story may be different.

What we have tried is gbm_gralloc + drm_hwcomposer
and I reported the issues here:
https://lists.freedesktop.org/archives/dri-devel/2017-September/153580.html

Sorry I didn't make more tests recently.
I CC Mauro to comment. He should have made more tests
with newer kernel and mesa.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tapani Pälli]

On 03/23/2018 03:20 PM, Tomasz Figa wrote:

On Fri, Mar 23, 2018 at 10:15 PM, Stefan Schake  wrote:

On Fri, Mar 23, 2018 at 1:02 PM, Tomasz Figa  wrote:

On Fri, Mar 23, 2018 at 8:52 PM, Robert Foss  wrote:

Hey Chih-Wei,


On 03/23/2018 03:43 AM, Chih-Wei Huang wrote:


2018-03-22 16:23 GMT+08:00 Tomasz Figa :


Hi Chih-Wei,

On Thu, Feb 22, 2018 at 2:53 PM, Chih-Wei Huang 
wrote:


2018-02-21 3:03 GMT+08:00 Rob Herring :



Perhaps worth revisiting. Given we've failed to progress at all since
then may change opinions some. We already have to handle multiple
opens share the same pipe_screen, so I don't think reusing the fd buys
us anything.

Maybe we're close to the point of removing the flink name support too.
The android-x86 folks have been working to get dma-bufs working.
Chih-Wei, any comments on this?



Ah! Sorry. I didn't catch or understand the details.
Did you mean the attempts to use drm_hwcomposer
in android-x86?
My understanding so far is most x86 GPUs won't work
except some very limited models.
It's due to kernel driver issues which may never be solved.
So we can't drop the flink name support.
Please correct me if I am wrong.



Could you elaborate a bit more on those GPUs that won't work and
corresponding driver issues? We're running cros_gralloc on Intel and
AMD GPUs, with DMA-buf and render-node only setup and we haven't seen
any problems.



Hi Tomasz,
I remember (in our previous discussion) you said
CrOS uses your own hwcomposer (proprietary?) so
the story may be different.

What we have tried is gbm_gralloc + drm_hwcomposer
and I reported the issues here:

https://lists.freedesktop.org/archives/dri-devel/2017-September/153580.html



I took the liberty of moving you nouveau issue into the drm_hwc gitlab
bugtracker.

https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/issues/1


Hmm, reading further in that thread, that seemed to be related to
missing nouveau.atomic=1 in kernel command line:
https://lists.freedesktop.org/archives/dri-devel/2017-September/153681.html

It went further, but didn't work very well. I suspect that could have
been related to DRM_GRALLOC_GET_FD being used, which makes the mess
from the system due to gralloc and Mesa stepping on each other's GEM
handles (which aren't reference counted by design and importing a
buffer several time will always return the same handle).


I think mixing in drm_hwcomposer here would be trying to do too much at once.
It has a lot of requirements (atomic driver, fence fds, alpha/rotation props)
and no fallback path to speak of. Coming from drm_gralloc where I think in
practice all composition is punted to the hwui GL compositor and DRM is
only around for putting the final framebuffer on a display, it's asking a lot
of some of the (legacy) hardware used for Android-x86.


Do we have any feasible alternative?

Chih-Wei pointed me to the drm_gralloc used on android-x86 and it
seems to seriously pre-date any DMA-buf support. Everything there is
done with the assumption that GEM names are used and adding DMA-buf
would require a significant rework of all the code.


One possibility for android-x86 project would be to have a separate 
lunch target (and libraries) for the old machines. Those ones unlikely 
support OpenGL ES 3.1?), hardware planes, Vulkan or other modern Android 
requirements anyway. These machines could run with old (and stable) 
Mesa, drm_gralloc and maybe the old hwcomposer from Intel that does hwc 
1.0/1.1, there were even patches to use planes there via drmModeSetPlane 
in video usecases IIRC.



That leaves us with gbm_gralloc (or possibly Chromium minigbm, used by
Android-IA too), which doesn't implement the legacy FB management, so
some hwcomposer would be needed.

Best regards,
Tomasz


// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

On Fri, Mar 23, 2018 at 10:15 PM, Stefan Schake  wrote:
> On Fri, Mar 23, 2018 at 1:02 PM, Tomasz Figa  wrote:
>> On Fri, Mar 23, 2018 at 8:52 PM, Robert Foss  
>> wrote:
>>> Hey Chih-Wei,
>>>
>>>
>>> On 03/23/2018 03:43 AM, Chih-Wei Huang wrote:

 2018-03-22 16:23 GMT+08:00 Tomasz Figa :
>
> Hi Chih-Wei,
>
> On Thu, Feb 22, 2018 at 2:53 PM, Chih-Wei Huang 
> wrote:
>>
>> 2018-02-21 3:03 GMT+08:00 Rob Herring :
>>>
>>>
>>> Perhaps worth revisiting. Given we've failed to progress at all since
>>> then may change opinions some. We already have to handle multiple
>>> opens share the same pipe_screen, so I don't think reusing the fd buys
>>> us anything.
>>>
>>> Maybe we're close to the point of removing the flink name support too.
>>> The android-x86 folks have been working to get dma-bufs working.
>>> Chih-Wei, any comments on this?
>>
>>
>> Ah! Sorry. I didn't catch or understand the details.
>> Did you mean the attempts to use drm_hwcomposer
>> in android-x86?
>> My understanding so far is most x86 GPUs won't work
>> except some very limited models.
>> It's due to kernel driver issues which may never be solved.
>> So we can't drop the flink name support.
>> Please correct me if I am wrong.
>
>
> Could you elaborate a bit more on those GPUs that won't work and
> corresponding driver issues? We're running cros_gralloc on Intel and
> AMD GPUs, with DMA-buf and render-node only setup and we haven't seen
> any problems.


 Hi Tomasz,
 I remember (in our previous discussion) you said
 CrOS uses your own hwcomposer (proprietary?) so
 the story may be different.

 What we have tried is gbm_gralloc + drm_hwcomposer
 and I reported the issues here:

 https://lists.freedesktop.org/archives/dri-devel/2017-September/153580.html
>>>
>>>
>>> I took the liberty of moving you nouveau issue into the drm_hwc gitlab
>>> bugtracker.
>>>
>>> https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/issues/1
>>
>> Hmm, reading further in that thread, that seemed to be related to
>> missing nouveau.atomic=1 in kernel command line:
>> https://lists.freedesktop.org/archives/dri-devel/2017-September/153681.html
>>
>> It went further, but didn't work very well. I suspect that could have
>> been related to DRM_GRALLOC_GET_FD being used, which makes the mess
>> from the system due to gralloc and Mesa stepping on each other's GEM
>> handles (which aren't reference counted by design and importing a
>> buffer several time will always return the same handle).
>
> I think mixing in drm_hwcomposer here would be trying to do too much at once.
> It has a lot of requirements (atomic driver, fence fds, alpha/rotation props)
> and no fallback path to speak of. Coming from drm_gralloc where I think in
> practice all composition is punted to the hwui GL compositor and DRM is
> only around for putting the final framebuffer on a display, it's asking a lot
> of some of the (legacy) hardware used for Android-x86.

Do we have any feasible alternative?

Chih-Wei pointed me to the drm_gralloc used on android-x86 and it
seems to seriously pre-date any DMA-buf support. Everything there is
done with the assumption that GEM names are used and adding DMA-buf
would require a significant rework of all the code.

That leaves us with gbm_gralloc (or possibly Chromium minigbm, used by
Android-IA too), which doesn't implement the legacy FB management, so
some hwcomposer would be needed.

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Stefan Schake]

On Fri, Mar 23, 2018 at 1:02 PM, Tomasz Figa  wrote:
> On Fri, Mar 23, 2018 at 8:52 PM, Robert Foss  
> wrote:
>> Hey Chih-Wei,
>>
>>
>> On 03/23/2018 03:43 AM, Chih-Wei Huang wrote:
>>>
>>> 2018-03-22 16:23 GMT+08:00 Tomasz Figa :

 Hi Chih-Wei,

 On Thu, Feb 22, 2018 at 2:53 PM, Chih-Wei Huang 
 wrote:
>
> 2018-02-21 3:03 GMT+08:00 Rob Herring :
>>
>>
>> Perhaps worth revisiting. Given we've failed to progress at all since
>> then may change opinions some. We already have to handle multiple
>> opens share the same pipe_screen, so I don't think reusing the fd buys
>> us anything.
>>
>> Maybe we're close to the point of removing the flink name support too.
>> The android-x86 folks have been working to get dma-bufs working.
>> Chih-Wei, any comments on this?
>
>
> Ah! Sorry. I didn't catch or understand the details.
> Did you mean the attempts to use drm_hwcomposer
> in android-x86?
> My understanding so far is most x86 GPUs won't work
> except some very limited models.
> It's due to kernel driver issues which may never be solved.
> So we can't drop the flink name support.
> Please correct me if I am wrong.


 Could you elaborate a bit more on those GPUs that won't work and
 corresponding driver issues? We're running cros_gralloc on Intel and
 AMD GPUs, with DMA-buf and render-node only setup and we haven't seen
 any problems.
>>>
>>>
>>> Hi Tomasz,
>>> I remember (in our previous discussion) you said
>>> CrOS uses your own hwcomposer (proprietary?) so
>>> the story may be different.
>>>
>>> What we have tried is gbm_gralloc + drm_hwcomposer
>>> and I reported the issues here:
>>>
>>> https://lists.freedesktop.org/archives/dri-devel/2017-September/153580.html
>>
>>
>> I took the liberty of moving you nouveau issue into the drm_hwc gitlab
>> bugtracker.
>>
>> https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/issues/1
>
> Hmm, reading further in that thread, that seemed to be related to
> missing nouveau.atomic=1 in kernel command line:
> https://lists.freedesktop.org/archives/dri-devel/2017-September/153681.html
>
> It went further, but didn't work very well. I suspect that could have
> been related to DRM_GRALLOC_GET_FD being used, which makes the mess
> from the system due to gralloc and Mesa stepping on each other's GEM
> handles (which aren't reference counted by design and importing a
> buffer several time will always return the same handle).

I think mixing in drm_hwcomposer here would be trying to do too much at once.
It has a lot of requirements (atomic driver, fence fds, alpha/rotation props)
and no fallback path to speak of. Coming from drm_gralloc where I think in
practice all composition is punted to the hwui GL compositor and DRM is
only around for putting the final framebuffer on a display, it's asking a lot
of some of the (legacy) hardware used for Android-x86.

Thanks,
Stefan
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

On Fri, Mar 23, 2018 at 8:52 PM, Robert Foss  wrote:
> Hey Chih-Wei,
>
>
> On 03/23/2018 03:43 AM, Chih-Wei Huang wrote:
>>
>> 2018-03-22 16:23 GMT+08:00 Tomasz Figa :
>>>
>>> Hi Chih-Wei,
>>>
>>> On Thu, Feb 22, 2018 at 2:53 PM, Chih-Wei Huang 
>>> wrote:

 2018-02-21 3:03 GMT+08:00 Rob Herring :
>
>
> Perhaps worth revisiting. Given we've failed to progress at all since
> then may change opinions some. We already have to handle multiple
> opens share the same pipe_screen, so I don't think reusing the fd buys
> us anything.
>
> Maybe we're close to the point of removing the flink name support too.
> The android-x86 folks have been working to get dma-bufs working.
> Chih-Wei, any comments on this?


 Ah! Sorry. I didn't catch or understand the details.
 Did you mean the attempts to use drm_hwcomposer
 in android-x86?
 My understanding so far is most x86 GPUs won't work
 except some very limited models.
 It's due to kernel driver issues which may never be solved.
 So we can't drop the flink name support.
 Please correct me if I am wrong.
>>>
>>>
>>> Could you elaborate a bit more on those GPUs that won't work and
>>> corresponding driver issues? We're running cros_gralloc on Intel and
>>> AMD GPUs, with DMA-buf and render-node only setup and we haven't seen
>>> any problems.
>>
>>
>> Hi Tomasz,
>> I remember (in our previous discussion) you said
>> CrOS uses your own hwcomposer (proprietary?) so
>> the story may be different.
>>
>> What we have tried is gbm_gralloc + drm_hwcomposer
>> and I reported the issues here:
>>
>> https://lists.freedesktop.org/archives/dri-devel/2017-September/153580.html
>
>
> I took the liberty of moving you nouveau issue into the drm_hwc gitlab
> bugtracker.
>
> https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/issues/1

Hmm, reading further in that thread, that seemed to be related to
missing nouveau.atomic=1 in kernel command line:
https://lists.freedesktop.org/archives/dri-devel/2017-September/153681.html

It went further, but didn't work very well. I suspect that could have
been related to DRM_GRALLOC_GET_FD being used, which makes the mess
from the system due to gralloc and Mesa stepping on each other's GEM
handles (which aren't reference counted by design and importing a
buffer several time will always return the same handle).

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Robert Foss]

Hey Chih-Wei,

On 03/23/2018 03:43 AM, Chih-Wei Huang wrote:

2018-03-22 16:23 GMT+08:00 Tomasz Figa :

Hi Chih-Wei,

On Thu, Feb 22, 2018 at 2:53 PM, Chih-Wei Huang  wrote:

2018-02-21 3:03 GMT+08:00 Rob Herring :


Perhaps worth revisiting. Given we've failed to progress at all since
then may change opinions some. We already have to handle multiple
opens share the same pipe_screen, so I don't think reusing the fd buys
us anything.

Maybe we're close to the point of removing the flink name support too.
The android-x86 folks have been working to get dma-bufs working.
Chih-Wei, any comments on this?


Ah! Sorry. I didn't catch or understand the details.
Did you mean the attempts to use drm_hwcomposer
in android-x86?
My understanding so far is most x86 GPUs won't work
except some very limited models.
It's due to kernel driver issues which may never be solved.
So we can't drop the flink name support.
Please correct me if I am wrong.


Could you elaborate a bit more on those GPUs that won't work and
corresponding driver issues? We're running cros_gralloc on Intel and
AMD GPUs, with DMA-buf and render-node only setup and we haven't seen
any problems.


Hi Tomasz,
I remember (in our previous discussion) you said
CrOS uses your own hwcomposer (proprietary?) so
the story may be different.

What we have tried is gbm_gralloc + drm_hwcomposer
and I reported the issues here:
https://lists.freedesktop.org/archives/dri-devel/2017-September/153580.html


I took the liberty of moving you nouveau issue into the drm_hwc gitlab 
bugtracker.

https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/issues/1



Sorry I didn't make more tests recently.
I CC Mauro to comment. He should have made more tests
with newer kernel and mesa.


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

On Fri, Mar 23, 2018 at 11:43 AM, Chih-Wei Huang  wrote:
> 2018-03-22 16:23 GMT+08:00 Tomasz Figa :
>> Hi Chih-Wei,
>>
>> On Thu, Feb 22, 2018 at 2:53 PM, Chih-Wei Huang  wrote:
>>> 2018-02-21 3:03 GMT+08:00 Rob Herring :

 Perhaps worth revisiting. Given we've failed to progress at all since
 then may change opinions some. We already have to handle multiple
 opens share the same pipe_screen, so I don't think reusing the fd buys
 us anything.

 Maybe we're close to the point of removing the flink name support too.
 The android-x86 folks have been working to get dma-bufs working.
 Chih-Wei, any comments on this?
>>>
>>> Ah! Sorry. I didn't catch or understand the details.
>>> Did you mean the attempts to use drm_hwcomposer
>>> in android-x86?
>>> My understanding so far is most x86 GPUs won't work
>>> except some very limited models.
>>> It's due to kernel driver issues which may never be solved.
>>> So we can't drop the flink name support.
>>> Please correct me if I am wrong.
>>
>> Could you elaborate a bit more on those GPUs that won't work and
>> corresponding driver issues? We're running cros_gralloc on Intel and
>> AMD GPUs, with DMA-buf and render-node only setup and we haven't seen
>> any problems.
>
> Hi Tomasz,
> I remember (in our previous discussion) you said
> CrOS uses your own hwcomposer (proprietary?) so
> the story may be different.
>
> What we have tried is gbm_gralloc + drm_hwcomposer
> and I reported the issues here:
> https://lists.freedesktop.org/archives/dri-devel/2017-September/153580.html
>
> Sorry I didn't make more tests recently.
> I CC Mauro to comment. He should have made more tests
> with newer kernel and mesa.

Thanks for clarification.

This sounds like something that could be fixed by making sure DMA-buf
support is properly implemented in gralloc and hwcomposer used by
android-x86. I can help figuring this out, if anyone would be
interested in driving this on your side. FYI, I'm UTC+9 time zone, so
based on .tw domain in your email, it should be possible for us to
communicate easily, for example by IRC.

If i915 DRI driver is also needed, it would need wiring support for
DMA-buf as well, but it shouldn't be too hard as mentioned earlier

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Jason Ekstrand]

On March 22, 2018 06:07:01 Tapani Pälli  wrote:

On 03/22/2018 01:01 PM, Emil Velikov wrote:
Hi guys,

On 22 March 2018 at 08:23, Tomasz Figa  wrote:
Hi Chih-Wei,

On Thu, Feb 22, 2018 at 2:53 PM, Chih-Wei Huang  wrote:
2018-02-21 3:03 GMT+08:00 Rob Herring :

Perhaps worth revisiting. Given we've failed to progress at all since
then may change opinions some. We already have to handle multiple
opens share the same pipe_screen, so I don't think reusing the fd buys
us anything.

Maybe we're close to the point of removing the flink name support too.
The android-x86 folks have been working to get dma-bufs working.
Chih-Wei, any comments on this?

Ah! Sorry. I didn't catch or understand the details.
Did you mean the attempts to use drm_hwcomposer
in android-x86?
My understanding so far is most x86 GPUs won't work
except some very limited models.
It's due to kernel driver issues which may never be solved.
So we can't drop the flink name support.
Please correct me if I am wrong.

Could you elaborate a bit more on those GPUs that won't work and
corresponding driver issues? We're running cros_gralloc on Intel and
AMD GPUs, with DMA-buf and render-node only setup and we haven't seen
any problems.
May I suggest an alternative to, diving into details of the issues
observed by the Android-x86 team.

Namely: Tomasz, please send a patch ripping all the flink stuff, thus
Chih-Wei and others can test it.

Ideally it'll be a quick and easy "works for me" ;-)

This has been discussed before and IIRC one reason is that i915 does not
have dmabuf implemented.

Or they can just send patches to add dmabuf to i915.  Clearly the kernel 
module supports it and it wouldn't take much to add it to the dri driver.



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tapani Pälli]

On 03/22/2018 01:01 PM, Emil Velikov wrote:

Hi guys,

On 22 March 2018 at 08:23, Tomasz Figa  wrote:

Hi Chih-Wei,

On Thu, Feb 22, 2018 at 2:53 PM, Chih-Wei Huang  wrote:

2018-02-21 3:03 GMT+08:00 Rob Herring :


Perhaps worth revisiting. Given we've failed to progress at all since
then may change opinions some. We already have to handle multiple
opens share the same pipe_screen, so I don't think reusing the fd buys
us anything.

Maybe we're close to the point of removing the flink name support too.
The android-x86 folks have been working to get dma-bufs working.
Chih-Wei, any comments on this?


Ah! Sorry. I didn't catch or understand the details.
Did you mean the attempts to use drm_hwcomposer
in android-x86?
My understanding so far is most x86 GPUs won't work
except some very limited models.
It's due to kernel driver issues which may never be solved.
So we can't drop the flink name support.
Please correct me if I am wrong.


Could you elaborate a bit more on those GPUs that won't work and
corresponding driver issues? We're running cros_gralloc on Intel and
AMD GPUs, with DMA-buf and render-node only setup and we haven't seen
any problems.


May I suggest an alternative to, diving into details of the issues
observed by the Android-x86 team.

Namely: Tomasz, please send a patch ripping all the flink stuff, thus
Chih-Wei and others can test it.

Ideally it'll be a quick and easy "works for me" ;-)


This has been discussed before and IIRC one reason is that i915 does not 
have dmabuf implemented.


Overall there seems to be more dmabuf users than flink users. Therefore 
it would make sense to default to dmabuf and flink users should carry 
patches instead.


// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Emil Velikov]

Hi guys,

On 22 March 2018 at 08:23, Tomasz Figa  wrote:
> Hi Chih-Wei,
>
> On Thu, Feb 22, 2018 at 2:53 PM, Chih-Wei Huang  wrote:
>> 2018-02-21 3:03 GMT+08:00 Rob Herring :
>>>
>>> Perhaps worth revisiting. Given we've failed to progress at all since
>>> then may change opinions some. We already have to handle multiple
>>> opens share the same pipe_screen, so I don't think reusing the fd buys
>>> us anything.
>>>
>>> Maybe we're close to the point of removing the flink name support too.
>>> The android-x86 folks have been working to get dma-bufs working.
>>> Chih-Wei, any comments on this?
>>
>> Ah! Sorry. I didn't catch or understand the details.
>> Did you mean the attempts to use drm_hwcomposer
>> in android-x86?
>> My understanding so far is most x86 GPUs won't work
>> except some very limited models.
>> It's due to kernel driver issues which may never be solved.
>> So we can't drop the flink name support.
>> Please correct me if I am wrong.
>
> Could you elaborate a bit more on those GPUs that won't work and
> corresponding driver issues? We're running cros_gralloc on Intel and
> AMD GPUs, with DMA-buf and render-node only setup and we haven't seen
> any problems.
>
May I suggest an alternative to, diving into details of the issues
observed by the Android-x86 team.

Namely: Tomasz, please send a patch ripping all the flink stuff, thus
Chih-Wei and others can test it.

Ideally it'll be a quick and easy "works for me" ;-)

HTH
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

Hi Chih-Wei,

On Thu, Feb 22, 2018 at 2:53 PM, Chih-Wei Huang  wrote:
> 2018-02-21 3:03 GMT+08:00 Rob Herring :
>>
>> Perhaps worth revisiting. Given we've failed to progress at all since
>> then may change opinions some. We already have to handle multiple
>> opens share the same pipe_screen, so I don't think reusing the fd buys
>> us anything.
>>
>> Maybe we're close to the point of removing the flink name support too.
>> The android-x86 folks have been working to get dma-bufs working.
>> Chih-Wei, any comments on this?
>
> Ah! Sorry. I didn't catch or understand the details.
> Did you mean the attempts to use drm_hwcomposer
> in android-x86?
> My understanding so far is most x86 GPUs won't work
> except some very limited models.
> It's due to kernel driver issues which may never be solved.
> So we can't drop the flink name support.
> Please correct me if I am wrong.

Could you elaborate a bit more on those GPUs that won't work and
corresponding driver issues? We're running cros_gralloc on Intel and
AMD GPUs, with DMA-buf and render-node only setup and we haven't seen
any problems.

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Emil Velikov]

On 22 February 2018 at 05:57, Tomasz Figa  wrote:
> On Thu, Feb 22, 2018 at 7:23 AM, Rob Herring  wrote:
>>
>> On Wed, Feb 21, 2018 at 1:22 PM, Emil Velikov  
>> wrote:
>> > On 21 February 2018 at 18:50, Rob Herring  wrote:
>> >> On Wed, Feb 21, 2018 at 10:01 AM, Emil Velikov  
>> >> wrote:
>> >>> Hi all,
>> >>>
>> >>> Pardon for dropping in late. I think you've got nearly everything
>> >>> settled down, just sharing a couple of ideas.
>> >>>
>> >>> On 21 February 2018 at 04:19, Tomasz Figa  wrote:
>>  On Wed, Feb 21, 2018 at 4:03 AM, Rob Herring  wrote:
>> > On Tue, Feb 20, 2018 at 4:26 AM, Tomasz Figa  
>> > wrote:
>> >>>
>>  It is actually incorrect to have the same device FD used for different
>>  screens, if PRIME is used, due to GEM namespace issues, e.g.
>>  - screen 0: drmPrimeFdToHandle(buffer A) -> 1,
>>  - screen 1: drmPrimeFdToHandle(buffer A) -> 1 (same handle in same
>>  namespace, due to semantics of PRIME import and same device FD used),
>>  - screen 0: GEM_CLOSE(handle = 1),
>>  - screen 1 still thinks handle 1 is valid...
>> 
>>  So this only works for control nodes using flink only. (Or if you
>>  always use libdrm_* for BO management and your particular
>>  implementation does its own GEM handle reference counting, but that's
>>  not guaranted.)
>> 
>> >>> Had a sneaky feeling that that != 1 will be returned for screen 1's
>> >>> drmPrimeFdToHandle call.
>> >>> Regardless, moving to DRI3/dmabuf-only setups is the [really] long
>> >>> term plan, I think.
>> >>> I don't know if we can achieve it outside of CrOS - with all the
>> >>> distros building and shipping things in subtly different manner.
>> >>
>> >> It's already possible for Android.
>> >>
>> > Great. Feel free to join me in pushing distributions forward ;-)
>>
>> I'll trade you Android for any thing else. :)
>>
>>
>> > How would that work if you support different GPUs in one image?
>> 
>>  It wouldn't and that's why I prefer probing available devices.
>> 
>>  For Chrome OS, we don't include GPU bits in system image, but rather
>>  have vendor images built separately for each board (although sharing
>>  any possible contents across board family, chipset, GPU type and so
>>  on), so we can have a custom .rc script (which sets a property) as
>>  well. It would even be possible to have paths hardcoded, but that
>>  would be prone to probe ordering issues which, with software fallback,
>>  could be actually not obvious to notice, and so we'd rather not go
>>  this route.
>> 
>>  So I'd agree here that we should revisit probing.
>> 
>> >>> As you pointed out the fallback is not a good idea.
>> >>
>> >> Software fallback is a desired feature. Basing it on the path is the bad 
>> >> idea.
>> >>
>> > Even considering that a) the transition is not obvious and b) it will
>> > cause serve performance degradation?
>> > For development and prototyping purposes it's great. For production
>> > ... very few users will like the abysmal performance :-\
>>
>> Better slow than not booting. There are also usecases such as running
>> in the cloud for running tests where performance is not too important.
>> AOSP has "devices" now that build images for GCE. CrOS does something
>> similar from what Tomasz has said.
>>
>> Software rendering should be the easy case, but some reason is not.
>
> I think software fallback can be done reasonably right. Perhaps it
> could kick in by default only if there is no matching hardware driver?
> If there is a matching driver, but initialization fails, perhaps it
> could make sense to boil out.
>
An explicit fallback or using SW on cloud-like services is perfectly reasonable.
FWIW there is an EGL API for explicit device selection, Mesa
implementation is WIP tough.

>>
>> >>> Plus, as the drm
>> >>> node vary, one can use an Android property and match it to the info.
>> >>> from drmGetDevice2.
>> >>> Say the HW bus location, (sub)device PCIID, other.
>> >>
>> >> That generally doesn't work for non-x86.
>> >>
>> > Errr... wrong. If PCI bus does not exist (some arm boards to have them
>> > though), one can use usb, platform or host1x specifics.
>> > If those are not enough, suggestions are welcome.
>>
>> How does this work across platforms? Feels like just punting the
>> problem to someone else. Currently it's in gralloc, so let's move it
>> to init to set a property? Also, I'm not sure that setting a property
>> is going to work in the future with Treble.
>>
>> But suppose we do make it a property. Does it really matter that the
>> property be a h/w device id and not be the device path? Whatever sets
>> the property can figure out the path based on bus info or whatever
>> logic it wants. And then for my simple uses, that logic can just be

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Chih-Wei Huang]

2018-02-21 3:03 GMT+08:00 Rob Herring :
>
> Perhaps worth revisiting. Given we've failed to progress at all since
> then may change opinions some. We already have to handle multiple
> opens share the same pipe_screen, so I don't think reusing the fd buys
> us anything.
>
> Maybe we're close to the point of removing the flink name support too.
> The android-x86 folks have been working to get dma-bufs working.
> Chih-Wei, any comments on this?

Ah! Sorry. I didn't catch or understand the details.
Did you mean the attempts to use drm_hwcomposer
in android-x86?
My understanding so far is most x86 GPUs won't work
except some very limited models.
It's due to kernel driver issues which may never be solved.
So we can't drop the flink name support.
Please correct me if I am wrong.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

On Thu, Feb 22, 2018 at 7:23 AM, Rob Herring  wrote:
>
> On Wed, Feb 21, 2018 at 1:22 PM, Emil Velikov  
> wrote:
> > On 21 February 2018 at 18:50, Rob Herring  wrote:
> >> On Wed, Feb 21, 2018 at 10:01 AM, Emil Velikov  
> >> wrote:
> >>> Hi all,
> >>>
> >>> Pardon for dropping in late. I think you've got nearly everything
> >>> settled down, just sharing a couple of ideas.
> >>>
> >>> On 21 February 2018 at 04:19, Tomasz Figa  wrote:
>  On Wed, Feb 21, 2018 at 4:03 AM, Rob Herring  wrote:
> > On Tue, Feb 20, 2018 at 4:26 AM, Tomasz Figa  wrote:
> >>>
>  It is actually incorrect to have the same device FD used for different
>  screens, if PRIME is used, due to GEM namespace issues, e.g.
>  - screen 0: drmPrimeFdToHandle(buffer A) -> 1,
>  - screen 1: drmPrimeFdToHandle(buffer A) -> 1 (same handle in same
>  namespace, due to semantics of PRIME import and same device FD used),
>  - screen 0: GEM_CLOSE(handle = 1),
>  - screen 1 still thinks handle 1 is valid...
> 
>  So this only works for control nodes using flink only. (Or if you
>  always use libdrm_* for BO management and your particular
>  implementation does its own GEM handle reference counting, but that's
>  not guaranted.)
> 
> >>> Had a sneaky feeling that that != 1 will be returned for screen 1's
> >>> drmPrimeFdToHandle call.
> >>> Regardless, moving to DRI3/dmabuf-only setups is the [really] long
> >>> term plan, I think.
> >>> I don't know if we can achieve it outside of CrOS - with all the
> >>> distros building and shipping things in subtly different manner.
> >>
> >> It's already possible for Android.
> >>
> > Great. Feel free to join me in pushing distributions forward ;-)
>
> I'll trade you Android for any thing else. :)
>
>
> > How would that work if you support different GPUs in one image?
> 
>  It wouldn't and that's why I prefer probing available devices.
> 
>  For Chrome OS, we don't include GPU bits in system image, but rather
>  have vendor images built separately for each board (although sharing
>  any possible contents across board family, chipset, GPU type and so
>  on), so we can have a custom .rc script (which sets a property) as
>  well. It would even be possible to have paths hardcoded, but that
>  would be prone to probe ordering issues which, with software fallback,
>  could be actually not obvious to notice, and so we'd rather not go
>  this route.
> 
>  So I'd agree here that we should revisit probing.
> 
> >>> As you pointed out the fallback is not a good idea.
> >>
> >> Software fallback is a desired feature. Basing it on the path is the bad 
> >> idea.
> >>
> > Even considering that a) the transition is not obvious and b) it will
> > cause serve performance degradation?
> > For development and prototyping purposes it's great. For production
> > ... very few users will like the abysmal performance :-\
>
> Better slow than not booting. There are also usecases such as running
> in the cloud for running tests where performance is not too important.
> AOSP has "devices" now that build images for GCE. CrOS does something
> similar from what Tomasz has said.
>
> Software rendering should be the easy case, but some reason is not.

I think software fallback can be done reasonably right. Perhaps it
could kick in by default only if there is no matching hardware driver?
If there is a matching driver, but initialization fails, perhaps it
could make sense to boil out.

>
> >>> Plus, as the drm
> >>> node vary, one can use an Android property and match it to the info.
> >>> from drmGetDevice2.
> >>> Say the HW bus location, (sub)device PCIID, other.
> >>
> >> That generally doesn't work for non-x86.
> >>
> > Errr... wrong. If PCI bus does not exist (some arm boards to have them
> > though), one can use usb, platform or host1x specifics.
> > If those are not enough, suggestions are welcome.
>
> How does this work across platforms? Feels like just punting the
> problem to someone else. Currently it's in gralloc, so let's move it
> to init to set a property? Also, I'm not sure that setting a property
> is going to work in the future with Treble.
>
> But suppose we do make it a property. Does it really matter that the
> property be a h/w device id and not be the device path? Whatever sets
> the property can figure out the path based on bus info or whatever
> logic it wants. And then for my simple uses, that logic can just be
> hardcoded (as we have today).

Well, we could have property that forces the probing logic to only
match requested driver or bus ID. If no property is set, it could use
the first one that matches any driver, which should cover 90% of the
systems.

Best regards,
Tomasz
___
mesa-dev mailing list

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Rob Herring]

On Wed, Feb 21, 2018 at 1:22 PM, Emil Velikov  wrote:
> On 21 February 2018 at 18:50, Rob Herring  wrote:
>> On Wed, Feb 21, 2018 at 10:01 AM, Emil Velikov  
>> wrote:
>>> Hi all,
>>>
>>> Pardon for dropping in late. I think you've got nearly everything
>>> settled down, just sharing a couple of ideas.
>>>
>>> On 21 February 2018 at 04:19, Tomasz Figa  wrote:
 On Wed, Feb 21, 2018 at 4:03 AM, Rob Herring  wrote:
> On Tue, Feb 20, 2018 at 4:26 AM, Tomasz Figa  wrote:
>>>
 It is actually incorrect to have the same device FD used for different
 screens, if PRIME is used, due to GEM namespace issues, e.g.
 - screen 0: drmPrimeFdToHandle(buffer A) -> 1,
 - screen 1: drmPrimeFdToHandle(buffer A) -> 1 (same handle in same
 namespace, due to semantics of PRIME import and same device FD used),
 - screen 0: GEM_CLOSE(handle = 1),
 - screen 1 still thinks handle 1 is valid...

 So this only works for control nodes using flink only. (Or if you
 always use libdrm_* for BO management and your particular
 implementation does its own GEM handle reference counting, but that's
 not guaranted.)

>>> Had a sneaky feeling that that != 1 will be returned for screen 1's
>>> drmPrimeFdToHandle call.
>>> Regardless, moving to DRI3/dmabuf-only setups is the [really] long
>>> term plan, I think.
>>> I don't know if we can achieve it outside of CrOS - with all the
>>> distros building and shipping things in subtly different manner.
>>
>> It's already possible for Android.
>>
> Great. Feel free to join me in pushing distributions forward ;-)

I'll trade you Android for any thing else. :)


> How would that work if you support different GPUs in one image?

 It wouldn't and that's why I prefer probing available devices.

 For Chrome OS, we don't include GPU bits in system image, but rather
 have vendor images built separately for each board (although sharing
 any possible contents across board family, chipset, GPU type and so
 on), so we can have a custom .rc script (which sets a property) as
 well. It would even be possible to have paths hardcoded, but that
 would be prone to probe ordering issues which, with software fallback,
 could be actually not obvious to notice, and so we'd rather not go
 this route.

 So I'd agree here that we should revisit probing.

>>> As you pointed out the fallback is not a good idea.
>>
>> Software fallback is a desired feature. Basing it on the path is the bad 
>> idea.
>>
> Even considering that a) the transition is not obvious and b) it will
> cause serve performance degradation?
> For development and prototyping purposes it's great. For production
> ... very few users will like the abysmal performance :-\

Better slow than not booting. There are also usecases such as running
in the cloud for running tests where performance is not too important.
AOSP has "devices" now that build images for GCE. CrOS does something
similar from what Tomasz has said.

Software rendering should be the easy case, but some reason is not.

>>> Plus, as the drm
>>> node vary, one can use an Android property and match it to the info.
>>> from drmGetDevice2.
>>> Say the HW bus location, (sub)device PCIID, other.
>>
>> That generally doesn't work for non-x86.
>>
> Errr... wrong. If PCI bus does not exist (some arm boards to have them
> though), one can use usb, platform or host1x specifics.
> If those are not enough, suggestions are welcome.

How does this work across platforms? Feels like just punting the
problem to someone else. Currently it's in gralloc, so let's move it
to init to set a property? Also, I'm not sure that setting a property
is going to work in the future with Treble.

But suppose we do make it a property. Does it really matter that the
property be a h/w device id and not be the device path? Whatever sets
the property can figure out the path based on bus info or whatever
logic it wants. And then for my simple uses, that logic can just be
hardcoded (as we have today).

Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Emil Velikov]

On 21 February 2018 at 18:50, Rob Herring  wrote:
> On Wed, Feb 21, 2018 at 10:01 AM, Emil Velikov  
> wrote:
>> Hi all,
>>
>> Pardon for dropping in late. I think you've got nearly everything
>> settled down, just sharing a couple of ideas.
>>
>> On 21 February 2018 at 04:19, Tomasz Figa  wrote:
>>> On Wed, Feb 21, 2018 at 4:03 AM, Rob Herring  wrote:
 On Tue, Feb 20, 2018 at 4:26 AM, Tomasz Figa  wrote:
>>
>>> It is actually incorrect to have the same device FD used for different
>>> screens, if PRIME is used, due to GEM namespace issues, e.g.
>>> - screen 0: drmPrimeFdToHandle(buffer A) -> 1,
>>> - screen 1: drmPrimeFdToHandle(buffer A) -> 1 (same handle in same
>>> namespace, due to semantics of PRIME import and same device FD used),
>>> - screen 0: GEM_CLOSE(handle = 1),
>>> - screen 1 still thinks handle 1 is valid...
>>>
>>> So this only works for control nodes using flink only. (Or if you
>>> always use libdrm_* for BO management and your particular
>>> implementation does its own GEM handle reference counting, but that's
>>> not guaranted.)
>>>
>> Had a sneaky feeling that that != 1 will be returned for screen 1's
>> drmPrimeFdToHandle call.
>> Regardless, moving to DRI3/dmabuf-only setups is the [really] long
>> term plan, I think.
>> I don't know if we can achieve it outside of CrOS - with all the
>> distros building and shipping things in subtly different manner.
>
> It's already possible for Android.
>
Great. Feel free to join me in pushing distributions forward ;-)

 How would that work if you support different GPUs in one image?
>>>
>>> It wouldn't and that's why I prefer probing available devices.
>>>
>>> For Chrome OS, we don't include GPU bits in system image, but rather
>>> have vendor images built separately for each board (although sharing
>>> any possible contents across board family, chipset, GPU type and so
>>> on), so we can have a custom .rc script (which sets a property) as
>>> well. It would even be possible to have paths hardcoded, but that
>>> would be prone to probe ordering issues which, with software fallback,
>>> could be actually not obvious to notice, and so we'd rather not go
>>> this route.
>>>
>>> So I'd agree here that we should revisit probing.
>>>
>> As you pointed out the fallback is not a good idea.
>
> Software fallback is a desired feature. Basing it on the path is the bad idea.
>
Even considering that a) the transition is not obvious and b) it will
cause serve performance degradation?
For development and prototyping purposes it's great. For production
... very few users will like the abysmal performance :-\

>> Plus, as the drm
>> node vary, one can use an Android property and match it to the info.
>> from drmGetDevice2.
>> Say the HW bus location, (sub)device PCIID, other.
>
> That generally doesn't work for non-x86.
>
Errr... wrong. If PCI bus does not exist (some arm boards to have them
though), one can use usb, platform or host1x specifics.
If those are not enough, suggestions are welcome.

https://cgit.freedesktop.org/mesa/drm/tree/xf86drm.h#n836

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Rob Herring]

On Wed, Feb 21, 2018 at 10:01 AM, Emil Velikov  wrote:
> Hi all,
>
> Pardon for dropping in late. I think you've got nearly everything
> settled down, just sharing a couple of ideas.
>
> On 21 February 2018 at 04:19, Tomasz Figa  wrote:
>> On Wed, Feb 21, 2018 at 4:03 AM, Rob Herring  wrote:
>>> On Tue, Feb 20, 2018 at 4:26 AM, Tomasz Figa  wrote:
>
>> It is actually incorrect to have the same device FD used for different
>> screens, if PRIME is used, due to GEM namespace issues, e.g.
>> - screen 0: drmPrimeFdToHandle(buffer A) -> 1,
>> - screen 1: drmPrimeFdToHandle(buffer A) -> 1 (same handle in same
>> namespace, due to semantics of PRIME import and same device FD used),
>> - screen 0: GEM_CLOSE(handle = 1),
>> - screen 1 still thinks handle 1 is valid...
>>
>> So this only works for control nodes using flink only. (Or if you
>> always use libdrm_* for BO management and your particular
>> implementation does its own GEM handle reference counting, but that's
>> not guaranted.)
>>
> Had a sneaky feeling that that != 1 will be returned for screen 1's
> drmPrimeFdToHandle call.
> Regardless, moving to DRI3/dmabuf-only setups is the [really] long
> term plan, I think.
> I don't know if we can achieve it outside of CrOS - with all the
> distros building and shipping things in subtly different manner.

It's already possible for Android.

>>> How would that work if you support different GPUs in one image?
>>
>> It wouldn't and that's why I prefer probing available devices.
>>
>> For Chrome OS, we don't include GPU bits in system image, but rather
>> have vendor images built separately for each board (although sharing
>> any possible contents across board family, chipset, GPU type and so
>> on), so we can have a custom .rc script (which sets a property) as
>> well. It would even be possible to have paths hardcoded, but that
>> would be prone to probe ordering issues which, with software fallback,
>> could be actually not obvious to notice, and so we'd rather not go
>> this route.
>>
>> So I'd agree here that we should revisit probing.
>>
> As you pointed out the fallback is not a good idea.

Software fallback is a desired feature. Basing it on the path is the bad idea.

> Plus, as the drm
> node vary, one can use an Android property and match it to the info.
> from drmGetDevice2.
> Say the HW bus location, (sub)device PCIID, other.

That generally doesn't work for non-x86.

>
> It should also help as one starts shipping devices with multiple GPUs,
> at some point in the future ;-)

Can we solve the simple cases first?

Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Emil Velikov]

Hi all,

Pardon for dropping in late. I think you've got nearly everything
settled down, just sharing a couple of ideas.

On 21 February 2018 at 04:19, Tomasz Figa  wrote:
> On Wed, Feb 21, 2018 at 4:03 AM, Rob Herring  wrote:
>> On Tue, Feb 20, 2018 at 4:26 AM, Tomasz Figa  wrote:

> It is actually incorrect to have the same device FD used for different
> screens, if PRIME is used, due to GEM namespace issues, e.g.
> - screen 0: drmPrimeFdToHandle(buffer A) -> 1,
> - screen 1: drmPrimeFdToHandle(buffer A) -> 1 (same handle in same
> namespace, due to semantics of PRIME import and same device FD used),
> - screen 0: GEM_CLOSE(handle = 1),
> - screen 1 still thinks handle 1 is valid...
>
> So this only works for control nodes using flink only. (Or if you
> always use libdrm_* for BO management and your particular
> implementation does its own GEM handle reference counting, but that's
> not guaranted.)
>
Had a sneaky feeling that that != 1 will be returned for screen 1's
drmPrimeFdToHandle call.
Regardless, moving to DRI3/dmabuf-only setups is the [really] long
term plan, I think.
I don't know if we can achieve it outside of CrOS - with all the
distros building and shipping things in subtly different manner.


>> How would that work if you support different GPUs in one image?
>
> It wouldn't and that's why I prefer probing available devices.
>
> For Chrome OS, we don't include GPU bits in system image, but rather
> have vendor images built separately for each board (although sharing
> any possible contents across board family, chipset, GPU type and so
> on), so we can have a custom .rc script (which sets a property) as
> well. It would even be possible to have paths hardcoded, but that
> would be prone to probe ordering issues which, with software fallback,
> could be actually not obvious to notice, and so we'd rather not go
> this route.
>
> So I'd agree here that we should revisit probing.
>
As you pointed out the fallback is not a good idea. Plus, as the drm
node vary, one can use an Android property and match it to the info.
from drmGetDevice2.
Say the HW bus location, (sub)device PCIID, other.

It should also help as one starts shipping devices with multiple GPUs,
at some point in the future ;-)

HTH
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

On Wed, Feb 21, 2018 at 4:03 AM, Rob Herring  wrote:
> On Tue, Feb 20, 2018 at 4:26 AM, Tomasz Figa  wrote:
>> On Tue, Feb 20, 2018 at 6:51 PM, Robert Foss  
>> wrote:
>>> Hey Tomasz,
>>>
>>> On 02/20/2018 09:55 AM, Tomasz Figa wrote:

 Hi Rob,

 On Fri, Feb 16, 2018 at 11:48 PM, Tomasz Figa  wrote:
>
> On Fri, Feb 16, 2018 at 11:33 PM, Robert Foss 
> wrote:
>>
>> Hey Tomasz,
>>
>>
>> On 02/16/2018 05:10 AM, Tomaszzz Figa wrote:
>>>
>>>
>>> On Fri, Feb 9, 2018 at 11:06 PM, Rob Herring  wrote:


 On Fri, Feb 9, 2018 at 3:58 AM, Tomasz Figa >>
>>> On Fri, Feb 2, 2018 at 2:01 AM, Tomasz Figa 
>>>
>>> wrote:


 Hi Rob,

 On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss
  wrote:
>>
>>
>> uint32_t (*get_fd)(buffer_handle_t handle, uint32_t
>> plane);
>> uint64_t (*get_modifier)(buffer_handle_t handle,
>> uint32_t
>> plane);
>> uint32_t (*get_offsets)(buffer_handle_t handle,
>> uint32_t
>> plane);
>> uint32_t (*get_stride)(buffer_handle_t handle,
>> uint32_t
>> plane);
>> ...
>> } gralloc_funcs_t;
>>
>>
>>
>>
>> These ones? >
>> Yeah, if we could retrieve such function pointer struct using
>> perform
>> or any equivalent (like the implementation-specific methods in
>> gralloc1, but not sure if that's going to be used in practice
>> anywhere), it could work for us.
>
>
>
>
> So this is where you and Rob Herring lose me, I don't think I
> understand
> quite how the gralloc1 call would be used, and how it would tie
> into
> this
> handle struct. I think I could do with some guidance on this.



 This would be very similar to gralloc0 perform call. gralloc1
 implementations need to provide getFunction() callback [1], which
 returns a pointer to given function. The list of standard
 functions
 is
 defined in the gralloc1.h header [2], but we could take some
 random
 big number and use it for our function that fills in provided
 gralloc_funcs_t struct with necessary pointers.

 [1]

 https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
 [2]

 https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134
>>>
>>>
>>>
>>> This is a deadend because it won't work with a HIDL based
>>> implementation (aka gralloc 2.0). You can't set function pointers
>>> (or
>>> any pointers) because gralloc runs in a different process. Yes,
>>> currently gralloc is a pass-thru HAL, but AIUI that will go away.
>>
>>
>>
>> Part of it. I can't see IMapper being implemented by a separate
>> process. You can't map a buffer into one process from another
>> process.
>>
>> But anyway, it's a good point, thanks, I almost forgot about its
>> existence. I'll do further investigation.
>
>
>
> Okay, so IMapper indeed breaks the approach I suggested. I'm not sure
> at the moment what we could do about it. (The idea of a dynamic
> library of a pre-defined name, exporting functions we specify, might
> still work, though.)
>
> Note that the DRM_GRALLOC_GET_FD used currently by Mesa will also be
> impossible to implement with IAllocator/IMapper. (Although I still
> think Mesa and Gralloc are free to have separate logic for choosing
> the DRM device to use.)



 I think the need for GET_FD goes away when the render node is used. We
 may still need the card node for s/w rendering (if I can ever get that
 working) though. Of course, if we use the vgem approach like CrOS then
 we wouldn't.
>>>
>>>
>>>
>>> Hmm, if so, then we probably wouldn't have any strict need for these
>>> function pointers anymore. We already have a makeshift format resolve
>>> in place and 

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Rob Herring]

On Tue, Feb 20, 2018 at 4:26 AM, Tomasz Figa  wrote:
> On Tue, Feb 20, 2018 at 6:51 PM, Robert Foss  
> wrote:
>> Hey Tomasz,
>>
>> On 02/20/2018 09:55 AM, Tomasz Figa wrote:
>>>
>>> Hi Rob,
>>>
>>> On Fri, Feb 16, 2018 at 11:48 PM, Tomasz Figa  wrote:

 On Fri, Feb 16, 2018 at 11:33 PM, Robert Foss 
 wrote:
>
> Hey Tomasz,
>
>
> On 02/16/2018 05:10 AM, Tomaszzz Figa wrote:
>>
>>
>> On Fri, Feb 9, 2018 at 11:06 PM, Rob Herring  wrote:
>>>
>>>
>>> On Fri, Feb 9, 2018 at 3:58 AM, Tomasz Figa >
>> On Fri, Feb 2, 2018 at 2:01 AM, Tomasz Figa 
>>
>> wrote:
>>>
>>>
>>> Hi Rob,
>>>
>>> On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss
>>>  wrote:
>
>
> uint32_t (*get_fd)(buffer_handle_t handle, uint32_t
> plane);
> uint64_t (*get_modifier)(buffer_handle_t handle,
> uint32_t
> plane);
> uint32_t (*get_offsets)(buffer_handle_t handle,
> uint32_t
> plane);
> uint32_t (*get_stride)(buffer_handle_t handle,
> uint32_t
> plane);
> ...
> } gralloc_funcs_t;
>
>
>
>
> These ones? >
> Yeah, if we could retrieve such function pointer struct using
> perform
> or any equivalent (like the implementation-specific methods in
> gralloc1, but not sure if that's going to be used in practice
> anywhere), it could work for us.




 So this is where you and Rob Herring lose me, I don't think I
 understand
 quite how the gralloc1 call would be used, and how it would tie
 into
 this
 handle struct. I think I could do with some guidance on this.
>>>
>>>
>>>
>>> This would be very similar to gralloc0 perform call. gralloc1
>>> implementations need to provide getFunction() callback [1], which
>>> returns a pointer to given function. The list of standard
>>> functions
>>> is
>>> defined in the gralloc1.h header [2], but we could take some
>>> random
>>> big number and use it for our function that fills in provided
>>> gralloc_funcs_t struct with necessary pointers.
>>>
>>> [1]
>>>
>>> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
>>> [2]
>>>
>>> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134
>>
>>
>>
>> This is a deadend because it won't work with a HIDL based
>> implementation (aka gralloc 2.0). You can't set function pointers
>> (or
>> any pointers) because gralloc runs in a different process. Yes,
>> currently gralloc is a pass-thru HAL, but AIUI that will go away.
>
>
>
> Part of it. I can't see IMapper being implemented by a separate
> process. You can't map a buffer into one process from another
> process.
>
> But anyway, it's a good point, thanks, I almost forgot about its
> existence. I'll do further investigation.



 Okay, so IMapper indeed breaks the approach I suggested. I'm not sure
 at the moment what we could do about it. (The idea of a dynamic
 library of a pre-defined name, exporting functions we specify, might
 still work, though.)

 Note that the DRM_GRALLOC_GET_FD used currently by Mesa will also be
 impossible to implement with IAllocator/IMapper. (Although I still
 think Mesa and Gralloc are free to have separate logic for choosing
 the DRM device to use.)
>>>
>>>
>>>
>>> I think the need for GET_FD goes away when the render node is used. We
>>> may still need the card node for s/w rendering (if I can ever get that
>>> working) though. Of course, if we use the vgem approach like CrOS then
>>> we wouldn't.
>>
>>
>>
>> Hmm, if so, then we probably wouldn't have any strict need for these
>> function pointers anymore. We already have a makeshift format resolve
>> in place and the only missing bits that we still need to patch up
>> downstream are removing GET_FD, dropping drm_gralloc.h and adding a
>> fallback to kms_swrast if hw driver loading fails.
>
>

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

On Tue, Feb 20, 2018 at 6:51 PM, Robert Foss  wrote:
> Hey Tomasz,
>
> On 02/20/2018 09:55 AM, Tomasz Figa wrote:
>>
>> Hi Rob,
>>
>> On Fri, Feb 16, 2018 at 11:48 PM, Tomasz Figa  wrote:
>>>
>>> On Fri, Feb 16, 2018 at 11:33 PM, Robert Foss 
>>> wrote:

 Hey Tomasz,


 On 02/16/2018 05:10 AM, Tomaszzz Figa wrote:
>
>
> On Fri, Feb 9, 2018 at 11:06 PM, Rob Herring  wrote:
>>
>>
>> On Fri, Feb 9, 2018 at 3:58 AM, Tomasz Figa 
> On Fri, Feb 2, 2018 at 2:01 AM, Tomasz Figa 
>
> wrote:
>>
>>
>> Hi Rob,
>>
>> On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss
>>  wrote:


 uint32_t (*get_fd)(buffer_handle_t handle, uint32_t
 plane);
 uint64_t (*get_modifier)(buffer_handle_t handle,
 uint32_t
 plane);
 uint32_t (*get_offsets)(buffer_handle_t handle,
 uint32_t
 plane);
 uint32_t (*get_stride)(buffer_handle_t handle,
 uint32_t
 plane);
 ...
 } gralloc_funcs_t;




 These ones? >
 Yeah, if we could retrieve such function pointer struct using
 perform
 or any equivalent (like the implementation-specific methods in
 gralloc1, but not sure if that's going to be used in practice
 anywhere), it could work for us.
>>>
>>>
>>>
>>>
>>> So this is where you and Rob Herring lose me, I don't think I
>>> understand
>>> quite how the gralloc1 call would be used, and how it would tie
>>> into
>>> this
>>> handle struct. I think I could do with some guidance on this.
>>
>>
>>
>> This would be very similar to gralloc0 perform call. gralloc1
>> implementations need to provide getFunction() callback [1], which
>> returns a pointer to given function. The list of standard
>> functions
>> is
>> defined in the gralloc1.h header [2], but we could take some
>> random
>> big number and use it for our function that fills in provided
>> gralloc_funcs_t struct with necessary pointers.
>>
>> [1]
>>
>> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
>> [2]
>>
>> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134
>
>
>
> This is a deadend because it won't work with a HIDL based
> implementation (aka gralloc 2.0). You can't set function pointers
> (or
> any pointers) because gralloc runs in a different process. Yes,
> currently gralloc is a pass-thru HAL, but AIUI that will go away.



 Part of it. I can't see IMapper being implemented by a separate
 process. You can't map a buffer into one process from another
 process.

 But anyway, it's a good point, thanks, I almost forgot about its
 existence. I'll do further investigation.
>>>
>>>
>>>
>>> Okay, so IMapper indeed breaks the approach I suggested. I'm not sure
>>> at the moment what we could do about it. (The idea of a dynamic
>>> library of a pre-defined name, exporting functions we specify, might
>>> still work, though.)
>>>
>>> Note that the DRM_GRALLOC_GET_FD used currently by Mesa will also be
>>> impossible to implement with IAllocator/IMapper. (Although I still
>>> think Mesa and Gralloc are free to have separate logic for choosing
>>> the DRM device to use.)
>>
>>
>>
>> I think the need for GET_FD goes away when the render node is used. We
>> may still need the card node for s/w rendering (if I can ever get that
>> working) though. Of course, if we use the vgem approach like CrOS then
>> we wouldn't.
>
>
>
> Hmm, if so, then we probably wouldn't have any strict need for these
> function pointers anymore. We already have a makeshift format resolve
> in place and the only missing bits that we still need to patch up
> downstream are removing GET_FD, dropping drm_gralloc.h and adding a
> fallback to kms_swrast if hw driver loading fails.



 So this discussion is slightly unrelated, but it is where me looking at
 this
 started.

 So I've got a kms_swrast fallback series[1], that I've been wanting to
 test
 before 

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Robert Foss]

Hey Tomasz,

On 02/20/2018 09:55 AM, Tomasz Figa wrote:

Hi Rob,

On Fri, Feb 16, 2018 at 11:48 PM, Tomasz Figa  wrote:

On Fri, Feb 16, 2018 at 11:33 PM, Robert Foss  wrote:

Hey Tomasz,


On 02/16/2018 05:10 AM, Tomaszzz Figa wrote:


On Fri, Feb 9, 2018 at 11:06 PM, Rob Herring  wrote:


On Fri, Feb 9, 2018 at 3:58 AM, Tomasz Figa 
wrote:


Hi Rob,

On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss
 wrote:


uint32_t (*get_fd)(buffer_handle_t handle, uint32_t
plane);
uint64_t (*get_modifier)(buffer_handle_t handle,
uint32_t
plane);
uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t
plane);
uint32_t (*get_stride)(buffer_handle_t handle, uint32_t
plane);
...
} gralloc_funcs_t;




These ones? >
Yeah, if we could retrieve such function pointer struct using
perform
or any equivalent (like the implementation-specific methods in
gralloc1, but not sure if that's going to be used in practice
anywhere), it could work for us.




So this is where you and Rob Herring lose me, I don't think I
understand
quite how the gralloc1 call would be used, and how it would tie into
this
handle struct. I think I could do with some guidance on this.



This would be very similar to gralloc0 perform call. gralloc1
implementations need to provide getFunction() callback [1], which
returns a pointer to given function. The list of standard functions
is
defined in the gralloc1.h header [2], but we could take some random
big number and use it for our function that fills in provided
gralloc_funcs_t struct with necessary pointers.

[1]
https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
[2]
https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134



This is a deadend because it won't work with a HIDL based
implementation (aka gralloc 2.0). You can't set function pointers (or
any pointers) because gralloc runs in a different process. Yes,
currently gralloc is a pass-thru HAL, but AIUI that will go away.



Part of it. I can't see IMapper being implemented by a separate
process. You can't map a buffer into one process from another process.

But anyway, it's a good point, thanks, I almost forgot about its
existence. I'll do further investigation.



Okay, so IMapper indeed breaks the approach I suggested. I'm not sure
at the moment what we could do about it. (The idea of a dynamic
library of a pre-defined name, exporting functions we specify, might
still work, though.)

Note that the DRM_GRALLOC_GET_FD used currently by Mesa will also be
impossible to implement with IAllocator/IMapper. (Although I still
think Mesa and Gralloc are free to have separate logic for choosing
the DRM device to use.)



I think the need for GET_FD goes away when the render node is used. We
may still need the card node for s/w rendering (if I can ever get that
working) though. Of course, if we use the vgem approach like CrOS then
we wouldn't.



Hmm, if so, then we probably wouldn't have any strict need for these
function pointers anymore. We already have a makeshift format resolve
in place and the only missing bits that we still need to patch up
downstream are removing GET_FD, dropping drm_gralloc.h and adding a
fallback to kms_swrast if hw driver loading fails.



So this discussion is slightly unrelated, but it is where me looking at this
started.

So I've got a kms_swrast fallback series[1], that I've been wanting to test
before trying to push upstream, but haven't been able to run it in the
Android environment and the arc++ + chromiumos has also been problematic for
various reasons (which are being looked into).

Tomasz: If you're interested in testing the series, it would be helpful.
Hopefully testing is everything that needed for upstreaming.

[1] https://gitlab.collabora.com/robertfoss/mesa/commits/kms_swrast




I checked the branch, but it isn't possible to test it directly with
Chrome OS, since we do not provide DRM_GRALLOC_GET_FD functionality
and adding support for probing devices changes the code introduced by
your patches significantly, which kind of defeats the purpose.

I think it might make sense to actually remove the requirement for
DRM_GRALLOC_GET_FD from upstream first (as in my original attempt from
long time ago) and then add kms_swrast fallback on top of that.



I would like to look into this, is your previous DRM_GRALLOC_GET_FD removal 
attempt available somewhere?


What kind of snags did you end up hitting? Just upstream approval or technical 
ones?


Rob.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

Hi Rob,

On Fri, Feb 16, 2018 at 11:48 PM, Tomasz Figa  wrote:
> On Fri, Feb 16, 2018 at 11:33 PM, Robert Foss  
> wrote:
>> Hey Tomasz,
>>
>>
>> On 02/16/2018 05:10 AM, Tomasz Figa wrote:
>>>
>>> On Fri, Feb 9, 2018 at 11:06 PM, Rob Herring  wrote:

 On Fri, Feb 9, 2018 at 3:58 AM, Tomasz Figa  wrote:
>
> On Fri, Feb 2, 2018 at 11:51 PM, Tomasz Figa  wrote:
>>
>> On Fri, Feb 2, 2018 at 11:00 PM, Rob Herring  wrote:
>>>
>>> On Fri, Feb 2, 2018 at 2:01 AM, Tomasz Figa 
>>> wrote:

 Hi Rob,

 On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss
  wrote:
>>
>>uint32_t (*get_fd)(buffer_handle_t handle, uint32_t
>> plane);
>>uint64_t (*get_modifier)(buffer_handle_t handle,
>> uint32_t
>> plane);
>>uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t
>> plane);
>>uint32_t (*get_stride)(buffer_handle_t handle, uint32_t
>> plane);
>>...
>> } gralloc_funcs_t;
>>
>>
>>
>> These ones? >
>> Yeah, if we could retrieve such function pointer struct using
>> perform
>> or any equivalent (like the implementation-specific methods in
>> gralloc1, but not sure if that's going to be used in practice
>> anywhere), it could work for us.
>
>
>
> So this is where you and Rob Herring lose me, I don't think I
> understand
> quite how the gralloc1 call would be used, and how it would tie into
> this
> handle struct. I think I could do with some guidance on this.


 This would be very similar to gralloc0 perform call. gralloc1
 implementations need to provide getFunction() callback [1], which
 returns a pointer to given function. The list of standard functions
 is
 defined in the gralloc1.h header [2], but we could take some random
 big number and use it for our function that fills in provided
 gralloc_funcs_t struct with necessary pointers.

 [1]
 https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
 [2]
 https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134
>>>
>>>
>>> This is a deadend because it won't work with a HIDL based
>>> implementation (aka gralloc 2.0). You can't set function pointers (or
>>> any pointers) because gralloc runs in a different process. Yes,
>>> currently gralloc is a pass-thru HAL, but AIUI that will go away.
>>
>>
>> Part of it. I can't see IMapper being implemented by a separate
>> process. You can't map a buffer into one process from another process.
>>
>> But anyway, it's a good point, thanks, I almost forgot about its
>> existence. I'll do further investigation.
>
>
> Okay, so IMapper indeed breaks the approach I suggested. I'm not sure
> at the moment what we could do about it. (The idea of a dynamic
> library of a pre-defined name, exporting functions we specify, might
> still work, though.)
>
> Note that the DRM_GRALLOC_GET_FD used currently by Mesa will also be
> impossible to implement with IAllocator/IMapper. (Although I still
> think Mesa and Gralloc are free to have separate logic for choosing
> the DRM device to use.)


 I think the need for GET_FD goes away when the render node is used. We
 may still need the card node for s/w rendering (if I can ever get that
 working) though. Of course, if we use the vgem approach like CrOS then
 we wouldn't.
>>>
>>>
>>> Hmm, if so, then we probably wouldn't have any strict need for these
>>> function pointers anymore. We already have a makeshift format resolve
>>> in place and the only missing bits that we still need to patch up
>>> downstream are removing GET_FD, dropping drm_gralloc.h and adding a
>>> fallback to kms_swrast if hw driver loading fails.
>>
>>
>> So this discussion is slightly unrelated, but it is where me looking at this
>> started.
>>
>> So I've got a kms_swrast fallback series[1], that I've been wanting to test
>> before trying to push upstream, but haven't been able to run it in the
>> Android environment and the arc++ + chromiumos has also been problematic for
>> various reasons (which are being looked into).
>>
>> Tomasz: If you're interested in testing the series, it would be helpful.
>> Hopefully testing is everything that needed for upstreaming.
>>
>> [1] https://gitlab.collabora.com/robertfoss/mesa/commits/kms_swrast
>

I checked the branch, but it isn't 

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

On Fri, Feb 16, 2018 at 11:33 PM, Robert Foss  wrote:
> Hey Tomasz,
>
>
> On 02/16/2018 05:10 AM, Tomasz Figa wrote:
>>
>> On Fri, Feb 9, 2018 at 11:06 PM, Rob Herring  wrote:
>>>
>>> On Fri, Feb 9, 2018 at 3:58 AM, Tomasz Figa  wrote:

 On Fri, Feb 2, 2018 at 11:51 PM, Tomasz Figa  wrote:
>
> On Fri, Feb 2, 2018 at 11:00 PM, Rob Herring  wrote:
>>
>> On Fri, Feb 2, 2018 at 2:01 AM, Tomasz Figa 
>> wrote:
>>>
>>> Hi Rob,
>>>
>>> On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss
>>>  wrote:
>
>uint32_t (*get_fd)(buffer_handle_t handle, uint32_t
> plane);
>uint64_t (*get_modifier)(buffer_handle_t handle,
> uint32_t
> plane);
>uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t
> plane);
>uint32_t (*get_stride)(buffer_handle_t handle, uint32_t
> plane);
>...
> } gralloc_funcs_t;
>
>
>
> These ones? >
> Yeah, if we could retrieve such function pointer struct using
> perform
> or any equivalent (like the implementation-specific methods in
> gralloc1, but not sure if that's going to be used in practice
> anywhere), it could work for us.



 So this is where you and Rob Herring lose me, I don't think I
 understand
 quite how the gralloc1 call would be used, and how it would tie into
 this
 handle struct. I think I could do with some guidance on this.
>>>
>>>
>>> This would be very similar to gralloc0 perform call. gralloc1
>>> implementations need to provide getFunction() callback [1], which
>>> returns a pointer to given function. The list of standard functions
>>> is
>>> defined in the gralloc1.h header [2], but we could take some random
>>> big number and use it for our function that fills in provided
>>> gralloc_funcs_t struct with necessary pointers.
>>>
>>> [1]
>>> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
>>> [2]
>>> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134
>>
>>
>> This is a deadend because it won't work with a HIDL based
>> implementation (aka gralloc 2.0). You can't set function pointers (or
>> any pointers) because gralloc runs in a different process. Yes,
>> currently gralloc is a pass-thru HAL, but AIUI that will go away.
>
>
> Part of it. I can't see IMapper being implemented by a separate
> process. You can't map a buffer into one process from another process.
>
> But anyway, it's a good point, thanks, I almost forgot about its
> existence. I'll do further investigation.


 Okay, so IMapper indeed breaks the approach I suggested. I'm not sure
 at the moment what we could do about it. (The idea of a dynamic
 library of a pre-defined name, exporting functions we specify, might
 still work, though.)

 Note that the DRM_GRALLOC_GET_FD used currently by Mesa will also be
 impossible to implement with IAllocator/IMapper. (Although I still
 think Mesa and Gralloc are free to have separate logic for choosing
 the DRM device to use.)
>>>
>>>
>>> I think the need for GET_FD goes away when the render node is used. We
>>> may still need the card node for s/w rendering (if I can ever get that
>>> working) though. Of course, if we use the vgem approach like CrOS then
>>> we wouldn't.
>>
>>
>> Hmm, if so, then we probably wouldn't have any strict need for these
>> function pointers anymore. We already have a makeshift format resolve
>> in place and the only missing bits that we still need to patch up
>> downstream are removing GET_FD, dropping drm_gralloc.h and adding a
>> fallback to kms_swrast if hw driver loading fails.
>
>
> So this discussion is slightly unrelated, but it is where me looking at this
> started.
>
> So I've got a kms_swrast fallback series[1], that I've been wanting to test
> before trying to push upstream, but haven't been able to run it in the
> Android environment and the arc++ + chromiumos has also been problematic for
> various reasons (which are being looked into).
>
> Tomasz: If you're interested in testing the series, it would be helpful.
> Hopefully testing is everything that needed for upstreaming.
>
> [1] https://gitlab.collabora.com/robertfoss/mesa/commits/kms_swrast

+Gurchetan, who I believe is recently taking care of swrast on our side.

Sure, I'd be more than happy to test it on Monday.

Best regards,
Tomasz
___
mesa-dev mailing list

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Robert Foss]

Hey Tomasz,

On 02/16/2018 05:10 AM, Tomasz Figa wrote:

On Fri, Feb 9, 2018 at 11:06 PM, Rob Herring  wrote:

On Fri, Feb 9, 2018 at 3:58 AM, Tomasz Figa  wrote:

On Fri, Feb 2, 2018 at 11:51 PM, Tomasz Figa  wrote:

On Fri, Feb 2, 2018 at 11:00 PM, Rob Herring  wrote:

On Fri, Feb 2, 2018 at 2:01 AM, Tomasz Figa  wrote:

Hi Rob,

On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss  wrote:

   uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
   uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t
plane);
   uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
   uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
   ...
} gralloc_funcs_t;



These ones? >
Yeah, if we could retrieve such function pointer struct using perform
or any equivalent (like the implementation-specific methods in
gralloc1, but not sure if that's going to be used in practice
anywhere), it could work for us.



So this is where you and Rob Herring lose me, I don't think I understand
quite how the gralloc1 call would be used, and how it would tie into this
handle struct. I think I could do with some guidance on this.


This would be very similar to gralloc0 perform call. gralloc1
implementations need to provide getFunction() callback [1], which
returns a pointer to given function. The list of standard functions is
defined in the gralloc1.h header [2], but we could take some random
big number and use it for our function that fills in provided
gralloc_funcs_t struct with necessary pointers.

[1] 
https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
[2] 
https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134


This is a deadend because it won't work with a HIDL based
implementation (aka gralloc 2.0). You can't set function pointers (or
any pointers) because gralloc runs in a different process. Yes,
currently gralloc is a pass-thru HAL, but AIUI that will go away.


Part of it. I can't see IMapper being implemented by a separate
process. You can't map a buffer into one process from another process.

But anyway, it's a good point, thanks, I almost forgot about its
existence. I'll do further investigation.


Okay, so IMapper indeed breaks the approach I suggested. I'm not sure
at the moment what we could do about it. (The idea of a dynamic
library of a pre-defined name, exporting functions we specify, might
still work, though.)

Note that the DRM_GRALLOC_GET_FD used currently by Mesa will also be
impossible to implement with IAllocator/IMapper. (Although I still
think Mesa and Gralloc are free to have separate logic for choosing
the DRM device to use.)


I think the need for GET_FD goes away when the render node is used. We
may still need the card node for s/w rendering (if I can ever get that
working) though. Of course, if we use the vgem approach like CrOS then
we wouldn't.


Hmm, if so, then we probably wouldn't have any strict need for these
function pointers anymore. We already have a makeshift format resolve
in place and the only missing bits that we still need to patch up
downstream are removing GET_FD, dropping drm_gralloc.h and adding a
fallback to kms_swrast if hw driver loading fails.


So this discussion is slightly unrelated, but it is where me looking at this 
started.


So I've got a kms_swrast fallback series[1], that I've been wanting to test 
before trying to push upstream, but haven't been able to run it in the Android 
environment and the arc++ + chromiumos has also been problematic for various 
reasons (which are being looked into).


Tomasz: If you're interested in testing the series, it would be helpful. 
Hopefully testing is everything that needed for upstreaming.


[1] https://gitlab.collabora.com/robertfoss/mesa/commits/kms_swrast
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

On Fri, Feb 9, 2018 at 11:06 PM, Rob Herring  wrote:
> On Fri, Feb 9, 2018 at 3:58 AM, Tomasz Figa  wrote:
>> On Fri, Feb 2, 2018 at 11:51 PM, Tomasz Figa  wrote:
>>> On Fri, Feb 2, 2018 at 11:00 PM, Rob Herring  wrote:
 On Fri, Feb 2, 2018 at 2:01 AM, Tomasz Figa  wrote:
> Hi Rob,
>
> On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss  
> wrote:
>>>   uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
>>>   uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t
>>> plane);
>>>   uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t 
>>> plane);
>>>   uint32_t (*get_stride)(buffer_handle_t handle, uint32_t 
>>> plane);
>>>   ...
>>> } gralloc_funcs_t;
>>>
>>>
>>> These ones? >
>>> Yeah, if we could retrieve such function pointer struct using perform
>>> or any equivalent (like the implementation-specific methods in
>>> gralloc1, but not sure if that's going to be used in practice
>>> anywhere), it could work for us.
>>
>>
>> So this is where you and Rob Herring lose me, I don't think I understand
>> quite how the gralloc1 call would be used, and how it would tie into this
>> handle struct. I think I could do with some guidance on this.
>
> This would be very similar to gralloc0 perform call. gralloc1
> implementations need to provide getFunction() callback [1], which
> returns a pointer to given function. The list of standard functions is
> defined in the gralloc1.h header [2], but we could take some random
> big number and use it for our function that fills in provided
> gralloc_funcs_t struct with necessary pointers.
>
> [1] 
> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
> [2] 
> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134

 This is a deadend because it won't work with a HIDL based
 implementation (aka gralloc 2.0). You can't set function pointers (or
 any pointers) because gralloc runs in a different process. Yes,
 currently gralloc is a pass-thru HAL, but AIUI that will go away.
>>>
>>> Part of it. I can't see IMapper being implemented by a separate
>>> process. You can't map a buffer into one process from another process.
>>>
>>> But anyway, it's a good point, thanks, I almost forgot about its
>>> existence. I'll do further investigation.
>>
>> Okay, so IMapper indeed breaks the approach I suggested. I'm not sure
>> at the moment what we could do about it. (The idea of a dynamic
>> library of a pre-defined name, exporting functions we specify, might
>> still work, though.)
>>
>> Note that the DRM_GRALLOC_GET_FD used currently by Mesa will also be
>> impossible to implement with IAllocator/IMapper. (Although I still
>> think Mesa and Gralloc are free to have separate logic for choosing
>> the DRM device to use.)
>
> I think the need for GET_FD goes away when the render node is used. We
> may still need the card node for s/w rendering (if I can ever get that
> working) though. Of course, if we use the vgem approach like CrOS then
> we wouldn't.

Hmm, if so, then we probably wouldn't have any strict need for these
function pointers anymore. We already have a makeshift format resolve
in place and the only missing bits that we still need to patch up
downstream are removing GET_FD, dropping drm_gralloc.h and adding a
fallback to kms_swrast if hw driver loading fails.

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Rob Herring]

On Fri, Feb 9, 2018 at 3:58 AM, Tomasz Figa  wrote:
> On Fri, Feb 2, 2018 at 11:51 PM, Tomasz Figa  wrote:
>> On Fri, Feb 2, 2018 at 11:00 PM, Rob Herring  wrote:
>>> On Fri, Feb 2, 2018 at 2:01 AM, Tomasz Figa  wrote:
 Hi Rob,

 On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss  
 wrote:
>>   uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
>>   uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t
>> plane);
>>   uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t 
>> plane);
>>   uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
>>   ...
>> } gralloc_funcs_t;
>>
>>
>> These ones? >
>> Yeah, if we could retrieve such function pointer struct using perform
>> or any equivalent (like the implementation-specific methods in
>> gralloc1, but not sure if that's going to be used in practice
>> anywhere), it could work for us.
>
>
> So this is where you and Rob Herring lose me, I don't think I understand
> quite how the gralloc1 call would be used, and how it would tie into this
> handle struct. I think I could do with some guidance on this.

 This would be very similar to gralloc0 perform call. gralloc1
 implementations need to provide getFunction() callback [1], which
 returns a pointer to given function. The list of standard functions is
 defined in the gralloc1.h header [2], but we could take some random
 big number and use it for our function that fills in provided
 gralloc_funcs_t struct with necessary pointers.

 [1] 
 https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
 [2] 
 https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134
>>>
>>> This is a deadend because it won't work with a HIDL based
>>> implementation (aka gralloc 2.0). You can't set function pointers (or
>>> any pointers) because gralloc runs in a different process. Yes,
>>> currently gralloc is a pass-thru HAL, but AIUI that will go away.
>>
>> Part of it. I can't see IMapper being implemented by a separate
>> process. You can't map a buffer into one process from another process.
>>
>> But anyway, it's a good point, thanks, I almost forgot about its
>> existence. I'll do further investigation.
>
> Okay, so IMapper indeed breaks the approach I suggested. I'm not sure
> at the moment what we could do about it. (The idea of a dynamic
> library of a pre-defined name, exporting functions we specify, might
> still work, though.)
>
> Note that the DRM_GRALLOC_GET_FD used currently by Mesa will also be
> impossible to implement with IAllocator/IMapper. (Although I still
> think Mesa and Gralloc are free to have separate logic for choosing
> the DRM device to use.)

I think the need for GET_FD goes away when the render node is used. We
may still need the card node for s/w rendering (if I can ever get that
working) though. Of course, if we use the vgem approach like CrOS then
we wouldn't.

Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

On Fri, Feb 2, 2018 at 11:51 PM, Tomasz Figa  wrote:
> On Fri, Feb 2, 2018 at 11:00 PM, Rob Herring  wrote:
>> On Fri, Feb 2, 2018 at 2:01 AM, Tomasz Figa  wrote:
>>> Hi Rob,
>>>
>>> On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss  
>>> wrote:
>   uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
>   uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t
> plane);
>   uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
>   uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
>   ...
> } gralloc_funcs_t;
>
>
> These ones? >
> Yeah, if we could retrieve such function pointer struct using perform
> or any equivalent (like the implementation-specific methods in
> gralloc1, but not sure if that's going to be used in practice
> anywhere), it could work for us.


 So this is where you and Rob Herring lose me, I don't think I understand
 quite how the gralloc1 call would be used, and how it would tie into this
 handle struct. I think I could do with some guidance on this.
>>>
>>> This would be very similar to gralloc0 perform call. gralloc1
>>> implementations need to provide getFunction() callback [1], which
>>> returns a pointer to given function. The list of standard functions is
>>> defined in the gralloc1.h header [2], but we could take some random
>>> big number and use it for our function that fills in provided
>>> gralloc_funcs_t struct with necessary pointers.
>>>
>>> [1] 
>>> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
>>> [2] 
>>> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134
>>
>> This is a deadend because it won't work with a HIDL based
>> implementation (aka gralloc 2.0). You can't set function pointers (or
>> any pointers) because gralloc runs in a different process. Yes,
>> currently gralloc is a pass-thru HAL, but AIUI that will go away.
>
> Part of it. I can't see IMapper being implemented by a separate
> process. You can't map a buffer into one process from another process.
>
> But anyway, it's a good point, thanks, I almost forgot about its
> existence. I'll do further investigation.

Okay, so IMapper indeed breaks the approach I suggested. I'm not sure
at the moment what we could do about it. (The idea of a dynamic
library of a pre-defined name, exporting functions we specify, might
still work, though.)

Note that the DRM_GRALLOC_GET_FD used currently by Mesa will also be
impossible to implement with IAllocator/IMapper. (Although I still
think Mesa and Gralloc are free to have separate logic for choosing
the DRM device to use.)

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

On Fri, Feb 2, 2018 at 11:00 PM, Rob Herring  wrote:
> On Fri, Feb 2, 2018 at 2:01 AM, Tomasz Figa  wrote:
>> Hi Rob,
>>
>> On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss  
>> wrote:
   uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
   uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t
 plane);
   uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
   uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
   ...
 } gralloc_funcs_t;


 These ones? >
 Yeah, if we could retrieve such function pointer struct using perform
 or any equivalent (like the implementation-specific methods in
 gralloc1, but not sure if that's going to be used in practice
 anywhere), it could work for us.
>>>
>>>
>>> So this is where you and Rob Herring lose me, I don't think I understand
>>> quite how the gralloc1 call would be used, and how it would tie into this
>>> handle struct. I think I could do with some guidance on this.
>>
>> This would be very similar to gralloc0 perform call. gralloc1
>> implementations need to provide getFunction() callback [1], which
>> returns a pointer to given function. The list of standard functions is
>> defined in the gralloc1.h header [2], but we could take some random
>> big number and use it for our function that fills in provided
>> gralloc_funcs_t struct with necessary pointers.
>>
>> [1] 
>> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
>> [2] 
>> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134
>
> This is a deadend because it won't work with a HIDL based
> implementation (aka gralloc 2.0). You can't set function pointers (or
> any pointers) because gralloc runs in a different process. Yes,
> currently gralloc is a pass-thru HAL, but AIUI that will go away.

Part of it. I can't see IMapper being implemented by a separate
process. You can't map a buffer into one process from another process.

But anyway, it's a good point, thanks, I almost forgot about its
existence. I'll do further investigation.

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Rob Herring]

On Fri, Feb 2, 2018 at 2:01 AM, Tomasz Figa  wrote:
> Hi Rob,
>
> On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss  
> wrote:
>>>   uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
>>>   uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t
>>> plane);
>>>   uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
>>>   uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
>>>   ...
>>> } gralloc_funcs_t;
>>>
>>>
>>> These ones? >
>>> Yeah, if we could retrieve such function pointer struct using perform
>>> or any equivalent (like the implementation-specific methods in
>>> gralloc1, but not sure if that's going to be used in practice
>>> anywhere), it could work for us.
>>
>>
>> So this is where you and Rob Herring lose me, I don't think I understand
>> quite how the gralloc1 call would be used, and how it would tie into this
>> handle struct. I think I could do with some guidance on this.
>
> This would be very similar to gralloc0 perform call. gralloc1
> implementations need to provide getFunction() callback [1], which
> returns a pointer to given function. The list of standard functions is
> defined in the gralloc1.h header [2], but we could take some random
> big number and use it for our function that fills in provided
> gralloc_funcs_t struct with necessary pointers.
>
> [1] 
> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
> [2] 
> https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134

This is a deadend because it won't work with a HIDL based
implementation (aka gralloc 2.0). You can't set function pointers (or
any pointers) because gralloc runs in a different process. Yes,
currently gralloc is a pass-thru HAL, but AIUI that will go away.

Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

Hi Rob,

On Tue, Jan 30, 2018 at 9:36 PM, Robert Foss  wrote:
>>   uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
>>   uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t
>> plane);
>>   uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
>>   uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
>>   ...
>> } gralloc_funcs_t;
>>
>>
>> These ones? >
>> Yeah, if we could retrieve such function pointer struct using perform
>> or any equivalent (like the implementation-specific methods in
>> gralloc1, but not sure if that's going to be used in practice
>> anywhere), it could work for us.
>
>
> So this is where you and Rob Herring lose me, I don't think I understand
> quite how the gralloc1 call would be used, and how it would tie into this
> handle struct. I think I could do with some guidance on this.

This would be very similar to gralloc0 perform call. gralloc1
implementations need to provide getFunction() callback [1], which
returns a pointer to given function. The list of standard functions is
defined in the gralloc1.h header [2], but we could take some random
big number and use it for our function that fills in provided
gralloc_funcs_t struct with necessary pointers.

[1] 
https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#300
[2] 
https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/gralloc1.h#134

With this, the buffer consumers (e.g. Mesa) would look like this:

consume_buffer(priv, handle)
{
gralloc_funcs_t funcs;

if (priv->gralloc1) {
DRM_GRALLOC1_PFN_GET_GRALLOC_FUNCS get_gralloc_funcs;

get_gralloc_func = priv->gralloc1->getFunction(priv->gralloc1,
DRM_GRALLOC1_FUNCTION_GET_GRALLOC_FUNCS);
get_gralloc_func(priv->gralloc1, handle, ); // Not sure
if the handle is even necessary
} else /* gralloc0 */ {
priv->gralloc0->perform(priv->gralloc0,
DRM_GRALLOC_PERFORM_GET_GRALLOC_FUNCS, );
}

// funcs has everything that the consumer needs to deal with the buffer
}

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Robert Foss]

On 01/30/2018 04:16 AM, Tomasz Figa wrote:

Hi Rob,

On Tue, Jan 30, 2018 at 1:17 AM, Robert Foss  wrote:

Hey Tomasz,

I'm tempted to split this work into two parts.
1) Move gbm gralloc struct


Alright, if we look at this only as an attempt to converge gbm_ and
drm_gralloc, it's out of my scope and no concern anymore.


Glad I asked, let's get that part over with, and continue with the accessors.




2) Accessor functions

I would like to get 1) out the door to support John Stultzs current HiKey
960 efforts. As for 2), it would seem that we have some more discussing to
do. But I'll keep pushing that forward.

Separately, I'd like to know if the below sketch of a func ptr solution is
what you had in mind. From what I've gathered this is exactly what you've
requested, but I would like to confirm it too.


I assume you mean the ones below?


Yes.





I'll send out a v2, that covers 1) later today.


Rob.


On 01/29/2018 01:03 PM, Robert Foss wrote:

[snip]





  uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
  uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t plane);
  uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
  uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
  ...
} gralloc_funcs_t;


These ones? >
Yeah, if we could retrieve such function pointer struct using perform
or any equivalent (like the implementation-specific methods in
gralloc1, but not sure if that's going to be used in practice
anywhere), it could work for us.


So this is where you and Rob Herring lose me, I don't think I understand quite 
how the gralloc1 call would be used, and how it would tie into this handle 
struct. I think I could do with some guidance on this.




I think we could also add .get_fourcc(handle) and
.get_num_planes(handle) callbacks, so that we could do away with the
whole sophisticated format guessing based on Android HAL format and
lock_ycbcr results.


I intentionally kept the list brief, but a more complete list would provide 
support for each element of the cros minigbm struct.




Perhaps an integer version field would also be useful, in case we end
up adding some more callbacks.


That's a good point.


Rob.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

Hi Rob,

On Tue, Jan 30, 2018 at 1:17 AM, Robert Foss  wrote:
> Hey Tomasz,
>
> I'm tempted to split this work into two parts.
> 1) Move gbm gralloc struct

Alright, if we look at this only as an attempt to converge gbm_ and
drm_gralloc, it's out of my scope and no concern anymore.

> 2) Accessor functions
>
> I would like to get 1) out the door to support John Stultzs current HiKey
> 960 efforts. As for 2), it would seem that we have some more discussing to
> do. But I'll keep pushing that forward.
>
> Separately, I'd like to know if the below sketch of a func ptr solution is
> what you had in mind. From what I've gathered this is exactly what you've
> requested, but I would like to confirm it too.

I assume you mean the ones below?

>
> I'll send out a v2, that covers 1) later today.
>
>
> Rob.
>
>
> On 01/29/2018 01:03 PM, Robert Foss wrote:
[snip]
>>
>>>
  uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
  uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t plane);
  uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
  uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
  ...
 } gralloc_funcs_t;

These ones?

Yeah, if we could retrieve such function pointer struct using perform
or any equivalent (like the implementation-specific methods in
gralloc1, but not sure if that's going to be used in practice
anywhere), it could work for us.

I think we could also add .get_fourcc(handle) and
.get_num_planes(handle) callbacks, so that we could do away with the
whole sophisticated format guessing based on Android HAL format and
lock_ycbcr results.

Perhaps an integer version field would also be useful, in case we end
up adding some more callbacks.

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Robert Foss]

Hey Tomasz,

I'm tempted to split this work into two parts.
1) Move gbm gralloc struct
2) Accessor functions

I would like to get 1) out the door to support John Stultzs current HiKey 960 
efforts. As for 2), it would seem that we have some more discussing to do. But 
I'll keep pushing that forward.


Separately, I'd like to know if the below sketch of a func ptr solution is what 
you had in mind. From what I've gathered this is exactly what you've requested, 
but I would like to confirm it too.


I'll send out a v2, that covers 1) later today.


Rob.

On 01/29/2018 01:03 PM, Robert Foss wrote:



On 01/25/2018 08:52 PM, Rob Herring wrote:

On Thu, Jan 25, 2018 at 10:21 AM, Robert Foss  wrote:

Hey Tomasz,

On 01/24/2018 11:04 AM, Tomasz Figa wrote:


Hi Robert,

On Wed, Jan 17, 2018 at 2:36 AM, Robert Foss 
wrote:


This series moves {gbm,drm,cros}_gralloc_handle_t struct to libdrm,
since at least 4 implementations exist, and share a lot of contents.
The idea is to keep the common stuff defined in one place, and libdrm
is the common codebase to all of these platforms.


Additionally, having this struct defined in libdrm will make it
easier for mesa and gralloc implementations to communicate.

Robert Foss (7):
    android: Move gralloc handle struct to libdrm
    android: Add version variable to gralloc_handle_t
    android: Mark gralloc_handle_t magic variable as const
    android: Remove member name from gralloc_handle_t
    android: Change gralloc_handle_t format from Android format to fourcc
    android: Change gralloc_handle_t members to be fixed width
    android: Add accessor functions for gralloc_handle_t variables



Again, thanks for working on this.

I looked through the series and it seems to be much different from
what I imagined when writing my previous reply. I must have
misunderstood your proposal back then.



Ah, glad we caught it before v2 then :)



Generally, current series doesn't solve Chromium OS main concern of
locking down the handle struct. Even though accessors are added, they
are implemented in libdrm and refer to the exact handle layout as per
the handle struct defined by libdrm.



So solving the problems of multiple projects is the goal, so reconsidering
is probably they way forward.



What I had in my mind, would be creating a secondary struct,
consisting only of callbacks, which would be filled in by particular
gralloc implementation running in the system with its accessors. This
would completely eliminate any dependencies on the handle struct
itself from consumers of gralloc buffers.



So just to sketch out the solution, it would look something like this?

struct gralloc_handle_t {


This is not a handle...


Ah, yes. gralloc_funcs_t was the intention.




 uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
 uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t plane);
 uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
 uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
 ...
} gralloc_funcs_t;

struct gralloc_handle_t {
 native_handle_t base;

 /* api variables */
 const int magic; /* differentiate between allocator impls */
 const int version; /* api version */

 gralloc_funcs_t funcs;


This doesn't go in the handle, but rather you would retrieve this
struct I guess with a "perform" call to gralloc AIUI.


I don't think I understand why? Exchanging handles containing function pointers 
may not be the the cleanest of solutions, but wouldn't it be possible?
Or is the need for having registerBuffer setting up the pointers going to be a 
problem?




Of course, if you have 1 perform call, then why not just a perform op
for each accessor. Does perform even exist in a gralloc 2
implementation?


 ...
} gralloc_handle_t;

For reasons of backwards compatability gralloc_handle_t should probably
contain whatever gbm_gralloc_handle_t contains now too.


Being backwards compatible with upstream (to the extent there is one)
was a goal. You can't really have that and what Tomasz proposes.



If we're looking at non-handle based solutions then we wouldn't be able to, no.
As noted above I don't think I understand why non-handle based solutions would 
be required.



Since we're going to version this struct, we can always drop extraneous
variables later.
Since we'll be able to drop variables, we could add more variables to
support the cros minigbm variables of even the intel minigbm ones.
This would be a bit high churn, but probably ease adoption.


I've yet to hear technical reasons why the handle struct needs to be different.


Thats' fair, the reason for adding the least common denominator of elements from 
the different implementation will maintain backwards compatability for 
cros minigbm, not just gbm gralloc.


In my mind this would be a 'nice to have' and if means simplifying this process, 
I'm happy to drop it.





Additionally the 

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Robert Foss]

On 01/25/2018 08:52 PM, Rob Herring wrote:

On Thu, Jan 25, 2018 at 10:21 AM, Robert Foss  wrote:

Hey Tomasz,

On 01/24/2018 11:04 AM, Tomasz Figa wrote:


Hi Robert,

On Wed, Jan 17, 2018 at 2:36 AM, Robert Foss 
wrote:


This series moves {gbm,drm,cros}_gralloc_handle_t struct to libdrm,
since at least 4 implementations exist, and share a lot of contents.
The idea is to keep the common stuff defined in one place, and libdrm
is the common codebase to all of these platforms.


Additionally, having this struct defined in libdrm will make it
easier for mesa and gralloc implementations to communicate.

Robert Foss (7):
android: Move gralloc handle struct to libdrm
android: Add version variable to gralloc_handle_t
android: Mark gralloc_handle_t magic variable as const
android: Remove member name from gralloc_handle_t
android: Change gralloc_handle_t format from Android format to fourcc
android: Change gralloc_handle_t members to be fixed width
android: Add accessor functions for gralloc_handle_t variables



Again, thanks for working on this.

I looked through the series and it seems to be much different from
what I imagined when writing my previous reply. I must have
misunderstood your proposal back then.



Ah, glad we caught it before v2 then :)



Generally, current series doesn't solve Chromium OS main concern of
locking down the handle struct. Even though accessors are added, they
are implemented in libdrm and refer to the exact handle layout as per
the handle struct defined by libdrm.



So solving the problems of multiple projects is the goal, so reconsidering
is probably they way forward.



What I had in my mind, would be creating a secondary struct,
consisting only of callbacks, which would be filled in by particular
gralloc implementation running in the system with its accessors. This
would completely eliminate any dependencies on the handle struct
itself from consumers of gralloc buffers.



So just to sketch out the solution, it would look something like this?

struct gralloc_handle_t {


This is not a handle...


Ah, yes. gralloc_funcs_t was the intention.




 uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
 uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t plane);
 uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
 uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
 ...
} gralloc_funcs_t;

struct gralloc_handle_t {
 native_handle_t base;

 /* api variables */
 const int magic; /* differentiate between allocator impls */
 const int version; /* api version */

 gralloc_funcs_t funcs;


This doesn't go in the handle, but rather you would retrieve this
struct I guess with a "perform" call to gralloc AIUI.


I don't think I understand why? Exchanging handles containing function pointers 
may not be the the cleanest of solutions, but wouldn't it be possible?
Or is the need for having registerBuffer setting up the pointers going to be a 
problem?




Of course, if you have 1 perform call, then why not just a perform op
for each accessor. Does perform even exist in a gralloc 2
implementation?


 ...
} gralloc_handle_t;

For reasons of backwards compatability gralloc_handle_t should probably
contain whatever gbm_gralloc_handle_t contains now too.


Being backwards compatible with upstream (to the extent there is one)
was a goal. You can't really have that and what Tomasz proposes.



If we're looking at non-handle based solutions then we wouldn't be able to, no.
As noted above I don't think I understand why non-handle based solutions would 
be required.



Since we're going to version this struct, we can always drop extraneous
variables later.
Since we'll be able to drop variables, we could add more variables to
support the cros minigbm variables of even the intel minigbm ones.
This would be a bit high churn, but probably ease adoption.


I've yet to hear technical reasons why the handle struct needs to be different.


Thats' fair, the reason for adding the least common denominator of elements from 
the different implementation will maintain backwards compatability for 
cros minigbm, not just gbm gralloc.


In my mind this would be a 'nice to have' and if means simplifying this process, 
I'm happy to drop it.





Additionally the gralloc buffer registering mechanism doesn't exist in any
of the gralloc implementations, so being able to start out with something
that works on all platforms would be nice.


Rob.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Rob Herring]

On Thu, Jan 25, 2018 at 10:21 AM, Robert Foss  wrote:
> Hey Tomasz,
>
> On 01/24/2018 11:04 AM, Tomasz Figa wrote:
>>
>> Hi Robert,
>>
>> On Wed, Jan 17, 2018 at 2:36 AM, Robert Foss 
>> wrote:
>>>
>>> This series moves {gbm,drm,cros}_gralloc_handle_t struct to libdrm,
>>> since at least 4 implementations exist, and share a lot of contents.
>>> The idea is to keep the common stuff defined in one place, and libdrm
>>> is the common codebase to all of these platforms.
>>>
>>>
>>> Additionally, having this struct defined in libdrm will make it
>>> easier for mesa and gralloc implementations to communicate.
>>>
>>> Robert Foss (7):
>>>android: Move gralloc handle struct to libdrm
>>>android: Add version variable to gralloc_handle_t
>>>android: Mark gralloc_handle_t magic variable as const
>>>android: Remove member name from gralloc_handle_t
>>>android: Change gralloc_handle_t format from Android format to fourcc
>>>android: Change gralloc_handle_t members to be fixed width
>>>android: Add accessor functions for gralloc_handle_t variables
>>
>>
>> Again, thanks for working on this.
>>
>> I looked through the series and it seems to be much different from
>> what I imagined when writing my previous reply. I must have
>> misunderstood your proposal back then.
>
>
> Ah, glad we caught it before v2 then :)
>
>>
>> Generally, current series doesn't solve Chromium OS main concern of
>> locking down the handle struct. Even though accessors are added, they
>> are implemented in libdrm and refer to the exact handle layout as per
>> the handle struct defined by libdrm.
>
>
> So solving the problems of multiple projects is the goal, so reconsidering
> is probably they way forward.
>
>>
>> What I had in my mind, would be creating a secondary struct,
>> consisting only of callbacks, which would be filled in by particular
>> gralloc implementation running in the system with its accessors. This
>> would completely eliminate any dependencies on the handle struct
>> itself from consumers of gralloc buffers.
>
>
> So just to sketch out the solution, it would look something like this?
>
> struct gralloc_handle_t {

This is not a handle...

> uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
> uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t plane);
> uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
> uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
> ...
> } gralloc_funcs_t;
>
> struct gralloc_handle_t {
> native_handle_t base;
>
> /* api variables */
> const int magic; /* differentiate between allocator impls */
> const int version; /* api version */
>
> gralloc_funcs_t funcs;

This doesn't go in the handle, but rather you would retrieve this
struct I guess with a "perform" call to gralloc AIUI.

Of course, if you have 1 perform call, then why not just a perform op
for each accessor. Does perform even exist in a gralloc 2
implementation?

> ...
> } gralloc_handle_t;
>
> For reasons of backwards compatability gralloc_handle_t should probably
> contain whatever gbm_gralloc_handle_t contains now too.

Being backwards compatible with upstream (to the extent there is one)
was a goal. You can't really have that and what Tomasz proposes.

> Since we're going to version this struct, we can always drop extraneous
> variables later.
> Since we'll be able to drop variables, we could add more variables to
> support the cros minigbm variables of even the intel minigbm ones.
> This would be a bit high churn, but probably ease adoption.

I've yet to hear technical reasons why the handle struct needs to be different.

> Additionally the gralloc buffer registering mechanism doesn't exist in any
> of the gralloc implementations, so being able to start out with something
> that works on all platforms would be nice.
>
>
> Rob.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Robert Foss]

Hey Tomasz,

On 01/24/2018 11:04 AM, Tomasz Figa wrote:

Hi Robert,

On Wed, Jan 17, 2018 at 2:36 AM, Robert Foss  wrote:

This series moves {gbm,drm,cros}_gralloc_handle_t struct to libdrm,
since at least 4 implementations exist, and share a lot of contents.
The idea is to keep the common stuff defined in one place, and libdrm
is the common codebase to all of these platforms.


Additionally, having this struct defined in libdrm will make it
easier for mesa and gralloc implementations to communicate.

Robert Foss (7):
   android: Move gralloc handle struct to libdrm
   android: Add version variable to gralloc_handle_t
   android: Mark gralloc_handle_t magic variable as const
   android: Remove member name from gralloc_handle_t
   android: Change gralloc_handle_t format from Android format to fourcc
   android: Change gralloc_handle_t members to be fixed width
   android: Add accessor functions for gralloc_handle_t variables


Again, thanks for working on this.

I looked through the series and it seems to be much different from
what I imagined when writing my previous reply. I must have
misunderstood your proposal back then.


Ah, glad we caught it before v2 then :)



Generally, current series doesn't solve Chromium OS main concern of
locking down the handle struct. Even though accessors are added, they
are implemented in libdrm and refer to the exact handle layout as per
the handle struct defined by libdrm.


So solving the problems of multiple projects is the goal, so reconsidering is 
probably they way forward.




What I had in my mind, would be creating a secondary struct,
consisting only of callbacks, which would be filled in by particular
gralloc implementation running in the system with its accessors. This
would completely eliminate any dependencies on the handle struct
itself from consumers of gralloc buffers.


So just to sketch out the solution, it would look something like this?

struct gralloc_handle_t {
uint32_t (*get_fd)(buffer_handle_t handle, uint32_t plane);
uint64_t (*get_modifier)(buffer_handle_t handle, uint32_t plane);
uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
...
} gralloc_funcs_t;

struct gralloc_handle_t {
native_handle_t base;

/* api variables */
const int magic; /* differentiate between allocator impls */
const int version; /* api version */

gralloc_funcs_t funcs;

...
} gralloc_handle_t;

For reasons of backwards compatability gralloc_handle_t should probably
contain whatever gbm_gralloc_handle_t contains now too.
Since we're going to version this struct, we can always drop extraneous 
variables later.
Since we'll be able to drop variables, we could add more variables to support 
the cros minigbm variables of even the intel minigbm ones.

This would be a bit high churn, but probably ease adoption.

Additionally the gralloc buffer registering mechanism doesn't exist in any of 
the gralloc implementations, so being able to start out with something that 
works on all platforms would be nice.



Rob.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Rob Herring]

On Wed, Jan 24, 2018 at 4:04 AM, Tomasz Figa  wrote:
> Hi Robert,
>
> On Wed, Jan 17, 2018 at 2:36 AM, Robert Foss  
> wrote:
>> This series moves {gbm,drm,cros}_gralloc_handle_t struct to libdrm,
>> since at least 4 implementations exist, and share a lot of contents.
>> The idea is to keep the common stuff defined in one place, and libdrm
>> is the common codebase to all of these platforms.
>>
>>
>> Additionally, having this struct defined in libdrm will make it
>> easier for mesa and gralloc implementations to communicate.
>>
>> Robert Foss (7):
>>   android: Move gralloc handle struct to libdrm
>>   android: Add version variable to gralloc_handle_t
>>   android: Mark gralloc_handle_t magic variable as const
>>   android: Remove member name from gralloc_handle_t
>>   android: Change gralloc_handle_t format from Android format to fourcc
>>   android: Change gralloc_handle_t members to be fixed width
>>   android: Add accessor functions for gralloc_handle_t variables
>
> Again, thanks for working on this.
>
> I looked through the series and it seems to be much different from
> what I imagined when writing my previous reply. I must have
> misunderstood your proposal back then.
>
> Generally, current series doesn't solve Chromium OS main concern of
> locking down the handle struct. Even though accessors are added, they
> are implemented in libdrm and refer to the exact handle layout as per
> the handle struct defined by libdrm.
>
> What I had in my mind, would be creating a secondary struct,
> consisting only of callbacks, which would be filled in by particular
> gralloc implementation running in the system with its accessors. This
> would completely eliminate any dependencies on the handle struct
> itself from consumers of gralloc buffers.

That's a non-goal IMO. The goal is to converge towards a common
definition, but the handle is not locked down. Chromium OS or any
downstream project is free to fork libdrm and make changes, implement
the accessor API in some other place, or submit changes upstream to
the handle.

To put it another way, why do we need differing handle definitions in
the first place? We have them today because no one has cared.

Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1 0/7] Implement commont gralloc_handle_t in libdrm

[Tomasz Figa]

Hi Robert,

On Wed, Jan 17, 2018 at 2:36 AM, Robert Foss  wrote:
> This series moves {gbm,drm,cros}_gralloc_handle_t struct to libdrm,
> since at least 4 implementations exist, and share a lot of contents.
> The idea is to keep the common stuff defined in one place, and libdrm
> is the common codebase to all of these platforms.
>
>
> Additionally, having this struct defined in libdrm will make it
> easier for mesa and gralloc implementations to communicate.
>
> Robert Foss (7):
>   android: Move gralloc handle struct to libdrm
>   android: Add version variable to gralloc_handle_t
>   android: Mark gralloc_handle_t magic variable as const
>   android: Remove member name from gralloc_handle_t
>   android: Change gralloc_handle_t format from Android format to fourcc
>   android: Change gralloc_handle_t members to be fixed width
>   android: Add accessor functions for gralloc_handle_t variables

Again, thanks for working on this.

I looked through the series and it seems to be much different from
what I imagined when writing my previous reply. I must have
misunderstood your proposal back then.

Generally, current series doesn't solve Chromium OS main concern of
locking down the handle struct. Even though accessors are added, they
are implemented in libdrm and refer to the exact handle layout as per
the handle struct defined by libdrm.

What I had in my mind, would be creating a secondary struct,
consisting only of callbacks, which would be filled in by particular
gralloc implementation running in the system with its accessors. This
would completely eliminate any dependencies on the handle struct
itself from consumers of gralloc buffers.

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev