date:20191023

Re: [PATCH] drm/amd/powerplay: skip unsupported clock limit settings on Arcturus V2

2019-10-23 Thread Alex Deucher

On Wed, Oct 23, 2019 at 4:53 AM Quan, Evan  wrote:
>
> For Arcturus, clock limit settings on uclk/socclk/fclk domains
> are not supported.
>
> V2: simplify the code to support both SGPU and MGPU cases
>
> Change-Id: I1286289e3770f0421f0d22989437e26d3f7b2ec4
> Signed-off-by: Evan Quan 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c   |  13 ++
>  drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 142 ---
>  2 files changed, 39 insertions(+), 116 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 36f36b35000d..660efe009749 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -2881,6 +2881,19 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
> DRM_ERROR("failed to create device file pp_dpm_sclk\n");
> return ret;
> }
> +
> +   /* Arcturus does not support standalone mclk/socclk/fclk level 
> setting */
> +   if (adev->asic_type == CHIP_ARCTURUS) {
> +   dev_attr_pp_dpm_mclk.attr.mode &= ~S_IWUGO;
> +   dev_attr_pp_dpm_mclk.store = NULL;
> +
> +   dev_attr_pp_dpm_socclk.attr.mode &= ~S_IWUGO;
> +   dev_attr_pp_dpm_socclk.store = NULL;
> +
> +   dev_attr_pp_dpm_fclk.attr.mode &= ~S_IWUGO;
> +   dev_attr_pp_dpm_fclk.store = NULL;
> +   }
> +
> ret = device_create_file(adev->dev, _attr_pp_dpm_mclk);
> if (ret) {
> DRM_ERROR("failed to create device file pp_dpm_mclk\n");
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c 
> b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index 9a5faac1ceea..1bcc5ab2873d 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -37,6 +37,7 @@
>  #include "smu_v11_0_pptable.h"
>  #include "arcturus_ppsmc.h"
>  #include "nbio/nbio_7_4_sh_mask.h"
> +#include "amdgpu_xgmi.h"
>
>  #define CTF_OFFSET_EDGE5
>  #define CTF_OFFSET_HOTSPOT 5
> @@ -849,84 +850,13 @@ static int arcturus_force_clk_levels(struct smu_context 
> *smu,
> break;
>
> case SMU_MCLK:
> -   single_dpm_table = &(dpm_table->mem_table);
> -
> -   if (soft_max_level >= single_dpm_table->count) {
> -   pr_err("Clock level specified %d is over max allowed 
> %d\n",
> -   soft_max_level, 
> single_dpm_table->count - 1);
> -   ret = -EINVAL;
> -   break;
> -   }
> -
> -   single_dpm_table->dpm_state.soft_min_level =
> -   single_dpm_table->dpm_levels[soft_min_level].value;
> -   single_dpm_table->dpm_state.soft_max_level =
> -   single_dpm_table->dpm_levels[soft_max_level].value;
> -
> -   ret = arcturus_upload_dpm_level(smu, false, 
> FEATURE_DPM_UCLK_MASK);
> -   if (ret) {
> -   pr_err("Failed to upload boot level to lowest!\n");
> -   break;
> -   }
> -
> -   ret = arcturus_upload_dpm_level(smu, true, 
> FEATURE_DPM_UCLK_MASK);
> -   if (ret)
> -   pr_err("Failed to upload dpm max level to 
> highest!\n");
> -
> -   break;
> -
> case SMU_SOCCLK:
> -   single_dpm_table = &(dpm_table->soc_table);
> -
> -   if (soft_max_level >= single_dpm_table->count) {
> -   pr_err("Clock level specified %d is over max allowed 
> %d\n",
> -   soft_max_level, 
> single_dpm_table->count - 1);
> -   ret = -EINVAL;
> -   break;
> -   }
> -
> -   single_dpm_table->dpm_state.soft_min_level =
> -   single_dpm_table->dpm_levels[soft_min_level].value;
> -   single_dpm_table->dpm_state.soft_max_level =
> -   single_dpm_table->dpm_levels[soft_max_level].value;
> -
> -   ret = arcturus_upload_dpm_level(smu, false, 
> FEATURE_DPM_SOCCLK_MASK);
> -   if (ret) {
> -   pr_err("Failed to upload boot level to lowest!\n");
> -   break;
> -   }
> -
> -   ret = arcturus_upload_dpm_level(smu, true, 
> FEATURE_DPM_SOCCLK_MASK);
> -   if (ret)
> -   pr_err("Failed to upload dpm max level to 
> highest!\n");
> -
> -   break;
> -
> case SMU_FCLK:
> -   single_dpm_table = &(dpm_table->fclk_table);
> -
> -   if (soft_max_level >= single_dpm_table->count) {
> -   pr_err("Clock level specified %d is over max allowed 
> %d\n",
> -   soft_max_level, 
> single_dpm_table->count - 1);
> -

[pull] amdgpu drm-fixes-5.4

2019-10-23 Thread Alex Deucher

Hi Dave, Daniel,

Fixes for 5.4 for amdgpu.

The following changes since commit 5c1e34b5159ec65bf33e2c1a62fa7158132c10cf:

  Merge tag 'drm-misc-fixes-2019-10-17' of 
git://anongit.freedesktop.org/drm/drm-misc into drm-fixes (2019-10-18 06:40:28 
+1000)

are available in the Git repository at:

  git://people.freedesktop.org/~agd5f/linux tags/drm-fixes-5.4-2019-10-23

for you to fetch changes up to ee027828c40faa92a7ef4c2b0641bbb3f4be95d3:

  drm/amdgpu/vce: fix allocation size in enc ring test (2019-10-17 17:12:34 
-0400)


drm-fixes-5.4-2019-10-23:

amdgpu:
- Fix suspend/resume issue related to multi-media engines
- Fix memory leak in user ptr code related to hmm conversion
- Fix possible VM faults when allocating page table memory
- Fix error handling in bo list ioctl


Alex Deucher (4):
  drm/amdgpu/uvd6: fix allocation size in enc ring test (v2)
  drm/amdgpu/uvd7: fix allocation size in enc ring test (v2)
  drm/amdgpu/vcn: fix allocation size in enc ring test
  drm/amdgpu/vce: fix allocation size in enc ring test

Christian König (2):
  drm/amdgpu: fix potential VM faults
  drm/amdgpu: fix error handling in amdgpu_bo_list_create

Philip Yang (1):
  drm/amdgpu: user pages array memory leak fix

 drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c |  7 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  8 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  |  3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 20 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 35 +++--
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c   | 31 -
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c   | 33 ++-
 8 files changed, 92 insertions(+), 46 deletions(-)
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] Ring range argument improvements (v2)

2019-10-23 Thread Tuikov, Luben

The valid contents of rings is invariably the
range [rptr, wptr). Augment the ring range to
interpret the '.' ("here") notation to mean rptr
or wptr when given on the left or right of the
range limits. This augments the range notation as
follows:

1) No range given, print the whole ring.

2) [.] or [.:.], print [rptr, wptr],

3) [.:k], k >= 0, print [rptr, rptr + k], this is
a range relative to the left limit, rptr, the
consumer pointer.

4) [k:.] or [k], k >= 0, print [wptr - k, wptr], this is
a range relative to the right limit, wptr, the
producer pointer.

5) [k:r], both greater than 0, print [k,r] of the
named ring. This is an absolute range limit
notation.

In any case, the ring contents is interpreted, if
the ring contents can be interpreted.

v2: Fix spelling mistake in the commit message:
"then" --> "than".

Signed-off-by: Luben Tuikov 
---
 doc/umr.1   | 33 +++-
 src/app/ring_read.c | 52 -
 2 files changed, 55 insertions(+), 30 deletions(-)

diff --git a/doc/umr.1 b/doc/umr.1
index 1e585fa..137ff1e 100644
--- a/doc/umr.1
+++ b/doc/umr.1
@@ -216,17 +216,30 @@ Disassemble 'size' bytes (in hex) from a given address 
(in hex).  The size can b
 specified as zero to have umr try and compute the shader size.
 
 .SH Ring and PM4 Decoding
-.IP "--ring, -R (from:to)"
-Read the contents of a ring named by the string without the
-.B amdgpu_ring_
-prefix.  By default it will read and display the entire ring.  A
-starting and ending address can be specified in decimal or a '.' can
-be used to indicate relative to the current
+.IP "--ring, -R [range]"
+Read the contents of the ring named by the string
+.B amdgpu_ring_,
+i.e. without the
+.B amdgpu_ring
+prefix. By default it reads and prints the entire ring.  A
+range is optional and has the format '[start:end]'. The
+starting and ending address are non-negative integers or
+the '.' (dot) symbol, which indicates the
+.B rptr
+when on the left side and
 .B wptr
-pointer.  For example, "-R gfx" would read the entire gfx ring,
-"-R gfx[0:16]" would display the contents from 0 to 16 inclusively, and
-"-R gfx[.]" or "-R gfx[.:.]" would display the last 32 words relative
-to rptr.
+when on the right side of the range.
+For instance,
+"-R gfx" prints the entire gfx ring, "-R gfx[0:16]" prints
+the contents from 0 to 16 inclusively, and "-R gfx[.]" or
+"-R gfx[.:.]" prints the range [rptr,wptr]. When one of
+the range limits is a number while the other is the dot, '.',
+then the number indicates the relative range before or after the
+corresponding ring pointer. For instance, "-R sdma0[16:.]"
+prints [wptr-16, wptr] words of the SDMA0 ring, and
+"-R sdma1[.:32]" prints [rptr, rptr+32] double-words of the
+SDMA1 ring. The contents of the ring is always interpreted,
+if it can be interpreted.
 .IP "--dump-ib, -di [vmid@]address length [pm]"
 Dump an IB packet at an address with an optional VMID.  The length is specified
 in bytes.  The type of decoder  is optional and defaults to PM4 packets.
diff --git a/src/app/ring_read.c b/src/app/ring_read.c
index ef0c711..9cbecb0 100644
--- a/src/app/ring_read.c
+++ b/src/app/ring_read.c
@@ -28,7 +28,7 @@
 void umr_read_ring(struct umr_asic *asic, char *ringpath)
 {
char ringname[32], from[32], to[32];
-   int use_decoder, enable_decoder, gprs;
+   int  enable_decoder, gprs;
uint32_t wptr, rptr, drv_wptr, ringsize, start, end, value,
 *ring_data;
struct umr_ring_decoder decoder, *pdecoder, *ppdecoder;
@@ -73,33 +73,46 @@ void umr_read_ring(struct umr_asic *asic, char *ringpath)
drv_wptr = ring_data[2]<<2;
 
/* default to reading entire ring */
-   use_decoder = 0;
if (!from[0]) {
start = 0;
end   = ringsize-4;
} else {
-   if (from[0] == '.' || !to[0] || to[0] == '.') {
-   /* start from 32 words prior to rptr up to wptr */
-   end = wptr;
-   if (rptr < (31*4)) {
-   start = rptr - 31*4;
-   start += ringsize;
+   if (from[0] == '.') {
+   if (to[0] == 0 || to[0] == '.') {
+   /* Notation: [.] or [.:.], meaning
+* [rptr, wptr].
+*/
+   start = rptr;
+   end = wptr;
} else {
-   start = rptr - 31*4;
+   /* Notation: [.:k], k >=0, meaning
+* [rptr, rtpr+k] double-words.
+*/
+   start = rptr;
+   sscanf(to, "%"SCNu32, );
+   end *= 4;
+   end = (start + end + ringsize) % ringsize;

[PATCH] Ring range argument improvements

2019-10-23 Thread Tuikov, Luben

The valid contents of rings is invariably the
range [rptr, wptr). Augment the ring range to
interpret the '.' ("here") notation to mean rptr
or wptr when given on the left or right of the
range limits. This augments the range notation as
follows:

1) No range given, print the whole ring.

2) [.] or [.:.], print [rptr, wptr],

3) [.:k], k >= 0, print [rptr, rptr + k], this is
a range relative to the left limit, rptr, the
consumer pointer.

4) [k:.] or [k], k >= 0, print [wptr - k, wptr], this is
a range relative to the right limit, wptr, the
producer pointer.

5) [k:r], both greater then 0, print [k,r] of the
named ring. This is an absolute range limit
notation.

In any case, the ring contents is interpreted, if
the ring contents can be interpreted.

Signed-off-by: Luben Tuikov 
---
 doc/umr.1   | 33 ++
 src/app/ring_read.c | 56 +
 2 files changed, 59 insertions(+), 30 deletions(-)

diff --git a/doc/umr.1 b/doc/umr.1
index 1e585fa..54b6a99 100644
--- a/doc/umr.1
+++ b/doc/umr.1
@@ -216,17 +216,30 @@ Disassemble 'size' bytes (in hex) from a given address 
(in hex).  The size can b
 specified as zero to have umr try and compute the shader size.
 
 .SH Ring and PM4 Decoding
-.IP "--ring, -R (from:to)"
-Read the contents of a ring named by the string without the
-.B amdgpu_ring_
-prefix.  By default it will read and display the entire ring.  A
-starting and ending address can be specified in decimal or a '.' can
-be used to indicate relative to the current
+.IP "--ring, -R [range]"
+Read the contents of the ring named by the string
+.B amdgpu_ring_,
+i.e. without the
+.B amdgpu_ring
+prefix. By default it reads and prints the entire ring.  A
+range is optional and has the format '[start:end]'. The
+starting and ending address are non-negative integers or
+the '.' (dot) symbol, which indicates the
+.B rptr
+when on the left side and
 .B wptr
-pointer.  For example, "-R gfx" would read the entire gfx ring,
-"-R gfx[0:16]" would display the contents from 0 to 16 inclusively, and
-"-R gfx[.]" or "-R gfx[.:.]" would display the last 32 words relative
-to rptr.
+when on the right side of the range.
+For instance,
+"-R gfx" reads the entire gfx ring, "-R gfx[0:16]" prints
+the contents from 0 to 16 inclusively, and "-R gfx[.]" or
+"-R gfx[.:.]" prints the range [rptr:wptr]. When one of
+the range limit is a number while the other is the dot, '.',
+then the number indicates the relative range before or after the
+corresponding ring pointer. For instance, "-R sdma0[16:.]"
+prints [wptr-16, wptr] words of the SDMA0 ring, and
+"-R sdma1[.:32]" prints [rptr, rptr+32] double-words of the
+SDMA1 ring. The contents of the ring is always interpreted,
+if it can be interpreted.
 .IP "--dump-ib, -di [vmid@]address length [pm]"
 Dump an IB packet at an address with an optional VMID.  The length is specified
 in bytes.  The type of decoder  is optional and defaults to PM4 packets.
diff --git a/src/app/ring_read.c b/src/app/ring_read.c
index ef0c711..31ead1e 100644
--- a/src/app/ring_read.c
+++ b/src/app/ring_read.c
@@ -28,7 +28,7 @@
 void umr_read_ring(struct umr_asic *asic, char *ringpath)
 {
char ringname[32], from[32], to[32];
-   int use_decoder, enable_decoder, gprs;
+   int  enable_decoder, gprs;
uint32_t wptr, rptr, drv_wptr, ringsize, start, end, value,
 *ring_data;
struct umr_ring_decoder decoder, *pdecoder, *ppdecoder;
@@ -73,33 +73,50 @@ void umr_read_ring(struct umr_asic *asic, char *ringpath)
drv_wptr = ring_data[2]<<2;
 
/* default to reading entire ring */
-   use_decoder = 0;
if (!from[0]) {
start = 0;
end   = ringsize-4;
} else {
-   if (from[0] == '.' || !to[0] || to[0] == '.') {
-   /* start from 32 words prior to rptr up to wptr */
-   end = wptr;
-   if (rptr < (31*4)) {
-   start = rptr - 31*4;
-   start += ringsize;
+   if (from[0] == '.') {
+   if (to[0] == 0 || to[0] == '.') {
+   /* Notation: [.] or [.:.], meaning
+* [rptr, wptr].
+*/
+   start = rptr;
+   end = wptr;
} else {
-   start = rptr - 31*4;
+   /* Notation: [.:k], k >=0, meaning
+* [rptr, rtpr+k] double-words.
+*/
+   start = rptr;
+   sscanf(to, "%"SCNu32, );
+   end *= 4;
+   end = (start + end + ringsize) % ringsize;
}
-
} else {

Re: [PATCH hmm 00/15] Consolidate the mmu notifier interval_tree and locking

2019-10-23 Thread Jerome Glisse

On Mon, Oct 21, 2019 at 07:06:00PM +, Jason Gunthorpe wrote:
> On Mon, Oct 21, 2019 at 02:40:41PM -0400, Jerome Glisse wrote:
> > On Tue, Oct 15, 2019 at 03:12:27PM -0300, Jason Gunthorpe wrote:
> > > From: Jason Gunthorpe 
> > > 
> > > 8 of the mmu_notifier using drivers (i915_gem, radeon_mn, umem_odp, hfi1,
> > > scif_dma, vhost, gntdev, hmm) drivers are using a common pattern where
> > > they only use invalidate_range_start/end and immediately check the
> > > invalidating range against some driver data structure to tell if the
> > > driver is interested. Half of them use an interval_tree, the others are
> > > simple linear search lists.
> > > 
> > > Of the ones I checked they largely seem to have various kinds of races,
> > > bugs and poor implementation. This is a result of the complexity in how
> > > the notifier interacts with get_user_pages(). It is extremely difficult to
> > > use it correctly.
> > > 
> > > Consolidate all of this code together into the core mmu_notifier and
> > > provide a locking scheme similar to hmm_mirror that allows the user to
> > > safely use get_user_pages() and reliably know if the page list still
> > > matches the mm.
> > > 
> > > This new arrangment plays nicely with the !blockable mode for
> > > OOM. Scanning the interval tree is done such that the intersection test
> > > will always succeed, and since there is no invalidate_range_end exposed to
> > > drivers the scheme safely allows multiple drivers to be subscribed.
> > > 
> > > Four places are converted as an example of how the new API is used.
> > > Four are left for future patches:
> > >  - i915_gem has complex locking around destruction of a registration,
> > >needs more study
> > >  - hfi1 (2nd user) needs access to the rbtree
> > >  - scif_dma has a complicated logic flow
> > >  - vhost's mmu notifiers are already being rewritten
> > > 
> > > This is still being tested, but I figured to send it to start getting help
> > > from the xen, amd and hfi drivers which I cannot test here.
> > 
> > It might be a good oportunity to also switch those users to
> > hmm_range_fault() instead of GUP as GUP is pointless for those
> > users. In fact the GUP is an impediment to normal mm operations.
> 
> I think vhost can use hmm_range_fault
> 
> hfi1 does actually need to have the page pin, it doesn't fence DMA
> during invalidate.
> 
> i915_gem feels alot like amdgpu, so probably it would benefit
> 
> No idea about scif_dma
> 
> > I will test on nouveau.
> 
> Thanks, hopefully it still works, I think Ralph was able to do some
> basic checks. But it is a pretty complicated series, I probably made
> some mistakes.

So it seems to work ok with nouveau, will let tests run in loop thought
there are not very advance test.

> 
> FWIW, I know that nouveau gets a lockdep splat now from Daniel
> Vetter's recent changes, it tries to do GFP_KERENEL allocations under
> a lock also held by the invalidate_range_start path.

I have not seen any splat so far, is it throught some new kernel config ?

Cheers,
Jérôme

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

Re: [PATCH hmm 00/15] Consolidate the mmu notifier interval_tree and locking

2019-10-23 Thread Jerome Glisse

On Wed, Oct 23, 2019 at 11:32:16AM +0200, Christian König wrote:
> Am 23.10.19 um 11:08 schrieb Daniel Vetter:
> > On Tue, Oct 22, 2019 at 03:01:13PM +, Jason Gunthorpe wrote:
> > > On Tue, Oct 22, 2019 at 09:57:35AM +0200, Daniel Vetter wrote:
> > > 
> > > > > The unusual bit in all of this is using a lock's critical region to
> > > > > 'protect' data for read, but updating that same data before the lock's
> > > > > critical secion. ie relying on the unlock barrier to 'release' program
> > > > > ordered stores done before the lock's own critical region, and the
> > > > > lock side barrier to 'acquire' those stores.
> > > > I think this unusual use of locks as barriers for other unlocked 
> > > > accesses
> > > > deserves comments even more than just normal barriers. Can you pls add
> > > > them? I think the design seeems sound ...
> > > > 
> > > > Also the comment on the driver's lock hopefully prevents driver
> > > > maintainers from moving the driver_lock around in a way that would very
> > > > subtle break the scheme, so I think having the acquire barrier commented
> > > > in each place would be really good.
> > > There is already a lot of documentation, I think it would be helpful
> > > if you could suggest some specific places where you think an addition
> > > would help? I think the perspective of someone less familiar with this
> > > design would really improve the documentation
> > Hm I just meant the usual recommendation that "barriers must have comments
> > explaining what they order, and where the other side of the barrier is".
> > Using unlock/lock as a barrier imo just makes that an even better idea.
> > Usually what I do is something like "we need to order $this against $that
> > below, and the other side of this barrier is in function()." With maybe a
> > bit more if it's not obvious how things go wrong if the orderin is broken.
> > 
> > Ofc seqlock.h itself skimps on that rule and doesn't bother explaining its
> > barriers :-/
> > 
> > > I've been tempted to force the driver to store the seq number directly
> > > under the driver lock - this makes the scheme much clearer, ie
> > > something like this:
> > > 
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c 
> > > b/drivers/gpu/drm/nouveau/nouveau_svm.c
> > > index 712c99918551bc..738fa670dcfb19 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> > > @@ -488,7 +488,8 @@ struct svm_notifier {
> > >   };
> > >   static bool nouveau_svm_range_invalidate(struct mmu_range_notifier *mrn,
> > > -const struct mmu_notifier_range 
> > > *range)
> > > +const struct mmu_notifier_range 
> > > *range,
> > > +unsigned long seq)
> > >   {
> > >  struct svm_notifier *sn =
> > >  container_of(mrn, struct svm_notifier, notifier);
> > > @@ -504,6 +505,7 @@ static bool nouveau_svm_range_invalidate(struct 
> > > mmu_range_notifier *mrn,
> > >  mutex_lock(>svmm->mutex);
> > >  else if (!mutex_trylock(>svmm->mutex))
> > >  return false;
> > > +   mmu_range_notifier_update_seq(mrn, seq);
> > >  mutex_unlock(>svmm->mutex);
> > >  return true;
> > >   }
> > > 
> > > 
> > > At the cost of making the driver a bit more complex, what do you
> > > think?
> > Hm, spinning this further ... could we initialize the mmu range notifier
> > with a pointer to the driver lock, so that we could put a
> > lockdep_assert_held into mmu_range_notifier_update_seq? I think that would
> > make this scheme substantially more driver-hacker proof :-)
> 
> Going another step further what hinders us to put the lock into the mmu
> range notifier itself and have _lock()/_unlock() helpers?
> 
> I mean having the lock in the driver only makes sense when the driver would
> be using the same lock for multiple things, e.g. multiple MMU range
> notifiers under the same lock. But I really don't see that use case here.

I actualy do, nouveau use one lock to protect the page table and that's the
lock that matter. You can have multiple range for a single page table, idea
being only a sub-set of the process address space is ever accessed by the
GPU and those it is better to focus on this sub-set and track invalidation in
a finer grain.

Cheers,
Jérôme

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

Re: [PATCH 18/37] drm/amd/display: Allow inverted gamma

2019-10-23 Thread Kazlauskas, Nicholas

On 2019-10-23 11:44 a.m., Li, Sun peng (Leo) wrote:
> 
> 
> On 2019-10-23 11:23 a.m., Michel Dänzer wrote:
>> On 2019-10-17 9:13 p.m., sunpeng...@amd.com wrote:
>>> From: Aidan Yang 
>>>
>>> [why]
>>> There's a use case for inverted gamma
>>> and it's been confirmed that negative slopes are ok.
>>>
>>> [how]
>>> Remove code for blocking non-monotonically increasing gamma
>>>
>>> Signed-off-by: Aidan Yang 
>>> Reviewed-by: Krunoslav Kovac 
>>> Acked-by: Leo Li 
>>> Acked-by: Reza Amini 
>>
>> Does this fix https://bugs.freedesktop.org/110677 ? If so, it should be
>> referenced in the commit log, and the report resolved once this lands in
>> drm-next or Linus' tree.
> 
> I don't think it would, not on reported platform at least. This change
> only modifies for DCN families, so RX580 wouldn't be affected.
> 
> Leo

Looks like the same check exists on DCE though, it could probably be 
dropped as well to fix this bug.

See:
dce110_translate_regamma_to_hw_format()

while (i != hw_points + 1) {
if (dc_fixpt_lt(rgb_plus_1->red, rgb->red))
rgb_plus_1->red = rgb->red;
if (dc_fixpt_lt(rgb_plus_1->green, rgb->green))
rgb_plus_1->green = rgb->green;
if (dc_fixpt_lt(rgb_plus_1->blue, rgb->blue))
rgb_plus_1->blue = rgb->blue;

rgb->delta_red = dc_fixpt_sub(rgb_plus_1->red, rgb->red);
rgb->delta_green = dc_fixpt_sub(rgb_plus_1->green, rgb->green);
rgb->delta_blue = dc_fixpt_sub(rgb_plus_1->blue, rgb->blue);

++rgb_plus_1;
++rgb;
++i;
}


Nicholas Kazlauskas

> 
>>
>>
> ___
> 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 v2] drm/amdgpu: Add DC feature mask to disable fractional pwm

2019-10-23 Thread Li, Sun peng (Leo)

On 2019-10-23 10:19 a.m., Lukáš Krejčí wrote:
> On Mon, 2019-10-21 at 15:44 -0400, sunpeng...@amd.com wrote:
>> From: Leo Li 
>>
>> [Why]
>>
>> Some LED panel drivers might not like fractional PWM. In such cases,
>> backlight flickering may be observed.
>>
>> [How]
>>
>> Add a DC feature mask to disable fractional PWM, and associate it with
>> the preexisting dc_config flag.
>>
>> The flag is only plumbed through the dmcu firmware, so plumb it through
>> the driver path as well.
>>
>> To disable, add the following to the linux cmdline:
>> amdgpu.dcfeaturemask=0x4
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204957
>> Signed-off-by: Leo Li 
>> ---
>>
>> v2: Add bugzilla link
>>
>>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
>>  drivers/gpu/drm/amd/display/dc/dce/dce_abm.c  | 4 
>>  drivers/gpu/drm/amd/include/amd_shared.h  | 1 +
>>  3 files changed, 8 insertions(+)
> 
> Tested on Ubuntu 19.04, vanilla v5.3.7 kernel with the patch applied
> and amdgpu.dcfeaturemask=0x4 added to the kernel command line, no
> issues after 8 reboots. (The issue could be reproduced without
> amdgpu.dcfeaturemask=0x4 on first boot.)
> 
> Tested-by: Lukáš Krejčí 

Applied, thanks!
Leo
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 18/37] drm/amd/display: Allow inverted gamma

2019-10-23 Thread Li, Sun peng (Leo)



On 2019-10-23 11:23 a.m., Michel Dänzer wrote:
> On 2019-10-17 9:13 p.m., sunpeng...@amd.com wrote:
>> From: Aidan Yang 
>>
>> [why]
>> There's a use case for inverted gamma
>> and it's been confirmed that negative slopes are ok.
>>
>> [how]
>> Remove code for blocking non-monotonically increasing gamma
>>
>> Signed-off-by: Aidan Yang 
>> Reviewed-by: Krunoslav Kovac 
>> Acked-by: Leo Li 
>> Acked-by: Reza Amini 
> 
> Does this fix https://bugs.freedesktop.org/110677 ? If so, it should be
> referenced in the commit log, and the report resolved once this lands in
> drm-next or Linus' tree.

I don't think it would, not on reported platform at least. This change
only modifies for DCN families, so RX580 wouldn't be affected.

Leo

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

Re: [PATCH] Cleanup: replace prefered with preferred

2019-10-23 Thread Mark Salyzyn


On 10/23/19 4:56 AM, Jarkko Sakkinen wrote:

On Tue, Oct 22, 2019 at 02:41:45PM -0700, Mark Salyzyn wrote:

Replace all occurrences of prefered with preferred to make future
checkpatch.pl's happy.  A few places the incorrect spelling is
matched with the correct spelling to preserve existing user space API.

Signed-off-by: Mark Salyzyn 

I'd fix such things when the code is otherwise change and scope this
patch only to Documentation/. There is no pragmatic benefit of doing
this for the code.

/Jarkko


The pragmatic benefit comes with the use of an ABI/API checker (which is 
a 'distro' thing, not a top of tree kernel thing) produces its map which 
is typically required to be co-located in the same tree as the kernel 
repository. Quite a few ABI/API update checkins result in a 
checkpatch.pl complaint about the misspelled elements being 
(re-)recorded due to proximity. We have a separate task to improve how 
it is tracked in Android to reduce milepost marker changes that result 
in sweeping changes to the database which would reduce the occurrences.


I will split this between pure and inert documentation/comments for now, 
with a followup later for the code portion which understandably is more 
controversial.


Cleanup is the least appreciated part of kernel maintenance ;-}.

Sincerely -- Mark Salyzyn

Re: [PATCH] drm/amdgpu: remove unused variable in amdgpu_gfx_kiq_free_ring

2019-10-23 Thread Nirmoy

Thanks Updated and pushed to  brahma HEAD:refs/for/amd-staging-drm-next

On 10/23/19 5:21 PM, Koenig, Christian wrote:
> Maybe say parameter instead of variable in the subject.
>
> Am 23.10.19 um 16:35 schrieb Nirmoy Das:
>> Signed-off-by: Nirmoy Das 
> Apart from that Acked-by: Christian König 
>
>> ---
>>drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 3 +--
>>drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 3 +--
>>drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c  | 2 +-
>>drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 2 +-
>>drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 2 +-
>>5 files changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> index 069515f57c2a..c9d1fada6188 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> @@ -319,8 +319,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
>>  return r;
>>}
>>
>> -void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring,
>> -  struct amdgpu_irq_src *irq)
>> +void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring)
>>{
>>  amdgpu_device_wb_free(ring->adev, ring->adev->virt.reg_val_offs);
>>  amdgpu_ring_fini(ring);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> index 35eff9e6ce16..459aa9059542 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> @@ -330,8 +330,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
>>   struct amdgpu_ring *ring,
>>   struct amdgpu_irq_src *irq);
>>
>> -void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring,
>> -  struct amdgpu_irq_src *irq);
>> +void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring);
>>
>>void amdgpu_gfx_kiq_fini(struct amdgpu_device *adev);
>>int amdgpu_gfx_kiq_init(struct amdgpu_device *adev,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> index 8fca6ab5fa8f..ac43b1af69e3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> @@ -1443,7 +1443,7 @@ static int gfx_v10_0_sw_fini(void *handle)
>>  amdgpu_ring_fini(>gfx.compute_ring[i]);
>>
>>  amdgpu_gfx_mqd_sw_fini(adev);
>> -amdgpu_gfx_kiq_free_ring(>gfx.kiq.ring, >gfx.kiq.irq);
>> +amdgpu_gfx_kiq_free_ring(>gfx.kiq.ring);
>>  amdgpu_gfx_kiq_fini(adev);
>>
>>  gfx_v10_0_pfp_fini(adev);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> index a7fe0ea24d1f..e4c645da4e28 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> @@ -2103,7 +2103,7 @@ static int gfx_v8_0_sw_fini(void *handle)
>>  amdgpu_ring_fini(>gfx.compute_ring[i]);
>>
>>  amdgpu_gfx_mqd_sw_fini(adev);
>> -amdgpu_gfx_kiq_free_ring(>gfx.kiq.ring, >gfx.kiq.irq);
>> +amdgpu_gfx_kiq_free_ring(>gfx.kiq.ring);
>>  amdgpu_gfx_kiq_fini(adev);
>>
>>  gfx_v8_0_mec_fini(adev);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> index dd345fcedb97..9fe95e7693d5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> @@ -2153,7 +2153,7 @@ static int gfx_v9_0_sw_fini(void *handle)
>>  amdgpu_ring_fini(>gfx.compute_ring[i]);
>>
>>  amdgpu_gfx_mqd_sw_fini(adev);
>> -amdgpu_gfx_kiq_free_ring(>gfx.kiq.ring, >gfx.kiq.irq);
>> +amdgpu_gfx_kiq_free_ring(>gfx.kiq.ring);
>>  amdgpu_gfx_kiq_fini(adev);
>>
>>  gfx_v9_0_mec_fini(adev);
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 18/37] drm/amd/display: Allow inverted gamma

2019-10-23 Thread Michel Dänzer

On 2019-10-17 9:13 p.m., sunpeng...@amd.com wrote:
> From: Aidan Yang 
> 
> [why]
> There's a use case for inverted gamma
> and it's been confirmed that negative slopes are ok.
> 
> [how]
> Remove code for blocking non-monotonically increasing gamma
> 
> Signed-off-by: Aidan Yang 
> Reviewed-by: Krunoslav Kovac 
> Acked-by: Leo Li 
> Acked-by: Reza Amini 

Does this fix https://bugs.freedesktop.org/110677 ? If so, it should be
referenced in the commit log, and the report resolved once this lands in
drm-next or Linus' tree.


-- 
Earthling Michel Dänzer   |   https://redhat.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: [PATCH] drm/amdgpu: remove unused variable in amdgpu_gfx_kiq_free_ring

2019-10-23 Thread Koenig, Christian

Maybe say parameter instead of variable in the subject.

Am 23.10.19 um 16:35 schrieb Nirmoy Das:
> Signed-off-by: Nirmoy Das 

Apart from that Acked-by: Christian König 

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 3 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 3 +--
>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c  | 2 +-
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 2 +-
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 2 +-
>   5 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 069515f57c2a..c9d1fada6188 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -319,8 +319,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
>   return r;
>   }
>   
> -void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring,
> -   struct amdgpu_irq_src *irq)
> +void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring)
>   {
>   amdgpu_device_wb_free(ring->adev, ring->adev->virt.reg_val_offs);
>   amdgpu_ring_fini(ring);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 35eff9e6ce16..459aa9059542 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -330,8 +330,7 @@ int amdgpu_gfx_kiq_init_ring(struct amdgpu_device *adev,
>struct amdgpu_ring *ring,
>struct amdgpu_irq_src *irq);
>   
> -void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring,
> -   struct amdgpu_irq_src *irq);
> +void amdgpu_gfx_kiq_free_ring(struct amdgpu_ring *ring);
>   
>   void amdgpu_gfx_kiq_fini(struct amdgpu_device *adev);
>   int amdgpu_gfx_kiq_init(struct amdgpu_device *adev,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 8fca6ab5fa8f..ac43b1af69e3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -1443,7 +1443,7 @@ static int gfx_v10_0_sw_fini(void *handle)
>   amdgpu_ring_fini(>gfx.compute_ring[i]);
>   
>   amdgpu_gfx_mqd_sw_fini(adev);
> - amdgpu_gfx_kiq_free_ring(>gfx.kiq.ring, >gfx.kiq.irq);
> + amdgpu_gfx_kiq_free_ring(>gfx.kiq.ring);
>   amdgpu_gfx_kiq_fini(adev);
>   
>   gfx_v10_0_pfp_fini(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index a7fe0ea24d1f..e4c645da4e28 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -2103,7 +2103,7 @@ static int gfx_v8_0_sw_fini(void *handle)
>   amdgpu_ring_fini(>gfx.compute_ring[i]);
>   
>   amdgpu_gfx_mqd_sw_fini(adev);
> - amdgpu_gfx_kiq_free_ring(>gfx.kiq.ring, >gfx.kiq.irq);
> + amdgpu_gfx_kiq_free_ring(>gfx.kiq.ring);
>   amdgpu_gfx_kiq_fini(adev);
>   
>   gfx_v8_0_mec_fini(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index dd345fcedb97..9fe95e7693d5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -2153,7 +2153,7 @@ static int gfx_v9_0_sw_fini(void *handle)
>   amdgpu_ring_fini(>gfx.compute_ring[i]);
>   
>   amdgpu_gfx_mqd_sw_fini(adev);
> - amdgpu_gfx_kiq_free_ring(>gfx.kiq.ring, >gfx.kiq.irq);
> + amdgpu_gfx_kiq_free_ring(>gfx.kiq.ring);
>   amdgpu_gfx_kiq_fini(adev);
>   
>   gfx_v9_0_mec_fini(adev);

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

Re: [PATCH] drm/radeon: remove assignment for return value

2019-10-23 Thread Harry Wentland

On 2019-10-19 3:32 a.m., Wambui Karuga wrote:
> Remove unnecessary assignment for return value and have the
> function return the required value directly.
> Issue found by coccinelle:
> @@
> local idexpression ret;
> expression e;
> @@
> 
> -ret =
> +return
>  e;
> -return ret;
> 
> Signed-off-by: Wambui Karuga 

Thanks for your patch.

Reviewed-by: Harry Wentland 

Harry


> ---
>  drivers/gpu/drm/radeon/cik.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
> index 62eab82a64f9..daff9a2af3be 100644
> --- a/drivers/gpu/drm/radeon/cik.c
> +++ b/drivers/gpu/drm/radeon/cik.c
> @@ -221,9 +221,7 @@ int ci_get_temp(struct radeon_device *rdev)
>   else
>   actual_temp = temp & 0x1ff;
>  
> - actual_temp = actual_temp * 1000;
> -
> - return actual_temp;
> + return actual_temp * 1000;
>  }
>  
>  /* get temperature in millidegrees */
> @@ -239,9 +237,7 @@ int kv_get_temp(struct radeon_device *rdev)
>   else
>   actual_temp = 0;
>  
> - actual_temp = actual_temp * 1000;
> -
> - return actual_temp;
> + return actual_temp * 1000;
>  }
>  
>  /*
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/amdgpu: correct length misspelling

2019-10-23 Thread Harry Wentland

On 2019-10-19 3:34 a.m., Wambui Karuga wrote:
> Correct the "_LENTH" mispelling in the AMDGPU_MAX_TIMEOUT_PARAM_LENGTH
> constant.
> 
> Signed-off-by: Wambui Karuga 

This patch would be better sent in a patch set with the "make undeclared
variables static" patch. You can do that by providing a range to "git
format-patch". I usually call git format-patch with the -o parameter to
put all my patches in a directory. Then I can send it with "git
send-email *" in that directory.

Reviewed-by: Harry Wentland 

This won't apply cleanly without "make undeclared variables static".
Please see my comments on that patch and send a v2 for this one.

Harry

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index c5b3c0c9193b..aaab37833659 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -86,7 +86,7 @@
>  #define KMS_DRIVER_MINOR 34
>  #define KMS_DRIVER_PATCHLEVEL0
>  
> -#define AMDGPU_MAX_TIMEOUT_PARAM_LENTH   256
> +#define AMDGPU_MAX_TIMEOUT_PARAM_LENGTH  256
>  
>  int amdgpu_vram_limit = 0;
>  int amdgpu_vis_vram_limit = 0;
> @@ -100,7 +100,7 @@ int amdgpu_disp_priority = 0;
>  int amdgpu_hw_i2c = 0;
>  int amdgpu_pcie_gen2 = -1;
>  int amdgpu_msi = -1;
> -static char amdgpu_lockup_timeout[AMDGPU_MAX_TIMEOUT_PARAM_LENTH];
> +static char amdgpu_lockup_timeout[AMDGPU_MAX_TIMEOUT_PARAM_LENGTH];
>  int amdgpu_dpm = -1;
>  int amdgpu_fw_load_type = -1;
>  int amdgpu_aspm = -1;
> @@ -1327,9 +1327,9 @@ int amdgpu_device_get_job_timeout_settings(struct 
> amdgpu_device *adev)
>   adev->sdma_timeout = adev->video_timeout = adev->gfx_timeout;
>   adev->compute_timeout = MAX_SCHEDULE_TIMEOUT;
>  
> - if (strnlen(input, AMDGPU_MAX_TIMEOUT_PARAM_LENTH)) {
> + if (strnlen(input, AMDGPU_MAX_TIMEOUT_PARAM_LENGTH)) {
>   while ((timeout_setting = strsep(, ",")) &&
> - strnlen(timeout_setting, 
> AMDGPU_MAX_TIMEOUT_PARAM_LENTH)) {
> + strnlen(timeout_setting, 
> AMDGPU_MAX_TIMEOUT_PARAM_LENGTH)) {
>   ret = kstrtol(timeout_setting, 0, );
>   if (ret)
>   return ret;
>

Re: [PATCH] drm/amd/amdgpu: make undeclared variables static

2019-10-23 Thread Harry Wentland

On 2019-10-19 3:24 a.m., Wambui Karuga wrote:
> Make the `amdgpu_lockup_timeout` and `amdgpu_exp_hw_support` variables
> static to remove the following sparse warnings:
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:103:19: warning: symbol 
> 'amdgpu_lockup_timeout' was not declared. Should it be static?

This should be declared in amdgpu.h. amdgpu is maintained on the
amd-staging-drm-next branch from
https://cgit.freedesktop.org/~agd5f/linux/?h=amd-staging-drm-next. Can
you check there?

> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:117:18: warning: symbol 
> 'amdgpu_exp_hw_support' was not declared. Should it be static?
> 
> Signed-off-by: Wambui Karuga 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 3fae1007143e..c5b3c0c9193b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -100,7 +100,7 @@ int amdgpu_disp_priority = 0;
>  int amdgpu_hw_i2c = 0;
>  int amdgpu_pcie_gen2 = -1;
>  int amdgpu_msi = -1;
> -char amdgpu_lockup_timeout[AMDGPU_MAX_TIMEOUT_PARAM_LENTH];
> +static char amdgpu_lockup_timeout[AMDGPU_MAX_TIMEOUT_PARAM_LENTH];
>  int amdgpu_dpm = -1;
>  int amdgpu_fw_load_type = -1;
>  int amdgpu_aspm = -1;
> @@ -114,7 +114,7 @@ int amdgpu_vm_block_size = -1;
>  int amdgpu_vm_fault_stop = 0;
>  int amdgpu_vm_debug = 0;
>  int amdgpu_vm_update_mode = -1;
> -int amdgpu_exp_hw_support = 0;
> +static int amdgpu_exp_hw_support;

This is indeed only used in this file but for consistency's sake it's
probably better to also declare it in amdgpu.h rather than make it
static here.

Harry

>  int amdgpu_dc = -1;
>  int amdgpu_sched_jobs = 32;
>  int amdgpu_sched_hw_submission = 2;
>

Re: [PATCH v2] drm/amdgpu: Add DC feature mask to disable fractional pwm

2019-10-23 Thread Lukáš Krejčí

On Mon, 2019-10-21 at 15:44 -0400, sunpeng...@amd.com wrote:
> From: Leo Li 
> 
> [Why]
> 
> Some LED panel drivers might not like fractional PWM. In such cases,
> backlight flickering may be observed.
> 
> [How]
> 
> Add a DC feature mask to disable fractional PWM, and associate it with
> the preexisting dc_config flag.
> 
> The flag is only plumbed through the dmcu firmware, so plumb it through
> the driver path as well.
> 
> To disable, add the following to the linux cmdline:
> amdgpu.dcfeaturemask=0x4
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204957
> Signed-off-by: Leo Li 
> ---
> 
> v2: Add bugzilla link
> 
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
>  drivers/gpu/drm/amd/display/dc/dce/dce_abm.c  | 4 
>  drivers/gpu/drm/amd/include/amd_shared.h  | 1 +
>  3 files changed, 8 insertions(+)

Tested on Ubuntu 19.04, vanilla v5.3.7 kernel with the patch applied
and amdgpu.dcfeaturemask=0x4 added to the kernel command line, no
issues after 8 reboots. (The issue could be reproduced without
amdgpu.dcfeaturemask=0x4 on first boot.)

Tested-by: Lukáš Krejčí 

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

Re: [PATCH] dc.c:use kzalloc without test

2019-10-23 Thread Harry Wentland

On 2019-10-23 4:32 a.m., zhongshiqi wrote:
> dc.c:583:null check is needed after using kzalloc function
> 
> Signed-off-by: zhongshiqi 

Reviewed-by: Harry Wentland 

Harry

> ---
>  drivers/gpu/drm/amd/display/dc/core/dc.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 5d1aded..4b8819c 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -580,6 +580,10 @@ static bool construct(struct dc *dc,
>  #ifdef CONFIG_DRM_AMD_DC_DCN2_0
>   // Allocate memory for the vm_helper
>   dc->vm_helper = kzalloc(sizeof(struct vm_helper), GFP_KERNEL);
> + if (!dc->vm_helper) {
> + dm_error("%s: failed to create dc->vm_helper\n", __func__);
> + goto fail;
> + }
>  
>  #endif
>   memcpy(>bb_overrides, _params->bb_overrides, 
> sizeof(dc->bb_overrides));
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu/sdma5: do not execute 0-sized IBs (v2)

2019-10-23 Thread Pelloux-prayer, Pierre-eric

Hi Alex,

On 23/10/2019 14:50, Deucher, Alexander wrote:
>> -Original Message-
>> From: amd-gfx  On Behalf Of
>> Christian König
>> Sent: Wednesday, October 23, 2019 3:33 AM
>> To: Pelloux-prayer, Pierre-eric ; amd-
>> g...@lists.freedesktop.org
>> Subject: Re: [PATCH] drm/amdgpu/sdma5: do not execute 0-sized IBs (v2)
>>
>> Am 22.10.19 um 19:22 schrieb Pelloux-prayer, Pierre-eric:
>>> This seems to help with
>> https://bugs.freedesktop.org/show_bug.cgi?id=111481.
>>>
>>> v2: insert a NOP instead of skipping all 0-sized IBs to avoid breaking older
>> hw
>>>
>>> Signed-off-by: Pierre-Eric Pelloux-Prayer > pra...@amd.com>
>>
>> Reviewed-by: Christian König 
> 
> Do nop packets have any alignment requirements on SDMA?  Some of the other 
> packets do.

There's no alignment requirements for nop packets.

Pierre-Eric

> 
> Alex
> 
>>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>>> index fb48622c2abd..6e1b25bd1fe7 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>>> @@ -309,6 +309,7 @@ static void gmc_v10_0_flush_gpu_tlb(struct
>> amdgpu_device *adev, uint32_t vmid,
>>>
>>> job->vm_pd_addr = amdgpu_gmc_pd_addr(adev->gart.bo);
>>> job->vm_needs_flush = true;
>>> +   job->ibs->ptr[job->ibs->length_dw++] = ring->funcs->nop;
>>> amdgpu_ring_pad_ib(ring, >ibs[0]);
>>> r = amdgpu_job_submit(job, >mman.entity,
>>>   AMDGPU_FENCE_OWNER_UNDEFINED, );
>>
>> ___
>> 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] drm/amdgpu/sdma5: do not execute 0-sized IBs (v2)

2019-10-23 Thread Deucher, Alexander

> -Original Message-
> From: amd-gfx  On Behalf Of
> Christian König
> Sent: Wednesday, October 23, 2019 3:33 AM
> To: Pelloux-prayer, Pierre-eric ; amd-
> g...@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu/sdma5: do not execute 0-sized IBs (v2)
> 
> Am 22.10.19 um 19:22 schrieb Pelloux-prayer, Pierre-eric:
> > This seems to help with
> https://bugs.freedesktop.org/show_bug.cgi?id=111481.
> >
> > v2: insert a NOP instead of skipping all 0-sized IBs to avoid breaking older
> hw
> >
> > Signed-off-by: Pierre-Eric Pelloux-Prayer  pra...@amd.com>
> 
> Reviewed-by: Christian König 

Do nop packets have any alignment requirements on SDMA?  Some of the other 
packets do.

Alex

> 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> > index fb48622c2abd..6e1b25bd1fe7 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> > @@ -309,6 +309,7 @@ static void gmc_v10_0_flush_gpu_tlb(struct
> amdgpu_device *adev, uint32_t vmid,
> >
> > job->vm_pd_addr = amdgpu_gmc_pd_addr(adev->gart.bo);
> > job->vm_needs_flush = true;
> > +   job->ibs->ptr[job->ibs->length_dw++] = ring->funcs->nop;
> > amdgpu_ring_pad_ib(ring, >ibs[0]);
> > r = amdgpu_job_submit(job, >mman.entity,
> >   AMDGPU_FENCE_OWNER_UNDEFINED, );
> 
> ___
> 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] drm/amdgpu: Fix SDMA hang when performing VKexample test

2019-10-23 Thread Deucher, Alexander

> -Original Message-
> From: amd-gfx  On Behalf Of
> chen gong
> Sent: Wednesday, October 23, 2019 2:51 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Gong, Curry 
> Subject: [PATCH] drm/amdgpu: Fix SDMA hang when performing VKexample
> test
> 
> VKexample test hang during Occlusion/SDMA/Varia runs.
> Clear XNACK_WATERMK in reg SDMA0_UTCL1_WATERMK to fix this issue.
> 
> Signed-off-by: chen gong 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 63a3792..45bd538 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -254,6 +254,7 @@ static const struct soc15_reg_golden
> golden_settings_sdma_4_3[] = {
>   SOC15_REG_GOLDEN_VALUE(SDMA0, 0,
> mmSDMA0_RLC0_RB_WPTR_POLL_CNTL, 0xfff7, 0x00403000),
>   SOC15_REG_GOLDEN_VALUE(SDMA0, 0,
> mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 0xfff7, 0x00403000),
>   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE,
> 0x03ff, 0x03c0),
> + SOC15_REG_GOLDEN_VALUE(SDMA0, 0,
> mmSDMA0_UTCL1_WATERMK, 0xfc00, 0x)
>  };
> 
>  static u32 sdma_v4_0_get_reg_offset(struct amdgpu_device *adev,
> --
> 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] drm/amdgpu: call amdgpu_vm_prt_fini before deleting the root PD

2019-10-23 Thread Christian König


Am 23.10.19 um 14:02 schrieb Pelloux-prayer, Pierre-eric:

amdgpu_vm_prt_fini uses "vm->root.base.bo" so it must still be valid when
we call it.

Fixes: b65709a92156 ("drm/amdgpu: reserve the root PD while freeing PASIDs")
Signed-off-by: Pierre-Eric Pelloux-Prayer 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 19 ++-
  1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d9bece987e60..c8ce42200059 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2975,6 +2975,16 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
vm->pasid = 0;
}
  
+	list_for_each_entry_safe(mapping, tmp, >freed, list) {

+   if (mapping->flags & AMDGPU_PTE_PRT && prt_fini_needed) {
+   amdgpu_vm_prt_fini(adev, vm);
+   prt_fini_needed = false;
+   }
+
+   list_del(>list);
+   amdgpu_vm_free_mapping(adev, vm, mapping, NULL);
+   }
+
amdgpu_vm_free_pts(adev, vm, NULL);
amdgpu_bo_unreserve(root);
amdgpu_bo_unref();
@@ -2994,15 +3004,6 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
list_del(>list);
kfree(mapping);
}
-   list_for_each_entry_safe(mapping, tmp, >freed, list) {
-   if (mapping->flags & AMDGPU_PTE_PRT && prt_fini_needed) {
-   amdgpu_vm_prt_fini(adev, vm);
-   prt_fini_needed = false;
-   }
-
-   list_del(>list);
-   amdgpu_vm_free_mapping(adev, vm, mapping, NULL);
-   }
  
  	dma_fence_put(vm->last_update);

for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)


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

[PATCH] drm/amdgpu: call amdgpu_vm_prt_fini before deleting the root PD

2019-10-23 Thread Pelloux-prayer, Pierre-eric

amdgpu_vm_prt_fini uses "vm->root.base.bo" so it must still be valid when
we call it.

Fixes: b65709a92156 ("drm/amdgpu: reserve the root PD while freeing PASIDs")
Signed-off-by: Pierre-Eric Pelloux-Prayer 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d9bece987e60..c8ce42200059 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2975,6 +2975,16 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
vm->pasid = 0;
}
 
+   list_for_each_entry_safe(mapping, tmp, >freed, list) {
+   if (mapping->flags & AMDGPU_PTE_PRT && prt_fini_needed) {
+   amdgpu_vm_prt_fini(adev, vm);
+   prt_fini_needed = false;
+   }
+
+   list_del(>list);
+   amdgpu_vm_free_mapping(adev, vm, mapping, NULL);
+   }
+
amdgpu_vm_free_pts(adev, vm, NULL);
amdgpu_bo_unreserve(root);
amdgpu_bo_unref();
@@ -2994,15 +3004,6 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
list_del(>list);
kfree(mapping);
}
-   list_for_each_entry_safe(mapping, tmp, >freed, list) {
-   if (mapping->flags & AMDGPU_PTE_PRT && prt_fini_needed) {
-   amdgpu_vm_prt_fini(adev, vm);
-   prt_fini_needed = false;
-   }
-
-   list_del(>list);
-   amdgpu_vm_free_mapping(adev, vm, mapping, NULL);
-   }
 
dma_fence_put(vm->last_update);
for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
-- 
2.23.0

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

Re: [PATCH] Cleanup: replace prefered with preferred

2019-10-23 Thread Jarkko Sakkinen

On Tue, Oct 22, 2019 at 02:41:45PM -0700, Mark Salyzyn wrote:
> Replace all occurrences of prefered with preferred to make future
> checkpatch.pl's happy.  A few places the incorrect spelling is
> matched with the correct spelling to preserve existing user space API.
> 
> Signed-off-by: Mark Salyzyn 

I'd fix such things when the code is otherwise change and scope this
patch only to Documentation/. There is no pragmatic benefit of doing
this for the code.

/Jarkko

Re: [PATCH] Cleanup: replace prefered with preferred

2019-10-23 Thread Laurent Pinchart

Hi Mark,

Thank you for the patch.

On Tue, Oct 22, 2019 at 02:41:45PM -0700, Mark Salyzyn wrote:
> Replace all occurrences of prefered with preferred to make future
> checkpatch.pl's happy.  A few places the incorrect spelling is
> matched with the correct spelling to preserve existing user space API.
> 
> Signed-off-by: Mark Salyzyn 
> ---
>  Documentation/networking/ip-sysctl.txt|   2 +-
>  .../firmware/efi/libstub/efi-stub-helper.c|   2 +-
>  .../gpu/drm/amd/display/dc/inc/compressor.h   |   4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   |   2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h   |   2 +-
>  drivers/media/usb/uvc/uvc_video.c |   6 +-
>  fs/nfs/nfs4xdr.c  |   2 +-
>  include/linux/ipv6.h  |   2 +-
>  include/net/addrconf.h|   4 +-
>  include/net/if_inet6.h|   2 +-
>  include/net/ndisc.h   |   8 +-
>  include/uapi/linux/if_addr.h  |   5 +-
>  include/uapi/linux/ipv6.h |   4 +-
>  include/uapi/linux/sysctl.h   |   4 +-
>  include/uapi/linux/usb/video.h|   5 +-
>  kernel/sysctl_binary.c|   3 +-
>  net/6lowpan/ndisc.c   |   4 +-
>  net/ipv4/devinet.c|  20 ++--
>  net/ipv6/addrconf.c   | 113 ++
>  19 files changed, 112 insertions(+), 82 deletions(-)

[snip]

> diff --git a/drivers/media/usb/uvc/uvc_video.c 
> b/drivers/media/usb/uvc/uvc_video.c
> index 8fa77a81dd7f..0096e6aacdb4 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -276,13 +276,13 @@ static int uvc_get_video_ctrl(struct uvc_streaming 
> *stream,
>   if (size >= 34) {
>   ctrl->dwClockFrequency = get_unaligned_le32([26]);
>   ctrl->bmFramingInfo = data[30];
> - ctrl->bPreferedVersion = data[31];
> + ctrl->bPreferredVersion = data[31];
>   ctrl->bMinVersion = data[32];
>   ctrl->bMaxVersion = data[33];
>   } else {
>   ctrl->dwClockFrequency = stream->dev->clock_frequency;
>   ctrl->bmFramingInfo = 0;
> - ctrl->bPreferedVersion = 0;
> + ctrl->bPreferredVersion = 0;
>   ctrl->bMinVersion = 0;
>   ctrl->bMaxVersion = 0;
>   }
> @@ -325,7 +325,7 @@ static int uvc_set_video_ctrl(struct uvc_streaming 
> *stream,
>   if (size >= 34) {
>   put_unaligned_le32(ctrl->dwClockFrequency, [26]);
>   data[30] = ctrl->bmFramingInfo;
> - data[31] = ctrl->bPreferedVersion;
> + data[31] = ctrl->bPreferredVersion;
>   data[32] = ctrl->bMinVersion;
>   data[33] = ctrl->bMaxVersion;
>   }

[snip]

> diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
> index d854cb19c42c..59167f0ed5c1 100644
> --- a/include/uapi/linux/usb/video.h
> +++ b/include/uapi/linux/usb/video.h
> @@ -448,7 +448,10 @@ struct uvc_streaming_control {
>   __u32 dwMaxPayloadTransferSize;
>   __u32 dwClockFrequency;
>   __u8  bmFramingInfo;
> - __u8  bPreferedVersion;
> + union {
> + __u8 bPreferredVersion;
> + __u8 bPreferedVersion __attribute__((deprecated)); /* NOTYPO */
> + } __attribute__((__packed__));

Quite interestingly this typo is part of the USB device class definition
for video devices (UVC) specification. I thus think we should keep using
the field name bPreferedVersion through the code, otherwise it wouldn't
match the spec.

>   __u8  bMinVersion;
>   __u8  bMaxVersion;
>  } __attribute__((__packed__));

[snip]

-- 
Regards,

Laurent Pinchart

Re: [PATCH hmm 00/15] Consolidate the mmu notifier interval_tree and locking

2019-10-23 Thread Christian König


Am 23.10.19 um 11:08 schrieb Daniel Vetter:

On Tue, Oct 22, 2019 at 03:01:13PM +, Jason Gunthorpe wrote:

On Tue, Oct 22, 2019 at 09:57:35AM +0200, Daniel Vetter wrote:


The unusual bit in all of this is using a lock's critical region to
'protect' data for read, but updating that same data before the lock's
critical secion. ie relying on the unlock barrier to 'release' program
ordered stores done before the lock's own critical region, and the
lock side barrier to 'acquire' those stores.

I think this unusual use of locks as barriers for other unlocked accesses
deserves comments even more than just normal barriers. Can you pls add
them? I think the design seeems sound ...

Also the comment on the driver's lock hopefully prevents driver
maintainers from moving the driver_lock around in a way that would very
subtle break the scheme, so I think having the acquire barrier commented
in each place would be really good.

There is already a lot of documentation, I think it would be helpful
if you could suggest some specific places where you think an addition
would help? I think the perspective of someone less familiar with this
design would really improve the documentation

Hm I just meant the usual recommendation that "barriers must have comments
explaining what they order, and where the other side of the barrier is".
Using unlock/lock as a barrier imo just makes that an even better idea.
Usually what I do is something like "we need to order $this against $that
below, and the other side of this barrier is in function()." With maybe a
bit more if it's not obvious how things go wrong if the orderin is broken.

Ofc seqlock.h itself skimps on that rule and doesn't bother explaining its
barriers :-/


I've been tempted to force the driver to store the seq number directly
under the driver lock - this makes the scheme much clearer, ie
something like this:

diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c 
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 712c99918551bc..738fa670dcfb19 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -488,7 +488,8 @@ struct svm_notifier {
  };
  
  static bool nouveau_svm_range_invalidate(struct mmu_range_notifier *mrn,

-const struct mmu_notifier_range *range)
+const struct mmu_notifier_range *range,
+unsigned long seq)
  {
 struct svm_notifier *sn =
 container_of(mrn, struct svm_notifier, notifier);
@@ -504,6 +505,7 @@ static bool nouveau_svm_range_invalidate(struct 
mmu_range_notifier *mrn,
 mutex_lock(>svmm->mutex);
 else if (!mutex_trylock(>svmm->mutex))
 return false;
+   mmu_range_notifier_update_seq(mrn, seq);
 mutex_unlock(>svmm->mutex);
 return true;
  }


At the cost of making the driver a bit more complex, what do you
think?

Hm, spinning this further ... could we initialize the mmu range notifier
with a pointer to the driver lock, so that we could put a
lockdep_assert_held into mmu_range_notifier_update_seq? I think that would
make this scheme substantially more driver-hacker proof :-)


Going another step further what hinders us to put the lock into the 
mmu range notifier itself and have _lock()/_unlock() helpers?


I mean having the lock in the driver only makes sense when the driver 
would be using the same lock for multiple things, e.g. multiple MMU 
range notifiers under the same lock. But I really don't see that use 
case here.


Regards,
Christian.



Cheers, Daniel


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

Re: [PATCH hmm 00/15] Consolidate the mmu notifier interval_tree and locking

2019-10-23 Thread Daniel Vetter

On Tue, Oct 22, 2019 at 03:01:13PM +, Jason Gunthorpe wrote:
> On Tue, Oct 22, 2019 at 09:57:35AM +0200, Daniel Vetter wrote:
> 
> > > The unusual bit in all of this is using a lock's critical region to
> > > 'protect' data for read, but updating that same data before the lock's
> > > critical secion. ie relying on the unlock barrier to 'release' program
> > > ordered stores done before the lock's own critical region, and the
> > > lock side barrier to 'acquire' those stores.
> > 
> > I think this unusual use of locks as barriers for other unlocked accesses
> > deserves comments even more than just normal barriers. Can you pls add
> > them? I think the design seeems sound ...
> > 
> > Also the comment on the driver's lock hopefully prevents driver
> > maintainers from moving the driver_lock around in a way that would very
> > subtle break the scheme, so I think having the acquire barrier commented
> > in each place would be really good.
> 
> There is already a lot of documentation, I think it would be helpful
> if you could suggest some specific places where you think an addition
> would help? I think the perspective of someone less familiar with this
> design would really improve the documentation

Hm I just meant the usual recommendation that "barriers must have comments
explaining what they order, and where the other side of the barrier is".
Using unlock/lock as a barrier imo just makes that an even better idea.
Usually what I do is something like "we need to order $this against $that
below, and the other side of this barrier is in function()." With maybe a
bit more if it's not obvious how things go wrong if the orderin is broken.

Ofc seqlock.h itself skimps on that rule and doesn't bother explaining its
barriers :-/

> I've been tempted to force the driver to store the seq number directly
> under the driver lock - this makes the scheme much clearer, ie
> something like this:
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c 
> b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index 712c99918551bc..738fa670dcfb19 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -488,7 +488,8 @@ struct svm_notifier {
>  };
>  
>  static bool nouveau_svm_range_invalidate(struct mmu_range_notifier *mrn,
> -const struct mmu_notifier_range 
> *range)
> +const struct mmu_notifier_range 
> *range,
> +unsigned long seq)
>  {
> struct svm_notifier *sn =
> container_of(mrn, struct svm_notifier, notifier);
> @@ -504,6 +505,7 @@ static bool nouveau_svm_range_invalidate(struct 
> mmu_range_notifier *mrn,
> mutex_lock(>svmm->mutex);
> else if (!mutex_trylock(>svmm->mutex))
> return false;
> +   mmu_range_notifier_update_seq(mrn, seq);
> mutex_unlock(>svmm->mutex);
> return true;
>  }
> 
> 
> At the cost of making the driver a bit more complex, what do you
> think?

Hm, spinning this further ... could we initialize the mmu range notifier
with a pointer to the driver lock, so that we could put a
lockdep_assert_held into mmu_range_notifier_update_seq? I think that would
make this scheme substantially more driver-hacker proof :-)

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

[PATCH] drm/amd/powerplay: skip unsupported clock limit settings on Arcturus V2

2019-10-23 Thread Quan, Evan

For Arcturus, clock limit settings on uclk/socclk/fclk domains
are not supported.

V2: simplify the code to support both SGPU and MGPU cases

Change-Id: I1286289e3770f0421f0d22989437e26d3f7b2ec4
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c   |  13 ++
 drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 142 ---
 2 files changed, 39 insertions(+), 116 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 36f36b35000d..660efe009749 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -2881,6 +2881,19 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
DRM_ERROR("failed to create device file pp_dpm_sclk\n");
return ret;
}
+
+   /* Arcturus does not support standalone mclk/socclk/fclk level setting 
*/
+   if (adev->asic_type == CHIP_ARCTURUS) {
+   dev_attr_pp_dpm_mclk.attr.mode &= ~S_IWUGO;
+   dev_attr_pp_dpm_mclk.store = NULL;
+
+   dev_attr_pp_dpm_socclk.attr.mode &= ~S_IWUGO;
+   dev_attr_pp_dpm_socclk.store = NULL;
+
+   dev_attr_pp_dpm_fclk.attr.mode &= ~S_IWUGO;
+   dev_attr_pp_dpm_fclk.store = NULL;
+   }
+
ret = device_create_file(adev->dev, _attr_pp_dpm_mclk);
if (ret) {
DRM_ERROR("failed to create device file pp_dpm_mclk\n");
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c 
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index 9a5faac1ceea..1bcc5ab2873d 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -37,6 +37,7 @@
 #include "smu_v11_0_pptable.h"
 #include "arcturus_ppsmc.h"
 #include "nbio/nbio_7_4_sh_mask.h"
+#include "amdgpu_xgmi.h"
 
 #define CTF_OFFSET_EDGE5
 #define CTF_OFFSET_HOTSPOT 5
@@ -849,84 +850,13 @@ static int arcturus_force_clk_levels(struct smu_context 
*smu,
break;
 
case SMU_MCLK:
-   single_dpm_table = &(dpm_table->mem_table);
-
-   if (soft_max_level >= single_dpm_table->count) {
-   pr_err("Clock level specified %d is over max allowed 
%d\n",
-   soft_max_level, single_dpm_table->count 
- 1);
-   ret = -EINVAL;
-   break;
-   }
-
-   single_dpm_table->dpm_state.soft_min_level =
-   single_dpm_table->dpm_levels[soft_min_level].value;
-   single_dpm_table->dpm_state.soft_max_level =
-   single_dpm_table->dpm_levels[soft_max_level].value;
-
-   ret = arcturus_upload_dpm_level(smu, false, 
FEATURE_DPM_UCLK_MASK);
-   if (ret) {
-   pr_err("Failed to upload boot level to lowest!\n");
-   break;
-   }
-
-   ret = arcturus_upload_dpm_level(smu, true, 
FEATURE_DPM_UCLK_MASK);
-   if (ret)
-   pr_err("Failed to upload dpm max level to highest!\n");
-
-   break;
-
case SMU_SOCCLK:
-   single_dpm_table = &(dpm_table->soc_table);
-
-   if (soft_max_level >= single_dpm_table->count) {
-   pr_err("Clock level specified %d is over max allowed 
%d\n",
-   soft_max_level, single_dpm_table->count 
- 1);
-   ret = -EINVAL;
-   break;
-   }
-
-   single_dpm_table->dpm_state.soft_min_level =
-   single_dpm_table->dpm_levels[soft_min_level].value;
-   single_dpm_table->dpm_state.soft_max_level =
-   single_dpm_table->dpm_levels[soft_max_level].value;
-
-   ret = arcturus_upload_dpm_level(smu, false, 
FEATURE_DPM_SOCCLK_MASK);
-   if (ret) {
-   pr_err("Failed to upload boot level to lowest!\n");
-   break;
-   }
-
-   ret = arcturus_upload_dpm_level(smu, true, 
FEATURE_DPM_SOCCLK_MASK);
-   if (ret)
-   pr_err("Failed to upload dpm max level to highest!\n");
-
-   break;
-
case SMU_FCLK:
-   single_dpm_table = &(dpm_table->fclk_table);
-
-   if (soft_max_level >= single_dpm_table->count) {
-   pr_err("Clock level specified %d is over max allowed 
%d\n",
-   soft_max_level, single_dpm_table->count 
- 1);
-   ret = -EINVAL;
-   break;
-   }
-
-   single_dpm_table->dpm_state.soft_min_level =
-   single_dpm_table->dpm_levels[soft_min_level].value;
-   single_dpm_table->dpm_state.soft_max_level =
-

Re: [PATCH hmm 00/15] Consolidate the mmu notifier interval_tree and locking

2019-10-23 Thread Jason Gunthorpe

On Tue, Oct 22, 2019 at 09:57:35AM +0200, Daniel Vetter wrote:

> > The unusual bit in all of this is using a lock's critical region to
> > 'protect' data for read, but updating that same data before the lock's
> > critical secion. ie relying on the unlock barrier to 'release' program
> > ordered stores done before the lock's own critical region, and the
> > lock side barrier to 'acquire' those stores.
> 
> I think this unusual use of locks as barriers for other unlocked accesses
> deserves comments even more than just normal barriers. Can you pls add
> them? I think the design seeems sound ...
> 
> Also the comment on the driver's lock hopefully prevents driver
> maintainers from moving the driver_lock around in a way that would very
> subtle break the scheme, so I think having the acquire barrier commented
> in each place would be really good.

There is already a lot of documentation, I think it would be helpful
if you could suggest some specific places where you think an addition
would help? I think the perspective of someone less familiar with this
design would really improve the documentation

I've been tempted to force the driver to store the seq number directly
under the driver lock - this makes the scheme much clearer, ie
something like this:

diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c 
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 712c99918551bc..738fa670dcfb19 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -488,7 +488,8 @@ struct svm_notifier {
 };
 
 static bool nouveau_svm_range_invalidate(struct mmu_range_notifier *mrn,
-const struct mmu_notifier_range *range)
+const struct mmu_notifier_range *range,
+unsigned long seq)
 {
struct svm_notifier *sn =
container_of(mrn, struct svm_notifier, notifier);
@@ -504,6 +505,7 @@ static bool nouveau_svm_range_invalidate(struct 
mmu_range_notifier *mrn,
mutex_lock(>svmm->mutex);
else if (!mutex_trylock(>svmm->mutex))
return false;
+   mmu_range_notifier_update_seq(mrn, seq);
mutex_unlock(>svmm->mutex);
return true;
 }


At the cost of making the driver a bit more complex, what do you
think?

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

Re: [PATCH hmm 00/15] Consolidate the mmu notifier interval_tree and locking

2019-10-23 Thread Jason Gunthorpe

On Tue, Oct 22, 2019 at 07:56:12AM -0400, Dennis Dalessandro wrote:
> On 10/21/2019 12:58 PM, Jason Gunthorpe wrote:
> > On Mon, Oct 21, 2019 at 11:55:51AM -0400, Dennis Dalessandro wrote:
> > > On 10/15/2019 2:12 PM, Jason Gunthorpe wrote:
> > > > This is still being tested, but I figured to send it to start getting 
> > > > help
> > > > from the xen, amd and hfi drivers which I cannot test here.
> > > 
> > > Sorry for the delay, I never seen this. Was not on Cc list and didn't
> > > register to me it impacted hfi. I'll take a look and run it through some
> > > hfi1 tests.
> > 
> > Hm, you were cc'd on the hfi1 patch of the series:
> > 
> > https://patchwork.kernel.org/patch/11191395/
> > 
> > So you saw that, right?
> 
> I do now.
> 
> > But it seems that git send-email didn't pull all the cc's together?
> 
> I don't know. I thought it did, at one time I recall trying to get it *not*
> to do that, when preparing some internal reviews. Haven't used it for a long
> time though, I've been using stgit.
> 
> At any rate can you give me a SHA or branch this applies on top of? I have
> pulled rdma/hmm, rdma/wip/jgg, linus/master but seem to have conflicts.

Here is a full tree:

https://github.com/jgunthorpe/linux/commits/mmu_notifier

It needs the ODP series on the mailing list to apply

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

[PATCH] Cleanup: replace prefered with preferred

2019-10-23 Thread Mark Salyzyn

Replace all occurrences of prefered with preferred to make future
checkpatch.pl's happy.  A few places the incorrect spelling is
matched with the correct spelling to preserve existing user space API.

Signed-off-by: Mark Salyzyn 
---
 Documentation/networking/ip-sysctl.txt|   2 +-
 .../firmware/efi/libstub/efi-stub-helper.c|   2 +-
 .../gpu/drm/amd/display/dc/inc/compressor.h   |   4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c   |   2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h   |   2 +-
 drivers/media/usb/uvc/uvc_video.c |   6 +-
 fs/nfs/nfs4xdr.c  |   2 +-
 include/linux/ipv6.h  |   2 +-
 include/net/addrconf.h|   4 +-
 include/net/if_inet6.h|   2 +-
 include/net/ndisc.h   |   8 +-
 include/uapi/linux/if_addr.h  |   5 +-
 include/uapi/linux/ipv6.h |   4 +-
 include/uapi/linux/sysctl.h   |   4 +-
 include/uapi/linux/usb/video.h|   5 +-
 kernel/sysctl_binary.c|   3 +-
 net/6lowpan/ndisc.c   |   4 +-
 net/ipv4/devinet.c|  20 ++--
 net/ipv6/addrconf.c   | 113 ++
 19 files changed, 112 insertions(+), 82 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt 
b/Documentation/networking/ip-sysctl.txt
index 49e95f438ed7..eab3d20fc016 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1792,7 +1792,7 @@ temp_valid_lft - INTEGER
valid lifetime (in seconds) for temporary addresses.
Default: 604800 (7 days)
 
-temp_prefered_lft - INTEGER
+temp_preferred_lft - INTEGER
Preferred lifetime (in seconds) for temporary addresses.
Default: 86400 (1 day)
 
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c 
b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 3caae7f2cf56..0b0ae3844a8f 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -717,7 +717,7 @@ efi_status_t efi_relocate_kernel(efi_system_table_t 
*sys_table_arg,
 * The EFI firmware loader could have placed the kernel image
 * anywhere in memory, but the kernel has restrictions on the
 * max physical address it can run at.  Some architectures
-* also have a prefered address, so first try to relocate
+* also have a preferred address, so first try to relocate
 * to the preferred address.  If that fails, allocate as low
 * as possible while respecting the required alignment.
 */
diff --git a/drivers/gpu/drm/amd/display/dc/inc/compressor.h 
b/drivers/gpu/drm/amd/display/dc/inc/compressor.h
index 7a147a9762a0..a8448f8e85d6 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/compressor.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/compressor.h
@@ -126,9 +126,9 @@ struct fbc_requested_compressed_size {
unsigned int   min_size_alignment;
union {
struct {
-   /* Above preferedSize must be allocated in FB pool */
+   /* Above preferred_size must be allocated in FB pool */
unsigned int preferred_must_be_framebuffer_pool : 1;
-   /* Above minSize must be allocated in FB pool */
+   /* Above min_size must be allocated in FB pool */
unsigned int min_must_be_framebuffer_pool : 1;
} bits;
unsigned int flags;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index f47d5710cc95..c8ac6968221a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -2306,7 +2306,7 @@ int vmw_du_connector_fill_modes(struct drm_connector 
*connector,
}
 
drm_connector_list_update(connector);
-   /* Move the prefered mode first, help apps pick the right mode. */
+   /* Move the preferred mode first, help apps pick the right mode. */
drm_mode_sort(>modes);
 
return 1;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 3ee03227607c..335d9484e28f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -355,7 +355,7 @@ struct vmw_display_unit {
unsigned unit;
 
/*
-* Prefered mode tracking.
+* Preferred mode tracking.
 */
unsigned pref_width;
unsigned pref_height;
diff --git a/drivers/media/usb/uvc/uvc_video.c 
b/drivers/media/usb/uvc/uvc_video.c
index 8fa77a81dd7f..0096e6aacdb4 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -276,13 +276,13 @@ static int uvc_get_video_ctrl(struct uvc_streaming 
*stream,
if (size >= 34) {
ctrl->dwClockFrequency =

Re: [PATCH hmm 00/15] Consolidate the mmu notifier interval_tree and locking

2019-10-23 Thread Dennis Dalessandro


On 10/21/2019 12:58 PM, Jason Gunthorpe wrote:

On Mon, Oct 21, 2019 at 11:55:51AM -0400, Dennis Dalessandro wrote:

On 10/15/2019 2:12 PM, Jason Gunthorpe wrote:

This is still being tested, but I figured to send it to start getting help
from the xen, amd and hfi drivers which I cannot test here.


Sorry for the delay, I never seen this. Was not on Cc list and didn't
register to me it impacted hfi. I'll take a look and run it through some
hfi1 tests.


Hm, you were cc'd on the hfi1 patch of the series:

https://patchwork.kernel.org/patch/11191395/

So you saw that, right?


I do now.


But it seems that git send-email didn't pull all the cc's together?


I don't know. I thought it did, at one time I recall trying to get it 
*not* to do that, when preparing some internal reviews. Haven't used it 
for a long time though, I've been using stgit.


At any rate can you give me a SHA or branch this applies on top of? I 
have pulled rdma/hmm, rdma/wip/jgg, linus/master but seem to have conflicts.


-Denny



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

[PATCH -next] drm/amdgpu: remove set but not used variable 'adev'

2019-10-23 Thread YueHaibing

drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:1221:24: warning: variable adev set but 
not used [-Wunused-but-set-variable]
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:488:24: warning: variable adev set but 
not used [-Wunused-but-set-variable]
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:547:24: warning: variable adev set but 
not used [-Wunused-but-set-variable]

It is never used, so can removed it.

Signed-off-by: YueHaibing 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index a61b0d9..ba00262 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -485,15 +485,12 @@ static int amdgpu_move_vram_ram(struct ttm_buffer_object 
*bo, bool evict,
struct ttm_operation_ctx *ctx,
struct ttm_mem_reg *new_mem)
 {
-   struct amdgpu_device *adev;
struct ttm_mem_reg *old_mem = >mem;
struct ttm_mem_reg tmp_mem;
struct ttm_place placements;
struct ttm_placement placement;
int r;
 
-   adev = amdgpu_ttm_adev(bo->bdev);
-
/* create space/pages for new_mem in GTT space */
tmp_mem = *new_mem;
tmp_mem.mm_node = NULL;
@@ -544,15 +541,12 @@ static int amdgpu_move_ram_vram(struct ttm_buffer_object 
*bo, bool evict,
struct ttm_operation_ctx *ctx,
struct ttm_mem_reg *new_mem)
 {
-   struct amdgpu_device *adev;
struct ttm_mem_reg *old_mem = >mem;
struct ttm_mem_reg tmp_mem;
struct ttm_placement placement;
struct ttm_place placements;
int r;
 
-   adev = amdgpu_ttm_adev(bo->bdev);
-
/* make space in GTT for old_mem buffer */
tmp_mem = *new_mem;
tmp_mem.mm_node = NULL;
@@ -1218,11 +1212,8 @@ static struct ttm_backend_func amdgpu_backend_func = {
 static struct ttm_tt *amdgpu_ttm_tt_create(struct ttm_buffer_object *bo,
   uint32_t page_flags)
 {
-   struct amdgpu_device *adev;
struct amdgpu_ttm_tt *gtt;
 
-   adev = amdgpu_ttm_adev(bo->bdev);
-
gtt = kzalloc(sizeof(struct amdgpu_ttm_tt), GFP_KERNEL);
if (gtt == NULL) {
return NULL;
-- 
2.7.4


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

Displayport on thunderbolt dock is down although X thinks it is up

2019-10-23 Thread Klaus Kusche


Hello,

the thunderbolt maintainers told me that the following question 
should be directed to the gfx people:

I use a dell precision 7740 notebook with amd polaris 12 graphics.
The notebook is connected or hotplugged to a noname thunderbolt dock
with displayport or hdmi monitors connected to the dock.

With gentoo, amdgpu kernel driver and modesetting X driver,
I've the following problem:

In almost all cases when warm booting (without powerdown), 
and in at least half of the attempts when hotplugging the tb dock,
and whenever changing resolution or monitor configuration
or switching from text mode to Xwindows or back, displayport fails in X:
Xwindows thinks that a monitor is connected and dp is active and working,
but in most cases no signal at all arrives at the monitor 
(the monitor remains in sleep state because of no active input),
or in rare cases, the monitor wakes up, but shows a black screen.

Hotplugging the dock is recognized (I configured my system to turn off 
the laptop display when an external monitor is present): When I unplug 
and replug the dock, the internal display goes on and off, reliably.
But the external display does not display anything...

In the kernel log I get
Oct 17 08:51:31 lap kernel: [drm:amdgpu_atombios_dp_link_train] *ERROR* clock 
recovery tried 5 times
Oct 17 08:51:31 lap kernel: [drm:amdgpu_atombios_dp_link_train] *ERROR* clock 
recovery failed
but as far as I can tell nothing else.

In many cases, "xset dpms force off ; sleep 2 ; xset dpms force on" 
brings the display back to live. If not, powercycling the display
or unplugging and replugging its displayport cable sometimes helps.
After cold booting (after powering notebook, dock and display off & on),
the situation is worse: 
"xset dpms force off ; sleep 2 ; xset dpms force on" almost never helps.

With amdgpu kernel driver and amdgpu Xwindow driver instead of modesetting,
the situation is much worse: I never managed to get anything displayed
on the external monitor, no matter what tricks I apply,
although the monitor works in text mode before starting X.

My wild guess, without knowing anything about the internals:
X does not realize that the tunnelled displyport link is actually 
completely down after mode switches etc.. It just tries to resync
instead of shutting the link down and completely re-initializing it.
Only a dpms off/on or a monitor off/on forces a complete link reinit.


With ubuntu (dell preinstalled, amdgpu+amdgpu), hotplugging the dock 
or warm booting (without powerdown) with the dock connected 
usually works fine. 
Cold booting (after powerdown) also fails as above.

I was unable to locate any significant difference in the kernel config
between my gentoo and ubuntu. However, ubuntu installs tons of udev rules,
also for drm and the graphic card (I don't have any such rules in gentoo). 
Hence, I suspect that their kernel has the same problem, 
but the problem is worked around by udev actions.


Questions:
* Best place to ask for this problem?
* Any hints about that problem?
* Why does amdgpu+modesetting behave much better than amdgpu+amdgpu?
* Any udev rules known or needed for displayport over hotplug thunderbolt,
or to fix the problem?


Many thanks!

-- 
Prof. Dr. Klaus Kusche
Private address: Rosenberg 41, 07546 Gera, Germany
+49 365 20413058 klaus.kus...@computerix.info https://www.computerix.info
Office address: DHGE Gera, Weg der Freundschaft 4, 07546 Gera, Germany
+49 365 4341 306 klaus.kus...@dhge.de https://www.dhge.de
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu/sdma5: do not execute 0-sized IBs (v2)

2019-10-23 Thread Christian König


Am 22.10.19 um 19:22 schrieb Pelloux-prayer, Pierre-eric:

This seems to help with https://bugs.freedesktop.org/show_bug.cgi?id=111481.

v2: insert a NOP instead of skipping all 0-sized IBs to avoid breaking older hw

Signed-off-by: Pierre-Eric Pelloux-Prayer 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index fb48622c2abd..6e1b25bd1fe7 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -309,6 +309,7 @@ static void gmc_v10_0_flush_gpu_tlb(struct amdgpu_device 
*adev, uint32_t vmid,
  
  	job->vm_pd_addr = amdgpu_gmc_pd_addr(adev->gart.bo);

job->vm_needs_flush = true;
+   job->ibs->ptr[job->ibs->length_dw++] = ring->funcs->nop;
amdgpu_ring_pad_ib(ring, >ibs[0]);
r = amdgpu_job_submit(job, >mman.entity,
  AMDGPU_FENCE_OWNER_UNDEFINED, );


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

RE: [PATCH] drm/amdgpu: Fix SDMA hang when performing VKexample test

2019-10-23 Thread Liu, Aaron

Reviewed-by: Aaron Liu 

BR,
Aaron Liu

> -Original Message-
> From: amd-gfx  On Behalf Of chen
> gong
> Sent: Wednesday, October 23, 2019 2:51 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Gong, Curry 
> Subject: [PATCH] drm/amdgpu: Fix SDMA hang when performing VKexample
> test
> 
> VKexample test hang during Occlusion/SDMA/Varia runs.
> Clear XNACK_WATERMK in reg SDMA0_UTCL1_WATERMK to fix this issue.
> 
> Signed-off-by: chen gong 
> ---
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 63a3792..45bd538 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -254,6 +254,7 @@ static const struct soc15_reg_golden
> golden_settings_sdma_4_3[] = {
>   SOC15_REG_GOLDEN_VALUE(SDMA0, 0,
> mmSDMA0_RLC0_RB_WPTR_POLL_CNTL, 0xfff7, 0x00403000),
>   SOC15_REG_GOLDEN_VALUE(SDMA0, 0,
> mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 0xfff7, 0x00403000),
>   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE,
> 0x03ff, 0x03c0),
> + SOC15_REG_GOLDEN_VALUE(SDMA0, 0,
> mmSDMA0_UTCL1_WATERMK, 0xfc00, 0x)
>  };
> 
>  static u32 sdma_v4_0_get_reg_offset(struct amdgpu_device *adev,
> --
> 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

[PATCH] drm/amdgpu: Fix SDMA hang when performing VKexample test

2019-10-23 Thread chen gong

VKexample test hang during Occlusion/SDMA/Varia runs.
Clear XNACK_WATERMK in reg SDMA0_UTCL1_WATERMK to fix this issue.

Signed-off-by: chen gong 
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 63a3792..45bd538 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -254,6 +254,7 @@ static const struct soc15_reg_golden 
golden_settings_sdma_4_3[] = {
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC0_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_RLC1_RB_WPTR_POLL_CNTL, 
0xfff7, 0x00403000),
SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_PAGE, 0x03ff, 
0x03c0),
+   SOC15_REG_GOLDEN_VALUE(SDMA0, 0, mmSDMA0_UTCL1_WATERMK, 0xfc00, 
0x)
 };
 
 static u32 sdma_v4_0_get_reg_offset(struct amdgpu_device *adev,
-- 
2.7.4

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

Re: [PATCH] UPSTREAM: drm/amd/display: Fix Apple dongle cannot be successfully detected

2019-10-23 Thread S, Shirish

The UPSTREAM tag in the commit message needs to be removed.

On 10/21/2019 1:24 PM, Louis Li wrote:
> [Why]
> External monitor cannot be displayed consistently, if connecting
> via this Apple dongle (A1621, USB Type-C to HDMI).
> By experiments, it is confirmed that the dongle needs 200ms at least
> to be ready for communication, after it sets HPD signal high.
>
> [How]
> When receiving HPD IRQ, delay 500ms at the beginning of handle_hpd_irq().

Am not sure how this delay shall impact on dongles that don't need it,

ideally it should be added as quirk, at least restrict it to these 
specific vendors.

Instead of delay, can you find any parameter to wait for for the 
communication to be ready,

in that way it shall be failsafe.

> Then run the original procedure.
> With this patch applied, the problem cannot be reproduced.
> With other dongles, test results are PASS.
> Test result is PASS after system resumes from suspend.
>
> Signed-off-by: Louis Li 
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +
>   1 file changed, 5 insertions(+)
>
> 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 0aef92b7c037..043ddac73862 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1025,6 +1025,11 @@ static void handle_hpd_irq(void *param)
>   struct drm_device *dev = connector->dev;
>   enum dc_connection_type new_connection_type = dc_connection_none;
>   
> +/* Some monitors/dongles need around 200ms to be ready for communication
> + * after they drive HPD signal high.
> + */
> +mdelay(500);
> +
>   /* In case of failure or MST no need to update connector status or 
> notify the OS
>* since (for MST case) MST does this in it's own context.
>*/

-- 
Regards,
Shirish S

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

38 matches

Mail list logo