Re: 6.5.5: UBSAN: radeon_atombios.c: index 1 is out of range for type 'UCHAR [1]'
On Mon, Apr 8, 2024 at 9:45 PM Kees Cook wrote: > > > > On April 8, 2024 5:45:29 PM PDT, Jeff Johnson > wrote: > >On 10/1/23 17:12, Justin Piszcz wrote: > > [Sun Oct 1 15:59:04 2023] UBSAN: array-index-out-of-bounds in > drivers/gpu/drm/radeon/radeon_atombios.c:2620:43 > [Sun Oct 1 15:59:04 2023] index 1 is out of range for type 'UCHAR [1]' > [Sun Oct 1 15:59:04 2023] CPU: 5 PID: 1 Comm: swapper/0 Tainted: G > T 6.5.5 #13 55df8de52754ef95effc50a55e9206abdea304ac > [Sun Oct 1 15:59:04 2023] Hardware name: Supermicro X9SRL-F/X9SRL-F, > BIOS 3.3 11/13/2018 > [Sun Oct 1 15:59:04 2023] Call Trace: > [Sun Oct 1 15:59:04 2023] > [Sun Oct 1 15:59:04 2023] dump_stack_lvl+0x36/0x50 > [Sun Oct 1 15:59:04 2023] __ubsan_handle_out_of_bounds+0xc7/0x110 > [Sun Oct 1 15:59:04 2023] radeon_atombios_get_power_modes+0x87a/0x8f0 > [Sun Oct 1 15:59:04 2023] radeon_pm_init+0x13a/0x7e0 > [Sun Oct 1 15:59:04 2023] evergreen_init+0x13d/0x3d0 > [Sun Oct 1 15:59:04 2023] radeon_device_init+0x60a/0xbf0 > [Sun Oct 1 15:59:04 2023] radeon_driver_load_kms+0xb1/0x250 > [Sun Oct 1 15:59:04 2023] drm_dev_register+0xfc/0x250 > [Sun Oct 1 15:59:04 2023] radeon_pci_probe+0xd0/0x150 > [Sun Oct 1 15:59:04 2023] pci_device_probe+0x97/0x130 > [Sun Oct 1 15:59:04 2023] really_probe+0xbe/0x2f0 > [Sun Oct 1 15:59:04 2023] ? __pfx___driver_attach+0x10/0x10 > [Sun Oct 1 15:59:04 2023] __driver_probe_device+0x6e/0x120 > [Sun Oct 1 15:59:04 2023] driver_probe_device+0x1a/0x90 > [Sun Oct 1 15:59:04 2023] __driver_attach+0xd4/0x170 > [Sun Oct 1 15:59:04 2023] bus_for_each_dev+0x87/0xe0 > [Sun Oct 1 15:59:04 2023] bus_add_driver+0xf3/0x1f0 > [Sun Oct 1 15:59:04 2023] driver_register+0x58/0x120 > [Sun Oct 1 15:59:04 2023] ? __pfx_radeon_module_init+0x10/0x10 > [Sun Oct 1 15:59:04 2023] do_one_initcall+0x93/0x4a0 > [Sun Oct 1 15:59:04 2023] kernel_init_freeable+0x301/0x580 > [Sun Oct 1 15:59:04 2023] ? __pfx_kernel_init+0x10/0x10 > [Sun Oct 1 15:59:04 2023] kernel_init+0x15/0x1b0 > [Sun Oct 1 15:59:04 2023] ret_from_fork+0x2f/0x50 > [Sun Oct 1 15:59:04 2023] ? __pfx_kernel_init+0x10/0x10 > [Sun Oct 1 15:59:04 2023] ret_from_fork_asm+0x1b/0x30 > [Sun Oct 1 15:59:04 2023] > [Sun Oct 1 15:59:04 2023] > > [Sun Oct 1 15:59:04 2023] [drm] radeon: dpm initialized > [Sun Oct 1 15:59:04 2023] [drm] GART: num cpu pages 262144, num gpu > pages 262144 > [Sun Oct 1 15:59:04 2023] [drm] enabling PCIE gen 2 link speeds, > disable with radeon.pcie_gen2=0 > [Sun Oct 1 15:59:04 2023] [drm] PCIE GART of 1024M enabled (table at > 0x0014C000). > [Sun Oct 1 15:59:04 2023] radeon :03:00.0: WB enabled > [Sun Oct 1 15:59:04 2023] radeon :03:00.0: fence driver on ring 0 > use gpu addr 0x4c00 > [Sun Oct 1 15:59:04 2023] radeon :03:00.0: fence driver on ring 3 > use gpu addr 0x4c0c > [Sun Oct 1 15:59:04 2023] radeon :03:00.0: fence driver on ring 5 > use gpu addr 0x0005c418 > [Sun Oct 1 15:59:04 2023] radeon :03:00.0: radeon: MSI limited to > 32-bit > [Sun Oct 1 15:59:04 2023] radeon :03:00.0: radeon: using MSI. > [Sun Oct 1 15:59:04 2023] [drm] radeon: irq initialized. > > >>> > >>> Please also open an issue on freedesktop tracker [1]. > >>> > >>> Thanks. > >>> > >>> [1]: https://gitlab.freedesktop.org/drm/amd/-/issues > >> > >> Issue opened: https://gitlab.freedesktop.org/drm/amd/-/issues/2894 > >> > >> Regards, > >> Justin > > > >+Kees since I've worked with him on several of these flexible array issues. > > > >I just happened to look at kernel logs today for my ath1*k driver > >maintenance and see the subject issue is present on my device, running > >6.9.0-rc1. The freedesktop issue tracker says the issue is closed, but any > >fix has not landed in the upstream kernel. Is there a -next patch somewhere? > > > >[ 12.105270] UBSAN: array-index-out-of-bounds in > >drivers/gpu/drm/radeon/radeon_atombios.c:2718:34 > >[ 12.105272] index 48 is out of range for type 'UCHAR [1]' > >[ > > > >If there isn't really an upstream fix, I can probably supply one. > > I would expect this to have fixed it: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/radeon/pptable.h?id=c63079c61177ba1b17fa05c6875699a36924fe39 > > If not, there must be something else happening? This patch should silence it I think: https://patchwork.freedesktop.org/patch/588305/ Alex > > -Kees > > -- > Kees Cook
RE: [PATCH] drm/amdgpu: Fix incorrect return value
[AMD Official Use Only - General]
> -Original Message-
> From: Chai, Thomas
> Sent: Wednesday, April 3, 2024 3:07 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Chai, Thomas ; Zhang, Hawking
> ; Zhou1, Tao ; Li, Candice
> ; Wang, Yang(Kevin) ; Yang,
> Stanley ; Chai, Thomas
> Subject: [PATCH] drm/amdgpu: Fix incorrect return value
>
> [Why]
> After calling amdgpu_vram_mgr_reserve_range multiple times with the same
> address, calling amdgpu_vram_mgr_query_page_status will always return -
> EBUSY.
> From the second call to amdgpu_vram_mgr_reserve_range, the same address
> will be added to the reservations_pending list again and is never moved to the
> reserved_pages list because the address had been reserved.
>
> [How]
> First add the address status check before calling
> amdgpu_vram_mgr_do_reserve, if the address is already reserved, do nothing; If
> the address is already in the reservations_pending list, directly reserve
> memory;
> only add new nodes for the addresses that are not in the reserved_pages list
> and
> reservations_pending list.
>
> Signed-off-by: YiPeng Chai
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 28 +---
> 1 file changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index 1e36c428d254..0bf3f4092900 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -317,7 +317,6 @@ static void amdgpu_vram_mgr_do_reserve(struct
> ttm_resource_manager *man)
>
> dev_dbg(adev->dev, "Reservation 0x%llx - %lld, Succeeded\n",
> rsv->start, rsv->size);
> -
> vis_usage = amdgpu_vram_mgr_vis_size(adev, block);
> atomic64_add(vis_usage, >vis_usage);
> spin_lock(>bdev->lru_lock);
> @@ -340,19 +339,30 @@ int amdgpu_vram_mgr_reserve_range(struct
> amdgpu_vram_mgr *mgr,
> uint64_t start, uint64_t size)
> {
> struct amdgpu_vram_reservation *rsv;
> + int ret = 0;
>
> - rsv = kzalloc(sizeof(*rsv), GFP_KERNEL);
> - if (!rsv)
> - return -ENOMEM;
> + ret = amdgpu_vram_mgr_query_page_status(mgr, start);
> + if (!ret)
> + return 0;
> +
> + if (ret == -ENOENT) {
> + rsv = kzalloc(sizeof(*rsv), GFP_KERNEL);
> + if (!rsv)
> + return -ENOMEM;
>
> - INIT_LIST_HEAD(>allocated);
> - INIT_LIST_HEAD(>blocks);
> + INIT_LIST_HEAD(>allocated);
> + INIT_LIST_HEAD(>blocks);
>
> - rsv->start = start;
> - rsv->size = size;
> + rsv->start = start;
> + rsv->size = size;
> +
> + mutex_lock(>lock);
> + list_add_tail(>blocks, >reservations_pending);
> + mutex_unlock(>lock);
[Tao] we can drop the mutex_unlock and add if (ret != -ENOENT) for the second
mutex_lock to avoid unlocking/locking repeatedly.
> +
> + }
>
> mutex_lock(>lock);
> - list_add_tail(>blocks, >reservations_pending);
> amdgpu_vram_mgr_do_reserve(>manager);
> mutex_unlock(>lock);
>
> --
> 2.34.1
Re: 6.5.5: UBSAN: radeon_atombios.c: index 1 is out of range for type 'UCHAR [1]'
On April 8, 2024 5:45:29 PM PDT, Jeff Johnson wrote: >On 10/1/23 17:12, Justin Piszcz wrote: [Sun Oct 1 15:59:04 2023] UBSAN: array-index-out-of-bounds in drivers/gpu/drm/radeon/radeon_atombios.c:2620:43 [Sun Oct 1 15:59:04 2023] index 1 is out of range for type 'UCHAR [1]' [Sun Oct 1 15:59:04 2023] CPU: 5 PID: 1 Comm: swapper/0 Tainted: G T 6.5.5 #13 55df8de52754ef95effc50a55e9206abdea304ac [Sun Oct 1 15:59:04 2023] Hardware name: Supermicro X9SRL-F/X9SRL-F, BIOS 3.3 11/13/2018 [Sun Oct 1 15:59:04 2023] Call Trace: [Sun Oct 1 15:59:04 2023] [Sun Oct 1 15:59:04 2023] dump_stack_lvl+0x36/0x50 [Sun Oct 1 15:59:04 2023] __ubsan_handle_out_of_bounds+0xc7/0x110 [Sun Oct 1 15:59:04 2023] radeon_atombios_get_power_modes+0x87a/0x8f0 [Sun Oct 1 15:59:04 2023] radeon_pm_init+0x13a/0x7e0 [Sun Oct 1 15:59:04 2023] evergreen_init+0x13d/0x3d0 [Sun Oct 1 15:59:04 2023] radeon_device_init+0x60a/0xbf0 [Sun Oct 1 15:59:04 2023] radeon_driver_load_kms+0xb1/0x250 [Sun Oct 1 15:59:04 2023] drm_dev_register+0xfc/0x250 [Sun Oct 1 15:59:04 2023] radeon_pci_probe+0xd0/0x150 [Sun Oct 1 15:59:04 2023] pci_device_probe+0x97/0x130 [Sun Oct 1 15:59:04 2023] really_probe+0xbe/0x2f0 [Sun Oct 1 15:59:04 2023] ? __pfx___driver_attach+0x10/0x10 [Sun Oct 1 15:59:04 2023] __driver_probe_device+0x6e/0x120 [Sun Oct 1 15:59:04 2023] driver_probe_device+0x1a/0x90 [Sun Oct 1 15:59:04 2023] __driver_attach+0xd4/0x170 [Sun Oct 1 15:59:04 2023] bus_for_each_dev+0x87/0xe0 [Sun Oct 1 15:59:04 2023] bus_add_driver+0xf3/0x1f0 [Sun Oct 1 15:59:04 2023] driver_register+0x58/0x120 [Sun Oct 1 15:59:04 2023] ? __pfx_radeon_module_init+0x10/0x10 [Sun Oct 1 15:59:04 2023] do_one_initcall+0x93/0x4a0 [Sun Oct 1 15:59:04 2023] kernel_init_freeable+0x301/0x580 [Sun Oct 1 15:59:04 2023] ? __pfx_kernel_init+0x10/0x10 [Sun Oct 1 15:59:04 2023] kernel_init+0x15/0x1b0 [Sun Oct 1 15:59:04 2023] ret_from_fork+0x2f/0x50 [Sun Oct 1 15:59:04 2023] ? __pfx_kernel_init+0x10/0x10 [Sun Oct 1 15:59:04 2023] ret_from_fork_asm+0x1b/0x30 [Sun Oct 1 15:59:04 2023] [Sun Oct 1 15:59:04 2023] [Sun Oct 1 15:59:04 2023] [drm] radeon: dpm initialized [Sun Oct 1 15:59:04 2023] [drm] GART: num cpu pages 262144, num gpu pages 262144 [Sun Oct 1 15:59:04 2023] [drm] enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0 [Sun Oct 1 15:59:04 2023] [drm] PCIE GART of 1024M enabled (table at 0x0014C000). [Sun Oct 1 15:59:04 2023] radeon :03:00.0: WB enabled [Sun Oct 1 15:59:04 2023] radeon :03:00.0: fence driver on ring 0 use gpu addr 0x4c00 [Sun Oct 1 15:59:04 2023] radeon :03:00.0: fence driver on ring 3 use gpu addr 0x4c0c [Sun Oct 1 15:59:04 2023] radeon :03:00.0: fence driver on ring 5 use gpu addr 0x0005c418 [Sun Oct 1 15:59:04 2023] radeon :03:00.0: radeon: MSI limited to 32-bit [Sun Oct 1 15:59:04 2023] radeon :03:00.0: radeon: using MSI. [Sun Oct 1 15:59:04 2023] [drm] radeon: irq initialized. >>> >>> Please also open an issue on freedesktop tracker [1]. >>> >>> Thanks. >>> >>> [1]: https://gitlab.freedesktop.org/drm/amd/-/issues >> >> Issue opened: https://gitlab.freedesktop.org/drm/amd/-/issues/2894 >> >> Regards, >> Justin > >+Kees since I've worked with him on several of these flexible array issues. > >I just happened to look at kernel logs today for my ath1*k driver maintenance >and see the subject issue is present on my device, running 6.9.0-rc1. The >freedesktop issue tracker says the issue is closed, but any fix has not landed >in the upstream kernel. Is there a -next patch somewhere? > >[ 12.105270] UBSAN: array-index-out-of-bounds in >drivers/gpu/drm/radeon/radeon_atombios.c:2718:34 >[ 12.105272] index 48 is out of range for type 'UCHAR [1]' >[ > >If there isn't really an upstream fix, I can probably supply one. I would expect this to have fixed it: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/radeon/pptable.h?id=c63079c61177ba1b17fa05c6875699a36924fe39 If not, there must be something else happening? -Kees -- Kees Cook
Re: [PATCH] Documentation/gpu: correct path of reference
Applied. Thanks!
Alex
On Sat, Apr 6, 2024 at 11:52 AM Simon Horman wrote:
>
> The path to GPU documentation is Documentation/gpu
> rather than Documentation/GPU
>
> This appears to have been introduced by commit ba162ae749a5
> ("Documentation/gpu: Introduce a simple contribution list for display code")
>
> Flagged by make htmldocs.
>
> Signed-off-by: Simon Horman
> ---
> Documentation/gpu/amdgpu/display/display-contributing.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/gpu/amdgpu/display/display-contributing.rst
> b/Documentation/gpu/amdgpu/display/display-contributing.rst
> index fdb2bea01d53..36f3077eee00 100644
> --- a/Documentation/gpu/amdgpu/display/display-contributing.rst
> +++ b/Documentation/gpu/amdgpu/display/display-contributing.rst
> @@ -135,7 +135,7 @@ Enable underlay
> ---
>
> AMD display has this feature called underlay (which you can read more about
> at
> -'Documentation/GPU/amdgpu/display/mpo-overview.rst') which is intended to
> +'Documentation/gpu/amdgpu/display/mpo-overview.rst') which is intended to
> save power when playing a video. The basic idea is to put a video in the
> underlay plane at the bottom and the desktop in the plane above it with a
> hole
> in the video area. This feature is enabled in ChromeOS, and from our data
>
Re: [PATCH -next] drm/amd/display: delete the redundant initialization in dcn3_51_soc
Acked-by: Alex Deucher
Applied.
Thanks,
Alex
On Mon, Apr 8, 2024 at 3:45 AM Xiang Yang wrote:
>
> the dram_clock_change_latency_us in dcn3_51_soc is initialized twice, so
> delete one of them.
>
> Signed-off-by: Xiang Yang
> ---
> drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
> b/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
> index b3ffab77cf88..f1c0d5b77f1b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
> +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
> @@ -237,7 +237,6 @@ struct _vcs_dpi_soc_bounding_box_st dcn3_51_soc = {
> .urgent_latency_adjustment_fabric_clock_component_us = 0,
> .urgent_latency_adjustment_fabric_clock_reference_mhz = 0,
> .num_chans = 4,
> - .dram_clock_change_latency_us = 11.72,
> .dispclk_dppclk_vco_speed_mhz = 2400.0,
> };
>
> --
> 2.34.1
>
Re: [PATCH v2] drm/radeon/radeon_display: Decrease the size of allocated memory
On Mon, Apr 1, 2024 at 8:35 AM Christian König wrote: > > Am 30.03.24 um 17:34 schrieb Erick Archer: > > This is an effort to get rid of all multiplications from allocation > > functions in order to prevent integer overflows [1] [2]. > > > > In this case, the memory allocated to store RADEONFB_CONN_LIMIT pointers > > to "drm_connector" structures can be avoided. This is because this > > memory area is never accessed. > > > > Also, in the kzalloc function, it is preferred to use sizeof(*pointer) > > instead of sizeof(type) due to the type of the variable can change and > > one needs not change the former (unlike the latter). > > > > At the same time take advantage to remove the "#if 0" block, the code > > where the removed memory area was accessed, and the RADEONFB_CONN_LIMIT > > constant due to now is never used. > > > > Link: > > https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments > > [1] > > Link: https://github.com/KSPP/linux/issues/160 [2] > > Signed-off-by: Erick Archer > > Well in general we don't do any new feature development any more for the > radeon driver. > > But this cleanup looks so straight forward that the risk of breaking > something is probably very low. > > Acked-by from my side, but Alex should probably take a look as well. I can't remember why that was done that way. Maybe some leftover from the early KMS days before we finalized the fbdev interactions? Anyway, patch applied. Thanks. Alex > > Regards, > Christian. > > > --- > > Changes in v2: > > - Rebase against linux-next. > > > > Previous versions: > > v1 -> > > https://lore.kernel.org/linux-hardening/20240222180431.7451-1-erick.arc...@gmx.com/ > > > > Hi everyone, > > > > Any comments would be greatly appreciated. The first version was > > not commented. > > > > Thanks, > > Erick > > --- > > drivers/gpu/drm/radeon/radeon.h | 1 - > > drivers/gpu/drm/radeon/radeon_display.c | 8 +--- > > 2 files changed, 1 insertion(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/radeon/radeon.h > > b/drivers/gpu/drm/radeon/radeon.h > > index 3e5ff17e3caf..0999c8eaae94 100644 > > --- a/drivers/gpu/drm/radeon/radeon.h > > +++ b/drivers/gpu/drm/radeon/radeon.h > > @@ -132,7 +132,6 @@ extern int radeon_cik_support; > > /* RADEON_IB_POOL_SIZE must be a power of 2 */ > > #define RADEON_IB_POOL_SIZE 16 > > #define RADEON_DEBUGFS_MAX_COMPONENTS 32 > > -#define RADEONFB_CONN_LIMIT 4 > > #define RADEON_BIOS_NUM_SCRATCH 8 > > > > /* internal ring indices */ > > diff --git a/drivers/gpu/drm/radeon/radeon_display.c > > b/drivers/gpu/drm/radeon/radeon_display.c > > index efd18c8d84c8..5f1d24d3120c 100644 > > --- a/drivers/gpu/drm/radeon/radeon_display.c > > +++ b/drivers/gpu/drm/radeon/radeon_display.c > > @@ -683,7 +683,7 @@ static void radeon_crtc_init(struct drm_device *dev, > > int index) > > struct radeon_device *rdev = dev->dev_private; > > struct radeon_crtc *radeon_crtc; > > > > - radeon_crtc = kzalloc(sizeof(struct radeon_crtc) + > > (RADEONFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL); > > + radeon_crtc = kzalloc(sizeof(*radeon_crtc), GFP_KERNEL); > > if (radeon_crtc == NULL) > > return; > > > > @@ -709,12 +709,6 @@ static void radeon_crtc_init(struct drm_device *dev, > > int index) > > dev->mode_config.cursor_width = radeon_crtc->max_cursor_width; > > dev->mode_config.cursor_height = radeon_crtc->max_cursor_height; > > > > -#if 0 > > - radeon_crtc->mode_set.crtc = _crtc->base; > > - radeon_crtc->mode_set.connectors = (struct drm_connector > > **)(radeon_crtc + 1); > > - radeon_crtc->mode_set.num_connectors = 0; > > -#endif > > - > > if (rdev->is_atom_bios && (ASIC_IS_AVIVO(rdev) || radeon_r4xx_atom)) > > radeon_atombios_init_crtc(dev, radeon_crtc); > > else >
[PATCH 2/5] drm/amdgpu: Use drm_crtc_vblank_crtc()
From: Ville Syrjälä
Replace the open coded drm_crtc_vblank_crtc() with the real
thing.
Cc: Alex Deucher
Cc: "Christian König"
Cc: "Pan, Xinhui"
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 8 ++--
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
index 8baa2e0935cc..258703145161 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
@@ -65,9 +65,7 @@ static enum hrtimer_restart
amdgpu_vkms_vblank_simulate(struct hrtimer *timer)
static int amdgpu_vkms_enable_vblank(struct drm_crtc *crtc)
{
- struct drm_device *dev = crtc->dev;
- unsigned int pipe = drm_crtc_index(crtc);
- struct drm_vblank_crtc *vblank = >vblank[pipe];
+ struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
struct amdgpu_vkms_output *out = drm_crtc_to_amdgpu_vkms_output(crtc);
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
@@ -91,10 +89,8 @@ static bool amdgpu_vkms_get_vblank_timestamp(struct drm_crtc
*crtc,
ktime_t *vblank_time,
bool in_vblank_irq)
{
- struct drm_device *dev = crtc->dev;
- unsigned int pipe = crtc->index;
struct amdgpu_vkms_output *output =
drm_crtc_to_amdgpu_vkms_output(crtc);
- struct drm_vblank_crtc *vblank = >vblank[pipe];
+ struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
if (!READ_ONCE(vblank->enabled)) {
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 71d2d44681b2..662d2d83473b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -528,7 +528,7 @@ static void dm_vupdate_high_irq(void *interrupt_params)
if (acrtc) {
vrr_active = amdgpu_dm_crtc_vrr_active_irq(acrtc);
drm_dev = acrtc->base.dev;
- vblank = _dev->vblank[acrtc->base.index];
+ vblank = drm_crtc_vblank_crtc(>base);
previous_timestamp =
atomic64_read(_params->previous_timestamp);
frame_duration_ns = vblank->time - previous_timestamp;
--
2.43.2
Re: [PATCH v2] drm/amdgpu: Fix discovery initialization failure during pci rescan
On Tue, Apr 2, 2024 at 7:56 AM Christian König wrote:
>
> Am 02.04.24 um 12:05 schrieb Ma Jun:
> > Waiting for system ready to fix the discovery initialization
> > failure issue. This failure usually occurs when dGPU is removed
> > and then rescanned via command line.
> > It's caused by following two errors:
> > [1] vram size is 0
> > [2] wrong binary signature
> >
> > Signed-off-by: Ma Jun
>
> I'm not an expert for that stuff, but using dev_is_removable() indeed
> seems to be incorrect here.
>
> Feel free to add an Acked-by: Christian König
> , but I would rather wait for Alex to come
> back from vacation and take a look as well.
>
> Might be that I missed something why the dev_is_removable() check is
> mandatory or something like that.
I added it originally for USB4/thunderbolt connected devices (hence
the removable check) and didn't want to add the extra latency all the
time, but I hadn't considered the rescan case.
Patch is:
Reviewed-by: Alex Deucher
Alex
>
> Regards,
> Christian.
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 17 ++---
> > 1 file changed, 6 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > index 07c5fca06178..90735e966318 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > @@ -255,7 +255,6 @@ static int amdgpu_discovery_read_binary_from_mem(struct
> > amdgpu_device *adev,
> > uint64_t vram_size;
> > u32 msg;
> > int i, ret = 0;
> > - int ip_discovery_ver = 0;
> >
> > /* It can take up to a second for IFWI init to complete on some dGPUs,
> >* but generally it should be in the 60-100ms range. Normally this
> > starts
> > @@ -265,17 +264,13 @@ static int
> > amdgpu_discovery_read_binary_from_mem(struct amdgpu_device *adev,
> >* continue.
> >*/
> >
> > - ip_discovery_ver = RREG32(mmIP_DISCOVERY_VERSION);
> > - if ((dev_is_removable(>pdev->dev)) ||
> > - (ip_discovery_ver == IP_DISCOVERY_V2) ||
> > - (ip_discovery_ver == IP_DISCOVERY_V4)) {
> > - for (i = 0; i < 1000; i++) {
> > - msg = RREG32(mmMP0_SMN_C2PMSG_33);
> > - if (msg & 0x8000)
> > - break;
> > - msleep(1);
> > - }
> > + for (i = 0; i < 1000; i++) {
> > + msg = RREG32(mmMP0_SMN_C2PMSG_33);
> > + if (msg & 0x8000)
> > + break;
> > + usleep_range(1000, 1100);
> > }
> > +
> > vram_size = (uint64_t)RREG32(mmRCC_CONFIG_MEMSIZE) << 20;
> >
> > if (vram_size) {
>
[PATCH] drm/amdgpu/mes11: print MES opcodes rather than numbers
Makes it easier to review the logs when there are MES
errors.
v2: use dbg for emitted, add helpers for fetching strings
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 78 --
1 file changed, 74 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
index 072c478665ade..69d39ba726e12 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
@@ -100,18 +100,72 @@ static const struct amdgpu_ring_funcs
mes_v11_0_ring_funcs = {
.insert_nop = amdgpu_ring_insert_nop,
};
+static const char *mes_v11_0_opcodes[] = {
+ "MES_SCH_API_SET_HW_RSRC",
+ "MES_SCH_API_SET_SCHEDULING_CONFIG",
+ "MES_SCH_API_ADD_QUEUE"
+ "MES_SCH_API_REMOVE_QUEUE"
+ "MES_SCH_API_PERFORM_YIELD"
+ "MES_SCH_API_SET_GANG_PRIORITY_LEVEL"
+ "MES_SCH_API_SUSPEND"
+ "MES_SCH_API_RESUME"
+ "MES_SCH_API_RESET"
+ "MES_SCH_API_SET_LOG_BUFFER"
+ "MES_SCH_API_CHANGE_GANG_PRORITY"
+ "MES_SCH_API_QUERY_SCHEDULER_STATUS"
+ "MES_SCH_API_PROGRAM_GDS"
+ "MES_SCH_API_SET_DEBUG_VMID"
+ "MES_SCH_API_MISC"
+ "MES_SCH_API_UPDATE_ROOT_PAGE_TABLE"
+ "MES_SCH_API_AMD_LOG"
+};
+
+static const char *mes_v11_0_misc_opcodes[] = {
+ "MESAPI_MISC__WRITE_REG",
+ "MESAPI_MISC__INV_GART",
+ "MESAPI_MISC__QUERY_STATUS",
+ "MESAPI_MISC__READ_REG",
+ "MESAPI_MISC__WAIT_REG_MEM",
+ "MESAPI_MISC__SET_SHADER_DEBUGGER",
+};
+
+static const char *mes_v11_0_get_op_string(union MESAPI__MISC *x_pkt)
+{
+ const char *op_str = NULL;
+
+ if (x_pkt->header.opcode < ARRAY_SIZE(mes_v11_0_opcodes))
+ op_str = mes_v11_0_opcodes[x_pkt->header.opcode];
+
+ return op_str;
+}
+
+static const char *mes_v11_0_get_misc_op_string(union MESAPI__MISC *x_pkt)
+{
+ const char *op_str = NULL;
+
+ if ((x_pkt->header.opcode == MES_SCH_API_MISC) &&
+ (x_pkt->opcode <= ARRAY_SIZE(mes_v11_0_misc_opcodes)))
+ op_str = mes_v11_0_misc_opcodes[x_pkt->opcode];
+
+ return op_str;
+}
+
static int mes_v11_0_submit_pkt_and_poll_completion(struct amdgpu_mes *mes,
void *pkt, int size,
int api_status_off)
{
int ndw = size / 4;
signed long r;
- union MESAPI__ADD_QUEUE *x_pkt = pkt;
+ union MESAPI__MISC *x_pkt = pkt;
struct MES_API_STATUS *api_status;
struct amdgpu_device *adev = mes->adev;
struct amdgpu_ring *ring = >ring;
unsigned long flags;
signed long timeout = adev->usec_timeout;
+ const char *op_str, *misc_op_str;
+
+ if (x_pkt->header.opcode >= MES_SCH_API_MAX)
+ return -EINVAL;
if (amdgpu_emu_mode) {
timeout *= 100;
@@ -135,13 +189,29 @@ static int
mes_v11_0_submit_pkt_and_poll_completion(struct amdgpu_mes *mes,
amdgpu_ring_commit(ring);
spin_unlock_irqrestore(>ring_lock, flags);
- DRM_DEBUG("MES msg=%d was emitted\n", x_pkt->header.opcode);
+ op_str = mes_v11_0_get_op_string(x_pkt);
+ misc_op_str = mes_v11_0_get_misc_op_string(x_pkt);
+
+ if (misc_op_str)
+ dev_dbg(adev->dev, "MES msg=%s (%s) was emitted\n", op_str,
misc_op_str);
+ else if (op_str)
+ dev_dbg(adev->dev, "MES msg=%s was emitted\n", op_str);
+ else
+ dev_dbg(adev->dev, "MES msg=%d was emitted\n",
x_pkt->header.opcode);
r = amdgpu_fence_wait_polling(ring, ring->fence_drv.sync_seq,
timeout);
if (r < 1) {
- DRM_ERROR("MES failed to response msg=%d\n",
- x_pkt->header.opcode);
+
+ if (misc_op_str)
+ dev_err(adev->dev, "MES failed to respond to msg=%s
(%s)\n",
+ op_str, misc_op_str);
+ else if (op_str)
+ dev_err(adev->dev, "MES failed to respond to msg=%s\n",
+ op_str);
+ else
+ dev_err(adev->dev, "MES failed to respond to msg=%d\n",
+ x_pkt->header.opcode);
while (halt_if_hws_hang)
schedule();
--
2.44.0
Re: [PATCH] drm/amdgpu: fix visible VRAM handling during faults
On Fri, Apr 5, 2024 at 3:57 AM Christian König
wrote:
>
> When we removed the hacky start code check we actually didn't took into
> account that *all* VRAM pages needs to be CPU accessible.
>
> Clean up the code and unify the handling into a single helper which
> checks if the whole resource is CPU accessible.
>
> The only place where a partial check would make sense is during
> eviction, but that is neglitible.
>
> Signed-off-by: Christian König
> Fixes: aed01a68047b ("drm/amdgpu: Remove TTM resource->start visible VRAM
> condition v2")
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 22
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 22
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 61 ++
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h| 3 ++
> 5 files changed, 53 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index bbbd8ad0171f..e9168677ef0a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -819,7 +819,7 @@ static int amdgpu_cs_bo_validate(void *param, struct
> amdgpu_bo *bo)
>
> p->bytes_moved += ctx.bytes_moved;
> if (!amdgpu_gmc_vram_full_visible(>gmc) &&
> - amdgpu_bo_in_cpu_visible_vram(bo))
> + amdgpu_res_cpu_visible(adev, bo->tbo.resource))
> p->bytes_moved_vis += ctx.bytes_moved;
>
> if (unlikely(r == -ENOMEM) && domain != bo->allowed_domains) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index eb7d824763b9..eff3f9fceada 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -620,8 +620,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
> return r;
>
> if (!amdgpu_gmc_vram_full_visible(>gmc) &&
> - bo->tbo.resource->mem_type == TTM_PL_VRAM &&
> - amdgpu_bo_in_cpu_visible_vram(bo))
> + amdgpu_res_cpu_visible(adev, bo->tbo.resource))
> amdgpu_cs_report_moved_bytes(adev, ctx.bytes_moved,
> ctx.bytes_moved);
> else
> @@ -1276,18 +1275,20 @@ void amdgpu_bo_move_notify(struct ttm_buffer_object
> *bo,
> void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
> struct amdgpu_mem_stats *stats)
> {
> + struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> + struct ttm_resource *res = bo->tbo.resource;
> uint64_t size = amdgpu_bo_size(bo);
> unsigned int domain;
>
> /* Abort if the BO doesn't currently have a backing store */
> - if (!bo->tbo.resource)
> + if (!res)
> return;
>
> - domain = amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
> + domain = amdgpu_mem_type_to_domain(res->mem_type);
> switch (domain) {
> case AMDGPU_GEM_DOMAIN_VRAM:
> stats->vram += size;
> - if (amdgpu_bo_in_cpu_visible_vram(bo))
> + if (amdgpu_res_cpu_visible(adev, bo->tbo.resource))
> stats->visible_vram += size;
> break;
> case AMDGPU_GEM_DOMAIN_GTT:
> @@ -1382,10 +1383,7 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct
> ttm_buffer_object *bo)
> /* Remember that this BO was accessed by the CPU */
> abo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>
> - if (bo->resource->mem_type != TTM_PL_VRAM)
> - return 0;
> -
> - if (amdgpu_bo_in_cpu_visible_vram(abo))
> + if (amdgpu_res_cpu_visible(adev, bo->resource))
> return 0;
>
> /* Can't move a pinned BO to visible VRAM */
> @@ -1409,7 +1407,7 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct
> ttm_buffer_object *bo)
>
> /* this should never happen */
> if (bo->resource->mem_type == TTM_PL_VRAM &&
> - !amdgpu_bo_in_cpu_visible_vram(abo))
> + !amdgpu_res_cpu_visible(adev, bo->resource))
> return VM_FAULT_SIGBUS;
>
> ttm_bo_move_to_lru_tail_unlocked(bo);
> @@ -1573,6 +1571,7 @@ uint32_t amdgpu_bo_get_preferred_domain(struct
> amdgpu_device *adev,
> */
> u64 amdgpu_bo_print_info(int id, struct amdgpu_bo *bo, struct seq_file *m)
> {
> + struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> struct dma_buf_attachment *attachment;
> struct dma_buf *dma_buf;
> const char *placement;
> @@ -1581,10 +1580,11 @@ u64 amdgpu_bo_print_info(int id, struct amdgpu_bo
> *bo, struct seq_file *m)
>
> if (dma_resv_trylock(bo->tbo.base.resv)) {
> unsigned int domain;
> +
> domain =
> amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type);
> switch (domain) {
>
Re: [PATCH v10 3/3] drm/tests: Add a test case for drm buddy clear allocation
On 08/04/2024 16:16, Arunpravin Paneer Selvam wrote:
Add a new test case for the drm buddy clear and dirty
allocation.
v2:(Matthew)
- make size as u32
- rename PAGE_SIZE with SZ_4K
- dont fragment the address space for all the order allocation
iterations. we can do it once and just increment and allocate
the size.
- create new mm with non power-of-two size to ensure the multi-root
force_merge during fini.
Signed-off-by: Arunpravin Paneer Selvam
Suggested-by: Matthew Auld
---
drivers/gpu/drm/tests/drm_buddy_test.c | 141 +
1 file changed, 141 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
b/drivers/gpu/drm/tests/drm_buddy_test.c
index 4621a860cb05..b07f132f2835 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -224,6 +224,146 @@ static void drm_test_buddy_alloc_range_bias(struct kunit
*test)
drm_buddy_fini();
}
+static void drm_test_buddy_alloc_clear(struct kunit *test)
+{
+ unsigned long n_pages, total, i = 0;
+ const unsigned long ps = SZ_4K;
+ struct drm_buddy_block *block;
+ const int max_order = 12;
+ LIST_HEAD(allocated);
+ struct drm_buddy mm;
+ unsigned int order;
+ u32 mm_size, size;
+ LIST_HEAD(dirty);
+ LIST_HEAD(clean);
+
+ mm_size = SZ_4K << max_order;
+ KUNIT_EXPECT_FALSE(test, drm_buddy_init(, mm_size, ps));
+
+ KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
+
+ /*
+* Idea is to allocate and free some random portion of the address
space,
+* returning those pages as non-dirty and randomly alternate between
+* requesting dirty and non-dirty pages (not going over the limit
+* we freed as non-dirty), putting that into two separate lists.
+* Loop over both lists at the end checking that the dirty list
+* is indeed all dirty pages and vice versa. Free it all again,
+* keeping the dirty/clear status.
+*/
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
+ 5 * ps, ps,
,
+
DRM_BUDDY_TOPDOWN_ALLOCATION),
+ "buddy_alloc hit an error size=%lu\n", 5 * ps);
+ drm_buddy_free_list(, , DRM_BUDDY_CLEARED);
+
+ n_pages = 10;
+ do {
+ unsigned long flags;
+ struct list_head *list;
+ int slot = i % 2;
+
+ if (slot == 0) {
+ list =
+ flags = 0;
+ } else {
+ list =
+ flags = DRM_BUDDY_CLEAR_ALLOCATION;
+ }
+
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0,
mm_size,
+ ps, ps,
list,
+ flags),
+ "buddy_alloc hit an error size=%lu\n",
ps);
+ } while (++i < n_pages);
+
+ list_for_each_entry(block, , link)
+ KUNIT_EXPECT_EQ(test, drm_buddy_block_is_clear(block), true);
+
+ list_for_each_entry(block, , link)
+ KUNIT_EXPECT_EQ(test, drm_buddy_block_is_clear(block), false);
+
+ drm_buddy_free_list(, , DRM_BUDDY_CLEARED);
+
+ /*
+* Trying to go over the clear limit for some allocation.
+* The allocation should never fail with reasonable page-size.
+*/
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
+ 10 * ps, ps, ,
+
DRM_BUDDY_CLEAR_ALLOCATION),
+ "buddy_alloc hit an error size=%lu\n", 10 * ps);
+
+ drm_buddy_free_list(, , DRM_BUDDY_CLEARED);
+ drm_buddy_free_list(, , 0);
+ drm_buddy_fini();
+
+ KUNIT_EXPECT_FALSE(test, drm_buddy_init(, mm_size, ps));
+
+ /*
+* Create a new mm. Intentionally fragment the address space by creating
+* two alternating lists. Free both lists, one as dirty the other as
clean.
+* Try to allocate double the previous size with matching
min_page_size. The
+* allocation should never fail as it calls the force_merge. Also check
that
+* the page is always dirty after force_merge. Free the page as dirty,
then
+* repeat the whole thing, increment the order until we hit the
max_order.
+*/
+
+ i = 0;
+ n_pages = mm_size / ps;
+ do {
+ struct list_head *list;
+ int slot = i % 2;
+
+ if (slot == 0)
+ list =
+ else
+ list =
+
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0,
mm_size,
+
[PATCH] drm/ttm: Make ttm shrinkers NUMA aware
Otherwise the nid is always passed as 0 during memory reclaim so make TTM shrinkers NUMA aware. Signed-off-by: Rajneesh Bhardwaj --- drivers/gpu/drm/ttm/ttm_pool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c index dbc96984d331..514261f44b78 100644 --- a/drivers/gpu/drm/ttm/ttm_pool.c +++ b/drivers/gpu/drm/ttm/ttm_pool.c @@ -794,7 +794,7 @@ int ttm_pool_mgr_init(unsigned long num_pages) _pool_debugfs_shrink_fops); #endif - mm_shrinker = shrinker_alloc(0, "drm-ttm_pool"); + mm_shrinker = shrinker_alloc(SHRINKER_NUMA_AWARE, "drm-ttm_pool"); if (!mm_shrinker) return -ENOMEM; -- 2.34.1
[PATCH] drm/radeon: silence UBSAN warning (v2)
Convert a variable sized array from [1] to [].
v2: fix up a few more.
Acked-by: Christian König (v1)
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/radeon/pptable.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/pptable.h b/drivers/gpu/drm/radeon/pptable.h
index 94947229888ba..3493b1f979fed 100644
--- a/drivers/gpu/drm/radeon/pptable.h
+++ b/drivers/gpu/drm/radeon/pptable.h
@@ -432,7 +432,7 @@ typedef struct _ATOM_PPLIB_STATE_V2
/**
* Driver will read the first ucNumDPMLevels in this array
*/
- UCHAR clockInfoIndex[1];
+ UCHAR clockInfoIndex[];
} ATOM_PPLIB_STATE_V2;
typedef struct _StateArray{
@@ -450,7 +450,7 @@ typedef struct _ClockInfoArray{
//sizeof(ATOM_PPLIB_CLOCK_INFO)
UCHAR ucEntrySize;
-UCHAR clockInfo[1];
+UCHAR clockInfo[];
}ClockInfoArray;
typedef struct _NonClockInfoArray{
@@ -460,7 +460,7 @@ typedef struct _NonClockInfoArray{
//sizeof(ATOM_PPLIB_NONCLOCK_INFO)
UCHAR ucEntrySize;
-ATOM_PPLIB_NONCLOCK_INFO nonClockInfo[1];
+ATOM_PPLIB_NONCLOCK_INFO nonClockInfo[];
}NonClockInfoArray;
typedef struct _ATOM_PPLIB_Clock_Voltage_Dependency_Record
--
2.44.0
[PATCH] drm/amdgpu/gfx11: properly handle regGRBM_GFX_CNTL in soft reset
Need to take the srbm_mutex and while we are here, use the
helper function soc21_grbm_select();
Signed-off-by: Alex Deucher
---
drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index 7a906318e4519..dc9c0f67607b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -4506,14 +4506,11 @@ static int gfx_v11_0_soft_reset(void *handle)
gfx_v11_0_set_safe_mode(adev, 0);
+ mutex_lock(>srbm_mutex);
for (i = 0; i < adev->gfx.mec.num_mec; ++i) {
for (j = 0; j < adev->gfx.mec.num_queue_per_pipe; j++) {
for (k = 0; k < adev->gfx.mec.num_pipe_per_mec; k++) {
- tmp = RREG32_SOC15(GC, 0, regGRBM_GFX_CNTL);
- tmp = REG_SET_FIELD(tmp, GRBM_GFX_CNTL, MEID,
i);
- tmp = REG_SET_FIELD(tmp, GRBM_GFX_CNTL,
QUEUEID, j);
- tmp = REG_SET_FIELD(tmp, GRBM_GFX_CNTL, PIPEID,
k);
- WREG32_SOC15(GC, 0, regGRBM_GFX_CNTL, tmp);
+ soc21_grbm_select(adev, i, k, j, 0);
WREG32_SOC15(GC, 0, regCP_HQD_DEQUEUE_REQUEST,
0x2);
WREG32_SOC15(GC, 0, regSPI_COMPUTE_QUEUE_RESET,
0x1);
@@ -4523,16 +4520,14 @@ static int gfx_v11_0_soft_reset(void *handle)
for (i = 0; i < adev->gfx.me.num_me; ++i) {
for (j = 0; j < adev->gfx.me.num_queue_per_pipe; j++) {
for (k = 0; k < adev->gfx.me.num_pipe_per_me; k++) {
- tmp = RREG32_SOC15(GC, 0, regGRBM_GFX_CNTL);
- tmp = REG_SET_FIELD(tmp, GRBM_GFX_CNTL, MEID,
i);
- tmp = REG_SET_FIELD(tmp, GRBM_GFX_CNTL,
QUEUEID, j);
- tmp = REG_SET_FIELD(tmp, GRBM_GFX_CNTL, PIPEID,
k);
- WREG32_SOC15(GC, 0, regGRBM_GFX_CNTL, tmp);
+ soc21_grbm_select(adev, i, k, j, 0);
WREG32_SOC15(GC, 0,
regCP_GFX_HQD_DEQUEUE_REQUEST, 0x1);
}
}
}
+ soc21_grbm_select(adev, 0, 0, 0, 0);
+ mutex_unlock(>srbm_mutex);
/* Try to acquire the gfx mutex before access to CP_VMID_RESET */
r = gfx_v11_0_request_gfx_index_mutex(adev, 1);
--
2.44.0
[linux-next:master] BUILD REGRESSION 11cb68ad52ac78c81e33b806b531f097e68edfa2
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 11cb68ad52ac78c81e33b806b531f097e68edfa2 Add linux-next specific files for 20240408 Error/Warning: (recently discovered and may have been fixed) drivers/gpu/drm/drm_mm.c:152:1: error: unused function 'drm_mm_interval_tree_insert' [-Werror,-Wunused-function] drivers/gpu/drm/drm_mm.c:152:1: error: unused function 'drm_mm_interval_tree_iter_next' [-Werror,-Wunused-function] drivers/gpu/drm/lima/lima_drv.c:387:13: error: cast to smaller integer type 'enum lima_gpu_id' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast] drivers/gpu/drm/pl111/pl111_versatile.c:488:24: error: cast to smaller integer type 'enum versatile_clcd' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast] Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-allyesconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- arm-allmodconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- arm-allyesconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- csky-allmodconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- csky-allyesconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- loongarch-allmodconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- microblaze-allmodconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- microblaze-allyesconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- openrisc-allyesconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- parisc-allmodconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | |-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size | `-- drivers-gpu-drm-nouveau-nvif-object.c:error:memcpy-accessing-or-more-bytes-at-offsets-and-overlaps-bytes-at-offset |-- parisc-allyesconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | |-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size | `-- drivers-gpu-drm-nouveau-nvif-object.c:error:memcpy-accessing-or-more-bytes-at-offsets-and-overlaps-bytes-at-offset |-- powerpc-allmodconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- s390-allyesconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d
Re: [PATCH v10 1/3] drm/buddy: Implement tracking clear page feature
Hi Matthew,
Could you please review these changes as few clients are waiting for
these patches.
Thanks,
Arun.
On 4/8/2024 8:46 PM, Arunpravin Paneer Selvam wrote:
- Add tracking clear page feature.
- Driver should enable the DRM_BUDDY_CLEARED flag if it
successfully clears the blocks in the free path. On the otherhand,
DRM buddy marks each block as cleared.
- Track the available cleared pages size
- If driver requests cleared memory we prefer cleared memory
but fallback to uncleared if we can't find the cleared blocks.
when driver requests uncleared memory we try to use uncleared but
fallback to cleared memory if necessary.
- When a block gets freed we clear it and mark the freed block as cleared,
when there are buddies which are cleared as well we can merge them.
Otherwise, we prefer to keep the blocks as separated.
- Add a function to support defragmentation.
v1:
- Depends on the flag check DRM_BUDDY_CLEARED, enable the block as
cleared. Else, reset the clear flag for each block in the list(Christian)
- For merging the 2 cleared blocks compare as below,
drm_buddy_is_clear(block) != drm_buddy_is_clear(buddy)(Christian)
- Defragment the memory beginning from min_order
till the required memory space is available.
v2: (Matthew)
- Add a wrapper drm_buddy_free_list_internal for the freeing of blocks
operation within drm buddy.
- Write a macro block_incompatible() to allocate the required blocks.
- Update the xe driver for the drm_buddy_free_list change in arguments.
- add a warning if the two blocks are incompatible on
defragmentation
- call full defragmentation in the fini() function
- place a condition to test if min_order is equal to 0
- replace the list with safe_reverse() variant as we might
remove the block from the list.
v3:
- fix Gitlab user reported lockup issue.
- Keep DRM_BUDDY_HEADER_CLEAR define sorted(Matthew)
- modify to pass the root order instead max_order in fini()
function(Matthew)
- change bool 1 to true(Matthew)
- add check if min_block_size is power of 2(Matthew)
- modify the min_block_size datatype to u64(Matthew)
v4:
- rename the function drm_buddy_defrag with __force_merge.
- Include __force_merge directly in drm buddy file and remove
the defrag use in amdgpu driver.
- Remove list_empty() check(Matthew)
- Remove unnecessary space, headers and placement of new variables(Matthew)
- Add a unit test case(Matthew)
v5:
- remove force merge support to actual range allocation and not to bail
out when contains && split(Matthew)
- add range support to force merge function.
Signed-off-by: Arunpravin Paneer Selvam
Signed-off-by: Matthew Auld
Suggested-by: Christian König
Suggested-by: Matthew Auld
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
drivers/gpu/drm/drm_buddy.c | 430 ++
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 6 +-
drivers/gpu/drm/tests/drm_buddy_test.c| 28 +-
drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 4 +-
include/drm/drm_buddy.h | 16 +-
6 files changed, 368 insertions(+), 122 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 8db880244324..c0c851409241 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -571,7 +571,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager
*man,
return 0;
error_free_blocks:
- drm_buddy_free_list(mm, >blocks);
+ drm_buddy_free_list(mm, >blocks, 0);
mutex_unlock(>lock);
error_fini:
ttm_resource_fini(man, >base);
@@ -604,7 +604,7 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager
*man,
amdgpu_vram_mgr_do_reserve(man);
- drm_buddy_free_list(mm, >blocks);
+ drm_buddy_free_list(mm, >blocks, 0);
mutex_unlock(>lock);
atomic64_sub(vis_usage, >vis_usage);
@@ -912,7 +912,7 @@ void amdgpu_vram_mgr_fini(struct amdgpu_device *adev)
kfree(rsv);
list_for_each_entry_safe(rsv, temp, >reserved_pages, blocks) {
- drm_buddy_free_list(>mm, >allocated);
+ drm_buddy_free_list(>mm, >allocated, 0);
kfree(rsv);
}
if (!adev->gmc.is_app_apu)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 5ebdd6f8f36e..83dbe252f727 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -38,8 +38,8 @@ static void drm_block_free(struct drm_buddy *mm,
kmem_cache_free(slab_blocks, block);
}
-static void list_insert_sorted(struct drm_buddy *mm,
- struct drm_buddy_block *block)
+static void list_insert(struct drm_buddy *mm,
+ struct drm_buddy_block *block)
{
struct drm_buddy_block *node;
struct list_head
[PATCH v10 3/3] drm/tests: Add a test case for drm buddy clear allocation
Add a new test case for the drm buddy clear and dirty
allocation.
v2:(Matthew)
- make size as u32
- rename PAGE_SIZE with SZ_4K
- dont fragment the address space for all the order allocation
iterations. we can do it once and just increment and allocate
the size.
- create new mm with non power-of-two size to ensure the multi-root
force_merge during fini.
Signed-off-by: Arunpravin Paneer Selvam
Suggested-by: Matthew Auld
---
drivers/gpu/drm/tests/drm_buddy_test.c | 141 +
1 file changed, 141 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
b/drivers/gpu/drm/tests/drm_buddy_test.c
index 4621a860cb05..b07f132f2835 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -224,6 +224,146 @@ static void drm_test_buddy_alloc_range_bias(struct kunit
*test)
drm_buddy_fini();
}
+static void drm_test_buddy_alloc_clear(struct kunit *test)
+{
+ unsigned long n_pages, total, i = 0;
+ const unsigned long ps = SZ_4K;
+ struct drm_buddy_block *block;
+ const int max_order = 12;
+ LIST_HEAD(allocated);
+ struct drm_buddy mm;
+ unsigned int order;
+ u32 mm_size, size;
+ LIST_HEAD(dirty);
+ LIST_HEAD(clean);
+
+ mm_size = SZ_4K << max_order;
+ KUNIT_EXPECT_FALSE(test, drm_buddy_init(, mm_size, ps));
+
+ KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
+
+ /*
+* Idea is to allocate and free some random portion of the address
space,
+* returning those pages as non-dirty and randomly alternate between
+* requesting dirty and non-dirty pages (not going over the limit
+* we freed as non-dirty), putting that into two separate lists.
+* Loop over both lists at the end checking that the dirty list
+* is indeed all dirty pages and vice versa. Free it all again,
+* keeping the dirty/clear status.
+*/
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
+ 5 * ps, ps,
,
+
DRM_BUDDY_TOPDOWN_ALLOCATION),
+ "buddy_alloc hit an error size=%lu\n", 5 * ps);
+ drm_buddy_free_list(, , DRM_BUDDY_CLEARED);
+
+ n_pages = 10;
+ do {
+ unsigned long flags;
+ struct list_head *list;
+ int slot = i % 2;
+
+ if (slot == 0) {
+ list =
+ flags = 0;
+ } else {
+ list =
+ flags = DRM_BUDDY_CLEAR_ALLOCATION;
+ }
+
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0,
mm_size,
+ ps, ps,
list,
+ flags),
+ "buddy_alloc hit an error size=%lu\n",
ps);
+ } while (++i < n_pages);
+
+ list_for_each_entry(block, , link)
+ KUNIT_EXPECT_EQ(test, drm_buddy_block_is_clear(block), true);
+
+ list_for_each_entry(block, , link)
+ KUNIT_EXPECT_EQ(test, drm_buddy_block_is_clear(block), false);
+
+ drm_buddy_free_list(, , DRM_BUDDY_CLEARED);
+
+ /*
+* Trying to go over the clear limit for some allocation.
+* The allocation should never fail with reasonable page-size.
+*/
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
+ 10 * ps, ps, ,
+
DRM_BUDDY_CLEAR_ALLOCATION),
+ "buddy_alloc hit an error size=%lu\n", 10 * ps);
+
+ drm_buddy_free_list(, , DRM_BUDDY_CLEARED);
+ drm_buddy_free_list(, , 0);
+ drm_buddy_fini();
+
+ KUNIT_EXPECT_FALSE(test, drm_buddy_init(, mm_size, ps));
+
+ /*
+* Create a new mm. Intentionally fragment the address space by creating
+* two alternating lists. Free both lists, one as dirty the other as
clean.
+* Try to allocate double the previous size with matching
min_page_size. The
+* allocation should never fail as it calls the force_merge. Also check
that
+* the page is always dirty after force_merge. Free the page as dirty,
then
+* repeat the whole thing, increment the order until we hit the
max_order.
+*/
+
+ i = 0;
+ n_pages = mm_size / ps;
+ do {
+ struct list_head *list;
+ int slot = i % 2;
+
+ if (slot == 0)
+ list =
+ else
+ list =
+
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0,
mm_size,
+ ps,
[PATCH v10 1/3] drm/buddy: Implement tracking clear page feature
- Add tracking clear page feature.
- Driver should enable the DRM_BUDDY_CLEARED flag if it
successfully clears the blocks in the free path. On the otherhand,
DRM buddy marks each block as cleared.
- Track the available cleared pages size
- If driver requests cleared memory we prefer cleared memory
but fallback to uncleared if we can't find the cleared blocks.
when driver requests uncleared memory we try to use uncleared but
fallback to cleared memory if necessary.
- When a block gets freed we clear it and mark the freed block as cleared,
when there are buddies which are cleared as well we can merge them.
Otherwise, we prefer to keep the blocks as separated.
- Add a function to support defragmentation.
v1:
- Depends on the flag check DRM_BUDDY_CLEARED, enable the block as
cleared. Else, reset the clear flag for each block in the list(Christian)
- For merging the 2 cleared blocks compare as below,
drm_buddy_is_clear(block) != drm_buddy_is_clear(buddy)(Christian)
- Defragment the memory beginning from min_order
till the required memory space is available.
v2: (Matthew)
- Add a wrapper drm_buddy_free_list_internal for the freeing of blocks
operation within drm buddy.
- Write a macro block_incompatible() to allocate the required blocks.
- Update the xe driver for the drm_buddy_free_list change in arguments.
- add a warning if the two blocks are incompatible on
defragmentation
- call full defragmentation in the fini() function
- place a condition to test if min_order is equal to 0
- replace the list with safe_reverse() variant as we might
remove the block from the list.
v3:
- fix Gitlab user reported lockup issue.
- Keep DRM_BUDDY_HEADER_CLEAR define sorted(Matthew)
- modify to pass the root order instead max_order in fini()
function(Matthew)
- change bool 1 to true(Matthew)
- add check if min_block_size is power of 2(Matthew)
- modify the min_block_size datatype to u64(Matthew)
v4:
- rename the function drm_buddy_defrag with __force_merge.
- Include __force_merge directly in drm buddy file and remove
the defrag use in amdgpu driver.
- Remove list_empty() check(Matthew)
- Remove unnecessary space, headers and placement of new variables(Matthew)
- Add a unit test case(Matthew)
v5:
- remove force merge support to actual range allocation and not to bail
out when contains && split(Matthew)
- add range support to force merge function.
Signed-off-by: Arunpravin Paneer Selvam
Signed-off-by: Matthew Auld
Suggested-by: Christian König
Suggested-by: Matthew Auld
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
drivers/gpu/drm/drm_buddy.c | 430 ++
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 6 +-
drivers/gpu/drm/tests/drm_buddy_test.c| 28 +-
drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 4 +-
include/drm/drm_buddy.h | 16 +-
6 files changed, 368 insertions(+), 122 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 8db880244324..c0c851409241 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -571,7 +571,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager
*man,
return 0;
error_free_blocks:
- drm_buddy_free_list(mm, >blocks);
+ drm_buddy_free_list(mm, >blocks, 0);
mutex_unlock(>lock);
error_fini:
ttm_resource_fini(man, >base);
@@ -604,7 +604,7 @@ static void amdgpu_vram_mgr_del(struct ttm_resource_manager
*man,
amdgpu_vram_mgr_do_reserve(man);
- drm_buddy_free_list(mm, >blocks);
+ drm_buddy_free_list(mm, >blocks, 0);
mutex_unlock(>lock);
atomic64_sub(vis_usage, >vis_usage);
@@ -912,7 +912,7 @@ void amdgpu_vram_mgr_fini(struct amdgpu_device *adev)
kfree(rsv);
list_for_each_entry_safe(rsv, temp, >reserved_pages, blocks) {
- drm_buddy_free_list(>mm, >allocated);
+ drm_buddy_free_list(>mm, >allocated, 0);
kfree(rsv);
}
if (!adev->gmc.is_app_apu)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index 5ebdd6f8f36e..83dbe252f727 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -38,8 +38,8 @@ static void drm_block_free(struct drm_buddy *mm,
kmem_cache_free(slab_blocks, block);
}
-static void list_insert_sorted(struct drm_buddy *mm,
- struct drm_buddy_block *block)
+static void list_insert(struct drm_buddy *mm,
+ struct drm_buddy_block *block)
{
struct drm_buddy_block *node;
struct list_head *head;
@@ -57,6 +57,16 @@ static void list_insert_sorted(struct drm_buddy *mm,
__list_add(>link, node->link.prev, >link);
}
+static void clear_reset(struct drm_buddy_block *block)
+{
+
[PATCH v10 2/3] drm/amdgpu: Enable clear page functionality
Add clear page support in vram memory region.
v1(Christian):
- Dont handle clear page as TTM flag since when moving the BO back
in from GTT again we don't need that.
- Make a specialized version of amdgpu_fill_buffer() which only
clears the VRAM areas which are not already cleared
- Drop the TTM_PL_FLAG_WIPE_ON_RELEASE check in
amdgpu_object.c
v2:
- Modify the function name amdgpu_ttm_* (Alex)
- Drop the delayed parameter (Christian)
- handle amdgpu_res_cleared() just above the size
calculation (Christian)
- Use AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE for clearing the buffers
in the free path to properly wait for fences etc.. (Christian)
v3(Christian):
- Remove buffer clear code in VRAM manager instead change the
AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE handling to set
the DRM_BUDDY_CLEARED flag.
- Remove ! from amdgpu_res_cleared() check.
v4(Christian):
- vres flag setting move to vram manager file
- use dma_fence_get_stub in amdgpu_ttm_clear_buffer function
- make fence a mandatory parameter and drop the if and the get/put dance
Signed-off-by: Arunpravin Paneer Selvam
Suggested-by: Christian König
Acked-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 9 +--
.../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h| 25 +++
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 70 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 5 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 10 +++
6 files changed, 116 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 8bc79924d171..e011a326fe86 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -39,6 +39,7 @@
#include "amdgpu.h"
#include "amdgpu_trace.h"
#include "amdgpu_amdkfd.h"
+#include "amdgpu_vram_mgr.h"
/**
* DOC: amdgpu_object
@@ -601,8 +602,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
if (!amdgpu_bo_support_uswc(bo->flags))
bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
- if (adev->ras_enabled)
- bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
+ bo->flags |= AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE;
bo->tbo.bdev = >mman.bdev;
if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
@@ -634,7 +634,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
bo->tbo.resource->mem_type == TTM_PL_VRAM) {
struct dma_fence *fence;
- r = amdgpu_fill_buffer(bo, 0, bo->tbo.base.resv, , true);
+ r = amdgpu_ttm_clear_buffer(bo, bo->tbo.base.resv, );
if (unlikely(r))
goto fail_unreserve;
@@ -1365,8 +1365,9 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object
*bo)
if (WARN_ON_ONCE(!dma_resv_trylock(bo->base.resv)))
return;
- r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv, , true);
+ r = amdgpu_fill_buffer(abo, 0, bo->base.resv, , true);
if (!WARN_ON(r)) {
+ amdgpu_vram_mgr_set_cleared(bo->resource);
amdgpu_bo_fence(abo, fence, false);
dma_fence_put(fence);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
index 381101d2bf05..50fcd86e1033 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -164,4 +164,29 @@ static inline void amdgpu_res_next(struct
amdgpu_res_cursor *cur, uint64_t size)
}
}
+/**
+ * amdgpu_res_cleared - check if blocks are cleared
+ *
+ * @cur: the cursor to extract the block
+ *
+ * Check if the @cur block is cleared
+ */
+static inline bool amdgpu_res_cleared(struct amdgpu_res_cursor *cur)
+{
+ struct drm_buddy_block *block;
+
+ switch (cur->mem_type) {
+ case TTM_PL_VRAM:
+ block = cur->node;
+
+ if (!amdgpu_vram_mgr_is_cleared(block))
+ return false;
+ break;
+ default:
+ return false;
+ }
+
+ return true;
+}
+
#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index fc418e670fda..f0b42b567357 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -378,11 +378,12 @@ static int amdgpu_move_blit(struct ttm_buffer_object *bo,
(abo->flags & AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE)) {
struct dma_fence *wipe_fence = NULL;
- r = amdgpu_fill_buffer(abo, AMDGPU_POISON, NULL, _fence,
- false);
+ r = amdgpu_fill_buffer(abo, 0, NULL, _fence,
+ false);
if (r) {
Re: [RFC PATCH v3 0/6] drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid
On 03/28, Jani Nikula wrote: > On Wed, 27 Mar 2024, Melissa Wen wrote: > > 2. Most of the edid data handled by `dm_helpers_parse_edid_caps()` are > >in drm_edid halpers, but there are still a few that are not managed by > >them yet. For example: > >``` > > edid_caps->serial_number = edid_buf->serial; > > edid_caps->manufacture_week = edid_buf->mfg_week; > > edid_caps->manufacture_year = edid_buf->mfg_year; > >``` > >AFAIU I see the same pending issue in i915/pnpid_get_panel_type() - > >that still uses drm_edid_raw(). > > See https://lore.kernel.org/r/cover.1711015462.git.jani.nik...@intel.com Thanks! I'll work on top of your series. Melissa > > BR, > Jani. > > > -- > Jani Nikula, Intel
Re: [RFC PATCH v3 3/6] drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid
On 03/28, Jani Nikula wrote:
> On Wed, 27 Mar 2024, Melissa Wen wrote:
> > Replace raw edid handling (struct edid) with the opaque EDID type
> > (struct drm_edid) on amdgpu_dm_connector for consistency. It may also
> > prevent mismatch of approaches in different parts of the driver code.
> > Working in progress. It was only exercised with IGT tests.
> >
> > v2: use const to fix warnings (Alex Hung)
> > v3: fix general protection fault on mst
> >
> > Signed-off-by: Melissa Wen
> > ---
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 99 +--
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 4 +-
> > .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 8 +-
> > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 24 ++---
> > 4 files changed, 67 insertions(+), 68 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 280562707cd0..bbbf9c9d40d5 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -3269,18 +3269,19 @@ void amdgpu_dm_update_connector_after_detect(
> > aconnector->dc_sink = sink;
> > dc_sink_retain(aconnector->dc_sink);
> > if (sink->dc_edid.length == 0) {
> > - aconnector->edid = NULL;
> > + drm_edid_free(aconnector->edid);
>
> If aconnector->edid is used as some kind of cache, it might be prudent
> to still set it to NULL.
Makes sense. Thanks for pointing it out.
>
> It's up to the amd maintainers, but personally I've renamed any ->edid
> fields to ->drm_edid when I've done these conversions to ensure every
> single place is covered, and also later stable backports will give a
> build failure if the change is not taken into account.
I see. Good points. I'll follow your advice and do this name change in
the next version.
>
> > if (aconnector->dc_link->aux_mode) {
> > drm_dp_cec_unset_edid(
> > >dm_dp_aux.aux);
> > }
> > } else {
> > - aconnector->edid =
> > - (struct edid *)sink->dc_edid.raw_edid;
> > + const struct edid *edid = (const struct edid
> > *)sink->dc_edid.raw_edid;
> > + aconnector->edid = drm_edid_alloc(edid,
> > sink->dc_edid.length);
> >
> > + /* FIXME: Get rid of drm_edid_raw() */
> > if (aconnector->dc_link->aux_mode)
> > drm_dp_cec_set_edid(>dm_dp_aux.aux,
> > - aconnector->edid);
> > +
> > drm_edid_raw(aconnector->edid));
>
> The goal should be to switch to use CEC functions that take the source
> physical address directly instead of parsing the EDID.
Yes, I understand I have the same goal as in the next patch:
- https://lore.kernel.org/dri-devel/20240327214953.367126-5-m...@igalia.com/
Am I missing something?
>
> > }
> >
> > if (!aconnector->timing_requested) {
> > @@ -3291,17 +3292,17 @@ void amdgpu_dm_update_connector_after_detect(
> > "failed to create
> > aconnector->requested_timing\n");
> > }
> >
> > - drm_connector_update_edid_property(connector, aconnector->edid);
> > + drm_edid_connector_update(connector, aconnector->edid);
> > amdgpu_dm_update_freesync_caps(connector, aconnector->edid);
> > update_connector_ext_caps(aconnector);
> > } else {
> > drm_dp_cec_unset_edid(>dm_dp_aux.aux);
> > amdgpu_dm_update_freesync_caps(connector, NULL);
> > - drm_connector_update_edid_property(connector, NULL);
> > + drm_edid_connector_update(connector, NULL);
> > aconnector->num_modes = 0;
> > dc_sink_release(aconnector->dc_sink);
> > aconnector->dc_sink = NULL;
> > - aconnector->edid = NULL;
> > + drm_edid_free(aconnector->edid);
> > kfree(aconnector->timing_requested);
> > aconnector->timing_requested = NULL;
> > /* Set CP to DESIRED if it was ENABLED, so we can re-enable it
> > again on hotplug */
> > @@ -6661,13 +6662,7 @@ static void amdgpu_dm_connector_funcs_force(struct
> > drm_connector *connector)
> > struct amdgpu_dm_connector *aconnector =
> > to_amdgpu_dm_connector(connector);
> > struct dc_link *dc_link = aconnector->dc_link;
> > struct dc_sink *dc_em_sink = aconnector->dc_em_sink;
> > - struct edid *edid;
> > - struct i2c_adapter *ddc;
> > -
> > - if (dc_link && dc_link->aux_mode)
> > - ddc = >dm_dp_aux.aux.ddc;
> > - else
> > - ddc = >i2c->base;
> > + const struct drm_edid *drm_edid;
> >
> > /*
> > * Note: drm_get_edid gets edid in
[PATCH] drm/amdgpu: Fix tlb_cb memory leaking
After updating GPU page table via CPU on large bar system, no fence
callback, call amdgpu_vm_tlb_seq_cb directly after command committed
to free tlb_cb.
memory leaking backtrace from kmemleakd:
unreferenced object 0xa036816b00c0 (size 32):
backtrace:
__kmem_cache_alloc_node+0x3fe/0x4d0
kmalloc_trace+0x2a/0xb0
amdgpu_vm_update_range+0x9b/0x8d0 [amdgpu]
amdgpu_vm_clear_freed+0xc1/0x210 [amdgpu]
unmap_bo_from_gpuvm.isra.36+0x37/0x50 [amdgpu]
amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu+0x118/0x1b0 [amdgpu]
kfd_process_device_free_bos+0x7c/0xe0 [amdgpu]
kfd_process_wq_release+0x273/0x3c0 [amdgpu]
process_scheduled_works+0x2a7/0x500
worker_thread+0x186/0x340
Fixes: 220ecde84bc8 ("drm/amdgpu: implement TLB flush fence")
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8af3f0fd3073..d0ef727cd7e1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -901,12 +901,9 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
{
struct amdgpu_vm *vm = params->vm;
- if (!fence || !*fence)
- return;
-
tlb_cb->vm = vm;
- if (!dma_fence_add_callback(*fence, _cb->cb,
- amdgpu_vm_tlb_seq_cb)) {
+ if (fence && *fence &&
+ !dma_fence_add_callback(*fence, _cb->cb, amdgpu_vm_tlb_seq_cb))
{
dma_fence_put(vm->last_tlb_flush);
vm->last_tlb_flush = dma_fence_get(*fence);
} else {
--
2.43.2
Re: [PATCH v0 10/14] sfc: falcon: Make I2C terminology more inclusive
On Thu, Apr 04, 2024 at 12:18:06PM -0700, Easwar Hariharan wrote: > On 4/2/2024 2:00 AM, Martin Habets wrote: > > On Fri, Mar 29, 2024 at 05:00:34PM +, Easwar Hariharan wrote: > >> I2C v7, SMBus 3.2, and I3C specifications have replaced "master/slave" > >> with more appropriate terms. Inspired by and following on to Wolfram's > >> series to fix drivers/i2c/[1], fix the terminology for users of > >> I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists > >> in the specification. > >> > >> Compile tested, no functionality changes intended > >> > >> [1]: > >> https://lore.kernel.org/all/20240322132619.6389-1-wsa+rene...@sang-engineering.com/ > >> > >> Signed-off-by: Easwar Hariharan > > > > Reviewed-by: Martin Habets > > > > Thank you, Martin, for reviewing. I believe that we are settling on > controller/target > terminology from feedback on the other drivers in this series. Would you want > to re-review > v1 with that change, or should I add your R-B in v1 despite the change? You can add in my Reviewed-by. Martin > Thanks, > Easwar >
RE: [PATCH] drm/amdgpu: Fix incorrect return value
[AMD Official Use Only - General]
-
Best Regards,
Thomas
-Original Message-
From: Zhou1, Tao
Sent: Monday, April 8, 2024 4:41 PM
To: Chai, Thomas ; amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Li, Candice ;
Wang, Yang(Kevin) ; Yang, Stanley
Subject: RE: [PATCH] drm/amdgpu: Fix incorrect return value
[AMD Official Use Only - General]
> -Original Message-
> From: Chai, Thomas
> Sent: Sunday, April 7, 2024 10:21 AM
> To: Zhou1, Tao ; amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking ; Li, Candice
> ; Wang, Yang(Kevin) ;
> Yang, Stanley
> Subject: RE: [PATCH] drm/amdgpu: Fix incorrect return value
>
> [AMD Official Use Only - General]
>
> -
> Best Regards,
> Thomas
>
> -Original Message-
> From: Zhou1, Tao
> Sent: Wednesday, April 3, 2024 6:36 PM
> To: Chai, Thomas ; amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking ; Li, Candice
> ; Wang, Yang(Kevin) ;
> Yang, Stanley
> Subject: RE: [PATCH] drm/amdgpu: Fix incorrect return value
>
> [AMD Official Use Only - General]
>
> > -Original Message-
> > From: Chai, Thomas
> > Sent: Wednesday, April 3, 2024 3:07 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Chai, Thomas ; Zhang, Hawking
> > ; Zhou1, Tao ; Li, Candice
> > ; Wang, Yang(Kevin) ;
> > Yang, Stanley ; Chai, Thomas
> >
> > Subject: [PATCH] drm/amdgpu: Fix incorrect return value
> >
> > [Why]
> > After calling amdgpu_vram_mgr_reserve_range multiple times with
> > the same address, calling amdgpu_vram_mgr_query_page_status will
> > always return - EBUSY.
>
> >[Tao] could you explain why we call amdgpu_vram_mgr_reserve_range
> >multiple
> times with the same address? IIRC, we skip duplicate address before
> reserve memory.
>
> [Thomas]
>When poison creation interrupt is received, since some poisoning
> addresses may have been allocated by some processes, reserving these memories
> will fail.
> These memory will be tried to reserve again after killing the poisoned
> process in the subsequent poisoning consumption interrupt handler.
> so amdgpu_vram_mgr_reserve_range needs to be called multiple times
> with the same address.
>
> > From the second call to amdgpu_vram_mgr_reserve_range, the same
> > address will be added to the reservations_pending list again and is
> > never moved to the reserved_pages list because the address had been
> reserved.
>[Tao] but if a page is added to reservations_pending list, it should also be
>put in data->bps array, and when we call amdgpu_ras_add_bad_pages again,
>amdgpu_ras_check_bad_page_unlock could ignore this page.
So except for amdgpu_ras_add_bad_pages, would you like to call
amdgpu_vram_mgr_reserve_range in other place?
[Thomas] Yes, Since after amdgpu_ras_add_bad_pages is called, the bad pages
will be saved to eeprom. When a large number of bad pages need to be reserved,
this will delay subsequent memory reservation.
I want to call amdgpu_vram_mgr_reserve_range to reserve memory immediately when
driver receives poison creation interrupt, this can reduce the probability of
bad memory pages being allocated and storing the bad pages to eeprom can be
done slowly.
> >
> > [How]
> > First add the address status check before calling
> > amdgpu_vram_mgr_do_reserve, if the address is already reserved, do
> > nothing; If the address is already in the reservations_pending list,
> > directly reserve memory; only add new nodes for the addresses that
> > are not in the reserved_pages list and reservations_pending list.
> >
> > Signed-off-by: YiPeng Chai
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 28
> > +---
> > 1 file changed, 19 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > index 1e36c428d254..0bf3f4092900 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > @@ -317,7 +317,6 @@ static void amdgpu_vram_mgr_do_reserve(struct
> > ttm_resource_manager *man)
> >
> > dev_dbg(adev->dev, "Reservation 0x%llx - %lld, Succeeded\n",
> > rsv->start, rsv->size);
> > -
> > vis_usage = amdgpu_vram_mgr_vis_size(adev, block);
> > atomic64_add(vis_usage, >vis_usage);
> > spin_lock(>bdev->lru_lock); @@ -340,19 +339,30 @@
> > int amdgpu_vram_mgr_reserve_range(struct
> > amdgpu_vram_mgr *mgr,
> > uint64_t start, uint64_t size) {
> > struct amdgpu_vram_reservation *rsv;
> > + int ret = 0;
> >
> > - rsv = kzalloc(sizeof(*rsv), GFP_KERNEL);
> > - if (!rsv)
> > - return -ENOMEM;
> > + ret = amdgpu_vram_mgr_query_page_status(mgr, start);
> > + if (!ret)
> > + return 0;
> > +
> > + if (ret == -ENOENT) {
> > + rsv = kzalloc(sizeof(*rsv), GFP_KERNEL);
> > + if (!rsv)
> > + return -ENOMEM;
> >
> >
RE: [PATCH] drm/amdgpu: Fix incorrect return value
[AMD Official Use Only - General]
> -Original Message-
> From: Chai, Thomas
> Sent: Sunday, April 7, 2024 10:21 AM
> To: Zhou1, Tao ; amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking ; Li, Candice
> ; Wang, Yang(Kevin) ; Yang,
> Stanley
> Subject: RE: [PATCH] drm/amdgpu: Fix incorrect return value
>
> [AMD Official Use Only - General]
>
> -
> Best Regards,
> Thomas
>
> -Original Message-
> From: Zhou1, Tao
> Sent: Wednesday, April 3, 2024 6:36 PM
> To: Chai, Thomas ; amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking ; Li, Candice
> ; Wang, Yang(Kevin) ; Yang,
> Stanley
> Subject: RE: [PATCH] drm/amdgpu: Fix incorrect return value
>
> [AMD Official Use Only - General]
>
> > -Original Message-
> > From: Chai, Thomas
> > Sent: Wednesday, April 3, 2024 3:07 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Chai, Thomas ; Zhang, Hawking
> > ; Zhou1, Tao ; Li, Candice
> > ; Wang, Yang(Kevin) ;
> > Yang, Stanley ; Chai, Thomas
> >
> > Subject: [PATCH] drm/amdgpu: Fix incorrect return value
> >
> > [Why]
> > After calling amdgpu_vram_mgr_reserve_range multiple times with the
> > same address, calling amdgpu_vram_mgr_query_page_status will always
> > return - EBUSY.
>
> >[Tao] could you explain why we call amdgpu_vram_mgr_reserve_range multiple
> times with the same address? IIRC, we skip duplicate address before reserve
> memory.
>
> [Thomas]
>When poison creation interrupt is received, since some poisoning addresses
> may
> have been allocated by some processes, reserving these memories will fail.
> These memory will be tried to reserve again after killing the poisoned
> process in
> the subsequent poisoning consumption interrupt handler.
> so amdgpu_vram_mgr_reserve_range needs to be called multiple times with the
> same address.
>
> > From the second call to amdgpu_vram_mgr_reserve_range, the same
> > address will be added to the reservations_pending list again and is
> > never moved to the reserved_pages list because the address had been
> reserved.
[Tao] but if a page is added to reservations_pending list, it should also be
put in data->bps array, and when we call amdgpu_ras_add_bad_pages again,
amdgpu_ras_check_bad_page_unlock could ignore this page.
So except for amdgpu_ras_add_bad_pages, would you like to call
amdgpu_vram_mgr_reserve_range in other place?
> >
> > [How]
> > First add the address status check before calling
> > amdgpu_vram_mgr_do_reserve, if the address is already reserved, do
> > nothing; If the address is already in the reservations_pending list,
> > directly reserve memory; only add new nodes for the addresses that are
> > not in the reserved_pages list and reservations_pending list.
> >
> > Signed-off-by: YiPeng Chai
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 28
> > +---
> > 1 file changed, 19 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > index 1e36c428d254..0bf3f4092900 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> > @@ -317,7 +317,6 @@ static void amdgpu_vram_mgr_do_reserve(struct
> > ttm_resource_manager *man)
> >
> > dev_dbg(adev->dev, "Reservation 0x%llx - %lld, Succeeded\n",
> > rsv->start, rsv->size);
> > -
> > vis_usage = amdgpu_vram_mgr_vis_size(adev, block);
> > atomic64_add(vis_usage, >vis_usage);
> > spin_lock(>bdev->lru_lock); @@ -340,19 +339,30 @@
> > int amdgpu_vram_mgr_reserve_range(struct
> > amdgpu_vram_mgr *mgr,
> > uint64_t start, uint64_t size) {
> > struct amdgpu_vram_reservation *rsv;
> > + int ret = 0;
> >
> > - rsv = kzalloc(sizeof(*rsv), GFP_KERNEL);
> > - if (!rsv)
> > - return -ENOMEM;
> > + ret = amdgpu_vram_mgr_query_page_status(mgr, start);
> > + if (!ret)
> > + return 0;
> > +
> > + if (ret == -ENOENT) {
> > + rsv = kzalloc(sizeof(*rsv), GFP_KERNEL);
> > + if (!rsv)
> > + return -ENOMEM;
> >
> > - INIT_LIST_HEAD(>allocated);
> > - INIT_LIST_HEAD(>blocks);
> > + INIT_LIST_HEAD(>allocated);
> > + INIT_LIST_HEAD(>blocks);
> >
> > - rsv->start = start;
> > - rsv->size = size;
> > + rsv->start = start;
> > + rsv->size = size;
> > +
> > + mutex_lock(>lock);
> > + list_add_tail(>blocks, >reservations_pending);
> > + mutex_unlock(>lock);
> > +
> > + }
> >
> > mutex_lock(>lock);
> > - list_add_tail(>blocks, >reservations_pending);
> > amdgpu_vram_mgr_do_reserve(>manager);
> > mutex_unlock(>lock);
> >
> > --
> > 2.34.1
>
>
RE: [PATCH] drm/amdkfd: make sure VM is ready for updating operations
[AMD Official Use Only - General]
>-Original Message-
>From: Koenig, Christian
>Sent: Monday, April 8, 2024 3:55 PM
>To: Yu, Lang ; amd-gfx@lists.freedesktop.org
>Cc: Kuehling, Felix
>Subject: Re: [PATCH] drm/amdkfd: make sure VM is ready for updating
>operations
>
>Am 07.04.24 um 06:52 schrieb Lang Yu:
>> When VM is in evicting state, amdgpu_vm_update_range would return -
>EBUSY.
>> Then restore_process_worker runs into a dead loop.
>>
>> Fixes: 2fdba514ad5a ("drm/amdgpu: Auto-validate DMABuf imports in
>> compute VMs")
>
>Mhm, while it would be good to have this case handled as error it should
>never occur in practice since we should have validated the VM before
>validating the DMA-bufs.
When page table BOs were evicted but not validated before updating page tables,
VM is still in evicting state, then the issue happened.
Regards,
Lang
>@Felix isn't that something we have taken care of?
>
>Regards,
>Christian.
>
>
>>
>> Signed-off-by: Lang Yu
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 ++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> index 0ae9fd844623..8c71fe07807a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> @@ -2900,6 +2900,12 @@ int
>> amdgpu_amdkfd_gpuvm_restore_process_bos(void *info, struct
>dma_fence
>> __rcu *
>>
>> amdgpu_sync_create(_obj);
>>
>> +ret = process_validate_vms(process_info, NULL);
>> +if (ret) {
>> +pr_debug("Validating VMs failed, ret: %d\n", ret);
>> +goto validate_map_fail;
>> +}
>> +
>> /* Validate BOs and map them to GPUVM (update VM page tables).
>*/
>> list_for_each_entry(mem, _info->kfd_bo_list,
>> validate_list) {
Re: [PATCH] drm/amdkfd: make sure VM is ready for updating operations
Am 07.04.24 um 06:52 schrieb Lang Yu:
When VM is in evicting state, amdgpu_vm_update_range would return -EBUSY.
Then restore_process_worker runs into a dead loop.
Fixes: 2fdba514ad5a ("drm/amdgpu: Auto-validate DMABuf imports in compute VMs")
Mhm, while it would be good to have this case handled as error it should
never occur in practice since we should have validated the VM before
validating the DMA-bufs.
@Felix isn't that something we have taken care of?
Regards,
Christian.
Signed-off-by: Lang Yu
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 0ae9fd844623..8c71fe07807a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -2900,6 +2900,12 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info,
struct dma_fence __rcu *
amdgpu_sync_create(_obj);
+ ret = process_validate_vms(process_info, NULL);
+ if (ret) {
+ pr_debug("Validating VMs failed, ret: %d\n", ret);
+ goto validate_map_fail;
+ }
+
/* Validate BOs and map them to GPUVM (update VM page tables). */
list_for_each_entry(mem, _info->kfd_bo_list,
validate_list) {
Re: [PATCH v0 00/14] Make I2C terminology more inclusive for I2C Algobit and consumers
On 05/04/2024 12:18, Wolfram Sang wrote: > Hello Easwar, > > On Fri, Mar 29, 2024 at 05:00:24PM +, Easwar Hariharan wrote: >> I2C v7, SMBus 3.2, and I3C specifications have replaced "master/slave" >> with more appropriate terms. Inspired by and following on to Wolfram's >> series to fix drivers/i2c/[1], fix the terminology for users of the >> I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists >> in the specification. > > I really appreciate that you want to assist in this task to improve the > I2C core. I do. I am afraid, however, that you took the second step > before the first one, though. As I mentioned in my original cover > letter, this is not only about renaming but also improving the I2C API > (splitting up header files...). So, drivers are not a priority right > now. They can be better fixed once the core is ready. > > It is true that I changed quite some controller drivers within the i2c > realm. I did this to gain experience. As you also noticed quite some > questions came up. We need to agree on answers first. And once we are > happy with the answers we found, then IMO we can go outside of the i2c > realm and send patches to other subsystems referencing agreed > precedence. I intentionally did not go outside i2c yet. Since your > patches are already there, you probably want to foster them until they > are ready for inclusion. Yet, regarding further patches, my suggestion > is to wait until the core is ready. That might take a while, though. > However, there is enough to discuss until the core is ready. So, your > collaboration there is highly appreciated! > >> The last patch updating the .master_xfer method to .xfer depends on >> patch 1 of Wolfram's series below, but the series is otherwise >> independent. It may make sense for the last patch to go in with > > Please drop the last patch from this series. It will nicely remove the > dependency. Also, like above, I first want to gain experience with i2c > before going to other subsystems. That was intended. OK, based on this I'll mark the media patches in this series as 'Deferred' in our patchwork. Regards, Hans > > All the best and happy hacking, > >Wolfram >
Re: [PATCH v0 00/14] Make I2C terminology more inclusive for I2C Algobit and consumers
Hi Easwar, > Sorry, got excited. :) There were drivers I'd been part of that I specifically > wanted to fixup, but then the scope grew to other users of algobit. Well, you got some positive feedback, so that is good. > > It is true that I changed quite some controller drivers within the i2c > > realm. I did this to gain experience. As you also noticed quite some > > questions came up. We need to agree on answers first. And once we are > > happy with the answers we found, then IMO we can go outside of the i2c > > realm and send patches to other subsystems referencing agreed > > precedence. I intentionally did not go outside i2c yet. Since your > > patches are already there, you probably want to foster them until they > > are ready for inclusion. > > Sorry, I don't quite follow what you mean by foster in this context. Are > you asking me to hold off on merging the series, or to follow through on > getting it merged? I think they are your patches, so this is up to you to decide. With "foster", I meant you keep working on them until everyone is happy. I haven't looked at the drivers you modify. I can't tell if they can be converted right away or if they use a lot of I2C API calls, so that it makes sense to wait until the core is converted. I trust you to decide this. Happy hacking, Wolfram signature.asc Description: PGP signature
[bug report] drm/amd/display: Remove plane and stream pointers from dc scratch
Hello Alvin Lee,
Commit 285a7054bf81 ("drm/amd/display: Remove plane and stream
pointers from dc scratch") from Mar 15, 2024 (linux-next), leads to
the following Smatch static checker warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn20/dcn20_hwseq.c:1112
dcn20_set_input_transfer_func() warn: address of
'plane_state->in_transfer_func' is non-NULL
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn10/dcn10_hwseq.c:1839
dcn10_set_input_transfer_func() warn: address of
'plane_state->in_transfer_func' is non-NULL
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dce110/dce110_hwseq.c:301
dce110_set_input_transfer_func() warn: address of
'plane_state->in_transfer_func' is non-NULL
drivers/gpu/drm/amd/amdgpu/../display/dc/hwss/dcn20/dcn20_hwseq.c
1094 bool dcn20_set_input_transfer_func(struct dc *dc,
1095 struct pipe_ctx *pipe_ctx,
1096 const struct dc_plane_state
*plane_state)
1097 {
1098 struct dce_hwseq *hws = dc->hwseq;
1099 struct dpp *dpp_base = pipe_ctx->plane_res.dpp;
1100 const struct dc_transfer_func *tf = NULL;
^
This assignment is not necessary now.
1101 bool result = true;
1102 bool use_degamma_ram = false;
1103
1104 if (dpp_base == NULL || plane_state == NULL)
1105 return false;
1106
1107 hws->funcs.set_shaper_3dlut(pipe_ctx, plane_state);
1108 hws->funcs.set_blend_lut(pipe_ctx, plane_state);
1109
1110 tf = _state->in_transfer_func;
^
Before there was an if statement but now tf is assigned unconditionally
--> 1112 if (tf == NULL) {
^
so these conditions are impossible.
1113 dpp_base->funcs->dpp_set_degamma(dpp_base,
1114 IPP_DEGAMMA_MODE_BYPASS);
1115 return true;
1116 }
1117
1118 if (tf->type == TF_TYPE_HWPWL || tf->type ==
TF_TYPE_DISTRIBUTED_POINTS)
1119 use_degamma_ram = true;
1120
1121 if (use_degamma_ram == true) {
1122 if (tf->type == TF_TYPE_HWPWL)
1123
dpp_base->funcs->dpp_program_degamma_pwl(dpp_base,
regards,
dan carpenter
Re: [PATCH] drm/amdgpu: once more fix the call oder in amdgpu_ttm_move()
Hello Christian.
On čtvrtek 21. března 2024 15:37:27, CEST Christian König wrote:
> Am 21.03.24 um 15:12 schrieb Tvrtko Ursulin:
> >
> > On 21/03/2024 12:43, Christian König wrote:
> >> This reverts drm/amdgpu: fix ftrace event amdgpu_bo_move always move
> >> on same heap. The basic problem here is that after the move the old
> >> location is simply not available any more.
> >>
> >> Some fixes where suggested, but essentially we should call the move
> >> notification before actually moving things because only this way we have
> >> the correct order for DMA-buf and VM move notifications as well.
> >>
> >> Also rework the statistic handling so that we don't update the eviction
> >> counter before the move.
> >>
> >> Signed-off-by: Christian König
> >
> > Don't forget:
> >
> > Fixes: 94aeb4117343 ("drm/amdgpu: fix ftrace event amdgpu_bo_move
> > always move on same heap")
> > Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3171
>
> Ah, thanks. I already wanted to ask if there is any bug report about
> that as well.
Do you happen to know if there's some pre-requisite for this patch to also be
picked while backporting your fix into v6.8? I've tried applying this single
patch on top of bare v6.8 and got lots of BUGs triggered:
```
BUG: unable to handle page fault for address: 001001c0
#PF: supervisor read access in kernel mode
#PF: error_code(0x) - not-present page
PGD 0 P4D 0
Oops: [#1] PREEMPT SMP NOPTI
CPU: 3 PID: 378 Comm: kworker/u68:0 Not tainted 6.8.0-pf3 #1
30fa7177996c08e3c7c351ca59508acf72424acd
Hardware name: ASUS System Product Name/Pro WS X570-ACE, BIOS 4702 10/20/2023
Workqueue: ttm ttm_bo_delayed_delete [ttm]
RIP: 0010:amdgpu_vm_bo_invalidate+0x22/0x390 [amdgpu]
Code: 90 90 90 90 90 90 90 90 66 0f 1f 00 0f 1f 44 00 00 41 56 41 55 41 54 55
48 89 f5 53 48 8b 86 58 02 00 00 48 85 c0 74 16 31 c9 <83> b8 c0 01 00 00 01 0f
84 cb 02 00 00 48 39 cd 48 0f 44 e8 48 8b
RSP: 0018:a43440e47e00 EFLAGS: 00010246
RAX: 0010 RBX: 93419508ae00 RCX:
RDX: RSI: 93419508ada8 RDI: 93419590
RBP: 93419508ada8 R08: R09:
R10: 0001 R11: 934180a6ea80 R12: 93418302f060
R13: 93419508ada8 R14: 93418d30de05 R15: 93419508afb0
FS: () GS:93602eac() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 001001c0 CR3: 00010a7ac000 CR4: 00f50ef0
PKRU: 5554
Call Trace:
amdgpu_bo_move_notify+0x3a/0xf0 [amdgpu
84c82d766599797bed2ef6971fa457123a4823ba]
ttm_bo_delayed_delete+0x59/0xd0 [ttm d0d6b8ddf810a50c01887c0fcb83d6ad65d08ff1]
process_one_work+0x17b/0x340
worker_thread+0x301/0x490
kthread+0xe8/0x120
ret_from_fork+0x34/0x50
ret_from_fork_asm+0x1b/0x30
```
Thank you.
> Regards,
> Christian.
>
> >
> > ;)
> >
> > Regards,
> >
> > Tvrtko
> >
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 15 +++
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 4 +-
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 48 --
> >> 3 files changed, 37 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> index 425cebcc5cbf..eb7d824763b9 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> @@ -1245,19 +1245,20 @@ int amdgpu_bo_get_metadata(struct amdgpu_bo
> >> *bo, void *buffer,
> >>* amdgpu_bo_move_notify - notification about a memory move
> >>* @bo: pointer to a buffer object
> >>* @evict: if this move is evicting the buffer from the graphics
> >> address space
> >> + * @new_mem: new resource for backing the BO
> >>*
> >>* Marks the corresponding _bo buffer object as invalid,
> >> also performs
> >>* bookkeeping.
> >>* TTM driver callback which is called when ttm moves a buffer.
> >>*/
> >> -void amdgpu_bo_move_notify(struct ttm_buffer_object *bo, bool evict)
> >> +void amdgpu_bo_move_notify(struct ttm_buffer_object *bo,
> >> + bool evict,
> >> + struct ttm_resource *new_mem)
> >> {
> >> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev);
> >> +struct ttm_resource *old_mem = bo->resource;
> >> struct amdgpu_bo *abo;
> >> -if (!amdgpu_bo_is_amdgpu_bo(bo))
> >> -return;
> >> -
> >> abo = ttm_to_amdgpu_bo(bo);
> >> amdgpu_vm_bo_invalidate(adev, abo, evict);
> >> @@ -1267,9 +1268,9 @@ void amdgpu_bo_move_notify(struct
> >> ttm_buffer_object *bo, bool evict)
> >> bo->resource->mem_type != TTM_PL_SYSTEM)
> >> dma_buf_move_notify(abo->tbo.base.dma_buf);
> >> -/* remember the eviction */
> >> -if (evict)
> >> -atomic64_inc(>num_evictions);
> >> +/* move_notify is called before move happens */
> >> +trace_amdgpu_bo_move(abo, new_mem ?
[PATCH -next] drm/amd/display: delete the redundant initialization in dcn3_51_soc
the dram_clock_change_latency_us in dcn3_51_soc is initialized twice, so
delete one of them.
Signed-off-by: Xiang Yang
---
drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
b/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
index b3ffab77cf88..f1c0d5b77f1b 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
@@ -237,7 +237,6 @@ struct _vcs_dpi_soc_bounding_box_st dcn3_51_soc = {
.urgent_latency_adjustment_fabric_clock_component_us = 0,
.urgent_latency_adjustment_fabric_clock_reference_mhz = 0,
.num_chans = 4,
- .dram_clock_change_latency_us = 11.72,
.dispclk_dppclk_vco_speed_mhz = 2400.0,
};
--
2.34.1
[bug report] drm/amd/display: Allow Z8 when stutter threshold is not met
Hello Bhawanpreet Lakha,
This is a semi-automatic email about new static checker warnings.
Commit e9a09a198bfe ("drm/amd/display: Allow Z8 when stutter
threshold is not met") from Mar 13, 2024, leads to the following
Smatch complaint:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c:1088
decide_zstate_support()
warn: variable dereferenced before check 'link' (see line 1087)
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/dcn20_fpu.c
1086 bool allow_z8 = context->bw_ctx.dml.vba.StutterPeriod >
(double)minmum_z8_residency;
1087 bool is_pwrseq0 = link->link_index == 0;
The existing code assumes link isn't NULL
1088 bool is_psr = (link && (link->psr_settings.psr_version
== DC_PSR_VERSION_1 ||
1089
link->psr_settings.psr_version == DC_PSR_VERSION_SU_1) &&
!link->panel_config.psr.disable_psr);
1090 bool is_replay = link &&
link->replay_settings.replay_feature_enabled;
but the patch assumes link can be NULL. Somebody is wrong.
regards,
dan carpenter
Re: [PATCH v0 00/14] Make I2C terminology more inclusive for I2C Algobit and consumers
Hi Wolfram, On 4/5/2024 3:18 AM, Wolfram Sang wrote: > Hello Easwar, > > On Fri, Mar 29, 2024 at 05:00:24PM +, Easwar Hariharan wrote: >> I2C v7, SMBus 3.2, and I3C specifications have replaced "master/slave" >> with more appropriate terms. Inspired by and following on to Wolfram's >> series to fix drivers/i2c/[1], fix the terminology for users of the >> I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists >> in the specification. > > I really appreciate that you want to assist in this task to improve the > I2C core. I do. I am afraid, however, that you took the second step > before the first one, though. As I mentioned in my original cover > letter, this is not only about renaming but also improving the I2C API > (splitting up header files...). So, drivers are not a priority right > now. They can be better fixed once the core is ready. > Sorry, got excited. :) There were drivers I'd been part of that I specifically wanted to fixup, but then the scope grew to other users of algobit. > It is true that I changed quite some controller drivers within the i2c > realm. I did this to gain experience. As you also noticed quite some > questions came up. We need to agree on answers first. And once we are > happy with the answers we found, then IMO we can go outside of the i2c > realm and send patches to other subsystems referencing agreed > precedence. I intentionally did not go outside i2c yet. Since your > patches are already there, you probably want to foster them until they > are ready for inclusion. Sorry, I don't quite follow what you mean by foster in this context. Are you asking me to hold off on merging the series, or to follow through on getting it merged? Yet, regarding further patches, my suggestion > is to wait until the core is ready. That might take a while, though. > However, there is enough to discuss until the core is ready. So, your > collaboration there is highly appreciated! > >> The last patch updating the .master_xfer method to .xfer depends on >> patch 1 of Wolfram's series below, but the series is otherwise >> independent. It may make sense for the last patch to go in with > > Please drop the last patch from this series. It will nicely remove the > dependency. Also, like above, I first want to gain experience with i2c > before going to other subsystems. That was intended. > Will do, thanks! > All the best and happy hacking, > >Wolfram >
[PATCH -next] drm/amd/display: delete the redundant initialization in dcn3_51_soc
the dram_clock_change_latency_us in dcn3_51_soc is initialized twice, so
delete one of them.
Signed-off-by: Xiang Yang
---
drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
b/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
index b3ffab77cf88..f1c0d5b77f1b 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn351/dcn351_fpu.c
@@ -237,7 +237,6 @@ struct _vcs_dpi_soc_bounding_box_st dcn3_51_soc = {
.urgent_latency_adjustment_fabric_clock_component_us = 0,
.urgent_latency_adjustment_fabric_clock_reference_mhz = 0,
.num_chans = 4,
- .dram_clock_change_latency_us = 11.72,
.dispclk_dppclk_vco_speed_mhz = 2400.0,
};
--
2.34.1
Re: [PATCH v0 13/14] drm/nouveau: Make I2C terminology more inclusive
On 4/5/2024 9:15 AM, Danilo Krummrich wrote:
> Hi Easwar,
>
> On 3/29/24 18:00, Easwar Hariharan wrote:
>> I2C v7, SMBus 3.2, and I3C specifications have replaced "master/slave"
>> with more appropriate terms. Inspired by and following on to Wolfram's
>> series to fix drivers/i2c/[1], fix the terminology for users of
>> I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists
>> in the specification.
>>
>> Compile tested, no functionality changes intended
>>
>> [1]:
>> https://lore.kernel.org/all/20240322132619.6389-1-wsa+rene...@sang-engineering.com/
>>
>> Signed-off-by: Easwar Hariharan
>> ---
>> drivers/gpu/drm/nouveau/dispnv04/dfp.c | 14 +++---
>> .../gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h | 2 +-
>> drivers/gpu/drm/nouveau/nouveau_bios.c | 4 ++--
>> 3 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
>> b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
>> index d5b129dc623b..65b791006b19 100644
>> --- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
>> +++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
>> @@ -149,7 +149,7 @@ void nv04_dfp_update_fp_control(struct drm_encoder
>> *encoder, int mode)
>> }
>> }
>> -static struct drm_encoder *get_tmds_slave(struct drm_encoder *encoder)
>> +static struct drm_encoder *get_tmds_client(struct drm_encoder *encoder)
>> {
>> struct drm_device *dev = encoder->dev;
>> struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
>> @@ -172,7 +172,7 @@ static struct drm_encoder *get_tmds_slave(struct
>> drm_encoder *encoder)
>> struct dcb_output *slave_dcb = nouveau_encoder(slave)->dcb;
>> if (slave_dcb->type == DCB_OUTPUT_TMDS && get_slave_funcs(slave)
>> &&
>> - slave_dcb->tmdsconf.slave_addr == dcb->tmdsconf.slave_addr)
>> + slave_dcb->tmdsconf.client_addr == dcb->tmdsconf.client_addr)
>> return slave;
>
> While, personally, I think master/slave was well suiting for the device
> relationship
> on those busses, I think that if we change it up to conform with the change in
> specification, we should make sure to update drivers consistently.
>
> We should make sure to also change the names of the sourrounding structures
> and variable
> names, otherwise we just make this code harder to read.
>
> - Danilo
>
Thanks for the review, and for the appetite to go further! So we are on the
same page, you would prefer
renaming to controller/target like the feedback on other drm drivers (i915,
gma500, radeon)?
Thanks,
Easwar
Re: [PATCH v0 00/14] Make I2C terminology more inclusive for I2C Algobit and consumers
Hello Easwar, On Fri, Mar 29, 2024 at 05:00:24PM +, Easwar Hariharan wrote: > I2C v7, SMBus 3.2, and I3C specifications have replaced "master/slave" > with more appropriate terms. Inspired by and following on to Wolfram's > series to fix drivers/i2c/[1], fix the terminology for users of the > I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists > in the specification. I really appreciate that you want to assist in this task to improve the I2C core. I do. I am afraid, however, that you took the second step before the first one, though. As I mentioned in my original cover letter, this is not only about renaming but also improving the I2C API (splitting up header files...). So, drivers are not a priority right now. They can be better fixed once the core is ready. It is true that I changed quite some controller drivers within the i2c realm. I did this to gain experience. As you also noticed quite some questions came up. We need to agree on answers first. And once we are happy with the answers we found, then IMO we can go outside of the i2c realm and send patches to other subsystems referencing agreed precedence. I intentionally did not go outside i2c yet. Since your patches are already there, you probably want to foster them until they are ready for inclusion. Yet, regarding further patches, my suggestion is to wait until the core is ready. That might take a while, though. However, there is enough to discuss until the core is ready. So, your collaboration there is highly appreciated! > The last patch updating the .master_xfer method to .xfer depends on > patch 1 of Wolfram's series below, but the series is otherwise > independent. It may make sense for the last patch to go in with Please drop the last patch from this series. It will nicely remove the dependency. Also, like above, I first want to gain experience with i2c before going to other subsystems. That was intended. All the best and happy hacking, Wolfram signature.asc Description: PGP signature
Re: [linux-next:master] BUILD REGRESSION 8568bb2ccc278f344e6ac44af6ed010a90aa88dc
On Fri, Apr 5, 2024 at 8:37 AM kernel test robot wrote: > > tree/branch: > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > branch HEAD: 8568bb2ccc278f344e6ac44af6ed010a90aa88dc Add linux-next > specific files for 20240405 > > Error/Warning reports: > > https://lore.kernel.org/oe-kbuild-all/202404051333.7und7ppw-...@intel.com > https://lore.kernel.org/oe-kbuild-all/202404051423.eiaxlwhx-...@intel.com > https://lore.kernel.org/oe-kbuild-all/202404051659.aawukguq-...@intel.com > https://lore.kernel.org/oe-kbuild-all/202404052022.cwf2ilbp-...@intel.com > > Error/Warning: (recently discovered and may have been fixed) > > aarch64-linux-ld: kernel/bpf/verifier.c:20223:(.text+0xdbb4): undefined > reference to `__SCK__perf_snapshot_branch_stack' > aarch64-linux-ld: verifier.c:(.text+0x17c3c): undefined reference to > `__SCK__perf_snapshot_branch_stack' > drivers/i2c/busses/i2c-i801.c:1407:(.text+0x1d2ef4a): undefined reference to > `i2c_root_adapter' > kernel/bpf/verifier.c:20223:(.text+0xdba4): dangerous relocation: unsupported > relocation > loongarch64-linux-ld: kernel/bpf/verifier.c:20223:(.text+0xa818): undefined > reference to `__SCK__perf_snapshot_branch_stack' > loongarch64-linux-ld: verifier.c:(.text+0xa964): undefined reference to > `__SCK__perf_snapshot_branch_stack' > mips64el-linux-ld: verifier.c:(.text.do_misc_fixups+0xd9c): undefined > reference to `__SCK__perf_snapshot_branch_stack' > riscv32-linux-ld: section .data LMA [00369000,00907967] overlaps section > .text LMA [0007899c,01a6a6af] > s390-linux-ld: verifier.c:(.text+0x13038): undefined reference to > `__SCK__perf_snapshot_branch_stack' > verifier.c:(.text+0x17c14): relocation truncated to fit: > R_AARCH64_ADR_PREL_PG_HI21 against undefined symbol > `__SCK__perf_snapshot_branch_stack' > verifier.c:(.text+0xa960): undefined reference to > `__SCK__perf_snapshot_branch_stack' > verifier.c:(.text+0xadd0): dangerous relocation: unsupported relocation > verifier.c:(.text.do_misc_fixups+0xd98): undefined reference to > `__SCK__perf_snapshot_branch_stack' Fixed in bpf-next with commit: https://lore.kernel.org/all/20240405142637.577046-1-a...@kernel.org/
RE: [PATCH] drm/amdgpu: differentiate exteranl rev id for gfx 11.5.0
[Public] Hi Yifan, -Original Message- From: Zhang, Yifan Sent: Sunday, April 7, 2024 10:10 PM To: amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander ; Huang, Tim ; Ma, Li ; Zhang, Yifan Subject: [PATCH] drm/amdgpu: differentiate exteranl rev id for gfx 11.5.0 > This patch to differentiate exteranl rev id for gfx 11.5.0. With the typo " exteranl " fixed, this patch is Reviewed-by: Tim Huang Signed-off-by: Yifan Zhang --- drivers/gpu/drm/amd/amdgpu/soc21.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c b/drivers/gpu/drm/amd/amdgpu/soc21.c index abe319b0f063..43ca63fe85ac 100644 --- a/drivers/gpu/drm/amd/amdgpu/soc21.c +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c @@ -720,7 +720,10 @@ static int soc21_common_early_init(void *handle) AMD_PG_SUPPORT_VCN | AMD_PG_SUPPORT_JPEG | AMD_PG_SUPPORT_GFX_PG; - adev->external_rev_id = adev->rev_id + 0x1; + if (adev->rev_id == 0) + adev->external_rev_id = 0x1; + else + adev->external_rev_id = adev->rev_id + 0x10; break; case IP_VERSION(11, 5, 1): adev->cg_flags = -- 2.37.3
RE: [PATCH] drm/amdgpu: Process bif doorbell event
[AMD Official Use Only - General]
Reviewed-by: Kenneth Feng
-Original Message-
From: Zhang, Hawking
Sent: Monday, April 1, 2024 7:15 PM
To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
; Feng, Kenneth ; Xiao, Jack
Cc: Zhang, Hawking
Subject: [PATCH] drm/amdgpu: Process bif doorbell event
When BACO exit is triggered by doorbell transaction, firmware will config bif
to issue msi interrupt to indicate doorbell transaction. If bif ring is not
enabled in such case, driver needs to ack the interrupt by clearing the
interrupt status.
Signed-off-by: Hawking Zhang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.h | 2 +
drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c | 56
drivers/gpu/drm/amd/amdgpu/soc21.c | 14 +-
3 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.h
index 7b8c03be1d9e..db341921cfc1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_nbio.h
@@ -102,6 +102,7 @@ struct amdgpu_nbio_funcs {
u32 (*get_memory_partition_mode)(struct amdgpu_device *adev,
u32 *supp_modes);
u64 (*get_pcie_replay_count)(struct amdgpu_device *adev);
+ u32 (*init_bif_doorbell_event)(struct amdgpu_device *adev);
};
struct amdgpu_nbio {
@@ -111,6 +112,7 @@ struct amdgpu_nbio {
struct ras_common_if *ras_if;
const struct amdgpu_nbio_funcs *funcs;
struct amdgpu_nbio_ras *ras;
+ struct amdgpu_irq_src bif_doorbell_irq;
};
int amdgpu_nbio_ras_sw_init(struct amdgpu_device *adev); diff --git
a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
index 7f88a298ac5f..e5a331b6eee9 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v4_3.c
@@ -477,6 +477,61 @@ static void nbio_v4_3_program_aspm(struct amdgpu_device
*adev) #endif }
+static int nbio_v4_3_set_bif_doorbell_irq_state(struct amdgpu_device *adev,
+ struct amdgpu_irq_src *src,
+ unsigned type,
+ enum amdgpu_interrupt_state
state) {
+/*let firmware to config bif doorbell irq state */
+return 0;
+}
+
+static int nbio_v4_3_process_bif_doorbell_irq(struct amdgpu_device *adev,
+ struct amdgpu_irq_src *source,
+ struct amdgpu_iv_entry *entry) {
+ /* pmfw will config bif doorbell irq to host if baco exit
+* is triggered by doorbell transaction. In such case, driver
+* needs to clear the interrupt status */
+
+ uint32_t reg_data;
+
+ reg_data = RREG32_SOC15(NBIO, 0, regBIF_BX0_BIF_RB_CNTL);
+
+ /* if bif ring is enabled, do nothing */
+ if (REG_GET_FIELD(reg_data, BIF_BX0_BIF_RB_CNTL, RB_ENABLE))
+ return 0;
+
+ /* write 1 to clear doorbell interrupt */
+ reg_data = RREG32_SOC15(NBIO, 0, regBIF_BX0_BIF_DOORBELL_INT_CNTL);
+ if (REG_GET_FIELD(reg_data, BIF_BX0_BIF_DOORBELL_INT_CNTL,
DOORBELL_INTERRUPT_STATUS)) {
+ reg_data = REG_SET_FIELD(reg_data,
+BIF_BX0_BIF_DOORBELL_INT_CNTL,
+DOORBELL_INTERRUPT_CLEAR, 1);
+ WREG32_SOC15(NBIO, 0, regBIF_BX0_BIF_DOORBELL_INT_CNTL,
reg_data);
+ }
+
+ return 0;
+}
+
+static const struct amdgpu_irq_src_funcs nbio_v4_3_bif_doorbell_irq_funcs = {
+ .set = nbio_v4_3_set_bif_doorbell_irq_state,
+ .process = nbio_v4_3_process_bif_doorbell_irq,
+};
+
+static u32 nbio_v4_3_init_bif_doorbell_event(struct amdgpu_device
+*adev) {
+ u32 r;
+
+ adev->nbio.bif_doorbell_irq.funcs = _v4_3_bif_doorbell_irq_funcs;
+ adev->nbio.bif_doorbell_irq.num_types = 1;
+
+ r = amdgpu_irq_add_id(adev, SOC21_IH_CLIENTID_BIF,
+ NBIF_7_4__SRCID__DOORBELL_INTERRUPT,
+ >nbio.bif_doorbell_irq);
+ return r;
+}
+
const struct amdgpu_nbio_funcs nbio_v4_3_funcs = {
.get_hdp_flush_req_offset = nbio_v4_3_get_hdp_flush_req_offset,
.get_hdp_flush_done_offset = nbio_v4_3_get_hdp_flush_done_offset,
@@ -499,6 +554,7 @@ const struct amdgpu_nbio_funcs nbio_v4_3_funcs = {
.remap_hdp_registers = nbio_v4_3_remap_hdp_registers,
.get_rom_offset = nbio_v4_3_get_rom_offset,
.program_aspm = nbio_v4_3_program_aspm,
+ .init_bif_doorbell_event = nbio_v4_3_init_bif_doorbell_event,
};
diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c
b/drivers/gpu/drm/amd/amdgpu/soc21.c
index abe319b0f063..ee6d810589c0 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc21.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
@@ -792,6 +792,9 @@ static int soc21_common_late_init(void *handle)
*
