Re: [PATCH v4] drm/amdgpu: Restore msix after FLR
Am 02.07.21 um 05:23 schrieb Peng Ju Zhou:
From: "Emily.Deng"
After FLR, the msix will be cleared, so need to re-enable it.
Signed-off-by: Emily.Deng
Signed-off-by: Peng Ju Zhou
---
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 90f50561b43a..034420c38352 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -277,6 +277,19 @@ static bool amdgpu_msi_ok(struct amdgpu_device *adev)
return true;
}
+void amdgpu_restore_msix(struct amdgpu_device *adev)
I think this function should be static and maybe add a one line comment
like "Clear and re-set the MSIX flags if they where set before to
trigger re-enabling it".
With that done feel free to add an Acked-by: Christian König
, but I would also wait what Alex has to say
about that.
Regards,
Christian
+{
+ u16 ctrl;
+
+ pci_read_config_word(adev->pdev, adev->pdev->msix_cap + PCI_MSIX_FLAGS,
&ctrl);
+ if (!(ctrl & PCI_MSIX_FLAGS_ENABLE))
+ return;
+
+ ctrl &= ~PCI_MSIX_FLAGS_ENABLE;
+ pci_write_config_word(adev->pdev, adev->pdev->msix_cap +
PCI_MSIX_FLAGS, ctrl);
+ ctrl |= PCI_MSIX_FLAGS_ENABLE;
+ pci_write_config_word(adev->pdev, adev->pdev->msix_cap +
PCI_MSIX_FLAGS, ctrl);
+}
/**
* amdgpu_irq_init - initialize interrupt handling
*
@@ -558,6 +571,7 @@ void amdgpu_irq_gpu_reset_resume_helper(struct
amdgpu_device *adev)
{
int i, j, k;
+ amdgpu_restore_msix(adev);
for (i = 0; i < AMDGPU_IRQ_CLIENTID_MAX; ++i) {
if (!adev->irq.client[i].sources)
continue;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/1] drm/amdgpu: Read clock counter via MMIO to reduce delay (v5)
On 2021-07-01 5:57 a.m., YuBiao Wang wrote:
> [Why]
> GPU timing counters are read via KIQ under sriov, which will introduce
> a delay.
>
> [How]
> It could be directly read by MMIO.
>
> v2: Add additional check to prevent carryover issue.
> v3: Only check for carryover for once to prevent performance issue.
> v4: Add comments of the rough frequency where carryover happens.
> v5: Remove mutex and gfxoff ctrl unused with current timing registers.
>
> Signed-off-by: YuBiao Wang
> Acked-by: Horace Chen
> ---
> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 19 ---
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index ff7e9f49040e..5f4eae9c9526 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -7609,10 +7609,8 @@ static int gfx_v10_0_soft_reset(void *handle)
>
> static uint64_t gfx_v10_0_get_gpu_clock_counter(struct amdgpu_device *adev)
> {
> - uint64_t clock;
> + uint64_t clock, clock_lo, clock_hi, hi_check;
>
> - amdgpu_gfx_off_ctrl(adev, false);
> - mutex_lock(&adev->gfx.gpu_clock_mutex);
> switch (adev->asic_type) {
> case CHIP_VANGOGH:
> case CHIP_YELLOW_CARP:
> @@ -7620,12 +7618,19 @@ static uint64_t
> gfx_v10_0_get_gpu_clock_counter(struct amdgpu_device *adev)
> ((uint64_t)RREG32_SOC15(SMUIO, 0,
> mmGOLDEN_TSC_COUNT_UPPER_Vangogh) << 32ULL);
> break;
> default:
> - clock = (uint64_t)RREG32_SOC15(SMUIO, 0,
> mmGOLDEN_TSC_COUNT_LOWER) |
> - ((uint64_t)RREG32_SOC15(SMUIO, 0,
> mmGOLDEN_TSC_COUNT_UPPER) << 32ULL);
> + clock_hi = RREG32_SOC15_NO_KIQ(SMUIO, 0,
> mmGOLDEN_TSC_COUNT_UPPER);
> + clock_lo = RREG32_SOC15_NO_KIQ(SMUIO, 0,
> mmGOLDEN_TSC_COUNT_LOWER);
> + hi_check = RREG32_SOC15_NO_KIQ(SMUIO, 0,
> mmGOLDEN_TSC_COUNT_UPPER);
> + /* The GFX clock frequency is 100MHz, which sets 32-bit carry
> over
> + * roughly every 42 seconds.
> + */
> + if (hi_check != clock_hi) {
Yeah, the comment is so much better now. Good job.
Reviewed-by: Luben Tuikov
Regards,
Luben
> + clock_lo = RREG32_SOC15_NO_KIQ(SMUIO, 0,
> mmGOLDEN_TSC_COUNT_LOWER);
> + clock_hi = hi_check;
> + }
> + clock = (uint64_t)clock_lo | ((uint64_t)clock_hi << 32ULL);
> break;
> }
> - mutex_unlock(&adev->gfx.gpu_clock_mutex);
> - amdgpu_gfx_off_ctrl(adev, true);
> return clock;
> }
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] drm/amdkfd: Only apply TLB flush optimization on ALdebaran
Am 2021-06-30 um 7:26 p.m. schrieb Eric Huang: > It is based on reverting two patches back. > drm/amdkfd: Make TLB flush conditional on mapping > drm/amdgpu: Add table_freed parameter to amdgpu_vm_bo_update > > Signed-off-by: Eric Huang I hope we'll be able to change this condition if firmware fixes become available. I think eventually this should only be needed for Vega20 XGMI. For now, this patch is Reviewed-by: Felix Kuehling > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > index a6c01d624246..13fcef6836d2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > @@ -1845,6 +1845,12 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu( > true); > ret = unreserve_bo_and_vms(&ctx, false, false); > > + /* Only apply no TLB flush on Aldebaran to > + * workaround regressions on other Asics. > + */ > + if (table_freed && (adev->asic_type != CHIP_ALDEBARAN)) > + *table_freed = true; > + > goto out; > > out_unreserve: ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Mesa-dev] Requests For Proposals for hosting XDC 2022 are now open
This is a reminder that the call for proposals for hosting XDC 2022 period finishes in two months. Be sure to prepare your submission before you leave on holiday! Sam On Thu, 2021-05-20 at 12:15 +0200, Samuel Iglesias Gonsálvez wrote: > Hello everyone! > > The X.org board is soliciting proposals to host XDC in 2022. Since > XDC 2021 is being held in Europe this year (although virtually), > we've > decided to host in North America. However, the board is open to other > locations, especially if there's an interesting co-location with > another conference. > > Of course though, due to the ongoing COVID-19 pandemic it's not yet > clear whether or not it will be possible to host XDC 2022 in person, > although is seems very likely. Because of this, we would like to > make it clear that sponsors should prepare for both the possibility > of an in person conference, and the possibility of a virtual > conference. We will work with organizers on coming up with a > deadline for deciding whether or not we'll be going virtual, likely > sometime around July 2022. > > If you're considering hosting XDC, we've assembled a wiki page with > what's generally expected and needed: > > https://www.x.org/wiki/Events/RFP/ > > When submitting your proposal, please make sure to include at least > the > key information about the potential location in question, possible > dates along with estimated costs. Proposals can be submitted to board > at foundation.x.org until the deadline of *September 1st, 2021*. > > Additionally, an quirk early heads-up to the board if you're > considering hosting would be appreciated, in case we need to adjust > the > schedule a bit. Also, earlier is better since there generally will be > a > bit of Q&A with organizers. > > And if you just have some questions about what organizing XDC > entails, > please feel free to chat with previous organizers, or someone from > the > board. > > Thanks, > > Sam > > ___ > mesa-dev mailing list > mesa-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: This is a digitally signed message part ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: AMDGPU error: "[drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out!"
I cp'd raven* and picasso* firmware files from upstream version 21.20 and replaced the ones from my distro and I just got another one of these errors. [Jul 1 17:08] amdgpu :08:00.0: amdgpu: [gfxhub0] retry page fault (src_id:0 ring:0 vmid:1 pasid:32778, for process mpv pid 7400 thread mpv:cs0 pid 7432) [ +0.14] amdgpu :08:00.0: amdgpu: in page starting at address 0x80010008d000 from client 27 [ +0.10] amdgpu :08:00.0: amdgpu: VM_L2_PROTECTION_FAULT_STATUS:0x00140C51 [ +0.02] amdgpu :08:00.0: amdgpu: Faulty UTCL2 client ID: CPG (0x6) [ +0.05] amdgpu :08:00.0: amdgpu: MORE_FAULTS: 0x1 [ +0.01] amdgpu :08:00.0: amdgpu: WALKER_ERROR: 0x0 [ +0.02] amdgpu :08:00.0: amdgpu: PERMISSION_FAULTS: 0x5 [ +0.01] amdgpu :08:00.0: amdgpu: MAPPING_ERROR: 0x0 [ +0.01] amdgpu :08:00.0: amdgpu: RW: 0x1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Correct the irq numbers for virtual ctrc
Reviewed-by: Alex Deucher
On Thu, Jul 1, 2021 at 5:18 AM Deng, Emily wrote:
>
> [AMD Official Use Only]
>
> Ping..
>
> >-Original Message-
> >From: Emily Deng
> >Sent: Thursday, July 1, 2021 12:38 PM
> >To: amd-gfx@lists.freedesktop.org
> >Cc: Deng, Emily ; Zhao, Victor
> >Subject: [PATCH] drm/amdgpu: Correct the irq numbers for virtual ctrc
> >
> >Signed-off-by: Emily Deng
> >Signed-off-by: Victor
> >---
> > drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> >b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> >index 33324427b555..7e0d8c092c7e 100644
> >--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> >+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> >@@ -766,7 +766,7 @@ static const struct amdgpu_irq_src_funcs
> >dce_virtual_crtc_irq_funcs = {
> >
> > static void dce_virtual_set_irq_funcs(struct amdgpu_device *adev) {
> >- adev->crtc_irq.num_types = AMDGPU_CRTC_IRQ_VBLANK6 + 1;
> >+ adev->crtc_irq.num_types = adev->mode_info.num_crtc;
> > adev->crtc_irq.funcs = &dce_virtual_crtc_irq_funcs; }
> >
> >--
> >2.25.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amd/pm: drop smu_v13_0_1.c|h files for yellow carp
[AMD Official Use Only] > >-Original Message- >From: Lazar, Lijo >Sent: Thursday, July 1, 2021 4:51 PM >To: Hou, Xiaomeng (Matthew) ; >amd-gfx@lists.freedesktop.org >Cc: Wang, Kevin(Yang) ; Liu, Aaron >Subject: Re: [PATCH] drm/amd/pm: drop smu_v13_0_1.c|h files for yellow carp > >One minor comment below. > >Reviewed-by: Lijo Lazar > > >On 7/1/2021 2:01 PM, Xiaomeng Hou wrote: >> Since there's nothing special in smu implementation for yellow carp, >> it's better to reuse the common smu_v13_0 interfaces and drop the >> specific smu_v13_0_1.c|h files. >> >> Signed-off-by: Xiaomeng Hou >> --- >> drivers/gpu/drm/amd/pm/inc/smu_v13_0.h| 1 + >> drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h | 57 >> drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile | 2 +- >> .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c| 26 ++ >> .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_1.c | 311 -- >> .../drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c | 39 ++- >> 6 files changed, 59 insertions(+), 377 deletions(-) >> delete mode 100644 drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h >> delete mode 100644 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_1.c >> >> diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h >> b/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h >> index 6119a36b2cba..3fea2430dec0 100644 >> --- a/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h >> +++ b/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h >> @@ -26,6 +26,7 @@ >> #include "amdgpu_smu.h" >> >> #define SMU13_DRIVER_IF_VERSION_INV 0x >> +#define SMU13_DRIVER_IF_VERSION_YELLOW_CARP 0x03 >> #define SMU13_DRIVER_IF_VERSION_ALDE 0x07 >> >> /* MP Apertures */ >> diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h >> b/drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h >> deleted file mode 100644 >> index b6c976a4d578.. >> --- a/drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h >> +++ /dev/null >> @@ -1,57 +0,0 @@ >> -/* >> - * Copyright 2020 Advanced Micro Devices, Inc. >> - * >> - * Permission is hereby granted, free of charge, to any person >> obtaining a >> - * copy of this software and associated documentation files (the >> "Software"), >> - * to deal in the Software without restriction, including without >> limitation >> - * the rights to use, copy, modify, merge, publish, distribute, >> sublicense, >> - * and/or sell copies of the Software, and to permit persons to whom >> the >> - * Software is furnished to do so, subject to the following conditions: >> - * >> - * The above copyright notice and this permission notice shall be >> included in >> - * all copies or substantial portions of the Software. >> - * >> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >> EXPRESS OR >> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> MERCHANTABILITY, >> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT >> SHALL >> - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, >> DAMAGES OR >> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR >> OTHERWISE, >> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE >> OR >> - * OTHER DEALINGS IN THE SOFTWARE. >> - * >> - */ >> -#ifndef __SMU_V13_0_1_H__ >> -#define __SMU_V13_0_1_H__ >> - >> -#include "amdgpu_smu.h" >> - >> -#define SMU13_0_1_DRIVER_IF_VERSION_INV 0x -#define >> SMU13_0_1_DRIVER_IF_VERSION_YELLOW_CARP 0x3 >> - >> -/* MP Apertures */ >> -#define MP0_Public 0x0380 >> -#define MP0_SRAM0x0390 >> -#define MP1_Public 0x03b0 >> -#define MP1_SRAM0x03c4 >> - >> -/* address block */ >> -#define smnMP1_FIRMWARE_FLAGS 0x3010024 >> - >> - >> -#if defined(SWSMU_CODE_LAYER_L2) || defined(SWSMU_CODE_LAYER_L3) >> - >> -int smu_v13_0_1_check_fw_status(struct smu_context *smu); >> - >> -int smu_v13_0_1_check_fw_version(struct smu_context *smu); >> - >> -int smu_v13_0_1_fini_smc_tables(struct smu_context *smu); >> - >> -int smu_v13_0_1_get_vbios_bootup_values(struct smu_context *smu); >> - >> -int smu_v13_0_1_set_default_dpm_tables(struct smu_context *smu); >> - >> -int smu_v13_0_1_set_driver_table_location(struct smu_context *smu); >> - >> -int smu_v13_0_1_gfx_off_control(struct smu_context *smu, bool >> enable); -#endif -#endif diff --git >> a/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile >> b/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile >> index 9b3a8503f5cd..d4c4c495762c 100644 >> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile >> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile >> @@ -23,7 +23,7 @@ >> # Makefile for the 'smu manager' sub-component of powerplay. >> # It provides the smu management services for the driver. >> >> -SMU13_MGR = smu_v13_0.o aldebaran_ppt.o smu_v13_0_1.o >> yellow_carp_ppt.o >> +SMU13_MGR = smu_v13_0.o aldebaran_ppt.o yellow_carp_ppt.o >> >> AMD_SWSMU_SMU13MGR = $(addprefix >> $(AMD_SWSMU_PATH)/smu13/,$(SMU13_MGR)) >> >> diff --git a/
Re: [PATCH 1/1] drm/amdgpu: Read clock counter via MMIO to reduce delay (v5)
A few comments to take care of before submitting the changes.
Reviewed-by: Lijo Lazar
On 7/1/2021 3:27 PM, YuBiao Wang wrote:
[Why]
GPU timing counters are read via KIQ under sriov, which will introduce
a delay.
[How]
It could be directly read by MMIO.
v2: Add additional check to prevent carryover issue.
v3: Only check for carryover for once to prevent performance issue.
v4: Add comments of the rough frequency where carryover happens.
v5: Remove mutex and gfxoff ctrl unused with current timing registers.
Signed-off-by: YuBiao Wang
Acked-by: Horace Chen
---
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index ff7e9f49040e..5f4eae9c9526 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -7609,10 +7609,8 @@ static int gfx_v10_0_soft_reset(void *handle)
static uint64_t gfx_v10_0_get_gpu_clock_counter(struct amdgpu_device *adev)
{
- uint64_t clock;
+ uint64_t clock, clock_lo, clock_hi, hi_check;
- amdgpu_gfx_off_ctrl(adev, false);
- mutex_lock(&adev->gfx.gpu_clock_mutex);
switch (adev->asic_type) {
case CHIP_VANGOGH:
case CHIP_YELLOW_CARP:
@@ -7620,12 +7618,19 @@ static uint64_t gfx_v10_0_get_gpu_clock_counter(struct
amdgpu_device *adev)
((uint64_t)RREG32_SOC15(SMUIO, 0,
mmGOLDEN_TSC_COUNT_UPPER_Vangogh) << 32ULL);
break;
default:
- clock = (uint64_t)RREG32_SOC15(SMUIO, 0,
mmGOLDEN_TSC_COUNT_LOWER) |
- ((uint64_t)RREG32_SOC15(SMUIO, 0, mmGOLDEN_TSC_COUNT_UPPER)
<< 32ULL);
+ clock_hi = RREG32_SOC15_NO_KIQ(SMUIO, 0,
mmGOLDEN_TSC_COUNT_UPPER);
+ clock_lo = RREG32_SOC15_NO_KIQ(SMUIO, 0,
mmGOLDEN_TSC_COUNT_LOWER);
+ hi_check = RREG32_SOC15_NO_KIQ(SMUIO, 0,
mmGOLDEN_TSC_COUNT_UPPER);
+ /* The GFX clock frequency is 100MHz, which sets 32-bit carry
over
+* roughly every 42 seconds.
+*/
SMUIO TSC is not ticking based on GFX clock frequency, it's running at a
fixed SMUIO TSC Clock frequency which is 100MHz (second part is right).
Replace GFX Clock with SMUIO TSC Clock.
+ if (hi_check != clock_hi) {
+ clock_lo = RREG32_SOC15_NO_KIQ(SMUIO, 0,
mmGOLDEN_TSC_COUNT_LOWER);
+ clock_hi = hi_check;
+ }
+ clock = (uint64_t)clock_lo | ((uint64_t)clock_hi << 32ULL);
These types are already uint64_t and looks cleaner without the cast.
Thanks,
Lijo
break;
}
- mutex_unlock(&adev->gfx.gpu_clock_mutex);
- amdgpu_gfx_off_ctrl(adev, true);
return clock;
}
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] drm/amdgpu: Avoid printing of stack contents on firmware load error
Applied. Thanks!
Alex
On Thu, Jul 1, 2021 at 4:33 AM Jiri Kosina wrote:
>
> On Thu, 24 Jun 2021, Jiri Kosina wrote:
>
> > From: Jiri Kosina
> >
> > In case when psp_init_asd_microcode() fails to load ASD microcode file,
> > psp_v12_0_init_microcode() tries to print the firmware filename that
> > failed to load before bailing out.
> >
> > This is wrong because:
> >
> > - the firmware filename it would want it print is an incorrect one as
> > psp_init_asd_microcode() and psp_v12_0_init_microcode() are loading
> > different filenames
> > - it tries to print fw_name, but that's not yet been initialized by that
> > time, so it prints random stack contents, e.g.
> >
> > amdgpu :04:00.0: Direct firmware load for amdgpu/renoir_asd.bin
> > failed with error -2
> > amdgpu :04:00.0: amdgpu: fail to initialize asd microcode
> > amdgpu :04:00.0: amdgpu: psp v12.0: Failed to load firmware
> > "\xfeTO\x8e\xff\xff"
> >
> > Fix that by bailing out immediately, instead of priting the bogus error
> > message.
>
> Friendly ping on this one too; priting a few bytes of stack is not a
> *huge* info leak, but I believe it should be fixed nevertheless.
>
> Thanks.
>
> >
> > Reported-by: Vojtech Pavlik
> > Signed-off-by: Jiri Kosina
>
>
> > ---
> >
> > v1 -> v2: remove now-unused label
> >
> > drivers/gpu/drm/amd/amdgpu/psp_v12_0.c | 7 +++
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> > b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> > index c4828bd3264b..b0ee77ee80b9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> > @@ -67,7 +67,7 @@ static int psp_v12_0_init_microcode(struct psp_context
> > *psp)
> >
> > err = psp_init_asd_microcode(psp, chip_name);
> > if (err)
> > - goto out;
> > + return err;
> >
> > snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
> > err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
> > @@ -80,7 +80,7 @@ static int psp_v12_0_init_microcode(struct psp_context
> > *psp)
> > } else {
> > err = amdgpu_ucode_validate(adev->psp.ta_fw);
> > if (err)
> > - goto out2;
> > + goto out;
> >
> > ta_hdr = (const struct ta_firmware_header_v1_0 *)
> >adev->psp.ta_fw->data;
> > @@ -105,10 +105,9 @@ static int psp_v12_0_init_microcode(struct psp_context
> > *psp)
> >
> > return 0;
> >
> > -out2:
> > +out:
> > release_firmware(adev->psp.ta_fw);
> > adev->psp.ta_fw = NULL;
> > -out:
> > if (err) {
> > dev_err(adev->dev,
> > "psp v12.0: Failed to load firmware \"%s\"\n",
> > --
> > 2.12.3
> >
>
> --
> Jiri Kosina
> SUSE Labs
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Fix resource leak on probe error path
Applied. Thanks!
Alex
On Thu, Jul 1, 2021 at 4:32 AM Jiri Kosina wrote:
>
> On Thu, 24 Jun 2021, Jiri Kosina wrote:
>
> > From: Jiri Kosina
> >
> > This reverts commit 4192f7b5768912ceda82be2f83c87ea7181f9980.
> >
> > It is not true (as stated in the reverted commit changelog) that we never
> > unmap the BAR on failure; it actually does happen properly on
> > amdgpu_driver_load_kms() -> amdgpu_driver_unload_kms() ->
> > amdgpu_device_fini() error path.
> >
> > What's worse, this commit actually completely breaks resource freeing on
> > probe failure (like e.g. failure to load microcode), as
> > amdgpu_driver_unload_kms() notices adev->rmmio being NULL and bails too
> > early, leaving all the resources that'd normally be freed in
> > amdgpu_acpi_fini() and amdgpu_device_fini() still hanging around, leading
> > to all sorts of oopses when someone tries to, for example, access the
> > sysfs and procfs resources which are still around while the driver is
> > gone.
> >
> > Fixes: 4192f7b57689 ("drm/amdgpu: unmap register bar on device init
> > failure")
> > Reported-by: Vojtech Pavlik
> > Signed-off-by: Jiri Kosina
>
> Friendly ping on this one (as it's easily triggerable in the wild by just
> missing proper firwmare).
>
> Thanks.
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++--
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 57ec108b5972..0f1c0e17a587 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -3414,13 +3414,13 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> > r = amdgpu_device_get_job_timeout_settings(adev);
> > if (r) {
> > dev_err(adev->dev, "invalid lockup_timeout parameter
> > syntax\n");
> > - goto failed_unmap;
> > + return r;
> > }
> >
> > /* early init functions */
> > r = amdgpu_device_ip_early_init(adev);
> > if (r)
> > - goto failed_unmap;
> > + return r;
> >
> > /* doorbell bar mapping and doorbell index init*/
> > amdgpu_device_doorbell_init(adev);
> > @@ -3646,10 +3646,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> > failed:
> > amdgpu_vf_error_trans_all(adev);
> >
> > -failed_unmap:
> > - iounmap(adev->rmmio);
> > - adev->rmmio = NULL;
> > -
> > return r;
> > }
> >
> > --
> > 2.12.3
> >
> >
>
> --
> Jiri Kosina
> SUSE Labs
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/1] drm/amdgpu: Read clock counter via MMIO to reduce delay (v5)
[Why]
GPU timing counters are read via KIQ under sriov, which will introduce
a delay.
[How]
It could be directly read by MMIO.
v2: Add additional check to prevent carryover issue.
v3: Only check for carryover for once to prevent performance issue.
v4: Add comments of the rough frequency where carryover happens.
v5: Remove mutex and gfxoff ctrl unused with current timing registers.
Signed-off-by: YuBiao Wang
Acked-by: Horace Chen
---
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index ff7e9f49040e..5f4eae9c9526 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -7609,10 +7609,8 @@ static int gfx_v10_0_soft_reset(void *handle)
static uint64_t gfx_v10_0_get_gpu_clock_counter(struct amdgpu_device *adev)
{
- uint64_t clock;
+ uint64_t clock, clock_lo, clock_hi, hi_check;
- amdgpu_gfx_off_ctrl(adev, false);
- mutex_lock(&adev->gfx.gpu_clock_mutex);
switch (adev->asic_type) {
case CHIP_VANGOGH:
case CHIP_YELLOW_CARP:
@@ -7620,12 +7618,19 @@ static uint64_t gfx_v10_0_get_gpu_clock_counter(struct
amdgpu_device *adev)
((uint64_t)RREG32_SOC15(SMUIO, 0,
mmGOLDEN_TSC_COUNT_UPPER_Vangogh) << 32ULL);
break;
default:
- clock = (uint64_t)RREG32_SOC15(SMUIO, 0,
mmGOLDEN_TSC_COUNT_LOWER) |
- ((uint64_t)RREG32_SOC15(SMUIO, 0,
mmGOLDEN_TSC_COUNT_UPPER) << 32ULL);
+ clock_hi = RREG32_SOC15_NO_KIQ(SMUIO, 0,
mmGOLDEN_TSC_COUNT_UPPER);
+ clock_lo = RREG32_SOC15_NO_KIQ(SMUIO, 0,
mmGOLDEN_TSC_COUNT_LOWER);
+ hi_check = RREG32_SOC15_NO_KIQ(SMUIO, 0,
mmGOLDEN_TSC_COUNT_UPPER);
+ /* The GFX clock frequency is 100MHz, which sets 32-bit carry
over
+* roughly every 42 seconds.
+*/
+ if (hi_check != clock_hi) {
+ clock_lo = RREG32_SOC15_NO_KIQ(SMUIO, 0,
mmGOLDEN_TSC_COUNT_LOWER);
+ clock_hi = hi_check;
+ }
+ clock = (uint64_t)clock_lo | ((uint64_t)clock_hi << 32ULL);
break;
}
- mutex_unlock(&adev->gfx.gpu_clock_mutex);
- amdgpu_gfx_off_ctrl(adev, true);
return clock;
}
--
2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
On Thu, 1 Jul 2021 14:50:13 +0200 Werner Sembach wrote: > Am 01.07.21 um 10:07 schrieb Pekka Paalanen: > > > On Wed, 30 Jun 2021 11:20:18 +0200 > > Werner Sembach wrote: > > > >> Am 30.06.21 um 10:41 schrieb Pekka Paalanen: > >> > >>> On Tue, 29 Jun 2021 13:39:18 +0200 > >>> Werner Sembach wrote: > >>> > Am 29.06.21 um 13:17 schrieb Pekka Paalanen: > > On Tue, 29 Jun 2021 08:12:54 + > > Simon Ser wrote: > > > >> On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen > >> wrote: > >> > >>> yes, I think this makes sense, even if it is a property that one can't > >>> tell for sure what it does before hand. > >>> > >>> Using a pair of properties, preference and active, to ask for > >>> something > >>> and then check what actually worked is good for reducing the > >>> combinatorial explosion caused by needing to "atomic TEST_ONLY commit" > >>> test different KMS configurations. Userspace has a better chance of > >>> finding a configuration that is possible. > >>> > >>> OTOH, this has the problem than in UI one cannot tell the user in > >>> advance which options are truly possible. Given that KMS properties > >>> are > >>> rarely completely independent, and in this case known to depend on > >>> several other KMS properties, I think it is good enough to know after > >>> the fact. > >>> > >>> If a driver does not use what userspace prefers, there is no way to > >>> understand why, or what else to change to make it happen. That problem > >>> exists anyway, because TEST_ONLY commits do not give useful feedback > >>> but only a yes/no. > >> By submitting incremental atomic reqs with TEST_ONLY (i.e. only > >> changing one > >> property at a time), user-space can discover which property makes the > >> atomic > >> commit fail. > > That works if the properties are independent of each other. Color > > range, color format, bpc and more may all be interconnected, > > allowing only certain combinations to work. > > > > If all these properties have "auto" setting too, then it would be > > possible to probe each property individually, but that still does not > > tell which combinations are valid. > > > > If you probe towards a certain configuration by setting the properties > > one by one, then depending on the order you pick the properties, you > > may come to a different conclusion on which property breaks the > > configuration. > My mind crossed another point that must be considered: When plugin in > a Monitor a list of possible Resolutions+Framerate combinations is > created for xrandr and other userspace (I guess by atomic checks? but > I don't know). > >>> Hi, > >>> > >>> I would not think so, but I hope to be corrected if I'm wrong. > >>> > >>> My belief is that the driver collects a list of modes from EDID, some > >>> standard modes, and maybe some other hardcoded modes, and then > >>> validates each entry against all the known limitations like vertical > >>> and horizontal frequency limits, discarding modes that do not fit. > >>> > >>> Not all limitations are known during that phase, which is why KMS > >>> property "link-status" exists. When userspace actually programs a mode > >>> (not a TEST_ONLY commit), the link training may fail. The kernel prunes > >>> the mode from the list and sets the link status property to signal > >>> failure, and sends a hotplug uevent. Userspace needs to re-check the > >>> mode list and try again. > >>> > >>> That is a generic escape hatch for when TEST_ONLY commit succeeds, but > >>> in reality the hardware cannot do it, you just cannot know until you > >>> actually try for real. It causes end user visible flicker if it happens > >>> on an already running connector, but since it usually happens when > >>> turning a connector on to begin with, there is no flicker to be seen, > >>> just a small delay in finding a mode that works. > >>> > During this drm > properties are already considered, which is no problem atm because as > far as i can tell there is currently no drm property that would make > a certain Resolutions+Framerate combination unreachable that would be > possible with everything on default. > >>> I would not expect KMS properties to be considered at all. It would > >>> reject modes that are actually possible if the some KMS properties were > >>> changed. So at least going forward, current KMS property values cannot > >>> factor in. > >> At least the debugfs variable "force_yuv420_output" did change the > >> available modes here: > >> https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L5165 > >> > >> before my patch > >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=68eb3ae3c63708f823aeeb63bb15197c727bd9bf >
RE: [PATCH] drm/amdgpu: Correct the irq numbers for virtual ctrc
[AMD Official Use Only]
Ping..
>-Original Message-
>From: Emily Deng
>Sent: Thursday, July 1, 2021 12:38 PM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deng, Emily ; Zhao, Victor
>Subject: [PATCH] drm/amdgpu: Correct the irq numbers for virtual ctrc
>
>Signed-off-by: Emily Deng
>Signed-off-by: Victor
>---
> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>index 33324427b555..7e0d8c092c7e 100644
>--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
>@@ -766,7 +766,7 @@ static const struct amdgpu_irq_src_funcs
>dce_virtual_crtc_irq_funcs = {
>
> static void dce_virtual_set_irq_funcs(struct amdgpu_device *adev) {
>- adev->crtc_irq.num_types = AMDGPU_CRTC_IRQ_VBLANK6 + 1;
>+ adev->crtc_irq.num_types = adev->mode_info.num_crtc;
> adev->crtc_irq.funcs = &dce_virtual_crtc_irq_funcs; }
>
>--
>2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/pm: drop smu_v13_0_1.c|h files for yellow carp
One minor comment below. Reviewed-by: Lijo Lazar On 7/1/2021 2:01 PM, Xiaomeng Hou wrote: Since there's nothing special in smu implementation for yellow carp, it's better to reuse the common smu_v13_0 interfaces and drop the specific smu_v13_0_1.c|h files. Signed-off-by: Xiaomeng Hou --- drivers/gpu/drm/amd/pm/inc/smu_v13_0.h| 1 + drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h | 57 drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile | 2 +- .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c| 26 ++ .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_1.c | 311 -- .../drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c | 39 ++- 6 files changed, 59 insertions(+), 377 deletions(-) delete mode 100644 drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h delete mode 100644 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_1.c diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h b/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h index 6119a36b2cba..3fea2430dec0 100644 --- a/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h +++ b/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h @@ -26,6 +26,7 @@ #include "amdgpu_smu.h" #define SMU13_DRIVER_IF_VERSION_INV 0x +#define SMU13_DRIVER_IF_VERSION_YELLOW_CARP 0x03 #define SMU13_DRIVER_IF_VERSION_ALDE 0x07 /* MP Apertures */ diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h b/drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h deleted file mode 100644 index b6c976a4d578.. --- a/drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright 2020 Advanced Micro Devices, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR - * OTHER DEALINGS IN THE SOFTWARE. - * - */ -#ifndef __SMU_V13_0_1_H__ -#define __SMU_V13_0_1_H__ - -#include "amdgpu_smu.h" - -#define SMU13_0_1_DRIVER_IF_VERSION_INV 0x -#define SMU13_0_1_DRIVER_IF_VERSION_YELLOW_CARP 0x3 - -/* MP Apertures */ -#define MP0_Public 0x0380 -#define MP0_SRAM 0x0390 -#define MP1_Public 0x03b0 -#define MP1_SRAM 0x03c4 - -/* address block */ -#define smnMP1_FIRMWARE_FLAGS 0x3010024 - - -#if defined(SWSMU_CODE_LAYER_L2) || defined(SWSMU_CODE_LAYER_L3) - -int smu_v13_0_1_check_fw_status(struct smu_context *smu); - -int smu_v13_0_1_check_fw_version(struct smu_context *smu); - -int smu_v13_0_1_fini_smc_tables(struct smu_context *smu); - -int smu_v13_0_1_get_vbios_bootup_values(struct smu_context *smu); - -int smu_v13_0_1_set_default_dpm_tables(struct smu_context *smu); - -int smu_v13_0_1_set_driver_table_location(struct smu_context *smu); - -int smu_v13_0_1_gfx_off_control(struct smu_context *smu, bool enable); -#endif -#endif diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile b/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile index 9b3a8503f5cd..d4c4c495762c 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile @@ -23,7 +23,7 @@ # Makefile for the 'smu manager' sub-component of powerplay. # It provides the smu management services for the driver. -SMU13_MGR = smu_v13_0.o aldebaran_ppt.o smu_v13_0_1.o yellow_carp_ppt.o +SMU13_MGR = smu_v13_0.o aldebaran_ppt.o yellow_carp_ppt.o AMD_SWSMU_SMU13MGR = $(addprefix $(AMD_SWSMU_PATH)/smu13/,$(SMU13_MGR)) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c index a3dc7194aaf8..cbce982f2717 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c @@ -41,6 +41,8 @@ #include "asic_reg/thm/thm_13_0_2_offset.h" #include "asic_reg/thm/thm_13_0_2_sh_mask.h" +#include "asic_reg/mp/mp_13_0_1_offset.h" +#include "asic_reg/mp/mp_13_0_1_sh_mask.h" If the offsets are same, are these files still needed or can they be not included/dropped completely? Thanks, Lijo ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https
Re: [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
Am 01.07.21 um 10:07 schrieb Pekka Paalanen: > On Wed, 30 Jun 2021 11:20:18 +0200 > Werner Sembach wrote: > >> Am 30.06.21 um 10:41 schrieb Pekka Paalanen: >> >>> On Tue, 29 Jun 2021 13:39:18 +0200 >>> Werner Sembach wrote: >>> Am 29.06.21 um 13:17 schrieb Pekka Paalanen: > On Tue, 29 Jun 2021 08:12:54 + > Simon Ser wrote: > >> On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen >> wrote: >> >>> yes, I think this makes sense, even if it is a property that one can't >>> tell for sure what it does before hand. >>> >>> Using a pair of properties, preference and active, to ask for something >>> and then check what actually worked is good for reducing the >>> combinatorial explosion caused by needing to "atomic TEST_ONLY commit" >>> test different KMS configurations. Userspace has a better chance of >>> finding a configuration that is possible. >>> >>> OTOH, this has the problem than in UI one cannot tell the user in >>> advance which options are truly possible. Given that KMS properties are >>> rarely completely independent, and in this case known to depend on >>> several other KMS properties, I think it is good enough to know after >>> the fact. >>> >>> If a driver does not use what userspace prefers, there is no way to >>> understand why, or what else to change to make it happen. That problem >>> exists anyway, because TEST_ONLY commits do not give useful feedback >>> but only a yes/no. >> By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing >> one >> property at a time), user-space can discover which property makes the >> atomic >> commit fail. > That works if the properties are independent of each other. Color > range, color format, bpc and more may all be interconnected, > allowing only certain combinations to work. > > If all these properties have "auto" setting too, then it would be > possible to probe each property individually, but that still does not > tell which combinations are valid. > > If you probe towards a certain configuration by setting the properties > one by one, then depending on the order you pick the properties, you > may come to a different conclusion on which property breaks the > configuration. My mind crossed another point that must be considered: When plugin in a Monitor a list of possible Resolutions+Framerate combinations is created for xrandr and other userspace (I guess by atomic checks? but I don't know). >>> Hi, >>> >>> I would not think so, but I hope to be corrected if I'm wrong. >>> >>> My belief is that the driver collects a list of modes from EDID, some >>> standard modes, and maybe some other hardcoded modes, and then >>> validates each entry against all the known limitations like vertical >>> and horizontal frequency limits, discarding modes that do not fit. >>> >>> Not all limitations are known during that phase, which is why KMS >>> property "link-status" exists. When userspace actually programs a mode >>> (not a TEST_ONLY commit), the link training may fail. The kernel prunes >>> the mode from the list and sets the link status property to signal >>> failure, and sends a hotplug uevent. Userspace needs to re-check the >>> mode list and try again. >>> >>> That is a generic escape hatch for when TEST_ONLY commit succeeds, but >>> in reality the hardware cannot do it, you just cannot know until you >>> actually try for real. It causes end user visible flicker if it happens >>> on an already running connector, but since it usually happens when >>> turning a connector on to begin with, there is no flicker to be seen, >>> just a small delay in finding a mode that works. >>> During this drm properties are already considered, which is no problem atm because as far as i can tell there is currently no drm property that would make a certain Resolutions+Framerate combination unreachable that would be possible with everything on default. >>> I would not expect KMS properties to be considered at all. It would >>> reject modes that are actually possible if the some KMS properties were >>> changed. So at least going forward, current KMS property values cannot >>> factor in. >> At least the debugfs variable "force_yuv420_output" did change the >> available modes here: >> https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L5165 >> >> before my patch >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=68eb3ae3c63708f823aeeb63bb15197c727bd9bf > Hi, > > debugfs is not proper UAPI, so we can just ignore it. Display servers > cannot be expected to poke in debugfs. Debugfs is not even supposed to > exist in production systems, but I'm sure people use it for hacking > stuff. But that's all it is for: developer testing and hacking. e.g. Ubun
Re: [PATCH] drm/amd/pm: drop smu_v13_0_1.c|h files for yellow carp
[AMD Official Use Only] Reviewed-by: Kevin Wang Best Regards, Kevin From: Hou, Xiaomeng (Matthew) Sent: Thursday, July 1, 2021 4:31 PM To: amd-gfx@lists.freedesktop.org Cc: Wang, Kevin(Yang) ; Lazar, Lijo ; Liu, Aaron ; Hou, Xiaomeng (Matthew) Subject: [PATCH] drm/amd/pm: drop smu_v13_0_1.c|h files for yellow carp Since there's nothing special in smu implementation for yellow carp, it's better to reuse the common smu_v13_0 interfaces and drop the specific smu_v13_0_1.c|h files. Signed-off-by: Xiaomeng Hou --- drivers/gpu/drm/amd/pm/inc/smu_v13_0.h| 1 + drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h | 57 drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile | 2 +- .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c| 26 ++ .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_1.c | 311 -- .../drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c | 39 ++- 6 files changed, 59 insertions(+), 377 deletions(-) delete mode 100644 drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h delete mode 100644 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_1.c diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h b/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h index 6119a36b2cba..3fea2430dec0 100644 --- a/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h +++ b/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h @@ -26,6 +26,7 @@ #include "amdgpu_smu.h" #define SMU13_DRIVER_IF_VERSION_INV 0x +#define SMU13_DRIVER_IF_VERSION_YELLOW_CARP 0x03 #define SMU13_DRIVER_IF_VERSION_ALDE 0x07 /* MP Apertures */ diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h b/drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h deleted file mode 100644 index b6c976a4d578.. --- a/drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright 2020 Advanced Micro Devices, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR - * OTHER DEALINGS IN THE SOFTWARE. - * - */ -#ifndef __SMU_V13_0_1_H__ -#define __SMU_V13_0_1_H__ - -#include "amdgpu_smu.h" - -#define SMU13_0_1_DRIVER_IF_VERSION_INV 0x -#define SMU13_0_1_DRIVER_IF_VERSION_YELLOW_CARP 0x3 - -/* MP Apertures */ -#define MP0_Public 0x0380 -#define MP0_SRAM 0x0390 -#define MP1_Public 0x03b0 -#define MP1_SRAM 0x03c4 - -/* address block */ -#define smnMP1_FIRMWARE_FLAGS 0x3010024 - - -#if defined(SWSMU_CODE_LAYER_L2) || defined(SWSMU_CODE_LAYER_L3) - -int smu_v13_0_1_check_fw_status(struct smu_context *smu); - -int smu_v13_0_1_check_fw_version(struct smu_context *smu); - -int smu_v13_0_1_fini_smc_tables(struct smu_context *smu); - -int smu_v13_0_1_get_vbios_bootup_values(struct smu_context *smu); - -int smu_v13_0_1_set_default_dpm_tables(struct smu_context *smu); - -int smu_v13_0_1_set_driver_table_location(struct smu_context *smu); - -int smu_v13_0_1_gfx_off_control(struct smu_context *smu, bool enable); -#endif -#endif diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile b/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile index 9b3a8503f5cd..d4c4c495762c 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile @@ -23,7 +23,7 @@ # Makefile for the 'smu manager' sub-component of powerplay. # It provides the smu management services for the driver. -SMU13_MGR = smu_v13_0.o aldebaran_ppt.o smu_v13_0_1.o yellow_carp_ppt.o +SMU13_MGR = smu_v13_0.o aldebaran_ppt.o yellow_carp_ppt.o AMD_SWSMU_SMU13MGR = $(addprefix $(AMD_SWSMU_PATH)/smu13/,$(SMU13_MGR)) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c index a3dc7194aaf8..cbce982f2717 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c @@ -41,6 +41,8 @@ #include "asic_reg/thm/thm_13_0_2_offset.h" #include "asic_reg/thm/thm_13_0_2_sh_mask.h" +#include "asic_reg/mp/mp_13_0_1_offset.h" +#include "asic_reg/mp/mp_13_0_1_sh_mask
[PATCH] drm/amd/pm: drop smu_v13_0_1.c|h files for yellow carp
Since there's nothing special in smu implementation for yellow carp, it's better to reuse the common smu_v13_0 interfaces and drop the specific smu_v13_0_1.c|h files. Signed-off-by: Xiaomeng Hou --- drivers/gpu/drm/amd/pm/inc/smu_v13_0.h| 1 + drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h | 57 drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile | 2 +- .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c| 26 ++ .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_1.c | 311 -- .../drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c | 39 ++- 6 files changed, 59 insertions(+), 377 deletions(-) delete mode 100644 drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h delete mode 100644 drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_1.c diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h b/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h index 6119a36b2cba..3fea2430dec0 100644 --- a/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h +++ b/drivers/gpu/drm/amd/pm/inc/smu_v13_0.h @@ -26,6 +26,7 @@ #include "amdgpu_smu.h" #define SMU13_DRIVER_IF_VERSION_INV 0x +#define SMU13_DRIVER_IF_VERSION_YELLOW_CARP 0x03 #define SMU13_DRIVER_IF_VERSION_ALDE 0x07 /* MP Apertures */ diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h b/drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h deleted file mode 100644 index b6c976a4d578.. --- a/drivers/gpu/drm/amd/pm/inc/smu_v13_0_1.h +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright 2020 Advanced Micro Devices, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR - * OTHER DEALINGS IN THE SOFTWARE. - * - */ -#ifndef __SMU_V13_0_1_H__ -#define __SMU_V13_0_1_H__ - -#include "amdgpu_smu.h" - -#define SMU13_0_1_DRIVER_IF_VERSION_INV 0x -#define SMU13_0_1_DRIVER_IF_VERSION_YELLOW_CARP 0x3 - -/* MP Apertures */ -#define MP0_Public 0x0380 -#define MP0_SRAM 0x0390 -#define MP1_Public 0x03b0 -#define MP1_SRAM 0x03c4 - -/* address block */ -#define smnMP1_FIRMWARE_FLAGS 0x3010024 - - -#if defined(SWSMU_CODE_LAYER_L2) || defined(SWSMU_CODE_LAYER_L3) - -int smu_v13_0_1_check_fw_status(struct smu_context *smu); - -int smu_v13_0_1_check_fw_version(struct smu_context *smu); - -int smu_v13_0_1_fini_smc_tables(struct smu_context *smu); - -int smu_v13_0_1_get_vbios_bootup_values(struct smu_context *smu); - -int smu_v13_0_1_set_default_dpm_tables(struct smu_context *smu); - -int smu_v13_0_1_set_driver_table_location(struct smu_context *smu); - -int smu_v13_0_1_gfx_off_control(struct smu_context *smu, bool enable); -#endif -#endif diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile b/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile index 9b3a8503f5cd..d4c4c495762c 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/Makefile @@ -23,7 +23,7 @@ # Makefile for the 'smu manager' sub-component of powerplay. # It provides the smu management services for the driver. -SMU13_MGR = smu_v13_0.o aldebaran_ppt.o smu_v13_0_1.o yellow_carp_ppt.o +SMU13_MGR = smu_v13_0.o aldebaran_ppt.o yellow_carp_ppt.o AMD_SWSMU_SMU13MGR = $(addprefix $(AMD_SWSMU_PATH)/smu13/,$(SMU13_MGR)) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c index a3dc7194aaf8..cbce982f2717 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c @@ -41,6 +41,8 @@ #include "asic_reg/thm/thm_13_0_2_offset.h" #include "asic_reg/thm/thm_13_0_2_sh_mask.h" +#include "asic_reg/mp/mp_13_0_1_offset.h" +#include "asic_reg/mp/mp_13_0_1_sh_mask.h" #include "asic_reg/mp/mp_13_0_2_offset.h" #include "asic_reg/mp/mp_13_0_2_sh_mask.h" #include "asic_reg/smuio/smuio_13_0_2_offset.h" @@ -210,6 +212,9 @@ int smu_v13_0_check_fw_version(struct smu_context *smu) case CHIP_ALDEBARAN: smu->smc_driver_if_version = SMU13_DRIVER_IF_VERSION_ALDE; break; + c
Re: [PATCH v4 03/17] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property
Am 01.07.21 um 09:42 schrieb Pekka Paalanen:
> On Wed, 30 Jun 2021 11:42:10 +0200
> Werner Sembach wrote:
>
>> Am 30.06.21 um 10:21 schrieb Pekka Paalanen:
>>> On Tue, 29 Jun 2021 13:02:05 +0200
>>> Werner Sembach wrote:
>>>
Am 28.06.21 um 19:03 schrieb Werner Sembach:
> Am 18.06.21 um 11:11 schrieb Werner Sembach:
>> Add a new general drm property "active bpc" which can be used by graphic
>> drivers to report the applied bit depth per pixel back to userspace.
>>
>> While "max bpc" can be used to change the color depth, there was no way
>> to
>> check which one actually got used. While in theory the driver chooses the
>> best/highest color depth within the max bpc setting a user might not be
>> fully aware what his hardware is or isn't capable off. This is meant as a
>> quick way to double check the setup.
>>
>> In the future, automatic color calibration for screens might also depend
>> on
>> this information being available.
>>
>> Signed-off-by: Werner Sembach
>> ---
>> drivers/gpu/drm/drm_connector.c | 51 +
>> include/drm/drm_connector.h | 8 ++
>> 2 files changed, 59 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c
>> b/drivers/gpu/drm/drm_connector.c
>> index da39e7ff6965..943f6b61053b 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -1197,6 +1197,14 @@ static const struct drm_prop_enum_list
>> dp_colorspaces[] = {
>>* drm_connector_attach_max_bpc_property() to create and attach the
>>* property to the connector during initialization.
>>*
>> + * active bpc:
>> + * This read-only range property tells userspace the pixel color
>> bit depth
>> + * actually used by the hardware display engine on "the cable" on a
>> + * connector. The chosen value depends on hardware capabilities,
>> both
>> + * display engine and connected monitor, and the "max bpc"
>> property.
>> + * Drivers shall use drm_connector_attach_active_bpc_property() to
>> install
>> + * this property.
>> + *
> Regarding "on the cable" and dithering: As far as I can tell, what the
> dithering option does, is setting a hardware
> register here:
>
> -
> https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/i915/display/intel_display.c#L4534
>
> -
> https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/i915/display/intel_display.c#L4571
>
> So dithering seems to be calculated by fixed purpose hardware/firmware
> outside of the driver?
>
> The Intel driver does not seem to set a target bpc/bpp for this hardware
> so I guess it defaults to 6 or 8 bpc?
Never mind it does. This switch-case does affect the dithering output:
https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/i915/display/intel_display.c#L4537
>>> Hi,
>>>
>>> I obviously do not know the intel driver or hardware at all, but
>>> to me that just looks like translating from bits per pixel to bits per
>>> channel in RGB mapping?
>> No, if i understand the documentation correctly: Writing bit depth here
>> with dithering enabled sets the dithering target bpc.
>>>
As found in this documentation p.548:
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-lkf-vol02c-commandreference-registers-part2.pdf
So max bpc and active bpc are affecting/affected by the bpc after
dithering.
>>> By definition, if the cable carries N bpc, then dithering does not
>>> change that. The cable still carries N bpc, but due to spatial or
>>> temporal dithering, the *observed* color resolution may or may not be
>>> higher than the cable bpc.
>> Yes, and max bpc and active bpc tell the cable bpc ist not the
>> *observed* bpc.
>>> Of course, if the cable bpc is 8, and dithering targets 6 bpc, then 2
>>> LSB on the cable are always zero, right?
>> I would assume that in this case only 6 bpc are actually send? Isn't the
>> whole thing of dithering that you can't send, for example, 8 bpc?
>>> Maybe one would want to do that if the monitor has a 6 bit panel and it
>>> simply ignored the 2 LSB, and the cable cannot go down to 6 bpc.
>> Is there dithering actually doing this? aka is my assumption above wrong?
>>
>> AMD code that confused me before, is hinting that you might be right:
>> https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c#L826
>>
>> there is a set_clamp depth and a separate DCP_SPATIAL_DITHER_DEPTH_30BPP
>>
>>> So, what does "max bpc" mean right now?
>>>
>>> It seems like dither on/off is insufficient information, one would also
>>> need to control the dithering target bpc. I suppose the driver has a
>>> policy on how it chooses the target bpc, but wha
Re: [PATCH 1/1] drm/amdgpu: Read clock counter via MMIO to reduce delay (v5)
Am 01.07.21 um 11:57 schrieb YuBiao Wang:
[Why]
GPU timing counters are read via KIQ under sriov, which will introduce
a delay.
[How]
It could be directly read by MMIO.
v2: Add additional check to prevent carryover issue.
v3: Only check for carryover for once to prevent performance issue.
v4: Add comments of the rough frequency where carryover happens.
v5: Remove mutex and gfxoff ctrl unused with current timing registers.
Signed-off-by: YuBiao Wang
Acked-by: Horace Chen
The preemption_disable()/_enable() would still be nice to have I think.
Anyway patch is Reviewed-by: Christian König
either way.
Regards,
Christian.
---
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index ff7e9f49040e..5f4eae9c9526 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -7609,10 +7609,8 @@ static int gfx_v10_0_soft_reset(void *handle)
static uint64_t gfx_v10_0_get_gpu_clock_counter(struct amdgpu_device *adev)
{
- uint64_t clock;
+ uint64_t clock, clock_lo, clock_hi, hi_check;
- amdgpu_gfx_off_ctrl(adev, false);
- mutex_lock(&adev->gfx.gpu_clock_mutex);
switch (adev->asic_type) {
case CHIP_VANGOGH:
case CHIP_YELLOW_CARP:
@@ -7620,12 +7618,19 @@ static uint64_t gfx_v10_0_get_gpu_clock_counter(struct
amdgpu_device *adev)
((uint64_t)RREG32_SOC15(SMUIO, 0,
mmGOLDEN_TSC_COUNT_UPPER_Vangogh) << 32ULL);
break;
default:
- clock = (uint64_t)RREG32_SOC15(SMUIO, 0,
mmGOLDEN_TSC_COUNT_LOWER) |
- ((uint64_t)RREG32_SOC15(SMUIO, 0, mmGOLDEN_TSC_COUNT_UPPER)
<< 32ULL);
+ clock_hi = RREG32_SOC15_NO_KIQ(SMUIO, 0,
mmGOLDEN_TSC_COUNT_UPPER);
+ clock_lo = RREG32_SOC15_NO_KIQ(SMUIO, 0,
mmGOLDEN_TSC_COUNT_LOWER);
+ hi_check = RREG32_SOC15_NO_KIQ(SMUIO, 0,
mmGOLDEN_TSC_COUNT_UPPER);
+ /* The GFX clock frequency is 100MHz, which sets 32-bit carry
over
+* roughly every 42 seconds.
+*/
+ if (hi_check != clock_hi) {
+ clock_lo = RREG32_SOC15_NO_KIQ(SMUIO, 0,
mmGOLDEN_TSC_COUNT_LOWER);
+ clock_hi = hi_check;
+ }
+ clock = (uint64_t)clock_lo | ((uint64_t)clock_hi << 32ULL);
break;
}
- mutex_unlock(&adev->gfx.gpu_clock_mutex);
- amdgpu_gfx_off_ctrl(adev, true);
return clock;
}
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Correct the irq numbers for virtual ctrc
Signed-off-by: Emily Deng
Signed-off-by: Victor
---
drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index 33324427b555..7e0d8c092c7e 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -766,7 +766,7 @@ static const struct amdgpu_irq_src_funcs
dce_virtual_crtc_irq_funcs = {
static void dce_virtual_set_irq_funcs(struct amdgpu_device *adev)
{
- adev->crtc_irq.num_types = AMDGPU_CRTC_IRQ_VBLANK6 + 1;
+ adev->crtc_irq.num_types = adev->mode_info.num_crtc;
adev->crtc_irq.funcs = &dce_virtual_crtc_irq_funcs;
}
--
2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2] drm/amdgpu: Avoid printing of stack contents on firmware load error
On Thu, 24 Jun 2021, Jiri Kosina wrote:
> From: Jiri Kosina
>
> In case when psp_init_asd_microcode() fails to load ASD microcode file,
> psp_v12_0_init_microcode() tries to print the firmware filename that
> failed to load before bailing out.
>
> This is wrong because:
>
> - the firmware filename it would want it print is an incorrect one as
> psp_init_asd_microcode() and psp_v12_0_init_microcode() are loading
> different filenames
> - it tries to print fw_name, but that's not yet been initialized by that
> time, so it prints random stack contents, e.g.
>
> amdgpu :04:00.0: Direct firmware load for amdgpu/renoir_asd.bin
> failed with error -2
> amdgpu :04:00.0: amdgpu: fail to initialize asd microcode
> amdgpu :04:00.0: amdgpu: psp v12.0: Failed to load firmware
> "\xfeTO\x8e\xff\xff"
>
> Fix that by bailing out immediately, instead of priting the bogus error
> message.
Friendly ping on this one too; priting a few bytes of stack is not a
*huge* info leak, but I believe it should be fixed nevertheless.
Thanks.
>
> Reported-by: Vojtech Pavlik
> Signed-off-by: Jiri Kosina
> ---
>
> v1 -> v2: remove now-unused label
>
> drivers/gpu/drm/amd/amdgpu/psp_v12_0.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> index c4828bd3264b..b0ee77ee80b9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> @@ -67,7 +67,7 @@ static int psp_v12_0_init_microcode(struct psp_context *psp)
>
> err = psp_init_asd_microcode(psp, chip_name);
> if (err)
> - goto out;
> + return err;
>
> snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
> err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
> @@ -80,7 +80,7 @@ static int psp_v12_0_init_microcode(struct psp_context *psp)
> } else {
> err = amdgpu_ucode_validate(adev->psp.ta_fw);
> if (err)
> - goto out2;
> + goto out;
>
> ta_hdr = (const struct ta_firmware_header_v1_0 *)
>adev->psp.ta_fw->data;
> @@ -105,10 +105,9 @@ static int psp_v12_0_init_microcode(struct psp_context
> *psp)
>
> return 0;
>
> -out2:
> +out:
> release_firmware(adev->psp.ta_fw);
> adev->psp.ta_fw = NULL;
> -out:
> if (err) {
> dev_err(adev->dev,
> "psp v12.0: Failed to load firmware \"%s\"\n",
> --
> 2.12.3
>
--
Jiri Kosina
SUSE Labs
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Fix resource leak on probe error path
On Thu, 24 Jun 2021, Jiri Kosina wrote:
> From: Jiri Kosina
>
> This reverts commit 4192f7b5768912ceda82be2f83c87ea7181f9980.
>
> It is not true (as stated in the reverted commit changelog) that we never
> unmap the BAR on failure; it actually does happen properly on
> amdgpu_driver_load_kms() -> amdgpu_driver_unload_kms() ->
> amdgpu_device_fini() error path.
>
> What's worse, this commit actually completely breaks resource freeing on
> probe failure (like e.g. failure to load microcode), as
> amdgpu_driver_unload_kms() notices adev->rmmio being NULL and bails too
> early, leaving all the resources that'd normally be freed in
> amdgpu_acpi_fini() and amdgpu_device_fini() still hanging around, leading
> to all sorts of oopses when someone tries to, for example, access the
> sysfs and procfs resources which are still around while the driver is
> gone.
>
> Fixes: 4192f7b57689 ("drm/amdgpu: unmap register bar on device init failure")
> Reported-by: Vojtech Pavlik
> Signed-off-by: Jiri Kosina
Friendly ping on this one (as it's easily triggerable in the wild by just
missing proper firwmare).
Thanks.
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 57ec108b5972..0f1c0e17a587 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3414,13 +3414,13 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> r = amdgpu_device_get_job_timeout_settings(adev);
> if (r) {
> dev_err(adev->dev, "invalid lockup_timeout parameter syntax\n");
> - goto failed_unmap;
> + return r;
> }
>
> /* early init functions */
> r = amdgpu_device_ip_early_init(adev);
> if (r)
> - goto failed_unmap;
> + return r;
>
> /* doorbell bar mapping and doorbell index init*/
> amdgpu_device_doorbell_init(adev);
> @@ -3646,10 +3646,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> failed:
> amdgpu_vf_error_trans_all(adev);
>
> -failed_unmap:
> - iounmap(adev->rmmio);
> - adev->rmmio = NULL;
> -
> return r;
> }
>
> --
> 2.12.3
>
>
--
Jiri Kosina
SUSE Labs
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] SWDEV-235359 drm/amdgpu: Correct the irq numbers for virtual ctrc
Change-Id: I02035f65b71ec52795c3e8ae979fb582c3cce592
Signed-off-by: Emily Deng
Signed-off-by: Victor
---
drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index 33324427b555..7e0d8c092c7e 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -766,7 +766,7 @@ static const struct amdgpu_irq_src_funcs
dce_virtual_crtc_irq_funcs = {
static void dce_virtual_set_irq_funcs(struct amdgpu_device *adev)
{
- adev->crtc_irq.num_types = AMDGPU_CRTC_IRQ_VBLANK6 + 1;
+ adev->crtc_irq.num_types = adev->mode_info.num_crtc;
adev->crtc_irq.funcs = &dce_virtual_crtc_irq_funcs;
}
--
2.25.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[pull] amdgpu, amdkfd, radeon drm-next-5.14
Hi Dave, Daniel, Fixes for 5.14. The following changes since commit b322a50d17ede5cff6622040f345228afecdcc45: Merge tag 'amd-drm-next-5.14-2021-06-22-1' of https://gitlab.freedesktop.org/agd5f/linux into drm-next (2021-06-24 07:57:41 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-next-5.14-2021-07-01 for you to fetch changes up to 93c5bcd4eaaafd7c25c062089806c86d9b7890dd: drm/amdgpu: Conditionally reset SDMA RAS error counts (2021-07-01 00:05:41 -0400) amd-drm-next-5.14-2021-07-01: amdgpu: - Misc Navi fixes - Powergating fix - Yellow Carp updates - Beige Goby updates - S0ix fix - Revert overlay validation fix - GPU reset fix for DC - PPC64 fix - Add new dimgrey cavefish DID - RAS fix amdkfd: - SVM fixes radeon: - Fix missing drm_gem_object_put in error path Aaron Liu (2): drm/amdgpu: enable tmz on yellow carp drm/amdgpu: enable sdma0 tmz for Raven/Renoir(V2) Alex Deucher (2): drm/amdgpu/display: drop unused variable drm/amdgpu: add new dimgrey cavefish DID Alex Sierra (11): drm/amdkfd: inc counter on child ranges with xnack off drm/amdkfd: device pgmap owner at the svm migrate init drm/amdkfd: add owner ref param to get hmm pages drm/amdkfd: set owner ref to svm range prefault drm/amdgpu: get owner ref in validate and map drm/amdkfd: use hmm range fault to get both domain pfns drm/amdkfd: classify and map mixed svm range pages in GPU drm/amdkfd: skip invalid pages during migrations drm/amdkfd: skip migration for pages already in VRAM drm/amdkfd: add invalid pages debug at vram migration drm/amdkfd: Maintain svm_bo reference in page->zone_device_data Chengming Gui (1): drm/amd/amdgpu: enable gpu recovery for beige_goby Chengzhe Liu (1): drm/amdgpu: Power down VCN and JPEG before disabling SMU features Darren Powell (1): amdgpu/pm: remove code duplication in show_power_cap calls Evan Quan (7): drm/amdgpu: correct tcp harvest setting drm/amdgpu: fix Navi1x tcp power gating hang when issuing lightweight invalidaiton drm/amdgpu: fix NAK-G generation during PCI-e link width switch drm/amdgpu: fix the hang caused by PCIe link width switch drm/amdgpu: correct clock gating settings on feature unsupported drm/amdgpu: update GFX MGCG settings drm/amdgpu: update HDP LS settings Guchun Chen (2): drm/amd/display: fix incorrrect valid irq check drm/amd/display: fix null pointer access in gpu reset Huang Rui (1): drm/amdgpu: move apu flags initialization to the start of device init Jing Xiangfeng (1): drm/radeon: Add the missed drm_gem_object_put() in radeon_user_framebuffer_create() Joseph Greathouse (1): drm/amdgpu: Update NV SIMD-per-CU to 2 Michal Suchanek (1): drm/amdgpu/dc: Really fix DCN3.1 Makefile for PPC64 Mukul Joshi (1): drm/amdgpu: Conditionally reset SDMA RAS error counts Nicholas Kazlauskas (1): drm/amd/display: Extend DMUB diagnostic logging to DCN3.1 Oak Zeng (1): drm/amdgpu: Set ttm caching flags during bo allocation Philip Yang (4): drm/amdkfd: add helper function for kfd sysfs create drm/amdkfd: fix sysfs kobj leak drm/amdkfd: add sysfs counters for vm fault and migration drm/amdkfd: implement counters for vm fault and migration Reka Norman (1): drm/amd/display: Respect CONFIG_FRAME_WARN=0 in dml Makefile Rodrigo Siqueira (1): Revert "drm/amd/display: Fix overlay validation by considering cursors" Shyam Sundar S K (1): drm/amd/pm: skip PrepareMp1ForUnload message in s0ix Tiezhu Yang (1): drm/radeon: Call radeon_suspend_kms() in radeon_pci_shutdown() for Loongson64 Veerabadhran Gopalakrishnan (1): amdgpu/nv.c - Added codec query for Beige Goby Zhan Liu (1): drm/amd/display: Enabling eDP no power sequencing with DAL feature mask drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 37 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 2 + drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.h | 2 + drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 + drivers/gpu/drm/amd/amdgpu/athub_v2_0.c| 12 +- drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 266 +++- drivers/gpu/drm/amd/amdgpu/hdp_v5_0.c | 85 --- drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c| 10 +- drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c | 51 +++- drivers/gpu/drm/amd/amdgpu/nv.c| 37 ++- drivers/gpu/drm/amd/amd
Re: [PATCH v4 12/17] drm/uAPI: Add "preferred color format" drm property as setting for userspace
On Wed, 30 Jun 2021 11:20:18 +0200 Werner Sembach wrote: > Am 30.06.21 um 10:41 schrieb Pekka Paalanen: > > > On Tue, 29 Jun 2021 13:39:18 +0200 > > Werner Sembach wrote: > > > >> Am 29.06.21 um 13:17 schrieb Pekka Paalanen: > >>> On Tue, 29 Jun 2021 08:12:54 + > >>> Simon Ser wrote: > >>> > On Tuesday, June 22nd, 2021 at 09:15, Pekka Paalanen > wrote: > > > yes, I think this makes sense, even if it is a property that one can't > > tell for sure what it does before hand. > > > > Using a pair of properties, preference and active, to ask for something > > and then check what actually worked is good for reducing the > > combinatorial explosion caused by needing to "atomic TEST_ONLY commit" > > test different KMS configurations. Userspace has a better chance of > > finding a configuration that is possible. > > > > OTOH, this has the problem than in UI one cannot tell the user in > > advance which options are truly possible. Given that KMS properties are > > rarely completely independent, and in this case known to depend on > > several other KMS properties, I think it is good enough to know after > > the fact. > > > > If a driver does not use what userspace prefers, there is no way to > > understand why, or what else to change to make it happen. That problem > > exists anyway, because TEST_ONLY commits do not give useful feedback > > but only a yes/no. > By submitting incremental atomic reqs with TEST_ONLY (i.e. only changing > one > property at a time), user-space can discover which property makes the > atomic > commit fail. > >>> That works if the properties are independent of each other. Color > >>> range, color format, bpc and more may all be interconnected, > >>> allowing only certain combinations to work. > >>> > >>> If all these properties have "auto" setting too, then it would be > >>> possible to probe each property individually, but that still does not > >>> tell which combinations are valid. > >>> > >>> If you probe towards a certain configuration by setting the properties > >>> one by one, then depending on the order you pick the properties, you > >>> may come to a different conclusion on which property breaks the > >>> configuration. > >> My mind crossed another point that must be considered: When plugin in > >> a Monitor a list of possible Resolutions+Framerate combinations is > >> created for xrandr and other userspace (I guess by atomic checks? but > >> I don't know). > > Hi, > > > > I would not think so, but I hope to be corrected if I'm wrong. > > > > My belief is that the driver collects a list of modes from EDID, some > > standard modes, and maybe some other hardcoded modes, and then > > validates each entry against all the known limitations like vertical > > and horizontal frequency limits, discarding modes that do not fit. > > > > Not all limitations are known during that phase, which is why KMS > > property "link-status" exists. When userspace actually programs a mode > > (not a TEST_ONLY commit), the link training may fail. The kernel prunes > > the mode from the list and sets the link status property to signal > > failure, and sends a hotplug uevent. Userspace needs to re-check the > > mode list and try again. > > > > That is a generic escape hatch for when TEST_ONLY commit succeeds, but > > in reality the hardware cannot do it, you just cannot know until you > > actually try for real. It causes end user visible flicker if it happens > > on an already running connector, but since it usually happens when > > turning a connector on to begin with, there is no flicker to be seen, > > just a small delay in finding a mode that works. > > > >> During this drm > >> properties are already considered, which is no problem atm because as > >> far as i can tell there is currently no drm property that would make > >> a certain Resolutions+Framerate combination unreachable that would be > >> possible with everything on default. > > I would not expect KMS properties to be considered at all. It would > > reject modes that are actually possible if the some KMS properties were > > changed. So at least going forward, current KMS property values cannot > > factor in. > > At least the debugfs variable "force_yuv420_output" did change the > available modes here: > https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L5165 > > before my patch > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=68eb3ae3c63708f823aeeb63bb15197c727bd9bf Hi, debugfs is not proper UAPI, so we can just ignore it. Display servers cannot be expected to poke in debugfs. Debugfs is not even supposed to exist in production systems, but I'm sure people use it for hacking stuff. But that's all it is for: developer testing and hacking. > Forcing a color format via a DRM property in this function would > rei
Re: [PATCH v4 03/17] drm/uAPI: Add "active bpc" as feedback channel for "max bpc" drm property
On Wed, 30 Jun 2021 11:42:10 +0200
Werner Sembach wrote:
> Am 30.06.21 um 10:21 schrieb Pekka Paalanen:
> > On Tue, 29 Jun 2021 13:02:05 +0200
> > Werner Sembach wrote:
> >
> >> Am 28.06.21 um 19:03 schrieb Werner Sembach:
> >>> Am 18.06.21 um 11:11 schrieb Werner Sembach:
> Add a new general drm property "active bpc" which can be used by graphic
> drivers to report the applied bit depth per pixel back to userspace.
>
> While "max bpc" can be used to change the color depth, there was no way
> to
> check which one actually got used. While in theory the driver chooses the
> best/highest color depth within the max bpc setting a user might not be
> fully aware what his hardware is or isn't capable off. This is meant as a
> quick way to double check the setup.
>
> In the future, automatic color calibration for screens might also depend
> on
> this information being available.
>
> Signed-off-by: Werner Sembach
> ---
> drivers/gpu/drm/drm_connector.c | 51 +
> include/drm/drm_connector.h | 8 ++
> 2 files changed, 59 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c
> b/drivers/gpu/drm/drm_connector.c
> index da39e7ff6965..943f6b61053b 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1197,6 +1197,14 @@ static const struct drm_prop_enum_list
> dp_colorspaces[] = {
> * drm_connector_attach_max_bpc_property() to create and attach the
> * property to the connector during initialization.
> *
> + * active bpc:
> + * This read-only range property tells userspace the pixel color
> bit depth
> + * actually used by the hardware display engine on "the cable" on a
> + * connector. The chosen value depends on hardware capabilities,
> both
> + * display engine and connected monitor, and the "max bpc"
> property.
> + * Drivers shall use drm_connector_attach_active_bpc_property() to
> install
> + * this property.
> + *
> >>> Regarding "on the cable" and dithering: As far as I can tell, what the
> >>> dithering option does, is setting a hardware
> >>> register here:
> >>>
> >>> -
> >>> https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/i915/display/intel_display.c#L4534
> >>>
> >>> -
> >>> https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/i915/display/intel_display.c#L4571
> >>>
> >>> So dithering seems to be calculated by fixed purpose hardware/firmware
> >>> outside of the driver?
> >>>
> >>> The Intel driver does not seem to set a target bpc/bpp for this hardware
> >>> so I guess it defaults to 6 or 8 bpc?
> >> Never mind it does. This switch-case does affect the dithering output:
> >> https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/i915/display/intel_display.c#L4537
> >>
> > Hi,
> >
> > I obviously do not know the intel driver or hardware at all, but
> > to me that just looks like translating from bits per pixel to bits per
> > channel in RGB mapping?
> No, if i understand the documentation correctly: Writing bit depth here
> with dithering enabled sets the dithering target bpc.
> >
> >> As found in this documentation p.548:
> >> https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-lkf-vol02c-commandreference-registers-part2.pdf
> >>
> >> So max bpc and active bpc are affecting/affected by the bpc after
> >> dithering.
> > By definition, if the cable carries N bpc, then dithering does not
> > change that. The cable still carries N bpc, but due to spatial or
> > temporal dithering, the *observed* color resolution may or may not be
> > higher than the cable bpc.
> Yes, and max bpc and active bpc tell the cable bpc ist not the
> *observed* bpc.
> >
> > Of course, if the cable bpc is 8, and dithering targets 6 bpc, then 2
> > LSB on the cable are always zero, right?
> I would assume that in this case only 6 bpc are actually send? Isn't the
> whole thing of dithering that you can't send, for example, 8 bpc?
> >
> > Maybe one would want to do that if the monitor has a 6 bit panel and it
> > simply ignored the 2 LSB, and the cable cannot go down to 6 bpc.
>
> Is there dithering actually doing this? aka is my assumption above wrong?
>
> AMD code that confused me before, is hinting that you might be right:
> https://elixir.bootlin.com/linux/v5.13/source/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c#L826
>
> there is a set_clamp depth and a separate DCP_SPATIAL_DITHER_DEPTH_30BPP
>
> >
> > So, what does "max bpc" mean right now?
> >
> > It seems like dither on/off is insufficient information, one would also
> > need to control the dithering target bpc. I suppose the driver has a
> > policy on how it chooses the target bpc, but what is that policy? Is
> > the dither target bpc t
