[PATCH] drm/amdgpu: fix compile warning in amdgpu_kms.c

2017-01-02 Thread Rex Zhu
warning:
comparison of distinct pointer types lacks a cast
 min((size_t)size, bios_size - bios_offset))

Change-Id: Ib77eac5b15d09d169d2e95cff7608b395fb0f64c
Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 3273d8c..47bc8e1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -561,7 +561,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
 
bios = adev->bios + bios_offset;
return copy_to_user(out, bios,
-   min((size_t)size, bios_size - bios_offset))
+   min(size, bios_size - bios_offset))
? -EFAULT : 0;
}
default:
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD handles (v3)

2017-01-02 Thread Christian König

Am 15.12.2016 um 23:43 schrieb Grazvydas Ignotas:

On Thu, Dec 15, 2016 at 4:12 PM, Christian König
 wrote:

Regarding which error code to return I think that Emil has the right idea
here.

Returning -EINVAL usually means that userspace provided an invalid value,
but in this case it doesn't matter which value the UMD provide all of them
would be invalid because starting with Polaris the hardware/firmware simply
doesn't work this way any more.

So using -ENODEV or maybe -ENODATA indeed sound like the right think to do
here.

What about ERANGE then, "Math result not representable" aka infinity?
To me ENODEV is more like "the GPU you are asking about is not there"
and ENODATA "information you ask for is not known", although the later
still somewhat makes sense.


Sorry for the delayed reply.

I think EDATA would still make more sense than ERANGE, because the 
number of open sessions is simply not known any more.


That the maximum number of sessions is now unlimited is just a side 
effect of the new handling if you ask me.


Regards,
Christian.



Gražvydas



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdgpu: fix compile warning in amdgpu_kms.c

2017-01-02 Thread Christian König

Am 02.01.2017 um 10:23 schrieb Rex Zhu:

warning:
comparison of distinct pointer types lacks a cast
  min((size_t)size, bios_size - bios_offset))

Change-Id: Ib77eac5b15d09d169d2e95cff7608b395fb0f64c
Signed-off-by: Rex Zhu 


Reviewed-by: Christian König .


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 3273d8c..47bc8e1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -561,7 +561,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
  
  			bios = adev->bios + bios_offset;

return copy_to_user(out, bios,
-   min((size_t)size, bios_size - bios_offset))
+   min(size, bios_size - bios_offset))
? -EFAULT : 0;
}
default:



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amd/amdgpu: fix spelling mistake: "comleted" -> "completed"

2017-01-02 Thread Edward O'Callaghan
Reviewed-by: Edward O'Callaghan 

On 12/30/2016 12:39 PM, Zhou, David(ChunMing) wrote:
> +amd-gfx, patch is Reviewed-by: Chunming Zhou 
> 
> -Original Message-
> From: Colin King [mailto:colin.k...@canonical.com] 
> Sent: Thursday, December 29, 2016 11:47 PM
> To: Deucher, Alexander ; Koenig, Christian 
> ; David Airlie ; Zhou, 
> David(ChunMing) ; StDenis, Tom ; 
> Liu, Monk ; dri-de...@lists.freedesktop.org
> Cc: linux-ker...@vger.kernel.org
> Subject: [PATCH] drm/amd/amdgpu: fix spelling mistake: "comleted" -> 
> "completed"
> 
> From: Colin Ian King 
> 
> trivial fix to spelling mistake in WARN message
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 60bd4af..9ca3167 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2335,7 +2335,7 @@ int amdgpu_gpu_reset(struct amdgpu_device *adev)
>   if (fence) {
>   r = dma_fence_wait(fence, false);
>   if (r) {
> - WARN(r, "recovery from shadow 
> isn't comleted\n");
> + WARN(r, "recovery from shadow 
> isn't completed\n");
>   break;
>   }
>   }
> @@ -2347,7 +2347,7 @@ int amdgpu_gpu_reset(struct amdgpu_device *adev)
>   if (fence) {
>   r = dma_fence_wait(fence, false);
>   if (r)
> - WARN(r, "recovery from shadow isn't 
> comleted\n");
> + WARN(r, "recovery from shadow isn't 
> completed\n");
>   }
>   dma_fence_put(fence);
>   }
> 



signature.asc
Description: OpenPGP digital signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [RFC] Mechanism for high priority scheduling in amdgpu

2017-01-02 Thread Christian König
I hate to keep bringing up display topics in an unrelated 
conversation, but I'm not sure where you got "Application -> X server 
-> compositor -> X server" from.
Sorry for that. It just sounded like you was assuming something about 
the current stack which looked like it won't work currently.


But when you are willing to implement that stuff then I'm not really 
concerned any more. Especially when Dave is already involved.


Regards,
Christian.

Am 23.12.2016 um 19:18 schrieb Pierre-Loup A. Griffais:
I hate to keep bringing up display topics in an unrelated 
conversation, but I'm not sure where you got "Application -> X server 
-> compositor -> X server" from. As I was saying before, we need to be 
presenting directly to the HMD display as no display server can be in 
the way, both for latency but also quality of service reasons (a buggy 
application cannot be allowed to accidentally display undistorted 
rendering into the HMD); we intend to do the necessary work for this, 
and the extent of X's (or a Wayland implementation, or any other 
display server) involvment will be to participate enough to know that 
the HMD display is off-limits. If you have more questions on the 
display aspect, or VR rendering in general, I'm happy to try to 
address them out-of-band from this conversation.


On 12/23/2016 02:54 AM, Christian König wrote:

But yes, in general you don't want another compositor in the way, so
we'll be acquiring the HMD display directly, separate from any desktop
or display server.

Assuming that the the HMD is attached to the rendering device in some
way you have the X server and the Compositor which both try to be DRM
master at the same time.

Please correct me if that was fixed in the meantime, but that sounds
like it will simply not work. Or is this what Andres mention below Dave
is working on ?.

Additional to that a compositor in combination with X is a bit counter
productive when you want to keep the latency low.

E.g. the "normal" flow of a GL or Vulkan surface filled with rendered
data to be displayed is from the Application -> X server -> compositor
-> X server.

The extra step between X server and compositor just means extra latency
and for this use case you probably don't want that.

Targeting something like Wayland and when you need X compatibility
XWayland sounds like the much better idea.

Regards,
Christian.

Am 22.12.2016 um 20:54 schrieb Pierre-Loup A. Griffais:

Display concerns are a separate issue, and as Andres said we have
other plans to address. But yes, in general you don't want another
compositor in the way, so we'll be acquiring the HMD display directly,
separate from any desktop or display server. Same with security, we
can have a separate conversation about that when the time comes.

On 12/22/2016 08:41 AM, Serguei Sagalovitch wrote:

Andres,

Did you measure  latency, etc. impact of __any__ compositor?

My understanding is that VR has pretty strict requirements related to
QoS.

Sincerely yours,
Serguei Sagalovitch


On 2016-12-22 11:35 AM, Andres Rodriguez wrote:

Hey Christian,

We are currently interested in X, but with some distros switching to
other compositors by default, we also need to consider those.

We agree, running the full vrcompositor in root isn't something that
we want to do. Too many security concerns. Having a small root helper
that does the privilege escalation for us is the initial idea.

For a long term approach, Pierre-Loup and Dave are working on dealing
with the "two compositors" scenario a little better in DRM+X.
Fullscreen isn't really a sufficient approach, since we don't want 
the
HMD to be used as part of the Desktop environment when a VR app is 
not

in use (this is extremely annoying).

When the above is settled, we should have an auth mechanism besides
DRM_MASTER or DRM_AUTH that allows the vrcompositor to take over the
HMD permanently away from X. Re-using that auth method to gate this
IOCTL is probably going to be the final solution.

I propose to start with ROOT_ONLY since it should allow us to respect
kernel IOCTL compatibility guidelines with the most flexibility. 
Going

from a restrictive to a more flexible permission model would be
inclusive, but going from a general to a restrictive model may 
exclude

some apps that used to work.

Regards,
Andres

On 12/22/2016 6:42 AM, Christian König wrote:

Hi Andres,

well using root might cause stability and security problems as well.
We worked quite hard to avoid exactly this for X.

We could make this feature depend on the compositor being DRM 
master,

but for example with X the X server is master (and e.g. can change
resolutions etc..) and not the compositor.

So another question is also what windowing system (if any) are you
planning to use? X, Wayland, Flinger or something completely
different ?

Regards,
Christian.

Am 20.12.2016 um 16:51 schrieb Andres Rodriguez:

Hi Christian,

That is definitely a concern. What we are currently thinking is to
make the high priority queues acces