Re: [PATCH 04/33] vt: More locking checks
On Fri, May 24, 2019 at 10:53:25AM +0200, Daniel Vetter wrote: > I honestly have no idea what the subtle differences between > con_is_visible, con_is_fg (internal to vt.c) and con_is_bound are. But > it looks like both vc->vc_display_fg and con_driver_map are protected > by the console_lock, so probably better if we hold that when checking > this. > > To do that I had to deinline the con_is_visible function. > > Signed-off-by: Daniel Vetter > Cc: Greg Kroah-Hartman > Cc: Nicolas Pitre > Cc: Martin Hostettler > Cc: Adam Borowski > Cc: Daniel Vetter > Cc: Mikulas Patocka Hi Greg, Do you want to merge this through your console tree or ack for merging through drm/fbdev? It's part of a bigger series, and I'd like to have more testing with this in our trees, but also ok to merge stand-alone if you prefer that. It's just locking checks and some docs. Same for the preceeding patch in this series here. Thanks, Daniel > --- > drivers/tty/vt/vt.c| 16 > include/linux/console_struct.h | 5 + > 2 files changed, 17 insertions(+), 4 deletions(-) > > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c > index bc9813b14c58..a8988a085138 100644 > --- a/drivers/tty/vt/vt.c > +++ b/drivers/tty/vt/vt.c > @@ -3815,6 +3815,8 @@ int con_is_bound(const struct consw *csw) > { > int i, bound = 0; > > + WARN_CONSOLE_UNLOCKED(); > + > for (i = 0; i < MAX_NR_CONSOLES; i++) { > if (con_driver_map[i] == csw) { > bound = 1; > @@ -3826,6 +3828,20 @@ int con_is_bound(const struct consw *csw) > } > EXPORT_SYMBOL(con_is_bound); > > +/** > + * con_is_visible - checks whether the current console is visible > + * @vc: virtual console > + * > + * RETURNS: zero if not visible, nonzero if visible > + */ > +bool con_is_visible(const struct vc_data *vc) > +{ > + WARN_CONSOLE_UNLOCKED(); > + > + return *vc->vc_display_fg == vc; > +} > +EXPORT_SYMBOL(con_is_visible); > + > /** > * con_debug_enter - prepare the console for the kernel debugger > * @sw: console driver > diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h > index ed798e114663..24d4c16e3ae0 100644 > --- a/include/linux/console_struct.h > +++ b/include/linux/console_struct.h > @@ -168,9 +168,6 @@ extern void vc_SAK(struct work_struct *work); > > #define CUR_DEFAULT CUR_UNDERLINE > > -static inline bool con_is_visible(const struct vc_data *vc) > -{ > - return *vc->vc_display_fg == vc; > -} > +bool con_is_visible(const struct vc_data *vc); > > #endif /* _LINUX_CONSOLE_STRUCT_H */ > -- > 2.20.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 14/33] staging/olpc: lock_fb_info can't fail
On Fri, May 24, 2019 at 10:53:35AM +0200, Daniel Vetter wrote: > Simply because olpc never unregisters the damn thing. It also > registers the framebuffer directly by poking around in fbdev > core internals, so it's all around rather broken. > > Signed-off-by: Daniel Vetter > Cc: Jens Frederich > Cc: Daniel Drake > Cc: Jon Nettleton Hi Greg, Somehow get_maintainers didn't pick you up for this. Ack for merging this through drm/fbdev? It's part of a bigger series to rework fbdev/fbcon interactions. Thanks, Daniel > --- > drivers/staging/olpc_dcon/olpc_dcon.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/staging/olpc_dcon/olpc_dcon.c > b/drivers/staging/olpc_dcon/olpc_dcon.c > index 6b714f740ac3..a254238be181 100644 > --- a/drivers/staging/olpc_dcon/olpc_dcon.c > +++ b/drivers/staging/olpc_dcon/olpc_dcon.c > @@ -250,11 +250,7 @@ static bool dcon_blank_fb(struct dcon_priv *dcon, bool > blank) > int err; > > console_lock(); > - if (!lock_fb_info(dcon->fbinfo)) { > - console_unlock(); > - dev_err(&dcon->client->dev, "unable to lock framebuffer\n"); > - return false; > - } > + lock_fb_info(dcon->fbinfo); > > dcon->ignore_fb_events = true; > err = fb_blank(dcon->fbinfo, > -- > 2.20.1 > -- 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
Re: [PATCH 32/33] staging/olpc_dcon: Add drm conversion to TODO
On Fri, May 24, 2019 at 10:53:53AM +0200, Daniel Vetter wrote: > this driver is pretty horrible from a design pov, and needs a complete > overhaul. Concrete thing that annoys me is that it looks at > registered_fb, which is an internal thing to fbmem.c and fbcon.c. And > ofc it gets the lifetime rules all wrong (it should at least use > get/put_fb_info). > > Looking at the history, there's been an attempt at dropping this from > staging in 2016, but that had to be reverted. Since then not real > effort except the usual stream of trivial patches, and fbdev has been > formally closed for any new hw support. Time to try again and drop > this? > > Signed-off-by: Daniel Vetter > Cc: Jens Frederich > Cc: Daniel Drake > Cc: Jon Nettleton Hi Greg Again get_mainatiners didn't pick you up on this somehow (I manually added you now for the next round). Do you want to pick this up to staging, or ack for merging through drm/fbdev as part of the larger fbdev/fbcon rework? Also, I think time to retry and attempt at dropping this imo ... Thanks, Daniel > --- > drivers/staging/olpc_dcon/TODO | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/staging/olpc_dcon/TODO b/drivers/staging/olpc_dcon/TODO > index 665a0b061719..fe09efbc7f77 100644 > --- a/drivers/staging/olpc_dcon/TODO > +++ b/drivers/staging/olpc_dcon/TODO > @@ -1,4 +1,11 @@ > TODO: > + - complete rewrite: > + 1. The underlying fbdev drivers need to be converted into drm kernel > + modesetting drivers. > + 2. The dcon low-power display mode can then be integrated using the > + drm damage tracking and self-refresh helpers. > + This bolted-on self-refresh support that digs around in fbdev > + internals, but isn't properly integrated, is not the correct solution. > - see if vx855 gpio API can be made similar enough to cs5535 so we can > share more code > - convert all uses of the old GPIO API from to the > -- > 2.20.1 > -- 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
dpms mode change with wayland on iMX.6
Dear All, I have a iMX.6 (arm 32) board with Linux Kernel 3.10 and debian platform running. The board is connected to one LCD screen and one HDMI monitor. It have DRM + Wayland setup for display. Also, I noticed that it have two dri interface: /dev/dri/card0 /dev/dri/card1 I am not very familiar with Linux Graphics/Display subsystem, so I am looking for some help here. My requirement is that I have turn off HDMI display screen using a command line utility or test program. I learn that for X-server we can use xset : xset dpms force off (and it works on my ubuntu desktop with 16.04). However this command does not exists on my board. So, my question is: Is there any equivalent DPMS commands for Wayland/Wetson? - Further, in order to explore more, I cloned libdrm code from here: url = https://gitlab.freedesktop.org/mesa/drm Then I found some test utility under: drm/tests folder. After exploring more, and few modification, somehow I could able to cross-compile "proptest" for my board using below: arm-linux-gnueabi-gcc -o proptest.out proptest.c -I./target/libdrm_include/ -L./target/libdrm_lib/ -ldrm I found that "/dev/dri/card0" is not working with this test. So, I changed the test utility like this: fd = drmOpen("imx-drm", NULL); OR fd = open("/dev/dri/card1", O_RDWR); When I default run it on my board, I see that "Connector_id: 29" is showing for the HDMI display and it can support DPMS property. {{{ Connector 29 (11-1) 1 EDID: flags: immutable blob blobs: value: XXX 2 DPMS: flags: enum enums: On=0 Standby=1 Suspend=2 Off=3 value: 0 CRTC 24 CRTC 27 }}} Then, when I try to run it using below command: # ./proptest.out 29 connector 2 3 The program just returns successfully without any errors, but nothing happens. The display does not turns off. I saw that in my kernel 3.10 the ioctl(DRM_IOCTL_MODE_SETPROPERTY) is already supported under DRM. So, I am wondering what is the right way to verify DPMS mode property on wayland ? If anybody have any suggestions, please help me. Thanks, Pintu ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/33] fbcon notifier begone!
On Sat, May 25, 2019 at 07:19:28PM +0200, Sam Ravnborg wrote: > Hi Daniel. > > Good work, nice cleanup all over. > > A few comments to a few patches - not something that warrant a > new series to be posted as long as it is fixed before the patches are > applied. Hm yeah good idea, I'll add that to the next version. > > btw for future plans: I think this is tricky enough (it's old code and all > > that) that we should let this soak for 2-3 kernel releases. I think the > > following would be nice subsequent cleanup/fixes: > > > > - push the console lock completely from fbmem.c to fbcon.c. I think we're > > mostly there with prep, but needs to pondering of corner cases. > I wonder - should this code consistently use __acquire() etc so we could > get a little static analysis help for the locking? > > I have not played with this for several years and I do not know the > maturity of this today. Ime these sparse annotations are pretty useless because too inflexible. I tend to use runtime locking checks based on lockdep. Those are more accurate, and work even when the place the lock is taken is very far away from where you're checking, without having to annoate all functions in between. We need that for something like console_lock which is a very big lock. Only downside is that only paths you hit at runtime are checked. > > - move fbcon.c from using indices for tracking fb_info (and accessing > > registered_fbs without proper locking all the time) to real fb_info > > pointers with the right amount of refcounting. Mostly motivated by the > > fun I had trying to simplify fbcon_exit(). > > > > - make sure that fbcon call lock/unlock_fb when it calls fbmem.c > > functions, and sprinkle assert_lockdep_held around in fbmem.c. This > > needs the console_lock cleanups first. > > > > But I think that's material for maybe next year or so. > Or maybe after next kernel release. > Could we put this nice plan into todo.rst or similar so we do not have > to hunt it down by asking google? > > For the whole series you can add my: > Reviewed-by: Sam Ravnborg > > Some parts are reviewed as "this looks entirely correct", other parts > I would claim that I actually understood. > And after having spend some hours on this a r-b seems in order. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 04/33] vt: More locking checks
On Mon, May 27, 2019 at 09:08:58AM +0200, Daniel Vetter wrote: > On Fri, May 24, 2019 at 10:53:25AM +0200, Daniel Vetter wrote: > > I honestly have no idea what the subtle differences between > > con_is_visible, con_is_fg (internal to vt.c) and con_is_bound are. But > > it looks like both vc->vc_display_fg and con_driver_map are protected > > by the console_lock, so probably better if we hold that when checking > > this. > > > > To do that I had to deinline the con_is_visible function. > > > > Signed-off-by: Daniel Vetter > > Cc: Greg Kroah-Hartman > > Cc: Nicolas Pitre > > Cc: Martin Hostettler > > Cc: Adam Borowski > > Cc: Daniel Vetter > > Cc: Mikulas Patocka > > Hi Greg, > > Do you want to merge this through your console tree or ack for merging > through drm/fbdev? It's part of a bigger series, and I'd like to have more > testing with this in our trees, but also ok to merge stand-alone if you > prefer that. It's just locking checks and some docs. > > Same for the preceeding patch in this series here. For all of these, please take them through your tree(s): Acked-by: Greg Kroah-Hartman
Re: [PATCH 14/33] staging/olpc: lock_fb_info can't fail
On Mon, May 27, 2019 at 09:10:10AM +0200, Daniel Vetter wrote: > On Fri, May 24, 2019 at 10:53:35AM +0200, Daniel Vetter wrote: > > Simply because olpc never unregisters the damn thing. It also > > registers the framebuffer directly by poking around in fbdev > > core internals, so it's all around rather broken. > > > > Signed-off-by: Daniel Vetter > > Cc: Jens Frederich > > Cc: Daniel Drake > > Cc: Jon Nettleton > > Hi Greg, > > Somehow get_maintainers didn't pick you up for this. Ack for merging this > through drm/fbdev? It's part of a bigger series to rework fbdev/fbcon > interactions. Again, all good for you to take: Acked-by: Greg Kroah-Hartman
Re: [PATCH 32/33] staging/olpc_dcon: Add drm conversion to TODO
On Mon, May 27, 2019 at 09:11:26AM +0200, Daniel Vetter wrote: > On Fri, May 24, 2019 at 10:53:53AM +0200, Daniel Vetter wrote: > > this driver is pretty horrible from a design pov, and needs a complete > > overhaul. Concrete thing that annoys me is that it looks at > > registered_fb, which is an internal thing to fbmem.c and fbcon.c. And > > ofc it gets the lifetime rules all wrong (it should at least use > > get/put_fb_info). > > > > Looking at the history, there's been an attempt at dropping this from > > staging in 2016, but that had to be reverted. Since then not real > > effort except the usual stream of trivial patches, and fbdev has been > > formally closed for any new hw support. Time to try again and drop > > this? > > > > Signed-off-by: Daniel Vetter > > Cc: Jens Frederich > > Cc: Daniel Drake > > Cc: Jon Nettleton > > Hi Greg > > Again get_mainatiners didn't pick you up on this somehow (I manually added > you now for the next round). Do you want to pick this up to staging, or > ack for merging through drm/fbdev as part of the larger fbdev/fbcon > rework? > > Also, I think time to retry and attempt at dropping this imo ... Acked-by: Greg Kroah-Hartman
[PATCH 08/13] drm/nouveau: drop DRM_AUTH from DRM_RENDER_ALLOW ioctls
From: Emil Velikov The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Note: the outstanding DRM_AUTH instance is: - legacy DRI1 ioctl, which is already neutered Cc: Ben Skeggs Cc: nouv...@lists.freedesktop.org Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/nouveau/nouveau_drm.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 22cd45845e07..ff5994c0d429 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -1045,20 +1045,20 @@ nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv) static const struct drm_ioctl_desc nouveau_ioctls[] = { - DRM_IOCTL_DEF_DRV(NOUVEAU_GETPARAM, nouveau_abi16_ioctl_getparam, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GETPARAM, nouveau_abi16_ioctl_getparam, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(NOUVEAU_SETPARAM, nouveau_abi16_ioctl_setparam, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), - DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_ALLOC, nouveau_abi16_ioctl_channel_alloc, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_FREE, nouveau_abi16_ioctl_channel_free, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_GROBJ_ALLOC, nouveau_abi16_ioctl_grobj_alloc, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_NOTIFIEROBJ_ALLOC, nouveau_abi16_ioctl_notifierobj_alloc, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_GPUOBJ_FREE, nouveau_abi16_ioctl_gpuobj_free, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_SVM_INIT, nouveau_svmm_init, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_SVM_BIND, nouveau_svmm_bind, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_NEW, nouveau_gem_ioctl_new, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_PUSHBUF, nouveau_gem_ioctl_pushbuf, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_ALLOC, nouveau_abi16_ioctl_channel_alloc, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_FREE, nouveau_abi16_ioctl_channel_free, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GROBJ_ALLOC, nouveau_abi16_ioctl_grobj_alloc, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_NOTIFIEROBJ_ALLOC, nouveau_abi16_ioctl_notifierobj_alloc, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GPUOBJ_FREE, nouveau_abi16_ioctl_gpuobj_free, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_SVM_INIT, nouveau_svmm_init, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_SVM_BIND, nouveau_svmm_bind, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_NEW, nouveau_gem_ioctl_new, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_PUSHBUF, nouveau_gem_ioctl_pushbuf, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_RENDER_ALLOW), }; long -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 02/13] drm/amdgpu: drop DRM_AUTH usage from the driver
From: Emil Velikov The authentication can be circumvented, by design, by using the render node. From the driver POV htere is no distinction between primary and render nodes, thus we can drop the token. Note: authentication is required on a single ioctl, due to a bug in userspace. The issue has been fixed recently, but as an interim solution we're using DRM_FORCE_AUTH for the ioctl. Cc: Alex Deucher Cc: Christian König Cc: amd-...@lists.freedesktop.org Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 31 + 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index b8076929440b..6c2ba38a33ff 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -1204,16 +1204,16 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe) } const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { - DRM_IOCTL_DEF_DRV(AMDGPU_GEM_CREATE, amdgpu_gem_create_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(AMDGPU_CTX, amdgpu_ctx_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(AMDGPU_VM, amdgpu_vm_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_GEM_CREATE, amdgpu_gem_create_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_CTX, amdgpu_ctx_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_VM, amdgpu_vm_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_SCHED, amdgpu_sched_ioctl, DRM_MASTER), - DRM_IOCTL_DEF_DRV(AMDGPU_BO_LIST, amdgpu_bo_list_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(AMDGPU_FENCE_TO_HANDLE, amdgpu_cs_fence_to_handle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_BO_LIST, amdgpu_bo_list_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_FENCE_TO_HANDLE, amdgpu_cs_fence_to_handle_ioctl, DRM_RENDER_ALLOW), /* KMS */ - DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(AMDGPU_GEM_WAIT_IDLE, amdgpu_gem_wait_idle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(AMDGPU_CS, amdgpu_cs_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_GEM_WAIT_IDLE, amdgpu_gem_wait_idle_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_CS, amdgpu_cs_ioctl, DRM_RENDER_ALLOW), /* The DRM_FORCE_AUTH is effectively a workaround for the RADV Mesa driver. * This is required for Mesa: * - the whole 18.2.x series, which is EOL @@ -1224,13 +1224,14 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { #if defined(DRM_AMDGPU_FORCE_AUTH) DRM_FORCE_AUTH| #endif - DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_CS, amdgpu_cs_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_FENCES, amdgpu_cs_wait_fences_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(AMDGPU_GEM_METADATA, amdgpu_gem_metadata_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(AMDGPU_GEM_VA, amdgpu_gem_va_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(AMDGPU_GEM_OP, amdgpu_gem_op_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, DRM_AUTH|DRM_RENDER_ALLOW) + DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, DRM_FORCE_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_CS, amdgpu_cs_wait_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_FENCES, amdgpu_cs_wait_fences_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_GEM_METADATA, amdgpu_gem_metadata_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_GEM_VA, amdgpu_gem_va_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_GEM_OP, amdgpu_gem_op_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, DRM_RENDER_ALLOW) }; const int amdgpu_max_kms_ioctl = ARRAY_SIZE(amdgpu_ioctls_kms); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 06/13] drm/lima: drop DRM_AUTH usage from the driver
From: Emil Velikov The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Cc: Qiang Yu Cc: l...@lists.freedesktop.org Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/lima/lima_drv.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/lima/lima_drv.c b/drivers/gpu/drm/lima/lima_drv.c index b29c26cd13b2..ae89938c63b1 100644 --- a/drivers/gpu/drm/lima/lima_drv.c +++ b/drivers/gpu/drm/lima/lima_drv.c @@ -231,13 +231,13 @@ static void lima_drm_driver_postclose(struct drm_device *dev, struct drm_file *f } static const struct drm_ioctl_desc lima_drm_driver_ioctls[] = { - DRM_IOCTL_DEF_DRV(LIMA_GET_PARAM, lima_ioctl_get_param, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(LIMA_GEM_CREATE, lima_ioctl_gem_create, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(LIMA_GEM_INFO, lima_ioctl_gem_info, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(LIMA_GEM_SUBMIT, lima_ioctl_gem_submit, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(LIMA_GEM_WAIT, lima_ioctl_gem_wait, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(LIMA_CTX_CREATE, lima_ioctl_ctx_create, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(LIMA_CTX_FREE, lima_ioctl_ctx_free, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(LIMA_GET_PARAM, lima_ioctl_get_param, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(LIMA_GEM_CREATE, lima_ioctl_gem_create, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(LIMA_GEM_INFO, lima_ioctl_gem_info, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(LIMA_GEM_SUBMIT, lima_ioctl_gem_submit, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(LIMA_GEM_WAIT, lima_ioctl_gem_wait, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(LIMA_CTX_CREATE, lima_ioctl_ctx_create, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(LIMA_CTX_FREE, lima_ioctl_ctx_free, DRM_RENDER_ALLOW), }; static const struct file_operations lima_drm_driver_fops = { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 12/13] drm/virtio: drop DRM_AUTH usage from the driver
From: Emil Velikov The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Cc: Gerd Hoffmann Cc: virtualizat...@lists.linux-foundation.org Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 949a264985fc..e72626faba52 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -553,34 +553,34 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev, struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS] = { DRM_IOCTL_DEF_DRV(VIRTGPU_MAP, virtio_gpu_map_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_UNLOCKED | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_EXECBUFFER, virtio_gpu_execbuffer_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_UNLOCKED | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_GETPARAM, virtio_gpu_getparam_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_UNLOCKED | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_RESOURCE_CREATE, virtio_gpu_resource_create_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_UNLOCKED | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_RESOURCE_INFO, virtio_gpu_resource_info_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_UNLOCKED | DRM_RENDER_ALLOW), /* make transfer async to the main ring? - no sure, can we * thread these in the underlying GL */ DRM_IOCTL_DEF_DRV(VIRTGPU_TRANSFER_FROM_HOST, virtio_gpu_transfer_from_host_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_UNLOCKED | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_TRANSFER_TO_HOST, virtio_gpu_transfer_to_host_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_UNLOCKED | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_WAIT, virtio_gpu_wait_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_UNLOCKED | DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(VIRTGPU_GET_CAPS, virtio_gpu_get_caps_ioctl, - DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW), + DRM_UNLOCKED | DRM_RENDER_ALLOW), }; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/13] drm/radeon: drop DRM_AUTH from DRM_RENDER_ALLOW ioctls
From: Emil Velikov The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Note: the outstanding DRM_AUTH instances are: - legacy DRI1 ioctls, which are already neutered - modern but deprecated ioctls Cc: Alex Deucher Cc: Christian König Cc: amd-...@lists.freedesktop.org Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/radeon/radeon_kms.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c index 6a8fb6fd183c..c010b8a88b8a 100644 --- a/drivers/gpu/drm/radeon/radeon_kms.c +++ b/drivers/gpu/drm/radeon/radeon_kms.c @@ -905,20 +905,20 @@ const struct drm_ioctl_desc radeon_ioctls_kms[] = { DRM_IOCTL_DEF_DRV(RADEON_SURF_ALLOC, drm_invalid_op, DRM_AUTH), DRM_IOCTL_DEF_DRV(RADEON_SURF_FREE, drm_invalid_op, DRM_AUTH), /* KMS */ - DRM_IOCTL_DEF_DRV(RADEON_GEM_INFO, radeon_gem_info_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(RADEON_GEM_CREATE, radeon_gem_create_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(RADEON_GEM_MMAP, radeon_gem_mmap_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(RADEON_GEM_SET_DOMAIN, radeon_gem_set_domain_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(RADEON_GEM_INFO, radeon_gem_info_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(RADEON_GEM_CREATE, radeon_gem_create_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(RADEON_GEM_MMAP, radeon_gem_mmap_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(RADEON_GEM_SET_DOMAIN, radeon_gem_set_domain_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(RADEON_GEM_PREAD, radeon_gem_pread_ioctl, DRM_AUTH), DRM_IOCTL_DEF_DRV(RADEON_GEM_PWRITE, radeon_gem_pwrite_ioctl, DRM_AUTH), - DRM_IOCTL_DEF_DRV(RADEON_GEM_WAIT_IDLE, radeon_gem_wait_idle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(RADEON_CS, radeon_cs_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(RADEON_INFO, radeon_info_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(RADEON_GEM_SET_TILING, radeon_gem_set_tiling_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(RADEON_GEM_GET_TILING, radeon_gem_get_tiling_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(RADEON_GEM_BUSY, radeon_gem_busy_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(RADEON_GEM_VA, radeon_gem_va_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(RADEON_GEM_OP, radeon_gem_op_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(RADEON_GEM_USERPTR, radeon_gem_userptr_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(RADEON_GEM_WAIT_IDLE, radeon_gem_wait_idle_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(RADEON_CS, radeon_cs_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(RADEON_INFO, radeon_info_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(RADEON_GEM_SET_TILING, radeon_gem_set_tiling_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(RADEON_GEM_GET_TILING, radeon_gem_get_tiling_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(RADEON_GEM_BUSY, radeon_gem_busy_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(RADEON_GEM_VA, radeon_gem_va_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(RADEON_GEM_OP, radeon_gem_op_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(RADEON_GEM_USERPTR, radeon_gem_userptr_ioctl, DRM_RENDER_ALLOW), }; int radeon_max_kms_ioctl = ARRAY_SIZE(radeon_ioctls_kms); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/13] drm/exynos: drop DRM_AUTH from DRM_RENDER_ALLOW ioctls
From: Emil Velikov The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Cc: Inki Dae Cc: Joonyoung Shim Cc: Seung-Woo Kim Cc: Kyungmin Park Cc: Tobias Jakobi Signed-off-by: Emil Velikov --- Gents, I've looked around for userspace and found only libdrm - the exynos library + simple apps and the X driver. All of which are safe with this patch. Please have a look through other some open-source userspace that you have around. Tobias, you mentioned userspace projects (mpv, libretro, other) where you've added exynos backend. Can you please check they work fine with this patch? Thanks Emil --- drivers/gpu/drm/exynos/exynos_drm_drv.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index e1ef9dc9ebf3..b461d89accff 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -81,29 +81,29 @@ static const struct vm_operations_struct exynos_drm_gem_vm_ops = { static const struct drm_ioctl_desc exynos_ioctls[] = { DRM_IOCTL_DEF_DRV(EXYNOS_GEM_CREATE, exynos_drm_gem_create_ioctl, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(EXYNOS_GEM_MAP, exynos_drm_gem_map_ioctl, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(EXYNOS_GEM_GET, exynos_drm_gem_get_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(EXYNOS_VIDI_CONNECTION, vidi_connection_ioctl, DRM_AUTH), DRM_IOCTL_DEF_DRV(EXYNOS_G2D_GET_VER, exynos_g2d_get_ver_ioctl, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(EXYNOS_G2D_SET_CMDLIST, exynos_g2d_set_cmdlist_ioctl, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(EXYNOS_G2D_EXEC, exynos_g2d_exec_ioctl, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(EXYNOS_IPP_GET_RESOURCES, exynos_drm_ipp_get_res_ioctl, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(EXYNOS_IPP_GET_CAPS, exynos_drm_ipp_get_caps_ioctl, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(EXYNOS_IPP_GET_LIMITS, exynos_drm_ipp_get_limits_ioctl, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(EXYNOS_IPP_COMMIT, exynos_drm_ipp_commit_ioctl, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), }; static const struct file_operations exynos_drm_driver_fops = { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/13] drm/msm: drop DRM_AUTH usage from the driver
From: Emil Velikov The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Cc: Rob Clark Cc: Sean Paul Cc: freedr...@lists.freedesktop.org Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/msm/msm_drv.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 31deb87abfc6..ac1c7a8a85d0 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -983,17 +983,17 @@ static int msm_ioctl_submitqueue_close(struct drm_device *dev, void *data, } static const struct drm_ioctl_desc msm_ioctls[] = { - DRM_IOCTL_DEF_DRV(MSM_GET_PARAM,msm_ioctl_get_param, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(MSM_GEM_NEW, msm_ioctl_gem_new, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(MSM_GEM_INFO, msm_ioctl_gem_info, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(MSM_GEM_CPU_PREP, msm_ioctl_gem_cpu_prep, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(MSM_GEM_CPU_FINI, msm_ioctl_gem_cpu_fini, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(MSM_GEM_SUBMIT, msm_ioctl_gem_submit, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(MSM_WAIT_FENCE, msm_ioctl_wait_fence, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(MSM_GEM_MADVISE, msm_ioctl_gem_madvise, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_NEW, msm_ioctl_submitqueue_new, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_CLOSE, msm_ioctl_submitqueue_close, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_QUERY, msm_ioctl_submitqueue_query, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(MSM_GET_PARAM,msm_ioctl_get_param, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(MSM_GEM_NEW, msm_ioctl_gem_new, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(MSM_GEM_INFO, msm_ioctl_gem_info, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(MSM_GEM_CPU_PREP, msm_ioctl_gem_cpu_prep, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(MSM_GEM_CPU_FINI, msm_ioctl_gem_cpu_fini, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(MSM_GEM_SUBMIT, msm_ioctl_gem_submit, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(MSM_WAIT_FENCE, msm_ioctl_wait_fence, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(MSM_GEM_MADVISE, msm_ioctl_gem_madvise, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_NEW, msm_ioctl_submitqueue_new, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_CLOSE, msm_ioctl_submitqueue_close, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_QUERY, msm_ioctl_submitqueue_query, DRM_RENDER_ALLOW), }; static const struct vm_operations_struct vm_ops = { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 05/13] drm/i915: drop DRM_AUTH from DRM_RENDER_ALLOW ioctls
From: Emil Velikov The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Note: the outstanding DRM_AUTH instances are: - legacy DRI1 ioctls, which are already neutered - modern but deprecated ioctls Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: intel-...@lists.freedesktop.org Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/i915/i915_drv.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 1ad88e6d7c04..ea7a713654dd 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -3098,7 +3098,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_BATCHBUFFER, drm_noop, DRM_AUTH), DRM_IOCTL_DEF_DRV(I915_IRQ_EMIT, drm_noop, DRM_AUTH), DRM_IOCTL_DEF_DRV(I915_IRQ_WAIT, drm_noop, DRM_AUTH), - DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_SETPARAM, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF_DRV(I915_ALLOC, drm_noop, DRM_AUTH), DRM_IOCTL_DEF_DRV(I915_FREE, drm_noop, DRM_AUTH), @@ -3111,13 +3111,13 @@ static const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_HWS_ADDR, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF_DRV(I915_GEM_INIT, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER, i915_gem_execbuffer_ioctl, DRM_AUTH), - DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2_WR, i915_gem_execbuffer2_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2_WR, i915_gem_execbuffer2_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_PIN, i915_gem_reject_pin_ioctl, DRM_AUTH|DRM_ROOT_ONLY), DRM_IOCTL_DEF_DRV(I915_GEM_UNPIN, i915_gem_reject_pin_ioctl, DRM_AUTH|DRM_ROOT_ONLY), - DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_SET_CACHING, i915_gem_set_caching_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_GET_CACHING, i915_gem_get_caching_ioctl, DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_ENTERVT, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF_DRV(I915_GEM_LEAVEVT, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, DRM_RENDER_ALLOW), @@ -3136,7 +3136,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs_ioctl, DRM_MASTER), DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_COLORKEY, intel_sprite_set_colorkey_ioctl, DRM_MASTER), DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_COLORKEY, drm_noop, DRM_MASTER), - DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE_EXT, i915_gem_context_create_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_RENDER_ALLOW), -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 13/13] drm: allow render capable master with DRM_AUTH ioctls
From: Emil Velikov There are cases (in mesa and applications) where one would open the primary node without properly authenticating the client. Sometimes we don't check if the authentication succeeds, but there's also cases we simply forget to do it. The former was a case for Mesa where it did not not check the return value of drmGetMagic() [1]. That was fixed recently although, there's the question of older drivers or other apps that exbibit this behaviour. While omitting the call results in issues as seen in [2] and [3]. In the libva case, libva itself doesn't authenticate the DRM client and the vaGetDisplayDRM documentation doesn't mention if the app should either. As of today, the official vainfo utility doesn't authenticate. To workaround issues like these, some users resort to running their apps under sudo. Which admittedly isn't always a good idea. Since any DRIVER_RENDER driver has sufficient isolation between clients, we can use that, for unauthenticated [primary node] ioctls that require DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. v2: - Rework/simplify if check (Daniel V) - Add examples to commit messages, elaborate. (Daniel V) v3: - Use single unlikely (Daniel V) v4: - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU issue is fixed with earlier patch. [1] https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136 [2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html [3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1 Testcase: igt/core_unauth_vs_render Cc: intel-...@lists.freedesktop.org Signed-off-by: Emil Velikov Reviewed-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190114085408.15933-2-emil.l.veli...@gmail.com --- drivers/gpu/drm/drm_ioctl.c | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 9841c0076f02..b64b022a2b29 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -511,6 +511,13 @@ int drm_version(struct drm_device *dev, void *data, return err; } +static inline bool +drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags) +{ + return drm_core_check_feature(dev, DRIVER_RENDER) && + (flags & DRM_RENDER_ALLOW); +} + /** * drm_ioctl_permit - Check ioctl permissions against caller * @@ -525,14 +532,19 @@ int drm_version(struct drm_device *dev, void *data, */ int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) { + const struct drm_device *dev = file_priv->minor->dev; + /* ROOT_ONLY is only for CAP_SYS_ADMIN */ if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN))) return -EACCES; - /* AUTH is only for authenticated or render client */ - if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) && -!file_priv->authenticated)) - return -EACCES; + /* AUTH is only for master ... */ + if (unlikely((flags & DRM_AUTH) && drm_is_primary_client(file_priv))) { + /* authenticated ones, or render capable on DRM_RENDER_ALLOW. */ + if (!file_priv->authenticated && + !drm_render_driver_and_ioctl(dev, flags)) + return -EACCES; + } /* MASTER is only for master or control clients */ if (unlikely((flags & DRM_MASTER) && -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 09/13] drm/omap: drop DRM_AUTH from DRM_RENDER_ALLOW ioctls
From: Emil Velikov The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Note: the outstanding DRM_AUTH instance is: - (badly coped) legacy DRI1 ioctl, which is a noop Cc: Tomi Valkeinen Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Emil Velikov Signed-off-by: Emil Velikov --- drivers/gpu/drm/omapdrm/omap_drv.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index 1b9b6f5e48e1..f97781f9d936 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -491,19 +491,19 @@ static int ioctl_gem_info(struct drm_device *dev, void *data, static const struct drm_ioctl_desc ioctls[DRM_COMMAND_END - DRM_COMMAND_BASE] = { DRM_IOCTL_DEF_DRV(OMAP_GET_PARAM, ioctl_get_param, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(OMAP_SET_PARAM, ioctl_set_param, DRM_AUTH | DRM_MASTER | DRM_ROOT_ONLY), DRM_IOCTL_DEF_DRV(OMAP_GEM_NEW, ioctl_gem_new, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), /* Deprecated, to be removed. */ DRM_IOCTL_DEF_DRV(OMAP_GEM_CPU_PREP, drm_noop, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), /* Deprecated, to be removed. */ DRM_IOCTL_DEF_DRV(OMAP_GEM_CPU_FINI, drm_noop, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(OMAP_GEM_INFO, ioctl_gem_info, - DRM_AUTH | DRM_RENDER_ALLOW), + DRM_RENDER_ALLOW), }; /* -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
From: Emil Velikov Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the render node. A seemingly deliberate design decision. Hence we can drop the DRM_AUTH all together (details in follow-up patch) yet not all userspace checks if it's authenticated, but instead uses uncommon assumptions. After days of digging through git log and testing, only a single (ab)use was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and assuming that failure implies lack of authentication. Affected versions are: - the whole 18.2.x series, which is EOL - the whole 18.3.x series, which is EOL - the 19.0.x series, prior to 19.0.4 Add a special quirk for that case, thus we can drop DRM_AUTH bits as mentioned earlier. Since all the affected userspace is EOL, we also add a kconfig option to disable this quirk. The whole approach is inspired by DRIVER_KMS_LEGACY_CONTEXT Cc: Alex Deucher Cc: Christian König Cc: amd-...@lists.freedesktop.org Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/amd/amdgpu/Kconfig | 16 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 12 +++- drivers/gpu/drm/drm_ioctl.c | 5 + include/drm/drm_ioctl.h | 17 + 4 files changed, 49 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index 9221e5489069..da415f445187 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -40,6 +40,22 @@ config DRM_AMDGPU_GART_DEBUGFS Selecting this option creates a debugfs file to inspect the mapped pages. Uses more memory for housekeeping, enable only for debugging. +config DRM_AMDGPU_FORCE_AUTH + bool "Force authentication check on AMDGPU_INFO ioctl" + default y + help + There were some version of the Mesa RADV drivers, which relied on + the ioctl failing, if the client is not authenticated. + + Namely, the following versions are affected: + - the whole 18.2.x series, which is EOL + - the whole 18.3.x series, which is EOL + - the 19.0.x series, prior to 19.0.4 + + Modern distributions, should disable this. That will allow various + other clients to work, that would otherwise require root privileges. + + source "drivers/gpu/drm/amd/acp/Kconfig" source "drivers/gpu/drm/amd/display/Kconfig" source "drivers/gpu/drm/amd/amdkfd/Kconfig" diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index b17d0545728e..b8076929440b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -1214,7 +1214,17 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_GEM_WAIT_IDLE, amdgpu_gem_wait_idle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_CS, amdgpu_cs_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + /* The DRM_FORCE_AUTH is effectively a workaround for the RADV Mesa driver. +* This is required for Mesa: +* - the whole 18.2.x series, which is EOL +* - the whole 18.3.x series, which is EOL +* - the 19.0.x series, prior to 19.0.4 +*/ + DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, +#if defined(DRM_AMDGPU_FORCE_AUTH) + DRM_FORCE_AUTH| +#endif + DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_CS, amdgpu_cs_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_FENCES, amdgpu_cs_wait_fences_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_GEM_METADATA, amdgpu_gem_metadata_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 2263e3ddd822..9841c0076f02 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -544,6 +544,11 @@ int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) drm_is_render_client(file_priv))) return -EACCES; + /* FORCE_AUTH is only for authenticated or render client */ + if (unlikely((flags & DRM_FORCE_AUTH) && !drm_is_render_client(file_priv) && +!file_priv->authenticated)) + return -EACCES; + return 0; } EXPORT_SYMBOL(drm_ioctl_permit); diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h index fafb6f592c4b..6084ee32043d 100644 --- a/include/drm/drm_ioctl.h +++ b/include/drm/drm_ioctl.h @@ -126,6 +126,23 @@ enum drm_ioctl_flags { * not set DRM_AUTH because they do not require authentication. */ DRM_RENDER_ALLOW= BIT(5), + /** +* @DRM_FORCE_AUTH: +* +* Authentic
[PATCH 11/13] drm/vgem: drop DRM_AUTH usage from the driver
From: Emil Velikov The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/vgem/vgem_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c index 11a8f99ba18c..0ba079f1b302 100644 --- a/drivers/gpu/drm/vgem/vgem_drv.c +++ b/drivers/gpu/drm/vgem/vgem_drv.c @@ -246,8 +246,8 @@ static int vgem_gem_dumb_map(struct drm_file *file, struct drm_device *dev, } static struct drm_ioctl_desc vgem_ioctls[] = { - DRM_IOCTL_DEF_DRV(VGEM_FENCE_ATTACH, vgem_fence_attach_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(VGEM_FENCE_SIGNAL, vgem_fence_signal_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(VGEM_FENCE_ATTACH, vgem_fence_attach_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(VGEM_FENCE_SIGNAL, vgem_fence_signal_ioctl, DRM_RENDER_ALLOW), }; static int vgem_mmap(struct file *filp, struct vm_area_struct *vma) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 03/13] drm/etnaviv: drop DRM_AUTH usage from the driver
From: Emil Velikov The authentication can be circumvented, by design, by using the render node. From the driver POV there is no distinction between primary and render nodes, thus we can drop the token. Cc: Lucas Stach Cc: Christian Gmeiner Cc: etna...@lists.freedesktop.org Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Emil Velikov --- drivers/gpu/drm/etnaviv/etnaviv_drv.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c index 7eb7cf9c3fa8..a3076704ba4f 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c @@ -430,17 +430,17 @@ static int etnaviv_ioctl_pm_query_sig(struct drm_device *dev, void *data, static const struct drm_ioctl_desc etnaviv_ioctls[] = { #define ETNA_IOCTL(n, func, flags) \ DRM_IOCTL_DEF_DRV(ETNAVIV_##n, etnaviv_ioctl_##func, flags) - ETNA_IOCTL(GET_PARAM,get_param,DRM_AUTH|DRM_RENDER_ALLOW), - ETNA_IOCTL(GEM_NEW, gem_new, DRM_AUTH|DRM_RENDER_ALLOW), - ETNA_IOCTL(GEM_INFO, gem_info, DRM_AUTH|DRM_RENDER_ALLOW), - ETNA_IOCTL(GEM_CPU_PREP, gem_cpu_prep, DRM_AUTH|DRM_RENDER_ALLOW), - ETNA_IOCTL(GEM_CPU_FINI, gem_cpu_fini, DRM_AUTH|DRM_RENDER_ALLOW), - ETNA_IOCTL(GEM_SUBMIT, gem_submit, DRM_AUTH|DRM_RENDER_ALLOW), - ETNA_IOCTL(WAIT_FENCE, wait_fence, DRM_AUTH|DRM_RENDER_ALLOW), - ETNA_IOCTL(GEM_USERPTR, gem_userptr, DRM_AUTH|DRM_RENDER_ALLOW), - ETNA_IOCTL(GEM_WAIT, gem_wait, DRM_AUTH|DRM_RENDER_ALLOW), - ETNA_IOCTL(PM_QUERY_DOM, pm_query_dom, DRM_AUTH|DRM_RENDER_ALLOW), - ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_AUTH|DRM_RENDER_ALLOW), + ETNA_IOCTL(GET_PARAM,get_param,DRM_RENDER_ALLOW), + ETNA_IOCTL(GEM_NEW, gem_new, DRM_RENDER_ALLOW), + ETNA_IOCTL(GEM_INFO, gem_info, DRM_RENDER_ALLOW), + ETNA_IOCTL(GEM_CPU_PREP, gem_cpu_prep, DRM_RENDER_ALLOW), + ETNA_IOCTL(GEM_CPU_FINI, gem_cpu_fini, DRM_RENDER_ALLOW), + ETNA_IOCTL(GEM_SUBMIT, gem_submit, DRM_RENDER_ALLOW), + ETNA_IOCTL(WAIT_FENCE, wait_fence, DRM_RENDER_ALLOW), + ETNA_IOCTL(GEM_USERPTR, gem_userptr, DRM_RENDER_ALLOW), + ETNA_IOCTL(GEM_WAIT, gem_wait, DRM_RENDER_ALLOW), + ETNA_IOCTL(PM_QUERY_DOM, pm_query_dom, DRM_RENDER_ALLOW), + ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW), }; static const struct vm_operations_struct vm_ops = { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/13] drm/i915: drop DRM_AUTH from DRM_RENDER_ALLOW ioctls
On Mon, 27 May 2019, Emil Velikov wrote: > From: Emil Velikov > > The authentication can be circumvented, by design, by using the render > node. > > From the driver POV there is no distinction between primary and render > nodes, thus we can drop the token. > > Note: the outstanding DRM_AUTH instances are: > - legacy DRI1 ioctls, which are already neutered > - modern but deprecated ioctls > > Cc: Jani Nikula > Cc: Joonas Lahtinen > Cc: Rodrigo Vivi > Cc: intel-...@lists.freedesktop.org > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Emil Velikov Please see commit b972fffa114b18a120a7bbde105d69a080d24970 Author: Christian König Date: Wed Apr 17 13:25:24 2019 +0200 drm/i915: remove DRM_AUTH from IOCTLs which also have DRM_RENDER_ALLOW It's headed to drm-next in [1]. BR, Jani. [1] 87sgt3n45z.fsf@intel.com">http://mid.mail-archive.com/87sgt3n45z.fsf@intel.com > --- > drivers/gpu/drm/i915/i915_drv.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 1ad88e6d7c04..ea7a713654dd 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -3098,7 +3098,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { > DRM_IOCTL_DEF_DRV(I915_BATCHBUFFER, drm_noop, DRM_AUTH), > DRM_IOCTL_DEF_DRV(I915_IRQ_EMIT, drm_noop, DRM_AUTH), > DRM_IOCTL_DEF_DRV(I915_IRQ_WAIT, drm_noop, DRM_AUTH), > - DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > + DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_SETPARAM, drm_noop, > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), > DRM_IOCTL_DEF_DRV(I915_ALLOC, drm_noop, DRM_AUTH), > DRM_IOCTL_DEF_DRV(I915_FREE, drm_noop, DRM_AUTH), > @@ -3111,13 +3111,13 @@ static const struct drm_ioctl_desc i915_ioctls[] = { > DRM_IOCTL_DEF_DRV(I915_HWS_ADDR, drm_noop, > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), > DRM_IOCTL_DEF_DRV(I915_GEM_INIT, drm_noop, > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), > DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER, i915_gem_execbuffer_ioctl, > DRM_AUTH), > - DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2_WR, i915_gem_execbuffer2_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > + DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2_WR, i915_gem_execbuffer2_ioctl, > DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_GEM_PIN, i915_gem_reject_pin_ioctl, > DRM_AUTH|DRM_ROOT_ONLY), > DRM_IOCTL_DEF_DRV(I915_GEM_UNPIN, i915_gem_reject_pin_ioctl, > DRM_AUTH|DRM_ROOT_ONLY), > - DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > + DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_GEM_SET_CACHING, i915_gem_set_caching_ioctl, > DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_GEM_GET_CACHING, i915_gem_get_caching_ioctl, > DRM_RENDER_ALLOW), > - DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > + DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, > DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_GEM_ENTERVT, drm_noop, > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), > DRM_IOCTL_DEF_DRV(I915_GEM_LEAVEVT, drm_noop, > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), > DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, > DRM_RENDER_ALLOW), > @@ -3136,7 +3136,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = { > DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs_ioctl, > DRM_MASTER), > DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_COLORKEY, > intel_sprite_set_colorkey_ioctl, DRM_MASTER), > DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_COLORKEY, drm_noop, DRM_MASTER), > - DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > + DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE_EXT, > i915_gem_context_create_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, > i915_gem_context_destroy_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_RENDER_ALLOW), -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/omapdrm: fix warning PTR_ERR_OR_ZERO can be used
Hi, On 25/05/2019 17:56, Matteo Croce wrote: On Sat, May 25, 2019 at 9:30 AM Hariprasad Kelam wrote: fix below warnings reported by coccicheck Hi, a similar patch was nacked because it makes backports more difficult: https://lore.kernel.org/lkml/3dec4093-824e-b13d-d712-2dedd445a...@ti.com/ Tomi, what's your thought? I don't particularly like the PTR_ERR_OR_ZERO, and changing old code to use it seem pointless. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm/vmwgfx: remove custom ioctl io encoding check
On 2019/05/25, Thomas Hellstrom wrote: > On Sat, 2019-05-25 at 00:39 +0200, Thomas Hellström wrote: > > Hi, Emil > > > > On Fri, 2019-05-24 at 16:26 +0100, Emil Velikov wrote: > > > On 2019/05/24, Thomas Hellstrom wrote: > > > > On Fri, 2019-05-24 at 13:14 +0100, Emil Velikov wrote: > > > > > On 2019/05/23, Thomas Hellstrom wrote: > > > > > > Hi, Emil, > > > > > > > > > > > > On Wed, 2019-05-22 at 17:41 +0100, Emil Velikov wrote: > > > > > > > From: Emil Velikov > > > > > > > > > > > > > > Drop the custom ioctl io encoding check - core drm does it > > > > > > > for > > > > > > > us. > > > > > > > > > > > > I fail to see where the core does this, or do I miss > > > > > > something? > > > > > > > > > > drm_ioctl() allows for the encoding to be changed and > > > > > attributes > > > > > that > > > > > only the > > > > > appropriate size is copied in/out of the kernel. > > > > > > > > > > Technically the function is more relaxed relative to the vmwgfx > > > > > check, yet > > > > > seems perfectly reasonable. > > > > > > > > > > Is there any corner-case that isn't but should be handled in > > > > > drm_ioctl()? > > > > > > > > I'd like to turn the question around and ask whether there's a > > > > reason > > > > we should relax the vmwgfx test? In the past it has trapped quite > > > > a > > > > few > > > > user-space errors. > > > > > > > The way I see it either: > > > - the check, as-is, is unnessesary, or > > > - it is needed, and we should do something equivalent for all of > > > DRM > > > > > > We had a very long brainstorming session with a colleague and we > > > could not see > > > any cases where this would cause a problem. If you recall anything > > > concrete > > > please let me know - I would be more than happy to take a closer > > > look. > > > > If you have a good reason to drop an ioctl sanity check, I'd be > > perfectly happy to do it. To me, a good reason even includes "I have > > a > > non-open-source customer having problems with this check" because of > > reason etc. etc. as long as I have a way to evaluate those reasons > > and > > determine if they're valid or not. "No other drm driver nor the core > > is > > doing this" is NOT a valid reason to me. In particular if the check > > is > > not affecting performance. So unless you provide additional reasons > > to > > drop this check, it's a solid NAK from my side. > > To clarify my point of view a bit, this check is useful to early catch > userspace using incorrect flags and sizes, which otherwise might make > it out to distros and after that, introducing a check like this would > be impossible, since it might break old user-space. For the same reason > it would probably be very difficult to introduce it in core drm. > I think we might be talking past each other, let's take a step back: - as of previous patch, all of vmwgfx ioctls size is consistently handled by the core - handling of featue flags, as always, is responsibility of the driver ifself - with this patch, ioctl direction is also handled by core. Here core ensures we only copy in/out as much data as the kernel implementation can handle. Let's consider the following real world example - msm and virtio_gpu. An in field of an _IOW ioctl becomes in/out aka _IORW ioctl. - we add a flag to annotate/request the out, as always invalid flags are return -EINVAL - we change the ioctl encoding As currently handled by core DRM, old kernel/new userspace and vice-versa works just fine. Sadly, vmwgfx will error out, while it could be avoided. As said above, I'll gladly adjust core and/or others, if this relaxed approach causes an issue somewhere. A specific use-case, real or hypothetical will be appreciated. All this is part of an "evil" plan of mine, to port cool features from vmwgfx to core and effectively remove the vmw_generic_ioctl() wrapper. Hope the bigger picture is clearer now, if not please let me know. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] drm/panel: drop drmP.h usage
On Sun, May 26, 2019 at 8:05 PM Sam Ravnborg wrote: > Drop use of the deprecated drmP.h header file. > > While touching the list of include files: > - Divide include files in blocks of linux/* video/* drm/* etc. > Be consistent in the order of the blocks > - Sort individual blocks of include files > > Signed-off-by: Sam Ravnborg > Cc: Thierry Reding > Cc: Linus Walleij > Cc: Stefan Mavrodiev > Cc: David Airlie > Cc: Daniel Vetter Reviewed-by: Linus Walleij Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 04/13] drm/exynos: drop DRM_AUTH from DRM_RENDER_ALLOW ioctls
Hi Emil, 19. 5. 27. 오후 5:17에 Emil Velikov 이(가) 쓴 글: > From: Emil Velikov > > The authentication can be circumvented, by design, by using the render > node. > >>From the driver POV there is no distinction between primary and render > nodes, thus we can drop the token. > > Cc: Inki Dae > Cc: Joonyoung Shim > Cc: Seung-Woo Kim > Cc: Kyungmin Park > Cc: Tobias Jakobi > Signed-off-by: Emil Velikov > --- > Gents, I've looked around for userspace and found only libdrm - the > exynos library + simple apps and the X driver. All of which are safe > with this patch. > > Please have a look through other some open-source userspace that you > have around. Acked-by: Inki Dae Thanks, Inki Dae > > Tobias, you mentioned userspace projects (mpv, libretro, other) where > you've added exynos backend. Can you please check they work fine with > this patch? > > Thanks > Emil > --- > drivers/gpu/drm/exynos/exynos_drm_drv.c | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c > b/drivers/gpu/drm/exynos/exynos_drm_drv.c > index e1ef9dc9ebf3..b461d89accff 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -81,29 +81,29 @@ static const struct vm_operations_struct > exynos_drm_gem_vm_ops = { > > static const struct drm_ioctl_desc exynos_ioctls[] = { > DRM_IOCTL_DEF_DRV(EXYNOS_GEM_CREATE, exynos_drm_gem_create_ioctl, > - DRM_AUTH | DRM_RENDER_ALLOW), > + DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(EXYNOS_GEM_MAP, exynos_drm_gem_map_ioctl, > - DRM_AUTH | DRM_RENDER_ALLOW), > + DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(EXYNOS_GEM_GET, exynos_drm_gem_get_ioctl, > DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(EXYNOS_VIDI_CONNECTION, vidi_connection_ioctl, > DRM_AUTH), > DRM_IOCTL_DEF_DRV(EXYNOS_G2D_GET_VER, exynos_g2d_get_ver_ioctl, > - DRM_AUTH | DRM_RENDER_ALLOW), > + DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(EXYNOS_G2D_SET_CMDLIST, exynos_g2d_set_cmdlist_ioctl, > - DRM_AUTH | DRM_RENDER_ALLOW), > + DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(EXYNOS_G2D_EXEC, exynos_g2d_exec_ioctl, > - DRM_AUTH | DRM_RENDER_ALLOW), > + DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(EXYNOS_IPP_GET_RESOURCES, > exynos_drm_ipp_get_res_ioctl, > - DRM_AUTH | DRM_RENDER_ALLOW), > + DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(EXYNOS_IPP_GET_CAPS, exynos_drm_ipp_get_caps_ioctl, > - DRM_AUTH | DRM_RENDER_ALLOW), > + DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(EXYNOS_IPP_GET_LIMITS, > exynos_drm_ipp_get_limits_ioctl, > - DRM_AUTH | DRM_RENDER_ALLOW), > + DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(EXYNOS_IPP_COMMIT, exynos_drm_ipp_commit_ioctl, > - DRM_AUTH | DRM_RENDER_ALLOW), > + DRM_RENDER_ALLOW), > }; > > static const struct file_operations exynos_drm_driver_fops = { > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: dpms mode change with wayland on iMX.6
On Mon, May 27, 2019 at 12:41 PM Pintu Agarwal wrote: > > Dear All, > > I have a iMX.6 (arm 32) board with Linux Kernel 3.10 and debian > platform running. > The board is connected to one LCD screen and one HDMI monitor. > It have DRM + Wayland setup for display. > Also, I noticed that it have two dri interface: > /dev/dri/card0 > /dev/dri/card1 > > I am not very familiar with Linux Graphics/Display subsystem, so I am > looking for some help here. > > My requirement is that I have turn off HDMI display screen using a > command line utility or test program. > I learn that for X-server we can use xset : xset dpms force off (and > it works on my ubuntu desktop with 16.04). > > However this command does not exists on my board. > So, my question is: > Is there any equivalent DPMS commands for Wayland/Wetson? > > - > Further, in order to explore more, I cloned libdrm code from here: > url = https://gitlab.freedesktop.org/mesa/drm > > Then I found some test utility under: drm/tests folder. > After exploring more, and few modification, somehow I could able to > cross-compile "proptest" for my board using below: > arm-linux-gnueabi-gcc -o proptest.out proptest.c > -I./target/libdrm_include/ -L./target/libdrm_lib/ -ldrm > > I found that "/dev/dri/card0" is not working with this test. > So, I changed the test utility like this: > fd = drmOpen("imx-drm", NULL); > OR > fd = open("/dev/dri/card1", O_RDWR); > > When I default run it on my board, I see that "Connector_id: 29" is > showing for the HDMI display and it can support DPMS property. > {{{ > Connector 29 (11-1) > 1 EDID: > flags: immutable blob > blobs: > > value: > XXX > 2 DPMS: > flags: enum > enums: On=0 Standby=1 Suspend=2 Off=3 > value: 0 > CRTC 24 > CRTC 27 > }}} > > Then, when I try to run it using below command: > # ./proptest.out 29 connector 2 3 > > The program just returns successfully without any errors, but nothing > happens. The display does not turns off. > I saw that in my kernel 3.10 the ioctl(DRM_IOCTL_MODE_SETPROPERTY) is > already supported under DRM. > > So, I am wondering what is the right way to verify DPMS mode property > on wayland ? > > If anybody have any suggestions, please help me. > > > Thanks, > Pintu + etna...@lists.freedesktop.org ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/stm: ltdc: No message if probe
Print display controller hardware version in debug mode only. Signed-off-by: Yannick Fertré --- drivers/gpu/drm/stm/ltdc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index d24ffc2..16b1103 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -1211,7 +1211,7 @@ int ltdc_load(struct drm_device *ddev) goto err; } - DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version); + DRM_DEBUG_DRIVER("ltdc hw version 0x%08x\n", ldev->caps.hw_version); /* Add endpoints panels or bridges if any */ for (i = 0; i < MAX_ENDPOINTS; i++) { -- 2.7.4
[PATCH v1 1/2] drm/bridge/synopsys: dsi: add power on/off optional phy ops
Add power on & off optional physical operation functions, helpful to program specific registers of the DSI physical part. Signed-off-by: Yannick Fertré --- drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8 include/drm/bridge/dw_mipi_dsi.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c index e915ae8..5bb676f 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c @@ -775,6 +775,10 @@ static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi *dsi) static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge) { struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge); + const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops; + + if (phy_ops->power_off) + phy_ops->power_off(dsi->plat_data->priv_data); /* * Switch to command mode before panel-bridge post_disable & @@ -874,11 +878,15 @@ static void dw_mipi_dsi_bridge_mode_set(struct drm_bridge *bridge, static void dw_mipi_dsi_bridge_enable(struct drm_bridge *bridge) { struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge); + const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops; /* Switch to video mode for panel-bridge enable & panel enable */ dw_mipi_dsi_set_mode(dsi, MIPI_DSI_MODE_VIDEO); if (dsi->slave) dw_mipi_dsi_set_mode(dsi->slave, MIPI_DSI_MODE_VIDEO); + + if (phy_ops->power_on) + phy_ops->power_on(dsi->plat_data->priv_data); } static enum drm_mode_status diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h index 7d3dd69..df6eda6 100644 --- a/include/drm/bridge/dw_mipi_dsi.h +++ b/include/drm/bridge/dw_mipi_dsi.h @@ -14,6 +14,8 @@ struct dw_mipi_dsi; struct dw_mipi_dsi_phy_ops { int (*init)(void *priv_data); + void (*power_on)(void *priv_data); + void (*power_off)(void *priv_data); int (*get_lane_mbps)(void *priv_data, const struct drm_display_mode *mode, unsigned long mode_flags, u32 lanes, u32 format, -- 2.7.4
[PATCH v1 0/2] dw-mipi-dsi: add power on & off optional phy ops and update stm
These patches fix a bug concerning an access issue to display controler (ltdc) registers. If the physical layer of the DSI is started too early then the fifo DSI are full very quickly which implies ltdc register's access hang up. To avoid this problem, it is necessary to start the DSI physical layer only when the bridge is enable. Yannick Fertré (2): drm/bridge/synopsys: dsi: add power on/off optional phy ops drm/stm: dsi: add power on/off phy ops drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8 drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 21 - include/drm/bridge/dw_mipi_dsi.h | 2 ++ 3 files changed, 30 insertions(+), 1 deletion(-) -- 2.7.4
[PATCH v1 2/2] drm/stm: dsi: add power on/off phy ops
These new physical operations are helpful to power_on/off the dsi wrapper. If the dsi wrapper is powered in video mode, the display controller (ltdc) register access will hang when DSI fifos are full. Signed-off-by: Yannick Fertré --- drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 21 - 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c index 01db020..0ab32fe 100644 --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c @@ -210,10 +210,27 @@ static int dw_mipi_dsi_phy_init(void *priv_data) if (ret) DRM_DEBUG_DRIVER("!TIMEOUT! waiting PLL, let's continue\n"); + return 0; +} + +static void dw_mipi_dsi_phy_power_on(void *priv_data) +{ + struct dw_mipi_dsi_stm *dsi = priv_data; + + DRM_DEBUG_DRIVER("\n"); + /* Enable the DSI wrapper */ dsi_set(dsi, DSI_WCR, WCR_DSIEN); +} - return 0; +static void dw_mipi_dsi_phy_power_off(void *priv_data) +{ + struct dw_mipi_dsi_stm *dsi = priv_data; + + DRM_DEBUG_DRIVER("\n"); + + /* Disable the DSI wrapper */ + dsi_clear(dsi, DSI_WCR, WCR_DSIEN); } static int @@ -287,6 +304,8 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct drm_display_mode *mode, static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_stm_phy_ops = { .init = dw_mipi_dsi_phy_init, + .power_on = dw_mipi_dsi_phy_power_on, + .power_off = dw_mipi_dsi_phy_power_off, .get_lane_mbps = dw_mipi_dsi_get_lane_mbps, }; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/1] drm/panel: truly: Add additional delay after pulling down reset gpio
MTP SDM845 panel seems to need additional delay to bring panel to a workable state. Running modetest without this change displays blurry artifacts. Signed-off-by: Vivek Gautam --- drivers/gpu/drm/panel/panel-truly-nt35597.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/panel/panel-truly-nt35597.c b/drivers/gpu/drm/panel/panel-truly-nt35597.c index fc2a66c53db4..aa7153fd3be4 100644 --- a/drivers/gpu/drm/panel/panel-truly-nt35597.c +++ b/drivers/gpu/drm/panel/panel-truly-nt35597.c @@ -280,6 +280,7 @@ static int truly_35597_power_on(struct truly_nt35597 *ctx) gpiod_set_value(ctx->reset_gpio, 1); usleep_range(1, 2); gpiod_set_value(ctx->reset_gpio, 0); + usleep_range(1, 2); return 0; } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
Am 27.05.19 um 10:17 schrieb Emil Velikov: > From: Emil Velikov > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > render node. A seemingly deliberate design decision. > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > yet not all userspace checks if it's authenticated, but instead uses > uncommon assumptions. > > After days of digging through git log and testing, only a single (ab)use > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > assuming that failure implies lack of authentication. > > Affected versions are: > - the whole 18.2.x series, which is EOL > - the whole 18.3.x series, which is EOL > - the 19.0.x series, prior to 19.0.4 Well you could have saved your time, cause this is still a NAK. For the record: I strongly think that we don't want to expose any render functionality on the primary node. To even go a step further I would say that at least on AMD hardware we should completely disable DRI2 for one of the next generations. As a follow up I would then completely disallow the DRM authentication for amdgpu, so that the command submission interface on the primary node can only be used by the display server. Regards, Christian. > > Add a special quirk for that case, thus we can drop DRM_AUTH bits as > mentioned earlier. > > Since all the affected userspace is EOL, we also add a kconfig option > to disable this quirk. > > The whole approach is inspired by DRIVER_KMS_LEGACY_CONTEXT > > Cc: Alex Deucher > Cc: Christian König > Cc: amd-...@lists.freedesktop.org > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Emil Velikov > --- > drivers/gpu/drm/amd/amdgpu/Kconfig | 16 > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 12 +++- > drivers/gpu/drm/drm_ioctl.c | 5 + > include/drm/drm_ioctl.h | 17 + > 4 files changed, 49 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig > b/drivers/gpu/drm/amd/amdgpu/Kconfig > index 9221e5489069..da415f445187 100644 > --- a/drivers/gpu/drm/amd/amdgpu/Kconfig > +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig > @@ -40,6 +40,22 @@ config DRM_AMDGPU_GART_DEBUGFS > Selecting this option creates a debugfs file to inspect the mapped > pages. Uses more memory for housekeeping, enable only for debugging. > > +config DRM_AMDGPU_FORCE_AUTH > + bool "Force authentication check on AMDGPU_INFO ioctl" > + default y > + help > + There were some version of the Mesa RADV drivers, which relied on > + the ioctl failing, if the client is not authenticated. > + > + Namely, the following versions are affected: > + - the whole 18.2.x series, which is EOL > + - the whole 18.3.x series, which is EOL > + - the 19.0.x series, prior to 19.0.4 > + > + Modern distributions, should disable this. That will allow various > + other clients to work, that would otherwise require root privileges. > + > + > source "drivers/gpu/drm/amd/acp/Kconfig" > source "drivers/gpu/drm/amd/display/Kconfig" > source "drivers/gpu/drm/amd/amdkfd/Kconfig" > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > index b17d0545728e..b8076929440b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > @@ -1214,7 +1214,17 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_WAIT_IDLE, amdgpu_gem_wait_idle_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_CS, amdgpu_cs_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > - DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > + /* The DRM_FORCE_AUTH is effectively a workaround for the RADV Mesa > driver. > + * This is required for Mesa: > + * - the whole 18.2.x series, which is EOL > + * - the whole 18.3.x series, which is EOL > + * - the 19.0.x series, prior to 19.0.4 > + */ > + DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, > +#if defined(DRM_AMDGPU_FORCE_AUTH) > + DRM_FORCE_AUTH| > +#endif > + DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_CS, amdgpu_cs_wait_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_FENCES, amdgpu_cs_wait_fences_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_METADATA, amdgpu_gem_metadata_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index 2263e3ddd822..9841c0076f02 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -544,6 +544,11 @@ int drm_ioctl_permit(u32 flags, struct drm_file > *file_priv) >drm_is_render_client(file_priv))) >
Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support
Hi, On 23/05/2019 23:07, Sebastian Reichel wrote: Hi, Here is another round of the DSI command mode panel patchset integrating the feedback from PATCHv5. The patches are based on v5.2-rc1 tag. It does not contain the patches required for OMAP3 support (it needs a workaround for a hardware bug) and for automatic display rotation. They should get their own series, once after everything has been moved to DRM panel API. I think DRM panel conversion should happen _after_ this series, since otherwise there is a high risk of bricking DSI support completely. I already started a WIP branch for converting DSI to the DRM panel API on top of this patchset. Looks good to me. For some reason I can't boot 5.2-rc2 (on x15) so I haven't been able to test yet. I'll pick the series up in any case, and I'll test it when I get the kernel booting. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH 10/10] drm/amdgpu: stop removing BOs from the LRU v3
Am 24.05.19 um 23:34 schrieb Kuehling, Felix: > On 2019-05-23 5:06 a.m., Christian König wrote: >> [CAUTION: External Email] >> >> Leaving BOs on the LRU is harmless. We always did this for VM page table >> and per VM BOs. >> >> The key point is that BOs which couldn't be reserved can't be evicted. >> So what happened is that an application used basically all of VRAM >> during CS and because of this X server couldn't pin a BO for scanout. >> >> Now we keep the BOs on the LRU and modify TTM to block for the CS to >> complete, which in turn allows the X server to pin its BO for scanout. > > OK, let me rephrase that to make sure I understand it correctly. I think > the point is that eviction candidates come from an LRU list, so leaving > things on the LRU makes more BOs available for eviction and avoids OOM > situations. To take advantage of that, patch 6 adds the ability to wait > for reserved BOs when there is nothing easier to evict. > > ROCm applications like to use lots of memory. So it probably makes sense > for us to stop removing our BOs from the LRU as well while we > mass-validate our BOs in amdgpu_amdkfd_gpuvm_restore_process_bos. Well that would allow concurrent calls of amdgpu_amdkfd_gpuvm_restore_process_bos() to wait for each other. If that's what you want then yeah that certainly makes sense. Regards, Christian. > > Regards, > Felix > > >> Christian. >> >> Am 22.05.19 um 21:43 schrieb Kuehling, Felix: >>> Can you explain how this avoids OOM situations? When is it safe to leave >>> a reserved BO on the LRU list? Could we do the same thing in >>> amdgpu_amdkfd_gpuvm.c? And if we did, what would be the expected side >>> effects or consequences? >>> >>> Thanks, >>> Felix >>> >>> On 2019-05-22 8:59 a.m., Christian König wrote: [CAUTION: External Email] This avoids OOM situations when we have lots of threads submitting at the same time. v3: apply this to the whole driver, not just CS Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 20f2955d2a55..3e2da24cd17a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -648,7 +648,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, } r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true, - &duplicates, true); + &duplicates, false); if (unlikely(r != 0)) { if (r != -ERESTARTSYS) DRM_ERROR("ttm_eu_reserve_buffers failed.\n"); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c index 06f83cac0d3a..f660628e6af9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c @@ -79,7 +79,7 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm, list_add(&csa_tv.head, &list); amdgpu_vm_get_pd_bo(vm, &list, &pd); - r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, true); + r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, false); if (r) { DRM_ERROR("failed to reserve CSA,PD BOs: err=%d\n", r); return r; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index d513a5ad03dd..ed25a4e14404 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -171,7 +171,7 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj, amdgpu_vm_get_pd_bo(vm, &list, &vm_pd); - r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates, true); + r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates, false); if (r) { dev_err(adev->dev, "leaking bo va because " "we fail to reserve bo (%d)\n", r); @@ -608,7 +608,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data, amdgpu_vm_get_pd_bo(&fpriv->vm, &list, &vm_pd); - r = ttm_eu_reserve_buffers(&ticket, &list, true, &duplicates, true); + r = ttm_eu_reserve_buffers(&ticket, &list, true, &duplicates, false); if (r) goto error_unref; diff --git a/drivers/gpu/drm/amd/amdg
Re: [PATCH v2 2/2] drm/panel: drop drmP.h usage
On Mon, May 27, 2019 at 11:11:18AM +0200, Linus Walleij wrote: > On Sun, May 26, 2019 at 8:05 PM Sam Ravnborg wrote: > > > Drop use of the deprecated drmP.h header file. > > > > While touching the list of include files: > > - Divide include files in blocks of linux/* video/* drm/* etc. > > Be consistent in the order of the blocks > > - Sort individual blocks of include files > > > > Signed-off-by: Sam Ravnborg > > Cc: Thierry Reding > > Cc: Linus Walleij > > Cc: Stefan Mavrodiev > > Cc: David Airlie > > Cc: Daniel Vetter > > Reviewed-by: Linus Walleij Thanks, can I get you to review patch 1/2 too? Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/12] dma-buf: add dma_buf_(un)map_attachment_locked variants v3
Thanks for the comments, but you are looking at a completely outdated patchset. If you are interested in the newest one please ping me and I'm going to CC you when I send out the next version. Christian. Am 25.05.19 um 03:04 schrieb Hillf Danton: On Tue, 16 Apr 2019 20:38:31 +0200 Christian König wrote: Add function variants which can be called with the reservation lock already held. v2: reordered, add lockdep asserts, fix kerneldoc v3: rebased on sgt caching Signed-off-by: Christian König --- drivers/dma-buf/dma-buf.c | 63 +++ include/linux/dma-buf.h | 5 2 files changed, 68 insertions(+) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 65161a82d4d5..ef480e5fb239 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -623,6 +623,43 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) } EXPORT_SYMBOL_GPL(dma_buf_detach); +/** + * dma_buf_map_attachment_locked - Maps the buffer into _device_ address space + * with the reservation lock held. Is a wrapper for map_dma_buf() of the Something is missing; seems to be s/of the/of the dma_buf_ops./ + * + * Returns the scatterlist table of the attachment; + * dma_buf_ops. Oh it is sitting here! + * @attach:[in]attachment whose scatterlist is to be returned + * @direction: [in]direction of DMA transfer + * + * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR + * on error. May return -EINTR if it is interrupted by a signal. + * EINTR looks impossible in the code. + * A mapping must be unmapped by using dma_buf_unmap_attachment_locked(). Note + * that the underlying backing storage is pinned for as long as a mapping + * exists, therefore users/importers should not hold onto a mapping for undue + * amounts of time. + */ +struct sg_table * +dma_buf_map_attachment_locked(struct dma_buf_attachment *attach, + enum dma_data_direction direction) +{ + struct sg_table *sg_table; + + might_sleep(); + reservation_object_assert_held(attach->dmabuf->resv); + + if (attach->sgt) + return attach->sgt; + + sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction); + if (!sg_table) + sg_table = ERR_PTR(-ENOMEM); + + return sg_table; +} +EXPORT_SYMBOL_GPL(dma_buf_map_attachment_locked); + Best Regards Hillf ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 24/33] Revert "backlight/fbcon: Add FB_EVENT_CONBLANK"
On 5/24/19 5:28 PM, Daniel Vetter wrote: > Hi Daniel, > > On Fri, May 24, 2019 at 3:14 PM Daniel Thompson > wrote: >> >> On Fri, May 24, 2019 at 10:53:45AM +0200, Daniel Vetter wrote: >>> This reverts commit 994efacdf9a087b52f71e620b58dfa526b0cf928. >>> >>> The justification is that if hw blanking fails (i.e. fbops->fb_blank) >>> fails, then we still want to shut down the backlight. Which is exactly >>> _not_ what fb_blank() does and so rather inconsistent if we end up >>> with different behaviour between fbcon and direct fbdev usage. Given >>> that the entire notifier maze is getting in the way anyway I figured >>> it's simplest to revert this not well justified commit. >>> >>> v2: Add static inline to the dummy version. >>> >>> Cc: Richard Purdie >>> Signed-off-by: Daniel Vetter >>> Cc: Lee Jones >>> Cc: Daniel Thompson >>> Cc: Jingoo Han >>> Cc: Bartlomiej Zolnierkiewicz >>> Cc: Daniel Vetter >>> Cc: Hans de Goede >>> Cc: Yisheng Xie >>> Cc: linux-fb...@vger.kernel.org >> >> Hi Daniel >> >> When this goes round again could you add me to the covering letter? >> >> I looked at all three of the patches and no objections on my side but >> I'm reluctant to send out acks because I'm not sure I understood the >> wider picture well enough. > > It's one of these patch series with some many different subsystems and > people you can't cc the cover to all of them or mailing lists start > rejecting you because too many recipients. I tried to spam sufficient > mailng lists, but I guess not enough. BTW Not all relevant patches were posted to linux-fbdev and me (i.e. "[PATCH 05/33] fbdev/sa1100fb: Remove dead code") - I found them on other mailing lists but it requires some additional work from me to find / process them. If possible please Cc: linux-fbdev & me on all patches in the patchset. Thanks! Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107209] DM_PPLIB causes a warning on Raven
https://bugs.freedesktop.org/show_bug.cgi?id=107209 --- Comment #3 from vono --- Same here on Fedora 30 with kernel 5.0.17-300.fc30.x86_64 [drm] DM_PPLIB: values for Invalid clock [drm] DM_PPLIB: 0 in kHz [drm] DM_PPLIB: 0 in kHz [drm] DM_PPLIB: 0 in kHz [drm] DM_PPLIB: 160 in kHz WARNING: CPU: 2 PID: 407 at drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:1380 dcn_bw_update_from_pplib+0xa2/0x2d0 [amdgpu] Modules linked in: amdgpu(+) chash amd_iommu_v2 gpu_sched i2c_algo_bit ttm drm_kms_helper crc32c_intel drm r8169 wmi pinctrl_amd video CPU: 2 PID: 407 Comm: systemd-udevd Not tainted 5.0.17-300.fc30.x86_64 #1 Hardware name: Micro-Star International Co., Ltd MS-7B07/A320M PRO-VH PLUS (MS-7B07), BIOS 3.G1 05/15/2019 RIP: 0010:dcn_bw_update_from_pplib+0xa2/0x2d0 [amdgpu] Code: 24 10 85 c9 74 24 8d 71 ff 48 8d 44 24 14 48 8d 54 f4 1c eb 0d 48 83 c0 08 48 39 c2 0f 84 2a 01 00 00 44 8b 00 45 85 c0 75 eb <0f> 0b e8 37 99 be cc 4c 89 > RSP: 0018:b8f481267760 EFLAGS: 00010246 RAX: b8f481267774 RBX: 981f4b477000 RCX: 0004 RDX: b8f481267794 RSI: 0003 RDI: RBP: 0001 R08: R09: 0364 R10: 000117bc R11: 0003 R12: b8f4812677c0 R13: 981f4d0f3800 R14: R15: 981f4b477000 FS: 7f4900a2d940() GS:981f5848() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 5588121a40b0 CR3: 00018ef1e000 CR4: 003406e0 Call Trace: dcn10_create_resource_pool+0x834/0xb30 [amdgpu] ? dal_aux_engine_dce110_create+0x3a/0x80 [amdgpu] dc_create_resource_pool+0x113/0x1b0 [amdgpu] ? _cond_resched+0x15/0x30 ? __kmalloc+0x16f/0x220 dc_create+0x233/0x640 [amdgpu] ? amdgpu_cgs_create_device+0x23/0x50 [amdgpu] dm_hw_init+0xe6/0x160 [amdgpu] amdgpu_device_init.cold+0x1134/0x1428 [amdgpu] amdgpu_driver_load_kms+0x88/0x330 [amdgpu] drm_dev_register+0x116/0x160 [drm] amdgpu_pci_probe+0xb5/0x120 [amdgpu] >From /proc/cpuinfo: vendor_id : AuthenticAMD cpu family : 23 model : 17 model name : AMD Ryzen 3 2200G with Radeon Vega Graphics stepping: 0 microcode : 0x8101013 cpu MHz : 1589.201 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107209] DM_PPLIB causes a warning on Raven
https://bugs.freedesktop.org/show_bug.cgi?id=107209 vono changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |DUPLICATE --- Comment #4 from vono --- Seems to be same bug than 107296 Mark this one as duplicate since the other has more activities. *** This bug has been marked as a duplicate of bug 107296 *** -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107296] WARNING: CPU: 0 PID: 370 at drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:1355 dcn_bw_update_from_pplib+0x16b/0x280 [amdgpu]
https://bugs.freedesktop.org/show_bug.cgi?id=107296 vono changed: What|Removed |Added CC||m...@fireburn.co.uk --- Comment #14 from vono --- *** Bug 107209 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/virtio: add plane check
Use drm_atomic_helper_check_plane_state() to sanity check the plane state. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_plane.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index 024c2aa0c929..3b377bb5b5cb 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -82,7 +82,22 @@ static const struct drm_plane_funcs virtio_gpu_plane_funcs = { static int virtio_gpu_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) { - return 0; + bool is_cursor = plane->type == DRM_PLANE_TYPE_CURSOR; + struct drm_crtc_state *crtc_state; + int ret; + + if (!state->fb | !state->crtc) + return 0; + + crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc); + if (IS_ERR(crtc_state)) +return PTR_ERR(crtc_state); + + ret = drm_atomic_helper_check_plane_state(state, crtc_state, + DRM_PLANE_HELPER_NO_SCALING, + DRM_PLANE_HELPER_NO_SCALING, + is_cursor, true); + return ret; } static void virtio_gpu_primary_plane_update(struct drm_plane *plane, -- 2.18.1
[ADV7393] DRM Encoder Slave or DRM Bridge
Vikash, As it's been quite a while, I want to know if the problem is solved successfully If so, could you please shed some light on the problem solving path? Working on a custom hardware based on TI AM5728, and having the same problem at hand, I just was curious if some one has been able to write a omapdrm based driver for ADV7393. Any help would greatly be appreciated. Kind Regards, Nasser Afshin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv6 0/4] omapdrm: DSI command mode panel support
* Tomi Valkeinen [190527 10:51]: > Hi, > > On 23/05/2019 23:07, Sebastian Reichel wrote: > > Hi, > > > > Here is another round of the DSI command mode panel patchset > > integrating the feedback from PATCHv5. The patches are based > > on v5.2-rc1 tag. It does not contain the patches required for > > OMAP3 support (it needs a workaround for a hardware bug) and > > for automatic display rotation. They should get their own series, > > once after everything has been moved to DRM panel API. I think > > DRM panel conversion should happen _after_ this series, since > > otherwise there is a high risk of bricking DSI support completely. > > I already started a WIP branch for converting DSI to the DRM panel > > API on top of this patchset. > > Looks good to me. For some reason I can't boot 5.2-rc2 (on x15) so I haven't > been able to test yet. I'll pick the series up in any case, and I'll test it > when I get the kernel booting. Great good to have these merged finally :) Hmm I wonder if some x15 models are affected by the SoC variant changes queued in my fixes branch? Regards, Tony
Re: [PATCH 4/5] drm/vmwgfx: remove custom ioctl io encoding check
Hi, Emil, On Mon, 2019-05-27 at 10:08 +0100, Emil Velikov wrote: > On 2019/05/25, Thomas Hellstrom wrote: > > On Sat, 2019-05-25 at 00:39 +0200, Thomas Hellström wrote: > > > Hi, Emil > > > > > > On Fri, 2019-05-24 at 16:26 +0100, Emil Velikov wrote: > > > > On 2019/05/24, Thomas Hellstrom wrote: > > > > > On Fri, 2019-05-24 at 13:14 +0100, Emil Velikov wrote: > > > > > > On 2019/05/23, Thomas Hellstrom wrote: > > > > > > > Hi, Emil, > > > > > > > > > > > > > > On Wed, 2019-05-22 at 17:41 +0100, Emil Velikov wrote: > > > > > > > > From: Emil Velikov > > > > > > > > > > > > > > > > Drop the custom ioctl io encoding check - core drm does > > > > > > > > it > > > > > > > > for > > > > > > > > us. > > > > > > > > > > > > > > I fail to see where the core does this, or do I miss > > > > > > > something? > > > > > > > > > > > > drm_ioctl() allows for the encoding to be changed and > > > > > > attributes > > > > > > that > > > > > > only the > > > > > > appropriate size is copied in/out of the kernel. > > > > > > > > > > > > Technically the function is more relaxed relative to the > > > > > > vmwgfx > > > > > > check, yet > > > > > > seems perfectly reasonable. > > > > > > > > > > > > Is there any corner-case that isn't but should be handled > > > > > > in > > > > > > drm_ioctl()? > > > > > > > > > > I'd like to turn the question around and ask whether there's > > > > > a > > > > > reason > > > > > we should relax the vmwgfx test? In the past it has trapped > > > > > quite > > > > > a > > > > > few > > > > > user-space errors. > > > > > > > > > The way I see it either: > > > > - the check, as-is, is unnessesary, or > > > > - it is needed, and we should do something equivalent for all > > > > of > > > > DRM > > > > > > > > We had a very long brainstorming session with a colleague and > > > > we > > > > could not see > > > > any cases where this would cause a problem. If you recall > > > > anything > > > > concrete > > > > please let me know - I would be more than happy to take a > > > > closer > > > > look. > > > > > > If you have a good reason to drop an ioctl sanity check, I'd be > > > perfectly happy to do it. To me, a good reason even includes "I > > > have > > > a > > > non-open-source customer having problems with this check" because > > > of > > > reason etc. etc. as long as I have a way to evaluate those > > > reasons > > > and > > > determine if they're valid or not. "No other drm driver nor the > > > core > > > is > > > doing this" is NOT a valid reason to me. In particular if the > > > check > > > is > > > not affecting performance. So unless you provide additional > > > reasons > > > to > > > drop this check, it's a solid NAK from my side. > > > > To clarify my point of view a bit, this check is useful to early > > catch > > userspace using incorrect flags and sizes, which otherwise might > > make > > it out to distros and after that, introducing a check like this > > would > > be impossible, since it might break old user-space. For the same > > reason > > it would probably be very difficult to introduce it in core drm. > > > I think we might be talking past each other, let's take a step back: > > - as of previous patch, all of vmwgfx ioctls size is consistently > handled by the core I don't think I follow you here, AFAICT patch 3/5 only affects and relaxes the execbuf checking (and in fact a little more than I would like)? > - handling of featue flags, as always, is responsibility of the > driver > ifself > - with this patch, ioctl direction is also handled by core. > > Here core ensures we only copy in/out as much data as the kernel > implementation can handle. > > > Let's consider the following real world example - msm and virtio_gpu. > > An in field of an _IOW ioctl becomes in/out aka _IORW ioctl. > - we add a flag to annotate/request the out, as always invalid flags > are return -EINVAL > - we change the ioctl encoding > > As currently handled by core DRM, old kernel/new userspace and > vice-versa works just fine. Sadly, vmwgfx will error out, while it > could > be avoided. IMO basically we have a tradeoff between strict checking in this case, and new user-space vs old kernel "hazzle-free" transition in the relaxed case. > > As said above, I'll gladly adjust core and/or others, if this relaxed > approach causes an issue somewhere. A specific use-case, real or > hypothetical will be appreciated. To me there are two important reasons to keep the strict approach. 1) Avoid user-space mistakes early in the development cycle. We can't distinguish between buggy user-space and "new" user-space. This is important because of [a]) below. 2) Catch a lot of fuzzer combinations and error out early instead of forwarding them to the ioctl function where they may cause harm. I think the new user-space vs old kernel can be handled nicely in user- space with feature flags or API versions. That's the way we've handled them up to now? [a] But you probably can't move the stricter
Re: [PATCH 0/2] drm: imx: Add NWL MIPI DSI host controller support
Hi, On Mon, May 27, 2019 at 10:24:02AM +0800, Shawn Guo wrote: > On Wed, May 08, 2019 at 07:18:27PM +0200, Guido Günther wrote: > > If somebody is working on DCSS support it'd be cool to know since this > > I have some time slots here and will start looking at it, if no one else > is already working on it. Nice. My current forward port of a minimal DCSS needed for DSI is here: https://source.puri.sm/guido.gunther/linux-imx8/tree/imx8-5.x-drm Cheers, -- Guido ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110712] [regression]Raven Ridge: System freeze but mouse cursor able to move when using Firefox Webrender.
https://bugs.freedesktop.org/show_bug.cgi?id=110712 Haxk20 changed: What|Removed |Added Component|DRM/AMDgpu |Drivers/Gallium/radeonsi Product|DRI |Mesa QA Contact||dri-devel@lists.freedesktop ||.org Version|XOrg git|git -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110712] [regression]Raven Ridge: System freeze but mouse cursor able to move when using Firefox Webrender.
https://bugs.freedesktop.org/show_bug.cgi?id=110712 --- Comment #2 from Haxk20 --- Moved the bug over to mesa as this is mesa bug. Sorry for reporting it incorrectly the first time. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/33] fbcon notifier begone!
On Mon, May 27, 2019 at 9:17 AM Daniel Vetter wrote: > > On Sat, May 25, 2019 at 07:19:28PM +0200, Sam Ravnborg wrote: > > Hi Daniel. > > > > Good work, nice cleanup all over. > > > > A few comments to a few patches - not something that warrant a > > new series to be posted as long as it is fixed before the patches are > > applied. > > Hm yeah good idea, I'll add that to the next version. Actually I thought a bit more about the locking story, and it's a bit worse than my simple plan here. I think better to just have that floating around, and not make it look like it's an easy simple cleanup. The case I forgot about is that any places that has a printk can recurse through the console_lock, which means we need a lot more try_lock than I originally thought we'd need. -Daniel > > > > btw for future plans: I think this is tricky enough (it's old code and all > > > that) that we should let this soak for 2-3 kernel releases. I think the > > > following would be nice subsequent cleanup/fixes: > > > > > > - push the console lock completely from fbmem.c to fbcon.c. I think we're > > > mostly there with prep, but needs to pondering of corner cases. > > I wonder - should this code consistently use __acquire() etc so we could > > get a little static analysis help for the locking? > > > > I have not played with this for several years and I do not know the > > maturity of this today. > > Ime these sparse annotations are pretty useless because too inflexible. I > tend to use runtime locking checks based on lockdep. Those are more > accurate, and work even when the place the lock is taken is very far away > from where you're checking, without having to annoate all functions in > between. We need that for something like console_lock which is a very big > lock. Only downside is that only paths you hit at runtime are checked. > > > > - move fbcon.c from using indices for tracking fb_info (and accessing > > > registered_fbs without proper locking all the time) to real fb_info > > > pointers with the right amount of refcounting. Mostly motivated by the > > > fun I had trying to simplify fbcon_exit(). > > > > > > - make sure that fbcon call lock/unlock_fb when it calls fbmem.c > > > functions, and sprinkle assert_lockdep_held around in fbmem.c. This > > > needs the console_lock cleanups first. > > > > > > But I think that's material for maybe next year or so. > > Or maybe after next kernel release. > > Could we put this nice plan into todo.rst or similar so we do not have > > to hunt it down by asking google? > > > > For the whole series you can add my: > > Reviewed-by: Sam Ravnborg > > > > Some parts are reviewed as "this looks entirely correct", other parts > > I would claim that I actually understood. > > And after having spend some hours on this a r-b seems in order. > > Thanks, Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/13] drm: allow render capable master with DRM_AUTH ioctls
Am 27.05.19 um 10:17 schrieb Emil Velikov: From: Emil Velikov There are cases (in mesa and applications) where one would open the primary node without properly authenticating the client. Sometimes we don't check if the authentication succeeds, but there's also cases we simply forget to do it. The former was a case for Mesa where it did not not check the return value of drmGetMagic() [1]. That was fixed recently although, there's the question of older drivers or other apps that exbibit this behaviour. While omitting the call results in issues as seen in [2] and [3]. In the libva case, libva itself doesn't authenticate the DRM client and the vaGetDisplayDRM documentation doesn't mention if the app should either. As of today, the official vainfo utility doesn't authenticate. To workaround issues like these, some users resort to running their apps under sudo. Which admittedly isn't always a good idea. Since any DRIVER_RENDER driver has sufficient isolation between clients, we can use that, for unauthenticated [primary node] ioctls that require DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. v2: - Rework/simplify if check (Daniel V) - Add examples to commit messages, elaborate. (Daniel V) v3: - Use single unlikely (Daniel V) v4: - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU issue is fixed with earlier patch. As far as I can see this only affects the following two IOCTLs after removing DRM_AUTH from the DRM_RENDER_ALLOW IOCTLs: DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW) So I think it would be simpler to just remove DRM_AUTH from those two instead of allowing it for everybody. Regards, Christian. [1] https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136 [2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html [3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1 Testcase: igt/core_unauth_vs_render Cc: intel-...@lists.freedesktop.org Signed-off-by: Emil Velikov Reviewed-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190114085408.15933-2-emil.l.veli...@gmail.com --- drivers/gpu/drm/drm_ioctl.c | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 9841c0076f02..b64b022a2b29 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -511,6 +511,13 @@ int drm_version(struct drm_device *dev, void *data, return err; } +static inline bool +drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags) +{ + return drm_core_check_feature(dev, DRIVER_RENDER) && + (flags & DRM_RENDER_ALLOW); +} + /** * drm_ioctl_permit - Check ioctl permissions against caller * @@ -525,14 +532,19 @@ int drm_version(struct drm_device *dev, void *data, */ int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) { + const struct drm_device *dev = file_priv->minor->dev; + /* ROOT_ONLY is only for CAP_SYS_ADMIN */ if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN))) return -EACCES; - /* AUTH is only for authenticated or render client */ - if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) && -!file_priv->authenticated)) - return -EACCES; + /* AUTH is only for master ... */ + if (unlikely((flags & DRM_AUTH) && drm_is_primary_client(file_priv))) { + /* authenticated ones, or render capable on DRM_RENDER_ALLOW. */ + if (!file_priv->authenticated && + !drm_render_driver_and_ioctl(dev, flags)) + return -EACCES; + } /* MASTER is only for master or control clients */ if (unlikely((flags & DRM_MASTER) && ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/13] drm/i915: drop DRM_AUTH from DRM_RENDER_ALLOW ioctls
On 2019/05/27, Jani Nikula wrote: > On Mon, 27 May 2019, Emil Velikov wrote: > > From: Emil Velikov > > > > The authentication can be circumvented, by design, by using the render > > node. > > > > From the driver POV there is no distinction between primary and render > > nodes, thus we can drop the token. > > > > Note: the outstanding DRM_AUTH instances are: > > - legacy DRI1 ioctls, which are already neutered > > - modern but deprecated ioctls > > > > Cc: Jani Nikula > > Cc: Joonas Lahtinen > > Cc: Rodrigo Vivi > > Cc: intel-...@lists.freedesktop.org > > Cc: David Airlie > > Cc: Daniel Vetter > > Signed-off-by: Emil Velikov > > Please see > > commit b972fffa114b18a120a7bbde105d69a080d24970 > Author: Christian König > Date: Wed Apr 17 13:25:24 2019 +0200 > > drm/i915: remove DRM_AUTH from IOCTLs which also have DRM_RENDER_ALLOW > > It's headed to drm-next in [1]. > Right my series is based on drm-misc-next, which does not (yet) have the patch. I'll drop my patch when the equivalent, shows up in drm-misc-next. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/12] dma-buf: add explicit buffer pinning
On Tue, 16 Apr 2019 20:38:34 +0200 Christian König wrote: > + /** > + * @unpin_dma_buf: > + * > + * This is called by dma_buf_unpin and lets the exporter know that an > + * importer doesn't need to the DMA-buf to stay were it is any more. > + * s/need to/need/ s/were/where/ > + * This is called with the dmabuf->resv object locked. > + * > + * This callback is optional. > + */ > + void (*unpin)(struct dma_buf *); > + BR Hillf ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next] drm/amd/display: Remove set but not used variable 'pixel_width'
Fixes gcc '-Wunused-but-set-variable' warning: drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_dpp.c: In function dpp_get_optimal_number_of_taps: drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_dpp.c:137:11: warning: variable pixel_width set but not used [-Wunused-but-set-variable] It is never used and can be remove. Signed-off-by: YueHaibing --- drivers/gpu/drm/amd/display/dc/dcn10/dcn10_dpp.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_dpp.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_dpp.c index 6f4b24756323..d963d361696e 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_dpp.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_dpp.c @@ -134,13 +134,6 @@ static bool dpp_get_optimal_number_of_taps( struct scaler_data *scl_data, const struct scaling_taps *in_taps) { - uint32_t pixel_width; - - if (scl_data->viewport.width > scl_data->recout.width) - pixel_width = scl_data->recout.width; - else - pixel_width = scl_data->viewport.width; - /* Some ASICs does not support FP16 scaling, so we reject modes require this*/ if (scl_data->format == PIXEL_FORMAT_FP16 && dpp->caps->dscl_data_proc_format == DSCL_DATA_PRCESSING_FIXED_FORMAT && -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/display: fix possible condition with no effect (if == else)
fix below warning reported by coccicheck ./drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c:1364:3-5: WARNING: possible condition with no effect (if == else) Signed-off-by: Hariprasad Kelam --- drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c index f3aa7b5..0706ced 100644 --- a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c +++ b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c @@ -1361,13 +1361,7 @@ static void calculate_bandwidth( /*if stutter and dram clock state change are gated before cursor then the cursor latency hiding does not limit stutter or dram clock state change*/ for (i = 0; i <= maximum_number_of_surfaces - 1; i++) { if (data->enable[i]) { - if (dceip->graphics_lb_nodownscaling_multi_line_prefetching == 1) { - data->maximum_latency_hiding[i] = bw_add(data->minimum_latency_hiding[i], bw_mul(bw_frc_to_fixed(5, 10), data->total_dmifmc_urgent_latency)); - } - else { - /*maximum_latency_hiding(i) = minimum_latency_hiding(i) + 1 / vsr(i) * h_total(i) / pixel_rate(i) + 0.5 * total_dmifmc_urgent_latency*/ - data->maximum_latency_hiding[i] = bw_add(data->minimum_latency_hiding[i], bw_mul(bw_frc_to_fixed(5, 10), data->total_dmifmc_urgent_latency)); - } + data->maximum_latency_hiding[i] = bw_add(data->minimum_latency_hiding[i], bw_mul(bw_frc_to_fixed(5, 10), data->total_dmifmc_urgent_latency)); data->maximum_latency_hiding_with_cursor[i] = bw_min2(data->maximum_latency_hiding[i], data->cursor_latency_hiding[i]); } } -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[no subject]
From tho...@m3y3r.de Sun May 26 13:49:04 2019 Subject: [PATCH] drm/omap: Make sure device_id tables are NULL terminated To: tomi.valkei...@ti.com, airl...@linux.ie, dan...@ffwll.ch, dri-devel@lists.freedesktop.org, linux-ker...@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patch: Cocci X-Mailer: DiffSplit Message-ID: <1558871364611-249425076-1-diffsplit-tho...@m3y3r.de> References: <1558871364605-1026448693-0-diffsplit-tho...@m3y3r.de> In-Reply-To: <1558871364605-1026448693-0-diffsplit-tho...@m3y3r.de> X-Serial-No: 1 Make sure (of/i2c/platform)_device_id tables are NULL terminated. Signed-off-by: Thomas Meyer --- diff -u -p a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c --- a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c +++ b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c @@ -198,6 +198,7 @@ static const struct of_device_id omapdss { .compatible = "toppoly,td028ttec1" }, { .compatible = "tpo,td028ttec1" }, { .compatible = "tpo,td043mtea1" }, + {}, }; static int __init omapdss_boot_init(void) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
PROBLEM: VirtIO DRM driver crashes when setting specific 16.16 fixed-point property values
VirtIO DRM driver crashes when setting specific 16.16 fixed-point property values When running a virtual machine with a VirtIO GPU, it's possible to crash the entire VM by setting the value of a 16.16 fixed-point property to any value below 65536 (1.0 in 16.16 format or 0x0001). As a specific example, setting the SRC_W property on a plane DRM object to a value of 3 will cause the VM to hard-shutdown. Keywords; DRM, GPU, Virtualization, KMS, Kernel, VirtIO, Virtualization Kernel information: Linux version 4.19.44 (nixbld@localhost) (gcc version 7.4.0 (GCC)) #1-NixOS SMP Thu May 16 17:41:32 UTC 2019 Log output: No related errors in the logs. To reproduce: Create a VM with a VirtIO GPU and set the property as described above. I have a personal project that lets you execute specific DRM commands one at a time: https://github.com/Smithay/drm-rs/blob/develop/examples/kms_interactive.rs Here's a snippet of what happens: ``` $ sudo cargo run --example kms_interactive ... KMS>> GetResources# List out DRM resource Connectors: [connector::Handle(31)] Encoders: [encoder::Handle(32)] CRTCS: [crtc::Handle(30)] Framebuffers: [...] Planes: [plane::Handle(28), plane::Handle(29)] KMS>> GetProperties 28# Get properties of plane with handle 28 Property: property::Handle(7)Value: 1 Property: property::Handle(16)Value: 69 ... Property: property::Handle(10)Value: 67108864 Property: property::Handle(11)Value: 50331648 KMS>> GetInfo 10# Get info of property 10 Name: "SRC_W" Mutable: true Atomic: false Value: UnsignedRange(0, 4294967295) KMS>> SetProperty 28 10 65536# Set the value of property 10 on plane 28 to value 65536 (succeeds) KMS>> SetProperty 28 10 6# Set the value of property 10 on plane 28 to value 6 ``` At this point the VM has shut down. Environment: Linux nixos 4.19.44 #1-NixOS SMP Thu May 16 17:41:32 UTC 2019 x86_64 GNU/Linux GNU C 7.4.0 Binutils 2.31.1 Util-linux 2.33.2 Mount 2.33.2 Module-init-tools 26 E2fsprogs 1.45.0 Linux C Library 2.27 Dynamic linker (ldd) 2.27 Procps 3.3.15 Net-tools 1.60 Kbd 2.0.4 Console-tools 2.0.4 Sh-utils 8.31 Udev 239 Modules Loaded 8021q aesni_intel aes_x86_64 af_packet agpgart ahci atkbd autofs4 btrfs button cfg80211 crc32c_generic crc32c_intel crc32_pclmul crc_ccitt crct10dif_pclmul cryptd crypto_simd deflate dm_mod drm drm_kms_helper efi_pstore efivarfs efivars ehci_hcd ehci_pci evdev failover fat fb_sys_fops ghash_clmulni_intel glue_helper hid hid_generic i2c_core i2c_i801 i8042 input_leds intel_agp intel_gtt ip6table_filter ip6table_raw ip6_tables ip6t_rpfilter iptable_filter iptable_nat iptable_raw ip_tables ipt_rpfilter ipv6 irqbypass iTCO_wdt joydev kvm led_class libahci libata libcrc32c libps2 loop lpc_ich mac_hid mousedev net_failover nf_conntrack nf_defrag_ipv4 nf_defrag_ipv6 nf_log_common nf_log_ipv4 nf_log_ipv6 nf_nat nf_nat_ipv4 nls_cp437 nls_iso8859_1 pcbc psmouse pstore qemu_fw_cfg raid6_pq rfkill rng_core rtc_cmos scsi_mod serio serio_raw snd snd_hda_codec snd_hda_codec_generic snd_hda_core snd_hda_intel snd_hwdep snd_pcm snd_timer soundcore syscopyarea sysfillrect sysimgblt ttm uhci_hcd usb_common usbcore usbhid vfat virtio virtio_balloon virtio_blk virtio_console virtio_gpu virtio_net virtio_pci virtio_ring virtio_rng xor x_tables xt_conntrack xt_LOG xt_pkttype xt_tcpudp xxhash zstd_compress zstd_decompress XML Configuration for VM: ``` DRM-test 07c20472-206a-4367-8a9c-11b39b836896 4194304 4194304 2 /machine hvm /run/libvirt/nix-ovmf/OVMF_CODE.fd /var/lib/libvirt/qemu/nvram/DRM-test_VARS.fd Skylake-Client-IBRS Intel destroy restart destroy /run/libvirt/nix-emulators/qemu-system-x86_64 +0:+0 +0:+0 ``` ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/
Re: [PATCH 03/12] dma-buf: lock the reservation object during (un)map_dma_buf v3
On Tue, 16 Apr 2019 20:38:32 +0200 Christian König wrote: > @@ -688,9 +689,9 @@ struct sg_table *dma_buf_map_attachment(struct > dma_buf_attachment *attach, > if (attach->sgt) > return attach->sgt; > > - sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction); > - if (!sg_table) > - sg_table = ERR_PTR(-ENOMEM); > + reservation_object_lock(attach->dmabuf->resv, NULL); > + sg_table = dma_buf_map_attachment_locked(attach, direction); > + reservation_object_unlock(attach->dmabuf->resv); > Looks better if sg_table is checked after mapping, and feed error info back in case there is anything unusual. > return sg_table; > } Best Regards Hillf ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
DRM/AST regression (likely 4.14 -> 4.19+), providing EDID manually fails
Hi all, I've a workstation which has internal VGA that is detected as AST 2400 and with it EDID has been always quite flaky (except for some time it worked with 4.14 long enough that I thought the problems would be past until the problems reappeared also with 4.14). Thus, I've provided manually the EDID that I extracted from the monitor using other computer (the monitor itself worked just fine on the earlier computer so it is likely fine). I setup the manual EDID using drm_kms_helper.edid_firmware, however, after upgrading to 4.19.45 it stopped working (no "Got external EDID base block" appears in dmesg, the text mode is kept in the lower res mode, and Xorg logs no longer dumps the EDID info like it did with 4.14). So I guess the EDID I provided manually on the command line is not correctly put into use with 4.19+ kernels. The 4.19 dmesg indicated that drm_kms_helper.edid_firmware is deprecated so I also tested with drm.edid_firmware it suggested as the replacement but with no luck (but I believe also the drm_kms_helper one should have worked as it was only "deprecated"). I also tried 5.1.2 but it did not work any better (and with it also tried removing all the manual *.edid_firmware from the command line so I still need to provide one manually to have it reliable working it seems). -- i. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd: fix warning PTR_ERR_OR_ZERO can be used
fix below warnings reported by coccicheck ./drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c:1057:1-3: WARNING: PTR_ERR_OR_ZERO can be used Signed-off-by: Hariprasad Kelam --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c index 1d5fc5a..1b1ec12 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c @@ -1054,8 +1054,6 @@ int dtn_debugfs_init(struct amdgpu_device *adev) ent = debugfs_create_file_unsafe("amdgpu_dm_visual_confirm", 0644, root, adev, &visual_confirm_fops); - if (IS_ERR(ent)) - return PTR_ERR(ent); - return 0; + return PTR_ERR_OR_ZERO(ent); } -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH -next] drm/amdkfd: Make deallocate_hiq_sdma_mqd static
Fix sparse warning: drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:1846:6: warning: symbol 'deallocate_hiq_sdma_mqd' was not declared. Should it be static? Reported-by: Hulk Robot Signed-off-by: YueHaibing --- drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c index ece35c7a77b5..89bb8edc04bc 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c @@ -1843,7 +1843,8 @@ struct device_queue_manager *device_queue_manager_init(struct kfd_dev *dev) return NULL; } -void deallocate_hiq_sdma_mqd(struct kfd_dev *dev, struct kfd_mem_obj *mqd) +static void deallocate_hiq_sdma_mqd(struct kfd_dev *dev, + struct kfd_mem_obj *mqd) { WARN(!mqd, "No hiq sdma mqd trunk to free"); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/stm: ltdc: restore calls to clk_{enable/disable}
From: Benjamin Gaignard Restore calls to clk_{enable/disable} deleted after applying the wrong version of the patch Fixes: fd6905fca4f0 ("drm/stm: ltdc: remove clk_round_rate comment") Signed-off-by: Benjamin Gaignard --- drivers/gpu/drm/stm/ltdc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index ae2aaf2a62ee..ac29890edeb6 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -507,10 +507,12 @@ static bool ltdc_crtc_mode_fixup(struct drm_crtc *crtc, struct ltdc_device *ldev = crtc_to_ltdc(crtc); int rate = mode->clock * 1000; + clk_disable(ldev->pixel_clk); if (clk_set_rate(ldev->pixel_clk, rate) < 0) { DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate); return false; } + clk_enable(ldev->pixel_clk); adjusted_mode->clock = clk_get_rate(ldev->pixel_clk) / 1000; -- 2.15.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/6] Add anx6345 DP/eDP bridge for Olimex Teres-I
Hi all, left over from my previous Teres-I device tree series, here comes the revised anx6345 node for the Teres-I, along with the driver. The innolux panel attached to it is already known; pinebooks can be enabled on top of this series, once their panels are introduced. Changes from the respective previous versions: * the reset polarity is corrected in DT and the driver; things should be clearer now. * as requested, add a panel (the known innolux,n116bge) and connect the ports. * renamed dvdd?? to *-supply to match the established scheme * trivial update to the #include list, to make it compile in 5.2 Torsten ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/12] drm: remove prime sg_table caching
On Tue, 16 Apr 2019 20:38:35 +0200 Christian König wrote: > @ -331,14 +282,19 @@ EXPORT_SYMBOL(drm_gem_map_dma_buf); > * @sgt: scatterlist info of the buffer to unmap > * @dir: direction of DMA transfer > * > - * Not implemented. The unmap is done at drm_gem_map_detach(). This can be > - * used as the &dma_buf_ops.unmap_dma_buf callback. > + * This can be used as the &dma_buf_ops.unmap_dma_buf callback. > */ > void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach, > struct sg_table *sgt, > enum dma_data_direction dir) > { > - /* nothing to be done here */ > + if (!sgt) if (WARN_ON(!sgt)) ? > + return; > + > + dma_unmap_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir, > +DMA_ATTR_SKIP_CPU_SYNC); > + sg_free_table(sgt); > + kfree(sgt); > } > EXPORT_SYMBOL(drm_gem_unmap_dma_buf); > > -- > 2.17.1 > BR Hillf ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu/powerplay: remove duplicate entry of nbio_6_1_offset.h
asic_reg/nbio/nbio_6_1_offset.h is included twice. Issue identified by includecheck Signed-off-by: Hariprasad Kelam --- drivers/gpu/drm/amd/powerplay/hwmgr/vega12_inc.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_inc.h b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_inc.h index e6d9e84..0d08c57 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_inc.h +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_inc.h @@ -35,7 +35,6 @@ #include "asic_reg/gc/gc_9_2_1_sh_mask.h" #include "asic_reg/nbio/nbio_6_1_offset.h" -#include "asic_reg/nbio/nbio_6_1_offset.h" #include "asic_reg/nbio/nbio_6_1_sh_mask.h" #endif -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 0/6] Allwinner H6 Mali GPU support
Hi Rob, On Wed, 22 May 2019 at 21:27, Rob Herring wrote: > > On Tue, May 21, 2019 at 11:11 AM Clément Péron wrote: > > > > Hi, > > > > The Allwinner H6 has a Mali-T720 MP2 which should be supported by > > the new panfrost driver. This series fix two issues and introduce the > > dt-bindings but a simple benchmark show that it's still NOT WORKING. > > > > I'm pushing it in case someone want to continue the work. > > > > This has been tested with Mesa3D 19.1.0-RC2 and a GPU bitness patch[1]. > > > > One patch is from Icenowy Zheng where I changed the order as required > > by Rob Herring[2]. > > > > Thanks, > > Clement > > > > [1] https://gitlab.freedesktop.org/kszaq/mesa/tree/panfrost_64_32 > > [2] https://patchwork.kernel.org/patch/10699829/ > > > > > > [ 345.204813] panfrost 180.gpu: mmu irq status=1 > > [ 345.209617] panfrost 180.gpu: Unhandled Page fault in AS0 at VA > > 0x02400400 > > [ 345.209617] Reason: TODO > > [ 345.209617] raw fault status: 0x82C1 > > [ 345.209617] decoded fault status: SLAVE FAULT > > [ 345.209617] exception type 0xC1: TRANSLATION_FAULT_LEVEL1 > > [ 345.209617] access type 0x2: READ > > [ 345.209617] source id 0x8000 > > [ 345.729957] panfrost 180.gpu: gpu sched timeout, js=0, > > status=0x8, head=0x2400400, tail=0x2400400, sched_job=9e204de9 > > [ 346.055876] panfrost 180.gpu: mmu irq status=1 > > [ 346.060680] panfrost 180.gpu: Unhandled Page fault in AS0 at VA > > 0x02C00A00 > > [ 346.060680] Reason: TODO > > [ 346.060680] raw fault status: 0x810002C1 > > [ 346.060680] decoded fault status: SLAVE FAULT > > [ 346.060680] exception type 0xC1: TRANSLATION_FAULT_LEVEL1 > > [ 346.060680] access type 0x2: READ > > [ 346.060680] source id 0x8100 > > [ 346.561955] panfrost 180.gpu: gpu sched timeout, js=1, > > status=0x8, head=0x2c00a00, tail=0x2c00a00, sched_job=b55a9a85 > > [ 346.573913] panfrost 180.gpu: mmu irq status=1 > > [ 346.578707] panfrost 180.gpu: Unhandled Page fault in AS0 at VA > > 0x02C00B80 > > > > Change in v5: > > - Remove fix indent > > > > Changes in v4: > > - Add bus_clock probe > > - Fix sanity check in io-pgtable > > - Add vramp-delay > > - Merge all boards into one patch > > - Remove upstreamed Neil A. patch > > > > Change in v3 (Thanks to Maxime Ripard): > > - Reauthor Icenowy for her path > > > > Changes in v2 (Thanks to Maxime Ripard): > > - Drop GPU OPP Table > > - Add clocks and clock-names in required > > > > Clément Péron (5): > > drm: panfrost: add optional bus_clock > > iommu: io-pgtable: fix sanity check for non 48-bit mali iommu > > dt-bindings: gpu: mali-midgard: Add H6 mali gpu compatible > > arm64: dts: allwinner: Add ARM Mali GPU node for H6 > > arm64: dts: allwinner: Add mali GPU supply for H6 boards > > > > Icenowy Zheng (1): > > dt-bindings: gpu: add bus clock for Mali Midgard GPUs > > I've applied patches 1 and 3 to drm-misc. I was going to do patch 4 > too, but it doesn't apply. Thanks, I have tried to applied on drm-misc/for-linux-next but it doesn't apply too. It looks like commit d5ff1adb3809e2f74a3b57cea2e57c8e37d693c4 is missing on drm-misc ? https://github.com/torvalds/linux/commit/d5ff1adb3809e2f74a3b57cea2e57c8e37d693c4#diff-c3172f5d421d492ff91d7bb44dd44917 Clément > > Patch 2 can go in via the iommu tree and the rest via the allwinner tree. > > Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [ADV7393] DRM Encoder Slave or DRM Bridge
Hi Nasser, No, problem was not solved and I left it as priorities of my work changed. Best Regards, Vikash On Mon, May 27, 2019 at 3:08 AM nasser afshin wrote: > Hi Vikash, > > As it's been quite a while, I want to know if the problem is solved > successfully > If so, could you please shed some light on the problem solving path? > > Working on a custom hardware based on TI AM5728, and having the same > problem at hand, I just was curious if some one has been able to write a > omapdrm based driver for ADV7393. > > Any help would greatly be appreciated. > > Kind Regards, > Nasser Afshin > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/msm/dpu: Use provided drm_minor to initialize debugfs
Quoting Sean Paul (2019-05-24 10:32:18) > From: Sean Paul > > Instead of reaching into dev->primary for debugfs_root, use the minor > passed into debugfs_init. > > This avoids creating the debug directory under /sys/kernel/debug/ and > instead creates the directory under the correct node in > /sys/kernel/debug/dri// So does this make it become /sys/kernel/debug/dri//debug? I wonder why it can't all be created under /sys/kernel/debug/dri/ and then avoid the extra "debug" directory that isn't adding any value? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/amdgpu: Remove duplicate including duplicate header
remove duplicate entry of soc15.h. Issue identified by includecheck Signed-off-by: Hariprasad Kelam --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index c763733..d723332 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -34,7 +34,6 @@ #include "vega10_enum.h" #include "hdp/hdp_4_0_offset.h" -#include "soc15.h" #include "soc15_common.h" #include "clearstate_gfx9.h" #include "v9_structs.h" -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/05/27, Koenig, Christian wrote: > Am 27.05.19 um 10:17 schrieb Emil Velikov: > > From: Emil Velikov > > > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > > render node. A seemingly deliberate design decision. > > > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > > yet not all userspace checks if it's authenticated, but instead uses > > uncommon assumptions. > > > > After days of digging through git log and testing, only a single (ab)use > > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > > assuming that failure implies lack of authentication. > > > > Affected versions are: > > - the whole 18.2.x series, which is EOL > > - the whole 18.3.x series, which is EOL > > - the 19.0.x series, prior to 19.0.4 > > Well you could have saved your time, cause this is still a NAK. > Did I mention that I've spent quite a bit of time in AMDVLK? Even fixed a bug while I was there :-) > For the record: I strongly think that we don't want to expose any render > functionality on the primary node. > > To even go a step further I would say that at least on AMD hardware we > should completely disable DRI2 for one of the next generations. > It's doable and overall pretty neat idea. There is a consern that this approach may cause far more regressions that this series. We are talking about years worth of Mesa drivers (et al) that depend on render functionality exposed via the primary node. I'm OK with writing the patches, but it'll be up-to the AMDGPU team to follow-up with any regressions. Are you ok with that? Fwiw I could also move the FORCE_AUTH hack to core drm, if you prefer. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dt-bindings: display: Convert Allwinner DSI to a schema
The Allwinner SoCs have a MIPI-DSI and MIPI-D-PHY controllers supported in Linux, with a matching Device Tree binding. Now that we have the DT validation in place, let's convert the device tree bindings for that controller over to a YAML schemas. Signed-off-by: Maxime Ripard --- .../display/allwinner,sun6i-a31-mipi-dsi.yaml | 100 ++ .../bindings/display/sunxi/sun6i-dsi.txt | 93 .../phy/allwinner,sun6i-a31-mipi-dphy.yaml| 57 ++ 3 files changed, 157 insertions(+), 93 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml delete mode 100644 Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt create mode 100644 Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml new file mode 100644 index ..47950fced28d --- /dev/null +++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml @@ -0,0 +1,100 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/allwinner,sun6i-a31-mipi-dsi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Allwinner A31 MIPI-DSI Controller Device Tree Bindings + +maintainers: + - Chen-Yu Tsai + - Maxime Ripard + +properties: + "#address-cells": true + "#size-cells": true + + compatible: +const: allwinner,sun6i-a31-mipi-dsi + + reg: +maxItems: 1 + + interrupts: +maxItems: 1 + + clocks: +items: + - description: Bus Clock + - description: Module Clock + + clock-names: +items: + - const: bus + - const: mod + + resets: +maxItems: 1 + + phys: +maxItems: 1 + + phy-names: +const: dphy + + port: +type: object +description: + A port node with endpoint definitions as defined in + Documentation/devicetree/bindings/media/video-interfaces.txt. That + port should be the input endpoint, usually coming from the + associated TCON. + +patternProperties: + "^panel@[0-9]+$": true + +required: + - "#address-cells" + - "#size-cells" + - compatible + - reg + - interrupts + - clocks + - clock-names + - phys + - phy-names + - resets + - port + +additionalProperties: false + +examples: + - | +dsi0: dsi@1ca { +compatible = "allwinner,sun6i-a31-mipi-dsi"; +reg = <0x01ca 0x1000>; +interrupts = <0 89 4>; +clocks = <&ccu 23>, <&ccu 96>; +clock-names = "bus", "mod"; +resets = <&ccu 4>; +phys = <&dphy0>; +phy-names = "dphy"; +#address-cells = <1>; +#size-cells = <0>; + +panel@0 { +compatible = "bananapi,lhr050h41", "ilitek,ili9881c"; +reg = <0>; +power-gpios = <&pio 1 7 0>; /* PB07 */ +reset-gpios = <&r_pio 0 5 1>; /* PL05 */ +backlight = <&pwm_bl>; +}; + +port { +dsi0_in_tcon0: endpoint { +remote-endpoint = <&tcon0_out_dsi0>; +}; +}; +}; + +... diff --git a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt b/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt deleted file mode 100644 index 6a6cf5de08b0.. --- a/Documentation/devicetree/bindings/display/sunxi/sun6i-dsi.txt +++ /dev/null @@ -1,93 +0,0 @@ -Allwinner A31 DSI Encoder -= - -The DSI pipeline consists of two separate blocks: the DSI controller -itself, and its associated D-PHY. - -DSI Encoder - -The DSI Encoder generates the DSI signal from the TCON's. - -Required properties: - - compatible: value must be one of: -* allwinner,sun6i-a31-mipi-dsi - - reg: base address and size of memory-mapped region - - interrupts: interrupt associated to this IP - - clocks: phandles to the clocks feeding the DSI encoder -* bus: the DSI interface clock -* mod: the DSI module clock - - clock-names: the clock names mentioned above - - phys: phandle to the D-PHY - - phy-names: must be "dphy" - - resets: phandle to the reset controller driving the encoder - - - ports: A ports node with endpoint definitions as defined in -Documentation/devicetree/bindings/media/video-interfaces.txt. The -first port should be the input endpoint, usually coming from the -associated TCON. - -Any MIPI-DSI device attached to this should be described according to -the bindings defined in ../mipi-dsi-bus.txt - -D-PHY -- - -Required properties: - - compatible: value must be one of: -* allwinner,sun6i-a31-mipi-dphy - - reg: base address and size of memory-mapped region - - clocks: phandles to the clocks feeding the DSI encoder -* bus: the DSI interface clock -* mod: the DSI module clock - - clock-names: the clock names mentioned abov
Re: [PATCH 13/13] drm: allow render capable master with DRM_AUTH ioctls
On 2019/05/27, Christian König wrote: > Am 27.05.19 um 10:17 schrieb Emil Velikov: > > From: Emil Velikov > > > > There are cases (in mesa and applications) where one would open the > > primary node without properly authenticating the client. > > > > Sometimes we don't check if the authentication succeeds, but there's > > also cases we simply forget to do it. > > > > The former was a case for Mesa where it did not not check the return > > value of drmGetMagic() [1]. That was fixed recently although, there's > > the question of older drivers or other apps that exbibit this behaviour. > > > > While omitting the call results in issues as seen in [2] and [3]. > > > > In the libva case, libva itself doesn't authenticate the DRM client and > > the vaGetDisplayDRM documentation doesn't mention if the app should > > either. > > > > As of today, the official vainfo utility doesn't authenticate. > > > > To workaround issues like these, some users resort to running their apps > > under sudo. Which admittedly isn't always a good idea. > > > > Since any DRIVER_RENDER driver has sufficient isolation between clients, > > we can use that, for unauthenticated [primary node] ioctls that require > > DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. > > > > v2: > > - Rework/simplify if check (Daniel V) > > - Add examples to commit messages, elaborate. (Daniel V) > > > > v3: > > - Use single unlikely (Daniel V) > > > > v4: > > - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU > > issue is fixed with earlier patch. > > As far as I can see this only affects the following two IOCTLs after > removing DRM_AUTH from the DRM_RENDER_ALLOW IOCTLs: > > DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, > > drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), > > DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, > > drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW) > > So I think it would be simpler to just remove DRM_AUTH from those two > instead of allowing it for everybody. > If I understand you correctly this will remove DRM_AUTH also for drivers which expose only a primary node. I'm not sure if that is a good idea. That said, if others are OK with the idea I will prepare a patch. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
Am 27.05.19 um 14:05 schrieb Emil Velikov: > On 2019/05/27, Koenig, Christian wrote: >> Am 27.05.19 um 10:17 schrieb Emil Velikov: >>> From: Emil Velikov >>> >>> Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the >>> render node. A seemingly deliberate design decision. >>> >>> Hence we can drop the DRM_AUTH all together (details in follow-up patch) >>> yet not all userspace checks if it's authenticated, but instead uses >>> uncommon assumptions. >>> >>> After days of digging through git log and testing, only a single (ab)use >>> was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and >>> assuming that failure implies lack of authentication. >>> >>> Affected versions are: >>>- the whole 18.2.x series, which is EOL >>>- the whole 18.3.x series, which is EOL >>>- the 19.0.x series, prior to 19.0.4 >> Well you could have saved your time, cause this is still a NAK. >> > Did I mention that I've spent quite a bit of time in AMDVLK? Even fixed > a bug while I was there :-) Yeah, thanks for doing this. But we have done so much stuff with customers which can't be audited this way, that I still have a really bad feeling about this :/ >> For the record: I strongly think that we don't want to expose any render >> functionality on the primary node. >> >> To even go a step further I would say that at least on AMD hardware we >> should completely disable DRI2 for one of the next generations. >> > It's doable and overall pretty neat idea. > > There is a consern that this approach may cause far more regressions > that this series. We are talking about years worth of Mesa drivers (et > al) that depend on render functionality exposed via the primary node. Yeah, that's I'm perfectly aware of. It's the reason why I said we should only do it for new hardware generations. But at some point I think we should do this and get rid of authentication/flink/DRI2/ > I'm OK with writing the patches, but it'll be up-to the AMDGPU team to > follow-up with any regressions. Are you ok with that? As long as we have a check like adev->family_id > WHATEVER_IS_THE_CURRENT_LATEST_UPSTREAM_HW, I think we are fine with that. If I understood Michel correctly xf86-video-amdgpu should work, but modeset might break and needs a patch. > Fwiw I could also move the FORCE_AUTH hack to core drm, if you prefer. Well, the hack is the least of my concerns. Thanks, Christian. > > Thanks > Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/13] drm: allow render capable master with DRM_AUTH ioctls
Am 27.05.19 um 14:10 schrieb Emil Velikov: > On 2019/05/27, Christian König wrote: >> Am 27.05.19 um 10:17 schrieb Emil Velikov: >>> From: Emil Velikov >>> >>> There are cases (in mesa and applications) where one would open the >>> primary node without properly authenticating the client. >>> >>> Sometimes we don't check if the authentication succeeds, but there's >>> also cases we simply forget to do it. >>> >>> The former was a case for Mesa where it did not not check the return >>> value of drmGetMagic() [1]. That was fixed recently although, there's >>> the question of older drivers or other apps that exbibit this behaviour. >>> >>> While omitting the call results in issues as seen in [2] and [3]. >>> >>> In the libva case, libva itself doesn't authenticate the DRM client and >>> the vaGetDisplayDRM documentation doesn't mention if the app should >>> either. >>> >>> As of today, the official vainfo utility doesn't authenticate. >>> >>> To workaround issues like these, some users resort to running their apps >>> under sudo. Which admittedly isn't always a good idea. >>> >>> Since any DRIVER_RENDER driver has sufficient isolation between clients, >>> we can use that, for unauthenticated [primary node] ioctls that require >>> DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. >>> >>> v2: >>> - Rework/simplify if check (Daniel V) >>> - Add examples to commit messages, elaborate. (Daniel V) >>> >>> v3: >>> - Use single unlikely (Daniel V) >>> >>> v4: >>> - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU >>> issue is fixed with earlier patch. >> As far as I can see this only affects the following two IOCTLs after >> removing DRM_AUTH from the DRM_RENDER_ALLOW IOCTLs: >>> DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, >>> drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW), >>> DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, >>> drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW) >> So I think it would be simpler to just remove DRM_AUTH from those two >> instead of allowing it for everybody. >> > If I understand you correctly this will remove DRM_AUTH also for drivers > which expose only a primary node. I'm not sure if that is a good idea. That's a good point, but I have doubts that those drivers implement the necessary callbacks and/or set the core feature flag for the IOCTLs. So the maximum what could happen is that you change the returned error from -EACCES into -EOPNOTSUPP/-ENOSYS. Regards, Christian. > That said, if others are OK with the idea I will prepare a patch. > > Thanks > Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/stm: ltdc: restore calls to clk_{enable/disable}
Hi Benjamin, Many thanks for this fix (and more generally for pushing STM patches on misc :-) Acked-by: Philippe Cornu Philippe :-) On 5/27/19 1:58 PM, Benjamin Gaignard wrote: > From: Benjamin Gaignard > > Restore calls to clk_{enable/disable} deleted after applying the wrong > version of the patch > > Fixes: fd6905fca4f0 ("drm/stm: ltdc: remove clk_round_rate comment") > > Signed-off-by: Benjamin Gaignard > --- > drivers/gpu/drm/stm/ltdc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c > index ae2aaf2a62ee..ac29890edeb6 100644 > --- a/drivers/gpu/drm/stm/ltdc.c > +++ b/drivers/gpu/drm/stm/ltdc.c > @@ -507,10 +507,12 @@ static bool ltdc_crtc_mode_fixup(struct drm_crtc *crtc, > struct ltdc_device *ldev = crtc_to_ltdc(crtc); > int rate = mode->clock * 1000; > > + clk_disable(ldev->pixel_clk); > if (clk_set_rate(ldev->pixel_clk, rate) < 0) { > DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate); > return false; > } > + clk_enable(ldev->pixel_clk); > > adjusted_mode->clock = clk_get_rate(ldev->pixel_clk) / 1000; > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/stm: ltdc: No message if probe
Hi Yannick, Thank you for your patch Acked-by: Philippe Cornu Philippe :-) On 5/27/19 12:14 PM, Yannick Fertré wrote: > Print display controller hardware version in debug mode only. > > Signed-off-by: Yannick Fertré > --- > drivers/gpu/drm/stm/ltdc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c > index d24ffc2..16b1103 100644 > --- a/drivers/gpu/drm/stm/ltdc.c > +++ b/drivers/gpu/drm/stm/ltdc.c > @@ -1211,7 +1211,7 @@ int ltdc_load(struct drm_device *ddev) > goto err; > } > > - DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version); > + DRM_DEBUG_DRIVER("ltdc hw version 0x%08x\n", ldev->caps.hw_version); > > /* Add endpoints panels or bridges if any */ > for (i = 0; i < MAX_ENDPOINTS; i++) { >
Re: [PATCH 4/5] drm/vmwgfx: remove custom ioctl io encoding check
Hi Thomas, On 2019/05/27, Thomas Hellstrom wrote: > > I think we might be talking past each other, let's take a step back: > > > > - as of previous patch, all of vmwgfx ioctls size is consistently > > handled by the core > > I don't think I follow you here, AFAICT patch 3/5 only affects and > relaxes the execbuf checking (and in fact a little more than I would > like)? > Precisely, it makes execbuf ioctl behave like all other ioctls - both vmwgfx and rest of drm. > > - handling of featue flags, as always, is responsibility of the > > driver > > ifself > > - with this patch, ioctl direction is also handled by core. > > > > Here core ensures we only copy in/out as much data as the kernel > > implementation can handle. > > > > > > Let's consider the following real world example - msm and virtio_gpu. > > > > An in field of an _IOW ioctl becomes in/out aka _IORW ioctl. > > - we add a flag to annotate/request the out, as always invalid flags > > are return -EINVAL > > - we change the ioctl encoding > > > > As currently handled by core DRM, old kernel/new userspace and > > vice-versa works just fine. Sadly, vmwgfx will error out, while it > > could > > be avoided. > > IMO basically we have a tradeoff between strict checking in this case, > and new user-space vs old kernel "hazzle-free" transition in the > relaxed case. > Precisely. If I read the code correctly, ATM new userspace will fail against old kernels. Unless userspace writes two versions of the ioctl - with with each encoding. > > > > As said above, I'll gladly adjust core and/or others, if this relaxed > > approach causes an issue somewhere. A specific use-case, real or > > hypothetical will be appreciated. > > To me there are two important reasons to keep the strict approach. > > 1) Avoid user-space mistakes early in the development cycle. We can't > distinguish between buggy user-space and "new" user-space. This is > important because of [a]) below. > Can you provide a concrete example, please? > 2) Catch a lot of fuzzer combinations and error out early instead of > forwarding them to the ioctl function where they may cause harm. > Struggling to see why this is a problem? At some point the fuzzer will get past this first line of defence, so we want to make the rest of the ioctl is robust. > I think the new user-space vs old kernel can be handled nicely in user- > space with feature flags or API versions. That's the way we've handled > them up to now? > How is a feature flag doing to help if the encoding changes from _IOW to _IORW? Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/13] drm: allow render capable master with DRM_AUTH ioctls
On 5/27/19 10:17 AM, Emil Velikov wrote: From: Emil Velikov There are cases (in mesa and applications) where one would open the primary node without properly authenticating the client. Sometimes we don't check if the authentication succeeds, but there's also cases we simply forget to do it. The former was a case for Mesa where it did not not check the return value of drmGetMagic() [1]. That was fixed recently although, there's the question of older drivers or other apps that exbibit this behaviour. While omitting the call results in issues as seen in [2] and [3]. In the libva case, libva itself doesn't authenticate the DRM client and the vaGetDisplayDRM documentation doesn't mention if the app should either. As of today, the official vainfo utility doesn't authenticate. To workaround issues like these, some users resort to running their apps under sudo. Which admittedly isn't always a good idea. Since any DRIVER_RENDER driver has sufficient isolation between clients, we can use that, for unauthenticated [primary node] ioctls that require DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. v2: - Rework/simplify if check (Daniel V) - Add examples to commit messages, elaborate. (Daniel V) v3: - Use single unlikely (Daniel V) v4: - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU issue is fixed with earlier patch. [1] https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136 [2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html [3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1 Testcase: igt/core_unauth_vs_render Cc: intel-...@lists.freedesktop.org Signed-off-by: Emil Velikov Reviewed-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190114085408.15933-2-emil.l.veli...@gmail.com --- drivers/gpu/drm/drm_ioctl.c | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 9841c0076f02..b64b022a2b29 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -511,6 +511,13 @@ int drm_version(struct drm_device *dev, void *data, return err; } +static inline bool +drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags) +{ + return drm_core_check_feature(dev, DRIVER_RENDER) && + (flags & DRM_RENDER_ALLOW); +} + /** * drm_ioctl_permit - Check ioctl permissions against caller * @@ -525,14 +532,19 @@ int drm_version(struct drm_device *dev, void *data, */ int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) { + const struct drm_device *dev = file_priv->minor->dev; + /* ROOT_ONLY is only for CAP_SYS_ADMIN */ if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN))) return -EACCES; - /* AUTH is only for authenticated or render client */ - if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) && -!file_priv->authenticated)) - return -EACCES; + /* AUTH is only for master ... */ + if (unlikely((flags & DRM_AUTH) && drm_is_primary_client(file_priv))) { + /* authenticated ones, or render capable on DRM_RENDER_ALLOW. */ + if (!file_priv->authenticated && + !drm_render_driver_and_ioctl(dev, flags)) + return -EACCES; + } This breaks vmwgfx primary client authentication in the surface_reference ioctl, which takes different paths in case of render clients and primary clients, but adding an auth check in the primary path in the vmwgfx code should fix this. /Thomas /* MASTER is only for master or control clients */ if (unlikely((flags & DRM_MASTER) && ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 1/2] drm/bridge/synopsys: dsi: add power on/off optional phy ops
Hi Yannick, and thank you for your patch. Tested successfully on stm32f too. Reviewed-by: Philippe Cornu Tested-by: Philippe Cornu Philippe :-) On 5/27/19 12:21 PM, Yannick Fertré wrote: > Add power on & off optional physical operation functions, helpful to > program specific registers of the DSI physical part. > > Signed-off-by: Yannick Fertré > --- > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8 > include/drm/bridge/dw_mipi_dsi.h | 2 ++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > index e915ae8..5bb676f 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > @@ -775,6 +775,10 @@ static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi > *dsi) > static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge) > { > struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge); > + const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops; > + > + if (phy_ops->power_off) > + phy_ops->power_off(dsi->plat_data->priv_data); > > /* >* Switch to command mode before panel-bridge post_disable & > @@ -874,11 +878,15 @@ static void dw_mipi_dsi_bridge_mode_set(struct > drm_bridge *bridge, > static void dw_mipi_dsi_bridge_enable(struct drm_bridge *bridge) > { > struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge); > + const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops; > > /* Switch to video mode for panel-bridge enable & panel enable */ > dw_mipi_dsi_set_mode(dsi, MIPI_DSI_MODE_VIDEO); > if (dsi->slave) > dw_mipi_dsi_set_mode(dsi->slave, MIPI_DSI_MODE_VIDEO); > + > + if (phy_ops->power_on) > + phy_ops->power_on(dsi->plat_data->priv_data); > } > > static enum drm_mode_status > diff --git a/include/drm/bridge/dw_mipi_dsi.h > b/include/drm/bridge/dw_mipi_dsi.h > index 7d3dd69..df6eda6 100644 > --- a/include/drm/bridge/dw_mipi_dsi.h > +++ b/include/drm/bridge/dw_mipi_dsi.h > @@ -14,6 +14,8 @@ struct dw_mipi_dsi; > > struct dw_mipi_dsi_phy_ops { > int (*init)(void *priv_data); > + void (*power_on)(void *priv_data); > + void (*power_off)(void *priv_data); > int (*get_lane_mbps)(void *priv_data, >const struct drm_display_mode *mode, >unsigned long mode_flags, u32 lanes, u32 format, > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/05/27, Koenig, Christian wrote: > Am 27.05.19 um 14:05 schrieb Emil Velikov: > > On 2019/05/27, Koenig, Christian wrote: > >> Am 27.05.19 um 10:17 schrieb Emil Velikov: > >>> From: Emil Velikov > >>> > >>> Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > >>> render node. A seemingly deliberate design decision. > >>> > >>> Hence we can drop the DRM_AUTH all together (details in follow-up patch) > >>> yet not all userspace checks if it's authenticated, but instead uses > >>> uncommon assumptions. > >>> > >>> After days of digging through git log and testing, only a eingle (ab)use > >>> was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > >>> assuming that failure implies lack of authentication. > >>> > >>> Affected versions are: > >>>- the whole 18.2.x series, which is EOL > >>>- the whole 18.3.x series, which is EOL > >>>- the 19.0.x series, prior to 19.0.4 > >> Well you could have saved your time, cause this is still a NAK. > >> > > Did I mention that I've spent quite a bit of time in AMDVLK? Even fixed > > a bug while I was there :-) > > Yeah, thanks for doing this. > > But we have done so much stuff with customers which can't be audited > this way, that I still have a really bad feeling about this :/ > Fair point, I've already thought about those. Example A: customer X, using closed source/private software Y. Ioctls A, B and C require the old behaviour - simply add FORCE_AUTH to the A B C and carry on happily. Example B: the team, as a whole thinks that this will be problematic for customer X - add FORCE_AUTH to all ioctls and carry on. I do see and understand why anyone can be hesitant about the series. IMHO the above examples, illustrate quite reasonable compromise between open-source and closed-source/private solutions. Don't you agree? > >> For the record: I strongly think that we don't want to expose any render > >> functionality on the primary node. > >> > >> To even go a step further I would say that at least on AMD hardware we > >> should completely disable DRI2 for one of the next generations. > >> > > It's doable and overall pretty neat idea. > > > > There is a consern that this approach may cause far more regressions > > that this series. We are talking about years worth of Mesa drivers (et > > al) that depend on render functionality exposed via the primary node. > > Yeah, that's I'm perfectly aware of. It's the reason why I said we > should only do it for new hardware generations. > > But at some point I think we should do this and get rid of > authentication/flink/DRI2/ > Fwiw I share a similar sentiment. If you've looked through the series, this is effectively step 1, towards nuking DRM_AUTH :-) > > I'm OK with writing the patches, but it'll be up-to the AMDGPU team to > > follow-up with any regressions. Are you ok with that? > > As long as we have a check like adev->family_id > > WHATEVER_IS_THE_CURRENT_LATEST_UPSTREAM_HW, I think we are fine with that. > > If I understood Michel correctly xf86-video-amdgpu should work, but > modeset might break and needs a patch. > Unless I have concrete WHATEVER_IS_THE... I cannot do much here :-( Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/2] drm/stm: dsi: add power on/off phy ops
Hi Yannick, and thank you for your patch. Tested successfully on stm32f too. Acked-by: Philippe Cornu Tested-by: Philippe Cornu Philippe :-) On 5/27/19 12:21 PM, Yannick Fertré wrote: > These new physical operations are helpful to power_on/off the dsi > wrapper. If the dsi wrapper is powered in video mode, the display > controller (ltdc) register access will hang when DSI fifos are full. > > Signed-off-by: Yannick Fertré > --- > drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 21 - > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > index 01db020..0ab32fe 100644 > --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > @@ -210,10 +210,27 @@ static int dw_mipi_dsi_phy_init(void *priv_data) > if (ret) > DRM_DEBUG_DRIVER("!TIMEOUT! waiting PLL, let's continue\n"); > > + return 0; > +} > + > +static void dw_mipi_dsi_phy_power_on(void *priv_data) > +{ > + struct dw_mipi_dsi_stm *dsi = priv_data; > + > + DRM_DEBUG_DRIVER("\n"); > + > /* Enable the DSI wrapper */ > dsi_set(dsi, DSI_WCR, WCR_DSIEN); > +} > > - return 0; > +static void dw_mipi_dsi_phy_power_off(void *priv_data) > +{ > + struct dw_mipi_dsi_stm *dsi = priv_data; > + > + DRM_DEBUG_DRIVER("\n"); > + > + /* Disable the DSI wrapper */ > + dsi_clear(dsi, DSI_WCR, WCR_DSIEN); > } > > static int > @@ -287,6 +304,8 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct > drm_display_mode *mode, > > static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_stm_phy_ops = { > .init = dw_mipi_dsi_phy_init, > + .power_on = dw_mipi_dsi_phy_power_on, > + .power_off = dw_mipi_dsi_phy_power_off, > .get_lane_mbps = dw_mipi_dsi_get_lane_mbps, > }; > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/13] drm: allow render capable master with DRM_AUTH ioctls
On 2019/05/27, Thomas Hellstrom wrote: > On 5/27/19 10:17 AM, Emil Velikov wrote: > > From: Emil Velikov > > > > There are cases (in mesa and applications) where one would open the > > primary node without properly authenticating the client. > > > > Sometimes we don't check if the authentication succeeds, but there's > > also cases we simply forget to do it. > > > > The former was a case for Mesa where it did not not check the return > > value of drmGetMagic() [1]. That was fixed recently although, there's > > the question of older drivers or other apps that exbibit this behaviour. > > > > While omitting the call results in issues as seen in [2] and [3]. > > > > In the libva case, libva itself doesn't authenticate the DRM client and > > the vaGetDisplayDRM documentation doesn't mention if the app should > > either. > > > > As of today, the official vainfo utility doesn't authenticate. > > > > To workaround issues like these, some users resort to running their apps > > under sudo. Which admittedly isn't always a good idea. > > > > Since any DRIVER_RENDER driver has sufficient isolation between clients, > > we can use that, for unauthenticated [primary node] ioctls that require > > DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. > > > > v2: > > - Rework/simplify if check (Daniel V) > > - Add examples to commit messages, elaborate. (Daniel V) > > > > v3: > > - Use single unlikely (Daniel V) > > > > v4: > > - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU > > issue is fixed with earlier patch. > > > > [1] > > https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136 > > [2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html > > [3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1 > > Testcase: igt/core_unauth_vs_render > > Cc: intel-...@lists.freedesktop.org > > Signed-off-by: Emil Velikov > > Reviewed-by: Daniel Vetter > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20190114085408.15933-2-emil.l.veli...@gmail.com > > --- > > drivers/gpu/drm/drm_ioctl.c | 20 > > 1 file changed, 16 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > > index 9841c0076f02..b64b022a2b29 100644 > > --- a/drivers/gpu/drm/drm_ioctl.c > > +++ b/drivers/gpu/drm/drm_ioctl.c > > @@ -511,6 +511,13 @@ int drm_version(struct drm_device *dev, void *data, > > return err; > > } > > +static inline bool > > +drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags) > > +{ > > + return drm_core_check_feature(dev, DRIVER_RENDER) && > > + (flags & DRM_RENDER_ALLOW); > > +} > > + > > /** > >* drm_ioctl_permit - Check ioctl permissions against caller > >* > > @@ -525,14 +532,19 @@ int drm_version(struct drm_device *dev, void *data, > >*/ > > int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) > > { > > + const struct drm_device *dev = file_priv->minor->dev; > > + > > /* ROOT_ONLY is only for CAP_SYS_ADMIN */ > > if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN))) > > return -EACCES; > > - /* AUTH is only for authenticated or render client */ > > - if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) && > > -!file_priv->authenticated)) > > - return -EACCES; > > + /* AUTH is only for master ... */ > > + if (unlikely((flags & DRM_AUTH) && drm_is_primary_client(file_priv))) { > > + /* authenticated ones, or render capable on DRM_RENDER_ALLOW. */ > > + if (!file_priv->authenticated && > > + !drm_render_driver_and_ioctl(dev, flags)) > > + return -EACCES; > > + } > > This breaks vmwgfx primary client authentication in the surface_reference > ioctl, which takes different paths in case of render clients and primary > clients, but adding an auth check in the primary path in the vmwgfx code > should fix this. > Ack. Thanks for having a look. Will include a permission check in v2 of the series. -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #8 from Nicholas Kazlauskas --- Do you happen to know if this was a regression? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On Mon, May 27, 2019 at 09:17:29AM +0100, Emil Velikov wrote: > From: Emil Velikov > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > render node. A seemingly deliberate design decision. > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > yet not all userspace checks if it's authenticated, but instead uses > uncommon assumptions. > > After days of digging through git log and testing, only a single (ab)use > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > assuming that failure implies lack of authentication. Maybe correction here: Id does not care about authentication at all. It wants to figure out whether it has access to modeset resources, which is something entirely different, but happened to match in amdgpu's case. > Affected versions are: > - the whole 18.2.x series, which is EOL > - the whole 18.3.x series, which is EOL > - the 19.0.x series, prior to 19.0.4 Hm I think I'm blind, I'm not seeing the fix merged to mesa master. Still there on gitlab: https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/amd/vulkan/radv_device.c#L291 What am I missing? > Add a special quirk for that case, thus we can drop DRM_AUTH bits as > mentioned earlier. > > Since all the affected userspace is EOL, we also add a kconfig option > to disable this quirk. > > The whole approach is inspired by DRIVER_KMS_LEGACY_CONTEXT > > Cc: Alex Deucher > Cc: Christian König > Cc: amd-...@lists.freedesktop.org > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Emil Velikov Aside from the nits I think reasonable approach. -Daniel > --- > drivers/gpu/drm/amd/amdgpu/Kconfig | 16 > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 12 +++- > drivers/gpu/drm/drm_ioctl.c | 5 + > include/drm/drm_ioctl.h | 17 + > 4 files changed, 49 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig > b/drivers/gpu/drm/amd/amdgpu/Kconfig > index 9221e5489069..da415f445187 100644 > --- a/drivers/gpu/drm/amd/amdgpu/Kconfig > +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig > @@ -40,6 +40,22 @@ config DRM_AMDGPU_GART_DEBUGFS > Selecting this option creates a debugfs file to inspect the mapped > pages. Uses more memory for housekeeping, enable only for debugging. > > +config DRM_AMDGPU_FORCE_AUTH > + bool "Force authentication check on AMDGPU_INFO ioctl" > + default y > + help > + There were some version of the Mesa RADV drivers, which relied on > + the ioctl failing, if the client is not authenticated. > + > + Namely, the following versions are affected: > + - the whole 18.2.x series, which is EOL > + - the whole 18.3.x series, which is EOL > + - the 19.0.x series, prior to 19.0.4 > + > + Modern distributions, should disable this. That will allow various > + other clients to work, that would otherwise require root privileges. > + > + > source "drivers/gpu/drm/amd/acp/Kconfig" > source "drivers/gpu/drm/amd/display/Kconfig" > source "drivers/gpu/drm/amd/amdkfd/Kconfig" > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > index b17d0545728e..b8076929440b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > @@ -1214,7 +1214,17 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_WAIT_IDLE, amdgpu_gem_wait_idle_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_CS, amdgpu_cs_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > - DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > + /* The DRM_FORCE_AUTH is effectively a workaround for the RADV Mesa > driver. > + * This is required for Mesa: > + * - the whole 18.2.x series, which is EOL > + * - the whole 18.3.x series, which is EOL > + * - the 19.0.x series, prior to 19.0.4 > + */ > + DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, > +#if defined(DRM_AMDGPU_FORCE_AUTH) > + DRM_FORCE_AUTH| > +#endif > + DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_CS, amdgpu_cs_wait_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_FENCES, amdgpu_cs_wait_fences_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_METADATA, amdgpu_gem_metadata_ioctl, > DRM_AUTH|DRM_RENDER_ALLOW), > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index 2263e3ddd822..9841c0076f02 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -544,6 +544,11 @@ int drm_ioctl_permit(u32 flags, struct drm_file > *file_priv) >drm_is_render_client(file_priv))) > return -EACCES; > > +
Re: [PATCH 13/13] drm: allow render capable master with DRM_AUTH ioctls
On Mon, May 27, 2019 at 02:39:18PM +0200, Thomas Hellstrom wrote: > On 5/27/19 10:17 AM, Emil Velikov wrote: > > From: Emil Velikov > > > > There are cases (in mesa and applications) where one would open the > > primary node without properly authenticating the client. > > > > Sometimes we don't check if the authentication succeeds, but there's > > also cases we simply forget to do it. > > > > The former was a case for Mesa where it did not not check the return > > value of drmGetMagic() [1]. That was fixed recently although, there's > > the question of older drivers or other apps that exbibit this behaviour. > > > > While omitting the call results in issues as seen in [2] and [3]. > > > > In the libva case, libva itself doesn't authenticate the DRM client and > > the vaGetDisplayDRM documentation doesn't mention if the app should > > either. > > > > As of today, the official vainfo utility doesn't authenticate. > > > > To workaround issues like these, some users resort to running their apps > > under sudo. Which admittedly isn't always a good idea. > > > > Since any DRIVER_RENDER driver has sufficient isolation between clients, > > we can use that, for unauthenticated [primary node] ioctls that require > > DRM_AUTH. But only if the respective ioctl is tagged as DRM_RENDER_ALLOW. > > > > v2: > > - Rework/simplify if check (Daniel V) > > - Add examples to commit messages, elaborate. (Daniel V) > > > > v3: > > - Use single unlikely (Daniel V) > > > > v4: > > - Patch was reverted because it broke AMDGPU, apply again. The AMDGPU > > issue is fixed with earlier patch. > > > > [1] > > https://gitlab.freedesktop.org/mesa/mesa/blob/2bc1f5c2e70fe3b4d41f060af9859bc2a94c5b62/src/egl/drivers/dri2/platform_wayland.c#L1136 > > [2] https://lists.freedesktop.org/archives/libva/2016-July/004185.html > > [3] https://gitlab.freedesktop.org/mesa/kmscube/issues/1 > > Testcase: igt/core_unauth_vs_render > > Cc: intel-...@lists.freedesktop.org > > Signed-off-by: Emil Velikov > > Reviewed-by: Daniel Vetter > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20190114085408.15933-2-emil.l.veli...@gmail.com > > --- > > drivers/gpu/drm/drm_ioctl.c | 20 > > 1 file changed, 16 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > > index 9841c0076f02..b64b022a2b29 100644 > > --- a/drivers/gpu/drm/drm_ioctl.c > > +++ b/drivers/gpu/drm/drm_ioctl.c > > @@ -511,6 +511,13 @@ int drm_version(struct drm_device *dev, void *data, > > return err; > > } > > +static inline bool > > +drm_render_driver_and_ioctl(const struct drm_device *dev, u32 flags) > > +{ > > + return drm_core_check_feature(dev, DRIVER_RENDER) && > > + (flags & DRM_RENDER_ALLOW); > > +} > > + > > /** > >* drm_ioctl_permit - Check ioctl permissions against caller > >* > > @@ -525,14 +532,19 @@ int drm_version(struct drm_device *dev, void *data, > >*/ > > int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) > > { > > + const struct drm_device *dev = file_priv->minor->dev; > > + > > /* ROOT_ONLY is only for CAP_SYS_ADMIN */ > > if (unlikely((flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN))) > > return -EACCES; > > - /* AUTH is only for authenticated or render client */ > > - if (unlikely((flags & DRM_AUTH) && !drm_is_render_client(file_priv) && > > -!file_priv->authenticated)) > > - return -EACCES; > > + /* AUTH is only for master ... */ > > + if (unlikely((flags & DRM_AUTH) && drm_is_primary_client(file_priv))) { > > + /* authenticated ones, or render capable on DRM_RENDER_ALLOW. */ > > + if (!file_priv->authenticated && > > + !drm_render_driver_and_ioctl(dev, flags)) > > + return -EACCES; > > + } > > This breaks vmwgfx primary client authentication in the surface_reference > ioctl, which takes different paths in case of render clients and primary > clients, but adding an auth check in the primary path in the vmwgfx code > should fix this. Hm yeah we need to adjust that ... otoh kinda not sure why this is gated on authentication status, and not on "am I master or not" status. At least from a very cursory read ... -Daniel > > /Thomas > > > > /* MASTER is only for master or control clients */ > > if (unlikely((flags & DRM_MASTER) && > > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- 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
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On Mon, May 27, 2019 at 10:47:39AM +, Koenig, Christian wrote: > Am 27.05.19 um 10:17 schrieb Emil Velikov: > > From: Emil Velikov > > > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > > render node. A seemingly deliberate design decision. > > > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > > yet not all userspace checks if it's authenticated, but instead uses > > uncommon assumptions. > > > > After days of digging through git log and testing, only a single (ab)use > > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > > assuming that failure implies lack of authentication. > > > > Affected versions are: > > - the whole 18.2.x series, which is EOL > > - the whole 18.3.x series, which is EOL > > - the 19.0.x series, prior to 19.0.4 > > Well you could have saved your time, cause this is still a NAK. > > For the record: I strongly think that we don't want to expose any render > functionality on the primary node. > > To even go a step further I would say that at least on AMD hardware we > should completely disable DRI2 for one of the next generations. > > As a follow up I would then completely disallow the DRM authentication > for amdgpu, so that the command submission interface on the primary node > can only be used by the display server. So amdgpu is running in one direction, while everyone else is running in the other direction? Please note that your patch to change i915 landed already, so that ship is sailing (but we could ofc revert that back again). Imo really not a great idea if we do a amdgpu vs. everyone else split here. If we want to deprecate dri2/flink/rendering on primary nodes across the stack, then that should be a stack wide decision, not an amdgpu one. Same for the other one, i.e. this stuff here. -Daniel > > Regards, > Christian. > > > > > Add a special quirk for that case, thus we can drop DRM_AUTH bits as > > mentioned earlier. > > > > Since all the affected userspace is EOL, we also add a kconfig option > > to disable this quirk. > > > > The whole approach is inspired by DRIVER_KMS_LEGACY_CONTEXT > > > > Cc: Alex Deucher > > Cc: Christian König > > Cc: amd-...@lists.freedesktop.org > > Cc: David Airlie > > Cc: Daniel Vetter > > Signed-off-by: Emil Velikov > > --- > > drivers/gpu/drm/amd/amdgpu/Kconfig | 16 > > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 12 +++- > > drivers/gpu/drm/drm_ioctl.c | 5 + > > include/drm/drm_ioctl.h | 17 + > > 4 files changed, 49 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig > > b/drivers/gpu/drm/amd/amdgpu/Kconfig > > index 9221e5489069..da415f445187 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/Kconfig > > +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig > > @@ -40,6 +40,22 @@ config DRM_AMDGPU_GART_DEBUGFS > > Selecting this option creates a debugfs file to inspect the mapped > > pages. Uses more memory for housekeeping, enable only for debugging. > > > > +config DRM_AMDGPU_FORCE_AUTH > > + bool "Force authentication check on AMDGPU_INFO ioctl" > > + default y > > + help > > + There were some version of the Mesa RADV drivers, which relied on > > + the ioctl failing, if the client is not authenticated. > > + > > + Namely, the following versions are affected: > > + - the whole 18.2.x series, which is EOL > > + - the whole 18.3.x series, which is EOL > > + - the 19.0.x series, prior to 19.0.4 > > + > > + Modern distributions, should disable this. That will allow various > > + other clients to work, that would otherwise require root privileges. > > + > > + > > source "drivers/gpu/drm/amd/acp/Kconfig" > > source "drivers/gpu/drm/amd/display/Kconfig" > > source "drivers/gpu/drm/amd/amdkfd/Kconfig" > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > index b17d0545728e..b8076929440b 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > @@ -1214,7 +1214,17 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { > > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, > > DRM_AUTH|DRM_RENDER_ALLOW), > > DRM_IOCTL_DEF_DRV(AMDGPU_GEM_WAIT_IDLE, amdgpu_gem_wait_idle_ioctl, > > DRM_AUTH|DRM_RENDER_ALLOW), > > DRM_IOCTL_DEF_DRV(AMDGPU_CS, amdgpu_cs_ioctl, > > DRM_AUTH|DRM_RENDER_ALLOW), > > - DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, > > DRM_AUTH|DRM_RENDER_ALLOW), > > + /* The DRM_FORCE_AUTH is effectively a workaround for the RADV Mesa > > driver. > > +* This is required for Mesa: > > +* - the whole 18.2.x series, which is EOL > > +* - the whole 18.3.x series, which is EOL > > +* - the 19.0.x series, prior to 19.0.4 > > +*/ > > + DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, > > +#if defined(DRM_AMDGPU_FORCE_AUTH) >
Re: [PATCH] drm/komeda: Added AFBC support for komeda driver
On Mon, May 27, 2019 at 06:51:18AM +, james qian wang (Arm Technology China) wrote: > On Fri, May 24, 2019 at 03:12:26PM +0300, Ville Syrjälä wrote: > > On Fri, May 24, 2019 at 11:10:09AM +, Brian Starkey wrote: > > > Hi, > > > > > > On Tue, May 21, 2019 at 09:45:58AM +0100, james qian wang (Arm Technology > > > China) wrote: > > > > On Thu, May 16, 2019 at 09:57:49PM +0800, Ayan Halder wrote: > > > > > On Thu, Apr 04, 2019 at 12:06:14PM +0100, james qian wang (Arm > > > > > Technology China) wrote: > > > > > > > > > > > > +static int > > > > > > +komeda_fb_afbc_size_check(struct komeda_fb *kfb, struct drm_file > > > > > > *file, > > > > > > + const struct drm_mode_fb_cmd2 *mode_cmd) > > > > > > +{ > > > > > > + struct drm_framebuffer *fb = &kfb->base; > > > > > > + const struct drm_format_info *info = fb->format; > > > > > > + struct drm_gem_object *obj; > > > > > > + u32 alignment_w = 0, alignment_h = 0, alignment_header; > > > > > > + u32 n_blocks = 0, min_size = 0; > > > > > > + > > > > > > + obj = drm_gem_object_lookup(file, mode_cmd->handles[0]); > > > > > > + if (!obj) { > > > > > > + DRM_DEBUG_KMS("Failed to lookup GEM object\n"); > > > > > > + return -ENOENT; > > > > > > + } > > > > > > + > > > > > > + switch (fb->modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) { > > > > > > + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8: > > > > > > + alignment_w = 32; > > > > > > + alignment_h = 8; > > > > > > + break; > > > > > > + case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16: > > > > > > + alignment_w = 16; > > > > > > + alignment_h = 16; > > > > > > + break; > > > > > > + default: > > > > > Can we have something like a warn here ? > > > > > > > > will add a WARN here. > > > > > > > > > > I think it's better not to. fb->modifier comes from > > > userspace, so a malicious app could spam us with WARNs, effectively > > > dos-ing the system. -EINVAL should be sufficient. > > > > Should probably check that the entire modifier+format is > > actually valid. Otherwise you risk passing on a bogus > > modifier deeper into the driver which may trigger > > interesting bugs. > > > > Also theoretically (however unlikely) some broken userspace > > might start to depend on the ability to create framebuffers > > with crap modifiers, which could later break if you change > > the way you handle the modifiers. Then you're stuck between > > the rock and hard place because you can't break existing > > userspace but you still want to change the way modifiers > > are handled in the kernel. > > > > Best not give userspace too much rope IMO. Two ways to go about > > that: > > 1) drm_any_plane_has_format() (assumes your .format_mod_supported() > >does its job properly) > > 2) roll your own > > > > -- > > Ville Syrjälä > > Intel > > Hi Brian & Ville: > > komed has a format+modifier check before the fb size check. > and for komeda_fb_create, the first step is do the format+modifier > check, the size check is the furthur check after the such format > valid check. and the detailed fb_create is like: > > struct drm_framebuffer * > komeda_fb_create(struct drm_device *dev, struct drm_file *file, >const struct drm_mode_fb_cmd2 *mode_cmd) > { > ... > /* Step 1: format+modifier valid check, if it can not be support, > * get_format_caps will return a NULL ptr. > */ > kfb->format_caps = komeda_get_format_caps(&mdev->fmt_tbl, > mode_cmd->pixel_format, > mode_cmd->modifier[0]); > if (!kfb->format_caps) { > DRM_DEBUG_KMS("FMT %x is not supported.\n", > mode_cmd->pixel_format); > kfree(kfb); > return ERR_PTR(-EINVAL); > } > > drm_helper_mode_fill_fb_struct(dev, &kfb->base, mode_cmd); > > /* step 2, do the size check */ > if (kfb->base.modifier) > ret = komeda_fb_afbc_size_check(kfb, file, mode_cmd); > else > ret = komeda_fb_none_afbc_size_check(mdev, kfb, file, mode_cmd); > if (ret < 0) > goto err_cleanup; > > ... > } > > So theoretically, the WARN in step2 is redundant if get_format_caps > function has no problem. :). the WARN here is only for reporting > the kernel BUG or code inconsitent with format caps check and the > fb size check. And I agree, basically it will not happene. > @Brian, I'm Ok to remove it. :) > > @Ville: > Basically komeda follow the way-1, but a little improvement for > matching komeda's requirement. for komeda which will do two level's > format+modifier check. > 1). In fb_create, A roughly check to see if the format+modifier can be > supported by current HW. Yeah, looks like it shouldn't allow any unspecfied modifiers to sneak through. So should be good. > 2). In plane_atomi
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On Mon, May 27, 2019 at 01:52:05PM +0100, Emil Velikov wrote: > On 2019/05/27, Koenig, Christian wrote: > > Am 27.05.19 um 14:05 schrieb Emil Velikov: > > > On 2019/05/27, Koenig, Christian wrote: > > >> Am 27.05.19 um 10:17 schrieb Emil Velikov: > > >>> From: Emil Velikov > > >>> > > >>> Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > > >>> render node. A seemingly deliberate design decision. > > >>> > > >>> Hence we can drop the DRM_AUTH all together (details in follow-up patch) > > >>> yet not all userspace checks if it's authenticated, but instead uses > > >>> uncommon assumptions. > > >>> > > >>> After days of digging through git log and testing, only a eingle (ab)use > > >>> was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > > >>> assuming that failure implies lack of authentication. > > >>> > > >>> Affected versions are: > > >>>- the whole 18.2.x series, which is EOL > > >>>- the whole 18.3.x series, which is EOL > > >>>- the 19.0.x series, prior to 19.0.4 > > >> Well you could have saved your time, cause this is still a NAK. > > >> > > > Did I mention that I've spent quite a bit of time in AMDVLK? Even fixed > > > a bug while I was there :-) > > > > Yeah, thanks for doing this. > > > > But we have done so much stuff with customers which can't be audited > > this way, that I still have a really bad feeling about this :/ > > > Fair point, I've already thought about those. > > Example A: customer X, using closed source/private software Y. > Ioctls A, B and C require the old behaviour - simply add FORCE_AUTH to > the A B C and carry on happily. Hm, if the entire concern here is all the bazillion different versions of blobs shipped in the past few years, why can't we just have a revert of this in the amdgpu DKMS? Not like one more patch among the hundres will matter. I'd suspect that the overlap of people wanting to run the blobs and people who don't run the DKMS or similar is roughly 0. Always been the case here at Intel at least. If there's other stuff that we need to audit (like rocm or whatever), then we should look into those ofc. -Daniel > > Example B: the team, as a whole thinks that this will be problematic for > customer X - add FORCE_AUTH to all ioctls and carry on. > > I do see and understand why anyone can be hesitant about the series. > > IMHO the above examples, illustrate quite reasonable compromise between > open-source and closed-source/private solutions. > > Don't you agree? > > > >> For the record: I strongly think that we don't want to expose any render > > >> functionality on the primary node. > > >> > > >> To even go a step further I would say that at least on AMD hardware we > > >> should completely disable DRI2 for one of the next generations. > > >> > > > It's doable and overall pretty neat idea. > > > > > > There is a consern that this approach may cause far more regressions > > > that this series. We are talking about years worth of Mesa drivers (et > > > al) that depend on render functionality exposed via the primary node. > > > > Yeah, that's I'm perfectly aware of. It's the reason why I said we > > should only do it for new hardware generations. > > > > But at some point I think we should do this and get rid of > > authentication/flink/DRI2/ > > > Fwiw I share a similar sentiment. If you've looked through the series, > this is effectively step 1, towards nuking DRM_AUTH :-) > > > > I'm OK with writing the patches, but it'll be up-to the AMDGPU team to > > > follow-up with any regressions. Are you ok with that? > > > > As long as we have a check like adev->family_id > > > WHATEVER_IS_THE_CURRENT_LATEST_UPSTREAM_HW, I think we are fine with that. > > > > If I understood Michel correctly xf86-video-amdgpu should work, but > > modeset might break and needs a patch. > > > Unless I have concrete WHATEVER_IS_THE... I cannot do much here :-( > > > Thanks > Emil -- 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
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/05/27, Daniel Vetter wrote: > On Mon, May 27, 2019 at 10:47:39AM +, Koenig, Christian wrote: > > Am 27.05.19 um 10:17 schrieb Emil Velikov: > > > From: Emil Velikov > > > > > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > > > render node. A seemingly deliberate design decision. > > > > > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > > > yet not all userspace checks if it's authenticated, but instead uses > > > uncommon assumptions. > > > > > > After days of digging through git log and testing, only a single (ab)use > > > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > > > assuming that failure implies lack of authentication. > > > > > > Affected versions are: > > > - the whole 18.2.x series, which is EOL > > > - the whole 18.3.x series, which is EOL > > > - the 19.0.x series, prior to 19.0.4 > > > > Well you could have saved your time, cause this is still a NAK. > > > > For the record: I strongly think that we don't want to expose any render > > functionality on the primary node. > > > > To even go a step further I would say that at least on AMD hardware we > > should completely disable DRI2 for one of the next generations. > > > > As a follow up I would then completely disallow the DRM authentication > > for amdgpu, so that the command submission interface on the primary node > > can only be used by the display server. > > So amdgpu is running in one direction, while everyone else is running in > the other direction? Please note that your patch to change i915 landed > already, so that ship is sailing (but we could ofc revert that back > again). > > Imo really not a great idea if we do a amdgpu vs. everyone else split > here. If we want to deprecate dri2/flink/rendering on primary nodes across > the stack, then that should be a stack wide decision, not an amdgpu one. > Cannot agree more - I would love to see drivers stay consistent. Fwiw, this series consistently paves the way toward nuking DRM_AUTH ;-) Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On Mon, May 27, 2019 at 3:26 PM Daniel Vetter wrote: > > On Mon, May 27, 2019 at 01:52:05PM +0100, Emil Velikov wrote: > > On 2019/05/27, Koenig, Christian wrote: > > > Am 27.05.19 um 14:05 schrieb Emil Velikov: > > > > On 2019/05/27, Koenig, Christian wrote: > > > >> Am 27.05.19 um 10:17 schrieb Emil Velikov: > > > >>> From: Emil Velikov > > > >>> > > > >>> Currently one can circumvent DRM_AUTH, when the ioctl is exposed via > > > >>> the > > > >>> render node. A seemingly deliberate design decision. > > > >>> > > > >>> Hence we can drop the DRM_AUTH all together (details in follow-up > > > >>> patch) > > > >>> yet not all userspace checks if it's authenticated, but instead uses > > > >>> uncommon assumptions. > > > >>> > > > >>> After days of digging through git log and testing, only a eingle > > > >>> (ab)use > > > >>> was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > > > >>> assuming that failure implies lack of authentication. > > > >>> > > > >>> Affected versions are: > > > >>>- the whole 18.2.x series, which is EOL > > > >>>- the whole 18.3.x series, which is EOL > > > >>>- the 19.0.x series, prior to 19.0.4 > > > >> Well you could have saved your time, cause this is still a NAK. > > > >> > > > > Did I mention that I've spent quite a bit of time in AMDVLK? Even fixed > > > > a bug while I was there :-) > > > > > > Yeah, thanks for doing this. > > > > > > But we have done so much stuff with customers which can't be audited > > > this way, that I still have a really bad feeling about this :/ > > > > > Fair point, I've already thought about those. > > > > Example A: customer X, using closed source/private software Y. > > Ioctls A, B and C require the old behaviour - simply add FORCE_AUTH to > > the A B C and carry on happily. > > Hm, if the entire concern here is all the bazillion different versions of > blobs shipped in the past few years, why can't we just have a revert of > this in the amdgpu DKMS? Not like one more patch among the hundres will > matter. I'd suspect that the overlap of people wanting to run the blobs > and people who don't run the DKMS or similar is roughly 0. Always been the > case here at Intel at least. > > If there's other stuff that we need to audit (like rocm or whatever), then > we should look into those ofc. Also note that Emil's patch actually doesn't break anything, since default y. So you don't even need the revert (except maybe in 10 years or so when we throw that option out). -Daniel > > Example B: the team, as a whole thinks that this will be problematic for > > customer X - add FORCE_AUTH to all ioctls and carry on. > > > > I do see and understand why anyone can be hesitant about the series. > > > > IMHO the above examples, illustrate quite reasonable compromise between > > open-source and closed-source/private solutions. > > > > Don't you agree? > > > > > >> For the record: I strongly think that we don't want to expose any > > > >> render > > > >> functionality on the primary node. > > > >> > > > >> To even go a step further I would say that at least on AMD hardware we > > > >> should completely disable DRI2 for one of the next generations. > > > >> > > > > It's doable and overall pretty neat idea. > > > > > > > > There is a consern that this approach may cause far more regressions > > > > that this series. We are talking about years worth of Mesa drivers (et > > > > al) that depend on render functionality exposed via the primary node. > > > > > > Yeah, that's I'm perfectly aware of. It's the reason why I said we > > > should only do it for new hardware generations. > > > > > > But at some point I think we should do this and get rid of > > > authentication/flink/DRI2/ > > > > > Fwiw I share a similar sentiment. If you've looked through the series, > > this is effectively step 1, towards nuking DRM_AUTH :-) > > > > > > I'm OK with writing the patches, but it'll be up-to the AMDGPU team to > > > > follow-up with any regressions. Are you ok with that? > > > > > > As long as we have a check like adev->family_id > > > > WHATEVER_IS_THE_CURRENT_LATEST_UPSTREAM_HW, I think we are fine with that. > > > > > > If I understood Michel correctly xf86-video-amdgpu should work, but > > > modeset might break and needs a patch. > > > > > Unless I have concrete WHATEVER_IS_THE... I cannot do much here :-( > > > > > > Thanks > > Emil > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] drm: imx: Add NWL MIPI DSI host controller support
Am Mittwoch, den 08.05.2019, 19:18 +0200 schrieb Guido Günther: > Hi, > On Thu, Mar 07, 2019 at 11:30:51AM +0100, Guido Günther wrote: > > This adds initial support for the NWL MIPI DSI Host controller found on > > i.MX8 > > SoCs. > > > > It adds support for the i.MX8MQ but the same IP core can also be found on > > e.g. > > i.MX8QXP. I added the necessary hooks to support other imx8 variants but > > since > > I only have imx8mq boards to test I omitted the platform data for other > > SoCs. > > > > The code is based on NXPs BSP so I added Robert Chiras as Co-authored-by but > > I'm happy to swap Author: and Co-authored-by: if that looks more > > appropriate. > > The most notable changes over the BSP driver are > > - Calculate HS mode timing from phy_configure_opts_mipi_dphy > > - Perform all clock setup via DT > > - Merge nwl-imx and nwl drivers > > - Add B0 silion revision quirk > > > > Posting this is likely a bit premature (hence v0) but I wanted for one show > > how > > this hooks into the mixel dphy posted earlier [1] and avoid duplicating > > work. > > So if there's other code out there doing the same I'm be happy to merge > > efforts. > > Since this is likely not going anywhere until we have a dcss driver aimed > for mainline I'm not going spam the list with further revisions. However > the 5.x version is maintained here: > > > https://source.puri.sm/guido.gunther/linux-imx8/tree/forward-upstream/next-20190506/imx-nwl/v1-wip > > Feedback is still welcome. It'll eventually be forwarded to newer > linux-next versions. > > Changes over the posted version are: > > - Add quirk for IMQ8MQ silicon B0 revision to not mess with the > system reset controller on power down since enable won't work > afterwards otherwise. > - Disable tx esc clock *after* the phy power down to unbreak > disable/enable (unblank/blank) > - Drop devm_free_irq() handled by the device driver core > - Add ports to dt binding docs > - Select GENERIC_PHY_MIPI_DPHY instead of GENERIC_PHY for > phy_mipi_dphy_get_default_config > - Include drm_print.h to fix build on next-20190408 > - Drop some debugging messages > - Newline terminate all DRM_ printouts > > If somebody is working on DCSS support it'd be cool to know since this > driver is currently a component of imx-display-subsystem which will only > work out if dcss is handled like this as well. We have been looking at how to support DCSS in mainline for a while, but most of the actual work got pushed behind in schedule due to other priorities. One thing I can can say for certain is that DCSS should not be integrated into imx-drm. It's a totally different hardware and downstream clearly shows that it's not a good idea to cram it into imx- drm. Also the artificial split between hardware control in drivers/gpu/imx/dcss and the DRM driver is just cargo-cult from the IPU/imx-drm split. For the IPU we did it as the IPU has legs in both DRM for the output parts and V4L2 for the input parts. As the DCSS has no video input capabilities the driver could be simplified a lot by moving it all into a single DRM driver. Regards, Lucas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
Am 27.05.19 um 15:26 schrieb Emil Velikov: > On 2019/05/27, Daniel Vetter wrote: >> On Mon, May 27, 2019 at 10:47:39AM +, Koenig, Christian wrote: >>> Am 27.05.19 um 10:17 schrieb Emil Velikov: From: Emil Velikov Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the render node. A seemingly deliberate design decision. Hence we can drop the DRM_AUTH all together (details in follow-up patch) yet not all userspace checks if it's authenticated, but instead uses uncommon assumptions. After days of digging through git log and testing, only a single (ab)use was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and assuming that failure implies lack of authentication. Affected versions are: - the whole 18.2.x series, which is EOL - the whole 18.3.x series, which is EOL - the 19.0.x series, prior to 19.0.4 >>> Well you could have saved your time, cause this is still a NAK. >>> >>> For the record: I strongly think that we don't want to expose any render >>> functionality on the primary node. >>> >>> To even go a step further I would say that at least on AMD hardware we >>> should completely disable DRI2 for one of the next generations. >>> >>> As a follow up I would then completely disallow the DRM authentication >>> for amdgpu, so that the command submission interface on the primary node >>> can only be used by the display server. >> So amdgpu is running in one direction, while everyone else is running in >> the other direction? Please note that your patch to change i915 landed >> already, so that ship is sailing (but we could ofc revert that back >> again). >> >> Imo really not a great idea if we do a amdgpu vs. everyone else split >> here. If we want to deprecate dri2/flink/rendering on primary nodes across >> the stack, then that should be a stack wide decision, not an amdgpu one. >> > Cannot agree more - I would love to see drivers stay consistent. Yeah, completely agree to that. That's why I think we should not do this at all and just let Intel fix it's userspace bugs :P Anyway my concern is really that we should stop extending functionality on the primary node. E.g. the render node is for use by the clients and the primary node for mode setting and use by the display server only. Regards, Christian. > Fwiw, this series consistently paves the way toward nuking DRM_AUTH ;-) > > Thanks > Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm/vmwgfx: remove custom ioctl io encoding check
On 5/27/19 2:35 PM, Emil Velikov wrote: Hi Thomas, On 2019/05/27, Thomas Hellstrom wrote: I think we might be talking past each other, let's take a step back: - as of previous patch, all of vmwgfx ioctls size is consistently handled by the core I don't think I follow you here, AFAICT patch 3/5 only affects and relaxes the execbuf checking (and in fact a little more than I would like)? Precisely, it makes execbuf ioctl behave like all other ioctls - both vmwgfx and rest of drm. But we're still enforcing a non-relaxed size check for the other vmwgfx private ioctls, right? Which is relaxed, together with the directions, in this commit? (Not that it matters much to the discussion, though). - handling of featue flags, as always, is responsibility of the driver ifself - with this patch, ioctl direction is also handled by core. Here core ensures we only copy in/out as much data as the kernel implementation can handle. Let's consider the following real world example - msm and virtio_gpu. An in field of an _IOW ioctl becomes in/out aka _IORW ioctl. - we add a flag to annotate/request the out, as always invalid flags are return -EINVAL - we change the ioctl encoding As currently handled by core DRM, old kernel/new userspace and vice-versa works just fine. Sadly, vmwgfx will error out, while it could be avoided. IMO basically we have a tradeoff between strict checking in this case, and new user-space vs old kernel "hazzle-free" transition in the relaxed case. Precisely. If I read the code correctly, ATM new userspace will fail against old kernels. Unless userspace writes two versions of the ioctl - with with each encoding. As said above, I'll gladly adjust core and/or others, if this relaxed approach causes an issue somewhere. A specific use-case, real or hypothetical will be appreciated. To me there are two important reasons to keep the strict approach. 1) Avoid user-space mistakes early in the development cycle. We can't distinguish between buggy user-space and "new" user-space. This is important because of [a]) below. Can you provide a concrete example, please? OK, let's say you were developing fence wait functionality. Like vmw_fence_obj_wait ioctl. Then suddenly you started to wonder why the wait never timed out as it should. The reason turn out to be that signals were restarting the waits with the original timeout. So you change the ioctl from W to RW and add a kernel-computed time to the argument. Everything is fine, except that you forget to change this in a user-space application somewhere. So now what happens, is that that user-space bug can live on undetected as in 1), and that means you can never go back and implement a stricter check because that would completely break old user-space. The current code will trap (and has historically trapped) code like this. That's mainly why I'm reluctant to give it up, but I guess it can be conditionally compiled in for debug purposes. 2) Catch a lot of fuzzer combinations and error out early instead of forwarding them to the ioctl function where they may cause harm. Struggling to see why this is a problem? At some point the fuzzer will get past this first line of defence, so we want to make the rest of the ioctl is robust. I think the new user-space vs old kernel can be handled nicely in user- space with feature flags or API versions. That's the way we've handled them up to now? How is a feature flag doing to help if the encoding changes from _IOW to _IORW? Ah, you're referring to old user-space new kernel? Yes, I was probably reading a bit too fast. Sorry about that. So we're basically landing in a tradeoff between trapping problems like above, and hazzle-free ioctl argument definition change. OK, so I'm ok with that as long as there is a way we can compile in strict checking, which will likely has to be as a vmwgfx-specific wrapper. /Thomas Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 5/6] dt-bindings: display: sii902x: Add HDMI audio bindings
The sii902x chip family supports also HDMI audio. Add binding for describing the necessary i2s and mclk wiring for it. Signed-off-by: Jyri Sarha Reviewed-by: Rob Herring --- .../bindings/display/bridge/sii902x.txt | 40 +++ 1 file changed, 40 insertions(+) diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt index c4c1855ca654..2df44b7d3821 100644 --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt @@ -9,6 +9,40 @@ Optional properties: about hotplug events. - reset-gpios: OF device-tree gpio specification for RST_N pin. + HDMI audio properties: + - #sound-dai-cells: <0> or <1>. <0> if only i2s or spdif pin + is wired, <1> if the both are wired. HDMI audio is + configured only if this property is found. + - sil,i2s-data-lanes: Array of up to 4 integers with values of 0-3 + Each integer indicates which i2s pin is connected to which + audio fifo. The first integer selects i2s audio pin for the + first audio fifo#0 (HDMI channels 1&2), second for fifo#1 + (HDMI channels 3&4), and so on. There is 4 fifos and 4 i2s + pins (SD0 - SD3). Any i2s pin can be connected to any fifo, + but there can be no gaps. E.g. an i2s pin must be mapped to + fifo#0 and fifo#1 before mapping a channel to fifo#2. Default + value is <0>, describing SD0 pin beiging routed to hdmi audio + fifo #0. + - clocks: phandle and clock specifier for each clock listed in + the clock-names property + - clock-names: "mclk" + Describes SII902x MCLK input. MCLK is used to produce + HDMI audio CTS values. This property is required if + "#sound-dai-cells"-property is present. This property follows + Documentation/devicetree/bindings/clock/clock-bindings.txt + consumer binding. + + If HDMI audio is configured the sii902x device becomes an I2S + and/or spdif audio codec component (e.g a digital audio sink), + that can be used in configuring a full audio devices with + simple-card or audio-graph-card binding. See their binding + documents on how to describe the way the sii902x device is + connected to the rest of the audio system: + Documentation/devicetree/bindings/sound/simple-card.txt + Documentation/devicetree/bindings/sound/audio-graph-card.txt + Note: In case of the audio-graph-card binding the used port + index should be 3. + Optional subnodes: - video input: this subnode can contain a video input port node to connect the bridge to a display controller output (See this @@ -21,6 +55,12 @@ Example: compatible = "sil,sii9022"; reg = <0x39>; reset-gpios = <&pioA 1 0>; + + #sound-dai-cells = <0>; + sil,i2s-data-lanes = < 0 1 2 >; + clocks = <&mclk>; + clock-names = "mclk"; + ports { #address-cells = <1>; #size-cells = <0>; -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 4/6] dt-bindings: display: sii902x: Remove trailing white space
Remove trailing white space from sii902x display bridge binding. Signed-off-by: Jyri Sarha Reviewed-by: Laurent Pinchart Reviewed-by: Rob Herring --- Documentation/devicetree/bindings/display/bridge/sii902x.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt index 72d2dc6c3e6b..c4c1855ca654 100644 --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt @@ -5,7 +5,7 @@ Required properties: - reg: i2c address of the bridge Optional properties: - - interrupts: describe the interrupt line used to inform the host + - interrupts: describe the interrupt line used to inform the host about hotplug events. - reset-gpios: OF device-tree gpio specification for RST_N pin. -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 0/6] drm/bridge: sii902x: HDMI-audio support and some fixes
I think these should be ready for applying to drm-misc. Changes since v7: - Debased on top of the lasts drm-misc-next and tested - "dt-bindings: display: sii902x: Add HDMI audio bindings" - Dropped off "or higher to avoid conflict with video ports" and added "Reviewed-by: Rob Herring " Ther previous round: https://patchwork.kernel.org/cover/10919173/ Jyri Sarha (5): drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz dt-bindings: display: sii902x: Remove trailing white space dt-bindings: display: sii902x: Add HDMI audio bindings drm/bridge: sii902x: Implement HDMI audio support Tomi Valkeinen (1): drm/bridge: sii902x: add input_bus_flags .../bindings/display/bridge/sii902x.txt | 42 +- drivers/gpu/drm/bridge/sii902x.c | 488 +- 2 files changed, 522 insertions(+), 8 deletions(-) -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 1/6] drm/bridge: sii902x: add input_bus_flags
From: Tomi Valkeinen The driver always sets InputBusFmt:EDGE to 0 (falling edge). Add drm_bridge_timings's input_bus_flags to reflect that the bridge samples on falling edges. Signed-off-by: Tomi Valkeinen Signed-off-by: Jyri Sarha Reviewed-by: Andrzej Hajda Reviewed-by: Laurent Pinchart --- drivers/gpu/drm/bridge/sii902x.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index cdb8dfdb2dff..0d3d730b97ff 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -461,6 +461,12 @@ static int sii902x_i2c_bypass_deselect(struct i2c_mux_core *mux, u32 chan_id) return 0; } +static const struct drm_bridge_timings default_sii902x_timings = { + .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE +| DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE +| DRM_BUS_FLAG_DE_HIGH, +}; + static int sii902x_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -531,6 +537,7 @@ static int sii902x_probe(struct i2c_client *client, sii902x->bridge.funcs = &sii902x_bridge_funcs; sii902x->bridge.of_node = dev->of_node; + sii902x->bridge.timings = &default_sii902x_timings; drm_bridge_add(&sii902x->bridge); i2c_set_clientdata(client, sii902x); -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 2/6] drm/bridge: sii902x: Set output mode to HDMI or DVI according to EDID
Set output mode to HDMI or DVI according to EDID HDMI signature. Signed-off-by: Jyri Sarha Reviewed-by: Andrzej Hajda Reviewed-by: Laurent Pinchart --- drivers/gpu/drm/bridge/sii902x.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 0d3d730b97ff..128d8fdb4ba6 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -181,12 +181,16 @@ static int sii902x_get_modes(struct drm_connector *connector) { struct sii902x *sii902x = connector_to_sii902x(connector); u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; + u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI; struct edid *edid; int num = 0, ret; edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]); drm_connector_update_edid_property(connector, edid); if (edid) { + if (drm_detect_hdmi_monitor(edid)) + output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI; + num = drm_add_edid_modes(connector, edid); kfree(edid); } @@ -196,6 +200,11 @@ static int sii902x_get_modes(struct drm_connector *connector) if (ret) return ret; + ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA, +SII902X_SYS_CTRL_OUTPUT_MODE, output_mode); + if (ret) + return ret; + return num; } -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 3/6] drm/bridge: sii902x: pixel clock unit is 10kHz instead of 1kHz
The pixel clock unit in the first two registers (0x00 and 0x01) of sii9022 is 10kHz, not 1kHz as in struct drm_display_mode. Division by 10 fixes the issue. Signed-off-by: Jyri Sarha Reviewed-by: Andrzej Hajda Reviewed-by: Laurent Pinchart --- drivers/gpu/drm/bridge/sii902x.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 128d8fdb4ba6..19f982a00dba 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -249,10 +249,11 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge, struct regmap *regmap = sii902x->regmap; u8 buf[HDMI_INFOFRAME_SIZE(AVI)]; struct hdmi_avi_infoframe frame; + u16 pixel_clock_10kHz = adj->clock / 10; int ret; - buf[0] = adj->clock; - buf[1] = adj->clock >> 8; + buf[0] = pixel_clock_10kHz & 0xff; + buf[1] = pixel_clock_10kHz >> 8; buf[2] = adj->vrefresh; buf[3] = 0x00; buf[4] = adj->hdisplay; -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 6/6] drm/bridge: sii902x: Implement HDMI audio support
Implement HDMI audio support by using ASoC HDMI codec. The commit implements the necessary callbacks and configuration for the HDMI codec and registers a virtual platform device for the codec to attach. Signed-off-by: Jyri Sarha Reviewed-by: Andrzej Hajda --- drivers/gpu/drm/bridge/sii902x.c | 469 ++- 1 file changed, 463 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 19f982a00dba..bc3325c5e5c3 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -34,6 +35,8 @@ #include #include +#include + #define SII902X_TPI_VIDEO_DATA 0x0 #define SII902X_TPI_PIXEL_REPETITION 0x8 @@ -75,6 +78,77 @@ #define SII902X_AVI_POWER_STATE_MSKGENMASK(1, 0) #define SII902X_AVI_POWER_STATE_D(l) ((l) & SII902X_AVI_POWER_STATE_MSK) +/* Audio */ +#define SII902X_TPI_I2S_ENABLE_MAPPING_REG 0x1f +#define SII902X_TPI_I2S_CONFIG_FIFO0 (0 << 0) +#define SII902X_TPI_I2S_CONFIG_FIFO1 (1 << 0) +#define SII902X_TPI_I2S_CONFIG_FIFO2 (2 << 0) +#define SII902X_TPI_I2S_CONFIG_FIFO3 (3 << 0) +#define SII902X_TPI_I2S_LEFT_RIGHT_SWAP(1 << 2) +#define SII902X_TPI_I2S_AUTO_DOWNSAMPLE(1 << 3) +#define SII902X_TPI_I2S_SELECT_SD0 (0 << 4) +#define SII902X_TPI_I2S_SELECT_SD1 (1 << 4) +#define SII902X_TPI_I2S_SELECT_SD2 (2 << 4) +#define SII902X_TPI_I2S_SELECT_SD3 (3 << 4) +#define SII902X_TPI_I2S_FIFO_ENABLE(1 << 7) + +#define SII902X_TPI_I2S_INPUT_CONFIG_REG 0x20 +#define SII902X_TPI_I2S_FIRST_BIT_SHIFT_YES(0 << 0) +#define SII902X_TPI_I2S_FIRST_BIT_SHIFT_NO (1 << 0) +#define SII902X_TPI_I2S_SD_DIRECTION_MSB_FIRST (0 << 1) +#define SII902X_TPI_I2S_SD_DIRECTION_LSB_FIRST (1 << 1) +#define SII902X_TPI_I2S_SD_JUSTIFY_LEFT(0 << 2) +#define SII902X_TPI_I2S_SD_JUSTIFY_RIGHT (1 << 2) +#define SII902X_TPI_I2S_WS_POLARITY_LOW(0 << 3) +#define SII902X_TPI_I2S_WS_POLARITY_HIGH (1 << 3) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_128(0 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_256(1 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_384(2 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_512(3 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_768(4 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_1024 (5 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_1152 (6 << 4) +#define SII902X_TPI_I2S_MCLK_MULTIPLIER_192(7 << 4) +#define SII902X_TPI_I2S_SCK_EDGE_FALLING (0 << 7) +#define SII902X_TPI_I2S_SCK_EDGE_RISING(1 << 7) + +#define SII902X_TPI_I2S_STRM_HDR_BASE 0x21 +#define SII902X_TPI_I2S_STRM_HDR_SIZE 5 + +#define SII902X_TPI_AUDIO_CONFIG_BYTE2_REG 0x26 +#define SII902X_TPI_AUDIO_CODING_STREAM_HEADER (0 << 0) +#define SII902X_TPI_AUDIO_CODING_PCM (1 << 0) +#define SII902X_TPI_AUDIO_CODING_AC3 (2 << 0) +#define SII902X_TPI_AUDIO_CODING_MPEG1 (3 << 0) +#define SII902X_TPI_AUDIO_CODING_MP3 (4 << 0) +#define SII902X_TPI_AUDIO_CODING_MPEG2 (5 << 0) +#define SII902X_TPI_AUDIO_CODING_AAC (6 << 0) +#define SII902X_TPI_AUDIO_CODING_DTS (7 << 0) +#define SII902X_TPI_AUDIO_CODING_ATRAC (8 << 0) +#define SII902X_TPI_AUDIO_MUTE_DISABLE (0 << 4) +#define SII902X_TPI_AUDIO_MUTE_ENABLE (1 << 4) +#define SII902X_TPI_AUDIO_LAYOUT_2_CHANNELS(0 << 5) +#define SII902X_TPI_AUDIO_LAYOUT_8_CHANNELS(1 << 5) +#define SII902X_TPI_AUDIO_INTERFACE_DISABLE(0 << 6) +#define SII902X_TPI_AUDIO_INTERFACE_SPDIF (1 << 6) +#define SII902X_TPI_AUDIO_INTERFACE_I2S(2 << 6) + +#define SII902X_TPI_AUDIO_CONFIG_BYTE3_REG 0x27 +#define SII902X_TPI_AUDIO_FREQ_STREAM (0 << 3) +#define SII902X_TPI_AUDIO_FREQ_32KHZ (1 << 3) +#define SII902X_TPI_AUDIO_FREQ_44KHZ (2 << 3) +#define SII902X_TPI_AUDIO_FREQ_48KHZ (3 << 3) +#define SII902X_TPI_AUDIO_FREQ_88KHZ (4 << 3) +#define SII902X_TPI_AUDIO_FREQ_96KHZ (5 << 3) +#define SII902X_TPI_AUDIO_FREQ_176KHZ (6 << 3) +#define SII902X_TPI_AUDIO_FREQ_192KHZ (7 << 3) +#define SII902X_TPI_AUDIO_SAMPLE_SIZE_STREAM (0 << 6) +#define SII902X_TPI_AUDIO_SAMPLE_SIZE_16 (1 << 6) +#define SII902X_TPI_AUDIO_SAMPLE_SIZE_20
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/05/27, Daniel Vetter wrote: > On Mon, May 27, 2019 at 09:17:29AM +0100, Emil Velikov wrote: > > From: Emil Velikov > > > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > > render node. A seemingly deliberate design decision. > > > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > > yet not all userspace checks if it's authenticated, but instead uses > > uncommon assumptions. > > > > After days of digging through git log and testing, only a single (ab)use > > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > > assuming that failure implies lack of authentication. > > Maybe correction here: Id does not care about authentication at all. It > wants to figure out whether it has access to modeset resources, which is > something entirely different, but happened to match in amdgpu's case. > It feels like we have conflated the two (perhaps due to historical reasons) and I'm not 100% sure which one the AMDGPU code inspects. How about: Hence we can drop the DRM_AUTH all together (details in follow-up patch) yet that cause a regression in some userspace, when it conflates the authentication status with access to modeset resources. After days of digging through git log and testing, only a single (ab)use was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl. > > Affected versions are: > > - the whole 18.2.x series, which is EOL > > - the whole 18.3.x series, which is EOL > > - the 19.0.x series, prior to 19.0.4 > > Hm I think I'm blind, I'm not seeing the fix merged to mesa master. Still > there on gitlab: > > https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/amd/vulkan/radv_device.c#L291 > > What am I missing? > Opted for a simple, more generic solution see commit c962a78f18284e2971301bf68c9c60500ca398e4 This way, the same check is: - enforced where it's used - present for all Mesa Vulkan drivers Will include the sha + commit title for v2. > > Add a special quirk for that case, thus we can drop DRM_AUTH bits as > > mentioned earlier. > > > > Since all the affected userspace is EOL, we also add a kconfig option > > to disable this quirk. > > > > The whole approach is inspired by DRIVER_KMS_LEGACY_CONTEXT > > > > Cc: Alex Deucher > > Cc: Christian König > > Cc: amd-...@lists.freedesktop.org > > Cc: David Airlie > > Cc: Daniel Vetter > > Signed-off-by: Emil Velikov > > Aside from the nits I think reasonable approach. Great, glad to hear. Now all we need is to bribe^Wconvince Christian. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel