Re: [PATCH 3/3] drm/amdgpu/radeon: Use radeon by default for CIK GPUs

2017-05-29 Thread Michel Dänzer 
On 29/05/17 06:20 PM, Michel Dänzer wrote:
> From: Michel Dänzer 
> 
> Even if CONFIG_DRM_AMDGPU_CIK is enabled.
> 
> There is no feature parity yet for CIK, in particular amdgpu doesn't
> support HDMI/DisplayPort without DC.
> 
> Signed-off-by: Michel Dänzer 

[...]

> diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
> b/drivers/gpu/drm/amd/amdgpu/Kconfig
> index 8d36087fc186..e0121f8b436e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
> +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
> @@ -17,11 +17,11 @@ config DRM_AMDGPU_CIK
>   help
> Choose this option if you want to enable support for CIK asics.
>  
> -   CIK is already supported in radeon. If you enable this option,
> -   support for CIK will be provided by amdgpu and disabled in
> -   radeon by default. Use module options to override this:
> +   CIK is already supported in radeon. Support for SI in amdgpu

Consider this "SI" typo fixed in v2.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/3] drm/amdgpu: Really leave SI support disabled by default

2017-05-29 Thread Michel Dänzer 
On 30/05/17 02:18 AM, Christian König wrote:
> Am 29.05.2017 um 11:20 schrieb Michel Dänzer:
>> From: Michel Dänzer 
>>
>> The default option value didn't match the help text and intention.
>>
>> Signed-off-by: Michel Dänzer 
> 
> I'm still unsure about the last one. The feature parity is a good
> argument but on the other hand we want people to use amdgpu for CIK
> these days, don't we?

We want to make it easy for people to test amdgpu on CIK, which is what
the options added by Felix are for. IMO we should not flip the default
(upstream) before there is feature parity.


> Anyway Reviewed-by: Christian König .

Thanks, I assume that applies to the whole series?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/3] drm/amdgpu/radeon: Use radeon by default for CIK GPUs

2017-05-29 Thread Michel Dänzer 
On 29/05/17 11:24 PM, Kai Wasserbäch wrote:
> Hey Michel,
> Michel Dänzer wrote on 29.05.2017 11:20:
>> From: Michel Dänzer 
>>
>> Even if CONFIG_DRM_AMDGPU_CIK is enabled.
>>
>> There is no feature parity yet for CIK, in particular amdgpu doesn't
>> support HDMI/DisplayPort without DC.
> 
> that can't be correct.

Indeed, I meant "HDMI/DisplayPort audio", sorry for the confusion. Will
be fixed in v2.


>> diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
>> b/drivers/gpu/drm/amd/amdgpu/Kconfig
>> index 8d36087fc186..e0121f8b436e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
>> +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
>> @@ -17,11 +17,11 @@ config DRM_AMDGPU_CIK
>>  help
>>Choose this option if you want to enable support for CIK asics.
>>  
>> -  CIK is already supported in radeon. If you enable this option,
>> -  support for CIK will be provided by amdgpu and disabled in
>> -  radeon by default. Use module options to override this:
>> +  CIK is already supported in radeon. Support for SI in amdgpu
>> +  will be disabled by default and is still provided by radeon.
>> +  Use module options to override this:
>>  
>> -  radeon.cik_support=1 amdgpu.cik_support=0
>> +  radeon.cik_support=0 amdgpu.cik_support=1
> 
> What happens if I have radeon blacklisted? Do I still need to set this
> additional flag?

Yes, you still need to set amdgpu.cik_support=1.

> If so I'd kindly request to at least fall back to amdgpu if somebody
> blacklisted radeon (or didn't compile it).

I'm afraid that's not possible, at least I don't know how it could be done.

The intention is that these options provide a cleaner way than
blacklisting for choosing between the drivers, so the options should be
used instead of blacklisting.


> Maybe issue a warning that not all features are supported yet.

The drivers print a message when they're ignoring a GPU due to these
options.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm v4 1/1] amdgpu: move asic id table to a separate file

2017-05-29 Thread Michel Dänzer 
On 30/05/17 06:16 AM, Samuel Li wrote:
> From: Xiaojie Yuan 

I took a closer look and noticed some details (and some non-details
about the amdgpu.ids file at the end).


> diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c
> new file mode 100644
> index 000..a43ca33
> --- /dev/null
> +++ b/amdgpu/amdgpu_asic_id.c

[...]

> +#include "xf86drm.h"
> +#include "amdgpu_drm.h"

Should be

#include 
#include 

since these header files are not located in the same directory as
amdgpu_asic_id.c.


> +static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
> +{
> + char *buf, *saveptr;
> + char *s_did;
> + char *s_rid;
> + char *s_name;
> + char *endptr;
> + int r = 0;

This function could be simplified slightly by initializing r = -EINVAL
here and only setting it to 0 just before the out label.


> +int amdgpu_parse_asic_ids(struct amdgpu_asic_id **p_asic_id_table)
> +{

[...]

> + /* 1st valid line is file version */
> + while ((n = getline(, , fp)) != -1) {
> + /* trim trailing newline */
> + if (line[n - 1] == '\n')
> + line[n - 1] = '\0';

Should probably increment line_num here, otherwise the line number in
the error message below might be confusing.


> + fprintf(stderr, "Invalid format: %s: line %d: %s\n",
> + AMDGPU_ASIC_ID_TABLE, line_num, line);

The second line should be indented to align with the opening parenthesis.


> + if (table_size >= table_max_size) {
> + /* double table size */
> + table_max_size *= 2;
> + asic_id_table = realloc(asic_id_table, table_max_size *
> + sizeof(struct amdgpu_asic_id));

Ditto.


> + /* end of table */
> + id = asic_id_table + table_size;
> + memset(id, 0, sizeof(struct amdgpu_asic_id));

Might also want to realloc asic_id_table according to the final table
size, to avoid wasting memory.


> + if (r && asic_id_table) {
> + while (table_size--) {
> + id = asic_id_table + table_size;
> + if (id->marketing_name !=  NULL)
> + free(id->marketing_name);

free(NULL) works fine (and parse_one_line returns an error for
id->marketing_name == NULL anyway), so this can be simplified to

free(id->marketing_name);


> @@ -140,6 +140,13 @@ static void 
> amdgpu_device_free_internal(amdgpu_device_handle dev)
>   close(dev->fd);
>   if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
>   close(dev->flink_fd);
> + if (dev->asic_ids) {
> + for (id = dev->asic_ids; id->did; id++) {
> + if (id->marketing_name !=  NULL)
> + free(id->marketing_name);
> + }

Ditto, this can be simplified to

for (id = dev->asic_ids; id->did; id++)
free(id->marketing_name);


> @@ -267,6 +274,11 @@ int amdgpu_device_initialize(int fd,
>   amdgpu_vamgr_init(>vamgr_32, start, max,
> dev->dev_info.virtual_address_alignment);
>  
> + r = amdgpu_parse_asic_ids(>asic_ids);
> + if (r)
> + fprintf(stderr, "%s: Can not parse asic ids, 0x%x.",
> + __func__, r);

"Cannot parse ASIC IDs"

Also, there should be curly braces around a multi-line statement.


> @@ -297,13 +309,15 @@ int amdgpu_device_deinitialize(amdgpu_device_handle dev)
>  
>  const char *amdgpu_get_marketing_name(amdgpu_device_handle dev)
>  {
> - const struct amdgpu_asic_id_table_t *t = amdgpu_asic_id_table;
> + const struct amdgpu_asic_id *id;
> +
> + if (!dev->asic_ids)
> + return NULL;
>  
> - while (t->did) {
> - if ((t->did == dev->info.asic_id) &&
> - (t->rid == dev->info.pci_rev_id))
> - return t->marketing_name;
> - t++;
> + for (id = dev->asic_ids; id->did; id++) {
> + if ((id->did == dev->info.asic_id) &&
> + (id->rid == dev->info.pci_rev_id))

The last line is indented incorrectly, should be 2 tabs and 4 spaces.


> diff --git a/include/drm/amdgpu.ids b/include/drm/amdgpu.ids
> new file mode 100644
> index 000..6d6b944
> --- /dev/null
> +++ b/include/drm/amdgpu.ids

I think the path of this file in the repository should be
amdgpu/amdgpu.ids rather than include/drm/amdgpu.ids.


> @@ -0,0 +1,170 @@
> +# List of AMDGPU ID's

This should say "IDs" instead of "ID's".


> +67FF,CF, 67FF:CF
> +67FF,EF, 67FF:EF

There should be no such dummy entries in the file. If it's useful,
amdgpu_get_marketing_name can return a dummy string based on the PCI ID
and revision when there's no matching entry in the file.


-- 
Earthling Michel Dänzer   |   http://www.amd.com

[PATCH] drm/amdgpu: Program ring for vce instance 1 at its own register space

2017-05-29 Thread Leo Liu 
when harvest part has only instance 1 available

Signed-off-by: Leo Liu 
---
 drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 61 +++
 1 file changed, 55 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c 
b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
index fb08193..77af395 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
@@ -77,13 +77,26 @@ static int vce_v3_0_set_clockgating_state(void *handle,
 static uint64_t vce_v3_0_ring_get_rptr(struct amdgpu_ring *ring)
 {
struct amdgpu_device *adev = ring->adev;
+   u32 v;
+
+   if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) {
+   mutex_lock(>grbm_idx_mutex);
+   WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(1));
+   }
 
if (ring == >vce.ring[0])
-   return RREG32(mmVCE_RB_RPTR);
+   v = RREG32(mmVCE_RB_RPTR);
else if (ring == >vce.ring[1])
-   return RREG32(mmVCE_RB_RPTR2);
+   v = RREG32(mmVCE_RB_RPTR2);
else
-   return RREG32(mmVCE_RB_RPTR3);
+   v = RREG32(mmVCE_RB_RPTR3);
+
+   if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) {
+   WREG32(mmGRBM_GFX_INDEX, mmGRBM_GFX_INDEX_DEFAULT);
+   mutex_unlock(>grbm_idx_mutex);
+   }
+
+   return v;
 }
 
 /**
@@ -96,13 +109,26 @@ static uint64_t vce_v3_0_ring_get_rptr(struct amdgpu_ring 
*ring)
 static uint64_t vce_v3_0_ring_get_wptr(struct amdgpu_ring *ring)
 {
struct amdgpu_device *adev = ring->adev;
+   u32 v;
+
+   if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) {
+   mutex_lock(>grbm_idx_mutex);
+   WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(1));
+   }
 
if (ring == >vce.ring[0])
-   return RREG32(mmVCE_RB_WPTR);
+   v = RREG32(mmVCE_RB_WPTR);
else if (ring == >vce.ring[1])
-   return RREG32(mmVCE_RB_WPTR2);
+   v = RREG32(mmVCE_RB_WPTR2);
else
-   return RREG32(mmVCE_RB_WPTR3);
+   v = RREG32(mmVCE_RB_WPTR3);
+
+   if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) {
+   WREG32(mmGRBM_GFX_INDEX, mmGRBM_GFX_INDEX_DEFAULT);
+   mutex_unlock(>grbm_idx_mutex);
+   }
+
+   return v;
 }
 
 /**
@@ -116,12 +142,22 @@ static void vce_v3_0_ring_set_wptr(struct amdgpu_ring 
*ring)
 {
struct amdgpu_device *adev = ring->adev;
 
+   if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) {
+   mutex_lock(>grbm_idx_mutex);
+   WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(1));
+   }
+
if (ring == >vce.ring[0])
WREG32(mmVCE_RB_WPTR, lower_32_bits(ring->wptr));
else if (ring == >vce.ring[1])
WREG32(mmVCE_RB_WPTR2, lower_32_bits(ring->wptr));
else
WREG32(mmVCE_RB_WPTR3, lower_32_bits(ring->wptr));
+
+   if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) {
+   WREG32(mmGRBM_GFX_INDEX, mmGRBM_GFX_INDEX_DEFAULT);
+   mutex_unlock(>grbm_idx_mutex);
+   }
 }
 
 static void vce_v3_0_override_vce_clock_gating(struct amdgpu_device *adev, 
bool override)
@@ -231,6 +267,14 @@ static int vce_v3_0_start(struct amdgpu_device *adev)
struct amdgpu_ring *ring;
int idx, r;
 
+   /* we need program ring buffer on instance 1 register space domain
+   when only if instance 1 available, with two instances or instance 0
+   we need only program instance 0 regsiter space domain for ring */
+   if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) {
+   mutex_lock(>grbm_idx_mutex);
+   WREG32(mmGRBM_GFX_INDEX, GET_VCE_INSTANCE(1));
+   }
+
ring = >vce.ring[0];
WREG32(mmVCE_RB_RPTR, lower_32_bits(ring->wptr));
WREG32(mmVCE_RB_WPTR, lower_32_bits(ring->wptr));
@@ -252,6 +296,11 @@ static int vce_v3_0_start(struct amdgpu_device *adev)
WREG32(mmVCE_RB_BASE_HI3, upper_32_bits(ring->gpu_addr));
WREG32(mmVCE_RB_SIZE3, ring->ring_size / 4);
 
+   if (adev->vce.harvest_config == AMDGPU_VCE_HARVEST_VCE0) {
+   WREG32(mmGRBM_GFX_INDEX, mmGRBM_GFX_INDEX_DEFAULT);
+   mutex_unlock(>grbm_idx_mutex);
+   }
+
mutex_lock(>grbm_idx_mutex);
for (idx = 0; idx < 2; ++idx) {
if (adev->vce.harvest_config & (1 << idx))
-- 
2.9.3

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

RE: [PATCH libdrm v3 1/1] amdgpu: move asic id table to a separate file

2017-05-29 Thread Li, Samuel 
Recently there are some changes internally, I can integrate them into this one.
There are still discussions on going regarding the format.

Sam

-Original Message-
From: Michel Dänzer [mailto:mic...@daenzer.net] 
Sent: Sunday, May 28, 2017 10:11 PM
To: Li, Samuel 
Cc: amd-gfx@lists.freedesktop.org; Yuan, Xiaojie 
Subject: Re: [PATCH libdrm v3 1/1] amdgpu: move asic id table to a separate file

On 27/05/17 04:23 AM, Samuel Li wrote:
> 
> diff --git a/include/drm/amdgpu.ids b/include/drm/amdgpu.ids new file 
> mode 100644 index 000..1b00b60
> --- /dev/null
> +++ b/include/drm/amdgpu.ids
> @@ -0,0 +1,154 @@
> +1.0.0
> +6600,0,AMD Radeon HD 8600/8700M

This doesn't look like the current format we've settled on internally.
There are supposed to be tabs after the commas to align the columns.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH libdrm v4 1/1] amdgpu: move asic id table to a separate file

2017-05-29 Thread Samuel Li 
From: Xiaojie Yuan 

v2: fix an off by one error and leading white spaces
v3: use thread safe strtok_r(); initialize len before calling getline();
change printf() to drmMsg(); add initial amdgpu.ids
v4: integrate some recent internal changes, including format changes

Change-Id: I12216da14910f5e2b0970bc1fafc2a20b0ef1ba9
Reviewed-by: Junwei Zhang 
Signed-off-by: Samuel Li 
---
 Makefile.am  |   3 +
 amdgpu/Makefile.am   |   2 +
 amdgpu/Makefile.sources  |   2 +-
 amdgpu/amdgpu_asic_id.c  | 206 +++
 amdgpu/amdgpu_asic_id.h  | 165 -
 amdgpu/amdgpu_device.c   |  28 +--
 amdgpu/amdgpu_internal.h |  10 +++
 include/drm/amdgpu.ids   | 170 ++
 8 files changed, 413 insertions(+), 173 deletions(-)
 create mode 100644 amdgpu/amdgpu_asic_id.c
 delete mode 100644 amdgpu/amdgpu_asic_id.h
 create mode 100644 include/drm/amdgpu.ids

diff --git a/Makefile.am b/Makefile.am
index dfb8fcd..8de8f6c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -45,6 +45,9 @@ AM_DISTCHECK_CONFIGURE_FLAGS = \
 
 pkgconfigdir = @pkgconfigdir@
 pkgconfig_DATA = libdrm.pc
+libdrmdatadir = $(datadir)/libdrm
+dist_libdrmdata_DATA = include/drm/amdgpu.ids
+export libdrmdatadir
 
 if HAVE_LIBKMS
 LIBKMS_SUBDIR = libkms
diff --git a/amdgpu/Makefile.am b/amdgpu/Makefile.am
index cf7bc1b..da71c1c 100644
--- a/amdgpu/Makefile.am
+++ b/amdgpu/Makefile.am
@@ -30,6 +30,8 @@ AM_CFLAGS = \
$(PTHREADSTUBS_CFLAGS) \
-I$(top_srcdir)/include/drm
 
+AM_CPPFLAGS = -DAMDGPU_ASIC_ID_TABLE=\"${libdrmdatadir}/amdgpu.ids\"
+
 libdrm_amdgpu_la_LTLIBRARIES = libdrm_amdgpu.la
 libdrm_amdgpu_ladir = $(libdir)
 libdrm_amdgpu_la_LDFLAGS = -version-number 1:0:0 -no-undefined
diff --git a/amdgpu/Makefile.sources b/amdgpu/Makefile.sources
index 487b9e0..bc3abaa 100644
--- a/amdgpu/Makefile.sources
+++ b/amdgpu/Makefile.sources
@@ -1,5 +1,5 @@
 LIBDRM_AMDGPU_FILES := \
-   amdgpu_asic_id.h \
+   amdgpu_asic_id.c \
amdgpu_bo.c \
amdgpu_cs.c \
amdgpu_device.c \
diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c
new file mode 100644
index 000..a43ca33
--- /dev/null
+++ b/amdgpu/amdgpu_asic_id.c
@@ -0,0 +1,206 @@
+/*
+ * Copyright © 2017 Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "xf86drm.h"
+#include "amdgpu_drm.h"
+#include "amdgpu_internal.h"
+
+static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
+{
+   char *buf, *saveptr;
+   char *s_did;
+   char *s_rid;
+   char *s_name;
+   char *endptr;
+   int r = 0;
+
+   buf = strdup(line);
+   if (!buf)
+   return -ENOMEM;
+
+   /* ignore empty line and commented line */
+   if (strlen(line) == 0 || line[0] == '#') {
+   r = -EAGAIN;
+   goto out;
+   }
+
+   /* device id */
+   s_did = strtok_r(buf, ",", );
+   if (!s_did) {
+   r = -EINVAL;
+   goto out;
+   }
+
+   id->did = strtol(s_did, , 16);
+   if (*endptr) {
+   r = -EINVAL;
+   goto out;
+   }
+
+   /* revision id */
+   s_rid = strtok_r(NULL, ",", );
+   if (!s_rid) {
+   r = -EINVAL;
+   goto out;
+   }
+
+   id->rid = strtol(s_rid, , 16);
+   if (*endptr) {
+   r = -EINVAL;
+   goto out;
+   }
+
+   /* marketing name */
+   s_name = strtok_r(NULL, ",", );
+   if (!s_name) {
+   r = -EINVAL;
+   goto out;
+   }
+   /* trim leading whitespaces or tabs */
+   while (*s_name == ' ' || *s_name ==

RE: [PATCH libdrm v3 1/1] amdgpu: move asic id table to a separate file

2017-05-29 Thread Li, Samuel 
Understood your point. However as discussed internally before, marketing names 
are there for a lot of reasons; my understanding of the policy is we do not 
need to touch them as long as there is no error in the names and they are 
allowed to be public.

Regards,
Sam

-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com] 
Sent: Friday, May 26, 2017 3:27 PM
To: Li, Samuel 
Cc: amd-gfx list ; Yuan, Xiaojie 

Subject: Re: [PATCH libdrm v3 1/1] amdgpu: move asic id table to a separate file

On Fri, May 26, 2017 at 3:23 PM, Samuel Li  wrote:
> From: Xiaojie Yuan 
>
> v2: fix an off by one error and leading white spaces
> v3: use thread safe strtok_r(); initialize len before calling getline();
> change printf() to drmMsg(); add initial amdgpu.ids
>
> Change-Id: I12216da14910f5e2b0970bc1fafc2a20b0ef1ba9
> Reviewed-by: Junwei Zhang 
> Signed-off-by: Samuel Li 
> ---
>  Makefile.am  |   3 +
>  amdgpu/Makefile.am   |   2 +
>  amdgpu/Makefile.sources  |   2 +-
>  amdgpu/amdgpu_asic_id.c  | 199 
> +++
>  amdgpu/amdgpu_asic_id.h  | 165 ---
>  amdgpu/amdgpu_device.c   |  28 +--
>  amdgpu/amdgpu_internal.h |  10 +++
>  include/drm/amdgpu.ids   | 154 
>  8 files changed, 390 insertions(+), 173 deletions(-)  create mode 
> 100644 amdgpu/amdgpu_asic_id.c  delete mode 100644 
> amdgpu/amdgpu_asic_id.h  create mode 100644 include/drm/amdgpu.ids
>
> diff --git a/Makefile.am b/Makefile.am index dfb8fcd..8de8f6c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -45,6 +45,9 @@ AM_DISTCHECK_CONFIGURE_FLAGS = \
>
>  pkgconfigdir = @pkgconfigdir@
>  pkgconfig_DATA = libdrm.pc
> +libdrmdatadir = $(datadir)/libdrm
> +dist_libdrmdata_DATA = include/drm/amdgpu.ids export libdrmdatadir
>
>  if HAVE_LIBKMS
>  LIBKMS_SUBDIR = libkms
> diff --git a/amdgpu/Makefile.am b/amdgpu/Makefile.am index 
> cf7bc1b..da71c1c 100644
> --- a/amdgpu/Makefile.am
> +++ b/amdgpu/Makefile.am
> @@ -30,6 +30,8 @@ AM_CFLAGS = \
> $(PTHREADSTUBS_CFLAGS) \
> -I$(top_srcdir)/include/drm
>
> +AM_CPPFLAGS = -DAMDGPU_ASIC_ID_TABLE=\"${libdrmdatadir}/amdgpu.ids\"
> +
>  libdrm_amdgpu_la_LTLIBRARIES = libdrm_amdgpu.la  libdrm_amdgpu_ladir 
> = $(libdir)  libdrm_amdgpu_la_LDFLAGS = -version-number 1:0:0 
> -no-undefined diff --git a/amdgpu/Makefile.sources 
> b/amdgpu/Makefile.sources index 487b9e0..bc3abaa 100644
> --- a/amdgpu/Makefile.sources
> +++ b/amdgpu/Makefile.sources
> @@ -1,5 +1,5 @@
>  LIBDRM_AMDGPU_FILES := \
> -   amdgpu_asic_id.h \
> +   amdgpu_asic_id.c \
> amdgpu_bo.c \
> amdgpu_cs.c \
> amdgpu_device.c \
> diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c new 
> file mode 100644 index 000..5b415e3
> --- /dev/null
> +++ b/amdgpu/amdgpu_asic_id.c
> @@ -0,0 +1,199 @@
> +/*
> + * Copyright © 2017 Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the 
> +"Software"),
> + * to deal in the Software without restriction, including without 
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute, 
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom 
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be 
> +included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
> +SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
> +DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
> +OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE 
> +OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "xf86drm.h"
> +#include "amdgpu_drm.h"
> +#include "amdgpu_internal.h"
> +
> +static int parse_one_line(const char *line, struct amdgpu_asic_id 
> +*id) {
> +   char *buf, *saveptr;
> +   char *s_did;
> +   char *s_rid;
> +   char *s_name;
> +   char *endptr;
> +   int r = 0;
> +
> +   buf = strdup(line);
> +   if (!buf)
> +   return -ENOMEM;
> +
> +   /* ignore empty line and commented line */
> +   if (strlen(line) == 0 || line[0] == '#') {
> +

Re: [PATCH 1/3] drm/amdgpu: Really leave SI support disabled by default

2017-05-29 Thread Christian König 

Am 29.05.2017 um 11:20 schrieb Michel Dänzer:

From: Michel Dänzer 

The default option value didn't match the help text and intention.

Signed-off-by: Michel Dänzer 


I'm still unsure about the last one. The feature parity is a good 
argument but on the other hand we want people to use amdgpu for CIK 
these days, don't we?


Anyway Reviewed-by: Christian König .

Regards,
Christian.


---

Maybe this can be squashed into the commit adding this option when it
goes upstream.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 17ecc2542af5..76dea5fe620b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -234,7 +234,7 @@ MODULE_PARM_DESC(param_buf_per_se, "the size of Off-Chip 
Pramater Cache per Shad
  module_param_named(param_buf_per_se, amdgpu_param_buf_per_se, int, 0444);
  
  #ifdef CONFIG_DRM_AMDGPU_SI

-int amdgpu_si_support = 1;
+int amdgpu_si_support = 0;
  MODULE_PARM_DESC(si_support, "SI support (1 = enabled, 0 = disabled 
(default))");
  module_param_named(si_support, amdgpu_si_support, int, 0444);
  #endif



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

Re: [PATCH] iommu/amd: flush IOTLB for specific domains only (v3)

2017-05-29 Thread Joerg Roedel 
Hi Arindam,

I met Tom Lendacky last week in Nuremberg last week and he told me he is
working on the same area of the code that this patch is for. His reason
for touching this code was to solve some locking problems. Maybe you two
can work together on a joint approach to improve this?

On Mon, May 22, 2017 at 01:18:01PM +0530, arindam.n...@amd.com wrote:
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 63cacf5..1edeebec 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -2227,15 +2227,26 @@ static struct iommu_group 
> *amd_iommu_device_group(struct device *dev)
>  
>  static void __queue_flush(struct flush_queue *queue)
>  {
> - struct protection_domain *domain;
> - unsigned long flags;
>   int idx;
>  
> - /* First flush TLB of all known domains */
> - spin_lock_irqsave(_iommu_pd_lock, flags);
> - list_for_each_entry(domain, _iommu_pd_list, list)
> - domain_flush_tlb(domain);
> - spin_unlock_irqrestore(_iommu_pd_lock, flags);
> + /* First flush TLB of all domains which were added to flush queue */
> + for (idx = 0; idx < queue->next; ++idx) {
> + struct flush_queue_entry *entry;
> +
> + entry = queue->entries + idx;
> +
> + /*
> +  * There might be cases where multiple IOVA entries for the
> +  * same domain are queued in the flush queue. To avoid
> +  * flushing the same domain again, we check whether the
> +  * flag is set or not. This improves the efficiency of
> +  * flush operation.
> +  */
> + if (!entry->dma_dom->domain.already_flushed) {
> + entry->dma_dom->domain.already_flushed = true;
> + domain_flush_tlb(>dma_dom->domain);
> + }
> + }

There is also a race condition here I have to look into. It is not
introduced by your patch, but needs fixing anyway. I'll look into this
too.


Regards,

Joerg

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

Re: [PATCH 3/3] drm/amdgpu/radeon: Use radeon by default for CIK GPUs

2017-05-29 Thread Kai Wasserbäch 
Hey Michel,
Michel Dänzer wrote on 29.05.2017 11:20:
> From: Michel Dänzer 
> 
> Even if CONFIG_DRM_AMDGPU_CIK is enabled.
> 
> There is no feature parity yet for CIK, in particular amdgpu doesn't
> support HDMI/DisplayPort without DC.

that can't be correct. I'm using amdgpu on a CIK ASIC (Hawaii Pro) and my
monitor is connected by DisplayPort. From my Xorg.0.log:
> [37.811] (II) AMDGPU(0): EDID for output DisplayPort-0
> [37.811] (II) AMDGPU(0): Manufacturer: SAM  Model: 83d  Serial#: 0
> [37.811] (II) AMDGPU(0): Year: 2011  Week: 7
> [37.811] (II) AMDGPU(0): EDID Version: 1.4
> [37.811] (II) AMDGPU(0): Digital Display Input
> [37.811] (II) AMDGPU(0): 8 bits per channel
> [37.811] (II) AMDGPU(0): Digital interface is DisplayPort
> [37.811] (II) AMDGPU(0): Max Image Size [cm]: horiz.: 52  vert.: 32
> [37.811] (II) AMDGPU(0): Gamma: 2.20
> [37.811] (II) AMDGPU(0): DPMS capabilities: Off
> [37.811] (II) AMDGPU(0): Supported color encodings: RGB 4:4:4 
> [37.811] (II) AMDGPU(0): First detailed timing is preferred mode
> [37.811] (II) AMDGPU(0): Preferred mode is native pixel format and 
> refresh rate
> [37.811] (II) AMDGPU(0): redX: 0.650 redY: 0.337   greenX: 0.296 greenY: 
> 0.604
> [37.811] (II) AMDGPU(0): blueX: 0.147 blueY: 0.074   whiteX: 0.312 
> whiteY: 0.329
> [37.811] (II) AMDGPU(0): Supported established timings: [...]

Or do you mean some features like audio over HDMI/DP?

> Signed-off-by: Michel Dänzer 
> ---
>  drivers/gpu/drm/amd/amdgpu/Kconfig  | 8 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 7 +--
>  drivers/gpu/drm/radeon/radeon_drv.c | 4 ++--
>  drivers/gpu/drm/radeon/radeon_kms.c | 5 +
>  5 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
> b/drivers/gpu/drm/amd/amdgpu/Kconfig
> index 8d36087fc186..e0121f8b436e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Kconfig
> +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
> @@ -17,11 +17,11 @@ config DRM_AMDGPU_CIK
>   help
> Choose this option if you want to enable support for CIK asics.
>  
> -   CIK is already supported in radeon. If you enable this option,
> -   support for CIK will be provided by amdgpu and disabled in
> -   radeon by default. Use module options to override this:
> +   CIK is already supported in radeon. Support for SI in amdgpu
> +   will be disabled by default and is still provided by radeon.
> +   Use module options to override this:
>  
> -   radeon.cik_support=1 amdgpu.cik_support=0
> +   radeon.cik_support=0 amdgpu.cik_support=1

What happens if I have radeon blacklisted? Do I still need to set this
additional flag? If so I'd kindly request to at least fall back to amdgpu if
somebody blacklisted radeon (or didn't compile it). Maybe issue a warning that
not all features are supported yet.

Thank you for your consideration!

Cheers,
Kai


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

Re: [PATCH 5/5] amdgpu: use drm sync objects for shared semaphores (v5)

2017-05-29 Thread Christian König 

Am 29.05.2017 um 09:30 schrieb Dave Airlie:

From: Dave Airlie 

This creates a new command submission chunk for amdgpu
to add in and out sync objects around the submission.

Sync objects are managed via the drm syncobj ioctls.

The command submission interface is enhanced with two new
chunks, one for syncobj pre submission dependencies,
and one for post submission sync obj signalling,
and just takes a list of handles for each.

This is based on work originally done by David Zhou at AMD,
with input from Christian Konig on what things should look like.

In theory VkFences could be backed with sync objects and
just get passed into the cs as syncobj handles as well.

NOTE: this interface addition needs a version bump to expose
it to userspace.

TODO: update to dep_sync when rebasing onto amdgpu master.
(with this - r-b from Christian)

v1.1: keep file reference on import.
v2: move to using syncobjs
v2.1: change some APIs to just use p pointer.
v3: make more robust against CS failures, we now add the
wait sems but only remove them once the CS job has been
submitted.
v4: rewrite names of API and base on new syncobj code.
v5: move post deps earlier, rename some apis

Signed-off-by: Dave Airlie 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 87 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 +-
  include/uapi/drm/amdgpu_drm.h   |  6 +++
  3 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 30225d7..3cbd547 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -27,6 +27,7 @@
  #include 
  #include 
  #include 
+#include 
  #include "amdgpu.h"
  #include "amdgpu_trace.h"
  
@@ -226,6 +227,8 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data)

break;
  
  		case AMDGPU_CHUNK_ID_DEPENDENCIES:

+   case AMDGPU_CHUNK_ID_SYNCOBJ_IN:
+   case AMDGPU_CHUNK_ID_SYNCOBJ_OUT:
break;
  
  		default:

@@ -1040,6 +1043,40 @@ static int amdgpu_cs_process_fence_dep(struct 
amdgpu_cs_parser *p,
return 0;
  }
  
+static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p,

+uint32_t handle)
+{
+   int r;
+   struct dma_fence *fence;
+   r = drm_syncobj_fence_get(p->filp, handle, );
+   if (r)
+   return r;
+
+   r = amdgpu_sync_fence(p->adev, >job->sync, fence);
+   dma_fence_put(fence);
+
+   return r;
+}
+
+static int amdgpu_cs_process_syncobj_in_dep(struct amdgpu_cs_parser *p,
+   struct amdgpu_cs_chunk *chunk)
+{
+   unsigned num_deps;
+   int i, r;
+   struct drm_amdgpu_cs_chunk_sem *deps;
+
+   deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata;
+   num_deps = chunk->length_dw * 4 /
+   sizeof(struct drm_amdgpu_cs_chunk_sem);
+
+   for (i = 0; i < num_deps; ++i) {
+   r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle);
+   if (r)
+   return r;
+   }
+   return 0;
+}
+
  static int amdgpu_cs_dependencies(struct amdgpu_device *adev,
  struct amdgpu_cs_parser *p)
  {
@@ -1054,12 +1091,54 @@ static int amdgpu_cs_dependencies(struct amdgpu_device 
*adev,
r = amdgpu_cs_process_fence_dep(p, chunk);
if (r)
return r;
+   } else if (chunk->chunk_id == AMDGPU_CHUNK_ID_SYNCOBJ_IN) {
+   r = amdgpu_cs_process_syncobj_in_dep(p, chunk);
+   if (r)
+   return r;
}
}
  
  	return 0;

  }
  
+static int amdgpu_cs_process_syncobj_out_dep(struct amdgpu_cs_parser *p,

+struct amdgpu_cs_chunk *chunk)
+{
+   unsigned num_deps;
+   int i, r;
+   struct drm_amdgpu_cs_chunk_sem *deps;
+
+   deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata;
+   num_deps = chunk->length_dw * 4 /
+   sizeof(struct drm_amdgpu_cs_chunk_sem);
+
+   for (i = 0; i < num_deps; ++i) {
+   r = drm_syncobj_replace_fence(p->filp, deps[i].handle,
+ p->fence);
+   if (r)
+   return r;


Thinking more about it, here is still a problem with the handling.

Imagine you have multiple semaphores to signal and only the last handle 
is invalid.


So if you have n handles then the sync objects 1..(n-1) will get a fence 
which is never signaled because n aborts the command submission.


I think the only valid approach to handle this is to resolve the handles 
upfront during the initial parsing of the chunks.


Christian.


+   }
+   return 0;

[PATCH 3/3] drm/amdgpu/radeon: Use radeon by default for CIK GPUs

2017-05-29 Thread Michel Dänzer 
From: Michel Dänzer 

Even if CONFIG_DRM_AMDGPU_CIK is enabled.

There is no feature parity yet for CIK, in particular amdgpu doesn't
support HDMI/DisplayPort without DC.

Signed-off-by: Michel Dänzer 
---
 drivers/gpu/drm/amd/amdgpu/Kconfig  | 8 
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 7 +--
 drivers/gpu/drm/radeon/radeon_drv.c | 4 ++--
 drivers/gpu/drm/radeon/radeon_kms.c | 5 +
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig 
b/drivers/gpu/drm/amd/amdgpu/Kconfig
index 8d36087fc186..e0121f8b436e 100644
--- a/drivers/gpu/drm/amd/amdgpu/Kconfig
+++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
@@ -17,11 +17,11 @@ config DRM_AMDGPU_CIK
help
  Choose this option if you want to enable support for CIK asics.
 
- CIK is already supported in radeon. If you enable this option,
- support for CIK will be provided by amdgpu and disabled in
- radeon by default. Use module options to override this:
+ CIK is already supported in radeon. Support for SI in amdgpu
+ will be disabled by default and is still provided by radeon.
+ Use module options to override this:
 
- radeon.cik_support=1 amdgpu.cik_support=0
+ radeon.cik_support=0 amdgpu.cik_support=1
 
 config DRM_AMDGPU_USERPTR
bool "Always enable userptr write support"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 76dea5fe620b..27599db7d630 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -240,8 +240,8 @@ module_param_named(si_support, amdgpu_si_support, int, 
0444);
 #endif
 
 #ifdef CONFIG_DRM_AMDGPU_CIK
-int amdgpu_cik_support = 1;
-MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled (default), 0 = 
disabled)");
+int amdgpu_cik_support = 0;
+MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled 
(default))");
 module_param_named(cik_support, amdgpu_cik_support, int, 0444);
 #endif
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index f4f77b99afeb..31901d2886d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -98,7 +98,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned 
long flags)
dev_info(dev->dev,
 "SI support provided by radeon.\n");
dev_info(dev->dev,
-   "Use radeon.si_support=0 amdgpu.si_support=1 to override.\n"
+"Use radeon.si_support=0 amdgpu.si_support=1 
to override.\n"
);
return -ENODEV;
}
@@ -113,7 +113,10 @@ int amdgpu_driver_load_kms(struct drm_device *dev, 
unsigned long flags)
case CHIP_KABINI:
case CHIP_MULLINS:
dev_info(dev->dev,
-"CIK support disabled by module param\n");
+"CIK support provided by radeon.\n");
+   dev_info(dev->dev,
+"Use radeon.cik_support=0 amdgpu.cik_support=1 
to override.\n"
+   );
return -ENODEV;
}
}
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index 1cefadfe4a41..853ef118a735 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -307,8 +307,8 @@ int radeon_si_support = 1;
 MODULE_PARM_DESC(si_support, "SI support (1 = enabled (default), 0 = 
disabled)");
 module_param_named(si_support, radeon_si_support, int, 0444);
 
-int radeon_cik_support = 0;
-MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled 
(default))");
+int radeon_cik_support = 1;
+MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled (default), 0 = 
disabled)");
 module_param_named(cik_support, radeon_cik_support, int, 0444);
 
 static struct pci_device_id pciidlist[] = {
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c 
b/drivers/gpu/drm/radeon/radeon_kms.c
index baaddf989f6d..8f2579772c34 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -118,10 +118,7 @@ int radeon_driver_load_kms(struct drm_device *dev, 
unsigned long flags)
case CHIP_KABINI:
case CHIP_MULLINS:
dev_info(dev->dev,
-"CIK support provided by amdgpu.\n");
-   dev_info(dev->dev,
-   "Use radeon.cik_support=1 amdgpu.cik_support=0 to override.\n"
-   );
+"CIK support disabled by module param\n");
return -ENODEV;

[PATCH 1/3] drm/amdgpu: Really leave SI support disabled by default

2017-05-29 Thread Michel Dänzer 
From: Michel Dänzer 

The default option value didn't match the help text and intention.

Signed-off-by: Michel Dänzer 
---

Maybe this can be squashed into the commit adding this option when it
goes upstream.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 17ecc2542af5..76dea5fe620b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -234,7 +234,7 @@ MODULE_PARM_DESC(param_buf_per_se, "the size of Off-Chip 
Pramater Cache per Shad
 module_param_named(param_buf_per_se, amdgpu_param_buf_per_se, int, 0444);
 
 #ifdef CONFIG_DRM_AMDGPU_SI
-int amdgpu_si_support = 1;
+int amdgpu_si_support = 0;
 MODULE_PARM_DESC(si_support, "SI support (1 = enabled, 0 = disabled 
(default))");
 module_param_named(si_support, amdgpu_si_support, int, 0444);
 #endif
-- 
2.11.0

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

[PATCH 2/3] drm/radeon: Make si_support and cik_support parameters always available

2017-05-29 Thread Michel Dänzer 
From: Michel Dänzer 

This will allow amdgpu-pro / other out-of-tree amdgpu builds to make use
of these options for using the out-of-tree amdgpu driver instead of the
in-tree radeon driver in a clean way.

Signed-off-by: Michel Dänzer 
---
 drivers/gpu/drm/radeon/radeon.h | 5 -
 drivers/gpu/drm/radeon/radeon_drv.c | 4 
 drivers/gpu/drm/radeon/radeon_kms.c | 4 
 3 files changed, 13 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index b0ce11ebc677..fb0c27be4ee6 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -116,13 +116,8 @@ extern int radeon_auxch;
 extern int radeon_mst;
 extern int radeon_uvd;
 extern int radeon_vce;
-
-#ifdef CONFIG_DRM_AMDGPU_SI
 extern int radeon_si_support;
-#endif
-#ifdef CONFIG_DRM_AMDGPU_CIK
 extern int radeon_cik_support;
-#endif
 
 /*
  * Copy from radeon_drv.h so we don't have to include both and have conflicting
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index c0c1b258d570..1cefadfe4a41 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -303,17 +303,13 @@ module_param_named(uvd, radeon_uvd, int, 0444);
 MODULE_PARM_DESC(vce, "vce enable/disable vce support (1 = enable, 0 = 
disable)");
 module_param_named(vce, radeon_vce, int, 0444);
 
-#ifdef CONFIG_DRM_AMDGPU_SI
 int radeon_si_support = 1;
 MODULE_PARM_DESC(si_support, "SI support (1 = enabled (default), 0 = 
disabled)");
 module_param_named(si_support, radeon_si_support, int, 0444);
-#endif
 
-#ifdef CONFIG_DRM_AMDGPU_CIK
 int radeon_cik_support = 0;
 MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled 
(default))");
 module_param_named(cik_support, radeon_cik_support, int, 0444);
-#endif
 
 static struct pci_device_id pciidlist[] = {
radeon_PCI_IDS
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c 
b/drivers/gpu/drm/radeon/radeon_kms.c
index 09fc0e1137a1..baaddf989f6d 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -98,7 +98,6 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned 
long flags)
struct radeon_device *rdev;
int r, acpi_status;
 
-#ifdef CONFIG_DRM_AMDGPU_SI
if (!radeon_si_support) {
switch (flags & RADEON_FAMILY_MASK) {
case CHIP_TAHITI:
@@ -111,8 +110,6 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned 
long flags)
return -ENODEV;
}
}
-#endif
-#ifdef CONFIG_DRM_AMDGPU_CIK
if (!radeon_cik_support) {
switch (flags & RADEON_FAMILY_MASK) {
case CHIP_KAVERI:
@@ -128,7 +125,6 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned 
long flags)
return -ENODEV;
}
}
-#endif
 
rdev = kzalloc(sizeof(struct radeon_device), GFP_KERNEL);
if (rdev == NULL) {
-- 
2.11.0

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

[PATCH xf86-video-amdgpu] Use reference counting for tracking KMS framebuffer lifetimes

2017-05-29 Thread Michel Dänzer 
From: Michel Dänzer 

References are held by the pixmaps corresponding to the FBs (so
the same KMS FB can be reused as long as the pixmap exists) and by the
CRTCs scanning out from them (so a KMS FB is only destroyed once it's
not being scanned out anymore, preventing intermittent black screens and
worse issues due to a CRTC turning off when it should be on).

v2:
* Only increase reference count in drmmode_fb_reference if it was sane
  before
* Make drmmode_fb_reference's indentation match the rest of
  drmmode_display.h

(Ported from radeon commit 55e513b978b2afc52b7cafc5bfcb0d1dc78d75f6)

Signed-off-by: Michel Dänzer 
---
 src/amdgpu_kms.c  |  70 +++---
 src/amdgpu_pixmap.h   |  58 +++
 src/amdgpu_present.c  |   9 ---
 src/drmmode_display.c | 157 --
 src/drmmode_display.h |  42 --
 5 files changed, 219 insertions(+), 117 deletions(-)

diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index a418cf9d3..69d61943d 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -675,6 +675,18 @@ amdgpu_prime_scanout_flip_abort(xf86CrtcPtr crtc, void 
*event_data)
 }
 
 static void
+amdgpu_prime_scanout_flip_handler(xf86CrtcPtr crtc, uint32_t msc, uint64_t 
usec,
+ void *event_data)
+{
+   AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn);
+   drmmode_crtc_private_ptr drmmode_crtc = event_data;
+
+   drmmode_fb_reference(pAMDGPUEnt->fd, _crtc->fb,
+drmmode_crtc->flip_pending);
+   amdgpu_prime_scanout_flip_abort(crtc, event_data);
+}
+
+static void
 amdgpu_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
 {
ScreenPtr screen = ent->slave_dst->drawable.pScreen;
@@ -701,7 +713,8 @@ amdgpu_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
drm_queue_seq = amdgpu_drm_queue_alloc(crtc,
   AMDGPU_DRM_QUEUE_CLIENT_DEFAULT,
   AMDGPU_DRM_QUEUE_ID_DEFAULT,
-  drmmode_crtc, NULL,
+  drmmode_crtc,
+  
amdgpu_prime_scanout_flip_handler,
   amdgpu_prime_scanout_flip_abort);
if (drm_queue_seq == AMDGPU_DRM_QUEUE_ERROR) {
xf86DrvMsg(scrn->scrnIndex, X_WARNING,
@@ -709,8 +722,17 @@ amdgpu_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
return;
}
 
+   drmmode_crtc->flip_pending =
+   amdgpu_pixmap_get_fb(drmmode_crtc->scanout[scanout_id].pixmap);
+   if (!drmmode_crtc->flip_pending) {
+   xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+  "Failed to get FB for PRIME flip.\n");
+   amdgpu_drm_abort_entry(drm_queue_seq);
+   return;
+   }
+
if (drmmode_page_flip_target_relative(pAMDGPUEnt, drmmode_crtc,
- 
drmmode_crtc->scanout[scanout_id].fb_id,
+ 
drmmode_crtc->flip_pending->handle,
  0, drm_queue_seq, 0) != 0) {
xf86DrvMsg(scrn->scrnIndex, X_WARNING, "flip queue failed in 
%s: %s\n",
   __func__, strerror(errno));
@@ -720,7 +742,6 @@ amdgpu_prime_scanout_flip(PixmapDirtyUpdatePtr ent)
 
drmmode_crtc->scanout_id = scanout_id;
drmmode_crtc->scanout_update_pending = TRUE;
-   drmmode_crtc->flip_pending = TRUE;
 }
 
 static void
@@ -950,10 +971,14 @@ amdgpu_scanout_update(xf86CrtcPtr xf86_crtc)
 static void
 amdgpu_scanout_flip_abort(xf86CrtcPtr crtc, void *event_data)
 {
-   drmmode_crtc_private_ptr drmmode_crtc = event_data;
+   amdgpu_prime_scanout_flip_abort(crtc, event_data);
+}
 
-   drmmode_crtc->scanout_update_pending = FALSE;
-   drmmode_clear_pending_flip(crtc);
+static void
+amdgpu_scanout_flip_handler(xf86CrtcPtr crtc, uint32_t msc, uint64_t usec,
+   void *event_data)
+{
+   amdgpu_prime_scanout_flip_handler(crtc, msc, usec, event_data);
 }
 
 static void
@@ -977,7 +1002,8 @@ amdgpu_scanout_flip(ScreenPtr pScreen, AMDGPUInfoPtr info,
drm_queue_seq = amdgpu_drm_queue_alloc(xf86_crtc,
   AMDGPU_DRM_QUEUE_CLIENT_DEFAULT,
   AMDGPU_DRM_QUEUE_ID_DEFAULT,
-  drmmode_crtc, NULL,
+  drmmode_crtc,
+  amdgpu_scanout_flip_handler,
   amdgpu_scanout_flip_abort);
if (drm_queue_seq == AMDGPU_DRM_QUEUE_ERROR) {
xf86DrvMsg(scrn->scrnIndex, X_WARNING,
@@ -985,8 +1011,17 @@ amdgpu_scanout_flip(ScreenPtr pScreen,

[PATCH 2/5] drm/syncobj: add sync obj wait interface. (v4)

2017-05-29 Thread Dave Airlie 
From: Dave Airlie 

This interface will allow sync object to be used to back
Vulkan fences. This API is pretty much the vulkan fence waiting
API, and I've ported the code from amdgpu.

v2: accept relative timeout, pass remaining time back
to userspace.
v3: return to absolute timeouts.
v4: absolute zero = poll,
rewrite any/all code to have same operation for arrays
return -EINVAL for 0 fences.

Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/drm_internal.h |   2 +
 drivers/gpu/drm/drm_ioctl.c|   2 +
 drivers/gpu/drm/drm_syncobj.c  | 129 +
 include/uapi/drm/drm.h |  14 +
 4 files changed, 147 insertions(+)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 3fdef2c..53e3f6b 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -156,3 +156,5 @@ int drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, 
void *data,
   struct drm_file *file_private);
 int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file_private);
+int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
+  struct drm_file *file_private);
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index f1e5681..385ce74 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -657,6 +657,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE, 
drm_syncobj_fd_to_handle_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
 };
 
 #define DRM_CORE_IOCTL_COUNT   ARRAY_SIZE( drm_ioctls )
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index b611480..5177b36 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1,5 +1,7 @@
 /*
  * Copyright 2017 Red Hat
+ * Parts ported from amdgpu (fence wait code).
+ * Copyright 2016 Advanced Micro Devices, Inc.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -31,6 +33,9 @@
  * that contain an optional fence. The fence can be updated with a new
  * fence, or be NULL.
  *
+ * syncobj's can be waited upon, where it will wait for the underlying
+ * fence.
+ *
  * syncobj's can be export to fd's and back, these fd's are opaque and
  * have no other use case, except passing the syncobj between processes.
  *
@@ -375,3 +380,127 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, 
void *data,
return drm_syncobj_fd_to_handle(file_private, args->fd,
>handle);
 }
+
+
+/**
+ * drm_timeout_abs_to_jiffies - calculate jiffies timeout from absolute value
+ *
+ * @timeout_ns: timeout in ns, 0 for poll
+ *
+ * Calculate the timeout in jiffies from an absolute timeout in ns.
+ */
+static unsigned long drm_timeout_abs_to_jiffies(uint64_t timeout_ns)
+{
+   unsigned long timeout_jiffies;
+   ktime_t timeout;
+
+   /* make 0 timeout means poll - absolute 0 doesn't seem valid */
+   if (timeout_ns == 0)
+   return 0;
+
+   /* clamp timeout if it's to large */
+   if (((int64_t)timeout_ns) < 0)
+   return MAX_SCHEDULE_TIMEOUT;
+
+   timeout = ktime_sub(ns_to_ktime(timeout_ns), ktime_get());
+   if (ktime_to_ns(timeout) < 0)
+   return 0;
+
+   timeout_jiffies = nsecs_to_jiffies(ktime_to_ns(timeout));
+   /*  clamp timeout to avoid infinite timeout */
+   if (timeout_jiffies >= MAX_SCHEDULE_TIMEOUT)
+   return MAX_SCHEDULE_TIMEOUT - 1;
+
+   return timeout_jiffies + 1;
+}
+
+static int drm_syncobj_wait_fences(struct drm_device *dev,
+  struct drm_file *file_private,
+  struct drm_syncobj_wait *wait,
+  struct dma_fence **fences)
+{
+   unsigned long timeout = drm_timeout_abs_to_jiffies(wait->timeout_ns);
+   int ret = 0;
+   uint32_t first = ~0;
+
+   if (wait->flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL) {
+   int i;
+   for (i = 0; i < wait->count_handles; i++) {
+   ret = dma_fence_wait_timeout(fences[i], true, timeout);
+
+   if (ret < 0)
+   return ret;
+   if (ret == 0)
+   break;
+   timeout = ret;
+   }
+   first = 0;
+   } else
+   ret = dma_fence_wait_any_timeout(fences,
+wait->count_handles,
+

[PATCH 3/5] drm/syncobj: add sync_file interaction. (v1.1)

2017-05-29 Thread Dave Airlie 
From: Dave Airlie 

This interface allows importing the fence from a sync_file into
an existing drm sync object, or exporting the fence attached to
an existing drm sync object into a new sync file object.

This should only be used to interact with sync files where necessary.

v1.1: fence put fixes (Chris), drop fence from ioctl names (Chris)

Reviewed-by: Sean Paul 
Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/drm_syncobj.c | 67 +--
 include/uapi/drm/drm.h|  2 ++
 2 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 5177b36..4694021 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -50,6 +50,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "drm_internal.h"
 #include 
@@ -276,6 +277,51 @@ static int drm_syncobj_fd_to_handle(struct drm_file 
*file_private,
return 0;
 }
 
+int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
+  int fd, int handle)
+{
+   struct dma_fence *fence = sync_file_get_fence(fd);
+   int ret;
+   if (!fence)
+   return -EINVAL;
+
+   ret = drm_syncobj_replace_fence(file_private, handle, fence);
+   dma_fence_put(fence);
+   return ret;
+}
+
+int drm_syncobj_export_sync_file(struct drm_file *file_private,
+int handle, int *p_fd)
+{
+   int ret;
+   struct dma_fence *fence;
+   struct sync_file *sync_file;
+   int fd = get_unused_fd_flags(O_CLOEXEC);
+
+   if (fd < 0)
+   return fd;
+
+   ret = drm_syncobj_fence_get(file_private, handle, );
+   if (ret)
+   goto err_put_fd;
+
+   sync_file = sync_file_create(fence);
+
+   dma_fence_put(fence);
+
+   if (!sync_file) {
+   ret = -EINVAL;
+   goto err_put_fd;
+   }
+
+   fd_install(fd, sync_file->file);
+
+   *p_fd = fd;
+   return 0;
+err_put_fd:
+   put_unused_fd(fd);
+   return ret;
+}
 /**
  * drm_syncobj_open - initalizes syncobj file-private structures at devnode 
open time
  * @dev: drm_device which is being opened by userspace
@@ -358,9 +404,17 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, 
void *data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
return -ENODEV;
 
-   if (args->pad || args->flags)
+   if (args->pad)
+   return -EINVAL;
+
+   if (args->flags != 0 &&
+   args->flags != DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
return -EINVAL;
 
+   if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE)
+   return drm_syncobj_export_sync_file(file_private, args->handle,
+   >fd);
+
return drm_syncobj_handle_to_fd(file_private, args->handle,
>fd);
 }
@@ -374,9 +428,18 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, 
void *data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
return -ENODEV;
 
-   if (args->pad || args->flags)
+   if (args->pad)
+   return -EINVAL;
+
+   if (args->flags != 0 &&
+   args->flags != DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE)
return -EINVAL;
 
+   if (args->flags & DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE)
+   return drm_syncobj_import_sync_file_fence(file_private,
+ args->fd,
+ args->handle);
+
return drm_syncobj_fd_to_handle(file_private, args->fd,
>handle);
 }
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index d6e2f62..49c4e69 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -708,6 +708,8 @@ struct drm_syncobj_destroy {
__u32 pad;
 };
 
+#define DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE (1 << 0)
+#define DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE (1 << 0)
 struct drm_syncobj_handle {
__u32 handle;
__u32 flags;
-- 
2.9.4

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

[PATCH 5/5] amdgpu: use drm sync objects for shared semaphores (v5)

2017-05-29 Thread Dave Airlie 
From: Dave Airlie 

This creates a new command submission chunk for amdgpu
to add in and out sync objects around the submission.

Sync objects are managed via the drm syncobj ioctls.

The command submission interface is enhanced with two new
chunks, one for syncobj pre submission dependencies,
and one for post submission sync obj signalling,
and just takes a list of handles for each.

This is based on work originally done by David Zhou at AMD,
with input from Christian Konig on what things should look like.

In theory VkFences could be backed with sync objects and
just get passed into the cs as syncobj handles as well.

NOTE: this interface addition needs a version bump to expose
it to userspace.

TODO: update to dep_sync when rebasing onto amdgpu master.
(with this - r-b from Christian)

v1.1: keep file reference on import.
v2: move to using syncobjs
v2.1: change some APIs to just use p pointer.
v3: make more robust against CS failures, we now add the
wait sems but only remove them once the CS job has been
submitted.
v4: rewrite names of API and base on new syncobj code.
v5: move post deps earlier, rename some apis

Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 87 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 +-
 include/uapi/drm/amdgpu_drm.h   |  6 +++
 3 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 30225d7..3cbd547 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "amdgpu.h"
 #include "amdgpu_trace.h"
 
@@ -226,6 +227,8 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void 
*data)
break;
 
case AMDGPU_CHUNK_ID_DEPENDENCIES:
+   case AMDGPU_CHUNK_ID_SYNCOBJ_IN:
+   case AMDGPU_CHUNK_ID_SYNCOBJ_OUT:
break;
 
default:
@@ -1040,6 +1043,40 @@ static int amdgpu_cs_process_fence_dep(struct 
amdgpu_cs_parser *p,
return 0;
 }
 
+static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p,
+uint32_t handle)
+{
+   int r;
+   struct dma_fence *fence;
+   r = drm_syncobj_fence_get(p->filp, handle, );
+   if (r)
+   return r;
+
+   r = amdgpu_sync_fence(p->adev, >job->sync, fence);
+   dma_fence_put(fence);
+
+   return r;
+}
+
+static int amdgpu_cs_process_syncobj_in_dep(struct amdgpu_cs_parser *p,
+   struct amdgpu_cs_chunk *chunk)
+{
+   unsigned num_deps;
+   int i, r;
+   struct drm_amdgpu_cs_chunk_sem *deps;
+
+   deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata;
+   num_deps = chunk->length_dw * 4 /
+   sizeof(struct drm_amdgpu_cs_chunk_sem);
+
+   for (i = 0; i < num_deps; ++i) {
+   r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle);
+   if (r)
+   return r;
+   }
+   return 0;
+}
+
 static int amdgpu_cs_dependencies(struct amdgpu_device *adev,
  struct amdgpu_cs_parser *p)
 {
@@ -1054,12 +1091,54 @@ static int amdgpu_cs_dependencies(struct amdgpu_device 
*adev,
r = amdgpu_cs_process_fence_dep(p, chunk);
if (r)
return r;
+   } else if (chunk->chunk_id == AMDGPU_CHUNK_ID_SYNCOBJ_IN) {
+   r = amdgpu_cs_process_syncobj_in_dep(p, chunk);
+   if (r)
+   return r;
}
}
 
return 0;
 }
 
+static int amdgpu_cs_process_syncobj_out_dep(struct amdgpu_cs_parser *p,
+struct amdgpu_cs_chunk *chunk)
+{
+   unsigned num_deps;
+   int i, r;
+   struct drm_amdgpu_cs_chunk_sem *deps;
+
+   deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata;
+   num_deps = chunk->length_dw * 4 /
+   sizeof(struct drm_amdgpu_cs_chunk_sem);
+
+   for (i = 0; i < num_deps; ++i) {
+   r = drm_syncobj_replace_fence(p->filp, deps[i].handle,
+ p->fence);
+   if (r)
+   return r;
+   }
+   return 0;
+}
+
+static int amdgpu_cs_post_dependencies(struct amdgpu_cs_parser *p)
+{
+   int i, r;
+
+   for (i = 0; i < p->nchunks; ++i) {
+   struct amdgpu_cs_chunk *chunk;
+
+   chunk = >chunks[i];
+
+   if (chunk->chunk_id == AMDGPU_CHUNK_ID_SYNCOBJ_OUT) {
+   r = amdgpu_cs_process_syncobj_out_dep(p, chunk);
+   if (r)
+   return r;
+   }
+   }
+

[PATCH 4/5] amdgpu/cs: split out fence dependency checking

2017-05-29 Thread Dave Airlie 
From: Dave Airlie 

This just splits out the fence depenency checking into it's
own function to make it easier to add semaphore dependencies.

Reviewed-by: Christian König 
Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 85 +++---
 1 file changed, 47 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 4e6b950..30225d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -995,56 +995,65 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
return 0;
 }
 
-static int amdgpu_cs_dependencies(struct amdgpu_device *adev,
- struct amdgpu_cs_parser *p)
+static int amdgpu_cs_process_fence_dep(struct amdgpu_cs_parser *p,
+  struct amdgpu_cs_chunk *chunk)
 {
struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
-   int i, j, r;
-
-   for (i = 0; i < p->nchunks; ++i) {
-   struct drm_amdgpu_cs_chunk_dep *deps;
-   struct amdgpu_cs_chunk *chunk;
-   unsigned num_deps;
+   unsigned num_deps;
+   int i, r;
+   struct drm_amdgpu_cs_chunk_dep *deps;
 
-   chunk = >chunks[i];
+   deps = (struct drm_amdgpu_cs_chunk_dep *)chunk->kdata;
+   num_deps = chunk->length_dw * 4 /
+   sizeof(struct drm_amdgpu_cs_chunk_dep);
 
-   if (chunk->chunk_id != AMDGPU_CHUNK_ID_DEPENDENCIES)
-   continue;
+   for (i = 0; i < num_deps; ++i) {
+   struct amdgpu_ring *ring;
+   struct amdgpu_ctx *ctx;
+   struct dma_fence *fence;
 
-   deps = (struct drm_amdgpu_cs_chunk_dep *)chunk->kdata;
-   num_deps = chunk->length_dw * 4 /
-   sizeof(struct drm_amdgpu_cs_chunk_dep);
+   r = amdgpu_cs_get_ring(p->adev, deps[i].ip_type,
+  deps[i].ip_instance,
+  deps[i].ring, );
+   if (r)
+   return r;
 
-   for (j = 0; j < num_deps; ++j) {
-   struct amdgpu_ring *ring;
-   struct amdgpu_ctx *ctx;
-   struct dma_fence *fence;
+   ctx = amdgpu_ctx_get(fpriv, deps[i].ctx_id);
+   if (ctx == NULL)
+   return -EINVAL;
 
-   r = amdgpu_cs_get_ring(adev, deps[j].ip_type,
-  deps[j].ip_instance,
-  deps[j].ring, );
+   fence = amdgpu_ctx_get_fence(ctx, ring,
+deps[i].handle);
+   if (IS_ERR(fence)) {
+   r = PTR_ERR(fence);
+   amdgpu_ctx_put(ctx);
+   return r;
+   } else if (fence) {
+   r = amdgpu_sync_fence(p->adev, >job->sync,
+ fence);
+   dma_fence_put(fence);
+   amdgpu_ctx_put(ctx);
if (r)
return r;
+   }
+   }
+   return 0;
+}
 
-   ctx = amdgpu_ctx_get(fpriv, deps[j].ctx_id);
-   if (ctx == NULL)
-   return -EINVAL;
+static int amdgpu_cs_dependencies(struct amdgpu_device *adev,
+ struct amdgpu_cs_parser *p)
+{
+   int i, r;
 
-   fence = amdgpu_ctx_get_fence(ctx, ring,
-deps[j].handle);
-   if (IS_ERR(fence)) {
-   r = PTR_ERR(fence);
-   amdgpu_ctx_put(ctx);
-   return r;
+   for (i = 0; i < p->nchunks; ++i) {
+   struct amdgpu_cs_chunk *chunk;
 
-   } else if (fence) {
-   r = amdgpu_sync_fence(adev, >job->sync,
- fence);
-   dma_fence_put(fence);
-   amdgpu_ctx_put(ctx);
-   if (r)
-   return r;
-   }
+   chunk = >chunks[i];
+
+   if (chunk->chunk_id == AMDGPU_CHUNK_ID_DEPENDENCIES) {
+   r = amdgpu_cs_process_fence_dep(p, chunk);
+   if (r)
+   return r;
}
}
 
-- 
2.9.4

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

[PATCH 1/5] drm: introduce sync objects (v3)

2017-05-29 Thread Dave Airlie 
From: Dave Airlie 

Sync objects are new toplevel drm object, that contain a
pointer to a fence. This fence can be updated via command
submission ioctls via drivers.

There is also a generic wait obj API modelled on the vulkan
wait API (with code modelled on some amdgpu code).

These objects can be converted to an opaque fd that can be
passes between processes.

v2: rename reference/unreference to put/get (Chris)
fix leaked reference (David Zhou)
drop mutex in favour of cmpxchg (Chris)
v3: cleanups from danvet, rebase on drm_fops rename
check fd_flags is 0 in ioctls.

Reviewed-by: Sean Paul 
Signed-off-by: Dave Airlie 
---
 Documentation/gpu/drm-internals.rst |   3 +
 Documentation/gpu/drm-mm.rst|  12 ++
 drivers/gpu/drm/Makefile|   2 +-
 drivers/gpu/drm/drm_file.c  |   8 +
 drivers/gpu/drm/drm_internal.h  |  13 ++
 drivers/gpu/drm/drm_ioctl.c |  12 ++
 drivers/gpu/drm/drm_syncobj.c   | 377 
 include/drm/drmP.h  |   1 -
 include/drm/drm_drv.h   |   1 +
 include/drm/drm_file.h  |   5 +
 include/drm/drm_syncobj.h   |  87 +
 include/uapi/drm/drm.h  |  24 +++
 12 files changed, 543 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_syncobj.c
 create mode 100644 include/drm/drm_syncobj.h

diff --git a/Documentation/gpu/drm-internals.rst 
b/Documentation/gpu/drm-internals.rst
index babfb61..2b23d78 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -98,6 +98,9 @@ DRIVER_ATOMIC
 implement appropriate obj->atomic_get_property() vfuncs for any
 modeset objects with driver specific properties.
 
+DRIVER_SYNCOBJ
+Driver support drm sync objects.
+
 Major, Minor and Patchlevel
 ~~~
 
diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index 96b9c34..9412798 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -484,3 +484,15 @@ DRM Cache Handling
 
 .. kernel-doc:: drivers/gpu/drm/drm_cache.c
:export:
+
+DRM Sync Objects
+===
+
+.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c
+   :doc: Overview
+
+.. kernel-doc:: include/drm/drm_syncobj.h
+   :export:
+
+.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c
+   :export:
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 59f0f9b..6f42188 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -16,7 +16,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
drm_framebuffer.o drm_connector.o drm_blend.o \
drm_encoder.o drm_mode_object.o drm_property.o \
drm_plane.o drm_color_mgmt.o drm_print.o \
-   drm_dumb_buffers.o drm_mode_config.o
+   drm_dumb_buffers.o drm_mode_config.o drm_syncobj.o
 
 drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
 drm-$(CONFIG_DRM_VM) += drm_vm.o
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 3783b65..a20d6a9 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -229,6 +229,9 @@ static int drm_open_helper(struct file *filp, struct 
drm_minor *minor)
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_open(dev, priv);
 
+   if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+   drm_syncobj_open(priv);
+
if (drm_core_check_feature(dev, DRIVER_PRIME))
drm_prime_init_file_private(>prime);
 
@@ -276,6 +279,8 @@ static int drm_open_helper(struct file *filp, struct 
drm_minor *minor)
 out_prime_destroy:
if (drm_core_check_feature(dev, DRIVER_PRIME))
drm_prime_destroy_file_private(>prime);
+   if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+   drm_syncobj_release(priv);
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_release(dev, priv);
put_pid(priv->pid);
@@ -398,6 +403,9 @@ int drm_release(struct inode *inode, struct file *filp)
drm_property_destroy_user_blobs(dev, file_priv);
}
 
+   if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+   drm_syncobj_release(file_priv);
+
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_release(dev, file_priv);
 
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 3d8e8f8..3fdef2c 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -142,4 +142,17 @@ static inline int drm_debugfs_crtc_crc_add(struct drm_crtc 
*crtc)
 {
return 0;
 }
+
 #endif
+
+/* drm_syncobj.c */
+void drm_syncobj_open(struct drm_file *file_private);
+void drm_syncobj_release(struct drm_file *file_private);
+int drm_syncobj_create_ioctl(struct drm_device *dev, void *data,
+struct drm_file *file_private);
+int

drm-syncobj - mostly wait changes

2017-05-29 Thread Dave Airlie 
The wait patch seemed to get the most discussion last time,
so I've overhauled it.

The others are mostly unchanged.

Dave.

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

Re: [PATCH 2/5] drm/syncobj: add sync obj wait interface. (v3)

2017-05-29 Thread Dave Airlie 
Hopefully addressing most of these in the upcoming set, some comments below.

>> +/**
>> + * drm_timeout_abs_to_jiffies - calculate jiffies timeout from absolute 
>> value
>> + *
>> + * @timeout_ns: timeout in ns
>> + *
>> + * Calculate the timeout in jiffies from an absolute timeout in ns.
>> + */
>> +unsigned long drm_timeout_abs_to_jiffies(uint64_t timeout_ns)
>
> Not in any header or otherwise exported, so static?

Done.
>> + /*  clamp timeout to avoid unsigned-> signed overflow */
>> + if (timeout_jiffies > MAX_SCHEDULE_TIMEOUT )
>> + return MAX_SCHEDULE_TIMEOUT - 1;
>
> This about avoiding the conversion into an infinite timeout, right?
> I think the comment is misleading (certainly doesn't explain
> MAX_SCHEDULE_TIMEOUT-1) and the test should be >= MAX_SCHEDULE_TIMEOUT.

Taken from AMD code, but I see your logic, I've adjust the code and
added the one as requested.

>
>> +
>> + return timeout_jiffies;
>
> Timeouts tend to use timeout_jiffies + 1 to avoid the timer interrupt
> screwing up the calculation, or just generally returning a fraction too
> early.
>
>> +}
>> +
>> +static int drm_syncobj_wait_all_fences(struct drm_device *dev,
>> +struct drm_file *file_private,
>> +struct drm_syncobj_wait *wait,
>> +uint32_t *handles)
>> +{
>> + uint32_t i;
>> + int ret;
>> + unsigned long timeout = drm_timeout_abs_to_jiffies(wait->timeout_ns);
>
> Also note that this doesn't handle timeout = 0 very gracefully with
> multiple fences. (dma_fence_wait_timeout will convert that on return to
> 1). Hmm, by using an absolute timeout we can't pass into timeout=0 to do a
> poll.

I've decdied to make timeout = 0 be poll, I can't see anyone having a realtime
clock of 0 making sense anyways.

There is a fix in flight for dma_fence_wait_timeout doing that, it's
in drm-next now.

>> + );
>> + if (ret)
>> + return ret;
>> +
>> + if (!fence)
>> + continue;
>
> I thought no fence for the syncobj was the *unsignaled* case, and to
> wait upon it was a user error. I second Jason's idea to make the lookup
> and error checking on the fences uniform between WAIT_ALL and WAIT_ANY.
>
>> + ret = dma_fence_wait_timeout(fence, true, timeout);
>> +
>> + dma_fence_put(fence);
>> + if (ret < 0)
>> + return ret;
>> + if (ret == 0)
>> + break;
>> + timeout = ret;
>> + }
>> +
>> + wait->out_timeout_ns = jiffies_to_nsecs(ret);
>> + wait->out_status = (ret > 0);
>> + wait->first_signaled = 0;
>> + return 0;
>> +}
>> +
>> +static int drm_syncobj_wait_any_fence(struct drm_device *dev,
>> +   struct drm_file *file_private,
>> +   struct drm_syncobj_wait *wait,
>> +   uint32_t *handles)
>> +{
>> + unsigned long timeout = drm_timeout_abs_to_jiffies(wait->timeout_ns);
>> + struct dma_fence **array;
>> + uint32_t i;
>> + int ret;
>> + uint32_t first = ~0;
>> +
>> + /* Prepare the fence array */
>> + array = kcalloc(wait->count_handles,
>> + sizeof(struct dma_fence *), GFP_KERNEL);
>> +
>> + if (array == NULL)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < wait->count_handles; i++) {
>> + struct dma_fence *fence;
>> +
>> + ret = drm_syncobj_fence_get(file_private, handles[i],
>> + );
>> + if (ret)
>> + goto err_free_fence_array;
>> + else if (fence)
>> + array[i] = fence;
>> + else { /* NULL, the fence has been already signaled */
>> + ret = 1;
>> + goto out;
>> + }
>> + }
>> +
>> + ret = dma_fence_wait_any_timeout(array, wait->count_handles, true, 
>> timeout,
>> +  );
>> + if (ret < 0)
>> + goto err_free_fence_array;
>> +out:
>> + wait->out_timeout_ns = jiffies_to_nsecs(ret);
>> + wait->out_status = (ret > 0);
>
> Didn't the amdgpu interface report which fence completed first? (I may
> be imagining that.)

Just below your comment down there is first_signaled getting assigned!
>
>> + wait->first_signaled = first;
>> + /* set return value 0 to indicate success */
>> + ret = 0;
>> +
>> +err_free_fence_array:
>> + for (i = 0; i < wait->count_handles; i++)
>> + dma_fence_put(array[i]);
>> + kfree(array);
>> +
>> + return ret;
>> +}
>> +
>> +int
>> +drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
>> +struct drm_file *file_private)
>> +{
>> + struct drm_syncobj_wait *args = data;
>> + uint32_t *handles;