* [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes @ 2018-07-24 8:22 Robert Foss 2018-08-01 14:24 ` Martin Fuzzey 2018-08-03 12:25 ` Emil Velikov 0 siblings, 2 replies; 19+ messages in thread From: Robert Foss @ 2018-07-24 8:22 UTC (permalink / raw) To: gustavo, maarten.lankhorst, seanpaul, airlied, dri-devel, linux-kernel, Emil Velikov, Tomasz Figa, Rob Herring, Eric Engestrom, Brian Paul Cc: Tomeu Vizoso, Nicolas Norvez, Robert Foss From: Tomasz Figa <tfiga@chromium.org> There is no particular reason to prevent userspace for using this IOCTL, considering that it already has access to other, platform-specific IOCTLs. This patch makes is possible to have the same userspace code work regardless on the underlying platform, which significantly simplifies the stack. Signed-off-by: Tomasz Figa <tfiga@chromium.org> Reviewed-by: Zach Reizner <zachr@chromium.org> Signed-off-by: Nicolas Norvez <norvez@chromium.org> Reviewed-by: Tomasz Figa <tfiga@chromium.org> Signed-off-by: Robert Foss <robert.foss@collabora.com> --- I've been looking into enabling a kms_swrast based driver for mesa on the Android platform[1]. But have come up against the issue of dumb buffer related ioctls below not being flagged with DRM_RENDER_ALLOW. DRM_IOCTL_MODE_CREATE_DUMB DRM_IOCTL_MODE_MAP_DUMB To be more precise, I've been seeing a failure due to DRM_IOCTL_MODE_MAP_DUMB not being allowed for /dev/dri/renderD* nodes, and used in mesa kms_sw_displaytarget_map(). As I understand it the DRM_RENDER_ALLOW flag being unset is a very intentional restriction placed on dumb buffers in order to minimize its use. But as far as alternative solutions for software renderers there seems to only be VGEM and mmap()-ing DMABUFs. While it would be convenient from the point of view of software renderers if dumb buffers had more promiscuous permissions, it may be a hard sell upstream. If dumb buffers aren't the way forward, what is? VGEM? Or are there any other preferable ways? [1] https://patchwork.freedesktop.org/series/45966/ drivers/gpu/drm/drm_ioctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 0d4cfb232576..ef716246baf6 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -642,8 +642,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_MODE_RMFB, drm_mode_rmfb, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_PAGE_FLIP, drm_mode_page_flip_ioctl, DRM_MASTER|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_DIRTYFB, drm_mode_dirtyfb_ioctl, DRM_MASTER|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_DUMB, drm_mode_create_dumb_ioctl, DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_MAP_DUMB, drm_mode_mmap_dumb_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_DUMB, drm_mode_create_dumb_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_MAP_DUMB, drm_mode_mmap_dumb_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_properties_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY, drm_mode_obj_set_property_ioctl, DRM_MASTER|DRM_UNLOCKED), -- 2.17.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes 2018-07-24 8:22 [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes Robert Foss @ 2018-08-01 14:24 ` Martin Fuzzey 2018-08-03 12:35 ` Emil Velikov 2018-08-03 12:25 ` Emil Velikov 1 sibling, 1 reply; 19+ messages in thread From: Martin Fuzzey @ 2018-08-01 14:24 UTC (permalink / raw) To: robert.foss Cc: airlied, brianp, dri-devel, emil.l.velikov, eric.engestrom, gustavo, linux-kernel, maarten.lankhorst, norvez, robh, seanpaul, tfiga, tomeu.vizoso Hi, I am running into the same problem using etnaviv on i.MX6 under Android 8.1 The mesa etnaviv code uses CREATE_DUMB / MAP_DUMB for the scanout buffers and is called from the surfaceflinger process. But, under Android 8.1 drm_hwcomposer runs in a seperate process to surfaceflinger. Since drm_hwcomposer needs to use the KMS API it must be the DRM master, that means that surface flinger cannot be DRM master too. Adding a render node to the imx drm device and configuring mesa to user fixes the problem but requires the render node to have access rights to use CREATE_DUMB / MAP_DUMB. Here is my full patch: Make imx-drm export a render node so that mesa can use it to allocate From: Martin Fuzzey <martin.fuzzey@flowbird.group> Date: 2018-07-26 15:37:28 +0200 dumb memory rather than requiring the master node. The problem with using the master node is that, on android 8.1, drm_hwcomposer is a seperate process and must be the master node (to use the KMS API), since only a single process may be master surfaceflinger cannot be master too. With this change surfaceflinger can use just a rendernode. Note that we also have to modify the permissions table to allow render nodes to use the dumb allocation functions. This is a hack and unlikely to be accepted upstream but it's better than the huge security hole of allowing everything we were using before. Need to discuss an upstream acceptable way for this... --- drivers/gpu/drm/drm_ioctl.c | 6 +++--- drivers/gpu/drm/imx/imx-drm-core.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index a9ae6dd..31c4c86 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -639,9 +639,9 @@ int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) DRM_IOCTL_DEF(DRM_IOCTL_MODE_RMFB, drm_mode_rmfb, DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_PAGE_FLIP, drm_mode_page_flip_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_DIRTYFB, drm_mode_dirtyfb_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_DUMB, drm_mode_create_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_MAP_DUMB, drm_mode_mmap_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_DUMB, drm_mode_create_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_MAP_DUMB, drm_mode_mmap_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_properties_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY, drm_mode_obj_set_property_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR2, drm_mode_cursor2_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index f91cb72..0c34306 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -177,7 +177,7 @@ int imx_drm_encoder_parse_of(struct drm_device *drm, static struct drm_driver imx_drm_driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | - DRIVER_ATOMIC, + DRIVER_ATOMIC | DRIVER_RENDER, .lastclose = imx_drm_driver_lastclose, .gem_free_object_unlocked = drm_gem_cma_free_object, .gem_vm_ops = &drm_gem_cma_vm_ops, If it is not acceptable to modify the access rights for the DUMB allocation functions how should this be done? Best regards, Martin ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes 2018-08-01 14:24 ` Martin Fuzzey @ 2018-08-03 12:35 ` Emil Velikov 2018-08-03 15:06 ` Martin Fuzzey 0 siblings, 1 reply; 19+ messages in thread From: Emil Velikov @ 2018-08-03 12:35 UTC (permalink / raw) To: Martin Fuzzey Cc: Robert Foss, David Airlie, Brian Paul, ML dri-devel, Eric Engestrom, Gustavo Padovan, Linux-Kernel@Vger. Kernel. Org, Maarten Lankhorst, Nicolas Norvez, Rob Herring, Sean Paul, Tomasz Figa, Tomeu Vizoso Hi Martin, On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: > Hi, > > I am running into the same problem using etnaviv on i.MX6 under Android 8.1 > > The mesa etnaviv code uses CREATE_DUMB / MAP_DUMB for the scanout buffers > and is called from the surfaceflinger process. > > But, under Android 8.1 drm_hwcomposer runs in a seperate process to > surfaceflinger. > Since drm_hwcomposer needs to use the KMS API it must be the DRM master, > that means that surface flinger cannot be DRM master too. > > Adding a render node to the imx drm device and configuring mesa to user > fixes > the problem but requires the render node to have access rights to use > CREATE_DUMB > / MAP_DUMB. > > > Here is my full patch: > > Make imx-drm export a render node so that mesa can use it to allocate > Let's start with the not-so obvious question: Why does one open the imx as render node? Of the top of my head: There is nothing in egl/android that should require an authenticated device. Hence, using a card node should be fine - the etnaviv code opens the render node it needs. It's been a while since I looked at the imx/etna code, so I may be missing something. HTH Emil ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes 2018-08-03 12:35 ` Emil Velikov @ 2018-08-03 15:06 ` Martin Fuzzey 2018-08-03 17:03 ` Emil Velikov 0 siblings, 1 reply; 19+ messages in thread From: Martin Fuzzey @ 2018-08-03 15:06 UTC (permalink / raw) To: Emil Velikov Cc: Robert Foss, David Airlie, Brian Paul, ML dri-devel, Eric Engestrom, Gustavo Padovan, Linux-Kernel@Vger. Kernel. Org, Maarten Lankhorst, Nicolas Norvez, Rob Herring, Sean Paul, Tomasz Figa, Tomeu Vizoso Hi Emil, On 03/08/18 14:35, Emil Velikov wrote: > Hi Martin, > > On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: > > Let's start with the not-so obvious question: > Why does one open the imx as render node? > > Of the top of my head: > There is nothing in egl/android that should require an authenticated device. > Hence, using a card node should be fine - the etnaviv code opens the > render node it needs. Yes, the problem is not in egl/android but in the scanout buffer allocation code. etnaviv opens the render node on the *GPU* (for submitting GPU commands), that part is fine. But scanout buffers need to be allocated from imx-drm not etnaviv. This done by renderonly_create_kms_dumb_buffer_for_resource() [src/gallium/auxiliary/renderonly/renderonly.c] Which uses DRM_IOCTL_MODE_CREATE_DUMB followed by DRM_IOCTL_PRIME_FD_TO_HANDLE on the "kms_fd" (probably poorly named because it's not actually used for modesetting) see imx_drm_screen_create()[ src/gallium/winsys/imx/drm/imx_drm_winsys.c] If the card node is used DRM_IOCTL_MODE_CREATE_DUMB works but DRM_IOCTL_PRIME_FD_TO_HANDLE fails, because the permissions are DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW In android 8.1 the hardware composer runs in a seperate process and it has to use the card node and be drm master (to use the KMS API), therefore, when the surface flinger calls renderonly_create_kms_dumb_buffer_for_resource() it is not authenticated. Making surface flinger use a render node fixes the problem for DRM_IOCTL_PRIME_FD_TO_HANDLE (because that already has DRM_RENDER_ALLOW), but DRM_IOCTL_MODE_CREATE_DUMB now fails without the patch. This probably worked in previous versions of Android where surface flinger and hwc were all in the same process. Regards, Martin ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes 2018-08-03 15:06 ` Martin Fuzzey @ 2018-08-03 17:03 ` Emil Velikov 0 siblings, 0 replies; 19+ messages in thread From: Emil Velikov @ 2018-08-03 17:03 UTC (permalink / raw) To: Martin Fuzzey Cc: Robert Foss, David Airlie, Brian Paul, ML dri-devel, Eric Engestrom, Gustavo Padovan, Linux-Kernel@Vger. Kernel. Org, Maarten Lankhorst, Nicolas Norvez, Rob Herring, Sean Paul, Tomasz Figa, Tomeu Vizoso On 3 August 2018 at 16:06, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: > Hi Emil, > > On 03/08/18 14:35, Emil Velikov wrote: >> >> Hi Martin, >> >> On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> >> wrote: >> >> Let's start with the not-so obvious question: >> Why does one open the imx as render node? >> >> Of the top of my head: >> There is nothing in egl/android that should require an authenticated >> device. >> Hence, using a card node should be fine - the etnaviv code opens the >> render node it needs. > > > Yes, the problem is not in egl/android but in the scanout buffer allocation > code. > > etnaviv opens the render node on the *GPU* (for submitting GPU commands), > that part is fine. > > But scanout buffers need to be allocated from imx-drm not etnaviv. > > This done by renderonly_create_kms_dumb_buffer_for_resource() > [src/gallium/auxiliary/renderonly/renderonly.c] > Which uses DRM_IOCTL_MODE_CREATE_DUMB followed by > DRM_IOCTL_PRIME_FD_TO_HANDLE > on the "kms_fd" (probably poorly named because it's not actually used for > modesetting) > see imx_drm_screen_create()[ src/gallium/winsys/imx/drm/imx_drm_winsys.c] > > > If the card node is used DRM_IOCTL_MODE_CREATE_DUMB works but > DRM_IOCTL_PRIME_FD_TO_HANDLE fails, because the permissions are > DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW > Right I missed the DRM_AUTH, in the fd <> handle IOCTLs. So in order for things to work, we'd need to either: - allow dumb buffers for render nodes, or - drop the DRM_AUTH for fd <> handle imports Pointing an alternative solution, for kernel developers to analyse and make a decision. > > In android 8.1 the hardware composer runs in a seperate process and it has > to use the card node and be drm master (to use the KMS API), > therefore, when the surface flinger calls > renderonly_create_kms_dumb_buffer_for_resource() it is not authenticated. > > Making surface flinger use a render node fixes the problem for > DRM_IOCTL_PRIME_FD_TO_HANDLE (because that already has DRM_RENDER_ALLOW), > but DRM_IOCTL_MODE_CREATE_DUMB now fails without the patch. > > > This probably worked in previous versions of Android where surface flinger > and hwc were all in the same process. > There has been varying hacks for Android through the years. Bringing details into the discussion will result in a significant diversion. Something we could avoid, for the time being ;-) Thanks Emil ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes @ 2018-08-03 17:03 ` Emil Velikov 0 siblings, 0 replies; 19+ messages in thread From: Emil Velikov @ 2018-08-03 17:03 UTC (permalink / raw) To: Martin Fuzzey Cc: Tomeu Vizoso, Nicolas Norvez, Robert Foss, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, Tomasz Figa, Eric Engestrom, David Airlie, Brian Paul On 3 August 2018 at 16:06, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: > Hi Emil, > > On 03/08/18 14:35, Emil Velikov wrote: >> >> Hi Martin, >> >> On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> >> wrote: >> >> Let's start with the not-so obvious question: >> Why does one open the imx as render node? >> >> Of the top of my head: >> There is nothing in egl/android that should require an authenticated >> device. >> Hence, using a card node should be fine - the etnaviv code opens the >> render node it needs. > > > Yes, the problem is not in egl/android but in the scanout buffer allocation > code. > > etnaviv opens the render node on the *GPU* (for submitting GPU commands), > that part is fine. > > But scanout buffers need to be allocated from imx-drm not etnaviv. > > This done by renderonly_create_kms_dumb_buffer_for_resource() > [src/gallium/auxiliary/renderonly/renderonly.c] > Which uses DRM_IOCTL_MODE_CREATE_DUMB followed by > DRM_IOCTL_PRIME_FD_TO_HANDLE > on the "kms_fd" (probably poorly named because it's not actually used for > modesetting) > see imx_drm_screen_create()[ src/gallium/winsys/imx/drm/imx_drm_winsys.c] > > > If the card node is used DRM_IOCTL_MODE_CREATE_DUMB works but > DRM_IOCTL_PRIME_FD_TO_HANDLE fails, because the permissions are > DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW > Right I missed the DRM_AUTH, in the fd <> handle IOCTLs. So in order for things to work, we'd need to either: - allow dumb buffers for render nodes, or - drop the DRM_AUTH for fd <> handle imports Pointing an alternative solution, for kernel developers to analyse and make a decision. > > In android 8.1 the hardware composer runs in a seperate process and it has > to use the card node and be drm master (to use the KMS API), > therefore, when the surface flinger calls > renderonly_create_kms_dumb_buffer_for_resource() it is not authenticated. > > Making surface flinger use a render node fixes the problem for > DRM_IOCTL_PRIME_FD_TO_HANDLE (because that already has DRM_RENDER_ALLOW), > but DRM_IOCTL_MODE_CREATE_DUMB now fails without the patch. > > > This probably worked in previous versions of Android where surface flinger > and hwc were all in the same process. > There has been varying hacks for Android through the years. Bringing details into the discussion will result in a significant diversion. Something we could avoid, for the time being ;-) Thanks Emil _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes 2018-08-03 17:03 ` Emil Velikov @ 2018-08-03 19:50 ` Sean Paul -1 siblings, 0 replies; 19+ messages in thread From: Sean Paul @ 2018-08-03 19:50 UTC (permalink / raw) To: Emil Velikov Cc: Martin Fuzzey, Robert Foss, David Airlie, Brian Paul, ML dri-devel, Eric Engestrom, Gustavo Padovan, Linux-Kernel@Vger. Kernel. Org, Maarten Lankhorst, Nicolas Norvez, Rob Herring, Sean Paul, Tomasz Figa, Tomeu Vizoso On Fri, Aug 03, 2018 at 06:03:50PM +0100, Emil Velikov wrote: > On 3 August 2018 at 16:06, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: > > Hi Emil, > > > > On 03/08/18 14:35, Emil Velikov wrote: > >> > >> Hi Martin, > >> > >> On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> > >> wrote: > >> > >> Let's start with the not-so obvious question: > >> Why does one open the imx as render node? > >> > >> Of the top of my head: > >> There is nothing in egl/android that should require an authenticated > >> device. > >> Hence, using a card node should be fine - the etnaviv code opens the > >> render node it needs. > > > > > > Yes, the problem is not in egl/android but in the scanout buffer allocation > > code. > > > > etnaviv opens the render node on the *GPU* (for submitting GPU commands), > > that part is fine. > > > > But scanout buffers need to be allocated from imx-drm not etnaviv. > > > > This done by renderonly_create_kms_dumb_buffer_for_resource() > > [src/gallium/auxiliary/renderonly/renderonly.c] > > Which uses DRM_IOCTL_MODE_CREATE_DUMB followed by > > DRM_IOCTL_PRIME_FD_TO_HANDLE > > on the "kms_fd" (probably poorly named because it's not actually used for > > modesetting) > > see imx_drm_screen_create()[ src/gallium/winsys/imx/drm/imx_drm_winsys.c] > > > > > > If the card node is used DRM_IOCTL_MODE_CREATE_DUMB works but > > DRM_IOCTL_PRIME_FD_TO_HANDLE fails, because the permissions are > > DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW > > > Right I missed the DRM_AUTH, in the fd <> handle IOCTLs. > So in order for things to work, we'd need to either: > - allow dumb buffers for render nodes, or > - drop the DRM_AUTH for fd <> handle imports > > Pointing an alternative solution, for kernel developers to analyse and > make a decision. > > > > > In android 8.1 the hardware composer runs in a seperate process and it has > > to use the card node and be drm master (to use the KMS API), > > therefore, when the surface flinger calls > > renderonly_create_kms_dumb_buffer_for_resource() it is not authenticated. > > > > Making surface flinger use a render node fixes the problem for > > DRM_IOCTL_PRIME_FD_TO_HANDLE (because that already has DRM_RENDER_ALLOW), > > but DRM_IOCTL_MODE_CREATE_DUMB now fails without the patch. > > > > > > This probably worked in previous versions of Android where surface flinger > > and hwc were all in the same process. > > > There has been varying hacks for Android through the years. Bringing > details into the discussion will result in a significant diversion. > Something we could avoid, for the time being ;-) Did someone say diversion?!? The way this was handled prior to using render/control nodes in drm_hwc/[drm/gbm]_gralloc is that all modesetting was done via gralloc which was master. The hwc implementation was basically a proxy backchanneling all of the work to gralloc. Anyways, we probably don't want to go back there. Fwiw, I'd lean towards allowing DUMB allocation from the render nodes. I understand it limits use cases that are undesirable, but it is also limiting usecases that are desirable. So, given that people are going to get "creative" regardless of how many safety railings we put up, we shouldn't make things unnecessarily hard on other trying to Get Work Done. Sean [Disclaimer: I'm totally and completely biased on this issue] > > Thanks > Emil -- Sean Paul, Software Engineer, Google / Chromium OS ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes @ 2018-08-03 19:50 ` Sean Paul 0 siblings, 0 replies; 19+ messages in thread From: Sean Paul @ 2018-08-03 19:50 UTC (permalink / raw) To: Emil Velikov Cc: Tomeu Vizoso, Nicolas Norvez, Robert Foss, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, Tomasz Figa, Eric Engestrom, David Airlie, Brian Paul, Martin Fuzzey On Fri, Aug 03, 2018 at 06:03:50PM +0100, Emil Velikov wrote: > On 3 August 2018 at 16:06, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: > > Hi Emil, > > > > On 03/08/18 14:35, Emil Velikov wrote: > >> > >> Hi Martin, > >> > >> On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> > >> wrote: > >> > >> Let's start with the not-so obvious question: > >> Why does one open the imx as render node? > >> > >> Of the top of my head: > >> There is nothing in egl/android that should require an authenticated > >> device. > >> Hence, using a card node should be fine - the etnaviv code opens the > >> render node it needs. > > > > > > Yes, the problem is not in egl/android but in the scanout buffer allocation > > code. > > > > etnaviv opens the render node on the *GPU* (for submitting GPU commands), > > that part is fine. > > > > But scanout buffers need to be allocated from imx-drm not etnaviv. > > > > This done by renderonly_create_kms_dumb_buffer_for_resource() > > [src/gallium/auxiliary/renderonly/renderonly.c] > > Which uses DRM_IOCTL_MODE_CREATE_DUMB followed by > > DRM_IOCTL_PRIME_FD_TO_HANDLE > > on the "kms_fd" (probably poorly named because it's not actually used for > > modesetting) > > see imx_drm_screen_create()[ src/gallium/winsys/imx/drm/imx_drm_winsys.c] > > > > > > If the card node is used DRM_IOCTL_MODE_CREATE_DUMB works but > > DRM_IOCTL_PRIME_FD_TO_HANDLE fails, because the permissions are > > DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW > > > Right I missed the DRM_AUTH, in the fd <> handle IOCTLs. > So in order for things to work, we'd need to either: > - allow dumb buffers for render nodes, or > - drop the DRM_AUTH for fd <> handle imports > > Pointing an alternative solution, for kernel developers to analyse and > make a decision. > > > > > In android 8.1 the hardware composer runs in a seperate process and it has > > to use the card node and be drm master (to use the KMS API), > > therefore, when the surface flinger calls > > renderonly_create_kms_dumb_buffer_for_resource() it is not authenticated. > > > > Making surface flinger use a render node fixes the problem for > > DRM_IOCTL_PRIME_FD_TO_HANDLE (because that already has DRM_RENDER_ALLOW), > > but DRM_IOCTL_MODE_CREATE_DUMB now fails without the patch. > > > > > > This probably worked in previous versions of Android where surface flinger > > and hwc were all in the same process. > > > There has been varying hacks for Android through the years. Bringing > details into the discussion will result in a significant diversion. > Something we could avoid, for the time being ;-) Did someone say diversion?!? The way this was handled prior to using render/control nodes in drm_hwc/[drm/gbm]_gralloc is that all modesetting was done via gralloc which was master. The hwc implementation was basically a proxy backchanneling all of the work to gralloc. Anyways, we probably don't want to go back there. Fwiw, I'd lean towards allowing DUMB allocation from the render nodes. I understand it limits use cases that are undesirable, but it is also limiting usecases that are desirable. So, given that people are going to get "creative" regardless of how many safety railings we put up, we shouldn't make things unnecessarily hard on other trying to Get Work Done. Sean [Disclaimer: I'm totally and completely biased on this issue] > > Thanks > Emil -- Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes 2018-08-03 19:50 ` Sean Paul @ 2018-08-06 19:05 ` Rob Herring -1 siblings, 0 replies; 19+ messages in thread From: Rob Herring @ 2018-08-06 19:05 UTC (permalink / raw) To: Sean Paul Cc: Emil Velikov, martin.fuzzey, Robert Foss, David Airlie, Brian Paul, dri-devel, eric.engestrom, Gustavo Padovan, linux-kernel, Maarten Lankhorst, norvez, Tomasz Figa, Tomeu Vizoso On Fri, Aug 3, 2018 at 1:50 PM Sean Paul <seanpaul@chromium.org> wrote: > > On Fri, Aug 03, 2018 at 06:03:50PM +0100, Emil Velikov wrote: > > On 3 August 2018 at 16:06, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: > > > Hi Emil, > > > > > > On 03/08/18 14:35, Emil Velikov wrote: > > >> > > >> Hi Martin, > > >> > > >> On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> > > >> wrote: > > >> > > >> Let's start with the not-so obvious question: > > >> Why does one open the imx as render node? > > >> > > >> Of the top of my head: > > >> There is nothing in egl/android that should require an authenticated > > >> device. > > >> Hence, using a card node should be fine - the etnaviv code opens the > > >> render node it needs. > > > > > > > > > Yes, the problem is not in egl/android but in the scanout buffer allocation > > > code. > > > > > > etnaviv opens the render node on the *GPU* (for submitting GPU commands), > > > that part is fine. > > > > > > But scanout buffers need to be allocated from imx-drm not etnaviv. > > > > > > This done by renderonly_create_kms_dumb_buffer_for_resource() > > > [src/gallium/auxiliary/renderonly/renderonly.c] > > > Which uses DRM_IOCTL_MODE_CREATE_DUMB followed by > > > DRM_IOCTL_PRIME_FD_TO_HANDLE > > > on the "kms_fd" (probably poorly named because it's not actually used for > > > modesetting) > > > see imx_drm_screen_create()[ src/gallium/winsys/imx/drm/imx_drm_winsys.c] > > > > > > > > > If the card node is used DRM_IOCTL_MODE_CREATE_DUMB works but > > > DRM_IOCTL_PRIME_FD_TO_HANDLE fails, because the permissions are > > > DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW > > > > > Right I missed the DRM_AUTH, in the fd <> handle IOCTLs. > > So in order for things to work, we'd need to either: > > - allow dumb buffers for render nodes, or > > - drop the DRM_AUTH for fd <> handle imports > > > > Pointing an alternative solution, for kernel developers to analyse and > > make a decision. > > > > > > > > In android 8.1 the hardware composer runs in a seperate process and it has > > > to use the card node and be drm master (to use the KMS API), > > > therefore, when the surface flinger calls > > > renderonly_create_kms_dumb_buffer_for_resource() it is not authenticated. > > > > > > Making surface flinger use a render node fixes the problem for > > > DRM_IOCTL_PRIME_FD_TO_HANDLE (because that already has DRM_RENDER_ALLOW), > > > but DRM_IOCTL_MODE_CREATE_DUMB now fails without the patch. > > > > > > > > > This probably worked in previous versions of Android where surface flinger > > > and hwc were all in the same process. > > > > > There has been varying hacks for Android through the years. Bringing > > details into the discussion will result in a significant diversion. > > Something we could avoid, for the time being ;-) > > Did someone say diversion?!? The way this was handled prior to using > render/control nodes in drm_hwc/[drm/gbm]_gralloc is that all modesetting was > done via gralloc which was master. The hwc implementation was basically a proxy > backchanneling all of the work to gralloc. > > Anyways, we probably don't want to go back there. > > Fwiw, I'd lean towards allowing DUMB allocation from the render nodes. I > understand it limits use cases that are undesirable, but it is also limiting > usecases that are desirable. So, given that people are going to get "creative" > regardless of how many safety railings we put up, we shouldn't make things > unnecessarily hard on other trying to Get Work Done. The problem with using render nodes is what if there isn't one? We require VGEM (and make VGEM allow dumb buffers) in that case? Rob ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes @ 2018-08-06 19:05 ` Rob Herring 0 siblings, 0 replies; 19+ messages in thread From: Rob Herring @ 2018-08-06 19:05 UTC (permalink / raw) To: Sean Paul Cc: Tomeu Vizoso, norvez, Robert Foss, Emil Velikov, linux-kernel, dri-devel, Tomasz Figa, eric.engestrom, David Airlie, Brian Paul, martin.fuzzey On Fri, Aug 3, 2018 at 1:50 PM Sean Paul <seanpaul@chromium.org> wrote: > > On Fri, Aug 03, 2018 at 06:03:50PM +0100, Emil Velikov wrote: > > On 3 August 2018 at 16:06, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: > > > Hi Emil, > > > > > > On 03/08/18 14:35, Emil Velikov wrote: > > >> > > >> Hi Martin, > > >> > > >> On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> > > >> wrote: > > >> > > >> Let's start with the not-so obvious question: > > >> Why does one open the imx as render node? > > >> > > >> Of the top of my head: > > >> There is nothing in egl/android that should require an authenticated > > >> device. > > >> Hence, using a card node should be fine - the etnaviv code opens the > > >> render node it needs. > > > > > > > > > Yes, the problem is not in egl/android but in the scanout buffer allocation > > > code. > > > > > > etnaviv opens the render node on the *GPU* (for submitting GPU commands), > > > that part is fine. > > > > > > But scanout buffers need to be allocated from imx-drm not etnaviv. > > > > > > This done by renderonly_create_kms_dumb_buffer_for_resource() > > > [src/gallium/auxiliary/renderonly/renderonly.c] > > > Which uses DRM_IOCTL_MODE_CREATE_DUMB followed by > > > DRM_IOCTL_PRIME_FD_TO_HANDLE > > > on the "kms_fd" (probably poorly named because it's not actually used for > > > modesetting) > > > see imx_drm_screen_create()[ src/gallium/winsys/imx/drm/imx_drm_winsys.c] > > > > > > > > > If the card node is used DRM_IOCTL_MODE_CREATE_DUMB works but > > > DRM_IOCTL_PRIME_FD_TO_HANDLE fails, because the permissions are > > > DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW > > > > > Right I missed the DRM_AUTH, in the fd <> handle IOCTLs. > > So in order for things to work, we'd need to either: > > - allow dumb buffers for render nodes, or > > - drop the DRM_AUTH for fd <> handle imports > > > > Pointing an alternative solution, for kernel developers to analyse and > > make a decision. > > > > > > > > In android 8.1 the hardware composer runs in a seperate process and it has > > > to use the card node and be drm master (to use the KMS API), > > > therefore, when the surface flinger calls > > > renderonly_create_kms_dumb_buffer_for_resource() it is not authenticated. > > > > > > Making surface flinger use a render node fixes the problem for > > > DRM_IOCTL_PRIME_FD_TO_HANDLE (because that already has DRM_RENDER_ALLOW), > > > but DRM_IOCTL_MODE_CREATE_DUMB now fails without the patch. > > > > > > > > > This probably worked in previous versions of Android where surface flinger > > > and hwc were all in the same process. > > > > > There has been varying hacks for Android through the years. Bringing > > details into the discussion will result in a significant diversion. > > Something we could avoid, for the time being ;-) > > Did someone say diversion?!? The way this was handled prior to using > render/control nodes in drm_hwc/[drm/gbm]_gralloc is that all modesetting was > done via gralloc which was master. The hwc implementation was basically a proxy > backchanneling all of the work to gralloc. > > Anyways, we probably don't want to go back there. > > Fwiw, I'd lean towards allowing DUMB allocation from the render nodes. I > understand it limits use cases that are undesirable, but it is also limiting > usecases that are desirable. So, given that people are going to get "creative" > regardless of how many safety railings we put up, we shouldn't make things > unnecessarily hard on other trying to Get Work Done. The problem with using render nodes is what if there isn't one? We require VGEM (and make VGEM allow dumb buffers) in that case? Rob _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes 2018-08-06 19:05 ` Rob Herring (?) @ 2018-08-07 9:18 ` Martin Fuzzey -1 siblings, 0 replies; 19+ messages in thread From: Martin Fuzzey @ 2018-08-07 9:18 UTC (permalink / raw) To: Rob Herring, Sean Paul Cc: Emil Velikov, Robert Foss, David Airlie, Brian Paul, dri-devel, eric.engestrom, Gustavo Padovan, linux-kernel, Maarten Lankhorst, norvez, Tomasz Figa, Tomeu Vizoso On 06/08/18 21:05, Rob Herring wrote: > On Fri, Aug 3, 2018 at 1:50 PM Sean Paul <seanpaul@chromium.org> wrote: >> Fwiw, I'd lean towards allowing DUMB allocation from the render nodes. I >> understand it limits use cases that are undesirable, but it is also limiting >> usecases that are desirable. So, given that people are going to get "creative" >> regardless of how many safety railings we put up, we shouldn't make things >> unnecessarily hard on other trying to Get Work Done. > The problem with using render nodes is what if there isn't one? We > require VGEM (and make VGEM allow dumb buffers) in that case? Try to open the render node and fall back to the card node if it doesn't exist? AFAICT VGEM doesn't provide contiguous buffers so it won't work for the imx-drm case. Regards, Martin ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes 2018-08-03 19:50 ` Sean Paul @ 2018-08-07 11:01 ` Emil Velikov -1 siblings, 0 replies; 19+ messages in thread From: Emil Velikov @ 2018-08-07 11:01 UTC (permalink / raw) To: Sean Paul Cc: Martin Fuzzey, Robert Foss, David Airlie, Brian Paul, ML dri-devel, Eric Engestrom, Gustavo Padovan, Linux-Kernel@Vger. Kernel. Org, Maarten Lankhorst, Nicolas Norvez, Rob Herring, Tomasz Figa, Tomeu Vizoso On 3 August 2018 at 20:50, Sean Paul <seanpaul@chromium.org> wrote: > On Fri, Aug 03, 2018 at 06:03:50PM +0100, Emil Velikov wrote: >> On 3 August 2018 at 16:06, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: >> > Hi Emil, >> > >> > On 03/08/18 14:35, Emil Velikov wrote: >> >> >> >> Hi Martin, >> >> >> >> On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> >> >> wrote: >> >> >> >> Let's start with the not-so obvious question: >> >> Why does one open the imx as render node? >> >> >> >> Of the top of my head: >> >> There is nothing in egl/android that should require an authenticated >> >> device. >> >> Hence, using a card node should be fine - the etnaviv code opens the >> >> render node it needs. >> > >> > >> > Yes, the problem is not in egl/android but in the scanout buffer allocation >> > code. >> > >> > etnaviv opens the render node on the *GPU* (for submitting GPU commands), >> > that part is fine. >> > >> > But scanout buffers need to be allocated from imx-drm not etnaviv. >> > >> > This done by renderonly_create_kms_dumb_buffer_for_resource() >> > [src/gallium/auxiliary/renderonly/renderonly.c] >> > Which uses DRM_IOCTL_MODE_CREATE_DUMB followed by >> > DRM_IOCTL_PRIME_FD_TO_HANDLE >> > on the "kms_fd" (probably poorly named because it's not actually used for >> > modesetting) >> > see imx_drm_screen_create()[ src/gallium/winsys/imx/drm/imx_drm_winsys.c] >> > >> > >> > If the card node is used DRM_IOCTL_MODE_CREATE_DUMB works but >> > DRM_IOCTL_PRIME_FD_TO_HANDLE fails, because the permissions are >> > DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW >> > >> Right I missed the DRM_AUTH, in the fd <> handle IOCTLs. >> So in order for things to work, we'd need to either: >> - allow dumb buffers for render nodes, or >> - drop the DRM_AUTH for fd <> handle imports >> >> Pointing an alternative solution, for kernel developers to analyse and >> make a decision. >> >> > >> > In android 8.1 the hardware composer runs in a seperate process and it has >> > to use the card node and be drm master (to use the KMS API), >> > therefore, when the surface flinger calls >> > renderonly_create_kms_dumb_buffer_for_resource() it is not authenticated. >> > >> > Making surface flinger use a render node fixes the problem for >> > DRM_IOCTL_PRIME_FD_TO_HANDLE (because that already has DRM_RENDER_ALLOW), >> > but DRM_IOCTL_MODE_CREATE_DUMB now fails without the patch. >> > >> > >> > This probably worked in previous versions of Android where surface flinger >> > and hwc were all in the same process. >> > >> There has been varying hacks for Android through the years. Bringing >> details into the discussion will result in a significant diversion. >> Something we could avoid, for the time being ;-) > > Did someone say diversion?!? The way this was handled prior to using > render/control nodes in drm_hwc/[drm/gbm]_gralloc is that all modesetting was > done via gralloc which was master. The hwc implementation was basically a proxy > backchanneling all of the work to gralloc. > > Anyways, we probably don't want to go back there. > Now that we got the diversion out of the way, any input on my proposal to drop the DRM_AUTH for fd <> imports. Am I missing something pretty obvious that makes the idea a no-go? Thanks Emil ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes @ 2018-08-07 11:01 ` Emil Velikov 0 siblings, 0 replies; 19+ messages in thread From: Emil Velikov @ 2018-08-07 11:01 UTC (permalink / raw) To: Sean Paul Cc: Tomeu Vizoso, Nicolas Norvez, Robert Foss, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, Tomasz Figa, Eric Engestrom, David Airlie, Brian Paul, Martin Fuzzey On 3 August 2018 at 20:50, Sean Paul <seanpaul@chromium.org> wrote: > On Fri, Aug 03, 2018 at 06:03:50PM +0100, Emil Velikov wrote: >> On 3 August 2018 at 16:06, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: >> > Hi Emil, >> > >> > On 03/08/18 14:35, Emil Velikov wrote: >> >> >> >> Hi Martin, >> >> >> >> On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> >> >> wrote: >> >> >> >> Let's start with the not-so obvious question: >> >> Why does one open the imx as render node? >> >> >> >> Of the top of my head: >> >> There is nothing in egl/android that should require an authenticated >> >> device. >> >> Hence, using a card node should be fine - the etnaviv code opens the >> >> render node it needs. >> > >> > >> > Yes, the problem is not in egl/android but in the scanout buffer allocation >> > code. >> > >> > etnaviv opens the render node on the *GPU* (for submitting GPU commands), >> > that part is fine. >> > >> > But scanout buffers need to be allocated from imx-drm not etnaviv. >> > >> > This done by renderonly_create_kms_dumb_buffer_for_resource() >> > [src/gallium/auxiliary/renderonly/renderonly.c] >> > Which uses DRM_IOCTL_MODE_CREATE_DUMB followed by >> > DRM_IOCTL_PRIME_FD_TO_HANDLE >> > on the "kms_fd" (probably poorly named because it's not actually used for >> > modesetting) >> > see imx_drm_screen_create()[ src/gallium/winsys/imx/drm/imx_drm_winsys.c] >> > >> > >> > If the card node is used DRM_IOCTL_MODE_CREATE_DUMB works but >> > DRM_IOCTL_PRIME_FD_TO_HANDLE fails, because the permissions are >> > DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW >> > >> Right I missed the DRM_AUTH, in the fd <> handle IOCTLs. >> So in order for things to work, we'd need to either: >> - allow dumb buffers for render nodes, or >> - drop the DRM_AUTH for fd <> handle imports >> >> Pointing an alternative solution, for kernel developers to analyse and >> make a decision. >> >> > >> > In android 8.1 the hardware composer runs in a seperate process and it has >> > to use the card node and be drm master (to use the KMS API), >> > therefore, when the surface flinger calls >> > renderonly_create_kms_dumb_buffer_for_resource() it is not authenticated. >> > >> > Making surface flinger use a render node fixes the problem for >> > DRM_IOCTL_PRIME_FD_TO_HANDLE (because that already has DRM_RENDER_ALLOW), >> > but DRM_IOCTL_MODE_CREATE_DUMB now fails without the patch. >> > >> > >> > This probably worked in previous versions of Android where surface flinger >> > and hwc were all in the same process. >> > >> There has been varying hacks for Android through the years. Bringing >> details into the discussion will result in a significant diversion. >> Something we could avoid, for the time being ;-) > > Did someone say diversion?!? The way this was handled prior to using > render/control nodes in drm_hwc/[drm/gbm]_gralloc is that all modesetting was > done via gralloc which was master. The hwc implementation was basically a proxy > backchanneling all of the work to gralloc. > > Anyways, we probably don't want to go back there. > Now that we got the diversion out of the way, any input on my proposal to drop the DRM_AUTH for fd <> imports. Am I missing something pretty obvious that makes the idea a no-go? Thanks Emil _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes 2018-08-07 11:01 ` Emil Velikov @ 2018-08-07 12:28 ` Daniel Vetter -1 siblings, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2018-08-07 12:28 UTC (permalink / raw) To: Emil Velikov Cc: Sean Paul, Tomeu Vizoso, Nicolas Norvez, Robert Foss, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, Tomasz Figa, Eric Engestrom, David Airlie, Brian Paul, Martin Fuzzey On Tue, Aug 07, 2018 at 12:01:50PM +0100, Emil Velikov wrote: > On 3 August 2018 at 20:50, Sean Paul <seanpaul@chromium.org> wrote: > > On Fri, Aug 03, 2018 at 06:03:50PM +0100, Emil Velikov wrote: > >> On 3 August 2018 at 16:06, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: > >> > Hi Emil, > >> > > >> > On 03/08/18 14:35, Emil Velikov wrote: > >> >> > >> >> Hi Martin, > >> >> > >> >> On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> > >> >> wrote: > >> >> > >> >> Let's start with the not-so obvious question: > >> >> Why does one open the imx as render node? > >> >> > >> >> Of the top of my head: > >> >> There is nothing in egl/android that should require an authenticated > >> >> device. > >> >> Hence, using a card node should be fine - the etnaviv code opens the > >> >> render node it needs. > >> > > >> > > >> > Yes, the problem is not in egl/android but in the scanout buffer allocation > >> > code. > >> > > >> > etnaviv opens the render node on the *GPU* (for submitting GPU commands), > >> > that part is fine. > >> > > >> > But scanout buffers need to be allocated from imx-drm not etnaviv. > >> > > >> > This done by renderonly_create_kms_dumb_buffer_for_resource() > >> > [src/gallium/auxiliary/renderonly/renderonly.c] > >> > Which uses DRM_IOCTL_MODE_CREATE_DUMB followed by > >> > DRM_IOCTL_PRIME_FD_TO_HANDLE > >> > on the "kms_fd" (probably poorly named because it's not actually used for > >> > modesetting) > >> > see imx_drm_screen_create()[ src/gallium/winsys/imx/drm/imx_drm_winsys.c] > >> > > >> > > >> > If the card node is used DRM_IOCTL_MODE_CREATE_DUMB works but > >> > DRM_IOCTL_PRIME_FD_TO_HANDLE fails, because the permissions are > >> > DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW > >> > > >> Right I missed the DRM_AUTH, in the fd <> handle IOCTLs. > >> So in order for things to work, we'd need to either: > >> - allow dumb buffers for render nodes, or > >> - drop the DRM_AUTH for fd <> handle imports > >> > >> Pointing an alternative solution, for kernel developers to analyse and > >> make a decision. > >> > >> > > >> > In android 8.1 the hardware composer runs in a seperate process and it has > >> > to use the card node and be drm master (to use the KMS API), > >> > therefore, when the surface flinger calls > >> > renderonly_create_kms_dumb_buffer_for_resource() it is not authenticated. > >> > > >> > Making surface flinger use a render node fixes the problem for > >> > DRM_IOCTL_PRIME_FD_TO_HANDLE (because that already has DRM_RENDER_ALLOW), > >> > but DRM_IOCTL_MODE_CREATE_DUMB now fails without the patch. > >> > > >> > > >> > This probably worked in previous versions of Android where surface flinger > >> > and hwc were all in the same process. > >> > > >> There has been varying hacks for Android through the years. Bringing > >> details into the discussion will result in a significant diversion. > >> Something we could avoid, for the time being ;-) > > > > Did someone say diversion?!? The way this was handled prior to using > > render/control nodes in drm_hwc/[drm/gbm]_gralloc is that all modesetting was > > done via gralloc which was master. The hwc implementation was basically a proxy > > backchanneling all of the work to gralloc. > > > > Anyways, we probably don't want to go back there. > > > Now that we got the diversion out of the way, any input on my proposal > to drop the DRM_AUTH for fd <> imports. > Am I missing something pretty obvious that makes the idea a no-go? Dropping DRM_AUTH is only relevant for the card node. And a card node might not be sufficiently isolated against concurrent other clients, which is why we don't allow it. What we could do is essentially check whether your driver supports render nodes (indicating sufficient amounts of separation), and then allow anything for unauthenicated clients if DRM_RENDER_ALLOW is set on the ioctl. But that's just reinventing render nodes on top of legacy card nodes, and I'm not clear on what that exactly gains us. I think the proposal for dumb render nodes (for drivers which only do dumb kms buffers and no rendering at all) that's been discusson on irc a bit makes a lot more sense. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes @ 2018-08-07 12:28 ` Daniel Vetter 0 siblings, 0 replies; 19+ messages in thread From: Daniel Vetter @ 2018-08-07 12:28 UTC (permalink / raw) To: Emil Velikov Cc: Tomeu Vizoso, Nicolas Norvez, Robert Foss, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, Tomasz Figa, Eric Engestrom, David Airlie, Martin Fuzzey, Brian Paul On Tue, Aug 07, 2018 at 12:01:50PM +0100, Emil Velikov wrote: > On 3 August 2018 at 20:50, Sean Paul <seanpaul@chromium.org> wrote: > > On Fri, Aug 03, 2018 at 06:03:50PM +0100, Emil Velikov wrote: > >> On 3 August 2018 at 16:06, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: > >> > Hi Emil, > >> > > >> > On 03/08/18 14:35, Emil Velikov wrote: > >> >> > >> >> Hi Martin, > >> >> > >> >> On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> > >> >> wrote: > >> >> > >> >> Let's start with the not-so obvious question: > >> >> Why does one open the imx as render node? > >> >> > >> >> Of the top of my head: > >> >> There is nothing in egl/android that should require an authenticated > >> >> device. > >> >> Hence, using a card node should be fine - the etnaviv code opens the > >> >> render node it needs. > >> > > >> > > >> > Yes, the problem is not in egl/android but in the scanout buffer allocation > >> > code. > >> > > >> > etnaviv opens the render node on the *GPU* (for submitting GPU commands), > >> > that part is fine. > >> > > >> > But scanout buffers need to be allocated from imx-drm not etnaviv. > >> > > >> > This done by renderonly_create_kms_dumb_buffer_for_resource() > >> > [src/gallium/auxiliary/renderonly/renderonly.c] > >> > Which uses DRM_IOCTL_MODE_CREATE_DUMB followed by > >> > DRM_IOCTL_PRIME_FD_TO_HANDLE > >> > on the "kms_fd" (probably poorly named because it's not actually used for > >> > modesetting) > >> > see imx_drm_screen_create()[ src/gallium/winsys/imx/drm/imx_drm_winsys.c] > >> > > >> > > >> > If the card node is used DRM_IOCTL_MODE_CREATE_DUMB works but > >> > DRM_IOCTL_PRIME_FD_TO_HANDLE fails, because the permissions are > >> > DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW > >> > > >> Right I missed the DRM_AUTH, in the fd <> handle IOCTLs. > >> So in order for things to work, we'd need to either: > >> - allow dumb buffers for render nodes, or > >> - drop the DRM_AUTH for fd <> handle imports > >> > >> Pointing an alternative solution, for kernel developers to analyse and > >> make a decision. > >> > >> > > >> > In android 8.1 the hardware composer runs in a seperate process and it has > >> > to use the card node and be drm master (to use the KMS API), > >> > therefore, when the surface flinger calls > >> > renderonly_create_kms_dumb_buffer_for_resource() it is not authenticated. > >> > > >> > Making surface flinger use a render node fixes the problem for > >> > DRM_IOCTL_PRIME_FD_TO_HANDLE (because that already has DRM_RENDER_ALLOW), > >> > but DRM_IOCTL_MODE_CREATE_DUMB now fails without the patch. > >> > > >> > > >> > This probably worked in previous versions of Android where surface flinger > >> > and hwc were all in the same process. > >> > > >> There has been varying hacks for Android through the years. Bringing > >> details into the discussion will result in a significant diversion. > >> Something we could avoid, for the time being ;-) > > > > Did someone say diversion?!? The way this was handled prior to using > > render/control nodes in drm_hwc/[drm/gbm]_gralloc is that all modesetting was > > done via gralloc which was master. The hwc implementation was basically a proxy > > backchanneling all of the work to gralloc. > > > > Anyways, we probably don't want to go back there. > > > Now that we got the diversion out of the way, any input on my proposal > to drop the DRM_AUTH for fd <> imports. > Am I missing something pretty obvious that makes the idea a no-go? Dropping DRM_AUTH is only relevant for the card node. And a card node might not be sufficiently isolated against concurrent other clients, which is why we don't allow it. What we could do is essentially check whether your driver supports render nodes (indicating sufficient amounts of separation), and then allow anything for unauthenicated clients if DRM_RENDER_ALLOW is set on the ioctl. But that's just reinventing render nodes on top of legacy card nodes, and I'm not clear on what that exactly gains us. I think the proposal for dumb render nodes (for drivers which only do dumb kms buffers and no rendering at all) that's been discusson on irc a bit makes a lot more sense. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes 2018-08-07 12:28 ` Daniel Vetter @ 2018-08-07 13:26 ` Emil Velikov -1 siblings, 0 replies; 19+ messages in thread From: Emil Velikov @ 2018-08-07 13:26 UTC (permalink / raw) To: Emil Velikov, Sean Paul, Tomeu Vizoso, Nicolas Norvez, Robert Foss, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, Tomasz Figa, Eric Engestrom, David Airlie, Brian Paul, Martin Fuzzey On 7 August 2018 at 13:28, Daniel Vetter <daniel@ffwll.ch> wrote: > On Tue, Aug 07, 2018 at 12:01:50PM +0100, Emil Velikov wrote: >> On 3 August 2018 at 20:50, Sean Paul <seanpaul@chromium.org> wrote: >> > On Fri, Aug 03, 2018 at 06:03:50PM +0100, Emil Velikov wrote: >> >> On 3 August 2018 at 16:06, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: >> >> > Hi Emil, >> >> > >> >> > On 03/08/18 14:35, Emil Velikov wrote: >> >> >> >> >> >> Hi Martin, >> >> >> >> >> >> On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> >> >> >> wrote: >> >> >> >> >> >> Let's start with the not-so obvious question: >> >> >> Why does one open the imx as render node? >> >> >> >> >> >> Of the top of my head: >> >> >> There is nothing in egl/android that should require an authenticated >> >> >> device. >> >> >> Hence, using a card node should be fine - the etnaviv code opens the >> >> >> render node it needs. >> >> > >> >> > >> >> > Yes, the problem is not in egl/android but in the scanout buffer allocation >> >> > code. >> >> > >> >> > etnaviv opens the render node on the *GPU* (for submitting GPU commands), >> >> > that part is fine. >> >> > >> >> > But scanout buffers need to be allocated from imx-drm not etnaviv. >> >> > >> >> > This done by renderonly_create_kms_dumb_buffer_for_resource() >> >> > [src/gallium/auxiliary/renderonly/renderonly.c] >> >> > Which uses DRM_IOCTL_MODE_CREATE_DUMB followed by >> >> > DRM_IOCTL_PRIME_FD_TO_HANDLE >> >> > on the "kms_fd" (probably poorly named because it's not actually used for >> >> > modesetting) >> >> > see imx_drm_screen_create()[ src/gallium/winsys/imx/drm/imx_drm_winsys.c] >> >> > >> >> > >> >> > If the card node is used DRM_IOCTL_MODE_CREATE_DUMB works but >> >> > DRM_IOCTL_PRIME_FD_TO_HANDLE fails, because the permissions are >> >> > DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW >> >> > >> >> Right I missed the DRM_AUTH, in the fd <> handle IOCTLs. >> >> So in order for things to work, we'd need to either: >> >> - allow dumb buffers for render nodes, or >> >> - drop the DRM_AUTH for fd <> handle imports >> >> >> >> Pointing an alternative solution, for kernel developers to analyse and >> >> make a decision. >> >> >> >> > >> >> > In android 8.1 the hardware composer runs in a seperate process and it has >> >> > to use the card node and be drm master (to use the KMS API), >> >> > therefore, when the surface flinger calls >> >> > renderonly_create_kms_dumb_buffer_for_resource() it is not authenticated. >> >> > >> >> > Making surface flinger use a render node fixes the problem for >> >> > DRM_IOCTL_PRIME_FD_TO_HANDLE (because that already has DRM_RENDER_ALLOW), >> >> > but DRM_IOCTL_MODE_CREATE_DUMB now fails without the patch. >> >> > >> >> > >> >> > This probably worked in previous versions of Android where surface flinger >> >> > and hwc were all in the same process. >> >> > >> >> There has been varying hacks for Android through the years. Bringing >> >> details into the discussion will result in a significant diversion. >> >> Something we could avoid, for the time being ;-) >> > >> > Did someone say diversion?!? The way this was handled prior to using >> > render/control nodes in drm_hwc/[drm/gbm]_gralloc is that all modesetting was >> > done via gralloc which was master. The hwc implementation was basically a proxy >> > backchanneling all of the work to gralloc. >> > >> > Anyways, we probably don't want to go back there. >> > >> Now that we got the diversion out of the way, any input on my proposal >> to drop the DRM_AUTH for fd <> imports. >> Am I missing something pretty obvious that makes the idea a no-go? > > Dropping DRM_AUTH is only relevant for the card node. And a card node > might not be sufficiently isolated against concurrent other clients, which > is why we don't allow it. > Right. I did not spot anything that would make a distinction based on the card vs render node used. > What we could do is essentially check whether your driver supports render > nodes (indicating sufficient amounts of separation), and then allow > anything for unauthenicated clients if DRM_RENDER_ALLOW is set on the > ioctl. > > But that's just reinventing render nodes on top of legacy card nodes, and > I'm not clear on what that exactly gains us. > As more of a userspace person, it makes sense to keep render nodes for GPU specifics and card ones - KMS/Display Controller. > I think the proposal for dumb render nodes (for drivers which only do dumb > kms buffers and no rendering at all) that's been discusson on irc a bit > makes a lot more sense. Ack. Thanks for shedding some light. -Emil ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes @ 2018-08-07 13:26 ` Emil Velikov 0 siblings, 0 replies; 19+ messages in thread From: Emil Velikov @ 2018-08-07 13:26 UTC (permalink / raw) To: Emil Velikov, Sean Paul, Tomeu Vizoso, Nicolas Norvez, Robert Foss, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, Tomasz Figa, Eric Engestrom, David Airlie, Brian Paul, Martin Fuzzey On 7 August 2018 at 13:28, Daniel Vetter <daniel@ffwll.ch> wrote: > On Tue, Aug 07, 2018 at 12:01:50PM +0100, Emil Velikov wrote: >> On 3 August 2018 at 20:50, Sean Paul <seanpaul@chromium.org> wrote: >> > On Fri, Aug 03, 2018 at 06:03:50PM +0100, Emil Velikov wrote: >> >> On 3 August 2018 at 16:06, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: >> >> > Hi Emil, >> >> > >> >> > On 03/08/18 14:35, Emil Velikov wrote: >> >> >> >> >> >> Hi Martin, >> >> >> >> >> >> On 1 August 2018 at 15:24, Martin Fuzzey <martin.fuzzey@flowbird.group> >> >> >> wrote: >> >> >> >> >> >> Let's start with the not-so obvious question: >> >> >> Why does one open the imx as render node? >> >> >> >> >> >> Of the top of my head: >> >> >> There is nothing in egl/android that should require an authenticated >> >> >> device. >> >> >> Hence, using a card node should be fine - the etnaviv code opens the >> >> >> render node it needs. >> >> > >> >> > >> >> > Yes, the problem is not in egl/android but in the scanout buffer allocation >> >> > code. >> >> > >> >> > etnaviv opens the render node on the *GPU* (for submitting GPU commands), >> >> > that part is fine. >> >> > >> >> > But scanout buffers need to be allocated from imx-drm not etnaviv. >> >> > >> >> > This done by renderonly_create_kms_dumb_buffer_for_resource() >> >> > [src/gallium/auxiliary/renderonly/renderonly.c] >> >> > Which uses DRM_IOCTL_MODE_CREATE_DUMB followed by >> >> > DRM_IOCTL_PRIME_FD_TO_HANDLE >> >> > on the "kms_fd" (probably poorly named because it's not actually used for >> >> > modesetting) >> >> > see imx_drm_screen_create()[ src/gallium/winsys/imx/drm/imx_drm_winsys.c] >> >> > >> >> > >> >> > If the card node is used DRM_IOCTL_MODE_CREATE_DUMB works but >> >> > DRM_IOCTL_PRIME_FD_TO_HANDLE fails, because the permissions are >> >> > DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW >> >> > >> >> Right I missed the DRM_AUTH, in the fd <> handle IOCTLs. >> >> So in order for things to work, we'd need to either: >> >> - allow dumb buffers for render nodes, or >> >> - drop the DRM_AUTH for fd <> handle imports >> >> >> >> Pointing an alternative solution, for kernel developers to analyse and >> >> make a decision. >> >> >> >> > >> >> > In android 8.1 the hardware composer runs in a seperate process and it has >> >> > to use the card node and be drm master (to use the KMS API), >> >> > therefore, when the surface flinger calls >> >> > renderonly_create_kms_dumb_buffer_for_resource() it is not authenticated. >> >> > >> >> > Making surface flinger use a render node fixes the problem for >> >> > DRM_IOCTL_PRIME_FD_TO_HANDLE (because that already has DRM_RENDER_ALLOW), >> >> > but DRM_IOCTL_MODE_CREATE_DUMB now fails without the patch. >> >> > >> >> > >> >> > This probably worked in previous versions of Android where surface flinger >> >> > and hwc were all in the same process. >> >> > >> >> There has been varying hacks for Android through the years. Bringing >> >> details into the discussion will result in a significant diversion. >> >> Something we could avoid, for the time being ;-) >> > >> > Did someone say diversion?!? The way this was handled prior to using >> > render/control nodes in drm_hwc/[drm/gbm]_gralloc is that all modesetting was >> > done via gralloc which was master. The hwc implementation was basically a proxy >> > backchanneling all of the work to gralloc. >> > >> > Anyways, we probably don't want to go back there. >> > >> Now that we got the diversion out of the way, any input on my proposal >> to drop the DRM_AUTH for fd <> imports. >> Am I missing something pretty obvious that makes the idea a no-go? > > Dropping DRM_AUTH is only relevant for the card node. And a card node > might not be sufficiently isolated against concurrent other clients, which > is why we don't allow it. > Right. I did not spot anything that would make a distinction based on the card vs render node used. > What we could do is essentially check whether your driver supports render > nodes (indicating sufficient amounts of separation), and then allow > anything for unauthenicated clients if DRM_RENDER_ALLOW is set on the > ioctl. > > But that's just reinventing render nodes on top of legacy card nodes, and > I'm not clear on what that exactly gains us. > As more of a userspace person, it makes sense to keep render nodes for GPU specifics and card ones - KMS/Display Controller. > I think the proposal for dumb render nodes (for drivers which only do dumb > kms buffers and no rendering at all) that's been discusson on irc a bit > makes a lot more sense. Ack. Thanks for shedding some light. -Emil _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes 2018-07-24 8:22 [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes Robert Foss @ 2018-08-03 12:25 ` Emil Velikov 2018-08-03 12:25 ` Emil Velikov 1 sibling, 0 replies; 19+ messages in thread From: Emil Velikov @ 2018-08-03 12:25 UTC (permalink / raw) To: Robert Foss Cc: Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie, ML dri-devel, Linux-Kernel@Vger. Kernel. Org, Tomasz Figa, Rob Herring, Eric Engestrom, Brian Paul, Tomeu Vizoso, Nicolas Norvez Hi all, Sharing some ideas on the topic. Personally I'm neither for, nor against this patch. On 24 July 2018 at 09:22, Robert Foss <robert.foss@collabora.com> wrote: > From: Tomasz Figa <tfiga@chromium.org> > > There is no particular reason to prevent userspace for using this IOCTL, > considering that it already has access to other, platform-specific > IOCTLs. This patch makes is possible to have the same userspace code > work regardless on the underlying platform, which significantly > simplifies the stack. > > Signed-off-by: Tomasz Figa <tfiga@chromium.org> > Reviewed-by: Zach Reizner <zachr@chromium.org> > Signed-off-by: Nicolas Norvez <norvez@chromium.org> > Reviewed-by: Tomasz Figa <tfiga@chromium.org> > Signed-off-by: Robert Foss <robert.foss@collabora.com> > --- > > I've been looking into enabling a kms_swrast based driver for mesa on > the Android platform[1]. > > But have come up against the issue of dumb buffer related ioctls below > not being flagged with DRM_RENDER_ALLOW. > > DRM_IOCTL_MODE_CREATE_DUMB > DRM_IOCTL_MODE_MAP_DUMB > > To be more precise, I've been seeing a failure due to DRM_IOCTL_MODE_MAP_DUMB > not being allowed for /dev/dri/renderD* nodes, and used in mesa > kms_sw_displaytarget_map(). > > > As I understand it the DRM_RENDER_ALLOW flag being unset is a very intentional > restriction placed on dumb buffers in order to minimize its use. > But as far as alternative solutions for software renderers there seems to only > be VGEM and mmap()-ing DMABUFs. > > While it would be convenient from the point of view of software renderers if > dumb buffers had more promiscuous permissions, it may be a hard sell upstream. > > If dumb buffers aren't the way forward, what is? VGEM? Or are there any other > preferable ways? > The description of VGEM says "... as used by Mesa's software renderer for enhanced performance." Although that hasn't been the case really, since we're opening an arbitrary GPU node with kms_swrast. I think we should fix that. On top of that we could also use the card node, which will remove the need for this patch. Yet again, there may be reasonable usecases where one needs the render node to support the dumb buffer ioctls. HTH Emil ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes @ 2018-08-03 12:25 ` Emil Velikov 0 siblings, 0 replies; 19+ messages in thread From: Emil Velikov @ 2018-08-03 12:25 UTC (permalink / raw) To: Robert Foss Cc: Tomeu Vizoso, Nicolas Norvez, David Airlie, Linux-Kernel@Vger. Kernel. Org, ML dri-devel, Tomasz Figa, Eric Engestrom, Brian Paul Hi all, Sharing some ideas on the topic. Personally I'm neither for, nor against this patch. On 24 July 2018 at 09:22, Robert Foss <robert.foss@collabora.com> wrote: > From: Tomasz Figa <tfiga@chromium.org> > > There is no particular reason to prevent userspace for using this IOCTL, > considering that it already has access to other, platform-specific > IOCTLs. This patch makes is possible to have the same userspace code > work regardless on the underlying platform, which significantly > simplifies the stack. > > Signed-off-by: Tomasz Figa <tfiga@chromium.org> > Reviewed-by: Zach Reizner <zachr@chromium.org> > Signed-off-by: Nicolas Norvez <norvez@chromium.org> > Reviewed-by: Tomasz Figa <tfiga@chromium.org> > Signed-off-by: Robert Foss <robert.foss@collabora.com> > --- > > I've been looking into enabling a kms_swrast based driver for mesa on > the Android platform[1]. > > But have come up against the issue of dumb buffer related ioctls below > not being flagged with DRM_RENDER_ALLOW. > > DRM_IOCTL_MODE_CREATE_DUMB > DRM_IOCTL_MODE_MAP_DUMB > > To be more precise, I've been seeing a failure due to DRM_IOCTL_MODE_MAP_DUMB > not being allowed for /dev/dri/renderD* nodes, and used in mesa > kms_sw_displaytarget_map(). > > > As I understand it the DRM_RENDER_ALLOW flag being unset is a very intentional > restriction placed on dumb buffers in order to minimize its use. > But as far as alternative solutions for software renderers there seems to only > be VGEM and mmap()-ing DMABUFs. > > While it would be convenient from the point of view of software renderers if > dumb buffers had more promiscuous permissions, it may be a hard sell upstream. > > If dumb buffers aren't the way forward, what is? VGEM? Or are there any other > preferable ways? > The description of VGEM says "... as used by Mesa's software renderer for enhanced performance." Although that hasn't been the case really, since we're opening an arbitrary GPU node with kms_swrast. I think we should fix that. On top of that we could also use the card node, which will remove the need for this patch. Yet again, there may be reasonable usecases where one needs the render node to support the dumb buffer ioctls. HTH Emil _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-08-07 13:26 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-24 8:22 [RFC] drm: Allow DRM_IOCTL_MODE_MAP_DUMB for render nodes Robert Foss 2018-08-01 14:24 ` Martin Fuzzey 2018-08-03 12:35 ` Emil Velikov 2018-08-03 15:06 ` Martin Fuzzey 2018-08-03 17:03 ` Emil Velikov 2018-08-03 17:03 ` Emil Velikov 2018-08-03 19:50 ` Sean Paul 2018-08-03 19:50 ` Sean Paul 2018-08-06 19:05 ` Rob Herring 2018-08-06 19:05 ` Rob Herring 2018-08-07 9:18 ` Martin Fuzzey 2018-08-07 11:01 ` Emil Velikov 2018-08-07 11:01 ` Emil Velikov 2018-08-07 12:28 ` Daniel Vetter 2018-08-07 12:28 ` Daniel Vetter 2018-08-07 13:26 ` Emil Velikov 2018-08-07 13:26 ` Emil Velikov 2018-08-03 12:25 ` Emil Velikov 2018-08-03 12:25 ` Emil Velikov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.