Re: [PATCH] drm/amdgpu: annotate a false positive locking dependency
The commit headline is misleading. An annotation would be something like
replacing mutex_lock with mutex_lock_nested. You're not annotating
anything, you're actually changing the locking.
Am 2020-08-05 um 9:24 p.m. schrieb Dennis Li:
> [ 264.483189] ==
> [ 264.483487] WARNING: possible circular locking dependency detected
> [ 264.483781] 5.6.0-deli-v5.6-2848-g3f3109b0e75f #1 Tainted: G OE
> [ 264.484076] --
> [ 264.484370] kworker/39:1/567 is trying to acquire lock:
> [ 264.484663] c15df4b0 (mgpu_info.mutex){+.+.}, at:
> amdgpu_unregister_gpu_instance+0x1d/0xc0 [amdgpu]
> [ 264.485081]
>but task is already holding lock:
> [ 264.485670] 965fd31647a0 (>reset_sem){}, at:
> amdgpu_device_gpu_recover+0x264/0x1030 [amdgpu]
> [ 264.486074]
>which lock already depends on the new lock.
[snip]
> Remove the lock(>hive_lock) out of amdgpu_get_xgmi_hive,
> to disable its locking dependency on xgmi_mutex.
Why is this safe? I think the idea with this was, that xgmi_mutex
protects the global xgmi_hives array and protects XGMI hives while they
are being added to or removed from it. Taking the hive_lock before
dropping the xgmi_mutex would ensure that the hive is always protected
by one of those locks.
I think you're opening up a small potential race condition now, where an
XGMI hive can be removed with amdgpu_xgmi_remove_device between
amdgpu_get_xgmi_hive and locking it. So you can't guarantee that the
hive returned by amdgpu_xgmi_remove_device is valid any more.
I think this is not a big deal, because the amdgpu_hive_info structures
are part of the global array and never freed. So there is no risk of
dereferencing an invalid pointer. The only risk is, that you're getting
an amdgpu_hive_info structure for a hive that no longer exists. You
could add a simple check after taking the hive_lock for
hive->number_devices == 0. In amdgpu_xgmi_remove_device you'd want to
avoid decrementing the device counter again in this case and return an
error instead. In amdgpu_xgmi_add_device, you will have to ensure a new
XGMI hive is created by calling amdgpu_xgmi_remove_device again, or just
return an error because someone concurrently removing a device while
adding it at the same time is very unexpected.
Regards,
Felix
>
> Signed-off-by: Dennis Li
> Change-Id: I2d9d80ee23f9f9ac6ce9e1b9e5e1b2b3530f5bdd
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 62ecac97fbd2..6c572db42d92 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2840,7 +2840,7 @@ static void amdgpu_device_xgmi_reset_func(struct
> work_struct *__work)
> {
> struct amdgpu_device *adev =
> container_of(__work, struct amdgpu_device, xgmi_reset_work);
> - struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, 0);
> + struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev);
>
> /* It's a bug to not have a hive within this function */
> if (WARN_ON(!hive))
> @@ -4283,7 +4283,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device
> *adev,
>* We always reset all schedulers for device and all devices for XGMI
>* hive so that should take care of them too.
>*/
> - hive = amdgpu_get_xgmi_hive(adev, false);
> + hive = amdgpu_get_xgmi_hive(adev);
> if (hive) {
> if (atomic_cmpxchg(>in_reset, 0, 1) != 0) {
> DRM_INFO("Bailing on TDR for s_job:%llx, hive: %llx as
> another already in progress",
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index 5680f7eafcb1..5cd42740461c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -1514,7 +1514,7 @@ static void amdgpu_ras_do_recovery(struct work_struct
> *work)
> struct amdgpu_device *remote_adev = NULL;
> struct amdgpu_device *adev = ras->adev;
> struct list_head device_list, *device_list_handle = NULL;
> - struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev, false);
> + struct amdgpu_hive_info *hive = amdgpu_get_xgmi_hive(adev);
>
> /* Build list of devices to query RAS related errors */
> if (hive && adev->gmc.xgmi.num_physical_nodes > 1)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 67a756f4337b..19fd5ce3e857 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -336,7 +336,7 @@ static void amdgpu_xgmi_sysfs_rem_dev_info(struct
> amdgpu_device *adev,
>
>
>
> -struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev,
> int lock)
> +struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev)
> {
>
RE: [PATCH] drm/amdgpu: unlock mutex on error
[AMD Public Use]
Maybe commit body needs to be improved a bit like "Make sure to unlock the
mutex for error case"
With above addressed, the patch is:
Reviewed-by: Guchun Chen
Regards,
Guchun
-Original Message-
From: Dennis Li
Sent: Wednesday, August 5, 2020 4:42 PM
To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
; Zhou1, Tao ; Zhang, Hawking
; Chen, Guchun
Cc: Li, Dennis
Subject: [PATCH] drm/amdgpu: unlock mutex on error
Make sure unlock the mutex when error happen
Signed-off-by: Dennis Li
Change-Id: I6c36a193df5fe70516282d8136b4eadf32d20915
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index a0ea663ecdbc..5e5369abc6fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -632,13 +632,14 @@ int amdgpu_amdkfd_submit_ib(struct kgd_dev *kgd, enum
kgd_engine_type engine,
}
ret = amdgpu_ib_schedule(ring, 1, ib, job, );
+
+ up_read(>reset_sem);
+
if (ret) {
DRM_ERROR("amdgpu: failed to schedule IB.\n");
goto err_ib_sched;
}
- up_read(>reset_sem);
-
ret = dma_fence_wait(f, false);
err_ib_sched:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 4e017f379eb6..67a756f4337b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -545,7 +545,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
}
ret = amdgpu_xgmi_update_topology(hive, tmp_adev);
if (ret)
- goto exit;
+ goto exit_unlock;
}
/* get latest topology info for each device from psp */ @@
-558,7 +558,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
tmp_adev->gmc.xgmi.node_id,
tmp_adev->gmc.xgmi.hive_id, ret);
/* To do : continue with some node failed or
disable the whole hive */
- goto exit;
+ goto exit_unlock;
}
}
}
@@ -566,7 +566,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
if (!ret)
ret = amdgpu_xgmi_sysfs_add_dev_info(adev, hive);
-
+exit_unlock:
mutex_unlock(>hive_lock);
exit:
if (!ret)
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2] drm/amdgpu: unlock mutex on error
Make sure to unlock the mutex when error happen
v2:
1. correct syntax error in the commit comment
2. remove change-Id
Acked-by: Nirmoy Das
Signed-off-by: Dennis Li
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index a0ea663ecdbc..5e5369abc6fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -632,13 +632,14 @@ int amdgpu_amdkfd_submit_ib(struct kgd_dev *kgd, enum
kgd_engine_type engine,
}
ret = amdgpu_ib_schedule(ring, 1, ib, job, );
+
+ up_read(>reset_sem);
+
if (ret) {
DRM_ERROR("amdgpu: failed to schedule IB.\n");
goto err_ib_sched;
}
- up_read(>reset_sem);
-
ret = dma_fence_wait(f, false);
err_ib_sched:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 4e017f379eb6..67a756f4337b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -545,7 +545,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
}
ret = amdgpu_xgmi_update_topology(hive, tmp_adev);
if (ret)
- goto exit;
+ goto exit_unlock;
}
/* get latest topology info for each device from psp */
@@ -558,7 +558,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
tmp_adev->gmc.xgmi.node_id,
tmp_adev->gmc.xgmi.hive_id, ret);
/* To do : continue with some node failed or
disable the whole hive */
- goto exit;
+ goto exit_unlock;
}
}
}
@@ -566,7 +566,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
if (!ret)
ret = amdgpu_xgmi_sysfs_add_dev_info(adev, hive);
-
+exit_unlock:
mutex_unlock(>hive_lock);
exit:
if (!ret)
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: annotate a false positive locking dependency
[ 264.483189] ==
[ 264.483487] WARNING: possible circular locking dependency detected
[ 264.483781] 5.6.0-deli-v5.6-2848-g3f3109b0e75f #1 Tainted: G OE
[ 264.484076] --
[ 264.484370] kworker/39:1/567 is trying to acquire lock:
[ 264.484663] c15df4b0 (mgpu_info.mutex){+.+.}, at:
amdgpu_unregister_gpu_instance+0x1d/0xc0 [amdgpu]
[ 264.485081]
but task is already holding lock:
[ 264.485670] 965fd31647a0 (>reset_sem){}, at:
amdgpu_device_gpu_recover+0x264/0x1030 [amdgpu]
[ 264.486074]
which lock already depends on the new lock.
[ 264.487043]
the existing dependency chain (in reverse order) is:
[ 264.487710]
-> #3 (>reset_sem){}:
[ 264.488400]down_write+0x49/0x120
[ 264.488783]amdgpu_device_gpu_recover+0x264/0x1030 [amdgpu]
[ 264.489179]amdgpu_ras_do_recovery+0x159/0x190 [amdgpu]
[ 264.489544]process_one_work+0x29e/0x630
[ 264.489910]worker_thread+0x3c/0x3f0
[ 264.490279]kthread+0x12f/0x150
[ 264.490649]ret_from_fork+0x3a/0x50
[ 264.491020]
-> #2 (>hive_lock){+.+.}:
[ 264.491764]__mutex_lock+0x95/0xa20
[ 264.492137]mutex_lock_nested+0x1b/0x20
[ 264.492553]amdgpu_get_xgmi_hive+0x352/0x400 [amdgpu]
[ 264.492972]amdgpu_xgmi_add_device+0xb8/0x460 [amdgpu]
[ 264.493387]amdgpu_device_init+0x12fb/0x1e10 [amdgpu]
[ 264.493807]amdgpu_driver_load_kms+0x5c/0x2c0 [amdgpu]
[ 264.494226]amdgpu_pci_probe+0x11d/0x200 [amdgpu]
[ 264.494617]local_pci_probe+0x47/0xa0
[ 264.494998]work_for_cpu_fn+0x1a/0x30
[ 264.495369]process_one_work+0x29e/0x630
[ 264.495746]worker_thread+0x22b/0x3f0
[ 264.496124]kthread+0x12f/0x150
[ 264.496504]ret_from_fork+0x3a/0x50
[ 264.496876]
-> #1 (xgmi_mutex){+.+.}:
[ 264.497596]__mutex_lock+0x95/0xa20
[ 264.497954]mutex_lock_nested+0x1b/0x20
[ 264.498346]amdgpu_get_xgmi_hive+0x38/0x400 [amdgpu]
[ 264.498741]amdgpu_xgmi_set_pstate+0x10/0x20 [amdgpu]
[ 264.499126]amdgpu_device_ip_late_init+0x219/0x230 [amdgpu]
[ 264.499506]amdgpu_device_init+0x1401/0x1e10 [amdgpu]
[ 264.499886]amdgpu_driver_load_kms+0x5c/0x2c0 [amdgpu]
[ 264.500264]amdgpu_pci_probe+0x11d/0x200 [amdgpu]
[ 264.500608]local_pci_probe+0x47/0xa0
[ 264.500945]work_for_cpu_fn+0x1a/0x30
[ 264.501276]process_one_work+0x29e/0x630
[ 264.501603]worker_thread+0x22b/0x3f0
[ 264.501927]kthread+0x12f/0x150
[ 264.502239]ret_from_fork+0x3a/0x50
[ 264.502541]
-> #0 (mgpu_info.mutex){+.+.}:
[ 264.503126]__lock_acquire+0x13ec/0x16e0
[ 264.503411]lock_acquire+0xb8/0x1c0
[ 264.503693]__mutex_lock+0x95/0xa20
[ 264.504019]mutex_lock_nested+0x1b/0x20
[ 264.504354]amdgpu_unregister_gpu_instance+0x1d/0xc0 [amdgpu]
[ 264.504691]amdgpu_device_gpu_recover+0x360/0x1030 [amdgpu]
[ 264.505029]amdgpu_ras_do_recovery+0x159/0x190 [amdgpu]
[ 264.505334]process_one_work+0x29e/0x630
[ 264.505617]worker_thread+0x3c/0x3f0
[ 264.505897]kthread+0x12f/0x150
[ 264.506176]ret_from_fork+0x3a/0x50
[ 264.506453]
other info that might help us debug this:
[ 264.507267] Chain exists of:
mgpu_info.mutex --> >hive_lock --> >reset_sem
[ 264.508102] Possible unsafe locking scenario:
[ 264.508664]CPU0CPU1
[ 264.508945]
[ 264.509221] lock(>reset_sem);
[ 264.509524]lock(>hive_lock);
[ 264.509818]lock(>reset_sem);
[ 264.510111] lock(mgpu_info.mutex);
[ 264.510401]
*** DEADLOCK ***
[ 264.511224] 4 locks held by kworker/39:1/567:
[ 264.511499] #0: 961ff5c1d348 ((wq_completion)events){+.+.}, at:
process_one_work+0x21f/0x630
[ 264.511793] #1: afa90e233e58
((work_completion)(>recovery_work)){+.+.}, at: process_one_work+0x21f/0x630
[ 264.512100] #2: c16245d0 (>hive_lock){+.+.}, at:
amdgpu_device_gpu_recover+0xb0/0x1030 [amdgpu]
[ 264.512450] #3: 965fd31647a0 (>reset_sem){}, at:
amdgpu_device_gpu_recover+0x264/0x1030 [amdgpu]
Remove the lock(>hive_lock) out of amdgpu_get_xgmi_hive,
to disable its locking dependency on xgmi_mutex.
Signed-off-by: Dennis Li
Change-Id: I2d9d80ee23f9f9ac6ce9e1b9e5e1b2b3530f5bdd
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 62ecac97fbd2..6c572db42d92 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2840,7 +2840,7 @@ static void amdgpu_device_xgmi_reset_func(struct
work_struct
Re: [PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
On Wed, 2020-08-05 at 17:27 -0400, Alex Deucher wrote:
> On Wed, Aug 5, 2020 at 4:53 PM Joe Perches wrote:
> > On Wed, 2020-08-05 at 16:01 -0400, Alex Deucher wrote:
> > > On Wed, Aug 5, 2020 at 7:35 AM Colin King
> > > wrote:
> > > > From: Colin Ian King
> > > >
> > > > There is a spelling mistake in a DRM_ERROR message. Fix it.
> > > >
> > > > Signed-off-by: Colin Ian King
> > >
> > > This is already fixed.
> >
> > This fix is not in today's -next.
> >
> > Perhaps whatever tree it's fixed in should be in -next.
> >
>
> Weird. It's in the drm-next tree as:
>
> commit 4afaa61db9cf5250b5734c2531b226e7b3a3d691
> Author: Colin Ian King
> Date: Fri Jul 10 09:37:58 2020 +0100
>
> drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
>
> There is a spelling mistake in a DRM_ERROR error message. Fix it.
>
> Signed-off-by: Colin Ian King
> Signed-off-by: Alex Deucher
>
> Alex
>
> > $ git show --oneline -s
> > d15fe4ec0435 (HEAD, tag: next-20200805, origin/master, origin/HEAD) Add
> > linux-next specific files for 20200805
> >
> > $ git grep -i falied drivers
> > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c:DRM_ERROR("Falied
> > to terminate tmr\n");
> >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > []
> > > > @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle)
> > > >
> > > > ret = psp_tmr_terminate(psp);
> > > > if (ret) {
> > > > - DRM_ERROR("Falied to terminate tmr\n");
> > > > + DRM_ERROR("Failed to terminate tmr\n");
> > > > return ret;
> > > > }
Dunno.
Maybe it's due to some ordering of trees in
how -next accumulates patches?
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
On 05/08/2020 22:27, Alex Deucher wrote:
> On Wed, Aug 5, 2020 at 4:53 PM Joe Perches wrote:
>>
>> On Wed, 2020-08-05 at 16:01 -0400, Alex Deucher wrote:
>>> On Wed, Aug 5, 2020 at 7:35 AM Colin King wrote:
>>>> From: Colin Ian King
>>>>
>>>> There is a spelling mistake in a DRM_ERROR message. Fix it.
>>>>
>>>> Signed-off-by: Colin Ian King
>>>
>>> This is already fixed.
>>
>> This fix is not in today's -next.
>>
>> Perhaps whatever tree it's fixed in should be in -next.
>>
>
> Weird. It's in the drm-next tree as:
>
> commit 4afaa61db9cf5250b5734c2531b226e7b3a3d691
> Author: Colin Ian King
> Date: Fri Jul 10 09:37:58 2020 +0100
>
> drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
>
> There is a spelling mistake in a DRM_ERROR error message. Fix it.
>
> Signed-off-by: Colin Ian King
> Signed-off-by: Alex Deucher
>
> Alex
>
>>
>> $ git show --oneline -s
>> d15fe4ec0435 (HEAD, tag: next-20200805, origin/master, origin/HEAD) Add
>> linux-next specific files for 20200805
>>
>> $ git grep -i falied drivers
>> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c:DRM_ERROR("Falied to
>> terminate tmr\n");
>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> []
>>>> @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle)
>>>>
>>>> ret = psp_tmr_terminate(psp);
>>>> if (ret) {
>>>> - DRM_ERROR("Falied to terminate tmr\n");
>>>> + DRM_ERROR("Failed to terminate tmr\n");
>>>> return ret;
>>>> }
>>
>>
Somehow I omitted adding this to my tracking list and forgot that I sent
this already. Apologies for patch deja-vu.
Colin
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
On Wed, 2020-08-05 at 16:01 -0400, Alex Deucher wrote:
> On Wed, Aug 5, 2020 at 7:35 AM Colin King wrote:
> > From: Colin Ian King
> >
> > There is a spelling mistake in a DRM_ERROR message. Fix it.
> >
> > Signed-off-by: Colin Ian King
>
> This is already fixed.
This fix is not in today's -next.
Perhaps whatever tree it's fixed in should be in -next.
$ git show --oneline -s
d15fe4ec0435 (HEAD, tag: next-20200805, origin/master, origin/HEAD) Add
linux-next specific files for 20200805
$ git grep -i falied drivers
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c:DRM_ERROR("Falied to
terminate tmr\n");
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
[]
> > @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle)
> >
> > ret = psp_tmr_terminate(psp);
> > if (ret) {
> > - DRM_ERROR("Falied to terminate tmr\n");
> > + DRM_ERROR("Failed to terminate tmr\n");
> > return ret;
> > }
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
On Wed, Aug 5, 2020 at 4:53 PM Joe Perches wrote:
>
> On Wed, 2020-08-05 at 16:01 -0400, Alex Deucher wrote:
> > On Wed, Aug 5, 2020 at 7:35 AM Colin King wrote:
> > > From: Colin Ian King
> > >
> > > There is a spelling mistake in a DRM_ERROR message. Fix it.
> > >
> > > Signed-off-by: Colin Ian King
> >
> > This is already fixed.
>
> This fix is not in today's -next.
>
> Perhaps whatever tree it's fixed in should be in -next.
>
Weird. It's in the drm-next tree as:
commit 4afaa61db9cf5250b5734c2531b226e7b3a3d691
Author: Colin Ian King
Date: Fri Jul 10 09:37:58 2020 +0100
drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
There is a spelling mistake in a DRM_ERROR error message. Fix it.
Signed-off-by: Colin Ian King
Signed-off-by: Alex Deucher
Alex
>
> $ git show --oneline -s
> d15fe4ec0435 (HEAD, tag: next-20200805, origin/master, origin/HEAD) Add
> linux-next specific files for 20200805
>
> $ git grep -i falied drivers
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c:DRM_ERROR("Falied to
> terminate tmr\n");
>
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> []
> > > @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle)
> > >
> > > ret = psp_tmr_terminate(psp);
> > > if (ret) {
> > > - DRM_ERROR("Falied to terminate tmr\n");
> > > + DRM_ERROR("Failed to terminate tmr\n");
> > > return ret;
> > > }
>
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 4/7] drm/amd/display: Use validated tiling_flags and tmz_surface in commit_tail
Reviewed-by: Rodrigo Siqueira
On 07/30, Nicholas Kazlauskas wrote:
> [Why]
> So we're not racing with userspace or deadlocking DM.
>
> [How]
> These flags are now stored on dm_plane_state itself and acquried and
> validated during commit_check, so just use those instead.
>
> Cc: Daniel Vetter
> Cc: Bhawanpreet Lakha
> Cc: Rodrigo Siqueira
> Signed-off-by: Nicholas Kazlauskas
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 18 --
> 1 file changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index f78c09c9585e..0d5f45742bb5 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -7094,8 +7094,6 @@ static void amdgpu_dm_commit_planes(struct
> drm_atomic_state *state,
> long r;
> unsigned long flags;
> struct amdgpu_bo *abo;
> - uint64_t tiling_flags;
> - bool tmz_surface = false;
> uint32_t target_vblank, last_flip_vblank;
> bool vrr_active = amdgpu_dm_vrr_active(acrtc_state);
> bool pflip_present = false;
> @@ -7179,20 +7177,12 @@ static void amdgpu_dm_commit_planes(struct
> drm_atomic_state *state,
> if (unlikely(r <= 0))
> DRM_ERROR("Waiting for fences timed out!");
>
> - /*
> - * We cannot reserve buffers here, which means the normal flag
> - * access functions don't work. Paper over this with READ_ONCE,
> - * but maybe the flags are invariant enough that not even that
> - * would be needed.
> - */
> - tiling_flags = READ_ONCE(abo->tiling_flags);
> - tmz_surface = READ_ONCE(abo->flags) &
> AMDGPU_GEM_CREATE_ENCRYPTED;
> -
> fill_dc_plane_info_and_addr(
> - dm->adev, new_plane_state, tiling_flags,
> + dm->adev, new_plane_state,
> + dm_new_plane_state->tiling_flags,
> >plane_infos[planes_count],
> - >flip_addrs[planes_count].address, tmz_surface,
> - false);
> + >flip_addrs[planes_count].address,
> + dm_new_plane_state->tmz_surface, false);
>
> DRM_DEBUG_DRIVER("plane: id=%d dcc_en=%d\n",
>new_plane_state->plane->index,
> --
> 2.25.1
>
--
Rodrigo Siqueira
https://siqueira.tech
signature.asc
Description: PGP signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 3/7] drm/amd/display: Avoid using unvalidated tiling_flags and tmz_surface in prepare_planes
Reviewed-by: Rodrigo Siqueira
On 07/30, Nicholas Kazlauskas wrote:
> [Why]
> We're racing with userspace as the flags could potentially change
> from when we acquired and validated them in commit_check.
>
> [How]
> We unfortunately can't drop this function in its entirety from
> prepare_planes since we don't know the afb->address at commit_check
> time yet.
>
> So instead of querying new tiling_flags and tmz_surface use the ones
> from the plane_state directly.
>
> While we're at it, also update the force_disable_dcc option based
> on the state from atomic check.
>
> Cc: Bhawanpreet Lakha
> Cc: Rodrigo Siqueira
> Signed-off-by: Nicholas Kazlauskas
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 ++-
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index bf1881bd492c..f78c09c9585e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5794,14 +5794,8 @@ static int dm_plane_helper_prepare_fb(struct drm_plane
> *plane,
> struct list_head list;
> struct ttm_validate_buffer tv;
> struct ww_acquire_ctx ticket;
> - uint64_t tiling_flags;
> uint32_t domain;
> int r;
> - bool tmz_surface = false;
> - bool force_disable_dcc = false;
> -
> - dm_plane_state_old = to_dm_plane_state(plane->state);
> - dm_plane_state_new = to_dm_plane_state(new_state);
>
> if (!new_state->fb) {
> DRM_DEBUG_DRIVER("No FB bound\n");
> @@ -5845,27 +5839,35 @@ static int dm_plane_helper_prepare_fb(struct
> drm_plane *plane,
> return r;
> }
>
> - amdgpu_bo_get_tiling_flags(rbo, _flags);
> -
> - tmz_surface = amdgpu_bo_encrypted(rbo);
> -
> ttm_eu_backoff_reservation(, );
>
> afb->address = amdgpu_bo_gpu_offset(rbo);
>
> amdgpu_bo_ref(rbo);
>
> + /**
> + * We don't do surface updates on planes that have been newly created,
> + * but we also don't have the afb->address during atomic check.
> + *
> + * Fill in buffer attributes depending on the address here, but only on
> + * newly created planes since they're not being used by DC yet and this
> + * won't modify global state.
> + */
> + dm_plane_state_old = to_dm_plane_state(plane->state);
> + dm_plane_state_new = to_dm_plane_state(new_state);
> +
> if (dm_plane_state_new->dc_state &&
> - dm_plane_state_old->dc_state !=
> dm_plane_state_new->dc_state) {
> - struct dc_plane_state *plane_state =
> dm_plane_state_new->dc_state;
> + dm_plane_state_old->dc_state != dm_plane_state_new->dc_state) {
> + struct dc_plane_state *plane_state =
> + dm_plane_state_new->dc_state;
> + bool force_disable_dcc = !plane_state->dcc.enable;
>
> - force_disable_dcc = adev->asic_type == CHIP_RAVEN &&
> adev->in_suspend;
> fill_plane_buffer_attributes(
> adev, afb, plane_state->format, plane_state->rotation,
> - tiling_flags, _state->tiling_info,
> - _state->plane_size, _state->dcc,
> - _state->address, tmz_surface,
> - force_disable_dcc);
> + dm_plane_state_new->tiling_flags,
> + _state->tiling_info, _state->plane_size,
> + _state->dcc, _state->address,
> + dm_plane_state_new->tmz_surface, force_disable_dcc);
> }
>
> return 0;
> --
> 2.25.1
>
--
Rodrigo Siqueira
https://siqueira.tech
signature.asc
Description: PGP signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/7] drm/amd/display: Reset plane when tiling flags change
On 07/30, Nicholas Kazlauskas wrote:
> [Why]
> Enabling or disable DCC or switching between tiled and linear formats
> can require bandwidth updates.
>
> They're currently skipping all DC validation by being treated as purely
> surface updates.
>
> [How]
> Treat tiling_flag changes (which encode DCC state) as a condition for
> resetting the plane.
>
> Cc: Bhawanpreet Lakha
> Cc: Rodrigo Siqueira
> Cc: Hersen Wu
> Signed-off-by: Nicholas Kazlauskas
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 7cc5ab90ce13..bf1881bd492c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8332,6 +8332,8 @@ static bool should_reset_plane(struct drm_atomic_state
> *state,
>* TODO: Come up with a more elegant solution for this.
>*/
> for_each_oldnew_plane_in_state(state, other, old_other_state,
> new_other_state, i) {
> + struct dm_plane_state *old_dm_plane_state, *new_dm_plane_state;
> +
> if (other->type == DRM_PLANE_TYPE_CURSOR)
> continue;
>
> @@ -8342,9 +8344,20 @@ static bool should_reset_plane(struct drm_atomic_state
> *state,
> if (old_other_state->crtc != new_other_state->crtc)
> return true;
>
> - /* TODO: Remove this once we can handle fast format changes. */
> - if (old_other_state->fb && new_other_state->fb &&
> - old_other_state->fb->format != new_other_state->fb->format)
> + /* Framebuffer checks fall at the end. */
> + if (!old_other_state->fb || !new_other_state->fb)
> + continue;
> +
> + /* Pixel format changes can require bandwidth updates. */
> + if (old_other_state->fb->format != new_other_state->fb->format)
> + return true;
> +
> + old_dm_plane_state = to_dm_plane_state(old_other_state);
> + new_dm_plane_state = to_dm_plane_state(new_other_state);
> +
> + /* Tiling and DCC changes also require bandwidth updates. */
> + if (old_dm_plane_state->tiling_flags !=
> + new_dm_plane_state->tiling_flags)
Why not add a case when we move to a TMZ area?
Reviewed-by: Rodrigo Siqueira
> return true;
> }
>
> --
> 2.25.1
>
--
Rodrigo Siqueira
https://siqueira.tech
signature.asc
Description: PGP signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/7] drm/amd/display: Store tiling_flags and tmz_surface on dm_plane_state
Reviewed-by: Rodrigo Siqueira
On 07/30, Nicholas Kazlauskas wrote:
> [Why]
> Store these in advance so we can reuse them later in commit_tail without
> having to reserve the fbo again.
>
> These will also be used for checking for tiling changes when deciding
> to reset the plane or not.
>
> [How]
> This change should mostly be a refactor. Only commit check is affected
> for now and I'll drop the get_fb_info calls in prepare_planes and
> commit_tail after.
>
> This runs a prepass loop once we think that all planes have been added
> to the context and replaces the get_fb_info calls with accessing the
> dm_plane_state instead.
>
> Cc: Bhawanpreet Lakha
> Cc: Rodrigo Siqueira
> Signed-off-by: Nicholas Kazlauskas
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 60 +++
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +
> 2 files changed, 37 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 8d64f5fde7e2..7cc5ab90ce13 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3700,8 +3700,17 @@ static int fill_dc_scaling_info(const struct
> drm_plane_state *state,
> static int get_fb_info(const struct amdgpu_framebuffer *amdgpu_fb,
> uint64_t *tiling_flags, bool *tmz_surface)
> {
> - struct amdgpu_bo *rbo = gem_to_amdgpu_bo(amdgpu_fb->base.obj[0]);
> - int r = amdgpu_bo_reserve(rbo, false);
> + struct amdgpu_bo *rbo;
> + int r;
> +
> + if (!amdgpu_fb) {
> + *tiling_flags = 0;
> + *tmz_surface = false;
> + return 0;
> + }
> +
> + rbo = gem_to_amdgpu_bo(amdgpu_fb->base.obj[0]);
> + r = amdgpu_bo_reserve(rbo, false);
>
> if (unlikely(r)) {
> /* Don't show error message when returning -ERESTARTSYS */
> @@ -4124,13 +4133,10 @@ static int fill_dc_plane_attributes(struct
> amdgpu_device *adev,
> struct drm_crtc_state *crtc_state)
> {
> struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(crtc_state);
> - const struct amdgpu_framebuffer *amdgpu_fb =
> - to_amdgpu_framebuffer(plane_state->fb);
> + struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
> struct dc_scaling_info scaling_info;
> struct dc_plane_info plane_info;
> - uint64_t tiling_flags;
> int ret;
> - bool tmz_surface = false;
> bool force_disable_dcc = false;
>
> ret = fill_dc_scaling_info(plane_state, _info);
> @@ -4142,15 +4148,12 @@ static int fill_dc_plane_attributes(struct
> amdgpu_device *adev,
> dc_plane_state->clip_rect = scaling_info.clip_rect;
> dc_plane_state->scaling_quality = scaling_info.scaling_quality;
>
> - ret = get_fb_info(amdgpu_fb, _flags, _surface);
> - if (ret)
> - return ret;
> -
> force_disable_dcc = adev->asic_type == CHIP_RAVEN && adev->in_suspend;
> - ret = fill_dc_plane_info_and_addr(adev, plane_state, tiling_flags,
> + ret = fill_dc_plane_info_and_addr(adev, plane_state,
> + dm_plane_state->tiling_flags,
> _info,
> _plane_state->address,
> - tmz_surface,
> + dm_plane_state->tmz_surface,
> force_disable_dcc);
> if (ret)
> return ret;
> @@ -5753,6 +5756,10 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane)
> dc_plane_state_retain(dm_plane_state->dc_state);
> }
>
> + /* Framebuffer hasn't been updated yet, so retain old flags. */
> + dm_plane_state->tiling_flags = old_dm_plane_state->tiling_flags;
> + dm_plane_state->tmz_surface = old_dm_plane_state->tmz_surface;
> +
> return _plane_state->base;
> }
>
> @@ -8557,13 +8564,9 @@ dm_determine_update_type_for_commit(struct
> amdgpu_display_manager *dm,
> continue;
>
> for_each_oldnew_plane_in_state(state, plane, old_plane_state,
> new_plane_state, j) {
> - const struct amdgpu_framebuffer *amdgpu_fb =
> - to_amdgpu_framebuffer(new_plane_state->fb);
> struct dc_plane_info *plane_info =
> >plane_infos[num_plane];
> struct dc_flip_addrs *flip_addr =
> >flip_addrs[num_plane];
> struct dc_scaling_info *scaling_info =
> >scaling_infos[num_plane];
> - uint64_t tiling_flags;
> - bool tmz_surface = false;
>
> new_plane_crtc = new_plane_state->crtc;
> new_dm_plane_state = to_dm_plane_state(new_plane_state);
> @@ -8610,16 +8613,12 @@
Re: [PATCH 6/7] drm/amd/display: Drop dm_determine_update_type_for_commit
Reviewed-by: Rodrigo Siqueira
On 07/30, Nicholas Kazlauskas wrote:
> [Why]
> This was added in the past to solve the issue of not knowing when
> to stall for medium and full updates in DM.
>
> Since DC is ultimately decides what requires bandwidth changes we
> wanted to make use of it directly to determine this.
>
> The problem is that we can't actually pass any of the stream or surface
> updates into DC global validation, so we don't actually check if the new
> configuration is valid - we just validate the old existing config
> instead and stall for outstanding commits to finish.
>
> There's also the problem of grabbing the DRM private object for
> pageflips which can lead to page faults in the case where commits
> execute out of order and free a DRM private object state that was
> still required for commit tail.
>
> [How]
> Now that we reset the plane in DM with the same conditions DC checks
> we can have planes go through DC validation and we know when we need
> to check and stall based on whether the stream or planes changed.
>
> We mark lock_and_validation_needed whenever we've done this, so just
> go back to using that instead of dm_determine_update_type_for_commit.
>
> Since we'll skip resetting the plane for a pageflip we will no longer
> grab the DRM private object for pageflips as well, avoiding the
> page fault issued caused by pageflipping under load with commits
> executing out of order.
>
> Cc: Harry Wentland
> Cc: Bhawanpreet Lakha
> Signed-off-by: Nicholas Kazlauskas
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 199 ++
> 1 file changed, 17 insertions(+), 182 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 2cbb29199e61..59829ec81694 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8542,161 +8542,6 @@ static int dm_update_plane_state(struct dc *dc,
> return ret;
> }
>
> -static int
> -dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm,
> - struct drm_atomic_state *state,
> - enum surface_update_type *out_type)
> -{
> - struct dc *dc = dm->dc;
> - struct dm_atomic_state *dm_state = NULL, *old_dm_state = NULL;
> - int i, j, num_plane, ret = 0;
> - struct drm_plane_state *old_plane_state, *new_plane_state;
> - struct dm_plane_state *new_dm_plane_state, *old_dm_plane_state;
> - struct drm_crtc *new_plane_crtc;
> - struct drm_plane *plane;
> -
> - struct drm_crtc *crtc;
> - struct drm_crtc_state *new_crtc_state, *old_crtc_state;
> - struct dm_crtc_state *new_dm_crtc_state, *old_dm_crtc_state;
> - struct dc_stream_status *status = NULL;
> - enum surface_update_type update_type = UPDATE_TYPE_FAST;
> - struct surface_info_bundle {
> - struct dc_surface_update surface_updates[MAX_SURFACES];
> - struct dc_plane_info plane_infos[MAX_SURFACES];
> - struct dc_scaling_info scaling_infos[MAX_SURFACES];
> - struct dc_flip_addrs flip_addrs[MAX_SURFACES];
> - struct dc_stream_update stream_update;
> - } *bundle;
> -
> - bundle = kzalloc(sizeof(*bundle), GFP_KERNEL);
> -
> - if (!bundle) {
> - DRM_ERROR("Failed to allocate update bundle\n");
> - /* Set type to FULL to avoid crashing in DC*/
> - update_type = UPDATE_TYPE_FULL;
> - goto cleanup;
> - }
> -
> - for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
> new_crtc_state, i) {
> -
> - memset(bundle, 0, sizeof(struct surface_info_bundle));
> -
> - new_dm_crtc_state = to_dm_crtc_state(new_crtc_state);
> - old_dm_crtc_state = to_dm_crtc_state(old_crtc_state);
> - num_plane = 0;
> -
> - if (new_dm_crtc_state->stream != old_dm_crtc_state->stream) {
> - update_type = UPDATE_TYPE_FULL;
> - goto cleanup;
> - }
> -
> - if (!new_dm_crtc_state->stream)
> - continue;
> -
> - for_each_oldnew_plane_in_state(state, plane, old_plane_state,
> new_plane_state, j) {
> - struct dc_plane_info *plane_info =
> >plane_infos[num_plane];
> - struct dc_flip_addrs *flip_addr =
> >flip_addrs[num_plane];
> - struct dc_scaling_info *scaling_info =
> >scaling_infos[num_plane];
> -
> - new_plane_crtc = new_plane_state->crtc;
> - new_dm_plane_state = to_dm_plane_state(new_plane_state);
> - old_dm_plane_state = to_dm_plane_state(old_plane_state);
> -
> - if (plane->type == DRM_PLANE_TYPE_CURSOR)
> - continue;
> -
> - if (new_dm_plane_state->dc_state !=
>
Re: [PATCH 5/7] drm/amd/display: Reset plane for anything that's not a FAST update
On 07/30, Nicholas Kazlauskas wrote: > [Why] > MEDIUM or FULL updates can require global validation or affect > bandwidth. By treating these all simply as surface updates we aren't > actually passing this through DC global validation. > > [How] > There's currently no way to pass surface updates through DC global > validation, nor do I think it's a good idea to change the interface > to accept these. > > DC global validation itself is currently stateless, and we can move > our update type checking to be stateless as well by duplicating DC > surface checks in DM based on DRM properties. > > We wanted to rely on DC automatically determining this since DC knows > best, but DM is ultimately what fills in everything into DC plane > state so it does need to know as well. > > There are basically only three paths that we exercise in DM today: > > 1) Cursor (async update) > 2) Pageflip (fast update) > 3) Full pipe programming (medium/full updates) > > Which means that anything that's more than a pageflip really needs to > go down path #3. > > So this change duplicates all the surface update checks based on DRM > state instead inside of should_reset_plane(). > > Next step is dropping dm_determine_update_type_for_commit and we no > longer require the old DC state at all for global validation. > > Optimization can come later so we don't reset DC planes at all for > MEDIUM udpates and avoid validation, but we might require some extra > checks in DM to achieve this. How about adding this optimization description in our TODO list under-display folder? Reviewed-by: Rodrigo Siqueira > Cc: Bhawanpreet Lakha > Cc: Hersen Wu > Signed-off-by: Nicholas Kazlauskas > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 25 +++ > 1 file changed, 25 insertions(+) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 0d5f45742bb5..2cbb29199e61 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -8336,6 +8336,31 @@ static bool should_reset_plane(struct drm_atomic_state > *state, > if (old_other_state->crtc != new_other_state->crtc) > return true; > > + /* Src/dst size and scaling updates. */ > + if (old_other_state->src_w != new_other_state->src_w || > + old_other_state->src_h != new_other_state->src_h || > + old_other_state->crtc_w != new_other_state->crtc_w || > + old_other_state->crtc_h != new_other_state->crtc_h) > + return true; > + > + /* Rotation / mirroring updates. */ > + if (old_other_state->rotation != new_other_state->rotation) > + return true; > + > + /* Blending updates. */ > + if (old_other_state->pixel_blend_mode != > + new_other_state->pixel_blend_mode) > + return true; > + > + /* Alpha updates. */ > + if (old_other_state->alpha != new_other_state->alpha) > + return true; > + > + /* Colorspace changes. */ > + if (old_other_state->color_range != > new_other_state->color_range || > + old_other_state->color_encoding != > new_other_state->color_encoding) > + return true; > + > /* Framebuffer checks fall at the end. */ > if (!old_other_state->fb || !new_other_state->fb) > continue; > -- > 2.25.1 > > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7CRodrigo.Siqueira%40amd.com%7Ccc095e7ce6164f529e2708d834c86d1b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637317382766607890sdata=omLC%2BizXVEjjGe6IylBpniZzyUGlzTATrgRoWEo6dHc%3Dreserved=0 -- Rodrigo Siqueira https://siqueira.tech signature.asc Description: PGP signature ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 7/7] drm/amd/display: Replace DRM private objects with subclassed DRM atomic state
Hi,
I have some minor inline comments, but everything looks fine when I
tested it on Raven; feel free to add
Tested-by: Rodrigo Siqueira
in the whole series.
On 07/30, Nicholas Kazlauskas wrote:
> [Why]
> DM atomic check was structured in a way that we required old DC state
> in order to dynamically add and remove planes and streams from the
> context to build the DC state context for validation.
>
> DRM private objects were used to carry over the last DC state and
> were added to the context on nearly every commit - regardless of fast
> or full so we could check whether or not the new state could affect
> bandwidth.
>
> The problem with this model is that DRM private objects do not
> implicitly stall out other commits.
>
> So if you have two commits touching separate DRM objects they could
> run concurrently and potentially execute out of order - leading to a
> use-after-free.
>
> If we want this to be safe we have two options:
> 1. Stall out concurrent commits since they touch the same private object
> 2. Refactor DM to not require old DC state and drop private object usage
>
> [How]
> This implements approach #2 since it still allows for judder free
> updates in multi-display scenarios.
>
> I'll list the big changes in order at a high level:
>
> 1. Subclass DRM atomic state instead of using DRM private objects.
>
> DC relied on the old state to determine which changes cause bandwidth
> updates but now we have DM perform similar checks based on DRM state
> instead - dropping the requirement for old state to exist at all.
>
> This means that we can now build a new DC context from scratch whenever
> we have something that DM thinks could affect bandwidth.
>
> Whenever we need to rebuild bandwidth we now add all CRTCs and planes
> to the DRM state in order to get the absolute set of DC streams and
> DC planes.
>
> This introduces a stall on other commits, but this stall already
> exists because of the lock_and_validation logic and it's necessary
> since updates may move around pipes and require full reprogramming.
>
> 2. Drop workarounds to add planes to maintain z-order early in atomic
> check. This is no longer needed because of the changes for (1).
>
> This also involves fixing up should_plane_reset checks since we can just
> avoid resetting streams and planes when they haven't actually changed.
>
> 3. Rework dm_update_crtc_state and dm_update_plane_state to be single
> pass instead of two pass.
>
> This is necessary since we no longer have the dc_state to add and
> remove planes to the context in and we want to defer creation to the
> end of commit_check.
>
> It also makes the logic a lot simpler to follow since as an added bonus.
>
> Cc: Bhawanpreet Lakha
> Cc: Harry Wentland
> Cc: Leo Li
> Cc: Daniel Vetter
> Signed-off-by: Nicholas Kazlauskas
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 720 +++---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 11 +-
> 2 files changed, 280 insertions(+), 451 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 59829ec81694..97a7dfc620e8 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1839,7 +1839,6 @@ static int dm_resume(void *handle)
> struct drm_plane *plane;
> struct drm_plane_state *new_plane_state;
> struct dm_plane_state *dm_new_plane_state;
> - struct dm_atomic_state *dm_state =
> to_dm_atomic_state(dm->atomic_obj.state);
> enum dc_connection_type new_connection_type = dc_connection_none;
> struct dc_state *dc_state;
> int i, r, j;
> @@ -1879,11 +1878,6 @@ static int dm_resume(void *handle)
>
> return 0;
> }
> - /* Recreate dc_state - DC invalidates it when setting power state to
> S3. */
> - dc_release_state(dm_state->context);
> - dm_state->context = dc_create_state(dm->dc);
> - /* TODO: Remove dc_state->dccg, use dc->dccg directly. */
> - dc_resource_state_construct(dm->dc, dm_state->context);
>
> /* Before powering on DC we need to re-initialize DMUB. */
> r = dm_dmub_hw_init(adev);
> @@ -2019,11 +2013,51 @@ const struct amdgpu_ip_block_version dm_ip_block =
> * *WIP*
> */
>
> +struct drm_atomic_state *dm_atomic_state_alloc(struct drm_device *dev)
> +{
> + struct dm_atomic_state *dm_state;
> +
> + dm_state = kzalloc(sizeof(*dm_state), GFP_KERNEL);
How about use GFP_ATOMIC here?
> +
> + if (!dm_state)
> + return NULL;
> +
> + if (drm_atomic_state_init(dev, _state->base) < 0) {
> + kfree(dm_state);
> + return NULL;
> + }
> +
> + return _state->base;
> +}
> +
> +void dm_atomic_state_free(struct drm_atomic_state *state)
> +{
> + struct dm_atomic_state *dm_state = to_dm_atomic_state(state);
> +
> + if (dm_state->context) {
> +
Re: [PATCH] drm/amdgpu: unlock mutex on error
On 2020-08-05 4:41 a.m., Dennis Li wrote:
> Make sure unlock the mutex when error happen
...^(to).(.)
Regards,
Luben
>
> Signed-off-by: Dennis Li
> Change-Id: I6c36a193df5fe70516282d8136b4eadf32d20915
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index a0ea663ecdbc..5e5369abc6fa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -632,13 +632,14 @@ int amdgpu_amdkfd_submit_ib(struct kgd_dev *kgd, enum
> kgd_engine_type engine,
> }
>
> ret = amdgpu_ib_schedule(ring, 1, ib, job, );
> +
> + up_read(>reset_sem);
> +
> if (ret) {
> DRM_ERROR("amdgpu: failed to schedule IB.\n");
> goto err_ib_sched;
> }
>
> - up_read(>reset_sem);
> -
> ret = dma_fence_wait(f, false);
>
> err_ib_sched:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 4e017f379eb6..67a756f4337b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -545,7 +545,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
> }
> ret = amdgpu_xgmi_update_topology(hive, tmp_adev);
> if (ret)
> - goto exit;
> + goto exit_unlock;
> }
>
> /* get latest topology info for each device from psp */
> @@ -558,7 +558,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
> tmp_adev->gmc.xgmi.node_id,
> tmp_adev->gmc.xgmi.hive_id, ret);
> /* To do : continue with some node failed or
> disable the whole hive */
> - goto exit;
> + goto exit_unlock;
> }
> }
> }
> @@ -566,7 +566,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
> if (!ret)
> ret = amdgpu_xgmi_sysfs_add_dev_info(adev, hive);
>
> -
> +exit_unlock:
> mutex_unlock(>hive_lock);
> exit:
> if (!ret)
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH][next] drm/amdgpu: fix spelling mistake "paramter" -> "parameter"
On Wed, Aug 5, 2020 at 8:15 AM Colin King wrote:
>
> From: Colin Ian King
>
> There is a spelling mistake in a dev_warn message. Fix it.
>
> Signed-off-by: Colin Ian King
Applied. Thanks!
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index b72aeeb0a226..16e23f053361 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1201,7 +1201,7 @@ static int amdgpu_device_check_arguments(struct
> amdgpu_device *adev)
>
> if (amdgpu_num_kcq > 8 || amdgpu_num_kcq < 0) {
> amdgpu_num_kcq = 8;
> - dev_warn(adev->dev, "set kernel compute queue number to 8 due
> to invalid paramter provided by user\n");
> + dev_warn(adev->dev, "set kernel compute queue number to 8 due
> to invalid parameter provided by user\n");
> }
>
> return 0;
> --
> 2.27.0
>
> ___
> 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/amdgpu: fix spelling mistake "Falied" -> "Failed"
On Wed, Aug 5, 2020 at 7:35 AM Colin King wrote:
>
> From: Colin Ian King
>
> There is a spelling mistake in a DRM_ERROR message. Fix it.
>
> Signed-off-by: Colin Ian King
This is already fixed.
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 49d4514aa6ed..c68369731b20 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle)
>
> ret = psp_tmr_terminate(psp);
> if (ret) {
> - DRM_ERROR("Falied to terminate tmr\n");
> + DRM_ERROR("Failed to terminate tmr\n");
> return ret;
> }
>
> --
> 2.27.0
>
> ___
> 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
[PATCH 6/9] drm/amd/display: Switch to immediate mode for updating infopackets
From: Anthony Koo
[Why]
Using FRAME_UPDATE will result in infopacket to be potentially updated
one frame late.
In commit stream scenarios for previously active stream, some stale
infopacket data from previous config might be erroneously sent out on
initial frame after stream is re-enabled.
[How]
Switch to using IMMEDIATE_UPDATE mode
Signed-off-by: Anthony Koo
Reviewed-by: Ashley Thomas
Acked-by: Qingqing Zhuo
---
.../amd/display/dc/dcn10/dcn10_stream_encoder.c | 16
.../amd/display/dc/dcn10/dcn10_stream_encoder.h | 14 ++
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_stream_encoder.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_stream_encoder.c
index 07b2f9399671..842abb4c475b 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_stream_encoder.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_stream_encoder.c
@@ -121,35 +121,35 @@ void enc1_update_generic_info_packet(
switch (packet_index) {
case 0:
REG_UPDATE(AFMT_VBI_PACKET_CONTROL1,
- AFMT_GENERIC0_FRAME_UPDATE, 1);
+ AFMT_GENERIC0_IMMEDIATE_UPDATE, 1);
break;
case 1:
REG_UPDATE(AFMT_VBI_PACKET_CONTROL1,
- AFMT_GENERIC1_FRAME_UPDATE, 1);
+ AFMT_GENERIC1_IMMEDIATE_UPDATE, 1);
break;
case 2:
REG_UPDATE(AFMT_VBI_PACKET_CONTROL1,
- AFMT_GENERIC2_FRAME_UPDATE, 1);
+ AFMT_GENERIC2_IMMEDIATE_UPDATE, 1);
break;
case 3:
REG_UPDATE(AFMT_VBI_PACKET_CONTROL1,
- AFMT_GENERIC3_FRAME_UPDATE, 1);
+ AFMT_GENERIC3_IMMEDIATE_UPDATE, 1);
break;
case 4:
REG_UPDATE(AFMT_VBI_PACKET_CONTROL1,
- AFMT_GENERIC4_FRAME_UPDATE, 1);
+ AFMT_GENERIC4_IMMEDIATE_UPDATE, 1);
break;
case 5:
REG_UPDATE(AFMT_VBI_PACKET_CONTROL1,
- AFMT_GENERIC5_FRAME_UPDATE, 1);
+ AFMT_GENERIC5_IMMEDIATE_UPDATE, 1);
break;
case 6:
REG_UPDATE(AFMT_VBI_PACKET_CONTROL1,
- AFMT_GENERIC6_FRAME_UPDATE, 1);
+ AFMT_GENERIC6_IMMEDIATE_UPDATE, 1);
break;
case 7:
REG_UPDATE(AFMT_VBI_PACKET_CONTROL1,
- AFMT_GENERIC7_FRAME_UPDATE, 1);
+ AFMT_GENERIC7_IMMEDIATE_UPDATE, 1);
break;
default:
break;
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_stream_encoder.h
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_stream_encoder.h
index ed385b1477be..30eae7459d50 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_stream_encoder.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_stream_encoder.h
@@ -281,7 +281,14 @@ struct dcn10_stream_enc_registers {
SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC2_FRAME_UPDATE,
mask_sh),\
SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC3_FRAME_UPDATE,
mask_sh),\
SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC4_FRAME_UPDATE,
mask_sh),\
+ SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC0_IMMEDIATE_UPDATE,
mask_sh),\
+ SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC1_IMMEDIATE_UPDATE,
mask_sh),\
+ SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC2_IMMEDIATE_UPDATE,
mask_sh),\
+ SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC3_IMMEDIATE_UPDATE,
mask_sh),\
SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC4_IMMEDIATE_UPDATE,
mask_sh),\
+ SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC5_IMMEDIATE_UPDATE,
mask_sh),\
+ SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC6_IMMEDIATE_UPDATE,
mask_sh),\
+ SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC7_IMMEDIATE_UPDATE,
mask_sh),\
SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC5_FRAME_UPDATE,
mask_sh),\
SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC6_FRAME_UPDATE,
mask_sh),\
SE_SF(DIG0_AFMT_VBI_PACKET_CONTROL1, AFMT_GENERIC7_FRAME_UPDATE,
mask_sh),\
@@ -345,7 +352,14 @@ struct dcn10_stream_enc_registers {
type AFMT_GENERIC2_FRAME_UPDATE;\
type AFMT_GENERIC3_FRAME_UPDATE;\
type AFMT_GENERIC4_FRAME_UPDATE;\
+ type AFMT_GENERIC0_IMMEDIATE_UPDATE;\
+ type AFMT_GENERIC1_IMMEDIATE_UPDATE;\
+ type AFMT_GENERIC2_IMMEDIATE_UPDATE;\
+ type AFMT_GENERIC3_IMMEDIATE_UPDATE;\
type AFMT_GENERIC4_IMMEDIATE_UPDATE;\
+ type AFMT_GENERIC5_IMMEDIATE_UPDATE;\
+ type AFMT_GENERIC6_IMMEDIATE_UPDATE;\
+ type AFMT_GENERIC7_IMMEDIATE_UPDATE;\
[PATCH 2/9] drm/amd/display: Fix incorrect backlight register offset for DCN
From: Aric Cyr [Why] Typo in backlight refactor inctroduced wrong register offset. [How] Change DCE to DCN register map for PWRSEQ_REF_DIV Cc: sta...@vger.kernel.org Signed-off-by: Aric Cyr Reviewed-by: Ashley Thomas Acked-by: Qingqing Zhuo --- drivers/gpu/drm/amd/display/dc/dce/dce_panel_cntl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_panel_cntl.h b/drivers/gpu/drm/amd/display/dc/dce/dce_panel_cntl.h index 70ec691e14d2..99c68ca9c7e0 100644 --- a/drivers/gpu/drm/amd/display/dc/dce/dce_panel_cntl.h +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_panel_cntl.h @@ -49,7 +49,7 @@ #define DCN_PANEL_CNTL_REG_LIST()\ DCN_PANEL_CNTL_SR(PWRSEQ_CNTL, LVTMA), \ DCN_PANEL_CNTL_SR(PWRSEQ_STATE, LVTMA), \ - DCE_PANEL_CNTL_SR(PWRSEQ_REF_DIV, LVTMA), \ + DCN_PANEL_CNTL_SR(PWRSEQ_REF_DIV, LVTMA), \ SR(BL_PWM_CNTL), \ SR(BL_PWM_CNTL2), \ SR(BL_PWM_PERIOD_CNTL), \ -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 7/9] drm/amd/display: Disconnect pipe separetely when disable pipe split
From: Alvin Lee
[Why]
When changing pixel formats for HDR (e.g. ARGB -> FP16)
there are configurations that change from 2 pipes to 1 pipe.
In these cases, it seems that disconnecting MPCC and doing
a surface update at the same time(after unlocking) causes
some registers to be updated slightly faster than others
after unlocking (e.g. if the pixel format is updated to FP16
before the new surface address is programmed, we get
corruption on the screen because the pixel formats aren't
matching). We separate disconnecting MPCC from the rest
of the pipe programming sequence to prevent this.
[How]
Move MPCC disconnect into separate operation than the
rest of the pipe programming.
Signed-off-by: Alvin Lee
Reviewed-by: Aric Cyr
Acked-by: Qingqing Zhuo
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 10 ++
.../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 146 ++
.../amd/display/dc/dcn10/dcn10_hw_sequencer.h | 6 +
.../gpu/drm/amd/display/dc/dcn10/dcn10_init.c | 2 +
.../gpu/drm/amd/display/dc/dcn20/dcn20_init.c | 2 +
.../gpu/drm/amd/display/dc/dcn21/dcn21_init.c | 2 +
.../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 2 +
.../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 4 +
8 files changed, 174 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 5aa3b89471c3..ebbb8182228d 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -2300,6 +2300,7 @@ static void commit_planes_for_stream(struct dc *dc,
enum surface_update_type update_type,
struct dc_state *context)
{
+ bool mpcc_disconnected = false;
int i, j;
struct pipe_ctx *top_pipe_to_program = NULL;
@@ -2330,6 +2331,15 @@ static void commit_planes_for_stream(struct dc *dc,
context_clock_trace(dc, context);
}
+ if (update_type != UPDATE_TYPE_FAST &&
dc->hwss.interdependent_update_lock &&
+ dc->hwss.disconnect_pipes && dc->hwss.wait_for_pending_cleared){
+ dc->hwss.interdependent_update_lock(dc, context, true);
+ mpcc_disconnected = dc->hwss.disconnect_pipes(dc, context);
+ dc->hwss.interdependent_update_lock(dc, context, false);
+ if (mpcc_disconnected)
+ dc->hwss.wait_for_pending_cleared(dc, context);
+ }
+
for (j = 0; j < dc->res_pool->pipe_count; j++) {
struct pipe_ctx *pipe_ctx = >res_ctx.pipe_ctx[j];
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index 57cd52789606..95e9d05f884b 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -2752,6 +2752,152 @@ static struct pipe_ctx *dcn10_find_top_pipe_for_stream(
return NULL;
}
+bool dcn10_disconnect_pipes(
+ struct dc *dc,
+ struct dc_state *context)
+{
+ bool found_stream = false;
+ int i, j;
+ struct dce_hwseq *hws = dc->hwseq;
+ struct dc_state *old_ctx = dc->current_state;
+ bool mpcc_disconnected = false;
+ struct pipe_ctx *old_pipe;
+ struct pipe_ctx *new_pipe;
+ DC_LOGGER_INIT(dc->ctx->logger);
+
+ /* Set pipe update flags and lock pipes */
+ for (i = 0; i < dc->res_pool->pipe_count; i++) {
+ old_pipe = >current_state->res_ctx.pipe_ctx[i];
+ new_pipe = >res_ctx.pipe_ctx[i];
+ new_pipe->update_flags.raw = 0;
+
+ if (!old_pipe->plane_state && !new_pipe->plane_state)
+ continue;
+
+ if (old_pipe->plane_state && !new_pipe->plane_state)
+ new_pipe->update_flags.bits.disable = 1;
+
+ /* Check for scl update */
+ if (memcmp(_pipe->plane_res.scl_data,
_pipe->plane_res.scl_data, sizeof(struct scaler_data)))
+ new_pipe->update_flags.bits.scaler = 1;
+
+ /* Check for vp update */
+ if (memcmp(_pipe->plane_res.scl_data.viewport,
_pipe->plane_res.scl_data.viewport, sizeof(struct rect))
+ ||
memcmp(_pipe->plane_res.scl_data.viewport_c,
+
_pipe->plane_res.scl_data.viewport_c, sizeof(struct rect)))
+ new_pipe->update_flags.bits.viewport = 1;
+
+ }
+
+ if (!IS_DIAG_DC(dc->ctx->dce_environment)) {
+ /* Disconnect mpcc here only if losing pipe split*/
+ for (i = 0; i < dc->res_pool->pipe_count; i++) {
+ if
[PATCH 1/9] drm/amd/display: Adjust static-ness of resource functions
From: Joshua Aberback
[Why]
Register definitions are asic-specific, so functions that use registers of
a particular asic should be static, to be exposed in asic-specific function
pointer structures.
[How]
- make register-definition-using functions static
- make some functions non-static, for future use
- remove duplicate function definition
Signed-off-by: Joshua Aberback
Reviewed-by: Nicholas Kazlauskas
Acked-by: Qingqing Zhuo
---
.../drm/amd/display/dc/dcn20/dcn20_resource.h | 1 -
.../drm/amd/display/dc/dcn30/dcn30_resource.c | 27 ++-
.../drm/amd/display/dc/dcn30/dcn30_resource.h | 3 +++
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
index 2c1959845c29..cdd39ee9761d 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
@@ -95,7 +95,6 @@ struct display_stream_compressor *dcn20_dsc_create(
struct dc_context *ctx, uint32_t inst);
void dcn20_dsc_destroy(struct display_stream_compressor **dsc);
-void dcn20_patch_bounding_box(struct dc *dc, struct
_vcs_dpi_soc_bounding_box_st *bb);
void dcn20_cap_soc_clocks(
struct _vcs_dpi_soc_bounding_box_st *bb,
struct pp_smu_nv_clock_table max_clocks);
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
index 653a571e366d..de53d26f61d5 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
@@ -871,7 +871,7 @@ void dcn30_dpp_destroy(struct dpp **dpp)
*dpp = NULL;
}
-struct dpp *dcn30_dpp_create(
+static struct dpp *dcn30_dpp_create(
struct dc_context *ctx,
uint32_t inst)
{
@@ -889,7 +889,8 @@ struct dpp *dcn30_dpp_create(
kfree(dpp);
return NULL;
}
-struct output_pixel_processor *dcn30_opp_create(
+
+static struct output_pixel_processor *dcn30_opp_create(
struct dc_context *ctx, uint32_t inst)
{
struct dcn20_opp *opp =
@@ -905,7 +906,7 @@ struct output_pixel_processor *dcn30_opp_create(
return >base;
}
-struct dce_aux *dcn30_aux_engine_create(
+static struct dce_aux *dcn30_aux_engine_create(
struct dc_context *ctx,
uint32_t inst)
{
@@ -924,6 +925,7 @@ struct dce_aux *dcn30_aux_engine_create(
return _engine->base;
}
+
#define i2c_inst_regs(id) { I2C_HW_ENGINE_COMMON_REG_LIST(id) }
static const struct dce_i2c_registers i2c_hw_regs[] = {
@@ -943,7 +945,7 @@ static const struct dce_i2c_mask i2c_masks = {
I2C_COMMON_MASK_SH_LIST_DCN2(_MASK)
};
-struct dce_i2c_hw *dcn30_i2c_hw_create(
+static struct dce_i2c_hw *dcn30_i2c_hw_create(
struct dc_context *ctx,
uint32_t inst)
{
@@ -958,6 +960,7 @@ struct dce_i2c_hw *dcn30_i2c_hw_create(
return dce_i2c_hw;
}
+
static struct mpc *dcn30_mpc_create(
struct dc_context *ctx,
int num_mpcc,
@@ -1008,7 +1011,7 @@ struct hubbub *dcn30_hubbub_create(struct dc_context *ctx)
return >base;
}
-struct timing_generator *dcn30_timing_generator_create(
+static struct timing_generator *dcn30_timing_generator_create(
struct dc_context *ctx,
uint32_t instance)
{
@@ -1042,7 +1045,7 @@ static const struct encoder_feature_support
link_enc_feature = {
.flags.bits.IS_TPS4_CAPABLE = true
};
-struct link_encoder *dcn30_link_encoder_create(
+static struct link_encoder *dcn30_link_encoder_create(
const struct encoder_init_data *enc_init_data)
{
struct dcn20_link_encoder *enc20 =
@@ -1063,7 +1066,7 @@ struct link_encoder *dcn30_link_encoder_create(
return >enc10.base;
}
-struct panel_cntl *dcn30_panel_cntl_create(const struct panel_cntl_init_data
*init_data)
+static struct panel_cntl *dcn30_panel_cntl_create(const struct
panel_cntl_init_data *init_data)
{
struct dce_panel_cntl *panel_cntl =
kzalloc(sizeof(struct dce_panel_cntl), GFP_KERNEL);
@@ -1311,7 +1314,7 @@ static void dcn30_resource_destruct(struct
dcn30_resource_pool *pool)
dcn_dccg_destroy(>base.dccg);
}
-struct hubp *dcn30_hubp_create(
+static struct hubp *dcn30_hubp_create(
struct dc_context *ctx,
uint32_t inst)
{
@@ -1330,7 +1333,7 @@ struct hubp *dcn30_hubp_create(
return NULL;
}
-bool dcn30_dwbc_create(struct dc_context *ctx, struct resource_pool *pool)
+static bool dcn30_dwbc_create(struct dc_context *ctx, struct resource_pool
*pool)
{
int i;
uint32_t pipe_count = pool->res_cap->num_dwb;
@@ -1355,7 +1358,7 @@ bool dcn30_dwbc_create(struct dc_context *ctx, struct
resource_pool *pool)
return true;
}
-bool dcn30_mmhubbub_create(struct dc_context *ctx, struct resource_pool *pool)
+static bool
[PATCH 3/9] drm/amd/display: Revert regression
From: Alvin Lee
[Why]
Caused pipe split regression
Signed-off-by: Alvin Lee
Reviewed-by: Aric Cyr
Acked-by: Qingqing Zhuo
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 10 --
.../drm/amd/display/dc/dcn20/dcn20_hwseq.c| 114 --
.../drm/amd/display/dc/dcn20/dcn20_hwseq.h| 7 --
.../gpu/drm/amd/display/dc/dcn20/dcn20_init.c | 2 -
.../gpu/drm/amd/display/dc/dcn21/dcn21_init.c | 2 -
.../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 2 -
.../gpu/drm/amd/display/dc/inc/hw_sequencer.h | 4 -
7 files changed, 141 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index ebbb8182228d..5aa3b89471c3 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -2300,7 +2300,6 @@ static void commit_planes_for_stream(struct dc *dc,
enum surface_update_type update_type,
struct dc_state *context)
{
- bool mpcc_disconnected = false;
int i, j;
struct pipe_ctx *top_pipe_to_program = NULL;
@@ -2331,15 +2330,6 @@ static void commit_planes_for_stream(struct dc *dc,
context_clock_trace(dc, context);
}
- if (update_type != UPDATE_TYPE_FAST &&
dc->hwss.interdependent_update_lock &&
- dc->hwss.disconnect_pipes && dc->hwss.wait_for_pending_cleared){
- dc->hwss.interdependent_update_lock(dc, context, true);
- mpcc_disconnected = dc->hwss.disconnect_pipes(dc, context);
- dc->hwss.interdependent_update_lock(dc, context, false);
- if (mpcc_disconnected)
- dc->hwss.wait_for_pending_cleared(dc, context);
- }
-
for (j = 0; j < dc->res_pool->pipe_count; j++) {
struct pipe_ctx *pipe_ctx = >res_ctx.pipe_ctx[j];
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
index 73eb4e76a0b1..66180b4332f1 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
@@ -1624,120 +1624,6 @@ static void dcn20_program_pipe(
}
}
-bool dcn20_disconnect_pipes(
- struct dc *dc,
- struct dc_state *context)
-{
- int i;
- struct dce_hwseq *hws = dc->hwseq;
- bool mpcc_disconnected = false;
- DC_LOGGER_INIT(dc->ctx->logger);
-
- /* Set pipe update flags and lock pipes */
- for (i = 0; i < dc->res_pool->pipe_count; i++)
-
dcn20_detect_pipe_changes(>current_state->res_ctx.pipe_ctx[i],
- >res_ctx.pipe_ctx[i]);
-
- if (!IS_DIAG_DC(dc->ctx->dce_environment)) {
- /* OTG blank before disabling all front ends */
- for (i = 0; i < dc->res_pool->pipe_count; i++) {
- if
(context->res_ctx.pipe_ctx[i].update_flags.bits.disable
- &&
!context->res_ctx.pipe_ctx[i].top_pipe
- &&
!context->res_ctx.pipe_ctx[i].prev_odm_pipe
- && context->res_ctx.pipe_ctx[i].stream)
{
- hws->funcs.blank_pixel_data(dc,
>res_ctx.pipe_ctx[i], true);
- }
- }
-
- /* Disconnect mpcc */
- for (i = 0; i < dc->res_pool->pipe_count; i++) {
- if
(context->res_ctx.pipe_ctx[i].update_flags.bits.disable) {
- hws->funcs.plane_atomic_disconnect(dc,
>current_state->res_ctx.pipe_ctx[i]);
- DC_LOG_DC("Reset mpcc for pipe %d\n",
dc->current_state->res_ctx.pipe_ctx[i].pipe_idx);
- mpcc_disconnected = true;
- }
- }
- }
-
- if (mpcc_disconnected) {
- for (i = 0; i < dc->res_pool->pipe_count; i++) {
- struct pipe_ctx *pipe_ctx =
>res_ctx.pipe_ctx[i];
- struct pipe_ctx *old_pipe =
>current_state->res_ctx.pipe_ctx[i];
- struct dc_plane_state *plane_state =
pipe_ctx->plane_state;
- struct hubp *hubp = pipe_ctx->plane_res.hubp;
-
- if (!pipe_ctx || !plane_state ||
!pipe_ctx->stream)
- continue;
-
- // Only update scaler and viewport here if we lose a
pipe split.
- // This is to prevent half the screen from being black
when we
- // unlock after disconnecting MPCC.
- if (!(old_pipe && !pipe_ctx->top_pipe &&
-
[PATCH 8/9] drm/amd/display: Fix EDID parsing after resume from suspend
From: Stylon Wang [Why] Resuming from suspend, CEA blocks from EDID are not parsed and no video modes can support YUV420. When this happens, output bpc cannot go over 8-bit with 4K modes on HDMI. [How] In amdgpu_dm_update_connector_after_detect(), drm_add_edid_modes() is called after drm_connector_update_edid_property() to fully parse EDID and update display info. Cc: sta...@vger.kernel.org Signed-off-by: Stylon Wang Reviewed-by: Nicholas Kazlauskas Acked-by: Qingqing Zhuo --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 53bf8f60d30c..bfb06c168fba 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2202,6 +2202,7 @@ void amdgpu_dm_update_connector_after_detect( drm_connector_update_edid_property(connector, aconnector->edid); + drm_add_edid_modes(connector, aconnector->edid); if (aconnector->dc_link->aux_mode) drm_dp_cec_set_edid(>dm_dp_aux.aux, -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 9/9] drm/amd/display: Blank stream before destroying HDCP session
From: Jaehyun Chung [Why] Stream disable sequence incorretly destroys HDCP session while stream is not blanked and while audio is not muted. This sequence causes a flash of corruption during mode change and an audio click. [How] Change sequence to blank stream before destroying HDCP session. Audio will also be muted by blanking the stream. Cc: sta...@vger.kernel.org Signed-off-by: Jaehyun Chung Reviewed-by: Alvin Lee Acked-by: Qingqing Zhuo --- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c b/drivers/gpu/drm/amd/display/dc/core/dc_link.c index 4bd6e03a7ef3..117d8aaf2a9b 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c @@ -3286,12 +3286,11 @@ void core_link_disable_stream(struct pipe_ctx *pipe_ctx) core_link_set_avmute(pipe_ctx, true); } + dc->hwss.blank_stream(pipe_ctx); #if defined(CONFIG_DRM_AMD_DC_HDCP) update_psp_stream_config(pipe_ctx, true); #endif - dc->hwss.blank_stream(pipe_ctx); - if (pipe_ctx->stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST) deallocate_mst_payload(pipe_ctx); -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 4/9] drm/amd/display: mpcc black color should not be impacted by pixel encoding format
From: Xiaodong Yan [Why] The format in MPCC should be 444 [How] do not modify the mpcc black color according to pixel encoding format Signed-off-by: Xiaodong Yan Reviewed-by: Eric Yang Acked-by: Qingqing Zhuo --- drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c index a643927e272b..57cd52789606 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c @@ -2368,14 +2368,6 @@ void dcn10_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx) _cfg.black_color); } - /* -* The way 420 is packed, 2 channels carry Y component, 1 channel -* alternate between Cb and Cr, so both channels need the pixel -* value for Y -*/ - if (pipe_ctx->stream->timing.pixel_encoding == PIXEL_ENCODING_YCBCR420) - blnd_cfg.black_color.color_r_cr = blnd_cfg.black_color.color_g_y; - if (per_pixel_alpha) blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA; else -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 5/9] drm/amd/display: Fix LFC multiplier changing erratically
From: Anthony Koo
[Why]
1. There is a calculation that is using frame_time_in_us instead of
last_render_time_in_us to calculate whether choosing an LFC multiplier
would cause the inserted frame duration to be outside of range.
2. We do not handle unsigned integer subtraction correctly and it underflows
to a really large value, which causes some logic errors.
[How]
1. Fix logic to calculate 'within range' using last_render_time_in_us
2. Split out delta_from_mid_point_delta_in_us calculation to ensure
we don't underflow and wrap around
Signed-off-by: Anthony Koo
Reviewed-by: Aric Cyr
Acked-by: Qingqing Zhuo
---
.../amd/display/modules/freesync/freesync.c | 36 +++
1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
index 81820f3d6b3b..d988533d4af5 100644
--- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
+++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
@@ -324,22 +324,44 @@ static void apply_below_the_range(struct core_freesync
*core_freesync,
/* Choose number of frames to insert based on how close it
* can get to the mid point of the variable range.
+* - Delta for CEIL: delta_from_mid_point_in_us_1
+* - Delta for FLOOR: delta_from_mid_point_in_us_2
*/
- if ((frame_time_in_us / mid_point_frames_ceil) >
in_out_vrr->min_duration_in_us &&
- (delta_from_mid_point_in_us_1 <
delta_from_mid_point_in_us_2 ||
- mid_point_frames_floor < 2)) {
+ if ((last_render_time_in_us / mid_point_frames_ceil) <
in_out_vrr->min_duration_in_us) {
+ /* Check for out of range.
+* If using CEIL produces a value that is out of range,
+* then we are forced to use FLOOR.
+*/
+ frames_to_insert = mid_point_frames_floor;
+ } else if (mid_point_frames_floor < 2) {
+ /* Check if FLOOR would result in non-LFC. In this case
+* choose to use CEIL
+*/
+ frames_to_insert = mid_point_frames_ceil;
+ } else if (delta_from_mid_point_in_us_1 <
delta_from_mid_point_in_us_2) {
+ /* If choosing CEIL results in a frame duration that is
+* closer to the mid point of the range.
+* Choose CEIL
+*/
frames_to_insert = mid_point_frames_ceil;
- delta_from_mid_point_delta_in_us =
delta_from_mid_point_in_us_2 -
- delta_from_mid_point_in_us_1;
} else {
+ /* If choosing FLOOR results in a frame duration that is
+* closer to the mid point of the range.
+* Choose FLOOR
+*/
frames_to_insert = mid_point_frames_floor;
- delta_from_mid_point_delta_in_us =
delta_from_mid_point_in_us_1 -
- delta_from_mid_point_in_us_2;
}
/* Prefer current frame multiplier when BTR is enabled unless
it drifts
* too far from the midpoint
*/
+ if (delta_from_mid_point_in_us_1 <
delta_from_mid_point_in_us_2) {
+ delta_from_mid_point_delta_in_us =
delta_from_mid_point_in_us_2 -
+ delta_from_mid_point_in_us_1;
+ } else {
+ delta_from_mid_point_delta_in_us =
delta_from_mid_point_in_us_1 -
+ delta_from_mid_point_in_us_2;
+ }
if (in_out_vrr->btr.frames_to_insert != 0 &&
delta_from_mid_point_delta_in_us <
BTR_DRIFT_MARGIN) {
if (((last_render_time_in_us /
in_out_vrr->btr.frames_to_insert) <
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 0/9] DC Patches August 10th, 2020
This DC patchset brings improvements in multiple areas. In summary, we highlight: * Fixes on LFC, pipe split, register mapping and others. * Code clean-up. Alvin Lee (2): drm/amd/display: Revert regression drm/amd/display: Disconnect pipe separetely when disable pipe split Anthony Koo (2): drm/amd/display: Fix LFC multiplier changing erratically drm/amd/display: Switch to immediate mode for updating infopackets Aric Cyr (1): drm/amd/display: Fix incorrect backlight register offset for DCN Jaehyun Chung (1): drm/amd/display: Blank stream before destroying HDCP session Joshua Aberback (1): drm/amd/display: Adjust static-ness of resource functions Stylon Wang (1): drm/amd/display: Fix EDID parsing after resume from suspend Xiaodong Yan (1): drm/amd/display: mpcc black color should not be impacted by pixel encoding format .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 + drivers/gpu/drm/amd/display/dc/core/dc_link.c | 3 +- .../drm/amd/display/dc/dce/dce_panel_cntl.h | 2 +- .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 154 +- .../amd/display/dc/dcn10/dcn10_hw_sequencer.h | 6 + .../gpu/drm/amd/display/dc/dcn10/dcn10_init.c | 2 + .../display/dc/dcn10/dcn10_stream_encoder.c | 16 +- .../display/dc/dcn10/dcn10_stream_encoder.h | 14 ++ .../drm/amd/display/dc/dcn20/dcn20_hwseq.c| 114 - .../drm/amd/display/dc/dcn20/dcn20_hwseq.h| 7 - .../gpu/drm/amd/display/dc/dcn20/dcn20_init.c | 4 +- .../drm/amd/display/dc/dcn20/dcn20_resource.h | 1 - .../gpu/drm/amd/display/dc/dcn21/dcn21_init.c | 4 +- .../gpu/drm/amd/display/dc/dcn30/dcn30_init.c | 4 +- .../drm/amd/display/dc/dcn30/dcn30_resource.c | 27 +-- .../drm/amd/display/dc/dcn30/dcn30_resource.h | 3 + .../amd/display/modules/freesync/freesync.c | 36 +++- 17 files changed, 232 insertions(+), 166 deletions(-) -- 2.17.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] SWDEV-220451 - Query guest's information by VF2PF message - Guest side - part 1
- Add guest side change to support VF2PF message
- Fix coding style
Change-Id: I82e5518cb10ec0b19fecaba7e05b02f4b7f2b409
Signed-off-by: Bokun Zhang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h| 29 +-
drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h | 276
2 files changed, 285 insertions(+), 20 deletions(-)
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
index b0b2bdc750df..ad2b2628ab67 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
@@ -24,6 +24,8 @@
#ifndef AMDGPU_VIRT_H
#define AMDGPU_VIRT_H
+#include "amdgv_sriovmsg.h"
+
#define AMDGPU_SRIOV_CAPS_SRIOV_VBIOS (1 << 0) /* vBIOS is sr-iov ready */
#define AMDGPU_SRIOV_CAPS_ENABLE_IOV (1 << 1) /* sr-iov is enabled on this
GPU */
#define AMDGPU_SRIOV_CAPS_IS_VF(1 << 2) /* this GPU is a virtual
function */
@@ -69,7 +71,10 @@ struct amdgpu_virt_fw_reserve {
struct amd_sriov_msg_vf2pf_info_header *p_vf2pf;
unsigned int checksum_key;
};
+
/*
+ * Legacy GIM header
+ *
* Defination between PF and VF
* Structures forcibly aligned to 4 to keep the same style as PF.
*/
@@ -89,15 +94,7 @@ enum AMDGIM_FEATURE_FLAG {
AMDGIM_FEATURE_HW_PERF_SIMULATION = (1 << 3),
};
-struct amd_sriov_msg_pf2vf_info_header {
- /* the total structure size in byte. */
- uint32_t size;
- /* version of this structure, written by the GIM */
- uint32_t version;
- /* reserved */
- uint32_t reserved[2];
-} __aligned(4);
-struct amdgim_pf2vf_info_v1 {
+struct amdgim_pf2vf_info_v1 {
/* header contains size and version */
struct amd_sriov_msg_pf2vf_info_header header;
/* max_width * max_height */
@@ -116,6 +113,7 @@ struct amdgim_pf2vf_info_v1 {
unsigned int checksum;
} __aligned(4);
+/* TODO: below struct is duplicated to amd_sriov_msg_pf2vf_info */
struct amdgim_pf2vf_info_v2 {
/* header contains size and version */
struct amd_sriov_msg_pf2vf_info_header header;
@@ -146,16 +144,6 @@ struct amdgim_pf2vf_info_v2 {
uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256, 0, 0, (9 +
sizeof(struct amd_sriov_msg_pf2vf_info_header)/sizeof(uint32_t)), 3)];
} __aligned(4);
-
-struct amd_sriov_msg_vf2pf_info_header {
- /* the total structure size in byte. */
- uint32_t size;
- /*version of this structure, written by the guest */
- uint32_t version;
- /* reserved */
- uint32_t reserved[2];
-} __aligned(4);
-
struct amdgim_vf2pf_info_v1 {
/* header contains size and version */
struct amd_sriov_msg_vf2pf_info_header header;
@@ -217,8 +205,9 @@ struct amdgim_vf2pf_info_v2 {
uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256, 64, 0, (12 +
sizeof(struct amd_sriov_msg_vf2pf_info_header)/sizeof(uint32_t)), 0)];
} __aligned(4);
+/* TODO: below macro and typedef will cause compile error, need to remove */
#define AMDGPU_FW_VRAM_VF2PF_VER 2
-typedef struct amdgim_vf2pf_info_v2 amdgim_vf2pf_info ;
+typedef struct amd_sriov_msg_vf2pf_info amdgim_vf2pf_info;
#define AMDGPU_FW_VRAM_VF2PF_WRITE(adev, field, val) \
do { \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
new file mode 100644
index ..5355827ed0ae
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgv_sriovmsg.h
@@ -0,0 +1,276 @@
+/*
+ * Copyright 2018-2019 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.
+ *
+ */
+
+#ifndef AMDGV_SRIOV_MSG__H_
+#define AMDGV_SRIOV_MSG__H_
+
+/* unit in kilobytes */
+#define AMD_SRIOV_MSG_VBIOS_OFFSET 0
+#define AMD_SRIOV_MSG_VBIOS_SIZE_KB 64
+#define AMD_SRIOV_MSG_DATAEXCHANGE_OFFSET_KBAMD_SRIOV_MSG_VBIOS_SIZE_KB
+#define
Re: [PATCH] drm/amd/mst: clean DP main link status only when unplug mst 1st link
On 08/04, Wayne Lin wrote: > [Why] > Under DP daisy chain scenario as below: > > Src - Monitor_1 - Monitor_2 > > If unplug 2nd Monitor_2 and plug in again, observe that Monitor_1 > doesn't light up. > > When unplug 2nd monitor, we clear the > dc_link->cur_link_settings.lane_count in dm_dp_destroy_mst_connector(). > However this link status is a shared data structure by all connected mst > monitors. Although the 2nd monitor is gone, this link status should > still be retained for other connected mst monitors. Otherwise, when we > plug the 2nd monitor in again, we find out that main link is not trained > and do link training again. Payload ID Table for Monitor_1 is ruined and > we don't reallocate it. > > [How] > In dm_dp_destroy_mst_connector(), only clean the cur_link_settings when > we no longer do mst mode. > > Signed-off-by: Wayne Lin > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > index 2c10352fa514..526f29598403 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > @@ -415,7 +415,10 @@ static void dm_dp_destroy_mst_connector(struct > drm_dp_mst_topology_mgr *mgr, > aconnector->dc_sink); > dc_sink_release(aconnector->dc_sink); > aconnector->dc_sink = NULL; > - aconnector->dc_link->cur_link_settings.lane_count = 0; > + mutex_lock(>lock); > + if (!mgr->mst_state) > + aconnector->dc_link->cur_link_settings.lane_count = 0; > + mutex_unlock(>lock); Hi Wayne, The change looks good to me. Reviewed-by: Rodrigo Siqueira Just for curiosity, do you know why we use a mutex instead of spin_lock_irq for this case? FWIU, the spin_lock_irq behavior looks better for this sort of manipulation. Thanks > } > > drm_connector_unregister(connector); > -- > 2.17.1 > -- Rodrigo Siqueira https://siqueira.tech signature.asc Description: PGP signature ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Amdgpu kernel oops and freezing graphics
Alex, Am 04.08.20 um 22:01 schrieb Alex Deucher: On Tue, Jul 21, 2020 at 2:21 PM Harvey wrote: Alex, tnak you so much - you're my hero! Am 21.07.20 um 18:17 schrieb Alex Deucher: On Mon, Jul 20, 2020 at 4:22 AM Harvey wrote: Hello, this is my first post to this list so please be patient with me ;) The facts: it is now one week that I own a new laptop, a MSI Bravo 17 A4DDR/MS-17FK with Ryzen 7 4800U and hybrid graphics on a Radeon RX 5500M. I installed my beloved Archlinux but I can't start any graphics withpout kernel oops on it beside the normal console, even calling 'lspci' on the console is provoking errors. I am using linux kernel 5.7.9 and linux-firmware 20200619.e96c121 (FWIW: I even tried with a self-cmpiled kernel 5.8-rc5 and linux-firmware directly from the git repository - no changes) The following is only part of the information I can provide but I didn't want to make this mail bigger than it already is. Does appending amdgpu.runpm=0 on the kernel command line in grub help? Yes it does. Woohoo! The system is not freezing anymore! Can I provide any further information to get this sorted? I will be happy to help investigating and testing if needed. Does appending pci=noats on the kernel command line in grub also fix the issue? No, it does not. I tested with pci=noats on kernel 5.7.11 and 5.8 and on both the kernel oopses and freezes again. Only amdgpu.runpm=0 does the trick. FWIW, booting kernel 5.8 without any parameters does not work either. Greetings Harvey -- I am root. If you see me laughing, you'd better have a backup! ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Enabling AMDGPU by default for SI & CIK
On Tue, Aug 4, 2020 at 10:48 PM Bridgman, John wrote: > > [AMD Official Use Only - Internal Distribution Only] > > > At the risk of asking a dumb question, does amdgpu default to using DC on SI > and CI ? > DC is disabled by default on SI and CI due to the lack of analog encoder support. > I'm asking because a lot of people seem to be using amdgpu successfully with > analog outputs today on SI/CI... suggests that they are not using DC ? Yes. > > If so then would enabling HDMI/DP audio support without DC be sufficient to > flip the switch assuming we felt that other risks were manageable ? Possibly. VCE support for SI is also missing in amdgpu. I think the biggest challenge will be making sure things like suspend and resume work. Alex > > Thanks, > John > > > From: amd-gfx on behalf of Alex > Deucher > Sent: August 4, 2020 1:35 PM > To: Michel Dänzer > Cc: Deucher, Alexander ; Koenig, Christian > ; amd-gfx mailing list > ; Bas Nieuwenhuizen > Subject: Re: Enabling AMDGPU by default for SI & CIK > > On Tue, Aug 4, 2020 at 4:38 AM Michel Dänzer wrote: > > > > On 2020-08-03 1:45 a.m., Bas Nieuwenhuizen wrote: > > > Hi all, > > > > > > Now that we have recently made some progress on getting feature parity > > > with the Radeon driver for SI, I'm wondering what it would take to > > > make AMDGPU the default driver for these generations. > > > > > > As far as I understand AMDGPU has had these features for CIK for a > > > while already but it is still not the default driver. What would it > > > take to make it the default? What is missing and/or broken? > > > > The main blockers I'm aware of for CIK are: > > > > 1) Lack of analogue connector support with DC > > 2) Lack of HDMI/DP audio support without DC > > > > > > 1) may apply to SI as well. > > Also, IIRC, there are suspend and resume problems with some CIK parts > using amdgpu. > > Alex > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cjohn.bridgman%40amd.com%7C26df81f9a6df4e2f9fbb08d8389cda79%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637321593620378481sdata=Ep%2BYRRT1dAcE8zSDIaZiXuVMb9gBVUnLnbtP1%2Be7Pkc%3Dreserved=0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH][next] drm/amdgpu: fix spelling mistake "paramter" -> "parameter"
From: Colin Ian King
There is a spelling mistake in a dev_warn message. Fix it.
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index b72aeeb0a226..16e23f053361 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1201,7 +1201,7 @@ static int amdgpu_device_check_arguments(struct
amdgpu_device *adev)
if (amdgpu_num_kcq > 8 || amdgpu_num_kcq < 0) {
amdgpu_num_kcq = 8;
- dev_warn(adev->dev, "set kernel compute queue number to 8 due
to invalid paramter provided by user\n");
+ dev_warn(adev->dev, "set kernel compute queue number to 8 due
to invalid parameter provided by user\n");
}
return 0;
--
2.27.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
From: Colin Ian King
There is a spelling mistake in a DRM_ERROR message. Fix it.
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 49d4514aa6ed..c68369731b20 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle)
ret = psp_tmr_terminate(psp);
if (ret) {
- DRM_ERROR("Falied to terminate tmr\n");
+ DRM_ERROR("Failed to terminate tmr\n");
return ret;
}
--
2.27.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/2] drm/amd/powerplay: put VCN/JPEG into PG ungate state before dpm table setup(V3)
As VCN related dpm table setup needs VCN be in PG ungate state. Same logics
applies to JPEG.
V2: fix paste typo
V3: code cosmetic
Change-Id: I0e4d97ebedc132b7d793dc3f36275066ff999eac
Signed-off-by: Evan Quan
Tested-by: Matt Coffin
Reviewed-by: Alex Deucher
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 89 +---
drivers/gpu/drm/amd/powerplay/smu_internal.h | 1 -
2 files changed, 75 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 1b64ca9ecccb..1ced80238f8a 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -133,6 +133,23 @@ int smu_get_dpm_freq_range(struct smu_context *smu,
return ret;
}
+static int smu_dpm_set_vcn_enable_locked(struct smu_context *smu,
+bool enable)
+{
+ struct smu_power_context *smu_power = >smu_power;
+ struct smu_power_gate *power_gate = _power->power_gate;
+ int ret = 0;
+
+ if (atomic_read(_gate->vcn_gated) ^ enable)
+ return 0;
+
+ ret = smu->ppt_funcs->dpm_set_vcn_enable(smu, enable);
+ if (!ret)
+ atomic_set(_gate->vcn_gated, !enable);
+
+ return ret;
+}
+
static int smu_dpm_set_vcn_enable(struct smu_context *smu,
bool enable)
{
@@ -145,19 +162,30 @@ static int smu_dpm_set_vcn_enable(struct smu_context *smu,
mutex_lock(_gate->vcn_gate_lock);
- if (atomic_read(_gate->vcn_gated) ^ enable)
- goto out;
-
- ret = smu->ppt_funcs->dpm_set_vcn_enable(smu, enable);
- if (!ret)
- atomic_set(_gate->vcn_gated, !enable);
+ ret = smu_dpm_set_vcn_enable_locked(smu, enable);
-out:
mutex_unlock(_gate->vcn_gate_lock);
return ret;
}
+static int smu_dpm_set_jpeg_enable_locked(struct smu_context *smu,
+ bool enable)
+{
+ struct smu_power_context *smu_power = >smu_power;
+ struct smu_power_gate *power_gate = _power->power_gate;
+ int ret = 0;
+
+ if (atomic_read(_gate->jpeg_gated) ^ enable)
+ return 0;
+
+ ret = smu->ppt_funcs->dpm_set_jpeg_enable(smu, enable);
+ if (!ret)
+ atomic_set(_gate->jpeg_gated, !enable);
+
+ return ret;
+}
+
static int smu_dpm_set_jpeg_enable(struct smu_context *smu,
bool enable)
{
@@ -170,14 +198,8 @@ static int smu_dpm_set_jpeg_enable(struct smu_context *smu,
mutex_lock(_gate->jpeg_gate_lock);
- if (atomic_read(_gate->jpeg_gated) ^ enable)
- goto out;
-
- ret = smu->ppt_funcs->dpm_set_jpeg_enable(smu, enable);
- if (!ret)
- atomic_set(_gate->jpeg_gated, !enable);
+ ret = smu_dpm_set_jpeg_enable_locked(smu, enable);
-out:
mutex_unlock(_gate->jpeg_gate_lock);
return ret;
@@ -403,6 +425,45 @@ static int smu_early_init(void *handle)
return smu_set_funcs(adev);
}
+static int smu_set_default_dpm_table(struct smu_context *smu)
+{
+ struct smu_power_context *smu_power = >smu_power;
+ struct smu_power_gate *power_gate = _power->power_gate;
+ int vcn_gate, jpeg_gate;
+ int ret = 0;
+
+ if (!smu->ppt_funcs->set_default_dpm_table)
+ return 0;
+
+ mutex_lock(_gate->vcn_gate_lock);
+ mutex_lock(_gate->jpeg_gate_lock);
+
+ vcn_gate = atomic_read(_gate->vcn_gated);
+ jpeg_gate = atomic_read(_gate->jpeg_gated);
+
+ ret = smu_dpm_set_vcn_enable_locked(smu, true);
+ if (ret)
+ goto err0_out;
+
+ ret = smu_dpm_set_jpeg_enable_locked(smu, true);
+ if (ret)
+ goto err1_out;
+
+ ret = smu->ppt_funcs->set_default_dpm_table(smu);
+ if (ret)
+ dev_err(smu->adev->dev,
+ "Failed to setup default dpm clock tables!\n");
+
+ smu_dpm_set_jpeg_enable_locked(smu, !jpeg_gate);
+err1_out:
+ smu_dpm_set_vcn_enable_locked(smu, !vcn_gate);
+err0_out:
+ mutex_unlock(_gate->jpeg_gate_lock);
+ mutex_unlock(_gate->vcn_gate_lock);
+
+ return ret;
+}
+
static int smu_late_init(void *handle)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
diff --git a/drivers/gpu/drm/amd/powerplay/smu_internal.h
b/drivers/gpu/drm/amd/powerplay/smu_internal.h
index f1d8f247e589..264073d4e263 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_internal.h
+++ b/drivers/gpu/drm/amd/powerplay/smu_internal.h
@@ -60,7 +60,6 @@
#define smu_disable_all_features_with_exception(smu, mask)
smu_ppt_funcs(disable_all_features_with_exception, 0, smu, mask)
#define smu_is_dpm_running(smu)
smu_ppt_funcs(is_dpm_running, 0 , smu)
#define smu_notify_display_change(smu)
[PATCH 1/2] drm/amd/powerplay: update swSMU VCN/JPEG PG logics
Add lock protections and avoid unnecessary actions
if the PG state is already the same as required.
Change-Id: I01400b84151d3ac6e3c8b0d7e264f9a68a9c2092
Signed-off-by: Evan Quan
Tested-by: Matt Coffin
Reviewed-by: Alex Deucher
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c| 57 ++-
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 4 --
.../gpu/drm/amd/powerplay/inc/amdgpu_smu.h| 6 +-
drivers/gpu/drm/amd/powerplay/navi10_ppt.c| 8 ---
drivers/gpu/drm/amd/powerplay/renoir_ppt.c| 8 ---
.../drm/amd/powerplay/sienna_cichlid_ppt.c| 9 ---
drivers/gpu/drm/amd/powerplay/smu_internal.h | 2 -
7 files changed, 60 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index f3f50b5add99..1b64ca9ecccb 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -133,6 +133,56 @@ int smu_get_dpm_freq_range(struct smu_context *smu,
return ret;
}
+static int smu_dpm_set_vcn_enable(struct smu_context *smu,
+ bool enable)
+{
+ struct smu_power_context *smu_power = >smu_power;
+ struct smu_power_gate *power_gate = _power->power_gate;
+ int ret = 0;
+
+ if (!smu->ppt_funcs->dpm_set_vcn_enable)
+ return 0;
+
+ mutex_lock(_gate->vcn_gate_lock);
+
+ if (atomic_read(_gate->vcn_gated) ^ enable)
+ goto out;
+
+ ret = smu->ppt_funcs->dpm_set_vcn_enable(smu, enable);
+ if (!ret)
+ atomic_set(_gate->vcn_gated, !enable);
+
+out:
+ mutex_unlock(_gate->vcn_gate_lock);
+
+ return ret;
+}
+
+static int smu_dpm_set_jpeg_enable(struct smu_context *smu,
+ bool enable)
+{
+ struct smu_power_context *smu_power = >smu_power;
+ struct smu_power_gate *power_gate = _power->power_gate;
+ int ret = 0;
+
+ if (!smu->ppt_funcs->dpm_set_jpeg_enable)
+ return 0;
+
+ mutex_lock(_gate->jpeg_gate_lock);
+
+ if (atomic_read(_gate->jpeg_gated) ^ enable)
+ goto out;
+
+ ret = smu->ppt_funcs->dpm_set_jpeg_enable(smu, enable);
+ if (!ret)
+ atomic_set(_gate->jpeg_gated, !enable);
+
+out:
+ mutex_unlock(_gate->jpeg_gate_lock);
+
+ return ret;
+}
+
/**
* smu_dpm_set_power_gate - power gate/ungate the specific IP block
*
@@ -650,6 +700,11 @@ static int smu_sw_init(void *handle)
smu->power_profile_mode = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
smu->default_power_profile_mode = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
+ atomic_set(>smu_power.power_gate.vcn_gated, 1);
+ atomic_set(>smu_power.power_gate.jpeg_gated, 1);
+ mutex_init(>smu_power.power_gate.vcn_gate_lock);
+ mutex_init(>smu_power.power_gate.jpeg_gate_lock);
+
smu->workload_mask = 1 <<
smu->workload_prority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT];
smu->workload_prority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT] = 0;
smu->workload_prority[PP_SMC_POWER_PROFILE_FULLSCREEN3D] = 1;
@@ -1974,7 +2029,7 @@ int smu_read_sensor(struct smu_context *smu,
*size = 4;
break;
case AMDGPU_PP_SENSOR_VCN_POWER_STATE:
- *(uint32_t *)data = smu->smu_power.power_gate.vcn_gated ? 0 : 1;
+ *(uint32_t *)data =
atomic_read(>smu_power.power_gate.vcn_gated) ? 0: 1;
*size = 4;
break;
case AMDGPU_PP_SENSOR_MIN_FAN_RPM:
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index e3a2d7f0aba1..e59e6fb6f0a8 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -1896,8 +1896,6 @@ static bool arcturus_is_dpm_running(struct smu_context
*smu)
static int arcturus_dpm_set_vcn_enable(struct smu_context *smu, bool enable)
{
- struct smu_power_context *smu_power = >smu_power;
- struct smu_power_gate *power_gate = _power->power_gate;
int ret = 0;
if (enable) {
@@ -1908,7 +1906,6 @@ static int arcturus_dpm_set_vcn_enable(struct smu_context
*smu, bool enable)
return ret;
}
}
- power_gate->vcn_gated = false;
} else {
if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_PG_BIT)) {
ret = smu_cmn_feature_set_enabled(smu,
SMU_FEATURE_VCN_PG_BIT, 0);
@@ -1917,7 +1914,6 @@ static int arcturus_dpm_set_vcn_enable(struct smu_context
*smu, bool enable)
return ret;
}
}
- power_gate->vcn_gated = true;
}
return ret;
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
index ec2d2aa7f4ec..23c2279bd500 100644
---
Re: [PATCH] drm/amdgpu: unlock mutex on error
Please remove change-Id before pushing.
Acked-by: Nirmoy Das
On 8/5/20 10:41 AM, Dennis Li wrote:
Make sure unlock the mutex when error happen
Signed-off-by: Dennis Li
Change-Id: I6c36a193df5fe70516282d8136b4eadf32d20915
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index a0ea663ecdbc..5e5369abc6fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -632,13 +632,14 @@ int amdgpu_amdkfd_submit_ib(struct kgd_dev *kgd, enum
kgd_engine_type engine,
}
ret = amdgpu_ib_schedule(ring, 1, ib, job, );
+
+ up_read(>reset_sem);
+
if (ret) {
DRM_ERROR("amdgpu: failed to schedule IB.\n");
goto err_ib_sched;
}
- up_read(>reset_sem);
-
ret = dma_fence_wait(f, false);
err_ib_sched:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 4e017f379eb6..67a756f4337b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -545,7 +545,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
}
ret = amdgpu_xgmi_update_topology(hive, tmp_adev);
if (ret)
- goto exit;
+ goto exit_unlock;
}
/* get latest topology info for each device from psp */
@@ -558,7 +558,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
tmp_adev->gmc.xgmi.node_id,
tmp_adev->gmc.xgmi.hive_id, ret);
/* To do : continue with some node failed or
disable the whole hive */
- goto exit;
+ goto exit_unlock;
}
}
}
@@ -566,7 +566,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
if (!ret)
ret = amdgpu_xgmi_sysfs_add_dev_info(adev, hive);
-
+exit_unlock:
mutex_unlock(>hive_lock);
exit:
if (!ret)
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: unlock mutex on error
Make sure unlock the mutex when error happen
Signed-off-by: Dennis Li
Change-Id: I6c36a193df5fe70516282d8136b4eadf32d20915
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index a0ea663ecdbc..5e5369abc6fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -632,13 +632,14 @@ int amdgpu_amdkfd_submit_ib(struct kgd_dev *kgd, enum
kgd_engine_type engine,
}
ret = amdgpu_ib_schedule(ring, 1, ib, job, );
+
+ up_read(>reset_sem);
+
if (ret) {
DRM_ERROR("amdgpu: failed to schedule IB.\n");
goto err_ib_sched;
}
- up_read(>reset_sem);
-
ret = dma_fence_wait(f, false);
err_ib_sched:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 4e017f379eb6..67a756f4337b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -545,7 +545,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
}
ret = amdgpu_xgmi_update_topology(hive, tmp_adev);
if (ret)
- goto exit;
+ goto exit_unlock;
}
/* get latest topology info for each device from psp */
@@ -558,7 +558,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
tmp_adev->gmc.xgmi.node_id,
tmp_adev->gmc.xgmi.hive_id, ret);
/* To do : continue with some node failed or
disable the whole hive */
- goto exit;
+ goto exit_unlock;
}
}
}
@@ -566,7 +566,7 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev)
if (!ret)
ret = amdgpu_xgmi_sysfs_add_dev_info(adev, hive);
-
+exit_unlock:
mutex_unlock(>hive_lock);
exit:
if (!ret)
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: new ids flag for tmz (v2)
Hi Christian,
On 30/07/2020 16:46, Christian König wrote:
> Am 30.07.20 um 15:54 schrieb Pierre-Eric Pelloux-Prayer:
>> Allows UMD to know if TMZ is supported and enabled.
>>
>> This commit also bumps KMS_DRIVER_MINOR because if we don't
>> UMD can't tell if "ids_flags & AMDGPU_IDS_FLAGS_TMZ == 0" means
>> "tmz is not enabled" or "tmz may be enabled but the kernel doesn't
>> report it".
>>
>> v2: use amdgpu_is_tmz() and reworded commit message.
>
> Your signed-off-by line is missing, but apart from that the patch is
> Reviewed-by: Christian König
Thanks, I'll add my s-o-b tag before pushing the patch.
Pierre-Eric
>
>> ---
>> Patch for using it in Mesa is at
>> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6049
>> (ac/gpu_info: add detection of TMZ support).
>> I've kept the AMDGPU_IDS_FLAGS_TMZ name to match the existing
>> AMDGPU_IDS_FLAGS_PREEMPTION flag.
>>
>> Pierre-Eric
>>
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 ++
>> include/uapi/drm/amdgpu_drm.h | 1 +
>> 3 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 6291f5f0d223..6dcab25914cf 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -88,9 +88,10 @@
>> * - 3.37.0 - L2 is invalidated before SDMA IBs, needed for correctness
>> * - 3.38.0 - Add AMDGPU_IB_FLAG_EMIT_MEM_SYNC
>> * - 3.39.0 - DMABUF implicit sync does a full pipeline sync
>> + * - 3.40.0 - Add AMDGPU_IDS_FLAGS_TMZ
>> */
>> #define KMS_DRIVER_MAJOR 3
>> -#define KMS_DRIVER_MINOR 39
>> +#define KMS_DRIVER_MINOR 40
>> #define KMS_DRIVER_PATCHLEVEL 0
>> int amdgpu_vram_limit = 0;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> index eebbe2103e32..d353ded353bb 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> @@ -736,6 +736,8 @@ static int amdgpu_info_ioctl(struct drm_device *dev,
>> void *data, struct drm_file
>> dev_info.ids_flags |= AMDGPU_IDS_FLAGS_FUSION;
>> if (amdgpu_mcbp || amdgpu_sriov_vf(adev))
>> dev_info.ids_flags |= AMDGPU_IDS_FLAGS_PREEMPTION;
>> + if (amdgpu_is_tmz(adev))
>> + dev_info.ids_flags |= AMDGPU_IDS_FLAGS_TMZ;
>> vm_size = adev->vm_manager.max_pfn * AMDGPU_GPU_PAGE_SIZE;
>> vm_size -= AMDGPU_VA_RESERVED_SIZE;
>> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
>> index 765a94ec4420..b826f2d6efe1 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -676,6 +676,7 @@ struct drm_amdgpu_cs_chunk_data {
>> */
>> #define AMDGPU_IDS_FLAGS_FUSION 0x1
>> #define AMDGPU_IDS_FLAGS_PREEMPTION 0x2
>> +#define AMDGPU_IDS_FLAGS_TMZ 0x4
>> /* indicate if acceleration can be working */
>> #define AMDGPU_INFO_ACCEL_WORKING 0x00
>
> ___
> 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 6/8] drm/amd/display: Set DC options from modifiers.
On Tue, Aug 04, 2020 at 11:31:17PM +0200, Bas Nieuwenhuizen wrote:
> This sets the DC tiling options from the modifier, if modifiers
> are used for the FB. This patch by itself does not expose the
> support yet though.
>
> There is not much validation yet to limit the scope of this
> patch, but the current validation is at the same level as
> the BO metadata path.
>
> Signed-off-by: Bas Nieuwenhuizen
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 109 +-
> 1 file changed, 103 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 6ef7f2f8acab..ac913b8f10ef 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3754,6 +3754,93 @@ fill_gfx9_plane_attributes_from_flags(struct
> amdgpu_device *adev,
> return 0;
> }
>
> +static bool
> +modifier_has_dcc(uint64_t modifier)
> +{
> + return IS_AMD_FMT_MOD(modifier) && AMD_FMT_MOD_GET(DCC, modifier);
> +}
> +
> +static unsigned
> +modifier_gfx9_swizzle_mode(uint64_t modifier)
> +{
> + if (modifier == DRM_FORMAT_MOD_LINEAR)
> + return 0;
> +
> + return AMD_FMT_MOD_GET(TILE, modifier);
> +}
> +
> +static void
> +fill_gfx9_tiling_info_from_modifier(const struct amdgpu_device *adev,
> + union dc_tiling_info *tiling_info,
> + uint64_t modifier)
> +{
> + unsigned int mod_bank_xor_bits = AMD_FMT_MOD_GET(BANK_XOR_BITS,
> modifier);
> + unsigned int mod_pipe_xor_bits = AMD_FMT_MOD_GET(PIPE_XOR_BITS,
> modifier);
> + unsigned int pkrs_log2 = AMD_FMT_MOD_GET(PACKERS, modifier);
> + unsigned int pipes_log2 = min(4u, mod_pipe_xor_bits);
> +
> + fill_gfx9_tiling_info_from_device(adev, tiling_info);
> +
> + if (!IS_AMD_FMT_MOD(modifier))
> + return;
> +
> + tiling_info->gfx9.num_pipes = 1u << pipes_log2;
> + tiling_info->gfx9.num_shader_engines = 1u << (mod_pipe_xor_bits -
> pipes_log2);
> +
> + if (adev->family >= AMDGPU_FAMILY_NV) {
> + tiling_info->gfx9.num_pkrs = 1u << pkrs_log2;
> + } else {
> + tiling_info->gfx9.num_banks = 1u << mod_bank_xor_bits;
> +
> + /* for DCC we know it isn't rb aligned, so rb_per_se doesn't
> matter. */
> + }
> +}
> +
> +static void
> +block_alignment(unsigned int blocksize_log2, unsigned int *width, unsigned
> int *height)
> +{
> + unsigned int height_log2 = blocksize_log2 / 2;
> + unsigned int width_log2 = blocksize_log2 - height_log2;
> +
> + *width = 1u << width_log2;
> + *height = 1u << height_log2;
> +}
> +
> +static int
> +fill_gfx9_plane_attributes_from_modifiers(struct amdgpu_device *adev,
> + const struct amdgpu_framebuffer *afb,
> + const enum surface_pixel_format format,
> + const enum dc_rotation_angle rotation,
> + const struct plane_size *plane_size,
> + union dc_tiling_info *tiling_info,
> + struct dc_plane_dcc_param *dcc,
> + struct dc_plane_address *address,
> + const bool force_disable_dcc)
> +{
> + const uint64_t modifier = afb->base.modifier;
> + int ret;
> +
> + fill_gfx9_tiling_info_from_modifier(adev, tiling_info, modifier);
> + tiling_info->gfx9.swizzle = modifier_gfx9_swizzle_mode(modifier);
> +
> + if (modifier_has_dcc(modifier) && !force_disable_dcc) {
> + uint64_t dcc_address = afb->address + afb->base.offsets[1];
> +
> + dcc->enable = 1;
> + dcc->meta_pitch = afb->base.pitches[1];
> + dcc->independent_64b_blks =
> AMD_FMT_MOD_GET(DCC_INDEPENDENT_64B, modifier);
> +
> + address->grph.meta_addr.low_part = lower_32_bits(dcc_address);
> + address->grph.meta_addr.high_part = upper_32_bits(dcc_address);
> + }
> +
> + ret = validate_dcc(adev, format, rotation, tiling_info, dcc, address,
> plane_size);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> static int
> fill_plane_buffer_attributes(struct amdgpu_device *adev,
>const struct amdgpu_framebuffer *afb,
> @@ -3823,12 +3910,22 @@ fill_plane_buffer_attributes(struct amdgpu_device
> *adev,
>
>
> if (adev->family >= AMDGPU_FAMILY_AI) {
> - ret = fill_gfx9_plane_attributes_from_flags(adev, afb, format,
> rotation,
> - plane_size,
> tiling_info, dcc,
> - address,
> tiling_flags,
> - force_disable_dcc);
> - if (ret)
> -
Re: [PATCH 3/8] drm/amd/display: Honor the offset for plane 0.
On Tue, Aug 04, 2020 at 11:31:14PM +0200, Bas Nieuwenhuizen wrote:
> With modifiers I'd like to support non-dedicated buffers for
> images.
>
> Signed-off-by: Bas Nieuwenhuizen
Uh, I think it'd be really good to preceed this with a bugfix (cc: stable)
which checks that the offset is 0). And then this patch here removing that
again. Or cc: stable this patch here, since we seem to have a gap in
validating addfb.
-Daniel
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 +-
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 005331c772b7..abc70fbe176d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3623,6 +3623,7 @@ fill_plane_dcc_attributes(struct amdgpu_device *adev,
> struct dc *dc = adev->dm.dc;
> struct dc_dcc_surface_param input;
> struct dc_surface_dcc_cap output;
> + uint64_t plane_address = afb->address + afb->base.offsets[0];
> uint32_t offset = AMDGPU_TILING_GET(info, DCC_OFFSET_256B);
> uint32_t i64b = AMDGPU_TILING_GET(info, DCC_INDEPENDENT_64B) != 0;
> uint64_t dcc_address;
> @@ -3666,7 +3667,7 @@ fill_plane_dcc_attributes(struct amdgpu_device *adev,
> AMDGPU_TILING_GET(info, DCC_PITCH_MAX) + 1;
> dcc->independent_64b_blks = i64b;
>
> - dcc_address = get_dcc_address(afb->address, info);
> + dcc_address = get_dcc_address(plane_address, info);
> address->grph.meta_addr.low_part = lower_32_bits(dcc_address);
> address->grph.meta_addr.high_part = upper_32_bits(dcc_address);
>
> @@ -3697,6 +3698,8 @@ fill_plane_buffer_attributes(struct amdgpu_device *adev,
> address->tmz_surface = tmz_surface;
>
> if (format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) {
> + uint64_t addr = afb->address + fb->offsets[0];
> +
> plane_size->surface_size.x = 0;
> plane_size->surface_size.y = 0;
> plane_size->surface_size.width = fb->width;
> @@ -3705,9 +3708,10 @@ fill_plane_buffer_attributes(struct amdgpu_device
> *adev,
> fb->pitches[0] / fb->format->cpp[0];
>
> address->type = PLN_ADDR_TYPE_GRAPHICS;
> - address->grph.addr.low_part = lower_32_bits(afb->address);
> - address->grph.addr.high_part = upper_32_bits(afb->address);
> + address->grph.addr.low_part = lower_32_bits(addr);
> + address->grph.addr.high_part = upper_32_bits(addr);
> } else if (format < SURFACE_PIXEL_FORMAT_INVALID) {
> + uint64_t luma_addr = afb->address + fb->offsets[0];
> uint64_t chroma_addr = afb->address + fb->offsets[1];
>
> plane_size->surface_size.x = 0;
> @@ -3728,9 +3732,9 @@ fill_plane_buffer_attributes(struct amdgpu_device *adev,
>
> address->type = PLN_ADDR_TYPE_VIDEO_PROGRESSIVE;
> address->video_progressive.luma_addr.low_part =
> - lower_32_bits(afb->address);
> + lower_32_bits(luma_addr);
> address->video_progressive.luma_addr.high_part =
> - upper_32_bits(afb->address);
> + upper_32_bits(luma_addr);
> address->video_progressive.chroma_addr.low_part =
> lower_32_bits(chroma_addr);
> address->video_progressive.chroma_addr.high_part =
> --
> 2.28.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
