Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86
On Tue, Jan 22, 2019 at 10:07:07PM +0100, Ard Biesheuvel wrote: > Yes, so much was clear. And the reason this breaks on some arm64 > systems is because > a) non-snooped PCIe TLP attributes may be ignored, and > b) non-x86 CPUs do not snoop the caches when accessing uncached mappings. > > I don't think there is actually any disagreement on this part. And I > think my patch is reasonable, only Christoph is objecting to it on the > grounds that drivers should not go around the DMA API and create > vmap()s of DMA pages with self chosen attributes. I object to it on various grounds. While the above is correct it really is a mid to long-term fix. But even in the short term your patch just maintains a random list of idefs in a random driver, pokes into the dma-mapping internals and lacks any comments in the code explaining on what is going on, leading to futher cargo culting. So it very clearly is not acceptable. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm: Split out drm_probe_helper.h
Hi Daniel. On Thu, Jan 17, 2019 at 10:03:34PM +0100, Daniel Vetter wrote: > Having the probe helper stuff (which pretty much everyone needs) in > the drm_crtc_helper.h file (which atomic drivers should never need) is > confusing. Split them out. > > To make sure I actually achieved the goal here I went through all > drivers. And indeed, all atomic drivers are now free of > drm_crtc_helper.h includes. How are the plans to get this patchset merged? There were dependencies on onging drmP.h removal in i915 IIRC? I guess my "Minimize drmP.h dependencies" patch-set also have a role in this. This does not hold up any work of mine, mainly wanted to make sure this was not lost between all the other patches. Sam ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86
On Wed, 23 Jan 2019 at 08:15, Christoph Hellwig wrote: > > On Tue, Jan 22, 2019 at 10:07:07PM +0100, Ard Biesheuvel wrote: > > Yes, so much was clear. And the reason this breaks on some arm64 > > systems is because > > a) non-snooped PCIe TLP attributes may be ignored, and > > b) non-x86 CPUs do not snoop the caches when accessing uncached mappings. > > > > I don't think there is actually any disagreement on this part. And I > > think my patch is reasonable, only Christoph is objecting to it on the > > grounds that drivers should not go around the DMA API and create > > vmap()s of DMA pages with self chosen attributes. > > I object to it on various grounds. While the above is correct it really > is a mid to long-term fix. > > But even in the short term your patch just maintains a random list of > idefs in a random driver, pokes into the dma-mapping internals and lacks > any comments in the code explaining on what is going on, leading to > futher cargo culting. So it very clearly is not acceptable. Fair enough. It would be helpful, though, if you could give more guidance on what you do find acceptable. You haven't answered my question on whether you think drivers should be allowed to reason at all about the device's cache coherence. In any case, I think we (you and I) could easily agree on the correct fix being: - introduce DMA_ATTR_NOSNOOP - implement it as a no-op for non-cache coherent devices (since nosnoop is implied) - permit non-x86 architectures to override/ignore it if not supported - rip out all the explicit vmap()/kmap()s of DMA pages from the DRM drivers, and instead, set the DMA_ATTR_NOSNOOP attribute where desired, and always use the mappings provided by the DMA API. The problem is that we will need the DRM/radeon/amdgpu maintainers on board with this, and until this happens, I am stuck in the middle with a broken arm64 system. So, given the above, what /would/ you find acceptable? ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: tighten gpu_recover in mailbox_flr to avoid duplicate recover in sriov
On 01/23/2019 04:50 AM, wentalou wrote: > sriov's gpu_recover inside xgpu_ai_mailbox_flr_work would cause duplicate > recover in TDR. > TDR's gpu_recover would be triggered by amdgpu_job_timedout, > that could avoid vk-cts failure by unexpected recover. > > Change-Id: Ifcba4ac43a0229ae19061aad3b0ddc96957ff9c6 > Signed-off-by: wentalou > --- > drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c > b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c > index b11a1c17..f227633 100644 > --- a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c > +++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c > @@ -266,7 +266,7 @@ static void xgpu_ai_mailbox_flr_work(struct work_struct > *work) > } > > /* Trigger recovery for world switch failure if no TDR */ > - if (amdgpu_device_should_recover_gpu(adev)) > + if (amdgpu_device_should_recover_gpu(adev) && amdgpu_lockup_timeout == > 0) Not sure I fully understand the intent here but amdgpu_lockup_timeout == 0 is forced to 1 in amdgpu_device_check_arguments and hence not an indication of disabled gpu recover - it will still happen. Andrey > amdgpu_device_gpu_recover(adev, NULL); > } > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 04/20] drm/amd/display: Know what a pageflip is
On 01/22/2019 01:28 PM, sunpeng...@amd.com wrote:
> From: David Francis
>
> [Why]
> We were assuming that any commit with allow_modeset == false
> was a pageflip. This was against drm intention and only
> worked by sheer luck
>
> [How]
> A pageflip is the change from one framebuffer to another
What about other references to state->allow_modeset in DM code ? You
have one in dm_determine_update_type_for_commit which will actually
cause you to assume fast update because of state->allow_modeset == false.
Also - what about other types of plane updates which are more drastic
(require full update) but will also have different FB, can't you wrongly
assume they require page flip while it's actually needs
commit_planes_to_stream ?
Andrey
>
> Signed-off-by: David Francis
> Reviewed-by: Harry Wentland
> Reviewed-by: Nicholas Kazlauskas
> Acked-by: Leo Li
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> 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 405c263..db060da 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4995,6 +4995,8 @@ static void amdgpu_dm_commit_planes(struct
> drm_atomic_state *state,
> struct drm_crtc *crtc = new_plane_state->crtc;
> struct drm_crtc_state *new_crtc_state;
> struct drm_framebuffer *fb = new_plane_state->fb;
> + struct amdgpu_framebuffer *afb = to_amdgpu_framebuffer(fb);
> + struct amdgpu_framebuffer *old_afb =
> to_amdgpu_framebuffer(old_plane_state->fb);
> bool pflip_needed;
> struct dm_plane_state *dm_new_plane_state =
> to_dm_plane_state(new_plane_state);
>
> @@ -5010,7 +5012,8 @@ static void amdgpu_dm_commit_planes(struct
> drm_atomic_state *state,
> if (!new_crtc_state->active)
> continue;
>
> - pflip_needed = !state->allow_modeset;
> + pflip_needed = old_plane_state->fb &&
> + (old_plane_state->fb != new_plane_state->fb ||
> afb->address != old_afb->address);
>
> spin_lock_irqsave(>dev->event_lock, flags);
> if (acrtc_attach->pflip_status != AMDGPU_FLIP_NONE) {
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 04/20] drm/amd/display: Know what a pageflip is
On 1/23/19 11:08 AM, Grodzovsky, Andrey wrote:
>
>
> On 01/22/2019 01:28 PM, sunpeng...@amd.com wrote:
>> From: David Francis
>>
>> [Why]
>> We were assuming that any commit with allow_modeset == false
>> was a pageflip. This was against drm intention and only
>> worked by sheer luck
>>
>> [How]
>> A pageflip is the change from one framebuffer to another
>
> What about other references to state->allow_modeset in DM code ? You
> have one in dm_determine_update_type_for_commit which will actually
> cause you to assume fast update because of state->allow_modeset == false.
> Also - what about other types of plane updates which are more drastic
> (require full update) but will also have different FB, can't you wrongly
> assume they require page flip while it's actually needs
> commit_planes_to_stream ?
>
> Andrey
Other references (in atomic check) will still need to be addressed after
this patch. This patch only fixes page-flips not working when
state->allow_modeset = true. Everything else is mostly just about
enabling us to do fast updates more frequently for better performance.
You're right about the fast vs full update bit (since we won't be
waiting for vblank) but this gets addressed with another patch in this
series, "drm/amd/display: Call into DC once per multiplane flip".
I don't remember the ordering of which these patches were originally
applied but they may have changed when sent out to the mailing list.
For bisection purposes I think the ordering of:
[PATCH 06/20] drm/amd/display: Let updates with no scaling changes be fast
[PATCH 03/20] drm/amd/display: Simplify underscan and ABM commit
[PATCH 05/20] drm/amd/display: Call into DC once per multiplane flip
[PATCH 07/20] drm/amd/display: Perform plane updates only when needed
[PATCH 04/20] drm/amd/display: Know what a pageflip is
...probably makes the most logical sense.
Nicholas Kazlauskas
>
>>
>> Signed-off-by: David Francis
>> Reviewed-by: Harry Wentland
>> Reviewed-by: Nicholas Kazlauskas
>> Acked-by: Leo Li
>> ---
>>drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 -
>>1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> 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 405c263..db060da 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -4995,6 +4995,8 @@ static void amdgpu_dm_commit_planes(struct
>> drm_atomic_state *state,
>> struct drm_crtc *crtc = new_plane_state->crtc;
>> struct drm_crtc_state *new_crtc_state;
>> struct drm_framebuffer *fb = new_plane_state->fb;
>> +struct amdgpu_framebuffer *afb = to_amdgpu_framebuffer(fb);
>> +struct amdgpu_framebuffer *old_afb =
>> to_amdgpu_framebuffer(old_plane_state->fb);
>> bool pflip_needed;
>> struct dm_plane_state *dm_new_plane_state =
>> to_dm_plane_state(new_plane_state);
>>
>> @@ -5010,7 +5012,8 @@ static void amdgpu_dm_commit_planes(struct
>> drm_atomic_state *state,
>> if (!new_crtc_state->active)
>> continue;
>>
>> -pflip_needed = !state->allow_modeset;
>> +pflip_needed = old_plane_state->fb &&
>> +(old_plane_state->fb != new_plane_state->fb ||
>> afb->address != old_afb->address);
>>
>> spin_lock_irqsave(>dev->event_lock, flags);
>> if (acrtc_attach->pflip_status != AMDGPU_FLIP_NONE) {
>
> ___
> 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: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86
I think we just want a driver-local check for those combinations
where we know this hack actually works, which really just seems
to be x86-64 with PAT. Something like the patch below, but maybe with
even more strong warnings to not do something like this elsewhere:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 728e15e5d68a..5fe657f20232 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -456,33 +456,16 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
bo->flags = bp->flags;
-#ifdef CONFIG_X86_32
- /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
-* See https://bugs.freedesktop.org/show_bug.cgi?id=84627
-*/
- bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
-#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
- /* Don't try to enable write-combining when it can't work, or things
-* may be slow
-* See https://bugs.freedesktop.org/show_bug.cgi?id=88758
-*/
-
-#ifndef CONFIG_COMPILE_TEST
-#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance \
-thanks to write-combining
-#endif
-
- if (bo->flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
- DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT for
"
- "better performance thanks to write-combining\n");
- bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
-#else
- /* For architectures that don't support WC memory,
-* mask out the WC flag from the BO
+ /*
+* Don't try to enable write-combined CPU mappings unless we 100%
+* positively know it works, otherwise there may be dragons.
+*
+* See:
+* - https://bugs.freedesktop.org/show_bug.cgi?id=88758
+* - https://bugs.freedesktop.org/show_bug.cgi?id=84627
*/
- if (!drm_arch_can_wc_memory())
+ if (!(IS_ENABLED(CONFIG_X86_64) && IS_ENABLED(CONFIG_X86_PAT)))
bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
-#endif
bo->tbo.bdev = >mman.bdev;
amdgpu_bo_placement_from_domain(bo, bp->domain);
diff --git a/drivers/gpu/drm/radeon/radeon_object.c
b/drivers/gpu/drm/radeon/radeon_object.c
index 833e909706a9..c1fb5ad4ab9a 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -226,32 +226,17 @@ int radeon_bo_create(struct radeon_device *rdev,
if (rdev->family >= CHIP_RV610 && rdev->family <= CHIP_RV635)
bo->flags &= ~(RADEON_GEM_GTT_WC | RADEON_GEM_GTT_UC);
-#ifdef CONFIG_X86_32
- /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
-* See https://bugs.freedesktop.org/show_bug.cgi?id=84627
-*/
- bo->flags &= ~(RADEON_GEM_GTT_WC | RADEON_GEM_GTT_UC);
-#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
- /* Don't try to enable write-combining when it can't work, or things
-* may be slow
-* See https://bugs.freedesktop.org/show_bug.cgi?id=88758
-*/
-#ifndef CONFIG_COMPILE_TEST
-#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance \
-thanks to write-combining
-#endif
- if (bo->flags & RADEON_GEM_GTT_WC)
- DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT for
"
- "better performance thanks to write-combining\n");
- bo->flags &= ~(RADEON_GEM_GTT_WC | RADEON_GEM_GTT_UC);
-#else
- /* For architectures that don't support WC memory,
-* mask out the WC flag from the BO
+ /*
+* Don't try to enable write-combined CPU mappings unless we 100%
+* positively know it works, otherwise there may be dragons.
+*
+* See:
+* - https://bugs.freedesktop.org/show_bug.cgi?id=88758
+* - https://bugs.freedesktop.org/show_bug.cgi?id=84627
*/
- if (!drm_arch_can_wc_memory())
- bo->flags &= ~RADEON_GEM_GTT_WC;
-#endif
+ if (!(IS_ENABLED(CONFIG_X86_64) && IS_ENABLED(CONFIG_X86_PAT)))
+ bo->flags &= ~(RADEON_GEM_GTT_WC | RADEON_GEM_GTT_UC);
radeon_ttm_placement_from_domain(bo, domain);
/* Kernel allocation are uninterruptible */
diff --git a/include/drm/drm_cache.h b/include/drm/drm_cache.h
index bfe1639df02d..6c3960f4c477 100644
--- a/include/drm/drm_cache.h
+++ b/include/drm/drm_cache.h
@@ -40,16 +40,4 @@ void drm_clflush_sg(struct sg_table *st);
void drm_clflush_virt_range(void *addr, unsigned long length);
u64 drm_get_max_iomem(void);
-
-static inline bool drm_arch_can_wc_memory(void)
-{
-#if defined(CONFIG_PPC) && !defined(CONFIG_NOT_COHERENT_CACHE)
- return false;
-#elif defined(CONFIG_MIPS) && defined(CONFIG_CPU_LOONGSON3)
- return false;
-#else
- return true;
-#endif
-}
-
#endif
___
amd-gfx mailing list
Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86
On Wed, 23 Jan 2019 at 17:44, Christoph Hellwig wrote:
>
> I think we just want a driver-local check for those combinations
> where we know this hack actually works, which really just seems
> to be x86-64 with PAT. Something like the patch below, but maybe with
> even more strong warnings to not do something like this elsewhere:
>
I agree that your patch seems like the right way to ensure that the WC
optimization hack is only used where we know it works.
But my concern is that it seems likely that non-cache coherent
implementations are relying on this hack as well. There must be a
reason that this hack is only disabled for PowerPC platforms if they
are cache coherent, for instance, and I suspect that that reason is
that the hack is the only thing ensuring that the CPU mapping
attributes match the device ones used for these buffers (the vmap()ed
ones), whereas the rings and other consistent data structures are
using the DMA API as intended, and thus getting uncached attributes in
the correct way.
Given that the same concern applies to ARM (and arm64 to some extent),
I'd be cautious to disable this optimization entirely, and this is why
I ended up disabling it only for cache coherent devices.
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 728e15e5d68a..5fe657f20232 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -456,33 +456,16 @@ static int amdgpu_bo_do_create(struct amdgpu_device
> *adev,
>
> bo->flags = bp->flags;
>
> -#ifdef CONFIG_X86_32
> - /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
> -* See https://bugs.freedesktop.org/show_bug.cgi?id=84627
> -*/
> - bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> -#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
> - /* Don't try to enable write-combining when it can't work, or things
> -* may be slow
> -* See https://bugs.freedesktop.org/show_bug.cgi?id=88758
> -*/
> -
> -#ifndef CONFIG_COMPILE_TEST
> -#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance
> \
> -thanks to write-combining
> -#endif
> -
> - if (bo->flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
> - DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT
> for "
> - "better performance thanks to
> write-combining\n");
> - bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> -#else
> - /* For architectures that don't support WC memory,
> -* mask out the WC flag from the BO
> + /*
> +* Don't try to enable write-combined CPU mappings unless we 100%
> +* positively know it works, otherwise there may be dragons.
> +*
> +* See:
> +* - https://bugs.freedesktop.org/show_bug.cgi?id=88758
> +* - https://bugs.freedesktop.org/show_bug.cgi?id=84627
> */
> - if (!drm_arch_can_wc_memory())
> + if (!(IS_ENABLED(CONFIG_X86_64) && IS_ENABLED(CONFIG_X86_PAT)))
> bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> -#endif
>
> bo->tbo.bdev = >mman.bdev;
> amdgpu_bo_placement_from_domain(bo, bp->domain);
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c
> b/drivers/gpu/drm/radeon/radeon_object.c
> index 833e909706a9..c1fb5ad4ab9a 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -226,32 +226,17 @@ int radeon_bo_create(struct radeon_device *rdev,
> if (rdev->family >= CHIP_RV610 && rdev->family <= CHIP_RV635)
> bo->flags &= ~(RADEON_GEM_GTT_WC | RADEON_GEM_GTT_UC);
>
> -#ifdef CONFIG_X86_32
> - /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
> -* See https://bugs.freedesktop.org/show_bug.cgi?id=84627
> -*/
> - bo->flags &= ~(RADEON_GEM_GTT_WC | RADEON_GEM_GTT_UC);
> -#elif defined(CONFIG_X86) && !defined(CONFIG_X86_PAT)
> - /* Don't try to enable write-combining when it can't work, or things
> -* may be slow
> -* See https://bugs.freedesktop.org/show_bug.cgi?id=88758
> -*/
> -#ifndef CONFIG_COMPILE_TEST
> -#warning Please enable CONFIG_MTRR and CONFIG_X86_PAT for better performance
> \
> -thanks to write-combining
> -#endif
>
> - if (bo->flags & RADEON_GEM_GTT_WC)
> - DRM_INFO_ONCE("Please enable CONFIG_MTRR and CONFIG_X86_PAT
> for "
> - "better performance thanks to
> write-combining\n");
> - bo->flags &= ~(RADEON_GEM_GTT_WC | RADEON_GEM_GTT_UC);
> -#else
> - /* For architectures that don't support WC memory,
> -* mask out the WC flag from the BO
> + /*
> +* Don't try to enable write-combined CPU mappings unless we 100%
> +* positively know it works, otherwise there may be dragons.
> +*
> +* See:
> +* -
Re: [PATCH AUTOSEL 4.20 034/117] drm/amdgpu: Correct get_crtc_scanoutpos behavior when vpos >= vtotal
On Wed, Jan 09, 2019 at 10:20:49AM +0100, Michel Dänzer wrote: On 2019-01-08 8:25 p.m., Sasha Levin wrote: From: Nicholas Kazlauskas [ Upstream commit 520f08df45fbe300ed650da786a74093d658b7e1 ] When variable refresh rate is active [...] Variable refresh rate (FreeSync) support is only landing in 5.0, therefore this fix isn't needed in older kernels. I'll drop it, thank you. -- Thanks, Sasha ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: tighten gpu_recover in mailbox_flr to avoid duplicate recover in sriov
sriov's gpu_recover inside xgpu_ai_mailbox_flr_work would cause duplicate recover in TDR. TDR's gpu_recover would be triggered by amdgpu_job_timedout, that could avoid vk-cts failure by unexpected recover. Change-Id: Ifcba4ac43a0229ae19061aad3b0ddc96957ff9c6 Signed-off-by: wentalou --- drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c index b11a1c17..f227633 100644 --- a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c +++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c @@ -266,7 +266,7 @@ static void xgpu_ai_mailbox_flr_work(struct work_struct *work) } /* Trigger recovery for world switch failure if no TDR */ - if (amdgpu_device_should_recover_gpu(adev)) + if (amdgpu_device_should_recover_gpu(adev) && amdgpu_lockup_timeout == 0) amdgpu_device_gpu_recover(adev, NULL); } -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 04/20] drm/amd/display: Know what a pageflip is
On 2019-01-22 7:28 p.m., sunpeng...@amd.com wrote: > From: David Francis > > [Why] > We were assuming that any commit with allow_modeset == false > was a pageflip. This was against drm intention and only > worked by sheer luck > > [How] > A pageflip is the change from one framebuffer to another > > Signed-off-by: David Francis > Reviewed-by: Harry Wentland > Reviewed-by: Nicholas Kazlauskas > Acked-by: Leo Li > > [...] > > @@ -5010,7 +5012,8 @@ static void amdgpu_dm_commit_planes(struct > drm_atomic_state *state, > if (!new_crtc_state->active) > continue; > > - pflip_needed = !state->allow_modeset; > + pflip_needed = old_plane_state->fb && > + (old_plane_state->fb != new_plane_state->fb || > afb->address != old_afb->address); The second check after the || cannot be true if old_plane_state->fb == new_plane_state->fb, so it's dead code. -- 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: [PATCH] drm/amdgpu: sriov put csa below AMDGPU_GMC_HOLE
I think it would be better to restrict max_pfn in general under SRIOV.
When the higher address space doesn't work for the CSA it will probably
cause problems elsewhere as well.
Christian.
Am 22.01.19 um 11:49 schrieb wentalou:
since vm_size enlarged to 0x4 GB,
sriov need to put csa below AMDGPU_GMC_HOLE.
or amdgpu_vm_alloc_pts would receive saddr among AMDGPU_GMC_HOLE,
and result in a range fault interrupt IIRC.
Change-Id: I405a25a01d949f3130889b346f71bedad8ebcae7
Signed-off-by: Wenta Lou
---
drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 6 --
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 --
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
index dd3bd01..7a93c36 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
@@ -26,8 +26,10 @@
uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev)
{
- uint64_t addr = min(adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT,
- AMDGPU_GMC_HOLE_START);
+ uint64_t addr = adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT;
+ /* sriov put csa below AMDGPU_GMC_HOLE */
+ if (amdgpu_sriov_vf(adev))
+ addr = min(addr, AMDGPU_GMC_HOLE_START);
addr -= AMDGPU_VA_RESERVED_SIZE;
addr = amdgpu_gmc_sign_extend(addr);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index f87f717..cf9ec28 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -707,8 +707,10 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void
*data, struct drm_file
vm_size = min(vm_size, 1ULL << 40);
dev_info.virtual_address_offset = AMDGPU_VA_RESERVED_SIZE;
- dev_info.virtual_address_max =
- min(vm_size, AMDGPU_GMC_HOLE_START);
+ if (amdgpu_sriov_vf(adev))
+ dev_info.virtual_address_max = min(vm_size,
AMDGPU_GMC_HOLE_START - AMDGPU_VA_RESERVED_SIZE);
+ else
+ dev_info.virtual_address_max = min(vm_size,
AMDGPU_GMC_HOLE_START);
if (vm_size > AMDGPU_GMC_HOLE_START) {
dev_info.high_va_offset = AMDGPU_GMC_HOLE_END;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 2/2] drm/amd/powerplay: enable MGPU fan boost feature on Vega10
This feature is only seen on the latest vbios. No requirement for SMU fw.
Regards,
Evan
> -Original Message-
> From: Alex Deucher
> Sent: 2019年1月23日 12:54
> To: Quan, Evan
> Cc: amd-gfx list
> Subject: Re: [PATCH 2/2] drm/amd/powerplay: enable MGPU fan boost
> feature on Vega10
>
> On Tue, Jan 22, 2019 at 10:43 PM Evan Quan wrote:
> >
> > For those SKUs which support this feature only.
> >
> > Change-Id: I74de00204d93f951e04073e5c4c4ce9c0d34f662
> > Signed-off-by: Evan Quan
>
> Is there a minimum smu version number required to support this? Is it safe
> to setup the structures and enable it if the using older smu fw?
> With that clarified, the series is:
> Reviewed-by: Alex Deucher
>
> Alex
>
> > ---
> > .../drm/amd/powerplay/hwmgr/vega10_hwmgr.c| 1 +
> > .../drm/amd/powerplay/hwmgr/vega10_thermal.c | 37
> >
> +++ .../drm/amd/powerplay/hwmgr/vega10_thermal.
> h |
> > 1 +
> > 3 files changed, 39 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> > b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> > index f0ba4254361b..1a4c6467a36d 100644
> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> > @@ -5168,6 +5168,7 @@ static const struct pp_hwmgr_func
> vega10_hwmgr_funcs = {
> > .set_asic_baco_state = vega10_baco_set_state,
> > .get_ppfeature_status = vega10_get_ppfeature_status,
> > .set_ppfeature_status = vega10_set_ppfeature_status,
> > + .enable_mgpu_fan_boost = vega10_enable_mgpu_fan_boost,
> > };
> >
> > int vega10_hwmgr_init(struct pp_hwmgr *hwmgr) diff --git
> > a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.c
> > b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.c
> > index 3f807d6c95ce..ba8763daa380 100644
> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.c
> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.c
> > @@ -556,6 +556,43 @@ int vega10_thermal_setup_fan_table(struct
> pp_hwmgr *hwmgr)
> > return ret;
> > }
> >
> > +int vega10_enable_mgpu_fan_boost(struct pp_hwmgr *hwmgr) {
> > + struct vega10_hwmgr *data = hwmgr->backend;
> > + PPTable_t *table = &(data->smc_state_table.pp_table);
> > + int ret;
> > +
> > + if (!data->smu_features[GNLD_FAN_CONTROL].supported)
> > + return 0;
> > +
> > + if (!hwmgr->thermal_controller.advanceFanControlParameters.
> > + usMGpuThrottlingRPMLimit)
> > + return 0;
> > +
> > + table->FanThrottlingRpm = hwmgr->thermal_controller.
> > +
> > + advanceFanControlParameters.usMGpuThrottlingRPMLimit;
> > +
> > + ret = smum_smc_table_manager(hwmgr,
> > + (uint8_t
> > *)(&(data->smc_state_table.pp_table)),
> > + PPTABLE, false);
> > + if (ret) {
> > + pr_info("Failed to update fan control table in pptable!");
> > + return ret;
> > + }
> > +
> > + ret = vega10_disable_fan_control_feature(hwmgr);
> > + if (ret) {
> > + pr_info("Attempt to disable SMC fan control feature
> > failed!");
> > + return ret;
> > + }
> > +
> > + ret = vega10_enable_fan_control_feature(hwmgr);
> > + if (ret)
> > + pr_info("Attempt to enable SMC fan control feature
> > + failed!");
> > +
> > + return ret;
> > +}
> > +
> > /**
> > * Start the fan control on the SMC.
> > * @paramhwmgr the address of the powerplay hardware manager.
> > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.h
> > b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.h
> > index 21e7c4dfa2ca..4a0ede7c1f07 100644
> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.h
> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_thermal.h
> > @@ -73,6 +73,7 @@ extern int vega10_thermal_disable_alert(struct
> > pp_hwmgr *hwmgr); extern int
> > vega10_fan_ctrl_start_smc_fan_control(struct pp_hwmgr *hwmgr);
> extern int vega10_start_thermal_controller(struct pp_hwmgr *hwmgr,
> > struct PP_TemperatureRange *range);
> > +extern int vega10_enable_mgpu_fan_boost(struct pp_hwmgr *hwmgr);
> >
> >
> > #endif
> > --
> > 2.20.1
> >
> > ___
> > 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
[pull] amdgpu drm-fixes-5.0
Hi Dave, Daniel, Just a couple of fixes for 5.0: - Overclock fix for vega10 - Hybrid gfx laptop fix The following changes since commit 35dad45d5cad3c9ca8d6a338cbf668cd7ea86469: drm/amd/display: Detach backlight from stream (2019-01-16 17:11:47 -0500) are available in the Git repository at: git://people.freedesktop.org/~agd5f/linux drm-fixes-5.0 for you to fetch changes up to 6d87dc97eb3341de3f7b1efa3156cb0e014f4a96: drm/amd/powerplay: OD setting fix on Vega10 (2019-01-21 15:01:43 -0500) Alex Deucher (1): drm/amdgpu: Add APTX quirk for Lenovo laptop Kenneth Feng (1): drm/amd/powerplay: OD setting fix on Vega10 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c | 1 + .../amd/powerplay/hwmgr/vega10_processpptables.c | 22 +- 2 files changed, 22 insertions(+), 1 deletion(-) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: sriov restrict max_pfn below AMDGPU_GMC_HOLE
sriov need to restrict max_pfn below AMDGPU_GMC_HOLE.
access the hole results in a range fault interrupt IIRC.
Change-Id: I0add197a24a54388a128a545056e9a9f0330abfb
Signed-off-by: Wentao Lou
---
drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 3 +--
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 6 +-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
index dd3bd01..7e22be7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
@@ -26,8 +26,7 @@
uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev)
{
- uint64_t addr = min(adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT,
- AMDGPU_GMC_HOLE_START);
+ uint64_t addr = adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT;
addr -= AMDGPU_VA_RESERVED_SIZE;
addr = amdgpu_gmc_sign_extend(addr);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 9c082f9..600259b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -965,7 +965,11 @@ static int gmc_v9_0_sw_init(void *handle)
* vm size is 256TB (48bit), maximum size of Vega10,
* block size 512 (9bit)
*/
- amdgpu_vm_adjust_size(adev, 256 * 1024, 9, 3, 48);
+ /* sriov restrict max_pfn below AMDGPU_GMC_HOLE */
+ if (amdgpu_sriov_vf(adev))
+ amdgpu_vm_adjust_size(adev, 256 * 1024, 9, 3, 47);
+ else
+ amdgpu_vm_adjust_size(adev, 256 * 1024, 9, 3, 48);
break;
default:
break;
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu: tighten gpu_recover in mailbox_flr to avoid duplicate recover in sriov
++Monk Yes amdgpu_lockup_timeout was forced to 1 in current code, that means amdgpu_device_gpu_recover was not triggered inside xgpu_ai_mailbox_flr_work. For sriov, amdgpu_device_gpu_recover was already called by amdgpu_job_timedout, unexpected amdgpu_device_gpu_recover called by xgpu_ai_mailbox_flr_work would make vk-cts failed. -Original Message- From: Grodzovsky, Andrey Sent: Thursday, January 24, 2019 12:27 AM To: Lou, Wentao ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/amdgpu: tighten gpu_recover in mailbox_flr to avoid duplicate recover in sriov On 01/23/2019 04:50 AM, wentalou wrote: > sriov's gpu_recover inside xgpu_ai_mailbox_flr_work would cause duplicate > recover in TDR. > TDR's gpu_recover would be triggered by amdgpu_job_timedout, that > could avoid vk-cts failure by unexpected recover. > > Change-Id: Ifcba4ac43a0229ae19061aad3b0ddc96957ff9c6 > Signed-off-by: wentalou > --- > drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c > b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c > index b11a1c17..f227633 100644 > --- a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c > +++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c > @@ -266,7 +266,7 @@ static void xgpu_ai_mailbox_flr_work(struct work_struct > *work) > } > > /* Trigger recovery for world switch failure if no TDR */ > - if (amdgpu_device_should_recover_gpu(adev)) > + if (amdgpu_device_should_recover_gpu(adev) && amdgpu_lockup_timeout > +== 0) Not sure I fully understand the intent here but amdgpu_lockup_timeout == 0 is forced to 1 in amdgpu_device_check_arguments and hence not an indication of disabled gpu recover - it will still happen. Andrey > amdgpu_device_gpu_recover(adev, NULL); > } > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/4] drm/amd/powerplay: support Vega10 SOCclk and DCEFclk dpm level settings
Enable SOCclk and DCEFclk dpm level retrieving and setting on Vega10.
Change-Id: I5bcd2e9984b5b7fdb991718d8384d65290672df6
Signed-off-by: Evan Quan
---
.../drm/amd/powerplay/hwmgr/vega10_hwmgr.c| 83 +++
.../drm/amd/powerplay/hwmgr/vega10_hwmgr.h| 1 +
2 files changed, 84 insertions(+)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index d1e262844619..a08e4fa3ae59 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -72,6 +72,21 @@ static const uint32_t channel_number[] = {1, 2, 0, 4, 0, 8,
0, 16, 2};
#define DF_CS_AON0_DramBaseAddress0__IntLvAddrSel_MASK
0x0700L
#define DF_CS_AON0_DramBaseAddress0__DramBaseAddr_MASK
0xF000L
+typedef enum {
+ CLK_SMNCLK = 0,
+ CLK_SOCCLK,
+ CLK_MP0CLK,
+ CLK_MP1CLK,
+ CLK_LCLK,
+ CLK_DCEFCLK,
+ CLK_VCLK,
+ CLK_DCLK,
+ CLK_ECLK,
+ CLK_UCLK,
+ CLK_GFXCLK,
+ CLK_COUNT,
+} CLOCK_ID_e;
+
static const ULONG PhwVega10_Magic = (ULONG)(PHM_VIslands_Magic);
struct vega10_power_state *cast_phw_vega10_power_state(
@@ -3486,6 +3501,17 @@ static int vega10_upload_dpm_bootup_level(struct
pp_hwmgr *hwmgr)
}
}
+ if (!data->registry_data.socclk_dpm_key_disabled) {
+ if (data->smc_state_table.soc_boot_level !=
+
data->dpm_table.soc_table.dpm_state.soft_min_level) {
+ smum_send_msg_to_smc_with_parameter(hwmgr,
+ PPSMC_MSG_SetSoftMinSocclkByIndex,
+ data->smc_state_table.soc_boot_level);
+ data->dpm_table.soc_table.dpm_state.soft_min_level =
+ data->smc_state_table.soc_boot_level;
+ }
+ }
+
return 0;
}
@@ -3517,6 +3543,17 @@ static int vega10_upload_dpm_max_level(struct pp_hwmgr
*hwmgr)
}
}
+ if (!data->registry_data.socclk_dpm_key_disabled) {
+ if (data->smc_state_table.soc_max_level !=
+ data->dpm_table.soc_table.dpm_state.soft_max_level) {
+ smum_send_msg_to_smc_with_parameter(hwmgr,
+ PPSMC_MSG_SetSoftMaxSocclkByIndex,
+ data->smc_state_table.soc_max_level);
+ data->dpm_table.soc_table.dpm_state.soft_max_level =
+ data->smc_state_table.soc_max_level;
+ }
+ }
+
return 0;
}
@@ -4029,6 +4066,24 @@ static int vega10_force_clock_level(struct pp_hwmgr
*hwmgr,
break;
+ case PP_SOCCLK:
+ data->smc_state_table.soc_boot_level = mask ? (ffs(mask) - 1) :
0;
+ data->smc_state_table.soc_max_level = mask ? (fls(mask) - 1) :
0;
+
+ PP_ASSERT_WITH_CODE(!vega10_upload_dpm_bootup_level(hwmgr),
+ "Failed to upload boot level to lowest!",
+ return -EINVAL);
+
+ PP_ASSERT_WITH_CODE(!vega10_upload_dpm_max_level(hwmgr),
+ "Failed to upload dpm max level to highest!",
+ return -EINVAL);
+
+ break;
+
+ case PP_DCEFCLK:
+ pr_info("Setting DCEFCLK min/max dpm level is not
supported!\n");
+ break;
+
case PP_PCIE:
default:
break;
@@ -4274,6 +4329,8 @@ static int vega10_print_clock_levels(struct pp_hwmgr
*hwmgr,
struct vega10_hwmgr *data = hwmgr->backend;
struct vega10_single_dpm_table *sclk_table =
&(data->dpm_table.gfx_table);
struct vega10_single_dpm_table *mclk_table =
&(data->dpm_table.mem_table);
+ struct vega10_single_dpm_table *soc_table =
&(data->dpm_table.soc_table);
+ struct vega10_single_dpm_table *dcef_table =
&(data->dpm_table.dcef_table);
struct vega10_pcie_table *pcie_table = &(data->dpm_table.pcie_table);
struct vega10_odn_clock_voltage_dependency_table *podn_vdd_dep = NULL;
@@ -4304,6 +4361,32 @@ static int vega10_print_clock_levels(struct pp_hwmgr
*hwmgr,
i, mclk_table->dpm_levels[i].value /
100,
(i == now) ? "*" : "");
break;
+ case PP_SOCCLK:
+ if (data->registry_data.socclk_dpm_key_disabled)
+ break;
+
+ smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetCurrentSocclkIndex);
+ now = smum_get_argument(hwmgr);
+
+ for (i = 0; i < soc_table->count; i++)
+ size += sprintf(buf + size, "%d: %uMhz %s\n",
+ i, soc_table->dpm_levels[i].value / 100,
+
[PATCH 2/4] drm/amd/powerplay: support Vega10 retrieving and setting ppfeatures
Enable retrieving and setting ppfeatures on Vega10.
Change-Id: I44ea8789ad3c59cfb40a367665a6190f4405610e
Signed-off-by: Evan Quan
---
.../drm/amd/powerplay/hwmgr/vega10_hwmgr.c| 101 ++
1 file changed, 101 insertions(+)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index a08e4fa3ae59..f0ba4254361b 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -4323,6 +4323,105 @@ static int
vega10_set_watermarks_for_clocks_ranges(struct pp_hwmgr *hwmgr,
return result;
}
+static int vega10_get_ppfeature_status(struct pp_hwmgr *hwmgr, char *buf)
+{
+ static const char *ppfeature_name[] = {
+ "DPM_PREFETCHER",
+ "GFXCLK_DPM",
+ "UCLK_DPM",
+ "SOCCLK_DPM",
+ "UVD_DPM",
+ "VCE_DPM",
+ "ULV",
+ "MP0CLK_DPM",
+ "LINK_DPM",
+ "DCEFCLK_DPM",
+ "AVFS",
+ "GFXCLK_DS",
+ "SOCCLK_DS",
+ "LCLK_DS",
+ "PPT",
+ "TDC",
+ "THERMAL",
+ "GFX_PER_CU_CG",
+ "RM",
+ "DCEFCLK_DS",
+ "ACDC",
+ "VR0HOT",
+ "VR1HOT",
+ "FW_CTF",
+ "LED_DISPLAY",
+ "FAN_CONTROL",
+ "FAST_PPT",
+ "DIDT",
+ "ACG",
+ "PCC_LIMIT"};
+ static const char *output_title[] = {
+ "FEATURES",
+ "BITMASK",
+ "ENABLEMENT"};
+ uint64_t features_enabled;
+ int i;
+ int ret = 0;
+ int size = 0;
+
+ ret = vega10_get_enabled_smc_features(hwmgr, _enabled);
+ PP_ASSERT_WITH_CODE(!ret,
+ "[EnableAllSmuFeatures] Failed to get enabled smc
features!",
+ return ret);
+
+ size += sprintf(buf + size, "Current ppfeatures: 0x%016llx\n",
features_enabled);
+ size += sprintf(buf + size, "%-19s %-22s %s\n",
+ output_title[0],
+ output_title[1],
+ output_title[2]);
+ for (i = 0; i < GNLD_FEATURES_MAX; i++) {
+ size += sprintf(buf + size, "%-19s 0x%016llx %6s\n",
+ ppfeature_name[i],
+ 1ULL << i,
+ (features_enabled & (1ULL << i)) ? "Y"
: "N");
+ }
+
+ return size;
+}
+
+static int vega10_set_ppfeature_status(struct pp_hwmgr *hwmgr, uint64_t
new_ppfeature_masks)
+{
+ uint64_t features_enabled;
+ uint64_t features_to_enable;
+ uint64_t features_to_disable;
+ int ret = 0;
+
+ if (new_ppfeature_masks >= (1ULL << GNLD_FEATURES_MAX))
+ return -EINVAL;
+
+ ret = vega10_get_enabled_smc_features(hwmgr, _enabled);
+ if (ret)
+ return ret;
+
+ features_to_disable =
+ (features_enabled ^ new_ppfeature_masks) & features_enabled;
+ features_to_enable =
+ (features_enabled ^ new_ppfeature_masks) ^ features_to_disable;
+
+ pr_debug("features_to_disable 0x%llx\n", features_to_disable);
+ pr_debug("features_to_enable 0x%llx\n", features_to_enable);
+
+ if (features_to_disable) {
+ ret = vega10_enable_smc_features(hwmgr, false,
features_to_disable);
+ if (ret)
+ return ret;
+ }
+
+ if (features_to_enable) {
+ ret = vega10_enable_smc_features(hwmgr, true,
features_to_enable);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static int vega10_print_clock_levels(struct pp_hwmgr *hwmgr,
enum pp_clock_type type, char *buf)
{
@@ -5067,6 +5166,8 @@ static const struct pp_hwmgr_func vega10_hwmgr_funcs = {
.get_asic_baco_capability = vega10_baco_get_capability,
.get_asic_baco_state = vega10_baco_get_state,
.set_asic_baco_state = vega10_baco_set_state,
+ .get_ppfeature_status = vega10_get_ppfeature_status,
+ .set_ppfeature_status = vega10_set_ppfeature_status,
};
int vega10_hwmgr_init(struct pp_hwmgr *hwmgr)
--
2.20.1
[PATCH 3/4] drm/amd/powerplay: support Vega12 SOCclk and DCEFclk dpm level settings
Enable SOCclk and DCEFclk dpm level retrieving and setting on Vega12.
Change-Id: Ieb3e06dcdfe5dd67c3f27f6fdb9a6bb408034faf
Signed-off-by: Evan Quan
---
.../drm/amd/powerplay/hwmgr/vega12_hwmgr.c| 98 ++-
1 file changed, 97 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
index 0c8212902275..45a45669d2ec 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
@@ -1093,6 +1093,16 @@ static int vega12_upload_dpm_min_level(struct pp_hwmgr
*hwmgr)
return ret);
}
+ if (data->smu_features[GNLD_DPM_DCEFCLK].enabled) {
+ min_freq = data->dpm_table.dcef_table.dpm_state.hard_min_level;
+
+ PP_ASSERT_WITH_CODE(!(ret = smum_send_msg_to_smc_with_parameter(
+ hwmgr, PPSMC_MSG_SetHardMinByFreq,
+ (PPCLK_DCEFCLK << 16) | (min_freq &
0x))),
+ "Failed to set hard min dcefclk!",
+ return ret);
+ }
+
return ret;
}
@@ -1818,7 +1828,7 @@ static int vega12_force_clock_level(struct pp_hwmgr
*hwmgr,
enum pp_clock_type type, uint32_t mask)
{
struct vega12_hwmgr *data = (struct vega12_hwmgr *)(hwmgr->backend);
- uint32_t soft_min_level, soft_max_level;
+ uint32_t soft_min_level, soft_max_level, hard_min_level;
int ret = 0;
switch (type) {
@@ -1863,6 +1873,56 @@ static int vega12_force_clock_level(struct pp_hwmgr
*hwmgr,
break;
+ case PP_SOCCLK:
+ soft_min_level = mask ? (ffs(mask) - 1) : 0;
+ soft_max_level = mask ? (fls(mask) - 1) : 0;
+
+ if (soft_max_level >= data->dpm_table.soc_table.count) {
+ pr_err("Clock level specified %d is over max allowed
%d\n",
+ soft_max_level,
+ data->dpm_table.soc_table.count - 1);
+ return -EINVAL;
+ }
+
+ data->dpm_table.soc_table.dpm_state.soft_min_level =
+
data->dpm_table.soc_table.dpm_levels[soft_min_level].value;
+ data->dpm_table.soc_table.dpm_state.soft_max_level =
+
data->dpm_table.soc_table.dpm_levels[soft_max_level].value;
+
+ ret = vega12_upload_dpm_min_level(hwmgr);
+ PP_ASSERT_WITH_CODE(!ret,
+ "Failed to upload boot level to lowest!",
+ return ret);
+
+ ret = vega12_upload_dpm_max_level(hwmgr);
+ PP_ASSERT_WITH_CODE(!ret,
+ "Failed to upload dpm max level to highest!",
+ return ret);
+
+ break;
+
+ case PP_DCEFCLK:
+ hard_min_level = mask ? (ffs(mask) - 1) : 0;
+
+ if (hard_min_level >= data->dpm_table.dcef_table.count) {
+ pr_err("Clock level specified %d is over max allowed
%d\n",
+ hard_min_level,
+ data->dpm_table.dcef_table.count - 1);
+ return -EINVAL;
+ }
+
+ data->dpm_table.dcef_table.dpm_state.hard_min_level =
+
data->dpm_table.dcef_table.dpm_levels[hard_min_level].value;
+
+ ret = vega12_upload_dpm_min_level(hwmgr);
+ PP_ASSERT_WITH_CODE(!ret,
+ "Failed to upload boot level to lowest!",
+ return ret);
+
+ //TODO: Setting DCEFCLK max dpm level is not supported
+
+ break;
+
case PP_PCIE:
break;
@@ -1912,6 +1972,42 @@ static int vega12_print_clock_levels(struct pp_hwmgr
*hwmgr,
(clocks.data[i].clocks_in_khz / 1000 == now /
100) ? "*" : "");
break;
+ case PP_SOCCLK:
+ PP_ASSERT_WITH_CODE(
+ smum_send_msg_to_smc_with_parameter(hwmgr,
+ PPSMC_MSG_GetDpmClockFreq,
(PPCLK_SOCCLK << 16)) == 0,
+ "Attempt to get Current SOCCLK Frequency
Failed!",
+ return -EINVAL);
+ now = smum_get_argument(hwmgr);
+
+ PP_ASSERT_WITH_CODE(
+ vega12_get_socclocks(hwmgr, ) == 0,
+ "Attempt to get soc clk levels Failed!",
+ return -1);
+ for (i = 0; i < clocks.num_levels; i++)
+ size += sprintf(buf + size, "%d: %uMhz %s\n",
+ i, clocks.data[i].clocks_in_khz / 1000,
+
[PATCH 4/4] drm/amd/powerplay: support Vega12 retrieving and setting ppfeatures
Enable retrieving and setting ppfeatures on Vega12.
Change-Id: Idad5eaadbb9e7ea73edd9e9d4fe4e1a5b17fb7a6
Signed-off-by: Evan Quan
---
.../drm/amd/powerplay/hwmgr/vega12_hwmgr.c| 100 ++
1 file changed, 100 insertions(+)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
index 45a45669d2ec..342f8b81ca82 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
@@ -1933,6 +1933,104 @@ static int vega12_force_clock_level(struct pp_hwmgr
*hwmgr,
return 0;
}
+static int vega12_get_ppfeature_status(struct pp_hwmgr *hwmgr, char *buf)
+{
+ static const char *ppfeature_name[] = {
+ "DPM_PREFETCHER",
+ "GFXCLK_DPM",
+ "UCLK_DPM",
+ "SOCCLK_DPM",
+ "UVD_DPM",
+ "VCE_DPM",
+ "ULV",
+ "MP0CLK_DPM",
+ "LINK_DPM",
+ "DCEFCLK_DPM",
+ "GFXCLK_DS",
+ "SOCCLK_DS",
+ "LCLK_DS",
+ "PPT",
+ "TDC",
+ "THERMAL",
+ "GFX_PER_CU_CG",
+ "RM",
+ "DCEFCLK_DS",
+ "ACDC",
+ "VR0HOT",
+ "VR1HOT",
+ "FW_CTF",
+ "LED_DISPLAY",
+ "FAN_CONTROL",
+ "DIDT",
+ "GFXOFF",
+ "CG",
+ "ACG"};
+ static const char *output_title[] = {
+ "FEATURES",
+ "BITMASK",
+ "ENABLEMENT"};
+ uint64_t features_enabled;
+ int i;
+ int ret = 0;
+ int size = 0;
+
+ ret = vega12_get_enabled_smc_features(hwmgr, _enabled);
+ PP_ASSERT_WITH_CODE(!ret,
+ "[EnableAllSmuFeatures] Failed to get enabled smc
features!",
+ return ret);
+
+ size += sprintf(buf + size, "Current ppfeatures: 0x%016llx\n",
features_enabled);
+ size += sprintf(buf + size, "%-19s %-22s %s\n",
+ output_title[0],
+ output_title[1],
+ output_title[2]);
+ for (i = 0; i < GNLD_FEATURES_MAX; i++) {
+ size += sprintf(buf + size, "%-19s 0x%016llx %6s\n",
+ ppfeature_name[i],
+ 1ULL << i,
+ (features_enabled & (1ULL << i)) ? "Y"
: "N");
+ }
+
+ return size;
+}
+
+static int vega12_set_ppfeature_status(struct pp_hwmgr *hwmgr, uint64_t
new_ppfeature_masks)
+{
+ uint64_t features_enabled;
+ uint64_t features_to_enable;
+ uint64_t features_to_disable;
+ int ret = 0;
+
+ if (new_ppfeature_masks >= (1ULL << GNLD_FEATURES_MAX))
+ return -EINVAL;
+
+ ret = vega12_get_enabled_smc_features(hwmgr, _enabled);
+ if (ret)
+ return ret;
+
+ features_to_disable =
+ (features_enabled ^ new_ppfeature_masks) & features_enabled;
+ features_to_enable =
+ (features_enabled ^ new_ppfeature_masks) ^ features_to_disable;
+
+ pr_debug("features_to_disable 0x%llx\n", features_to_disable);
+ pr_debug("features_to_enable 0x%llx\n", features_to_enable);
+
+ if (features_to_disable) {
+ ret = vega12_enable_smc_features(hwmgr, false,
features_to_disable);
+ if (ret)
+ return ret;
+ }
+
+ if (features_to_enable) {
+ ret = vega12_enable_smc_features(hwmgr, true,
features_to_enable);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static int vega12_print_clock_levels(struct pp_hwmgr *hwmgr,
enum pp_clock_type type, char *buf)
{
@@ -2528,6 +2626,8 @@ static const struct pp_hwmgr_func vega12_hwmgr_funcs = {
.start_thermal_controller = vega12_start_thermal_controller,
.powergate_gfx = vega12_gfx_off_control,
.get_performance_level = vega12_get_performance_level,
+ .get_ppfeature_status = vega12_get_ppfeature_status,
+ .set_ppfeature_status = vega12_set_ppfeature_status,
};
int vega12_hwmgr_init(struct pp_hwmgr *hwmgr)
--
2.20.1
___
amd-gfx mailing list
