RE: [PATCH] drm/amdgpu/gmc11: disable AGP aperture again
[AMD Official Use Only - General]
Settings for gfxhub_v3_0_3.c seem missing.
Evan
> -Original Message-
> From: amd-gfx On Behalf Of Alex
> Deucher
> Sent: Thursday, February 9, 2023 12:25 PM
> To: Zhang, Hawking
> Cc: Deucher, Alexander ; amd-
> g...@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu/gmc11: disable AGP aperture again
>
> Actually, nevermind, I found the bug. New patch on the way.
>
> Alex
>
> On Wed, Feb 8, 2023 at 9:52 PM Zhang, Hawking
> wrote:
> >
> > [AMD Official Use Only - General]
> >
> > Reviewed-by: Hawking Zhang
> >
> > Regards,
> > Hawking
> > -Original Message-
> > From: amd-gfx On Behalf Of
> > Alex Deucher
> > Sent: Thursday, February 9, 2023 05:24
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander
> > Subject: [PATCH] drm/amdgpu/gmc11: disable AGP aperture again
> >
> > It seems not all of the issues with SDMA firmware have been resolved
> leading to spurious GPU page faults on some variants.
> >
> > Signed-off-by: Alex Deucher
> > ---
> > drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c | 7 +++
> > drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c| 1 -
> > drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c | 7 +++
> > drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c | 5 +++--
> > drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c | 6 +++---
> > 5 files changed, 12 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
> > index 7c069010ca9a..fa42d1907dfa 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
> > @@ -151,11 +151,10 @@ static void
> gfxhub_v3_0_init_system_aperture_regs(struct amdgpu_device *adev) {
> > uint64_t value;
> >
> > - /* Program the AGP BAR */
> > + /* Disable AGP. */
> > WREG32_SOC15(GC, 0, regGCMC_VM_AGP_BASE, 0);
> > - WREG32_SOC15(GC, 0, regGCMC_VM_AGP_BOT, adev-
> >gmc.agp_start >> 24);
> > - WREG32_SOC15(GC, 0, regGCMC_VM_AGP_TOP, adev-
> >gmc.agp_end >> 24);
> > -
> > + WREG32_SOC15(GC, 0, regGCMC_VM_AGP_TOP, 0);
> > + WREG32_SOC15(GC, 0, regGCMC_VM_AGP_BOT, 0x00FF);
> >
> > /* Program the system aperture low logical page number. */
> > WREG32_SOC15(GC, 0,
> regGCMC_VM_SYSTEM_APERTURE_LOW_ADDR,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> > index 0a31a341aa43..5e0018fe7e7d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> > @@ -673,7 +673,6 @@ static void gmc_v11_0_vram_gtt_location(struct
> > amdgpu_device *adev,
> >
> > amdgpu_gmc_vram_location(adev, >gmc, base);
> > amdgpu_gmc_gart_location(adev, mc);
> > - amdgpu_gmc_agp_location(adev, mc);
> >
> > /* base offset of vram pages */
> > if (amdgpu_sriov_vf(adev))
> > diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
> > b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
> > index 923fc09bc8fc..ae9cd1a4cfee 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
> > @@ -177,11 +177,10 @@ static void
> mmhub_v3_0_init_system_aperture_regs(struct amdgpu_device *adev)
> > * these regs, and they will be programed at host.
> > * so skip programing these regs.
> > */
> > - /* Program the AGP BAR */
> > + /* Disable AGP. */
> > WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BASE, 0);
> > - WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, adev-
> >gmc.agp_start >> 24);
> > - WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, adev-
> >gmc.agp_end >> 24);
> > -
> > + WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, 0);
> > + WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, 0x00FF);
> > /* Program the system aperture low logical page number. */
> > WREG32_SOC15(MMHUB, 0,
> regMMMC_VM_SYSTEM_APERTURE_LOW_ADDR,
> > adev->gmc.vram_start >> 18); diff --git
> > a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
> > b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
> > index c8d478f2afdc..fb2f0eb72f69 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
> > @@ -173,8 +173,9 @@ static void
> > mmhub_v3_0_1_init_system_aperture_regs(struct amdgpu_device *adev)
> >
> > /* Program the AGP BAR */
> > WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BASE, 0);
> > - WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, adev-
> >gmc.agp_start >> 24);
> > - WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, adev-
> >gmc.agp_end >> 24);
> > + WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, 0);
> > + WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, 0x00FF);
> > +
> >
> > /*
> > * the new L1 policy will block SRIOV guest from writing diff
> > --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
> > b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
> > index
RE: [PATCH 2/2] drm/amd/display: minor cleanup of vm_setup
[AMD Official Use Only - General]
Sorry, I made a mistake. There is no mmhub_v3_0_3. Only gfxhub_v3_0_3 need the
same fix
Regards,
Hawking
-Original Message-
From: Zhang, Hawking
Sent: Thursday, February 9, 2023 15:15
To: Zhang, Hawking ; Deucher, Alexander
; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander
Subject: RE: [PATCH 2/2] drm/amd/display: minor cleanup of vm_setup
[AMD Official Use Only - General]
BTW, @Deucher, Alexander gfxhub_v3_0_3/mmhub_v3_0_3 also need similar fixes
Regards,
Hawking
-Original Message-
From: amd-gfx On Behalf Of Zhang,
Hawking
Sent: Thursday, February 9, 2023 15:13
To: Deucher, Alexander ;
amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander
Subject: RE: [PATCH 2/2] drm/amd/display: minor cleanup of vm_setup
[AMD Official Use Only - General]
[AMD Official Use Only - General]
Series is
Reviewed-by: Hawking Zhang
Regards,
Hawking
-Original Message-
From: amd-gfx On Behalf Of Alex Deucher
Sent: Thursday, February 9, 2023 12:46
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander
Subject: [PATCH 2/2] drm/amd/display: minor cleanup of vm_setup
Use fb_start/end for consistency with gmc code for non- XGMI systems, they are
equivalent to vram_start/end.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +++---
1 file changed, 3 insertions(+), 3 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 8ba4a57d8e6f..bf06875e6a01 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1191,7 +1191,7 @@ static void mmhub_read_system_context(struct
amdgpu_device *adev, struct dc_phy_
/* AGP aperture is disabled */
if (agp_bot == agp_top) {
- logical_addr_low = adev->gmc.vram_start >> 18;
+ logical_addr_low = adev->gmc.fb_start >> 18;
if (adev->apu_flags & AMD_APU_IS_RAVEN2)
/*
* Raven2 has a HW issue that it is unable to use the
vram which @@ -1201,9 +1201,9 @@ static void mmhub_read_system_context(struct
amdgpu_device *adev, struct dc_phy_
*/
logical_addr_high = (adev->gmc.fb_end >> 18) + 0x1;
else
- logical_addr_high = adev->gmc.vram_end >> 18;
+ logical_addr_high = adev->gmc.fb_end >> 18;
} else {
- logical_addr_low = min(adev->gmc.fb_start,
adev->gmc.agp_start) >> 18;
+ logical_addr_low = min(adev->gmc.fb_start,
+adev->gmc.agp_start) >> 18;
if (adev->apu_flags & AMD_APU_IS_RAVEN2)
/*
* Raven2 has a HW issue that it is unable to use the
vram which
--
2.39.1
RE: [PATCH 2/2] drm/amd/display: minor cleanup of vm_setup
[AMD Official Use Only - General]
BTW, @Deucher, Alexander gfxhub_v3_0_3/mmhub_v3_0_3 also need similar fixes
Regards,
Hawking
-Original Message-
From: amd-gfx On Behalf Of Zhang,
Hawking
Sent: Thursday, February 9, 2023 15:13
To: Deucher, Alexander ;
amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander
Subject: RE: [PATCH 2/2] drm/amd/display: minor cleanup of vm_setup
[AMD Official Use Only - General]
[AMD Official Use Only - General]
Series is
Reviewed-by: Hawking Zhang
Regards,
Hawking
-Original Message-
From: amd-gfx On Behalf Of Alex Deucher
Sent: Thursday, February 9, 2023 12:46
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander
Subject: [PATCH 2/2] drm/amd/display: minor cleanup of vm_setup
Use fb_start/end for consistency with gmc code for non- XGMI systems, they are
equivalent to vram_start/end.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +++---
1 file changed, 3 insertions(+), 3 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 8ba4a57d8e6f..bf06875e6a01 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1191,7 +1191,7 @@ static void mmhub_read_system_context(struct
amdgpu_device *adev, struct dc_phy_
/* AGP aperture is disabled */
if (agp_bot == agp_top) {
- logical_addr_low = adev->gmc.vram_start >> 18;
+ logical_addr_low = adev->gmc.fb_start >> 18;
if (adev->apu_flags & AMD_APU_IS_RAVEN2)
/*
* Raven2 has a HW issue that it is unable to use the
vram which @@ -1201,9 +1201,9 @@ static void mmhub_read_system_context(struct
amdgpu_device *adev, struct dc_phy_
*/
logical_addr_high = (adev->gmc.fb_end >> 18) + 0x1;
else
- logical_addr_high = adev->gmc.vram_end >> 18;
+ logical_addr_high = adev->gmc.fb_end >> 18;
} else {
- logical_addr_low = min(adev->gmc.fb_start,
adev->gmc.agp_start) >> 18;
+ logical_addr_low = min(adev->gmc.fb_start,
+adev->gmc.agp_start) >> 18;
if (adev->apu_flags & AMD_APU_IS_RAVEN2)
/*
* Raven2 has a HW issue that it is unable to use the
vram which
--
2.39.1
RE: [PATCH 2/2] drm/amd/display: minor cleanup of vm_setup
[AMD Official Use Only - General]
Series is
Reviewed-by: Hawking Zhang
Regards,
Hawking
-Original Message-
From: amd-gfx On Behalf Of Alex Deucher
Sent: Thursday, February 9, 2023 12:46
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander
Subject: [PATCH 2/2] drm/amd/display: minor cleanup of vm_setup
Use fb_start/end for consistency with gmc code for non- XGMI systems, they are
equivalent to vram_start/end.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +++---
1 file changed, 3 insertions(+), 3 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 8ba4a57d8e6f..bf06875e6a01 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1191,7 +1191,7 @@ static void mmhub_read_system_context(struct
amdgpu_device *adev, struct dc_phy_
/* AGP aperture is disabled */
if (agp_bot == agp_top) {
- logical_addr_low = adev->gmc.vram_start >> 18;
+ logical_addr_low = adev->gmc.fb_start >> 18;
if (adev->apu_flags & AMD_APU_IS_RAVEN2)
/*
* Raven2 has a HW issue that it is unable to use the
vram which @@ -1201,9 +1201,9 @@ static void mmhub_read_system_context(struct
amdgpu_device *adev, struct dc_phy_
*/
logical_addr_high = (adev->gmc.fb_end >> 18) + 0x1;
else
- logical_addr_high = adev->gmc.vram_end >> 18;
+ logical_addr_high = adev->gmc.fb_end >> 18;
} else {
- logical_addr_low = min(adev->gmc.fb_start,
adev->gmc.agp_start) >> 18;
+ logical_addr_low = min(adev->gmc.fb_start, adev->gmc.agp_start)
>>
+18;
if (adev->apu_flags & AMD_APU_IS_RAVEN2)
/*
* Raven2 has a HW issue that it is unable to use the
vram which
--
2.39.1
[PATCH v2] drm/amdkfd: To fix sdma page fault issue for GC 11.x
From: Ruili Ji
For the MQD memory, KMD would always allocate 4K memory,
and mes scheduler would write to the end of MQD for unmap flag.
Signed-off-by: Ruili Ji
---
.../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 5 +++--
drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c | 15 ++-
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index c06ada0844ba..7a95698d83f7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -2373,7 +2373,7 @@ struct device_queue_manager
*device_queue_manager_init(struct kfd_dev *dev)
if (init_mqd_managers(dqm))
goto out_free;
- if (allocate_hiq_sdma_mqd(dqm)) {
+ if (!dev->shared_resources.enable_mes && allocate_hiq_sdma_mqd(dqm)) {
pr_err("Failed to allocate hiq sdma mqd trunk buffer\n");
goto out_free;
}
@@ -2397,7 +2397,8 @@ static void deallocate_hiq_sdma_mqd(struct kfd_dev *dev,
void device_queue_manager_uninit(struct device_queue_manager *dqm)
{
dqm->ops.uninitialize(dqm);
- deallocate_hiq_sdma_mqd(dqm->dev, >hiq_sdma_mqd);
+ if (!dqm->dev->shared_resources.enable_mes)
+ deallocate_hiq_sdma_mqd(dqm->dev, >hiq_sdma_mqd);
kfree(dqm);
}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
index 4f6390f3236e..4a9af800b1f1 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
@@ -308,11 +308,16 @@ static void init_mqd_sdma(struct mqd_manager *mm, void
**mqd,
struct queue_properties *q)
{
struct v11_sdma_mqd *m;
+ int size;
m = (struct v11_sdma_mqd *) mqd_mem_obj->cpu_ptr;
- memset(m, 0, sizeof(struct v11_sdma_mqd));
+ if (mm->dev->shared_resources.enable_mes)
+ size = PAGE_SIZE;
+ else
+ size = sizeof(struct v11_sdma_mqd);
+ memset(m, 0, size);
*mqd = m;
if (gart_addr)
*gart_addr = mqd_mem_obj->gpu_addr;
@@ -443,6 +448,14 @@ struct mqd_manager *mqd_manager_init_v11(enum KFD_MQD_TYPE
type,
#if defined(CONFIG_DEBUG_FS)
mqd->debugfs_show_mqd = debugfs_show_mqd_sdma;
#endif
+ /*
+* To allocate SDMA MQDs by generic functions
+* when MES is enabled.
+*/
+ if (dev->shared_resources.enable_mes) {
+ mqd->allocate_mqd = allocate_mqd;
+ mqd->free_mqd = kfd_free_mqd_cp;
+ }
pr_debug("%s@%i\n", __func__, __LINE__);
break;
default:
--
2.25.1
[pull] amdgpu drm-fixes-6.2
Hi Dave, Daniel, Fixes for 6.2. The following changes since commit 04119ab1a49fc41cb70f0472be5455af268fa260: nvidiafb: detect the hardware support before removing console. (2023-02-07 08:42:29 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-fixes-6.2-2023-02-08 for you to fetch changes up to c6ac406cd8ff610a2d5da298b1d3071acfcde7f0: drm/amdgpu/smu: skip pptable init under sriov (2023-02-08 22:33:37 -0500) amd-drm-fixes-6.2-2023-02-08: amdgpu: - Flickering fixes for DCN 2.1, 3.1.2/3 - Re-enable S/G display on DCN 3.1.4 - Properly fix S/G display with AGP aperture enabled - Fix cursor offset with 180 rotation - SMU13 fixes - Use TGID for GPUVM traces - Fix oops on in fence error path - Don't run IB tests on hw rings when sw rings are in use Alex Deucher (4): drm/amd/display: disable S/G display on DCN 2.1.0 drm/amd/display: disable S/G display on DCN 3.1.2/3 drm/amd/display: properly handling AGP aperture in vm setup Revert "drm/amd/display: disable S/G display on DCN 3.1.4" Evan Quan (3): drm/amd/pm: add SMU 13.0.7 missing GetPptLimit message mapping drm/amd/pm: bump SMU 13.0.0 driver_if header version drm/amd/pm: bump SMU 13.0.7 driver_if header version Friedrich Vock (1): drm/amdgpu: Use the TGID for trace_amdgpu_vm_update_ptes Guilherme G. Piccoli (1): drm/amdgpu/fence: Fix oops due to non-matching drm_sched init/fini Jane Jian (1): drm/amdgpu/smu: skip pptable init under sriov JesseZhang (1): amd/amdgpu: remove test ib on hw ring Kenneth Feng (1): drm/amd/amdgpu: enable athub cg 11.0.3 Kent Russell (1): drm/amdgpu: Add unique_id support for GC 11.0.1/2 Melissa Wen (1): drm/amd/display: fix cursor offset on rotation 180 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 8 +++- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 2 +- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 1 - drivers/gpu/drm/amd/amdgpu/soc21.c | 4 +- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 46 ++ .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 2 +- drivers/gpu/drm/amd/pm/amdgpu_pm.c | 2 + .../pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h | 5 ++- .../pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h | 29 +++--- drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h | 4 +- .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 6 +++ .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 1 + 13 files changed, 71 insertions(+), 41 deletions(-)
[PATCH 2/2] drm/amd/display: minor cleanup of vm_setup
Use fb_start/end for consistency with gmc code for non-
XGMI systems, they are equivalent to vram_start/end.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +++---
1 file changed, 3 insertions(+), 3 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 8ba4a57d8e6f..bf06875e6a01 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1191,7 +1191,7 @@ static void mmhub_read_system_context(struct
amdgpu_device *adev, struct dc_phy_
/* AGP aperture is disabled */
if (agp_bot == agp_top) {
- logical_addr_low = adev->gmc.vram_start >> 18;
+ logical_addr_low = adev->gmc.fb_start >> 18;
if (adev->apu_flags & AMD_APU_IS_RAVEN2)
/*
* Raven2 has a HW issue that it is unable to use the
vram which
@@ -1201,9 +1201,9 @@ static void mmhub_read_system_context(struct
amdgpu_device *adev, struct dc_phy_
*/
logical_addr_high = (adev->gmc.fb_end >> 18) + 0x1;
else
- logical_addr_high = adev->gmc.vram_end >> 18;
+ logical_addr_high = adev->gmc.fb_end >> 18;
} else {
- logical_addr_low = min(adev->gmc.fb_start,
adev->gmc.agp_start) >> 18;
+ logical_addr_low = min(adev->gmc.fb_start, adev->gmc.agp_start)
>> 18;
if (adev->apu_flags & AMD_APU_IS_RAVEN2)
/*
* Raven2 has a HW issue that it is unable to use the
vram which
--
2.39.1
[PATCH 1/2] drm/amdgpu/gmc11: fix system aperture set when AGP is enabled
Need to cover both FB and AGP apertures.
Fixes: c6eafee038ed ("Revert "Revert "drm/amdgpu/gmc11: enable AGP aperture""")
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c | 4 ++--
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
b/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
index 7c069010ca9a..be0d0f47415e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
@@ -159,9 +159,9 @@ static void gfxhub_v3_0_init_system_aperture_regs(struct
amdgpu_device *adev)
/* Program the system aperture low logical page number. */
WREG32_SOC15(GC, 0, regGCMC_VM_SYSTEM_APERTURE_LOW_ADDR,
-adev->gmc.vram_start >> 18);
+min(adev->gmc.fb_start, adev->gmc.agp_start) >> 18);
WREG32_SOC15(GC, 0, regGCMC_VM_SYSTEM_APERTURE_HIGH_ADDR,
-adev->gmc.vram_end >> 18);
+max(adev->gmc.fb_end, adev->gmc.agp_end) >> 18);
/* Set default page address. */
value = adev->mem_scratch.gpu_addr - adev->gmc.vram_start
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
index 923fc09bc8fc..164948c50ac3 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
@@ -184,9 +184,9 @@ static void mmhub_v3_0_init_system_aperture_regs(struct
amdgpu_device *adev)
/* Program the system aperture low logical page number. */
WREG32_SOC15(MMHUB, 0, regMMMC_VM_SYSTEM_APERTURE_LOW_ADDR,
-adev->gmc.vram_start >> 18);
+min(adev->gmc.fb_start, adev->gmc.agp_start) >> 18);
WREG32_SOC15(MMHUB, 0, regMMMC_VM_SYSTEM_APERTURE_HIGH_ADDR,
-adev->gmc.vram_end >> 18);
+max(adev->gmc.fb_end, adev->gmc.agp_end) >> 18);
/* Set default page address. */
value = adev->mem_scratch.gpu_addr - adev->gmc.vram_start +
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
index c8d478f2afdc..26509b6b8c24 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
@@ -183,9 +183,9 @@ static void mmhub_v3_0_1_init_system_aperture_regs(struct
amdgpu_device *adev)
*/
/* Program the system aperture low logical page number. */
WREG32_SOC15(MMHUB, 0, regMMMC_VM_SYSTEM_APERTURE_LOW_ADDR,
-adev->gmc.vram_start >> 18);
+min(adev->gmc.fb_start, adev->gmc.agp_start) >> 18);
WREG32_SOC15(MMHUB, 0, regMMMC_VM_SYSTEM_APERTURE_HIGH_ADDR,
-adev->gmc.vram_end >> 18);
+max(adev->gmc.fb_end, adev->gmc.agp_end) >> 18);
/* Set default page address. */
value = adev->mem_scratch.gpu_addr - adev->gmc.vram_start +
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
index 51580302ec42..26abbc6a47ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
@@ -175,9 +175,9 @@ static void mmhub_v3_0_2_init_system_aperture_regs(struct
amdgpu_device *adev)
*/
/* Program the system aperture low logical page number. */
WREG32_SOC15(MMHUB, 0, regMMMC_VM_SYSTEM_APERTURE_LOW_ADDR,
-adev->gmc.vram_start >> 18);
+min(adev->gmc.fb_start, adev->gmc.agp_start) >>
18);
WREG32_SOC15(MMHUB, 0, regMMMC_VM_SYSTEM_APERTURE_HIGH_ADDR,
-adev->gmc.vram_end >> 18);
+max(adev->gmc.fb_end, adev->gmc.agp_end) >> 18);
}
/* Set default page address. */
--
2.39.1
Re: [PATCH] drm/amdgpu/gmc11: disable AGP aperture again
Actually, nevermind, I found the bug. New patch on the way.
Alex
On Wed, Feb 8, 2023 at 9:52 PM Zhang, Hawking wrote:
>
> [AMD Official Use Only - General]
>
> Reviewed-by: Hawking Zhang
>
> Regards,
> Hawking
> -Original Message-
> From: amd-gfx On Behalf Of Alex
> Deucher
> Sent: Thursday, February 9, 2023 05:24
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander
> Subject: [PATCH] drm/amdgpu/gmc11: disable AGP aperture again
>
> It seems not all of the issues with SDMA firmware have been resolved leading
> to spurious GPU page faults on some variants.
>
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c | 7 +++
> drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c| 1 -
> drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c | 7 +++
> drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c | 5 +++--
> drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c | 6 +++---
> 5 files changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
> index 7c069010ca9a..fa42d1907dfa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
> @@ -151,11 +151,10 @@ static void
> gfxhub_v3_0_init_system_aperture_regs(struct amdgpu_device *adev) {
> uint64_t value;
>
> - /* Program the AGP BAR */
> + /* Disable AGP. */
> WREG32_SOC15(GC, 0, regGCMC_VM_AGP_BASE, 0);
> - WREG32_SOC15(GC, 0, regGCMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
> - WREG32_SOC15(GC, 0, regGCMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
> -
> + WREG32_SOC15(GC, 0, regGCMC_VM_AGP_TOP, 0);
> + WREG32_SOC15(GC, 0, regGCMC_VM_AGP_BOT, 0x00FF);
>
> /* Program the system aperture low logical page number. */
> WREG32_SOC15(GC, 0, regGCMC_VM_SYSTEM_APERTURE_LOW_ADDR,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> index 0a31a341aa43..5e0018fe7e7d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> @@ -673,7 +673,6 @@ static void gmc_v11_0_vram_gtt_location(struct
> amdgpu_device *adev,
>
> amdgpu_gmc_vram_location(adev, >gmc, base);
> amdgpu_gmc_gart_location(adev, mc);
> - amdgpu_gmc_agp_location(adev, mc);
>
> /* base offset of vram pages */
> if (amdgpu_sriov_vf(adev))
> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
> b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
> index 923fc09bc8fc..ae9cd1a4cfee 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
> @@ -177,11 +177,10 @@ static void mmhub_v3_0_init_system_aperture_regs(struct
> amdgpu_device *adev)
> * these regs, and they will be programed at host.
> * so skip programing these regs.
> */
> - /* Program the AGP BAR */
> + /* Disable AGP. */
> WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BASE, 0);
> - WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
> - WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
> -
> + WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, 0);
> + WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, 0x00FF);
> /* Program the system aperture low logical page number. */
> WREG32_SOC15(MMHUB, 0, regMMMC_VM_SYSTEM_APERTURE_LOW_ADDR,
> adev->gmc.vram_start >> 18);
> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
> b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
> index c8d478f2afdc..fb2f0eb72f69 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
> @@ -173,8 +173,9 @@ static void mmhub_v3_0_1_init_system_aperture_regs(struct
> amdgpu_device *adev)
>
> /* Program the AGP BAR */
> WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BASE, 0);
> - WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
> - WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
> + WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, 0);
> + WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, 0x00FF);
> +
>
> /*
> * the new L1 policy will block SRIOV guest from writing diff --git
> a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
> b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
> index 51580302ec42..c30e40e52fb2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
> @@ -162,10 +162,10 @@ static void
> mmhub_v3_0_2_init_system_aperture_regs(struct amdgpu_device *adev)
> uint64_t value;
> uint32_t tmp;
>
> - /* Program the AGP BAR */
> + /* Disable AGP. */
> WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BASE, 0);
> - WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
> - WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
> +
Re: [PATCH v3] drm/amdkfd: Prevent user space using both svm and kfd api to register same user buffer
Am 2023-02-08 um 18:26 schrieb Xiaogang.Chen:
From: Xiaogang Chen
When xnack is on user space can use svm page restore to set a vm range without
setup it first, then use regular api to register. Currently kfd api and svm are
not interoperable. We already have check on that, but for user buffer the
mapping
address is not same as buffer cpu virtual address. Add checking on that to
avoid error propagate to hmm.
Signed-off-by: Xiaogang Chen
Reviewed-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index f79b8e964140..072fa4fbd27f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1065,6 +1065,20 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file
*filep,
mutex_unlock(>svms.lock);
return -EADDRINUSE;
}
+
+ /* When register user buffer check if it has been registered by svm by
+* buffer cpu virtual address.
+*/
+ if ((flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) &&
+ interval_tree_iter_first(>svms.objects,
+args->mmap_offset >> PAGE_SHIFT,
+(args->mmap_offset + args->size - 1) >>
PAGE_SHIFT)) {
+ pr_err("User Buffer Address: 0x%llx already allocated by SVM\n",
+ args->mmap_offset);
+ mutex_unlock(>svms.lock);
+ return -EADDRINUSE;
+ }
+
mutex_unlock(>svms.lock);
#endif
mutex_lock(>mutex);
RE: [PATCH] drm/amdgpu/gmc11: disable AGP aperture again
[AMD Official Use Only - General]
Reviewed-by: Hawking Zhang
Regards,
Hawking
-Original Message-
From: amd-gfx On Behalf Of Alex Deucher
Sent: Thursday, February 9, 2023 05:24
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander
Subject: [PATCH] drm/amdgpu/gmc11: disable AGP aperture again
It seems not all of the issues with SDMA firmware have been resolved leading to
spurious GPU page faults on some variants.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c | 7 +++
drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c| 1 -
drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c | 7 +++
drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c | 5 +++--
drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c | 6 +++---
5 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
b/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
index 7c069010ca9a..fa42d1907dfa 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
@@ -151,11 +151,10 @@ static void gfxhub_v3_0_init_system_aperture_regs(struct
amdgpu_device *adev) {
uint64_t value;
- /* Program the AGP BAR */
+ /* Disable AGP. */
WREG32_SOC15(GC, 0, regGCMC_VM_AGP_BASE, 0);
- WREG32_SOC15(GC, 0, regGCMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
- WREG32_SOC15(GC, 0, regGCMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
-
+ WREG32_SOC15(GC, 0, regGCMC_VM_AGP_TOP, 0);
+ WREG32_SOC15(GC, 0, regGCMC_VM_AGP_BOT, 0x00FF);
/* Program the system aperture low logical page number. */
WREG32_SOC15(GC, 0, regGCMC_VM_SYSTEM_APERTURE_LOW_ADDR,
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
index 0a31a341aa43..5e0018fe7e7d 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
@@ -673,7 +673,6 @@ static void gmc_v11_0_vram_gtt_location(struct
amdgpu_device *adev,
amdgpu_gmc_vram_location(adev, >gmc, base);
amdgpu_gmc_gart_location(adev, mc);
- amdgpu_gmc_agp_location(adev, mc);
/* base offset of vram pages */
if (amdgpu_sriov_vf(adev))
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
index 923fc09bc8fc..ae9cd1a4cfee 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
@@ -177,11 +177,10 @@ static void mmhub_v3_0_init_system_aperture_regs(struct
amdgpu_device *adev)
* these regs, and they will be programed at host.
* so skip programing these regs.
*/
- /* Program the AGP BAR */
+ /* Disable AGP. */
WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BASE, 0);
- WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
- WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
-
+ WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, 0);
+ WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, 0x00FF);
/* Program the system aperture low logical page number. */
WREG32_SOC15(MMHUB, 0, regMMMC_VM_SYSTEM_APERTURE_LOW_ADDR,
adev->gmc.vram_start >> 18);
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
index c8d478f2afdc..fb2f0eb72f69 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
@@ -173,8 +173,9 @@ static void mmhub_v3_0_1_init_system_aperture_regs(struct
amdgpu_device *adev)
/* Program the AGP BAR */
WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BASE, 0);
- WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
- WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
+ WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, 0);
+ WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, 0x00FF);
+
/*
* the new L1 policy will block SRIOV guest from writing diff --git
a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
index 51580302ec42..c30e40e52fb2 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
@@ -162,10 +162,10 @@ static void mmhub_v3_0_2_init_system_aperture_regs(struct
amdgpu_device *adev)
uint64_t value;
uint32_t tmp;
- /* Program the AGP BAR */
+ /* Disable AGP. */
WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BASE, 0);
- WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
- WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
+ WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, 0);
+ WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, 0x00FF);
if (!amdgpu_sriov_vf(adev)) {
/*
--
2.39.1
[PATCH v3] drm/amdkfd: Prevent user space using both svm and kfd api to register same user buffer
From: Xiaogang Chen
When xnack is on user space can use svm page restore to set a vm range without
setup it first, then use regular api to register. Currently kfd api and svm are
not interoperable. We already have check on that, but for user buffer the
mapping
address is not same as buffer cpu virtual address. Add checking on that to
avoid error propagate to hmm.
Signed-off-by: Xiaogang Chen
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index f79b8e964140..072fa4fbd27f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1065,6 +1065,20 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file
*filep,
mutex_unlock(>svms.lock);
return -EADDRINUSE;
}
+
+ /* When register user buffer check if it has been registered by svm by
+* buffer cpu virtual address.
+*/
+ if ((flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) &&
+ interval_tree_iter_first(>svms.objects,
+args->mmap_offset >> PAGE_SHIFT,
+(args->mmap_offset + args->size - 1) >>
PAGE_SHIFT)) {
+ pr_err("User Buffer Address: 0x%llx already allocated by SVM\n",
+ args->mmap_offset);
+ mutex_unlock(>svms.lock);
+ return -EADDRINUSE;
+ }
+
mutex_unlock(>svms.lock);
#endif
mutex_lock(>mutex);
--
2.25.1
[PATCH] drm/amdgpu/gmc11: disable AGP aperture again
It seems not all of the issues with SDMA firmware have
been resolved leading to spurious GPU page faults on
some variants.
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c | 7 +++
drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c| 1 -
drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c | 7 +++
drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c | 5 +++--
drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c | 6 +++---
5 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
b/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
index 7c069010ca9a..fa42d1907dfa 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v3_0.c
@@ -151,11 +151,10 @@ static void gfxhub_v3_0_init_system_aperture_regs(struct
amdgpu_device *adev)
{
uint64_t value;
- /* Program the AGP BAR */
+ /* Disable AGP. */
WREG32_SOC15(GC, 0, regGCMC_VM_AGP_BASE, 0);
- WREG32_SOC15(GC, 0, regGCMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
- WREG32_SOC15(GC, 0, regGCMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
-
+ WREG32_SOC15(GC, 0, regGCMC_VM_AGP_TOP, 0);
+ WREG32_SOC15(GC, 0, regGCMC_VM_AGP_BOT, 0x00FF);
/* Program the system aperture low logical page number. */
WREG32_SOC15(GC, 0, regGCMC_VM_SYSTEM_APERTURE_LOW_ADDR,
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
index 0a31a341aa43..5e0018fe7e7d 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
@@ -673,7 +673,6 @@ static void gmc_v11_0_vram_gtt_location(struct
amdgpu_device *adev,
amdgpu_gmc_vram_location(adev, >gmc, base);
amdgpu_gmc_gart_location(adev, mc);
- amdgpu_gmc_agp_location(adev, mc);
/* base offset of vram pages */
if (amdgpu_sriov_vf(adev))
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
index 923fc09bc8fc..ae9cd1a4cfee 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0.c
@@ -177,11 +177,10 @@ static void mmhub_v3_0_init_system_aperture_regs(struct
amdgpu_device *adev)
* these regs, and they will be programed at host.
* so skip programing these regs.
*/
- /* Program the AGP BAR */
+ /* Disable AGP. */
WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BASE, 0);
- WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
- WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
-
+ WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, 0);
+ WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, 0x00FF);
/* Program the system aperture low logical page number. */
WREG32_SOC15(MMHUB, 0, regMMMC_VM_SYSTEM_APERTURE_LOW_ADDR,
adev->gmc.vram_start >> 18);
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
index c8d478f2afdc..fb2f0eb72f69 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_1.c
@@ -173,8 +173,9 @@ static void mmhub_v3_0_1_init_system_aperture_regs(struct
amdgpu_device *adev)
/* Program the AGP BAR */
WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BASE, 0);
- WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
- WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
+ WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, 0);
+ WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, 0x00FF);
+
/*
* the new L1 policy will block SRIOV guest from writing
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
index 51580302ec42..c30e40e52fb2 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v3_0_2.c
@@ -162,10 +162,10 @@ static void mmhub_v3_0_2_init_system_aperture_regs(struct
amdgpu_device *adev)
uint64_t value;
uint32_t tmp;
- /* Program the AGP BAR */
+ /* Disable AGP. */
WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BASE, 0);
- WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
- WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
+ WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_TOP, 0);
+ WREG32_SOC15(MMHUB, 0, regMMMC_VM_AGP_BOT, 0x00FF);
if (!amdgpu_sriov_vf(adev)) {
/*
--
2.39.1
Re: [PATCH v2] drm/amdkfd: Prevent user space using both svm and kfd api to register same user buffer
Am 2023-02-08 um 14:57 schrieb Xiaogang.Chen:
From: Xiaogang Chen
When xnack is on user space can use svm page restore to set a vm range without
setup it first, then use regular api to register. Currently kfd api and svm are
not interoperable. We already have check on that, but for user buffer the
mapping
address is not same as buffer cpu virtual address. Add checking on that to
avoid error propagate to hmm.
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index f79b8e964140..6d9cf860d2da 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1065,6 +1065,21 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file
*filep,
mutex_unlock(>svms.lock);
return -EADDRINUSE;
}
+
+ /* When register user buffer check if it has been registered by svm by
+* buffer cpu virtual address.
+*/
+ if ((flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) &&
+ interval_tree_iter_first(>svms.objects,
+ args->mmap_offset >> PAGE_SHIFT,
+ (args->mmap_offset + args->size - 1) >> PAGE_SHIFT)) {
The indentation would be more readable if it was properly aligned with
the opening (, both for the if statement and the
interval_tree_iter_first call. Our coding style is not completely
consistent in this, but the existing call of interval_tree_iter_first in
this function uses that style for reference.
Regards,
Felix
+
+ pr_err("User Buffer Address: 0x%llx already allocated by SVM\n",
+ args->mmap_offset);
+ mutex_unlock(>svms.lock);
+ return -EADDRINUSE;
+ }
+
mutex_unlock(>svms.lock);
#endif
mutex_lock(>mutex);
Re: [PATCH] drm/amd/display: don't call dc_interrupt_set() for disabled crtcs
On 2/8/23 15:01, Hamza Mahfooz wrote:
> As made mention of in commit 4ea7fc09539b ("drm/amd/display: Do not
> program interrupt status on disabled crtc"), we shouldn't program
> disabled crtcs. So, filter out disabled crtcs in dm_set_vupdate_irq()
> and dm_set_vblank().
>
> Fixes: 589d2739332d ("drm/amd/display: Use crtc enable/disable_vblank hooks")
> Fixes: d2574c33bb71 ("drm/amd/display: In VRR mode, do DRM core vblank
> handling at end of vblank. (v2)")
> Signed-off-by: Hamza Mahfooz
Reviewed-by: Harry Wentland
Harry
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> index 1e39d0939700..dc4f37240beb 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> @@ -77,6 +77,9 @@ int dm_set_vupdate_irq(struct drm_crtc *crtc, bool enable)
> struct amdgpu_device *adev = drm_to_adev(crtc->dev);
> int rc;
>
> + if (acrtc->otg_inst == -1)
> + return 0;
> +
> irq_source = IRQ_TYPE_VUPDATE + acrtc->otg_inst;
>
> rc = dc_interrupt_set(adev->dm.dc, irq_source, enable) ? 0 : -EBUSY;
> @@ -151,6 +154,9 @@ static inline int dm_set_vblank(struct drm_crtc *crtc,
> bool enable)
> struct vblank_control_work *work;
> int rc = 0;
>
> + if (acrtc->otg_inst == -1)
> + goto skip;
> +
> if (enable) {
> /* vblank irq on -> Only need vupdate irq in vrr mode */
> if (amdgpu_dm_vrr_active(acrtc_state))
> @@ -168,6 +174,7 @@ static inline int dm_set_vblank(struct drm_crtc *crtc,
> bool enable)
> if (!dc_interrupt_set(adev->dm.dc, irq_source, enable))
> return -EBUSY;
>
> +skip:
> if (amdgpu_in_reset(adev))
> return 0;
>
[PATCH] drm/amd/display: don't call dc_interrupt_set() for disabled crtcs
As made mention of in commit 4ea7fc09539b ("drm/amd/display: Do not
program interrupt status on disabled crtc"), we shouldn't program
disabled crtcs. So, filter out disabled crtcs in dm_set_vupdate_irq()
and dm_set_vblank().
Fixes: 589d2739332d ("drm/amd/display: Use crtc enable/disable_vblank hooks")
Fixes: d2574c33bb71 ("drm/amd/display: In VRR mode, do DRM core vblank handling
at end of vblank. (v2)")
Signed-off-by: Hamza Mahfooz
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
index 1e39d0939700..dc4f37240beb 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
@@ -77,6 +77,9 @@ int dm_set_vupdate_irq(struct drm_crtc *crtc, bool enable)
struct amdgpu_device *adev = drm_to_adev(crtc->dev);
int rc;
+ if (acrtc->otg_inst == -1)
+ return 0;
+
irq_source = IRQ_TYPE_VUPDATE + acrtc->otg_inst;
rc = dc_interrupt_set(adev->dm.dc, irq_source, enable) ? 0 : -EBUSY;
@@ -151,6 +154,9 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, bool
enable)
struct vblank_control_work *work;
int rc = 0;
+ if (acrtc->otg_inst == -1)
+ goto skip;
+
if (enable) {
/* vblank irq on -> Only need vupdate irq in vrr mode */
if (amdgpu_dm_vrr_active(acrtc_state))
@@ -168,6 +174,7 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, bool
enable)
if (!dc_interrupt_set(adev->dm.dc, irq_source, enable))
return -EBUSY;
+skip:
if (amdgpu_in_reset(adev))
return 0;
--
2.39.1
[PATCH v2] drm/amdkfd: Prevent user space using both svm and kfd api to register same user buffer
From: Xiaogang Chen
When xnack is on user space can use svm page restore to set a vm range without
setup it first, then use regular api to register. Currently kfd api and svm are
not interoperable. We already have check on that, but for user buffer the
mapping
address is not same as buffer cpu virtual address. Add checking on that to
avoid error propagate to hmm.
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index f79b8e964140..6d9cf860d2da 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1065,6 +1065,21 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file
*filep,
mutex_unlock(>svms.lock);
return -EADDRINUSE;
}
+
+ /* When register user buffer check if it has been registered by svm by
+* buffer cpu virtual address.
+*/
+ if ((flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR) &&
+ interval_tree_iter_first(>svms.objects,
+ args->mmap_offset >> PAGE_SHIFT,
+ (args->mmap_offset + args->size - 1) >> PAGE_SHIFT)) {
+
+ pr_err("User Buffer Address: 0x%llx already allocated by SVM\n",
+ args->mmap_offset);
+ mutex_unlock(>svms.lock);
+ return -EADDRINUSE;
+ }
+
mutex_unlock(>svms.lock);
#endif
mutex_lock(>mutex);
--
2.25.1
Re: [PATCH] drm/amd/display: fix dm irq error message in gpu recover
On 2/8/23 02:25, Tianci Yin wrote:
From: tiancyin
[Why]
Variable adev->crtc_irq.num_types was initialized as the value of
adev->mode_info.num_crtc at early_init stage, later at hw_init stage,
the num_crtc changed due to the display pipe harvest on some SKUs,
but the num_types was not updated accordingly, that cause below error
in gpu recover.
*ERROR* amdgpu_dm_set_crtc_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_crtc_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_crtc_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_vupdate_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_vupdate_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_vupdate_irq_state: crtc is NULL at id :3
[How]
Defer the initialization of num_types to eliminate the error logs.
Signed-off-by: tiancyin
Reviewed-by: Hamza Mahfooz
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index b31cfda30ff9..506699c0d316 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4226,6 +4226,8 @@ static int amdgpu_dm_initialize_drm_device(struct
amdgpu_device *adev)
/* Update the actual used number of crtc */
adev->mode_info.num_crtc = adev->dm.display_indexes_num;
+ amdgpu_dm_set_irq_funcs(adev);
+
link_cnt = dm->dc->caps.max_links;
if (amdgpu_dm_mode_config_init(dm->adev)) {
DRM_ERROR("DM: Failed to initialize mode config\n");
@@ -4714,8 +4716,6 @@ static int dm_early_init(void *handle)
break;
}
- amdgpu_dm_set_irq_funcs(adev);
-
if (adev->mode_info.funcs == NULL)
adev->mode_info.funcs = _display_funcs;
--
Hamza
[linux-next:master] BUILD REGRESSION 38d2b86a665b5e86371a1a30228bce259aa6c101
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 38d2b86a665b5e86371a1a30228bce259aa6c101 Add linux-next specific files for 20230208 Error/Warning reports: https://lore.kernel.org/oe-kbuild-all/202301300743.bp7dpazv-...@intel.com https://lore.kernel.org/oe-kbuild-all/202301301801.y5o08tqx-...@intel.com https://lore.kernel.org/oe-kbuild-all/202301302110.metnwkbd-...@intel.com https://lore.kernel.org/oe-kbuild-all/202301310939.tagcoezb-...@intel.com https://lore.kernel.org/oe-kbuild-all/202302011836.ka3bxqdy-...@intel.com https://lore.kernel.org/oe-kbuild-all/202302061911.c7xvhx9v-...@intel.com https://lore.kernel.org/oe-kbuild-all/202302062224.byzetxh1-...@intel.com Error/Warning: (recently discovered and may have been fixed) Documentation/riscv/uabi.rst:24: WARNING: Enumerated list ends without a blank line; unexpected unindent. ERROR: modpost: "devm_platform_ioremap_resource" [drivers/dma/fsl-edma.ko] undefined! ERROR: modpost: "devm_platform_ioremap_resource" [drivers/dma/idma64.ko] undefined! FAILED: load BTF from vmlinux: No data available arch/arm64/kvm/arm.c:2207: warning: expecting prototype for Initialize Hyp(). Prototype was for kvm_arm_init() instead drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.h:62:10: fatal error: mod_info_packet.h: No such file or directory drivers/gpu/drm/amd/amdgpu/../display/dc/dcn31/dcn31_hubbub.c:1011:6: warning: no previous prototype for 'hubbub31_init' [-Wmissing-prototypes] drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_hubbub.c:948:6: warning: no previous prototype for 'hubbub32_init' [-Wmissing-prototypes] drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_hubp.c:158:6: warning: no previous prototype for 'hubp32_init' [-Wmissing-prototypes] drivers/gpu/drm/amd/amdgpu/../display/dc/dcn32/dcn32_resource_helpers.c:62:18: warning: variable 'cursor_bpp' set but not used [-Wunused-but-set-variable] drivers/gpu/drm/amd/amdgpu/../display/dc/link/accessories/link_dp_trace.c:148:6: warning: no previous prototype for 'link_dp_trace_set_edp_power_timestamp' [-Wmissing-prototypes] drivers/gpu/drm/amd/amdgpu/../display/dc/link/accessories/link_dp_trace.c:158:10: warning: no previous prototype for 'link_dp_trace_get_edp_poweron_timestamp' [-Wmissing-prototypes] drivers/gpu/drm/amd/amdgpu/../display/dc/link/accessories/link_dp_trace.c:163:10: warning: no previous prototype for 'link_dp_trace_get_edp_poweroff_timestamp' [-Wmissing-prototypes] drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_dp_capability.c:1295:32: warning: variable 'result_write_min_hblank' set but not used [-Wunused-but-set-variable] drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_dp_capability.c:279:42: warning: variable 'ds_port' set but not used [-Wunused-but-set-variable] drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_dp_training.c:1585:38: warning: variable 'result' set but not used [-Wunused-but-set-variable] drivers/net/can/dev/bittiming.c:145:24: error: too many arguments to function 'can_calc_bittiming' drivers/net/can/dev/bittiming.c:79:24: error: too many arguments to function 'can_calc_bittiming' ftrace-ops.c:(.init.text+0x2b8): undefined reference to `__udivdi3' libbpf: failed to find '.BTF' ELF section in vmlinux Unverified Error/Warning (likely false positive, please contact us if interested): arch/s390/boot/vmem.c:256 setup_vmem() error: uninitialized symbol 'start'. arch/s390/boot/vmem.c:258 setup_vmem() error: uninitialized symbol 'end'. arch/s390/include/asm/mem_detect.h:86 get_mem_detect_end() error: uninitialized symbol 'end'. Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-link-accessories-link_dp_trace.c:warning:no-previous-prototype-for-link_dp_trace_get_edp_poweroff_timestamp | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-link-accessories-link_dp_trace.c:warning:no-previous-prototype-for-link_dp_trace_get_edp_poweron_timestamp | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-link-accessories-link_dp_trace.c:warning:no-previous-prototype-for-link_dp_trace_set_edp_power_timestamp | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-link-protocols-link_dp_capability.c:warning:variable-ds_port-set-but-not-used | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-link-protocols-link_dp_capability.c:warning:variable-result_write_min_hblank-set-but-not-used | `-- drivers-gpu-drm-amd-amdgpu-..-display-dc-link-protocols-link_dp_training.c:warning:variable-result-set-but-not-used |-- alpha-randconfig-r005-20230205 | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-link-accessories-link_dp_trace.c:warning:no-previous-prototype-for-link_dp_trace_get_edp_poweroff_timestamp | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-link-accessories-link_dp_trace.c:warning:no-previous-prototype-for-link_dp_trace_get_edp_poweron_timestamp | |-- drivers-gpu-drm-amd-
Re: [PATCH] drm/amd/display: fix dm irq error message in gpu recover
On 2/8/23 09:26, Alex Deucher wrote:
On Wed, Feb 8, 2023 at 2:26 AM Tianci Yin wrote:
From: tiancyin
[Why]
Variable adev->crtc_irq.num_types was initialized as the value of
adev->mode_info.num_crtc at early_init stage, later at hw_init stage,
the num_crtc changed due to the display pipe harvest on some SKUs,
but the num_types was not updated accordingly, that cause below error
in gpu recover.
*ERROR* amdgpu_dm_set_crtc_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_crtc_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_crtc_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_vupdate_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_vupdate_irq_state: crtc is NULL at id :3
*ERROR* amdgpu_dm_set_vupdate_irq_state: crtc is NULL at id :3
[How]
Defer the initialization of num_types to eliminate the error logs.
Signed-off-by: tiancyin
Acked-by: Alex Deucher
This seems to be the right thing to do.
Reviewed-by: Harry Wentland
Harry
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index b31cfda30ff9..506699c0d316 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4226,6 +4226,8 @@ static int amdgpu_dm_initialize_drm_device(struct
amdgpu_device *adev)
/* Update the actual used number of crtc */
adev->mode_info.num_crtc = adev->dm.display_indexes_num;
+ amdgpu_dm_set_irq_funcs(adev);
+
link_cnt = dm->dc->caps.max_links;
if (amdgpu_dm_mode_config_init(dm->adev)) {
DRM_ERROR("DM: Failed to initialize mode config\n");
@@ -4714,8 +4716,6 @@ static int dm_early_init(void *handle)
break;
}
- amdgpu_dm_set_irq_funcs(adev);
-
if (adev->mode_info.funcs == NULL)
adev->mode_info.funcs = _display_funcs;
--
2.34.1
Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
On Wed, Feb 08, 2023 at 11:18:42AM +0200, Pekka Paalanen wrote: > On Fri, 3 Feb 2023 16:02:51 +0200 > Ville Syrjälä wrote: > > > On Fri, Feb 03, 2023 at 02:52:50PM +0100, Sebastian Wick wrote: > > > On Fri, Feb 3, 2023 at 2:35 PM Ville Syrjälä > > > wrote: > > > > > > > > On Fri, Feb 03, 2023 at 01:59:07PM +0100, Sebastian Wick wrote: > > > > > On Fri, Feb 3, 2023 at 11:40 AM Ville Syrjälä > > > > > wrote: > > > > > > > > > > > > On Fri, Feb 03, 2023 at 02:07:44AM +, Joshua Ashton wrote: > > > > > > > Userspace has no way of controlling or knowing the pixel encoding > > > > > > > currently, so there is no way for it to ever get the right values > > > > > > > here. > > > > > > > > > > > > That applies to a lot of the other values as well (they are > > > > > > explicitly RGB or YCC). The idea was that this property sets the > > > > > > infoframe/MSA/SDP value exactly, and other properties should be > > > > > > added to for use userspace to control the pixel encoding/colorspace > > > > > > conversion(if desired, or userspace just makes sure to > > > > > > directly feed in correct kind of data). > > > > > > > > > > I'm all for getting userspace control over pixel encoding but even > > > > > then the kernel always knows which pixel encoding is selected and > > > > > which InfoFrame has to be sent. Is there a reason why userspace would > > > > > want to control the variant explicitly to the wrong value? > > > > > > > > What do you mean wrong value? Userspace sets it based on what > > > > kind of data it has generated (or asked the display hardware > > > > to generate if/when we get explicit control over that part). > > > > > > Wrong in the sense of sending the YCC variant when the pixel encoding > > > is RGB for example. > > > > > > Maybe I'm missing something here but my assumption is that the kernel > > > always has to know the pixel encoding anyway. The color pipeline also > > > assumes that the pixel values are RGB. User space might be able to > > > generate YCC content but for subsampling etc the pixel encoding still > > > has to be explicitly set. > > > > The kernel doesn't really know much atm. In theory you can just > > configure the thing to do a straight passthough and put anything you > > want into your pixels. > > But it's impossible to use a YCbCr framebuffer and have that *not* > converted to RGB for the KMS color pipeline even if userspace wanted it > to be strictly pass-through, only to be converted again to YCbCr for > the cable, is it not? > > Even more so with 4:2:0. > > How could it be possible to stop the driver from doing those two > YUV-to-RGB and RGB-to-YCbCr conversions at the beginning and at the end > of the KMS color pipeline? You can stop the conversion at the start of the pipeline by using a "RGB" framebuffer. At the end of the pipe it's not possible with the current props. -- Ville Syrjälä Intel
Re: [PATCH] drm/amd/display: fix dm irq error message in gpu recover
On Wed, Feb 8, 2023 at 2:26 AM Tianci Yin wrote:
>
> From: tiancyin
>
> [Why]
> Variable adev->crtc_irq.num_types was initialized as the value of
> adev->mode_info.num_crtc at early_init stage, later at hw_init stage,
> the num_crtc changed due to the display pipe harvest on some SKUs,
> but the num_types was not updated accordingly, that cause below error
> in gpu recover.
>
> *ERROR* amdgpu_dm_set_crtc_irq_state: crtc is NULL at id :3
> *ERROR* amdgpu_dm_set_crtc_irq_state: crtc is NULL at id :3
> *ERROR* amdgpu_dm_set_crtc_irq_state: crtc is NULL at id :3
> *ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3
> *ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3
> *ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3
> *ERROR* amdgpu_dm_set_pflip_irq_state: crtc is NULL at id :3
> *ERROR* amdgpu_dm_set_vupdate_irq_state: crtc is NULL at id :3
> *ERROR* amdgpu_dm_set_vupdate_irq_state: crtc is NULL at id :3
> *ERROR* amdgpu_dm_set_vupdate_irq_state: crtc is NULL at id :3
>
> [How]
> Defer the initialization of num_types to eliminate the error logs.
>
> Signed-off-by: tiancyin
Acked-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index b31cfda30ff9..506699c0d316 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4226,6 +4226,8 @@ static int amdgpu_dm_initialize_drm_device(struct
> amdgpu_device *adev)
> /* Update the actual used number of crtc */
> adev->mode_info.num_crtc = adev->dm.display_indexes_num;
>
> + amdgpu_dm_set_irq_funcs(adev);
> +
> link_cnt = dm->dc->caps.max_links;
> if (amdgpu_dm_mode_config_init(dm->adev)) {
> DRM_ERROR("DM: Failed to initialize mode config\n");
> @@ -4714,8 +4716,6 @@ static int dm_early_init(void *handle)
> break;
> }
>
> - amdgpu_dm_set_irq_funcs(adev);
> -
> if (adev->mode_info.funcs == NULL)
> adev->mode_info.funcs = _display_funcs;
>
> --
> 2.34.1
>
Re: [PATCH] drm/amdgpu: Fix the warning info when unload or remove amdgpu
On Wed, Feb 8, 2023 at 2:26 AM Ma Jun wrote: > > Checking INVOKE_CMD to fix the below warning info when > unload or remove amdgpu driver > > [ 319.489809] Call Trace: > [ 319.489810] > [ 319.489812] psp_ta_unload+0x9a/0xd0 [amdgpu] > [ 319.489926] ? smu_smc_hw_cleanup+0x2f6/0x360 [amdgpu] > [ 319.490072] psp_hw_fini+0xea/0x170 [amdgpu] > [ 319.490231] amdgpu_device_fini_hw+0x2fc/0x413 [amdgpu] > [ 319.490398] ? blocking_notifier_chain_unregister+0x56/0xb0 > [ 319.490401] amdgpu_driver_unload_kms+0x51/0x60 [amdgpu] > [ 319.490493] amdgpu_pci_remove+0x5a/0x140 [amdgpu] > [ 319.490583] ? __pm_runtime_resume+0x60/0x90 > [ 319.490586] pci_device_remove+0x3b/0xb0 > [ 319.490588] __device_release_driver+0x1a8/0x2a0 > [ 319.490591] driver_detach+0xf3/0x140 > [ 319.490593] bus_remove_driver+0x6c/0xf0 > [ 319.490595] driver_unregister+0x31/0x60 > [ 319.490597] pci_unregister_driver+0x40/0x90 > [ 319.490599] amdgpu_exit+0x15/0x44e [amdgpu] > > Signed-off-by: Ma Jun Acked-by: Alex Deucher > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > index 466054719842..5fb919cd9330 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > @@ -620,7 +620,8 @@ psp_cmd_submit_buf(struct psp_context *psp, > */ > if (!dev_entered) > WARN_ON(psp->cmd_buf_mem->cmd_id != GFX_CMD_ID_LOAD_ASD && > - psp->cmd_buf_mem->cmd_id != GFX_CMD_ID_UNLOAD_TA); > + psp->cmd_buf_mem->cmd_id != GFX_CMD_ID_UNLOAD_TA && > + psp->cmd_buf_mem->cmd_id != GFX_CMD_ID_INVOKE_CMD); > > memset(psp->cmd_buf_mem, 0, PSP_CMD_BUFFER_SIZE); > > -- > 2.25.1 >
Re: [PATCH 3/6] drm/ttm: Change the meaning of resource->start from pfn to bytes
That finally starts to look sane. I'm going to make a few more
adjustments and then send this out.
Christian.
Am 08.02.23 um 10:01 schrieb Somalapuram Amaranath:
Change resource->start from pfn to bytes to
allow allocating objects smaller than a page.
Change all DRM drivers using ttm_resource start and size pfn to bytes.
Change amdgpu_res_first() cur->start, cur->size from pfn to bytes.
Replacing ttm_resource resource->start field with cursor.start.
Change amdgpu_gtt_mgr_new() allocation from pfn to bytes.
Signed-off-by: Somalapuram Amaranath
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 13 +++--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 +++-
drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h | 8
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 10 +++---
.../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 6 +-
drivers/gpu/drm/drm_gem_vram_helper.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_bo.c| 13 ++---
drivers/gpu/drm/nouveau/nouveau_bo0039.c| 4 ++--
drivers/gpu/drm/nouveau/nouveau_mem.c | 10 +-
drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 +-
drivers/gpu/drm/nouveau/nv17_fence.c| 2 +-
drivers/gpu/drm/nouveau/nv50_fence.c| 2 +-
drivers/gpu/drm/qxl/qxl_drv.h | 2 +-
drivers/gpu/drm/qxl/qxl_object.c| 2 +-
drivers/gpu/drm/qxl/qxl_ttm.c | 5 ++---
drivers/gpu/drm/radeon/radeon_object.c | 6 +++---
drivers/gpu/drm/radeon/radeon_object.h | 2 +-
drivers/gpu/drm/radeon/radeon_ttm.c | 13 ++---
drivers/gpu/drm/radeon/radeon_vm.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 3 +--
23 files changed, 63 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 44367f03316f..a1fbfc5984d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -116,7 +116,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager
*man,
struct ttm_resource **res)
{
struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
- uint32_t num_pages = PFN_UP(tbo->base.size);
struct ttm_range_mgr_node *node;
int r;
@@ -134,8 +133,10 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
if (place->lpfn) {
spin_lock(>lock);
r = drm_mm_insert_node_in_range(>mm, >mm_nodes[0],
- num_pages, tbo->page_alignment,
- 0, place->fpfn, place->lpfn,
+ tbo->base.size,
+ tbo->page_alignment <<
PAGE_SHIFT, 0,
+ place->fpfn << PAGE_SHIFT,
+ place->lpfn << PAGE_SHIFT,
DRM_MM_INSERT_BEST);
spin_unlock(>lock);
if (unlikely(r))
@@ -144,7 +145,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager
*man,
node->base.start = node->mm_nodes[0].start;
} else {
node->mm_nodes[0].start = 0;
- node->mm_nodes[0].size = PFN_UP(node->base.size);
+ node->mm_nodes[0].size = node->base.size;
node->base.start = AMDGPU_BO_INVALID_OFFSET;
}
@@ -285,8 +286,8 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size)
ttm_resource_manager_init(man, >mman.bdev, gtt_size);
- start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
- size = (adev->gmc.gart_size >> PAGE_SHIFT) - start;
+ start = (AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS)
<< PAGE_SHIFT;
+ size = adev->gmc.gart_size - start;
drm_mm_init(>mm, start, size);
spin_lock_init(>lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index d835ee2131d2..f5d5eee09cea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1488,9 +1488,11 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo)
{
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+ struct amdgpu_res_cursor cursor;
uint64_t offset;
- offset = (bo->tbo.resource->start << PAGE_SHIFT) +
+ amdgpu_res_first(bo->tbo.resource, 0,
Re: [PATCH 2/6] drm/amdgpu: Remove TTM resource->start visible VRAM condition
Am 08.02.23 um 10:01 schrieb Somalapuram Amaranath:
Use amdgpu_bo_in_cpu_visible_vram() instead.
Signed-off-by: Somalapuram Amaranath
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 981010de0a28..d835ee2131d2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -600,7 +600,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (!amdgpu_gmc_vram_full_visible(>gmc) &&
bo->tbo.resource->mem_type == TTM_PL_VRAM &&
- bo->tbo.resource->start < adev->gmc.visible_vram_size >> PAGE_SHIFT)
+ amdgpu_bo_in_cpu_visible_vram(bo))
amdgpu_cs_report_moved_bytes(adev, ctx.bytes_moved,
ctx.bytes_moved);
else
@@ -1346,7 +1346,6 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct
ttm_buffer_object *bo)
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
struct ttm_operation_ctx ctx = { false, false };
struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
- unsigned long offset;
int r;
/* Remember that this BO was accessed by the CPU */
@@ -1355,8 +1354,7 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct
ttm_buffer_object *bo)
if (bo->resource->mem_type != TTM_PL_VRAM)
return 0;
- offset = bo->resource->start << PAGE_SHIFT;
- if ((offset + bo->base.size) <= adev->gmc.visible_vram_size)
+ if (amdgpu_bo_in_cpu_visible_vram(abo))
return 0;
/* Can't move a pinned BO to visible VRAM */
@@ -1378,10 +1376,9 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct
ttm_buffer_object *bo)
else if (unlikely(r))
return VM_FAULT_SIGBUS;
- offset = bo->resource->start << PAGE_SHIFT;
/* this should never happen */
if (bo->resource->mem_type == TTM_PL_VRAM &&
- (offset + bo->base.size) > adev->gmc.visible_vram_size)
+ amdgpu_bo_in_cpu_visible_vram(abo))
This check needs to be inversed. E.g. we return the error if the BO is
not in visible VRAM.
Apart from that the patch looks good to me.
Regards,
Christian.
return VM_FAULT_SIGBUS;
ttm_bo_move_to_lru_tail_unlocked(bo);
Re: [PATCH] drm: Rename headers to match DP2.1 spec
On Mon, Feb 06, 2023 at 02:37:08PM -0500, jdhillon wrote:
> This patch changes the headers defined in drm_dp.h to match
> the DP 2.1 spec.
>
> Signed-off-by: Jasdeep Dhillon
> ---
> drivers/gpu/drm/tegra/dp.c | 2 +-
> include/drm/display/drm_dp.h | 13 +++--
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/dp.c b/drivers/gpu/drm/tegra/dp.c
> index 08fbd8f151a1..f33e468ece0a 100644
> --- a/drivers/gpu/drm/tegra/dp.c
> +++ b/drivers/gpu/drm/tegra/dp.c
> @@ -499,7 +499,7 @@ static int drm_dp_link_apply_training(struct drm_dp_link
> *link)
> for (i = 0; i < lanes; i++)
> values[i / 2] |= DP_LANE_POST_CURSOR(i, pc[i]);
>
> - err = drm_dp_dpcd_write(aux, DP_TRAINING_LANE0_1_SET2, values,
> + err = drm_dp_dpcd_write(aux, DP_LINK_SQUARE_PATTERN, values,
> DIV_ROUND_UP(lanes, 2));
> if (err < 0) {
> DRM_ERROR("failed to set post-cursor: %d\n", err);
> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
> index ed10e6b6f99d..2093c1f8d8e0 100644
> --- a/include/drm/display/drm_dp.h
> +++ b/include/drm/display/drm_dp.h
> @@ -641,12 +641,11 @@
> # define DP_LINK_QUAL_PATTERN_CUSTOM0x40
> # define DP_LINK_QUAL_PATTERN_SQUARE0x48
>
> -#define DP_TRAINING_LANE0_1_SET2 0x10f
> -#define DP_TRAINING_LANE2_3_SET2 0x110
> -# define DP_LANE02_POST_CURSOR2_SET_MASK(3 << 0)
> -# define DP_LANE02_MAX_POST_CURSOR2_REACHED (1 << 2)
> -# define DP_LANE13_POST_CURSOR2_SET_MASK(3 << 4)
> -# define DP_LANE13_MAX_POST_CURSOR2_REACHED (1 << 6)
> +#define DP_LINK_SQUARE_PATTERN 0x10f
I think it'd be better to add new definitions for these rather than
replace the existing ones. DP v1.2 calls these TRAINING_LANE0_1_SET2 and
TRAINING_LANE2_3_SET2, respectively, so within the context of DP v1.2
hardware the new names don't make any sense.
While at it, maybe add a comment to the new definitions that the old
ones were deprecated in DP v1.3 and have been repurposed in v2.0 and
v2.1, respectively.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
On Mon, 6 Feb 2023 12:16:28 -0500 Harry Wentland wrote: > On 2/6/23 04:47, Ville Syrjälä wrote: > > On Sat, Feb 04, 2023 at 06:09:45AM +, Joshua Ashton wrote: > >> > >> > >> On 2/3/23 19:34, Ville Syrjälä wrote: > >>> On Fri, Feb 03, 2023 at 09:25:38PM +0200, Ville Syrjälä wrote: > On Fri, Feb 03, 2023 at 08:56:55PM +0200, Ville Syrjälä wrote: > > On Fri, Feb 03, 2023 at 01:28:20PM -0500, Harry Wentland wrote: > >> > >> > >> On 2/3/23 11:00, Ville Syrjälä wrote: > >>> On Fri, Feb 03, 2023 at 10:24:52AM -0500, Harry Wentland wrote: > > > On 2/3/23 10:19, Ville Syrjälä wrote: > > On Fri, Feb 03, 2023 at 09:39:42AM -0500, Harry Wentland wrote: > >> > >> > >> On 2/3/23 07:59, Sebastian Wick wrote: > >>> On Fri, Feb 3, 2023 at 11:40 AM Ville Syrjälä > >>> wrote: > > On Fri, Feb 03, 2023 at 02:07:44AM +, Joshua Ashton wrote: > > Userspace has no way of controlling or knowing the pixel > > encoding > > currently, so there is no way for it to ever get the right > > values here. > > That applies to a lot of the other values as well (they are > explicitly RGB or YCC). The idea was that this property sets the > infoframe/MSA/SDP value exactly, and other properties should be > added to for use userspace to control the pixel > encoding/colorspace > conversion(if desired, or userspace just makes sure to > directly feed in correct kind of data). > >>> > >>> I'm all for getting userspace control over pixel encoding but even > >>> then the kernel always knows which pixel encoding is selected and > >>> which InfoFrame has to be sent. Is there a reason why userspace > >>> would > >>> want to control the variant explicitly to the wrong value? > >>> > >> > >> I've asked this before but haven't seen an answer: Is there an > >> existing > >> upstream userspace project that makes use of this property (other > >> than > >> what Joshua is working on in gamescope right now)? That would help > >> us > >> understand the intent better. > > > > The intent was to control the infoframe colorimetry bits, > > nothing more. No idea what real userspace there was, if any. > >> > >> Controlling the infoframe alone isn't useful at all unless you can > >> guarantee the wire encoding, which we cannot do. > >> > > > >> > >> I don't think giving userspace explicit control over the exact > >> infoframe > >> values is the right thing to do. > >> > >> +1 > >> > > > > Only userspace knows what kind of data it's stuffing into > > the pixels (and/or how it configures the csc units/etc.) to > > generate them. > > > > Yes, but userspace doesn't control or know whether we drive > RGB or YCbCr on the wire. In fact, in some cases our driver > needs to fallback to YCbCr420 for bandwidth reasons. There > is currently no way for userspace to know that and I don't > think it makes sense. > >>> > >>> People want that control as well for whatever reason. We've > >>> been asked to allow YCbCr 4:4:4 output many times. > >>> > >>> The automagic 4:2:0 fallback I think is rather fundementally > >>> incompatible with fancy color management. How would we even > >>> know whether to use eg. BT.2020 vs. BT.709 matrix? In i915 > >>> that stuff is just always BT.709 limited range, no questions > >>> asked. > >> > >> That's what the Colorspace property *should* be determining here. > >> That's what we have it set up to do in SteamOS/my tree right now. > >> > >>> > >> > >> We use what we're telling the display, i.e., the value in the > >> colorspace property. That way we know whether to use a BT.2020 > >> or BT.709 matrix. > > > > And given how these things have gone in the past I think > > that is likey to bite someone at in the future. Also not > > what this property was meant to do nor does on any other > > driver AFAIK. > > > >> I don't see how it's fundamentally incompatible with fancy > >> color management stuff. > >> > >> If we start forbidding drivers from falling back to YCbCr > >> (whether 4:4:4 or 4:2:0) we will break existing behavior on > >> amdgpu and will see bug reports. > > > > The compositors could deal with that if/when they start doing > > the full color management stuff. The current stuff only really > > works when the kernel is allowed to do whatever it wants. > > > >> > >>> So I think if userspace
Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
On Fri, 3 Feb 2023 14:33:46 -0500 Harry Wentland wrote: > On 2/3/23 14:25, Ville Syrjälä wrote: > > On Fri, Feb 03, 2023 at 08:56:55PM +0200, Ville Syrjälä wrote: > >> On Fri, Feb 03, 2023 at 01:28:20PM -0500, Harry Wentland wrote: > >>> > >>> > >>> On 2/3/23 11:00, Ville Syrjälä wrote: > On Fri, Feb 03, 2023 at 10:24:52AM -0500, Harry Wentland wrote: > > > > > > On 2/3/23 10:19, Ville Syrjälä wrote: > >> On Fri, Feb 03, 2023 at 09:39:42AM -0500, Harry Wentland wrote: > >>> > >>> > >>> On 2/3/23 07:59, Sebastian Wick wrote: > On Fri, Feb 3, 2023 at 11:40 AM Ville Syrjälä > wrote: > > > > On Fri, Feb 03, 2023 at 02:07:44AM +, Joshua Ashton wrote: > >> Userspace has no way of controlling or knowing the pixel encoding > >> currently, so there is no way for it to ever get the right values > >> here. > > > > That applies to a lot of the other values as well (they are > > explicitly RGB or YCC). The idea was that this property sets the > > infoframe/MSA/SDP value exactly, and other properties should be > > added to for use userspace to control the pixel encoding/colorspace > > conversion(if desired, or userspace just makes sure to > > directly feed in correct kind of data). > > I'm all for getting userspace control over pixel encoding but even > then the kernel always knows which pixel encoding is selected and > which InfoFrame has to be sent. Is there a reason why userspace would > want to control the variant explicitly to the wrong value? > > >>> > >>> I've asked this before but haven't seen an answer: Is there an > >>> existing > >>> upstream userspace project that makes use of this property (other than > >>> what Joshua is working on in gamescope right now)? That would help us > >>> understand the intent better. > >> > >> The intent was to control the infoframe colorimetry bits, > >> nothing more. No idea what real userspace there was, if any. > >> > >>> > >>> I don't think giving userspace explicit control over the exact > >>> infoframe > >>> values is the right thing to do. > >> > >> Only userspace knows what kind of data it's stuffing into > >> the pixels (and/or how it configures the csc units/etc.) to > >> generate them. > >> > > > > Yes, but userspace doesn't control or know whether we drive > > RGB or YCbCr on the wire. In fact, in some cases our driver > > needs to fallback to YCbCr420 for bandwidth reasons. There > > is currently no way for userspace to know that and I don't > > think it makes sense. > > People want that control as well for whatever reason. We've > been asked to allow YCbCr 4:4:4 output many times. > > The automagic 4:2:0 fallback I think is rather fundementally > incompatible with fancy color management. How would we even > know whether to use eg. BT.2020 vs. BT.709 matrix? In i915 > that stuff is just always BT.709 limited range, no questions > asked. > > >>> > >>> We use what we're telling the display, i.e., the value in the > >>> colorspace property. That way we know whether to use a BT.2020 > >>> or BT.709 matrix. > >> > >> And given how these things have gone in the past I think > >> that is likey to bite someone at in the future. Also not > >> what this property was meant to do nor does on any other > >> driver AFAIK. > >> > >>> I don't see how it's fundamentally incompatible with fancy > >>> color management stuff. > >>> > >>> If we start forbidding drivers from falling back to YCbCr > >>> (whether 4:4:4 or 4:2:0) we will break existing behavior on > >>> amdgpu and will see bug reports. > >> > >> The compositors could deal with that if/when they start doing > >> the full color management stuff. The current stuff only really > >> works when the kernel is allowed to do whatever it wants. > >> > >>> > So I think if userspace wants real color management it's > going to have to set up the whole pipeline. And for that > we need at least one new property to control the RGB->YCbCr > conversion (or to explicitly avoid it). > > And given that the proposed patch just swept all the > non-BT.2020 issues under the rug makes me think no > one has actually come up with any kind of consistent > plan for anything else really. > > >>> > >>> Does anyone actually use the non-BT.2020 colorspace stuff? > >> > >> No idea if anyone is using any of it. It's a bit hard to do > >> right now outside the full passthrough case since we have no > >> properties to control how the hardware will convert stuff. > >> > >> Anyways, sounds like what you're basically proposing is > >> getting rid of this property and starting from scratch. > > > > Hmm. I
[PATCH] drm/amdgpu: Revert programming GRBM_GFX_* in RLCG interface to support GFX9
[Why]
Regression of commit a291321cce8e("drm/amdgpu: Remove writing GRBM_GFX_CNTL in
RLCG interface under SRIOV") on GFX9.
According to GFX9 VF using different method to access GC registers including
MMIO(direct) and RLCG(indirect),
removing GRBM_GFX_* writing would make PIPE/ME/VM/QUEUE selection chaos leading
to some OCL benchmark failure.
For example,
using RLCG interface to program GRBM_GFX_CNTL/INDEX for selecting MEC(actually
the value is only in scratch2/3),
then using MMIO directly program a MEC register in VF driver.
The register programming are invalid due to GC switched to incorrect ME.
[How]
With checking RLCG accessing flag, keep writing GRBM_GFX_* as a legacy way.
But it is still skipped on GFX10+ to avoid violation occurrence.
Signed-off-by: Yifan Zha
---
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index ca5a1d026f5a..f2e2cbaa7fde 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -983,9 +983,13 @@ static u32 amdgpu_virt_rlcg_reg_rw(struct amdgpu_device
*adev, u32 offset, u32 v
if (offset == reg_access_ctrl->grbm_cntl) {
/* if the target reg offset is grbm_cntl, write to scratch_reg2
*/
writel(v, scratch_reg2);
+ if (flag == AMDGPU_RLCG_GC_WRITE_LEGACY)
+ writel(v, ((void __iomem *)adev->rmmio) + (offset * 4));
} else if (offset == reg_access_ctrl->grbm_idx) {
/* if the target reg offset is grbm_idx, write to scratch_reg3
*/
writel(v, scratch_reg3);
+ if (flag == AMDGPU_RLCG_GC_WRITE_LEGACY)
+ writel(v, ((void __iomem *)adev->rmmio) + (offset * 4));
} else {
/*
* SCRATCH_REG0 = read/write value
--
2.25.1
Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
On Fri, 3 Feb 2023 02:07:44 +
Joshua Ashton wrote:
> Userspace has no way of controlling or knowing the pixel encoding
> currently, so there is no way for it to ever get the right values here.
>
> When we do add pixel_encoding control from userspace,we can pick the
> right value for the colorimetry packet based on the
> pixel_encoding + the colorspace.
>
> Let's deprecate these values, and have one BT.2020 colorspace entry
> that userspace can use.
>
> Note: _CYCC was effectively 'removed' by this change, but that was not
> possible to be taken advantage of anyway, as there is currently no
> pixel_encoding control so it would not be possible to output
> linear YCbCr.
>
> Signed-off-by: Joshua Ashton
>
> Cc: Pekka Paalanen
> Cc: Sebastian Wick
> Cc: vitaly.pros...@amd.com
> Cc: Uma Shankar
> Cc: Ville Syrjälä
> Cc: Joshua Ashton
> Cc: dri-de...@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> ---
> drivers/gpu/drm/display/drm_hdmi_helper.c | 9 -
> drivers/gpu/drm/drm_connector.c | 12 ++--
> drivers/gpu/drm/i915/display/intel_dp.c | 20 +---
> include/drm/drm_connector.h | 19 ++-
> 4 files changed, 29 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_hdmi_helper.c
> b/drivers/gpu/drm/display/drm_hdmi_helper.c
> index 0264abe55278..c85860600395 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_helper.c
> @@ -99,8 +99,7 @@ EXPORT_SYMBOL(drm_hdmi_infoframe_set_hdr_metadata);
> #define HDMI_COLORIMETRY_OPYCC_601 (C(3) | EC(3) | ACE(0))
> #define HDMI_COLORIMETRY_OPRGB (C(3) | EC(4) | ACE(0))
> #define HDMI_COLORIMETRY_BT2020_CYCC (C(3) | EC(5) | ACE(0))
> -#define HDMI_COLORIMETRY_BT2020_RGB (C(3) | EC(6) | ACE(0))
> -#define HDMI_COLORIMETRY_BT2020_YCC (C(3) | EC(6) | ACE(0))
> +#define HDMI_COLORIMETRY_BT2020 (C(3) | EC(6) | ACE(0))
> #define HDMI_COLORIMETRY_DCI_P3_RGB_D65 (C(3) | EC(7) | ACE(0))
> #define HDMI_COLORIMETRY_DCI_P3_RGB_THEATER (C(3) | EC(7) | ACE(1))
>
> @@ -113,9 +112,9 @@ static const u32 hdmi_colorimetry_val[] = {
> [DRM_MODE_COLORIMETRY_SYCC_601] = HDMI_COLORIMETRY_SYCC_601,
> [DRM_MODE_COLORIMETRY_OPYCC_601] = HDMI_COLORIMETRY_OPYCC_601,
> [DRM_MODE_COLORIMETRY_OPRGB] = HDMI_COLORIMETRY_OPRGB,
> - [DRM_MODE_COLORIMETRY_BT2020_CYCC] = HDMI_COLORIMETRY_BT2020_CYCC,
> - [DRM_MODE_COLORIMETRY_BT2020_RGB] = HDMI_COLORIMETRY_BT2020_RGB,
> - [DRM_MODE_COLORIMETRY_BT2020_YCC] = HDMI_COLORIMETRY_BT2020_YCC,
> + [DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_1] = HDMI_COLORIMETRY_BT2020,
> + [DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_2] = HDMI_COLORIMETRY_BT2020,
> + [DRM_MODE_COLORIMETRY_BT2020] = HDMI_COLORIMETRY_BT2020,
> };
>
> #undef C
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 61c29ce74b03..58699ab15a6a 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1029,11 +1029,11 @@ static const struct drm_prop_enum_list
> hdmi_colorspaces[] = {
> /* Colorimetry based on IEC 61966-2-5 */
> { DRM_MODE_COLORIMETRY_OPRGB, "opRGB" },
> /* Colorimetry based on ITU-R BT.2020 */
> - { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
> + { DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_1, "BT2020_DEPRECATED_1" },
> /* Colorimetry based on ITU-R BT.2020 */
> - { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
> + { DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_2, "BT2020_DEPRECATED_2" },
> /* Colorimetry based on ITU-R BT.2020 */
> - { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
> + { DRM_MODE_COLORIMETRY_BT2020, "BT2020" },
> /* Added as part of Additional Colorimetry Extension in 861.G */
> { DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65, "DCI-P3_RGB_D65" },
> { DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER, "DCI-P3_RGB_Theater" },
> @@ -1054,7 +1054,7 @@ static const struct drm_prop_enum_list dp_colorspaces[]
> = {
> /* Colorimetry based on SMPTE RP 431-2 */
> { DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65, "DCI-P3_RGB_D65" },
> /* Colorimetry based on ITU-R BT.2020 */
> - { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
> + { DRM_MODE_COLORIMETRY_BT2020, "BT2020" },
> { DRM_MODE_COLORIMETRY_BT601_YCC, "BT601_YCC" },
> { DRM_MODE_COLORIMETRY_BT709_YCC, "BT709_YCC" },
> /* Standard Definition Colorimetry based on IEC 61966-2-4 */
> @@ -1066,9 +1066,9 @@ static const struct drm_prop_enum_list dp_colorspaces[]
> = {
> /* Colorimetry based on IEC 61966-2-5 [33] */
> { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" },
> /* Colorimetry based on ITU-R BT.2020 */
> - { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
> + { DRM_MODE_COLORIMETRY_BT2020_DEPRECATED_1, "BT2020_DEPRECATED_1" },
> /*
Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
On Fri, 3 Feb 2023 20:56:55 +0200 Ville Syrjälä wrote: > Anyways, sounds like what you're basically proposing is > getting rid of this property and starting from scratch. I would be happy with that (throwing "Colorspace" out and defining something new). Then we can start with enum values we care and know about. Thanks, pq pgpNyNicMbsuM.pgp Description: OpenPGP digital signature
Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
On Fri, 3 Feb 2023 18:00:44 +0200 Ville Syrjälä wrote: > On Fri, Feb 03, 2023 at 10:24:52AM -0500, Harry Wentland wrote: > > > > > > On 2/3/23 10:19, Ville Syrjälä wrote: > > > On Fri, Feb 03, 2023 at 09:39:42AM -0500, Harry Wentland wrote: > > >> > > >> > > >> On 2/3/23 07:59, Sebastian Wick wrote: > > >>> On Fri, Feb 3, 2023 at 11:40 AM Ville Syrjälä > > >>> wrote: > > > > On Fri, Feb 03, 2023 at 02:07:44AM +, Joshua Ashton wrote: > > > Userspace has no way of controlling or knowing the pixel encoding > > > currently, so there is no way for it to ever get the right values > > > here. > > > > That applies to a lot of the other values as well (they are > > explicitly RGB or YCC). The idea was that this property sets the > > infoframe/MSA/SDP value exactly, and other properties should be > > added to for use userspace to control the pixel encoding/colorspace > > conversion(if desired, or userspace just makes sure to > > directly feed in correct kind of data). > > >>> > > >>> I'm all for getting userspace control over pixel encoding but even > > >>> then the kernel always knows which pixel encoding is selected and > > >>> which InfoFrame has to be sent. Is there a reason why userspace would > > >>> want to control the variant explicitly to the wrong value? > > >>> > > >> > > >> I've asked this before but haven't seen an answer: Is there an existing > > >> upstream userspace project that makes use of this property (other than > > >> what Joshua is working on in gamescope right now)? That would help us > > >> understand the intent better. > > > > > > The intent was to control the infoframe colorimetry bits, > > > nothing more. No idea what real userspace there was, if any. > > > > > >> > > >> I don't think giving userspace explicit control over the exact infoframe > > >> values is the right thing to do. > > > > > > Only userspace knows what kind of data it's stuffing into > > > the pixels (and/or how it configures the csc units/etc.) to > > > generate them. > > > > > > > Yes, but userspace doesn't control or know whether we drive > > RGB or YCbCr on the wire. In fact, in some cases our driver > > needs to fallback to YCbCr420 for bandwidth reasons. There > > is currently no way for userspace to know that and I don't > > think it makes sense. > > People want that control as well for whatever reason. We've > been asked to allow YCbCr 4:4:4 output many times. > > The automagic 4:2:0 fallback I think is rather fundementally > incompatible with fancy color management. How would we even > know whether to use eg. BT.2020 vs. BT.709 matrix? In i915 > that stuff is just always BT.709 limited range, no questions > asked. The difference between 4:4:4 and 4:2:0 is purely the sub-sampling. It has absolutely no implication to colorimetry nor MatrixCoefficients at all. Thanks, pq pgprYHeBRUNlQ.pgp Description: OpenPGP digital signature
Re: [PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum
On Fri, 3 Feb 2023 16:02:51 +0200 Ville Syrjälä wrote: > On Fri, Feb 03, 2023 at 02:52:50PM +0100, Sebastian Wick wrote: > > On Fri, Feb 3, 2023 at 2:35 PM Ville Syrjälä > > wrote: > > > > > > On Fri, Feb 03, 2023 at 01:59:07PM +0100, Sebastian Wick wrote: > > > > On Fri, Feb 3, 2023 at 11:40 AM Ville Syrjälä > > > > wrote: > > > > > > > > > > On Fri, Feb 03, 2023 at 02:07:44AM +, Joshua Ashton wrote: > > > > > > Userspace has no way of controlling or knowing the pixel encoding > > > > > > currently, so there is no way for it to ever get the right values > > > > > > here. > > > > > > > > > > That applies to a lot of the other values as well (they are > > > > > explicitly RGB or YCC). The idea was that this property sets the > > > > > infoframe/MSA/SDP value exactly, and other properties should be > > > > > added to for use userspace to control the pixel encoding/colorspace > > > > > conversion(if desired, or userspace just makes sure to > > > > > directly feed in correct kind of data). > > > > > > > > I'm all for getting userspace control over pixel encoding but even > > > > then the kernel always knows which pixel encoding is selected and > > > > which InfoFrame has to be sent. Is there a reason why userspace would > > > > want to control the variant explicitly to the wrong value? > > > > > > What do you mean wrong value? Userspace sets it based on what > > > kind of data it has generated (or asked the display hardware > > > to generate if/when we get explicit control over that part). > > > > Wrong in the sense of sending the YCC variant when the pixel encoding > > is RGB for example. > > > > Maybe I'm missing something here but my assumption is that the kernel > > always has to know the pixel encoding anyway. The color pipeline also > > assumes that the pixel values are RGB. User space might be able to > > generate YCC content but for subsampling etc the pixel encoding still > > has to be explicitly set. > > The kernel doesn't really know much atm. In theory you can just > configure the thing to do a straight passthough and put anything you > want into your pixels. But it's impossible to use a YCbCr framebuffer and have that *not* converted to RGB for the KMS color pipeline even if userspace wanted it to be strictly pass-through, only to be converted again to YCbCr for the cable, is it not? Even more so with 4:2:0. How could it be possible to stop the driver from doing those two YUV-to-RGB and RGB-to-YCbCr conversions at the beginning and at the end of the KMS color pipeline? From uAPI point of view: "Colorspace" currently defines (or does it? see my patch 2 review) the colorimetry and the color model encoding. If a driver chooses the cable encoding independently, the "Colorspace" color model encoding is often wrong. If we have another KMS property to choose the cable encoding, then it is possible to still set "Colorspace" to disagree with the actual cable encoding. What's the use of that possibility to configure things wrong? Thanks, pq > > > > > So with the kernel always knowing exactly what pixel encoding is sent, > > why do we need those variants? I just don't see why this is necessary. > > > > > > > > -- > > > Ville Syrjälä > > > Intel > > > > pgpWh1fOGS_Au.pgp Description: OpenPGP digital signature
[PATCH 6/6] drm/amdgpu: Cleanup the GDS, GWS and OA allocations
Change the size of GDS, GWS and OA from pages to bytes.
The initialized gds_size, gws_size and oa_size in bytes,
remove PAGE_SHIFT in amdgpu_ttm_init_on_chip().
:
Signed-off-by: Somalapuram Amaranath
---
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c| 12 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 3 +--
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index c3d9d75143f4..4641b25956fd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -142,16 +142,16 @@ void amdgpu_job_set_resources(struct amdgpu_job *job,
struct amdgpu_bo *gds,
struct amdgpu_bo *gws, struct amdgpu_bo *oa)
{
if (gds) {
- job->gds_base = amdgpu_bo_gpu_offset(gds) >> PAGE_SHIFT;
- job->gds_size = amdgpu_bo_size(gds) >> PAGE_SHIFT;
+ job->gds_base = amdgpu_bo_gpu_offset(gds);
+ job->gds_size = amdgpu_bo_size(gds);
}
if (gws) {
- job->gws_base = amdgpu_bo_gpu_offset(gws) >> PAGE_SHIFT;
- job->gws_size = amdgpu_bo_size(gws) >> PAGE_SHIFT;
+ job->gws_base = amdgpu_bo_gpu_offset(gws);
+ job->gws_size = amdgpu_bo_size(gws);
}
if (oa) {
- job->oa_base = amdgpu_bo_gpu_offset(oa) >> PAGE_SHIFT;
- job->oa_size = amdgpu_bo_size(oa) >> PAGE_SHIFT;
+ job->oa_base = amdgpu_bo_gpu_offset(oa);
+ job->oa_size = amdgpu_bo_size(oa);
}
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index f5d5eee09cea..9285037d6d88 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -541,12 +541,11 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA)) {
/* GWS and OA don't need any alignment. */
page_align = bp->byte_align;
- size <<= PAGE_SHIFT;
} else if (bp->domain & AMDGPU_GEM_DOMAIN_GDS) {
/* Both size and alignment must be a multiple of 4. */
page_align = ALIGN(bp->byte_align, 4);
- size = ALIGN(size, 4) << PAGE_SHIFT;
+ size = ALIGN(size, 4);
} else {
/* Memory should be aligned at least to a page size. */
page_align = ALIGN(bp->byte_align, PAGE_SIZE) >> PAGE_SHIFT;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index f0dabdfd3780..a8e444a31d8f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -77,8 +77,7 @@ static int amdgpu_ttm_init_on_chip(struct amdgpu_device *adev,
unsigned int type,
uint64_t size)
{
- return ttm_range_man_init(>mman.bdev, type,
- false, size << PAGE_SHIFT);
+ return ttm_range_man_init(>mman.bdev, type, false, size);
}
/**
--
2.32.0
[PATCH 5/6] drm/ttm: Change the meaning of the fields in the drm_mm_nodes structure from pfn to bytes
Change the ttm_range_man_alloc() allocation from pages to size in bytes.
Fix the dependent drm_mm_nodes start and size from pages to bytes.
Signed-off-by: Somalapuram Amaranath
---
drivers/gpu/drm/i915/i915_scatterlist.c | 6 +++---
drivers/gpu/drm/ttm/ttm_range_manager.c | 15 +++
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_scatterlist.c
b/drivers/gpu/drm/i915/i915_scatterlist.c
index 756289e43dff..7defda1219d0 100644
--- a/drivers/gpu/drm/i915/i915_scatterlist.c
+++ b/drivers/gpu/drm/i915/i915_scatterlist.c
@@ -94,7 +94,7 @@ struct i915_refct_sgt *i915_rsgt_from_mm_node(const struct
drm_mm_node *node,
if (!rsgt)
return ERR_PTR(-ENOMEM);
- i915_refct_sgt_init(rsgt, node->size << PAGE_SHIFT);
+ i915_refct_sgt_init(rsgt, node->size);
st = >table;
/* restricted by sg_alloc_table */
if (WARN_ON(overflows_type(DIV_ROUND_UP_ULL(node->size, segment_pages),
@@ -110,8 +110,8 @@ struct i915_refct_sgt *i915_rsgt_from_mm_node(const struct
drm_mm_node *node,
sg = st->sgl;
st->nents = 0;
prev_end = (resource_size_t)-1;
- block_size = node->size << PAGE_SHIFT;
- offset = node->start << PAGE_SHIFT;
+ block_size = node->size;
+ offset = node->start;
while (block_size) {
u64 len;
diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c
b/drivers/gpu/drm/ttm/ttm_range_manager.c
index 62fddcc59f02..ff9962f7f81d 100644
--- a/drivers/gpu/drm/ttm/ttm_range_manager.c
+++ b/drivers/gpu/drm/ttm/ttm_range_manager.c
@@ -83,9 +83,10 @@ static int ttm_range_man_alloc(struct ttm_resource_manager
*man,
spin_lock(>lock);
ret = drm_mm_insert_node_in_range(mm, >mm_nodes[0],
- PFN_UP(node->base.size),
- bo->page_alignment, 0,
- place->fpfn, lpfn, mode);
+ node->base.size,
+ bo->page_alignment << PAGE_SHIFT, 0,
+ place->fpfn << PAGE_SHIFT,
+ lpfn << PAGE_SHIFT, mode);
spin_unlock(>lock);
if (unlikely(ret)) {
@@ -119,11 +120,10 @@ static bool ttm_range_man_intersects(struct
ttm_resource_manager *man,
size_t size)
{
struct drm_mm_node *node = _ttm_range_mgr_node(res)->mm_nodes[0];
- u32 num_pages = PFN_UP(size);
/* Don't evict BOs outside of the requested placement range */
- if (place->fpfn >= (node->start + num_pages) ||
- (place->lpfn && place->lpfn <= node->start))
+ if ((place->fpfn << PAGE_SHIFT) >= (node->start + size) ||
+ (place->lpfn && (place->lpfn << PAGE_SHIFT) <= node->start))
return false;
return true;
@@ -135,10 +135,9 @@ static bool ttm_range_man_compatible(struct
ttm_resource_manager *man,
size_t size)
{
struct drm_mm_node *node = _ttm_range_mgr_node(res)->mm_nodes[0];
- u32 num_pages = PFN_UP(size);
if (node->start < place->fpfn ||
- (place->lpfn && (node->start + num_pages) > place->lpfn))
+ (place->lpfn && (node->start + size) > place->lpfn << PAGE_SHIFT))
return false;
return true;
--
2.32.0
[PATCH 4/6] drm/ttm: Change the parameters of ttm_range_man_init() from pages to bytes
Change the parameters of ttm_range_man_init_nocheck()
size from page size to byte size.
Cleanup the PAGE_SHIFT operation on the depended caller functions.
Signed-off-by: Somalapuram Amaranath
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++--
drivers/gpu/drm/drm_gem_vram_helper.c | 2 +-
drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++--
drivers/gpu/drm/ttm/ttm_range_manager.c | 8
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
include/drm/ttm/ttm_range_manager.h | 6 +++---
6 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 6b270d4662a3..f0dabdfd3780 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -75,10 +75,10 @@ static void amdgpu_ttm_backend_unbind(struct ttm_device
*bdev,
static int amdgpu_ttm_init_on_chip(struct amdgpu_device *adev,
unsigned int type,
- uint64_t size_in_page)
+ uint64_t size)
{
return ttm_range_man_init(>mman.bdev, type,
- false, size_in_page);
+ false, size << PAGE_SHIFT);
}
/**
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index e7be562790de..db1915414e4a 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -999,7 +999,7 @@ static int drm_vram_mm_init(struct drm_vram_mm *vmm, struct
drm_device *dev,
return ret;
ret = ttm_range_man_init(>bdev, TTM_PL_VRAM,
-false, vram_size >> PAGE_SHIFT);
+false, vram_size);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 777d38b211d2..aa8785b6b1e8 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -70,13 +70,13 @@ struct radeon_device *radeon_get_rdev(struct ttm_device
*bdev)
static int radeon_ttm_init_vram(struct radeon_device *rdev)
{
return ttm_range_man_init(>mman.bdev, TTM_PL_VRAM,
- false, rdev->mc.real_vram_size >> PAGE_SHIFT);
+ false, rdev->mc.real_vram_size);
}
static int radeon_ttm_init_gtt(struct radeon_device *rdev)
{
return ttm_range_man_init(>mman.bdev, TTM_PL_TT,
- true, rdev->mc.gtt_size >> PAGE_SHIFT);
+ true, rdev->mc.gtt_size);
}
static void radeon_evict_flags(struct ttm_buffer_object *bo,
diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c
b/drivers/gpu/drm/ttm/ttm_range_manager.c
index ae11d07eb63a..62fddcc59f02 100644
--- a/drivers/gpu/drm/ttm/ttm_range_manager.c
+++ b/drivers/gpu/drm/ttm/ttm_range_manager.c
@@ -169,7 +169,7 @@ static const struct ttm_resource_manager_func
ttm_range_manager_func = {
* @bdev: ttm device
* @type: memory manager type
* @use_tt: if the memory manager uses tt
- * @p_size: size of area to be managed in pages.
+ * @size: size of area to be managed in bytes.
*
* The range manager is installed for this device in the type slot.
*
@@ -177,7 +177,7 @@ static const struct ttm_resource_manager_func
ttm_range_manager_func = {
*/
int ttm_range_man_init_nocheck(struct ttm_device *bdev,
unsigned type, bool use_tt,
- unsigned long p_size)
+ u64 size)
{
struct ttm_resource_manager *man;
struct ttm_range_manager *rman;
@@ -191,9 +191,9 @@ int ttm_range_man_init_nocheck(struct ttm_device *bdev,
man->func = _range_manager_func;
- ttm_resource_manager_init(man, bdev, p_size);
+ ttm_resource_manager_init(man, bdev, size);
- drm_mm_init(>mm, 0, p_size);
+ drm_mm_init(>mm, 0, size);
spin_lock_init(>lock);
ttm_set_driver_manager(bdev, type, >manager);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 9ad28346aff7..4926e7c73e75 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -700,7 +700,7 @@ static int vmw_vram_manager_init(struct vmw_private
*dev_priv)
{
int ret;
ret = ttm_range_man_init(_priv->bdev, TTM_PL_VRAM, false,
-dev_priv->vram_size >> PAGE_SHIFT);
+dev_priv->vram_size);
ttm_resource_manager_set_used(ttm_manager_type(_priv->bdev,
TTM_PL_VRAM), false);
return ret;
}
diff --git a/include/drm/ttm/ttm_range_manager.h
b/include/drm/ttm/ttm_range_manager.h
index 7963b957e9ef..05bffded1b53 100644
--- a/include/drm/ttm/ttm_range_manager.h
+++ b/include/drm/ttm/ttm_range_manager.h
@@ -36,15 +36,15 @@ to_ttm_range_mgr_node(struct ttm_resource *res)
int
[PATCH 3/6] drm/ttm: Change the meaning of resource->start from pfn to bytes
Change resource->start from pfn to bytes to
allow allocating objects smaller than a page.
Change all DRM drivers using ttm_resource start and size pfn to bytes.
Change amdgpu_res_first() cur->start, cur->size from pfn to bytes.
Replacing ttm_resource resource->start field with cursor.start.
Change amdgpu_gtt_mgr_new() allocation from pfn to bytes.
Signed-off-by: Somalapuram Amaranath
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 13 +++--
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 +++-
drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h | 8
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 10 +++---
.../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 6 +-
drivers/gpu/drm/drm_gem_vram_helper.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_bo.c| 13 ++---
drivers/gpu/drm/nouveau/nouveau_bo0039.c| 4 ++--
drivers/gpu/drm/nouveau/nouveau_mem.c | 10 +-
drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 +-
drivers/gpu/drm/nouveau/nv17_fence.c| 2 +-
drivers/gpu/drm/nouveau/nv50_fence.c| 2 +-
drivers/gpu/drm/qxl/qxl_drv.h | 2 +-
drivers/gpu/drm/qxl/qxl_object.c| 2 +-
drivers/gpu/drm/qxl/qxl_ttm.c | 5 ++---
drivers/gpu/drm/radeon/radeon_object.c | 6 +++---
drivers/gpu/drm/radeon/radeon_object.h | 2 +-
drivers/gpu/drm/radeon/radeon_ttm.c | 13 ++---
drivers/gpu/drm/radeon/radeon_vm.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_cmd.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 3 +--
23 files changed, 63 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 44367f03316f..a1fbfc5984d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -116,7 +116,6 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager
*man,
struct ttm_resource **res)
{
struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
- uint32_t num_pages = PFN_UP(tbo->base.size);
struct ttm_range_mgr_node *node;
int r;
@@ -134,8 +133,10 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager
*man,
if (place->lpfn) {
spin_lock(>lock);
r = drm_mm_insert_node_in_range(>mm, >mm_nodes[0],
- num_pages, tbo->page_alignment,
- 0, place->fpfn, place->lpfn,
+ tbo->base.size,
+ tbo->page_alignment <<
PAGE_SHIFT, 0,
+ place->fpfn << PAGE_SHIFT,
+ place->lpfn << PAGE_SHIFT,
DRM_MM_INSERT_BEST);
spin_unlock(>lock);
if (unlikely(r))
@@ -144,7 +145,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager
*man,
node->base.start = node->mm_nodes[0].start;
} else {
node->mm_nodes[0].start = 0;
- node->mm_nodes[0].size = PFN_UP(node->base.size);
+ node->mm_nodes[0].size = node->base.size;
node->base.start = AMDGPU_BO_INVALID_OFFSET;
}
@@ -285,8 +286,8 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev,
uint64_t gtt_size)
ttm_resource_manager_init(man, >mman.bdev, gtt_size);
- start = AMDGPU_GTT_MAX_TRANSFER_SIZE * AMDGPU_GTT_NUM_TRANSFER_WINDOWS;
- size = (adev->gmc.gart_size >> PAGE_SHIFT) - start;
+ start = (AMDGPU_GTT_MAX_TRANSFER_SIZE *
AMDGPU_GTT_NUM_TRANSFER_WINDOWS) << PAGE_SHIFT;
+ size = adev->gmc.gart_size - start;
drm_mm_init(>mm, start, size);
spin_lock_init(>lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index d835ee2131d2..f5d5eee09cea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1488,9 +1488,11 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo)
{
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+ struct amdgpu_res_cursor cursor;
uint64_t offset;
- offset = (bo->tbo.resource->start << PAGE_SHIFT) +
+ amdgpu_res_first(bo->tbo.resource, 0, bo->tbo.resource->size, );
+ offset = cursor.start +
amdgpu_ttm_domain_start(adev, bo->tbo.resource->mem_type);
return amdgpu_gmc_sign_extend(offset);
[PATCH 2/6] drm/amdgpu: Remove TTM resource->start visible VRAM condition
Use amdgpu_bo_in_cpu_visible_vram() instead.
Signed-off-by: Somalapuram Amaranath
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 981010de0a28..d835ee2131d2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -600,7 +600,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (!amdgpu_gmc_vram_full_visible(>gmc) &&
bo->tbo.resource->mem_type == TTM_PL_VRAM &&
- bo->tbo.resource->start < adev->gmc.visible_vram_size >> PAGE_SHIFT)
+ amdgpu_bo_in_cpu_visible_vram(bo))
amdgpu_cs_report_moved_bytes(adev, ctx.bytes_moved,
ctx.bytes_moved);
else
@@ -1346,7 +1346,6 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct
ttm_buffer_object *bo)
struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
struct ttm_operation_ctx ctx = { false, false };
struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
- unsigned long offset;
int r;
/* Remember that this BO was accessed by the CPU */
@@ -1355,8 +1354,7 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct
ttm_buffer_object *bo)
if (bo->resource->mem_type != TTM_PL_VRAM)
return 0;
- offset = bo->resource->start << PAGE_SHIFT;
- if ((offset + bo->base.size) <= adev->gmc.visible_vram_size)
+ if (amdgpu_bo_in_cpu_visible_vram(abo))
return 0;
/* Can't move a pinned BO to visible VRAM */
@@ -1378,10 +1376,9 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct
ttm_buffer_object *bo)
else if (unlikely(r))
return VM_FAULT_SIGBUS;
- offset = bo->resource->start << PAGE_SHIFT;
/* this should never happen */
if (bo->resource->mem_type == TTM_PL_VRAM &&
- (offset + bo->base.size) > adev->gmc.visible_vram_size)
+ amdgpu_bo_in_cpu_visible_vram(abo))
return VM_FAULT_SIGBUS;
ttm_bo_move_to_lru_tail_unlocked(bo);
--
2.32.0
[PATCH 1/6] drm/gem: Remove BUG_ON in drm_gem_private_object_init
ttm_resource can allocate size in bytes to support less than page size.
Signed-off-by: Somalapuram Amaranath
---
drivers/gpu/drm/drm_gem.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 59a0bb5ebd85..ee8b5c2b6c60 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -152,8 +152,6 @@ EXPORT_SYMBOL(drm_gem_object_init);
void drm_gem_private_object_init(struct drm_device *dev,
struct drm_gem_object *obj, size_t size)
{
- BUG_ON((size & (PAGE_SIZE - 1)) != 0);
-
obj->dev = dev;
obj->filp = NULL;
--
2.32.0
Re: [PATCH 2/3] drm/connector: Add enum documentation to drm_colorspace
On Fri, 3 Feb 2023 02:07:43 +
Joshua Ashton wrote:
> To match the other enums, and add more information about these values.
>
> Signed-off-by: Joshua Ashton
>
> Cc: Pekka Paalanen
> Cc: Sebastian Wick
> Cc: vitaly.pros...@amd.com
> Cc: Uma Shankar
> Cc: Ville Syrjälä
> Cc: Joshua Ashton
> Cc: dri-de...@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> ---
> include/drm/drm_connector.h | 41 +++--
> 1 file changed, 39 insertions(+), 2 deletions(-)
Hi Joshua,
sorry for pushing you into a rabbit hole a bit. :-)
>
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index edef65388c29..eb4cc9076e16 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -363,13 +363,50 @@ enum drm_privacy_screen_status {
> PRIVACY_SCREEN_ENABLED_LOCKED,
> };
>
> -/*
> - * This is a consolidated colorimetry list supported by HDMI and
> +/**
> + * enum drm_colorspace - color space
Documenting this enum is really nice. What would be even better if
there was similar documentation in the UAPI doc of "Colorspace" under
https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#standard-connector-properties
listing the strings that userspace must use/expect and what they refer
to.
> + *
> + * This enum is a consolidated colorimetry list supported by HDMI and
> * DP protocol standard. The respective connectors will register
> * a property with the subset of this list (supported by that
> * respective protocol). Userspace will set the colorspace through
> * a colorspace property which will be created and exposed to
Could this refer to "Colorspace" property explicitly instead of some
unmentioned property?
> * userspace.
> + *
> + * @DRM_MODE_COLORIMETRY_DEFAULT:
> + * sRGB (IEC 61966-2-1) or
> + * ITU-R BT.601 colorimetry format
Is this what the "driver will set the colorspace" comment actually
means? If so, I think the comment "driver will set the colorspace"
could be better or replaced with "not from any standard" or "undefined".
sRGB and BT.601 have different primaries. There are actually two
different cases of BT.601 primaries: the 525 line and 625 line. How
does that work? Are the drivers really choosing anything, or will they
just send "undefined" to the sink, and then the sink does whatever it
does?
Or is this *only* about the RGB-to-YCbCr conversion matrix and not
about colorimetry at all?
If it's only about the conversion matrix (MatrixCoefficients in CICP
(H.273) terms), then which ones of the below also define only the
MatrixCoefficients but no colorimetry?
> + * @DRM_MODE_COLORIMETRY_SMPTE_170M_YCC:
> + * SMPTE ST 170M colorimetry format
> + * @DRM_MODE_COLORIMETRY_BT709_YCC:
> + * ITU-R BT.709 colorimetry format
> + * @DRM_MODE_COLORIMETRY_XVYCC_601:
> + * xvYCC601 colorimetry format
> + * @DRM_MODE_COLORIMETRY_XVYCC_709:
> + * xvYCC709 colorimetry format
> + * @DRM_MODE_COLORIMETRY_SYCC_601:
> + * sYCC601 colorimetry format
> + * @DRM_MODE_COLORIMETRY_OPYCC_601:
> + * opYCC601 colorimetry format
> + * @DRM_MODE_COLORIMETRY_OPRGB:
> + * opRGB colorimetry format
> + * @DRM_MODE_COLORIMETRY_BT2020_CYCC:
> + * ITU-R BT.2020 Y'c C'bc C'rc (linear) colorimetry format
Is this one known as the constant luminance variant which requires
KMS/driver/hardware knowing also the transfer characteristic function?
Is there perhaps an assumed TF here, since there is no KMS property to
set a TF? Oh, maybe all of these imply the respective TF from the spec?
I suspect the "linear" should read as "constant luminance".
> + * @DRM_MODE_COLORIMETRY_BT2020_RGB:
> + * ITU-R BT.2020 R' G' B' colorimetry format
> + * @DRM_MODE_COLORIMETRY_BT2020_YCC:
> + * ITU-R BT.2020 Y' C'b C'r colorimetry format
...compared to this one known as the non-constant luminance variant,
i.e. "the simple RGB-to-YCbCr conversion"?
> + * @DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65:
> + * DCI-P3 (SMPTE RP 431-2) colorimetry format
> + * @DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER:
> + * DCI-P3 (SMPTE RP 431-2) colorimetry format
These two can't both be the same, right? That is, the description is
missing something.
> + * @DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED:
> + * RGB wide gamut fixed point colorimetry format
Is this one scRGB too?
> + * @DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT:
> + * RGB wide gamut floating point
> + * (scRGB (IEC 61966-2-2)) colorimetry format
> + * @DRM_MODE_COLORIMETRY_BT601_YCC:
> + * ITU-R BT.609 colorimetry format
Typo: BT.609
Which one of the two BT.601?
> */
> enum drm_colorspace {
> /* For Default case, driver will set the colorspace */
Thanks,
pq
pgpX86rVBMgvD.pgp
Description: OpenPGP digital signature
Re: [PATCH 1/3] drm/connector: Convert DRM_MODE_COLORIMETRY to enum
On Fri, 3 Feb 2023 02:07:42 +
Joshua Ashton wrote:
> From: Harry Wentland
>
> This allows us to use strongly typed arguments.
>
> Signed-off-by: Harry Wentland
> Reviewed-by: Simon Ser
>
> Cc: Pekka Paalanen
> Cc: Sebastian Wick
> Cc: vitaly.pros...@amd.com
> Cc: Uma Shankar
> Cc: Ville Syrjälä
> Cc: Joshua Ashton
> Cc: dri-de...@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> ---
> include/drm/display/drm_dp.h | 2 +-
> include/drm/drm_connector.h | 48 ++--
> 2 files changed, 25 insertions(+), 25 deletions(-)
>
Hi,
the code changes I can actually see here look good, but the test bot
found something else to fix. I feel the disappearance of
DRM_MODE_COLORIMETRY_NO_DATA could use an explanation in the commit
message.
I can only guess that NO_DATA comes from HDMI or DP spec or some such
to indicate undefined or something. However, the API here repurposes
that code point for "driver picks whatever".
I suppose it's kernel style to not write out the enum values when the C
standard rules produce the right values, but personally I think that is
hard to review and prone to accidental breakage if someone goes to add
a new value in the middle. Assuming these values are supposed to match
with a spec. I have no idea if they are.
Thanks,
pq
> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
> index ed10e6b6f99d..28899a03245c 100644
> --- a/include/drm/display/drm_dp.h
> +++ b/include/drm/display/drm_dp.h
> @@ -1623,7 +1623,7 @@ enum dp_pixelformat {
> *
> * This enum is used to indicate DP VSC SDP Colorimetry formats.
> * It is based on DP 1.4 spec [Table 2-117: VSC SDP Payload for DB16 through
> - * DB18] and a name of enum member follows DRM_MODE_COLORIMETRY definition.
> + * DB18] and a name of enum member follows drm_colorimetry definition.
> *
> * @DP_COLORIMETRY_DEFAULT: sRGB (IEC 61966-2-1) or
> * ITU-R BT.601 colorimetry format
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 4d830fc55a3d..edef65388c29 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -371,29 +371,29 @@ enum drm_privacy_screen_status {
> * a colorspace property which will be created and exposed to
> * userspace.
> */
> -
> -/* For Default case, driver will set the colorspace */
> -#define DRM_MODE_COLORIMETRY_DEFAULT 0
> -/* CEA 861 Normal Colorimetry options */
> -#define DRM_MODE_COLORIMETRY_NO_DATA 0
> -#define DRM_MODE_COLORIMETRY_SMPTE_170M_YCC 1
> -#define DRM_MODE_COLORIMETRY_BT709_YCC 2
> -/* CEA 861 Extended Colorimetry Options */
> -#define DRM_MODE_COLORIMETRY_XVYCC_601 3
> -#define DRM_MODE_COLORIMETRY_XVYCC_709 4
> -#define DRM_MODE_COLORIMETRY_SYCC_6015
> -#define DRM_MODE_COLORIMETRY_OPYCC_601 6
> -#define DRM_MODE_COLORIMETRY_OPRGB 7
> -#define DRM_MODE_COLORIMETRY_BT2020_CYCC 8
> -#define DRM_MODE_COLORIMETRY_BT2020_RGB 9
> -#define DRM_MODE_COLORIMETRY_BT2020_YCC 10
> -/* Additional Colorimetry extension added as part of CTA 861.G */
> -#define DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65 11
> -#define DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER 12
> -/* Additional Colorimetry Options added for DP 1.4a VSC Colorimetry Format */
> -#define DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED 13
> -#define DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT 14
> -#define DRM_MODE_COLORIMETRY_BT601_YCC 15
> +enum drm_colorspace {
> + /* For Default case, driver will set the colorspace */
> + DRM_MODE_COLORIMETRY_DEFAULT,
> + /* CEA 861 Normal Colorimetry options */
> + DRM_MODE_COLORIMETRY_SMPTE_170M_YCC,
> + DRM_MODE_COLORIMETRY_BT709_YCC,
> + /* CEA 861 Extended Colorimetry Options */
> + DRM_MODE_COLORIMETRY_XVYCC_601,
> + DRM_MODE_COLORIMETRY_XVYCC_709,
> + DRM_MODE_COLORIMETRY_SYCC_601,
> + DRM_MODE_COLORIMETRY_OPYCC_601,
> + DRM_MODE_COLORIMETRY_OPRGB,
> + DRM_MODE_COLORIMETRY_BT2020_CYCC,
> + DRM_MODE_COLORIMETRY_BT2020_RGB,
> + DRM_MODE_COLORIMETRY_BT2020_YCC,
> + /* Additional Colorimetry extension added as part of CTA 861.G */
> + DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65,
> + DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER,
> + /* Additional Colorimetry Options added for DP 1.4a VSC Colorimetry
> Format */
> + DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED,
> + DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT,
> + DRM_MODE_COLORIMETRY_BT601_YCC,
> +};
>
> /**
> * enum drm_bus_flags - bus_flags info for _display_info
> @@ -826,7 +826,7 @@ struct drm_connector_state {
>* colorspace change on Sink. This is most commonly used to switch
>* to wider color gamuts like BT2020.
>*/
> - u32 colorspace;
> +
