On Mon, Apr 27, 2020 at 1:05 PM Peter Collingbourne <pcc@xxxxxxxxxx> wrote: > > Render nodes are not just useful for devices supporting GPU hardware > acceleration. Even on devices that only support dumb frame buffers, > they are useful in situations where composition (using software > rasterization) and KMS are done in different processes with buffer > sharing being used to send frame buffers between them. This is the > situation on Android, where surfaceflinger is the compositor and the > composer HAL uses KMS to display the buffers. Thus it is beneficial > to expose render nodes on all devices that support buffer sharing. > > Because all drivers that currently support render nodes also support > buffer sharing, the DRIVER_RENDER flag is no longer necessary to mark > devices as supporting render nodes, so remove it and just rely on > the presence of a prime_handle_to_fd function pointer to determine > whether buffer sharing is supported. I'm definitely interested in seeing a patch like this land, as I think the current state is an ugly historical artifact. We just have to be careful. Were there any instances of drivers with render engines exposing PRIME but not RENDER? We should be careful to make sure that we're not exposing new privileges for those through adding render nodes. There's a UAPI risk I see here. Right now, on a system with a single renderer GPU, we can just open /dev/dri/renderD128 and get the GPU for rendering, and various things are relying on that (such as libwaffle, used in piglit among other things) Adding render nodes for kms-only drivers could displace the actual GPU to 129, and the question is whether this will be disruptive. For Mesa, I think this works out, because kmsro should load on the kms device's node and then share buffers over to the real GPU that it digs around to find at init time. Just saying, I'm not sure I know all of the userspace well enough to say "this should be safe despite that" (And, maybe, if we decide that it's not safe enough, we could punt kms-only drivers to a higher starting number?) > @@ -260,12 +258,6 @@ static int vc4_drm_bind(struct device *dev) > if (!vc4) > return -ENOMEM; > > - /* If VC4 V3D is missing, don't advertise render nodes. */ > - node = of_find_matching_node_and_match(NULL, vc4_v3d_dt_match, NULL); > - if (!node || !of_device_is_available(node)) > - vc4_drm_driver.driver_features &= ~DRIVER_RENDER; > - of_node_put(node); > - > drm = drm_dev_alloc(&vc4_drm_driver, dev); > if (IS_ERR(drm)) > return PTR_ERR(drm); Looks like dropping this code from vc4 should be fine -- kmsro looks for a render node from each driver name it supports in turn, so even if v3d moves from renderD128 to renderD129, things should still work. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
