[linux-next:master] BUILD REGRESSION 2a2aa3f05338270aecbe2492fda910d6c17e0102
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 2a2aa3f05338270aecbe2492fda910d6c17e0102 Add linux-next specific files for 20220705 Error/Warning reports: https://lore.kernel.org/linux-doc/202207051821.3f0erisl-...@intel.com Error/Warning: (recently discovered and may have been fixed) Documentation/PCI/endpoint/pci-vntb-function.rst:82: WARNING: Unexpected indentation. Documentation/PCI/endpoint/pci-vntb-howto.rst:131: WARNING: Title underline too short. drivers/pci/endpoint/functions/pci-epf-vntb.c:975:5: warning: no previous prototype for 'pci_read' [-Wmissing-prototypes] drivers/pci/endpoint/functions/pci-epf-vntb.c:984:5: warning: no previous prototype for 'pci_write' [-Wmissing-prototypes] Unverified Error/Warning (likely false positive, please contact us if interested): block/partitions/efi.c:223:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 block/sed-opal.c:427:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 crypto/asymmetric_keys/pkcs7_verify.c:311:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/ata/libata-core.c:2802:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/ata/libata-eh.c:2842:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/ata/sata_dwc_460ex.c:691:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/base/power/runtime.c:1570:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/block/rbd.c:6142:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/bluetooth/hci_ll.c:588:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/bluetooth/hci_qca.c:2137:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/cdrom/cdrom.c:1041:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/char/ipmi/ipmi_ssif.c:1918:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/char/pcmcia/cm4000_cs.c:922:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/char/random.c:869:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/char/tpm/tpm_tis_core.c:1122:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/clk/bcm/clk-iproc-armpll.c:139:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/clk/clk-bd718x7.c:50:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/clk/clk-lochnagar.c:187:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/crypto/ccree/cc_request_mgr.c:206:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/crypto/qce/sha.c:73:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/crypto/qce/skcipher.c:61:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/cxl/core/hdm.c:38:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/cxl/core/pci.c:67:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/dma-buf/dma-buf.c:1100:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/firmware/arm_scmi/bus.c:152:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/firmware/arm_scmi/clock.c:394:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/firmware/arm_scmi/powercap.c:376:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/firmware/arm_scmi/sensors.c:673:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/firmware/arm_scmi/voltage.c:363:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/fpga/dfl-fme-mgr.c:163:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/gnss/usb.c:68:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_debug.c:175:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:1006:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_resource.c:1035:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/gpu/drm/amd/amdgpu/../display/dc/dce112/dce112_resource.c:955:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu7_hwmgr.c:3895:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/smu8_hwmgr.c:754:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/gpu/drm/amd/amdgpu/../pm/powerplay/hwmgr/vega10_powertune.c:1214:1: internal compiler error: in arc_ifcvt, at config/arc/arc.c:9637 drivers/gpu/drm/amd/amdgpu/../pm/powerplay/smumgr/smu7_smum
[pull] amdgpu, amdkfd, radeon drm-next-5.20
Hi Dave, Daniel, New stuff for 5.20. The following changes since commit bf23729c7a5f44f0e863666b9364a64741fd3241: Merge tag 'amd-drm-next-5.19-2022-05-26-2' of https://gitlab.freedesktop.org/agd5f/linux into drm-next (2022-06-01 10:56:11 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-next-5.20-2022-07-05 for you to fetch changes up to c5da61cf5bab30059f22ea368702c445ee87171a: drm/amdgpu/display: add missing FP_START/END checks dcn32_clk_mgr.c (2022-06-30 19:35:21 -0400) amd-drm-next-5.20-2022-07-05: amdgpu: - Various spelling and grammer fixes - Various eDP fixes - Various DMCUB fixes - VCN fixes - GMC 11 fixes - RAS fixes - TMZ support for GC 10.3.7 - GPUVM TLB flush fixes - SMU 13.0.x updates - DCN 3.2 Support - DCN 3.2.1 Support - MES updates - GFX11 modifiers support - USB-C fixes - MMHUB 3.0.1 support - SDMA 6.0 doorbell fixes - Initial devcoredump support - Enable high priority gfx queue on asics which support it - Enable GPU reset for SMU 13.0.4 - OLED display fixes - MPO fixes - DC frame size fixes - ASPM support for PCIE 7.4/7.6 - GPU reset support for SMU 13.0.0 - GFX11 updates - VCN JPEG fix - BACO support for SMU 13.0.7 - VCN instance handling fix - GFX8 GPUVM TLB flush fix - GPU reset rework - VCN 4.0.2 support - GTT size fixes - DP link training fixes - LSDMA 6.0.1 support - Various backlight fixes - Color encoding fixes - Backlight config cleanup - VCN 4.x unified queue cleanup amdkfd: - MMU notifier fixes - Updates for GC 10.3.6 and 10.3.7 - P2P DMA support using dma-buf - Add available memory IOCTL - SDMA 6.0.1 fix - MES fixes - HMM profiler support radeon: - License fix - Backlight config cleanup UAPI: - Add available memory IOCTL to amdkfd Proposed userspace: https://www.mail-archive.com/amd-gfx@lists.freedesktop.org/msg75743.html - HMM profiler support for amdkfd Proposed userspace: https://lists.freedesktop.org/archives/amd-gfx/2022-June/080805.html Ahmad Othman (2): drm/amd/display: Add support for HF-VSIF drm/amd/display: Adding VTEM to dc Alan Liu (1): drm/amdgpu/display/dc: Add ACP_DATA register Alex Deucher (27): drm/amdgpu: update VCN codec support for Yellow Carp drm/amdgpu: simplify amdgpu_device_asic_has_dc_support() drm/amdgpu: convert sienna_cichlid_get_default_config_table_settings() to IP version drm/amdgpu: convert sienna_cichlid_populate_umd_state_clk() to use IP version drm/amdgpu: simplify the logic in amdgpu_device_parse_gpu_info_fw() drm/amdgpu: convert nbio_v2_3_clear_doorbell_interrupt() to IP version drm/amdgpu/gmc11: enable AGP aperture drm/amdgpu/swsmu: add SMU mailbox registers in SMU context drm/amdgpu/swsmu: use new register offsets for smu_cmn.c drm/amdgpu: fix up comment in amdgpu_device_asic_has_dc_support() drm/amdgpu/display: Protect some functions with CONFIG_DRM_AMD_DC_DCN drm/amdgpu/discovery: add comments about VCN instance handling drm/amdgpu/display: make some functions static drm/amdgpu/display: fix DCN3.2 Makefiles for non-x86 drm/amdgpu/soc21: add mode2 asic reset for SMU IP v13.0.4 drm/amdgpu: simplify amdgpu_ucode_get_load_type() drm/amdkfd: fix warning when CONFIG_HSA_AMD_P2P is not set Revert "drm/amdgpu/display: Protect some functions with CONFIG_DRM_AMD_DC_DCN" drm/amdgpu/display: make FP handling in Makefiles consistent drm/radeon: fix incorrrect SPDX-License-Identifiers drm/amdgpu: Adjust logic around GTT size (v3) drm/amdgpu: fix adev variable used in amdgpu_device_gpu_recover() Revert "drm/amdgpu/display: set vblank_disable_immediate for DC" drm/amdgpu/display: reduce stack size in dml32_ModeSupportAndSystemConfigurationFull() drm/amdgpu/display: drop set but unused variable drm/amdgpu: fix documentation warning drm/amdgpu/display: add missing FP_START/END checks dcn32_clk_mgr.c Alexey Kodanev (1): drm/radeon: fix potential buffer overflow in ni_set_mc_special_registers() Alvin (1): drm/amd/display: Don't clear ref_dtbclk value Alvin Lee (7): drm/amd/display: Add missing instance for clock source register drm/amd/display: Implement WM table transfer for DCN32/DCN321 drm/amd/display: Remove W/A for ODM memory pins drm/amd/display: Implement DTBCLK ref switching on dcn32 drm/amd/display: Add debug option for exiting idle optimizations on cursor updates drm/amd/display: Update DPPCLK programming sequence drm/amd/display: Update SW state correctly for FCLK Andrey Grodzovsky (11): Revert "workqueue: remove unused cancel_work()" drm/amdgpu: Cache result of last reset at reset domain level. drm/admgpu: Serialize RAS recovery work directly into reset domain queue.
Re: [PATCH] drm/amd: Add debug mask for subviewport mclk switch
On 7/5/22 16:24, Aurabindo Pillai wrote:
>
>
> On 2022-07-05 15:53, Harry Wentland wrote:
>>
>>
>> On 2022-06-28 17:26, Aurabindo Pillai wrote:
>>> [Why&How]
>>> Expose a new debugfs enum to force a subviewport memory clock switch
>>> to facilitate easy testing.
>>>
>>
>> Does this force a single switch? Or at regular intervals?
>
> If this debug option is set, each time a MCLK switch happens, a SubVP
> sequences shall be initiated. That is, during the MCLK switch the scanout
> shall be from the Subviewport memory. Setting this option doesnt trigger an
> MCLK switch though.
>
>>
>> It would be useful to describe a bit better what it does.
>
> Will keep that in mind, thanks! Unfortunately I merged it already since I got
> the ack last week.
> No worries :)
Harry
>>
>>> Signed-off-by: Aurabindo Pillai
>>
>> Either way, since this is for debug purposes only this is
>> Reviewed-by: Harry Wentland
>>
>> Harry
>>
>>> ---
>>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
>>> drivers/gpu/drm/amd/include/amd_shared.h | 1 +
>>> 2 files changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> index c9145864ed2b..7a034ca95be2 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -1559,6 +1559,9 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>>> if (amdgpu_dc_debug_mask & DC_DISABLE_CLOCK_GATING)
>>> adev->dm.dc->debug.disable_clock_gate = true;
>>> + if (amdgpu_dc_debug_mask & DC_FORCE_SUBVP_MCLK_SWITCH)
>>> + adev->dm.dc->debug.force_subvp_mclk_switch = true;
>>> +
>>> r = dm_dmub_hw_init(adev);
>>> if (r) {
>>> DRM_ERROR("DMUB interface failed to initialize: status=%d\n", r);
>>> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h
>>> b/drivers/gpu/drm/amd/include/amd_shared.h
>>> index bcdf7453a403..b1c55dd7b498 100644
>>> --- a/drivers/gpu/drm/amd/include/amd_shared.h
>>> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
>>> @@ -247,6 +247,7 @@ enum DC_DEBUG_MASK {
>>> DC_DISABLE_DSC = 0x4,
>>> DC_DISABLE_CLOCK_GATING = 0x8,
>>> DC_DISABLE_PSR = 0x10,
>>> + DC_FORCE_SUBVP_MCLK_SWITCH = 0x20,
>>> };
>>> enum amd_dpm_forced_level;
>>
Re: [PATCH] drm/amdkfd: Fix warnings from static analyzer Smatch
On 7/5/22 15:43, Errabolu, Ramesh wrote:
> [AMD Official Use Only - General]
>
> Maira,
>
> Thanks for taking time to review. I understand the request to tag the patch
> as version 2. However I don't follow your comments on "Fixes" block. Looking
> at the git log of the branch I see many "Fixes" block that precede the
> "Signed-off-by" statement.
>
Hi Ramesh,
The canonical patch format is basically, as described by [1]:
...
Signed-off-by: Author
---
V2 -> V3: Removed redundant helper function
V1 -> V2: Cleaned up coding style and addressed review comments
path/to/file | 5+++--
...
So, on your case, we have the commit message describing the warning
reported by Smatch and the error log, then we got the tags. The tags
should be in chronological order, so your tags should be:
Fixes: 40d6aa758b13 ("drm/amdkfd: Extend KFD device topology to surface
peer-to-peer links")
Reported-by: Dan Carpenter
Signed-off-by: Ramesh Errabolu
See that this canonical format reflects the chronological history of the
patch insofar as possible. After the ---, you describe the changes
between v1 and v2 in a small changelog.
The documentation linked in [1] explains this in details. So, some
examples are exposed in [2], [3] and [4].
[1] https://docs.kernel.org/process/submitting-patches.html
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1db88c5343712e411a2dd45375f27c477e33dc07
[3]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=34ad61514c4c3657df21a058f9961c3bb2f84ff2
[4]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d3f2a14b8906df913cb04a706367b012db94a6e8
Best Regards,
- Maíra Canal
> Could you provide an example.
>
> Regards,
> Ramesh
>
> -Original Message-
> From: Maíra Canal
> Sent: Wednesday, June 29, 2022 6:25 PM
> To: Errabolu, Ramesh ;
> amd-gfx@lists.freedesktop.org; dan.carpen...@oracle.com
> Subject: Re: [PATCH] drm/amdkfd: Fix warnings from static analyzer Smatch
>
> [CAUTION: External Email]
>
> Hi Ramesh,
>
> On 6/29/22 14:04, Ramesh Errabolu wrote:
>> The patch fixes couple of warnings, as reported by Smatch a static
>> analyzer.
>>
>> Fixes: 40d6aa758b13 ("drm/amdkfd: Extend KFD device topology to
>> surface peer-to-peer links")>
>> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1428
>> kfd_create_indirect_link_prop() warn: iterator used outside loop: 'cpu_link'
>> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1462
>> kfd_create_indirect_link_prop() error: we previously assumed 'cpu_dev'
>> could be null (see line 1420)
>> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1516 kfd_add_peer_prop()
>> warn: iterator used outside loop: 'iolink3'
>>
>
> Usually, the Fixes tag would go here, after the commit message.
>
>> Signed-off-by: Ramesh Errabolu
>> Reported-by: Dan Carpenter
>> ---
>
> As this is a v2 PATCH, it would be nice to have a small changelog here,
> describing what has changed between the v1 and v2 versions of the patch.
> Also, you can mark the patch as v2 with git send-email by adding the flag
> -v2. More on the canonical patch format can be seen in [1].
>
> [1]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.kernel.org%2Fprocess%2Fsubmitting-patches.html%23the-canonical-patch-format&data=05%7C01%7CRamesh.Errabolu%40amd.com%7Cc54753a9471843cc9d1f08da5a26898d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637921418813227961%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=PAc5A8z2EvJAOUiY378K9XyVBCKewQNsSNCr9pB3Ias%3D&reserved=0
>
> Best Regards,
> - Maíra Canal
>
>> drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 34
>> +++
>> 1 file changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>> index 25990bec600d..ca4825e555b7 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>> @@ -1417,15 +1417,15 @@ static int
>> kfd_create_indirect_link_prop(struct kfd_topology_device *kdev, int g
>>
>> /* find CPU <--> CPU links */
>> cpu_dev = kfd_topology_device_by_proximity_domain(i);
>> - if (cpu_dev) {
>> - list_for_each_entry(cpu_link,
>> - &cpu_dev->io_link_props, list) {
>> - if (cpu_link->node_to == gpu_link->node_to)
>> - break;
>> - }
>> - }
>> + if (!cpu_dev)
>> + continue;
>> +
>> + list_for_each_entry(cpu_link, &cpu_dev->io_link_props, list)
>> + if (cpu_link->node_to == gpu_link->node_to)
>> + break;
>>
>> - if (cpu_link->node_to != gpu_link->node_to)
>> + /* Ensures we didn't exit fr
Re: [PATCH] drm/amd: Add debug mask for subviewport mclk switch
On 2022-07-05 15:53, Harry Wentland wrote:
On 2022-06-28 17:26, Aurabindo Pillai wrote:
[Why&How]
Expose a new debugfs enum to force a subviewport memory clock switch
to facilitate easy testing.
Does this force a single switch? Or at regular intervals?
If this debug option is set, each time a MCLK switch happens, a SubVP
sequences shall be initiated. That is, during the MCLK switch the
scanout shall be from the Subviewport memory. Setting this option doesnt
trigger an MCLK switch though.
It would be useful to describe a bit better what it does.
Will keep that in mind, thanks! Unfortunately I merged it already since
I got the ack last week.
Signed-off-by: Aurabindo Pillai
Either way, since this is for debug purposes only this is
Reviewed-by: Harry Wentland
Harry
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
drivers/gpu/drm/amd/include/amd_shared.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c9145864ed2b..7a034ca95be2 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1559,6 +1559,9 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
if (amdgpu_dc_debug_mask & DC_DISABLE_CLOCK_GATING)
adev->dm.dc->debug.disable_clock_gate = true;
+ if (amdgpu_dc_debug_mask & DC_FORCE_SUBVP_MCLK_SWITCH)
+ adev->dm.dc->debug.force_subvp_mclk_switch = true;
+
r = dm_dmub_hw_init(adev);
if (r) {
DRM_ERROR("DMUB interface failed to initialize: status=%d\n",
r);
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h
b/drivers/gpu/drm/amd/include/amd_shared.h
index bcdf7453a403..b1c55dd7b498 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -247,6 +247,7 @@ enum DC_DEBUG_MASK {
DC_DISABLE_DSC = 0x4,
DC_DISABLE_CLOCK_GATING = 0x8,
DC_DISABLE_PSR = 0x10,
+ DC_FORCE_SUBVP_MCLK_SWITCH = 0x20,
};
enum amd_dpm_forced_level;
Re: [PATCH] drm/amd: Add debug mask for subviewport mclk switch
On 2022-06-28 17:26, Aurabindo Pillai wrote:
> [Why&How]
> Expose a new debugfs enum to force a subviewport memory clock switch
> to facilitate easy testing.
>
Does this force a single switch? Or at regular intervals?
It would be useful to describe a bit better what it does.
> Signed-off-by: Aurabindo Pillai
Either way, since this is for debug purposes only this is
Reviewed-by: Harry Wentland
Harry
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
> drivers/gpu/drm/amd/include/amd_shared.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index c9145864ed2b..7a034ca95be2 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1559,6 +1559,9 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
> if (amdgpu_dc_debug_mask & DC_DISABLE_CLOCK_GATING)
> adev->dm.dc->debug.disable_clock_gate = true;
>
> + if (amdgpu_dc_debug_mask & DC_FORCE_SUBVP_MCLK_SWITCH)
> + adev->dm.dc->debug.force_subvp_mclk_switch = true;
> +
> r = dm_dmub_hw_init(adev);
> if (r) {
> DRM_ERROR("DMUB interface failed to initialize: status=%d\n",
> r);
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h
> b/drivers/gpu/drm/amd/include/amd_shared.h
> index bcdf7453a403..b1c55dd7b498 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -247,6 +247,7 @@ enum DC_DEBUG_MASK {
> DC_DISABLE_DSC = 0x4,
> DC_DISABLE_CLOCK_GATING = 0x8,
> DC_DISABLE_PSR = 0x10,
> + DC_FORCE_SUBVP_MCLK_SWITCH = 0x20,
> };
>
> enum amd_dpm_forced_level;
Re: [PATCH 1/1] drm/amdkfd: Remove queue sysfs and doorbell after unmapping
On 2022-07-05 15:12, Felix Kuehling
wrote:
On
2022-07-05 15:02, philip yang wrote:
On 2022-07-05 14:48, Felix Kuehling wrote:
On 2022-06-10 21:03, Philip Yang wrote:
If destroying/unmapping queue failed,
application may destroy queue
again, cause below kernel warning backtrace.
For outstanding queues, either applications forget to
destroy or failed
to destroy, kfd_process_notifier_release will remove queue
sysfs
objects, kfd_process_wq_release will free queue doorbell.
refcount_t: underflow; use-after-free.
WARNING: CPU: 7 PID: 3053 at lib/refcount.c:28
Call Trace:
kobject_put+0xd6/0x1a0
kfd_procfs_del_queue+0x27/0x30 [amdgpu]
pqm_destroy_queue+0xeb/0x240 [amdgpu]
kfd_ioctl_destroy_queue+0x32/0x70 [amdgpu]
kfd_ioctl+0x27d/0x500 [amdgpu]
do_syscall_64+0x35/0x80
WARNING: CPU: 2 PID: 3053 at
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:400
Call Trace:
deallocate_doorbell.isra.0+0x39/0x40 [amdgpu]
destroy_queue_cpsch+0xb3/0x270 [amdgpu]
pqm_destroy_queue+0x108/0x240 [amdgpu]
kfd_ioctl_destroy_queue+0x32/0x70 [amdgpu]
kfd_ioctl+0x27d/0x500 [amdgpu]
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 4
++--
drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 2
+-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git
a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index e1797657b04c..1c519514ca1a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1876,8 +1876,6 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
mqd_mgr =
dqm->mqd_mgrs[get_mqd_type_from_queue_type(
q->properties.type)];
- deallocate_doorbell(qpd, q);
-
if ((q->properties.type == KFD_QUEUE_TYPE_SDMA) ||
(q->properties.type ==
KFD_QUEUE_TYPE_SDMA_XGMI)) {
deallocate_sdma_queue(dqm, q);
@@ -1898,6 +1896,8 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
}
}
+ deallocate_doorbell(qpd, q);
+
I'm not sure what this change is meant to do. I don't see an
early return in this function, so deallocate_doorbell will
always be executed either way.
If app destroy queue deallocate_doorbell, but then unmap queue
failed, app will destroy queue again when app exit,
deallocate_doorbell second time will trigger the WARN backtrace.
I get that. But even with your change, deallocate_doorbell will
still be executed if the queue unmap fails because there is no
early return or "goto error".
Yes, that was fixed in patch v2. [PATCH v2 2/2] drm/amdkfd: Free
queue after unmap queue success
As queue_count and q->list is used to
alloc ring buf in execute_queues_cpsch, so this change causes
regression on gfx9, I will submit new patch to handle unmap
failed case with MES and fix those issues.
I think the intention of the code was to treat HWS in a way that
does not prevent queue destruction. Basically, there is not point
reporting HWS errors to the application because the application
cannot do anything about them anyway. Eventually it will cause a
GPU reset and the application will be killed. If you look at how
-ETIME is handled in pqm_destroy_queue, you see that it finish
Re: [PATCH 1/1] drm/amdkfd: Remove queue sysfs and doorbell after unmapping
On 2022-07-05 15:02, philip yang wrote:
On 2022-07-05 14:48, Felix Kuehling wrote:
On 2022-06-10 21:03, Philip Yang wrote:
If destroying/unmapping queue failed, application may destroy queue
again, cause below kernel warning backtrace.
For outstanding queues, either applications forget to destroy or failed
to destroy, kfd_process_notifier_release will remove queue sysfs
objects, kfd_process_wq_release will free queue doorbell.
refcount_t: underflow; use-after-free.
WARNING: CPU: 7 PID: 3053 at lib/refcount.c:28
Call Trace:
kobject_put+0xd6/0x1a0
kfd_procfs_del_queue+0x27/0x30 [amdgpu]
pqm_destroy_queue+0xeb/0x240 [amdgpu]
kfd_ioctl_destroy_queue+0x32/0x70 [amdgpu]
kfd_ioctl+0x27d/0x500 [amdgpu]
do_syscall_64+0x35/0x80
WARNING: CPU: 2 PID: 3053 at
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:400
Call Trace:
deallocate_doorbell.isra.0+0x39/0x40 [amdgpu]
destroy_queue_cpsch+0xb3/0x270 [amdgpu]
pqm_destroy_queue+0x108/0x240 [amdgpu]
kfd_ioctl_destroy_queue+0x32/0x70 [amdgpu]
kfd_ioctl+0x27d/0x500 [amdgpu]
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 4 ++--
drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index e1797657b04c..1c519514ca1a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1876,8 +1876,6 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
q->properties.type)];
- deallocate_doorbell(qpd, q);
-
if ((q->properties.type == KFD_QUEUE_TYPE_SDMA) ||
(q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)) {
deallocate_sdma_queue(dqm, q);
@@ -1898,6 +1896,8 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
}
}
+ deallocate_doorbell(qpd, q);
+
I'm not sure what this change is meant to do. I don't see an early
return in this function, so deallocate_doorbell will always be
executed either way.
If app destroy queue deallocate_doorbell, but then unmap queue failed,
app will destroy queue again when app exit, deallocate_doorbell second
time will trigger the WARN backtrace.
I get that. But even with your change, deallocate_doorbell will still be
executed if the queue unmap fails because there is no early return or
"goto error".
As queue_count and q->list is used to alloc ring buf in
execute_queues_cpsch, so this change causes regression on gfx9, I will
submit new patch to handle unmap failed case with MES and fix those
issues.
I think the intention of the code was to treat HWS in a way that does
not prevent queue destruction. Basically, there is not point reporting
HWS errors to the application because the application cannot do anything
about them anyway. Eventually it will cause a GPU reset and the
application will be killed. If you look at how -ETIME is handled in
pqm_destroy_queue, you see that it finishes the job regardless.
However, this has always been somewhat inconsistent. With MES maybe it's
getting worse because there may be other error conditions we didn't hit
before. Are you seeing those backtraces on a GPU with MES by any chance?
Regards,
Felix
Regards,
Philip
/*
* Unconditionally decrement this counter, regardless of the
queue's
* type
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index dc00484ff484..99f2a6412201 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -419,7 +419,6 @@ int pqm_destroy_queue(struct
process_queue_manager *pqm, unsigned int qid)
}
if (pqn->q) {
- kfd_procfs_del_queue(pqn->q);
dqm = pqn->q->device->dqm;
retval = dqm->ops.destroy_queue(dqm, &pdd->qpd, pqn->q);
if (retval) {
@@ -439,6 +438,7 @@ int pqm_destroy_queue(struct
process_queue_manager *pqm, unsigned int qid)
if (dev->shared_resources.enable_mes)
amdgpu_amdkfd_free_gtt_mem(dev->adev,
pqn->q->gang_ctx_bo);
+ kfd_procfs_del_queue(pqn->q);
This part makes sense.
Regards,
Felix
uninit_queue(pqn->q);
}
Re: [PATCH 1/1] drm/amdkfd: Remove queue sysfs and doorbell after unmapping
On 2022-07-05 14:48, Felix Kuehling
wrote:
On
2022-06-10 21:03, Philip Yang wrote:
If destroying/unmapping queue failed,
application may destroy queue
again, cause below kernel warning backtrace.
For outstanding queues, either applications forget to destroy or
failed
to destroy, kfd_process_notifier_release will remove queue sysfs
objects, kfd_process_wq_release will free queue doorbell.
refcount_t: underflow; use-after-free.
WARNING: CPU: 7 PID: 3053 at lib/refcount.c:28
Call Trace:
kobject_put+0xd6/0x1a0
kfd_procfs_del_queue+0x27/0x30 [amdgpu]
pqm_destroy_queue+0xeb/0x240 [amdgpu]
kfd_ioctl_destroy_queue+0x32/0x70 [amdgpu]
kfd_ioctl+0x27d/0x500 [amdgpu]
do_syscall_64+0x35/0x80
WARNING: CPU: 2 PID: 3053 at
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:400
Call Trace:
deallocate_doorbell.isra.0+0x39/0x40 [amdgpu]
destroy_queue_cpsch+0xb3/0x270 [amdgpu]
pqm_destroy_queue+0x108/0x240 [amdgpu]
kfd_ioctl_destroy_queue+0x32/0x70 [amdgpu]
kfd_ioctl+0x27d/0x500 [amdgpu]
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 4
++--
drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git
a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index e1797657b04c..1c519514ca1a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1876,8 +1876,6 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
q->properties.type)];
- deallocate_doorbell(qpd, q);
-
if ((q->properties.type == KFD_QUEUE_TYPE_SDMA) ||
(q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)) {
deallocate_sdma_queue(dqm, q);
@@ -1898,6 +1896,8 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
}
}
+ deallocate_doorbell(qpd, q);
+
I'm not sure what this change is meant to do. I don't see an early
return in this function, so deallocate_doorbell will always be
executed either way.
If app destroy queue deallocate_doorbell, but then unmap queue
failed, app will destroy queue again when app exit,
deallocate_doorbell second time will trigger the WARN backtrace.
As queue_count and q->list is used to alloc ring buf in
execute_queues_cpsch, so this change causes regression on gfx9, I
will submit new patch to handle unmap failed case with MES and fix
those issues.
Regards,
Philip
/*
* Unconditionally decrement this counter, regardless of
the queue's
* type
diff --git
a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index dc00484ff484..99f2a6412201 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -419,7 +419,6 @@ int pqm_destroy_queue(struct
process_queue_manager *pqm, unsigned int qid)
}
if (pqn->q) {
- kfd_procfs_del_queue(pqn->q);
dqm = pqn->q->device->dqm;
retval = dqm->ops.destroy_queue(dqm,
&pdd->qpd, pqn->q);
if (retval) {
@@ -439,6 +438,7 @@ int pqm_destroy_queue(struct
process_queue_manager *pqm, unsigned int qid)
if (dev->shared_resources.enable_mes)
amdgpu_amdkfd_free_gtt_mem(dev->adev,
pqn->q->gang_ctx_bo);
+ kfd_procfs_del_queue(pqn->q);
This part makes sense.
Re: [PATCH 1/1] drm/amdkfd: Remove queue sysfs and doorbell after unmapping
On 2022-06-10 21:03, Philip Yang wrote:
If destroying/unmapping queue failed, application may destroy queue
again, cause below kernel warning backtrace.
For outstanding queues, either applications forget to destroy or failed
to destroy, kfd_process_notifier_release will remove queue sysfs
objects, kfd_process_wq_release will free queue doorbell.
refcount_t: underflow; use-after-free.
WARNING: CPU: 7 PID: 3053 at lib/refcount.c:28
Call Trace:
kobject_put+0xd6/0x1a0
kfd_procfs_del_queue+0x27/0x30 [amdgpu]
pqm_destroy_queue+0xeb/0x240 [amdgpu]
kfd_ioctl_destroy_queue+0x32/0x70 [amdgpu]
kfd_ioctl+0x27d/0x500 [amdgpu]
do_syscall_64+0x35/0x80
WARNING: CPU: 2 PID: 3053 at
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device_queue_manager.c:400
Call Trace:
deallocate_doorbell.isra.0+0x39/0x40 [amdgpu]
destroy_queue_cpsch+0xb3/0x270 [amdgpu]
pqm_destroy_queue+0x108/0x240 [amdgpu]
kfd_ioctl_destroy_queue+0x32/0x70 [amdgpu]
kfd_ioctl+0x27d/0x500 [amdgpu]
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 4 ++--
drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index e1797657b04c..1c519514ca1a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1876,8 +1876,6 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
q->properties.type)];
- deallocate_doorbell(qpd, q);
-
if ((q->properties.type == KFD_QUEUE_TYPE_SDMA) ||
(q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)) {
deallocate_sdma_queue(dqm, q);
@@ -1898,6 +1896,8 @@ static int destroy_queue_cpsch(struct
device_queue_manager *dqm,
}
}
+ deallocate_doorbell(qpd, q);
+
I'm not sure what this change is meant to do. I don't see an early
return in this function, so deallocate_doorbell will always be executed
either way.
/*
* Unconditionally decrement this counter, regardless of the queue's
* type
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index dc00484ff484..99f2a6412201 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -419,7 +419,6 @@ int pqm_destroy_queue(struct process_queue_manager *pqm,
unsigned int qid)
}
if (pqn->q) {
- kfd_procfs_del_queue(pqn->q);
dqm = pqn->q->device->dqm;
retval = dqm->ops.destroy_queue(dqm, &pdd->qpd, pqn->q);
if (retval) {
@@ -439,6 +438,7 @@ int pqm_destroy_queue(struct process_queue_manager *pqm,
unsigned int qid)
if (dev->shared_resources.enable_mes)
amdgpu_amdkfd_free_gtt_mem(dev->adev,
pqn->q->gang_ctx_bo);
+ kfd_procfs_del_queue(pqn->q);
This part makes sense.
Regards,
Felix
uninit_queue(pqn->q);
}
RE: [PATCH] drm/amdkfd: Fix warnings from static analyzer Smatch
[AMD Official Use Only - General]
Maira,
Thanks for taking time to review. I understand the request to tag the patch as
version 2. However I don't follow your comments on "Fixes" block. Looking at
the git log of the branch I see many "Fixes" block that precede the
"Signed-off-by" statement.
Could you provide an example.
Regards,
Ramesh
-Original Message-
From: Maíra Canal
Sent: Wednesday, June 29, 2022 6:25 PM
To: Errabolu, Ramesh ; amd-gfx@lists.freedesktop.org;
dan.carpen...@oracle.com
Subject: Re: [PATCH] drm/amdkfd: Fix warnings from static analyzer Smatch
[CAUTION: External Email]
Hi Ramesh,
On 6/29/22 14:04, Ramesh Errabolu wrote:
> The patch fixes couple of warnings, as reported by Smatch a static
> analyzer.
>
> Fixes: 40d6aa758b13 ("drm/amdkfd: Extend KFD device topology to
> surface peer-to-peer links")>
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1428
> kfd_create_indirect_link_prop() warn: iterator used outside loop: 'cpu_link'
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1462
> kfd_create_indirect_link_prop() error: we previously assumed 'cpu_dev'
> could be null (see line 1420)
> drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1516 kfd_add_peer_prop()
> warn: iterator used outside loop: 'iolink3'
>
Usually, the Fixes tag would go here, after the commit message.
> Signed-off-by: Ramesh Errabolu
> Reported-by: Dan Carpenter
> ---
As this is a v2 PATCH, it would be nice to have a small changelog here,
describing what has changed between the v1 and v2 versions of the patch.
Also, you can mark the patch as v2 with git send-email by adding the flag -v2.
More on the canonical patch format can be seen in [1].
[1]
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.kernel.org%2Fprocess%2Fsubmitting-patches.html%23the-canonical-patch-format&data=05%7C01%7CRamesh.Errabolu%40amd.com%7Cc54753a9471843cc9d1f08da5a26898d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637921418813227961%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=PAc5A8z2EvJAOUiY378K9XyVBCKewQNsSNCr9pB3Ias%3D&reserved=0
Best Regards,
- Maíra Canal
> drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 34
> +++
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> index 25990bec600d..ca4825e555b7 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> @@ -1417,15 +1417,15 @@ static int
> kfd_create_indirect_link_prop(struct kfd_topology_device *kdev, int g
>
> /* find CPU <--> CPU links */
> cpu_dev = kfd_topology_device_by_proximity_domain(i);
> - if (cpu_dev) {
> - list_for_each_entry(cpu_link,
> - &cpu_dev->io_link_props, list) {
> - if (cpu_link->node_to == gpu_link->node_to)
> - break;
> - }
> - }
> + if (!cpu_dev)
> + continue;
> +
> + list_for_each_entry(cpu_link, &cpu_dev->io_link_props, list)
> + if (cpu_link->node_to == gpu_link->node_to)
> + break;
>
> - if (cpu_link->node_to != gpu_link->node_to)
> + /* Ensures we didn't exit from list search with no hits */
> + if (list_entry_is_head(cpu_link,
> + &cpu_dev->io_link_props, list))
> return -ENOMEM;
>
> /* CPU <--> CPU <--> GPU, GPU node*/ @@ -1510,16
> +1510,16 @@ static int kfd_add_peer_prop(struct kfd_topology_device *kdev,
> cpu_dev =
> kfd_topology_device_by_proximity_domain(iolink1->node_to);
> if (cpu_dev) {
> list_for_each_entry(iolink3, &cpu_dev->io_link_props,
> list)
> - if (iolink3->node_to == iolink2->node_to)
> + if (iolink3->node_to == iolink2->node_to) {
> + props->weight += iolink3->weight;
> + props->min_latency +=
> iolink3->min_latency;
> + props->max_latency +=
> iolink3->max_latency;
> + props->min_bandwidth =
> min(props->min_bandwidth,
> +
> iolink3->min_bandwidth);
> + props->max_bandwidth =
> min(props->max_bandwidth,
> +
> + iolink3->max_bandwidth);
> break;
> -
> - props->weight += iolink3->weight;
> - props->min_latency += iolink3->min_latency;
> - props->max_latency +=
Re: [PATCH 2/3] drm/amdkfd: track unified memory reservation with xnack off
On 2022-07-05 12:09, philip yang wrote:
On 2022-06-27 20:23, Alex Sierra wrote:
[WHY]
Unified memory with xnack off should be tracked, as userptr mappings
and legacy allocations do. To avoid oversuscribe system memory when
xnack off.
I think this also apply to XNACK ON (remove p->xnack_enabled check),
to avoid oversubscribe system memory OOM killer, if we don't account
swap space as it will degrade performance.
That's not the GPU driver's job. If applications allocate too much
memory, that's the application's fault. With XNACK ON, the driver
doesn't need to make memory resident before the GPU starts executing,
and the GPU can continue executing when unused memory is swapped out.
Allowing memory overcommitment is one of the desirable features enabled
by XNACK ON. Therefore we don't need to artificially limit the amount of
memory that can be GPU mapped in this mode.
Regards,
Felix
[How]
Exposing functions reserve_mem_limit and unreserve_mem_limit to SVM
API and call them on every prange creation and free.
Signed-off-by: Alex Sierra
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 4 ++
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 25
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 58 +--
3 files changed, 58 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index b25b41f50213..e6244182a3a4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -305,6 +305,10 @@ bool amdgpu_amdkfd_bo_mapped_to_dev(struct amdgpu_device
*adev, struct kgd_mem *
void amdgpu_amdkfd_block_mmu_notifications(void *p);
int amdgpu_amdkfd_criu_resume(void *p);
bool amdgpu_amdkfd_ras_query_utcl2_poison_status(struct amdgpu_device *adev);
+int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
+ uint64_t size, u32 alloc_flag);
+void amdgpu_amdkfd_unreserve_mem_limit(struct amdgpu_device *adev,
+ uint64_t size, u32 alloc_flag);
#if IS_ENABLED(CONFIG_HSA_AMD)
void amdgpu_amdkfd_gpuvm_init_mem_limits(void);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 9142f6cc3f4d..9719577ecc6d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -128,7 +128,7 @@ void amdgpu_amdkfd_reserve_system_mem(uint64_t size)
*
* Return: returns -ENOMEM in case of error, ZERO otherwise
*/
-static int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
+int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
uint64_t size, u32 alloc_flag)
{
uint64_t reserved_for_pt =
@@ -168,7 +168,7 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
amdgpu_device *adev,
kfd_mem_limit.max_system_mem_limit && !no_system_mem_limit) ||
(kfd_mem_limit.ttm_mem_used + ttm_mem_needed >
kfd_mem_limit.max_ttm_mem_limit) ||
- (adev->kfd.vram_used + vram_needed >
+ (adev && adev->kfd.vram_used + vram_needed >
adev->gmc.real_vram_size -
atomic64_read(&adev->vram_pin_size) -
reserved_for_pt)) {
@@ -179,7 +179,10 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
amdgpu_device *adev,
/* Update memory accounting by decreasing available system
* memory, TTM memory and GPU memory as computed above
*/
- adev->kfd.vram_used += vram_needed;
+ WARN_ONCE(vram_needed && !adev,
+ "adev reference can't be null when vram is used");
+ if (adev)
+ adev->kfd.vram_used += vram_needed;
kfd_mem_limit.system_mem_used += system_mem_needed;
kfd_mem_limit.ttm_mem_used += ttm_mem_needed;
@@ -188,7 +191,7 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
return ret;
}
-static void unreserve_mem_limit(struct amdgpu_device *adev,
+void amdgpu_amdkfd_unreserve_mem_limit(struct amdgpu_device *adev,
uint64_t size, u32 alloc_flag)
{
spin_lock(&kfd_mem_limit.mem_limit_lock);
@@ -197,7 +200,10 @@ static void unreserve_mem_limit(struct amdgpu_device *adev,
kfd_mem_limit.system_mem_used -= size;
kfd_mem_limit.ttm_mem_used -= size;
} else if (alloc_flag & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
- adev->kfd.vram_used -= ALIGN(size, VRAM_ALLOCATION_ALIGN);
+ WARN_ONCE(!adev,
+ "adev reference can't be null when alloc mem flags vram is
set");
+ if (adev)
+ adev->kfd.vram_used -= ALIGN(size,
VRAM_ALLOCATION_ALIGN);
} else if (alloc_flag & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) {
kfd_mem_limit.system_mem_used -= size;
} else if (!(alloc_flag &
@@ -206,11 +212,8 @@ static void unreserve_mem_limit(struct
Re: [PATCH 3/3] drm/amdgpu: add debugfs for kfd system and ttm mem used
On 2022-06-27 20:23, Alex Sierra wrote:
This keeps track of kfd system mem used and kfd ttm mem used.
We could also dump vram used by checking topology_device_list per
dev->vram_used, but it can add later if needed.
Reviewed-by: Philip Yang
Signed-off-by: Alex Sierra
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 3 +++
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 19 +++
drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c | 2 ++
3 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index e6244182a3a4..53cdf7f00b3f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -172,6 +172,9 @@ int amdgpu_queue_mask_bit_to_set_resource_bit(struct amdgpu_device *adev,
struct amdgpu_amdkfd_fence *amdgpu_amdkfd_fence_create(u64 context,
struct mm_struct *mm,
struct svm_range_bo *svm_bo);
+#if defined(CONFIG_DEBUG_FS)
+int kfd_debugfs_kfd_mem_limits(struct seq_file *m, void *data);
+#endif
#if IS_ENABLED(CONFIG_HSA_AMD)
bool amdkfd_fence_check_mm(struct dma_fence *f, struct mm_struct *mm);
struct amdgpu_amdkfd_fence *to_amdgpu_amdkfd_fence(struct dma_fence *f);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 9719577ecc6d..c48557b683c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -2935,3 +2935,22 @@ bool amdgpu_amdkfd_bo_mapped_to_dev(struct amdgpu_device *adev, struct kgd_mem *
}
return false;
}
+
+#if defined(CONFIG_DEBUG_FS)
+
+int kfd_debugfs_kfd_mem_limits(struct seq_file *m, void *data)
+{
+
+ spin_lock(&kfd_mem_limit.mem_limit_lock);
+ seq_printf(m, "System mem used %lldM out of %lluM\n",
+ (kfd_mem_limit.system_mem_used >> 20),
+ (kfd_mem_limit.max_system_mem_limit >> 20));
+ seq_printf(m, "TTM mem used %lldM out of %lluM\n",
+ (kfd_mem_limit.ttm_mem_used >> 20),
+ (kfd_mem_limit.max_ttm_mem_limit >> 20));
+ spin_unlock(&kfd_mem_limit.mem_limit_lock);
+
+ return 0;
+}
+
+#endif
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c b/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
index 581c3a30fee1..ad5a40a685ac 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
@@ -101,6 +101,8 @@ void kfd_debugfs_init(void)
kfd_debugfs_rls_by_device, &kfd_debugfs_fops);
debugfs_create_file("hang_hws", S_IFREG | 0200, debugfs_root,
kfd_debugfs_hang_hws_read, &kfd_debugfs_hang_hws_fops);
+ debugfs_create_file("mem_limit", S_IFREG | 0200, debugfs_root,
+ kfd_debugfs_kfd_mem_limits, &kfd_debugfs_fops);
}
void kfd_debugfs_fini(void)
Re: [PATCH 2/3] drm/amdkfd: track unified memory reservation with xnack off
On 2022-06-27 20:23, Alex Sierra wrote:
[WHY]
Unified memory with xnack off should be tracked, as userptr mappings
and legacy allocations do. To avoid oversuscribe system memory when
xnack off.
I think this also apply to XNACK ON (remove p->xnack_enabled
check), to avoid oversubscribe system memory OOM killer, if we don't
account swap space as it will degrade performance.
[How]
Exposing functions reserve_mem_limit and unreserve_mem_limit to SVM
API and call them on every prange creation and free.
Signed-off-by: Alex Sierra
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 4 ++
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 25
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 58 +--
3 files changed, 58 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index b25b41f50213..e6244182a3a4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -305,6 +305,10 @@ bool amdgpu_amdkfd_bo_mapped_to_dev(struct amdgpu_device *adev, struct kgd_mem *
void amdgpu_amdkfd_block_mmu_notifications(void *p);
int amdgpu_amdkfd_criu_resume(void *p);
bool amdgpu_amdkfd_ras_query_utcl2_poison_status(struct amdgpu_device *adev);
+int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
+ uint64_t size, u32 alloc_flag);
+void amdgpu_amdkfd_unreserve_mem_limit(struct amdgpu_device *adev,
+ uint64_t size, u32 alloc_flag);
#if IS_ENABLED(CONFIG_HSA_AMD)
void amdgpu_amdkfd_gpuvm_init_mem_limits(void);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 9142f6cc3f4d..9719577ecc6d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -128,7 +128,7 @@ void amdgpu_amdkfd_reserve_system_mem(uint64_t size)
*
* Return: returns -ENOMEM in case of error, ZERO otherwise
*/
-static int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
+int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
uint64_t size, u32 alloc_flag)
{
uint64_t reserved_for_pt =
@@ -168,7 +168,7 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
kfd_mem_limit.max_system_mem_limit && !no_system_mem_limit) ||
(kfd_mem_limit.ttm_mem_used + ttm_mem_needed >
kfd_mem_limit.max_ttm_mem_limit) ||
- (adev->kfd.vram_used + vram_needed >
+ (adev && adev->kfd.vram_used + vram_needed >
adev->gmc.real_vram_size -
atomic64_read(&adev->vram_pin_size) -
reserved_for_pt)) {
@@ -179,7 +179,10 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
/* Update memory accounting by decreasing available system
* memory, TTM memory and GPU memory as computed above
*/
- adev->kfd.vram_used += vram_needed;
+ WARN_ONCE(vram_needed && !adev,
+ "adev reference can't be null when vram is used");
+ if (adev)
+ adev->kfd.vram_used += vram_needed;
kfd_mem_limit.system_mem_used += system_mem_needed;
kfd_mem_limit.ttm_mem_used += ttm_mem_needed;
@@ -188,7 +191,7 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev,
return ret;
}
-static void unreserve_mem_limit(struct amdgpu_device *adev,
+void amdgpu_amdkfd_unreserve_mem_limit(struct amdgpu_device *adev,
uint64_t size, u32 alloc_flag)
{
spin_lock(&kfd_mem_limit.mem_limit_lock);
@@ -197,7 +200,10 @@ static void unreserve_mem_limit(struct amdgpu_device *adev,
kfd_mem_limit.system_mem_used -= size;
kfd_mem_limit.ttm_mem_used -= size;
} else if (alloc_flag & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) {
- adev->kfd.vram_used -= ALIGN(size, VRAM_ALLOCATION_ALIGN);
+ WARN_ONCE(!adev,
+ "adev reference can't be null when alloc mem flags vram is set");
+ if (adev)
+ adev->kfd.vram_used -= ALIGN(size, VRAM_ALLOCATION_ALIGN);
} else if (alloc_flag & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) {
kfd_mem_limit.system_mem_used -= size;
} else if (!(alloc_flag &
@@ -206,11 +212,8 @@ static void unreserve_mem_limit(struct amdgpu_device *adev,
pr_err("%s: Invalid BO type %#x\n", __func__, alloc_flag);
goto release;
}
-
- WARN_ONCE(adev->kfd.vram_used < 0,
+ WARN_ONCE(adev && adev->kfd.vram_used < 0,
"KFD VRAM memory accounting unbalanced");
- WARN_ONCE(kfd_mem_limit.ttm_mem_used < 0,
- "KFD TTM memory accounting unbalanced");
WARN_ONCE(kfd_mem_limit.system_mem_used < 0,
"KFD system memory accounting unbalanced");
@@ -224,7 +227,7 @@ void amdgpu_amdkfd_release_notify(struct amdgpu_bo *bo)
u32 alloc_flags = bo->kfd_bo->alloc_flags;
u64 size = amdgpu_bo_size(bo);
- unreserve_mem_limit(adev, size, alloc_flags);
+ amdgpu_amdkfd_unreserve_mem_limit(adev, size, alloc_flags);
kfree(bo->kfd_bo);
}
@@ -1806,7 +1809,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
/* Don't unreserve system mem limit twice */
Re: [PATCH RESEND V2 1/3] drm/amdgpu: fix checkpatch warnings
On 7/5/22 4:51 PM, Christian König wrote:
>
>
> Am 04.07.22 um 15:54 schrieb Vijendar Mukunda:
>> From: vijendar
>>
>> Fixed below checkpatch warnings and errors
>>
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:131: CHECK: Comparison to NULL
>> could be written "apd"
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:150: CHECK: Comparison to NULL
>> could be written "apd"
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:196: CHECK: Prefer kernel type
>> 'u64' over 'uint64_t'
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:224: CHECK: Please don't use
>> multiple blank lines
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:226: CHECK: Comparison to NULL
>> could be written "!adev->acp.acp_genpd"
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:233: CHECK: Please don't use
>> multiple blank lines
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:239: CHECK: Alignment should
>> match open parenthesis
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:241: CHECK: Comparison to NULL
>> could be written "!adev->acp.acp_cell"
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:247: CHECK: Comparison to NULL
>> could be written "!adev->acp.acp_res"
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:253: CHECK: Comparison to NULL
>> could be written "!i2s_pdata"
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:350: CHECK: Alignment should
>> match open parenthesis
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:550: ERROR: that open brace {
>> should be on the previous line
>>
>> Signed-off-by: Vijendar Mukunda
>> Reviewed-by: Alex Deucher
>>
>> changes since v1:
>> Modified commit label as drm/amdgpu
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 27 +
>> 1 file changed, 10 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
>> index cc9c9f8b23b2..ba1605ff521f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
>> @@ -128,7 +128,7 @@ static int acp_poweroff(struct generic_pm_domain
>> *genpd)
>> struct amdgpu_device *adev;
>> apd = container_of(genpd, struct acp_pm_domain, gpd);
>> - if (apd != NULL) {
>> + if (apd) {
>
> Well that's still not correct. The variable apd is compute by an upcast
> of the genpd parameter.
>
> Since that upcast never returns a NULL value (gpd is not the first
> member of the structure) this whole check is completely nonsense.
>
> I strongly suggest to just remove most of those NULL checks.
Will remove Null checks and post the new patch.
-
Vijendar
>
> Only the ones directly after memory allocation really make sense.
>
> Regards,
> Christian.
>
>> adev = apd->adev;
>> /* call smu to POWER GATE ACP block
>> * smu will
>> @@ -147,7 +147,7 @@ static int acp_poweron(struct generic_pm_domain
>> *genpd)
>> struct amdgpu_device *adev;
>> apd = container_of(genpd, struct acp_pm_domain, gpd);
>> - if (apd != NULL) {
>> + if (apd) {
>> adev = apd->adev;
>> /* call smu to UNGATE ACP block
>> * smu will
>> @@ -193,7 +193,7 @@ static int acp_genpd_remove_device(struct device
>> *dev, void *data)
>> static int acp_hw_init(void *handle)
>> {
>> int r;
>> - uint64_t acp_base;
>> + u64 acp_base;
>> u32 val = 0;
>> u32 count = 0;
>> struct i2s_platform_data *i2s_pdata = NULL;
>> @@ -220,37 +220,32 @@ static int acp_hw_init(void *handle)
>> return -EINVAL;
>> acp_base = adev->rmmio_base;
>> -
>> -
>> adev->acp.acp_genpd = kzalloc(sizeof(struct acp_pm_domain),
>> GFP_KERNEL);
>> - if (adev->acp.acp_genpd == NULL)
>> + if (!adev->acp.acp_genpd)
>> return -ENOMEM;
>> adev->acp.acp_genpd->gpd.name = "ACP_AUDIO";
>> adev->acp.acp_genpd->gpd.power_off = acp_poweroff;
>> adev->acp.acp_genpd->gpd.power_on = acp_poweron;
>> -
>> -
>> adev->acp.acp_genpd->adev = adev;
>> pm_genpd_init(&adev->acp.acp_genpd->gpd, NULL, false);
>> - adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
>> - GFP_KERNEL);
>> + adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
>> GFP_KERNEL);
>> - if (adev->acp.acp_cell == NULL) {
>> + if (!adev->acp.acp_cell) {
>> r = -ENOMEM;
>> goto failure;
>> }
>> adev->acp.acp_res = kcalloc(5, sizeof(struct resource),
>> GFP_KERNEL);
>> - if (adev->acp.acp_res == NULL) {
>> + if (!adev->acp.acp_res) {
>> r = -ENOMEM;
>> goto failure;
>> }
>> i2s_pdata = kcalloc(3, sizeof(struct i2s_platform_data),
>> GFP_KERNEL);
>> - if (i2s_pdata == NULL) {
>> + if (!i2s_pdata) {
>> r = -ENOMEM;
>> goto failure;
>> }
>> @@ -346,8 +341,7 @@ static int acp_hw_init(void *handle)
>> adev->acp.acp_cell[3].platform_data = &i2s_pdata[2];
>> adev->acp.acp_cell[3].pdata_size = sizeof(struct
>> i2s_platform_data);
>> - r = mfd_add_hotplug
Re: [PATCH] drm/amd/display: Remove unused variables from vba_vars_st
Applied. Thanks!
Alex
On Thu, Jun 30, 2022 at 5:53 PM Maíra Canal wrote:
>
> Some variables from the struct vba_vars_st are not referenced in any
> other place on the codebase. As they are not used, this commit removes
> those variables.
>
> Signed-off-by: Maíra Canal
> ---
>
> Unused variables from structs are not warned by compilers, so they are a bit
> harder to find. In order to find these unused variables, I used git grep and
> checked if they were used anywhere else.
>
> Any feedback or suggestion (maybe a tool to check unused variables from
> structs)
> is welcomed!
>
> Best Regards,
> - Maíra Canal
>
> ---
> .../drm/amd/display/dc/dml/display_mode_vba.c | 1 -
> .../drm/amd/display/dc/dml/display_mode_vba.h | 33 ---
> 2 files changed, 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.c
> b/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.c
> index ed23c7c79d86..6b3918609d26 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.c
> @@ -346,7 +346,6 @@ static void fetch_socbb_params(struct display_mode_lib
> *mode_lib)
> mode_lib->vba.DRAMClockChangeRequirementFinal = 1;
> mode_lib->vba.FCLKChangeRequirementFinal = 1;
> mode_lib->vba.USRRetrainingRequiredFinal = 1;
> - mode_lib->vba.ConfigurableDETSizeEnFinal = 0;
> mode_lib->vba.AllowForPStateChangeOrStutterInVBlankFinal =
> soc->allow_for_pstate_or_stutter_in_vblank_final;
> mode_lib->vba.DRAMClockChangeLatency =
> soc->dram_clock_change_latency_us;
> mode_lib->vba.DummyPStateCheck = soc->dram_clock_change_latency_us ==
> soc->dummy_pstate_latency_us;
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.h
> b/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.h
> index 25a9a606ab6f..e95b2199d85a 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.h
> +++ b/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.h
> @@ -232,7 +232,6 @@ struct vba_vars_st {
> double DISPCLKWithRampingRoundedToDFSGranularity;
> double DISPCLKWithoutRampingRoundedToDFSGranularity;
> double MaxDispclkRoundedToDFSGranularity;
> - double MaxDppclkRoundedToDFSGranularity;
> bool DCCEnabledAnyPlane;
> double ReturnBandwidthToDCN;
> unsigned int TotalActiveDPP;
> @@ -249,7 +248,6 @@ struct vba_vars_st {
> double VBlankTime;
> double SmallestVBlank;
> enum dm_prefetch_modes AllowForPStateChangeOrStutterInVBlankFinal; //
> Mode Support only
> - double DCFCLKDeepSleepPerSurface[DC__NUM_DPP__MAX];
> double DCFCLKDeepSleepPerPlane[DC__NUM_DPP__MAX];
> double EffectiveDETPlusLBLinesLuma;
> double EffectiveDETPlusLBLinesChroma;
> @@ -297,7 +295,6 @@ struct vba_vars_st {
> double SMNLatency;
> double FCLKChangeLatency;
> unsigned int MALLAllocatedForDCNFinal;
> - double DefaultGPUVMMinPageSizeKBytes; // Default for the project
> double
> MaxAveragePercentOfIdealFabricBWDisplayCanUseInNormalSystemOperation;
> double
> MaxAveragePercentOfIdealDRAMBWDisplayCanUseInNormalSystemOperationSTROBE;
> double PercentOfIdealDRAMBWReceivedAfterUrgLatencySTROBE;
> @@ -819,8 +816,6 @@ struct vba_vars_st {
> double dummy8[DC__NUM_DPP__MAX];
> double dummy13[DC__NUM_DPP__MAX];
> double dummy_double_array[2][DC__NUM_DPP__MAX];
> - unsigned intdummyinteger1ms[DC__NUM_DPP__MAX];
> - doubledummyinteger2ms[DC__NUM_DPP__MAX];
> unsigned intdummyinteger3[DC__NUM_DPP__MAX];
> unsigned intdummyinteger4[DC__NUM_DPP__MAX];
> unsigned intdummyinteger5;
> @@ -830,16 +825,7 @@ struct vba_vars_st {
> unsigned intdummyinteger9;
> unsigned intdummyinteger10;
> unsigned intdummyinteger11;
> - unsigned intdummyinteger12;
> - unsigned intdummyinteger30;
> - unsigned intdummyinteger31;
> - unsigned intdummyinteger32;
> - unsigned intdummyintegerarr1[DC__NUM_DPP__MAX];
> - unsigned intdummyintegerarr2[DC__NUM_DPP__MAX];
> - unsigned intdummyintegerarr3[DC__NUM_DPP__MAX];
> - unsigned intdummyintegerarr4[DC__NUM_DPP__MAX];
> unsigned intdummy_integer_array[8][DC__NUM_DPP__MAX];
> - unsigned intdummy_integer_array22[22][DC__NUM_DPP__MAX];
>
> bool dummysinglestring;
> bool SingleDPPViewportSizeSupportPerPlane[DC__NUM_DPP__MAX];
> @@ -980,7 +966,6 @@ struct vba_vars_st {
> double TimePerChromaMetaChunkFlip[DC__NUM_DPP__MAX];
> unsigned int DCCCMaxUncompressedBlock[DC__NUM_DPP__MAX];
> unsigned int DCCCMaxCompressedBlock[DC__NUM_DPP__MAX];
> - unsigned int DCCCIndependen
Re: [PATCH] drm/amd/display: Remove duplicate code across dcn30 and dcn31
Applied. Thanks!
Alex
On Thu, Jun 30, 2022 at 4:18 PM Maíra Canal wrote:
>
> The function CalculateBytePerPixelAnd256BBlockSizes was defined four
> times: on display_mode_vba_30.c, display_rq_dlg_calc_30.c,
> display_mode_vba_31.c and display_rq_dlg_calc_31.c. In order to avoid
> code duplication, the CalculateBytePerPixelAnd256BBlockSizes is defined
> on display_mode_vba_30.h and used across dcn30 and dcn31.
>
> Signed-off-by: Maíra Canal
> ---
> .../dc/dml/dcn30/display_mode_vba_30.c| 21 +---
> .../dc/dml/dcn30/display_mode_vba_30.h| 11 ++
> .../dc/dml/dcn30/display_rq_dlg_calc_30.c | 93 +--
> .../dc/dml/dcn31/display_mode_vba_31.c| 106 +-
> .../dc/dml/dcn31/display_rq_dlg_calc_31.c | 91 +--
> 5 files changed, 23 insertions(+), 299 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c
> b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c
> index fb4aa4c800bf..842eb94ebe04 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c
> @@ -712,18 +712,6 @@ static double CalculateUrgentLatency(
> double UrgentLatencyAdjustmentFabricClockReference,
> double FabricClockSingle);
>
> -static bool CalculateBytePerPixelAnd256BBlockSizes(
> - enum source_format_class SourcePixelFormat,
> - enum dm_swizzle_mode SurfaceTiling,
> - unsigned int *BytePerPixelY,
> - unsigned int *BytePerPixelC,
> - double *BytePerPixelDETY,
> - double *BytePerPixelDETC,
> - unsigned int *BlockHeight256BytesY,
> - unsigned int *BlockHeight256BytesC,
> - unsigned int *BlockWidth256BytesY,
> - unsigned int *BlockWidth256BytesC);
> -
> void dml30_recalculate(struct display_mode_lib *mode_lib)
> {
> ModeSupportAndSystemConfiguration(mode_lib);
> @@ -2095,7 +2083,7 @@ static void
> DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerforman
> DTRACE(" return_bus_bw = %f", v->ReturnBW);
>
> for (k = 0; k < v->NumberOfActivePlanes; ++k) {
> - CalculateBytePerPixelAnd256BBlockSizes(
> + dml30_CalculateBytePerPixelAnd256BBlockSizes(
> v->SourcePixelFormat[k],
> v->SurfaceTiling[k],
> &v->BytePerPixelY[k],
> @@ -3165,7 +3153,7 @@ static void DisplayPipeConfiguration(struct
> display_mode_lib *mode_lib)
>
> for (k = 0; k < mode_lib->vba.NumberOfActivePlanes; ++k) {
>
> - CalculateBytePerPixelAnd256BBlockSizes(
> + dml30_CalculateBytePerPixelAnd256BBlockSizes(
> mode_lib->vba.SourcePixelFormat[k],
> mode_lib->vba.SurfaceTiling[k],
> &BytePerPixY[k],
> @@ -3218,7 +3206,7 @@ static void DisplayPipeConfiguration(struct
> display_mode_lib *mode_lib)
> &dummysinglestring);
> }
>
> -static bool CalculateBytePerPixelAnd256BBlockSizes(
> +void dml30_CalculateBytePerPixelAnd256BBlockSizes(
> enum source_format_class SourcePixelFormat,
> enum dm_swizzle_mode SurfaceTiling,
> unsigned int *BytePerPixelY,
> @@ -3305,7 +3293,6 @@ static bool CalculateBytePerPixelAnd256BBlockSizes(
> *BlockWidth256BytesY = 256U / *BytePerPixelY /
> *BlockHeight256BytesY;
> *BlockWidth256BytesC = 256U / *BytePerPixelC /
> *BlockHeight256BytesC;
> }
> - return true;
> }
>
> static double CalculateTWait(
> @@ -3709,7 +3696,7 @@ void dml30_ModeSupportAndSystemConfigurationFull(struct
> display_mode_lib *mode_l
> /*Bandwidth Support Check*/
>
> for (k = 0; k <= v->NumberOfActivePlanes - 1; k++) {
> - CalculateBytePerPixelAnd256BBlockSizes(
> + dml30_CalculateBytePerPixelAnd256BBlockSizes(
> v->SourcePixelFormat[k],
> v->SurfaceTiling[k],
> &v->BytePerPixelY[k],
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.h
> b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.h
> index 4e249eaabfdb..daaf0883b84d 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.h
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.h
> @@ -39,5 +39,16 @@ double dml30_CalculateWriteBackDISPCLK(
> long WritebackDestinationWidth,
> unsigned int HTotal,
> unsigned int WritebackLineBufferSize);
> +void dml30_CalculateBytePerPixelAnd256BBlockSizes(
> + enum source_format_class SourcePixelFormat,
> + enum
Re: [PATCH -next] drm/amd/display: clean up some inconsistent indenting
Applied. Thanks!
Alex
On Fri, Jul 1, 2022 at 5:24 AM Yang Li wrote:
>
> Eliminate the follow smatch warning:
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:3151
> commit_planes_for_stream() warn: inconsistent indenting
>
> Signed-off-by: Yang Li
> ---
> drivers/gpu/drm/amd/display/dc/core/dc.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 146fd4b864b2..d31da9c0256a 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -3148,15 +3148,15 @@ static void commit_planes_for_stream(struct dc *dc,
> else
> dc->hwss.pipe_control_lock(dc, top_pipe_to_program,
> false);
>
> - if ((update_type != UPDATE_TYPE_FAST) &&
> stream->update_flags.bits.dsc_changed)
> - if
> (top_pipe_to_program->stream_res.tg->funcs->lock_doublebuffer_enable) {
> -
> top_pipe_to_program->stream_res.tg->funcs->wait_for_state(
> + if ((update_type != UPDATE_TYPE_FAST) &&
> stream->update_flags.bits.dsc_changed)
> + if
> (top_pipe_to_program->stream_res.tg->funcs->lock_doublebuffer_enable) {
> +
> top_pipe_to_program->stream_res.tg->funcs->wait_for_state(
> top_pipe_to_program->stream_res.tg,
> CRTC_STATE_VACTIVE);
> -
> top_pipe_to_program->stream_res.tg->funcs->wait_for_state(
> +
> top_pipe_to_program->stream_res.tg->funcs->wait_for_state(
> top_pipe_to_program->stream_res.tg,
> CRTC_STATE_VBLANK);
> -
> top_pipe_to_program->stream_res.tg->funcs->wait_for_state(
> +
> top_pipe_to_program->stream_res.tg->funcs->wait_for_state(
> top_pipe_to_program->stream_res.tg,
> CRTC_STATE_VACTIVE);
>
> --
> 2.20.1.7.g153144c
>
Re: [PATCH] drm/amd/display: Remove return value of Calculate256BBlockSizes
Applied. Thanks! Alex On Thu, Jun 30, 2022 at 2:50 PM Maíra Canal wrote: > > The function Calculate256BBlockSizes always returns true, regardless of > the parameters. As any file checks the return of the function, this > commit changes the return value to void. > > Signed-off-by: Maíra Canal > --- > drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.c | 3 +-- > drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.h | 2 +- > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.c > b/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.c > index 2676710a5f2b..ed23c7c79d86 100644 > --- a/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.c > +++ b/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.c > @@ -957,7 +957,7 @@ static void recalculate_params( > } > } > > -bool Calculate256BBlockSizes( > +void Calculate256BBlockSizes( > enum source_format_class SourcePixelFormat, > enum dm_swizzle_mode SurfaceTiling, > unsigned int BytePerPixelY, > @@ -995,7 +995,6 @@ bool Calculate256BBlockSizes( > *BlockWidth256BytesY = 256 / BytePerPixelY / > *BlockHeight256BytesY; > *BlockWidth256BytesC = 256 / BytePerPixelC / > *BlockHeight256BytesC; > } > - return true; > } > > bool CalculateMinAndMaxPrefetchMode( > diff --git a/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.h > b/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.h > index 10ff536ef2a4..25a9a606ab6f 100644 > --- a/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.h > +++ b/drivers/gpu/drm/amd/display/dc/dml/display_mode_vba.h > @@ -169,7 +169,7 @@ bool get_is_phantom_pipe(struct display_mode_lib > *mode_lib, > unsigned int pipe_idx); > void PixelClockAdjustmentForProgressiveToInterlaceUnit(struct > display_mode_lib *mode_lib); > > -bool Calculate256BBlockSizes( > +void Calculate256BBlockSizes( > enum source_format_class SourcePixelFormat, > enum dm_swizzle_mode SurfaceTiling, > unsigned int BytePerPixelY, > -- > 2.36.1 >
Re: [PATCH] drm/amdpgu/debugfs: Simplify some exit paths
Applied. Thanks!
On Mon, Jul 4, 2022 at 12:23 PM Christian König
wrote:
>
> Am 04.07.22 um 15:45 schrieb André Almeida:
> > To avoid code repetition, unify the function exit path when possible. No
> > functional changes.
> >
> > Signed-off-by: André Almeida
>
> Acked-by: Christian König
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 107
> > 1 file changed, 42 insertions(+), 65 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > index f3ac7912c29c..f3b3c688e4e7 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > @@ -383,12 +383,8 @@ static ssize_t amdgpu_debugfs_regs_pcie_read(struct
> > file *f, char __user *buf,
> >
> > value = RREG32_PCIE(*pos);
> > r = put_user(value, (uint32_t *)buf);
> > - if (r) {
> > - pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> > - pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > - amdgpu_virt_disable_access_debugfs(adev);
> > - return r;
> > - }
> > + if (r)
> > + goto out;
> >
> > result += 4;
> > buf += 4;
> > @@ -396,11 +392,12 @@ static ssize_t amdgpu_debugfs_regs_pcie_read(struct
> > file *f, char __user *buf,
> > size -= 4;
> > }
> >
> > + r = result;
> > +out:
> > pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> > pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > -
> > amdgpu_virt_disable_access_debugfs(adev);
> > - return result;
> > + return r;
> > }
> >
> > /**
> > @@ -441,12 +438,8 @@ static ssize_t amdgpu_debugfs_regs_pcie_write(struct
> > file *f, const char __user
> > uint32_t value;
> >
> > r = get_user(value, (uint32_t *)buf);
> > - if (r) {
> > - pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> > - pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > - amdgpu_virt_disable_access_debugfs(adev);
> > - return r;
> > - }
> > + if (r)
> > + goto out;
> >
> > WREG32_PCIE(*pos, value);
> >
> > @@ -456,11 +449,12 @@ static ssize_t amdgpu_debugfs_regs_pcie_write(struct
> > file *f, const char __user
> > size -= 4;
> > }
> >
> > + r = result;
> > +out:
> > pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> > pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > -
> > amdgpu_virt_disable_access_debugfs(adev);
> > - return result;
> > + return r;
> > }
> >
> > /**
> > @@ -502,12 +496,8 @@ static ssize_t amdgpu_debugfs_regs_didt_read(struct
> > file *f, char __user *buf,
> >
> > value = RREG32_DIDT(*pos >> 2);
> > r = put_user(value, (uint32_t *)buf);
> > - if (r) {
> > - pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> > - pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > - amdgpu_virt_disable_access_debugfs(adev);
> > - return r;
> > - }
> > + if (r)
> > + goto out;
> >
> > result += 4;
> > buf += 4;
> > @@ -515,11 +505,12 @@ static ssize_t amdgpu_debugfs_regs_didt_read(struct
> > file *f, char __user *buf,
> > size -= 4;
> > }
> >
> > + r = result;
> > +out:
> > pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> > pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > -
> > amdgpu_virt_disable_access_debugfs(adev);
> > - return result;
> > + return r;
> > }
> >
> > /**
> > @@ -560,12 +551,8 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct
> > file *f, const char __user
> > uint32_t value;
> >
> > r = get_user(value, (uint32_t *)buf);
> > - if (r) {
> > - pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> > - pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > - amdgpu_virt_disable_access_debugfs(adev);
> > - return r;
> > - }
> > + if (r)
> > + goto out;
> >
> > WREG32_DIDT(*pos >> 2, value);
> >
> > @@ -575,11 +562,12 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct
> > file *f, const char __user
> > size -= 4;
> > }
> >
> > + r = result;
> > +out:
> > pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
> > pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > -
> > amdgpu_virt_disable_access_debugfs(adev);
> > - return result;
> > + return r;
> > }
> >
> > /**
> > @@ -6
Re: [PATCH] drm/amdgpu/mes: Fix an error handling path in amdgpu_mes_self_test()
Applied. Thanks!
On Tue, Jul 5, 2022 at 9:46 AM Jianglei Nie wrote:
>
> if amdgpu_mes_ctx_alloc_meta_data() fails, we should call amdgpu_vm_fini()
> to handle amdgpu_vm_init().
>
> Add a new lable before amdgpu_vm_init() and goto this lable when
> amdgpu_mes_ctx_alloc_meta_data() fails.
>
> Signed-off-by: Jianglei Nie
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> index 69a70a0aaed9..7c196b8ac49f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> @@ -1157,7 +1157,7 @@ int amdgpu_mes_self_test(struct amdgpu_device *adev)
> r = amdgpu_mes_ctx_alloc_meta_data(adev, &ctx_data);
> if (r) {
> DRM_ERROR("failed to alloc ctx meta data\n");
> - goto error_pasid;
> + goto error_fini;
> }
>
> ctx_data.meta_data_gpu_addr = AMDGPU_VA_RESERVED_SIZE;
> @@ -1215,6 +1215,8 @@ int amdgpu_mes_self_test(struct amdgpu_device *adev)
> BUG_ON(amdgpu_bo_reserve(ctx_data.meta_data_obj, true));
> amdgpu_vm_bo_del(adev, ctx_data.meta_data_va);
> amdgpu_bo_unreserve(ctx_data.meta_data_obj);
> +
> +error_fini:
> amdgpu_vm_fini(adev, vm);
>
> error_pasid:
> --
> 2.25.1
>
RE: [PATCH] drm/amdgpu: Fix GTT size reporting in amdgpu_ioctl
Hi Alex,
I think we need to revert this patch on amd-staging-drm-next branch, as its
base commit like " drm/amdgpu: remove GTT accounting v2" does not present on
5.16. Instead, the series is part of upcoming 5.18 based amd-staging-drm-next
branch. Otherwise, incorrect GTT size reporting switched from page to bytes
will crash several vulkan APPs.
Regards,
Guchun
-Original Message-
From: amd-gfx On Behalf Of Alex Deucher
Sent: Saturday, June 11, 2022 12:01 AM
To: Michel Dänzer
Cc: Deucher, Alexander ; Pan, Xinhui
; amd-gfx list ; Koenig,
Christian ; Maling list - DRI developers
Subject: Re: [PATCH] drm/amdgpu: Fix GTT size reporting in amdgpu_ioctl
Applied. Thanks!
Alex
On Fri, Jun 10, 2022 at 10:01 AM Michel Dänzer wrote:
>
> From: Michel Dänzer
>
> The commit below changed the TTM manager size unit from pages to
> bytes, but failed to adjust the corresponding calculations in
> amdgpu_ioctl.
>
> Fixes: dfa714b88eb0 ("drm/amdgpu: remove GTT accounting v2")
> Bug:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> ab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1930&data=05%7C01%7C
> guchun.chen%40amd.com%7C28ed180d765c4588474008da4afa68e1%7C3dd8961fe48
> 84e608e11a82d994e183d%7C0%7C0%7C637904736611555668%7CUnknown%7CTWFpbGZ
> sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7C3000%7C%7C%7C&sdata=%2Bmr%2BJWj5q%2BfB04L4hmNSG%2BYpfhny6YayNV
> gt2xty6bo%3D&reserved=0
> Bug:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> ab.freedesktop.org%2Fmesa%2Fmesa%2F-%2Fissues%2F6642&data=05%7C01%
> 7Cguchun.chen%40amd.com%7C28ed180d765c4588474008da4afa68e1%7C3dd8961fe
> 4884e608e11a82d994e183d%7C0%7C0%7C637904736611555668%7CUnknown%7CTWFpb
> GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> %3D%7C3000%7C%7C%7C&sdata=yN1jFKsffHu2Ik2crsrRxGBxCRylXckSj9zILxTZ
> QzE%3D&reserved=0
> Signed-off-by: Michel Dänzer
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 801f6fa692e9..6de63ea6687e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -642,7 +642,6 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp)
> atomic64_read(&adev->visible_pin_size),
> vram_gtt.vram_size);
> vram_gtt.gtt_size = ttm_manager_type(&adev->mman.bdev,
> TTM_PL_TT)->size;
> - vram_gtt.gtt_size *= PAGE_SIZE;
> vram_gtt.gtt_size -= atomic64_read(&adev->gart_pin_size);
> return copy_to_user(out, &vram_gtt,
> min((size_t)size,
> sizeof(vram_gtt))) ? -EFAULT : 0; @@ -675,7 +674,6 @@ int
> amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> mem.cpu_accessible_vram.usable_heap_size * 3 /
> 4;
>
> mem.gtt.total_heap_size = gtt_man->size;
> - mem.gtt.total_heap_size *= PAGE_SIZE;
> mem.gtt.usable_heap_size = mem.gtt.total_heap_size -
> atomic64_read(&adev->gart_pin_size);
> mem.gtt.heap_usage =
> ttm_resource_manager_usage(gtt_man);
> --
> 2.36.1
>
[PATCH] drm/amdgpu/mes: Fix an error handling path in amdgpu_mes_self_test()
if amdgpu_mes_ctx_alloc_meta_data() fails, we should call amdgpu_vm_fini()
to handle amdgpu_vm_init().
Add a new lable before amdgpu_vm_init() and goto this lable when
amdgpu_mes_ctx_alloc_meta_data() fails.
Signed-off-by: Jianglei Nie
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
index 69a70a0aaed9..7c196b8ac49f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -1157,7 +1157,7 @@ int amdgpu_mes_self_test(struct amdgpu_device *adev)
r = amdgpu_mes_ctx_alloc_meta_data(adev, &ctx_data);
if (r) {
DRM_ERROR("failed to alloc ctx meta data\n");
- goto error_pasid;
+ goto error_fini;
}
ctx_data.meta_data_gpu_addr = AMDGPU_VA_RESERVED_SIZE;
@@ -1215,6 +1215,8 @@ int amdgpu_mes_self_test(struct amdgpu_device *adev)
BUG_ON(amdgpu_bo_reserve(ctx_data.meta_data_obj, true));
amdgpu_vm_bo_del(adev, ctx_data.meta_data_va);
amdgpu_bo_unreserve(ctx_data.meta_data_obj);
+
+error_fini:
amdgpu_vm_fini(adev, vm);
error_pasid:
--
2.25.1
RE: [PATCH 00/40] DC Patches Jun 30, 2022
[Public] Hi all, This week this patchset was tested on the following systems: HP Envy 360, with Ryzen 5 4500U Lenovo Thinkpad T14s Gen2, with AMD Ryzen 5 5650U Sapphire Pulse RX5700XT Reference AMD RX6800 Engineering board with Ryzen 9 5900H These systems were tested on the following display types: eDP, (1080p 60hz [4500U, 5650U, 5900H]) 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 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 - 14ce34dd0d7dc6fe5b030405bac074acf402e2e1) 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 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: Siqueira, Rodrigo Sent: June 30, 2022 3:13 PM 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 ; Zuo, Jerry ; Mahfooz, Hamza ; Wheeler, Daniel Subject: [PATCH 00/40] DC Patches Jun 30, 2022 This DC patchset is a large one that brings improvements in multiple areas. In summary, we highlight: - Program ACP-related registers - Fixes for DMUB, DPIA, PSR, and others - Improvements in the pipe split - Add SubVP code - Add basic setup for FAMS support - Improve BB capabilities Cc: Daniel Wheeler Thanks Siqueira Alan Liu (1): drm/amd/display: Program ACP related register Alvin Lee (5): drm/amd/display: Add SubVP required code drm/amd/display: Change DET policy for MPO cases drm/amd/display: Make OPTC3 function accessible to other DCN drm/amd/display: Don't set dram clock change requirement for SubVP drm/amd/display: Maintain old audio programming sequence Aric Cyr (1): drm/amd/display: 3.2.192 Chris Park (3): drm/amd/display: Switch to correct DTO on HDMI drm/amd/display: Indicate stream change on ODM change drm/amd/display: OVT Update on InfoFrame and Mode Management Dmytro Laktyushkin (2): drm/amd/display: disable timing sync b/w odm halves drm/amd/display: disable otg toggle w/a on boot Duncan Ma (1): drm/amd/display: Add flag to modify MST delay Eric Bernstein (3): drm/amd/display: Add function to set pixels per cycle drm/amd/display: Update gpuvm_max_page_table_levels IP param drm/amd/display: Fix null timing generator resource Evgenii Krasnikov (1): drm/amd/display: add an option to skip wait for HPD when powering on eDP panel Fangzhi Zuo (1): drm/amd/display: Fix dmub soft hang for PSR 1 Hamza Mahfooz (2): drm/amd/display: enable PCON SST support for newer ASICs drm/amd/display: rename hdmi_frl_pcon_support Harry Wentland (1): drm/amd/display: Move all linux includes into OS types Jimmy Kizito (3): drm/amd/display: Maintain consistent mode of operation during encoder assignment drm/amd/display: Disable TBT3 DSC work around by default. drm/amd/display: Fix uninitialized variable. Jun Lei (1): drm/amd/display: Extend soc BB capabilitiy Martin Leung (2): drm/amd/display: Prepare for new interfaces drm/amd/display: guard for virtual calling destroy_link_encoders Meenakshikumar Somasundaram (1): drm/amd/display: Remove configuration option for dpia hpd delay Michael Strauss (1): drm/amd/display: Initialize lt_settings on instantiation Nicholas Kazlauskas (4): drm/amd/display: Fix stream->link_enc unassigned during stream removal drm/amd/display: Guard against ddc_pin being NULL for AUX drm/amd/display: Remove incorrect ASSERT check for link_enc drm/amd/display: Guard against NULL link encoder in log hw state Rodrigo Siqueira (6): drm/amd/display: Add missing registers for ACP drm/amd/display: Use two pixel per container for k1/k2 div drm/amd/display: Add basic infrastructure for enabling FAMS drm/amd/display: Add SubVP control lock drm/amd/display: Add minimal pipe split transition state drm/amd/display: Fix refresh rate issue on Club 3D Samson Tam (1): drm/amd/display: Apply ODM 2:1 policy for single display configuration .../g
Re: [PATCH v6 14/22] dma-buf: Introduce new locking convention
On 7/5/22 01:38, Dmitry Osipenko wrote: ... >>> Also i915 will run into trouble with attach. In particular since i915 >>> starts a full ww transaction in its attach callback to be able to lock >>> other objects if migration is needed. I think i915 CI would catch this >>> in a selftest. >> Seems it indeed it should deadlock. But i915 selftests apparently >> should've caught it and they didn't, I'll re-check what happened. >> > > The i915 selftests use a separate mock_dmabuf_ops. That's why it works > for the selftests, i.e. there is no deadlock. > > Thomas, would i915 CI run a different set of tests or will it be the > default i915 selftests ran by IGT? > Nevermind, I had a local kernel change that was forgotten about.. it prevented the i915 live tests from running. -- Best regards, Dmitry
Re: [Linaro-mm-sig] Re: [PATCH v6 02/22] drm/gem: Move mapping of imported dma-bufs to drm_gem_mmap_obj()
On 7/4/22 15:33, Christian König wrote: > Am 30.06.22 um 01:06 schrieb Dmitry Osipenko: >> On 6/29/22 11:43, Thomas Hellström (Intel) wrote: >>> On 6/29/22 10:22, Dmitry Osipenko wrote: On 6/29/22 09:40, Thomas Hellström (Intel) wrote: > On 5/27/22 01:50, Dmitry Osipenko wrote: >> Drivers that use drm_gem_mmap() and drm_gem_mmap_obj() helpers don't >> handle imported dma-bufs properly, which results in mapping of >> something >> else than the imported dma-buf. For example, on NVIDIA Tegra we get a >> hard >> lockup when userspace writes to the memory mapping of a dma-buf that >> was >> imported into Tegra's DRM GEM. >> >> To fix this bug, move mapping of imported dma-bufs to >> drm_gem_mmap_obj(). >> Now mmaping of imported dma-bufs works properly for all DRM drivers. > Same comment about Fixes: as in patch 1, >> Cc: sta...@vger.kernel.org >> Signed-off-by: Dmitry Osipenko >> --- >> drivers/gpu/drm/drm_gem.c | 3 +++ >> drivers/gpu/drm/drm_gem_shmem_helper.c | 9 - >> drivers/gpu/drm/tegra/gem.c | 4 >> 3 files changed, 7 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c >> index 86d670c71286..7c0b025508e4 100644 >> --- a/drivers/gpu/drm/drm_gem.c >> +++ b/drivers/gpu/drm/drm_gem.c >> @@ -1038,6 +1038,9 @@ int drm_gem_mmap_obj(struct drm_gem_object >> *obj, >> unsigned long obj_size, >> if (obj_size < vma->vm_end - vma->vm_start) >> return -EINVAL; >> + if (obj->import_attach) >> + return dma_buf_mmap(obj->dma_buf, vma, 0); > If we start enabling mmaping of imported dma-bufs on a majority of > drivers in this way, how do we ensure that user-space is not blindly > using the object mmap without calling the needed DMA_BUF_IOCTL_SYNC > which is needed before and after cpu access of mmap'ed dma-bufs? > > I was under the impression (admittedly without looking) that the few > drivers that actually called into dma_buf_mmap() had some private > user-mode driver code in place that ensured this happened. Since it's a userspace who does the mapping, then it should be a responsibility of userspace to do all the necessary syncing. >>> Sure, but nothing prohibits user-space to ignore the syncing thinking >>> "It works anyway", testing those drivers where the syncing is a NOP. And >>> when a driver that finally needs syncing is tested it's too late to fix >>> all broken user-space. >>> I'm not sure whether anyone in userspace really needs to map imported dma-bufs in practice. Nevertheless, this use-case is broken and should be fixed by either allowing to do the mapping or prohibiting it. >>> Then I'd vote for prohibiting it, at least for now. And for the future >>> moving forward we could perhaps revisit the dma-buf need for syncing, >>> requiring those drivers that actually need it to implement emulated >>> coherent memory which can be done not too inefficiently (vmwgfx being >>> one example). >> Alright, I'll change it to prohibit the mapping. This indeed should be a >> better option. > > Oh, yes please. But I would expect that some people start screaming. > > Over time I've got tons of TTM patches because people illegally tried to > mmap() imported DMA-bufs in their driver. > > Anyway this is probably the right thing to do and we can work on fixing > the fallout later on. I already sent out the patch [1] that prohibits the mapping. Would be great if you all could take a look and give a r-b, thanks in advance. [1] https://patchwork.freedesktop.org/patch/492148/ -- Best regards, Dmitry
Re: [PATCH v6 14/22] dma-buf: Introduce new locking convention
On 7/1/22 13:43, Dmitry Osipenko wrote: > On 6/29/22 00:26, Thomas Hellström (Intel) wrote: >> On 5/30/22 15:57, Dmitry Osipenko wrote: >>> On 5/30/22 16:41, Christian König wrote: Hi Dmitry, Am 30.05.22 um 15:26 schrieb Dmitry Osipenko: > Hello Christian, > > On 5/30/22 09:50, Christian König wrote: >> Hi Dmitry, >> >> First of all please separate out this patch from the rest of the >> series, >> since this is a complex separate structural change. > I assume all the patches will go via the DRM tree in the end since the > rest of the DRM patches in this series depend on this dma-buf change. > But I see that separation may ease reviewing of the dma-buf changes, so > let's try it. That sounds like you are underestimating a bit how much trouble this will be. >> I have tried this before and failed because catching all the locks in >> the right code paths are very tricky. So expect some fallout from this >> and make sure the kernel test robot and CI systems are clean. > Sure, I'll fix up all the reported things in the next iteration. > > BTW, have you ever posted yours version of the patch? Will be great if > we could compare the changed code paths. No, I never even finished creating it after realizing how much work it would be. >>> This patch introduces new locking convention for dma-buf users. From >>> now >>> on all dma-buf importers are responsible for holding dma-buf >>> reservation >>> lock around operations performed over dma-bufs. >>> >>> This patch implements the new dma-buf locking convention by: >>> >>> 1. Making dma-buf API functions to take the reservation lock. >>> >>> 2. Adding new locked variants of the dma-buf API functions for >>> drivers >>> that need to manage imported dma-bufs under the held lock. >> Instead of adding new locked variants please mark all variants which >> expect to be called without a lock with an _unlocked postfix. >> >> This should make it easier to remove those in a follow up patch set >> and >> then fully move the locking into the importer. > Do we really want to move all the locks to the importers? Seems the > majority of drivers should be happy with the dma-buf helpers handling > the locking for them. Yes, I clearly think so. >>> 3. Converting all drivers to the new locking scheme. >> I have strong doubts that you got all of them. At least radeon and >> nouveau should grab the reservation lock in their ->attach callbacks >> somehow. > Radeon and Nouveau use gem_prime_import_sg_table() and they take resv > lock already, seems they should be okay (?) You are looking at the wrong side. You need to fix the export code path, not the import ones. See for example attach on radeon works like this drm_gem_map_attach->drm_gem_pin->radeon_gem_prime_pin->radeon_bo_reserve->ttm_bo_reserve->dma_resv_lock. >>> Yeah, I was looking at the both sides, but missed this one. >> Also i915 will run into trouble with attach. In particular since i915 >> starts a full ww transaction in its attach callback to be able to lock >> other objects if migration is needed. I think i915 CI would catch this >> in a selftest. > Seems it indeed it should deadlock. But i915 selftests apparently > should've caught it and they didn't, I'll re-check what happened. > The i915 selftests use a separate mock_dmabuf_ops. That's why it works for the selftests, i.e. there is no deadlock. Thomas, would i915 CI run a different set of tests or will it be the default i915 selftests ran by IGT? -- Best regards, Dmitry
RE: [PATCH] drm/amdgpu/mes11: fix to unmap legacy queue
[Public]
Reviewed-by: Hawking Zhang
Regards,
Hawking
-Original Message-
From: amd-gfx On Behalf Of Jack Xiao
Sent: Tuesday, July 5, 2022 11:24
To: amd-gfx@lists.freedesktop.org
Cc: Xiao, Jack
Subject: [PATCH] drm/amdgpu/mes11: fix to unmap legacy queue
MES fw updated to support unmapping legacy gfx/compute queue.
Signed-off-by: Jack Xiao
---
drivers/gpu/drm/amd/amdgpu/mes_v11_0.c| 9 -
drivers/gpu/drm/amd/include/mes_v11_api_def.h | 6 +-
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
index 5bdc2babb070..6b07a8b23d67 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
@@ -216,7 +216,7 @@ static int mes_v11_0_unmap_legacy_queue(struct amdgpu_mes
*mes,
mes_remove_queue_pkt.header.opcode = MES_SCH_API_REMOVE_QUEUE;
mes_remove_queue_pkt.header.dwsize = API_FRAME_SIZE_IN_DWORDS;
- mes_remove_queue_pkt.doorbell_offset = input->doorbell_offset << 2;
+ mes_remove_queue_pkt.doorbell_offset = input->doorbell_offset;
mes_remove_queue_pkt.gang_context_addr = 0;
mes_remove_queue_pkt.pipe_id = input->pipe_id; @@ -228,10 +228,9 @@
static int mes_v11_0_unmap_legacy_queue(struct amdgpu_mes *mes,
mes_remove_queue_pkt.tf_data =
lower_32_bits(input->trail_fence_data);
} else {
- if (input->queue_type == AMDGPU_RING_TYPE_GFX)
- mes_remove_queue_pkt.unmap_legacy_gfx_queue = 1;
- else
- mes_remove_queue_pkt.unmap_kiq_utility_queue = 1;
+ mes_remove_queue_pkt.unmap_legacy_queue = 1;
+ mes_remove_queue_pkt.queue_type =
+ convert_to_mes_queue_type(input->queue_type);
}
mes_remove_queue_pkt.api_status.api_completion_fence_addr = diff --git
a/drivers/gpu/drm/amd/include/mes_v11_api_def.h
b/drivers/gpu/drm/amd/include/mes_v11_api_def.h
index 1d37ec2cd737..80dab1146439 100644
--- a/drivers/gpu/drm/amd/include/mes_v11_api_def.h
+++ b/drivers/gpu/drm/amd/include/mes_v11_api_def.h
@@ -227,6 +227,7 @@ union MESAPI_SET_HW_RESOURCES {
uint32_tuint32_t_all;
};
uint32_toversubscription_timer;
+ uint64_tdoorbell_info;
};
uint32_tmax_dwords_in_api[API_FRAME_SIZE_IN_DWORDS];
@@ -286,7 +287,8 @@ union MESAPI__REMOVE_QUEUE {
uint32_t unmap_legacy_gfx_queue : 1;
uint32_t unmap_kiq_utility_queue : 1;
uint32_t preempt_legacy_gfx_queue : 1;
- uint32_t reserved : 29;
+ uint32_t unmap_legacy_queue : 1;
+ uint32_t reserved : 28;
};
struct MES_API_STATUS api_status;
@@ -295,6 +297,8 @@ union MESAPI__REMOVE_QUEUE {
uint64_ttf_addr;
uint32_ttf_data;
+
+ enum MES_QUEUE_TYPE queue_type;
};
uint32_tmax_dwords_in_api[API_FRAME_SIZE_IN_DWORDS];
--
2.35.1
RE: [RESEND RFC 15/18] drm/display/dp_mst: Skip releasing payloads if last connected port isn't connected
[Public]
> -Original Message-
> From: Lyude Paul
> Sent: Wednesday, June 8, 2022 3:30 AM
> To: dri-de...@lists.freedesktop.org; nouv...@lists.freedesktop.org; amd-
> g...@lists.freedesktop.org
> Cc: Lin, Wayne ; Ville Syrjälä
> ; Zuo, Jerry ; Jani Nikula
> ; Imre Deak ; Daniel Vetter
> ; Sean Paul ; David Airlie
> ; Daniel Vetter ; Thomas Zimmermann
> ; Lakha, Bhawanpreet
> ; open list
> Subject: [RESEND RFC 15/18] drm/display/dp_mst: Skip releasing payloads if
> last connected port isn't connected
>
> In the past, we've ran into strange issues regarding errors in response to
> trying to destroy payloads after a port has been unplugged. We fixed this
> back in:
>
> This is intended to replace the workaround that was added here:
>
> commit 3769e4c0af5b ("drm/dp_mst: Avoid to mess up payload table by
> ports in stale topology")
>
> which was intended fix to some of the payload leaks that were observed
> before, where we would attempt to determine if the port was still
> connected to the topology before updating payloads using
> drm_dp_mst_port_downstream_of_branch. This wasn't a particularly good
> solution, since one of the points of still having port and mstb validation is
> to
> avoid sending messages to newly disconnected branches wherever possible
> - thus the required use of drm_dp_mst_port_downstream_of_branch
> would indicate something may be wrong with said validation.
>
> It seems like it may have just been races and luck that made
> drm_dp_mst_port_downstream_of_branch work however, as while I was
> trying to figure out the true cause of this issue when removing the legacy
> MST code - I discovered an important excerpt in section 2.14.2.3.3.6 of the DP
> 2.0
> specs:
>
> "BAD_PARAM - This reply is transmitted when a Message Transaction
> parameter is in error; for example, the next port number is invalid or /no
> device is connected/ to the port associated with the port number."
>
> Sure enough - removing the calls to
> drm_dp_mst_port_downstream_of_branch()
> and instead checking the ->ddps field of the parent port to see whether we
> should release a given payload or not seems to totally fix the issue. This
> does
> actually make sense to me, as it seems the implication is that given a
> topology where an MSTB is removed, the payload for the MST parent's port
> will be released automatically if that port is also marked as disconnected.
> However, if there's another parent in the chain after that which is connected
> - payloads must be released there with an ALLOCATE_PAYLOAD message.
>
> So, let's do that!
>
> Signed-off-by: Lyude Paul
> Cc: Wayne Lin
> Cc: Ville Syrjälä
> Cc: Fangzhi Zuo
> Cc: Jani Nikula
> Cc: Imre Deak
> Cc: Daniel Vetter
> Cc: Sean Paul
> ---
> drivers/gpu/drm/display/drm_dp_mst_topology.c | 51 +++
> 1 file changed, 17 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index dd314586bac3..70adb8db4335 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -3137,7 +3137,7 @@ static struct drm_dp_mst_port
> *drm_dp_get_last_connected_port_to_mstb(struct drm static struct
> drm_dp_mst_branch * drm_dp_get_last_connected_port_and_mstb(struct
> drm_dp_mst_topology_mgr *mgr,
> struct drm_dp_mst_branch *mstb,
> - int *port_num)
> + struct drm_dp_mst_port **last_port)
> {
> struct drm_dp_mst_branch *rmstb = NULL;
> struct drm_dp_mst_port *found_port;
> @@ -3153,7 +3153,8 @@
> drm_dp_get_last_connected_port_and_mstb(struct
> drm_dp_mst_topology_mgr *mgr,
>
> if (drm_dp_mst_topology_try_get_mstb(found_port-
> >parent)) {
> rmstb = found_port->parent;
> - *port_num = found_port->port_num;
> + *last_port = found_port;
> + drm_dp_mst_get_port_malloc(found_port);
> } else {
> /* Search again, starting from this parent */
> mstb = found_port->parent;
> @@ -3170,7 +3171,7 @@ static int drm_dp_payload_send_msg(struct
> drm_dp_mst_topology_mgr *mgr,
> int pbn)
> {
> struct drm_dp_sideband_msg_tx *txmsg;
> - struct drm_dp_mst_branch *mstb;
> + struct drm_dp_mst_branch *mstb = NULL;
> int ret, port_num;
> u8 sinks[DRM_DP_MAX_SDP_STREAMS];
> int i;
> @@ -3178,12 +3179,22 @@ static int drm_dp_payload_send_msg(struct
> drm_dp_mst_topology_mgr *mgr,
> port_num = port->port_num;
> mstb = drm_dp_mst_topology_get_mstb_validated(mgr, port-
> >parent);
> if (!mstb) {
> - mstb = drm_dp_get_last_connected_port_and_mstb(mgr,
> -port->parent,
> -
RE: [RESEND RFC 18/18] drm/display/dp_mst: Move all payload info into the atomic state
[Public] > -Original Message- > From: Lyude Paul > Sent: Wednesday, June 8, 2022 3:30 AM > To: dri-de...@lists.freedesktop.org; nouv...@lists.freedesktop.org; amd- > g...@lists.freedesktop.org > Cc: Lin, Wayne ; Ville Syrjälä > ; Zuo, Jerry ; Jani Nikula > ; Imre Deak ; Daniel Vetter > ; Sean Paul ; Wentland, Harry > ; Li, Sun peng (Leo) ; > Siqueira, Rodrigo ; Deucher, Alexander > ; Koenig, Christian > ; Pan, Xinhui ; David > Airlie ; Daniel Vetter ; Jani Nikula > ; Joonas Lahtinen > ; Rodrigo Vivi ; > Tvrtko Ursulin ; Ben Skeggs > ; Karol Herbst ; Kazlauskas, > Nicholas ; Li, Roman > ; Shih, Jude ; Simon Ser > ; Lakha, Bhawanpreet > ; Mikita Lipski ; > Claudio Suarez ; Chen, Ian ; Colin Ian > King ; Wu, Hersen ; Liu, > Wenjing ; Lei, Jun ; Strauss, > Michael ; Ma, Leo ; > Thomas Zimmermann ; José Roberto de Souza > ; He Ying ; Anshuman > Gupta ; Andi Shyti > ; Ashutosh Dixit ; > Juston Li ; Sean Paul ; > Fernando Ramos ; Luo Jiaxing > ; Javier Martinez Canillas ; > open list ; open list:INTEL DRM DRIVERS > > Subject: [RESEND RFC 18/18] drm/display/dp_mst: Move all payload info into > the atomic state > > Now that we've finally gotten rid of the non-atomic MST users leftover in > the kernel, we can finally get rid of all of the legacy payload code we > have and move as much as possible into the MST atomic state structs. The > main purpose of this is to make the MST code a lot less confusing to work > on, as there's a lot of duplicated logic that doesn't really need to be > here. As well, this should make introducing features like fallback link > retraining and DSC support far easier. > > Since the old payload code was pretty gnarly and there's a Lot of changes > here, I expect this might be a bit difficult to review. So to make things > as easy as possible for reviewers, I'll sum up how both the old and new > code worked here (it took me a while to figure this out too!). > > The old MST code basically worked by maintaining two different payload > tables - proposed_vcpis, and payloads. proposed_vcpis would hold the > modified payload we wanted to push to the topology, while payloads held > the > payload table that was currently programmed in hardware. Modifications to > proposed_vcpis would be handled through drm_dp_allocate_vcpi(), > drm_dp_mst_deallocate_vcpi(), and drm_dp_mst_reset_vcpi_slots(). Then, > they > would be pushed via drm_dp_mst_update_payload_step1() and > drm_dp_mst_update_payload_step2(). > > Furthermore, it's important to note how adding and removing VC payloads > actually worked with drm_dp_mst_update_payload_step1(). When a VC > payload > is removed from the VC table, all VC payloads which come after the removed > VC payload's slots must have their time slots shifted towards the start of > the table. The old code handles this by looping through the entire payload > table and recomputing the start slot for every payload in the topology from > scratch. While very much overkill, this ends up doing the right thing > because we always order the VCPIs for payloads from first to last starting > timeslot. > > It's important to also note that drm_dp_mst_update_payload_step2() isn't > actually limited to updating a single payload - the driver can use it to > queue up multiple payload changes so that as many of them can be sent as > possible before waiting for the ACT. Hi Lyude, I have concern for updating payload table multiple times for multiple payload changes before sending the ACT. Also consult with one branch vendor, they say their fw will expect receiving one ALLOCATE_PAYLOAD after setting DPCD 002c0h bit 0 (VC payload ID table updated). Thanks! > > drm_dp_mst_update_payload_step2() is pretty self explanatory and > basically > the same between the old and new code, save for the fact we don't have a > second step for deleting payloads anymore -and thus rename it to > drm_dp_mst_add_payload_step2(). > > The new payload code stores all of the current payload info within the MST > atomic state and computes as much of the state as possible ahead of time. > This has the one exception of the starting timeslots for payloads, which > can't be determined at atomic check time since the starting time slots will > vary depending on what order CRTCs are enabled in the atomic state - which > varies from driver to driver. These are still stored in the atomic MST > state, but are only copied from the old MST state during atomic commit > time. Likewise, this is when new start slots are determined. > > Adding/removing payloads now works much more closely to how things are > described in the spec. When we delete a payload, we loop through the > current list of payloads and update the start slots for any payloads whose > time slots came after the payload we just deleted. Determining the starting > time slots for new payloads being added is done by simply keeping track of > where the end of the VC table is in > drm_dp_mst_topology_mgr->next_start_slot. Additionally, it's worth noting >
Re: [PATCH RESEND V2 1/3] drm/amdgpu: fix checkpatch warnings
Am 04.07.22 um 15:54 schrieb Vijendar Mukunda:
From: vijendar
Fixed below checkpatch warnings and errors
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:131: CHECK: Comparison to NULL could be written
"apd"
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:150: CHECK: Comparison to NULL could be written
"apd"
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:196: CHECK: Prefer kernel type 'u64'
over 'uint64_t'
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:224: CHECK: Please don't use multiple
blank lines
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:226: CHECK: Comparison to NULL could be written
"!adev->acp.acp_genpd"
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:233: CHECK: Please don't use multiple
blank lines
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:239: CHECK: Alignment should match open
parenthesis
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:241: CHECK: Comparison to NULL could be written
"!adev->acp.acp_cell"
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:247: CHECK: Comparison to NULL could be written
"!adev->acp.acp_res"
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:253: CHECK: Comparison to NULL could be written
"!i2s_pdata"
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:350: CHECK: Alignment should match open
parenthesis
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c:550: ERROR: that open brace { should be
on the previous line
Signed-off-by: Vijendar Mukunda
Reviewed-by: Alex Deucher
changes since v1:
Modified commit label as drm/amdgpu
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 27 +
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
index cc9c9f8b23b2..ba1605ff521f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
@@ -128,7 +128,7 @@ static int acp_poweroff(struct generic_pm_domain *genpd)
struct amdgpu_device *adev;
apd = container_of(genpd, struct acp_pm_domain, gpd);
- if (apd != NULL) {
+ if (apd) {
Well that's still not correct. The variable apd is compute by an upcast
of the genpd parameter.
Since that upcast never returns a NULL value (gpd is not the first
member of the structure) this whole check is completely nonsense.
I strongly suggest to just remove most of those NULL checks.
Only the ones directly after memory allocation really make sense.
Regards,
Christian.
adev = apd->adev;
/* call smu to POWER GATE ACP block
* smu will
@@ -147,7 +147,7 @@ static int acp_poweron(struct generic_pm_domain *genpd)
struct amdgpu_device *adev;
apd = container_of(genpd, struct acp_pm_domain, gpd);
- if (apd != NULL) {
+ if (apd) {
adev = apd->adev;
/* call smu to UNGATE ACP block
* smu will
@@ -193,7 +193,7 @@ static int acp_genpd_remove_device(struct device *dev, void
*data)
static int acp_hw_init(void *handle)
{
int r;
- uint64_t acp_base;
+ u64 acp_base;
u32 val = 0;
u32 count = 0;
struct i2s_platform_data *i2s_pdata = NULL;
@@ -220,37 +220,32 @@ static int acp_hw_init(void *handle)
return -EINVAL;
acp_base = adev->rmmio_base;
-
-
adev->acp.acp_genpd = kzalloc(sizeof(struct acp_pm_domain), GFP_KERNEL);
- if (adev->acp.acp_genpd == NULL)
+ if (!adev->acp.acp_genpd)
return -ENOMEM;
adev->acp.acp_genpd->gpd.name = "ACP_AUDIO";
adev->acp.acp_genpd->gpd.power_off = acp_poweroff;
adev->acp.acp_genpd->gpd.power_on = acp_poweron;
-
-
adev->acp.acp_genpd->adev = adev;
pm_genpd_init(&adev->acp.acp_genpd->gpd, NULL, false);
- adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
- GFP_KERNEL);
+ adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
GFP_KERNEL);
- if (adev->acp.acp_cell == NULL) {
+ if (!adev->acp.acp_cell) {
r = -ENOMEM;
goto failure;
}
adev->acp.acp_res = kcalloc(5, sizeof(struct resource), GFP_KERNEL);
- if (adev->acp.acp_res == NULL) {
+ if (!adev->acp.acp_res) {
r = -ENOMEM;
goto failure;
}
i2s_pdata = kcalloc(3, sizeof(struct i2s_platform_data), GFP_KERNEL);
- if (i2s_pdata == NULL) {
+ if (!i2s_pdata) {
r = -ENOMEM;
goto failure;
}
@@ -346,8 +341,7 @@ static int acp_hw_init(void *handle)
adev->acp.acp_cell[3].platform_data = &i2s_pdata[2];
adev->acp.acp_cell[3].pdata_size = sizeof(struct i2s_platform_data);
- r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell,
- ACP_DEVS);
+ r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell,
ACP_DEVS);
if (r)
goto failure;
@@ -546,8 +540,7 @@ static const struct amd_
[PATCH] drm/amdgpu/mes11: fix to unmap legacy queue
MES fw updated to support unmapping legacy gfx/compute queue.
Signed-off-by: Jack Xiao
---
drivers/gpu/drm/amd/amdgpu/mes_v11_0.c| 9 -
drivers/gpu/drm/amd/include/mes_v11_api_def.h | 6 +-
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
index 5bdc2babb070..6b07a8b23d67 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
@@ -216,7 +216,7 @@ static int mes_v11_0_unmap_legacy_queue(struct amdgpu_mes
*mes,
mes_remove_queue_pkt.header.opcode = MES_SCH_API_REMOVE_QUEUE;
mes_remove_queue_pkt.header.dwsize = API_FRAME_SIZE_IN_DWORDS;
- mes_remove_queue_pkt.doorbell_offset = input->doorbell_offset << 2;
+ mes_remove_queue_pkt.doorbell_offset = input->doorbell_offset;
mes_remove_queue_pkt.gang_context_addr = 0;
mes_remove_queue_pkt.pipe_id = input->pipe_id;
@@ -228,10 +228,9 @@ static int mes_v11_0_unmap_legacy_queue(struct amdgpu_mes
*mes,
mes_remove_queue_pkt.tf_data =
lower_32_bits(input->trail_fence_data);
} else {
- if (input->queue_type == AMDGPU_RING_TYPE_GFX)
- mes_remove_queue_pkt.unmap_legacy_gfx_queue = 1;
- else
- mes_remove_queue_pkt.unmap_kiq_utility_queue = 1;
+ mes_remove_queue_pkt.unmap_legacy_queue = 1;
+ mes_remove_queue_pkt.queue_type =
+ convert_to_mes_queue_type(input->queue_type);
}
mes_remove_queue_pkt.api_status.api_completion_fence_addr =
diff --git a/drivers/gpu/drm/amd/include/mes_v11_api_def.h
b/drivers/gpu/drm/amd/include/mes_v11_api_def.h
index 1d37ec2cd737..80dab1146439 100644
--- a/drivers/gpu/drm/amd/include/mes_v11_api_def.h
+++ b/drivers/gpu/drm/amd/include/mes_v11_api_def.h
@@ -227,6 +227,7 @@ union MESAPI_SET_HW_RESOURCES {
uint32_tuint32_t_all;
};
uint32_toversubscription_timer;
+ uint64_tdoorbell_info;
};
uint32_tmax_dwords_in_api[API_FRAME_SIZE_IN_DWORDS];
@@ -286,7 +287,8 @@ union MESAPI__REMOVE_QUEUE {
uint32_t unmap_legacy_gfx_queue : 1;
uint32_t unmap_kiq_utility_queue : 1;
uint32_t preempt_legacy_gfx_queue : 1;
- uint32_t reserved : 29;
+ uint32_t unmap_legacy_queue : 1;
+ uint32_t reserved : 28;
};
struct MES_API_STATUS api_status;
@@ -295,6 +297,8 @@ union MESAPI__REMOVE_QUEUE {
uint64_ttf_addr;
uint32_ttf_data;
+
+ enum MES_QUEUE_TYPE queue_type;
};
uint32_tmax_dwords_in_api[API_FRAME_SIZE_IN_DWORDS];
--
2.35.1
