Re: [PATCH] drm/amdgpu/vcn3: drop extraneous Beige Goby hunk

2021-06-15 Thread Zhu, James
[AMD Official Use Only]

This patch is Reviewed-by: James Zhu 


Thanks & Best Regards!


James Zhu


From: amd-gfx  on behalf of Alex Deucher 

Sent: Tuesday, June 15, 2021 5:32 PM
To: amd-gfx@lists.freedesktop.org 
Cc: Deucher, Alexander 
Subject: [PATCH] drm/amdgpu/vcn3: drop extraneous Beige Goby hunk

Probably a rebase leftover.  This doesn't apply to SR-IOV, and
the non-SR-IOV code below it already handles this properly.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
index 4c36fc5c9738..ea6487ca997a 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
@@ -93,11 +93,6 @@ static int vcn_v3_0_early_init(void *handle)
 adev->vcn.harvest_config = 0;
 adev->vcn.num_enc_rings = 1;

-   if (adev->asic_type == CHIP_BEIGE_GOBY) {
-   adev->vcn.num_vcn_inst = 1;
-   adev->vcn.num_enc_rings = 0;
-   }
-
 } else {
 if (adev->asic_type == CHIP_SIENNA_CICHLID) {
 u32 harvest;
--
2.31.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-gfx&data=04%7C01%7Cjames.zhu%40amd.com%7C438470def59345ef4f1f08d9304565a2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637593896894033208%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=rTKUaT%2BRjI0S%2B8le%2BdbtSUGjksyDTF%2BV9SdBTwFpjxk%3D&reserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] drm/amdgpu/vcn3: drop extraneous Beige Goby hunk

2021-06-15 Thread Alex Deucher
Probably a rebase leftover.  This doesn't apply to SR-IOV, and
the non-SR-IOV code below it already handles this properly.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
index 4c36fc5c9738..ea6487ca997a 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
@@ -93,11 +93,6 @@ static int vcn_v3_0_early_init(void *handle)
adev->vcn.harvest_config = 0;
adev->vcn.num_enc_rings = 1;
 
-   if (adev->asic_type == CHIP_BEIGE_GOBY) {
-   adev->vcn.num_vcn_inst = 1;
-   adev->vcn.num_enc_rings = 0;
-   }
-
} else {
if (adev->asic_type == CHIP_SIENNA_CICHLID) {
u32 harvest;
-- 
2.31.1

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


Re: [PATCH] drm/amdkfd: Fix circular lock in nocpsch path

2021-06-15 Thread Felix Kuehling
[+Xinhui]


Am 2021-06-15 um 1:50 p.m. schrieb Amber Lin:
> Calling free_mqd inside of destroy_queue_nocpsch_locked can cause a
> circular lock. destroy_queue_nocpsch_locked is called under a DQM lock,
> which is taken in MMU notifiers, potentially in FS reclaim context.
> Taking another lock, which is BO reservation lock from free_mqd, while
> causing an FS reclaim inside the DQM lock creates a problematic circular
> lock dependency. Therefore move free_mqd out of
> destroy_queue_nocpsch_locked and call it after unlocking DQM.
>
> Signed-off-by: Amber Lin 
> Reviewed-by: Felix Kuehling 

Let's submit this patch as is. I'm making some comments inline for
things that Xinhui can address in his race condition patch.


> ---
>  .../drm/amd/amdkfd/kfd_device_queue_manager.c  | 18 +-
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 72bea5278add..c069fa259b30 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -486,9 +486,6 @@ static int destroy_queue_nocpsch_locked(struct 
> device_queue_manager *dqm,
>   if (retval == -ETIME)
>   qpd->reset_wavefronts = true;
>  
> -
> - mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> -
>   list_del(&q->list);
>   if (list_empty(&qpd->queues_list)) {
>   if (qpd->reset_wavefronts) {
> @@ -523,6 +520,8 @@ static int destroy_queue_nocpsch(struct 
> device_queue_manager *dqm,
>   int retval;
>   uint64_t sdma_val = 0;
>   struct kfd_process_device *pdd = qpd_to_pdd(qpd);
> + struct mqd_manager *mqd_mgr =
> + dqm->mqd_mgrs[get_mqd_type_from_queue_type(q->properties.type)];
>  
>   /* Get the SDMA queue stats */
>   if ((q->properties.type == KFD_QUEUE_TYPE_SDMA) ||
> @@ -540,6 +539,8 @@ static int destroy_queue_nocpsch(struct 
> device_queue_manager *dqm,
>   pdd->sdma_past_activity_counter += sdma_val;
>   dqm_unlock(dqm);
>  
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> +
>   return retval;
>  }
>  
> @@ -1629,7 +1630,7 @@ static bool set_cache_memory_policy(struct 
> device_queue_manager *dqm,
>  static int process_termination_nocpsch(struct device_queue_manager *dqm,
>   struct qcm_process_device *qpd)
>  {
> - struct queue *q, *next;
> + struct queue *q;
>   struct device_process_node *cur, *next_dpn;
>   int retval = 0;
>   bool found = false;
> @@ -1637,12 +1638,19 @@ static int process_termination_nocpsch(struct 
> device_queue_manager *dqm,
>   dqm_lock(dqm);
>  
>   /* Clear all user mode queues */
> - list_for_each_entry_safe(q, next, &qpd->queues_list, list) {
> + while (!list_empty(&qpd->queues_list)) {
> + struct mqd_manager *mqd_mgr;
>   int ret;
>  
> + q = list_first_entry(&qpd->queues_list, struct queue, list);
> + mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
> + q->properties.type)];
>   ret = destroy_queue_nocpsch_locked(dqm, qpd, q);
>   if (ret)
>   retval = ret;
> + dqm_unlock(dqm);
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> + dqm_lock(dqm);

This is the correct way to clean up the list when dropping the dqm-lock
in the middle. Xinhui, you can use the same method in
process_termination_cpsch.

I believe the swapping of the q->mqd with a temporary variable is not
needed. When free_mqd is called, the queue is no longer on the
qpd->queues_list, so destroy_queue cannot race with it. If we ensure
that queues are always removed from the list before calling free_mqd,
and that list-removal happens under the dqm_lock, then there should be
no risk of a race condition that causes a double-free.

Regards,
  Felix


>   }
>  
>   /* Unregister process */
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] drm/amdkfd: Fix circular lock in nocpsch path

2021-06-15 Thread Amber Lin
Calling free_mqd inside of destroy_queue_nocpsch_locked can cause a
circular lock. destroy_queue_nocpsch_locked is called under a DQM lock,
which is taken in MMU notifiers, potentially in FS reclaim context.
Taking another lock, which is BO reservation lock from free_mqd, while
causing an FS reclaim inside the DQM lock creates a problematic circular
lock dependency. Therefore move free_mqd out of
destroy_queue_nocpsch_locked and call it after unlocking DQM.

Signed-off-by: Amber Lin 
Reviewed-by: Felix Kuehling 
---
 .../drm/amd/amdkfd/kfd_device_queue_manager.c  | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 72bea5278add..c069fa259b30 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -486,9 +486,6 @@ static int destroy_queue_nocpsch_locked(struct 
device_queue_manager *dqm,
if (retval == -ETIME)
qpd->reset_wavefronts = true;
 
-
-   mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
-
list_del(&q->list);
if (list_empty(&qpd->queues_list)) {
if (qpd->reset_wavefronts) {
@@ -523,6 +520,8 @@ static int destroy_queue_nocpsch(struct 
device_queue_manager *dqm,
int retval;
uint64_t sdma_val = 0;
struct kfd_process_device *pdd = qpd_to_pdd(qpd);
+   struct mqd_manager *mqd_mgr =
+   dqm->mqd_mgrs[get_mqd_type_from_queue_type(q->properties.type)];
 
/* Get the SDMA queue stats */
if ((q->properties.type == KFD_QUEUE_TYPE_SDMA) ||
@@ -540,6 +539,8 @@ static int destroy_queue_nocpsch(struct 
device_queue_manager *dqm,
pdd->sdma_past_activity_counter += sdma_val;
dqm_unlock(dqm);
 
+   mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
+
return retval;
 }
 
@@ -1629,7 +1630,7 @@ static bool set_cache_memory_policy(struct 
device_queue_manager *dqm,
 static int process_termination_nocpsch(struct device_queue_manager *dqm,
struct qcm_process_device *qpd)
 {
-   struct queue *q, *next;
+   struct queue *q;
struct device_process_node *cur, *next_dpn;
int retval = 0;
bool found = false;
@@ -1637,12 +1638,19 @@ static int process_termination_nocpsch(struct 
device_queue_manager *dqm,
dqm_lock(dqm);
 
/* Clear all user mode queues */
-   list_for_each_entry_safe(q, next, &qpd->queues_list, list) {
+   while (!list_empty(&qpd->queues_list)) {
+   struct mqd_manager *mqd_mgr;
int ret;
 
+   q = list_first_entry(&qpd->queues_list, struct queue, list);
+   mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
+   q->properties.type)];
ret = destroy_queue_nocpsch_locked(dqm, qpd, q);
if (ret)
retval = ret;
+   dqm_unlock(dqm);
+   mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
+   dqm_lock(dqm);
}
 
/* Unregister process */
-- 
2.17.1

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


Re: [RFC PATCH] drm/ttm: Do page counting after populate callback succeed

2021-06-15 Thread Christian König



Am 15.06.21 um 17:06 schrieb Felix Kuehling:

Am 2021-06-15 um 8:18 a.m. schrieb Christian König:

Am 15.06.21 um 14:11 schrieb Pan, Xinhui:

2021年6月15日 20:01,Christian König 
写道:

Am 15.06.21 um 13:57 schrieb xinhui pan:

Amdgpu set SG flag in populate callback. So TTM still count pages
in SG
BO.

It's probably better to fix this instead. E.g. why does amdgpu
modify the SG flag during populate and not during initial creation?
That doesn't seem to make sense.

fair enough. Let me have a try.
No idea why we set SG flag in populate years ago.

That's pretty recent IIRC. Felix moved the code around a bit to fix
another problem.

I moved some code from populate/unpopulate to backend_bind/unbind for
attaching and detaching dmabufs. I didn't change the setting/unsetting
of SG flags for userptr BOs. That goes back all the way to 2015.

As far as I can tell, this is needed because userptr BOs are not created
as SG BOs. That's something I've also pointed out before.


Ah, yes. That's because we need the pages array for userptr BOs.

Then we should probably move that to amdgpu_ttm_tt_set_userptr().

Thanks,
Christian.



Regards,
   Felix



Christian.


Christian.


One easy way to fix this is lets count pages after populate callback.

We hit one issue that amdgpu alloc many SG BOs, but TTM try to do swap
again and again even if swapout does not swap SG BOs at all.

Signed-off-by: xinhui pan 
---
   drivers/gpu/drm/ttm/ttm_tt.c | 32 +---
   1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c
b/drivers/gpu/drm/ttm/ttm_tt.c
index a1a25410ec74..4fa0a8cd71c0 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -317,13 +317,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
   if (ttm_tt_is_populated(ttm))
   return 0;
   -    if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-    atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
-    if (bdev->pool.use_dma32)
-    atomic_long_add(ttm->num_pages,
-    &ttm_dma32_pages_allocated);
-    }
-
   while (atomic_long_read(&ttm_pages_allocated) >
ttm_pages_limit ||
  atomic_long_read(&ttm_dma32_pages_allocated) >
  ttm_dma32_pages_limit) {
@@ -342,6 +335,13 @@ int ttm_tt_populate(struct ttm_device *bdev,
   if (ret)
   goto error;
   +    if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
+    atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
+    if (bdev->pool.use_dma32)
+    atomic_long_add(ttm->num_pages,
+    &ttm_dma32_pages_allocated);
+    }
+
   ttm_tt_add_mapping(bdev, ttm);
   ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED;
   if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) {
@@ -355,12 +355,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
   return 0;
     error:
-    if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-    atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
-    if (bdev->pool.use_dma32)
-    atomic_long_sub(ttm->num_pages,
-    &ttm_dma32_pages_allocated);
-    }
   return ret;
   }
   EXPORT_SYMBOL(ttm_tt_populate);
@@ -384,12 +378,6 @@ void ttm_tt_unpopulate(struct ttm_device
*bdev, struct ttm_tt *ttm)
   if (!ttm_tt_is_populated(ttm))
   return;
   -    ttm_tt_clear_mapping(ttm);
-    if (bdev->funcs->ttm_tt_unpopulate)
-    bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
-    else
-    ttm_pool_free(&bdev->pool, ttm);
-
   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
   atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
   if (bdev->pool.use_dma32)
@@ -397,6 +385,12 @@ void ttm_tt_unpopulate(struct ttm_device
*bdev, struct ttm_tt *ttm)
   &ttm_dma32_pages_allocated);
   }
   +    ttm_tt_clear_mapping(ttm);
+    if (bdev->funcs->ttm_tt_unpopulate)
+    bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
+    else
+    ttm_pool_free(&bdev->pool, ttm);
+
   ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED;
   }
   


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


Re: [PATCH] drm: display: Fix duplicate field initialization in dcn31

2021-06-15 Thread Alex Deucher
Applied.  Thanks!


On Tue, Jun 15, 2021 at 8:54 AM Rodrigo Siqueira
 wrote:
>
> On 06/15, Wan Jiabing wrote:
> > Fix the following coccicheck warning:
> > drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c:917:56-57:
> > pstate_enabled: first occurrence line 935, second occurrence line 937
> >
> > Signed-off-by: Wan Jiabing 
> > ---
> >  drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c 
> > b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
> > index 0d6cb6caad81..c67bc9544f5d 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
> > @@ -934,7 +934,6 @@ static const struct dc_debug_options debug_defaults_drv 
> > = {
> >   .dmub_command_table = true,
> >   .pstate_enabled = true,
> >   .use_max_lb = true,
> > - .pstate_enabled = true,
> >   .enable_mem_low_power = {
> >   .bits = {
> >   .vga = false,
> > --
> > 2.20.1
> >
>
> Reviewed-by: Rodrigo Siqueira 
>
> Thanks
>
> --
> Rodrigo Siqueira
> https://siqueira.tech
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdkfd: Fix a race between queue destroy and process termination

2021-06-15 Thread Felix Kuehling
Am 2021-06-15 um 11:32 a.m. schrieb Felix Kuehling:
> [+Amber, moving amd-gfx to BCC]

Actually didn't move it to BCC. But let's not name that NPI branch in
public. ;)

Thanks,
  Felix


>
> Amber worked on a related problem on an NPI branch recently in the
> nocpsch version of this code. We should port that fix to
> amd-staging-drm-next. Then lets come up with a common solution for the 
> cpsch code path as well.
>
> See one comment inline.
>
>
> Am 2021-06-15 um 4:02 a.m. schrieb xinhui pan:
>> We call free_mqd without dqm lock hold, that causes double free of
>> mqd_mem_obj. Fix it by using a tmp pointer.
>> We need walk through the queues_list with dqm lock hold. Otherwise hit
>> list corruption.
>>
>> Signed-off-by: xinhui pan 
>> ---
>>  .../drm/amd/amdkfd/kfd_device_queue_manager.c   | 17 +
>>  1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> index e6366b408420..575c895fc241 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> @@ -1484,6 +1484,7 @@ static int destroy_queue_cpsch(struct 
>> device_queue_manager *dqm,
>>  struct mqd_manager *mqd_mgr;
>>  uint64_t sdma_val = 0;
>>  struct kfd_process_device *pdd = qpd_to_pdd(qpd);
>> +void *ptr = NULL;
>>  
>>  /* Get the SDMA queue stats */
>>  if ((q->properties.type == KFD_QUEUE_TYPE_SDMA) ||
>> @@ -1543,10 +1544,12 @@ static int destroy_queue_cpsch(struct 
>> device_queue_manager *dqm,
>>  pr_debug("Total of %d queues are accountable so far\n",
>>  dqm->total_queue_count);
>>  
>> +swap(ptr, q->mqd_mem_obj);
>>  dqm_unlock(dqm);
>>  
>>  /* Do free_mqd after dqm_unlock(dqm) to avoid circular locking */
>> -mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
>> +if (ptr)
>> +mqd_mgr->free_mqd(mqd_mgr, q->mqd, ptr);
>>  
>>  return retval;
>>  
>> @@ -1709,6 +1712,7 @@ static int process_termination_cpsch(struct 
>> device_queue_manager *dqm,
>>  enum kfd_unmap_queues_filter filter =
>>  KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES;
>>  bool found = false;
>> +void *ptr = NULL;
>>  
>>  retval = 0;
>>  
>> @@ -1737,8 +1741,6 @@ static int process_termination_cpsch(struct 
>> device_queue_manager *dqm,
>>  qpd->mapped_gws_queue = false;
>>  }
>>  }
>> -
>> -dqm->total_queue_count--;
>>  }
>>  
>>  /* Unregister process */
>> @@ -1770,13 +1772,20 @@ static int process_termination_cpsch(struct 
>> device_queue_manager *dqm,
>>  /* Lastly, free mqd resources.
>>   * Do free_mqd() after dqm_unlock to avoid circular locking.
>>   */
>> +dqm_lock(dqm);
>>  list_for_each_entry_safe(q, next, &qpd->queues_list, list) {
>>  mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
>>  q->properties.type)];
>>  list_del(&q->list);
>>  qpd->queue_count--;
>> -mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
>> +dqm->total_queue_count--;
>> +swap(ptr, q->mqd_mem_obj);
>> +dqm_unlock(dqm);
> This still risks list corruption because the list can be modified while
> the lock is dropped. However you can make it safe by changing the loop
> to something like
>
>   dqm_lock(dqm);
>   while (!list_empty(...)) {
>   q = list_first_entry(...);
>   dqm_unlock(dqm);
>   ...
>   mqd_mgr->free_mqd(...);
>   ...
>   dqm_lock(dqm);
>   }
>   dqm_unlock();
>
> Regards,
>   Felix
>
>
>> +if (ptr)
>> +mqd_mgr->free_mqd(mqd_mgr, q->mqd, ptr);
>> +dqm_lock(dqm);
>>  }
>> +dqm_unlock(dqm);
>>  
>>  return retval;
>>  }
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdkfd: Fix a race between queue destroy and process termination

2021-06-15 Thread Felix Kuehling
[+Amber, moving amd-gfx to BCC]

Amber worked on a related problem on an NPI branch recently in the
nocpsch version of this code. We should port that fix to
amd-staging-drm-next. Then lets come up with a common solution for the 
cpsch code path as well.

See one comment inline.


Am 2021-06-15 um 4:02 a.m. schrieb xinhui pan:
> We call free_mqd without dqm lock hold, that causes double free of
> mqd_mem_obj. Fix it by using a tmp pointer.
> We need walk through the queues_list with dqm lock hold. Otherwise hit
> list corruption.
>
> Signed-off-by: xinhui pan 
> ---
>  .../drm/amd/amdkfd/kfd_device_queue_manager.c   | 17 +
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index e6366b408420..575c895fc241 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -1484,6 +1484,7 @@ static int destroy_queue_cpsch(struct 
> device_queue_manager *dqm,
>   struct mqd_manager *mqd_mgr;
>   uint64_t sdma_val = 0;
>   struct kfd_process_device *pdd = qpd_to_pdd(qpd);
> + void *ptr = NULL;
>  
>   /* Get the SDMA queue stats */
>   if ((q->properties.type == KFD_QUEUE_TYPE_SDMA) ||
> @@ -1543,10 +1544,12 @@ static int destroy_queue_cpsch(struct 
> device_queue_manager *dqm,
>   pr_debug("Total of %d queues are accountable so far\n",
>   dqm->total_queue_count);
>  
> + swap(ptr, q->mqd_mem_obj);
>   dqm_unlock(dqm);
>  
>   /* Do free_mqd after dqm_unlock(dqm) to avoid circular locking */
> - mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> + if (ptr)
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, ptr);
>  
>   return retval;
>  
> @@ -1709,6 +1712,7 @@ static int process_termination_cpsch(struct 
> device_queue_manager *dqm,
>   enum kfd_unmap_queues_filter filter =
>   KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES;
>   bool found = false;
> + void *ptr = NULL;
>  
>   retval = 0;
>  
> @@ -1737,8 +1741,6 @@ static int process_termination_cpsch(struct 
> device_queue_manager *dqm,
>   qpd->mapped_gws_queue = false;
>   }
>   }
> -
> - dqm->total_queue_count--;
>   }
>  
>   /* Unregister process */
> @@ -1770,13 +1772,20 @@ static int process_termination_cpsch(struct 
> device_queue_manager *dqm,
>   /* Lastly, free mqd resources.
>* Do free_mqd() after dqm_unlock to avoid circular locking.
>*/
> + dqm_lock(dqm);
>   list_for_each_entry_safe(q, next, &qpd->queues_list, list) {
>   mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
>   q->properties.type)];
>   list_del(&q->list);
>   qpd->queue_count--;
> - mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> + dqm->total_queue_count--;
> + swap(ptr, q->mqd_mem_obj);
> + dqm_unlock(dqm);

This still risks list corruption because the list can be modified while
the lock is dropped. However you can make it safe by changing the loop
to something like

dqm_lock(dqm);
while (!list_empty(...)) {
q = list_first_entry(...);
dqm_unlock(dqm);
...
mqd_mgr->free_mqd(...);
...
dqm_lock(dqm);
}
dqm_unlock();

Regards,
  Felix


> + if (ptr)
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, ptr);
> + dqm_lock(dqm);
>   }
> + dqm_unlock(dqm);
>  
>   return retval;
>  }
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH v2] drm/amd/amdgpu: Use IP discovery data to determine VCN enablement instead of MMSCH

2021-06-15 Thread Alex Deucher
On Tue, Jun 15, 2021 at 3:46 AM Peng Ju Zhou  wrote:
>
> From: Bokun Zhang 
>
> In the past, we use MMSCH to determine whether a VCN is enabled or not.
> This is not reliable since after a FLR, MMSCH may report junk data.
>
> It is better to use IP discovery data.
>
> Signed-off-by: Bokun Zhang 
> Signed-off-by: Peng Ju Zhou 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c |  8 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h |  3 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c   | 23 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h   | 13 +
>  drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 52 +--
>  5 files changed, 60 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index f949ed8bfd9e..e02405a24fe3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -373,6 +373,14 @@ int amdgpu_discovery_get_ip_version(struct amdgpu_device 
> *adev, int hw_id, int n
> return -EINVAL;
>  }
>
> +
> +int amdgpu_discovery_get_vcn_version(struct amdgpu_device *adev, int 
> vcn_instance,
> +int *major, int *minor, int *revision)
> +{
> +   return amdgpu_discovery_get_ip_version(adev, VCN_HWID,
> +  vcn_instance, major, minor, 
> revision);
> +}
> +

I think you can drop this wrapper and just call
amdgpu_discovery_get_ip_version() directly from
amdgpu_vcn_is_disabled_vcn().

>  void amdgpu_discovery_harvest_ip(struct amdgpu_device *adev)
>  {
> struct binary_header *bhdr;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h
> index 02e340cd3a38..48e6b88cfdfe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h
> @@ -32,6 +32,9 @@ int amdgpu_discovery_reg_base_init(struct amdgpu_device 
> *adev);
>  void amdgpu_discovery_harvest_ip(struct amdgpu_device *adev);
>  int amdgpu_discovery_get_ip_version(struct amdgpu_device *adev, int hw_id, 
> int number_instance,
>  int *major, int *minor, int *revision);
> +
> +int amdgpu_discovery_get_vcn_version(struct amdgpu_device *adev, int 
> vcn_instance,
> +int *major, int *minor, int *revision);
>  int amdgpu_discovery_get_gfx_info(struct amdgpu_device *adev);
>
>  #endif /* __AMDGPU_DISCOVERY__ */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 9492b505e69b..84b025405578 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -287,6 +287,29 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
> return 0;
>  }
>
> +bool amdgpu_vcn_is_disabled_vcn(struct amdgpu_device *adev, enum 
> vcn_ring_type type, uint32_t vcn_instance)
> +{
> +   bool ret = false;
> +
> +   int major;
> +   int minor;
> +   int revision;
> +
> +   /* if cannot find IP data, then this VCN does not exist */
> +   if (amdgpu_discovery_get_vcn_version(adev, vcn_instance, &major, 
> &minor, &revision) != 0)

Just call amdgpu_discovery_get_ip_version() directly here.

> +   return true;
> +
> +   if ((type == VCN_ENCODE_RING) && (revision & 
> VCN_BLOCK_ENCODE_DISABLE_MASK)) {
> +   ret = true;
> +   } else if ((type == VCN_DECODE_RING) && (revision & 
> VCN_BLOCK_DECODE_DISABLE_MASK)) {
> +   ret = true;
> +   } else if ((type == VCN_UNIFIED_RING) && (revision & 
> VCN_BLOCK_QUEUE_DISABLE_MASK)) {
> +   ret = true;
> +   }
> +
> +   return ret;
> +}
> +
>  int amdgpu_vcn_suspend(struct amdgpu_device *adev)
>  {
> unsigned size;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index bc76cab67697..d74c62b49795 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -280,6 +280,16 @@ struct amdgpu_vcn_decode_buffer {
> uint32_t pad[30];
>  };
>
> +#define VCN_BLOCK_ENCODE_DISABLE_MASK 0x80
> +#define VCN_BLOCK_DECODE_DISABLE_MASK 0x40
> +#define VCN_BLOCK_QUEUE_DISABLE_MASK 0xC0
> +
> +enum vcn_ring_type {
> +   VCN_ENCODE_RING,
> +   VCN_DECODE_RING,
> +   VCN_UNIFIED_RING,
> +};
> +
>  int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
>  int amdgpu_vcn_sw_fini(struct amdgpu_device *adev);
>  int amdgpu_vcn_suspend(struct amdgpu_device *adev);
> @@ -287,6 +297,9 @@ int amdgpu_vcn_resume(struct amdgpu_device *adev);
>  void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring);
>  void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring);
>
> +bool amdgpu_vcn_is_disabled_vcn(struct amdgpu_device *adev,
> +   enum vcn_ring_type type, uint32_t 
> vcn_instance);
> +
>  int 

Re: [RFC PATCH] drm/ttm: Do page counting after populate callback succeed

2021-06-15 Thread Felix Kuehling
Am 2021-06-15 um 8:18 a.m. schrieb Christian König:
> Am 15.06.21 um 14:11 schrieb Pan, Xinhui:
>>> 2021年6月15日 20:01,Christian König 
>>> 写道:
>>>
>>> Am 15.06.21 um 13:57 schrieb xinhui pan:
 Amdgpu set SG flag in populate callback. So TTM still count pages
 in SG
 BO.
>>> It's probably better to fix this instead. E.g. why does amdgpu
>>> modify the SG flag during populate and not during initial creation?
>>> That doesn't seem to make sense.
>> fair enough. Let me have a try.
>> No idea why we set SG flag in populate years ago.
>
> That's pretty recent IIRC. Felix moved the code around a bit to fix
> another problem.

I moved some code from populate/unpopulate to backend_bind/unbind for
attaching and detaching dmabufs. I didn't change the setting/unsetting
of SG flags for userptr BOs. That goes back all the way to 2015.

As far as I can tell, this is needed because userptr BOs are not created
as SG BOs. That's something I've also pointed out before.

Regards,
  Felix


>
> Christian.
>
>>
>>> Christian.
>>>
 One easy way to fix this is lets count pages after populate callback.

 We hit one issue that amdgpu alloc many SG BOs, but TTM try to do swap
 again and again even if swapout does not swap SG BOs at all.

 Signed-off-by: xinhui pan 
 ---
   drivers/gpu/drm/ttm/ttm_tt.c | 32 +---
   1 file changed, 13 insertions(+), 19 deletions(-)

 diff --git a/drivers/gpu/drm/ttm/ttm_tt.c
 b/drivers/gpu/drm/ttm/ttm_tt.c
 index a1a25410ec74..4fa0a8cd71c0 100644
 --- a/drivers/gpu/drm/ttm/ttm_tt.c
 +++ b/drivers/gpu/drm/ttm/ttm_tt.c
 @@ -317,13 +317,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
   if (ttm_tt_is_populated(ttm))
   return 0;
   -    if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
 -    atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
 -    if (bdev->pool.use_dma32)
 -    atomic_long_add(ttm->num_pages,
 -    &ttm_dma32_pages_allocated);
 -    }
 -
   while (atomic_long_read(&ttm_pages_allocated) >
 ttm_pages_limit ||
  atomic_long_read(&ttm_dma32_pages_allocated) >
  ttm_dma32_pages_limit) {
 @@ -342,6 +335,13 @@ int ttm_tt_populate(struct ttm_device *bdev,
   if (ret)
   goto error;
   +    if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
 +    atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
 +    if (bdev->pool.use_dma32)
 +    atomic_long_add(ttm->num_pages,
 +    &ttm_dma32_pages_allocated);
 +    }
 +
   ttm_tt_add_mapping(bdev, ttm);
   ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED;
   if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) {
 @@ -355,12 +355,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
   return 0;
     error:
 -    if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
 -    atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
 -    if (bdev->pool.use_dma32)
 -    atomic_long_sub(ttm->num_pages,
 -    &ttm_dma32_pages_allocated);
 -    }
   return ret;
   }
   EXPORT_SYMBOL(ttm_tt_populate);
 @@ -384,12 +378,6 @@ void ttm_tt_unpopulate(struct ttm_device
 *bdev, struct ttm_tt *ttm)
   if (!ttm_tt_is_populated(ttm))
   return;
   -    ttm_tt_clear_mapping(ttm);
 -    if (bdev->funcs->ttm_tt_unpopulate)
 -    bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
 -    else
 -    ttm_pool_free(&bdev->pool, ttm);
 -
   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
   atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
   if (bdev->pool.use_dma32)
 @@ -397,6 +385,12 @@ void ttm_tt_unpopulate(struct ttm_device
 *bdev, struct ttm_tt *ttm)
   &ttm_dma32_pages_allocated);
   }
   +    ttm_tt_clear_mapping(ttm);
 +    if (bdev->funcs->ttm_tt_unpopulate)
 +    bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
 +    else
 +    ttm_pool_free(&bdev->pool, ttm);
 +
   ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED;
   }
   
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 36/40] drm/amdgpu: Optimizations to EEPROM RAS table I/O

2021-06-15 Thread Alex Deucher
On Mon, Jun 14, 2021 at 1:47 PM Luben Tuikov  wrote:
>
> Read and write the table in one go, then using a
> separate stage to decode or encode the data and
> reading/writing the table, as opposed to on the
> fly, which keeps the I2C bus busy. Use a single
> read/write to read/write the table or at most two
> if the number of records we're reading/writing
> wraps around.
>
> Check the check-sum of a table in EEPROM on init.
>
> When updating the table header signature, when the
> threshold was increased on boot, also update the
> check-sum at that time.
>
> Split functionality between read and write, which
> simplifies the code and exposes areas of
> optimization and complexity.

Is there a way to split this patch into several smaller patches?  It's
so big I'm having a hard time understanding them all in a single
context.

Alex

>
> Cc: Alexander Deucher 
> Cc: Andrey Grodzovsky 
> Signed-off-by: Luben Tuikov 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c|  20 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h|  23 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c   |  20 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 903 +++---
>  .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.h|  53 +-
>  5 files changed, 655 insertions(+), 364 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> index a4815af111ed12..4c3c65a5acae9b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> @@ -176,8 +176,8 @@ static int __amdgpu_eeprom_xfer(struct i2c_adapter 
> *i2c_adap, u32 eeprom_addr,
>   *
>   * Returns the number of bytes read/written; -errno on error.
>   */
> -int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr,
> -  u8 *eeprom_buf, u16 buf_size, bool read)
> +static int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr,
> + u8 *eeprom_buf, u16 buf_size, bool read)
>  {
> const struct i2c_adapter_quirks *quirks = i2c_adap->quirks;
> u16 limit;
> @@ -221,3 +221,19 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 
> eeprom_addr,
> return res;
> }
>  }
> +
> +int amdgpu_eeprom_read(struct i2c_adapter *i2c_adap,
> +  u32 eeprom_addr, u8 *eeprom_buf,
> +  u16 bytes)
> +{
> +   return amdgpu_eeprom_xfer(i2c_adap, eeprom_addr, eeprom_buf, bytes,
> + true);
> +}
> +
> +int amdgpu_eeprom_write(struct i2c_adapter *i2c_adap,
> +   u32 eeprom_addr, u8 *eeprom_buf,
> +   u16 bytes)
> +{
> +   return amdgpu_eeprom_xfer(i2c_adap, eeprom_addr, eeprom_buf, bytes,
> + false);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h
> index 966b434f0de2b7..6935adb2be1f1c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h
> @@ -26,23 +26,12 @@
>
>  #include 
>
> -int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr,
> -  u8 *eeprom_buf, u16 bytes, bool read);
> +int amdgpu_eeprom_read(struct i2c_adapter *i2c_adap,
> +  u32 eeprom_addr, u8 *eeprom_buf,
> +  u16 bytes);
>
> -static inline int amdgpu_eeprom_read(struct i2c_adapter *i2c_adap,
> -u32 eeprom_addr, u8 *eeprom_buf,
> -u16 bytes)
> -{
> -   return amdgpu_eeprom_xfer(i2c_adap, eeprom_addr, eeprom_buf, bytes,
> - true);
> -}
> -
> -static inline int amdgpu_eeprom_write(struct i2c_adapter *i2c_adap,
> - u32 eeprom_addr, u8 *eeprom_buf,
> - u16 bytes)
> -{
> -   return amdgpu_eeprom_xfer(i2c_adap, eeprom_addr, eeprom_buf, bytes,
> - false);
> -}
> +int amdgpu_eeprom_write(struct i2c_adapter *i2c_adap,
> +   u32 eeprom_addr, u8 *eeprom_buf,
> +   u16 bytes);
>
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index ecdf2d80eee8f0..37e52cf0ce1d92 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -451,7 +451,7 @@ static ssize_t amdgpu_ras_debugfs_eeprom_write(struct 
> file *f,
> ret = amdgpu_ras_eeprom_reset_table(
> &(amdgpu_ras_get_context(adev)->eeprom_control));
>
> -   if (ret > 0) {
> +   if (!ret) {
> /* Something was written to EEPROM.
>  */
> amdgpu_ras_get_context(adev)->flags = RAS_DEFAULT_FLAGS;
> @@ -1818,12 +1818,12 @@ int amdgpu_ras_save_bad_pages(struct amdgpu_device 
> *adev)
>
> control = &con->ee

Re: [PATCH 20/40] drm/amdgpu: EEPROM respects I2C quirks

2021-06-15 Thread Alex Deucher
On Mon, Jun 14, 2021 at 1:47 PM Luben Tuikov  wrote:
>
> Consult the i2c_adapter.quirks table for
> the maximum read/write data length per bus
> transaction. Do not exceed this transaction
> limit.
>
> Cc: Jean Delvare 
> Cc: Alexander Deucher 
> Cc: Andrey Grodzovsky 
> Cc: Lijo Lazar 
> Cc: Stanley Yang 
> Cc: Hawking Zhang 
> Signed-off-by: Luben Tuikov 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 80 +-
>  1 file changed, 64 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> index 7fdb5bd2fc8bc8..94aeda1c7f8ca0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> @@ -32,20 +32,9 @@
>
>  #define EEPROM_OFFSET_SIZE 2
>
> -/**
> - * amdgpu_eeprom_xfer -- Read/write from/to an I2C EEPROM device
> - * @i2c_adap: pointer to the I2C adapter to use
> - * @slave_addr: I2C address of the slave device
> - * @eeprom_addr: EEPROM address from which to read/write
> - * @eeprom_buf: pointer to data buffer to read into/write from
> - * @buf_size: the size of @eeprom_buf
> - * @read: True if reading from the EEPROM, false if writing
> - *
> - * Returns the number of bytes read/written; -errno on error.
> - */
> -int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
> -  u16 slave_addr, u16 eeprom_addr,
> -  u8 *eeprom_buf, u16 buf_size, bool read)
> +static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
> +   u16 slave_addr, u16 eeprom_addr,
> +   u8 *eeprom_buf, u16 buf_size, bool read)
>  {
> u8 eeprom_offset_buf[EEPROM_OFFSET_SIZE];
> struct i2c_msg msgs[] = {
> @@ -65,8 +54,8 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
> u16 len;
>
> r = 0;
> -   for (len = 0; buf_size > 0;
> -buf_size -= len, eeprom_addr += len, eeprom_buf += len) {
> +   for ( ; buf_size > 0;
> + buf_size -= len, eeprom_addr += len, eeprom_buf += len) {
> /* Set the EEPROM address we want to write to/read from.
>  */
> msgs[0].buf[0] = (eeprom_addr >> 8) & 0xff;
> @@ -120,3 +109,62 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
>
> return r < 0 ? r : eeprom_buf - p;
>  }
> +
> +/**
> + * amdgpu_eeprom_xfer -- Read/write from/to an I2C EEPROM device
> + * @i2c_adap: pointer to the I2C adapter to use
> + * @slave_addr: I2C address of the slave device
> + * @eeprom_addr: EEPROM address from which to read/write
> + * @eeprom_buf: pointer to data buffer to read into/write from
> + * @buf_size: the size of @eeprom_buf
> + * @read: True if reading from the EEPROM, false if writing
> + *
> + * Returns the number of bytes read/written; -errno on error.
> + */
> +int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
> +  u16 slave_addr, u16 eeprom_addr,
> +  u8 *eeprom_buf, u16 buf_size, bool read)
> +{
> +   const struct i2c_adapter_quirks *quirks = i2c_adap->quirks;
> +   u16 limit;
> +
> +   if (!quirks)
> +   limit = 0;
> +   else if (read)
> +   limit = quirks->max_read_len;
> +   else
> +   limit = quirks->max_write_len;
> +
> +   if (limit == 0) {
> +   return __amdgpu_eeprom_xfer(i2c_adap, slave_addr, eeprom_addr,
> +   eeprom_buf, buf_size, read);
> +   } else if (limit <= EEPROM_OFFSET_SIZE) {
> +   dev_err_ratelimited(&i2c_adap->dev,
> +   "maddr:0x%04X size:0x%02X:quirk 
> max_%s_len must be > %d",
> +   eeprom_addr, buf_size,
> +   read ? "read" : "write", 
> EEPROM_OFFSET_SIZE);
> +   return -EINVAL;
> +   } else {
> +   u16 ps; /* Partial size */
> +   int res = 0, r;
> +
> +   /* The "limit" includes all data bytes sent/received,
> +* which would include the EEPROM_OFFSET_SIZE bytes.
> +* Account for them here.
> +*/
> +   limit -= EEPROM_OFFSET_SIZE;
> +   for ( ; buf_size > 0;
> + buf_size -= ps, eeprom_addr += ps, eeprom_buf += ps) {
> +   ps = min(limit, buf_size);
> +
> +   r = __amdgpu_eeprom_xfer(i2c_adap,
> +slave_addr, eeprom_addr,
> +eeprom_buf, ps, read);
> +   if (r < 0)
> +   return r;
> +   res += r;
> +   }
> +
> +   return res;
> +   }
> +}
> --
> 2.32.0
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org

Re: [PATCH 40/40] drm/amdgpu: Correctly disable the I2C IP block

2021-06-15 Thread Alex Deucher
On Mon, Jun 14, 2021 at 1:47 PM Luben Tuikov  wrote:
>
> On long transfers to the EEPROM device,
> i.e. write, it is observed that the driver aborts
> the transfer.
>
> The reason for this is that the driver isn't
> patient enough--the IC_STATUS register's contents
> is 0x27, which is MST_ACTIVITY | TFE | TFNF |
> ACTIVITY. That is, while the transmission FIFO is
> empty, we, the I2C master device, are still
> driving the bus.
>
> Implement the correct procedure to disable
> the block, as described in the DesignWare I2C
> Databook, section 3.8.3 Disabling DW_apb_i2c on
> page 56. Now there are no premature aborts on long
> data transfers.
>
> Cc: Alexander Deucher 
> Cc: Andrey Grodzovsky 
> Signed-off-by: Luben Tuikov 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 80 +-
>  1 file changed, 62 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c 
> b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c
> index 751ea2517c4380..7d74d6204d8d0a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c
> +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c
> @@ -54,12 +54,48 @@ static void smu_v11_0_i2c_set_clock_gating(struct 
> i2c_adapter *control, bool en)
> WREG32_SOC15(SMUIO, 0, mmSMUIO_PWRMGT, reg);
>  }
>
> +/* The T_I2C_POLL_US is defined as follows:
> + *
> + * "Define a timer interval (t_i2c_poll) equal to 10 times the
> + *  signalling period for the highest I2C transfer speed used in the
> + *  system and supported by DW_apb_i2c. For instance, if the highest
> + *  I2C data transfer mode is 400 kb/s, then t_i2c_poll is 25 us."  --
> + * DesignWare DW_apb_i2c Databook, Version 1.21a, section 3.8.3.1,
> + * page 56, with grammar and syntax corrections.
> + *
> + * Vcc for our device is at 1.8V which puts it at 400 kHz,
> + * see Atmel AT24CM02 datasheet, section 8.3 DC Characteristics table, page 
> 14.
> + *
> + * The procedure to disable the IP block is described in section
> + * 3.8.3 Disabling DW_apb_i2c on page 56.
> + */
> +#define I2C_SPEED_MODE_FAST 2
> +#define T_I2C_POLL_US   25
> +#define I2C_MAX_T_POLL_COUNT1000
>
> -static void smu_v11_0_i2c_enable(struct i2c_adapter *control, bool enable)
> +static int smu_v11_0_i2c_enable(struct i2c_adapter *control, bool enable)
>  {
> struct amdgpu_device *adev = to_amdgpu_device(control);
>
> WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_ENABLE, enable ? 1 : 0);
> +
> +   if (!enable) {
> +   int ii;
> +
> +   for (ii = I2C_MAX_T_POLL_COUNT; ii > 0; ii--) {
> +   u32 en_stat = RREG32_SOC15(SMUIO,
> +  0,
> +  
> mmCKSVII2C_IC_ENABLE_STATUS);
> +   if (REG_GET_FIELD(en_stat, CKSVII2C_IC_ENABLE_STATUS, 
> IC_EN))
> +   udelay(T_I2C_POLL_US);
> +   else
> +   return I2C_OK;
> +   }
> +
> +   return I2C_ABORT;
> +   }
> +
> +   return I2C_OK;
>  }
>
>  static void smu_v11_0_i2c_clear_status(struct i2c_adapter *control)
> @@ -81,8 +117,13 @@ static void smu_v11_0_i2c_configure(struct i2c_adapter 
> *control)
> reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_RESTART_EN, 1);
> reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_10BITADDR_MASTER, 0);
> reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_10BITADDR_SLAVE, 0);
> -   /* Standard mode */
> -   reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_MAX_SPEED_MODE, 2);
> +   /* The values of IC_MAX_SPEED_MODE are,
> +* 1: standard mode, 0 - 100 Kb/s,
> +* 2: fast mode, <= 400 Kb/s, or fast mode plus, <= 1000 Kb/s,
> +* 3: high speed mode, <= 3.4 Mb/s.
> +*/
> +   reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_MAX_SPEED_MODE,
> +   I2C_SPEED_MODE_FAST);
> reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_MASTER_MODE, 1);
>
> WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_CON, reg);
> @@ -404,7 +445,6 @@ static void smu_v11_0_i2c_abort(struct i2c_adapter 
> *control)
> DRM_DEBUG_DRIVER("I2C_Abort() Done.");
>  }
>
> -
>  static bool smu_v11_0_i2c_activity_done(struct i2c_adapter *control)
>  {
> struct amdgpu_device *adev = to_amdgpu_device(control);
> @@ -416,7 +456,6 @@ static bool smu_v11_0_i2c_activity_done(struct 
> i2c_adapter *control)
> reg_ic_enable_status = RREG32_SOC15(SMUIO, 0, 
> mmCKSVII2C_IC_ENABLE_STATUS);
> reg_ic_enable = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_ENABLE);
>
> -
> if ((REG_GET_FIELD(reg_ic_enable, CKSVII2C_IC_ENABLE, ENABLE) == 0) &&
> (REG_GET_FIELD(reg_ic_enable_status, CKSVII2C_IC_ENABLE_STATUS, 
> IC_EN) == 1)) {
> /*
> @@ -446,6 +485,8 @@ static bool smu_v11_0_i2c_activity_done(struct 
> i2c_adapter *control)
>
>  static void smu_v11_0_i2c_init(struct i2c_adap

Re: [PATCH 1/1] drm/amdgpu: Use spinlock_irqsave for pasid_lock

2021-06-15 Thread Zeng, Oak
Reviewed-by: Oak Zeng 

Regards,
Oak 

 

On 2021-06-14, 6:07 PM, "amd-gfx on behalf of Felix Kuehling" 
 
wrote:

This should fix a kernel LOCKDEP warning on Vega10:
[  149.416604] 
[  149.420877] WARNING: inconsistent lock state
[  149.425152] 5.11.0-kfd-fkuehlin #517 Not tainted
[  149.429770] 
[  149.434053] inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
[  149.440059] swapper/3/0 [HC1[1]:SC0[0]:HE0:SE1] takes:
[  149.445198] 9ac80e005d68 (&adev->vm_manager.pasid_lock){?.+.}-{2:2}, 
at: amdgpu_vm_get_task_info+0x25/0x90 [amdgpu]
[  149.456252] {HARDIRQ-ON-W} state was registered at:
[  149.461136]   lock_acquire+0x242/0x390
[  149.464895]   _raw_spin_lock+0x2c/0x40
[  149.468647]   amdgpu_vm_handle_fault+0x44/0x380 [amdgpu]
[  149.474187]   gmc_v9_0_process_interrupt+0xa8/0x410 [amdgpu]
...

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 3b6c0b48d0b1..0b63686fc31a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -3394,11 +3394,12 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device 
*adev, u32 pasid,
 {
bool is_compute_context = false;
struct amdgpu_bo *root;
+   unsigned long irqflags;
uint64_t value, flags;
struct amdgpu_vm *vm;
int r;

-   spin_lock(&adev->vm_manager.pasid_lock);
+   spin_lock_irqsave(&adev->vm_manager.pasid_lock, irqflags);
vm = idr_find(&adev->vm_manager.pasid_idr, pasid);
if (vm) {
root = amdgpu_bo_ref(vm->root.base.bo);
@@ -3406,7 +3407,7 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device 
*adev, u32 pasid,
} else {
root = NULL;
}
-   spin_unlock(&adev->vm_manager.pasid_lock);
+   spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, irqflags);

if (!root)
return false;
@@ -3424,11 +3425,11 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device 
*adev, u32 pasid,
goto error_unref;

/* Double check that the VM still exists */
-   spin_lock(&adev->vm_manager.pasid_lock);
+   spin_lock_irqsave(&adev->vm_manager.pasid_lock, irqflags);
vm = idr_find(&adev->vm_manager.pasid_idr, pasid);
if (vm && vm->root.base.bo != root)
vm = NULL;
-   spin_unlock(&adev->vm_manager.pasid_lock);
+   spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, irqflags);
if (!vm)
goto error_unlock;

-- 
2.32.0

___
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-gfx&data=04%7C01%7Coak.zeng%40amd.com%7Ccbbdf385c6a2403bddc408d92f80c6da%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637593052418313221%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=MdlZzRjnc9sg9ikXsu8dNnih%2FlOKRh7a4X9pNEWPSIo%3D&reserved=0

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


Re: [PATCH v3 03/14] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property

2021-06-15 Thread Werner Sembach

Am 15.06.21 um 16:14 schrieb Werner Sembach:

Add a new general drm property "active bpc" which can be used by graphic drivers
to report the applied bit depth per pixel back to userspace.

While "max bpc" can be used to change the color depth, there was no way to check
which one actually got used. While in theory the driver chooses the best/highest
color depth within the max bpc setting a user might not be fully aware what his
hardware is or isn't capable off. This is meant as a quick way to double check
the setup.

In the future, automatic color calibration for screens might also depend on this
information being available.

Signed-off-by: Werner Sembach 
---
  drivers/gpu/drm/drm_connector.c | 50 +
  include/drm/drm_connector.h |  8 ++
  2 files changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index da39e7ff6965..02c310c1ac2d 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1197,6 +1197,14 @@ static const struct drm_prop_enum_list dp_colorspaces[] 
= {
   *drm_connector_attach_max_bpc_property() to create and attach the
   *property to the connector during initialization.
   *
+ * active bpc:
+ * This read-only range property tells userspace the pixel color bit depth
+ * actually used by the hardware display engine on "the cable" on a
+ * connector. The chosen value depends on hardware capabilities, both
+ * display engine and connected monitor, and the "max bpc" property.
+ * Drivers shall use drm_connector_attach_active_bpc_property() to install
+ * this property.
+ *
   * Connectors also have one standardized atomic property:
   *
   * CRTC_ID:
@@ -2152,6 +2160,48 @@ int drm_connector_attach_max_bpc_property(struct 
drm_connector *connector,
  }
  EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
  
+/**

+ * drm_connector_attach_active_bpc_property - attach "active bpc" property
+ * @connector: connector to attach active bpc property on.
+ * @min: The minimum bit depth supported by the connector.
+ * @max: The maximum bit depth supported by the connector.
+ *
+ * This is used to check the applied bit depth on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_active_bpc_property(struct drm_connector *connector, 
int min, int max)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->active_bpc_property) {
+   prop = drm_property_create_range(dev, DRM_MODE_PROP_IMMUTABLE, 
"active bpc", min, max);
+   if (!prop)
+   return -ENOMEM;
+
+   connector->active_bpc_property = prop;
+   drm_object_attach_property(&connector->base, prop, 0);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_active_bpc_property);
+
+/**
+ * drm_connector_set_active_bpc_property - sets the active bits per color 
property for a connector
+ * @connector: drm connector
+ * @active_bpc: bits per color for the connector currently active on "the 
cable"
+ *
+ * Should be used by atomic drivers to update the active bits per color over a 
connector.
+ */
+void drm_connector_set_active_bpc_property(struct drm_connector *connector, 
int active_bpc)
+{
+   drm_object_property_set_value(&connector->base, 
connector->active_bpc_property, active_bpc);
+}
+EXPORT_SYMBOL(drm_connector_set_active_bpc_property);
+
  /**
   * drm_connector_attach_hdr_output_metadata_property - attach 
"HDR_OUTPUT_METADA" property
   * @connector: connector to attach the property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 714d1a01c065..eee86de62a5f 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1380,6 +1380,12 @@ struct drm_connector {
 */
struct drm_property *max_bpc_property;
  
+	/**

+* @active_bpc_property: Default connector property for the active bpc
+* to be driven out of the connector.
+*/
+   struct drm_property *active_bpc_property;
+
  #define DRM_CONNECTOR_POLL_HPD (1 << 0)
  #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
  #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
@@ -1702,6 +1708,8 @@ int drm_connector_set_panel_orientation_with_quirk(
int width, int height);
  int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
  int min, int max);
+int drm_connector_attach_active_bpc_property(struct drm_connector *connector, 
int min, int max);
+void drm_connector_set_active_bpc_property(struct drm_connector *connector, 
int active_bpc);
  
  /**

   * struct drm_tile_group - Tile group metadata


I looked into DSC:

In both the amd and intel implementation it's:

max_bpc >= uncompressed bpc >= dsc bpc

Currently the patch is setting active bpc to the uncompressed bpc.

I gave the DSC specification a (very br

Re: [PATCH] drm: display: Fix duplicate field initialization in dcn31

2021-06-15 Thread Rodrigo Siqueira
On 06/15, Wan Jiabing wrote:
> Fix the following coccicheck warning:
> drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c:917:56-57:
> pstate_enabled: first occurrence line 935, second occurrence line 937
> 
> Signed-off-by: Wan Jiabing 
> ---
>  drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c 
> b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
> index 0d6cb6caad81..c67bc9544f5d 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
> @@ -934,7 +934,6 @@ static const struct dc_debug_options debug_defaults_drv = 
> {
>   .dmub_command_table = true,
>   .pstate_enabled = true,
>   .use_max_lb = true,
> - .pstate_enabled = true,
>   .enable_mem_low_power = {
>   .bits = {
>   .vga = false,
> -- 
> 2.20.1
> 

Reviewed-by: Rodrigo Siqueira 

Thanks

-- 
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: [RFC PATCH] drm/ttm: Do page counting after populate callback succeed

2021-06-15 Thread Christian König

Am 15.06.21 um 14:11 schrieb Pan, Xinhui:

2021年6月15日 20:01,Christian König  写道:

Am 15.06.21 um 13:57 schrieb xinhui pan:

Amdgpu set SG flag in populate callback. So TTM still count pages in SG
BO.

It's probably better to fix this instead. E.g. why does amdgpu modify the SG 
flag during populate and not during initial creation? That doesn't seem to make 
sense.

fair enough. Let me have a try.
No idea why we set SG flag in populate years ago.


That's pretty recent IIRC. Felix moved the code around a bit to fix 
another problem.


Christian.




Christian.


One easy way to fix this is lets count pages after populate callback.

We hit one issue that amdgpu alloc many SG BOs, but TTM try to do swap
again and again even if swapout does not swap SG BOs at all.

Signed-off-by: xinhui pan 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 32 +---
  1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index a1a25410ec74..4fa0a8cd71c0 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -317,13 +317,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
if (ttm_tt_is_populated(ttm))
return 0;
  - if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-   atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
-   if (bdev->pool.use_dma32)
-   atomic_long_add(ttm->num_pages,
-   &ttm_dma32_pages_allocated);
-   }
-
while (atomic_long_read(&ttm_pages_allocated) > ttm_pages_limit ||
   atomic_long_read(&ttm_dma32_pages_allocated) >
   ttm_dma32_pages_limit) {
@@ -342,6 +335,13 @@ int ttm_tt_populate(struct ttm_device *bdev,
if (ret)
goto error;
  + if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
+   atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
+   if (bdev->pool.use_dma32)
+   atomic_long_add(ttm->num_pages,
+   &ttm_dma32_pages_allocated);
+   }
+
ttm_tt_add_mapping(bdev, ttm);
ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED;
if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) {
@@ -355,12 +355,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
return 0;
error:
-   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-   atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
-   if (bdev->pool.use_dma32)
-   atomic_long_sub(ttm->num_pages,
-   &ttm_dma32_pages_allocated);
-   }
return ret;
  }
  EXPORT_SYMBOL(ttm_tt_populate);
@@ -384,12 +378,6 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
ttm_tt *ttm)
if (!ttm_tt_is_populated(ttm))
return;
  - ttm_tt_clear_mapping(ttm);
-   if (bdev->funcs->ttm_tt_unpopulate)
-   bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
-   else
-   ttm_pool_free(&bdev->pool, ttm);
-
if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
if (bdev->pool.use_dma32)
@@ -397,6 +385,12 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
ttm_tt *ttm)
&ttm_dma32_pages_allocated);
}
  + ttm_tt_clear_mapping(ttm);
+   if (bdev->funcs->ttm_tt_unpopulate)
+   bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
+   else
+   ttm_pool_free(&bdev->pool, ttm);
+
ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED;
  }
  


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


Re: [RFC PATCH] drm/ttm: Do page counting after populate callback succeed

2021-06-15 Thread Pan, Xinhui


> 2021年6月15日 20:01,Christian König  写道:
> 
> Am 15.06.21 um 13:57 schrieb xinhui pan:
>> Amdgpu set SG flag in populate callback. So TTM still count pages in SG
>> BO.
> 
> It's probably better to fix this instead. E.g. why does amdgpu modify the SG 
> flag during populate and not during initial creation? That doesn't seem to 
> make sense.

fair enough. Let me have a try.
No idea why we set SG flag in populate years ago.

> 
> Christian.
> 
>> One easy way to fix this is lets count pages after populate callback.
>> 
>> We hit one issue that amdgpu alloc many SG BOs, but TTM try to do swap
>> again and again even if swapout does not swap SG BOs at all.
>> 
>> Signed-off-by: xinhui pan 
>> ---
>>  drivers/gpu/drm/ttm/ttm_tt.c | 32 +---
>>  1 file changed, 13 insertions(+), 19 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
>> index a1a25410ec74..4fa0a8cd71c0 100644
>> --- a/drivers/gpu/drm/ttm/ttm_tt.c
>> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
>> @@ -317,13 +317,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
>>  if (ttm_tt_is_populated(ttm))
>>  return 0;
>>  -   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
>> -atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
>> -if (bdev->pool.use_dma32)
>> -atomic_long_add(ttm->num_pages,
>> -&ttm_dma32_pages_allocated);
>> -}
>> -
>>  while (atomic_long_read(&ttm_pages_allocated) > ttm_pages_limit ||
>> atomic_long_read(&ttm_dma32_pages_allocated) >
>> ttm_dma32_pages_limit) {
>> @@ -342,6 +335,13 @@ int ttm_tt_populate(struct ttm_device *bdev,
>>  if (ret)
>>  goto error;
>>  +   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
>> +atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
>> +if (bdev->pool.use_dma32)
>> +atomic_long_add(ttm->num_pages,
>> +&ttm_dma32_pages_allocated);
>> +}
>> +
>>  ttm_tt_add_mapping(bdev, ttm);
>>  ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED;
>>  if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) {
>> @@ -355,12 +355,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
>>  return 0;
>>error:
>> -if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
>> -atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
>> -if (bdev->pool.use_dma32)
>> -atomic_long_sub(ttm->num_pages,
>> -&ttm_dma32_pages_allocated);
>> -}
>>  return ret;
>>  }
>>  EXPORT_SYMBOL(ttm_tt_populate);
>> @@ -384,12 +378,6 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
>> ttm_tt *ttm)
>>  if (!ttm_tt_is_populated(ttm))
>>  return;
>>  -   ttm_tt_clear_mapping(ttm);
>> -if (bdev->funcs->ttm_tt_unpopulate)
>> -bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
>> -else
>> -ttm_pool_free(&bdev->pool, ttm);
>> -
>>  if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
>>  atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
>>  if (bdev->pool.use_dma32)
>> @@ -397,6 +385,12 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
>> ttm_tt *ttm)
>>  &ttm_dma32_pages_allocated);
>>  }
>>  +   ttm_tt_clear_mapping(ttm);
>> +if (bdev->funcs->ttm_tt_unpopulate)
>> +bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
>> +else
>> +ttm_pool_free(&bdev->pool, ttm);
>> +
>>  ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED;
>>  }
>>  
> 

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


Re: [PATCH 2/2] drm/amdgpu: move shadow_list to amdgpu_bo_vm

2021-06-15 Thread Das, Nirmoy


On 6/15/2021 1:57 PM, Christian König wrote:



Am 15.06.21 um 13:51 schrieb Nirmoy Das:

Move shadow_list to struct amdgpu_bo_vm as shadow BOs
are part of PT/PD BOs.

Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  5 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 14 --
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  6 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c |  2 +-
  4 files changed, 14 insertions(+), 13 deletions(-)

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

index f2636f411a25..3f51b142fc83 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4063,6 +4063,7 @@ static int amdgpu_device_recover_vram(struct 
amdgpu_device *adev)

  {
  struct dma_fence *fence = NULL, *next = NULL;
  struct amdgpu_bo *shadow;
+    struct amdgpu_bo_vm *vmbo;
  long r = 1, tmo;
    if (amdgpu_sriov_runtime(adev))
@@ -4072,8 +4073,8 @@ static int amdgpu_device_recover_vram(struct 
amdgpu_device *adev)

    dev_info(adev->dev, "recover vram bo from shadow start\n");
  mutex_lock(&adev->shadow_list_lock);
-    list_for_each_entry(shadow, &adev->shadow_list, shadow_list) {
-
+    list_for_each_entry(vmbo, &adev->shadow_list, shadow_list) {
+    shadow = &vmbo->bo;
  /* No need to recover an evicted BO */
  if (shadow->tbo.mem.mem_type != TTM_PL_TT ||
  shadow->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET ||
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c

index ea54fd739c41..ea339eaac399 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -103,11 +103,13 @@ static void amdgpu_bo_vm_destroy(struct 
ttm_buffer_object *tbo)

  {
  struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
  struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+    struct amdgpu_bo_vm *vmbo;
  +    vmbo = to_amdgpu_bo_vm(bo);
  /* in case amdgpu_device_recover_vram got NULL of bo->parent */
-    if (!list_empty(&bo->shadow_list)) {
+    if (!list_empty(&vmbo->shadow_list)) {
  mutex_lock(&adev->shadow_list_lock);
-    list_del_init(&bo->shadow_list);
+    list_del_init(&vmbo->shadow_list);
  mutex_unlock(&adev->shadow_list_lock);
  }
  @@ -583,7 +585,6 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
  if (bo == NULL)
  return -ENOMEM;
  drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, 
size);

-    INIT_LIST_HEAD(&bo->shadow_list);
  bo->vm_bo = NULL;
  bo->preferred_domains = bp->preferred_domain ? 
bp->preferred_domain :

  bp->domain;
@@ -713,6 +714,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
  return r;
    *vmbo_ptr = to_amdgpu_bo_vm(bo_ptr);
+    INIT_LIST_HEAD(&(*vmbo_ptr)->shadow_list);
  return r;
  }
  @@ -757,12 +759,12 @@ int amdgpu_bo_validate(struct amdgpu_bo *bo)
   *
   * Insert a BO to the shadow list.
   */
-void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo)
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo)
  {
-    struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+    struct amdgpu_device *adev = amdgpu_ttm_adev(vmbo->bo.tbo.bdev);
    mutex_lock(&adev->shadow_list_lock);
-    list_add_tail(&bo->shadow_list, &adev->shadow_list);
+    list_add_tail(&vmbo->shadow_list, &adev->shadow_list);
  mutex_unlock(&adev->shadow_list_lock);
  }
  diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h

index a8c702634e1b..18cb2f28e4de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -109,9 +109,6 @@ struct amdgpu_bo {
  #ifdef CONFIG_MMU_NOTIFIER
  struct mmu_interval_notifier    notifier;
  #endif
-
-    struct list_head    shadow_list;
-
  struct kgd_mem  *kfd_bo;
  };
  @@ -127,6 +124,7 @@ struct amdgpu_bo_user {
  struct amdgpu_bo_vm {
  struct amdgpu_bo    bo;
  struct amdgpu_bo    *shadow;
+    struct list_head    shadow_list;
  struct amdgpu_vm_bo_base    entries[];
  };
  @@ -333,7 +331,7 @@ u64 amdgpu_bo_gpu_offset_no_check(struct 
amdgpu_bo *bo);

  int amdgpu_bo_validate(struct amdgpu_bo *bo);
  void amdgpu_bo_get_memory(struct amdgpu_bo *bo, uint64_t *vram_mem,
  uint64_t *gtt_mem, uint64_t *cpu_mem);
-void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo);
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo);
  int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
   struct dma_fence **fence);
  uint32_t amdgpu_bo_get_preferred_pin_domain(struct amdgpu_device 
*adev,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index 4c4c56581780..812c225538a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/driv

[RFC PATCH] drm/ttm: Do page counting after populate callback succeed

2021-06-15 Thread xinhui pan
Amdgpu set SG flag in populate callback. So TTM still count pages in SG
BO.
One easy way to fix this is lets count pages after populate callback.

We hit one issue that amdgpu alloc many SG BOs, but TTM try to do swap
again and again even if swapout does not swap SG BOs at all.

Signed-off-by: xinhui pan 
---
 drivers/gpu/drm/ttm/ttm_tt.c | 32 +---
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index a1a25410ec74..4fa0a8cd71c0 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -317,13 +317,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
if (ttm_tt_is_populated(ttm))
return 0;
 
-   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-   atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
-   if (bdev->pool.use_dma32)
-   atomic_long_add(ttm->num_pages,
-   &ttm_dma32_pages_allocated);
-   }
-
while (atomic_long_read(&ttm_pages_allocated) > ttm_pages_limit ||
   atomic_long_read(&ttm_dma32_pages_allocated) >
   ttm_dma32_pages_limit) {
@@ -342,6 +335,13 @@ int ttm_tt_populate(struct ttm_device *bdev,
if (ret)
goto error;
 
+   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
+   atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
+   if (bdev->pool.use_dma32)
+   atomic_long_add(ttm->num_pages,
+   &ttm_dma32_pages_allocated);
+   }
+
ttm_tt_add_mapping(bdev, ttm);
ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED;
if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) {
@@ -355,12 +355,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
return 0;
 
 error:
-   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-   atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
-   if (bdev->pool.use_dma32)
-   atomic_long_sub(ttm->num_pages,
-   &ttm_dma32_pages_allocated);
-   }
return ret;
 }
 EXPORT_SYMBOL(ttm_tt_populate);
@@ -384,12 +378,6 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
ttm_tt *ttm)
if (!ttm_tt_is_populated(ttm))
return;
 
-   ttm_tt_clear_mapping(ttm);
-   if (bdev->funcs->ttm_tt_unpopulate)
-   bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
-   else
-   ttm_pool_free(&bdev->pool, ttm);
-
if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
if (bdev->pool.use_dma32)
@@ -397,6 +385,12 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
ttm_tt *ttm)
&ttm_dma32_pages_allocated);
}
 
+   ttm_tt_clear_mapping(ttm);
+   if (bdev->funcs->ttm_tt_unpopulate)
+   bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
+   else
+   ttm_pool_free(&bdev->pool, ttm);
+
ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED;
 }
 
-- 
2.25.1

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


Re: [PATCH 2/2] drm/amdgpu: move shadow_list to amdgpu_bo_vm

2021-06-15 Thread Christian König



Am 15.06.21 um 13:51 schrieb Nirmoy Das:

Move shadow_list to struct amdgpu_bo_vm as shadow BOs
are part of PT/PD BOs.

Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  5 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 14 --
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  6 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c |  2 +-
  4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f2636f411a25..3f51b142fc83 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4063,6 +4063,7 @@ static int amdgpu_device_recover_vram(struct 
amdgpu_device *adev)
  {
struct dma_fence *fence = NULL, *next = NULL;
struct amdgpu_bo *shadow;
+   struct amdgpu_bo_vm *vmbo;
long r = 1, tmo;
  
  	if (amdgpu_sriov_runtime(adev))

@@ -4072,8 +4073,8 @@ static int amdgpu_device_recover_vram(struct 
amdgpu_device *adev)
  
  	dev_info(adev->dev, "recover vram bo from shadow start\n");

mutex_lock(&adev->shadow_list_lock);
-   list_for_each_entry(shadow, &adev->shadow_list, shadow_list) {
-
+   list_for_each_entry(vmbo, &adev->shadow_list, shadow_list) {
+   shadow = &vmbo->bo;
/* No need to recover an evicted BO */
if (shadow->tbo.mem.mem_type != TTM_PL_TT ||
shadow->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET ||
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index ea54fd739c41..ea339eaac399 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -103,11 +103,13 @@ static void amdgpu_bo_vm_destroy(struct ttm_buffer_object 
*tbo)
  {
struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+   struct amdgpu_bo_vm *vmbo;
  
+	vmbo = to_amdgpu_bo_vm(bo);

/* in case amdgpu_device_recover_vram got NULL of bo->parent */
-   if (!list_empty(&bo->shadow_list)) {
+   if (!list_empty(&vmbo->shadow_list)) {
mutex_lock(&adev->shadow_list_lock);
-   list_del_init(&bo->shadow_list);
+   list_del_init(&vmbo->shadow_list);
mutex_unlock(&adev->shadow_list_lock);
}
  
@@ -583,7 +585,6 @@ int amdgpu_bo_create(struct amdgpu_device *adev,

if (bo == NULL)
return -ENOMEM;
drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
-   INIT_LIST_HEAD(&bo->shadow_list);
bo->vm_bo = NULL;
bo->preferred_domains = bp->preferred_domain ? bp->preferred_domain :
bp->domain;
@@ -713,6 +714,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
return r;
  
  	*vmbo_ptr = to_amdgpu_bo_vm(bo_ptr);

+   INIT_LIST_HEAD(&(*vmbo_ptr)->shadow_list);
return r;
  }
  
@@ -757,12 +759,12 @@ int amdgpu_bo_validate(struct amdgpu_bo *bo)

   *
   * Insert a BO to the shadow list.
   */
-void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo)
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo)
  {
-   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+   struct amdgpu_device *adev = amdgpu_ttm_adev(vmbo->bo.tbo.bdev);
  
  	mutex_lock(&adev->shadow_list_lock);

-   list_add_tail(&bo->shadow_list, &adev->shadow_list);
+   list_add_tail(&vmbo->shadow_list, &adev->shadow_list);
mutex_unlock(&adev->shadow_list_lock);
  }
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h

index a8c702634e1b..18cb2f28e4de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -109,9 +109,6 @@ struct amdgpu_bo {
  #ifdef CONFIG_MMU_NOTIFIER
struct mmu_interval_notifiernotifier;
  #endif
-
-   struct list_headshadow_list;
-
struct kgd_mem  *kfd_bo;
  };
  
@@ -127,6 +124,7 @@ struct amdgpu_bo_user {

  struct amdgpu_bo_vm {
struct amdgpu_bobo;
struct amdgpu_bo*shadow;
+   struct list_headshadow_list;
struct amdgpu_vm_bo_baseentries[];
  };
  
@@ -333,7 +331,7 @@ u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);

  int amdgpu_bo_validate(struct amdgpu_bo *bo);
  void amdgpu_bo_get_memory(struct amdgpu_bo *bo, uint64_t *vram_mem,
uint64_t *gtt_mem, uint64_t *cpu_mem);
-void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo);
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo);
  int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
 struct dma_fence **fence);
  uint32_t amdgpu_bo_get_preferred_pin_domain(struct amdgpu_device *adev,
diff --git a/drivers/gpu/drm/amd/amdgpu/a

[PATCH v3 1/2] drm/amdgpu: parameterize ttm BO destroy callback

2021-06-15 Thread Nirmoy Das
Make provision to pass different ttm BO destroy callback
while creating a amdgpu_bo.

v3: remove unnecessary amdgpu_bo_destroy_base.
v2: remove whitespace.
call amdgpu_bo_destroy_base() at the end for cleaner code.

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 41 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 9092ac12a270..ea54fd739c41 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -75,9 +75,7 @@ static void amdgpu_bo_subtract_pin_size(struct amdgpu_bo *bo)

 static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
 {
-   struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
-   struct amdgpu_bo_user *ubo;

if (bo->tbo.pin_count > 0)
amdgpu_bo_subtract_pin_size(bo);
@@ -87,20 +85,33 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
if (bo->tbo.base.import_attach)
drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
drm_gem_object_release(&bo->tbo.base);
+   amdgpu_bo_unref(&bo->parent);
+   kvfree(bo);
+}
+
+static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
+{
+   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+   struct amdgpu_bo_user *ubo;
+
+   ubo = to_amdgpu_bo_user(bo);
+   kfree(ubo->metadata);
+   amdgpu_bo_destroy(tbo);
+}
+
+static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
+{
+   struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
+   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+
/* in case amdgpu_device_recover_vram got NULL of bo->parent */
if (!list_empty(&bo->shadow_list)) {
mutex_lock(&adev->shadow_list_lock);
list_del_init(&bo->shadow_list);
mutex_unlock(&adev->shadow_list_lock);
}
-   amdgpu_bo_unref(&bo->parent);
-
-   if (bo->tbo.type != ttm_bo_type_kernel) {
-   ubo = to_amdgpu_bo_user(bo);
-   kfree(ubo->metadata);
-   }

-   kvfree(bo);
+   amdgpu_bo_destroy(tbo);
 }

 /**
@@ -115,8 +126,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object 
*tbo)
  */
 bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
 {
-   if (bo->destroy == &amdgpu_bo_destroy)
+   if (bo->destroy == &amdgpu_bo_destroy ||
+   bo->destroy == &amdgpu_bo_user_destroy ||
+   bo->destroy == &amdgpu_bo_vm_destroy)
return true;
+
return false;
 }

@@ -592,9 +606,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bp->type == ttm_bo_type_kernel)
bo->tbo.priority = 1;

+   if (!bp->destroy)
+   bp->destroy = &amdgpu_bo_destroy;
+
r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type,
 &bo->placement, page_align, &ctx,  NULL,
-bp->resv, &amdgpu_bo_destroy);
+bp->resv, bp->destroy);
if (unlikely(r != 0))
return r;

@@ -658,6 +675,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
int r;

bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
+   bp->destroy = &amdgpu_bo_user_destroy;
r = amdgpu_bo_create(adev, bp, &bo_ptr);
if (r)
return r;
@@ -689,6 +707,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
 * num of amdgpu_vm_pt entries.
 */
BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
+   bp->destroy = &amdgpu_bo_vm_destroy;
r = amdgpu_bo_create(adev, bp, &bo_ptr);
if (r)
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index e36b84516b4e..a8c702634e1b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -55,7 +55,8 @@ struct amdgpu_bo_param {
u64 flags;
enum ttm_bo_typetype;
boolno_wait_gpu;
-   struct dma_resv *resv;
+   struct dma_resv *resv;
+   void(*destroy)(struct ttm_buffer_object 
*bo);
 };

 /* bo virtual addresses in a vm */
--
2.31.1

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


[PATCH 2/2] drm/amdgpu: move shadow_list to amdgpu_bo_vm

2021-06-15 Thread Nirmoy Das
Move shadow_list to struct amdgpu_bo_vm as shadow BOs
are part of PT/PD BOs.

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  5 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 14 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  6 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c |  2 +-
 4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f2636f411a25..3f51b142fc83 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4063,6 +4063,7 @@ static int amdgpu_device_recover_vram(struct 
amdgpu_device *adev)
 {
struct dma_fence *fence = NULL, *next = NULL;
struct amdgpu_bo *shadow;
+   struct amdgpu_bo_vm *vmbo;
long r = 1, tmo;
 
if (amdgpu_sriov_runtime(adev))
@@ -4072,8 +4073,8 @@ static int amdgpu_device_recover_vram(struct 
amdgpu_device *adev)
 
dev_info(adev->dev, "recover vram bo from shadow start\n");
mutex_lock(&adev->shadow_list_lock);
-   list_for_each_entry(shadow, &adev->shadow_list, shadow_list) {
-
+   list_for_each_entry(vmbo, &adev->shadow_list, shadow_list) {
+   shadow = &vmbo->bo;
/* No need to recover an evicted BO */
if (shadow->tbo.mem.mem_type != TTM_PL_TT ||
shadow->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET ||
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index ea54fd739c41..ea339eaac399 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -103,11 +103,13 @@ static void amdgpu_bo_vm_destroy(struct ttm_buffer_object 
*tbo)
 {
struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+   struct amdgpu_bo_vm *vmbo;
 
+   vmbo = to_amdgpu_bo_vm(bo);
/* in case amdgpu_device_recover_vram got NULL of bo->parent */
-   if (!list_empty(&bo->shadow_list)) {
+   if (!list_empty(&vmbo->shadow_list)) {
mutex_lock(&adev->shadow_list_lock);
-   list_del_init(&bo->shadow_list);
+   list_del_init(&vmbo->shadow_list);
mutex_unlock(&adev->shadow_list_lock);
}
 
@@ -583,7 +585,6 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bo == NULL)
return -ENOMEM;
drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
-   INIT_LIST_HEAD(&bo->shadow_list);
bo->vm_bo = NULL;
bo->preferred_domains = bp->preferred_domain ? bp->preferred_domain :
bp->domain;
@@ -713,6 +714,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
return r;
 
*vmbo_ptr = to_amdgpu_bo_vm(bo_ptr);
+   INIT_LIST_HEAD(&(*vmbo_ptr)->shadow_list);
return r;
 }
 
@@ -757,12 +759,12 @@ int amdgpu_bo_validate(struct amdgpu_bo *bo)
  *
  * Insert a BO to the shadow list.
  */
-void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo)
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo)
 {
-   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+   struct amdgpu_device *adev = amdgpu_ttm_adev(vmbo->bo.tbo.bdev);
 
mutex_lock(&adev->shadow_list_lock);
-   list_add_tail(&bo->shadow_list, &adev->shadow_list);
+   list_add_tail(&vmbo->shadow_list, &adev->shadow_list);
mutex_unlock(&adev->shadow_list_lock);
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index a8c702634e1b..18cb2f28e4de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -109,9 +109,6 @@ struct amdgpu_bo {
 #ifdef CONFIG_MMU_NOTIFIER
struct mmu_interval_notifiernotifier;
 #endif
-
-   struct list_headshadow_list;
-
struct kgd_mem  *kfd_bo;
 };
 
@@ -127,6 +124,7 @@ struct amdgpu_bo_user {
 struct amdgpu_bo_vm {
struct amdgpu_bobo;
struct amdgpu_bo*shadow;
+   struct list_headshadow_list;
struct amdgpu_vm_bo_baseentries[];
 };
 
@@ -333,7 +331,7 @@ u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);
 int amdgpu_bo_validate(struct amdgpu_bo *bo);
 void amdgpu_bo_get_memory(struct amdgpu_bo *bo, uint64_t *vram_mem,
uint64_t *gtt_mem, uint64_t *cpu_mem);
-void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo);
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo);
 int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
 struct dma_fence **fence);
 uint32_t amdgpu_bo_get_preferred_pin_domain(struct amdgpu_device *adev,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 4

[PATCH AUTOSEL 4.4 3/3] radeon: use memcpy_to/fromio for UVD fw upload

2021-06-15 Thread Sasha Levin
From: Chen Li 

[ Upstream commit ab8363d3875a83f4901eb1cc00ce8afd24de6c85 ]

I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Signed-off-by: Chen Li 
Reviewed-by: Christian König 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c 
b/drivers/gpu/drm/radeon/radeon_uvd.c
index b35ebabd6a9f..eab985fdcfbd 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -242,7 +242,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
if (rdev->uvd.vcpu_bo == NULL)
return -EINVAL;
 
-   memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+   memcpy_toio((void __iomem *)rdev->uvd.cpu_addr, rdev->uvd_fw->data, 
rdev->uvd_fw->size);
 
size = radeon_bo_size(rdev->uvd.vcpu_bo);
size -= rdev->uvd_fw->size;
@@ -250,7 +250,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
ptr = rdev->uvd.cpu_addr;
ptr += rdev->uvd_fw->size;
 
-   memset(ptr, 0, size);
+   memset_io((void __iomem *)ptr, 0, size);
 
return 0;
 }
-- 
2.30.2

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


[PATCH AUTOSEL 4.9 5/5] radeon: use memcpy_to/fromio for UVD fw upload

2021-06-15 Thread Sasha Levin
From: Chen Li 

[ Upstream commit ab8363d3875a83f4901eb1cc00ce8afd24de6c85 ]

I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Signed-off-by: Chen Li 
Reviewed-by: Christian König 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c 
b/drivers/gpu/drm/radeon/radeon_uvd.c
index 16239b07ce45..2610919eb709 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -286,7 +286,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
if (rdev->uvd.vcpu_bo == NULL)
return -EINVAL;
 
-   memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+   memcpy_toio((void __iomem *)rdev->uvd.cpu_addr, rdev->uvd_fw->data, 
rdev->uvd_fw->size);
 
size = radeon_bo_size(rdev->uvd.vcpu_bo);
size -= rdev->uvd_fw->size;
@@ -294,7 +294,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
ptr = rdev->uvd.cpu_addr;
ptr += rdev->uvd_fw->size;
 
-   memset(ptr, 0, size);
+   memset_io((void __iomem *)ptr, 0, size);
 
return 0;
 }
-- 
2.30.2

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


[PATCH AUTOSEL 4.14 5/8] radeon: use memcpy_to/fromio for UVD fw upload

2021-06-15 Thread Sasha Levin
From: Chen Li 

[ Upstream commit ab8363d3875a83f4901eb1cc00ce8afd24de6c85 ]

I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Signed-off-by: Chen Li 
Reviewed-by: Christian König 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c 
b/drivers/gpu/drm/radeon/radeon_uvd.c
index 95f4db70dd22..fde9c69ecc86 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -286,7 +286,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
if (rdev->uvd.vcpu_bo == NULL)
return -EINVAL;
 
-   memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+   memcpy_toio((void __iomem *)rdev->uvd.cpu_addr, rdev->uvd_fw->data, 
rdev->uvd_fw->size);
 
size = radeon_bo_size(rdev->uvd.vcpu_bo);
size -= rdev->uvd_fw->size;
@@ -294,7 +294,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
ptr = rdev->uvd.cpu_addr;
ptr += rdev->uvd_fw->size;
 
-   memset(ptr, 0, size);
+   memset_io((void __iomem *)ptr, 0, size);
 
return 0;
 }
-- 
2.30.2

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


[PATCH AUTOSEL 4.19 09/12] radeon: use memcpy_to/fromio for UVD fw upload

2021-06-15 Thread Sasha Levin
From: Chen Li 

[ Upstream commit ab8363d3875a83f4901eb1cc00ce8afd24de6c85 ]

I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Signed-off-by: Chen Li 
Reviewed-by: Christian König 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c 
b/drivers/gpu/drm/radeon/radeon_uvd.c
index 95f4db70dd22..fde9c69ecc86 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -286,7 +286,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
if (rdev->uvd.vcpu_bo == NULL)
return -EINVAL;
 
-   memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+   memcpy_toio((void __iomem *)rdev->uvd.cpu_addr, rdev->uvd_fw->data, 
rdev->uvd_fw->size);
 
size = radeon_bo_size(rdev->uvd.vcpu_bo);
size -= rdev->uvd_fw->size;
@@ -294,7 +294,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
ptr = rdev->uvd.cpu_addr;
ptr += rdev->uvd_fw->size;
 
-   memset(ptr, 0, size);
+   memset_io((void __iomem *)ptr, 0, size);
 
return 0;
 }
-- 
2.30.2

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


[PATCH AUTOSEL 5.4 12/15] radeon: use memcpy_to/fromio for UVD fw upload

2021-06-15 Thread Sasha Levin
From: Chen Li 

[ Upstream commit ab8363d3875a83f4901eb1cc00ce8afd24de6c85 ]

I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Signed-off-by: Chen Li 
Reviewed-by: Christian König 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c 
b/drivers/gpu/drm/radeon/radeon_uvd.c
index 1ad5c3b86b64..a18bf70a251e 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -286,7 +286,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
if (rdev->uvd.vcpu_bo == NULL)
return -EINVAL;
 
-   memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+   memcpy_toio((void __iomem *)rdev->uvd.cpu_addr, rdev->uvd_fw->data, 
rdev->uvd_fw->size);
 
size = radeon_bo_size(rdev->uvd.vcpu_bo);
size -= rdev->uvd_fw->size;
@@ -294,7 +294,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
ptr = rdev->uvd.cpu_addr;
ptr += rdev->uvd_fw->size;
 
-   memset(ptr, 0, size);
+   memset_io((void __iomem *)ptr, 0, size);
 
return 0;
 }
-- 
2.30.2

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


[PATCH AUTOSEL 5.10 26/30] radeon: use memcpy_to/fromio for UVD fw upload

2021-06-15 Thread Sasha Levin
From: Chen Li 

[ Upstream commit ab8363d3875a83f4901eb1cc00ce8afd24de6c85 ]

I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Signed-off-by: Chen Li 
Reviewed-by: Christian König 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c 
b/drivers/gpu/drm/radeon/radeon_uvd.c
index 57fb3eb3a4b4..1f4e3396d097 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -286,7 +286,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
if (rdev->uvd.vcpu_bo == NULL)
return -EINVAL;
 
-   memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+   memcpy_toio((void __iomem *)rdev->uvd.cpu_addr, rdev->uvd_fw->data, 
rdev->uvd_fw->size);
 
size = radeon_bo_size(rdev->uvd.vcpu_bo);
size -= rdev->uvd_fw->size;
@@ -294,7 +294,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
ptr = rdev->uvd.cpu_addr;
ptr += rdev->uvd_fw->size;
 
-   memset(ptr, 0, size);
+   memset_io((void __iomem *)ptr, 0, size);
 
return 0;
 }
-- 
2.30.2

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


[PATCH AUTOSEL 5.12 28/33] radeon: use memcpy_to/fromio for UVD fw upload

2021-06-15 Thread Sasha Levin
From: Chen Li 

[ Upstream commit ab8363d3875a83f4901eb1cc00ce8afd24de6c85 ]

I met a gpu addr bug recently and the kernel log
tells me the pc is memcpy/memset and link register is
radeon_uvd_resume.

As we know, in some architectures, optimized memcpy/memset
may not work well on device memory. Trival memcpy_toio/memset_io
can fix this problem.

BTW, amdgpu has already done it in:
commit ba0b2275a678 ("drm/amdgpu: use memcpy_to/fromio for UVD fw upload"),
that's why it has no this issue on the same gpu and platform.

Signed-off-by: Chen Li 
Reviewed-by: Christian König 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/radeon/radeon_uvd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_uvd.c 
b/drivers/gpu/drm/radeon/radeon_uvd.c
index dfa9fdbe98da..06bb24d7a9fe 100644
--- a/drivers/gpu/drm/radeon/radeon_uvd.c
+++ b/drivers/gpu/drm/radeon/radeon_uvd.c
@@ -286,7 +286,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
if (rdev->uvd.vcpu_bo == NULL)
return -EINVAL;
 
-   memcpy(rdev->uvd.cpu_addr, rdev->uvd_fw->data, rdev->uvd_fw->size);
+   memcpy_toio((void __iomem *)rdev->uvd.cpu_addr, rdev->uvd_fw->data, 
rdev->uvd_fw->size);
 
size = radeon_bo_size(rdev->uvd.vcpu_bo);
size -= rdev->uvd_fw->size;
@@ -294,7 +294,7 @@ int radeon_uvd_resume(struct radeon_device *rdev)
ptr = rdev->uvd.cpu_addr;
ptr += rdev->uvd_fw->size;
 
-   memset(ptr, 0, size);
+   memset_io((void __iomem *)ptr, 0, size);
 
return 0;
 }
-- 
2.30.2

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


Re: [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback

2021-06-15 Thread Das, Nirmoy


On 6/15/2021 12:48 PM, Christian König wrote:



Am 15.06.21 um 11:23 schrieb Nirmoy Das:

Make provision to pass different ttm BO destroy callback
while creating a amdgpu_bo.

v2: remove whitespace.
 call amdgpu_bo_destroy_base() at the end for cleaner code.

Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 48 --
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
  2 files changed, 38 insertions(+), 13 deletions(-)

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

index 9092ac12a270..8473d153650f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct 
amdgpu_bo *bo)

  }
  }

-static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)


I think you don't even need to rename the function.


  {
-    struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
  struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
-    struct amdgpu_bo_user *ubo;

  if (bo->tbo.pin_count > 0)
  amdgpu_bo_subtract_pin_size(bo);
@@ -87,20 +85,38 @@ static void amdgpu_bo_destroy(struct 
ttm_buffer_object *tbo)

  if (bo->tbo.base.import_attach)
  drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
  drm_gem_object_release(&bo->tbo.base);
+    amdgpu_bo_unref(&bo->parent);
+    kvfree(bo);
+}
+
+static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+{
+    amdgpu_bo_destroy_base(tbo);
+}


Nor has that wrapper here.

Apart from that looks good to me.



Thanks, let me resend with that.


Nirmoy



Christian.


+
+static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
+{
+    struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+    struct amdgpu_bo_user *ubo;
+
+    ubo = to_amdgpu_bo_user(bo);
+    kfree(ubo->metadata);
+    amdgpu_bo_destroy_base(tbo);
+}
+
+static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
+{
+    struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
+    struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+
  /* in case amdgpu_device_recover_vram got NULL of bo->parent */
  if (!list_empty(&bo->shadow_list)) {
  mutex_lock(&adev->shadow_list_lock);
  list_del_init(&bo->shadow_list);
  mutex_unlock(&adev->shadow_list_lock);
  }
-    amdgpu_bo_unref(&bo->parent);
-
-    if (bo->tbo.type != ttm_bo_type_kernel) {
-    ubo = to_amdgpu_bo_user(bo);
-    kfree(ubo->metadata);
-    }

-    kvfree(bo);
+    amdgpu_bo_destroy_base(tbo);
  }

  /**
@@ -115,8 +131,11 @@ static void amdgpu_bo_destroy(struct 
ttm_buffer_object *tbo)

   */
  bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
  {
-    if (bo->destroy == &amdgpu_bo_destroy)
+    if (bo->destroy == &amdgpu_bo_destroy ||
+    bo->destroy == &amdgpu_bo_user_destroy ||
+    bo->destroy == &amdgpu_bo_vm_destroy)
  return true;
+
  return false;
  }

@@ -592,9 +611,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
  if (bp->type == ttm_bo_type_kernel)
  bo->tbo.priority = 1;

+    if (!bp->destroy)
+    bp->destroy = &amdgpu_bo_destroy;
+
  r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, 
bp->type,

   &bo->placement, page_align, &ctx, NULL,
- bp->resv, &amdgpu_bo_destroy);
+ bp->resv, bp->destroy);
  if (unlikely(r != 0))
  return r;

@@ -658,6 +680,7 @@ int amdgpu_bo_create_user(struct amdgpu_device 
*adev,

  int r;

  bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
+    bp->destroy = &amdgpu_bo_user_destroy;
  r = amdgpu_bo_create(adev, bp, &bo_ptr);
  if (r)
  return r;
@@ -689,6 +712,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
   * num of amdgpu_vm_pt entries.
   */
  BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
+    bp->destroy = &amdgpu_bo_vm_destroy;
  r = amdgpu_bo_create(adev, bp, &bo_ptr);
  if (r)
  return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h

index e36b84516b4e..a8c702634e1b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -55,7 +55,8 @@ struct amdgpu_bo_param {
  u64    flags;
  enum ttm_bo_type    type;
  bool    no_wait_gpu;
-    struct dma_resv    *resv;
+    struct dma_resv    *resv;
+    void    (*destroy)(struct ttm_buffer_object *bo);
  };

  /* bo virtual addresses in a vm */
--
2.31.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-gfx&data=04%7C01%7Cnirmoy.das%40amd.com%7Cb321ddc590404256e9c808d92feb2a0a%7C3dd8961fe48

RE: [PATCH 1/1] drm/amdkfd: remove unused variable

2021-06-15 Thread Kim, Jonathan
[AMD Official Use Only]

Thanks for the catch.
Reviewed-by: Jonathan Kim 

> -Original Message-
> From: Das, Nirmoy 
> Sent: Tuesday, June 15, 2021 6:35 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Kim, Jonathan ; Kuehling, Felix
> 
> Subject: Re: [PATCH 1/1] drm/amdkfd: remove unused variable
>
>
> On 6/15/2021 12:33 PM, Nirmoy Das wrote:
> > Remove it.
> >
> > CC: jonathan@amd.com
> > CC: felix.kuehl...@amd.com
> > Fixes: d7b132507384c("drm/amdkfd: fix circular locking on get_wave_state")
> > Signed-off-by: Nirmoy Das 
> > ---
> >   drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > index e6366b408420..539212039876 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > @@ -997,7 +997,7 @@ static int start_nocpsch(struct
> device_queue_manager *dqm)
> >   {
> > pr_info("SW scheduler is used");
> > init_interrupts(dqm);
> > -
> > +
> Please ignore this extra space. I will push without it once I get a r-b.
> > if (dqm->dev->device_info->asic_family == CHIP_HAWAII)
> > return pm_init(&dqm->packets, dqm);
> > dqm->sched_running = true;
> > @@ -1674,7 +1674,6 @@ static int get_wave_state(struct
> device_queue_manager *dqm,
> >   u32 *save_area_used_size)
> >   {
> > struct mqd_manager *mqd_mgr;
> > -   int r;
> >
> > dqm_lock(dqm);
> >
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 1/1] drm/amdkfd: remove unused variable

2021-06-15 Thread Das, Nirmoy



On 6/15/2021 12:33 PM, Nirmoy Das wrote:

Remove it.

CC: jonathan@amd.com
CC: felix.kuehl...@amd.com
Fixes: d7b132507384c("drm/amdkfd: fix circular locking on get_wave_state")
Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index e6366b408420..539212039876 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -997,7 +997,7 @@ static int start_nocpsch(struct device_queue_manager *dqm)
  {
pr_info("SW scheduler is used");
init_interrupts(dqm);
-   
+

Please ignore this extra space. I will push without it once I get a r-b.

if (dqm->dev->device_info->asic_family == CHIP_HAWAII)
return pm_init(&dqm->packets, dqm);
dqm->sched_running = true;
@@ -1674,7 +1674,6 @@ static int get_wave_state(struct device_queue_manager 
*dqm,
  u32 *save_area_used_size)
  {
struct mqd_manager *mqd_mgr;
-   int r;
  
  	dqm_lock(dqm);
  

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


[PATCH 1/1] drm/amdkfd: remove unused variable

2021-06-15 Thread Nirmoy Das
Remove it.

CC: jonathan@amd.com
CC: felix.kuehl...@amd.com
Fixes: d7b132507384c("drm/amdkfd: fix circular locking on get_wave_state")
Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index e6366b408420..539212039876 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -997,7 +997,7 @@ static int start_nocpsch(struct device_queue_manager *dqm)
 {
pr_info("SW scheduler is used");
init_interrupts(dqm);
-   
+
if (dqm->dev->device_info->asic_family == CHIP_HAWAII)
return pm_init(&dqm->packets, dqm);
dqm->sched_running = true;
@@ -1674,7 +1674,6 @@ static int get_wave_state(struct device_queue_manager 
*dqm,
  u32 *save_area_used_size)
 {
struct mqd_manager *mqd_mgr;
-   int r;
 
dqm_lock(dqm);
 
-- 
2.31.1

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


[PATCH v3 14/14] drm/i915/display: Add handling for new "preferred color format" property

2021-06-15 Thread Werner Sembach
This commit implements the "preferred color format" drm property for the Intel 
GPU
driver.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/i915/display/intel_dp.c | 7 ++-
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 5 +
 drivers/gpu/drm/i915/display/intel_hdmi.c   | 5 +
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index d648582d0786..5c83ae624ad1 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -616,9 +616,12 @@ intel_dp_output_format(struct drm_connector *connector,
 {
struct intel_dp *intel_dp = 
intel_attached_dp(to_intel_connector(connector));
const struct drm_display_info *info = &connector->display_info;
+   const struct drm_connector_state *connector_state = connector->state;
 
if (!connector->ycbcr_420_allowed ||
-   !drm_mode_is_420_only(info, mode))
+   !(drm_mode_is_420_only(info, mode) ||
+   (drm_mode_is_420_also(info, mode) && connector_state &&
+   connector_state->preferred_color_format == 
DRM_COLOR_FORMAT_YCRCB420)))
return INTEL_OUTPUT_FORMAT_RGB;
 
if (intel_dp->dfp.rgb_to_ycbcr &&
@@ -4691,12 +4694,14 @@ intel_dp_add_properties(struct intel_dp *intel_dp, 
struct drm_connector *connect
if (HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 6, 10);
drm_connector_attach_active_bpc_property(connector, 6, 10);
+   drm_connector_attach_preferred_color_format_property(connector);
drm_connector_attach_active_color_format_property(connector);
drm_connector_attach_active_color_range_property(connector);
}
else if (DISPLAY_VER(dev_priv) >= 5) {
drm_connector_attach_max_bpc_property(connector, 6, 12);
drm_connector_attach_active_bpc_property(connector, 6, 12);
+   drm_connector_attach_preferred_color_format_property(connector);
drm_connector_attach_active_color_format_property(connector);
drm_connector_attach_active_color_range_property(connector);
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index cb876175258f..67f0fb649876 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -856,6 +856,11 @@ static struct drm_connector 
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
if (connector->active_bpc_property)
drm_connector_attach_active_bpc_property(connector, 6, 12);
 
+   connector->preferred_color_format_property =
+   
intel_dp->attached_connector->base.preferred_color_format_property;
+   if (connector->preferred_color_format_property)
+   drm_connector_attach_preferred_color_format_property(connector);
+
connector->active_color_format_property =
intel_dp->attached_connector->base.active_color_format_property;
if (connector->active_color_format_property)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index dacac23a6c30..0d917d61482d 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2153,6 +2153,10 @@ static int intel_hdmi_compute_output_format(struct 
intel_encoder *encoder,
crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
}
 
+   if (connector->ycbcr_420_allowed && conn_state->preferred_color_format 
== DRM_COLOR_FORMAT_YCRCB420 &&
+   drm_mode_is_420_also(&connector->display_info, adjusted_mode))
+   crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+
ret = intel_hdmi_compute_clock(encoder, crtc_state);
if (ret) {
if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 &&
@@ -2516,6 +2520,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, 
struct drm_connector *c
if (!HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 8, 12);
drm_connector_attach_active_bpc_property(connector, 8, 12);
+   drm_connector_attach_preferred_color_format_property(connector);
drm_connector_attach_active_color_format_property(connector);
drm_connector_attach_active_color_range_property(connector);
}
-- 
2.25.1

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


[PATCH v3 12/14] drm/uAPI: Add "preferred color format" drm property as setting for userspace

2021-06-15 Thread Werner Sembach
Add a new general drm property "preferred color format" which can be used by
userspace to tell the graphic drivers to which color format to use.

Possible options are:
- auto (default/current behaviour)
- rgb
- ycbcr444
- ycbcr422 (not supported by both amdgpu and i915)
- ycbcr420

In theory the auto option should choose the best available option for the
current setup, but because of bad internal conversion some monitors look better
with rgb and some with ycbcr444.

Also, because of bad shielded connectors and/or cables, it might be preferable
to use the less bandwidth heavy ycbcr422 and ycbcr420 formats for a signal that
is less deceptible to interference.

In the future, automatic color calibration for screens might also depend on this
option being available.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/drm_atomic_helper.c |  4 +++
 drivers/gpu/drm/drm_atomic_uapi.c   |  4 +++
 drivers/gpu/drm/drm_connector.c | 46 -
 include/drm/drm_connector.h | 17 +++
 4 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index bc3487964fb5..90d62f305257 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -687,6 +687,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
if (old_connector_state->max_requested_bpc !=
new_connector_state->max_requested_bpc)
new_crtc_state->connectors_changed = true;
+
+   if (old_connector_state->preferred_color_format !=
+   new_connector_state->preferred_color_format)
+   new_crtc_state->connectors_changed = true;
}
 
if (funcs->atomic_check)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 438e9585b225..c536f5e22016 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -796,6 +796,8 @@ static int drm_atomic_connector_set_property(struct 
drm_connector *connector,
   fence_ptr);
} else if (property == connector->max_bpc_property) {
state->max_requested_bpc = val;
+   } else if (property == connector->preferred_color_format_property) {
+   state->preferred_color_format = val;
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -873,6 +875,8 @@ drm_atomic_connector_get_property(struct drm_connector 
*connector,
*val = 0;
} else if (property == connector->max_bpc_property) {
*val = state->max_requested_bpc;
+   } else if (property == connector->preferred_color_format_property) {
+   *val = state->preferred_color_format;
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b4aff7d6910f..1dc537514c71 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -889,6 +889,14 @@ static const struct drm_prop_enum_list 
drm_dp_subconnector_enum_list[] = {
{ DRM_MODE_SUBCONNECTOR_Native,  "Native"}, /* DP */
 };
 
+static const struct drm_prop_enum_list drm_preferred_color_format_enum_list[] 
= {
+   { 0, "auto" },
+   { DRM_COLOR_FORMAT_RGB444, "rgb" },
+   { DRM_COLOR_FORMAT_YCRCB444, "ycbcr444" },
+   { DRM_COLOR_FORMAT_YCRCB422, "ycbcr422" },
+   { DRM_COLOR_FORMAT_YCRCB420, "ycbcr420" },
+};
+
 static const struct drm_prop_enum_list drm_active_color_format_enum_list[] = {
{ 0, "unknown" },
{ DRM_COLOR_FORMAT_RGB444, "rgb" },
@@ -1219,11 +1227,19 @@ static const struct drm_prop_enum_list dp_colorspaces[] 
= {
  * Drivers shall use drm_connector_attach_active_bpc_property() to install
  * this property.
  *
+ * preferred color format:
+ * This property is used by userspace to change the used color format. When
+ * used the driver will use the selected format if valid for the hardware,
+ * sink, and current resolution and refresh rate combination. Drivers to
+ * use the function drm_connector_attach_preferred_color_format_property()
+ * to create and attach the property to the connector during
+ * initialization.
+ *
  * active color format:
  * This read-only property tells userspace the color format actually used
  * by the hardware display engine on "the cable" on a connector. The chosen
  * value depends on hardware capabilities, both display engine and
- * connected monitor. Drivers shall use
+ * connecte

[PATCH v3 09/14] drm/uAPI: Add "active color range" drm property as feedback for userspace

2021-06-15 Thread Werner Sembach
Add a new general drm property "active color range" which can be used by graphic
drivers to report the used color range back to userspace.

There was no way to check which color range got actually used on a given
monitor. To surely predict this, one must know the exact capabilities of the
monitor and what the default behaviour of the used driver is. This property
helps eliminating the guessing at this point.

In the future, automatic color calibration for screens might also depend on this
information being available.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/drm_connector.c | 54 +
 include/drm/drm_connector.h | 26 
 2 files changed, 80 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2411b964c342..b4aff7d6910f 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -897,6 +897,12 @@ static const struct drm_prop_enum_list 
drm_active_color_format_enum_list[] = {
{ DRM_COLOR_FORMAT_YCRCB420, "ycbcr420" },
 };
 
+static const struct drm_prop_enum_list drm_active_color_range_enum_list[] = {
+   { DRM_MODE_COLOR_RANGE_UNSET, "Unknown" },
+   { DRM_MODE_COLOR_RANGE_FULL, "Full" },
+   { DRM_MODE_COLOR_RANGE_LIMITED_16_235, "Limited 16:235" },
+};
+
 DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name,
 drm_dp_subconnector_enum_list)
 
@@ -1221,6 +1227,14 @@ static const struct drm_prop_enum_list dp_colorspaces[] 
= {
  * drm_connector_attach_active_color_format_property() to install this
  * property.
  *
+ * active color range:
+ * This read-only property tells userspace the color range actually used by
+ * the hardware display engine on "the cable" on a connector. The chosen
+ * value depends on hardware capabilities of the monitor and the used color
+ * format. Drivers shall use
+ * drm_connector_attach_active_color_range_property() to install this
+ * property.
+ *
  * Connectors also have one standardized atomic property:
  *
  * CRTC_ID:
@@ -2258,6 +2272,46 @@ void 
drm_connector_set_active_color_format_property(struct drm_connector *connec
 }
 EXPORT_SYMBOL(drm_connector_set_active_color_format_property);
 
+/**
+ * drm_connector_attach_active_color_range_property - attach "active color 
range" property
+ * @connector: connector to attach active color range property on.
+ *
+ * This is used to check the applied color range on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_active_color_range_property(struct drm_connector 
*connector)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->active_color_range_property) {
+   prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE, 
"active color range", drm_active_color_range_enum_list, 
ARRAY_SIZE(drm_active_color_range_enum_list));
+   if (!prop)
+   return -ENOMEM;
+
+   connector->active_color_range_property = prop;
+   drm_object_attach_property(&connector->base, prop, 
DRM_MODE_COLOR_RANGE_UNSET);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_active_color_range_property);
+
+/**
+ * drm_connector_set_active_color_range_property - sets the active color range 
property for a connector
+ * @connector: drm connector
+ * @active_color_range: color range for the connector currently active on "the 
cable"
+ *
+ * Should be used by atomic drivers to update the active color range over a 
connector.
+ */
+void drm_connector_set_active_color_range_property(struct drm_connector 
*connector, enum drm_mode_color_range active_color_range)
+{
+   drm_object_property_set_value(&connector->base, 
connector->active_color_range_property, active_color_range);
+}
+EXPORT_SYMBOL(drm_connector_set_active_color_range_property);
+
 /**
  * drm_connector_attach_hdr_output_metadata_property - attach 
"HDR_OUTPUT_METADA" property
  * @connector: connector to attach the property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 99e4989dd6b3..5cb122812727 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -648,6 +648,24 @@ struct drm_tv_connector_state {
unsigned int hue;
 };
 
+/**
+ * enum drm_mode_color_range - color_range info for &drm_connector
+ *
+ * This enum is used to represent full or limited color range on the display
+ * connector signal.
+ *
+ * @DRM_MODE_COLOR_RANGE_UNSET:Color range is 
unspecified/default.
+ * @DRM_MODE_COLOR_RANGE_FULL: Color range is full range, 0-255 for
+ * 8-Bit color depth.
+ * DRM_MODE_COLOR_RANGE_LIMITED_16_235:Color range is limited range, 
16-235 for
+ * 8-Bit color depth.
+ */
+enum drm_mode_color_range {
+   DRM_MODE_COLOR_RANGE_UNSET,
+   DRM_MO

[PATCH v3 13/14] drm/amd/display: Add handling for new "preferred color format" property

2021-06-15 Thread Werner Sembach
This commit implements the "preferred color format" drm property for the AMD GPU
driver.

Signed-off-by: Werner Sembach 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 ++-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  4 
 2 files changed, 22 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 705cd2e015af..ead246b2ec57 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5351,15 +5351,26 @@ static void 
fill_stream_properties_from_drm_display_mode(
timing_out->h_border_right = 0;
timing_out->v_border_top = 0;
timing_out->v_border_bottom = 0;
-   /* TODO: un-hardcode */
-   if (drm_mode_is_420_only(info, mode_in)
-   || (drm_mode_is_420_also(info, mode_in) && 
aconnector->force_yuv420_output))
+
+   if (connector_state && (connector_state->preferred_color_format == 
DRM_COLOR_FORMAT_YCRCB420
+   || aconnector->force_yuv420_output) && 
drm_mode_is_420(info, mode_in))
timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
-   else if ((connector->display_info.color_formats & 
DRM_COLOR_FORMAT_YCRCB444)
-   && stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
+   else if (connector_state && connector_state->preferred_color_format == 
DRM_COLOR_FORMAT_YCRCB444
+   && connector->display_info.color_formats & 
DRM_COLOR_FORMAT_YCRCB444)
timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR444;
-   else
+   else if (connector_state && connector_state->preferred_color_format == 
DRM_COLOR_FORMAT_RGB444
+   && !drm_mode_is_420_only(info, mode_in))
timing_out->pixel_encoding = PIXEL_ENCODING_RGB;
+   else /* connector_state->preferred_color_format not possible
+   || connector_state->preferred_color_format == 0 (auto)
+   || connector_state->preferred_color_format == 
DRM_COLOR_FORMAT_YCRCB422 */
+   if (drm_mode_is_420_only(info, mode_in))
+   timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
+   else if ((connector->display_info.color_formats & 
DRM_COLOR_FORMAT_YCRCB444)
+   && stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
+   timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR444;
+   else
+   timing_out->pixel_encoding = PIXEL_ENCODING_RGB;
 
timing_out->timing_3d_format = TIMING_3D_FORMAT_NONE;
timing_out->display_color_depth = convert_color_depth_from_display_info(
@@ -7760,6 +7771,7 @@ void amdgpu_dm_connector_init_helper(struct 
amdgpu_display_manager *dm,
if (!aconnector->mst_port) {
drm_connector_attach_max_bpc_property(&aconnector->base, 8, 16);
drm_connector_attach_active_bpc_property(&aconnector->base, 8, 
16);
+   
drm_connector_attach_preferred_color_format_property(&aconnector->base);

drm_connector_attach_active_color_format_property(&aconnector->base);

drm_connector_attach_active_color_range_property(&aconnector->base);
}
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 b5d57bbbdd20..2563788ba95a 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
@@ -413,6 +413,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr 
*mgr,
if (connector->active_bpc_property)
drm_connector_attach_active_bpc_property(&aconnector->base, 8, 
16);
 
+   connector->preferred_color_format_property = 
master->base.preferred_color_format_property;
+   if (connector->preferred_color_format_property)
+   
drm_connector_attach_preferred_color_format_property(&aconnector->base);
+
connector->active_color_format_property = 
master->base.active_color_format_property;
if (connector->active_color_format_property)

drm_connector_attach_active_color_format_property(&aconnector->base);
-- 
2.25.1

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


[PATCH v3 11/14] drm/i915/display: Add handling for new "active color range" property

2021-06-15 Thread Werner Sembach
This commit implements the "active color range" drm property for the Intel GPU
driver.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/i915/display/intel_display.c | 4 
 drivers/gpu/drm/i915/display/intel_dp.c  | 2 ++
 drivers/gpu/drm/i915/display/intel_dp_mst.c  | 5 +
 drivers/gpu/drm/i915/display/intel_hdmi.c| 1 +
 4 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 2cf09599ddc3..09d7c3da105a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10933,10 +10933,14 @@ static int intel_atomic_commit(struct drm_device *dev,
drm_connector_set_active_bpc_property(connector, 
new_crtc_state->pipe_bpp / 3);

drm_connector_set_active_color_format_property(connector,

convert_intel_output_format_into_drm_color_format(new_crtc_state->output_format));
+   drm_connector_set_active_color_range_property(connector,
+   new_crtc_state->limited_color_range? 
DRM_MODE_COLOR_RANGE_LIMITED_16_235
+  : 
DRM_MODE_COLOR_RANGE_FULL);
}
else {
drm_connector_set_active_bpc_property(connector, 0);

drm_connector_set_active_color_format_property(connector, 0);
+   
drm_connector_set_active_color_range_property(connector, 
DRM_MODE_COLOR_RANGE_UNSET);
}
}
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 01fdb9141592..d648582d0786 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4692,11 +4692,13 @@ intel_dp_add_properties(struct intel_dp *intel_dp, 
struct drm_connector *connect
drm_connector_attach_max_bpc_property(connector, 6, 10);
drm_connector_attach_active_bpc_property(connector, 6, 10);
drm_connector_attach_active_color_format_property(connector);
+   drm_connector_attach_active_color_range_property(connector);
}
else if (DISPLAY_VER(dev_priv) >= 5) {
drm_connector_attach_max_bpc_property(connector, 6, 12);
drm_connector_attach_active_bpc_property(connector, 6, 12);
drm_connector_attach_active_color_format_property(connector);
+   drm_connector_attach_active_color_range_property(connector);
}
 
/* Register HDMI colorspace for case of lspcon */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 3e4237df3360..cb876175258f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -861,6 +861,11 @@ static struct drm_connector 
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
if (connector->active_color_format_property)
drm_connector_attach_active_color_format_property(connector);
 
+   connector->active_color_range_property =
+   intel_dp->attached_connector->base.active_color_range_property;
+   if (connector->active_color_range_property)
+   drm_connector_attach_active_color_range_property(connector);
+
return connector;
 
 err:
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 367aba57b55f..dacac23a6c30 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2517,6 +2517,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, 
struct drm_connector *c
drm_connector_attach_max_bpc_property(connector, 8, 12);
drm_connector_attach_active_bpc_property(connector, 8, 12);
drm_connector_attach_active_color_format_property(connector);
+   drm_connector_attach_active_color_range_property(connector);
}
 }
 
-- 
2.25.1

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


[PATCH v3 07/14] drm/amd/display: Add handling for new "active color format" property

2021-06-15 Thread Werner Sembach
This commit implements the "active color format" drm property for the AMD GPU
driver.

Signed-off-by: Werner Sembach 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 28 +--
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  4 +++
 2 files changed, 30 insertions(+), 2 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 f31bbcb11f03..b66336a1403e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6715,6 +6715,23 @@ static int convert_dc_color_depth_into_bpc (enum 
dc_color_depth display_color_de
return 0;
 }
 
+static int convert_dc_pixel_encoding_into_drm_color_format(enum 
dc_pixel_encoding display_pixel_encoding)
+{
+   switch (display_pixel_encoding) {
+   case PIXEL_ENCODING_RGB:
+   return DRM_COLOR_FORMAT_RGB444;
+   case PIXEL_ENCODING_YCBCR422:
+   return DRM_COLOR_FORMAT_YCRCB422;
+   case PIXEL_ENCODING_YCBCR444:
+   return DRM_COLOR_FORMAT_YCRCB444;
+   case PIXEL_ENCODING_YCBCR420:
+   return DRM_COLOR_FORMAT_YCRCB420;
+   default:
+   break;
+   }
+   return 0;
+}
+
 static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
  struct drm_crtc_state *crtc_state,
  struct drm_connector_state 
*conn_state)
@@ -7716,6 +7733,7 @@ void amdgpu_dm_connector_init_helper(struct 
amdgpu_display_manager *dm,
if (!aconnector->mst_port) {
drm_connector_attach_max_bpc_property(&aconnector->base, 8, 16);
drm_connector_attach_active_bpc_property(&aconnector->base, 8, 
16);
+   
drm_connector_attach_active_color_format_property(&aconnector->base);
}
 
/* This defaults to the max in the range, but we want 8bpc for non-edp. 
*/
@@ -9091,13 +9109,19 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
if (crtc) {
new_crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);
dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-   if (dm_new_crtc_state->stream)
+   if (dm_new_crtc_state->stream) {
drm_connector_set_active_bpc_property(connector,
convert_dc_color_depth_into_bpc(

dm_new_crtc_state->stream->timing.display_color_depth));
+   
drm_connector_set_active_color_format_property(connector,
+   
convert_dc_pixel_encoding_into_drm_color_format(
+   
dm_new_crtc_state->stream->timing.pixel_encoding));
+   }
}
-   else
+   else {
drm_connector_set_active_bpc_property(connector, 0);
+   
drm_connector_set_active_color_format_property(connector, 0);
+   }
}
 
/* Count number of newly disabled CRTCs for dropping PM refs later. */
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 0cf38743ec47..13151d13aa73 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
@@ -413,6 +413,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr 
*mgr,
if (connector->active_bpc_property)
drm_connector_attach_active_bpc_property(&aconnector->base, 8, 
16);
 
+   connector->active_color_format_property = 
master->base.active_color_format_property;
+   if (connector->active_color_format_property)
+   
drm_connector_attach_active_color_format_property(&aconnector->base);
+
connector->vrr_capable_property = master->base.vrr_capable_property;
if (connector->vrr_capable_property)
drm_connector_attach_vrr_capable_property(connector);
-- 
2.25.1

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


[PATCH v3 08/14] drm/i915/display: Add handling for new "active color format" property

2021-06-15 Thread Werner Sembach
This commit implements the "active color format" drm property for the Intel GPU
driver.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/i915/display/intel_display.c | 21 +++-
 drivers/gpu/drm/i915/display/intel_dp.c  |  2 ++
 drivers/gpu/drm/i915/display/intel_dp_mst.c  |  5 +
 drivers/gpu/drm/i915/display/intel_hdmi.c|  1 +
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index ee3669bd4662..2cf09599ddc3 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10609,6 +10609,21 @@ static void 
intel_atomic_prepare_plane_clear_colors(struct intel_atomic_state *s
}
 }
 
+static int convert_intel_output_format_into_drm_color_format(enum 
intel_output_format output_format)
+{
+   switch (output_format) {
+   case INTEL_OUTPUT_FORMAT_RGB:
+   return DRM_COLOR_FORMAT_RGB444;
+   case INTEL_OUTPUT_FORMAT_YCBCR420:
+   return DRM_COLOR_FORMAT_YCRCB420;
+   case INTEL_OUTPUT_FORMAT_YCBCR444:
+   return DRM_COLOR_FORMAT_YCRCB444;
+   default:
+   break;
+   }
+   return 0;
+}
+
 static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 {
struct drm_device *dev = state->base.dev;
@@ -10916,9 +10931,13 @@ static int intel_atomic_commit(struct drm_device *dev,
if (crtc) {
struct intel_crtc_state *new_crtc_state = 
intel_atomic_get_new_crtc_state(state, crtc);
drm_connector_set_active_bpc_property(connector, 
new_crtc_state->pipe_bpp / 3);
+   
drm_connector_set_active_color_format_property(connector,
+   
convert_intel_output_format_into_drm_color_format(new_crtc_state->output_format));
}
-   else
+   else {
drm_connector_set_active_bpc_property(connector, 0);
+   
drm_connector_set_active_color_format_property(connector, 0);
+   }
}
 
drm_atomic_state_get(&state->base);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 404a27e56ceb..01fdb9141592 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4691,10 +4691,12 @@ intel_dp_add_properties(struct intel_dp *intel_dp, 
struct drm_connector *connect
if (HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 6, 10);
drm_connector_attach_active_bpc_property(connector, 6, 10);
+   drm_connector_attach_active_color_format_property(connector);
}
else if (DISPLAY_VER(dev_priv) >= 5) {
drm_connector_attach_max_bpc_property(connector, 6, 12);
drm_connector_attach_active_bpc_property(connector, 6, 12);
+   drm_connector_attach_active_color_format_property(connector);
}
 
/* Register HDMI colorspace for case of lspcon */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 16bfc59570a5..3e4237df3360 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -856,6 +856,11 @@ static struct drm_connector 
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
if (connector->active_bpc_property)
drm_connector_attach_active_bpc_property(connector, 6, 12);
 
+   connector->active_color_format_property =
+   intel_dp->attached_connector->base.active_color_format_property;
+   if (connector->active_color_format_property)
+   drm_connector_attach_active_color_format_property(connector);
+
return connector;
 
 err:
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 9160e21ac9d6..367aba57b55f 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2516,6 +2516,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, 
struct drm_connector *c
if (!HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 8, 12);
drm_connector_attach_active_bpc_property(connector, 8, 12);
+   drm_connector_attach_active_color_format_property(connector);
}
 }
 
-- 
2.25.1

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


[PATCH v3 10/14] drm/amd/display: Add handling for new "active color range" property

2021-06-15 Thread Werner Sembach
This commit implements the "active color range" drm property for the AMD GPU
driver.

Signed-off-by: Werner Sembach 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 +++
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  4 +++
 2 files changed, 36 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 b66336a1403e..705cd2e015af 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6732,6 +6732,33 @@ static int 
convert_dc_pixel_encoding_into_drm_color_format(enum dc_pixel_encodin
return 0;
 }
 
+static int convert_dc_color_space_into_drm_mode_color_range(enum 
dc_color_space color_space)
+{
+   if (color_space == COLOR_SPACE_SRGB ||
+   color_space == COLOR_SPACE_XR_RGB ||
+   color_space == COLOR_SPACE_MSREF_SCRGB ||
+   color_space == COLOR_SPACE_2020_RGB_FULLRANGE ||
+   color_space == COLOR_SPACE_ADOBERGB ||
+   color_space == COLOR_SPACE_DCIP3 ||
+   color_space == COLOR_SPACE_DOLBYVISION ||
+   color_space == COLOR_SPACE_YCBCR601 ||
+   color_space == COLOR_SPACE_XV_YCC_601 ||
+   color_space == COLOR_SPACE_YCBCR709 ||
+   color_space == COLOR_SPACE_XV_YCC_709 ||
+   color_space == COLOR_SPACE_2020_YCBCR ||
+   color_space == COLOR_SPACE_YCBCR709_BLACK ||
+   color_space == COLOR_SPACE_DISPLAYNATIVE ||
+   color_space == COLOR_SPACE_APPCTRL ||
+   color_space == COLOR_SPACE_CUSTOMPOINTS)
+   return DRM_MODE_COLOR_RANGE_FULL;
+   if (color_space == COLOR_SPACE_SRGB_LIMITED ||
+   color_space == COLOR_SPACE_2020_RGB_LIMITEDRANGE ||
+   color_space == COLOR_SPACE_YCBCR601_LIMITED ||
+   color_space == COLOR_SPACE_YCBCR709_LIMITED)
+   return DRM_MODE_COLOR_RANGE_LIMITED_16_235;
+   return DRM_MODE_COLOR_RANGE_UNSET;
+}
+
 static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
  struct drm_crtc_state *crtc_state,
  struct drm_connector_state 
*conn_state)
@@ -7734,6 +7761,7 @@ void amdgpu_dm_connector_init_helper(struct 
amdgpu_display_manager *dm,
drm_connector_attach_max_bpc_property(&aconnector->base, 8, 16);
drm_connector_attach_active_bpc_property(&aconnector->base, 8, 
16);

drm_connector_attach_active_color_format_property(&aconnector->base);
+   
drm_connector_attach_active_color_range_property(&aconnector->base);
}
 
/* This defaults to the max in the range, but we want 8bpc for non-edp. 
*/
@@ -9116,11 +9144,15 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)

drm_connector_set_active_color_format_property(connector,

convert_dc_pixel_encoding_into_drm_color_format(

dm_new_crtc_state->stream->timing.pixel_encoding));
+   
drm_connector_set_active_color_range_property(connector,
+   
convert_dc_color_space_into_drm_mode_color_range(
+   
dm_new_crtc_state->stream->output_color_space));
}
}
else {
drm_connector_set_active_bpc_property(connector, 0);

drm_connector_set_active_color_format_property(connector, 0);
+   
drm_connector_set_active_color_range_property(connector, 
DRM_MODE_COLOR_RANGE_UNSET);
}
}
 
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 13151d13aa73..b5d57bbbdd20 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
@@ -417,6 +417,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr 
*mgr,
if (connector->active_color_format_property)

drm_connector_attach_active_color_format_property(&aconnector->base);
 
+   connector->active_color_range_property = 
master->base.active_color_range_property;
+   if (connector->active_color_range_property)
+   
drm_connector_attach_active_color_range_property(&aconnector->base);
+
connector->vrr_capable_property = master->base.vrr_capable_property;
if (connector->vrr_capable_property)
drm_connector_attach_vrr_capable_property(connector);
-- 
2.25.1

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


[PATCH v3 05/14] drm/i915/display: Add handling for new "active bpc" property

2021-06-15 Thread Werner Sembach
This commit implements the "active bpc" drm property for the Intel GPU driver.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/i915/display/intel_display.c | 14 ++
 drivers/gpu/drm/i915/display/intel_dp.c  |  8 ++--
 drivers/gpu/drm/i915/display/intel_dp_mst.c  |  5 +
 drivers/gpu/drm/i915/display/intel_hdmi.c|  4 +++-
 4 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 6be1b31af07b..ee3669bd4662 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10839,6 +10839,9 @@ static int intel_atomic_commit(struct drm_device *dev,
 {
struct intel_atomic_state *state = to_intel_atomic_state(_state);
struct drm_i915_private *dev_priv = to_i915(dev);
+   struct drm_connector *connector;
+   struct drm_connector_state *new_conn_state;
+   int i;
int ret = 0;
 
state->wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
@@ -10907,6 +10910,17 @@ static int intel_atomic_commit(struct drm_device *dev,
intel_shared_dpll_swap_state(state);
intel_atomic_track_fbs(state);
 
+   /* Extract information from crtc to communicate it to userspace as 
connector properties */
+   for_each_new_connector_in_state(&state->base, connector, 
new_conn_state, i) {
+   struct intel_crtc *crtc = to_intel_crtc(new_conn_state->crtc);
+   if (crtc) {
+   struct intel_crtc_state *new_crtc_state = 
intel_atomic_get_new_crtc_state(state, crtc);
+   drm_connector_set_active_bpc_property(connector, 
new_crtc_state->pipe_bpp / 3);
+   }
+   else
+   drm_connector_set_active_bpc_property(connector, 0);
+   }
+
drm_atomic_state_get(&state->base);
INIT_WORK(&state->base.commit_work, intel_atomic_commit_work);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 5c983044..404a27e56ceb 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4688,10 +4688,14 @@ intel_dp_add_properties(struct intel_dp *intel_dp, 
struct drm_connector *connect
intel_attach_force_audio_property(connector);
 
intel_attach_broadcast_rgb_property(connector);
-   if (HAS_GMCH(dev_priv))
+   if (HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 6, 10);
-   else if (DISPLAY_VER(dev_priv) >= 5)
+   drm_connector_attach_active_bpc_property(connector, 6, 10);
+   }
+   else if (DISPLAY_VER(dev_priv) >= 5) {
drm_connector_attach_max_bpc_property(connector, 6, 12);
+   drm_connector_attach_active_bpc_property(connector, 6, 12);
+   }
 
/* Register HDMI colorspace for case of lspcon */
if (intel_bios_is_lspcon_present(dev_priv, port)) {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index b170e272bdee..16bfc59570a5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -851,6 +851,11 @@ static struct drm_connector 
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
if (connector->max_bpc_property)
drm_connector_attach_max_bpc_property(connector, 6, 12);
 
+   connector->active_bpc_property =
+   intel_dp->attached_connector->base.active_bpc_property;
+   if (connector->active_bpc_property)
+   drm_connector_attach_active_bpc_property(connector, 6, 12);
+
return connector;
 
 err:
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 7e51c98c475e..9160e21ac9d6 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2513,8 +2513,10 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, 
struct drm_connector *c
if (DISPLAY_VER(dev_priv) >= 10)
drm_connector_attach_hdr_output_metadata_property(connector);
 
-   if (!HAS_GMCH(dev_priv))
+   if (!HAS_GMCH(dev_priv)) {
drm_connector_attach_max_bpc_property(connector, 8, 12);
+   drm_connector_attach_active_bpc_property(connector, 8, 12);
+   }
 }
 
 /*
-- 
2.25.1

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


[PATCH v3 02/14] drm/amd/display: Add missing cases convert_dc_color_depth_into_bpc

2021-06-15 Thread Werner Sembach
convert_dc_color_depth_into_bpc() that converts the enum dc_color_depth to an
integer had the casses for COLOR_DEPTH_999 and COLOR_DEPTH_11 missing.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 
 1 file changed, 4 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 44757720b15f..cd1df5cf4815 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6705,6 +6705,10 @@ static int convert_dc_color_depth_into_bpc (enum 
dc_color_depth display_color_de
return 14;
case COLOR_DEPTH_161616:
return 16;
+   case COLOR_DEPTH_999:
+   return 9;
+   case COLOR_DEPTH_11:
+   return 11;
default:
break;
}
-- 
2.25.1

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


[PATCH v3 03/14] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property

2021-06-15 Thread Werner Sembach
Add a new general drm property "active bpc" which can be used by graphic drivers
to report the applied bit depth per pixel back to userspace.

While "max bpc" can be used to change the color depth, there was no way to check
which one actually got used. While in theory the driver chooses the best/highest
color depth within the max bpc setting a user might not be fully aware what his
hardware is or isn't capable off. This is meant as a quick way to double check
the setup.

In the future, automatic color calibration for screens might also depend on this
information being available.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/drm_connector.c | 50 +
 include/drm/drm_connector.h |  8 ++
 2 files changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index da39e7ff6965..02c310c1ac2d 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1197,6 +1197,14 @@ static const struct drm_prop_enum_list dp_colorspaces[] 
= {
  * drm_connector_attach_max_bpc_property() to create and attach the
  * property to the connector during initialization.
  *
+ * active bpc:
+ * This read-only range property tells userspace the pixel color bit depth
+ * actually used by the hardware display engine on "the cable" on a
+ * connector. The chosen value depends on hardware capabilities, both
+ * display engine and connected monitor, and the "max bpc" property.
+ * Drivers shall use drm_connector_attach_active_bpc_property() to install
+ * this property.
+ *
  * Connectors also have one standardized atomic property:
  *
  * CRTC_ID:
@@ -2152,6 +2160,48 @@ int drm_connector_attach_max_bpc_property(struct 
drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
 
+/**
+ * drm_connector_attach_active_bpc_property - attach "active bpc" property
+ * @connector: connector to attach active bpc property on.
+ * @min: The minimum bit depth supported by the connector.
+ * @max: The maximum bit depth supported by the connector.
+ *
+ * This is used to check the applied bit depth on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_active_bpc_property(struct drm_connector *connector, 
int min, int max)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->active_bpc_property) {
+   prop = drm_property_create_range(dev, DRM_MODE_PROP_IMMUTABLE, 
"active bpc", min, max);
+   if (!prop)
+   return -ENOMEM;
+
+   connector->active_bpc_property = prop;
+   drm_object_attach_property(&connector->base, prop, 0);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_active_bpc_property);
+
+/**
+ * drm_connector_set_active_bpc_property - sets the active bits per color 
property for a connector
+ * @connector: drm connector
+ * @active_bpc: bits per color for the connector currently active on "the 
cable"
+ *
+ * Should be used by atomic drivers to update the active bits per color over a 
connector.
+ */
+void drm_connector_set_active_bpc_property(struct drm_connector *connector, 
int active_bpc)
+{
+   drm_object_property_set_value(&connector->base, 
connector->active_bpc_property, active_bpc);
+}
+EXPORT_SYMBOL(drm_connector_set_active_bpc_property);
+
 /**
  * drm_connector_attach_hdr_output_metadata_property - attach 
"HDR_OUTPUT_METADA" property
  * @connector: connector to attach the property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 714d1a01c065..eee86de62a5f 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1380,6 +1380,12 @@ struct drm_connector {
 */
struct drm_property *max_bpc_property;
 
+   /**
+* @active_bpc_property: Default connector property for the active bpc
+* to be driven out of the connector.
+*/
+   struct drm_property *active_bpc_property;
+
 #define DRM_CONNECTOR_POLL_HPD (1 << 0)
 #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
 #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
@@ -1702,6 +1708,8 @@ int drm_connector_set_panel_orientation_with_quirk(
int width, int height);
 int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
  int min, int max);
+int drm_connector_attach_active_bpc_property(struct drm_connector *connector, 
int min, int max);
+void drm_connector_set_active_bpc_property(struct drm_connector *connector, 
int active_bpc);
 
 /**
  * struct drm_tile_group - Tile group metadata
-- 
2.25.1

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


[PATCH v3 04/14] drm/amd/display: Add handling for new "active bpc" property

2021-06-15 Thread Werner Sembach
This commit implements the "active bpc" drm property for the AMD GPU driver.

Signed-off-by: Werner Sembach 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  4 
 2 files changed, 22 insertions(+), 1 deletion(-)

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 cd1df5cf4815..f31bbcb11f03 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7713,8 +7713,10 @@ void amdgpu_dm_connector_init_helper(struct 
amdgpu_display_manager *dm,
adev->mode_info.underscan_vborder_property,
0);
 
-   if (!aconnector->mst_port)
+   if (!aconnector->mst_port) {
drm_connector_attach_max_bpc_property(&aconnector->base, 8, 16);
+   drm_connector_attach_active_bpc_property(&aconnector->base, 8, 
16);
+   }
 
/* This defaults to the max in the range, but we want 8bpc for non-edp. 
*/
aconnector->base.state->max_bpc = (connector_type == 
DRM_MODE_CONNECTOR_eDP) ? 16 : 8;
@@ -9083,6 +9085,21 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
mutex_unlock(&dm->dc_lock);
}
 
+   /* Extract information from crtc to communicate it to userspace as 
connector properties */
+   for_each_new_connector_in_state(state, connector, new_con_state, i) {
+   struct drm_crtc *crtc = new_con_state->crtc;
+   if (crtc) {
+   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);
+   dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+   if (dm_new_crtc_state->stream)
+   drm_connector_set_active_bpc_property(connector,
+   convert_dc_color_depth_into_bpc(
+   
dm_new_crtc_state->stream->timing.display_color_depth));
+   }
+   else
+   drm_connector_set_active_bpc_property(connector, 0);
+   }
+
/* Count number of newly disabled CRTCs for dropping PM refs later. */
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
  new_crtc_state, i) {
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 5568d4e518e6..0cf38743ec47 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
@@ -409,6 +409,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr 
*mgr,
if (connector->max_bpc_property)
drm_connector_attach_max_bpc_property(connector, 8, 16);
 
+   connector->active_bpc_property = master->base.active_bpc_property;
+   if (connector->active_bpc_property)
+   drm_connector_attach_active_bpc_property(&aconnector->base, 8, 
16);
+
connector->vrr_capable_property = master->base.vrr_capable_property;
if (connector->vrr_capable_property)
drm_connector_attach_vrr_capable_property(connector);
-- 
2.25.1

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


[PATCH v3 06/14] drm/uAPI: Add "active color format" drm property as feedback for userspace

2021-06-15 Thread Werner Sembach
Add a new general drm property "active color format" which can be used by
graphic drivers to report the used color format back to userspace.

There was no way to check which color format got actually used on a given
monitor. To surely predict this, one must know the exact capabilities of the
monitor, the GPU, and the connection used and what the default behaviour of the
used driver is (e.g. amdgpu prefers YCbCr 4:4:4 while i915 prefers RGB). This
property helps eliminating the guessing on this point.

In the future, automatic color calibration for screens might also depend on this
information being available.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/drm_connector.c | 56 +
 include/drm/drm_connector.h |  8 +
 2 files changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 02c310c1ac2d..2411b964c342 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -889,6 +889,14 @@ static const struct drm_prop_enum_list 
drm_dp_subconnector_enum_list[] = {
{ DRM_MODE_SUBCONNECTOR_Native,  "Native"}, /* DP */
 };
 
+static const struct drm_prop_enum_list drm_active_color_format_enum_list[] = {
+   { 0, "unknown" },
+   { DRM_COLOR_FORMAT_RGB444, "rgb" },
+   { DRM_COLOR_FORMAT_YCRCB444, "ycbcr444" },
+   { DRM_COLOR_FORMAT_YCRCB422, "ycbcr422" },
+   { DRM_COLOR_FORMAT_YCRCB420, "ycbcr420" },
+};
+
 DRM_ENUM_NAME_FN(drm_get_dp_subconnector_name,
 drm_dp_subconnector_enum_list)
 
@@ -1205,6 +1213,14 @@ static const struct drm_prop_enum_list dp_colorspaces[] 
= {
  * Drivers shall use drm_connector_attach_active_bpc_property() to install
  * this property.
  *
+ * active color format:
+ * This read-only property tells userspace the color format actually used
+ * by the hardware display engine on "the cable" on a connector. The chosen
+ * value depends on hardware capabilities, both display engine and
+ * connected monitor. Drivers shall use
+ * drm_connector_attach_active_color_format_property() to install this
+ * property.
+ *
  * Connectors also have one standardized atomic property:
  *
  * CRTC_ID:
@@ -2202,6 +2218,46 @@ void drm_connector_set_active_bpc_property(struct 
drm_connector *connector, int
 }
 EXPORT_SYMBOL(drm_connector_set_active_bpc_property);
 
+/**
+ * drm_connector_attach_active_color_format_property - attach "active color 
format" property
+ * @connector: connector to attach active color format property on.
+ *
+ * This is used to check the applied color format on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_active_color_format_property(struct drm_connector 
*connector)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->active_color_format_property) {
+   prop = drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE, 
"active color format", drm_active_color_format_enum_list, 
ARRAY_SIZE(drm_active_color_format_enum_list));
+   if (!prop)
+   return -ENOMEM;
+
+   connector->active_color_format_property = prop;
+   drm_object_attach_property(&connector->base, prop, 0);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_active_color_format_property);
+
+/**
+ * drm_connector_set_active_color_format_property - sets the active color 
format property for a connector
+ * @connector: drm connector
+ * @active_color_format: color format for the connector currently active on 
"the cable"
+ *
+ * Should be used by atomic drivers to update the active color format over a 
connector.
+ */
+void drm_connector_set_active_color_format_property(struct drm_connector 
*connector, u32 active_color_format)
+{
+   drm_object_property_set_value(&connector->base, 
connector->active_color_format_property, active_color_format);
+}
+EXPORT_SYMBOL(drm_connector_set_active_color_format_property);
+
 /**
  * drm_connector_attach_hdr_output_metadata_property - attach 
"HDR_OUTPUT_METADA" property
  * @connector: connector to attach the property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index eee86de62a5f..99e4989dd6b3 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1386,6 +1386,12 @@ struct drm_connector {
 */
struct drm_property *active_bpc_property;
 
+   /**
+* @active_color_format_property: Default connector property for the
+* active color format to be driven out of the connector.
+*/
+   struct drm_property *active_color_format_property;
+
 #define DRM_CONNECTOR_POLL_HPD (1 << 0)
 #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
 #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
@@ -1710,6 +1716,8 @@ int drm_connector_attach_max_bpc_property(struct 
drm_connector *connector,
 

[PATCH v3 01/14] drm/amd/display: Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check

2021-06-15 Thread Werner Sembach
Remove unnecessary SIGNAL_TYPE_HDMI_TYPE_A check that was performed in the
drm_mode_is_420_only() case, but not in the drm_mode_is_420_also() &&
force_yuv420_output case.

Without further knowledge if YCbCr 4:2:0 is supported outside of HDMI, there is
no reason to use RGB when the display reports drm_mode_is_420_only() even on a
non HDMI connection.

This patch also moves both checks in the same if-case. This  eliminates an extra
else-if-case.

Signed-off-by: Werner Sembach 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +
 1 file changed, 1 insertion(+), 4 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 6fda0dfb78f8..44757720b15f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5353,10 +5353,7 @@ static void fill_stream_properties_from_drm_display_mode(
timing_out->v_border_bottom = 0;
/* TODO: un-hardcode */
if (drm_mode_is_420_only(info, mode_in)
-   && stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
-   timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
-   else if (drm_mode_is_420_also(info, mode_in)
-   && aconnector->force_yuv420_output)
+   || (drm_mode_is_420_also(info, mode_in) && 
aconnector->force_yuv420_output))
timing_out->pixel_encoding = PIXEL_ENCODING_YCBCR420;
else if ((connector->display_info.color_formats & 
DRM_COLOR_FORMAT_YCRCB444)
&& stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
-- 
2.25.1

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


[PATCH v3 00/14] New uAPI drm properties for color management

2021-06-15 Thread Werner Sembach
I started work on my proposal for better color handling in Linux display
drivers: https://lkml.org/lkml/2021/5/12/764

In this 3rd revision everything except the generalised Broadcast RGB
implementation is included. I did however not yet include everything suggested
in the feedback for v1 and v2.

I rebased the patch series on drm-tip to have the latest changes in i915's
YCbCr420 handling and to make the intel-gfx ci not fail on merge anymore.

The read only properties are now correctly marked as immutable.

Some questions I already have:

I think Broadcast RGB is really no good name for the property as, at least in
theory, YCbCr can also be "Limited" or "Full". Should the new implementation
have a different name and make "Broadcast RGB" an alias for it? I propose
"preferred color range" as the new name.

I have not tested dp mst (both on AMD and Intel) as i have no adapter for it at
hand. If one can test it, please let me know if things break or not.

I found the DRM_MODE_PROP_ATOMIC flag and from the documentation it sounds like
"max bpc" (and "preferred color format" and "Broadcast RGB") should have it. Is
there a reason it doesn't?

I have not yet looked into dsc and dithering behaviour.

I have already submitted the first two patches separately to the lkml as they 
fix
potential bugs and don't introduce new uAPI.

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


[PATCH 2/2] drm/amdgpu/gfx10: enlarge CP_MEC_DOORBELL_RANGE_UPPER to cover full doorbell.

2021-06-15 Thread Yifan Zhang
If GC has entered CGPG, ringing doorbell > first page doesn't wakeup GC.
Enlarge CP_MEC_DOORBELL_RANGE_UPPER to workaround this issue.

Signed-off-by: Yifan Zhang 
Reviewed-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 15ae9e33b925..7bfe6f9d3a52 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -6974,8 +6974,12 @@ static int gfx_v10_0_kiq_init_register(struct 
amdgpu_ring *ring)
if (ring->use_doorbell) {
WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_LOWER,
(adev->doorbell_index.kiq * 2) << 2);
+   /* If GC has entered CGPG, ringing doorbell > first page doesn't
+* wakeup GC. Enlarge CP_MEC_DOORBELL_RANGE_UPPER to workaround
+* this issue.
+*/
WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
-   (adev->doorbell_index.userqueue_end * 2) << 2);
+   (adev->doorbell.size - 4));
}
 
WREG32_SOC15(GC, 0, mmCP_HQD_PQ_DOORBELL_CONTROL,
-- 
2.25.1

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


[PATCH 1/2] drm/amdgpu/gfx9: fix the doorbell missing when in CGPG issue.

2021-06-15 Thread Yifan Zhang
If GC has entered CGPG, ringing doorbell > first page doesn't wakeup GC.
Enlarge CP_MEC_DOORBELL_RANGE_UPPER to workaround this issue.

Signed-off-by: Yifan Zhang 
Reviewed-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 044076ec1d03..922420a2c102 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -3675,8 +3675,12 @@ static int gfx_v9_0_kiq_init_register(struct amdgpu_ring 
*ring)
if (ring->use_doorbell) {
WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_LOWER,
(adev->doorbell_index.kiq * 2) << 2);
+   /* If GC has entered CGPG, ringing doorbell > first page doesn't
+* wakeup GC. Enlarge CP_MEC_DOORBELL_RANGE_UPPER to workaround
+* this issue.
+*/
WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
-   (adev->doorbell_index.userqueue_end * 
2) << 2);
+   (adev->doorbell.size - 4));
}
 
WREG32_SOC15_RLC(GC, 0, mmCP_HQD_PQ_DOORBELL_CONTROL,
-- 
2.25.1

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


Re: [PATCH 1/2] drm/amdgpu/gfx9: fix the doorbell missing when in CGPG issue.

2021-06-15 Thread Alex Deucher
Series is:
Reviewed-by: Alex Deucher 

On Tue, Jun 15, 2021 at 6:04 AM Yifan Zhang  wrote:
>
> If GC has entered CGPG, ringing doorbell > first page doesn't wakeup GC.
> Enlarge CP_MEC_DOORBELL_RANGE_UPPER to workaround this issue.
>
> Signed-off-by: Yifan Zhang 
> Reviewed-by: Felix Kuehling 
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 044076ec1d03..922420a2c102 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -3675,8 +3675,12 @@ static int gfx_v9_0_kiq_init_register(struct 
> amdgpu_ring *ring)
> if (ring->use_doorbell) {
> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_LOWER,
> (adev->doorbell_index.kiq * 2) << 2);
> +   /* If GC has entered CGPG, ringing doorbell > first page 
> doesn't
> +* wakeup GC. Enlarge CP_MEC_DOORBELL_RANGE_UPPER to 
> workaround
> +* this issue.
> +*/
> WREG32_SOC15(GC, 0, mmCP_MEC_DOORBELL_RANGE_UPPER,
> -   (adev->doorbell_index.userqueue_end * 
> 2) << 2);
> +   (adev->doorbell.size - 4));
> }
>
> WREG32_SOC15_RLC(GC, 0, mmCP_HQD_PQ_DOORBELL_CONTROL,
> --
> 2.25.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH v2 1/1] drm/amdgpu: remove amdgpu_vm_pt

2021-06-15 Thread Christian König

Am 14.06.21 um 14:31 schrieb Nirmoy Das:

Page table entries are now in embedded in VM BO, so
we do not need struct amdgpu_vm_pt. This patch replaces
struct amdgpu_vm_pt with struct amdgpu_vm_bo_base.

v2: change "!(cursor->level < AMDGPU_VM_PTB)" --> "(cursor->level == 
AMDGPU_VM_PTB)"

Signed-off-by: Nirmoy Das 


Reviewed-by: Christian König 


---
  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  26 +--
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c|   2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c   |   4 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c   |   2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c|   4 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c   |  12 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |   2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h|   2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 164 --
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h|   9 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c   |   2 +-
  drivers/gpu/drm/amd/amdkfd/kfd_svm.c  |   2 +-
  12 files changed, 105 insertions(+), 126 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index fb6bcc386de1..f96598279593 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -356,7 +356,7 @@ static int amdgpu_amdkfd_validate_vm_bo(void *_unused, 
struct amdgpu_bo *bo)
   */
  static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm)
  {
-   struct amdgpu_bo *pd = vm->root.base.bo;
+   struct amdgpu_bo *pd = vm->root.bo;
struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev);
int ret;

@@ -372,7 +372,7 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm)
return ret;
}

-   vm->pd_phys_addr = amdgpu_gmc_pd_addr(vm->root.base.bo);
+   vm->pd_phys_addr = amdgpu_gmc_pd_addr(vm->root.bo);

if (vm->use_cpu_for_update) {
ret = amdgpu_bo_kmap(pd, NULL);
@@ -387,7 +387,7 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm)

  static int vm_update_pds(struct amdgpu_vm *vm, struct amdgpu_sync *sync)
  {
-   struct amdgpu_bo *pd = vm->root.base.bo;
+   struct amdgpu_bo *pd = vm->root.bo;
struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev);
int ret;

@@ -1153,7 +1153,7 @@ static int process_sync_pds_resv(struct 
amdkfd_process_info *process_info,

list_for_each_entry(peer_vm, &process_info->vm_list_head,
vm_list_node) {
-   struct amdgpu_bo *pd = peer_vm->root.base.bo;
+   struct amdgpu_bo *pd = peer_vm->root.bo;

ret = amdgpu_sync_resv(NULL, sync, pd->tbo.base.resv,
   AMDGPU_SYNC_NE_OWNER,
@@ -1220,7 +1220,7 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void 
**process_info,
vm->process_info = *process_info;

/* Validate page directory and attach eviction fence */
-   ret = amdgpu_bo_reserve(vm->root.base.bo, true);
+   ret = amdgpu_bo_reserve(vm->root.bo, true);
if (ret)
goto reserve_pd_fail;
ret = vm_validate_pt_pd_bos(vm);
@@ -1228,16 +1228,16 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void 
**process_info,
pr_err("validate_pt_pd_bos() failed\n");
goto validate_pd_fail;
}
-   ret = amdgpu_bo_sync_wait(vm->root.base.bo,
+   ret = amdgpu_bo_sync_wait(vm->root.bo,
  AMDGPU_FENCE_OWNER_KFD, false);
if (ret)
goto wait_pd_fail;
-   ret = dma_resv_reserve_shared(vm->root.base.bo->tbo.base.resv, 1);
+   ret = dma_resv_reserve_shared(vm->root.bo->tbo.base.resv, 1);
if (ret)
goto reserve_shared_fail;
-   amdgpu_bo_fence(vm->root.base.bo,
+   amdgpu_bo_fence(vm->root.bo,
&vm->process_info->eviction_fence->base, true);
-   amdgpu_bo_unreserve(vm->root.base.bo);
+   amdgpu_bo_unreserve(vm->root.bo);

/* Update process info */
mutex_lock(&vm->process_info->lock);
@@ -1251,7 +1251,7 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void 
**process_info,
  reserve_shared_fail:
  wait_pd_fail:
  validate_pd_fail:
-   amdgpu_bo_unreserve(vm->root.base.bo);
+   amdgpu_bo_unreserve(vm->root.bo);
  reserve_pd_fail:
vm->process_info = NULL;
if (info) {
@@ -1306,7 +1306,7 @@ void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device 
*adev,
struct amdgpu_vm *vm)
  {
struct amdkfd_process_info *process_info = vm->process_info;
-   struct amdgpu_bo *pd = vm->root.base.bo;
+   struct amdgpu_bo *pd = vm->root.bo;

if (!process_info)
return;
@@ -1362,7 +1362,7 @@ void amdgpu_amdkfd_gpuvm_release_process_vm(struct 
kgd_dev *kgd, void *drm_priv)
  uint64_t amdgpu_amdkfd_gpuvm_get_process_

[PATCH 2/2] drm/amdgpu: move shadow_list to amdgpu_bo_vm

2021-06-15 Thread Nirmoy Das
Move shadow_list to struct amdgpu_bo_vm as shadow BOs
are part of PT/PD BOs.

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  5 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 14 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  6 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c |  2 +-
 4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f2636f411a25..3f51b142fc83 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4063,6 +4063,7 @@ static int amdgpu_device_recover_vram(struct 
amdgpu_device *adev)
 {
struct dma_fence *fence = NULL, *next = NULL;
struct amdgpu_bo *shadow;
+   struct amdgpu_bo_vm *vmbo;
long r = 1, tmo;
 
if (amdgpu_sriov_runtime(adev))
@@ -4072,8 +4073,8 @@ static int amdgpu_device_recover_vram(struct 
amdgpu_device *adev)
 
dev_info(adev->dev, "recover vram bo from shadow start\n");
mutex_lock(&adev->shadow_list_lock);
-   list_for_each_entry(shadow, &adev->shadow_list, shadow_list) {
-
+   list_for_each_entry(vmbo, &adev->shadow_list, shadow_list) {
+   shadow = &vmbo->bo;
/* No need to recover an evicted BO */
if (shadow->tbo.mem.mem_type != TTM_PL_TT ||
shadow->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET ||
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8473d153650f..7dbf711e41ce 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -108,11 +108,13 @@ static void amdgpu_bo_vm_destroy(struct ttm_buffer_object 
*tbo)
 {
struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+   struct amdgpu_bo_vm *vmbo;
 
+   vmbo = to_amdgpu_bo_vm(bo);
/* in case amdgpu_device_recover_vram got NULL of bo->parent */
-   if (!list_empty(&bo->shadow_list)) {
+   if (!list_empty(&vmbo->shadow_list)) {
mutex_lock(&adev->shadow_list_lock);
-   list_del_init(&bo->shadow_list);
+   list_del_init(&vmbo->shadow_list);
mutex_unlock(&adev->shadow_list_lock);
}
 
@@ -588,7 +590,6 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bo == NULL)
return -ENOMEM;
drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
-   INIT_LIST_HEAD(&bo->shadow_list);
bo->vm_bo = NULL;
bo->preferred_domains = bp->preferred_domain ? bp->preferred_domain :
bp->domain;
@@ -718,6 +719,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
return r;
 
*vmbo_ptr = to_amdgpu_bo_vm(bo_ptr);
+   INIT_LIST_HEAD(&(*vmbo_ptr)->shadow_list);
return r;
 }
 
@@ -762,12 +764,12 @@ int amdgpu_bo_validate(struct amdgpu_bo *bo)
  *
  * Insert a BO to the shadow list.
  */
-void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo)
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo)
 {
-   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+   struct amdgpu_device *adev = amdgpu_ttm_adev(vmbo->bo.tbo.bdev);
 
mutex_lock(&adev->shadow_list_lock);
-   list_add_tail(&bo->shadow_list, &adev->shadow_list);
+   list_add_tail(&vmbo->shadow_list, &adev->shadow_list);
mutex_unlock(&adev->shadow_list_lock);
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index a8c702634e1b..18cb2f28e4de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -109,9 +109,6 @@ struct amdgpu_bo {
 #ifdef CONFIG_MMU_NOTIFIER
struct mmu_interval_notifiernotifier;
 #endif
-
-   struct list_headshadow_list;
-
struct kgd_mem  *kfd_bo;
 };
 
@@ -127,6 +124,7 @@ struct amdgpu_bo_user {
 struct amdgpu_bo_vm {
struct amdgpu_bobo;
struct amdgpu_bo*shadow;
+   struct list_headshadow_list;
struct amdgpu_vm_bo_baseentries[];
 };
 
@@ -333,7 +331,7 @@ u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);
 int amdgpu_bo_validate(struct amdgpu_bo *bo);
 void amdgpu_bo_get_memory(struct amdgpu_bo *bo, uint64_t *vram_mem,
uint64_t *gtt_mem, uint64_t *cpu_mem);
-void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo);
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo);
 int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
 struct dma_fence **fence);
 uint32_t amdgpu_bo_get_preferred_pin_domain(struct amdgpu_device *adev,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 4

[PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback

2021-06-15 Thread Nirmoy Das
Make provision to pass different ttm BO destroy callback
while creating a amdgpu_bo.

v2: remove whitespace.
call amdgpu_bo_destroy_base() at the end for cleaner code.

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 48 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
 2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 9092ac12a270..8473d153650f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct amdgpu_bo *bo)
}
 }

-static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)
 {
-   struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
-   struct amdgpu_bo_user *ubo;

if (bo->tbo.pin_count > 0)
amdgpu_bo_subtract_pin_size(bo);
@@ -87,20 +85,38 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
if (bo->tbo.base.import_attach)
drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
drm_gem_object_release(&bo->tbo.base);
+   amdgpu_bo_unref(&bo->parent);
+   kvfree(bo);
+}
+
+static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+{
+   amdgpu_bo_destroy_base(tbo);
+}
+
+static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
+{
+   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+   struct amdgpu_bo_user *ubo;
+
+   ubo = to_amdgpu_bo_user(bo);
+   kfree(ubo->metadata);
+   amdgpu_bo_destroy_base(tbo);
+}
+
+static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
+{
+   struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
+   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+
/* in case amdgpu_device_recover_vram got NULL of bo->parent */
if (!list_empty(&bo->shadow_list)) {
mutex_lock(&adev->shadow_list_lock);
list_del_init(&bo->shadow_list);
mutex_unlock(&adev->shadow_list_lock);
}
-   amdgpu_bo_unref(&bo->parent);
-
-   if (bo->tbo.type != ttm_bo_type_kernel) {
-   ubo = to_amdgpu_bo_user(bo);
-   kfree(ubo->metadata);
-   }

-   kvfree(bo);
+   amdgpu_bo_destroy_base(tbo);
 }

 /**
@@ -115,8 +131,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object 
*tbo)
  */
 bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
 {
-   if (bo->destroy == &amdgpu_bo_destroy)
+   if (bo->destroy == &amdgpu_bo_destroy ||
+   bo->destroy == &amdgpu_bo_user_destroy ||
+   bo->destroy == &amdgpu_bo_vm_destroy)
return true;
+
return false;
 }

@@ -592,9 +611,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bp->type == ttm_bo_type_kernel)
bo->tbo.priority = 1;

+   if (!bp->destroy)
+   bp->destroy = &amdgpu_bo_destroy;
+
r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type,
 &bo->placement, page_align, &ctx,  NULL,
-bp->resv, &amdgpu_bo_destroy);
+bp->resv, bp->destroy);
if (unlikely(r != 0))
return r;

@@ -658,6 +680,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
int r;

bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
+   bp->destroy = &amdgpu_bo_user_destroy;
r = amdgpu_bo_create(adev, bp, &bo_ptr);
if (r)
return r;
@@ -689,6 +712,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
 * num of amdgpu_vm_pt entries.
 */
BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
+   bp->destroy = &amdgpu_bo_vm_destroy;
r = amdgpu_bo_create(adev, bp, &bo_ptr);
if (r)
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index e36b84516b4e..a8c702634e1b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -55,7 +55,8 @@ struct amdgpu_bo_param {
u64 flags;
enum ttm_bo_typetype;
boolno_wait_gpu;
-   struct dma_resv *resv;
+   struct dma_resv *resv;
+   void(*destroy)(struct ttm_buffer_object 
*bo);
 };

 /* bo virtual addresses in a vm */
--
2.31.1

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


[PATCH] drm: display: Fix duplicate field initialization in dcn31

2021-06-15 Thread Wan Jiabing
Fix the following coccicheck warning:
drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c:917:56-57:
pstate_enabled: first occurrence line 935, second occurrence line 937

Signed-off-by: Wan Jiabing 
---
 drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
index 0d6cb6caad81..c67bc9544f5d 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c
@@ -934,7 +934,6 @@ static const struct dc_debug_options debug_defaults_drv = {
.dmub_command_table = true,
.pstate_enabled = true,
.use_max_lb = true,
-   .pstate_enabled = true,
.enable_mem_low_power = {
.bits = {
.vga = false,
-- 
2.20.1

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


Re: [PATCH v2 1/1] drm/amdgpu: remove amdgpu_vm_pt

2021-06-15 Thread Das, Nirmoy

ping.

On 6/14/2021 2:31 PM, Nirmoy Das wrote:

Page table entries are now in embedded in VM BO, so
we do not need struct amdgpu_vm_pt. This patch replaces
struct amdgpu_vm_pt with struct amdgpu_vm_bo_base.

v2: change "!(cursor->level < AMDGPU_VM_PTB)" --> "(cursor->level == 
AMDGPU_VM_PTB)"

Signed-off-by: Nirmoy Das 
---
  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  26 +--
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c|   2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c   |   4 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c   |   2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c|   4 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c   |  12 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |   2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h|   2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 164 --
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h|   9 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c   |   2 +-
  drivers/gpu/drm/amd/amdkfd/kfd_svm.c  |   2 +-
  12 files changed, 105 insertions(+), 126 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index fb6bcc386de1..f96598279593 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -356,7 +356,7 @@ static int amdgpu_amdkfd_validate_vm_bo(void *_unused, 
struct amdgpu_bo *bo)
   */
  static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm)
  {
-   struct amdgpu_bo *pd = vm->root.base.bo;
+   struct amdgpu_bo *pd = vm->root.bo;
struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev);
int ret;

@@ -372,7 +372,7 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm)
return ret;
}

-   vm->pd_phys_addr = amdgpu_gmc_pd_addr(vm->root.base.bo);
+   vm->pd_phys_addr = amdgpu_gmc_pd_addr(vm->root.bo);

if (vm->use_cpu_for_update) {
ret = amdgpu_bo_kmap(pd, NULL);
@@ -387,7 +387,7 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm)

  static int vm_update_pds(struct amdgpu_vm *vm, struct amdgpu_sync *sync)
  {
-   struct amdgpu_bo *pd = vm->root.base.bo;
+   struct amdgpu_bo *pd = vm->root.bo;
struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev);
int ret;

@@ -1153,7 +1153,7 @@ static int process_sync_pds_resv(struct 
amdkfd_process_info *process_info,

list_for_each_entry(peer_vm, &process_info->vm_list_head,
vm_list_node) {
-   struct amdgpu_bo *pd = peer_vm->root.base.bo;
+   struct amdgpu_bo *pd = peer_vm->root.bo;

ret = amdgpu_sync_resv(NULL, sync, pd->tbo.base.resv,
   AMDGPU_SYNC_NE_OWNER,
@@ -1220,7 +1220,7 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void 
**process_info,
vm->process_info = *process_info;

/* Validate page directory and attach eviction fence */
-   ret = amdgpu_bo_reserve(vm->root.base.bo, true);
+   ret = amdgpu_bo_reserve(vm->root.bo, true);
if (ret)
goto reserve_pd_fail;
ret = vm_validate_pt_pd_bos(vm);
@@ -1228,16 +1228,16 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void 
**process_info,
pr_err("validate_pt_pd_bos() failed\n");
goto validate_pd_fail;
}
-   ret = amdgpu_bo_sync_wait(vm->root.base.bo,
+   ret = amdgpu_bo_sync_wait(vm->root.bo,
  AMDGPU_FENCE_OWNER_KFD, false);
if (ret)
goto wait_pd_fail;
-   ret = dma_resv_reserve_shared(vm->root.base.bo->tbo.base.resv, 1);
+   ret = dma_resv_reserve_shared(vm->root.bo->tbo.base.resv, 1);
if (ret)
goto reserve_shared_fail;
-   amdgpu_bo_fence(vm->root.base.bo,
+   amdgpu_bo_fence(vm->root.bo,
&vm->process_info->eviction_fence->base, true);
-   amdgpu_bo_unreserve(vm->root.base.bo);
+   amdgpu_bo_unreserve(vm->root.bo);

/* Update process info */
mutex_lock(&vm->process_info->lock);
@@ -1251,7 +1251,7 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void 
**process_info,
  reserve_shared_fail:
  wait_pd_fail:
  validate_pd_fail:
-   amdgpu_bo_unreserve(vm->root.base.bo);
+   amdgpu_bo_unreserve(vm->root.bo);
  reserve_pd_fail:
vm->process_info = NULL;
if (info) {
@@ -1306,7 +1306,7 @@ void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device 
*adev,
struct amdgpu_vm *vm)
  {
struct amdkfd_process_info *process_info = vm->process_info;
-   struct amdgpu_bo *pd = vm->root.base.bo;
+   struct amdgpu_bo *pd = vm->root.bo;

if (!process_info)
return;
@@ -1362,7 +1362,7 @@ void amdgpu_amdkfd_gpuvm_release_process_vm(struct 
kgd_dev *kgd, void *drm_priv)
  uint64_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *drm_priv)
  {

Re: [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback

2021-06-15 Thread Das, Nirmoy



On 6/14/2021 10:00 PM, Alex Deucher wrote:

On Mon, Jun 14, 2021 at 3:27 PM Nirmoy Das  wrote:

Make provision to pass different ttm BO destroy callback
while creating a amdgpu_bo.

v2: pass destroy callback using amdgpu_bo_param.

Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 52 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
  2 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 9092ac12a270..f4f57a73d095 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -12,7 +12,7 @@
   *
   * 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 NON-INFRINGEMENT. IN NO EVENT SHALL
+ * FITNEsS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
   * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS 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

Unrelated whitespace change.  Please drop.



Thanks Alex. I will remove it in time.




Alex


@@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct amdgpu_bo *bo)
 }
  }

-static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)
  {
-   struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
 struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
-   struct amdgpu_bo_user *ubo;

 if (bo->tbo.pin_count > 0)
 amdgpu_bo_subtract_pin_size(bo);
@@ -87,18 +85,40 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
 if (bo->tbo.base.import_attach)
 drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
 drm_gem_object_release(&bo->tbo.base);
+   amdgpu_bo_unref(&bo->parent);
+}
+
+static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+{
+   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+
+   amdgpu_bo_destroy_base(tbo);
+   kvfree(bo);
+}
+
+static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
+{
+   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+   struct amdgpu_bo_user *ubo;
+
+   amdgpu_bo_destroy_base(tbo);
+   ubo = to_amdgpu_bo_user(bo);
+   kfree(ubo->metadata);
+   kvfree(bo);
+}
+
+static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
+{
+   struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
+   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+
+   amdgpu_bo_destroy_base(tbo);
 /* in case amdgpu_device_recover_vram got NULL of bo->parent */
 if (!list_empty(&bo->shadow_list)) {
 mutex_lock(&adev->shadow_list_lock);
 list_del_init(&bo->shadow_list);
 mutex_unlock(&adev->shadow_list_lock);
 }
-   amdgpu_bo_unref(&bo->parent);
-
-   if (bo->tbo.type != ttm_bo_type_kernel) {
-   ubo = to_amdgpu_bo_user(bo);
-   kfree(ubo->metadata);
-   }

 kvfree(bo);
  }
@@ -115,8 +135,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object 
*tbo)
   */
  bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
  {
-   if (bo->destroy == &amdgpu_bo_destroy)
+   if (bo->destroy == &amdgpu_bo_destroy ||
+   bo->destroy == &amdgpu_bo_user_destroy ||
+   bo->destroy == &amdgpu_bo_vm_destroy)
 return true;
+
 return false;
  }

@@ -592,9 +615,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 if (bp->type == ttm_bo_type_kernel)
 bo->tbo.priority = 1;

+   if (!bp->destroy)
+   bp->destroy = &amdgpu_bo_destroy;
+
 r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type,
  &bo->placement, page_align, &ctx,  NULL,
-bp->resv, &amdgpu_bo_destroy);
+bp->resv, bp->destroy);
 if (unlikely(r != 0))
 return r;

@@ -658,6 +684,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
 int r;

 bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
+   bp->destroy = &amdgpu_bo_user_destroy;
 r = amdgpu_bo_create(adev, bp, &bo_ptr);
 if (r)
 return r;
@@ -689,6 +716,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
  * num of amdgpu_vm_pt entries.
  */
 BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
+   bp->destroy = &amdgpu_bo_vm_destroy;
 r = amdgpu_bo_create(adev, bp, &bo_ptr);
 if (r)
 return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_obj

Re: [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback

2021-06-15 Thread Das, Nirmoy


On 6/15/2021 8:53 AM, Christian König wrote:

Am 14.06.21 um 21:26 schrieb Nirmoy Das:

Make provision to pass different ttm BO destroy callback
while creating a amdgpu_bo.

v2: pass destroy callback using amdgpu_bo_param.

Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 52 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
  2 files changed, 42 insertions(+), 13 deletions(-)

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

index 9092ac12a270..f4f57a73d095 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -12,7 +12,7 @@
   *
   * 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 NON-INFRINGEMENT. IN NO 
EVENT SHALL
+ * FITNEsS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO 
EVENT SHALL
   * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS 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
@@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct 
amdgpu_bo *bo)

  }
  }

-static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)
  {
-    struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
  struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
-    struct amdgpu_bo_user *ubo;

  if (bo->tbo.pin_count > 0)
  amdgpu_bo_subtract_pin_size(bo);
@@ -87,18 +85,40 @@ static void amdgpu_bo_destroy(struct 
ttm_buffer_object *tbo)

  if (bo->tbo.base.import_attach)
  drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
  drm_gem_object_release(&bo->tbo.base);
+    amdgpu_bo_unref(&bo->parent);
+}
+
+static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+{
+    struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+
+    amdgpu_bo_destroy_base(tbo);
+    kvfree(bo);
+}
+
+static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
+{
+    struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+    struct amdgpu_bo_user *ubo;
+
+    amdgpu_bo_destroy_base(tbo);
+    ubo = to_amdgpu_bo_user(bo);
+    kfree(ubo->metadata);
+    kvfree(bo);


Why not freeing the metadata first and having the kvfree() in 
amdgpu_bo_destroy()?



Didn't think about that, it would be me much cleaner.


Thanks,

Nirmoy



Apart from that looks good to me.

Christian.


+}
+
+static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
+{
+    struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
+    struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+
+    amdgpu_bo_destroy_base(tbo);
  /* in case amdgpu_device_recover_vram got NULL of bo->parent */
  if (!list_empty(&bo->shadow_list)) {
  mutex_lock(&adev->shadow_list_lock);
  list_del_init(&bo->shadow_list);
  mutex_unlock(&adev->shadow_list_lock);
  }
-    amdgpu_bo_unref(&bo->parent);
-
-    if (bo->tbo.type != ttm_bo_type_kernel) {
-    ubo = to_amdgpu_bo_user(bo);
-    kfree(ubo->metadata);
-    }

  kvfree(bo);
  }
@@ -115,8 +135,11 @@ static void amdgpu_bo_destroy(struct 
ttm_buffer_object *tbo)

   */
  bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
  {
-    if (bo->destroy == &amdgpu_bo_destroy)
+    if (bo->destroy == &amdgpu_bo_destroy ||
+    bo->destroy == &amdgpu_bo_user_destroy ||
+    bo->destroy == &amdgpu_bo_vm_destroy)
  return true;
+
  return false;
  }

@@ -592,9 +615,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
  if (bp->type == ttm_bo_type_kernel)
  bo->tbo.priority = 1;

+    if (!bp->destroy)
+    bp->destroy = &amdgpu_bo_destroy;
+
  r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, 
bp->type,

   &bo->placement, page_align, &ctx, NULL,
- bp->resv, &amdgpu_bo_destroy);
+ bp->resv, bp->destroy);
  if (unlikely(r != 0))
  return r;

@@ -658,6 +684,7 @@ int amdgpu_bo_create_user(struct amdgpu_device 
*adev,

  int r;

  bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
+    bp->destroy = &amdgpu_bo_user_destroy;
  r = amdgpu_bo_create(adev, bp, &bo_ptr);
  if (r)
  return r;
@@ -689,6 +716,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
   * num of amdgpu_vm_pt entries.
   */
  BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
+    bp->destroy = &amdgpu_bo_vm_destroy;
  r = amdgpu_bo_create(adev, bp, &bo_ptr);
  if (r)
  return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h

index e36b84516b4e..a8c702634e1b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -55,7 +5

[PATCH] drm/amdkfd: Fix a race between queue destroy and process termination

2021-06-15 Thread xinhui pan
We call free_mqd without dqm lock hold, that causes double free of
mqd_mem_obj. Fix it by using a tmp pointer.
We need walk through the queues_list with dqm lock hold. Otherwise hit
list corruption.

Signed-off-by: xinhui pan 
---
 .../drm/amd/amdkfd/kfd_device_queue_manager.c   | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index e6366b408420..575c895fc241 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1484,6 +1484,7 @@ static int destroy_queue_cpsch(struct 
device_queue_manager *dqm,
struct mqd_manager *mqd_mgr;
uint64_t sdma_val = 0;
struct kfd_process_device *pdd = qpd_to_pdd(qpd);
+   void *ptr = NULL;
 
/* Get the SDMA queue stats */
if ((q->properties.type == KFD_QUEUE_TYPE_SDMA) ||
@@ -1543,10 +1544,12 @@ static int destroy_queue_cpsch(struct 
device_queue_manager *dqm,
pr_debug("Total of %d queues are accountable so far\n",
dqm->total_queue_count);
 
+   swap(ptr, q->mqd_mem_obj);
dqm_unlock(dqm);
 
/* Do free_mqd after dqm_unlock(dqm) to avoid circular locking */
-   mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
+   if (ptr)
+   mqd_mgr->free_mqd(mqd_mgr, q->mqd, ptr);
 
return retval;
 
@@ -1709,6 +1712,7 @@ static int process_termination_cpsch(struct 
device_queue_manager *dqm,
enum kfd_unmap_queues_filter filter =
KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES;
bool found = false;
+   void *ptr = NULL;
 
retval = 0;
 
@@ -1737,8 +1741,6 @@ static int process_termination_cpsch(struct 
device_queue_manager *dqm,
qpd->mapped_gws_queue = false;
}
}
-
-   dqm->total_queue_count--;
}
 
/* Unregister process */
@@ -1770,13 +1772,20 @@ static int process_termination_cpsch(struct 
device_queue_manager *dqm,
/* Lastly, free mqd resources.
 * Do free_mqd() after dqm_unlock to avoid circular locking.
 */
+   dqm_lock(dqm);
list_for_each_entry_safe(q, next, &qpd->queues_list, list) {
mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
q->properties.type)];
list_del(&q->list);
qpd->queue_count--;
-   mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
+   dqm->total_queue_count--;
+   swap(ptr, q->mqd_mem_obj);
+   dqm_unlock(dqm);
+   if (ptr)
+   mqd_mgr->free_mqd(mqd_mgr, q->mqd, ptr);
+   dqm_lock(dqm);
}
+   dqm_unlock(dqm);
 
return retval;
 }
-- 
2.25.1

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


Re: [RFC PATCH] drm/ttm: Do page counting after populate callback succeed

2021-06-15 Thread Christian König

Am 15.06.21 um 13:57 schrieb xinhui pan:

Amdgpu set SG flag in populate callback. So TTM still count pages in SG
BO.


It's probably better to fix this instead. E.g. why does amdgpu modify 
the SG flag during populate and not during initial creation? That 
doesn't seem to make sense.


Christian.


One easy way to fix this is lets count pages after populate callback.

We hit one issue that amdgpu alloc many SG BOs, but TTM try to do swap
again and again even if swapout does not swap SG BOs at all.

Signed-off-by: xinhui pan 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 32 +---
  1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index a1a25410ec74..4fa0a8cd71c0 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -317,13 +317,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
if (ttm_tt_is_populated(ttm))
return 0;
  
-	if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {

-   atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
-   if (bdev->pool.use_dma32)
-   atomic_long_add(ttm->num_pages,
-   &ttm_dma32_pages_allocated);
-   }
-
while (atomic_long_read(&ttm_pages_allocated) > ttm_pages_limit ||
   atomic_long_read(&ttm_dma32_pages_allocated) >
   ttm_dma32_pages_limit) {
@@ -342,6 +335,13 @@ int ttm_tt_populate(struct ttm_device *bdev,
if (ret)
goto error;
  
+	if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {

+   atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
+   if (bdev->pool.use_dma32)
+   atomic_long_add(ttm->num_pages,
+   &ttm_dma32_pages_allocated);
+   }
+
ttm_tt_add_mapping(bdev, ttm);
ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED;
if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) {
@@ -355,12 +355,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
return 0;
  
  error:

-   if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
-   atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
-   if (bdev->pool.use_dma32)
-   atomic_long_sub(ttm->num_pages,
-   &ttm_dma32_pages_allocated);
-   }
return ret;
  }
  EXPORT_SYMBOL(ttm_tt_populate);
@@ -384,12 +378,6 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
ttm_tt *ttm)
if (!ttm_tt_is_populated(ttm))
return;
  
-	ttm_tt_clear_mapping(ttm);

-   if (bdev->funcs->ttm_tt_unpopulate)
-   bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
-   else
-   ttm_pool_free(&bdev->pool, ttm);
-
if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
if (bdev->pool.use_dma32)
@@ -397,6 +385,12 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct 
ttm_tt *ttm)
&ttm_dma32_pages_allocated);
}
  
+	ttm_tt_clear_mapping(ttm);

+   if (bdev->funcs->ttm_tt_unpopulate)
+   bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
+   else
+   ttm_pool_free(&bdev->pool, ttm);
+
ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED;
  }
  


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


[PATCH v2] drm/amd/amdgpu: Use IP discovery data to determine VCN enablement instead of MMSCH

2021-06-15 Thread Peng Ju Zhou
From: Bokun Zhang 

In the past, we use MMSCH to determine whether a VCN is enabled or not.
This is not reliable since after a FLR, MMSCH may report junk data.

It is better to use IP discovery data.

Signed-off-by: Bokun Zhang 
Signed-off-by: Peng Ju Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c |  8 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h |  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c   | 23 
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h   | 13 +
 drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 52 +--
 5 files changed, 60 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index f949ed8bfd9e..e02405a24fe3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -373,6 +373,14 @@ int amdgpu_discovery_get_ip_version(struct amdgpu_device 
*adev, int hw_id, int n
return -EINVAL;
 }
 
+
+int amdgpu_discovery_get_vcn_version(struct amdgpu_device *adev, int 
vcn_instance,
+int *major, int *minor, int *revision)
+{
+   return amdgpu_discovery_get_ip_version(adev, VCN_HWID,
+  vcn_instance, major, minor, 
revision);
+}
+
 void amdgpu_discovery_harvest_ip(struct amdgpu_device *adev)
 {
struct binary_header *bhdr;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h
index 02e340cd3a38..48e6b88cfdfe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.h
@@ -32,6 +32,9 @@ int amdgpu_discovery_reg_base_init(struct amdgpu_device 
*adev);
 void amdgpu_discovery_harvest_ip(struct amdgpu_device *adev);
 int amdgpu_discovery_get_ip_version(struct amdgpu_device *adev, int hw_id, int 
number_instance,
 int *major, int *minor, int *revision);
+
+int amdgpu_discovery_get_vcn_version(struct amdgpu_device *adev, int 
vcn_instance,
+int *major, int *minor, int *revision);
 int amdgpu_discovery_get_gfx_info(struct amdgpu_device *adev);
 
 #endif /* __AMDGPU_DISCOVERY__ */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 9492b505e69b..84b025405578 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -287,6 +287,29 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
return 0;
 }
 
+bool amdgpu_vcn_is_disabled_vcn(struct amdgpu_device *adev, enum vcn_ring_type 
type, uint32_t vcn_instance)
+{
+   bool ret = false;
+
+   int major;
+   int minor;
+   int revision;
+
+   /* if cannot find IP data, then this VCN does not exist */
+   if (amdgpu_discovery_get_vcn_version(adev, vcn_instance, &major, 
&minor, &revision) != 0)
+   return true;
+
+   if ((type == VCN_ENCODE_RING) && (revision & 
VCN_BLOCK_ENCODE_DISABLE_MASK)) {
+   ret = true;
+   } else if ((type == VCN_DECODE_RING) && (revision & 
VCN_BLOCK_DECODE_DISABLE_MASK)) {
+   ret = true;
+   } else if ((type == VCN_UNIFIED_RING) && (revision & 
VCN_BLOCK_QUEUE_DISABLE_MASK)) {
+   ret = true;
+   }
+
+   return ret;
+}
+
 int amdgpu_vcn_suspend(struct amdgpu_device *adev)
 {
unsigned size;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index bc76cab67697..d74c62b49795 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -280,6 +280,16 @@ struct amdgpu_vcn_decode_buffer {
uint32_t pad[30];
 };
 
+#define VCN_BLOCK_ENCODE_DISABLE_MASK 0x80
+#define VCN_BLOCK_DECODE_DISABLE_MASK 0x40
+#define VCN_BLOCK_QUEUE_DISABLE_MASK 0xC0
+
+enum vcn_ring_type {
+   VCN_ENCODE_RING,
+   VCN_DECODE_RING,
+   VCN_UNIFIED_RING,
+};
+
 int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
 int amdgpu_vcn_sw_fini(struct amdgpu_device *adev);
 int amdgpu_vcn_suspend(struct amdgpu_device *adev);
@@ -287,6 +297,9 @@ int amdgpu_vcn_resume(struct amdgpu_device *adev);
 void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring);
 void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring);
 
+bool amdgpu_vcn_is_disabled_vcn(struct amdgpu_device *adev,
+   enum vcn_ring_type type, uint32_t vcn_instance);
+
 int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring);
 int amdgpu_vcn_dec_ring_test_ib(struct amdgpu_ring *ring, long timeout);
 int amdgpu_vcn_dec_sw_ring_test_ring(struct amdgpu_ring *ring);
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
index ce3c794c176f..a79ae86bc752 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
@@ -85,9 +85,12 @@ static void vcn_v3_0_enc_ring_set_wptr(struct amdgpu_r

Re: [PATCH 2/2] drm/amdgpu: move shadow_list to amdgpu_bo_vm

2021-06-15 Thread Christian König

Am 14.06.21 um 21:26 schrieb Nirmoy Das:

Move shadow_list to struct amdgpu_bo_vm as shadow BOs
are part of PT/PD BOs.

Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  5 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 13 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  6 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c |  2 +-
  4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f2636f411a25..3f51b142fc83 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4063,6 +4063,7 @@ static int amdgpu_device_recover_vram(struct 
amdgpu_device *adev)
  {
struct dma_fence *fence = NULL, *next = NULL;
struct amdgpu_bo *shadow;
+   struct amdgpu_bo_vm *vmbo;
long r = 1, tmo;
  
  	if (amdgpu_sriov_runtime(adev))

@@ -4072,8 +4073,8 @@ static int amdgpu_device_recover_vram(struct 
amdgpu_device *adev)
  
  	dev_info(adev->dev, "recover vram bo from shadow start\n");

mutex_lock(&adev->shadow_list_lock);
-   list_for_each_entry(shadow, &adev->shadow_list, shadow_list) {
-
+   list_for_each_entry(vmbo, &adev->shadow_list, shadow_list) {
+   shadow = &vmbo->bo;
/* No need to recover an evicted BO */
if (shadow->tbo.mem.mem_type != TTM_PL_TT ||
shadow->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET ||
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index f4f57a73d095..9f1e6bd8601b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -111,12 +111,13 @@ static void amdgpu_bo_vm_destroy(struct ttm_buffer_object 
*tbo)
  {
struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+   struct amdgpu_bo_vm *vmbo = to_amdgpu_bo_vm(bo);
  
  	amdgpu_bo_destroy_base(tbo);

/* in case amdgpu_device_recover_vram got NULL of bo->parent */
-   if (!list_empty(&bo->shadow_list)) {
+   if (!list_empty(&vmbo->shadow_list)) {
mutex_lock(&adev->shadow_list_lock);
-   list_del_init(&bo->shadow_list);
+   list_del_init(&vmbo->shadow_list);
mutex_unlock(&adev->shadow_list_lock);
}
  
@@ -592,7 +593,6 @@ int amdgpu_bo_create(struct amdgpu_device *adev,

if (bo == NULL)
return -ENOMEM;
drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
-   INIT_LIST_HEAD(&bo->shadow_list);
bo->vm_bo = NULL;
bo->preferred_domains = bp->preferred_domain ? bp->preferred_domain :
bp->domain;
@@ -722,6 +722,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
return r;
  
  	*vmbo_ptr = to_amdgpu_bo_vm(bo_ptr);

+   INIT_LIST_HEAD(&(*vmbo_ptr)->shadow_list);
return r;
  }
  
@@ -766,12 +767,12 @@ int amdgpu_bo_validate(struct amdgpu_bo *bo)

   *
   * Insert a BO to the shadow list.
   */
-void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo)
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo)
  {
-   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+   struct amdgpu_device *adev = amdgpu_ttm_adev(vmbo->bo.tbo.bdev);
  
  	mutex_lock(&adev->shadow_list_lock);

-   list_add_tail(&bo->shadow_list, &adev->shadow_list);
+   list_add_tail(&vmbo->shadow_list, &adev->shadow_list);
mutex_unlock(&adev->shadow_list_lock);
  }
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h

index a8c702634e1b..18cb2f28e4de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -109,9 +109,6 @@ struct amdgpu_bo {
  #ifdef CONFIG_MMU_NOTIFIER
struct mmu_interval_notifiernotifier;
  #endif
-
-   struct list_headshadow_list;
-
struct kgd_mem  *kfd_bo;
  };
  
@@ -127,6 +124,7 @@ struct amdgpu_bo_user {

  struct amdgpu_bo_vm {
struct amdgpu_bobo;
struct amdgpu_bo*shadow;
+   struct list_headshadow_list;


We would actually need an amdgpu_bo_shadow for this, but not in this 
patch probably.


Christian.


struct amdgpu_vm_bo_baseentries[];
  };
  
@@ -333,7 +331,7 @@ u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);

  int amdgpu_bo_validate(struct amdgpu_bo *bo);
  void amdgpu_bo_get_memory(struct amdgpu_bo *bo, uint64_t *vram_mem,
uint64_t *gtt_mem, uint64_t *cpu_mem);
-void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo *bo);
+void amdgpu_bo_add_to_shadow_list(struct amdgpu_bo_vm *vmbo);
  int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
 struct dma_fence *

Re: [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback

2021-06-15 Thread Christian König

Am 14.06.21 um 21:26 schrieb Nirmoy Das:

Make provision to pass different ttm BO destroy callback
while creating a amdgpu_bo.

v2: pass destroy callback using amdgpu_bo_param.

Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 52 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
  2 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 9092ac12a270..f4f57a73d095 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -12,7 +12,7 @@
   *
   * 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 NON-INFRINGEMENT. IN NO EVENT SHALL
+ * FITNEsS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
   * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS 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
@@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct amdgpu_bo *bo)
}
  }

-static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)
  {
-   struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
-   struct amdgpu_bo_user *ubo;

if (bo->tbo.pin_count > 0)
amdgpu_bo_subtract_pin_size(bo);
@@ -87,18 +85,40 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
if (bo->tbo.base.import_attach)
drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
drm_gem_object_release(&bo->tbo.base);
+   amdgpu_bo_unref(&bo->parent);
+}
+
+static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+{
+   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+
+   amdgpu_bo_destroy_base(tbo);
+   kvfree(bo);
+}
+
+static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
+{
+   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+   struct amdgpu_bo_user *ubo;
+
+   amdgpu_bo_destroy_base(tbo);
+   ubo = to_amdgpu_bo_user(bo);
+   kfree(ubo->metadata);
+   kvfree(bo);


Why not freeing the metadata first and having the kvfree() in 
amdgpu_bo_destroy()?


Apart from that looks good to me.

Christian.


+}
+
+static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
+{
+   struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
+   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+
+   amdgpu_bo_destroy_base(tbo);
/* in case amdgpu_device_recover_vram got NULL of bo->parent */
if (!list_empty(&bo->shadow_list)) {
mutex_lock(&adev->shadow_list_lock);
list_del_init(&bo->shadow_list);
mutex_unlock(&adev->shadow_list_lock);
}
-   amdgpu_bo_unref(&bo->parent);
-
-   if (bo->tbo.type != ttm_bo_type_kernel) {
-   ubo = to_amdgpu_bo_user(bo);
-   kfree(ubo->metadata);
-   }

kvfree(bo);
  }
@@ -115,8 +135,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object 
*tbo)
   */
  bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
  {
-   if (bo->destroy == &amdgpu_bo_destroy)
+   if (bo->destroy == &amdgpu_bo_destroy ||
+   bo->destroy == &amdgpu_bo_user_destroy ||
+   bo->destroy == &amdgpu_bo_vm_destroy)
return true;
+
return false;
  }

@@ -592,9 +615,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bp->type == ttm_bo_type_kernel)
bo->tbo.priority = 1;

+   if (!bp->destroy)
+   bp->destroy = &amdgpu_bo_destroy;
+
r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type,
 &bo->placement, page_align, &ctx,  NULL,
-bp->resv, &amdgpu_bo_destroy);
+bp->resv, bp->destroy);
if (unlikely(r != 0))
return r;

@@ -658,6 +684,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
int r;

bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
+   bp->destroy = &amdgpu_bo_user_destroy;
r = amdgpu_bo_create(adev, bp, &bo_ptr);
if (r)
return r;
@@ -689,6 +716,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
 * num of amdgpu_vm_pt entries.
 */
BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
+   bp->destroy = &amdgpu_bo_vm_destroy;
r = amdgpu_bo_create(adev, bp, &bo_ptr);
if (r)
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index e36b84516b4e..a8c702634e1b 100644
--- 

Re: [PATCH v2 1/2] drm/amdgpu: parameterize ttm BO destroy callback

2021-06-15 Thread Christian König




Am 15.06.21 um 11:23 schrieb Nirmoy Das:

Make provision to pass different ttm BO destroy callback
while creating a amdgpu_bo.

v2: remove whitespace.
 call amdgpu_bo_destroy_base() at the end for cleaner code.

Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 48 --
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 +-
  2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 9092ac12a270..8473d153650f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -73,11 +73,9 @@ static void amdgpu_bo_subtract_pin_size(struct amdgpu_bo *bo)
}
  }

-static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+static void amdgpu_bo_destroy_base(struct ttm_buffer_object *tbo)


I think you don't even need to rename the function.


  {
-   struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
-   struct amdgpu_bo_user *ubo;

if (bo->tbo.pin_count > 0)
amdgpu_bo_subtract_pin_size(bo);
@@ -87,20 +85,38 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
if (bo->tbo.base.import_attach)
drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
drm_gem_object_release(&bo->tbo.base);
+   amdgpu_bo_unref(&bo->parent);
+   kvfree(bo);
+}
+
+static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
+{
+   amdgpu_bo_destroy_base(tbo);
+}


Nor has that wrapper here.

Apart from that looks good to me.

Christian.


+
+static void amdgpu_bo_user_destroy(struct ttm_buffer_object *tbo)
+{
+   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+   struct amdgpu_bo_user *ubo;
+
+   ubo = to_amdgpu_bo_user(bo);
+   kfree(ubo->metadata);
+   amdgpu_bo_destroy_base(tbo);
+}
+
+static void amdgpu_bo_vm_destroy(struct ttm_buffer_object *tbo)
+{
+   struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
+   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+
/* in case amdgpu_device_recover_vram got NULL of bo->parent */
if (!list_empty(&bo->shadow_list)) {
mutex_lock(&adev->shadow_list_lock);
list_del_init(&bo->shadow_list);
mutex_unlock(&adev->shadow_list_lock);
}
-   amdgpu_bo_unref(&bo->parent);
-
-   if (bo->tbo.type != ttm_bo_type_kernel) {
-   ubo = to_amdgpu_bo_user(bo);
-   kfree(ubo->metadata);
-   }

-   kvfree(bo);
+   amdgpu_bo_destroy_base(tbo);
  }

  /**
@@ -115,8 +131,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object 
*tbo)
   */
  bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
  {
-   if (bo->destroy == &amdgpu_bo_destroy)
+   if (bo->destroy == &amdgpu_bo_destroy ||
+   bo->destroy == &amdgpu_bo_user_destroy ||
+   bo->destroy == &amdgpu_bo_vm_destroy)
return true;
+
return false;
  }

@@ -592,9 +611,12 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bp->type == ttm_bo_type_kernel)
bo->tbo.priority = 1;

+   if (!bp->destroy)
+   bp->destroy = &amdgpu_bo_destroy;
+
r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type,
 &bo->placement, page_align, &ctx,  NULL,
-bp->resv, &amdgpu_bo_destroy);
+bp->resv, bp->destroy);
if (unlikely(r != 0))
return r;

@@ -658,6 +680,7 @@ int amdgpu_bo_create_user(struct amdgpu_device *adev,
int r;

bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
+   bp->destroy = &amdgpu_bo_user_destroy;
r = amdgpu_bo_create(adev, bp, &bo_ptr);
if (r)
return r;
@@ -689,6 +712,7 @@ int amdgpu_bo_create_vm(struct amdgpu_device *adev,
 * num of amdgpu_vm_pt entries.
 */
BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo_vm));
+   bp->destroy = &amdgpu_bo_vm_destroy;
r = amdgpu_bo_create(adev, bp, &bo_ptr);
if (r)
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index e36b84516b4e..a8c702634e1b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -55,7 +55,8 @@ struct amdgpu_bo_param {
u64 flags;
enum ttm_bo_typetype;
boolno_wait_gpu;
-   struct dma_resv *resv;
+   struct dma_resv *resv;
+   void(*destroy)(struct ttm_buffer_object 
*bo);
  };

  /* bo virtual addresses in a vm */
--
2.31.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https