Re: [PATCH 04/33] vt: More locking checks

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Pintu Agarwal
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!

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Greg Kroah-Hartman
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

2019-05-27 Thread Greg KH
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

2019-05-27 Thread Greg KH
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

2019-05-27 Thread 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.

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

2019-05-27 Thread Emil Velikov
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

2019-05-27 Thread 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: 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

2019-05-27 Thread 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: 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

2019-05-27 Thread 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.

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

2019-05-27 Thread 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.

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

2019-05-27 Thread 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: 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

2019-05-27 Thread 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.

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

2019-05-27 Thread 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.

[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

2019-05-27 Thread 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.

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

2019-05-27 Thread 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

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

2019-05-27 Thread 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: 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

2019-05-27 Thread 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: 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

2019-05-27 Thread Jani Nikula
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

2019-05-27 Thread Tomi Valkeinen

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

2019-05-27 Thread Emil Velikov
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

2019-05-27 Thread Linus Walleij
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

2019-05-27 Thread Inki Dae
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

2019-05-27 Thread Pintu Agarwal
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

2019-05-27 Thread Yannick Fertré
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

2019-05-27 Thread Yannick Fertré
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

2019-05-27 Thread Yannick Fertré
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

2019-05-27 Thread Yannick Fertré
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

2019-05-27 Thread Vivek Gautam
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

2019-05-27 Thread Koenig, Christian
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

2019-05-27 Thread Tomi Valkeinen

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

2019-05-27 Thread Koenig, Christian
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

2019-05-27 Thread Sam Ravnborg
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

2019-05-27 Thread Christian König
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"

2019-05-27 Thread Bartlomiej Zolnierkiewicz

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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread bugzilla-daemon
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]

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Gerd Hoffmann
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

2019-05-27 Thread Nasser
 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

2019-05-27 Thread Tony Lindgren
* 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

2019-05-27 Thread Thomas Hellstrom
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

2019-05-27 Thread Guido Günther
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.

2019-05-27 Thread bugzilla-daemon
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.

2019-05-27 Thread bugzilla-daemon
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!

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Christian König

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

2019-05-27 Thread Emil Velikov
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

2019-05-27 Thread Hillf Danton

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'

2019-05-27 Thread YueHaibing
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)

2019-05-27 Thread Hariprasad Kelam
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]

2019-05-27 Thread Thomas Meyer
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

2019-05-27 Thread Tyler Slabinski
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

2019-05-27 Thread Hillf Danton

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

2019-05-27 Thread Ilpo Järvinen
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

2019-05-27 Thread Hariprasad Kelam
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

2019-05-27 Thread YueHaibing
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}

2019-05-27 Thread Benjamin Gaignard
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

2019-05-27 Thread Torsten Duwe
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

2019-05-27 Thread Hillf Danton

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

2019-05-27 Thread Hariprasad Kelam
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

2019-05-27 Thread Clément Péron
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

2019-05-27 Thread Vikas Patil
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

2019-05-27 Thread Stephen Boyd
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

2019-05-27 Thread Hariprasad Kelam
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

2019-05-27 Thread 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 :-)

> 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

2019-05-27 Thread Maxime Ripard
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

2019-05-27 Thread 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 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

2019-05-27 Thread Koenig, Christian
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

2019-05-27 Thread Koenig, Christian
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}

2019-05-27 Thread Philippe CORNU
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

2019-05-27 Thread Philippe CORNU
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

2019-05-27 Thread Emil Velikov
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

2019-05-27 Thread Thomas Hellstrom

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

2019-05-27 Thread Philippe CORNU
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

2019-05-27 Thread Emil Velikov
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

2019-05-27 Thread Philippe CORNU
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

2019-05-27 Thread Emil Velikov
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

2019-05-27 Thread bugzilla-daemon
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Ville Syrjälä
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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread 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.

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

2019-05-27 Thread Daniel Vetter
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

2019-05-27 Thread Lucas Stach
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

2019-05-27 Thread Koenig, Christian
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

2019-05-27 Thread Thomas Hellstrom

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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Jyri Sarha
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

2019-05-27 Thread Emil Velikov
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

  1   2   >