Re: [PATCH v3] drm/amdgpu: enable pp_od_clk_voltage for gfx 9.4.3 SRIOV
On 2/28/2024 12:30 PM, Yang Wang wrote:
> v1:
> enabel pp_od_clk_voltage node for gfx 9.4.3 SRIOV and BM.
>
> v2:
> add onevf check for gfx 9.4.3
>
> v3:
> refine code check order to make function clearly.
>
> Signed-off-by: Yang Wang
Reviewed-by: Lijo Lazar
Thanks,
Lijo
> ---
> drivers/gpu/drm/amd/pm/amdgpu_pm.c | 32 +-
> 1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 087d57850304..ad4e260c8052 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -2034,6 +2034,31 @@ static int ss_bias_attr_update(struct amdgpu_device
> *adev, struct amdgpu_device_
> return 0;
> }
>
> +static int pp_od_clk_voltage_attr_update(struct amdgpu_device *adev, struct
> amdgpu_device_attr *attr,
> + uint32_t mask, enum
> amdgpu_device_attr_states *states)
> +{
> + uint32_t gc_ver = amdgpu_ip_version(adev, GC_HWIP, 0);
> +
> + *states = ATTR_STATE_SUPPORTED;
> +
> + if (!amdgpu_dpm_is_overdrive_supported(adev)) {
> + *states = ATTR_STATE_UNSUPPORTED;
> + return 0;
> + }
> +
> + /* Enable pp_od_clk_voltage node for gc 9.4.3 SRIOV/BM support */
> + if (gc_ver == IP_VERSION(9, 4, 3)) {
> + if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev))
> + *states = ATTR_STATE_UNSUPPORTED;
> + return 0;
> + }
> +
> + if (!(attr->flags & mask))
> + *states = ATTR_STATE_UNSUPPORTED;
> +
> + return 0;
> +}
> +
> /* Following items will be read out to indicate current plpd policy:
> * - -1: none
> * - 0: disallow
> @@ -2118,7 +2143,8 @@ static struct amdgpu_device_attr amdgpu_device_attrs[]
> = {
> AMDGPU_DEVICE_ATTR_RW(pp_sclk_od,
> ATTR_FLAG_BASIC),
> AMDGPU_DEVICE_ATTR_RW(pp_mclk_od,
> ATTR_FLAG_BASIC),
> AMDGPU_DEVICE_ATTR_RW(pp_power_profile_mode,
> ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
> - AMDGPU_DEVICE_ATTR_RW(pp_od_clk_voltage,
> ATTR_FLAG_BASIC),
> + AMDGPU_DEVICE_ATTR_RW(pp_od_clk_voltage,
> ATTR_FLAG_BASIC,
> + .attr_update = pp_od_clk_voltage_attr_update),
> AMDGPU_DEVICE_ATTR_RO(gpu_busy_percent,
> ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
> AMDGPU_DEVICE_ATTR_RO(mem_busy_percent,
> ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
> AMDGPU_DEVICE_ATTR_RO(pcie_bw,
> ATTR_FLAG_BASIC),
> @@ -2163,10 +2189,6 @@ static int default_attr_update(struct amdgpu_device
> *adev, struct amdgpu_device_
> } else if (DEVICE_ATTR_IS(pp_dpm_fclk)) {
> if (mp1_ver < IP_VERSION(10, 0, 0))
> *states = ATTR_STATE_UNSUPPORTED;
> - } else if (DEVICE_ATTR_IS(pp_od_clk_voltage)) {
> - *states = ATTR_STATE_UNSUPPORTED;
> - if (amdgpu_dpm_is_overdrive_supported(adev))
> - *states = ATTR_STATE_SUPPORTED;
> } else if (DEVICE_ATTR_IS(mem_busy_percent)) {
> if ((adev->flags & AMD_IS_APU &&
>gc_ver != IP_VERSION(9, 4, 3)) ||
[PATCH v3] drm/amdgpu: enable pp_od_clk_voltage for gfx 9.4.3 SRIOV
v1:
enabel pp_od_clk_voltage node for gfx 9.4.3 SRIOV and BM.
v2:
add onevf check for gfx 9.4.3
v3:
refine code check order to make function clearly.
Signed-off-by: Yang Wang
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 32 +-
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 087d57850304..ad4e260c8052 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2034,6 +2034,31 @@ static int ss_bias_attr_update(struct amdgpu_device
*adev, struct amdgpu_device_
return 0;
}
+static int pp_od_clk_voltage_attr_update(struct amdgpu_device *adev, struct
amdgpu_device_attr *attr,
+uint32_t mask, enum
amdgpu_device_attr_states *states)
+{
+ uint32_t gc_ver = amdgpu_ip_version(adev, GC_HWIP, 0);
+
+ *states = ATTR_STATE_SUPPORTED;
+
+ if (!amdgpu_dpm_is_overdrive_supported(adev)) {
+ *states = ATTR_STATE_UNSUPPORTED;
+ return 0;
+ }
+
+ /* Enable pp_od_clk_voltage node for gc 9.4.3 SRIOV/BM support */
+ if (gc_ver == IP_VERSION(9, 4, 3)) {
+ if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev))
+ *states = ATTR_STATE_UNSUPPORTED;
+ return 0;
+ }
+
+ if (!(attr->flags & mask))
+ *states = ATTR_STATE_UNSUPPORTED;
+
+ return 0;
+}
+
/* Following items will be read out to indicate current plpd policy:
* - -1: none
* - 0: disallow
@@ -2118,7 +2143,8 @@ static struct amdgpu_device_attr amdgpu_device_attrs[] = {
AMDGPU_DEVICE_ATTR_RW(pp_sclk_od,
ATTR_FLAG_BASIC),
AMDGPU_DEVICE_ATTR_RW(pp_mclk_od,
ATTR_FLAG_BASIC),
AMDGPU_DEVICE_ATTR_RW(pp_power_profile_mode,
ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
- AMDGPU_DEVICE_ATTR_RW(pp_od_clk_voltage,
ATTR_FLAG_BASIC),
+ AMDGPU_DEVICE_ATTR_RW(pp_od_clk_voltage,
ATTR_FLAG_BASIC,
+ .attr_update = pp_od_clk_voltage_attr_update),
AMDGPU_DEVICE_ATTR_RO(gpu_busy_percent,
ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
AMDGPU_DEVICE_ATTR_RO(mem_busy_percent,
ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
AMDGPU_DEVICE_ATTR_RO(pcie_bw,
ATTR_FLAG_BASIC),
@@ -2163,10 +2189,6 @@ static int default_attr_update(struct amdgpu_device
*adev, struct amdgpu_device_
} else if (DEVICE_ATTR_IS(pp_dpm_fclk)) {
if (mp1_ver < IP_VERSION(10, 0, 0))
*states = ATTR_STATE_UNSUPPORTED;
- } else if (DEVICE_ATTR_IS(pp_od_clk_voltage)) {
- *states = ATTR_STATE_UNSUPPORTED;
- if (amdgpu_dpm_is_overdrive_supported(adev))
- *states = ATTR_STATE_SUPPORTED;
} else if (DEVICE_ATTR_IS(mem_busy_percent)) {
if ((adev->flags & AMD_IS_APU &&
gc_ver != IP_VERSION(9, 4, 3)) ||
--
2.34.1
[PATCH] drm/amd/pm: Fix esm reg mask use to get pcie speed
Fix mask used for esm ctrl register to get pcie link
speed on smu_v11_0_3, smu_v13_0_2 & smu_v13_0_6
Fixes: 511a95552ec8 ("drm/amd/pm: Add SMU 13.0.6 support")
Fixes: c05d1c401572 ("drm/amd/swsmu: add aldebaran smu13 ip support (v3)")
Fixes: f1c378593153 ("drm/amd/powerplay: add Arcturus support for gpu metrics
export")
Signed-off-by: Asad Kamal
Reviewed-by: Lijo Lazar
---
drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c| 4 ++--
drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 4 ++--
drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index bcad42534da4..1d96eb274d72 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -2272,8 +2272,8 @@ static uint16_t
arcturus_get_current_pcie_link_speed(struct smu_context *smu)
/* TODO: confirm this on real target */
esm_ctrl = RREG32_PCIE(smnPCIE_ESM_CTRL);
- if ((esm_ctrl >> 15) & 0x1)
- return (uint16_t)(((esm_ctrl >> 8) & 0x3F) + 128);
+ if ((esm_ctrl >> 15) & 0x1)
+ return (uint16_t)(((esm_ctrl >> 8) & 0x7F) + 128);
return smu_v11_0_get_current_pcie_link_speed(smu);
}
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
index f122ef49106c..0467864a1aa8 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -1683,8 +1683,8 @@ static int aldebaran_get_current_pcie_link_speed(struct
smu_context *smu)
/* TODO: confirm this on real target */
esm_ctrl = RREG32_PCIE(smnPCIE_ESM_CTRL);
- if ((esm_ctrl >> 15) & 0x1)
- return (((esm_ctrl >> 8) & 0x3F) + 128);
+ if ((esm_ctrl >> 15) & 0x1)
+ return (((esm_ctrl >> 8) & 0x7F) + 128);
return smu_v13_0_get_current_pcie_link_speed(smu);
}
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
index 69c64bc6e2dc..744c84f3029f 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
@@ -2148,8 +2148,8 @@ static int smu_v13_0_6_get_current_pcie_link_speed(struct
smu_context *smu)
/* TODO: confirm this on real target */
esm_ctrl = RREG32_PCIE(smnPCIE_ESM_CTRL);
- if ((esm_ctrl >> 15) & 0x1)
- return (((esm_ctrl >> 8) & 0x3F) + 128);
+ if ((esm_ctrl >> 15) & 0x1)
+ return (((esm_ctrl >> 8) & 0x7F) + 128);
speed_level = (RREG32_PCIE(smnPCIE_LC_SPEED_CNTL) &
PCIE_LC_SPEED_CNTL__LC_CURRENT_DATA_RATE_MASK)
--
2.42.0
Re: [PATCH] drm/amdgpu: Fix assignment errors in 'si_common_early_init' functions
Am 28.02.24 um 02:44 schrieb Lu Yao: uvd_ctx_rreg/uvd_ctx_wreg correct value requires function pointer. Yeah, but that is completely irrelevant here. We usually don't use the & for function pointers since that is unnecessary in C. Regards, Christian. Signed-off-by: Lu Yao --- drivers/gpu/drm/amd/amdgpu/si.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/si.c b/drivers/gpu/drm/amd/amdgpu/si.c index a757526153e5..455d49f7bd9c 100644 --- a/drivers/gpu/drm/amd/amdgpu/si.c +++ b/drivers/gpu/drm/amd/amdgpu/si.c @@ -2032,8 +2032,8 @@ static int si_common_early_init(void *handle) adev->pcie_wreg = _pcie_wreg; adev->pciep_rreg = _pciep_rreg; adev->pciep_wreg = _pciep_wreg; - adev->uvd_ctx_rreg = si_uvd_ctx_rreg; - adev->uvd_ctx_wreg = si_uvd_ctx_wreg; + adev->uvd_ctx_rreg = _uvd_ctx_rreg; + adev->uvd_ctx_wreg = _uvd_ctx_wreg; adev->didt_rreg = NULL; adev->didt_wreg = NULL;
Re: [PATCH v2] drm/amdgpu: enable pp_od_clk_voltage for gfx 9.4.3 SRIOV
On 2/28/2024 12:08 PM, Yang Wang wrote:
> v1:
> enabel pp_od_clk_voltage node for gfx 9.4.3 SRIOV and BM.
>
> v2:
> add onevf check for gfx 9.4.3
>
> Signed-off-by: Yang Wang
> ---
> drivers/gpu/drm/amd/pm/amdgpu_pm.c | 35 +-
> 1 file changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 087d57850304..7e5f00530769 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -2034,6 +2034,34 @@ static int ss_bias_attr_update(struct amdgpu_device
> *adev, struct amdgpu_device_
> return 0;
> }
>
> +static int pp_od_clk_voltage_attr_update(struct amdgpu_device *adev, struct
> amdgpu_device_attr *attr,
> + uint32_t mask, enum
> amdgpu_device_attr_states *states)
> +{
> + uint32_t gc_ver = amdgpu_ip_version(adev, GC_HWIP, 0);
> +
> + *states = ATTR_STATE_UNSUPPORTED;
> +
> + /* Enable pp_od_clk_voltage node for gc 9.4.3 SRIOV/BM support */
> + if (gc_ver == IP_VERSION(9, 4, 3)) {
> + if (!amdgpu_dpm_is_overdrive_supported(adev) ||
A reorder may better work for readability -
Move the !amdgpu_dpm_is_overdrive_supported(adev) check first, and then
for 9.4.3/SMU 13.0.6, only do one-VF check.
Thanks,
Lijo
> + (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)))
> + *states = ATTR_STATE_UNSUPPORTED;
> + else
> + *states = ATTR_STATE_SUPPORTED;
> + return 0;
> + }
> +
> + if (!(attr->flags & mask)) {
> + *states = ATTR_STATE_UNSUPPORTED;
> + return 0;
> + }
> +
> + if (amdgpu_dpm_is_overdrive_supported(adev))
> + *states = ATTR_STATE_SUPPORTED;
> +
> + return 0;
> +}
> +
> /* Following items will be read out to indicate current plpd policy:
> * - -1: none
> * - 0: disallow
> @@ -2118,7 +2146,8 @@ static struct amdgpu_device_attr amdgpu_device_attrs[]
> = {
> AMDGPU_DEVICE_ATTR_RW(pp_sclk_od,
> ATTR_FLAG_BASIC),
> AMDGPU_DEVICE_ATTR_RW(pp_mclk_od,
> ATTR_FLAG_BASIC),
> AMDGPU_DEVICE_ATTR_RW(pp_power_profile_mode,
> ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
> - AMDGPU_DEVICE_ATTR_RW(pp_od_clk_voltage,
> ATTR_FLAG_BASIC),
> + AMDGPU_DEVICE_ATTR_RW(pp_od_clk_voltage,
> ATTR_FLAG_BASIC,
> + .attr_update = pp_od_clk_voltage_attr_update),
> AMDGPU_DEVICE_ATTR_RO(gpu_busy_percent,
> ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
> AMDGPU_DEVICE_ATTR_RO(mem_busy_percent,
> ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
> AMDGPU_DEVICE_ATTR_RO(pcie_bw,
> ATTR_FLAG_BASIC),
> @@ -2163,10 +2192,6 @@ static int default_attr_update(struct amdgpu_device
> *adev, struct amdgpu_device_
> } else if (DEVICE_ATTR_IS(pp_dpm_fclk)) {
> if (mp1_ver < IP_VERSION(10, 0, 0))
> *states = ATTR_STATE_UNSUPPORTED;
> - } else if (DEVICE_ATTR_IS(pp_od_clk_voltage)) {
> - *states = ATTR_STATE_UNSUPPORTED;
> - if (amdgpu_dpm_is_overdrive_supported(adev))
> - *states = ATTR_STATE_SUPPORTED;
> } else if (DEVICE_ATTR_IS(mem_busy_percent)) {
> if ((adev->flags & AMD_IS_APU &&
>gc_ver != IP_VERSION(9, 4, 3)) ||
Re: [PATCH] Revert "drm/amdgpu: remove vm sanity check from amdgpu_vm_make_compute" for Raven
Am 28.02.24 um 06:04 schrieb Jesse.Zhang:
fix the issue when run clinfo:
"amdgpu: Failed to create process VM object".
when amdgpu initialized, seq64 do mampping and update bo mapping in vm page
table.
But when clifo run. It also initializes a vm for a process device through the
function kfd_process_device_init_vm
and ensure the root PD is clean through the function amdgpu_vm_pt_is_root_clean.
So they have a conflict, and clinfo always failed.
Big NAK for this, you removed the check but didn't solved the problem in
any way.
When Raven still needs the ats feature than it is intentional that this
fails.
Regards,
Christian.
Signed-off-by: Jesse Zhang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index ed4a8c5d26d7..0bc0bc75be15 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2361,12 +2361,6 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev,
struct amdgpu_vm *vm)
* changing any other state, in case it fails.
*/
if (pte_support_ats != vm->pte_support_ats) {
- /* Sanity checks */
- if (!amdgpu_vm_pt_is_root_clean(adev, vm)) {
- r = -EINVAL;
- goto unreserve_bo;
- }
-
vm->pte_support_ats = pte_support_ats;
r = amdgpu_vm_pt_clear(adev, vm, to_amdgpu_bo_vm(vm->root.bo),
false);
[PATCH v2] drm/amdgpu: enable pp_od_clk_voltage for gfx 9.4.3 SRIOV
v1:
enabel pp_od_clk_voltage node for gfx 9.4.3 SRIOV and BM.
v2:
add onevf check for gfx 9.4.3
Signed-off-by: Yang Wang
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 35 +-
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 087d57850304..7e5f00530769 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2034,6 +2034,34 @@ static int ss_bias_attr_update(struct amdgpu_device
*adev, struct amdgpu_device_
return 0;
}
+static int pp_od_clk_voltage_attr_update(struct amdgpu_device *adev, struct
amdgpu_device_attr *attr,
+uint32_t mask, enum
amdgpu_device_attr_states *states)
+{
+ uint32_t gc_ver = amdgpu_ip_version(adev, GC_HWIP, 0);
+
+ *states = ATTR_STATE_UNSUPPORTED;
+
+ /* Enable pp_od_clk_voltage node for gc 9.4.3 SRIOV/BM support */
+ if (gc_ver == IP_VERSION(9, 4, 3)) {
+ if (!amdgpu_dpm_is_overdrive_supported(adev) ||
+ (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev)))
+ *states = ATTR_STATE_UNSUPPORTED;
+ else
+ *states = ATTR_STATE_SUPPORTED;
+ return 0;
+ }
+
+ if (!(attr->flags & mask)) {
+ *states = ATTR_STATE_UNSUPPORTED;
+ return 0;
+ }
+
+ if (amdgpu_dpm_is_overdrive_supported(adev))
+ *states = ATTR_STATE_SUPPORTED;
+
+ return 0;
+}
+
/* Following items will be read out to indicate current plpd policy:
* - -1: none
* - 0: disallow
@@ -2118,7 +2146,8 @@ static struct amdgpu_device_attr amdgpu_device_attrs[] = {
AMDGPU_DEVICE_ATTR_RW(pp_sclk_od,
ATTR_FLAG_BASIC),
AMDGPU_DEVICE_ATTR_RW(pp_mclk_od,
ATTR_FLAG_BASIC),
AMDGPU_DEVICE_ATTR_RW(pp_power_profile_mode,
ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
- AMDGPU_DEVICE_ATTR_RW(pp_od_clk_voltage,
ATTR_FLAG_BASIC),
+ AMDGPU_DEVICE_ATTR_RW(pp_od_clk_voltage,
ATTR_FLAG_BASIC,
+ .attr_update = pp_od_clk_voltage_attr_update),
AMDGPU_DEVICE_ATTR_RO(gpu_busy_percent,
ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
AMDGPU_DEVICE_ATTR_RO(mem_busy_percent,
ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
AMDGPU_DEVICE_ATTR_RO(pcie_bw,
ATTR_FLAG_BASIC),
@@ -2163,10 +2192,6 @@ static int default_attr_update(struct amdgpu_device
*adev, struct amdgpu_device_
} else if (DEVICE_ATTR_IS(pp_dpm_fclk)) {
if (mp1_ver < IP_VERSION(10, 0, 0))
*states = ATTR_STATE_UNSUPPORTED;
- } else if (DEVICE_ATTR_IS(pp_od_clk_voltage)) {
- *states = ATTR_STATE_UNSUPPORTED;
- if (amdgpu_dpm_is_overdrive_supported(adev))
- *states = ATTR_STATE_SUPPORTED;
} else if (DEVICE_ATTR_IS(mem_busy_percent)) {
if ((adev->flags & AMD_IS_APU &&
gc_ver != IP_VERSION(9, 4, 3)) ||
--
2.34.1
Re: [PATCH 1/2] Revert "drm/amd: Remove freesync video mode amdgpu parameter"
Am 27.02.24 um 19:48 schrieb Alex Deucher: This reverts commit e94e787e37b99645e7c02d20d0a1ba0f8a18a82a. This conflicts with how compositors want to handle VRR. Now that compositors actually handle VRR, we probably don't need freesync video. Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2985 Scratching my head what actually happens here? Doesn't the problem then just depend on a module parameter? Regards, Christian. Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 27 + 2 files changed, 28 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 0e365cadcc3fc..925026c183f41 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -194,6 +194,7 @@ extern int amdgpu_emu_mode; extern uint amdgpu_smu_memory_pool_size; extern int amdgpu_smu_pptable_id; extern uint amdgpu_dc_feature_mask; +extern uint amdgpu_freesync_vid_mode; extern uint amdgpu_dc_debug_mask; extern uint amdgpu_dc_visual_confirm; extern int amdgpu_dm_abm_level; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 15a8a64fc4e28..82b154b103f43 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -199,6 +199,7 @@ int amdgpu_mes_kiq; int amdgpu_noretry = -1; int amdgpu_force_asic_type = -1; int amdgpu_tmz = -1; /* auto */ +uint amdgpu_freesync_vid_mode; int amdgpu_reset_method = -1; /* auto */ int amdgpu_num_kcq = -1; int amdgpu_smartshift_bias; @@ -883,6 +884,32 @@ module_param_named(damageclips, amdgpu_damage_clips, int, 0444); MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto (default), 0 = off, 1 = on)"); module_param_named(tmz, amdgpu_tmz, int, 0444); +/** + * DOC: freesync_video (uint) + * Enable the optimization to adjust front porch timing to achieve seamless + * mode change experience when setting a freesync supported mode for which full + * modeset is not needed. + * + * The Display Core will add a set of modes derived from the base FreeSync + * video mode into the corresponding connector's mode list based on commonly + * used refresh rates and VRR range of the connected display, when users enable + * this feature. From the userspace perspective, they can see a seamless mode + * change experience when the change between different refresh rates under the + * same resolution. Additionally, userspace applications such as Video playback + * can read this modeset list and change the refresh rate based on the video + * frame rate. Finally, the userspace can also derive an appropriate mode for a + * particular refresh rate based on the FreeSync Mode and add it to the + * connector's mode list. + * + * Note: This is an experimental feature. + * + * The default value: 0 (off). + */ +MODULE_PARM_DESC( + freesync_video, + "Enable freesync modesetting optimization feature (0 = off (default), 1 = on)"); +module_param_named(freesync_video, amdgpu_freesync_vid_mode, uint, 0444); + /** * DOC: reset_method (int) * GPU reset method (-1 = auto (default), 0 = legacy, 1 = mode0, 2 = mode1, 3 = mode2, 4 = baco)
Re: [PATCH] drm/amdgpu: enable pp_od_clk_voltage for gfx 9.4.3 SRIOV
On 2/28/2024 11:28 AM, Yang Wang wrote:
> enabel pp_od_clk_voltage node for gfx 9.4.3 SRIOV and BM.
>
> Signed-off-by: Yang Wang
> ---
> drivers/gpu/drm/amd/pm/amdgpu_pm.c | 29 -
> 1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 087d57850304..233b562950a7 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -2034,6 +2034,28 @@ static int ss_bias_attr_update(struct amdgpu_device
> *adev, struct amdgpu_device_
> return 0;
> }
>
> +static int pp_od_clk_voltage_attr_update(struct amdgpu_device *adev, struct
> amdgpu_device_attr *attr,
> + uint32_t mask, enum
> amdgpu_device_attr_states *states)
> +{
> + uint32_t gc_ver = amdgpu_ip_version(adev, GC_HWIP, 0);
> +
> + /* Enable pp_od_clk_voltage node for gc 9.4.3 SRIOV/BM support */
> + if (gc_ver == IP_VERSION(9, 4, 3)) {
> + *states = ATTR_STATE_SUPPORTED;
For VF, need an additional check to support this only in one-VF mode.
Thanks,
Lijo
> + return 0;
> + }
> +
> + if (!(attr->flags & mask)) {
> + *states = ATTR_STATE_UNSUPPORTED;
> + return 0;
> + }
> +
> + if (amdgpu_dpm_is_overdrive_supported(adev))
> + *states = ATTR_STATE_SUPPORTED;
> +
> + return 0;
> +}
> +
> /* Following items will be read out to indicate current plpd policy:
> * - -1: none
> * - 0: disallow
> @@ -2118,7 +2140,8 @@ static struct amdgpu_device_attr amdgpu_device_attrs[]
> = {
> AMDGPU_DEVICE_ATTR_RW(pp_sclk_od,
> ATTR_FLAG_BASIC),
> AMDGPU_DEVICE_ATTR_RW(pp_mclk_od,
> ATTR_FLAG_BASIC),
> AMDGPU_DEVICE_ATTR_RW(pp_power_profile_mode,
> ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
> - AMDGPU_DEVICE_ATTR_RW(pp_od_clk_voltage,
> ATTR_FLAG_BASIC),
> + AMDGPU_DEVICE_ATTR_RW(pp_od_clk_voltage,
> ATTR_FLAG_BASIC,
> + .attr_update = pp_od_clk_voltage_attr_update),
> AMDGPU_DEVICE_ATTR_RO(gpu_busy_percent,
> ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
> AMDGPU_DEVICE_ATTR_RO(mem_busy_percent,
> ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
> AMDGPU_DEVICE_ATTR_RO(pcie_bw,
> ATTR_FLAG_BASIC),
> @@ -2163,10 +2186,6 @@ static int default_attr_update(struct amdgpu_device
> *adev, struct amdgpu_device_
> } else if (DEVICE_ATTR_IS(pp_dpm_fclk)) {
> if (mp1_ver < IP_VERSION(10, 0, 0))
> *states = ATTR_STATE_UNSUPPORTED;
> - } else if (DEVICE_ATTR_IS(pp_od_clk_voltage)) {
> - *states = ATTR_STATE_UNSUPPORTED;
> - if (amdgpu_dpm_is_overdrive_supported(adev))
> - *states = ATTR_STATE_SUPPORTED;
> } else if (DEVICE_ATTR_IS(mem_busy_percent)) {
> if ((adev->flags & AMD_IS_APU &&
>gc_ver != IP_VERSION(9, 4, 3)) ||
[PATCH] drm/amdgpu: enable pp_od_clk_voltage for gfx 9.4.3 SRIOV
enabel pp_od_clk_voltage node for gfx 9.4.3 SRIOV and BM.
Signed-off-by: Yang Wang
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 29 -
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 087d57850304..233b562950a7 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2034,6 +2034,28 @@ static int ss_bias_attr_update(struct amdgpu_device
*adev, struct amdgpu_device_
return 0;
}
+static int pp_od_clk_voltage_attr_update(struct amdgpu_device *adev, struct
amdgpu_device_attr *attr,
+uint32_t mask, enum
amdgpu_device_attr_states *states)
+{
+ uint32_t gc_ver = amdgpu_ip_version(adev, GC_HWIP, 0);
+
+ /* Enable pp_od_clk_voltage node for gc 9.4.3 SRIOV/BM support */
+ if (gc_ver == IP_VERSION(9, 4, 3)) {
+ *states = ATTR_STATE_SUPPORTED;
+ return 0;
+ }
+
+ if (!(attr->flags & mask)) {
+ *states = ATTR_STATE_UNSUPPORTED;
+ return 0;
+ }
+
+ if (amdgpu_dpm_is_overdrive_supported(adev))
+ *states = ATTR_STATE_SUPPORTED;
+
+ return 0;
+}
+
/* Following items will be read out to indicate current plpd policy:
* - -1: none
* - 0: disallow
@@ -2118,7 +2140,8 @@ static struct amdgpu_device_attr amdgpu_device_attrs[] = {
AMDGPU_DEVICE_ATTR_RW(pp_sclk_od,
ATTR_FLAG_BASIC),
AMDGPU_DEVICE_ATTR_RW(pp_mclk_od,
ATTR_FLAG_BASIC),
AMDGPU_DEVICE_ATTR_RW(pp_power_profile_mode,
ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
- AMDGPU_DEVICE_ATTR_RW(pp_od_clk_voltage,
ATTR_FLAG_BASIC),
+ AMDGPU_DEVICE_ATTR_RW(pp_od_clk_voltage,
ATTR_FLAG_BASIC,
+ .attr_update = pp_od_clk_voltage_attr_update),
AMDGPU_DEVICE_ATTR_RO(gpu_busy_percent,
ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
AMDGPU_DEVICE_ATTR_RO(mem_busy_percent,
ATTR_FLAG_BASIC|ATTR_FLAG_ONEVF),
AMDGPU_DEVICE_ATTR_RO(pcie_bw,
ATTR_FLAG_BASIC),
@@ -2163,10 +2186,6 @@ static int default_attr_update(struct amdgpu_device
*adev, struct amdgpu_device_
} else if (DEVICE_ATTR_IS(pp_dpm_fclk)) {
if (mp1_ver < IP_VERSION(10, 0, 0))
*states = ATTR_STATE_UNSUPPORTED;
- } else if (DEVICE_ATTR_IS(pp_od_clk_voltage)) {
- *states = ATTR_STATE_UNSUPPORTED;
- if (amdgpu_dpm_is_overdrive_supported(adev))
- *states = ATTR_STATE_SUPPORTED;
} else if (DEVICE_ATTR_IS(mem_busy_percent)) {
if ((adev->flags & AMD_IS_APU &&
gc_ver != IP_VERSION(9, 4, 3)) ||
--
2.34.1
[PATCH] Revert "drm/amdgpu: remove vm sanity check from amdgpu_vm_make_compute" for Raven
fix the issue when run clinfo:
"amdgpu: Failed to create process VM object".
when amdgpu initialized, seq64 do mampping and update bo mapping in vm page
table.
But when clifo run. It also initializes a vm for a process device through the
function kfd_process_device_init_vm
and ensure the root PD is clean through the function amdgpu_vm_pt_is_root_clean.
So they have a conflict, and clinfo always failed.
Signed-off-by: Jesse Zhang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index ed4a8c5d26d7..0bc0bc75be15 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2361,12 +2361,6 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev,
struct amdgpu_vm *vm)
* changing any other state, in case it fails.
*/
if (pte_support_ats != vm->pte_support_ats) {
- /* Sanity checks */
- if (!amdgpu_vm_pt_is_root_clean(adev, vm)) {
- r = -EINVAL;
- goto unreserve_bo;
- }
-
vm->pte_support_ats = pte_support_ats;
r = amdgpu_vm_pt_clear(adev, vm, to_amdgpu_bo_vm(vm->root.bo),
false);
--
2.34.1
Re: [PATCH] drm/amd/pm: Skip reporting pcie width/speed on vfs
On 2/28/2024 9:30 AM, Asad Kamal wrote:
> Skip reporting pcie link width/speed on vfs for
> smu_v13_0_6 & smu_v13_0_2
>
> Signed-off-by: Asad Kamal
> Reviewed-by: Yang Wang
Reviewed-by: Lijo Lazar
Thanks,
Lijo
> ---
> .../gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 10 ++
> .../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 18 ++
> 2 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> index f1440869d1ce..f122ef49106c 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -1747,10 +1747,12 @@ static ssize_t aldebaran_get_gpu_metrics(struct
> smu_context *smu,
>
> gpu_metrics->current_fan_speed = 0;
>
> - gpu_metrics->pcie_link_width =
> - smu_v13_0_get_current_pcie_link_width(smu);
> - gpu_metrics->pcie_link_speed =
> - aldebaran_get_current_pcie_link_speed(smu);
> + if (!amdgpu_sriov_vf(smu->adev)) {
> + gpu_metrics->pcie_link_width =
> + smu_v13_0_get_current_pcie_link_width(smu);
> + gpu_metrics->pcie_link_speed =
> + aldebaran_get_current_pcie_link_speed(smu);
> + }
>
> gpu_metrics->system_clock_counter = ktime_get_boottime_ns();
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> index 2b7a60b23d6b..69c64bc6e2dc 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> @@ -2229,14 +2229,16 @@ static ssize_t smu_v13_0_6_get_gpu_metrics(struct
> smu_context *smu, void **table
> gpu_metrics->gfxclk_lock_status = GET_METRIC_FIELD(GfxLockXCDMak) >>
> GET_INST(GC, 0);
>
> if (!(adev->flags & AMD_IS_APU)) {
> - link_width_level =
> smu_v13_0_6_get_current_pcie_link_width_level(smu);
> - if (link_width_level > MAX_LINK_WIDTH)
> - link_width_level = 0;
> -
> - gpu_metrics->pcie_link_width =
> - DECODE_LANE_WIDTH(link_width_level);
> - gpu_metrics->pcie_link_speed =
> - smu_v13_0_6_get_current_pcie_link_speed(smu);
> + if (!amdgpu_sriov_vf(adev)) {
> + link_width_level =
> smu_v13_0_6_get_current_pcie_link_width_level(smu);
> + if (link_width_level > MAX_LINK_WIDTH)
> + link_width_level = 0;
> +
> + gpu_metrics->pcie_link_width =
> + DECODE_LANE_WIDTH(link_width_level);
> + gpu_metrics->pcie_link_speed =
> + smu_v13_0_6_get_current_pcie_link_speed(smu);
> + }
> gpu_metrics->pcie_bandwidth_acc =
> SMUQ10_ROUND(metrics_x->PcieBandwidthAcc[0]);
> gpu_metrics->pcie_bandwidth_inst =
[PATCH] drm/amd/pm: Skip reporting pcie width/speed on vfs
Skip reporting pcie link width/speed on vfs for
smu_v13_0_6 & smu_v13_0_2
Signed-off-by: Asad Kamal
Reviewed-by: Yang Wang
---
.../gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 10 ++
.../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c | 18 ++
2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
index f1440869d1ce..f122ef49106c 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -1747,10 +1747,12 @@ static ssize_t aldebaran_get_gpu_metrics(struct
smu_context *smu,
gpu_metrics->current_fan_speed = 0;
- gpu_metrics->pcie_link_width =
- smu_v13_0_get_current_pcie_link_width(smu);
- gpu_metrics->pcie_link_speed =
- aldebaran_get_current_pcie_link_speed(smu);
+ if (!amdgpu_sriov_vf(smu->adev)) {
+ gpu_metrics->pcie_link_width =
+ smu_v13_0_get_current_pcie_link_width(smu);
+ gpu_metrics->pcie_link_speed =
+ aldebaran_get_current_pcie_link_speed(smu);
+ }
gpu_metrics->system_clock_counter = ktime_get_boottime_ns();
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
index 2b7a60b23d6b..69c64bc6e2dc 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
@@ -2229,14 +2229,16 @@ static ssize_t smu_v13_0_6_get_gpu_metrics(struct
smu_context *smu, void **table
gpu_metrics->gfxclk_lock_status = GET_METRIC_FIELD(GfxLockXCDMak) >>
GET_INST(GC, 0);
if (!(adev->flags & AMD_IS_APU)) {
- link_width_level =
smu_v13_0_6_get_current_pcie_link_width_level(smu);
- if (link_width_level > MAX_LINK_WIDTH)
- link_width_level = 0;
-
- gpu_metrics->pcie_link_width =
- DECODE_LANE_WIDTH(link_width_level);
- gpu_metrics->pcie_link_speed =
- smu_v13_0_6_get_current_pcie_link_speed(smu);
+ if (!amdgpu_sriov_vf(adev)) {
+ link_width_level =
smu_v13_0_6_get_current_pcie_link_width_level(smu);
+ if (link_width_level > MAX_LINK_WIDTH)
+ link_width_level = 0;
+
+ gpu_metrics->pcie_link_width =
+ DECODE_LANE_WIDTH(link_width_level);
+ gpu_metrics->pcie_link_speed =
+ smu_v13_0_6_get_current_pcie_link_speed(smu);
+ }
gpu_metrics->pcie_bandwidth_acc =
SMUQ10_ROUND(metrics_x->PcieBandwidthAcc[0]);
gpu_metrics->pcie_bandwidth_inst =
--
2.42.0
Re: [PATCH 00/13] drm: Fix reservation locking for pin/unpin and console
On Tue, Feb 27, 2024 at 6:38 AM Thomas Zimmermann wrote:
>
> Dma-buf locking semantics require the caller of pin and unpin to hold
> the buffer's reservation lock. Fix DRM to adhere to the specs. This
> enables to fix the locking in DRM's console emulation. Similar changes
> for vmap and mmap have been posted at [1][2]
>
> Most DRM drivers and memory managers acquire the buffer object's
> reservation lock within their GEM pin and unpin callbacks. This
> violates dma-buf locking semantics. We get away with it because PRIME
> does not provide pin/unpin, but attach/detach, for which the locking
> semantics is correct.
>
> Patches 1 to 8 rework DRM GEM code in various implementations to
> acquire the reservation lock when entering the pin and unpin callbacks.
> This prepares them for the next patch. Drivers that are not affected
> by these patches either don't acquire the reservation lock (amdgpu)
> or don't need preparation (loongson).
>
> Patch 9 moves reservation locking from the GEM pin/unpin callbacks
> into drm_gem_pin() and drm_gem_unpin(). As PRIME uses these functions
> internally it still gets the reservation lock.
>
> With the updated GEM callbacks, the rest of the patchset fixes the
> fbdev emulation's buffer locking. Fbdev emulation needs to keep its
> GEM buffer object inplace while updating its content. This required
> a implicit pinning and apparently amdgpu didn't do this at all.
>
> Patch 10 introduces drm_client_buffer_vmap_local() and _vunmap_local().
> The former function map a GEM buffer into the kernel's address space
> with regular vmap operations, but keeps holding the reservation lock.
> The _vunmap_local() helper undoes the vmap and releases the lock. The
> updated GEM callbacks make this possible. Between the two calls, the
> fbdev emulation can update the buffer content without have the buffer
> moved or evicted. Update fbdev-generic to use vmap_local helpers,
> which fix amdgpu. The idea of adding a "local vmap" has previously been
> attempted at [3] in a different form.
>
> Patch 11 adds implicit pinning to the DRM client's regular vmap
> helper so that long-term vmap'ed buffers won't be evicted. This only
> affects fbdev-dma, but GEM DMA helpers don't require pinning. So
> there are no practical changes.
>
> Patches 12 and 13 remove implicit pinning from the vmap and vunmap
> operations in gem-vram and qxl. These pin operations are not supposed
> to be part of vmap code, but were required to keep the buffers in place
> for fbdev emulation. With the conversion o ffbdev-generic to to
> vmap_local helpers, that code can finally be removed.
>
> Tested with amdgpu, nouveau, radeon, simpledrm and vc4.
>
> [1] https://patchwork.freedesktop.org/series/106371/
> [2] https://patchwork.freedesktop.org/series/116001/
> [3] https://patchwork.freedesktop.org/series/84732/
>
> Thomas Zimmermann (13):
> drm/gem-shmem: Acquire reservation lock in GEM pin/unpin callbacks
> drm/gem-vram: Acquire reservation lock in GEM pin/unpin callbacks
> drm/msm: Provide msm_gem_get_pages_locked()
> drm/msm: Acquire reservation lock in GEM pin/unpin callback
> drm/nouveau: Provide nouveau_bo_{pin,unpin}_locked()
> drm/nouveau: Acquire reservation lock in GEM pin/unpin callbacks
> drm/qxl: Provide qxl_bo_{pin,unpin}_locked()
> drm/qxl: Acquire reservation lock in GEM pin/unpin callbacks
> drm/gem: Acquire reservation lock in drm_gem_{pin/unpin}()
> drm/fbdev-generic: Fix locking with drm_client_buffer_vmap_local()
> drm/client: Pin vmap'ed GEM buffers
> drm/gem-vram: Do not pin buffer objects for vmap
> drm/qxl: Do not pin buffer objects for vmap
>
> drivers/gpu/drm/drm_client.c| 92 ++---
> drivers/gpu/drm/drm_fbdev_generic.c | 4 +-
> drivers/gpu/drm/drm_gem.c | 34 +++-
> drivers/gpu/drm/drm_gem_shmem_helper.c | 6 +-
> drivers/gpu/drm/drm_gem_vram_helper.c | 101 ++--
> drivers/gpu/drm/drm_internal.h | 2 +
> drivers/gpu/drm/loongson/lsdc_gem.c | 13 +--
> drivers/gpu/drm/msm/msm_gem.c | 20 ++---
> drivers/gpu/drm/msm/msm_gem.h | 4 +-
> drivers/gpu/drm/msm/msm_gem_prime.c | 20 +++--
> drivers/gpu/drm/nouveau/nouveau_bo.c| 43 +++---
> drivers/gpu/drm/nouveau/nouveau_bo.h| 2 +
> drivers/gpu/drm/nouveau/nouveau_prime.c | 8 +-
> drivers/gpu/drm/qxl/qxl_object.c| 26 +++---
> drivers/gpu/drm/qxl/qxl_object.h| 2 +
> drivers/gpu/drm/qxl/qxl_prime.c | 4 +-
> drivers/gpu/drm/radeon/radeon_prime.c | 11 ---
> drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 25 ++
> include/drm/drm_client.h| 10 +++
> include/drm/drm_gem.h | 3 +
> include/drm/drm_gem_shmem_helper.h | 7 +-
> 21 files changed, 265 insertions(+), 172 deletions(-)
>
>
> base-commit: 7291e2e67dff0ff573900266382c9c9248a7dea5
> prerequisite-patch-id: bdfa0e6341b30cc9d7647172760b3473007c1216
>
Re: [PATCH 09/13] drm/gem: Acquire reservation lock in drm_gem_{pin/unpin}()
On Tue, Feb 27, 2024 at 6:39 AM Thomas Zimmermann wrote:
>
> Acquire the buffer object's reservation lock in drm_gem_pin() and
> remove locking the drivers' GEM callbacks where necessary. Same for
> unpin().
>
> DRM drivers and memory managers modified by this patch will now have
> correct dma-buf locking semantics: the caller is responsible for
> holding the reservation lock when calling the pin or unpin callback.
>
> DRM drivers and memory managers that are not modified will now be
> protected against concurent invocation of their pin and unpin callbacks.
>
> PRIME does not implement struct dma_buf_ops.pin, which requires
> the caller to hold the reservation lock. It does implement struct
> dma_buf_ops.attach, which requires to callee to acquire the
> reservation lock. The PRIME code uses drm_gem_pin(), so locks
> are now taken as specified. Same for unpin and detach.
>
> The patch harmonizes GEM pin and unpin to have non-interruptible
> reservation locking across all drivers, as is already the case for
> vmap and vunmap. This affects gem-shmem, gem-vram, loongson, qxl and
> radeon.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_gem.c | 22 --
> drivers/gpu/drm/drm_gem_vram_helper.c | 15 +--
> drivers/gpu/drm/drm_internal.h | 2 ++
> drivers/gpu/drm/loongson/lsdc_gem.c | 13 ++---
> drivers/gpu/drm/msm/msm_gem_prime.c | 4
> drivers/gpu/drm/nouveau/nouveau_prime.c | 11 ---
> drivers/gpu/drm/qxl/qxl_prime.c | 14 +-
> drivers/gpu/drm/radeon/radeon_prime.c | 11 ---
> drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 25 ++---
> include/drm/drm_gem_shmem_helper.h | 11 +--
> 10 files changed, 33 insertions(+), 95 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 44a948b80ee14..e0f80c6a7096f 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1161,7 +1161,7 @@ void drm_gem_print_info(struct drm_printer *p, unsigned
> int indent,
> obj->funcs->print_info(p, indent, obj);
> }
>
> -int drm_gem_pin(struct drm_gem_object *obj)
> +int drm_gem_pin_locked(struct drm_gem_object *obj)
> {
> if (obj->funcs->pin)
> return obj->funcs->pin(obj);
> @@ -1169,12 +1169,30 @@ int drm_gem_pin(struct drm_gem_object *obj)
> return 0;
> }
>
> -void drm_gem_unpin(struct drm_gem_object *obj)
> +void drm_gem_unpin_locked(struct drm_gem_object *obj)
> {
> if (obj->funcs->unpin)
> obj->funcs->unpin(obj);
> }
>
> +int drm_gem_pin(struct drm_gem_object *obj)
> +{
> + int ret;
> +
> + dma_resv_lock(obj->resv, NULL);
> + ret = drm_gem_pin_locked(obj);
> + dma_resv_unlock(obj->resv);
> +
> + return ret;
> +}
> +
> +void drm_gem_unpin(struct drm_gem_object *obj)
> +{
> + dma_resv_lock(obj->resv, NULL);
> + drm_gem_unpin_locked(obj);
> + dma_resv_unlock(obj->resv);
> +}
> +
> int drm_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map)
> {
> int ret;
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
> b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 15029d89badf8..5a16b3e0a4134 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -774,11 +774,6 @@
> EXPORT_SYMBOL(drm_gem_vram_simple_display_pipe_cleanup_fb);
> static int drm_gem_vram_object_pin(struct drm_gem_object *gem)
> {
> struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
> - int ret;
> -
> - ret = ttm_bo_reserve(>bo, true, false, NULL);
> - if (ret)
> - return ret;
>
> /*
> * Fbdev console emulation is the use case of these PRIME
> @@ -789,10 +784,7 @@ static int drm_gem_vram_object_pin(struct drm_gem_object
> *gem)
> * the buffer to be pinned to VRAM, implement a callback that
> * sets the flags accordingly.
> */
> - ret = drm_gem_vram_pin_locked(gbo, 0);
> - ttm_bo_unreserve(>bo);
> -
> - return ret;
> + return drm_gem_vram_pin_locked(gbo, 0);
> }
>
> /**
> @@ -803,13 +795,8 @@ static int drm_gem_vram_object_pin(struct drm_gem_object
> *gem)
> static void drm_gem_vram_object_unpin(struct drm_gem_object *gem)
> {
> struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
> - int ret;
>
> - ret = ttm_bo_reserve(>bo, true, false, NULL);
> - if (ret)
> - return;
> drm_gem_vram_unpin_locked(gbo);
> - ttm_bo_unreserve(>bo);
> }
>
> /**
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 8e4faf0a28e6c..40b2d3a274d6c 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -170,6 +170,8 @@ void drm_gem_release(struct drm_device *dev, struct
> drm_file *file_private);
> void drm_gem_print_info(struct drm_printer
Re: [PATCH 08/13] drm/qxl: Acquire reservation lock in GEM pin/unpin callbacks
On Tue, Feb 27, 2024 at 6:39 AM Thomas Zimmermann wrote:
>
> Acquire the reservation lock directly in GEM pin callback. Same for
> unpin. Prepares for further changes.
>
> Dma-buf locking semantics require callers to hold the buffer's
> reservation lock when invoking the pin and unpin callbacks. Prepare
> qxl accordingly by pushing locking out of the implementation. A
> follow-up patch will fix locking for all GEM code at once.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/qxl/qxl_prime.c | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
> index 9169c26357d36..f2646603e12eb 100644
> --- a/drivers/gpu/drm/qxl/qxl_prime.c
> +++ b/drivers/gpu/drm/qxl/qxl_prime.c
> @@ -31,15 +31,27 @@
> int qxl_gem_prime_pin(struct drm_gem_object *obj)
> {
> struct qxl_bo *bo = gem_to_qxl_bo(obj);
> + int r;
>
> - return qxl_bo_pin(bo);
> + r = qxl_bo_reserve(bo);
> + if (r)
> + return r;
> + r = qxl_bo_pin_locked(bo);
> + qxl_bo_unreserve(bo);
> +
> + return r;
> }
>
> void qxl_gem_prime_unpin(struct drm_gem_object *obj)
> {
> struct qxl_bo *bo = gem_to_qxl_bo(obj);
> + int r;
>
> - qxl_bo_unpin(bo);
> + r = qxl_bo_reserve(bo);
> + if (r)
> + return;
> + qxl_bo_unpin_locked(bo);
> + qxl_bo_unreserve(bo);
> }
It looks like gem_prime_pin/unpin is largely the same between a lot of
drivers now. That might be a nice cleanup in the future.
z
Please apply commit 5b750b22530fe53bf7fd6a30baacd53ada26911b to linux-6.1.y
Hi Greg and Sasha,
Please apply upstream commit 5b750b22530f ("drm/amd/display: Increase
frame warning limit with KASAN or KCSAN in dml") to linux-6.1.y, as it
is needed to avoid instances of -Wframe-larger-than in allmodconfig,
which has -Werror enabled. It applies cleanly for me and it is already
in 6.6 and 6.7. The fixes tag is not entirely accurate and commit
e63e35f0164c ("drm/amd/display: Increase frame-larger-than for all
display_mode_vba files"), which was recently applied to that tree,
depends on it (I should have made that clearer in the patch).
If there are any issues, please let me know.
Cheers,
Nathan
DisplayPort: handling of HPD events / link training
Hello, We are currently looking at checking and/or possibly redesigning the way the MSM DRM driver handles the HPD events and link training. After a quick glance at the drivers implementing DP support, I noticed following main approaches: - Perform link training at the atomic_enable time, don't report failures (mtk, analogix, zynqmp, tegra, nouveau) - Perform link training at the atomic_enable time, report errors using link_status property (i915, mhdp8546) - Perform link training on the plug event (msm, it8605). - Perform link training from the DPMS handler, also calling it from the enable callback (AMDGPU, radeon). It looks like the majority wins and we should move HPD to atomic_enable time. Is that assumption correct? Also two related questions: - Is there a plan to actually make use of the link_status property? Intel presented it at FOSDEM 2018, but since that time it was not picked up by other drivers. - Is there any plan to create generic DP link training helpers? After glancing through the DP drivers there is a lot of similar code in the link training functions, with minor differences here and there. And it's those minor differences that bug me. It means that drivers might respond differently to similar devices. Or that there might be minor bugs here and there. -- With best wishes Dmitry
Re: [PATCH 1/2] Revert "drm/amd: Remove freesync video mode amdgpu parameter"
On 2/27/24 13:48, Alex Deucher wrote: This reverts commit e94e787e37b99645e7c02d20d0a1ba0f8a18a82a. This conflicts with how compositors want to handle VRR. Now that compositors actually handle VRR, we probably don't need freesync video. Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2985 Signed-off-by: Alex Deucher Series is: Acked-by: Hamza Mahfooz --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 27 + 2 files changed, 28 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 0e365cadcc3fc..925026c183f41 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -194,6 +194,7 @@ extern int amdgpu_emu_mode; extern uint amdgpu_smu_memory_pool_size; extern int amdgpu_smu_pptable_id; extern uint amdgpu_dc_feature_mask; +extern uint amdgpu_freesync_vid_mode; extern uint amdgpu_dc_debug_mask; extern uint amdgpu_dc_visual_confirm; extern int amdgpu_dm_abm_level; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 15a8a64fc4e28..82b154b103f43 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -199,6 +199,7 @@ int amdgpu_mes_kiq; int amdgpu_noretry = -1; int amdgpu_force_asic_type = -1; int amdgpu_tmz = -1; /* auto */ +uint amdgpu_freesync_vid_mode; int amdgpu_reset_method = -1; /* auto */ int amdgpu_num_kcq = -1; int amdgpu_smartshift_bias; @@ -883,6 +884,32 @@ module_param_named(damageclips, amdgpu_damage_clips, int, 0444); MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto (default), 0 = off, 1 = on)"); module_param_named(tmz, amdgpu_tmz, int, 0444); +/** + * DOC: freesync_video (uint) + * Enable the optimization to adjust front porch timing to achieve seamless + * mode change experience when setting a freesync supported mode for which full + * modeset is not needed. + * + * The Display Core will add a set of modes derived from the base FreeSync + * video mode into the corresponding connector's mode list based on commonly + * used refresh rates and VRR range of the connected display, when users enable + * this feature. From the userspace perspective, they can see a seamless mode + * change experience when the change between different refresh rates under the + * same resolution. Additionally, userspace applications such as Video playback + * can read this modeset list and change the refresh rate based on the video + * frame rate. Finally, the userspace can also derive an appropriate mode for a + * particular refresh rate based on the FreeSync Mode and add it to the + * connector's mode list. + * + * Note: This is an experimental feature. + * + * The default value: 0 (off). + */ +MODULE_PARM_DESC( + freesync_video, + "Enable freesync modesetting optimization feature (0 = off (default), 1 = on)"); +module_param_named(freesync_video, amdgpu_freesync_vid_mode, uint, 0444); + /** * DOC: reset_method (int) * GPU reset method (-1 = auto (default), 0 = legacy, 1 = mode0, 2 = mode1, 3 = mode2, 4 = baco) -- Hamza
[PATCH] drm/amd/display: check dc_link before dereferencing
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6683
amdgpu_dm_connector_funcs_force()
warn: variable dereferenced before check 'dc_link' (see line 6663)
Fixes: 967176179215 ("drm/amd/display: fix null-pointer dereference on edid
reading")
Reported-by: Dan Carpenter
Signed-off-by: Melissa Wen
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 32efce81a5a7..46dd06e8fc7e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6653,7 +6653,7 @@ static void amdgpu_dm_connector_funcs_force(struct
drm_connector *connector)
struct edid *edid;
struct i2c_adapter *ddc;
- if (dc_link->aux_mode)
+ if (dc_link && dc_link->aux_mode)
ddc = >dm_dp_aux.aux.ddc;
else
ddc = >i2c->base;
--
2.43.0
[PATCH 2/2] Reapply "Revert drm/amd/display: Enable Freesync Video Mode by default"
This reverts commit 11b92df8a2f7f4605ccc764ce6ae4a72760674df.
This conflicts with how compositors want to handle VRR. Now
that compositors actually handle VRR, we probably don't need
freesync video.
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2985
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 5765e492c4557..2c283c47bcf6a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6230,7 +6230,8 @@ create_stream_for_sink(struct drm_connector *connector,
*/
DRM_DEBUG_DRIVER("No preferred mode found\n");
} else if (aconnector) {
- recalculate_timing = is_freesync_video_mode(, aconnector);
+ recalculate_timing = amdgpu_freesync_vid_mode &&
+is_freesync_video_mode(, aconnector);
if (recalculate_timing) {
freesync_mode =
get_highest_refresh_rate_mode(aconnector, false);
drm_mode_copy(_mode, );
@@ -7541,7 +7542,7 @@ static void amdgpu_dm_connector_add_freesync_modes(struct
drm_connector *connect
struct amdgpu_dm_connector *amdgpu_dm_connector =
to_amdgpu_dm_connector(connector);
- if (!edid)
+ if (!(amdgpu_freesync_vid_mode && edid))
return;
if (amdgpu_dm_connector->max_vfreq - amdgpu_dm_connector->min_vfreq >
10)
@@ -9835,7 +9836,8 @@ static int dm_update_crtc_state(struct
amdgpu_display_manager *dm,
* TODO: Refactor this function to allow this check to work
* in all conditions.
*/
- if (dm_new_crtc_state->stream &&
+ if (amdgpu_freesync_vid_mode &&
+ dm_new_crtc_state->stream &&
is_timing_unchanged_for_freesync(new_crtc_state,
old_crtc_state))
goto skip_modeset;
@@ -9875,7 +9877,7 @@ static int dm_update_crtc_state(struct
amdgpu_display_manager *dm,
}
/* Now check if we should set freesync video mode */
- if (dm_new_crtc_state->stream &&
+ if (amdgpu_freesync_vid_mode && dm_new_crtc_state->stream &&
dc_is_stream_unchanged(new_stream,
dm_old_crtc_state->stream) &&
dc_is_stream_scaling_unchanged(new_stream,
dm_old_crtc_state->stream) &&
is_timing_unchanged_for_freesync(new_crtc_state,
@@ -9888,7 +9890,7 @@ static int dm_update_crtc_state(struct
amdgpu_display_manager *dm,
set_freesync_fixed_config(dm_new_crtc_state);
goto skip_modeset;
- } else if (aconnector &&
+ } else if (amdgpu_freesync_vid_mode && aconnector &&
is_freesync_video_mode(_crtc_state->mode,
aconnector)) {
struct drm_display_mode *high_mode;
--
2.43.2
[PATCH 1/2] Revert "drm/amd: Remove freesync video mode amdgpu parameter"
This reverts commit e94e787e37b99645e7c02d20d0a1ba0f8a18a82a. This conflicts with how compositors want to handle VRR. Now that compositors actually handle VRR, we probably don't need freesync video. Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2985 Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 27 + 2 files changed, 28 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 0e365cadcc3fc..925026c183f41 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -194,6 +194,7 @@ extern int amdgpu_emu_mode; extern uint amdgpu_smu_memory_pool_size; extern int amdgpu_smu_pptable_id; extern uint amdgpu_dc_feature_mask; +extern uint amdgpu_freesync_vid_mode; extern uint amdgpu_dc_debug_mask; extern uint amdgpu_dc_visual_confirm; extern int amdgpu_dm_abm_level; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 15a8a64fc4e28..82b154b103f43 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -199,6 +199,7 @@ int amdgpu_mes_kiq; int amdgpu_noretry = -1; int amdgpu_force_asic_type = -1; int amdgpu_tmz = -1; /* auto */ +uint amdgpu_freesync_vid_mode; int amdgpu_reset_method = -1; /* auto */ int amdgpu_num_kcq = -1; int amdgpu_smartshift_bias; @@ -883,6 +884,32 @@ module_param_named(damageclips, amdgpu_damage_clips, int, 0444); MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto (default), 0 = off, 1 = on)"); module_param_named(tmz, amdgpu_tmz, int, 0444); +/** + * DOC: freesync_video (uint) + * Enable the optimization to adjust front porch timing to achieve seamless + * mode change experience when setting a freesync supported mode for which full + * modeset is not needed. + * + * The Display Core will add a set of modes derived from the base FreeSync + * video mode into the corresponding connector's mode list based on commonly + * used refresh rates and VRR range of the connected display, when users enable + * this feature. From the userspace perspective, they can see a seamless mode + * change experience when the change between different refresh rates under the + * same resolution. Additionally, userspace applications such as Video playback + * can read this modeset list and change the refresh rate based on the video + * frame rate. Finally, the userspace can also derive an appropriate mode for a + * particular refresh rate based on the FreeSync Mode and add it to the + * connector's mode list. + * + * Note: This is an experimental feature. + * + * The default value: 0 (off). + */ +MODULE_PARM_DESC( + freesync_video, + "Enable freesync modesetting optimization feature (0 = off (default), 1 = on)"); +module_param_named(freesync_video, amdgpu_freesync_vid_mode, uint, 0444); + /** * DOC: reset_method (int) * GPU reset method (-1 = auto (default), 0 = legacy, 1 = mode0, 2 = mode1, 3 = mode2, 4 = baco) -- 2.43.2
Re: [PATCH 00/13] drm: Fix reservation locking for pin/unpin and console
Am 27.02.24 um 19:14 schrieb Dmitry Osipenko: Hello, Thank you for the patches! On 2/27/24 13:14, Thomas Zimmermann wrote: Dma-buf locking semantics require the caller of pin and unpin to hold the buffer's reservation lock. Fix DRM to adhere to the specs. This enables to fix the locking in DRM's console emulation. Similar changes for vmap and mmap have been posted at [1][2] Most DRM drivers and memory managers acquire the buffer object's reservation lock within their GEM pin and unpin callbacks. This violates dma-buf locking semantics. We get away with it because PRIME does not provide pin/unpin, but attach/detach, for which the locking semantics is correct. Patches 1 to 8 rework DRM GEM code in various implementations to acquire the reservation lock when entering the pin and unpin callbacks. This prepares them for the next patch. Drivers that are not affected by these patches either don't acquire the reservation lock (amdgpu) or don't need preparation (loongson). Patch 9 moves reservation locking from the GEM pin/unpin callbacks into drm_gem_pin() and drm_gem_unpin(). As PRIME uses these functions internally it still gets the reservation lock. With the updated GEM callbacks, the rest of the patchset fixes the fbdev emulation's buffer locking. Fbdev emulation needs to keep its GEM buffer object inplace while updating its content. This required a implicit pinning and apparently amdgpu didn't do this at all. Patch 10 introduces drm_client_buffer_vmap_local() and _vunmap_local(). The former function map a GEM buffer into the kernel's address space with regular vmap operations, but keeps holding the reservation lock. The _vunmap_local() helper undoes the vmap and releases the lock. The updated GEM callbacks make this possible. Between the two calls, the fbdev emulation can update the buffer content without have the buffer moved or evicted. Update fbdev-generic to use vmap_local helpers, which fix amdgpu. The idea of adding a "local vmap" has previously been attempted at [3] in a different form. Patch 11 adds implicit pinning to the DRM client's regular vmap helper so that long-term vmap'ed buffers won't be evicted. This only affects fbdev-dma, but GEM DMA helpers don't require pinning. So there are no practical changes. Patches 12 and 13 remove implicit pinning from the vmap and vunmap operations in gem-vram and qxl. These pin operations are not supposed to be part of vmap code, but were required to keep the buffers in place for fbdev emulation. With the conversion o ffbdev-generic to to vmap_local helpers, that code can finally be removed. Isn't it a common behaviour for all DRM drivers to implicitly pin BO while it's vmapped? I was sure it should be common /o\ No, at least amdgpu and radon doesn't pin kmapped BOs and I don't think nouveau does either. Why would you want to kmap BO that isn't pinned? The usual use case is to call the ttm kmap function when you need CPU access. When the buffer hasn't moved we can use the cached CPU mapping, if the buffer has moved since the last time or this is the first time that is called we setup a new mapping. Shouldn't TTM's vmap() be changed to do the pinning? Absolutely not, no. That would break tons of use cases. Regards, Christian. I missed that TTM doesn't pin BO on vmap() and now surprised to see it. It should be a rather serious problem requiring backporting of the fixes, but I don't see the fixes tags on the patches (?)
[PATCH] drm/amd/display: Add monitor patch for specific eDP
From: Ivan Lipski
[WHY]
Some eDP panels's ext caps don't write initial value cause the value of
dpcd_addr(0x317) is random. It means that sometimes the eDP will
clarify it is OLED, miniLED...etc cause the backlight control interface
is incorrect.
[HOW]
Add a new panel patch to remove sink ext caps(HDR,OLED...etc)
Cc: sta...@vger.kernel.org # 6.5.x
Cc: Hamza Mahfooz
Cc: Tsung-hua Lin
Cc: Chris Chi
Cc: Harry Wentland
Tested-by: Daniel Wheeler
Reviewed-by: Sun peng Li
Acked-by: Rodrigo Siqueira
Signed-off-by: Ivan Lipski
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index d9a482908380..764dc3ffd91b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -63,6 +63,12 @@ static void apply_edid_quirks(struct edid *edid, struct
dc_edid_caps *edid_caps)
DRM_DEBUG_DRIVER("Disabling FAMS on monitor with panel id
%X\n", panel_id);
edid_caps->panel_patch.disable_fams = true;
break;
+ /* Workaround for some monitors that do not clear DPCD 0x317 if
FreeSync is unsupported */
+ case drm_edid_encode_panel_id('A', 'U', 'O', 0xA7AB):
+ case drm_edid_encode_panel_id('A', 'U', 'O', 0xE69B):
+ DRM_DEBUG_DRIVER("Clearing DPCD 0x317 on monitor with panel id
%X\n", panel_id);
+ edid_caps->panel_patch.remove_sink_ext_caps = true;
+ break;
default:
return;
}
--
2.43.0
[PATCH] drm/amdgpu: Fix multiple truncation issues in multiple driver files
Fixes snprintf function by writing more bytes into various buffers than they can hold. In several files - smu_v13_0.c, gfx_v11_0.c, gfx_v10_0.c, gfx_v9_0.c, and amdgpu_mes.c. They were related to different directives, such as '%s', '_pfp.bin', '_me.bin', '_rlc.bin', '_mec.bin', '_mec2', and '_mes.bin'. The buffers sizes have been adjusted to accommodate the maximum possible string size. Fixes the below with gcc W=1: drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu13/smu_v13_0.c:108:52: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c:523:54: warning: ‘_pfp.bin’ directive output may be truncated writing 8 bytes into a region of size between 4 and 33 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c:540:54: warning: ‘_me.bin’ directive output may be truncated writing 7 bytes into a region of size between 4 and 33 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c:557:70: warning: ‘_rlc.bin’ directive output may be truncated writing 8 bytes into a region of size between 4 and 33 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c:569:54: warning: ‘_mec.bin’ directive output may be truncated writing 8 bytes into a region of size between 4 and 33 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c:3979:58: warning: ‘%s’ directive output may be truncated writing up to 4 bytes into a region of size between 0 and 29 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c:3985:57: warning: ‘%s’ directive output may be truncated writing up to 4 bytes into a region of size between 1 and 30 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c:3991:57: warning: ‘%s’ directive output may be truncated writing up to 4 bytes into a region of size between 1 and 30 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c:3998:62: warning: ‘_rlc.bin’ directive output may be truncated writing 8 bytes into a region of size between 4 and 33 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c:4014:58: warning: ‘%s’ directive output may be truncated writing up to 4 bytes into a region of size between 0 and 29 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c:4021:54: warning: ‘_mec2’ directive output may be truncated writing 5 bytes into a region of size between 4 and 33 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:1255:52: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:1261:52: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:1267:52: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:1303:60: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:1309:60: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:1311:60: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:1344:60: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:1346:60: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:1356:68: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:1358:68: warning: ‘%s’ directive output may be truncated writing up to 29 bytes into a region of size 23 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1486:66: warning: ‘%s’ directive output may be truncated writing up to 1 bytes into a region of size between 0 and 29 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1481:66: warning: ‘%s’ directive output may be truncated writing 1 byte into a region of size between 0 and 29 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1481:66: warning: ‘%s’ directive output may be truncated writing 2 bytes into a region of size between 0 and 29 [-Wformat-truncation=] drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c:1493:62: warning: ‘_mes.bin’ directive output may be truncated writing 8 bytes into a region of size between 4 and 33 [-Wformat-truncation=] Cc: Christian König Cc: Alex Deucher Signed-off-by: Srinivasan Shanmugam --- drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
[PATCH] drm/amdgpu: Removed used parameter
Also passing adev is misleading if BO is associated with different adev.
In this case BO is mapped to a different device
Signed-off-by: Harish Kasiviswanathan
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 4 ++--
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 4fb32d86cd0e..0ef223c2affb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -320,7 +320,7 @@ int amdgpu_amdkfd_gpuvm_map_gtt_bo_to_kernel(struct kgd_mem
*mem,
void **kptr, uint64_t *size);
void amdgpu_amdkfd_gpuvm_unmap_gtt_bo_from_kernel(struct kgd_mem *mem);
-int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_device *adev, struct
amdgpu_bo *bo);
+int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_bo *bo);
int amdgpu_amdkfd_gpuvm_restore_process_bos(void *process_info,
struct dma_fence __rcu **ef);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index ef71b12062a1..d0819fa5fcd8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -2189,13 +2189,13 @@ int amdgpu_amdkfd_gpuvm_sync_memory(
/**
* amdgpu_amdkfd_map_gtt_bo_to_gart - Map BO to GART and increment reference
count
- * @adev: Device to which allocated BO belongs
* @bo: Buffer object to be mapped
*
+ * BO will be mapped to GART of adev to which it is previously associated with
* Before return, bo reference count is incremented. To release the reference
and unpin/
* unmap the BO, call amdgpu_amdkfd_free_gtt_mem.
*/
-int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_device *adev, struct
amdgpu_bo *bo)
+int amdgpu_amdkfd_map_gtt_bo_to_gart(struct amdgpu_bo *bo)
{
int ret;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 824e660283b2..f030cafc5a0a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -371,7 +371,7 @@ static int kfd_ioctl_create_queue(struct file *filep,
struct kfd_process *p,
goto err_wptr_map_gart;
}
- err = amdgpu_amdkfd_map_gtt_bo_to_gart(dev->adev, wptr_bo);
+ err = amdgpu_amdkfd_map_gtt_bo_to_gart(wptr_bo);
if (err) {
pr_err("Failed to map wptr bo to GART\n");
goto err_wptr_map_gart;
--
2.34.1
[PATCH] drm/amdgpu: Fix potential truncation by increasing SMU_FW_NAME_LEN
Increases the size of SMU_FW_NAME_LEN from 0x24 (36 in decimal) to 0x2A
(42 in decimal). This change prevents truncation when the snprintf
function writes into the fw_name buffer in the smu_v11_0_init_microcode
function.
Previously, snprintf could write between 12 and 41 bytes into fw_name,
which can only hold 36 bytes. This could lead to truncation if the size
of the string is larger than 36 bytes. By increasing the size of
SMU_FW_NAME_LEN to 42, we ensure that fw_name can accommodate the
maximum possible string size.
Fixes the below with gcc W=1
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/smu_v11_0.c: In function
‘smu_v11_0_init_microcode’:
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/smu_v11_0.c:110:54: warning:
‘.bin’ directive output may be truncated writing 4 bytes into a region of size
between 0 and 29 [-Wformat-truncation=]
110 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin",
ucode_prefix);
| ^~~~
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/smu_v11_0.c:110:9: note:
‘snprintf’ output between 12 and 41 bytes into a destination of size 36
110 | snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin",
ucode_prefix);
|
^
Fixes: 6b54496238cc ("drm/amd: Convert SMUv11 microcode to use
`amdgpu_ucode_ip_version_decode`")
Cc: Mario Limonciello
Cc: Christian König
Cc: Alex Deucher
Cc: Lijo Lazar
Signed-off-by: Srinivasan Shanmugam
---
drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
index a870bdd49a4e..3d98b0e0eec2 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
@@ -35,7 +35,7 @@
#define SMU_THERMAL_MINIMUM_ALERT_TEMP 0
#define SMU_THERMAL_MAXIMUM_ALERT_TEMP 255
#define SMU_TEMPERATURE_UNITS_PER_CENTIGRADES 1000
-#define SMU_FW_NAME_LEN0x24
+#define SMU_FW_NAME_LEN0x2A
#define SMU_DPM_USER_PROFILE_RESTORE (1 << 0)
#define SMU_CUSTOM_FAN_SPEED_RPM (1 << 1)
--
2.34.1
Re: [PATCH 00/13] drm: Fix reservation locking for pin/unpin and console
Hi
Am 27.02.24 um 15:03 schrieb Christian König:
Nice, looks totally valid to me.
Feel free to add to patch #2, #9, #10, #11 and #12 Reviewed-by:
Christian König
And Acked-by: Christian König to the rest.
Oh, wow. That was quick! Thanks a lot.
Best regards
Thomas
Regards,
Christian.
Am 27.02.24 um 11:14 schrieb Thomas Zimmermann:
Dma-buf locking semantics require the caller of pin and unpin to hold
the buffer's reservation lock. Fix DRM to adhere to the specs. This
enables to fix the locking in DRM's console emulation. Similar changes
for vmap and mmap have been posted at [1][2]
Most DRM drivers and memory managers acquire the buffer object's
reservation lock within their GEM pin and unpin callbacks. This
violates dma-buf locking semantics. We get away with it because PRIME
does not provide pin/unpin, but attach/detach, for which the locking
semantics is correct.
Patches 1 to 8 rework DRM GEM code in various implementations to
acquire the reservation lock when entering the pin and unpin callbacks.
This prepares them for the next patch. Drivers that are not affected
by these patches either don't acquire the reservation lock (amdgpu)
or don't need preparation (loongson).
Patch 9 moves reservation locking from the GEM pin/unpin callbacks
into drm_gem_pin() and drm_gem_unpin(). As PRIME uses these functions
internally it still gets the reservation lock.
With the updated GEM callbacks, the rest of the patchset fixes the
fbdev emulation's buffer locking. Fbdev emulation needs to keep its
GEM buffer object inplace while updating its content. This required
a implicit pinning and apparently amdgpu didn't do this at all.
Patch 10 introduces drm_client_buffer_vmap_local() and _vunmap_local().
The former function map a GEM buffer into the kernel's address space
with regular vmap operations, but keeps holding the reservation lock.
The _vunmap_local() helper undoes the vmap and releases the lock. The
updated GEM callbacks make this possible. Between the two calls, the
fbdev emulation can update the buffer content without have the buffer
moved or evicted. Update fbdev-generic to use vmap_local helpers,
which fix amdgpu. The idea of adding a "local vmap" has previously been
attempted at [3] in a different form.
Patch 11 adds implicit pinning to the DRM client's regular vmap
helper so that long-term vmap'ed buffers won't be evicted. This only
affects fbdev-dma, but GEM DMA helpers don't require pinning. So
there are no practical changes.
Patches 12 and 13 remove implicit pinning from the vmap and vunmap
operations in gem-vram and qxl. These pin operations are not supposed
to be part of vmap code, but were required to keep the buffers in place
for fbdev emulation. With the conversion o ffbdev-generic to to
vmap_local helpers, that code can finally be removed.
Tested with amdgpu, nouveau, radeon, simpledrm and vc4.
[1] https://patchwork.freedesktop.org/series/106371/
[2] https://patchwork.freedesktop.org/series/116001/
[3] https://patchwork.freedesktop.org/series/84732/
Thomas Zimmermann (13):
drm/gem-shmem: Acquire reservation lock in GEM pin/unpin callbacks
drm/gem-vram: Acquire reservation lock in GEM pin/unpin callbacks
drm/msm: Provide msm_gem_get_pages_locked()
drm/msm: Acquire reservation lock in GEM pin/unpin callback
drm/nouveau: Provide nouveau_bo_{pin,unpin}_locked()
drm/nouveau: Acquire reservation lock in GEM pin/unpin callbacks
drm/qxl: Provide qxl_bo_{pin,unpin}_locked()
drm/qxl: Acquire reservation lock in GEM pin/unpin callbacks
drm/gem: Acquire reservation lock in drm_gem_{pin/unpin}()
drm/fbdev-generic: Fix locking with drm_client_buffer_vmap_local()
drm/client: Pin vmap'ed GEM buffers
drm/gem-vram: Do not pin buffer objects for vmap
drm/qxl: Do not pin buffer objects for vmap
drivers/gpu/drm/drm_client.c | 92 ++---
drivers/gpu/drm/drm_fbdev_generic.c | 4 +-
drivers/gpu/drm/drm_gem.c | 34 +++-
drivers/gpu/drm/drm_gem_shmem_helper.c | 6 +-
drivers/gpu/drm/drm_gem_vram_helper.c | 101 ++--
drivers/gpu/drm/drm_internal.h | 2 +
drivers/gpu/drm/loongson/lsdc_gem.c | 13 +--
drivers/gpu/drm/msm/msm_gem.c | 20 ++---
drivers/gpu/drm/msm/msm_gem.h | 4 +-
drivers/gpu/drm/msm/msm_gem_prime.c | 20 +++--
drivers/gpu/drm/nouveau/nouveau_bo.c | 43 +++---
drivers/gpu/drm/nouveau/nouveau_bo.h | 2 +
drivers/gpu/drm/nouveau/nouveau_prime.c | 8 +-
drivers/gpu/drm/qxl/qxl_object.c | 26 +++---
drivers/gpu/drm/qxl/qxl_object.h | 2 +
drivers/gpu/drm/qxl/qxl_prime.c | 4 +-
drivers/gpu/drm/radeon/radeon_prime.c | 11 ---
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 25 ++
include/drm/drm_client.h | 10 +++
include/drm/drm_gem.h | 3 +
include/drm/drm_gem_shmem_helper.h | 7 +-
21 files changed, 265 insertions(+), 172
Re: [RFC PATCH v4 06/42] drm/vkms: Add kunit tests for VKMS LUT handling
On 2024-02-27 07:14, Arthur Grillo wrote:
>
>
> On 26/02/24 18:10, Harry Wentland wrote:
>> Debugging LUT math is much easier when we can unit test
>> it. Add kunit functionality to VKMS and add tests for
>> - get_lut_index
>> - lerp_u16
>>
>> v4:
>> - Test the critical points of the lerp function (Pekka)
>>
>> v3:
>> - Use include way of testing static functions (Arthur)
>>
>> Signed-off-by: Harry Wentland
>> Cc: Arthur Grillo
>> ---
>> drivers/gpu/drm/vkms/Kconfig | 5 +
>> drivers/gpu/drm/vkms/tests/.kunitconfig | 4 +
>> drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 163 ++
>> drivers/gpu/drm/vkms/vkms_composer.c | 8 +-
>> 4 files changed, 178 insertions(+), 2 deletions(-)
>> create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig
>> create mode 100644 drivers/gpu/drm/vkms/tests/vkms_color_tests.c
>>
>> diff --git a/drivers/gpu/drm/vkms/Kconfig b/drivers/gpu/drm/vkms/Kconfig
>> index b9ecdebecb0b..c1f8b343ff0e 100644
>> --- a/drivers/gpu/drm/vkms/Kconfig
>> +++ b/drivers/gpu/drm/vkms/Kconfig
>> @@ -13,3 +13,8 @@ config DRM_VKMS
>>a VKMS.
>>
>>If M is selected the module will be called vkms.
>> +
>> +config DRM_VKMS_KUNIT_TESTS
>> +tristate "Tests for VKMS" if !KUNIT_ALL_TESTS
>> +depends on DRM_VKMS && KUNIT
>> +default KUNIT_ALL_TESTS
>> diff --git a/drivers/gpu/drm/vkms/tests/.kunitconfig
>> b/drivers/gpu/drm/vkms/tests/.kunitconfig
>> new file mode 100644
>> index ..70e378228cbd
>> --- /dev/null
>> +++ b/drivers/gpu/drm/vkms/tests/.kunitconfig
>> @@ -0,0 +1,4 @@
>> +CONFIG_KUNIT=y
>> +CONFIG_DRM=y
>> +CONFIG_DRM_VKMS=y
>> +CONFIG_DRM_VKMS_KUNIT_TESTS=y
>> diff --git a/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
>> b/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
>> new file mode 100644
>> index ..fc73e48aa57c
>> --- /dev/null
>> +++ b/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
>> @@ -0,0 +1,163 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +
>> +#include
>> +
>> +#include
>> +
>> +#define TEST_LUT_SIZE 16
>> +
>> +static struct drm_color_lut test_linear_array[TEST_LUT_SIZE] = {
>> +{ 0x0, 0x0, 0x0, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +{ 0x, 0x, 0x, 0 },
>> +};
>> +
>> +const struct vkms_color_lut test_linear_lut = {
>> +.base = test_linear_array,
>> +.lut_length = TEST_LUT_SIZE,
>> +.channel_value2index_ratio = 0xf000fll
>> +};
>> +
>> +
>> +static void vkms_color_test_get_lut_index(struct kunit *test)
>> +{
>> +int i;
>> +
>> +KUNIT_EXPECT_EQ(test, drm_fixp2int(get_lut_index(_linear_lut,
>> test_linear_array[0].red)), 0);
>> +
>> +for (i = 0; i < TEST_LUT_SIZE; i++)
>> +KUNIT_EXPECT_EQ(test,
>> drm_fixp2int_ceil(get_lut_index(_linear_lut,
>> test_linear_array[i].red)), i);
>> +}
>> +
>> +static void vkms_color_test_lerp(struct kunit *test)
>> +{
>> +/*** half-way round down ***/
>> +s64 t = 0x8000 - 1;
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x8);
>> +
>> +/* odd a */
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0x10, t), 0x8);
>> +
>> +/* odd b */
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0xf, t), 0x8);
>> +
>> +/* b = a */
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x10, t), 0x10);
>> +
>> +/* b = a + 1 */
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x11, t), 0x10);
>> +
>> +
>> +/*** half-way round up ***/
>> +t = 0x8000;
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x8);
>> +
>> +/* odd a */
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0x10, t), 0x9);
>> +
>> +/* odd b */
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0xf, t), 0x8);
>> +
>> +/* b = a */
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x10, t), 0x10);
>> +
>> +/* b = a + 1 */
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x11, t), 0x11);
>> +
>> +/*** t = 0.0 ***/
>> +t = 0x0;
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x0);
>> +
>> +/* odd a */
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0x10, t), 0x1);
>> +
>> +/* odd b */
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0xf, t), 0x1);
>> +
>> +/* b = a */
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x10, t), 0x10);
>> +
>> +/* b = a + 1 */
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x11, t), 0x10);
>> +
>> +/*** t = 1.0 ***/
>> +t = 0x1;
>> +KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x10);
>> +
>> +/* odd a */
>> +
Re: [PATCH 00/13] drm: Fix reservation locking for pin/unpin and console
Nice, looks totally valid to me.
Feel free to add to patch #2, #9, #10, #11 and #12 Reviewed-by:
Christian König
And Acked-by: Christian König to the rest.
Regards,
Christian.
Am 27.02.24 um 11:14 schrieb Thomas Zimmermann:
Dma-buf locking semantics require the caller of pin and unpin to hold
the buffer's reservation lock. Fix DRM to adhere to the specs. This
enables to fix the locking in DRM's console emulation. Similar changes
for vmap and mmap have been posted at [1][2]
Most DRM drivers and memory managers acquire the buffer object's
reservation lock within their GEM pin and unpin callbacks. This
violates dma-buf locking semantics. We get away with it because PRIME
does not provide pin/unpin, but attach/detach, for which the locking
semantics is correct.
Patches 1 to 8 rework DRM GEM code in various implementations to
acquire the reservation lock when entering the pin and unpin callbacks.
This prepares them for the next patch. Drivers that are not affected
by these patches either don't acquire the reservation lock (amdgpu)
or don't need preparation (loongson).
Patch 9 moves reservation locking from the GEM pin/unpin callbacks
into drm_gem_pin() and drm_gem_unpin(). As PRIME uses these functions
internally it still gets the reservation lock.
With the updated GEM callbacks, the rest of the patchset fixes the
fbdev emulation's buffer locking. Fbdev emulation needs to keep its
GEM buffer object inplace while updating its content. This required
a implicit pinning and apparently amdgpu didn't do this at all.
Patch 10 introduces drm_client_buffer_vmap_local() and _vunmap_local().
The former function map a GEM buffer into the kernel's address space
with regular vmap operations, but keeps holding the reservation lock.
The _vunmap_local() helper undoes the vmap and releases the lock. The
updated GEM callbacks make this possible. Between the two calls, the
fbdev emulation can update the buffer content without have the buffer
moved or evicted. Update fbdev-generic to use vmap_local helpers,
which fix amdgpu. The idea of adding a "local vmap" has previously been
attempted at [3] in a different form.
Patch 11 adds implicit pinning to the DRM client's regular vmap
helper so that long-term vmap'ed buffers won't be evicted. This only
affects fbdev-dma, but GEM DMA helpers don't require pinning. So
there are no practical changes.
Patches 12 and 13 remove implicit pinning from the vmap and vunmap
operations in gem-vram and qxl. These pin operations are not supposed
to be part of vmap code, but were required to keep the buffers in place
for fbdev emulation. With the conversion o ffbdev-generic to to
vmap_local helpers, that code can finally be removed.
Tested with amdgpu, nouveau, radeon, simpledrm and vc4.
[1] https://patchwork.freedesktop.org/series/106371/
[2] https://patchwork.freedesktop.org/series/116001/
[3] https://patchwork.freedesktop.org/series/84732/
Thomas Zimmermann (13):
drm/gem-shmem: Acquire reservation lock in GEM pin/unpin callbacks
drm/gem-vram: Acquire reservation lock in GEM pin/unpin callbacks
drm/msm: Provide msm_gem_get_pages_locked()
drm/msm: Acquire reservation lock in GEM pin/unpin callback
drm/nouveau: Provide nouveau_bo_{pin,unpin}_locked()
drm/nouveau: Acquire reservation lock in GEM pin/unpin callbacks
drm/qxl: Provide qxl_bo_{pin,unpin}_locked()
drm/qxl: Acquire reservation lock in GEM pin/unpin callbacks
drm/gem: Acquire reservation lock in drm_gem_{pin/unpin}()
drm/fbdev-generic: Fix locking with drm_client_buffer_vmap_local()
drm/client: Pin vmap'ed GEM buffers
drm/gem-vram: Do not pin buffer objects for vmap
drm/qxl: Do not pin buffer objects for vmap
drivers/gpu/drm/drm_client.c| 92 ++---
drivers/gpu/drm/drm_fbdev_generic.c | 4 +-
drivers/gpu/drm/drm_gem.c | 34 +++-
drivers/gpu/drm/drm_gem_shmem_helper.c | 6 +-
drivers/gpu/drm/drm_gem_vram_helper.c | 101 ++--
drivers/gpu/drm/drm_internal.h | 2 +
drivers/gpu/drm/loongson/lsdc_gem.c | 13 +--
drivers/gpu/drm/msm/msm_gem.c | 20 ++---
drivers/gpu/drm/msm/msm_gem.h | 4 +-
drivers/gpu/drm/msm/msm_gem_prime.c | 20 +++--
drivers/gpu/drm/nouveau/nouveau_bo.c| 43 +++---
drivers/gpu/drm/nouveau/nouveau_bo.h| 2 +
drivers/gpu/drm/nouveau/nouveau_prime.c | 8 +-
drivers/gpu/drm/qxl/qxl_object.c| 26 +++---
drivers/gpu/drm/qxl/qxl_object.h| 2 +
drivers/gpu/drm/qxl/qxl_prime.c | 4 +-
drivers/gpu/drm/radeon/radeon_prime.c | 11 ---
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 25 ++
include/drm/drm_client.h| 10 +++
include/drm/drm_gem.h | 3 +
include/drm/drm_gem_shmem_helper.h | 7 +-
21 files changed, 265 insertions(+), 172 deletions(-)
base-commit: 7291e2e67dff0ff573900266382c9c9248a7dea5
prerequisite-patch-id:
Re: [RFC PATCH v4 00/42] Color Pipeline API w/ VKMS
On 2024-02-27 05:26, Joshua Ashton wrote: > > > On 2/26/24 21:10, Harry Wentland wrote: >> This is an RFC set for a color pipeline API, along with a sample >> implementation in VKMS. All the key API bits are here. VKMS now >> supports two named transfer function colorops and two matrix >> colorops. We have IGT tests that check all four of these colorops >> with a pixel-by-pixel comparison that checks that these colorops >> do what we expect them to do with a +/- 1 8 bpc code point margin. >> >> The big new change with v4 is the addition of an amdgpu color >> pipeline, for all AMD GPUs with DCN 3 and newer. Amdgpu now support >> the following: >> >> 1. 1D Curve EOTF >> 2. 3x4 CTM >> 3. Multiplier >> 4. 1D Curve Inverse EOTF >> 5. 1D LUT >> 6. 1D Curve EOTF >> 7. 1D LUT >> >> The supported curves for the 1D Curve type are: >> - sRGB EOTF and its inverse >> - PQ EOTF, scaled to [0.0, 125.0] and its inverse >> - BT.2020/BT.709 OETF and its inverse >> >> Note that the 1st and 5th colorops take the EOTF or Inverse >> OETF while the 3rd colorop takes the Inverse EOTF or OETF. >> >> We are working on two more ops for amdgpu, the HDR multiplier >> and the 3DLUT, which will give us this: >> >> 1. 1D Curve EOTF >> 2. 3x4 CTM >> 3. HDR Multiplier >> 4. 1D Curve Inverse EOTF >> 5. 1D LUT >> 6. 3D LUT >> 7. 1D Curve EOTF >> 8. 1D LUT >> >> This, essentially mirrors the color pipeline used by gamescope >> and presented by Melissa Wen, with the exception of the DEGAM >> LUT, which is not currently used. See >> [1] >> https://indico.freedesktop.org/event/4/contributions/186/attachments/138/218/xdc2023-TheRainbowTreasureMap-MelissaWen.pdf >> >> After this we'd like to also add the following ops: >> - Scaler (Informational only) > > Why informational only? Having NEAREST and in general custom taps should be > possible on AMDGPU right? > > We don't have to solve this now, but I just want to make sure that we aren't > locking this to info only. > No, this isn't locking it to information only. We could allow for NEAREST or even custom taps in the future. Just don't want to open that debate now if we don't have a good reason to. Harry > Thanks > > - Joshie ✨ > >> - Color Encoding, to replace drm_plane's COLOR_ENCODING >> - Color Range, to replace drm_plane's COLOR_RANGE >> >> This patchset is grouped as follows: >> - Patches 1-3: couple general patches/fixes >> - Patches 4-7: introduce kunit to VKMS >> - Patch 7: description of motivation and details behind the >> Color Pipeline API. If you're reading nothing else >> but are interested in the topic I highly recommend >> you take a look at this. >> - Patches 7-27: DRM core and VKMS changes for color pipeline API >> - Patches 28-40: DRM core and amdgpu changes for color pipeline API >> >> VKMS patches could still be improved in a few ways, though the >> payoff might be limited and I would rather focus on other work >> at the moment. The most obvious thing to improve would be to >> eliminate the hard-coded LUTs for identity, and sRGB, and replace >> them with fixed-point math instead. >> >> There are plenty of things that I would like to see here but >> haven't had a chance to look at. These will (hopefully) be >> addressed in future iterations, either in VKMS or amdgpu: >> - Clear documentation for each drm_colorop_type >> - Add custom LUT colorops to VKMS >> - Add pre-blending 3DLUT >> - How to support HW which can't bypass entire pipeline? >> - Add ability to create colorops that don't have BYPASS >> - Can we do a LOAD / COMMIT model for LUTs (and other properties)? >> - read-only scaling colorop which defines scaling taps and position >> - read-only color format colorop to define supported color formats >> for a pipeline >> - named matrices, for things like converting YUV to RGB >> >> IGT tests can be found at >> https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/merge_requests/1 >> >> IGT patches are also being sent to the igt-dev mailing list. >> >> If you prefer a gitlab MR for review you can find it at >> https://gitlab.freedesktop.org/hwentland/linux/-/merge_requests/5 >> >> v4: >> - Add amdgpu color pipeline (WIP) >> - Don't block setting of deprecated properties, instead pass client cap >> to atomic check so drivers can ignore these props >> - Drop IOCTL definitions (Pekka) >> - Use enum property for colorop TYPE (Pekka) >> - A few cleanups to the docs (Pekka) >> - Rework the TYPE enum to name relation to avoid code duplication (Pekka) >> - Add missing function declarations (Chaitanya Kumar Borah) >> - Allow setting of NEXT property to NULL in _set_ function (Chaitanya >> Kumar Borah) >> - Add helper for creation of pipeline drm_plane property (Pekka) >> - Always create Bypass pipeline (Pekka) >> - A bunch of changes to VKMS kunit tests (Pekka) >> - Fix index in CTM doc (Pekka) >> >> v3: >> - Abandon IOCTLs and discover colorops as clients iterate
Re: [RFC PATCH v4 06/42] drm/vkms: Add kunit tests for VKMS LUT handling
On 26/02/24 18:10, Harry Wentland wrote:
> Debugging LUT math is much easier when we can unit test
> it. Add kunit functionality to VKMS and add tests for
> - get_lut_index
> - lerp_u16
>
> v4:
> - Test the critical points of the lerp function (Pekka)
>
> v3:
> - Use include way of testing static functions (Arthur)
>
> Signed-off-by: Harry Wentland
> Cc: Arthur Grillo
> ---
> drivers/gpu/drm/vkms/Kconfig | 5 +
> drivers/gpu/drm/vkms/tests/.kunitconfig | 4 +
> drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 163 ++
> drivers/gpu/drm/vkms/vkms_composer.c | 8 +-
> 4 files changed, 178 insertions(+), 2 deletions(-)
> create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig
> create mode 100644 drivers/gpu/drm/vkms/tests/vkms_color_tests.c
>
> diff --git a/drivers/gpu/drm/vkms/Kconfig b/drivers/gpu/drm/vkms/Kconfig
> index b9ecdebecb0b..c1f8b343ff0e 100644
> --- a/drivers/gpu/drm/vkms/Kconfig
> +++ b/drivers/gpu/drm/vkms/Kconfig
> @@ -13,3 +13,8 @@ config DRM_VKMS
> a VKMS.
>
> If M is selected the module will be called vkms.
> +
> +config DRM_VKMS_KUNIT_TESTS
> + tristate "Tests for VKMS" if !KUNIT_ALL_TESTS
> + depends on DRM_VKMS && KUNIT
> + default KUNIT_ALL_TESTS
> diff --git a/drivers/gpu/drm/vkms/tests/.kunitconfig
> b/drivers/gpu/drm/vkms/tests/.kunitconfig
> new file mode 100644
> index ..70e378228cbd
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/tests/.kunitconfig
> @@ -0,0 +1,4 @@
> +CONFIG_KUNIT=y
> +CONFIG_DRM=y
> +CONFIG_DRM_VKMS=y
> +CONFIG_DRM_VKMS_KUNIT_TESTS=y
> diff --git a/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
> b/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
> new file mode 100644
> index ..fc73e48aa57c
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
> @@ -0,0 +1,163 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#include
> +
> +#include
> +
> +#define TEST_LUT_SIZE 16
> +
> +static struct drm_color_lut test_linear_array[TEST_LUT_SIZE] = {
> + { 0x0, 0x0, 0x0, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> +};
> +
> +const struct vkms_color_lut test_linear_lut = {
> + .base = test_linear_array,
> + .lut_length = TEST_LUT_SIZE,
> + .channel_value2index_ratio = 0xf000fll
> +};
> +
> +
> +static void vkms_color_test_get_lut_index(struct kunit *test)
> +{
> + int i;
> +
> + KUNIT_EXPECT_EQ(test, drm_fixp2int(get_lut_index(_linear_lut,
> test_linear_array[0].red)), 0);
> +
> + for (i = 0; i < TEST_LUT_SIZE; i++)
> + KUNIT_EXPECT_EQ(test,
> drm_fixp2int_ceil(get_lut_index(_linear_lut, test_linear_array[i].red)),
> i);
> +}
> +
> +static void vkms_color_test_lerp(struct kunit *test)
> +{
> + /*** half-way round down ***/
> + s64 t = 0x8000 - 1;
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x8);
> +
> + /* odd a */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0x10, t), 0x8);
> +
> + /* odd b */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0xf, t), 0x8);
> +
> + /* b = a */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x10, t), 0x10);
> +
> + /* b = a + 1 */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x11, t), 0x10);
> +
> +
> + /*** half-way round up ***/
> + t = 0x8000;
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x8);
> +
> + /* odd a */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0x10, t), 0x9);
> +
> + /* odd b */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0xf, t), 0x8);
> +
> + /* b = a */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x10, t), 0x10);
> +
> + /* b = a + 1 */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x11, t), 0x11);
> +
> + /*** t = 0.0 ***/
> + t = 0x0;
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x0);
> +
> + /* odd a */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0x10, t), 0x1);
> +
> + /* odd b */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0xf, t), 0x1);
> +
> + /* b = a */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x10, t), 0x10);
> +
> + /* b = a + 1 */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x10, 0x11, t), 0x10);
> +
> + /*** t = 1.0 ***/
> + t = 0x1;
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, t), 0x10);
> +
> + /* odd a */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0x10, t), 0x10);
> +
> + /* odd b */
> + KUNIT_EXPECT_EQ(test, lerp_u16(0x1, 0xf, t), 0xf);
> +
> + /* b = a
[PATCH 11/13] drm/client: Pin vmap'ed GEM buffers
The function drm_client_buffer_vmap() establishes a long-term mapping
of the client's buffer object into the kernel address space. Make sure
that buffer does not move by pinning it to its current location. Same
for vunmap with unpin.
The only caller of drm_client_buffer_vmap() is fbdev-dma, which uses
gem-dma. As DMA-backed GEM buffers do not move, this change is for
correctness with little impact in practice.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_client.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 2cc81831236b5..77fe217aeaf36 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -388,16 +388,30 @@ int
drm_client_buffer_vmap(struct drm_client_buffer *buffer,
struct iosys_map *map_copy)
{
+ struct drm_gem_object *gem = buffer->gem;
struct iosys_map *map = >map;
int ret;
- ret = drm_gem_vmap_unlocked(buffer->gem, map);
+ drm_gem_lock(gem);
+
+ ret = drm_gem_pin_locked(gem);
if (ret)
- return ret;
+ goto err_drm_gem_pin_locked;
+ ret = drm_gem_vmap(gem, map);
+ if (ret)
+ goto err_drm_gem_vmap;
+
+ drm_gem_unlock(gem);
*map_copy = *map;
return 0;
+
+err_drm_gem_vmap:
+ drm_gem_unpin_locked(buffer->gem);
+err_drm_gem_pin_locked:
+ drm_gem_unlock(gem);
+ return ret;
}
EXPORT_SYMBOL(drm_client_buffer_vmap);
@@ -411,9 +425,13 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
*/
void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
{
+ struct drm_gem_object *gem = buffer->gem;
struct iosys_map *map = >map;
- drm_gem_vunmap_unlocked(buffer->gem, map);
+ drm_gem_lock(gem);
+ drm_gem_vunmap(gem, map);
+ drm_gem_unpin_locked(gem);
+ drm_gem_unlock(gem);
}
EXPORT_SYMBOL(drm_client_buffer_vunmap);
--
2.43.2
[PATCH 13/13] drm/qxl: Do not pin buffer objects for vmap
Pin and vmap are distinct operations. Do not perform a pin as part
of the vmap call. This used to be necessary to keep the fbdev buffer
in place while it is being updated. Fbdev emulation has meanwhile
been fixed to lock the buffer correctly. Same for vunmap.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/qxl/qxl_object.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 39218e979a807..5893e27a7ae50 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -164,10 +164,6 @@ int qxl_bo_vmap_locked(struct qxl_bo *bo, struct iosys_map
*map)
goto out;
}
- r = qxl_bo_pin_locked(bo);
- if (r)
- return r;
-
r = ttm_bo_vmap(>tbo, >map);
if (r) {
qxl_bo_unpin_locked(bo);
@@ -243,7 +239,6 @@ void qxl_bo_vunmap_locked(struct qxl_bo *bo)
return;
bo->kptr = NULL;
ttm_bo_vunmap(>tbo, >map);
- qxl_bo_unpin_locked(bo);
}
int qxl_bo_vunmap(struct qxl_bo *bo)
--
2.43.2
[PATCH 12/13] drm/gem-vram: Do not pin buffer objects for vmap
Pin and vmap are distinct operations. Do not perform a pin as part
of the vmap call. This used to be necessary to keep the fbdev buffer
in place while it is being updated. Fbdev emulation has meanwhile
been fixed to lock the buffer correctly. Same for vunmap.
For refactoring the code, remove the pin calls from the helper's
vmap implementation in drm_gem_vram_vmap() and inline the call to
drm_gem_vram_kmap_locked(). This gives a vmap helper that only
maps the buffer object's memory pages without pinning or locking.
Do a similar refactoring for vunmap.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_gem_vram_helper.c | 90 ++-
1 file changed, 32 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 5a16b3e0a4134..45650b9b3de91 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -368,11 +368,28 @@ int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo)
}
EXPORT_SYMBOL(drm_gem_vram_unpin);
-static int drm_gem_vram_kmap_locked(struct drm_gem_vram_object *gbo,
- struct iosys_map *map)
+/**
+ * drm_gem_vram_vmap() - Pins and maps a GEM VRAM object into kernel address
+ * space
+ * @gbo: The GEM VRAM object to map
+ * @map: Returns the kernel virtual address of the VRAM GEM object's backing
+ * store.
+ *
+ * The vmap function pins a GEM VRAM object to its current location, either
+ * system or video memory, and maps its buffer into kernel address space.
+ * As pinned object cannot be relocated, you should avoid pinning objects
+ * permanently. Call drm_gem_vram_vunmap() with the returned address to
+ * unmap and unpin the GEM VRAM object.
+ *
+ * Returns:
+ * 0 on success, or a negative error code otherwise.
+ */
+int drm_gem_vram_vmap(struct drm_gem_vram_object *gbo, struct iosys_map *map)
{
int ret;
+ dma_resv_assert_held(gbo->bo.base.resv);
+
if (gbo->vmap_use_count > 0)
goto out;
@@ -393,12 +410,23 @@ static int drm_gem_vram_kmap_locked(struct
drm_gem_vram_object *gbo,
return 0;
}
+EXPORT_SYMBOL(drm_gem_vram_vmap);
-static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo,
- struct iosys_map *map)
+/**
+ * drm_gem_vram_vunmap() - Unmaps and unpins a GEM VRAM object
+ * @gbo: The GEM VRAM object to unmap
+ * @map: Kernel virtual address where the VRAM GEM object was mapped
+ *
+ * A call to drm_gem_vram_vunmap() unmaps and unpins a GEM VRAM buffer. See
+ * the documentation for drm_gem_vram_vmap() for more information.
+ */
+void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo,
+struct iosys_map *map)
{
struct drm_device *dev = gbo->bo.base.dev;
+ dma_resv_assert_held(gbo->bo.base.resv);
+
if (drm_WARN_ON_ONCE(dev, !gbo->vmap_use_count))
return;
@@ -415,60 +443,6 @@ static void drm_gem_vram_kunmap_locked(struct
drm_gem_vram_object *gbo,
* from memory. See drm_gem_vram_bo_driver_move_notify().
*/
}
-
-/**
- * drm_gem_vram_vmap() - Pins and maps a GEM VRAM object into kernel address
- * space
- * @gbo: The GEM VRAM object to map
- * @map: Returns the kernel virtual address of the VRAM GEM object's backing
- * store.
- *
- * The vmap function pins a GEM VRAM object to its current location, either
- * system or video memory, and maps its buffer into kernel address space.
- * As pinned object cannot be relocated, you should avoid pinning objects
- * permanently. Call drm_gem_vram_vunmap() with the returned address to
- * unmap and unpin the GEM VRAM object.
- *
- * Returns:
- * 0 on success, or a negative error code otherwise.
- */
-int drm_gem_vram_vmap(struct drm_gem_vram_object *gbo, struct iosys_map *map)
-{
- int ret;
-
- dma_resv_assert_held(gbo->bo.base.resv);
-
- ret = drm_gem_vram_pin_locked(gbo, 0);
- if (ret)
- return ret;
- ret = drm_gem_vram_kmap_locked(gbo, map);
- if (ret)
- goto err_drm_gem_vram_unpin_locked;
-
- return 0;
-
-err_drm_gem_vram_unpin_locked:
- drm_gem_vram_unpin_locked(gbo);
- return ret;
-}
-EXPORT_SYMBOL(drm_gem_vram_vmap);
-
-/**
- * drm_gem_vram_vunmap() - Unmaps and unpins a GEM VRAM object
- * @gbo: The GEM VRAM object to unmap
- * @map: Kernel virtual address where the VRAM GEM object was mapped
- *
- * A call to drm_gem_vram_vunmap() unmaps and unpins a GEM VRAM buffer. See
- * the documentation for drm_gem_vram_vmap() for more information.
- */
-void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo,
-struct iosys_map *map)
-{
- dma_resv_assert_held(gbo->bo.base.resv);
-
- drm_gem_vram_kunmap_locked(gbo, map);
- drm_gem_vram_unpin_locked(gbo);
-}
EXPORT_SYMBOL(drm_gem_vram_vunmap);
/**
--
2.43.2
[PATCH 10/13] drm/fbdev-generic: Fix locking with drm_client_buffer_vmap_local()
Temporarily lock the fbdev buffer object during updates to prevent
memory managers from evicting/moving the buffer. Moving a buffer
object while update its content results in undefined behaviour.
Fbdev-generic updates its buffer object from a shadow buffer. Gem-shmem
and gem-dma helpers do not move buffer objects, so they are safe to be
used with fbdev-generic. Gem-vram and qxl are based on TTM, but pin
buffer objects are part of the vmap operation. So both are also safe
to be used with fbdev-generic.
Amdgpu and nouveau do not pin or lock the buffer object during an
update. Their TTM-based memory management could move the buffer object
while the update is ongoing.
The new vmap_local and vunmap_local helpers hold the buffer object's
reservation lock during the buffer update. This prevents moving the
buffer object on all memory managers.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_client.c| 68 +
drivers/gpu/drm/drm_fbdev_generic.c | 4 +-
drivers/gpu/drm/drm_gem.c | 12 +
include/drm/drm_client.h| 10 +
include/drm/drm_gem.h | 3 ++
5 files changed, 87 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 9403b3f576f7b..2cc81831236b5 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -304,6 +304,66 @@ drm_client_buffer_create(struct drm_client_dev *client,
u32 width, u32 height,
return ERR_PTR(ret);
}
+/**
+ * drm_client_buffer_vmap_local - Map DRM client buffer into address space
+ * @buffer: DRM client buffer
+ * @map_copy: Returns the mapped memory's address
+ *
+ * This function maps a client buffer into kernel address space. If the
+ * buffer is already mapped, it returns the existing mapping's address.
+ *
+ * Client buffer mappings are not ref'counted. Each call to
+ * drm_client_buffer_vmap_local() should be closely followed by a call to
+ * drm_client_buffer_vunmap_local(). See drm_client_buffer_vmap() for
+ * long-term mappings.
+ *
+ * The returned address is a copy of the internal value. In contrast to
+ * other vmap interfaces, you don't need it for the client's vunmap
+ * function. So you can modify it at will during blit and draw operations.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer,
+struct iosys_map *map_copy)
+{
+ struct drm_gem_object *gem = buffer->gem;
+ struct iosys_map *map = >map;
+ int ret;
+
+ drm_gem_lock(gem);
+
+ ret = drm_gem_vmap(gem, map);
+ if (ret)
+ goto err_drm_gem_vmap_unlocked;
+ *map_copy = *map;
+
+ return 0;
+
+err_drm_gem_vmap_unlocked:
+ drm_gem_unlock(gem);
+ return 0;
+}
+EXPORT_SYMBOL(drm_client_buffer_vmap_local);
+
+/**
+ * drm_client_buffer_vunmap_local - Unmap DRM client buffer
+ * @buffer: DRM client buffer
+ *
+ * This function removes a client buffer's memory mapping established
+ * with drm_client_buffer_vunmap_local(). Calling this function is only
+ * required by clients that manage their buffer mappings by themselves.
+ */
+void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer)
+{
+ struct drm_gem_object *gem = buffer->gem;
+ struct iosys_map *map = >map;
+
+ drm_gem_vunmap(gem, map);
+ drm_gem_unlock(gem);
+}
+EXPORT_SYMBOL(drm_client_buffer_vunmap_local);
+
/**
* drm_client_buffer_vmap - Map DRM client buffer into address space
* @buffer: DRM client buffer
@@ -331,14 +391,6 @@ drm_client_buffer_vmap(struct drm_client_buffer *buffer,
struct iosys_map *map = >map;
int ret;
- /*
-* FIXME: The dependency on GEM here isn't required, we could
-* convert the driver handle to a dma-buf instead and use the
-* backend-agnostic dma-buf vmap support instead. This would
-* require that the handle2fd prime ioctl is reworked to pull the
-* fd_install step out of the driver backend hooks, to make that
-* final step optional for internal users.
-*/
ret = drm_gem_vmap_unlocked(buffer->gem, map);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/drm_fbdev_generic.c
b/drivers/gpu/drm/drm_fbdev_generic.c
index d647d89764cb9..be357f926faec 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -197,14 +197,14 @@ static int drm_fbdev_generic_damage_blit(struct
drm_fb_helper *fb_helper,
*/
mutex_lock(_helper->lock);
- ret = drm_client_buffer_vmap(buffer, );
+ ret = drm_client_buffer_vmap_local(buffer, );
if (ret)
goto out;
dst = map;
drm_fbdev_generic_damage_blit_real(fb_helper, clip, );
- drm_client_buffer_vunmap(buffer);
+ drm_client_buffer_vunmap_local(buffer);
out:
[PATCH 09/13] drm/gem: Acquire reservation lock in drm_gem_{pin/unpin}()
Acquire the buffer object's reservation lock in drm_gem_pin() and
remove locking the drivers' GEM callbacks where necessary. Same for
unpin().
DRM drivers and memory managers modified by this patch will now have
correct dma-buf locking semantics: the caller is responsible for
holding the reservation lock when calling the pin or unpin callback.
DRM drivers and memory managers that are not modified will now be
protected against concurent invocation of their pin and unpin callbacks.
PRIME does not implement struct dma_buf_ops.pin, which requires
the caller to hold the reservation lock. It does implement struct
dma_buf_ops.attach, which requires to callee to acquire the
reservation lock. The PRIME code uses drm_gem_pin(), so locks
are now taken as specified. Same for unpin and detach.
The patch harmonizes GEM pin and unpin to have non-interruptible
reservation locking across all drivers, as is already the case for
vmap and vunmap. This affects gem-shmem, gem-vram, loongson, qxl and
radeon.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_gem.c | 22 --
drivers/gpu/drm/drm_gem_vram_helper.c | 15 +--
drivers/gpu/drm/drm_internal.h | 2 ++
drivers/gpu/drm/loongson/lsdc_gem.c | 13 ++---
drivers/gpu/drm/msm/msm_gem_prime.c | 4
drivers/gpu/drm/nouveau/nouveau_prime.c | 11 ---
drivers/gpu/drm/qxl/qxl_prime.c | 14 +-
drivers/gpu/drm/radeon/radeon_prime.c | 11 ---
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 25 ++---
include/drm/drm_gem_shmem_helper.h | 11 +--
10 files changed, 33 insertions(+), 95 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 44a948b80ee14..e0f80c6a7096f 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1161,7 +1161,7 @@ void drm_gem_print_info(struct drm_printer *p, unsigned
int indent,
obj->funcs->print_info(p, indent, obj);
}
-int drm_gem_pin(struct drm_gem_object *obj)
+int drm_gem_pin_locked(struct drm_gem_object *obj)
{
if (obj->funcs->pin)
return obj->funcs->pin(obj);
@@ -1169,12 +1169,30 @@ int drm_gem_pin(struct drm_gem_object *obj)
return 0;
}
-void drm_gem_unpin(struct drm_gem_object *obj)
+void drm_gem_unpin_locked(struct drm_gem_object *obj)
{
if (obj->funcs->unpin)
obj->funcs->unpin(obj);
}
+int drm_gem_pin(struct drm_gem_object *obj)
+{
+ int ret;
+
+ dma_resv_lock(obj->resv, NULL);
+ ret = drm_gem_pin_locked(obj);
+ dma_resv_unlock(obj->resv);
+
+ return ret;
+}
+
+void drm_gem_unpin(struct drm_gem_object *obj)
+{
+ dma_resv_lock(obj->resv, NULL);
+ drm_gem_unpin_locked(obj);
+ dma_resv_unlock(obj->resv);
+}
+
int drm_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map)
{
int ret;
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 15029d89badf8..5a16b3e0a4134 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -774,11 +774,6 @@ EXPORT_SYMBOL(drm_gem_vram_simple_display_pipe_cleanup_fb);
static int drm_gem_vram_object_pin(struct drm_gem_object *gem)
{
struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
- int ret;
-
- ret = ttm_bo_reserve(>bo, true, false, NULL);
- if (ret)
- return ret;
/*
* Fbdev console emulation is the use case of these PRIME
@@ -789,10 +784,7 @@ static int drm_gem_vram_object_pin(struct drm_gem_object
*gem)
* the buffer to be pinned to VRAM, implement a callback that
* sets the flags accordingly.
*/
- ret = drm_gem_vram_pin_locked(gbo, 0);
- ttm_bo_unreserve(>bo);
-
- return ret;
+ return drm_gem_vram_pin_locked(gbo, 0);
}
/**
@@ -803,13 +795,8 @@ static int drm_gem_vram_object_pin(struct drm_gem_object
*gem)
static void drm_gem_vram_object_unpin(struct drm_gem_object *gem)
{
struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
- int ret;
- ret = ttm_bo_reserve(>bo, true, false, NULL);
- if (ret)
- return;
drm_gem_vram_unpin_locked(gbo);
- ttm_bo_unreserve(>bo);
}
/**
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 8e4faf0a28e6c..40b2d3a274d6c 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -170,6 +170,8 @@ void drm_gem_release(struct drm_device *dev, struct
drm_file *file_private);
void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
const struct drm_gem_object *obj);
+int drm_gem_pin_locked(struct drm_gem_object *obj);
+void drm_gem_unpin_locked(struct drm_gem_object *obj);
int drm_gem_pin(struct drm_gem_object *obj);
void drm_gem_unpin(struct drm_gem_object *obj);
int
[PATCH 08/13] drm/qxl: Acquire reservation lock in GEM pin/unpin callbacks
Acquire the reservation lock directly in GEM pin callback. Same for
unpin. Prepares for further changes.
Dma-buf locking semantics require callers to hold the buffer's
reservation lock when invoking the pin and unpin callbacks. Prepare
qxl accordingly by pushing locking out of the implementation. A
follow-up patch will fix locking for all GEM code at once.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/qxl/qxl_prime.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
index 9169c26357d36..f2646603e12eb 100644
--- a/drivers/gpu/drm/qxl/qxl_prime.c
+++ b/drivers/gpu/drm/qxl/qxl_prime.c
@@ -31,15 +31,27 @@
int qxl_gem_prime_pin(struct drm_gem_object *obj)
{
struct qxl_bo *bo = gem_to_qxl_bo(obj);
+ int r;
- return qxl_bo_pin(bo);
+ r = qxl_bo_reserve(bo);
+ if (r)
+ return r;
+ r = qxl_bo_pin_locked(bo);
+ qxl_bo_unreserve(bo);
+
+ return r;
}
void qxl_gem_prime_unpin(struct drm_gem_object *obj)
{
struct qxl_bo *bo = gem_to_qxl_bo(obj);
+ int r;
- qxl_bo_unpin(bo);
+ r = qxl_bo_reserve(bo);
+ if (r)
+ return;
+ qxl_bo_unpin_locked(bo);
+ qxl_bo_unreserve(bo);
}
struct sg_table *qxl_gem_prime_get_sg_table(struct drm_gem_object *obj)
--
2.43.2
[PATCH 07/13] drm/qxl: Provide qxl_bo_{pin,unpin}_locked()
Rename __qxl_bo_pin() to qxl_bo_pin_locked() and update all callers.
The function will be helpful for implementing the GEM pin callback
with correct semantics. Same for __qxl_bo_unpin().
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/qxl/qxl_object.c | 25 +
drivers/gpu/drm/qxl/qxl_object.h | 2 ++
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 1e46b0a6e4787..39218e979a807 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -29,9 +29,6 @@
#include "qxl_drv.h"
#include "qxl_object.h"
-static int __qxl_bo_pin(struct qxl_bo *bo);
-static void __qxl_bo_unpin(struct qxl_bo *bo);
-
static void qxl_ttm_bo_destroy(struct ttm_buffer_object *tbo)
{
struct qxl_bo *bo;
@@ -167,13 +164,13 @@ int qxl_bo_vmap_locked(struct qxl_bo *bo, struct
iosys_map *map)
goto out;
}
- r = __qxl_bo_pin(bo);
+ r = qxl_bo_pin_locked(bo);
if (r)
return r;
r = ttm_bo_vmap(>tbo, >map);
if (r) {
- __qxl_bo_unpin(bo);
+ qxl_bo_unpin_locked(bo);
return r;
}
bo->map_count = 1;
@@ -246,7 +243,7 @@ void qxl_bo_vunmap_locked(struct qxl_bo *bo)
return;
bo->kptr = NULL;
ttm_bo_vunmap(>tbo, >map);
- __qxl_bo_unpin(bo);
+ qxl_bo_unpin_locked(bo);
}
int qxl_bo_vunmap(struct qxl_bo *bo)
@@ -290,12 +287,14 @@ struct qxl_bo *qxl_bo_ref(struct qxl_bo *bo)
return bo;
}
-static int __qxl_bo_pin(struct qxl_bo *bo)
+int qxl_bo_pin_locked(struct qxl_bo *bo)
{
struct ttm_operation_ctx ctx = { false, false };
struct drm_device *ddev = bo->tbo.base.dev;
int r;
+ dma_resv_assert_held(bo->tbo.base.resv);
+
if (bo->tbo.pin_count) {
ttm_bo_pin(>tbo);
return 0;
@@ -309,14 +308,16 @@ static int __qxl_bo_pin(struct qxl_bo *bo)
return r;
}
-static void __qxl_bo_unpin(struct qxl_bo *bo)
+void qxl_bo_unpin_locked(struct qxl_bo *bo)
{
+ dma_resv_assert_held(bo->tbo.base.resv);
+
ttm_bo_unpin(>tbo);
}
/*
* Reserve the BO before pinning the object. If the BO was reserved
- * beforehand, use the internal version directly __qxl_bo_pin.
+ * beforehand, use the internal version directly qxl_bo_pin_locked.
*
*/
int qxl_bo_pin(struct qxl_bo *bo)
@@ -327,14 +328,14 @@ int qxl_bo_pin(struct qxl_bo *bo)
if (r)
return r;
- r = __qxl_bo_pin(bo);
+ r = qxl_bo_pin_locked(bo);
qxl_bo_unreserve(bo);
return r;
}
/*
* Reserve the BO before pinning the object. If the BO was reserved
- * beforehand, use the internal version directly __qxl_bo_unpin.
+ * beforehand, use the internal version directly qxl_bo_unpin_locked.
*
*/
int qxl_bo_unpin(struct qxl_bo *bo)
@@ -345,7 +346,7 @@ int qxl_bo_unpin(struct qxl_bo *bo)
if (r)
return r;
- __qxl_bo_unpin(bo);
+ qxl_bo_unpin_locked(bo);
qxl_bo_unreserve(bo);
return 0;
}
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index 53392cb90eecf..1cf5bc7591016 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -67,6 +67,8 @@ void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev, struct
qxl_bo *bo, int pa
void qxl_bo_kunmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo,
void *map);
extern struct qxl_bo *qxl_bo_ref(struct qxl_bo *bo);
extern void qxl_bo_unref(struct qxl_bo **bo);
+extern int qxl_bo_pin_locked(struct qxl_bo *bo);
+extern void qxl_bo_unpin_locked(struct qxl_bo *bo);
extern int qxl_bo_pin(struct qxl_bo *bo);
extern int qxl_bo_unpin(struct qxl_bo *bo);
extern void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32 domain);
--
2.43.2
[PATCH 06/13] drm/nouveau: Acquire reservation lock in GEM pin/unpin callbacks
Acquire the reservation lock directly in GEM pin callback. Same for
unpin. Prepares for further changes.
Dma-buf locking semantics require callers to hold the buffer's
reservation lock when invoking the pin and unpin callbacks. Prepare
nouveau accordingly by pushing locking out of the implementation. A
follow-up patch will fix locking for all GEM code at once.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/nouveau/nouveau_prime.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c
b/drivers/gpu/drm/nouveau/nouveau_prime.c
index 1b2ff0c40fc1c..774f9bd031102 100644
--- a/drivers/gpu/drm/nouveau/nouveau_prime.c
+++ b/drivers/gpu/drm/nouveau/nouveau_prime.c
@@ -86,21 +86,32 @@ struct drm_gem_object
*nouveau_gem_prime_import_sg_table(struct drm_device *dev,
int nouveau_gem_prime_pin(struct drm_gem_object *obj)
{
struct nouveau_bo *nvbo = nouveau_gem_object(obj);
+ struct ttm_buffer_object *bo = >bo;
int ret;
- /* pin buffer into GTT */
- ret = nouveau_bo_pin(nvbo, NOUVEAU_GEM_DOMAIN_GART, false);
+ ret = ttm_bo_reserve(bo, false, false, NULL);
if (ret)
return -EINVAL;
+ /* pin buffer into GTT */
+ ret = nouveau_bo_pin_locked(nvbo, NOUVEAU_GEM_DOMAIN_GART, false);
+ if (ret)
+ ret = -EINVAL;
+ ttm_bo_unreserve(bo);
- return 0;
+ return ret;
}
void nouveau_gem_prime_unpin(struct drm_gem_object *obj)
{
struct nouveau_bo *nvbo = nouveau_gem_object(obj);
+ struct ttm_buffer_object *bo = >bo;
+ int ret;
- nouveau_bo_unpin(nvbo);
+ ret = ttm_bo_reserve(bo, false, false, NULL);
+ if (ret)
+ return;
+ nouveau_bo_unpin_locked(nvbo);
+ ttm_bo_unreserve(bo);
}
struct dma_buf *nouveau_gem_prime_export(struct drm_gem_object *gobj,
--
2.43.2
[PATCH 05/13] drm/nouveau: Provide nouveau_bo_{pin,unpin}_locked()
Implement pinning without locking in nouveau_bo_pin_locked(). Keep
nouveau_bo_pin() for acquiring the buffer object's reservation lock.
The new helper will be useful for implementing the GEM pin callback
with correct semantics. Same for unpin.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/nouveau/nouveau_bo.c | 43 +++-
drivers/gpu/drm/nouveau/nouveau_bo.h | 2 ++
2 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 56dcd25db1ce2..4a7c002a325a4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -467,17 +467,14 @@ nouveau_bo_placement_set(struct nouveau_bo *nvbo,
uint32_t domain,
set_placement_range(nvbo, domain);
}
-int
-nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t domain, bool contig)
+int nouveau_bo_pin_locked(struct nouveau_bo *nvbo, uint32_t domain, bool
contig)
{
struct nouveau_drm *drm = nouveau_bdev(nvbo->bo.bdev);
struct ttm_buffer_object *bo = >bo;
bool force = false, evict = false;
- int ret;
+ int ret = 0;
- ret = ttm_bo_reserve(bo, false, false, NULL);
- if (ret)
- return ret;
+ dma_resv_assert_held(bo->base.resv);
if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_TESLA &&
domain == NOUVEAU_GEM_DOMAIN_VRAM && contig) {
@@ -540,20 +537,15 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t domain,
bool contig)
out:
if (force && ret)
nvbo->contig = false;
- ttm_bo_unreserve(bo);
return ret;
}
-int
-nouveau_bo_unpin(struct nouveau_bo *nvbo)
+void nouveau_bo_unpin_locked(struct nouveau_bo *nvbo)
{
struct nouveau_drm *drm = nouveau_bdev(nvbo->bo.bdev);
struct ttm_buffer_object *bo = >bo;
- int ret;
- ret = ttm_bo_reserve(bo, false, false, NULL);
- if (ret)
- return ret;
+ dma_resv_assert_held(bo->base.resv);
ttm_bo_unpin(>bo);
if (!nvbo->bo.pin_count) {
@@ -568,8 +560,33 @@ nouveau_bo_unpin(struct nouveau_bo *nvbo)
break;
}
}
+}
+
+int nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t domain, bool contig)
+{
+ struct ttm_buffer_object *bo = >bo;
+ int ret;
+ ret = ttm_bo_reserve(bo, false, false, NULL);
+ if (ret)
+ return ret;
+ ret = nouveau_bo_pin_locked(nvbo, domain, contig);
+ ttm_bo_unreserve(bo);
+
+ return ret;
+}
+
+int nouveau_bo_unpin(struct nouveau_bo *nvbo)
+{
+ struct ttm_buffer_object *bo = >bo;
+ int ret;
+
+ ret = ttm_bo_reserve(bo, false, false, NULL);
+ if (ret)
+ return ret;
+ nouveau_bo_unpin_locked(nvbo);
ttm_bo_unreserve(bo);
+
return 0;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h
b/drivers/gpu/drm/nouveau/nouveau_bo.h
index e9dfab6a81560..4e891752c2551 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.h
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.h
@@ -85,6 +85,8 @@ int nouveau_bo_new(struct nouveau_cli *, u64 size, int
align, u32 domain,
u32 tile_mode, u32 tile_flags, struct sg_table *sg,
struct dma_resv *robj,
struct nouveau_bo **);
+int nouveau_bo_pin_locked(struct nouveau_bo *nvbo, uint32_t domain, bool
contig);
+void nouveau_bo_unpin_locked(struct nouveau_bo *nvbo);
int nouveau_bo_pin(struct nouveau_bo *, u32 flags, bool contig);
int nouveau_bo_unpin(struct nouveau_bo *);
int nouveau_bo_map(struct nouveau_bo *);
--
2.43.2
[PATCH 04/13] drm/msm: Acquire reservation lock in GEM pin/unpin callback
Export msm_gem_pin_pages_locked() and acquire the reservation lock
directly in GEM pin callback. Same for unpin. Prepares for further
changes.
Dma-buf locking semantics require callers to hold the buffer's
reservation lock when invoking the pin and unpin callbacks. Prepare
msm accordingly by pushing locking out of the implementation. A
follow-up patch will fix locking for all GEM code at once.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/msm/msm_gem.c | 12 ++--
drivers/gpu/drm/msm/msm_gem.h | 4 ++--
drivers/gpu/drm/msm/msm_gem_prime.c | 24 +++-
3 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index bb729353d3a8d..a5c6498a43f06 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -257,24 +257,24 @@ static void pin_obj_locked(struct drm_gem_object *obj)
mutex_unlock(>lru.lock);
}
-struct page **msm_gem_pin_pages(struct drm_gem_object *obj)
+struct page **msm_gem_pin_pages_locked(struct drm_gem_object *obj)
{
struct page **p;
- msm_gem_lock(obj);
+ msm_gem_assert_locked(obj);
+
p = msm_gem_get_pages_locked(obj, MSM_MADV_WILLNEED);
if (!IS_ERR(p))
pin_obj_locked(obj);
- msm_gem_unlock(obj);
return p;
}
-void msm_gem_unpin_pages(struct drm_gem_object *obj)
+void msm_gem_unpin_pages_locked(struct drm_gem_object *obj)
{
- msm_gem_lock(obj);
+ msm_gem_assert_locked(obj);
+
msm_gem_unpin_locked(obj);
- msm_gem_unlock(obj);
}
static pgprot_t msm_gem_pgprot(struct msm_gem_object *msm_obj, pgprot_t prot)
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 8d414b072c29d..85f0257e83dab 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -140,8 +140,8 @@ int msm_gem_get_and_pin_iova(struct drm_gem_object *obj,
void msm_gem_unpin_iova(struct drm_gem_object *obj,
struct msm_gem_address_space *aspace);
void msm_gem_pin_obj_locked(struct drm_gem_object *obj);
-struct page **msm_gem_pin_pages(struct drm_gem_object *obj);
-void msm_gem_unpin_pages(struct drm_gem_object *obj);
+struct page **msm_gem_pin_pages_locked(struct drm_gem_object *obj);
+void msm_gem_unpin_pages_locked(struct drm_gem_object *obj);
int msm_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
struct drm_mode_create_dumb *args);
int msm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
diff --git a/drivers/gpu/drm/msm/msm_gem_prime.c
b/drivers/gpu/drm/msm/msm_gem_prime.c
index 0915f3b68752e..0d22df53ab98a 100644
--- a/drivers/gpu/drm/msm/msm_gem_prime.c
+++ b/drivers/gpu/drm/msm/msm_gem_prime.c
@@ -47,13 +47,27 @@ struct drm_gem_object *msm_gem_prime_import_sg_table(struct
drm_device *dev,
int msm_gem_prime_pin(struct drm_gem_object *obj)
{
- if (!obj->import_attach)
- msm_gem_pin_pages(obj);
- return 0;
+ struct page **pages;
+ int ret = 0;
+
+ if (obj->import_attach)
+ return 0;
+
+ msm_gem_lock(obj);
+ pages = msm_gem_pin_pages_locked(obj);
+ if (IS_ERR(pages))
+ ret = PTR_ERR(pages);
+ msm_gem_unlock(obj);
+
+ return ret;
}
void msm_gem_prime_unpin(struct drm_gem_object *obj)
{
- if (!obj->import_attach)
- msm_gem_unpin_pages(obj);
+ if (obj->import_attach)
+ return;
+
+ msm_gem_lock(obj);
+ msm_gem_unpin_pages_locked(obj);
+ msm_gem_unlock(obj);
}
--
2.43.2
[PATCH 03/13] drm/msm: Provide msm_gem_get_pages_locked()
Rename msm_gem_pin_pages_locked() to msm_gem_get_pages_locked(). The
function doesn't pin any pages, but only acquires them. Renaming the
function makes the old name available.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/msm/msm_gem.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 175ee4ab8a6f7..bb729353d3a8d 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -219,7 +219,7 @@ static void put_pages(struct drm_gem_object *obj)
}
}
-static struct page **msm_gem_pin_pages_locked(struct drm_gem_object *obj,
+static struct page **msm_gem_get_pages_locked(struct drm_gem_object *obj,
unsigned madv)
{
struct msm_gem_object *msm_obj = to_msm_bo(obj);
@@ -262,7 +262,7 @@ struct page **msm_gem_pin_pages(struct drm_gem_object *obj)
struct page **p;
msm_gem_lock(obj);
- p = msm_gem_pin_pages_locked(obj, MSM_MADV_WILLNEED);
+ p = msm_gem_get_pages_locked(obj, MSM_MADV_WILLNEED);
if (!IS_ERR(p))
pin_obj_locked(obj);
msm_gem_unlock(obj);
@@ -489,7 +489,7 @@ int msm_gem_pin_vma_locked(struct drm_gem_object *obj,
struct msm_gem_vma *vma)
msm_gem_assert_locked(obj);
- pages = msm_gem_pin_pages_locked(obj, MSM_MADV_WILLNEED);
+ pages = msm_gem_get_pages_locked(obj, MSM_MADV_WILLNEED);
if (IS_ERR(pages))
return PTR_ERR(pages);
@@ -703,7 +703,7 @@ static void *get_vaddr(struct drm_gem_object *obj, unsigned
madv)
if (obj->import_attach)
return ERR_PTR(-ENODEV);
- pages = msm_gem_pin_pages_locked(obj, madv);
+ pages = msm_gem_get_pages_locked(obj, madv);
if (IS_ERR(pages))
return ERR_CAST(pages);
--
2.43.2
[PATCH 02/13] drm/gem-vram: Acquire reservation lock in GEM pin/unpin callbacks
Acquire the reservation lock directly in GEM pin callback. Same for
unpin. Prepares for further changes.
Dma-buf locking semantics require callers to hold the buffer's
reservation lock when invoking the pin and unpin callbacks. Prepare
gem-vram accordingly by pushing locking out of the implementation.
A follow-up patch will fix locking for all GEM code at once.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_gem_vram_helper.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 75f2eaf0d5b61..15029d89badf8 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -283,6 +283,8 @@ static int drm_gem_vram_pin_locked(struct
drm_gem_vram_object *gbo,
struct ttm_operation_ctx ctx = { false, false };
int ret;
+ dma_resv_assert_held(gbo->bo.base.resv);
+
if (gbo->bo.pin_count)
goto out;
@@ -338,6 +340,8 @@ EXPORT_SYMBOL(drm_gem_vram_pin);
static void drm_gem_vram_unpin_locked(struct drm_gem_vram_object *gbo)
{
+ dma_resv_assert_held(gbo->bo.base.resv);
+
ttm_bo_unpin(>bo);
}
@@ -770,8 +774,14 @@ EXPORT_SYMBOL(drm_gem_vram_simple_display_pipe_cleanup_fb);
static int drm_gem_vram_object_pin(struct drm_gem_object *gem)
{
struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
+ int ret;
- /* Fbdev console emulation is the use case of these PRIME
+ ret = ttm_bo_reserve(>bo, true, false, NULL);
+ if (ret)
+ return ret;
+
+ /*
+* Fbdev console emulation is the use case of these PRIME
* helpers. This may involve updating a hardware buffer from
* a shadow FB. We pin the buffer to it's current location
* (either video RAM or system memory) to prevent it from
@@ -779,7 +789,10 @@ static int drm_gem_vram_object_pin(struct drm_gem_object
*gem)
* the buffer to be pinned to VRAM, implement a callback that
* sets the flags accordingly.
*/
- return drm_gem_vram_pin(gbo, 0);
+ ret = drm_gem_vram_pin_locked(gbo, 0);
+ ttm_bo_unreserve(>bo);
+
+ return ret;
}
/**
@@ -790,8 +803,13 @@ static int drm_gem_vram_object_pin(struct drm_gem_object
*gem)
static void drm_gem_vram_object_unpin(struct drm_gem_object *gem)
{
struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
+ int ret;
- drm_gem_vram_unpin(gbo);
+ ret = ttm_bo_reserve(>bo, true, false, NULL);
+ if (ret)
+ return;
+ drm_gem_vram_unpin_locked(gbo);
+ ttm_bo_unreserve(>bo);
}
/**
--
2.43.2
[PATCH 01/13] drm/gem-shmem: Acquire reservation lock in GEM pin/unpin callbacks
Export drm_gem_shmem_pin_locked() and acquire the reservation lock
directly in GEM pin callback. Same for unpin. Prepares for further
changes.
Dma-buf locking semantics require callers to hold the buffer's
reservation lock when invoking the pin and unpin callbacks. Prepare
gem-shmem accordingly by pushing locking out of the implementation.
A follow-up patch will fix locking for all GEM code at once.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 6 --
include/drm/drm_gem_shmem_helper.h | 16 ++--
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index e435f986cd135..0ac3dddb917f3 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -228,7 +228,7 @@ void drm_gem_shmem_put_pages(struct drm_gem_shmem_object
*shmem)
}
EXPORT_SYMBOL(drm_gem_shmem_put_pages);
-static int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem)
+int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem)
{
int ret;
@@ -238,13 +238,15 @@ static int drm_gem_shmem_pin_locked(struct
drm_gem_shmem_object *shmem)
return ret;
}
+EXPORT_SYMBOL(drm_gem_shmem_pin_locked);
-static void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem)
+void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem)
{
dma_resv_assert_held(shmem->base.resv);
drm_gem_shmem_put_pages(shmem);
}
+EXPORT_SYMBOL(drm_gem_shmem_unpin_locked);
/**
* drm_gem_shmem_pin - Pin backing pages for a shmem GEM object
diff --git a/include/drm/drm_gem_shmem_helper.h
b/include/drm/drm_gem_shmem_helper.h
index bf0c31aa8fbe4..eb12aa9a8c556 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -108,6 +108,9 @@ void drm_gem_shmem_vunmap(struct drm_gem_shmem_object
*shmem,
struct iosys_map *map);
int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct
vm_area_struct *vma);
+int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem);
+void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem);
+
int drm_gem_shmem_madvise(struct drm_gem_shmem_object *shmem, int madv);
static inline bool drm_gem_shmem_is_purgeable(struct drm_gem_shmem_object
*shmem)
@@ -172,8 +175,15 @@ static inline void drm_gem_shmem_object_print_info(struct
drm_printer *p, unsign
static inline int drm_gem_shmem_object_pin(struct drm_gem_object *obj)
{
struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+ int ret;
+
+ ret = dma_resv_lock_interruptible(shmem->base.resv, NULL);
+ if (ret)
+ return ret;
+ ret = drm_gem_shmem_pin_locked(shmem);
+ dma_resv_unlock(shmem->base.resv);
- return drm_gem_shmem_pin(shmem);
+ return ret;
}
/**
@@ -187,7 +197,9 @@ static inline void drm_gem_shmem_object_unpin(struct
drm_gem_object *obj)
{
struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
- drm_gem_shmem_unpin(shmem);
+ dma_resv_lock(shmem->base.resv, NULL);
+ drm_gem_shmem_unpin_locked(shmem);
+ dma_resv_unlock(shmem->base.resv);
}
/**
--
2.43.2
[PATCH 00/13] drm: Fix reservation locking for pin/unpin and console
Dma-buf locking semantics require the caller of pin and unpin to hold
the buffer's reservation lock. Fix DRM to adhere to the specs. This
enables to fix the locking in DRM's console emulation. Similar changes
for vmap and mmap have been posted at [1][2]
Most DRM drivers and memory managers acquire the buffer object's
reservation lock within their GEM pin and unpin callbacks. This
violates dma-buf locking semantics. We get away with it because PRIME
does not provide pin/unpin, but attach/detach, for which the locking
semantics is correct.
Patches 1 to 8 rework DRM GEM code in various implementations to
acquire the reservation lock when entering the pin and unpin callbacks.
This prepares them for the next patch. Drivers that are not affected
by these patches either don't acquire the reservation lock (amdgpu)
or don't need preparation (loongson).
Patch 9 moves reservation locking from the GEM pin/unpin callbacks
into drm_gem_pin() and drm_gem_unpin(). As PRIME uses these functions
internally it still gets the reservation lock.
With the updated GEM callbacks, the rest of the patchset fixes the
fbdev emulation's buffer locking. Fbdev emulation needs to keep its
GEM buffer object inplace while updating its content. This required
a implicit pinning and apparently amdgpu didn't do this at all.
Patch 10 introduces drm_client_buffer_vmap_local() and _vunmap_local().
The former function map a GEM buffer into the kernel's address space
with regular vmap operations, but keeps holding the reservation lock.
The _vunmap_local() helper undoes the vmap and releases the lock. The
updated GEM callbacks make this possible. Between the two calls, the
fbdev emulation can update the buffer content without have the buffer
moved or evicted. Update fbdev-generic to use vmap_local helpers,
which fix amdgpu. The idea of adding a "local vmap" has previously been
attempted at [3] in a different form.
Patch 11 adds implicit pinning to the DRM client's regular vmap
helper so that long-term vmap'ed buffers won't be evicted. This only
affects fbdev-dma, but GEM DMA helpers don't require pinning. So
there are no practical changes.
Patches 12 and 13 remove implicit pinning from the vmap and vunmap
operations in gem-vram and qxl. These pin operations are not supposed
to be part of vmap code, but were required to keep the buffers in place
for fbdev emulation. With the conversion o ffbdev-generic to to
vmap_local helpers, that code can finally be removed.
Tested with amdgpu, nouveau, radeon, simpledrm and vc4.
[1] https://patchwork.freedesktop.org/series/106371/
[2] https://patchwork.freedesktop.org/series/116001/
[3] https://patchwork.freedesktop.org/series/84732/
Thomas Zimmermann (13):
drm/gem-shmem: Acquire reservation lock in GEM pin/unpin callbacks
drm/gem-vram: Acquire reservation lock in GEM pin/unpin callbacks
drm/msm: Provide msm_gem_get_pages_locked()
drm/msm: Acquire reservation lock in GEM pin/unpin callback
drm/nouveau: Provide nouveau_bo_{pin,unpin}_locked()
drm/nouveau: Acquire reservation lock in GEM pin/unpin callbacks
drm/qxl: Provide qxl_bo_{pin,unpin}_locked()
drm/qxl: Acquire reservation lock in GEM pin/unpin callbacks
drm/gem: Acquire reservation lock in drm_gem_{pin/unpin}()
drm/fbdev-generic: Fix locking with drm_client_buffer_vmap_local()
drm/client: Pin vmap'ed GEM buffers
drm/gem-vram: Do not pin buffer objects for vmap
drm/qxl: Do not pin buffer objects for vmap
drivers/gpu/drm/drm_client.c| 92 ++---
drivers/gpu/drm/drm_fbdev_generic.c | 4 +-
drivers/gpu/drm/drm_gem.c | 34 +++-
drivers/gpu/drm/drm_gem_shmem_helper.c | 6 +-
drivers/gpu/drm/drm_gem_vram_helper.c | 101 ++--
drivers/gpu/drm/drm_internal.h | 2 +
drivers/gpu/drm/loongson/lsdc_gem.c | 13 +--
drivers/gpu/drm/msm/msm_gem.c | 20 ++---
drivers/gpu/drm/msm/msm_gem.h | 4 +-
drivers/gpu/drm/msm/msm_gem_prime.c | 20 +++--
drivers/gpu/drm/nouveau/nouveau_bo.c| 43 +++---
drivers/gpu/drm/nouveau/nouveau_bo.h| 2 +
drivers/gpu/drm/nouveau/nouveau_prime.c | 8 +-
drivers/gpu/drm/qxl/qxl_object.c| 26 +++---
drivers/gpu/drm/qxl/qxl_object.h| 2 +
drivers/gpu/drm/qxl/qxl_prime.c | 4 +-
drivers/gpu/drm/radeon/radeon_prime.c | 11 ---
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 25 ++
include/drm/drm_client.h| 10 +++
include/drm/drm_gem.h | 3 +
include/drm/drm_gem_shmem_helper.h | 7 +-
21 files changed, 265 insertions(+), 172 deletions(-)
base-commit: 7291e2e67dff0ff573900266382c9c9248a7dea5
prerequisite-patch-id: bdfa0e6341b30cc9d7647172760b3473007c1216
prerequisite-patch-id: bc27ac702099f481890ae2c7c4a9c531f4a62d64
prerequisite-patch-id: f5d4bf16dc45334254527c2e31ee21ba4582761c
prerequisite-patch-id: 734c87e610747779aa41be12eb9e4c984bdfa743
prerequisite-patch-id:
Re: [RFC PATCH v4 00/42] Color Pipeline API w/ VKMS
On 2/26/24 21:10, Harry Wentland wrote: This is an RFC set for a color pipeline API, along with a sample implementation in VKMS. All the key API bits are here. VKMS now supports two named transfer function colorops and two matrix colorops. We have IGT tests that check all four of these colorops with a pixel-by-pixel comparison that checks that these colorops do what we expect them to do with a +/- 1 8 bpc code point margin. The big new change with v4 is the addition of an amdgpu color pipeline, for all AMD GPUs with DCN 3 and newer. Amdgpu now support the following: 1. 1D Curve EOTF 2. 3x4 CTM 3. Multiplier 4. 1D Curve Inverse EOTF 5. 1D LUT 6. 1D Curve EOTF 7. 1D LUT The supported curves for the 1D Curve type are: - sRGB EOTF and its inverse - PQ EOTF, scaled to [0.0, 125.0] and its inverse - BT.2020/BT.709 OETF and its inverse Note that the 1st and 5th colorops take the EOTF or Inverse OETF while the 3rd colorop takes the Inverse EOTF or OETF. We are working on two more ops for amdgpu, the HDR multiplier and the 3DLUT, which will give us this: 1. 1D Curve EOTF 2. 3x4 CTM 3. HDR Multiplier 4. 1D Curve Inverse EOTF 5. 1D LUT 6. 3D LUT 7. 1D Curve EOTF 8. 1D LUT This, essentially mirrors the color pipeline used by gamescope and presented by Melissa Wen, with the exception of the DEGAM LUT, which is not currently used. See [1] https://indico.freedesktop.org/event/4/contributions/186/attachments/138/218/xdc2023-TheRainbowTreasureMap-MelissaWen.pdf After this we'd like to also add the following ops: - Scaler (Informational only) Why informational only? Having NEAREST and in general custom taps should be possible on AMDGPU right? We don't have to solve this now, but I just want to make sure that we aren't locking this to info only. Thanks - Joshie ✨ - Color Encoding, to replace drm_plane's COLOR_ENCODING - Color Range, to replace drm_plane's COLOR_RANGE This patchset is grouped as follows: - Patches 1-3: couple general patches/fixes - Patches 4-7: introduce kunit to VKMS - Patch 7: description of motivation and details behind the Color Pipeline API. If you're reading nothing else but are interested in the topic I highly recommend you take a look at this. - Patches 7-27: DRM core and VKMS changes for color pipeline API - Patches 28-40: DRM core and amdgpu changes for color pipeline API VKMS patches could still be improved in a few ways, though the payoff might be limited and I would rather focus on other work at the moment. The most obvious thing to improve would be to eliminate the hard-coded LUTs for identity, and sRGB, and replace them with fixed-point math instead. There are plenty of things that I would like to see here but haven't had a chance to look at. These will (hopefully) be addressed in future iterations, either in VKMS or amdgpu: - Clear documentation for each drm_colorop_type - Add custom LUT colorops to VKMS - Add pre-blending 3DLUT - How to support HW which can't bypass entire pipeline? - Add ability to create colorops that don't have BYPASS - Can we do a LOAD / COMMIT model for LUTs (and other properties)? - read-only scaling colorop which defines scaling taps and position - read-only color format colorop to define supported color formats for a pipeline - named matrices, for things like converting YUV to RGB IGT tests can be found at https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/merge_requests/1 IGT patches are also being sent to the igt-dev mailing list. If you prefer a gitlab MR for review you can find it at https://gitlab.freedesktop.org/hwentland/linux/-/merge_requests/5 v4: - Add amdgpu color pipeline (WIP) - Don't block setting of deprecated properties, instead pass client cap to atomic check so drivers can ignore these props - Drop IOCTL definitions (Pekka) - Use enum property for colorop TYPE (Pekka) - A few cleanups to the docs (Pekka) - Rework the TYPE enum to name relation to avoid code duplication (Pekka) - Add missing function declarations (Chaitanya Kumar Borah) - Allow setting of NEXT property to NULL in _set_ function (Chaitanya Kumar Borah) - Add helper for creation of pipeline drm_plane property (Pekka) - Always create Bypass pipeline (Pekka) - A bunch of changes to VKMS kunit tests (Pekka) - Fix index in CTM doc (Pekka) v3: - Abandon IOCTLs and discover colorops as clients iterate the pipeline - Remove need for libdrm - Add color_pipeline client cap and make mutually exclusive with COLOR_RANGE and COLOR_ENCODING properties - add CTM colorop to VKMS - Use include way for kunit testing static functions (Arthur) - Make TYPE a range property - Move enum drm_colorop_type to uapi header - and a bunch of smaller bits that are highlighted in the relevant commit description v2: - Rebased on drm-misc-next - Introduce a VKMS Kunit so we can test LUT functionality in vkms_composer - Incorporate feedback in
[bug report] drm/amd/display: fix null-pointer dereference on edid reading
Hello Melissa Wen,
This is a semi-automatic email about new static checker warnings.
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:6683
amdgpu_dm_connector_funcs_force()
warn: variable dereferenced before check 'dc_link' (see line 6663)
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
6662
6663 if (dc_link->aux_mode)
^
The patch adds an unchecked dereference
6664 ddc = >dm_dp_aux.aux.ddc;
6665 else
ddc = >i2c->base;
6667
6668 /*
6669 * Note: drm_get_edid gets edid in the following order:
6670 * 1) override EDID if set via edid_override debugfs,
6671 * 2) firmware EDID if set via edid_firmware module parameter
6672 * 3) regular DDC read.
6673 */
6674 edid = drm_get_edid(connector, ddc);
6675 if (!edid) {
6676 DRM_ERROR("No EDID found on connector: %s.\n",
connector->name);
6677 return;
6678 }
6679
6680 aconnector->edid = edid;
6681
6682 /* Update emulated (virtual) sink's EDID */
6683 if (dc_em_sink && dc_link) {
^^^
The existing code assumed dc_link could be NULL? Can it? If not then
let's delete this check.
6684 memset(_em_sink->edid_caps, 0, sizeof(struct
dc_edid_caps));
6685 memmove(dc_em_sink->dc_edid.raw_edid, edid,
(edid->extensions + 1) * EDID_LENGTH);
regards,
dan carpenter
