RE: [PATCH] drm/amdgpu: don't enable baco on boco platforms in runpm

[Quan, Evan]

[AMD Official Use Only]

Reviewed-by: Evan Quan 

> -Original Message-
> From: amd-gfx  On Behalf Of Alex
> Deucher
> Sent: Tuesday, August 3, 2021 12:46 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander 
> Subject: [PATCH] drm/amdgpu: don't enable baco on boco platforms in
> runpm
> 
> If the platform uses BOCO, don't use BACO in runtime suspend.
> We could end up executing the BACO path if the platform supports both.
> 
> Bug:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
> b.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1669data=04%7C01%7Cevan.quan%40amd.com%7Cd65
> c86a3b85b452e049708d955d4fe72%7C3dd8961fe4884e608e11a82d994e183d
> %7C0%7C0%7C63763519369506%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1
> 000sdata=7eXhQgYtlr6ek%2BNtuLQyrCKvyGCePctiD0cKlFjsIhw%3D
> mp;reserved=0
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 91a5ed96bfbe..d96aaa2aa5ba 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1566,6 +1566,8 @@ static int amdgpu_pmops_runtime_suspend(struct
> device *dev)
>   pci_ignore_hotplug(pdev);
>   pci_set_power_state(pdev, PCI_D3cold);
>   drm_dev->switch_power_state =
> DRM_SWITCH_POWER_DYNAMIC_OFF;
> + } else if (amdgpu_device_supports_boco(drm_dev)) {
> + /* nothing to do */
>   } else if (amdgpu_device_supports_baco(drm_dev)) {
>   amdgpu_device_baco_enter(drm_dev);
>   }
> --
> 2.31.1

RE: [PATCH] drm/amdgpu: added synchronization for psp cmd buf access

[Chen, Guchun]

[Public]

Thanks John. As in the same context, it's meaningless that two mutex target 
almost the same thing.

Regards,
Guchun

From: Clements, John 
Sent: Wednesday, August 4, 2021 11:34 AM
To: Chen, Guchun ; amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Li, Candice ; 
Lazar, Lijo 
Subject: RE: [PATCH] drm/amdgpu: added synchronization for psp cmd buf access


[Public]

@Chen, Guchun,
Based off your feedback I double checked the code, and I changed my opinion 
about it, I think it's better just to reuse the original mutex for now. I've 
submitted an updated patch for review

From: Clements, John mailto:john.cleme...@amd.com>>
Sent: Tuesday, August 3, 2021 10:07 PM
To: Chen, Guchun mailto:guchun.c...@amd.com>>; 
amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking mailto:hawking.zh...@amd.com>>; Li, 
Candice mailto:candice...@amd.com>>; Lazar, Lijo 
mailto:lijo.la...@amd.com>>
Subject: Re: [PATCH] drm/amdgpu: added synchronization for psp cmd buf access

Hello Guchun,

In most of those cases you are right it is redundant, the reason i kept them 
separate for now is to resolve this bug while also keeping those interfaces 
modular, and not affecting the psp submit sequence yet. We are planning a 
bigger change to that source to remove alot of the duplicate code regarding the 
cmd buffer prepare/submit flow and will probably go back down to one mutex 
there.

Thank you,
John Clements


From: Chen, Guchun mailto:guchun.c...@amd.com>>
Sent: Tuesday, August 3, 2021 9:58 PM
To: Clements, John mailto:john.cleme...@amd.com>>; 
amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Zhang, Hawking mailto:hawking.zh...@amd.com>>; Li, 
Candice mailto:candice...@amd.com>>; Lazar, Lijo 
mailto:lijo.la...@amd.com>>
Subject: RE: [PATCH] drm/amdgpu: added synchronization for psp cmd buf access


[Public]



Before calling into psp_cmd_submit_buf, a mutex psp->cmd_buf_mutex is there, 
and after entering psp_cmd_submit_buf, there is another mutex psp->mutex, is it 
a bit redundant?



Regards,

Guchun



From: Clements, John mailto:john.cleme...@amd.com>>
Sent: Tuesday, August 3, 2021 5:50 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking mailto:hawking.zh...@amd.com>>; Li, 
Candice mailto:candice...@amd.com>>; Lazar, Lijo 
mailto:lijo.la...@amd.com>>; Chen, Guchun 
mailto:guchun.c...@amd.com>>
Subject: [PATCH] drm/amdgpu: added synchronization for psp cmd buf access



[AMD Official Use Only]



Submitting patch to synchronize access to psp cmd submission memory to resolve 
potential race conditions.

RE: [PATCH] drm/amdgpu: add DID for beige goby

[Chen, Guchun]

[Public]

Reviewed-by: Guchun Chen 

Regards,
Guchun

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Wednesday, August 4, 2021 3:39 AM
To: amd-gfx@lists.freedesktop.org
Cc: Gui, Jack ; Deucher, Alexander 
Subject: [PATCH] drm/amdgpu: add DID for beige goby

From: Chengming Gui 

Add device ids.

Signed-off-by: Chengming Gui 
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 91a5ed96bfbe..b02c87ae4245 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1215,6 +1215,13 @@ static const struct pci_device_id pciidlist[] = {
/* CYAN_SKILLFISH */
{0x1002, 0x13FE, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_CYAN_SKILLFISH|AMD_IS_APU},
 
+   /* BEIGE_GOBY */
+   {0x1002, 0x7420, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+   {0x1002, 0x7421, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+   {0x1002, 0x7422, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+   {0x1002, 0x7423, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+   {0x1002, 0x743F, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+
{0, 0, 0}
 };
 
-- 
2.31.1

[PATCH] drm/amdgpu: Add driver version

[Peng Ju Zhou]

From: David M Nieto 

This sysfs is only defined in DKMS drivers
it exposes the internal AMDGPU version

Signed-off-by: David M Nieto 
Signed-off-by: Peng Ju Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2659 +++-
 1 file changed, 941 insertions(+), 1718 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 9e53ff851496..d93d1c966bad 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -65,13 +66,9 @@
 #include "amdgpu_ras.h"
 #include "amdgpu_pmu.h"
 #include "amdgpu_fru_eeprom.h"
-#include "amdgpu_reset.h"
 
 #include 
 #include 
-#include 
-
-#include 
 
 MODULE_FIRMWARE("amdgpu/vega10_gpu_info.bin");
 MODULE_FIRMWARE("amdgpu/vega12_gpu_info.bin");
@@ -83,8 +80,6 @@ MODULE_FIRMWARE("amdgpu/renoir_gpu_info.bin");
 MODULE_FIRMWARE("amdgpu/navi10_gpu_info.bin");
 MODULE_FIRMWARE("amdgpu/navi14_gpu_info.bin");
 MODULE_FIRMWARE("amdgpu/navi12_gpu_info.bin");
-MODULE_FIRMWARE("amdgpu/vangogh_gpu_info.bin");
-MODULE_FIRMWARE("amdgpu/yellow_carp_gpu_info.bin");
 
 #define AMDGPU_RESUME_MS   2000
 
@@ -114,17 +109,9 @@ const char *amdgpu_asic_name[] = {
"RAVEN",
"ARCTURUS",
"RENOIR",
-   "ALDEBARAN",
"NAVI10",
-   "CYAN_SKILLFISH",
"NAVI14",
"NAVI12",
-   "SIENNA_CICHLID",
-   "NAVY_FLOUNDER",
-   "VANGOGH",
-   "DIMGREY_CAVEFISH",
-   "BEIGE_GOBY",
-   "YELLOW_CARP",
"LAST",
 };
 
@@ -141,10 +128,10 @@ static ssize_t amdgpu_device_get_pcie_replay_count(struct 
device *dev,
struct device_attribute *attr, char *buf)
 {
struct drm_device *ddev = dev_get_drvdata(dev);
-   struct amdgpu_device *adev = drm_to_adev(ddev);
+   struct amdgpu_device *adev = ddev->dev_private;
uint64_t cnt = amdgpu_asic_get_pcie_replay_count(adev);
 
-   return sysfs_emit(buf, "%llu\n", cnt);
+   return snprintf(buf, PAGE_SIZE, "%llu\n", cnt);
 }
 
 static DEVICE_ATTR(pcie_replay_count, S_IRUGO,
@@ -166,9 +153,9 @@ static ssize_t amdgpu_device_get_product_name(struct device 
*dev,
struct device_attribute *attr, char *buf)
 {
struct drm_device *ddev = dev_get_drvdata(dev);
-   struct amdgpu_device *adev = drm_to_adev(ddev);
+   struct amdgpu_device *adev = ddev->dev_private;
 
-   return sysfs_emit(buf, "%s\n", adev->product_name);
+   return snprintf(buf, PAGE_SIZE, "%s\n", adev->product_name);
 }
 
 static DEVICE_ATTR(product_name, S_IRUGO,
@@ -188,9 +175,9 @@ static ssize_t amdgpu_device_get_product_number(struct 
device *dev,
struct device_attribute *attr, char *buf)
 {
struct drm_device *ddev = dev_get_drvdata(dev);
-   struct amdgpu_device *adev = drm_to_adev(ddev);
+   struct amdgpu_device *adev = ddev->dev_private;
 
-   return sysfs_emit(buf, "%s\n", adev->product_number);
+   return snprintf(buf, PAGE_SIZE, "%s\n", adev->product_number);
 }
 
 static DEVICE_ATTR(product_number, S_IRUGO,
@@ -210,45 +197,27 @@ static ssize_t amdgpu_device_get_serial_number(struct 
device *dev,
struct device_attribute *attr, char *buf)
 {
struct drm_device *ddev = dev_get_drvdata(dev);
-   struct amdgpu_device *adev = drm_to_adev(ddev);
+   struct amdgpu_device *adev = ddev->dev_private;
 
-   return sysfs_emit(buf, "%s\n", adev->serial);
+   return snprintf(buf, PAGE_SIZE, "%s\n", adev->serial);
 }
 
 static DEVICE_ATTR(serial_number, S_IRUGO,
amdgpu_device_get_serial_number, NULL);
 
 /**
- * amdgpu_device_supports_px - Is the device a dGPU with ATPX power control
- *
- * @dev: drm_device pointer
- *
- * Returns true if the device is a dGPU with ATPX power control,
- * otherwise return false.
- */
-bool amdgpu_device_supports_px(struct drm_device *dev)
-{
-   struct amdgpu_device *adev = drm_to_adev(dev);
-
-   if ((adev->flags & AMD_IS_PX) && !amdgpu_is_atpx_hybrid())
-   return true;
-   return false;
-}
-
-/**
- * amdgpu_device_supports_boco - Is the device a dGPU with ACPI power resources
+ * amdgpu_device_supports_boco - Is the device a dGPU with HG/PX power control
  *
  * @dev: drm_device pointer
  *
- * Returns true if the device is a dGPU with ACPI power control,
+ * Returns true if the device is a dGPU with HG/PX power control,
  * otherwise return false.
  */
 bool amdgpu_device_supports_boco(struct drm_device *dev)
 {
-   struct amdgpu_device *adev = drm_to_adev(dev);
+   struct amdgpu_device *adev = dev->dev_private;
 
-   if (adev->has_pr3 ||
-   ((adev->flags & AMD_IS_PX) && amdgpu_is_atpx_hybrid()))
+   if (adev->flags & AMD_IS_PX)
return true;
return false;
 }
@@ -263,32 +232,15 @@ bool amdgpu_device_supports_boco(struct drm_device *dev)
  */
 bool 

[PATCH] drm/amdgpu: add done BO list

[Peng Ju Zhou]

From: David M Nieto 

backport of "add a list in VM for BOs in the done state"

Signed-off-by: David M Nieto 
Signed-off-by: Peng Ju Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 1196 +++-
 1 file changed, 561 insertions(+), 635 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 2a88ed5d983b..ecf7f2039de0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -25,22 +25,16 @@
  *  Alex Deucher
  *  Jerome Glisse
  */
-
 #include 
 #include 
 #include 
-#include 
 
 #include 
-#include 
 #include "amdgpu.h"
 #include "amdgpu_trace.h"
 #include "amdgpu_amdkfd.h"
 #include "amdgpu_gmc.h"
 #include "amdgpu_xgmi.h"
-#include "amdgpu_dma_buf.h"
-#include "amdgpu_res_cursor.h"
-#include "kfd_svm.h"
 
 /**
  * DOC: GPUVM
@@ -89,46 +83,6 @@ struct amdgpu_prt_cb {
 };
 
 /**
- * amdgpu_vm_set_pasid - manage pasid and vm ptr mapping
- *
- * @adev: amdgpu_device pointer
- * @vm: amdgpu_vm pointer
- * @pasid: the pasid the VM is using on this GPU
- *
- * Set the pasid this VM is using on this GPU, can also be used to remove the
- * pasid by passing in zero.
- *
- */
-int amdgpu_vm_set_pasid(struct amdgpu_device *adev, struct amdgpu_vm *vm,
-   u32 pasid)
-{
-   int r;
-
-   if (vm->pasid == pasid)
-   return 0;
-
-   if (vm->pasid) {
-   r = xa_err(xa_erase_irq(>vm_manager.pasids, vm->pasid));
-   if (r < 0)
-   return r;
-
-   vm->pasid = 0;
-   }
-
-   if (pasid) {
-   r = xa_err(xa_store_irq(>vm_manager.pasids, pasid, vm,
-   GFP_KERNEL));
-   if (r < 0)
-   return r;
-
-   vm->pasid = pasid;
-   }
-
-
-   return 0;
-}
-
-/*
  * vm eviction_lock can be taken in MMU notifiers. Make sure no reclaim-FS
  * happens while holding this lock anywhere to prevent deadlocks when
  * an MMU notifier runs in reclaim-FS context.
@@ -136,13 +90,13 @@ int amdgpu_vm_set_pasid(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
 static inline void amdgpu_vm_eviction_lock(struct amdgpu_vm *vm)
 {
mutex_lock(>eviction_lock);
-   vm->saved_flags = memalloc_noreclaim_save();
+   vm->saved_flags = memalloc_nofs_save();
 }
 
 static inline int amdgpu_vm_eviction_trylock(struct amdgpu_vm *vm)
 {
if (mutex_trylock(>eviction_lock)) {
-   vm->saved_flags = memalloc_noreclaim_save();
+   vm->saved_flags = memalloc_nofs_save();
return 1;
}
return 0;
@@ -150,7 +104,7 @@ static inline int amdgpu_vm_eviction_trylock(struct 
amdgpu_vm *vm)
 
 static inline void amdgpu_vm_eviction_unlock(struct amdgpu_vm *vm)
 {
-   memalloc_noreclaim_restore(vm->saved_flags);
+   memalloc_nofs_restore(vm->saved_flags);
mutex_unlock(>eviction_lock);
 }
 
@@ -372,7 +326,7 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base 
*base,
base->next = bo->vm_bo;
bo->vm_bo = base;
 
-   if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+   if (amdkcl_ttm_resvp(>tbo) != 
amdkcl_ttm_resvp(>root.base.bo->tbo))
return;
 
vm->bulk_moveable = false;
@@ -382,7 +336,7 @@ static void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base 
*base,
amdgpu_vm_bo_idle(base);
 
if (bo->preferred_domains &
-   amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type))
+   amdgpu_mem_type_to_domain(bo->tbo.mem.mem_type))
return;
 
/*
@@ -401,14 +355,14 @@ static void amdgpu_vm_bo_base_init(struct 
amdgpu_vm_bo_base *base,
  * Helper to get the parent entry for the child page table. NULL if we are at
  * the root page directory.
  */
-static struct amdgpu_vm_bo_base *amdgpu_vm_pt_parent(struct amdgpu_vm_bo_base 
*pt)
+static struct amdgpu_vm_pt *amdgpu_vm_pt_parent(struct amdgpu_vm_pt *pt)
 {
-   struct amdgpu_bo *parent = pt->bo->parent;
+   struct amdgpu_bo *parent = pt->base.bo->parent;
 
if (!parent)
return NULL;
 
-   return parent->vm_bo;
+   return container_of(parent->vm_bo, struct amdgpu_vm_pt, base);
 }
 
 /*
@@ -416,8 +370,8 @@ static struct amdgpu_vm_bo_base *amdgpu_vm_pt_parent(struct 
amdgpu_vm_bo_base *p
  */
 struct amdgpu_vm_pt_cursor {
uint64_t pfn;
-   struct amdgpu_vm_bo_base *parent;
-   struct amdgpu_vm_bo_base *entry;
+   struct amdgpu_vm_pt *parent;
+   struct amdgpu_vm_pt *entry;
unsigned level;
 };
 
@@ -456,17 +410,17 @@ static bool amdgpu_vm_pt_descendant(struct amdgpu_device 
*adev,
 {
unsigned mask, shift, idx;
 
-   if ((cursor->level == AMDGPU_VM_PTB) || !cursor->entry ||
-   !cursor->entry->bo)
+   if (!cursor->entry->entries)
return false;
 
+   BUG_ON(!cursor->entry->base.bo);

[PATCH] DRM: amd: amdgpu: Fixed code style and removed unnecessary if statement

[Sergio Miguéns Iglesias]

The file didn't follow the code style so it was changed. The "if" statement
checked if the framebuffer was initialized was also changed since that
condition is unlikely to happen. An unnecessary "if" was removed too since it
didn't execute any code if the condition was met.

Signed-off-by: Sergio Miguéns Iglesias 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index 09b048647523..d2ab07eb00fb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -42,15 +42,16 @@
 #include "amdgpu_display.h"
 
 /* object hierarchy -
-   this contains a helper + a amdgpu fb
-   the helper contains a pointer to amdgpu framebuffer baseclass.
-*/
+ * this contains a helper + a amdgpu fb
+ * the helper contains a pointer to amdgpu framebuffer baseclass.
+ */
 
 static int
 amdgpufb_open(struct fb_info *info, int user)
 {
struct drm_fb_helper *fb_helper = info->par;
int ret = pm_runtime_get_sync(fb_helper->dev->dev);
+
if (ret < 0 && ret != -EACCES) {
pm_runtime_mark_last_busy(fb_helper->dev->dev);
pm_runtime_put_autosuspend(fb_helper->dev->dev);
@@ -182,9 +183,8 @@ static int amdgpufb_create_pinned_object(struct 
amdgpu_fbdev *rfbdev,
 
ret = amdgpu_bo_kmap(abo, NULL);
amdgpu_bo_unreserve(abo);
-   if (ret) {
+   if (ret)
goto out_unref;
-   }
 
*gobj_p = gobj;
return 0;
@@ -233,7 +233,7 @@ static int amdgpufb_create(struct drm_fb_helper *helper,
 
ret = amdgpu_display_gem_fb_init(adev_to_drm(adev), >rfb,
 _cmd, gobj);
-   if (ret) {
+   if (unlikely(ret)) {
DRM_ERROR("failed to initialize framebuffer %d\n", ret);
goto out;
}
@@ -258,7 +258,7 @@ static int amdgpufb_create(struct drm_fb_helper *helper,
 
/* Use default scratch pixmap (info->pixmap.flags = FB_PIXMAP_SYSTEM) */
 
-   if (info->screen_base == NULL) {
+   if (unlikely(info->screen_base == NULL)) {
ret = -ENOSPC;
goto out;
}
@@ -273,9 +273,6 @@ static int amdgpufb_create(struct drm_fb_helper *helper,
return 0;
 
 out:
-   if (abo) {
-
-   }
if (fb && ret) {
drm_gem_object_put(gobj);
drm_framebuffer_unregister_private(fb);
-- 
2.32.0

Re: [PATCH] drm/amdkfd: AIP mGPUs best prefetch location for xnack on

[Felix Kuehling]

Am 2021-08-02 um 12:28 p.m. schrieb Philip Yang:
> For xnack on, if range ACCESS or ACCESS_IN_PLACE (AIP) by single GPU, or
> range is ACCESS_IN_PLACE by mGPUs and all mGPUs connection on xgmi same
> hive, the best prefetch location is prefetch_loc GPU. Otherwise, the best
> prefetch location is always CPU because GPU can not map vram of other
> GPUs through small bar PCIe.
>
> Signed-off-by: Philip Yang 
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 44 +---
>  1 file changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 69237d2ab2ad..6160c301f78b 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -2754,22 +2754,29 @@ svm_range_add(struct kfd_process *p, uint64_t start, 
> uint64_t size,
>   return 0;
>  }
>  
> -/* svm_range_best_prefetch_location - decide the best prefetch location
> +/**
> + * svm_range_best_prefetch_location - decide the best prefetch location
>   * @prange: svm range structure
>   *
>   * For xnack off:
> - * If range map to single GPU, the best acutal location is prefetch loc, 
> which
> + * If range map to single GPU, the best prefetch location is prefetch_loc, 
> which
>   * can be CPU or GPU.
>   *
> - * If range map to multiple GPUs, only if mGPU connection on xgmi same hive,
> - * the best actual location could be prefetch_loc GPU. If mGPU connection on
> - * PCIe, the best actual location is always CPU, because GPU cannot access 
> vram
> - * of other GPUs, assuming PCIe small bar (large bar support is not 
> upstream).
> + * If range is ACCESS or ACCESS_IN_PLACE by mGPUs, only if mGPU connection on
> + * xgmi same hive, the best prefetch location is prefetch_loc GPU. If mGPUs
> + * connection on PCIe, the best prefetch location is always CPU, because GPU
> + * cannot map vram of other GPUs, assuming PCIe small bar (large bar support 
> is
> + * not upstream).
>   *
>   * For xnack on:
> - * The best actual location is prefetch location. If mGPU connection on xgmi
> - * same hive, range map to multiple GPUs. Otherwise, the range only map to
> - * actual location GPU. Other GPU access vm fault will trigger migration.
> + * If range map to single GPU or is not ACCESS_IN_PLACE by mGPUs, the best
> + * prefetch location is prefetch_loc, other GPU access vm fault will trigger
> + * migration.
> + *
> + * If range is ACCESS_IN_PLACE by mGPUs, only if mGPU connection on xgmi same
> + * hive, the best prefetch location is prefetch_loc GPU. If mGPUs connection 
> on
> + * PCIe, the best prefetch location is always CPU, because GPU cannot map 
> vram
> + * of other GPUs.
>   *
>   * Context: Process context
>   *
> @@ -2787,14 +2794,13 @@ svm_range_best_prefetch_location(struct svm_range 
> *prange)
>   struct kfd_process *p;
>   uint32_t gpuidx;
>  
> - p = container_of(prange->svms, struct kfd_process, svms);
> -
> - /* xnack on */
> - if (p->xnack_enabled)
> + if (!best_loc || best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED)
>   goto out;
>  
> - /* xnack off */
> - if (!best_loc || best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED)
> + p = container_of(prange->svms, struct kfd_process, svms);
> +
> + if (p->xnack_enabled &&
> +bitmap_weight(prange->bitmap_aip, MAX_GPU_INSTANCE) <= 1)

I'm not sure why this condition is needed. I think there is an implicit
assumption that the prefetch_location is the one GPU that's set in the
bitmap_aip. But is that really a safe assumption? If you're prefetching
to a different GPU than the one that needs to access the memory
in-place, then the access is still going to trigger a page fault, so
it's not "in-place".

I think you can just drop this condition. It's a questionable short-cut
for something that's handled more correctly in the loop below.

Regards,
  Felix


>   goto out;
>  
>   bo_adev = svm_range_get_adev_by_id(prange, best_loc);
> @@ -2803,8 +2809,12 @@ svm_range_best_prefetch_location(struct svm_range 
> *prange)
>   best_loc = 0;
>   goto out;
>   }
> - bitmap_or(bitmap, prange->bitmap_access, prange->bitmap_aip,
> -   MAX_GPU_INSTANCE);
> +
> + if (p->xnack_enabled)
> + bitmap_copy(bitmap, prange->bitmap_aip, MAX_GPU_INSTANCE);
> + else
> + bitmap_or(bitmap, prange->bitmap_access, prange->bitmap_aip,
> +   MAX_GPU_INSTANCE);
>  
>   for_each_set_bit(gpuidx, bitmap, MAX_GPU_INSTANCE) {
>   pdd = kfd_process_device_from_gpuidx(p, gpuidx);

Re: [PATCH] drm/amdkfd: support file backed SVM range

[philip yang]

  


On 2021-08-03 1:01 p.m., Felix Kuehling
  wrote:


  Am 2021-08-02 um 10:56 a.m. schrieb Philip Yang:

  
HMM migrate helper migrate_vma_pages do not migrate file backed pages to
replace it with device pages because the pages are used by file cache.
We can not migrate the file backed range to VRAM, otherwise CPU access
range will not trigger page fault to migrate updated data from VRAM back
to system memory.

For file backed range, don't prefetch migrate range to VRAM, always map
system pages to GPU and also use system pages to recover GPU retry
fault.

Add helper to check if range is file backed or anonymous mapping.

Signed-off-by: Philip Yang 

  
  
This patch should not be submitted to amd-staging-drm-next. As I
understand it, the real fix is in Alex's partial-migration patch series.
This patch is a hack that could be useful for older release branches
that won't get Alex's patch series because it's too invasive. So let's
review this on the internal mailing list.

As partial-migration patch series are checked into dkms branch,
  and file backed range works fine, so this patch is not needed, I
  will abandon it.
Thanks,
Philip


  

Thanks,
  Felix



  
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 40 
 1 file changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index f811a3a24cd2..69237d2ab2ad 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2400,6 +2400,36 @@ svm_range_count_fault(struct amdgpu_device *adev, struct kfd_process *p,
 		WRITE_ONCE(pdd->faults, pdd->faults + 1);
 }
 
+/**
+ * svm_range_is_file_backed - decide if prange is file backed mmap
+ * @mm: the mm structure
+ * @prange: svm range structure
+ *
+ * Context: caller must hold mmap_read_lock
+ *
+ * Return:
+ * false if entire range is anonymous mapping
+ * true if entire or partial range is file backed, or invalid mapping address
+ */
+static bool
+svm_range_is_file_backed(struct mm_struct *mm, struct svm_range *prange)
+{
+	struct vm_area_struct *vma;
+	unsigned long start, end;
+
+	start = prange->start << PAGE_SHIFT;
+	end = (prange->last + 1) << PAGE_SHIFT;
+
+	do {
+		vma = find_vma(mm, start);
+		if (!vma || !vma_is_anonymous(vma))
+			return true;
+		start = min(end, vma->vm_end);
+	} while (start < end);
+
+	return false;
+}
+
 int
 svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
 			uint64_t addr)
@@ -2496,6 +2526,10 @@ svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
 		 svms, prange->start, prange->last, best_loc,
 		 prange->actual_loc);
 
+	/* for file backed range, use system memory pages for GPU mapping */
+	if (svm_range_is_file_backed(mm, prange))
+		goto out_validate_and_map;
+
 	if (prange->actual_loc != best_loc) {
 		if (best_loc) {
 			r = svm_migrate_to_vram(prange, best_loc, mm);
@@ -2520,6 +2554,7 @@ svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
 		}
 	}
 
+out_validate_and_map:
 	r = svm_range_validate_and_map(mm, prange, gpuidx, false, false);
 	if (r)
 		pr_debug("failed %d to map svms 0x%p [0x%lx 0x%lx] to gpus\n",
@@ -2850,6 +2885,11 @@ svm_range_trigger_migration(struct mm_struct *mm, struct svm_range *prange,
 	int r = 0;
 
 	*migrated = false;
+
+	/* Don't migrate file backed range to VRAM */
+	if (svm_range_is_file_backed(mm, prange))
+		return 0;
+
 	best_loc = svm_range_best_prefetch_location(prange);
 
 	if (best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED ||

  

  

Re: [PATCH] drm/amdkfd: add parameter force in kfd_process_evict_queues

[Felix Kuehling]

m 2021-08-03 um 2:57 p.m. schrieb Eric Huang:
> It is to differenciate case scenario for proper behavior when
> calling evict queues, such as GPU reset doesn't need to roll
> back restoring partial evicted queues.
>
> Signed-off-by: Eric Huang 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h |  4 ++--
>  drivers/gpu/drm/amd/amdkfd/kfd_device.c| 10 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h  |  4 ++--
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c   | 18 ++
>  5 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index 77044e8ba4e6..59ce5a17a834 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -190,7 +190,7 @@ void amdgpu_amdkfd_interrupt(struct amdgpu_device *adev,
>  void amdgpu_amdkfd_suspend(struct amdgpu_device *adev, bool run_pm)
>  {
>   if (adev->kfd.dev)
> - kgd2kfd_suspend(adev->kfd.dev, run_pm);
> + kgd2kfd_suspend(adev->kfd.dev, run_pm, false);

If suspend fails, this should return an error that should be handled in
amdgpu_device_suspend. Maybe this could be fixed in a follow up patch.
This means kgd2kfd_suspend and kfd_suspend_all_processes should not
return void and return an error code on failures at least if force=false.

Otherwise this patch is

Reviewed-by: Felix Kuehling 


>  }
>  
>  int amdgpu_amdkfd_resume(struct amdgpu_device *adev, bool run_pm, bool sync)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 332ccba00e69..b7e46ad0507e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -372,7 +372,7 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
>struct drm_device *ddev,
>const struct kgd2kfd_shared_resources *gpu_resources);
>  void kgd2kfd_device_exit(struct kfd_dev *kfd);
> -void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm);
> +void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm, bool force);
>  int kgd2kfd_resume(struct kfd_dev *kfd, bool run_pm, bool sync);
>  int kgd2kfd_pre_reset(struct kfd_dev *kfd);
>  int kgd2kfd_post_reset(struct kfd_dev *kfd);
> @@ -407,7 +407,7 @@ static inline void kgd2kfd_device_exit(struct kfd_dev 
> *kfd)
>  {
>  }
>  
> -static inline void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm)
> +static inline void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm, bool 
> force)
>  {
>  }
>  
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index 24b5e0aa1eac..48e51ee8de56 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -940,7 +940,7 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
>  void kgd2kfd_device_exit(struct kfd_dev *kfd)
>  {
>   if (kfd->init_complete) {
> - kgd2kfd_suspend(kfd, false);
> + kgd2kfd_suspend(kfd, false, true);
>   svm_migrate_fini((struct amdgpu_device *)kfd->kgd);
>   device_queue_manager_uninit(kfd->dqm);
>   kfd_interrupt_exit(kfd);
> @@ -965,7 +965,7 @@ int kgd2kfd_pre_reset(struct kfd_dev *kfd)
>  
>   kfd->dqm->ops.pre_reset(kfd->dqm);
>  
> - kgd2kfd_suspend(kfd, false);
> + kgd2kfd_suspend(kfd, false, true);
>  
>   kfd_signal_reset_event(kfd);
>   return 0;
> @@ -1001,7 +1001,7 @@ bool kfd_is_locked(void)
>   return  (atomic_read(_locked) > 0);
>  }
>  
> -void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm)
> +void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm, bool force)
>  {
>   if (!kfd->init_complete)
>   return;
> @@ -1010,7 +1010,7 @@ void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm)
>   if (!run_pm) {
>   /* For first KFD device suspend all the KFD processes */
>   if (atomic_inc_return(_locked) == 1)
> - kfd_suspend_all_processes();
> + kfd_suspend_all_processes(force);
>   }
>  
>   kfd->dqm->ops.stop(kfd->dqm);
> @@ -1122,7 +1122,7 @@ int kgd2kfd_quiesce_mm(struct mm_struct *mm)
>   return -ESRCH;
>  
>   WARN(debug_evictions, "Evicting pid %d", p->lead_thread->pid);
> - r = kfd_process_evict_queues(p);
> + r = kfd_process_evict_queues(p, true);
>  
>   kfd_unref_process(p);
>   return r;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 3d5d3994d8a4..e80fb64a6dcc 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -1042,9 +1042,9 @@ static inline struct kfd_process_device 
> *kfd_process_device_from_gpuidx(
>  }
>  
>  void kfd_unref_process(struct kfd_process *p);
> -int kfd_process_evict_queues(struct kfd_process 

[PATCH] drm/amdgpu: add DID for beige goby

[Alex Deucher]

From: Chengming Gui 

Add device ids.

Signed-off-by: Chengming Gui 
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 91a5ed96bfbe..b02c87ae4245 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1215,6 +1215,13 @@ static const struct pci_device_id pciidlist[] = {
/* CYAN_SKILLFISH */
{0x1002, 0x13FE, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 
CHIP_CYAN_SKILLFISH|AMD_IS_APU},
 
+   /* BEIGE_GOBY */
+   {0x1002, 0x7420, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+   {0x1002, 0x7421, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+   {0x1002, 0x7422, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+   {0x1002, 0x7423, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+   {0x1002, 0x743F, PCI_ANY_ID, PCI_ANY_ID, 0, 0, CHIP_BEIGE_GOBY},
+
{0, 0, 0}
 };
 
-- 
2.31.1

[PATCH] drm/amdkfd: add parameter force in kfd_process_evict_queues

[Eric Huang]

It is to differenciate case scenario for proper behavior when
calling evict queues, such as GPU reset doesn't need to roll
back restoring partial evicted queues.

Signed-off-by: Eric Huang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h |  4 ++--
 drivers/gpu/drm/amd/amdkfd/kfd_device.c| 10 +-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h  |  4 ++--
 drivers/gpu/drm/amd/amdkfd/kfd_process.c   | 18 ++
 5 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 77044e8ba4e6..59ce5a17a834 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -190,7 +190,7 @@ void amdgpu_amdkfd_interrupt(struct amdgpu_device *adev,
 void amdgpu_amdkfd_suspend(struct amdgpu_device *adev, bool run_pm)
 {
if (adev->kfd.dev)
-   kgd2kfd_suspend(adev->kfd.dev, run_pm);
+   kgd2kfd_suspend(adev->kfd.dev, run_pm, false);
 }
 
 int amdgpu_amdkfd_resume(struct amdgpu_device *adev, bool run_pm, bool sync)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 332ccba00e69..b7e46ad0507e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -372,7 +372,7 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
 struct drm_device *ddev,
 const struct kgd2kfd_shared_resources *gpu_resources);
 void kgd2kfd_device_exit(struct kfd_dev *kfd);
-void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm);
+void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm, bool force);
 int kgd2kfd_resume(struct kfd_dev *kfd, bool run_pm, bool sync);
 int kgd2kfd_pre_reset(struct kfd_dev *kfd);
 int kgd2kfd_post_reset(struct kfd_dev *kfd);
@@ -407,7 +407,7 @@ static inline void kgd2kfd_device_exit(struct kfd_dev *kfd)
 {
 }
 
-static inline void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm)
+static inline void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm, bool 
force)
 {
 }
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 24b5e0aa1eac..48e51ee8de56 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -940,7 +940,7 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
 void kgd2kfd_device_exit(struct kfd_dev *kfd)
 {
if (kfd->init_complete) {
-   kgd2kfd_suspend(kfd, false);
+   kgd2kfd_suspend(kfd, false, true);
svm_migrate_fini((struct amdgpu_device *)kfd->kgd);
device_queue_manager_uninit(kfd->dqm);
kfd_interrupt_exit(kfd);
@@ -965,7 +965,7 @@ int kgd2kfd_pre_reset(struct kfd_dev *kfd)
 
kfd->dqm->ops.pre_reset(kfd->dqm);
 
-   kgd2kfd_suspend(kfd, false);
+   kgd2kfd_suspend(kfd, false, true);
 
kfd_signal_reset_event(kfd);
return 0;
@@ -1001,7 +1001,7 @@ bool kfd_is_locked(void)
return  (atomic_read(_locked) > 0);
 }
 
-void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm)
+void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm, bool force)
 {
if (!kfd->init_complete)
return;
@@ -1010,7 +1010,7 @@ void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm)
if (!run_pm) {
/* For first KFD device suspend all the KFD processes */
if (atomic_inc_return(_locked) == 1)
-   kfd_suspend_all_processes();
+   kfd_suspend_all_processes(force);
}
 
kfd->dqm->ops.stop(kfd->dqm);
@@ -1122,7 +1122,7 @@ int kgd2kfd_quiesce_mm(struct mm_struct *mm)
return -ESRCH;
 
WARN(debug_evictions, "Evicting pid %d", p->lead_thread->pid);
-   r = kfd_process_evict_queues(p);
+   r = kfd_process_evict_queues(p, true);
 
kfd_unref_process(p);
return r;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 3d5d3994d8a4..e80fb64a6dcc 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1042,9 +1042,9 @@ static inline struct kfd_process_device 
*kfd_process_device_from_gpuidx(
 }
 
 void kfd_unref_process(struct kfd_process *p);
-int kfd_process_evict_queues(struct kfd_process *p);
+int kfd_process_evict_queues(struct kfd_process *p, bool force);
 int kfd_process_restore_queues(struct kfd_process *p);
-void kfd_suspend_all_processes(void);
+void kfd_suspend_all_processes(bool force);
 /*
  * kfd_resume_all_processes:
  * bool sync: If kfd_resume_all_processes() should wait for the
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 38a9dee40785..a41ece37bc3c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ 

RE: [PATCH v2 00/14] drm: Make DRM's IRQ helpers legacy

[Chrisanthus, Anitha]

Hi Thomas,
Can you please hold off on applying the kmb patch, I am seeing some issues 
while testing. Modetest works, but video playback only plays once, and it fails 
the second time with this patch.

Thanks,
Anitha


> -Original Message-
> From: Sam Ravnborg 
> Sent: Tuesday, August 3, 2021 8:05 AM
> To: Thomas Zimmermann 
> Cc: dan...@ffwll.ch; airl...@linux.ie; alexander.deuc...@amd.com;
> christian.koe...@amd.com; liviu.du...@arm.com; brian.star...@arm.com;
> bbrezil...@kernel.org; nicolas.fe...@microchip.com;
> maarten.lankho...@linux.intel.com; mrip...@kernel.org; ste...@agner.ch;
> alison.w...@nxp.com; patrik.r.jakobs...@gmail.com; Chrisanthus, Anitha
> ; robdcl...@gmail.com; Dea, Edmund J
> ; s...@poorly.run; shawn...@kernel.org;
> s.ha...@pengutronix.de; ker...@pengutronix.de; jyri.sa...@iki.fi;
> to...@kernel.org; dan.sned...@microchip.com;
> tomi.valkei...@ideasonboard.com; amd-gfx@lists.freedesktop.org; dri-
> de...@lists.freedesktop.org; linux-arm-ker...@lists.infradead.org; linux-arm-
> m...@vger.kernel.org; freedr...@lists.freedesktop.org
> Subject: Re: [PATCH v2 00/14] drm: Make DRM's IRQ helpers legacy
> 
> Hi Thomas,
> 
> On Tue, Aug 03, 2021 at 11:06:50AM +0200, Thomas Zimmermann wrote:
> > DRM's IRQ helpers are only helpful for old, non-KMS drivers. Move
> > the code behind CONFIG_DRM_LEGACY. Convert KMS drivers to Linux
> > IRQ interfaces.
> >
> > DRM provides IRQ helpers for setting up, receiving and removing IRQ
> > handlers. It's an abstraction over plain Linux functions. The code
> > is mid-layerish with several callbacks to hook into the rsp drivers.
> > Old UMS driver have their interrupts enabled via ioctl, so these
> > abstractions makes some sense. Modern KMS manage all their interrupts
> > internally. Using the DRM helpers adds indirection without benefits.
> >
> > Most KMS drivers already use Linux IRQ functions instead of DRM's
> > abstraction layer. Patches 1 to 12 convert the remaining ones.
> > The patches also resolve a bug for devices without assigned interrupt
> > number. DRM helpers don't test for IRQ_NOTCONNECTED, so drivers do
> > not detect if the device has no interrupt assigned.
> >
> > Patch 13 removes an unused function.
> >
> > Patch 14 moves the DRM IRQ helpers behind CONFIG_DRM_LEGACY. Only
> > the old non-KMS drivers still use the functionality.
> >
> > v2:
> > * drop IRQ_NOTCONNECTED test from atmel-hlcdc (Sam)
> > * use devm_request_irq() in atmel-hlcdc (Sam)
> > * unify variable names in arm/hlcdc (Sam)
> >
> > Thomas Zimmermann (14):
> 
> The following patches are all:
> Acked-by: Sam Ravnborg 
> 
> >   drm/fsl-dcu: Convert to Linux IRQ interfaces
> >   drm/gma500: Convert to Linux IRQ interfaces
> >   drm/kmb: Convert to Linux IRQ interfaces
> >   drm/msm: Convert to Linux IRQ interfaces
> >   drm/mxsfb: Convert to Linux IRQ interfaces
> >   drm/tidss: Convert to Linux IRQ interfaces
> >   drm/vc4: Convert to Linux IRQ interfaces
> >   drm: Remove unused devm_drm_irq_install()
> 
> The remaining patches I either skipped or already had a feedback from
> me or I asked a question.
> 
>   Sam

Re: [Freedreno] [PATCH v2 07/14] drm/msm: Convert to Linux IRQ interfaces

[abhinavk]

On 2021-08-03 02:06, Thomas Zimmermann wrote:

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.

DRM IRQ callbacks are now being called directly or inlined.

Signed-off-by: Thomas Zimmermann 

Reviewed-by: Abhinav Kumar 

---
 drivers/gpu/drm/msm/msm_drv.c | 113 --
 drivers/gpu/drm/msm/msm_kms.h |   2 +-
 2 files changed, 69 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c 
b/drivers/gpu/drm/msm/msm_drv.c

index 1594ae39d54f..a332b09a5a11 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -14,7 +14,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -201,6 +200,71 @@ void msm_rmw(void __iomem *addr, u32 mask, u32 or)
msm_writel(val | or, addr);
 }

+static irqreturn_t msm_irq(int irq, void *arg)
+{
+   struct drm_device *dev = arg;
+   struct msm_drm_private *priv = dev->dev_private;
+   struct msm_kms *kms = priv->kms;
+
+   BUG_ON(!kms);
+
+   return kms->funcs->irq(kms);
+}
+
+static void msm_irq_preinstall(struct drm_device *dev)
+{
+   struct msm_drm_private *priv = dev->dev_private;
+   struct msm_kms *kms = priv->kms;
+
+   BUG_ON(!kms);
+
+   kms->funcs->irq_preinstall(kms);
+}
+
+static int msm_irq_postinstall(struct drm_device *dev)
+{
+   struct msm_drm_private *priv = dev->dev_private;
+   struct msm_kms *kms = priv->kms;
+
+   BUG_ON(!kms);
+
+   if (kms->funcs->irq_postinstall)
+   return kms->funcs->irq_postinstall(kms);
+
+   return 0;
+}
+
+static int msm_irq_install(struct drm_device *dev, unsigned int irq)
+{
+   int ret;
+
+   if (irq == IRQ_NOTCONNECTED)
+   return -ENOTCONN;
+
+   msm_irq_preinstall(dev);
+
+   ret = request_irq(irq, msm_irq, 0, dev->driver->name, dev);
+   if (ret)
+   return ret;
+
+   ret = msm_irq_postinstall(dev);
+   if (ret) {
+   free_irq(irq, dev);
+   return ret;
+   }
+
+   return 0;
+}
+
+static void msm_irq_uninstall(struct drm_device *dev)
+{
+   struct msm_drm_private *priv = dev->dev_private;
+   struct msm_kms *kms = priv->kms;
+
+   kms->funcs->irq_uninstall(kms);
+   free_irq(kms->irq, dev);
+}
+
 struct msm_vblank_work {
struct work_struct work;
int crtc_id;
@@ -265,7 +329,7 @@ static int msm_drm_uninit(struct device *dev)
}

/* We must cancel and cleanup any pending vblank enable/disable
-* work before drm_irq_uninstall() to avoid work re-enabling an
+* work before msm_irq_uninstall() to avoid work re-enabling an
 * irq after uninstall has disabled it.
 */

@@ -294,7 +358,7 @@ static int msm_drm_uninit(struct device *dev)
drm_mode_config_cleanup(ddev);

pm_runtime_get_sync(dev);
-   drm_irq_uninstall(ddev);
+   msm_irq_uninstall(ddev);
pm_runtime_put_sync(dev);

if (kms && kms->funcs)
@@ -553,7 +617,7 @@ static int msm_drm_init(struct device *dev, const
struct drm_driver *drv)

if (kms) {
pm_runtime_get_sync(dev);
-   ret = drm_irq_install(ddev, kms->irq);
+   ret = msm_irq_install(ddev, kms->irq);
pm_runtime_put_sync(dev);
if (ret < 0) {
DRM_DEV_ERROR(dev, "failed to install IRQ handler\n");
@@ -662,43 +726,6 @@ static void msm_postclose(struct drm_device *dev,
struct drm_file *file)
context_close(ctx);
 }

-static irqreturn_t msm_irq(int irq, void *arg)
-{
-   struct drm_device *dev = arg;
-   struct msm_drm_private *priv = dev->dev_private;
-   struct msm_kms *kms = priv->kms;
-   BUG_ON(!kms);
-   return kms->funcs->irq(kms);
-}
-
-static void msm_irq_preinstall(struct drm_device *dev)
-{
-   struct msm_drm_private *priv = dev->dev_private;
-   struct msm_kms *kms = priv->kms;
-   BUG_ON(!kms);
-   kms->funcs->irq_preinstall(kms);
-}
-
-static int msm_irq_postinstall(struct drm_device *dev)
-{
-   struct msm_drm_private *priv = dev->dev_private;
-   struct msm_kms *kms = priv->kms;
-   BUG_ON(!kms);
-
-   if (kms->funcs->irq_postinstall)
-   return kms->funcs->irq_postinstall(kms);
-
-   return 0;
-}
-
-static void msm_irq_uninstall(struct drm_device *dev)
-{
-   struct msm_drm_private *priv = dev->dev_private;
-   struct msm_kms *kms = priv->kms;
-   BUG_ON(!kms);
-   kms->funcs->irq_uninstall(kms);
-}
-
 int msm_crtc_enable_vblank(struct drm_crtc *crtc)
 {
struct drm_device *dev = crtc->dev;
@@ -1051,10 +1078,6 @@ static const struct drm_driver msm_driver = {
.open   = msm_open,
.postclose   = msm_postclose,
.lastclose  = drm_fb_helper_lastclose,
-   .irq_handler= msm_irq,

Re: [PATCH] drm/amdkfd: support file backed SVM range

[Felix Kuehling]

Am 2021-08-02 um 10:56 a.m. schrieb Philip Yang:
> HMM migrate helper migrate_vma_pages do not migrate file backed pages to
> replace it with device pages because the pages are used by file cache.
> We can not migrate the file backed range to VRAM, otherwise CPU access
> range will not trigger page fault to migrate updated data from VRAM back
> to system memory.
>
> For file backed range, don't prefetch migrate range to VRAM, always map
> system pages to GPU and also use system pages to recover GPU retry
> fault.
>
> Add helper to check if range is file backed or anonymous mapping.
>
> Signed-off-by: Philip Yang 

This patch should not be submitted to amd-staging-drm-next. As I
understand it, the real fix is in Alex's partial-migration patch series.
This patch is a hack that could be useful for older release branches
that won't get Alex's patch series because it's too invasive. So let's
review this on the internal mailing list.

Thanks,
  Felix


> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 40 
>  1 file changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index f811a3a24cd2..69237d2ab2ad 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -2400,6 +2400,36 @@ svm_range_count_fault(struct amdgpu_device *adev, 
> struct kfd_process *p,
>   WRITE_ONCE(pdd->faults, pdd->faults + 1);
>  }
>  
> +/**
> + * svm_range_is_file_backed - decide if prange is file backed mmap
> + * @mm: the mm structure
> + * @prange: svm range structure
> + *
> + * Context: caller must hold mmap_read_lock
> + *
> + * Return:
> + * false if entire range is anonymous mapping
> + * true if entire or partial range is file backed, or invalid mapping address
> + */
> +static bool
> +svm_range_is_file_backed(struct mm_struct *mm, struct svm_range *prange)
> +{
> + struct vm_area_struct *vma;
> + unsigned long start, end;
> +
> + start = prange->start << PAGE_SHIFT;
> + end = (prange->last + 1) << PAGE_SHIFT;
> +
> + do {
> + vma = find_vma(mm, start);
> + if (!vma || !vma_is_anonymous(vma))
> + return true;
> + start = min(end, vma->vm_end);
> + } while (start < end);
> +
> + return false;
> +}
> +
>  int
>  svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
>   uint64_t addr)
> @@ -2496,6 +2526,10 @@ svm_range_restore_pages(struct amdgpu_device *adev, 
> unsigned int pasid,
>svms, prange->start, prange->last, best_loc,
>prange->actual_loc);
>  
> + /* for file backed range, use system memory pages for GPU mapping */
> + if (svm_range_is_file_backed(mm, prange))
> + goto out_validate_and_map;
> +
>   if (prange->actual_loc != best_loc) {
>   if (best_loc) {
>   r = svm_migrate_to_vram(prange, best_loc, mm);
> @@ -2520,6 +2554,7 @@ svm_range_restore_pages(struct amdgpu_device *adev, 
> unsigned int pasid,
>   }
>   }
>  
> +out_validate_and_map:
>   r = svm_range_validate_and_map(mm, prange, gpuidx, false, false);
>   if (r)
>   pr_debug("failed %d to map svms 0x%p [0x%lx 0x%lx] to gpus\n",
> @@ -2850,6 +2885,11 @@ svm_range_trigger_migration(struct mm_struct *mm, 
> struct svm_range *prange,
>   int r = 0;
>  
>   *migrated = false;
> +
> + /* Don't migrate file backed range to VRAM */
> + if (svm_range_is_file_backed(mm, prange))
> + return 0;
> +
>   best_loc = svm_range_best_prefetch_location(prange);
>  
>   if (best_loc == KFD_IOCTL_SVM_LOCATION_UNDEFINED ||

Re: [PATCH] drm/amdgpu: Fix vm free pts race when process exiting

[Felix Kuehling]

Am 2021-08-02 um 6:33 p.m. schrieb Philip Yang:
> Take vm->invalidated_lock spinlock to remove vm pd and pt bos, to avoid
> link list corruption with crash backtrace:
>
> [ 2290.505111] list_del corruption. next->prev should be
>  9b2514ec0018, but was 4e03280211010f04
> [ 2290.505154] kernel BUG at lib/list_debug.c:56!
> [ 2290.505176] invalid opcode:  [#1] SMP NOPTI
> [ 2290.505254] Workqueue: events delayed_fput
> [ 2290.505271] RIP: 0010:__list_del_entry_valid.cold.1+0x20/0x4c
> [ 2290.505513] Call Trace:
> [ 2290.505623]  amdgpu_vm_free_table+0x26/0x80 [amdgpu]
> [ 2290.505705]  amdgpu_vm_free_pts+0x7a/0xf0 [amdgpu]
> [ 2290.505786]  amdgpu_vm_fini+0x1f0/0x440 [amdgpu]
> [ 2290.505864]  amdgpu_driver_postclose_kms+0x172/0x290 [amdgpu]
> [ 2290.505893]  drm_file_free.part.10+0x1d4/0x270 [drm]
> [ 2290.505916]  drm_release+0xa9/0xe0 [drm]
> [ 2290.505930]  __fput+0xb7/0x230
> [ 2290.505942]  delayed_fput+0x1c/0x30
> [ 2290.505957]  process_one_work+0x1a7/0x360
> [ 2290.505971]  worker_thread+0x30/0x390
> [ 2290.505985]  ? create_worker+0x1a0/0x1a0
> [ 2290.505999]  kthread+0x112/0x130
> [ 2290.506011]  ? kthread_flush_work_fn+0x10/0x10
> [ 2290.506027]  ret_from_fork+0x22/0x40
>
> Signed-off-by: Philip Yang 

The patch is

Acked-by: Felix Kuehling 


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 2a88ed5d983b..5c4c355e7d6b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1045,7 +1045,9 @@ static void amdgpu_vm_free_table(struct 
> amdgpu_vm_bo_base *entry)
>   return;
>   shadow = amdgpu_bo_shadowed(entry->bo);
>   entry->bo->vm_bo = NULL;
> + spin_lock(>vm->invalidated_lock);
>   list_del(>vm_status);
> + spin_unlock(>vm->invalidated_lock);
>   amdgpu_bo_unref();
>   amdgpu_bo_unref(>bo);
>  }

Re: [PATCH v2] drm/amdgpu: fix fdinfo race with process exit

[Felix Kuehling]

Am 2021-07-29 um 10:13 p.m. schrieb Philip Yang:
> Get process vm root BO ref in case process is exiting and root BO is
> freed, to avoid NULL pointer dereference backtrace:
>
> BUG: unable to handle kernel NULL pointer dereference at
> 
> Call Trace:
> amdgpu_show_fdinfo+0xfe/0x2a0 [amdgpu]
> seq_show+0x12c/0x180
> seq_read+0x153/0x410
> vfs_read+0x91/0x140[ 3427.206183]  ksys_read+0x4f/0xb0
> do_syscall_64+0x5b/0x1a0
> entry_SYSCALL_64_after_hwframe+0x65/0xca
>
> v2: rebase to staging
>
> Signed-off-by: Philip Yang 

The patch is

Reviewed-by: Felix Kuehling 


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
> index d94c5419ec25..5a6857c44bb6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
> @@ -59,6 +59,7 @@ void amdgpu_show_fdinfo(struct seq_file *m, struct file *f)
>   uint64_t vram_mem = 0, gtt_mem = 0, cpu_mem = 0;
>   struct drm_file *file = f->private_data;
>   struct amdgpu_device *adev = drm_to_adev(file->minor->dev);
> + struct amdgpu_bo *root;
>   int ret;
>  
>   ret = amdgpu_file_to_fpriv(f, );
> @@ -69,13 +70,19 @@ void amdgpu_show_fdinfo(struct seq_file *m, struct file 
> *f)
>   dev = PCI_SLOT(adev->pdev->devfn);
>   fn = PCI_FUNC(adev->pdev->devfn);
>  
> - ret = amdgpu_bo_reserve(fpriv->vm.root.bo, false);
> + root = amdgpu_bo_ref(fpriv->vm.root.bo);
> + if (!root)
> + return;
> +
> + ret = amdgpu_bo_reserve(root, false);
>   if (ret) {
>   DRM_ERROR("Fail to reserve bo\n");
>   return;
>   }
>   amdgpu_vm_get_memory(>vm, _mem, _mem, _mem);
> - amdgpu_bo_unreserve(fpriv->vm.root.bo);
> + amdgpu_bo_unreserve(root);
> + amdgpu_bo_unref();
> +
>   seq_printf(m, "pdev:\t%04x:%02x:%02x.%d\npasid:\t%u\n", domain, bus,
>   dev, fn, fpriv->vm.pasid);
>   seq_printf(m, "vram mem:\t%llu kB\n", vram_mem/1024UL);

Re: [PATCH] drm/amdkfd: Restore all process on post reset

[Felix Kuehling]

Am 2021-08-03 um 11:02 a.m. schrieb Eric Huang:
>
>
> On 2021-07-30 5:26 p.m., Felix Kuehling wrote:
>> Am 2021-07-28 um 1:31 p.m. schrieb Eric Huang:
>>> It is to fix a bug of gpu_recovery on multiple GPUs,
>>> When one gpu is reset, the application running on other
>>> gpu hangs, because kfd post reset doesn't restore the
>>> running process.
>> This will resume all processes, even those that were affected by the GPU
>> reset.
>>
>> The assumption we made here is, that KFD processes can use all GPUs. So
>> when one GPU is reset, all processes are affected. If we want to refine
>> that, we'll need some more complex logic to identify the processes
>> affected by a GPU reset and keep only those processes suspended. This
>> could be based on the GPUs that a process has created queues on, or
>> allocated memory on.
>>
>> What we don't want, is processes continuing with bad data or
>> inconsistent state after a GPU reset.
>>
> Current code doesn't take care of this assumption. When a GPU hangs,
> evicting queues will fail on it and roll back to restore all processes
> queues on other GPUs, and continue to running with unclear state and
> data after a GPU reset.

That's a bug.


>
> The original thought about this patch is to call
> kfd_suspend_all_processes and kfd_restore_all_processes in pair on
> pre_reset and post_reset. And It keeps the consistent behavior for
> both amdgpu_gpu_recover and hang_hws.

This is true for suspend/resume, but not for GPU reset. After a GPU
reset we want queues of affected processes to stay evicted.


>>>   And it also fixes a bug in the function
>>> kfd_process_evict_queues, when one gpu hangs, process
>>> running on other gpus can't be evicted.
>> This should be a separate patch. The code you're removing was there as
>> an attempt to make kfd_process_evict_queues transactional. That means,
>> it either succeeds completely or it fails completely. I wanted to avoid
>> putting the system into an unknown state where some queues are suspended
>> and others are not and the caller has no idea how to proceed. So I roll
>> back a partial queue eviction if something failed.
> Can we let the caller to decide if roll-back is needed? because no all
> the callers need to roll back, e.g. kfd_suspend_all_processes and
> kgd2kfd_quiesce_mm.

I don't think so. If suspend partially succeeds, how can the caller do a
partial roll back? The caller doesn't have the information about what
succeeded and what failed.

But you have a good point. It may depend on the caller whether a
rollback is needed or not. In the case of GPU reset, we don't care so
much about the HW state, but we do care about the queue eviction state.
That must be updated in order to prevent queues to resume after the
reset, no matter what else fails. This could be done by adding a "bool
force" flag to kfd_suspend_all_processes and kfd_process_evict_queues.
If the flag is set we don't roll back and we keep going to suspend
everything even after partial failures.

I think kfd_suspend_all_processes should return an error if force=false
and some error occurred so that kgd2kfd_suspend can return a failure. In
the case of suspend/resume that can stop the suspend process.


>>
>> Your changing this to "try to evict as much as possible". Then a failure
>> does not mean "eviction failed" but "eviction completed but something
>> hung". Then the GPU reset can take care of the hanging part. I think
>> that's a reasonable strategy. But we need to be sure that there are no
>> other error conditions (other than hangs) that could cause a queue
>> eviction to fail.
>>
>> There were some recent changes in pqm_destroy_queue that check the
>> return value of dqm->ops.destroy_queue, which indirectly comes from
>> execute_queues (sam as in the eviction case). -ETIME means a hang. Other
>> errors are possible. Those other errors may still need the roll-back.
>> Otherwise we'd leave stuff running on a non-hanging GPU after we
>> promised that we evicted everything.
> I think it depends on case scenario. GPU reset doesn't need to know
> the return state. Memory eviction may need. Does Memory notifier
> invalidate range need?

You're right in the sense, that for a GPU reset we don't care about the
HW state because we're about to reset it anyway. But we do care about
the queue eviction state of the queues (q->properties.is_evicted). After
the reset completes we don't want the queues of affected processes to be
in evicted state so they are not resumed on the hardware.

In the MMU notifier case (kgd2kfd_quiesce_mm) we do care about the HW
state. When the MMU notifier returns to the caller, it promises that the
HW will no longer access the memory. If the GPU is hanging, that's
probably OK as far as the MMU notifier cares. So this is another case
where we don't want to roll back. Other error cases (not GPU hangs)
would be problematic.

Regards,
  Felix


>>
>> See one more comment inline.
>>
>>
>>> Signed-off-by: Eric Huang 
>>> ---
>>>   

Issue with USB-C lanes or DSC / bandwidth on amdgpu

[Thomas Gfeller]

Hi everyone.

I currently try to go all-in on Linux. Therefore I have one last issue that
I still didn't manage to resolve:

https://gitlab.freedesktop.org/drm/amd/-/issues/1616

I found that you guys implemented parts of the mentioned features here:
https://lists.freedesktop.org/archives/amd-gfx/2020-January/044739.html

I suspect, that there is an issue with bandwidth allocation for DP alt mode
(on USB-C) or DSC / HBR3 or both.

Any help to get this fixed is highly appreciated. Unfortunately I have no
experience in developing drivers for Linux - this is pretty advanced stuff.
Thank you very much for your contributions!

Best

Thomas

Re: [PATCH v2 00/14] drm: Make DRM's IRQ helpers legacy

[Sam Ravnborg]

Hi Thomas,

On Tue, Aug 03, 2021 at 11:06:50AM +0200, Thomas Zimmermann wrote:
> DRM's IRQ helpers are only helpful for old, non-KMS drivers. Move
> the code behind CONFIG_DRM_LEGACY. Convert KMS drivers to Linux
> IRQ interfaces.
> 
> DRM provides IRQ helpers for setting up, receiving and removing IRQ
> handlers. It's an abstraction over plain Linux functions. The code
> is mid-layerish with several callbacks to hook into the rsp drivers.
> Old UMS driver have their interrupts enabled via ioctl, so these
> abstractions makes some sense. Modern KMS manage all their interrupts
> internally. Using the DRM helpers adds indirection without benefits.
> 
> Most KMS drivers already use Linux IRQ functions instead of DRM's
> abstraction layer. Patches 1 to 12 convert the remaining ones.
> The patches also resolve a bug for devices without assigned interrupt
> number. DRM helpers don't test for IRQ_NOTCONNECTED, so drivers do
> not detect if the device has no interrupt assigned.
> 
> Patch 13 removes an unused function.
> 
> Patch 14 moves the DRM IRQ helpers behind CONFIG_DRM_LEGACY. Only
> the old non-KMS drivers still use the functionality.
> 
> v2:
>   * drop IRQ_NOTCONNECTED test from atmel-hlcdc (Sam)
>   * use devm_request_irq() in atmel-hlcdc (Sam)
>   * unify variable names in arm/hlcdc (Sam)
> 
> Thomas Zimmermann (14):

The following patches are all:
Acked-by: Sam Ravnborg 

>   drm/fsl-dcu: Convert to Linux IRQ interfaces
>   drm/gma500: Convert to Linux IRQ interfaces
>   drm/kmb: Convert to Linux IRQ interfaces
>   drm/msm: Convert to Linux IRQ interfaces
>   drm/mxsfb: Convert to Linux IRQ interfaces
>   drm/tidss: Convert to Linux IRQ interfaces
>   drm/vc4: Convert to Linux IRQ interfaces
>   drm: Remove unused devm_drm_irq_install()

The remaining patches I either skipped or already had a feedback from
me or I asked a question.

Sam

Re: [PATCH] drm/amdkfd: Restore all process on post reset

[Eric Huang]

On 2021-07-30 5:26 p.m., Felix Kuehling wrote:

Am 2021-07-28 um 1:31 p.m. schrieb Eric Huang:

It is to fix a bug of gpu_recovery on multiple GPUs,
When one gpu is reset, the application running on other
gpu hangs, because kfd post reset doesn't restore the
running process.

This will resume all processes, even those that were affected by the GPU
reset.

The assumption we made here is, that KFD processes can use all GPUs. So
when one GPU is reset, all processes are affected. If we want to refine
that, we'll need some more complex logic to identify the processes
affected by a GPU reset and keep only those processes suspended. This
could be based on the GPUs that a process has created queues on, or
allocated memory on.

What we don't want, is processes continuing with bad data or
inconsistent state after a GPU reset.

Current code doesn't take care of this assumption. When a GPU hangs, 
evicting queues will fail on it and roll back to restore all processes 
queues on other GPUs, and continue to running with unclear state and 
data after a GPU reset.


The original thought about this patch is to call 
kfd_suspend_all_processes and kfd_restore_all_processes in pair on 
pre_reset and post_reset. And It keeps the consistent behavior for both 
amdgpu_gpu_recover and hang_hws.

  And it also fixes a bug in the function
kfd_process_evict_queues, when one gpu hangs, process
running on other gpus can't be evicted.

This should be a separate patch. The code you're removing was there as
an attempt to make kfd_process_evict_queues transactional. That means,
it either succeeds completely or it fails completely. I wanted to avoid
putting the system into an unknown state where some queues are suspended
and others are not and the caller has no idea how to proceed. So I roll
back a partial queue eviction if something failed.
Can we let the caller to decide if roll-back is needed? because no all 
the callers need to roll back, e.g. kfd_suspend_all_processes and 
kgd2kfd_quiesce_mm.


Your changing this to "try to evict as much as possible". Then a failure
does not mean "eviction failed" but "eviction completed but something
hung". Then the GPU reset can take care of the hanging part. I think
that's a reasonable strategy. But we need to be sure that there are no
other error conditions (other than hangs) that could cause a queue
eviction to fail.

There were some recent changes in pqm_destroy_queue that check the
return value of dqm->ops.destroy_queue, which indirectly comes from
execute_queues (sam as in the eviction case). -ETIME means a hang. Other
errors are possible. Those other errors may still need the roll-back.
Otherwise we'd leave stuff running on a non-hanging GPU after we
promised that we evicted everything.
I think it depends on case scenario. GPU reset doesn't need to know the 
return state. Memory eviction may need. Does Memory notifier invalidate 
range need?


See one more comment inline.



Signed-off-by: Eric Huang 
---
  drivers/gpu/drm/amd/amdkfd/kfd_device.c  |  2 +-
  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 24 +---
  2 files changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 24b5e0aa1eac..daf1c19bd799 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -984,7 +984,7 @@ int kgd2kfd_post_reset(struct kfd_dev *kfd)
if (!kfd->init_complete)
return 0;
  
-	ret = kfd_resume(kfd);

+   ret = kgd2kfd_resume(kfd, false, true);

Which branch is this for? On amd-staging-drm-next kgd2kfd_resume only
has two parameters.
Sorry, it is based on dkms staging 5.11. I didn't notice there is 
difference between two branches.


Regards,
Eric


Regards,
   Felix



if (ret)
return ret;
atomic_dec(_locked);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 38a9dee40785..9272a12c1db8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1879,36 +1879,14 @@ int kfd_process_evict_queues(struct kfd_process *p)
  {
int r = 0;
int i;
-   unsigned int n_evicted = 0;
  
  	for (i = 0; i < p->n_pdds; i++) {

struct kfd_process_device *pdd = p->pdds[i];
  
  		r = pdd->dev->dqm->ops.evict_process_queues(pdd->dev->dqm,

>qpd);
-   if (r) {
+   if (r)
pr_err("Failed to evict process queues\n");
-   goto fail;
-   }
-   n_evicted++;
-   }
-
-   return r;
-
-fail:
-   /* To keep state consistent, roll back partial eviction by
-* restoring queues
-*/
-   for (i = 0; i < p->n_pdds; i++) {
-   struct kfd_process_device *pdd = p->pdds[i];
-
-   if (n_evicted == 0)
-  

Re: [PATCH v2 11/14] drm/tilcdc: Convert to Linux IRQ interfaces

[Sam Ravnborg]

Hi Thomas,

On Tue, Aug 03, 2021 at 11:07:01AM +0200, Thomas Zimmermann wrote:
> Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
> IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
> don't benefit from using it.
> 
> DRM IRQ callbacks are now being called directly or inlined.
> 
> Calls to platform_get_irq() can fail with a negative errno code.
> Abort initialization in this case. The DRM IRQ midlayer does not
> handle this case correctly.

I cannot see why the irq_enabled flag is needed here, and the changelog
do not help me.
What do I miss?

Sam


> 
> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c | 51 ++---
>  drivers/gpu/drm/tilcdc/tilcdc_drv.h |  3 ++
>  2 files changed, 43 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c 
> b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index f1d3a9f919fd..6b03f89a98d4 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -20,7 +20,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -124,6 +123,39 @@ static int cpufreq_transition(struct notifier_block *nb,
>  }
>  #endif
>  
> +static irqreturn_t tilcdc_irq(int irq, void *arg)
> +{
> + struct drm_device *dev = arg;
> + struct tilcdc_drm_private *priv = dev->dev_private;
> +
> + return tilcdc_crtc_irq(priv->crtc);
> +}
> +
> +static int tilcdc_irq_install(struct drm_device *dev, unsigned int irq)
> +{
> + struct tilcdc_drm_private *priv = dev->dev_private;
> + int ret;
> +
> + ret = request_irq(irq, tilcdc_irq, 0, dev->driver->name, dev);
> + if (ret)
> + return ret;
> +
> + priv->irq_enabled = false;
> +
> + return 0;
> +}
> +
> +static void tilcdc_irq_uninstall(struct drm_device *dev)
> +{
> + struct tilcdc_drm_private *priv = dev->dev_private;
> +
> + if (!priv->irq_enabled)
> + return;
> +
> + free_irq(priv->irq, dev);
> + priv->irq_enabled = false;
> +}
> +
>  /*
>   * DRM operations:
>   */
> @@ -145,7 +177,7 @@ static void tilcdc_fini(struct drm_device *dev)
>   drm_dev_unregister(dev);
>  
>   drm_kms_helper_poll_fini(dev);
> - drm_irq_uninstall(dev);
> + tilcdc_irq_uninstall(dev);
>   drm_mode_config_cleanup(dev);
>  
>   if (priv->clk)
> @@ -336,7 +368,12 @@ static int tilcdc_init(const struct drm_driver *ddrv, 
> struct device *dev)
>   goto init_failed;
>   }
>  
> - ret = drm_irq_install(ddev, platform_get_irq(pdev, 0));
> + ret = platform_get_irq(pdev, 0);
> + if (ret < 0)
> + goto init_failed;
> + priv->irq = ret;
> +
> + ret = tilcdc_irq_install(ddev, priv->irq);
>   if (ret < 0) {
>   dev_err(dev, "failed to install IRQ handler\n");
>   goto init_failed;
> @@ -360,13 +397,6 @@ static int tilcdc_init(const struct drm_driver *ddrv, 
> struct device *dev)
>   return ret;
>  }
>  
> -static irqreturn_t tilcdc_irq(int irq, void *arg)
> -{
> - struct drm_device *dev = arg;
> - struct tilcdc_drm_private *priv = dev->dev_private;
> - return tilcdc_crtc_irq(priv->crtc);
> -}
> -
>  #if defined(CONFIG_DEBUG_FS)
>  static const struct {
>   const char *name;
> @@ -454,7 +484,6 @@ DEFINE_DRM_GEM_CMA_FOPS(fops);
>  
>  static const struct drm_driver tilcdc_driver = {
>   .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> - .irq_handler= tilcdc_irq,
>   DRM_GEM_CMA_DRIVER_OPS,
>  #ifdef CONFIG_DEBUG_FS
>   .debugfs_init   = tilcdc_debugfs_init,
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h 
> b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> index d29806ca8817..b818448c83f6 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> @@ -46,6 +46,8 @@ struct tilcdc_drm_private {
>   struct clk *clk; /* functional clock */
>   int rev; /* IP revision */
>  
> + unsigned int irq;
> +
>   /* don't attempt resolutions w/ higher W * H * Hz: */
>   uint32_t max_bandwidth;
>   /*
> @@ -82,6 +84,7 @@ struct tilcdc_drm_private {
>  
>   bool is_registered;
>   bool is_componentized;
> + bool irq_enabled;
>  };
>  
>  /* Sub-module for display.  Since we don't know at compile time what panels
> -- 
> 2.32.0

Re: [PATCH v2 07/14] drm/msm: Convert to Linux IRQ interfaces

[Rob Clark]

On Tue, Aug 3, 2021 at 2:37 AM Dmitry Baryshkov
 wrote:
>
> On 03/08/2021 12:06, Thomas Zimmermann wrote:
> > Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
> > IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
> > don't benefit from using it.
> >
> > DRM IRQ callbacks are now being called directly or inlined.
> >
> > Signed-off-by: Thomas Zimmermann 
>
> Reviewed-by: Dmitry Baryshkov 
>
> Rob should probably also give his blessing on this patch though.

It looks ok.. I can't really test it this week, but it should be
pretty obvious if it wasn't working

BR,
-R

>
> > ---
> >   drivers/gpu/drm/msm/msm_drv.c | 113 --
> >   drivers/gpu/drm/msm/msm_kms.h |   2 +-
> >   2 files changed, 69 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > index 1594ae39d54f..a332b09a5a11 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > @@ -14,7 +14,6 @@
> >   #include 
> >   #include 
> >   #include 
> > -#include 
> >   #include 
> >   #include 
> >   #include 
> > @@ -201,6 +200,71 @@ void msm_rmw(void __iomem *addr, u32 mask, u32 or)
> >   msm_writel(val | or, addr);
> >   }
> >
> > +static irqreturn_t msm_irq(int irq, void *arg)
> > +{
> > + struct drm_device *dev = arg;
> > + struct msm_drm_private *priv = dev->dev_private;
> > + struct msm_kms *kms = priv->kms;
> > +
> > + BUG_ON(!kms);
> > +
> > + return kms->funcs->irq(kms);
> > +}
> > +
> > +static void msm_irq_preinstall(struct drm_device *dev)
> > +{
> > + struct msm_drm_private *priv = dev->dev_private;
> > + struct msm_kms *kms = priv->kms;
> > +
> > + BUG_ON(!kms);
> > +
> > + kms->funcs->irq_preinstall(kms);
> > +}
> > +
> > +static int msm_irq_postinstall(struct drm_device *dev)
> > +{
> > + struct msm_drm_private *priv = dev->dev_private;
> > + struct msm_kms *kms = priv->kms;
> > +
> > + BUG_ON(!kms);
> > +
> > + if (kms->funcs->irq_postinstall)
> > + return kms->funcs->irq_postinstall(kms);
> > +
> > + return 0;
> > +}
> > +
> > +static int msm_irq_install(struct drm_device *dev, unsigned int irq)
> > +{
> > + int ret;
> > +
> > + if (irq == IRQ_NOTCONNECTED)
> > + return -ENOTCONN;
> > +
> > + msm_irq_preinstall(dev);
> > +
> > + ret = request_irq(irq, msm_irq, 0, dev->driver->name, dev);
> > + if (ret)
> > + return ret;
> > +
> > + ret = msm_irq_postinstall(dev);
> > + if (ret) {
> > + free_irq(irq, dev);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void msm_irq_uninstall(struct drm_device *dev)
> > +{
> > + struct msm_drm_private *priv = dev->dev_private;
> > + struct msm_kms *kms = priv->kms;
> > +
> > + kms->funcs->irq_uninstall(kms);
> > + free_irq(kms->irq, dev);
> > +}
> > +
> >   struct msm_vblank_work {
> >   struct work_struct work;
> >   int crtc_id;
> > @@ -265,7 +329,7 @@ static int msm_drm_uninit(struct device *dev)
> >   }
> >
> >   /* We must cancel and cleanup any pending vblank enable/disable
> > -  * work before drm_irq_uninstall() to avoid work re-enabling an
> > +  * work before msm_irq_uninstall() to avoid work re-enabling an
> >* irq after uninstall has disabled it.
> >*/
> >
> > @@ -294,7 +358,7 @@ static int msm_drm_uninit(struct device *dev)
> >   drm_mode_config_cleanup(ddev);
> >
> >   pm_runtime_get_sync(dev);
> > - drm_irq_uninstall(ddev);
> > + msm_irq_uninstall(ddev);
> >   pm_runtime_put_sync(dev);
> >
> >   if (kms && kms->funcs)
> > @@ -553,7 +617,7 @@ static int msm_drm_init(struct device *dev, const 
> > struct drm_driver *drv)
> >
> >   if (kms) {
> >   pm_runtime_get_sync(dev);
> > - ret = drm_irq_install(ddev, kms->irq);
> > + ret = msm_irq_install(ddev, kms->irq);
> >   pm_runtime_put_sync(dev);
> >   if (ret < 0) {
> >   DRM_DEV_ERROR(dev, "failed to install IRQ handler\n");
> > @@ -662,43 +726,6 @@ static void msm_postclose(struct drm_device *dev, 
> > struct drm_file *file)
> >   context_close(ctx);
> >   }
> >
> > -static irqreturn_t msm_irq(int irq, void *arg)
> > -{
> > - struct drm_device *dev = arg;
> > - struct msm_drm_private *priv = dev->dev_private;
> > - struct msm_kms *kms = priv->kms;
> > - BUG_ON(!kms);
> > - return kms->funcs->irq(kms);
> > -}
> > -
> > -static void msm_irq_preinstall(struct drm_device *dev)
> > -{
> > - struct msm_drm_private *priv = dev->dev_private;
> > - struct msm_kms *kms = priv->kms;
> > - BUG_ON(!kms);
> > - kms->funcs->irq_preinstall(kms);
> > -}
> > -
> > -static int msm_irq_postinstall(struct drm_device *dev)
> > -{
> > - struct msm_drm_private *priv = dev->dev_private;
> > - struct msm_kms *kms = priv->kms;
> 

Re: [PATCH] drm/radeon: Update pitch for page flip

[Alex Deucher]

On Tue, Aug 3, 2021 at 4:34 AM Michel Dänzer  wrote:
>
> On 2021-08-02 4:51 p.m., Alex Deucher wrote:
> > On Mon, Aug 2, 2021 at 4:31 AM Daniel Vetter  wrote:
> >>
> >> On Mon, Aug 02, 2021 at 10:12:47AM +0200, Christian König wrote:
> >>> Am 02.08.21 um 09:43 schrieb Zhenneng Li:
>  When primary bo is updated, crtc's pitch may
>  have not been updated, this will lead to show
>  disorder content when user changes display mode,
>  we update crtc's pitch in page flip to avoid
>  this bug.
>  This refers to amdgpu's pageflip.
> >>>
> >>> Alex is the expert to ask about that code, but I'm not sure if that is
> >>> really correct for the old hardware.
> >>>
> >>> As far as I know the crtc's pitch should not change during a page flip, 
> >>> but
> >>> only during a full mode set.
> >>>
> >>> So could you elaborate a bit more how you trigger this?
> >>
> >> legacy page_flip ioctl only verifies that the fb->format stays the same.
> >> It doesn't check anything else (afair never has), this is all up to
> >> drivers to verify.
> >>
> >> Personally I'd say add a check to reject this, since testing this and
> >> making sure it really works everywhere is probably a bit much on this old
> >> hw.
> >
> > If just the pitch changed, that probably wouldn't be much of a
> > problem, but if the pitch is changing, that probably implies other
> > stuff has changed as well and we'll just be chasing changes.  I agree
> > it would be best to just reject anything other than updating the
> > scanout address.
>
> FWIW, that means page flipping cannot be used in some cases which work fine 
> by changing the pitch, which can result in tearing: 
> https://gitlab.freedesktop.org/xorg/xserver/-/issues/839 (which says the i915 
> driver handles this as well).
>

Ok.  In that case, @Zhenneng can you update all of the pitch in all of
the page_flip functions in radeon rather than just the evergreen one?

Thanks,

Alex


>
> --
> Earthling Michel Dänzer   |   https://redhat.com
> Libre software enthusiast | Mesa and X developer

Re: [PATCH] drm/amdgpu: added synchronization for psp cmd buf access

[Clements, John]

[Public]

Hello Guchun,

In most of those cases you are right it is redundant, the reason i kept them 
separate for now is to resolve this bug while also keeping those interfaces 
modular, and not affecting the psp submit sequence yet. We are planning a 
bigger change to that source to remove alot of the duplicate code regarding the 
cmd buffer prepare/submit flow and will probably go back down to one mutex 
there.

Thank you,
John Clements


From: Chen, Guchun 
Sent: Tuesday, August 3, 2021 9:58 PM
To: Clements, John ; amd-gfx@lists.freedesktop.org 

Cc: Zhang, Hawking ; Li, Candice ; 
Lazar, Lijo 
Subject: RE: [PATCH] drm/amdgpu: added synchronization for psp cmd buf access


[Public]



Before calling into psp_cmd_submit_buf, a mutex psp->cmd_buf_mutex is there, 
and after entering psp_cmd_submit_buf, there is another mutex psp->mutex, is it 
a bit redundant?



Regards,

Guchun



From: Clements, John 
Sent: Tuesday, August 3, 2021 5:50 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Li, Candice ; 
Lazar, Lijo ; Chen, Guchun 
Subject: [PATCH] drm/amdgpu: added synchronization for psp cmd buf access



[AMD Official Use Only]



Submitting patch to synchronize access to psp cmd submission memory to resolve 
potential race conditions.

Re: [PATCH v2] drm/amdgpu: fix fdinfo race with process exit

[philip yang]

  
ping?

On 2021-07-29 10:13 p.m., Philip Yang
  wrote:


  Get process vm root BO ref in case process is exiting and root BO is
freed, to avoid NULL pointer dereference backtrace:

BUG: unable to handle kernel NULL pointer dereference at

Call Trace:
amdgpu_show_fdinfo+0xfe/0x2a0 [amdgpu]
seq_show+0x12c/0x180
seq_read+0x153/0x410
vfs_read+0x91/0x140[ 3427.206183]  ksys_read+0x4f/0xb0
do_syscall_64+0x5b/0x1a0
entry_SYSCALL_64_after_hwframe+0x65/0xca

v2: rebase to staging

Signed-off-by: Philip Yang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
index d94c5419ec25..5a6857c44bb6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
@@ -59,6 +59,7 @@ void amdgpu_show_fdinfo(struct seq_file *m, struct file *f)
 	uint64_t vram_mem = 0, gtt_mem = 0, cpu_mem = 0;
 	struct drm_file *file = f->private_data;
 	struct amdgpu_device *adev = drm_to_adev(file->minor->dev);
+	struct amdgpu_bo *root;
 	int ret;
 
 	ret = amdgpu_file_to_fpriv(f, );
@@ -69,13 +70,19 @@ void amdgpu_show_fdinfo(struct seq_file *m, struct file *f)
 	dev = PCI_SLOT(adev->pdev->devfn);
 	fn = PCI_FUNC(adev->pdev->devfn);
 
-	ret = amdgpu_bo_reserve(fpriv->vm.root.bo, false);
+	root = amdgpu_bo_ref(fpriv->vm.root.bo);
+	if (!root)
+		return;
+
+	ret = amdgpu_bo_reserve(root, false);
 	if (ret) {
 		DRM_ERROR("Fail to reserve bo\n");
 		return;
 	}
 	amdgpu_vm_get_memory(>vm, _mem, _mem, _mem);
-	amdgpu_bo_unreserve(fpriv->vm.root.bo);
+	amdgpu_bo_unreserve(root);
+	amdgpu_bo_unref();
+
 	seq_printf(m, "pdev:\t%04x:%02x:%02x.%d\npasid:\t%u\n", domain, bus,
 			dev, fn, fpriv->vm.pasid);
 	seq_printf(m, "vram mem:\t%llu kB\n", vram_mem/1024UL);


  

RE: [PATCH] drm/amdgpu: added synchronization for psp cmd buf access

[Chen, Guchun]

[Public]

Before calling into psp_cmd_submit_buf, a mutex psp->cmd_buf_mutex is there, 
and after entering psp_cmd_submit_buf, there is another mutex psp->mutex, is it 
a bit redundant?

Regards,
Guchun

From: Clements, John 
Sent: Tuesday, August 3, 2021 5:50 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Li, Candice ; 
Lazar, Lijo ; Chen, Guchun 
Subject: [PATCH] drm/amdgpu: added synchronization for psp cmd buf access


[AMD Official Use Only]

Submitting patch to synchronize access to psp cmd submission memory to resolve 
potential race conditions.

RE: [PATCH 0/9] DC Patches for August 2, 2021

[Wheeler, Daniel]

[Public]

Hi all,
 
This week this patchset was tested on the following systems:
 
HP Envy 360, with Ryzen 5 4500U, with the following display types: eDP 1080p 
60hz, 4k 60hz  (via USB-C to DP/HDMI), 1440p 144hz (via USB-C to DP/HDMI), 
1680*1050 60hz (via USB-C to DP and then DP to DVI/VGA)
 
AMD Ryzen 9 5900H, with the following display types: eDP 1080p 60hz, 4k 60hz  
(via USB-C to DP/HDMI), 1440p 144hz (via USB-C to DP/HDMI), 1680*1050 60hz (via 
USB-C to DP and then DP to DVI/VGA)
 
Sapphire Pulse RX5700XT with the following display types:
4k 60hz  (via DP/HDMI), 1440p 144hz (via DP/HDMI), 1680*1050 60hz (via DP to 
DVI/VGA)
 
Reference AMD RX6800 with the following display types:
4k 60hz  (via DP/HDMI and USB-C to DP/HDMI), 1440p 144hz (via USB-C to DP/HDMI 
and USB-C to DP/HDMI), 1680*1050 60hz (via DP to DVI/VGA)
 
Included testing using a Startech DP 1.4 MST hub at 2x 4k 60hz, and 3x 1080p 
60hz on all systems.
 
 
Tested-by: Daniel Wheeler 
 
 
Thank you,
 
Dan Wheeler
Technologist  |  AMD
SW Display
--
1 Commerce Valley Dr E, Thornhill, ON L3T 7X6
Facebook |  Twitter |  amd.com  

-Original Message-
From: amd-gfx  On Behalf Of Aurabindo 
Pillai
Sent: July 29, 2021 4:06 PM
To: amd-gfx@lists.freedesktop.org
Cc: eryk.b...@amd.com; Li, Sun peng (Leo) ; Wentland, Harry 
; Zhuo, Qingqing ; Siqueira, 
Rodrigo ; Jacob, Anson ; Pillai, 
Aurabindo ; Lakha, Bhawanpreet 
; R, Bindu 
Subject: [PATCH 0/9] DC Patches for August 2, 2021

This DC patchset brings improvements in multiple areas. In summary, we have:

* DC version 3.2.147
* DMUB FW release 0.0.77
* LTTPR, MPO improvements
* General bug fixes and stability improvements

--

Anthony Koo (1):
  drm/amd/display: [FW Promotion] Release 0.0.77

Aric Cyr (1):
  drm/amd/display: 3.2.147

Chung (1):
  drm/amd/display: Add check for validating unsupported ODM plus MPO
case

Guo, Bing (2):
  drm/amd/display: Fix Dynamic bpp issue with 8K30 with Navi 1X
  drm/amd/display: Increase stutter watermark for dcn303

Jude Shih (1):
  drm/amd/display: Fix resetting DCN3.1 HW when resuming from S4

Qingqing Zhuo (1):
  drm/amd/display: workaround for hard hang on HPD on native DP

Roman Li (1):
  drm/amd/display: Remove redundant vblank workqueues in DM

Wesley Chalmers (1):
  drm/amd/display: Assume LTTPR interop for DCN31+

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  8 ++-  
.../amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c |  4 +++-  
.../gpu/drm/amd/display/dc/core/dc_link_dp.c  | 21 ++-  
.../gpu/drm/amd/display/dc/core/dc_resource.c |  5 +
 drivers/gpu/drm/amd/display/dc/dc.h   |  4 +++-
 .../gpu/drm/amd/display/dc/dcn20/dcn20_optc.c |  2 +-  
.../drm/amd/display/dc/dcn30/dcn30_resource.c | 20 ++
 .../amd/display/dc/dcn303/dcn303_resource.c   |  4 ++--
 .../drm/amd/display/dc/dcn31/dcn31_resource.c | 16 ++
 .../gpu/drm/amd/display/dmub/inc/dmub_cmd.h   |  4 ++--
 .../gpu/drm/amd/display/dmub/src/dmub_dcn31.c |  8 ---
 11 files changed, 61 insertions(+), 35 deletions(-)

--
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-gfxdata=04%7C01%7Cdaniel.wheeler%40amd.com%7Cd897a79ebeb244afd03408d952cc5fd9%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637631860013972704%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=tf%2BFzw1oQ3Uijba3S0TeNFYoKbNwFqqnh7j2E4jftY8%3Dreserved=0

[PATCH] drm/amdgpu: added synchronization for psp cmd buf access

[Clements, John]

[AMD Official Use Only]

Submitting patch to synchronize access to psp cmd submission memory to resolve 
potential race conditions.


0001-drm-amdgpu-added-synchronization-for-psp-cmd-buf-acc.patch
Description: 0001-drm-amdgpu-added-synchronization-for-psp-cmd-buf-acc.patch

Re: [PATCH] drm/amdgpu: update PSP BL cmd IDs

[Deucher, Alexander]

[Public]

Acked-by: Alex Deucher 

From: amd-gfx  on behalf of Clements, 
John 
Sent: Tuesday, August 3, 2021 4:20 AM
To: amd-gfx@lists.freedesktop.org 
Cc: Zhang, Hawking 
Subject: [PATCH] drm/amdgpu: update PSP BL cmd IDs


[AMD Official Use Only]


Submitting patch to resolve issue with incorrect PSP BL cmd IDs

Re: [PATCH v2 07/14] drm/msm: Convert to Linux IRQ interfaces

[Dmitry Baryshkov]

On 03/08/2021 12:06, Thomas Zimmermann wrote:

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.

DRM IRQ callbacks are now being called directly or inlined.

Signed-off-by: Thomas Zimmermann 


Reviewed-by: Dmitry Baryshkov 

Rob should probably also give his blessing on this patch though.


---
  drivers/gpu/drm/msm/msm_drv.c | 113 --
  drivers/gpu/drm/msm/msm_kms.h |   2 +-
  2 files changed, 69 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 1594ae39d54f..a332b09a5a11 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -14,7 +14,6 @@
  #include 
  #include 
  #include 
-#include 
  #include 
  #include 
  #include 
@@ -201,6 +200,71 @@ void msm_rmw(void __iomem *addr, u32 mask, u32 or)
msm_writel(val | or, addr);
  }
  
+static irqreturn_t msm_irq(int irq, void *arg)

+{
+   struct drm_device *dev = arg;
+   struct msm_drm_private *priv = dev->dev_private;
+   struct msm_kms *kms = priv->kms;
+
+   BUG_ON(!kms);
+
+   return kms->funcs->irq(kms);
+}
+
+static void msm_irq_preinstall(struct drm_device *dev)
+{
+   struct msm_drm_private *priv = dev->dev_private;
+   struct msm_kms *kms = priv->kms;
+
+   BUG_ON(!kms);
+
+   kms->funcs->irq_preinstall(kms);
+}
+
+static int msm_irq_postinstall(struct drm_device *dev)
+{
+   struct msm_drm_private *priv = dev->dev_private;
+   struct msm_kms *kms = priv->kms;
+
+   BUG_ON(!kms);
+
+   if (kms->funcs->irq_postinstall)
+   return kms->funcs->irq_postinstall(kms);
+
+   return 0;
+}
+
+static int msm_irq_install(struct drm_device *dev, unsigned int irq)
+{
+   int ret;
+
+   if (irq == IRQ_NOTCONNECTED)
+   return -ENOTCONN;
+
+   msm_irq_preinstall(dev);
+
+   ret = request_irq(irq, msm_irq, 0, dev->driver->name, dev);
+   if (ret)
+   return ret;
+
+   ret = msm_irq_postinstall(dev);
+   if (ret) {
+   free_irq(irq, dev);
+   return ret;
+   }
+
+   return 0;
+}
+
+static void msm_irq_uninstall(struct drm_device *dev)
+{
+   struct msm_drm_private *priv = dev->dev_private;
+   struct msm_kms *kms = priv->kms;
+
+   kms->funcs->irq_uninstall(kms);
+   free_irq(kms->irq, dev);
+}
+
  struct msm_vblank_work {
struct work_struct work;
int crtc_id;
@@ -265,7 +329,7 @@ static int msm_drm_uninit(struct device *dev)
}
  
  	/* We must cancel and cleanup any pending vblank enable/disable

-* work before drm_irq_uninstall() to avoid work re-enabling an
+* work before msm_irq_uninstall() to avoid work re-enabling an
 * irq after uninstall has disabled it.
 */
  
@@ -294,7 +358,7 @@ static int msm_drm_uninit(struct device *dev)

drm_mode_config_cleanup(ddev);
  
  	pm_runtime_get_sync(dev);

-   drm_irq_uninstall(ddev);
+   msm_irq_uninstall(ddev);
pm_runtime_put_sync(dev);
  
  	if (kms && kms->funcs)

@@ -553,7 +617,7 @@ static int msm_drm_init(struct device *dev, const struct 
drm_driver *drv)
  
  	if (kms) {

pm_runtime_get_sync(dev);
-   ret = drm_irq_install(ddev, kms->irq);
+   ret = msm_irq_install(ddev, kms->irq);
pm_runtime_put_sync(dev);
if (ret < 0) {
DRM_DEV_ERROR(dev, "failed to install IRQ handler\n");
@@ -662,43 +726,6 @@ static void msm_postclose(struct drm_device *dev, struct 
drm_file *file)
context_close(ctx);
  }
  
-static irqreturn_t msm_irq(int irq, void *arg)

-{
-   struct drm_device *dev = arg;
-   struct msm_drm_private *priv = dev->dev_private;
-   struct msm_kms *kms = priv->kms;
-   BUG_ON(!kms);
-   return kms->funcs->irq(kms);
-}
-
-static void msm_irq_preinstall(struct drm_device *dev)
-{
-   struct msm_drm_private *priv = dev->dev_private;
-   struct msm_kms *kms = priv->kms;
-   BUG_ON(!kms);
-   kms->funcs->irq_preinstall(kms);
-}
-
-static int msm_irq_postinstall(struct drm_device *dev)
-{
-   struct msm_drm_private *priv = dev->dev_private;
-   struct msm_kms *kms = priv->kms;
-   BUG_ON(!kms);
-
-   if (kms->funcs->irq_postinstall)
-   return kms->funcs->irq_postinstall(kms);
-
-   return 0;
-}
-
-static void msm_irq_uninstall(struct drm_device *dev)
-{
-   struct msm_drm_private *priv = dev->dev_private;
-   struct msm_kms *kms = priv->kms;
-   BUG_ON(!kms);
-   kms->funcs->irq_uninstall(kms);
-}
-
  int msm_crtc_enable_vblank(struct drm_crtc *crtc)
  {
struct drm_device *dev = crtc->dev;
@@ -1051,10 +1078,6 @@ static const struct drm_driver msm_driver = {
.open   = msm_open,
.postclose   = msm_postclose,

Re: [PATCH] drm/amdgpu/display: fix DMUB firmware version info

[Deucher, Alexander]

[AMD Official Use Only]

Reviewed-by: Alex Deucher 

From: S, Shirish 
Sent: Tuesday, August 3, 2021 4:42 AM
To: Wentland, Harry ; Kazlauskas, Nicholas 
; Deucher, Alexander 
Cc: amd-gfx@lists.freedesktop.org ; S, Shirish 

Subject: [PATCH] drm/amdgpu/display: fix DMUB firmware version info

DMUB firmware info is printed before it gets initialized.
Correct this order to ensure true value is conveyed.

Signed-off-by: Shirish S 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
 1 file changed, 1 insertion(+), 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 7e09b6d26a51..396a2dca2fe0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1548,6 +1548,7 @@ static int dm_dmub_sw_init(struct amdgpu_device *adev)
 }

 hdr = (const struct dmcub_firmware_header_v1_0 
*)adev->dm.dmub_fw->data;
+   adev->dm.dmcub_fw_version = le32_to_cpu(hdr->header.ucode_version);

 if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
 adev->firmware.ucode[AMDGPU_UCODE_ID_DMCUB].ucode_id =
@@ -1561,7 +1562,6 @@ static int dm_dmub_sw_init(struct amdgpu_device *adev)
  adev->dm.dmcub_fw_version);
 }

-   adev->dm.dmcub_fw_version = le32_to_cpu(hdr->header.ucode_version);

 adev->dm.dmub_srv = kzalloc(sizeof(*adev->dm.dmub_srv), GFP_KERNEL);
 dmub_srv = adev->dm.dmub_srv;
--
2.17.1

[PATCH AUTOSEL 5.10 9/9] drm/amdgpu/display: only enable aux backlight control for OLED panels

[Sasha Levin]

From: Alex Deucher 

[ Upstream commit f2ad3accefc63e72e9932e141c21875cc04beec8 ]

We've gotten a number of reports about backlight control not
working on panels which indicate that they use aux backlight
control.  A recent patch:

commit 2d73eabe2984a435737498ab39bb1500a9ffe9a9
Author: Camille Cho 
Date:   Thu Jul 8 18:28:37 2021 +0800

drm/amd/display: Only set default brightness for OLED

[Why]
We used to unconditionally set backlight path as AUX for panels capable
of backlight adjustment via DPCD in set default brightness.

[How]
This should be limited to OLED panel only since we control backlight via
PWM path for SDR mode in LCD HDR panel.

Reviewed-by: Krunoslav Kovac 
Acked-by: Rodrigo Siqueira 
Signed-off-by: Camille Cho 
Signed-off-by: Alex Deucher 

Changes some other code to only use aux for backlight control on
OLED panels.  The commit message seems to indicate that PWM should
be used for SDR mode on HDR panels.  Do something similar for
backlight control in general.  This may need to be revisited if and
when HDR started to get used.

Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
Bug: https://bugzilla.kernel.org/show_bug.cgi?id=213715
Reviewed-by: Roman Li 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
 1 file changed, 2 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 6eb308670f48..8d537123e4ad 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2162,9 +2162,9 @@ static void update_connector_ext_caps(struct 
amdgpu_dm_connector *aconnector)
max_cll = conn_base->hdr_sink_metadata.hdmi_type1.max_cll;
min_cll = conn_base->hdr_sink_metadata.hdmi_type1.min_cll;
 
-   if (caps->ext_caps->bits.oled == 1 ||
+   if (caps->ext_caps->bits.oled == 1 /*||
caps->ext_caps->bits.sdr_aux_backlight_control == 1 ||
-   caps->ext_caps->bits.hdr_aux_backlight_control == 1)
+   caps->ext_caps->bits.hdr_aux_backlight_control == 1*/)
caps->aux_support = true;
 
if (amdgpu_backlight == 0)
-- 
2.30.2

[PATCH AUTOSEL 5.13 09/11] drm/amdgpu/display: only enable aux backlight control for OLED panels

[Sasha Levin]

From: Alex Deucher 

[ Upstream commit f2ad3accefc63e72e9932e141c21875cc04beec8 ]

We've gotten a number of reports about backlight control not
working on panels which indicate that they use aux backlight
control.  A recent patch:

commit 2d73eabe2984a435737498ab39bb1500a9ffe9a9
Author: Camille Cho 
Date:   Thu Jul 8 18:28:37 2021 +0800

drm/amd/display: Only set default brightness for OLED

[Why]
We used to unconditionally set backlight path as AUX for panels capable
of backlight adjustment via DPCD in set default brightness.

[How]
This should be limited to OLED panel only since we control backlight via
PWM path for SDR mode in LCD HDR panel.

Reviewed-by: Krunoslav Kovac 
Acked-by: Rodrigo Siqueira 
Signed-off-by: Camille Cho 
Signed-off-by: Alex Deucher 

Changes some other code to only use aux for backlight control on
OLED panels.  The commit message seems to indicate that PWM should
be used for SDR mode on HDR panels.  Do something similar for
backlight control in general.  This may need to be revisited if and
when HDR started to get used.

Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
Bug: https://bugzilla.kernel.org/show_bug.cgi?id=213715
Reviewed-by: Roman Li 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
 1 file changed, 2 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 eeaae1cf2bc2..f02ad2eb154b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2367,9 +2367,9 @@ static void update_connector_ext_caps(struct 
amdgpu_dm_connector *aconnector)
max_cll = conn_base->hdr_sink_metadata.hdmi_type1.max_cll;
min_cll = conn_base->hdr_sink_metadata.hdmi_type1.min_cll;
 
-   if (caps->ext_caps->bits.oled == 1 ||
+   if (caps->ext_caps->bits.oled == 1 /*||
caps->ext_caps->bits.sdr_aux_backlight_control == 1 ||
-   caps->ext_caps->bits.hdr_aux_backlight_control == 1)
+   caps->ext_caps->bits.hdr_aux_backlight_control == 1*/)
caps->aux_support = true;
 
if (amdgpu_backlight == 0)
-- 
2.30.2

Re: [PATCH] drm/amd/amdgpu: remove redundant host to psp cmd buf

[Lazar, Lijo]

Command buffer in psp context means different command buffers cannot be 
prepared in parallel. Any case of submitting commands for different TAs 
in parallel - ex: for RAS and some other TA?


Thanks,
Lijo

On 8/3/2021 8:35 AM, Li, Candice wrote:

[Public]


Signed-off-by: Candice Li candice...@amd.com 

---

drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 253 

1 file changed, 78 insertions(+), 175 deletions(-)

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


index ed731144ca7f..9c18558e3bc0 100644

--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c

+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c

@@ -253,6 +253,12 @@ static int psp_sw_init(void *handle)

  struct psp_runtime_boot_cfg_entry boot_cfg_entry;

  struct psp_memory_training_context *mem_training_ctx = 
>mem_train_ctx;


+    psp->cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), 
GFP_KERNEL);


+    if (!psp->cmd) {

+   DRM_ERROR("Failed to allocate memory to 
command buffer!\n");


+   ret = -ENOMEM;

+    }

+

  if (!amdgpu_sriov_vf(adev)) {

     ret = psp_init_microcode(psp);

     if (ret) {

@@ -315,25 +321,30 @@ static int psp_sw_init(void *handle)

static int psp_sw_fini(void *handle)

{

  struct amdgpu_device *adev = (struct amdgpu_device *)handle;

+    struct psp_context *psp = >psp;

+    struct psp_gfx_cmd_resp *cmd = psp->cmd;

- psp_memory_training_fini(>psp);

- if (adev->psp.sos_fw) {

-   release_firmware(adev->psp.sos_fw);

-   adev->psp.sos_fw = NULL;

+    psp_memory_training_fini(psp);

+    if (psp->sos_fw) {

+   release_firmware(psp->sos_fw);

+   psp->sos_fw = NULL;

  }

- if (adev->psp.asd_fw) {

-   release_firmware(adev->psp.asd_fw);

+    if (psp->asd_fw) {

+   release_firmware(psp->asd_fw);

     adev->psp.asd_fw = NULL;

  }

- if (adev->psp.ta_fw) {

-   release_firmware(adev->psp.ta_fw);

-   adev->psp.ta_fw = NULL;

+    if (psp->ta_fw) {

+   release_firmware(psp->ta_fw);

+   psp->ta_fw = NULL;

  }

   if (adev->asic_type == CHIP_NAVI10 ||

  adev->asic_type == CHIP_SIENNA_CICHLID)

     psp_sysfs_fini(adev);

+    kfree(cmd);

+    cmd = NULL;

+

  return 0;

}

@@ -491,6 +502,8 @@ static void psp_prep_tmr_cmd_buf(struct psp_context 
*psp,


  uint32_t size = amdgpu_bo_size(tmr_bo);

  uint64_t tmr_pa = amdgpu_gmc_vram_pa(adev, tmr_bo);

+    memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));

+

  if (amdgpu_sriov_vf(psp->adev))

     cmd->cmd_id = GFX_CMD_ID_SETUP_VMR;

  else

@@ -506,6 +519,8 @@ static void psp_prep_tmr_cmd_buf(struct psp_context 
*psp,


static void psp_prep_load_toc_cmd_buf(struct psp_gfx_cmd_resp *cmd,

    uint64_t 
pri_buf_mc, uint32_t size)


{

+    memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));

+

  cmd->cmd_id = GFX_CMD_ID_LOAD_TOC;

  cmd->cmd.cmd_load_toc.toc_phy_addr_lo = 
lower_32_bits(pri_buf_mc);


  cmd->cmd.cmd_load_toc.toc_phy_addr_hi = 
upper_32_bits(pri_buf_mc);


@@ -517,11 +532,8 @@ static int psp_load_toc(struct psp_context *psp,

   uint32_t *tmr_size)

{

  int ret;

- struct psp_gfx_cmd_resp *cmd;

+    struct psp_gfx_cmd_resp *cmd = psp->cmd;

- cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);

- if (!cmd)

-   return -ENOMEM;

  /* Copy toc to psp firmware private buffer */

  psp_copy_fw(psp, psp->toc.start_addr, psp->toc.size_bytes);

@@ -531,7 +543,7 @@ static int psp_load_toc(struct psp_context *psp,

  
  psp->fence_buf_mc_addr);


  if (!ret)

     *tmr_size = psp->cmd_buf_mem->resp.tmr_size;

- kfree(cmd);

+

  return ret;

}

@@ -588,7 +600,7 @@ static bool psp_skip_tmr(struct psp_context *psp)

static int psp_tmr_load(struct psp_context *psp)

{

  int ret;

- struct psp_gfx_cmd_resp *cmd;

+    struct psp_gfx_cmd_resp *cmd = psp->cmd;

   /* For Navi12 and CHIP_SIENNA_CICHLID SRIOV, do not set 
up TMR.


   * Already set 

New uAPI for color management proposal and feedback request v2

[Werner Sembach]

Greetings,

Original proposal: 
https://www.mail-archive.com/amd-gfx@lists.freedesktop.org/msg62387.html

Abstract: Add "preferred color format", "active color format", "active bpc", 
and "active Broadcast RGB" drm properties,
to control color information send to the monitor.

It seems that the "preferred-" properties is not what is actually the most 
useful for the userspace devs.

Preferable (Note: with only a sample size of 2 people) would be a "force color 
format" property. If the color format is
not available for the current Monitor and GPU combo. the TEST_ONLY check should 
fail and the property should not be setable.

This however opens another problem: When a Monitor is disconnected and a new 
one is connected, the drm properties do not
get resetted. So if the old monitor did allow to set for example ycbcr420, but 
the new monitor does not support this
color format at all, it will stay permanently black until the drm property is 
set to a correct value by hand. This is
not an expected behavior imho.

So a discussion questions: Does it make sense that connector properties are 
keep for different Monitors?

If no: On connecting a new Monitor all atomic drm properties should be reset to 
a default value.

I have an idea how this could be implemented (correct me if i'm wrong): When an 
atomic property is attached it get
assigned an inital value. But if I understood the docu correctly, this value is 
ignored because atomic properties use
the getter and setter methods when their values are read or written. My 
implementation suggestion would be to iterate
over all attached atomic properties once a new monitor is connected and reset 
them to this initial value, which should
be unchanged since initialization? This assumes that besides the initial value 
being unused it's still a sane default
for all drivers.

Kind Regards,

Werner Sembach

[PATCH v2 13/14] drm: Remove unused devm_drm_irq_install()

[Thomas Zimmermann]

DRM IRQ helpers will become legacy. The function devm_drm_irq_install()
is unused and won't be required later.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_irq.c | 32 
 include/drm/drm_irq.h |  1 -
 2 files changed, 33 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 201eae4bba6c..dc6e38fa8a48 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -209,38 +209,6 @@ int drm_irq_uninstall(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_irq_uninstall);
 
-static void devm_drm_irq_uninstall(void *data)
-{
-   drm_irq_uninstall(data);
-}
-
-/**
- * devm_drm_irq_install - install IRQ handler
- * @dev: DRM device
- * @irq: IRQ number to install the handler for
- *
- * devm_drm_irq_install is a  help function of drm_irq_install.
- *
- * if the driver uses devm_drm_irq_install, there is no need
- * to call drm_irq_uninstall when the drm module get unloaded,
- * as this will done automagically.
- *
- * Returns:
- * Zero on success or a negative error code on failure.
- */
-int devm_drm_irq_install(struct drm_device *dev, int irq)
-{
-   int ret;
-
-   ret = drm_irq_install(dev, irq);
-   if (ret)
-   return ret;
-
-   return devm_add_action_or_reset(dev->dev,
-   devm_drm_irq_uninstall, dev);
-}
-EXPORT_SYMBOL(devm_drm_irq_install);
-
 #if IS_ENABLED(CONFIG_DRM_LEGACY)
 int drm_legacy_irq_control(struct drm_device *dev, void *data,
   struct drm_file *file_priv)
diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h
index 631b22f9757d..53634b988f57 100644
--- a/include/drm/drm_irq.h
+++ b/include/drm/drm_irq.h
@@ -28,5 +28,4 @@ struct drm_device;
 
 int drm_irq_install(struct drm_device *dev, int irq);
 int drm_irq_uninstall(struct drm_device *dev);
-int devm_drm_irq_install(struct drm_device *dev, int irq);
 #endif
-- 
2.32.0

[PATCH v2 12/14] drm/vc4: Convert to Linux IRQ interfaces

[Thomas Zimmermann]

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.

DRM IRQ callbacks are now being called directly or inlined.

Calls to platform_get_irq() can fail with a negative errno code.
Abort initialization in this case. The DRM IRQ midlayer does not
handle this case correctly.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/vc4/vc4_drv.c |  4 ---
 drivers/gpu/drm/vc4/vc4_drv.h |  8 +++---
 drivers/gpu/drm/vc4/vc4_irq.c | 48 +++
 drivers/gpu/drm/vc4/vc4_v3d.c | 17 -
 4 files changed, 53 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 73335feb712f..f6c16c5aee68 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -168,10 +168,6 @@ static struct drm_driver vc4_drm_driver = {
DRIVER_SYNCOBJ),
.open = vc4_open,
.postclose = vc4_close,
-   .irq_handler = vc4_irq,
-   .irq_preinstall = vc4_irq_preinstall,
-   .irq_postinstall = vc4_irq_postinstall,
-   .irq_uninstall = vc4_irq_uninstall,
 
 #if defined(CONFIG_DEBUG_FS)
.debugfs_init = vc4_debugfs_init,
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 5dceadc61600..ef73e0aaf726 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -74,6 +74,8 @@ struct vc4_perfmon {
 struct vc4_dev {
struct drm_device base;
 
+   unsigned int irq;
+
struct vc4_hvs *hvs;
struct vc4_v3d *v3d;
struct vc4_dpi *dpi;
@@ -895,9 +897,9 @@ extern struct platform_driver vc4_vec_driver;
 extern struct platform_driver vc4_txp_driver;
 
 /* vc4_irq.c */
-irqreturn_t vc4_irq(int irq, void *arg);
-void vc4_irq_preinstall(struct drm_device *dev);
-int vc4_irq_postinstall(struct drm_device *dev);
+void vc4_irq_enable(struct drm_device *dev);
+void vc4_irq_disable(struct drm_device *dev);
+int vc4_irq_install(struct drm_device *dev, int irq);
 void vc4_irq_uninstall(struct drm_device *dev);
 void vc4_irq_reset(struct drm_device *dev);
 
diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c
index e226c24e543f..20fa8e34c20b 100644
--- a/drivers/gpu/drm/vc4/vc4_irq.c
+++ b/drivers/gpu/drm/vc4/vc4_irq.c
@@ -45,6 +45,10 @@
  * current job can make progress.
  */
 
+#include 
+
+#include 
+
 #include "vc4_drv.h"
 #include "vc4_regs.h"
 
@@ -192,7 +196,7 @@ vc4_irq_finish_render_job(struct drm_device *dev)
schedule_work(>job_done_work);
 }
 
-irqreturn_t
+static irqreturn_t
 vc4_irq(int irq, void *arg)
 {
struct drm_device *dev = arg;
@@ -234,8 +238,8 @@ vc4_irq(int irq, void *arg)
return status;
 }
 
-void
-vc4_irq_preinstall(struct drm_device *dev)
+static void
+vc4_irq_prepare(struct drm_device *dev)
 {
struct vc4_dev *vc4 = to_vc4_dev(dev);
 
@@ -251,24 +255,22 @@ vc4_irq_preinstall(struct drm_device *dev)
V3D_WRITE(V3D_INTCTL, V3D_DRIVER_IRQS);
 }
 
-int
-vc4_irq_postinstall(struct drm_device *dev)
+void
+vc4_irq_enable(struct drm_device *dev)
 {
struct vc4_dev *vc4 = to_vc4_dev(dev);
 
if (!vc4->v3d)
-   return 0;
+   return;
 
/* Enable the render done interrupts. The out-of-memory interrupt is
 * enabled as soon as we have a binner BO allocated.
 */
V3D_WRITE(V3D_INTENA, V3D_INT_FLDONE | V3D_INT_FRDONE);
-
-   return 0;
 }
 
 void
-vc4_irq_uninstall(struct drm_device *dev)
+vc4_irq_disable(struct drm_device *dev)
 {
struct vc4_dev *vc4 = to_vc4_dev(dev);
 
@@ -282,11 +284,37 @@ vc4_irq_uninstall(struct drm_device *dev)
V3D_WRITE(V3D_INTCTL, V3D_DRIVER_IRQS);
 
/* Finish any interrupt handler still in flight. */
-   disable_irq(dev->irq);
+   disable_irq(vc4->irq);
 
cancel_work_sync(>overflow_mem_work);
 }
 
+int vc4_irq_install(struct drm_device *dev, int irq)
+{
+   int ret;
+
+   if (irq == IRQ_NOTCONNECTED)
+   return -ENOTCONN;
+
+   vc4_irq_prepare(dev);
+
+   ret = request_irq(irq, vc4_irq, 0, dev->driver->name, dev);
+   if (ret)
+   return ret;
+
+   vc4_irq_enable(dev);
+
+   return 0;
+}
+
+void vc4_irq_uninstall(struct drm_device *dev)
+{
+   struct vc4_dev *vc4 = to_vc4_dev(dev);
+
+   vc4_irq_disable(dev);
+   free_irq(vc4->irq, dev);
+}
+
 /** Reinitializes interrupt registers when a GPU reset is performed. */
 void vc4_irq_reset(struct drm_device *dev)
 {
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index 73d63d72575b..7bb3067f8425 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -10,8 +10,6 @@
 #include 
 #include 
 
-#include 
-
 #include "vc4_drv.h"
 #include "vc4_regs.h"
 
@@ -361,7 +359,7 @@ static int vc4_v3d_runtime_suspend(struct device *dev)
struct vc4_v3d *v3d 

[PATCH v2 14/14] drm: IRQ midlayer is now legacy

[Thomas Zimmermann]

Hide the DRM midlayer behind CONFIG_DRM_LEGACY, make functions use
the prefix drm_legacy_, and move declarations to drm_legacy.h.
In struct drm_device, move the fields irq and irq_enabled behind
CONFIG_DRM_LEGACY.

All callers have been updated.

Signed-off-by: Thomas Zimmermann 
Acked-by: Sam Ravnborg 
---
 drivers/gpu/drm/drm_irq.c | 63 ---
 drivers/gpu/drm/drm_legacy_misc.c |  3 +-
 drivers/gpu/drm/drm_vblank.c  |  8 ++--
 drivers/gpu/drm/i810/i810_dma.c   |  3 +-
 drivers/gpu/drm/mga/mga_dma.c |  2 +-
 drivers/gpu/drm/mga/mga_drv.h |  1 -
 drivers/gpu/drm/r128/r128_cce.c   |  3 +-
 drivers/gpu/drm/via/via_mm.c  |  3 +-
 include/drm/drm_device.h  | 18 ++---
 include/drm/drm_drv.h | 44 ++---
 include/drm/drm_irq.h | 31 ---
 include/drm/drm_legacy.h  |  3 ++
 12 files changed, 27 insertions(+), 155 deletions(-)
 delete mode 100644 include/drm/drm_irq.h

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index dc6e38fa8a48..13e1d5c4ec82 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -60,46 +60,14 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 
 #include "drm_internal.h"
 
-/**
- * DOC: irq helpers
- *
- * The DRM core provides very simple support helpers to enable IRQ handling on 
a
- * device through the drm_irq_install() and drm_irq_uninstall() functions. This
- * only supports devices with a single interrupt on the main device stored in
- * _device.dev and set as the device paramter in drm_dev_alloc().
- *
- * These IRQ helpers are strictly optional. Since these helpers don't 
automatically
- * clean up the requested interrupt like e.g. devm_request_irq() they're not 
really
- * recommended.
- */
-
-/**
- * drm_irq_install - install IRQ handler
- * @dev: DRM device
- * @irq: IRQ number to install the handler for
- *
- * Initializes the IRQ related data. Installs the handler, calling the driver
- * _driver.irq_preinstall and _driver.irq_postinstall functions before
- * and after the installation.
- *
- * This is the simplified helper interface provided for drivers with no special
- * needs.
- *
- * @irq must match the interrupt number that would be passed to request_irq(),
- * if called directly instead of using this helper function.
- *
- * _driver.irq_handler is called to handle the registered interrupt.
- *
- * Returns:
- * Zero on success or a negative error code on failure.
- */
-int drm_irq_install(struct drm_device *dev, int irq)
+#if IS_ENABLED(CONFIG_DRM_LEGACY)
+static int drm_legacy_irq_install(struct drm_device *dev, int irq)
 {
int ret;
unsigned long sh_flags = 0;
@@ -144,24 +112,8 @@ int drm_irq_install(struct drm_device *dev, int irq)
 
return ret;
 }
-EXPORT_SYMBOL(drm_irq_install);
 
-/**
- * drm_irq_uninstall - uninstall the IRQ handler
- * @dev: DRM device
- *
- * Calls the driver's _driver.irq_uninstall function and unregisters the 
IRQ
- * handler.  This should only be called by drivers which used drm_irq_install()
- * to set up their interrupt handler.
- *
- * Note that for kernel modesetting drivers it is a bug if this function fails.
- * The sanity checks are only to catch buggy user modesetting drivers which 
call
- * the same function through an ioctl.
- *
- * Returns:
- * Zero on success or a negative error code on failure.
- */
-int drm_irq_uninstall(struct drm_device *dev)
+int drm_legacy_irq_uninstall(struct drm_device *dev)
 {
unsigned long irqflags;
bool irq_enabled;
@@ -207,9 +159,8 @@ int drm_irq_uninstall(struct drm_device *dev)
 
return 0;
 }
-EXPORT_SYMBOL(drm_irq_uninstall);
+EXPORT_SYMBOL(drm_legacy_irq_uninstall);
 
-#if IS_ENABLED(CONFIG_DRM_LEGACY)
 int drm_legacy_irq_control(struct drm_device *dev, void *data,
   struct drm_file *file_priv)
 {
@@ -238,13 +189,13 @@ int drm_legacy_irq_control(struct drm_device *dev, void 
*data,
ctl->irq != irq)
return -EINVAL;
mutex_lock(>struct_mutex);
-   ret = drm_irq_install(dev, irq);
+   ret = drm_legacy_irq_install(dev, irq);
mutex_unlock(>struct_mutex);
 
return ret;
case DRM_UNINST_HANDLER:
mutex_lock(>struct_mutex);
-   ret = drm_irq_uninstall(dev);
+   ret = drm_legacy_irq_uninstall(dev);
mutex_unlock(>struct_mutex);
 
return ret;
diff --git a/drivers/gpu/drm/drm_legacy_misc.c 
b/drivers/gpu/drm/drm_legacy_misc.c
index 83db43b7a25e..d4c5434062d7 100644
--- a/drivers/gpu/drm/drm_legacy_misc.c
+++ b/drivers/gpu/drm/drm_legacy_misc.c
@@ -35,7 +35,6 @@
 
 #include 
 #include 
-#include 
 #include 
 
 #include "drm_internal.h"
@@ -78,7 +77,7 @@ int drm_legacy_setup(struct drm_device * dev)
 void drm_legacy_dev_reinit(struct drm_device *dev)
 {
if 

[PATCH v2 11/14] drm/tilcdc: Convert to Linux IRQ interfaces

[Thomas Zimmermann]

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.

DRM IRQ callbacks are now being called directly or inlined.

Calls to platform_get_irq() can fail with a negative errno code.
Abort initialization in this case. The DRM IRQ midlayer does not
handle this case correctly.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 51 ++---
 drivers/gpu/drm/tilcdc/tilcdc_drv.h |  3 ++
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c 
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index f1d3a9f919fd..6b03f89a98d4 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -20,7 +20,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -124,6 +123,39 @@ static int cpufreq_transition(struct notifier_block *nb,
 }
 #endif
 
+static irqreturn_t tilcdc_irq(int irq, void *arg)
+{
+   struct drm_device *dev = arg;
+   struct tilcdc_drm_private *priv = dev->dev_private;
+
+   return tilcdc_crtc_irq(priv->crtc);
+}
+
+static int tilcdc_irq_install(struct drm_device *dev, unsigned int irq)
+{
+   struct tilcdc_drm_private *priv = dev->dev_private;
+   int ret;
+
+   ret = request_irq(irq, tilcdc_irq, 0, dev->driver->name, dev);
+   if (ret)
+   return ret;
+
+   priv->irq_enabled = false;
+
+   return 0;
+}
+
+static void tilcdc_irq_uninstall(struct drm_device *dev)
+{
+   struct tilcdc_drm_private *priv = dev->dev_private;
+
+   if (!priv->irq_enabled)
+   return;
+
+   free_irq(priv->irq, dev);
+   priv->irq_enabled = false;
+}
+
 /*
  * DRM operations:
  */
@@ -145,7 +177,7 @@ static void tilcdc_fini(struct drm_device *dev)
drm_dev_unregister(dev);
 
drm_kms_helper_poll_fini(dev);
-   drm_irq_uninstall(dev);
+   tilcdc_irq_uninstall(dev);
drm_mode_config_cleanup(dev);
 
if (priv->clk)
@@ -336,7 +368,12 @@ static int tilcdc_init(const struct drm_driver *ddrv, 
struct device *dev)
goto init_failed;
}
 
-   ret = drm_irq_install(ddev, platform_get_irq(pdev, 0));
+   ret = platform_get_irq(pdev, 0);
+   if (ret < 0)
+   goto init_failed;
+   priv->irq = ret;
+
+   ret = tilcdc_irq_install(ddev, priv->irq);
if (ret < 0) {
dev_err(dev, "failed to install IRQ handler\n");
goto init_failed;
@@ -360,13 +397,6 @@ static int tilcdc_init(const struct drm_driver *ddrv, 
struct device *dev)
return ret;
 }
 
-static irqreturn_t tilcdc_irq(int irq, void *arg)
-{
-   struct drm_device *dev = arg;
-   struct tilcdc_drm_private *priv = dev->dev_private;
-   return tilcdc_crtc_irq(priv->crtc);
-}
-
 #if defined(CONFIG_DEBUG_FS)
 static const struct {
const char *name;
@@ -454,7 +484,6 @@ DEFINE_DRM_GEM_CMA_FOPS(fops);
 
 static const struct drm_driver tilcdc_driver = {
.driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
-   .irq_handler= tilcdc_irq,
DRM_GEM_CMA_DRIVER_OPS,
 #ifdef CONFIG_DEBUG_FS
.debugfs_init   = tilcdc_debugfs_init,
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h 
b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index d29806ca8817..b818448c83f6 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -46,6 +46,8 @@ struct tilcdc_drm_private {
struct clk *clk; /* functional clock */
int rev; /* IP revision */
 
+   unsigned int irq;
+
/* don't attempt resolutions w/ higher W * H * Hz: */
uint32_t max_bandwidth;
/*
@@ -82,6 +84,7 @@ struct tilcdc_drm_private {
 
bool is_registered;
bool is_componentized;
+   bool irq_enabled;
 };
 
 /* Sub-module for display.  Since we don't know at compile time what panels
-- 
2.32.0

[PATCH v2 06/14] drm/kmb: Convert to Linux IRQ interfaces

[Thomas Zimmermann]

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.

DRM IRQ callbacks are now being called directly or inlined.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/kmb/kmb_drv.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/kmb/kmb_drv.c b/drivers/gpu/drm/kmb/kmb_drv.c
index f54392ec4fab..1c2f4799f421 100644
--- a/drivers/gpu/drm/kmb/kmb_drv.c
+++ b/drivers/gpu/drm/kmb/kmb_drv.c
@@ -17,7 +17,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -413,14 +412,29 @@ static void kmb_irq_reset(struct drm_device *drm)
kmb_write_lcd(to_kmb(drm), LCD_INT_ENABLE, 0);
 }
 
+static int kmb_irq_install(struct drm_device *drm, unsigned int irq)
+{
+   if (irq == IRQ_NOTCONNECTED)
+   return -ENOTCONN;
+
+   kmb_irq_reset(drm);
+
+   return request_irq(irq, kmb_isr, 0, drm->driver->name, drm);
+}
+
+static void kmb_irq_uninstall(struct drm_device *drm)
+{
+   struct kmb_drm_private *kmb = to_kmb(drm);
+
+   kmb_irq_reset(drm);
+   free_irq(kmb->irq_lcd, drm);
+}
+
 DEFINE_DRM_GEM_CMA_FOPS(fops);
 
 static const struct drm_driver kmb_driver = {
.driver_features = DRIVER_GEM |
DRIVER_MODESET | DRIVER_ATOMIC,
-   .irq_handler = kmb_isr,
-   .irq_preinstall = kmb_irq_reset,
-   .irq_uninstall = kmb_irq_reset,
/* GEM Operations */
.fops = ,
DRM_GEM_CMA_DRIVER_OPS_VMAP,
@@ -442,7 +456,7 @@ static int kmb_remove(struct platform_device *pdev)
of_node_put(kmb->crtc.port);
kmb->crtc.port = NULL;
pm_runtime_get_sync(drm->dev);
-   drm_irq_uninstall(drm);
+   kmb_irq_uninstall(drm);
pm_runtime_put_sync(drm->dev);
pm_runtime_disable(drm->dev);
 
@@ -532,7 +546,7 @@ static int kmb_probe(struct platform_device *pdev)
if (ret)
goto err_free;
 
-   ret = drm_irq_install(>drm, kmb->irq_lcd);
+   ret = kmb_irq_install(>drm, kmb->irq_lcd);
if (ret < 0) {
drm_err(>drm, "failed to install IRQ handler\n");
goto err_irq;
-- 
2.32.0

[PATCH v2 09/14] drm/radeon: Convert to Linux IRQ interfaces

[Thomas Zimmermann]

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.

DRM IRQ callbacks are now being called directly or inlined.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Alex Deucher 
---
 drivers/gpu/drm/radeon/radeon_drv.c |  4 ---
 drivers/gpu/drm/radeon/radeon_irq_kms.c | 44 +
 drivers/gpu/drm/radeon/radeon_kms.h |  4 ---
 3 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index c8dd68152d65..b74cebca1f89 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -607,10 +607,6 @@ static const struct drm_driver kms_driver = {
.postclose = radeon_driver_postclose_kms,
.lastclose = radeon_driver_lastclose_kms,
.unload = radeon_driver_unload_kms,
-   .irq_preinstall = radeon_driver_irq_preinstall_kms,
-   .irq_postinstall = radeon_driver_irq_postinstall_kms,
-   .irq_uninstall = radeon_driver_irq_uninstall_kms,
-   .irq_handler = radeon_driver_irq_handler_kms,
.ioctls = radeon_ioctls_kms,
.num_ioctls = ARRAY_SIZE(radeon_ioctls_kms),
.dumb_create = radeon_mode_dumb_create,
diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c 
b/drivers/gpu/drm/radeon/radeon_irq_kms.c
index a36ce826d0c0..3907785d0798 100644
--- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
@@ -31,7 +31,7 @@
 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -51,7 +51,7 @@
  * radeon_irq_process is a macro that points to the per-asic
  * irq handler callback.
  */
-irqreturn_t radeon_driver_irq_handler_kms(int irq, void *arg)
+static irqreturn_t radeon_driver_irq_handler_kms(int irq, void *arg)
 {
struct drm_device *dev = (struct drm_device *) arg;
struct radeon_device *rdev = dev->dev_private;
@@ -118,7 +118,7 @@ static void radeon_dp_work_func(struct work_struct *work)
  * Gets the hw ready to enable irqs (all asics).
  * This function disables all interrupt sources on the GPU.
  */
-void radeon_driver_irq_preinstall_kms(struct drm_device *dev)
+static void radeon_driver_irq_preinstall_kms(struct drm_device *dev)
 {
struct radeon_device *rdev = dev->dev_private;
unsigned long irqflags;
@@ -150,7 +150,7 @@ void radeon_driver_irq_preinstall_kms(struct drm_device 
*dev)
  * Handles stuff to be done after enabling irqs (all asics).
  * Returns 0 on success.
  */
-int radeon_driver_irq_postinstall_kms(struct drm_device *dev)
+static int radeon_driver_irq_postinstall_kms(struct drm_device *dev)
 {
struct radeon_device *rdev = dev->dev_private;
 
@@ -169,7 +169,7 @@ int radeon_driver_irq_postinstall_kms(struct drm_device 
*dev)
  *
  * This function disables all interrupt sources on the GPU (all asics).
  */
-void radeon_driver_irq_uninstall_kms(struct drm_device *dev)
+static void radeon_driver_irq_uninstall_kms(struct drm_device *dev)
 {
struct radeon_device *rdev = dev->dev_private;
unsigned long irqflags;
@@ -194,6 +194,36 @@ void radeon_driver_irq_uninstall_kms(struct drm_device 
*dev)
spin_unlock_irqrestore(>irq.lock, irqflags);
 }
 
+static int radeon_irq_install(struct radeon_device *rdev, int irq)
+{
+   struct drm_device *dev = rdev->ddev;
+   int ret;
+
+   if (irq == IRQ_NOTCONNECTED)
+   return -ENOTCONN;
+
+   radeon_driver_irq_preinstall_kms(dev);
+
+   /* PCI devices require shared interrupts. */
+   ret = request_irq(irq, radeon_driver_irq_handler_kms,
+ IRQF_SHARED, dev->driver->name, dev);
+   if (ret)
+   return ret;
+
+   radeon_driver_irq_postinstall_kms(dev);
+
+   return 0;
+}
+
+static void radeon_irq_uninstall(struct radeon_device *rdev)
+{
+   struct drm_device *dev = rdev->ddev;
+   struct pci_dev *pdev = to_pci_dev(dev->dev);
+
+   radeon_driver_irq_uninstall_kms(dev);
+   free_irq(pdev->irq, dev);
+}
+
 /**
  * radeon_msi_ok - asic specific msi checks
  *
@@ -314,7 +344,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
INIT_WORK(>audio_work, r600_audio_update_hdmi);
 
rdev->irq.installed = true;
-   r = drm_irq_install(rdev->ddev, rdev->pdev->irq);
+   r = radeon_irq_install(rdev, rdev->pdev->irq);
if (r) {
rdev->irq.installed = false;
flush_delayed_work(>hotplug_work);
@@ -335,7 +365,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
 void radeon_irq_kms_fini(struct radeon_device *rdev)
 {
if (rdev->irq.installed) {
-   drm_irq_uninstall(rdev->ddev);
+   radeon_irq_uninstall(rdev);
rdev->irq.installed = false;
if (rdev->msi_enabled)
pci_disable_msi(rdev->pdev);
diff --git 

[PATCH v2 10/14] drm/tidss: Convert to Linux IRQ interfaces

[Thomas Zimmermann]

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.

DRM IRQ callbacks are now being called directly or inlined.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Tomi Valkeinen 
---
 drivers/gpu/drm/tidss/tidss_drv.c | 15 +--
 drivers/gpu/drm/tidss/tidss_drv.h |  2 ++
 drivers/gpu/drm/tidss/tidss_irq.c | 27 ---
 drivers/gpu/drm/tidss/tidss_irq.h |  4 +---
 4 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_drv.c 
b/drivers/gpu/drm/tidss/tidss_drv.c
index 66e3c86eb5c7..d620f35688da 100644
--- a/drivers/gpu/drm/tidss/tidss_drv.c
+++ b/drivers/gpu/drm/tidss/tidss_drv.c
@@ -16,7 +16,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -118,11 +117,6 @@ static const struct drm_driver tidss_driver = {
.date   = "20180215",
.major  = 1,
.minor  = 0,
-
-   .irq_preinstall = tidss_irq_preinstall,
-   .irq_postinstall= tidss_irq_postinstall,
-   .irq_handler= tidss_irq_handler,
-   .irq_uninstall  = tidss_irq_uninstall,
 };
 
 static int tidss_probe(struct platform_device *pdev)
@@ -172,10 +166,11 @@ static int tidss_probe(struct platform_device *pdev)
ret = irq;
goto err_runtime_suspend;
}
+   tidss->irq = irq;
 
-   ret = drm_irq_install(ddev, irq);
+   ret = tidss_irq_install(ddev, irq);
if (ret) {
-   dev_err(dev, "drm_irq_install failed: %d\n", ret);
+   dev_err(dev, "tidss_irq_install failed: %d\n", ret);
goto err_runtime_suspend;
}
 
@@ -196,7 +191,7 @@ static int tidss_probe(struct platform_device *pdev)
return 0;
 
 err_irq_uninstall:
-   drm_irq_uninstall(ddev);
+   tidss_irq_uninstall(ddev);
 
 err_runtime_suspend:
 #ifndef CONFIG_PM
@@ -219,7 +214,7 @@ static int tidss_remove(struct platform_device *pdev)
 
drm_atomic_helper_shutdown(ddev);
 
-   drm_irq_uninstall(ddev);
+   tidss_irq_uninstall(ddev);
 
 #ifndef CONFIG_PM
/* If we don't have PM, we need to call suspend manually */
diff --git a/drivers/gpu/drm/tidss/tidss_drv.h 
b/drivers/gpu/drm/tidss/tidss_drv.h
index 7de4bba52e6f..d7f27b0b0315 100644
--- a/drivers/gpu/drm/tidss/tidss_drv.h
+++ b/drivers/gpu/drm/tidss/tidss_drv.h
@@ -27,6 +27,8 @@ struct tidss_device {
unsigned int num_planes;
struct drm_plane *planes[TIDSS_MAX_PLANES];
 
+   unsigned int irq;
+
spinlock_t wait_lock;   /* protects the irq masks */
dispc_irq_t irq_mask;   /* enabled irqs in addition to wait_list */
 };
diff --git a/drivers/gpu/drm/tidss/tidss_irq.c 
b/drivers/gpu/drm/tidss/tidss_irq.c
index 2ed3e3296776..0c681c7600bc 100644
--- a/drivers/gpu/drm/tidss/tidss_irq.c
+++ b/drivers/gpu/drm/tidss/tidss_irq.c
@@ -4,6 +4,9 @@
  * Author: Tomi Valkeinen 
  */
 
+#include 
+
+#include 
 #include 
 
 #include "tidss_crtc.h"
@@ -50,7 +53,7 @@ void tidss_irq_disable_vblank(struct drm_crtc *crtc)
spin_unlock_irqrestore(>wait_lock, flags);
 }
 
-irqreturn_t tidss_irq_handler(int irq, void *arg)
+static irqreturn_t tidss_irq_handler(int irq, void *arg)
 {
struct drm_device *ddev = (struct drm_device *)arg;
struct tidss_device *tidss = to_tidss(ddev);
@@ -90,7 +93,7 @@ void tidss_irq_resume(struct tidss_device *tidss)
spin_unlock_irqrestore(>wait_lock, flags);
 }
 
-void tidss_irq_preinstall(struct drm_device *ddev)
+static void tidss_irq_preinstall(struct drm_device *ddev)
 {
struct tidss_device *tidss = to_tidss(ddev);
 
@@ -104,7 +107,7 @@ void tidss_irq_preinstall(struct drm_device *ddev)
tidss_runtime_put(tidss);
 }
 
-int tidss_irq_postinstall(struct drm_device *ddev)
+static void tidss_irq_postinstall(struct drm_device *ddev)
 {
struct tidss_device *tidss = to_tidss(ddev);
unsigned long flags;
@@ -129,6 +132,22 @@ int tidss_irq_postinstall(struct drm_device *ddev)
spin_unlock_irqrestore(>wait_lock, flags);
 
tidss_runtime_put(tidss);
+}
+
+int tidss_irq_install(struct drm_device *ddev, unsigned int irq)
+{
+   int ret;
+
+   if (irq == IRQ_NOTCONNECTED)
+   return -ENOTCONN;
+
+   tidss_irq_preinstall(ddev);
+
+   ret = request_irq(irq, tidss_irq_handler, 0, ddev->driver->name, ddev);
+   if (ret)
+   return ret;
+
+   tidss_irq_postinstall(ddev);
 
return 0;
 }
@@ -140,4 +159,6 @@ void tidss_irq_uninstall(struct drm_device *ddev)
tidss_runtime_get(tidss);
dispc_set_irqenable(tidss->dispc, 0);
tidss_runtime_put(tidss);
+
+   free_irq(tidss->irq, ddev);
 }
diff --git a/drivers/gpu/drm/tidss/tidss_irq.h 
b/drivers/gpu/drm/tidss/tidss_irq.h
index 4aaad5dfd7c2..b512614d5863 100644
--- 

[PATCH v2 08/14] drm/mxsfb: Convert to Linux IRQ interfaces

[Thomas Zimmermann]

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.

DRM IRQ callbacks are now being called directly or inlined.

Calls to platform_get_irq() can fail with a negative errno code.
Abort initialization in this case. The DRM IRQ midlayer does not
handle this case correctly.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c | 81 +++
 drivers/gpu/drm/mxsfb/mxsfb_drv.h |  2 +
 2 files changed, 52 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c 
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index c277d3f61a5e..ec0432fe1bdf 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -24,7 +24,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -153,6 +152,49 @@ static int mxsfb_attach_bridge(struct mxsfb_drm_private 
*mxsfb)
return 0;
 }
 
+static irqreturn_t mxsfb_irq_handler(int irq, void *data)
+{
+   struct drm_device *drm = data;
+   struct mxsfb_drm_private *mxsfb = drm->dev_private;
+   u32 reg;
+
+   reg = readl(mxsfb->base + LCDC_CTRL1);
+
+   if (reg & CTRL1_CUR_FRAME_DONE_IRQ)
+   drm_crtc_handle_vblank(>crtc);
+
+   writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
+
+   return IRQ_HANDLED;
+}
+
+static void mxsfb_irq_disable(struct drm_device *drm)
+{
+   struct mxsfb_drm_private *mxsfb = drm->dev_private;
+
+   mxsfb_enable_axi_clk(mxsfb);
+   mxsfb->crtc.funcs->disable_vblank(>crtc);
+   mxsfb_disable_axi_clk(mxsfb);
+}
+
+static int mxsfb_irq_install(struct drm_device *dev, int irq)
+{
+   if (irq == IRQ_NOTCONNECTED)
+   return -ENOTCONN;
+
+   mxsfb_irq_disable(dev);
+
+   return request_irq(irq, mxsfb_irq_handler, 0,  dev->driver->name, dev);
+}
+
+static void mxsfb_irq_uninstall(struct drm_device *dev)
+{
+   struct mxsfb_drm_private *mxsfb = dev->dev_private;
+
+   mxsfb_irq_disable(dev);
+   free_irq(mxsfb->irq, dev);
+}
+
 static int mxsfb_load(struct drm_device *drm,
  const struct mxsfb_devdata *devdata)
 {
@@ -226,8 +268,13 @@ static int mxsfb_load(struct drm_device *drm,
 
drm_mode_config_reset(drm);
 
+   ret = platform_get_irq(pdev, 0);
+   if (ret < 0)
+   goto err_vblank;
+   mxsfb->irq = ret;
+
pm_runtime_get_sync(drm->dev);
-   ret = drm_irq_install(drm, platform_get_irq(pdev, 0));
+   ret = mxsfb_irq_install(drm, mxsfb->irq);
pm_runtime_put_sync(drm->dev);
 
if (ret < 0) {
@@ -255,7 +302,7 @@ static void mxsfb_unload(struct drm_device *drm)
drm_mode_config_cleanup(drm);
 
pm_runtime_get_sync(drm->dev);
-   drm_irq_uninstall(drm);
+   mxsfb_irq_uninstall(drm);
pm_runtime_put_sync(drm->dev);
 
drm->dev_private = NULL;
@@ -263,38 +310,10 @@ static void mxsfb_unload(struct drm_device *drm)
pm_runtime_disable(drm->dev);
 }
 
-static void mxsfb_irq_disable(struct drm_device *drm)
-{
-   struct mxsfb_drm_private *mxsfb = drm->dev_private;
-
-   mxsfb_enable_axi_clk(mxsfb);
-   mxsfb->crtc.funcs->disable_vblank(>crtc);
-   mxsfb_disable_axi_clk(mxsfb);
-}
-
-static irqreturn_t mxsfb_irq_handler(int irq, void *data)
-{
-   struct drm_device *drm = data;
-   struct mxsfb_drm_private *mxsfb = drm->dev_private;
-   u32 reg;
-
-   reg = readl(mxsfb->base + LCDC_CTRL1);
-
-   if (reg & CTRL1_CUR_FRAME_DONE_IRQ)
-   drm_crtc_handle_vblank(>crtc);
-
-   writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
-
-   return IRQ_HANDLED;
-}
-
 DEFINE_DRM_GEM_CMA_FOPS(fops);
 
 static const struct drm_driver mxsfb_driver = {
.driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
-   .irq_handler= mxsfb_irq_handler,
-   .irq_preinstall = mxsfb_irq_disable,
-   .irq_uninstall  = mxsfb_irq_disable,
DRM_GEM_CMA_DRIVER_OPS,
.fops   = ,
.name   = "mxsfb-drm",
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h 
b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
index 7c720e226fdf..ddb5b0417a82 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
@@ -33,6 +33,8 @@ struct mxsfb_drm_private {
struct clk  *clk_axi;
struct clk  *clk_disp_axi;
 
+   unsigned intirq;
+
struct drm_device   *drm;
struct {
struct drm_planeprimary;
-- 
2.32.0

[PATCH v2 05/14] drm/gma500: Convert to Linux IRQ interfaces

[Thomas Zimmermann]

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it. DRM IRQ callbacks are now being called
directly or inlined.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/gma500/power.c   |  1 +
 drivers/gpu/drm/gma500/psb_drv.c |  8 ++--
 drivers/gpu/drm/gma500/psb_drv.h |  5 -
 drivers/gpu/drm/gma500/psb_irq.c | 26 --
 drivers/gpu/drm/gma500/psb_irq.h |  4 ++--
 5 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/gma500/power.c b/drivers/gpu/drm/gma500/power.c
index f07641dfa5a4..20ace6010f9f 100644
--- a/drivers/gpu/drm/gma500/power.c
+++ b/drivers/gpu/drm/gma500/power.c
@@ -32,6 +32,7 @@
 #include "psb_drv.h"
 #include "psb_reg.h"
 #include "psb_intel_reg.h"
+#include "psb_irq.h"
 #include 
 #include 
 
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 3850842d58f3..58bce1a60a4d 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -33,6 +32,7 @@
 #include "power.h"
 #include "psb_drv.h"
 #include "psb_intel_reg.h"
+#include "psb_irq.h"
 #include "psb_reg.h"
 
 static const struct drm_driver driver;
@@ -380,7 +380,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned 
long flags)
PSB_WVDC32(0x, PSB_INT_MASK_R);
spin_unlock_irqrestore(_priv->irqmask_lock, irqflags);
 
-   drm_irq_install(dev, pdev->irq);
+   psb_irq_install(dev, pdev->irq);
 
dev->max_vblank_count = 0xff; /* only 24 bits of frame count */
 
@@ -515,10 +515,6 @@ static const struct drm_driver driver = {
.lastclose = drm_fb_helper_lastclose,
 
.num_ioctls = ARRAY_SIZE(psb_ioctls),
-   .irq_preinstall = psb_irq_preinstall,
-   .irq_postinstall = psb_irq_postinstall,
-   .irq_uninstall = psb_irq_uninstall,
-   .irq_handler = psb_irq_handler,
 
.dumb_create = psb_gem_dumb_create,
.ioctls = psb_ioctls,
diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h
index d6e7c2c2c947..f2bae270ca7b 100644
--- a/drivers/gpu/drm/gma500/psb_drv.h
+++ b/drivers/gpu/drm/gma500/psb_drv.h
@@ -624,11 +624,6 @@ static inline struct drm_psb_private *psb_priv(struct 
drm_device *dev)
 }
 
 /* psb_irq.c */
-extern irqreturn_t psb_irq_handler(int irq, void *arg);
-extern void psb_irq_preinstall(struct drm_device *dev);
-extern int psb_irq_postinstall(struct drm_device *dev);
-extern void psb_irq_uninstall(struct drm_device *dev);
-
 extern void psb_irq_uninstall_islands(struct drm_device *dev, int hw_islands);
 extern int psb_vblank_wait2(struct drm_device *dev, unsigned int *sequence);
 extern int psb_vblank_wait(struct drm_device *dev, unsigned int *sequence);
diff --git a/drivers/gpu/drm/gma500/psb_irq.c b/drivers/gpu/drm/gma500/psb_irq.c
index 104009e78487..deb1fbc1f748 100644
--- a/drivers/gpu/drm/gma500/psb_irq.c
+++ b/drivers/gpu/drm/gma500/psb_irq.c
@@ -8,6 +8,7 @@
  *
  **/
 
+#include 
 #include 
 
 #include "power.h"
@@ -222,7 +223,7 @@ static void psb_sgx_interrupt(struct drm_device *dev, u32 
stat_1, u32 stat_2)
PSB_RSGX32(PSB_CR_EVENT_HOST_CLEAR2);
 }
 
-irqreturn_t psb_irq_handler(int irq, void *arg)
+static irqreturn_t psb_irq_handler(int irq, void *arg)
 {
struct drm_device *dev = arg;
struct drm_psb_private *dev_priv = dev->dev_private;
@@ -304,7 +305,7 @@ void psb_irq_preinstall(struct drm_device *dev)
spin_unlock_irqrestore(_priv->irqmask_lock, irqflags);
 }
 
-int psb_irq_postinstall(struct drm_device *dev)
+void psb_irq_postinstall(struct drm_device *dev)
 {
struct drm_psb_private *dev_priv = dev->dev_private;
unsigned long irqflags;
@@ -332,12 +333,31 @@ int psb_irq_postinstall(struct drm_device *dev)
dev_priv->ops->hotplug_enable(dev, true);
 
spin_unlock_irqrestore(_priv->irqmask_lock, irqflags);
+}
+
+int psb_irq_install(struct drm_device *dev, unsigned int irq)
+{
+   int ret;
+
+   if (irq == IRQ_NOTCONNECTED)
+   return -ENOTCONN;
+
+   psb_irq_preinstall(dev);
+
+   /* PCI devices require shared interrupts. */
+   ret = request_irq(irq, psb_irq_handler, IRQF_SHARED, dev->driver->name, 
dev);
+   if (ret)
+   return ret;
+
+   psb_irq_postinstall(dev);
+
return 0;
 }
 
 void psb_irq_uninstall(struct drm_device *dev)
 {
struct drm_psb_private *dev_priv = dev->dev_private;
+   struct pci_dev *pdev = to_pci_dev(dev->dev);
unsigned long irqflags;
unsigned int i;
 
@@ -366,6 +386,8 @@ void psb_irq_uninstall(struct drm_device *dev)
/* This register is safe even if display island is off */
PSB_WVDC32(PSB_RVDC32(PSB_INT_IDENTITY_R), PSB_INT_IDENTITY_R);
   

[PATCH v2 07/14] drm/msm: Convert to Linux IRQ interfaces

[Thomas Zimmermann]

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.

DRM IRQ callbacks are now being called directly or inlined.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/msm/msm_drv.c | 113 --
 drivers/gpu/drm/msm/msm_kms.h |   2 +-
 2 files changed, 69 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 1594ae39d54f..a332b09a5a11 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -14,7 +14,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -201,6 +200,71 @@ void msm_rmw(void __iomem *addr, u32 mask, u32 or)
msm_writel(val | or, addr);
 }
 
+static irqreturn_t msm_irq(int irq, void *arg)
+{
+   struct drm_device *dev = arg;
+   struct msm_drm_private *priv = dev->dev_private;
+   struct msm_kms *kms = priv->kms;
+
+   BUG_ON(!kms);
+
+   return kms->funcs->irq(kms);
+}
+
+static void msm_irq_preinstall(struct drm_device *dev)
+{
+   struct msm_drm_private *priv = dev->dev_private;
+   struct msm_kms *kms = priv->kms;
+
+   BUG_ON(!kms);
+
+   kms->funcs->irq_preinstall(kms);
+}
+
+static int msm_irq_postinstall(struct drm_device *dev)
+{
+   struct msm_drm_private *priv = dev->dev_private;
+   struct msm_kms *kms = priv->kms;
+
+   BUG_ON(!kms);
+
+   if (kms->funcs->irq_postinstall)
+   return kms->funcs->irq_postinstall(kms);
+
+   return 0;
+}
+
+static int msm_irq_install(struct drm_device *dev, unsigned int irq)
+{
+   int ret;
+
+   if (irq == IRQ_NOTCONNECTED)
+   return -ENOTCONN;
+
+   msm_irq_preinstall(dev);
+
+   ret = request_irq(irq, msm_irq, 0, dev->driver->name, dev);
+   if (ret)
+   return ret;
+
+   ret = msm_irq_postinstall(dev);
+   if (ret) {
+   free_irq(irq, dev);
+   return ret;
+   }
+
+   return 0;
+}
+
+static void msm_irq_uninstall(struct drm_device *dev)
+{
+   struct msm_drm_private *priv = dev->dev_private;
+   struct msm_kms *kms = priv->kms;
+
+   kms->funcs->irq_uninstall(kms);
+   free_irq(kms->irq, dev);
+}
+
 struct msm_vblank_work {
struct work_struct work;
int crtc_id;
@@ -265,7 +329,7 @@ static int msm_drm_uninit(struct device *dev)
}
 
/* We must cancel and cleanup any pending vblank enable/disable
-* work before drm_irq_uninstall() to avoid work re-enabling an
+* work before msm_irq_uninstall() to avoid work re-enabling an
 * irq after uninstall has disabled it.
 */
 
@@ -294,7 +358,7 @@ static int msm_drm_uninit(struct device *dev)
drm_mode_config_cleanup(ddev);
 
pm_runtime_get_sync(dev);
-   drm_irq_uninstall(ddev);
+   msm_irq_uninstall(ddev);
pm_runtime_put_sync(dev);
 
if (kms && kms->funcs)
@@ -553,7 +617,7 @@ static int msm_drm_init(struct device *dev, const struct 
drm_driver *drv)
 
if (kms) {
pm_runtime_get_sync(dev);
-   ret = drm_irq_install(ddev, kms->irq);
+   ret = msm_irq_install(ddev, kms->irq);
pm_runtime_put_sync(dev);
if (ret < 0) {
DRM_DEV_ERROR(dev, "failed to install IRQ handler\n");
@@ -662,43 +726,6 @@ static void msm_postclose(struct drm_device *dev, struct 
drm_file *file)
context_close(ctx);
 }
 
-static irqreturn_t msm_irq(int irq, void *arg)
-{
-   struct drm_device *dev = arg;
-   struct msm_drm_private *priv = dev->dev_private;
-   struct msm_kms *kms = priv->kms;
-   BUG_ON(!kms);
-   return kms->funcs->irq(kms);
-}
-
-static void msm_irq_preinstall(struct drm_device *dev)
-{
-   struct msm_drm_private *priv = dev->dev_private;
-   struct msm_kms *kms = priv->kms;
-   BUG_ON(!kms);
-   kms->funcs->irq_preinstall(kms);
-}
-
-static int msm_irq_postinstall(struct drm_device *dev)
-{
-   struct msm_drm_private *priv = dev->dev_private;
-   struct msm_kms *kms = priv->kms;
-   BUG_ON(!kms);
-
-   if (kms->funcs->irq_postinstall)
-   return kms->funcs->irq_postinstall(kms);
-
-   return 0;
-}
-
-static void msm_irq_uninstall(struct drm_device *dev)
-{
-   struct msm_drm_private *priv = dev->dev_private;
-   struct msm_kms *kms = priv->kms;
-   BUG_ON(!kms);
-   kms->funcs->irq_uninstall(kms);
-}
-
 int msm_crtc_enable_vblank(struct drm_crtc *crtc)
 {
struct drm_device *dev = crtc->dev;
@@ -1051,10 +1078,6 @@ static const struct drm_driver msm_driver = {
.open   = msm_open,
.postclose   = msm_postclose,
.lastclose  = drm_fb_helper_lastclose,
-   .irq_handler= msm_irq,
-   .irq_preinstall = msm_irq_preinstall,
-   

[PATCH v2 02/14] drm/arm/hdlcd: Convert to Linux IRQ interfaces

[Thomas Zimmermann]

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.

DRM IRQ callbacks are now being called directly or inlined.

Calls to platform_get_irq() can fail with a negative errno code.
Abort initialization in this case. The DRM IRQ midlayer does not
handle this case correctly.

v2:
* name struct drm_device variables 'drm' (Sam)

Signed-off-by: Thomas Zimmermann 
Acked-by: Sam Ravnborg 
---
 drivers/gpu/drm/arm/hdlcd_drv.c | 174 ++--
 drivers/gpu/drm/arm/hdlcd_drv.h |   1 +
 2 files changed, 97 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index 81ae92390736..479c2422a2e0 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -29,7 +29,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -38,6 +37,94 @@
 #include "hdlcd_drv.h"
 #include "hdlcd_regs.h"
 
+static irqreturn_t hdlcd_irq(int irq, void *arg)
+{
+   struct drm_device *drm = arg;
+   struct hdlcd_drm_private *hdlcd = drm->dev_private;
+   unsigned long irq_status;
+
+   irq_status = hdlcd_read(hdlcd, HDLCD_REG_INT_STATUS);
+
+#ifdef CONFIG_DEBUG_FS
+   if (irq_status & HDLCD_INTERRUPT_UNDERRUN)
+   atomic_inc(>buffer_underrun_count);
+
+   if (irq_status & HDLCD_INTERRUPT_DMA_END)
+   atomic_inc(>dma_end_count);
+
+   if (irq_status & HDLCD_INTERRUPT_BUS_ERROR)
+   atomic_inc(>bus_error_count);
+
+   if (irq_status & HDLCD_INTERRUPT_VSYNC)
+   atomic_inc(>vsync_count);
+
+#endif
+   if (irq_status & HDLCD_INTERRUPT_VSYNC)
+   drm_crtc_handle_vblank(>crtc);
+
+   /* acknowledge interrupt(s) */
+   hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, irq_status);
+
+   return IRQ_HANDLED;
+}
+
+static void hdlcd_irq_preinstall(struct drm_device *drm)
+{
+   struct hdlcd_drm_private *hdlcd = drm->dev_private;
+   /* Ensure interrupts are disabled */
+   hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, 0);
+   hdlcd_write(hdlcd, HDLCD_REG_INT_CLEAR, ~0);
+}
+
+static void hdlcd_irq_postinstall(struct drm_device *drm)
+{
+#ifdef CONFIG_DEBUG_FS
+   struct hdlcd_drm_private *hdlcd = drm->dev_private;
+   unsigned long irq_mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK);
+
+   /* enable debug interrupts */
+   irq_mask |= HDLCD_DEBUG_INT_MASK;
+
+   hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask);
+#endif
+}
+
+static int hdlcd_irq_install(struct drm_device *drm, int irq)
+{
+   int ret;
+
+   if (irq == IRQ_NOTCONNECTED)
+   return -ENOTCONN;
+
+   hdlcd_irq_preinstall(drm);
+
+   ret = request_irq(irq, hdlcd_irq, 0, drm->driver->name, drm);
+   if (ret)
+   return ret;
+
+   hdlcd_irq_postinstall(drm);
+
+   return 0;
+}
+
+static void hdlcd_irq_uninstall(struct drm_device *drm)
+{
+   struct hdlcd_drm_private *hdlcd = drm->dev_private;
+   /* disable all the interrupts that we might have enabled */
+   unsigned long irq_mask = hdlcd_read(hdlcd, HDLCD_REG_INT_MASK);
+
+#ifdef CONFIG_DEBUG_FS
+   /* disable debug interrupts */
+   irq_mask &= ~HDLCD_DEBUG_INT_MASK;
+#endif
+
+   /* disable vsync interrupts */
+   irq_mask &= ~HDLCD_INTERRUPT_VSYNC;
+   hdlcd_write(hdlcd, HDLCD_REG_INT_MASK, irq_mask);
+
+   free_irq(hdlcd->irq, drm);
+}
+
 static int hdlcd_load(struct drm_device *drm, unsigned long flags)
 {
struct hdlcd_drm_private *hdlcd = drm->dev_private;
@@ -90,7 +177,12 @@ static int hdlcd_load(struct drm_device *drm, unsigned long 
flags)
goto setup_fail;
}
 
-   ret = drm_irq_install(drm, platform_get_irq(pdev, 0));
+   ret = platform_get_irq(pdev, 0);
+   if (ret < 0)
+   goto irq_fail;
+   hdlcd->irq = ret;
+
+   ret = hdlcd_irq_install(drm, hdlcd->irq);
if (ret < 0) {
DRM_ERROR("failed to install IRQ handler\n");
goto irq_fail;
@@ -122,76 +214,6 @@ static void hdlcd_setup_mode_config(struct drm_device *drm)
drm->mode_config.funcs = _mode_config_funcs;
 }
 
-static irqreturn_t hdlcd_irq(int irq, void *arg)
-{
-   struct drm_device *drm = arg;
-   struct hdlcd_drm_private *hdlcd = drm->dev_private;
-   unsigned long irq_status;
-
-   irq_status = hdlcd_read(hdlcd, HDLCD_REG_INT_STATUS);
-
-#ifdef CONFIG_DEBUG_FS
-   if (irq_status & HDLCD_INTERRUPT_UNDERRUN)
-   atomic_inc(>buffer_underrun_count);
-
-   if (irq_status & HDLCD_INTERRUPT_DMA_END)
-   atomic_inc(>dma_end_count);
-
-   if (irq_status & HDLCD_INTERRUPT_BUS_ERROR)
-   atomic_inc(>bus_error_count);
-
-   if (irq_status & HDLCD_INTERRUPT_VSYNC)
-   atomic_inc(>vsync_count);
-
-#endif
-   if (irq_status & HDLCD_INTERRUPT_VSYNC)
- 

[PATCH v2 04/14] drm/fsl-dcu: Convert to Linux IRQ interfaces

[Thomas Zimmermann]

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it. DRM IRQ callbacks are now being called
directly or inlined.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 78 +--
 1 file changed, 46 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c 
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 7528e8a2d359..660fe573db96 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -51,7 +50,7 @@ static const struct regmap_config fsl_dcu_regmap_config = {
.volatile_reg = fsl_dcu_drm_is_volatile_reg,
 };
 
-static void fsl_dcu_irq_uninstall(struct drm_device *dev)
+static void fsl_dcu_irq_reset(struct drm_device *dev)
 {
struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
 
@@ -59,6 +58,45 @@ static void fsl_dcu_irq_uninstall(struct drm_device *dev)
regmap_write(fsl_dev->regmap, DCU_INT_MASK, ~0);
 }
 
+static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
+{
+   struct drm_device *dev = arg;
+   struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+   unsigned int int_status;
+   int ret;
+
+   ret = regmap_read(fsl_dev->regmap, DCU_INT_STATUS, _status);
+   if (ret) {
+   dev_err(dev->dev, "read DCU_INT_STATUS failed\n");
+   return IRQ_NONE;
+   }
+
+   if (int_status & DCU_INT_STATUS_VBLANK)
+   drm_handle_vblank(dev, 0);
+
+   regmap_write(fsl_dev->regmap, DCU_INT_STATUS, int_status);
+
+   return IRQ_HANDLED;
+}
+
+static int fsl_dcu_irq_install(struct drm_device *dev, unsigned int irq)
+{
+   if (irq == IRQ_NOTCONNECTED)
+   return -ENOTCONN;
+
+   fsl_dcu_irq_reset(dev);
+
+   return request_irq(irq, fsl_dcu_drm_irq, 0, dev->driver->name, dev);
+}
+
+static void fsl_dcu_irq_uninstall(struct drm_device *dev)
+{
+   struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+
+   fsl_dcu_irq_reset(dev);
+   free_irq(fsl_dev->irq, dev);
+}
+
 static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
 {
struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
@@ -73,13 +111,13 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned 
long flags)
ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
if (ret < 0) {
dev_err(dev->dev, "failed to initialize vblank\n");
-   goto done;
+   goto done_vblank;
}
 
-   ret = drm_irq_install(dev, fsl_dev->irq);
+   ret = fsl_dcu_irq_install(dev, fsl_dev->irq);
if (ret < 0) {
dev_err(dev->dev, "failed to install IRQ handler\n");
-   goto done;
+   goto done_irq;
}
 
if (legacyfb_depth != 16 && legacyfb_depth != 24 &&
@@ -90,11 +128,11 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned 
long flags)
}
 
return 0;
-done:
+done_irq:
drm_kms_helper_poll_fini(dev);
 
drm_mode_config_cleanup(dev);
-   drm_irq_uninstall(dev);
+done_vblank:
dev->dev_private = NULL;
 
return ret;
@@ -106,41 +144,17 @@ static void fsl_dcu_unload(struct drm_device *dev)
drm_kms_helper_poll_fini(dev);
 
drm_mode_config_cleanup(dev);
-   drm_irq_uninstall(dev);
+   fsl_dcu_irq_uninstall(dev);
 
dev->dev_private = NULL;
 }
 
-static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
-{
-   struct drm_device *dev = arg;
-   struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
-   unsigned int int_status;
-   int ret;
-
-   ret = regmap_read(fsl_dev->regmap, DCU_INT_STATUS, _status);
-   if (ret) {
-   dev_err(dev->dev, "read DCU_INT_STATUS failed\n");
-   return IRQ_NONE;
-   }
-
-   if (int_status & DCU_INT_STATUS_VBLANK)
-   drm_handle_vblank(dev, 0);
-
-   regmap_write(fsl_dev->regmap, DCU_INT_STATUS, int_status);
-
-   return IRQ_HANDLED;
-}
-
 DEFINE_DRM_GEM_CMA_FOPS(fsl_dcu_drm_fops);
 
 static const struct drm_driver fsl_dcu_drm_driver = {
.driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
.load   = fsl_dcu_load,
.unload = fsl_dcu_unload,
-   .irq_handler= fsl_dcu_drm_irq,
-   .irq_preinstall = fsl_dcu_irq_uninstall,
-   .irq_uninstall  = fsl_dcu_irq_uninstall,
DRM_GEM_CMA_DRIVER_OPS,
.fops   = _dcu_drm_fops,
.name   = "fsl-dcu-drm",
-- 
2.32.0

[PATCH v2 03/14] drm/atmel-hlcdc: Convert to Linux IRQ interfaces

[Thomas Zimmermann]

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it. DRM IRQ callbacks are now being called
directly or inlined.

v2:
* use managed release via devm_request_irq() (Sam)
* drop extra test for irq != IRQ_NOTCONNECTED (Sam)

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Sam Ravnborg 
Tested-by: Dan Sneddon 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 80 
 1 file changed, 47 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index f09b6dd8754c..1656d27b78b6 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -22,7 +22,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -557,6 +556,51 @@ static irqreturn_t atmel_hlcdc_dc_irq_handler(int irq, 
void *data)
return IRQ_HANDLED;
 }
 
+static void atmel_hlcdc_dc_irq_postinstall(struct drm_device *dev)
+{
+   struct atmel_hlcdc_dc *dc = dev->dev_private;
+   unsigned int cfg = 0;
+   int i;
+
+   /* Enable interrupts on activated layers */
+   for (i = 0; i < ATMEL_HLCDC_MAX_LAYERS; i++) {
+   if (dc->layers[i])
+   cfg |= ATMEL_HLCDC_LAYER_STATUS(i);
+   }
+
+   regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IER, cfg);
+}
+
+static void atmel_hlcdc_dc_irq_disable(struct drm_device *dev)
+{
+   struct atmel_hlcdc_dc *dc = dev->dev_private;
+   unsigned int isr;
+
+   regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IDR, 0x);
+   regmap_read(dc->hlcdc->regmap, ATMEL_HLCDC_ISR, );
+}
+
+static int atmel_hlcdc_dc_irq_install(struct drm_device *dev, unsigned int irq)
+{
+   int ret;
+
+   atmel_hlcdc_dc_irq_disable(dev);
+
+   ret = devm_request_irq(dev->dev, irq, atmel_hlcdc_dc_irq_handler, 0,
+  dev->driver->name, dev);
+   if (ret)
+   return ret;
+
+   atmel_hlcdc_dc_irq_postinstall(dev);
+
+   return 0;
+}
+
+static void atmel_hlcdc_dc_irq_uninstall(struct drm_device *dev)
+{
+   atmel_hlcdc_dc_irq_disable(dev);
+}
+
 static const struct drm_mode_config_funcs mode_config_funcs = {
.fb_create = drm_gem_fb_create,
.atomic_check = drm_atomic_helper_check,
@@ -647,7 +691,7 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
drm_mode_config_reset(dev);
 
pm_runtime_get_sync(dev->dev);
-   ret = drm_irq_install(dev, dc->hlcdc->irq);
+   ret = atmel_hlcdc_dc_irq_install(dev, dc->hlcdc->irq);
pm_runtime_put_sync(dev->dev);
if (ret < 0) {
dev_err(dev->dev, "failed to install IRQ handler\n");
@@ -676,7 +720,7 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev)
drm_mode_config_cleanup(dev);
 
pm_runtime_get_sync(dev->dev);
-   drm_irq_uninstall(dev);
+   atmel_hlcdc_dc_irq_uninstall(dev);
pm_runtime_put_sync(dev->dev);
 
dev->dev_private = NULL;
@@ -685,40 +729,10 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev)
clk_disable_unprepare(dc->hlcdc->periph_clk);
 }
 
-static int atmel_hlcdc_dc_irq_postinstall(struct drm_device *dev)
-{
-   struct atmel_hlcdc_dc *dc = dev->dev_private;
-   unsigned int cfg = 0;
-   int i;
-
-   /* Enable interrupts on activated layers */
-   for (i = 0; i < ATMEL_HLCDC_MAX_LAYERS; i++) {
-   if (dc->layers[i])
-   cfg |= ATMEL_HLCDC_LAYER_STATUS(i);
-   }
-
-   regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IER, cfg);
-
-   return 0;
-}
-
-static void atmel_hlcdc_dc_irq_uninstall(struct drm_device *dev)
-{
-   struct atmel_hlcdc_dc *dc = dev->dev_private;
-   unsigned int isr;
-
-   regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IDR, 0x);
-   regmap_read(dc->hlcdc->regmap, ATMEL_HLCDC_ISR, );
-}
-
 DEFINE_DRM_GEM_CMA_FOPS(fops);
 
 static const struct drm_driver atmel_hlcdc_dc_driver = {
.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
-   .irq_handler = atmel_hlcdc_dc_irq_handler,
-   .irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
-   .irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
-   .irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
DRM_GEM_CMA_DRIVER_OPS,
.fops = ,
.name = "atmel-hlcdc",
-- 
2.32.0

[PATCH v2 01/14] drm/amdgpu: Convert to Linux IRQ interfaces

[Thomas Zimmermann]

Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.

DRM IRQ callbacks are now being called directly or inlined.

The interrupt number returned by pci_msi_vector() is now stored
in struct amdgpu_irq. Calls to pci_msi_vector() can fail and return
a negative errno code. Abort initlaizaton in thi case. The DRM IRQ
midlayer does not handle this correctly.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Alex Deucher 
Acked-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 21 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h |  2 +-
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index d637b0536f84..2b0b0e8a6acb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1777,7 +1777,6 @@ static const struct drm_driver amdgpu_kms_driver = {
.open = amdgpu_driver_open_kms,
.postclose = amdgpu_driver_postclose_kms,
.lastclose = amdgpu_driver_lastclose_kms,
-   .irq_handler = amdgpu_irq_handler,
.ioctls = amdgpu_ioctls_kms,
.num_ioctls = ARRAY_SIZE(amdgpu_ioctls_kms),
.dumb_create = amdgpu_mode_dumb_create,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 7dfdabe1cdf9..3ac39b44a211 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -46,7 +46,6 @@
 #include 
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -184,7 +183,7 @@ void amdgpu_irq_disable_all(struct amdgpu_device *adev)
  * Returns:
  * result of handling the IRQ, as defined by _t
  */
-irqreturn_t amdgpu_irq_handler(int irq, void *arg)
+static irqreturn_t amdgpu_irq_handler(int irq, void *arg)
 {
struct drm_device *dev = (struct drm_device *) arg;
struct amdgpu_device *adev = drm_to_adev(dev);
@@ -307,6 +306,7 @@ static void amdgpu_restore_msix(struct amdgpu_device *adev)
 int amdgpu_irq_init(struct amdgpu_device *adev)
 {
int r = 0;
+   unsigned int irq;
 
spin_lock_init(>irq.lock);
 
@@ -349,15 +349,22 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
INIT_WORK(>irq.ih2_work, amdgpu_irq_handle_ih2);
INIT_WORK(>irq.ih_soft_work, amdgpu_irq_handle_ih_soft);
 
-   adev->irq.installed = true;
-   /* Use vector 0 for MSI-X */
-   r = drm_irq_install(adev_to_drm(adev), pci_irq_vector(adev->pdev, 0));
+   /* Use vector 0 for MSI-X. */
+   r = pci_irq_vector(adev->pdev, 0);
+   if (r < 0)
+   return r;
+   irq = r;
+
+   /* PCI devices require shared interrupts. */
+   r = request_irq(irq, amdgpu_irq_handler, IRQF_SHARED, 
adev_to_drm(adev)->driver->name,
+   adev_to_drm(adev));
if (r) {
-   adev->irq.installed = false;
if (!amdgpu_device_has_dc_support(adev))
flush_work(>hotplug_work);
return r;
}
+   adev->irq.installed = true;
+   adev->irq.irq = irq;
adev_to_drm(adev)->max_vblank_count = 0x00ff;
 
DRM_DEBUG("amdgpu: irq initialized.\n");
@@ -368,7 +375,7 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
 void amdgpu_irq_fini_hw(struct amdgpu_device *adev)
 {
if (adev->irq.installed) {
-   drm_irq_uninstall(>ddev);
+   free_irq(adev->irq.irq, adev_to_drm(adev));
adev->irq.installed = false;
if (adev->irq.msi_enabled)
pci_free_irq_vectors(adev->pdev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
index 78ad4784cc74..e9f2c11ea416 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
@@ -80,6 +80,7 @@ struct amdgpu_irq_src_funcs {
 
 struct amdgpu_irq {
boolinstalled;
+   unsigned intirq;
spinlock_t  lock;
/* interrupt sources */
struct amdgpu_irq_clientclient[AMDGPU_IRQ_CLIENTID_MAX];
@@ -100,7 +101,6 @@ struct amdgpu_irq {
 };
 
 void amdgpu_irq_disable_all(struct amdgpu_device *adev);
-irqreturn_t amdgpu_irq_handler(int irq, void *arg);
 
 int amdgpu_irq_init(struct amdgpu_device *adev);
 void amdgpu_irq_fini_sw(struct amdgpu_device *adev);
-- 
2.32.0

[PATCH v2 00/14] drm: Make DRM's IRQ helpers legacy

[Thomas Zimmermann]

DRM's IRQ helpers are only helpful for old, non-KMS drivers. Move
the code behind CONFIG_DRM_LEGACY. Convert KMS drivers to Linux
IRQ interfaces.

DRM provides IRQ helpers for setting up, receiving and removing IRQ
handlers. It's an abstraction over plain Linux functions. The code
is mid-layerish with several callbacks to hook into the rsp drivers.
Old UMS driver have their interrupts enabled via ioctl, so these
abstractions makes some sense. Modern KMS manage all their interrupts
internally. Using the DRM helpers adds indirection without benefits.

Most KMS drivers already use Linux IRQ functions instead of DRM's
abstraction layer. Patches 1 to 12 convert the remaining ones.
The patches also resolve a bug for devices without assigned interrupt
number. DRM helpers don't test for IRQ_NOTCONNECTED, so drivers do
not detect if the device has no interrupt assigned.

Patch 13 removes an unused function.

Patch 14 moves the DRM IRQ helpers behind CONFIG_DRM_LEGACY. Only
the old non-KMS drivers still use the functionality.

v2:
* drop IRQ_NOTCONNECTED test from atmel-hlcdc (Sam)
* use devm_request_irq() in atmel-hlcdc (Sam)
* unify variable names in arm/hlcdc (Sam)

Thomas Zimmermann (14):
  drm/amdgpu: Convert to Linux IRQ interfaces
  drm/arm/hdlcd: Convert to Linux IRQ interfaces
  drm/atmel-hlcdc: Convert to Linux IRQ interfaces
  drm/fsl-dcu: Convert to Linux IRQ interfaces
  drm/gma500: Convert to Linux IRQ interfaces
  drm/kmb: Convert to Linux IRQ interfaces
  drm/msm: Convert to Linux IRQ interfaces
  drm/mxsfb: Convert to Linux IRQ interfaces
  drm/radeon: Convert to Linux IRQ interfaces
  drm/tidss: Convert to Linux IRQ interfaces
  drm/tilcdc: Convert to Linux IRQ interfaces
  drm/vc4: Convert to Linux IRQ interfaces
  drm: Remove unused devm_drm_irq_install()
  drm: IRQ midlayer is now legacy

 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  |   1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c  |  21 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h  |   2 +-
 drivers/gpu/drm/arm/hdlcd_drv.c  | 174 ++-
 drivers/gpu/drm/arm/hdlcd_drv.h  |   1 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  80 +
 drivers/gpu/drm/drm_irq.c|  95 +-
 drivers/gpu/drm/drm_legacy_misc.c|   3 +-
 drivers/gpu/drm/drm_vblank.c |   8 +-
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c|  78 +
 drivers/gpu/drm/gma500/power.c   |   1 +
 drivers/gpu/drm/gma500/psb_drv.c |   8 +-
 drivers/gpu/drm/gma500/psb_drv.h |   5 -
 drivers/gpu/drm/gma500/psb_irq.c |  26 ++-
 drivers/gpu/drm/gma500/psb_irq.h |   4 +-
 drivers/gpu/drm/i810/i810_dma.c  |   3 +-
 drivers/gpu/drm/kmb/kmb_drv.c|  26 ++-
 drivers/gpu/drm/mga/mga_dma.c|   2 +-
 drivers/gpu/drm/mga/mga_drv.h|   1 -
 drivers/gpu/drm/msm/msm_drv.c| 113 +++-
 drivers/gpu/drm/msm/msm_kms.h|   2 +-
 drivers/gpu/drm/mxsfb/mxsfb_drv.c|  81 +
 drivers/gpu/drm/mxsfb/mxsfb_drv.h|   2 +
 drivers/gpu/drm/r128/r128_cce.c  |   3 +-
 drivers/gpu/drm/radeon/radeon_drv.c  |   4 -
 drivers/gpu/drm/radeon/radeon_irq_kms.c  |  44 -
 drivers/gpu/drm/radeon/radeon_kms.h  |   4 -
 drivers/gpu/drm/tidss/tidss_drv.c|  15 +-
 drivers/gpu/drm/tidss/tidss_drv.h|   2 +
 drivers/gpu/drm/tidss/tidss_irq.c|  27 ++-
 drivers/gpu/drm/tidss/tidss_irq.h|   4 +-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  |  51 --
 drivers/gpu/drm/tilcdc/tilcdc_drv.h  |   3 +
 drivers/gpu/drm/vc4/vc4_drv.c|   4 -
 drivers/gpu/drm/vc4/vc4_drv.h|   8 +-
 drivers/gpu/drm/vc4/vc4_irq.c|  48 +++--
 drivers/gpu/drm/vc4/vc4_v3d.c|  17 +-
 drivers/gpu/drm/via/via_mm.c |   3 +-
 include/drm/drm_device.h |  18 +-
 include/drm/drm_drv.h|  44 +
 include/drm/drm_irq.h|  32 
 include/drm/drm_legacy.h |   3 +
 42 files changed, 567 insertions(+), 504 deletions(-)
 delete mode 100644 include/drm/drm_irq.h


base-commit: c9d6903562aa335593daf44b4a1edeaef6bf9206
prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
prerequisite-patch-id: c3f32630e1d2de2eb74316c930578847d4b83fb3
prerequisite-patch-id: b32ca0abfc255601f8a5052d3b88be09527dabcb
prerequisite-patch-id: 22a3f264168bacb04ef65306b32b86be8dc982ef
prerequisite-patch-id: 095a0acb604eb02956e1a7e53da41371c64eb813
prerequisite-patch-id: 7a2417d5d8d453204bd94aa873e3faae812f26fc
--
2.32.0

[PATCH] drm/amdgpu/display: fix DMUB firmware version info

[Shirish S]

DMUB firmware info is printed before it gets initialized.
Correct this order to ensure true value is conveyed.

Signed-off-by: Shirish S 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
 1 file changed, 1 insertion(+), 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 7e09b6d26a51..396a2dca2fe0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1548,6 +1548,7 @@ static int dm_dmub_sw_init(struct amdgpu_device *adev)
}
 
hdr = (const struct dmcub_firmware_header_v1_0 *)adev->dm.dmub_fw->data;
+   adev->dm.dmcub_fw_version = le32_to_cpu(hdr->header.ucode_version);
 
if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
adev->firmware.ucode[AMDGPU_UCODE_ID_DMCUB].ucode_id =
@@ -1561,7 +1562,6 @@ static int dm_dmub_sw_init(struct amdgpu_device *adev)
 adev->dm.dmcub_fw_version);
}
 
-   adev->dm.dmcub_fw_version = le32_to_cpu(hdr->header.ucode_version);
 
adev->dm.dmub_srv = kzalloc(sizeof(*adev->dm.dmub_srv), GFP_KERNEL);
dmub_srv = adev->dm.dmub_srv;
-- 
2.17.1

Re: [PATCH] drm/radeon: Update pitch for page flip

[Michel Dänzer]

On 2021-08-02 4:51 p.m., Alex Deucher wrote:
> On Mon, Aug 2, 2021 at 4:31 AM Daniel Vetter  wrote:
>>
>> On Mon, Aug 02, 2021 at 10:12:47AM +0200, Christian König wrote:
>>> Am 02.08.21 um 09:43 schrieb Zhenneng Li:
 When primary bo is updated, crtc's pitch may
 have not been updated, this will lead to show
 disorder content when user changes display mode,
 we update crtc's pitch in page flip to avoid
 this bug.
 This refers to amdgpu's pageflip.
>>>
>>> Alex is the expert to ask about that code, but I'm not sure if that is
>>> really correct for the old hardware.
>>>
>>> As far as I know the crtc's pitch should not change during a page flip, but
>>> only during a full mode set.
>>>
>>> So could you elaborate a bit more how you trigger this?
>>
>> legacy page_flip ioctl only verifies that the fb->format stays the same.
>> It doesn't check anything else (afair never has), this is all up to
>> drivers to verify.
>>
>> Personally I'd say add a check to reject this, since testing this and
>> making sure it really works everywhere is probably a bit much on this old
>> hw.
> 
> If just the pitch changed, that probably wouldn't be much of a
> problem, but if the pitch is changing, that probably implies other
> stuff has changed as well and we'll just be chasing changes.  I agree
> it would be best to just reject anything other than updating the
> scanout address.

FWIW, that means page flipping cannot be used in some cases which work fine by 
changing the pitch, which can result in tearing: 
https://gitlab.freedesktop.org/xorg/xserver/-/issues/839 (which says the i915 
driver handles this as well).


-- 
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer

RE: [PATCH] drm/amd/amdgpu: remove redundant host to psp cmd buf

[Chen, Guchun]

[Public]

Yeah, you are right, Lijo. @Li, Candice @Clements, John please address this 
before submitting this patch.

Regards,
Guchun

-Original Message-
From: Lazar, Lijo  
Sent: Tuesday, August 3, 2021 4:16 PM
To: Chen, Guchun ; Li, Candice ; 
amd-gfx@lists.freedesktop.org
Cc: Clements, John 
Subject: Re: [PATCH] drm/amd/amdgpu: remove redundant host to psp cmd buf

Not really. Below clearing and changing the buffer happens outside of the 
mutex, so it's not fine till the memcpy happens in psp_cmd_submit_buf.

memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));


Thanks,
Lijo


On 8/3/2021 1:37 PM, Chen, Guchun wrote:
> [Public]
> 
> In psp_cmd_submit_buf, it has psp->mutex to guard this, so it should be fine.
> 
> Regards,
> Guchun
> 
> -Original Message-
> From: amd-gfx  On Behalf Of 
> Lazar, Lijo
> Sent: Tuesday, August 3, 2021 2:30 PM
> To: Li, Candice ; amd-gfx@lists.freedesktop.org
> Cc: Clements, John 
> Subject: Re: [PATCH] drm/amd/amdgpu: remove redundant host to psp cmd 
> buf
> 
> Command buffer in psp context means different command buffers cannot be 
> prepared in parallel. Any case of submitting commands for different TAs in 
> parallel - ex: for RAS and some other TA?
> 
> Thanks,
> Lijo
> 
> On 8/3/2021 8:35 AM, Li, Candice wrote:
>> [Public]
>>
>>
>> Signed-off-by: Candice Li candice...@amd.com 
>> 
>>
>> ---
>>
>> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 253 
>> 
>>
>> 1 file changed, 78 insertions(+), 175 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>
>> index ed731144ca7f..9c18558e3bc0 100644
>>
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>
>> @@ -253,6 +253,12 @@ static int psp_sw_init(void *handle)
>>
>>    struct psp_runtime_boot_cfg_entry boot_cfg_entry;
>>
>>    struct psp_memory_training_context *mem_training_ctx = 
>> >mem_train_ctx;
>>
>> +    psp->cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp),
>> GFP_KERNEL);
>>
>> +    if (!psp->cmd) {
>>
>> +   DRM_ERROR("Failed to allocate memory to
>> command buffer!\n");
>>
>> +   ret = -ENOMEM;
>>
>> +    }
>>
>> +
>>
>>    if (!amdgpu_sriov_vf(adev)) {
>>
>>       ret = psp_init_microcode(psp);
>>
>>       if (ret) {
>>
>> @@ -315,25 +321,30 @@ static int psp_sw_init(void *handle)
>>
>> static int psp_sw_fini(void *handle)
>>
>> {
>>
>>    struct amdgpu_device *adev = (struct amdgpu_device 
>> *)handle;
>>
>> +    struct psp_context *psp = >psp;
>>
>> +    struct psp_gfx_cmd_resp *cmd = psp->cmd;
>>
>> - psp_memory_training_fini(>psp);
>>
>> - if (adev->psp.sos_fw) {
>>
>> -   release_firmware(adev->psp.sos_fw);
>>
>> -   adev->psp.sos_fw = NULL;
>>
>> +    psp_memory_training_fini(psp);
>>
>> +    if (psp->sos_fw) {
>>
>> +   release_firmware(psp->sos_fw);
>>
>> +   psp->sos_fw = NULL;
>>
>>    }
>>
>> - if (adev->psp.asd_fw) {
>>
>> -   release_firmware(adev->psp.asd_fw);
>>
>> +    if (psp->asd_fw) {
>>
>> +   release_firmware(psp->asd_fw);
>>
>>       adev->psp.asd_fw = NULL;
>>
>>    }
>>
>> - if (adev->psp.ta_fw) {
>>
>> -   release_firmware(adev->psp.ta_fw);
>>
>> -   adev->psp.ta_fw = NULL;
>>
>> +    if (psp->ta_fw) {
>>
>> +   release_firmware(psp->ta_fw);
>>
>> +   psp->ta_fw = NULL;
>>
>>    }
>>
>>     if (adev->asic_type == CHIP_NAVI10 ||
>>
>>    adev->asic_type == CHIP_SIENNA_CICHLID)
>>
>>       psp_sysfs_fini(adev);
>>
>> +    kfree(cmd);
>>
>> +    cmd = NULL;
>>
>> +
>>
>>    return 0;
>>
>> }
>>
>> @@ -491,6 +502,8 @@ static void psp_prep_tmr_cmd_buf(struct 
>> psp_context *psp,
>>
>>    uint32_t size = amdgpu_bo_size(tmr_bo);
>>
>>    uint64_t tmr_pa = amdgpu_gmc_vram_pa(adev, tmr_bo);
>>
>> +    memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));
>>
>> +
>>
>>    if (amdgpu_sriov_vf(psp->adev))
>>
>>       cmd->cmd_id = GFX_CMD_ID_SETUP_VMR;
>>
>>    else
>>
>> @@ -506,6 +519,8 @@ static void psp_prep_tmr_cmd_buf(struct 
>> psp_context *psp,
>>
>> static void psp_prep_load_toc_cmd_buf(struct psp_gfx_cmd_resp *cmd,
>>
>>  
>> uint64_t pri_buf_mc, uint32_t size)
>>
>> {
>>
>> +    memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));
>>
>> +
>>
>>  

[PATCH] drm/amdgpu: update PSP BL cmd IDs

[Clements, John]

[AMD Official Use Only]

Submitting patch to resolve issue with incorrect PSP BL cmd IDs


0001-drm-amdgpu-update-PSP-BL-cmd-IDs.patch
Description: 0001-drm-amdgpu-update-PSP-BL-cmd-IDs.patch

Re: [PATCH] drm/amd/amdgpu: remove redundant host to psp cmd buf

[Lazar, Lijo]

Not really. Below clearing and changing the buffer happens outside of 
the mutex, so it's not fine till the memcpy happens in psp_cmd_submit_buf.


memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));


Thanks,
Lijo


On 8/3/2021 1:37 PM, Chen, Guchun wrote:

[Public]

In psp_cmd_submit_buf, it has psp->mutex to guard this, so it should be fine.

Regards,
Guchun

-Original Message-
From: amd-gfx  On Behalf Of Lazar, Lijo
Sent: Tuesday, August 3, 2021 2:30 PM
To: Li, Candice ; amd-gfx@lists.freedesktop.org
Cc: Clements, John 
Subject: Re: [PATCH] drm/amd/amdgpu: remove redundant host to psp cmd buf

Command buffer in psp context means different command buffers cannot be 
prepared in parallel. Any case of submitting commands for different TAs in 
parallel - ex: for RAS and some other TA?

Thanks,
Lijo

On 8/3/2021 8:35 AM, Li, Candice wrote:

[Public]


Signed-off-by: Candice Li candice...@amd.com


---

drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 253 

1 file changed, 78 insertions(+), 175 deletions(-)

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

index ed731144ca7f..9c18558e3bc0 100644

--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c

+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c

@@ -253,6 +253,12 @@ static int psp_sw_init(void *handle)

   struct psp_runtime_boot_cfg_entry boot_cfg_entry;

   struct psp_memory_training_context *mem_training_ctx =
>mem_train_ctx;

+    psp->cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp),
GFP_KERNEL);

+    if (!psp->cmd) {

+   DRM_ERROR("Failed to allocate memory to
command buffer!\n");

+   ret = -ENOMEM;

+    }

+

   if (!amdgpu_sriov_vf(adev)) {

      ret = psp_init_microcode(psp);

      if (ret) {

@@ -315,25 +321,30 @@ static int psp_sw_init(void *handle)

static int psp_sw_fini(void *handle)

{

   struct amdgpu_device *adev = (struct amdgpu_device
*)handle;

+    struct psp_context *psp = >psp;

+    struct psp_gfx_cmd_resp *cmd = psp->cmd;

- psp_memory_training_fini(>psp);

- if (adev->psp.sos_fw) {

-   release_firmware(adev->psp.sos_fw);

-   adev->psp.sos_fw = NULL;

+    psp_memory_training_fini(psp);

+    if (psp->sos_fw) {

+   release_firmware(psp->sos_fw);

+   psp->sos_fw = NULL;

   }

- if (adev->psp.asd_fw) {

-   release_firmware(adev->psp.asd_fw);

+    if (psp->asd_fw) {

+   release_firmware(psp->asd_fw);

      adev->psp.asd_fw = NULL;

   }

- if (adev->psp.ta_fw) {

-   release_firmware(adev->psp.ta_fw);

-   adev->psp.ta_fw = NULL;

+    if (psp->ta_fw) {

+   release_firmware(psp->ta_fw);

+   psp->ta_fw = NULL;

   }

    if (adev->asic_type == CHIP_NAVI10 ||

   adev->asic_type == CHIP_SIENNA_CICHLID)

      psp_sysfs_fini(adev);

+    kfree(cmd);

+    cmd = NULL;

+

   return 0;

}

@@ -491,6 +502,8 @@ static void psp_prep_tmr_cmd_buf(struct
psp_context *psp,

   uint32_t size = amdgpu_bo_size(tmr_bo);

   uint64_t tmr_pa = amdgpu_gmc_vram_pa(adev, tmr_bo);

+    memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));

+

   if (amdgpu_sriov_vf(psp->adev))

      cmd->cmd_id = GFX_CMD_ID_SETUP_VMR;

   else

@@ -506,6 +519,8 @@ static void psp_prep_tmr_cmd_buf(struct
psp_context *psp,

static void psp_prep_load_toc_cmd_buf(struct psp_gfx_cmd_resp *cmd,

 
uint64_t pri_buf_mc, uint32_t size)


{

+    memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));

+

   cmd->cmd_id = GFX_CMD_ID_LOAD_TOC;

   cmd->cmd.cmd_load_toc.toc_phy_addr_lo =
lower_32_bits(pri_buf_mc);

   cmd->cmd.cmd_load_toc.toc_phy_addr_hi =
upper_32_bits(pri_buf_mc);

@@ -517,11 +532,8 @@ static int psp_load_toc(struct psp_context *psp,

    uint32_t *tmr_size)

{

   int ret;

- struct psp_gfx_cmd_resp *cmd;

+    struct psp_gfx_cmd_resp *cmd = psp->cmd;

- cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp),
GFP_KERNEL);

- if (!cmd)

-   return -ENOMEM;

   /* Copy toc to psp firmware private buffer */

   psp_copy_fw(psp, psp->toc.start_addr,
psp->toc.size_bytes);

@@ -531,7 +543,7 @@ static int psp_load_toc(struct 

RE: [PATCH] drm/amd/amdgpu: remove redundant host to psp cmd buf

[Chen, Guchun]

[Public]

In psp_cmd_submit_buf, it has psp->mutex to guard this, so it should be fine.

Regards,
Guchun

-Original Message-
From: amd-gfx  On Behalf Of Lazar, Lijo
Sent: Tuesday, August 3, 2021 2:30 PM
To: Li, Candice ; amd-gfx@lists.freedesktop.org
Cc: Clements, John 
Subject: Re: [PATCH] drm/amd/amdgpu: remove redundant host to psp cmd buf

Command buffer in psp context means different command buffers cannot be 
prepared in parallel. Any case of submitting commands for different TAs in 
parallel - ex: for RAS and some other TA?

Thanks,
Lijo

On 8/3/2021 8:35 AM, Li, Candice wrote:
> [Public]
> 
> 
> Signed-off-by: Candice Li candice...@amd.com 
> 
> 
> ---
> 
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 253 
> 
> 1 file changed, 78 insertions(+), 175 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> 
> index ed731144ca7f..9c18558e3bc0 100644
> 
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> 
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> 
> @@ -253,6 +253,12 @@ static int psp_sw_init(void *handle)
> 
>   struct psp_runtime_boot_cfg_entry boot_cfg_entry;
> 
>   struct psp_memory_training_context *mem_training_ctx = 
> >mem_train_ctx;
> 
> +    psp->cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp),
> GFP_KERNEL);
> 
> +    if (!psp->cmd) {
> 
> +   DRM_ERROR("Failed to allocate memory to
> command buffer!\n");
> 
> +   ret = -ENOMEM;
> 
> +    }
> 
> +
> 
>   if (!amdgpu_sriov_vf(adev)) {
> 
>      ret = psp_init_microcode(psp);
> 
>      if (ret) {
> 
> @@ -315,25 +321,30 @@ static int psp_sw_init(void *handle)
> 
> static int psp_sw_fini(void *handle)
> 
> {
> 
>   struct amdgpu_device *adev = (struct amdgpu_device 
> *)handle;
> 
> +    struct psp_context *psp = >psp;
> 
> +    struct psp_gfx_cmd_resp *cmd = psp->cmd;
> 
> - psp_memory_training_fini(>psp);
> 
> - if (adev->psp.sos_fw) {
> 
> -   release_firmware(adev->psp.sos_fw);
> 
> -   adev->psp.sos_fw = NULL;
> 
> +    psp_memory_training_fini(psp);
> 
> +    if (psp->sos_fw) {
> 
> +   release_firmware(psp->sos_fw);
> 
> +   psp->sos_fw = NULL;
> 
>   }
> 
> - if (adev->psp.asd_fw) {
> 
> -   release_firmware(adev->psp.asd_fw);
> 
> +    if (psp->asd_fw) {
> 
> +   release_firmware(psp->asd_fw);
> 
>      adev->psp.asd_fw = NULL;
> 
>   }
> 
> - if (adev->psp.ta_fw) {
> 
> -   release_firmware(adev->psp.ta_fw);
> 
> -   adev->psp.ta_fw = NULL;
> 
> +    if (psp->ta_fw) {
> 
> +   release_firmware(psp->ta_fw);
> 
> +   psp->ta_fw = NULL;
> 
>   }
> 
>    if (adev->asic_type == CHIP_NAVI10 ||
> 
>   adev->asic_type == CHIP_SIENNA_CICHLID)
> 
>      psp_sysfs_fini(adev);
> 
> +    kfree(cmd);
> 
> +    cmd = NULL;
> 
> +
> 
>   return 0;
> 
> }
> 
> @@ -491,6 +502,8 @@ static void psp_prep_tmr_cmd_buf(struct 
> psp_context *psp,
> 
>   uint32_t size = amdgpu_bo_size(tmr_bo);
> 
>   uint64_t tmr_pa = amdgpu_gmc_vram_pa(adev, tmr_bo);
> 
> +    memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));
> 
> +
> 
>   if (amdgpu_sriov_vf(psp->adev))
> 
>      cmd->cmd_id = GFX_CMD_ID_SETUP_VMR;
> 
>   else
> 
> @@ -506,6 +519,8 @@ static void psp_prep_tmr_cmd_buf(struct 
> psp_context *psp,
> 
> static void psp_prep_load_toc_cmd_buf(struct psp_gfx_cmd_resp *cmd,
> 
>     
> uint64_t pri_buf_mc, uint32_t size)
> 
> {
> 
> +    memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));
> 
> +
> 
>   cmd->cmd_id = GFX_CMD_ID_LOAD_TOC;
> 
>   cmd->cmd.cmd_load_toc.toc_phy_addr_lo = 
> lower_32_bits(pri_buf_mc);
> 
>   cmd->cmd.cmd_load_toc.toc_phy_addr_hi = 
> upper_32_bits(pri_buf_mc);
> 
> @@ -517,11 +532,8 @@ static int psp_load_toc(struct psp_context *psp,
> 
>    uint32_t *tmr_size)
> 
> {
> 
>   int ret;
> 
> - struct psp_gfx_cmd_resp *cmd;
> 
> +    struct psp_gfx_cmd_resp *cmd = psp->cmd;
> 
> - cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), 
> GFP_KERNEL);
> 
> - if (!cmd)
> 
> -   return -ENOMEM;
> 
>   /* Copy toc to psp firmware private buffer */
> 
>   psp_copy_fw(psp, 

RE: [PATCH] drm/amd/amdgpu: remove redundant host to psp cmd buf

[Li, Candice]

[Public]

Exactly, I forgot to change it accordingly. Thanks @Chen, 
Guchun



Thanks,
Candice

From: Chen, Guchun 
Sent: Tuesday, August 3, 2021 2:04 PM
To: Li, Candice ; amd-gfx@lists.freedesktop.org
Cc: Clements, John ; Li, Candice 
Subject: RE: [PATCH] drm/amd/amdgpu: remove redundant host to psp cmd buf


[Public]

+if (psp->asd_fw) {
+   release_firmware(psp->asd_fw);
adev->psp.asd_fw = NULL;
 }

Use "psp->asd_fw = NULL" should be more simple?

Regards,
Guchun

From: amd-gfx 
mailto:amd-gfx-boun...@lists.freedesktop.org>>
 On Behalf Of Li, Candice
Sent: Tuesday, August 3, 2021 11:06 AM
To: amd-gfx@lists.freedesktop.org
Cc: Clements, John mailto:john.cleme...@amd.com>>; Li, 
Candice mailto:candice...@amd.com>>
Subject: [PATCH] drm/amd/amdgpu: remove redundant host to psp cmd buf


[Public]

Signed-off-by: Candice Li candice...@amd.com
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 253 
1 file changed, 78 insertions(+), 175 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index ed731144ca7f..9c18558e3bc0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -253,6 +253,12 @@ static int psp_sw_init(void *handle)
 struct psp_runtime_boot_cfg_entry boot_cfg_entry;
 struct psp_memory_training_context *mem_training_ctx = 
>mem_train_ctx;

+psp->cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
+if (!psp->cmd) {
+   DRM_ERROR("Failed to allocate memory to command 
buffer!\n");
+   ret = -ENOMEM;
+}
+
 if (!amdgpu_sriov_vf(adev)) {
ret = psp_init_microcode(psp);
if (ret) {
@@ -315,25 +321,30 @@ static int psp_sw_init(void *handle)
static int psp_sw_fini(void *handle)
{
 struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+struct psp_context *psp = >psp;
+struct psp_gfx_cmd_resp *cmd = psp->cmd;

- psp_memory_training_fini(>psp);
- if (adev->psp.sos_fw) {
-   release_firmware(adev->psp.sos_fw);
-   adev->psp.sos_fw = NULL;
+psp_memory_training_fini(psp);
+if (psp->sos_fw) {
+   release_firmware(psp->sos_fw);
+   psp->sos_fw = NULL;
 }
- if (adev->psp.asd_fw) {
-   release_firmware(adev->psp.asd_fw);
+if (psp->asd_fw) {
+   release_firmware(psp->asd_fw);
adev->psp.asd_fw = NULL;
 }
- if (adev->psp.ta_fw) {
-   release_firmware(adev->psp.ta_fw);
-   adev->psp.ta_fw = NULL;
+if (psp->ta_fw) {
+   release_firmware(psp->ta_fw);
+   psp->ta_fw = NULL;
 }

  if (adev->asic_type == CHIP_NAVI10 ||
 adev->asic_type == CHIP_SIENNA_CICHLID)
psp_sysfs_fini(adev);

+kfree(cmd);
+cmd = NULL;
+
 return 0;
}

@@ -491,6 +502,8 @@ static void psp_prep_tmr_cmd_buf(struct psp_context *psp,
 uint32_t size = amdgpu_bo_size(tmr_bo);
 uint64_t tmr_pa = amdgpu_gmc_vram_pa(adev, tmr_bo);

+memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));
+
 if (amdgpu_sriov_vf(psp->adev))
cmd->cmd_id = GFX_CMD_ID_SETUP_VMR;
 else
@@ -506,6 +519,8 @@ static void psp_prep_tmr_cmd_buf(struct psp_context *psp,
static void psp_prep_load_toc_cmd_buf(struct psp_gfx_cmd_resp *cmd,
   uint64_t 
pri_buf_mc, uint32_t size)
{
+memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));
+
 cmd->cmd_id = GFX_CMD_ID_LOAD_TOC;
 cmd->cmd.cmd_load_toc.toc_phy_addr_lo = lower_32_bits(pri_buf_mc);
 cmd->cmd.cmd_load_toc.toc_phy_addr_hi = upper_32_bits(pri_buf_mc);
@@ -517,11 +532,8 @@ static int psp_load_toc(struct psp_context *psp,
  uint32_t *tmr_size)
{
 int ret;
- struct psp_gfx_cmd_resp *cmd;
+struct psp_gfx_cmd_resp *cmd = psp->cmd;

- cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
- if (!cmd)
-   return -ENOMEM;
 /* Copy toc to psp firmware private buffer */
 psp_copy_fw(psp, psp->toc.start_addr, psp->toc.size_bytes);

@@ -531,7 +543,7 @@ static int psp_load_toc(struct psp_context *psp,

RE: [PATCH] drm/amd/amdgpu: remove redundant host to psp cmd buf

[Chen, Guchun]

[Public]

+if (psp->asd_fw) {
+   release_firmware(psp->asd_fw);
adev->psp.asd_fw = NULL;
 }

Use "psp->asd_fw = NULL" should be more simple?

Regards,
Guchun

From: amd-gfx  On Behalf Of Li, Candice
Sent: Tuesday, August 3, 2021 11:06 AM
To: amd-gfx@lists.freedesktop.org
Cc: Clements, John ; Li, Candice 
Subject: [PATCH] drm/amd/amdgpu: remove redundant host to psp cmd buf


[Public]

Signed-off-by: Candice Li candice...@amd.com
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 253 
1 file changed, 78 insertions(+), 175 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index ed731144ca7f..9c18558e3bc0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -253,6 +253,12 @@ static int psp_sw_init(void *handle)
 struct psp_runtime_boot_cfg_entry boot_cfg_entry;
 struct psp_memory_training_context *mem_training_ctx = 
>mem_train_ctx;

+psp->cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
+if (!psp->cmd) {
+   DRM_ERROR("Failed to allocate memory to command 
buffer!\n");
+   ret = -ENOMEM;
+}
+
 if (!amdgpu_sriov_vf(adev)) {
ret = psp_init_microcode(psp);
if (ret) {
@@ -315,25 +321,30 @@ static int psp_sw_init(void *handle)
static int psp_sw_fini(void *handle)
{
 struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+struct psp_context *psp = >psp;
+struct psp_gfx_cmd_resp *cmd = psp->cmd;

- psp_memory_training_fini(>psp);
- if (adev->psp.sos_fw) {
-   release_firmware(adev->psp.sos_fw);
-   adev->psp.sos_fw = NULL;
+psp_memory_training_fini(psp);
+if (psp->sos_fw) {
+   release_firmware(psp->sos_fw);
+   psp->sos_fw = NULL;
 }
- if (adev->psp.asd_fw) {
-   release_firmware(adev->psp.asd_fw);
+if (psp->asd_fw) {
+   release_firmware(psp->asd_fw);
adev->psp.asd_fw = NULL;
 }
- if (adev->psp.ta_fw) {
-   release_firmware(adev->psp.ta_fw);
-   adev->psp.ta_fw = NULL;
+if (psp->ta_fw) {
+   release_firmware(psp->ta_fw);
+   psp->ta_fw = NULL;
 }

  if (adev->asic_type == CHIP_NAVI10 ||
 adev->asic_type == CHIP_SIENNA_CICHLID)
psp_sysfs_fini(adev);

+kfree(cmd);
+cmd = NULL;
+
 return 0;
}

@@ -491,6 +502,8 @@ static void psp_prep_tmr_cmd_buf(struct psp_context *psp,
 uint32_t size = amdgpu_bo_size(tmr_bo);
 uint64_t tmr_pa = amdgpu_gmc_vram_pa(adev, tmr_bo);

+memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));
+
 if (amdgpu_sriov_vf(psp->adev))
cmd->cmd_id = GFX_CMD_ID_SETUP_VMR;
 else
@@ -506,6 +519,8 @@ static void psp_prep_tmr_cmd_buf(struct psp_context *psp,
static void psp_prep_load_toc_cmd_buf(struct psp_gfx_cmd_resp *cmd,
   uint64_t 
pri_buf_mc, uint32_t size)
{
+memset(cmd, 0, sizeof(struct psp_gfx_cmd_resp));
+
 cmd->cmd_id = GFX_CMD_ID_LOAD_TOC;
 cmd->cmd.cmd_load_toc.toc_phy_addr_lo = lower_32_bits(pri_buf_mc);
 cmd->cmd.cmd_load_toc.toc_phy_addr_hi = upper_32_bits(pri_buf_mc);
@@ -517,11 +532,8 @@ static int psp_load_toc(struct psp_context *psp,
  uint32_t *tmr_size)
{
 int ret;
- struct psp_gfx_cmd_resp *cmd;
+struct psp_gfx_cmd_resp *cmd = psp->cmd;

- cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
- if (!cmd)
-   return -ENOMEM;
 /* Copy toc to psp firmware private buffer */
 psp_copy_fw(psp, psp->toc.start_addr, psp->toc.size_bytes);

@@ -531,7 +543,7 @@ static int psp_load_toc(struct psp_context *psp,
  
psp->fence_buf_mc_addr);
 if (!ret)
*tmr_size = psp->cmd_buf_mem->resp.tmr_size;
- kfree(cmd);
+
 return ret;
}

@@ -588,7 +600,7 @@ static bool psp_skip_tmr(struct psp_context *psp)
static int psp_tmr_load(struct psp_context *psp)
{
 int ret;
- struct psp_gfx_cmd_resp *cmd;
+struct psp_gfx_cmd_resp *cmd = psp->cmd;

  /* For Navi12 and