[PATCH 1/3] Revert "drm/amd/powerplay: Enable/Disable NBPSTATE on On/OFF of UVD"

[Guttula, Suresh]

From: Shirish S 

This reverts commit dbd8299c32f6f413f6cfe322fe0308f3cfc577e8.

Reason for revert:
This patch sends  msg PPSMC_MSG_DisableLowMemoryPstate(0x002e)
in wrong of sequence to SMU which is before PPSMC_MSG_UVDPowerON (0x0008).
This leads to SMU failing to service the request as it is
dependent on UVD to be powered ON, since it accesses UVD
registers.

This msg should ideally be sent only when the UVD is about to decode
a 4k video.

Signed-off-by: Shirish S 
Signed-off-by: suresh guttula 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
index fef111d..53cf787 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
@@ -1228,17 +1228,14 @@ static int smu8_dpm_force_dpm_level(struct pp_hwmgr 
*hwmgr,
 
 static int smu8_dpm_powerdown_uvd(struct pp_hwmgr *hwmgr)
 {
-   if (PP_CAP(PHM_PlatformCaps_UVDPowerGating)) {
-   smu8_nbdpm_pstate_enable_disable(hwmgr, true, true);
+   if (PP_CAP(PHM_PlatformCaps_UVDPowerGating))
return smum_send_msg_to_smc(hwmgr, PPSMC_MSG_UVDPowerOFF);
-   }
return 0;
 }
 
 static int smu8_dpm_powerup_uvd(struct pp_hwmgr *hwmgr)
 {
if (PP_CAP(PHM_PlatformCaps_UVDPowerGating)) {
-   smu8_nbdpm_pstate_enable_disable(hwmgr, false, true);
return smum_send_msg_to_smc_with_parameter(
hwmgr,
PPSMC_MSG_UVDPowerON,
-- 
2.7.4

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

[PATCH v3 3/3] drm/amd:Enable/Disable NBPSTATE on On/OFF of UVD

[Guttula, Suresh]

We observe black lines (underflow) on display when playing a
4K video with UVD. On Disabling Low memory P state this issue is
not seen.
In this patch,disabling low memory P state only when video
size >= 4k.
Multiple runs of power measurement shows no impact

Signed-off-by: suresh guttula 
---
v2: Enable/disable low memory pstate logic added to
amdgpu_dpm_enable_uvd() instead of parser function

v3: Fix compilation error of struct pp_instance

 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c  | 13 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  2 ++
 3 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 7235cd0..7431943 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -33,6 +33,8 @@
 #include 
 #include 
 #include 
+#include "hwmgr.h"
+#define WIDTH_4K 3840
 
 static int amdgpu_debugfs_pm_init(struct amdgpu_device *adev);
 
@@ -1956,6 +1958,17 @@ void amdgpu_dpm_enable_uvd(struct amdgpu_device *adev, 
bool enable)
amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_UVD, 
!enable);
mutex_unlock(>pm.mutex);
}
+   /* enable/disable Low Memory PState for UVD (4k videos) */
+   if (adev->asic_type == CHIP_STONEY &&
+   adev->uvd.decode_image_width >= WIDTH_4K) {
+   struct pp_hwmgr *hwmgr =
+   adev->powerplay.pp_handle;
+   if (hwmgr && hwmgr->hwmgr_func &&
+   hwmgr->hwmgr_func->update_nbdpm_pstate)
+   hwmgr->hwmgr_func->update_nbdpm_pstate(hwmgr,
+   !enable,
+   true);
+   }
 }
 
 void amdgpu_dpm_enable_vce(struct amdgpu_device *adev, bool enable)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index 69896f4..4e5d13e4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -692,6 +692,8 @@ static int amdgpu_uvd_cs_msg_decode(struct amdgpu_device 
*adev, uint32_t *msg,
buf_sizes[0x1] = dpb_size;
buf_sizes[0x2] = image_size;
buf_sizes[0x4] = min_ctx_size;
+   /* store image width to adjust nb memory pstate */
+   adev->uvd.decode_image_width = width;
return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
index a3ab1a4..5eb6328 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
@@ -65,6 +65,8 @@ struct amdgpu_uvd {
struct drm_sched_entity entity;
struct delayed_work idle_work;
unsignedharvest_config;
+   /* store image width to adjust nb memory state */
+   unsigneddecode_image_width;
 };
 
 int amdgpu_uvd_sw_init(struct amdgpu_device *adev);
-- 
2.7.4

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

[PATCH v2 2/3] drm/amd/powerplay:add hwmgr callback to update nbpstate on Carrizo

[Guttula, Suresh]

Add hwmgr callback "update_nbdpm_pstate".This will use to access
"cz_nbdpm_pstate_enable_disable" function to enable/disable low
memory pstate.

Signed-off-by: suresh guttula 
---

v2: commit message edited to explain more details
 drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c | 1 +
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h| 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
index 53cf787..553a203 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu8_hwmgr.c
@@ -1992,6 +1992,7 @@ static const struct pp_hwmgr_func smu8_hwmgr_funcs = {
.power_state_set = smu8_set_power_state_tasks,
.dynamic_state_management_disable = smu8_disable_dpm_tasks,
.notify_cac_buffer_info = smu8_notify_cac_buffer_info,
+   .update_nbdpm_pstate = smu8_nbdpm_pstate_enable_disable,
.get_thermal_temperature_range = smu8_get_thermal_temperature_range,
 };
 
diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h 
b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
index 07d180ce..fb0f96f 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
@@ -317,6 +317,9 @@ struct pp_hwmgr_func {
uint32_t mc_addr_low,
uint32_t mc_addr_hi,
uint32_t size);
+   int (*update_nbdpm_pstate)(struct pp_hwmgr *hwmgr,
+   bool enable,
+   bool lock);
int (*get_thermal_temperature_range)(struct pp_hwmgr *hwmgr,
struct PP_TemperatureRange *range);
int (*get_power_profile_mode)(struct pp_hwmgr *hwmgr, char *buf);
-- 
2.7.4

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

RE: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ

[S, Shirish]

Thanks Alex, found that patch.
My patch is no more required.


Regards,
Shirish S

From: Deucher, Alexander 
Sent: Monday, November 26, 2018 7:46 PM
To: S, Shirish ; Li, Sun peng (Leo) ; 
Wentland, Harry 
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ


I thought there was a patch to do this already that got sent out a few weeks 
ago.  Basically limit ST/CZ to modes that do not require a retimer.  Is an 
additional patch needed?



Alex


From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of S, Shirish mailto:shiris...@amd.com>>
Sent: Monday, November 26, 2018 1:36:30 AM
To: Li, Sun peng (Leo); Wentland, Harry
Cc: amd-gfx@lists.freedesktop.org; S, 
Shirish
Subject: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ

[Why]
ST/CZ (dce110) advertises modes such as 4k@60Hz etc.,
that it cannot handle correctly, hence  resulting in
several issues like flickering, black lines/flashes and so on.

[How]
These modes are basically high pixel clock ones, hence
limit the same to be advertised to avoid bad user experiences

Signed-off-by: Shirish S mailto:shiris...@amd.com>>
Suggested-by: Harry Wentland 
mailto:harry.wentl...@amd.com>>
---
 .../gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c| 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
index 1b2fe0d..1b8fe99 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
@@ -1121,6 +1121,16 @@ bool dce110_timing_generator_validate_timing(
 if (!timing)
 return false;

+   /* Limit all modes that have a high pixel clock
+* which seems to be problematic on dce110
+* These include: 4k@60Hz, 1080p@144Hz,1440p@120Hz
+* based on the below formula:
+* refresh rate = pixel clock / (htotal * vtotal)
+*/
+   if (timing->pix_clk_khz > 30)
+   return false;
+
+
 hsync_offset = timing->h_border_right + timing->h_front_porch;
 h_sync_start = timing->h_addressable + hsync_offset;

--
2.7.4

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

Re: [PATCH] drm/amdgpu: add the checking to avoid NULL pointer dereference

[zhoucm1]

Yeah, you need another drm patch as well when you apply my patch. Attached.

-David


On 2018年11月27日 08:40, Sharma, Deepak wrote:


On 11/26/18 1:57 AM, Zhou, David(ChunMing) wrote:



-Original Message-
From: Christian König 
Sent: Monday, November 26, 2018 5:23 PM
To: Sharma, Deepak ; Zhou, David(ChunMing)
; Koenig, Christian ;
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: add the checking to avoid NULL pointer
dereference

Am 26.11.18 um 02:59 schrieb Sharma, Deepak:

在 2018/11/24 2:10, Koenig, Christian 写道:

Am 23.11.18 um 15:10 schrieb Zhou, David(ChunMing):

在 2018/11/23 21:30, Koenig, Christian 写道:

Am 23.11.18 um 14:27 schrieb Zhou, David(ChunMing):

在 2018/11/22 19:25, Christian König 写道:

Am 22.11.18 um 07:56 schrieb Sharma, Deepak:

when returned fence is not valid mostly due to userspace ignored
previous error causes NULL pointer dereference.

Again, this is clearly incorrect. The my other mails on the
earlier patch.

Sorry for I didn't get your history, but looks from the patch
itself, it is still a valid patch, isn't it?

No, the semantic of amdgpu_ctx_get_fence() is that we return NULL
when the fence is already signaled.

So this patch could totally break userspace because it changes the
behavior when we try to sync to an already signaled fence.

Ah, I got your meaning, how about attached patch?

Yeah something like this, but I would just give the
DRM_SYNCOBJ_CREATE_SIGNALED instead.

I mean that's what this flag is good for isn't it?

Yeah, I give a flag initally when creating patch, but as you know, there is a

swich case not be able to use that flag:

    case AMDGPU_FENCE_TO_HANDLE_GET_SYNC_FILE_FD:
    fd = get_unused_fd_flags(O_CLOEXEC);
    if (fd < 0) {
    dma_fence_put(fence);
    return fd;
    }

    sync_file = sync_file_create(fence);
    dma_fence_put(fence);
    if (!sync_file) {
    put_unused_fd(fd);
    return -ENOMEM;
    }

    fd_install(fd, sync_file->file);
    info->out.handle = fd;
    return 0;

So I change to stub fence instead.

Yeah, I've missed that case. Not sure if the sync_file can deal with a NULL
fence.

We should then probably move the stub fence function into
dma_fence_stub.c under drivers/dma-buf to keep the stuff together.

Yes, you wrap it to review first with your stub fence, we can do it separately 
first.

-David

-David

I have not applied this patch.
The issue was trying to address is when amdgpu_cs_ioctl() failed due to

low memory (ENOMEM) but userspace chose to proceed and called
amdgpu_cs_fence_to_handle_ioctl().

In amdgpu_cs_fence_to_handle_ioctl(), fence is null and later causing
NULL pointer dereference, this patch was to avoid that and system panic

But I understand now that its a valid case retuning NULL if fence was already
signaled but need to handle case so avoid kernel panic. Seems David patch
should fix this, I will test it tomorrow.

Mhm, but don't we bail out with an error if we ask for a failed command
submission? If not that sounds like a bug as well.

Christian.


Where do we do that?
I see error
[drm:amdgpu_cs_ioctl] *ERROR* amdgpu_cs_list_validate(validated) failed.
[drm:amdgpu_cs_ioctl] *ERROR* Not enough memory for command submission!
BUG: unable to handle kernel NULL pointer dereference at 0008
Did some more debugging,dma_fence_is_array() is causing NULL pointer
dereference call through sync_file_ioctl.

Also I think changes in David patch cant be applied on
amd-staging-drm-next, which all patches I should take to get it correctly?


-Deepak

Christian.


-David

If that patch was applied then please revert it immediately.

Christian.


-David

If you have already pushed the patch then please revert.

Christian.


Signed-off-by: Deepak Sharma 
---
     drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 ++
     1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 024dfbd87f11..14166cd8a12f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1403,6 +1403,8 @@ static struct dma_fence
*amdgpu_cs_get_fence(struct amdgpu_device *adev,
       fence = amdgpu_ctx_get_fence(ctx, entity, user->seq_no);
     amdgpu_ctx_put(ctx);
+    if(!fence)
+    return ERR_PTR(-EINVAL);
       return fence;
     }

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


Received: from SN1PR12MB0509.namprd12.prod.outlook.com (2603:10b6:a02:a8::31)
 by BY1PR12MB0502.namprd12.prod.outlook.com with HTTPS via
 BYAPR03CA0018.NAMPRD03.PROD.OUTLOOK.COM; Thu, 15 Nov 2018 11:12:57 +
Received: from MWHPR12CA0057.namprd12.prod.outlook.com (2603:10b6:300:103::19)
 by SN1PR12MB0509.namprd12.prod.outlook.com (2a01:111:e400:5866::25) 

Re: [PATCH] drm/amdgpu: disable UVD/VCE for some polaris 12 variants

[Zhang, Jerry(Junwei)]

On 11/26/18 5:28 PM, Christian König wrote:

Am 26.11.18 um 03:38 schrieb Zhang, Jerry(Junwei):

On 11/24/18 3:32 AM, Deucher, Alexander wrote:


Is this required? Are the harvesting fuses incorrect?  If the blocks 
are harvested, we should bail out of the blocks properly during 
init.  Also, please make this more explicit if we still need it.  E.g.,





The harvest fuse is indeed disabling UVD and VCE, as it's a mining card.
Then any command to UVD/VCE causing NULL pointer issue, like amdgpu_test.


In this case we should fix the NULL pointer issue instead. Do you have 
a backtrace for this?


Sorry to miss the detail.
The NULL pointer is caused by UVD is not initialized as it's disabled in 
VBIOS for this kind of card.


When cs submit, it will check ring->funcs->parse_cs in amdgpu_cs_ib_fill().
However, uvd_v6_0_early_init() skip the set ring function, as 
CC_HARVEST_FUSES is set UVD/VCE disabled.

Then the access to UVD/VCE ring's funcs will cause NULL pointer issue.

BTW, Windows driver disables UVD/VCE for it as well.

Regards,
Jerry



Regards,
Christian.



AFAIW, windows also disable UVD and VCE in initialization.


   if ((adev->pdev->device == 0x67df) &&
  (adev->pdev->revision == 0xf7)) {

/* Some polaris12 variants don't support UVD/VCE */

  } else  {

amdgpu_device_ip_block_add(adev, _v6_3_ip_block);

amdgpu_device_ip_block_add(adev, _v3_4_ip_block);

    }




OK, will explicit the process.

Regards,
Jerry


That way if we re-arrange the order later, it will be easier to track.


Alex


*From:* amd-gfx  on behalf of 
Junwei Zhang 

*Sent:* Friday, November 23, 2018 3:32:27 AM
*To:* amd-gfx@lists.freedesktop.org
*Cc:* Zhang, Jerry
*Subject:* [PATCH] drm/amdgpu: disable UVD/VCE for some polaris 12 
variants

Some variants don't support UVD and VCE.

Signed-off-by: Junwei Zhang 
---
 drivers/gpu/drm/amd/amdgpu/vi.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c 
b/drivers/gpu/drm/amd/amdgpu/vi.c

index f3a4cf1f013a..3338b013ded4 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -1660,6 +1660,10 @@ int vi_set_ip_blocks(struct amdgpu_device *adev)
amdgpu_device_ip_block_add(adev, _v11_2_ip_block);
 amdgpu_device_ip_block_add(adev, _v8_0_ip_block);
 amdgpu_device_ip_block_add(adev, _v3_1_ip_block);
+   /* Some polaris12 variants don't support UVD/VCE */
+   if ((adev->pdev->device == 0x67df) &&
+ (adev->pdev->revision == 0xf7))
+   break;
 amdgpu_device_ip_block_add(adev, _v6_3_ip_block);
 amdgpu_device_ip_block_add(adev, _v3_4_ip_block);
 break;
--
2.17.1

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



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




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

Re: [PATCH] drm/amdgpu: add the checking to avoid NULL pointer dereference

[Deepak Sharma]

On 11/26/18 1:57 AM, Zhou, David(ChunMing) wrote:
> 
> 
>> -Original Message-
>> From: Christian König 
>> Sent: Monday, November 26, 2018 5:23 PM
>> To: Sharma, Deepak ; Zhou, David(ChunMing)
>> ; Koenig, Christian ;
>> amd-gfx@lists.freedesktop.org
>> Subject: Re: [PATCH] drm/amdgpu: add the checking to avoid NULL pointer
>> dereference
>>
>> Am 26.11.18 um 02:59 schrieb Sharma, Deepak:
>>>
>>> 在 2018/11/24 2:10, Koenig, Christian 写道:
 Am 23.11.18 um 15:10 schrieb Zhou, David(ChunMing):
> 在 2018/11/23 21:30, Koenig, Christian 写道:
>> Am 23.11.18 um 14:27 schrieb Zhou, David(ChunMing):
>>> 在 2018/11/22 19:25, Christian König 写道:
 Am 22.11.18 um 07:56 schrieb Sharma, Deepak:
> when returned fence is not valid mostly due to userspace ignored
> previous error causes NULL pointer dereference.
 Again, this is clearly incorrect. The my other mails on the
 earlier patch.
>>> Sorry for I didn't get your history, but looks from the patch
>>> itself, it is still a valid patch, isn't it?
>> No, the semantic of amdgpu_ctx_get_fence() is that we return NULL
>> when the fence is already signaled.
>>
>> So this patch could totally break userspace because it changes the
>> behavior when we try to sync to an already signaled fence.
> Ah, I got your meaning, how about attached patch?
 Yeah something like this, but I would just give the
 DRM_SYNCOBJ_CREATE_SIGNALED instead.

 I mean that's what this flag is good for isn't it?
>>> Yeah, I give a flag initally when creating patch, but as you know, there is 
>>> a
>> swich case not be able to use that flag:
>>>
>>>    case AMDGPU_FENCE_TO_HANDLE_GET_SYNC_FILE_FD:
>>>    fd = get_unused_fd_flags(O_CLOEXEC);
>>>    if (fd < 0) {
>>>    dma_fence_put(fence);
>>>    return fd;
>>>    }
>>>
>>>    sync_file = sync_file_create(fence);
>>>    dma_fence_put(fence);
>>>    if (!sync_file) {
>>>    put_unused_fd(fd);
>>>    return -ENOMEM;
>>>    }
>>>
>>>    fd_install(fd, sync_file->file);
>>>    info->out.handle = fd;
>>>    return 0;
>>>
>>> So I change to stub fence instead.
>>
>> Yeah, I've missed that case. Not sure if the sync_file can deal with a NULL
>> fence.
>>
>> We should then probably move the stub fence function into
>> dma_fence_stub.c under drivers/dma-buf to keep the stuff together.
> 
> Yes, you wrap it to review first with your stub fence, we can do it 
> separately first.
> 
> -David
>>
>>>
>>> -David
>>>
>>> I have not applied this patch.
>>> The issue was trying to address is when amdgpu_cs_ioctl() failed due to
>> low memory (ENOMEM) but userspace chose to proceed and called
>> amdgpu_cs_fence_to_handle_ioctl().
>>> In amdgpu_cs_fence_to_handle_ioctl(), fence is null and later causing
>>> NULL pointer dereference, this patch was to avoid that and system panic
>> But I understand now that its a valid case retuning NULL if fence was already
>> signaled but need to handle case so avoid kernel panic. Seems David patch
>> should fix this, I will test it tomorrow.
>>
>> Mhm, but don't we bail out with an error if we ask for a failed command
>> submission? If not that sounds like a bug as well.
>>
>> Christian.
>>
Where do we do that?
I see error
[drm:amdgpu_cs_ioctl] *ERROR* amdgpu_cs_list_validate(validated) failed.
[drm:amdgpu_cs_ioctl] *ERROR* Not enough memory for command submission!
BUG: unable to handle kernel NULL pointer dereference at 0008
Did some more debugging,dma_fence_is_array() is causing NULL pointer 
dereference call through sync_file_ioctl.

Also I think changes in David patch cant be applied on 
amd-staging-drm-next, which all patches I should take to get it correctly?

>>>
>>> -Deepak
 Christian.

> -David
>> If that patch was applied then please revert it immediately.
>>
>> Christian.
>>
>>> -David
 If you have already pushed the patch then please revert.

 Christian.

> Signed-off-by: Deepak Sharma 
> ---
>     drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 ++
>     1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 024dfbd87f11..14166cd8a12f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1403,6 +1403,8 @@ static struct dma_fence
> *amdgpu_cs_get_fence(struct amdgpu_device *adev,
>       fence = amdgpu_ctx_get_fence(ctx, entity, user->seq_no);
>     amdgpu_ctx_put(ctx);
> +    if(!fence)
> +    return ERR_PTR(-EINVAL);
>       return fence;
>     }
>>> ___
>>> 

[PATCH 3/8] drm/amdgpu: Vega10 doorbell index initialization

[Oak Zeng]

v2: Use enum definition instead of hardcoded value
v3: Remove unused enum definition

Change-Id: Ib72058337f0aa53adfc6c6aae5341a7cd665111a
Signed-off-by: Oak Zeng 
Suggested-by: Felix Kuehling 
Suggested-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h  | 14 --
 drivers/gpu/drm/amd/amdgpu/soc15.c   |  1 +
 drivers/gpu/drm/amd/amdgpu/soc15.h   |  1 +
 drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c | 28 
 4 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 686652d..ea4dbcf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -434,20 +434,14 @@ typedef enum _AMDGPU_DOORBELL64_ASSIGNMENT
 * default non-graphics QWORD index is 0xe0 - 0xFF inclusive
 */
 
-   /* sDMA engines  reserved from 0xe0 -0xef  */
-   AMDGPU_DOORBELL64_sDMA_ENGINE0= 0xE0,
-   AMDGPU_DOORBELL64_sDMA_HI_PRI_ENGINE0 = 0xE1,
-   AMDGPU_DOORBELL64_sDMA_ENGINE1= 0xE8,
-   AMDGPU_DOORBELL64_sDMA_HI_PRI_ENGINE1 = 0xE9,
-
/* For vega10 sriov, the sdma doorbell must be fixed as follow
 * to keep the same setting with host driver, or it will
 * happen conflicts
 */
-   AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0= 0xF0,
-   AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 = 0xF1,
-   AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1= 0xF2,
-   AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 = 0xF3,
+   AMDGPU_DOORBELL64_sDMA_ENGINE0= 0xF0,
+   AMDGPU_DOORBELL64_sDMA_HI_PRI_ENGINE0 = 0xF1,
+   AMDGPU_DOORBELL64_sDMA_ENGINE1= 0xF2,
+   AMDGPU_DOORBELL64_sDMA_HI_PRI_ENGINE1 = 0xF3,
 
/* Interrupt handler */
AMDGPU_DOORBELL64_IH  = 0xF4,  /* For legacy 
interrupt ring buffer */
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index 4cc0dcb..cae25dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -613,6 +613,7 @@ static const struct amdgpu_asic_funcs soc15_asic_funcs =
.flush_hdp = _flush_hdp,
.invalidate_hdp = _invalidate_hdp,
.need_full_reset = _need_full_reset,
+   .init_doorbell_index = _doorbell_index_init,
 };
 
 static int soc15_common_early_init(void *handle)
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.h 
b/drivers/gpu/drm/amd/amdgpu/soc15.h
index f8ad780..d37c57d 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.h
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.h
@@ -58,4 +58,5 @@ void soc15_program_register_sequence(struct amdgpu_device 
*adev,
 int vega10_reg_base_init(struct amdgpu_device *adev);
 int vega20_reg_base_init(struct amdgpu_device *adev);
 
+void vega10_doorbell_index_init(struct amdgpu_device *adev);
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c 
b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
index c5c9b2b..422674b 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
@@ -56,4 +56,32 @@ int vega10_reg_base_init(struct amdgpu_device *adev)
return 0;
 }
 
+void vega10_doorbell_index_init(struct amdgpu_device *adev)
+{
+   adev->doorbell_index.kiq = AMDGPU_DOORBELL64_KIQ;
+   adev->doorbell_index.mec_ring0 = AMDGPU_DOORBELL64_MEC_RING0;
+   adev->doorbell_index.mec_ring1 = AMDGPU_DOORBELL64_MEC_RING1;
+   adev->doorbell_index.mec_ring2 = AMDGPU_DOORBELL64_MEC_RING2;
+   adev->doorbell_index.mec_ring3 = AMDGPU_DOORBELL64_MEC_RING3;
+   adev->doorbell_index.mec_ring4 = AMDGPU_DOORBELL64_MEC_RING4;
+   adev->doorbell_index.mec_ring5 = AMDGPU_DOORBELL64_MEC_RING5;
+   adev->doorbell_index.mec_ring6 = AMDGPU_DOORBELL64_MEC_RING6;
+   adev->doorbell_index.mec_ring7 = AMDGPU_DOORBELL64_MEC_RING7;
+   adev->doorbell_index.userqueue_start = 
AMDGPU_DOORBELL64_USERQUEUE_START;
+   adev->doorbell_index.userqueue_end = AMDGPU_DOORBELL64_USERQUEUE_END;
+   adev->doorbell_index.gfx_ring0 = AMDGPU_DOORBELL64_GFX_RING0;
+   adev->doorbell_index.sdma_engine0 = AMDGPU_DOORBELL64_sDMA_ENGINE0;
+   adev->doorbell_index.sdma_engine1 = AMDGPU_DOORBELL64_sDMA_ENGINE1;
+   adev->doorbell_index.ih = AMDGPU_DOORBELL64_IH;
+   adev->doorbell_index.uvd_vce.uvd_ring0_1 = 
AMDGPU_DOORBELL64_UVD_RING0_1;
+   adev->doorbell_index.uvd_vce.uvd_ring2_3 = 
AMDGPU_DOORBELL64_UVD_RING2_3;
+   adev->doorbell_index.uvd_vce.uvd_ring4_5 = 
AMDGPU_DOORBELL64_UVD_RING4_5;
+   adev->doorbell_index.uvd_vce.uvd_ring6_7 = 
AMDGPU_DOORBELL64_UVD_RING6_7;
+   adev->doorbell_index.uvd_vce.vce_ring0_1 = 
AMDGPU_DOORBELL64_VCE_RING0_1;
+   adev->doorbell_index.uvd_vce.vce_ring2_3 = 
AMDGPU_DOORBELL64_VCE_RING2_3;
+   adev->doorbell_index.uvd_vce.vce_ring4_5 = 
AMDGPU_DOORBELL64_VCE_RING4_5;
+   

Re: [PATCH] pci: fix incorrect value returned from pcie_get_speed_cap

[Bjorn Helgaas]

On Mon, Nov 19, 2018 at 06:47:04PM -0600, Bjorn Helgaas wrote:
> On Tue, Oct 30, 2018 at 12:36:08PM -0400, Mikulas Patocka wrote:
> > The macros PCI_EXP_LNKCAP_SLS_*GB are values, not bit masks. We must mask
> > the register and compare it against them.
> > 
> > This patch fixes errors "amdgpu: [powerplay] failed to send message 261
> > ret is 0" errors when PCIe-v3 card is plugged into PCIe-v1 slot, because
> > the slot is being incorrectly reported as PCIe-v3 capable.
> > 
> > Signed-off-by: Mikulas Patocka 
> > Fixes: 6cf57be0f78e ("PCI: Add pcie_get_speed_cap() to find max supported 
> > link speed")
> > Cc: sta...@vger.kernel.org  # v4.17+
> > 
> > ---
> >  drivers/pci/pci.c |8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > Index: linux-4.19/drivers/pci/pci.c
> > ===
> > --- linux-4.19.orig/drivers/pci/pci.c   2018-10-30 16:58:58.0 
> > +0100
> > +++ linux-4.19/drivers/pci/pci.c2018-10-30 16:58:58.0 +0100
> > @@ -5492,13 +5492,13 @@ enum pci_bus_speed pcie_get_speed_cap(st
> >  
> > pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, );
> > if (lnkcap) {
> > -   if (lnkcap & PCI_EXP_LNKCAP_SLS_16_0GB)
> > +   if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_16_0GB)
> > return PCIE_SPEED_16_0GT;
> > -   else if (lnkcap & PCI_EXP_LNKCAP_SLS_8_0GB)
> > +   else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == 
> > PCI_EXP_LNKCAP_SLS_8_0GB)
> > return PCIE_SPEED_8_0GT;
> > -   else if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB)
> > +   else if ((lnkcap & PCI_EXP_LNKCAP_SLS) 
> > ==PCI_EXP_LNKCAP_SLS_5_0GB)
> > return PCIE_SPEED_5_0GT;
> > -   else if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB)
> > +   else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == 
> > PCI_EXP_LNKCAP_SLS_2_5GB)
> > return PCIE_SPEED_2_5GT;
> > }

> We also need similar fixes in pci_set_bus_speed(), pcie_speeds()
> (hfi1), cobalt_pcie_status_show(), hba_ioctl_callback(),
> qla24xx_pci_info_str(), and maybe a couple other places.

Does anybody want to volunteer to fix the places above as well?  I
found them by grepping for PCI_EXP_LNKCAP, and they're all broken in
ways similar to pcie_get_speed_cap().  Possibly some of these places
could use pcie_get_speed_cap() directly.

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

Re: [PATCH] pci: fix incorrect value returned from pcie_get_speed_cap

[Bjorn Helgaas]

On Tue, Oct 30, 2018 at 12:36:08PM -0400, Mikulas Patocka wrote:
> The macros PCI_EXP_LNKCAP_SLS_*GB are values, not bit masks. We must mask
> the register and compare it against them.
> 
> This patch fixes errors "amdgpu: [powerplay] failed to send message 261
> ret is 0" errors when PCIe-v3 card is plugged into PCIe-v1 slot, because
> the slot is being incorrectly reported as PCIe-v3 capable.
> 
> Signed-off-by: Mikulas Patocka 

Applied the patch below to for-linus for v4.20 and backport to v4.17+
stable kernels, thanks!

I removed the "if (lnkcap)" test because the LNKCAP case is not
parallel to the LNKCAP2 case.  LNKCAP2 is optional so we have to test
for lnkcap2 being non-zero.  But LNKCAP is required for all devices.
If the config read of it fails, we should get either 0 or ~0, and
neither one will satisfy the 5.0 or 2.5 checks.

> Fixes: 6cf57be0f78e ("PCI: Add pcie_get_speed_cap() to find max supported 
> link speed")
> Cc: sta...@vger.kernel.org# v4.17+
> 
> ---
>  drivers/pci/pci.c |8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> Index: linux-4.19/drivers/pci/pci.c
> ===
> --- linux-4.19.orig/drivers/pci/pci.c 2018-10-30 16:58:58.0 +0100
> +++ linux-4.19/drivers/pci/pci.c  2018-10-30 16:58:58.0 +0100
> @@ -5492,13 +5492,13 @@ enum pci_bus_speed pcie_get_speed_cap(st
>  
>   pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, );
>   if (lnkcap) {
> - if (lnkcap & PCI_EXP_LNKCAP_SLS_16_0GB)
> + if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_16_0GB)
>   return PCIE_SPEED_16_0GT;
> - else if (lnkcap & PCI_EXP_LNKCAP_SLS_8_0GB)
> + else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == 
> PCI_EXP_LNKCAP_SLS_8_0GB)
>   return PCIE_SPEED_8_0GT;
> - else if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB)
> + else if ((lnkcap & PCI_EXP_LNKCAP_SLS) 
> ==PCI_EXP_LNKCAP_SLS_5_0GB)
>   return PCIE_SPEED_5_0GT;
> - else if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB)
> + else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == 
> PCI_EXP_LNKCAP_SLS_2_5GB)
>   return PCIE_SPEED_2_5GT;
>   }
>  

commit 94ea01a6d9a6
Author: Mikulas Patocka 
Date:   Mon Nov 26 10:37:13 2018 -0600

PCI: Fix incorrect value returned from pcie_get_speed_cap()

The macros PCI_EXP_LNKCAP_SLS_*GB are values, not bit masks.  We must mask
the register and compare it against them.

This fixes errors like this:

  amdgpu: [powerplay] failed to send message 261 ret is 0

when PCIe-v3 card is plugged into a PCIe-v1 slot, because the slot is being
incorrectly reported as PCIe-v3 capable.

6cf57be0f78e, which appeared in v4.17, added pcie_get_speed_cap() with the
incorrect test of PCI_EXP_LNKCAP_SLS as a bitmask.  5d9a63304032, which
appeared in v4.19, changed amdgpu to use pcie_get_speed_cap(), so the
amdgpu bug reports below are regressions in v4.19.

Fixes: 6cf57be0f78e ("PCI: Add pcie_get_speed_cap() to find max supported 
link speed")
Fixes: 5d9a63304032 ("drm/amdgpu: use pcie functions for link width and 
speed")
Link: https://bugs.freedesktop.org/show_bug.cgi?id=108704
Link: https://bugs.freedesktop.org/show_bug.cgi?id=108778
Signed-off-by: Mikulas Patocka 
[bhelgaas: update comment, remove use of PCI_EXP_LNKCAP_SLS_8_0GB and
PCI_EXP_LNKCAP_SLS_16_0GB since those should be covered by PCI_EXP_LNKCAP2,
remove test of PCI_EXP_LNKCAP for zero, since that register is required]
Signed-off-by: Bjorn Helgaas 
Acked-by: Alex Deucher 
Cc: sta...@vger.kernel.org  # v4.17+

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d068f11d08a7..c9d8e3c837de 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5556,9 +5556,13 @@ enum pci_bus_speed pcie_get_speed_cap(struct pci_dev 
*dev)
u32 lnkcap2, lnkcap;
 
/*
-* PCIe r4.0 sec 7.5.3.18 recommends using the Supported Link
-* Speeds Vector in Link Capabilities 2 when supported, falling
-* back to Max Link Speed in Link Capabilities otherwise.
+* Link Capabilities 2 was added in PCIe r3.0, sec 7.8.18.  The
+* implementation note there recommends using the Supported Link
+* Speeds Vector in Link Capabilities 2 when supported.
+*
+* Without Link Capabilities 2, i.e., prior to PCIe r3.0, software
+* should use the Supported Link Speeds field in Link Capabilities,
+* where only 2.5 GT/s and 5.0 GT/s speeds were defined.
 */
pcie_capability_read_dword(dev, PCI_EXP_LNKCAP2, );
if (lnkcap2) { /* PCIe r3.0-compliant */
@@ -5574,16 +5578,10 @@ enum pci_bus_speed pcie_get_speed_cap(struct pci_dev 
*dev)
}
 
pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, );
-   if (lnkcap) {
-   if (lnkcap 

[PATCH 2/2] drm/amdgpu: Avoid endless loop in GPUVM fragment processing

[Kuehling, Felix]

Don't bounce back to the root level for fragment processing, because
huge pages are not supported at that level. This is unlikely to happen
with the default VM size on Vega, but can be exposed by limiting the
VM size with the amdgpu.vm_size module parameter.

Signed-off-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index be3e360..0877ff9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1656,9 +1656,11 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
if (!amdgpu_vm_pt_descendant(adev, ))
return -ENOENT;
continue;
-   } else if (frag >= parent_shift) {
+   } else if (frag >= parent_shift &&
+  cursor.level - 1 != adev->vm_manager.root_level) {
/* If the fragment size is even larger than the parent
-* shift we should go up one level and check it again.
+* shift we should go up one level and check it again
+* unless one level up is the root level.
 */
if (!amdgpu_vm_pt_ancestor())
return -ENOENT;
-- 
2.7.4

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

[PATCH 1/2] drm/amdgpu: Cast to uint64_t before left shift

[Kuehling, Felix]

Avoid potential integer overflows with left shift in huge-page mapping
code by casting the operand to uin64_t first.

Signed-off-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index dad0e23..be3e360 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -181,7 +181,7 @@ static unsigned amdgpu_vm_num_entries(struct amdgpu_device 
*adev,
 
if (level == adev->vm_manager.root_level)
/* For the root directory */
-   return round_up(adev->vm_manager.max_pfn, 1 << shift) >> shift;
+   return round_up(adev->vm_manager.max_pfn, 1ULL << shift) >> 
shift;
else if (level != AMDGPU_VM_PTB)
/* Everything in between */
return 512;
@@ -1666,10 +1666,10 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
}
 
/* Looks good so far, calculate parameters for the update */
-   incr = AMDGPU_GPU_PAGE_SIZE << shift;
+   incr = (uint64_t)AMDGPU_GPU_PAGE_SIZE << shift;
mask = amdgpu_vm_entries_mask(adev, cursor.level);
pe_start = ((cursor.pfn >> shift) & mask) * 8;
-   entry_end = (mask + 1) << shift;
+   entry_end = (uint64_t)(mask + 1) << shift;
entry_end += cursor.pfn & ~(entry_end - 1);
entry_end = min(entry_end, end);
 
@@ -1682,7 +1682,7 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
  flags | AMDGPU_PTE_FRAG(frag));
 
pe_start += nptes * 8;
-   dst += nptes * AMDGPU_GPU_PAGE_SIZE << shift;
+   dst += (uint64_t)nptes * AMDGPU_GPU_PAGE_SIZE << shift;
 
frag_start = upd_end;
if (frag_start >= frag_end) {
-- 
2.7.4

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

Re: [PATCH v7 3/5] drm: Document variable refresh properties

[Wentland, Harry]

On 2018-11-12 12:05 p.m., Kazlauskas, Nicholas wrote:
> On 11/12/18 11:12 AM, Wentland, Harry wrote:
>> On 2018-11-08 9:43 a.m., Nicholas Kazlauskas wrote:
>>> These include the drm_connector 'vrr_capable' and the drm_crtc
>>> 'vrr_enabled' properties.
>>>
>>> Signed-off-by: Nicholas Kazlauskas 
>>> Cc: Harry Wentland 
>>> Cc: Manasi Navare 
>>> Cc: Pekka Paalanen 
>>> Cc: Ville Syrjälä 
>>> Cc: Michel Dänzer 
>>
>> Looks good. Whole series is
>> Reviewed-by: Harry Wentland 
>>
>> How are we with the userspace patches? We should probably hold off pushing 
>> the kernel patches until the userspace work is all reviewed.
>>
>> Harry
> 
> Thanks for the review.
> 
> The xf86-video-amdgpu patches have been reviewed and the mesa patches 
> have been partially reviewed.
> 

Alex, Michel,

how do we merge changes like this that provide a kernel API that goes together 
with userspace changes?

I imagine we'd want to get the kernel changes in first, and then merge the 
xf86-video-amdgpu and mesa changes. Please correct me if I'm wrong. Any 
objections to getting this merged via the usual amd-stg?

Thanks,
Harry


> Nicholas Kazlauskas
> 
>>
>>> ---
>>>   Documentation/gpu/drm-kms.rst   |  7 
>>>   drivers/gpu/drm/drm_connector.c | 68 +
>>>   2 files changed, 75 insertions(+)
>>>
>>> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
>>> index 4b1501b4835b..8da2a178cf85 100644
>>> --- a/Documentation/gpu/drm-kms.rst
>>> +++ b/Documentation/gpu/drm-kms.rst
>>> @@ -575,6 +575,13 @@ Explicit Fencing Properties
>>>   .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c
>>>  :doc: explicit fencing properties
>>>   
>>> +
>>> +Variable Refresh Properties
>>> +---
>>> +
>>> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
>>> +   :doc: Variable refresh properties
>>> +
>>>   Existing KMS Properties
>>>   ---
>>>   
>>> diff --git a/drivers/gpu/drm/drm_connector.c 
>>> b/drivers/gpu/drm/drm_connector.c
>>> index 49290060ab7b..0e4287461997 100644
>>> --- a/drivers/gpu/drm/drm_connector.c
>>> +++ b/drivers/gpu/drm/drm_connector.c
>>> @@ -1255,6 +1255,74 @@ int drm_mode_create_scaling_mode_property(struct 
>>> drm_device *dev)
>>>   }
>>>   EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
>>>   
>>> +/**
>>> + * DOC: Variable refresh properties
>>> + *
>>> + * Variable refresh rate capable displays can dynamically adjust their
>>> + * refresh rate by extending the duration of their vertical front porch
>>> + * until page flip or timeout occurs. This can reduce or remove stuttering
>>> + * and latency in scenarios where the page flip does not align with the
>>> + * vblank interval.
>>> + *
>>> + * An example scenario would be an application flipping at a constant rate
>>> + * of 48Hz on a 60Hz display. The page flip will frequently miss the vblank
>>> + * interval and the same contents will be displayed twice. This can be
>>> + * observed as stuttering for content with motion.
>>> + *
>>> + * If variable refresh rate was active on a display that supported a
>>> + * variable refresh range from 35Hz to 60Hz no stuttering would be 
>>> observable
>>> + * for the example scenario. The minimum supported variable refresh rate of
>>> + * 35Hz is below the page flip frequency and the vertical front porch can
>>> + * be extended until the page flip occurs. The vblank interval will be
>>> + * directly aligned to the page flip rate.
>>> + *
>>> + * Not all userspace content is suitable for use with variable refresh 
>>> rate.
>>> + * Large and frequent changes in vertical front porch duration may worsen
>>> + * perceived stuttering for input sensitive applications.
>>> + *
>>> + * Panel brightness will also vary with vertical front porch duration. Some
>>> + * panels may have noticeable differences in brightness between the minimum
>>> + * vertical front porch duration and the maximum vertical front porch 
>>> duration.
>>> + * Large and frequent changes in vertical front porch duration may produce
>>> + * observable flickering for such panels.
>>> + *
>>> + * Userspace control for variable refresh rate is supported via properties
>>> + * on the _connector and _crtc objects.
>>> + *
>>> + * "vrr_capable":
>>> + * Optional _connector boolean property that drivers should attach
>>> + * with drm_connector_attach_vrr_capable_property() on connectors that
>>> + * could support variable refresh rates. Drivers should update the
>>> + * property value by calling drm_connector_set_vrr_capable_property().
>>> + *
>>> + * Absence of the property should indicate absence of support.
>>> + *
>>> + * "vrr_enabled":
>>> + * Default _crtc boolean property that notifies the driver that the
>>> + * content on the CRTC is suitable for variable refresh rate presentation.
>>> + * The driver will take this property as a hint to enable variable
>>> + * refresh rate support if the receiver supports it, ie. if the
>>> + * "vrr_capable" property is true on 

Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices

[Kasiviswanathan, Harish]

Thanks Tejun,Eric and Christian for your replies.

We want GPUs resource management to work seamlessly with containers and 
container orchestration. With the Intel / bpf based approach this is not 
possible. 

From your response we gather the following. GPU resources need to be 
abstracted. We will send a new proposal in same vein. Our current thinking is 
to start with a single abstracted resource and build a framework that can be 
expanded to include additional resources. We plan to start with “GPU cores”. We 
believe all GPUs have some concept of cores or compute unit.

Your feedback is highly appreciated.

Best Regards,
Harish



From: amd-gfx  on behalf of Tejun Heo 

Sent: Tuesday, November 20, 2018 5:30 PM
To: Ho, Kenny
Cc: cgro...@vger.kernel.org; intel-...@lists.freedesktop.org; 
y2ke...@gmail.com; amd-gfx@lists.freedesktop.org; 
dri-de...@lists.freedesktop.org
Subject: Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific 
DRM devices
  

Hello,

On Tue, Nov 20, 2018 at 10:21:14PM +, Ho, Kenny wrote:
> By this reply, are you suggesting that vendor specific resources
> will never be acceptable to be managed under cgroup?  Let say a user

I wouldn't say never but whatever which gets included as a cgroup
controller should have clearly defined resource abstractions and the
control schemes around them including support for delegation.  AFAICS,
gpu side still seems to have a long way to go (and it's not clear
whether that's somewhere it will or needs to end up).

> want to have similar functionality as what cgroup is offering but to
> manage vendor specific resources, what would you suggest as a
> solution?  When you say keeping vendor specific resource regulation
> inside drm or specific drivers, do you mean we should replicate the
> cgroup infrastructure there or do you mean either drm or specific
> driver should query existing hierarchy (such as device or perhaps
> cpu) for the process organization information?
> 
> To put the questions in more concrete terms, let say a user wants to
> expose certain part of a gpu to a particular cgroup similar to the
> way selective cpu cores are exposed to a cgroup via cpuset, how
> should we go about enabling such functionality?

Do what the intel driver or bpf is doing?  It's not difficult to hook
into cgroup for identification purposes.

Thanks.

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


amd-gfx Info Page - freedesktop.org
lists.freedesktop.org
To see the collection of prior postings to the list, visit the amd-gfx 
Archives.. Using amd-gfx: To post a message to all the list members, send email 
to amd-gfx@lists.freedesktop.org. You can subscribe to the list, or change your 
existing subscription, in the sections below.

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

Re: [PATCH] drm/amdgpu: fix spelling mistake "Pramater" -> "Parameter"

[Alex Deucher]

On Mon, Nov 26, 2018 at 10:02 AM Colin King  wrote:
>
> From: Colin Ian King 
>
> There is a spelling mistake in the module description text and
> a comment too, fix them. Also line break overly long comment.
>
> Signed-off-by: Colin Ian King 

Applied.  thanks!

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 8de55f7f1a3a..dd03fe77b3ed 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -454,9 +454,10 @@ module_param_named(cntl_sb_buf_per_se, 
> amdgpu_cntl_sb_buf_per_se, int, 0444);
>
>  /**
>   * DOC: param_buf_per_se (int)
> - * Override the size of Off-Chip Pramater Cache per Shader Engine in Byte. 
> The default is 0 (depending on gfx).
> + * Override the size of Off-Chip Parameter Cache per Shader Engine in Byte.
> + * The default is 0 (depending on gfx).
>   */
> -MODULE_PARM_DESC(param_buf_per_se, "the size of Off-Chip Pramater Cache per 
> Shader Engine (default depending on gfx)");
> +MODULE_PARM_DESC(param_buf_per_se, "the size of Off-Chip Parameter Cache per 
> Shader Engine (default depending on gfx)");
>  module_param_named(param_buf_per_se, amdgpu_param_buf_per_se, int, 0444);
>
>  /**
> --
> 2.19.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: Remove duplicate header

[Alex Deucher]

On Fri, Nov 23, 2018 at 2:43 AM Brajeswar Ghosh
 wrote:
>
> Remove dce/dce_mem_input.h which is included more than once
>
> Signed-off-by: Brajeswar Ghosh 

Applied.  thanks!

Alex

> ---
>  drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c 
> b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> index d68f951f9869..c41408c3eaf1 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c
> @@ -40,7 +40,6 @@
>  #include "dce/dce_mem_input.h"
>  #include "dce/dce_link_encoder.h"
>  #include "dce/dce_stream_encoder.h"
> -#include "dce/dce_mem_input.h"
>  #include "dce/dce_ipp.h"
>  #include "dce/dce_transform.h"
>  #include "dce/dce_opp.h"
> --
> 2.17.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm/amdgpu: Refactor GPU reset for XGMI hive case.

[Christian König]

What I mean is - should we get rid of dma_fence_add/remove_callback 
logic in drm_sched_job_timedout and do it for each driver in between


scheduler deactivation  and activation back ?



Yes, exactly. That's the reason why I already have a revert for the 
patch and remove the dance from drm_sched_job_timedout again.


Christian.


Am 26.11.18 um 20:28 schrieb Grodzovsky, Andrey:



Actually, after looking again at drm_sched_job_timedout  from which 
the amdgpu_device_gpu_recover will be called I see that we already 
disconnect all the pending scheduler fences from the HW fence, including
the guilty job. I also see that in drm_sched_job_timedout 
job_list_lock is released before calling sched->ops->timedout_job and 
then required after, so new jobs can slip into ring_mirror_list in 
between.


And also i will end up going over the ring_mirror_list twice, once 
from amdgpu_device_post_asic_reset and later from 
drm_sched_job_timedout - this might cause double fence processing.


Isn't it more correct only do the disconnect from HW fence after the 
schedules have been stopped and connect back before we restart the 
schedulers (as you pointed out here before)


What I mean is - should we get rid of dma_fence_add/remove_callback 
logic in drm_sched_job_timedout and do it for each driver in between


scheduler deactivation  and activation back ?

Andrey


On 11/22/2018 02:56 PM, Grodzovsky, Andrey wrote:

Additional to that I would try improve the pre, middle, post handling
towards checking if we made some progress in between.

In other words we stop all schedulers in the pre handling and
disconnect the scheduler fences from the hardware fence like I did in
patch "drm/sched: fix timeout handling v2".

Then before we do the actual reset in the middle handling we check if
the offending job has completed or at least made some progress in the
meantime.

I understand how to check if the job completed - if it's fence already
signaled, but how do I test if the job made 'at least some progress' ?

Good question. Maybe we can somehow query from the hardware the number
of primitives or pixels processed so far and then compare after a moment?

I will check on this later. In the mean while I will update the code
with the proposed per hive locking and I will add the check if the
guilty job completed before ASIC reset skipping the reset if it's did.

Andrey




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


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

Re: [PATCH 5/5] drm/amdgpu: Refactor GPU reset for XGMI hive case.

[Grodzovsky, Andrey]

Actually, after looking again at drm_sched_job_timedout  from which the 
amdgpu_device_gpu_recover will be called I see that we already disconnect all 
the pending scheduler fences from the HW fence, including
the guilty job. I also see that in drm_sched_job_timedout job_list_lock is 
released before calling sched->ops->timedout_job and then required after, so 
new jobs can slip into ring_mirror_list in between.

And also i will end up going over the ring_mirror_list twice, once from 
amdgpu_device_post_asic_reset and later from drm_sched_job_timedout - this 
might cause double fence processing.

Isn't it more correct only do the disconnect from HW fence after the schedules 
have been stopped and connect back before we restart the schedulers (as you 
pointed out here before)

What I mean is - should we get rid of dma_fence_add/remove_callback logic in 
drm_sched_job_timedout and do it for each driver in between

scheduler deactivation  and activation back ?

Andrey

On 11/22/2018 02:56 PM, Grodzovsky, Andrey wrote:

Additional to that I would try improve the pre, middle, post handling
towards checking if we made some progress in between.

In other words we stop all schedulers in the pre handling and
disconnect the scheduler fences from the hardware fence like I did in
patch "drm/sched: fix timeout handling v2".

Then before we do the actual reset in the middle handling we check if
the offending job has completed or at least made some progress in the
meantime.


I understand how to check if the job completed - if it's fence already
signaled, but how do I test if the job made 'at least some progress' ?


Good question. Maybe we can somehow query from the hardware the number
of primitives or pixels processed so far and then compare after a moment?


I will check on this later. In the mean while I will update the code
with the proposed per hive locking and I will add the check if the
guilty job completed before ASIC reset skipping the reset if it's did.

Andrey



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

Re: [PATCH 4/7] drm/amdgpu: Doorbell index initialization for ASICs before vega10

[Deucher, Alexander]

Correct.  SI does not use doorbells.


Alex


From: amd-gfx  on behalf of Zeng, Oak 

Sent: Monday, November 26, 2018 12:20:10 PM
To: Alex Deucher
Cc: amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH 4/7] drm/amdgpu: Doorbell index initialization for ASICs 
before vega10

Ok, I can change it legacy_doorbell_index_init(). I will keep this function in 
vi.c. The reason I don't want to put it in cik.c is cik.c is only compiled 
optionally.

Another thing I want to know is, my understanding is si doesn't use doorbell, 
does it?

thanks,
Oak

-Original Message-
From: Alex Deucher 
Sent: Sunday, November 25, 2018 1:46 PM
To: Zeng, Oak 
Subject: Fwd: [PATCH 4/7] drm/amdgpu: Doorbell index initialization for ASICs 
before vega10

-- Forwarded message -
From: Alex Deucher 
Date: Wed, Nov 21, 2018 at 5:40 PM
Subject: Re: [PATCH 4/7] drm/amdgpu: Doorbell index initialization for ASICs 
before vega10
To: 
Cc: amd-gfx list , Oak Zeng 


On Wed, Nov 21, 2018 at 5:00 PM Oak Zeng  wrote:
>
> v2: Use enum definition instead of hardcoded number
>
> Change-Id: Id64eb98f5b1c24b51eb2fd5a083086fc3515813d
> Signed-off-by: Oak Zeng 
> Suggested-by: Felix Kuehling 
> Suggested-by: Alex Deucher 

Since this covers cik and vi, maybe call it cik_doorbell_index_init() or 
legacy_doorbell_index_init()?

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/Makefile  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/vi.h  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/vi_reg_init.c | 43
> 
>  3 files changed, 45 insertions(+), 2 deletions(-)  create mode 100644
> drivers/gpu/drm/amd/amdgpu/vi_reg_init.c
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
> b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 1cef9e1..ae29bf5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -63,7 +63,7 @@ amdgpu-$(CONFIG_DRM_AMDGPU_SI)+= si.o gmc_v6_0.o
> gfx_v6_0.o si_ih.o si_dma.o dce
>
>  amdgpu-y += \
> vi.o mxgpu_vi.o nbio_v6_1.o soc15.o emu_soc.o mxgpu_ai.o nbio_v7_0.o 
> vega10_reg_init.o \
> -   vega20_reg_init.o nbio_v7_4.o vega12_reg_init.o
> +   vega20_reg_init.o nbio_v7_4.o vega12_reg_init.o vi_reg_init.o
>
>  # add DF block
>  amdgpu-y += \
> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.h
> b/drivers/gpu/drm/amd/amdgpu/vi.h index 0429fe3..abcb52e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vi.h
> +++ b/drivers/gpu/drm/amd/amdgpu/vi.h
> @@ -29,5 +29,5 @@
>  void vi_srbm_select(struct amdgpu_device *adev,
> u32 me, u32 pipe, u32 queue, u32 vmid);  int
> vi_set_ip_blocks(struct amdgpu_device *adev);
> -
> +void vi_doorbell_index_init(struct amdgpu_device *adev);
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/vi_reg_init.c
> b/drivers/gpu/drm/amd/amdgpu/vi_reg_init.c
> new file mode 100644
> index 000..cdeb88d
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/vi_reg_init.c
> @@ -0,0 +1,43 @@
> +/*
> + * Copyright 2018 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"),
> + * 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.
> + *
> + */
> +
> +#include "amdgpu.h"
> +
> +void vi_doorbell_index_init(struct amdgpu_device *adev) {
> +   adev->doorbell_index.kiq = AMDGPU_DOORBELL_KIQ;
> +   adev->doorbell_index.mec_ring0 = AMDGPU_DOORBELL_MEC_RING0;
> +   adev->doorbell_index.mec_ring1 = AMDGPU_DOORBELL_MEC_RING1;
> +   adev->doorbell_index.mec_ring2 = AMDGPU_DOORBELL_MEC_RING2;
> +   adev->doorbell_index.mec_ring3 = AMDGPU_DOORBELL_MEC_RING3;
> +   adev->doorbell_index.mec_ring4 = AMDGPU_DOORBELL_MEC_RING4;
> +   adev->doorbell_index.mec_ring5 = AMDGPU_DOORBELL_MEC_RING5;
> +   adev->doorbell_index.mec_ring6 = AMDGPU_DOORBELL_MEC_RING6;
> +   adev->doorbell_index.mec_ring7 = AMDGPU_DOORBELL_MEC_RING7;
> +   adev->doorbell_index.gfx_ring0 = AMDGPU_DOORBELL_GFX_RING0;
> +   

[PATCH 7/8] drm/amdgpu: Use asic specific doorbell index instead of macro definition

[Oak Zeng]

Change-Id: I84475efcfb482c474fccb133010abb5df5f4
Signed-off-by: Oak Zeng 
Suggested-by: Felix Kuehling 
Suggested-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 27 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 10 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  |  8 
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 25 +
 drivers/gpu/drm/amd/amdgpu/tonga_ih.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c  |  4 ++--
 drivers/gpu/drm/amd/amdgpu/vce_v4_0.c  |  4 ++--
 drivers/gpu/drm/amd/amdgpu/vega10_ih.c |  2 +-
 12 files changed, 36 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 1c1fed6..d693b804 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -181,25 +181,14 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
 * process in case of 64-bit doorbells so we
 * can use each doorbell assignment twice.
 */
-   if (adev->asic_type == CHIP_VEGA10) {
-   gpu_resources.sdma_doorbell[0][i] =
-   AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 + 
(i >> 1);
-   gpu_resources.sdma_doorbell[0][i+1] =
-   AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 + 
0x200 + (i >> 1);
-   gpu_resources.sdma_doorbell[1][i] =
-   AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 + 
(i >> 1);
-   gpu_resources.sdma_doorbell[1][i+1] =
-   AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 + 
0x200 + (i >> 1);
-   } else {
-   gpu_resources.sdma_doorbell[0][i] =
-   AMDGPU_DOORBELL64_sDMA_ENGINE0 + (i >> 
1);
-   gpu_resources.sdma_doorbell[0][i+1] =
-   AMDGPU_DOORBELL64_sDMA_ENGINE0 + 0x200 
+ (i >> 1);
-   gpu_resources.sdma_doorbell[1][i] =
-   AMDGPU_DOORBELL64_sDMA_ENGINE1 + (i >> 
1);
-   gpu_resources.sdma_doorbell[1][i+1] =
-   AMDGPU_DOORBELL64_sDMA_ENGINE1 + 0x200 
+ (i >> 1);
-   }
+   gpu_resources.sdma_doorbell[0][i] =
+   adev->doorbell_index.sdma_engine0 + (i >> 1);
+   gpu_resources.sdma_doorbell[0][i+1] =
+   adev->doorbell_index.sdma_engine0 + 0x200 + (i 
>> 1);
+   gpu_resources.sdma_doorbell[1][i] =
+   adev->doorbell_index.sdma_engine1 + (i >> 1);
+   gpu_resources.sdma_doorbell[1][i+1] =
+   adev->doorbell_index.sdma_engine1 + 0x200 + (i 
>> 1);
}
/* Doorbells 0x0e0-0ff and 0x2e0-2ff are reserved for
 * SDMA, IH and VCN. So don't use them for the CP.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index a8b1c9c..fdbc2c2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -532,7 +532,7 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device 
*adev)
adev->doorbell.size = pci_resource_len(adev->pdev, 2);
 
adev->doorbell.num_doorbells = min_t(u32, adev->doorbell.size / 
sizeof(u32),
-AMDGPU_DOORBELL_MAX_ASSIGNMENT+1);
+
adev->doorbell_index.max_assignment+1);
if (adev->doorbell.num_doorbells == 0)
return -EINVAL;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 6a70c0b..97a60da 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -250,7 +250,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
ring->adev = NULL;
ring->ring_obj = NULL;
ring->use_doorbell = true;
-   ring->doorbell_index = AMDGPU_DOORBELL_KIQ;
+   ring->doorbell_index = adev->doorbell_index.kiq;
 
r = amdgpu_gfx_kiq_acquire(adev, ring);
if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index f467b9b..3a9fb60 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -4363,7 

[PATCH 3/8] drm/amdgpu: Vega10 doorbell index initialization

[Oak Zeng]

v2: Use enum definition instead of hardcoded value

Change-Id: Ib72058337f0aa53adfc6c6aae5341a7cd665111a
Signed-off-by: Oak Zeng 
Suggested-by: Felix Kuehling 
Suggested-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/soc15.c   |  1 +
 drivers/gpu/drm/amd/amdgpu/soc15.h   |  1 +
 drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c | 28 
 3 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index 4cc0dcb..cae25dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -613,6 +613,7 @@ static const struct amdgpu_asic_funcs soc15_asic_funcs =
.flush_hdp = _flush_hdp,
.invalidate_hdp = _invalidate_hdp,
.need_full_reset = _need_full_reset,
+   .init_doorbell_index = _doorbell_index_init,
 };
 
 static int soc15_common_early_init(void *handle)
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.h 
b/drivers/gpu/drm/amd/amdgpu/soc15.h
index f8ad780..d37c57d 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.h
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.h
@@ -58,4 +58,5 @@ void soc15_program_register_sequence(struct amdgpu_device 
*adev,
 int vega10_reg_base_init(struct amdgpu_device *adev);
 int vega20_reg_base_init(struct amdgpu_device *adev);
 
+void vega10_doorbell_index_init(struct amdgpu_device *adev);
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c 
b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
index c5c9b2b..9ba8e02 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
@@ -56,4 +56,32 @@ int vega10_reg_base_init(struct amdgpu_device *adev)
return 0;
 }
 
+void vega10_doorbell_index_init(struct amdgpu_device *adev)
+{
+   adev->doorbell_index.kiq = AMDGPU_DOORBELL64_KIQ;
+   adev->doorbell_index.mec_ring0 = AMDGPU_DOORBELL64_MEC_RING0;
+   adev->doorbell_index.mec_ring1 = AMDGPU_DOORBELL64_MEC_RING1;
+   adev->doorbell_index.mec_ring2 = AMDGPU_DOORBELL64_MEC_RING2;
+   adev->doorbell_index.mec_ring3 = AMDGPU_DOORBELL64_MEC_RING3;
+   adev->doorbell_index.mec_ring4 = AMDGPU_DOORBELL64_MEC_RING4;
+   adev->doorbell_index.mec_ring5 = AMDGPU_DOORBELL64_MEC_RING5;
+   adev->doorbell_index.mec_ring6 = AMDGPU_DOORBELL64_MEC_RING6;
+   adev->doorbell_index.mec_ring7 = AMDGPU_DOORBELL64_MEC_RING7;
+   adev->doorbell_index.userqueue_start = 
AMDGPU_DOORBELL64_USERQUEUE_START;
+   adev->doorbell_index.userqueue_end = AMDGPU_DOORBELL64_USERQUEUE_END;
+   adev->doorbell_index.gfx_ring0 = AMDGPU_DOORBELL64_GFX_RING0;
+   adev->doorbell_index.sdma_engine0 = 
AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0;
+   adev->doorbell_index.sdma_engine1 = 
AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1;
+   adev->doorbell_index.ih = AMDGPU_DOORBELL64_IH;
+   adev->doorbell_index.uvd_vce.uvd_ring0_1 = 
AMDGPU_DOORBELL64_UVD_RING0_1;
+   adev->doorbell_index.uvd_vce.uvd_ring2_3 = 
AMDGPU_DOORBELL64_UVD_RING2_3;
+   adev->doorbell_index.uvd_vce.uvd_ring4_5 = 
AMDGPU_DOORBELL64_UVD_RING4_5;
+   adev->doorbell_index.uvd_vce.uvd_ring6_7 = 
AMDGPU_DOORBELL64_UVD_RING6_7;
+   adev->doorbell_index.uvd_vce.vce_ring0_1 = 
AMDGPU_DOORBELL64_VCE_RING0_1;
+   adev->doorbell_index.uvd_vce.vce_ring2_3 = 
AMDGPU_DOORBELL64_VCE_RING2_3;
+   adev->doorbell_index.uvd_vce.vce_ring4_5 = 
AMDGPU_DOORBELL64_VCE_RING4_5;
+   adev->doorbell_index.uvd_vce.vce_ring6_7 = 
AMDGPU_DOORBELL64_VCE_RING6_7;
+   /* In unit of dword doorbell */
+   adev->doorbell_index.max_assignment = AMDGPU_DOORBELL64_MAX_ASSIGNMENT 
<< 1;
+}
 
-- 
2.7.4

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

[PATCH 5/8] drm/amdgpu: Doorbell layout for vega20 and future asic

[Oak Zeng]

v2: Use enum definition instead of hardcoded value

Change-Id: I04d22fb717ac50483c0835f160a2e860e344f358
Signed-off-by: Oak Zeng 
Suggested-by: Felix Kuehling 
Suggested-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h  | 50 
 drivers/gpu/drm/amd/amdgpu/soc15.c   | 22 +++-
 drivers/gpu/drm/amd/amdgpu/soc15.h   |  1 +
 drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 33 ++
 4 files changed, 105 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 719da32..2fc5713 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -392,6 +392,56 @@ struct amdgpu_doorbell {
u32 num_doorbells;  /* Number of doorbells actually 
reserved for amdgpu. */
 };
 
+typedef enum _AMDGPU_VEGA20_DOORBELL_ASSIGNMENT
+{
+   /* Compute + GFX: 0~255 */
+   AMDGPU_VEGA20_DOORBELL_KIQ = 0x000,
+   AMDGPU_VEGA20_DOORBELL_HIQ = 0x001,
+   AMDGPU_VEGA20_DOORBELL_DIQ = 0x002,
+   AMDGPU_VEGA20_DOORBELL_MEC_RING0   = 0x003,
+   AMDGPU_VEGA20_DOORBELL_MEC_RING1   = 0x004,
+   AMDGPU_VEGA20_DOORBELL_MEC_RING2   = 0x005,
+   AMDGPU_VEGA20_DOORBELL_MEC_RING3   = 0x006,
+   AMDGPU_VEGA20_DOORBELL_MEC_RING4   = 0x007,
+   AMDGPU_VEGA20_DOORBELL_MEC_RING5   = 0x008,
+   AMDGPU_VEGA20_DOORBELL_MEC_RING6   = 0x009,
+   AMDGPU_VEGA20_DOORBELL_MEC_RING7   = 0x00A,
+   AMDGPU_VEGA20_DOORBELL_USERQUEUE_START = 0x00B,
+   AMDGPU_VEGA20_DOORBELL_USERQUEUE_END   = 0x08A,
+   AMDGPU_VEGA20_DOORBELL_GFX_RING0   = 0x08B,
+   /* SDMA:256~335*/
+   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE0= 0x100,
+   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE1= 0x10A,
+   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE2= 0x114,
+   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE3= 0x11E,
+   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE4= 0x128,
+   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE5= 0x132,
+   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE6= 0x13C,
+   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE7= 0x146,
+   /* IH: 376~391 */
+   AMDGPU_VEGA20_DOORBELL_IH  = 0x178,
+   /* MMSCH: 392~407
+* overlap the doorbell assignment with VCN as they are  mutually 
exclusive
+* VCE engine's doorbell is 32 bit and two VCE ring share one QWORD
+*/
+   AMDGPU_VEGA20_DOORBELL64_VCN0_1  = 0x188, /* lower 32 
bits for VNC0 and upper 32 bits for VNC1 */
+   AMDGPU_VEGA20_DOORBELL64_VCN2_3  = 0x189,
+   AMDGPU_VEGA20_DOORBELL64_VCN4_5  = 0x18A,
+   AMDGPU_VEGA20_DOORBELL64_VCN6_7  = 0x18B,
+
+   AMDGPU_VEGA20_DOORBELL64_UVD_RING0_1 = 0x188,
+   AMDGPU_VEGA20_DOORBELL64_UVD_RING2_3 = 0x189,
+   AMDGPU_VEGA20_DOORBELL64_UVD_RING4_5 = 0x18A,
+   AMDGPU_VEGA20_DOORBELL64_UVD_RING6_7 = 0x18B,
+
+   AMDGPU_VEGA20_DOORBELL64_VCE_RING0_1 = 0x18C,
+   AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3 = 0x18D,
+   AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5 = 0x18E,
+   AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7 = 0x18F,
+   AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT= 0x18F,
+   AMDGPU_VEGA20_DOORBELL_INVALID   = 0x
+} AMDGPU_VEGA20_DOORBELL_ASSIGNMENT;
+
 /*
  * 64bit doorbell, offset are in QWORD, occupy 2KB doorbell space
  */
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
b/drivers/gpu/drm/amd/amdgpu/soc15.c
index cae25dd..83624e1 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
@@ -616,6 +616,23 @@ static const struct amdgpu_asic_funcs soc15_asic_funcs =
.init_doorbell_index = _doorbell_index_init,
 };
 
+static const struct amdgpu_asic_funcs vega20_asic_funcs =
+{
+   .read_disabled_bios = _read_disabled_bios,
+   .read_bios_from_rom = _read_bios_from_rom,
+   .read_register = _read_register,
+   .reset = _asic_reset,
+   .set_vga_state = _vga_set_state,
+   .get_xclk = _get_xclk,
+   .set_uvd_clocks = _set_uvd_clocks,
+   .set_vce_clocks = _set_vce_clocks,
+   .get_config_memsize = _get_config_memsize,
+   .flush_hdp = _flush_hdp,
+   .invalidate_hdp = _invalidate_hdp,
+   .need_full_reset = _need_full_reset,
+   .init_doorbell_index = _doorbell_index_init,
+};
+
 static int soc15_common_early_init(void *handle)
 {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -633,11 +650,11 @@ static int soc15_common_early_init(void *handle)
adev->se_cac_rreg = _se_cac_rreg;

[PATCH 4/8] drm/amdgpu: Doorbell index initialization for ASICs before vega10

[Oak Zeng]

v2: Use enum definition instead of hardcoded number

Change-Id: Id64eb98f5b1c24b51eb2fd5a083086fc3515813d
Signed-off-by: Oak Zeng 
Suggested-by: Felix Kuehling 
Suggested-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/cik.c |  1 +
 drivers/gpu/drm/amd/amdgpu/cik.h |  1 +
 drivers/gpu/drm/amd/amdgpu/vi.c  | 19 +++
 drivers/gpu/drm/amd/amdgpu/vi.h  |  1 +
 4 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/cik.c b/drivers/gpu/drm/amd/amdgpu/cik.c
index f41f5f5..71c50d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik.c
@@ -1755,6 +1755,7 @@ static const struct amdgpu_asic_funcs cik_asic_funcs =
.flush_hdp = _flush_hdp,
.invalidate_hdp = _invalidate_hdp,
.need_full_reset = _need_full_reset,
+   .init_doorbell_index = _doorbell_index_init,
 };
 
 static int cik_common_early_init(void *handle)
diff --git a/drivers/gpu/drm/amd/amdgpu/cik.h b/drivers/gpu/drm/amd/amdgpu/cik.h
index e49c6f1..54c625a 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik.h
+++ b/drivers/gpu/drm/amd/amdgpu/cik.h
@@ -30,4 +30,5 @@ void cik_srbm_select(struct amdgpu_device *adev,
 u32 me, u32 pipe, u32 queue, u32 vmid);
 int cik_set_ip_blocks(struct amdgpu_device *adev);
 
+void legacy_doorbell_index_init(struct amdgpu_device *adev);
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index 07880d3..ff2906c 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -955,6 +955,7 @@ static const struct amdgpu_asic_funcs vi_asic_funcs =
.flush_hdp = _flush_hdp,
.invalidate_hdp = _invalidate_hdp,
.need_full_reset = _need_full_reset,
+   .init_doorbell_index = _doorbell_index_init,
 };
 
 #define CZ_REV_BRISTOL(rev) \
@@ -1712,3 +1713,21 @@ int vi_set_ip_blocks(struct amdgpu_device *adev)
 
return 0;
 }
+
+void legacy_doorbell_index_init(struct amdgpu_device *adev)
+{
+   adev->doorbell_index.kiq = AMDGPU_DOORBELL_KIQ;
+   adev->doorbell_index.mec_ring0 = AMDGPU_DOORBELL_MEC_RING0;
+   adev->doorbell_index.mec_ring1 = AMDGPU_DOORBELL_MEC_RING1;
+   adev->doorbell_index.mec_ring2 = AMDGPU_DOORBELL_MEC_RING2;
+   adev->doorbell_index.mec_ring3 = AMDGPU_DOORBELL_MEC_RING3;
+   adev->doorbell_index.mec_ring4 = AMDGPU_DOORBELL_MEC_RING4;
+   adev->doorbell_index.mec_ring5 = AMDGPU_DOORBELL_MEC_RING5;
+   adev->doorbell_index.mec_ring6 = AMDGPU_DOORBELL_MEC_RING6;
+   adev->doorbell_index.mec_ring7 = AMDGPU_DOORBELL_MEC_RING7;
+   adev->doorbell_index.gfx_ring0 = AMDGPU_DOORBELL_GFX_RING0;
+   adev->doorbell_index.sdma_engine0 = AMDGPU_DOORBELL_sDMA_ENGINE0;
+   adev->doorbell_index.sdma_engine1 = AMDGPU_DOORBELL_sDMA_ENGINE1;
+   adev->doorbell_index.ih = AMDGPU_DOORBELL_IH;
+   adev->doorbell_index.max_assignment = AMDGPU_DOORBELL_MAX_ASSIGNMENT;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/vi.h b/drivers/gpu/drm/amd/amdgpu/vi.h
index 0429fe3..8de0772 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.h
+++ b/drivers/gpu/drm/amd/amdgpu/vi.h
@@ -30,4 +30,5 @@ void vi_srbm_select(struct amdgpu_device *adev,
u32 me, u32 pipe, u32 queue, u32 vmid);
 int vi_set_ip_blocks(struct amdgpu_device *adev);
 
+void legacy_doorbell_index_init(struct amdgpu_device *adev);
 #endif
-- 
2.7.4

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

[PATCH 8/8] drm/amdgpu: Moved doorbell structures to seperate file

[Oak Zeng]

Move doorbell structures, enum definitions and helper functions
from amdgpu.h to amdgpu_doorbell.h. No functional change

Change-Id: I09f7b84869b6d3c688b7a2506ff78d62b3de23f5
Signed-off-by: Oak Zeng 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h  | 227 +---
 drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 249 +++
 2 files changed, 250 insertions(+), 226 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 2fc5713..70342aa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -81,6 +81,7 @@
 #include "amdgpu_job.h"
 #include "amdgpu_bo_list.h"
 #include "amdgpu_gem.h"
+#include "amdgpu_doorbell.h"
 
 #define MAX_GPU_INSTANCE   16
 
@@ -361,173 +362,6 @@ int amdgpu_fence_slab_init(void);
 void amdgpu_fence_slab_fini(void);
 
 /*
- * GPU doorbell structures, functions & helpers
- */
-typedef enum _AMDGPU_DOORBELL_ASSIGNMENT
-{
-   AMDGPU_DOORBELL_KIQ = 0x000,
-   AMDGPU_DOORBELL_HIQ = 0x001,
-   AMDGPU_DOORBELL_DIQ = 0x002,
-   AMDGPU_DOORBELL_MEC_RING0   = 0x010,
-   AMDGPU_DOORBELL_MEC_RING1   = 0x011,
-   AMDGPU_DOORBELL_MEC_RING2   = 0x012,
-   AMDGPU_DOORBELL_MEC_RING3   = 0x013,
-   AMDGPU_DOORBELL_MEC_RING4   = 0x014,
-   AMDGPU_DOORBELL_MEC_RING5   = 0x015,
-   AMDGPU_DOORBELL_MEC_RING6   = 0x016,
-   AMDGPU_DOORBELL_MEC_RING7   = 0x017,
-   AMDGPU_DOORBELL_GFX_RING0   = 0x020,
-   AMDGPU_DOORBELL_sDMA_ENGINE0= 0x1E0,
-   AMDGPU_DOORBELL_sDMA_ENGINE1= 0x1E1,
-   AMDGPU_DOORBELL_IH  = 0x1E8,
-   AMDGPU_DOORBELL_MAX_ASSIGNMENT  = 0x3FF,
-   AMDGPU_DOORBELL_INVALID = 0x
-} AMDGPU_DOORBELL_ASSIGNMENT;
-
-struct amdgpu_doorbell {
-   /* doorbell mmio */
-   resource_size_t base;
-   resource_size_t size;
-   u32 __iomem *ptr;
-   u32 num_doorbells;  /* Number of doorbells actually 
reserved for amdgpu. */
-};
-
-typedef enum _AMDGPU_VEGA20_DOORBELL_ASSIGNMENT
-{
-   /* Compute + GFX: 0~255 */
-   AMDGPU_VEGA20_DOORBELL_KIQ = 0x000,
-   AMDGPU_VEGA20_DOORBELL_HIQ = 0x001,
-   AMDGPU_VEGA20_DOORBELL_DIQ = 0x002,
-   AMDGPU_VEGA20_DOORBELL_MEC_RING0   = 0x003,
-   AMDGPU_VEGA20_DOORBELL_MEC_RING1   = 0x004,
-   AMDGPU_VEGA20_DOORBELL_MEC_RING2   = 0x005,
-   AMDGPU_VEGA20_DOORBELL_MEC_RING3   = 0x006,
-   AMDGPU_VEGA20_DOORBELL_MEC_RING4   = 0x007,
-   AMDGPU_VEGA20_DOORBELL_MEC_RING5   = 0x008,
-   AMDGPU_VEGA20_DOORBELL_MEC_RING6   = 0x009,
-   AMDGPU_VEGA20_DOORBELL_MEC_RING7   = 0x00A,
-   AMDGPU_VEGA20_DOORBELL_USERQUEUE_START = 0x00B,
-   AMDGPU_VEGA20_DOORBELL_USERQUEUE_END   = 0x08A,
-   AMDGPU_VEGA20_DOORBELL_GFX_RING0   = 0x08B,
-   /* SDMA:256~335*/
-   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE0= 0x100,
-   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE1= 0x10A,
-   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE2= 0x114,
-   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE3= 0x11E,
-   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE4= 0x128,
-   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE5= 0x132,
-   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE6= 0x13C,
-   AMDGPU_VEGA20_DOORBELL_sDMA_ENGINE7= 0x146,
-   /* IH: 376~391 */
-   AMDGPU_VEGA20_DOORBELL_IH  = 0x178,
-   /* MMSCH: 392~407
-* overlap the doorbell assignment with VCN as they are  mutually 
exclusive
-* VCE engine's doorbell is 32 bit and two VCE ring share one QWORD
-*/
-   AMDGPU_VEGA20_DOORBELL64_VCN0_1  = 0x188, /* lower 32 
bits for VNC0 and upper 32 bits for VNC1 */
-   AMDGPU_VEGA20_DOORBELL64_VCN2_3  = 0x189,
-   AMDGPU_VEGA20_DOORBELL64_VCN4_5  = 0x18A,
-   AMDGPU_VEGA20_DOORBELL64_VCN6_7  = 0x18B,
-
-   AMDGPU_VEGA20_DOORBELL64_UVD_RING0_1 = 0x188,
-   AMDGPU_VEGA20_DOORBELL64_UVD_RING2_3 = 0x189,
-   AMDGPU_VEGA20_DOORBELL64_UVD_RING4_5 = 0x18A,
-   AMDGPU_VEGA20_DOORBELL64_UVD_RING6_7 = 0x18B,
-
-   AMDGPU_VEGA20_DOORBELL64_VCE_RING0_1 = 0x18C,
-   AMDGPU_VEGA20_DOORBELL64_VCE_RING2_3 = 0x18D,
-   AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5 = 0x18E,
-   AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7 = 0x18F,

[PATCH 6/8] drm/amdgpu: Call doorbell index init on device initialization

[Oak Zeng]

Also call functioin amdgpu_device_doorbell_init after
amdgpu_device_ip_early_init because the former depends
on the later to set up asic-specific init_doorbell_index
function

Change-Id: I2f004bbbe2565035460686f4fc16e86b77a2a9b5
Signed-off-by: Oak Zeng 
Suggested-by: Felix Kuehling 
Suggested-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index cb06e68..a8b1c9c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -513,6 +513,8 @@ void amdgpu_device_pci_config_reset(struct amdgpu_device 
*adev)
  */
 static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)
 {
+   amdgpu_asic_init_doorbell_index(adev);
+
/* No doorbell on SI hardware generation */
if (adev->asic_type < CHIP_BONAIRE) {
adev->doorbell.base = 0;
@@ -2464,9 +2466,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
DRM_INFO("register mmio base: 0x%08X\n", (uint32_t)adev->rmmio_base);
DRM_INFO("register mmio size: %u\n", (unsigned)adev->rmmio_size);
 
-   /* doorbell bar mapping */
-   amdgpu_device_doorbell_init(adev);
-
/* io port mapping */
for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
if (pci_resource_flags(adev->pdev, i) & IORESOURCE_IO) {
@@ -2485,6 +2484,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
if (r)
return r;
 
+   /* doorbell bar mapping and doorbell index init*/
+   amdgpu_device_doorbell_init(adev);
+
/* if we have > 1 VGA cards, then disable the amdgpu VGA resources */
/* this will fail for cards that aren't VGA class devices, just
 * ignore it */
-- 
2.7.4

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

[PATCH 2/8] drm/amdgpu: Add asic func interface to init doorbell index

[Oak Zeng]

Change-Id: I7e8a9c9dfd4f3bd0902679a1771c1a043ece2674
Signed-off-by: Oak Zeng 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index b7ee4ef..719da32 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -654,6 +654,8 @@ struct amdgpu_asic_funcs {
   struct amdgpu_ring *ring);
/* check if the asic needs a full reset of if soft reset will work */
bool (*need_full_reset)(struct amdgpu_device *adev);
+   /* initialize doorbell layout for specific asic*/
+   void (*init_doorbell_index)(struct amdgpu_device *adev);
 };
 
 /*
@@ -1212,6 +1214,7 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
 #define amdgpu_asic_flush_hdp(adev, r) (adev)->asic_funcs->flush_hdp((adev), 
(r))
 #define amdgpu_asic_invalidate_hdp(adev, r) 
(adev)->asic_funcs->invalidate_hdp((adev), (r))
 #define amdgpu_asic_need_full_reset(adev) 
(adev)->asic_funcs->need_full_reset((adev))
+#define amdgpu_asic_init_doorbell_index(adev) 
(adev)->asic_funcs->init_doorbell_index((adev))
 
 /* Common functions */
 bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
-- 
2.7.4

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

amdgpu-drm-next branch compile error

[Zeng, Oak]

Hello all,

When I pull the latest code, I have this compilation error. Anything I can do 
to fix it?

/home/ozeng/git/compute/kernel/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c: In
function 'amdgpu_dpm_enable_uvd':
/home/ozeng/git/compute/kernel/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:1968:21:
error: dereferencing pointer to incomplete type 'struct pp_instance'
hwmgr = pp_handle->hwmgr;
 ^

thanks,
Oak

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

RE: [PATCH 4/7] drm/amdgpu: Doorbell index initialization for ASICs before vega10

[Zeng, Oak]

Ok, I can change it legacy_doorbell_index_init(). I will keep this function in 
vi.c. The reason I don't want to put it in cik.c is cik.c is only compiled 
optionally. 

Another thing I want to know is, my understanding is si doesn't use doorbell, 
does it?

thanks,
Oak

-Original Message-
From: Alex Deucher  
Sent: Sunday, November 25, 2018 1:46 PM
To: Zeng, Oak 
Subject: Fwd: [PATCH 4/7] drm/amdgpu: Doorbell index initialization for ASICs 
before vega10

-- Forwarded message -
From: Alex Deucher 
Date: Wed, Nov 21, 2018 at 5:40 PM
Subject: Re: [PATCH 4/7] drm/amdgpu: Doorbell index initialization for ASICs 
before vega10
To: 
Cc: amd-gfx list , Oak Zeng 


On Wed, Nov 21, 2018 at 5:00 PM Oak Zeng  wrote:
>
> v2: Use enum definition instead of hardcoded number
>
> Change-Id: Id64eb98f5b1c24b51eb2fd5a083086fc3515813d
> Signed-off-by: Oak Zeng 
> Suggested-by: Felix Kuehling 
> Suggested-by: Alex Deucher 

Since this covers cik and vi, maybe call it cik_doorbell_index_init() or 
legacy_doorbell_index_init()?

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/Makefile  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/vi.h  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/vi_reg_init.c | 43 
> 
>  3 files changed, 45 insertions(+), 2 deletions(-)  create mode 100644 
> drivers/gpu/drm/amd/amdgpu/vi_reg_init.c
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
> b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 1cef9e1..ae29bf5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -63,7 +63,7 @@ amdgpu-$(CONFIG_DRM_AMDGPU_SI)+= si.o gmc_v6_0.o 
> gfx_v6_0.o si_ih.o si_dma.o dce
>
>  amdgpu-y += \
> vi.o mxgpu_vi.o nbio_v6_1.o soc15.o emu_soc.o mxgpu_ai.o nbio_v7_0.o 
> vega10_reg_init.o \
> -   vega20_reg_init.o nbio_v7_4.o vega12_reg_init.o
> +   vega20_reg_init.o nbio_v7_4.o vega12_reg_init.o vi_reg_init.o
>
>  # add DF block
>  amdgpu-y += \
> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.h 
> b/drivers/gpu/drm/amd/amdgpu/vi.h index 0429fe3..abcb52e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vi.h
> +++ b/drivers/gpu/drm/amd/amdgpu/vi.h
> @@ -29,5 +29,5 @@
>  void vi_srbm_select(struct amdgpu_device *adev,
> u32 me, u32 pipe, u32 queue, u32 vmid);  int 
> vi_set_ip_blocks(struct amdgpu_device *adev);
> -
> +void vi_doorbell_index_init(struct amdgpu_device *adev);
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/vi_reg_init.c 
> b/drivers/gpu/drm/amd/amdgpu/vi_reg_init.c
> new file mode 100644
> index 000..cdeb88d
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/vi_reg_init.c
> @@ -0,0 +1,43 @@
> +/*
> + * Copyright 2018 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"),
> + * 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.
> + *
> + */
> +
> +#include "amdgpu.h"
> +
> +void vi_doorbell_index_init(struct amdgpu_device *adev) {
> +   adev->doorbell_index.kiq = AMDGPU_DOORBELL_KIQ;
> +   adev->doorbell_index.mec_ring0 = AMDGPU_DOORBELL_MEC_RING0;
> +   adev->doorbell_index.mec_ring1 = AMDGPU_DOORBELL_MEC_RING1;
> +   adev->doorbell_index.mec_ring2 = AMDGPU_DOORBELL_MEC_RING2;
> +   adev->doorbell_index.mec_ring3 = AMDGPU_DOORBELL_MEC_RING3;
> +   adev->doorbell_index.mec_ring4 = AMDGPU_DOORBELL_MEC_RING4;
> +   adev->doorbell_index.mec_ring5 = AMDGPU_DOORBELL_MEC_RING5;
> +   adev->doorbell_index.mec_ring6 = AMDGPU_DOORBELL_MEC_RING6;
> +   adev->doorbell_index.mec_ring7 = AMDGPU_DOORBELL_MEC_RING7;
> +   adev->doorbell_index.gfx_ring0 = AMDGPU_DOORBELL_GFX_RING0;
> +   adev->doorbell_index.sdma_engine0 = AMDGPU_DOORBELL_sDMA_ENGINE0;
> +   adev->doorbell_index.sdma_engine1 = AMDGPU_DOORBELL_sDMA_ENGINE1;
> +   adev->doorbell_index.ih = AMDGPU_DOORBELL_IH;
> +   adev->doorbell_index.max_assignment = 
> +AMDGPU_DOORBELL_MAX_ASSIGNMENT; }
> +
> --
> 

RE: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device initialization

[Zeng, Oak]

That is good idea. Will do it.

thanks,
Oak

From: Deucher, Alexander
Sent: Monday, November 26, 2018 11:02 AM
To: Zeng, Oak ; Alex Deucher 
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization


Maybe it would be better to add this as a callback to struct amdgpu_asic_funcs 
to avoid adding asic specific code to amdgpu_device.c



Alex


From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Deucher, Alexander 
mailto:alexander.deuc...@amd.com>>
Sent: Monday, November 26, 2018 10:59:02 AM
To: Zeng, Oak; Alex Deucher
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization


Ah, ok.  no problem.



Alex


From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 on behalf of Zeng, Oak mailto:oak.z...@amd.com>>
Sent: Monday, November 26, 2018 10:31:04 AM
To: Alex Deucher
Cc: amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization

Yes, that was my first version. But during debug, I found 
amdgpu_device_doorbell_init uses adev->doorbell_index.max_assignment which is 
initialized in *_doorbell_index_init. So I have to call *_doorbell_index_init 
before amdgpu_device_doorbell_init. *_set_ip_blocks() is too late.

In a later version, I moved calling of *_doorbell_index_init to the top of 
amdgpu_device_doorbell_init.

thanks,
Oak

-Original Message-
From: Alex Deucher mailto:alexdeuc...@gmail.com>>
Sent: Sunday, November 25, 2018 1:43 PM
To: Zeng, Oak mailto:oak.z...@amd.com>>
Subject: Fwd: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization

-- Forwarded message -
From: Alex Deucher mailto:alexdeuc...@gmail.com>>
Date: Wed, Nov 21, 2018 at 11:21 AM
Subject: Re: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization
To: mailto:oz...@amd.com>>
Cc: amd-gfx list 
mailto:amd-gfx@lists.freedesktop.org>>, Oak Zeng 
mailto:oak.z...@amd.com>>


On Wed, Nov 21, 2018 at 10:52 AM Oak Zeng mailto:oz...@amd.com>> 
wrote:
>
> Change-Id: I2f004bbbe2565035460686f4fc16e86b77a2a9b5
> Signed-off-by: Oak Zeng mailto:oz...@amd.com>>
> Suggested-by: Felix Kuehling 
> mailto:felix.kuehl...@amd.com>>
> Suggested-by: Alex Deucher 
> mailto:alexander.deuc...@amd.com>>

I think it would be cleaner to add this to the *_set_ip_blocks() functions in 
cik.c, vi.c, and soc15.c so it aligns with the
*_reg_base_init() calls.

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index cb06e68..a942a88 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -513,6 +513,13 @@ void amdgpu_device_pci_config_reset(struct amdgpu_device 
> *adev)
>   */
>  static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)  {
> +   if (adev->asic_type < CHIP_VEGA10)
> +   vi_doorbell_index_init(adev);
> +   else if (adev->asic_type == CHIP_VEGA10)
> +   vega10_doorbell_index_init(adev);
> +   else
> +   vega12_doorbell_index_init(adev);
> +
> /* No doorbell on SI hardware generation */
> if (adev->asic_type < CHIP_BONAIRE) {
> adev->doorbell.base = 0;
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device initialization

[Deucher, Alexander]

Maybe it would be better to add this as a callback to struct amdgpu_asic_funcs 
to avoid adding asic specific code to amdgpu_device.c


Alex


From: amd-gfx  on behalf of Deucher, 
Alexander 
Sent: Monday, November 26, 2018 10:59:02 AM
To: Zeng, Oak; Alex Deucher
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization


Ah, ok.  no problem.


Alex


From: amd-gfx  on behalf of Zeng, Oak 

Sent: Monday, November 26, 2018 10:31:04 AM
To: Alex Deucher
Cc: amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization

Yes, that was my first version. But during debug, I found 
amdgpu_device_doorbell_init uses adev->doorbell_index.max_assignment which is 
initialized in *_doorbell_index_init. So I have to call *_doorbell_index_init 
before amdgpu_device_doorbell_init. *_set_ip_blocks() is too late.

In a later version, I moved calling of *_doorbell_index_init to the top of 
amdgpu_device_doorbell_init.

thanks,
Oak

-Original Message-
From: Alex Deucher 
Sent: Sunday, November 25, 2018 1:43 PM
To: Zeng, Oak 
Subject: Fwd: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization

-- Forwarded message -
From: Alex Deucher 
Date: Wed, Nov 21, 2018 at 11:21 AM
Subject: Re: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization
To: 
Cc: amd-gfx list , Oak Zeng 


On Wed, Nov 21, 2018 at 10:52 AM Oak Zeng  wrote:
>
> Change-Id: I2f004bbbe2565035460686f4fc16e86b77a2a9b5
> Signed-off-by: Oak Zeng 
> Suggested-by: Felix Kuehling 
> Suggested-by: Alex Deucher 

I think it would be cleaner to add this to the *_set_ip_blocks() functions in 
cik.c, vi.c, and soc15.c so it aligns with the
*_reg_base_init() calls.

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index cb06e68..a942a88 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -513,6 +513,13 @@ void amdgpu_device_pci_config_reset(struct amdgpu_device 
> *adev)
>   */
>  static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)  {
> +   if (adev->asic_type < CHIP_VEGA10)
> +   vi_doorbell_index_init(adev);
> +   else if (adev->asic_type == CHIP_VEGA10)
> +   vega10_doorbell_index_init(adev);
> +   else
> +   vega12_doorbell_index_init(adev);
> +
> /* No doorbell on SI hardware generation */
> if (adev->asic_type < CHIP_BONAIRE) {
> adev->doorbell.base = 0;
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device initialization

[Deucher, Alexander]

Ah, ok.  no problem.


Alex


From: amd-gfx  on behalf of Zeng, Oak 

Sent: Monday, November 26, 2018 10:31:04 AM
To: Alex Deucher
Cc: amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization

Yes, that was my first version. But during debug, I found 
amdgpu_device_doorbell_init uses adev->doorbell_index.max_assignment which is 
initialized in *_doorbell_index_init. So I have to call *_doorbell_index_init 
before amdgpu_device_doorbell_init. *_set_ip_blocks() is too late.

In a later version, I moved calling of *_doorbell_index_init to the top of 
amdgpu_device_doorbell_init.

thanks,
Oak

-Original Message-
From: Alex Deucher 
Sent: Sunday, November 25, 2018 1:43 PM
To: Zeng, Oak 
Subject: Fwd: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization

-- Forwarded message -
From: Alex Deucher 
Date: Wed, Nov 21, 2018 at 11:21 AM
Subject: Re: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization
To: 
Cc: amd-gfx list , Oak Zeng 


On Wed, Nov 21, 2018 at 10:52 AM Oak Zeng  wrote:
>
> Change-Id: I2f004bbbe2565035460686f4fc16e86b77a2a9b5
> Signed-off-by: Oak Zeng 
> Suggested-by: Felix Kuehling 
> Suggested-by: Alex Deucher 

I think it would be cleaner to add this to the *_set_ip_blocks() functions in 
cik.c, vi.c, and soc15.c so it aligns with the
*_reg_base_init() calls.

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index cb06e68..a942a88 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -513,6 +513,13 @@ void amdgpu_device_pci_config_reset(struct amdgpu_device 
> *adev)
>   */
>  static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)  {
> +   if (adev->asic_type < CHIP_VEGA10)
> +   vi_doorbell_index_init(adev);
> +   else if (adev->asic_type == CHIP_VEGA10)
> +   vega10_doorbell_index_init(adev);
> +   else
> +   vega12_doorbell_index_init(adev);
> +
> /* No doorbell on SI hardware generation */
> if (adev->asic_type < CHIP_BONAIRE) {
> adev->doorbell.base = 0;
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device initialization

[Zeng, Oak]

Yes, that was my first version. But during debug, I found 
amdgpu_device_doorbell_init uses adev->doorbell_index.max_assignment which is 
initialized in *_doorbell_index_init. So I have to call *_doorbell_index_init 
before amdgpu_device_doorbell_init. *_set_ip_blocks() is too late.

In a later version, I moved calling of *_doorbell_index_init to the top of 
amdgpu_device_doorbell_init.

thanks,
Oak

-Original Message-
From: Alex Deucher  
Sent: Sunday, November 25, 2018 1:43 PM
To: Zeng, Oak 
Subject: Fwd: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization

-- Forwarded message -
From: Alex Deucher 
Date: Wed, Nov 21, 2018 at 11:21 AM
Subject: Re: [PATCH 5/7] drm/amdgpu: Call doorbell index init on device 
initialization
To: 
Cc: amd-gfx list , Oak Zeng 


On Wed, Nov 21, 2018 at 10:52 AM Oak Zeng  wrote:
>
> Change-Id: I2f004bbbe2565035460686f4fc16e86b77a2a9b5
> Signed-off-by: Oak Zeng 
> Suggested-by: Felix Kuehling 
> Suggested-by: Alex Deucher 

I think it would be cleaner to add this to the *_set_ip_blocks() functions in 
cik.c, vi.c, and soc15.c so it aligns with the
*_reg_base_init() calls.

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index cb06e68..a942a88 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -513,6 +513,13 @@ void amdgpu_device_pci_config_reset(struct amdgpu_device 
> *adev)
>   */
>  static int amdgpu_device_doorbell_init(struct amdgpu_device *adev)  {
> +   if (adev->asic_type < CHIP_VEGA10)
> +   vi_doorbell_index_init(adev);
> +   else if (adev->asic_type == CHIP_VEGA10)
> +   vega10_doorbell_index_init(adev);
> +   else
> +   vega12_doorbell_index_init(adev);
> +
> /* No doorbell on SI hardware generation */
> if (adev->asic_type < CHIP_BONAIRE) {
> adev->doorbell.base = 0;
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: fix spelling mistake "Pramater" -> "Parameter"

[Colin King]

From: Colin Ian King 

There is a spelling mistake in the module description text and
a comment too, fix them. Also line break overly long comment.

Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 8de55f7f1a3a..dd03fe77b3ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -454,9 +454,10 @@ module_param_named(cntl_sb_buf_per_se, 
amdgpu_cntl_sb_buf_per_se, int, 0444);
 
 /**
  * DOC: param_buf_per_se (int)
- * Override the size of Off-Chip Pramater Cache per Shader Engine in Byte. The 
default is 0 (depending on gfx).
+ * Override the size of Off-Chip Parameter Cache per Shader Engine in Byte.
+ * The default is 0 (depending on gfx).
  */
-MODULE_PARM_DESC(param_buf_per_se, "the size of Off-Chip Pramater Cache per 
Shader Engine (default depending on gfx)");
+MODULE_PARM_DESC(param_buf_per_se, "the size of Off-Chip Parameter Cache per 
Shader Engine (default depending on gfx)");
 module_param_named(param_buf_per_se, amdgpu_param_buf_per_se, int, 0444);
 
 /**
-- 
2.19.1

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

Re: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ

[Deucher, Alexander]

I thought there was a patch to do this already that got sent out a few weeks 
ago.  Basically limit ST/CZ to modes that do not require a retimer.  Is an 
additional patch needed?


Alex


From: amd-gfx  on behalf of S, Shirish 

Sent: Monday, November 26, 2018 1:36:30 AM
To: Li, Sun peng (Leo); Wentland, Harry
Cc: amd-gfx@lists.freedesktop.org; S, Shirish
Subject: [PATCH] drm/amd/display: limit high pixel clock modes on ST/CZ

[Why]
ST/CZ (dce110) advertises modes such as 4k@60Hz etc.,
that it cannot handle correctly, hence  resulting in
several issues like flickering, black lines/flashes and so on.

[How]
These modes are basically high pixel clock ones, hence
limit the same to be advertised to avoid bad user experiences

Signed-off-by: Shirish S 
Suggested-by: Harry Wentland 
---
 .../gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c| 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
index 1b2fe0d..1b8fe99 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_timing_generator.c
@@ -1121,6 +1121,16 @@ bool dce110_timing_generator_validate_timing(
 if (!timing)
 return false;

+   /* Limit all modes that have a high pixel clock
+* which seems to be problematic on dce110
+* These include: 4k@60Hz, 1080p@144Hz,1440p@120Hz
+* based on the below formula:
+* refresh rate = pixel clock / (htotal * vtotal)
+*/
+   if (timing->pix_clk_khz > 30)
+   return false;
+
+
 hsync_offset = timing->h_border_right + timing->h_front_porch;
 h_sync_start = timing->h_addressable + hsync_offset;

--
2.7.4

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

RE: [PATCH] drm/amdgpu: add the checking to avoid NULL pointer dereference

[Zhou, David(ChunMing)]

> -Original Message-
> From: Christian König 
> Sent: Monday, November 26, 2018 5:23 PM
> To: Sharma, Deepak ; Zhou, David(ChunMing)
> ; Koenig, Christian ;
> amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: add the checking to avoid NULL pointer
> dereference
> 
> Am 26.11.18 um 02:59 schrieb Sharma, Deepak:
> >
> > 在 2018/11/24 2:10, Koenig, Christian 写道:
> >> Am 23.11.18 um 15:10 schrieb Zhou, David(ChunMing):
> >>> 在 2018/11/23 21:30, Koenig, Christian 写道:
>  Am 23.11.18 um 14:27 schrieb Zhou, David(ChunMing):
> > 在 2018/11/22 19:25, Christian König 写道:
> >> Am 22.11.18 um 07:56 schrieb Sharma, Deepak:
> >>> when returned fence is not valid mostly due to userspace ignored
> >>> previous error causes NULL pointer dereference.
> >> Again, this is clearly incorrect. The my other mails on the
> >> earlier patch.
> > Sorry for I didn't get your history, but looks from the patch
> > itself, it is still a valid patch, isn't it?
>  No, the semantic of amdgpu_ctx_get_fence() is that we return NULL
>  when the fence is already signaled.
> 
>  So this patch could totally break userspace because it changes the
>  behavior when we try to sync to an already signaled fence.
> >>> Ah, I got your meaning, how about attached patch?
> >> Yeah something like this, but I would just give the
> >> DRM_SYNCOBJ_CREATE_SIGNALED instead.
> >>
> >> I mean that's what this flag is good for isn't it?
> > Yeah, I give a flag initally when creating patch, but as you know, there is 
> > a
> swich case not be able to use that flag:
> >
> >       case AMDGPU_FENCE_TO_HANDLE_GET_SYNC_FILE_FD:
> >       fd = get_unused_fd_flags(O_CLOEXEC);
> >       if (fd < 0) {
> >       dma_fence_put(fence);
> >       return fd;
> >       }
> >
> >       sync_file = sync_file_create(fence);
> >       dma_fence_put(fence);
> >       if (!sync_file) {
> >       put_unused_fd(fd);
> >       return -ENOMEM;
> >       }
> >
> >       fd_install(fd, sync_file->file);
> >       info->out.handle = fd;
> >       return 0;
> >
> > So I change to stub fence instead.
> 
> Yeah, I've missed that case. Not sure if the sync_file can deal with a NULL
> fence.
> 
> We should then probably move the stub fence function into
> dma_fence_stub.c under drivers/dma-buf to keep the stuff together.

Yes, you wrap it to review first with your stub fence, we can do it separately 
first.

-David 
> 
> >
> > -David
> >
> > I have not applied this patch.
> > The issue was trying to address is when amdgpu_cs_ioctl() failed due to
> low memory (ENOMEM) but userspace chose to proceed and called
> amdgpu_cs_fence_to_handle_ioctl().
> > In amdgpu_cs_fence_to_handle_ioctl(), fence is null and later causing
> > NULL pointer dereference, this patch was to avoid that and system panic
> But I understand now that its a valid case retuning NULL if fence was already
> signaled but need to handle case so avoid kernel panic. Seems David patch
> should fix this, I will test it tomorrow.
> 
> Mhm, but don't we bail out with an error if we ask for a failed command
> submission? If not that sounds like a bug as well.
> 
> Christian.
> 
> >
> > -Deepak
> >> Christian.
> >>
> >>> -David
>  If that patch was applied then please revert it immediately.
> 
>  Christian.
> 
> > -David
> >> If you have already pushed the patch then please revert.
> >>
> >> Christian.
> >>
> >>> Signed-off-by: Deepak Sharma 
> >>> ---
> >>>    drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 ++
> >>>    1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> >>> index 024dfbd87f11..14166cd8a12f 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> >>> @@ -1403,6 +1403,8 @@ static struct dma_fence
> >>> *amdgpu_cs_get_fence(struct amdgpu_device *adev,
> >>>      fence = amdgpu_ctx_get_fence(ctx, entity, user->seq_no);
> >>>    amdgpu_ctx_put(ctx);
> >>> +    if(!fence)
> >>> +    return ERR_PTR(-EINVAL);
> >>>      return fence;
> >>>    }
> > ___
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

Re: [PATCH] drm/amdgpu: disable UVD/VCE for some polaris 12 variants

[Christian König]

Am 26.11.18 um 03:38 schrieb Zhang, Jerry(Junwei):

On 11/24/18 3:32 AM, Deucher, Alexander wrote:


Is this required?  Are the harvesting fuses incorrect?  If the blocks 
are harvested, we should bail out of the blocks properly during init. 
 Also, please make this more explicit if we still need it.  E.g.,





The harvest fuse is indeed disabling UVD and VCE, as it's a mining card.
Then any command to UVD/VCE causing NULL pointer issue, like amdgpu_test.


In this case we should fix the NULL pointer issue instead. Do you have a 
backtrace for this?


Regards,
Christian.



AFAIW, windows also disable UVD and VCE in initialization.


   if ((adev->pdev->device == 0x67df) &&
  (adev->pdev->revision == 0xf7)) {

/* Some polaris12 variants don't support UVD/VCE */

  } else  {

amdgpu_device_ip_block_add(adev, _v6_3_ip_block);

amdgpu_device_ip_block_add(adev, _v3_4_ip_block);

    }




OK, will explicit the process.

Regards,
Jerry


That way if we re-arrange the order later, it will be easier to track.


Alex


*From:* amd-gfx  on behalf of 
Junwei Zhang 

*Sent:* Friday, November 23, 2018 3:32:27 AM
*To:* amd-gfx@lists.freedesktop.org
*Cc:* Zhang, Jerry
*Subject:* [PATCH] drm/amdgpu: disable UVD/VCE for some polaris 12 
variants

Some variants don't support UVD and VCE.

Signed-off-by: Junwei Zhang 
---
 drivers/gpu/drm/amd/amdgpu/vi.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c 
b/drivers/gpu/drm/amd/amdgpu/vi.c

index f3a4cf1f013a..3338b013ded4 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -1660,6 +1660,10 @@ int vi_set_ip_blocks(struct amdgpu_device *adev)
amdgpu_device_ip_block_add(adev, _v11_2_ip_block);
 amdgpu_device_ip_block_add(adev, _v8_0_ip_block);
 amdgpu_device_ip_block_add(adev, _v3_1_ip_block);
+   /* Some polaris12 variants don't support UVD/VCE */
+   if ((adev->pdev->device == 0x67df) &&
+ (adev->pdev->revision == 0xf7))
+   break;
 amdgpu_device_ip_block_add(adev, _v6_3_ip_block);
 amdgpu_device_ip_block_add(adev, _v3_4_ip_block);
 break;
--
2.17.1

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



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


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

Re: [PATCH] drm/amdgpu: add the checking to avoid NULL pointer dereference

[Christian König]

Am 26.11.18 um 02:59 schrieb Sharma, Deepak:


在 2018/11/24 2:10, Koenig, Christian 写道:

Am 23.11.18 um 15:10 schrieb Zhou, David(ChunMing):

在 2018/11/23 21:30, Koenig, Christian 写道:

Am 23.11.18 um 14:27 schrieb Zhou, David(ChunMing):

在 2018/11/22 19:25, Christian König 写道:

Am 22.11.18 um 07:56 schrieb Sharma, Deepak:

when returned fence is not valid mostly due to userspace ignored
previous error causes NULL pointer dereference.

Again, this is clearly incorrect. The my other mails on the
earlier patch.

Sorry for I didn't get your history, but looks from the patch
itself, it is still a valid patch, isn't it?

No, the semantic of amdgpu_ctx_get_fence() is that we return NULL
when the fence is already signaled.

So this patch could totally break userspace because it changes the
behavior when we try to sync to an already signaled fence.

Ah, I got your meaning, how about attached patch?

Yeah something like this, but I would just give the
DRM_SYNCOBJ_CREATE_SIGNALED instead.

I mean that's what this flag is good for isn't it?

Yeah, I give a flag initally when creating patch, but as you know, there is a 
swich case not be able to use that flag:

      case AMDGPU_FENCE_TO_HANDLE_GET_SYNC_FILE_FD:
      fd = get_unused_fd_flags(O_CLOEXEC);
      if (fd < 0) {
      dma_fence_put(fence);
      return fd;
      }

      sync_file = sync_file_create(fence);
      dma_fence_put(fence);
      if (!sync_file) {
      put_unused_fd(fd);
      return -ENOMEM;
      }

      fd_install(fd, sync_file->file);
      info->out.handle = fd;
      return 0;

So I change to stub fence instead.


Yeah, I've missed that case. Not sure if the sync_file can deal with a 
NULL fence.


We should then probably move the stub fence function into 
dma_fence_stub.c under drivers/dma-buf to keep the stuff together.




-David

I have not applied this patch.
The issue was trying to address is when amdgpu_cs_ioctl() failed due to low 
memory (ENOMEM) but userspace chose to proceed and called 
amdgpu_cs_fence_to_handle_ioctl().
In amdgpu_cs_fence_to_handle_ioctl(), fence is null and later causing NULL 
pointer dereference, this patch was to avoid that and system panic
But I understand now that its a valid case retuning NULL if fence was already 
signaled but need to handle case so avoid kernel panic. Seems David patch 
should fix this, I will test it tomorrow.


Mhm, but don't we bail out with an error if we ask for a failed command 
submission? If not that sounds like a bug as well.


Christian.



-Deepak

Christian.


-David

If that patch was applied then please revert it immediately.

Christian.


-David

If you have already pushed the patch then please revert.

Christian.


Signed-off-by: Deepak Sharma 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 ++
   1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 024dfbd87f11..14166cd8a12f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1403,6 +1403,8 @@ static struct dma_fence
*amdgpu_cs_get_fence(struct amdgpu_device *adev,
     fence = amdgpu_ctx_get_fence(ctx, entity, user->seq_no);
   amdgpu_ctx_put(ctx);
+    if(!fence)
+    return ERR_PTR(-EINVAL);
     return fence;
   }

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


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