[PATCH v2] amdgpu: add a filter condition when set brightness
There is a lenovo ThinkBook 14 G3 ACL notebook, when the laptop is plugged into AC power supply, the brightness obtained ACPI may be smaller than current brightness.As a result the screen becomes dark,this is not what people want. So we should add So we should filter out very small brightness values obtained from ACPI Signed-off-by: Yuanzhi Wang --- drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c index d4196fcb85a0..20e7a178765d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c @@ -406,6 +406,7 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev, { struct amdgpu_atif *atif = _acpi_priv.atif; int count; + int old_brightness; DRM_DEBUG_DRIVER("event, device_class = %s, type = %#x\n", event->device_class, event->type); @@ -443,7 +444,14 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev, * hardwired to post BACKLIGHT_UPDATE_SYSFS. * It probably should accept 'reason' parameter. */ - backlight_device_set_brightness(atif->bd, req.backlight_level); + old_brightness = backlight_get_brightness(atif->bd); + if (old_brightness > req.backlight_level) + DRM_WARN( + "Old brightness %d is larger than ACPI brightness %d\n", +old_brightness, req.backlight_level); + else + backlight_device_set_brightness(atif->bd, +req.backlight_level); } } -- 2.20.1
Re: [PATCH] amdgpu: add a filter condition when set brightness
Hello, I have fixed compilation errors. The lenovo ThinkBook 14 G3 ACL notebook bios reports very small brightness value, 3 eg. On 2023/2/27 22:20, Mario Limonciello wrote: On 2/27/23 01:39, Yuanzhi Wang wrote: When the laptop is plugged into AC or DC power supply, the brightness obtained ACPI may be smaller than current brightness.As a result the screen becomes dark,this is not what people want. Do you have a matching bug report with more information included? Some relevant details I think we need: kernel version laptop model BIOS version if it's the latest BIOS some sample numbers that this new warning/behavior catches acpidump At least as described this sounds like a BIOS bug. Signed-off-by: Yuanzhi Wang --- drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c index d4196fcb85a0..93f1567028c5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c @@ -406,6 +406,7 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev, { struct amdgpu_atif *atif = _acpi_priv.atif; int count; + int old_brightness; DRM_DEBUG_DRIVER("event, device_class = %s, type = %#x\n", event->device_class, event->type); @@ -443,7 +444,13 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev, * hardwired to post BACKLIGHT_UPDATE_SYSFS. * It probably should accept 'reason' parameter. */ - backlight_device_set_brightness(atif->bd, req.backlight_level); + old_brightness = backlight_get_brightness(atif->bd); + if (old_brightness > req.backlight_level) + DRM_WARN("old brightness %d is greater than ACPI brightness + %d\n", old_brightness, req.backlight_level); + else + backlight_device_set_brightness(atif->bd, + req.backlight_level); } }
RE: [PATCH v2 3/3] drm/amd: Add special handling for system s0ix state w/ dGPUs
[AMD Official Use Only - General] > -Original Message- > From: amd-gfx On Behalf Of > Mario Limonciello > Sent: Tuesday, February 28, 2023 12:43 PM > To: amd-gfx@lists.freedesktop.org > Cc: Peter Kopec ; Limonciello, Mario > > Subject: [PATCH v2 3/3] drm/amd: Add special handling for system s0ix state > w/ dGPUs > > With dGPUs that support BACO or BOCO we want them to go into those > states when the system goes to s2idle. Detect that the system will > be targeting this state and force the call into runtime suspend. > > If the runtime suspend call fails for any reason, then fallback to > standard suspend flow. The "standard suspend" means here is normal s2idle flow. Right? > > Signed-off-by: Mario Limonciello > --- > v1->v2: > * New patch > drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 3 +-- > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 +++- > 2 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > index 711f2a1bf525..7c3c6380135a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c > @@ -1073,8 +1073,7 @@ bool amdgpu_acpi_should_gpu_reset(struct > amdgpu_device *adev) > */ > bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev) > { > - if (!(adev->flags & AMD_IS_APU) || > - (pm_suspend_target_state != PM_SUSPEND_TO_IDLE)) > + if (pm_suspend_target_state != PM_SUSPEND_TO_IDLE) > return false; > > if (adev->asic_type < CHIP_RAVEN) > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > index 750984517192..acc032c4c250 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > @@ -2415,8 +2415,18 @@ static int amdgpu_pmops_suspend(struct device > *dev) > struct drm_device *drm_dev = dev_get_drvdata(dev); > struct amdgpu_device *adev = drm_to_adev(drm_dev); > > - if (amdgpu_acpi_is_s0ix_active(adev)) > + if (amdgpu_acpi_is_s0ix_active(adev)) { > + /* try to explicitly enter runtime suspend for s2idle on > BACO/BOCO */ > + if (dev_pm_test_driver_flags(drm_dev->dev, > DPM_FLAG_SMART_SUSPEND)) { > + int ret; > + > + ret = pm_runtime_suspend(dev); > + if (!ret) > + return 0; "ret" seems redundant and can be dropped. BR Evan > + DRM_WARN("failed to enter runtime suspend, > running system suspend: %d\n", ret); > + } > adev->in_s0ix = true; > + } > else if (amdgpu_acpi_is_s3_active(adev)) > adev->in_s3 = true; > if (!adev->in_s0ix && !adev->in_s3) > -- > 2.34.1
RE: [PATCH v2 0/3] Adjust dGPU handling for BACO
[AMD Official Use Only - General] > -Original Message- > From: amd-gfx On Behalf Of > Mario Limonciello > Sent: Tuesday, February 28, 2023 12:43 PM > To: amd-gfx@lists.freedesktop.org > Cc: Peter Kopec ; Limonciello, Mario > > Subject: [PATCH v2 0/3] Adjust dGPU handling for BACO > > This series adjusts the handling for dGPUs when the system is going into > s2idle. The intent is to match the following truth table below: > > +---+--+-- > +-+ > | | s2idle (no FADT) | s2idle (FADT) > | deep > | > +---+--+-- > +-+ > | APU Prepare | 0| 0 > | 0 | > | APU Suspend | Run | Run > | Run > | > | BACO dGPU Prepare | 1 if suspended | 1 if suspended > | 1 > if suspended | > | BACO dGPU Suspend | Runtime suspend if prepare was 0 | Runtime > suspend if prepare was 0 | S3 suspend if prepare was 0 | > | BOCO dGPU Prepare | 1| 1 > | 1 if suspended > | For BOCO Prepare, it should be also "1 if suspsended" instead of "1" for s2idle per patch1. Do I miss anything? BR Evan > | BOCO dGPU Suspend | Runtime suspend if prepare was 0 | Runtime > suspend if prepare was 0 | S3 suspend if prepare was 0 | > +---+--+-- > +-+ > > That is BACO and BOCO are handled very similarly when system is doing > s2idle. > > v1->v2: > * Rework flags and flow > * Try to do runtime suspend first, and if it fails do system suspend > > Mario Limonciello (3): > drm/amd: Allow dGPUs that support BACO to use smart suspend > drm/amd: Don't always set s3 for dGPUs in all sleep modes > drm/amd: Add special handling for system s0ix state w/ dGPUs > > drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 11 +++--- > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 44 +++- > > 2 files changed, 34 insertions(+), 21 deletions(-) > > -- > 2.34.1
[PATCH v2 2/3] drm/amd: Don't always set s3 for dGPUs in all sleep modes
dGPUs that will be using BACO or BOCO shouldn't be put into S3 when the system is being put into s2idle. Cc: Peter Kopec Signed-off-by: Mario Limonciello --- v1->v2: * Whitespace --- drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c index 25e902077caf..711f2a1bf525 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c @@ -1038,8 +1038,12 @@ void amdgpu_acpi_detect(void) */ bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev) { - return !(adev->flags & AMD_IS_APU) || - (pm_suspend_target_state == PM_SUSPEND_MEM); + if (pm_suspend_target_state == PM_SUSPEND_MEM) + return true; + if (adev->flags & AMD_IS_APU) + return false; + return !amdgpu_device_supports_baco(>ddev) && + !amdgpu_device_supports_boco(>ddev); } /** -- 2.34.1
[PATCH v2 3/3] drm/amd: Add special handling for system s0ix state w/ dGPUs
With dGPUs that support BACO or BOCO we want them to go into those states when the system goes to s2idle. Detect that the system will be targeting this state and force the call into runtime suspend. If the runtime suspend call fails for any reason, then fallback to standard suspend flow. Signed-off-by: Mario Limonciello --- v1->v2: * New patch drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 3 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 +++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c index 711f2a1bf525..7c3c6380135a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c @@ -1073,8 +1073,7 @@ bool amdgpu_acpi_should_gpu_reset(struct amdgpu_device *adev) */ bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev) { - if (!(adev->flags & AMD_IS_APU) || - (pm_suspend_target_state != PM_SUSPEND_TO_IDLE)) + if (pm_suspend_target_state != PM_SUSPEND_TO_IDLE) return false; if (adev->asic_type < CHIP_RAVEN) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 750984517192..acc032c4c250 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2415,8 +2415,18 @@ static int amdgpu_pmops_suspend(struct device *dev) struct drm_device *drm_dev = dev_get_drvdata(dev); struct amdgpu_device *adev = drm_to_adev(drm_dev); - if (amdgpu_acpi_is_s0ix_active(adev)) + if (amdgpu_acpi_is_s0ix_active(adev)) { + /* try to explicitly enter runtime suspend for s2idle on BACO/BOCO */ + if (dev_pm_test_driver_flags(drm_dev->dev, DPM_FLAG_SMART_SUSPEND)) { + int ret; + + ret = pm_runtime_suspend(dev); + if (!ret) + return 0; + DRM_WARN("failed to enter runtime suspend, running system suspend: %d\n", ret); + } adev->in_s0ix = true; + } else if (amdgpu_acpi_is_s3_active(adev)) adev->in_s3 = true; if (!adev->in_s0ix && !adev->in_s3) -- 2.34.1
[PATCH v2 1/3] drm/amd: Allow dGPUs that support BACO to use smart suspend
If a dGPU is already runtime suspended using BACO, there is no point to waking it up to run regular suspend callbacks. Cc: Peter Kopec Signed-off-by: Mario Limonciello --- v1->v2: * Simplify prepare call --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 32 - 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index e11f83bd1653..750984517192 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2190,8 +2190,9 @@ static int amdgpu_pci_probe(struct pci_dev *pdev, /* only need to skip on ATPX */ if (amdgpu_device_supports_px(ddev)) dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_NO_DIRECT_COMPLETE); - /* we want direct complete for BOCO */ - if (amdgpu_device_supports_boco(ddev)) + /* we want direct complete for BOCO and for BACO */ + if (amdgpu_device_supports_boco(ddev) || + amdgpu_device_supports_baco(ddev)) dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_SMART_PREPARE | DPM_FLAG_SMART_SUSPEND | DPM_FLAG_MAY_SKIP_RESUME); @@ -2384,25 +2385,24 @@ static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work) return; } +/** + * amdgpu_pmops_prepare + * + * @dev: device pointer + * + * Run the "prepare" PM operation. For devices supporting + * BOCO or BACO use DPM_FLAG_SMART_PREPARE to skip rest of + * suspend process. + * + */ static int amdgpu_pmops_prepare(struct device *dev) { struct drm_device *drm_dev = dev_get_drvdata(dev); - struct amdgpu_device *adev = drm_to_adev(drm_dev); - /* Return a positive number here so -* DPM_FLAG_SMART_SUSPEND works properly -*/ - if (amdgpu_device_supports_boco(drm_dev)) - return pm_runtime_suspended(dev); - - /* if we will not support s3 or s2i for the device -* then skip suspend -*/ - if (!amdgpu_acpi_is_s0ix_active(adev) && - !amdgpu_acpi_is_s3_active(adev)) - return 1; + if (!dev_pm_test_driver_flags(drm_dev->dev, DPM_FLAG_SMART_PREPARE)) + return 0; - return 0; + return pm_runtime_suspended(dev); } static void amdgpu_pmops_complete(struct device *dev) -- 2.34.1
[PATCH v2 0/3] Adjust dGPU handling for BACO
This series adjusts the handling for dGPUs when the system is going into s2idle. The intent is to match the following truth table below: +---+--+--+-+ | | s2idle (no FADT) | s2idle (FADT) | deep| +---+--+--+-+ | APU Prepare | 0| 0 | 0 | | APU Suspend | Run | Run | Run | | BACO dGPU Prepare | 1 if suspended | 1 if suspended | 1 if suspended | | BACO dGPU Suspend | Runtime suspend if prepare was 0 | Runtime suspend if prepare was 0 | S3 suspend if prepare was 0 | | BOCO dGPU Prepare | 1| 1 | 1 if suspended | | BOCO dGPU Suspend | Runtime suspend if prepare was 0 | Runtime suspend if prepare was 0 | S3 suspend if prepare was 0 | +---+--+--+-+ That is BACO and BOCO are handled very similarly when system is doing s2idle. v1->v2: * Rework flags and flow * Try to do runtime suspend first, and if it fails do system suspend Mario Limonciello (3): drm/amd: Allow dGPUs that support BACO to use smart suspend drm/amd: Don't always set s3 for dGPUs in all sleep modes drm/amd: Add special handling for system s0ix state w/ dGPUs drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 11 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 44 +++- 2 files changed, 34 insertions(+), 21 deletions(-) -- 2.34.1
Re: [PATCH] drm/amdkfd: Cal vram offset in page for each svm_migrate_copy_to_vram
On 2023-02-27 18:07, Xiaogang.Chen wrote: From: Xiaogang Chen svm_migrate_ram_to_vram migrate a prange from sys ram to vram. The prange may cross multiple vma. Need remember current dst vram offset in page for each migration. Good catch. It's not the offset in the page, but the offset in the TTM resource, I think. See more nit-picks inline. Signed-off-by: Xiaogang Chen --- drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 17 ++--- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c index 1c625433ff30..60664e0cbc1c 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c @@ -294,7 +294,7 @@ static unsigned long svm_migrate_unsuccessful_pages(struct migrate_vma *migrate) static int svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange, struct migrate_vma *migrate, struct dma_fence **mfence, -dma_addr_t *scratch) +dma_addr_t *scratch, uint64_t *cur_dst) The name cur_dst is a bit confusing. There is a local variable "dst" in this function, which has a very different meaning. It's the pointer to an array of VRAM physical addresses. A better name for cur_dst would be ttm_res_offset, as it is the offset from the start of the TTM resource. I'd prefer if it was not a pointer. The calculations to increment the offset should be done at the top level, where it iterates through the VMAs. This low level code doesn't need to make any assumptions about how that iteration works. { uint64_t npages = migrate->npages; struct device *dev = adev->dev; @@ -304,8 +304,8 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange, uint64_t i, j; int r; - pr_debug("svms 0x%p [0x%lx 0x%lx]\n", prange->svms, prange->start, -prange->last); + pr_debug("svms 0x%p [0x%lx 0x%lx 0x%lx]\n", prange->svms, prange->start, +prange->last, *cur_dst); src = scratch; dst = (uint64_t *)(scratch + npages); @@ -316,7 +316,7 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange, goto out; } - amdgpu_res_first(prange->ttm_res, prange->offset << PAGE_SHIFT, + amdgpu_res_first(prange->ttm_res, *cur_dst << PAGE_SHIFT, Maybe do the PAGE_SHIFT in the caller, where ttm_res_offset is calculated and updated. npages << PAGE_SHIFT, ); for (i = j = 0; i < npages; i++) { struct page *spage; @@ -381,6 +381,7 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange, migrate->dst[i] = 0; } } + *cur_dst = *cur_dst + i; #ifdef DEBUG_FORCE_MIXED_DOMAINS for (i = 0, j = 0; i < npages; i += 4, j++) { @@ -403,7 +404,7 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange, static long svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange, struct vm_area_struct *vma, uint64_t start, - uint64_t end, uint32_t trigger) + uint64_t end, uint32_t trigger, uint64_t *cur_dst) Same as above. { struct kfd_process *p = container_of(prange->svms, struct kfd_process, svms); uint64_t npages = (end - start) >> PAGE_SHIFT; @@ -456,7 +457,7 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange, else pr_debug("0x%lx pages migrated\n", cpages); - r = svm_migrate_copy_to_vram(adev, prange, , , scratch); + r = svm_migrate_copy_to_vram(adev, prange, , , scratch, cur_dst); migrate_vma_pages(); pr_debug("successful/cpages/npages 0x%lx/0x%lx/0x%lx\n", @@ -504,6 +505,7 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc, unsigned long addr, start, end; struct vm_area_struct *vma; struct amdgpu_device *adev; + uint64_t cur_dst; unsigned long cpages = 0; long r = 0; @@ -524,6 +526,7 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc, start = prange->start << PAGE_SHIFT; end = (prange->last + 1) << PAGE_SHIFT; + cur_dst = prange->offset; You could do the PAGE_SHIFT here. for (addr = start; addr < end;) { unsigned long next; @@ -533,7 +536,7 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc, break; next = min(vma->vm_end, end); - r = svm_migrate_vma_to_vram(adev, prange, vma, addr, next, trigger); + r = svm_migrate_vma_to_vram(adev, prange, vma, addr, next, trigger, _dst); if (r < 0) { pr_debug("failed %ld to migrate\n", r);
[PATCH] drm/amdkfd: Cal vram offset in page for each svm_migrate_copy_to_vram
From: Xiaogang Chen svm_migrate_ram_to_vram migrate a prange from sys ram to vram. The prange may cross multiple vma. Need remember current dst vram offset in page for each migration. Signed-off-by: Xiaogang Chen --- drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 17 ++--- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c index 1c625433ff30..60664e0cbc1c 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c @@ -294,7 +294,7 @@ static unsigned long svm_migrate_unsuccessful_pages(struct migrate_vma *migrate) static int svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange, struct migrate_vma *migrate, struct dma_fence **mfence, -dma_addr_t *scratch) +dma_addr_t *scratch, uint64_t *cur_dst) { uint64_t npages = migrate->npages; struct device *dev = adev->dev; @@ -304,8 +304,8 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange, uint64_t i, j; int r; - pr_debug("svms 0x%p [0x%lx 0x%lx]\n", prange->svms, prange->start, -prange->last); + pr_debug("svms 0x%p [0x%lx 0x%lx 0x%lx]\n", prange->svms, prange->start, +prange->last, *cur_dst); src = scratch; dst = (uint64_t *)(scratch + npages); @@ -316,7 +316,7 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange, goto out; } - amdgpu_res_first(prange->ttm_res, prange->offset << PAGE_SHIFT, + amdgpu_res_first(prange->ttm_res, *cur_dst << PAGE_SHIFT, npages << PAGE_SHIFT, ); for (i = j = 0; i < npages; i++) { struct page *spage; @@ -381,6 +381,7 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange, migrate->dst[i] = 0; } } + *cur_dst = *cur_dst + i; #ifdef DEBUG_FORCE_MIXED_DOMAINS for (i = 0, j = 0; i < npages; i += 4, j++) { @@ -403,7 +404,7 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange, static long svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange, struct vm_area_struct *vma, uint64_t start, - uint64_t end, uint32_t trigger) + uint64_t end, uint32_t trigger, uint64_t *cur_dst) { struct kfd_process *p = container_of(prange->svms, struct kfd_process, svms); uint64_t npages = (end - start) >> PAGE_SHIFT; @@ -456,7 +457,7 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange, else pr_debug("0x%lx pages migrated\n", cpages); - r = svm_migrate_copy_to_vram(adev, prange, , , scratch); + r = svm_migrate_copy_to_vram(adev, prange, , , scratch, cur_dst); migrate_vma_pages(); pr_debug("successful/cpages/npages 0x%lx/0x%lx/0x%lx\n", @@ -504,6 +505,7 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc, unsigned long addr, start, end; struct vm_area_struct *vma; struct amdgpu_device *adev; + uint64_t cur_dst; unsigned long cpages = 0; long r = 0; @@ -524,6 +526,7 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc, start = prange->start << PAGE_SHIFT; end = (prange->last + 1) << PAGE_SHIFT; + cur_dst = prange->offset; for (addr = start; addr < end;) { unsigned long next; @@ -533,7 +536,7 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc, break; next = min(vma->vm_end, end); - r = svm_migrate_vma_to_vram(adev, prange, vma, addr, next, trigger); + r = svm_migrate_vma_to_vram(adev, prange, vma, addr, next, trigger, _dst); if (r < 0) { pr_debug("failed %ld to migrate\n", r); break; -- 2.25.1
Re: [bug][vaapi][h264] The commit 7cbe08a930a132d84b4cf79953b00b074ec7a2a7 on certain video files leads to problems with VAAPI hardware decoding.
+ Felix On Fri, Feb 17, 2023 at 4:50 PM Mikhail Gavrilov wrote: > > On Fri, Feb 17, 2023 at 8:30 PM Alex Deucher wrote: > > > > On Fri, Feb 17, 2023 at 1:10 AM Mikhail Gavrilov > > wrote: > > > > > > On Fri, Dec 9, 2022 at 7:37 PM Leo Liu wrote: > > > > > > > > Please try the latest AMDGPU driver: > > > > > > > > https://gitlab.freedesktop.org/agd5f/linux/-/commits/amd-staging-drm-next/ > > > > > > > > > > Sorry Leo, I miss your message. > > > This issue is still actual for 6.2-rc8. > > > > > > In my first message I was mistaken. > > > > > > > Before kernel 5.16 this only led to an artifact in the form of > > > > a green bar at the top of the screen, then starting from 5.17 > > > > the GPU began to freeze. > > > > > > The real behaviour before 5.18: > > > - vlc could plays video with small artifacts in the form of a green > > > bar on top of the video > > > - after playing video process vlc correctly exiting > > > > > > On 5.18 this behaviour changed: > > > - vlc show black screen instead of playing video > > > - after playing the process not exiting > > > - if I tries kill vlc process with 'kill -9' vlc became zombi process > > > and many other processes start hangs (in kernel log appears follow > > > lines after 2 minutes) > > > > > > INFO: task vlc:sh8:5248 blocked for more than 122 seconds. > > > Tainted: GWL --- > > > 5.18.0-60.fc37.x86_64+debug #1 > > > "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. > > > task:vlc:sh8 state:D stack:13616 pid: 5248 ppid: 1934 > > > flags:0x4006 > > > Call Trace: > > > > > > __schedule+0x492/0x1650 > > > ? _raw_spin_unlock_irqrestore+0x40/0x60 > > > ? debug_check_no_obj_freed+0x12d/0x250 > > > schedule+0x4e/0xb0 > > > schedule_timeout+0xe1/0x120 > > > ? lock_release+0x215/0x460 > > > ? trace_hardirqs_on+0x1a/0xf0 > > > ? _raw_spin_unlock_irqrestore+0x40/0x60 > > > dma_fence_default_wait+0x197/0x240 > > > ? __bpf_trace_dma_fence+0x10/0x10 > > > dma_fence_wait_timeout+0x229/0x260 > > > drm_sched_entity_fini+0x101/0x270 [gpu_sched] > > > amdgpu_vm_fini+0x2b5/0x460 [amdgpu] > > > ? idr_destroy+0x70/0xb0 > > > ? mutex_destroy+0x1e/0x50 > > > amdgpu_driver_postclose_kms+0x1ec/0x2c0 [amdgpu] > > > drm_file_free.part.0+0x20d/0x260 > > > drm_release+0x6a/0x120 > > > __fput+0xab/0x270 > > > task_work_run+0x5c/0xa0 > > > do_exit+0x394/0xc40 > > > ? rcu_read_lock_sched_held+0x10/0x70 > > > do_group_exit+0x33/0xb0 > > > get_signal+0xbbc/0xbc0 > > > arch_do_signal_or_restart+0x30/0x770 > > > ? do_futex+0xfd/0x190 > > > ? __x64_sys_futex+0x63/0x190 > > > exit_to_user_mode_prepare+0x172/0x270 > > > syscall_exit_to_user_mode+0x16/0x50 > > > do_syscall_64+0x67/0x80 > > > ? do_syscall_64+0x67/0x80 > > > ? rcu_read_lock_sched_held+0x10/0x70 > > > ? trace_hardirqs_on_prepare+0x5e/0x110 > > > ? do_syscall_64+0x67/0x80 > > > ? rcu_read_lock_sched_held+0x10/0x70 > > > entry_SYSCALL_64_after_hwframe+0x44/0xae > > > RIP: 0033:0x7f82c2364529 > > > RSP: 002b:7f8210ff8c00 EFLAGS: 0246 ORIG_RAX: 00ca > > > RAX: fe00 RBX: RCX: 7f82c2364529 > > > RDX: RSI: 0189 RDI: 7f823022542c > > > RBP: 7f8210ff8c30 R08: R09: > > > R10: R11: 0246 R12: > > > R13: R14: 0001 R15: 7f823022542c > > > > > > INFO: lockdep is turned off. > > > > > > I bisected this issue and problematic commit is > > > > > > ❯ git bisect bad > > > 5f3854f1f4e211f494018160b348a1c16e58013f is the first bad commit > > > commit 5f3854f1f4e211f494018160b348a1c16e58013f > > > Author: Alex Deucher > > > Date: Thu Mar 24 18:04:00 2022 -0400 > > > > > > drm/amdgpu: add more cases to noretry=1 > > > > > > Port current list from amd-staging-drm-next. > > > > > > Signed-off-by: Alex Deucher > > > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > Unfortunately I couldn't simply revert this commit on 6.2-rc8 for > > > checking, because it leads to conflicts. > > > > > > Alex, you as author of this commit could help me with it? > > > > append amdgpu.noretry=0 to the kernel command line in grub. > > Thanks, I checked the "amdgpu.noretry=0" and after the page fault > occurs vlc could play video with little artifacts. > > So I have some questions: > > 1. Why retrys was disabled by default if it really stills needed for > recoverable page faults? As Christian answered me before here: > https://lore.kernel.org/all/f253ff1f-3c5c-c785-1272-e4fe69a36...@amd.com/T/#m73a0a6eb7b2531eacf24fd498e8d2eec675f05a6 > You don't actually want retry page faults, because for gfx apps, nothing is going to page in the missing pages. The retry stuff is for demand paging type scenarios and only certain GPUs (GFX9-based) actually support the necessary semantics to make this work. Even then
[PATCH v2 1/1] drm/doc: Document DRM device reset expectations
Create a section that specifies how to deal with DRM device resets for kernel and userspace drivers. Signed-off-by: André Almeida --- Documentation/gpu/drm-uapi.rst | 51 ++ 1 file changed, 51 insertions(+) diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 65fb3036a580..3d6c3ed392ea 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -285,6 +285,57 @@ for GPU1 and GPU2 from different vendors, and a third handler for mmapped regular files. Threads cause additional pain with signal handling as well. +Device reset + + +The GPU stack is really complex and is prone to errors, from hardware bugs, +faulty applications and everything in the many layers in between. To recover +from this kind of state, sometimes is needed to reset the GPU. Unproper handling +of GPU resets can lead to an unstable userspace. This section describes what's +the expected behaviour from DRM drivers to do in those situations, from usermode +drivers and compositors as well. The end goal is to have a seamless experience +as possible, either the stack being able to recover itself or resetting to a new +stable state. + +Robustness +-- + +First of all, application robust APIs, when available, should be used. This +allows the application to correctly recover and continue to run after a reset. +Apps that doesn't use this should be promptly killed when the kernel driver +detects that it's in broken state. Specifically guidelines for some APIs: + +- OpenGL: KMD signals the abortion of submitted commands and the UMD should then + react accordingly and abort the application. + +- Vulkan: Assumes that every app is able to deal with ``VK_ERROR_DEVICE_LOST``. + If it doesn't do it right, it's considered a broken application and UMD will + deal with it, aborting it. + +Kernel mode driver +-- + +The KMD must be able to detect that something is wrong with the application +and that a reset is needed to take place to recover the device (e.g. an endless +wait). It needs to properly track the context that is broken and mark it as +dead, so any other syscalls to that context should be further rejected. The +other contexts should be preserved when possible, avoid crashing the rest of +userspace. KMD can ban a file descriptor that keeps causing resets, as it's +likely in a broken loop. + +User mode driver + + +During a reset, UMD should be aware that rejected syscalls indicates that the +context is broken and for robust apps the recovery should happen for the +context. Non-robust apps must be terminated. + +Compositors +--- + +Compositors should be robust as well to properly deal with its errors. + + .. _drm_driver_ioctl: IOCTL Support on Device Nodes -- 2.39.2
[PATCH v2 0/1] drm: Add doc about GPU reset
Hi, Thanks everyone that gave feedback. v2 Changes: - This new version is a section of drm-uapi instead of a new file - Drop requirement for KMD to kill applications - Drop role of init systems on compositors recover - Drop assumption that robust apps creates new contexts Original cover letter bellow: Due to the complexity of its stack and the apps that we run on it, GPU resets are for granted. What's left for driver developers is how to make resets a smooth experience as possible. While some OS's can recover or show an error message in such cases, Linux is more a hit-and-miss due to its lack of standardization and guidelines of what to do in such cases. This is the goal of this document, to proper define what should happen after a GPU reset so developers can start acting on top of this. An IGT test should be created to validate this for each driver. Initially my approach was to expose an uevent for GPU resets, as it can be seen here[1]. However, even if an uevent can be useful for some use cases (e.g. telemetry and error reporting), for the "OS integration" case of GPU resets it would be more productive to have something defined through the stack. Thanks, André [1] https://lore.kernel.org/amd-gfx/20221125175203.52481-1-andrealm...@igalia.com/ André Almeida (1): drm/doc: Document DRM device reset expectations Documentation/gpu/drm-uapi.rst | 51 ++ 1 file changed, 51 insertions(+) -- 2.39.2
Re: [PATCH v3 1/2] drm/edid: parse DRM VESA dsc bpp target
On Mon, 27 Feb 2023, Harry Wentland wrote: > On 2/27/23 12:12, Jani Nikula wrote: >> On Mon, 27 Feb 2023, Harry Wentland wrote: >>> On 2/26/23 09:10, Yaroslav Bolyukin wrote: As per DisplayID v2.0 Errata E9 spec "DSC pass-through timing support" VESA vendor-specific data block may contain target DSC bits per pixel fields >>> >>> According to the errata this should only apply to VII timings. The way >>> it is currently implemented will make it apply to everything which is >>> not what we want. >>> >>> Can we add this field to drm_mode_info instead of drm_display_info and >>> set it inside drm_mode_displayid_detailed when parsing a type_7 timing? >> >> That's actually difficult to do nicely. I think the patch at hand is >> fine, and it's fine to add the information to drm_display_info. It's a >> dependency to parsing the modes. >> >> How the info will actually be used is a different matter, and obviously >> needs to follow the spec. As it is, *this* patch doesn't say anything >> about that. But whether it's handled in VII timings parsing or >> elsewhere, I still think this part is fine. >> > > This patch itself is okay but without further work can't be used by > drivers since they don't currently have an indication whether a mode > is based on a VII timing. Right, agreed. All I'm saying is info->dp_dsc_bpp is the way to pass that info from VESA vendor block to mode parsing. Come to think of it, this patch should probably also rename drm_update_mso() and drm_parse_vesa_mso_data() to reflect the more generic VESA vendor block parsing instead of just MSO. BR, Jani. > > Harry > >> >> BR, >> Jani. >> >>> >>> Harry >>> >>> Signed-off-by: Yaroslav Bolyukin --- drivers/gpu/drm/drm_edid.c | 38 + include/drm/drm_connector.h | 6 ++ include/drm/drm_displayid.h | 4 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 3d0a4da661bc..aa88ac82cbe0 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -6338,7 +6338,7 @@ static void drm_parse_vesa_mso_data(struct drm_connector *connector, if (oui(vesa->oui[0], vesa->oui[1], vesa->oui[2]) != VESA_IEEE_OUI) return; - if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) { + if (block->num_bytes < 5) { drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] Unexpected VESA vendor block size\n", connector->base.id, connector->name); @@ -6361,24 +6361,37 @@ static void drm_parse_vesa_mso_data(struct drm_connector *connector, break; } - if (!info->mso_stream_count) { - info->mso_pixel_overlap = 0; - return; - } + info->mso_pixel_overlap = 0; + + if (info->mso_stream_count) { + info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, vesa->mso); + + if (info->mso_pixel_overlap > 8) { + drm_dbg_kms(connector->dev, + "[CONNECTOR:%d:%s] Reserved MSO pixel overlap value %u\n", + connector->base.id, connector->name, + info->mso_pixel_overlap); + info->mso_pixel_overlap = 8; + } - info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, vesa->mso); - if (info->mso_pixel_overlap > 8) { drm_dbg_kms(connector->dev, - "[CONNECTOR:%d:%s] Reserved MSO pixel overlap value %u\n", + "[CONNECTOR:%d:%s] MSO stream count %u, pixel overlap %u\n", connector->base.id, connector->name, - info->mso_pixel_overlap); - info->mso_pixel_overlap = 8; + info->mso_stream_count, info->mso_pixel_overlap); + } + + if (block->num_bytes < 7) { + /* DSC bpp is optional */ + return; } + info->dp_dsc_bpp = FIELD_GET(DISPLAYID_VESA_DSC_BPP_INT, vesa->dsc_bpp_int) * 16 + + FIELD_GET(DISPLAYID_VESA_DSC_BPP_FRACT, vesa->dsc_bpp_fract); + drm_dbg_kms(connector->dev, - "[CONNECTOR:%d:%s] MSO stream count %u, pixel overlap %u\n", + "[CONNECTOR:%d:%s] DSC bits per pixel %u\n", connector->base.id, connector->name, - info->mso_stream_count, info->mso_pixel_overlap); + info->dp_dsc_bpp); } static void drm_update_mso(struct drm_connector *connector, @@ -6425,6 +6438,7 @@ static void drm_reset_display_info(struct drm_connector *connector)
Re: [PATCH] drm/amdgpu: fix return value check in kfd
On 2023-02-27 09:52, Shashank Sharma wrote: From: Shashank Sharma This patch fixes a return value check in kfd doorbell handling. This function should return 0(error) only when the ida_simple_get returns < 0(error), return > 0 is a success case. Cc: Felix Kuehling Cc: Alex Deucher Signed-off-by: Shashank Sharma Thanks for catching this. I wonder why this hasn't caused any obvious issues before. You could add Fixes: 16f0013157bf ("drm/amdkfd: Allocate doorbells only when needed") Reviewed-by: Felix Kuehling --- drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c index cbef2e147da5..38c9e1ca6691 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c @@ -280,7 +280,7 @@ phys_addr_t kfd_get_process_doorbells(struct kfd_process_device *pdd) if (!pdd->doorbell_index) { int r = kfd_alloc_process_doorbells(pdd->dev, >doorbell_index); - if (r) + if (r < 0) return 0; }
Re: [PATCH v3 1/2] drm/edid: parse DRM VESA dsc bpp target
On 2/27/23 12:12, Jani Nikula wrote: > On Mon, 27 Feb 2023, Harry Wentland wrote: >> On 2/26/23 09:10, Yaroslav Bolyukin wrote: >>> As per DisplayID v2.0 Errata E9 spec "DSC pass-through timing support" >>> VESA vendor-specific data block may contain target DSC bits per pixel >>> fields >>> >> >> According to the errata this should only apply to VII timings. The way >> it is currently implemented will make it apply to everything which is >> not what we want. >> >> Can we add this field to drm_mode_info instead of drm_display_info and >> set it inside drm_mode_displayid_detailed when parsing a type_7 timing? > > That's actually difficult to do nicely. I think the patch at hand is > fine, and it's fine to add the information to drm_display_info. It's a > dependency to parsing the modes. > > How the info will actually be used is a different matter, and obviously > needs to follow the spec. As it is, *this* patch doesn't say anything > about that. But whether it's handled in VII timings parsing or > elsewhere, I still think this part is fine. > This patch itself is okay but without further work can't be used by drivers since they don't currently have an indication whether a mode is based on a VII timing. Harry > > BR, > Jani. > >> >> Harry >> >> >>> Signed-off-by: Yaroslav Bolyukin >>> --- >>> drivers/gpu/drm/drm_edid.c | 38 + >>> include/drm/drm_connector.h | 6 ++ >>> include/drm/drm_displayid.h | 4 >>> 3 files changed, 36 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >>> index 3d0a4da661bc..aa88ac82cbe0 100644 >>> --- a/drivers/gpu/drm/drm_edid.c >>> +++ b/drivers/gpu/drm/drm_edid.c >>> @@ -6338,7 +6338,7 @@ static void drm_parse_vesa_mso_data(struct >>> drm_connector *connector, >>> if (oui(vesa->oui[0], vesa->oui[1], vesa->oui[2]) != VESA_IEEE_OUI) >>> return; >>> >>> - if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) { >>> + if (block->num_bytes < 5) { >>> drm_dbg_kms(connector->dev, >>> "[CONNECTOR:%d:%s] Unexpected VESA vendor block >>> size\n", >>> connector->base.id, connector->name); >>> @@ -6361,24 +6361,37 @@ static void drm_parse_vesa_mso_data(struct >>> drm_connector *connector, >>> break; >>> } >>> >>> - if (!info->mso_stream_count) { >>> - info->mso_pixel_overlap = 0; >>> - return; >>> - } >>> + info->mso_pixel_overlap = 0; >>> + >>> + if (info->mso_stream_count) { >>> + info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, >>> vesa->mso); >>> + >>> + if (info->mso_pixel_overlap > 8) { >>> + drm_dbg_kms(connector->dev, >>> + "[CONNECTOR:%d:%s] Reserved MSO pixel >>> overlap value %u\n", >>> + connector->base.id, connector->name, >>> + info->mso_pixel_overlap); >>> + info->mso_pixel_overlap = 8; >>> + } >>> >>> - info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, >>> vesa->mso); >>> - if (info->mso_pixel_overlap > 8) { >>> drm_dbg_kms(connector->dev, >>> - "[CONNECTOR:%d:%s] Reserved MSO pixel overlap value >>> %u\n", >>> + "[CONNECTOR:%d:%s] MSO stream count %u, pixel >>> overlap %u\n", >>> connector->base.id, connector->name, >>> - info->mso_pixel_overlap); >>> - info->mso_pixel_overlap = 8; >>> + info->mso_stream_count, info->mso_pixel_overlap); >>> + } >>> + >>> + if (block->num_bytes < 7) { >>> + /* DSC bpp is optional */ >>> + return; >>> } >>> >>> + info->dp_dsc_bpp = FIELD_GET(DISPLAYID_VESA_DSC_BPP_INT, >>> vesa->dsc_bpp_int) * 16 >>> + + FIELD_GET(DISPLAYID_VESA_DSC_BPP_FRACT, vesa->dsc_bpp_fract); >>> + >>> drm_dbg_kms(connector->dev, >>> - "[CONNECTOR:%d:%s] MSO stream count %u, pixel overlap %u\n", >>> + "[CONNECTOR:%d:%s] DSC bits per pixel %u\n", >>> connector->base.id, connector->name, >>> - info->mso_stream_count, info->mso_pixel_overlap); >>> + info->dp_dsc_bpp); >>> } >>> >>> static void drm_update_mso(struct drm_connector *connector, >>> @@ -6425,6 +6438,7 @@ static void drm_reset_display_info(struct >>> drm_connector *connector) >>> info->mso_stream_count = 0; >>> info->mso_pixel_overlap = 0; >>> info->max_dsc_bpp = 0; >>> + info->dp_dsc_bpp = 0; >>> >>> kfree(info->vics); >>> info->vics = NULL; >>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h >>> index 7b5048516185..1d01e0146a7f 100644 >>> --- a/include/drm/drm_connector.h >>> +++ b/include/drm/drm_connector.h >>> @@ -719,6 +719,12 @@ struct drm_display_info { >>> */
Re: [PATCH v3 2/2] drm/amd: use fixed dsc bits-per-pixel from edid
On 2/26/23 09:10, Yaroslav Bolyukin wrote: > VESA vendor header from DisplayID spec may contain fixed bit per pixel > rate, it should be respected by drm driver > This will apply the fixed bpp for all modes. I don't think that's right. It should apply only to VII timings. Harry > Signed-off-by: Yaroslav Bolyukin > Reviewed-by: Wayne Lin > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 ++ > drivers/gpu/drm/amd/display/dc/core/dc_stream.c | 2 ++ > drivers/gpu/drm/amd/display/dc/dc_types.h | 3 +++ > 3 files changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > index 6fdc2027c2b4..dba720d5df4c 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > @@ -89,6 +89,8 @@ enum dc_edid_status dm_helpers_parse_edid_caps( > > edid_caps->edid_hdmi = connector->display_info.is_hdmi; > > + edid_caps->dsc_fixed_bits_per_pixel_x16 = > connector->display_info.dp_dsc_bpp; > + > sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, ); > if (sad_count <= 0) > return result; > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c > b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c > index 72b261ad9587..a82362417379 100644 > --- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c > @@ -103,6 +103,8 @@ static bool dc_stream_construct(struct dc_stream_state > *stream, > > /* EDID CAP translation for HDMI 2.0 */ > stream->timing.flags.LTE_340MCSC_SCRAMBLE = > dc_sink_data->edid_caps.lte_340mcsc_scramble; > + stream->timing.dsc_fixed_bits_per_pixel_x16 = > + dc_sink_data->edid_caps.dsc_fixed_bits_per_pixel_x16; > > memset(>timing.dsc_cfg, 0, sizeof(stream->timing.dsc_cfg)); > stream->timing.dsc_cfg.num_slices_h = 0; > diff --git a/drivers/gpu/drm/amd/display/dc/dc_types.h > b/drivers/gpu/drm/amd/display/dc/dc_types.h > index 27d0242d6cbd..22fedf4c7547 100644 > --- a/drivers/gpu/drm/amd/display/dc/dc_types.h > +++ b/drivers/gpu/drm/amd/display/dc/dc_types.h > @@ -228,6 +228,9 @@ struct dc_edid_caps { > bool edid_hdmi; > bool hdr_supported; > > + /* DisplayPort caps */ > + uint32_t dsc_fixed_bits_per_pixel_x16; > + > struct dc_panel_patch panel_patch; > }; >
Re: [PATCH v3 1/2] drm/edid: parse DRM VESA dsc bpp target
On Mon, 27 Feb 2023, Harry Wentland wrote: > On 2/26/23 09:10, Yaroslav Bolyukin wrote: >> As per DisplayID v2.0 Errata E9 spec "DSC pass-through timing support" >> VESA vendor-specific data block may contain target DSC bits per pixel >> fields >> > > According to the errata this should only apply to VII timings. The way > it is currently implemented will make it apply to everything which is > not what we want. > > Can we add this field to drm_mode_info instead of drm_display_info and > set it inside drm_mode_displayid_detailed when parsing a type_7 timing? That's actually difficult to do nicely. I think the patch at hand is fine, and it's fine to add the information to drm_display_info. It's a dependency to parsing the modes. How the info will actually be used is a different matter, and obviously needs to follow the spec. As it is, *this* patch doesn't say anything about that. But whether it's handled in VII timings parsing or elsewhere, I still think this part is fine. BR, Jani. > > Harry > > >> Signed-off-by: Yaroslav Bolyukin >> --- >> drivers/gpu/drm/drm_edid.c | 38 + >> include/drm/drm_connector.h | 6 ++ >> include/drm/drm_displayid.h | 4 >> 3 files changed, 36 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index 3d0a4da661bc..aa88ac82cbe0 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -6338,7 +6338,7 @@ static void drm_parse_vesa_mso_data(struct >> drm_connector *connector, >> if (oui(vesa->oui[0], vesa->oui[1], vesa->oui[2]) != VESA_IEEE_OUI) >> return; >> >> -if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) { >> +if (block->num_bytes < 5) { >> drm_dbg_kms(connector->dev, >> "[CONNECTOR:%d:%s] Unexpected VESA vendor block >> size\n", >> connector->base.id, connector->name); >> @@ -6361,24 +6361,37 @@ static void drm_parse_vesa_mso_data(struct >> drm_connector *connector, >> break; >> } >> >> -if (!info->mso_stream_count) { >> -info->mso_pixel_overlap = 0; >> -return; >> -} >> +info->mso_pixel_overlap = 0; >> + >> +if (info->mso_stream_count) { >> +info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, >> vesa->mso); >> + >> +if (info->mso_pixel_overlap > 8) { >> +drm_dbg_kms(connector->dev, >> +"[CONNECTOR:%d:%s] Reserved MSO pixel >> overlap value %u\n", >> +connector->base.id, connector->name, >> +info->mso_pixel_overlap); >> +info->mso_pixel_overlap = 8; >> +} >> >> -info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, >> vesa->mso); >> -if (info->mso_pixel_overlap > 8) { >> drm_dbg_kms(connector->dev, >> -"[CONNECTOR:%d:%s] Reserved MSO pixel overlap value >> %u\n", >> +"[CONNECTOR:%d:%s] MSO stream count %u, pixel >> overlap %u\n", >> connector->base.id, connector->name, >> -info->mso_pixel_overlap); >> -info->mso_pixel_overlap = 8; >> +info->mso_stream_count, info->mso_pixel_overlap); >> +} >> + >> +if (block->num_bytes < 7) { >> +/* DSC bpp is optional */ >> +return; >> } >> >> +info->dp_dsc_bpp = FIELD_GET(DISPLAYID_VESA_DSC_BPP_INT, >> vesa->dsc_bpp_int) * 16 >> ++ FIELD_GET(DISPLAYID_VESA_DSC_BPP_FRACT, vesa->dsc_bpp_fract); >> + >> drm_dbg_kms(connector->dev, >> -"[CONNECTOR:%d:%s] MSO stream count %u, pixel overlap %u\n", >> +"[CONNECTOR:%d:%s] DSC bits per pixel %u\n", >> connector->base.id, connector->name, >> -info->mso_stream_count, info->mso_pixel_overlap); >> +info->dp_dsc_bpp); >> } >> >> static void drm_update_mso(struct drm_connector *connector, >> @@ -6425,6 +6438,7 @@ static void drm_reset_display_info(struct >> drm_connector *connector) >> info->mso_stream_count = 0; >> info->mso_pixel_overlap = 0; >> info->max_dsc_bpp = 0; >> +info->dp_dsc_bpp = 0; >> >> kfree(info->vics); >> info->vics = NULL; >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h >> index 7b5048516185..1d01e0146a7f 100644 >> --- a/include/drm/drm_connector.h >> +++ b/include/drm/drm_connector.h >> @@ -719,6 +719,12 @@ struct drm_display_info { >> */ >> u32 max_dsc_bpp; >> >> +/** >> + * @dp_dsc_bpp: DP Display-Stream-Compression (DSC) timing's target >> + * DST bits per pixel in 6.4 fixed point format. 0 means undefined >> + */ >> +u16 dp_dsc_bpp; >> + >> /** >> * @vics: Array of vics_len VICs. Internal to
Re: [PATCH v3 1/2] drm/edid: parse DRM VESA dsc bpp target
On 2/26/23 09:10, Yaroslav Bolyukin wrote: > As per DisplayID v2.0 Errata E9 spec "DSC pass-through timing support" > VESA vendor-specific data block may contain target DSC bits per pixel > fields > According to the errata this should only apply to VII timings. The way it is currently implemented will make it apply to everything which is not what we want. Can we add this field to drm_mode_info instead of drm_display_info and set it inside drm_mode_displayid_detailed when parsing a type_7 timing? Harry > Signed-off-by: Yaroslav Bolyukin > --- > drivers/gpu/drm/drm_edid.c | 38 + > include/drm/drm_connector.h | 6 ++ > include/drm/drm_displayid.h | 4 > 3 files changed, 36 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 3d0a4da661bc..aa88ac82cbe0 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -6338,7 +6338,7 @@ static void drm_parse_vesa_mso_data(struct > drm_connector *connector, > if (oui(vesa->oui[0], vesa->oui[1], vesa->oui[2]) != VESA_IEEE_OUI) > return; > > - if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) { > + if (block->num_bytes < 5) { > drm_dbg_kms(connector->dev, > "[CONNECTOR:%d:%s] Unexpected VESA vendor block > size\n", > connector->base.id, connector->name); > @@ -6361,24 +6361,37 @@ static void drm_parse_vesa_mso_data(struct > drm_connector *connector, > break; > } > > - if (!info->mso_stream_count) { > - info->mso_pixel_overlap = 0; > - return; > - } > + info->mso_pixel_overlap = 0; > + > + if (info->mso_stream_count) { > + info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, > vesa->mso); > + > + if (info->mso_pixel_overlap > 8) { > + drm_dbg_kms(connector->dev, > + "[CONNECTOR:%d:%s] Reserved MSO pixel > overlap value %u\n", > + connector->base.id, connector->name, > + info->mso_pixel_overlap); > + info->mso_pixel_overlap = 8; > + } > > - info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, > vesa->mso); > - if (info->mso_pixel_overlap > 8) { > drm_dbg_kms(connector->dev, > - "[CONNECTOR:%d:%s] Reserved MSO pixel overlap value > %u\n", > + "[CONNECTOR:%d:%s] MSO stream count %u, pixel > overlap %u\n", > connector->base.id, connector->name, > - info->mso_pixel_overlap); > - info->mso_pixel_overlap = 8; > + info->mso_stream_count, info->mso_pixel_overlap); > + } > + > + if (block->num_bytes < 7) { > + /* DSC bpp is optional */ > + return; > } > > + info->dp_dsc_bpp = FIELD_GET(DISPLAYID_VESA_DSC_BPP_INT, > vesa->dsc_bpp_int) * 16 > + + FIELD_GET(DISPLAYID_VESA_DSC_BPP_FRACT, vesa->dsc_bpp_fract); > + > drm_dbg_kms(connector->dev, > - "[CONNECTOR:%d:%s] MSO stream count %u, pixel overlap %u\n", > + "[CONNECTOR:%d:%s] DSC bits per pixel %u\n", > connector->base.id, connector->name, > - info->mso_stream_count, info->mso_pixel_overlap); > + info->dp_dsc_bpp); > } > > static void drm_update_mso(struct drm_connector *connector, > @@ -6425,6 +6438,7 @@ static void drm_reset_display_info(struct drm_connector > *connector) > info->mso_stream_count = 0; > info->mso_pixel_overlap = 0; > info->max_dsc_bpp = 0; > + info->dp_dsc_bpp = 0; > > kfree(info->vics); > info->vics = NULL; > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 7b5048516185..1d01e0146a7f 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -719,6 +719,12 @@ struct drm_display_info { >*/ > u32 max_dsc_bpp; > > + /** > + * @dp_dsc_bpp: DP Display-Stream-Compression (DSC) timing's target > + * DST bits per pixel in 6.4 fixed point format. 0 means undefined > + */ > + u16 dp_dsc_bpp; > + > /** >* @vics: Array of vics_len VICs. Internal to EDID parsing. >*/ > diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h > index 49649eb8447e..0fc3afbd1675 100644 > --- a/include/drm/drm_displayid.h > +++ b/include/drm/drm_displayid.h > @@ -131,12 +131,16 @@ struct displayid_detailed_timing_block { > > #define DISPLAYID_VESA_MSO_OVERLAP GENMASK(3, 0) > #define DISPLAYID_VESA_MSO_MODE GENMASK(6, 5) > +#define DISPLAYID_VESA_DSC_BPP_INT GENMASK(5, 0) > +#define DISPLAYID_VESA_DSC_BPP_FRACT GENMASK(3, 0) > > struct
Re: [PATCH 2/2] drm/amdgpu: Synchronize after mapping into a compute VM
On 2023-02-27 04:42, Christian König wrote: Am 25.02.23 um 00:36 schrieb Felix Kuehling: Compute VMs use user mode queues for command submission. They cannot use a CS ioctl to synchronize with pending PTE updates and flush TLBs. Do this synchronization in amdgpu_gem_va_ioctl for compute VMs. Signed-off-by: Felix Kuehling --- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index 6936cd63df42..7de5057c40ec 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -601,7 +601,7 @@ int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void *data, static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev, struct amdgpu_vm *vm, struct amdgpu_bo_va *bo_va, - uint32_t operation) + uint32_t operation, uint32_t flags) { int r; @@ -620,6 +620,18 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev, } r = amdgpu_vm_update_pdes(adev, vm, false); + if (r) + goto error; + + if (vm->is_compute_context) { + if (bo_va->last_pt_update) + r = dma_fence_wait(bo_va->last_pt_update, true); + if (!r && vm->last_update) + r = dma_fence_wait(vm->last_update, true); + if (!r) + r = amdgpu_amdkfd_flush_tlb(adev, vm, + TLB_FLUSH_LEGACY); + } Mhm, that's not really something we can do here. The GEM VA IOCTL is supposed to be async and never block. Can we do that on the KFD side in some IOCTL instead? Not really. There is no existing KFD ioctl I would call after GEM_VA. The whole point was to use GEM ioctls to manages virtual address mappings for KFD and avoid adding more KFD ioctls that duplicate similar functionality. If you wanted to tie it into the existing amdgpu memory manager, I guess we'd need a dummy command submission that does the synchronization and TLB flush and the wait for the fence from that. Regards, Felix Regards, Christian. error: if (r && r != -ERESTARTSYS) @@ -789,7 +801,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data, } if (!r && !(args->flags & AMDGPU_VM_DELAY_UPDATE) && !amdgpu_vm_debug) amdgpu_gem_va_update_vm(adev, >vm, bo_va, - args->operation); + args->operation, args->flags); error_backoff: ttm_eu_backoff_reservation(, );
Re: [PATCH] drm/amdgpu: fix return value check in kfd
Am 27.02.23 um 15:52 schrieb Shashank Sharma: From: Shashank Sharma This patch fixes a return value check in kfd doorbell handling. This function should return 0(error) only when the ida_simple_get returns < 0(error), return > 0 is a success case. Cc: Felix Kuehling Cc: Alex Deucher Signed-off-by: Shashank Sharma Acked-by: Christian König --- drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c index cbef2e147da5..38c9e1ca6691 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c @@ -280,7 +280,7 @@ phys_addr_t kfd_get_process_doorbells(struct kfd_process_device *pdd) if (!pdd->doorbell_index) { int r = kfd_alloc_process_doorbells(pdd->dev, >doorbell_index); - if (r) + if (r < 0) return 0; }
RE: [PATCH 00/20] DC Patches Feb 27th, 2023
[Public] Hi all, This week this patchset was tested on the following systems: Lenovo Thinkpad T14s Gen2, with AMD Ryzen 5 5650U Lenovo Thinkpad T13s Gen4 with AMD Ryzen 5 6600U Reference AMD RX6800 These systems were tested on the following display types: eDP, (1080p 60hz [5650U]) (1920x1200 60hz [6600U]) (2560x1600 120hz[6600U]) VGA and DVI (1680x1050 60HZ [DP to VGA/DVI, USB-C to DVI/VGA]) DP/HDMI/USB-C (1440p 170hz, 4k 60hz, 4k 144hz [Includes USB-C to DP/HDMI adapters]) MST tested with Startech MST14DP123DP and 2x 4k 60Hz displays DSC tested with Cable Matters 101075 (DP to 3x DP), and 201375 (USB-C to 3x DP) with 3x 4k60 displays HP Hook G2 with 1 and 2 4k60 Displays The testing is a mix of automated and manual tests. Manual testing includes (but is not limited to): Changing display configurations and settings Benchmark testing Feature testing (Freesync, etc.) Automated testing includes (but is not limited to): Script testing (scripts to automate some of the manual checks) IGT testing The patchset consists of the amd-staging-drm-next branch (Head commit aaac77ad65330444aa506614529bb7883e024d9e -> Revert "drm: hide unregistered connectors from GETCONNECTOR IOCTL") with new patches added on top of it. This branch is used for both Ubuntu and Chrome OS testing (ChromeOS on a bi-weekly basis). Tested on Ubuntu 22.04.1 and Chrome OS Tested-by: Daniel Wheeler Thank you, Dan Wheeler Sr. Technologist | AMD SW Display -- 1 Commerce Valley Dr E, Thornhill, ON L3T 7X6 amd.com -Original Message- From: Zhuo, Qingqing (Lillian) Sent: February 22, 2023 1:40 AM To: amd-gfx@lists.freedesktop.org Cc: Wentland, Harry ; Li, Sun peng (Leo) ; Lakha, Bhawanpreet ; Siqueira, Rodrigo ; Pillai, Aurabindo ; Zhuo, Qingqing (Lillian) ; Li, Roman ; Lin, Wayne ; Wang, Chao-kai (Stylon) ; Chiu, Solomon ; Kotarac, Pavle ; Gutierrez, Agustin ; Wheeler, Daniel Subject: [PATCH 00/20] DC Patches Feb 27th, 2023 This DC patchset brings improvements in multiple areas. In summary, we highlight: - Correct DML calculation - Extend Freesync over Pcon support - Fixes in pstate hang and more - Code cleanup for dc_link.h and dc_link.c Cc: Daniel Wheeler Alex Hung (1): drm/amd/display: fix shift-out-of-bounds in CalculateVMAndRowBytes Alvin Lee (1): drm/amd/display: DAL to program DISPCLK WDIVIDER if PMFW doesn't Aric Cyr (6): drm/amd/display: Reduce CPU busy-waiting for long delays Revert "drm/amd/display: Do not set DRR on pipe commit" Revert "drm/amd/display: Fix FreeSync active bit issue" drm/amd/display: Do not update DRR while BW optimizations pending drm/amd/display: Only wait for blank completion if OTG active drm/amd/display: Promote DAL to 3.2.224 Ayush Gupta (1): drm/amd/display: populate subvp cmd info only for the top pipe Hersen Wu (1): drm/amd/display: dcn32/321 dsc_pg_control not executed properly Jasdeep Dhillon (1): drm/amd/display: Updating Video Format Fall Back Policy. Mustapha Ghaddar (2): drm/amd/display: Allocation at stream Enable drm/amd/display: Update BW ALLOCATION Function declaration Paul Hsieh (1): drm/amd/display: Correct DML calculation as HW SpreadSheet Ryan Lin (1): drm/amd/display: Ext displays with dock can't recognized after resume Samson Tam (1): drm/amd/display: enable DPG when disabling plane for phantom pipe Sung Joon Kim (1): drm/amd/display: Extend Freesync over PCon support for more devices Wenjing Liu (2): drm/amd/display: merge dc_link.h into dc.h and dc_types.h drm/amd/display: remove empty dc_link.c Yihan Zhu (1): drm/amd/display: update pixel format in DP hw sequence .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 17 +- .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 2 +- .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c| 1 + .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 + .../gpu/drm/amd/display/dc/clk_mgr/clk_mgr.c | 4 +- .../display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c | 41 +- drivers/gpu/drm/amd/display/dc/core/dc.c | 253 +++- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 27 - .../drm/amd/display/dc/core/dc_link_exports.c | 87 +++ drivers/gpu/drm/amd/display/dc/dc.h | 556 +- drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c | 3 +- drivers/gpu/drm/amd/display/dc/dc_dp_types.h | 107 drivers/gpu/drm/amd/display/dc/dc_hw_types.h | 14 + drivers/gpu/drm/amd/display/dc/dc_link.h | 8 +- drivers/gpu/drm/amd/display/dc/dc_types.h | 104 drivers/gpu/drm/amd/display/dc/dce/dce_aux.c | 9 +- drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c | 2 +- drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c | 2 +- drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c | 2 +- drivers/gpu/drm/amd/display/dc/dce/dmub_psr.h | 5 +- .../display/dc/dce110/dce110_hw_sequencer.c | 2 +-
[PATCH] drm/amdgpu: fix return value check in kfd
From: Shashank Sharma This patch fixes a return value check in kfd doorbell handling. This function should return 0(error) only when the ida_simple_get returns < 0(error), return > 0 is a success case. Cc: Felix Kuehling Cc: Alex Deucher Signed-off-by: Shashank Sharma --- drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c index cbef2e147da5..38c9e1ca6691 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c @@ -280,7 +280,7 @@ phys_addr_t kfd_get_process_doorbells(struct kfd_process_device *pdd) if (!pdd->doorbell_index) { int r = kfd_alloc_process_doorbells(pdd->dev, >doorbell_index); - if (r) + if (r < 0) return 0; } -- 2.34.1
Re: [PATCH] amdgpu: add a filter condition when set brightness
On 2/27/23 01:39, Yuanzhi Wang wrote: When the laptop is plugged into AC or DC power supply, the brightness obtained ACPI may be smaller than current brightness.As a result the screen becomes dark,this is not what people want. Do you have a matching bug report with more information included? Some relevant details I think we need: kernel version laptop model BIOS version if it's the latest BIOS some sample numbers that this new warning/behavior catches acpidump At least as described this sounds like a BIOS bug. Signed-off-by: Yuanzhi Wang --- drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c index d4196fcb85a0..93f1567028c5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c @@ -406,6 +406,7 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev, { struct amdgpu_atif *atif = _acpi_priv.atif; int count; + int old_brightness; DRM_DEBUG_DRIVER("event, device_class = %s, type = %#x\n", event->device_class, event->type); @@ -443,7 +444,13 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev, * hardwired to post BACKLIGHT_UPDATE_SYSFS. * It probably should accept 'reason' parameter. */ - backlight_device_set_brightness(atif->bd, req.backlight_level); + old_brightness = backlight_get_brightness(atif->bd); + if (old_brightness > req.backlight_level) + DRM_WARN("old brightness %d is greater than ACPI brightness + %d\n", old_brightness, req.backlight_level); + else + backlight_device_set_brightness(atif->bd, + req.backlight_level); } }
Re: [PATCH v3 1/2] drm/edid: parse DRM VESA dsc bpp target
On Sun, 26 Feb 2023, Yaroslav Bolyukin wrote: > As per DisplayID v2.0 Errata E9 spec "DSC pass-through timing support" > VESA vendor-specific data block may contain target DSC bits per pixel > fields > > Signed-off-by: Yaroslav Bolyukin > --- > drivers/gpu/drm/drm_edid.c | 38 + > include/drm/drm_connector.h | 6 ++ > include/drm/drm_displayid.h | 4 > 3 files changed, 36 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 3d0a4da661bc..aa88ac82cbe0 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -6338,7 +6338,7 @@ static void drm_parse_vesa_mso_data(struct > drm_connector *connector, > if (oui(vesa->oui[0], vesa->oui[1], vesa->oui[2]) != VESA_IEEE_OUI) > return; > > - if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) { > + if (block->num_bytes < 5) { > drm_dbg_kms(connector->dev, > "[CONNECTOR:%d:%s] Unexpected VESA vendor block > size\n", > connector->base.id, connector->name); > @@ -6361,24 +6361,37 @@ static void drm_parse_vesa_mso_data(struct > drm_connector *connector, > break; > } > > - if (!info->mso_stream_count) { > - info->mso_pixel_overlap = 0; > - return; > - } > + info->mso_pixel_overlap = 0; > + > + if (info->mso_stream_count) { > + info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, > vesa->mso); > + > + if (info->mso_pixel_overlap > 8) { > + drm_dbg_kms(connector->dev, > + "[CONNECTOR:%d:%s] Reserved MSO pixel > overlap value %u\n", > + connector->base.id, connector->name, > + info->mso_pixel_overlap); > + info->mso_pixel_overlap = 8; > + } > > - info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, > vesa->mso); > - if (info->mso_pixel_overlap > 8) { > drm_dbg_kms(connector->dev, > - "[CONNECTOR:%d:%s] Reserved MSO pixel overlap value > %u\n", > + "[CONNECTOR:%d:%s] MSO stream count %u, pixel > overlap %u\n", > connector->base.id, connector->name, > - info->mso_pixel_overlap); > - info->mso_pixel_overlap = 8; > + info->mso_stream_count, info->mso_pixel_overlap); > + } > + > + if (block->num_bytes < 7) { > + /* DSC bpp is optional */ > + return; > } > > + info->dp_dsc_bpp = FIELD_GET(DISPLAYID_VESA_DSC_BPP_INT, > vesa->dsc_bpp_int) * 16 > + + FIELD_GET(DISPLAYID_VESA_DSC_BPP_FRACT, vesa->dsc_bpp_fract); > + Matter of taste, but I'd probably use << 4 and |. *shrug* > drm_dbg_kms(connector->dev, > - "[CONNECTOR:%d:%s] MSO stream count %u, pixel overlap %u\n", > + "[CONNECTOR:%d:%s] DSC bits per pixel %u\n", > connector->base.id, connector->name, > - info->mso_stream_count, info->mso_pixel_overlap); > + info->dp_dsc_bpp); > } > > static void drm_update_mso(struct drm_connector *connector, > @@ -6425,6 +6438,7 @@ static void drm_reset_display_info(struct drm_connector > *connector) > info->mso_stream_count = 0; > info->mso_pixel_overlap = 0; > info->max_dsc_bpp = 0; > + info->dp_dsc_bpp = 0; > > kfree(info->vics); > info->vics = NULL; > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 7b5048516185..1d01e0146a7f 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -719,6 +719,12 @@ struct drm_display_info { >*/ > u32 max_dsc_bpp; > > + /** > + * @dp_dsc_bpp: DP Display-Stream-Compression (DSC) timing's target > + * DST bits per pixel in 6.4 fixed point format. 0 means undefined DST? Reviewed-by: Jani Nikula > + */ > + u16 dp_dsc_bpp; > + > /** >* @vics: Array of vics_len VICs. Internal to EDID parsing. >*/ > diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h > index 49649eb8447e..0fc3afbd1675 100644 > --- a/include/drm/drm_displayid.h > +++ b/include/drm/drm_displayid.h > @@ -131,12 +131,16 @@ struct displayid_detailed_timing_block { > > #define DISPLAYID_VESA_MSO_OVERLAP GENMASK(3, 0) > #define DISPLAYID_VESA_MSO_MODE GENMASK(6, 5) > +#define DISPLAYID_VESA_DSC_BPP_INT GENMASK(5, 0) > +#define DISPLAYID_VESA_DSC_BPP_FRACT GENMASK(3, 0) > > struct displayid_vesa_vendor_specific_block { > struct displayid_block base; > u8 oui[3]; > u8 data_structure_type; > u8 mso; > + u8 dsc_bpp_int; > + u8 dsc_bpp_fract; > } __packed; > > /* DisplayID iteration */
[linux-next:master] BUILD REGRESSION 7f7a8831520f12a3cf894b0627641fad33971221
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 7f7a8831520f12a3cf894b0627641fad33971221 Add linux-next specific files for 20230227 Error/Warning reports: https://lore.kernel.org/oe-kbuild-all/202302111601.jty4lkra-...@intel.com https://lore.kernel.org/oe-kbuild-all/202302170355.ljqlzucu-...@intel.com https://lore.kernel.org/oe-kbuild-all/202302210350.lynwcl4t-...@intel.com https://lore.kernel.org/oe-kbuild-all/202302242257.4w4myb9z-...@intel.com https://lore.kernel.org/oe-kbuild-all/202302262324.xemtp2zk-...@intel.com Error/Warning: (recently discovered and may have been fixed) FAILED: load BTF from vmlinux: No data available drivers/block/zram/zram_drv.c:1234:23: error: incompatible pointer types passing 'atomic_long_t *' (aka 'atomic_t *') to parameter of type 'const atomic64_t *' [-Werror,-Wincompatible-pointer-types] drivers/block/zram/zram_drv.c:1234:44: error: passing argument 1 of 'atomic64_read' from incompatible pointer type [-Werror=incompatible-pointer-types] drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_optc.c:294:6: warning: no previous prototype for 'optc3_wait_drr_doublebuffer_pending_clear' [-Wmissing-prototypes] idma64.c:(.text+0x145a): undefined reference to `devm_platform_ioremap_resource' idma64.c:(.text+0xacc): undefined reference to `devm_platform_ioremap_resource' include/asm-generic/div64.h:238:36: error: passing argument 1 of '__div64_32' from incompatible pointer type [-Werror=incompatible-pointer-types] Unverified Error/Warning (likely false positive, please contact us if interested): drivers/usb/gadget/composite.c:2082:33: sparse: sparse: restricted __le16 degrades to integer mm/page_alloc.c:257:1: sparse: sparse: symbol 'check_pages_enabled' was not declared. Should it be static? Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-randconfig-s031-20230226 | |-- drivers-usb-gadget-composite.c:sparse:sparse:restricted-__le16-degrades-to-integer | `-- mm-page_alloc.c:sparse:sparse:symbol-check_pages_enabled-was-not-declared.-Should-it-be-static |-- arc-allyesconfig | |-- drivers-block-zram-zram_drv.c:error:passing-argument-of-atomic64_read-from-incompatible-pointer-type | `-- include-asm-generic-div64.h:error:passing-argument-of-__div64_32-from-incompatible-pointer-type |-- arm-allmodconfig | |-- drivers-block-zram-zram_drv.c:error:passing-argument-of-atomic64_read-from-incompatible-pointer-type | `-- include-asm-generic-div64.h:error:passing-argument-of-__div64_32-from-incompatible-pointer-type |-- arm-allyesconfig | |-- drivers-block-zram-zram_drv.c:error:passing-argument-of-atomic64_read-from-incompatible-pointer-type | `-- include-asm-generic-div64.h:error:passing-argument-of-__div64_32-from-incompatible-pointer-type |-- arm64-allyesconfig | `-- drivers-gpu-drm-amd-amdgpu-..-display-dc-dcn30-dcn30_optc.c:warning:no-previous-prototype-for-optc3_wait_drr_doublebuffer_pending_clear |-- csky-allmodconfig | |-- drivers-block-zram-zram_drv.c:error:passing-argument-of-atomic64_read-from-incompatible-pointer-type | `-- include-asm-generic-div64.h:error:passing-argument-of-__div64_32-from-incompatible-pointer-type |-- csky-randconfig-r001-20230226 | `-- drivers-block-zram-zram_drv.c:error:passing-argument-of-atomic64_read-from-incompatible-pointer-type |-- i386-allyesconfig | |-- drivers-block-zram-zram_drv.c:error:passing-argument-of-atomic64_read-from-incompatible-pointer-type | `-- drivers-gpu-drm-amd-amdgpu-..-display-dc-dcn30-dcn30_optc.c:warning:no-previous-prototype-for-optc3_wait_drr_doublebuffer_pending_clear |-- m68k-allmodconfig | `-- drivers-block-zram-zram_drv.c:error:passing-argument-of-atomic64_read-from-incompatible-pointer-type |-- m68k-defconfig | `-- drivers-block-zram-zram_drv.c:error:passing-argument-of-atomic64_read-from-incompatible-pointer-type |-- m68k-hp300_defconfig | `-- drivers-block-zram-zram_drv.c:error:passing-argument-of-atomic64_read-from-incompatible-pointer-type |-- m68k-mvme16x_defconfig | `-- drivers-block-zram-zram_drv.c:error:passing-argument-of-atomic64_read-from-incompatible-pointer-type |-- microblaze-randconfig-c043-20230226 | `-- drivers-block-zram-zram_drv.c:error:passing-argument-of-atomic64_read-from-incompatible-pointer-type |-- mips-allmodconfig | `-- drivers-block-zram-zram_drv.c:error:passing-argument-of-atomic64_read-from-incompatible-pointer-type |-- mips-allyesconfig | `-- drivers-block-zram-zram_drv.c:error:passing-argument-of-atomic64_read-from-incompatible-pointer-type |-- nios2-randconfig-s032-20230226 | `-- mm-page_alloc.c:sparse:sparse:symbol-check_pages_enabled-was-not-declared.-Should-it-be-static |-- nios2-randconfig-s053-20230226 | |-- drivers-usb-gadget-composite.c:sparse:sparse:restricted-__le16-degrades-to-integer | |-- include-asm-generic-cmpxchg-local.h:sparse:sparse:cast-truncates-bits-from-constant-value-(-becomes-) | |-- include-asm-generic
Re: [PATCH] amdgpu: add a filter condition when set brightness
Hi Yuanzhi, Thank you for the patch! Yet something to improve: [auto build test ERROR on drm-misc/drm-misc-next] [also build test ERROR on linus/master v6.2 next-20230227] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Yuanzhi-Wang/amdgpu-add-a-filter-condition-when-set-brightness/20230227-154108 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20230227073953.326-1-wangyuanzhi%40uniontech.com patch subject: [PATCH] amdgpu: add a filter condition when set brightness config: loongarch-randconfig-r036-20230226 (https://download.01.org/0day-ci/archive/20230227/202302272122.p3dfx4s8-...@intel.com/config) compiler: loongarch64-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/95d9579e31d0b601aa1422cf767ca5138d3efcee git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Yuanzhi-Wang/amdgpu-add-a-filter-condition-when-set-brightness/20230227-154108 git checkout 95d9579e31d0b601aa1422cf767ca5138d3efcee # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash drivers/gpu/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot | Link: https://lore.kernel.org/oe-kbuild-all/202302272122.p3dfx4s8-...@intel.com/ All errors (new ones prefixed by >>): drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c: In function 'amdgpu_atif_handler': drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:50: warning: missing terminating " character 449 | DRM_WARN("old brightness %d is greater than ACPI brightness | ^ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:450:53: warning: missing terminating " character 450 | %d\n", old_brightness, req.backlight_level); | ^ >> cc1: error: unterminated argument list invoking macro "DRM_WARN" >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:41: error: 'DRM_WARN' >> undeclared (first use in this function) 449 | DRM_WARN("old brightness %d is greater than ACPI brightness | ^~~~ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:41: note: each undeclared identifier is reported only once for each function it appears in >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:49: error: expected ';' at end >> of input 449 | DRM_WARN("old brightness %d is greater than ACPI brightness | ^ | ; .. drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:448:33: note: '-Wmisleading-indentation' is disabled from this point onwards, since column-tracking was disabled due to the size of the code/headers 448 | if (old_brightness > req.backlight_level) | ^~ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:448:33: note: adding '-flarge-source-files' will allow for more column-tracking support, at the expense of compilation time and memory >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:41: error: expected declaration >> or statement at end of input 449 | DRM_WARN("old brightness %d is greater than ACPI brightness | ^~~~ >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:41: error: expected declaration >> or statement at end of input >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:41: error: expected declaration >> or statement at end of input >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:41: error: expected declaration >> or statement at end of input drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c: At top level: drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:404:12: warning: 'amdgpu_atif_handler' defined but not used [-Wunused-function] 404 | static int amdgpu_atif_handler(struct amdgpu_device *adev,
Re: [PATCH 5/6] drm/amdgpu: Implement userqueue signal/wait IOCTL functions
On 2/27/2023 6:53 PM, Christian König wrote: Hi Arun, Am 27.02.23 um 14:20 schrieb Arunpravin Paneer Selvam: Hi Christian, On 2/27/2023 6:29 PM, Christian König wrote: Am 26.02.23 um 17:54 schrieb Arunpravin Paneer Selvam: This patch introduces new IOCTL for userqueue secure semaphore. The signal IOCTL called from userspace application creates a drm syncobj and array of bo GEM handles and passed in as parameter to the driver to install the fence into it. The wait IOCTL gets an array of drm syncobjs, finds the fences attached to the drm syncobjs and obtain the array of memory_address/fence_value combintion which are returned to userspace. v2: Worked on review comments from Christian for the following modifications - Install fence into GEM BO object. - Lock all BO's using the dma resv subsystem - Reorder the sequence in signal IOCTL function. - Get write pointer from the shadow wptr - use userq_fence to fetch the va/value in wait IOCTL. Signed-off-by: Arunpravin Paneer Selvam --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 + drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 + .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 258 ++ .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 6 + drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 1 + 5 files changed, 270 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 1c3eba2d0390..255d73795493 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -964,6 +964,8 @@ struct amdgpu_device { struct amdgpu_mes mes; struct amdgpu_mqd mqds[AMDGPU_HW_IP_NUM]; + struct amdgpu_userq_mgr *userq_mgr; + /* df */ struct amdgpu_df df; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 6b7ac1ebd04c..66a7304fabe3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2752,6 +2752,9 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_USERQ, amdgpu_userq_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_DOORBELL_RING, amdgpu_userq_doorbell_ring_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_SIGNAL, amdgpu_userq_signal_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_WAIT, amdgpu_userq_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + }; static const struct drm_driver amdgpu_kms_driver = { diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c index 609a7328e9a6..26fd1d4f758a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c @@ -249,3 +249,261 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = { .signaled = amdgpu_userq_fence_signaled, .release = amdgpu_userq_fence_release, }; + +static int amdgpu_userq_fence_read_wptr(struct drm_file *filp, + struct amdgpu_usermode_queue *queue, + u64 *wptr) +{ + struct amdgpu_fpriv *fpriv = filp->driver_priv; + struct amdgpu_bo_va_mapping *mapping; + struct amdgpu_vm *vm = >vm; + struct amdgpu_bo *bo; + u64 *ptr; + int r; + + mapping = amdgpu_vm_bo_lookup_mapping(vm, queue->wptr_gpu_addr >> PAGE_SHIFT); + if (!mapping) + return -EINVAL; + + bo = mapping->bo_va->base.bo; + r = amdgpu_bo_kmap(bo, (void **)); Oh, that's not something you can do that easily. The BO must be reserved (locked) first if you want to call amdgpu_bo_kmap() on it. sure, I will take care + if (r) { + DRM_ERROR("Failed mapping the userqueue wptr bo"); + return r; + } + + *wptr = le64_to_cpu(*ptr); + + return 0; +} + +int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp) +{ + struct drm_amdgpu_userq_signal *args = data; + struct amdgpu_device *adev = drm_to_adev(dev); + struct amdgpu_userq_mgr *userq_mgr = adev->userq_mgr; + struct amdgpu_usermode_queue *queue; + struct drm_syncobj *syncobj = NULL; + struct drm_gem_object **gobj; + u64 num_bo_handles, wptr; + struct dma_fence *fence; + u32 *bo_handles; + bool shared; + int r, i; + + /* Retrieve the user queue */ + queue = idr_find(_mgr->userq_idr, args->queue_id); + if (!queue) + return -ENOENT; + + r = amdgpu_userq_fence_read_wptr(filp, queue, ); + if (r) + return -EINVAL; + + /* Find Syncobj if any */ + syncobj = drm_syncobj_find(filp, args->handle); + + /* Array of bo handles */ + num_bo_handles = args->num_bo_handles; + bo_handles = kmalloc_array(num_bo_handles, sizeof(*bo_handles), GFP_KERNEL);
Re: [PATCH 5/6] drm/amdgpu: Implement userqueue signal/wait IOCTL functions
Hi Arun, Am 27.02.23 um 14:20 schrieb Arunpravin Paneer Selvam: Hi Christian, On 2/27/2023 6:29 PM, Christian König wrote: Am 26.02.23 um 17:54 schrieb Arunpravin Paneer Selvam: This patch introduces new IOCTL for userqueue secure semaphore. The signal IOCTL called from userspace application creates a drm syncobj and array of bo GEM handles and passed in as parameter to the driver to install the fence into it. The wait IOCTL gets an array of drm syncobjs, finds the fences attached to the drm syncobjs and obtain the array of memory_address/fence_value combintion which are returned to userspace. v2: Worked on review comments from Christian for the following modifications - Install fence into GEM BO object. - Lock all BO's using the dma resv subsystem - Reorder the sequence in signal IOCTL function. - Get write pointer from the shadow wptr - use userq_fence to fetch the va/value in wait IOCTL. Signed-off-by: Arunpravin Paneer Selvam --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 + drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 + .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 258 ++ .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 6 + drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 1 + 5 files changed, 270 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 1c3eba2d0390..255d73795493 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -964,6 +964,8 @@ struct amdgpu_device { struct amdgpu_mes mes; struct amdgpu_mqd mqds[AMDGPU_HW_IP_NUM]; + struct amdgpu_userq_mgr *userq_mgr; + /* df */ struct amdgpu_df df; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 6b7ac1ebd04c..66a7304fabe3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2752,6 +2752,9 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_USERQ, amdgpu_userq_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_DOORBELL_RING, amdgpu_userq_doorbell_ring_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_SIGNAL, amdgpu_userq_signal_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_WAIT, amdgpu_userq_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + }; static const struct drm_driver amdgpu_kms_driver = { diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c index 609a7328e9a6..26fd1d4f758a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c @@ -249,3 +249,261 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = { .signaled = amdgpu_userq_fence_signaled, .release = amdgpu_userq_fence_release, }; + +static int amdgpu_userq_fence_read_wptr(struct drm_file *filp, + struct amdgpu_usermode_queue *queue, + u64 *wptr) +{ + struct amdgpu_fpriv *fpriv = filp->driver_priv; + struct amdgpu_bo_va_mapping *mapping; + struct amdgpu_vm *vm = >vm; + struct amdgpu_bo *bo; + u64 *ptr; + int r; + + mapping = amdgpu_vm_bo_lookup_mapping(vm, queue->wptr_gpu_addr >> PAGE_SHIFT); + if (!mapping) + return -EINVAL; + + bo = mapping->bo_va->base.bo; + r = amdgpu_bo_kmap(bo, (void **)); Oh, that's not something you can do that easily. The BO must be reserved (locked) first if you want to call amdgpu_bo_kmap() on it. sure, I will take care + if (r) { + DRM_ERROR("Failed mapping the userqueue wptr bo"); + return r; + } + + *wptr = le64_to_cpu(*ptr); + + return 0; +} + +int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp) +{ + struct drm_amdgpu_userq_signal *args = data; + struct amdgpu_device *adev = drm_to_adev(dev); + struct amdgpu_userq_mgr *userq_mgr = adev->userq_mgr; + struct amdgpu_usermode_queue *queue; + struct drm_syncobj *syncobj = NULL; + struct drm_gem_object **gobj; + u64 num_bo_handles, wptr; + struct dma_fence *fence; + u32 *bo_handles; + bool shared; + int r, i; + + /* Retrieve the user queue */ + queue = idr_find(_mgr->userq_idr, args->queue_id); + if (!queue) + return -ENOENT; + + r = amdgpu_userq_fence_read_wptr(filp, queue, ); + if (r) + return -EINVAL; + + /* Find Syncobj if any */ + syncobj = drm_syncobj_find(filp, args->handle); + + /* Array of bo handles */ + num_bo_handles = args->num_bo_handles; + bo_handles = kmalloc_array(num_bo_handles, sizeof(*bo_handles), GFP_KERNEL); + if (bo_handles == NULL) +
Re: [PATCH 5/6] drm/amdgpu: Implement userqueue signal/wait IOCTL functions
Hi Christian, On 2/27/2023 6:29 PM, Christian König wrote: Am 26.02.23 um 17:54 schrieb Arunpravin Paneer Selvam: This patch introduces new IOCTL for userqueue secure semaphore. The signal IOCTL called from userspace application creates a drm syncobj and array of bo GEM handles and passed in as parameter to the driver to install the fence into it. The wait IOCTL gets an array of drm syncobjs, finds the fences attached to the drm syncobjs and obtain the array of memory_address/fence_value combintion which are returned to userspace. v2: Worked on review comments from Christian for the following modifications - Install fence into GEM BO object. - Lock all BO's using the dma resv subsystem - Reorder the sequence in signal IOCTL function. - Get write pointer from the shadow wptr - use userq_fence to fetch the va/value in wait IOCTL. Signed-off-by: Arunpravin Paneer Selvam --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 + drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 + .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 258 ++ .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 6 + drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 1 + 5 files changed, 270 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 1c3eba2d0390..255d73795493 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -964,6 +964,8 @@ struct amdgpu_device { struct amdgpu_mes mes; struct amdgpu_mqd mqds[AMDGPU_HW_IP_NUM]; + struct amdgpu_userq_mgr *userq_mgr; + /* df */ struct amdgpu_df df; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 6b7ac1ebd04c..66a7304fabe3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2752,6 +2752,9 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_USERQ, amdgpu_userq_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_DOORBELL_RING, amdgpu_userq_doorbell_ring_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_SIGNAL, amdgpu_userq_signal_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_WAIT, amdgpu_userq_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + }; static const struct drm_driver amdgpu_kms_driver = { diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c index 609a7328e9a6..26fd1d4f758a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c @@ -249,3 +249,261 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = { .signaled = amdgpu_userq_fence_signaled, .release = amdgpu_userq_fence_release, }; + +static int amdgpu_userq_fence_read_wptr(struct drm_file *filp, + struct amdgpu_usermode_queue *queue, + u64 *wptr) +{ + struct amdgpu_fpriv *fpriv = filp->driver_priv; + struct amdgpu_bo_va_mapping *mapping; + struct amdgpu_vm *vm = >vm; + struct amdgpu_bo *bo; + u64 *ptr; + int r; + + mapping = amdgpu_vm_bo_lookup_mapping(vm, queue->wptr_gpu_addr >> PAGE_SHIFT); + if (!mapping) + return -EINVAL; + + bo = mapping->bo_va->base.bo; + r = amdgpu_bo_kmap(bo, (void **)); Oh, that's not something you can do that easily. The BO must be reserved (locked) first if you want to call amdgpu_bo_kmap() on it. sure, I will take care + if (r) { + DRM_ERROR("Failed mapping the userqueue wptr bo"); + return r; + } + + *wptr = le64_to_cpu(*ptr); + + return 0; +} + +int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp) +{ + struct drm_amdgpu_userq_signal *args = data; + struct amdgpu_device *adev = drm_to_adev(dev); + struct amdgpu_userq_mgr *userq_mgr = adev->userq_mgr; + struct amdgpu_usermode_queue *queue; + struct drm_syncobj *syncobj = NULL; + struct drm_gem_object **gobj; + u64 num_bo_handles, wptr; + struct dma_fence *fence; + u32 *bo_handles; + bool shared; + int r, i; + + /* Retrieve the user queue */ + queue = idr_find(_mgr->userq_idr, args->queue_id); + if (!queue) + return -ENOENT; + + r = amdgpu_userq_fence_read_wptr(filp, queue, ); + if (r) + return -EINVAL; + + /* Find Syncobj if any */ + syncobj = drm_syncobj_find(filp, args->handle); + + /* Array of bo handles */ + num_bo_handles = args->num_bo_handles; + bo_handles = kmalloc_array(num_bo_handles, sizeof(*bo_handles), GFP_KERNEL); + if (bo_handles == NULL) + return -ENOMEM; + + if (copy_from_user(bo_handles,
Re: [PATCH 5/6] drm/amdgpu: Implement userqueue signal/wait IOCTL functions
Am 26.02.23 um 17:54 schrieb Arunpravin Paneer Selvam: This patch introduces new IOCTL for userqueue secure semaphore. The signal IOCTL called from userspace application creates a drm syncobj and array of bo GEM handles and passed in as parameter to the driver to install the fence into it. The wait IOCTL gets an array of drm syncobjs, finds the fences attached to the drm syncobjs and obtain the array of memory_address/fence_value combintion which are returned to userspace. v2: Worked on review comments from Christian for the following modifications - Install fence into GEM BO object. - Lock all BO's using the dma resv subsystem - Reorder the sequence in signal IOCTL function. - Get write pointer from the shadow wptr - use userq_fence to fetch the va/value in wait IOCTL. Signed-off-by: Arunpravin Paneer Selvam --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 + drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 + .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 258 ++ .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 6 + drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 1 + 5 files changed, 270 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 1c3eba2d0390..255d73795493 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -964,6 +964,8 @@ struct amdgpu_device { struct amdgpu_mes mes; struct amdgpu_mqd mqds[AMDGPU_HW_IP_NUM]; + struct amdgpu_userq_mgr *userq_mgr; + /* df */ struct amdgpu_dfdf; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 6b7ac1ebd04c..66a7304fabe3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2752,6 +2752,9 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_USERQ, amdgpu_userq_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_DOORBELL_RING, amdgpu_userq_doorbell_ring_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_SIGNAL, amdgpu_userq_signal_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_WAIT, amdgpu_userq_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + }; static const struct drm_driver amdgpu_kms_driver = { diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c index 609a7328e9a6..26fd1d4f758a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c @@ -249,3 +249,261 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = { .signaled = amdgpu_userq_fence_signaled, .release = amdgpu_userq_fence_release, }; + +static int amdgpu_userq_fence_read_wptr(struct drm_file *filp, + struct amdgpu_usermode_queue *queue, + u64 *wptr) +{ + struct amdgpu_fpriv *fpriv = filp->driver_priv; + struct amdgpu_bo_va_mapping *mapping; + struct amdgpu_vm *vm = >vm; + struct amdgpu_bo *bo; + u64 *ptr; + int r; + + mapping = amdgpu_vm_bo_lookup_mapping(vm, queue->wptr_gpu_addr >> PAGE_SHIFT); + if (!mapping) + return -EINVAL; + + bo = mapping->bo_va->base.bo; + r = amdgpu_bo_kmap(bo, (void **)); Oh, that's not something you can do that easily. The BO must be reserved (locked) first if you want to call amdgpu_bo_kmap() on it. + if (r) { + DRM_ERROR("Failed mapping the userqueue wptr bo"); + return r; + } + + *wptr = le64_to_cpu(*ptr); + + return 0; +} + +int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp) +{ + struct drm_amdgpu_userq_signal *args = data; + struct amdgpu_device *adev = drm_to_adev(dev); + struct amdgpu_userq_mgr *userq_mgr = adev->userq_mgr; + struct amdgpu_usermode_queue *queue; + struct drm_syncobj *syncobj = NULL; + struct drm_gem_object **gobj; + u64 num_bo_handles, wptr; + struct dma_fence *fence; + u32 *bo_handles; + bool shared; + int r, i; + + /* Retrieve the user queue */ + queue = idr_find(_mgr->userq_idr, args->queue_id); + if (!queue) + return -ENOENT; + + r = amdgpu_userq_fence_read_wptr(filp, queue, ); + if (r) + return -EINVAL; + + /* Find Syncobj if any */ + syncobj = drm_syncobj_find(filp, args->handle); + + /* Array of bo handles */ + num_bo_handles = args->num_bo_handles; + bo_handles = kmalloc_array(num_bo_handles,
Re: [PATCH 4/6] drm/amdgpu: UAPI headers for userqueue Secure semaphore
Am 26.02.23 um 17:54 schrieb Arunpravin Paneer Selvam: - Add UAPI header support for userqueue Secure semaphore v2: (Christian) - Add bo handles,bo flags and padding fields. - Include value/va in a combined array. Signed-off-by: Alex Deucher Signed-off-by: Arunpravin Paneer Selvam --- .../amd/amdgpu/amdgpu_userqueue_mqd_gfx_v11.c | 1 + include/uapi/drm/amdgpu_drm.h | 46 +++ 2 files changed, 47 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue_mqd_gfx_v11.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue_mqd_gfx_v11.c index b8943e6aea22..5cb255a39732 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue_mqd_gfx_v11.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue_mqd_gfx_v11.c @@ -22,6 +22,7 @@ */ #include "amdgpu.h" #include "amdgpu_userqueue.h" +#include "amdgpu_userq_fence.h" #include "v11_structs.h" #include "amdgpu_mes.h" #include "mes_api_def.h" diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h index 2d94cca566e0..bd37c715f5a7 100644 --- a/include/uapi/drm/amdgpu_drm.h +++ b/include/uapi/drm/amdgpu_drm.h @@ -56,6 +56,8 @@ extern "C" { #define DRM_AMDGPU_SCHED 0x15 #define DRM_AMDGPU_USERQ 0x16 #define DRM_AMDGPU_USERQ_DOORBELL_RING0x17 +#define DRM_AMDGPU_USERQ_SIGNAL0x18 +#define DRM_AMDGPU_USERQ_WAIT 0x19 #define DRM_IOCTL_AMDGPU_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_CREATE, union drm_amdgpu_gem_create) #define DRM_IOCTL_AMDGPU_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_MMAP, union drm_amdgpu_gem_mmap) @@ -75,6 +77,8 @@ extern "C" { #define DRM_IOCTL_AMDGPU_SCHEDDRM_IOW(DRM_COMMAND_BASE + DRM_AMDGPU_SCHED, union drm_amdgpu_sched) #define DRM_IOCTL_AMDGPU_USERQDRM_IOW(DRM_COMMAND_BASE + DRM_AMDGPU_USERQ, union drm_amdgpu_userq) #define DRM_IOCTL_AMDGPU_USERQ_DOORBELL_RING DRM_IOW(DRM_COMMAND_BASE + DRM_AMDGPU_USERQ_DOORBELL_RING, struct drm_amdgpu_db_ring) +#define DRM_IOCTL_AMDGPU_USERQ_SIGNAL DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_USERQ_SIGNAL, struct drm_amdgpu_userq_signal) +#define DRM_IOCTL_AMDGPU_USERQ_WAITDRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_USERQ_WAIT, struct drm_amdgpu_userq_wait) /** * DOC: memory domains @@ -361,6 +365,48 @@ union drm_amdgpu_userq { struct drm_amdgpu_userq_out out; }; +/* userq signal/wait ioctl */ +struct drm_amdgpu_userq_signal { + /** Queue ID */ + __u32 queue_id; + /** Flags */ + __u32 flags; + /** Sync obj handle */ + __u32 handle; Maybe rename to obj_handle or even syncobj_handle. + __u32 pad; + /* Sync obj timeline */ + __u64 point; Same prefix here, either obj_point or even better syncobj_point. + /** number of BO handles */ + __u64 num_bo_handles; This can be 32bit I think. And when you move the bo_flags after this field we don't even need a padding. + /** array of BO handles */ + __u64 bo_handles_array; + /** bo flags */ + __u32 bo_flags; +}; + +struct drm_amdgpu_userq_fence_info { + __u64 va; + __u64 value; +}; + +struct drm_amdgpu_userq_wait { + /** Flags */ + __u32 flags; + /** array of Sync obj handles */ + __u64 handles; + __u32 pad; That padding is misplaced as far as I can see. + /** number of Sync obj handles */ + __u64 count_handles; Better let's us use num_syncobj_handles here as well. And u32 should be sufficient. If you re-arrange the fields you could also drop the padding. + /** number of BO handles */ + __u64 num_bo_handles; Again u32 is sufficient for the number of handles. + /** array of BO handles */ + __u64 bo_handles_array; + /** bo flags */ + __u32 bo_wait_flags; + /** array of addr/values */ + __u64 userq_fence_info; We need a number for that as well. It can be that a BO is idle and has no fences and it can be that we have tons of fences on a single BO. The usual semantics is that you call the kernel driver with the userq_fence_info==0 first to get how much space you need to reserve and then once more after you allocated enough. See function sync_file_ioctl_fence_info() for a good example how to do this. Regards, Christian. +}; + /* vm ioctl */ #define AMDGPU_VM_OP_RESERVE_VMID 1 #define AMDGPU_VM_OP_UNRESERVE_VMID 2
Re: [PATCH 2/6] drm/amdgpu: Implement a new userqueue fence driver
Am 26.02.23 um 17:54 schrieb Arunpravin Paneer Selvam: Developed a userqueue fence driver for the userqueue process shared BO synchronization. Create a dma fence having write pointer as the seqno and allocate a seq64 memory for each user queue process and feed this memory address into the firmware/hardware, thus the firmware writes the read pointer into the given address when the process completes it execution. Compare wptr and rptr, if rptr >= wptr, signal the fences for the waiting process to consume the buffers. v2: Worked on review comments from Christian for the following modifications - Add wptr as sequence number into the fence - Add a reference count for the fence driver - Add dma_fence_put below the list_del as it might frees the userq fence. - Trim unnecessary code in interrupt handler. - Check dma fence signaled state in dma fence creation function for a potential problem of hardware completing the job processing beforehand. - Add necessary locks. - Create a list and process all the unsignaled fences. - clean up fences in destroy function. - implement .enabled callback function A few more nit picks below, but from the technical side that looks mostly clean. Signed-off-by: Arunpravin Paneer Selvam --- drivers/gpu/drm/amd/amdgpu/Makefile | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 + .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 251 ++ .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 61 + drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 20 ++ .../gpu/drm/amd/include/amdgpu_userqueue.h| 2 + 6 files changed, 341 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile index a239533a895f..ea09273b585f 100644 --- a/drivers/gpu/drm/amd/amdgpu/Makefile +++ b/drivers/gpu/drm/amd/amdgpu/Makefile @@ -59,7 +59,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \ amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \ amdgpu_fw_attestation.o amdgpu_securedisplay.o \ amdgpu_eeprom.o amdgpu_mca.o amdgpu_psp_ta.o amdgpu_lsdma.o \ - amdgpu_ring_mux.o amdgpu_seq64.o + amdgpu_ring_mux.o amdgpu_seq64.o amdgpu_userq_fence.o amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index bd3462d0da5f..6b7ac1ebd04c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -53,6 +53,7 @@ #include "amdgpu_xgmi.h" #include "amdgpu_reset.h" #include "amdgpu_userqueue.h" +#include "amdgpu_userq_fence.h" /* * KMS wrapper. @@ -2827,6 +2828,10 @@ static int __init amdgpu_init(void) if (r) goto error_fence; + r = amdgpu_userq_fence_slab_init(); + if (r) + goto error_fence; + DRM_INFO("amdgpu kernel modesetting enabled.\n"); amdgpu_register_atpx_handler(); amdgpu_acpi_detect(); @@ -2851,6 +2856,7 @@ static void __exit amdgpu_exit(void) amdgpu_unregister_atpx_handler(); amdgpu_sync_fini(); amdgpu_fence_slab_fini(); + amdgpu_userq_fence_slab_fini(); mmu_notifier_synchronize(); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c new file mode 100644 index ..609a7328e9a6 --- /dev/null +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c @@ -0,0 +1,251 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright 2023 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + */ + +#include +#include + +#include + +#include "amdgpu.h" +#include "amdgpu_userq_fence.h" +#include
Re: [PATCH] amdgpu: add a filter condition when set brightness
Hi Yuanzhi, Thank you for the patch! Yet something to improve: [auto build test ERROR on drm-misc/drm-misc-next] [also build test ERROR on linus/master v6.2 next-20230227] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Yuanzhi-Wang/amdgpu-add-a-filter-condition-when-set-brightness/20230227-154108 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20230227073953.326-1-wangyuanzhi%40uniontech.com patch subject: [PATCH] amdgpu: add a filter condition when set brightness config: arm64-randconfig-r012-20230226 (https://download.01.org/0day-ci/archive/20230227/202302272051.kdjqryl5-...@intel.com/config) compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project db89896bbbd2251fff457699635acbbedeead27f) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm64 cross compiling tool for clang build # apt-get install binutils-aarch64-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/95d9579e31d0b601aa1422cf767ca5138d3efcee git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Yuanzhi-Wang/amdgpu-add-a-filter-condition-when-set-brightness/20230227-154108 git checkout 95d9579e31d0b601aa1422cf767ca5138d3efcee # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/gpu/drm/amd/amdgpu/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot | Link: https://lore.kernel.org/oe-kbuild-all/202302272051.kdjqryl5-...@intel.com/ All error/warnings (new ones prefixed by >>): >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:15: warning: missing >> terminating '"' character [-Winvalid-pp-token] DRM_WARN("old brightness %d is greater than ACPI brightness ^ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:450:11: warning: missing terminating '"' character [-Winvalid-pp-token] %d\n", old_brightness, req.backlight_level); ^ >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:6: error: unterminated >> function-like macro invocation DRM_WARN("old brightness %d is greater than ACPI brightness ^ include/drm/drm_print.h:543:9: note: macro 'DRM_WARN' defined here #define DRM_WARN(fmt, ...) \ ^ >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:1105:28: error: expected expression #endif /* CONFIG_SUSPEND */ ^ >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:1105:28: warning: misleading >> indentation; statement is not part of the previous 'if' >> [-Wmisleading-indentation] drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:448:5: note: previous statement is here if (old_brightness > req.backlight_level) ^ >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:1105:28: error: expected '}' #endif /* CONFIG_SUSPEND */ ^ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:439:18: note: to match this '{' if (atif->bd) { ^ >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:1105:28: error: expected '}' #endif /* CONFIG_SUSPEND */ ^ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:438:59: note: to match this '{' if (req.pending & ATIF_PANEL_BRIGHTNESS_CHANGE_REQUEST) { ^ >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:1105:28: error: expected '}' #endif /* CONFIG_SUSPEND */ ^ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:427:38: note: to match this '{' if (atif->functions.sbios_requests) { ^ >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:1105:28: error: expected '}' #endif /* CONFIG_SUSPEND */ ^ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:406:1: note: to match this '{' {
Re: amdgpu didn't start with pci=nocrs parameter, get error "Fatal error during GPU init"
Am 24.02.23 um 17:21 schrieb Mikhail Gavrilov: On Fri, Feb 24, 2023 at 8:31 PM Christian König wrote: Sorry I totally missed that you attached the full dmesg to your original mail. Yeah, the driver did fail gracefully. But then X doesn't come up and then gdm just dies. Are you sure that these messages should be present when the driver fails gracefully? Unfortunately yes. We could clean that up a bit more so that you don't run into a BUG() assertion, but what essentially happens here is that we completely fail to talk to the hardware. In this situation we can't even re-enable vesa or text console any more. Regards, Christian. turning off the locking correctness validator. CPU: 14 PID: 470 Comm: (udev-worker) Tainted: G L --- --- 6.3.0-0.rc0.20230222git5b7c4cabbb65.3.fc39.x86_64+debug #1 Hardware name: ASUSTeK COMPUTER INC. ROG Strix G513QY_G513QY/G513QY, BIOS G513QY.320 09/07/2022 Call Trace: dump_stack_lvl+0x57/0x90 register_lock_class+0x47d/0x490 __lock_acquire+0x74/0x21f0 ? lock_release+0x155/0x450 lock_acquire+0xd2/0x320 ? amdgpu_irq_disable_all+0x37/0xf0 [amdgpu] ? lock_is_held_type+0xce/0x120 _raw_spin_lock_irqsave+0x4d/0xa0 ? amdgpu_irq_disable_all+0x37/0xf0 [amdgpu] amdgpu_irq_disable_all+0x37/0xf0 [amdgpu] amdgpu_device_fini_hw+0x43/0x2c0 [amdgpu] amdgpu_driver_load_kms+0xe8/0x190 [amdgpu] amdgpu_pci_probe+0x140/0x420 [amdgpu] local_pci_probe+0x41/0x90 pci_device_probe+0xc3/0x230 really_probe+0x1b6/0x410 __driver_probe_device+0x78/0x170 driver_probe_device+0x1f/0x90 __driver_attach+0xd2/0x1c0 ? __pfx___driver_attach+0x10/0x10 bus_for_each_dev+0x8a/0xd0 bus_add_driver+0x141/0x230 driver_register+0x77/0x120 ? __pfx_init_module+0x10/0x10 [amdgpu] do_one_initcall+0x6e/0x350 do_init_module+0x4a/0x220 __do_sys_init_module+0x192/0x1c0 do_syscall_64+0x5b/0x80 ? asm_exc_page_fault+0x22/0x30 ? lockdep_hardirqs_on+0x7d/0x100 entry_SYSCALL_64_after_hwframe+0x72/0xdc RIP: 0033:0x7fd58cfcb1be Code: 48 8b 0d 4d 0c 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 af 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1a 0c 0c 00 f7 d8 64 89 01 RSP: 002b:7ffd1d1065d8 EFLAGS: 0246 ORIG_RAX: 00af RAX: ffda RBX: 55b0b5aa6d70 RCX: 7fd58cfcb1be RDX: 55b0b5a96670 RSI: 016b6156 RDI: 7fd589392010 RBP: 7ffd1d106690 R08: 55b0b5a93bd0 R09: 016b6ff0 R10: 55b5eea2c333 R11: 0246 R12: 55b0b5a96670 R13: 0002 R14: 55b0b5a9c170 R15: 55b0b5aa58a0 amdgpu: probe of :03:00.0 failed with error -12 amdgpu :08:00.0: enabling device (0006 -> 0007) [drm] initializing kernel modesetting (RENOIR 0x1002:0x1638 0x1043:0x16C2 0xC4). list_add corruption. prev->next should be next (c0940328), but was . (prev=8c9b734062b0). [ cut here ] kernel BUG at lib/list_debug.c:30! invalid opcode: [#1] PREEMPT SMP NOPTI CPU: 14 PID: 470 Comm: (udev-worker) Tainted: G L --- --- 6.3.0-0.rc0.20230222git5b7c4cabbb65.3.fc39.x86_64+debug #1 Hardware name: ASUSTeK COMPUTER INC. ROG Strix G513QY_G513QY/G513QY, BIOS G513QY.320 09/07/2022 RIP: 0010:__list_add_valid+0x74/0x90 Code: 8d ff 0f 0b 48 89 c1 48 c7 c7 a0 3d b3 99 e8 a3 ed 8d ff 0f 0b 48 89 d1 48 89 c6 4c 89 c2 48 c7 c7 f8 3d b3 99 e8 8c ed 8d ff <0f> 0b 48 89 f2 48 89 c1 48 89 fe 48 c7 c7 50 3e b3 99 e8 75 ed 8d RSP: 0018:a50f81aafa00 EFLAGS: 00010246 RAX: 0075 RBX: 8c9b734062b0 RCX: RDX: RSI: 0027 RDI: RBP: 8c9b734062b0 R08: R09: a50f81aaf8a0 R10: 0003 R11: 8caa1d2fffe8 R12: 8c9b7c0a5e48 R13: R14: c13a6d20 R15: FS: 7fd58c6a5940() GS:8ca9d9a0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 55b0b5a955e0 CR3: 00017e86 CR4: 00750ee0 PKRU: 5554 Call Trace: ttm_device_init+0x184/0x1c0 [ttm] amdgpu_ttm_init+0xb8/0x610 [amdgpu] ? _printk+0x60/0x80 gmc_v9_0_sw_init+0x4a3/0x7c0 [amdgpu] amdgpu_device_init+0x14e5/0x2520 [amdgpu] amdgpu_driver_load_kms+0x15/0x190 [amdgpu] amdgpu_pci_probe+0x140/0x420 [amdgpu] local_pci_probe+0x41/0x90 pci_device_probe+0xc3/0x230 really_probe+0x1b6/0x410 __driver_probe_device+0x78/0x170 driver_probe_device+0x1f/0x90 __driver_attach+0xd2/0x1c0 ? __pfx___driver_attach+0x10/0x10 bus_for_each_dev+0x8a/0xd0 bus_add_driver+0x141/0x230 driver_register+0x77/0x120 ? __pfx_init_module+0x10/0x10 [amdgpu] do_one_initcall+0x6e/0x350 do_init_module+0x4a/0x220 __do_sys_init_module+0x192/0x1c0 do_syscall_64+0x5b/0x80 ? asm_exc_page_fault+0x22/0x30 ? lockdep_hardirqs_on+0x7d/0x100 entry_SYSCALL_64_after_hwframe+0x72/0xdc RIP: 0033:0x7fd58cfcb1be Code: 48 8b 0d 4d 0c 0c 00
Re: [PATCH] amdgpu: add a filter condition when set brightness
Hi Yuanzhi, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on linus/master v6.2 next-20230227] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Yuanzhi-Wang/amdgpu-add-a-filter-condition-when-set-brightness/20230227-154108 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20230227073953.326-1-wangyuanzhi%40uniontech.com patch subject: [PATCH] amdgpu: add a filter condition when set brightness config: loongarch-randconfig-r036-20230226 (https://download.01.org/0day-ci/archive/20230227/202302271822.zzyrdztn-...@intel.com/config) compiler: loongarch64-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/95d9579e31d0b601aa1422cf767ca5138d3efcee git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Yuanzhi-Wang/amdgpu-add-a-filter-condition-when-set-brightness/20230227-154108 git checkout 95d9579e31d0b601aa1422cf767ca5138d3efcee # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash drivers/gpu/drm/amd/amdgpu/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot | Link: https://lore.kernel.org/oe-kbuild-all/202302271822.zzyrdztn-...@intel.com/ All warnings (new ones prefixed by >>): drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c: In function 'amdgpu_atif_handler': >> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:50: warning: missing >> terminating " character 449 | DRM_WARN("old brightness %d is greater than ACPI brightness | ^ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:450:53: warning: missing terminating " character 450 | %d\n", old_brightness, req.backlight_level); | ^ cc1: error: unterminated argument list invoking macro "DRM_WARN" drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:41: error: 'DRM_WARN' undeclared (first use in this function) 449 | DRM_WARN("old brightness %d is greater than ACPI brightness | ^~~~ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:41: note: each undeclared identifier is reported only once for each function it appears in drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:49: error: expected ';' at end of input 449 | DRM_WARN("old brightness %d is greater than ACPI brightness | ^ | ; .. drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:448:33: note: '-Wmisleading-indentation' is disabled from this point onwards, since column-tracking was disabled due to the size of the code/headers 448 | if (old_brightness > req.backlight_level) | ^~ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:448:33: note: adding '-flarge-source-files' will allow for more column-tracking support, at the expense of compilation time and memory drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:41: error: expected declaration or statement at end of input 449 | DRM_WARN("old brightness %d is greater than ACPI brightness | ^~~~ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:41: error: expected declaration or statement at end of input drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:41: error: expected declaration or statement at end of input drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:449:41: error: expected declaration or statement at end of input drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c: At top level: drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:404:12: warning: 'amdgpu_atif_handler' defined but not used [-Wunused-function] 404 | static int amdgpu_atif_handler(struct amdgpu_device *adev, |^~~ drivers/gpu/drm/am
[PATCH] drm/amdgpu: make umc_v8_10_convert_error_address static
This symbol is not used outside of umc_v8_10.c, so marks it static. drivers/gpu/drm/amd/amdgpu/umc_v8_10.c:212:6: warning: no previous prototype for function 'umc_v8_10_convert_error_address'. Reported-by: Abaci Robot Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4230 Signed-off-by: Jiapeng Chong --- drivers/gpu/drm/amd/amdgpu/umc_v8_10.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v8_10.c b/drivers/gpu/drm/amd/amdgpu/umc_v8_10.c index 66158219f791..048ab4202e9f 100644 --- a/drivers/gpu/drm/amd/amdgpu/umc_v8_10.c +++ b/drivers/gpu/drm/amd/amdgpu/umc_v8_10.c @@ -209,7 +209,7 @@ static int umc_v8_10_swizzle_mode_na_to_pa(struct amdgpu_device *adev, return 0; } -void umc_v8_10_convert_error_address(struct amdgpu_device *adev, +static void umc_v8_10_convert_error_address(struct amdgpu_device *adev, struct ras_err_data *err_data, uint64_t err_addr, uint32_t ch_inst, uint32_t umc_inst, uint32_t node_inst, uint64_t mc_umc_status) -- 2.20.1.7.g153144c
[bug report] drm/amd/display: Various logs added
Hello Leo Chen, This is a semi-automatic email about new static checker warnings. The patch 7ef414375fcc: "drm/amd/display: Various logs added" from Aug 29, 2022, leads to the following Smatch complaint: ./drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c:1862 get_firmware_info_v3_2() warn: variable dereferenced before check 'smu_info_v3_2' (see line 1861) ./drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c 1860 DATA_TABLES(smu_info)); 1861 DC_LOG_BIOS("gpuclk_ss_percentage (unit of 0.001 percent): %d\n", smu_info_v3_2->gpuclk_ss_percentage); ^^^ Log adds a crash. 1862 if (!smu_info_v3_2) ^ Too late. 1863 return BP_RESULT_BADBIOSTABLE; 1864 regards, dan carpenter
[PATCH v3] drm/edid DSC pass-through timing support
VESA DisplayID spec allows the device to force its DSC bits per pixel value. For example, the HTC Vive Pro 2 VR headset uses this value in high-resolution modes (3680x1836@90-120, 4896x2448@90-120), and when the kernel doesn't respect this parameter, the garbage is displayed on HMD instead. I am unaware of any other hardware using this value; however, multiple users have tested this patch on the Vive Pro 2, and it is known to work and not break anything else. Regarding driver support - I have looked at amdgpu and Nvidia's open-gpu-kernel-modules, and both seem to have some indication for this value; however, in Linux, it is unused in both. Amdgpu integration was trivial, so I implemented it in the second patch; other kernel drivers still need the support of this value, and I don't have the necessary hardware to implement and test the handling of this value on them. BR, Yaroslav Yaroslav Bolyukin (2): drm/edid: parse DRM VESA dsc bpp target drm/amd: use fixed dsc bits-per-pixel from edid .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 + .../gpu/drm/amd/display/dc/core/dc_stream.c | 2 + drivers/gpu/drm/amd/display/dc/dc_types.h | 3 ++ drivers/gpu/drm/drm_edid.c| 42 --- include/drm/drm_connector.h | 6 +++ include/drm/drm_displayid.h | 4 ++ 6 files changed, 45 insertions(+), 14 deletions(-) base-commit: a48bba98380cb0b43dcd01d276c7efc282e3c33f -- 2.39.1
[PATCH v3 2/2] drm/amd: use fixed dsc bits-per-pixel from edid
VESA vendor header from DisplayID spec may contain fixed bit per pixel rate, it should be respected by drm driver Signed-off-by: Yaroslav Bolyukin Reviewed-by: Wayne Lin --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 ++ drivers/gpu/drm/amd/display/dc/core/dc_stream.c | 2 ++ drivers/gpu/drm/amd/display/dc/dc_types.h | 3 +++ 3 files changed, 7 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index 6fdc2027c2b4..dba720d5df4c 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -89,6 +89,8 @@ enum dc_edid_status dm_helpers_parse_edid_caps( edid_caps->edid_hdmi = connector->display_info.is_hdmi; + edid_caps->dsc_fixed_bits_per_pixel_x16 = connector->display_info.dp_dsc_bpp; + sad_count = drm_edid_to_sad((struct edid *) edid->raw_edid, ); if (sad_count <= 0) return result; diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c index 72b261ad9587..a82362417379 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c @@ -103,6 +103,8 @@ static bool dc_stream_construct(struct dc_stream_state *stream, /* EDID CAP translation for HDMI 2.0 */ stream->timing.flags.LTE_340MCSC_SCRAMBLE = dc_sink_data->edid_caps.lte_340mcsc_scramble; + stream->timing.dsc_fixed_bits_per_pixel_x16 = + dc_sink_data->edid_caps.dsc_fixed_bits_per_pixel_x16; memset(>timing.dsc_cfg, 0, sizeof(stream->timing.dsc_cfg)); stream->timing.dsc_cfg.num_slices_h = 0; diff --git a/drivers/gpu/drm/amd/display/dc/dc_types.h b/drivers/gpu/drm/amd/display/dc/dc_types.h index 27d0242d6cbd..22fedf4c7547 100644 --- a/drivers/gpu/drm/amd/display/dc/dc_types.h +++ b/drivers/gpu/drm/amd/display/dc/dc_types.h @@ -228,6 +228,9 @@ struct dc_edid_caps { bool edid_hdmi; bool hdr_supported; + /* DisplayPort caps */ + uint32_t dsc_fixed_bits_per_pixel_x16; + struct dc_panel_patch panel_patch; }; -- 2.39.1
[PATCH] amdgpu: add a filter condition when set brightness
When the laptop is plugged into AC or DC power supply, the brightness obtained ACPI may be smaller than current brightness.As a result the screen becomes dark,this is not what people want. Signed-off-by: Yuanzhi Wang --- drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c index d4196fcb85a0..93f1567028c5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c @@ -406,6 +406,7 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev, { struct amdgpu_atif *atif = _acpi_priv.atif; int count; + int old_brightness; DRM_DEBUG_DRIVER("event, device_class = %s, type = %#x\n", event->device_class, event->type); @@ -443,7 +444,13 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev, * hardwired to post BACKLIGHT_UPDATE_SYSFS. * It probably should accept 'reason' parameter. */ - backlight_device_set_brightness(atif->bd, req.backlight_level); + old_brightness = backlight_get_brightness(atif->bd); + if (old_brightness > req.backlight_level) + DRM_WARN("old brightness %d is greater than ACPI brightness + %d\n", old_brightness, req.backlight_level); + else + backlight_device_set_brightness(atif->bd, + req.backlight_level); } } -- 2.20.1
[PATCH v3 1/2] drm/edid: parse DRM VESA dsc bpp target
As per DisplayID v2.0 Errata E9 spec "DSC pass-through timing support" VESA vendor-specific data block may contain target DSC bits per pixel fields Signed-off-by: Yaroslav Bolyukin --- drivers/gpu/drm/drm_edid.c | 38 + include/drm/drm_connector.h | 6 ++ include/drm/drm_displayid.h | 4 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 3d0a4da661bc..aa88ac82cbe0 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -6338,7 +6338,7 @@ static void drm_parse_vesa_mso_data(struct drm_connector *connector, if (oui(vesa->oui[0], vesa->oui[1], vesa->oui[2]) != VESA_IEEE_OUI) return; - if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) { + if (block->num_bytes < 5) { drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] Unexpected VESA vendor block size\n", connector->base.id, connector->name); @@ -6361,24 +6361,37 @@ static void drm_parse_vesa_mso_data(struct drm_connector *connector, break; } - if (!info->mso_stream_count) { - info->mso_pixel_overlap = 0; - return; - } + info->mso_pixel_overlap = 0; + + if (info->mso_stream_count) { + info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, vesa->mso); + + if (info->mso_pixel_overlap > 8) { + drm_dbg_kms(connector->dev, + "[CONNECTOR:%d:%s] Reserved MSO pixel overlap value %u\n", + connector->base.id, connector->name, + info->mso_pixel_overlap); + info->mso_pixel_overlap = 8; + } - info->mso_pixel_overlap = FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, vesa->mso); - if (info->mso_pixel_overlap > 8) { drm_dbg_kms(connector->dev, - "[CONNECTOR:%d:%s] Reserved MSO pixel overlap value %u\n", + "[CONNECTOR:%d:%s] MSO stream count %u, pixel overlap %u\n", connector->base.id, connector->name, - info->mso_pixel_overlap); - info->mso_pixel_overlap = 8; + info->mso_stream_count, info->mso_pixel_overlap); + } + + if (block->num_bytes < 7) { + /* DSC bpp is optional */ + return; } + info->dp_dsc_bpp = FIELD_GET(DISPLAYID_VESA_DSC_BPP_INT, vesa->dsc_bpp_int) * 16 + + FIELD_GET(DISPLAYID_VESA_DSC_BPP_FRACT, vesa->dsc_bpp_fract); + drm_dbg_kms(connector->dev, - "[CONNECTOR:%d:%s] MSO stream count %u, pixel overlap %u\n", + "[CONNECTOR:%d:%s] DSC bits per pixel %u\n", connector->base.id, connector->name, - info->mso_stream_count, info->mso_pixel_overlap); + info->dp_dsc_bpp); } static void drm_update_mso(struct drm_connector *connector, @@ -6425,6 +6438,7 @@ static void drm_reset_display_info(struct drm_connector *connector) info->mso_stream_count = 0; info->mso_pixel_overlap = 0; info->max_dsc_bpp = 0; + info->dp_dsc_bpp = 0; kfree(info->vics); info->vics = NULL; diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 7b5048516185..1d01e0146a7f 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -719,6 +719,12 @@ struct drm_display_info { */ u32 max_dsc_bpp; + /** +* @dp_dsc_bpp: DP Display-Stream-Compression (DSC) timing's target +* DST bits per pixel in 6.4 fixed point format. 0 means undefined +*/ + u16 dp_dsc_bpp; + /** * @vics: Array of vics_len VICs. Internal to EDID parsing. */ diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index 49649eb8447e..0fc3afbd1675 100644 --- a/include/drm/drm_displayid.h +++ b/include/drm/drm_displayid.h @@ -131,12 +131,16 @@ struct displayid_detailed_timing_block { #define DISPLAYID_VESA_MSO_OVERLAP GENMASK(3, 0) #define DISPLAYID_VESA_MSO_MODEGENMASK(6, 5) +#define DISPLAYID_VESA_DSC_BPP_INT GENMASK(5, 0) +#define DISPLAYID_VESA_DSC_BPP_FRACT GENMASK(3, 0) struct displayid_vesa_vendor_specific_block { struct displayid_block base; u8 oui[3]; u8 data_structure_type; u8 mso; + u8 dsc_bpp_int; + u8 dsc_bpp_fract; } __packed; /* DisplayID iteration */ -- 2.39.1
Re: [PATCH 2/2] drm/amdgpu: Synchronize after mapping into a compute VM
Am 25.02.23 um 00:36 schrieb Felix Kuehling: Compute VMs use user mode queues for command submission. They cannot use a CS ioctl to synchronize with pending PTE updates and flush TLBs. Do this synchronization in amdgpu_gem_va_ioctl for compute VMs. Signed-off-by: Felix Kuehling --- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index 6936cd63df42..7de5057c40ec 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -601,7 +601,7 @@ int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void *data, static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev, struct amdgpu_vm *vm, struct amdgpu_bo_va *bo_va, - uint32_t operation) + uint32_t operation, uint32_t flags) { int r; @@ -620,6 +620,18 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev, } r = amdgpu_vm_update_pdes(adev, vm, false); + if (r) + goto error; + + if (vm->is_compute_context) { + if (bo_va->last_pt_update) + r = dma_fence_wait(bo_va->last_pt_update, true); + if (!r && vm->last_update) + r = dma_fence_wait(vm->last_update, true); + if (!r) + r = amdgpu_amdkfd_flush_tlb(adev, vm, + TLB_FLUSH_LEGACY); + } Mhm, that's not really something we can do here. The GEM VA IOCTL is supposed to be async and never block. Can we do that on the KFD side in some IOCTL instead? Regards, Christian. error: if (r && r != -ERESTARTSYS) @@ -789,7 +801,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data, } if (!r && !(args->flags & AMDGPU_VM_DELAY_UPDATE) && !amdgpu_vm_debug) amdgpu_gem_va_update_vm(adev, >vm, bo_va, - args->operation); + args->operation, args->flags); error_backoff: ttm_eu_backoff_reservation(, );