| Message ID | 20210610213659.22728-1-dongwon.kim@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: set DRM_RENDER_ALLOW flag on DRM_IOCTL_MODE_CREATE/DESTROY_DUMB ioctls | expand |
On Thu, Jun 10, 2021 at 02:36:59PM -0700, Dongwon Kim wrote: > Render clients should be able to create/destroy dumb object to import > and use it as render buffer in case the default DRM device is different > from the render device (i.e. kmsro). > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> Uh no. Well I know everyone just hacks around this, but the idea behind dumb buffer objects is that they're for kms scanout only. Furthermore on many drivers they allocate a limited resource like CMA memory. Handing that out like candy isn't a great idea. And it's exactly those drivers that kmsro currently is used for where the display driver needs special memory. -Daniel > --- > 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 98ae00661656..f2f72e132741 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -685,9 +685,9 @@ static const struct drm_ioctl_desc drm_ioctls[] = { > DRM_IOCTL_DEF(DRM_IOCTL_MODE_RMFB, drm_mode_rmfb_ioctl, 0), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_PAGE_FLIP, drm_mode_page_flip_ioctl, DRM_MASTER), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_DIRTYFB, drm_mode_dirtyfb_ioctl, DRM_MASTER), > - DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_DUMB, drm_mode_create_dumb_ioctl, 0), > + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_DUMB, drm_mode_create_dumb_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_MAP_DUMB, drm_mode_mmap_dumb_ioctl, 0), > - DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, 0), > + DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_properties_ioctl, 0), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY, drm_mode_obj_set_property_ioctl, DRM_MASTER), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR2, drm_mode_cursor2_ioctl, DRM_MASTER), > -- > 2.20.1 >
On Fri, 11 Jun 2021 at 10:47, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Thu, Jun 10, 2021 at 02:36:59PM -0700, Dongwon Kim wrote: > > Render clients should be able to create/destroy dumb object to import > > and use it as render buffer in case the default DRM device is different > > from the render device (i.e. kmsro). > > > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > > Uh no. > > Well I know everyone just hacks around this, but the idea behind dumb > buffer objects is that they're for kms scanout only. Furthermore on many > drivers they allocate a limited resource like CMA memory. Handing that out > like candy isn't a great idea. > > And it's exactly those drivers that kmsro currently is used for where the > display driver needs special memory. Couldn't agree more. Perhaps we should add an inline comment and/or reference to a thread why? -Emil
Understood. I saw weston client apps were failing to create render buffers from kmsro driver and found it was because they were not allowed to create and destroy dumb objects then I came up with this patch. I just thought it's the simplest solution. I didn't know it violates the rule. I think I should look into kmsro to make the client app to get the render buffer from ro-device instead. Thanks On Fri, Jun 11, 2021 at 11:39:46AM +0100, Emil Velikov wrote: > On Fri, 11 Jun 2021 at 10:47, Daniel Vetter <daniel@ffwll.ch> wrote: > > > > On Thu, Jun 10, 2021 at 02:36:59PM -0700, Dongwon Kim wrote: > > > Render clients should be able to create/destroy dumb object to import > > > and use it as render buffer in case the default DRM device is different > > > from the render device (i.e. kmsro). > > > > > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > > > > Uh no. > > > > Well I know everyone just hacks around this, but the idea behind dumb > > buffer objects is that they're for kms scanout only. Furthermore on many > > drivers they allocate a limited resource like CMA memory. Handing that out > > like candy isn't a great idea. > > > > And it's exactly those drivers that kmsro currently is used for where the > > display driver needs special memory. > > Couldn't agree more. Perhaps we should add an inline comment and/or > reference to a thread why? > > -Emil
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 98ae00661656..f2f72e132741 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -685,9 +685,9 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_MODE_RMFB, drm_mode_rmfb_ioctl, 0), DRM_IOCTL_DEF(DRM_IOCTL_MODE_PAGE_FLIP, drm_mode_page_flip_ioctl, DRM_MASTER), DRM_IOCTL_DEF(DRM_IOCTL_MODE_DIRTYFB, drm_mode_dirtyfb_ioctl, DRM_MASTER), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_DUMB, drm_mode_create_dumb_ioctl, 0), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_DUMB, drm_mode_create_dumb_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_MODE_MAP_DUMB, drm_mode_mmap_dumb_ioctl, 0), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, 0), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_properties_ioctl, 0), DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY, drm_mode_obj_set_property_ioctl, DRM_MASTER), DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR2, drm_mode_cursor2_ioctl, DRM_MASTER),
Render clients should be able to create/destroy dumb object to import and use it as render buffer in case the default DRM device is different from the render device (i.e. kmsro). Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> --- drivers/gpu/drm/drm_ioctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)