[PATCH] drm/amdgpu: atombios stuck executing during S3 stress test

2016-10-19 Thread jimqu
Sometimes, atombios may be stucked when resume back. Duo to PCIE has
set to D3cold, the ACC of GFX adapter is power-down. it should re-init
the scratch registers. with the change, test is pass more than 80 cycles
on Fiji.

Change-Id: I9cf40c475c4a5a31216949d50cc5e30b74dc21c4
Signed-off-by: JimQu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index e82c487..bfe9a5c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2098,6 +2098,7 @@ int amdgpu_device_resume(struct drm_device *dev, bool 
resume, bool fbcon)
console_unlock();
return r;
}
+   amdgpu_atombios_scratch_regs_init(adev);
}
 
/* post card */
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH xf86-video-amdgpu] Order unique chipsets according to first appearance in ati_pciids.csv

2016-10-19 Thread Michel Dänzer
From: Michel Dänzer 

Instead of lexically. This makes it more likely for similar generations
to be close to each other in the list of unique chipsets.

(Ported from radeon commit 1ce1b1656acc6211deb2091ff7f28d51b6daf86b,
 plus change $numunique++ => ++$numunique to fix OLAND getting listed
 twice)

Signed-off-by: Michel Dänzer 
---
 src/amdgpu_chipset_gen.h   | 26 +-
 src/pcidb/parse_pci_ids.pl |  9 +++--
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/src/amdgpu_chipset_gen.h b/src/amdgpu_chipset_gen.h
index 3bfab87..1598941 100644
--- a/src/amdgpu_chipset_gen.h
+++ b/src/amdgpu_chipset_gen.h
@@ -195,22 +195,22 @@ SymTabRec AMDGPUChipsets[] = {
 };
 
 SymTabRec AMDGPUUniqueChipsets[] = {
-  { 0, "BONAIRE" },
-  { 0, "CARRIZO" },
-  { 0, "FIJI" },
+  { 0, "OLAND" },
   { 0, "HAINAN" },
-  { 0, "HAWAII" },
+  { 0, "TAHITI" },
+  { 0, "PITCAIRN" },
+  { 0, "VERDE" },
+  { 0, "BONAIRE" },
   { 0, "KABINI" },
-  { 0, "KAVERI" },
   { 0, "MULLINS" },
-  { 0, "OLAND" },
-  { 0, "PITCAIRN" },
-  { 0, "POLARIS10" },
-  { 0, "POLARIS11" },
-  { 0, "STONEY" },
-  { 0, "TAHITI" },
-  { 0, "TONGA" },
+  { 0, "KAVERI" },
+  { 0, "HAWAII" },
   { 0, "TOPAZ" },
-  { 0, "VERDE" },
+  { 0, "TONGA" },
+  { 0, "CARRIZO" },
+  { 0, "FIJI" },
+  { 0, "STONEY" },
+  { 0, "POLARIS11" },
+  { 0, "POLARIS10" },
   { -1, NULL }
 };
diff --git a/src/pcidb/parse_pci_ids.pl b/src/pcidb/parse_pci_ids.pl
index 9b6c6f2..1234d79 100755
--- a/src/pcidb/parse_pci_ids.pl
+++ b/src/pcidb/parse_pci_ids.pl
@@ -17,6 +17,8 @@ my $amdgpuchipsetfile = 'amdgpu_chipset_gen.h';
 my $amdgpuchipinfofile  = 'amdgpu_chipinfo_gen.h';
 
 my %uniquechipsets;
+my @uniquearray;
+my $numunique = 0;
 
 my $csv = Text::CSV_XS->new();
 
@@ -50,7 +52,10 @@ while () {
print PCIDEVICEMATCH " ATI_DEVICE_MATCH( PCI_CHIP_$columns[1], 0 ),\n";
 
print AMDGPUCHIPSET "  { PCI_CHIP_$columns[1], \"$columns[3]\" },\n";
-   $uniquechipsets{$columns[3]} = 1;
+   if (!$uniquechipsets{$columns[3]}) {
+   $uniquearray[$numunique] = $columns[3];
+   $uniquechipsets{$columns[3]} = ++$numunique;
+   }
 
print AMDGPUCHIPINFO " { $columns[0], CHIP_FAMILY_$columns[2] },\n";
   }
@@ -63,7 +68,7 @@ while () {
 
 print AMDGPUCHIPINFO "};\n";
 print AMDGPUCHIPSET "  { -1, NULL }\n};\n\nSymTabRec 
AMDGPUUniqueChipsets[] = {\n";
-foreach (sort keys %uniquechipsets) {
+foreach (@uniquearray) {
print AMDGPUCHIPSET "  { 0, \"$_\" },\n";
 }
 print AMDGPUCHIPSET "  { -1, NULL }\n};\n";
-- 
2.9.3

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH xf86-video-amdgpu] amdgpu_probe: Do not close server managed drm fds

2016-10-19 Thread Hans de Goede

Hi,

On 19-10-16 04:42, Michel Dänzer wrote:


[ Moving to the amd-gfx mailing list, where xf86-video-amdgpu (and -ati)
patches are reviewed ]

On 18/10/16 11:48 PM, Hans de Goede wrote:

This fixes the xserver only seeing AMD/ATI devices supported by the amdgpu
driver, as by the time xf86-video-ati gets a chance to probe them, the
fd has been closed.


That basically makes sense, but I have one question: Why does amdgpu get
probed in the first place in that case? I was under the impression that
this should only happen for GPUs bound to the amdgpu kernel driver, due
to Section "OutputClass" in /usr/share/X11/xorg.conf.d/10-amdgpu.conf .


Good question, I honestly don't know and I do not have time to investigate.
You should be able to reproduce this yourself, if not let me know.


diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index 213d245..5d17fe5 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -142,6 +142,15 @@ static int amdgpu_kernel_open_fd(ScrnInfoPtr pScrn, char 
*busid,
return fd;
 }

+static void amdgpu_kernel_close_fd(AMDGPUEntPtr pAMDGPUEnt)
+{
+#ifdef XF86_PDEV_SERVER_FD
+   if (!(pAMDGPUEnt->platform_dev &&
+ pAMDGPUEnt->platform_dev->flags & XF86_PDEV_SERVER_FD))
+#endif
+   drmClose(pAMDGPUEnt->fd);
+}


AMDGPUFreeRec could also be refactored to call this function.


Ack.


@@ -164,7 +173,7 @@ static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn, 
AMDGPUEntPtr pAMDGPUEnt,
if (err != 0) {
xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
   "[drm] failed to set drm interface version.\n");
-   drmClose(pAMDGPUEnt->fd);
+   amdgpu_kernel_close_fd(pAMDGPUEnt);
return FALSE;
}
@@ -252,7 +261,7 @@ static Bool amdgpu_get_scrninfo(int entity_num, struct 
pci_device *pci_dev)
return TRUE;

 error_amdgpu:
-   drmClose(pAMDGPUEnt->fd);
+   amdgpu_kernel_close_fd(pAMDGPUEnt);
 error_fd:
free(pPriv->ptr);
 error:


These two hunks aren't really necessary, right? These only get called
from amdgpu_pci_probe, in which case pAMDGPUEnt->platform_dev remains NULL.


The names of the functions do not imply amdgpu_pci_probe() use only, so even
though that is true today it might not be true in the future. IOW better
safe then sorry.


@@ -347,6 +356,7 @@ amdgpu_platform_probe(DriverPtr pDriver,

pPriv->ptr = xnfcalloc(sizeof(AMDGPUEntRec), 1);
pAMDGPUEnt = pPriv->ptr;
+   pAMDGPUEnt->platform_dev = dev;
pAMDGPUEnt->fd = amdgpu_kernel_open_fd(pScrn, busid, dev);
if (pAMDGPUEnt->fd < 0)
goto error_fd;
@@ -365,7 +375,6 @@ amdgpu_platform_probe(DriverPtr pDriver,
pAMDGPUEnt = pPriv->ptr;
pAMDGPUEnt->fd_ref++;
}
-   pAMDGPUEnt->platform_dev = dev;

xf86SetEntityInstanceForScreen(pScrn, pEnt->index,
   xf86GetNumEntityInstances(pEnt->


These two hunks aren't really necessary either, are they?


Actually they are, quoting some more of the (new after the patch) code:

pAMDGPUEnt->platform_dev = dev;
pAMDGPUEnt->fd = amdgpu_kernel_open_fd(pScrn, busid, dev);
if (pAMDGPUEnt->fd < 0)
goto error_fd;

pAMDGPUEnt->fd_ref = 1;

if (amdgpu_device_initialize(pAMDGPUEnt->fd,
 &major_version,
 &minor_version,
 &pAMDGPUEnt->pDev)) {
xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
   "amdgpu_device_initialize failed\n");
goto error_amdgpu;
}

...

error_amdgpu:
amdgpu_kernel_close_fd(pAMDGPUEnt);
error_fd:
free(pPriv->ptr);
error:
free(busid);
return FALSE;
}

And amdgpu_kernel_close_fd accesses pAMDGPUEnt->platform_dev, so it needs
to be set earlier then before.

Shall I send a v2 with AMDGPUFreeRec refactored to call amdgpu_kernel_close_fd
and the rest kept as is ?

Regards,

Hans
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: mm: fix cache mode tracking in vm_insert_mixed() breaks AMDGPU [was: Re: Latest testing with drm-next-4.9-wip and latest LLVM/mesa stack - Regression in PowerPlay/DPM on CIK?]

2016-10-19 Thread Marek Olšák
On Wed, Oct 19, 2016 at 8:42 AM, Dave Airlie  wrote:
> On 18 October 2016 at 23:53, Dan Williams  wrote:
>> On Mon, Oct 17, 2016 at 8:48 PM, Dave Airlie  wrote:
>> [..]
> Aren't there only 2 possibilities for this regression?
>
> 1/ a memtype entry was never made so track_pfn_insert() returns an
> uncached mapping
>
> 2/ a conflicting memtype entry exists and undefined behavior due to
> mixed mapping types is avoided with the change.

 3/ The CPU usage through this path goes up, and slows things down,
 though I suspect you it's more an uncached mapping showing up
 when we don't expect it.
>>>
>>> It's looking line number 1, there is no mapping, now we get uncached
>>> where we used to get write through.
>>>
>>> difference in page prot 7f7bbc0e, pfn 200e71e4,
>>> 8037, 802f
>>>
>>> 0x2f is the vma pg prot which has PWT set in it, 0x37 is the returned
>>> pgprot which lacks that bit.
>>>
>>> not sure where to go from here, suggestions?
>>
>> If the driver established an ioremap_wt() across the range, or just
>> called reserve_memtype() directly that should restore WT mappings.
>>
>> Although Daniel's suggestion to use the i915 mapping helpers sounds
>> like it avoids problem 3/ as well.
>
> Well we shouldn't be doing that many VRAM mappings on the CPU so
> I doubt we'll hit the overheads here that often.
>
> Ideally we'd always use DMA to move stuff in/out of VRAM, but there
> are some places where we still do WC VRAM writes for uploads.

WC VRAM for uploads is better than WC GART IMO.

Marek
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amd/amdgpu: Allow broadcast on debugfs read (v2)

2016-10-19 Thread StDenis, Tom
The broadcast read was required to attempt to debug a pro-stack problem which I 
believe the "cache gca config" patches are meant to mitigate.  The broadcast 
write support is required to select CUs for wave readings.  libdrm performs a 
broadcast read when reading raster config data so apparently that's a thing.

Since it's a debugfs interface any stability problems a broadcast read may or 
may not cause isn't really a concern for day to day operations.  FWIW I've 
never yet seen an issue with broadcast reading raster config registers.

But to put it simply, the patch was part of a series that was blocking other 
work and there wasn't sufficient cause to NAK it.

Tom


From: Michel Dänzer 
Sent: Tuesday, October 18, 2016 23:49
To: StDenis, Tom; Christian König
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/amdgpu: Allow broadcast on debugfs read (v2)

On 13/10/16 04:20 PM, Michel Dänzer wrote:
> On 13/10/16 12:39 AM, StDenis, Tom wrote:
>> It comes from amdgpu_query_gpu_info_init()
>>
>>
>> for (i = 0; i < (int)dev->info.num_shader_engines; i++) {
>> unsigned instance = (i << AMDGPU_INFO_MMR_SE_INDEX_SHIFT) |
>> (*AMDGPU_INFO_MMR_SH_INDEX_MASK*<<
>>  AMDGPU_INFO_MMR_SH_INDEX_SHIFT);
>>
>> r = amdgpu_read_mm_registers(dev, 0x263d, 1, instance, 0,
>>  &dev->info.backend_disable[i]);
>>
>> This effectively reads from 0/* where the kernel adds the instance of *
>> so it's 0/*/*.  That line was last changed  by Alex
>>
>> *0936139536380* (Alex Deucher  2015-04-20 12:04:22 -0400 174)
>>   (AMDGPU_INFO_MMR_SH_INDEX_MASK <<
>
> As a side note, following that to the end in the kernel code, I noticed
> an interesting minor difference between the AMDGPU_INFO_READ_MMR_REG
> functionality used by this code and the debugfs interface:
>
> With AMDGPU_INFO_READ_MMR_REG, the effect is that
> amdgpu_asic_read_register() doesn't call amdgpu_gfx_select_se_sh() at
> all before reading the register, so the read is performed from whichever
> SH instance is currently selected.
>
> Whereas with this patch, amdgpu_debugfs_regs_read() calls
> amdgpu_gfx_select_se_sh(adev, se_bank, 0x, instance_bank) before
> the register read, which translates to only the SH_BROADCAST_WRITES bit
> being set for the SH instance index.
>
> The end result should be the same though, since
> amdgpu_gfx_select_se_sh(adev, 0x, 0x, 0x) is
> normally called after every register read.
>
>
>> I still don't get why this is a reason to hit pause on the patch(es)
>> though.
>
> At the very least, it should be documented in an appropriate place
> (commit log and/or code, or any other place where the debugfs interface
> semantics are documented) what actually happens when passing all ones
> for the SE/SH index. Does the hardware ignore the *_BROADCAST_WRITES bit
> for reads, so they're performed from instance 0, or does it combine the
> values from all instances with logical and/or?

I'm not sure how to interpret the fact that this patch has landed
without any changes or followups.

FWIW, I'm still interested in (pointers to) information about what the
libdrm_amdgpu code above expects and what the hardware does for reads
with the broadcast bit enabled, from anyone.


--
Earthling Michel Dänzer   |   http://www.amd.com
Graphics, Processors and Immersive VR Solutions | AMD 
www.amd.com
Explore a wide range of innovative next generation computing processors, 
graphics, and Immersive VR solutions by Advanced Micro Devices (AMD). Visit 
AMD.com now!



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: [PATCH] drm/amd/powerplay: don't give up if DPM is already running

2016-10-19 Thread Zhu, Rex
Hi Gražvydas,

Sorry for so late response.

I can reproduce this issue in gpu passthrough case.

When we forced power off the VM or forced reset the VM without shutting down 
the Os, the rmmod will not be called. So dpm was still running.

If we skipped the enable dpm tasks, some clock tables were not be initialized, 
so kernel panic when we visited those tables in set power state.

I will send the fix patch for review tomorrow.

Thanks.

Best Regards
Rex

-Original Message-
From: Grazvydas Ignotas [mailto:nota...@gmail.com] 
Sent: Sunday, October 16, 2016 2:55 AM
To: Zhu, Rex
Cc: Alex Deucher; amd-gfx list; Maling list - DRI developers
Subject: Re: [PATCH] drm/amd/powerplay: don't give up if DPM is already running

Hi,

On Thu, Oct 13, 2016 at 10:45 AM, Zhu, Rex  wrote:
>
> The attached patches were also for this issue.
> Disable dpm when rmmod amdgpu.

It works for modprobe-rmmod-modprobe test, thanks.

However with GPU passthrough (giving control of the GPU to a Windows virtual 
machine using iommu, then shutting down the VM and loading
amdgpu) the problem is still there, same backtrace as in my commit message. It 
seems the Windows driver leaves DPM enabled on shutdown.
With my patch the crash goes away.

It would be nice to have GPU passthrough working, this is the only issue that 
breaks it. Windows can drive the GPU after amdgpu fine already, only amdgpu has 
problems to run after Windows.

Gražvydas
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH xf86-video-amdgpu] Order unique chipsets according to first appearance in ati_pciids.csv

2016-10-19 Thread Alex Deucher
On Wed, Oct 19, 2016 at 5:28 AM, Michel Dänzer  wrote:
> From: Michel Dänzer 
>
> Instead of lexically. This makes it more likely for similar generations
> to be close to each other in the list of unique chipsets.
>
> (Ported from radeon commit 1ce1b1656acc6211deb2091ff7f28d51b6daf86b,
>  plus change $numunique++ => ++$numunique to fix OLAND getting listed
>  twice)
>
> Signed-off-by: Michel Dänzer 

Reviewed-by: Alex Deucher 

> ---
>  src/amdgpu_chipset_gen.h   | 26 +-
>  src/pcidb/parse_pci_ids.pl |  9 +++--
>  2 files changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/src/amdgpu_chipset_gen.h b/src/amdgpu_chipset_gen.h
> index 3bfab87..1598941 100644
> --- a/src/amdgpu_chipset_gen.h
> +++ b/src/amdgpu_chipset_gen.h
> @@ -195,22 +195,22 @@ SymTabRec AMDGPUChipsets[] = {
>  };
>
>  SymTabRec AMDGPUUniqueChipsets[] = {
> -  { 0, "BONAIRE" },
> -  { 0, "CARRIZO" },
> -  { 0, "FIJI" },
> +  { 0, "OLAND" },
>{ 0, "HAINAN" },
> -  { 0, "HAWAII" },
> +  { 0, "TAHITI" },
> +  { 0, "PITCAIRN" },
> +  { 0, "VERDE" },
> +  { 0, "BONAIRE" },
>{ 0, "KABINI" },
> -  { 0, "KAVERI" },
>{ 0, "MULLINS" },
> -  { 0, "OLAND" },
> -  { 0, "PITCAIRN" },
> -  { 0, "POLARIS10" },
> -  { 0, "POLARIS11" },
> -  { 0, "STONEY" },
> -  { 0, "TAHITI" },
> -  { 0, "TONGA" },
> +  { 0, "KAVERI" },
> +  { 0, "HAWAII" },
>{ 0, "TOPAZ" },
> -  { 0, "VERDE" },
> +  { 0, "TONGA" },
> +  { 0, "CARRIZO" },
> +  { 0, "FIJI" },
> +  { 0, "STONEY" },
> +  { 0, "POLARIS11" },
> +  { 0, "POLARIS10" },
>{ -1, NULL }
>  };
> diff --git a/src/pcidb/parse_pci_ids.pl b/src/pcidb/parse_pci_ids.pl
> index 9b6c6f2..1234d79 100755
> --- a/src/pcidb/parse_pci_ids.pl
> +++ b/src/pcidb/parse_pci_ids.pl
> @@ -17,6 +17,8 @@ my $amdgpuchipsetfile = 'amdgpu_chipset_gen.h';
>  my $amdgpuchipinfofile  = 'amdgpu_chipinfo_gen.h';
>
>  my %uniquechipsets;
> +my @uniquearray;
> +my $numunique = 0;
>
>  my $csv = Text::CSV_XS->new();
>
> @@ -50,7 +52,10 @@ while () {
> print PCIDEVICEMATCH " ATI_DEVICE_MATCH( PCI_CHIP_$columns[1], 0 
> ),\n";
>
> print AMDGPUCHIPSET "  { PCI_CHIP_$columns[1], \"$columns[3]\" },\n";
> -   $uniquechipsets{$columns[3]} = 1;
> +   if (!$uniquechipsets{$columns[3]}) {
> +   $uniquearray[$numunique] = $columns[3];
> +   $uniquechipsets{$columns[3]} = ++$numunique;
> +   }
>
> print AMDGPUCHIPINFO " { $columns[0], CHIP_FAMILY_$columns[2] },\n";
>}
> @@ -63,7 +68,7 @@ while () {
>
>  print AMDGPUCHIPINFO "};\n";
>  print AMDGPUCHIPSET "  { -1, NULL }\n};\n\nSymTabRec 
> AMDGPUUniqueChipsets[] = {\n";
> -foreach (sort keys %uniquechipsets) {
> +foreach (@uniquearray) {
> print AMDGPUCHIPSET "  { 0, \"$_\" },\n";
>  }
>  print AMDGPUCHIPSET "  { -1, NULL }\n};\n";
> --
> 2.9.3
>
> ___
> 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/amdgpu: explicitly set pg_flags for ST

2016-10-19 Thread Alex Deucher
No need to retain previous settings as this is the first time
we set pg_flags.  Probably a copy/paste typo from the CZ code.
Avoids confusion.

No change in behavior as adev is kzallocated.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/vi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index e33399c..b2a1cf4 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -1000,7 +1000,7 @@ static int vi_common_early_init(void *handle)
AMD_CG_SUPPORT_SDMA_MGCG |
AMD_CG_SUPPORT_SDMA_LS |
AMD_CG_SUPPORT_VCE_MGCG;
-   adev->pg_flags |= AMD_PG_SUPPORT_GFX_PG |
+   adev->pg_flags = AMD_PG_SUPPORT_GFX_PG |
AMD_PG_SUPPORT_GFX_SMG |
AMD_PG_SUPPORT_GFX_PIPELINE |
AMD_PG_SUPPORT_UVD |
-- 
2.5.5

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdgpu: explicitly set pg_flags for ST

2016-10-19 Thread StDenis, Tom

Reviewed-by: Tom St Denis 


From: amd-gfx  on behalf of Alex Deucher 

Sent: Wednesday, October 19, 2016 13:10
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander
Subject: [PATCH] drm/amdgpu: explicitly set pg_flags for ST

No need to retain previous settings as this is the first time
we set pg_flags.  Probably a copy/paste typo from the CZ code.
Avoids confusion.

No change in behavior as adev is kzallocated.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/vi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index e33399c..b2a1cf4 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -1000,7 +1000,7 @@ static int vi_common_early_init(void *handle)
 AMD_CG_SUPPORT_SDMA_MGCG |
 AMD_CG_SUPPORT_SDMA_LS |
 AMD_CG_SUPPORT_VCE_MGCG;
-   adev->pg_flags |= AMD_PG_SUPPORT_GFX_PG |
+   adev->pg_flags = AMD_PG_SUPPORT_GFX_PG |
 AMD_PG_SUPPORT_GFX_SMG |
 AMD_PG_SUPPORT_GFX_PIPELINE |
 AMD_PG_SUPPORT_UVD |
--
2.5.5

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
amd-gfx Info Page - 
lists.freedesktop.org
lists.freedesktop.org
To see the collection of prior postings to the list, visit the amd-gfx 
Archives. Using amd-gfx: To post a message to all the list members, send email 
...



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdgpu: atombios stuck executing during S3 stress test

2016-10-19 Thread Alex Deucher
On Wed, Oct 19, 2016 at 4:25 AM, jimqu  wrote:
> Sometimes, atombios may be stucked when resume back. Duo to PCIE has
> set to D3cold, the ACC of GFX adapter is power-down. it should re-init
> the scratch registers. with the change, test is pass more than 80 cycles
> on Fiji.
>
> Change-Id: I9cf40c475c4a5a31216949d50cc5e30b74dc21c4
> Signed-off-by: JimQu 

I think we probably want to save/restore these, although this
shouldn't hurt.  How about this patch?

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index e82c487..bfe9a5c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2098,6 +2098,7 @@ int amdgpu_device_resume(struct drm_device *dev, bool 
> resume, bool fbcon)
> console_unlock();
> return r;
> }
> +   amdgpu_atombios_scratch_regs_init(adev);
> }
>
> /* post card */
> --
> 1.9.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
From bf70713edda60da5713e4acd8f1615144e02abe2 Mon Sep 17 00:00:00 2001
From: Alex Deucher 
Date: Wed, 19 Oct 2016 14:40:58 -0400
Subject: [PATCH] drm/amdgpu: move atom scratch register save/restore to common
 code

We need this for more than just DCE.  Move it out of the DCE modules
and into the device code.  This way we can be sure the scratch registers
are initialized properly before we run asic_init which happens before
DCE IPs are restored.

Fixes atombios hangs in asic_init.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 -
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 6 --
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 6 --
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c  | 6 --
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c  | 6 --
 5 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index eb1b9e39..b7b6542 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2004,6 +2004,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool suspend, bool fbcon)
 	 */
 	amdgpu_bo_evict_vram(adev);
 
+	amdgpu_atombios_scratch_regs_save(adev);
 	pci_save_state(dev->pdev);
 	if (suspend) {
 		/* Shut down the device */
@@ -2055,6 +2056,7 @@ int amdgpu_device_resume(struct drm_device *dev, bool resume, bool fbcon)
 			return r;
 		}
 	}
+	amdgpu_atombios_scratch_regs_restore(adev);
 
 	/* post card */
 	if (!amdgpu_card_posted(adev) || !resume) {
@@ -2322,8 +2324,9 @@ retry:
 			amdgpu_display_stop_mc_access(adev, &save);
 			amdgpu_wait_for_idle(adev, AMD_IP_BLOCK_TYPE_GMC);
 		}
-
+		amdgpu_atombios_scratch_regs_save(adev);
 		r = amdgpu_asic_reset(adev);
+		amdgpu_atombios_scratch_regs_restore(adev);
 		/* post card */
 		amdgpu_atom_asic_init(adev->mode_info.atom_context);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index e5d0edf..0c01852 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -3067,10 +3067,6 @@ static int dce_v10_0_hw_fini(void *handle)
 
 static int dce_v10_0_suspend(void *handle)
 {
-	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-
-	amdgpu_atombios_scratch_regs_save(adev);
-
 	return dce_v10_0_hw_fini(handle);
 }
 
@@ -3081,8 +3077,6 @@ static int dce_v10_0_resume(void *handle)
 
 	ret = dce_v10_0_hw_init(handle);
 
-	amdgpu_atombios_scratch_regs_restore(adev);
-
 	/* turn on the BL */
 	if (adev->mode_info.bl_encoder) {
 		u8 bl_level = amdgpu_display_backlight_get_level(adev,
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 9719b2b..2946674 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -3131,10 +3131,6 @@ static int dce_v11_0_hw_fini(void *handle)
 
 static int dce_v11_0_suspend(void *handle)
 {
-	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-
-	amdgpu_atombios_scratch_regs_save(adev);
-
 	return dce_v11_0_hw_fini(handle);
 }
 
@@ -3145,8 +3141,6 @@ static int dce_v11_0_resume(void *handle)
 
 	ret = dce_v11_0_hw_init(handle);
 
-	amdgpu_atombios_scratch_regs_restore(adev);
-
 	/* turn on the BL */
 	if (adev->mode_info.bl_encoder) {
 		u8 bl_level = amdgpu_display_backlight_get_level(adev,
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index 44547f9..5742333 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -2403,10 +2403,6 @@ static int dce_v6_0_hw_fini(void *handle)
 
 static int dce_v6_0_suspend(void *handle)

Re: [PATCH xf86-video-amdgpu] amdgpu_probe: Do not close server managed drm fds

2016-10-19 Thread Michel Dänzer
On 19/10/16 06:49 PM, Hans de Goede wrote:
> On 19-10-16 04:42, Michel Dänzer wrote:
>> On 18/10/16 11:48 PM, Hans de Goede wrote:
>>> This fixes the xserver only seeing AMD/ATI devices supported by the
>>> amdgpu
>>> driver, as by the time xf86-video-ati gets a chance to probe them, the
>>> fd has been closed.
>>
>> That basically makes sense, but I have one question: Why does amdgpu get
>> probed in the first place in that case? I was under the impression that
>> this should only happen for GPUs bound to the amdgpu kernel driver, due
>> to Section "OutputClass" in /usr/share/X11/xorg.conf.d/10-amdgpu.conf .
> 
> Good question, I honestly don't know and I do not have time to investigate.
> You should be able to reproduce this yourself, if not let me know.

I haven't run into this myself and am not sure how I could reproduce it.
Anyway, the patch is clearly the right thing to do regardless, so it's
not a big deal but just curiosity on my part.


>>> @@ -164,7 +173,7 @@ static Bool amdgpu_open_drm_master(ScrnInfoPtr
>>> pScrn, AMDGPUEntPtr pAMDGPUEnt,
>>>  if (err != 0) {
>>>  xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
>>> "[drm] failed to set drm interface version.\n");
>>> -drmClose(pAMDGPUEnt->fd);
>>> +amdgpu_kernel_close_fd(pAMDGPUEnt);
>>>  return FALSE;
>>>  }
>>> @@ -252,7 +261,7 @@ static Bool amdgpu_get_scrninfo(int entity_num,
>>> struct pci_device *pci_dev)
>>>  return TRUE;
>>>
>>>  error_amdgpu:
>>> -drmClose(pAMDGPUEnt->fd);
>>> +amdgpu_kernel_close_fd(pAMDGPUEnt);
>>>  error_fd:
>>>  free(pPriv->ptr);
>>>  error:
>>
>> These two hunks aren't really necessary, right? These only get called
>> from amdgpu_pci_probe, in which case pAMDGPUEnt->platform_dev remains
>> NULL.
> 
> The names of the functions do not imply amdgpu_pci_probe() use only, so
> even
> though that is true today it might not be true in the future. IOW better
> safe then sorry.

Makes sense.


>>> @@ -347,6 +356,7 @@ amdgpu_platform_probe(DriverPtr pDriver,
>>>
>>>  pPriv->ptr = xnfcalloc(sizeof(AMDGPUEntRec), 1);
>>>  pAMDGPUEnt = pPriv->ptr;
>>> +pAMDGPUEnt->platform_dev = dev;
>>>  pAMDGPUEnt->fd = amdgpu_kernel_open_fd(pScrn, busid, dev);
>>>  if (pAMDGPUEnt->fd < 0)
>>>  goto error_fd;
>>> @@ -365,7 +375,6 @@ amdgpu_platform_probe(DriverPtr pDriver,
>>>  pAMDGPUEnt = pPriv->ptr;
>>>  pAMDGPUEnt->fd_ref++;
>>>  }
>>> -pAMDGPUEnt->platform_dev = dev;
>>>
>>>  xf86SetEntityInstanceForScreen(pScrn, pEnt->index,
>>> xf86GetNumEntityInstances(pEnt->
>>
>> These two hunks aren't really necessary either, are they?
> 
> Actually they are, quoting some more of the (new after the patch) code:

[...]

> And amdgpu_kernel_close_fd accesses pAMDGPUEnt->platform_dev, so it needs
> to be set earlier then before.

Gotcha, thanks.


> Shall I send a v2 with AMDGPUFreeRec refactored to call
> amdgpu_kernel_close_fd
> and the rest kept as is ?

Yes, please.


-- 
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: mm: fix cache mode tracking in vm_insert_mixed() breaks AMDGPU [was: Re: Latest testing with drm-next-4.9-wip and latest LLVM/mesa stack - Regression in PowerPlay/DPM on CIK?]

2016-10-19 Thread Michel Dänzer
On 19/10/16 07:33 PM, Marek Olšák wrote:
> On Wed, Oct 19, 2016 at 8:42 AM, Dave Airlie  wrote:
>> On 18 October 2016 at 23:53, Dan Williams  wrote:
>>> On Mon, Oct 17, 2016 at 8:48 PM, Dave Airlie  wrote:
>>> [..]
>> Aren't there only 2 possibilities for this regression?
>>
>> 1/ a memtype entry was never made so track_pfn_insert() returns an
>> uncached mapping
>>
>> 2/ a conflicting memtype entry exists and undefined behavior due to
>> mixed mapping types is avoided with the change.
>
> 3/ The CPU usage through this path goes up, and slows things down,
> though I suspect you it's more an uncached mapping showing up
> when we don't expect it.

 It's looking line number 1, there is no mapping, now we get uncached
 where we used to get write through.

 difference in page prot 7f7bbc0e, pfn 200e71e4,
 8037, 802f

 0x2f is the vma pg prot which has PWT set in it, 0x37 is the returned
 pgprot which lacks that bit.

 not sure where to go from here, suggestions?
>>>
>>> If the driver established an ioremap_wt() across the range, or just
>>> called reserve_memtype() directly that should restore WT mappings.
>>>
>>> Although Daniel's suggestion to use the i915 mapping helpers sounds
>>> like it avoids problem 3/ as well.
>>
>> Well we shouldn't be doing that many VRAM mappings on the CPU so
>> I doubt we'll hit the overheads here that often.
>>
>> Ideally we'd always use DMA to move stuff in/out of VRAM, but there
>> are some places where we still do WC VRAM writes for uploads.
> 
> WC VRAM for uploads is better than WC GART IMO.

It's not a simple choice I'm afraid. While writing directly to WC VRAM
can be faster than writing to WC GART and then DMA'ing to VRAM, doing so
increases pressure on the first 256MB of VRAM. That's why I disabled
direct VRAM writes for streaming uploads again in
https://cgit.freedesktop.org/mesa/mesa/commit/?id=7b4276d7acf2e0f77044cb50caa6ad936fa78786
. It's possible that something has changed since then though, feel free
to play with enabling it again.


-- 
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


[PATCH xf86-video-ati] Sayōnara, AM_MAINTAINER_MODE!

2016-10-19 Thread Michel Dänzer
From: Michel Dänzer 

If --enable-maintainer-mode got lost from config.status for any reason,
builds would fail in mysterious ways after changing between different
Git commits.

There are more reasons for dropping it in the automake manual:

https://www.gnu.org/software/automake/manual/html_node/maintainer_002dmode.html

I'm not aware of any reason why --disable-maintainer-mode would ever be
useful with this project.

Signed-off-by: Michel Dänzer 
---
 autogen.sh   | 2 +-
 configure.ac | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/autogen.sh b/autogen.sh
index b47abdc..fc34bd5 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -10,5 +10,5 @@ autoreconf -v --install || exit 1
 cd $ORIGDIR || exit $?
 
 if test -z "$NOCONFIGURE"; then
-$srcdir/configure --enable-maintainer-mode "$@"
+$srcdir/configure "$@"
 fi
diff --git a/configure.ac b/configure.ac
index 8dc55d8..3fc967e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,8 +36,6 @@ AC_CONFIG_AUX_DIR(.)
 AM_INIT_AUTOMAKE([foreign dist-bzip2])
 AC_SYS_LARGEFILE
 
-AM_MAINTAINER_MODE
-
 # Require X.Org macros 1.8 or later for MAN_SUBSTS set by XORG_MANPAGE_SECTIONS
 m4_ifndef([XORG_MACROS_VERSION],
   [m4_fatal([must install xorg-macros 1.8 or later before running 
autoconf/autogen])])
-- 
2.9.3

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx