Re: [PATCH v9 4/5] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors v5
Am 27.11.2017 um 19:30 schrieb Boris Ostrovsky: On 11/23/2017 09:12 AM, Boris Ostrovsky wrote: On 11/23/2017 03:11 AM, Christian König wrote: Am 22.11.2017 um 18:27 schrieb Boris Ostrovsky: On 11/22/2017 11:54 AM, Christian König wrote: Am 22.11.2017 um 17:24 schrieb Boris Ostrovsky: On 11/22/2017 05:09 AM, Christian König wrote: Am 21.11.2017 um 23:26 schrieb Boris Ostrovsky: On 11/21/2017 08:34 AM, Christian König wrote: Hi Boris, attached are two patches. The first one is a trivial fix for the infinite loop issue, it now correctly aborts the fixup when it can't find address space for the root window. The second is a workaround for your board. It simply checks if there is exactly one Processor Function to apply this fix on. Both are based on linus current master branch. Please test if they fix your issue. Yes, they do fix it but that's because the feature is disabled. Do you know what the actual problem was (on Xen)? I still haven't understood what you actually did with Xen. When you used PCI pass through with those devices then you have made a major configuration error. When the problem happened on dom0 then the explanation is most likely that some PCI device ended up in the configured space, but the routing was only setup correctly on one CPU socket. The problem is that dom0 can be (and was in my case() booted with less than full physical memory and so the "rest" of the host memory is not necessarily reflected in iomem. Your patch then tried to configure that memory for MMIO and the system hang. And so my guess is that this patch will break dom0 on a single-socket system as well. Oh, thanks! I've thought about that possibility before, but wasn't able to find a system which actually does that. May I ask why the rest of the memory isn't reported to the OS? That memory doesn't belong to the OS (dom0), it is owned by the hypervisor. Sounds like I can't trust Linux resource management and probably need to read the DRAM config to figure things out after all. My question is whether what you are trying to do should ever be done for a guest at all (any guest, not necessarily Xen). The issue is probably that I don't know enough about Xen: What exactly is dom0? My understanding was that dom0 is the hypervisor, but that seems to be incorrect. The issue is that under no circumstances *EVER* a virtualized guest should have access to the PCI devices marked as "Processor Function" on AMD platforms. Otherwise it is trivial to break out of the virtualization. When dom0 is something like the system domain with all hardware access then the approach seems legitimate, but then the hypervisor should report the stolen memory to the OS using the e820 table. When the hypervisor doesn't do that and the Linux kernel isn't aware that there is memory at a given location mapping PCI space there will obviously crash the hypervisor. Possible solutions as far as I can see are either disabling this feature when we detect that we are a Xen dom0, scanning the DRAM settings to update Linux resource handling or fixing Xen to report stolen memory to the dom0 OS as reserved. Opinions? You are right, these functions are not exposed to a regular guest. I think for dom0 (which is a special Xen guest, with additional privileges) we may be able to add a reserved e820 region for host memory that is not assigned to dom0. Let me try it on Monday (I am out until then). One thing I realized while looking at solution for Xen dom0 is that this patch may cause problems for memory hotplug. Good point. My assumption was that when you got an BIOS which can handle memory hotplug then you also got an BIOS which doesn't need this fixup. But I've never validated that assumption. What happens if new memory is added to the system and we have everything above current memory set to MMIO? In theory the BIOS would search for address space and won't find anything, so the hotplug operation should fail even before it reaches the kernel in the first place. In practice I think that nobody ever tested if that works correctly. So I'm pretty sure the system would just crash. How about the attached patch? It limits the newly added MMIO space to the upper 256GB of the address space. That should still be enough for most devices, but we avoid both issues with Xen dom0 as most likely problems with memory hotplug as well. Christian. -boris >From 586bd9d67ebb6ca48bd0a6b1bd9203e94093cc8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Tue, 28 Nov 2017 10:02:35 +0100 Subject: [PATCH] x86/PCI: limit the size of the 64bit BAR to 256GB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This avoids problems with Xen which hides some memory resources from the OS and potentially also allows memory hotplug while this fixup is enabled. Signed-off-by: Christian König --- arch/x86/pci/fixup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ar
Re: [PATCH 1/3] drm/amdgpu: Add SOC15 IP offset define file
Am 27.11.2017 um 23:30 schrieb Tom St Denis: On 27/11/17 04:28 PM, Christian König wrote: Am 27.11.2017 um 21:56 schrieb Alex Deucher: On Mon, Nov 27, 2017 at 3:44 PM, Christian König wrote: Am 27.11.2017 um 21:01 schrieb Felix Kuehling: On 2017-11-27 02:37 PM, Koenig, Christian wrote: And that is a clear NAK to this approach. Hi Christian, Do you have other objections than the style issues? If so, please explain. No, the technical aspect actually looks rather reasonable. Please clarify, why this file needs to be treated differently from other files under include/asic_reg? All those files are auto-generated by HW teams. Fixing the coding style adds no value and makes future updates more complicated. We already got complains about that and most likely will need to fix the rest as well. I'd like to stay as close as possible to the headers formats we are using internally across teams for consistency. To be honest I strongly disagree on that. The bad quality of the internal AMD headers is the reason we had to basically have the VMHUB code for Vega10 twice for example. At the very least the globals we use per ip block should be version specific. That way if you cscope/ctags around you can find the actual references and not collisions. Yeah, completely agree on that. Regards, Christian. Tom ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/amdgpu: set gtt size according to system memory size only
Change-Id: Ib634375b90d875fe04a890fc82fb1e3b7112676a
Signed-off-by: Roger He
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 17bf0ce..d773c5e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1328,13 +1328,19 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
if (amdgpu_gtt_size == -1) {
struct sysinfo si;
+ uint64_t sys_mem_size;
si_meminfo(&si);
- gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
- adev->mc.mc_vram_size),
- ((uint64_t)si.totalram * si.mem_unit * 3/4));
- }
- else
+ sys_mem_size = (uint64_t)si.totalram * si.mem_unit;
+ gtt_size = AMDGPU_DEFAULT_GTT_SIZE_MB << 20;
+
+ /* leave 2GB for OS to work with */
+ if (sys_mem_size > (2ULL << 30)) {
+ gtt_size = max(gtt_size, sys_mem_size - (2ULL << 30));
+ } else
+ DRM_INFO("amdgpu: Too small system memory %llu MB\n",
+ sys_mem_size >> 20);
+ } else
gtt_size = (uint64_t)amdgpu_gtt_size << 20;
r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> PAGE_SHIFT);
if (r) {
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 10/14] drm/amdkfd: Use ref count to prevent kfd_process destruction
Am 28.11.2017 um 00:29 schrieb Felix Kuehling:
Use a reference counter instead of a lock to prevent process
destruction while functions running out of process context are using
the kfd_process structure. In many cases these functions don't need
the structure to be locked. In the few cases that really do need the
process lock, take it explicitly.
This helps simplify lock dependencies between the process lock and
other locks, particularly amdgpu and mm_struct locks. This will be
important when amdgpu calls back to amdkfd for memory evictions.
Actually that is not only an optimization or cleanup, but a rather
important bug fix.
Using a mutex as protection to prevent object deletion is illegal
because mutex_unlock() can accesses the mutex object even after it is
unlocked.
See this LWN article as well https://lwn.net/Articles/575460/.
If you have other use cases like this in the KFD it should better be
fixed as well.
Signed-off-by: Felix Kuehling
Acked-by: Christian König
Regards,
Christian.
---
drivers/gpu/drm/amd/amdkfd/kfd_events.c | 14 +++---
drivers/gpu/drm/amd/amdkfd/kfd_priv.h| 1 +
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 16 +---
3 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
index cb92d4b..93aae5c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
@@ -441,7 +441,7 @@ void kfd_signal_event_interrupt(unsigned int pasid,
uint32_t partial_id,
/*
* Because we are called from arbitrary context (workqueue) as opposed
* to process context, kfd_process could attempt to exit while we are
-* running so the lookup function returns a locked process.
+* running so the lookup function increments the process ref count.
*/
struct kfd_process *p = kfd_lookup_process_by_pasid(pasid);
@@ -493,7 +493,7 @@ void kfd_signal_event_interrupt(unsigned int pasid, uint32_t partial_id,
}
mutex_unlock(&p->event_mutex);
- mutex_unlock(&p->mutex);
+ kfd_unref_process(p);
}
static struct kfd_event_waiter *alloc_event_waiters(uint32_t num_events)
@@ -847,7 +847,7 @@ void kfd_signal_iommu_event(struct kfd_dev *dev, unsigned
int pasid,
/*
* Because we are called from arbitrary context (workqueue) as opposed
* to process context, kfd_process could attempt to exit while we are
-* running so the lookup function returns a locked process.
+* running so the lookup function increments the process ref count.
*/
struct kfd_process *p = kfd_lookup_process_by_pasid(pasid);
struct mm_struct *mm;
@@ -860,7 +860,7 @@ void kfd_signal_iommu_event(struct kfd_dev *dev, unsigned
int pasid,
*/
mm = get_task_mm(p->lead_thread);
if (!mm) {
- mutex_unlock(&p->mutex);
+ kfd_unref_process(p);
return; /* Process is exiting */
}
@@ -903,7 +903,7 @@ void kfd_signal_iommu_event(struct kfd_dev *dev, unsigned int pasid,
&memory_exception_data);
mutex_unlock(&p->event_mutex);
- mutex_unlock(&p->mutex);
+ kfd_unref_process(p);
}
void kfd_signal_hw_exception_event(unsigned int pasid)
@@ -911,7 +911,7 @@ void kfd_signal_hw_exception_event(unsigned int pasid)
/*
* Because we are called from arbitrary context (workqueue) as opposed
* to process context, kfd_process could attempt to exit while we are
-* running so the lookup function returns a locked process.
+* running so the lookup function increments the process ref count.
*/
struct kfd_process *p = kfd_lookup_process_by_pasid(pasid);
@@ -924,5 +924,5 @@ void kfd_signal_hw_exception_event(unsigned int pasid)
lookup_events_by_type_and_signal(p, KFD_EVENT_TYPE_HW_EXCEPTION, NULL);
mutex_unlock(&p->event_mutex);
- mutex_unlock(&p->mutex);
+ kfd_unref_process(p);
}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 248e4f5..0c96a6b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -606,6 +606,7 @@ void kfd_process_destroy_wq(void);
struct kfd_process *kfd_create_process(struct file *filep);
struct kfd_process *kfd_get_process(const struct task_struct *);
struct kfd_process *kfd_lookup_process_by_pasid(unsigned int pasid);
+void kfd_unref_process(struct kfd_process *p);
struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev,
struct kfd_process *p);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index e02e8a2..509f987 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@
Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only
Am 28.11.2017 um 10:40 schrieb Roger He:
Change-Id: Ib634375b90d875fe04a890fc82fb1e3b7112676a
Signed-off-by: Roger He
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 17bf0ce..d773c5e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1328,13 +1328,19 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
if (amdgpu_gtt_size == -1) {
struct sysinfo si;
+ uint64_t sys_mem_size;
si_meminfo(&si);
- gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
- adev->mc.mc_vram_size),
- ((uint64_t)si.totalram * si.mem_unit * 3/4));
- }
- else
+ sys_mem_size = (uint64_t)si.totalram * si.mem_unit;
+ gtt_size = AMDGPU_DEFAULT_GTT_SIZE_MB << 20;
+
+ /* leave 2GB for OS to work with */
+ if (sys_mem_size > (2ULL << 30)) {
+ gtt_size = max(gtt_size, sys_mem_size - (2ULL << 30));
+ } else
No need for the "{}" here.
+ DRM_INFO("amdgpu: Too small system memory %llu MB\n",
+ sys_mem_size >> 20);
+ } else
I have a preference to stick with the 75% rule similar to how TTM does
things, but that isn't a hard opinion if you have a good argument.
Regards,
Christian.
gtt_size = (uint64_t)amdgpu_gtt_size << 20;
r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> PAGE_SHIFT);
if (r) {
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/7] drm/amdgpu: fix VM PD addr shift
Am 28.11.2017 um 03:42 schrieb Chunming Zhou:
On 2017年11月28日 00:02, Christian König wrote:
The block size only affects the leave nodes, everything else is fixed.
This is not true, block size affects every level entries, see the
register explains for VM_CONTEXTx_CNTL.PAGE_TABLE_BLOCK_SIZE:
"LOG2(number of 2MB logical address ranges) pointed to by a PDE0 page
text directory entry. The native page size for the component ptes
comes from the PDE0.block_fragment_size field or
BASE_ADDR.block_fragment_size field (for a flat page table). The
PAGE_TABLE_BLOCK_SIZE field only has an effect on address calculations
for >= 2 level page tables however. A flat page table will utilize as
many ptes as are required to represent the logical address between
START_ADDR and END_ADDR, not limited by PAGE_TABLE_BLOCK_SIZE."
The description of the register is incorrect or at least misleading.
I've fallen into the same trap as well.
Take a look at the VM documentation, there is a small footnote that
intermediate page directories are always 512 entries in size.
I've confirmed that by configuring a Vega10 into 3 level page directory
instead of the usual 4 and then using a block size of 18 for the page
tables.
Regards,
Christian.
Regards,
David Zhou
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 28
+++-
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 122379dfc7d8..f1e541e9b514 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -139,6 +139,24 @@ struct amdgpu_prt_cb {
};
/**
+ * amdgpu_vm_level_shift - return the addr shift for each level
+ *
+ * @adev: amdgpu_device pointer
+ *
+ * Returns the number of bits the pfn needs to be right shifted for
a level.
+ */
+static unsigned amdgpu_vm_level_shift(struct amdgpu_device *adev,
+ unsigned level)
+{
+ if (level != adev->vm_manager.num_level)
+ return 9 * (adev->vm_manager.num_level - level - 1) +
+ adev->vm_manager.block_size;
+ else
+ /* For the page tables on the leaves */
+ return 0;
+}
+
+/**
* amdgpu_vm_num_entries - return the number of entries in a PD/PT
*
* @adev: amdgpu_device pointer
@@ -288,8 +306,7 @@ static int amdgpu_vm_alloc_levels(struct
amdgpu_device *adev,
uint64_t saddr, uint64_t eaddr,
unsigned level)
{
- unsigned shift = (adev->vm_manager.num_level - level) *
- adev->vm_manager.block_size;
+ unsigned shift = amdgpu_vm_level_shift(adev, level);
unsigned pt_idx, from, to;
int r;
u64 flags;
@@ -1302,18 +1319,19 @@ void amdgpu_vm_get_entry(struct
amdgpu_pte_update_params *p, uint64_t addr,
struct amdgpu_vm_pt **entry,
struct amdgpu_vm_pt **parent)
{
- unsigned idx, level = p->adev->vm_manager.num_level;
+ unsigned level = 0;
*parent = NULL;
*entry = &p->vm->root;
while ((*entry)->entries) {
- idx = addr >> (p->adev->vm_manager.block_size * level--);
+ unsigned idx = addr >> amdgpu_vm_level_shift(p->adev, level++);
+
idx %= amdgpu_bo_size((*entry)->base.bo) / 8;
*parent = *entry;
*entry = &(*entry)->entries[idx];
}
- if (level)
+ if (level != p->adev->vm_manager.num_level)
*entry = NULL;
}
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Xen-devel] [PATCH v9 4/5] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors v5
Am 28.11.2017 um 10:46 schrieb Jan Beulich: On 28.11.17 at 10:12, wrote: In theory the BIOS would search for address space and won't find anything, so the hotplug operation should fail even before it reaches the kernel in the first place. How would the BIOS know what the OS does or plans to do? As far as I know the ACPI BIOS should work directly with the register content. So when we update the register content to enable the MMIO decoding the BIOS should know that as well. I think it's the other way around - the OS needs to avoid using any regions for MMIO which are marked as hotpluggable in SRAT. I was under the impression that this is exactly what acpi_numa_memory_affinity_init() does. Since there is no vNUMA yet for Xen Dom0, that would need special handling. I think that the problem is rather that SRAT is NUMA specific and if I'm not totally mistaken the content is ignored when NUMA support isn't compiled into the kernel. When Xen steals some memory from Dom0 by hocking up itself into the e820 call then I would say the cleanest way is to report this memory in e820 as reserved as well. But take that with a grain of salt, I'm seriously not a Xen expert. Regards, Christian. Jan ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 02/13] fbdev: add remove_conflicting_pci_framebuffers()
On Mon, Nov 27, 2017 at 08:52:19PM +, Sudip Mukherjee wrote: > On Mon, Nov 27, 2017 at 11:27:59AM +0100, Daniel Vetter wrote: > > On Fri, Nov 24, 2017 at 06:53:31PM +0100, Michał Mirosław wrote: > > > Almost all drivers using remove_conflicting_framebuffers() wrap it with > > > the same code. Extract common part from PCI drivers into separate > > > remove_conflicting_pci_framebuffers(). > > > > > > Signed-off-by: Michał Mirosław > > > > Since the only driver that seems to use this is the staging one, which imo > > is a DOA project, not sure it's worth to bother with this here. > > afaik, this device is used in production by few manufacturers and it is > usefull for them to have it in the tree and the only reason it is still > in staging is because Tomi announced he will not take any new drivers in > fbdev. And, so I have not taken the initiative to properly move it out > of staging. I think Bartlomiej will also not accept new drivers in fbdev. Imo, if no one cares about porting it to kms (which will give you an fbdev driver for free), then we should throw it out. At least I thought staging was only for stuff that had a reasonable chance to get mainlined. Not as a dumping ground for drivers that people use, but don't bother to get ready for mainline. Greg? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 2/2] drm/amd/display: Don't call dm_log_to_buffer directly in dc_conn_log
Ping on this series.
This patch is v2 of a previously single patch, which was reviewed by Harry.
On 2017-11-23 06:48 PM, Michel Dänzer wrote:
> From: Michel Dänzer
>
> dm_log_to_buffer logs unconditionally, so calling it directly resulted
> in the main message being logged even when the event type isn't enabled
> in the event mask.
>
> To fix this, use the new dm_logger_append_va API.
>
> Fixes spurious messages like
>
> [drm] {1920x1200, 2080x1235@154000Khz}
>
> in dmesg when a mode is set.
>
> v2:
> * Use new dm_logger_append_va API, fixes incorrect va_list usage in v1
> * Just use and decrease entry.buf_offset to get rid of the trailing
> newline
>
> Signed-off-by: Michel Dänzer
> ---
> drivers/gpu/drm/amd/display/dc/basics/log_helpers.c | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
> b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
> index 785b943b60ed..fe1648f81d71 100644
> --- a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
> +++ b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
> @@ -80,15 +80,11 @@ void dc_conn_log(struct dc_context *ctx,
> link->link_index);
>
> va_start(args, msg);
> - entry.buf_offset += dm_log_to_buffer(
> - &entry.buf[entry.buf_offset],
> - LOG_MAX_LINE_SIZE - entry.buf_offset,
> - msg, args);
> + dm_logger_append_va(&entry, msg, args);
>
> - if (entry.buf[strlen(entry.buf) - 1] == '\n') {
> - entry.buf[strlen(entry.buf) - 1] = '\0';
> + if (entry.buf_offset > 0 &&
> + entry.buf[entry.buf_offset - 1] == '\n')
> entry.buf_offset--;
> - }
>
> if (hex_data)
> for (i = 0; i < hex_data_count; i++)
>
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Xen-devel] [PATCH v9 4/5] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors v5
Am 28.11.2017 um 11:53 schrieb Jan Beulich: On 28.11.17 at 11:17, wrote: Am 28.11.2017 um 10:46 schrieb Jan Beulich: On 28.11.17 at 10:12, wrote: In theory the BIOS would search for address space and won't find anything, so the hotplug operation should fail even before it reaches the kernel in the first place. How would the BIOS know what the OS does or plans to do? As far as I know the ACPI BIOS should work directly with the register content. So when we update the register content to enable the MMIO decoding the BIOS should know that as well. I'm afraid I don't follow: During memory hotplug, surely you don't expect the BIOS to do a PCI bus scan? Plus even if it did, it would be racy - some device could, at this very moment, have memory decoding disabled, just for the OS to re-enable it a millisecond later. Yet looking at BAR values is meaningless when memory decode of a device is disabled. No, sorry you misunderstood me. The PCI bus is not even involved here. In AMD Family CPUs you have four main types of address space routed by the NB: 1. Memory space targeting system DRAM. 2. Memory space targeting IO (MMIO). 3. IO space. 4. Configuration space. See section "2.8.2 NB Routing" in the BIOS and Kernel Developer’s Guide (https://support.amd.com/TechDocs/49125_15h_Models_30h-3Fh_BKDG.pdf). Long story short you have fix addresses for configuration and legacy IO (VGA for example) and then configurable memory space for DRAM and MMIO. What the ACPI BIOS does (or at least should do) is taking a look at the registers to find space during memory hotplug. Now in theory the MMIO space should be configurable by similar ACPI BIOS functions, but unfortunately most BIOS doesn't enable that function because it can break some older Windows versions. So what we do here is just what the BIOS should have provided in the first place. I think it's the other way around - the OS needs to avoid using any regions for MMIO which are marked as hotpluggable in SRAT. I was under the impression that this is exactly what acpi_numa_memory_affinity_init() does. Perhaps, except that (when I last looked) insufficient state is (was) being recorded to have that information readily available at the time MMIO space above 4Gb needs to be allocated for some device. That was also my concern, but in the most recent version I'm intentionally doing this fixup very late after all the PCI setup is already done. This way the extra address space is only available for devices which are added by PCI hotplug or for resizing BARs on the fly (which is the use case I'm interested in). Since there is no vNUMA yet for Xen Dom0, that would need special handling. I think that the problem is rather that SRAT is NUMA specific and if I'm not totally mistaken the content is ignored when NUMA support isn't compiled into the kernel. When Xen steals some memory from Dom0 by hocking up itself into the e820 call then I would say the cleanest way is to report this memory in e820 as reserved as well. But take that with a grain of salt, I'm seriously not a Xen expert. The E820 handling in PV Linux is all fake anyway - there's a single chunk of memory given to a PV guest (including Dom0), contiguous in what PV guests know as "physical address space" (not to be mixed up with "machine address space", which is where MMIO needs to be allocated from). Xen code in the kernel then mimics an E820 matching the host one, moving around pieces of memory in physical address space if necessary. Good to know. Since Dom0 knows the machine E820, MMIO allocation shouldn't need to be much different there from the non-Xen case. Yes, completely agree. I think even if we don't do MMIO allocation with this fixup letting the kernel in Dom0 know which memory/address space regions are in use is still a good idea. Otherwise we will run into exactly the same problem when we do the MMIO allocation with an ACPI method and that is certainly going to come in the next BIOS generations because Microsoft is pushing for it. Regards, Christian. Jan ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm 1/2] amdgpu: Switch amdgpu CS tests enabling to new API.
Am 27.11.2017 um 13:31 schrieb Andrey Grodzovsky:
Signed-off-by: Andrey Grodzovsky
Reviewed-by: Christian König
---
tests/amdgpu/amdgpu_test.c | 2 +-
tests/amdgpu/amdgpu_test.h | 5
tests/amdgpu/cs_tests.c| 64 +++---
3 files changed, 38 insertions(+), 33 deletions(-)
diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
index ee64152..e611276 100644
--- a/tests/amdgpu/amdgpu_test.c
+++ b/tests/amdgpu/amdgpu_test.c
@@ -146,7 +146,7 @@ static Suites_Active_Status suites_active_stat[] = {
},
{
.pName = CS_TESTS_STR,
- .pActive = always_active,
+ .pActive = suite_cs_tests_enable,
},
{
.pName = VCE_TESTS_STR,
diff --git a/tests/amdgpu/amdgpu_test.h b/tests/amdgpu/amdgpu_test.h
index 414fcb8..3238e05 100644
--- a/tests/amdgpu/amdgpu_test.h
+++ b/tests/amdgpu/amdgpu_test.h
@@ -85,6 +85,11 @@ int suite_cs_tests_init();
int suite_cs_tests_clean();
/**
+ * Decide if the suite is enabled by default or not.
+ */
+CU_BOOL suite_cs_tests_enable(void);
+
+/**
* Tests in cs test suite
*/
extern CU_TestInfo cs_tests[];
diff --git a/tests/amdgpu/cs_tests.c b/tests/amdgpu/cs_tests.c
index 3b2f17d..4880b74 100644
--- a/tests/amdgpu/cs_tests.c
+++ b/tests/amdgpu/cs_tests.c
@@ -66,6 +66,26 @@ CU_TestInfo cs_tests[] = {
CU_TEST_INFO_NULL,
};
+CU_BOOL suite_cs_tests_enable(void)
+{
+ if (amdgpu_device_initialize(drm_amdgpu[0], &major_version,
+&minor_version, &device_handle))
+ return CU_FALSE;
+
+ family_id = device_handle->info.family_id;
+
+ if (amdgpu_device_deinitialize(device_handle))
+ return CU_FALSE;
+
+
+ if (family_id >= AMDGPU_FAMILY_RV || family_id == AMDGPU_FAMILY_SI) {
+ printf("\n\nThe ASIC NOT support UVD, suite disabled\n");
+ return CU_FALSE;
+ }
+
+ return CU_TRUE;
+}
+
int suite_cs_tests_init(void)
{
amdgpu_bo_handle ib_result_handle;
@@ -90,11 +110,6 @@ int suite_cs_tests_init(void)
chip_rev = device_handle->info.chip_rev;
chip_id = device_handle->info.chip_external_rev;
- if (family_id >= AMDGPU_FAMILY_RV || family_id == AMDGPU_FAMILY_SI) {
- printf("\n\nThe ASIC NOT support UVD, all sub-tests will
pass\n");
- return CUE_SUCCESS;
- }
-
r = amdgpu_cs_ctx_create(device_handle, &context_handle);
if (r)
return CUE_SINIT_FAILED;
@@ -119,24 +134,18 @@ int suite_cs_tests_clean(void)
{
int r;
- if (family_id >= AMDGPU_FAMILY_RV || family_id == AMDGPU_FAMILY_SI) {
- r = amdgpu_device_deinitialize(device_handle);
- if (r)
- return CUE_SCLEAN_FAILED;
- } else {
- r = amdgpu_bo_unmap_and_free(ib_handle, ib_va_handle,
-ib_mc_address, IB_SIZE);
- if (r)
- return CUE_SCLEAN_FAILED;
-
- r = amdgpu_cs_ctx_free(context_handle);
- if (r)
- return CUE_SCLEAN_FAILED;
-
- r = amdgpu_device_deinitialize(device_handle);
- if (r)
- return CUE_SCLEAN_FAILED;
- }
+ r = amdgpu_bo_unmap_and_free(ib_handle, ib_va_handle,
+ib_mc_address, IB_SIZE);
+ if (r)
+ return CUE_SCLEAN_FAILED;
+
+ r = amdgpu_cs_ctx_free(context_handle);
+ if (r)
+ return CUE_SCLEAN_FAILED;
+
+ r = amdgpu_device_deinitialize(device_handle);
+ if (r)
+ return CUE_SCLEAN_FAILED;
return CUE_SUCCESS;
}
@@ -203,9 +212,6 @@ static void amdgpu_cs_uvd_create(void)
void *msg;
int i, r;
- if (family_id >= AMDGPU_FAMILY_RV || family_id == AMDGPU_FAMILY_SI)
- return;
-
req.alloc_size = 4*1024;
req.preferred_heap = AMDGPU_GEM_DOMAIN_GTT;
@@ -277,9 +283,6 @@ static void amdgpu_cs_uvd_decode(void)
uint8_t *ptr;
int i, r;
- if (family_id >= AMDGPU_FAMILY_RV || family_id == AMDGPU_FAMILY_SI)
-return;
-
req.alloc_size = 4*1024; /* msg */
req.alloc_size += 4*1024; /* fb */
if (family_id >= AMDGPU_FAMILY_VI)
@@ -419,9 +422,6 @@ static void amdgpu_cs_uvd_destroy(void)
void *msg;
int i, r;
- if (family_id >= AMDGPU_FAMILY_RV || family_id == AMDGPU_FAMILY_SI)
-return;
-
req.alloc_size = 4*1024;
req.preferred_heap = AMDGPU_GEM_DOMAIN_GTT;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH libdrm] [drm] - Adding amdgpu_cs_create_syncobj2 to create syncobj as signaled initially
Anyone can help to review the change?
Thanks.
Best Regards,
David
-Original Message-
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of David
Mao
Sent: Tuesday, November 28, 2017 11:26 AM
To: amd-gfx@lists.freedesktop.org
Subject: [PATCH libdrm] [drm] - Adding amdgpu_cs_create_syncobj2 to create
syncobj as signaled initially
Change-Id: Icf8d29bd4b50ee76936faacbbe099492cf0557cc
Signed-off-by: David Mao
---
amdgpu/amdgpu.h| 15 +++
amdgpu/amdgpu_cs.c | 10 ++
2 files changed, 25 insertions(+)
diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h index 78fbd1e..47bdb3a 100644
--- a/amdgpu/amdgpu.h
+++ b/amdgpu/amdgpu.h
@@ -1727,6 +1727,21 @@ const char
*amdgpu_get_marketing_name(amdgpu_device_handle dev);
/**
* Create kernel sync object
*
+ * \param dev - \c [in] device handle
+ * \param flags - \c [in] flags that affect creation
+ * \param syncobj - \c [out] sync object handle
+ *
+ * \return 0 on success\n
+ * <0 - Negative POSIX Error code
+ *
+*/
+int amdgpu_cs_create_syncobj2(amdgpu_device_handle dev,
+ uint32_t flags,
+ uint32_t *syncobj);
+
+/**
+ * Create kernel sync object
+ *
* \param dev - \c [in] device handle
* \param syncobj - \c [out] sync object handle
*
diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c index 64ad911..a9fbab9
100644
--- a/amdgpu/amdgpu_cs.c
+++ b/amdgpu/amdgpu_cs.c
@@ -606,6 +606,16 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle
sem)
return amdgpu_cs_unreference_sem(sem); }
+int amdgpu_cs_create_syncobj2(amdgpu_device_handle dev,
+ uint32_t flags,
+ uint32_t *handle)
+{
+ if (NULL == dev)
+ return -EINVAL;
+
+ return drmSyncobjCreate(dev->fd, flags, handle); }
+
int amdgpu_cs_create_syncobj(amdgpu_device_handle dev,
uint32_t *handle)
{
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm] [drm] - Adding amdgpu_cs_create_syncobj2 to create syncobj as signaled initially
Reviewed-by: Christian König
But in general for libdrm changes I would ping Marek, Nicolai, Michel
and in this special case Dave Airlie because he added the patch with the
missing flags field.
And I strongly assume you don't have commit rights, don't you?
Regards,
Christian.
Am 28.11.2017 um 14:22 schrieb Mao, David:
Anyone can help to review the change?
Thanks.
Best Regards,
David
-Original Message-
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of David
Mao
Sent: Tuesday, November 28, 2017 11:26 AM
To: amd-gfx@lists.freedesktop.org
Subject: [PATCH libdrm] [drm] - Adding amdgpu_cs_create_syncobj2 to create
syncobj as signaled initially
Change-Id: Icf8d29bd4b50ee76936faacbbe099492cf0557cc
Signed-off-by: David Mao
---
amdgpu/amdgpu.h| 15 +++
amdgpu/amdgpu_cs.c | 10 ++
2 files changed, 25 insertions(+)
diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h index 78fbd1e..47bdb3a 100644
--- a/amdgpu/amdgpu.h
+++ b/amdgpu/amdgpu.h
@@ -1727,6 +1727,21 @@ const char
*amdgpu_get_marketing_name(amdgpu_device_handle dev);
/**
* Create kernel sync object
*
+ * \param dev - \c [in] device handle
+ * \param flags - \c [in] flags that affect creation
+ * \param syncobj - \c [out] sync object handle
+ *
+ * \return 0 on success\n
+ * <0 - Negative POSIX Error code
+ *
+*/
+int amdgpu_cs_create_syncobj2(amdgpu_device_handle dev,
+ uint32_t flags,
+ uint32_t *syncobj);
+
+/**
+ * Create kernel sync object
+ *
* \param dev - \c [in] device handle
* \param syncobj - \c [out] sync object handle
*
diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c index 64ad911..a9fbab9
100644
--- a/amdgpu/amdgpu_cs.c
+++ b/amdgpu/amdgpu_cs.c
@@ -606,6 +606,16 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle
sem)
return amdgpu_cs_unreference_sem(sem); }
+int amdgpu_cs_create_syncobj2(amdgpu_device_handle dev,
+ uint32_t flags,
+ uint32_t *handle)
+{
+ if (NULL == dev)
+ return -EINVAL;
+
+ return drmSyncobjCreate(dev->fd, flags, handle); }
+
int amdgpu_cs_create_syncobj(amdgpu_device_handle dev,
uint32_t *handle)
{
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH libdrm] [drm] - Adding amdgpu_cs_create_syncobj2 to create syncobj as signaled initially
I have never tried to commit the change before. So I guess the answer is no.
Could you let me know, how I can apply for the commit right?
Thanks.
Best Regards,
David
-Original Message-
From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com]
Sent: Tuesday, November 28, 2017 9:29 PM
To: Mao, David ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH libdrm] [drm] - Adding amdgpu_cs_create_syncobj2 to create
syncobj as signaled initially
Reviewed-by: Christian König
But in general for libdrm changes I would ping Marek, Nicolai, Michel and in
this special case Dave Airlie because he added the patch with the missing flags
field.
And I strongly assume you don't have commit rights, don't you?
Regards,
Christian.
Am 28.11.2017 um 14:22 schrieb Mao, David:
> Anyone can help to review the change?
> Thanks.
>
> Best Regards,
> David
>
> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of David Mao
> Sent: Tuesday, November 28, 2017 11:26 AM
> To: amd-gfx@lists.freedesktop.org
> Subject: [PATCH libdrm] [drm] - Adding amdgpu_cs_create_syncobj2 to
> create syncobj as signaled initially
>
> Change-Id: Icf8d29bd4b50ee76936faacbbe099492cf0557cc
> Signed-off-by: David Mao
> ---
> amdgpu/amdgpu.h| 15 +++
> amdgpu/amdgpu_cs.c | 10 ++
> 2 files changed, 25 insertions(+)
>
> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h index 78fbd1e..47bdb3a
> 100644
> --- a/amdgpu/amdgpu.h
> +++ b/amdgpu/amdgpu.h
> @@ -1727,6 +1727,21 @@ const char
> *amdgpu_get_marketing_name(amdgpu_device_handle dev);
> /**
>* Create kernel sync object
>*
> + * \param dev - \c [in] device handle
> + * \param flags - \c [in] flags that affect creation
> + * \param syncobj - \c [out] sync object handle
> + *
> + * \return 0 on success\n
> + * <0 - Negative POSIX Error code
> + *
> +*/
> +int amdgpu_cs_create_syncobj2(amdgpu_device_handle dev,
> + uint32_t flags,
> + uint32_t *syncobj);
> +
> +/**
> + * Create kernel sync object
> + *
>* \param dev - \c [in] device handle
>* \param syncobj - \c [out] sync object handle
>*
> diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c index
> 64ad911..a9fbab9 100644
> --- a/amdgpu/amdgpu_cs.c
> +++ b/amdgpu/amdgpu_cs.c
> @@ -606,6 +606,16 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle
> sem)
> return amdgpu_cs_unreference_sem(sem); }
>
> +int amdgpu_cs_create_syncobj2(amdgpu_device_handle dev,
> + uint32_t flags,
> + uint32_t *handle)
> +{
> + if (NULL == dev)
> + return -EINVAL;
> +
> + return drmSyncobjCreate(dev->fd, flags, handle); }
> +
> int amdgpu_cs_create_syncobj(amdgpu_device_handle dev,
>uint32_t *handle)
> {
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amd/amdgpu: Add debugfs support for reading GPRs (v2)
Hello Tom St Denis,
The patch c5a60ce81b49: "drm/amd/amdgpu: Add debugfs support for
reading GPRs (v2)" from Dec 5, 2016, leads to the following static
checker warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3774
amdgpu_debugfs_gpr_read()
error: buffer overflow 'data' 1024 <= 4095
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
3731 static ssize_t amdgpu_debugfs_gpr_read(struct file *f, char __user *buf,
3732 size_t size, loff_t *pos)
3733 {
3734 struct amdgpu_device *adev = f->f_inode->i_private;
3735 int r;
3736 ssize_t result = 0;
3737 uint32_t offset, se, sh, cu, wave, simd, thread, bank, *data;
3738
3739 if (size & 3 || *pos & 3)
3740 return -EINVAL;
3741
3742 /* decode offset */
3743 offset = *pos & GENMASK_ULL(11, 0);
^^
offset is set to 0-4095.
3744 se = (*pos & GENMASK_ULL(19, 12)) >> 12;
3745 sh = (*pos & GENMASK_ULL(27, 20)) >> 20;
3746 cu = (*pos & GENMASK_ULL(35, 28)) >> 28;
3747 wave = (*pos & GENMASK_ULL(43, 36)) >> 36;
3748 simd = (*pos & GENMASK_ULL(51, 44)) >> 44;
3749 thread = (*pos & GENMASK_ULL(59, 52)) >> 52;
3750 bank = (*pos & GENMASK_ULL(61, 60)) >> 60;
3751
3752 data = kmalloc_array(1024, sizeof(*data), GFP_KERNEL);
data is a 1024 element array
3753 if (!data)
3754 return -ENOMEM;
3755
3756 /* switch to the specific se/sh/cu */
3757 mutex_lock(&adev->grbm_idx_mutex);
3758 amdgpu_gfx_select_se_sh(adev, se, sh, cu);
3759
3760 if (bank == 0) {
3761 if (adev->gfx.funcs->read_wave_vgprs)
3762 adev->gfx.funcs->read_wave_vgprs(adev, simd,
wave, thread, offset, size>>2, data);
3763 } else {
3764 if (adev->gfx.funcs->read_wave_sgprs)
3765 adev->gfx.funcs->read_wave_sgprs(adev, simd,
wave, offset, size>>2, data);
3766 }
3767
3768 amdgpu_gfx_select_se_sh(adev, 0x, 0x,
0x);
3769 mutex_unlock(&adev->grbm_idx_mutex);
3770
3771 while (size) {
3772 uint32_t value;
3773
3774 value = data[offset++];
^^
We're possibly reading beyond the end of the array. Maybe we are
supposed to divide the offset /= sizeof(*data)?
3775 r = put_user(value, (uint32_t *)buf);
regards,
dan carpenter
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: Fix potential NULL pointer dereferences in amdgpu_dm_atomic_commit_tail
Reviewed-by: Andrey Grodzovsky
Thanks,
Andrey
On 11/27/2017 09:57 AM, Gustavo A. R. Silva wrote:
dm_new_crtc_state->stream and disconnected_acrtc are being dereferenced
before they are null checked, hence there is a potential null pointer
dereference.
Fix this by null checking such pointers before they are dereferenced.
Addresses-Coverity-ID: 1423745 ("Dereference before null check")
Addresses-Coverity-ID: 1423833 ("Dereference before null check")
Fixes: e7b07ceef2a6 ("drm/amd/display: Merge amdgpu_dm_types and amdgpu_dm")
Fixes: 54d765752467 ("drm/amd/display: Unify amdgpu_dm state variable namings.")
Signed-off-by: Gustavo A. R. Silva
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 889ed24..3bdd137 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4190,6 +4190,9 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
+ if (!dm_new_crtc_state->stream)
+ continue;
+
update_stream_scaling_settings(&dm_new_con_state->base.crtc->mode,
dm_new_con_state, (struct dc_stream_state
*)dm_new_crtc_state->stream);
@@ -4197,9 +4200,6 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
WARN_ON(!status);
WARN_ON(!status->plane_count);
- if (!dm_new_crtc_state->stream)
- continue;
-
/*TODO How it works with MPO ?*/
if (!dc_commit_planes_to_stream(
dm->dc,
@@ -4332,9 +4332,11 @@ void dm_restore_drm_connector_state(struct drm_device
*dev,
return;
disconnected_acrtc = to_amdgpu_crtc(connector->encoder->crtc);
- acrtc_state = to_dm_crtc_state(disconnected_acrtc->base.state);
+ if (!disconnected_acrtc)
+ return;
- if (!disconnected_acrtc || !acrtc_state->stream)
+ acrtc_state = to_dm_crtc_state(disconnected_acrtc->base.state);
+ if (!acrtc_state->stream)
return;
/*
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm] [drm] - Adding amdgpu_cs_create_syncobj2 to create syncobj as signaled initially
You need a freedesktop.org account:
https://www.freedesktop.org/wiki/AccountRequests/
Marek
On Tue, Nov 28, 2017 at 2:32 PM, Mao, David wrote:
> I have never tried to commit the change before. So I guess the answer is no.
> Could you let me know, how I can apply for the commit right?
>
> Thanks.
> Best Regards,
> David
>
> -Original Message-
> From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com]
> Sent: Tuesday, November 28, 2017 9:29 PM
> To: Mao, David ; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH libdrm] [drm] - Adding amdgpu_cs_create_syncobj2 to
> create syncobj as signaled initially
>
> Reviewed-by: Christian König
>
> But in general for libdrm changes I would ping Marek, Nicolai, Michel and in
> this special case Dave Airlie because he added the patch with the missing
> flags field.
>
> And I strongly assume you don't have commit rights, don't you?
>
> Regards,
> Christian.
>
> Am 28.11.2017 um 14:22 schrieb Mao, David:
>> Anyone can help to review the change?
>> Thanks.
>>
>> Best Regards,
>> David
>>
>> -Original Message-
>> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
>> Of David Mao
>> Sent: Tuesday, November 28, 2017 11:26 AM
>> To: amd-gfx@lists.freedesktop.org
>> Subject: [PATCH libdrm] [drm] - Adding amdgpu_cs_create_syncobj2 to
>> create syncobj as signaled initially
>>
>> Change-Id: Icf8d29bd4b50ee76936faacbbe099492cf0557cc
>> Signed-off-by: David Mao
>> ---
>> amdgpu/amdgpu.h| 15 +++
>> amdgpu/amdgpu_cs.c | 10 ++
>> 2 files changed, 25 insertions(+)
>>
>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h index 78fbd1e..47bdb3a
>> 100644
>> --- a/amdgpu/amdgpu.h
>> +++ b/amdgpu/amdgpu.h
>> @@ -1727,6 +1727,21 @@ const char
>> *amdgpu_get_marketing_name(amdgpu_device_handle dev);
>> /**
>>* Create kernel sync object
>>*
>> + * \param dev - \c [in] device handle
>> + * \param flags - \c [in] flags that affect creation
>> + * \param syncobj - \c [out] sync object handle
>> + *
>> + * \return 0 on success\n
>> + * <0 - Negative POSIX Error code
>> + *
>> +*/
>> +int amdgpu_cs_create_syncobj2(amdgpu_device_handle dev,
>> + uint32_t flags,
>> + uint32_t *syncobj);
>> +
>> +/**
>> + * Create kernel sync object
>> + *
>>* \param dev - \c [in] device handle
>>* \param syncobj - \c [out] sync object handle
>>*
>> diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c index
>> 64ad911..a9fbab9 100644
>> --- a/amdgpu/amdgpu_cs.c
>> +++ b/amdgpu/amdgpu_cs.c
>> @@ -606,6 +606,16 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle
>> sem)
>> return amdgpu_cs_unreference_sem(sem); }
>>
>> +int amdgpu_cs_create_syncobj2(amdgpu_device_handle dev,
>> + uint32_t flags,
>> + uint32_t *handle)
>> +{
>> + if (NULL == dev)
>> + return -EINVAL;
>> +
>> + return drmSyncobjCreate(dev->fd, flags, handle); }
>> +
>> int amdgpu_cs_create_syncobj(amdgpu_device_handle dev,
>>uint32_t *handle)
>> {
>> --
>> 2.7.4
>>
>> ___
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> ___
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [bug report] drm/amd/amdgpu: Add debugfs support for reading GPRs (v2)
On 28/11/17 09:29 AM, Dan Carpenter wrote:
Hello Tom St Denis,
The patch c5a60ce81b49: "drm/amd/amdgpu: Add debugfs support for
reading GPRs (v2)" from Dec 5, 2016, leads to the following static
checker warning:
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3774
amdgpu_debugfs_gpr_read()
error: buffer overflow 'data' 1024 <= 4095
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
3731 static ssize_t amdgpu_debugfs_gpr_read(struct file *f, char __user
*buf,
3732 size_t size, loff_t *pos)
3733 {
3734 struct amdgpu_device *adev = f->f_inode->i_private;
3735 int r;
3736 ssize_t result = 0;
3737 uint32_t offset, se, sh, cu, wave, simd, thread, bank, *data;
3738
3739 if (size & 3 || *pos & 3)
3740 return -EINVAL;
3741
3742 /* decode offset */
3743 offset = *pos & GENMASK_ULL(11, 0);
^^
offset is set to 0-4095.
3744 se = (*pos & GENMASK_ULL(19, 12)) >> 12;
3745 sh = (*pos & GENMASK_ULL(27, 20)) >> 20;
3746 cu = (*pos & GENMASK_ULL(35, 28)) >> 28;
3747 wave = (*pos & GENMASK_ULL(43, 36)) >> 36;
3748 simd = (*pos & GENMASK_ULL(51, 44)) >> 44;
3749 thread = (*pos & GENMASK_ULL(59, 52)) >> 52;
3750 bank = (*pos & GENMASK_ULL(61, 60)) >> 60;
3751
3752 data = kmalloc_array(1024, sizeof(*data), GFP_KERNEL);
data is a 1024 element array
3753 if (!data)
3754 return -ENOMEM;
3755
3756 /* switch to the specific se/sh/cu */
3757 mutex_lock(&adev->grbm_idx_mutex);
3758 amdgpu_gfx_select_se_sh(adev, se, sh, cu);
3759
3760 if (bank == 0) {
3761 if (adev->gfx.funcs->read_wave_vgprs)
3762 adev->gfx.funcs->read_wave_vgprs(adev, simd, wave,
thread, offset, size>>2, data);
3763 } else {
3764 if (adev->gfx.funcs->read_wave_sgprs)
3765 adev->gfx.funcs->read_wave_sgprs(adev, simd, wave,
offset, size>>2, data);
3766 }
3767
3768 amdgpu_gfx_select_se_sh(adev, 0x, 0x,
0x);
3769 mutex_unlock(&adev->grbm_idx_mutex);
3770
3771 while (size) {
3772 uint32_t value;
3773
3774 value = data[offset++];
^^
We're possibly reading beyond the end of the array. Maybe we are
supposed to divide the offset /= sizeof(*data)?
Hi Dan,
umr only reads from offset zero but to be consistent I think yes the
offset should be /= 4 first since we ensure it's 4 byte aligned it's
clearly a 4 byte offset.
Thanks for finding this, I'll craft up a patch shortly.
Cheers,
Tom
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH libdrm] amdgpu: Add explicit dependency test.
The test is as following:
1) Create context A & B
2) Send a command submission using context A which fires up a compute shader.
3) The shader wait a bit and then write a value to a memory location.
4) Send a command submission using context B which writes another value to the
same memory location, but having an explicit dependency on the first command
submission.
5) Wait with the CPU for both submissions to finish and inspect the written
value.
Test passes if the value seen in the memory location after both submissions is
from command B.
Signed-off-by: Andrey Grodzovsky
---
tests/amdgpu/amdgpu_test.c | 18
tests/amdgpu/basic_tests.c | 264 +
2 files changed, 282 insertions(+)
diff --git a/tests/amdgpu/amdgpu_test.c b/tests/amdgpu/amdgpu_test.c
index 50da17c..8fa3399 100644
--- a/tests/amdgpu/amdgpu_test.c
+++ b/tests/amdgpu/amdgpu_test.c
@@ -49,6 +49,7 @@
#include "CUnit/Basic.h"
#include "amdgpu_test.h"
+#include "amdgpu_internal.h"
/* Test suit names */
#define BASIC_TESTS_STR "Basic Tests"
@@ -401,9 +402,20 @@ static int amdgpu_find_device(uint8_t bus, uint16_t dev)
static void amdgpu_disable_suits()
{
+ amdgpu_device_handle device_handle;
+ uint32_t major_version, minor_version, family_id;
int i;
int size = sizeof(suites_active_stat) / sizeof(suites_active_stat[0]);
+ if (amdgpu_device_initialize(drm_amdgpu[0], &major_version,
+ &minor_version, &device_handle))
+ return;
+
+ family_id = device_handle->info.family_id;
+
+ if (amdgpu_device_deinitialize(device_handle))
+ return;
+
/* Set active status for suits based on their policies */
for (i = 0; i < size; ++i)
if (amdgpu_set_suite_active(suites_active_stat[i].pName,
@@ -420,6 +432,12 @@ static void amdgpu_disable_suits()
if (amdgpu_set_test_active(BO_TESTS_STR, "Metadata", CU_FALSE))
fprintf(stderr, "test deactivation failed - %s\n",
CU_get_error_msg());
+
+
+ /* This test was ran on GFX8 and GFX9 only */
+ if (family_id < AMDGPU_FAMILY_VI || family_id > AMDGPU_FAMILY_RV)
+ if (amdgpu_set_test_active(BASIC_TESTS_STR, "Sync dependency
Test", CU_FALSE))
+ fprintf(stderr, "test deactivation failed - %s\n",
CU_get_error_msg());
}
/* The main() function for setting up and running the tests.
diff --git a/tests/amdgpu/basic_tests.c b/tests/amdgpu/basic_tests.c
index e7f48e3..a78cf52 100644
--- a/tests/amdgpu/basic_tests.c
+++ b/tests/amdgpu/basic_tests.c
@@ -50,6 +50,7 @@ static void amdgpu_command_submission_multi_fence(void);
static void amdgpu_command_submission_sdma(void);
static void amdgpu_userptr_test(void);
static void amdgpu_semaphore_test(void);
+static void amdgpu_sync_dependency_test(void);
static void amdgpu_command_submission_write_linear_helper(unsigned ip_type);
static void amdgpu_command_submission_const_fill_helper(unsigned ip_type);
@@ -63,6 +64,7 @@ CU_TestInfo basic_tests[] = {
{ "Command submission Test (Multi-Fence)",
amdgpu_command_submission_multi_fence },
{ "Command submission Test (SDMA)", amdgpu_command_submission_sdma },
{ "SW semaphore Test", amdgpu_semaphore_test },
+ { "Sync dependency Test", amdgpu_sync_dependency_test },
CU_TEST_INFO_NULL,
};
#define BUFFER_SIZE (8 * 1024)
@@ -226,6 +228,60 @@ CU_TestInfo basic_tests[] = {
*/
# define PACKET3_DMA_DATA_SI_CP_SYNC (1 << 31)
+
+#define PKT3_CONTEXT_CONTROL 0x28
+#define CONTEXT_CONTROL_LOAD_ENABLE(x) (((unsigned)(x) & 0x1) << 31)
+#define CONTEXT_CONTROL_LOAD_CE_RAM(x) (((unsigned)(x) & 0x1) << 28)
+#define CONTEXT_CONTROL_SHADOW_ENABLE(x) (((unsigned)(x) & 0x1) << 31)
+
+#define PKT3_CLEAR_STATE 0x12
+
+#define PKT3_SET_SH_REG0x76
+#definePACKET3_SET_SH_REG_START
0x2c00
+
+#definePACKET3_DISPATCH_DIRECT 0x15
+
+
+/* gfx 8 */
+#define mmCOMPUTE_PGM_LO
0x2e0c
+#define mmCOMPUTE_PGM_RSRC1
0x2e12
+#define mmCOMPUTE_TMPRING_SIZE
0x2e18
+#define mmCOMPUTE_USER_DATA_0
0x2e40
+#define mmCOMPUTE_USER_DATA_1
0x2e41
+#define mmCOMPUTE_RESOURCE_LIMITS
0x2e15
+#define mmCOMPUTE_NUM_THREAD_X
0x2e07
+
+
+
+#define SWAP_32(num) ((num>>24)&0xff) | \
+ ((num<<8)&0xff) | \
+ ((num>>8)&0xff00) | \
+ ((num<<24)&0xff00)
+
+
+/* Shader code
+
Re: [PATCH] drm/amd/display: Print type if we get wrong ObjectID from bios
On 11/27/2017 05:23 PM, Shawn Starr wrote: Hi Harry, here's dmesg from kernel with dc=1 with patch applied, this is against Linus's 4.15-rc1 git tag. Thanks, Shawn. Hi Harry, Here some background from an issue I encountered last summer: These two patches were used https://lists.freedesktop.org/archives/amd-gfx/2016-August/001674.html https://lists.freedesktop.org/archives/amd-gfx/2016-August/001673.html From bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97460 This might add some context, Thanks, Shawn ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [amdgpu] script to generate a VCE 1.0 amdgpu compatible firmware
On 2017-11-23 00:53, Alexandre Demers wrote: > Hi, > > I just want to let you know that I'm still alive and still committed to > porting VCE 1.0 from radeon to amdgpu. However, for many reasons, I've > been pretty much unable to work on the code since my last communication. > > That being said, I've created a script based on Piotr Redlewski's work > to generate a compatible VCE 1.0 firmware with amdgpu. The script can be > found on GithubGist: > https://gist.github.com/Oxalin/ba9aa9c1c1912e68f76dadcad436d1a4 > > Now, I've read the patchset sent by Redlewski for adding UVD on GCN 1 > devices. Christian, since you were to see if AMD could release a new UVD > firmware with header and correct 40bit addressing, could you ask if an > official VCE 1.0 firmware with an official header (and whatever else > could be needed) be released at the same time? > > Cheers > The script now lives on github at the following address: g...@github.com:Oxalin/vce-1.0-fw-convertor.git Christian, any news about official VCE and UVD firmwares from AMD? Alexandre Demers ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [amdgpu] script to generate a VCE 1.0 amdgpu compatible firmware
Am 28.11.2017 um 17:34 schrieb Alexandre Demers: On 2017-11-23 00:53, Alexandre Demers wrote: Hi, I just want to let you know that I'm still alive and still committed to porting VCE 1.0 from radeon to amdgpu. However, for many reasons, I've been pretty much unable to work on the code since my last communication. That being said, I've created a script based on Piotr Redlewski's work to generate a compatible VCE 1.0 firmware with amdgpu. The script can be found on GithubGist: https://gist.github.com/Oxalin/ba9aa9c1c1912e68f76dadcad436d1a4 Now, I've read the patchset sent by Redlewski for adding UVD on GCN 1 devices. Christian, since you were to see if AMD could release a new UVD firmware with header and correct 40bit addressing, could you ask if an official VCE 1.0 firmware with an official header (and whatever else could be needed) be released at the same time? Cheers The script now lives on github at the following address: g...@github.com:Oxalin/vce-1.0-fw-convertor.git Christian, any news about official VCE and UVD firmwares from AMD? Yeah, we talked about that just yesterday. VCE shouldn't be to much of an issue. The main problem is that we need to figure out the latest tested version for SI and then throw that into the generator. UVD is a bigger problem. Currently still looking into this. Regards, Christian. Alexandre Demers ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 02/13] fbdev: add remove_conflicting_pci_framebuffers()
On Tue, Nov 28, 2017 at 12:32:38PM +0100, Greg KH wrote: > On Tue, Nov 28, 2017 at 11:22:17AM +0100, Daniel Vetter wrote: > > On Mon, Nov 27, 2017 at 08:52:19PM +, Sudip Mukherjee wrote: > > > On Mon, Nov 27, 2017 at 11:27:59AM +0100, Daniel Vetter wrote: > > > > On Fri, Nov 24, 2017 at 06:53:31PM +0100, Michał Mirosław wrote: > > > > > Almost all drivers using remove_conflicting_framebuffers() wrap it > > > > > with > > > > > the same code. Extract common part from PCI drivers into separate > > > > > remove_conflicting_pci_framebuffers(). > > > > > > > > > > Signed-off-by: Michał Mirosław > > > > > > > > Since the only driver that seems to use this is the staging one, which > > > > imo > > > > is a DOA project, not sure it's worth to bother with this here. > > > > > > afaik, this device is used in production by few manufacturers and it is > > > usefull for them to have it in the tree and the only reason it is still > > > in staging is because Tomi announced he will not take any new drivers in > > > fbdev. And, so I have not taken the initiative to properly move it out > > > of staging. I think Bartlomiej will also not accept new drivers in fbdev. > > > > Imo, if no one cares about porting it to kms (which will give you an fbdev > > driver for free), then we should throw it out. At least I thought staging > > was only for stuff that had a reasonable chance to get mainlined. Not as a > > dumping ground for drivers that people use, but don't bother to get ready > > for mainline. > > > > Greg? > > Yes, if no one is working to get it out of staging, that means no one > cares about it, and it needs to be removed from the tree. Agreed. I was not working on getting it out of staging as there is no place for it to go. But, Teddy Wang told me that they have a working drm driver for it, but it is not atomic like Daniel was asking for. If it is ok, then I can send in a patch to remove the existing sm750 from staging and add the new sm750 drm driver to staging. And after it is ready, we can go ahead with moving it out of staging to drm. Greg? Daniel? -- Regards Sudip ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Xen-devel] [PATCH v9 4/5] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors v5
>>> On 28.11.17 at 11:17, wrote: > Am 28.11.2017 um 10:46 schrieb Jan Beulich: > On 28.11.17 at 10:12, wrote: >>> In theory the BIOS would search for address space and won't find >>> anything, so the hotplug operation should fail even before it reaches >>> the kernel in the first place. >> How would the BIOS know what the OS does or plans to do? > > As far as I know the ACPI BIOS should work directly with the register > content. > > So when we update the register content to enable the MMIO decoding the > BIOS should know that as well. I'm afraid I don't follow: During memory hotplug, surely you don't expect the BIOS to do a PCI bus scan? Plus even if it did, it would be racy - some device could, at this very moment, have memory decoding disabled, just for the OS to re-enable it a millisecond later. Yet looking at BAR values is meaningless when memory decode of a device is disabled. >> I think >> it's the other way around - the OS needs to avoid using any regions >> for MMIO which are marked as hotpluggable in SRAT. > > I was under the impression that this is exactly what > acpi_numa_memory_affinity_init() does. Perhaps, except that (when I last looked) insufficient state is (was) being recorded to have that information readily available at the time MMIO space above 4Gb needs to be allocated for some device. >> Since there is >> no vNUMA yet for Xen Dom0, that would need special handling. > > I think that the problem is rather that SRAT is NUMA specific and if I'm > not totally mistaken the content is ignored when NUMA support isn't > compiled into the kernel. > > When Xen steals some memory from Dom0 by hocking up itself into the e820 > call then I would say the cleanest way is to report this memory in e820 > as reserved as well. But take that with a grain of salt, I'm seriously > not a Xen expert. The E820 handling in PV Linux is all fake anyway - there's a single chunk of memory given to a PV guest (including Dom0), contiguous in what PV guests know as "physical address space" (not to be mixed up with "machine address space", which is where MMIO needs to be allocated from). Xen code in the kernel then mimics an E820 matching the host one, moving around pieces of memory in physical address space if necessary. Since Dom0 knows the machine E820, MMIO allocation shouldn't need to be much different there from the non-Xen case. Jan ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 02/13] fbdev: add remove_conflicting_pci_framebuffers()
On Tue, Nov 28, 2017 at 11:22:17AM +0100, Daniel Vetter wrote: > On Mon, Nov 27, 2017 at 08:52:19PM +, Sudip Mukherjee wrote: > > On Mon, Nov 27, 2017 at 11:27:59AM +0100, Daniel Vetter wrote: > > > On Fri, Nov 24, 2017 at 06:53:31PM +0100, Michał Mirosław wrote: > > > > Almost all drivers using remove_conflicting_framebuffers() wrap it with > > > > the same code. Extract common part from PCI drivers into separate > > > > remove_conflicting_pci_framebuffers(). > > > > > > > > Signed-off-by: Michał Mirosław > > > > > > Since the only driver that seems to use this is the staging one, which imo > > > is a DOA project, not sure it's worth to bother with this here. > > > > afaik, this device is used in production by few manufacturers and it is > > usefull for them to have it in the tree and the only reason it is still > > in staging is because Tomi announced he will not take any new drivers in > > fbdev. And, so I have not taken the initiative to properly move it out > > of staging. I think Bartlomiej will also not accept new drivers in fbdev. > > Imo, if no one cares about porting it to kms (which will give you an fbdev > driver for free), then we should throw it out. At least I thought staging > was only for stuff that had a reasonable chance to get mainlined. Not as a > dumping ground for drivers that people use, but don't bother to get ready > for mainline. > > Greg? Yes, if no one is working to get it out of staging, that means no one cares about it, and it needs to be removed from the tree. thanks, greg k-h ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [Xen-devel] [PATCH v9 4/5] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors v5
>>> On 28.11.17 at 10:12, wrote: > In theory the BIOS would search for address space and won't find > anything, so the hotplug operation should fail even before it reaches > the kernel in the first place. How would the BIOS know what the OS does or plans to do? I think it's the other way around - the OS needs to avoid using any regions for MMIO which are marked as hotpluggable in SRAT. Since there is no vNUMA yet for Xen Dom0, that would need special handling. Jan ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 02/13] fbdev: add remove_conflicting_pci_framebuffers()
On Tue, Nov 28, 2017 at 12:30:30PM +, Sudip Mukherjee wrote: > On Tue, Nov 28, 2017 at 12:32:38PM +0100, Greg KH wrote: > > On Tue, Nov 28, 2017 at 11:22:17AM +0100, Daniel Vetter wrote: > > > On Mon, Nov 27, 2017 at 08:52:19PM +, Sudip Mukherjee wrote: > > > > On Mon, Nov 27, 2017 at 11:27:59AM +0100, Daniel Vetter wrote: > > > > > On Fri, Nov 24, 2017 at 06:53:31PM +0100, Michał Mirosław wrote: > > > > > > Almost all drivers using remove_conflicting_framebuffers() wrap it > > > > > > with > > > > > > the same code. Extract common part from PCI drivers into separate > > > > > > remove_conflicting_pci_framebuffers(). > > > > > > > > > > > > Signed-off-by: Michał Mirosław > > > > > > > > > > Since the only driver that seems to use this is the staging one, > > > > > which imo > > > > > is a DOA project, not sure it's worth to bother with this here. > > > > > > > > afaik, this device is used in production by few manufacturers and it is > > > > usefull for them to have it in the tree and the only reason it is still > > > > in staging is because Tomi announced he will not take any new drivers in > > > > fbdev. And, so I have not taken the initiative to properly move it out > > > > of staging. I think Bartlomiej will also not accept new drivers in > > > > fbdev. > > > > > > Imo, if no one cares about porting it to kms (which will give you an fbdev > > > driver for free), then we should throw it out. At least I thought staging > > > was only for stuff that had a reasonable chance to get mainlined. Not as a > > > dumping ground for drivers that people use, but don't bother to get ready > > > for mainline. > > > > > > Greg? > > > > Yes, if no one is working to get it out of staging, that means no one > > cares about it, and it needs to be removed from the tree. > > Agreed. I was not working on getting it out of staging as there is no > place for it to go. > But, Teddy Wang told me that they have a working drm driver for it, but > it is not atomic like Daniel was asking for. If it is ok, then I can send > in a patch to remove the existing sm750 from staging and add the new sm750 > drm driver to staging. And after it is ready, we can go ahead with moving > it out of staging to drm. > > Greg? Daniel? I'll always take patches that delete code from staging :) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH umr] Add more detail and colour to the ring decoder
Now you can use '-O use_colour' to spruce up the ring/ib
decoder. You can previously use '-O bits' to decode
register writes which has also been cleaned up.
Assumes a wide display which for a graphics company is probably
safe to assume you're not on an 80x25 MDA display.
Signed-off-by: Tom St Denis
---
src/app/ring_read.c | 6 +-
src/lib/dump_ib.c | 19 +++-
src/lib/ring_decode.c | 304 --
src/umr.h | 10 +-
4 files changed, 202 insertions(+), 137 deletions(-)
diff --git a/src/app/ring_read.c b/src/app/ring_read.c
index 1e569875f779..424e44288f8c 100644
--- a/src/app/ring_read.c
+++ b/src/app/ring_read.c
@@ -118,7 +118,10 @@ void umr_read_ring(struct umr_asic *asic, char *ringpath)
do {
value = ring_data[(start+12)>>2];
- printf("%s.%s.ring[%4lu] == 0x%08lx ", asic->asicname,
ringname, (unsigned long)start >> 2, (unsigned long)value);
+ printf("%s.%s.ring[%s%4lu%s] == %s0x%08lx%s ",
+ asic->asicname, ringname,
+ BLUE, (unsigned long)start >> 2, RST,
+ YELLOW, (unsigned long)value, RST);
if (enable_decoder && start == rptr && start != wptr) {
use_decoder = 1;
decoder.pm4.cur_opcode = 0x;
@@ -127,6 +130,7 @@ void umr_read_ring(struct umr_asic *asic, char *ringpath)
(start == rptr) ? 'r' : '.',
(start == wptr) ? 'w' : '.',
(start == drv_wptr) ? 'D' : '.');
+ decoder.next_ib_info.addr = start / 4;
if (use_decoder)
umr_print_decode(asic, &decoder, value);
printf("\n");
diff --git a/src/lib/dump_ib.c b/src/lib/dump_ib.c
index cba497373fe2..18600fbc8010 100644
--- a/src/lib/dump_ib.c
+++ b/src/lib/dump_ib.c
@@ -30,19 +30,32 @@ void umr_dump_ib(struct umr_asic *asic, struct
umr_ring_decoder *decoder)
uint32_t *data = NULL, x;
static const char *hubs[] = { "gfxhub", "mmhub" };
- printf("Dumping IB at (%s) VMID:%u 0x%llx of %u words\n",
- hubs[decoder->next_ib_info.vmid >> 8],
+ printf("Dumping IB at (%s%s%s) VMID:%u 0x%llx of %u words from ",
+ GREEN, hubs[decoder->next_ib_info.vmid >> 8], RST,
(unsigned)decoder->next_ib_info.vmid & 0xFF,
(unsigned long long)decoder->next_ib_info.ib_addr,
(unsigned)decoder->next_ib_info.size/4);
+ if (decoder->src.ib_addr == 0)
+ printf("ring[%s%u%s]", BLUE, (unsigned)decoder->src.addr, RST);
+ else
+ printf("IB[%s%u%s] at %s%d%s:%s0x%llx%s",
+ BLUE, (unsigned)decoder->src.addr, RST,
+ YELLOW, decoder->src.vmid, RST,
+ YELLOW, (unsigned long long)decoder->src.ib_addr, RST);
+
+ printf("\n");
+
// read IB
data = calloc(sizeof(*data), decoder->next_ib_info.size/sizeof(*data));
if (data && !umr_read_vram(asic, decoder->next_ib_info.vmid,
decoder->next_ib_info.ib_addr, decoder->next_ib_info.size, data)) {
// dump IB
decoder->pm4.cur_opcode = 0x;
for (x = 0; x < decoder->next_ib_info.size/4; x++) {
- printf("IB[%5u] = 0x%08lx ... ", (unsigned)x, (unsigned
long)data[x]);
+ decoder->next_ib_info.addr = x;
+ printf("IB[%s%5u%s] = %s0x%08lx%s ... ",
+ BLUE, (unsigned)x, RST,
+ YELLOW, (unsigned long)data[x], RST);
umr_print_decode(asic, decoder, data[x]);
printf("\n");
}
diff --git a/src/lib/ring_decode.c b/src/lib/ring_decode.c
index a2cc1ef6a668..8079069649bb 100644
--- a/src/lib/ring_decode.c
+++ b/src/lib/ring_decode.c
@@ -357,6 +357,11 @@ static void add_ib(struct umr_ring_decoder *decoder)
pdecoder->next_ib_info.vmid= decoder->pm4.next_ib_state.ib_vmid;
pdecoder->next_ib_info.vm_base_addr = ~0ULL; // not used yet.
+
+ pdecoder->src.ib_addr = decoder->next_ib_info.ib_addr;
+ pdecoder->src.vmid= decoder->next_ib_info.vmid;
+ pdecoder->src.addr= decoder->next_ib_info.addr;
+
memset(&decoder->pm4.next_ib_state, 0,
sizeof(decoder->pm4.next_ib_state));
}
@@ -369,14 +374,14 @@ static char *umr_reg_name(struct umr_asic *asic, uint64_t
addr)
reg = umr_find_reg_by_addr(asic, addr, &ip);
if (ip && reg) {
- sprintf(name, "%s.%s", ip->ipname, reg->regname);
+ sprintf(name, "%s%s.%s%s", RED, ip->ipname, reg->regname, RST);
return name;
} else {
return "";
}
}
-static void print_bits(struct umr_asic *asic, uint32_t regno, uint32_t value)
+static void print_bits(struct umr_asic
Re: [PATCH v9 4/5] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors v5
On 11/28/2017 04:12 AM, Christian König wrote: > > > How about the attached patch? It limits the newly added MMIO space to > the upper 256GB of the address space. That should still be enough for > most devices, but we avoid both issues with Xen dom0 as most likely > problems with memory hotplug as well. It certainly makes the problem to be less likely so I guess we could do this for now. Thanks. -boris ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Fixes for 4.15-rc1
Hi Alex, I cherry-picked a bunch of fixes for 4.15. These can be found at hwentlan/4.15-rc1-fixes. Of the changes the highlighted ones (with *) in particular are highly recommended, but even the other ones are probably good to have. 1086abf3e8ab drm/amd/display: USB-C / thunderbolt dock specific workaround * fc59fdfdbd52 drm/amd/display: Switch to drm_atomic_helper_wait_for_flip_done * c386a22129a1 drm/amd/display: fix gamma setting * af54c36e0c30 drm/amd/display: Do not put drm_atomic_state on resume 89be3facaeb1 drm/amd/display: Fix couple more inconsistent NULL checks in dc_resource 75909a2c505a drm/amd/display: Fix potential NULL and mem leak in create_links dfa21baf4699 drm/amd/display: Fix hubp check in set_cursor_position 4cea995b2636 drm/amd/display: Fix use before NULL check in validate_timing 9a130225c042 drm/amd/display: Bunch of smatch error and warning fixes in DC 65ca5b19622e drm/amd/display: Fix amdgpu_dm bugs found by smatch 8ae2f517cf1d drm/amd/display: try to find matching audio inst for enc inst first 86b16c34c2ef drm/amd/display: fix seq issue: turn on clock before programming afmt. b35eb4120a68 drm/amd/display: fix memory leaks on error exit return ab164036ba33 drm/amd/display: check plane state before validating fbc 70d7f684a4d8 drm/amd/display: Do DC mode-change check when adding CRTCs 1baa90a34571 drm/amd/display: Revert noisy assert messages 7397a660300a drm/amd/display: fix split viewport rounding error bd4eb434d515 drm/amd/display: Check aux channel before MST resume 65633318cedd drm/amd/display: fix split recout offset 9e403e8d7d07 drm/amd/display: Don't reject 3D timings 277a9b0854c4 drm/amd/display: Handle as MST first and then DP dongle if sink support both d5d60701f3ec drm/amd/display: fix split recout calculation e947765cc008 drm/amd/display: Fix S3 topology change 405b3c636ac5 drm/amd/display: Add timing validation against dongle cap * 1f63dba38d42 drm/amd/display: Should disable when new stream is null fa4e15633102 drm/amd/display: Add null check for 24BPP (xfm and dpp) I confirmed on Polaris11 the bolded changes fix: * DPMS * Gamma The other fixes address a bunch of Raven stuff, as well as dock/dongle issues, and potential problems found by smatch, but I haven't been able to confirm those myself. I gave S3 a spin since that is also problematic without "Do not put drm_atomic_state on resume" but on my motherboard the system never comes out of S3 on 4.15rc1, even without loading amdgpu and no X. This works on amd-staging-drm-next, so must be a regression elsewhere in the kernel. Can you pick these up for your fixes branch? Harry PS: I succesfully rebased all of amd-staging-dal-drm-next on 4.15-rc1 last night. It was painless. Just had to skip these two patches: drm/amdgpu Moving amdgpu asic types to a separate file x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v2 2/2] drm/amd/display: Don't call dm_log_to_buffer directly in dc_conn_log
On Tue, Nov 28, 2017 at 6:09 AM, Michel Dänzer wrote:
>
> Ping on this series.
>
> This patch is v2 of a previously single patch, which was reviewed by Harry.
>
>
Series is:
Reviewed-by: Alex Deucher
> On 2017-11-23 06:48 PM, Michel Dänzer wrote:
>> From: Michel Dänzer
>>
>> dm_log_to_buffer logs unconditionally, so calling it directly resulted
>> in the main message being logged even when the event type isn't enabled
>> in the event mask.
>>
>> To fix this, use the new dm_logger_append_va API.
>>
>> Fixes spurious messages like
>>
>> [drm] {1920x1200, 2080x1235@154000Khz}
>>
>> in dmesg when a mode is set.
>>
>> v2:
>> * Use new dm_logger_append_va API, fixes incorrect va_list usage in v1
>> * Just use and decrease entry.buf_offset to get rid of the trailing
>> newline
>>
>> Signed-off-by: Michel Dänzer
>> ---
>> drivers/gpu/drm/amd/display/dc/basics/log_helpers.c | 10 +++---
>> 1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
>> b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
>> index 785b943b60ed..fe1648f81d71 100644
>> --- a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
>> +++ b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
>> @@ -80,15 +80,11 @@ void dc_conn_log(struct dc_context *ctx,
>> link->link_index);
>>
>> va_start(args, msg);
>> - entry.buf_offset += dm_log_to_buffer(
>> - &entry.buf[entry.buf_offset],
>> - LOG_MAX_LINE_SIZE - entry.buf_offset,
>> - msg, args);
>> + dm_logger_append_va(&entry, msg, args);
>>
>> - if (entry.buf[strlen(entry.buf) - 1] == '\n') {
>> - entry.buf[strlen(entry.buf) - 1] = '\0';
>> + if (entry.buf_offset > 0 &&
>> + entry.buf[entry.buf_offset - 1] == '\n')
>> entry.buf_offset--;
>> - }
>>
>> if (hex_data)
>> for (i = 0; i < hex_data_count; i++)
>>
>
>
> --
> Earthling Michel Dänzer | http://www.amd.com
> Libre software enthusiast | Mesa and X developer
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Fixes for 4.15-rc1
On 2017 Nov 28, Harry Wentland wrote: > Hi Alex, > > I cherry-picked a bunch of fixes for 4.15. These can be found at > hwentlan/4.15-rc1-fixes. > > Of the changes the highlighted ones (with *) in particular are highly > recommended, but even the other ones are probably good to have. > > * af54c36e0c30 drm/amd/display: Do not put drm_atomic_state on resume This one is really needed, cause it fixes a use-after-free. See this thread: https://lists.freedesktop.org/archives/amd-gfx/2017-November/016236.html Additionally, another use-after-free waits for fixing in 4.15-rc: https://lists.freedesktop.org/archives/amd-gfx/2017-October/014827.html -- Regards, Johannes ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/1] drm/amdgpu: Pin to gtt before cpu accesses dma buf.
To improve cpu read performance. This is implemented for APUs currently.
Change-Id: I300a7daed8f2b0ba6be71a43196a6b8617ddc62e
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 10 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 108 ++
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 +-
5 files changed, 126 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index f8657c3..193db70 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -417,6 +417,8 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev,
struct dma_buf *amdgpu_gem_prime_export(struct drm_device *dev,
struct drm_gem_object *gobj,
int flags);
+struct drm_gem_object *amdgpu_gem_prime_import(struct drm_device *dev,
+ struct dma_buf *dma_buf);
int amdgpu_gem_prime_pin(struct drm_gem_object *obj);
void amdgpu_gem_prime_unpin(struct drm_gem_object *obj);
struct reservation_object *amdgpu_gem_prime_res_obj(struct drm_gem_object *);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index d704a45..b5483e4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -147,6 +147,7 @@ int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc,
struct drm_device *dev = crtc->dev;
struct amdgpu_device *adev = dev->dev_private;
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
+ bool gtt_scannable = (adev->asic_type >= CHIP_CARRIZO && adev->flags &
AMD_IS_APU);
struct amdgpu_framebuffer *old_amdgpu_fb;
struct amdgpu_framebuffer *new_amdgpu_fb;
struct drm_gem_object *obj;
@@ -190,8 +191,13 @@ int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc,
r = amdgpu_bo_pin(new_abo, AMDGPU_GEM_DOMAIN_VRAM, &base);
if (unlikely(r != 0)) {
- DRM_ERROR("failed to pin new abo buffer before flip\n");
- goto unreserve;
+ /* latest APUs support gtt scan out */
+ if (gtt_scannable)
+ r = amdgpu_bo_pin(new_abo, AMDGPU_GEM_DOMAIN_GTT,
&base);
+ if (unlikely(r != 0)) {
+ DRM_ERROR("failed to pin new abo buffer before flip\n");
+ goto unreserve;
+ }
}
r = reservation_object_get_fences_rcu(new_abo->tbo.resv, &work->excl,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 31383e0..df30b08 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -868,7 +868,7 @@ static struct drm_driver kms_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_export = amdgpu_gem_prime_export,
- .gem_prime_import = drm_gem_prime_import,
+ .gem_prime_import = amdgpu_gem_prime_import,
.gem_prime_pin = amdgpu_gem_prime_pin,
.gem_prime_unpin = amdgpu_gem_prime_unpin,
.gem_prime_res_obj = amdgpu_gem_prime_res_obj,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
index ae9c106..9e1424d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
@@ -164,6 +164,82 @@ struct reservation_object *amdgpu_gem_prime_res_obj(struct
drm_gem_object *obj)
return bo->tbo.resv;
}
+static int amdgpu_gem_begin_cpu_access(struct dma_buf *dma_buf, enum
dma_data_direction direction)
+{
+ struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
+ struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+ long i, ret = 0;
+ unsigned old_count;
+ bool reads = (direction == DMA_BIDIRECTIONAL || direction ==
DMA_FROM_DEVICE);
+ bool gtt_scannable = (adev->asic_type >= CHIP_CARRIZO && adev->flags &
AMD_IS_APU);
+ u32 domain;
+
+ if (!reads || !gtt_scannable)
+ return 0;
+
+ ret = amdgpu_bo_reserve(bo, false);
+ if (unlikely(ret != 0))
+ return ret;
+
+ /*
+* Wait for all shared fences to complete before we switch to future
+* use of exclusive fence on this prime shared bo.
+*/
+ ret = reservation_object_wait_timeout_rcu(bo->tbo.resv, true, false,
+ MAX_SCHEDULE_TIMEOUT);
+
+ if (unlikely(ret < 0)) {
+ DRM_DEBUG_PRIME("Fence wait failed: %li\n", ret);
+ amdgpu_bo_unreserve(bo);
+ return ret;
+ }
+
+ ret = 0;
+ /* Pin to gtt */
+
RE: [PATCH] drm/amd/amdgpu: set gtt size according to system memory size only
-Original Message-
From: Koenig, Christian
Sent: Tuesday, November 28, 2017 6:01 PM
To: He, Roger ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/amdgpu: set gtt size according to system memory
size only
Am 28.11.2017 um 10:40 schrieb Roger He:
> Change-Id: Ib634375b90d875fe04a890fc82fb1e3b7112676a
> Signed-off-by: Roger He
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 16 +++-
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 17bf0ce..d773c5e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1328,13 +1328,19 @@ int amdgpu_ttm_init(struct amdgpu_device
> *adev)
>
> if (amdgpu_gtt_size == -1) {
> struct sysinfo si;
> + uint64_t sys_mem_size;
>
> si_meminfo(&si);
> - gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
> -adev->mc.mc_vram_size),
> -((uint64_t)si.totalram * si.mem_unit * 3/4));
> - }
> - else
> + sys_mem_size = (uint64_t)si.totalram * si.mem_unit;
> + gtt_size = AMDGPU_DEFAULT_GTT_SIZE_MB << 20;
> +
> + /* leave 2GB for OS to work with */
> + if (sys_mem_size > (2ULL << 30)) {
> + gtt_size = max(gtt_size, sys_mem_size - (2ULL << 30));
> + } else
No need for the "{}" here.
> + DRM_INFO("amdgpu: Too small system memory %llu MB\n",
> + sys_mem_size >> 20);
> + } else
I have a preference to stick with the 75% rule similar to how TTM does things,
but that isn't a hard opinion if you have a good argument.
[Roger]: originally I used the 75% rule as well. But for a special test case,
test failed. Anyway, let's keep 75% here since seems it is more reasonable. And
for the special test case, will use module parameter to change GTT size
temporarily.
Thanks
Roger(Hongbo.He)
Regards,
Christian.
> gtt_size = (uint64_t)amdgpu_gtt_size << 20;
> r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_TT, gtt_size >> PAGE_SHIFT);
> if (r) {
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: correct vce4.0 fw config for SRIOV (V2)
Hi Christian,
I have talked with hw team for the reason why adding the masks. the answer is
"bits 24-27 of the VCE_VCPU_CACHE_OFFSETx registers should be set to the cache
window # (0 for window 0, 1 for window 1, etc.)"
Best Regards,
Frank
-邮件原件-
发件人: Min, Frank
发送时间: 2017年11月23日 12:08
收件人: Koenig, Christian ;
amd-gfx@lists.freedesktop.org; Liu, Leo
主题: RE: [PATCH] drm/amdgpu: correct vce4.0 fw config for SRIOV (V2)
Hi Leo,
Would you please comments on Christian's questions?
Best Regards,
Frank
-Original Message-
From: Min, Frank
Sent: Wednesday, November 22, 2017 4:04 PM
To: Koenig, Christian ;
amd-gfx@lists.freedesktop.org; Liu, Leo
Subject: RE: [PATCH] drm/amdgpu: correct vce4.0 fw config for SRIOV (V2)
Hi Christian,
Thanks again for your review.
And for the mask change my understanding is it is to be used for mark different
part of fw (1<<24 is for stack and 2<<24 is for the data).
And more detailed background would need Leo give us.
Best Regards,
Frank
-Original Message-
From: Koenig, Christian
Sent: Wednesday, November 22, 2017 3:57 PM
To: Min, Frank ; amd-gfx@lists.freedesktop.org; Liu, Leo
Subject: Re: [PATCH] drm/amdgpu: correct vce4.0 fw config for SRIOV (V2)
Hi Frank,
thanks, the patch looks much better now.
> The masks using is to programming the stack and data part for vce fw. And
> this part of code is borrowed from the non-sriov sequences.
In this case Leo can you explain this strange masks used for the
VCE_VCPU_CACHE_OFFSET* registers?
> MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
> mmVCE_VCPU_CACHE_OFFSET0),
> - offset & 0x7FFF);
> + offset & ~0x0f00);
...
> MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
> mmVCE_VCPU_CACHE_OFFSET1),
> - offset & 0x7FFF);
> + (offset & ~0x0f00) | (1 << 24));
Using ~0x0f00 looks really odd here and what should the "| (1 << 24)" part
be about?
Thanks,
Christian.
Am 22.11.2017 um 06:11 schrieb Min, Frank:
> Hi Christian,
> Patch updated according to your suggestions.
> The masks using is to programming the stack and data part for vce fw. And
> this part of code is borrowed from the non-sriov sequences.
>
> Best Regards,
> Frank
>
> 1. program vce 4.0 fw with 48 bit address 2. correct vce 4.0 fw stack
> and date offset
>
> Change-Id: Ic1bc49c21d3a90c477d11162f9d6d9e2073fbbd3
> Signed-off-by: Frank Min
> ---
> drivers/gpu/drm/amd/amdgpu/vce_v4_0.c | 38
> +++
> 1 file changed, 25 insertions(+), 13 deletions(-) mode change
> 100644 => 100755 drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> old mode 100644
> new mode 100755
> index 7574554..024a1be
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> @@ -243,37 +243,49 @@ static int vce_v4_0_sriov_start(struct amdgpu_device
> *adev)
> MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
> mmVCE_LMI_VM_CTRL), 0);
>
> if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
> - MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
> mmVCE_LMI_VCPU_CACHE_40BIT_BAR0),
> -
> adev->firmware.ucode[AMDGPU_UCODE_ID_VCE].mc_addr >> 8);
> - MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
> mmVCE_LMI_VCPU_CACHE_40BIT_BAR1),
> -
> adev->firmware.ucode[AMDGPU_UCODE_ID_VCE].mc_addr >> 8);
> - MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
> mmVCE_LMI_VCPU_CACHE_40BIT_BAR2),
> + MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
> +
> mmVCE_LMI_VCPU_CACHE_40BIT_BAR0),
>
> adev->firmware.ucode[AMDGPU_UCODE_ID_VCE].mc_addr >> 8);
> + MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
> +
> mmVCE_LMI_VCPU_CACHE_64BIT_BAR0),
> +
> (adev->firmware.ucode[AMDGPU_UCODE_ID_VCE].mc_addr >> 40) &
> +0xff);
> } else {
> - MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
> mmVCE_LMI_VCPU_CACHE_40BIT_BAR0),
> + MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
> +
> mmVCE_LMI_VCPU_CACHE_40BIT_BAR0),
> adev->vce.gpu_addr >> 8);
> - MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
> mmVCE_LMI_VCPU_CACHE_40BIT_BAR1),
> + MMSCH_V1_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCE, 0,
> +
> mmVCE_
